fix(ai): warn first when an MCP call's argument was ignored by Makisuo · Pull Request #1386 · MapleTechLabs/maple

Makisuo · 2026-10-10T23:17:04Z

Problem

An agent called list_services with {"time_range":"today"}. The tool takes start_time/end_time (default 6 hours), so the key was dropped and the default window ran. The registry did report it, but the report was easy to miss and half-wrong:

## Services

Time range: 2026-10-10 17:15:10 to 2026-10-10 23:15:10 · Now (UTC): ...

Note: `time_range` is not a parameter of `list_services` and was ignored. Did you mean `start_time`?

It came after the scope line, as a Note: alongside informational notes (clamped windows).
It never said the result was computed without the key, so the agent read the default window as its answer.
time_range names a whole window, but the edit-distance match suggested only start_time.

Change

ToolDoc gains warnings, rendered directly under the title as Warning: lines. The registry puts ignored-argument reports there for every tool; notices stays for informational notes.
The warning says the result is for the call without the key.
A key naming a whole window (time_range, lookback, period, since, ...) on a tool that takes both bounds suggests start_time/end_time. A single-bound key (start) still suggests the one bound. The decode-failure message uses the same suggestions.

Now:

## Services

Warning: `time_range` is not a parameter of `list_services`. It was ignored, and this result is for the call without it. Did you mean `start_time`/`end_time`?

Time range: ...

Not done: a `time_range` alias on `P.timeWindow`

Aliases rename a key and keep the value, so time_range: "today" would land in start_time and fail to decode as a timestamp. time_range was never a parameter name for these tools, so it is not a retired name. It already means something else on create_dashboard (the dashboard's saved relative range). Accepting relative windows would be a new parameter, not an alias, so it does not fit "one spelling per concept".

Tests

decode-issues.test.ts: window keys suggest both bounds, single bounds stay single, no window suggestion on tools without one, warning text.
tool-doc.test.ts: warnings render before scope, notices after.
tool-contract.test.ts: list_services with time_range succeeds and the warning is the first thing under the title.

Ran those three plus registry.contract.test.ts and @maple/ai typecheck. The full suite and repo-wide typecheck were not run.

🤖 Generated with Claude Code

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

An unknown key was reported as a Note below the scope line, suggested only start_time for time_range, and never said the result ignored it. Ignored keys are now a Warning directly under the title that says the result is for the call without the key, and a window key points at both start_time/end_time.

maple-review-bot · 2026-10-10T23:17:16Z

Maple review

🔴 Confidence 4/10 · risky as written
Held at 4 because a critical finding is open.
_{quality 75/100 · 1 critical · tests covered · risk medium}

Moves ignored-argument reports from notices to a new ToolDoc.warnings rendered under the title, and makes a window-named unknown key suggest both start_time and end_time. The rendering change itself is sound, but the rename left a caller broken. The registry-stack error issues listed in the kickoff are tool failures unrelated to this rendering path, and the diff does not address them.

ToolDoc.warnings renders Warning: lines under the title, before scope
argumentWarnings replaces argumentNotices and says the result is for the call without the key
Window-named unknown keys suggest both start_time and end_time

Findings

🔴 Critical · F1 · argumentNotices rename leaves pr-review-local.ts importing a missing export

correctness · apps/ai/src/mcp/lib/decode-issues.ts:171-175

argumentNotices is gone from this module, but apps/ai/scripts/pr-review-local.ts:61 still does import { normalizeArguments, argumentNotices } from "@/mcp/lib/decode-issues" and calls it at line 769, so the local PR-review runner fails at import. apps/ai/src/chat/pr-review-replay.test.ts imports makeExecutor from that script, so bun run test in @maple/ai (vitest include: ["src/**/*.test.ts"]) fails too. This slipped CI because apps/ai/tsconfig.json includes only src/** and excludes *.test.ts.

Update both references to `argumentWarnings` in `apps/ai/scripts/pr-review-local.ts` (the import on line 61 and the call on line 769).

🤖 Prompt to fix this finding with an AI agent

Findings from an automated review of commit 162a1d90bf29a5e618b69d7bc3c7568ef4b139a6. Verify each one against the current code before changing anything, fix only those that still apply, and keep each fix to the lines it names.

---

F1 · Critical · correctness · apps/ai/src/mcp/lib/decode-issues.ts:171-175
`argumentNotices` rename leaves `pr-review-local.ts` importing a missing export
`argumentNotices` is gone from this module, but `apps/ai/scripts/pr-review-local.ts:61` still does `import { normalizeArguments, argumentNotices } from "@/mcp/lib/decode-issues"` and calls it at line 769, so the local PR-review runner fails at import. `apps/ai/src/chat/pr-review-replay.test.ts` imports `makeExecutor` from that script, so `bun run test` in `@maple/ai` (vitest `include: ["src/**/*.test.ts"]`) fails too. This slipped CI because `apps/ai/tsconfig.json` includes only `src/**` and excludes `*.test.ts`.
Suggested fix: Update both references to `argumentWarnings` in `apps/ai/scripts/pr-review-local.ts` (the import on line 61 and the call on line 769).

Production impact

Open errors in the changed files

Issue	Service	Occurrences	File
@maple/api/vcs/VcsSourceRepositoryNotFoundError	maple-ai	68	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	42	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	36	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	18	`apps/ai/src/mcp/tools/registry.ts`
@maple/api/vcs/GithubAppError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	14	`apps/ai/src/mcp/tools/registry.ts`
@maple/http/errors/IntegrationsUpstreamError	maple-chat	11	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-ai	9	`apps/ai/src/mcp/tools/registry.ts`
@maple/mcp/errors/McpQueryError	maple-chat	8	`apps/ai/src/mcp/tools/registry.ts`

After this merges, Maple checks whether they stop.

What was checked

Window suggestion is gated on the tool actually declaring both bounds (decode-issues.ts:101)
Single-bound keys still fall back to suggestParameter (decode-issues.ts:104)
No other consumer of NormalizedArguments.unknown reads the old suggestion field

_{162a1d9 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.}

coderabbitai · 2026-10-10T23:17:17Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: adfbf720-b3a6-4734-94c5-635e52adebab

📥 Commits

Reviewing files that changed from the base of the PR and between cce3e33 and 162a1d9.

📒 Files selected for processing (7)

apps/ai/src/mcp/__evals__/tool-contract.test.ts
apps/ai/src/mcp/lib/decode-issues.test.ts
apps/ai/src/mcp/lib/decode-issues.ts
apps/ai/src/mcp/lib/tool-doc.test.ts
apps/ai/src/mcp/lib/tool-doc.ts
apps/ai/src/mcp/tools/registry.ts
apps/ai/src/mcp/tools/tool-output.ts

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.}

maple-review-bot

1 inline note from Maple's review. The score and summary are in the review comment above.

maple-review-bot · 2026-10-10T23:19:13Z

+ * Warnings for the result text, one per ignored key. The call still ran, so each says the result
+ * is for the call without that key: a model that sent `time_range` otherwise reads a default window.
+ */
+export const argumentWarnings = (normalized: NormalizedArguments, tool: string): ReadonlyArray<string> =>


Caution
argumentNotices rename leaves pr-review-local.ts importing a missing export

_{F1 · Critical · correctness}

argumentNotices is gone from this module, but apps/ai/scripts/pr-review-local.ts:61 still does import { normalizeArguments, argumentNotices } from "@/mcp/lib/decode-issues" and calls it at line 769, so the local PR-review runner fails at import. apps/ai/src/chat/pr-review-replay.test.ts imports makeExecutor from that script, so bun run test in @maple/ai (vitest include: ["src/**/*.test.ts"]) fails too. This slipped CI because apps/ai/tsconfig.json includes only src/** and excludes *.test.ts.

Update both references to `argumentWarnings` in `apps/ai/scripts/pr-review-local.ts` (the import on line 61 and the call on line 769).

🤖 Prompt to fix with an AI agent

In `apps/ai/src/mcp/lib/decode-issues.ts:171-175`: `argumentNotices` rename leaves `pr-review-local.ts` importing a missing export. `argumentNotices` is gone from this module, but `apps/ai/scripts/pr-review-local.ts:61` still does `import { normalizeArguments, argumentNotices } from "@/mcp/lib/decode-issues"` and calls it at line 769, so the local PR-review runner fails at import. `apps/ai/src/chat/pr-review-replay.test.ts` imports `makeExecutor` from that script, so `bun run test` in `@maple/ai` (vitest `include: ["src/**/*.test.ts"]`) fails too. This slipped CI because `apps/ai/tsconfig.json` includes only `src/**` and excludes `*.test.ts`. Suggested fix: Update both references to `argumentWarnings` in `apps/ai/scripts/pr-review-local.ts` (the import on line 61 and the call on line 769). Verify the problem exists at that location before changing it, and keep the fix to those lines.

maple-review-bot Bot reviewed Oct 10, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(ai): warn first when an MCP call's argument was ignored - #1386

fix(ai): warn first when an MCP call's argument was ignored#1386
Makisuo wants to merge 1 commit into
mainfrom
fix/mcp-unknown-param-notice

Makisuo commented Oct 10, 2026 •

edited by blacksmith-sh Bot

Loading

Uh oh!

maple-review-bot Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 10, 2026

Review limit reached

Uh oh!

maple-review-bot Bot left a comment

Uh oh!

maple-review-bot Bot Oct 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Makisuo commented Oct 10, 2026 • edited by blacksmith-sh Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Not done: a time_range alias on P.timeWindow

Tests

Uh oh!

maple-review-bot Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Maple review

Findings

Production impact

Uh oh!

coderabbitai Bot commented Oct 10, 2026

Review limit reached

Uh oh!

maple-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

maple-review-bot Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Makisuo commented Oct 10, 2026 •

edited by blacksmith-sh Bot

Loading

Not done: a `time_range` alias on `P.timeWindow`

maple-review-bot Bot commented Oct 10, 2026 •

edited

Loading