From 655376b62b2049562d950bc90320264a7747f068 Mon Sep 17 00:00:00 2001 From: Santosh Yadav Date: Sat, 10 Oct 2026 23:04:56 +0200 Subject: [PATCH] chore: add monorepo doctor skill --- .agents/skills/monorepo-doctor/SKILL.md | 36 ++++ .../monorepo-doctor/references/boundaries.md | 43 ++++ .../monorepo-doctor/references/caching.md | 36 ++++ .../monorepo-doctor/references/ci-coverage.md | 31 +++ .../references/circular-deps.md | 105 +++++++++ .../monorepo-doctor/references/detection.md | 165 ++++++++++++++ .../references/prod-leakage.md | 43 ++++ .../references/report-format.md | 142 ++++++++++++ .../skills/monorepo-doctor/references/scan.md | 203 ++++++++++++++++++ monorepo-doctor-report.json | 139 ++++++++++++ monorepo-doctor-report.md | 111 ++++++++++ skills-lock.json | 11 + 12 files changed, 1065 insertions(+) create mode 100644 .agents/skills/monorepo-doctor/SKILL.md create mode 100644 .agents/skills/monorepo-doctor/references/boundaries.md create mode 100644 .agents/skills/monorepo-doctor/references/caching.md create mode 100644 .agents/skills/monorepo-doctor/references/ci-coverage.md create mode 100644 .agents/skills/monorepo-doctor/references/circular-deps.md create mode 100644 .agents/skills/monorepo-doctor/references/detection.md create mode 100644 .agents/skills/monorepo-doctor/references/prod-leakage.md create mode 100644 .agents/skills/monorepo-doctor/references/report-format.md create mode 100644 .agents/skills/monorepo-doctor/references/scan.md create mode 100644 monorepo-doctor-report.json create mode 100644 monorepo-doctor-report.md create mode 100644 skills-lock.json diff --git a/.agents/skills/monorepo-doctor/SKILL.md b/.agents/skills/monorepo-doctor/SKILL.md new file mode 100644 index 00000000..b0dc1957 --- /dev/null +++ b/.agents/skills/monorepo-doctor/SKILL.md @@ -0,0 +1,36 @@ +--- +name: monorepo-doctor +description: Use when a user asks for a JS/TS monorepo audit, monorepo health check, Nx/Turborepo cache issues, affected tests not running, circular dependencies, package boundary drift, or test code shipped to production. Read-only; writes Markdown and JSON reports. +--- + +# Monorepo Doctor + +Audit a JS/TS monorepo for five classes of problems that teams routinely miss, and produce an evidence-backed report. + +Prerequisites: Node ≥ 22.5. The work dir is `$W` = `$(node -p 'require("os").tmpdir()')/monorepo-doctor` (see detection.md). + +## Rules + +- **Read-only.** Never install dependencies, build, run tests, or edit project files. Allowed writes: `monorepo-doctor-report.md`, `monorepo-doctor-report.json` (workspace root or a user-given dir) and `$W`. +- Allowed commands: the snippets in the reference files, `npx --yes knip@6.41.0`, `npm_config_ignore_scripts=true npx --yes publint@0.3.25` (exact pinned versions — never unpinned or `@latest`; bump only after review), `npm pack --dry-run --json --ignore-scripts`, and `npx nx` / `npx turbo` **only if** detection reports them `installed` (graph / `--dry=json` / `boundaries` only — never real task runs, with read-only env vars from the references). +- **Never abort a check.** If a tool fails, mark the check `partial` or `skipped` with a reason and continue. +- **Every finding needs evidence** (config snippet, file:line, or command output). No evidence → no finding. +- Use only the finding ids defined in the reference files. + +## Workflow + +1. **Detect** — follow [references/detection.md](references/detection.md). Stop if `NO_WORKSPACE`. +2. **Scan** — follow [references/scan.md](references/scan.md). +3. **Check** — run all five unless the user asked for a subset (write `{"status":"not-run","reason":"not requested","findings":[]}` to `$W/findings/.json` for the others): + - [references/boundaries.md](references/boundaries.md) — package split & public API + - [references/circular-deps.md](references/circular-deps.md) — package and file cycles + - [references/caching.md](references/caching.md) — Nx / Turborepo cache correctness + - [references/ci-coverage.md](references/ci-coverage.md) — tests that silently don't run in CI + - [references/prod-leakage.md](references/prod-leakage.md) — test code shipped to production + The checks are independent; if you can run sub-agents, run them in parallel. +4. **Report** — follow [references/report-format.md](references/report-format.md): validate each `findings/.json`, render, then show the user the summary line and the top 5 findings. +5. **CI mode** (only if asked, or when invoked non-interactively with a threshold) — run the exit-code snippet from report-format.md. + +## Tracking + +Create a todo per phase/check and mark each done as its findings file is written. diff --git a/.agents/skills/monorepo-doctor/references/boundaries.md b/.agents/skills/monorepo-doctor/references/boundaries.md new file mode 100644 index 00000000..b63c48dc --- /dev/null +++ b/.agents/skills/monorepo-doctor/references/boundaries.md @@ -0,0 +1,43 @@ +# Check 1 — Package boundaries + +Question: are packages split along real seams, and is each package's public API respected? +Inputs: `workspace.json`, `scan.json`. Output: `findings/boundaries.json`. + +## Rules + +| id | severity | detect | evidence | +| ---------------------------------------------- | -------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | +| `boundaries/oversized-package` | low | `scan.packages[p].sourceFiles > 500` or `loc > 50000` | counts | +| `boundaries/deep-import` | medium | each `scan.imports` entry with `deep && (declared \|\| fromSource === "project.json")` (undeclared package.json importers are reported by ci-coverage instead) | file:line + import specifier + target's `exports` keys | +| `boundaries/unlisted-dependency` | medium | knip `unlisted` entries | knip output line | +| `boundaries/unused-dependency` | low | knip `dependencies` / `devDependencies` entries | knip output line | +| `boundaries/nx-missing-module-boundaries-rule` | medium | Nx detected and no ESLint config in the root mentions `@nx/enforce-module-boundaries` | list of ESLint config files checked | +| `boundaries/nx-permissive-module-boundaries` | low | the rule exists but `depConstraints` contains `{ sourceTag: '*', onlyDependOnLibsWithTags: ['*'] }` | the config snippet | +| `boundaries/nx-untagged-project` | low | Nx detected, rule absent or permissive, and a project has empty `tags` | project.json path | +| `boundaries/turbo-boundaries-violation` | medium | each violation printed by `turbo boundaries` | output line | + +Group `deep-import` findings: one finding per (from, target) pair, listing up to 5 locations in `evidence`; `location` is the first occurrence. + +## Steps + +1. Load `workspace.json` and `scan.json`. Apply the size and deep-import rules directly. +2. **knip** — only if `hasNodeModules`: + ```bash + W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" + npx --yes knip@6.41.0 --reporter json --include dependencies,unlisted --no-progress --no-exit-code + ``` + Parse stdout as JSON. Its shape varies by version; look for per-file `dependencies`, `devDependencies` and `unlisted` arrays of `{ name }`. Map the file to its package via `workspace.json`. If knip fails or the output can't be parsed, mark the check `partial` with reason `knip unavailable: `. Do **not** report `unused-dependency` for `@types/*` packages or for tools only referenced in scripts. +3. **Nx** — only if Nx is an orchestrator: read ESLint configs in the root (`eslint.config.{js,mjs,cjs,ts}`, `.eslintrc{,.json,.js,.cjs}`) and every package dir. Apply the three `nx-*` rules. A config that contains `@nx/enforce-module-boundaries` or legacy `@nrwl/nx/enforce-module-boundaries` anywhere counts as present. Group `nx-untagged-project` consistently as one finding per untagged project found from the root and package-dir project configs. +4. **Turborepo** — only if turbo is an orchestrator **and installed**: + ```bash + W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" + TURBO_TELEMETRY_DISABLED=1 npx turbo boundaries + ``` + One finding per reported violation. If the command errors with an unknown-command message (turbo < 2.4), skip this sub-step and note it in `reason`. +5. Write `findings/boundaries.json`. + +## Fix guidance + +- deep-import: add the subpath to the target's `exports`, or re-export from its entry point, then import from the package root. +- oversized-package: identify clusters by directory and by which consumers import which files (`scan.imports` subpaths); propose 2–3 candidate splits. +- nx-missing-module-boundaries-rule: add `@nx/enforce-module-boundaries` with `depConstraints` based on `scope:*`/`type:*` tags. diff --git a/.agents/skills/monorepo-doctor/references/caching.md b/.agents/skills/monorepo-doctor/references/caching.md new file mode 100644 index 00000000..a6153c28 --- /dev/null +++ b/.agents/skills/monorepo-doctor/references/caching.md @@ -0,0 +1,36 @@ +# Check 3 — Task caching + +Question: will a cache hit ever replay a wrong result, and are tasks re-running when they should hit? +Output: `findings/caching.json`. If no orchestrator is detected, only `caching/uncached-root-script` applies; set status `ok` with reason `no task orchestrator`. + +## Facts the rules rely on + +- **Turborepo**: a task with no `outputs` caches only logs — on a hit, `dist/` is NOT restored, so downstream tasks see stale or missing files. A package's hash covers its own files, its `package.json`, `turbo.json` and the lockfile; changes in workspace dependencies reach a task's hash **only** through `dependsOn` entries with `^` (e.g. `^build`). Env vars affect the hash only when listed in `env`, `globalEnv` (or `passThroughEnv` for runtime-only); framework prefixes such as `NEXT_PUBLIC_`, `VITE_` are auto-included. Turbo 2 strict env mode can also restrict which env vars are visible to tasks, so undeclared env is both a cache-key and runtime-visibility risk. +- **Nx**: when `outputs` is omitted, Nx caches `{workspaceRoot}/dist/{projectRoot}`, `{projectRoot}/dist`, `{projectRoot}/build`, `{projectRoot}/public`. Env vars affect the hash only through an `{ "env": "NAME" }` entry in `inputs`. Without a `production` named input that excludes tests, editing a spec file invalidates the build cache. + +## Rules + +| id | severity | detect | +| ------------------------------------ | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `caching/missing-outputs` | high | **Turbo**: a cacheable task named `build`/`compile`/`bundle`/`*:build` (or whose package script runs `tsc`, `vite build`, `tsup`, `rollup`, `webpack`, `next build`, `esbuild`) has no `outputs`. Package-level `turbo.json` overrides count. **Nx**: cacheable build-like target without `outputs` whose real out dir (`outDir` in the build tsconfig, `outDir`/`--out-dir` flags, `build.outDir` in vite config) is not one of Nx's defaults — one finding per affected project, with `package`. Turbo findings are per task: `location.file: "turbo.json"`, no `package`. | +| `caching/build-inputs-include-tests` | low | **Turbo**: build-like task has no `inputs`, or `inputs` that don't exclude test globs (`!**/*.test.*`, `!**/*.spec.*`, `!**/__tests__/**`). **Nx**: no `production` named input, or `build` inputs don't reference it, or `production` doesn't negate test globs. Low severity because `$TURBO_DEFAULT$` already covers all git-tracked files; the finding is about avoiding rebuilds when tests change, not correctness. | +| `caching/undeclared-env` | medium | for each `scan.env` entry that can affect a cacheable task output. Flag build tasks only when the build inlines env (Next/Vite framework bundlers, webpack/Rspack/Rsbuild `define`, esbuild/tsup `define`, Rollup/plugin-replace, or similar bundlers), not runtime-only reads in a plain `tsc` service. Always flag cacheable test tasks when tests execute code that reads the env var (`scan.env.isTest`, or a test file in the package imports the module with the env read), because a cached pass can be replayed under a different value. **Turbo**: name not matched by the task's `env` or `globalEnv` (`*` wildcards; `!` negation) and not framework-prefixed. `passThroughEnv` / `globalPassThroughEnv` do **not** count: they make the var available without adding it to the hash, so a cached result can still be replayed under a different value. **Nx**: no `{ "env": NAME }` in the target's inputs (resolve named inputs). Ignore `NODE_ENV`, `CI`, `PATH`, `HOME`, `TZ`, `PWD`. One finding per (package, name), listing up to 5 locations. | +| `caching/turbo-cache-disabled` | medium | Turbo task with `cache: false` that is not `persistent: true` and whose name isn't `dev`, `start`, `serve`, `watch`, `deploy`, `release`, `publish`, `clean`. | +| `caching/unstable-hash` | high | turbo installed: run `TURBO_TELEMETRY_DISABLED=1 npx turbo run build --dry=json` twice; any task whose `hash` differs between runs. Report the task and the `inputs`/`environmentVariables` that differ. | +| `caching/volatile-input` | high | non-negated task `inputs` or `globalDependencies` include build artifacts or machine-local files: `dist`, `build`, `.turbo`, `.nx`, `*.tsbuildinfo`, `**/*.log`, `node_modules`. Ignore negated globs (`!…`). Do not flag `.env*` files: including env files usually makes caches more correct. | +| `caching/uncached-root-script` | low | a root `package.json` script that runs tasks across packages bypassing the orchestrator: `-r`/`--recursive`, `--workspaces`/`-ws`, `yarn workspaces foreach`, `lerna run`. In repos **without** an orchestrator, say "consider adding Nx or Turborepo" in `fix`. | + +## Steps + +1. Read `turbo.json` (Turbo v2 `tasks` and Turbo v1 `pipeline`, plus `//#task` root tasks and `/turbo.json` overrides with `"extends": ["//"]`) and/or `nx.json` + every `project.json` / `package.json#nx`. For Nx, also read `targetDefaults` (including defaults keyed by executor), `namedInputs`, and legacy `tasksRunnerOptions.*.options.cacheableOperations`, and remember that Nx plugins (`plugins` in `nx.json`) can infer targets — if a target exists only via plugin inference, report rules that depend on its config as `partial` and say so. +2. Apply the rules above. Quote the exact JSON in `evidence` (task name + its config object). Config-level findings (Turbo tasks, Nx `targetDefaults`/`namedInputs`) omit `location.package`; only set `package` when the problem is specific to one package (e.g. its own `turbo.json`/`project.json`, or an env read in its source). +3. `unstable-hash` runs only when turbo is installed; otherwise mark the check `partial` with reason `turbo not installed; dry-run hash comparison skipped`. Never run real tasks — only `--dry=json`. +4. Write `findings/caching.json`. + +## Fix guidance + +- missing-outputs (Turbo): `"outputs": ["dist/**"]` (add `".next/**", "!.next/cache/**"` for Next.js). +- missing-outputs (Nx): `"outputs": ["{projectRoot}/lib"]` matching the real out dir. +- build-inputs-include-tests (Nx): `"namedInputs": { "production": ["default", "!{projectRoot}/**/*.spec.ts", "!{projectRoot}/**/*.test.ts"] }` and `"build": { "inputs": ["production", "^production"] }`. +- build-inputs-include-tests (Turbo): `"inputs": ["$TURBO_DEFAULT$", "!**/*.test.*", "!**/*.spec.*"]` (`$TURBO_DEFAULT$` means all git-tracked files in the package). +- undeclared-env: Turbo `"env": ["API_URL"]`; Nx `"inputs": ["production", { "env": "API_URL" }]`. diff --git a/.agents/skills/monorepo-doctor/references/ci-coverage.md b/.agents/skills/monorepo-doctor/references/ci-coverage.md new file mode 100644 index 00000000..fcd10fa4 --- /dev/null +++ b/.agents/skills/monorepo-doctor/references/ci-coverage.md @@ -0,0 +1,31 @@ +# Check 4 — CI test coverage + +Question: can a change land without the tests that cover it running in CI? +Inputs: `workspace.json` (`ciFiles`), `scan.json`. Output: `findings/ci-coverage.json`. + +## Rules + +| id | severity | detect | +| --------------------------------------------- | -------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `ci-coverage/undeclared-workspace-dependency` | critical (medium when Nx is an orchestrator, since Nx infers edges from imports) | `scan.imports` entry with `declared: false`, `isTest: false` and `fromSource: "package.json"`. Affected/filtered runs and `^` caching won't treat the importer as depending on the target, so changing the target skips the importer's tests. One finding per (from, target) pair. | +| `ci-coverage/missing-test-script` | high | package has `scan.packages[p].testFiles.length > 0` but no package-level test entry point is covered. Accept `test`, `e2e`, or `test:*` package scripts; an Nx `test` target; inferred Nx test plugins (`@nx/vite/plugin`, `@nx/jest/plugin`, `@nx/vitest`); root `vitest.workspace.*` / root `vitest.config.*` `test.projects`; root Jest `projects`; or a root test script whose globs explicitly include the package path. Do **not** treat `pnpm -r test`, `npm --workspaces test`, or similar recursive root scripts as coverage for packages without their own test script (generic-pnpm deliberately still flags `@acme/app`). | +| `ci-coverage/tests-not-run-in-ci` | high | at least one CI file exists, but no followed CI file matches `/\b(npm\|pnpm\|yarn\|bun\|npx\|nx\|turbo)\b[^\n]*\btest\b\|\b(vitest\|jest\|mocha)\b\|\b(make\|task\|just)\s+test\b/` (each `\|` is a Markdown table escape for a regex alternation bar). If `ciFiles` is empty, set this check status to `partial` with reason `no CI configuration found` and emit no `tests-not-run-in-ci` finding. | +| `ci-coverage/shallow-checkout-affected` | high | an affected/filtered CI job lacks full base history. GitHub Actions: job runs `nx affected`, `turbo … --affected`, or `--filter=…[]`, uses `actions/checkout`, and does **not** set `fetch-depth: 0`. GitLab: affected logic is used while `GIT_DEPTH` is unset/nonzero (default 20) or the MR target branch is not fetched. Azure: `checkout: self` lacks `fetchDepth: 0`. Bitbucket: `clone: depth` is set while affected logic is used. | +| `ci-coverage/nx-affected-base-missing` | high | `nx affected` in CI with no `--base`, no `nrwl/nx-set-shas` step, no `NX_BASE` env, and no `nx.json#defaultBase`. | +| `ci-coverage/nx-test-inputs-ignore-deps` | high | Nx `test` target (targetDefaults or project) is cacheable and explicitly sets `inputs` with no `^` dependency entry after resolving named inputs. Omitted `inputs` are ok because Nx defaults to `default` + `^default`. | +| `ci-coverage/turbo-test-missing-dependson` | high | Turbo `test` (or `*:test`) task is cacheable and its `dependsOn` graph has no transitive `^` entry. Follow local task indirections such as `"transit": { "dependsOn": ["^transit"] }`, `"test": { "dependsOn": ["transit"] }`; that pattern satisfies the rule. | + +## Steps + +1. Read every file in `ciFiles`. Also follow GitLab `include:` entries that point to local files and local reusable GitHub workflows referenced with `uses: ./.github/workflows/x.yml`. For GitHub Actions, reason per job: a checkout step's `with.fetch-depth` applies to the steps after it in the same job. If there are no CI files, write `status: "partial"`, `reason: "no CI configuration found"`, and do not emit `ci-coverage/tests-not-run-in-ci`. +2. Resolve Nx targets as in caching (targetDefaults merged with project config). Resolve Turbo tasks from `turbo.json`/`pipeline` and package overrides. +3. Apply the rules; quote the CI step / config object as `evidence`. +4. Write `findings/ci-coverage.json`. + +## Fix guidance + +- undeclared-workspace-dependency: add `"": "workspace:*"` (pnpm/yarn/bun) or `"*"` (npm) to the importer's dependencies, and import via the package name. +- shallow-checkout-affected: `with: { fetch-depth: 0 }` on `actions/checkout`. +- nx-affected-base-missing: add `- uses: nrwl/nx-set-shas@v4` before `nx affected`. +- nx-test-inputs-ignore-deps: `"test": { "inputs": ["default", "^production"] }`. +- turbo-test-missing-dependson: `"test": { "dependsOn": ["^build"] }` (or `["^test"]` when tests run on sources). diff --git a/.agents/skills/monorepo-doctor/references/circular-deps.md b/.agents/skills/monorepo-doctor/references/circular-deps.md new file mode 100644 index 00000000..26f06000 --- /dev/null +++ b/.agents/skills/monorepo-doctor/references/circular-deps.md @@ -0,0 +1,105 @@ +# Check 2 — Circular dependencies + +Output: `findings/circular-deps.json`. + +| id | severity | detect | +| ----------------------------- | -------- | -------------------------------------------------------------------------------------------- | +| `circular-deps/package-cycle` | high | a cycle in the package graph (`workspaceDeps`, or the Nx project graph when Nx is installed) | +| `circular-deps/file-cycle` | medium | a cycle among a package's own files (`scan.fileEdges`) | + +Package cycles break topological builds (`^build` order is undefined), cause stale caches and make packages unpublishable independently. +File cycles cause `undefined` at import time in ESM/CJS and block tree-shaking. + +## Steps + +1. If Nx is installed, refresh the graph and use it as the package graph: + ```bash + W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" + rm -f "$W/nx-graph.json" "$W/nx-graph.failed" + NX_DAEMON=false NX_NO_CLOUD=true NX_WORKSPACE_DATA_DIRECTORY="$W/nx-data" npx nx graph --file="$W/nx-graph.json" \ + || touch "$W/nx-graph.failed" + ``` + The file has `graph.dependencies[project] = [{ target }]`. Otherwise use `workspace.json` `workspaceDeps`. + If Turborepo is installed, `TURBO_TELEMETRY_DISABLED=1 npx turbo run build --dry=json` failing with a message containing `cyclic` is also evidence. Turbo dry-run JSON writes no project files beyond Turborepo's own cache directory. + Status: the `workspaceDeps` graph is complete for package.json-based workspaces, so missing turbo does **not** make this check partial. Under Nx without a local install, mark `partial` with reason `nx not installed; project graph inferred from package.json only`; if Nx is installed but `nx graph` fails, mark `partial` with reason `nx graph failed; project graph inferred from package.json only` (Nx also infers edges from imports and `implicitDependencies`). +2. Run the cycle snippet: + +```bash +W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" +cat > "$W/cycles.mjs" <<'EOF' +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +const work = path.join(os.tmpdir(), 'monorepo-doctor'); +const ws = JSON.parse(fs.readFileSync(path.join(work, 'workspace.json'), 'utf8')); +const scan = JSON.parse(fs.readFileSync(path.join(work, 'scan.json'), 'utf8')); +const nxGraphPath = path.join(work, 'nx-graph.json'); + +function cycles(graph) { + const found = new Map(); + const state = new Map(); + const stack = []; + const visit = (n) => { + state.set(n, 1); stack.push(n); + for (const m of graph.get(n) ?? []) { + if (state.get(m) === 1) { + const cyc = stack.slice(stack.indexOf(m)); + const i = cyc.indexOf([...cyc].sort()[0]); + const norm = [...cyc.slice(i), ...cyc.slice(0, i)]; + found.set(norm.join('\u0000'), norm); + } else if (!state.has(m)) visit(m); + } + stack.pop(); state.set(n, 2); + }; + for (const n of graph.keys()) if (!state.has(n)) visit(n); + return [...found.values()]; +} + +const nx = ws.orchestrators.find((o) => o.name === 'nx'); +const graphFailed = Boolean(nx?.installed) && (fs.existsSync(path.join(work, 'nx-graph.failed')) || !fs.existsSync(nxGraphPath)); +const useNxGraph = Boolean(nx?.installed) && !graphFailed; +let pkgGraph; +if (useNxGraph) { + const g = JSON.parse(fs.readFileSync(nxGraphPath, 'utf8')).graph; + pkgGraph = new Map(Object.entries(g.dependencies).map(([k, v]) => [k, v.map((d) => d.target).filter((t) => g.nodes[t])])); +} else { + pkgGraph = new Map(ws.packages.map((p) => [p.name, p.workspaceDeps])); +} +const byName = new Map(ws.packages.map((p) => [p.name, p])); +const findings = []; +for (const c of cycles(pkgGraph)) { + const first = byName.get(c[0]); + findings.push({ + id: 'circular-deps/package-cycle', severity: 'high', + title: `Package cycle: ${[...c, c[0]].join(' → ')}`, + location: { file: first ? `${first.dir}/package.json` : 'package.json', package: c[0] }, + evidence: c.map((n, i) => `${n} depends on ${c[(i + 1) % c.length]}`).join('\n'), + fix: 'Extract the shared code into a new package both can depend on, or invert one edge (dependency injection / events). Remove the back-edge from package.json.', + }); +} +for (const [pkg, edges] of Object.entries(scan.fileEdges)) { + const g = new Map(); + for (const [a, b] of edges) { if (!g.has(a)) g.set(a, []); g.get(a).push(b); } + for (const c of cycles(g)) { + findings.push({ + id: 'circular-deps/file-cycle', severity: 'medium', + title: `File cycle in ${pkg} (${c.length} files)`, + location: { file: c[0], package: pkg }, + evidence: [...c, c[0]].join('\n → '), + fix: 'Move the shared symbols into a third module that both import, or merge the files.', + }); + } +} +const status = nx && !useNxGraph ? 'partial' : 'ok'; +const result = { status, findings }; +if (status === 'partial') result.reason = graphFailed + ? 'nx graph failed; project graph inferred from package.json only' + : 'nx not installed; project graph inferred from package.json only'; +fs.writeFileSync(path.join(work, 'findings', 'circular-deps.json'), JSON.stringify(result, null, 2)); +console.log(JSON.stringify({ packageCycles: findings.filter((f) => f.id.endsWith('package-cycle')).length, fileCycles: findings.filter((f) => f.id.endsWith('file-cycle')).length })); +EOF +node "$W/cycles.mjs" +``` + +3. Review: for each cycle, open the import lines to confirm they are value imports. Drop false positives (e.g., an import inside a comment) before rendering. +4. If more than 20 file cycles exist in one package, keep the 20 shortest and add `"reason": "N file cycles; showing 20 shortest"` with status `partial`. diff --git a/.agents/skills/monorepo-doctor/references/detection.md b/.agents/skills/monorepo-doctor/references/detection.md new file mode 100644 index 00000000..8fc5a3c3 --- /dev/null +++ b/.agents/skills/monorepo-doctor/references/detection.md @@ -0,0 +1,165 @@ +# Phase 1 — Detection + +Goal: build an inventory at `$W/workspace.json`. Every later phase reads it. + +Requires Node ≥ 22.5 because this phase uses `fs.globSync` and `path.matchesGlob`. The snippet below exits 2 with `monorepo-doctor requires Node >= 22.5` when either API is missing. +This phase resets the work directory. In every shell, start with `W="$(node -p 'require("os").tmpdir()')/monorepo-doctor"`; all later phases reuse `$W`, and you should recompute it in each new shell. `os.tmpdir()` honors `TMPDIR`: if the default temp dir is not writable in your environment, `export TMPDIR=` in every shell before computing `$W`, so the shell and the Node snippets agree. + +Run from the workspace root (the directory the user pointed at; default: current directory): + +```bash +W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" +mkdir -p "$W" +cat > "$W/detect.mjs" <<'EOF' +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +if (typeof fs.globSync !== 'function' || typeof path.matchesGlob !== 'function') { + console.error('monorepo-doctor requires Node >= 22.5'); + process.exit(2); +} +const root = path.resolve(process.argv[2] ?? '.'); +const work = path.join(os.tmpdir(), 'monorepo-doctor'); +fs.rmSync(work, { recursive: true, force: true }); +fs.mkdirSync(path.join(work, 'findings'), { recursive: true }); +const toPosix = (p) => p.split(path.sep).join('/'); +const readJson = (p) => { try { return JSON.parse(fs.readFileSync(p, 'utf8')); } catch { return null; } }; +const exists = (p) => fs.existsSync(path.join(root, p)); + +const pm = exists('pnpm-lock.yaml') || exists('pnpm-workspace.yaml') ? 'pnpm' + : exists('bun.lockb') || exists('bun.lock') ? 'bun' + : exists('yarn.lock') ? 'yarn' + : exists('package-lock.json') ? 'npm' : 'unknown'; + +const rootPkg = readJson(path.join(root, 'package.json')) ?? {}; +const stripYamlComment = (s) => { + let out = '', quote = null; + for (let i = 0; i < s.length; i++) { + const ch = s[i]; + if (quote) { if (ch === quote && s[i - 1] !== '\\') quote = null; out += ch; } + else if (ch === '"' || ch === "'") { quote = ch; out += ch; } + else if (ch === '#') break; + else out += ch; + } + return out.trim(); +}; +const unquote = (s) => stripYamlComment(s).trim().replace(/^['"]|['"]$/g, '').trim(); +const splitFlow = (s) => { + const out = []; + let cur = '', quote = null; + for (let i = 0; i < s.length; i++) { + const ch = s[i]; + if (quote) { if (ch === quote && s[i - 1] !== '\\') quote = null; cur += ch; } + else if (ch === '"' || ch === "'") { quote = ch; cur += ch; } + else if (ch === ',') { const v = unquote(cur); if (v) out.push(v); cur = ''; } + else cur += ch; + } + const v = unquote(cur); if (v) out.push(v); + return out; +}; +function pnpmWorkspaceGlobs(file) { + const lines = fs.readFileSync(file, 'utf8').split(/\r?\n/); + const globs = []; + for (let i = 0; i < lines.length; i++) { + const m = lines[i].match(/^packages:\s*(.*)$/); + if (!m) continue; + const rest = stripYamlComment(m[1]); + if (rest.startsWith('[') && rest.endsWith(']')) globs.push(...splitFlow(rest.slice(1, -1))); + for (let j = i + 1; j < lines.length; j++) { + const line = lines[j]; + if (!line.trim() || line.trimStart().startsWith('#')) continue; + const item = line.match(/^\s*-\s*(.+)$/); + if (item) { const v = unquote(item[1]); if (v) globs.push(v); continue; } + if (/^\S/.test(line)) break; + } + break; + } + return globs; +} +let globs = []; +if (exists('pnpm-workspace.yaml')) { + globs = pnpmWorkspaceGlobs(path.join(root, 'pnpm-workspace.yaml')); + if (!globs.length) console.error('WARN: pnpm-workspace.yaml contains no package globs'); +} +if (!globs.length) globs = Array.isArray(rootPkg.workspaces) ? rootPkg.workspaces : rootPkg.workspaces?.packages ?? []; +const include = globs.filter((g) => !g.startsWith('!')); +const exclude = globs.filter((g) => g.startsWith('!')).map((g) => g.slice(1).replace(/\/$/, '')); +const isExcluded = (dir) => exclude.some((g) => path.matchesGlob(dir, g) || path.matchesGlob(`${dir}/package.json`, `${g}/package.json`)); + +const SKIP_NAMES = new Set(['node_modules', 'dist', 'build', '.git', '.nx', '.turbo', 'coverage']); +const hasSkippedPart = (p) => p.split('/').some((part) => SKIP_NAMES.has(part)); +const prune = (p) => SKIP_NAMES.has(path.basename(String(p))); + +const orchestrators = []; +if (exists('nx.json')) orchestrators.push({ name: 'nx', installed: exists('node_modules/.bin/nx') }); +if (exists('turbo.json')) orchestrators.push({ name: 'turbo', installed: exists('node_modules/.bin/turbo') }); + +const dirs = new Map(); +for (const g of include) { + for (const raw of fs.globSync(`${g.replace(/\/$/, '')}/package.json`, { cwd: root, exclude: prune })) { + const m = toPosix(raw); + const d = path.posix.dirname(m); + if (!hasSkippedPart(m) && !isExcluded(d)) dirs.set(d, 'package.json'); + } +} +if (orchestrators.some((o) => o.name === 'nx')) { + for (const raw of fs.globSync('**/project.json', { cwd: root, exclude: prune })) { + const m = toPosix(raw); + if (hasSkippedPart(m)) continue; + const d = path.posix.dirname(m); + if (d !== '.' && !dirs.has(d)) dirs.set(d, 'project.json'); + } +} + +const packages = []; +for (const [dir, source] of dirs) { + const abs = path.join(root, dir); + const pkg = readJson(path.join(abs, 'package.json')) ?? {}; + const proj = readJson(path.join(abs, 'project.json')); + const name = pkg.name ?? proj?.name ?? dir; + const exportKeys = typeof pkg.exports === 'string' ? ['.'] + : pkg.exports && typeof pkg.exports === 'object' ? Object.entries(pkg.exports).filter(([k, v]) => k.startsWith('.') && v !== null).map(([k]) => k) : []; + const allDeps = { ...pkg.peerDependencies, ...pkg.optionalDependencies, ...pkg.devDependencies, ...pkg.dependencies }; + packages.push({ + name, dir: toPosix(dir), source, + private: pkg.private === true, version: pkg.version ?? null, + scripts: pkg.scripts ?? {}, files: pkg.files ?? null, main: pkg.main ?? null, + exports: pkg.exports ?? null, exportKeys, + dependencies: pkg.dependencies ?? {}, devDependencies: pkg.devDependencies ?? {}, allDeps, + tags: proj?.tags ?? pkg.nx?.tags ?? [], nxTargets: { ...pkg.nx?.targets, ...proj?.targets }, nxName: proj?.name ?? null, + }); +} +const names = new Set(packages.map((p) => p.name)); +for (const p of packages) { + p.workspaceDeps = Object.keys(p.allDeps).filter((d) => names.has(d)); + p.hasTestScript = typeof p.scripts.test === 'string' && !/no test specified/.test(p.scripts.test); +} + +const ci = [ + ...fs.globSync('.github/workflows/*.{yml,yaml}', { cwd: root, exclude: prune }).map(toPosix), + ...fs.globSync('.buildkite/*.{yml,yaml}', { cwd: root, exclude: prune }).map(toPosix), + ...['.gitlab-ci.yml', 'azure-pipelines.yml', '.circleci/config.yml', 'bitbucket-pipelines.yml', 'Jenkinsfile', '.travis.yml'].filter(exists), +]; + +const ws = { + root, packageManager: pm, orchestrators, workspaceGlobs: globs, + rootScripts: rootPkg.scripts ?? {}, + hasNodeModules: exists('node_modules'), + ciFiles: ci.map(toPosix), + packages, +}; +fs.writeFileSync(path.join(work, 'workspace.json'), JSON.stringify(ws, null, 2)); +if (!packages.length) { console.error('NO_WORKSPACE: no workspace packages found under ' + root); process.exit(2); } +console.log(JSON.stringify({ root, packageManager: pm, orchestrators, packages: packages.length, ciFiles: ws.ciFiles.length })); +EOF +node "$W/detect.mjs" "" +``` + +## Interpreting the result + +- Exit code 2 / `NO_WORKSPACE`: this is not a monorepo. Stop and tell the user; do not produce a report. +- `orchestrators[].installed: false`: never call `npx nx` / `npx turbo` (it would download a different version). Use static config analysis only, and mark dependent sub-steps `partial`. +- `hasNodeModules: false`: skip `knip` and any step that needs resolved dependencies; mark `partial`. +- `packages[].source === "project.json"`: an Nx project without its own package.json — package.json-based rules (missing test script, undeclared dependency, npm pack) do not apply to it. +- Print a one-line summary of the inventory to the user before continuing. +- `ciFiles` contains the top-level CI files for GitHub Actions, GitLab CI, Azure Pipelines, CircleCI, Bitbucket Pipelines, Jenkins, Buildkite, and Travis. The CI coverage check follows local GitLab `include:` entries and local GitHub reusable workflows referenced from these files. diff --git a/.agents/skills/monorepo-doctor/references/prod-leakage.md b/.agents/skills/monorepo-doctor/references/prod-leakage.md new file mode 100644 index 00000000..5a4c6345 --- /dev/null +++ b/.agents/skills/monorepo-doctor/references/prod-leakage.md @@ -0,0 +1,43 @@ +# Check 5 — Test code in production + +Question: do test files, fixtures or test libraries end up in a published package or a deployable build? +Output: `findings/prod-leakage.json`. + +Test files are `scan.packages[p].testFiles` (the scan classifies `*.spec.*`, `*.test.*`, and files under `__tests__/`, `__mocks__/`, `test/`, `tests/`, `e2e/`, or `cypress/`) plus fixture directories (`fixtures?/`) when checking package tarballs and build outputs. +Test libraries: `vitest`, `jest`, `@jest/*`, `mocha`, `chai`, `sinon`, `@testing-library/*`, `cypress`, `playwright`, `@playwright/test`, `msw`, `nock`, `supertest`, `ts-jest`, `@vitest/*`. + +## Rules + +| id | severity | detect | +| -------------------------------------------- | ------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `prod-leakage/test-files-in-package` | critical | publishable package (`private !== true`) whose tarball contains test files. | +| `prod-leakage/test-files-in-build-output` | critical | a package's existing output dir contains compiled test files (`*.spec.js`, `*.test.js`, `*.spec.d.ts`, …). Only treat a dir as build output when it is the tsconfig `outDir`, referenced by `main`/`exports`/`files`, or gitignored, **and** the package has a build script or Nx build target. Only check dirs that exist — never build. Evidence should note the output may be stale. | +| `prod-leakage/build-tsconfig-includes-tests` | high | the build tsconfig `include`s the test files' location and its `exclude` doesn't match them. Build tsconfig = file passed via `-p`/`--project` in the `build` script, else Nx build target `options.tsConfig`, else `tsconfig.build.json`, else `tsconfig.lib.json`, else `tsconfig.json` when the build script is plain `tsc`. Skip packages with neither a build script nor an Nx build target. | +| `prod-leakage/test-lib-in-dependencies` | high (medium if `private`) | a test library appears in `dependencies` (not `devDependencies`). Consumers install it in production. | +| `prod-leakage/publint-issue` | error→high, warning→medium, suggestion→low | `publint` messages for publishable packages. | + +## Steps + +1. For each publishable package (non-private, `source: package.json`): + ```bash + W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" + (cd "" && npm pack --dry-run --json --ignore-scripts 2>&1) + ``` + Parse `[0].files[].path`; any match of the test pattern → `test-files-in-package` (list up to 10 in `evidence`). + Parse stdout as JSON. If `npm pack` fails (e.g. `workspace:` protocol under npm), capture the first stderr/stdout line, then fall back to reasoning: if the package has no `files` field and no `.npmignore`, every file not in `.gitignore` ships — compare against `scan.packages[p].testFiles`. If `.npmignore` exists without `files`, honor its simple ignore/negation patterns where possible and state that this is a fallback. If `files` is present, test files ship only if a `files` entry matches them. If `publishConfig.directory` is set, evaluate that directory as the publish root. State in `evidence` that the fallback was used and include the first failure line. +2. Check existing build output dirs for compiled test files. +3. Resolve each package's build tsconfig and check `include`/`exclude` against `scan.packages[p].testFiles`. Treat `include: ["src"]` as `src/**/*`. Follow `extends` one level for `include`/`exclude` when the file itself has none. +4. Check `dependencies` against the test-library list. +5. If `hasNodeModules`, for each publishable package that has its build output present: + ```bash + W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" + npm_config_ignore_scripts=true npx --yes publint@0.3.25 "" 2>&1 + ``` + publint prints sections `Errors:`, `Warnings:`, `Suggestions:` followed by numbered messages. One finding per message. Skip when `node_modules` is missing. Skip unbuilt packages and set status `partial` with reason `publint skipped for unbuilt packages: …`. +6. Write `findings/prod-leakage.json`. + +## Fix guidance + +- test-files-in-package: add `"files": ["dist"]` to package.json (preferred over `.npmignore`). +- build-tsconfig-includes-tests: `"exclude": ["**/*.spec.ts", "**/*.test.ts", "**/__tests__/**"]`. +- test-lib-in-dependencies: move it to `devDependencies`. diff --git a/.agents/skills/monorepo-doctor/references/report-format.md b/.agents/skills/monorepo-doctor/references/report-format.md new file mode 100644 index 00000000..0c5faf05 --- /dev/null +++ b/.agents/skills/monorepo-doctor/references/report-format.md @@ -0,0 +1,142 @@ +# Report format + +Every check writes one file: `$W/findings/.json` +where `` is one of `boundaries`, `circular-deps`, `caching`, `ci-coverage`, `prod-leakage`. + +```json +{ + "status": "ok | partial | skipped | not-run", + "reason": "required unless status is ok", + "findings": [ + { + "id": "caching/missing-outputs", + "severity": "critical | high | medium | low", + "title": "One line, specific: name the package/task", + "location": { "file": "turbo.json", "line": 4, "package": "@acme/ui" }, + "evidence": "The exact config/code/command output that proves it", + "fix": "Concrete change, ideally a snippet" + } + ] +} +``` + +- `ok`: check ran fully. `partial`: some sub-steps skipped (say which in `reason`). + `skipped`: check could not run at all. `not-run`: user scoped it out. A missing findings file is rendered as `skipped` with reason `no findings file`; for user-scoped-out checks, write an explicit `not-run` file at `$W/findings/.json`. +- `location.file` is relative to the workspace root. Include `package` whenever the finding belongs to one package. +- Never emit a finding without `evidence`. If you are unsure, lower severity and say so in `evidence`; do not guess. +- Use only the ids defined in the check reference files. + +## Severity rubric + +| Severity | Meaning | +| -------- | ------------------------------------------------------------------ | +| critical | Wrong code can ship or tests are silently skipped for real changes | +| high | Likely incorrect builds/caching or broken CI signal | +| medium | Correctness risk under specific conditions, or a boundary erosion | +| low | Hygiene, performance, or maintainability | + +Severity may be lowered one level with a stated reason (e.g., package is private). Never raise it. + +## Render + +After all checks, run this snippet. It validates every findings file and fails loudly (`stderr`, exit 1) if a status is outside the enum, `reason` is missing for a non-`ok` status, a finding id does not start with `/`, or evidence is missing/empty. Arguments: output directory (default: workspace root). + +```bash +W="$(node -p 'require("os").tmpdir()')/monorepo-doctor" +cat > "$W/render.mjs" <<'EOF' +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +const work = path.join(os.tmpdir(), 'monorepo-doctor'); +const ws = JSON.parse(fs.readFileSync(path.join(work, 'workspace.json'), 'utf8')); +const outDir = path.resolve(process.argv[2] ?? ws.root); +fs.mkdirSync(outDir, { recursive: true }); +const CHECKS = ['boundaries', 'circular-deps', 'caching', 'ci-coverage', 'prod-leakage']; +const STATUS = ['ok', 'partial', 'skipped', 'not-run']; +const SEV = ['critical', 'high', 'medium', 'low']; +const checks = [], findings = [], problems = []; +for (const check of CHECKS) { + const p = path.join(work, 'findings', `${check}.json`); + let r; + if (fs.existsSync(p)) { + try { r = JSON.parse(fs.readFileSync(p, 'utf8')); } + catch (e) { problems.push(`${check}: invalid JSON: ${e.message}`); r = { status: 'skipped', reason: 'invalid findings file', findings: [] }; } + } else { + r = { status: 'skipped', reason: 'no findings file', findings: [] }; + } + if (!STATUS.includes(r.status)) problems.push(`${check}: invalid status ${JSON.stringify(r.status)}`); + if (r.status !== 'ok' && !String(r.reason ?? '').trim()) problems.push(`${check}: reason is required when status is ${r.status}`); + const list = Array.isArray(r.findings) ? r.findings : (problems.push(`${check}: findings must be an array`), []); + checks.push({ check, status: r.status, reason: r.reason, findingCount: list.length }); + for (const f of list) { + if (!SEV.includes(f.severity)) problems.push(`${check}: unknown severity ${JSON.stringify(f.severity)}${f.id ? ` (${f.id})` : ''}`); + if (typeof f.id !== 'string' || !f.id.startsWith(`${check}/`)) problems.push(`${check}: finding id must start with ${check}/ (${f.id ?? 'missing'})`); + if (!String(f.evidence ?? '').trim()) problems.push(`${check}: finding ${f.id ?? '?'} missing evidence`); + findings.push({ ...f, check }); + } +} +if (problems.length) { + for (const problem of problems) console.error(`ERROR: ${problem}`); + process.exit(1); +} +const safe = (s) => String(s ?? ''); +findings.sort((a, b) => SEV.indexOf(a.severity) - SEV.indexOf(b.severity) + || safe(a.id).localeCompare(safe(b.id)) + || safe(a.location?.file).localeCompare(safe(b.location?.file)) + || ((a.location?.line ?? 0) - (b.location?.line ?? 0))); +const summary = Object.fromEntries(SEV.map((s) => [s, findings.filter((f) => f.severity === s).length])); +const report = { + version: 1, + generatedAt: new Date().toISOString(), + workspace: { root: ws.root, packageManager: ws.packageManager, orchestrators: ws.orchestrators, packageCount: ws.packages.length }, + summary, checks, findings, +}; +fs.writeFileSync(path.join(outDir, 'monorepo-doctor-report.json'), JSON.stringify(report, null, 2) + '\n'); +const cell = (v) => safe(v).replace(/\|/g, '\\|').replace(/\r?\n/g, '
'); +const loc = (l) => l?.file ? `\`${l.file}${l.line ? `:${l.line}` : ''}\`${l.package ? ` (${l.package})` : ''}` : '—'; +const fenceFor = (s) => '`'.repeat(Math.max(3, ...[...safe(s).matchAll(/`+/g)].map((m) => m[0].length + 1))); +const md = [ + '# Monorepo Doctor Report', '', + `**Workspace:** ${ws.packageManager} · ${ws.orchestrators.map((o) => o.name).join(', ') || 'no orchestrator'} · ${ws.packages.length} packages`, '', + `**Summary:** ${SEV.map((s) => `${summary[s]} ${s}`).join(' · ')}`, '', + '| Check | Status | Findings | Note |', '|---|---|---|---|', + ...checks.map((c) => `| ${cell(c.check)} | ${cell(c.status)} | ${c.findingCount} | ${cell(c.reason ?? '')} |`), '', +]; +for (const s of SEV) { + const fs_ = findings.filter((f) => f.severity === s); + if (!fs_.length) continue; + md.push(`## ${s[0].toUpperCase() + s.slice(1)}`, ''); + for (const f of fs_) { + const fence = fenceFor(f.evidence); + md.push(`### ${cell(f.title)}`, '', `- **id:** ${f.id ? `\`${f.id}\`` : '—'}`, `- **where:** ${loc(f.location)}`, '', '**Evidence**', '', fence, safe(f.evidence), fence, '', `**Fix:** ${safe(f.fix) || '—'}`, ''); + } +} +if (!findings.length) md.push('No findings. 🎉', ''); +fs.writeFileSync(path.join(outDir, 'monorepo-doctor-report.md'), md.join('\n')); +console.log(JSON.stringify({ outDir, summary })); +EOF +node "$W/render.mjs" "" # replace with a literal path; omit it to write to the workspace root +``` + +## CI mode + +When the user asks for CI mode (or a fail threshold), write the reports first. In hosted CI, the agent step itself may not be able to fail the job reliably, so document/run this threshold check as a separate workflow step against the report file: + +```bash +node -e ' +const fs = require("node:fs"); +const path = require("node:path"); +const order = ["low", "medium", "high", "critical"]; +const reportPath = process.argv[1]; +const threshold = process.argv[2] || "high"; +const min = order.indexOf(threshold); +if (!reportPath || min === -1) { + console.error("usage: node -e