Skip to content

refactor(claude): queue owns its done latch + steering Event (C4)#326552

Draft
TylerLeonhardt wants to merge 1 commit into
claude-agent-host-consolidation-c9from
claude-agent-host-consolidation-c4
Draft

refactor(claude): queue owns its done latch + steering Event (C4)#326552
TylerLeonhardt wants to merge 1 commit into
claude-agent-host-consolidation-c9from
claude-agent-host-consolidation-c4

Conversation

@TylerLeonhardt

Copy link
Copy Markdown
Member

Stacked on #326270 (C9). Review that first; this PR's diff is the single commit on top. Base retargets as the stack lands.

What

C4 — delete the last two back-reference closures. ClaudeSdkPipeline injected two closures into ClaudePromptQueue: () => this._abortController.signal (read on every next()) 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.

  • Abort → the queue owns a terminal _done latch, set by notifyAborted() (which the pipeline already calls after aborting its controller). next() checks the latch instead of reaching back through a () => signal closure into the pipeline's swap-prone field.
  • SteeringonDidYieldSteering: Event<string> replaces the injected callback; the pipeline subscribes, matching the existing router→pipeline Event pattern.
  • resetForRebind deleted — 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.
  • _wireAbortHandler now 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

  • _done faithfully models "controller aborted": every abort path (session.abort, pipeline.abort, shutdownAndWait, dispose) aborts the controller → fires _wireAbortHandlernotifyAborted()_done. The queue's termination no longer depends on reading an external signal that could go stale.
  • Steering behavior is unchanged — steering_consumed still fires the moment a steering entry is handed to the SDK, now via the Event.

Validation

  • Suites green: claudePromptQueue 12, claudeSdkPipeline 12, claudeAgent 197.
  • typecheck-client / eslint / valid-layers-check clean.
  • The abort/steering/parked-next() semantics are pinned by claudePromptQueue.test.ts; the end-to-end abort + steering paths by claudeAgent.test.ts (incl. this stack's C9 abort/rebuild-race edge tests).

🤖 Generated with Claude Code

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant