Skip to content

porch check timeout is defeatable: 'close'-based resolution + shell-only kill let a wedged child hold a phase forever #1706

Description

@amrmelsayed

Field incident

Two specimens in one morning, same lane, on an adopter workspace: porch done build checks hung on a stale turbo daemon in a builder worktree. One wedged 80+ minutes on a turbo build child that had consumed 0.36s of CPU (blocked on the daemon socket, not working); the second cleared after turbo daemon stop in the worktree, after which the same build completed in 347ms. Remedy that works in the field: turbo daemon stop in the worktree. Worktree-heavy setups appear to stale the daemon. Full transcripts available from the reporting workspace on request.

Root cause (verified against source)

packages/codev/src/commands/porch/checks.ts already has a check timeout (DEFAULT_TIMEOUT_MS = 5 min, SIGTERM then SIGKILL escalation), but it is defeated twice:

  1. The promise resolves only on proc.on('close') (line 82). The check is spawned with shell: true, so proc is the shell and the real build (turbo and its children) inherits the stdout/stderr pipes. When the timeout fires, proc.kill('SIGTERM') kills the shell only; the wedged grandchild keeps the inherited pipes open, 'close' never fires, the promise never resolves, and porch done hangs indefinitely — the timeout ran and changed nothing observable.

  2. The SIGKILL escalation is a no-op (lines 67-71): the guard is if (!proc.killed), but Node sets proc.killed = true as soon as a signal has been sent (the SIGTERM at line 66), not when the process exits. The escalation branch is unreachable. Even if it fired, it too targets the shell, not the process tree.

Fix shape (hypothesis, for the builder to validate)

Generic process-tree hygiene, no tool-specific knowledge:

  • Kill the process group, not the shell: spawn with detached: true and on timeout signal the group (process.kill(-proc.pid, 'SIGTERM'), escalating to SIGKILL on the group), so children spawned by the shell die with it.
  • Resolve on 'exit' with a bounded output drain, or equivalently add a hard deadline that resolves the promise even if inherited pipes never close. Nothing porch shells out to should be able to hold a phase forever — the timeout must bound wall-clock, not "wall-clock provided every descendant is polite".
  • Fix the escalation guard: track kill state explicitly (e.g. resolve/escalate on a timer keyed to the 'exit' event) instead of consulting proc.killed.
  • Regression test: a check command whose child ignores SIGTERM and holds stdout open must produce a timed-out CheckResult within the budget (plus escalation), not a hang.

Deliberately out of scope

Turbo-specific mitigation (TURBO_DAEMON=false, pre-stopping the daemon) is workspace configuration — projects can set env in their own check commands. Porch stays tool-neutral; the group-kill + bounded-resolution fix covers any misbehaving child, turbo included. The issue exists because porch's own timeout contract was violable, not because of what turbo did.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/porchArea: Porch state machine / protocol orchestration

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions