Skip to content

fix(server): MCP list tools clamp an oversized limit instead of failing - #30976

Open
MarcoDotIO wants to merge 12 commits into
pingdotgg:mainfrom
MarcoDotIO:fix/mcp-limits
Open

MarcoDotIO wants to merge 12 commits into
pingdotgg:mainfrom
MarcoDotIO:fix/mcp-limits

Conversation

@MarcoDotIO

@MarcoDotIO MarcoDotIO commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Three MCP list tools reject a limit above a maximum the model never sees: t3_project_list (100), t3_queue with action: "list" (100), and t3_preview_list (50). The Codex harness does not show the model a tool parameter's maximum, and these fields have no description. An agent cannot know about the bound until the call fails with -32602: Invalid parameters for tool '<tool>': Expected a value between 1 and 100 (or 50) and it has to retry. #22901 fixed this for t3_thread_read's page sizes, and #27814 does the same for t3_thread_list.

Call path: an agent calls, for example, t3_queue with { action: "list", limit: 200 }. McpServer decodes the arguments against the tool's parameter schema (apps/server/src/mcp/toolkits/{project,thread,previewControls}/tools.ts). The Schema.Int.check(isBetween({ minimum: 1, maximum: N })) refinement fails, so the handler never runs.

Change

  • Each of the three limit parameters uses clampedPositiveInt(N), the helper t3_thread_read already uses: clampedPositiveInt(100) for t3_project_list and t3_queue, clampedPositiveInt(50) for t3_preview_list. A value above the maximum decodes to the maximum, and each tool's existing nextCursor pages through the rest. 0 and negative values still fail.
  • clampedPositiveInt in packages/contracts/src/orchestratorMcp.ts is now exported so the server toolkits can use it. Its body is unchanged.

The in-tree callers of t3_project_list, t3 thread in the CLI (listAllProjects) and remote delegation's project lookup, already pass limit: 100, so they behave the same.

Deliberately out of scope: t3_worktree_list and t3_thread_search have the same pattern, but their limit schemas are shared with RPCs (VcsListRefsInput, OrchestrationSearchThreadsInput). Clamping there would change RPC behaviour too. t3_thread_list is covered by #27814.

Scope and approval

This is a small, focused fix for one underlying problem: MCP-only list tools fail on a bound the model cannot see. It applies the clamp that #22901 merged for t3_thread_read to the three remaining MCP-only tools with that bound, one parameter each. Behavior and result shapes stay the same for every value that worked before.

Verification

Added two table tests to apps/server/src/mcp/McpHttpServer.test.ts, each run for all three tools through the real MCP server decode path:

  • clamps a $tool limit above its maximum instead of rejecting the call: twice the maximum reaches the handler as the maximum, and limit: 7 stays 7.
  • still rejects a $tool limit below its minimum: limit: 0 still fails, and the handler is not called.

With the source files reverted to upstream/main:

× clamps a t3_preview_list limit above its maximum instead of rejecting the call
× clamps a t3_project_list limit above its maximum instead of rejecting the call
× clamps a t3_queue limit above its maximum instead of rejecting the call
effect/ai/McpSchema/InvalidParams: Invalid parameters for tool 't3_preview_list': Expected a value between 1 and 50
effect/ai/McpSchema/InvalidParams: Invalid parameters for tool 't3_project_list': Expected a value between 1 and 100
effect/ai/McpSchema/InvalidParams: Invalid parameters for tool 't3_queue': Expected a value between 1 and 100

(The below-minimum cases also fail there, only because main's error wording differs; main does reject 0.)

With the fix:

vp test run src/mcp/McpHttpServer.test.ts src/provider/AgentContextBudget.test.ts
Test Files  2 passed (2)
Tests  37 passed (37)

tsc --noEmit passes in packages/contracts and apps/server. Lint and format pass on packages/contracts/src and apps/server/src/mcp, and knip passes in the commit hook.

The clamped schemas no longer list maximum, so the MCP tool list shrinks: agentContextBudget.json records mcpToolListJsonBytes 49806 → 49765 (against current main) (AgentContextBudget.test.ts passes). I did not add limit descriptions, so agent context does not grow.

Not checked: a live Codex session calling these tools.

Proved with table tests through the real MCP decode path that fail on main and pass with the fix.

Created with Claude Opus 5.5 in the Claude Code desktop app, and reviewed by GPT 6.1 Sol in Codex CLI.

🤖 Generated with Claude Code

MarcoDotIO and others added 2 commits November 4, 2026 11:24
…ling

Codex hides a tool parameter's `maximum` from the model, so an agent that
asks t3_project_list for `limit: 200` got InvalidParams and had to retry.
The limit now clamps to 100 on decode, as t3_thread_read does, and
`nextCursor` pages through the rest. Values below 1 still fail.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
t3_queue (list) and t3_preview_list had the same hidden bound as
t3_project_list: Codex never shows the model a parameter's `maximum`,
so `limit` above 100 (queue) or 50 (preview tabs) failed the call with
InvalidParams. Both now clamp on decode with clampedPositiveInt, and
`nextCursor` pages through the rest. Values below 1 still fail.

t3_worktree_list and t3_thread_search are left alone: their limit
schemas are shared with RPCs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched Author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 4, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 75201ed

Macroscope's review found this PR approvable — Oversized MCP list limits now clamp to existing page-size caps instead of failing validation. Default page sizes, access controls, and handlers remain unchanged, keeping the change narrowly bounded.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The project-list, queue, and preview-list tools now clamp limits above their configured maxima and reject limits below one. The shared clampedPositiveInt helper is exported, and tests cover these limit behaviors.

Changes: MCP list limit validation

Layer / File(s) Summary
Export and apply list-limit validation
packages/contracts/src/orchestratorMcp.ts, apps/server/src/mcp/toolkits/project/tools.ts, apps/server/src/mcp/toolkits/thread/tools.ts, apps/server/src/mcp/toolkits/previewControls/tools.ts, apps/server/src/mcp/McpHttpServer.test.ts, apps/server/src/provider/agentContextBudget.json
clampedPositiveInt is exported and applied to the three list-tool limits. Tests verify that limits above 100 or 50 are clamped, a limit of 7 is preserved, and zero is rejected before a recorded call. The mcpToolListJsonBytes value changes from 48217 to 48176.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

  • pingdotgg/t3code#27814: Applies clampedPositiveInt(100) to t3_thread_list; this PR applies the shared helper to three other list tools.

Suggested labels: bugfix

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: Minimal · up to 3811c

Oversized list limits can proceed at each tool’s maximum, while invalid nonpositive limits remain rejected. No actionable merge-blocking risk is identified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly summarizes the main change: MCP list tools now clamp oversized limits instead of rejecting them.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description follows the repository template closely. It has Problem, Change, Scope, and Verification sections, plus the required agent attribution. The Problem section gives the failing call path …
Full details: Description check

Explanation

The description follows the repository template closely. It has Problem, Change, Scope, and Verification sections, plus the required agent attribution. The Problem section gives the failing call path and the exact error text. The Change section names each edited parameter and the exported helper. The Scope section links related PRs (#22901, #27814) and explains why worktree and thread-search limits are excluded. The Verification section gives focused test names, before/after failure output, and a passing test run. It also states what was not checked: a live Codex session. The template asks for a conventional commit title. The PR title in the objectives is "fix(server): MCP list tools clamp an oversized limit instead of failing," which meets that rule. The description does not link a tracked issue, but it does explain the scope and points to prior fixes. The description also has a minor inconsistency: it says agentContextBudget.json moved from 49806 to 49765, while the raw summary and PR comments show 48217 to 48176. That discrepancy should be fixed, but it does not make the description incomplete.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 0eb3bb7 and 3811c50.

Files selected for processing: 3

  • apps/server/src/mcp/McpHttpServer.test.ts
  • apps/server/src/provider/agentContextBudget.json
  • packages/contracts/src/orchestratorMcp.ts

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

@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Merged current main (merge commit, no rebase) to clear the conflict in apps/server/src/provider/agentContextBudget.json. Took main's values and recomputed mcpToolListJsonBytes: 48217 → 48176, the same 41-byte drop from the clamped limit schemas. No other changes. The PR body's byte counts now match.

vp run check:changed passes (format, lint, knip, typecheck of 37 packages, McpHttpServer.test.ts, AgentContextBudget.test.ts, orchestratorMcp.test.ts).

MarcoDotIO and others added 3 commits November 13, 2026 16:27
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

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

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched Author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants