Repository navigation
Name the spawning call on a Codex subagent's start - #1939
Open
realtonyyoung wants to merge 1 commit into
Open
realtonyyoung wants to merge 1 commit into
realtonyyoung wants to merge 1 commit into
Conversation
An import posts every start after the parent transcript, so the server can place a subagent's row only by the call; a grandchild names its root-level ancestor's call, since it is imported onto the root Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Spawn-call discovery and payload src/Capacitor.Cli.Core/Harness/Codex/CodexSubagentDiscovery.cs |
The helper maps task_name values from parent-rollout spawn calls to call IDs. It resolves an agent path to its own or nearest ancestor’s call ID, and adds spawn_call_id to the payload when a value is available. |
Import and watch registration src/Capacitor.Cli/Commands/SessionImporter.cs, src/Capacitor.Cli/Commands/WatchCommand.cs, test/Capacitor.Cli.Tests.Unit/Harness/Codex/CodexSubagentDiscoveryTests.cs |
The import and watch flows pass resolved call IDs when registering subagents. Tests check exact and ancestor path matching, unmatched paths, and the resulting import payloads. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant SessionImporter
participant WatchCommand
participant CodexSubagentDiscovery
participant ParentRollout
participant SubagentStartEndpoint
SessionImporter->>CodexSubagentDiscovery: ReadSpawnCalls for parent rollout
CodexSubagentDiscovery->>ParentRollout: Scan spawn_agent calls and outputs
ParentRollout-->>CodexSubagentDiscovery: Return task names and call IDs
CodexSubagentDiscovery-->>SessionImporter: Return spawn-call mapping
SessionImporter->>CodexSubagentDiscovery: Resolve call for agent path
SessionImporter->>SubagentStartEndpoint: Send subagent-start payload
WatchCommand->>CodexSubagentDiscovery: Read and resolve spawn calls on demand
CodexSubagentDiscovery->>ParentRollout: Scan spawn_agent calls and outputs
CodexSubagentDiscovery-->>WatchCommand: Return matching call ID
WatchCommand->>SubagentStartEndpoint: Send subagent-start payload
Suggested reviewers: inok
Merge Risk: 
56856
-
The spawn-call association is ready to merge after normal checks; no actionable risk remains.
Security architecture risk: 
56856
-
The change preserves existing client-side child identity and registration safeguards. No introduced security vulnerability was established, but server-side ownership validation and compatibility with older deployments could not be confirmed.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The demonstrated new exposure is a transcript-derived string crossing the existing lifecycle-hook boundary for imported or watched Codex children. The inspected client changes do not grant tool execution, credentials, or a new destination. Effective downstream scope cannot be bounded without the receiver's session and tenant checks.
Trust Boundaries and Controls
- observed — Transcript content influences the outbound call reference, but unrelated tool outputs are excluded unless their call ID was previously recorded for spawn_agent. JSON shape guards reject wrong-typed fields, malformed JSON is skipped, and I/O or access failures produce no mapping. These parsing controls do not authenticate transcript contents or establish authorization for the referenced server object.
Resilience and Maintainability Implications
- observed — Existing failure containment remains: live watching starts child streaming only after successful registration and removes failed registrations from the seen set for retry. Import likewise skips content after failed registration and withholds stop after transcript delivery failure. The new field does not alter these transition gates.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding the spawning call name to Codex subagent start events. 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.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
realtonyyoung
marked this pull request as ready for review
October 25, 2026 02:44
PR Summary by QodoPlace Codex subagents at their spawning calls
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
Code Review by Qodo
1. JSON type checks bypass shared helpers
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+307
to
+308
| if (output.TryGetProperty("output", out var body) && body.ValueKind == JsonValueKind.String) { | ||
| try { |
There was a problem hiding this comment.
1. Json type checks bypass shared helpers 📘 Rule violation ⚙ Maintainability
TaskNameOf compares JsonElement.ValueKind directly for string and object detection instead of using the available JsonElementExtensions helpers. Any future type-checking change must be repeated in these branches, leaving the new Codex rollout parsing path inconsistent with the shared JSON conventions.
Agent Prompt
## Issue description
The new `TaskNameOf` implementation directly compares `JsonElement.ValueKind`, contrary to the shared JSON helper convention.
## Fix Focus Areas
- src/Capacitor.Cli.Core/Harness/Codex/CodexSubagentDiscovery.cs[307-311]
## Recommended Fix
Replace the direct string and object `ValueKind` comparisons with the corresponding `JsonElementExtensions` methods, such as `IsString()` and `IsObject()`, while preserving the existing parsing behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+263
to
+268
| var day = DayDir(tmp.Path); | ||
| var parent = WriteParent(day); | ||
| File.AppendAllLines(parent, [ | ||
| Item("""{"type":"function_call","name":"spawn_agent","namespace":"collaboration","call_id":"call_spawn","arguments":"{\"task_name\":\"spec_quality\"}"}"""), | ||
| Item("""{"type":"function_call_output","call_id":"call_spawn","output":"{\"task_name\":\"/root/spec_quality\"}"}"""), | ||
| Item("""{"type":"function_call","name":"exec_command","call_id":"call_exec","arguments":"{}"}"""), |
There was a problem hiding this comment.
2. Test writes temp files outside helpers 📘 Rule violation ⚙ Maintainability
The new test appends directly with File.AppendAllLines(parent, ...) after deriving parent from tmp.Path, rather than using the TempDir file helpers for state under the temporary root. This bypasses the helper's path and file-operation abstraction in the new filesystem-writing path, even though disposal still occurs at the end of the test.
Agent Prompt
## Issue description
The new test performs direct filesystem writes under `tmp.Path` instead of using `TempDir` path and file helpers.
## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Harness/Codex/CodexSubagentDiscoveryTests.cs[263-271]
## Recommended Fix
Construct the rollout paths with `TempDir.PathTo` or `CreateDir`, and create or update the parent rollout through `TempDir.CreateFile` or another helper-backed operation rather than calling `File.AppendAllLines` directly under the temporary root.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of AI-3070 (no GitHub issue)
What & why
A Codex import posts every subagent's start after the whole parent transcript, so the server puts every Codex agent row under the last turn. The start now carries
spawn_call_id: the parent'sspawn_agentcall whose output names the agent'sagent_pathas itstask_name. A grandchild is imported onto the root, so it names its nearest root-level ancestor's call. The live watcher sends the same field, read once per scan that finds a new childThe server half, which stores the call on the start and places the row by it, is kurrent-io/kcap-server#2673. A server without it ignores the field
Where to look
ReadSpawnCallsreads the whole parent rollout once per import. A spawn whose output is not written yet names no call, and the start keeps its own positionVerification
An_imported_subagent_start_names_the_call_that_spawned_it_or_its_ancestordrivesSessionImporter.ImportSessionAsyncover a rollout tree with a child, a grandchild, an agent whose path only another tool's output carries, and a truncated last line. Red when the importer sends no call, and when a non-spawn output is accepted:Expected to be equivalent to [[019fec44…, call_spawn], …]A_path_takes_its_own_spawning_call_or_its_nearest_ancestorsis red on/root/a/b/cwhen the walk stops at the exact pathtask_nameoutputs match real Codex 0.146 rollouts🤖 Generated with Claude Code
Summary by CodeRabbit