diff --git a/.changeset/senderror-carries-declaredcode.md b/.changeset/senderror-carries-declaredcode.md new file mode 100644 index 0000000000..48372d7882 --- /dev/null +++ b/.changeset/senderror-carries-declaredcode.md @@ -0,0 +1,40 @@ +--- +"@objectstack/types": minor +--- + +fix(types): let `sendError`'s `extra` carry `declaredCode`, so a nested-envelope route can emit the ADR-0112 open channel (#11719) + +`ApiErrorSchema` has declared `declaredCode` since #9106 — the open, +author-authored channel that carries a metadata app's own `.code` verbatim when +the spelling is not a member of the closed `code` vocabulary. ADR-0112's +2026-08-17 amendment rules that demote **platform-wide**, and #9232 extended it +to the flat `/data` door, which emits the pair today. + +The shared nested-envelope writer could not. `sendError`'s `extra` was typed +`Pick`, so +passing a demoted spelling was a **compile error** and every route answering the +nested envelope dropped it. Nothing invalid shipped — the closed `code` still +carried the member derived from the status — which is exactly what made the loss +silent and one-directional: the author's spelling gone, and a consumer told by +the ADR to read `declaredCode` finding nothing there. Declared-but-unemittable +is a `declared = enforced` gap, closed here at the one writer rather than per +module. + +Additive: `declaredCode` joins the `Pick`. No existing call site changes, no +wire byte moves for any body already being emitted, and the contract's accept +set is untouched — the schema has always permitted the field. + +⛔ Presence still MEANS demotion, and the writer does not re-derive that. The +caller passes `demotedDeclaredCode(thrown)` (`@objectstack/types`), exactly as +the flat door's `thrownCodeFields` does; that helper answers `undefined` when +the producer's spelling is already the vocabulary member sitting in `code`, so a +registered refusal never carries two spellings of one fact. Vocabulary and +position stay two decisions (#9232). + +Pinned in `response-envelope.test.ts` by driving the real pipeline — a +sandbox-shaped throw carrying a tenant-authored `.code` through +`resolveThrownHttpError` and `demotedDeclaredCode` — and by parsing the emitted +body with the real `ApiErrorSchema`, asserting the field is still on it *after* +the parse. `ApiErrorSchema` is a plain `z.object` that strips undeclared keys, +so a `.success` assertion alone would have passed against a schema declaring +nothing. diff --git a/packages/types/src/response-envelope.test.ts b/packages/types/src/response-envelope.test.ts index ba0179dbff..e73a67ee66 100644 --- a/packages/types/src/response-envelope.test.ts +++ b/packages/types/src/response-envelope.test.ts @@ -23,8 +23,9 @@ */ import { describe, it, expect } from 'vitest'; -import { BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; +import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api'; import { sendOk, sendError, type EnvelopeResponse } from './response-envelope.js'; +import { resolveThrownHttpError, demotedDeclaredCode } from './thrown-http-error.js'; /** Captures what a route would have put on the wire. */ function capture() { @@ -133,3 +134,114 @@ describe('sendError', () => { sendError(res, 400, 'NOT_A_REGISTERED_CODE', 'invented'); }); }); + +/** + * The ADR-0112 open channel on the NESTED envelope (#9106, #9232). + * + * `ApiErrorSchema` has declared `declaredCode` since #9106 and the flat `/data` + * door emits it, but `sendError`'s `extra` did not admit the field — so putting + * a demoted producer spelling on a nested-envelope route was a COMPILE ERROR, + * and the author's own code was dropped while the derived closed member shipped + * in its place. Nothing invalid went on the wire, which is exactly what made the + * loss silent. + * + * These drive the REAL pipeline rather than asserting the type: a thrown error + * shaped like a sandboxed hook's refusal goes through `resolveThrownHttpError` + * and `demotedDeclaredCode` — the one rule both doors read — and the body is + * parsed by the real `ApiErrorSchema`. A type-level assertion would have passed + * against a schema that declares nothing. + */ +describe('sendError — the `declaredCode` open channel', () => { + /** + * The reachable producer ADR-0112's amendment names: a metadata app's own + * `.code` crossing the QuickJS boundary (#7867) on a hook refusal. It is + * NOT a member of `StandardErrorCode ∪ ERROR_CODE_LEDGER`, so the closed + * slot cannot hold it. + */ + const tenantAuthored = () => Object.assign( + new Error('Quota exceeded for this app.'), + { code: 'crm.quota_exceeded', status: 403 }, + ); + + it('carries the producer spelling beside the derived closed member', () => { + const { res, seen } = capture(); + const thrown = resolveThrownHttpError(tenantAuthored()); + const demoted = demotedDeclaredCode(thrown); + + sendError(res, thrown.status, thrown.code, thrown.message, { + ...(demoted !== undefined ? { declaredCode: demoted } : {}), + }); + + expect(seen.status).toBe(403); + expect(seen.body).toEqual({ + success: false, + error: { + // Derived from the status, because the spelling is unregistered. + code: 'PERMISSION_DENIED', + message: 'Quota exceeded for this app.', + // …and the author's own spelling survives, verbatim. + declaredCode: 'crm.quota_exceeded', + }, + }); + }); + + it('emits a body the REAL schemas accept, with the field still on it', () => { + // `.success` alone would not have caught this: `ApiErrorSchema` is a + // plain `z.object`, so an UNDECLARED sibling parses clean by being + // STRIPPED. The reading that means something is that the field is still + // there AFTER the parse — i.e. the schema declares it. + const { res, seen } = capture(); + const thrown = resolveThrownHttpError(tenantAuthored()); + sendError(res, thrown.status, thrown.code, thrown.message, { + declaredCode: demotedDeclaredCode(thrown)!, + }); + + const body = seen.body as { error: unknown }; + expect(BaseResponseSchema.safeParse(seen.body).success).toBe(true); + expect(envelopeViolations(seen.body)).toEqual([]); + + const parsed = ApiErrorSchema.safeParse(body.error); + expect(parsed.success).toBe(true); + expect((parsed as { data: { declaredCode?: string } }).data.declaredCode) + .toBe('crm.quota_exceeded'); + }); + + it('the survives-the-parse reading can say NO — an undeclared sibling is stripped', () => { + // The control for the assertion above, on a term that is not a + // substring of the one under test. `ApiErrorSchema` strips rather than + // rejects, so "parsed clean" is worthless on its own; this pins that the + // instrument distinguishes a DECLARED field from a tolerated one. + const parsed = ApiErrorSchema.safeParse({ + code: 'PERMISSION_DENIED', + message: 'denied', + declaredCode: 'crm.quota_exceeded', + namespace: 'branding', + }); + + expect(parsed.success).toBe(true); + const data = (parsed as { data: Record }).data; + expect(data.declaredCode).toBe('crm.quota_exceeded'); + expect('namespace' in data).toBe(false); + }); + + it('stays ABSENT when the producer spelled a registered code', () => { + // `ApiErrorSchema.declaredCode`'s documented invariant: presence MEANS + // demotion. A registered spelling is already in `code`, and repeating it + // would make one refusal carry two spellings of one fact. The writer does + // not re-derive that — `demotedDeclaredCode` does, for both doors. + const { res, seen } = capture(); + const thrown = resolveThrownHttpError( + Object.assign(new Error('denied'), { code: 'PERMISSION_DENIED', status: 403 }), + ); + const demoted = demotedDeclaredCode(thrown); + + expect(demoted).toBeUndefined(); + + sendError(res, thrown.status, thrown.code, thrown.message, { + ...(demoted !== undefined ? { declaredCode: demoted } : {}), + }); + + expect(Object.keys((seen.body as { error: object }).error)) + .toEqual(['code', 'message']); + }); +}); diff --git a/packages/types/src/response-envelope.ts b/packages/types/src/response-envelope.ts index b5bd78a3a2..5d84c894dc 100644 --- a/packages/types/src/response-envelope.ts +++ b/packages/types/src/response-envelope.ts @@ -110,7 +110,8 @@ export function sendOk(res: EnvelopeResponse, data: unknown, status = 200): void * ## `extra` is `ApiError`'s own optional fields, not a `Record` * * Merged into `error`, and typed as exactly what `ApiErrorSchema` declares - * beside `code` and `message` — `details`, `category`, `requestId`, `httpStatus`. + * beside `code` and `message` — `details`, `category`, `requestId`, + * `httpStatus`, `declaredCode`. * `details` is the slot for structured context: `package-routes` puts a partial * delete's per-item failures there, `settings-routes` the whole * `SettingsActionResult`. @@ -126,13 +127,43 @@ export function sendOk(res: EnvelopeResponse, data: unknown, status = 200): void * Closing it at the shared builder is the part that lasts: an undeclared sibling * is now a compile error in every module at once, rather than a key that quietly * evaporates at the schema boundary in whichever module reintroduces it. + * + * ## `declaredCode` — declared by the schema, barred by this writer + * + * ADR-0112's 2026-08-17 amendment (#9106, extended to the flat `/data` door by + * #9232) rules the demote at EVERY door: `code` stays the closed vocabulary, + * and a thrown code that is not a member is demoted to a declared sibling, + * `ApiError.declaredCode` — the open, author-authored channel that carries a + * metadata app's OWN `.code` across the QuickJS boundary (#7867) and onto the + * wire. + * + * `ApiErrorSchema` has declared that field since #9106 and the flat door emits + * it, but it was absent from the `Pick` above — so it was a COMPILE ERROR for + * any route answering the NESTED envelope to pass one, and every such route + * dropped the producer's spelling. Nothing invalid shipped (the closed `code` + * still carried the derived member), which is what made the loss silent and + * one-directional: the author's spelling gone, and a consumer told by the ADR + * to read `declaredCode` finding nothing there. Declared-but-unemittable is a + * `declared = enforced` gap, and admitting the field closes it at the ONE + * writer rather than in each module that later notices. + * + * ⛔ Presence MEANS demotion, and this writer does not re-derive that — the + * CALLER does, with `demotedDeclaredCode` (`thrown-http-error.ts`, one file + * over), exactly as the flat door's `thrownCodeFields` already does. That + * helper answers `undefined` when the producer's spelling IS the vocabulary + * member already sitting in `code`, which is what stops a registered refusal + * from carrying two spellings of one fact — `ApiErrorSchema.declaredCode`'s + * documented invariant. Passing a raw `thrown.declaredCode` re-opens exactly + * that, and no type here can catch it: vocabulary and position stay two + * decisions (#9232), so the demotion rule stays with the resolver that owns + * it rather than being restated in the envelope writer. */ export function sendError( res: EnvelopeResponse, status: number, code: ErrorCode, message: string, - extra?: Pick, + extra?: Pick, ): void { res.status(status).json({ success: false, error: { code, message, ...extra } }); }