Repository navigation
fix(responses): read V1 collaboration tools rendered inside code-mode exec - #6977
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The change detects V1 collaboration tools listed in code-mode exec descriptions. It fails closed on ambiguous or missing evidence, and no merge-blocking risk is identified. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #6844 requires proactive V1 Max/Ultra guidance when V1 tools are available behind Codex code mode, while preserving direct detection and negative controls. The PR implements only rendered-listin… |
Add a reliable request-visible capability signal for deferred V1 code-mode turns, or add an equivalent host-provided signal before detection. Keep bare exec, saved mode, and unsupported hosts as negative controls. Add tests for the deferr… |
| Docstring Coverage | |
Docstring coverage is 57.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 3 files. (5 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (3 passed)
-
Check name Status Explanation Out of Scope Changes check The changed implementation, collaboration integration, regression tests, layout fixture, and documentation all support the code-mode collaboration-surface objective in issue #6844. The plan record doc…Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: detecting V1 collaboration tools rendered inside code-mode exec descriptions.
Full details: Linked Issues check
-
Explanation
Issue
#6844requires proactive V1 Max/Ultra guidance when V1 tools are available behind Codex code mode, while preserving direct detection and negative controls. The PR implements only rendered-listing evidence insrc/server/responses/code-mode-collab-surface.ts:1-186.src/server/responses/collaboration.ts:317-351still returns the direct result when no listing exists. The new regression test explicitly confirms that the deferred-V1 shape remainsnulland emits no guidance intests/codex-integration/code-mode-collab-surface.test.ts. The PR description states that every OpenCodex catalog row uses deferred tools. Therefore, the affected deferred case remains unresolved, even though the rendered-listing case and effort gates are tested.Resolution
Add a reliable request-visible capability signal for deferred V1 code-mode turns, or add an equivalent host-provided signal before detection. Keep bare
exec, saved mode, and unsupported hosts as negative controls. Add tests for the deferred-V1 positive case, the negative controls, direct-tool compatibility, and the existing Max/Ultra and guidance gates. If no reliable signal exists, do not claim that issue#6844is fixed.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 57.89% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 3 files. (5 skipped: 5 unsupported.)
- Fix all pre-merge checks with AI
✨ 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
-
-
-
-
-
-
-
- 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔇 Additional comments (8)
devlog/_plan/261012_codemode_collab_surface/010_plan.md (1)
1-139: LGTM!src/server/responses/collaboration.ts (1)
320-327: LGTM!tests/codex-integration/code-mode-collab-surface.test.ts (1)
1-340: LGTM!scripts/test-layout/layout.json (1)
1247-1247: LGTM!tests/fixtures/test-layout-expected.json (1)
776-776: LGTM!structure/subagents.md (1)
432-451: LGTM!docs-site/src/content/docs/guides/sub-agent-surface.md (1)
131-133: LGTM!src/server/responses/code-mode-collab-surface.ts-122-136 (1)
122-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Fix: a declaration with an open fence inside a heading section can mis-skip lines.
At Line 122, the declaration branch runs only if
lines[i + 1] === DECLARATION_FENCE. The check is strict string equality, so a trailing space or\rmakes the fence fail the check. This is consistent with the "fail closed" intent for evidence. The line goes toopenedFenceinstead, and the loop treats the declaration as a quoted fence. That is safe for V1 evidence.CRLF line endings are a concrete case. A description with
\r\nfails every===check, andTOOL_HEADINGfails on the trailing\r. The result isnull, which is a fail-closed outcome. The contradiction check inmentionedToolNamesfails the same way, because the heading regex does not match.declare const tools:still matches bymatchAll, so the check remains liberal there.No defect is established for the changed path. Add one regression test that covers a CRLF description. The test pins the intended fail-closed outcome.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
49b9ed25-dfa0-464d-9b29-af07fff4a97f
📒 Files selected for processing (8)
devlog/_plan/261012_codemode_collab_surface/010_plan.mddocs-site/src/content/docs/guides/sub-agent-surface.mdscripts/test-layout/layout.jsonsrc/server/responses/code-mode-collab-surface.tssrc/server/responses/collaboration.tsstructure/subagents.mdtests/codex-integration/code-mode-collab-surface.test.tstests/fixtures/test-layout-expected.json
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
![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: c2fb8ee7d5
ℹ️ 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".
…6844) collabSurface() only recognised a directly declared spawn_agent. Codex code-mode-only turns nest the collaboration tools inside the custom exec description, so the surface was null and multiAgentGuidanceText() returned before the V1 Max/Ultra gate. Read the nested listing codex-rs renders (tool heading, description, final "exec tool declaration:" fence for the same tool) from a genuine freeform code-mode exec, and accept only multi_agent_v1__spawn_agent as V1 evidence. Any other spawn_agent or V2 companion beside it, or a different direct surface, is contradictory and stays null; nested evidence never yields v2. Deferred V1 tools (every OpenCodex catalog row advertises tool search) are not rendered and still carry no request-visible signal, so those turns keep receiving no V1 proactive text. Refs #6844
…ions Track CommonMark fences by character and length, end a nested section at its own declaration so usage-hint headings stay inside it, classify identity from the raw heading name, and keep scripts/test-layout/layout.json under the 2000-line ratchet.
An unowned example declaration no longer clears pending headings, the declaration fence closes by the shared CommonMark rule, and only a complete rendered declaration counts.
A raw-name suffix must normalize to its heading, the outermost same-name heading owns a declaration, and a declaration needs a named typed parameter and a non-empty return type.
V1 evidence still needs a rendered section, but another surface is now detected from any heading or declaration line, quoted or not, so copied text can only make a turn null.
A direct-only MCP namespace such as mcp__slack.send_message no longer suppresses a rendered V1 listing, matching the nested check that already ignores mcp__ names.
450bed3 to
799542a
Compare
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
Independent focused review of 799542a8bf73bfc9706d948e74806844668867b8, based on trusted dev a0191a1b6fd921a6e5a9d12cb123c8df88b023e7.
I read the complete production parser/collaboration diff, the 41-case fixture and the contract/docs changes. Executed bun test --no-env-file --preload ./tests/preload.ts --isolate ./tests/codex-integration/code-mode-collab-surface.test.ts --timeout 15000: 41 passed, 0 failed, 57 assertions, Bun1.4.0/Linux. This is synthetic request parsing/guidance generation, not a native Codex host acceptance test or a live provider call.
The narrow boundary is retained: a complete rendered V1 declaration can establish V1 evidence, bare/quoted/truncated/foreign shapes cannot, contradictory native collaboration names return null, and MCP companion names do not veto otherwise consistent native V1. The fixtures explicitly pin the important exception: contradictory rendered V1 beside direct V2 withdraws the V2 designation and therefore its effort cap; this is a deliberate behavior change, not a claim that direct V2 is universally untouched. Medium/disabled guidance and deferred V1 remain silent.
This still does not fix the common deferred-tool shape described in #6844; no request-visible evidence is added for that shape and the issue should remain open. A future fallback needs a separate owner decision, not inference from the saved catalog/mode. The rendering examples and mutation receipts in the description remain author evidence beyond my fixture run.
No new concrete source blocker found in the reviewed scope. This is COMMENTED, not approval: current-head Cross-platform CI 38220051016 and target/hygiene metadata runs are queued; I have not independently run typecheck, full suites, native host rendering or a new scan. No CI authorization, sponsorship, branch modification or merge. The single test unit used fresh private homes and verified CPU75%/memoryHigh1280MiB/max1536MiB/swap0/tasks64/nice10 with a finite deadline; IOWeight20 is configured without an enforcement claim. No live configuration or service was changed.
|
Maintainer integration into |
…tools> fragment Codex defers the V1 collaboration tools whenever the model supports tool search, which every OpenCodex catalog row does. The code-mode exec description then lists none of them (#6977 reads only the rendered case), so collabSurface() stayed null and V1 Max/Ultra turns never received the proactive delegation guidance. When its deferred_tool_world_state feature is on (Codex Desktop sessions receive it), codex-rs sends a developer <tools> fragment listing the deferred namespaces, then Added/Removed diffs on later turns. deferred-collab-surface.ts replays those fragments in input order from developer messages only and treats a current multi_agent_v1 row as V1 evidence, beside a genuine code-mode exec or a structured tool_search host. Omitted rows never establish a namespace; omitted removals and malformed fragments clear what was carried. The evidence combines with the direct and rendered surfaces exactly like #6977: it never yields v2 and yields to any contradicting surface. Clients without the feature still carry no V1 evidence, and the row cannot show whether an embedder's allowlist dropped spawn_agent alone; both limits are documented in structure/subagents.md. Refs #6844
…tools> fragment Codex defers the V1 collaboration tools whenever the model supports tool search, which every OpenCodex catalog row does. The code-mode exec description then lists none of them (#6977 reads only the rendered case), so collabSurface() stayed null and V1 Max/Ultra turns never received the proactive delegation guidance. When its deferred_tool_world_state feature is on (Codex Desktop sessions receive it), codex-rs sends a developer <tools> fragment listing the deferred namespaces, then Added/Removed diffs on later turns. deferred-collab-surface.ts replays those fragments in input order from developer messages only and treats a current multi_agent_v1 row as V1 evidence, beside a genuine code-mode exec or a structured tool_search host. Omitted rows never establish a namespace; omitted removals and malformed fragments clear what was carried. The evidence combines with the direct and rendered surfaces exactly like #6977: it never yields v2 and yields to any contradicting surface. Clients without the feature still carry no V1 evidence, and the row cannot show whether an embedder's allowlist dropped spawn_agent alone; both limits are documented in structure/subagents.md. Refs #6844
…tools> fragment Codex defers the V1 collaboration tools whenever the model supports tool search, which every OpenCodex catalog row does. The code-mode exec description then lists none of them (#6977 reads only the rendered case), so collabSurface() stayed null and V1 Max/Ultra turns never received the proactive delegation guidance. When its deferred_tool_world_state feature is on (Codex Desktop sessions receive it), codex-rs sends a developer <tools> fragment listing the deferred namespaces, then Added/Removed diffs on later turns. deferred-collab-surface.ts replays those fragments in input order from developer messages only and treats a current multi_agent_v1 row as V1 evidence, beside a genuine code-mode exec or a structured tool_search host. Omitted rows never establish a namespace; omitted removals and malformed fragments clear what was carried. The evidence combines with the direct and rendered surfaces exactly like #6977: it never yields v2 and yields to any contradicting surface. Clients without the feature still carry no V1 evidence, and the row cannot show whether an embedder's allowlist dropped spawn_agent alone; both limits are documented in structure/subagents.md. Refs #6844
Summary
Refs #6844. This fixes the part of the issue the request can prove. The rest needs a decision only a maintainer can make, described below.
Problem.
collabSurface()recognised a collaboration surface only from a directly declaredspawn_agent. In Codex code-mode-only turns the collaboration tools are not top-level wire tools, so the surface wasnullandmultiAgentGuidanceText()returned before the V1 Max/Ultra gate. "Tell Codex how to split work" never reached those turns.What a code-mode request can prove (codex-rs
openai/codex@ ec8251c54, cross-checked against the 2026-09-22 tree):build_exec_tool_descriptionrenders each one inside the customexecdescription as a tool heading, its description and a finalexec tool declaration:ts fence. Namespaced tools are flattened, so V1 readsmulti_agent_v1__spawn_agent.spec_plan.rs, V1 branch ofadd_collaboration_tools) and leaves it out of the description. Every OpenCodex catalog row setssupports_search_tool: true(src/codex/catalog/parsing.ts; all 12 bundled rows as well), so this is the shape OpenCodex users get, and almost certainly the one the issue reports. In a Codex code-mode session routed through OpenCodex, the isolate'sALL_TOOLSlistsmulti_agent_v1__spawn_agentand its four companions, while the request-visibleexecdescription lists none of them.<multi_agent_mode>item only on V2.ModelCatalogStatedepends on the default-offmodel_catalog_in_contextfeature. No header names the surface. The session also pins its version in history, so neither the saved OpenCodex mode nor the catalog stamp proves which surface a conversation uses.Change.
src/server/responses/code-mode-collab-surface.tsreads the rendered listing from a genuine code-modeexec: un-namespaced, freeform, not beside a bare shell bridge, and starting with the codex-rs template line. A section counts only when a top-level tool heading is followed by its own declaration. Code fences are tracked the CommonMark way, and identity comes from the raw heading name. Onlymulti_agent_v1__spawn_agentcounts as V1 evidence.collabSurface()is unchanged when no listing exists. If the listing also contains anotherspawn_agentor a V2 companion, or the direct tools show a different surface, the result isnull. Nested evidence never yieldsv2, so the V2 designation, the V2 effort cap and native steering behave exactly as today. The one exception is a direct V2 surface beside a rendered V1 listing: codex-rs never sends that pair, it is now treated as contradictory, and the test pins that outcome.Not changed: deferred V1. These turns still carry no evidence and stay
null, so they still receive no V1 proactive text. Whether to add some fallback for them without a host signal is a design decision. This PR does not make it, so it does not close #6844.A tool description that copies a complete V1 section verbatim cannot be told apart from the host's own rendering. The worst outcome is the V1 proactive hint, or a contradictory
nullbeside a direct V2 surface.Docs:
structure/subagents.mdgains a paragraph on surface detection, and the Englishsub-agent-surfaceguide gains one sentence. The translated pages do not discuss detection, so they do not contradict the English text.Verification
Local, on head
799542a8bf(rebased ontoorigin/deva0191a1) unless noted:bun run typecheck: exit 0.bun test tests/codex-integration/code-mode-collab-surface.test.ts tests/codex-integration/multi-agent-compat.test.ts tests/codex-integration/effort-policy.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts tests/ci-workflows/structure-ssot.test.ts: 235 pass / 0 fail (rerun after the final rebase).bun run structure:check: passed.bun run privacy:scan: passed.bun testfrom the repository root (bunfig preload sandboxes HOME, OPENCODEX_HOME and CODEX_HOME): every test file that referencescollabSurface,multiAgentGuidanceText, steering normalization,effortCapAppliesToorspawn_agent, plus all oftests/responses/. Result: 4694 tests, 4662 pass / 31 fail / 1 skip. All 31 failures are inresponses-compaction-recovery.test.tsandmessages-native-bridge-policy.test.ts, which share state when they run in one process. Run alone, each passes (55/55 and 7/7).execdescription was assembled from the V1 tool descriptions of a live Codex code-mode session (five V1 tools and six helpers, 14,748 characters; kept out of the repository). It givesv1and emits guidance at Ultra. With the V1 sections removed (the deferred shape), it givesnull.Full suite: AGENTS.md resource exception.
bun run testwaited more than 30 minutes on the shared per-user Bun test lock while five other worktrees' runs held or queued for it, and it did not start. An earlier, interrupted full run on a previous revision of this branch completed 1599 of 2237 test files. Its only failures were the file-size ratchet, since fixed by putting the layout entry on an existing line, and a 5-second timeout intests/cli/cli-access-audio-live.test.ts, which passes alone (36/36) and does not import the changed code. A second full run did get the lock later. It ran on revisionc2fb8ee7d5and was stopped when the branch was rebased under it. By then it had covered 1877 test files with 38,312 passes and one failure, the same audio timeout. Full coverage on the final head is left to hosted CI.Review. The Codex PR review P2 finding is fixed in
799542a8bf: anmcp__direct tool such asmcp__slack.send_messageno longer vetoes the rendered V1 listing, and a regression test covers it. GPT-6.1-Sol (high), a different model family from the builder, reviewed the work. The plan audit went FAIL, then NEAR-PASS. Code review went FAIL four times, with every finding folded in, then PASS on the final approach. The record is indevlog/_plan/261012_codemode_collab_surface/010_plan.md.No live service,
~/.opencodexsetting, Codex app or GOAT server was touched.Checklist