Skip to content

Commit 3c99486

Browse files
committed
refactor(record): spread the request scope into manifest re-resolution and show it in the request log
The manifest lane hand-picked the scope and lease keys to forward; a key added to ResolveDeviceFlags later would be dropped silently on this lane. Admission (namesSessionWithoutTarget) already proves the request states no device, udid, serial, platform, or target, so spreading its flags carries exactly the scope and lease keys — now and later — while the manifest still decides family and identity. This deletes LeaseDiscoveryContext, its key list, and the helper. The resolve_target_device phase now echoes the requested udid and simulator set when the request states them, so a manifest-first recovery is provable from the request log alone (the maintainer's scoped-set evidence requirement). Pinned: dropping the spread fails all three recovery scope pins; dropping either echoed field fails the new phase test.
1 parent d1b6071 commit 3c99486

7 files changed

Lines changed: 80 additions & 52 deletions

File tree

‎packages/daemon/src/core/device/device-selector-intent.ts‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,39 +8,6 @@ const LOCKABLE_DEVICE_SCOPE_KEYS: ReadonlyArray<keyof CommandFlags> = [
88
'androidDeviceAllowlist',
99
];
1010

11-
/**
12-
* Request-scoped lease and provider-discovery context. These say WHO may see which devices
13-
* (a lease or a provider client narrows the inventory the resolver can reach), never WHICH
14-
* device, so a lane that rebuilds a selector must forward them or resolution silently
15-
* widens outside the lease. `device-selector-intent` classifies selectors; this is the one
16-
* read that keeps that classification's non-identity discovery keys together.
17-
*/
18-
export type LeaseDiscoveryContext = {
19-
leaseId?: string;
20-
leaseProvider?: string;
21-
deviceKey?: string;
22-
clientId?: string;
23-
};
24-
25-
const LEASE_DISCOVERY_KEYS = [
26-
'leaseId',
27-
'leaseProvider',
28-
'deviceKey',
29-
'clientId',
30-
] as const satisfies ReadonlyArray<keyof LeaseDiscoveryContext & keyof CommandFlags>;
31-
32-
export function leaseDiscoveryContextFromFlags(
33-
flags: CommandFlags | undefined,
34-
): LeaseDiscoveryContext {
35-
if (!flags) return {};
36-
const context: Record<string, string> = {};
37-
for (const key of LEASE_DISCOVERY_KEYS) {
38-
const value = flags[key];
39-
if (typeof value === 'string' && value.trim().length > 0) context[key] = value;
40-
}
41-
return context;
42-
}
43-
4411
export function hasExplicitDeviceSelector(flags: CommandFlags | undefined): boolean {
4512
return hasExplicitDeviceIdentitySelector({
4613
deviceName: flags?.device,

‎packages/daemon/src/sessions/handlers/record-runtime.ts‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,7 @@ import {
2929
resolveTargetDevice,
3030
type ResolveDeviceFlags,
3131
} from '@agent-device/device-selection/dispatch-resolve';
32-
import {
33-
hasDeviceSelectionInput,
34-
leaseDiscoveryContextFromFlags,
35-
} from '../../core/device/device-selector-intent.ts';
32+
import { hasDeviceSelectionInput } from '../../core/device/device-selector-intent.ts';
3633
import { ensureBoundDeviceReady } from '../../core/request-runtime-binding.ts';
3734
import { type ScreenRecordingAdmissionLedger } from '@agent-device/capture-kit/screen-recording-admission-ledger';
3835
import { createScreenRecordingRecoveryControl } from '@agent-device/capture-kit/screen-recording-recovery';
@@ -176,9 +173,10 @@ async function resolveRecordingSession(
176173
*
177174
* A request that names a session this daemon does not hold and states no device is answered from
178175
* the durable manifest the session address finds, never from a sweep of every device on the host:
179-
* a decodable manifest is re-resolved under the manifest's family (plus the request's host
180-
* scoping), and a start or a stop with no decodable manifest to recover from refuses with the
181-
* named-session error. `assertManifestBelongsToRequest` owns the proof that the resolved device
176+
* a decodable manifest is re-resolved with the request's scope and lease keys under the manifest's
177+
* family (admission proved the request states no device), and a start or a stop with no decodable
178+
* manifest to recover from refuses with the named-session error.
179+
* `assertManifestBelongsToRequest` owns the proof that the resolved device
182180
* and session address match the manifest.
183181
*
184182
* The two shapes that keep their chosen target are unchanged: a request that never named a
@@ -210,24 +208,20 @@ function namesSessionWithoutTarget(req: DaemonRequest): boolean {
210208
}
211209

212210
/**
213-
* Re-resolves the device a durable manifest names: the manifest's family always, its identity flag
214-
* where the family has one, plus the request's host-scoping and lease/discovery selectors —
215-
* `iosSimulatorDeviceSet` and `androidDeviceAllowlist` say WHERE to look, the lease fields say WHO
216-
* may see the inventory, and none of them states WHICH device. Dropping any of them would resolve
217-
* a scoped or leased recording outside the scope or lease it started under. No request
218-
* device/identity selector reaches here: `assertManifestBelongsToRequest` proves the resolved
219-
* device against the manifest.
211+
* Re-resolves the device a durable manifest names. Admission (`namesSessionWithoutTarget`) has
212+
* already proven this request states no device, udid, serial, platform, or target, so spreading
213+
* its flags carries exactly the scope and lease keys `ResolveDeviceFlags` declares — now and
214+
* later — without carrying any device choice. The manifest then decides which device: family
215+
* always, identity flag where the family has one. `assertManifestBelongsToRequest` proves the
216+
* resolved device against the manifest.
220217
*/
221218
async function resolveManifestDevice(
222219
params: RecordRuntimeHandlerParams,
223220
recorded: DeviceIdentity,
224221
): Promise<DeviceInfo> {
225-
const { iosSimulatorDeviceSet, androidDeviceAllowlist } = params.req.flags ?? {};
226222
const selector: ResolveDeviceFlags = {
223+
...(params.req.flags ?? {}),
227224
platform: recorded.family,
228-
...leaseDiscoveryContextFromFlags(params.req.flags),
229-
...(iosSimulatorDeviceSet === undefined ? {} : { iosSimulatorDeviceSet }),
230-
...(androidDeviceAllowlist === undefined ? {} : { androidDeviceAllowlist }),
231225
};
232226
const identityFlag = deviceIdentityFlag(recorded.family);
233227
if (identityFlag) selector[identityFlag] = recorded.id;

‎packages/device-selection/package.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,8 @@
3535
"types": "./src/inventory-gateways.fixtures.ts",
3636
"default": "./src/inventory-gateways.fixtures.ts"
3737
}
38+
},
39+
"devDependencies": {
40+
"@agent-device/test-kit": "workspace:*"
3841
}
3942
}

‎packages/device-selection/src/__tests__/dispatch-resolve.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { afterEach, beforeEach, test, vi } from 'vitest';
22
import assert from 'node:assert/strict';
3+
import fs from 'node:fs';
4+
import { mkdtempForTestSync } from '@agent-device/test-kit';
5+
import {
6+
flushDiagnosticsToSessionFile,
7+
withDiagnosticsScope,
8+
} from '@agent-device/host-kit/diagnostics';
39

410
const mockFindIosSimulatorInstalledApp = vi.fn();
511
const mockListAppleDevices = vi.fn();
@@ -599,6 +605,42 @@ test('resolveTargetDevice keeps simulator-set scope separate from the macOS host
599605
assert.equal(requests[1]?.iosSimulatorSetPath, undefined);
600606
});
601607

608+
test('the resolve_target_device phase shows the requested udid and simulator set', async () => {
609+
// Recovery lanes answer FROM the request log: a manifest-first stop must be provable from the
610+
// phase line alone, so the requested identity and the set it was scoped to are echoed there.
611+
const logDir = mkdtempForTestSync('dispatch-resolve-diag-');
612+
const logPath = `${logDir}/requests.ndjson`;
613+
await withTestDeviceInventory(
614+
{
615+
local: async () => [simulator],
616+
findInstalledApp: mockFindIosSimulatorInstalledApp,
617+
},
618+
async () => {
619+
await withDiagnosticsScope(
620+
{ session: 'qa', command: 'record', debug: true, logPath },
621+
async () => {
622+
await resolveTargetDeviceInContext({
623+
platform: 'apple',
624+
udid: simulator.id,
625+
iosSimulatorDeviceSet: '/tmp/recovery-set',
626+
});
627+
flushDiagnosticsToSessionFile({ force: true });
628+
},
629+
);
630+
},
631+
);
632+
633+
const phases = fs
634+
.readFileSync(logPath, 'utf8')
635+
.split('\n')
636+
.filter(Boolean)
637+
.map((line) => JSON.parse(line) as { phase: string; data?: Record<string, unknown> })
638+
.filter((entry) => entry.phase === 'resolve_target_device');
639+
assert.equal(phases.length, 1);
640+
assert.equal(phases[0]?.data?.udid, simulator.id);
641+
assert.equal(phases[0]?.data?.iosSimulatorSetPath, '/tmp/recovery-set');
642+
});
643+
602644
test('resolveTargetDevice discovers simulators in the daemon operator set when the request names none', async () => {
603645
vi.stubEnv('AGENT_DEVICE_IOS_SIMULATOR_DEVICE_SET', '/tmp/operator-set');
604646
const requests: DeviceInventoryRequest[] = [];

‎packages/device-selection/src/dispatch-resolve.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ export async function resolveTargetDeviceSelection(
104104
const diagnosticData = {
105105
platform: inventoryRequest.platform,
106106
target: flags.target,
107+
// The selector the resolution was asked for, echoed for recovery lanes whose whole job is
108+
// re-resolving one stated device: which udid was requested and which simulator set was
109+
// scoped must be visible in the request log without debug-only dumps.
110+
...(inventoryRequest.udid ? { udid: inventoryRequest.udid } : {}),
111+
...(iosSimulatorSetPath ? { iosSimulatorSetPath } : {}),
107112
cacheHit: false,
108113
};
109114
return await withDiagnosticTimer(

‎packages/device-selection/tsconfig.json‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,18 @@
1010
"tsBuildInfoFile": "./dist-types/index.tsbuildinfo"
1111
},
1212
"include": ["src"],
13-
"references": [{ "path": "../contracts" }, { "path": "../host-kit" }, { "path": "../kernel" }]
13+
"references": [
14+
{
15+
"path": "../contracts"
16+
},
17+
{
18+
"path": "../host-kit"
19+
},
20+
{
21+
"path": "../kernel"
22+
},
23+
{
24+
"path": "../test-kit"
25+
}
26+
]
1427
}

‎pnpm-lock.yaml‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)