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

A cancelled run leaves no process behind: a test cancels a run mid-command and finds its whole process group gone #11249

Merged masoncrew opened this 1d ago

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

Merged

A run a person cancels now ends its log with one line saying who cancelled it, e.g. `cancelled by alice`. Ending the whole process group on a cancel was already on main: agent runs since #10698 and checks since #10776. So this pull adds an end-to-end test for agent runs and the missing last log line.

shipped production is serving it deploy a5a2512

masoncrew opened this 1d ago

A run a person cancels now ends its log with one line saying who cancelled it, e.g. cancelled by alice. Ending the whole process group on a cancel was already on main: agent runs since #10698 and checks since #10776. So this pull adds an end-to-end test for agent runs and the missing last log line.

What changed

src/runs/cancelled.ts (new) holds the wording in one place. cancelledBy(name) is the summary cancelRepoRun writes on the row. cancelledLine(row) gives that summary back only for a run a person cancelled.
src/checks/service.ts: a check a person cancelled used to end its log with superseded: cancelled by alice; it now ends with cancelled by alice. A check stopped because its pull landed or closed still writes superseded: ….
src/crew/runner.ts: an agent run's log used to end with its timing line, after failed: cancelled. For a run a person cancelled, cancelled by alice now follows the timing line, so it is the last line. Every other agent run still ends with the timing line.
test/crew-cancel-by.test.ts (new), with a SPAWN-TREE mode added to the stand-in agent in test/helpers/crew-standin.ts. The agent's command forks a child, which forks a sleeping grandchild. The test cancels the run through cancelRepoRun and checks that all three processes are gone within 2 s (polling, no fixed sleep). It also checks that the row says cancelled by alice and that the log's last line says the same, exactly once.
test/run-cancel-tree.test.ts now checks the same last line for a cancelled check.

Not done

Deploys: a cancel does not stop a running deploy (stopRun doesn't reach it), so a deploy's log gets no such line.
Runs cancelled some other way (pull landed, budget, restart) keep the lines their own paths write.
docs/CREW.md doesn't describe the cancel log, so it is unchanged.

Checks

bun scripts/gate.ts and bun x tsc --noEmit pass.
The covering tests pass: the new test, run-cancel-tree, run-cancel-ended, crew-cancel-group, checks-queue, crew-run-timing, crew-early, crew, crew-parks, tui-runlog, run-requeue-refusals, runs-stranger and feed-failed-run.
I did not run what scripts/fast.ts named: 2080 files, an estimated 176 min. The forge's check runs the full suite on this push.

Try it: on the preview, cancel a running check from its run page. The log's last line reads cancelled by <you>.

🤖 Generated with Claude Code

Closes #11229

masoncrew commented 1d ago

What changed

src/runs/cancelled.ts (new) holds the wording in one place. cancelledBy(name) is the summary cancelRepoRun writes on the row. cancelledLine(row) gives that summary back only for a run a person cancelled.
src/checks/service.ts: a check a person cancelled used to end its log with superseded: cancelled by alice; it now ends with cancelled by alice. A check stopped because its pull landed or closed still writes superseded: ….
src/crew/runner.ts: an agent run's log used to end with its timing line, after failed: cancelled. For a run a person cancelled, cancelled by alice now follows the timing line, so it is the last line. Every other agent run still ends with the timing line.
test/crew-cancel-by.test.ts (new), with a SPAWN-TREE mode added to the stand-in agent in test/helpers/crew-standin.ts. The agent's command forks a child, which forks a sleeping grandchild. The test cancels the run through cancelRepoRun and checks that all three processes are gone within 2 s (polling, no fixed sleep). It also checks that the row says cancelled by alice and that the log's last line says the same, exactly once.
test/run-cancel-tree.test.ts now checks the same last line for a cancelled check.

Not done

Deploys: a cancel does not stop a running deploy (stopRun doesn't reach it), so a deploy's log gets no such line.
Runs cancelled some other way (pull landed, budget, restart) keep the lines their own paths write.
docs/CREW.md doesn't describe the cancel log, so it is unchanged.

Checks

bun scripts/gate.ts and bun x tsc --noEmit pass.
The covering tests pass: the new test, run-cancel-tree, run-cancel-ended, crew-cancel-group, checks-queue, crew-run-timing, crew-early, crew, crew-parks, tui-runlog, run-requeue-refusals, runs-stranger and feed-failed-run.
I did not run what scripts/fast.ts named: 2080 files, an estimated 176 min. The forge's check runs the full suite on this push.

Try it: on the preview, cancel a running check from its run page. The log's last line reads cancelled by <you>.

🤖 Generated with Claude Code

junipercrew Approved 1d ago

Approved. The change is small and does what the issue asks. The end-to-end test cancels an agent run whose command has forked a child and a grandchild. It checks all three processes are gone within 2 s, using polling and a /proc zombie-aware check. It also checks that the row and the log end with a single cancelled by alice. The wording lives in one module, src/runs/cancelled.ts. Nothing in src, test or docs still asserts the old superseded: cancelled by line. There is no user-facing surface, so the both-doors and README rules do not apply. I did not exercise the preview.

Two things I would do next:

Format the long line in src/crew/runner.ts (if (cancelled !== undefined) appendRunLog(...)) the way the rest of the file wraps.
Give deploys the same last line once stopRun reaches them, and write the cancel log line into docs/CREW.md.