fix(session-pid): stop a recycled pid from preserving a stale session mapping

[leozhad]

Problem / Motivation

cleanup_orphaned_sessions() prunes a session_pid_<pid>.txt mapping only when platform_compat.pid_exists() is false — and on POSIX that is literally os.kill(pid, 0).

Linux draws thread ids from the same space as process ids, exposes /proc/<tid> for them, and permits signalling a tid. So once a dead session's pid is recycled as a thread of an unrelated live process, the mapping passes the check and survives indefinitely. ps -p <tid> reports nothing (it lists thread-group leaders), which makes the state easy to miss when looking by hand.

Observed on a host whose pid counter had wrapped — /proc/sys/kernel/pid_max is 4194304 and the highest live pid was 4193346, so it recycles in hours: 233 mappings on disk, one of them already resolving to a 6-day-dead Slack session through a thread of an unrelated process.

Why it matters

The stale file is not inert while it lingers.

What that costs depends on the mapping. One that records a start token is already safe to resolve: session_pid_sig._pid_recycled() compares the live start token and refuses on a mismatch on both the strict and the lenient path, and a tid's live start token cannot match the dead process's. A legacy token-less mapping has no recorded token for that guard to compare, so peer_resolve keeps answering for whatever process now holds that pid with the previous owner's session key.

That residue is same-uid only — an unrelated local process cannot read the signing key without already having the same access — so it is a robustness problem rather than a privilege boundary. The root-cause guard for token-bearing mappings (binding the mapping to the process incarnation, #8343) is already present in the base this branch targets; what is left is the legacy token-less form, plus the fact that nothing retires any of these files until the next gateway restart.

Two things made this worth fixing at the predicate rather than papering over:

• the adjacent _pid_gone_or_unmanaged() deliberately retains on an inconclusive answer, because untracking a live survivor would orphan it permanently. That reasoning is about the kill-tracking file and is correct there; it does not apply to an identity breadcrumb, whose only cost of deletion is one re-publish.
• the module already guards recycling elsewhere (_is_managed_agent_process), so the sweep is not naive — this one pass just predates the guard.

What changed

Adds platform_compat.live_thread_group_leaders(): a single os.listdir("/proc") for the whole sweep. That top-level listing enumerates ONLY thread-group leaders — a non-leader tid is absent from it even though /proc/<tid> stays directly openable, which is exactly why the cheaper per-pid probes cannot tell the two apart. One directory read replaces a synchronous /proc/<pid>/status read per mapping, so the sweep performs no per-entry file I/O at all; measured on Linux that is 1.5 ms once against 7.8 ms across 233 mappings. It is also cheaper than process_matches(), which shells out to ps on macOS and so cannot be used per entry in a sweep over hundreds of files.

cleanup_orphaned_sessions()'s stale-mapping pass now prunes when the pid is gone or is provably only a thread. That pass is extracted as _prune_stale_session_pid_files(narrow_with_leaders=) so the narrowing can be asked for independently of the rest of the sweep — which is what lets the boot path skip it (below).

The helper fails open — it returns None, never an empty set, on non-Linux, on an unreadable /proc, and on a listing carrying no pids, and the caller treats None as retain-everything. An inconclusive answer can therefore never be the thing that decides a pid is stale; it only ever narrows an existing liveness check. The leaders snapshot is taken after globbing the mapping files, so a pid starting in that window is retained and one exiting in it is pruned. The pruning predicate is Linux-only, so macOS and Windows keep the pre-existing pid_exists-only outcome. The asyncio.to_thread move below is not so scoped: both orchestrator sweeps now run on a worker thread on every platform, so cleanup_orphaned_sessions takes the pid-file lock and signals tracked pids from a worker thread there too. That widens where the existing work runs rather than changing what it does — cleanup_orphaned_sessions calls nothing bound to the loop or the main thread (no signal.signal, no loop access, no asyncio use anywhere in the function).

The narrowing is asked for on ONE call site, and the sweep runs only where it already ran. The gateway's boot path and its force-exit handler both pass narrow_with_leaders=False, so each does exactly the work it did before this branch: no-new-work-on-gateway-boot-path names orphan sweeps specifically and the leaders snapshot is a /proc read that path may not carry, and a signal handler that reaches for extra work before its os._exit is a handler that may not get there. The graceful-shutdown sweep asks for the narrowing.

That placement is the point rather than a compromise. Nothing is spawning a session by then, so the pass is not racing a mapping publisher, and it holds exactly the position the sweep already held — so this change adds no concurrency main did not already have, and needs no new lock, no new lock file in an agent-writable directory, and no work on a path that has to stay short. The cost is that a recycled-pid mapping is retired at shutdown rather than mid-run: bounding accumulation across restarts is what the sweep is for, and resolution of a token-bearing mapping is already guarded by the recorded start token independently of it. Owned plainly: a gateway that is hard-killed never reaches the graceful path, so on that host its recycled-pid mappings wait for a later clean exit. The pass is _prune_stale_session_pid_files — private, since it has one production consumer.

Snapshot staleness is still handled, because the guarantee should not rest on the call site alone. The leaders set is read once for the pass, so a pid recycled after that read would be absent from it while naming a live process. Absence from the snapshot therefore selects a candidate only, and the decision takes a reading for that one pid: new platform_compat.is_thread_group_leader() reads Tgid from /proc/<pid>/status, and a mapping is unlinked only on a definite "not a process" — retained on True and on every inconclusive answer. That read is paid only for candidates, so the common path still performs no per-entry file I/O and the host-wide listing keeps doing the cheap filtering it was added for.

The startup and graceful-shutdown sweeps run inside the orchestrator coroutine, so each is offloaded with an inline await asyncio.to_thread(...) — this repo's prevailing idiom rather than a new wrapper, so no public surface is added. That also takes the pre-existing glob, probe and unlink work off the loop, so those paths are strictly less blocking than before this branch. The force-exit signal handler keeps the synchronous call: a handler cannot await, and the process calls os._exit immediately afterwards, so loop latency is not meaningful there.
The mapping files are materialized before the snapshot is read, and that ordering is load-bearing. A mapping file is written at spawn, so every path in the list belongs to a process that already existed when the snapshot was taken, is therefore present in it, and is retained. Iterating the glob lazily instead would let an entry yielded after the snapshot belong to a pid absent from it — pruning a live session's mapping. The list is bounded by the same mapping count the sweep was already reading per-entry (233 on the host measured above), and it replaces 233 synchronous file reads with one directory read.

Also deliberately deferred: _skip_tagged in the same file still asks only pid_exists of the owning gateway pid, so a gateway pid recycled as a tid keeps its orphans from being reaped. That is the same class this change fixes one pass later and the leaders set is already in hand, but narrowing a kill-safety predicate changes what gets SIGKILLed rather than what gets unlinked, so it belongs in its own change with its own tests.

It does not prune full-process recycling (a pid reused by an unrelated process, not thread). Resolution of a token-bearing mapping already refuses that case via the recorded start token, so it is not a correctness gap here; pruning such files proactively through that same token is a reasonable follow-up and is deliberately left out of this change to keep it to one behaviour.

Tests

TestLiveThreadGroupLeaders in test/test_platform_compat.py — leader is True; a real live thread's native_id is False while pid_exists() on that same tid is True (the exact split the old predicate could not make); unknown pid fails open; non-Linux fails open.

test_pid_file_recycled_as_a_thread_is_deleted in test/test_pid_lifecycle.py — drives cleanup_orphaned_sessions() with two real mappings, one keyed on a live thread's tid and one on the process's own pid, and asserts the thread's mapping is pruned while the leader's is retained. Deliberately does not patch os.kill: both pids are genuinely signalable, which is the condition that reproduces the bug.

test_boot_setting_reads_no_proc and test_prune_pass_leaves_the_shared_pid_file_alone in test/test_pid_lifecycle.py — the first replaces the leaders helper with one that raises and drives the False setting, so a regression that narrows on the boot or force-exit path fails the suite instead of silently costing a /proc scan where it must not. The second pins that the pass touches mapping files and nothing else, leaving the shared kiro_session_pids.txt byte-equal.

TestIsThreadGroupLeader in test/test_platform_compat.py and test_stale_snapshot_does_not_delete_a_live_mapping in test/test_pid_lifecycle.py — the per-pid re-read on this process, on a real live thread's tid (which pid_exists reports alive, the exact trap), on non-Linux, on a vanished pid and on a malformed status; plus a regression test that hands the pass a snapshot taken before this process existed and asserts the live mapping survives.

Every new gate was verified by inversion, not just by passing: removing the per-pid revalidation fails the stale-snapshot test, and flipping the setting to the narrowed form fails the no-/proc test.

The thread-based tests use a real thread rather than a synthetic /proc, so they rest on kernel behaviour rather than on a fixture's model of it.

Verified:

• test_platform_compat.py + test_pid_lifecycle.py → 461 passed, 23 skipped on this branch against current main (base is 447 passed / 23 skipped; this branch adds 14 tests there and removes none), plus 500 passed across the publish-path, gateway, spawn-offload and sweep-helper suites
• negative control: with the one-line predicate change reverted, the new pruner test fails with AssertionError: a pid that is only a thread must be pruned
• isort, flake8 and mypy src/kiro_crew/ clean (mypy: no issues in 1344 source files); the six baselined gates in Backend Lint & Type Check all pass on the 5 changed files

That is the whole test surface — fourteen tests in five groups: five covering the host-wide helper's branches (TestLiveThreadGroupLeaders), five covering the per-pid re-read (TestIsThreadGroupLeader), one behaviour test proving a real recycled thread tid does not preserve a mapping, two pinning what the False setting must not do, and one pinning that a stale snapshot does not cost a live mapping.

Note on black: test/test_pid_lifecycle.py does not satisfy black --check at HEAD either (the flagged hunks are at lines 528+, unrelated to this change at line 371), so this PR deliberately leaves that pre-existing formatting alone rather than mixing a reformat into the diff.

Manual verification

Verified against a live host whose pid counter had wrapped: enumerated every mapping, classified each pid as gone / thread-of-another-process / live thread-group leader, and confirmed exactly one mapping was being preserved by a recycled thread id. After the change that class is pruned and live leaders are retained. The automated tests reproduce the same condition with a real thread, so no manual step is required to review this change.

Pattern harvest

Rule candidate: semgrep
Pattern: os.kill(pid, 0) - or a pid_exists() wrapper - used as the sole liveness test for a previously recorded pid

The general lesson is that pid identity is not a liveness signal once a counter can wrap: os.kill(pid, 0) answers a narrower question than callers usually mean. Anywhere a recorded pid is later compared against a live one, the comparison wants a second dimension (thread-group leadership at minimum, start time ideally).

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 6af13ff33f61d94b6ec1f4d897d67afbd1ce29a3 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.

First-Principles-Verdict: CONCERNS

The recorded start token already discriminates every recycled pid — the two new /proc helpers re-derive a weaker answer that catches only the thread subcase.

Not justified as shipped

• Item 1 — symptom-level: the cause is "the predicate checks pid-number liveness, not identity." A pid recycled as an unrelated process passes pid_exists + the leaders set, so the same two harms the description names (legacy token-less resolution, accumulation) survive on that sibling — 1 unfixed case, read straight off the new predicate. The existing session_pid_sig._pid_recycled (session_pid_sig.py:332, over /proc/<pid>/stat, which a tid also has) catches both cases wherever a token was recorded; the description itself concedes "a tid's live start token cannot match the dead process's". The macOS-cost argument against per-entry identity checks is moot — the shipped predicate is Linux-only anyway.
• Item 2 — oversized: the host-wide snapshot's only named benefit is 1.5 ms vs 7.8 ms on a shutdown worker thread, and its staleness is what forces item 3 plus both "load-bearing" orderings.
• Item 3 — oversized: exists solely to patch item 2's snapshot staleness; alone it would suffice (1 consumer each, grepped: _prune_stale_session_pid_files only).

What this change ships

Inventory (7 items) — 3 justified

Intent: stop a dead session's pid, recycled as a thread of an unrelated process, from keeping its session_pid mapping alive forever — a FIX (provenance: added test fails on base via a real tid).

1. Mappings whose pid now names only a thread are deleted at graceful shutdown — symptom-level (process-recycled sibling unfixed; recorded-token compare covers both)
2. New platform_compat.live_thread_group_leaders() snapshot — oversized (saves ~6 ms off-loop; forces item 3)
3. New platform_compat.is_thread_group_leader() per-pid re-read — oversized (patches item 2's staleness; 1 consumer)
4. Boot and force-exit sweeps opt out via narrow_with_leaders=False — justified
5. Boot and shutdown sweeps now run on a worker thread on every platform — rides along
6. Stale-mapping pass extracted as private _prune_stale_session_pid_files — justified
7. platform-compat and session specs updated in the same commit — justified

Watch

• The process-recycled sibling: on Linux a token-less mapping is inherently stale (publish_session_pid degrades to single-line only on "Windows, probe failure"), and every live session re-publishes each turn (messaging/identity.py:36) — so the whole named harm is removable without either new helper. The truncated description ends "Also deliberately defer", so this may be owned there; the visible text never weighs the token-compare alternative.
  Clears when: the sweep prunes on _pid_recycled mismatch (token-less on Linux pruned outright), or the description's deferred-scope text explicitly owns the process-recycled case.

Subtractions

• Replace both new helpers: read the mapping body with session_pid_sig._parse_mapping_body and prune on session_pid_sig._pid_recycled(pid, token); on Linux prune token-less bodies outright (cost: one re-publish next turn). Deletes both helpers, the snapshot, and both ordering constraints.
• Failing that, drop live_thread_group_leaders() and the snapshot; call is_thread_group_leader(pid) per surviving pid — 7.8 ms across 233 mappings by the PR's own measurement, on a worker thread at shutdown.

[FIRST-PRINCIPLES-REVIEWED] 6af13ff

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

No findings.
[GPT-REVIEWED] 6af13ff

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

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

The claims check out against the base tree: the three gateway call sites, the no-new-work-on-gateway-boot-path rule (clause 4 explicitly names orphan sweeps), the existing _pid_recycled start-token guard (#8343), and the asyncio.to_thread idiom already used two hunks earlier on the same boot path. The fail-open (None, never False) contract, the snapshot-after-glob ordering, the per-pid revalidation before unlink, and the Linux-only scoping are all sound and pinned by inversion-verified tests; specs are updated in the same commit.

Design-Verdict: PASS

Fail-open narrowing at the right predicate, with the stale-snapshot race closed function-level rather than by call-site luck — sound and proportionate.

Suggestions

• Hosts that only ever hard-kill the gateway never reach the graceful-shutdown sweep, so the fix never runs there; since the per-pid re-read already makes the pass safe beside live publishers (your own stale-snapshot test pins this), a post-ready tracked background task — the exact _backfill_unclean_session_telemetry idiom two hunks below the boot call — would deliver the pruning on every launch at zero readiness cost. Fine as a follow-up.

[DESIGN-REVIEWED] 6af13ff

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 6af13ff

[leozhad]

Thanks — the premise checks out, so I measured the cost before acting on the prescribed revert.

Where the call actually runs. pid_is_thread_group_leader is reached from exactly one place in the tree: cleanup_orphaned_sessions() (session_pid.py:766). That function has three call sites, all inside GatewayOrchestrator.run():

• gateway.py:10185 — startup, before any session is created
• gateway.py:10423 — inside _on_signal, on the second signal (the force-exit path), immediately before the hard exit
• gateway.py:10644 — after await self._shutdown() returns and "Goodbye!" is printed, immediately before the hard exit

_periodic_pid_sweep does not call it, so no recurring on-loop task gained work. session.py documents this function as "startup + shutdown only".

Measured cost (Linux, median of 5 × 20k calls per function):

	µs/call
pid_exists (pre-existing)	2.1
pid_is_thread_group_leader (added)	33.5

That is +31 µs per live mapping, or +7.3 ms at the 233-mapping count cited in the code comment. Dead pids pay nothing — the condition is not pid_exists(pid) or not pid_is_thread_group_leader(pid), so the probe is short-circuited for exactly the entries that get pruned.

Baseline. The loop this joins is already synchronous I/O on the same thread: a directory glob to drive it, a pid_exists per entry, and two unlinks per pruned entry. So the change makes an existing sync-I/O loop ~7 ms slower once at startup and once at exit rather than introducing blocking I/O to an otherwise async path.

On the prescribed remedy. Moving the cleanup off-loop is not a drop-in here: one of the three call sites is the _on_signal handler, which is sync by necessity and cannot await — so that is a change to the shutdown path rather than a tweak to this diff. I also measured the cheaper single-read() variant of the probe (one read() of status, parse the buffer): only 1.1x faster, so no implementation tweak removes the objection.

Two follow-ups I am happy to do if a maintainer prefers one: (a) gate the probe behind an entry-count threshold, or (b) land the off-loop cleanup as its own PR and rebase this on top. I will also revert on request — flagging first because the revert reinstates the misattribution this PR fixes, where a lingering mapping makes peer_resolve answer for a recycled pid with the previous owner's session key.

[leozhad]

Offloaded rather than reverted, so the fix stays in.

764fb9e41 adds cleanup_orphaned_sessions_async(), which runs the whole sweep through asyncio.to_thread, and switches the two call sites that sit inside the orchestrator coroutine — startup and post-shutdown — to await it. The /proc listing no longer runs on the loop.

The force-exit signal handler keeps the synchronous form: a signal handler cannot await, and the process calls os._exit immediately after it returns, so loop latency is not meaningful on that path.

I moved the whole function rather than just the listing on purpose. The snapshot has to happen after the glob — a process that starts between the two must be retained, not pruned — so hoisting a pre-computed snapshot into the caller would invert that and drop a live session's mapping. Offloading the whole sweep also takes the pre-existing glob, per-entry probe and unlinks off the loop, so this path is now strictly less blocking than it was before this branch.

Tests: test_platform_compat.py + test_pid_lifecycle.py = 435 passed / 22 skipped (433 before, +2 new). The two additions pin the offload itself — one asserts the sweep runs on a non-loop thread, the other blocks the sweep and asserts a concurrent coroutine is still scheduled. Both fail if the to_thread call is removed. flake8 clean.

[leozhad]

Both findings are addressed in e611ab237, along with the premise review's subtraction.

BLOCKING — /proc walk on the force-exit path. Correct, and the offload used at the other two call sites isn't available here: _on_signal is a signal-handler callback, so it can't await. Rather than revert the snapshot, the sweep now takes a keyword-only narrow_thread_leaders and force-exit passes False, so that path performs no /proc listing at all. None was already the snapshot's fail-open value, so skipping it degrades force-exit to exactly the pre-existing pid_exists check — never more aggressive, only less precise — and the next startup sweep prunes whatever it left behind.

FINDING — top-level-imports. Worth noting for the record: that in-method import isn't added by this PR — it's at the same line in the base tree, and the diff had only widened it from one name to two. Dropping the async wrapper returns the line to its original single-name form, byte-identical to base.

Premise review's subtraction, applied as written. cleanup_orphaned_sessions_async and its session.py re-export are gone; both coroutine call sites now use inline await asyncio.to_thread(cleanup_orphaned_sessions), the prevailing idiom here. The snapshot-ordering rationale stays inside cleanup_orphaned_sessions. session.py now carries no change at all, so the undeclared public surface both design reviews flagged no longer exists — the PR is 5 changed files, not 6.

The two wrapper tests are replaced by three for the bypass, including an exact mirror of the thread-recycle prune test: same live-thread setup, flag flipped, mapping retained instead of pruned. I checked those in both directions rather than only that they pass — with the flag ignored, test_bypass_performs_no_proc_listing and test_bypass_retains_a_thread_recycled_mapping both fail.

436 passed / 22 skipped; flake8 clean on all 5 changed files. The description is updated too — the offload and the bypass are now stated in What changed instead of riding along implicitly, and three stale lines in Tests are corrected (the test count, the changed-file count, and a class name that no longer existed).

[leozhad]

Rebased onto current main to clear a merge conflict.

The conflict appeared when #8906 merged: it appends TestPublishDirNoreplace to the end of test/test_platform_compat.py, and this branch appends TestLiveThreadGroupLeaders to the same place. Both sides add an unrelated test class at the file tail, so the resolution is simply to keep both.

Verification:

• The contribution is unchanged: 193 insertions / 5 deletions across the same 5 files, and the added lines are byte-identical to the previous head. Only their offset moved.
• test/test_platform_compat.py test/test_pid_lifecycle.py -> 450 passed, 23 skipped, no failures. The count is higher than before because main has since added tests to that file; the six added by this PR are unchanged.
• flake8 clean on all five changed files. Worth noting: deleting the conflict markers alone leaves the two classes with no separator and trips E302, so the two blank lines between them are required, not cosmetic.
• git merge-tree against the newest main exits 0, so this resolution still applies after the further commits that landed while it was being prepared.

New head: 7d5a1306b9e3698ac1fbf66d84f7a80d274a3346. No review findings were addressed in this push and no source behaviour changed.

[leozhad]

Thanks — the First Principles block is correct, and I've taken the subtraction.

The premise checks out against the tree this branch targets: session_pid_sig.py already records the process start token as a MAC-covered second line, _pid_recycled() compares it, and both read_session_pid_txt() and verify_session_pid() refuse on a mismatch. A tid's live start token cannot match the dead process's, so for a token-bearing mapping peer_resolve never answers with the previous owner's key. The claim that the sidecar binds "not the process's identity" was wrong, and I had written it into the prune-site comment, the test docstring, the commit message and the description.

Pushed as b800dd4c41cf2579deae1b0e15db88d6bd8f26c1 (amended, still one commit):

• Prune-site comment in session_pid.py — dropped the misattribution rationale; it now names the existing start-token guard and scopes this sweep's value to what it actually fixes: legacy token-less mappings, where the guard has no recorded token to compare (_parse_mapping_body returns None for that form and both call sites are gated on token is not None), plus bounding accumulation.
• test_pid_file_recycled_as_a_thread_is_deleted docstring — same correction.
• Commit message — replaced the same paragraph.
• Description — corrected the central claim, removed the "would be a file-format change" premise (the two-line token format already exists), and stopped describing The pid to session mapping and its HMAC bind only the pid number, so a recycled pid resolves to the previous owner's session key until the next restart #8343 as future work, since its guard is present in the base.

No behaviour change in this push: comments, a docstring and prose only. test_platform_compat.py + test_pid_lifecycle.py → 450 passed, 23 skipped (base 444, this branch adds 6 tests and removes none); flake8 clean on all five files. I also corrected a stale test count in the description while there — it read 433/22 from before a rebase moved the base forward.

On the second subtraction — pruning token-bearing mappings via the recorded token, which would also catch full-process recycling — that's a fair point and I've stopped claiming it's blocked by a format change. I've left it out of this push deliberately rather than quietly: it changes what the sweep does, where everything above is text, and I'd rather not fold a behaviour change into a correction. The description now says that plainly. Happy to do it here if you'd prefer it in one go.

[bolichen97]

Requesting changes at 5de0061 — the Tgid-based fix is sound; the test and the docs around it are not.

1. Host-dependent test. test/test_pid_lifecycle.py test_prune_pass_leaves_the_shared_pid_file_alone (~L449-470) depends on pid 99999 being dead on the host, with no probe pin — a direct violation of docs/system-specs/common/testing-conventions.md ("the host is an input"; pid_max is 4194304, so 99999 is a live pid on a long-running runner). The sibling tests at ~L319-347 patch os.kill; this one must too.
2. Spec not updated. docs/system-specs/modules/session.md ~L1453 ("Stale cleanup … for dead pids") and the platform-compat.md helper table do not mention the new prune predicate or the two new helpers at platform_compat.py ~L2814/2855. Same-commit rule per AGENTS.md.
3. Body drift. "Behaviour on macOS and Windows is unchanged" — the diff moves BOTH sweeps to asyncio.to_thread on every platform (cleanup_orphaned_sessions takes the pid-file lock and kills tracked pids from a worker thread now). No thread-affinity problem found, but it is a platform-wide change the body understates. The body also names prune_stale_session_pid_files(narrow_with_leaders=); only the private _prune_stale_session_pid_files exists.
4. Undisclosed edits in slack/gateway.py ~L11249 and ~L11466-11468 (comment rewrites dropping a #7518 reference). Those hunks also do not line up with current main here; please confirm the rebase state.

Good: session_pid.py ~L857-866 prunes only on pid_exists ∧ not-in-snapshot ∧ is_thread_group_leader(pid) is False, both helpers fail open (None), non-Linux unchanged, boot path passes narrow_with_leaders=False per AUTOSDE.yaml. Disclosed and defensible that this is Tgid-based rather than start-id-based.

[leozhad]

Thanks — all four are addressed at 033f0c90f, and the rebase question turned out to explain two of them.

Rebase state (your point 4). The branch was 44 commits behind main when you reviewed it. It is now rebased onto 67716d722, behind_by 0, still one commit — 7 files, +475/−28.

The two slack/gateway.py hunks are gone entirely, reverted rather than re-based. git diff origin/main -- src/kiro_crew/slack/gateway.py is now functional-only: the three call-site changes and the comments explaining them, with nothing pre-existing touched. The #7518 provenance reference is back verbatim at gateway.py:11466, and so is no longer has to finish on the telemetry comment.

Owning what those were, since "undisclosed" is the right word for them: both were rewrites of comments I did not author, made to keep slack/gateway.py under its scripts/check_comment_history.py baseline entry. Editing someone else's comments to satisfy a gate was the wrong remedy regardless of the arithmetic — and the arithmetic no longer holds either. Measured on this head:

	slack/gateway.py
baseline entry	67
pristine origin/main scans	67
this branch's HEAD scans	67

So there is no inherited overage on current main to work around — that drift has been closed upstream since the reword was written. The gate passes with both reverts in place (comment-history gate passed, 7 files in scope).

1. Host-dependent test. Pinned, following the siblings you pointed at. test_pid_lifecycle.py:470 now wraps the call in patch("os.kill", side_effect=ProcessLookupError), so platform_compat.pid_exists(99999) resolves False by construction and the host's pid table stops being an input. Verified by inversion rather than only by passing: with the pin removed and the host faked as one where 99999 is a live thread-group leader, assert removed == 1 fails — the load-dependent runner flake you described, reproduced deliberately.

2. Specs, same commit. Both updated:

• docs/system-specs/modules/session.md:1469 — the Stale cleanup bullet now states the predicate instead of "for dead pids": a mapping goes when the pid is unsignalable, or when it is absent from one live_thread_group_leaders() snapshot and a per-pid is_thread_group_leader(pid) re-read returns False. It also records that both helpers answer None when the question is unknowable and that None never licenses a removal, the glob-then-snapshot ordering, absence-selects-a-candidate-not-an-outcome, and the narrow_with_leaders split across the three call sites.
• docs/system-specs/common/platform-compat.md:34 — a table row covering both new helpers, with the pid_exists-alone trap in the NOT column and the None-means-retain contract.

3. Body drift. Both corrected. The platform sentence now separates the two scopes rather than collapsing them: the pruning predicate is Linux-only, while the asyncio.to_thread move is not — both orchestrator sweeps now run on a worker thread on every platform. It also says why that is safe instead of asserting it, which I should have done the first time: cleanup_orphaned_sessions calls nothing bound to the loop or the main thread — no signal.signal, no loop access, no asyncio use anywhere in the function. The symbol now reads _prune_stale_session_pid_files in both places. The commit message carried the same two claims and is corrected there too, along with a stale mypy file count.

Local gates on this head: isort and flake8 clean, mypy src/kiro_crew/ clean over 1378 source files, scripts/docs-lint.sh all checks passed, scripts/check_comment_history.py passed, and 461 passed / 23 skipped across the two touched test files against 447 / 23 on main.