fix(host-kit): process lock proves ownership before release and never evicts a live owner

[thymikee]

Summary

acquireProcessLock released its lock directory unconditionally, so a contender that reclaimed the lock
while the previous holder was away had that holder delete the exclusion belonging to whoever holds it
now. Release now removes only while owner.json still names the acquirer and reports a typed
ownerReleaseUnverified otherwise; an unreadable record is an owner of unknown identity, not an absent
one, and no longer ages out at the grace window. A reclaim re-decides in place, and a record naming this
live process is dead once the claim inside it is spent — claims are issued per acquisition and stamped
with the loading of this module that issued them, so a second bundled copy cannot have a live claim
cleared under it.

Lock-and-run callers stop answering "which of my two failures does the caller hear" by hand: the work
inside the lock outranks a lock that could not be handed back, and work that succeeded still reports it.
That moves the Apple runner's artifact, cache, lease, session and disposal paths, the
managed-allocation and device-claim stores, atomic publishes, the Swift recording cache and the
agent-browser setup onto withProcessLock, and so changes which error those callers report.

The XCTest device-set redirect answers the same question once. One try covers the restore of an
interrupted build's leftovers, the same-set check and the rename, so the lock can never be handed back and
then worked under; its catch hands the set back and throws. handBackDeviceSet owns that hand-back —
restore the host's own XCTestDevices, then release the lock, then record whatever the release could not
verify — and it observes where the host's set actually sits while it still holds the lock, so a report
names a backup path only while the host's set really is renamed aside there. Order decides twice: the
leftovers are undone before the same-set check, which follows symlinks, so a XCTestDevices left pointing
into this simulator's own set can no longer read as "already redirected" and hand the next build the host's
devices; and every path that hands the set back leaves the acquire by a throw or a return.

Closes #2523. 32 files across host-kit, managed-allocation, capture-kit, platform-apple,
platform-web and the daemon.

Validation

Rebased on origin/main at fffa01e02c; tested at 615c58cc87. pnpm check:affected --run passed: 4674
tests in 625 files, typecheck, lint, layering and Fallow clean; the two scripts/ gates it does not select
pass locally. The redirect function had grown past Fallow's complexity threshold while absorbing these
rules, so the rename step and the error report moved into two named helpers rather than a suppression.

Live runs on this Mac, no fault injection. A wrapper-bound one: starting from what an interrupted build
leaves (the host's set renamed aside, and XCTestDevices a symlink into the simulator's scoped set),
withXcodebuildSimulatorSetRedirect ran xcodebuild build-for-testing -scheme AgentDeviceRunner; during the
build the lock was held and XCTestDevices was still that symlink into the set, the build succeeded in 20 s,
and on the way out XCTestDevices was a real directory again with the host's own file back inside it, the
backup consumed and the lock released. A daemon-routed one: open <AgentDeviceTester.app> --platform ios --ios-simulator-device-set <set> --udid <udid> resolved that scoped simulator and built the runner through
the redirect — a 1 Hz watcher recorded 122 consecutive samples of XCTestDevices -> <set> with the lock held
and the backup present, then the set back as a real directory with the lock free and the backup consumed.
That request did not reach a snapshot: an xcrun step inside it failed with Invalid device: <udid> (exit
148) at the moment the scoped set's device was removed from disk by something outside agent-device. The same
host condition explains the DEVICE_NOT_FOUND reports I posted earlier — xcrun simctl --set <dir> create
returns a UDID and the directory is empty seconds later (twice under /private/tmp, once under $HOME),
while devices and doctor resolved that very device in the same minutes. It is not filed as an issue: the
flag is plumbed through the daemon, and nothing here pins the deletion on agent-device.

Mutations, one at a time, each red only where claimed:

Mutation	Result
drop the spent-claim disjunct from the reclaim	the next acquire waits out its whole timeout
drop the issuer check from the spent-claim rule	another loading's live claim is cleared as spent
put the same-set check back before the reconcile	the leftover-symlink test alone goes red, handle null
let a failed first restore fall through to the same-set check	the retry test alone goes red: a handle, lock already gone
let a refused restore pass unraised	the refused-restore test alone goes red
release before restoring inside the hand-back	the give-back ordering test alone goes red
keep a failed restore out of the report	the install-failure test alone goes red
name a backup even while the host's own set is in place	the half-restored test alone goes red: the hint names the older leftover
a handle that stops throwing the restore failure	the two restore-precedence tests go red
let releaseBestEffort forgive an unverifiable release	the build-that-succeeded test goes red

Gaps. The wedge needs a refused unlink, so it is fault-injected: no device run yields EACCES on its own,
and the live coverage of the redirect is the CI lanes that build and launch through it plus the two runs
above. The daemon-routed run built and handed back, but no scoped-set session reached a green snapshot on
this host, so that leg is still only CI-covered. The rule that a backup is named only while XCTestDevices is absent or a symlink is pinned by the
half-restored case above: a refused first rename, the hand-back restoring the host's set and then being
refused while deleting an older leftover. The two-loading
case is pinned with a foreign issuer id, not a second real bundle. The hand-back's own warn line is not
asserted — the ordering test watches the filesystem. No win32 evidence here.

[github-actions]

Size Report

Metric	Base	Current	Diff
Installed (including dependencies)	4.59 MB	4.59 MB	+3.9 kB
Package (unpacked)	4.59 MB	4.59 MB	+3.9 kB
Package (download)	1.36 MB	1.36 MB	+1.4 kB

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	26.6 ms	25.1 ms	-1.5 ms
CLI --help	71.7 ms	71.5 ms	-0.2 ms

[thymikee]

Release now checks ownership, but at 2ec6027 a stale reclaim can still give the lock to two contenders. In clearStaleProcessLock, B reads the dead owner's record and decides to reclaim. Before B renames, A renames the stale directory aside, runs mkdir and writes its record. B's renameSync(lockDirPath, asidePath) then moves A's live directory away, and B's mkdir succeeds too. Both now hold the lock, which is what #2523 asks to prevent. After the rename, please confirm that the moved directory is the one you judged stale (for example, compare its inode with the statSync taken before the decision, or re-read the moved record), and put it back if it is not. A test that publishes a live record between the read and the rename would cover it.

Release can now reject with ownerReleaseUnverified, and one caller does not handle that. In packages/platform-apple/src/snapshot-source/host.ts, the cancel path runs void pending.then((release) => release(), () => undefined). The second handler catches only a failed acquire, so a rejected release becomes an unhandled rejection, and the daemon's unhandledRejection handler shuts the daemon down. Please catch it there, and check the callers that release in finally, because a release error now replaces their task result.

A smaller-design question: could each acquisition write a random token into owner.json? Release could compare that token instead of pid and start time, and reclaim could check the token of the directory it moved, which would also close the race above.

Coverage fails because of this change: src/daemon/__tests__/atomic-publish-ownership.test.ts lists process-lock.ts as a simple publisher and rejects fs.renameSync(. The rename is a directory reclaim, not a file publish, so please decide at that rule whether the code or the rule should change. Smoke Tests were still running, and there are no conflicts.

[thymikee]

All four points addressed. The reclaim race and the release callers in 7d4b8f3, the claim token in 9ccc2bc.

The rename race. After the rename, what arrived is now compared against what was judged: the inode and device from the statSync taken before the decision, and the record re-read from the moved directory. Anything that turns out to name a live owner, or is not the inode we judged, is renamed back rather than removed, and the contender goes back to polling. a live owner published between the stale read and the rename keeps its lock publishes a live record from inside a fs.renameSync spy, which is the window you described; the live owner is still in place at the lock path afterwards and no .reclaimed- directory survives. Removing the verification turns that test red.

Release callers. The cancel path in packages/platform-apple/src/snapshot-source/host.ts now releases with release().catch(() => undefined) on the abandoned branch, so a release that cannot prove ownership no longer reaches the daemon's unhandledRejection handler. I read that branch's contract as "the task is gone, the lock goes to the stale-clear path", which is why it is swallowed rather than logged. The abandoned-acquire case has no test harness today, so that one is reasoned rather than covered.

The finally sites were a real problem, as you expected: a release error replaced the task result. withDeviceClaimLock, withRunnerLeaseLock, setupManagedAgentBrowser, ensureSwiftExecutable and acquireXcodebuildSimulatorSetRedirect now keep the task's own failure as the reportable one, and report the unverifiable release only when the task succeeded — a lock that is still standing resolves itself through stale-clear, while nothing else recovers why the task failed. runner-lease-release-ownership.test.ts pins both directions; reverting to finally { await release() } turns the task-failure test red.

The claim token. Adopted, and it turned out to close a hole the inode check left. Each acquisition writes a random claimToken into owner.json; release matches the token instead of pid plus start time alone, and the reclaim compares the token inside the moved directory with the one it judged. The case my inode check missed: a contender reclaims, mkdirs, and has not published yet — same path, no record, and if the inode came back the same the identity check would have been satisfied and I would have deleted a lock that was never mine. A token that does not match refuses it. release leaves a lock that a new acquisition of the same process republished covers release against a record with our pid and start time but a different claim, and is red without the comparison; a reacquired lock publishes a claim that its predecessor cannot reuse pins that consecutive acquisitions differ. A record written before tokens exist reads as an unmatched claim: release never clears it, and reclaim judges it by its owner's liveness as before. The reclaim-side token comparison has no test of its own because inode reuse cannot be provoked hermetically; it is defense in depth behind the identity and liveness checks, both of which are covered.

The coverage rule. I changed the rule. packages/host-kit/src/internal/process-lock.ts was listed among the simple same-directory publishers, and that framing is what broke: publishing a file into a directory and reclaiming a lock directory are different claims of ownership, and the rename is the second one. In src/daemon/__tests__/atomic-publish-ownership.test.ts the file is no longer a simple publisher, and two narrower assertions say what still holds there — its owner record goes through publishFileSync and it writes no file by hand, and every fs.renameSync is addressed between the lock path and the reclaimed name, checked against the actual call sites. It cannot go back to hand-publishing files, and it cannot quietly start renaming something unrelated to the lock.

Gate at 9ccc2bc: pnpm check:affected --run passed, 605 test files. The same gate passed at 7d4b8f3; the first run there failed check:fallow on reclaimProcessLockDirectory at cyclomatic 10, which I repaired by extracting the win32 rename-refusal fallback into its own function rather than by moving a baseline.

[thymikee]

At 9ccc2bc the inode and claim-token checks stop the race from the last review, and the five release sites you changed now keep the task's own error. Two gaps remain.

The reclaim still has a window where the lock path is empty while a live lock sits under the .reclaimed- name. In reclaimProcessLockDirectory, nothing is at the lock path between the renameSync and the rename back. A polling contender can mkdir there. If it has already written its record, the rename back fails with ENOTEMPTY, that error is swallowed, and both processes hold the lock. When the inode matches, the ps liveness probe also runs inside that window, and inode numbers can be reused after the stale directory is removed. No test runs a contender during the rename back.

Four callers still release in finally, so ownerReleaseUnverified can replace the task's error: platform-apple/src/runner/runner-artifact.ts:114, runner-cache.ts:113, managed-allocation/src/store.ts:95 and store-lock.ts:27.

A smaller-design question: could a short reclaim mutex (for example mkdir <lock>.reclaim around stat, read, judge and remove) replace the rename aside, the inode compare and the rename back? Under that mutex only the dead owner's directory can be at the path, so nothing has to be put back. And could one withProcessLock(task) helper in host-kit own the release pattern for all nine callers, so they cannot drift apart?

CI is green and there are no conflicts.

[thymikee]

Pushed e62cdd81bb. Took the reclaim-mutex shape you sketched, and the helper for the callers.

1. The rename window is gone because the rename is gone. A reclaim no longer moves anything: it takes mkdir <lock>.reclaim, re-decides from what is on disk, and removes in place.

• Nobody else is deciding while we hold the mutex, so the path is never emptied out from under a contender and there is no rename back to fail. The mutex keeps the .lock suffix (x.reclaim.lock) so a sibling scanner still reads the name as a lock, and the two-contenders test now asserts nothing matching .reclaim survives either.
• The re-decision compares inode, directory mtime, liveness, and claim token. Mtime earns its place: a recycled inode can answer your reuse objection, and an acquisition that has only managed its mkdir has written nothing yet to identify itself — but the directory it created a moment ago is not the one abandoned past its grace.
• Removal is attributed, not recursive-by-default. A directory whose record was read goes as that claim's property. A directory with no record is only ever unlink(owner.json) + rmdir, so contents nobody attributed to a claim are never destroyed — an abandoned lock with no record and something else inside is left alone.
• releaseProcessLock lost its rmSync({recursive: true}) for the same reason: it unlinks the record it verified and removes the directory only while empty.

The contender you asked for, injected at the moment the judge is admitted:

a contender that claims the path during a reclaim keeps its lock

It clears the dead claim, publishes itself, and the acquirer must time out with ownerLiveness: 'live' on its record, intact, token and all. Drop the re-decision and that one goes red on its own (1 failed | 27 passed) with its record deleted. Three more pin the mutex itself, each with a mutant that only it catches: a mutex another process holds leaves the abandoned lock standing (bypass the mutex → red), a mutex left behind by a dead process is cleared by age, and a rmdir that cannot empty the directory leaves the judged record rather than forcing it. The rename rule now lives in atomic-publish-ownership.test.ts, which used to allow exactly two renames between the lock path and its reclaimed name and now asserts the file contains no renameSync/asidePath/.reclaimed- at all.

2. Four finally callers, and the bug one of them was hiding. All four (runner-artifact.ts:114, runner-cache.ts:113, managed-allocation/src/store.ts:95, store-lock.ts:27) went to the helper, so ownerReleaseUnverified can no longer speak over the work that failed. The three that already hand-rolled the correct precedence (withDeviceClaimLock, withRunnerLeaseLock, setupManagedAgentBrowser) lost that copy too.

setupManagedAgentBrowser had a second defect from the same shape: if (freshStatus.installed) return freshStatus; returned from inside the try, skipping the only await release() in the function, and left the install lock for the stale-clear path to notice five seconds later. managed agent-browser setup gives the lock back on every path out runs setup twice — the second call takes that early return — and asserts no .lock directory survives anywhere under the state dir. Take the release out of the helper's success path and that test both fails and burns its whole 5s budget waiting on the lock it leaked.

3. Not every caller is task-shaped, and one should stay best-effort. runner-device-set.ts hands its release across a request boundary, snapshot-source/host.ts acquires in the background and releases when it is told to; neither has a task to wrap. swift-cache.ts releases best-effort on the success path on purpose: a helper that finished compiling should not fail its caller because a lock file lingered, and withProcessLock's success path is deliberately strict, because a lock this process could not give back is not a completed task.

Gate: pnpm check:affected --run green, 28 process-lock tests, plus eager-closure-budgets and test-file-size-ratchet explicitly. pnpm typecheck also caught one thing worth noting: tsc -p tsconfig.json alone does not see packages/host-kit/** test files, and a leftover RECLAIMED_MARK reference survived until the real typecheck ran.

[thymikee]

Verified at e62cdd81bb, which is the shape you asked for rather than a patch over the window:

• Reclaim runs under a mkdir-based mutex on <lock>.reclaim (RECLAIM_MUTEX_SUFFIX), so the judge/read/remove sequence never leaves the lock path missing for a polling contender. Nothing has to be renamed back, which is what made the old renameSync round-trip racy (ENOTEMPTY swallowed → two holders).
• Ownership is a claim token, not an inode: claimToken: crypto.randomUUID() is published with the owner record and a release that finds a different token gets not-owner and leaves the directory alone. Inode reuse can no longer make a stale release look valid.
• The release pattern is owned by one helper: withProcessLock is exported from @agent-device/host-kit/file and the four sites that were releasing in finally (runner-artifact.ts, runner-cache.ts, managed-allocation/src/store.ts, managed-allocation/src/store-lock.ts) go through it, along with runner-lease.ts, device-claim-store.ts and agent-browser-tool.ts. Callers can no longer forget the release or release someone else's lock.

process-lock.test.ts covers the mutex, the token mismatch and the abandoned-owner reclaim.

[thymikee]

Reviewed at e62cdd8. Reclaim now runs under the .reclaim mutex and keeps the lock path in place, so the empty-path window and the swallowed ENOTEMPTY from the last review are gone, and a reclaimer that crashes does not wedge contenders. A few gaps remain.

A failed runner build can now show a lock error instead of the build error. runner-artifact.ts#L527 still releases in finally, and that release ends in releaseLock() (runner-device-set.ts#L126), which this PR makes throw ownerReleaseUnverified when the owner record cannot be read. When xcodebuild build-for-testing fails and the record is unreadable, the user sees "Cannot verify ownership" instead of the xcodebuild failure and its hint. Could the error path ignore the release error and rethrow the build error, as withProcessLock does, or could this redirect go through withProcessLock?

Reclaim can still remove a lock that a new contender just took. judgedLockIsStillThere at process-lock.ts#L311 returns true when lstat fails, and reclaim then runs a recursive rmSync on the path. Another process can mkdir and publish its record between the two calls. If the path is already gone under the mutex, could reclaim return without touching disk, so the loop retries mkdir?

A similar question at process-lock.ts#L273: for an abandoned directory with no record, clearLockDirectory unlinks owner.json before rmdir. Could a slow acquirer publish its record between the "unwritten" re-check and the unlink? If so, could this path use rmdirSync only and treat ENOTEMPTY as a new owner?

store-filesystem.ts#L117 still says a stale reclaim renames a lock aside, and store.test.ts:131 still builds a .reclaimed-4242-0.lock fixture, but nothing renames a lock aside now; the sibling is <stem>.reclaim.lock. Could the comment and the fixture use the current name?

The two-contender test at process-lock.test.ts#L269 runs both acquirers in one process, and reclaim is synchronous, so the mutex is never contended; with ownerGraceMs: 0, a held mutex would also count as abandoned at once. Could the test hold the mutex while the second contender reclaims, with a nonzero grace?

Would a smaller shape work under the mutex? A directory with a record could be judged by its claim alone, and an empty one by rmdir alone, which would drop the ino/dev/mtime comparison and most of the JudgedLock stats. runner-device-set could also release through withProcessLock, so no caller keeps a hand-written finally.

CI is green on e62cdd8, and there are no conflicts. The next step is the build-error masking in runner-artifact.ts and the reclaim that removes a path after lstat found it absent.

[thymikee]

Addressed at 0f37589085.

A failed build reports the build. buildRunnerXctestrun runs inside withXcodebuildSimulatorSetRedirect, which hands the release to withProcessLock, so the precedence lives in one place: the task's error wins, and an unverifiable release is reported only when the task succeeded. The two sites that are not tasks — the launch-failure release in runner-session.ts and the teardown release in runner-disposal.ts — call releaseXcodebuildSimulatorSetRedirectBestEffort, so no caller keeps a hand-written finally. runner-device-set.test.ts pins both directions: a build that failed outranks the redirect it could not give back, and a build that succeeded still reports the redirect it could not give back.

A reclaim that finds the path gone touches nothing. judgedLockIsStillThere is deleted. clearStaleProcessLock answers true when stat fails, so the loop's next mkdir simply wins the path. Inside the mutex each branch decides again from what is on disk: the dead-claim branch re-reads owner.json and removes only while that record still answers to the judged claim token.

An abandoned directory with no record is rmdir'd and nothing else. removeAbandonedEmptyLock stats, re-asks the age, and calls rmdirSync; ENOTEMPTY is a publication saying so. releaseProcessLock lost its recursive removal for the same reason — unlink the record it verified, rmdir while empty.

Names. store-filesystem.ts and the store.test.ts fixture both say <stem>.reclaim.lock, which is the mutex. The parked-directory name is gone from the code and the docs.

The two-contender test asks with a second of grace now: with zero, the mutex the winner holds reads as abandoned and the test measures a reclaim that skipped the mutex. Where the mutex is held (default 5s grace) the test also asserts the contender kept polling rather than giving up after one attempt. Reclaim is synchronous in-process, so genuine two-process contention is not producible there; the interleaving is injected in a contender that claims the path during a reclaim keeps its lock.

Smaller shape taken. sameDirectory, judgedLockIsStillThere, forceRemoveLockDirectory, unlinkStrayLockPath and readClaimToken are gone with the ino/dev/mtime plumbing; JudgedLock is a three-case union (dead-claim / empty / stray) and the module is 443 lines where it was 464. A stray path is now simply unlinked: the lstat isDirectory guard could not be made red by any mutation, because unlink answers EISDIR for the case it guarded.

What each remaining decision is worth, by mutation:

mutation	test that goes red
withProcessLock rethrows the release error on the task's error path	a build that failed outranks the redirect it could not give back
removeDeadClaimLock removes without comparing the claim token	a contender that claims the path during a reclaim keeps its lock
removeAbandonedEmptyLock swaps rmdirSync for rmSync(recursive)	an abandoned lock with no record and something else inside is left alone
removeAbandonedEmptyLock stops re-asking the age	a lock directory made anew while a reclaim holds the mutex is not the one that was abandoned
releaseXcodebuildSimulatorSetRedirectBestEffort propagates	a redirect handed back after its task keeps quiet about a release it cannot verify

Two reclaim tests are new (a claim published while a reclaim holds the mutex outlives the empty directory it filled, a lock directory made anew…) alongside the three redirect tests. pnpm check:affected --run passes on this tree, as do typecheck, lint, check:layering and check:fallow.

[thymikee]

Reviewed at 0f37589, as a follow-up to the review at e62cdd8. The claim token and the rmdir-only clear under the mutex are in, and the build error now wins over a release error because the precedence lives in withProcessLock. Two gaps remain.

From the code, a release that fails for a reason other than a lost race can wedge the device-set lock until the daemon restarts. Release returns "unverified" on any non-ENOENT read or unlink error, for example EMFILE or EACCES (process-lock.ts#L40, with the call sites at L143, L162 and L168). The lock directory stays, and its record still names this live daemon with a token that nothing holds. releaseXcodebuildSimulatorSetRedirectBestEffort has already set released = true (runner-device-set.ts#L164) and drops the handle, so nothing retries. The next acquire, also from this daemon, sees a live owner in clearStaleProcessLock and never reclaims. Every later runner launch or build then waits 30 s and fails with "Timed out waiting for ...". The comments at process-lock.ts:38-40 and runner-device-set.ts:52-53, 67-68 and 134-135 say this case heals itself. Could a record that names this live process, with a claim token this process no longer holds, count as a dead claim (for example, a module-level set of live tokens that clearStaleProcessLock checks)? Or could the handle stay retryable until releaseLock succeeds? In both cases, a swallowed release should emit a diagnostic.

The bare catch {} in the best-effort helper (runner-device-set.ts#L70) also swallows errors from reconcileXcodebuildSimulatorSetRedirect (L166-170), which restores the host's real ~/Library/Developer/XCTestDevices from its backup. Before this change, runner-disposal.ts and runner-session.ts let those errors propagate. Now a failed restore at teardown is silent, and the symlink can stay pointed at the agent-device set. Could the catch take only an AppError with details.ownerReleaseUnverified === true, rethrow the rest, and call emitDiagnostic on the swallowed path? A test where reconcile throws would show that teardown reports it.

Is the separate best-effort helper still needed? It reads as a second copy of the withProcessLock catch branch, with a wider swallow. Teardown and the launch-failure path could call one handle method that swallows only ownerReleaseUnverified and logs it. The recovery rule for the first gap decides what that method must do, so that rule comes first.

All 18 checks pass at 0f37589, and there are no conflicts. The next step is a recovery rule for a lock with a spent claim token, plus the narrower catch.

[thymikee]

Reviewed at bb64eac, as a follow-up to the review at 0f37589. The delta takes the smaller design the last review asked for: one module-level set and a handle method instead of the standalone helper.

One problem is left in release(). It calls reconcileXcodebuildSimulatorSetRedirect inside try and await releaseLock() inside finally (runner-device-set.ts#L151). If the restore throws (for example EACCES on renameSync) and releaseLock also throws the ownerReleaseUnverified error, the finally error replaces the restore error, and releaseBestEffort (lines 163-168) then swallows it. EACCES and EMFILE often hit both steps together. The user impact: a failed restore of ~/Library/Developer/XCTestDevices goes silent again, and the symlink stays pointed at our simulator set. The new test covers only a restore failure with a successful lock release. Can you catch the reconcile error, still run releaseLock, and rethrow the reconcile error (attaching an unverified release as a cause or a log line)? A test where both the rename and the owner.json unlink fail, asserting that releaseBestEffort rejects with EACCES, would pin it.

A question on the live-claim set: it is per module instance. If process-lock.ts loads twice in one process (a duplicated bundle chunk), could one copy read the other copy's live claim with the same pid as spent and take over a live owner?

Coverage fails only on scripts/__tests__/test-file-size-ratchet.test.ts: runner-session.test.ts is 1961 lines against 1955 at the merge-base. This PR edits that file, so this failure belongs to the PR; a shared redirect mock factory could bring it back under the limit. Typecheck & Package fails with TS2353 on installArgs in packages/platform-android/src/__tests__/snapshot-helper-install.test.ts. This PR does not touch that file, and main fails the same way at 2cafab3 because #2618 removed the field while that test still sets it, so the failure is unrelated to this PR. A rebase will not clear it until main is fixed.

Next: fix the error precedence in release() and the test-file size.

[thymikee]

Reviewed again at 0f3758; answered at 4f18834125, rebased on origin/main at 5f6481cafb.

A release that fails for a reason other than a race no longer wedges the lock, and the reason it could not is now recorded. You were right that the fix had to be about the claim and not the removal. A token is issued per acquisition and held in liveClaimTokens; the release drops it from that set the moment it is asked for, before it touches the filesystem, because from that instant nobody in this process is acting on the claim whatever the unlink does next. clearStaleProcessLock now treats a record naming this live pid under a token this process no longer holds as the dead claim it is, alongside the ones it already reclaimed. So an EACCES, an EMFILE, or an unreadable record costs one diagnostic line and no longer a 30 s wait that only a daemon restart ends. The diagnostic is process_lock_release_unverified at the owning site — inside the release closure, so it covers every lock in the package rather than whichever caller happens to be listening.

The swallow is narrowed and it moved. release() does two things: restore the host's own ~/Library/Developer/XCTestDevices, and give the lock back. Only the second is a teardown's own business — that claim is spent, and the reclaim above reads it as dead — so releaseBestEffort drops exactly an AppError whose details carry ownerReleaseUnverified and rethrows everything else. A device-set restore that answered EACCES now reaches the caller instead of disappearing, which is the shape you asked to see: the test spies renameSync throwing EACCES and asserts the rethrow.

No separate helper. The standalone best-effort function is gone; the handle carries release for the build path, where withProcessLock owns which of two failures gets reported, and releaseBestEffort for the two sites whose own outcome is already decided. One place decides what a teardown forgives.

The double honesty point from the same round was load-bearing, not cosmetic: two fixtures that pretended to be a rival process were naming process.pid with a token it never issued, which after this change is exactly the signature of a spent claim. They name process.ppid now — a live other process, which is what a contender has to be for the test to mean anything.

Mutations, each run against the tree with only that change:

Mutation	Result
drop the spent-claim disjunct from the reclaim decision	the second acquire from this process waits out its whole timeout, the reported failure verbatim
let releaseBestEffort swallow every error, as the first draft did	the EACCES restore test stops reporting it
use the strict release at teardown	the same restore failure displaces the command's own outcome

Gate at 4f18834125: pnpm check:affected --run passed, 619 test files, 4630 tests, typecheck, lint, layering and the Fallow audit clean. The Coverage job had failed at the earlier head on test-file-size-ratchet, which is not inside check:affected's selection: the two per-file redirect doubles grew runner-session.test.ts past its 1,955-line merge-base count. The handle now lives once in runner-session-fixtures.ts, which the family already shares, and the family is five lines shorter; the ratchet and the eager-closure budgets (614 assertions) pass locally.

Honest gaps. The wedge needs a unlink that is refused, which is fault injection at the seam — no device run produces EACCES on its own, so the live coverage is the CI lanes that build and launch the runner through the redirect this PR adds, not a natural occurrence of the fault. The reclaim after a failed release is pinned by an in-process double rather than a second daemon. And there is no win32 evidence here: the forced-removal fallback has no CI.

[thymikee]

Reviewed at 4f18834, as a follow-up to the review at bb64eac. The delta adjusts the spent-claim commit and adds one shared device-set redirect double, but the redirect give-back problem is still open.

In runner-device-set.ts#L149, release() runs reconcileXcodebuildSimulatorSetRedirect in the try and await releaseLock() in finally. If renameSync throws EACCES and the owner.json read or unlink also fails, the finally throws ownerReleaseUnverified and replaces the EACCES; releaseBestEffort (L155-L164) then matches isOwnerReleaseUnverified and drops it. Both steps use the same filesystem, so they tend to fail together. A failed restore of ~/Library/Developer/XCTestDevices then goes silent and the symlink stays on the agent-device simulator set. The restore error should win: catch it, still give the lock back and log an unverified release, then rethrow. A test where both the rename and the owner.json unlink fail, and releaseBestEffort rejects with EACCES, would pin this.

The no-redirect branch looks like it has a related gap. At runner-device-set.ts#L109, await releaseLock() is inside the try, so an unverified release goes to the catch, which reconciles, releases again, and throws COMMAND_FAILED "Failed to redirect XCTest device set path". Would a simulator whose set already is XCTestDevices now fail its build or launch with that misleading redirect error when owner.json is unreadable? Can this branch follow the same rule as teardown?

Could the redirect handle have one ordered give-back instead: restore, then release, where the restore error always wins and an unverified release is only logged? Then release and releaseBestEffort would differ only in whether an unverified release throws, and the three exit paths of acquireXcodebuildSimulatorSetRedirect (L109, L129, L149) would share it. What needs to be decided first is where that precedence rule lives, in withProcessLock or in the handle.

Two doc fixes. The JSDoc at runner-session-fixtures.ts#L189 has a garbled sentence ("the the host's device set came back, and which door..."). The PR body says "Four files; one is outside host-kit", but the diff touches 30 files: device-claim-store, the managed-allocation store, agent-browser setup, and the Apple runner artifact, lease and disposal paths move onto withProcessLock, which changes which error those callers report. Can the body and the CHANGELOG entry describe that migration and the new error precedence? The bb64eac question about a second bundled copy of process-lock.ts treating another copy's live claim as spent is also still open.

All checks passed at 4f18834 and there are no conflicts. The newer head d055691 is the same patch rebased onto main, so this applies to it unchanged. Next: make the restore error win in release() and add the double-failure test.

[thymikee]

Rebased on origin/main at e3cbc91a1a; head is f334710c9b. No textual conflict, and the branch met main's new close-finalization tests in a way worth naming.

Those tests caught a real gap between the two branches. runner-close-finalization.test.ts and runner-session-close.test.ts landed on main answering the device-set redirect with { release }, which was the whole handle the day they were written; disposal in this branch asks the session for releaseBestEffort. Every test in those two files that closes a session with a redirect failed with session.simulatorSetRedirect?.releaseBestEffort is not a function — four tests, which is how the local gate found it before CI did.

The double is what is incomplete, not the teardown, so f334710c9b takes the handle from runner-session-fixtures.ts in both files, the module their six sibling test files already use for it, and drops the per-file spy that appeared only in setup. Teardown is not guarded for a handle that does not implement its own type: that would teach the next caller that handing one over is allowed.

Why typecheck passed over it: mockAcquireXcodebuildSimulatorSetRedirect is a bare vi.fn(), so mockResolvedValue accepts any shape. The shared double is what makes the seven session-family doubles agree now; making that mock answer-typed is a separate change and this PR does not do it. Proof of coverage: no { release }-only literal remains anywhere except the object runner-device-set.ts builds and the session field's own two-operation type.

Gate at f334710c9b: pnpm check:affected --run passed, 621 test files, 4644 tests, typecheck, lint, layering and the Fallow audit clean; the apple-runner lane alone is 44 files, 430 tests, including the four that were red on the unmerged pair. Earlier evidence at 4f18834125 and 0f37589 stands as attributed there.

[thymikee]

Checked at f334710. The new commit only moves the two close tests onto the shared redirect double, and runner-device-set.ts is unchanged, so the review at 4f18834 still applies.

In release() at runner-device-set.ts#L144-L151, an unverified lock release thrown from finally still replaces a restore error, and releaseBestEffort then drops it. A failed restore of ~/Library/Developer/XCTestDevices stays silent. The no-redirect branch at L109-L111 still turns an unverified release into "Failed to redirect XCTest device set path".

All checks pass and there are no conflicts. Next: make the restore error win in one ordered give-back, and add the test where both the rename and the owner.json unlink fail.

[thymikee]

Reviewed at 5680258423 and reaffirmed at 5680920139; answered at 868d0ad713.

The restore error now wins, and there is one ordered give-back. release() reconciled inside try and gave the lock back in finally, so when both failed the finally threw ownerReleaseUnverified over the restore's EACCES, and releaseBestEffort then dropped it exactly as you described: a XCTestDevices left pointed at this simulator's set and the only report naming the lock. The handle now runs restore, then release; the restore failure is thrown whichever door was used, the lock goes back regardless, and the release's own failure is recorded either way. release and releaseBestEffort differ in the one thing you said they should: whether the caller also throws an unverified release.

Where the precedence rule lives: the handle, not withProcessLock. That helper is host-kit and knows only that a lock was taken and a task ran; which of "the host's device set could not be restored" and "this claim could not be verified" a caller should hear is a fact about this redirect, and putting it in the lock would make every other lock caller answer a question it does not have. withProcessLock keeps its own, narrower rule — the task outranks the give-back — which is what the artifact, lease, store and publish callers needed.

The no-redirect branch followed the same rule. It ran releaseLock() inside the try, so an unverified ownership check landed in the catch, reconciled, released again, and raised the redirect error for a redirect that was never needed. A test reaches that window by answering the lock at the seam the module acquires it through (vi.mock of ../host.ts, passing everything else through) and asserts a null handle; putting the release back inside the try brings Failed to redirect XCTest device set path back verbatim, which the run printed. The third exit path — a redirect whose install failed — now also survives a second failure in its own clean-up: the log records the restore that could not run and the caller still hears why the redirect failed.

The new tests. Both faults at once (restore renameSync refusing EACCES, record refusing the unlink) rejected through release and releaseBestEffort alike with EACCES, the lock still standing as proof the pair happened. Reverting the order to the old finally reddens that test through both doors. The install-failure test keeps EPERM visible inside the redirect error, and the no-redirect test asserts the null handle.

The bundled-copy question from bb64eac was real and is now answered in code, not prose. A pid names a process, and the spent-claim rule reads a record that names one; two loadings of process-lock.ts in one process share the pid and the start time and cannot see each other's liveClaimTokens, so the second one's live claim read as spent. Records carry claimIssuerId now and the rule requires it to be this loading's; a record without one — written before issuers, or by other code — is not evidence of a spent claim and stays subject to the liveness answer, so nothing that reclaimed before stops reclaiming now. The field is parsed as optional rather than added to the required shape, because a pre-upgrade record must still parse instead of reading as unreadable. Deleting the issuer check reddens the new test alone, 1 failed 31 passed.

Doc fixes. The fixtures comment lost a clause mid-edit and now says what the double actually does. The body overstated the diff as four files; it says 32 and names the migration, and the CHANGELOG gained the entries it was missing: the withProcessLock error precedence and the redirect's ordered give-back, beside the lock recovery it already described.

Gate at 868d0ad713: pnpm check:affected --run passed, 622 test files, 4648 tests, typecheck, lint, layering and Fallow clean.

[thymikee]

Reviewed at 868d0ad, as a follow-up to the review at f334710. The handle's give-back and the no-redirect branch now let a restore error win, but the install-failure catch still hides one.

In acquireXcodebuildSimulatorSetRedirect (runner-device-set.ts#L137), take an install that fails after renameSync(xctestDeviceSetPath, backupPath), for example symlinkSync with EPERM, where the rename back also fails with EACCES. The catch throws with the EPERM text only, and the restore error reaches only the ios_runner_xctest_device_set_restore_failed diagnostic. The host's XCTestDevices then stays in the backup path until the next acquire, and the CLI and MCP output do not say so. The new install-failure test asserts only EPERM, so it pins this gap. Can this catch add restoreError to the AppError details, with a hint that names backupPath, and can the test assert that details.restoreError matches /EACCES/?

Would one ordered give-back helper do for all three sites? It would restore, then release, and return { restoreFailure, releaseFailure }, so the install catch uses the same result as the handle instead of its own try/catch. Then this gap cannot come back. If there is a reason to keep the sites separate, can the PR say it?

All 18 checks pass and there are no conflicts. Next: surface the restore error from the install catch, with its test.

[thymikee]

Answered at 534d3f4e30, rebased on origin/main at 354d3516ec.

One ordered hand-back now answers for all three exits, which is the shape you asked for and the one
that removes the class rather than the instance. handBackDeviceSet(paths, lockDirPath, releaseLock)
restores, then releases, and returns { restoreFailure, releaseFailure }; it records the release failure
itself, so an exit cannot report one half and stay quiet about the other. The install catch dropped its
own try/catch pair and the ios_runner_xctest_device_set_restore_failed diagnostic went with it —
that half-failure now reaches the caller instead of the log, and there is one place left where an order
could be chosen wrongly. The three sites differ only in what they say, which is the part that genuinely
belongs to each: the handle throws the restore and, at the strict door, the unverifiable release; the
no-redirect exit says nothing; the install failure reports the redirect and carries the restore with it.
The precedence decision still lives in the handle rather than withProcessLock, for the reason in the
earlier reply — the lock helper has no idea that a device set is what it is guarding.

The install failure names what it left behind. details.restoreError carries the restore's own error
and details.hint names the path: "The host's own device set is still renamed aside at
<backupPath>: restore it, or remove that path, before another runner build redirects it." The test
faults the install after the rename (symlinkSync → EPERM) and the restore on the way out (renameSync
→ EACCES), and now asserts restoreError matches /EACCES/ and the hint contains the backup path, with
backupPath passed in explicitly rather than reconstructed from the production suffix.

Writing the helper surfaced a gap the old tests had. The double-fault test's injected failures are
order-independent, so collecting both failures in either order passes it: I swapped restore and release
inside the new helper and the suite stayed green. The order is the contract — the lock is what lets
another build read XCTestDevices, so releasing first hands out a directory that is still a symlink into
this simulator's set — so the host's device set is back before the lock is records what actually
happened: renamed-aside, symlink-installed, restored, lock-released, watched at renameSync and
rmdirSync of the lock path. Swapping the two steps reddens that test and nothing else.

Mutations: keeping the restore out of the install error's details reddens the install-failure test alone;
swapping the hand-back's order reddens the ordering test alone; and a handle that stops throwing the
restore reddens the two restore-precedence tests. The recording moving into the helper is behavior
unchanged, so it owes no new coverage — but note the honest gap now in the body: the hand-back's warn
line is not asserted by any test, since the ordering test watches the filesystem.

Gate at 534d3f4e30: pnpm check:affected --run passed, 623 test files, 4661 tests. The rebase took one
CHANGELOG resolution against the merged #2599 and #2610 entries.

[thymikee]

Reviewed at 534d3f4, as a follow-up to the review at 868d0ad. The lock-ownership fix looks right, and one ordered handBackDeviceSet is the shape asked for. But the redirect decision now runs in a different order, and one exit still drops a restore failure.

The same-set check at runner-device-set.ts#L109 now runs before reconcileXcodebuildSimulatorSetRedirect. At 868d0ad and on main, reconcile ran first. sameResolvedPath follows symlinks, so after an interrupted build leaves XCTestDevices symlinked into this simulator's requested set, the check reads it as "already the same set". The no-redirect branch then hands the device set back, which removes the symlink, and returns null. The next xcodebuild runs against the host's own XCTestDevices, so the first runner build after an interrupted one targets the wrong device set. The tests at runner-xctestrun.test.ts:524 and :556 use a symlink to a different or deleted set, so they don't cover this. Can reconcile run before the same-set check again, or can the check use lstat so a symlink never counts as the same set? A test that leaves XCTestDevices -> requestedSetPath plus a backup and expects a non-null handle would pin it.

The no-redirect branch at #L113 reads only handBack.releaseFailure and drops restoreFailure. If the host's XCTestDevices can't be restored there (for example EACCES), the call returns success and nothing reports it. Before this delta, a reconcile failure on this path threw and emitted ios_runner_xctest_device_set_restore_failed. Can this exit surface restoreFailure like the throwing branch does, with an EACCES test?

The hint at #L140 always says the device set is still renamed aside at backupPath. Is that true when reconcile found a legacy backup elsewhere, when the failure was unlinking an orphaned symlink with no backup, or when the catch runs before the rename? Could the hint name backupPath only when a backup is there?

Would it be simpler if handBackDeviceSet owned the whole report: restoreFailure, a hint built from what reconcile found, and the recorded releaseFailure? Each call site would then only decide whether to throw, and no exit could read one field and drop the other.

This changes the runner's device-set redirect, so it needs a live run: a Simulator with a scoped simulatorSetPath, starting from a leftover XCTestDevices symlink into that set, showing the runner build uses the redirect and XCTestDevices is a real directory again after close.

All 18 checks pass and there are no conflicts. Next: restore the reconcile-first order and surface restoreFailure on the no-redirect exit, each with a test.

[thymikee]

Answered at 774166ed7c, rebased on origin/main at 9173d3c797. One correction first: the "records the
release inside the hand-back" I described last round was not in 534d3f4e30 — a mutation restore had put
the file back and I described the intended shape as the shipped one. It is in this head, and the body now
matches the code.

The reconcile runs before the decision again, and for the reason you gave: sameResolvedPath follows
symlinks, so a leftover XCTestDevices -> requestedSetPath read as "already the same set", the hand-back
removed the symlink, and the next xcodebuild got the host's own devices. The check is also not fixable
with lstat alone: with a backup waiting and the host's set absent, the install would rename the stale
symlink into the backup slot, so only the restore knows how to undo that state.
an interrupted build that left the symlink is redirected again, not read as already done builds exactly
your leftovers — backup holding the host's file, XCTestDevices symlinked into the requested set — asserts
a non-null handle, the set pointed at this simulator, the host's file present exactly once, and the
directory back as a real one after release. Moving the check back in front of the reconcile reddens that
test and nothing else.

A refused restore is raised, and both exits share the raiser. After the order fix the reachable moment
is the restore that runs before the decision: the symlink keeps both paths resolving equal, so the
"nothing to do" reading and "the host has no device set" are the same observation, and only the rename tells
them apart. a restore that was refused on the way in is not reported as a simulator that needs no redirect
faults that rename with EACCES and asserts the call rejects with it, the backup still standing as the fact
the caller needs, and the lock gone anyway. Removing that raise — handing back and carrying on — reddens
that test alone. The no-redirect exit calls the same handBackOrRaise, so it cannot drop a restore failure
even though, with the order fixed, its own restore is a no-op by construction; I did not find a state that
reaches it, so I am not claiming a test for one.

The hint now says only what is on disk. The hand-back observes the filesystem while it still holds the
lock and before the release moves anything (findDeviceSetBackup: this run's backup, then the legacy
prefixes), and returns a path or nothing; redirectFailure names one only when it got a path. So a catch
that runs before the rename, or a failure that was only ever an orphaned symlink, no longer sends the reader
to a backup that does not exist — a redirect that failed before it moved anything names no backup that is not there pins the first of those with restoreError and hint both absent and the host's file still in
place.

One helper owns the report, which is simpler than what I had: handBackDeviceSet returns
{ restoreFailure, renamedAsidePath, releaseFailure } and records the release itself, so an exit cannot
report one half quietly; the two exits with nothing of their own to say call handBackOrRaise; and the one
exit with a bigger report — the failed install — passes the hand-back to redirectFailure. Call sites
decide only whether to throw. That extraction also cleared the Fallow complexity finding the redirect had
grown into, without a suppression.

Live run, no fault injection, on this Mac: from your leftovers (host's set renamed aside with a marker
file, XCTestDevices a symlink into the simulator's scoped set),
withXcodebuildSimulatorSetRedirect around xcodebuild build-for-testing -scheme AgentDeviceRunner —
during the build the lock was held and XCTestDevices was still that symlink into the set; the build
succeeded in 20 s; on the way out XCTestDevices was a real directory again with the marker back inside it,
backup consumed, lock released. Two deviations, both forced by this host: the caller was a script bound to
the real host seams (kernel resolver, host-kit diagnostics/process/lock) rather than the daemon, because
every daemon-routed open/prepare with --ios-simulator-device-set returned DEVICE_NOT_FOUND while a
daemonless devices call found that very device; and the destination was generic/platform=iOS Simulator
instead of id=<udid>, because other sessions on this machine deleted the simulators I created between
attempts. The first of those looks like a real gap in --ios-simulator-device-set on the daemon path and
should get its own issue rather than ride in here.

Gate at 774166ed7c: pnpm check:affected --run passed, 4664 tests, Fallow clean.

[thymikee]

Reviewed at 774166e, as a follow-up to the review at 534d3f4. The hand-back now owns the release record, but one reconcile path may still run on after the lock is released.

At runner-device-set.ts#L116, when the first reconcile fails and the retry inside handBackOrRaise succeeds, nothing throws. The hand-back releases the lock (L231), and execution then continues into the same-set check and installDeviceSetRedirect (L128) with no lock held. A second process could take the device-set lock while this one is renaming XCTestDevices aside, which is the race #2523 closes. The first reconcile error is also lost, and the returned handle's giveBack would release the spent claim again. Can every path through handBackDeviceSet leave the acquire by a throw or a return? One option is to keep reconcile, the same-set check and install under the single try that main had, with the catch handing back and throwing redirectFailure. A test where the first reconcile throws once and the retry succeeds, expecting a rejection and no lock directory, would pin it.

renamedAsidePath at #L228 is the first existing candidate among backupPath and the legacy prefixes. If reconcile failed while deleting a leftover legacy backup after the host set was already restored, the hint would tell the user the host set is renamed aside at that stale path, and following it could overwrite the current simulator set. Should the hint name a path only when XCTestDevices is absent or is a symlink?

The live run posted in the comments used a script bound to the host seams with generic/platform=iOS Simulator, not the daemon's open or prepare with --ios-simulator-device-set against the scoped simulator's udid. The author reports that route returns DEVICE_NOT_FOUND. So no run yet shows a runner build for a simulator in a scoped set resolving through the redirect, with XCTestDevices a real directory again after close. If DEVICE_NOT_FOUND blocks that route, could it be filed and linked here, with the script run's log attached as interim evidence?

All 18 checks pass, and there are no conflicts. Next: make the reconcile-failure path exit with the lock released only on a throw or return, add the retry test, and settle the live run or the linked issue.

[thymikee]

Answered at e732fe641a, rebased on origin/main at fffa01e02c.

The acquire now leaves only by throw or return. I took your option: reconcile, the same-set check and
the install sit in one try, and its catch hands back and throws redirectFailure. That removes the shape
rather than guarding it — the hand-back is no longer reachable from a point that could continue — so the
three exits are: the catch throws, the no-redirect exit throws or returns null, and only a redirect that
was actually installed returns a handle. All three of your consequences go with it: the first reconcile error
is now the report's details.error, and giveBack cannot re-release a spent claim on a path that never got
a handle.
a restore that only works on the second look still ends the acquire is your test: the first rename out of
the leftovers is refused with EACCES and the retry inside the hand-back succeeds, and it asserts the
acquire rejects, the lock directory is gone, and the host's own file is back in a real XCTestDevices with
the backup consumed. Swallowing the first reconcile — the shape you found — reddens that test and none of the
other eleven.

The hint names a path only when the set really is out of place. findDeviceSetBackup now returns nothing
unless XCTestDevices is absent or a symlink, so the state you describe — host's set already back, a legacy
backup left behind by a deletion that failed — names no path a reader could copy over the set in use. I am
not claiming coverage for it: with one hand-back per path I could not find a report built from a restore that
half-succeeded, so this is the field being made to mean what it is called rather than a bug fixed. I had
deleted this guard last round arguing exactly that, and your reachable-looking case is why it is back.

On the live run, I withdraw the DEVICE_NOT_FOUND claim — it was not the flag. open <AgentDeviceTester.app> --platform ios --ios-simulator-device-set <set> --udid <udid> did resolve that scoped simulator and built the
runner through the redirect; a 1 Hz watcher around it recorded

12:22:31 symlink->/Users/thymikee/.agent-device-sets/ad2598 locked backup-present
   … 122 consecutive samples …
12:24:26 realdir(0) free no-backup

so a runner build for a simulator in a scoped set does resolve through the redirect and leaves
XCTestDevices a real directory again. What it did not get is a session: an xcrun step in the same request
failed with Invalid device: 51CE507A-… (exit 148), and the reason is that this host deletes the simulators
under us — xcrun simctl --set <dir> create returns a UDID and the directory is empty seconds later, twice
under /private/tmp and once under $HOME, and a run that succeeded at 12:22 found its own device gone at
12:25. devices --ios-simulator-device-set <set> --json and doctor resolved the same device in those same
minutes, which is why I no longer think the daemon drops the flag. I have not filed it: nothing in the logs
pins the deletion on agent-device, and an issue blaming it for DEVICE_NOT_FOUND would be wrong. Say the
word and I will file it as a host/tooling question with the watcher and request logs attached, or point me at
a machine where a scoped set survives and I will get the session leg green.

Gate at e732fe641a: pnpm check:affected --run passed, 4673 tests in 625 files, Fallow clean; body updated
with the new mutation row and the narrowed gaps.

[thymikee]

Reviewed at e732fe6. The earlier reconcile finding is fixed: reconcile, the same-set check and the install now share one try whose catch hands back and throws, and the new second-look test would fail if the first reconcile error were swallowed.

Can the restored guard in findDeviceSetBackup get a test? It is at runner-device-set.ts#L250, and one state seems to reach it: XCTestDevices is a symlink, and both the current backup and a legacy .agent-device-xctestdevices-backup-* directory exist. Reconcile unlinks the symlink and renames the backup into place, and then the rmSync of the legacy directory in the cleanup loop (#L313-L316) throws. The hand-back then records restoreFailure while XCTestDevices is already a real directory. If the guard is removed, the hint names the legacy copy as the host's own set, and every current test still passes. A user who follows that hint could copy a stale set over the one in use. One acquire test would pin it: a symlinked XCTestDevices, a backup and a legacy backup, and an rmSync spy that throws only for the legacy path, asserting that details.restoreError is set and details.hint is undefined.

The live run you posted shows the scoped-set open building the runner through the redirect and leaving XCTestDevices a real directory afterwards, which covers the changed redirect and lock route. The later Invalid device error comes after the build step, so it does not block this change.

All 18 checks pass, and there are no conflicts. Next: the guard test above.

[thymikee]

Pinned at 615c58cc87 by a restore that half-finished names the backup it was refused, not an older leftover, and it does pin it: with the guard removed that test is the only one of thirteen that goes red, on
hint naming the older leftover.

I wrote your state first and it does not reach the guard. With XCTestDevices a symlink, this run's backup and
a .agent-device-xctestdevices-backup-* beside it, and rmSync refused for the legacy path, the first
reconcile does what you said — unlink, rename the backup into place, then fail on the leftover — but that
failure is the one the acquire's catch already has as details.error. The guard decides the hint, and the hint
comes from the hand-back's own reconcile, which by then finds the host's set standing as a real directory next
to a backup it did not move. That is the collision branch: it warns
(ios_runner_xctest_device_set_restore_collision) and returns, so restoreFailure is never set and there is no
hint to get wrong. The test rejected with the redirect failure and restoreError undefined, and deleting the
guard left it green — which is exactly the failure mode you were guarding against, just one level down.

What does reach it is a restore that is refused before it puts the host's set back, so the hand-back is the
pass that half-finishes: the first rename out of the leftovers fails with EACCES after the symlink is already
down, the hand-back renames the backup into place and is then refused with EPERM while deleting the older
leftover. restoreFailure is set and XCTestDevices is a real directory — and findDeviceSetBackup, without
the guard, answers with the older leftover because this run's backup is consumed by the rename that just
succeeded. The test asserts details.restoreError matches EPERM, details.hint is undefined, the leftover is
still on disk, the host's own file is back inside XCTestDevices, and the lock directory is gone.

I also tried making the collision branch clear older leftovers as well as warn, which would have made your state
reach the guard as written. I dropped it: it changes what a collision deletes, and the case above needed no
change in what gets deleted — only in what gets named. Production is unchanged in this round; the diff is the
one test.

Gate at 615c58cc87: pnpm check:affected --run passed, 4674 tests in 625 files, Fallow clean. Body updated
with the new mutation row and the narrowed gap.

[thymikee]

Reviewed at 615c58c. The guard test from the last round is in, and your trace holds. The state from the last review ends in the restore-collision branch, so it never reaches findDeviceSetBackup. The new test does reach the guard: the first rename is refused with EACCES, then the hand-back is refused with EPERM on the leftover, and without the guard the hint assertion fails because the hint names the older leftover. I checked this by reading the code at 615c58c; I did not run the mutation.

Production code is unchanged since e732fe6, so the earlier live run of the redirect and lock route still applies. All 18 checks pass, and there are no conflicts. Nothing else is open from my side.

[thymikee]

Reviewed at 6aa2f85, a follow-up on the clean review at 615c58c. The two new commits keep the lock rule: the device-set lock and the three build-artifact locks (runner, Swift recorder, snapshot bridge) still release only a lock they acquired, and the seven device-set redirect tests moved from runner-xctestrun.test.ts into runner-device-set.test.ts with their assertions intact. I found nothing blocking. Production code changed after the live run at e732fe6, but the working-lock path is the same, and the smoke run on this head shows the runner starting and captures completing.

Not blocking: the delta changes which error wins when a build fails and the lock release also fails, in the snapshot bridge build (packages/platform-apple/src/snapshot-source/) and the Swift recorder build (packages/capture-kit/src/recording/); one test per site would keep that from regressing.

The failing Smoke Tests job (an iOS fixture setup wait timeout) looks unrelated. Six of seven wait polls returned readable captures and the wait timed out on missing text, not on a lock or release error. The same timeout also showed up on Android smoke in #2652, which does not touch these files. Please rerun it before merge.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-17 05:53 UTC