diff --git a/src/daemon/__tests__/device-claim-admission.test.ts b/src/daemon/__tests__/device-claim-admission.test.ts index 8540a0740..fdf7f6c25 100644 --- a/src/daemon/__tests__/device-claim-admission.test.ts +++ b/src/daemon/__tests__/device-claim-admission.test.ts @@ -9,7 +9,7 @@ import { retainOrphanedDeviceClaims, } from '../../__tests__/test-utils/device-claim-store.ts'; import { createDeviceClaimAdmission } from '../device-claim-admission.ts'; -import { acquireDeviceClaim } from '../device-claims.ts'; +import { abandonDeviceClaim, acquireDeviceClaim } from '../device-claims.ts'; import { inspectDeviceClaims } from '../device-claim-inspection.ts'; import { createRequestExecutionScope } from '../request-execution-scope.ts'; import { LeaseRegistry } from '../lease-registry.ts'; @@ -109,6 +109,28 @@ test('a claim already held by this daemon covers the command instead of collidin expect(claimedSessions()).toEqual(['open-session']); }); +test("an abandoned claim becomes this command's own transient claim and is released", async () => { + const { stateDir } = setup(); + const aborted = await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'aborted-open', + workspace: '/worktrees/current', + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + expect(aborted.status).toBe('acquired'); + if (aborted.status !== 'acquired') return; + expect(await abandonDeviceClaim(aborted.ownership)).toBe('abandoned'); + + const admission = makeAdmission('transient-exclusive', stateDir, 'install'); + await admission.admit(ANDROID_EMULATOR, localAndroid); + + // Coverage would have left the abandoned record owning the device with nothing to release it. + expect(claimedSessions()).toEqual(['transient:install']); + await admission[Symbol.asyncDispose](); + expect(inspectDeviceClaims({})).toEqual([]); +}); + test('a provider-owned device takes no host-local claim', async () => { const { stateDir, claimsDir } = setup(); const admission = makeAdmission('transient-exclusive', stateDir); diff --git a/src/daemon/__tests__/device-claims.test.ts b/src/daemon/__tests__/device-claims.test.ts index 995964636..b4800b282 100644 --- a/src/daemon/__tests__/device-claims.test.ts +++ b/src/daemon/__tests__/device-claims.test.ts @@ -4,6 +4,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { afterEach, test, vi } from 'vitest'; import { + abandonDeviceClaim, acquireDeviceClaim as acquireProductionDeviceClaim, clearDeviceClaim, } from '../device-claims.ts'; @@ -528,3 +529,108 @@ test('never treats a claim owned by the inspecting process as superseded', async assert.equal(second.conflict.classification, 'live'); assert.equal(reconcile.mock.calls.length, 0); }); + +test('an abandoned claim yields to the next acquire of the daemon that abandoned it', async () => { + const root = useClaimsRoot(); + const aborted = await acquireDeviceClaim({ + device, + session: 'attempt-1', + workspace: '/worktrees/suite', + stateDir: root, + }); + assert.equal(aborted.status, 'acquired'); + if (aborted.status !== 'acquired') return; + assert.equal(await abandonDeviceClaim(aborted.ownership), 'abandoned'); + assert.equal( + typeof inspectDeviceClaims({ serial: device.id })[0]?.claim?.abandonedAtMs, + 'number', + ); + const reconcile = vi.fn(async () => ({ status: 'reconciled' as const })); + + const retry = await acquireDeviceClaim({ + device, + session: 'attempt-2', + workspace: '/worktrees/suite', + stateDir: root, + reconcileOrphanedDeviceClaim: reconcile, + }); + + assert.equal(retry.status, 'acquired'); + const claim = inspectDeviceClaims({ serial: device.id })[0]?.claim; + assert.equal(claim?.session, 'attempt-2'); + assert.equal(claim?.abandonedAtMs, undefined); + // Abandonment is an owner's own record, not a proof about a dead owner: it never + // routes through orphan reconciliation. + assert.equal(reconcile.mock.calls.length, 0); + assert.equal(await clearDeviceClaim(aborted.ownership), 'ownership-changed'); +}); + +test('an abandoned claim keeps fencing a daemon that does not own it', async () => { + const root = useClaimsRoot(); + const stateDir = path.join(root, 'foreign-state'); + await seedForeignLiveClaim(root, stateDir); + const stored = JSON.parse(fs.readFileSync(claimPath(root), 'utf8')) as Record; + fs.writeFileSync(claimPath(root), JSON.stringify({ ...stored, abandonedAtMs: 1 })); + const reconcile = vi.fn(async () => ({ status: 'reconciled' as const })); + + const second = await acquireDeviceClaim({ + device, + session: 'other', + workspace: '/w', + stateDir, + reconcileOrphanedDeviceClaim: reconcile, + }); + + assert.equal(second.status, 'conflict'); + if (second.status !== 'conflict') return; + assert.equal(second.conflict.classification, 'live'); + assert.equal(second.conflict.claim?.session, 'cwd:/w:default'); + assert.equal(reconcile.mock.calls.length, 0); +}); + +test("an abandoned claim keeps fencing this process on another daemon's state dir", async () => { + const root = useClaimsRoot(); + const acquired = await acquireDeviceClaim({ + device, + session: 'attempt-1', + workspace: '/worktrees/suite', + stateDir: path.join(root, 'owner-state'), + }); + assert.equal(acquired.status, 'acquired'); + if (acquired.status !== 'acquired') return; + assert.equal(await abandonDeviceClaim(acquired.ownership), 'abandoned'); + + const second = await acquireDeviceClaim({ + device, + session: 'attempt-2', + workspace: '/worktrees/suite', + stateDir: path.join(root, 'other-state'), + }); + + assert.equal(second.status, 'conflict'); + if (second.status !== 'conflict') return; + assert.equal(second.conflict.claim?.session, 'attempt-1'); +}); + +test('reports the exact outcome of abandoning an owned, missing, and unowned claim', async () => { + const root = useClaimsRoot(); + const acquired = await acquireDeviceClaim({ + device, + session: 'owner', + workspace: '/worktrees/owner', + stateDir: root, + }); + assert.equal(acquired.status, 'acquired'); + if (acquired.status !== 'acquired') return; + + assert.equal(await abandonDeviceClaim(acquired.ownership), 'abandoned'); + const stored = JSON.parse(fs.readFileSync(claimPath(root), 'utf8')) as Record; + fs.writeFileSync( + claimPath(root), + JSON.stringify({ ...stored, ownerToken: 'successor-token', session: 'successor' }), + ); + assert.equal(await abandonDeviceClaim(acquired.ownership), 'ownership-changed'); + fs.rmSync(claimPath(root)); + assert.equal(await abandonDeviceClaim(acquired.ownership), 'absent'); + assert.equal(await abandonDeviceClaim(undefined), 'absent'); +}); diff --git a/src/daemon/device-claim-inspection.ts b/src/daemon/device-claim-inspection.ts index 0b5d94107..375270e95 100644 --- a/src/daemon/device-claim-inspection.ts +++ b/src/daemon/device-claim-inspection.ts @@ -278,10 +278,15 @@ function decodeClaimOwner( function decodeClaimTimestamps( raw: Record, -): Pick | null { - const { createdAtMs, updatedAtMs } = raw; +): Pick | null { + const { createdAtMs, updatedAtMs, abandonedAtMs } = raw; if (!isFiniteNumber(createdAtMs) || !isFiniteNumber(updatedAtMs)) return null; - return { createdAtMs, updatedAtMs }; + if (abandonedAtMs !== undefined && !isFiniteNumber(abandonedAtMs)) return null; + return { + createdAtMs, + updatedAtMs, + ...(isFiniteNumber(abandonedAtMs) ? { abandonedAtMs } : {}), + }; } function isNonEmptyString(value: unknown): value is string { diff --git a/src/daemon/device-claims.ts b/src/daemon/device-claims.ts index c84d3fa0f..8951dad0f 100644 --- a/src/daemon/device-claims.ts +++ b/src/daemon/device-claims.ts @@ -40,6 +40,8 @@ export type DeviceClaim = { ownerToken: string; createdAtMs: number; updatedAtMs: number; + /** Set by {@link abandonDeviceClaim}; absent while the claim still holds the device for its owner. */ + abandonedAtMs?: number; }; export type DeviceClaimReconciliationResult = @@ -102,10 +104,11 @@ export async function acquireDeviceClaim(params: { /** * #1320 `transient-exclusive`: exclusive ownership for the duration of one * sessionless device mutation. Identical to a session claim except that a claim - * already held by this daemon process covers the command instead of colliding - * with it — the claim file itself, not the in-memory session table, is the - * authority for that, so a claim acquired earlier in the same request (`open`'s, - * for instance) can never lock the daemon out of its own device. + * this daemon still holds covers the command instead of colliding with it — the + * claim file itself, not the in-memory session table, is the authority for that, + * so a claim acquired earlier in the same request (`open`'s, for instance) can + * never lock the daemon out of its own device. An abandoned claim holds nothing, + * so it is superseded into this command's own transient claim instead. */ export async function acquireTransientDeviceClaim(params: { device: DeviceInfo; @@ -120,6 +123,7 @@ export async function acquireTransientDeviceClaim(params: { const existing = inspectDeviceClaimFile(resolveDeviceClaimPath(deviceKey)); if ( existing?.claim && + !isAbandonedDeviceClaim(existing.claim) && isClaimOwnedByThisDaemon(existing.claim, params.stateDir, readCurrentOwnerIdentity()) ) { return { status: 'covered-by-owned-claim' }; @@ -189,6 +193,18 @@ function isClaimOwnedByThisDaemon( ); } +function isAbandonedDeviceClaim(claim: DeviceClaim): boolean { + return claim.abandonedAtMs !== undefined; +} + +function isAbandonedClaimOfThisDaemon( + claim: DeviceClaim, + stateDir: string, + owner: ReturnType, +): boolean { + return isAbandonedDeviceClaim(claim) && isClaimOwnedByThisDaemon(claim, stateDir, owner); +} + function deviceClaimIdentity(device: DeviceInfo): DeviceIdentity { return deviceIdentity({ ...device, @@ -206,6 +222,13 @@ async function resolveExistingClaim(params: { }): Promise { const existing = inspectDeviceClaimFile(resolveDeviceClaimPath(params.deviceKey)); if (!existing) return { status: 'available' }; + if ( + existing.claim && + isAbandonedClaimOfThisDaemon(existing.claim, params.stateDir, params.owner) + ) { + emitClaimSupersede(params.deviceKey, existing.claim); + return { status: 'available' }; + } if (existing.claim && isCurrentClaimOwner(existing.claim, params, params.owner)) { return { status: 'acquired', ownership: ownershipFromClaim(existing.claim) }; } @@ -258,16 +281,7 @@ export async function clearDeviceClaim( const inspected = inspectDeviceClaimFile(claimPath); if (!inspected) return 'absent'; const claim = inspected.claim; - if ( - !claim || - claim.ownerToken !== ownership.ownerToken || - !ownerIdentityMatches( - { pid: claim.ownerPid, startTime: claim.ownerStartTime }, - { pid: ownership.ownerPid, startTime: ownership.ownerStartTime }, - ) - ) { - return 'ownership-changed'; - } + if (!claim || !claimMatchesOwnership(claim, ownership)) return 'ownership-changed'; try { fs.unlinkSync(claimPath); } catch (error) { @@ -278,6 +292,47 @@ export async function clearDeviceClaim( }); } +/** + * What abandoning a claim did, in the terms {@link DeviceClaimClearOutcome} uses: + * + * - `abandoned` — the claim we acquired now holds the device for nobody. + * - `absent` — no claim remains for the device; nothing to mark. + * - `ownership-changed`— a claim remains, but it is not the one we acquired. + */ +export type DeviceClaimAbandonOutcome = 'abandoned' | 'absent' | 'ownership-changed'; + +/** + * Keeps the device fenced against every other owner while recording that this claim holds it for + * nobody. Only the daemon that abandoned it may take it back. + */ +export async function abandonDeviceClaim( + ownership: DeviceClaimSessionOwnership | undefined, +): Promise { + if (!ownership) return 'absent'; + return await withDeviceClaimLock(ownership.deviceKey, async () => { + const inspected = inspectDeviceClaimFile(resolveDeviceClaimPath(ownership.deviceKey)); + if (!inspected) return 'absent'; + const claim = inspected.claim; + if (!claim || !claimMatchesOwnership(claim, ownership)) return 'ownership-changed'; + const now = Date.now(); + writeClaim({ ...claim, abandonedAtMs: now, updatedAtMs: now }); + return 'abandoned'; + }); +} + +function claimMatchesOwnership( + claim: DeviceClaim, + ownership: DeviceClaimSessionOwnership, +): boolean { + return ( + claim.ownerToken === ownership.ownerToken && + ownerIdentityMatches( + { pid: claim.ownerPid, startTime: claim.ownerStartTime }, + { pid: ownership.ownerPid, startTime: ownership.ownerStartTime }, + ) + ); +} + /** * Reconciles claims whose owner can no longer release them and clears only * claims whose attributable durable resources reached a safe terminal state. @@ -387,6 +442,18 @@ function emitClaimConflict( }); } +function emitClaimSupersede(deviceKey: string, abandoned: DeviceClaim): void { + emitDiagnostic({ + level: 'info', + phase: 'device_claim_abandoned_superseded', + data: { + deviceKey, + abandonedSession: abandoned.session, + abandonedAtMs: abandoned.abandonedAtMs, + }, + }); +} + async function settleVerifiedOrphanedClaim( claim: DeviceClaim, reconcile: DeviceClaimReconciler, diff --git a/src/daemon/handlers/__tests__/session-device-claims.test.ts b/src/daemon/handlers/__tests__/session-device-claims.test.ts index b6ddcea2d..221970df0 100644 --- a/src/daemon/handlers/__tests__/session-device-claims.test.ts +++ b/src/daemon/handlers/__tests__/session-device-claims.test.ts @@ -181,7 +181,9 @@ test('failed local open after dispatch retains its device claim for recovery', a }), (error: unknown) => error === rejectionError, ); - assert.equal(inspectDeviceClaims({ serial: android.id })[0]?.classification, 'live'); + const retained = inspectDeviceClaims({ serial: android.id })[0]; + assert.equal(retained?.classification, 'live'); + assert.equal(typeof retained?.claim?.abandonedAtMs, 'number'); }); test('failed local runtime-hint setup retains its device claim before open dispatch', async () => { @@ -271,6 +273,55 @@ test('cancellation after local device setup retains the device claim for recover } }); +test('a canceled attempt lets the next attempt of the same suite open the device', async () => { + const { store, stateDir } = setup(); + const requestId = 'suite:1-gesture-pan-duration:attempt:1'; + mockResolveTargetDevice.mockResolvedValue(android); + mockDispatch.mockResolvedValue(undefined); + markRequestCanceled(requestId); + try { + const timedOut = await handleOpenCommand({ + req: { + command: 'open', + token: 'test', + session: 'suite:1-gesture-pan-duration:attempt-1', + positionals: ['Demo'], + flags: { platform: 'android' }, + meta: { requestId }, + }, + sessionName: 'suite:1-gesture-pan-duration:attempt-1', + logPath: path.join(stateDir, 'daemon.log'), + sessionStore: store, + }); + assert.equal(timedOut.ok, false); + } finally { + clearRequestCanceled(requestId); + } + assert.equal(store.get('suite:1-gesture-pan-duration:attempt-1'), undefined); + + const retry = await handleOpenCommand({ + req: { + command: 'open', + token: 'test', + session: 'suite:1-gesture-pan-duration:attempt-2', + positionals: ['Demo'], + flags: { platform: 'android' }, + }, + sessionName: 'suite:1-gesture-pan-duration:attempt-2', + logPath: path.join(stateDir, 'daemon.log'), + sessionStore: store, + }); + + assert.equal(retry.ok, true); + const claim = inspectDeviceClaims({ serial: android.id })[0]?.claim; + assert.equal(claim?.session, 'suite:1-gesture-pan-duration:attempt-2'); + assert.equal(claim?.abandonedAtMs, undefined); + assert.equal( + store.get('suite:1-gesture-pan-duration:attempt-2')?.deviceClaim?.ownerToken, + claim?.ownerToken, + ); +}); + test('provider-owned open creates no host-local device claim from its selected owner', async () => { const { store, stateDir } = setup(); mockResolveTargetDevice.mockResolvedValue(android); diff --git a/src/daemon/handlers/session-open-execution.ts b/src/daemon/handlers/session-open-execution.ts index 0144926d8..64554fcad 100644 --- a/src/daemon/handlers/session-open-execution.ts +++ b/src/daemon/handlers/session-open-execution.ts @@ -46,6 +46,7 @@ import { import { resolveSessionLeaseForRequest } from '../lease-lifecycle.ts'; import { applicationLifecycleExecutionFromRequest } from '../application-lifecycle-execution.ts'; import { + abandonDeviceClaim, acquireDeviceClaim, clearDeviceClaim, isLocalDeviceClaimTarget, @@ -438,6 +439,13 @@ export async function openNewSessionWithDeviceClaim(params: { } const deviceClaim = localClaim.status === 'acquired' ? localClaim.ownership : undefined; const effects: NewSessionOpenEffects = { mayHaveStarted: false }; + const rollbackClaim = async () => + await rollbackNewSessionClaim({ + ownership: deviceClaim, + effects, + sessionName, + sessionStore, + }); try { const details = await prepareOpenCommandDetails({ req, @@ -450,7 +458,7 @@ export async function openNewSessionWithDeviceClaim(params: { foreground: req.flags?.foreground === true && openTarget === undefined, }); if (details.type === 'response') { - await rollbackNewSessionClaim(deviceClaim, effects); + await rollbackClaim(); return details.response; } // Preparation can boot the device or warm caches, but it cannot establish session ownership. @@ -484,26 +492,31 @@ export async function openNewSessionWithDeviceClaim(params: { deviceClaim, selection, }); - if (!response.ok) await rollbackNewSessionClaim(deviceClaim, effects); + if (!response.ok) await rollbackClaim(); return response; } catch (error) { - await rollbackNewSessionClaim(deviceClaim, effects); + await rollbackClaim(); throw error; } } -async function rollbackNewSessionClaim( - ownership: DeviceClaimSessionOwnership | undefined, - effects: NewSessionOpenEffects, -): Promise { +async function rollbackNewSessionClaim(params: { + ownership: DeviceClaimSessionOwnership | undefined; + effects: NewSessionOpenEffects; + sessionName: string; + sessionStore: SessionStore; +}): Promise { + const { ownership, effects, sessionName, sessionStore } = params; if (!ownership) return; - if (effects.mayHaveStarted) { - emitDiagnostic({ - level: 'warn', - phase: 'device_claim_open_effects_unconfirmed', - data: { deviceKey: ownership.deviceKey }, - }); + if (!effects.mayHaveStarted) { + await clearDeviceClaim(ownership); return; } - await clearDeviceClaim(ownership); + if (sessionStore.get(sessionName)?.deviceClaim?.ownerToken === ownership.ownerToken) return; + const outcome = await abandonDeviceClaim(ownership); + emitDiagnostic({ + level: 'warn', + phase: 'device_claim_open_effects_unconfirmed', + data: { deviceKey: ownership.deviceKey, outcome }, + }); }