[AI-1839] Capture Codex collab subagents: child watchers, parent-aware import, locator guard - #515
Conversation
…e import, locator guard Codex CLI 0.146+ (multi_agent_version v2) forks every spawn_agent subagent into its OWN rollout under the shared ~/.codex/sessions tree, linked back via the child session_meta's parent_thread_id / thread_source:"subagent". kcap had zero handling: children were never watched live, import would land them as unrelated top-level sessions, and the daemon's rollout locator could mis-link a hosted session to one of its own children (same cwd, newer timestamp). - CodexSubagentDiscovery (Core): shared header-linkage discovery for the watcher scan, the teardown and the import walk. Pins the trap that a child session_meta's `session_id` holds the PARENT's id (own id only in `id`). - WatchCommand.ScanCodexSubagents: codex arm of the parent watcher's live subagent scan (mirrors Gemini/OpenCode) — fail-closed subagent-start, then a detached child watcher streaming the child rollout under its dashless thread id (AgentSubsession-*). Disk enumeration, so a restarted parent still recovers already-spawned children; definitive non-children cached, mid-write headers retried. - CodexSubagentTeardown: parent session-end synthesis (idle_timeout AND parent-exit — codex has no session-end hook) kills each child watcher, drains its tail and posts subagent-stop, before SessionEnded. - Import: subagent rollouts excluded from top-level discovery; the codex arm of SessionImporter.ImportSessionAsync imports every transitive descendant as a direct subagent of the root (flat AgentSubsession model, like Gemini), with vendor:"codex" stamped on child batches. - CodexSessionRolloutLocator.MatchRollout: a subagent rollout is a definitive non-match even when its cwd matches. Known residual: a parent already fully imported/recorded before this change is classified AlreadyImported on re-import, so its children are not retroactively attached; capture is forward-looking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Summary by QodoCapture Codex collab subagents via shared discovery, import nesting, and locator guard
AI Description
Diagram
High-Level Assessment
Files changed (8)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cc14c47e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // import each child as an unrelated top-level session — and the child's session_meta | ||
| // `session_id` field even holds the PARENT's id (its own id is in `id`), so nothing | ||
| // downstream may ever key a child by `session_id`. | ||
| transcripts = [.. transcripts.Where(t => CodexSubagentDiscovery.TryReadMeta(t.FilePath) is not { IsSubagent: true })]; |
There was a problem hiding this comment.
When an import overlaps creation of a collab child rollout, TryReadMeta deliberately returns null for a partial, locked, or temporarily unreadable header, but this predicate retains that rollout as a top-level session. It can then be imported under the child ID as an unrelated session and later imported again beneath its parent once the header becomes readable, leaving persistent duplicate/mis-associated history. Treat the unknown result as retryable or skip it for this discovery pass rather than classifying it as non-subagent.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b46a545 — ReadHeader now returns a tri-state verdict, and import discovery keeps only DEFINITIVE non-subagents: an Indeterminate header (empty/truncated — a session actively starting mid-import) is skipped for that discovery pass and picked up by the next run, so a child can no longer be imported top-level during the creation window.
| await SendAgentLifecycle( | ||
| httpClient, baseUrl, sessionId, sub.ChildDashlessId, subType, sub.FilePath, cwd, | ||
| transcriptPath, progress, vendor: "codex"); |
There was a problem hiding this comment.
When /hooks/subagent-start is rejected or remains unavailable after retries, this call still streams the child because SendAgentLifecycle catches the start failure; its transcript sender also defaults to failOnError: false, and the child ID is then reported before the outer import posts session-end. The server therefore may never open the AgentSubsession stream or accept its lines even though the import completes successfully, silently losing the newly supported Codex child history. As in the Gemini/OpenCode descendant import paths, require an acknowledged start and strict transcript delivery before continuing to stop/completion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in b46a545 — the codex descendant import is now fail-closed like the Gemini/OpenCode paths: no content without an acknowledged subagent-start, strict transcript delivery (failOnError: true), and no subagent-stop after a failed tail; a re-import retries idempotently.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…down auth, README - CodexSubagentDiscovery.ReadHeader replaces TryReadMeta with a tri-state verdict: Subagent / NotSubagent are DEFINITIVE (cacheable — a permanently malformed first line is ruled out instead of re-opened every polling tick), Indeterminate (empty / EOF-truncated line / IO error) is retried and never cached. Completeness is judged by newline presence, with a parseable-but- unterminated session_meta still judged on content. - Import discovery keeps only DEFINITIVE non-subagents: an indeterminate header (a session actively starting mid-import) is skipped for that pass rather than risking a child imported top-level now and nested next run. - The codex descendant import is now fail-closed like Gemini/OpenCode: no content without an acknowledged subagent-start, strict transcript delivery (failOnError), and no subagent-stop after a failed tail — a re-import retries idempotently. - CodexSubagentTeardown threads baseUrl into auth resolution so token/server selection matches the URL posted to. - README: Codex collab subagent capture + import nesting documented; the issue-tracker token in a test comment removed (CI rule). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
CI triage:
All six bot findings (2 Codex P1 + 4 Qodo) addressed in b46a545 — see the inline replies. |
|
Windows CI addendum: the re-run now fails on a different main-side test — |
|
Windows lane root-caused far enough to attribute definitively: the run that introduced Filed as AI-1848 with the full run table and starting points. Re-ran the failed job on this PR — main's |
1 participant
Closes the CLI half of AI-1839 — Codex CLI 0.146+ (
multi_agent_version: v2) forks everyspawn_agentcollab subagent into its own rollout under the shared~/.codex/sessionstree, and kcap captured none of it: children were never watched live,kcap import --codexwould land them as unrelated top-level sessions, and the daemon's rollout locator could mis-link a hosted session to one of its own children (children inherit the parent's cwd and are created moments later).What's here
CodexSubagentDiscovery(Core, new) — shared discovery over the childsession_metalinkage (thread_source: "subagent"+parent_thread_id), used by the live scan, the teardown and the import walk. Pins the trap that a child'ssession_meta.session_idholds the parent's id (its own id is only inid/the filename) — nothing may ever key a child bysession_id.WatchCommand.ScanCodexSubagents, the codex arm of the parent watcher's subagent scan (mirrors Gemini/OpenCode): fail-closedsubagent-start, then a detached child watcher streaming the child rollout under its dashless thread id (→AgentSubsession-*). Disk enumeration (not the in-bandsub_agent_activityevents) so a restarted parent watcher still recovers already-spawned children; definitive non-children are cached per-file, a mid-write header is retried, a later-day-dir child (midnight rollover) is found.CodexSubagentTeardown(mirrorsGeminiSubagentTeardown): on the parent's session-end synthesis (which for codex coversidle_timeoutand parent-exit — there is no session-end hook), kill each child watcher, drain its tail, POSTsubagent-stop, all beforeSessionEnded.SessionImporter.ImportSessionAsync's codex arm imports every transitive descendant as a direct subagent of the root (flatAgentSubsessionmodel, like the Gemini import), withvendor: "codex"stamped on child batches.CodexSessionRolloutLocator.MatchRolloutreturns a definitiveNofor any subagent rollout, even on a cwd match.Testing
CodexSubagentDiscoveryTests(12) pin the linkage parsing (incl. the parent-id trap), rule-out caching vs mid-write retry, midnight-rollover discovery, descendant flattening, and agent-type derivation; 2 newCodexSessionRolloutLocatorTestspin the guard. All pass.Capacitor.Cli.Tests.Unitrun: the only failures are the knownAgentOrchestratorVendorTests-family PTY timing flakes, which fail identically on an untouchedmaincheckout under load and pass individually in isolation (verified both).Residual
A parent session already fully imported/recorded before this change classifies
AlreadyImportedon re-import, so its children are not retroactively attached — capture is forward-looking. The server half (normalizer support foragent_messagehandoff prose + subagentsession_metasuppression) is the kcap-server PR on the same issue; this PR works against existing servers (thesubagent-start/stop+ agent-batch wire is vendor-neutral and long shipped).🤖 Generated with Claude Code