fix(harness): treat durable stream EOF as detach, not Turn ended - #853
Conversation
POST /api/turns body EOF without SSE done/error was folded as model-finished (or empty-complete) and wrote turnStatus completed, poisoning E19 attach while the workflow kept running. Classify this-turn onTurnStarted + !sawStreamTerminal as D18 detach; do not retry after the run has started; forward turnRunId on stream-read failure. Fixes #852 Refs #849
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Implement-plan for #852 (source #849). Host fold: this-turn Verification (agent workspace): |
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #853
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main (745855c) ← plan/durable-stream-eof-detach (5b60e47) · 6 files · durable SSE EOF fold
Plan: #852 (source #849)
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no trust-boundary / secret / API change; L4 no CI / wasm / workflow files; L5 retry is strictly more conservative; L7 no host/env hardcoding)
AGENTS.md read: yes (plus docs/feature-divide.md; SECURITY.md not required)
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Minor | L3 / L9 | EOF detach returns { ok: false, error } to the same-epoch host caller. HarnessHost.tsx runPrompt then setHostNote(result.error) and paints ember DOM chrome (host: …) while the canvas is Ready with no turn-end line. The ok: true + tokens + no done path is new chrome that previously did not exist (ok: true used to skip setHostNote). |
Start a durable Busy turn; drop the live POST /api/turns reader (alt-tab / proxy EOF) after some text_delta. Canvas: Ready, no Turn ended. Above the canvas: host: Stream ended without a terminal event. (or Empty model response. if still Waiting). Operator treats a detach as a host failure; a follow-up send 409s (C15) and paints an error row onto a still-running session. |
Plan locked { ok: false } to the host caller; setHostNote is pre-existing for every fail; D18 leave-site skips it via turnEpoch mismatch. That does not cover same-tab EOF, which this PR makes the common path, and it does not excuse EMBER chrome for a non-failure. Fix lives in HarnessHost (skip note when session.turnStatus === 'running' / detach) or in the fold (distinct result that is not an error string). |
high |
| Nit | L6 | New durableIncomplete predicate is !sawStreamTerminal. No test pins onTurnStarted + SSE error → error fold (not detach) and no second POST. Existing error rows omit onTurnStarted. |
A later edit that sets sawStreamTerminal only on done (or keys detach on !agentResult.ok) would keep a failed producer run as running with no error line. E19 would attach to a dead workflow. |
onEvent already sets sawStreamTerminal on error (~1458); plan edge table states this; code matches. Gap is a missing pin, not a live miss. |
high |
Residual risk
Same-tab Ready + turnStatus: 'running' is now the alt-tab path, not only D18 unmount. Composer is free (setFailLifecycle → Ready, inflightRef cleared). A second send is C15 409, not a second start, if the envelope PUT from onTurnStarted landed. Browser AbortError without DETACH_ABORT_REASON still classifies as 'stop' (Request cancelled.) and will paint you-stopped + clear the id — this PR does not treat that as detach (plan: abort stays reason-only). Viewport stays blind until E19 (#813). Clean producer close with tokens but no SSE done is also detach by design. Review did not re-run full vitest (PR claims 2342 / +7; GHA not on this PR; Vercel build step ignored).
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; nits optional
- Suggested follow-up (not merge-blocking): suppress
hostNoteon same-tab detach; addonTurnStarted+ SSEerrorpin
What was not attacked
Live Workflows run, production Gateway, actual browser alt-tab / bfcache, E19 GET attach (#813), Wasm/protocol, C15 409 against a real envelope store, DO runner.
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #853
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main (745855c) ← plan/durable-stream-eof-detach (00e4f1c) · 9 files · durable SSE EOF fold + round-1 amendment
Plan: #852 (source #849)
Lenses run: L1, L3, L6, L8, L9 (skip: L2 no trust-boundary / secret / API-route change — turnApi only forwards an already-visible run id on stream-read catch; L4 no CI / wasm / workflow files; L5 retry is strictly more conservative; L7 no host/env hardcoding)
AGENTS.md read: yes (plus docs/feature-divide.md; SECURITY.md not required)
Prior review: COMMENT on 5b60e47 (PASS WITH NOTES). This pass is HEAD 00e4f1c.
Findings
None that survived self-refutation on HEAD.
Round-1 survivors were addressed in 00e4f1c:
| Round-1 | HEAD |
|---|---|
Minor L3/L9 — same-tab EOF { ok: false } lit ember hostNote while canvas Ready + running |
shouldSetHostTurnNote + HarnessHost gate on folded.turnStatus !== 'running'; source-lock in lib/detachTurn.test.ts |
Nit L6 — no pin that onTurnStarted + SSE error is error fold, not detach, and no second POST |
New harnessChat.test.ts row: one POST, completed, error line, Lifecycle.Error |
Fold itself (traced): sawDurableStart is set only in this-turn onTurnStarted; durableIncomplete requires streamAgent && sawDurableStart && !sawStreamTerminal && stopKind !== 'stop'; ok: true without done skips the success block (which still pushTurnEnd('model') + turnStatus: 'completed') and reuses the D18 persist (keep id + running, no turn-end line, setFailLifecycle(..., 'detach') → Ready, promote false). Leftover completed id does not freeze retries. JSON/HTTP turnRunId without onTurnStarted is not detach. Stop/Esc still wins. DI/cost: new rows are mock sendAgentStream / stub fetch / fake timers — no new PGlite( / createDbConnection(.
Residual risk
Same-tab Ready + turnStatus: 'running' is now the alt-tab path, not only D18 unmount. Composer is free (setFailLifecycle → Ready; inflightRef cleared). A second send is C15 409, not a second start, if the server C14d running PATCH landed — 409 JSON has no x-workflow-run-id, so the fail fold keeps leftover running (does not poison E19) but paints an error row + Error lifecycle onto a still-live workflow. shouldSetHostTurnNote keys off leftover running, so that 409 skips DOM hostNote; canvas still shows the error line (product surface). Browser AbortError without DETACH_ABORT_REASON still classifies as 'stop' (Request cancelled.) and will paint you-stopped + clear the id — this PR does not treat that as detach (plan: abort stays reason-only). Viewport stays blind until E19 (#813). Clean producer close with tokens but no SSE done is also detach by design. Timeout/504 that only EOFs the reader (no SSE error) is detach by the locked incomplete predicate, not a timeout line — C15 getRun terminal-or-missing allows the next start. Review did not re-run full vitest (PR claims 2342 / +7 on first commit; amendment adds hostNote + SSE-error pins; GHA test/typecheck/build not on this PR; Vercel GitHub status SUCCESS with deploy ignored).
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; nits optional
- Round-1 suggested follow-ups are in HEAD; no further merge-blocking work from this pass
- Operator smoke after Preview/Production still required by the plan: alt-tab mid durable Busy → no Turn ended; session
runningwith the same id; Workflows run still working
What was not attacked
Live Workflows run, production Gateway, actual browser alt-tab / bfcache / Safari background abort, E19 GET attach (#813), Wasm/protocol, C15 409 against a real envelope store, DO runner, full vitest in this workspace.
Summary
Durable
POST /api/turnsis start and the live SSE reader. When that reader dies without a producerdone/error(alt-tab / background tab / proxy EOF), the host was folding Turn ended · model finished (or empty-as-complete) and writingturnStatus: 'completed'. The Workflows run kept going. Refresh then looked finished, so E19 attach cannot help.This PR treats that incomplete durable read as D18 detach: keep
turnRunId+running, skip the turn-end line, do not cancel the run, do not POST/api/turnsagain.Plan
Implements #852 (HANDOFF-READY). Source bug #849.
Fixes #852
Refs #849
Refs #794
Refs #813
What changed
lib/harnessChat.ts—sawDurableStartset only in this-turnonTurnStarted. Retry is permanent oncestreamPainted || sawDurableStart. After the attempt,streamAgent && sawDurableStart && !sawStreamTerminal && not stop(includingok: truewithoutdone) takes the D18 detach persist fold.lib/turnApi.ts— non-abort stream-read catch forwardsturnRunId/turnWarning.docs/agent-stream.md+AGENTS.md— End of turn / where-to-change: detach paints no line; empty/EOF after durable start is not retried.Tests
main@745855c)New rows:
ok: truewithoutdoneis detach; empty withoutdoneis detach;donestill completes; no second POST afteronTurnStarted; leftover completed id still retries; JSON/HTTPturnRunIdwithoutonTurnStartedis not detach;sendTurnStreamread-throw forwards the run id.npm run typecheckgreen. Fullnode_modules/vitest/vitest.mjs rungreen in the agent workspace.Out of scope
Operator smoke (after Preview/Production)
Start a durable Busy turn, drop the live reader (alt-tab or equivalent) before SSE
done. Canvas must not paint Turn ended. Session staysrunningwith the sameturnRunId. Workflows run still working.