fix(agent-chat): avoid duplicate Claude child close

[teamleaderleo]

What changed

Claude child runs are bracketed by the spawn tool's PreToolUse and PostToolUse events. Claude also emits SubagentStop, but that event has no request id in this path. The second close therefore used FIFO and could settle a sibling that was still running when two children were spawned in parallel.

The registry now ignores SubagentStop for Claude. Codex, pi, and OMP still close children through SubagentStop. The sidebar documentation now describes the FIFO behavior and the _opencode_request_id field that the code actually decodes instead of agent_id.

The defect was found while reviewing PR 15865.

Tests

• python3 scripts/verify-local.py --all passed all 15 checks.
• The new regression covers two parallel Claude spawns and the duplicate stop event.
• The macOS test was not run locally. CI exercises it.

Changelog

• Fixed Claude parallel child runs being settled twice by duplicate hook events.

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Fixes Claude parallel child runs settling twice when both the spawn tool's PostToolUse and a SubagentStop closed the same child, FIFO-closing a sibling still running.

The registry now ignores PostToolUse for Claude — a background spawn returns at detach while the child keeps running — and settles the child on SubagentStop instead. Codex, pi, and OMP keep closing children via their SubagentStart/SubagentStop pair, with the FIFO fallback when no request id is present. Sidebar docs now describe the FIFO closing behavior and note that agent_id is not read; only _opencode_request_id can correlate a stop. Adds regression tests covering two parallel Claude spawns with the duplicate stop and the background child lifecycle.

^{Written for commit a65cfb5. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes
  • Claude stop events without a request ID no longer close the wrong child run.
  • Codex stop events continue to close child runs using their request ID and timestamp.
• Documentation
  • Clarified that stop events without a matching request ID close the oldest running child, and that agent_id does not correlate stop events to children.

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 8 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9ad328d8-59f1-475e-a716-46f9e38a5907

📥 Commits

Reviewing files that changed from the base of the PR and between 6ba74c6 and a65cfb5.

📒 Files selected for processing (3)

• Sources/Mobile/AgentChat/AgentChatSessionRegistry+ChildRuns.swift
• cmuxTests/AgentChatChildRunTests.swift
• docs/custom-sidebars.md

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 191a480a-1ebf-45ef-9f86-ee406f25bfd0

📥 Commits

Reviewing files that changed from the base of the PR and between b413b7a and 6ba74c6.

📒 Files selected for processing (3)

• Sources/Mobile/AgentChat/AgentChatSessionRegistry+ChildRuns.swift
• cmuxTests/AgentChatChildRunTests.swift
• docs/custom-sidebars.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The registry ignores Claude-sourced SubagentStop events. Other SubagentStop events can still close a child using the event’s request ID and timestamp. Tests and hook documentation reflect child closure behavior.

Changes

Child run stop handling

Layer / File(s)	Summary
Stop event handling and validation Sources/Mobile/AgentChat/AgentChatSessionRegistry+ChildRuns.swift, cmuxTests/AgentChatChildRunTests.swift, docs/custom-sidebars.md	Claude SubagentStop events no longer reach closeChild. Tests verify that these events leave a sibling child running, and that Codex stop events use the non-Claude path. The hook documentation states that stop events close the oldest running child in FIFO order and that only _opencode_request_id can correlate a stop event to a child.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: lawrencecchen

Merge Risk: ⚪ Minimal · up to 6ba74

Claude Task/Agent children still close through PostToolUse, and the telemetry stop event no longer risks closing a sibling. No merge-blocking regression is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 6ba74

The change prevents duplicate completion events from marking another child finished, without adding access or privileges. Remaining uncertainty concerns child-status accuracy when completion events are missing; parent completion still clears open children.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The changed mutation operates on child timing state within the selected session record. The handler itself performs no command execution, credential access, permission decision, or network operation; the patch removes one event's ability to close a child.

Trust Boundaries and Controls

• inferred — The source-string predicate is telemetry classification, not authentication. Authenticity of upstream source labels was not established, but the patch does not grant authority to those labels or expand accepted entrypoints. Existing session identity handling precedes the changed mutation.

Resilience and Maintainability Implications

• inferred — If a Claude child lacks its corresponding PostToolUse event, ignoring SubagentStop can leave its displayed status running until parent stop or sessionEnd. The inspected standard settings do not establish general Task/Agent PostToolUse delivery. This is a bounded status-accuracy uncertainty, not evidence of continued execution or gained authority.

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Cmux Full Internationalization	❌ Error	The PR changes user-facing documentation in docs/custom-sidebars.md: it changes the stop-event behavior text to say FIFO closing and that only _opencode_request_id correlates a child. The document…	Route the changed documentation through the locale-specific documentation system and provide matching translated content for every locale in web/i18n/routing.ts (en, ja, zh-CN, zh-TW, ko, de, es, fr, it, da, pl, ru…
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (23 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the primary change: preventing duplicate Claude child-run closure.
Description check	✅ Passed	The description explains the defect, resulting behavior, affected integrations, tests, unverified macOS coverage, and changelog entry. It is mostly complete, although it omits the template's Demo Vide…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input	✅ Passed	PASS: The pull request changes only AgentChat child-run bookkeeping, related tests, and sidebar documentation. The diff introduces no Cloud terminal creation, cmux-tui transport, renderer, PTY readine…
Cmux Swift Actor Isolation	✅ Passed	PASS. The production diff only adds a source check to an existing nonisolated helper. It does not add or alter actor annotations, protocols, models, loggers, or shared reference types. `WorkstreamEv…
Cmux Swift Blocking Runtime	✅ Passed	The production Swift diff only adds a source predicate to the existing SubagentStop case. It adds no semaphore, blocking wait, sleep, delayed dispatch, polling, main-queue sync, timer, or manual loc…
Cmux Browser Automation Off-Main	✅ Passed	PASS: The PR changes only AgentChat child-run handling, its tests, and sidebar documentation. It does not modify the rule-covered browser automation files, and no added lines reference browser socket …
Cmux Expensive Synchronous Load	✅ Passed	PASS: The only production Swift change is a where event.source != "claude" predicate on the existing SubagentStop case. It adds no history loader, file read, directory scan, syscall loop, or large…
Cmux Cache Substitution Correctness	✅ Passed	PASS. The production diff only adds a source check for Claude SubagentStop in event-driven child-run bookkeeping. It does not replace a fresh authoritative read with a cached or opportunistic value,…
Cmux No Hacky Sleeps	✅ Passed	The pull request changes only Swift source/tests and Markdown documentation. It adds no TypeScript, JavaScript, shell, or build/runtime-script changes, and the diff contains no sleeps, timers, polling…
Cmux Algorithmic Complexity	✅ Passed	PASS. The only production change adds a scalar event.source guard before the existing .subagentStop handling. It adds no scan, sort, filter, join, or nested iteration. The existing child collectio…
Cmux Swift Concurrency	✅ Passed	The Swift production diff only adds a source guard to the existing .subagentStop case. The test diff adds event-source plumbing and a regression test. No new Dispatch, Combine, completion-handler, o…
Cmux Swift @Concurrent	✅ Passed	The Swift diff only changes the synchronous, existing nonisolated applyChildRunEvent switch. It adds no async function, @concurrent annotation, actor-isolation change, or heavy async call site…
Cmux Swift Package Boundaries	✅ Passed	PASS. The production diff changes only the existing app-target child-run switch: it adds a Claude-specific guard for duplicate SubagentStop handling. It does not introduce or materially expand a fea…
Cmux Swiftpm Lockfiles	✅ Passed	PASS: The reviewed PR changes only AgentChat Swift source, tests, and documentation. It does not change Package.swift, Package.resolved, .gitignore, Xcode project/workspace package references, workflo…
Cmux Swift Logging	✅ Passed	The production Swift diff only adds a Claude-specific SubagentStop guard and comments. It adds or changes no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or sensitive-data …
Cmux User-Facing Error Privacy	✅ Passed	The production diff only adds a Claude-specific condition to child-run bookkeeping and a developer-only comment. It does not add or change a user-facing error, alert, command output, API error body, o…
Cmux Swiftui State Layout	✅ Passed	PASS — The pull request does not introduce a SwiftUI view, state wrapper, geometry measurement, lazy/list row store reference, or render-time state mutation. The Swift production diff only adds a sour…
Cmux Architecture Rethink	✅ Passed	PASS. The Swift change is a small local correctness fix in AgentChatSessionRegistry, which remains the sole owner of child-run state. It adds no timing repair, polling, lock, observer, cache, side c…
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PASS — The PR changes only AgentChat child-run bookkeeping, its test fixture, and documentation. The Swift production diff modifies applyChildRunEvent and adds no NSWindow, NSPanel, `NSWindowCon…
Cmux Source Artifacts	✅ Passed	The PR changes only hand-written Swift source, a Swift test, and durable documentation: Sources/Mobile/AgentChat/AgentChatSessionRegistry+ChildRuns.swift, cmuxTests/AgentChatChildRunTests.swift, a…
Cmux No Test Or Debug Seam In Production Source	✅ Passed	PASS. The only changed Swift file under Sources/ modifies applyChildRunEvent by adding a Claude-specific SubagentStop condition and comments. The diff adds no #if DEBUG or test-build block, no…

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (1 skipped: 1 unsupported.)

Full details: Cmux Full Internationalization

Explanation

The PR changes user-facing documentation in docs/custom-sidebars.md: it changes the stop-event behavior text to say FIFO closing and that only _opencode_request_id correlates a child. The document is exposed through https://cmux.com/docs/custom-sidebars and the raw authoring-guide URL in CLI/CMUXCLI+DocsSettings.swift and the Settings UI. The changed markdown is English-only and the diff adds no locale-specific source or message entries. The Swift production change adds only a protocol guard and comments; tests are allowed.

Resolution

Route the changed documentation through the locale-specific documentation system and provide matching translated content for every locale in web/i18n/routing.ts (en, ja, zh-CN, zh-TW, ko, de, es, fr, it, da, pl, ru, bs, ar, no, pt-BR, th, tr, km, and uk). Update the corresponding web/messages/*.json entries or locale-specific markdown sources, and ensure the /docs/custom-sidebars page and raw documentation link use those localized entries.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[teamleaderleo]

Review: the guard is on the wrong event

Reviewed at 6ba74c6e58c. The double-close is genuine and the two-commit shape proves it. One change to the fix, and one catch-up item.

Ignore PostToolUse, not SubagentStop

The patch drops SubagentStop for Claude and keeps the spawn tool's PostToolUse as the close. That keeps the less accurate of the two signals. The doc comment at the top of AgentChatSessionRegistry+ChildRuns.swift already records why, under "Honest limits": a backgrounded spawn returns from PostToolUse at detach while the child keeps running, so a background child reads as settled the moment it detaches. SubagentStop fires when the child finishes.

For Claude the accurate pairing is: open on the spawn tool's PreToolUse, close on SubagentStop, ignore the spawn tool's PostToolUse. Same one-line shape, moved:

case .postToolUse where isTaskSpawn(event) && event.source != "claude":

with case .subagentStop: left unguarded. That closes the double-close and the premature background close together.

Two things make it safe, both checked in the tree:

• cmux installs the SubagentStop hook itself for Claude (CLI/CMUXCLI+ClaudeHookSettings.swift:44), so the event is present for any session cmux set up. Claude has no SubagentStart hook installed, so there is no matching double-open to worry about.
• Where the hook is absent, case .stop, .sessionEnd: already closes every open child. The worst case is a row that stays open until the parent's turn ends, not one that never closes.

Worth a second regression: a Claude child whose PostToolUse arrives long before its SubagentStop must stay running until the SubagentStop.

The branch is behind a doc comment that contradicts it

The merge base predates e1dc959396b (#15865). That commit rewrote the same doc comment to say that for Claude "both that event and the spawn tool's PostToolUse fall through to the FIFO branch of closeChild and one child closes two rows". This fix makes that sentence false, so the paragraph needs rewriting after a catch-up merge, and the docs/custom-sidebars.md wording here should be re-read against the final behavior.

🤖 Generated with Claude Code

[teamleaderleo]

Review: the fresh read-only review found no concrete functional, test, documentation, or repository-rule issues.

Fixed: Claude PostToolUse is now ignored for spawn bookkeeping, while SubagentStop remains the completion signal. The parallel-child regression and the background-spawn regression cover sibling safety and detach timing. The #15865 lifecycle comment now matches the implementation, and Codex, pi, and OMP retain their existing FIFO fallback behavior.

Left: the macOS test target was not run locally. CI exercises it.

[teamleaderleo]

Both review items are addressed at e24b5eb1a35

Checked the pushed tree rather than the summary.

The guard moved to the spawn tool's PostToolUse and case .subagentStop: is unguarded again, so a Claude child is opened by PreToolUse and closed by SubagentStop. That closes the double-close and the premature background close together: the child of a backgrounded spawn no longer settles the moment it detaches.

claudeBackgroundSpawnStaysRunningUntilSubagentStop covers the case the first shape could not deliver, and claudeSubagentStopClosesFirstChildWithoutClosingSibling now asserts that neither sibling settles on PostToolUse. missingRequestIdClosesOldestOpenChild and subagentStartStopTrackChildren are sourced to codex, so the FIFO fallback stays covered for the agents that still use it.

Main is merged in at 1b06f84cbd9, and the doc comment #15865 landed has been rewritten rather than left contradicting the code. It now describes the two shapes as they are: Claude opening on the spawn tool's PreToolUse and closing on SubagentStop, with the spawn tool's PostToolUse ignored for Claude, and Codex, pi and OMP on their dedicated pair with the FIFO fallback. The "honest limits" paragraph no longer claims background children read as settled at detach, which is no longer true.

One thing left alone: taskSpawnOpensAndClosesChild keeps its name although the close now comes from SubagentStop rather than the spawn tool. It asserts the right lifecycle, so it is not worth another cycle.

No findings remain from my side. This is a fix, so it merges on green rather than going to team review.

🤖 Generated with Claude Code

[github-actions]

CI failure attribution

CI passes on a65cfb5536 (run 36713235657 attempt 2).

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's.

[github-actions]

Dogfood tours of a65cfb55

sidebar-and-chrome-tour at a65cfb55: not run

skipped: CI built this head on a runner pool whose products the UI test Macs cannot load, and media never compiles one; gh workflow run pr-media.yml -f pr=<n> -f allow_compile=true does

_{Tours are picked by the paths globs in dogfood/scenarios/*.json; a Dogfood-tours: a, b line in the description picks them instead (none turns this off). Look at every frame before merging: a green tour only means no step failed.}

[teamleaderleo]

The four reds are inherited from the merge point, and main already fixed them

The failing step is Compile app-host test product, with cannot find type 'BonsplitContrastPalette' in scope at Sources/TerminalSizeBoundsOverlayView.swift:28 and three follow-on inference errors in Sources/TerminalSizeParticipantRow.swift. The other three reds are routing and status jobs that depend on it, including the required ci-status.

None of it comes from this branch. The cause is a submodule pointer that lagged its own call sites on main:

• vendor/bonsplit at this PR's head is 83857fa043b, the same pointer the merge base carried.
• Main at 1b06f84cbd9, which this branch merged, also carried 83857fa043b while TerminalSizeBoundsOverlayView.swift already referenced BonsplitContrastPalette seven times. Main did not compile at that commit.
• 7d246ed4e5a on main, which lands after 1b06f84cbd9, bumps the pointer to b32f48b9200. Current main is 4d9bec3bc1d and carries the fixed pointer.

So merging current main into the branch picks up the bumped pointer and the compile error goes away. Nothing in the agent-chat change needs to move.

Worth noting for the merge itself: a submodule pointer is the kind of thing a zero-conflict merge resolves quietly to the wrong side, so confirm git ls-tree HEAD vendor/bonsplit reads b32f48b9200 after merging, rather than trusting that the merge was clean.

🤖 Generated with Claude Code

[teamleaderleo]

Correction: I had the bonsplit pointer backwards

My previous comment named b32f48b9200 as the fixed pin and 83857fa043b as the stale one. It is the other way round, and the conclusion changes with it.

7d246ed4e5a (#15747) reverted the pin down to b32f48b9200, which predates BonsplitContrastPalette and TabPresence. Main's own macOS app target does not compile at current main:

Sources/TerminalSharingDisplay.swift:102:27: error: cannot find type 'TabPresence' in scope
Sources/TerminalSizeBoundsOverlayView.swift:19:34: error: cannot find type 'BonsplitContrastPalette' in scope

This branch's own pin, 83857fa043b, was already the good one. PR CI compiles the merge with main, so every PR inherits main's pin whatever its branch carries, which is why this one went red in two files it never touched. My #15238 carries b32f48b9200 and fails with the same four jobs plus TabPresence, from the same cause.

So there is nothing to change here and no merge to do. It is tracked on #15488, and #15930 restores the pin. This PR waits for that to land, then needs only a rerun.

Apologies for the detour; the instruction I sent alongside the earlier comment would have replaced a working pin with the broken one.

🤖 Generated with Claude Code

[teamleaderleo]

ios-simulator (ipad) is failing here on an inherited main red, not on this change.

The failing test is MobileSimulatorAuthPersistenceTests.missingAppIdentityCannotPersistTokens(), which exercises MobileAuthComposition.tokenStore and never touches AgentChatSessionRegistry+ChildRuns.swift:

✘ Test missingAppIdentityCannotPersistTokens() recorded an issue at
  MobileSimulatorAuthPersistenceTests.swift:64:25: Issue recorded
** TEST EXECUTE FAILED **

#14302 (5eda9315bba, merged this morning) changed tokenStore so an unresolvable bundle identifier selects .memory instead of .none, because .none leaves StackClientApp without a token store and any authenticated operation then traps in the SDK. That pull request touched only MobileAuthComposition.swift, so the assertion still requires .none and now contradicts the behavior it guards. It fails on any branch whose merge point includes that commit.

Fix is open as #16024. ios-simulator (iphone) is still queued here and will fail the same way.

[github-actions]

Merge receipt for a65cfb5536, merged 2026-09-30 14:03:01 UTC

• Not verified at merge: ios-simulator (ipad) (failure), ios-simulator (iphone) (in progress)
• Verified: ci-status, macOS compile admission, Web complexity, web-validation, CI fast guards, CI timing, detect-ios-changes, Fast static checks, GhosttyKit release check, guards (17), ios-e2e-status, ios-simulator-build, and 10 more
• Skipped by policy: app-host unit tests, admission-placement, backend, browser, Claude wrapper regressions, CLI product tests, Dogfood build #​${{ github.event.pull_request.number }}, ios-e2e, mac-host, release-admission, release-build, remote-daemon, and 8 more
• Full suite: runs on main after merge.