diff --git a/.github/workflows/registry-audit.yml b/.github/workflows/registry-audit.yml new file mode 100644 index 00000000..f16fa787 --- /dev/null +++ b/.github/workflows/registry-audit.yml @@ -0,0 +1,67 @@ +# Asks npm what it actually holds, and opens an issue when it disagrees with this tree. +# +# NOT a step of `x verify`, and it must never become one. The gate is the shippability contract and +# runs on free runners; a step that resolves the network makes green depend on something the runner +# does not control. More to the point, a gate step could not do this job anyway: the three states +# this catches all appear BETWEEN commits, not at one. +# +# The state it exists for: a package added after a release run is unpublished until a human +# bootstraps it, and nothing says so until the next release reaches it and aborts with everything +# ahead of it already published irreversibly. `@ultimat3/flags` sat that way until 2.0.0 and +# `@ultimat3/scraping` until 2026-08-19, each while the docs described the other. + +name: registry-audit + +on: + schedule: + # Daily, well away from the hour a release is likely to be cut by hand. + - cron: '17 6 * * *' + workflow_dispatch: + +# Read-only: it resolves npm and may open an issue. It never publishes, and it holds no +# `id-token`, so it cannot be a path to the registry even if it is compromised. +permissions: + contents: read + issues: write + +concurrency: + group: registry-audit + cancel-in-progress: true + +jobs: + audit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: ./.github/actions/setup + + - name: audit the registry + id: audit + # `pipefail` is load-bearing: without it `tee` decides the step's exit status, the audit's + # non-zero is discarded, `outcome` is always `success`, and every step below that keys on + # `failure` never runs — a workflow that reports clean whatever npm says. + shell: bash + run: | + set -o pipefail + bun run scripts/registry-audit.ts --json | tee audit.json + continue-on-error: true + + - name: open an issue when the registry disagrees + if: steps.audit.outcome == 'failure' + env: + GH_TOKEN: ${{ github.token }} + # One open issue at a time: a daily job that files a duplicate every morning trains everyone + # to ignore it, which is the failure mode this audit exists to correct. + run: | + existing="$(gh issue list --label registry-drift --state open --limit 1 --json number --jq '.[0].number // empty')" + summary="$(jq -r '.summary' audit.json)" + body="$(jq -r '"**" + .summary + "**\n\n" + ([.findings[] | "- `" + .code + "` " + (.at // "") + " — " + .cause + "\n - fix: `" + .fix + "`"] | join("\n"))' audit.json)" + if [ -n "$existing" ]; then + gh issue comment "$existing" --body "$body" + else + gh issue create --label registry-drift --title "registry: $summary" --body "$body" + fi + + - name: fail the run so the badge is honest + if: steps.audit.outcome == 'failure' + run: exit 1 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 327dcb05..a8e3b8f7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -14,8 +14,11 @@ name: release # # Two things must be TRUE outside this file for the gate below to be a gate, and both are named in # PUBLISHING.md as human steps: the `npm-publish` environment needs required reviewers configured in -# Settings → Environments, and `@ultimat3/flags` needs its first manual publish (the derived list -# below now includes it, and trusted publishing cannot bootstrap a package that does not exist yet). +# Settings → Environments, and every package in the derived list needs a trusted publisher plus its +# first manual publish. Trusted publishing cannot bootstrap a package that does not exist yet, so a +# package added AFTER a release run is unpublished until a human bootstraps it — and this run will +# reach it and abort with everything ahead of it already published irreversibly. +# `bun run scripts/registry-audit.ts` answers whether that is owed, before a release finds out. on: release: @@ -139,11 +142,12 @@ jobs: # imports it — and the list is DERIVED, never kept by hand. # # It used to be seven hand-written steps of `-w` flags, and `@ultimat3/flags` was on none of - # them. That is why the registry answers 404 for it while the other 28 are published: flags - # declares the same `publishConfig` as its siblings, every consumer resolves it through the - # workspace, and so nothing in this repo could notice. The missing entry was the symptom; a - # list that has to match a derived one is the defect, and adding one line would have fixed - # today and re-broken on the next package somebody adds. + # them, so the registry answered 404 for it while its siblings published: flags declares the + # same `publishConfig`, every consumer resolves it through the workspace, and nothing in this + # repo could notice. The missing entry was the symptom; a list that has to match a derived one + # is the defect, and adding one line would have fixed that day and re-broken on the next + # package somebody added — which is exactly what happened to `@ultimat3/scraping`, added after + # the 2.0.0 run and 404 until it was bootstrapped by hand on 2026-08-19. # # `scripts/list-workspaces.ts` reads the real package.json files rather than the tier table, # so a package that exists on disk cannot be invisible here — and `create-ultimate` lands last diff --git a/README.md b/README.md index 5bd92774..261045b8 100644 --- a/README.md +++ b/README.md @@ -17,17 +17,6 @@ -> **Status: 3.0.0 — repository, `v3.0.0` tag and npm all agree**, `As of 2026-08-19`. 29 `@ultimat3/*` packages plus the unscoped `create-ultimate` — 30 in all — are **versioned** in lockstep at 3.0.0 (one version, one commit, one tag) and **published** at 3.0.0, every one by [`release.yml`](.github/workflows/release.yml) over OIDC with a provenance attestation. `bunx create-ultimate myapp` gives you 3.0.0. **3.0.0 is a major**: [CHANGELOG.md](CHANGELOG.md)'s 3.0.0 section carries 10 entries marked `BREAKING —` from a five-agent bug sweep, and no codemod ships with them, so each is a manual edit its entry names ([Upgrading](https://github.com/developerz-ai/ultimate/wiki/Upgrading)). 2.0.0 was the first major and carried 33. Semver applies — a breaking change to a documented API needs a major. That is what the version number means: a stable API under semver, not a promise about your infrastructure. - -| Fact | Check it, never this table | -|---|---| -| what npm serves | `npm view @ultimat3/core version` | -| the tarball is attested, and by whom | `npm view @ultimat3/core@3.0.0 dist.attestations _npmUser` | -| the 30 names that move together | `bun run scripts/release-workflow.ts --json` | -| the repository is stamped at one version | `bun run scripts/release.ts --check 3.0.0` | - -Release history worth knowing: 1.0.0 was the manual bootstrap; 1.1.0 was the first release the workflow published over OIDC; **2.0.0 was hand-published** — no trusted publisher was attached for the OIDC exchange to verify against — so it is the one release whose tarballs carry no attestation; 3.0.0 is the first the workflow has published since 1.2.0. **No publication holes**: `@ultimat3/scraping` was the last one and it is closed, bootstrapped by hand at 2.0.0 (`npm publish --access public --provenance=false`), the one-time step every package needs before a trusted publisher can attach ([PUBLISHING.md](PUBLISHING.md)). - ## Built by agents, for agents, maintained by agents Nobody writes this code by hand anymore, and the framework is designed for that rather than diff --git a/bun.lock b/bun.lock index 4c71581e..542bf8a2 100644 --- a/bun.lock +++ b/bun.lock @@ -193,7 +193,7 @@ }, "packages/action": { "name": "@ultimat3/action", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/cache": "2.0.0", "@ultimat3/core": "2.0.0", @@ -204,7 +204,7 @@ }, "packages/admin": { "name": "@ultimat3/admin", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/action": "1.2.0", "@ultimat3/ai": "1.2.0", @@ -225,7 +225,7 @@ }, "packages/ai": { "name": "@ultimat3/ai", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/action": "2.0.0", "@ultimat3/cache": "2.0.0", @@ -240,7 +240,7 @@ }, "packages/auth": { "name": "@ultimat3/auth", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "1.2.0", "@ultimat3/db": "1.2.0", @@ -249,14 +249,14 @@ }, "packages/cache": { "name": "@ultimat3/cache", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "2.0.0", + "@ultimat3/core": "3.0.0", }, }, "packages/cli": { "name": "@ultimat3/cli", - "version": "2.0.0", + "version": "3.0.0", "bin": { "x": "./src/bin.ts", }, @@ -288,11 +288,11 @@ }, "packages/core": { "name": "@ultimat3/core", - "version": "2.0.0", + "version": "3.0.0", }, "packages/create-ultimate": { "name": "create-ultimate", - "version": "2.0.0", + "version": "3.0.0", "bin": { "create-ultimate": "./src/bin.ts", }, @@ -302,9 +302,9 @@ }, "packages/db": { "name": "@ultimat3/db", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "2.0.0", + "@ultimat3/core": "3.0.0", }, "peerDependencies": { "@electric-sql/pglite": ">=0.5.0", @@ -315,24 +315,24 @@ }, "packages/entity": { "name": "@ultimat3/entity", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "2.0.0", - "@ultimat3/db": "2.0.0", - "@ultimat3/schema": "2.0.0", - "@ultimat3/time": "2.0.0", + "@ultimat3/core": "3.0.0", + "@ultimat3/db": "3.0.0", + "@ultimat3/schema": "3.0.0", + "@ultimat3/time": "3.0.0", }, }, "packages/flags": { "name": "@ultimat3/flags", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "1.2.0", }, }, "packages/http": { "name": "@ultimat3/http", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "2.0.0", "@ultimat3/i18n": "2.0.0", @@ -342,35 +342,35 @@ }, "packages/i18n": { "name": "@ultimat3/i18n", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "2.0.0", + "@ultimat3/core": "3.0.0", }, }, "packages/jobs": { "name": "@ultimat3/jobs", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "2.0.0", - "@ultimat3/entity": "2.0.0", - "@ultimat3/schema": "2.0.0", - "@ultimat3/time": "2.0.0", + "@ultimat3/core": "3.0.0", + "@ultimat3/entity": "3.0.0", + "@ultimat3/schema": "3.0.0", + "@ultimat3/time": "3.0.0", }, }, "packages/mail": { "name": "@ultimat3/mail", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "1.2.0", - "@ultimat3/i18n": "1.2.0", - "@ultimat3/jobs": "1.2.0", - "@ultimat3/schema": "1.2.0", - "@ultimat3/time": "1.2.0", + "@ultimat3/core": "3.0.0", + "@ultimat3/i18n": "3.0.0", + "@ultimat3/jobs": "3.0.0", + "@ultimat3/schema": "3.0.0", + "@ultimat3/time": "3.0.0", }, }, "packages/manifest": { "name": "@ultimat3/manifest", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/action": "1.2.0", "@ultimat3/core": "1.2.0", @@ -381,7 +381,7 @@ }, "packages/mcp": { "name": "@ultimat3/mcp", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/action": "1.2.0", "@ultimat3/core": "1.2.0", @@ -394,7 +394,7 @@ }, "packages/money": { "name": "@ultimat3/money", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "2.0.0", "@ultimat3/schema": "2.0.0", @@ -402,21 +402,21 @@ }, "packages/policy": { "name": "@ultimat3/policy", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "2.0.0", + "@ultimat3/core": "3.0.0", }, }, "packages/pwa": { "name": "@ultimat3/pwa", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "1.2.0", }, }, "packages/query": { "name": "@ultimat3/query", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/cache": "1.2.0", "@ultimat3/core": "1.2.0", @@ -427,7 +427,7 @@ }, "packages/realtime": { "name": "@ultimat3/realtime", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "1.2.0", "@ultimat3/query": "1.2.0", @@ -436,7 +436,7 @@ }, "packages/render": { "name": "@ultimat3/render", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/cache": "1.2.0", "@ultimat3/core": "1.2.0", @@ -447,11 +447,11 @@ }, "packages/schema": { "name": "@ultimat3/schema", - "version": "2.0.0", + "version": "3.0.0", }, "packages/scraping": { "name": "@ultimat3/scraping", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "2.0.0", "@ultimat3/jobs": "2.0.0", @@ -461,41 +461,43 @@ }, "packages/seo": { "name": "@ultimat3/seo", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "1.2.0", }, }, "packages/storage": { "name": "@ultimat3/storage", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "2.0.0", }, }, "packages/testing": { "name": "@ultimat3/testing", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/cache": "1.2.0", - "@ultimat3/core": "1.2.0", - "@ultimat3/db": "1.2.0", - "@ultimat3/entity": "1.2.0", - "@ultimat3/jobs": "1.2.0", - "@ultimat3/mail": "1.2.0", - "@ultimat3/time": "1.2.0", + "@ultimat3/cache": "3.0.0", + "@ultimat3/core": "3.0.0", + "@ultimat3/db": "3.0.0", + "@ultimat3/entity": "3.0.0", + "@ultimat3/i18n": "3.0.0", + "@ultimat3/jobs": "3.0.0", + "@ultimat3/mail": "3.0.0", + "@ultimat3/policy": "3.0.0", + "@ultimat3/time": "3.0.0", }, }, "packages/time": { "name": "@ultimat3/time", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { - "@ultimat3/core": "2.0.0", + "@ultimat3/core": "3.0.0", }, }, "packages/ui": { "name": "@ultimat3/ui", - "version": "2.0.0", + "version": "3.0.0", "dependencies": { "@ultimat3/core": "1.2.0", "@ultimat3/i18n": "1.2.0", diff --git a/docs/architecture/13-topology-runtime.md b/docs/architecture/13-topology-runtime.md index 9e2e9ddd..4ca14fdc 100644 --- a/docs/architecture/13-topology-runtime.md +++ b/docs/architecture/13-topology-runtime.md @@ -181,7 +181,7 @@ Skew handling during a rolling deploy: | `ROLE=migrate` must exit 0 before new `web`/`sync` start | a `web` replica whose build ID does not match the applied migration reports not-ready | | Migrations are additive across one deploy | old and new code run simultaneously during a rollout; a destructive change needs two deploys | | Skew is observable | `x status --json` reports the build-ID distribution of connected clients | -| No forced reload without a grace period | except `x deploy --critical`, which sets a countdown deadline and saves in-flight state through the mutator queue | +| No forced reload without a grace period | no exception ships `As of 2026-08`. `x deploy --critical` records the intent in the plan JSON and nothing acts on it, and `updateSignal` — the function that would compute a forced deadline — has **no runtime caller anywhere in the repo**. The grace default is 6h, not 30m, and a forced deadline is `now`, not a countdown | ## Codes diff --git a/docs/idea/08-pwa-offline.md b/docs/idea/08-pwa-offline.md index 9f83ba62..270a7674 100644 --- a/docs/idea/08-pwa-offline.md +++ b/docs/idea/08-pwa-offline.md @@ -91,7 +91,7 @@ The client is not broken and the server is not broken; they disagree about which | 2 | **Client sends its build ID on every request** | `X-Ultimate-Build` header on RPC, query, and WS handshake. The server can answer "you are stale" instead of guessing | | 3 | **N-deploy asset retention** | old builds' assets stay served for N deploys (default 10) or a minimum window (default 7d), whichever is longer. A build-A chunk resolves after six deploys | | 4 | **`AppUpdateAvailable` signal, not a 404** | a Solid signal flips when the server reports a newer build. The app renders its own "Update available — reload" affordance. **No forced navigation, no lost form state, no dinosaur** | -| 5 | **Forced reload after a grace period** | security-flagged deploys (`x deploy --critical`) set a deadline. Client shows a countdown, saves in-flight state via the mutator queue, then reloads. Grace default 30m; a hard patch can set minutes | +| 5 | **Forced reload after a grace period** | **designed, not wired `As of 2026-08`.** The generated service worker posts only `{ type: 'AppUpdateAvailable', to: BUILD_ID }` on activation — no `from`, no `forced`, no `deadlineAt` — so even the *unforced* stale-build notification arrives without the fields a client would decide on. `updateSignal`, which computes all four, is exported and has **no runtime caller**; neither the stale-response nor the WebSocket notification path exists. `x deploy --critical` records the intent in the plan JSON and nothing reads it. Wiring it needs a caller passing `BUILD_ID_HEADER` into `updateSignal` and posting the full message, plus a per-release reason on the container. The grace default is **6h**, and a forced deadline is `now` — there is no framework-run countdown | | 6 | **Skew is observable** | `/_x` and `x status --json` report the build-ID distribution of connected clients, so "how many users are three deploys behind" has an answer | | 7 | **Build ID scopes the SW cache** | preview/branch builds get their own cache namespace and SW scope, so a preview can never poison prod caches ([`09-ai-first.md`](./09-ai-first.md)) | @@ -136,5 +136,5 @@ All of them are `route`/`action`/`job` primitives underneath ([`02-primitives.md - Never hand-edit `sw.js`. Change the route, rebuild. - Never cache an authenticated response without an explicit `offline` field on the route. - Never use a timestamp or `latest` as a build ID. -- Never force-reload a user without a grace period, except on a `--critical` deploy. +- Never force-reload a user without a grace period. The `--critical` exception is designed and not wired — see row 5. - `x verify` checks: precache budget, fallback presence, SW checksum, retention config, and that every `precache` route is actually prerenderable. diff --git a/framework.manifest.json b/framework.manifest.json index 60fe49cb..b18f5880 100644 --- a/framework.manifest.json +++ b/framework.manifest.json @@ -1,6 +1,6 @@ { "version": 1, - "buildId": "f47fe42062833be184261d0a52a957687aedc0e86567372bd1417ff2a07d4005", + "buildId": "27841743eb56d4eab25f194cb87cf188492844fc82821cca94743d13ad9ddf21", "tiers": { "0": [ "core", @@ -587,6 +587,11 @@ "owner": "cli", "at": "packages/cli/src/error-codes.ts" }, + { + "code": "X_CLI_FLAG_UNREAD", + "owner": "cli", + "at": "packages/cli/src/error-codes.ts" + }, { "code": "X_CLI_UNEXPECTED", "owner": "cli", @@ -1707,6 +1712,26 @@ "owner": "core", "at": "packages/core/src/error-codes.ts" }, + { + "code": "X_REGISTRY_BOOTSTRAP_OWED", + "owner": "scripts", + "at": "scripts/registry-audit.ts" + }, + { + "code": "X_REGISTRY_UNATTESTED", + "owner": "scripts", + "at": "scripts/registry-audit.ts" + }, + { + "code": "X_REGISTRY_UNREACHABLE", + "owner": "scripts", + "at": "scripts/registry-audit.ts" + }, + { + "code": "X_REGISTRY_VERSION_BEHIND", + "owner": "scripts", + "at": "scripts/registry-audit.ts" + }, { "code": "X_RELEASE_VERSION_SKEW", "owner": "cli", diff --git a/packages/action/src/contract-test.ts b/packages/action/src/contract-test.ts index d2241228..586f4428 100644 --- a/packages/action/src/contract-test.ts +++ b/packages/action/src/contract-test.ts @@ -75,7 +75,7 @@ export function contractTestsFor( if (document.paths[path] === undefined) { throw new ContractDriftError( `OpenAPI document has no entry for ${path}`, - 'x verify --contract', + 'x verify --json # the contract suite is a step of it', ); } }, diff --git a/packages/cli/CLAUDE.md b/packages/cli/CLAUDE.md index ba33b8ed..593d3f6b 100644 --- a/packages/cli/CLAUDE.md +++ b/packages/cli/CLAUDE.md @@ -117,7 +117,9 @@ a shallow spread keeps one of them. | File | Job | |---|---| -| `ts-scan.ts` | the strings a `fix:` can evaluate to, the `X_*` codes a file declares, and the ones it says it borrows | +| `ts-scan.ts` | the masking every scan shares, the `X_*` codes a file declares, and the ones it says it borrows | +| `fix-scan.ts` | the strings a `fix:` can evaluate to: under a key, at a factory's argument, at a class constructor's | +| `fix-imports.ts` | which of those factories a file can call that it did not declare — one relative specifier, one file read | | `error-contract.ts` | the rules, the two checks that turn them into findings, and `collectDeclaredCodes` | | `fix-command.ts` | resolving an `x ` a `fix:` cites against the registry | | `source-files.ts` | which files are shipped source — shared with `filesize`, never a second list | @@ -177,7 +179,7 @@ and the step says so rather than guessing. helper, so the file held no `fix:` at all and `scanFixes` returned `[]` for all of it — the citation resolver was never given a string to judge, and two stale `x db branch ` lines shipped through the hole. `scanFixes` now also reads the argument in the `fix: string` position of -a **local** helper, under four rules, each with its own case in `ts-scan.test.ts`: the helper must +a **local** helper, under four rules, each with its own case in `fix-scan.test.ts`: the helper must BUILD an error (`code` key or `new …Error(` in its body), or `citedCommandProblem(fix, catalog)` — which takes a fix to *judge* it — would have its call sites read as declarations; the parameter list may hold no rest or destructured parameter, because neither has a reliable position; the call @@ -186,12 +188,31 @@ because `prefix + 'x doctor'` reads as one literal there and publishing half a f publishing none. Measured over the whole tree: 16 files gained readable fixes, `readonly-sql.ts` went from 0 to 7, and **zero** new findings. -What it still cannot see is **cross-file**: `dbNotImplemented` is exported from `@ultimat3/db` and -called from `pglite-branch.ts`, and resolving that means an import graph and a per-symbol parameter -table. Same for an error class with a positional `constructor(cause, fix)` — `@ultimat3/render`'s -`errors.ts` has fourteen, and 15 of its codes have never had a fix line read. Measured: **zero** -same-file call sites for that form, so a constructor rule would be dead code today. Named, not -guessed at. +**And a fix does not always arrive in the file that declares its builder**, which is where four +bad `fix:` lines in `packages/ui/src/icons/build-icons.ts` shipped: `invalidIconDataError` is +declared in `packages/ui/src/errors.ts`, and a per-package `errors.ts` full of factories is the +house pattern, so the same-file rule left the most common shape of all unchecked. `fix-imports.ts` +resolves it — the specifier is relative, the candidate paths are `.ts{,x}` and +`/index.ts{,x}`, and the parameter position is the callee's. An alias is renamed to what the +CALLER writes; a local declaration of the same name wins, because that is the function the call +actually reaches. One module cache per run: `errors.ts` is imported by every file in its package. + +**An error CLASS is the same helper one keyword away**, and is now read too: the name is the +class's, the parameter list its `constructor`'s, `new X(…)` is a call like any other. It was +measured as dead code in the same-file rule — zero same-file call sites — and cross-file it is +`@ultimat3/render`'s fourteen classes plus `@ultimat3/core`'s three image ones. + +Measured over the whole tree, `As of 2026-08`: **791 → 877** fix literals read, 37 files gained +one, and **3 findings** the gate had never been able to see — `x verify --contract` and +`x build --route` (two flags no command declares) and one `check …` line with no command token. + +What it still cannot see is a builder imported from another **package**: `candidatePaths` refuses a +non-relative specifier, because resolving one means guessing which of 29 packages a bare name came +from and a wrong guess reads an unrelated function's argument as a fix. Measured: 3 call sites in +this repo, none of them a finding. It is **not** left silent — the step's `output` carries +`checked {n} fix line(s), could not read {m}`, counted at `FixScan.unreadable`: an argument in a +KNOWN fix position that is not one literal. Deliberately not "imports I could not open", which is +1504 names here and 1310 of them are `join` and `UltimateError` — a number nobody can act on. `cli → admin` is a declared sideways edge (`scripts/lib/tiers.ts`): `x dev` **mounts** the dashboard, it never grows a second one. The CLI's only contribution is the facts no registry @@ -646,6 +667,35 @@ locked by a process that no longer exists. Commands: `bun test`, `bunx tsc --noEmit -p tsconfig.json`. +## A declared flag with no reader is a promise `x help` makes and nothing keeps + +`x deploy --critical` said *"security deploy: forces clients to reload"* and forced nothing: the +value is written into the plan JSON (`cmd-deploy.ts`) and **no package reads that field**. The +parser accepts every declared flag, so this is neither a parse error nor a type error — the flag +worked perfectly and meant nothing, to the operator most likely to be shipping a security patch. + +`flag-reads.ts` is the rule that can see the class of defect: **every flag a command declares is +read by something in the CLI's own source**, as `X_CLI_FLAG_UNREAD`. The four global flags are +excluded — `--json`, `--help`, `--cwd` and `--verbose` are the parser's, read once for every +command, and a per-command rule would report all thirty declarations of `--json`. The read test is +deliberately generous: a bare `'name'` literal anywhere outside a `name:`/`short:` spec field +counts, so a flag echoed only into `--json`, or read through a shared constant, is read. A gate +that guessed at intent would report findings about working commands. + +It is enforced by `flag-reads.test.ts`, in the `unit` step — the same shape `cmd-planned.test.ts` +and `error-catalog.test.ts` use for a rule about the CLI's own declarations, and the reason its +`fix:` is a `bun test` line rather than an `x` command: the rule can only ever fire in this repo. +Promoting it to `x verify`'s `boundaries` host check is one line in `scripts/verify.ts`. + +**It does not catch `--critical`, and that is the honest limit.** The flag IS read — +`flagBool(ctx.args, 'critical')` — and what had no consumer was the plan FIELD, one level below any +rule over names. Two stronger rules were measured and rejected: "the read must not be a property +initializer" reports six flags, five of which work (`x db --allow-destructive`, `x jobs --queue`); +"the summary must match the behaviour" is undecidable. So the flag's summary now says what it does, +and forcing a reload stays what it always was — `@ultimat3/pwa`'s `updateSignal({ reason: +'security' })`, which `As of 2026-08` has **no runtime caller anywhere**, in that package or +outside it. Wiring the flag means giving that function a caller first. + ## Planned commands are commands Every command in `wiki/CLI-Reference.md`'s planned table is in the registry, built from diff --git a/packages/cli/src/cmd-deploy.ts b/packages/cli/src/cmd-deploy.ts index 3a8fd8df..580d6fdf 100644 --- a/packages/cli/src/cmd-deploy.ts +++ b/packages/cli/src/cmd-deploy.ts @@ -119,7 +119,16 @@ export const deployCommand: CliCommand = { { name: 'image', type: 'string', summary: 'image reference to deploy' }, { name: 'method', type: 'string', summary: 'compose | helm', default: 'compose' }, { name: 'dry-run', type: 'boolean', summary: 'print the plan, run nothing' }, - { name: 'critical', type: 'boolean', summary: 'security deploy: forces clients to reload' }, + // Says what it does, not what it was going to do. "forces clients to reload" was read by + // nothing: the flag lands in the plan JSON below and no package reads that field, so an + // operator shipping a security patch was told an outcome that did not occur. Forcing a + // reload is `@ultimat3/pwa`'s `updateSignal({ reason: 'security' })`, which as of 2026-08 + // has no runtime caller anywhere — wiring this flag to it is a change in that package. + { + name: 'critical', + type: 'boolean', + summary: 'record a security deploy in the plan (no client is forced to reload)', + }, ], }, async run(ctx: CommandContext): Promise { diff --git a/packages/cli/src/cmd-verify.test.ts b/packages/cli/src/cmd-verify.test.ts index 69a7ea94..6c97909b 100644 --- a/packages/cli/src/cmd-verify.test.ts +++ b/packages/cli/src/cmd-verify.test.ts @@ -326,6 +326,25 @@ describe('unit · x verify', () => { expect(result.steps?.[0]?.findings[0]?.code).toBe('X_BOUNDARY_VIOLATION'); }); + // A step that reports findings alone claims a completeness a parser-less scan does not have. + // The coverage line rides in `output`, which `--json` carries verbatim and `--verbose` prints. + test('the errors step reports what it read and what it could not', async () => { + const step = VERIFY_STEPS.find((candidate) => candidate.name === 'errors'); + const root = await mkdtemp(join(tmpdir(), 'x-errors-')); + try { + await Bun.write( + join(root, 'packages', 'db', 'src', 'errors.ts'), + "export const raise = (cause: string, fix: string) => new E({ code: 'X_A', cause, fix });\n" + + "raise('one', 'x db migrate --json');\nraise('two', computed);\n", + ); + const outcome = await step?.run({ ...ctx, root }); + expect(outcome?.ok).toBe(true); + expect(outcome?.output).toBe('checked 1 fix line(s), could not read 1'); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + // `openapi.json` is a published contract on its own — the typed client is generated from it — // so gating the step on `x.manifest.json` let a stale spec ship a wrong client unchecked. describe('contract-diff applies to either committed contract', () => { diff --git a/packages/cli/src/cmd-verify.ts b/packages/cli/src/cmd-verify.ts index 08356078..bedd31f8 100644 --- a/packages/cli/src/cmd-verify.ts +++ b/packages/cli/src/cmd-verify.ts @@ -24,7 +24,7 @@ import type { CliCommand, CommandContext } from './command'; import { checkDestructiveMigrations } from './db-destructive'; import { checkDocumentStyles, documentSurfaces } from './document-styles'; import { checkSourceDrift } from './drift'; -import { checkErrorFixes } from './error-contract'; +import { checkErrorFixReport } from './error-contract'; import { readIntFlag } from './flag-number'; import { guardFindings } from './guards'; import { msg } from './messages'; @@ -115,8 +115,21 @@ export const VERIFY_STEPS: readonly VerifyStep[] = [ summary: 'every X_* code has a runnable fix and a docs page', // The fix-line half runs anywhere source does. The docs half needs a reference page to check // against, and which file that is belongs to the host repo — hence `hostFindings`. - run: async (ctx) => - fromFindings([...(await checkErrorFixes(ctx.root)), ...(await hostFindings(ctx, 'errors'))]), + // + // The coverage line rides in `output`, which `--json` carries verbatim: a scan without a + // parser cannot read every fix, and a step that reports only findings claims a completeness + // it does not have. "checked 412, could not read 27" is what a reader can act on. + async run(ctx) { + const report = await checkErrorFixReport(ctx.root); + const findings = [...report.findings, ...(await hostFindings(ctx, 'errors'))]; + return { + ...fromFindings(findings), + output: msg('cli.verify.fixCoverage', { + checked: report.checked, + unreadable: report.unreadable, + }), + }; + }, }, ...TEST_STEPS, { diff --git a/packages/cli/src/error-codes.ts b/packages/cli/src/error-codes.ts index 6b0d0e9c..8d4d55a9 100644 --- a/packages/cli/src/error-codes.ts +++ b/packages/cli/src/error-codes.ts @@ -86,6 +86,10 @@ export const CLI_OWNED_ERROR_CODES = [ 'X_GUARD_INVALID', 'X_GUARD_FAILED', 'X_GUARD_FINDING_INVALID', + // The CLI's own declarations, held to each other. A flag the parser accepts and no code reads + // is a promise in `x help` with nothing behind it — `x deploy --critical` said "forces clients + // to reload" and reached no reader outside the plan JSON it was written into. + 'X_CLI_FLAG_UNREAD', // The two halves of `x secrets edit` that belong to the terminal rather than to the envelope. // `@ultimat3/core` owns every X_SECRETS_* code about the file and the key; an editor is the // CLI's problem alone, and core would have no `fix:` to offer for one. @@ -179,6 +183,7 @@ export const CLI_ERROR_TITLES: Readonly> = { X_GUARD_INVALID: 'a file in guards/ exports no usable guard', X_GUARD_FAILED: 'an app guard threw instead of returning findings', X_GUARD_FINDING_INVALID: "an app guard's finding breaks the error contract", + X_CLI_FLAG_UNREAD: 'a command declares a flag no code reads', X_SECRETS_EDITOR_MISSING: 'no $EDITOR to open the decrypted secrets in', X_SECRETS_EDIT_FAILED: 'the editor exited non-zero, so nothing was resealed', }; diff --git a/packages/cli/src/error-contract.test.ts b/packages/cli/src/error-contract.test.ts index fb7867aa..b6b38e5b 100644 --- a/packages/cli/src/error-contract.test.ts +++ b/packages/cli/src/error-contract.test.ts @@ -169,6 +169,96 @@ describe('the checks, over a repo', () => { expect(await checkErrorFixes(root)).toEqual([]); }); + // The hole that shipped four bad fix lines in `packages/ui/src/icons/build-icons.ts`: the + // helper is declared in a SIBLING module, so the same-file rule read the call site's argument as + // nobody's fix and the gate checked nothing. A per-package `errors.ts` full of factories is the + // house pattern, so this is the shape most fixes arrive in. + test('a fix handed to a factory in a sibling module is read', async () => { + await write( + 'packages/ui/src/errors.ts', + 'export function invalidIconDataError(found: string, fix: string) {\n' + + " return new UiError({ code: 'X_UI_INVALID_VALUE', cause: found, fix });\n" + + '}\n', + ); + await write( + 'packages/ui/src/icons/build-icons.ts', + "import { invalidIconDataError } from '../errors';\n" + + "throw invalidIconDataError('bad', 'check the network, then re-run the generator');\n", + ); + const [finding, ...rest] = await checkErrorFixes(root); + expect(rest).toEqual([]); + expect(finding?.code).toBe('X_ERROR_FIX_INVALID'); + expect(finding?.at).toBe('packages/ui/src/icons/build-icons.ts:2'); + }); + + // The other rule the same seam owes: a citation resolved against the registry, not just the + // text rule. `x ui icons` names a command, which is exactly why the text rule passes it. + test('a cross-file fix citing a command this build does not ship is a finding', async () => { + await write( + 'packages/ui/src/errors.ts', + "const raise = (cause: string, fix: string) => new E({ code: 'X_A', cause, fix });\n" + + 'export { raise };\n', + ); + await write( + 'packages/ui/src/icons/build-icons.ts', + "import { raise } from '../errors';\nraise('bad', 'x ui icons --json');\n", + ); + const [finding] = await checkErrorFixes(root); + expect(finding?.cause).toContain('x ui'); + }); + + // `@ultimat3/render`'s `errors.ts` declares fourteen classes taking `(cause, fix)` positionally + // and calls them from its own modules. Measured: zero SAME-file call sites, so the same-file + // rule was dead code for the entire form and 15 codes never had a fix line read. + test('a fix handed to an error class in a sibling module is read', async () => { + await write( + 'packages/render/src/errors.ts', + 'export class RouteModeInvalidError extends UltimateError {\n' + + " static readonly code = 'X_ROUTE_MODE_INVALID' as const;\n" + + ' constructor(cause: string, fix: string) {\n' + + ' super({ code: RouteModeInvalidError.code, cause, fix });\n' + + ' }\n' + + '}\n', + ); + await write( + 'packages/render/src/modes.ts', + "import { RouteModeInvalidError } from './errors';\n" + + "throw new RouteModeInvalidError('static may not read the request', 'try another mode');\n", + ); + const [finding, ...rest] = await checkErrorFixes(root); + expect(rest).toEqual([]); + expect(finding?.at).toBe('packages/render/src/modes.ts:2'); + }); + + // The importer names the symbol; the declaration names the position. An alias is the one place + // those two disagree, and reading the declaration's name at the call site would resolve nothing. + test('an aliased import is resolved under the name the caller uses', async () => { + await write( + 'packages/db/src/errors.ts', + "export function dbNotImplemented(cause: string, fix: string) { throw new E({ code: 'X_A', cause, fix }); }\n", + ); + await write( + 'packages/db/src/pglite-branch.ts', + "import { dbNotImplemented as unsupported } from './errors';\n" + + "unsupported('pglite has no branches', 'see the docs');\n", + ); + const [finding] = await checkErrorFixes(root); + expect(finding?.at).toBe('packages/db/src/pglite-branch.ts:2'); + }); + + test('a runnable cross-file fix is not a finding', async () => { + await write( + 'packages/db/src/errors.ts', + "export function dbNotImplemented(cause: string, fix: string) { throw new E({ code: 'X_A', cause, fix }); }\n", + ); + await write( + 'packages/db/src/pglite-branch.ts', + "import { dbNotImplemented } from './errors';\n" + + "dbNotImplemented('pglite has no branches', 'x db branch ls --json');\n", + ); + expect(await checkErrorFixes(root)).toEqual([]); + }); + // A test fixture asserting on a bad fix is a test, not a shipped error. test('checkErrorFixes skips tests and generated declarations', async () => { await write('packages/db/src/thing.test.ts', "expect(e.fix).toBe('check the connection');\n"); @@ -387,5 +477,8 @@ describe('this repo', () => { // the test, so the timeout is what moves. Same shape as `scripts/verify.test.ts`. test('every shipped fix line is runnable', async () => { expect(await checkErrorFixes(root)).toEqual([]); - }, 30_000); + // 90s, matching `scripts/lib/run.ts`'s `REPO_SCAN_TIMEOUT_MS`. The number is duplicated rather + // than imported because `packages/cli` cannot reach `scripts/` — a package may not depend on + // the repo that ships it. Raise both together; ~5s alone, ~30s under eight competing workers. + }, 90_000); }); diff --git a/packages/cli/src/error-contract.ts b/packages/cli/src/error-contract.ts index 796cd0fb..f007940f 100644 --- a/packages/cli/src/error-contract.ts +++ b/packages/cli/src/error-contract.ts @@ -8,10 +8,12 @@ import { join } from 'node:path'; import { docsFor } from './error-codes'; import { citedCommandProblem, loadCommandCatalog } from './fix-command'; +import { createHelperResolver } from './fix-imports'; +import { scanFixSites } from './fix-scan'; import type { Finding } from './output'; import { eachSourceFile, isGenerated, isTest } from './source-files'; import type { CodeSite, FixSite } from './ts-scan'; -import { isCodeRegistry, scanBorrowedCodes, scanCodes, scanFixes } from './ts-scan'; +import { isCodeRegistry, scanBorrowedCodes, scanCodes } from './ts-scan'; /** Advice, not instruction. The list is the one in `docs/architecture/04-error-contract.md`. */ export const BANNED_PHRASES: readonly RegExp[] = [ @@ -77,12 +79,33 @@ const fixFinding = (site: FixSite, problem: string): Finding => ({ * The catalog is loaded ONCE per run rather than per fix line: it is a dynamic import (see * `fix-command.ts` for the cycle it breaks) and this walks every shipped source file. */ -export async function checkErrorFixes(root: string): Promise { +export interface ErrorFixReport { + readonly findings: readonly Finding[]; + /** Fix literals actually read, and held to both rules. */ + readonly checked: number; + /** + * Fix arguments at a known builder that hold no single literal — a parameter passed through, a + * concatenation, a table lookup. The step prints it, because a gate that says "checked 412, + * could not read 27" is honest and one that says nothing is the false green this check exists to + * close. It does NOT cover a builder imported from another PACKAGE: `candidatePaths` resolves + * relative specifiers only, and that gap is 3 call sites across this repo, measured 2026-08. + */ + readonly unreadable: number; +} + +export async function checkErrorFixReport(root: string): Promise { const findings: Finding[] = []; const catalog = await loadCommandCatalog(); + const imports = createHelperResolver(root); + let checked = 0; + let unreadable = 0; for await (const path of eachSourceFile(root)) { if (isTest(path) || isGenerated(path)) continue; - for (const site of scanFixes(await Bun.file(join(root, path)).text(), path)) { + const source = await Bun.file(join(root, path)).text(); + const scan = scanFixSites(source, path, await imports(path, source)); + checked += scan.sites.length; + unreadable += scan.unreadable; + for (const site of scan.sites) { // The interpolation-blanked form for both rules: `x ${name}` names no command this can // resolve, and reading `` as one would be a finding nobody can act on. const fix = staticFix(site.fix); @@ -90,9 +113,13 @@ export async function checkErrorFixes(root: string): Promise if (problem !== undefined) findings.push(fixFinding(site, problem)); } } - return findings; + return { findings, checked, unreadable }; } +/** The findings alone, for every caller that reports no coverage line. */ +export const checkErrorFixes = async (root: string): Promise => + (await checkErrorFixReport(root)).findings; + /** * A code is documented when the reference page names it. Deliberately not "owns a table row": the * page legitimately groups near-identical codes onto one row, and a rule that forbade that would diff --git a/packages/cli/src/fix-imports.test.ts b/packages/cli/src/fix-imports.test.ts new file mode 100644 index 00000000..b980f332 --- /dev/null +++ b/packages/cli/src/fix-imports.test.ts @@ -0,0 +1,146 @@ +// The resolver's fixtures are real files, because the thing under test is opening one: `node:fs` +// builds an isolated tree per test and removes it, so two runs cannot share a module cache or a +// stale `errors.ts` — the four bad `fix:` lines this closes lived in a file that imported its +// builder, and a fixture in memory would prove nothing about resolving the specifier. + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { candidatePaths, createHelperResolver, scanImports } from './fix-imports'; + +describe('scanImports', () => { + test('reads a named import and its specifier', () => { + expect(scanImports("import { a, b } from './errors';")).toEqual([ + { + specifier: './errors', + names: [ + { exported: 'a', local: 'a' }, + { exported: 'b', local: 'b' }, + ], + }, + ]); + }); + + test('an alias is the name the CALLER uses, and the declaration keeps its own', () => { + expect(scanImports("import { dbNotImplemented as unsupported } from './errors';")).toEqual([ + { + specifier: './errors', + names: [{ exported: 'dbNotImplemented', local: 'unsupported' }], + }, + ]); + }); + + // A type has no call site, so reading one could only produce a helper nothing calls. + test('a type-only import, in either spelling, carries no value', () => { + expect(scanImports("import type { FixSite } from './ts-scan';")).toEqual([]); + expect(scanImports("import { type FixSite, scanFixes } from './ts-scan';")).toEqual([ + { specifier: './ts-scan', names: [{ exported: 'scanFixes', local: 'scanFixes' }] }, + ]); + }); + + // `errors.raise(…)` is a member access, which `helperFixSites` refuses by design — matching the + // namespace here would hand it a name it can never resolve a call for. + test('a namespace import is not a named one', () => { + expect(scanImports("import * as errors from './errors';")).toEqual([]); + }); + + // `packages/cli/src/templates/` emits app source, imports included, inside template literals. + // Read as declarations they pointed the resolver at modules that exist only in a generated app. + test('an import written inside a comment or a template is not one', () => { + expect(scanImports("// import { raise } from './errors';\n")).toEqual([]); + expect(scanImports("const doc = `\nimport { raise } from './errors';\n`;")).toEqual([]); + }); +}); + +describe('candidatePaths', () => { + test('a relative specifier resolves against the importing file, extension first', () => { + expect(candidatePaths('packages/ui/src/icons/build-icons.ts', '../errors')).toEqual([ + 'packages/ui/src/errors.ts', + 'packages/ui/src/errors.tsx', + 'packages/ui/src/errors/index.ts', + 'packages/ui/src/errors/index.tsx', + ]); + }); + + // A package specifier is another package's file set: resolving one means guessing which of 29 + // packages a bare name came from, and a wrong guess reads an unrelated function's argument as + // a fix. Measured 2026-08: 3 call sites in this repo import a fix builder this way. + test('a package or node specifier resolves to nothing', () => { + expect(candidatePaths('packages/cli/src/a.ts', '@ultimat3/db')).toEqual([]); + expect(candidatePaths('packages/cli/src/a.ts', 'node:path')).toEqual([]); + }); + + test('a specifier that climbs out of the repo root names no file this may open', () => { + expect(candidatePaths('packages/cli/src/a.ts', '../../../../secrets')).toEqual([]); + }); +}); + +describe('createHelperResolver', () => { + let root = ''; + + const write = async (path: string, text: string): Promise => { + await mkdir(join(root, path, '..'), { recursive: true }); + await writeFile(join(root, path), text); + }; + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'fix-imports-')); + }); + + afterEach(async () => { + await rm(root, { recursive: true, force: true }); + }); + + const ERRORS = + 'export function raise(cause: string, fix: string) {\n' + + " return new E({ code: 'X_A', cause, fix });\n" + + '}\n' + + 'export const shout = (fix: string) => new E({ code: "X_B", fix });\n'; + + test('a helper in a sibling module is callable here, at its declared position', async () => { + await write('packages/ui/src/errors.ts', ERRORS); + const resolve = createHelperResolver(root); + const found = await resolve( + 'packages/ui/src/icons/build-icons.ts', + "import { raise, shout } from '../errors';", + ); + expect(found).toEqual([ + { name: 'raise', index: 1 }, + { name: 'shout', index: 0 }, + ]); + }); + + test('an alias renames the helper to what the call site writes', async () => { + await write('packages/ui/src/errors.ts', ERRORS); + const resolve = createHelperResolver(root); + const found = await resolve('packages/ui/src/a.ts', "import { raise as bad } from './errors';"); + expect(found).toEqual([{ name: 'bad', index: 1 }]); + }); + + test('a directory specifier resolves through its index', async () => { + await write('packages/ui/src/errors/index.ts', ERRORS); + const resolve = createHelperResolver(root); + const found = await resolve('packages/ui/src/a.ts', "import { raise } from './errors';"); + expect(found).toEqual([{ name: 'raise', index: 1 }]); + }); + + test('a name the module declares but does not build an error with is not a helper', async () => { + await write('packages/ui/src/errors.ts', 'export const label = (fix: string) => fix.trim();\n'); + const resolve = createHelperResolver(root); + const found = await resolve('packages/ui/src/a.ts', "import { label } from './errors';"); + expect(found).toEqual([]); + }); + + // The named limit: another PACKAGE's file set is not resolved, and a relative path that is not + // there resolves to nothing rather than to a guess. What the gate reports about the fixes those + // hide is `FixScan.unreadable`, counted at the call site. + test('a package specifier and a missing file both resolve to no helper', async () => { + const resolve = createHelperResolver(root); + const found = await resolve( + 'packages/ui/src/a.ts', + "import { UltimateError } from '@ultimat3/core';\nimport { gone } from './missing';", + ); + expect(found).toEqual([]); + }); +}); diff --git a/packages/cli/src/fix-imports.ts b/packages/cli/src/fix-imports.ts new file mode 100644 index 00000000..ec61cd31 --- /dev/null +++ b/packages/cli/src/fix-imports.ts @@ -0,0 +1,118 @@ +// A fix-building helper the calling file did not declare: `invalidIconDataError` lives in +// `packages/ui/src/errors.ts` and every fix it is handed is written in `icons/build-icons.ts`. +// One specifier, one file read, one parameter position — deliberately still not `tsc`. + +// `dirname`/`join` are `node:`-only by necessity: Bun exposes no path-join primitive. +import { dirname, join } from 'node:path'; +import type { FixHelper } from './fix-scan'; +import { scanFixHelpers } from './fix-scan'; +import { endOfLiteral, maskLiterals } from './ts-scan'; + +/** + * A named import, matched over the MASKED source and anchored at the start of a line, so an + * `import …` written inside a template literal is not read as one — `packages/cli/src/templates/` + * emits a dozen of them as generated app source, and resolving those pointed the scan at a module + * that only exists in the app the template writes. Masking blanks a literal's contents and keeps + * its delimiters, so the specifier is read back out of the raw source at the quote's own offset. + * + * `import type` is skipped whole: a type has no call site. A default or namespace import is not + * matched at all — this repo ships no default exports, and `errors.raise(…)` is a member access + * that `helperFixSites` refuses by design. + */ +const IMPORT_CLAUSE = /^import\s+(type\s+)?\{([^}]*)\}\s*from\s*['"]/gm; + +/** `a`, `a as b`, and the inline `type a` that carries no value. */ +const parseClause = (clause: string): { readonly exported: string; readonly local: string }[] => + clause + .split(',') + .map((part) => part.trim()) + .filter((part) => part !== '' && !/^type\s/.test(part)) + .map((part) => { + const [exported, local] = part.split(/\s+as\s+/); + return { exported: (exported ?? '').trim(), local: (local ?? exported ?? '').trim() }; + }) + .filter((name) => /^[A-Za-z_$][\w$]*$/.test(name.exported) && name.local !== ''); + +interface LocalImport { + readonly specifier: string; + readonly names: readonly { readonly exported: string; readonly local: string }[]; +} + +/** Every value import in this file, specifier as written. */ +export function scanImports(source: string): readonly LocalImport[] { + const masked = maskLiterals(source); + const imports: LocalImport[] = []; + for (const match of masked.matchAll(IMPORT_CLAUSE)) { + if (match[1] !== undefined) continue; + const names = parseClause(match[2] ?? ''); + const quote = match.index + match[0].length - 1; + const specifier = source.slice(quote + 1, endOfLiteral(masked, quote) - 1); + if (names.length > 0 && specifier !== '') imports.push({ specifier, names }); + } + return imports; +} + +/** + * The repo-relative paths a relative specifier could name, in resolution order. Only relative + * ones: `@ultimat3/db` and `node:path` are somebody else's file set, and a scanner that guessed + * which package a bare name came from would read an unrelated function's argument as a fix. That + * gap is real and named — `x verify`'s `errors` step counts what it could not read rather than + * passing over it silently, which is the failure this file exists to end. + */ +export function candidatePaths(from: string, specifier: string): readonly string[] { + if (!specifier.startsWith('./') && !specifier.startsWith('../')) return []; + const base = join(dirname(from), specifier); + // A path that climbed out of the repo root is not a file this scan may open. + if (base.startsWith('..')) return []; + return [`${base}.ts`, `${base}.tsx`, join(base, 'index.ts'), join(base, 'index.tsx')]; +} + +/** + * The helpers one file can call, named as that file names them. + * + * Deliberately NOT also a count of the imports it could not open: that number is 1504 in this + * repo and 1310 of them are `@ultimat3/*` names like `join` and `UltimateError` — a figure nobody + * can act on. What the gate reports instead is `FixScan.unreadable`, which counts only arguments + * in a KNOWN fix position, and is therefore a count of fixes rather than of imports. + */ +export type HelperResolver = (path: string, source: string) => Promise; + +/** + * One resolver per run, because the module cache is the whole point: `packages/ui/src/errors.ts` + * is imported by every file in the package, and re-reading and re-scanning it per importer turns + * a one-pass walk into a quadratic one. + * + * A name declared in the imported module wins by NAME alone — no export check. The tree + * typechecks, so a name this file imports is a name that module exports; a second rule reading + * `export` keywords would only be able to disagree with `tsc`, never to add anything. + */ +export function createHelperResolver(root: string): HelperResolver { + const modules = new Map(); + + const helpersIn = async (path: string): Promise => { + const cached = modules.get(path); + if (cached !== undefined || modules.has(path)) return cached; + const file = Bun.file(join(root, path)); + const found = (await file.exists()) + ? scanFixHelpers(maskLiterals(await file.text())) + : undefined; + modules.set(path, found); + return found; + }; + + return async (path, source) => { + const helpers: FixHelper[] = []; + for (const declaration of scanImports(source)) { + let declared: readonly FixHelper[] | undefined; + for (const candidate of candidatePaths(path, declaration.specifier)) { + declared = await helpersIn(candidate); + if (declared !== undefined) break; + } + for (const name of declaration.names) { + const helper = (declared ?? []).find((one) => one.name === name.exported); + if (helper !== undefined) helpers.push({ name: name.local, index: helper.index }); + } + } + return helpers; + }; +} diff --git a/packages/cli/src/fix-scan.test.ts b/packages/cli/src/fix-scan.test.ts new file mode 100644 index 00000000..6270403c --- /dev/null +++ b/packages/cli/src/fix-scan.test.ts @@ -0,0 +1,263 @@ +// Three shapes a `fix:` arrives in, one describe each: under a key, positionally at a local error +// builder's parameter, and at an error class constructor's — plus the same builder resolved from +// another file. Each gap shipped stale lines the gate could not see: `@ultimat3/mcp`'s two +// `x db branch ` through the positional helper, `@ultimat3/ui`'s four through the imported one. + +import { describe, expect, test } from 'bun:test'; +import { scanFixes, scanFixSites } from './fix-scan'; + +const fixes = (source: string): readonly string[] => + scanFixes(source, 'a.ts').map((site) => site.fix); + +describe('scanFixes', () => { + test('reads a plain literal', () => { + expect(fixes("throw new E({ fix: 'x doctor --json' });")).toEqual(['x doctor --json']); + }); + + test('reads both branches of a ternary and a ?? default', () => { + expect(fixes("({ fix: hit ? 'x help' : 'x verify --json' })")).toEqual([ + 'x help', + 'x verify --json', + ]); + expect(fixes("({ fix: input.fix ?? 'x help' })")).toEqual(['x help']); + }); + + // The bug this guards: every literal in the expression used to count, so `.join(' ')`'s + // separator and `TABLE['key']`'s key were reported as empty and vague fix lines. + test('ignores literals nested inside a call or an index', () => { + expect(fixes("({ fix: command.join(' ') })")).toEqual([]); + expect(fixes("({ fix: FIXES['starttls'] ?? '' })")).toEqual(['']); + }); + + test('stops at the property that follows', () => { + expect(fixes("({ fix: 'x help', docs: 'https://ultimate.dev/errors/X_A' })")).toEqual([ + 'x help', + ]); + }); + + // The bug this guards: the quotes inside a regex like this file's own `CODE_LITERAL` read as + // string delimiters, so the masking desynced and blanked the `fix:` declared after it. The file + // then reported no fixes at all and the gate passed over error text nobody had checked. + test('a regex holding quote characters does not hide the declaration after it', () => { + const source = `const CODE_LITERAL = /(['"\`])(X_[A-Z0-9_]+)\\1/g; +throw new E({ fix: 'x doctor --json' });`; + expect(fixes(source)).toEqual(['x doctor --json']); + }); + + // Same masking desync, reached from JSX rather than from a regex: the file's `fix:` lines + // vanished wholesale, `x verify`'s `errors` step checked none of them, and `scanCodes` still + // found the code — so `X_ERROR_CODE_UNDOCUMENTED` passed and hid the hole. + test('an apostrophe in JSX text does not hide the declaration below it', () => { + const source = + 'export function Panel() {\n' + + " return

Don't panic

;\n" + + '}\n' + + "throw new E({ code: 'X_A', fix: 'x doctor --json' });"; + expect(scanFixes(source, 'a.tsx')).toEqual([{ at: 'a.tsx', line: 4, fix: 'x doctor --json' }]); + }); + + test('an escape and a character class do not close the regex early', () => { + expect(fixes("const re = /[/']|a\\/b/;\nthrow new E({ fix: 'x help' });")).toEqual(['x help']); + }); + + test('a division is read as one, so the declaration after it survives', () => { + expect(fixes("const per = total / count;\nthrow new E({ fix: 'x help' });")).toEqual([ + 'x help', + ]); + }); + + test('ignores a fix: written in a comment or interpolated into a message', () => { + expect(fixes('// fix: x db gen "add publish_at"\nconst a = 1;')).toEqual([]); + // biome-ignore lint/suspicious/noTemplateCurlyInString: the input is source text — a literal ${…} is the case under test + expect(fixes('const line = `${code}: ${cause} (fix: ${fix})`;')).toEqual([]); + }); + + // `cond ? e.fix : ''` is a ternary on a property named fix, not a fix declaration. + test('ignores a member access followed by a ternary colon', () => { + expect(fixes("const fix = typeof e.fix === 'string' ? e.fix : '';")).toEqual([]); + }); + + test('reports the line the literal is on, not the line the key is on', () => { + const source = "throw new E({\n code: 'X_A',\n fix:\n 'x help',\n});"; + expect(scanFixes(source, 'a.ts')[0]).toEqual({ at: 'a.ts', line: 4, fix: 'x help' }); + }); + + test('a runtime-computed fix has no literal to read', () => { + expect(fixes('({ fix: input.fix })')).toEqual([]); + expect(fixes('interface F { readonly fix: string }')).toEqual([]); + }); +}); + +/** + * The blind spot that let two stale fix lines ship. `@ultimat3/mcp`'s `readonly-sql.ts` passes its + * fixes POSITIONALLY into local `rejected(cause, fix)` / `notBranch(cause, fix)` helpers, so there + * is no `fix:` key anywhere in the file and the scanner returned `[]` for all of it — the citation + * resolver was never handed a single string to judge. + */ +describe('scanFixes · a fix passed positionally into a local error builder', () => { + const BUILDER = + 'function rejected(cause: string, fix: string) {\n' + + " return new McpError({ code: 'X_A', cause, fix });\n" + + '}\n'; + + test('reads the argument in the fix parameter position', () => { + expect(fixes(`${BUILDER}rejected('not one statement', 'x db branch ls --json');`)).toEqual([ + 'x db branch ls --json', + ]); + }); + + test('the position is the declared one, not "the last argument"', () => { + const source = + 'const fail = (field: string, fix: string, meta: object) => {\n' + + " throw new E({ code: 'X_B', cause: field, fix });\n" + + '};\n' + + "fail('amount', 'x g migration money', { a: 1 });"; + expect(fixes(source)).toEqual(['x g migration money']); + }); + + test('a helper that CONSUMES a fix is not a helper that declares one', () => { + // `citedCommandProblem(fix: string, catalog)` takes a fix in order to judge it. Reading its + // call sites as declarations would report findings about strings that are already findings — + // so a helper only counts when its body builds an error. + const source = + 'function citedCommandProblem(fix: string, catalog: Catalog) {\n' + + ' return catalog.resolve(fix);\n' + + '}\n' + + "citedCommandProblem('x db branch lst', catalog);"; + expect(fixes(source)).toEqual([]); + }); + + test('a rest or destructured parameter list has no reliable position, so it is skipped', () => { + // Both fixtures BUILD an error, or the builder discriminator would be what rejects them and + // these two cases would pass without the rules they exist to pin. + const rest = + 'function raise(kind: string, fix: string, ...rest: unknown[]) { throw new MyError({ code: kind, fix }); }\n' + + "raise('X_C', 'x doctor --json');"; + const destructured = + 'function raise({ kind, fix }: { kind: string; fix: string }) { throw new MyError({ code: kind, fix }); }\n' + + "raise({ kind: 'X_C', fix: 'x doctor --json' });"; + expect(fixes(rest)).toEqual([]); + // The object form needs no rule of its own: the `fix:` key at the call site is already read. + expect(fixes(destructured)).toEqual(['x doctor --json']); + }); + + test("a `{` further down the file is not this declaration's body", () => { + // The builder discriminator reads the helper's BODY, and `bodyOf` took the next `{` anywhere + // below it — so an expression-bodied helper that only formats a string was classified by + // whatever object literal happened to follow. Every call to it then handed the gate a string + // to judge as a fix, and a gate that fails on innocent source is worse than one that misses. + const source = + 'const label = (fix: string): string => fix.trim();\n' + + "const TITLES = { code: 'X_A' };\n" + + "label('x db branch lst');"; + expect(fixes(source)).toEqual([]); + }); + + test('a concise arrow body is still read, so the bound did not just switch the rule off', () => { + const source = + "const rejected = (cause: string, fix: string): Finding => ({ code: 'X_A', cause, fix });\n" + + "rejected('a', 'x doctor --json');"; + expect(fixes(source)).toEqual(['x doctor --json']); + }); + + test("a method call on some other object is not this file's helper", () => { + expect(fixes(`${BUILDER}reporter.rejected('a', 'x doctor --json');`)).toEqual([]); + }); + + test('an argument that is not a sole literal has nothing to read', () => { + expect(fixes(`${BUILDER}rejected('a', input.fix);`)).toEqual([]); + expect(fixes(`${BUILDER}rejected('a', prefix + 'x doctor');`)).toEqual([]); + expect(fixes(`${BUILDER}rejected('a');`)).toEqual([]); + }); + + test('the site carries the line the argument is on', () => { + const source = `${BUILDER}rejected(\n 'a',\n 'x doctor --json',\n);`; + expect(scanFixes(source, 'a.ts')).toEqual([{ at: 'a.ts', line: 6, fix: 'x doctor --json' }]); + }); +}); + +/** + * The third shape. `@ultimat3/render`'s `errors.ts` declares fourteen error classes taking + * `(cause, fix)` positionally and `@ultimat3/core`'s image pipeline three more; a scan that reads + * only functions read none of their fix lines, and 15 of render's codes had never had one checked. + */ +describe('scanFixes · a fix passed positionally into an error class', () => { + const CLASS = + 'export class RouteModeInvalidError extends UltimateError {\n' + + " static readonly code = 'X_ROUTE_MODE_INVALID' as const;\n" + + ' constructor(cause: string, fix: string) {\n' + + ' super({ code: RouteModeInvalidError.code, cause, fix });\n' + + ' }\n' + + '}\n'; + + test('reads the argument in the constructor fix position', () => { + expect(fixes(`${CLASS}throw new RouteModeInvalidError('static read the request', 'x help');`)) // + .toEqual(['x help']); + }); + + test('the helper is the CLASS name and the position is the constructor’s', () => { + const source = + 'class Fail extends UltimateError {\n' + + ' constructor(field: string, fix: string, meta: object) {\n' + + " super({ code: 'X_B', cause: field, fix });\n" + + ' }\n' + + '}\n' + + "new Fail('amount', 'x g migration money', { a: 1 });"; + expect(fixes(source)).toEqual(['x g migration money']); + }); + + // Same discriminator as the function form: a class that merely carries a fix is not one that + // builds an error, and reading its call sites would report findings about judged strings. + test('a class whose constructor builds no error is not a helper', () => { + const source = + 'class Advice {\n' + + ' constructor(cause: string, fix: string) {\n' + + ' this.text = cause.concat(fix);\n' + + ' }\n' + + '}\n' + + "new Advice('a', 'x db branch lst');"; + expect(fixes(source)).toEqual([]); + }); + + test('a class with no constructor at all declares nothing', () => { + const source = + "class Plain extends UltimateError { static readonly code = 'X_C' as const; }\n" + + "new Plain('x db branch lst');"; + expect(fixes(source)).toEqual([]); + }); +}); + +/** + * The blind spot #157 names: a helper declared in a SIBLING module. `fix-imports.ts` resolves the + * specifier and hands the table in — this is the half that reads a call site against it. + */ +describe('scanFixes · a helper resolved from another file', () => { + const IMPORTED = [{ name: 'invalidIconData', index: 1 }] as const; + + test('reads the argument at the imported position', () => { + const source = "invalidIconData('parsed as null', 'x doctor --json');"; + expect(scanFixes(source, 'a.ts', IMPORTED)).toEqual([ + { at: 'a.ts', line: 1, fix: 'x doctor --json' }, + ]); + }); + + // A file that declares the name itself is calling ITS OWN function, whatever it also imports — + // and reading both tables would report one argument twice, at two different positions. + test('a local declaration of the same name wins over the imported one', () => { + const source = + "function invalidIconData(fix: string, cause: string) { throw new E({ code: 'X_A', fix }); }\n" + + "invalidIconData('x doctor --json', 'parsed as null');"; + expect(scanFixes(source, 'a.ts', IMPORTED).map((site) => site.fix)).toEqual([ + 'x doctor --json', + ]); + }); + + test('an unreadable argument is counted rather than dropped', () => { + expect(scanFixSites("invalidIconData('a', input.fix);", 'a.ts', IMPORTED)).toEqual({ + sites: [], + unreadable: 1, + }); + // One argument short is not an unreadable fix: nothing was written there to read. + expect(scanFixSites("invalidIconData('a');", 'a.ts', IMPORTED).unreadable).toBe(0); + }); +}); diff --git a/packages/cli/src/fix-scan.ts b/packages/cli/src/fix-scan.ts new file mode 100644 index 00000000..cf5a2c6f --- /dev/null +++ b/packages/cli/src/fix-scan.ts @@ -0,0 +1,251 @@ +// Every string a `fix:` can evaluate to, in the three shapes a fix arrives in: under a key, in the +// argument position of a factory that builds an error, and in the argument position of an error +// class's constructor. Split out of `ts-scan.ts` when the third shape and cross-file resolution +// (`fix-imports.ts`) took the file past the 500-line ceiling; the masking primitives stay there. + +import type { FixSite } from './ts-scan'; +import { + CLOSERS, + endOfLiteral, + lineIndex, + maskLiterals, + OPENERS, + QUOTES, + valueLiterals, +} from './ts-scan'; + +/** The lookbehind rejects member access: `cond ? e.fix : ''` is a ternary, not a declaration. */ +const FIX_KEY = /(?` in the + // very same list, so counting it makes `(fn: () => void)` end the span in the wrong place. + if (OPENERS.has(ch)) depth += 1; + else if (CLOSERS.has(ch)) { + depth -= 1; + if (depth === 0) return masked.slice(open + 1, i); + } + } + return undefined; +} + +/** Split at depth-0 commas. Safe on masked text, where a comma inside a literal is already gone. */ +function topLevelParts(text: string): readonly string[] { + const parts: string[] = []; + let depth = 0; + let start = 0; + for (let i = 0; i < text.length; i += 1) { + const ch = text[i] as string; + if (OPENERS.has(ch)) depth += 1; + else if (CLOSERS.has(ch)) depth -= 1; + else if (ch === ',' && depth === 0) { + parts.push(text.slice(start, i)); + start = i + 1; + } + } + parts.push(text.slice(start)); + return parts; +} + +/** A callable that builds an error and takes its fix positionally, and where in its list. */ +export interface FixHelper { + readonly name: string; + readonly index: number; +} + +const HELPER_DECL = + /(? fix.trim();` followed + * anywhere by a `{ code: … }` was read as an error builder and every `label(…)` call handed the + * gate a string to judge as a fix — a false gate failure over innocent source. + * + * The scan therefore ends at the `;` that ends the declaration, or at a bracket closing a scope + * this declaration is inside. Both directions of that bound answer `''`, which classifies the + * helper as a non-builder: a missed fix line costs one unchecked citation, a wrongly claimed one + * costs a build. A `{` inside a return-type annotation (`(): { ok: boolean } => …`) is read as the + * body and answers `''` for the same reason. + */ +function bodyOf(masked: string, after: number): string { + for (let i = after; i < masked.length; i += 1) { + const ch = masked[i] as string; + if (ch === '{') return bracketSpan(masked, i) ?? ''; + if (ch === ';' || CLOSERS.has(ch)) break; + } + return ''; +} + +/** + * One declaration judged: it must build an error, and its fix must sit at a position a call site + * can be read at. A rest parameter makes the position of everything after it unknowable, and a + * destructured one has no position at all — its `fix:` key at the CALL site is already read by + * `FIX_KEY`. + */ +function helperAt(masked: string, name: string, open: number): FixHelper | undefined { + const params = bracketSpan(masked, open); + if (params === undefined || params.includes('...')) return undefined; + const parts = topLevelParts(params); + if (parts.some((part) => /^\s*[[{]/.test(part))) return undefined; + const index = parts.findIndex((part) => FIX_PARAM.test(part)); + if (index === -1) return undefined; + if (!BUILDS_ERROR.test(bodyOf(masked, open + params.length + 2))) return undefined; + return { name, index }; +} + +/** + * Every fix-building callable this file declares — a function, an arrow bound to a const, or a + * class whose constructor takes the fix. Exported because `fix-imports.ts` asks the same question + * of a file this one merely imports FROM; there is no second reader of a declaration. + */ +export function scanFixHelpers(masked: string): readonly FixHelper[] { + const helpers: FixHelper[] = []; + for (const declaration of masked.matchAll(HELPER_DECL)) { + const name = declaration[1] ?? declaration[2]; + const open = declaration.index + declaration[0].length - 1; + if (name === undefined || masked[open] !== '(') continue; + const helper = helperAt(masked, name, open); + if (helper !== undefined) helpers.push(helper); + } + for (const declaration of masked.matchAll(CLASS_DECL)) { + const name = declaration[1]; + const body = declaration.index + declaration[0].length - 1; + const inner = bracketSpan(masked, body); + const found = inner === undefined ? null : CONSTRUCTOR.exec(inner); + if (name === undefined || found === null) continue; + const helper = helperAt(masked, name, body + 1 + found.index + found[0].length - 1); + if (helper !== undefined) helpers.push(helper); + } + return helpers; +} + +/** + * A fix argument this scan could not read, at a call site it could: the callee is a known helper + * and the fix position holds something other than one literal. Counted rather than dropped, so + * `x verify`'s `errors` step can say "checked 412, could not read 27" — a gate that stays silent + * about its own blind spot is the false green this file exists to close (axiom 4 applies to it too). + */ +export interface FixScan { + readonly sites: readonly FixSite[]; + readonly unreadable: number; +} + +/** + * The argument in that position at every call to that helper in this file. `new X(…)` is a call + * like any other here: the lookbehind sees the space after `new`, so a class needs no second rule. + */ +function helperFixSites( + masked: string, + source: string, + at: string, + helper: FixHelper, + lineAt: (index: number) => number, + unreadable: { count: number }, +): readonly FixSite[] { + const sites: FixSite[] = []; + // The lookbehind is `FIX_KEY`'s: `reporter.rejected(…)` is some other object's method. + const call = new RegExp(`(? n + part.length + 1, open + 1); + const literals = valueLiterals(masked, source, from, lineAt); + if (literals.length === 1) sites.push({ ...(literals[0] as FixSite), at }); + else unreadable.count += 1; + } + return sites; +} + +/** + * Every string a `fix:` can evaluate to. Searched over the masked source, so a `fix:` written + * inside a doc comment or interpolated into a message is not mistaken for a declaration. A `fix` + * computed at runtime — a bare identifier, a parameter, a table lookup with no literal fallback — + * has nothing to read and is beyond a static scan; the gate says so rather than guessing. + * + * Three shapes, because a fix does not always arrive under a key. `@ultimat3/mcp`'s + * `readonly-sql.ts` hands every one of its fixes positionally to a local `rejected(cause, fix)` + * helper, so the key rule alone returned `[]` for the whole file — 20 non-test files in that + * package and the scanner saw fixes in three — and two stale `x db branch ` lines shipped + * through the hole. + * + * `imported` is the third: the helpers this file can call that it did not declare, resolved by + * `fix-imports.ts` and passed in, because a scanner over one string cannot open a second file. + * The four rules a call site is read under are `helperAt`'s and do not change with where the + * declaration was found. + */ +export function scanFixSites( + source: string, + at: string, + imported: readonly FixHelper[] = [], +): FixScan { + const unreadable = { count: 0 }; + const masked = maskLiterals(source); + const lineAt = lineIndex(masked); + const sites: FixSite[] = []; + for (const key of masked.matchAll(FIX_KEY)) { + const start = key.index + key[0].length; + for (const literal of valueLiterals(masked, source, start, lineAt)) { + sites.push({ ...literal, at }); + } + } + // A name declared here wins over one imported under the same name: the declaration is what a + // call in this file actually reaches, and reading both would report one argument twice. + const local = scanFixHelpers(masked); + const names = new Set(local.map((helper) => helper.name)); + for (const helper of [...local, ...imported.filter((one) => !names.has(one.name))]) { + sites.push(...helperFixSites(masked, source, at, helper, lineAt, unreadable)); + } + return { sites, unreadable: unreadable.count }; +} + +/** The sites alone, for every caller that has no second file to resolve an import against. */ +export const scanFixes = ( + source: string, + at: string, + imported: readonly FixHelper[] = [], +): readonly FixSite[] => scanFixSites(source, at, imported).sites; diff --git a/packages/cli/src/flag-reads.test.ts b/packages/cli/src/flag-reads.test.ts new file mode 100644 index 00000000..20c11d6b --- /dev/null +++ b/packages/cli/src/flag-reads.test.ts @@ -0,0 +1,179 @@ +// One rule: every flag a command declares is read by something in the CLI's own source, the four +// global flags excepted. The last describe runs it over THIS build's registry rather than a +// fixture — a rule proved only against fixtures is a utility, and `x deploy --critical` shipped +// because nothing ever asked the shipped specs the question. + +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { checkFlagReads, declaredFlags, readsFlag } from './flag-reads'; +import type { CommandSpec } from './parse'; +import { SPECS } from './registry'; + +const spec = (name: string, flags: CommandSpec['flags']): CommandSpec => ({ + name, + summary: 's', + usage: `x ${name}`, + ...(flags === undefined ? {} : { flags }), +}); + +describe('declaredFlags', () => { + test('lists a command’s own flags with the command that declares them', () => { + const specs = [spec('deploy', [{ name: 'critical', type: 'boolean', summary: 'security' }])]; + expect(declaredFlags(specs)).toEqual([ + { command: 'deploy', flag: { name: 'critical', type: 'boolean', summary: 'security' } }, + ]); + }); + + // The parser and the dispatcher read these once, for every command. A per-command rule would + // report all thirty declarations of `--json` as unread and be turned off the same afternoon. + test('the global flags are the parser’s, and are never a command’s to read', () => { + const specs = [ + spec('deploy', [{ name: 'json', type: 'boolean', summary: 'machine-readable' }]), + ]; + expect(declaredFlags(specs)).toEqual([]); + }); +}); + +describe('readsFlag', () => { + test('a declaration is not a read', () => { + expect(readsFlag("{ name: 'critical', type: 'boolean', summary: 'x' }", 'critical')).toBe( + false, + ); + expect(readsFlag("{ short: 'j', name: 'json' }", 'j')).toBe(false); + }); + + test('any other occurrence of the bare literal is one', () => { + expect(readsFlag("flagBool(ctx.args, 'critical')", 'critical')).toBe(true); + // Read through a shared constant rather than by name at the call site — still read. + expect(readsFlag("const CRITICAL = 'critical';", 'critical')).toBe(true); + }); + + // The name inside a longer string is a citation, never a read: `x deploy --critical` in a `fix:` + // line is the flag being NAMED to a reader, which is the very case that shipped unimplemented. + test('the name inside a longer string is not a read', () => { + expect(readsFlag("fix: 'x deploy --critical --json'", 'critical')).toBe(false); + }); +}); + +describe('checkFlagReads', () => { + let root = ''; + + const write = async (path: string, text: string): Promise => { + await mkdir(join(root, path, '..'), { recursive: true }); + await writeFile(join(root, path), text); + }; + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'flag-reads-')); + }); + + afterEach(async () => { + await rm(root, { recursive: true, force: true }); + }); + + const DECLARED = "flags: [{ name: 'critical', type: 'boolean', summary: 'forces a reload' }]"; + + test('a flag nothing reads is a finding naming the file that declares it', async () => { + await write('cmd-deploy.ts', `export const deployCommand = { spec: { ${DECLARED} } };\n`); + const [finding, ...rest] = await checkFlagReads( + [spec('deploy', [{ name: 'critical', type: 'boolean', summary: 'forces a reload' }])], + root, + ); + expect(rest).toEqual([]); + expect(finding?.code).toBe('X_CLI_FLAG_UNREAD'); + expect(finding?.cause).toContain('x deploy declares --critical'); + expect(finding?.at).toContain('cmd-deploy.ts'); + expect(finding?.fix).toContain("flagBool(ctx.args, 'critical')"); + }); + + test('a reader anywhere in the source, not only in the declaring file, satisfies it', async () => { + await write('cmd-deploy.ts', `export const deployCommand = { spec: { ${DECLARED} } };\n`); + await write( + 'deploy-plan.ts', + "export const critical = (args) => flagBool(args, 'critical');\n", + ); + expect( + await checkFlagReads( + [spec('deploy', [{ name: 'critical', type: 'boolean', summary: 'forces a reload' }])], + root, + ), + ).toEqual([]); + }); + + // A flag named only in the prose above its spec is exactly the flag most likely to be dead. + test('a mention in a comment is not a reader', async () => { + await write( + 'cmd-deploy.ts', + `// 'critical' is handled below\nexport const deployCommand = { spec: { ${DECLARED} } };\n`, + ); + expect( + await checkFlagReads( + [spec('deploy', [{ name: 'critical', type: 'boolean', summary: 'forces a reload' }])], + root, + ), + ).toHaveLength(1); + }); + + test('a root that does not exist decides nothing, rather than raising ENOENT', async () => { + expect(await checkFlagReads(SPECS, join(root, 'nowhere', 'src'))).toEqual([]); + }); + + // The absent root is `Bun.Glob.scan`'s alone. A file the scan FOUND and this cannot read is a + // half of the source the rule did not see, and answering [] there is the false green the whole + // check exists to remove — so the read is outside the `try` and the failure travels. + // White-box on purpose: `Bun.file` is the reader, and no file a glob yields can be made + // unreadable without `chmod`, which decides differently as root and on a CI runner. + test('a file that cannot be read fails the check, it does not answer no findings', async () => { + await write('cmd-deploy.ts', `export const deployCommand = { spec: { ${DECLARED} } };\n`); + const bun: { file: typeof Bun.file } = Bun; + const real = bun.file; + bun.file = ((path: string) => ({ + text: (): Promise => Promise.reject(new Error(`EACCES: permission denied, ${path}`)), + })) as unknown as typeof Bun.file; + let raised: unknown; + try { + await checkFlagReads( + [spec('deploy', [{ name: 'critical', type: 'boolean', summary: 'forces a reload' }])], + root, + ); + } catch (error) { + raised = error; + } finally { + bun.file = real; + } + expect(raised).toBeInstanceOf(Error); + expect((raised as Error).message).toContain('EACCES'); + }); + + // A test asserting on a flag is not a command reading one; the suite is not shipped behaviour. + test('a test file is not a reader', async () => { + await write('cmd-deploy.ts', `export const deployCommand = { spec: { ${DECLARED} } };\n`); + await write('cmd-deploy.test.ts', "expect(flagBool(args, 'critical')).toBe(true);\n"); + expect( + await checkFlagReads( + [spec('deploy', [{ name: 'critical', type: 'boolean', summary: 'forces a reload' }])], + root, + ), + ).toHaveLength(1); + }); +}); + +// The rule applied to this build, which is what makes it a build error rather than a utility. +// It answers zero today: `--critical` IS read — `cmd-deploy.ts` writes it into the plan JSON — +// and what was unimplemented was the plan field's consumer, one level below any rule over names. +describe('this CLI', () => { + test('every flag every command declares is read by something', async () => { + expect(await checkFlagReads(SPECS, import.meta.dir)).toEqual([]); + }, 15_000); + + test('a root with no CLI source decides nothing, rather than reporting every flag', async () => { + // `tierBoundaries` runs against fixture roots that ship no `packages/cli/src`. Reporting all 30 + // declared flags there would be the false-positive direction; throwing ENOENT would be worse. + const empty = join(await mkdtemp(join(tmpdir(), 'flag-reads-')), 'src'); + await mkdir(empty, { recursive: true }); + + expect(await checkFlagReads(SPECS, empty)).toEqual([]); + }); +}); diff --git a/packages/cli/src/flag-reads.ts b/packages/cli/src/flag-reads.ts new file mode 100644 index 00000000..a1459dca --- /dev/null +++ b/packages/cli/src/flag-reads.ts @@ -0,0 +1,110 @@ +// A flag a command declares and nothing reads: `x deploy --critical` parsed, printed itself in +// `x help deploy`, and changed nothing about the deploy. The parser accepts every declared flag, +// so a flag with no reader is not a parse error and not a type error — it is a promise in the help +// text with no code behind it, and only a rule over the two halves together can see that. + +// `join`/`relative` are `node:`-only by necessity: Bun exposes no path-join primitive. +import { join, relative } from 'node:path'; +import { docsFor } from './error-codes'; +import type { Finding } from './output'; +import type { CommandSpec, FlagSpec } from './parse'; +import { GLOBAL_FLAGS } from './parse'; +import { stripComments } from './ts-scan'; + +/** A flag as declared, with the command that declares it. */ +export interface DeclaredFlag { + readonly command: string; + readonly flag: FlagSpec; +} + +/** + * Every flag a command declares. The global four are excluded: `--json`, `--help`, `--cwd` and + * `--verbose` are the parser's and the dispatcher's, read once for every command rather than by + * the command that lists them, and a per-command rule would report all 30 of them as unread. + */ +export function declaredFlags(specs: readonly CommandSpec[]): readonly DeclaredFlag[] { + const global = new Set(GLOBAL_FLAGS.map((flag) => flag.name)); + return specs.flatMap((spec) => + (spec.flags ?? []) + .filter((flag) => !global.has(flag.name)) + .map((flag) => ({ command: spec.name, flag })), + ); +} + +/** A flag name is `[a-z][a-z-]*`, so nothing in it is a regex metacharacter to escape. */ +const literalOf = (name: string): RegExp => new RegExp(`(['"\`])${name}\\1`, 'g'); + +/** + * The declaration itself, which is never a read. `{ name: 'critical', … }` and its `short:` twin + * are the two places the name appears as a spec field; every other occurrence of the bare literal + * is a reader — `flagBool(ctx.args, 'critical')`, a table key, a constant the reader indexes with. + */ +const declarationOf = (name: string): RegExp => + new RegExp(`(?:name|short)\\s*:\\s*(['"\`])${name}\\1`, 'g'); + +const countIn = (text: string, pattern: RegExp): number => [...text.matchAll(pattern)].length; + +/** + * Whether this file reads the flag, as against merely declaring it. Deliberately generous: a flag + * consumed only by being echoed into `--json`, or read through a shared constant rather than by + * name at the call site, is still read — the rule exists to catch a flag NOTHING mentions, and a + * gate that guessed at intent would report findings about working commands. + */ +export const readsFlag = (text: string, name: string): boolean => + countIn(text, literalOf(name)) > countIn(text, declarationOf(name)); + +const declaresFlag = (text: string, name: string): boolean => + countIn(text, declarationOf(name)) > 0; + +const unreadFinding = (declared: DeclaredFlag, at: string): Finding => ({ + code: 'X_CLI_FLAG_UNREAD', + cause: `x ${declared.command} declares --${declared.flag.name} ("${declared.flag.summary}") and no file in the CLI's source reads it, so the flag parses and changes nothing`, + fix: `read it in ${at} with flag${declared.flag.type === 'boolean' ? 'Bool' : 'String'}(ctx.args, '${declared.flag.name}'), or delete it from the spec's flags`, + docs: docsFor('X_CLI_FLAG_UNREAD'), + at, +}); + +/** + * Every declared flag held to one rule: something reads it. + * + * Scans source rather than the runtime, because "is this value ever consumed?" is not a question + * a `run` can be asked without running it — and running every command is not a check, it is the + * program. Comments are stripped first: a flag named only in the prose above the spec is not read, + * and a scanner that counted it would pass exactly the flags most likely to be dead. + */ +export async function checkFlagReads( + specs: readonly CommandSpec[], + srcDir: string, +): Promise { + const paths: string[] = []; + try { + for await (const path of new Bun.Glob('**/*.ts').scan({ cwd: srcDir, absolute: false })) { + if (!/\.test\.tsx?$/.test(path)) paths.push(path); + } + } catch { + // The directory is not there. `Bun.Glob.scan` raises rather than yielding nothing, so the + // absent case has to be caught here — see the `texts.size` guard below for why it answers []. + // The scan is ALL that is inside the `try`, deliberately: a file the scan found and this + // cannot read must propagate, or an unreadable source answers "no findings" and the rule + // reports green over the half it could not see. + return []; + } + const texts = new Map(); + for (const path of paths) { + texts.set(path, stripComments(await Bun.file(join(srcDir, path)).text())); + } + // No CLI source under this root: the rule holds two halves against each other and only one is + // here, so there is nothing it can decide. Derived, not "is this the framework repo" — the same + // condition `scripts/release-workflow.ts` uses for a tree with no publishable workspace. Scanning + // on would report EVERY declared flag as unread, which is the false-positive direction and the + // one that trains a reader to ignore the check. + if (texts.size === 0) return []; + const findings: Finding[] = []; + for (const declared of declaredFlags(specs)) { + const name = declared.flag.name; + if ([...texts.values()].some((text) => readsFlag(text, name))) continue; + const declaringFile = [...texts].find(([, text]) => declaresFlag(text, name))?.[0]; + findings.push(unreadFinding(declared, join(relative('', srcDir), declaringFile ?? ''))); + } + return findings; +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index dee2c4a7..19912583 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -117,12 +117,14 @@ export { } from './error-catalog'; export type { CliErrorCode } from './error-codes'; export { CLI_ERROR_CODES, CLI_ERROR_TITLES } from './error-codes'; +export type { ErrorFixReport } from './error-contract'; export { BANNED_PHRASES, COMMAND_TOKENS, checkErrorCodeDocs, checkErrorCodeRegistry, checkErrorFixes, + checkErrorFixReport, collectDeclaredCodes, documentedCodes, fixProblem, @@ -167,6 +169,12 @@ export { fixCitations, loadCommandCatalog, } from './fix-command'; +export type { HelperResolver } from './fix-imports'; +export { candidatePaths, createHelperResolver, scanImports } from './fix-imports'; +export type { FixHelper, FixScan } from './fix-scan'; +export { scanFixes, scanFixHelpers, scanFixSites } from './fix-scan'; +export type { DeclaredFlag } from './flag-reads'; +export { checkFlagReads, declaredFlags, readsFlag } from './flag-reads'; export type { Guard } from './guards'; export { findingProblem, GUARD_DIR, guardFindings, guardPaths } from './guards'; export type { DrainFailure, DrainOutcome, DrainSkip } from './jobs-drain'; @@ -241,7 +249,6 @@ export { scanBorrowedCodes, scanCodeFixSites, scanCodes, - scanFixes, stripComments, } from './ts-scan'; // The one spelling rule for a `references` entry. Exported because the two gate scripts ask the diff --git a/packages/cli/src/mcp-errors.ts b/packages/cli/src/mcp-errors.ts index 2e3da5ee..2114fdfd 100644 --- a/packages/cli/src/mcp-errors.ts +++ b/packages/cli/src/mcp-errors.ts @@ -22,6 +22,11 @@ const CLI_FIXES: Readonly> = { // Runnable first, the narrowing behind a `#`: `x help --json` pasted into a shell // is a redirect, not a command, and this table is copied verbatim by whoever reads it. X_CLI_BAD_FLAG: 'x help --json # then narrow to the command the cause names', + // Not an `x` command: this rule is about the CLI's OWN declarations, it can only fire in this + // repo, and the suite that applies it is what reproduces the finding. A placeholder command + // would fail this table's own no-`` rule, and rightly — it would not run. + X_CLI_FLAG_UNREAD: + 'bun test packages/cli/src/flag-reads.test.ts # the finding names the flag and the file to read it in', X_VERIFY_FAILED: 'x verify --json', X_NOT_IN_APP: 'x new myapp --json && cd myapp', X_BUN_VERSION: 'bun upgrade', diff --git a/packages/cli/src/messages.ts b/packages/cli/src/messages.ts index 559a5f73..8982949f 100644 --- a/packages/cli/src/messages.ts +++ b/packages/cli/src/messages.ts @@ -170,6 +170,9 @@ const CATALOG = { 'cli.verify.passSkipped': '{passed} of {count} steps passed in {ms}ms — {skipped} skipped: {names}', 'cli.verify.failSkipped': '{failed} of {count} steps failed — {skipped} skipped: {names}', + // The `errors` step's own coverage, in `output`: a scan without a parser reads most fix lines + // and not all of them, and a step that reports findings alone claims a completeness it lacks. + 'cli.verify.fixCoverage': 'checked {checked} fix line(s), could not read {unreadable}', 'cli.verify.serial': 'serial', 'cli.verify.workers': '{workers} workers', 'cli.env.checked': '{count} declared variable(s), all present and valid', diff --git a/packages/cli/src/templates/emitted-contract.test.ts b/packages/cli/src/templates/emitted-contract.test.ts index 3b92f49c..eb8656bc 100644 --- a/packages/cli/src/templates/emitted-contract.test.ts +++ b/packages/cli/src/templates/emitted-contract.test.ts @@ -12,8 +12,9 @@ import { generate } from '../cmd-generate'; import { planNewApp } from '../cmd-new'; import { fixProblem, staticFix } from '../error-contract'; import { citedCommandProblem, loadCommandCatalog } from '../fix-command'; +import { scanFixes } from '../fix-scan'; import { scaffoldVariants } from '../scaffold-fixture'; -import { scanFixes, stripComments } from '../ts-scan'; +import { stripComments } from '../ts-scan'; /** Four levels: `templates` → `src` → `cli` → `packages` → the repo root. */ const REPO_ROOT = join(import.meta.dir, '..', '..', '..', '..'); diff --git a/packages/cli/src/ts-scan.test.ts b/packages/cli/src/ts-scan.test.ts index 9b6f4694..71169d0e 100644 --- a/packages/cli/src/ts-scan.test.ts +++ b/packages/cli/src/ts-scan.test.ts @@ -4,13 +4,9 @@ import { maskLiterals, scanBorrowedCodes, scanCodes, - scanFixes, stripComments, } from './ts-scan'; -const fixes = (source: string): readonly string[] => - scanFixes(source, 'a.ts').map((site) => site.fix); - describe('stripComments', () => { test('blanks line and block comments but keeps line numbers', () => { const out = stripComments('const a = 1; // note\n/* two\nlines */\nconst b = 2;'); @@ -69,173 +65,6 @@ describe('maskLiterals', () => { }); }); -describe('scanFixes', () => { - test('reads a plain literal', () => { - expect(fixes("throw new E({ fix: 'x doctor --json' });")).toEqual(['x doctor --json']); - }); - - test('reads both branches of a ternary and a ?? default', () => { - expect(fixes("({ fix: hit ? 'x help' : 'x verify --json' })")).toEqual([ - 'x help', - 'x verify --json', - ]); - expect(fixes("({ fix: input.fix ?? 'x help' })")).toEqual(['x help']); - }); - - // The bug this guards: every literal in the expression used to count, so `.join(' ')`'s - // separator and `TABLE['key']`'s key were reported as empty and vague fix lines. - test('ignores literals nested inside a call or an index', () => { - expect(fixes("({ fix: command.join(' ') })")).toEqual([]); - expect(fixes("({ fix: FIXES['starttls'] ?? '' })")).toEqual(['']); - }); - - test('stops at the property that follows', () => { - expect(fixes("({ fix: 'x help', docs: 'https://ultimate.dev/errors/X_A' })")).toEqual([ - 'x help', - ]); - }); - - // The bug this guards: the quotes inside a regex like this file's own `CODE_LITERAL` read as - // string delimiters, so the masking desynced and blanked the `fix:` declared after it. The file - // then reported no fixes at all and the gate passed over error text nobody had checked. - test('a regex holding quote characters does not hide the declaration after it', () => { - const source = `const CODE_LITERAL = /(['"\`])(X_[A-Z0-9_]+)\\1/g; -throw new E({ fix: 'x doctor --json' });`; - expect(fixes(source)).toEqual(['x doctor --json']); - }); - - // Same masking desync, reached from JSX rather than from a regex: the file's `fix:` lines - // vanished wholesale, `x verify`'s `errors` step checked none of them, and `scanCodes` still - // found the code — so `X_ERROR_CODE_UNDOCUMENTED` passed and hid the hole. - test('an apostrophe in JSX text does not hide the declaration below it', () => { - const source = - 'export function Panel() {\n' + - " return

Don't panic

;\n" + - '}\n' + - "throw new E({ code: 'X_A', fix: 'x doctor --json' });"; - expect(scanFixes(source, 'a.tsx')).toEqual([{ at: 'a.tsx', line: 4, fix: 'x doctor --json' }]); - }); - - test('an escape and a character class do not close the regex early', () => { - expect(fixes("const re = /[/']|a\\/b/;\nthrow new E({ fix: 'x help' });")).toEqual(['x help']); - }); - - test('a division is read as one, so the declaration after it survives', () => { - expect(fixes("const per = total / count;\nthrow new E({ fix: 'x help' });")).toEqual([ - 'x help', - ]); - }); - - test('ignores a fix: written in a comment or interpolated into a message', () => { - expect(fixes('// fix: x db gen "add publish_at"\nconst a = 1;')).toEqual([]); - // biome-ignore lint/suspicious/noTemplateCurlyInString: the input is source text — a literal ${…} is the case under test - expect(fixes('const line = `${code}: ${cause} (fix: ${fix})`;')).toEqual([]); - }); - - // `cond ? e.fix : ''` is a ternary on a property named fix, not a fix declaration. - test('ignores a member access followed by a ternary colon', () => { - expect(fixes("const fix = typeof e.fix === 'string' ? e.fix : '';")).toEqual([]); - }); - - test('reports the line the literal is on, not the line the key is on', () => { - const source = "throw new E({\n code: 'X_A',\n fix:\n 'x help',\n});"; - expect(scanFixes(source, 'a.ts')[0]).toEqual({ at: 'a.ts', line: 4, fix: 'x help' }); - }); - - test('a runtime-computed fix has no literal to read', () => { - expect(fixes('({ fix: input.fix })')).toEqual([]); - expect(fixes('interface F { readonly fix: string }')).toEqual([]); - }); -}); - -/** - * The blind spot that let two stale fix lines ship. `@ultimat3/mcp`'s `readonly-sql.ts` passes its - * fixes POSITIONALLY into local `rejected(cause, fix)` / `notBranch(cause, fix)` helpers, so there - * is no `fix:` key anywhere in the file and the scanner returned `[]` for all of it — the citation - * resolver was never handed a single string to judge. - */ -describe('scanFixes · a fix passed positionally into a local error builder', () => { - const BUILDER = - 'function rejected(cause: string, fix: string) {\n' + - " return new McpError({ code: 'X_A', cause, fix });\n" + - '}\n'; - - test('reads the argument in the fix parameter position', () => { - expect(fixes(`${BUILDER}rejected('not one statement', 'x db branch ls --json');`)).toEqual([ - 'x db branch ls --json', - ]); - }); - - test('the position is the declared one, not "the last argument"', () => { - const source = - 'const fail = (field: string, fix: string, meta: object) => {\n' + - " throw new E({ code: 'X_B', cause: field, fix });\n" + - '};\n' + - "fail('amount', 'x g migration money', { a: 1 });"; - expect(fixes(source)).toEqual(['x g migration money']); - }); - - test('a helper that CONSUMES a fix is not a helper that declares one', () => { - // `citedCommandProblem(fix: string, catalog)` takes a fix in order to judge it. Reading its - // call sites as declarations would report findings about strings that are already findings — - // so a helper only counts when its body builds an error. - const source = - 'function citedCommandProblem(fix: string, catalog: Catalog) {\n' + - ' return catalog.resolve(fix);\n' + - '}\n' + - "citedCommandProblem('x db branch lst', catalog);"; - expect(fixes(source)).toEqual([]); - }); - - test('a rest or destructured parameter list has no reliable position, so it is skipped', () => { - // Both fixtures BUILD an error, or the builder discriminator would be what rejects them and - // these two cases would pass without the rules they exist to pin. - const rest = - 'function raise(kind: string, fix: string, ...rest: unknown[]) { throw new MyError({ code: kind, fix }); }\n' + - "raise('X_C', 'x doctor --json');"; - const destructured = - 'function raise({ kind, fix }: { kind: string; fix: string }) { throw new MyError({ code: kind, fix }); }\n' + - "raise({ kind: 'X_C', fix: 'x doctor --json' });"; - expect(fixes(rest)).toEqual([]); - // The object form needs no rule of its own: the `fix:` key at the call site is already read. - expect(fixes(destructured)).toEqual(['x doctor --json']); - }); - - test("a `{` further down the file is not this declaration's body", () => { - // The builder discriminator reads the helper's BODY, and `bodyOf` took the next `{` anywhere - // below it — so an expression-bodied helper that only formats a string was classified by - // whatever object literal happened to follow. Every call to it then handed the gate a string - // to judge as a fix, and a gate that fails on innocent source is worse than one that misses. - const source = - 'const label = (fix: string): string => fix.trim();\n' + - "const TITLES = { code: 'X_A' };\n" + - "label('x db branch lst');"; - expect(fixes(source)).toEqual([]); - }); - - test('a concise arrow body is still read, so the bound did not just switch the rule off', () => { - const source = - "const rejected = (cause: string, fix: string): Finding => ({ code: 'X_A', cause, fix });\n" + - "rejected('a', 'x doctor --json');"; - expect(fixes(source)).toEqual(['x doctor --json']); - }); - - test("a method call on some other object is not this file's helper", () => { - expect(fixes(`${BUILDER}reporter.rejected('a', 'x doctor --json');`)).toEqual([]); - }); - - test('an argument that is not a sole literal has nothing to read', () => { - expect(fixes(`${BUILDER}rejected('a', input.fix);`)).toEqual([]); - expect(fixes(`${BUILDER}rejected('a', prefix + 'x doctor');`)).toEqual([]); - expect(fixes(`${BUILDER}rejected('a');`)).toEqual([]); - }); - - test('the site carries the line the argument is on', () => { - const source = `${BUILDER}rejected(\n 'a',\n 'x doctor --json',\n);`; - expect(scanFixes(source, 'a.ts')).toEqual([{ at: 'a.ts', line: 6, fix: 'x doctor --json' }]); - }); -}); - describe('scanCodes', () => { test('collects throw sites anywhere', () => { expect(scanCodes("throw new E({ code: 'X_A', fix: 'x help' });", 'thing.ts')).toEqual([ diff --git a/packages/cli/src/ts-scan.ts b/packages/cli/src/ts-scan.ts index 98dafe91..70b2ac81 100644 --- a/packages/cli/src/ts-scan.ts +++ b/packages/cli/src/ts-scan.ts @@ -1,7 +1,8 @@ -// Reading two things out of TypeScript source without a parser: the strings a `fix:` can evaluate -// to, and the `X_*` codes a package declares. Deliberately not `tsc` — a regex over a masked file -// is the whole job. Masking is the load-bearing part: the contract's own 3-line rendering appears -// verbatim in doc blocks and template literals, and a scanner that reads it as code invents work. +// Reading TypeScript source without a parser: the masking every scan here shares, and the `X_*` +// codes a package declares. Deliberately not `tsc` — a regex over a masked file is the whole job. +// Masking is the load-bearing part: the contract's own 3-line rendering appears verbatim in doc +// blocks and template literals, and a scanner that reads it as code invents work. What a `fix:` +// can evaluate to is `fix-scan.ts`, which reads these primitives and is the only file that grew. export interface SourceSite { /** Repo-relative file the site was read from. */ @@ -18,9 +19,10 @@ export interface CodeSite extends SourceSite { readonly code: string; } -const QUOTES = new Set(["'", '"', '`']); -const OPENERS = new Set(['(', '[', '{']); -const CLOSERS = new Set([')', ']', '}']); +// `ReadonlySet`, so a consumer cannot mutate what every scan in this package reads. +export const QUOTES: ReadonlySet = new Set(["'", '"', '`']); +export const OPENERS: ReadonlySet = new Set(['(', '[', '{']); +export const CLOSERS: ReadonlySet = new Set([')', ']', '}']); const WORD = /[\w$]/; /** After one of these words a `/` opens a regex; after any other identifier it divides. */ @@ -36,7 +38,7 @@ const REGEX_AFTER_WORDS = new Set( * silently emptying the `errors` gate for the whole file. Same rule `endOfRegex` applies to a `/`. * An escaped newline is still a continuation: the escape is consumed before the line test. */ -function endOfLiteral(text: string, from: number): number { +export function endOfLiteral(text: string, from: number): number { const quote = text[from] as string; const spansLines = quote === '`'; for (let i = from + 1; i < text.length; i += 1) { @@ -132,7 +134,7 @@ export const maskLiterals = (text: string): string => blankRegions(text, true); * asking for a line per literal pays once per literal — measured at ~15s over the framework's own * package tree, against ~1s for the same walk with this. One offset table, then a binary search. */ -function lineIndex(text: string): (index: number) => number { +export function lineIndex(text: string): (index: number) => number { const newlines: number[] = []; for (let i = 0; i < text.length; i += 1) if (text[i] === '\n') newlines.push(i); return (index) => { @@ -153,7 +155,7 @@ function lineIndex(text: string): (index: number) => number { * instead of silently skipped; the depth rule is what keeps `command.join(' ')`'s separator and * `table['key']`'s key out — an argument is not a fix. */ -function valueLiterals( +export function valueLiterals( masked: string, source: string, from: number, @@ -182,170 +184,6 @@ function valueLiterals( })); } -/** The lookbehind rejects member access: `cond ? e.fix : ''` is a ternary, not a declaration. */ -const FIX_KEY = /(?` in the - // very same list, so counting it makes `(fn: () => void)` end the span in the wrong place. - if (OPENERS.has(ch)) depth += 1; - else if (CLOSERS.has(ch)) { - depth -= 1; - if (depth === 0) return masked.slice(open + 1, i); - } - } - return undefined; -} - -/** Split at depth-0 commas. Safe on masked text, where a comma inside a literal is already gone. */ -function topLevelParts(text: string): readonly string[] { - const parts: string[] = []; - let depth = 0; - let start = 0; - for (let i = 0; i < text.length; i += 1) { - const ch = text[i] as string; - if (OPENERS.has(ch)) depth += 1; - else if (CLOSERS.has(ch)) depth -= 1; - else if (ch === ',' && depth === 0) { - parts.push(text.slice(start, i)); - start = i + 1; - } - } - parts.push(text.slice(start)); - return parts; -} - -/** A local function that builds an error and takes its fix positionally, and where in its list. */ -interface FixHelper { - readonly name: string; - readonly index: number; -} - -const HELPER_DECL = - /(? fix.trim();` followed - * anywhere by a `{ code: … }` was read as an error builder and every `label(…)` call handed the - * gate a string to judge as a fix — a false gate failure over innocent source. - * - * The scan therefore ends at the `;` that ends the declaration, or at a bracket closing a scope - * this declaration is inside. Both directions of that bound answer `''`, which classifies the - * helper as a non-builder: a missed fix line costs one unchecked citation, a wrongly claimed one - * costs a build. A `{` inside a return-type annotation (`(): { ok: boolean } => …`) is read as the - * body and answers `''` for the same reason. - */ -function bodyOf(masked: string, after: number): string { - for (let i = after; i < masked.length; i += 1) { - const ch = masked[i] as string; - if (ch === '{') return bracketSpan(masked, i) ?? ''; - if (ch === ';' || CLOSERS.has(ch)) break; - } - return ''; -} - -function fixHelpers(masked: string): readonly FixHelper[] { - const helpers: FixHelper[] = []; - for (const declaration of masked.matchAll(HELPER_DECL)) { - const name = declaration[1] ?? declaration[2]; - const open = declaration.index + declaration[0].length - 1; - if (name === undefined || masked[open] !== '(') continue; - const params = bracketSpan(masked, open); - // A rest parameter makes the position of everything after it unknowable, and a destructured - // one has no position at all — its `fix:` key at the CALL site is already read by `FIX_KEY`. - if (params === undefined || params.includes('...')) continue; - const parts = topLevelParts(params); - if (parts.some((part) => /^\s*[[{]/.test(part))) continue; - const index = parts.findIndex((part) => FIX_PARAM.test(part)); - if (index === -1) continue; - if (!BUILDS_ERROR.test(bodyOf(masked, open + params.length + 2))) continue; - helpers.push({ name, index }); - } - return helpers; -} - -/** - * The argument in that position at every call to that helper IN THIS FILE. - * - * Same file, deliberately: resolving `dbNotImplemented` imported from `@ultimat3/db` would mean a - * cross-file symbol table, and a scanner that guessed at which import a name came from would read - * an unrelated function's argument as a fix. The gap that leaves is named in `CLAUDE.md`. - */ -function helperFixSites( - masked: string, - source: string, - at: string, - helper: FixHelper, - lineAt: (index: number) => number, -): readonly FixSite[] { - const sites: FixSite[] = []; - // The lookbehind is `FIX_KEY`'s: `reporter.rejected(…)` is some other object's method. - const call = new RegExp(`(? n + part.length + 1, open + 1); - const literals = valueLiterals(masked, source, from, lineAt); - if (literals.length === 1) sites.push({ ...(literals[0] as FixSite), at }); - } - return sites; -} - -/** - * Every string a `fix:` can evaluate to. Searched over the masked source, so a `fix:` written - * inside a doc comment or interpolated into a message is not mistaken for a declaration. A `fix` - * computed at runtime — a bare identifier, a parameter, a table lookup with no literal fallback — - * has nothing to read and is beyond a static scan; the gate says so rather than guessing. - * - * Two shapes, because a fix does not always arrive under a key. `@ultimat3/mcp`'s `readonly-sql.ts` - * hands every one of its fixes positionally to a local `rejected(cause, fix)` helper, so the key - * rule alone returned `[]` for the whole file — 20 non-test files in that package and the scanner - * saw fixes in three — and two stale `x db branch ` lines shipped through the hole. - */ -export function scanFixes(source: string, at: string): readonly FixSite[] { - const masked = maskLiterals(source); - const lineAt = lineIndex(masked); - const sites: FixSite[] = []; - for (const key of masked.matchAll(FIX_KEY)) { - const start = key.index + key[0].length; - for (const literal of valueLiterals(masked, source, start, lineAt)) { - sites.push({ ...literal, at }); - } - } - for (const helper of fixHelpers(masked)) { - sites.push(...helperFixSites(masked, source, at, helper, lineAt)); - } - return sites; -} - const CODE_TABLE = /\bexport const [A-Z][A-Z0-9_]*_ERROR_(?:CODES|TITLES)\b/; /** diff --git a/packages/core/src/image/errors.ts b/packages/core/src/image/errors.ts index 3624c39d..745f2dc5 100644 --- a/packages/core/src/image/errors.ts +++ b/packages/core/src/image/errors.ts @@ -42,7 +42,7 @@ export const imageDecodeFailed = ( ): ImageDecodeFailedError => new ImageDecodeFailedError( cause, - 'check the file is a complete, uncorrupted image: `file ` then re-export it', + 're-export the image from its source: `file ` reports what these bytes actually are', meta, ); diff --git a/packages/i18n/CLAUDE.md b/packages/i18n/CLAUDE.md index a0cca509..a0f1f767 100644 --- a/packages/i18n/CLAUDE.md +++ b/packages/i18n/CLAUDE.md @@ -22,6 +22,11 @@ Imported by every package that renders a string. `registerCatalog` / `configureLocales` are its internals — an app calling them is a second path. - Reading: `t('ns.key')` for one string, `useI18n()` where the keys must be typed. There is no `currentTranslator`; `useI18n` replaced it. +- **`configureLocales` is process-global and MERGES, so `resetLocaleConfig()` is the only way back.** + `defineCatalogs()` calls it at an app's module scope; a module evaluates once per `bun test` process, + so one file that loads an app narrowed `supported` for every file after it and `Accept-Language: de-DE` + negotiated `en` in a file that never mentioned locales. No partial call can widen the set back. The + test harness restores it at each file boundary (`@ultimat3/testing`'s `registry-snapshot.ts`). - A miss renders `⟦key⟧`. Never add a fallback locale chain — it hides gaps. - Only an **own** property of `vars` is a variable — `interpolate` guards with `Object.hasOwn`. A plain object inherits `constructor`, `toString`, `valueOf` and `__proto__`, so a bare diff --git a/packages/i18n/src/context.test.ts b/packages/i18n/src/context.test.ts index 32411940..0ea53629 100644 --- a/packages/i18n/src/context.test.ts +++ b/packages/i18n/src/context.test.ts @@ -1,14 +1,22 @@ -import { beforeEach, describe, expect, test } from 'bun:test'; +/** + * Guards the locale-configuration RESET seam: `configureLocales` is process-global and merges, so + * a wrong reset is a wrong `` in every later file of the same `bun test` process. + */ +import { afterEach, beforeEach, describe, expect, test } from 'bun:test'; import { flattenCatalog } from './catalog'; import { + configureLocales, currentLocale, + localeConfig, localeCookieOf, registerCatalog, resetCatalogs, + resetLocaleConfig, resolveLocale, t, useI18n, } from './context'; +import { DEFAULT_LOCALE, SUPPORTED_LOCALES } from './locales'; const supported = ['en', 'es', 'de'] as const; @@ -105,3 +113,76 @@ describe('ambient translator', () => { expect(useI18n()('errors.notFound.title')).toBe('Lost?'); }); }); + +describe('resetLocaleConfig', () => { + afterEach(() => { + resetLocaleConfig(); + }); + + // The leak this exists to close: `defineCatalogs()` runs at an APP's module scope, so one test + // that loads an app narrows `supported` for every later file of the same `bun test` process — + // and `` answers `en` to `Accept-Language: de-DE` in a file that never mentioned + // locales. `configureLocales` merges, so no partial call can widen the set back. + test('puts the shipped supported set back, which a partial configureLocales cannot', () => { + configureLocales({ supported: ['en'], fallback: 'en' }); + expect(localeConfig().supported).toEqual(['en']); + + resetLocaleConfig(); + + expect(localeConfig().supported).toEqual(SUPPORTED_LOCALES); + expect(localeConfig().fallback).toBe(DEFAULT_LOCALE); + }); + + // The documented behaviour the leak was corrupting, asserted on its own: with `de` registered, + // a `de-DE` header negotiates `de`. Read through the ambient config — no `overrides` argument — + // because the overrides argument is exactly what hides a corrupted module-level config. + test('a de-DE header negotiates de again once the narrowing is undone', () => { + configureLocales({ supported: ['en'], fallback: 'en' }); + expect(resolveLocale({ header: 'de-DE,de;q=0.9,en;q=0.7' }).locale).toBe('en'); + + resetLocaleConfig(); + + expect(resolveLocale({ header: 'de-DE,de;q=0.9,en;q=0.7' })).toEqual({ + locale: 'de', + direction: 'ltr', + source: 'header', + }); + }); + + // A "default" shared by reference is not a default. `localeConfig()` hands out the LIVE object, + // and that object used to BE `DEFAULT_LOCALE_CONFIG` — so one caller writing through it corrupted + // the value the reset restores FROM, and every later reset replayed the corruption. + test('a caller that mutates the live config cannot corrupt what the reset restores', () => { + const shipped = [...SUPPORTED_LOCALES]; + expect(shipped.length).toBeGreaterThan(1); + + // The cast is the point: a `readonly` annotation stops a compiler, not the JS caller this + // seam exists for. + const live = localeConfig() as unknown as { + supported: string[]; + fallback: string; + order: string[]; + }; + live.supported.length = 0; + live.order.length = 0; + live.fallback = 'zz'; + + resetLocaleConfig(); + + expect(localeConfig().supported).toEqual(shipped); + expect(localeConfig().fallback).toBe(DEFAULT_LOCALE); + // The arrays too, and `SUPPORTED_LOCALES` is a module export half the framework reads. + expect(SUPPORTED_LOCALES).toEqual(shipped); + // Precedence back as behaviour, not only as a field. + expect(resolveLocale({ header: 'en-US,en;q=0.9', cookie: 'es' }).source).toBe('cookie'); + }); + + test('the source precedence is restored too, not only the supported set', () => { + configureLocales({ order: ['header'] }); + expect(resolveLocale({ header: 'en-US,en;q=0.9', cookie: 'es' }).source).toBe('header'); + + resetLocaleConfig(); + + expect(resolveLocale({ header: 'en-US,en;q=0.9', cookie: 'es' }).source).toBe('cookie'); + }); +}); diff --git a/packages/i18n/src/context.ts b/packages/i18n/src/context.ts index 45e23ecd..9cce9556 100644 --- a/packages/i18n/src/context.ts +++ b/packages/i18n/src/context.ts @@ -54,12 +54,30 @@ export interface LocaleConfig { */ const DEFAULT_ORDER: readonly LocaleSourceName[] = ['query', 'cookie', 'user', 'header']; -let config: LocaleConfig = { +/** Hoisted so `resetLocaleConfig` has one value to name, rather than a second literal to drift. */ +const DEFAULT_LOCALE_CONFIG: LocaleConfig = { supported: SUPPORTED_LOCALES, fallback: DEFAULT_LOCALE, order: DEFAULT_ORDER, }; +/** + * A fresh object AND fresh arrays, every time. Handing out `DEFAULT_LOCALE_CONFIG` itself made the + * live config the shipped default, so one caller writing through `localeConfig()` corrupted the + * value `resetLocaleConfig()` restores FROM — and the reset replayed the corruption for the rest of + * the process. `readonly` in the type stops a compiler, not a caller, and this seam exists because + * callers do what the types did not expect. + */ +function freshDefaultConfig(): LocaleConfig { + return { + supported: [...DEFAULT_LOCALE_CONFIG.supported], + fallback: DEFAULT_LOCALE_CONFIG.fallback, + order: [...DEFAULT_LOCALE_CONFIG.order], + }; +} + +let config: LocaleConfig = freshDefaultConfig(); + /** Called once at boot from `app.config.ts`. */ export function configureLocales(partial: Partial): LocaleConfig { config = { ...config, ...partial }; @@ -173,6 +191,19 @@ export function t(key: string, vars?: TranslateVars): string { return useI18n()(key, vars); } +/** + * Test/CLI seam: back to the shipped supported set, fallback and precedence. + * + * `defineCatalogs()` calls `configureLocales()` at an APP's module scope, and a module evaluates + * once per `bun test` process — so one file that loads an app narrows `supported` for every file + * after it, and `Accept-Language: de-DE` negotiates `en` in a file that never mentioned locales. + * `configureLocales` MERGES, so no partial call can widen the set back; only a value the framework + * owns can, which is why this is a reset and not a documented "remember to restore it". + */ +export function resetLocaleConfig(): void { + config = freshDefaultConfig(); +} + /** Test/CLI seam: drop every registered catalog. */ export function resetCatalogs(): void { registry.clear(); diff --git a/packages/i18n/src/index.ts b/packages/i18n/src/index.ts index fa0f6635..73f21c8d 100644 --- a/packages/i18n/src/index.ts +++ b/packages/i18n/src/index.ts @@ -26,6 +26,7 @@ export { registerCatalog, registeredLocales, resetCatalogs, + resetLocaleConfig, resolveLocale, t, translatorFor, diff --git a/packages/policy/CLAUDE.md b/packages/policy/CLAUDE.md index 5a8d937a..975bba5a 100644 --- a/packages/policy/CLAUDE.md +++ b/packages/policy/CLAUDE.md @@ -51,6 +51,11 @@ two differ, and it is why a surface that decides on input alone needs no edit. everything while nothing is declared, so the first declaration anywhere in the process turns strict checking on for everyone. A test that uses `can()` declares the set it uses and restores the one it found — never leans on the empty registry. +- **`clearPermissions()` / `clearRoles()` are one-way; `restorePermissions()` / `restoreRoles()` are the + other halves.** Both declaration calls run at MODULE scope, and a module evaluates once per `bun test` + process — so a clear in one test file is permanent for every file after it, whose own `import` is a + cache hit that declares nothing. `restoreRoles` takes the declaration sites too: `defineRoles()` derives + them from the CALLER's stack, so restoring through it would make `X_ROLE_REDEFINED` name the harness. - **`not()` never inverts `X_UNAUTHENTICATED`.** A null actor is not a fact about this actor's grants; inverting it makes `not(can('order:internal'))` a public door into the internal one. Any denial carrying that code propagates unchanged. diff --git a/packages/policy/src/index.ts b/packages/policy/src/index.ts index 778162ea..1c07f7bb 100644 --- a/packages/policy/src/index.ts +++ b/packages/policy/src/index.ts @@ -42,6 +42,7 @@ export { isKnownPermission, knownPermissions, resourceOf, + restorePermissions, verbOf, } from './permissions'; export type { @@ -70,6 +71,7 @@ export { defineRoles, expandRoles, grantMatches, + restoreRoles, roleDeclarationSites, roleDefinitions, roleMapGeneration, diff --git a/packages/policy/src/permissions.test.ts b/packages/policy/src/permissions.test.ts index bd2dfb33..d10d08d8 100644 --- a/packages/policy/src/permissions.test.ts +++ b/packages/policy/src/permissions.test.ts @@ -10,6 +10,7 @@ import { isKnownPermission, knownPermissions, resourceOf, + restorePermissions, verbOf, } from './permissions'; @@ -122,3 +123,43 @@ describe('clearPermissions()', () => { expect(isKnownPermission('post:anything')).toBe(true); }); }); + +describe('restorePermissions()', () => { + // `clearPermissions()` is one-way, and `definePermissions()` runs at MODULE scope — + // `@ultimat3/admin` declares `admin:*` on its barrel's import. A module evaluates once per + // `bun test` process, so a clear in one file is permanent for every file after it: the later + // file's own `await import('@ultimat3/admin')` is a cache hit that registers nothing, and its + // `can('admin:read')` throws X_PERMISSION_UNKNOWN for a permission the process did declare. + test('puts back a set clearPermissions destroyed, which re-importing cannot', () => { + definePermissions(['admin:read', 'admin:write']); + const captured = knownPermissions(); + + clearPermissions(); + expect(knownPermissions()).toEqual([]); + + restorePermissions(captured); + expect(knownPermissions()).toEqual(['admin:read', 'admin:write']); + expect(isKnownPermission('admin:read')).toBe(true); + }); + + test('replaces rather than merges — a captured set is the whole truth about the process', () => { + definePermissions(['post:read']); + const captured = knownPermissions(); + definePermissions(['org:admin']); + + restorePermissions(captured); + + expect(knownPermissions()).toEqual(['post:read']); + }); + + test('an empty capture restores emptiness, which is what "nothing was declared" means', () => { + const captured = knownPermissions(); + definePermissions(['post:read']); + + restorePermissions(captured); + + expect(knownPermissions()).toEqual([]); + // Empty is the "no app has declared its set" state, where every string is admitted. + expect(isKnownPermission('anything:at-all')).toBe(true); + }); +}); diff --git a/packages/policy/src/permissions.ts b/packages/policy/src/permissions.ts index cb4142e0..74e15a12 100644 --- a/packages/policy/src/permissions.ts +++ b/packages/policy/src/permissions.ts @@ -75,3 +75,18 @@ export const definePermissions = ( /** Test seam; production never forgets a permission it declared. */ export const clearPermissions = (): void => declared.clear(); + +/** + * `clearPermissions()`'s other half, taking exactly what `knownPermissions()` answers. + * + * `definePermissions()` runs at MODULE scope — `@ultimat3/admin` declares `admin:*` on its + * barrel's import — and a module evaluates once per `bun test` process. So a clear in one test + * file is permanent for every file after it: that file's own `import` is a cache hit which + * registers nothing, and `can('admin:read')` throws X_PERMISSION_UNKNOWN for a permission the + * process really did declare. Only putting the captured set back repairs it; re-importing cannot. + * Replaces rather than merges — a capture is the whole truth about the process, not an addition. + */ +export const restorePermissions = (permissions: readonly string[]): void => { + declared.clear(); + for (const permission of permissions) declared.add(permission); +}; diff --git a/packages/policy/src/roles.test.ts b/packages/policy/src/roles.test.ts index ac46a7a1..28c1b1fd 100644 --- a/packages/policy/src/roles.test.ts +++ b/packages/policy/src/roles.test.ts @@ -9,7 +9,10 @@ import { defineRoles, expandRoles, grantMatches, + type RoleDef, type RoleMap, + restoreRoles, + roleDeclarationSites, roleDefinitions, roleMapGeneration, rolesGranting, @@ -186,3 +189,56 @@ describe('rolesGranting()', () => { expect(rolesGranting('x:y')).toEqual([]); }); }); + +describe('restoreRoles()', () => { + // Same one-way hazard as `clearPermissions()`: `defineRoles()` runs at an app's MODULE scope, + // and a module evaluates once per `bun test` process, so a `clearRoles()` in one test file is + // permanent for every file after it — a later `import` is a cache hit that declares nothing. + test('puts back a map clearRoles destroyed, which re-importing cannot', () => { + defineRoles(roles); + const captured = roleDefinitions(); + const capturedSites = roleDeclarationSites(); + + clearRoles(); + expect(roleDefinitions()).toEqual({}); + + restoreRoles(captured, capturedSites); + + expect(roleDefinitions()).toEqual(roles); + expect(expandRoles(['editor'])).toEqual(['post:publish', 'post:read']); + }); + + // `defineRoles()` cannot be the restore: it re-derives the declaration site from the CALLER's + // stack, so every role would report the harness as its origin and X_ROLE_REDEFINED would name + // a frame no reader can act on. + test('keeps each role declared where it was actually declared', () => { + defineRoles({ viewer: roles['viewer'] as RoleDef }); + const captured = roleDefinitions(); + const capturedSites = roleDeclarationSites(); + expect(capturedSites['viewer']).toContain('roles.test.ts'); + + clearRoles(); + restoreRoles(captured, capturedSites); + + expect(roleDeclarationSites()['viewer']).toBe(capturedSites['viewer'] as string); + }); + + // The memo in `grant-index.ts` is invalidated by the generation alone, so a restore that did + // not bump it would leave a flattened grant set computed against the cleared map. + test('bumps the generation, or the grant memo answers from the map it just replaced', () => { + defineRoles(roles); + const captured = roleDefinitions(); + + clearRoles(); + // Read AFTER the clear: `clearRoles()` bumps too, so a baseline taken before it passes on a + // `restoreRoles` that never bumps at all. + const before = roleMapGeneration(); + restoreRoles(captured, roleDeclarationSites()); + + expect(roleMapGeneration()).toBeGreaterThan(before); + expect(actorPermissions({ id: 'u1', roles: ['editor'] })).toEqual([ + 'post:publish', + 'post:read', + ]); + }); +}); diff --git a/packages/policy/src/roles.ts b/packages/policy/src/roles.ts index bd14edfa..059f7397 100644 --- a/packages/policy/src/roles.ts +++ b/packages/policy/src/roles.ts @@ -106,6 +106,23 @@ export const clearRoles = (): void => { generation += 1; }; +/** + * `clearRoles()`'s other half, taking what `roleDefinitions()` and `roleDeclarationSites()` + * answer. `defineRoles()` runs at an app's MODULE scope and a module evaluates once per + * `bun test` process, so a clear in one test file is permanent for every file after it — the + * later file's `import` is a cache hit that declares nothing. + * + * `defineRoles()` cannot be the restore: it re-derives the declaration site from the CALLER's + * stack, so every role would report this frame as its origin and `X_ROLE_REDEFINED` would name a + * site no reader can open. The generation is bumped for `grant-index.ts`, whose per-actor memo is + * invalidated by that number alone. + */ +export const restoreRoles = (map: RoleMap, declaredAt: Readonly>): void => { + roleMap = { ...map }; + sites = { ...declaredAt }; + generation += 1; +}; + /** * Depth-first expansion with a visited set: `owner -> admin -> editor` collapses to * one list, and `a -> b -> a` terminates instead of blowing the stack. diff --git a/packages/render/src/render-static.test.ts b/packages/render/src/render-static.test.ts index 1257315d..eb33960d 100644 --- a/packages/render/src/render-static.test.ts +++ b/packages/render/src/render-static.test.ts @@ -248,7 +248,14 @@ describe('renderStatic', () => { await expect(renderStatic(entry, render, { buildId: 'b1' })).rejects.toMatchObject({ code: 'X_PRERENDER_FAILED', message: expect.stringContaining('rendering /blog/b failed:'), - fix: expect.stringContaining('x build --route /blog/b'), + // The fix names a command this build can run: `x build` declares target/tag/out and no + // --route, so the old line was an instruction that fails — caught once the errors gate + // learned to read a `fix:` handed to a factory in a sibling module (#157). + fix: expect.stringMatching( + // Both halves: the command has to be one this build runs, AND it has to name the route that + // failed — a fix that reproduces the wrong page reproduces nothing. + /x build --target static --json[\s\S]*\/blog\/b/, + ), }); }); diff --git a/packages/render/src/render-static.ts b/packages/render/src/render-static.ts index 61c522a6..d1b0e30c 100644 --- a/packages/render/src/render-static.ts +++ b/packages/render/src/render-static.ts @@ -120,7 +120,7 @@ export async function renderStatic( } catch (error) { throw new PrerenderFailedError( `rendering ${path} failed: ${describe(error)}`, - `run \`x build --route ${path}\` to reproduce, then fix ${entry.file}`, + `x build --target static --json # reproduces ${path}, then fix ${entry.file}`, ); } const hash = contentHash(html); diff --git a/packages/testing/CLAUDE.md b/packages/testing/CLAUDE.md index 9a9571e2..b790e8e2 100644 --- a/packages/testing/CLAUDE.md +++ b/packages/testing/CLAUDE.md @@ -36,6 +36,7 @@ is its own entry point and not part of the barrel. | Registry hygiene | the fixture registry is process-global; a test that clears it snapshots with `fixtureSnapshot()` and hands it back in `afterAll` | | Leaks are the file's, not the next file's | `installRegistryLeakGuard()` runs from the preload and fails the run naming the FILE that left cache tags declared or a cache tier registered after its last test (`X_TEST_REGISTRY_LEAK`). `bun test` is one process, so without it the failure lands on an innocent suite in another package. What a file's MODULE graph declares is its environment; what the file installs after that is its own to undo | | The baseline is not a hook | measured on Bun 1.3.14 the order is onLoad → module eval → file `beforeAll` → describe `beforeAll` → preload `beforeEach`, so a preload hook cannot sample before the file's own `beforeAll` — a `declareTags()` there read as environment and the run went green. The load handler appends the sample to the file's source instead: after evaluation, before any hook the file registers. It is also the only signal carrying file identity, which `bun:test` hooks do not | +| Reported and restored are different sets | the guard also RESTORES, at the same file boundary, the registries whose module-scope declarations a neighbour's cleanup destroys — the locale config, the catalogs, the permission set and the role map (`registry-snapshot.ts`). A module evaluates once per process, so a later file's own `import` is a cache hit that declares nothing: `clearPermissions()` in one CLI test took `admin:*` from `@ultimat3/admin`'s barrel for the whole run, and a `defineCatalogs()` inside a loaded app narrowed `supported` so `Accept-Language: de-DE` answered `en` in files that never mentioned locales. Nothing restored is reported and nothing reported is restored — a repair followed by a failure over it would be two answers to one question | | Guarded state is boot state | only the two registries whose honest invariant is "clean when the file ends" — `declareTags` and `registerTier` are boot installs. `entity()`, `job()` and `defineRoute()` register at MODULE scope, which is how an app declares itself, so a filled registry there is idiomatic and unguarded | | An empty registry is a premise you state | a test whose subject is "nothing is declared" — `x db gen` with nothing to generate — calls `isolateEntityRegistry()` and restores in a `finally`. Inheriting it means the test passes until a neighbouring file imports an entity | | That one helper is off the barrel | `@ultimat3/testing/registry-isolation`, its own entry point. It is the only module here that value-imports `@ultimat3/entity` — the restore is handed back synchronously, so it cannot be a dynamic import inside the call — and a static re-export from `src/index.ts` would load the entity registry into every test that imports this package for `expect` | diff --git a/packages/testing/README.md b/packages/testing/README.md index cbf9d561..50f39fd8 100644 --- a/packages/testing/README.md +++ b/packages/testing/README.md @@ -21,7 +21,8 @@ frozen clock. Never let a test reach the network unmocked — it fails by design | `fixture-{clock,mail,jobs,network,statements}.ts` | the five fixtures the framework builds in-process | | `fixture-drivers.ts` | the five it declares but a driver must build — `page` `budget` `signIn` `deploy` `subscribe` | | `framework-fixtures.ts` | registers both sets; the app registers only what it owns | -| `registry-leak-guard.ts` | fails the run naming the FILE that left a process-global registry dirty | +| `registry-leak-guard.ts` | fails the run naming the FILE that left a process-global registry dirty, and restores the ones that can be restored at the same boundary | +| `registry-snapshot.ts` | `captureProcessRegistries()` / `restoreProcessRegistries()` — the locale config, the catalogs, the permission set and the role map, put back as a file inherited them. A module-scope declaration evaluates once per process (`bun test` without `--isolate`, `As of 2026-08`), so a neighbour's `clearPermissions()` is otherwise permanent | | `registry-isolation.ts` | `isolateEntityRegistry()` — an empty entity registry, and the process's back after. Its own entry point (`@ultimat3/testing/registry-isolation`), never the barrel: it value-imports `@ultimat3/entity`, and the barrel is what a tier-0 test imports for `expect` | | `preload.ts` | the bunfig preload that installs all of the above | diff --git a/packages/testing/package.json b/packages/testing/package.json index 1bb6765f..f3947bc8 100644 --- a/packages/testing/package.json +++ b/packages/testing/package.json @@ -37,8 +37,10 @@ "@ultimat3/core": "3.0.0", "@ultimat3/db": "3.0.0", "@ultimat3/entity": "3.0.0", + "@ultimat3/i18n": "3.0.0", "@ultimat3/jobs": "3.0.0", "@ultimat3/mail": "3.0.0", + "@ultimat3/policy": "3.0.0", "@ultimat3/time": "3.0.0" } } diff --git a/packages/testing/src/index.ts b/packages/testing/src/index.ts index 797ab200..e5238021 100644 --- a/packages/testing/src/index.ts +++ b/packages/testing/src/index.ts @@ -110,6 +110,8 @@ export { matchersInstalled, recordSteps } from './matchers'; // so it gets its own entry point instead. export type { RegistryLeak, RegistrySample } from './registry-leak-guard'; export { installRegistryLeakGuard, leakBetween, sampleRegistries } from './registry-leak-guard'; +export type { ProcessRegistrySnapshot } from './registry-snapshot'; +export { captureProcessRegistries, restoreProcessRegistries } from './registry-snapshot'; export type { MockRoute, NetworkState } from './sealed-network'; // `setNetworkState` is deliberately not here: it is the offline gate's one writer, and a test that // called it directly would bypass the `network` fixture's disposal and leave the whole process diff --git a/packages/testing/src/registry-leak-guard.test.ts b/packages/testing/src/registry-leak-guard.test.ts index 1fb2e41d..e3369c0c 100644 --- a/packages/testing/src/registry-leak-guard.test.ts +++ b/packages/testing/src/registry-leak-guard.test.ts @@ -115,6 +115,8 @@ describe('RegistryLeakError', () => { const SRC = import.meta.dir; const PRELOAD = join(SRC, 'preload.ts'); const CACHE = join(SRC, '..', '..', 'cache', 'src', 'index.ts'); +const I18N = join(SRC, '..', '..', 'i18n', 'src', 'index.ts'); +const POLICY = join(SRC, '..', '..', 'policy', 'src', 'index.ts'); /** * Absolute specifiers: the fixture lives in a temp dir with no `node_modules` to resolve through. @@ -208,3 +210,57 @@ describe('the guard, across files, in one process', () => { expect(output).toContain('bootfixture'); }, 30_000); }); + +/** + * The declaration a module makes ONCE. Both fixtures below import it, and a module evaluates once + * per process — so whichever file bun happens to run second gets a cache hit that declares + * nothing, and inherits whatever the first file left. That is the whole defect, in three lines. + */ +const DECLARE = `import { registerCatalog } from '${I18N}'; +import { definePermissions } from '${POLICY}'; + +export const permissions = definePermissions(['leak:probe']); +registerCatalog('en', { 'probe.title': 'Probe' }); +`; + +/** + * Deliberately IDENTICAL, so the assertion cannot depend on which one bun runs first: each file + * declares by import, asserts the declaration holds, and then does what a well-behaved suite here + * already does in cleanup — `clearPermissions()` (six CLI test files) and a narrowing + * `defineCatalogs()` (every test that loads an app). The second file to run is the victim. + */ +const restoreFixture = (name: string): string => `import { afterAll, expect, test } from 'bun:test'; +import { catalogFor, configureLocales, resetCatalogs, resolveLocale } from '${I18N}'; +import { clearPermissions, knownPermissions } from '${POLICY}'; +import './declare'; + +test('${name} sees the process-scope declarations it inherited', () => { + expect(knownPermissions()).toContain('leak:probe'); + expect(catalogFor('en')['probe.title']).toBe('Probe'); + // \`de\` is in the framework's shipped set, so this is \`de\` unless a file narrowed it. + expect(resolveLocale({ header: 'de-DE,de;q=0.9,en;q=0.7' }).locale).toBe('de'); +}); + +afterAll(() => { + clearPermissions(); + resetCatalogs(); + configureLocales({ supported: ['en'], fallback: 'en' }); +}); +`; + +describe('the restore, across files, in one process', () => { + // Not writable as an ordinary test: the leak only exists BETWEEN files of one process, and the + // module cache is what makes it unrepairable from inside the victim. `registry-snapshot.test.ts` + // pins the arithmetic; this pins that the guard actually applies it at the boundary. + test('a file that clears a module-scope declaration does not take it from the next file', async () => { + const { output, exitCode } = await runFixtures({ + 'declare.ts': DECLARE, + 'first.test.ts': restoreFixture('first'), + 'second.test.ts': restoreFixture('second'), + }); + + expect(output).toContain('2 pass'); + expect(output).not.toContain('1 fail'); + expect(exitCode).toBe(0); + }, 30_000); +}); diff --git a/packages/testing/src/registry-leak-guard.ts b/packages/testing/src/registry-leak-guard.ts index 5fee4564..2874f183 100644 --- a/packages/testing/src/registry-leak-guard.ts +++ b/packages/testing/src/registry-leak-guard.ts @@ -1,19 +1,26 @@ -// Cross-file state pollution, caught at the boundary it crosses. `bun test` runs every file of one -// invocation in ONE process — only `--isolate` gives each file its own module registry — so a file -// that leaves a process-global registry dirty changes what every file after it sees, and the -// failure lands on an innocent suite in another package. This names the file that leaked. +// Cross-file state pollution, caught at the boundary it crosses and — where a registry can be put +// back — repaired there. `bun test` runs one invocation in ONE process, so a file that leaves a +// process-global registry dirty changes what every file after it sees and the failure lands on an +// innocent suite in another package. What is REPORTED and what is RESTORED are disjoint sets. import { afterAll } from 'bun:test'; import { knownTags, registeredTiers } from '@ultimat3/cache'; import { RegistryLeakError } from './errors'; +import type { ProcessRegistrySnapshot } from './registry-snapshot'; +import { captureProcessRegistries, restoreProcessRegistries } from './registry-snapshot'; /** - * What is guarded, and why only these two. Both are BOOT installs — `declareTags` takes the + * What is REPORTED, and why only these two. Both are BOOT installs — `declareTags` takes the * manifest's entity names, `registerTier` takes `app.config.ts`'s tiers — so "empty again when the * file ends" is the honest invariant for a test. The entity, job, route and permission registries * are not here: `entity()` and `job()` register at module scope, which is how an app declares * itself, so a file that leaves them filled is idiomatic rather than leaky. A test whose subject is * an EMPTY one of those establishes it itself — `isolateEntityRegistry()`. + * + * Neither is RESTORED, and that is the same judgement read the other way: `@ultimat3/cache` + * publishes no un-declare for a tag, so there is nothing to put a tag registry back WITH. The + * registries that are restored are `registry-snapshot.ts`'s, and none of them is reported — + * repairing a state and then failing the run over it would be two answers to one question. */ export interface RegistrySample { readonly tags: readonly string[]; @@ -87,13 +94,23 @@ export function installRegistryLeakGuard(): void { installed = true; let pending: string | undefined; - let current: { readonly file: string; readonly before: RegistrySample } | undefined; + let current: + | { + readonly file: string; + readonly before: RegistrySample; + readonly snapshot: ProcessRegistrySnapshot; + } + | undefined; const leaks: RegistryLeak[] = []; const close = (): void => { if (current === undefined) return; const leak = leakBetween(current.file, current.before, sampleRegistries()); if (leak !== undefined) leaks.push(leak); + // The repair, at the only point it is safe: the file is over and the next one has not + // evaluated yet, so what goes back is exactly what that file inherited — module-scope + // declarations included, which is the half a plain `resetX()` in a `beforeEach` destroys. + restoreProcessRegistries(current.snapshot); current = undefined; }; @@ -102,7 +119,11 @@ export function installRegistryLeakGuard(): void { // how an app declares its tags — and everything after this point is the file's own to undo. hookHost[BASELINE_HOOK] = () => { if (pending === undefined) return; - current = { file: pending, before: sampleRegistries() }; + current = { + file: pending, + before: sampleRegistries(), + snapshot: captureProcessRegistries(), + }; pending = undefined; }; diff --git a/packages/testing/src/registry-snapshot.test.ts b/packages/testing/src/registry-snapshot.test.ts new file mode 100644 index 00000000..3176a432 --- /dev/null +++ b/packages/testing/src/registry-snapshot.test.ts @@ -0,0 +1,153 @@ +import { describe, expect, test } from 'bun:test'; +import { flattenCatalog } from '../../i18n/src/catalog'; +import { + catalogFor, + configureLocales, + localeConfig, + registerCatalog, + registeredLocales, + resetCatalogs, + resetLocaleConfig, + resolveLocale, +} from '../../i18n/src/context'; +import { + clearPermissions, + definePermissions, + knownPermissions, +} from '../../policy/src/permissions'; +import { + clearRoles, + defineRoles, + roleDeclarationSites, + roleDefinitions, +} from '../../policy/src/roles'; +import { captureProcessRegistries, restoreProcessRegistries } from './registry-snapshot'; + +/** Every test here mutates process globals; each one hands them back the way it found them. */ +const around = (body: () => void): void => { + const outer = captureProcessRegistries(); + try { + body(); + } finally { + restoreProcessRegistries(outer); + } +}; + +describe('captureProcessRegistries / restoreProcessRegistries', () => { + test('a locale set narrowed after the capture is wide again after the restore', () => + around(() => { + resetLocaleConfig(); + const snapshot = captureProcessRegistries(); + + // What `defineCatalogs({ default: 'en', locales: { en, fr } })` does at an app's module + // scope, which is why one test that loads an app decided `` for every file + // after it in the same process. + configureLocales({ supported: ['en', 'fr'], fallback: 'en' }); + expect(resolveLocale({ header: 'de-DE,de;q=0.9,en;q=0.7' }).locale).toBe('en'); + + restoreProcessRegistries(snapshot); + + // The documented behaviour the leak corrupted: `de` is registered, so `de-DE` is `de`. + expect(resolveLocale({ header: 'de-DE,de;q=0.9,en;q=0.7' })).toEqual({ + locale: 'de', + direction: 'ltr', + source: 'header', + }); + })); + + test('permissions a later file cleared are declared again after the restore', () => + around(() => { + // Stands in for `@ultimat3/admin`'s barrel, which declares `admin:*` at module scope — the + // registration a `clearPermissions()` in another file destroys for the whole process. + definePermissions(['admin:read', 'admin:write']); + const snapshot = captureProcessRegistries(); + + clearPermissions(); + expect(knownPermissions()).toEqual([]); + + restoreProcessRegistries(snapshot); + + expect(knownPermissions()).toEqual(expect.arrayContaining(['admin:read', 'admin:write'])); + })); + + test('permissions declared after the capture are gone after the restore', () => + around(() => { + clearPermissions(); + const snapshot = captureProcessRegistries(); + + definePermissions(['post:read']); + expect(knownPermissions()).toEqual(['post:read']); + + restoreProcessRegistries(snapshot); + + expect(knownPermissions()).toEqual([]); + })); + + test('the role map and its declaration sites both come back', () => + around(() => { + defineRoles({ editor: { grants: ['post:publish'] } }); + // The site is THIS file, captured before the clear: it is what makes `X_ROLE_REDEFINED` + // name the app's own declaration rather than whatever frame restored the map. + const declaredAt = roleDeclarationSites()['editor']; + expect(declaredAt).toBeString(); + const snapshot = captureProcessRegistries(); + + clearRoles(); + expect(roleDefinitions()).toEqual({}); + expect(roleDeclarationSites()).toEqual({}); + + restoreProcessRegistries(snapshot); + + expect(roleDefinitions()['editor']).toEqual({ grants: ['post:publish'] }); + expect(roleDeclarationSites()['editor']).toBe(declaredAt); + })); + + test('a catalog registered after the capture is gone, and one captured is back', () => + around(() => { + resetCatalogs(); + registerCatalog('en', flattenCatalog({ nav: { home: 'Home' } })); + const snapshot = captureProcessRegistries(); + + resetCatalogs(); + registerCatalog('fr', flattenCatalog({ nav: { home: 'Accueil' } })); + + restoreProcessRegistries(snapshot); + + expect(registeredLocales()).toEqual(['en']); + expect(catalogFor('en')['nav.home']).toBe('Home'); + })); + + test('a snapshot survives later writes — nothing captured is held by reference', () => + around(() => { + resetLocaleConfig(); + clearPermissions(); + clearRoles(); + const snapshot = captureProcessRegistries(); + + // Each of these replaces the module-level value rather than mutating it, so the capture + // must still describe the process as it was. A capture that aliased the live object would + // restore whatever the last writer left. + configureLocales({ supported: ['en'] }); + definePermissions(['post:read']); + defineRoles({ editor: { grants: ['post:publish'] } }); + + expect(snapshot.locales.supported.length).toBeGreaterThan(1); + expect(snapshot.permissions).toEqual([]); + expect(snapshot.roles).toEqual({}); + })); + + test('restoring is idempotent — the same snapshot applied twice is one state', () => + around(() => { + resetLocaleConfig(); + clearPermissions(); + definePermissions(['post:read']); + const snapshot = captureProcessRegistries(); + + definePermissions(['org:admin']); + restoreProcessRegistries(snapshot); + restoreProcessRegistries(snapshot); + + expect(knownPermissions()).toEqual(['post:read']); + expect(localeConfig().supported).toEqual(snapshot.locales.supported); + })); +}); diff --git a/packages/testing/src/registry-snapshot.ts b/packages/testing/src/registry-snapshot.ts new file mode 100644 index 00000000..4744ea1f --- /dev/null +++ b/packages/testing/src/registry-snapshot.ts @@ -0,0 +1,61 @@ +// The process registries a test file inherits, captured and handed back at the file boundary; +// `registry-leak-guard.ts` owns WHEN. A snapshot rather than a reset to defaults, because what a +// module declares at MODULE scope evaluates once per `bun test` process — a neighbour's clear is +// permanent and there is no second evaluation left to redo it. + +import type { Catalog, Locale, LocaleConfig } from '@ultimat3/i18n'; +import { + catalogFor, + configureLocales, + localeConfig, + registerCatalog, + registeredLocales, + resetCatalogs, +} from '@ultimat3/i18n'; +import type { RoleMap } from '@ultimat3/policy'; +import { + knownPermissions, + restorePermissions, + restoreRoles, + roleDeclarationSites, + roleDefinitions, +} from '@ultimat3/policy'; + +/** + * Every member is captured by value or by a reference its owner replaces rather than mutates + * (`configureLocales`, `defineRoles` and `registerCatalog` all build a new object and assign it), + * so a snapshot describes the process at the instant it was taken and no later write reaches it. + */ +export interface ProcessRegistrySnapshot { + readonly locales: LocaleConfig; + readonly catalogs: readonly (readonly [Locale, Catalog])[]; + readonly permissions: readonly string[]; + readonly roles: RoleMap; + /** Kept beside the map: restoring through `defineRoles()` would rewrite every site. */ + readonly roleSites: Readonly>; +} + +export function captureProcessRegistries(): ProcessRegistrySnapshot { + return { + locales: localeConfig(), + catalogs: registeredLocales().map((locale) => [locale, catalogFor(locale)] as const), + permissions: knownPermissions(), + roles: roleDefinitions(), + roleSites: roleDeclarationSites(), + }; +} + +/** + * Idempotent, and a REPLACE on every registry rather than a merge: a snapshot is the whole truth + * about the process at capture time, so anything declared since must go as surely as anything + * cleared since must come back. + */ +export function restoreProcessRegistries(snapshot: ProcessRegistrySnapshot): void { + // A full `LocaleConfig`, so the merge `configureLocales` performs replaces all three fields — + // a partial call can never widen `supported` back. + configureLocales(snapshot.locales); + resetCatalogs(); + for (const [locale, catalog] of snapshot.catalogs) registerCatalog(locale, catalog); + restorePermissions(snapshot.permissions); + restoreRoles(snapshot.roles, snapshot.roleSites); +} diff --git a/packages/testing/tsconfig.json b/packages/testing/tsconfig.json index 33d2ed7c..ca134063 100644 --- a/packages/testing/tsconfig.json +++ b/packages/testing/tsconfig.json @@ -20,12 +20,18 @@ { "path": "../entity" }, + { + "path": "../i18n" + }, { "path": "../jobs" }, { "path": "../mail" }, + { + "path": "../policy" + }, { "path": "../time" } diff --git a/scripts/lib/run.ts b/scripts/lib/run.ts index 60af66dc..94430a23 100644 --- a/scripts/lib/run.ts +++ b/scripts/lib/run.ts @@ -64,5 +64,11 @@ export const repoRoot = (): string => new URL('../..', import.meta.url).pathname * twice: the second round of failures happened because CI shards SIX ways while the local * reproduction used EIGHT, so a different test crossed the line each time and no local run had * ever seen the one that broke main. A per-site number is a per-site guess. + * + * 30s -> 90s on 2026-08-19, a third time and for a real reason rather than flake: the `errors` + * step's fix scan now RESOLVES cross-file helpers (#157) and the `boundaries` step now reads every + * file under `packages/cli/src` to check that a declared flag has a reader (#161). Both scans grew, + * both are the point of their test, and each takes ~5s alone against ~30s under eight competing + * workers. Raise this, never narrow a scan, and never delete a test for being slow. */ -export const REPO_SCAN_TIMEOUT_MS = 30_000; +export const REPO_SCAN_TIMEOUT_MS = 90_000; diff --git a/scripts/registry-audit.test.ts b/scripts/registry-audit.test.ts new file mode 100644 index 00000000..5cba58c3 --- /dev/null +++ b/scripts/registry-audit.test.ts @@ -0,0 +1,189 @@ +// The audit's four answers and its one non-answer, driven by fixtures. The network is injected on +// purpose: a test that resolves npm reports the registry's mood, not this repo's state — and the +// measured propagation lag (minutes, PUBLISHING.md) makes "absent" the flakiest possible fixture. + +import { describe, expect, test } from 'bun:test'; +import { ScriptError } from './lib/script-error'; +import type { PublishState, RegistryFetch } from './registry-audit'; +import { + auditRegistry, + findingFor, + ordinalLabel, + packumentUrl, + registryFindings, +} from './registry-audit'; + +/** An empty audit must fail the test loudly, never hand `findingFor` a fabricated state. */ +const only = (states: readonly PublishState[]): PublishState => { + const first = states[0]; + if (first === undefined || states.length !== 1) { + throw new ScriptError({ + code: 'X_CLI_UNEXPECTED', + cause: `the audit answered ${states.length} states where the fixture supplies one`, + fix: 'pass exactly one target to auditRegistry() in scripts/registry-audit.test.ts', + }); + } + return first; +}; + +const attested = { + dist: { tarball: 'https://registry.npmjs.org/x.tgz', attestations: { url: 'https://x/att' } }, + _npmUser: { name: 'GitHub Actions' }, +}; + +const handPublished = { + dist: { tarball: 'https://registry.npmjs.org/x.tgz' }, + _npmUser: { name: 'sebyx07' }, +}; + +const packument = (name: string, versions: Record, latest: string): string => + JSON.stringify({ name, 'dist-tags': { latest }, versions }); + +/** Answers per URL; anything unlisted is a 404, which is what the registry does. */ +const fetcherFor = (bodies: Record): RegistryFetch => { + return (url) => { + const answer = bodies[url]; + if (answer === undefined) return Promise.resolve(new Response('{}', { status: 404 })); + return Promise.resolve(new Response(answer.body, { status: answer.status ?? 200 })); + }; +}; + +const target = (name: string, version = '3.0.0'): { name: string; version: string } => ({ + name, + version, +}); + +describe('a network failure is never reported as absent', () => { + test('a fetch that throws is unreachable, and says so', async () => { + const thrown: RegistryFetch = () => Promise.reject(new Error('ETIMEDOUT')); + const states = await auditRegistry([target('@ultimat3/core')], thrown); + expect(states[0]?.kind).toBe('unreachable'); + const finding = findingFor(only(states)); + expect(finding?.code).toBe('X_REGISTRY_UNREACHABLE'); + expect(finding?.cause).toContain('ETIMEDOUT'); + // The whole point of the separate outcome: nobody may read a timeout as a bootstrap. + expect(finding?.cause).toContain('not evidence'); + expect(finding?.fix).not.toContain('npm publish'); + }); + + test('a rate limit is unreachable, not absent', async () => { + const limited = fetcherFor({ + [packumentUrl('@ultimat3/core')]: { body: '{"error":"rate limit"}', status: 429 }, + }); + const states = await auditRegistry([target('@ultimat3/core')], limited); + expect(states[0]?.kind).toBe('unreachable'); + expect(findingFor(only(states))?.cause).toContain('429'); + }); + + test('a 200 that is not a packument is unreachable, not present', async () => { + const html = fetcherFor({ [packumentUrl('@ultimat3/core')]: { body: 'maintenance' } }); + expect((await auditRegistry([target('@ultimat3/core')], html))[0]?.kind).toBe('unreachable'); + // Valid JSON, and still not a packument — the shape guard, not the parse, is what catches it. + const nul = fetcherFor({ [packumentUrl('@ultimat3/core')]: { body: 'null' } }); + expect((await auditRegistry([target('@ultimat3/core')], nul))[0]?.kind).toBe('unreachable'); + }); +}); + +describe('absent from the registry names the ordinal and the cost', () => { + test('the third of four says a run publishes two before it dies', async () => { + const names = ['a', 'b', 'c', 'd'].map((n) => target(`@ultimat3/${n}`)); + const present = Object.fromEntries( + names + .filter((entry) => entry.name !== '@ultimat3/c') + .map((entry) => [ + packumentUrl(entry.name), + { body: packument(entry.name, { '3.0.0': attested }, '3.0.0') }, + ]), + ); + const findings = registryFindings(await auditRegistry(names, fetcherFor(present))); + expect(findings).toHaveLength(1); + const finding = findings[0]; + expect(finding?.code).toBe('X_REGISTRY_BOOTSTRAP_OWED'); + expect(finding?.cause).toContain('3rd of 4'); + expect(finding?.cause).toContain('publishes 2'); + // The bootstrap command PUBLISHING.md step 1 names, verbatim — never a re-run of the workflow. + expect(finding?.fix).toContain('npm publish -w @ultimat3/c --access public --provenance=false'); + }); + + test('ordinals read as English, 11th through 13th included', () => { + expect([1, 2, 3, 4, 11, 12, 13, 21, 27, 30].map(ordinalLabel)).toEqual([ + '1st', + '2nd', + '3rd', + '4th', + '11th', + '12th', + '13th', + '21st', + '27th', + '30th', + ]); + }); +}); + +describe('present but behind the stamped version', () => { + test('the registry holding only the previous version is behind, not ok', async () => { + const behind = fetcherFor({ + [packumentUrl('@ultimat3/core')]: { + body: packument('@ultimat3/core', { '2.0.0': attested }, '2.0.0'), + }, + }); + const states = await auditRegistry([target('@ultimat3/core', '3.0.0')], behind); + expect(states[0]?.kind).toBe('behind'); + const finding = findingFor(only(states)); + expect(finding?.code).toBe('X_REGISTRY_VERSION_BEHIND'); + expect(finding?.cause).toContain('2.0.0'); + expect(finding?.cause).toContain('3.0.0'); + expect(finding?.fix).toContain('gh workflow run release.yml --ref v3.0.0 -f version=3.0.0'); + }); +}); + +describe('present at the stamped version, hand-published', () => { + test('no dist.attestations is a finding that names who published it', async () => { + const unattested = fetcherFor({ + [packumentUrl('@ultimat3/core')]: { + body: packument('@ultimat3/core', { '2.0.0': handPublished }, '2.0.0'), + }, + }); + const states = await auditRegistry([target('@ultimat3/core', '2.0.0')], unattested); + expect(states[0]?.kind).toBe('unattested'); + const finding = findingFor(only(states)); + expect(finding?.code).toBe('X_REGISTRY_UNATTESTED'); + expect(finding?.cause).toContain('sebyx07'); + expect(finding?.fix).toContain('bun run scripts/trust-publishers.ts'); + }); + + test('an attested version is ok and produces no finding', async () => { + const good = fetcherFor({ + [packumentUrl('@ultimat3/core')]: { + body: packument('@ultimat3/core', { '3.0.0': attested }, '3.0.0'), + }, + }); + const states = await auditRegistry([target('@ultimat3/core')], good); + expect(states[0]?.kind).toBe('ok'); + expect(states[0]?.publishedBy).toBe('GitHub Actions'); + expect(findingFor(only(states))).toBeUndefined(); + expect(registryFindings(states)).toHaveLength(0); + }); +}); + +describe('the request itself', () => { + test('a scoped name is escaped for the packument path', () => { + expect(packumentUrl('@ultimat3/core')).toBe('https://registry.npmjs.org/@ultimat3%2fcore'); + }); + + test('a request that never answers aborts on the timeout, as unreachable', async () => { + const signals: AbortSignal[] = []; + // Answers only when aborted, which is what a hung connection does — so this passes only while + // the request carries a REAL deadline. A bare controller here hangs the whole audit. + const hangs: RegistryFetch = (_url, init) => + new Promise((_resolve, reject) => { + signals.push(init.signal); + init.signal.addEventListener('abort', () => reject(new Error('aborted by the deadline'))); + }); + const states = await auditRegistry([target('@ultimat3/core')], hangs, 5); + expect(states[0]?.kind).toBe('unreachable'); + expect(findingFor(only(states))?.cause).toContain('aborted by the deadline'); + expect(signals[0]).toBeInstanceOf(AbortSignal); + }); +}); diff --git a/scripts/registry-audit.ts b/scripts/registry-audit.ts new file mode 100644 index 00000000..aaefaf8e --- /dev/null +++ b/scripts/registry-audit.ts @@ -0,0 +1,228 @@ +#!/usr/bin/env bun +// Compare the REGISTRY to this tree: for every publishable workspace, is it on npm at all, is it +// there at the version this tree stamps, and was that version published by the workflow. Nothing +// else asks — `scripts/release-workflow.ts` proves the workflow NAMES every package, which says +// nothing about whether npm holds it, and that gap is why the docs described a publish state npm +// had not been in for two releases (`flags`, then `scraping`). +// +// An OPERATOR command, not a `x verify` step, and it must not become one — the same rule +// `scripts/trust-publishers.ts` states for the same reason: the gate is the shippability contract +// and runs on free CI runners, so a step that resolves npm makes green depend on a network the +// runner does not control. Read-only always: it fetches packuments and publishes nothing, so there +// is no write mode to gate behind a `--check`. +// +// bun run scripts/registry-audit.ts [--json] + +import { parseScriptArgs } from './lib/args'; +import type { Finding } from './lib/log'; +import { report } from './lib/log'; +import { repoRoot } from './lib/run'; +import { listWorkspaces, publishOrder } from './lib/workspaces'; + +export const REGISTRY = 'https://registry.npmjs.org'; + +/** One packument is a small JSON document; a slow one must not hold the other 29 hostage. */ +export const REQUEST_TIMEOUT_MS = 15_000; + +/** The seam. Injected so the tests drive fixtures — a test that resolves npm is a flake. */ +export type RegistryFetch = ( + url: string, + init: { readonly signal: AbortSignal }, +) => Promise; + +/** What the audit needs of a workspace: its published name and the version this tree stamps. */ +export interface AuditTarget { + readonly name: string; + readonly version: string; +} + +/** + * `unreachable` is a first-class answer, never folded into `absent`: npm rate-limits, and its + * public packument lagged a real publish by MINUTES (PUBLISHING.md), so a request that did not + * answer is not a package that is not there — and the fix for one is an irreversible publish. + */ +export type PublishStateKind = 'ok' | 'absent' | 'behind' | 'unattested' | 'unreachable'; + +export interface PublishState extends AuditTarget { + /** 1-based position in `publishOrder`, which is the order a release run publishes in. */ + readonly ordinal: number; + readonly total: number; + readonly kind: PublishStateKind; + /** `dist-tags.latest`, when the registry answered. */ + readonly latest?: string; + /** `_npmUser` on the stamped version — `GitHub Actions` for a workflow publish. */ + readonly publishedBy?: string; + /** Only for `unreachable`: how the request failed, in words a reader can act on. */ + readonly detail?: string; +} + +/** + * `@ultimat3/core` -> `@ultimat3%2fcore`. Only the separator is escaped: that is the path npm's own + * client builds, and `encodeURIComponent` on the whole name would escape the `@` too. + */ +export const packumentUrl = (name: string): string => `${REGISTRY}/${name.replace('/', '%2f')}`; + +/** 27th, not 27nd. The ordinal is what makes the cost of a missing bootstrap legible. */ +export function ordinalLabel(n: number): string { + const teens = n % 100; + if (teens >= 11 && teens <= 13) return `${n}th`; + return `${n}${['th', 'st', 'nd', 'rd'][n % 10] ?? 'th'}`; +} + +interface VersionRecord { + readonly dist?: { readonly attestations?: unknown }; + readonly _npmUser?: { readonly name?: unknown } | string; +} + +interface Packument { + readonly 'dist-tags'?: { readonly latest?: unknown }; + readonly versions?: Readonly>; +} + +/** `_npmUser` is an object on a version record and a bare string in older documents. */ +function publisherOf(record: VersionRecord | undefined): string | undefined { + const user = record?._npmUser; + if (typeof user === 'string') return user; + const name = typeof user === 'object' && user !== null ? user.name : undefined; + return typeof name === 'string' ? name : undefined; +} + +/** + * Total by construction, so the one place an audit touches a value it did not create cannot throw + * while describing the throw. No `String(failure)` and no bare interpolation: both die on a Proxy. + */ +function failureDetail(failure: unknown): string { + if (failure instanceof Error && typeof failure.message === 'string') return failure.message; + return 'the request failed with no message'; +} + +export type Lookup = + | { readonly kind: 'found'; readonly packument: Packument } + | { readonly kind: 'absent' } + | { readonly kind: 'unreachable'; readonly detail: string }; + +/** One packument, or the reason there is none. Every non-404 failure is `unreachable`. */ +export async function lookup( + name: string, + fetcher: RegistryFetch, + timeoutMs: number, +): Promise { + try { + const response = await fetcher(packumentUrl(name), { signal: AbortSignal.timeout(timeoutMs) }); + if (response.status === 404) return { kind: 'absent' }; + if (!response.ok) return { kind: 'unreachable', detail: `HTTP ${response.status}` }; + // A 200 carrying an error page is not a package with no versions; it is no answer at all. + const body: unknown = await response.json(); + if (typeof body !== 'object' || body === null) { + return { kind: 'unreachable', detail: 'the 200 body is not a packument object' }; + } + return { kind: 'found', packument: body as Packument }; + } catch (failure) { + return { kind: 'unreachable', detail: failureDetail(failure) }; + } +} + +export function classify( + target: AuditTarget, + seat: Pick, + found: Lookup, +): PublishState { + // Projected field by field, never spread: a caller passes a whole `Workspace`, and spreading it + // would put this machine's absolute `path` into the `--json` an operator pastes into an issue. + const base = { name: target.name, version: target.version, ...seat }; + if (found.kind === 'absent') return { ...base, kind: 'absent' }; + if (found.kind === 'unreachable') return { ...base, kind: 'unreachable', detail: found.detail }; + const tag = found.packument['dist-tags']?.latest; + const latest = typeof tag === 'string' ? tag : 'none'; + const record = found.packument.versions?.[target.version]; + if (record === undefined) return { ...base, kind: 'behind', latest }; + const publishedBy = publisherOf(record); + const attested = record.dist?.attestations !== undefined && record.dist.attestations !== null; + return { + ...base, + kind: attested ? 'ok' : 'unattested', + latest, + ...(publishedBy === undefined ? {} : { publishedBy }), + }; +} + +const defaultFetch: RegistryFetch = (url, init) => fetch(url, init); + +/** Sequential on purpose: 30 small GETs, and a burst is what npm rate-limits. */ +export async function auditRegistry( + targets: readonly AuditTarget[], + fetcher: RegistryFetch = defaultFetch, + timeoutMs: number = REQUEST_TIMEOUT_MS, +): Promise { + const states: PublishState[] = []; + for (const [index, target] of targets.entries()) { + const found = await lookup(target.name, fetcher, timeoutMs); + states.push(classify(target, { ordinal: index + 1, total: targets.length }, found)); + } + return states; +} + +const absentFinding = (state: PublishState): Finding => ({ + code: 'X_REGISTRY_BOOTSTRAP_OWED', + at: state.name, + cause: `${state.name} is publishable and the registry has no such package — it is ${ordinalLabel(state.ordinal)} of ${state.total} in the publish order, so a release run publishes ${state.ordinal - 1} packages irreversibly and then dies on this one`, + fix: `npm publish -w ${state.name} --access public --provenance=false # PUBLISHING.md step 1, the one-time bootstrap; then bun run scripts/trust-publishers.ts`, +}); + +const behindFinding = (state: PublishState): Finding => ({ + code: 'X_REGISTRY_VERSION_BEHIND', + at: state.name, + cause: `this tree stamps ${state.name} at ${state.version} and the registry's newest is ${state.latest ?? 'none'}, so the release that should have published ${state.version} did not reach this package`, + fix: `gh workflow run release.yml --ref v${state.version} -f version=${state.version} # when the run aborts EPUBLISHCONFLICT on a sibling already at ${state.version}, publish this one alone: npm publish -w ${state.name} --access public --provenance=false`, +}); + +const unattestedFinding = (state: PublishState): Finding => ({ + code: 'X_REGISTRY_UNATTESTED', + at: state.name, + cause: `${state.name}@${state.version} is on the registry with no dist.attestations, published by ${state.publishedBy ?? 'an account the packument does not name'} rather than by release.yml — so nothing proves the tarball a consumer installs was built from this repo`, + fix: `bun run scripts/trust-publishers.ts --json # attach the OIDC publisher, then release the next version through the workflow (bun run scripts/release.ts --bump patch); npm publishes are immutable, so ${state.version} itself can never gain an attestation`, +}); + +const unreachableFinding = (state: PublishState): Finding => ({ + code: 'X_REGISTRY_UNREACHABLE', + at: state.name, + cause: `the registry did not answer for ${state.name}: ${state.detail ?? 'no detail'} — this is not evidence that ${state.name} is unpublished`, + fix: `curl -sS -o /dev/null -w '%{http_code}\\n' ${packumentUrl(state.name)} # then re-run: bun run scripts/registry-audit.ts --json`, +}); + +const FINDINGS: Readonly Finding) | undefined>> = + { + ok: undefined, + absent: absentFinding, + behind: behindFinding, + unattested: unattestedFinding, + unreachable: unreachableFinding, + }; + +export const findingFor = (state: PublishState): Finding | undefined => + FINDINGS[state.kind]?.(state); + +export const registryFindings = (states: readonly PublishState[]): readonly Finding[] => + states.map(findingFor).filter((finding): finding is Finding => finding !== undefined); + +if (import.meta.main) { + const args = parseScriptArgs(Bun.argv.slice(2)); + const targets = publishOrder(await listWorkspaces(repoRoot())); + const states = await auditRegistry(targets); + const findings = registryFindings(states); + const attested = states.filter((state) => state.kind === 'ok').length; + const version = targets[0]?.version ?? 'unknown'; + report( + { + ok: findings.length === 0, + script: 'registry-audit', + summary: + findings.length === 0 + ? `${attested}/${states.length} publishable packages are on npm at ${version}, every one attested` + : `${findings.length} registry gap(s) across ${states.length} publishable packages`, + findings, + data: { registry: REGISTRY, states }, + }, + args.json, + ); +} diff --git a/scripts/verify.ts b/scripts/verify.ts index 8c964956..cc5ee52e 100644 --- a/scripts/verify.ts +++ b/scripts/verify.ts @@ -6,16 +6,19 @@ // // bun run scripts/verify.ts [--json] [--verbose] +import { join } from 'node:path'; import type { HostCheck, VerifyStepName } from '@ultimat3/cli'; import { checkErrorCodeDocs, checkErrorCodeRegistry, + checkFlagReads, collectDeclaredCodes, exec, exitCodeFor, registeredErrorCodes, render, runVerify, + SPECS, VERIFY_STEPS, } from '@ultimat3/cli'; import { benchClaimFindings } from './bench-claims'; @@ -77,6 +80,12 @@ export const tierBoundaries: HostCheck = async (root) => [ ...checkAdminFlattener(await collectAdminFiles(root)).map(adminFlattenerFindingFor), ...(await frameworkCatalogFindings(root)), ...(await imageContractFindings(root)), + // The CLI's own declarations, held to each other: a flag the parser accepts that no file reads is + // a promise `x help` prints with nothing behind it. `x deploy --critical` said "forces clients to + // reload" and reached no reader outside the plan JSON. Host-side, because a generated app ships no + // `packages/cli/src` — and on `boundaries` rather than an eighteenth step, for the reason the + // `errors` step's comment already gives: `VerifyStepName` is a closed union the CLI owns. + ...(await checkFlagReads(SPECS, join(root, 'packages', 'cli', 'src'))), ]; /** diff --git a/wiki/Error-Codes.md b/wiki/Error-Codes.md index 0b9cb8e9..eee11963 100644 --- a/wiki/Error-Codes.md +++ b/wiki/Error-Codes.md @@ -600,6 +600,7 @@ Two sets override the table, in `failures.ts`: | `X_ERROR_STATUS_BACKLOG_STALE` | a pin in the status-table ratchet has been resolved and left behind | `scripts/error-map-backlog.ts` still lists a code that `packages/http/src/error-map.ts` now maps, or one no package declares any more. "Can this code reach a request?" is not derivable from the source, so the classification is pinned and ratcheted — and a pin nobody removes is a pin nobody reads | the `fix` names the code and its group — delete that entry from `scripts/error-map-backlog.ts` | | `X_ERROR_STATUS_UNKNOWN_CODE` | the status table maps a code no package declares | a mistyped or renamed row in `ERROR_STATUS`. It reads as enforced and maps nothing, while the real code still falls through to 500 | the `fix` names the row — delete it from `packages/http/src/error-map.ts`, or register the code in its owning package's `src/errors.ts` | | `X_CATALOG_KEY_UNREACHABLE` | the framework catalog defines a key no framework source can reach | `packages/i18n/src/catalogs/en.json` carries a key in a namespace the framework itself renders (`admin.*`, `dev.*`, `ui.*`) that no `t()` call and no key-shaped literal in `packages/*/src` names — so it describes a screen that no longer exists and reads to the next author as a key that works. An `admin.nav.*`/`admin.table.*` block was in that state while every admin screen rendered `⟦admin.list.loading⟧`. Namespaces the framework only ships for an app (`common.*`, `auth.*`, `errors.*`, `pagination.*`, `validation.*`, `time.*`) are exempt, derived from the source rather than listed | the `fix` names the key — delete it from `packages/i18n/src/catalogs/en.json`, or render it: `t('')` in the view that needs it. Run `bun run scripts/i18n-catalog.ts --json` | +| `X_CLI_FLAG_UNREAD` | a command declares a flag no code reads | the CLI's own declarations held to each other: a `FlagSpec` the parser accepts and no file under `packages/cli/src` reads is a promise `x help` makes with nothing behind it. `x deploy --critical` said *"forces clients to reload"* and reached no reader outside the plan JSON. The four global flags are excluded — the parser owns them — and any bare `'name'` literal outside a `name:`/`short:` spec field counts as a read, so a flag echoed into `--json` still counts | `bun test packages/cli/src/flag-reads.test.ts` — the finding names the flag and the file to read it in | | `X_PUBLISH_LIST_INCOMPLETE` | `.github/workflows/release.yml` does not publish every publishable workspace — or contains no `npm publish` command at all | the publish list was kept by hand and a package was added without one. `@ultimat3/flags` was in exactly this state from 1.0.0 to 2026-08: the registry answered 404 while every consumer resolved it through the workspace, so nothing in the repo noticed | `bun run scripts/release-workflow.ts --json` — the `fix` carries the exact `-w ` and the step to add it to, or derive the list from `scripts/list-workspaces.ts` and it cannot recur | | `X_PUBLISH_LIST_UNKNOWN` | a publish step names a workspace this tree cannot publish — no such package, or a private one | a rename, a deletion, or a typo in a `-w` flag. `npm publish` exits non-zero on it, and the packages published by the steps before it cannot be unpublished | `bun run scripts/release-workflow.ts --json` — the `fix` carries the exact `-w ` to delete from `.github/workflows/release.yml` | | `X_BENCH_CLAIM_STALE` | a realtime capacity figure in `CLAUDE.md` is not the figure `scripts/bench/results/*.json` carries | the bench was re-run and the prose was not updated, or the prose was edited by hand. A duration renders as `(ms / 1000).toFixed(1)` seconds and a count comma-grouped every three digits; nothing else is accepted, and a reworded sentence reports as unstated rather than passing silently | `bun run scripts/bench-claims.ts --json` — the `fix` carries the exact string to write into `CLAUDE.md` | @@ -616,6 +617,10 @@ Two sets override the table, in `failures.ts`: | `X_SETUP_INSTALL_FAILED` | `bun install` failed during `bin/setup` | a conflicted lockfile, or a half-written `node_modules` | `rm -rf node_modules bun.lock && bun install` | | `X_RELEASE_VERSION_SKEW` | a workspace is not at the lockstep version | a package bumped on its own, or a release that stopped half-way | `bun run scripts/release.ts --bump patch --dry-run --json` to see the realignment, then run it without `--dry-run` and review the `package.json` diff | | `X_TRUST_PUBLISHER_MISSING` | a published package has no OIDC trusted publisher | a package added after the trust rollout, or a config revoked on npmjs.com — release day then falls back to token auth, which this repo has no token for | `bun run scripts/trust-publishers.ts` | +| `X_REGISTRY_BOOTSTRAP_OWED` | a publishable workspace is absent from the registry — its packument answers 404 | a package added **after** a release run: no run has ever seen it, so trusted publishing has nothing to bootstrap from. The finding names its ordinal in the derived publish order and how many packages a release publishes irreversibly before dying on this one. `@ultimat3/flags` was this until 2.0.0; `@ultimat3/scraping` until 2026-08-19 | `npm publish -w --access public --provenance=false` ([PUBLISHING.md](https://github.com/developerz-ai/ultimate/blob/main/PUBLISHING.md) step 1), then `bun run scripts/trust-publishers.ts` | +| `X_REGISTRY_VERSION_BEHIND` | the registry holds the package but not the version this tree stamps | the release that should have published it did not reach it — an aborted run leaves every package after the failure behind | `gh workflow run release.yml --ref v -f version=`; when that aborts `EPUBLISHCONFLICT` on a sibling already at the version, `npm publish -w --access public --provenance=false` | +| `X_REGISTRY_UNATTESTED` | the stamped version is on the registry with no `dist.attestations`, published by a person | it was published by hand rather than by `release.yml`, so nothing proves the workflow built that tarball. Every `@ultimat3/*` package at 2.0.0 is in this state | `bun run scripts/trust-publishers.ts --json`, then release the **next** version through the workflow (`bun run scripts/release.ts --bump patch`) — npm publishes are immutable, so the affected version can never gain an attestation | +| `X_REGISTRY_UNREACHABLE` | the registry did not answer — timeout, non-2xx, or a 200 that is not a packument | a rate limit or a network fault. Deliberately **not** reported as absent: a request that failed is not evidence that a package is unpublished, and treating it as one would send someone to re-bootstrap a live package | `curl -sS -o /dev/null -w '%{http_code}\n' https://registry.npmjs.org/`, then re-run `bun run scripts/registry-audit.ts --json` | | `X_TRUST_PUBLISHER_FAILED` | attaching a trusted publisher failed | npm rejected the call for a reason other than auth — most often `E409`, a stale config from an earlier attempt that must be revoked before it can be replaced | the `fix` names the package; revoke the old one (`npm trust list --json`, then `npm trust revoke --id `) and re-run `bun run scripts/trust-publishers.ts` | | `X_TRUST_2FA_REQUIRED` | npm refused the trust change on the credential's class | a granular access token that bypasses two-factor auth — attaching a publisher is an account-level trust change, so npm requires a session that carries 2FA, and no OTP flag rescues a bypass token | `npm login`, then `bun run scripts/trust-publishers.ts` | | `X_TRUST_ENVIRONMENT_EMPTY` | `--environment` was given as an empty string | an empty value attaches a publisher npm honours from **any** environment, which disables the required reviewer and the `v*` tag rule the `npm-publish` environment carries — and because the check compared `''` to `''`, the same argument that turned the gate off also made it report configured | `bun run scripts/trust-publishers.ts --environment npm-publish` |