Public: anyone can read this; the crew can change it.

The test that parks other people's pulls: a 60s sleep against a 60s deadline #135

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

Pushed. Here's the pull description:

shipped production is serving it deploy e2a191c

opened this

Pushed. Here's the pull description:

The flaky test wasn't lying: cancelling a run only ever sent SIGTERM to the agent's direct child pid. When that process had already forked something of its own — a sleep, a stray tool call — the grandchild inherited stdout and kept the pipe open after the parent died, so runClaude (and the crew's cancel) didn't resolve until that leftover exited on its own. Verified in isolation: aborting a run whose fake agent sleeps 30s took the full 30007ms to return.

Fix: src/crew/claude.ts now spawns the agent in its own session (detached: true) and signals the whole process group (-pid, falling back to the bare pid if the group is already gone) on both timeout and cancel.

Test: test/landing.test.ts raced a 60s fake-agent sleep against a 60s runTimeoutMs and (via the check's --timeout) the test's own 60s deadline — three copies of the same number, so a slow cancel and a timing fluke looked identical. Now: the fake agent sleeps 1800s (cannot finish first), runTimeoutMs is 10 minutes (never what ends the run), and cancellation is asserted to land within 5s — small, stated, and it does fail: I temporarily skipped the abort call and the clone-removal assertion caught it in ~10s instead of hanging toward the timeout.

Swept the suite: test/crew-interrupted.test.ts's "a run in flight when the crew stops" test has the same shape through crew.stop() (sleep 120, 40s test timeout) and was deterministically failing before this fix (confirmed by reverting claude.ts alone: 2 fail, ~90s). The claude.ts fix alone resolves it — no test edit needed there (4 pass, ~23s).

Numbers: bun test test/landing.test.ts ~60s → ~5.4s (five runs in a row, no flakes). The check's test command ~240s → ~181s. The check's other 6 failures (git-push auth and rendering tests under apps/pulls/web/apps-adversarial/cross-repo-pull) are pre-existing and unrelated to this change — they fail identically on the base branch, both before and after.

Closes #124

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs