Prevent RPC session crashes on concurrent multi-image traffic

[aebrer]

Closes #495

A dashboard-driven RPC session can be force-killed by the 16 MiB stdout backpressure guard when multiple images are in flight simultaneously — both 2+ image prompt uploads and turns where the agent reads 2-3+ images quickly. Image base64 crosses the child stdout multiple times per turn (prompt re-emission, 3× per read-tool result, full agent_end transcript), and the dashboard's single-threaded consume stalls long enough for the queue to pass the cap.

Implementation plan posted as a comment below.

[aebrer]

Implementation Plan

Problem analysis

A dashboard session (RPC child process) dies when multiple images are in flight simultaneously. The death is always the 16 MiB stdout backpressure guard (output-guard.ts, added to prevent the unbounded output queues of issue 448): while the dashboard process is busy — synchronous JSONL parsing of multi-MiB lines, strict base64 decode + re-encode + SHA-256 per image block, preview decoding, full-transcript re-fetches — the child's stdout backpressures, and queued writes past 16 MiB trip process.exit(1) mid-turn.

Legitimate multi-image traffic exceeds the cap because:

1. Images cross stdout multiple times per turn. A prompt with N attached images is re-emitted in message_start + message_end (the agent loop re-emits prompt messages); each image-read tool result appears in tool_execution_end + message_start + message_end; and agent_end re-sends the whole turn's messages array. A turn reading 3 resized images is roughly 54 MiB of raw base64 on the pipe in one burst.
2. The dashboard is a slow single-threaded consumer. Every inline image block in every event is strictly decoded, re-encoded, and hashed at the server projection boundary. In originals display mode the browser auto-fetches every tool-result image, and every cache miss triggers a full-transcript getMessages() round trip that re-transfers every image in the transcript — concurrent misses each trigger their own full re-fetch (original() has no single-flight; only previews do).
3. The guard cannot distinguish slow-but-alive from dead. It exits immediately once the queue passes 16 MiB while backpressured, even though the consumer is actively draining.

Secondary items called out in the issue: the "Fatal: stdout write queue exceeded..." diagnostic goes to child stderr only, while formatRpcExit surfaces just code/signal (opaque RPC process exited (code 1, signal null)); and RpcClient writes child stdin with no 'error' listener, so a large prompt line racing a dying child can EPIPE and take down the dashboard process itself (all sessions).

Deliverables

1. Child-side image dedupe (dashboard uiType only) — in the RPC event projection path (rpc-event-projection.ts + rpc-mode.ts): each unique image's base64 crosses stdout at most once per child process lifetime. Later occurrences (across all live events: prompt re-emission, tool results, agent_end transcript, nested background_agent_event) are replaced with {type:"image_reference", id, mimeType, size} where id = sha256(mimeType + 0x00 + decodedBytes) — byte-identical to the dashboard's existing dashboardImageId, so the dashboard's cache/LRU/recovery pipeline works unchanged (it already passes image_reference through and caches the first inline occurrence). Live events only — get_messages/snapshot responses keep full base64 (they are the authoritative source the dashboard's recoverOriginal depends on after cache eviction or dashboard restart).
2. Backpressure grace in the stdout guard (output-guard.ts) — the fatal exit fires only when the queue is over 16 MiB and stdout has made no drain progress for a grace window (default 30 s, overridable in tests). A slow-but-alive consumer drains → the session survives; a dead or stalled-beyond-grace consumer still gets the loud abort. The guard's original intent (no unbounded growth against a dead consumer) is preserved.
3. Single-flight original recovery (dashboard-images.ts) — concurrent cache-miss original() calls for the same scope+id share one authoritative getMessages() load (mirroring the existing preview-flight map), collapsing the originals-mode auto-fetch storm from N full-transcript re-fetches to 1.
4. Exit observability (rpc-client.ts, runtime-pool.ts) — the non-spawn-error RpcExitInfo variant carries a truncated stderr tail; formatRpcExit appends it so the surfaced error reads like RPC process exited (code 1, signal null). Stderr tail: Fatal: stdout write queue exceeded ....
5. RpcClient pipe safety (rpc-client.ts) — 'error' listeners on the child's stdin/stdout/stderr pipes (fail pending requests / mark the client dead / accumulate into the stderr buffer) so an EPIPE on a dying child's stdin cannot kill the dashboard process via an unhandled 'error' event.
6. Tests — see test plan; every new behavior is covered.
7. Docs — packages/coding-agent/docs/dashboard.md (Transcript images section: child-side dedupe + guard grace); one-line note in packages/coding-agent/docs/rpc.md if dashboard event projection is documented there. No root README change (behavioral bug fix, no feature surface change).

Acceptance criteria → how they are met

• 2+ image prompt survives: prompt-image re-emission is deduped to one copy per image (halves that burst); when the queue still exceeds 16 MiB, the grace window keeps the child alive while the dashboard drains. Covered by the regression test (prompt with images under simulated backpressure).
• 2-3+ quick image reads survive: tool-result copies go 3×→1× (plus agent_end transcript → references); the grace window covers the residual burst; originals-mode auto-fetch no longer multiplies full-transcript re-fetches (single-flight). Covered by the regression test (multi-image tool results under simulated backpressure).
• Automated regression test: a new child-side stack test drives runRpcMode against the faux harness (real agent loop, real output-guard, custom faux tool returning 4 × ~4 MiB images; consumer backpressured, then drains) and asserts: no process.exit, every frame delivered in order, each unique base64 serialized exactly once, later occurrences are valid image_references. Dashboard-side single-flight and exit-tail behavior get unit coverage.
• No display regression: round-trip assertion — deduped frames projected through the dashboard server's DashboardImageService yield the same image_reference set as projecting the original inline frames (first occurrence always inline; ids byte-identical to dashboardImageId). Non-dashboard RPC (uiType ≠ dashboard) is untouched.

Files to create or modify

File	Change
packages/coding-agent/src/modes/rpc/rpc-event-projection.ts	Stateful image dedupe: factory wrapping the existing per-event projection with a per-process seen-id set; recursive walk (incl. nested background_agent_event), non-mutating (shallow copies, matching existing style); gate: allowlisted mimeType (png/jpeg/gif/webp) + base64-decodable block → image_reference; first occurrence kept inline
packages/coding-agent/src/modes/rpc/rpc-mode.ts	Instantiate the dedupe for uiType === "dashboard" and apply it in the session.subscribe output path (non-dashboard unchanged)
packages/coding-agent/src/core/output-guard.ts	Drain-progress grace before the fatal path; updated diagnostic wording; grace window overridable for tests
packages/coding-agent/src/modes/rpc/rpc-client.ts	Stderr tail on exit info (truncated); 'error' listeners on child stdin/stdout/stderr
packages/dashboard/src/server/dashboard-images.ts	Scope-bound single-flight map for original() recovery
packages/dashboard/src/server/runtime-pool.ts	formatRpcExit appends the stderr tail when present
packages/coding-agent/test/rpc-event-projection.test.ts	Extend: dedupe behavior (first occurrence inline, later references, id correctness, non-mutation, nested events, non-dashboard passthrough)
packages/coding-agent/test/output-guard.test.ts	Extend: over-cap + drain within grace → survives; over-cap + no drain for the grace window → still fatal
new packages/coding-agent/test/rpc-mode-multimage-survival.test.ts	The regression test (separate file: output-guard queue state is process-global)
packages/coding-agent/test/rpc-client-spawn.test.ts (or small new file)	Pipe 'error' handling: pending requests fail, no unhandled error event
packages/dashboard/test/dashboard-images.test.ts	Extend: concurrent original() misses share one authoritative load; scope isolation
packages/dashboard/test/runtime-pool.test.ts	Extend: exit error message includes stderr tail
packages/coding-agent/docs/dashboard.md (± rpc.md)	Document child-side dedupe + guard grace in the Transcript images section

Test plan

1. Survival regression (core) — new file, runRpcMode + faux harness (custom faux tool, dashboard uiType):
   • Prompt with 2 attached images + a turn whose faux tool returns 4 distinct ~4 MiB images (well over the 16 MiB cap with JSON overhead headroom); raw stdout stub keeps returning false (backpressured) until the test flips it mid-turn.
   • Assert: process.exit never called (spy); all event frames captured in order; each unique base64 appears in exactly one serialized frame; later occurrences are image_references with ids equal to sha256(mime+0x00+bytes).
   • Old code fails this test (exits at 16 MiB); passes with dedupe + grace.
2. Projection unit tests — dedupe: first-occurrence rule across event types (prompt re-emission, tool_execution_end, message_start/message_end, agent_end, nested background_agent_event); id formula; non-mutation; unknown/future event types untouched; non-dashboard uiType passthrough (no dedupe).
3. Guard unit tests — over-cap with drain within grace → no exit, queue drains in order; over-cap with zero drain for the grace window → diagnostic + exit (existing behavior preserved); grace resets on progress.
4. Dashboard unit tests — original() single-flight (two concurrent misses → one loadAuthoritative call, both resolve; distinct scopes don't share); formatRpcExit tail surfacing (truncated, code/signal preserved).
5. Round-trip (display safety) — cross-package test under packages/dashboard/test/: feed deduped frames through DashboardImageService.project and assert the resulting references are identical to projecting the original inline frames.
6. Run npm test (all workspaces) + npm run build before manual verification; manual check: dashboard with 2+ attached images and a turn reading 3 images, in each image display mode (placeholders/previews/originals).

Risks and open questions

• Id formula duplication — sha256(mime + 0x00 + bytes) is duplicated from the dashboard into the coding-agent package (dependency direction forbids importing dashboard code). Mitigated by the round-trip test asserting the dashboard accepts the child's references and resolves them from cache.
• Invalid-image corner — dedupe is gated on allowlisted mimeType + base64-decodable (no raster-signature check on the child). For a corrupt image block: the first occurrence is still delivered inline (the dashboard drops it exactly as today); later duplicates become references → 404 instead of silent DROP. Accepted corner; real tool results pass magic-byte detection in the read tool.
• Memory during a stall — while a slow-but-alive consumer stalls, the queue may exceed 16 MiB until it drains (bounded by stall duration × production rate). The issue-448 death path remains for genuinely dead consumers after the 30 s grace.
• 30 s grace side effect — dev pauses (breakpoints/inspector) longer than 30 s now survive instead of aborting; desirable.
• Out of scope → separate issues: dashboard-side uncaughtException/unhandledRejection handlers; unbounded stdin JSONL line buffering in jsonl.ts (practically bounded by the 25 MiB body cap); base64/mime validation of prompt images on the RPC child side (the child currently trusts the dashboard, which shape-checks); reference-based bounding of get_messages responses.

---

Plan created by mach6

[github-actions]

Vitest coverage

Metric	Covered	Total	Coverage
Statements	40808	56724	71.94%
Branches	22154	35715	62.02%
Functions	8653	11891	72.76%
Lines	29487	40783	72.3%

View full coverage run

[aebrer]

Progress Update

Implementation of the plan is complete. All five levers are in, with tests and docs.

What changed (18 files):

1. Child-side image dedupe (dashboard mode only) — rpc-event-projection.ts adds a stateful createDashboardRpcEventProjector() that content-addresses inline image blocks (sha256(mimeType + 0x00 + decodedBytes)) and replaces every re-occurrence with a small image_reference frame. Each unique image's base64 now crosses the child stdout pipe at most once per process lifetime; later occurrences (prompt re-emission, tool-result re-delivery, agent_end transcript) are tiny references the dashboard resolves from its existing image cache. Non-allowlisted/malformed blocks stay inline and are rejected by the dashboard's strict decode as before. Command responses (get_messages, get_dashboard_snapshot) are untouched.
2. stdout guard drain-progress grace — output-guard.ts no longer kills the child the instant the 16 MiB queue cap is exceeded. Over-cap now arms a 30 s no-drain window; any drain progress disarms it, so a slow-but-alive dashboard consumer survives multi-image bursts. A truly dead consumer is still aborted loudly after the grace window.
3. Single-flight original() recovery — dashboard-images.ts coalesces concurrent cache-miss original() calls for the same scope+id into one authoritative getMessages() load (previously N concurrent misses each triggered a full re-fetch over the same pipe).
4. stderr tail on exit — RpcClient captures the last 2000 chars of child stderr and includes it in the exit notification; runtime-pool.formatRpcExit surfaces it, so a guard abort shows its real reason instead of a bare exit code.
5. RpcClient pipe 'error' safety — stdin/stdout/stderr pipes now have error listeners that record the cause and fail in-flight requests, so an EPIPE from a dying child no longer crashes the dashboard host (which would kill every session at once).

Tests: new multi-image survival regression test (real runRpcMode + real guard, 6×4 MiB images through a backpressured-then-draining stdout), 3 new output-guard grace tests, 3 new RpcClient pipe-error tests + stderr-tail exit test, 3 new dashboard single-flight tests, and a new cross-package round-trip test proving child-emitted references resolve from the dashboard cache without the authoritative reload path (including an id-formula parity pin). Updated existing tests for the new exit-info shape.

Docs: root README, coding-agent README, docs/dashboard.md (dedupe + guard grace), docs/rpc.md (dashboard-mode wire projection image dedupe + process exit/pipe-error section).

Verification: full workspace test suite green (npm test, 5967 passed / 0 failed), tsgo --noEmit clean, biome check clean.

Commit: 62fe217

---

Progress tracked by mach6

[aebrer]

Progress Update

Ignore dashboard runtime data under .dreb* in .gitignore. The dashboard session screen uploads attached files into a .dreb-dashboard-uploads/ directory at the workspace root (see UPLOAD_DIR_NAME in packages/dashboard/src/client/screens/session.tsx), so these artifacts should not appear as untracked/commit-able files in the working tree. The new pattern also subsumes the previous **/.dreb/ entry (which is replaced).

Commit: 956210c6bbd1421e6f4899d6c534e4486e15353d

---

Progress tracked by mach6

[aebrer]

Unverified Review Candidates — Pending Assessment

Review round: 1
Reviewed commit: 956210c

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

(none)

Important

F1 — Child-side dedupe identity is looser than the dashboard's strict decode → dangling image references (confidence 88/100; corroborated by a second specialist, empirically verified)

rpc-event-projection.ts (imageBlockIdentity) accepts any allowlisted-MIME block whose base64 is well-formed (length % 4, alphabet, non-empty decode). The dashboard's decodeDashboardImage is stricter: it additionally requires round-trip base64 canonicality and a raster signature (PNG/JPEG/GIF/WebP magic). For a block the child accepts but the dashboard rejects — e.g. JPEG bytes named .png (no signature/mime validation anywhere on the upload or child-ingestion path), or a truncated PNG read by the read tool:

1. First occurrence crosses stdout inline; the dashboard strict-decode drops it — nothing cached, no id in the image map.
2. Every later re-emission (message_end, tool-result re-delivery, agent_end transcript) becomes an image_reference because the child's loose check passed.
3. The reference can never resolve: cache miss → authoritative getMessages() re-decode also fails strict decode → DashboardImageNotFoundError → 404 on the image route.

Pre-PR such blocks were silently dropped at every occurrence (no element, no request); post-PR the transcript renders a broken-image slot per dangling reference, and each render triggers a full authoritative transcript round trip over the same pipe — re-triggering the multi-MiB re-fetch bursts this PR exists to eliminate (N corrupted uploads → N full-transcript fetches per view; no negative caching). Also contradicts the PR docs' claim that such blocks "stay inline and are rejected … exactly as before" — true only for occurrence 1. Fix direction: make the child's gate as strict as the dashboard's (round-trip + signature), or cache negative entries on the dashboard so references to dropped ids render absent.

F2 — Partial drain (progress while still over cap) is untested, and the existing "slow consumer survives" test is provably masked (confidence 92/100, empirically verified)

No test covers the over-cap + partial drain scenario: no-drain window armed, a drain fires but only some bytes are accepted so the guard queue stays over 16 MiB, and the window must be reset because progress was made. The PR's only over-cap survival test is a full drain (queue to zero). Verified by experiment: deleting disarmNoDrainAbort() from the drain handler still passes the existing full-drain test (the defensive early return in abortForStalledConsumer — queue ≤ cap → return — masks the missing reset), while a partial-drain variant (30 × 1 MiB queued, 5 accepted, 25 MiB still queued) kills at t=30s on the regressed module and survives on the current one. A regression removing/weakening the drain-disarm — reintroducing exactly the issue 495 death for the dashboard's burst-draining decode loop — would ship green.

F3 — AC1's "the session remains usable" is unasserted, and the child-stdin half of the prompt path is bypassed (confidence 90/100)

The new survival test makes exactly one prompt, drains, and ends: no follow-up prompt is ever sent, so issue 495 acceptance criterion 1's "the session remains usable" is not exercised. A regression that leaves guard state dirty after a burst (residual queued bytes, a stuck backpressure flag, or a leftover armed timer) would kill or stall the next prompt — invisible to a test that stops at the drain. Also, the test mocks attachJsonlLineReader and calls session.prompt() directly, so the child-stdin JSONL boundary (the dashboard writing a ~10 MiB prompt line with 2+ prompt images into the child) is only exercised at byte scale elsewhere.

Suggestions

F4 — dashboard.md claims the child stdout queue "caps at 16 MiB" — it no longer does (confidence 90/100) — packages/coding-agent/docs/dashboard.md (Events section): "its stdout write queue caps at 16 MiB, and a burst past the cap aborts the child only after 30 seconds without drain progress" is self-contradictory; enqueueStdout deliberately keeps queuing past 16 MiB for a slow-but-alive consumer — 16 MiB is now the threshold that arms the no-drain window, not a hard cap. The root README wording ("the child's 16 MiB stdout queue aborts only after 30 s without drain progress") is accurate; dashboard.md is the outlier.

F5 — AC4's "images continue to be delivered to the model" has no assertion (confidence 90/100) — the survival test never inspects harness.faux.contexts; nothing pins that the prompt-image blocks (full base64, not references) actually reached the model's context. One expect over the last captured context would close it.

F6 — Boundary and defensive branches with zero coverage (confidence 88/100) — (a) queue exactly at the 16 MiB cap: a > → >= flip (arming the kill window at exactly cap) would be invisible; (b) STDERR_TAIL_CHARS truncation never exercised with a >2000-char stderr; (c) onPipeError's stale-process guard (this.process !== procRef) untested; (d) formatRpcExit with an empty-string tail never asserted at the pool level.

F7 — original() re-copies preview()'s single-flight dance (confidence 88/100) — the new originalFlights get → join → set → await/finally-delete sequence in dashboard-images.ts is line-for-line the pattern preview() already implements with previewFlights; a small private singleFlight helper would govern both maps (~10 duplicated lines removed).

F8 — abortForStalledConsumer's under-cap early return is unreachable (confidence 82/100) — the backlog can only decrease via the drain listener, which disarms the timer before flushing, so the timer can never fire while under cap. It is a defensive net on a process-kill path — keep-or-remove should be a deliberate decision rather than an apparent handled case no test exercises.

F9 — Unrelated .gitignore change bundled into the PR (confidence 95/100, scope note) — commit 956210c changes **/.dreb/ → .dreb*. Maps to no issue criterion or plan deliverable; verified harmless (strict superset covering the dashboard's .dreb-dashboard-uploads runtime dir; zero tracked paths affected). Flagged so the PR's scope is explicit, not to block it.

F10 — Root README edited despite the plan's "No root README change" (confidence 90/100, scope note) — the added sentence is factually accurate against the code and the repo's development guide mandates root-README accuracy for feature changes; noted for completeness, no action needed.

Strengths

• Cross-boundary id parity is pinned by test, not assumption. The round-trip test composes the real child projector with the real DashboardImageService and asserts child-emitted references carry exactly the ids the dashboard assigns (down to re-running dashboardImageId as oracle), with a mustNotLoad bomb proving cache resolution without the authoritative reload.
• Real end-to-end regression test. Real runRpcMode + real output guard under permanent backpressure: no exit, in-order frame delivery, each of six 4 MiB payloads crossing stdout exactly once, and an agent_end transcript carrying only stable references.
• Grace window is race-safe by construction. Singular unref'd timer, idempotent arm, disarm on any real drain, re-verify backlog still over cap before killing, stderr diagnostic + 1 s force-exit backstop on the kill path.
• Pipe-error handling is correctly scoped. Stale-procRef guard, idempotent failPendingRequests, exit handler remains sole owner of exit notifications — the EPIPE-dying-child crash is fixed without conflating the three failure channels.
• The dedupe projector preserves the module's non-mutation / by-reference contract. Image-free events pass through by reference with zero allocation; first occurrences are never mutated.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[aebrer]

Review Assessment

#496 (comment)

Classifications

Finding	Classification	Reasoning
F1 — child dedupe identity looser than dashboard strict decode → dangling references	useful follow-up	Factual: PASS — imageBlockIdentity (no round-trip canonicality, no raster signature) accepts a strict superset of decodeDashboardImage; chain verified end-to-end incl. client 404 + one full getMessages() re-fetch per remount/retry. Scope: PASS — PR-introduced delta: pre-PR every occurrence was dropped silently; post-PR occurrence 2+ becomes an unresolvable reference; the docs omit this bucket entirely (they scope "as before" to non-allowlisted/non-canonical-base64 blocks, where they are accurate). Practical: PASS, corner — mislabeled (JPEG as .png) or truncated raster: a new broken-image card with manual retry, where the image was silently absent pre-PR; no crash, no valid-image impact, client degrades gracefully, and the failing inputs are ones the dashboard deliberately rejects by design.
F2 — partial drain untested; slow-consumer test provably masked	useful follow-up	Factual: PASS — proven by experiment: deleting disarmNoDrainAbort() from the drain handler still passes the existing full-drain test (the defensive under-cap early return in abortForStalledConsumer masks the missing reset); no test drives a partial drain. Scope: PASS — tests are plan item 6; the gap could let a silent return of issue 495 ship green. Practical: PASS (missing-test rubric) — the regression this gap hides is exactly the death this PR removes: a slow-but-alive consumer killed 30 s into a long multi-image turn, with no CI signal.
F3 — AC1 "session remains usable" unasserted; child-stdin boundary bypassed	useful follow-up	Factual: PASS — handleInputLine is captured but never invoked; no second prompt after the burst. Scope: PARTIAL — the bypassed JSONL ingress is pre-existing code untouched by this PR. Practical: WEAK — residual risk ≈ 0: the #495 crash mechanism is on the stdout side and is fully covered (exit spy, ordered frames, >16 MiB backlog verified); session.prompt resolving already proves the turn completes.
F4 — dashboard.md "caps at 16 MiB" wording	useful follow-up	Factual: PASS — the Events-section sentence is self-contradictory; the code deliberately keeps queuing past 16 MiB for a slow-but-alive consumer (the PR's own survival test asserts backlog > 16 MiB). Scope: PASS — docs are plan item 7; this sentence is added by this PR. Practical: WEAK — docs only; an operator would wrongly believe child memory is hard-capped at 16 MiB.
F5 — no assertion that images reach the model	nitpick	Factual: PASS. Scope/Practical: FAIL — the projector is provably pure and non-mutating, confined to the subscriber output path; model delivery is structurally untouched; no residual risk to close.
F6 — boundary/defensive branches with zero coverage	nitpick	Factual: mostly PASS — sub-claim (d) is near-false: the falsy-tail branch is already exercised via the undefined-tail test (identical branch in formatRpcExit). Scope/Practical: FAIL — the branches exist and are correct; no workflow degrades.
F7 — original() re-copies preview()'s single-flight dance	nitpick	Factual: PASS — the duplication is real. Scope/Practical: FAIL — maintainability only; the two dances already diverge (key shape, pre-authorization step).
F8 — under-cap early return in abortForStalledConsumer unreachable	nitpick	Factual: PASS — proven: the backlog decreases only in the drain listener, which disarms the timer before flushing. Scope/Practical: FAIL — harmless dead defensive code (and the same branch that masks F2).
F9 — .gitignore **/.dreb/ → .dreb*	nitpick	Factual: PASS on the change; corrects the "unrelated" framing — the pattern ignores the dashboard's .dreb-upload-*/.dreb-memory-* temp files the old pattern missed (this PR's surface, per the commit message); strict superset, zero tracked paths affected, no harm.
F10 — root README edited despite plan's "No root README change"	nitpick	Factual: PASS on the facts. Scope: the deviation is mandated by the repo's development guide (root README must track feature changes) and the added text was verified accurate against code — policy-aligned. Practical: FAIL — no harm.

Acceptance criteria (for the record)

• AC1 (2+ attached images → no kill, turn completes, session usable): PARTIALLY MET — survival and completion genuinely asserted; "remains usable" and the stdin JSONL half are not exercised (F3).
• AC2 (2-3+ concurrent image reads → no kill): MET — four 4 MiB tool-result images under permanent backpressure, no abort.
• AC3 (automated regression test through the dashboard RPC stack): PARTIALLY MET at component level — real child stack (real runRpcMode + guard + projector) and real dashboard DashboardImageService are covered separately, with a cross-boundary round-trip pin; no single test spans both processes and the stdin boundary is bypassed (F3).
• AC4 (no display regression): PARTIALLY MET — rendering side proven (references id-identical to the inline path, resolvable from cache without reload); model-delivery side met by construction (projector provably non-mutating), not by test (F5).

Action Plan

None — no merge blockers.

Useful follow-ups F1–F4 remain outside the action plan (candidate for a follow-up PR/issue: tighten imageBlockIdentity to the dashboard's strict decode or cache negative entries; add the partial-drain guard test; fix the dashboard.md threshold wording; route the survival test through the real JSONL ingress plus a post-burst prompt).

---

Assessment by mach6

[aebrer]

Progress Update

Fixed findings 1, 2, and 4 from the review round.

Finding 1 — child dedupe gate was looser than the dashboard's strict decode (dangling references).
imageBlockIdentity in rpc-event-projection.ts now mirrors the dashboard's decodeDashboardImage exactly: allowlisted MIME type + canonical base64 (round-trip) + matching raster signature (PNG IHDR/IEND, JPEG SOI/EOI, GIF trailer, WebP RIFF/VP8x — hasRasterSignature mirrored with a keep-in-sync comment). The child now only turns a block into a reference when the dashboard's strict decode would accept it. A block the dashboard rejects (e.g. JPEG bytes labeled .png, a truncated PNG) stays inline at every occurrence and is dropped exactly as pre-PR — no dangling image_reference, no broken-image slot, no per-render authoritative re-fetch. Test fixtures updated to real raster shapes (the old arbitrary-byte fixtures would no longer dedupe), including the 4 MiB survival-test payload. New tests: signature-mismatch blocks stay inline (child side), and a cross-boundary dashboard round-trip test proving a rejected block stays inline, caches nothing, and is dropped — never a 404.

Finding 2 — partial drain over cap was untested (and the slow-consumer test provably masked).
New output-guard test: 30 × 1 MiB queued (backlog over the 16 MiB cap), a drain accepting only 5 MiB so the backlog stays over the cap → the no-drain window resets on the progress and a full further grace elapses with no kill; a fresh over-cap write re-arms the window and a never-draining consumer is still aborted loudly. Probe-verified: removing disarmNoDrainAbort() from the drain handler fails this test while the existing full-drain test still passes (the masking the finding described).

Finding 4 — dashboard.md claimed the child stdout queue "caps at 16 MiB".
Reworded the Events section: the queue may accumulate past 16 MiB while a slow-but-alive consumer keeps making drain progress; a backlog over 16 MiB aborts only after 30 seconds without drain progress. The dedupe paragraphs in dashboard.md and rpc.md now document the strict-gate contract too (F1-related).

Verification: full workspace suite via test.sh --no-live-api → 5970 passed / 0 failed; biome check --error-on-warnings . clean; tsgo --noEmit clean; npm run build OK.

Commit: e16766f

---

Progress tracked by mach6

[aebrer]

Unverified Review Candidates — Pending Assessment

Review round: 2
Reviewed commit: e16766f

These are unverified candidates. Severity reflects reviewer confidence; do not treat any item as a merge blocker until the assessment comment is posted.

Critical

(none)

Important

F11 — Strict dedupe gate: several MIME/base64 branches unpinned by tests (confidence 84/100)

packages/coding-agent/src/modes/rpc/rpc-event-projection.ts (imageBlockIdentity :156, hasRasterSignature :97) vs. the PR's test files. In the reject-loop test ("keeps non-allowlisted, non-canonical, or signature-mismatched blocks inline"), every fixture is caught by an earlier gate check: MIME allowlist (image/svg+xml), length%4/alphabet (sliced string, "not::base64!!", empty), or the raster signature (PNG-magic-only, [1,2,3,4,5], JPEG-bytes-as-PNG). As a result:

• The canonical-base64 round-trip reject branch (bytes.toString("base64") !== data) is never reached by any fixture in any PR test file — the shape that reaches it is a trailing-garbage-bit spelling (e.g. "AAT=" decodes to [0,4] and round-trips to "AAE="), and no fixture uses that class.
• The WebP branch of hasRasterSignature has zero executions (claim and reject sides) across both packages' tests.
• The JPEG claim side runs (the "different MIME type" test's first occurrence) but its claim outcome is never asserted anywhere.
• The GIF reject side (e.g. missing 0x3b trailer) is untested; only valid GIFs are exercised.

A future edit (or partial revert) desyncing the child's mirrored gate from the dashboard's decodeDashboardImage on any untested branch would let the child claim a block the dashboard rejects → 2nd+ occurrences become image_references the dashboard cannot resolve → dangling references / 404s — the exact F1 bug class, an AC4 display break — while CI stays green (first-occurrence fixtures cannot distinguish gates; the cross-boundary parity test only exercises PNG+GIF). Note: no in-tree producer currently emits non-canonical base64 (read tool uses Buffer.toString("base64"), dashboard uploads use browser btoa — both canonical), so there is no live trigger today; this pins the exact-parity invariant the F1 fix declared. Fix direction: add a trailing-garbage round-trip fixture (assert 2nd occurrence stays inline and the dashboard caches nothing), a WebP valid+reject fixture pair, a GIF reject fixture, and an assertion on the JPEG claim outcome.

Suggestions

F12 — Unreachable byteLength === 0 check in imageBlockIdentity (confidence 85/100) — rpc-event-projection.ts:160. After the pre-checks (non-empty, length%4, strict alphabet), the minimal admitted input is "AA==" (4 chars), which decodes to 1 byte — verified empirically ("AA=="→1, "AAA="→2, "AAAA"→3), so the check is provably unreachable. The dashboard mirror target decodeDashboardImage (dashboard-images.ts:110-119) has no such check, so this line is the one check that breaks the F1 "mirror exactly" invariant. Deleting it is a 1-line fidelity cleanup with no behavior change.

F13 — Three structurally identical pipe-error tests (confidence 82/100) — packages/coding-agent/test/rpc-client-spawn.test.ts:232-257. The stdin/stdout/stderr pipe 'error' tests are identical modulo stream name and error message (~6 lines each); the codebase has an established parameterized pattern (it.each, for-loop test() blocks). Counterweight worth keeping in mind: the explicit test names surface the failing pipe directly in CI failure output, which matters for a crash-class bug — keeping them explicit is defensible.

F14 — New rpc entry exports have no consumers (confidence 80/100) — packages/coding-agent/src/modes/rpc/index.ts:10-13. This PR adds createDashboardRpcEventProjector/projectDashboardRpcEvent to the published @dreb/coding-agent/rpc entry (verified absent at base commit); no in-repo importer uses either name through the entry (telegram/runtime-pool import only RpcClient + types; internal code and all tests use relative paths). The plan's file table never lists index.ts. The names are dashboard-internal (the projector is created solely for uiType === "dashboard"); publishing them now starts an API-compat obligation with no recorded intent. Remove the block, or keep deliberately if they should be public.

Strengths

• The F1 fix is an exact behavioral mirror, verified three ways. The child's gate is check-for-check identical to the dashboard's decodeDashboardImage across all four MIME branches (line-level comparison by two independent reviewers plus the assessor); the no-dangling property is pinned end-to-end by the round-trip test (rejected block: stays inline at every occurrence, byteSize 0, dropped — never a 404).
• The F2 partial-drain test genuinely pins the reset. Assessor verification: removing disarmNoDrainAbort() from the drain handler and replaying the test steps fires the abort at the grace deadline (fatal diagnostic + forced exit), so the no-exit/empty-stderr assertions fail.
• The survival regression test is real end-to-end. Real runRpcMode + real output guard (no write mocking), ~32 MiB of unique image payload under permanent backpressure; asserts backlog > 16 MiB, in-order frames, each of six 4 MiB payloads crossing stdout exactly once, and dashboard-parity ids in the final transcript — it fails on the pre-fix code (the throwing process.exit spy catches the old immediate kill).
• All seven plan deliverables are present and verified at e16766f; 120 tests across the touched files pass (assessor re-ran the 22 core projection/guard tests independently, all green).
• Pipe-error handling converges cleanly. All three failure channels (exit, spawn error, pipe error) funnel into failPendingRequests + exit notification with a sound stale-procRef guard; the stderr-tail chain (guard diagnostic → child stderr → stderrTail → fleet error) is verified end-to-end, bounded to 2000 chars at the surface.

Agents run: code-reviewer, error-auditor, test-reviewer, completeness-checker, simplifier

---

Reviewed by mach6

[aebrer]

Review Assessment

#496 (comment)

Classifications

Finding	Classification	Reasoning
F11 — strict dedupe gate: MIME/base64 branch fixtures missing (round-trip reject, WebP, GIF reject, JPEG claim outcome)	useful follow-up	Factual: PASS — verified per fixture: none reaches the round-trip reject branch (the shape that would reach it — a trailing-garbage-bit spelling like AAT= — appears in no fixture), WebP has zero fixtures in both packages, the JPEG claim outcome is asserted nowhere, and the GIF reject side is untested. Scope: FAIL — AC3 requires the multi-image survival regression test, which is present; no criterion mandates per-branch gate pinning, and the gate at e16766f is a verified strict mirror with no regression introduced. Practical: FAIL — no present divergence (parity holds for all four MIME branches); no in-tree producer emits non-canonical base64 (read tool Buffer.toString("base64") and dashboard btoa uploads are both canonical encoders); harm would require a future desyncing edit and a non-canonical or signature-invalid block co-occurring — a compound future failure, not a present defect.
F12 — unreachable byteLength === 0 check in imageBlockIdentity	nitpick	Factual: PASS — provably unreachable (minimal admitted input "AA==" decodes to 1 byte; verified empirically) and absent from the dashboard mirror target decodeDashboardImage. Scope: FAIL — no criterion requires deleting it; the F1 "mirror exactly" wording creates only a textual mismatch. Practical: FAIL — an unreachable check cannot change any input/output pair; the only effect is speculative maintenance fidelity.
F13 — three structurally identical pipe-error tests	nitpick	Factual: PASS — the stdin/stdout/stderr tests are identical modulo stream name and error message; parameterized idioms exist in the codebase. Scope: FAIL — no scope or plan requirement on test style. Practical: FAIL — no behavior either way; explicit test names surface the failing pipe in CI output, which argues for keeping them.
F14 — new rpc entry exports have no consumers	nitpick	Factual: PASS — the export block was added by this PR (absent at base); zero consumers use either name through the @dreb/coding-agent/rpc entry (all in-repo importers verified). Scope: FAIL — additive; nothing breaks either way. Practical: FAIL — no user-visible outcome; an API-surface judgment call on a published package, with no documentation or plan entry recording intent to expose dashboard-internal names.

Acceptance criteria (for the record)

• AC1 (2+ attached images → no kill, turn completes, session usable): PARTIALLY MET (unchanged) — survival and completion asserted; "remains usable" (a post-burst prompt) and the child-stdin JSONL half are not exercised (deferred F3).
• AC2 (2-3+ concurrent image reads → no kill): MET — four 4 MiB tool-result images under permanent backpressure, backlog verified > 16 MiB, no abort.
• AC3 (automated regression test through the dashboard RPC stack): PARTIALLY MET at component level, strengthened — real child stack (real runRpcMode + guard + projector) and real dashboard DashboardImageService covered separately with a cross-boundary pin; F2's new partial-drain test strengthens the guard half; F11's branch fixtures remain the residual test-depth gap (deferred).
• AC4 (no display regression): MET on the rendering side, improved by F1 — the dangling-reference class is eliminated (child gate is a strict mirror of the dashboard decode, verified line-for-line) and pinned by the no-dangle round-trip test (rejected block: inline at every occurrence, nothing cached, dropped — never a 404); model-delivery side met by construction (projector confined to the stdout subscriber path, provably non-mutating; F5 deferred).

Action Plan

None — no merge blockers.

Deferred follow-ups (outside the action plan): F3 (route the survival test through the real JSONL ingress and send a post-burst prompt), F5 (assert prompt-image blocks reach the model context), F11 (gate branch fixtures: trailing-garbage round-trip, WebP valid+reject, GIF reject, JPEG claim assertion), F12 (delete the unreachable check). F13/F14 are optional cleanups the author may take or leave.

---

Assessment by mach6