fix(jsonrpc): isolate malformed frames so one bad message can't cancel all in-flight requests

[chuwik]

Problem

The desktop app's model picker is empty on launch (github/app#836). The log shows the JSON-RPC read loop dying on a single bad frame and taking every concurrent request down with it:

ERROR jsonrpc_read_loop: error reading from CLI error=unexpected end of hex escape at line 1 column N
ERROR handlers::skills_config: failed to list global skills error=request cancelled
WARN  handlers::misc: failed to fetch account quota error=request cancelled
ERROR handlers::misc: failed to list models error=request cancelled

Root cause — blast radius, not partial-chunk framing

read_loop parsed each Content-Length frame inside read_message and treated a serde_json body-parse error identically to a fatal I/O error: it logged error reading from CLI, broke the loop, and then drained all pending requests. So one corrupt frame cancelled every in-flight request sharing the connection (list_models + list_global_skills + account.getQuota), leaving the picker empty.

Content-Length framing is honest: read_message reads the header, read_exact(length) assembles the complete body, then parses — so the reader is always byte-aligned to the next frame regardless of any single body's content. A body-level JSON error is therefore self-contained and must not tear down the shared connection. (The popular "a \uXXXX escape split across read chunks causes a partial parse" theory is a red herring — there is no partial-chunk parsing.)

Fix

Separate framing from parsing so a bad body fails only the implicated request:

• read_message → read_frame, returning the raw body bytes. I/O and Content-Length/protocol/framing errors still propagate as Err (fatal → break → reconnect); clean EOF at a frame boundary still returns Ok(None).
• read_loop parses the body itself. On Ok it dispatches as before; on a parse error it calls fail_unparseable_frame and continues instead of breaking.
• fail_unparseable_frame best-effort recovers the response id from the frame head (responses serialize id ahead of the truncated payload) and delivers a synthetic -32700 parse-error response to just that one awaiter, so it wakes immediately instead of hanging. Frames with no recoverable id (notifications, server requests) are dropped. A warn! records the serde error, frame_len, and recovered id.
• New extract_response_id scans the first ≤256 bytes; returns None if the frame is a notification/request ("method" present), else parses the integer after the first "id" key.
• New error_codes::PARSE_ERROR = -32700.

If framing were ever desynced by a dishonest CLI, the next read_frame hits a framing error → Err → fatal → break → reconnect, so the loop self-heals within ~1 frame. For the honest-framing reality, this is a strict improvement: one bad message is skipped and every other concurrent request succeeds.

Tests

• Inline unit tests for extract_response_id (truncated body, whitespace/error frames, notification → None).
• Integration test malformed_frame_fails_only_its_own_request: two concurrent requests share the read loop; the server replies to one with a correctly-framed-but-truncated body (ends mid-\u) and to the other with a valid response. Asserts the healthy request succeeds (not collateral-cancelled) and the malformed one resolves promptly with code -32700 (no hang).

Resolves github/app#836.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

This PR hardens the Rust JSON-RPC client read loop so that a single correctly-framed but malformed JSON body (e.g., truncated mid-escape) no longer tears down the shared connection and cancels unrelated in-flight requests—addressing the “empty model picker on launch” failure mode described in github/app#836.

Changes:

• Split framing from parsing (read_message → read_frame) so JSON parse failures can be handled per-frame without treating them as fatal transport errors.
• Add best-effort recovery for malformed response frames: attempt to extract the response id and fail only that pending request with JSON-RPC -32700 (parse error).
• Add unit tests for extract_response_id and an integration regression test ensuring one malformed response does not cancel other concurrent requests.

Show a summary per file

File	Description
rust/src/jsonrpc.rs	Refactors read loop to isolate malformed frames; adds parse-error code, id recovery, and associated unit tests.
rust/tests/jsonrpc_test.rs	Adds integration regression test ensuring malformed frames fail only their own request.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 1

[SteveSandersonMS]

@chuwik Do we know the root cause in the sense of why there was a "bad frame" in the first place? And what makes this Rust-specific?

I'd be reluctant to simply ignore bad data (even if logging a warning). The protocol relies on guaranteed delivery - any missed messages might brick the session, since the session may strictly require a response to a call. If we can explain why the bad frame is unavoidable then maybe it would be a justification for ignoring it, but also maybe not. Our transports are stdio or TCP, both of which provide their own delivery guarantees as long as the connection as a whole isn't dropped.

The issue on github/app says:

Reproduced on 21 of 21 recent launch logs.

... so it seems like this may be easy enough to repro.

[chuwik]

@chuwik Do we know the root cause in the sense of why there was a "bad frame" in the first place? And what makes this Rust-specific?

I'd be reluctant to simply ignore bad data (even if logging a warning). The protocol relies on guaranteed delivery - any missed messages might brick the session, since the session may strictly require a response to a call. If we can explain why the bad frame is unavoidable then maybe it would be a justification for ignoring it, but also maybe not. Our transports are stdio or TCP, both of which provide their own delivery guarantees as long as the connection as a whole isn't dropped.

The issue on github/app says:

Reproduced on 21 of 21 recent launch logs.

... so it seems like this may be easy enough to repro.

Sorry for the noise, agent researching decided to send a PR; it's not ready for review, though I appreciate your input! Still trying to figure out the root cause of the bad frame, user cannot repro anymore. Unless I find a smoking gun, I may leave as-is unless it comes back again.