feat(cli): Agent Protocol stream reattachment (PR2) + AG-UI run-slot authorization fix - #489
Open
blove wants to merge 7 commits into
Open
feat(cli): Agent Protocol stream reattachment (PR2) + AG-UI run-slot authorization fix#489blove wants to merge 7 commits into
blove wants to merge 7 commits into
Conversation
The AG-UI handler ran runRegistry.begin before rechecking the concrete thread row, so on the create-race path a caller the recheck ultimately denies held the victim thread's run slot for the width of that recheck. Because the thread id is client-chosen on this endpoint, a denied caller could brick a concurrent authorized run on the same thread with a transient run_in_flight 409. Move the row authorization (and the implicit create, when the turn makes one) ahead of the run-slot claim, mirroring the Agent Protocol run handlers. Adds a deterministic concurrency regression test that blocks a denied caller's recheck mid-flight and asserts a concurrent authorized run still claims the slot. Also corrects a stale comment on the static module manifest's thread-access channel (it is emitted and validated, not inert). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…urrent main The 2026-08-09 provisional plan predated PR1/PR B landing; its line anchors and wire assertions were stale by construction. This re-draft pins every anchor to the current tree, corrects two stale-spec items (the interrupt value is already surfaced; the hub belongs beside runRegistry, not threadRouteMap), and structures the work as durable-path-first so each task is independently testable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pull-based fan-out for AP reattachment: open()/publish()/close() on the producer side, attach() handing each subscriber a point-in-time digest snapshot plus a live tail via next(). Coalesces consecutive chunk frames and per-callId subagent.message frames, drops the digest whole on byte overflow, isolates per-subscriber queue overflow, and guards producers against writing into a replaced/closed entry (mirrors the run-registry identity guard). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the GET mirror of POST /threads/:id/runs/stream so a disconnected
client can reattach to a thread: one `event: state` snapshot frame
(status, values, parked interrupts) followed by an immediate
`done{output:null}` + jittered retry hint on the durable path (no live
turn in this process). Also wires the live-path branch against the
already-landed LiveTurnHub, though nothing calls hub.open yet, so every
attach in this PR is durable.
Gates identically to GET /pending_interrupts: thread lookup, thread-access
read gate (denial byte-identical to a genuine 404), route-identity
resolution, then middleware with method:"GET". Adds the ThreadOperation
member `thread.attach` to @dawn-ai/sdk so policies can distinguish it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oducers Wires hub.open/publish/close into handleApStreamRequest, handleResumeRequest, and the AG-UI handler, so GET /threads/:id/runs/stream actually tails a running turn instead of always falling back to the durable path. Each producer opens a live turn right after claiming the run slot (anchored on the latest checkpoint before its own route stream runs), publishes every non-terminal chunk, and closes with the same terminal chunk the primary client saw, unconditionally, in its finally — regardless of done, error, park, or cancel. The AG-UI handler taps the raw StreamChunk stream before AG-UI translation so an attacher sees AP-vocabulary frames. handler.close() now drains the hub too, fanning a terminal frame to any hanging viewer. Also completes handleApAttachRequest's live-path `values` read (left null by the prior PR): it now reads the checkpoint at the live turn's anchor, which races nothing since old checkpoints are immutable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues the Agent Protocol thread-authorization arc. Two logical units on one branch:
1. Security fix (audit pass) — authorize the AG-UI row before claiming its run slot
An adversarial re-audit of the thread-authorization surface (three reviewers: stamp-forgery/prototype-chain, gate-ordering/TOCTOU, fail-open/manifest) found the surface solid, with one real LOW finding:
POST /agui/:routeIdcalledrunRegistry.beginbefore rechecking the concrete thread row on the create-race path. Because the thread id is client-chosen there, a caller the recheck ultimately denies could hold a victim thread's run slot for the width of that recheck, bricking a concurrent authorized run with a transientrun_in_flight409.The fix moves row authorization (and the implicit create) ahead of the run-slot claim, mirroring the AP run handlers. A deterministic concurrency regression test blocks a denied caller's recheck mid-flight and proves a concurrent authorized run still claims the slot. Also corrects a stale comment on the static-module manifest's thread-access channel.
2. AP stream reattachment PR2 —
GET /threads/{id}/runs/streamLets a disconnected client rejoin a running turn (spec:
docs/superpowers/specs/2026-08-09-ap-stream-reattach-design.md; re-drafted plan included).LiveTurnHub(packages/cli/src/lib/dev/live-turn-hub.ts) — a per-handler-instance bounded in-memory digest of the current turn with pull-based SSE fan-out: consecutive-chunkand per-callIdsubagent.messagecoalescing, whole-digest drop on a 2 MiB overflow, per-subscriber queue caps, a 16-viewer cap, and an identity guard so a zombie route cannot write into a successor turn.event: statesnapshot (channel values, the turn's frames so far, parked interrupts) then the live tail; or, when no live turn exists in-process, an immediate durable snapshot +done+ retry hint. Being a bodyless GET, it is the first Agent Protocol stream a stockEventSourcecan consume. Gated exactly as strictly asGET /threads/{id}/pending_interrupts(thread-accessread+ the parking route's middleware) — verified by adversarial review as a sound, bypass-free mirror.StreamChunks (AP vocabulary), with unconditional close-in-finallyordering;handler.close()drains the hub.@dawn-ai/sdkgains one additiveThreadOperationmember,thread.attach.The durable path works across restarts, replicas, and serverless; live tail is best-effort on the process holding the run. Interrupt
valuesurfacing and parked-status honesty already shipped in #443, and are reused here.Testing
pnpm ci:validatepasses lint, build, typecheck, and the full test suite (4600+ tests). New coverage: the LiveTurnHub unit suite, the attach-endpoint durable + live + resume + AG-UI + shutdown integration tests (all non-vacuous — each verified to fail when its producer hook is disabled), the AG-UI slot-ordering concurrency regression, and the route-coverage table classifying the new gated endpoint. Two full-suite runs showed only load-induced per-test timeout flakes (a TS-compile test in@dawn-ai/coreand aspawnSyncsubprocess test inweb, neither touched here); both pass in isolation.Follow-up: PR3 (
dawn threads tail+ docs). TheapAttachDigestMaxBytes/apAttachMaxViewersbounds are internal defaults, not yetDawnConfigfields.🤖 Generated with Claude Code