Skip to content

fix(mcp): omit unused optional tool args#3304

Open
roboomp wants to merge 2 commits into
mainfrom
farm/8a720b5a/mcp-tools-omit-unused-optional-args
Open

fix(mcp): omit unused optional tool args#3304
roboomp wants to merge 2 commits into
mainfrom
farm/8a720b5a/mcp-tools-omit-unused-optional-args

Conversation

@roboomp

@roboomp roboomp commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Repro

Constructed an MCPTool with required symbol and optional language/options, executed it with { symbol: "Foo", language: "", options: {} }, and observed the pre-fix tools/call payload forwarding {"symbol":"Foo","language":"","options":{}} (exit 42 from the repro guard).

Cause

packages/coding-agent/src/mcp/tool-bridge.ts normalized model-emitted MCP tool params with normalizeToolArgs() and passed them directly to callTool() in both MCPTool.execute() and DeferredMCPTool.execute(). That preserved empty optional placeholders even when the MCP input schema declared the fields as optional.

Fix

  • Added schema-aware pruning in tool-bridge.ts that omits optional "", {}, and undefined placeholders before tools/call.
  • Preserved required schema fields plus meaningful falsy values such as 0 and false.
  • Added active and deferred MCP regression tests in packages/coding-agent/test/mcp-tool-args.test.ts.
  • Added the coding-agent changelog entry.

Verification

bun test packages/coding-agent/test/mcp-tool-args.test.ts passed (2 tests). bun --cwd=packages/coding-agent run check passed. The original repro command now forwards {"symbol":"Foo"} and exits 0. Fixes #3302

Pruned empty optional MCP argument placeholders before tools/call while preserving required fields and meaningful falsy values.

Added regression coverage for active and deferred MCP tools.

Fixes #3302
@github-actions github-actions Bot added the vouched Passed the vouch gate label Jun 23, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60348404a0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/coding-agent/CHANGELOG.md Outdated

### Fixed

- Fixed MCP tool calls forwarding empty optional placeholder arguments (`""` and `{}`) to `tools/call`; optional placeholders are now omitted while required fields and meaningful falsy values are preserved. ([#3302](https://github.com/can1357/oh-my-pi/issues/3302))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Move changelog entry back to Unreleased

The root AGENTS.md changelog rule says new entries must go under ## [Unreleased] and already-released sections are immutable. This entry was added under ## [16.1.16] - 2026-06-23 instead, so it mutates a released changelog section and will not be picked up as an unreleased fix for the next release; move it to the Unreleased ### Fixed section.

Useful? React with 👍 / 👎.

@roboomp

roboomp commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

Moved the MCP fix changelog entry from the released ## [16.1.16] section into ## [Unreleased]### Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vouched Passed the vouch gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP tools forward unused optional arguments

1 participant