Conversation
Pi passes the run's AbortSignal as the third execute() argument, but the pi MCP bridge dropped it: execute() awaited callTool() with no timeout (tools/call intentionally uses POSITIVE_INFINITY, mksglu#643) and no notifications/cancelled, so an in-flight ctx_execute froze the TUI until the tool returned. Two-part fix: 1. Client (adapters/pi/mcp-bridge.ts): forward the signal through execute() -> callTool() -> request(). On abort the pending request is rejected immediately and a notifications/cancelled notification is sent to the MCP server. 2. Server (server.ts, executor.ts): thread the per-request abort signal from the SDK handler extra into PolyglotExecutor; on abort the spawned sandbox process group is killed (killTree). The existing wrapToolHandler (mksglu#854 in-flight marker) dropped the handler's second argument (extra), so it now forwards it. Verified: standalone harness — callTool rejects ~1ms after abort and server-side sandbox child is killed ~6ms after abort; 162 repo tests pass (pi-mcp-bridge, pi-bridge-parent-death, executor). Tested locally with Pi. Fixes: mksglu#1175 Signed-off-by: rabi <mishra.rabi@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What / Why / How
Pi passes the run's AbortSignal as the third execute() argument, but the pi MCP bridge dropped it: execute() awaited callTool() with no timeout (tools/call intentionally uses POSITIVE_INFINITY, #643) and no notifications/cancelled, so an in-flight ctx_execute froze the TUI until the tool returned.
Two-part fix:
Client (adapters/pi/mcp-bridge.ts): forward the signal through execute() -> callTool() -> request(). On abort the pending request is rejected immediately and a notifications/cancelled notification is sent to the MCP server.
Server (server.ts, executor.ts): thread the per-request abort signal from the SDK handler extra into PolyglotExecutor; on abort the spawned sandbox process group is killed (killTree). The existing wrapToolHandler (Pi host: MCP servers accumulate under a long-lived parent — self-shutdown gated entirely on parent-death (same class as #565, still present in 1.0.163) #854 in-flight marker) dropped the handler's second argument (extra), so it now forwards it.
Affected platforms
Test plan
Verified: standalone harness — callTool rejects ~1ms after abort and server-side sandbox child is killed ~6ms after abort; 162 repo tests pass (pi-mcp-bridge, pi-bridge-parent-death, executor). Tested locally with Pi.
Checklist
npm testpassesnpm run typecheckpassesFixes: #1175