Skip to content

fix(daemon): take a foreign device claim the device's own reboot invalidated - #2570

Merged
thymikee merged 2 commits into
t3code/2538-device-boot-observationfrom
t3code/fix-2538-quality-review
Sep 14, 2026
Merged

thymikee merged 2 commits into
t3code/2538-device-boot-observationfrom
t3code/fix-2538-quality-review

Conversation

@thymikee

@thymikee thymikee commented Sep 13, 2026

Copy link
Copy Markdown
Member

Summary

open now settles a foreign device claim that the device's own reboot invalidated. Claims outlive the device they describe: shutting a Simulator down takes its app process, runner, and accessibility connection with it, while the claiming daemon and claim file stay healthy. Every later open refused with DEVICE_IN_USE, naming an owner with nothing left to close.

open asks the device when its current boot began, releases a claim predating that through the same exact-owner reconciliation device release --stale uses, and reports the release in warnings. The bound is the last open its owner made, renewed by every open that reaches the device, including one that boots it. The probe runs only for a conflicting foreign claim, after owner-liveness answers.

Part 3 of 3 for #2538, on #2575 and #2574. 1,065 gross lines.

Closes #2538

Validation

Tested at 6e243cb754. pnpm check:affected --run passes every lane, including related vitest (1,986 tests), except mutation-model, which fails identically at base locally where CI is green.

The routed claim tests live in request-router-open-claim.test.ts. Its boot-during-preparation regression arms the fixture's readiness seam, so the device boots between the two renewals and the foreign open asks from a second store; it fails when either renewal is removed. Live, two state dirs on one simulator: a foreign open on a simulator its owner's preparation had booted was refused; a reboot with no owner reopen let a foreign open take the device and say so. Experiment device cleaned.

@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.52 MB 4.52 MB +4.6 kB
Package (unpacked) 4.52 MB 4.52 MB +4.6 kB
Package (download) 1.33 MB 1.33 MB +1.7 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 20.3 ms 18.7 ms -1.6 ms
CLI --help 53.4 ms 52.5 ms -0.9 ms

@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-14 09:31 UTC

@thymikee

Copy link
Copy Markdown
Member Author

Two ownership cases still need fixing at f0ae9d7.

In device-claim-reboot.ts:35, the original claim timestamp remains the only cutoff even after its owner successfully reopens the app after reboot. The existing-session open path preserves that claim, so a later foreign open can reclaim a device that the original session is actively using and clean up its resources. Renew or validate ownership against the current boot when the owner resumes, and cover owner reopen followed by foreign open.

In packages/platform-android/src/device-boot.ts:18, subtracting sampled uptime from the time the ADB response arrives moves the inferred boot forward by response latency. Shortly after boot, this can make a valid newer claim look stale without another reboot. Use a conservative bound from before the probe and cover a delayed response.

The diff is also 1,442 gross lines, above the repository’s 1,000-line PR limit; split the behavior-preserving extraction from the takeover change. The current smoke failure is the automation-longpress scroll assertion seen on #2565 before its successful retry, so it appears unrelated to these changes.

@thymikee
thymikee force-pushed the t3code/fix-2538-quality-review branch 2 times, most recently from 21db6ff to 0dd2527 Compare September 13, 2026 20:49
@thymikee
thymikee changed the base branch from main to t3code/2538-device-boot-observation September 13, 2026 20:49
@thymikee
thymikee added this pull request to stack #2576 September 13, 2026 20:50
@thymikee

Copy link
Copy Markdown
Member Author

Both ownership cases are fixed at 0dd2527678.

The bound is now the instant the owner last vouched for the device, not the first write. resolveExistingClaim renews updatedAtMs on the held path, and rebootedDeviceClaim compares the observed boot against that stamp, so an owner that reopened its app after the reboot keeps the device and its resources. Covered by an owner that reopened its app after the reboot keeps the device in src/daemon/__tests__/device-claim-settlement.test.ts and by a claim its owner renewed after the boot describes that boot in src/daemon/__tests__/device-claim-reboot.test.ts.

Android now samples the host clock before the adb round trip, so probeStartedAtMs - uptime is a bound never later than the real boot instant: latency can only push the answer earlier, which cannot make a newer claim look stale. Covered by a slow uptime answer cannot move the boot instant past the moment the probe began.

The diff is split as stack #2576: #2574 is the behavior-preserving claim-settlement move (772 gross), #2575 is the boot-observation contract, probes, and hub (489 gross), and this PR is the takeover alone at 957 gross lines.

@thymikee

Copy link
Copy Markdown
Member Author

The Android timing fix and PR split resolve those two points, but the owner-reopen bug remains at 0dd2527. renewHeldClaim only runs through acquireDeviceClaim; the existing-session open path in session-open.ts:162–243 still calls completeOpenCommand directly and never renews the claim. The new regression calls acquireDeviceClaim twice, so it does not exercise the reported production path. Wire renewal into a successful existing-session reopen and cover that routed flow followed by a foreign open, with live confirmation. Checks are green; this remains a code blocker.

@thymikee

Copy link
Copy Markdown
Member Author

Renewal now rides the production reopen at 2f8cbfec72.

renewDeviceClaim is the claim-side write, and completeOpenCommand calls it for a successful existing-session open, which is the branch session-open.ts reaches without ever acquiring a claim. The acquire-path stamp stays for the case that does go through acquisition. All three ownership-matched claim writes (clear, abandon, renew) now share one writeOwnedDeviceClaim seam under the claim lock.

Routed coverage: open renews the claim of an existing session that reopened its app after a reboot in src/daemon/__tests__/request-router-open.test.ts drives two routed opens for one session, asserts the stored updatedAtMs moved, then opens a second workspace with the boot observed between the two stamps and must get DEVICE_IN_USE with the claim still naming the owner.

Live on a fresh iPhone 17 Pro simulator, two state dirs and one device, Safari as the app:

  • pre-reboot foreign openDEVICE_IN_USE for session ad-owner;
  • simctl shutdown + boot, then owner open → succeeded, claim went createdAt 00:43:21Z / updatedAt 00:44:04Z;
  • foreign open again → DEVICE_IN_USE. That is the case that used to take the device.
  • rebooting once more with no owner reopen → foreign open succeeded with Took the device from session "ad-owner" …, so the release still happens when the owner really is gone.

Sessions closed, daemons stopped, state dirs removed, simulator deleted, device status --stale reports no local claims.

@thymikee

Copy link
Copy Markdown
Member Author

The sequential owner-reopen case is now fixed and has the requested live evidence. One ownership race remains at session-open-execution.ts:245: openApplication runs before renewal, so a foreign daemon can take the still-preboot claim while the owner is reopening. renewDeviceClaim then returns ownership-changed or absent, but the result is ignored and the old owner still reports success. The request execution locks are local to each daemon, so they do not serialize this case. Establish ownership across reopen and renewal, handle lost ownership, and cover a foreign takeover interleaved with owner reopen.

The layer is also back over the 1,000-line budget: 848 additions plus 211 deletions, or 1,059 gross lines. Reduce it by at least 59 lines or move the behavior-preserving extraction into the lower layer. CI is green at 2f8cbfe; these remain the blockers.

@thymikee
thymikee force-pushed the t3code/fix-2538-quality-review branch from 2f8cbfe to 512736c Compare September 14, 2026 02:50
@thymikee

Copy link
Copy Markdown
Member Author

Ownership now spans the reopen at 512736caf4, and the layer is back under budget at 787 additions and 60 deletions.

Renewal moved ahead of all device work: session-open.ts renews the existing session's claim right after argument validation, before prepareOpenCommandDetails can boot anything, and completeOpenCommand no longer writes a claim after launching. renewDeviceClaim answers with the record that took the device, so losing ownership is a typed outcome rather than an ignored string: a lost renewal ends the reopen with the ordinary DEVICE_IN_USE conflict response, or a claim-lost-during-reopen refusal when the claim is gone, and the app is never launched on a device this session no longer owns.

Coverage: an owner reopen that lost the device mid-flight reports the loss instead of launching in src/daemon/__tests__/request-router-open.test.ts takes the owner's claim away with a foreign one between two routed opens and asserts the refusal names the new owner and leaves that claim in place. The settlement cases are now one file, which also removed the duplicated copies of six acquire tests this layer had been adding back to device-claims.test.ts — that is where the 172 lines came from rather than moving the extraction again.

Live, two state dirs on one simulator: owner open, then simctl shutdown and boot, then the foreign open took the device; the owner's next open returned DEVICE_IN_USE … owned by session "race-b" … instead of launching over it. Sessions closed, daemons stopped, state dirs and experiment simulator removed, device status --stale clean.

@thymikee

Copy link
Copy Markdown
Member Author

The lost-owner refusal and size limit are resolved at 512736c. One boot-order case remains: session-open.ts renews the claim before prepareOpenCommandDetails, which can cold-boot a stopped simulator through prepareApplicationOpen → ensureAppleReady → ensureBootedSimulator. That successful reopen leaves updatedAtMs earlier than the device’s new boot, so a later foreign open still treats the active owner as stale and takes its claim. The reported live run boots the simulator before reopening, so it does not cover this path. Keep ownership protected while open boots the device and establish the claim for that boot; cover owner open on a stopped simulator followed by foreign open. CI is green and the layer is now under budget.

@thymikee
thymikee force-pushed the t3code/fix-2538-quality-review branch from 512736c to a9a4b8e Compare September 14, 2026 05:07
@thymikee

Copy link
Copy Markdown
Member Author

Boot order fixed at a9a4b8ea2e. Renewal now runs after preparation instead of only before it, on both open paths, so a device an open itself booted counts as a boot its owner caused: a fresh claim is stamped once preparation is done and before any launch, and a reopen is stamped the same way. A claim another session took during preparation still ends the open with the usual conflict rather than a launch on a device it no longer owns.

Covered by the routed test a reopen that booted the device keeps the device against the next open, and live on a fresh stopped Simulator: the owner's open cold-booted it and the next foreign open returned DEVICE_IN_USE; after simctl shutdown + boot with no owner reopen a foreign open still took the device and said so. Experiment Simulator, sessions and state dirs cleaned; device status --stale clean.

@thymikee

Copy link
Copy Markdown
Member Author

The post-preparation renewal at a9a4b8e fixes the cold-boot ordering, and the reported stopped-Simulator run covers the original failure. CI is green. The remaining validation gap is the regression at request-router-open.test.ts:953: it sets the boot time before reopening, so the old pre-preparation renewal satisfies its timestamp assertion; the foreign open also shares the owner’s session store and is refused before reboot-based claim settlement. Make preparation advance the boot time, exercise the foreign claim path from a separate store, and show that removing the post-preparation renewal makes the regression fail. No remaining production-code finding in this follow-up; readiness is waiting on that regression proof.

@thymikee
thymikee force-pushed the t3code/fix-2538-quality-review branch from a9a4b8e to 3d58ec2 Compare September 14, 2026 07:20
@thymikee

Copy link
Copy Markdown
Member Author

Regression proof at 3d58ec2917. The test is now an open that booted the device keeps the device against a foreign open.

Preparation does the booting: the fixture's platform-tools readiness seam is armed before the reopen, so the fake device's boot instant lands strictly between the pre-preparation renewal and the post-preparation one, and every later probe reports it. The foreign open now runs through its own session store and lease registry, so nothing refuses it before reboot-based settlement.

Removing only the post-preparation renewal fails it twice over: expected 1789370356079 to be greater than or equal to 1789370356154 on the claim stamp, and with that assertion set aside the foreign open returns ok: true and the claim names boot-foreign — the takeover the fix exists to prevent. Restoring it makes the file green again (18 tests).

thymikee and others added 2 commits September 14, 2026 10:20
…lidated

An open that found a claim belonging to another session gave up even when
the device had rebooted since that claim was taken, leaving the surface
unreachable for every session. A reboot already took the app and the runner
away, so the claim guarded nothing.

Ask the device when its current boot began and release a foreign claim whose
stamp predates it. The stamp is the last instant the owner vouched for the
device, renewed by every open that reaches it, including the one that boots
the device on the way in, so an owner that boots the device for its own work
keeps it and only an owner that never came back loses it.

Co-Authored-By: Claude <noreply@anthropic.com>
@thymikee
thymikee force-pushed the t3code/fix-2538-quality-review branch from 3d58ec2 to 6e243cb Compare September 14, 2026 08:21
@thymikee

Copy link
Copy Markdown
Member Author

Regression proof is in at 6e243cb754, and the routed claim tests now live in request-router-open-claim.test.ts: request-router-open.test.ts reached 1,009 lines and the 1,000-line test-file ratchet (scripts/__tests__/test-file-size-ratchet.test.ts) forbids a file over the tripwire from growing, so the split is forced by the gate rather than taste.

The boot-during-preparation test arms the fixture's platform-tools readiness seam before the reopen, so the fake device's boot instant falls strictly between the pre- and post-preparation renewals and every later probe reports it; the foreign open runs through its own session store and lease registry, so nothing refuses it before reboot-based settlement. Deleting only the post-preparation renewal fails it twice: expected 1789373943292 to be greater than or equal to 1789373943381 on the stamp, and with that assertion set aside the foreign open returns ok: true and the claim names boot-foreign.

One honest delta: the layer is 1,065 gross lines, not 924. The extra 65 is the new test module's own harness, which the regression the review asked for needs. If you want it back under 1,000 I would rather delete the settlement-level duplicate takes a live foreign claim whose device rebooted after the claim was taken than cut coverage — say which you prefer.

@thymikee

thymikee commented Sep 14, 2026

Copy link
Copy Markdown
Member Author

The regression gap is resolved at 3d58ec2: preparation supplies the boot timestamp, the foreign open uses a separate store, and the reported red run reproduces takeover when post-preparation renewal is removed. No remaining correctness or device-evidence finding on that revision. At that revision, merge still needed the coverage gate fixed: request-router-open.test.ts grew to 1,009 lines and must be split along its owning module. The iOS smoke failure is likely unrelated: it repeats the automation-longpress scroll-visibility failure, while this follow-up changes only the regression test.

A newer head, 6e243cb, arrived before this comment was published. This verdict applies only to 3d58ec2; the new head still needs review.

@thymikee thymikee added ready-for-human Valid work that needs human implementation, judgment, or maintainer merge and removed ready-for-human Valid work that needs human implementation, judgment, or maintainer merge labels Sep 14, 2026
@thymikee
thymikee merged commit 1e50f96 into main Sep 14, 2026
21 checks passed
@thymikee
thymikee deleted the t3code/fix-2538-quality-review branch September 14, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A session claim on a device that was shut down and rebooted since the claim should not block open with DEVICE_IN_USE

1 participant