fix(session): route the gatewayd orphan kill through platform_compat

[DeryFerd]

Problem / Motivation

The gatewayd orphan reaper in src/kiro_crew/session_pid.py signalled with the raw POSIX spellings: os.kill(pid, signal.SIGTERM) and the escalation os.kill(pid, signal.SIGKILL) in _kill_orphan_gatewayd (session_pid.py:1554 and :1577), a raw os.kill(pid, 0) liveness probe in its TERM grace loop (:1560), and a raw os.kill(pid, signal.SIGKILL) for the sweep's shared-pgid fallback (:2030). Two of its sibling functions in the same file already avoid exactly these spellings: _kill_orphan_work_tree kills through platform_compat.kill_pid(target, platform_compat.SIGKILL) (:2167), and _kill_orphan_browser_daemon probes through platform_compat.pid_exists with a written rationale for doing so ("routing through the shim keeps it correct on its own terms rather than depending on a caller's early-out", :1615-1618). The gatewayd reaper is the one that never got the same treatment, and it is also the function the windows-kill-probe audit carries an explicit GATED_PROBES exemption for — an exemption whose justification is precisely that the raw spellings inside cannot run on Windows.

Why it matters

The raw spellings are the ones the repo's cross-platform table documents as unavailable or destructive on Windows: signal.SIGKILL does not exist there (an AttributeError, not a caught OSError), and os.kill(pid, 0) is not a liveness probe at all there — it terminates the target. None of that is reachable today, because the sweep entry point kill_orphan_mcps early-outs under if platform_compat.IS_WINDOWS, and that is exactly the shape the file's own browser-daemon comment argues against: correctness that depends on remembering which caller gates what, rather than on the function itself. The test_windows_kill_probe_audit.py tripwire exists because this class of latent hazard keeps regressing; every entry on its allowlist is a place where "cannot happen on Windows" is load-bearing with no local proof. This PR removes one of those entries instead of adding one.

What changed (motivation → approach → change)

Symptom: one orphan-reaper function uses the raw signal spellings its siblings and the audit both route around. Approach: move the pid-targeted paths onto the same shim the file already uses elsewhere, and leave the group-targeted paths alone. Concretely, in _kill_orphan_gatewayd:

• the initial SIGTERM and the escalation SIGKILL now go through platform_compat.kill_pid with platform_compat.SIGTERM/SIGKILL; on POSIX this is the identical os.kill call, and the shim propagates the same exception types (ProcessLookupError, PermissionError, OSError) the existing handlers already catch
• the grace-loop liveness probe now goes through platform_compat.pid_exists, which preserves the exact meaning the loop had: only ProcessLookupError reads as "gone" (the shim maps EPERM/other OSError to "still exists"), so the loop's behavior is unchanged on every input
• the sweep's shared-pgid direct kill in kill_orphan_mcps routes through platform_compat.kill_pid the same way

What deliberately does not change: the os.getpgid / os.getpgrp / os.killpg group-targeted branches. There is no shim spelling for signalling a process group rather than a pid, and the isolated-leader predicate guarding those calls (pgid == pid and pgid != os.getpgrp() and pgid > 1) is POSIX process-group semantics that cannot be expressed through a pid-targeted helper. Rewriting them would change what the code protects, not just how it spells it.

Because the raw signal-0 probe this function carried is gone, its GATED_PROBES entry in test/test_windows_kill_probe_audit.py is removed in the same change. That is not bookkeeping I noticed later — the audit's own test_allowlist_has_no_stale_entries goes red the moment a listed site no longer contains a raw probe, which is how this PR found out the allowlist tracks these sites at all. GATED_PROBES shrinks from four entries to three.

Tests

New file test/test_session_pid_reaper_shim.py, four tests that pin the routing with spies on the shim rather than on os:

• test_term_is_sent_through_kill_pid — the TERM goes through kill_pid, and a daemon that exits before the first poll counts as one kill
• test_escalation_sigkill_goes_through_kill_pid — wedged daemon past the grace window, identity recheck passing, shared pgid: the direct SIGKILL goes through kill_pid, on both the Linux and non-Linux identity paths
• test_liveness_probe_uses_pid_exists_not_signal_zero — the grace loop consults pid_exists and never os.kill, with the raw call asserted not-called
• test_shared_pgid_orphan_killed_via_kill_pid — through the full kill_orphan_mcps sweep, the pid-targeted branch calls kill_pid(pid, SIGKILL) and not killpg

I wrote the tests first and watched them fail against the unmodified module — two of the four failures were the raw spellings executing for real on Windows (OSError: [WinError 87] from os.kill with a signal constant, AttributeError for the absent os.getpgid), which is the failure mode this PR removes the spelling of. The killpg branch keeps its existing coverage in test_pid_lifecycle.py, and the sweep's IS_WINDOWS gate keeps its existing pin in test_windows_kill_probe_audit.py.

python -m pytest test/test_session_pid_reaper_shim.py test/test_windows_kill_probe_audit.py -q  # 12 passed
python -m pytest test/test_pid_lifecycle.py test/test_gatewayd_self_exit.py test/test_session_cleanup.py test/test_devfleet_proc_stamp_orphan.py test/test_session_pid_reaper_shim.py test/test_windows_kill_probe_audit.py -q  # 198 passed, 7 failed
python -m flake8 src/kiro_crew/session_pid.py test/test_session_pid_reaper_shim.py test/test_windows_kill_probe_audit.py  # clean
python -m black --check <the three changed files>  # unchanged
python -m mypy src/kiro_crew/session_pid.py  # Success: no issues found in 1 source file

Manual verification

N/A — unit coverage is sufficient here. The reaper's observable behavior is entirely which function receives which signal, which is what the spy tests assert; the POSIX execution path is the identical os.kill call by the shim's own implementation, and the Windows path cannot execute by the sweep's existing gate, which the audit test pins.

The 7 failures in the second command are pre-existing on this Windows machine and unrelated: I re-ran the same set on a clean checkout of main with the change stashed and got the same seven (os.getuid/os.getpgrp absent on frozen Windows os in test_pid_lifecycle.py and one gatewayd socket-path test), all inside POSIX-only test paths that this change does not touch.

Screenshots / video

N/A — no user-visible change.

Related Issues

No dedicated issue; this follows the cross-platform routing convention from the platform_compat table in AGENTS.md. The audit trail for the removed exemption is test/test_windows_kill_probe_audit.py itself.

Pattern harvest

Not generalizable: the defect class already has its rule, the whole-tree AST tripwire in test_windows_kill_probe_audit.py plus the justified GATED_PROBES allowlist, and this PR is that rule working as designed (one raw probe removed, one allowlist entry pruned, the stale-entries guard forcing the cleanup). The remaining gap in the class — raw spellings that are real signals rather than probes, like the ones this PR converted — is a per-site migration with no mechanical rule to add beyond what the table and review already cover.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title — one commit: fix(session): route the gatewayd orphan kill through platform_compat
• Existing tests pass and new tests added for new functionality
• Self-review completed; routing follows the file's own established shim usage
• Documentation updated (if applicable) — N/A: no doc describes these call sites; the cross-platform convention lives in the platform_compat table and the audit test
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 514da2a149467b600aafd8cbc426022caed8d065 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The change checks out against the base: kill_pid on POSIX is exactly os.kill with identical exception propagation (platform_compat.py:2883), pid_exists preserves the "only ProcessLookupError means gone" semantics (platform_compat.py:2519), and the diff mirrors the pattern the sibling _kill_orphan_browser_daemon already established at session_pid.py:1615-1619. The group-targeted killpg branches are correctly left alone (no shim spelling exists for group signals), and the removed GATED_PROBES entry corresponds exactly to the removed raw probe, forced by the audit's own stale-entry guard.

Design-Verdict: PASS

Converges a divergent reaper onto the file's own established shim pattern, retiring an audit exemption at its root rather than re-justifying it.

[DESIGN-REVIEWED] 514da2a

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 514da2a149467b600aafd8cbc426022caed8d065 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 514da2a

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 514da2a149467b600aafd8cbc426022caed8d065 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 514da2a

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of 514da2a149467b600aafd8cbc426022caed8d065 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims in the intent check out against the base tree: the four converted sites are the only raw os.kill calls in session_pid.py; the shim preserves the exact exception semantics the handlers rely on (kill_pid propagates on POSIX, pid_exists maps only ProcessLookupError to "gone"); the allowlist removal is forced by test_allowlist_has_no_stale_entries; and the remaining raw-signal sites elsewhere (cli_server.py:495, port_reclaim.py:195) each carry a local Windows branch, so they are not unfixed siblings of this hazard class. The shim spelling is mandated by the AGENTS.md platform_compat table, which makes every code item derived by documented invariant.

First-Principles-Verdict: PASS

Every item is mandated by the AGENTS.md platform_compat table or forced by the audit's own stale-entry guard; nothing rides along.

What this change ships

Intent: make the gatewayd orphan reaper safe by construction on Windows instead of by a caller's gate — a FIX (latent-hazard removal).

1. Gatewayd TERM and escalation SIGKILL now go through platform_compat.kill_pid — justified (AGENTS.md shim table).
2. Grace-loop liveness check uses pid_exists, not the signal-0 probe that terminates on Windows — justified.
3. Sweep's shared-pgid direct SIGKILL routed through kill_pid — justified.
4. Windows-probe audit allowlist drops its gatewayd exemption — justified (the stale-entry test forces it).
5. Four new tests pin the shim routing per function — justified (pins the fix).
6. Existing sweep test re-spies on the shim instead of os.kill — justified (required by 1–3).

Counts run: os.kill( in session_pid.py — the 4 converted sites were the only call sites (rest are comments). Same pattern across src/kiro_crew — the only remaining same-class site is acp/client.py::_kill_escaped_children, which keeps its own locally-gated GATED_PROBES entry; cli_server.py:495 and dashboard/port_reclaim.py:195 are POSIX fall-throughs behind explicit Windows branches, not siblings. Shim exception semantics verified identical on POSIX (platform_compat.py:2883, :2519), so the loop's "only ProcessLookupError reads as gone" meaning is preserved.

[FIRST-PRINCIPLES-REVIEWED] 514da2a

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• This PR is OVERLAPPING with PR #8276. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7871: KEEP. Independent change in a different function; can merge in either order. Files: src/kiro_crew/session_pid.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.
• This PR is OVERLAPPING with PR #8304. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7871: KEEP. Complementary and both worth having, but they collide on one branch of kill_orphan_mcps. Land 7871 first (4 files, no behavior change, all AI lanes PASS), then have 8304 rebase and adopt platform_compat.kill_pid for its root kill so the retired exemption is not reintroduced; either order needs an explicit conflict resolution rather than an independent merge. Files: src/kiro_crew/session_pid.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[DeryFerd]

@bolichen97 Thanks for the relationship audit, and the coordination plan sounds right to me. Happy for this one to land first: it is conversion-only (no behavior change on POSIX), every required check is green on 514da2a and it merges clean against current main, so the only thing it is waiting on is a maintainer review.

On #8304, agreed that adopting platform_compat.kill_pid for its root kill on rebase is the right call rather than resolving the conflict the other way. One thing that helps here: the four shim-spy tests added in this PR (test/test_session_pid_reaper_shim.py) pin the shared-pgid branch of kill_orphan_mcps to kill_pid, so once this lands, that adoption is enforced by tests, not just convention. On #8276, agreed it is independent (different function), so no special coordination needed beyond normal conflict resolution if timings overlap.

[bolichen97]

Verified the routing is behavior-preserving rather than merely cosmetic: platform_compat.kill_pid delegates to os.kill on POSIX and lets ProcessLookupError propagate, so the reaper's early-out on an already-dead pid still fires, and pid_exists maps only ProcessLookupError to False (EPERM and other OSError read as still-alive), which is exactly what the old grace loop's bare except ProcessLookupError did. Leaving the os.getpgid/getpgrp/killpg group-targeted branches raw is the right call since no shim spells a process-group signal, and pruning the now-dead session_pid GATED_PROBES entry keeps the audit allowlist honest instead of growing it.