Skip to content

Commit d17f710

Browse files
authored
fix(daemon): scope a transient device claim to the session that holds it (#3714)
* fix(daemon): scope a transient device claim to the session that holds it #1809 made install/reinstall and their transient-exclusive siblings covered by any claim this daemon's process held, so a second session in the same daemon could install over a device another session had open. Sessions of one daemon are separate device owners: admission now compares the request's routed session against the held claim, and a foreign session's claim refuses the command with DEVICE_IN_USE the way open already does. Session addresses are copyable, so the covered claim must additionally record the caller's own workspace, canonicalized on both sides through the same scope root session routing hashes; a request without a cwd carries no workspace claim. A release whose unlink fails now abandons the claim instead of leaving it live, so the owner is never locked out of its own device. The versioned help and user docs name the second DEVICE_IN_USE flavor's real owner; main's device-verification notes already attribute that claim to another session, so no guidance budget moves there. * test(integration): run the raw file push from inside the claiming workspace #3345's coverage rule pairs the routed session with the workspace that took the claim. The raw push request carried a temp cwd, so it routed under another workspace, and the open session's claim no longer covered it. Running it from a directory of the checkout reproduces the real CLI owner shape the client calls above already form, and keeps the relative payload resolution the test is for. Probe evidence at the pre-fix commit: the push refused with DEVICE_CLAIM_LIVE_OWNER naming the checkout workspace. * fix(daemon): record which kind of owner holds a device claim An in-flight command's transient claim and an open session's claim differed only by the shape of the claim's `session` label, so a request whose routed session string happened to equal `transient:<command>` — reachable today with `--session transient:install`, which the store sanitizes only in directory segments — matched the coverage branch and ran while the first request still held the device (review r4295092657). The record now stores the kind: `transientCommand` names the scoped command (schema v3), and coverage admits only open-session claims; a command-scoped claim is a live owner to every request but its holder, proven by the release token at the admission layer. Legacy v1/v2 records translate the label that encoded the kind, once, in the record decode (tag history: v2 is released in v0.21.28, so live records use it). A session literally named this command's label is refused rather than renew-adopted, so its claim keeps only what its principal grants. Fold in the review's documentation findings: the DEVICE_IN_USE help names in-flight commands as owners a caller cannot close (r4295092708 sibling), commands.md and sessions.md state the workspace condition next to session ownership without implying every caller sends a workspace (r4295092708, r4295092739), and sessions.md names the owner only when the claim is readable, pointing at `device status` otherwise (r4295092723). Shrink the android-lifecycle raw-push fixture under the merge-base length: the file is over the 1,000-line ratchet tripwire and may not grow, so the push payloads are written directly into the claiming workspace by a loop (Coverage job test-file-size ratchet, run 38905771494). * fix(daemon): decode legacy device claims kindless instead of guessing a kind r4295744998: released v1/v2 recorded a command-scoped claim and an open session literally named `transient:<command>` as the same record, so the label is not a kind proof, and stamping a guessed kind made the schema version alone decide whether a colon-named session's claim covered its commands. Legacy records now decode kindless, which is safe by construction rather than by guess: every consumer of a claim's kind first requires `isClaimOwnedByThisDaemon` (same stateDir, pid, and process start time), a live process writes only v3 records whose kind is stored, and the forged-identity window that reaches a kind decision anyway is pinned by a closest-negative test: a kindless legacy record under this process's identity behaves exactly as a colon-named open session's claim — refused to foreign commands by the label guard, covering for the named session — never as a command claim any in-flight request can adopt or leak. The record test pins the kind assertions directly (ordinary legacy and current claims decode `transientCommand` undefined; the v3 kind comes only from its stored field; malformed kinds fail closed) and the android-lifecycle provenance comment states what the no-cwd client calls actually record: the daemon-side fallback workspace at the routed `default` address, joined explicitly by the pushes whose checkout-child cwd exists for relative payload resolution, not as a workspace-authority test (r4295745008, r4295745014). * test(daemon): tighten the device-claim admission tests Drop the duplicated transient-claim refusal case (its label-guard sibling already proves it) and rewrite comments that narrated mutations and review rounds into statements of the constraint each test pins. * fix(daemon): state a claim kind only from a record that states one A migrated v1/v2 record decoded with no kind at all, and coverage read an absent transientCommand as proof of an open session. The ownership check in front of that decision matches pid plus start time, and two processes agree on both whenever a start time is unreadable, so a leftover v2 command claim can outlive PID reuse and present as this daemon's own session. Decoded claims now carry a persisted claimOwnerKind. A current writer states open-session or transient-command; a record that predates the field states unstated and can hold and fence a device but can never cover a command. The kind is stored rather than derived from schemaVersion so renewal, abandon, and clear keep writing the current shape — a renewed record that kept its legacy version would be stamped with the migrated device shape and read back as inconsistent. * fix(daemon): hold an existing claim only for its own claim kind An acquirer now holds an existing claim only when the record states the very kind of claim the acquirer would write itself — the same transientCommand for transient admission — under the same session, workspace, and daemon identity. An unstated record matches no acquirer, so acquisition never adopts a migrated v1/v2 record, and an open session named after a command label never inherits an in-flight command's owner token. claimHeldDevice passes the acquirer's typed kind and command to resolveExistingClaim, the boundary every acquire route goes through, which retires the label guard in acquireTransientDeviceClaim. Ref: #3714 (comment)
1 parent 92db564 commit d17f710

18 files changed

Lines changed: 985 additions & 103 deletions

‎packages/daemon-contracts/src/device-claim-reboot.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ const device: DeviceInfo = {
1818
};
1919

2020
const CLAIM: DeviceClaim = {
21-
schemaVersion: 2,
21+
schemaVersion: 3,
22+
claimOwnerKind: 'open-session',
2223
deviceKey: 'local:apple:ios:SIM-1',
2324
device: { family: 'apple', id: device.id, name: device.name, kind: device.kind },
2425
session: 'cwd:/w:default',

‎packages/daemon-contracts/src/device-claim-reconciliation.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ const unavailable = Object.freeze({
146146

147147
function makeClaim(stateDir: string): DeviceClaim {
148148
return {
149-
schemaVersion: 2,
149+
schemaVersion: 3,
150+
claimOwnerKind: 'open-session',
150151
deviceKey: 'local:android:none:emulator-5554',
151152
device: { family: 'android', id: 'emulator-5554', name: 'Pixel', kind: 'emulator' },
152153
session: 'claimed-session',

‎packages/daemon-contracts/src/device-claim-record.test.ts‎

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function legacyClaim(overrides: Record<string, unknown> = {}): Record<string, un
2020
};
2121
}
2222

23-
function currentClaim(overrides: Record<string, unknown> = {}): Record<string, unknown> {
23+
function v2Claim(overrides: Record<string, unknown> = {}): Record<string, unknown> {
2424
return {
2525
...legacyClaim(),
2626
schemaVersion: 2,
@@ -29,24 +29,76 @@ function currentClaim(overrides: Record<string, unknown> = {}): Record<string, u
2929
};
3030
}
3131

32-
test('v1 and v2 records decode to the process-owned claim with its process principal intact', () => {
33-
for (const raw of [legacyClaim(), currentClaim()]) {
32+
function currentClaim(overrides: Record<string, unknown> = {}): Record<string, unknown> {
33+
return {
34+
...v2Claim(),
35+
schemaVersion: 3,
36+
claimOwnerKind: 'open-session',
37+
...overrides,
38+
};
39+
}
40+
41+
test('v1, v2, and v3 records decode to the process-owned claim with its process principal intact', () => {
42+
for (const raw of [legacyClaim(), v2Claim(), currentClaim()]) {
3443
const record = decodeStoredDeviceClaim(raw);
3544
expect(record).not.toBeNull();
3645
if (!record) throw new Error('expected process-owned');
37-
expect(record.schemaVersion).toBe(2);
46+
expect(record.schemaVersion).toBe(3);
3847
expect(record.session).toBe('work');
3948
expect(record.ownerPid).toBe(4242);
4049
expect(record.ownerToken).toBe('token');
50+
expect(record.transientCommand).toBeUndefined();
51+
}
52+
});
53+
54+
// Released v1/v2 recorded a command-scoped claim and an open session *named* `transient:<command>`
55+
// as the same record, so no stored field can say which one a legacy record was. Such a record
56+
// still decodes — clearing, reconciliation, and stale settlement need its principal — but it
57+
// states no kind, so a migrated record can never cover a command.
58+
test('a migrated record normalizes to the current schema and states no claim kind', () => {
59+
for (const raw of [
60+
legacyClaim({ session: 'transient:install' }),
61+
v2Claim({ session: 'transient:install' }),
62+
]) {
63+
const record = decodeStoredDeviceClaim(raw);
64+
expect(record).not.toBeNull();
65+
if (!record) throw new Error('expected a decoded claim');
66+
expect(record.schemaVersion).toBe(3);
67+
expect(record.session).toBe('transient:install');
68+
expect(record.claimOwnerKind).toBe('unstated');
69+
expect(record.transientCommand).toBeUndefined();
4170
}
71+
});
72+
73+
// A current record states its kind, and the kind fields have to agree. An open session may
74+
// legitimately be named after a command; a malformed kind fails the record closed.
75+
test('a current-version claim kind comes only from its stored fields', () => {
76+
const named = decodeStoredDeviceClaim(currentClaim({ session: 'transient:install' }));
77+
expect(named).not.toBeNull();
78+
if (!named) throw new Error('expected a decoded claim');
79+
expect(named.claimOwnerKind).toBe('open-session');
80+
expect(named.transientCommand).toBeUndefined();
81+
82+
const scoped = decodeStoredDeviceClaim(
83+
currentClaim({ claimOwnerKind: 'transient-command', transientCommand: 'install' }),
84+
);
85+
expect(scoped).not.toBeNull();
86+
if (!scoped) throw new Error('expected a decoded claim');
87+
expect(scoped.claimOwnerKind).toBe('transient-command');
88+
expect(scoped.transientCommand).toBe('install');
89+
90+
expect(decodeStoredDeviceClaim(currentClaim({ transientCommand: 'install' }))).toBeNull();
4291
expect(
43-
decodeStoredDeviceClaim(legacyClaim({ session: 'transient:install' }))?.schemaVersion,
44-
).toBe(2);
92+
decodeStoredDeviceClaim(
93+
currentClaim({ claimOwnerKind: 'transient-command', transientCommand: '' }),
94+
),
95+
).toBeNull();
96+
expect(decodeStoredDeviceClaim(currentClaim({ claimOwnerKind: 'bogus' }))).toBeNull();
4597
});
4698

4799
test('a record of an unknown schema version is unreadable', () => {
48100
// A future-schema record carrying a whole process principal is still not migrated into a claim.
49-
for (const schemaVersion of [0, 3, 4]) {
101+
for (const schemaVersion of [0, 4, 5]) {
50102
expect(decodeStoredDeviceClaim(currentClaim({ schemaVersion }))).toBeNull();
51103
}
52104
expect(decodeStoredDeviceClaim(null)).toBeNull();

‎packages/daemon-contracts/src/device-claim-record.ts‎

Lines changed: 76 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,30 @@ import { isRecord } from '@agent-device/kernel/record';
88

99
import { canonicalLocalDeviceKey } from './device-claim-paths.ts';
1010

11-
/** Schema version of a process-owned claim record; v1 records migrate to it on read. */
12-
export const DEVICE_CLAIM_SCHEMA_VERSION = 2;
11+
/** Schema version of a process-owned claim record; v1/v2 records normalize to it on read. */
12+
export const DEVICE_CLAIM_SCHEMA_VERSION = 3;
13+
14+
/**
15+
* What a claim's record states about who owns it. A current writer always states one.
16+
* `unstated` is what a record migrated from the released v1/v2 schemas decodes to, because those
17+
* schemas wrote a command-scoped claim and an open session *named* `transient:<command>`
18+
* identically; the session label is never re-read as the discriminant. A claim that states no kind
19+
* can hold and fence a device but can never cover a command.
20+
*/
21+
export type DeviceClaimOwnerKind = 'open-session' | 'transient-command' | 'unstated';
22+
23+
const DEVICE_CLAIM_OWNER_KINDS: readonly DeviceClaimOwnerKind[] = [
24+
'open-session',
25+
'transient-command',
26+
'unstated',
27+
];
1328

1429
/**
1530
* A claim owned by one process: the session or the sessionless command that took it. Its principal
1631
* is `ownerPid`/`ownerStartTime`/`ownerToken`, which is what every clearing surface matches on.
1732
*/
1833
export type DeviceClaim = {
19-
schemaVersion: 2;
34+
schemaVersion: 3;
2035
deviceKey: string;
2136
device: DeviceIdentity & { name: string };
2237
session: string;
@@ -27,6 +42,15 @@ export type DeviceClaim = {
2742
ownerToken: string;
2843
createdAtMs: number;
2944
updatedAtMs: number;
45+
/**
46+
* The persisted claim-kind discriminant (#3345). Coverage reads this and never the `session`
47+
* label: an open session may legitimately be named `transient:<command>`, and any printed
48+
* address can be pasted back into `--session`. It is stored rather than derived from the schema
49+
* version so a record renewed or abandoned after migration keeps saying the same thing.
50+
*/
51+
claimOwnerKind: DeviceClaimOwnerKind;
52+
/** The command a `transient-command` claim is scoped to; present for exactly that kind. */
53+
transientCommand?: string;
3054
/** Set by `abandonDeviceClaim`; absent while the claim still holds the device for its owner. */
3155
abandonedAtMs?: number;
3256
};
@@ -52,20 +76,53 @@ export function ownershipFromClaim(claim: DeviceClaim): DeviceClaimSessionOwners
5276
};
5377
}
5478

79+
/**
80+
* Every stored record normalizes to the current schema so that a renewal or an abandon can write a
81+
* decoded claim back without changing its shape. A v1/v2 record has no kind field, and the session
82+
* label cannot supply one, so it decodes as `unstated`: it keeps its full principal for clearing,
83+
* reconciliation, and stale settlement, and states no ownership kind.
84+
*/
5585
export function decodeStoredDeviceClaim(value: unknown): DeviceClaim | null {
5686
if (!isRecord(value)) return null;
5787
if (value.schemaVersion === DEVICE_CLAIM_SCHEMA_VERSION) return decodeCurrentClaim(value);
88+
if (value.schemaVersion === 2) return decodeClaimBody(value, 'unstated');
5889
if (value.schemaVersion === 1) return migrateLegacyClaim(value);
5990
return null;
6091
}
6192

93+
/**
94+
* A current record states its kind, and the two kind fields have to agree: a command claim names
95+
* its command, any other kind names none. Anything else fails the record closed rather than
96+
* guessing which owner a session label suggests.
97+
*/
6298
function decodeCurrentClaim(raw: Record<string, unknown>): DeviceClaim | null {
99+
const { claimOwnerKind, transientCommand } = raw;
100+
if (!isKnownOwnerKind(claimOwnerKind)) return null;
101+
if (claimOwnerKind === 'transient-command') {
102+
if (!isNonEmptyString(transientCommand)) return null;
103+
const claim = decodeClaimBody(raw, claimOwnerKind);
104+
return claim ? { ...claim, transientCommand } : null;
105+
}
106+
if (transientCommand !== undefined) return null;
107+
return decodeClaimBody(raw, claimOwnerKind);
108+
}
109+
110+
function isKnownOwnerKind(value: unknown): value is DeviceClaimOwnerKind {
111+
return (
112+
typeof value === 'string' && (DEVICE_CLAIM_OWNER_KINDS as readonly string[]).includes(value)
113+
);
114+
}
115+
116+
function decodeClaimBody(
117+
raw: Record<string, unknown>,
118+
claimOwnerKind: DeviceClaimOwnerKind,
119+
): DeviceClaim | null {
63120
if (!isRecord(raw.device) || !isNonEmptyString(raw.device.name)) return null;
64121
const identity = decodeDeviceIdentity(raw.device);
65-
return identity ? buildDecodedClaim(raw, identity, raw.device.name) : null;
122+
return identity ? decodeClaimFields(raw, identity, raw.device.name, claimOwnerKind) : null;
66123
}
67124

68-
/** Released schema-v1 advisory claims are normalized into the canonical v2 model on read. */
125+
/** Released schema-v1 claims differ only in device shape; they state no claim kind. */
69126
function migrateLegacyClaim(raw: Record<string, unknown>): DeviceClaim | null {
70127
if (!isRecord(raw.device)) return null;
71128
const legacy = raw.device;
@@ -87,13 +144,14 @@ function migrateLegacyClaim(raw: Record<string, unknown>): DeviceClaim | null {
87144
? {}
88145
: { iosPhysicalDeviceBackend: legacy.iosPhysicalDeviceBackend }),
89146
});
90-
return identity ? buildDecodedClaim(raw, identity, name) : null;
147+
return identity ? decodeClaimFields(raw, identity, name, 'unstated') : null;
91148
}
92149

93-
function buildDecodedClaim(
150+
function decodeClaimFields(
94151
raw: Record<string, unknown>,
95152
identity: DeviceIdentity,
96153
name: string,
154+
claimOwnerKind: DeviceClaimOwnerKind,
97155
): DeviceClaim | null {
98156
const location = decodeClaimLocation(raw);
99157
const owner = decodeClaimOwner(raw);
@@ -106,9 +164,20 @@ function buildDecodedClaim(
106164
device: { ...identity, name },
107165
...owner,
108166
...timestamps,
167+
claimOwnerKind,
109168
};
110169
}
111170

171+
/**
172+
* The claim `session` recorded for a command-scoped claim. Lives with the record so the label and
173+
* its inverse stay one owner; `claimOwnerKind` is what decides a kind, never this shape.
174+
*/
175+
export function transientDeviceClaimSession(command: string): string {
176+
return `${TRANSIENT_CLAIM_LABEL_PREFIX}${command}`;
177+
}
178+
179+
const TRANSIENT_CLAIM_LABEL_PREFIX = 'transient:';
180+
112181
function decodeClaimLocation(
113182
raw: Record<string, unknown>,
114183
): Pick<DeviceClaim, 'deviceKey' | 'session' | 'workspace' | 'stateDir'> | null {

‎packages/daemon-contracts/src/device-claim-settlement.ts‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { resolveDeviceClaimPath } from './device-claim-paths.ts';
2121
import {
2222
ownershipFromClaim,
2323
type DeviceClaim,
24+
type DeviceClaimOwnerKind,
2425
type DeviceClaimSessionOwnership,
2526
} from './device-claim-record.ts';
2627
import { writeDeviceClaim } from './device-claim-store.ts';
@@ -43,6 +44,13 @@ export type DeviceClaimReconciler = (
4344
claim: DeviceClaim,
4445
) => Promise<DeviceClaimReconciliationResult>;
4546

47+
/**
48+
* The acquirer whose ownership `resolveExistingClaim` judges: the kind of claim it could have
49+
* written itself — never `unstated`, which no acquirer can be — plus the command a transient
50+
* acquirer scopes to.
51+
*/
52+
export type DeviceClaimAcquirerKind = Exclude<DeviceClaimOwnerKind, 'unstated'>;
53+
4654
/**
4755
* Settles the claim file an acquisition found, and decides whether this caller may write its own.
4856
* The recorded owner's own state answers first — a dead, unreachable, or superseded owner is
@@ -57,6 +65,10 @@ export async function resolveExistingClaim(params: {
5765
session: string;
5866
workspace: string;
5967
stateDir: string;
68+
/** The kind of claim this acquirer would write, so `isCurrentClaimOwner` can compare kinds. */
69+
claimOwnerKind: DeviceClaimAcquirerKind;
70+
/** The command a `transient-command` acquirer scopes to; absent exactly when the kind is. */
71+
transientCommand?: string;
6072
reconcileOrphanedDeviceClaim: DeviceClaimReconciler;
6173
observeDeviceBoot?: DeviceBootObservationService;
6274
}): Promise<ExistingClaimResolution> {
@@ -187,11 +199,31 @@ function isAbandonedClaimOfThisDaemon(
187199
return isAbandonedDeviceClaim(claim) && isClaimOwnedByThisDaemon(claim, stateDir, owner);
188200
}
189201

202+
/**
203+
* An acquirer holds an existing claim only when the record states the very kind of claim this
204+
* acquirer would write itself — the same `transientCommand` for transient admission — under the
205+
* same session, workspace, and daemon. `unstated` matches no acquirer, so a migrated record can
206+
* hold and fence the device but can never be adopted at acquisition, however exactly its session
207+
* label or copyable `--session` address lines up (#3345).
208+
*/
190209
function isCurrentClaimOwner(
191210
claim: DeviceClaim,
192-
params: { session: string; workspace: string; stateDir: string },
211+
params: {
212+
session: string;
213+
workspace: string;
214+
stateDir: string;
215+
claimOwnerKind: DeviceClaimAcquirerKind;
216+
transientCommand?: string;
217+
},
193218
owner: ReturnType<typeof readCurrentOwnerIdentity>,
194219
): boolean {
220+
if (claim.claimOwnerKind !== params.claimOwnerKind) return false;
221+
if (
222+
params.claimOwnerKind === 'transient-command' &&
223+
claim.transientCommand !== params.transientCommand
224+
) {
225+
return false;
226+
}
195227
return (
196228
claim.session === params.session &&
197229
claim.workspace === params.workspace &&

0 commit comments

Comments
 (0)