fix: forward MCP tool abort signals by joshavant · Pull Request #82443 · openclaw/openclaw

joshavant · 2026-05-16T04:46:45Z

Summary

Problem: the MCP plugin tools stdio server ignored the host-provided request AbortSignal when dispatching tools/call.
Why it matters: cancelled MCP tool calls kept running inside plugin tool.execute(...) instead of observing host cancellation.
What changed: the stdio server forwards MCP handler extra.signal through createPluginToolsMcpHandlers(...).callTool(...) and into tool.execute(...).
What did NOT change (scope boundary): no tool approval policy, hook behavior, or plugin manifest contract changes.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes [Bug]: MCP plugin tools server drops host AbortSignal — in-flight tool.execute cannot be cancelled #82424
Related fix(mcp): forward host cancellation signal to plugin tool.execute #82426
This PR fixes a bug or regression

Real behavior proof (required for external PRs)

Behavior addressed: MCP plugin tool cancellation now reaches the in-flight plugin tool.execute(...) call.
Real environment tested: Crabbox AWS Linux host running OpenClaw from main, using the built MCP plugin tools stdio server over a real StdioClientTransport child process.
Exact steps or command run after this patch: built the repo in Crabbox, spawned dist/mcp/plugin-tools-serve.js with an injected probe tool, called the tool through an MCP client with an aborting request signal, and compared sideband probe output before and after the patch.
Evidence after fix: Crabbox run run_caccc1747346; sideband showed signalDefined: true, signalIsAbortSignal: true, abortObserved: true, signalAbortedAtEnd: true.
Observed result after fix: the client request abort still rejected with the expected MCP abort error, and the plugin tool received and observed the same cancellation signal while executing.
What was not tested: non-stdio MCP transports and every bundled plugin implementation.
Before evidence: Crabbox run run_388253d140c1 on main at 639107b7db559c4d937fdcd05ac7d78a88dc4555; sideband showed signalDefined: false, signalIsAbortSignal: false, abortObserved: false, signalAbortedAtEnd: false.

Root Cause (if applicable)

Root cause: the MCP SDK passes request cancellation through the handler extra.signal, but the stdio server handler only accepted the request object and dropped extra before reaching plugin tool execution.
Missing detection / guardrail: there was no MCP client/server cancellation regression test for plugin tool calls.
Contributing context (if known): AgentTool.execute(...) already accepts an optional AbortSignal, so the fix is to preserve the existing dependency contract across the MCP server boundary.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/mcp/plugin-tools-handlers.cancel.test.ts
Scenario the test should lock in: an MCP client cancels an in-flight tools/call, and the plugin tool receives an AbortSignal that becomes aborted.
Why this is the smallest reliable guardrail: it exercises the MCP SDK client/server request path with in-memory transport while avoiding process and network flake.
Existing test that already covers this (if any): none.
If no new test is added, why not: N/A.

User-visible / Behavior Changes

Cancelled MCP plugin tool calls can now be observed by plugin tools through their existing AbortSignal argument.

Diagram (if applicable)

Before:
MCP client cancel -> SDK handler extra.signal -> dropped -> tool.execute(..., undefined)

After:
MCP client cancel -> SDK handler extra.signal -> handlers.callTool(..., signal) -> tool.execute(..., signal)

Security Impact (required)

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No
If any Yes, explain risk + mitigation: N/A

Repro + Verification

Environment

OS: Crabbox AWS Linux; local macOS for focused tests/lint.
Runtime/container: Node 22.22.2 and pnpm 11.1.0 in Crabbox AWS; local repo Node/pnpm environment for focused checks.
Model/provider: N/A.
Integration/channel (if any): MCP plugin tools stdio server.
Relevant config (redacted): no credentials used.

Steps

Reproduce on main by spawning the built MCP plugin tools stdio server with an injected long-running probe tool.
Call the probe tool through an MCP client and abort the request while it is in flight.
Apply this patch, rebuild, and repeat the same stdio MCP call.
Run focused local regression tests and touched-file format/lint checks.

Expected

The in-flight plugin tool receives an AbortSignal and observes abort when the MCP client cancels the call.

Actual

Before this patch, the plugin tool received no signal.
After this patch, the plugin tool receives and observes the aborted signal.

Evidence

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Focused commands run after rebase:

node scripts/run-vitest.mjs src/mcp/plugin-tools-handlers.cancel.test.ts src/mcp/plugin-tools-serve.test.ts
node_modules/.bin/oxfmt --check --threads=1 src/mcp/tools-stdio-server.ts src/mcp/plugin-tools-handlers.ts src/mcp/plugin-tools-handlers.cancel.test.ts
node scripts/run-oxlint.mjs src/mcp/tools-stdio-server.ts src/mcp/plugin-tools-handlers.ts src/mcp/plugin-tools-handlers.cancel.test.ts
git diff --check HEAD~1..HEAD

All passed.

Human Verification (required)

Verified scenarios: broken main repro in Crabbox AWS, patched Crabbox AWS stdio behavior, focused MCP cancellation regression test, touched-file formatting and lint.
Edge cases checked: unknown tool path remains covered by existing plugin tools server tests; no-signal direct handler path remains covered by existing tests.
What you did not verify: non-stdio MCP transports and full release CI.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No
If yes, exact upgrade steps: N/A

Risks and Mitigations

Risk: a plugin may ignore the signal.
- Mitigation: this preserves the existing optional tool.execute(..., signal) contract; plugins that ignore cancellation keep current behavior.

clawsweeper · 2026-05-16T04:50:50Z

Codex review: needs maintainer review before merge.

Summary
The PR forwards the MCP SDK request AbortSignal through the plugin tools MCP server into AnyAgentTool.execute and adds an MCP cancellation regression test.

Reproducibility: yes. Current main visibly drops the MCP SDK RequestHandlerExtra.signal before tool.execute, and the PR body supplies Crabbox before/after stdio probe output showing the signal is absent before the patch and observed after it.

Real behavior proof
Sufficient (logs): The PR body includes after-fix Crabbox AWS stdio-server proof with sideband output showing the in-flight plugin tool received an AbortSignal and observed cancellation.

Next step before merge
No repair job should be queued because this PR already implements the narrow fix, overlaps #82426, and carries a protected maintainer label requiring a maintainer landing choice.

Security
Cleared: The diff only forwards an existing AbortSignal and adds a focused test; it does not change dependencies, workflows, secrets handling, permissions, downloads, or command execution scope.

Review details

Best possible solution:

Land one narrow MCP cancellation fix with regression coverage, then close the duplicate parallel PR and linked issue through that merge.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main visibly drops the MCP SDK RequestHandlerExtra.signal before tool.execute, and the PR body supplies Crabbox before/after stdio probe output showing the signal is absent before the patch and observed after it.

Is this the best way to solve the issue?

Yes. Forwarding the existing SDK extra.signal into the existing AnyAgentTool.execute signal parameter is the narrowest maintainable fix; the maintainer choice is only which duplicate PR to land.

What I checked:

Current main drops MCP handler extra: On current main, createToolsMcpServer registers the tools/call request handler as a one-argument callback and calls handlers.callTool(request.params), so the SDK handler extra object is not accepted or forwarded. (src/mcp/tools-stdio-server.ts:17, f78434985af9)
Current main calls tool.execute without signal: createPluginToolsMcpHandlers.callTool currently accepts only call params and invokes tool.execute with the call id and arguments, leaving the existing cancellation parameter undefined. (src/mcp/plugin-tools-handlers.ts:45, f78434985af9)
Tool contract already has signal parameter: AnyAgentTool.execute already declares signal?: AbortSignal as the third argument before onUpdate, so this PR wires an existing contract rather than adding a new one. (src/agents/tools/common.ts:21, f78434985af9)
Dependency contract supplies request signal: The pinned @modelcontextprotocol/sdk 1.29.0 types define RequestHandlerExtra.signal: AbortSignal; Server.setRequestHandler handlers receive (request, extra), and the SDK protocol creates an AbortController for inbound requests and passes fullExtra to the handler. (package.json:1766, f78434985af9)
PR forwards the signal through the affected path: The PR changes the tools/call handler to accept (request, extra), passes extra.signal into handlers.callTool, and passes that signal as the third argument to tool.execute. (src/mcp/tools-stdio-server.ts:17, c807771f4c5d)
Regression test exercises SDK cancellation wiring: The added test uses a real MCP SDK Client and Server over InMemoryTransport, observes the signal passed to the probe tool, aborts the client request, and checks that the tool saw the abort. (src/mcp/plugin-tools-handlers.cancel.test.ts:1, c807771f4c5d)

Likely related people:

steipete: GitHub path history shows Peter Steinberger introduced the shared MCP tools stdio server used by both plugin-tools and openclaw-tools. (role: feature-history contributor; confidence: high; commits: 61ab68f5c904, e75cd46ba658; files: src/mcp/tools-stdio-server.ts, src/mcp/plugin-tools-serve.ts, src/mcp/openclaw-tools-serve.ts)
vincentkoc: Local blame and GitHub path history show Vincent Koc recently carried the current MCP handler snapshot and adjacent fixes for result serialization and owner-only filtering around the affected handler path. (role: recent MCP handler contributor; confidence: high; commits: 5f85e6b69213, e4b09e1bf3b2, 5fa0d282a81a; files: src/mcp/plugin-tools-handlers.ts, src/mcp/tools-stdio-server.ts, src/mcp/plugin-tools-serve.test.ts)

Remaining risk / open question:

A parallel open PR covers the same root cause, so maintainers need to choose one branch to land and close the other as superseded.
This read-only review did not rerun the Crabbox stdio probe or focused tests; the behavior assessment relies on source inspection, dependency source/types, and the PR's supplied proof.

Codex review notes: model gpt-5.5, reasoning high; reviewed against f78434985af9.