audit-architecture

# audit-architecture Whole-tree non-deterministic audit. Where `/review` checks the diff, this checks every file in the repository for slow drift accumulated across many small changes. ## When to invoke - User `/audit-architecture`. - Scheduled drift sweep (weekly). - Before milestones / version bumps. - After large refactors. ## How it runs 1. Determine the repository root from the current working directory: `git rev-parse --show-toplevel` (or use the cwd if not a git repo). All paths in the prompts are repo-relative — every subagent inherits the cwd. 2. Spawn **eight** read-only subagents **in parallel** in a single message (multiple `Agent` tool calls in one block). Each is `subagent_type=Explore`. Each prompt below is self-contained — copy verbatim. Subagents have no conversation context. 3. After all eight return, build the aggregated report (format at the bottom). --- ## Subagent 1 — Layering, import direction & package boundary > Audit `packages/client/src/` and `packages/cli/src/` in the current repository for layering violations. Use ripgrep / file reads — read-only, no edits. > > Skip `*.test.ts` files entirely — per CLAUDE.md, "Layering & boundary rules are declared per package and apply to production source only; `*.test.ts` files are exempt and may import across layers." Audit only production `.ts` files. > > This is a bun workspace: `packages/client` (`@metabase/client`) is the private Metabase API client, `packages/cli` (`@metabase/cli`) is the publishable CLI built on it. > > **Package boundary (hardest rule):** > > - Nothing under `packages/client/src/**` may import from `packages/cli` — not by an `@metabase/cli` specifier, and not by a relative path that resolves outside the client's own src tree (`../../cli/src/...` reads as ordinary navigation but lands in the other package). > - The client's dependency budget is `zod` and `node:` builtins. Root devDependencies hoist, so any third-party import resolves at runtime — report every production client file importing a package outside that budget. > - `packages/cli/src/**` consumes the client by specifier: `@metabase/client/<path>` (e.g. `@metabase/client/domain/card`, `@metabase/client/json`, `@metabase/client/errors`). A relative path from the CLI into `packages/client/src/` is a violation. > > **Within `packages/client/src/`:** > > - `domain/**` may import only from `zod` and other `domain/**` files. > - `resources/**` — one file per Metabase resource, holding every `/api/` path the client sends — may be imported only by `client.ts` and by sibling `resources/**` files. A `domain/**`, `http/**`, `auth/**`, `version/**` or root-level client file importing a resource is a violation; the direction runs one way, and the permitted importers are an allowlist, so a directory added later is not exempt for having arrived after the rule. > - Everything else may import freely within the client, subject to the boundary rules above. > > **Within `packages/cli/src/`:** > > - `commands/**` may import from any other CLI location and any `@metabase/client/*` path. It reaches Metabase only as `client.<resource>.<method>(…)` — a command naming an `/api/…` path literal or calling `requestParsed` / `requestRaw` / `requestStream` / `paginatePages` is a violation. Both rules are absolute and admit no exception, so report every offending file. Report separately any command that reaches the network by another name: `commands/auth/login.ts`'s `tryDiscoverMetadata` is the one sanctioned case, and no pattern rule can see a second one. > - `core/**` may import from `runtime/**`, other `core/**`, and `@metabase/client/*`. Never from `commands/` or `output/`. > - `output/**` may import from `runtime/**`, `core/**` except `core/auth/`, and any `@metabase/client` path **except `@metabase/client/http`** and anything beneath it (presentation consumes the error taxonomy from `@metabase/client/errors`, never the HTTP boundary). Never from `commands/`. > - `runtime/**` may import from `@metabase/client/*` (typed errors are the project's pattern: `runtime/body.ts` uses `parseJson` and `ConfigError`, `runtime/yaml.ts` uses `ValidationError`). It must not import from `commands/`, `core/`, or `output/`. > > For every violation, report `path:line: <import-stmt> — violates <rule>`. If zero violations, output the single line `(clean)`. --- ## Subagent 2 — Type strictness > Audit `packages/client/src/`, `packages/cli/src/`, and `tests/` in the current repository for type-strictness violations. Read-only. > > Patterns to find: > > - `as <Type>`, `as unknown as`, `as never`, `as any` (any cast form). > - `: any` or `<any>` annotations. > - `Record<string, unknown>` used as a typed value (the sole sanctioned home is `packages/cli/src/output/projection.ts` — flag every other instance and let the aggregator decide). > - `!` non-null assertions: `\.\.\.?!\.`, `\.\.\.?!\[`, `! ;`. > - `// @ts-ignore`, `// @ts-nocheck`, `// @ts-expect-error` (allowed only when paired with `// reason:` and `// FIXME(date)`). > > For each, report `path:line: <pattern>: <snippet>`. If zero violations, output `(clean)`. --- ## Subagent 3 — Boundary & process discipline > Audit `packages/client/src/` and `packages/cli/src/` in the current repository for boundary violations. Read-only. Skip `*.test.ts` files entirely (production source only — tests are exempt). > > The permitted homes are declared **per package** in CLAUDE.md — a pattern's home in one package says nothing about the other. Judge each file against the table for the package it lives in. All paths below are relative to that package's `src/`. > > The client — every file under `packages/client/src/`: > > | Pattern | Allowed only in | > | ---------------------------------------------------------------------------------------------- | --------------------------------------- | > | bare `fetch(`, `globalThis.fetch`, third-party HTTP libs (`got`/`axios`/`node-fetch`/`undici`) | `http/**` | > | `new URL(` | `http/**`, `url.ts` | > | `JSON.parse(` | `json.ts` | > | `setTimeout` inside an `await` wait loop | `poll.ts`, `http/retry.ts` | > | `process.stdout.write`, `process.stderr.write` | nowhere — the client owns no output | > | `process.exit(` | nowhere — the client never ends the run | > | `@clack/prompts`, `@clack/core` | nowhere — prompting is the CLI's job | > | `fs`, `fs/promises`, `node:fs`, `node:fs/promises` | nowhere — the client touches no files | > | `child_process`, `node:child_process` | nowhere — the client spawns nothing | > | an import specifier ending in `.ts` | nowhere — imports are extensionless | > > The CLI — every file under `packages/cli/src/`: > > | Pattern | Allowed only in | > | ---------------------------------------------------------------------- | --------------------------------------------------------------------------- | > | `process.exit(` | `cli.ts` | > | `process.stderr.write` | `cli.ts`, `output/**` | > | `process.stdout.write` | `output/**` | > | `JSON.parse(` | nowhere — use `parseJson` from `@metabase/client/json` | > | `new URL(` | nowhere — use `@metabase/client/url` | > | bare `fetch(`, `globalThis.fetch`, third-party HTTP libs | `core/npm-registry.ts` | > | `setTimeout` inside an `await` wait loop | nowhere — use `pollUntil` from `@metabase/client/poll` | > | `child_process` (`spawn`, `exec`, etc.) | `runtime/process.ts` | > | `@clack/prompts`, `@clack/core` | `output/prompt.ts` | > | `process.env["MB_*"]`, `process.env["METABASE_*"]` | `core/env.ts` — a CLAUDE.md rule, not walker-enforced; judge it by reading | > | an import specifier ending in `.ts` | nowhere — imports are extensionless | > | an `/api/…` path literal (`"/api/` or `` `/api/ ``) | nowhere under `commands/` — endpoints are the client's `resources/` layer | > | `.requestParsed(`, `.requestRaw(`, `.requestStream(`, `paginatePages(` | nowhere under `commands/` — a command calls `client.<resource>.<method>(…)` | > > The last two are absolute: neither has a sanctioned home anywhere in the CLI. Report every offending file. > > Two rules bind both packages but come from elsewhere, so apply them repo-wide rather than per-package: > > - `console.{log,warn,error,info,debug}` — forbidden everywhere, including `output/**` and `tests/`. `.oxlintrc.json` sets `no-console: error` with no overrides. Report every occurrence. > - Platform env vars (`XDG_CONFIG_HOME`, `APPDATA`, `npm_config_prefix`) are read where they are used — `core/paths.ts`, `core/install-method.ts`, `core/auth/temp-config-home.ts`. `readEnv` owns the `MB_`/`METABASE_` family only; do not report the others. > > For each violation, report `path:line: <rule>: <snippet>`. If zero violations, output `(clean)`. --- ## Subagent 4 — Test parity > Audit pairing between the two packages' source and their tests in the current repository. Read-only. > > Test layout: this project uses two tiers — colocated unit tests at `packages/<p>/src/**/*.test.ts` (beside the code, inside the package that owns it) and end-to-end at the repo root under `tests/e2e/<noun>.e2e.test.ts` (workspace-wide, driving the built `packages/cli/dist/cli.mjs`). There are no `tests/integration/` or `tests/smoke/` directories, and neither package has its own e2e tier. > > Required pairings: > > - For every leaf command at `packages/cli/src/commands/<noun>/<verb>.ts`: a `tests/e2e/<noun>.e2e.test.ts` exists and exercises that subcommand via `runCli`. Adding a new command without an e2e test is a missing pairing. > - For every leaf command at `packages/cli/src/commands/<noun>/<verb>.ts`: the literal `ALL_COMMANDS` list in `packages/cli/src/runtime/command-help.test.ts` must include `"<noun> <verb>"`. > - For every `packages/client/src/domain/<r>.ts`: **no** fixture-and-parse-test pair is required (a unit test of the form `Schema.parse(fixture).toEqual(fixture)` is a tautology). The schema's contract is tested by the e2e tier when a command consuming it runs against the live API. Report a missing pairing only if no command consumes the schema AND no e2e test parses through it. > - For every `packages/client/src/resources/<r>.ts`: a `packages/client/src/resources/<r>.test.ts` beside it, covering every exported method with a wire assertion over `capture.calls` (URL, verb, headers, body) built on `captureFetch` from `packages/client/src/testing/fetch-capture.ts`. Report a method the wire test never sends — nothing else pins its path or query vocabulary at the unit tier. > - For every pure helper file in `packages/client/src/**` and in `packages/cli/src/{core,output,runtime}/**` (excluding `index.ts` re-exports, the test doubles `packages/client/src/testing/fake-client.ts`, `packages/client/src/testing/fetch-capture.ts` and `packages/cli/src/core/auth/keyring-mock.ts`, and types-only files): a colocated unit test (`<name>.test.ts` next to `<name>.ts`, in the same package) exercises it. > - For every helper with infinite input space (URL parsing, JSON parsing, projection, byte capping, polling): a property test using `fast-check` should exist. > > For each missing pairing, report `<src-path> → expected <test-path>`. If all pairings are present, output `(clean)`. ---