Skip to content

fix(agent): allow Code Mode subagent tools - #2473

Merged
zerob13 merged 1 commit into
devfrom
dimagent/fix-2471-code-mode-contract
Oct 27, 2026
Merged

zerob13 merged 1 commit into
devfrom
dimagent/fix-2471-code-mode-contract

Conversation

@zerob13

@zerob13 zerob13 commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Code Mode subagents reject native and MCP subtool calls because their frozen ExecutionContract lists only provider-visible entrypoints. On the Codex frontend, native exec also collides with the distinct Code Mode exec target.

Bind nested dispatch to the configured outer run_code, exec, or wait entrypoint. Validate its frozen target and execution mode, and bound the child's effect by both the current parent policy and the frozen ceiling. Keep request identity, live tool/server restrictions, workspace, and subagent-depth checks in place.

Fixes #2471.

Validation:

  • 133 tests passed across ExecutionContract, ToolService, and Code Mode suites, including both function and Codex frontends.
  • pnpm run format:check
  • pnpm run i18n
  • pnpm run lint
  • pnpm run typecheck

The integration regression uses a simulated utility-process transport. Live desktop/model end-to-end verification was not performed.

Summary by CodeRabbit

  • Bug Fixes
    • Nested tool calls in Code Mode now run under the applicable parent tool’s execution limits, including effect and mode restrictions.
    • Improved handling of nested calls across supported Code Mode entry points, including execution, reading, file matching, and MCP tools.
    • Calls that exceed the parent tool’s limits or have invalid targets continue to be rejected.

@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ad1954f0-2fb6-486b-81e2-fb57b55d30c2
Commits

Reviewing files that changed from the base of the PR and between 2a8a36b and a087ba1.

Files selected for processing: 4
  • src/main/tape/domain/executionContract.ts
  • src/main/tool/index.ts
  • test/main/tape/executionContract.test.ts
  • test/main/tool/toolService.test.ts

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

Walkthrough

Code Mode now passes its entry tool definition to nested execution-contract checks. Those checks use the parent tool’s frozen ceiling to validate nested dispatch. Tests cover contract limits and nested Agent and MCP tool calls.

Changes: Nested Code Mode Dispatch

Layer / File(s) Summary
Parent tool dispatch authority
src/main/tape/domain/executionContract.ts, test/main/tape/executionContract.test.ts
Dispatch checks use the parent tool’s frozen ceiling when provided. Tests cover parent membership, target matching, effect and mode limits, workspace, subagent depth, and request identity.
Code Mode parent propagation
src/main/tool/index.ts, test/main/tool/toolService.test.ts
Code Mode passes the matching entry-tool definition through runtime options to nested dispatch validation. Tests exercise nested Agent and MCP calls under both provider entrypoints.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: yyhhyyyyyy

Merge Risk: Minimal · up to a087b

No actionable merge-blocking issue was established; the change is ready to merge after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
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. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: enabling Code Mode subagent tools. It matches the pull request objectives and changes.
Linked Issues check Passed Issue #2471 requires Code Mode nested tools to use the outer entrypoint ceiling, limit child effects, and preserve workspace and subagent-depth checks. executionContract.ts uses the supplied parent …
Out of Scope Changes check Passed The changes stay within issue #2471. Production changes add parent-entrypoint dispatch authorization and pass the entrypoint through Code Mode runtime options. The added tests verify the required nest…
Full details: Linked Issues check

Explanation

Issue #2471 requires Code Mode nested tools to use the outer entrypoint ceiling, limit child effects, and preserve workspace and subagent-depth checks. executionContract.ts uses the supplied parent tool for frozen membership, target, and mode validation. It also requires the child effect to fit within the parent effect. tool/index.ts supplies the configured function or Codex Code Mode entrypoint as the parent. Tests cover exec, read, glob, and an MCP tool for both deepseek and openai-codex. The contract tests retain coverage for workspace, depth, and request identity checks.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #2471. Production changes add parent-entrypoint dispatch authorization and pass the entrypoint through Code Mode runtime options. The added tests verify the required nested native and MCP calls and the existing security restrictions. No unrelated behavior or files appear in the reviewed diff.

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. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 4 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

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.

❤️ Share

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

@zerob13 zerob13 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: approve, no blocking findings

This is a self-authored PR, so GitHub does not allow submitting a formal approve or request-changes review. Recording the verdict here instead.

What it does

Before this change, a Code Mode turn froze only its top-level entrypoint (run_code, or exec / wait on the Codex frontend) into the execution contract, but nested subtool calls were validated against that same frozen list. Since native and MCP subtools are never in that list, every tools.read(...) / tools.edit(...) / tools.mcp__x__y(...) call was rejected. The model could see the tools and still not use them.

Now a nested call is bounded by its parent entrypoint: the parent must be in the frozen View with a matching target and mode, and the child's effect must fit inside the parent's effect. The child no longer needs its own entry in the View. Identity, workspace, subagent-depth, disabled-tool and enabled-MCP-server checks are unchanged.

Why I think it holds

  • The child definition is not model-chosen: nested calls are resolved by bindingId from the cell's own binding table (src/main/tool/codeMode/runCodeRuntimeManager.ts:641), and the definition hash is journaled. The new check therefore cannot be used to reach a tool that was never offered to the sandbox.
  • wait is safe: a resumed cell inherits the wait call's options, and wait is a write-effect tool present in the Codex View, so subtool calls after a resume still satisfy both containment checks.
  • The execution catalog is re-resolved every turn (src/main/agent/deepchat/runtime/turnCoordinator.ts:390, src/main/agent/deepchat/runtime/deepChatLoopRunner.ts:1015), so requiring the parent entrypoint to be in the current request's View still enforces per-request exposure of Code Mode itself.
  • CI on a087ba19 is green, and the service-level test does reproduce the original failure path rather than asserting internals.

Non-blocking notes

  1. src/main/tape/domain/executionContract.ts:980 — the error text names currentTool even when the parent is the one over the ceiling. The two combined conditions share one message, so a parent-level violation reports the wrong tool. Split them or include both names.
  2. src/main/tape/domain/executionContract.ts:986 — only the parent's execution.mode is compared with the frozen ceiling; the child's mode is now unchecked. If mode is purely a scheduling hint this is fine, but it is the one contract field that lost its child-side check. Worth one line in the comment or a test that pins the intended behavior.
  3. src/main/tool/index.ts:637 — input.mode === 'agent' ? [] : ... stores dead state; leaving codeEntryTools undefined for agent/minimal mode is equivalent and less surface.
  4. src/main/tool/index.ts:91 — MainProcessToolCallOptions now carries codeModeNested and codeModeParentTool, and codeModeNested is what bypasses the Code Mode direct-call guard at src/main/tool/index.ts:682. Nothing imports this type today, so there is no practical exposure, but keeping those two fields on the internal runtime-callback type would stop a future caller from accidentally opting into nested dispatch.
  5. test/main/tape/executionContract.test.ts:559 — one it covers about eleven boundaries (membership, target, effect, mode, workspace, depth, identity). A failure will not say which boundary broke; splitting it would make regressions cheaper to read.

Test coverage

The contract test covers the new parent-authority path plus the retained workspace, depth and identity checks. The service test simulates the utility-process host for both frontends and asserts that exec, read and glob reach the agent tool manager and the MCP tool reaches the MCP service, all with ok: true nested results — that is the regression that matters here. Codex wait resume and the live desktop/model path are not covered; the PR body states the latter explicitly, which is fine for this fix.

@zerob13
zerob13 merged commit 830745f into dev Oct 27, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Code Mode 下子会话调用的子工具全部被 frozen View ceiling 拒绝

1 participant