fix: list only the called action's arguments on an unknown argument #859

Merged

hexajon merged 2 commits from issue-756-scoped-valid-args into main 2026-10-21 03:25:34 +00:00

Conversation 0 Commits 2 Files changed 3 +211 -1

hexajon commented 2026-10-21 03:25:31 +00:00

Owner

Copy link

Summary

What was wrong

When a call to an action-dispatched tool carried an unknown argument, the rejection listed every argument the tool accepts across all its actions. For madt_repos that is about seventy names. For example, madt_repos action=search query=madtea rejected query and then listed the arguments of create, merge, mirror, and the other actions. Most of those names do not apply to search.

What changed

• internal/mcp/registry.go: new method Registry.actionScopedFieldsFor. It reads action (and op) from the raw call arguments. It returns the sorted arguments that this action accepts, plus those of the selected op. It uses opspec.AcceptedParamSet and keeps only names that the tool's input struct records. It lists op only when the action is op-dispatched. It looks in both the consolidated tool slot and the addTool slot, so tools such as madt_worktrees are covered. It reports ok=false when the call has no action, the tool has no action manifest, or the action is not one the tool has.
• internal/mcp/register.go: installValidationEnrichment now passes the scoped list to formatUnknownArgs. When actionScopedFieldsFor reports ok=false, it falls back to reg.knownFieldsFor, the full field set.
• The did-you-mean matcher still reads the full field set, so no suggestion changes. This change narrows only the listed fallback.

Tests

All tests are in internal/mcp/arg_scope_test.go and send real calls over an in-memory MCP transport.

• TestUnknownArgValidList_ScopedToCalledAction: madt_repos action=search with query lists exactly the 18 arguments that search accepts.
• TestUnknownArgValidList_ScopedToSelectedOp: madt_issues action=body op=append lists op and content, and omits replacements, title, and labels.
• TestUnknownArgValidList_ScopedOnAddToolManifest: madt_worktrees action=rebase lists exactly action, dir, onto, path, raw_json.
• TestUnknownArgValidList_FullSetWithoutKnownAction: a call with no action, and a call with an unknown action, both list the full set.
• TestUnknownArgValidList_ToolWithoutActionsKeepsFullSet: madt_commit still lists its full set.
• TestUnknownArgValidList_SuggestionUnchanged: a typo of an argument that another action takes (default_brnch) still suggests default_branch, while the search list does not carry it.

Refs #756

./scripts/gate.sh passes on this commit.

Closes #756

## Summary ## What was wrong When a call to an action-dispatched tool carried an unknown argument, the rejection listed every argument the tool accepts across all its actions. For `madt_repos` that is about seventy names. For example, `madt_repos action=search query=madtea` rejected `query` and then listed the arguments of create, merge, mirror, and the other actions. Most of those names do not apply to `search`. ## What changed - `internal/mcp/registry.go`: new method `Registry.actionScopedFieldsFor`. It reads `action` (and `op`) from the raw call arguments. It returns the sorted arguments that this action accepts, plus those of the selected op. It uses `opspec.AcceptedParamSet` and keeps only names that the tool's input struct records. It lists `op` only when the action is op-dispatched. It looks in both the consolidated tool slot and the `addTool` slot, so tools such as `madt_worktrees` are covered. It reports `ok=false` when the call has no action, the tool has no action manifest, or the action is not one the tool has. - `internal/mcp/register.go`: `installValidationEnrichment` now passes the scoped list to `formatUnknownArgs`. When `actionScopedFieldsFor` reports `ok=false`, it falls back to `reg.knownFieldsFor`, the full field set. - The did-you-mean matcher still reads the full field set, so no suggestion changes. This change narrows only the listed fallback. ## Tests All tests are in `internal/mcp/arg_scope_test.go` and send real calls over an in-memory MCP transport. - `TestUnknownArgValidList_ScopedToCalledAction`: `madt_repos action=search` with `query` lists exactly the 18 arguments that `search` accepts. - `TestUnknownArgValidList_ScopedToSelectedOp`: `madt_issues action=body op=append` lists `op` and `content`, and omits `replacements`, `title`, and `labels`. - `TestUnknownArgValidList_ScopedOnAddToolManifest`: `madt_worktrees action=rebase` lists exactly `action`, `dir`, `onto`, `path`, `raw_json`. - `TestUnknownArgValidList_FullSetWithoutKnownAction`: a call with no action, and a call with an unknown action, both list the full set. - `TestUnknownArgValidList_ToolWithoutActionsKeepsFullSet`: `madt_commit` still lists its full set. - `TestUnknownArgValidList_SuggestionUnchanged`: a typo of an argument that another action takes (`default_brnch`) still suggests `default_branch`, while the `search` list does not carry it. Refs #756 `./scripts/gate.sh` passes on this commit. Closes #756

hexajon added 2 commits 2026-10-21 03:25:31 +00:00

fix: list only the called action's arguments on an unknown argument ... 069d6b55d3

When a call to an action-dispatched tool names a known action, the
unknown-argument rejection now lists the arguments that action accepts
(and the selected op's, for an op-dispatched action), not the tool's
whole field set. A call with no action, or an action the tool does not
have, still lists the full set. The did-you-mean matcher still reads the
full field set, so no suggestion changes.

Refs #756

test: use strings.Cut in the valid-argument parser, as go fix asks 29ce6bf8b8

hexajon merged commit 29ce6bf8b8 into main 2026-10-21 03:25:34 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

breaking

Breaking change - flag for release notes

  

bug

Something is not working

  

documentation

Documentation only

  

enhancement

New feature

  

epic

Tracking epic with dependency children

  

good first issue

Interested in contributing? Get started here.

  

help wanted

Maintainer would welcome outside help

  

refactoring

Code restructuring without behavior changes

  

resolution/duplicate

Closed as duplicate of another issue

  

resolution/invalid

Closed as invalid or not reproducible

  

resolution/wontfix

Closed as won't fix

  

security

Security issue or hardening

  

severity/critical

Security severity: critical

  

severity/high

Security severity: high

  

severity/low

Security severity: low

  

severity/medium

Security severity: medium

  

status/abandoned

Someone started, then walked away - up for grabs

  

status/blocked

Blocked by another issue or an external dependency

  

status/needs-decision

Waiting on a maintainer decision; workable once decided

  

status/needs-info

Waiting on reporter / more information before work can start

  

status/needs-verification

Done except verification in a live interactive session

  

testing

Test suite / CI infrastructure work

  

upstream

Related to an upstream repository, already reported there

No labels

breaking

bug

documentation

enhancement

epic

good first issue

help wanted

refactoring

resolution/duplicate

resolution/invalid

resolution/wontfix

security

severity/critical

severity/high

severity/low

severity/medium

status/abandoned

status/blocked

status/needs-decision

status/needs-info

status/needs-verification

testing

upstream

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea!859

No description provided.