Repository navigation
docs: four checks that make doc drift a build error, and the ~60 claims they found - #119
Merged
Merged
Conversation
…ms they found
The last slice of the audit. Its stated deliverable was never the corrections —
it was the enforcement, because a corrected doc drifts again.
Four new gate rules, 12 new X_* codes:
- doc-fixes every Fix cell of wiki/Error-Codes.md must be runnable (403 cells)
- doc-commands every `x …` in wiki/ and docs/ must resolve — command, subcommand,
positional AND flag (95 pages, 386 flag citations, 0 false positives)
- readme-fences every ts/tsx fence in a package README must compile (170 fences,
155 pinned on a ratchet that only lowers)
- version-stamps one page states the version, and the 29 workspaces must agree
The flag half is what matters: the old resolver judged only the command name, so
`x env check --fix`, `x jobs retry --from` and `x test summarize` all passed while
none of them runs. `x test summarize` was a shipped fix: line.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D
|
Caution Review failedAn error occurred during the review process. Please try again later. Comment |
…issues filed Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D
Contributor
Author
|
@coderabbitai review |
❌ Action failedReview failed.
|
This was referenced Aug 17, 2026
sebyx07
added a commit
that referenced
this pull request
Aug 17, 2026
…he wrong thing (#131) * fix(cli,money)!: two shipped fix: lines that resolved and still did the wrong thing Both passed #119's new doc checks, because the checks verify an invocation PARSES, not that it does what its sentence says. That gap is the whole of this change. `x branch` is planned, and its fix: line read `x db branch ls --json`. But `cmd-db.ts` fell through — any unrecognised subcommand became a branch NAME — so an agent following the fix verbatim CREATED a database called `<source>_branch_ls` instead of listing one. `x db branch` now takes a verb from a closed set: `ls | create <name> | drop <name>`. Verb-first is the only shape where a name cannot be read as a subcommand, and every one of those three verbs is itself a legal branch name. The repo already documented that surface and contradicted itself about it: four shipped `fix:` lines in `@ultimat3/db`'s own registry, its README and a test all spelled `x db branch create|drop <name>` for commands that did not exist, while the wiki said in so many words that no such subcommands existed. External create no longer shells out to `psql`. Forced by `ls`: branches are found by the `comment on database` marker `createBranch` writes, and the psql path wrote the database and no comment — so `x db branch ls` would have reported "no branches" over branches the CLI itself made. The `money` fix: line named a seam that did not exist — "add yours in `app.config.ts`", where there is no currency field and no registration function. `@ultimat3/money` now exports `registerCurrency()`. The decisive argument was not that a closed table is unfriendly; it is that THREE layers already treated the set as open — `moneySchema`'s `^[A-Z]{3}$`, entity's Postgres CHECK, and the PUBLISHED OpenAPI `pattern`. An app could already accept `GHS` over HTTP, validate it, store it and read it back, and only money's arithmetic refused it. That is one package disagreeing with the boundary, the storage layer and the contract. Consolidating that bound turned up five copies of `^[A-Z]{3}$`, not the two we knew about — including the hardcoded one in the published contract. It is now one tier-0 declaration (`CURRENCY_CODE_PATTERN` + `isCurrencyCode`), with the Postgres CHECK interpolating the same constant. A live test applies the emitted CHECK to a real Postgres and asserts it accepts exactly what the predicate accepts, SQLSTATE 23514, so a DDL typo that refuses everything cannot read as agreement. Two copies remain by necessity: core is tier 0 alongside schema and may not import it, so it keeps a documented duplicate with its own corpus test, plus a cross-package pin in `cli`, which may import both. Three further defects found on the way, each the same class: - `production` was hardcoded `false` in the only producer of a `DatabaseTarget`, so `assertBranchDatabase`'s "production is never migratable from MCP" arm was DEAD for every database the CLI makes. A production `DATABASE_URL` ending `/shop_branch_hotfix` read as a branch and was migratable through an MCP tool call. It now reads core's env exactly as `x doctor` does, and fails CLOSED on a malformed `ULTIMATE_ENV`. - `scanFixes` only ever saw a `fix:` PROPERTY, so a fix passed positionally into a local helper was invisible to the gate — 22 such helpers across 8 packages, 13 files declaring codes and yielding zero readable fixes. That is why the two stale mcp lines survived. Now read under four measured rules: 16 files gained fixes, 0 new findings. - `MissingSubcommandError`'s own fix: was `x db --help`, which throws `MissingSubcommandError` again — a fix line that reproduced its own failure forever. BREAKING: `x db branch <name>` is gone; use `create`. `branchSql` is removed from `@ultimat3/cli`. Fixes #122 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D * fix(http): an unregistered currency over HTTP answered 500 and paged the on-call Found by the gate, not by reading: adding `registerCurrency`'s two refusals made the `errors` step demand a status for them, and checking whether they could reach a request turned up a third code that already could. `X_CURRENCY_UNKNOWN` had no row in `packages/http/src/error-map.ts`, so it took DEFAULT_STATUS — and `stages.ts` reports every `status >= 500` to the error monitor. It is 400 now, beside `X_LOCALE_UNSUPPORTED`. It was never theoretical, and this branch is what makes it undeniable: every surface between the wire and the throw accepts any `^[A-Z]{3}$` — schema's `CURRENCY_CODE_PATTERN`, the OpenAPI `pattern` emitted from it, and entity's `char(3)` CHECK. So `{"minor":100, "currency":"ZWL"}` parsed, validated, reached `assertCurrency`, and the caller was told the server had broken over a code the framework's own schema had just accepted. Deleting the row fails 3 of the 4 new tests — including the one that asserts the monitor records nothing, which comes back with 37 lines where `[]` was expected. The two NEW codes go to the backlog instead, with the argument written down rather than implied: `registerCurrency` is their only thrower, it is a boot-time module-scope registry like `registerRoute`, and its `REGISTERED` map is per process while the chart runs three web replicas — so a request-driven registration is broken on the second replica before a status could describe it. Every comparable registry refusal in the repo is pinned the same way, and the one counter-precedent applies to all 228 pins equally, so it does not distinguish these. Also: both `packages/money/README.md` fences were missing their import line and did not compile. Fixing the pre-existing one as well let the ratchet FALL — money leaves `readme-fences-backlog.ts` entirely, so any future breakage there is a build error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D * fix: review findings — a drop that could delete a database we never created CodeRabbit raised 17 on this PR. Six were Major; several were real, two were wrong, and one was right about the severity while wrong about the mechanism. The worst: `x db branch drop <name>` could `DROP DATABASE` one that carries no marker at all — a database this framework never made. `listBranches()` filters on the marker and the marker records a timestamp, never the source, so on a server hosting two Ultimate apps `analytics_branch_feat` reduced to `feat` for a session on `postly` and SATISFIED THE GUARD. The reviewer read that as dropping the other app's database; it is not — the target is always derived from the connected source — but the foreign row authorised a drop of an unmarked `postly_branch_feat` belonging to someone else. Fixture reproduces it: before the fix the command returned success having dropped it. `ls` and `drop` now require the marker AND this source's prefix, and the check moved inside the deleting function on the same connection, one statement before the DROP. Seven `fix:` lines in @ultimat3/money emitted UNPARSABLE JavaScript for legal input — a currency named `O'Reilly` produced `fromDecimal('1.5', 'O'Reilly')`. `error-render` passed them because the values are typed `string`, not `unknown` (issue #97's exact gap). There was a second route with no quote involved: `fromDecimal` trims before parsing but hands the untrimmed value to the error, so a trailing newline broke the line and the 3-line render. Where the value has a known shape it is now GATED, not escaped — escaping alone yields a parsable `assertCurrency("O'Reilly")` that throws the same error when pasted, and this file already said an instruction that raises the error it answers is not an instruction. `bodyOf` in ts-scan took the next `{` anywhere after an expression-bodied arrow, so a later object with a `code:` key could classify a non-error helper as an error builder and reject valid source. Latent, not firing — measured 857 fix sites before and after, none lost. Two rejections, both with evidence. "Put the retry guidance in a separate field" — there is no fourth field on UltimateError, and inventing one is a convention with no build error behind it; the real defect was the joiner, since `ARGUMENT_END` makes `#` terminate the scan and 64 shipped lines use it against 9 comma-joined. "`<name>` is wrong" — the placeholder rule matches the third slot only, where a closed verb set makes substitution impossible; a branch name is caller-chosen and `packages/db` ships `<branch>` there. And the reviewer had the advisory-lock finding backwards: Error-Codes.md was RIGHT and six other pages were wrong. `X_MIGRATE_CONCURRENT` is thrown — one `pg_try_advisory_lock` per 500ms against a 60s budget. "Reserved, never thrown" was true of 1.2.0 and became false when #98 merged, which `git merge-base --is-ancestor` confirms is not an ancestor of the tag. Ten pages now say one thing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBwWKBJkiogA4mDaJiJf3D --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slice 13 — the last one. Four agents on disjoint directories in one checkout.
The corrections are not the deliverable; the four checks are. The slice document says so in its
own words — "that enforcement gap is the slice's real deliverable" — because this repo has
corrected these same pages before and they drifted again. Twelve new
X_*codes, four new gaterules, and the ~60 stale claims they found on the way in.
The old rule judged the command name and nothing below it
checkErrorFixesresolved a citedx <command>against the registry — and stopped there. So everyone of these passed the gate:
x test summarizefix:linesummarizeis not aTestTypex env check --fixwiki/Installation.mdX_CLI_BAD_FLAGx jobs retry --fromdocs/architecture/pages--from-step, andpackages/cli/README.mdalready spelled it rightx db query,x entity explainwiki/Error-Codes.mdx entities describe)x i18n add <key>docs/architecture/10-cross-cutting.mdThe resolver now works four levels down — command → planned → subcommand/positional → flag.
CommandSpec.positionalChoicesis declarative andx testreads it fromTEST_TYPES, the sameconstant the runtime validates against, so the two cannot drift.
The flag half ships enforcing, with a number: 386 flag citations across
wiki/anddocs/, 23flagged, 0 false positives after skipping planned commands. A sibling agent measured a similar
rule at 21 flagged / 20 false positives earlier in this audit and correctly refused to ship it —
this one met the bar.
Four checks
doc-fixesFixcell ofwiki/Error-Codes.mddoc-commands`x …`in prose acrosswiki/anddocs/readme-fencests/tsxfence in a package READMEversion-stampsreadme-fencesships pinned deliberately. The slice framed it as catching three READMEs; it is 28of 29 READMEs and ~170 fences, most of them illustrative fragments. A ratchet that only lowers means
a new broken example fails from day one while the existing 155 retire over time — a real edge,
honest about what it does not yet cover.
Two things it had to get right, both measured rather than assumed:
tscreports no semantic diagnostics at all once a program holds one syntax error. Verifieddirectly: a fixture with
const a: number = 'x'beside one unparseable fence reports the fenceand says nothing about the assignment. A single-pass check would have gone silently green across
all 27 packages the first time a README gained an elided
…. Hence two passes.exclude: ["node_modules"]silences the whole fixture if inherited(
TS18003: No inputs were found) — the exact fail-open a reviewer caught in three of four checksbuilt earlier this month. A
tscthat refuses to run isX_README_EXAMPLE_UNSCANNED, never a pass.Every one of the four fails on a missing or unreadable input, and each has a test that proves it.
23 stamps became one
23 wiki pages carried
v1.1.0 \As of 2026-08`while the tree was at 1.2.0. **Not find-and-replaced to 1.2.0** —[Unreleased]carries **30BREAKING —` entries**, so the next cut is a major and all23 would have been wrong again in one release. 23 hand-copies of one fact is what axioms 1 and 2
forbid.
_Footer.mdrenders on every wiki page, so it is the one page that states the version; the other 22keep their date. The check enforces the sentence the footer now makes, and asserts the lockstep
itself — a documented rule (
CHANGELOG.md, rootCLAUDE.md) that nothing verified, and a repo thathas already once cut nine tags with no version bump at all.
Versioning is lockstep; publication is not —
@ultimat3/flagshas never been published. Thecheck asserts the version invariant only; the registry needs a network call and the gate runs offline.
Premises falsified
packages/action/README.mdwas right and the code is wrong. One action is reachable underpublishPoston the wire (packages/mcp/src/from-action.ts:74— the only nametools/callaccepts) while
openapi.jsonandx.manifest.jsonboth publishpublish_post. Thepublished contract advertises a name nothing serves. Not unified here: it is BREAKING, changes
both tracked apps' committed
openapi.json, and re-pins nine test files. Recorded as a datedknown defect at the README so no agent copies it as design, and filed for its own PR.
packages/ai/src/eval-errors.tsdoes not citex test summarize— it readsx test eval --filter …with a comment explaining why. The convention was being held bymemory;
positionalChoicesmakes it a build error. Only the README still printed the bad form.createPgLeaderexists (driver-pg.ts:383), contrary to the report that it does not. Thedefect was that
scheduler.ts:42recommended it — the session-scoped one whose grant dies withthe pooled connection — instead of
createPgLeaseLeader. Eleven files described schedulerleader election as an advisory lock, one with a
SELECT pg_try_advisory_lock(...)block.since it was written —
x db status,x live explain,x db drift,x jobs run,x build --helm,x cache purgeand more. Each verified and dropped. The checks would havecaught every one.
slice document claimed. Both derivations now sit next to the number.
docker/README.md's two rows were already correct — the healthz claim was fixed and thedependency stage really is lockfile-only now (a
manifestsstage strips the rest). Dropped.x env checkwas documented as planned and ships — a working diagnostic everyX_ENV_MISSINGfix points at, documented as unavailable.
The escape hatches were fiction
docs/architecture/02-boundaries.mddocumented two boundary escape hatches —boundaries.allowanda
// boundary-allow:comment with a 90-day expiry — with zero implementation anywhere. Deleted,pointing at
guards/, which is the real, discovered, tested mechanism an app writes its own ruleswith. A second escape hatch would be axiom 1 broken. The same page named
schema → coreas thesideways exception;
SIDEWAYS_ALLOWhas no such entry andpackages/schema/src/errors.tssaysoutright that schema may not import core.
Conventions documented as rules that nothing enforced
Each is now labelled rather than asserted, per axiom 3:
hardcoded-string,raw-hex,date-no-tz,money-as-number(no Biome rule, no gate step), cached-query tagging, idempotency-keydeterminism,
og.imageand broken-link checks, SW checksum and strategy coherence, the 50-chardescription minimum (
DESCRIPTION_MIN_LENGTHis exported and read by nothing). Eighteen phantomX_*codes across the docs were replaced with the codes that really throw.Also fixed
packages/admin/src/dev/server.ts:185shippedfix: x dev --panel <key>, andx devdeclaresport,roleandonce. It was failingerror-contract.test.tsat the moment this branchopened — found by a check in this PR, in a package no agent in the wave owned.
Gate
bun run verify— 14 of 17, 3 skipped.bun run scripts/reference-app-gate.ts— every pin holds,examples/dummy11/17,dummy/social-media-clone15/17. Manifest at 417 codes.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.