diff --git a/packages/daemon-contracts/src/device-claim-reboot.test.ts b/packages/daemon-contracts/src/device-claim-reboot.test.ts index a41ab90309..aa83669d4d 100644 --- a/packages/daemon-contracts/src/device-claim-reboot.test.ts +++ b/packages/daemon-contracts/src/device-claim-reboot.test.ts @@ -18,7 +18,8 @@ const device: DeviceInfo = { }; const CLAIM: DeviceClaim = { - schemaVersion: 2, + schemaVersion: 3, + claimOwnerKind: 'open-session', deviceKey: 'local:apple:ios:SIM-1', device: { family: 'apple', id: device.id, name: device.name, kind: device.kind }, session: 'cwd:/w:default', diff --git a/packages/daemon-contracts/src/device-claim-reconciliation.test.ts b/packages/daemon-contracts/src/device-claim-reconciliation.test.ts index ebbe776150..8596db4d48 100644 --- a/packages/daemon-contracts/src/device-claim-reconciliation.test.ts +++ b/packages/daemon-contracts/src/device-claim-reconciliation.test.ts @@ -146,7 +146,8 @@ const unavailable = Object.freeze({ function makeClaim(stateDir: string): DeviceClaim { return { - schemaVersion: 2, + schemaVersion: 3, + claimOwnerKind: 'open-session', deviceKey: 'local:android:none:emulator-5554', device: { family: 'android', id: 'emulator-5554', name: 'Pixel', kind: 'emulator' }, session: 'claimed-session', diff --git a/packages/daemon-contracts/src/device-claim-record.test.ts b/packages/daemon-contracts/src/device-claim-record.test.ts index d56ff0962e..2c5ea6ddc9 100644 --- a/packages/daemon-contracts/src/device-claim-record.test.ts +++ b/packages/daemon-contracts/src/device-claim-record.test.ts @@ -20,7 +20,7 @@ function legacyClaim(overrides: Record = {}): Record = {}): Record { +function v2Claim(overrides: Record = {}): Record { return { ...legacyClaim(), schemaVersion: 2, @@ -29,24 +29,76 @@ function currentClaim(overrides: Record = {}): Record { - for (const raw of [legacyClaim(), currentClaim()]) { +function currentClaim(overrides: Record = {}): Record { + return { + ...v2Claim(), + schemaVersion: 3, + claimOwnerKind: 'open-session', + ...overrides, + }; +} + +test('v1, v2, and v3 records decode to the process-owned claim with its process principal intact', () => { + for (const raw of [legacyClaim(), v2Claim(), currentClaim()]) { const record = decodeStoredDeviceClaim(raw); expect(record).not.toBeNull(); if (!record) throw new Error('expected process-owned'); - expect(record.schemaVersion).toBe(2); + expect(record.schemaVersion).toBe(3); expect(record.session).toBe('work'); expect(record.ownerPid).toBe(4242); expect(record.ownerToken).toBe('token'); + expect(record.transientCommand).toBeUndefined(); + } +}); + +// Released v1/v2 recorded a command-scoped claim and an open session *named* `transient:` +// as the same record, so no stored field can say which one a legacy record was. Such a record +// still decodes — clearing, reconciliation, and stale settlement need its principal — but it +// states no kind, so a migrated record can never cover a command. +test('a migrated record normalizes to the current schema and states no claim kind', () => { + for (const raw of [ + legacyClaim({ session: 'transient:install' }), + v2Claim({ session: 'transient:install' }), + ]) { + const record = decodeStoredDeviceClaim(raw); + expect(record).not.toBeNull(); + if (!record) throw new Error('expected a decoded claim'); + expect(record.schemaVersion).toBe(3); + expect(record.session).toBe('transient:install'); + expect(record.claimOwnerKind).toBe('unstated'); + expect(record.transientCommand).toBeUndefined(); } +}); + +// A current record states its kind, and the kind fields have to agree. An open session may +// legitimately be named after a command; a malformed kind fails the record closed. +test('a current-version claim kind comes only from its stored fields', () => { + const named = decodeStoredDeviceClaim(currentClaim({ session: 'transient:install' })); + expect(named).not.toBeNull(); + if (!named) throw new Error('expected a decoded claim'); + expect(named.claimOwnerKind).toBe('open-session'); + expect(named.transientCommand).toBeUndefined(); + + const scoped = decodeStoredDeviceClaim( + currentClaim({ claimOwnerKind: 'transient-command', transientCommand: 'install' }), + ); + expect(scoped).not.toBeNull(); + if (!scoped) throw new Error('expected a decoded claim'); + expect(scoped.claimOwnerKind).toBe('transient-command'); + expect(scoped.transientCommand).toBe('install'); + + expect(decodeStoredDeviceClaim(currentClaim({ transientCommand: 'install' }))).toBeNull(); expect( - decodeStoredDeviceClaim(legacyClaim({ session: 'transient:install' }))?.schemaVersion, - ).toBe(2); + decodeStoredDeviceClaim( + currentClaim({ claimOwnerKind: 'transient-command', transientCommand: '' }), + ), + ).toBeNull(); + expect(decodeStoredDeviceClaim(currentClaim({ claimOwnerKind: 'bogus' }))).toBeNull(); }); test('a record of an unknown schema version is unreadable', () => { // A future-schema record carrying a whole process principal is still not migrated into a claim. - for (const schemaVersion of [0, 3, 4]) { + for (const schemaVersion of [0, 4, 5]) { expect(decodeStoredDeviceClaim(currentClaim({ schemaVersion }))).toBeNull(); } expect(decodeStoredDeviceClaim(null)).toBeNull(); diff --git a/packages/daemon-contracts/src/device-claim-record.ts b/packages/daemon-contracts/src/device-claim-record.ts index 974e1400fc..bce2d0b752 100644 --- a/packages/daemon-contracts/src/device-claim-record.ts +++ b/packages/daemon-contracts/src/device-claim-record.ts @@ -8,15 +8,30 @@ import { isRecord } from '@agent-device/kernel/record'; import { canonicalLocalDeviceKey } from './device-claim-paths.ts'; -/** Schema version of a process-owned claim record; v1 records migrate to it on read. */ -export const DEVICE_CLAIM_SCHEMA_VERSION = 2; +/** Schema version of a process-owned claim record; v1/v2 records normalize to it on read. */ +export const DEVICE_CLAIM_SCHEMA_VERSION = 3; + +/** + * What a claim's record states about who owns it. A current writer always states one. + * `unstated` is what a record migrated from the released v1/v2 schemas decodes to, because those + * schemas wrote a command-scoped claim and an open session *named* `transient:` + * identically; the session label is never re-read as the discriminant. A claim that states no kind + * can hold and fence a device but can never cover a command. + */ +export type DeviceClaimOwnerKind = 'open-session' | 'transient-command' | 'unstated'; + +const DEVICE_CLAIM_OWNER_KINDS: readonly DeviceClaimOwnerKind[] = [ + 'open-session', + 'transient-command', + 'unstated', +]; /** * A claim owned by one process: the session or the sessionless command that took it. Its principal * is `ownerPid`/`ownerStartTime`/`ownerToken`, which is what every clearing surface matches on. */ export type DeviceClaim = { - schemaVersion: 2; + schemaVersion: 3; deviceKey: string; device: DeviceIdentity & { name: string }; session: string; @@ -27,6 +42,15 @@ export type DeviceClaim = { ownerToken: string; createdAtMs: number; updatedAtMs: number; + /** + * The persisted claim-kind discriminant (#3345). Coverage reads this and never the `session` + * label: an open session may legitimately be named `transient:`, and any printed + * address can be pasted back into `--session`. It is stored rather than derived from the schema + * version so a record renewed or abandoned after migration keeps saying the same thing. + */ + claimOwnerKind: DeviceClaimOwnerKind; + /** The command a `transient-command` claim is scoped to; present for exactly that kind. */ + transientCommand?: string; /** Set by `abandonDeviceClaim`; absent while the claim still holds the device for its owner. */ abandonedAtMs?: number; }; @@ -52,20 +76,53 @@ export function ownershipFromClaim(claim: DeviceClaim): DeviceClaimSessionOwners }; } +/** + * Every stored record normalizes to the current schema so that a renewal or an abandon can write a + * decoded claim back without changing its shape. A v1/v2 record has no kind field, and the session + * label cannot supply one, so it decodes as `unstated`: it keeps its full principal for clearing, + * reconciliation, and stale settlement, and states no ownership kind. + */ export function decodeStoredDeviceClaim(value: unknown): DeviceClaim | null { if (!isRecord(value)) return null; if (value.schemaVersion === DEVICE_CLAIM_SCHEMA_VERSION) return decodeCurrentClaim(value); + if (value.schemaVersion === 2) return decodeClaimBody(value, 'unstated'); if (value.schemaVersion === 1) return migrateLegacyClaim(value); return null; } +/** + * A current record states its kind, and the two kind fields have to agree: a command claim names + * its command, any other kind names none. Anything else fails the record closed rather than + * guessing which owner a session label suggests. + */ function decodeCurrentClaim(raw: Record): DeviceClaim | null { + const { claimOwnerKind, transientCommand } = raw; + if (!isKnownOwnerKind(claimOwnerKind)) return null; + if (claimOwnerKind === 'transient-command') { + if (!isNonEmptyString(transientCommand)) return null; + const claim = decodeClaimBody(raw, claimOwnerKind); + return claim ? { ...claim, transientCommand } : null; + } + if (transientCommand !== undefined) return null; + return decodeClaimBody(raw, claimOwnerKind); +} + +function isKnownOwnerKind(value: unknown): value is DeviceClaimOwnerKind { + return ( + typeof value === 'string' && (DEVICE_CLAIM_OWNER_KINDS as readonly string[]).includes(value) + ); +} + +function decodeClaimBody( + raw: Record, + claimOwnerKind: DeviceClaimOwnerKind, +): DeviceClaim | null { if (!isRecord(raw.device) || !isNonEmptyString(raw.device.name)) return null; const identity = decodeDeviceIdentity(raw.device); - return identity ? buildDecodedClaim(raw, identity, raw.device.name) : null; + return identity ? decodeClaimFields(raw, identity, raw.device.name, claimOwnerKind) : null; } -/** Released schema-v1 advisory claims are normalized into the canonical v2 model on read. */ +/** Released schema-v1 claims differ only in device shape; they state no claim kind. */ function migrateLegacyClaim(raw: Record): DeviceClaim | null { if (!isRecord(raw.device)) return null; const legacy = raw.device; @@ -87,13 +144,14 @@ function migrateLegacyClaim(raw: Record): DeviceClaim | null { ? {} : { iosPhysicalDeviceBackend: legacy.iosPhysicalDeviceBackend }), }); - return identity ? buildDecodedClaim(raw, identity, name) : null; + return identity ? decodeClaimFields(raw, identity, name, 'unstated') : null; } -function buildDecodedClaim( +function decodeClaimFields( raw: Record, identity: DeviceIdentity, name: string, + claimOwnerKind: DeviceClaimOwnerKind, ): DeviceClaim | null { const location = decodeClaimLocation(raw); const owner = decodeClaimOwner(raw); @@ -106,9 +164,20 @@ function buildDecodedClaim( device: { ...identity, name }, ...owner, ...timestamps, + claimOwnerKind, }; } +/** + * The claim `session` recorded for a command-scoped claim. Lives with the record so the label and + * its inverse stay one owner; `claimOwnerKind` is what decides a kind, never this shape. + */ +export function transientDeviceClaimSession(command: string): string { + return `${TRANSIENT_CLAIM_LABEL_PREFIX}${command}`; +} + +const TRANSIENT_CLAIM_LABEL_PREFIX = 'transient:'; + function decodeClaimLocation( raw: Record, ): Pick | null { diff --git a/packages/daemon-contracts/src/device-claim-settlement.ts b/packages/daemon-contracts/src/device-claim-settlement.ts index 93a4ff3c0f..cadc58543f 100644 --- a/packages/daemon-contracts/src/device-claim-settlement.ts +++ b/packages/daemon-contracts/src/device-claim-settlement.ts @@ -21,6 +21,7 @@ import { resolveDeviceClaimPath } from './device-claim-paths.ts'; import { ownershipFromClaim, type DeviceClaim, + type DeviceClaimOwnerKind, type DeviceClaimSessionOwnership, } from './device-claim-record.ts'; import { writeDeviceClaim } from './device-claim-store.ts'; @@ -43,6 +44,13 @@ export type DeviceClaimReconciler = ( claim: DeviceClaim, ) => Promise; +/** + * The acquirer whose ownership `resolveExistingClaim` judges: the kind of claim it could have + * written itself — never `unstated`, which no acquirer can be — plus the command a transient + * acquirer scopes to. + */ +export type DeviceClaimAcquirerKind = Exclude; + /** * Settles the claim file an acquisition found, and decides whether this caller may write its own. * The recorded owner's own state answers first — a dead, unreachable, or superseded owner is @@ -57,6 +65,10 @@ export async function resolveExistingClaim(params: { session: string; workspace: string; stateDir: string; + /** The kind of claim this acquirer would write, so `isCurrentClaimOwner` can compare kinds. */ + claimOwnerKind: DeviceClaimAcquirerKind; + /** The command a `transient-command` acquirer scopes to; absent exactly when the kind is. */ + transientCommand?: string; reconcileOrphanedDeviceClaim: DeviceClaimReconciler; observeDeviceBoot?: DeviceBootObservationService; }): Promise { @@ -187,11 +199,31 @@ function isAbandonedClaimOfThisDaemon( return isAbandonedDeviceClaim(claim) && isClaimOwnedByThisDaemon(claim, stateDir, owner); } +/** + * An acquirer holds an existing claim only when the record states the very kind of claim this + * acquirer would write itself — the same `transientCommand` for transient admission — under the + * same session, workspace, and daemon. `unstated` matches no acquirer, so a migrated record can + * hold and fence the device but can never be adopted at acquisition, however exactly its session + * label or copyable `--session` address lines up (#3345). + */ function isCurrentClaimOwner( claim: DeviceClaim, - params: { session: string; workspace: string; stateDir: string }, + params: { + session: string; + workspace: string; + stateDir: string; + claimOwnerKind: DeviceClaimAcquirerKind; + transientCommand?: string; + }, owner: ReturnType, ): boolean { + if (claim.claimOwnerKind !== params.claimOwnerKind) return false; + if ( + params.claimOwnerKind === 'transient-command' && + claim.transientCommand !== params.transientCommand + ) { + return false; + } return ( claim.session === params.session && claim.workspace === params.workspace && diff --git a/packages/daemon-contracts/src/device-claims.ts b/packages/daemon-contracts/src/device-claims.ts index 2fbd71f0db..012ea48437 100644 --- a/packages/daemon-contracts/src/device-claims.ts +++ b/packages/daemon-contracts/src/device-claims.ts @@ -29,6 +29,7 @@ import { import { DEVICE_CLAIM_SCHEMA_VERSION, ownershipFromClaim, + transientDeviceClaimSession, type DeviceClaim, type DeviceClaimSessionOwnership, } from './device-claim-record.ts'; @@ -37,6 +38,7 @@ import { isClaimOwnedByThisDaemon, resolveExistingClaim, settleVerifiedOrphanedClaim, + type DeviceClaimAcquirerKind, type DeviceClaimReconciler, } from './device-claim-settlement.ts'; import { withDeviceClaimLock, writeDeviceClaim } from './device-claim-store.ts'; @@ -67,23 +69,16 @@ export type DeviceClaimAcquireResult = | { status: 'conflict'; conflict: InspectedDeviceClaim }; /** - * A `transient-exclusive` command may find the device already claimed by a - * session of the very daemon executing it. That session claim already carries - * the exclusion the command needs, so the command adds none of its own. + * A `transient-exclusive` command may find the device already claimed by this daemon. + * Only an open session's own claim, taken from the request's workspace, carries the exclusion + * the command needs, so that one adds none of its own. Any other claim — another session's, or + * any command-scoped claim this daemon holds in flight — is a foreign live owner (#3345): + * sessions of one daemon are separate owners of a device. */ export type TransientDeviceClaimResult = | DeviceClaimAcquireResult | { status: 'covered-by-owned-claim' }; -/** - * The claim `session` recorded for a command-scoped claim. Claim records carry - * no separate kind discriminant, so the session field is what tells `device - * status` that the owner is a command in flight rather than an open session. - */ -function transientDeviceClaimSession(command: string): string { - return `transient:${command}`; -} - export async function acquireDeviceClaim(params: { device: DeviceInfo; session: string; @@ -106,16 +101,39 @@ 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 - * 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. + * sessionless device mutation. Identical to a session claim except that the claim + * of the session this command runs in 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 session out of its own device. A claim this daemon holds for a + * *different* session collides, because #3345 established that one daemon serving + * several worktrees does not make those worktrees one owner. An abandoned claim holds + * nothing, so it is superseded into this command's own transient claim instead. + * + * Coverage proves ownership the way {@link acquireDeviceClaim} does: the covered claim + * must have been recorded by this request's workspace, not merely carry the same session + * string. Session addresses are copyable — `device status` prints them and refusal + * details carry them — so without that, a caller in another workspace could paste the + * owner's address into `--session` and pass as the owner. */ export async function acquireTransientDeviceClaim(params: { device: DeviceInfo; command: string; + /** + * The session this command runs in, i.e. the session whose own claim covers it. + * Unlike {@link acquireDeviceClaim}, this is not the session the record gets: a + * command-scoped claim stays attributable to the command that took it. + */ + coveredBySession: string; + /** + * Whether the live claim under {@link coveredBySession} was taken by this request's + * workspace rather than merely recorded under the same session string. Session + * addresses are copyable (#3345), so session equality alone lets a caller that + * pasted another workspace's address act as that workspace's owner. Only an + * `open-session` claim is ever a candidate: the record's `claimOwnerKind`, not its + * label, decides whether it can cover anything. + */ + coveredClaimIsOwnedByRequest: (claim: DeviceClaim) => boolean; workspace: string; stateDir: string; reconcileOrphanedDeviceClaim: DeviceClaimReconciler; @@ -129,13 +147,31 @@ export async function acquireTransientDeviceClaim(params: { !isAbandonedDeviceClaim(existing.claim) && isClaimOwnedByThisDaemon(existing.claim, params.stateDir, readCurrentOwnerIdentity()) ) { - return { status: 'covered-by-owned-claim' }; + const claimKind = existing.claim.claimOwnerKind; + if ( + claimKind === 'open-session' && + existing.claim.session === params.coveredBySession && + params.coveredClaimIsOwnedByRequest(existing.claim) + ) { + return { status: 'covered-by-owned-claim' }; + } + // Only a record that states its kind can take part in a kind decision. A migrated record + // states none, and `isClaimOwnedByThisDaemon` matches pid plus start time — which two + // processes can share once a start time is unreadable — so coverage has to refuse rather + // than assume an old record was not some other owner's command claim (#3345). A live claim + // of any other kind — another session's or a command's, including one labeled exactly like + // this command — refuses here, because renewing it would hand this request a token it + // never took. + if (claimKind !== 'open-session') { + return { status: 'conflict', conflict: existing }; + } } return await claimHeldDevice({ device: params.device, deviceKey, identity, session: transientDeviceClaimSession(params.command), + transientCommand: params.command, workspace: params.workspace, stateDir: params.stateDir, reconcileOrphanedDeviceClaim: params.reconcileOrphanedDeviceClaim, @@ -149,6 +185,8 @@ async function claimHeldDevice(params: { deviceKey: string; identity: DeviceIdentity; session: string; + /** The command a `transient-exclusive` acquisition scopes its claim to; absent for sessions. */ + transientCommand?: string; workspace: string; stateDir: string; reconcileOrphanedDeviceClaim: DeviceClaimReconciler; @@ -156,9 +194,12 @@ async function claimHeldDevice(params: { }): Promise { const { deviceKey, identity } = params; const owner = readCurrentOwnerIdentity(); + const claimOwnerKind: DeviceClaimAcquirerKind = + params.transientCommand === undefined ? 'open-session' : 'transient-command'; const existing = await resolveExistingClaim({ ...params, owner, + claimOwnerKind, }); if (existing.status === 'conflict') return existing; if (existing.status === 'held') return { status: 'acquired', ownership: existing.ownership }; @@ -178,6 +219,8 @@ async function claimHeldDevice(params: { ownerToken: crypto.randomUUID(), createdAtMs: now, updatedAtMs: now, + claimOwnerKind, + ...(params.transientCommand === undefined ? {} : { transientCommand: params.transientCommand }), }; writeDeviceClaim(claim); return { diff --git a/packages/daemon/src/resources/device/__tests__/device-claim-admission.test.ts b/packages/daemon/src/resources/device/__tests__/device-claim-admission.test.ts index 57830bb24d..078d435d27 100644 --- a/packages/daemon/src/resources/device/__tests__/device-claim-admission.test.ts +++ b/packages/daemon/src/resources/device/__tests__/device-claim-admission.test.ts @@ -1,5 +1,5 @@ import { ANDROID_EMULATOR, deviceClaimFilePath } from '@agent-device/test-kit'; -import { expect, test } from 'vitest'; +import { expect, test, vi } from 'vitest'; import { daemonPlatformServicesFixture, daemonClaimRecoveryGatewayFixture, @@ -7,7 +7,8 @@ import { import fs from 'node:fs'; import path from 'node:path'; import { localRuntimeOwner, providerRuntimeOwner } from '@agent-device/contracts/platform-runtime'; -import { asAppError } from '@agent-device/kernel/errors'; +import { asAppError, type AppError } from '@agent-device/kernel/errors'; +import * as hostProcess from '@agent-device/host-kit/process'; import { isolatedDeviceClaimStores, retainOrphanedDeviceClaims, @@ -20,6 +21,10 @@ import { canonicalLocalDeviceKey, } from '@agent-device/daemon-contracts/device-claims'; import { createRequestExecutionScope } from '../../../server/request-execution-scope.ts'; +import { + resolveCallerScopeRoot, + resolveEffectiveSessionName, +} from '../../../sessions/session-routing.ts'; import { LeaseRegistry } from '../../lease-registry.ts'; import { SessionStore } from '../../../core/session-store.ts'; import { unavailableDeviceRuntimeGateway } from '../../../__tests__/support/unavailable-device-runtime-gateway.ts'; @@ -28,11 +33,23 @@ import type { DeviceClaimPolicy } from '@agent-device/command-registry/types'; const setup = isolatedDeviceClaimStores('agent-device-claim-admission-'); const localAndroid = localRuntimeOwner('android'); -function makeAdmission(policy: DeviceClaimPolicy, stateDir: string, command = 'made-up-command') { +// `callerCwd: null` means the request carried no cwd at all (an SDK caller), which +// the default would otherwise hide: a default parameter fires on an explicit +// undefined too. +function makeAdmission( + policy: DeviceClaimPolicy, + stateDir: string, + command = 'made-up-command', + session = 'request-session', + callerCwd: string | null = '/worktrees/current', +) { return createDeviceClaimAdmission({ policy, command, - workspace: '/worktrees/current', + session, + workspace: callerCwd ?? '/worktrees/current', + callerCwd: callerCwd ?? undefined, + resolveScopeRoot: resolveCallerScopeRoot, stateDir, reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, }); @@ -99,7 +116,11 @@ test('a foreign live claim refuses the command before it can reach device operat expect(claimedSessions()).toEqual(['owner-session']); }); -test('a claim already held by this daemon covers the command instead of colliding with it', async () => { +// #3345: sessions of one daemon are separate owners of a device. A claim this daemon +// holds covers a `transient-exclusive` command only when the command runs in the +// session that claim belongs to; a daemon that serves more than one session or +// worktree must not let one session install over another's device. +test('a claim this daemon holds for the same session covers the command instead of colliding with it', async () => { const { stateDir } = setup(); await acquireDeviceClaim({ device: ANDROID_EMULATOR, @@ -109,7 +130,7 @@ test('a claim already held by this daemon covers the command instead of collidin reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, }); - const admission = makeAdmission('transient-exclusive', stateDir, 'install'); + const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'open-session'); await admission.admit(ANDROID_EMULATOR, localAndroid); await admission[Symbol.asyncDispose](); @@ -117,6 +138,321 @@ test('a claim already held by this daemon covers the command instead of collidin expect(claimedSessions()).toEqual(['open-session']); }); +test('a claim this daemon holds for another session refuses the command like any live owner', async () => { + const { stateDir } = setup(); + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'owning-session', + workspace: '/worktrees/current', + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + + const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'other-session'); + const error = asAppError( + await admission.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error), + ); + + expect(error.code).toBe('DEVICE_IN_USE'); + expect(error.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER'); + expect(error.details?.retriable).toBe(false); + expect(error.details?.owner).toEqual({ + session: 'owning-session', + workspace: '/worktrees/current', + stateDir, + }); + + await admission[Symbol.asyncDispose](); + // The refusal wrote nothing and released nothing: the owning session still holds + // the device with its original claim. + expect(claimedSessions()).toEqual(['owning-session']); +}); + +// Adopting a claim whose `transientCommand` is set hands the adopter the release token of a +// command that is still running, and disposing it unlinks a claim in use. `--session` accepts +// any name the store can sanitize, so a copy of the label is a reachable request; only the +// stored kind can tell the holder's own session from that copy. +test('an in-flight transient claim refuses a request named after its label, while a real open session of that name covers', async () => { + const { stateDir } = setup(); + const holder = makeAdmission('transient-exclusive', stateDir, 'install', 'open-a'); + await holder.admit(ANDROID_EMULATOR, localAndroid); + expect(claimedSessions()).toEqual(['transient:install']); + + // The intruder routes to exactly "transient:install" (an explicit `--session` does this) + // in the holder's workspace, and asks for a DIFFERENT command so the label is not its own. + const intruder = makeAdmission('transient-exclusive', stateDir, 'reinstall', 'transient:install'); + const error = asAppError( + await intruder.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error), + ); + expect(error.code).toBe('DEVICE_IN_USE'); + expect(error.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER'); + await intruder[Symbol.asyncDispose](); + // The held command still owns the device. + expect(claimedSessions()).toEqual(['transient:install']); + + await holder[Symbol.asyncDispose](); + expect(inspectDeviceClaims({})).toEqual([]); + + // A genuinely open session may carry that name; its claim is the session kind and covers + // its own workspace's command. + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'transient:install', + workspace: '/worktrees/current', + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + const own = makeAdmission('transient-exclusive', stateDir, 'install', 'transient:install'); + await own.admit(ANDROID_EMULATOR, localAndroid); + await own[Symbol.asyncDispose](); + expect(claimedSessions()).toEqual(['transient:install']); +}); + +// A claim's `session` is only a label to the code that renews a held claim, and renewing hands +// the caller that claim's release token. `install` builds the label `transient:install`, so a +// request in another session could otherwise renew an open session's claim of that name and +// delete it on release. The stored kind is what refuses it. +test("an open session named after a command's label is not renew-adopted by that command's requests", async () => { + const { stateDir } = setup(); + const open = await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'transient:install', + workspace: '/worktrees/current', + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + expect(open.status).toBe('acquired'); + if (open.status !== 'acquired') return; + + const foreign = makeAdmission('transient-exclusive', stateDir, 'install', 'other-session'); + const error = asAppError( + await foreign.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error), + ); + expect(error.code).toBe('DEVICE_IN_USE'); + expect(error.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER'); + await foreign[Symbol.asyncDispose](); + + // The open session's claim survived, with its original token: nothing was adopted. + const held = inspectDeviceClaims({})[0]?.claim; + expect(held?.ownerToken).toBe(open.ownership.ownerToken); + expect(await abandonDeviceClaim(open.ownership)).toBe('abandoned'); +}); + +// A legacy record states no kind, so coverage cannot treat it as an open session's claim. The +// proof it would lean on — `isClaimOwnedByThisDaemon` — matches pid plus start time, and two +// different processes agree on both whenever a start time is unreadable (`null === null`), so a +// leftover v1/v2 command claim can outlive PID reuse and present as this daemon's own session. +// Each case writes the record by hand with the identity this process reports. +test('a legacy record under this process identity covers nothing, not even under an unreadable start time', async () => { + for (const startTimeUnavailable of [false, true]) { + const { stateDir, claimsDir } = setup(); + fs.mkdirSync(claimsDir, { recursive: true }); + // A host this process cannot read its own start time from is the window PID reuse walks + // through, so pin the identity the daemon reads rather than only the one on disk. + const identity = startTimeUnavailable + ? { pid: process.pid, startTime: null } + : hostProcess.readCurrentOwnerIdentity(); + const owner = vi.spyOn(hostProcess, 'readCurrentOwnerIdentity').mockReturnValue(identity); + try { + fs.writeFileSync( + deviceClaimFilePath(canonicalLocalDeviceKey(ANDROID_EMULATOR)), + JSON.stringify({ + schemaVersion: 2, + deviceKey: canonicalLocalDeviceKey(ANDROID_EMULATOR), + device: { + family: 'android', + id: ANDROID_EMULATOR.id, + name: ANDROID_EMULATOR.name, + kind: ANDROID_EMULATOR.kind, + }, + session: 'transient:install', + workspace: '/worktrees/current', + stateDir, + ownerPid: identity.pid, + ownerStartTime: identity.startTime, + ownerToken: 'legacy-token', + createdAtMs: 1, + updatedAtMs: 1, + }), + ); + + // Addressed exactly as the v2 record's own session, from that record's workspace: the most a + // caller can do without stating a kind, and still not enough to take the device. + const claimOwner = makeAdmission( + 'transient-exclusive', + stateDir, + 'install', + 'transient:install', + ); + const error = asAppError( + await claimOwner.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error), + ); + expect(error.code).toBe('DEVICE_IN_USE'); + expect(error.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER'); + await claimOwner[Symbol.asyncDispose](); + expect(inspectDeviceClaims({})[0]?.claim?.ownerToken).toBe('legacy-token'); + } finally { + owner.mockRestore(); + } + } +}); + +// The same workspace and session address do cover once the record can state its kind, so the +// refusal above is about an unstated kind rather than about the identity or workspace checks. +test('a current-version open-session claim of this workspace still covers', async () => { + const { stateDir } = setup(); + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'transient:install', + workspace: '/worktrees/current', + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + + const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'transient:install'); + await admission.admit(ANDROID_EMULATOR, localAndroid); + await admission[Symbol.asyncDispose](); + expect(claimedSessions()).toEqual(['transient:install']); +}); + +// Binding dedupes a request's devices by `deviceIdentityKey`, which carries `target`, while the +// claim key is the narrower `canonicalLocalDeviceKey`, so one device can reach admission twice. +// A claim this request took is the only claim that covers the second pass; anything else must +// still refuse rather than be adopted on the token this request wrote. +test('a transient claim this request took covers re-admitting the same device', async () => { + const { stateDir } = setup(); + const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'request-session'); + await admission.admit(ANDROID_EMULATOR, localAndroid); + expect(claimedSessions()).toEqual(['transient:install']); + + // Android TV is a distinct form-factor leaf: same family/id, so the same claim + // key, while the identity key carries the target and binding treats them apart. + await admission.admit({ ...ANDROID_EMULATOR, target: 'tv' }, localAndroid); + await admission[Symbol.asyncDispose](); + expect(inspectDeviceClaims({})).toEqual([]); +}); + +// Session addresses are printed by `device status` and re-enter through `--session`, so +// coverage has to compare what the claim records, not the string it carries. Both sides go +// through the same canonicalization the router hashes into an implicit address, so one +// checkout stays one workspace across its directories. +test("a claim at this workspace's implicit address still covers its own workspace's command", async () => { + const { root, stateDir } = setup(); + const workspace = path.join(root, 'worktree'); + const subdirectory = path.join(workspace, 'packages', 'app'); + fs.mkdirSync(subdirectory, { recursive: true }); + // The canonical root is the git worktree root, which is what makes the two + // directories one workspace; without a checkout the two are just two paths. + fs.mkdirSync(path.join(workspace, '.git'), { recursive: true }); + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'cwd:aaa111:android', + workspace, + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + + // The owner issues `install` from a subdirectory: raw cwd comparison would refuse it. + const admission = makeAdmission( + 'transient-exclusive', + stateDir, + 'install', + 'cwd:aaa111:android', + subdirectory, + ); + await admission.admit(ANDROID_EMULATOR, localAndroid); + await admission[Symbol.asyncDispose](); + expect(claimedSessions()).toEqual(['cwd:aaa111:android']); +}); + +test('naming another workspace\u2019s session address does not cover a transient command', async () => { + const { root, stateDir } = setup(); + const ownerWorkspace = path.join(root, 'worktree-a'); + const foreignWorkspace = path.join(root, 'worktree-b'); + fs.mkdirSync(ownerWorkspace, { recursive: true }); + fs.mkdirSync(foreignWorkspace, { recursive: true }); + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'cwd:aaa111:android', + workspace: ownerWorkspace, + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + + const admission = makeAdmission( + 'transient-exclusive', + stateDir, + 'install', + 'cwd:aaa111:android', + foreignWorkspace, + ); + const error = asAppError( + await admission.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error), + ); + + expect(error.code).toBe('DEVICE_IN_USE'); + expect(error.details?.owner).toMatchObject({ session: 'cwd:aaa111:android' }); + await admission[Symbol.asyncDispose](); + expect(claimedSessions()).toEqual(['cwd:aaa111:android']); +}); + +// A request without a cwd (SDK callers) cannot prove a workspace either way, so the +// workspace check stays out rather than refusing against the daemon's own directory. +test('a request that carries no cwd keeps coverage of its session\u2019s claim', async () => { + const { stateDir } = setup(); + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'sdk-session', + workspace: '/somewhere/else', + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + + const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'sdk-session', null); + await admission.admit(ANDROID_EMULATOR, localAndroid); + await admission[Symbol.asyncDispose](); + expect(claimedSessions()).toEqual(['sdk-session']); +}); + +// A transient claim outlives its request when the release unlink fails, and a live survivor now +// refuses its own session's next command. The recovery is to abandon the record the unlink left +// behind, which is the state acquireTransientDeviceClaim already supersedes. +test('a failed release abandons the claim so the next command is not locked out', async () => { + const { stateDir } = setup(); + const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'own-session'); + await admission.admit(ANDROID_EMULATOR, localAndroid); + expect(claimedSessions()).toEqual(['transient:install']); + + // The fault lands on the one unlink the release needs: the owned claim file. + // Everything else — including the process lock's reclaim-mutex unlinks taken on + // the way in and out — forwards to the real implementation. + const claimPath = deviceClaimFilePath(canonicalLocalDeviceKey(ANDROID_EMULATOR)); + const originalUnlink = fs.unlinkSync; + const unlink = vi.spyOn(fs, 'unlinkSync').mockImplementation((target) => { + if (String(target) === claimPath) { + throw Object.assign(new Error('read-only file system'), { code: 'EROFS' }); + } + return originalUnlink(target as Parameters[0]); + }); + try { + await admission[Symbol.asyncDispose](); + } finally { + unlink.mockRestore(); + } + + // The claim survived the failed unlink as abandoned, not live: a live survivor would + // refuse this session's next install. + const survivor = inspectDeviceClaims({}); + expect(survivor.map((entry) => entry.claim?.session)).toEqual(['transient:install']); + expect(survivor[0]?.claim?.abandonedAtMs).toBeTypeOf('number'); + + const retry = makeAdmission('transient-exclusive', stateDir, 'install', 'own-session'); + await retry.admit(ANDROID_EMULATOR, localAndroid); + expect(claimedSessions()).toEqual(['transient:install']); + await retry[Symbol.asyncDispose](); + expect(inspectDeviceClaims({})).toEqual([]); +}); + test("an abandoned claim becomes this command's own transient claim and is released", async () => { const { stateDir } = setup(); const aborted = await acquireDeviceClaim({ @@ -169,7 +505,7 @@ test('an ordinary owner is refused for an unknown-schema claim record, not waved const deviceKey = canonicalLocalDeviceKey(ANDROID_EMULATOR); const claimPath = deviceClaimFilePath(deviceKey); const record = JSON.stringify({ - schemaVersion: 3, + schemaVersion: 4, deviceKey, device: { family: 'android', @@ -220,13 +556,25 @@ test('the none policy still reaches no device state at all', async () => { expect(fs.readdirSync(claimsDir)).toEqual(before); }); -/** Claims visible while one command holds a device binding from the real request scope. */ -async function claimsWhileBound(command: string, stateDir: string) { +/** What one command sees when it binds a device through the real request scope. */ +async function bindInRequestSession(params: { + command: string; + stateDir: string; + session?: string; + cwd?: string; +}): Promise<{ claims: (string | undefined)[]; error?: AppError }> { const scope = await createRequestExecutionScope({ platformServices: daemonPlatformServicesFixture(), claimRecoveryGateway: daemonClaimRecoveryGatewayFixture(), - req: { token: 't', session: 'default', command, positionals: [], flags: {} }, - sessionStore: new SessionStore(path.join(stateDir, 'sessions')), + req: { + token: 't', + session: params.session ?? '', + command: params.command, + positionals: [], + flags: {}, + meta: params.cwd ? { cwd: params.cwd } : undefined, + }, + sessionStore: new SessionStore(path.join(params.stateDir, 'sessions')), leaseRegistry: new LeaseRegistry(), deviceRuntimeGateway: unavailableDeviceRuntimeGateway, platformRequestScope: { @@ -240,7 +588,9 @@ async function claimsWhileBound(command: string, stateDir: string) { required: [], preferred: [], }); - return claimedSessions(); + return { claims: claimedSessions() }; + } catch (error) { + return { claims: claimedSessions(), error: asAppError(error) }; } finally { await scope[Symbol.asyncDispose](); } @@ -250,7 +600,105 @@ test('the request scope claims only for commands whose descriptor declares trans const { stateDir } = setup(); // Same binding call, same device: the difference is the descriptor policy. - expect(await claimsWhileBound('shutdown', stateDir)).toEqual(['transient:shutdown']); - expect(await claimsWhileBound('snapshot', stateDir)).toEqual([]); + expect( + (await bindInRequestSession({ command: 'shutdown', stateDir, session: 'default' })).claims, + ).toEqual(['transient:shutdown']); + expect( + (await bindInRequestSession({ command: 'snapshot', stateDir, session: 'default' })).claims, + ).toEqual([]); + expect(inspectDeviceClaims({})).toEqual([]); +}); + +// `--session transient:install` is a legal explicit name — the store sanitizes only directory +// segments — so the router hands admission that exact address. It must not read as the owner of +// a command claim that is still in flight, and the refusal must leave that command holding the +// device. +test('the request scope refuses a session addressed by a transient label while a command holds the device', async () => { + const { stateDir } = setup(); + const holder = makeAdmission('transient-exclusive', stateDir, 'install', 'open-a'); + await holder.admit(ANDROID_EMULATOR, localAndroid); + + const intruder = await bindInRequestSession({ + command: 'reinstall', + stateDir, + session: 'transient:install', + }); + expect(intruder.error?.code).toBe('DEVICE_IN_USE'); + expect(intruder.error?.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER'); + expect(intruder.claims).toEqual(['transient:install']); + + await holder[Symbol.asyncDispose](); expect(inspectDeviceClaims({})).toEqual([]); }); + +test('the request scope admits a transient command only in the session whose claim owns the device', async () => { + const { stateDir } = setup(); + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: 'owning-session', + workspace: '/worktrees/current', + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + + const owner = await bindInRequestSession({ + command: 'install', + stateDir, + session: 'owning-session', + }); + expect(owner.error).toBeUndefined(); + expect(owner.claims).toEqual(['owning-session']); + + const intruder = await bindInRequestSession({ + command: 'install', + stateDir, + session: 'intruder-session', + }); + expect(intruder.error?.code).toBe('DEVICE_IN_USE'); + expect(intruder.error?.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER'); + expect(intruder.error?.details?.owner).toMatchObject({ session: 'owning-session' }); + expect(intruder.claims).toEqual(['owning-session']); +}); + +// Coverage must compare the claim against the *routed* session address, not the requested +// name: an implicit CLI request carries no name, and its claim lives at the workspace's +// implicit address. Passing the bare request name here would refuse the owner's own +// install on every real CLI session. +test('the request scope covers the implicit workspace session that routed to the claim', async () => { + const { root, stateDir } = setup(); + const workspace = path.join(root, 'worktree'); + const elsewhere = path.join(root, 'other-worktree'); + fs.mkdirSync(workspace, { recursive: true }); + fs.mkdirSync(elsewhere, { recursive: true }); + const sessionStore = new SessionStore(path.join(stateDir, 'sessions')); + const routedAddress = resolveEffectiveSessionName( + { + token: 't', + session: '', + command: 'install', + positionals: [], + flags: {}, + meta: { cwd: workspace }, + }, + sessionStore, + { attachesToSession: true }, + ); + // The point of the pin: the owning address is not the name the request carried. + expect(routedAddress).not.toBe('default'); + await acquireDeviceClaim({ + device: ANDROID_EMULATOR, + session: routedAddress, + workspace, + stateDir, + reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims, + }); + + const owner = await bindInRequestSession({ command: 'install', stateDir, cwd: workspace }); + expect(owner.error).toBeUndefined(); + expect(owner.claims).toEqual([routedAddress]); + + const intruder = await bindInRequestSession({ command: 'install', stateDir, cwd: elsewhere }); + expect(intruder.error?.code).toBe('DEVICE_IN_USE'); + expect(intruder.error?.details?.owner).toMatchObject({ session: routedAddress }); + expect(intruder.claims).toEqual([routedAddress]); +}); diff --git a/packages/daemon/src/resources/device/__tests__/device-claim-conflict.test.ts b/packages/daemon/src/resources/device/__tests__/device-claim-conflict.test.ts index 037847ae0c..884bbf5799 100644 --- a/packages/daemon/src/resources/device/__tests__/device-claim-conflict.test.ts +++ b/packages/daemon/src/resources/device/__tests__/device-claim-conflict.test.ts @@ -20,7 +20,8 @@ function conflict(classification: InspectedDeviceClaim['classification']): Inspe deviceKey: 'local:android:none:emulator-5554', classification, claim: { - schemaVersion: 2, + schemaVersion: 3, + claimOwnerKind: 'open-session', deviceKey: 'local:android:none:emulator-5554', device: { family: 'android', id: device.id, name: device.name, kind: device.kind }, session: 'checkout', diff --git a/packages/daemon/src/resources/device/__tests__/device-claim-settlement.test.ts b/packages/daemon/src/resources/device/__tests__/device-claim-settlement.test.ts index e43e2bebc6..35fe383fb7 100644 --- a/packages/daemon/src/resources/device/__tests__/device-claim-settlement.test.ts +++ b/packages/daemon/src/resources/device/__tests__/device-claim-settlement.test.ts @@ -6,8 +6,10 @@ import { afterEach, test, vi } from 'vitest'; import * as hostProcess from '@agent-device/host-kit/process'; import { acquireDeviceClaim as acquireProductionDeviceClaim, + acquireTransientDeviceClaim, canonicalLocalDeviceKey, inspectDeviceClaims, + renewDeviceClaim, } from '@agent-device/daemon-contracts/device-claims'; import type { DeviceBootObservationService } from '@agent-device/contracts/device-boot'; import type { DeviceInfo } from '@agent-device/kernel/device'; @@ -75,7 +77,7 @@ test('preserves and reports a live foreign claim without overwriting it', async }); assert.equal(first.status, 'acquired'); const persisted = JSON.parse(fs.readFileSync(claimPath(root), 'utf8')) as Record; - assert.equal(persisted.schemaVersion, 2); + assert.equal(persisted.schemaVersion, 3); assert.deepEqual(persisted.device, { id: device.id, family: device.platform, @@ -94,6 +96,168 @@ test('preserves and reports a live foreign claim without overwriting it', async assert.equal(inspectDeviceClaims({ serial: device.id })[0]?.claim?.session, 'first'); }); +// The maintainer's trace on #3714: `install` holds a transient claim under the label +// `transient:install`; an `open` from the same workspace addressed at that label must not be +// handed the command's ownership token. Any session string can be typed into `--session`; only +// the stored kind says who holds the claim, so an open-session acquirer never holds a +// transient-command record and vice versa. +test('an open session addressed at a command label never adopts the in-flight command claim', async () => { + const root = useClaimsRoot(); + const holder = await acquireTransientDeviceClaim({ + device, + command: 'install', + coveredBySession: 'open-a', + coveredClaimIsOwnedByRequest: () => false, + workspace: '/worktrees/first', + stateDir: root, + reconcileOrphanedDeviceClaim: async () => ({ status: 'retained', reason: 'test-no-recovery' }), + }); + assert.equal(holder.status, 'acquired'); + if (holder.status !== 'acquired') return; + + const open = await acquireDeviceClaim({ + device, + session: 'transient:install', + workspace: '/worktrees/first', + stateDir: root, + }); + + assert.equal(open.status, 'conflict'); + if (open.status !== 'conflict') return; + assert.equal(open.conflict.classification, 'live'); + // The refusal wrote nothing: the command still holds the device with its own token. + assert.equal(storedClaim().ownerToken, holder.ownership.ownerToken); +}); + +// The mirror direction, pinned at the settlement boundary every acquire route goes through. Two +// live requests can never share one claim: a second `reinstall` or `install` reaching the running +// `install`'s claim — same workspace, label as session — is refused, and only the request that +// took the claim re-enters it, through the token the admission layer already pins. +test('a second request never holds an in-flight command claim, under its own label or another’s', async () => { + const root = useClaimsRoot(); + const holder = await acquireTransientDeviceClaim({ + device, + command: 'install', + coveredBySession: 'open-a', + coveredClaimIsOwnedByRequest: () => false, + workspace: '/worktrees/first', + stateDir: root, + reconcileOrphanedDeviceClaim: async () => ({ status: 'retained', reason: 'test-no-recovery' }), + }); + assert.equal(holder.status, 'acquired'); + if (holder.status !== 'acquired') return; + + const intruder = await acquireTransientDeviceClaim({ + device, + command: 'reinstall', + coveredBySession: 'transient:install', + coveredClaimIsOwnedByRequest: () => true, + workspace: '/worktrees/first', + stateDir: root, + reconcileOrphanedDeviceClaim: async () => ({ status: 'retained', reason: 'test-no-recovery' }), + }); + assert.equal(intruder.status, 'conflict'); + assert.equal(storedClaim().ownerToken, holder.ownership.ownerToken); + + const sameCommand = await acquireTransientDeviceClaim({ + device, + command: 'install', + coveredBySession: 'some-other-session', + coveredClaimIsOwnedByRequest: () => false, + workspace: '/worktrees/first', + stateDir: root, + reconcileOrphanedDeviceClaim: async () => ({ status: 'retained', reason: 'test-no-recovery' }), + }); + // A different request of the SAME command still refuses: only the request holding the claim's + // release token re-enters it (the admission layer pins that), so a live command claim is never + // disposed from under its holder by a second same-named request. + assert.equal(sameCommand.status, 'conflict'); + assert.equal(storedClaim().ownerToken, holder.ownership.ownerToken); +}); + +// A migrated record states no kind, so no acquirer's kind can equal it: even the same principal +// in the same workspace cannot hold it at acquisition, and a record whose start time is unreadable +// (`null === null` against this process) is the case that most needs the refusal. Acquisition +// never renews it either — the token the record itself names is the only renewal route left. +test('a migrated record states no kind, so acquisition never holds it', async () => { + for (const startTimeUnavailable of [false, true]) { + const root = useClaimsRoot(); + const identity = startTimeUnavailable + ? { pid: process.pid, startTime: null } + : hostProcess.readCurrentOwnerIdentity(); + const owner = vi.spyOn(hostProcess, 'readCurrentOwnerIdentity').mockReturnValue(identity); + try { + fs.writeFileSync( + claimPath(root), + JSON.stringify({ + schemaVersion: 2, + deviceKey: canonicalLocalDeviceKey(device), + device: { family: 'android', id: device.id, name: device.name, kind: device.kind }, + session: 'legacy-owner', + workspace: '/worktrees/legacy', + stateDir: root, + ownerPid: identity.pid, + ownerStartTime: identity.startTime, + ownerToken: 'legacy-token', + createdAtMs: 1, + updatedAtMs: 1, + }), + ); + + const held = await acquireDeviceClaim({ + device, + session: 'legacy-owner', + workspace: '/worktrees/legacy', + stateDir: root, + }); + + assert.equal(held.status, 'conflict'); + assert.equal(storedClaim().ownerToken, 'legacy-token'); + assert.equal(storedClaim().updatedAtMs, 1); + } finally { + owner.mockRestore(); + } + } +}); + +// Renewal, abandon, and clear write a decoded claim back. A migrated record must therefore carry +// the current schema version: a legacy version on the migrated device shape reads back as +// inconsistent and locks the device out of any settlement path that reads it. +test('a renewal through a migrated record’s own token writes back the shape the reader expects', async () => { + const root = useClaimsRoot(); + fs.writeFileSync( + claimPath(root), + JSON.stringify({ + schemaVersion: 1, + deviceKey: canonicalLocalDeviceKey(device), + device: { platform: 'android', id: device.id, name: device.name, kind: device.kind }, + session: 'legacy-owner', + workspace: '/worktrees/legacy', + stateDir: root, + ownerPid: process.pid, + ownerStartTime: 'test-process-start-time', + ownerToken: 'legacy-token', + createdAtMs: 1, + updatedAtMs: 1, + }), + ); + + const renewal = await renewDeviceClaim({ + deviceKey: canonicalLocalDeviceKey(device), + ownerToken: 'legacy-token', + ownerPid: process.pid, + ownerStartTime: 'test-process-start-time', + }); + assert.equal(renewal.status, 'renewed'); + + const persisted = JSON.parse(fs.readFileSync(claimPath(root), 'utf8')) as Record; + assert.equal(persisted.schemaVersion, 3); + const reread = inspectDeviceClaims({ serial: device.id })[0]; + assert.equal(reread?.classification, 'live'); + assert.equal(reread?.claim?.session, 'legacy-owner'); + assert.equal(reread?.claim?.claimOwnerKind, 'unstated'); +}); + test('does not treat a same-named session in another worktree as its claim owner', async () => { const root = useClaimsRoot(); const first = await acquireDeviceClaim({ diff --git a/packages/daemon/src/resources/device/__tests__/device-claims.test.ts b/packages/daemon/src/resources/device/__tests__/device-claims.test.ts index 74340b5561..cce909c75c 100644 --- a/packages/daemon/src/resources/device/__tests__/device-claims.test.ts +++ b/packages/daemon/src/resources/device/__tests__/device-claims.test.ts @@ -747,7 +747,7 @@ test('an unknown-schema record is never migrated into a claim, reconciled, or re const root = useClaimsRoot(); // A dead process principal: were this record read as a process claim, it would be releasable. const record = JSON.stringify({ - schemaVersion: 3, + schemaVersion: 4, deviceKey: canonicalLocalDeviceKey(device), device: { family: 'android', id: device.id, name: device.name, kind: device.kind }, session: 'future-owner', diff --git a/packages/daemon/src/resources/device/device-claim-admission.ts b/packages/daemon/src/resources/device/device-claim-admission.ts index 3dd98c5728..27c1b39406 100644 --- a/packages/daemon/src/resources/device/device-claim-admission.ts +++ b/packages/daemon/src/resources/device/device-claim-admission.ts @@ -5,6 +5,7 @@ import { emitDiagnostic } from '@agent-device/host-kit/diagnostics'; import { deviceClaimConflictError } from './device-claim-conflict.ts'; import { deviceClaimRuleForOwner } from './device-claim-rule.ts'; import { + abandonDeviceClaim, acquireTransientDeviceClaim, clearDeviceClaim, type DeviceClaimReconciler, @@ -35,13 +36,37 @@ export type DeviceClaimAdmission = AsyncDisposable & export function createDeviceClaimAdmission(params: { policy: DeviceClaimPolicy; command: string; + /** + * The session this request runs in, which is the only session whose own claim + * covers a `transient-exclusive` command. Sessions of one daemon are separate + * device owners (#3345), so the request's routed session address — not the + * daemon's process identity — is what admission compares a held claim against. + * Addresses are copyable, so session equality alone is not ownership: the + * covered claim must also record this request's workspace. + */ + session: string; workspace: string; + /** + * The caller's raw cwd when the request carried one, or nothing for a caller without a + * workspace of its own (SDK requests). The covered-claim check compares workspaces; + * {@link workspace} cannot serve it because its daemon-directory fallback would refuse + * exactly the SDK requests whose own session claims should cover them. + */ + callerCwd: string | undefined; + /** + * Canonicalizes a workspace to the root session routing hashes into a session address. + * The claim layer cannot reach routing itself, so the composition root hands over the same + * function routing uses: comparing raw directories would separate a caller that ran `open` + * at the checkout root from its own claim recorded against that root. + */ + resolveScopeRoot: (workspace: string | undefined) => string | undefined; stateDir: string; reconcileOrphanedDeviceClaim: DeviceClaimReconciler; }): DeviceClaimAdmission { // The caller admits once per device binding, so this only has to remember what // it took in order to give it back. const acquired: DeviceClaimSessionOwnership[] = []; + const requestScopeRoot = params.resolveScopeRoot(params.callerCwd); /** * Only `transient-exclusive` writes the claim store here: `observe`/`require-owner` never do, @@ -53,12 +78,24 @@ export function createDeviceClaimAdmission(params: { const result = await acquireTransientDeviceClaim({ device, command: params.command, + coveredBySession: params.session, + coveredClaimIsOwnedByRequest: (claim) => + !requestScopeRoot || params.resolveScopeRoot(claim.workspace) === requestScopeRoot, workspace: params.workspace, stateDir: params.stateDir, reconcileOrphanedDeviceClaim: params.reconcileOrphanedDeviceClaim, }); - if (result.status === 'conflict') throw deviceClaimConflictError(device, result.conflict); - if (result.status === 'acquired') acquired.push(result.ownership); + if (result.status === 'acquired') { + acquired.push(result.ownership); + return; + } + if (result.status === 'conflict') { + // A command-scoped claim this request took for this device covers a re-bind of + // it; the same claim seen by any other request is a live owner. + const held = result.conflict.claim; + if (held && acquired.some((ownership) => ownership.ownerToken === held.ownerToken)) return; + throw deviceClaimConflictError(device, result.conflict); + } } return { @@ -84,6 +121,22 @@ export function createDeviceClaimAdmission(params: { error: error instanceof Error ? error.message : String(error), }, }); + // A live record whose unlink failed would refuse this session's own next + // command, so it must not stay live: abandoned means "holds it for nobody", + // which the next acquire supersedes. + try { + await abandonDeviceClaim(ownership); + } catch (abandonError) { + emitDiagnostic({ + level: 'error', + phase: 'transient_device_claim_abandon_failed', + data: { + command: params.command, + deviceKey: ownership.deviceKey, + error: abandonError instanceof Error ? abandonError.message : String(abandonError), + }, + }); + } } } }, diff --git a/packages/daemon/src/server/device/__tests__/device-claim-owner-recovery.test.ts b/packages/daemon/src/server/device/__tests__/device-claim-owner-recovery.test.ts index 5e6a22eabf..0f993a6e50 100644 --- a/packages/daemon/src/server/device/__tests__/device-claim-owner-recovery.test.ts +++ b/packages/daemon/src/server/device/__tests__/device-claim-owner-recovery.test.ts @@ -19,7 +19,8 @@ const scope = { function makeClaim(stateDir: string, session: string): DeviceClaim { return { - schemaVersion: 2, + schemaVersion: 3, + claimOwnerKind: 'open-session', deviceKey: `local:android:none:${session}`, device: { family: 'android', id: session, name: 'Pixel', kind: 'emulator' }, session, diff --git a/packages/daemon/src/server/request-execution-scope.ts b/packages/daemon/src/server/request-execution-scope.ts index 21eef92c7a..bffe64b459 100644 --- a/packages/daemon/src/server/request-execution-scope.ts +++ b/packages/daemon/src/server/request-execution-scope.ts @@ -21,7 +21,10 @@ import { contextFromFlags as contextFromFlagsWithLog, } from '../core/context.ts'; import { assertSessionSelectorMatches } from '../core/session-selector.ts'; -import { resolveEffectiveSessionName } from '../sessions/session-routing.ts'; +import { + resolveCallerScopeRoot, + resolveEffectiveSessionName, +} from '../sessions/session-routing.ts'; import { assertRequestLeaseAdmissionPreflight, scopeRequestSession, @@ -257,8 +260,10 @@ export async function createRequestExecutionScope(params: { const dispatchLedger = requestDispatchLedger(scopedReq); const { claimAdmission, runtimeBindings } = createRequestDeviceAccess({ command, + sessionName, dispatchLedger, workspace: scopedReq.meta?.cwd ?? process.cwd(), + callerCwd: scopedReq.meta?.cwd, stateDir: sessionStore.resolveDaemonStateDir(), deviceRuntimeGateway: params.deviceRuntimeGateway, platformRequestScope: params.platformRequestScope, @@ -391,8 +396,10 @@ export async function createRequestExecutionScope(params: { */ function createRequestDeviceAccess(params: { command: string; + sessionName: string; dispatchLedger: RequestDispatchLedger; workspace: string; + callerCwd: string | undefined; stateDir: string; deviceRuntimeGateway: DeviceRuntimeGateway | undefined; platformRequestScope: PlatformRequestScope | undefined; @@ -409,7 +416,10 @@ function createRequestDeviceAccess(params: { const claimAdmission = createDeviceClaimAdmission({ policy: resolveCommandDeviceClaimPolicy(params.command), command: params.command, + session: params.sessionName, workspace: params.workspace, + callerCwd: params.callerCwd, + resolveScopeRoot: resolveCallerScopeRoot, stateDir: params.stateDir, reconcileOrphanedDeviceClaim: createOwnerScopedDeviceClaimReconciler({ scope: platformRequestScope, diff --git a/packages/daemon/src/sessions/session-routing.ts b/packages/daemon/src/sessions/session-routing.ts index 8f8aff815d..4caed91518 100644 --- a/packages/daemon/src/sessions/session-routing.ts +++ b/packages/daemon/src/sessions/session-routing.ts @@ -305,7 +305,14 @@ function hashScopeRoot(scopeRoot: string): string { return crypto.createHash('sha256').update(scopeRoot).digest('hex').slice(0, 16); } -function resolveCallerScopeRoot(rawCwd: string | undefined): string | undefined { +/** + * The canonical workspace root a caller's cwd belongs to: the git worktree root when the cwd sits + * inside one, otherwise the realpath of the directory. It is the identity the implicit session + * address hashes, and therefore the key anything else that asks "did this caller and that record + * come from the same workspace?" must compare on — comparing raw directories instead would + * separate `open` run at the checkout root from `install` run in a subdirectory of it. + */ +export function resolveCallerScopeRoot(rawCwd: string | undefined): string | undefined { if (!rawCwd || rawCwd.trim().length === 0) return undefined; const cwd = resolveExistingPath(rawCwd); return findGitWorktreeRoot(cwd) ?? cwd; diff --git a/src/commands/schema/cli-help.ts b/src/commands/schema/cli-help.ts index 4413e79e60..4d6b8bbb12 100644 --- a/src/commands/schema/cli-help.ts +++ b/src/commands/schema/cli-help.ts @@ -334,7 +334,7 @@ Example: agent-device network dump --include headers Device busy and ownership: - DEVICE_IN_USE has two flavors. "already in use by session X" is this daemon: reuse it with --session X, or run close --session X first. "owned by session X in workspace Y" is another worktree's daemon holding the host-global device claim: it is never retriable — run the error's exact recovery command instead of retrying. + DEVICE_IN_USE has two flavors. "already in use by session X" is this daemon's session table: reuse it with --session X, or run close --session X first. "owned by session X in workspace Y" is the host-global device claim another session — from this daemon or another worktree's daemon — or an in-flight command holds; there may be no session to close, and it is never retriable, so run the error's exact recovery command instead of retrying. Inspect ownership without any daemon: agent-device device status (add --stale for proven-dead owners; settle and release those with agent-device device release --stale). devices marks rows that are claimed, so pick an unclaimed device instead of contending. A live foreign owner is released by closing its session from its own workspace or stopping its daemon: agent-device daemon stop --state-dir (the error names the state dir). The device itself can settle a claim: when it rebooted after that claim was taken, its app, runner, and accessibility session were destroyed, so open reconciles the owner's resources, takes the claim, and reports the release in warnings. Never recover by hunting PIDs with ps/kill. boot/install/shutdown take the same claims as open and refuse foreign-claimed devices identically; they do not ask the device about its boot. diff --git a/test/integration/provider-scenarios/android-lifecycle.test.ts b/test/integration/provider-scenarios/android-lifecycle.test.ts index d0432296a7..7de74923ed 100644 --- a/test/integration/provider-scenarios/android-lifecycle.test.ts +++ b/test/integration/provider-scenarios/android-lifecycle.test.ts @@ -314,7 +314,7 @@ async function runAndroidSetupAndInstallWorkflow( world: AndroidSettingsWorld, client: AgentDeviceClient, ): Promise { - const { daemon, apkPath, aabPath, manifestApkPath, selection, tempRoot } = world; + const { daemon, apkPath, aabPath, manifestApkPath, selection } = world; const devices = await client.devices.list({ platform: 'android' }); assert.equal(devices.length, 1); @@ -430,45 +430,37 @@ async function runAndroidSetupAndInstallWorkflow( assert.equal(push.action, 'com.example.demo.PUSH'); assert.equal(push.extrasCount, 3); - const pushPayloadPath = path.join(tempRoot, 'payload.json'); - fs.writeFileSync( - pushPayloadPath, - JSON.stringify({ - action: 'com.example.demo.FILE_PUSH', - extras: { source: 'relative-file' }, - }), - 'utf8', - ); - const filePush = await daemon.callCommand( - 'push', - ['com.example.demo', './payload.json'], - selection, - { - meta: { cwd: tempRoot }, - }, - ); - assert.equal(filePush.json.result.data.package, 'com.example.demo'); - assert.equal(filePush.json.result.data.action, 'com.example.demo.FILE_PUSH'); - assert.equal(filePush.json.result.data.extrasCount, 1); - - const bracePayloadPath = path.join(tempRoot, '{payload}.json'); - fs.writeFileSync( - bracePayloadPath, - JSON.stringify({ - action: 'com.example.demo.BRACE_PUSH', - extras: { source: 'brace-file' }, - }), - 'utf8', - ); - const braceFilePush = await daemon.callCommand( - 'push', - ['com.example.demo', './{payload}.json'], - selection, - { meta: { cwd: tempRoot } }, - ); - assert.equal(braceFilePush.json.result.data.package, 'com.example.demo'); - assert.equal(braceFilePush.json.result.data.action, 'com.example.demo.BRACE_PUSH'); - assert.equal(braceFilePush.json.result.data.extrasCount, 1); + // #3345: coverage pairs the routed session with the workspace a claim records. The client + // calls above sent no cwd, so their open claim records the daemon-side fallback workspace — + // this checkout — at the routed `default` address. These pushes state their provenance + // explicitly: sessionExplicit joins that address, and their cwd — a checkout child directory, + // chosen so the relative payloads resolve against it — canonicalizes to that recorded + // workspace. A foreign cwd refuses even under the same session name: the copyable-address rule. + fs.mkdirSync(path.join(process.cwd(), '.tmp'), { recursive: true }); + const callerWorkspace = fs.mkdtempSync(path.join(process.cwd(), '.tmp', 'push-')); + try { + for (const [payloadFile, action, source] of [ + ['payload.json', 'com.example.demo.FILE_PUSH', 'relative-file'], + ['{payload}.json', 'com.example.demo.BRACE_PUSH', 'brace-file'], + ] as const) { + fs.writeFileSync( + path.join(callerWorkspace, payloadFile), + JSON.stringify({ action, extras: { source } }), + 'utf8', + ); + const pushed = await daemon.callCommand( + 'push', + ['com.example.demo', `./${payloadFile}`], + selection, + { meta: { cwd: callerWorkspace, sessionExplicit: true } }, + ); + assert.equal(pushed.json.result.data.package, 'com.example.demo'); + assert.equal(pushed.json.result.data.action, action); + assert.equal(pushed.json.result.data.extrasCount, 1); + } + } finally { + fs.rmSync(callerWorkspace, { recursive: true, force: true }); + } const clipboard = await client.command.clipboard({ action: 'read', ...selection }); if (clipboard.action !== 'read') throw new Error('expected clipboard read result'); diff --git a/website/docs/docs/commands.md b/website/docs/docs/commands.md index 9a4010823d..0c05d87de0 100644 --- a/website/docs/docs/commands.md +++ b/website/docs/docs/commands.md @@ -82,7 +82,7 @@ Select a device: Device claims and the daemon: - `device status` lists host-local device claims without starting or contacting a daemon; `--stale` shows the stale claims it hides by default (owner exited, PID reused, state directory deleted, or daemon replaced), some of whose owners may still be running. Scope it with `--platform` plus `--udid` (Apple) or `--serial` (Android). A claim held by another live or uncertain owner blocks `open`, unless the device rebooted after the claim was taken: then `open` settles that owner's durable resources, takes the claim, and reports the release in `warnings`. -- `device release --stale` settles the durable resources of an owner that provably can't release its claim (its process exited or a newer daemon replaced it), then clears the claim. Live, uncertain, PID-reused, corrupt, and deleted-state-directory claims are refused with the reason. See [Sessions](/docs/sessions#release-a-device-another-workspace-holds). +- `device release --stale` settles the durable resources of an owner that provably can't release its claim (its process exited or a newer daemon replaced it), then clears the claim. Live, uncertain, PID-reused, corrupt, and deleted-state-directory claims are refused with the reason. See [Sessions](/docs/sessions#release-a-device-another-session-or-workspace-holds). - A command that needs the shared local daemon starts it for its state directory when none runs; `device status` and `device release --stale` never contact one. When the running daemon comes from another version or build, the command replaces it only if it reports no work: no request in flight, no session, no repair session and no live lease. A resumable upload still streaming and a host human-control hold that names no lease are not counted, so a replacement can end them. Otherwise the command fails with `daemon-busy` and leaves the daemon alone. A daemon from a release that predates this check is never replaced, and fails with `daemon-incompatible`. End a busy or older daemon deliberately with `daemon stop`. Node.js clients choose this per client; see [Daemon lifecycle](/docs/client-api#daemon-lifecycle). - `daemon stop --state-dir ` stops a local daemon after verifying its identity. `--clean` also removes the Apple runner processes and leases that daemon retained. @@ -90,7 +90,7 @@ Open apps and links: - `open [app|url] [url]` boots and activates the selected target when needed. `open ` opens deep links on Android and iOS, and `open ` opens a deep link in that app on iOS. - On iOS devices, `http(s)://` URLs open in Safari when no app is active. Custom-scheme URLs need an active app in the session. -- `open --wait ` waits up to that budget for a device another session holds, then fails with `DEVICE_IN_USE`. The wait extends the command's timeout, so you don't need a longer `--timeout`. A claim held by another workspace's daemon is not waited for; `open` returns its recovery command at once. See [Sessions](/docs/sessions#wait-for-a-slow-or-busy-device). +- `open --wait ` waits up to that budget for a device another session holds, then fails with `DEVICE_IN_USE`. The wait extends the command's timeout, so you don't need a longer `--timeout`. A claim held by another workspace's daemon is not waited for; `open` returns its recovery command at once. Device-mutating commands like `install` and `reinstall` never wait either: a claim held by another session fails them at once, and when the caller sends a workspace, another workspace's claim under the same session name fails too. See [Sessions](/docs/sessions#wait-for-a-slow-or-busy-device). - `open --launch-console ` captures launch-time stdout/stderr for iOS simulator app launches. It is not valid for URL opens or other targets. - `open --platform macos --surface app|frontmost-app|desktop|menubar` selects the macOS session surface; `app` is the default when you name an app. See [Desktop targets](#desktop-targets). `--save-script` records the surface so `replay` reopens it; with no surface given, replay opens an app session. - Android React Native: `open --relaunch` needs an installed package name. Install or reinstall the APK first; `open --relaunch` is rejected. For Metro-backed JS changes, run `metro reload` before reaching for `open --relaunch`. diff --git a/website/docs/docs/sessions.md b/website/docs/docs/sessions.md index b419bac96d..a23cd2237d 100644 --- a/website/docs/docs/sessions.md +++ b/website/docs/docs/sessions.md @@ -149,11 +149,19 @@ budget, so several agents can queue on one device. `--wait` waits only for another session. A device claimed by another workspace's daemon is never waited for; `open` fails at once and names the command that releases it. +Device-mutating commands like `install` and `reinstall` don't wait either: while a +session other than yours holds the device, they fail at once with `DEVICE_IN_USE`. +Copying the owner's `--session` address doesn't change that — the claim must also +record your workspace, so a command run from another checkout is refused even under +the owner's name. The error names the owner when the claim is readable; when it +isn't, `agent-device device status` shows what holds the device. -## Release a device another workspace holds +## Release a device another session or workspace holds Each daemon claims the devices its sessions use. A claim whose owner is live, or can't be proven -gone, blocks `open` from another workspace. List the claims on this host without starting a daemon: +gone, blocks `open` from another workspace, and blocks device-mutating commands like `install` +from any other owner — another session, or the same session name from another workspace. +List the claims on this host without starting a daemon: ```bash agent-device device status