Repository navigation
fix(native-chat): preserve running subagent rows in long chats - #26798
Conversation
|
Long chats could discard an old spawn group while its subagents were still running. Claude then marked those children as contact lost; Codex could lose the group's full membership on a later report and leave children shown as working after the session ended. The cache deleted ownership based on age rather than completion. Claude, Codex, and agents connected through Agent Client Protocol (ACP) now use one retention rule: keep every group with unfinished children and release only settled history after its final row is accepted and published or queued for delivery. A bounded history of 2,048 recent finished-child identifiers prevents delayed reports from recreating working rows while allowing a real new run. Claude keeps its existing background-child and turn/session-end behavior. The existing Codex review found and fixed missing suppression of a child's earlier completed turns. The Claude adversarial review found two P2 identity regressions in the first extension: ignoring a replayed start also skipped restoring its spawn-call mapping, and child traffic could immediately evict a claimed journal row before its resume announcement. Both are fixed and independently verified failing before/passing after. Replayed starts restore the mapping without rewriting the roster. A just-accessed inherited row retains its original membership, labels, turn placement, and run count through the following resume. Intended difference: at most one accessed settled row may temporarily exceed the cache target; this exception lasts for one trim call and is re-derived on the next relevant event. There is no stored exemption or new durable obligation. Final readiness found no new issues; no PR findings remain open. Verified locally on macOS: 187 distinct tests across 15 targeted suites, final queued node typecheck, all 22 repository static checks before push, and the staged changed-code gate. Tests cover more than 33 spawn groups, complete membership, real child completion, session sweep, refused final append/publication and retry, settled eviction, late reports after alias-cache churn, and new runs of an existing child. Existing suites exercise real journals and restart behavior. Fresh review additionally verified 2,500 inherited settled rows with exactly 32 live groups plus one accessed row, the 2,048-identifier cap, preservation of the fifth run, original labels, and turn placement, old-result fencing, and zero trim calls for repeated activity on a loaded child. Live Electron QA on the same final head now verifies the rendered long-chat behavior using matched protocol stand-ins on base and head. Both original children remain represented after 34 turns, real completion settles them, and the delayed child-2 report creates no working row. The supervising agent inspected all accepted screenshots and timestamps and checked native identities and journals. The PR body now contains before/after proof and the full 24-cell normal-flow table: 22 PASS, Codex individual Stop unsupported on base, and Grok native-history UI reopening blocked on base. Claude/Codex same-chat reopening passed through the real Git-workspace History UI; Grok persisted-row reopening passed separately with explicit fixture assistance. Folder-history reopening and Grok native ownership are coordinator follow-ups, not roster regressions. All test rigs stayed hidden and used disposable isolated homes; drivers were Codex gpt-5.6-terra at medium effort. All QA workers were released, only recorded owned test processes were stopped, and final real-home hashes were recorded with zero errors. 10 changed paths were reported with before/after hashes and available UTC mtimes (the two temporary SQLite files disappeared after hashing); an unrelated pre-existing interactive Grok process was verified read-only and left untouched. No authentication contents were read, copied, or restored. Not verified: real long provider chats or interactive Windows/Linux/WSL/SSH/mobile app QA. No structured-Claude production file changed, so the conditional real-Claude CLI suite did not apply. Deferred scope remains Codex reconstruction after a restart for activity outside a turn, and the independently reproduced baseline case where heavy execution-cache churn can reopen a settled sibling inside a still-live group. A unified tracker across agents is a separate follow-up. Final head: Rendered proof and the normal-flow result table: PR Visual Proof. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to Long chats keep rows for running subagents instead of evicting them, and settled history stays bounded. No concrete merge-blocking risk was found. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 16 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly identifies the main change: preserving running subagent rows during long chats. It is concise and directly related to the implementation. Description check The description is detailed and covers the user impact, implementation, rationale, testing, review, visual evidence, and known limitations. It does not provide a linked issue number and omits some tem…
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
brennanb2025
changed the title
brennanb2025
changed the title
* fix(codex): keep running subagent groups in long chats * fix(codex): remember superseded settled child turns * fix(native-chat): retain running Claude subagent groups (cherry picked from commit dab3f6e)
ELI5
Long chats could lose track of subagents that were still running. Claude marked them as contact lost after more than 32 spawning turns; Codex could drop their full membership and leave them shown as working after the session ended.
What Changed
Claude, Codex, and agents connected through Agent Client Protocol (ACP) now share one rule: keep groups with unfinished children; release only settled groups after their final row is accepted and published or queued for delivery. Running groups keep all their children and receive their real completion or the existing session-end verdict.
A bounded history of 2,048 recently finished child identifiers prevents late reports from recreating working rows, while permitting a real new run of the same child. Claude keeps its existing background-child, turn-end, session-end, and journal inheritance behavior. Replayed Claude announcements still map the spawn call to the child, so later output stays attributed to the right child. A just-inherited row remains reachable while child traffic precedes its resume announcement; the next relevant event applies the ordinary retention rule again. At most one accessed settled row can temporarily exceed the cache target.
Why
Deleting the oldest group discarded ownership of live work. Increasing the limit would postpone the same failure. The shared policy releases settled history while retaining active obligations; the approved scope extends existing retention rather than building another tracker.
A unified subagent tracker is a separate follow-up. Codex reconstruction after a restart for activity outside a turn remains outside this change.
Linked Issue
Maintainer follow-up; no pre-existing issue.
Visual Proof
Hidden, compiled macOS Electron QA exercised the actual renderer through Playwright and Chrome DevTools Protocol. The UI driver was Codex
gpt-5.6-terraat medium effort; the supervising agent personally inspected each accepted screenshot and its file timestamp, then checked the native chat identity and persisted roster journal. Protocol stand-ins supplied the agent events; these were simulated chats, not paid 34-turn provider conversations.The same final stand-in script for each agent ran on base
6ef42adfc0effb744d83f8240268079e8987af39before head9b459756805420d46f469e046105154027246048. Turn 1 starts two children that stay running while turns 2–34 each start and finish another child. A later progress report checks preservation of the original membership. The original children then settle, and a delayed start/progress report for finished child 2 must leave it settled.Long chats
Claude: PASS. Base marks both still-running originals as status unavailable. Head keeps both children, their full count, running strip, and sidebar work.
Base:
Head:
Codex: PASS. Base shrinks the original two-child row to one child on a later progress report. Head retains both originals, count 2, running strip, and sidebar work.
Base:
Head:
Grok (Agent Client Protocol): PASS. Control: both base and head retain the two running originals. Head also passes settlement and delayed-report checks.
Base:
Head:
Every head journal preserves 34 groups and all 35 children. Claude and Grok support stopping one original through its visible row while the sibling keeps working, then settle the sibling normally. Codex settles both normally; individual Stop is already unsupported on base. The delayed report leaves child 2 completed and creates no working group. Additional rendered evidence:
Normal-flow regressions
22 PASS; 1 unsupported capability; 1 native-history UI blocker reproduced on base. Each cell links the rendered evidence; the reasons below distinguish the two exceptions.
Saved-chat sources before closing: Claude, Codex, Grok.
Completion shows the full settled roster. Parallel children overlap before both finish. Parent Stop settles both children. Individual Stop leaves the sibling running before final settlement. Background children remain working after the parent finishes and later complete. Reopened chats keep the same native identity and original settled child; each following fresh chat has a different identity and completes its first turn. Short chats finish with no child roster, running strip, or child sidebar.
Saved-chat checks used Git workspaces. Because the protocol stand-ins do not write CLI history, their actual logged messages were written to history fixtures inside the isolated test homes. Claude and Codex were closed and reopened through the real History UI, with the exact native chat identifier checked before and after. Grok's UI route is blocked: its history projection omits native ownership. The existing host reveal operation then reopened that same closed Grok chat as an explicitly fixture-assisted reopening; its original settled row remained correct, and the following fresh Grok chat passed. This does not count as a passing Grok History UI test: fixture-assisted persisted-row proof.
Existing limitations, held outside this roster PR at the coordinator's direction:
Isolation and limits
All test windows remained hidden; no desktop input or focus activation was used. Each compiled rig used its own disposable profile and native home, absolute copied stand-in binaries, an environment allowlist, mock keychain/basic password store, disabled login-shell discovery, disabled agent hooks, and telemetry opt-out. Only verified recorded test PIDs were stopped. All supervised QA drivers were released and the laptop QA slot was freed after teardown.
Real-home SHA-256 checks completed before activation and after teardown (67,248 before / 67,250 after files; zero hashing errors). Changed paths:
.grok/auth.json,.grok/auth.json.lock,.grok/logs/unified.jsonl,.grok/models_cache.json,.grok/sessions/session_search.sqlite,.grok/sessions/session_search.sqlite-shm,.grok/sessions/session_search.sqlite-wal,.grok/settings_cache.json,.grok/version.json,.grok/worktrees.db. Full before/after hashes and available UTC file mtimes are retained in the local QA report. The two SQLite temporary files disappeared after hashing, so their report-time mtimes are unavailable..orca(including keybindings), Claude settings, Codex configuration with trust rows stripped, and Codex hooks were unchanged. An unrelated interactive Grok process was already running since October 8 in another workspace; its PID and working-directory metadata were verified read-only, and it was left untouched. The rigs used isolated Grok homes and never invoked the real Grok CLI. Authentication contents were never read, copied, or restored.Untested: real long provider conversations and interactive Windows/Linux/WSL/SSH/mobile app flows. Stale Codex evidence from an expired stand-in and an earlier incomplete protocol fixture was rejected; only the corrected, matched base/head runs above count. No production changes were needed during live QA; the PR remains unmerged.
Testing
Review
The existing Codex/ACP work completed its architecture challenge and adversarial review. The Claude extension received one adversarial review and a fresh review of the fixes, plus a final readiness review. The review-summary comment records fixed and deferred findings and verification limits.
Agent skill upstream boundary
Checklist