ENG-981: heal lone surrogates before compile() in the scratchpad

[alecantu7]

What

The scratchpad child crashes at compile(code, "<scratchpad>", "exec") (scratchpad_boot.py) when the cell source contains a lone surrogate (\udcXX). Heal the source before compile.

Why (traced to source, 3 real desktop users)

A non-ASCII Windows path byte — Área de Trabalho (sabrina), an emoji filename (eddie), and tim (Tech With Tim, dogfooding creator) — is surrogate-escaped upstream and passed through the belt's lenient surrogateescape stdin, becoming a lone surrogate in code. compile() strict-encodes the source and rejects it → surrogates not allowed, killing the cell before any user code runs.

Distinct from ENG-824 / ENG-940, and neither fixes it:

• ENG-824 belt (PYTHONUTF8) enables it — the lenient stdin creates the surrogate that strict compile() rejects.
• ENG-940 adds encoding="utf-8" to chat.py reads — different file/channel; strict UTF-8 wouldn't pass a lone surrogate anyway.

How

heal_surrogate_source() in wire.py (shared by child + parent), called in scratchpad_boot.py right before compile():

• re-encode via surrogateescape → decode UTF-8: reassembles the correct character when the surrogates are the byte-escaped halves of a real multibyte char (the common path case — cell compiles and opens the right file);
• falls back to a replacement-char scrub for a genuinely-lone surrogate (compile succeeds instead of crashing);
• no-op on clean source.

This is the recover variant, chosen over the ticket's suggested surrogatepass→replace scrub because the scrub corrupts recoverable paths into mojibake (verified) — recover makes the task actually work.

Tests (14/14 green)

• New ENG-981 coverage: recover a byte-escaped Á path and an escaped emoji (assert exact reconstruction + compiles); scrub a truly-lone surrogate (no \udcXX, compiles); no-op on clean source; premise guard (bare compile() on a lone surrogate still raises).
• Corrected two ENG-940 (ENG-940: explicit encoding="utf-8" on scratchpad/chat reads (launcher-independent UTF-8) #263) tests that overclaimed: _encode_cell_payload's surrogateescape transport only makes the pipe lossless — it does not fix the compile() crash. Relabeled to pin transport fidelity only.

Security

No new surface — the change only normalizes the encoding of already-received cell source before compilation; no new input, no trust-boundary change.

Delivery

Targets staging. The fix site in scratchpad_boot.py is stable across main/staging, so this cherry-picks cleanly to a main hotfix PR to follow (ENG-981 is Urgent/Customer; desktop users bleeding). Deploy caveat: reaching sabrina/eddie/tim needs a desktop app build from the release ref, not just the branch merge.

Related: ENG-824 (belt), ENG-940 (read encoding), ENG-980 (churn tracker).

[alecantu7]

Self-review

Test status — one thing reviewers should know: the two spawn-the-child e2e tests in test_chat_scratchpad.py (test_scratchpad_exec_via_chat, test_scratchpad_in_streaming_path) fail in my local env — but they fail identically on clean origin/staging (verified), so they're pre-existing/environmental (the subprocess can't spawn here), not a regression from this PR. Consequence: I unit-verified heal_surrogate_source thoroughly (14/14 in test_scratchpad_utf8.py, incl. the 3 real crash shapes → compile OK) but could not exercise the full spawned-child → compile() path locally. Worth confirming in CI / QA on a real environment.

Adversarial pass — no functional findings:

• Runs on every cell, but the any(surrogate) guard makes it a cheap scan + early-return for clean source (the hot path); only surrogate-bearing cells hit encode/decode.
• Can't corrupt legitimate code: a real lone surrogate in source never compiles anyway, so healing it is strictly better than crashing; a properly-escaped \udc81 literal is ASCII in the source and doesn't trip the guard.
• Fallback scrub leaves no residual surrogate (surrogatepass-encode → replace-decode → U+FFFD), asserted in test_heal_scrubs_truly_lone_surrogate_so_compile_succeeds.
• Composes with _encode_cell_payload's surrogateescape transport: byte-escaped pairs already heal at stdin decode (no-op here), lone bytes get scrubbed here — no double-processing hazard.

Requesting review.

[alecantu7]

Hard review — findings

F1 · Correctness (fixed, c4a022b). The recover path used a strict decode + full surrogatepass scrub fallback. In a mixed cell — a recoverable byte-escaped path and an unrelated lone byte — the strict decode raised on the lone byte, so the whole source fell to the scrub and the recoverable Área was mojibako'd too. Verified: p="C:/\udcc3\udc81rea"; q="\udc81" → strict+scrub gives C:/������rea, escape+replace gives C:/Área + one U+FFFD. Switched the recover decode to errors="replace" (heals recoverable chars and scrubs strays in one pass); surrogatepass fallback now only guards the encode-raise for high/unpaired surrogates. Added test_heal_preserves_recoverable_char_in_mixed_cell.

F2 · Coverage gap (flagging, not fixed). No test exercises the actual scratchpad_boot.py call site — the unit tests cover heal_surrogate_source in isolation, and the spawn-the-child e2e tests are broken in my local env (pre-existing). So the wiring (boot calls heal before compile; a real spawned child recovers a mangled-path cell end-to-end) is verified only by reading the diff. Recommend CI runs the e2e / a boot-integration test before we rely on this in prod.

F3 · Behavior note (accepted). A genuinely-lone/high surrogate degrades to U+FFFD, so the cell runs with a scrubbed char in that spot — for a path that surfaces as a clear FileNotFoundError, not silent success. Only affects unrecoverable cases that couldn't compile at all otherwise. Acceptable; documented in the docstring.

Verified good (no action): the function is total (never raises — critical for a boot script), leaves no residual surrogate (compile always succeeds), preserves real text, and composes with _encode_cell_payload's surrogateescape transport without double-processing. Guard covers the full U+D800..U+DFFF range.

F4 · Design note (kept as-is). Heals proactively via a per-cell char-scan guard rather than reactively (try compile / except UnicodeEncodeError → heal → recompile). Reactive would be zero-cost on the clean hot path, but entangles with the existing except-block; for a hotfix I kept the isolated proactive heal — the guard early-exits and the scan cost is negligible vs compile()+exec() itself.

[pnewsam]

Code Review

Verdict: REQUEST_CHANGES

The normal recovery path and compile-site wiring look sound, and CI run 837 passed. However, the fallback for surrogates outside DC80–DCFF applies surrogatepass to the entire cell, destroying otherwise recoverable UTF-8 path bytes elsewhere in that cell. This is the same mixed-cell data-loss class the latest commit intends to fix.

Summary of findings

Severity	File	Issue
Major	anton/core/backends/wire.py:43	Unsupported surrogate fallback corrupts recoverable surrogate-escaped characters elsewhere in the cell
Minor	tests/test_scratchpad_utf8.py:153	Comment incorrectly names surrogatepass instead of surrogateescape

What's working well

The clean-source fast path is appropriately cheap, healing occurs immediately before compile(), and the focused recovery tests cover accented and emoji UTF-8 sequences. CI is green.

The author’s noted boot-integration coverage gap remains a non-blocking deployment risk.

[pnewsam]

Major: This fallback corrupts recoverable surrogate-escaped UTF-8 elsewhere in the same cell.

For example, p="C:/\udcc3\udc81rea"; q="\ud800" enters this branch because of \ud800. Applying surrogatepass to the entire source turns the recoverable \udcc3\udc81 into replacement characters too, producing C:/������rea. This is the same mixed-cell data-loss class the latest commit intends to prevent.

Please scrub unsupported surrogates before doing the surrogateescape round trip, so DC80–DCFF sequences remain recoverable, and add this mixed-range regression case.

[pnewsam]

Minor: This comment says surrogatepass keeps the parent encoding alive, but _encode_cell_payload() uses surrogateescape, and the newly added explanation below correctly says surrogatepass breaks the round trip. Please update the stale wording.

[alecantu7]

@pnewsam thanks — both addressed in 9e74c60.

Major — you're right, real data loss I missed. A high/unpaired surrogate (outside DC80..DCFF, e.g. \ud800) makes the surrogateescape encode raise, so my except sent the whole cell to the surrogatepass scrub — mojibako-ing the recoverable Á too. My earlier errors="replace" fix only covered strays that fail on decode; yours fails on encode. Same data-loss class, other branch.

Fix is your suggestion — scrub the unmappable surrogates up front, then run the surrogateescape+replace recover on what's left. Bonus: with no surrogate outside DC80..DCFF reaching the encode, it can't raise, so the try/except is gone entirely. Verified your exact case p="C:/\udcc3\udc81rea"; q="\ud800" → C:/Área + one U+FFFD (was C:/������rea). Added test_heal_preserves_recoverable_char_beside_unmappable_surrogate.

Minor — fixed. Rewrote the stale ENG-940 transport-test header comment; it no longer misnames surrogatepass / overclaims a host-side crash.

16/16 green; function still total + no-residual on the full soup. Re-requesting review.

[pnewsam]

Code Review

Verdict: APPROVE

Re-reviewed at 9e74c60. Both previous findings are fully addressed: unmappable surrogates are now scrubbed before the surrogateescape recovery pass, preserving recoverable DC80–DCFF sequences elsewhere in mixed cells, and the stale transport documentation has been corrected.

No issues found. Code looks good to merge.

What's working well

The new regression test covers the exact mixed-range failure case. The implementation is total across the surrogate range, preserves clean input, leaves no residual surrogates, and CI run 838 is green.