diff --git a/docs/dependency-graph-findings.md b/docs/dependency-graph-findings.md index dd24681e1c..8b49ea36ab 100644 --- a/docs/dependency-graph-findings.md +++ b/docs/dependency-graph-findings.md @@ -74,8 +74,8 @@ type-only inversions, R7 pins SessionState field ownership, and the shared selec (`daemon/interaction/internal/interaction-common.ts`, step 3) and the recorder boundary (`session-action-recorder.ts`, step 4). The two-pass structure is two call sites, not a scattered concern. -- Still outside every rule: **dynamic** import direction (0 inversions today, nothing watching), - and anything inside a zone. +- Still outside every rule: **dynamic** import direction, out of scope by decision with the reason + recorded at R5 in `scripts/layering/check.ts` (#3280), and anything inside a zone. ## 0. Where the inversions ended up (and why 5 is the floor for now) @@ -263,6 +263,16 @@ moving `DaemonRequest` is most of what §1's second cluster needs. ## 3. `(root)`: what is left is there for a reason +> **Status after #3280:** the section below is historical. Every root module now declares its zone +> in `scripts/layering/root-module-zones.ts`, `(root)` ranks above the spine and holds only +> `bin.ts`, `cli.ts` and `daemon.ts`, and R80 asserts that the zone graph of static value imports is +> acyclic. That reduces `(root)` in the logical graph only: the modules still live directly under +> `src/`, and the declaration is a bridge whose rows go as groups move into zone folders or +> packages. The physical moves remain outstanding, starting with #3294. Ranking the composition roots did not weaken R2: it still rejects a direct `daemon/` -> +> `commands/` import, and the daemon reaches the command surface through the ranked +> `command-runtime` zone, which the zone graph now shows instead of hiding. The SDK entries rank +> above `daemon-client`, because the typed client they publish reaches the daemon through it. + 13 files: 11 published `package.json` entrypoints, the three executables (`bin.ts`, `cli.ts`, `daemon.ts`), and the composition roots — `runtime.ts`, `agent-device-client.ts`, `provider-device-runtime.ts`, `command-catalog.ts`. diff --git a/packages/contracts/src/daemon-http.test.ts b/packages/contracts/src/daemon-http.test.ts index 93273d5b66..9f3b25b214 100644 --- a/packages/contracts/src/daemon-http.test.ts +++ b/packages/contracts/src/daemon-http.test.ts @@ -6,7 +6,10 @@ import { buildDaemonHttpBaseUrl, buildDaemonHttpTenantHeaders, buildDaemonHttpUrl, + buildRemoteTempArtifactDirPath, + buildRemoteTempArtifactPath, DAEMON_RPC_PROTOCOL_VERSION, + isRemoteTempArtifactPath, } from './daemon-http.ts'; test('buildDaemonHttpBaseUrl appends the public agent-device base path', () => { @@ -64,3 +67,33 @@ test('buildDaemonHealthPayload takes the version from its caller and keeps the p }, ); }); + +test('the daemon recognizes every temp path a remote client names, however the extension is spelled', () => { + for (const built of ['png', '.png']) { + for (const checked of ['png', '.png']) { + assert.equal( + isRemoteTempArtifactPath( + buildRemoteTempArtifactPath('screenshot', built), + 'screenshot', + checked, + ), + true, + `built with '${built}', checked with '${checked}'`, + ); + } + } +}); + +test('a temp path of another prefix, a directory, or an escape is not a remote temp artifact', () => { + const screenshot = buildRemoteTempArtifactPath('screenshot', '.png'); + assert.equal(isRemoteTempArtifactPath(screenshot, 'recording', '.png'), false); + assert.equal( + isRemoteTempArtifactPath(buildRemoteTempArtifactDirPath('screenshot'), 'screenshot', '.png'), + false, + ); + assert.equal(isRemoteTempArtifactPath(`${screenshot}/../x.png`, 'screenshot', '.png'), false); + assert.equal( + isRemoteTempArtifactPath('/Users/me/agent-device-screenshot-1-a.png', 'screenshot', '.png'), + false, + ); +}); diff --git a/packages/contracts/src/daemon-http.ts b/packages/contracts/src/daemon-http.ts index 868338e76b..86772a61ad 100644 --- a/packages/contracts/src/daemon-http.ts +++ b/packages/contracts/src/daemon-http.ts @@ -1,7 +1,10 @@ +import path from 'node:path'; + // The daemon HTTP wire vocabulary shared by the daemon server, the remote // proxy, and every client that talks to them: the base path, the tenant and -// network-access header names, the URL/auth/tenant header builders, and the -// /health payload. Client and server must agree on all of it, so neither side +// network-access header names, the URL/auth/tenant header builders, the +// /health payload, and the temp artifact paths a remote client names on the +// daemon host. Client and server must agree on all of it, so neither side // owns it (ADR 0006). export const DAEMON_HTTP_BASE_PATH = '/agent-device'; export const DAEMON_HTTP_TENANT_HEADER = 'x-agent-device-tenant'; @@ -83,3 +86,43 @@ export function buildDaemonHealthPayload( ...(options.upstream !== undefined ? { upstream: options.upstream } : {}), }; } + +// A remote client names a daemon-host temp path for each artifact it downloads afterwards, and the +// daemon accepts exactly that shape where it otherwise refuses host paths. +const REMOTE_TEMP_DIR = '/tmp'; + +/** The daemon-host temp path a remote client names for an artifact it downloads afterwards. */ +export function buildRemoteTempArtifactPath(prefix: string, extension: string): string { + return path.posix.join( + REMOTE_TEMP_DIR, + `${remoteTempArtifactStem(prefix)}${dottedExtension(extension)}`, + ); +} + +/** A directory temp path — unlike `buildRemoteTempArtifactPath`, no extension is ever appended. */ +export function buildRemoteTempArtifactDirPath(prefix: string): string { + return path.posix.join(REMOTE_TEMP_DIR, remoteTempArtifactStem(prefix)); +} + +/** Whether `value` has the shape `buildRemoteTempArtifactPath(prefix, extension)` returns. */ +export function isRemoteTempArtifactPath( + value: string, + prefix: string, + extension: string, +): boolean { + const dotted = dottedExtension(extension); + const stem = path.posix.basename(value, dotted); + return ( + value === path.posix.join(REMOTE_TEMP_DIR, `${stem}${dotted}`) && + stem.startsWith(`agent-device-${prefix}-`) && + /^\d+-[a-z0-9]+$/.test(stem.slice(`agent-device-${prefix}-`.length)) + ); +} + +function dottedExtension(extension: string): string { + return extension.startsWith('.') ? extension : `.${extension}`; +} + +function remoteTempArtifactStem(prefix: string): string { + return `agent-device-${prefix}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; +} diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index dda8227948..d7da774b7d 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -3,10 +3,12 @@ // // Ranked target spine, as rank groups lowest to highest. `A ◄ B` means B may not // be outranked by A (the back-edge order the gate rejects), NOT that every displayed import exists: -// { contracts, request, selectors } ◄ core ◄ commands -// ◄ { client, daemon-server } ◄ daemon-client ◄ cli -// (authoritative ranks: `TARGET_DAG_RANK` in model.ts. The former rank-0 kernel -// zone lives in packages/kernel since #1490 W0; R11 owns its boundary.) +// { contracts, selectors, ... } ◄ { core, daemon-contracts } ◄ { commands, command-runtime } +// ◄ { client, daemon-server, platform-runtime, ... } ◄ daemon-client ◄ { sdk, ai-sdk } +// ◄ cli ◄ platform-runtime-host ◄ (root) +// (authoritative ranks: `TARGET_DAG_RANK` in model.ts; root modules declare their zone in +// root-module-zones.ts. The former rank-0 kernel zone lives in packages/kernel since #1490 W0; +// R11 owns its boundary.) // `commands/schema/` renders the rest of commands/'s facets and reads them; commands never // imports it back (#2543). It shares the commands zone (#2679 folded the standalone // cli-schema zone into it), so the ranked spine and the R2 zone-policy table cannot see the @@ -22,6 +24,8 @@ // - Over the RANKED SPINE only: rejection of every spine back-edge (R5), i.e. // an import whose source zone outranks its target zone, plus a ratchet on the // same inversion measured over TYPE-ONLY edges (R6). +// - Over the ZONE GRAPH: static value imports between zones form no cycle (R80), which R5 +// cannot see between two zones of the same rank. // - Over the DAEMON and its capture-admission adapters: SessionState field ownership (R7), because the session // record is store-owned mutable state that any daemon module can write; and the terminal // concrete-platform boundary (R65), which rejects every import form into the retired @@ -52,9 +56,9 @@ // R6, R9, and the R10 R7 counts are ratchets with no written-down reference: each is the same // measurement taken over the merge-base with origin/main (`ratchet-reference.ts`), so growth // fails, a shrink needs no edit, and no change can bank headroom. -// `(root)` holds entrypoints and composition roots. The retired `src/utils` zone is deliberately -// outside the spine and is rejected separately by R14; extracted workspace package zones are -// classified separately and held behind R11 instead of the src folder spine. +// `(root)` holds the executables and ranks above the spine. The retired `src/utils` zone is +// deliberately outside the spine and is rejected separately by R14; extracted workspace package +// zones are classified separately and held behind R11 instead of the src folder spine. import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; @@ -79,6 +83,7 @@ import { type ResolvedImportEdge, } from './model.ts'; import { checkTypeInversions } from './type-inversion-ratchet.ts'; +import { checkZoneValueDag } from './zone-value-dag.ts'; import { measureRatchets, mergeBaseRatchets, @@ -237,6 +242,15 @@ function checkRecordRuntimeOwnership(sources: ReadonlyMap): Laye * import direction no longer matters. Splitting zones into packages would not replace it: the * A4 spike found an undeclared workspace package still resolves through root node_modules and * a relative tunnel into another package's src still compiles. + * Scope: static value imports. Dynamic imports are out of scope for R5 and R80 by decision + * (#3280). An `import()` target never joins the importer's eager closure, so a lazy edge cannot + * cost what a value back-edge costs, and the code places lazy seams exactly where a module + * defers one ranked above it or one that depends back on it: `cli/process-entry.ts` loads + * `cli.ts`, the MCP tools load the typed client, the daemon's provider registry loads the + * Limrun runtime from `src/sdk/`, and the platform runtime loads its operation host + * (`loadHost`), which reads daemon session artifacts. Ranking them would reject that pattern + * rather than a mistake; what a lazy edge may load is owned by R13's laziness policy and the + * eager-closure budgets. */ function checkBackEdges(edges: readonly ResolvedImportEdge[]): LayeringViolation[] { const seen = new Set(); @@ -398,7 +412,8 @@ function report( process.stdout.write( `Layering guard: OK — ${files.length} source files satisfy R2 and contain no ` + `value-import cycles (both checked globally); the ranked target spine contains no ` + - `back-edges; the ranked spine's type-only inversions hold at or under the merge-base ` + + `back-edges and the zone graph no value-import cycle (R80); the ranked spine's ` + + `type-only inversions hold at or under the merge-base ` + `${reference.ref.slice(0, 10)} per zone pair (R6, ${inversions} remaining); ` + `${RETIRED_PATH_RULES.R14.rule} permits no tracked paths under retired src/utils; ` + `all ${sessionStateFieldCount()} SessionState fields are classified and every write is ` + @@ -473,6 +488,7 @@ export const LAYERING_RULE_IDS = [ 'substrate-domain-shape', 'selector-pipeline-ownership', 'back-edges', + 'zone-value-dag', 'type-spine-inversions', 'session-state-ownership', 'daemon-modularity-ratchets', @@ -514,6 +530,7 @@ export const LAYERING_RULES: Readonly> = { 'selector-pipeline-ownership': (context) => selectorPipelineOwnershipViolations(context.edges, workspaceSpecifierTargets(repoRoot)), 'back-edges': (context) => checkBackEdges(context.edges), + 'zone-value-dag': (context) => checkZoneValueDag(context.edges), 'type-spine-inversions': (context) => checkTypeInversions(context.edges, context.reference.typeInversions), 'session-state-ownership': (context) => checkSessionStateOwnership(context.sources), diff --git a/scripts/layering/model.test.ts b/scripts/layering/model.test.ts index 0ed0d2c372..a0e14bf2c2 100644 --- a/scripts/layering/model.test.ts +++ b/scripts/layering/model.test.ts @@ -285,12 +285,11 @@ test('ranked and unranked zones are disjoint and both non-empty', () => { test('classifyZone separates the ranked spine from intentionally-unranked zones', () => { assert.equal(classifyZone('contracts'), 'ranked'); assert.equal(classifyZone('daemon-server'), 'ranked'); - assert.equal(classifyZone('(root)'), 'unranked'); - assert.equal(classifyZone('platform-runtime'), 'unranked'); assert.equal(classifyZone('platforms'), 'unclassified'); assert.equal(classifyZone('utils'), 'unclassified'); - // Every satellite zone joined the spine; only the composition root stays out, because R2 - // forbids daemon/ from importing commands/ so the files that wire them cannot be ranked. + // Every src/ zone is on the spine, `(root)` and the zones root modules declare included. + assert.equal(classifyZone('(root)'), 'ranked'); + assert.equal(classifyZone('platform-runtime'), 'ranked'); assert.equal(classifyZone('mcp'), 'ranked'); assert.equal(classifyZone('screenshot-diff'), 'ranked'); // A zone that is neither ranked nor listed peripheral must be flagged, never @@ -302,7 +301,7 @@ test('every production zone is deliberately classified as ranked or unranked', ( // Drift guard: a new src// (or a daemon-client/server split) forces a // deliberate ranked-vs-peripheral decision here instead of silently escaping // spine back-edge detection. If this fails, add the new zone to TARGET_DAG_RANK - // (ranked spine) or UNRANKED_ZONES (root/peripheral) in model.ts. + // (ranked spine) or UNRANKED_ZONES (peripheral) in model.ts. const productionFiles = listSourceFiles(); assert.deepEqual(unclassifiedZones(productionFiles), []); diff --git a/scripts/layering/model.ts b/scripts/layering/model.ts index 3377abe76d..704c4afa7c 100644 --- a/scripts/layering/model.ts +++ b/scripts/layering/model.ts @@ -2,6 +2,7 @@ import path from 'node:path'; import { PLATFORMS } from '@agent-device/kernel/device'; import { parseSync } from 'oxc-parser'; import { destructuredDynamicImportBindings, visitAst } from './layering-ast.ts'; +import { declaredRootModuleZone } from './root-module-zones.ts'; export type ImportEdge = { spec: string; @@ -56,17 +57,24 @@ const TARGET_DAG_RANK = new Map([ ['selectors', 1], ['session-journal', 1], ['core', 2], + ['daemon-contracts', 2], + ['command-runtime', 3], ['commands', 3], ['mcp', 3], - ['ai-sdk', 4], ['client', 4], ['daemon-server', 4], ['metro', 4], + ['platform-runtime', 4], ['remote', 4], - ['sdk', 4], ['plugins', 4], ['daemon-client', 5], - ['cli', 6], + // The SDK entries publish the typed client, which reaches the daemon through daemon-client. + ['ai-sdk', 6], + ['sdk', 6], + ['cli', 7], + // Reached only through `loadHost`'s import(), so every spine zone ranks below it. + ['platform-runtime-host', 8], + ['(root)', 9], ]); export const RANKED_ZONES: ReadonlySet = new Set(TARGET_DAG_RANK.keys()); @@ -81,25 +89,19 @@ export function zoneRank(zone: string): number | null { } // Zones deliberately left OUT of the src folder spine. They are NOT unenforced: -// every file remains under the global value-cycle rule (R4). `(root)` composes -// the spine from above; extracted package zones are held by R11 package exports -// and the no-root-back-import rule instead of their former src folder rank. +// every file remains under the global value-cycle rule (R4) and the zone-level value DAG +// (R80). Extracted package zones are held by R11 package exports and the no-root-back-import +// rule instead of a src folder rank. // -// The satellite zones used to be listed here too, on the grounds that ranking them would -// invent an order the architecture had not committed to. Once `(root)` was emptied of shared -// contracts, every one of them turned out to have a consistent rank already — so the order was -// there, just unasserted. The former `utils` zone was retired into owning modules and packages. -// Extracted workspace packages are not src/ zones: R11 owns their physical seams, and their zone -// names only appear in workspace-aware graphs. The platform packages additionally carry R13's +// Every other src/ zone is ranked, `(root)` included: root modules declare their zone in +// `root-module-zones.ts`, and `(root)` ranks above the spine. Extracted workspace packages are +// not src/ zones: R11 owns their physical seams, and their zone names only appear in +// workspace-aware graphs. The platform packages additionally carry R13's // exact-family/composition/laziness policy. export const UNRANKED_ZONES: ReadonlySet = new Set([ - '(root)', // Stand-ins the bundler resolves in place of a dependency it deliberately omits. Nothing in // the production graph imports them, so ranking them would claim an edge the alias replaces. 'vendor', - // Private implementation submodules of the canonical root composition. R13 owns their exact - // importer and concrete-platform authority; giving them a spine rank would duplicate that seam. - 'platform-runtime', 'kernel', 'host-kit', 'capture-kit', @@ -294,7 +296,7 @@ export function targetDagZone(file: string): string { // #2342 relocated the daemon client to its own `src/daemon-client/` folder, so the // client zone now falls out of the folder itself; `src/daemon/` is server-only. if (file.startsWith('src/daemon/')) return 'daemon-server'; - return topFolder(file); + return declaredRootModuleZone(file) ?? topFolder(file); } // The set of zones every production file resolves into. A zone that is neither ranked diff --git a/scripts/layering/root-module-zones.test.ts b/scripts/layering/root-module-zones.test.ts new file mode 100644 index 0000000000..549a3d2c81 --- /dev/null +++ b/scripts/layering/root-module-zones.test.ts @@ -0,0 +1,109 @@ +import assert from 'node:assert/strict'; +import path from 'node:path'; +import { test } from 'node:test'; +import { listSourceFiles } from './check.ts'; +import { + collectBackEdges, + RANKED_ZONES, + resolveImportEdges, + targetDagZone, + zoneRank, +} from './model.ts'; +import { rootModuleZoneDrift } from './root-module-zones.ts'; + +test('every root module is declared in exactly one zone, and every declaration names a module', () => { + assert.deepEqual( + rootModuleZoneDrift(listSourceFiles()), + { undeclared: [], stale: [], duplicated: [] }, + 'place a new module under its zone folder; each module still directly under src/ needs ' + + 'exactly one ROOT_MODULE_ZONES row (root-module-zones.ts)', + ); +}); + +test('drift names a root module with no declared zone and a declaration with no module', () => { + const drift = rootModuleZoneDrift(['src/bin.ts', 'src/new-helper.ts', 'src/cli/new-helper.ts']); + assert.deepEqual(drift.undeclared, ['src/new-helper.ts']); + assert.ok(drift.stale.includes('src/daemon.ts')); + assert.ok(!drift.stale.includes('src/bin.ts')); +}); + +test('(root) ranks above every other zone', () => { + const root = zoneRank('(root)')!; + for (const zone of RANKED_ZONES) { + if (zone !== '(root)') assert.ok(zoneRank(zone)! < root, `${zone} must rank below (root)`); + } +}); + +test('R5 ranks a root module by its declared zone, so no lower zone reaches the daemon client through it', () => { + const edges = resolveImportEdges( + new Map([ + ['src/sdk/index.ts', "export { createAgentDeviceClient } from '../agent-device-client.ts';"], + [ + 'src/ai-sdk/index.ts', + "import { createAgentDeviceClient } from '../agent-device-client.ts';", + ], + [ + 'src/client/lease.ts', + "import { createAgentDeviceClient } from '../agent-device-client.ts';", + ], + [ + 'src/agent-device-client.ts', + "import { createRequestGuard } from './daemon-client/daemon-client-transport.ts';", + ], + ['src/daemon-client/daemon-client-transport.ts', 'export const createRequestGuard = 1;'], + ]), + ); + + // The published SDK entries rank above the typed client they publish; the client zone does not. + assert.deepEqual(collectBackEdges(edges), { + 'client -> daemon-client': ['src/client/lease.ts -> src/agent-device-client.ts'], + }); +}); + +test('every ranked zone but (root) reaches the operation host only through import()', () => { + const host = 'src/platform-runtime-operation-host.ts'; + const lazyImporter = 'src/platform-runtime.ts'; + const files = listSourceFiles().sort(); + // One real file per ranked zone, so a zone added to the spine is covered too. + const importerByZone = new Map(); + for (const zone of RANKED_ZONES) { + if (zone === '(root)' || zone === targetDagZone(host)) continue; + const importer = files.find((file) => file !== lazyImporter && targetDagZone(file) === zone); + assert.ok(importer, `no production file in ${zone} to import the host from`); + importerByZone.set(zone, importer); + } + const staticImport = (file: string) => { + const specifier = path.posix.relative(path.posix.dirname(file), host); + return `import '${specifier.startsWith('.') ? specifier : `./${specifier}`}';`; + }; + const edges = resolveImportEdges( + new Map([ + [lazyImporter, "void import('./platform-runtime-operation-host.ts');"], + [host, ''], + ...[...importerByZone.values()].map((file) => [file, staticImport(file)] as const), + ]), + ); + + assert.deepEqual( + collectBackEdges(edges), + Object.fromEntries( + [...importerByZone].map(([zone, file]) => [ + `${zone} -> platform-runtime-host`, + [`${file} -> ${host}`], + ]), + ), + ); +}); + +test('an undeclared root module is (root), so a ranked import of it is a back-edge', () => { + const edges = resolveImportEdges( + new Map([ + ['src/commands/surface.ts', "import { shared } from '../new-helper.ts';"], + ['src/new-helper.ts', 'export const shared = 1;'], + ]), + ); + + assert.deepEqual(collectBackEdges(edges), { + 'commands -> (root)': ['src/commands/surface.ts -> src/new-helper.ts'], + }); +}); diff --git a/scripts/layering/root-module-zones.ts b/scripts/layering/root-module-zones.ts new file mode 100644 index 0000000000..0bbc2aef5a --- /dev/null +++ b/scripts/layering/root-module-zones.ts @@ -0,0 +1,154 @@ +/** + * The zone of every process-root module (`src/.ts`). + * + * A folder names the zone of every file under it; a root module has no folder, so its zone is + * declared here and `targetDagZone` reads it. `(root)` ranks above every other zone, so it holds + * only the executables: any ranked module importing a `(root)` member is a spine back-edge (R5). + * Every root module is declared, so none can sit outside the ranked spine unnoticed. + * + * This reduces `(root)` in the logical graph only: the declared modules still live directly under + * `src/`, and classifying them is not collocating them. The table bridges that layout and should + * shrink. When a group moves into a folder named for its zone (`src/command-runtime/`, + * `src/daemon-contracts/`, ...) or into a package, `topFolder` derives the same zone and the + * group's rows are deleted. The end state: renaming a module within its owner needs no edit here, + * while a forbidden dependency still fails R5 or R80. + * + * Zone membership is a layering fact only. Rules keyed on physical paths — R2's `topFolder` + * zones, R13's composition files, R76's daemon-to-root inventory, R78's `src/daemon-client/**` + * scope — keep reading the path, not this declaration. + */ +export const ROOT_MODULE_ZONES: Readonly> = { + '(root)': ['src/bin.ts', 'src/cli.ts', 'src/daemon.ts'], + // Imported by core's interactor resolution, so they sit at core's rank. + core: ['src/platform-runtime-android-adb-host.ts', 'src/provider-device-runtime.ts'], + // On-disk and wire contracts a daemon process shares with its clients (#2559): the client + // reaches the daemon over the network, so both sides read these from below. + 'daemon-contracts': [ + 'src/daemon-diagnostics-scope.ts', + 'src/daemon-owner-cleanup.ts', + 'src/daemon-policy-file.ts', + 'src/daemon-process.ts', + 'src/daemon-registration-owner.ts', + 'src/daemon-registration.ts', + 'src/daemon-resolution.ts', + 'src/daemon-shutdown-report.ts', + 'src/request-progress-protocol.ts', + 'src/session-repair-tombstone.ts', + ], + // The in-process command runtime: the backend and artifact contracts it drives and the + // assemblies that bind command families onto it. The daemon executes commands through it. + 'command-runtime': [ + 'src/backend-snapshot-options.ts', + 'src/backend.ts', + 'src/io.ts', + 'src/runtime-command-surface.ts', + 'src/runtime-contract.ts', + 'src/runtime-factory.ts', + 'src/runtime.ts', + ], + // The platform composition the daemon, CLI and SDK import eagerly. It shares the zone with its + // private `src/platform-runtime/` submodule. + 'platform-runtime': [ + 'src/managed-device-reachability.ts', + 'src/platform-runtime-android-mechanics.ts', + 'src/platform-runtime-android-observation-host.ts', + 'src/platform-runtime-apple-resources.ts', + 'src/platform-runtime-apple-runner-owner.ts', + 'src/platform-runtime-daemon-lifecycle.ts', + 'src/platform-runtime-daemon-owner-cleanup.ts', + 'src/platform-runtime-device-boot.ts', + 'src/platform-runtime-device-inventory.ts', + 'src/platform-runtime-device-ready.ts', + 'src/platform-runtime-gateway.fixtures.ts', + 'src/platform-runtime-gateway.ts', + 'src/platform-runtime-host-diagnostics.ts', + 'src/platform-runtime-managed-web-backend.ts', + 'src/platform-runtime-open-target.ts', + 'src/platform-runtime-resource-cleanup.ts', + 'src/platform-runtime.ts', + 'src/provider-credential-fingerprint.ts', + 'src/provider-device-runtimes.ts', + 'src/provider-limrun-credentials.ts', + 'src/provider-webdriver.ts', + ], + // The operation host the platform runtime loads through `import()` (`loadHost`) and its facets. + // It ranks above every spine zone, so a static import of it is a back-edge from anywhere but + // `(root)`; some facets read daemon session artifacts, so it can never rank below daemon-server. + 'platform-runtime-host': [ + 'src/platform-runtime-android-application-tools.ts', + 'src/platform-runtime-android-deployment-executor.ts', + 'src/platform-runtime-android-emulator-host.ts', + 'src/platform-runtime-android-tool-host.ts', + 'src/platform-runtime-app-log-android-transport.ts', + 'src/platform-runtime-app-log-output.ts', + 'src/platform-runtime-app-log-process.ts', + 'src/platform-runtime-apple-application-tools.ts', + 'src/platform-runtime-apple-automation-keep-hot.ts', + 'src/platform-runtime-apple-deployment-executor.ts', + 'src/platform-runtime-apple-physical-readiness.ts', + 'src/platform-runtime-apple-tool-host.ts', + 'src/platform-runtime-application-resources.ts', + 'src/platform-runtime-audio-probe-host.ts', + 'src/platform-runtime-device-shutdown-host.ts', + 'src/platform-runtime-host-device-shell.ts', + 'src/platform-runtime-host.ts', + 'src/platform-runtime-local-application-interactors.ts', + 'src/platform-runtime-managed-owner.ts', + 'src/platform-runtime-network-host.ts', + 'src/platform-runtime-network-web-transport.ts', + 'src/platform-runtime-operation-host.ts', + 'src/platform-runtime-perf-capture-host.ts', + 'src/platform-runtime-perf-host.ts', + 'src/platform-runtime-runtime-hints.ts', + 'src/platform-runtime-screen-recording-android-host.ts', + 'src/platform-runtime-screen-recording-apple-host.ts', + 'src/platform-runtime-screen-recording-apple-runner-host.ts', + 'src/platform-runtime-screen-recording-apple-runner-transport.ts', + 'src/platform-runtime-screen-recording-apple-simulator-host.ts', + 'src/platform-runtime-screen-recording-apple-transport.ts', + 'src/platform-runtime-screen-recording-finalizer-host.ts', + 'src/platform-runtime-screen-recording-harmony-host.ts', + 'src/platform-runtime-screen-recording-host.ts', + 'src/platform-runtime-screen-recording-output-host.ts', + 'src/platform-runtime-screen-recording-process-host.ts', + 'src/platform-runtime-screen-recording-web-host.ts', + 'src/platform-runtime-toolchain-host.ts', + ], + // The typed client over the daemon transport. + 'daemon-client': ['src/agent-device-client.ts'], + sdk: ['src/finders.ts', 'src/provider-limrun-runtime.ts'], +}; + +const ZONE_BY_ROOT_MODULE: ReadonlyMap = new Map( + Object.entries(ROOT_MODULE_ZONES).flatMap(([zone, modules]) => + modules.map((module) => [module, zone] as const), + ), +); + +/** Whether `file` is a process-root module: a `.ts` file directly under `src/`. */ +export function isRootModule(file: string): boolean { + return /^src\/[^/]+\.ts$/.test(file); +} + +/** The declared zone of a root module, or `undefined` when the module is not declared. */ +export function declaredRootModuleZone(file: string): string | undefined { + return ZONE_BY_ROOT_MODULE.get(file); +} + +/** + * Root modules in `files` with no declared zone, declared modules that are not in `files`, and + * modules declared in more than one zone. All empty means the declaration describes the tree. + */ +export function rootModuleZoneDrift(files: readonly string[]): { + undeclared: string[]; + stale: string[]; + duplicated: string[]; +} { + const present = new Set(files.filter(isRootModule)); + const declared = Object.values(ROOT_MODULE_ZONES).flat(); + return { + undeclared: [...present].filter((file) => !ZONE_BY_ROOT_MODULE.has(file)).sort(), + stale: [...ZONE_BY_ROOT_MODULE.keys()].filter((file) => !present.has(file)).sort(), + duplicated: [...new Set(declared.filter((file, index) => declared.indexOf(file) !== index))], + }; +} diff --git a/scripts/layering/zone-value-dag.test.ts b/scripts/layering/zone-value-dag.test.ts new file mode 100644 index 0000000000..b39c876ecc --- /dev/null +++ b/scripts/layering/zone-value-dag.test.ts @@ -0,0 +1,46 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { collectBackEdges, findValueImportCycles, resolveImportEdges } from './model.ts'; +import { checkZoneValueDag, zoneValueCycles } from './zone-value-dag.ts'; + +test('two same-rank zones importing each other form a cycle no file-level or rank check sees', () => { + const edges = resolveImportEdges( + new Map([ + ['src/daemon/lease.ts', "import { isTempPath } from '../remote/temp-path.ts';"], + ['src/daemon/capture.ts', "import { isTempPath } from '../remote/temp-path.ts';"], + ['src/remote/temp-path.ts', 'export const isTempPath = true;'], + ['src/remote/diagnostics.ts', "import { logPath } from '../daemon/paths.ts';"], + ['src/daemon/paths.ts', 'export const logPath = 1;'], + ]), + ); + + assert.deepEqual(findValueImportCycles(edges), []); + assert.deepEqual(collectBackEdges(edges), {}); + assert.deepEqual(checkZoneValueDag(edges), [ + { + rule: 'R80 zone-value-dag', + file: 'src/daemon/capture.ts', + line: 1, + message: + 'zone-level value-import cycle: daemon-server -> remote -> daemon-server ' + + '(daemon-server -> remote: src/daemon/capture.ts -> src/remote/temp-path.ts; ' + + 'remote -> daemon-server: src/remote/diagnostics.ts -> src/daemon/paths.ts). ' + + 'Move the contract both zones read below both of them.', + }, + ]); +}); + +test('type-only, dynamic and same-zone imports never close a zone cycle', () => { + const edges = resolveImportEdges( + new Map([ + ['src/remote/diagnostics.ts', "import { logPath } from '../daemon/paths.ts';"], + ['src/daemon/paths.ts', "import type { TempPath } from '../remote/temp-path.ts';"], + ['src/daemon/lease.ts', "void import('../remote/temp-path.ts');"], + ['src/daemon/capture.ts', "import { logPath } from './paths.ts';"], + ['src/remote/temp-path.ts', 'export type TempPath = string;'], + ]), + ); + + assert.deepEqual(zoneValueCycles(edges), []); + assert.deepEqual(checkZoneValueDag(edges), []); +}); diff --git a/scripts/layering/zone-value-dag.ts b/scripts/layering/zone-value-dag.ts new file mode 100644 index 0000000000..057737960f --- /dev/null +++ b/scripts/layering/zone-value-dag.ts @@ -0,0 +1,57 @@ +import { findValueImportCycles, type LayeringViolation, type ResolvedImportEdge } from './model.ts'; + +export const ZONE_VALUE_DAG_RULE = 'R80 zone-value-dag'; + +/** + * Cycles in the zone graph of static value imports, each a closed zone path. Imports inside one + * zone are dropped first: they are R4's subject, and a zone does not form a cycle with itself. + */ +export function zoneValueCycles(edges: readonly ResolvedImportEdge[]): string[][] { + return findValueImportCycles( + edges + .filter((edge) => edge.fromZone !== edge.toZone) + .map((edge) => ({ ...edge, file: edge.fromZone, target: edge.toZone })), + ); +} + +/** + * Catches: two zones value-importing each other through files that never close a file-level + * cycle — invisible to R4, and to R5 when both zones share a rank, since R5 only orders zones of + * different ranks. + * Evidence: #3280 measured nine zones in one cycle, every closing edge through the then-unranked + * `(root)`, plus the same-rank `remote ⇄ daemon-server` pair. + * Cost: 103 LOC (57 rule + 46 test); one projection of the edge set onto zones through R4's + * cycle finder. + * Kill criterion: zones stop being the unit of layering, or R5 orders every zone pair strictly. + * Dynamic imports stay out for the reason recorded at R5 in check.ts. + */ +export function checkZoneValueDag(edges: readonly ResolvedImportEdge[]): LayeringViolation[] { + const witnessByPair = new Map(); + for (const edge of edges) { + if (edge.dynamic || edge.typeOnly || edge.fromZone === edge.toZone) continue; + const pair = `${edge.fromZone} -> ${edge.toZone}`; + const witness = witnessByPair.get(pair); + if (!witness || identity(edge).localeCompare(identity(witness)) < 0) { + witnessByPair.set(pair, edge); + } + } + return zoneValueCycles(edges).map((cycle) => { + const hops = cycle.slice(1).map((zone, index) => { + const pair = `${cycle[index]} -> ${zone}`; + return { pair, witness: witnessByPair.get(pair)! }; + }); + return { + rule: ZONE_VALUE_DAG_RULE, + file: hops[0]!.witness.file, + line: hops[0]!.witness.line, + message: + `zone-level value-import cycle: ${cycle.join(' -> ')} ` + + `(${hops.map(({ pair, witness }) => `${pair}: ${identity(witness)}`).join('; ')}). ` + + 'Move the contract both zones read below both of them.', + }; + }); +} + +function identity(edge: ResolvedImportEdge): string { + return `${edge.file} -> ${edge.target}`; +} diff --git a/src/daemon/macos-app-lease.ts b/src/daemon/macos-app-lease.ts index a739aed375..e973240b2a 100644 --- a/src/daemon/macos-app-lease.ts +++ b/src/daemon/macos-app-lease.ts @@ -1,5 +1,6 @@ import os from 'node:os'; import { normalizeBatchCommandName } from '@agent-device/command-registry/batch-policy'; +import { isRemoteTempArtifactPath } from '@agent-device/contracts/daemon-http'; import type { DeviceLease } from '@agent-device/contracts/device'; import { readMacOsAppBackend } from '@agent-device/contracts/session'; import { runCmd } from '@agent-device/host-kit/command'; @@ -7,7 +8,6 @@ import { isProcessAlive, readHostEnvironmentVariable } from '@agent-device/host- import { isMacOs } from '@agent-device/kernel/device'; import { AppError, type DaemonError } from '@agent-device/kernel/errors'; import { isAppLeaseAllowed } from './daemon-command-registry.ts'; -import { isRemoteTempArtifactPath } from '../remote/daemon-artifacts.ts'; import type { DaemonRequest, DaemonResponse, DaemonResponseData } from './daemon-request.ts'; import type { LeaseRegistry } from './lease-registry.ts'; import type { SessionState } from './session-state.ts'; diff --git a/src/remote/daemon-artifacts.ts b/src/remote/daemon-artifacts.ts index d7940a45ef..ad9f0f5fbe 100644 --- a/src/remote/daemon-artifacts.ts +++ b/src/remote/daemon-artifacts.ts @@ -1,5 +1,9 @@ import fs from 'node:fs'; import path from 'node:path'; +import { + buildRemoteTempArtifactDirPath, + buildRemoteTempArtifactPath, +} from '@agent-device/contracts/daemon-http'; import { AppError } from '@agent-device/kernel/errors'; import type { DaemonArtifact, DaemonRequest, DaemonResponse } from '../daemon/daemon-request.ts'; import { @@ -341,37 +345,6 @@ function hasNonEmptyString(value: unknown): value is string { return typeof value === 'string' && value.trim().length > 0; } -const REMOTE_TEMP_DIR = '/tmp'; - -/** The daemon-host temp path a remote client names for an artifact it downloads afterwards. */ -function buildRemoteTempArtifactPath(prefix: string, extension: string): string { - const safeExtension = extension.startsWith('.') ? extension : `.${extension}`; - return path.posix.join(REMOTE_TEMP_DIR, `${remoteTempArtifactStem(prefix)}${safeExtension}`); -} - -/** A directory temp path — unlike `buildRemoteTempArtifactPath`, no extension is ever appended. */ -function buildRemoteTempArtifactDirPath(prefix: string): string { - return path.posix.join(REMOTE_TEMP_DIR, remoteTempArtifactStem(prefix)); -} - -/** Whether `value` has the shape `buildRemoteTempArtifactPath(prefix, extension)` returns. */ -export function isRemoteTempArtifactPath( - value: string, - prefix: string, - extension: string, -): boolean { - const stem = path.posix.basename(value, extension); - return ( - value === path.posix.join(REMOTE_TEMP_DIR, `${stem}${extension}`) && - stem.startsWith(`agent-device-${prefix}-`) && - /^\d+-[a-z0-9]+$/.test(stem.slice(`agent-device-${prefix}-`.length)) - ); -} - -function remoteTempArtifactStem(prefix: string): string { - return `agent-device-${prefix}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; -} - export async function materializeRemoteArtifacts( info: DaemonArtifactEndpoint, req: DaemonRequest,