From 592bd5e30cbbd9559ad447368334a725a3361cb6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 02:42:24 +0000 Subject: [PATCH] fix(rest): type the REST door's author-side error responder to the closed ErrorCode union MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The REST door exported `sendError(res, error: any, object?)` — a name that collided with the strict shared writer `sendError(res, status, code: ErrorCode, message)` in `@objectstack/types`. A cross-door parity note cited the strict one's closed parameter as the reason the REST door could not put an unregistered code on the wire; route modules used the loose one. The door read as closed while any handler could emit a fresh unregistered code silently. Split the two responsibilities by name and type the author-side one: - `sendThrownError` (renamed) keeps `error: any` deliberately — narrowing what a CAUGHT error may carry is an ADR-0112 public-contract decision, not an internal typing one. - `sendDeclaredFault` is new: `code: ErrorCode`, for refusals this repo DECIDES. It delegates to `sendThrownError`, so the wire answer is byte-identical — same #5437 5xx prose-withholding, same #5423 4xx truncation, same flat `{ error, code }` dialect (#7035 is untouched). All five author-declared emissions now route through it and are checked by `tsc --noEmit`. `check:dispatcher-error-vocabulary` gains a structural door-typing half so the narrowing cannot be widened back silently. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza --- packages/rest/src/error-response.ts | 106 ++++++++-- packages/rest/src/rest-server.ts | 57 +++--- .../src/dispatcher-error-vocabulary.ts | 16 +- .../src/package-door-error-parity.test.ts | 27 ++- scripts/check-dispatcher-error-vocabulary.mjs | 187 +++++++++++++++++- 5 files changed, 342 insertions(+), 51 deletions(-) diff --git a/packages/rest/src/error-response.ts b/packages/rest/src/error-response.ts index fa2ac5b26d..90be67c7b5 100644 --- a/packages/rest/src/error-response.ts +++ b/packages/rest/src/error-response.ts @@ -27,7 +27,8 @@ * resolution {@link resolveErrorResponse} — the same answer WITHOUT * emitting it, so the logging decision and the responder can * never form two opinions (#4886). - * emission `sendError` / `handleRouteError` — the doors a route catch + * emission `sendThrownError` / `sendDeclaredFault` / `handleRouteError` + * — the doors a route catch block and a deciding handler use. * block uses. * log verdict {@link isExpectedRouteError} and friends — whether a * response is a fault worth "[REST] Unhandled error". @@ -54,6 +55,7 @@ import { INTERNAL_ERROR_MESSAGE, } from '@objectstack/types'; import type { DroppedFieldsEvent } from '@objectstack/spec/data'; +import type { ErrorCode } from '@objectstack/spec/api'; import { logError } from './log.js'; /** @@ -113,7 +115,7 @@ function truncateClientMessage(message: string): string { * it is a function now only so the missing-relation branch above it cannot * drift into a second spelling of the same verdict. 500 is deliberately outside * `isExpectedDataStatus`, which is what buys the log line the silent 404 never - * had — `handleRouteError` prints `[REST] Unhandled error` and `sendError`'s + * had — `handleRouteError` prints `[REST] Unhandled error` and `sendThrownError`'s * `logWithheldServerFault` (#5437) covers the routes that bypass it, so the * withheld driver text always lands somewhere an operator can read it. */ @@ -148,7 +150,7 @@ const DATA_STORE_FAULT = (): { status: number; body: Record } = * recorded that a negative from a keyword heuristic is not evidence of * safety. The words still reach the operator: 500 is outside * `isExpectedDataStatus`, so `handleRouteError` prints `[REST] Unhandled - * error` with the whole error, and `sendError`'s `logWithheldServerFault` + * error` with the whole error, and `sendThrownError`'s `logWithheldServerFault` * covers the routes that bypass it. * * `INTERNAL_ERROR` rather than {@link DATA_STORE_FAULT}'s `DATABASE_ERROR`, and @@ -217,7 +219,7 @@ const UNCLASSIFIED_FAULT = (): { status: number; body: Record } * * The words are not lost: 500 is outside `isExpectedDataStatus`, so * `handleRouteError` prints `[REST] Unhandled error` with the whole error, and - * `sendError`'s `logWithheldServerFault` (#5437) covers the routes that bypass + * `sendThrownError`'s `logWithheldServerFault` (#5437) covers the routes that bypass * it — the same operator path {@link UNCLASSIFIED_FAULT} relies on. */ const NATIVE_ERROR_NAME_RE = @@ -434,7 +436,7 @@ export function mapDataError(error: any, object?: string): { status: number; bod // hooks reject sys_comment / sys_attachment inserts fail-closed when the // TARGET object's capability flag disallows them. 403 like // CLONE_DISABLED; surfaced by `code` because the generic data routes map - // through here (they never reach sendError's `.status` passthrough). + // through here (they never reach sendThrownError's `.status` passthrough). // `error.object` names the gated TARGET object (not the join table), so // prefer it. if (error?.code === 'FEEDS_DISABLED' || error?.code === 'FILES_DISABLED') { @@ -561,7 +563,7 @@ export function mapDataError(error: any, object?: string): { status: number; bod } // Generic passthrough for domain errors that already carry an explicit // HTTP status (e.g. plugin-sharing's record-scope denial: status 403 + - // code FORBIDDEN) — mirrors sendError's `.status` handling, which the + // code FORBIDDEN) — mirrors sendThrownError's `.status` handling, which the // generic data routes bypass by calling mapDataError directly (#2926 ⑦). // Placed AFTER the structured-code branches above (409s carry rich fields // this envelope would drop). @@ -1030,22 +1032,85 @@ export function mapDataError(error: any, object?: string): { status: number; bod } /** - * Centralized error responder for all REST handlers. Ensures raw driver - * messages (SQLite/Postgres dumps, stack traces, unique-constraint - * payloads with table names, etc.) never reach clients. Honors - * structured errors that already carry an explicit `status` so callers + * The CLASSIFICATION door: a THROWN thing becomes a sanitized HTTP answer. + * Ensures raw driver messages (SQLite/Postgres dumps, stack traces, + * unique-constraint payloads with table names, etc.) never reach clients. + * Honors structured errors that already carry an explicit `status` so callers * can surface domain-specific codes (e.g. 422 from a metadata save * validator), and routes everything else through `mapDataError` so the * security / validation / SQL-leak / unknown-object envelopes apply * uniformly across CRUD, batch, metadata, UI and discovery routes. + * + * [#9098] Named `sendError` until this change, which was a collision with the + * SHARED envelope writer of the same name in `@objectstack/types` — a + * different arity, a different envelope dialect and a different strictness. + * The two were never interchangeable, and the collision was not cosmetic: the + * cross-door parity note in `packages/runtime` cited "`sendError`'s closed + * `ErrorCode` parameter" as the reason the REST door could not put an + * unregistered code on the wire, which is true of the `@objectstack/types` + * one and false of this one — so the door's real hole read as closed. Three + * separate comments in `rest-server.ts` had each been written to warn the + * reader off the same conflation. Prose had already failed; the names are + * different now. + * + * ⛔ `error` stays `any` DELIBERATELY. This parameter is a caught value — a + * driver error, a `TypeError`, anything a `catch` can bind — and narrowing + * what a thrown error may carry would change what the REST door is allowed to + * emit. That is a public-contract decision (ADR-0112), not an internal typing + * one; the dispatcher door's equivalent required a maintainer ruling. What + * #9098 closed is the AUTHOR-side hole: see {@link sendDeclaredFault}. */ -export function sendError(res: any, error: any, object?: string): void { +export function sendThrownError(res: any, error: any, object?: string): void { const resolved = resolveErrorResponse(error, object); // [#5437] The client no longer reads a 5xx's own words; the operator must. logWithheldServerFault(error, resolved); res.status(resolved.status).json(resolved.body); } +/** + * [#9098] The AUTHOR-side door: a refusal this repo's own code DECIDED, with + * `code` typed to the closed ADR-0112 vocabulary. + * + * ## The hole this closes + * + * A handler that decides a refusal does not throw — it constructs the answer. + * Until this function existed, the only way to emit one in the flat dialect + * was to hand an object literal to the classification door above, whose + * `error: any` accepts any spelling at all. So an author could put a fresh, + * unregistered `code` on the wire with no type error, no lint and no review + * signal — the body would then fail `ApiErrorSchema` (and, because + * `BaseResponseSchema` embeds it, the WHOLE body) at the only place anyone + * would notice: a client's parse. `FIELD_VISIBILITY_UNRESOLVED` shipped in + * exactly that state and was found by a gate sweep, not by the door. + * + * `code: ErrorCode` makes the same mistake a compile error at the call site. + * The five author-declared emissions this repo had are routed through here, + * and `packages/rest`'s `tsc --noEmit` compiles every one of them — so the + * narrowing is checked by the build rather than asserted by a comment. + * + * ## What it deliberately does NOT change + * + * The wire answer is byte-identical to what the classification door produced + * for the same literal: this delegates to {@link sendThrownError} rather than + * re-implementing the response, so the #5437 5xx prose-withholding, the #5423 + * 4xx truncation and the FLAT `{ error, code }` dialect all still apply, + * unchanged and in one place. + * + * ⛔ In particular this is NOT a migration to the `@objectstack/types` + * envelope writer. That one emits the NESTED `{ success: false, error: { code, + * message } }` and applies no sanitization — routing these emissions through + * it would move the envelope POSITION (open finding #7035, deliberately out of + * #9098's scope) and would re-open the #5437 leak class by shipping a declared + * 5xx's own prose. Narrowing the vocabulary and moving the dialect are two + * separate decisions; this is only the first. + */ +export function sendDeclaredFault( + res: any, + fault: { code: ErrorCode; status: number; message: string }, +): void { + sendThrownError(res, fault); +} + /** * [ADR-0106 D6 tier 3] Refuse an object-schema read whose field visibility * could not be evaluated. @@ -1059,9 +1124,16 @@ export function sendError(res: any, error: any, object?: string): void { * * 503 rather than 500: the condition is an unhealthy dependency and a retry is * the right client behaviour. + * + * [#9098] Emits through {@link sendDeclaredFault}, so `FIELD_VISIBILITY_UNRESOLVED` + * is now checked against the closed ADR-0112 vocabulary at COMPILE time. It was + * this call — an object literal handed to an `error: any` parameter — that put + * an unregistered code on the wire for as long as it did (#8885 registered it; + * this makes the next one impossible rather than merely findable). The wire + * answer is unchanged: 503, `code`, and the #5437-withheld prose. */ export function sendFieldVisibilityFault(res: any, objectName: string): void { - sendError(res, { + sendDeclaredFault(res, { code: 'FIELD_VISIBILITY_UNRESOLVED', message: `Field visibility for object '${objectName}' could not be evaluated; the object schema is not being served.`, status: 503, @@ -1079,7 +1151,7 @@ export function sendFieldVisibilityFault(res: any, objectName: string): void { * this issue was raised on is exactly the fault an operator must be able to * diagnose (the in-memory registry has already diverged from the database). * - * `sendError` had no logging at all, so its 5xx band went from "the client can + * `sendThrownError` had no logging at all, so its 5xx band went from "the client can * read the driver error" straight to "nobody can" without this. The routes that * exit through `handleRouteError` already print the whole error object for a * genuine fault — this fires only in the gap that predicate leaves: 502/503, @@ -1100,8 +1172,8 @@ function logWithheldServerFault( } /** - * The wire response `sendError` would emit for a thrown route error, WITHOUT - * emitting it. Split out of `sendError` so the logging decision + * The wire response `sendThrownError` would emit for a thrown route error, + * WITHOUT emitting it. Split out of `sendThrownError` so the logging decision * (`handleRouteError`) reads the exact status/body the client is about to get * instead of forming a second opinion that can drift from the responder — the * drift this whole seam exists to prevent (#4886). @@ -1127,7 +1199,7 @@ function resolveErrorResponse(error: any, object?: string): { status: number; bo // sibling branch stopped at 4xx *on purpose* — "5xx messages keep going // through the sanitizing heuristics below so internal/SQL details never // reach the client verbatim". Two opposite verdicts on one question, - // and every route that reports through `sendError` (metadata, UI, + // and every route that reports through `sendThrownError` (metadata, UI, // discovery, batch) got the permissive one: a declared 500 shorter than // `CLIENT_MESSAGE_MAX` was returned word for word, past `isSqlLeak`, // past `looksLikeInternalErrorLeak`, past `Internal data error`. @@ -1302,7 +1374,7 @@ export function logUnexpectedRouteError(error: any, resolved: { status: number; /** * The single door a route catch block should use: resolve the response once, * log it only if it is a real fault, then send it. Wire behaviour is identical - * to a bare `sendError(res, error, object)` — this only decides whether the log + * to a bare `sendThrownError(res, error, object)` — this only decides whether the log * line is printed. */ export function handleRouteError(res: any, error: any, object?: string): void { diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 70abe3095b..03153c2405 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -80,12 +80,16 @@ import { apiExposureDenialReason, DATA_ACTION_TO_API_OPERATION, } from '@objectstack/spec/data'; -// [#8013] The SHARED envelope writer (#3973), aliased: this module already has a -// module-scope `sendError` of its own — the sanitizing responder that maps a -// THROWN error onto a status — and the two are not interchangeable. This one -// emits the declared `{ success: false, error: { code, message } }` for a refusal -// the handler DECIDED, with `code` typed to the closed ADR-0112 vocabulary rather -// than `string`. Adding a call site here moves no `check:route-envelope` count: +// [#8013] The SHARED envelope writer (#3973), aliased. [#9098] The alias no +// longer exists to dodge a NAME collision — the local responder this used to +// collide with is `sendThrownError` now — it marks the ENVELOPE DIALECT, which +// is the difference that actually matters and the one still open. This one +// emits the declared, NESTED `{ success: false, error: { code, message } }` for +// a refusal the handler DECIDED; `sendDeclaredFault` emits the same refusal in +// this package's FLAT `{ error, code }` dialect (open finding #7035). Both type +// `code` to the closed ADR-0112 vocabulary rather than `string`, so the choice +// between them is about POSITION only, never strictness. +// Adding a call site here moves no `check:route-envelope` count: // the body literal lives in `@objectstack/types` (the pinned `SHARED_BUILDER`), // and this file is audited `dialectOnly` for the two non-conforming dialects it // still emits — which this deliberately is not. @@ -127,7 +131,8 @@ import { logError, logWarn } from './log.js'; // surface `./rest-server.js` has always offered is byte-identical. import { mapDataError, - sendError, + sendThrownError, + sendDeclaredFault, sendFieldVisibilityFault, handleRouteError, logUnexpectedRouteError, @@ -3277,7 +3282,7 @@ export class RestServer { res.json(enriched); } catch (error: any) { logError('[REST] openapi.json error:', error); - sendError(res, error); + sendThrownError(res, error); } }; @@ -4259,9 +4264,9 @@ export class RestServer { }); if (!audienceAllows((book as any).audience, caller)) { if (!caller.authenticated) { - sendError(res, { code: 'UNAUTHENTICATED', message: 'This documentation requires sign-in', status: 401 }); + sendDeclaredFault(res, { code: 'UNAUTHENTICATED', message: 'This documentation requires sign-in', status: 401 }); } else { - sendError(res, { code: 'PERMISSION_DENIED', message: 'This documentation is limited to holders of a permission set you do not have', status: 403 }); + sendDeclaredFault(res, { code: 'PERMISSION_DENIED', message: 'This documentation is limited to holders of a permission set you do not have', status: 403 }); } return; } @@ -4694,8 +4699,10 @@ export class RestServer { // this `code`. // // Written through the shared `sendError` - // (`@objectstack/types`), aliased because this - // module has a local function of that name. + // (`@objectstack/types`), aliased to mark the + // envelope dialect — see the note at the import + // (#9098 removed the name collision that used + // to be the alias's reason). // That builder emits the DECLARED envelope // `{ success: false, error: { code, message } }`, // so the console reads `body.error.code` — the @@ -4789,9 +4796,9 @@ export class RestServer { } if (!allowed) { if (!caller.authenticated) { - sendError(res, { code: 'UNAUTHENTICATED', message: 'This documentation requires sign-in', status: 401 }); + sendDeclaredFault(res, { code: 'UNAUTHENTICATED', message: 'This documentation requires sign-in', status: 401 }); } else { - sendError(res, { code: 'PERMISSION_DENIED', message: 'This documentation is limited to holders of a permission set you do not have', status: 403 }); + sendDeclaredFault(res, { code: 'PERMISSION_DENIED', message: 'This documentation is limited to holders of a permission set you do not have', status: 403 }); } return; } @@ -8425,12 +8432,11 @@ export class RestServer { * `suggested-bindings` met two shapes inside one `security` family. * * Emitted through the SHARED builder (`sendError` from - * `@objectstack/types`, imported as `sendEnvelopeError` because this - * module has a local `sendError` of its own — the sanitizing responder - * for THROWN errors, a different thing). That is what makes this the - * reference shape by construction rather than a ninth local literal - * agreeing with the eight it replaced, and it types `code` to the - * closed vocabulary for free. + * `@objectstack/types`, imported as `sendEnvelopeError` — see the note + * at the import). That is what makes this the reference shape by + * construction rather than a ninth local literal agreeing with the + * eight it replaced, and it types `code` to the closed vocabulary for + * free. * * ⛔ Status codes are untouched: only the POSITION of `code` and * `message` moves. `detail` — the 400 arm's Zod-issue dump — moves to @@ -8646,12 +8652,11 @@ export class RestServer { * #8073 (PR #8174) from the `/security/explain` pair. * * Emitted through the SHARED builder (`sendError` from - * `@objectstack/types`, imported as `sendEnvelopeError` because this - * module has a local `sendError` of its own — the sanitizing responder - * for THROWN errors, a different thing). That is what makes this the - * reference shape by construction rather than a tenth local literal - * agreeing with the nine it replaced, and it types `code` to the - * closed ADR-0112 vocabulary for free. + * `@objectstack/types`, imported as `sendEnvelopeError` — see the note + * at the import). That is what makes this the reference shape by + * construction rather than a tenth local literal agreeing with the nine + * it replaced, and it types `code` to the closed ADR-0112 vocabulary + * for free. * * ⛔ Status codes are untouched and no code VALUE moves: only the * POSITION of `code` and `message` changes. diff --git a/packages/runtime/src/dispatcher-error-vocabulary.ts b/packages/runtime/src/dispatcher-error-vocabulary.ts index 5b4bb0d9ad..3969904652 100644 --- a/packages/runtime/src/dispatcher-error-vocabulary.ts +++ b/packages/runtime/src/dispatcher-error-vocabulary.ts @@ -68,9 +68,19 @@ export type CodeStampShape = /** * Where the stamped code can end up. `dispatcher` is the door this card is - * about; `rest` is the direct-mount registrar, whose own `sendError` overload - * (`packages/rest/src/error-response.ts`, `error: any`) does NOT narrow; - * `none` means the value never reaches an HTTP error envelope at all. + * about; `rest` is the direct-mount registrar; `none` means the value never + * reaches an HTTP error envelope at all. + * + * [#9098] The `rest` door SPLIT, and the split is why this classification still + * earns its keep. Its author-side responder (`sendDeclaredFault`, + * `packages/rest/src/error-response.ts`) now takes `code: ErrorCode`, so a + * refusal this repo DECIDES is narrowed by the compiler and can never be a + * finding here. Its classification responder (`sendThrownError`, same file) + * still takes `error: any` — deliberately, since narrowing what a CAUGHT error + * may carry is an ADR-0112 contract decision rather than an internal typing one + * — so a code stamped on a thrown value and passed through remains exactly the + * reachability question this table answers. Both were spelled `sendError` until + * #9098; that collision is what let the door's hole read as closed. */ export type CodeDoor = 'dispatcher' | 'rest' | 'none'; diff --git a/packages/runtime/src/package-door-error-parity.test.ts b/packages/runtime/src/package-door-error-parity.test.ts index c457e7c3c3..da717d6244 100644 --- a/packages/runtime/src/package-door-error-parity.test.ts +++ b/packages/runtime/src/package-door-error-parity.test.ts @@ -128,9 +128,30 @@ describe('#8016 — the dispatcher package door answers the shared mapping', () * difference rather than a drift. * * This door puts a producer's code on the wire verbatim. The REST door - * cannot: `@objectstack/types`' `sendError` takes the closed `ErrorCode`, - * and that door's conformance suite parses its bodies against the ledger, so - * an unregistered code there is a failing test rather than a wire answer. + * cannot — but ONLY for the codes its own handlers DECIDE, and that + * distinction is the correction #9098 landed here. + * + * The sentence this paragraph used to carry ("the REST door cannot: + * `@objectstack/types`' `sendError` takes the closed `ErrorCode`") named a + * real, strict function and drew a false conclusion from it. `packages/rest` + * had a SECOND exported `sendError` — its own sanitizing responder, typed + * `error: any` — and that was the one its route modules reached for. So the + * strictness cited here was never on the path being described, and the door + * read as closed while an author could put any spelling at all on the wire + * (`FIELD_VISIBILITY_UNRESOLVED` did, and a gate sweep found it, not this + * pin). #9098 renamed the responder to `sendThrownError` so the two cannot + * be conflated again, and added `sendDeclaredFault` — `code: ErrorCode` — + * as the typed author-side door. + * + * What is true now, stated per path: + * - AUTHOR-decided refusals: narrowed at COMPILE time, at both REST + * doors — `sendDeclaredFault` (flat dialect) and `@objectstack/types`' + * `sendError` (nested). An unregistered code is a build failure. + * - THROWN errors: NOT narrowed, deliberately and symmetrically with this + * door. `sendThrownError` passes a caught error's `code` through + * verbatim; narrowing that is an ADR-0112 public-contract decision, not + * an internal typing one. `check:dispatcher-error-vocabulary` is what + * keeps that path honest, at both doors. * * The STATUS agrees either way, which is what #8016 was about. * diff --git a/scripts/check-dispatcher-error-vocabulary.mjs b/scripts/check-dispatcher-error-vocabulary.mjs index 428c6bd5ee..bcb5c14923 100644 --- a/scripts/check-dispatcher-error-vocabulary.mjs +++ b/scripts/check-dispatcher-error-vocabulary.mjs @@ -76,6 +76,11 @@ * - The sandbox limb is OUT of this scan's reach by construction — a metadata * app's action code is authored at runtime, not in this repo. See * `SANDBOX_AUTHORED_LIMB` in the declaration file. + * - [#9098] The scan answers "is this code registered", never "could an + * unregistered one be written here tomorrow". The second question is the + * DOOR TYPING half — see `checkDoorTyping` below, which is structural + * rather than vocabulary and is why this gate's name now undersells it: it + * guards both HTTP doors, not only the dispatcher. */ import { readFileSync, readdirSync, statSync, existsSync } from 'node:fs'; @@ -326,13 +331,103 @@ export function reconcile({ sites, declared, registered, unresolved }) { return findings; } +// --------------------------------------------------------------------------- +// The REST door's TYPING (#9098) +// --------------------------------------------------------------------------- + +/** + * [#9098] The vocabulary half of this gate answers "is this code registered". + * It cannot answer "could an unregistered one be written here tomorrow" — and + * that second question is what the REST door failed. + * + * The history, because it is the whole argument for these three assertions: + * `packages/rest` exported a `sendError(res, error: any)` whose name collided + * with `@objectstack/types`' strict `sendError(res, status, code: ErrorCode, + * message)`. A cross-door parity note cited the strict one's closed parameter + * as the reason the REST door was safe; route modules used the loose one. The + * door read as closed for as long as anyone cared to look, and the hole was + * eventually found by this scan (`FIELD_VISIBILITY_UNRESOLVED`) rather than by + * the door. #9098 split the two responsibilities by name and typed the + * author-side one. + * + * Typing alone does not stay put — it is three keystrokes from `any`, and the + * scan above would go on passing because a code that IS registered is invisible + * to it. So the structure is pinned here: + * + * ① the author-side door exists and narrows `code` to the closed `ErrorCode`; + * ② nothing in that file re-exports the name `sendError`, so the collision + * that hid the hole cannot be reintroduced; + * ③ no author-declared literal is handed to the CLASSIFICATION door — that + * parameter is `any` by design (narrowing a caught error's `code` is an + * ADR-0112 contract decision, not this gate's), which is exactly why a + * decided refusal must not travel through it. + * + * ⛔ An anchor this cannot find is a FINDING, never a pass — the same rule the + * header states for unresolvable constants. A structural gate that silently + * matches nothing is the failure it exists to prevent, one layer down. + */ +export const REST_DOOR_FILE = 'packages/rest/src/error-response.ts'; + +/** Author-declared literal handed to the classification door. Flat object literal on purpose. */ +const DECIDED_THROUGH_THROWN_RE = + /\bsendThrownError\s*\(\s*[A-Za-z_$][\w$]*\s*,\s*\{[^{}]*\bcode\s*:\s*'([A-Za-z][A-Za-z0-9_]*)'/g; + +export function checkDoorTyping({ doorSource, files }) { + const findings = []; + const add = (text) => findings.push({ kind: 'door-typing', text }); + + if (doorSource === null || doorSource === undefined) { + add(`${REST_DOOR_FILE} could not be read — this gate's door-typing anchor moved. ` + + `Point REST_DOOR_FILE at the file that now owns the REST error doors.`); + return findings; + } + const stripped = stripComments(doorSource); + + // ① the author-side door narrows to the closed vocabulary + const decl = /export\s+function\s+sendDeclaredFault\s*\(([\s\S]*?)\)\s*:\s*void/.exec(stripped); + if (!decl) { + add(`${REST_DOOR_FILE}: no \`export function sendDeclaredFault(...): void\` found. ` + + `That is the REST door's typed author-side responder (#9098); without it a decided ` + + `refusal has no narrowed door and an unregistered code reaches the wire silently.`); + } else if (!/\bcode\s*:\s*ErrorCode\b/.test(decl[1])) { + add(`${REST_DOOR_FILE}: \`sendDeclaredFault\` no longer types its \`code\` as \`ErrorCode\`. ` + + `The closed ADR-0112 union is the only thing making an unregistered code a BUILD failure ` + + `at this door — widening it re-opens #9098 while every test here stays green.`); + } + + // ② the collision that hid the hole cannot come back + if (/export\s+(?:function|const)\s+sendError\b/.test(stripped)) { + add(`${REST_DOOR_FILE} exports \`sendError\` again. That name belongs to the SHARED strict ` + + `writer in \`@objectstack/types\`; a second one here is the #9098 collision, which is how ` + + `the door's own looseness came to be documented as strictness. Name the local doors ` + + `\`sendThrownError\` (caught values) and \`sendDeclaredFault\` (decided refusals).`); + } + + // ③ decided refusals do not travel through the `any` door + for (const { rel, source } of files) { + if (!rel.startsWith('packages/rest/')) continue; + const s = stripComments(source); + DECIDED_THROUGH_THROWN_RE.lastIndex = 0; + for (const m of s.matchAll(DECIDED_THROUGH_THROWN_RE)) { + add(`${rel}: hands the author-declared code '${m[1]}' to \`sendThrownError\`, whose \`error\` ` + + `parameter is \`any\` by design (it takes CAUGHT values). A refusal this repo decides must ` + + `go through \`sendDeclaredFault\`, which narrows \`code\` to the closed union. Same wire ` + + `answer, checked at compile time.`); + } + } + + findings.sort((a, b) => a.text.localeCompare(b.text)); + return findings; +} + // --------------------------------------------------------------------------- // Self-test // --------------------------------------------------------------------------- function selfTest() { const fail = []; - const ok = (cond, what) => { if (!cond) fail.push(what); }; + let cases = 0; + const ok = (cond, what) => { cases += 1; if (!cond) fail.push(what); }; // Each published SHAPE matches what it claims to. const samples = { @@ -481,12 +576,90 @@ function selfTest() { ); } + // [#9098] Door typing. Each assertion is pinned in BOTH directions: the + // healthy shape passes, and the specific regression it names fails. A + // structural gate that only ever ran against a healthy tree would be a + // phantom check — it must be shown capable of failing. + { + const healthy = ` + import type { ErrorCode } from '@objectstack/spec/api'; + export function sendThrownError(res: any, error: any, object?: string): void {} + export function sendDeclaredFault( + res: any, + fault: { code: ErrorCode; status: number; message: string }, + ): void { sendThrownError(res, fault); } + export function sendFieldVisibilityFault(res: any, o: string): void { + sendDeclaredFault(res, { code: 'FIELD_VISIBILITY_UNRESOLVED', message: o, status: 503 }); + }`; + ok( + checkDoorTyping({ doorSource: healthy, files: [] }).length === 0, + 'the healthy door shape produced a door-typing finding', + ); + + const widened = healthy.replace('code: ErrorCode;', 'code: string;'); + ok( + checkDoorTyping({ doorSource: widened, files: [] }).some((f) => /no longer types/.test(f.text)), + 'widening sendDeclaredFault\'s `code` to string did not fail', + ); + + const missing = healthy.replace(/export function sendDeclaredFault[\s\S]*?\): void \{[^}]*\}/, ''); + ok( + checkDoorTyping({ doorSource: missing, files: [] }).some((f) => /no .*sendDeclaredFault/.test(f.text)), + 'deleting the typed author-side door did not fail', + ); + + const collided = `${healthy}\nexport function sendError(res: any, error: any): void {}`; + ok( + checkDoorTyping({ doorSource: collided, files: [] }).some((f) => /collision/.test(f.text)), + 'reintroducing the `sendError` collision did not fail', + ); + + // ③ both directions, and the CAUGHT-value call must stay legal. + const bypass = `sendThrownError(res, { code: 'SOMETHING_NEW', message: 'x', status: 400 });`; + ok( + checkDoorTyping({ + doorSource: healthy, + files: [{ rel: 'packages/rest/src/some-routes.ts', source: bypass }], + }).some((f) => /SOMETHING_NEW/.test(f.text)), + 'an author-declared literal sent through the `any` door did not fail', + ); + ok( + checkDoorTyping({ + doorSource: healthy, + files: [{ rel: 'packages/rest/src/some-routes.ts', source: `sendThrownError(res, error, object);` }], + }).length === 0, + 'passing a CAUGHT error through the classification door was flagged — that is its job', + ); + ok( + checkDoorTyping({ + doorSource: healthy, + files: [{ rel: 'packages/rest/src/x.ts', source: `// sendThrownError(res, { code: 'IN_A_COMMENT' });` }], + }).length === 0, + 'a commented-out bypass produced a door-typing finding', + ); + + // A missing door file is a FINDING, never a quiet pass. + ok( + checkDoorTyping({ doorSource: null, files: [] }).some((f) => /anchor moved/.test(f.text)), + 'an unreadable door file passed quietly instead of failing', + ); + + // And the real file on disk satisfies all three. + ok( + existsSync(join(ROOT, REST_DOOR_FILE)), + `${REST_DOOR_FILE} does not exist — the door-typing anchor moved`, + ); + } + if (fail.length) { console.error('check-dispatcher-error-vocabulary --self-test FAILED:'); for (const f of fail) console.error(` - ${f}`); process.exit(1); } - console.log(`check-dispatcher-error-vocabulary --self-test: ${Object.keys(samples).length} shapes + 10 cases OK`); + console.log( + `check-dispatcher-error-vocabulary --self-test: ${Object.keys(samples).length} shapes ` + + `+ ${cases} assertions OK (vocabulary + #9098 door typing)`, + ); } // --------------------------------------------------------------------------- @@ -511,11 +684,21 @@ function main() { const declared = parseDeclaration(readFileSync(join(ROOT, DECLARATION), 'utf8')); const findings = reconcile({ sites, declared, registered, unresolved }); + // [#9098] The door-typing half. `walkSources` skips the door file only if it + // is a test or the declaration — it is neither, so read it from the scanned + // set and fall back to disk rather than assuming either. + const doorEntry = files.find((f) => f.rel === REST_DOOR_FILE); + const doorAbs = join(ROOT, REST_DOOR_FILE); + const doorSource = doorEntry?.source ?? (existsSync(doorAbs) ? readFile(doorAbs) : null); + findings.push(...checkDoorTyping({ doorSource, files })); + const pending = declared.filter((d) => d.verdict === 'pending-registration'); const bounds = ` scope: ${files.length} non-test source files under ${SCAN_ROOT}/; ` + `${registered.size} registered codes (${ledger.size} ledger + ${standard.size} standard); ` + `${sites.length} unregistered code-stamping site(s) found; ${declared.length} classified.\n` + + ` door typing (#9098): ${REST_DOOR_FILE} checked for the typed author-side responder, the ` + + `absence of a second \`sendError\`, and decided refusals bypassing it.\n` + ` the sandbox limb (author-thrown codes from metadata-app action code) is outside this scan ` + `by construction — see SANDBOX_AUTHORED_LIMB in ${DECLARATION}.`;