POK-85: Fix Ctrl-C / graceful exit in aikit update

[saheljalal]

What & why

aikit update couldn't be aborted cleanly with Ctrl-C (reported through tmux, but the defect is environment-independent). Tracing SIGINT handling showed the root cause is subprocess lifecycle, not signal delivery:

run() executed each update/install command with subprocess.run(shell=True, capture_output=True). On interrupt Python's subprocess.run only kills the immediate shell, so the real installer tree it spawned (npm/pip/curl and their children) was orphaned and kept running. aikit itself returned, but control never cleanly came back to the terminal — which reads as "couldn't exit gracefully," and is most visible under tmux where the orphans stay attached to the pane.

Fix

aikit run() (_terminate_process_tree + rewrite)

• Launch captured commands in their own session/process group (start_new_session, POSIX) with stdin closed.
• On KeyboardInterrupt or timeout, tear down the entire group (SIGINT/SIGTERM → SIGKILL), then re-raise so the single termination point in sk.run_cli still exits 130. aikit — not the tty — owns the teardown, so behaviour is identical in tmux and a plain shell.
• Timeouts now kill the whole tree instead of leaking a runaway process.
• Non-POSIX / non-captured runs fall back to single-process teardown. Exit codes, return shape, and output are unchanged.

scriptkit parallel_map() (used by aikit agent discovery: aikit, doctor, setup, first run)

• The old ThreadPoolExecutor blocked at context-manager exit until every in-flight task finished, so Ctrl-C during discovery hung until the slowest version check's network timeout (up to ~20s) returned. Replaced with daemon workers + an interruptible queue drain, so the interrupt is raised in the main thread immediately. Signature, completion-order results, progress rendering, and worker-exception propagation are unchanged.

This follows the repo convention that graceful exit lives in scriptkit/sk.run_cli — no new try/except KeyboardInterrupt was added to the tool.

Behavioral note — captured stdin (intentional, reviewed safe)

Captured commands (capture=True, the default and every current caller) are now launched with stdin=subprocess.DEVNULL instead of inheriting the parent's stdin. This is a deliberate part of the fix: a session-isolated background command has no controlling terminal, so an inherited stdin could block on a read (or raise SIGTTIN) and hang the run. With stdin closed, a stray read gets EOF immediately instead. The Code Reviewer confirmed no caller pipes input into run(), so nothing any caller relies on changes — these are non-interactive install/update/version-check commands whose output is already captured. capture=False runs are unaffected and still inherit stdin.

Tests

New regression tests, all green (venv/bin/python -m pytest; pre-existing unrelated pluck failures aside):

• tests/test_aikit_interrupt.py — session isolation, timeout tree-teardown, and interrupt tree-teardown (liveness verified via a heartbeat file so a killed-but-unreaped zombie can't masquerade as alive).
• tests/test_cli_progress_tables.py — parallel_map completion order, worker-exception propagation, and prompt-interrupt (asserts Ctrl-C returns without awaiting 5s workers).

Verified end-to-end by driving a real SIGINT at a process group: before, a signal-ignoring grandchild survived; after, the whole tree is gone and the process exits 130.

Versioning

• aikit 1.15.4 → 1.15.5 (PATCH — bug fix, no interface change)
• scriptkit 1.2.0 → 1.2.1 (PATCH — bug fix, no interface change)

Closes POK-85

[saheljalal]

Review — POK-85 Ctrl-C / graceful exit in aikit update

Verdict: Approve. Root cause is correctly identified (orphaned installer tree, not signal delivery), the fix is defensively coded with sane fallbacks, and the regression tests are meaningful and pass. I checked out the branch, traced the paths, and ran the suite locally — everything below the diff holds up.

What I verified

• run() teardown (aikit:1111, _terminate_process_tree at aikit:1059): capturing pgid = proc.pid up front (before the group leader can exit) is the right call — os.killpg(pgid, …) still reaches the tree after the leader dies. SIGINT/SIGTERM→SIGKILL escalation with a grace reap in between is correct, and the pgid is None fallback (capture=False / non-POSIX) tears down only the single child, so we can never signal aikit's own group. signal is imported, hasattr(os, "killpg") guards the group path.
• Interrupt semantics: start_new_session=True puts the child in its own session, so the tty's Ctrl-C reaches only aikit; aikit forwards SIGINT to the group and re-raises, letting the single termination point in sk.run_cli (scriptkit/cli.py:63) exit 130. Confirmed that convention exists — no new try/except KeyboardInterrupt in the tool, per repo style. 👍
• Timeout path: after TimeoutExpired, tearing the tree then a second communicate(timeout=5) to drain is correct; the (TimeoutExpired, ValueError) guard covers the reaped-then-drain case.
• parallel_map rewrite (scriptkit/progress.py:79): daemon workers + a 0.1s-polled drain gives prompt interrupt; every item yields exactly one completed entry so the range(len(items)) drain can't deadlock; completion-order and exception propagation preserved.
• Tests: tests/test_aikit_interrupt.py and the new parallel_map cases pass locally. The heartbeat-file liveness check (vs os.kill(pid,0)) is a nice touch — a killed-but-unreaped zombie can't masquerade as alive. Full suite green except the pre-existing pluck characterization failures, which I confirmed also fail on main — unrelated to this PR.

Notes (non-blocking)

• Nit / confirm — captured commands now get stdin=DEVNULL (aikit:1132). Previously subprocess.run(capture_output=True) left stdin inherited. This is a behavioral change and, I'd argue, an improvement (a captured child that prompts on stdin was already invisible since stdout is piped, so it would hang; EOF now fails fast and the child can't steal terminal input). I checked every run() caller — none pipe input — so this is safe, but it's worth a one-line mention in the PR body since it's a real semantics change beyond the interrupt fix.
• Nit — parallel_map worker-exception cleanup (scriptkit/progress.py): on the first worker exception _drain raises immediately, leaving the remaining daemon workers running (they're abandoned, not awaited). The old ThreadPoolExecutor blocked at __exit__ until in-flight tasks drained. For the discovery use case (read-only version checks, no shared mutation) this is harmless and arguably better, but the daemons keep pulling from pending/pushing to an orphaned completed queue until they finish. Fine as-is; flagging only so it's a conscious trade-off.

Nice, well-scoped fix — correct diagnosis, minimal surface, good coverage.