Skip to content

refactor(claude): unify subprocess teardown into one idempotent verb (C8)#326558

Draft
TylerLeonhardt wants to merge 1 commit into
claude-agent-host-consolidation-c4from
claude-agent-host-consolidation-c8
Draft

refactor(claude): unify subprocess teardown into one idempotent verb (C8)#326558
TylerLeonhardt wants to merge 1 commit into
claude-agent-host-consolidation-c4from
claude-agent-host-consolidation-c8

Conversation

@TylerLeonhardt

Copy link
Copy Markdown
Member

Stacked on #326552 (C4) → #326270 (C9) → #326256 (C1). Review the stack in order; this PR's diff is one commit.

What

C8 — one teardown path, explicitly idempotent. The pipeline had two teardown spellings that both async-disposed the WarmQuery, and their safety-under-double-call rested on the SDK's close() cleanup happening to be memoized (report 02, H1/H2). C8 collapses them into one private _teardown() guarded by a _torndown flag:

  • shutdownAndWait() is the awaited face — used where we must not spawn a fresh subprocess reusing the same --session-id until the old one has released <id>.jsonl (yield-restart / rebuild resume).
  • dispose() fires the same _teardown() and forgets. The abort + WarmQuery.asyncDispose run synchronously before dispose() returns, so the subprocess is still signalled to stop promptly.
  • Both run abort → WarmQuery.asyncDispose → Query.return in one place, at most once, regardless of which caller wins.
  • Replaces the two dispose-chain toDisposables (separate abort + async-dispose) with one.

C9 already deleted the third spelling (_rebindQuery's manual asyncDispose), so this closes out the set.

Why it's safe

  • Idempotency is now a property of our code (_torndown), not an SDK implementation detail. A shutdownAndWait followed by dispose (the rebuild path) is a guarded no-op instead of a redundant double-dispose.
  • No behavior change to the await-vs-fire-and-forget distinction — shutdownAndWait still awaits actual process exit; dispose still fires-and-forgets.

Validation

  • New pipeline test pins the idempotency: shutdownAndWait then dispose async-disposes the WarmQuery exactly once.
  • Suites green: claudeSdkPipeline 13, claudePromptQueue 12, claudeAgent 197.
  • typecheck-client / eslint / valid-layers-check clean.
  • Live smoke suite 19/19 (test/agent-host-e2e/): materialize / warm reuse / recover-rebuild / abort-churn with subprocess reaping — the scenarios that exercise teardown end-to-end against a real subprocess.

🤖 Generated with Claude Code

…(C8)

Collapses the pipeline's teardown spellings into a single private `_teardown()`,
guarded by a `_torndown` flag so the WarmQuery is async-disposed exactly once —
idempotency now rests on our own flag, not on the SDK `close()` cleanup happening
to be memoized (report 02 H1/H2).

- `shutdownAndWait()` is the awaited face (callers that must not reuse the
  `--session-id` until `<id>.jsonl` is released); `dispose()` fires the same
  teardown and forgets. Both run `abort -> WarmQuery.asyncDispose -> Query.return`
  in one place, at most once.
- Replaces the two dispose-chain `toDisposable`s (separate abort + async-dispose)
  with one; the abort still runs synchronously before `dispose()` returns.
- C9 already deleted the third spelling (`_rebindQuery`'s manual asyncDispose), so
  this is the last of the three.

Adds a pipeline test pinning the idempotency (shutdownAndWait then dispose async-
disposes once). Suites green: claudeSdkPipeline 13, claudePromptQueue 12,
claudeAgent 197. typecheck-client / eslint / valid-layers clean. Live smoke suite
19/19 (materialize / reuse / recover-rebuild / abort-churn + reaping).

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