fix: preserve process lock exclusion across publication and reclaim

[thymikee]

Summary

A paused publisher or reclaimer could lose process-lock exclusion through age-based recovery. Hardened users now serialize publication, reclaim and release with one non-expiring guard and one claim check. Failed publication rolls back its own empty directory; partial release preserves owner evidence.

Adds nonblocking acquisition, bounded acquisition with ownership assertion, and read-only inspection. Unknown claims remain retained with recovery guidance.

The exclusive-file guard blocks fresh legacy reclaim admission. It cannot revoke an already-admitted legacy reclaimer: shared paths require draining every legacy user and preventing legacy versions returning. ADR 0030 records that support boundary and manual recovery requirement.

Removes per-test idle-fixture deletion that raced late file writes; shared run cleanup owns these directories.

Foundational slice of #3116; 12 files touched. Registration and session migration follow in dependent PRs.

Validation

Tested commit: e7856a9fc6, rebased on main at be3c8104dd.

• pnpm check:affected --run at ba51ec2c4d: passed; 5,913 tests, plus format, lint, types, layering, Fallow and build. e7856a9fc6 changes only one test and ADR 0030; the 45 lock tests and the test-size ratchet pass on it.
• Two-process release under contention, 50 runs on e7856a9fc6: process A holds the lock through withProcessLock while process B polls acquireProcessLock on the same path every 100 ms. 50 of 50 passed, with 0 "Cannot verify ownership" rejections and no lock directory or .reclaim.lock left. B acquired at most 117 ms after A released, which is one poll plus process start. The same run also passed 50 of 50 on the previous code on this host, so it shows that release under contention works; it does not reproduce the slow-ps race.
• New regressions fail on the previous code: release waits for a guard that another taker holds, and keeps the lock and guard in place while it waits; a contender judges a live owner without taking the guard.
• At fb23259: planted red regressions for aged-guard stealing, failed publication, partial release and fresh legacy reclaim admission.
• Real SIGSTOP publisher experiment (at fb23259) against current and pinned c237027 legacy implementations: both contenders refused; only the child published; release removed the lock.
• The already-admitted legacy reclaimer schedule stays outside the contract, as ADR 0030 records.

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.94 MB	4.95 MB	+2.3 kB
Package (unpacked)	4.94 MB	4.94 MB	+2.3 kB
Package (download)	1.48 MB	1.48 MB	+542 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	21.4 ms	20.6 ms	-0.8 ms
CLI --help	63.9 ms	60.4 ms	-3.6 ms

[cubic-dev-ai]

All reported issues were addressed across 8 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[thymikee]

I found two problems in fb23259 that should be fixed before merge. The lock suite and the SIGSTOP experiment were not run for this review.

release() makes one attempt to take the guard. If another process holds it at that moment, the outcome stays 'unverified', and release throws "Cannot verify ownership". The owner's live-pid record then stays on disk. A contender that polls acquireProcessLockAcquisition takes the guard on every 100 ms poll, and under the guard it runs up to four synchronous ps probes. So the guard is busy for much of each poll cycle. A guard that is merely held is not evidence of lost ownership, and the holder is only reading. Before this PR, release did not take the mutex, so this is a regression. When it fires, withProcessLock replaces a successful task result with COMMAND_FAILED. Other daemons and worktrees then see a live pid with a spent token and treat the lock as busy until their own timeout (30 s for the runner lease, 10 min for the xctestrun cache). The rule: the guard excludes short filesystem compare-and-mutate steps, and every taker that must finish (release and the rollback paths) waits a bounded time for it instead of reading "busy" as "unverified". How often this fires depends on host ps latency. The mechanism is clear from the code, but I have no measurement.

acquireUnderMutationGuard calls inspectProcessLock and isLiveProcessLockOwner while it holds the non-expiring guard. Both spawn ps synchronously. A waiter polling a live lock therefore holds .reclaim.lock for tens of milliseconds, and up to about 4 s under load, on every poll. If that waiter dies in the window (SIGKILL on daemon stop escalation, CI cancel, OOM, or SIGINT/SIGTERM during spawnSync), the guard file is never removed. After that, every tryAcquireProcessLock on that path returns 'busy', and inspection reports 'publishing' forever. ADR 0030 accepts a retained guard after a crash, but it assumed a window of a few syscalls, not a whole liveness probe repeated on every poll. A killed waiter can then wedge the runner lease, the xctestrun cache or the Swift helper cache until someone deletes the file by hand. The rule: no process probe or other blocking call runs while the guard is held. Judge liveness outside the guard, as the code did before. Under the guard, re-read the record, check that the token judged dead is still there, then rmdir/mkdir/publish.

Could you keep the new guard but scope it to those filesystem steps only, with release waiting a bounded time for it? That keeps the exclusion property, drops the long synchronous critical section, and keeps the ADR tradeoff where it assumes. The change stays inside process-lock.ts.

For validation, please add a test where a second process (or an openSync spy) holds the guard while the owner calls release(). It should assert that release resolves and that the waiter then acquires. The current suite runs release in one process, where the guard can never be contended. Please also run two real processes: A holds a lock through withProcessLock while B polls acquireProcessLock on the same path at the default 100 ms. A's release() must resolve without "Cannot verify ownership", and B must acquire within one poll. Repeat at least 50 times, since the race is probabilistic. The SIGSTOP experiment covers publication only and never exercises release under guard contention.

Two smaller questions. Does publishFileSync put its temp file inside the lock directory? If so, a failed rename would leave rmdir rollback with ENOTEMPTY and an ownerless directory that stays forever. Also, inspectProcessLock called under the caller's own guard reports an ownerless directory as 'publishing', because the guard file exists, so an 'unproven' attempt carries a misleading inspection.

The Smoke Tests failure looks unrelated. It is a Swift timing assertion in RunnerTests+SynthesizedTextEntryTests.swift (11 edits in 376 ms against a 400 ms minimum), in a step that never calls the TS process lock. Before merge, release() must wait for a guard that is held but not abandoned, and the ps probes must move outside the guard.

[cubic-dev-ai]

All reported issues were addressed across 4 files (changes from recent commits).

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[thymikee]

Thanks for the update. The release path now waits for the guard, and the earlier open question on a shared guard is addressed in code at ba51ec2. One piece of evidence is still missing.

The two-process run from the earlier review (#3122 (comment)) has not been reported. The Validation section still names fb23259, and the new unit test simulates the second process with a writeFileSync and a setTimeout in the same process (https://github.com/callstack/agent-device/blob/ba51ec2/packages/host-kit/src/internal/process-lock.ts#L364). That never makes a real second taker contend for the guard. So we still have no proof that release under real contention stops throwing "Cannot verify ownership" and stops leaving a live-pid record that blocks runner-lease and xctestrun-cache users. Please run this on ba51ec2 at least 50 times: A holds the lock through withProcessLock, B polls acquireProcessLock on the same path at 100 ms. Report the number of release rejections (expected 0), B's acquire latency after A's release (expected within one poll), and that no .reclaim.lock file is left at the end. Then update the Validation section to the tested head.

The three inline threads from the other review are covered in one go. The two open P2 threads still stand: the test only checks the end state (#3122 (comment)) and the ADR names only a guard timeout as unverified (#3122 (comment)). The P1 on the already-admitted legacy reclaimer does not apply, because ADR 0030 and the PR body record that boundary and the delta does not reopen it, so it can be resolved (#3122 (comment)).

I read the diff and did not run tests. I did not re-check whether the atomic publish puts its temp file inside the lock directory, which decides whether the swallowed rmdir rollback can leave an ownerless directory. I also did not read the swift-cache.test.ts change, and I did not check whether any caller bounds release() by a deadline shorter than the new 5 s guard wait.

Both Smoke Tests jobs are still queued or running at ba51ec2, so there is no failure to attribute yet. This diff touches the route they exercise, because runner-lease.ts and runner-cache.ts take this lock during iOS runner preparation. If a job fails, please check it for lock-timeout or ownerReleaseUnverified details before calling it unrelated.

Before merge, we need the two-process run on ba51ec2 with zero unverified releases and B acquiring within one poll, and the queued Smoke Tests need to finish.

[thymikee]

Updated to e7856a9, and the Validation section now names that commit.

Two-process run on e7856a9, 50 times: process A holds the lock through withProcessLock while process B polls acquireProcessLock on the same path every 100 ms. All 50 passed. There were 0 release rejections ("Cannot verify ownership"), and no lock directory or .reclaim.lock was left after any run. B acquired at most 117 ms after A released, which is one poll plus process start. The same run also passed 50 of 50 on the previous code on this Mac, where ps is fast, so it shows that release under real contention works but does not reproduce the slow-ps race.

Both P2 threads are fixed in e7856a9. The release test now checks that the lock and the other taker's guard both stay in place while release() waits, and the test fails if release deletes that guard. ADR 0030 now says the bound only covers waiting for the guard; an unreadable owner record or a directory that cannot be removed also leave a release unverified.

On the two open questions. The atomic publish does put its temp file inside the lock directory, but withAtomicPublishTempPathSync deletes it in a finally on every path, so a failed rename does not leave the directory non-empty. Only if that delete itself fails is an ownerless directory left, and inspection then reports it as unproven and keeps it, as ADR 0030 says. No production caller wraps release() in a deadline shorter than the 5 s guard wait.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-03 19:08 UTC