fix(process): identity-aware liveness and win32 console-ctrl safety

[dracic]

What

Make engine process-liveness identity-aware and harden Windows signal handling. Liveness now answers "is the specific engine I launched still alive?" — comparing a persisted process identity, not just a PID number — and on Windows the engine ignores stray console Ctrl+C during a run so it can't be killed mid-flight.

Why

Liveness was computed as bare PID existence (does this pid number exist?). The OS recycles a dead process's PID — immediately on Windows, once the last handle closes — so a stranger inheriting the number read as a false "alive". That single defect cascaded:

• resume refused to resume a genuinely-dead run,
• worktree reclaim was stranded,
• agent sessions leaked (never pruned),
• the TUI showed a dead run as RUNNING (crash hidden).

Separately, on native Windows a terminal/ConPTY Ctrl+C is a console broadcast (GenerateConsoleCtrlEvent) that can deliver CTRL_C_EVENT to the engine sharing that console and kill it mid-run.

How

• Persist identity, then compare it. write_pid now records the process identity (POSIX /proc start-time / Windows psutil create_time) as a second token next to the pid; read_pid_identity reads both. A new ProcessHost.alive_and_ours(pid, identity) is a distinct identity-aware op — is_alive stays the raw existence primitive — and is routed through engine_alive, the TUI liveness column, and stop_run's pre-terminate check, so a reused PID reads as dead-and-gone.
• Fail closed, degrade safely. A malformed/non-finite identity token fails closed (reads as not-ours); a genuine legacy pid-only file degrades to bare existence (today's behaviour, no crash). The forced-path stop keeps a stop-time identity fallback for legacy files so a wedged pre-upgrade run can still be force-killed — and it never force-kills on a None/mismatched identity.
• Windows signal safety. During a run the stop handler ignores console SIGINT/SIGBREAK on win32 (journaling the event) so a broadcast Ctrl+C can't kill the engine; SIGTERM and non-win32 SIGINT still stop cleanly. The deliberate stop path on Windows is taskkill via the process-host seam, not Ctrl+C.

Process management: win32 vs POSIX

The change exists because process lifecycle differs in kind, not just in API, between the two platforms. Everything sits behind a ProcessHost seam (PosixProcessHost / WindowsProcessHost, selected by a platform registry) so the engine, runs, and the TUI never branch on sys.platform at the call site.

Concern	POSIX (Linux/macOS/WSL)	Native Windows (win32)
Liveness probe	os.kill(pid, 0) — a harmless, read-only existence check	os.kill(pid, 0) is destructive: Python maps signal 0 to CTRL_C_EVENT and sends a real Ctrl+C to the target's console. Must use psutil (pid_exists / Process.is_running) instead
PID reuse	Kernel cycles the whole PID space (default pid_max 32768) before wrapping → reuse is delayed and rare	PID freed for immediate reuse once the last handle to the dead process closes → reuse is routine
Process identity	/proc/<pid>/stat field 22 (start-time in clock ticks)	psutil Process.create_time()
Polite stop	SIGTERM (catchable, the engine unwinds and marks itself stopped)	taskkill /PID (posts WM_CLOSE); SIGTERM generally does not run a Python handler — the process is ended via TerminateProcess
Forced stop	SIGKILL	taskkill /F /T (kills the process tree)
Ctrl+C delivery	SIGINT to the foreground process group	GenerateConsoleCtrlEvent broadcasts CTRL_C/CTRL_BREAK to every process attached to the console — a Ctrl+C meant for one pane can hit the engine

Two consequences drive this PR directly:

1. PID reuse is a routine hazard on Windows, a rare one on POSIX. Bare existence liveness is "usually fine" on Linux but wrong often enough on Windows to break resume/reclaim/prune. Persisting and comparing identity makes the answer correct on both — a reused PID has a different create_time/start-time, so it reads as not-ours.
2. os.kill(pid, 0) and console Ctrl+C are the same underlying Windows behaviour (GenerateConsoleCtrlEvent). The liveness path avoids os.kill entirely on win32 (psutil), and the engine ignores console-ctrl during a run so an errant broadcast can't take it down.

Testing

New/updated unit tests across test_process_host.py, test_runs.py, test_tui_data.py, test_engine.py, and test_cli.py cover: identity match vs. reuse (reused PID → dead / INTERRUPTED), legacy pid-file degradation, malformed-identity fail-closed (garbage/nan/inf), pre-stop PID reuse → clean stop (no stray SIGTERM, no error), the legacy force-kill fallback, and the win32 console-ctrl-ignore path (both platforms). The POSIX self-liveness test that calls os.kill(pid, 0) is skipped on win32 (it would fire a real Ctrl+C and abort the runner) with a psutil-based WindowsProcessHost self-liveness test covering Windows.

Green on Linux (WSL — full touched-file suite passes; the full suite is green aside from two pre-existing, unrelated skill-sync failures) and on native Windows (targeted Windows-safe subset). Lint clean: ruff, black, isort, bandit.

Follow-up

A native Windows CI runner (windows-latest job) plus correct POSIX/Win32 test-skipping — gating the POSIX-only tests that crash or fail on Windows (the os.kill(pid, 0) self-liveness probe, true-binary dead-PID helpers, test -f verify commands, and the real-signal self-terminate test) — so the whole suite runs green on Windows CI and continuously validates this PR's Windows behaviour. Kept out of this PR to keep it focused on the process-safety change; tracked as a separate follow-up.

Separate PR, on top of this one): the inverse, native-Windows counterpart of what this PR fixes. On win32 the identity-aware read can report a live pid as DEAD when its identity is merely unreadable rather than gone - psutil.Process(pid).create_time() raising ERROR_ACCESS_DENIED (cross-session / elevation mismatch), a failure mode the POSIX /proc start-time path doesn't have — surfacing a running engine as INTERRUPTED. It belongs to this same win32 process-safety line and will be handled on its own by biasing a non-destructive liveness read to unknown/alive on an unreadable-but-existing pid (the destructive stop path keeps failing safe); kept out of this PR because it builds directly on this read path and is a read-vs-stop bias design decision.

Summary by CodeRabbit

• Bug Fixes
  • Improved run stop handling so reused process IDs are less likely to be mistaken for active runs.
  • Run status now better reflects interrupted or dead states when a recorded process no longer matches the original one.
  • Enhanced stop behavior on Windows console-control events to avoid unintended stop signals.

[coderabbitai]

Walkthrough

This PR introduces identity-aware PID liveness checks to prevent false positives from PID reuse. ProcessHost gains alive_and_ours, runs.py persists and validates an identity token alongside PIDs in engine.pid, TUI liveness checks adopt the same logic, and the engine adds Windows-specific console-ctrl signal handling.

Changes

Identity-aware PID liveness and Windows signal handling

Layer / File(s)	Summary
Process liveness primitive src/automator/process_host.py, tests/test_process_host.py	Adds alive_and_ours(pid, identity) to ProcessHost, rejecting non-positive PIDs, falling back to is_alive when identity is None, otherwise requiring identity match; adds tests for these branches and platform-specific is_alive coverage.
Persisted identity token and stop-run guards src/automator/runs.py, tests/test_runs.py, tests/test_cli.py	write_pid persists a two-token pid+identity format; read_pid_identity parses it (failing closed on corrupt values); engine_alive and stop_run use alive_and_ours to avoid signaling/force-killing reused PIDs; tests cover parsing, mismatch, legacy files, and clean-stop-on-reuse scenarios.
TUI liveness identity check src/automator/tui/data.py, tests/test_tui_data.py	liveness() now reads pid+identity via read_pid_identity and calls alive_and_ours, treating reused PIDs as dead; adds a watcher test for the reuse scenario.
Windows console-ctrl signal handling src/automator/engine.py, tests/test_engine.py	Engine imports sys and ignores Windows console-ctrl signals (SIGINT/SIGBREAK) by journaling console-ctrl-ignored instead of raising RunStopped; non-ignored signals retain existing stop behavior; adds Windows and non-Windows tests.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Possibly related issues

• engine.pid: verify process start-time identity to defend against PID reuse #12: This PR implements the identity-token approach alongside engine.pid to prevent PID-reuse false positives in engine_alive, stop_run, and TUI liveness checks.

Poem

A pid alone can lie and cheat,
reused by strangers on the street,
so now I check an identity too,
before I say "the run is through!"
Hop, hop, guard the process true. 🐇🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the two main changes: identity-aware process liveness and Windows console-control handling.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[augmentcode]

🤖 Augment PR Summary

Summary: This PR makes engine liveness identity-aware (detects PID reuse) and hardens Windows console Ctrl+C/Ctrl+Break handling so runs can’t be killed mid-flight by console broadcasts.

Changes:

• Persist process identity alongside PID in .automator/runs/<id>/engine.pid (legacy pid-only files still supported).
• Add read_pid_identity() and route liveness checks through ProcessHost.alive_and_ours() to distinguish “our engine” vs. a reused PID.
• Update engine_alive(), TUI liveness/status, and stop_run() to avoid signalling reused PIDs and to force-kill only when identity can be verified (with a legacy fallback).
• On win32, ignore console SIGINT/SIGBREAK during a run (while still handling SIGTERM).
• Expand unit tests (CLI, engine, runs, TUI, process_host) to cover identity match/mismatch, malformed identity tokens, legacy degradation, and Windows ctrl-signal behavior.

Technical Notes: Malformed/non-finite identity tokens fail closed (treated as not-ours), while true legacy pid-only files degrade to PID-existence behavior.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. 2 suggestions posted.

Comment augment review to trigger a new review at any time.

[coderabbitai]

🧹 Nitpick comments (1)

src/automator/engine.py (1)

284-294: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Suppress the flagged try/except/pass per Ruff S110.

Static analysis flags this block (S110). The rest of this file suppresses the same pattern with an explicit reason comment (e.g. # noqa: BLE001 # nosec B110 - best-effort teardown...), but this new block only silences BLE001, leaving S110 unaddressed.

🔧 Suggested fix

             if sys.platform == "win32" and signum in windows_ctrl_signals:
                 try:
                     self.journal.append("console-ctrl-ignored", signum=signum)
-                except Exception:  # noqa: BLE001  # best-effort signal-handler journal
+                except Exception:  # noqa: BLE001, S110  # best-effort signal-handler journal; safe to swallow inside a signal handler
                     pass
                 return

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/automator/engine.py` around lines 284 - 294, Update the signal handler in
engine.py so the best-effort journal write no longer triggers Ruff S110: keep
the existing try/except around self.journal.append in handler, but add the same
explicit nosec/justification-style suppression used elsewhere in this file for
similar teardown paths, and make sure the exception handler is documented to
satisfy both BLE001 and S110. Locate the change in the handler nested inside the
automator engine logic, and align its comment/suppression with the other
teardown-safe try/except/pass blocks.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/automator/engine.py`:
- Around line 284-294: Update the signal handler in engine.py so the best-effort
journal write no longer triggers Ruff S110: keep the existing try/except around
self.journal.append in handler, but add the same explicit
nosec/justification-style suppression used elsewhere in this file for similar
teardown paths, and make sure the exception handler is documented to satisfy
both BLE001 and S110. Locate the change in the handler nested inside the
automator engine logic, and align its comment/suppression with the other
teardown-safe try/except/pass blocks.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 58a8535a-48ef-45e6-bf4e-ecc5040818fd

📥 Commits

Reviewing files that changed from the base of the PR and between f79bd12 and 738f11b.

📒 Files selected for processing (9)

• src/automator/engine.py
• src/automator/process_host.py
• src/automator/runs.py
• src/automator/tui/data.py
• tests/test_cli.py
• tests/test_engine.py
• tests/test_process_host.py
• tests/test_runs.py
• tests/test_tui_data.py

[pbean]

✅ Validation — safe to merge

Validated against current main, focused on whether it regresses the POSIX (Linux/macOS) path. Verdict: valid, well-scoped, no Linux/macOS regression. CI green (lint + py3.11–3.14 + version-sync). Implements #12.

What it fixes

Liveness was bare PID existence, so a recycled PID read as a false "alive" — cascading into refused resume, stranded worktree reclaim, leaked sessions, and dead runs shown as RUNNING. Persisting + comparing a process identity (/proc start-time / psutil create_time) fixes it; the win32 console-ctrl guard stops a broadcast Ctrl+C from killing the engine mid-run.

POSIX safety — checked

• macOS-safe: PosixProcessHost.identity() reads /proc only on Linux; on macOS it falls back to psutil and returns None when absent → alive_and_ours degrades to today's is_alive. No change without psutil; the fix is gained with it. is_alive stays os.kill(pid, 0).
• No missed readers: the only two spots that parsed engine.pid as a bare int (runs.read_pid, tui/data.liveness) are both updated; nothing else reads the file, so the new "<pid> <identity>" format breaks nothing. read_pid stays a wrapper.
• stop_run is strictly safer: the pre-terminate alive_and_ours check means a reused PID is never signalled (previously it sampled the stranger's identity and could terminate it). Legacy pid-only files skip the pre-check and keep the stop-time force-kill fallback — no capability loss for pre-upgrade runs.
• engine.py: the ignore-branch is gated on sys.platform == "win32"; on Linux SIGBREAK does not exist, so the signal set/handler are byte-identical to today.
• Fail-closed: a corrupt/nan/inf identity token maps to an impossible sentinel → reads not-ours, never a false alive. Float round-trip is exact.

Residual (Windows-only, out of scope here)

On win32 a live pid whose create_time is unreadable (ERROR_ACCESS_DENIED, cross-session/elevation) reads DEAD → a running engine could surface as INTERRUPTED. POSIX /proc start-time has no such failure mode. This is the read-vs-stop bias the author deferred — filed as #33 (follow-up #2). Follow-up #1 (native Windows CI + POSIX/Win32 test gating) is #31.

LGTM.

_{🤖 Validation assisted by Claude Code.}

[pbean]

Approving — full validation in the comment above: valid, well-scoped, no Linux/macOS regression, CI green. Windows-only residual tracked as #33.