Skip to content

Commit 76766ea

Browse files
committed
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.
1 parent b3777d3 commit 76766ea

13 files changed

Lines changed: 277 additions & 159 deletions

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const device: DeviceInfo = {
1919

2020
const CLAIM: DeviceClaim = {
2121
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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ const unavailable = Object.freeze({
147147
function makeClaim(stateDir: string): DeviceClaim {
148148
return {
149149
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: 39 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,12 @@ function v2Claim(overrides: Record<string, unknown> = {}): Record<string, unknow
3030
}
3131

3232
function currentClaim(overrides: Record<string, unknown> = {}): Record<string, unknown> {
33-
return { ...v2Claim(), schemaVersion: 3, ...overrides };
33+
return {
34+
...v2Claim(),
35+
schemaVersion: 3,
36+
claimOwnerKind: 'open-session',
37+
...overrides,
38+
};
3439
}
3540

3641
test('v1, v2, and v3 records decode to the process-owned claim with its process principal intact', () => {
@@ -47,35 +52,48 @@ test('v1, v2, and v3 records decode to the process-owned claim with its process
4752
});
4853

4954
// Released v1/v2 recorded a command-scoped claim and an open session *named* `transient:<command>`
50-
// as the same record — no stored field separates them, so the kind of a legacy claim is not a
51-
// fact the decoder can read. Both decode kindless, which is safe by construction, not by guess:
52-
// the kind is consulted only for claims `isClaimOwnedByThisDaemon` accepts (same stateDir, pid,
53-
// and process start time), and a live process reads and writes only v3 records, whose kind is a
54-
// stored field. No in-flight command's claim is ever a legacy record; the original #3345 bypass
55-
// needs a current-process claim, which carries its kind.
56-
test('a legacy claim with a command-label session decodes kindless, not as a guessed command claim', () => {
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', () => {
5759
for (const raw of [
5860
legacyClaim({ session: 'transient:install' }),
5961
v2Claim({ session: 'transient:install' }),
6062
]) {
6163
const record = decodeStoredDeviceClaim(raw);
62-
expect(record?.transientCommand).toBeUndefined();
63-
expect(record?.session).toBe('transient:install');
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();
6470
}
6571
});
6672

67-
// The kind is a stored field from v3 on. A v3 open session may be named after a command, and a
68-
// v3 record without the field is exactly that session's claim; a malformed kind fails the record
69-
// closed instead of guessing.
70-
test('a v3 claim kind comes only from its stored field', () => {
71-
expect(
72-
decodeStoredDeviceClaim(currentClaim({ session: 'transient:install' }))?.transientCommand,
73-
).toBeUndefined();
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();
7491
expect(
75-
decodeStoredDeviceClaim(currentClaim({ transientCommand: 'install' }))?.transientCommand,
76-
).toBe('install');
77-
expect(decodeStoredDeviceClaim(currentClaim({ transientCommand: '' }))).toBeNull();
78-
expect(decodeStoredDeviceClaim(currentClaim({ transientCommand: 7 }))).toBeNull();
92+
decodeStoredDeviceClaim(
93+
currentClaim({ claimOwnerKind: 'transient-command', transientCommand: '' }),
94+
),
95+
).toBeNull();
96+
expect(decodeStoredDeviceClaim(currentClaim({ claimOwnerKind: 'bogus' }))).toBeNull();
7997
});
8098

8199
test('a record of an unknown schema version is unreadable', () => {

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

Lines changed: 56 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,24 @@ 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/v2 records migrate to it on read. */
11+
/** Schema version of a process-owned claim record; v1/v2 records normalize to it on read. */
1212
export const DEVICE_CLAIM_SCHEMA_VERSION = 3;
1313

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+
];
28+
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.
@@ -28,13 +43,13 @@ export type DeviceClaim = {
2843
createdAtMs: number;
2944
updatedAtMs: number;
3045
/**
31-
* Set when a `transient-exclusive` command took the claim: the command it is scoped to. This is
32-
* the claim-kind discriminant (#3345) — a command-scoped claim is never an open session's claim,
33-
* even when its `session` label reads `transient:<command>` and a caller copies that label into
34-
* `--session`. Absent means an open session owns it, including every legacy record: a v1/v2
35-
* record cannot prove which it was, and no kind decision is ever taken on one (see
36-
* {@link decodeStoredDeviceClaim}).
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.
3750
*/
51+
claimOwnerKind: DeviceClaimOwnerKind;
52+
/** The command a `transient-command` claim is scoped to; present for exactly that kind. */
3853
transientCommand?: string;
3954
/** Set by `abandonDeviceClaim`; absent while the claim still holds the device for its owner. */
4055
abandonedAtMs?: number;
@@ -62,50 +77,53 @@ export function ownershipFromClaim(claim: DeviceClaim): DeviceClaimSessionOwners
6277
}
6378

6479
/**
65-
* Legacy (v1/v2) records decode kindless — as open-session claims — because they cannot state a
66-
* kind: released v1/v2 recorded a command-scoped claim and an open session *named*
67-
* `transient:<command>` identically, so the label is no proof of kind. Decoding kindless is safe
68-
* by construction, not by guess: every consumer of a claim's kind first requires
69-
* `isClaimOwnedByThisDaemon` (same stateDir, pid, and process start time), and a current process
70-
* writes only v3 records whose kind is stored — a legacy record can only be one this process
71-
* could not have written, so its owner identity is proven not to match. Recovery for a legacy
72-
* record needs nothing special: the recording process still holds its token, and any other owner
73-
* is settled or conflicted by the principal-based paths, none of which reads a kind.
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.
7484
*/
7585
export function decodeStoredDeviceClaim(value: unknown): DeviceClaim | null {
7686
if (!isRecord(value)) return null;
7787
if (value.schemaVersion === DEVICE_CLAIM_SCHEMA_VERSION) return decodeCurrentClaim(value);
78-
if (value.schemaVersion === 2) return decodeClaimBody(value);
88+
if (value.schemaVersion === 2) return decodeClaimBody(value, 'unstated');
7989
if (value.schemaVersion === 1) return migrateLegacyClaim(value);
8090
return null;
8191
}
8292

8393
/**
84-
* The current schema states its claim kind explicitly: a missing `transientCommand` is an
85-
* open-session claim, never a pattern re-derived from the session label.
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.
8697
*/
8798
function decodeCurrentClaim(raw: Record<string, unknown>): DeviceClaim | null {
88-
const claim = decodeClaimBody(raw);
89-
if (!claim) return null;
90-
if (raw.transientCommand === undefined) return claim;
91-
return isNonEmptyString(raw.transientCommand)
92-
? { ...claim, transientCommand: raw.transientCommand }
93-
: 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);
94108
}
95109

96-
type DecodedClaimBody = Omit<DeviceClaim, 'transientCommand' | 'abandonedAtMs'> & {
97-
schemaVersion: 3;
98-
abandonedAtMs?: number;
99-
};
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+
}
100115

101-
function decodeClaimBody(raw: Record<string, unknown>): DecodedClaimBody | null {
116+
function decodeClaimBody(
117+
raw: Record<string, unknown>,
118+
claimOwnerKind: DeviceClaimOwnerKind,
119+
): DeviceClaim | null {
102120
if (!isRecord(raw.device) || !isNonEmptyString(raw.device.name)) return null;
103121
const identity = decodeDeviceIdentity(raw.device);
104-
return identity ? decodeClaimFields(raw, identity, raw.device.name) : null;
122+
return identity ? decodeClaimFields(raw, identity, raw.device.name, claimOwnerKind) : null;
105123
}
106124

107-
/** Released schema-v1 advisory claims are normalized into the canonical v3 model on read. */
108-
function migrateLegacyClaim(raw: Record<string, unknown>): DecodedClaimBody | null {
125+
/** Released schema-v1 claims differ only in device shape; they state no claim kind. */
126+
function migrateLegacyClaim(raw: Record<string, unknown>): DeviceClaim | null {
109127
if (!isRecord(raw.device)) return null;
110128
const legacy = raw.device;
111129
const name = legacy.name;
@@ -126,14 +144,15 @@ function migrateLegacyClaim(raw: Record<string, unknown>): DecodedClaimBody | nu
126144
? {}
127145
: { iosPhysicalDeviceBackend: legacy.iosPhysicalDeviceBackend }),
128146
});
129-
return identity ? decodeClaimFields(raw, identity, name) : null;
147+
return identity ? decodeClaimFields(raw, identity, name, 'unstated') : null;
130148
}
131149

132150
function decodeClaimFields(
133151
raw: Record<string, unknown>,
134152
identity: DeviceIdentity,
135153
name: string,
136-
): DecodedClaimBody | null {
154+
claimOwnerKind: DeviceClaimOwnerKind,
155+
): DeviceClaim | null {
137156
const location = decodeClaimLocation(raw);
138157
const owner = decodeClaimOwner(raw);
139158
const timestamps = decodeClaimTimestamps(raw);
@@ -145,13 +164,13 @@ function decodeClaimFields(
145164
device: { ...identity, name },
146165
...owner,
147166
...timestamps,
167+
claimOwnerKind,
148168
};
149169
}
150170

151171
/**
152172
* The claim `session` recorded for a command-scoped claim. Lives with the record so the label and
153-
* its inverse stay one owner; admission decides on {@link DeviceClaim.transientCommand}, never on
154-
* this shape.
173+
* its inverse stay one owner; `claimOwnerKind` is what decides a kind, never this shape.
155174
*/
156175
export function transientDeviceClaimSession(command: string): string {
157176
return `${TRANSIENT_CLAIM_LABEL_PREFIX}${command}`;

‎packages/daemon-contracts/src/device-claims.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@ export async function acquireTransientDeviceClaim(params: {
128128
* Whether the live claim under {@link coveredBySession} was taken by this request's
129129
* workspace rather than merely recorded under the same session string. Session
130130
* addresses are copyable (#3345), so session equality alone lets a caller that
131-
* pasted another workspace's address act as that workspace's owner. Only open
132-
* sessions' claims are ever candidates: a claim's `transientCommand` kind, not its
131+
* pasted another workspace's address act as that workspace's owner. Only an
132+
* `open-session` claim is ever a candidate: the record's `claimOwnerKind`, not its
133133
* label, decides whether it can cover anything.
134134
*/
135135
coveredClaimIsOwnedByRequest: (claim: DeviceClaim) => boolean;
@@ -146,18 +146,19 @@ export async function acquireTransientDeviceClaim(params: {
146146
!isAbandonedDeviceClaim(existing.claim) &&
147147
isClaimOwnedByThisDaemon(existing.claim, params.stateDir, readCurrentOwnerIdentity())
148148
) {
149+
const claimKind = existing.claim.claimOwnerKind;
149150
if (
150-
existing.claim.transientCommand === undefined &&
151+
claimKind === 'open-session' &&
151152
existing.claim.session === params.coveredBySession &&
152153
params.coveredClaimIsOwnedByRequest(existing.claim)
153154
) {
154155
return { status: 'covered-by-owned-claim' };
155156
}
156-
// A command-scoped claim belongs to the one request in flight, and the request that holds
157-
// it is covered by its release token at the admission layer — never by an address match.
158-
// The kind decides this, so a caller that copies the claim's `transient:<command>` label
159-
// into `--session` is refused instead of adopting it (#3345).
160-
if (existing.claim.transientCommand !== undefined) {
157+
// Only a record that states its kind can take part in a kind decision. A migrated record
158+
// states none, and `isClaimOwnedByThisDaemon` matches pid plus start time — which two
159+
// processes can share once a start time is unreadable — so coverage has to refuse rather
160+
// than assume an old record was not some other owner's command claim (#3345).
161+
if (claimKind !== 'open-session') {
161162
return { status: 'conflict', conflict: existing };
162163
}
163164
// A session literally named this command's label is not this command's claim either:
@@ -217,6 +218,7 @@ async function claimHeldDevice(params: {
217218
ownerToken: crypto.randomUUID(),
218219
createdAtMs: now,
219220
updatedAtMs: now,
221+
claimOwnerKind: params.transientCommand === undefined ? 'open-session' : 'transient-command',
220222
...(params.transientCommand === undefined ? {} : { transientCommand: params.transientCommand }),
221223
};
222224
writeDeviceClaim(claim);

‎packages/daemon/src/core/caller-scope-root.ts‎

Lines changed: 0 additions & 39 deletions
This file was deleted.

0 commit comments

Comments
 (0)