fix(workflows): hold one writer for durable live SSE - #856
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
implement-plan for #855.
Verify: Not merged. Next: adversarial-review on this PR. |
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #856
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/durable-stream-writer-hold · 8 files · durable live-SSE writer hold
Lenses run: L1, L2, L5, L6, L7, L8 (skip: L3 no host/Wasm/dual-chat; L4 no GHA/artifacts; L9 no UI)
AGENTS.md read: yes (SSE where-to-change, Workflows, DI/test-cost). SECURITY.md + docs/feature-divide.md also read.
Findings
None. Attack did not breach with current evidence.
Candidate breaks that died in self-refutation (not findings): getWritable() throw moving outside onEvent is still closed by runTurnLoop catch → fail() + turnWorkflow finally closeTurnSse; write_error from generateOneRound still releaseLocks because the helper awaits fn and finallys; writeTurnSse cannot race the held lock because it is a later 'use step' and the SDK pushes state.promise onto ctx.ops (flush on releaseLock, poll 100ms, pendingOps === 0); BYOK/assemble stay outside the hold; DI/cost gate clean (mocked workflow, no PGlite/Redis).
Residual risk
Hold-across-streamText is the SDK chat example (streamTextStep keeps one getWriter for the UI-message read loop) and getWritable() in a step really does construct a new serialize pipe every call (@workflow/core step/writable-stream.js). That is the #854 cause this PR removes. It is not proven on a live Workflows Preview: if the VM rejects a writer held across generateOneRound awaits, the named error is Not supported in workflow functions (or equivalent) — plan forbids shipping the per-line fallback unless that error is observed. Replay of modelGenerateStep would re-append tokens to the same Redis stream (pre-existing). Operator close-out for #854 remains a long durable Busy turn after deploy (thinking in order, English assistant, no mid-sentence death while the run is still working). This review did not execute vitest or a live Workflows run.
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; nits optional (none logged)
- Do not treat this as Preview proof of #854. merge-pr still owns the full
vitest rungate
What was not attacked
Live Vercel Workflows / Redis stream, prod Gateway, host parseSseChunk / growAssistant / Wasm (out of scope by plan), E19 attach (#813), EOF detach (#853), DO runner, full vitest process.
Summary
Durable live SSE was calling
getWritable()per token. Each call opens a new Workflows serialize pipe to the same Redis stream; chunks land out of order. The host appends whatever still parses → word-salad assistant, frozen thinking, dead reader.This PR holds one writer for the model-step burst (
withDefaultStreamWriteraroundgenerateOneRound). Sparse loop writes (writeTurnSse→writeOnDefaultStream) stay one write per step.Implements #855 (HANDOFF-READY). Source bug #854.
Fixes #855
Refs #854
Refs #794
Refs #850
What changed
lib/workflows/turnSseWrite.ts—withDefaultStreamWriter(onegetWritable/ heldgetWriter/releaseLockinfinally; does not close).lib/workflows/modelGenerateStep.ts— liveonEventwrites through that held writer. NowriteOnDefaultStreamin this step.docs/agent-stream.md+AGENTS.md— one writer per model round, not per token.Tests
main@2f77992)--changed)lib/workflows/turnSseWrite.test.tsNew rows: N writes → 1 pipe;
releaseLockonfnthrow;releaseLockon mid-burst write throw; ordered payloads concatenate; sparsewriteOnDefaultStreamstill per-call.npm run typecheckgreen. Targeted:turnSseWrite/turnWorkflow/turnLoop/staticGraph/turnSseFormat68 passed.vitest run --changed66 passed / failed=0. Fullvitest runis the merge-pr gate.Out of scope
parseSseChunkAgentStreamEventOperator smoke (after Preview)
Long durable Busy turn (
/plan_reviewor reasoning): thinking grows in order, assistant is readable English, turn does not die mid-sentence while Workflows is still working.