Skip to content

fix(pi): propagate Pi's AbortSignal — kill the bridge server tree to stop runaway executors - #1164

Open
cxh0312 wants to merge 1 commit into
mksglu:mainfrom
cxh0312:fix/pi-abort-signal-propagation
Open

cxh0312 wants to merge 1 commit into
mksglu:mainfrom
cxh0312:fix/pi-abort-signal-propagation

Conversation

@cxh0312

@cxh0312 cxh0312 commented Sep 15, 2026

Copy link
Copy Markdown

Pi adapter: propagate Pi's AbortSignal — kill the bridge server tree to stop runaway executors

The bug

Pi passes an AbortSignal as the third argument of a custom tool's execute() (pi extension contract: "the current abort signal, or undefined when the agent is not streaming"). The pi adapter dropped it:

// src/adapters/pi/mcp-bridge.ts (before)
async execute(_toolCallId, params) {
  const result = await client.callTool(tool.name, params ?? {});

So when a model-generated ctx_execute script entered an infinite loop (real incident: a regex without the g flag feeding while(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/call stayed pending forever (by design, #643), and AgentSession.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 (only cleanupBackgrounded() pids), and on Windows child.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 new MCPStdioClient.killTree(), which terminates the MCP server and its whole process tree:
    • Windows: taskkill /T /F walks the tree (server + executors).
    • POSIX: executors are spawned detached (own process group, see executor.ts), so killing the server alone never reaches them — killTree() walks ps children-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.
  • In-flight requests settle immediately via onExit() and the next request() 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-up ctx_* call in the same session self-heals.
  • start()'s exit/error handlers gain an identity guard: a stale exit event from a killed child must not be applied to the current child's state. taskkill/kill are 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's exited flag and rejects its in-flight requests.
  • 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.

Verification

  • 4 new regression tests in 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.
  • Full existing pi-adapter suites pass (67/67 across the other tests/adapters/pi-*.test.ts files).
  • End-to-end against a real Pi session (pi SDK createAgentSession → model calls ctx_execute with 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-up ctx_execute in the same session works through the respawned bridge.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant