refactor: break import cycles; module-boundary law + design doc (isolation 1/7)

.oxlintrc.json

@@ -8,7 +8,8 @@
     "no-unused-vars": "warn",
     "unicorn/no-useless-spread": "off",
     "typescript/unbound-method": "off",
-    "typescript/await-thenable": "off"
+    "typescript/await-thenable": "off",
+    "import/no-cycle": "error"
   },
   "ignorePatterns": ["node_modules", "dist"]
 }

docs/design/module-isolation-2026-06.md

@@ -0,0 +1,139 @@
+# Module isolation — design
+
+> **Status:** accepted, in progress (June 2026)
+> **Feeds:** Phase 4 of `architecture-overhaul-2026-06.md`
+> **Principle source:** PR #108 (modules-first), adopted in place — no rebuild.
+
+## What we're solving
+
+`src/` already has directory-shaped concerns (`workspace/`, `graph/`,
+`cache/`, `exec/`, `orchestrator/`, `cli/`, `util/`), but nothing
+enforces them: 30+ cross-directory imports target internal files
+directly, two import cycles exist (one through `index.ts`, one inside
+orchestrator), three orchestrator files are actually workspace
+concerns, and one file (`execute-task.ts`) mixes execution glue with
+cache-key derivation that two other files reach in for. This doc is
+(1) the audit, (2) the target module contracts, (3) the mechanical PR
+series.
+
+## 1. Audit findings (2026-06-10, tip of main)
+
+### Cycles
+
+1. **`index.ts` ↔ `orchestrator.ts`** (and `cli.ts → index.ts`):
+   `orchestrator.ts` / `cli.ts` import `VERSION` from `./index.js`
+   while `index.ts` imports `run` from `./orchestrator.js`. Works only
+   because `VERSION` is a hoisted const under ESM live bindings.
+   Fix: extract `src/version.ts`.
+2. **`orchestrator.ts` ↔ `orchestrator/prepare.ts`** (file-level,
+   type-only): `prepare.ts` imports `RunOptions` upward from the
+   module entry. Fix: move `RunOptions`/`RunSummary` to
+   `orchestrator/options.ts`; entry re-exports.
+
+No other cycles at file or directory level.
+
+### Deep imports past would-be boundaries
+
+- `cli/run.ts → orchestrator/plan-format.ts` — plan rendering is CLI
+  presentation living in the wrong module; move to `cli/`.
+- `cli/cache.ts → cache/cache.ts` — legitimate composition, must flow
+  through the cache contract once it exists.
+- `plan.ts`/`prepare.ts → execute-task.ts` for the hashing surface
+  only — evidence `execute-task.ts` hosts two concerns; split
+  `task-hash.ts`.
+- All other cross-directory edges are directionally fine and need
+  only re-pointing at module index files.
+
+### Type homes
+
+| Type                                    | Verdict                                                                                                                                                                                              |
+| --------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `TaskNode`, `TaskOutcome`, `TaskStatus` | Stay in `graph` — they are the graph/scheduler's product. The cache-flavored statuses are opaque to the scheduler (documented; a generic `TaskOutcome<S>` is ceremony for one consumer).             |
+| `ProjectEntry`                          | Move to `workspace` — it's "discovered project + loaded config", the joint output of discovery + loading. Today's home in graph forces `nested-dirs` (pure workspace geometry) to import from graph. |
+| `CacheLayer` + friends                  | Stay in `cache/cache.ts`, exported via `cache/index.ts`.                                                                                                                                             |
+| `RunOptions`/`RunSummary`               | Move declarations to `orchestrator/options.ts` (kills cycle 2).                                                                                                                                      |
+| `VERSION`                               | Move to `src/version.ts` (kills cycle 1); `index.ts` re-export keeps the public name.                                                                                                                |
+
+### Mixed-concern files
+
+- `orchestrator/execute-task.ts` → split the hashing surface
+  (`computeTaskHash`, `computeGroupHash`, `hashTaskConfig`,
+  `hashProjectPackageJson`, `createHashCache`) into
+  `orchestrator/task-hash.ts`. NOT into `cache/` — key-part selection
+  composes graph types; pushing it down would force `cache → graph`.
+- `orchestrator/nested-dirs.ts`, `orchestrator/fingerprint.ts` →
+  move to `workspace/` (workspace geometry / workspace-file hashing).
+- `orchestrator/plan-format.ts` → move to `cli/` (pure presentation;
+  orchestrator keeps the `RunPlan` data types).
+- `orchestrator/upstream.ts` stays — same `cache → graph` argument as
+  task-hash.
+
+## 2. Target design
+
+Eight modules. A module is a directory with an `index.ts` contract,
+or a single root file when it has no internals to hide.
+
+| Module         | Form                                               | Contract highlights                                                                                                                                             |
+| -------------- | -------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `util`         | dir + index                                        | `UserError`, `xxh3*`, `relPosix`/`toPosix`, `ulid`                                                                                                              |
+| `config`       | single file `src/config.ts`                        | schema types + `defineProject`/`defineWorkspace`. Stays at ROOT — cache/exec/graph/workspace all consume it; nesting under workspace inverts direction          |
+| `workspace`    | dir + index                                        | discovery, loaders, package-graph, filter, affected, `ProjectEntry` (moved in), `computeNestedProjectDirs` (moved in), `computeWorkspaceFingerprint` (moved in) |
+| `graph`        | dir + index                                        | task-graph, scheduler, dependency-spec, `TaskNode`/`TaskOutcome`/`TaskStatus`                                                                                   |
+| `cache`        | dir + index                                        | `Cache`, `CacheLayer`, `LayeredCache`, `RemoteCache`, inputs/outputs resolution. `tar.ts` stays internal                                                        |
+| `exec`         | dir + index                                        | runner, env, sandbox-runtime                                                                                                                                    |
+| `orchestrator` | dir + index (entry moves to `orchestrator/run.ts`) | `run`, `planRun`, options/summary types, `Logger`, `defaultLogger`, `RunPlan` types                                                                             |
+| `cli`          | dir + index (`cli.ts` moves in)                    | dispatcher + test-facing parser exports                                                                                                                         |
+
+Root files outside the module set: `bin.ts`, `index.ts` (public
+façade — exports unchanged), `version.ts` (new).
+
+### Allowed dependency matrix (rows import columns, via index only)
+
+|                  | util | config | version | workspace | graph | cache | exec | orchestrator | cli |
+| ---------------- | ---- | ------ | ------- | --------- | ----- | ----- | ---- | ------------ | --- |
+| **workspace**    | ✓    | ✓      |         | —         |       |       |      |              |     |
+| **graph**        | ✓    | ✓      |         | ✓         | —     |       |      |              |     |
+| **cache**        | ✓    | ✓      |         |           |       | —     |      |              |     |
+| **exec**         | ✓    | ✓      |         |           |       |       | —    |              |     |
+| **orchestrator** | ✓    | ✓      | ✓       | ✓         | ✓     | ✓     | ✓    | —            |     |
+| **cli**          | ✓    | ✓      | ✓       | ✓         | ✓     | ✓     |      | ✓            | —   |
+| **index**        |      | ✓      | ✓       |           | ✓     |       |      | ✓            |     |
+| **bin**          | ✓    |        |         |           |       |       |      |              | ✓   |
+
+Composition happens only at `orchestrator` and `cli`. `cli → cache`
+stays (prune/stats open the cache without a run). `cli → exec` is
+deliberately absent.
+
+### Enforcement
+
+Primary: `tests/module-boundaries.test.ts` — scans `src/**/*.ts`
+import specifiers, asserts (rule 1) every cross-module edge is in the
+ALLOWED matrix, and (rule 2) imports of modules listed in a
+`CONTRACTED` set target the module's `index.ts` only. `CONTRACTED`
+starts empty and grows as contract PRs land — a ratchet. Tests in
+`tests/` are exempt (they may exercise internals). Zero new deps;
+runs under the existing `bun test` CI gate.
+
+Secondary: `import/no-cycle` in oxlint, if available — verify it
+fires before trusting it; the boundary test catches directory-level
+cycles either way.
+
+## 3. Migration plan (each PR: zero behavior change, suite green)
+
+| PR  | What                                                                                                                |
+| --- | ------------------------------------------------------------------------------------------------------------------- |
+| 1   | This doc + boundary test (matrix law from day one) + cycle breaks: `version.ts`, `orchestrator/options.ts`          |
+| 2   | Relocations: `ProjectEntry` → workspace; `nested-dirs`, `fingerprint` → workspace; `plan-format` → cli              |
+| 3   | Split `execute-task.ts` hashing surface → `orchestrator/task-hash.ts`                                               |
+| 4   | Contracts: `cache/index.ts`, `exec/index.ts`, `util/index.ts`; rewrite importers; CONTRACTED += {cache, exec, util} |
+| 5   | Contracts: `workspace/index.ts`, `graph/index.ts`; CONTRACTED += {workspace, graph}                                 |
+| 6   | Entries: `orchestrator.ts` → `orchestrator/{index,run}.ts`; `cli.ts` → `cli/index.ts`; CONTRACTED complete          |
+| 7   | Docs: architecture.md module map, modules/ pages, CLAUDE.md layout                                                  |
+
+## Out of scope
+
+Test colocation (tests stay in `tests/`); renaming public exports;
+splitting `cache/cache.ts` internals; exec's `RunOptions` rename
+(flagged, optional); package-level splitting; any behavior change.
+Cache key derivation untouched — no CACHE_VERSION bump anywhere in
+the series.

src/cli.ts

@@ -2,7 +2,7 @@
 // `src/cli/<name>.ts`. Re-exports below are for the test suite,
 // which asserts on the pure parsers and formatters directly.
 
-import { VERSION } from './index.js'
+import { VERSION } from './version.js'
 import { runCmd } from './cli/run.js'
 import { watchCmd } from './cli/watch.js'
 import { cacheCmd } from './cli/cache.js'

src/index.ts

@@ -1,6 +1,6 @@
 // Public API for @vzn/vx.
 
-export const VERSION = '0.0.0'
+export { VERSION } from './version.js'
 
 // Schema types and helpers (used by user vx.config files and presets).
 export type {

src/orchestrator.ts

@@ -4,7 +4,7 @@
 
 import type { RunRecord } from './cache/cache.js'
 import { LayeredCache } from './cache/layered-cache.js'
-import { VERSION } from './index.js'
+import { VERSION } from './version.js'
 import { runGraph, type TaskOutcome } from './graph/scheduler.js'
 import { isGroupTask } from './graph/task-graph.js'
 import { ulid } from './util/ulid.js'
@@ -20,47 +20,8 @@ import { writeRunProfile, writeRunSummary } from './orchestrator/run-artifacts.t
 import { formatRunSummary } from './orchestrator/summary.ts'
 
 export type { Logger } from './orchestrator/logger.ts'
-
-export interface RunOptions {
-  cwd: string
-  /**
-   * Task specs to run. Each may be a bare task name (`'build'`) —
-   * applied across `projects` to every project that declares it —
-   * or an anchored `'pkg#task'` — added directly to the requested
-   * set regardless of `projects`.
-   */
-  tasks: readonly string[]
-  projects?: string[]
-  concurrency?: number
-  /** Skip cache reads AND writes. Every task runs and nothing is persisted. */
-  noCache?: boolean
-  /**
-   * Filter `dependsOn` expansion. `'all'` drops every edge (just the
-   * requested task runs). A string array drops only those task names
-   * from both `self` and `dependencies` buckets.
-   */
-  excludeDependencies?: 'all' | readonly string[]
-  /** Forwarded to the last step of each task's exec array (shell-quoted). */
-  forwardArgs?: readonly string[]
-  /**
-   * If set, write a per-run JSON summary at end of run. Empty string
-   * picks the default path `<cacheDir>/runs/<run_id>.json`; anything
-   * else is treated as the literal file path (cwd-relative).
-   */
-  summarize?: string
-  /**
-   * If set, write a Chrome-trace JSON profile of the run's wallclock
-   * spans. Path is cwd-relative. Default `profile.json` is selected
-   * by the CLI parser, not here.
-   */
-  profile?: string
-  log?: Logger
-}
-
-export interface RunSummary {
-  ok: boolean
-  outcomes: TaskOutcome[]
-}
+export type { RunOptions, RunSummary } from './orchestrator/options.ts'
+import type { RunOptions, RunSummary } from './orchestrator/options.ts'
 
 export async function run(options: RunOptions): Promise<RunSummary> {
   // Color decision: a custom logger (tests, embedders) handles its

src/orchestrator/options.ts

@@ -0,0 +1,47 @@
+// Run option/summary types live in a leaf file (not the module entry)
+// so internals like prepare.ts can import them without an upward
+// import of orchestrator.ts — the module entry must stay cycle-free.
+
+import type { TaskOutcome } from '../graph/scheduler.js'
+import type { Logger } from './logger.ts'
+
+export interface RunOptions {
+  cwd: string
+  /**
+   * Task specs to run. Each may be a bare task name (`'build'`) —
+   * applied across `projects` to every project that declares it —
+   * or an anchored `'pkg#task'` — added directly to the requested
+   * set regardless of `projects`.
+   */
+  tasks: readonly string[]
+  projects?: string[]
+  concurrency?: number
+  /** Skip cache reads AND writes. Every task runs and nothing is persisted. */
+  noCache?: boolean
+  /**
+   * Filter `dependsOn` expansion. `'all'` drops every edge (just the
+   * requested task runs). A string array drops only those task names
+   * from both `self` and `dependencies` buckets.
+   */
+  excludeDependencies?: 'all' | readonly string[]
+  /** Forwarded to the last step of each task's exec array (shell-quoted). */
+  forwardArgs?: readonly string[]
+  /**
+   * If set, write a per-run JSON summary at end of run. Empty string
+   * picks the default path `<cacheDir>/runs/<run_id>.json`; anything
+   * else is treated as the literal file path (cwd-relative).
+   */
+  summarize?: string
+  /**
+   * If set, write a Chrome-trace JSON profile of the run's wallclock
+   * spans. Path is cwd-relative. Default `profile.json` is selected
+   * by the CLI parser, not here.
+   */
+  profile?: string
+  log?: Logger
+}
+
+export interface RunSummary {
+  ok: boolean
+  outcomes: TaskOutcome[]
+}

src/orchestrator/prepare.ts

@@ -29,7 +29,7 @@ import { computeWorkspaceFingerprint } from './fingerprint.js'
 import { wrapWithRemoteCache } from './remote-cache-setup.js'
 import { createHashCache, type HashCache } from './execute-task.js'
 import type { Logger } from './logger.js'
-import type { RunOptions } from '../orchestrator.js'
+import type { RunOptions } from './options.js'
 
 export interface PreparedRun {
   workspaceRoot: string

src/version.ts

@@ -0,0 +1,4 @@
+// Leaf module so orchestrator/cli can read the version without
+// importing the public façade (index.ts imports the orchestrator —
+// that edge must stay one-directional).
+export const VERSION = '0.0.0'

tests/module-boundaries.test.ts

@@ -0,0 +1,100 @@
+// Module-boundary law. See docs/design/module-isolation-2026-06.md.
+//
+// Rule 1: a file in module A may import from module B only when the
+//         ALLOWED matrix grants A → B.
+// Rule 2: once a module is listed in CONTRACTED, cross-module imports
+//         of it must target its index.ts (the contract), never an
+//         internal file. CONTRACTED is a ratchet — it grows as the
+//         contract PRs land and never shrinks.
+//
+// Only src/ is scanned. Tests are exempt: they may exercise internals.
+
+import path from 'node:path'
+import { describe, expect, it } from 'bun:test'
+
+const SRC = path.join(import.meta.dir, '..', 'src')
+
+/** Module of a src-relative file path. Root files are their own modules. */
+function moduleOf(rel: string): string {
+  const seg = rel.split('/')
+  if (seg.length > 1) return seg[0]!
+  return rel.replace(/\.ts$/, '') // bin / index / cli / orchestrator / config / version
+}
+
+const ALLOWED: Record<string, readonly string[]> = {
+  util: [],
+  config: [],
+  version: [],
+  workspace: ['util', 'config'],
+  graph: ['util', 'config', 'workspace'],
+  cache: ['util', 'config'],
+  exec: ['util', 'config'],
+  orchestrator: ['util', 'config', 'version', 'workspace', 'graph', 'cache', 'exec'],
+  cli: ['util', 'config', 'version', 'workspace', 'graph', 'cache', 'orchestrator'],
+  index: ['config', 'version', 'graph', 'orchestrator'],
+  bin: ['util', 'cli'],
+}
+
+// Modules whose contract (index.ts) is the only legal cross-module
+// import target. Grows per migration PR; target: every directory module.
+const CONTRACTED: readonly string[] = []
+
+interface Edge {
+  from: string
+  to: string
+  fromModule: string
+  toModule: string
+  specifier: string
+}
+
+async function collectEdges(): Promise<Edge[]> {
+  const edges: Edge[] = []
+  const glob = new Bun.Glob('**/*.ts')
+  for await (const rel of glob.scan({ cwd: SRC })) {
+    const norm = rel.split(path.sep).join('/')
+    const text = await Bun.file(path.join(SRC, rel)).text()
+    // Static `import ... from 'x'` / `export ... from 'x'` specifiers.
+    // No dynamic imports exist on boundary paths today; if one appears
+    // the matrix below is the place to encode the decision.
+    for (const m of text.matchAll(/^(?:import|export)[^'"]*from\s+['"]([^'"]+)['"]/gm)) {
+      const spec = m[1]!
+      if (!spec.startsWith('.')) continue // bare imports = packages, not modules
+      const resolved = path
+        .normalize(path.join(path.dirname(norm), spec))
+        .split(path.sep)
+        .join('/')
+        .replace(/\.(js|ts)$/, '')
+      const fromModule = moduleOf(norm)
+      const toModule = moduleOf(`${resolved}.ts`)
+      if (fromModule === toModule) continue
+      edges.push({ from: norm, to: resolved, fromModule, toModule, specifier: spec })
+    }
+  }
+  return edges
+}
+
+describe('module boundaries', () => {
+  it('every cross-module import edge is in the ALLOWED matrix', async () => {
+    const edges = await collectEdges()
+    expect(edges.length).toBeGreaterThan(0)
+    const violations = edges.filter((e) => {
+      const allowed = ALLOWED[e.fromModule]
+      // Unknown module = new top-level file/dir; force a matrix decision.
+      if (allowed === undefined) return true
+      return !allowed.includes(e.toModule)
+    })
+    expect(
+      violations.map((v) => `${v.from} → ${v.specifier} (${v.fromModule} → ${v.toModule})`),
+    ).toEqual([])
+  })
+
+  it('contracted modules are imported only via their index', async () => {
+    const edges = await collectEdges()
+    const violations = edges.filter(
+      (e) => CONTRACTED.includes(e.toModule) && e.to !== `${e.toModule}/index`,
+    )
+    expect(
+      violations.map((v) => `${v.from} → ${v.specifier} (must import ${v.toModule}/index)`),
+    ).toEqual([])
+  })
+})