Skip to content

Commit 01fcba5

Browse files
committed
fix: enforce device claims for sessionless device mutations
`boot` and `shutdown` never consulted the host-global device claim store, so a daemon in one state directory could terminate an emulator another daemon held a verified-live claim on and report success (#1799). Rather than adding a claim check to those two handlers, this makes the class unrepresentable: `CommandDescriptor` gains a REQUIRED `deviceClaimPolicy` trait (#1320's vocabulary), and the request-execution scope enforces it where the request runtime bindings create a device binding — the one seam through which any handler can obtain device operations, and already the place per-device deduplication lives. A `transient-exclusive` command acquires a command-scoped claim before operations reach the handler, refuses a foreign live claim with the existing DEVICE_IN_USE/DEVICE_CLAIM_LIVE_OWNER error, and releases in the scope's finally. Every other policy performs no claim-store I/O, so session-bound commands keep #1320's non-goal intact. `hover` also gains the `frameworkTier` it was missing: #1786 landed after #1804's branch point, leaving main's own parity gate red.
1 parent 142d156 commit 01fcba5

16 files changed

Lines changed: 951 additions & 83 deletions

CONTEXT.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ task touches:
6363
cloud bridge, or `limrun`.
6464
- Runner/process lease: backend helper mutual-exclusion guard for platform runners or tools; it is
6565
not the remote client ownership boundary.
66+
- Device claim: host-global exclusive ownership of one local device, held by an open session or by a
67+
single sessionless device-mutating command. Local only; remote targets use device leases instead.
68+
- Device-claim policy: required command-descriptor trait declaring a command's relationship to the
69+
claim store (`none`, `observe`, `require-owner`, `transient-exclusive`, `acquire-session`,
70+
`release-session`). The request-execution scope enforces it at the device binding seam:
71+
`transient-exclusive` takes a command-scoped claim and refuses a foreign one, and every other
72+
policy performs no claim-store I/O.
6673
- iOS physical-device control: Apple-local module selected from discovery evidence. CoreDevice
6774
devices retain the `devicectl` controller; devices found only by `xctrace` use the XCTest
6875
controller for readiness, app activation/termination, and cable-bound usbmux runner transport
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import { afterEach } from 'vitest';
4+
import { mkdtempForTestSync } from './tmp-dir.ts';
5+
import type { DeviceClaimReconciler } from '../../daemon/device-claims.ts';
6+
7+
export type IsolatedDeviceClaimStore = {
8+
/** Temporary root holding both the claim store and the daemon state dir. */
9+
root: string;
10+
stateDir: string;
11+
claimsDir: string;
12+
};
13+
14+
/**
15+
* Device claims are host-global by design, so a test that exercises them must
16+
* redirect the store. Registers cleanup once and returns a per-test factory.
17+
*/
18+
export function isolatedDeviceClaimStores(prefix: string): () => IsolatedDeviceClaimStore {
19+
const roots: string[] = [];
20+
afterEach(() => {
21+
delete process.env.AGENT_DEVICE_CLAIMS_DIR;
22+
for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true });
23+
});
24+
return () => {
25+
const root = mkdtempForTestSync(prefix);
26+
roots.push(root);
27+
const claimsDir = path.join(root, 'claims');
28+
process.env.AGENT_DEVICE_CLAIMS_DIR = claimsDir;
29+
const stateDir = path.join(root, 'state');
30+
fs.mkdirSync(stateDir, { recursive: true });
31+
return { root, stateDir, claimsDir };
32+
};
33+
}
34+
35+
/** Fail-closed reconciler: a test owner is never treated as recoverable. */
36+
export const retainOrphanedDeviceClaims: DeviceClaimReconciler = async () => ({
37+
status: 'retained',
38+
reason: 'test-live-owner',
39+
});
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { test } from 'vitest';
2+
import assert from 'node:assert/strict';
3+
import { PUBLIC_COMMANDS } from '../../../command-catalog.ts';
4+
import { commandDescriptors, resolveCommandDeviceClaimPolicy } from '../registry.ts';
5+
import type { DeviceClaimPolicy } from '../types.ts';
6+
7+
// #1320 completeness gate. TypeScript already makes the trait required on every
8+
// raw descriptor, so these tests only pin what types cannot: the CLASSIFICATION,
9+
// and the structural precondition that makes it enforceable.
10+
11+
function commandsByPolicy(): Partial<Record<DeviceClaimPolicy, string[]>> {
12+
const grouped: Partial<Record<DeviceClaimPolicy, string[]>> = {};
13+
for (const descriptor of commandDescriptors) {
14+
(grouped[descriptor.deviceClaimPolicy] ??= []).push(descriptor.name);
15+
}
16+
for (const names of Object.values(grouped)) names.sort();
17+
return grouped;
18+
}
19+
20+
test('every public command resolves the policy its descriptor declares', () => {
21+
const byName = new Map(commandDescriptors.map((descriptor) => [descriptor.name, descriptor]));
22+
for (const command of Object.values(PUBLIC_COMMANDS)) {
23+
const descriptor = byName.get(command);
24+
assert.ok(descriptor, `public command ${command} is missing from the descriptor registry`);
25+
assert.equal(resolveCommandDeviceClaimPolicy(command), descriptor.deviceClaimPolicy);
26+
}
27+
// Command names outside the registry stay claim-free rather than fail closed
28+
// into an acquisition no owner would ever release.
29+
assert.equal(resolveCommandDeviceClaimPolicy(undefined), 'require-owner');
30+
assert.equal(resolveCommandDeviceClaimPolicy('not-a-registered-command'), 'require-owner');
31+
});
32+
33+
test('every command that deviates from require-owner is a reviewed, diffable set', () => {
34+
// CONSERVATIVE: these lists may only change in the same PR that updates them
35+
// here. A `transient-exclusive` command takes host-global exclusive ownership
36+
// of its device for one request, so adding one changes cross-worktree
37+
// behavior for everyone sharing that device (#1799); `none` is for
38+
// host/config-only commands and pure delegators, whose device work runs inside
39+
// the request scope of the command they dispatch.
40+
const { 'require-owner': _sessionBound, ...deviating } = commandsByPolicy();
41+
assert.deepEqual(deviating, {
42+
'acquire-session': ['open'],
43+
'release-session': ['close'],
44+
'transient-exclusive': [
45+
'boot',
46+
'install',
47+
'install_source',
48+
'prepare',
49+
'push',
50+
'reinstall',
51+
'shutdown',
52+
],
53+
observe: ['apps', 'appstate', 'capabilities', 'device', 'devices', 'doctor'],
54+
none: [
55+
'artifacts',
56+
'auth',
57+
'batch',
58+
'cdp',
59+
'connect',
60+
'connection',
61+
'daemon',
62+
'debug',
63+
'disconnect',
64+
'install-from-source',
65+
'lease_allocate',
66+
'lease_heartbeat',
67+
'lease_release',
68+
'mcp',
69+
'metro',
70+
'proxy',
71+
'react-devtools',
72+
'release_materialized_paths',
73+
'session',
74+
'session_list',
75+
'session_save_script',
76+
'web',
77+
],
78+
});
79+
});
80+
81+
test('a transient-exclusive command can actually reach the device binding seam', () => {
82+
// The claim gate lives on the request scope's device binding, which only ADR
83+
// 0019 `device-runtime` commands pass through: an unmigrated (`legacy`)
84+
// command reaches its device through dispatch instead, so declaring
85+
// `transient-exclusive` there would be a claim nobody ever acquires. Live
86+
// verification of #1799 caught exactly that on `keyboard`. Those commands stay
87+
// `require-owner` until their platform execution migrates.
88+
for (const descriptor of commandDescriptors) {
89+
if (descriptor.deviceClaimPolicy !== 'transient-exclusive') continue;
90+
assert.ok(descriptor.daemon, `${descriptor.name}: transient-exclusive without a daemon route`);
91+
assert.equal(
92+
descriptor.platformExecution.kind,
93+
'device-runtime',
94+
`${descriptor.name}: transient-exclusive cannot be enforced without device-runtime execution`,
95+
);
96+
}
97+
});

0 commit comments

Comments
 (0)