SDK, Runtime: Recover JSON-RPC frames containing unpaired UTF-16 surrogates

[Chuxel]

Why

This PR fixes a problem that appeared in the github-app but appears to be a broader fix in the SDK.

Fixes github/app#678
Fixes github/app#1055

...and likely others.

The symptom is that the model picker in the App is permanently empty for some users, along with the quota counter and the agents/skills lists. All four report request cancelled, the failure is 100% reproducible across restarts, and re-authorization does not clear it.

Root cause appears to be a single unpaired UTF-16 surrogate in a CLI response (github/app#1055 has good data):

• serde_json correctly rejects lone surrogates per RFC 8259, failing with unexpected end of hex escape.
• The JSON-RPC read loop treats any decode failure as fatal: it breaks, then drains pending_requests, so every in-flight call resolves as RequestCancelled.
• Consumers that multiplex several RPCs over one client lose all of them at once. In the desktop app a single shared client carries models.list, account.getQuota, agents.discover and skills.discover, so one bad frame takes out all four.
• is_transport_failure() is true, so the consumer spawns a fresh CLI, which refetches the same payload and dies at the same byte offset. Deterministic, and it survives restarts — matching the reporter's "same column 256886 across every restart".

The CLI subprocess is the victim here, not the cause: its write EPIPE flood is just the SDK having closed the pipe after the parse error.

What changed

Repair unpaired surrogates to U+FFFD (matching JavaScript's String.prototype.toWellFormed) and retry the decode once.

Two properties keep this contained:

• The repair only runs after a strict parse has already failed. Well-formed frames never enter the repair path, so they are byte-identical to before and the happy path allocates nothing.
• Frames the repair cannot fix stay fatal, exactly as before. Without a trustworthy decode we cannot tell which pending request a frame belonged to, and silently dropping it would hang that caller forever — send_request_with_inline_callback awaits its oneshot with no transport-layer deadline. Those still return the original serde_json error, preserving the existing fast-fail-and-replace recovery.

Net effect: exactly one input class changes behavior — a frame whose only defect is an unpaired surrogate.

The scanner tracks string context and consumes non-\u escapes whole, so a literal C:\\ud83d is never mistaken for an escape prefix, and valid surrogate pairs pass through untouched.

Demo

rust/tests/jsonrpc_test.rs covers this end of it:

• the root-cause payload produces the exact error string from the issue logs
• a #1055-shaped frame is recovered and delivered with U+FFFD substituted, and the connection stays up for a subsequent request
• an unrepairable frame remains fatal — this guards the narrow scope above and fails if a caller is ever left hanging
• valid surrogate pairs and escaped backslashes are untouched, verified in a frame that also contains a lone surrogate so the repair path genuinely runs

Reverting the fix while keeping the tests fails lone_surrogate_frame_is_recovered_without_closing_connection and valid_pairs_and_escaped_backslashes_are_untouched, both unwrapping Protocol(RequestCancelled).

The fix was also validated against the real desktop app and real CLI subprocesses before being brought upstream, by splicing a lone surrogate into live models.list frames via a temporary debug-only injector (never committed).

Same injected payload, two builds:

	repair disabled	repair enabled
ERROR ... error reading from CLI	4	0
failed to list models / fetch account quota	yes	0
request cancelled	yes	0
connection lost / client replaced	yes	0
frames recovered	—	3
CLI respawns	repeated	0 (one pid throughout)

With the fix, a single CLI pid survived all three corrupted frames, across two different RPCs on the shared channel:

WARN jsonrpc_read_loop: recovered JSON-RPC frame containing unpaired UTF-16 surrogates
     error=unexpected end of hex escape at line 1 column 72  length=25422

The original serde_json message is preserved in the error= field, so the diagnostic is kept rather than swallowed.

Notes

• Review focus: repair_lone_surrogates. It is a hand-rolled byte scanner on the JSON-RPC read path, so the string-context and backslash-parity handling is where a bug would hide. It only ever runs on already-failed frames.
• This is client-side hardening. The real fix belongs with whatever produces the lone surrogate — it should not reach the wire — but the SDK should not lose an entire CLI connection over one bad character. The producer path is not in this repo, so its origin could not be established here.
• Adjacent issue found while investigating, deliberately not addressed here: send_request_with_inline_callback has no timeout, so any lost response hangs its caller indefinitely. That is the reason unrepairable frames must stay fatal, and it is worth revisiting on its own.
• Other language SDKs are out of scope. JavaScript's JSON.parse accepts lone surrogates natively so the TS SDK likely never had this bug; the strict-parser SDKs should be assessed separately rather than receiving unverified cross-language changes.
• github-app vendors this crate, so the fix reaches the desktop app via scripts/sync-copilot-sdk.sh once this merges.

[Copilot]

Pull request overview

Hardens Rust JSON-RPC decoding against unpaired UTF-16 surrogates without disrupting valid frames or masking other parse failures.

Changes:

• Repairs lone surrogate escapes to U+FFFD after strict decoding fails.
• Preserves fatal handling for unrecoverable frames.
• Adds integration coverage for recovery, connection continuity, valid pairs, and escaped backslashes.

Show a summary per file

File	Description
rust/src/jsonrpc.rs	Adds targeted surrogate repair and decode retry.
rust/tests/jsonrpc_test.rs	Verifies repair behavior and transport lifecycle.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Balanced

[Copilot]

Review details

Suppressed comments (1)

rust/src/jsonrpc.rs:224

• The low-surrogate branch is not covered: every malformed test uses the high surrogate \ud83d. Because the function promises to repair all unpaired surrogates and standalone low halves take a distinct path that never enters is_pair, add a case for \udc00–\udfff that verifies replacement and continued connection usability.

                if (0xD800..0xE000).contains(&unit) {
                    let output = repaired.get_or_insert_with(|| body.to_vec());
                    output[index..index + 6].copy_from_slice(br"\ufffd");
                }

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Balanced