From 8ee2cd924e0b3523c9db09071982b4188f5225a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 11:12:25 +0000 Subject: [PATCH 1/5] fix(rest): serve a sandboxed hook's own sentence on the bulk write routes, not the QuickJS debug wrapper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The declared-status passthrough in `resolveErrorResponse` answered a 4xx from `error.message`, which for a sandboxed hook refusal is the `hook '' threw: Error: ` debug wrapper. Every route reporting through `handleRouteError` — batch, createMany, updateMany, deleteMany, clone — shipped that to the end user, while the single-row `PATCH` on the same object answered with the business message alone. The arm now reads the business text via `sandboxBusinessMessage`, the unwrap door's own two conditions named once. The passthrough keeps deciding the STATUS, so #5437/#5582's 5xx prose withhold does not move: only the sentence the 4xx arm reads for the caller changes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- packages/rest/src/error-response.ts | 93 +++- .../rest-hook-refusal-message-parity.test.ts | 468 ++++++++++++++++++ 2 files changed, 558 insertions(+), 3 deletions(-) create mode 100644 packages/rest/src/rest-hook-refusal-message-parity.test.ts diff --git a/packages/rest/src/error-response.ts b/packages/rest/src/error-response.ts index bfe7b80adc..e9a73c0334 100644 --- a/packages/rest/src/error-response.ts +++ b/packages/rest/src/error-response.ts @@ -232,6 +232,41 @@ function isScriptFaultMessage(message: string): boolean { return NATIVE_ERROR_NAME_RE.test(message.trim()); } +/** + * [#11588] The caller-addressed BUSINESS text a sandboxed hook/action body + * threw, or `undefined` when this error is not a sandbox refusal. + * + * QuickJS bodies throw a `SandboxError` whose `.message` is the + * ` '' threw: ` debug wrapper and whose `.innerMessage` is the + * text the author addressed to the end user (see + * `runtime/src/sandbox/quickjs-runner.ts`). The wrapper "belongs in server + * logs" — {@link classifyDataError}'s unwrap door exists precisely to keep it + * off the wire. This is that door's read, named once so the door ABOVE it in + * {@link resolveErrorResponse} can ask the same question instead of shipping + * `error.message` raw. + * + * Both conditions are the door's, in the door's order: + * + * - a non-empty string `.innerMessage`, which is what makes this a sandbox + * error at all; + * - NOT {@link isScriptFaultMessage}. A body that CRASHED arrives with the + * same shape, and its `TypeError: not a function` is an internal fault + * rather than a business message (#7543). This answers `undefined` there, + * so a crash is never mistaken for authored text. + * + * ⛔ It is deliberately a READ of a field the sandbox populated, never a + * pattern-strip of the wrapper off `.message`. Stripping prose by regex would + * also rewrite a plain error whose own text happens to contain `threw:`, and + * the caller's message is the remedy on a 4xx (#5423) — the one thing this + * boundary must not paraphrase. `rest-hook-refusal-message-parity.test.ts` §5 + * is the control that keeps it a read. + */ +function sandboxBusinessMessage(error: any): string | undefined { + if (typeof error?.innerMessage !== 'string' || !error.innerMessage) return undefined; + if (isScriptFaultMessage(error.innerMessage)) return undefined; + return error.innerMessage; +} + /** * [#5462] Does a driver's missing-relation message name the very object this * request asked for? @@ -661,6 +696,15 @@ function classifyDataError(error: any, object?: string): { status: number; body: // VOCABULARY either — an unregistered spelling is demoted to // `declaredCode` by the same shared resolver, so this door stops being the // one flat exit #9232 could not reach. + // + // [#11588] The same two reads, in the same order, are named as + // {@link sandboxBusinessMessage} for the declared-status passthrough in + // {@link resolveErrorResponse}, which sits ABOVE this door and used to ship + // the wrapper verbatim. This door keeps its own spelling because its crash + // case is a TERMINAL (the sanitised 500) rather than a fall-through, which + // is a different answer to the same question; the two are held together by + // a door-to-door pin (`rest-hook-refusal-message-parity.test.ts` §4) rather + // than by this comment. if (typeof error?.innerMessage === 'string' && error.innerMessage) { // [#7543] …but only when the body REPORTED something. A body that // CRASHED arrives here too, and its `TypeError: not a function` is an @@ -1495,9 +1539,52 @@ function resolveErrorResponse(error: any, object?: string): { status: number; bo // [#5423] 4xx keeps the bound as a TRUNCATION, not a replacement: a 4xx // message is addressed TO the caller and is the remedy. Unchanged by // #5437 — see {@link truncateClientMessage}. - const safeMsg = typeof error.message !== 'string' - ? 'Request failed' - : truncateClientMessage(error.message); + // + // [#11588] …and for a SANDBOX refusal the text addressed to the caller + // is `.innerMessage`, not `.message` — see + // {@link sandboxBusinessMessage}. Without this read, every route that + // reports through `handleRouteError` (batch, createMany, updateMany, + // deleteMany, clone, and the metadata/UI/import/export families that + // share the exit) shipped the QuickJS DEBUG WRAPPER to the end user: + // `hook 'guard' threw: Error: Opportunity is closed.` where the + // single-row `PATCH` on the same object answered `Opportunity is + // closed.` One hook, one refusal, two different sentences depending on + // which route the caller happened to use. + // + // ⛔ This is NOT the reorder it looks like from the card. The unwrap + // door lives in `mapDataError`, BELOW this arm, and moving it above is + // ruled out by this arm's own argument two paragraphs up: `mapDataError` + // derives a status from the message TEXT, so a declared 5xx handed to + // it comes back re-labelled (`404 OBJECT_NOT_FOUND` for the + // overlay-delete fault) and stops being logged. The passthrough stays + // exactly where it is and keeps deciding the STATUS; only the sentence + // it reads for the caller changes. Nothing about the 5xx arm above — + // #5437/#5582's unconditional prose withhold — moves, and a sandbox + // refusal declaring a 5xx still exits there with the prose dropped. + // + // What this restores is an invariant THIS DOCBLOCK already asserts. The + // #7525 paragraph at the top of the arm says an error declaring + // `statusCode` instead "falls to `mapDataError` below … So the two + // doors already agree on the wire answer". For a sandbox refusal that + // sentence was false: `statusCode` fell through and was unwrapped, + // `status` was answered here from the wrapper, and one hook produced + // two message shapes on one route depending on the spelling its author + // picked. The two doors agree again now — pinned door-to-door rather + // than asserted, in `rest-hook-refusal-message-parity.test.ts` §4. + // + // Recorded because it is measured and NOT repaired here: a body that + // CRASHED while carrying a declared 4xx `status` still answers with + // that status and the wrapper, where `mapDataError` would sanitise it + // to a 500. `sandboxBusinessMessage` declines the crash (#7543) so this + // arm's answer for it is byte-identical to before. Closing that gap + // means moving the STATUS this arm decided, which is the contract + // question this card was fenced away from — filed separately. + const businessMessage = sandboxBusinessMessage(error); + const safeMsg = businessMessage !== undefined + ? truncateClientMessage(businessMessage) + : typeof error.message !== 'string' + ? 'Request failed' + : truncateClientMessage(error.message); // [#9232] Narrowed, same as the three arms above. return withDeclaredUserMessage(error, { status: error.status, diff --git a/packages/rest/src/rest-hook-refusal-message-parity.test.ts b/packages/rest/src/rest-hook-refusal-message-parity.test.ts new file mode 100644 index 0000000000..f963c8c891 --- /dev/null +++ b/packages/rest/src/rest-hook-refusal-message-parity.test.ts @@ -0,0 +1,468 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#11588] A sandboxed hook's refusal reaches the client in the AUTHOR'S words +// on every write route — never wearing the QuickJS debug wrapper. +// +// --------------------------------------------------------------------------- +// The rule +// --------------------------------------------------------------------------- +// `classifyDataError`'s sandbox unwrap door exists precisely to keep the +// ` '' threw: ` wrapper off the wire. Its own words: a hook's +// `throw new Error('删除被阻断…')` is "a deliberate business rule, not a fault", +// "End users must see only the business message", and the wrapper "belongs in +// server logs". `hook-error-format.dogfood.test.ts` pins that end to end — but +// only for a single-row `DELETE`, which is the one route family that reaches +// the door. +// +// Six routes never did. Measured on `main` at `cad8b42f`, and reproduced in §1: +// +// PATCH /data/:object/:id Opportunity is closed. ← correct +// POST /data/:object/batch hook 'guard' threw: Error: … ← the defect +// POST …/createMany · …/updateMany · …/deleteMany · …/:id/clone ← the defect +// POST /analytics/dataset/query ← NOT fixed here +// +// --------------------------------------------------------------------------- +// The branch, and why the repair is NOT the reorder it looks like +// --------------------------------------------------------------------------- +// Batch / bulk / clone exit through `handleRouteError` → `resolveErrorResponse`, +// whose declared-status passthrough is checked BEFORE it delegates to +// `mapDataError`. The passthrough's 4xx arm answered from `error.message` — the +// wrapper — and the unwrap door below was never reached. +// +// Moving the unwrap above the passthrough is ruled out by the passthrough's own +// argument: `mapDataError` derives a status from the message TEXT, so a declared +// 5xx handed to it comes back re-labelled (`404 OBJECT_NOT_FOUND` for the +// overlay-delete fault) and stops being logged — the #5437/#5582 withhold is +// load-bearing exactly where it sits. So the passthrough stays put and keeps +// deciding the STATUS; only the sentence it reads for the caller changes. +// +// What that restores is an invariant the passthrough docblock ALREADY asserts. +// Its #7525 paragraph says an error declaring `statusCode` instead "falls to +// `mapDataError` below … So the two doors already agree on the wire answer." +// For a sandbox refusal that sentence was false — `statusCode` was unwrapped, +// `status` was not — which is the two-spellings asymmetry the card named. §4 +// pins the agreement door-to-door rather than restating it in a comment. +// +// --------------------------------------------------------------------------- +// Anti-vacuity — directions predicted BEFORE running, measured after +// --------------------------------------------------------------------------- +// Baseline leg: this file run with ONLY `error-response.ts` reverted to +// `origin/main` (the fix committed first; revert `git checkout origin/main -- +// `, restore `git checkout HEAD -- `, both under `trap … EXIT INT +// TERM`, the mutation proven on disk by grepping both the injected and the +// removed text, and the restore re-verified by re-reading the file). +// +// No rebuild between legs, and the reason is load-bearing rather than an +// omission: every symbol under test is reached by a RELATIVE import inside this +// package, which vitest transforms from source — no `dist/` sits between the +// mutation and the assertion. The `exports`-resolved workspace deps here +// (`@objectstack/types`) are untouched by the mutation. +// +// §1 predicted RED 5 / GREEN 1 measured 5 red — as predicted. +// §2 predicted RED 5 measured 5 red — as predicted. +// §3 predicted GREEN measured green — as predicted. +// §4 predicted RED 1 measured 1 red — as predicted. +// §5 predicted GREEN throughout measured green — as predicted. These are the +// positive controls: a fix that stripped the wrapper by PATTERN instead of +// reading `.innerMessage` reddens here and nowhere else. +// §6 predicted GREEN throughout measured green — as predicted. +// §7 predicted GREEN both sides measured green — as predicted. It pins a +// divergence this card does NOT repair; see its own comment. +// +// Total 11 of 30 red pre-fix (prediction: 11). +// --------------------------------------------------------------------------- + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { INTERNAL_ERROR_MESSAGE } from '@objectstack/types'; +// `.js` extension deliberately: this package resolves `nodenext`, so an +// extensionless relative import is a `tsc` error (TS2835). +import { mapDataError, handleRouteError } from './error-response.js'; +import { RestServer } from './rest-server.js'; + +const DATA_COLLECTION = '/api/v1/data/:object'; +const DATA_ITEM = '/api/v1/data/:object/:id'; + +/** The wrapper text that must never reach a client. */ +const WRAPPER_RE = /threw:|hook '/; + +// --------------------------------------------------------------------------- +// Fixtures — the shape `runtime/src/sandbox/quickjs-runner.ts` produces: +// `.message` is the ` '' threw: ` debug wrapper, `.innerMessage` +// the business text, `.status` / `.statusCode` the #7867 side-channel. +// Reproduced here so `@objectstack/rest` does not depend on +// `@objectstack/runtime` to run its own tests. +// --------------------------------------------------------------------------- + +function sandboxRefusal( + businessMessage: string, + extra: Record = {}, + hook = 'guard', +) { + const err: any = new Error(`hook '${hook}' threw: Error: ${businessMessage}`); + err.name = 'SandboxError'; + err.innerMessage = businessMessage; + return Object.assign(err, extra); +} + +/** + * The same refusal thrown OUTSIDE the sandbox — no `.innerMessage`. The control + * that isolates the branch: one property is the whole difference, and this twin + * must come through byte-identical to before the fix. + */ +function plainRefusal(message: string, extra: Record = {}) { + return Object.assign(new Error(message), extra); +} + +function createMockServer() { + return { + get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(), use: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined), + }; +} + +function makeRes() { + const res: any = { statusCode: 200, body: undefined }; + res.status = vi.fn((c: number) => { res.statusCode = c; return res; }); + res.json = vi.fn((b: any) => { res.body = b; return res; }); + res.header = vi.fn(() => res); + res.setHeader = vi.fn(); res.write = vi.fn(); res.end = vi.fn(); res.send = vi.fn(); + return res; +} + +function setup(protocolOverrides: Record = {}) { + const protocol: any = { + getDiscovery: vi.fn().mockResolvedValue({ + version: 'v0', routes: { data: '', metadata: '', ui: '', auth: '/auth' }, + }), + getMetaTypes: vi.fn().mockResolvedValue([]), + getMetaItems: vi.fn().mockResolvedValue([{ name: 'crm_account' }]), + getMetaItem: vi.fn().mockResolvedValue({}), + findData: vi.fn().mockResolvedValue([]), + createData: vi.fn().mockResolvedValue({}), + updateData: vi.fn().mockResolvedValue({}), + deleteData: vi.fn().mockResolvedValue({}), + batchData: vi.fn().mockResolvedValue({}), + createManyData: vi.fn().mockResolvedValue({}), + updateManyData: vi.fn().mockResolvedValue({}), + deleteManyData: vi.fn().mockResolvedValue({}), + ...protocolOverrides, + }; + const rest = new RestServer( + createMockServer() as any, + protocol, + { api: { requireAuth: false } } as any, + ); + (rest as any).resolveExecCtx = async () => ({ userId: 'u1' }); + rest.registerRoutes(); + return rest; +} + +function routeOf(rest: any, method: string, path: string) { + const route = rest.getRoutes().find((r: any) => r.method === method && r.path === path); + if (!route) throw new Error(`${method} ${path} route not registered`); + return route; +} + +async function call(rest: any, method: string, path: string, req: Record) { + const res = makeRes(); + await routeOf(rest, method, path).handler({ method, query: {}, headers: {}, ...req }, res); + return res; +} + +/** The wire answer `resolveErrorResponse` produces, reached through its one exported caller. */ +function throughRouteDoor(error: any, object?: string): { status: number; body: any } { + const res = makeRes(); + handleRouteError(res, error, object); + return { status: res.statusCode, body: res.body }; +} + +let errorSpy: ReturnType; +beforeEach(() => { errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); }); +afterEach(() => { errorSpy.mockRestore(); }); + +const BULK_ROUTES: Array<{ + name: string; path: string; method: string; protocolKey: string; + business: string; req: Record; +}> = [ + { + name: 'batch', method: 'POST', path: `${DATA_COLLECTION}/batch`, protocolKey: 'batchData', + business: 'Opportunity is closed (closed_won); only description, next_step, notes may be edited.', + req: { params: { object: 'crm_opportunity' }, body: { operation: 'update', records: [{ id: 'r1', name: 'x' }] } }, + }, + { + name: 'createMany', method: 'POST', path: `${DATA_COLLECTION}/createMany`, protocolKey: 'createManyData', + business: 'Another contact (Ada Lovelace) with email ada@example.com already exists.', + req: { params: { object: 'crm_contact' }, body: [{ email: 'a@b.com' }] }, + }, + { + name: 'updateMany', method: 'POST', path: `${DATA_COLLECTION}/updateMany`, protocolKey: 'updateManyData', + business: '制作基地被「项目主计划批次」引用(3 条),删除被阻断,请先解除引用', + req: { params: { object: 'crm_opportunity' }, body: { records: [{ id: 'r1', data: { name: 'x' } }] } }, + }, + { + name: 'deleteMany', method: 'POST', path: `${DATA_COLLECTION}/deleteMany`, protocolKey: 'deleteManyData', + business: 'Cannot delete customer account: 1 open opportunity still references it.', + req: { params: { object: 'crm_account' }, body: { ids: ['r1'] } }, + }, + { + name: 'clone', method: 'POST', path: `${DATA_ITEM}/clone`, protocolKey: 'cloneData', + business: 'Do Not Call is set on this contact.', + req: { params: { object: 'crm_account', id: 'r1' }, body: {} }, + }, +]; + +// --------------------------------------------------------------------------- +// §1 The card's rows, walked on the real route handlers in process +// --------------------------------------------------------------------------- + +describe('[#11588] a bulk write answers with the hook\'s own sentence, not the debug wrapper', () => { + for (const route of BULK_ROUTES) { + it(`${route.name}: a \`status\`-declared 409 reaches the client unwrapped`, async () => { + const rest = setup({ + [route.protocolKey]: vi.fn().mockRejectedValue( + sandboxRefusal(route.business, { code: 'RECORD_LOCKED', status: 409 }), + ), + }); + + const res = await call(rest, route.method, route.path, route.req); + + expect(res.statusCode).toBe(409); + expect(res.body.error).toBe(route.business); + // The prefix is the defect, asserted on the WHOLE body: a repair + // that merely moved the wrapper into a second field would pass an + // `error`-only pin. + expect(JSON.stringify(res.body)).not.toMatch(WRAPPER_RE); + // #11590's half must survive this one — same envelope, other field. + expect(res.body.code).toBe('RECORD_LOCKED'); + }, 60_000); + } + + it('CONTROL — the single-row PATCH the card measured as correct is still correct', async () => { + // The row that proves the defect was a BRANCH, not a policy about bulk + // writes: same producer, same status, different exit. Green before and + // after; it reddens only if the repair disturbed the unwrap door. + const rest = setup({ + updateData: vi.fn().mockRejectedValue(sandboxRefusal( + 'Opportunity is closed (closed_won); only description, next_step, notes may be edited.', + { code: 'RECORD_LOCKED', status: 409 }, + )), + }); + + const res = await call(rest, 'PATCH', DATA_ITEM, { + params: { object: 'crm_opportunity', id: 'rec1' }, body: { amount: 10 }, + }); + + expect(res.statusCode).toBe(409); + expect(res.body.error).toBe( + 'Opportunity is closed (closed_won); only description, next_step, notes may be edited.', + ); + expect(JSON.stringify(res.body)).not.toMatch(WRAPPER_RE); + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// §2 The two spellings, on one route — the asymmetry the card named +// --------------------------------------------------------------------------- + +describe('[#11588] one hook cannot yield two sentences depending on the spelling its author picked', () => { + for (const route of BULK_ROUTES) { + it(`${route.name}: \`status\` and \`statusCode\` answer with the SAME message`, async () => { + const answers = []; + for (const spelling of ['status', 'statusCode'] as const) { + const rest = setup({ + [route.protocolKey]: vi.fn().mockRejectedValue( + sandboxRefusal(route.business, { code: 'RECORD_LOCKED', [spelling]: 409 }), + ), + }); + const res = await call(rest, route.method, route.path, route.req); + answers.push({ spelling, status: res.statusCode, error: res.body.error }); + } + + // Asserted as the DIFFERENCE, not as one reading: pre-fix + // `statusCode` was already unwrapped and `status` was not. + expect(answers[0].error).toBe(answers[1].error); + expect(answers[0].status).toBe(answers[1].status); + expect(answers[0].error).toBe(route.business); + }, 60_000); + } +}); + +// --------------------------------------------------------------------------- +// §3 The dogfood-pinned default, unmoved +// --------------------------------------------------------------------------- + +describe('[#11588] the undeclared-status refusal still answers 400 with its own words', () => { + it('a bulk refusal declaring NO status keeps the verbatim-message 400', async () => { + // Green before and after: with no declared `status` the passthrough + // never opens and the error reaches the unwrap door as it always did. + // Here so a repair that widened the passthrough's gate is caught. + const rest = setup({ + deleteManyData: vi.fn().mockRejectedValue( + sandboxRefusal('month-end close is in progress'), + ), + }); + + const res = await call(rest, 'POST', `${DATA_COLLECTION}/deleteMany`, { + params: { object: 'crm_account' }, body: { ids: ['r1'] }, + }); + + expect(res.statusCode).toBe(400); + expect(res.body.error).toBe('month-end close is in progress'); + }, 60_000); +}); + +// --------------------------------------------------------------------------- +// §4 Door-to-door — the invariant `resolveErrorResponse`'s own docblock asserts +// --------------------------------------------------------------------------- + +describe('[#11588 / #7525] the two doors agree on the wire answer for a sandbox refusal', () => { + it('every 4xx status, both spellings, both doors — one sentence', () => { + // The passthrough docblock claims the two doors "already agree on the + // wire answer". Pinned mechanically rather than by comment: the claim + // was false for exactly this producer, and a comment cannot go red. + for (const status of [400, 401, 403, 404, 409, 422, 423, 451]) { + const routeDoor = throughRouteDoor( + sandboxRefusal('this record is frozen', { code: 'RECORD_LOCKED', status }), + ); + const dataDoor = mapDataError( + sandboxRefusal('this record is frozen', { code: 'RECORD_LOCKED', statusCode: status }), + ); + + expect(routeDoor.status, `status=${status}`).toBe(status); + expect(dataDoor.status, `statusCode=${status}`).toBe(status); + expect(routeDoor.body.error, `status=${status}`).toBe('this record is frozen'); + expect(dataDoor.body.error, `statusCode=${status}`).toBe('this record is frozen'); + expect(routeDoor.body.code, `status=${status}`).toBe(dataDoor.body.code); + } + }); +}); + +// --------------------------------------------------------------------------- +// §5 POSITIVE CONTROLS — the repair is a READ, never a pattern-strip +// +// A fix that removed the wrapper by matching `hook '…' threw: ` off +// `error.message` would satisfy every assertion above and redden here. That is +// the whole reason this section exists. +// --------------------------------------------------------------------------- + +describe('[#11588] a NON-sandboxed error\'s message is untouched, character for character', () => { + it('a plain 4xx refusal comes through verbatim', () => { + const r = throughRouteDoor(plainRefusal('Opportunity is closed.', { status: 409 })); + expect(r.status).toBe(409); + expect(r.body.error).toBe('Opportunity is closed.'); + }); + + it('⭐ a plain error whose OWN text looks like the wrapper is NOT rewritten', () => { + // The trap for a pattern-strip. No `.innerMessage`, so nothing about + // this error was produced by the sandbox — its message is the + // producer's, wrapper-shaped or not, and the boundary does not + // paraphrase a 4xx (#5423). + const text = "hook 'guard' threw: Error: locked"; + const r = throughRouteDoor(plainRefusal(text, { status: 409, code: 'RECORD_LOCKED' })); + expect(r.body.error).toBe(text); + }); + + it('⭐ an `innerMessage` that is not a non-empty string falls back to `.message`', () => { + // The gate is the sandbox's own field, read exactly as the unwrap door + // reads it — not "does this look like a hook error". + for (const inner of [undefined, null, '', 42, {}]) { + const err = plainRefusal("hook 'g' threw: Error: refused", { status: 409 }); + (err as any).innerMessage = inner; + expect(throughRouteDoor(err).body.error, String(inner)).toBe( + "hook 'g' threw: Error: refused", + ); + } + }); + + it('⭐ a declared `userMessage` still rides, and is still the producer\'s text', () => { + // #9934's marked channel is orthogonal to the unwrap and must stay so. + const err = sandboxRefusal('this record is frozen', { + status: 409, code: 'RECORD_LOCKED', userMessage: '该记录已锁定', + }); + const r = throughRouteDoor(err); + expect(r.body.error).toBe('this record is frozen'); + expect(r.body.userMessage).toBe('该记录已锁定'); + }); +}); + +// --------------------------------------------------------------------------- +// §6 The withholds and defaults this repair must not move +// --------------------------------------------------------------------------- + +describe('[#11588] #5437/#5582 and #5423 are exactly where they were', () => { + it('a sandbox refusal declaring a 5xx still has its prose withheld — both spellings', () => { + // The load-bearing ordering. The 4xx arm is the only one this card + // touched; if the unwrap had been hoisted above the passthrough instead, + // the business text would ship at 503 and this goes red. + const viaRoute = throughRouteDoor( + sandboxRefusal('upstream ledger unavailable at 10.0.0.5:5432', { + code: 'SERVICE_UNAVAILABLE', status: 503, + }), + ); + expect(viaRoute.status).toBe(503); + expect(viaRoute.body.error).toBe(INTERNAL_ERROR_MESSAGE); + + const viaData = mapDataError( + sandboxRefusal('upstream ledger unavailable at 10.0.0.5:5432', { + code: 'SERVICE_UNAVAILABLE', statusCode: 503, + }), + ); + expect(viaData.status).toBe(503); + expect(viaData.body.error).toBe(INTERNAL_ERROR_MESSAGE); + }); + + it('an over-long business message is TRUNCATED, not replaced (#5423)', () => { + const long = 'x'.repeat(700); + const r = throughRouteDoor(sandboxRefusal(long, { status: 409 })); + expect(r.body.error).toHaveLength(500); + expect(r.body.error.endsWith('…')).toBe(true); + }); + + it('a non-string message with no `innerMessage` still degrades to `Request failed`', () => { + const err: any = { status: 409, message: { not: 'a string' } }; + expect(throughRouteDoor(err).body.error).toBe('Request failed'); + }); + + it('an EMPTY string message with no `innerMessage` is still served as itself', () => { + // Deliberately not `'Request failed'`: this arm's degrade is keyed on + // the TYPE, unlike `classifyDataError`'s sibling which also checks + // length. Pinned because the repair rewrote this very expression. + expect(throughRouteDoor(plainRefusal('', { status: 409 })).body.error).toBe(''); + }); + + it('`OBJECT_NOT_FOUND` still bypasses the passthrough entirely (#3770)', () => { + const r = throughRouteDoor( + sandboxRefusal('no such object', { code: 'OBJECT_NOT_FOUND', status: 409 }), + ); + expect(r.body.code).toBe('OBJECT_NOT_FOUND'); + }); +}); + +// --------------------------------------------------------------------------- +// §7 MEASURED AND NOT REPAIRED — recorded so it is not rediscovered as new +// --------------------------------------------------------------------------- + +describe('[#11588] the crash-with-a-declared-4xx divergence this card does NOT close', () => { + it('a CRASHED body carrying a declared 4xx still answers differently at the two doors', () => { + // `sandboxBusinessMessage` declines a script fault (#7543), so the + // passthrough's answer here is byte-identical to before this card — + // deliberately. The unwrap door sanitises the same error to a 500; + // making the two agree means moving the STATUS the passthrough + // decided, which is a contract question and not this card's. Green on + // both sides of the fix: it documents the gap, it does not bless it. + const crash = () => sandboxRefusal('x', { status: 409 }); + const withCrash = () => { + const e = crash(); + (e as any).innerMessage = 'TypeError: obj.foo is not a function'; + return e; + }; + + const viaRoute = throughRouteDoor(withCrash()); + expect(viaRoute.status).toBe(409); + expect(viaRoute.body.error).toBe("hook 'guard' threw: Error: x"); + + const viaData = mapDataError(withCrash()); + expect(viaData.status).toBe(500); + expect(viaData.body.error).toBe(INTERNAL_ERROR_MESSAGE); + }); +}); From 7bf57f4e3244083108000d33ce522263d11eb801 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 11:15:43 +0000 Subject: [PATCH 2/5] test(rest): re-shelve the mis-predicted pin, and add the changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `userMessage` case was drafted as a §5 positive control and predicted green; it measured RED pre-fix because its fixture is a sandboxed refusal, so its `error` assertion reads the defect like §1 does. Moved to §6 and the claim corrected — the prediction is recorded as wrong rather than re-fitted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- .../bulk-write-refusal-message-parity.md | 58 +++++++++++++++++++ .../rest-hook-refusal-message-parity.test.ts | 43 +++++++++----- 2 files changed, 88 insertions(+), 13 deletions(-) create mode 100644 .changeset/bulk-write-refusal-message-parity.md diff --git a/.changeset/bulk-write-refusal-message-parity.md b/.changeset/bulk-write-refusal-message-parity.md new file mode 100644 index 0000000000..3c2b2a9987 --- /dev/null +++ b/.changeset/bulk-write-refusal-message-parity.md @@ -0,0 +1,58 @@ +--- +'@objectstack/rest': patch +--- + +Serve a sandboxed hook's own refusal sentence on the bulk write routes, instead +of the QuickJS debug wrapper + +A hook's `throw new Error('删除被阻断…')` is a deliberate business rule, and +`classifyDataError`'s sandbox unwrap door exists precisely so the end user sees +only that sentence — the ` '' threw: ` prefix "belongs in +server logs", in the door's own words. Six write routes never reached the door. +Measured against the real route handlers: `PATCH /api/v1/data/:object/:id` +answered `Opportunity is closed.` while `POST …/batch`, `…/createMany`, +`…/updateMany`, `…/deleteMany` and `…/:id/clone` answered +`hook 'guard' threw: Error: Opportunity is closed.` — one hook, one refusal, a +different sentence depending on which route the caller happened to use. + +The branch is `resolveErrorResponse`'s declared-status passthrough, which is +checked *before* it delegates to `mapDataError` and answered its 4xx arm from +`error.message`. It now reads the business text through `sandboxBusinessMessage` +— the unwrap door's own two conditions (a non-empty string `.innerMessage`, and +not a `isScriptFaultMessage` crash) named once so the two doors ask the same +question. + +**Not a reorder.** The passthrough's own docblock argues the ordering: handing a +declared 5xx to `mapDataError` re-labels it from the message TEXT (the +overlay-delete fault comes back `404 OBJECT_NOT_FOUND` and stops being logged), +so the arm stays exactly where it is and keeps deciding the status. Only the +sentence it reads changes. #5437/#5582's unconditional 5xx prose withhold is +untouched — a sandbox refusal declaring a 5xx still answers with the generic +text, pinned on both spellings. + +What this restores is an invariant the same docblock already asserts. Its #7525 +paragraph says an error declaring `statusCode` instead falls to `mapDataError`, +"So the two doors already agree on the wire answer." For a sandbox refusal that +was false — `statusCode` was unwrapped and `status` was not — which is the +two-spellings asymmetry this card was filed on. The doors agree again, pinned +door-to-door across the whole 4xx band rather than asserted in a comment. + +**Bump level: `patch`, argued rather than defaulted.** The change is to message +TEXT on shipped routes, so the level is not automatic. It is a patch because +nothing about the envelope's contract moves: same status, same `code`, same +field set, no request newly accepted or refused. The delta is that one string +loses a debug prefix that this boundary already declares must never be on the +wire, and that the single-row routes never emitted — so no client could have +been reading it uniformly in the first place. Keying on the prefix would mean +substring-matching prose that is localised and deliberately reworded over time, +which is the practice the ADR-0112 `code` vocabulary exists to remove. + +**Not covered, measured and reported rather than quietly widened:** +`POST /api/v1/analytics/dataset/query` builds its own `{ code, message }` +envelope inline in `rest-server.ts` and reads `error.message` directly; it +shares no branch with the above and still serves the wrapper. The record-share +routes (`…/:id/shares`) are a third branch again — `respondSharingError` matches +on `error.message` and the fallback interpolates it into a `500`. Both live in a +file held by another open PR this round and are filed separately. +`POST …/import` and `GET …/export` exit through `handleRouteError` like the +bulk routes and are repaired by the same change. diff --git a/packages/rest/src/rest-hook-refusal-message-parity.test.ts b/packages/rest/src/rest-hook-refusal-message-parity.test.ts index f963c8c891..776994fc9d 100644 --- a/packages/rest/src/rest-hook-refusal-message-parity.test.ts +++ b/packages/rest/src/rest-hook-refusal-message-parity.test.ts @@ -62,14 +62,25 @@ // §2 predicted RED 5 measured 5 red — as predicted. // §3 predicted GREEN measured green — as predicted. // §4 predicted RED 1 measured 1 red — as predicted. -// §5 predicted GREEN throughout measured green — as predicted. These are the -// positive controls: a fix that stripped the wrapper by PATTERN instead of -// reading `.innerMessage` reddens here and nowhere else. -// §6 predicted GREEN throughout measured green — as predicted. +// §5 predicted GREEN throughout measured GREEN — as predicted, and these +// three are the real positive controls: a fix that stripped the wrapper by +// PATTERN instead of reading `.innerMessage` reddens here and nowhere +// else. A FOURTH case was originally written into this section and the +// prediction for it was WRONG — see §6's `userMessage` pin below. +// §6 predicted GREEN throughout measured 1 RED. The prediction was wrong, +// recorded rather than rewritten. "a declared `userMessage` still rides" +// was drafted into §5 and predicted green as a control, but its fixture is +// a SANDBOXED refusal, so its `error` assertion reads the defect and reds +// pre-fix like §1 does. It is not a control and never was; it was +// mis-shelved, the red is the correct answer for it, and it has been moved +// here — to the section for "what must not move" — rather than having its +// prediction quietly re-fitted. Nothing about the test changed, only the +// shelf and the claim made about it. // §7 predicted GREEN both sides measured green — as predicted. It pins a // divergence this card does NOT repair; see its own comment. // -// Total 11 of 30 red pre-fix (prediction: 11). +// Total 12 of 23 red pre-fix. Prediction: 11 of 23 — off by the one case above. +// Measured: `Tests 12 failed | 11 passed (23)`. // --------------------------------------------------------------------------- import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; @@ -373,9 +384,21 @@ describe('[#11588] a NON-sandboxed error\'s message is untouched, character for ); } }); +}); + +// --------------------------------------------------------------------------- +// §6 The withholds and defaults this repair must not move +// --------------------------------------------------------------------------- - it('⭐ a declared `userMessage` still rides, and is still the producer\'s text', () => { - // #9934's marked channel is orthogonal to the unwrap and must stay so. +describe('[#11588] #5437/#5582 and #5423 are exactly where they were', () => { + it('a declared `userMessage` still rides, beside the now-unwrapped `error`', () => { + // ⚠️ Drafted as a §5 positive control and predicted GREEN. It measured + // RED pre-fix, because the fixture is a SANDBOXED refusal: the `error` + // assertion reads the defect exactly as §1 does, and only the + // `userMessage` half is direction-insensitive. Moved here and the claim + // corrected rather than the prediction re-fitted — #9934's marked + // channel is orthogonal to the unwrap and must stay so, which is a + // "must not move" and not a control. const err = sandboxRefusal('this record is frozen', { status: 409, code: 'RECORD_LOCKED', userMessage: '该记录已锁定', }); @@ -383,13 +406,7 @@ describe('[#11588] a NON-sandboxed error\'s message is untouched, character for expect(r.body.error).toBe('this record is frozen'); expect(r.body.userMessage).toBe('该记录已锁定'); }); -}); - -// --------------------------------------------------------------------------- -// §6 The withholds and defaults this repair must not move -// --------------------------------------------------------------------------- -describe('[#11588] #5437/#5582 and #5423 are exactly where they were', () => { it('a sandbox refusal declaring a 5xx still has its prose withheld — both spellings', () => { // The load-bearing ordering. The 4xx arm is the only one this card // touched; if the unwrap had been hoisted above the passthrough instead, From e7f55ec5de83e5ffa9b590c2abf6fdf4c6a09770 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 11:29:07 +0000 Subject: [PATCH 3/5] fix(rest): unwrap the sandbox refusal on /analytics/dataset/query too The route builds its `{ code, message }` envelope by hand and read `error.message` directly, sharing no branch with either door in `error-response.ts`. It now reads the exported `sandboxBusinessMessage`, so the analytics face and the /data face cannot answer one refusal two ways. Scoped to what the client reads: `logError` still receives the whole error and `looksLikeInternalErrorLeak` still reads the raw text. Neither arm's status moves. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- packages/rest/src/error-response.ts | 9 +- .../rest-hook-refusal-message-parity.test.ts | 97 +++++++++++++++++++ packages/rest/src/rest-server.ts | 28 +++++- 3 files changed, 131 insertions(+), 3 deletions(-) diff --git a/packages/rest/src/error-response.ts b/packages/rest/src/error-response.ts index e9a73c0334..48df585c42 100644 --- a/packages/rest/src/error-response.ts +++ b/packages/rest/src/error-response.ts @@ -260,8 +260,15 @@ function isScriptFaultMessage(message: string): boolean { * the caller's message is the remedy on a 4xx (#5423) — the one thing this * boundary must not paraphrase. `rest-hook-refusal-message-parity.test.ts` §5 * is the control that keeps it a read. + * + * Exported for the SECOND boundary that has to ask the same question: + * `/analytics/dataset/query` builds its own `{ code, message }` envelope inline + * in `rest-server.ts` and shares no branch with either door here. It reads this + * rather than re-deriving the unwrap, so the analytics face and the `/data` + * face cannot drift into two answers for one refusal — the door-disagreement + * shape #7525/#8016 keeps producing when a boundary open-codes the read. */ -function sandboxBusinessMessage(error: any): string | undefined { +export function sandboxBusinessMessage(error: any): string | undefined { if (typeof error?.innerMessage !== 'string' || !error.innerMessage) return undefined; if (isScriptFaultMessage(error.innerMessage)) return undefined; return error.innerMessage; diff --git a/packages/rest/src/rest-hook-refusal-message-parity.test.ts b/packages/rest/src/rest-hook-refusal-message-parity.test.ts index 776994fc9d..f38ddc8c3a 100644 --- a/packages/rest/src/rest-hook-refusal-message-parity.test.ts +++ b/packages/rest/src/rest-hook-refusal-message-parity.test.ts @@ -483,3 +483,100 @@ describe('[#11588] the crash-with-a-declared-4xx divergence this card does NOT c expect(viaData.body.error).toBe(INTERNAL_ERROR_MESSAGE); }); }); + +// --------------------------------------------------------------------------- +// §8 `/analytics/dataset/query` — the SEVENTH row, and a THIRD branch again +// +// This route builds its `{ code, message }` envelope by hand and reads +// `error.message` directly. It touches neither `classifyDataError`'s unwrap door +// nor `resolveErrorResponse`'s passthrough, so nothing in §1–§7 reaches it: it +// needed the same read applied at its own boundary, importing +// `sandboxBusinessMessage` rather than re-deriving the unwrap. +// +// Both of its client emissions are covered, because the card's repro exercised +// only the first: +// ① a declared 4xx + `code` → `{ code, message }` +// ③ everything else → `500 ANALYTICS_QUERY_FAILED` +// Neither arm's STATUS moves here. ③ answering 500 for an undeclared hook +// refusal (where `/data` answers 400) is a separate defect and is NOT touched. +// +// Predicted before running: §8a RED, §8b RED, §8c GREEN, §8d GREEN. +// --------------------------------------------------------------------------- + +const ANALYTICS_PATH = '/api/v1/analytics/dataset/query'; +const DATASET = { + name: 'pipeline', + label: 'Pipeline', + object: 'crm_opportunity', + dimensions: [{ name: 'stage', field: 'stage', type: 'string' }], + measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }], +}; +const SELECTION = { dimensions: ['stage'], measures: ['revenue'] }; + +async function analyticsRefusal(error: any) { + const rest = setup(); + (rest as any).analyticsServiceProvider = async () => ({ + queryDataset: vi.fn().mockRejectedValue(error), + }); + const res = makeRes(); + const route = rest.getRoutes().find( + (r: any) => r.method === 'POST' && r.path === ANALYTICS_PATH, + ); + if (!route) throw new Error(`${ANALYTICS_PATH} route not registered`); + await route.handler( + { method: 'POST', params: {}, query: {}, headers: {}, body: { dataset: DATASET, selection: SELECTION } }, + res, + ); + return res; +} + +describe('[#11588] the analytics dataset face answers in the hook\'s words too', () => { + it('§8a a declared 4xx + code reaches the client unwrapped', async () => { + const res = await analyticsRefusal( + sandboxRefusal('This dataset is locked during month-end close.', { + code: 'RECORD_LOCKED', status: 409, + }), + ); + + expect(res.statusCode).toBe(409); + expect(res.body.code).toBe('RECORD_LOCKED'); + expect(res.body.message).toBe('This dataset is locked during month-end close.'); + expect(JSON.stringify(res.body)).not.toMatch(WRAPPER_RE); + }, 60_000); + + it('§8b an UNDECLARED refusal reaches the 500 arm unwrapped — the status is NOT moved', async () => { + // The sub-case the card's repro did not exercise. `/data` answers 400 + // with this sentence and analytics answers 500 with it; that status + // disagreement is a separate defect and is deliberately left standing — + // asserted here so the next reader sees it was measured, not missed. + const res = await analyticsRefusal(sandboxRefusal('month-end close is in progress')); + + expect(res.statusCode).toBe(500); + expect(res.body.code).toBe('ANALYTICS_QUERY_FAILED'); + expect(res.body.error).toBe('month-end close is in progress'); + expect(JSON.stringify(res.body)).not.toMatch(WRAPPER_RE); + }, 60_000); + + it('§8c a declared 5xx still has its prose withheld', async () => { + const res = await analyticsRefusal( + sandboxRefusal('connect ECONNREFUSED 10.0.0.5:5432', { + code: 'SERVICE_UNAVAILABLE', status: 503, + }), + ); + + expect(res.statusCode).toBe(500); + expect(res.body.error).toBe(INTERNAL_ERROR_MESSAGE); + }, 60_000); + + it('§8d ⭐ POSITIVE CONTROL — a non-sandboxed refusal is still verbatim', async () => { + // Same trap as §5: no `.innerMessage`, wrapper-shaped text, must survive + // character for character. A pattern-strip at this boundary reddens here. + const text = "hook 'guard' threw: Error: locked"; + const res = await analyticsRefusal( + plainRefusal(text, { code: 'RECORD_LOCKED', status: 409 }), + ); + + expect(res.statusCode).toBe(409); + expect(res.body.message).toBe(text); + }, 60_000); +}); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index a1e41dfee9..b1fbe095d7 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -192,6 +192,7 @@ import { logError, logWarn } from './log.js'; // surface `./rest-server.js` has always offered is byte-identical. import { mapDataError, + sandboxBusinessMessage, sendThrownError, sendDeclaredFault, sendFieldVisibilityFault, @@ -9078,6 +9079,29 @@ export class RestServer { res.json(result); } catch (error: any) { const msg = String(error?.message ?? error ?? ''); + // [#11588] The text addressed to the CALLER, which for a + // sandboxed hook refusal is the business message rather than + // the `hook '' threw: Error: …` QuickJS debug wrapper + // sitting on `.message`. This route builds its envelope by + // hand and shares no branch with `classifyDataError`'s + // unwrap door or with `resolveErrorResponse`'s passthrough, + // so it shipped the wrapper on every hook refusal while the + // single-row `/data` routes served the author's sentence — + // one refusal, two wordings, decided by which face caught it. + // {@link sandboxBusinessMessage} is imported rather than + // re-derived precisely so a third answer cannot appear here. + // + // Deliberately scoped to what the CLIENT reads. `logError` + // below still receives the whole error, wrapper and all — + // that is the half the wrapper was written for. And + // `looksLikeInternalErrorLeak` below still reads the RAW + // `msg`: feeding it the unwrapped text could only make it + // withhold LESS, and a leak predicate must never be handed a + // narrower input than the one it was calibrated on. + // + // Neither arm's STATUS moves. ① keeps answering the declared + // 4xx and ③ keeps answering 500; only the sentence changes. + const clientMsg = sandboxBusinessMessage(error) ?? msg; // ── [#5352] ① The ADR-0112 envelope, read FIRST ────────── // A thrown error that already carries `code` + a 4xx // `status` has ANSWERED the classification question. This @@ -9109,7 +9133,7 @@ export class RestServer { const envelopeStatus = typeof error?.status === 'number' ? error.status : undefined; const envelopeCode = typeof error?.code === 'string' && error.code.length > 0 ? error.code : undefined; if (envelopeStatus !== undefined && envelopeStatus >= 400 && envelopeStatus < 500 && envelopeCode) { - return res.status(envelopeStatus).json({ code: envelopeCode, message: msg.slice(0, 1000) }); + return res.status(envelopeStatus).json({ code: envelopeCode, message: clientMsg.slice(0, 1000) }); } // ── ② … is GONE. The message-sniffing list is retired ──── // [#5367] `/analytics/dataset/query` used to classify six @@ -9197,7 +9221,7 @@ export class RestServer { logError('[REST] Analytics dataset query error:', error); const outward = declaresServerFault(error) || looksLikeInternalErrorLeak(msg) ? INTERNAL_ERROR_MESSAGE - : msg.slice(0, 500); + : clientMsg.slice(0, 500); res.status(500).json({ code: 'ANALYTICS_QUERY_FAILED', error: outward }); } }, From de2819fca454fb08b8e7728bf5b78a3715a916fa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 11:30:34 +0000 Subject: [PATCH 4/5] =?UTF-8?q?docs(rest):=20record=20the=20=C2=A78=20meas?= =?UTF-8?q?urement=20and=20correct=20the=20changeset=20for=20the=20analyti?= =?UTF-8?q?cs=20half?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- .../bulk-write-refusal-message-parity.md | 29 +++++++++++++------ .../rest-hook-refusal-message-parity.test.ts | 15 ++++++++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/.changeset/bulk-write-refusal-message-parity.md b/.changeset/bulk-write-refusal-message-parity.md index 3c2b2a9987..dfb6ccced0 100644 --- a/.changeset/bulk-write-refusal-message-parity.md +++ b/.changeset/bulk-write-refusal-message-parity.md @@ -47,12 +47,23 @@ been reading it uniformly in the first place. Keying on the prefix would mean substring-matching prose that is localised and deliberately reworded over time, which is the practice the ADR-0112 `code` vocabulary exists to remove. -**Not covered, measured and reported rather than quietly widened:** -`POST /api/v1/analytics/dataset/query` builds its own `{ code, message }` -envelope inline in `rest-server.ts` and reads `error.message` directly; it -shares no branch with the above and still serves the wrapper. The record-share -routes (`…/:id/shares`) are a third branch again — `respondSharingError` matches -on `error.message` and the fallback interpolates it into a `500`. Both live in a -file held by another open PR this round and are filed separately. -`POST …/import` and `GET …/export` exit through `handleRouteError` like the -bulk routes and are repaired by the same change. +`POST /api/v1/analytics/dataset/query` — the seventh row — needed its own +repair: it builds a `{ code, message }` envelope inline and touches neither +door. It now imports the same `sandboxBusinessMessage` rather than re-deriving +the unwrap, so the analytics face and the `/data` face cannot answer one refusal +two ways. Both of its client emissions are covered (the declared-4xx envelope +and the `500 ANALYTICS_QUERY_FAILED` fallback); `logError` still receives the +whole error and `looksLikeInternalErrorLeak` still reads the raw text, so the +operator's copy and the leak heuristic are untouched. + +`POST …/import` and `GET …/export` exit through `handleRouteError` like the bulk +routes, so they are repaired by the same change — measured rather than assumed. + +**Measured and deliberately NOT repaired here**, each recorded so it is not +rediscovered as new: the record-share routes (`…/:id/shares`, list/grant/revoke) +are a third branch again — `respondSharingError` classifies by +`message.startsWith(CODE)` and its fallback interpolates `error.message` into a +hand-built `500`, ignoring a declared `status`/`code` entirely. And on the +analytics route an *undeclared* hook refusal answers `500` where `/data` answers +`400`; only the sentence was corrected, the status disagreement is a separate +defect. Both are filed as their own issues. diff --git a/packages/rest/src/rest-hook-refusal-message-parity.test.ts b/packages/rest/src/rest-hook-refusal-message-parity.test.ts index f38ddc8c3a..780230e4ba 100644 --- a/packages/rest/src/rest-hook-refusal-message-parity.test.ts +++ b/packages/rest/src/rest-hook-refusal-message-parity.test.ts @@ -79,8 +79,19 @@ // §7 predicted GREEN both sides measured green — as predicted. It pins a // divergence this card does NOT repair; see its own comment. // -// Total 12 of 23 red pre-fix. Prediction: 11 of 23 — off by the one case above. -// Measured: `Tests 12 failed | 11 passed (23)`. +// §8 predicted RED 2 / GREEN 2 measured 2 red — as predicted. Added after +// the file hold on `rest-server.ts` lifted mid-task; its legs were re-run +// against the merged base rather than carried across it. +// +// Total 14 of 27 red pre-fix, measured `Tests 14 failed | 13 passed (27)`. +// Predictions: 11 of 23 for §1–§7 (off by the one mis-shelved case above) and +// 2 of 4 for §8 (exact). +// +// The §1–§7 legs reverted `error-response.ts` alone; the final leg reverted +// BOTH it and `rest-server.ts` to the merged `origin/main`. Mutation proven on +// disk each time by grepping the injected AND the removed text in both files +// (injected 1/1 → 0/0, removed 0/0 → 1/2), and the restore re-verified by +// re-reading both files from the repository root rather than trusting the trap. // --------------------------------------------------------------------------- import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; From f93aa39e37bf9b38f24ce978b31104fffdd97200 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 11:53:34 +0000 Subject: [PATCH 5/5] test(rest): keep the analytics harness off the TEST_DEBT ratchet The hand-built request literal is not assignable to IHttpRequest, which added one raw tsc error to @objectstack/rest's TEST_DEBT entry (155 -> 156). Typed `any` like the sibling route helpers in this file, so the ledger is back at its recorded 155. Fixed rather than raising the entry -- the ratchet is shrink-only. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019siH5jDmk5hrayvfyojUqR --- packages/rest/src/rest-hook-refusal-message-parity.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/rest/src/rest-hook-refusal-message-parity.test.ts b/packages/rest/src/rest-hook-refusal-message-parity.test.ts index 780230e4ba..e563196b27 100644 --- a/packages/rest/src/rest-hook-refusal-message-parity.test.ts +++ b/packages/rest/src/rest-hook-refusal-message-parity.test.ts @@ -525,7 +525,12 @@ const DATASET = { const SELECTION = { dimensions: ['stage'], measures: ['revenue'] }; async function analyticsRefusal(error: any) { - const rest = setup(); + // `any` deliberately, matching `routeOf`/`call` above: the route's `handler` + // is typed against the server's full `IHttpRequest`, and a hand-built + // request literal is not assignable to it. Keeping the seam untyped here is + // what the sibling helpers already do; typing it would mean constructing a + // whole request just to satisfy the parameter. + const rest: any = setup(); (rest as any).analyticsServiceProvider = async () => ({ queryDataset: vi.fn().mockRejectedValue(error), });