diff --git a/packages/device-selection/src/inventory-gateways.fixtures.ts b/packages/device-selection/src/inventory-gateways.fixtures.ts index fc9b3fa734..190cbe1fd6 100644 --- a/packages/device-selection/src/inventory-gateways.fixtures.ts +++ b/packages/device-selection/src/inventory-gateways.fixtures.ts @@ -24,7 +24,7 @@ export type TestDeviceInventoryOptions = Readonly<{ findInstalledApp?: InstalledAppProbe; }>; -const testRequestScope: PlatformRequestScope = Object.freeze({ +export const testRequestScope: PlatformRequestScope = Object.freeze({ signal: new AbortController().signal, diagnostics: Object.freeze({ emit: () => {} }), progress: Object.freeze({ report: () => {} }), diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index 00f5860c8b..d08c7e13dd 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -132,6 +132,7 @@ import { recordRuntimeRegistryJoinViolations } from './record-runtime-registry-p import { recordRuntimeDaemonMechanicsViolations } from './record-runtime-mechanics-policy.ts'; import { checkDaemonPlatformBoundary } from './daemon-platform-boundary.ts'; import { checkDaemonPlatformRuntimeInventory } from './daemon-platform-runtime-inventory.ts'; +import { checkDaemonProcessReach } from './daemon-process-reach.ts'; import { checkDaemonClientEntry } from './daemon-client-entry.ts'; import { checkDaemonLayers, DAEMON_ROOT } from './daemon-layers.ts'; import { checkSessionAuthorityOverlay, handlerOwnedOverlay } from './session-authority-overlay.ts'; @@ -433,7 +434,8 @@ function report( `free of ` + `concrete platform imports in every executable and type-only form; ` + `no production daemon file reaches the root platform-runtime family or a hub that ` + - `does (R76); and the handler-owned SessionState/SessionStore ` + + `does (R76); the daemon process reaches no caller-side replay source module over ` + + `static or dynamic edges (R82); and the handler-owned SessionState/SessionStore ` + `authority overlay holds at or under the merge-base (R75, ` + `${handlerOwnedShapeFiles} shape / ${handlerOwnedAuthorityFiles} authority files); and ` + `every production daemon file lives under a layer folder with no upward static edges ` + @@ -516,6 +518,7 @@ export const LAYERING_RULE_IDS = [ 'provider-snapshot-presentation-ownership', 'snapshot-assembly-presentation-neutrality', 'daemon-platform-runtime-inventory', + 'daemon-process-reach', 'daemon-client-entry', 'session-authority-overlay', 'daemon-layers', @@ -580,6 +583,7 @@ export const LAYERING_RULES: Readonly> = { snapshotAssemblyPresentationViolations(context.sources, context.edges), 'daemon-platform-runtime-inventory': (context) => checkDaemonPlatformRuntimeInventory(context.edges), + 'daemon-process-reach': (context) => checkDaemonProcessReach(context.sourceFiles, context.edges), 'daemon-client-entry': (context) => checkDaemonClientEntry(context.edges), 'session-authority-overlay': (context) => checkSessionAuthorityOverlay( diff --git a/scripts/layering/client-only-subpaths.test.ts b/scripts/layering/client-only-subpaths.test.ts deleted file mode 100644 index 6cc4903a96..0000000000 --- a/scripts/layering/client-only-subpaths.test.ts +++ /dev/null @@ -1,56 +0,0 @@ -// `CLIENT_ONLY_SUBPATHS` in package-boundaries.ts: the caller-side replay source collector stays -// out of daemon production code, which receives the client's source bundle in the request. - -import assert from 'node:assert/strict'; -import { test } from 'node:test'; -import { - CLIENT_ONLY_SUBPATHS, - checkPackageInternalSites, - checkRootSites, - specifierSites, - type WorkspacePackage, -} from './package-boundaries.ts'; - -const daemon: WorkspacePackage = { - dir: 'packages/daemon', - name: '@agent-device/daemon', - exportTargets: new Map(), - workspaceDependencies: new Set(['@agent-device/replay-port']), - workspaceDependencyField: 'dependencies', - externalDependencies: new Map(), -}; - -test('the caller-side replay source subpaths are denied to daemon production code only', () => { - // Pin the set independently: iterating the checker's own set would let deleting an entry - // silently delete its accept/deny coverage with it. - assert.deepEqual([...CLIENT_ONLY_SUBPATHS].sort(), [ - '@agent-device/replay-port/script-source-bundle', - '@agent-device/replay-port/source-discovery', - ]); - const rootDeps = new Set(daemon.workspaceDependencies); - for (const specifier of CLIENT_ONLY_SUBPATHS) { - const sites = (file: string) => specifierSites(file, `import { f } from '${specifier}';`); - const check = (file: string) => - file.startsWith('src/') - ? checkRootSites(sites(file), [], rootDeps, true) - : checkPackageInternalSites(daemon, sites(file), [], true); - for (const file of [ - 'src/commands/replay/index.ts', - 'packages/daemon/src/__tests__/replay.test.ts', - ]) { - assert.deepEqual( - check(file).filter((v) => /caller-side replay sources/.test(v.message)), - [], - file, - ); - } - for (const file of [ - 'packages/daemon/src/replay-runtime.ts', - 'src/daemon.ts', - 'src/daemon-composition.ts', - ]) { - const violations = check(file).filter((v) => /caller-side replay sources/.test(v.message)); - assert.equal(violations.length, 1, file); - } - } -}); diff --git a/scripts/layering/daemon-process-reach.test.ts b/scripts/layering/daemon-process-reach.test.ts new file mode 100644 index 0000000000..826ec4d0c4 --- /dev/null +++ b/scripts/layering/daemon-process-reach.test.ts @@ -0,0 +1,88 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { resolveImportEdges } from './model.ts'; +import { + checkDaemonProcessReach, + DAEMON_PROCESS_DENIED_MODULES, + DAEMON_PROCESS_REACH_RULE, +} from './daemon-process-reach.ts'; + +const COLLECTOR = 'packages/replay-port/src/source-discovery.ts'; + +/** Every denied module exists as a stub, so a case fails only on what it plants. */ +function reach(sources: Record) { + const tree = new Map([ + ...DAEMON_PROCESS_DENIED_MODULES.map( + (module) => [module, 'export const collect = 1;\n'] as const, + ), + ...Object.entries(sources), + ]); + return checkDaemonProcessReach([...tree.keys()], resolveImportEdges(tree)); +} + +test('the denied set names replay-port caller-side collector modules', () => { + // Pinned independently so deleting an entry fails here instead of silently shrinking the rule. + assert.deepEqual([...DAEMON_PROCESS_DENIED_MODULES].sort(), [ + 'packages/replay-port/src/script-source-bundle.ts', + 'packages/replay-port/src/source-discovery.ts', + ]); +}); + +for (const denied of DAEMON_PROCESS_DENIED_MODULES) { + test(`a root platform-runtime module behind a dynamic import cannot reach ${denied}`, () => { + const found = reach({ + 'src/daemon.ts': "export const load = () => import('./daemon-composition.ts');\n", + 'src/daemon-composition.ts': "import './platform-runtime-daemon-services.ts';\n", + 'src/platform-runtime-daemon-services.ts': `import '../${denied}';\n`, + }); + assert.equal(found.length, 1); + assert.equal(found[0]!.rule, DAEMON_PROCESS_REACH_RULE); + assert.equal(found[0]!.file, 'src/platform-runtime-daemon-services.ts'); + assert.ok( + found[0]!.message.includes( + `Chain: src/daemon.ts -> src/daemon-composition.ts -> src/platform-runtime-daemon-services.ts -> ${denied}.`, + ), + found[0]!.message, + ); + }); + + test(`a static-only chain from the entry cannot reach ${denied}`, () => { + const found = reach({ + 'src/daemon.ts': "import './daemon-server.ts';\n", + 'src/daemon-server.ts': `import '../${denied}';\n`, + }); + assert.equal(found.length, 1); + assert.equal(found[0]!.file, 'src/daemon-server.ts'); + assert.equal(found[0]!.line, 1); + assert.ok( + found[0]!.message.includes(`Chain: src/daemon.ts -> src/daemon-server.ts -> ${denied}.`), + found[0]!.message, + ); + }); +} + +test('a type-only edge loads nothing, and the client may import the collector', () => { + assert.deepEqual( + reach({ + 'src/daemon.ts': + "import type { Collect } from '../packages/replay-port/src/source-discovery.ts';\n", + 'src/commands/replay/index.ts': + "import '../../../packages/replay-port/src/source-discovery.ts';\n", + [COLLECTOR]: 'export type Collect = () => [];\n', + }), + [], + ); +}); + +test('a renamed entry or denied module fails as stale instead of passing silently', () => { + const found = checkDaemonProcessReach( + ['src/daemon.ts', 'packages/replay-port/src/script-source-bundle.ts'], + [], + ); + assert.deepEqual( + found.map((violation) => violation.message.split(' is not')[0]), + [`stale module: ${COLLECTOR}`], + ); + assert.equal(found[0]!.rule, DAEMON_PROCESS_REACH_RULE); + assert.equal(checkDaemonProcessReach([COLLECTOR], [], 'src/daemon.ts', [COLLECTOR]).length, 1); +}); diff --git a/scripts/layering/daemon-process-reach.ts b/scripts/layering/daemon-process-reach.ts new file mode 100644 index 0000000000..43992933ec --- /dev/null +++ b/scripts/layering/daemon-process-reach.ts @@ -0,0 +1,81 @@ +import type { LayeringViolation, ResolvedImportEdge } from './model.ts'; + +export const DAEMON_PROCESS_REACH_RULE = 'R82 daemon-process-reach'; + +/** The module a daemon process starts from. */ +export const DAEMON_PROCESS_ENTRY = 'src/daemon.ts'; + +/** + * Modules the daemon process must never load. The client reads the caller's replay scripts and + * ships their contents in the request; the daemon only ever sees that bundle, never caller paths. + */ +export const DAEMON_PROCESS_DENIED_MODULES: readonly string[] = [ + 'packages/replay-port/src/script-source-bundle.ts', + 'packages/replay-port/src/source-discovery.ts', +]; + +/** + * Catches: any module the daemon process can load — over static and dynamic edges, so the + * composition root and every platform-runtime module behind a function-scoped `await import` + * count — reaching a denied module. The violation names the edge that brings the denied module + * in and the shortest chain from the entry to it. An entry or denied module that is not a + * tracked source fails as stale, so a rename or move cannot turn the walk into a silent pass. + * Evidence: the client-only placement of replay-port's caller-side collector was a convention + * until #3517; a path-scoped R11 gate covered only files under the daemon root plus two named + * root files, so a root platform-runtime module or a reached package could still import it. + * Cost: one breadth-first walk over the already-resolved edges, inside `pnpm check:layering`. + * Kill criterion: the collector moves into a package the daemon package cannot depend on. + */ +export function checkDaemonProcessReach( + sourceFiles: readonly string[], + edges: readonly ResolvedImportEdge[], + entry: string = DAEMON_PROCESS_ENTRY, + denied: readonly string[] = DAEMON_PROCESS_DENIED_MODULES, +): LayeringViolation[] { + const tracked = new Set(sourceFiles); + const stale = [entry, ...denied].filter((module) => !tracked.has(module)); + if (stale.length > 0) { + return stale.map((module) => ({ + rule: DAEMON_PROCESS_REACH_RULE, + file: 'scripts/layering/daemon-process-reach.ts', + line: 1, + message: + `stale module: ${module} is not a tracked source. Point DAEMON_PROCESS_ENTRY or ` + + `DAEMON_PROCESS_DENIED_MODULES at its new path so the reach check keeps a real target.`, + })); + } + const outgoing = new Map(); + for (const edge of edges) { + if (edge.typeOnly && !edge.dynamic) continue; + const list = outgoing.get(edge.file) ?? []; + list.push(edge); + outgoing.set(edge.file, list); + } + const cameFrom = new Map([[entry, null]]); + const queue = [entry]; + for (let head = 0; head < queue.length; head += 1) { + const current = queue[head]!; + for (const edge of outgoing.get(current) ?? []) { + if (cameFrom.has(edge.target)) continue; + cameFrom.set(edge.target, edge); + queue.push(edge.target); + } + } + const violations: LayeringViolation[] = []; + for (const module of denied) { + const via = cameFrom.get(module); + if (!via) continue; + const chain: string[] = [module]; + for (let at = via; at; at = cameFrom.get(at.file) ?? null) chain.unshift(at.file); + violations.push({ + rule: DAEMON_PROCESS_REACH_RULE, + file: via.file, + line: via.line, + message: + `${entry} reaches ${module}, which the daemon process must never load: the daemon ` + + `receives the client's replay source bundle and does not read caller paths. ` + + `Chain: ${chain.join(' -> ')}.`, + }); + } + return violations; +} diff --git a/scripts/layering/package-boundaries.ts b/scripts/layering/package-boundaries.ts index 3f71bce35e..22f59b864d 100644 --- a/scripts/layering/package-boundaries.ts +++ b/scripts/layering/package-boundaries.ts @@ -26,17 +26,12 @@ // instanceof), and, for `scripts/` files which sit outside the type graph by design, the // root-side unknown-package and exports-map checks that compiled files get from TS2307. // On top, `TEST_ONLY_FIXTURE_SUBPATHS` mirrors R13's Android fixture gate for the non-platform -// packages: a production file importing one of those fixture subpaths is an R11 violation, and -// `CLIENT_ONLY_SUBPATHS` keeps the caller-side replay source collector out of daemon code. +// packages: a production file importing one of those fixture subpaths is an R11 violation. import fs from 'node:fs'; import path from 'node:path'; import { parseImports, resolveWorkspaceSpecifier } from './model.ts'; -import { - DAEMON_SOURCE_ROOT, - listTrackedPackageManifests, - listTrackedProductionSources, -} from './tracked-sources.ts'; +import { listTrackedPackageManifests, listTrackedProductionSources } from './tracked-sources.ts'; export type PackageBoundaryViolation = { rule: string; @@ -263,30 +258,6 @@ function importsTestOnlyFixtureFromProduction(file: string, specifier: string): return TEST_ONLY_FIXTURE_SUBPATHS.has(specifier) && !isTestConsumerFile(file); } -/** - * Caller-side replay source acquisition: the client reads script files from the caller's - * filesystem and ships their contents in the request. Daemon production code receives that - * bundle and must never read caller paths itself, so it may not import these subpaths. - */ -export const CLIENT_ONLY_SUBPATHS: ReadonlySet = new Set([ - '@agent-device/replay-port/script-source-bundle', - '@agent-device/replay-port/source-discovery', -]); - -// The daemon process also runs its root entry and composition root, outside the daemon package. -const DAEMON_PROCESS_ROOT_FILES: ReadonlySet = new Set([ - 'src/daemon.ts', - 'src/daemon-composition.ts', -]); - -function importsClientOnlyFromDaemon(file: string, specifier: string): boolean { - return ( - CLIENT_ONLY_SUBPATHS.has(specifier) && - (file.startsWith(DAEMON_SOURCE_ROOT) || DAEMON_PROCESS_ROOT_FILES.has(file)) && - !isTestConsumerFile(file) - ); -} - /** * Rules for files INSIDE a package. An undeclared sibling import is R11's alone: pnpm links * every workspace package under the root `node_modules`, so the import resolves, compiles, and @@ -345,16 +316,6 @@ export function checkPackageInternalSites( `import it; use the package's production subpath instead.`, }); } - if (importsClientOnlyFromDaemon(site.file, site.specifier)) { - violations.push({ - rule: 'R11 package-boundaries', - file: site.file, - line: site.line, - message: - `'${site.specifier}' reads caller-side replay sources — daemon code receives the ` + - `client's source bundle and may not read caller paths; take it from the request.`, - }); - } const target = packageByName(allPackages, name); if (!target) { if (!compiled) { @@ -441,16 +402,6 @@ export function checkRootSites( `import it; use the package's production subpath instead.`, }); } - if (importsClientOnlyFromDaemon(site.file, site.specifier)) { - violations.push({ - rule: 'R11 package-boundaries', - file: site.file, - line: site.line, - message: - `'${site.specifier}' reads caller-side replay sources — daemon code receives the ` + - `client's source bundle and may not read caller paths; take it from the request.`, - }); - } const target = packageByName(packages, name); if (!target) { if (!compiled) { diff --git a/src/platform-runtime-device-inventory.test.ts b/src/platform-runtime-device-inventory.test.ts index 28cf1a54b6..e9badcb920 100644 --- a/src/platform-runtime-device-inventory.test.ts +++ b/src/platform-runtime-device-inventory.test.ts @@ -3,8 +3,10 @@ import { type InventoryPlatformModule, createPlatformModuleRegistry, } from '@agent-device/contracts/platform-module'; -import type { PlatformRequestScope } from '@agent-device/contracts/platform-runtime-host'; -import { testInventoryHost } from '@agent-device/device-selection/inventory-gateways-fixtures'; +import { + testInventoryHost, + testRequestScope, +} from '@agent-device/device-selection/inventory-gateways-fixtures'; import { resolveTargetDeviceSelection } from '@agent-device/device-selection/dispatch-resolve'; import { withDeviceInventoryContext } from '@agent-device/device-selection/device-inventory-context'; import { PLATFORMS, type DeviceInfo } from '@agent-device/kernel/device'; @@ -16,12 +18,6 @@ const simctl = vi.hoisted(() => ({ calls: [] as string[], })); -const scope: PlatformRequestScope = Object.freeze({ - signal: new AbortController().signal, - diagnostics: Object.freeze({ emit: () => {} }), - progress: Object.freeze({ report: () => {} }), -}); - function simulator(id: string): DeviceInfo { return { platform: 'apple', @@ -79,7 +75,7 @@ describe('production device inventory composition', () => { devicectl: { run: async () => ({ stdout: '', stderr: '', exitCode: 1 }) }, }), () => - withDeviceInventoryContext({ ...gateways, requestScope: scope }, () => + withDeviceInventoryContext({ ...gateways, requestScope: testRequestScope }, () => resolveTargetDeviceSelection( { platform: 'ios' }, {