Skip to content

Commit 30e2b73

Browse files
committed
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.
1 parent e365e2a commit 30e2b73

1 file changed

Lines changed: 29 additions & 67 deletions

File tree

‎packages/daemon/src/resources/device/__tests__/device-claim-admission.test.ts‎

Lines changed: 29 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,10 @@ test('a claim this daemon holds for another session refuses the command like any
164164
expect(claimedSessions()).toEqual(['owning-session']);
165165
});
166166

167-
// The claim a transient command holds is command-scoped (`transient:<command>`), never
168-
// request-scoped, so the session comparison alone cannot tell two requests of the same
169-
// command apart. The one in flight must not be adopted — adopting it hands the second
170-
// request the first's release token, and its dispose unlinks a claim still in use.
171-
// A public session name equal to a transient claim's label must not read as that claim's
172-
// owner: an in-flight command's claim is exclusive even to a request that copies its
173-
// address, because adopting it hands the intruder the command's release token.
174-
// Mutation: read the claim kind off the session string instead of the record — the
175-
// open session named below then stops covering, or the in-flight claim starts covering.
167+
// Adopting a claim whose `transientCommand` is set hands the adopter the release token of a
168+
// command that is still running, and disposing it unlinks a claim in use. `--session` accepts
169+
// any name the store can sanitize, so a copy of the label is a reachable request; only the
170+
// stored kind can tell the holder's own session from that copy.
176171
test('an in-flight transient claim refuses a request named after its label, while a real open session of that name covers', async () => {
177172
const { stateDir } = setup();
178173
const holder = makeAdmission('transient-exclusive', stateDir, 'install', 'open-a');
@@ -209,12 +204,10 @@ test('an in-flight transient claim refuses a request named after its label, whil
209204
expect(claimedSessions()).toEqual(['transient:install']);
210205
});
211206

212-
// The label-guard branch in acquireTransientDeviceClaim: the open session's claim below is
213-
// named exactly this command's label, and a DIFFERENT session's `install` builds the same
214-
// label as its own claim session. Without the guard, resolveExistingClaim's raw string match
215-
// would hand that request the open session's release token (a renew-adopt), and disposing it
216-
// would delete a claim it never took. Mutation: drop the guard — the foreign session acquires,
217-
// and its dispose unlinks the open session's claim.
207+
// A claim's `session` is only a label to the code that renews a held claim, and renewing hands
208+
// the caller that claim's release token. `install` builds the label `transient:install`, so a
209+
// request in another session could otherwise renew an open session's claim of that name and
210+
// delete it on release. The stored kind is what refuses it.
218211
test("an open session named after a command's label is not renew-adopted by that command's requests", async () => {
219212
const { stateDir } = setup();
220213
const open = await acquireDeviceClaim({
@@ -232,6 +225,7 @@ test("an open session named after a command's label is not renew-adopted by that
232225
await foreign.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error),
233226
);
234227
expect(error.code).toBe('DEVICE_IN_USE');
228+
expect(error.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER');
235229
await foreign[Symbol.asyncDispose]();
236230

237231
// The open session's claim survived, with its original token: nothing was adopted.
@@ -240,13 +234,11 @@ test("an open session named after a command's label is not renew-adopted by that
240234
expect(await abandonDeviceClaim(open.ownership)).toBe('abandoned');
241235
});
242236

243-
// The safety argument for kindless legacy decoding, pinned rather than commented. A record that
244-
// predates the kind field decodes as an open-session claim, and that is only reachable at a kind
245-
// decision through `isClaimOwnedByThisDaemon` — same stateDir, pid, AND process start time —
246-
// which a live process's command claims can never be, because a live process writes v3 kinds.
247-
// This forges that window (the only way in) and pins what a kindless claim may and may not do:
248-
// it behaves exactly as a colon-named open session's claim, never as a command claim an in-flight
249-
// request can adopt or have adopted from it.
237+
// A legacy record carries no kind, so decoding it kindless is only safe while no kind decision
238+
// can reach one. That decision sits behind `isClaimOwnedByThisDaemon`, which matches stateDir,
239+
// pid, and process start time; a record this process wrote states its kind. A record that names
240+
// this live process identity without stating a kind is therefore forged or a PID-reuse survivor,
241+
// and here it is written by hand to pin what such a record may never do: act as a command claim.
250242
test('a kindless legacy record under this process identity reads as the named session, never as a command claim', async () => {
251243
const { stateDir, claimsDir } = setup();
252244
fs.mkdirSync(claimsDir, { recursive: true });
@@ -290,32 +282,10 @@ test('a kindless legacy record under this process identity reads as the named se
290282
expect(inspectDeviceClaims({})[0]?.claim?.ownerToken).toBe('legacy-token');
291283
});
292284

293-
test("another request in flight's transient claim of the same command refuses, and holds nothing", async () => {
294-
const { stateDir } = setup();
295-
const held = await acquireDeviceClaim({
296-
device: ANDROID_EMULATOR,
297-
session: 'transient:install',
298-
workspace: '/worktrees/current',
299-
stateDir,
300-
reconcileOrphanedDeviceClaim: retainOrphanedDeviceClaims,
301-
});
302-
expect(held.status).toBe('acquired');
303-
304-
const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'other-session');
305-
const error = asAppError(
306-
await admission.admit(ANDROID_EMULATOR, localAndroid).catch((error: unknown) => error),
307-
);
308-
309-
expect(error.code).toBe('DEVICE_IN_USE');
310-
expect(error.details?.reason).toBe('DEVICE_CLAIM_LIVE_OWNER');
311-
await admission[Symbol.asyncDispose]();
312-
expect(claimedSessions()).toEqual(['transient:install']);
313-
});
314-
315-
// The ownerToken exemption in device-claim-admission.ts. Mutation: delete that branch —
316-
// the second admit below throws DEVICE_IN_USE. Reachability: binding dedupes by
317-
// `deviceIdentityKey` (carries `target`) while the claim key is the narrower
318-
// `canonicalLocalDeviceKey`, so one device through two DeviceInfo shapes double-admits.
285+
// Binding dedupes a request's devices by `deviceIdentityKey`, which carries `target`, while the
286+
// claim key is the narrower `canonicalLocalDeviceKey`, so one device can reach admission twice.
287+
// A claim this request took is the only claim that covers the second pass; anything else must
288+
// still refuse rather than be adopted on the token this request wrote.
319289
test('a transient claim this request took covers re-admitting the same device', async () => {
320290
const { stateDir } = setup();
321291
const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'request-session');
@@ -329,11 +299,10 @@ test('a transient claim this request took covers re-admitting the same device',
329299
expect(inspectDeviceClaims({})).toEqual([]);
330300
});
331301

332-
// #3345's copyable-address door: session addresses are printed by `device status` and
333-
// re-enter through `--session`, so coverage requires the covered claim's workspace to
334-
// be the caller's own, canonicalized on both sides. Mutation for the pair: drop the
335-
// `coveredClaimIsOwnedByRequest` call in the contract's covered check — the foreign
336-
// workspace below then covers instead of refusing.
302+
// Session addresses are printed by `device status` and re-enter through `--session`, so
303+
// coverage has to compare what the claim records, not the string it carries. Both sides go
304+
// through the same canonicalization the router hashes into an implicit address, so one
305+
// checkout stays one workspace across its directories.
337306
test("a claim at this workspace's implicit address still covers its own workspace's command", async () => {
338307
const { root, stateDir } = setup();
339308
const workspace = path.join(root, 'worktree');
@@ -412,11 +381,9 @@ test('a request that carries no cwd keeps coverage of its session\u2019s claim',
412381
expect(claimedSessions()).toEqual(['sdk-session']);
413382
});
414383

415-
// The release path is where a transient claim outlives its request: an unlink that
416-
// throws leaves the record live, and a live claim now refuses its own session's next
417-
// command. This injects the failure at the real unlink inside the real clear
418-
// transaction and pins the recovery: the survivor ends abandoned, so the owner's next
419-
// acquire supersedes it — the shape acquireTransientDeviceClaim already handles.
384+
// A transient claim outlives its request when the release unlink fails, and a live survivor now
385+
// refuses its own session's next command. The recovery is to abandon the record the unlink left
386+
// behind, which is the state acquireTransientDeviceClaim already supersedes.
420387
test('a failed release abandons the claim so the next command is not locked out', async () => {
421388
const { stateDir } = setup();
422389
const admission = makeAdmission('transient-exclusive', stateDir, 'install', 'own-session');
@@ -609,15 +576,10 @@ test('the request scope claims only for commands whose descriptor declares trans
609576
expect(inspectDeviceClaims({})).toEqual([]);
610577
});
611578

612-
// #3345 at the seam the router runs: one daemon serves several sessions and
613-
// worktrees, so the owning session's `install` has to keep working while
614-
// another session's `install` is refused before its handler can reach a device
615-
// operation.
616-
// The P1 attack in the shape the CLI sends it: `--session transient:install` is a legal
617-
// explicit name (the store sanitizes only directory segments), so the router hands admission
618-
// that exact address. It must not read as the owner of the in-flight command's claim — the
619-
// claim's kind, not its label, decides — and the refusal must leave the first command holding
620-
// the device.
579+
// `--session transient:install` is a legal explicit name — the store sanitizes only directory
580+
// segments — so the router hands admission that exact address. It must not read as the owner of
581+
// a command claim that is still in flight, and the refusal must leave that command holding the
582+
// device.
621583
test('the request scope refuses a session addressed by a transient label while a command holds the device', async () => {
622584
const { stateDir } = setup();
623585
const holder = makeAdmission('transient-exclusive', stateDir, 'install', 'open-a');

0 commit comments

Comments
 (0)