refactor(claude): queue owns its done latch + steering Event (C4)#326552
Draft
TylerLeonhardt wants to merge 1 commit into
Draft
refactor(claude): queue owns its done latch + steering Event (C4)#326552TylerLeonhardt wants to merge 1 commit into
TylerLeonhardt wants to merge 1 commit into
Conversation
Deletes the two back-reference closures the pipeline injected into ClaudePromptQueue — the last of the "closures passed down that call instance functions" from the original review. - Abort: the queue owns a terminal `_done` latch set by `notifyAborted()` (which the pipeline already calls after aborting its controller), so `next()` no longer reads the pipeline's swap-prone `AbortSignal` through a `() => signal` closure. - Steering: `onDidYieldSteering: Event<string>` replaces the injected `(pendingId) => fire(...)` callback; the pipeline subscribes, matching the existing router->pipeline Event pattern. - Deletes `resetForRebind` — dead since C9 made the pipeline immutable (a rebuild mints a fresh queue rather than reviving one), so the latch never needs clearing. - `_wireAbortHandler` now also handles an already-aborted controller directly (addEventListener does not fire retroactively); the queue is constructed before wiring so the latch is reachable. Queue ctor drops from 4 args to 2. Suites green: claudePromptQueue 12, claudeSdkPipeline 12, claudeAgent 197. typecheck-client / eslint / valid-layers clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
What
C4 — delete the last two back-reference closures.
ClaudeSdkPipelineinjected two closures intoClaudePromptQueue:() => this._abortController.signal(read on everynext()) and(pendingId) => this._onDidProduceSignal.fire(...)(steering). These are the "closures passed down that call instance functions" from the original review. C4 removes both, so the queue holds no back-reference into the pipeline._donelatch, set bynotifyAborted()(which the pipeline already calls after aborting its controller).next()checks the latch instead of reaching back through a() => signalclosure into the pipeline's swap-prone field.onDidYieldSteering: Event<string>replaces the injected callback; the pipeline subscribes, matching the existing router→pipelineEventpattern.resetForRebinddeleted — dead since C9 made the pipeline immutable (a rebuild mints a fresh queue rather than reviving one), so the latch never needs clearing. This is why C4 stacks naturally on C9: the immutability removed the only reason the latch would ever reset._wireAbortHandlernow also handles an already-aborted controller directly (addEventListener('abort')doesn't fire retroactively); the queue is constructed before wiring so the latch is reachable.The queue ctor drops from 4 args to 2. Net −30 lines.
Why it's safe
_donefaithfully models "controller aborted": every abort path (session.abort,pipeline.abort,shutdownAndWait, dispose) aborts the controller → fires_wireAbortHandler→notifyAborted()→_done. The queue's termination no longer depends on reading an external signal that could go stale.steering_consumedstill fires the moment a steering entry is handed to the SDK, now via the Event.Validation
claudePromptQueue12,claudeSdkPipeline12,claudeAgent197.typecheck-client/eslint/valid-layers-checkclean.next()semantics are pinned byclaudePromptQueue.test.ts; the end-to-end abort + steering paths byclaudeAgent.test.ts(incl. this stack's C9 abort/rebuild-race edge tests).🤖 Generated with Claude Code