Conversation
Pi passes an AbortSignal as the third execute() argument ("the current
abort signal, or undefined when the agent is not streaming"), but the
pi adapter's execute(_toolCallId, params) dropped it, so a stuck
tools/call kept the executor running forever: a model-generated
infinite loop burned a full core for 14+ minutes with no way to stop it
short of killing processes by hand. The server's own graceful shutdown
does not kill RUNNING foreground executors either (only backgrounded
pids), and on Windows a lone child.kill("SIGTERM") is a hard
TerminateProcess that never runs handlers, so the executors orphan.
- execute() now accepts the signal: on abort it calls killTree(), which
terminates the MCP server AND its descendants — taskkill /T /F on
Windows, a children-first ps walk with process-group kills on POSIX
(executors are spawned detached, so killing the server alone never
reaches them).
- In-flight requests settle immediately via onExit() and the next
request respawns the server through the existing mksglu#583 machinery, so
a follow-up ctx_* call in the same session self-heals.
- start() exit/error handlers gain an identity guard: a stale exit
event from a killed child must not flip the respawned child's state
(taskkill/kill are asynchronous reapers; the respawn can already be
live when the old child's exit event reaches the loop).
- An already-aborted signal short-circuits before touching a healthy
server, and the listener is removed on settle so a late abort of a
finished turn never kills the next turn's bridge.
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
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.
Pi adapter: propagate Pi's AbortSignal — kill the bridge server tree to stop runaway executors
The bug
Pi passes an
AbortSignalas the third argument of a custom tool'sexecute()(pi extension contract: "the current abort signal, or undefined when the agent is not streaming"). The pi adapter dropped it:So when a model-generated
ctx_executescript entered an infinite loop (real incident: a regex without thegflag feedingwhile(re.exec(html))), the executor grandchild burned a full core for 14+ minutes with no way to stop it: the user pressed Stop, Pi broadcast the abort signal, the bridge ignored it,tools/callstayed pending forever (by design, #643), andAgentSession.abort()never settled — the UI hung on "Stopping…".shutdown()doesn't help either: it only SIGTERMs the direct child, the server's graceful shutdown does not kill running foreground executors (onlycleanupBackgrounded()pids), and on Windowschild.kill("SIGTERM")is a hard TerminateProcess that never runs signal handlers — so the executors orphan.The fix
execute()now accepts the signal. On abort it calls the newMCPStdioClient.killTree(), which terminates the MCP server and its whole process tree:taskkill /T /Fwalks the tree (server + executors).detached(own process group, seeexecutor.ts), so killing the server alone never reaches them —killTree()walkspschildren-first and kills each descendant's process group (falling back to the direct pid when it is not a group leader), then the server itself.onExit()and the nextrequest()respawns the server through the existing Pi ctx_* tools become stale after MCP idle shutdown (regression in 1.0.132) #583 machinery, so a follow-upctx_*call in the same session self-heals.start()'s exit/error handlers gain an identity guard: a staleexitevent from a killed child must not be applied to the current child's state.taskkill/killare asynchronous reapers — the respawn can already be live when the old child's exit event reaches the event loop; without the guard that stale handler flips the fresh child'sexitedflag and rejects its in-flight requests.Verification
tests/adapters/pi-mcp-bridge.test.ts(34/34 in the file pass): abort mid-call kills the tree including a spawned grandchild standing in for a runaway executor; already-aborted short-circuit; late-abort listener hygiene;killTree()settles in-flight requests and the next call respawns.tests/adapters/pi-*.test.tsfiles).createAgentSession→ model callsctx_executewith a 10-minute busy loop →session.abort()): the abort that previously hung forever now settles in 294 ms, the executor and bridge processes are gone, and a follow-upctx_executein the same session works through the respawned bridge.