Skip to content

[repo-study-facetime-bridge-spawn-timeout-escalation] Escalate bufferedSpawn timeouts from SIGTERM to SIGKILL so a wedged child can't outlive its verdict #5301

Description

@atomantic

Provenance

Studied repo: kingbootoshi/facetime-bridge (MIT). Studied 2026-08-28. Clean-room: technique described, no code copied.

Rationale

facetime-bridge's bounded child-process runner treats a timeout as terminal: it sends SIGKILL and never lets a timed-out child outlive the caller's verdict. PortOS's shared bufferedSpawn does half of that — on timeoutMs it sends a single SIGTERM via killProcessTree and immediately resolves { timedOut: true }, so a child that traps or ignores SIGTERM (an npm run build stuck in a native compile, a git waiting on a credential helper, a runtime installer mid-download) keeps running after the caller has already reported "timed out" and moved on — still holding the cwd, ports, lockfiles, and CPU. The escalation helper already exists (killWithEscalation, extracted in #1956) but bufferedSpawn doesn't use it.

Fix

  • server/lib/bufferedSpawn.js — in the timeoutMs branch of bufferedSpawn, replace the bare killProcessTree(child) with the SIGTERM → grace → SIGKILL shape: on POSIX call killProcessTree(child, 'SIGTERM'), then arm an unref()'d timer (reuse killWithEscalation from server/lib/killWithEscalation.js with stillRunning: () => child.exitCode === null && child.signalCode === null, or inline the same guard) that sends SIGKILL after a grace window (default 8 s, matching killWithEscalation and detachedSpawn's cancel escalation; expose killGraceMs as an option). Windows already force-kills the whole tree via taskkill /T /F, so the escalation is a no-op there — keep that branch as is.
  • Keep the resolve-immediately-on-timeout contract (callers rely on timedOut: true arriving at the deadline, not 8 s later); the escalation is fire-and-forget cleanup, wrapped in try/catch because it runs outside the request lifecycle.
  • server/lib/bufferedSpawn.test.js — one contract test with fake timers: spawn process.execPath -e "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000)", pass a short timeoutMs, assert the result resolves timedOut: true at the deadline AND that the child's signalCode becomes SIGKILL after the grace window. Add a second assertion that a child which exits on SIGTERM is never sent SIGKILL.
  • Update the bufferedSpawn.js README row in server/lib/README.md to say timeout kills escalate.

Scope

One function branch, one test file, one README row. Callers (appBuilder, appUpdater, providerRuntimeInstaller, llamaServerManager, lmStudioManager, mtplxModelManager, icloudFile, CoS spawners) inherit the fix with no signature change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:devtoolsDevtools/workspace/code-review surfaceseffort:lowDispatch reasoning effort: lowgood first issueSelf-contained and well-specified for a new contributormodel:lightRecommended implementation capability: lightplanTracked by /do:replanrepo-studyProposed from a study of a captured reference repository

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions