Hard stop cannot interrupt blocking post-session work on native Windows (file channel has no signal-path equivalent)

[pbean]

Split out of the #319 review (PR #697), where it was raised by codex as a P1 against engine.py's raise site B. The premise is correct and the gap is real, but the remedy proposed there is inert, and the remedy that works is a separate mechanism — so it is tracked here rather than bolted onto that PR.

The gap

#319 made a hard stop ride stop-request.json, honored at item boundaries and mid-session via a per-iteration poll in each adapter's wait loop. Between raise site B (the post-_save() check in _run_session) and the next item boundary, the engine performs deterministic post-session work that reads the control file nowhere:

• the post_session plugin hook (plugins/model.py:75 — default timeout_sec = 120)
• artifact processing / _verify_dev_artifacts
• verify.run_verify_commands (engine.py → verify.py:3803)

COMMAND_TIMEOUT_S = 30 * 60 (verify.py:42) — thirty minutes, applied per command, looped over every entry in policy.verify.commands (verify.py:3813), with the suite run once on the dev leg and again on the review leg. So the unpolled window is worst-case N × 30 min, not 30 minutes.

_STOP_WAIT_S = 10.0 (runs.py). Any phase in this window exceeding 10 seconds ends in a force-kill.

Why it is a genuine parity gap, not just latency

The POSIX signal path does strictly better here. The handler raises, it does not set a flag consulted later (engine.py):

if self._stopping:
    return
self._stopping = True
raise RunStopped()

Measured: SIGTERM delivered 1.0s into a blocking subprocess.run shaped like run_verify_commands unwinds immediately, with no orphaned child. So on POSIX a stop during verify lands within a tick; on native Windows, where no inter-process signal is delivered at all, the file channel has unbounded latency in this same window. The file channel is weaker than the signal path precisely where it is meant to replace it.

Not a regression

Pre-#319, WindowsProcessHost.terminate shelled taskkill /PID without /F — a no-op against a console process — so native Windows force-killed after the full grace window in every phase. #319 narrowed that to this one window. This issue is about closing the remainder, not about a behavior that got worse.

The obvious fix does not work

Adding hard-request checks around the post-session phases changes nothing. The engine is inside subprocess.run and executes no bytecode until the child exits; a check placed before or after a blocking call cannot shorten it. Any single phase over 10 seconds force-kills regardless of how many checks bracket it — and the 30-minute command is exactly the case a bracketing check cannot touch. _thread.interrupt_main() is not a shortcut either: on Windows it takes effect only at the next bytecode boundary and will not break the main thread out of the handle wait in Popen.communicate, which is the platform this is about.

What would actually work — and its trap

A watchdog thread plus a child-process registry, so the blocking child itself becomes killable. The mechanism is sound; measured:

[watchdog] awake at t=1.00s, killed child
[main]     unblocked at t=1.00s with rc=-9

But a naive watchdog creates a worse bug than it fixes. The killed command surfaces as CommandResult(command, -9, ...) (verify.py:3835), and verify_command_results_outcome (verify.py:3872) classifies it:

if result.returncode != 0:
    return VerifyOutcome.retry(..., fixable=True)

So the engine would answer an operator's stop request by dispatching a repair session — spawning a fresh LLM session — on the strength of a "failure" it caused itself.

A workable design therefore needs three parts, and the second is the substantial one:

1. a child-process registry + watchdog polling the control file;
2. stop-awareness threaded into result classification — verify.py, plugins/bus.py, and the _run_git chokepoint each need to tell "killed because we are stopping" apart from "this failed";
3. the main thread still has to unwind to a check point inside the grace window.

Note that (2) touches _run_git, which is a hard invariant chokepoint, and that the whole change alters behavior on every platform, not only the one with the gap.

A cheaper variant — a watchdog that performs teardown itself and calls os._exit, sidestepping verify entirely — was considered and rejected: it skips every finally block, leaking worktrees and temp state.

Scope check: what is actually at stake

Worth stating plainly, because it bounds how much this deserves. The current force-kill does not corrupt anything:

• save_state writes a .json.tmp sibling then atomic_replace (journal.py:168), so SIGKILL cannot tear state.json.
• The durability ordering at engine.py ("make the completed session durable before the usage read, post-session hooks, and follow-up verification") was written for this window.
• The run stays resumable and _finish_inflight replays the interrupted phase.

The residue is an orphaned verify child on POSIX (force_kill signals the engine pid only); on Windows taskkill /F /T takes the tree, so there is none.

So the win from closing this is: the engine tears itself down rather than being killed, the "engine is the single writer of stopped" invariant holds without the fallback, no orphaned child on POSIX, and run-stop fallback=True keeps meaning what docs/FEATURES.md now says it means — a genuinely wedged engine. That is an invariant-and-accuracy win, not a data-safety one.

Acceptance

• A hard stop landing while the engine is inside a long verify command is honored by the engine's own teardown on native Windows, within the grace window.
• A command killed by that teardown is not classified as a verify failure and does not trigger a retry, repair session, or escalation.
• Ablation-proven: deleting the stop-awareness in classification reddens a test that asserts no repair session is dispatched.

Related: #319, PR #697.

[pbean]

Second window, same mechanism: run finalization

An automated review of #697 (round 6) surfaced a second region with no hard-stop read. This issue's window is the per-item one — between raise site B and the next item boundary. The new one runs once, after _loop returns:

engine.py:668   if read_stop_request_mode(...) == "hard":  ← last read on the clean-finish path
engine.py:671   self.state.finished = True                 ← in memory only
engine.py:672   self._gc_run_worktrees()                   ← no read
engine.py:673   self._emit("post_run")                     ← no read
engine.py:812   clear_graceful_stop(...)                   ← discards, never honors
engine.py:815   self._save()                               ← first persist of finished=True

Different window, but it blocks on the same two chokepoints this issue already names: plugins/bus.py (which serves post_run as well as post_session) and the _run_git chokepoint (which serves every call gc_run_worktrees makes). Any fix satisfying this issue covers it as a by-product, so recording it here rather than opening a third issue.

Bounds. _gc_run_worktrees issues roughly 4·D + 1 git subprocesses for D done units, each bounded by GIT_TIMEOUT_S = 120 but unbounded in aggregate; the GitError fallback _rmtree_confined (workspace.py:291) has no timeout, and in-process Python hooks (plugins/bus.py:242) have none either. Note COMMAND_TIMEOUT_S = 1800 is not in this window — that one is per-item, which is a useful discriminator between the two.

"The obvious fix does not work" applies here verbatim. Against _STOP_WAIT_S = 10.0, the best worst-case unpolled interval any placement of checks can achieve equals the longest single atomic blocking call — 120s by default, unbounded for rmtree and Python hooks. Polling between calls drops the worst case from ~4,900s to 120s and still force-kills.

Worse, here it would buy the wrong action even when it fires. At engine.py:668 the work queue is already exhausted, so aborting _gc_run_worktrees/post_run leaks worktrees and branches, skips plugin teardown, can tear discard_worktree's four-call sequence mid-way on a run that must stay resumable, and hands post_run to a prefix of plugins that all got pre_run — a third state where tests/test_sweep.py:4411 pins two.

Not a regression in #697. End state in this window is identical to origin/main across all four sub-cases (POSIX signal, Windows no-signal, real force-kill, unsignalable engine); main had no run-end check at all and no file channel in this region on native Windows, so #697 strictly narrows the window rather than opening one.

Residual worth doing here: a one-shot late consume

Separate from the watchdog design, and much cheaper. After _emit("post_run") and before journal.append("run-complete"), re-read the control file once. If a hard request is now pending, consume it and record it as received-late instead of letting the finally at :812 discard it as stop-request-discarded.

• Costs: one small read at a point already doing I/O. No abort, no thread, no bracketing of blocking calls.
• Buys: the operator's request is journaled accurately, and stop_run's if load_state(run_dir).stopped: early-out (runs.py:1341) then suppresses the spurious run-stop fallback=True — the accuracy invariant this family exists to protect.
• Does not buy: any deadline win (a long finalization still force-kills), any interruption of GC or post_run, and no status change — documents.py:275 still reports finished, which is truthful once the queue is exhausted.

Related: while grading this, one adjacent pre-existing defect turned up that is not a polling problem and may deserve its own issue — on the path where stop_run marks stopped while the engine is still alive, the engine's later _save() at engine.py:815 rewrites its whole in-memory state and clobbers stopped=True back to False. Present identically on main.