fix: #3618734 Route tools/call through ToolRunner so clients learn which input failed

ToolApi::execute() hands the call to Tool API's ToolRunner::run() (tool#3583078 (closed)) with the bridge invoker and maps the returned ToolInvocation onto the CallToolResult. The bridge no longer drives the instance itself: no setInputValue() loop, no explicit validateInputs(), no access() call, no InputException or handle-exception catch.

What the client sees

  • An input failure reads Invalid input: followed by one name: reason line per violation. The name comes from the violation's property path, which tool#3583085 (closed) sets to the input's machine name, so a value that cannot be stored (a scalar for a map) and a malformed handle name their argument too. No typed-data root, no bare colon, no Input schema text before the schema block.
  • An access denial reads Access denied for tool <id>. <reason> from either stage, the declared permission or the tool's own access(), so the caller cannot tell which denied. The same text tool:run prints.
  • Every message is rendered to plain text before it reaches the client, so quotes arrive as quotes, not entities.
  • The declared permission is checked before any client value is processed: an account without it is denied before input transforms resolve handles or refiners run.
  • The input schema is still echoed for correctable failures and omitted for denials, runtime failures and refusals before the tool existed.

Merged with 1.x

1.x is merged in, with #3618726 (closed) (plain text), #3618729 (closed) (permission before input), #3618720 (closed) (tools/list filtered by the declared permission) and #3618728 (closed) (plugin cache cleared on config change).

  • execute() keeps the runner path; the bridge's own permission pre-check from #3618729 (closed) and its accessDenied() helper go, since the runner checks the declared permission first and words the denial.
  • checkAccess() from #3618720 (closed) stays, so ToolManager is injected beside ToolRunner. The plainText() helper from #3618726 (closed) stays and renders the runner's message, so a plain-string message keeps its literal text.
  • @mglaman's tests are kept with the runner's wording, and QuotedMessageTool keeps his plain-string denial reason. testAccessDenialReasonKeepsLiteralText() expects Access denied for tool mcp_server_test:quoted_message. About <us> &amp; "team" is locked., which needs tool#3583109 (closed).
  • A second commit moves the expectations to the 1.0.0-beta12 shapes: no label prefix in descriptions, entity: ... and fields.0: ... on the path, and the runner's denial text in ToolDiscoveryAccessTest.

Supersedes

!25 (closed) (#3618731 (closed), one violation per line without the root prefix): the runner's name: reason lines give it. Its one-per-line test is carried over in ToolApiFailureResultTest.

Dropped: the client gateway hand-over

The runner creates the instance and processes the inputs in one call, so there is no point at which the bridge can call setClientGateway(), and nothing implements ClientGatewayAwareInterface apart from the fixture. The hand-over, ProgressReportingTool, ToolApiClientGatewayTest and the kenkeep node that described it are removed; what purpose a gateway-aware tool serves, and through which Tool API seam it should get one, is an open comment on this issue.

Dependencies

  • The runner ships in tool 1.0.0-beta12. composer.json and info.yml require beta12, and CI tests against tagged tool releases.
  • The literal denial reason needs tool#3583109 (closed), which shipped in beta12.
  • ToolInvocation::getTool() is used only to echo the input schema. It is the deprecation-window accessor that tool#3583092 retires, so the bridge needs a schema seam that takes the invocation before then.

Destructive default

Since tool#3583086 (closed), a tool that leaves destructive undeclared follows its operation. An undeclared Trigger or Write tool now advertises destructiveHint: true. The bridge code doesn't change; the deriver comment and its kenkeep node now say so.

Knowledge base

The kenkeep nodes for the execution contract and the API surface are rewritten, the gateway node is removed, and the index is rebuilt.

Edited by Matt Glaman

Merge request reports

Loading
Loading