From 441ee44644d84852bb9337ef5bbbcfaa6a285fd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Wed, 2 Sep 2026 00:28:29 +0200 Subject: [PATCH] refactor: enforce src/utils retirement (#2149) --- .fallowrc.json | 2 +- docs/adr/0012-interactive-replay.md | 2 +- docs/adr/0018-unified-event-journal.md | 11 +- .../0019-request-bound-platform-runtime.md | 4 +- docs/agents/testing.md | 2 +- docs/dependency-graph-findings.md | 5 +- fallow-baselines/health.json | 30 +---- .../src/__tests__/adb-executor.test.ts | 4 +- .../runner-lease-recycled-pid.test.ts | 4 +- packages/selectors/src/internal/parse.ts | 5 +- scripts/depgraph/README.md | 2 +- scripts/depgraph/affected-run.ts | 2 +- scripts/depgraph/affected.test.ts | 3 + scripts/layering/check.ts | 21 ++-- scripts/layering/package-boundaries.test.ts | 3 + scripts/layering/platform-package-policy.ts | 16 ++- .../record-runtime-mechanics-policy.test.ts | 4 +- .../record-runtime-mechanics-policy.ts | 2 +- scripts/layering/retired-zone-policy.ts | 13 ++ scripts/layering/src-utils-retirement.test.ts | 114 ++++++++++++++++++ scripts/layering/src-utils-retirement.ts | 15 +++ scripts/layering/tracked-sources.ts | 33 ++--- .../vitest-slow-test-reporter.test.ts | 4 +- 23 files changed, 220 insertions(+), 81 deletions(-) create mode 100644 scripts/layering/retired-zone-policy.ts create mode 100644 scripts/layering/src-utils-retirement.test.ts create mode 100644 scripts/layering/src-utils-retirement.ts diff --git a/.fallowrc.json b/.fallowrc.json index cb747e769c..6d5212dcc7 100644 --- a/.fallowrc.json +++ b/.fallowrc.json @@ -17,7 +17,7 @@ "src/bin.ts", "src/client/companion-tunnel.ts", "src/daemon.ts", - "src/utils/png-worker.ts", + "packages/capture-kit/src/png-worker.ts", "scripts/patch-xcuitest-runner-icon.ts", "scripts/runner-request-count/run.ts", "packages/capture-kit/src/ios-snapshot-engine/replay.ts", diff --git a/docs/adr/0012-interactive-replay.md b/docs/adr/0012-interactive-replay.md index 7994a303c4..3257312b3b 100644 --- a/docs/adr/0012-interactive-replay.md +++ b/docs/adr/0012-interactive-replay.md @@ -1251,7 +1251,7 @@ both `.ad` and Maestro paths) grounds the same conclusions from the caller's sea `--json`. Structurally: replay's success payload (`{ replayed, healed, session, artifactPaths }`, `session-replay-runtime.ts:186-195`) has no `message` field, so the generic CLI success path prints nothing (`writeGenericCliOutput` → `readCommandMessage` → `writeCommandOutput`, - `src/cli/commands/generic.ts:68-71`, `src/utils/success-text.ts:12-14`, + `src/cli/commands/generic.ts:68-71`, `packages/kernel/src/success-text.ts:12-14`, `src/cli/commands/shared.ts:4-15`). An agent pays a verification turn just to learn what happened. - **Failure output today is step + action + selector + a generic hint — no screen evidence.** The live divergence hit was pure app state: the RN example app persists navigation state, so relaunch+deeplink diff --git a/docs/adr/0018-unified-event-journal.md b/docs/adr/0018-unified-event-journal.md index e57a5dc960..b50d18d58b 100644 --- a/docs/adr/0018-unified-event-journal.md +++ b/docs/adr/0018-unified-event-journal.md @@ -52,7 +52,8 @@ Normative summary of the proposal; contracts and rationale below. The codebase has grown four parallel event vocabularies, each with its own emit call, shape, redaction discipline, and sink (inventoried 2026-07-24): -1. **Diagnostics** (`src/utils/diagnostics.ts`). ~155 distinct stringly-typed `phase` values across +1. **Diagnostics** (`packages/host-kit/src/internal/diagnostics.ts`). + ~155 distinct stringly-typed `phase` values across ~70 files, an `AsyncLocalStorage` request scope entered in exactly three places (CLI pre-parse, daemon per-request in `request-router.ts`, daemon fatal catch-all), an in-memory buffer plus a `phaseCounts` tally, and debug-mode live streaming to the per-request ndjson file (after @@ -134,9 +135,9 @@ Consumers derive kind sets from traits: `RUNNER_ROUND_TRIP_PHASES` becomes deliberately do not, preserving today's cost semantics). A parity test pins the derived sets so a trait edit is a reviewed decision, not a drift. -The catalog lives in `contracts` (ranked, kernel-adjacent) so every zone — including unranked -peripherals and `utils` — may import it without a layering back-edge. The journal runtime evolves -in place in `src/utils/diagnostics.ts`, keeping all existing import directions legal. +The catalog lives in `contracts` (ranked, kernel-adjacent) so every zone may import it without a +layering back-edge. The journal runtime evolves in place in +`packages/host-kit/src/internal/diagnostics.ts`, keeping all existing import directions legal. ### 2. One journal, explicit sinks, defined scope model @@ -361,7 +362,7 @@ gets built. field is either numeric, an enum imported from the owning registry (command names, error codes, flag keys), or a hash — and that no open-string field exists; adding one is a failing gate, not a review comment. -- Layering: `scripts/layering/check.ts` stays green — catalog in `contracts`, runtime in `utils`, +- Layering: `scripts/layering/check.ts` stays green — catalog in `contracts`, runtime in `host-kit`, no new back-edges. ## Migration plan diff --git a/docs/adr/0019-request-bound-platform-runtime.md b/docs/adr/0019-request-bound-platform-runtime.md index d8b97f04fe..b3b2ef8978 100644 --- a/docs/adr/0019-request-bound-platform-runtime.md +++ b/docs/adr/0019-request-bound-platform-runtime.md @@ -635,8 +635,8 @@ The final gates passed: - `pnpm check:layering` passed 131 structural/model tests and scanned 1,157 production source files. R11 owns 17 workspace packages behind 39 exported subpaths with no root back-imports; R13 keeps six private implementation-lazy platform packages above capture-kit behind one canonical composition - root and its single private provider-composition implementation submodule; R14 - and R15 retain one typed route for `logs` and `network` with no legacy route. + root and its single private provider-composition implementation submodule; the historical R14 + and R15 rules retained one typed route for `logs` and `network` with no legacy route. - Six local inventory/runtime owners, all enumerated Apple leaf/kind cells, and the production BrowserStack, AWS Device Farm, and Limrun provider modes remain covered. Provider ownership and inventory are fail-closed; exact-owner recovery and provider-authoritative tests prove there is no diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 2d5c266759..cc6f414526 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -129,7 +129,7 @@ whether tests distinguish changed decision logic. Do not infer redundancy from l Run `pnpm depgraph affected` before touching a high-fan-in module: ```sh -pnpm depgraph affected packages/capture-kit/src/exec.ts +pnpm depgraph affected packages/host-kit/src/command.ts pnpm depgraph affected src/daemon/ref-frame.ts --json --limit 25 ``` diff --git a/docs/dependency-graph-findings.md b/docs/dependency-graph-findings.md index 59464dc436..5a0dbaa771 100644 --- a/docs/dependency-graph-findings.md +++ b/docs/dependency-graph-findings.md @@ -56,8 +56,9 @@ raw edge count reads higher. What moved: the platform-plugin contract and its four facet tags, `NetworkEntry`, the click-button / recording-export-quality / interactor-types / runner-lease-context vocabularies, -and 16 internal modules out of `(root)`; `utils` joined the spine at rank 1 after its two upward -files moved to the zones they were reaching for. Three new gate scopes keep it: R6 ratchets +and 16 internal modules out of `(root)`; the historical audit reported that `utils` joined the +spine at rank 1 after its two upward files moved to the zones they were reaching for. Three new +gate scopes keep it: R6 ratchets type-only inversions, R7 pins SessionState field ownership, and the shared selector checks in `selectors/` are covered by their own tests. diff --git a/fallow-baselines/health.json b/fallow-baselines/health.json index 8604759aee..fabd6febc4 100644 --- a/fallow-baselines/health.json +++ b/fallow-baselines/health.json @@ -466,11 +466,6 @@ "count": 1 } }, - "src/utils/exec.ts": { - "complexity_moderate": { - "count": 1 - } - }, "src/commands/output/snapshot.ts": { "crap_moderate": { "count": 1 @@ -481,24 +476,11 @@ "count": 1 } }, - "src/utils/retry.ts": { - "complexity_high": { - "count": 1 - } - }, "src/snapshot/scroll-edge-state.ts": { "crap_moderate": { "count": 1 } }, - "src/utils/source-value.ts": { - "complexity_high": { - "count": 1 - }, - "crap_moderate": { - "count": 1 - } - }, "src/snapshot/snapshot-presentation/text-surface.ts": { "crap_high": { "count": 1 @@ -570,14 +552,13 @@ "src/daemon/handlers/session-state.ts:complexity", "src/commands/cli-grammar/common.ts:high impact", "src/snapshot/snapshot-presentation/tree.ts:high impact", - "src/utils/success-text.ts:high impact", + "packages/kernel/src/success-text.ts:high impact", "src/cli.ts:complexity", - "src/utils/timeouts.ts:high impact", "src/commands/interaction/output.ts:high impact", "src/snapshot/snapshot-lines.ts:high impact", - "src/utils/rect-center.ts:high impact", + "packages/kernel/src/rect-center.ts:high impact", "packages/platform-apple/src/core/app-launch.ts:complexity", - "src/utils/parsing.ts:high impact", + "packages/kernel/src/record.ts:high impact", "src/daemon/daemon-command-registry.ts:high impact", "src/replay/script.ts:complexity", "src/daemon/handlers/session-doctor-output.ts:high impact", @@ -593,14 +574,13 @@ "packages/platform-linux/src/snapshot.ts:high impact", "src/core/interaction-targeting.ts:high impact", "packages/maestro/src/internal/runtime-targets.ts:high impact", - "src/utils/source-value.ts:high impact", + "packages/kernel/src/source-value.ts:high impact", "src/request/cancel.ts:high impact", "src/commands/interaction/selectors.ts:untested risk", "packages/selectors/src/internal/build.ts:high impact", "src/kernel/redaction.ts:high impact", - "src/utils/rect-visibility.ts:high impact", "packages/provider-webdriver/src/webdriver-utils.ts:high impact", - "src/utils/keyed-lock.ts:high impact", + "packages/kernel/src/keyed-lock.ts:high impact", "src/daemon/request-progress-protocol.ts:high impact", "src/cli/replay-test/reporters/format.ts:high impact", "src/daemon/replay/internal/session-test-infrastructure.ts:high impact", diff --git a/packages/platform-android/src/__tests__/adb-executor.test.ts b/packages/platform-android/src/__tests__/adb-executor.test.ts index d89d9a2837..a914a39c0e 100644 --- a/packages/platform-android/src/__tests__/adb-executor.test.ts +++ b/packages/platform-android/src/__tests__/adb-executor.test.ts @@ -485,8 +485,8 @@ test('the local adb executor flags transient transport failures retriable', asyn test('the local adb executor classifies exec-layer timeouts as a wedged adb server', async () => { mockRunCmd.mockClear(); - // Shape of createTimeoutError in utils/exec.ts: no stderr to classify, the - // structured `timeoutMs` detail is the signal. + // Shape of `createTimeoutError` in `packages/host-kit/src/internal/exec.ts`: no + // stderr to classify, the structured `timeoutMs` detail is the signal. mockRunCmd.mockRejectedValueOnce( new AppError('COMMAND_FAILED', 'adb timed out after 10000ms', { cmd: 'adb', diff --git a/packages/platform-apple/src/runner/__tests__/runner-lease-recycled-pid.test.ts b/packages/platform-apple/src/runner/__tests__/runner-lease-recycled-pid.test.ts index 46248662e7..fc8998433a 100644 --- a/packages/platform-apple/src/runner/__tests__/runner-lease-recycled-pid.test.ts +++ b/packages/platform-apple/src/runner/__tests__/runner-lease-recycled-pid.test.ts @@ -41,8 +41,8 @@ import { * pinning the read is the point rather than a workaround. */ -// Process execution goes through utils/exec.ts, never node:child_process -// directly (AGENTS.md hard rule) — including in tests. +// Process execution goes through @agent-device/host-kit/command, never node:child_process +// directly — including in tests. let child: ExecBackgroundResult | undefined; let previousLeaseDir: string | undefined; diff --git a/packages/selectors/src/internal/parse.ts b/packages/selectors/src/internal/parse.ts index 157a259f5a..c4d95717a2 100644 --- a/packages/selectors/src/internal/parse.ts +++ b/packages/selectors/src/internal/parse.ts @@ -58,9 +58,8 @@ export const SELECTOR_KEY_NAMES: readonly SelectorKey[] = [...ALL_KEYS]; // `button` in `button="Push Article"`). Superset of the ROLE_LABELS vocabulary in // src/snapshot/snapshot-lines.ts (plus a few common role words like list/tab/alert/dialog/header, // minus valid selector keys such as `text`, which ALL_KEYS short-circuits before this set is -// consulted), kept as a local copy to avoid a utils/ -> snapshot/ layering dependency -// (scripts/layering/check.ts). Drifts silently if ROLE_LABELS grows; that's fine here since this -// is only used to sharpen a hint, not to validate anything. +// consulted), kept locally to avoid a selectors -> snapshot layering dependency. Drifts silently +// if ROLE_LABELS grows; that's fine here since this only sharpens a hint, not validates anything. const ROLE_HINT_WORDS = new Set([ 'button', 'imagebutton', diff --git a/scripts/depgraph/README.md b/scripts/depgraph/README.md index 6c1e46d3b7..4859a4c24a 100644 --- a/scripts/depgraph/README.md +++ b/scripts/depgraph/README.md @@ -13,7 +13,7 @@ productive artifact is the JSON, queried directly. ## Blast radius of one file ```sh -pnpm depgraph affected src/utils/exec.ts # bounded text +pnpm depgraph affected packages/host-kit/src/command.ts # bounded text pnpm depgraph affected src/daemon/ref-frame.ts --json --limit 25 ``` diff --git a/scripts/depgraph/affected-run.ts b/scripts/depgraph/affected-run.ts index 1ba7a7bbf6..25346fd657 100644 --- a/scripts/depgraph/affected-run.ts +++ b/scripts/depgraph/affected-run.ts @@ -1,6 +1,6 @@ // `pnpm depgraph affected ` — the blast radius of one file in one query. // -// pnpm depgraph affected src/utils/exec.ts +// pnpm depgraph affected packages/host-kit/src/command.ts // pnpm depgraph affected src/daemon/ref-frame.ts --json --limit 20 // // Answers, from the sources of truth rather than a second copy of them: diff --git a/scripts/depgraph/affected.test.ts b/scripts/depgraph/affected.test.ts index 9b751b2627..65203cb3f5 100644 --- a/scripts/depgraph/affected.test.ts +++ b/scripts/depgraph/affected.test.ts @@ -86,6 +86,7 @@ test('zoneBreakdown and rankByFanIn order by weight, then name', () => { }); test('commandsReaching follows the dynamic import a route uses to load its handler', () => { + // These are in-memory arbitrary paths for graph traversal, not repository ownership metadata. const edges = edgesOf({ 'src/daemon/handlers/session.ts': "import { helper } from '../../utils/helper.ts';\nexport const h = helper;", @@ -169,6 +170,7 @@ test('the real guarantee matrix contributes module-qualified rows', () => { ); }); +// These parser inputs are arbitrary paths, not references to repository ownership metadata. test('a flag keeps its value whichever side of the path it is on', () => { const expected = { file: 'src/utils/exec.ts', json: true, limit: 25 }; assert.deepEqual(parseInvocation(['src/utils/exec.ts', '--json', '--limit', '25']), expected); @@ -190,6 +192,7 @@ test('parseInvocation rejects a missing path, a second path, and a non-positive }); test('text output bounds every list and says what it hid', () => { + // This is an arbitrary report input used to exercise formatting, not a live source path. const text = formatBlastRadius( { file: 'src/utils/exec.ts', diff --git a/scripts/layering/check.ts b/scripts/layering/check.ts index 56e7eb8ad0..95ba74cdd4 100644 --- a/scripts/layering/check.ts +++ b/scripts/layering/check.ts @@ -13,8 +13,8 @@ // rejection of all production static value-import cycles (R4). R1 kernel-sink // retired with the kernel's move to packages/kernel (#1490 W0); R8 // zero-dep-job-closure retired with the last `install-deps: false` job -// (#1781 A6) — its invariant has no subjects, and both numbers are spent, so -// a new rule takes the next free id rather than reusing them. +// (#1781 A6) — its invariant has no subjects. R14 is reserved for the terminal +// src/utils retirement rule (#2149); other new rules take the next free id. // - 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). @@ -40,9 +40,9 @@ // code cannot manufacture or repair a narrowed runtime proof (R66). // - Over CONTRACTS PRODUCTION SOURCE: contracts owns vocabulary only — host, process, and timer // mechanics belong in capture-kit or an adapter (R18). -// Only `(root)` is unranked among src/ zones (see `UNRANKED_ZONES` in model.ts): -// it holds entrypoints and composition roots. Extracted workspace package zones -// are classified separately and held behind R11 instead of the src folder spine. +// `(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. import { execFileSync } from 'node:child_process'; import fs from 'node:fs'; @@ -104,6 +104,7 @@ import { checkDaemonPlatformBoundary } from './daemon-platform-boundary.ts'; import { listTrackedPlatformZoneFiles, listTrackedProductionSources, + listTrackedSrcUtilsFiles, listTrackedTypeScriptFiles, } from './tracked-sources.ts'; import { runtimeExecutionIntegrityViolations } from './runtime-execution-policy.ts'; @@ -112,6 +113,7 @@ import { sessionResourceOwnershipViolations } from './session-resource-ownership import { replayOwnershipViolations } from './replay-ownership.ts'; import { applicationLifecycleOwnershipViolations } from './application-lifecycle-policy.ts'; import { iosSnapshotEngineOwnershipViolations } from './ios-snapshot-engine-policy.ts'; +import { SRC_UTILS_RETIREMENT_RULE, srcUtilsRetirementViolations } from './src-utils-retirement.ts'; const repoRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', @@ -482,8 +484,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 (only the composition root is unranked among src zones), and its type-only ` + - `inversions match the R6 ratchet (${Object.values(TYPE_INVERSION_BASELINE).reduce((sum, count) => sum + count, 0)} remaining); ` + + `back-edges; the ranked spine's type-only inversions match the R6 ratchet (${Object.values(TYPE_INVERSION_BASELINE).reduce((sum, count) => sum + count, 0)} remaining); ` + + `${SRC_UTILS_RETIREMENT_RULE} permits no tracked paths under retired src/utils; ` + `all ${sessionStateFieldCount()} SessionState fields are classified and every write is ` + `inside its declared owner (R7); the largest type-level cycle is ${typeCycle} files ` + `(R9); ${daemonModularitySummary()}; ` + @@ -524,6 +526,7 @@ export type LayeringContext = Readonly<{ sourceFiles: readonly string[]; sources: ReadonlyMap; allTypeScriptSources: ReadonlyMap; + trackedSrcUtilsFiles: readonly string[]; edges: readonly ResolvedImportEdge[]; typeCycleMembers: readonly string[]; }>; @@ -558,6 +561,7 @@ export const LAYERING_RULE_IDS = [ 'package-boundaries', 'platform-package-policy', 'retired-platforms-zone', + 'src-utils-retirement', 'replay-ownership', 'ios-snapshot-engine-ownership', ] as const; @@ -601,6 +605,7 @@ export const LAYERING_RULES: Readonly> = { { untrackedProductionFiles: listUntrackedProductionTypeScriptFiles(repoRoot) }, ), 'retired-platforms-zone': () => checkRetiredPlatformsZone(listTrackedPlatformZoneFiles(repoRoot)), + 'src-utils-retirement': (context) => srcUtilsRetirementViolations(context.trackedSrcUtilsFiles), 'replay-ownership': (context) => replayOwnershipViolations(context.sourceFiles), 'ios-snapshot-engine-ownership': (context) => iosSnapshotEngineOwnershipViolations( @@ -612,6 +617,7 @@ export function main(): number { const sourceFiles = listSourceFiles(); const sources = readSources(sourceFiles); const allTypeScriptSources = readSources(listTypeScriptFiles()); + const trackedSrcUtilsFiles = listTrackedSrcUtilsFiles(repoRoot); const edges = resolveImportEdges(sources, workspaceSpecifierTargets(repoRoot)); // Computed once and threaded: the rule and the success line must report the same number. const typeCycleMembers = largestTypeCycleMembers(edges); @@ -620,6 +626,7 @@ export function main(): number { sourceFiles, sources, allTypeScriptSources, + trackedSrcUtilsFiles, edges, typeCycleMembers, }; diff --git a/scripts/layering/package-boundaries.test.ts b/scripts/layering/package-boundaries.test.ts index 167cbe4da1..dae11b6de5 100644 --- a/scripts/layering/package-boundaries.test.ts +++ b/scripts/layering/package-boundaries.test.ts @@ -308,6 +308,7 @@ function reExportSources(source: string): string[] { return [...found]; } +// These `src/utils` paths are in-memory arbitrary parser fixtures, not live repository paths. test('double-quoted and re-export routes into packages are not invisible to R11', () => { // The scanner is the layering parser, so quote style and statement form // cannot carve out a bypass: a double-quoted import, a re-export, and a @@ -337,6 +338,7 @@ test('double-quoted and re-export routes into packages are not invisible to R11' assert.equal(checkPackageInternalSites(kernel, packageEscape, ALL).length, 1); }); +// This `src/utils` path is another in-memory arbitrary parser fixture, not a live repository path. test('a package file importing root src is a violation', () => { const sites = specifierSites( 'packages/kernel/src/errors.ts', @@ -390,6 +392,7 @@ test('a cross-package import needs a workspace:* declaration and an exported sub }); test('a root src file tunnelling into packages/*/src relatively is a violation', () => { + // This `src/utils` path is an in-memory arbitrary parser fixture, not a live repository path. const sites = specifierSites( 'src/utils/exec.ts', "import { AppError } from '../../packages/kernel/src/errors.ts';", diff --git a/scripts/layering/platform-package-policy.ts b/scripts/layering/platform-package-policy.ts index 6eae5358b7..3d447d84fb 100644 --- a/scripts/layering/platform-package-policy.ts +++ b/scripts/layering/platform-package-policy.ts @@ -3,6 +3,7 @@ import { PLATFORMS } from '@agent-device/kernel/device'; import { parseImports, type LayeringViolation } from './model.ts'; import { checkPlatformComposition } from './platform-composition-policy.ts'; import { checkPlatformPackageSourcePolicy } from './platform-package-source-policy.ts'; +import { retiredPathViolations } from './retired-zone-policy.ts'; export const CANONICAL_PLATFORM_FAMILIES = PLATFORMS; type PlatformFamily = (typeof CANONICAL_PLATFORM_FAMILIES)[number]; @@ -43,15 +44,12 @@ const PLATFORM_RUNTIME_HOST_FILES = new Set([ export const APPLE_RUNNER_SUBTREE = 'packages/platform-apple/src/runner/'; export function checkRetiredPlatformsZone(files: readonly string[]): LayeringViolation[] { - return files - .filter((file) => file.startsWith('src/platforms/')) - .map((file) => ({ - rule: 'retired-platforms-zone', - file, - line: 1, - message: - 'src/platforms is retired; family code belongs in its platform package, shared mechanics in an owning substrate package, and cross-family tests in their root or package test owner', - })); + return retiredPathViolations( + files, + 'src/platforms', + 'retired-platforms-zone', + 'src/platforms is retired; family code belongs in its platform package, shared mechanics in an owning substrate package, and cross-family tests in their root or package test owner', + ); } const APPLE_RUNNER_FACADE = '@agent-device/platform-apple/runner'; const APPLE_RUNNER_TEST_HOST = '@agent-device/platform-apple/runner/test-host'; diff --git a/scripts/layering/record-runtime-mechanics-policy.test.ts b/scripts/layering/record-runtime-mechanics-policy.test.ts index ef1336844f..0e4bc2cd9a 100644 --- a/scripts/layering/record-runtime-mechanics-policy.test.ts +++ b/scripts/layering/record-runtime-mechanics-policy.test.ts @@ -8,7 +8,7 @@ test('R16 rejects native mechanics in the daemon record owner without scanning p path: 'src/daemon/handlers/record-runtime.ts', source: ` import fs from 'node:fs'; - import { runCmdBackground } from '../../utils/exec.ts'; + import { runCmdBackground } from '@agent-device/host-kit/command'; // setTimeout and runCmd are mechanics only when executable. const prose = 'spawn xcrun'; setTimeout(() => runCmdBackground('xcrun', []), 1); @@ -21,7 +21,7 @@ test('R16 rejects native mechanics in the daemon record owner without scanning p ]); assert.deepEqual(violations, [ 'src/daemon/handlers/record-runtime.ts: daemon record owner imports native mechanic node:fs', - 'src/daemon/handlers/record-runtime.ts: daemon record owner imports native mechanic ../../utils/exec.ts', + 'src/daemon/handlers/record-runtime.ts: daemon record owner imports native mechanic @agent-device/host-kit/command', 'src/daemon/handlers/record-runtime.ts: daemon record owner calls native mechanic setTimeout', 'src/daemon/handlers/record-runtime.ts: daemon record owner calls native mechanic runCmdBackground', ]); diff --git a/scripts/layering/record-runtime-mechanics-policy.ts b/scripts/layering/record-runtime-mechanics-policy.ts index 87520be5fe..5fc516cc4e 100644 --- a/scripts/layering/record-runtime-mechanics-policy.ts +++ b/scripts/layering/record-runtime-mechanics-policy.ts @@ -65,7 +65,7 @@ function isDaemonRecordMechanicImport(imported: string): boolean { imported === 'node:fs' || imported.includes('/platforms/') || imported.includes('/provider-') || - imported.includes('/utils/exec') || + imported === '@agent-device/host-kit/command' || imported.includes('/recording/overlay') ); } diff --git a/scripts/layering/retired-zone-policy.ts b/scripts/layering/retired-zone-policy.ts new file mode 100644 index 0000000000..616564eade --- /dev/null +++ b/scripts/layering/retired-zone-policy.ts @@ -0,0 +1,13 @@ +import type { LayeringViolation } from './model.ts'; + +export function retiredPathViolations( + files: readonly string[], + root: string, + rule: string, + message: string, +): LayeringViolation[] { + const prefix = `${root}/`; + return files + .filter((file) => file === root || file.startsWith(prefix)) + .map((file) => ({ rule, file, line: 1, message })); +} diff --git a/scripts/layering/src-utils-retirement.test.ts b/scripts/layering/src-utils-retirement.test.ts new file mode 100644 index 0000000000..18c0953751 --- /dev/null +++ b/scripts/layering/src-utils-retirement.test.ts @@ -0,0 +1,114 @@ +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { test } from 'node:test'; +import { LAYERING_RULES, type LayeringContext } from './check.ts'; +import { + listTrackedPackageManifests, + listTrackedSrcUtilsFiles, + listTrackedTypeScriptFiles, +} from './tracked-sources.ts'; +import { SRC_UTILS_RETIREMENT_RULE, srcUtilsRetirementViolations } from './src-utils-retirement.ts'; + +const repoRoot = path.resolve(import.meta.dirname, '../..'); + +function contextWithTrackedSrcUtilsFiles(trackedSrcUtilsFiles: readonly string[]): LayeringContext { + return { + sourceFiles: [], + sources: new Map(), + allTypeScriptSources: new Map(), + trackedSrcUtilsFiles, + edges: [], + typeCycleMembers: [], + }; +} + +test('R14 rejects every tracked path under the retired src/utils zone', () => { + const trackedFiles = ['src/utils', 'src/utils/regrown.ts', 'src/utils/__fixtures__/capture.json']; + const violations = srcUtilsRetirementViolations(trackedFiles); + + assert.equal(SRC_UTILS_RETIREMENT_RULE, 'R14 src-utils-retirement'); + assert.deepEqual( + violations.map(({ rule, file }) => ({ rule, file })), + trackedFiles.map((file) => ({ rule: SRC_UTILS_RETIREMENT_RULE, file })), + ); + assert.deepEqual( + LAYERING_RULES['src-utils-retirement'](contextWithTrackedSrcUtilsFiles(trackedFiles)), + violations, + ); + for (const violation of violations) { + assert.match(violation.message, /src\/utils is retired; move this path/); + } +}); + +test('R14 ignores paths outside the retired zone', () => { + assert.deepEqual( + srcUtilsRetirementViolations([ + 'src/snapshot/scroll-edge-state.ts', + 'src/utils-next.ts', + 'test/fixtures/src/utils/example.json', + ]), + [], + ); +}); + +test('tracked-path discovery includes top-level and nested retired paths', () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'src-utils-retirement-')); + fs.mkdirSync(path.join(repo, 'src/utils/nested'), { recursive: true }); + fs.mkdirSync(path.join(repo, 'packages/example/src'), { recursive: true }); + fs.mkdirSync(path.join(repo, 'src/snapshot'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'src/café.ts'), 'export {}\n'); + fs.writeFileSync(path.join(repo, 'src/utils/café.ts'), 'export {}\n'); + fs.writeFileSync(path.join(repo, 'src/utils/regrown.ts'), 'export {}\n'); + fs.writeFileSync(path.join(repo, 'src/utils/nested/capture.json'), '{}\n'); + fs.writeFileSync(path.join(repo, 'src/snapshot/current.ts'), 'export {}\n'); + fs.writeFileSync(path.join(repo, 'packages/example/package.json'), '{}\n'); + fs.writeFileSync(path.join(repo, 'packages/example/src/café.ts'), 'export {}\n'); + execFileSync('git', ['init', '-q'], { cwd: repo }); + execFileSync('git', ['add', '.'], { cwd: repo }); + + assert.deepEqual(listTrackedSrcUtilsFiles(repo), [ + 'src/utils/café.ts', + 'src/utils/nested/capture.json', + 'src/utils/regrown.ts', + ]); + assert.deepEqual(listTrackedTypeScriptFiles(repo), [ + 'packages/example/src/café.ts', + 'src/café.ts', + 'src/snapshot/current.ts', + 'src/utils/café.ts', + 'src/utils/regrown.ts', + ]); + assert.deepEqual(listTrackedPackageManifests(repo), ['packages/example/package.json']); + assert.deepEqual( + LAYERING_RULES['src-utils-retirement']( + contextWithTrackedSrcUtilsFiles(listTrackedSrcUtilsFiles(repo)), + ).map(({ rule, file }) => ({ rule, file })), + [ + { rule: SRC_UTILS_RETIREMENT_RULE, file: 'src/utils/café.ts' }, + { rule: SRC_UTILS_RETIREMENT_RULE, file: 'src/utils/nested/capture.json' }, + { rule: SRC_UTILS_RETIREMENT_RULE, file: 'src/utils/regrown.ts' }, + ], + ); +}); + +test('tracked-path discovery includes an exact retired root entry', () => { + const repo = fs.mkdtempSync(path.join(os.tmpdir(), 'src-utils-retirement-root-')); + fs.mkdirSync(path.join(repo, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo, 'src/utils'), 'retired root\n'); + execFileSync('git', ['init', '-q'], { cwd: repo }); + execFileSync('git', ['add', '.'], { cwd: repo }); + + assert.deepEqual(listTrackedSrcUtilsFiles(repo), ['src/utils']); +}); + +test('the committed tree has no tracked src/utils path', () => { + assert.deepEqual( + LAYERING_RULES['src-utils-retirement']( + contextWithTrackedSrcUtilsFiles(listTrackedSrcUtilsFiles(repoRoot)), + ), + [], + ); +}); diff --git a/scripts/layering/src-utils-retirement.ts b/scripts/layering/src-utils-retirement.ts new file mode 100644 index 0000000000..bf684db50e --- /dev/null +++ b/scripts/layering/src-utils-retirement.ts @@ -0,0 +1,15 @@ +import type { LayeringViolation } from './model.ts'; +import { retiredPathViolations } from './retired-zone-policy.ts'; + +export const SRC_UTILS_RETIREMENT_RULE = 'R14 src-utils-retirement'; + +const RETIRED_SRC_UTILS_ROOT = 'src/utils'; + +export function srcUtilsRetirementViolations(trackedFiles: readonly string[]): LayeringViolation[] { + return retiredPathViolations( + trackedFiles, + RETIRED_SRC_UTILS_ROOT, + SRC_UTILS_RETIREMENT_RULE, + 'src/utils is retired; move this path to its owning package or command zone', + ); +} diff --git a/scripts/layering/tracked-sources.ts b/scripts/layering/tracked-sources.ts index 95fe47eaa1..eefebe21d7 100644 --- a/scripts/layering/tracked-sources.ts +++ b/scripts/layering/tracked-sources.ts @@ -1,6 +1,6 @@ // Layering scans read tracked repository paths only. Most rules consume production TypeScript; -// the retired-platform zone also consumes every tracked path under its former root so non-TS -// fixtures cannot bypass the ownership boundary. +// the retired zones also consume every tracked path under their former roots so non-TS fixtures +// cannot bypass the ownership boundary. // // A leaf module on purpose. `check.ts` owns the scan and imports `package-boundaries.ts`, so the // boundary rules cannot import `check.ts` back for its file list; without a shared leaf the two @@ -39,29 +39,34 @@ const TRACKED_SOURCE_PATHSPECS = [ * enumeration widened to every workspace package. */ export function listTrackedPackageManifests(repoRoot: string): string[] { - const out = execFileSync('git', ['ls-files', 'packages/*/package.json'], { - cwd: repoRoot, - encoding: 'utf8', - }); - return out.split('\n').filter(Boolean); + return listTrackedFiles(repoRoot, ['packages/*/package.json']); } /** Every tracked `.ts` source file under the scanned roots, repo-root-relative. */ export function listTrackedTypeScriptFiles(repoRoot: string): string[] { - const out = execFileSync('git', ['ls-files', ...TRACKED_SOURCE_PATHSPECS], { - cwd: repoRoot, - encoding: 'utf8', - }); - return out.split('\n').filter(Boolean); + return listTrackedFiles(repoRoot, TRACKED_SOURCE_PATHSPECS); } /** Every tracked file under the retired platform root, regardless of extension. */ export function listTrackedPlatformZoneFiles(repoRoot: string): string[] { - const out = execFileSync('git', ['ls-files', '--', 'src/platforms'], { + return listTrackedFilesUnderRoot(repoRoot, 'src/platforms'); +} + +/** Every tracked path at or under the retired `src/utils` root, regardless of extension. */ +export function listTrackedSrcUtilsFiles(repoRoot: string): string[] { + return listTrackedFilesUnderRoot(repoRoot, 'src/utils'); +} + +function listTrackedFilesUnderRoot(repoRoot: string, root: string): string[] { + return listTrackedFiles(repoRoot, [root]); +} + +function listTrackedFiles(repoRoot: string, pathspecs: readonly string[]): string[] { + const out = execFileSync('git', ['ls-files', '-z', '--', ...pathspecs], { cwd: repoRoot, encoding: 'utf8', }); - return out.split('\n').filter(Boolean); + return out.split('\0').filter(Boolean); } /** Production sources only: test files and `__tests__/` trees are not layering subjects. */ diff --git a/src/__tests__/vitest-slow-test-reporter.test.ts b/src/__tests__/vitest-slow-test-reporter.test.ts index 03be365da6..a6f331ded9 100644 --- a/src/__tests__/vitest-slow-test-reporter.test.ts +++ b/src/__tests__/vitest-slow-test-reporter.test.ts @@ -4,7 +4,7 @@ import { classifySlowTest, reportSlowTests } from '../../scripts/vitest-slow-tes const base = { root: '/repo', - moduleId: '/repo/src/utils/__tests__/example.test.ts', + moduleId: '/repo/src/__tests__/example.test.ts', name: 'does a thing', fullName: 'group > does a thing', }; @@ -20,7 +20,7 @@ test('over-budget unit tests enter the warn band; 2x budget enforces', () => { const fail = classifySlowTest({ ...base, durationMs: 5_100 }); assert.ok(fail); assert.equal(fail.enforce, true); - assert.equal(fail.key, 'src/utils/__tests__/example.test.ts :: group does a thing'); + assert.equal(fail.key, 'src/__tests__/example.test.ts :: group does a thing'); }); test('integration paths get the larger budget', () => {