fix(ios): never signal a recycled runner pid from a stale lease

[thymikee]

Part of #1596.

What the #1596 evidence actually shows

Investigating the three element-17/20/29 zero-action deaths refuted the issue's headline mechanism — the daemon replace path did not kill the calling command (full evidence posted on the issue). But the audit surfaced a real adjacent defect on the exact code path the issue suspected:

The defect

cleanupLeasedRunnerProcesses kills lease.runnerPid raw — kill(-pid) + kill(pid) + pkill -P pid — from an on-disk lease file. The lease owner pid gets start-time identity verification; the runner pid gets none. A SIGKILLed daemon leaves its lease behind for days, pids get recycled (this machine's pid space observably wrapped within hours under bench load), and the next daemon's stale-lease cleanup then tree-kills whatever innocent process — including its whole process group — now holds that pid.

The fix

• RunnerLease records runnerStartTime at construction (both the fresh-spawn and adoption sites already go through buildRunnerLease).
• Before the SIGTERM/SIGKILL tree kills, the pid must be proven to still be the leased runner: recorded start-time match, or — for legacy leases without one — a runner-shaped xcodebuild … AgentDeviceRunner command line. Otherwise the tree kill is skipped and a ios_runner_lease_recycled_pid_skipped warn diagnostic is emitted.
• The pattern-based xcodebuild pkill -f runs unconditionally as before — genuinely stray runner processes are still collected; only the unverifiable raw-pid signal is withheld.
• Adoption (tryAdoptRunnerSessionFromLease) skips a recycled pid too: the adopted session's later disposal would signal it.

Tests

• 4 new stale-lease cleanup tests through prepareRunnerLeaseForStartup with a recording adapter: recycled pid skipped, matching pid killed, legacy lease trusts only runner-shaped commands, dead pid skipped — all asserting the xcodebuild pattern cleanup still runs.
• 1 new adoption test: recycled pid is not adopted.
• vitest (83/83 in touched files), typecheck, lint green.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.97 MB	1.97 MB	+619 B
JS gzip	631.7 kB	631.8 kB	+189 B
npm tarball	761.6 kB	761.8 kB	+186 B
npm unpacked	2.67 MB	2.67 MB	+619 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.5 ms	27.1 ms	-0.4 ms
CLI --help	65.1 ms	64.9 ms	-0.2 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/runner-client.js	+103 B	+21 B

[thymikee]

Not ready — two P1 process-safety gaps remain:

1. cleanupLeasedRunnerProcesses verifies runnerPid once, then reuses it for SIGTERM and SIGKILL with an awaited pattern cleanup in between. If TERM ends the original runner and the PID is recycled before KILL, the second raw tree kill can still hit an innocent process. Re-verify identity immediately before every raw signal; add a regression where start time changes after TERM and KILL receives no PID.

2. Adoption only checks identity when runnerStartTime exists. A legacy lease with no timestamp can adopt a recycled live PID if the leased port still answers, and later disposal will signal that unrelated PID. Apply the same fail-closed/runner-shaped-command fallback used by cleanup and cover foreign legacy PID + healthy probe => no adoption.

The iOS Smoke check is also red (wait capture stalled with zero readable captures). It may be an unrelated infrastructure/capture flake, but it is a confirmed failing gate. Other static, integration, Android/macOS, and Swift checks are green. Part of #1596 is correctly scoped.

[thymikee]

Both P1s addressed in 3e128b2:

P1 — verify-once across TERM→KILL: cleanupLeasedRunnerProcesses now resolves a freshly verified pid immediately before each signal instead of reusing one resolution. The realistic window was exactly the one flagged: the runner usually dies on the SIGTERM, and the awaited xcodebuild pattern sweep between the signals gives the kernel plenty of time to recycle the pid before the SIGKILL escalation. After the change, the second resolve returns undefined for a dead pid and refuses (with the recycled-pid diagnostic) a recycled live one. The residual verify→signal race is now the microsecond readProcessStartTime→kill(2) window, which is as tight as it gets without pidfd-style primitives (macOS has none). Regression test: identity flips between the two signals → SIGTERM lands on the verified pid, SIGKILL gets undefined.

P1 — legacy-lease adoption bypass: the start-time check in tryAdoptRunnerSessionFromLease was lease.runnerStartTime && …, so a legacy lease skipped verification entirely — and adoption then re-stamped the lease with the live pid's start time, laundering a recycled pid into a strongly-verified lease that disposal would later confidently kill. Adoption now shares the disposal path's verification helper (verifyLeaseRunnerPidIdentity), including the runner-shaped command-line fallback for legacy leases. Two regression tests: non-runner-shaped live pid on a legacy lease is refused before the port probe; runner-shaped one still adopts.

iOS Smoke: agreed it reads as the known wait-capture flake; this push re-rolls it — will confirm it greens before calling this ready.

[thymikee]

The two prior P1s are fixed and their regressions are meaningful, but one P1 race remains in adoption. PID identity is verified, then probeRunnerAnswersUptime is awaited before buildAdoptedRunnerSession/buildRunnerLease re-stamps the PID. The original xcodebuild can exit and its PID can be recycled during that probe while the old port still answers, causing the recycled PID to be adopted and later tree-killed. Re-verify the same lease/PID identity immediately after the probe and before constructing or writing the adopted session. Add a regression where identity matches initially, changes while the successful probe is in flight, and adoption returns null.

No ready label yet. The PR is conflict-free and all completed checks are green; iOS/Android Smoke, Coverage, and FreeRange are still running.

[thymikee]

Fixed in 2ef6c0c: adoption now re-verifies isProcessAlive + verifyLeaseRunnerPidIdentity immediately after the awaited uptime probe, before buildAdoptedRunnerSession/writeRunnerLease — and everything from that check to the lease write is synchronous, so the probe was the last await that could hide a recycle. Regression test added: identity matches at the first check, flips inside the (successful) probe, adoption returns null and the stale lease on disk survives untouched (ownership never transferred).

[thymikee]

Re-reviewed exact head 2ef6c0c7d. The post-probe PID identity recheck closes the remaining adoption race: it runs after the final await and before synchronous session/lease construction. The new regression is non-vacuous and proves a PID recycled during a successful probe is refused without transferring lease ownership. The earlier per-signal cleanup and legacy-adoption fixes remain intact. No remaining code finding; mergeable and conflict-free. Completed checks are green, with iOS/Android Smoke, Coverage, and FreeRange still running.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-05 16:38 UTC