fix(platform): give get_process_start_id a Windows arm

[DeryFerd]

Problem / Motivation

platform_compat.get_process_start_id is the tree's per-process incarnation identity: a stable string that differs across two processes sharing one PID, so a caller can tell "still the process I spawned" from "this PID was recycled". The docstring promises an arm for every platform, and Linux (field 22 of /proc/<pid>/stat) and macOS (libproc, microsecond resolution) both deliver one. Windows falls through to return None.

Four consumers read that token, and all four degrade to their token-less legacy behavior on Windows:

• session_pid.py writes sweep entries in the legacy 2-field <gw>:<pid> form, so the sweep cannot prove an entry is stale by token mismatch and falls back to cmdline matching plus spawn-grace heuristics.
• session_pid_sig.py publishes the signed session_pid_<pid>.txt mapping without the token line, so the PID-recycle guard added in fix(session-pid): bind the signed pid mapping to the process incarnation #8467 never fires there.
• mcp_gateway/claim.py sends pid_start_id: null in its claim frames, so gatewayd cannot skip a connection whose PID was recycled underneath it.
• metrics/sessions.py can refine crumb ownership by PID only, never by incarnation.

Windows is the platform where this matters most. Its PID space is small and recycled aggressively, so the misattribution window #8343 describes (a recycled PID keeps answering for the previous owner's session key until the next restart's sweep) is widest there, and the guard that closes it works least there. #8473 was filed as the follow-up, carrying the Design review lane's watch item on #8467.

Why it matters

#8467 bound the signed pid mapping to the process incarnation, but its Windows behavior is "publish the legacy form and let readers treat the token as unknown". That degrade is safe by design, which is also why it sat quietly: nothing errors and nothing logs, the guard just never fires. On Windows the mapping behaves as it did before #8467, and the one platform where a recycled PID is most likely to hand another process's identity to a new owner is the one platform the guard skips.

The fix is also cheap to review, because everything downstream was built to accept a token the day #8467 landed. The readers dual-parse the legacy and token-bearing forms, and every comparison treats None as "identity unknown", never as a mismatch. So this PR changes one function and four consumers pick the guard up with zero code changes; the rest of the diff is tests and disclosures that no longer apply.

What changed (motivation → approach → change)

Symptom: get_process_start_id returns None on Windows. Root cause: the function never had an IS_WINDOWS branch. Fix: give it one, reading the process creation FILETIME through the seams the codebase already trusts. process_start_time(), the PID-reuse guard the kill paths use, already reads that value on Windows through _open_process_query_handle (QUERY_LIMITED_INFORMATION only), _windows_process_handle_identity, and _close_process_handle. The new branch makes the same three calls and returns str(identity[1]), the creation FILETIME as a decimal string.

The reuse is the main design decision. The issue sketched fresh ctypes code, but two identity APIs on one platform should not drift in how they acquire or release a process handle, and reusing the seams makes get_process_start_id(pid) == process_start_time(pid) true by construction on Windows. A token persisted by either caller then compares equal to the other's value for the same process, which is a property consumers would otherwise have to test into existence.

One consumer deliberately does not pick the token up: session_pid._pid_age_seconds derives the spawn-grace window by interpreting the darwin token as epoch seconds (time.time() - float(start_id)), and the Windows FILETIME counts 100-ns units since 1601. Feeding it in would compute an absurd age, so that function keeps its existing Windows gate and its docstring now records why. Spawn-grace behavior on Windows is unchanged; what changes there is the sweep entry form.

The token stays opaque on purpose. Nothing parses it, and the one numeric consumer is now fenced off, so the tree's existing dual-parse (by form, not by units) keeps working untouched.

The diff is nine files, and only the first three carry behavior:

• src/kiro_crew/platform_compat.py — the IS_WINDOWS branch, plus a docstring that documents the arm and stops listing Windows among the unknown-failure cases.
• src/kiro_crew/session_pid.py — no code change; _pid_start_token no longer lists Windows as a None case, and _pid_age_seconds explains why its gate must stay.
• src/kiro_crew/metrics/sessions.py — no code change; the liveness-versus-identity rationale no longer cites Windows as a platform with no token.
• test/test_windows_process_start_id.py (new) — live tests through the real seams, Windows-only. test_platform_compat_coverage.py deliberately skips itself on Windows hosts (it simulates both platforms by flipping attributes that do not exist on win32), so real-seam coverage needs its own file.
• test/test_platform_compat_coverage.py — the old "Windows is unknown" placeholder is replaced by three mocked arm tests for the ubuntu CI lane: the token equals the mocked creation FILETIME with the handle opened once and closed, a failed open is unknown, and an unreadable identity is unknown with the handle still released.
• test/test_pid_lifecycle.py — one docstring: the legacy-fallback test patches the token to None directly, and Windows is no longer one of the None cases.
• docs/system-specs/modules/metrics.md, docs/system-specs/modules/session.md, docs/architecture/design-notes/mcp-gateway-claim-push.md — the three specs that documented the missing arm as a permanent state stop describing it that way, since the project's docs rule updates the spec in the same commit as the behavior.

Tests

• I wrote the live Windows test first and watched it fail (assert None is not None), then added the branch and watched it pass, so the test catches the regression it exists for instead of decorating the fix.
• test/test_windows_process_start_id.py, 3/3 on my Windows 10 machine (native run): the identity is non-None, digits only, colon-free; it equals process_start_time(os.getpid()); it is stable across calls; an unopenable PID reads as None; and session_pid._pid_start_token returns a token on Windows, which is what flips sweep entries out of the legacy form.
• Scoped run over the consumer suites (test_pid_lifecycle.py, test_session_pid_sig.py, test_mcp_gateway_claim.py, test/metrics/test_session_duration.py): 286 passed, 33 skipped. The 6 failures are the known Windows-baseline ones (tests that monkeypatch os.getpgrp/os.getuid directly, which a frozen Windows os module does not have); I confirmed they fail identically on a clean checkout of this branch's base with the change stashed, so none come from this PR.
• test_windows_kill_probe_audit.py — 4/4. The new handle is query-only, so the audit's raw-kill-probe allowlist is untouched.
• flake8, isort, mypy (scoped to the three changed modules), black on the non-baseline files, and the docs lint all pass.

Manual verification

The mocked tests prove the seams are called; they cannot prove the token is real on a live OS. On the Windows 10 machine this was written on, get_process_start_id(os.getpid()) returns a decimal string equal to process_start_time(os.getpid()), stable across calls within the same process. A reviewer on any Windows box can repeat that in a REPL in under a minute.

The user-visible effect on Windows once this lands: the signed session_pid_<pid>.txt mapping publishes the token-bearing two-line form, so the PID-recycle guard fires there, and sweep entries carry the <gw>:<pid>:<start_token> shape. Before this PR those were the legacy forms with the guard silently inert.

Screenshots / video

Nothing visual changes; the observable behavior is an on-disk mapping format and a sweep-entry shape, which the tests pin. If a maintainer wants a capture of the mapping file before and after, I can add one under temp-screenshots/.

Related Issues

Closes #8473. Builds on #8467 (signed pid mapping) and #8343 (the misattribution report that motivated the incarnation guard). The metrics.md, session.md, and mcp-gateway-claim-push.md updates ride along because all three documented the missing arm.

Checklist

• Test-first: the live Windows test failed before the arm existed and passes after
• Specs updated in the same commit as the behavior change
• The kill-probe audit and its allowlist are untouched; the new handle requests no terminate rights

Pattern harvest

Rule candidate: review-prompt — a new platform arm of an identity primitive must read through the same handle-acquisition seams as the platform's existing identity readers, so the two APIs cannot drift in how they open or close the handle; reusing process_start_time's QUERY-ONLY seams here is also what makes the two tokens equal by construction.
Rule candidate: testing conventions — when a coverage test file deliberately skips itself on a real platform (it simulates branches via attributes that platform lacks), live-path coverage for that platform's arm needs a dedicated file gated to the real host; a mocked file proves seam wiring, never the OS value.
Not generalizable: the spawn-grace age read keeps its Windows gate because it interprets the darwin token as epoch seconds and the FILETIME is not epoch-based; that is a property of this one numeric consumer, not a pattern other consumers need.

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

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

The CI has a real backend-test-windows lane on windows-latest, so the new live test file is CI-guarded, not author-machine-only. All my candidate concerns are resolved: the old test_windows_is_unknown_rather_than_a_mismatch pin was a documented gap with a filed follow-up (#8473), not a decision — the base docs (instances.md) called the empty Windows token "an unsatisfiable requirement"; the new arm reuses the exact query-only handle seams process_start_time's shipped Windows arm already uses, making the two tokens equal by construction; failure degrades to None → identity-unknown → pre-PR behavior everywhere; and the want_exit_time=False mode preserves default semantics for the kill-path callers while honoring the event-loop non-blocking contract.

Design-Verdict: PASS

Root-cause fix through the platform's existing identity seams, fail-safe to prior behavior on any read failure, with live Windows coverage in the CI shard.

[DESIGN-REVIEWED] 977f361

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

FINDING -- src/kiro_crew/session_pid.py:173 -- “a ps failure” contradicts the get_process_start_id()-only path -> Fix: say “an identity-probe failure”.
[GPT-REVIEWED] 977f361

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 977f361

[github-actions]

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

Premise-level review of 977f361cbd1fdf800ca8db004b4f0387fa3ba376 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 verification is done. The base tree confirms every load-bearing claim: the missing Windows arm (platform_compat.py:1534), the four-plus consumers that treat None as unknown, the spawn-grace gate at session_pid.py:63 that fences the FILETIME off from epoch arithmetic, process_start_time's identical three-seam Windows read, and the coverage file's module-wide self-skip on Windows that justifies the new live test file. Here is the review:

First-Principles-Verdict: PASS

Only the author's Windows 10 box has run the real seams — confirm the Windows CI lane collects and passes test_windows_process_start_id.py on this PR.

What this change ships

Inventory (10 items) — 10 justified

Intent: make the PID-recycle guard actually fire on Windows by giving the per-process incarnation token a Windows arm — a FIX (linked #8473/#8467/#8343; the base's own docs at instances.md:1068 and run_marker.py:127 record the gap).

1. A Windows process now has a real incarnation token (creation FILETIME) instead of none — justified
2. Windows sweep entries flip from legacy 2-field to the token-bearing <gw>:<pid>:<token> form — justified
3. The signed pid mapping on Windows carries the token line, so the recycle guard fires there — justified
4. Windows claim frames carry a real pid_start_id, so gatewayd can skip recycled-PID connections — justified
5. Metrics crumb ownership on Windows refines by incarnation, not pid alone — justified
6. Identity-only reads skip the exit-time publication poll (want_exit_time=False), honoring the documented non-blocking event-loop contract — justified
7. A zero creation FILETIME reads as unknown before the exit-code read (moved check, same outcome for the existing caller; required by item 6's early return) — justified
8. Spawn-grace stays absent on Windows, with the FILETIME-units reason now recorded at its gate — justified
9. The "Windows returns None" pin becomes three mocked-arm tests plus a live Windows test file (the pin recorded the fixed defect; failure cases keep unknown-never-mismatch) — justified
10. Seven docs/docstrings stop listing Windows among the unknown-token cases — justified (AGENTS.md same-commit docs rule)

The delegation alternative (return process_start_time(pid) on Windows) was checked and is not the smaller fix: it inherits the up-to-250ms exit-FILETIME poll (platform_compat.py:2160-2166), which the function's docstring contract ("non-blocking… safe to call directly from the asyncio event loop") forbids — so item 6 is derived, not decoration. Grepped get_process_start_id callers: 6 modules, all None-tolerant by contract, zero code changes needed.

[FIRST-PRINCIPLES-REVIEWED] 977f361

[DeryFerd]

Heads-up for anyone reading the red checks: the failures in Backend Tests (3.12, 3) and Backend Tests (Windows) (3) are not from this PR. They are the test_push_branch_gate.py::TestUnrecognisedOptionsReadProtectively and test_security.py::TestGitPublishSubshellGluing tests, and they fail identically on a clean checkout of current main (verified at 6d1b51704: 11 failed / 1952 passed in those two files, with no PR code present). It looks like the push-guard changes in #7808 / #8491 landed after this branch was cut, and the fork CI runs the live merge ref. The Coverage Gate failure is just the fail-closed consequence of those shards. Happy to rebase once the floor tests are settled on main.

[bolichen97]

Rebased onto main 02d7a2d06 by a maintainer as part of the 2026-09-08 open-PR audit. bd2f789 -> f45ce75.

Clean rebase: no conflicts. The single commit replayed unchanged and the diff is byte-identical in scope (9 files, +223/-29), so the Windows arm on get_process_start_id, the want_exit_time kwarg, and the doc/spec updates are all preserved.

Gates run locally on the changed files only: black (clean; test/test_platform_compat_coverage.py is in .github/black-baseline.txt and reports the same 28 pre-existing lines on main, none from this PR), isort, flake8, and pytest on test_windows_process_start_id.py, test_platform_compat_coverage.py, test_pid_lifecycle.py -> 464 passed, 4 skipped.

Please review the rebase. A maintainer push makes the maintainer the last pusher, so under the repo's last-push rule a second approver is needed. Reply if anything looks wrong.

[DeryFerd]

@bolichen97 Rebase verified on my side too: range-diff between bd2f78967 and f45ce75ee shows the patch replayed unchanged (1: bd2f78967 = 1: f45ce75ee), and the base 02d7a2d06 is on main. Nothing looks wrong — thanks for the clean rebase and for running the gates locally.

Two notes on this round's red checks: Backend Lint & Type Check flagged a comment-history offender in the new test file (an issue number inside a module docstring) — fixed and pushed as eba209f15. Backend Tests (Windows) (4) fails on test_work_ledger.py::test_two_conductors_binding_one_worker_at_once_yield_exactly_one_binding, which looks like a Windows timing flake independent of this PR: on a clean checkout of 02d7a2d06 with no PR code present, two consecutive runs gave one pass and one failure (three threads race a binding; one lands on a path-traversal refusal instead of the clean already-bound answer).

[bolichen97]

Requesting changes at eba209f — the Windows arm is correct, but the PR leaves the codebase saying two contradictory things about it.

1. Spec contradicts the code. docs/system-specs/modules/instances.md ~L1074-1077 still documents get_process_start_id as Linux/macOS-only with None on Windows. AGENTS.md requires the spec change in the same PR. The body says "three specs documented the missing arm" — instances.md is a fourth and is unchanged.
2. Stale "Windows → None" comments survive in instances/run_marker.py ~L127, session_pid_sig.py ~L342/377, mcp_gateway/claim.py ~L77, mcp_gateway/gatewayd.py ~L1926, session_pid.py ~L170. A reader of run_marker.py will believe Windows still depends on the process_start_time fallback leg and may remove or reorder it. run_marker.py ~L141 and gatewayd.py ~L2615 are also consumers the body's "four consumers" omits.
3. Missing guard. The new want_exit_time=False branch of _windows_process_handle_identity omits the creation_value <= 0 → None guard the default branch keeps, so a degenerate GetProcessTimes result yields "0" where process_start_time yields None. Two "0" tokens then compare EQUAL ("same process") instead of "unknown", which breaks the body's own get_process_start_id == process_start_time invariant. One-line fix.

What is good: the core arm opens QUERY-LIMITED, reads the creation FILETIME, closes in finally; identity is stable per process and distinct across PID reuse; POSIX untouched; test_windows_process_start_id.py gates on real IS_WINDOWS and the coverage test mocks the seams deterministically. Also note the diff re-signatures _windows_process_handle_identity (adds want_exit_time), which "What changed" does not list.

[DeryFerd]

@bolichen97 The two failing shards trace to test_workflows_app.py::test_main_boots_platform_before_binding_server — a test introduced by #9066, which landed on main after this branch was cut. It fails on a clean checkout of current main (f60256038) with no PR code present: on Windows the xdist workers crash mid-test (three attempts, three crashes), and the POSIX shard hit the 120s timeout. The Coverage Gate failure is just the fail-closed consequence. Everything else on this PR is green, including the comment-history gate and the review lanes.