fix(rest): consult the bespoke structured arms before the declared-status passthrough, so both error doors answer one refusal with one body - #14713
Conversation
…sthrough Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…ror-response-passthrough-ordering
…plains Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
📓 Docs Drift CheckThis PR changes 1 package(s): 18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 7964fd61772560903b0b639d1ca17afb828cd657 && git checkout 7964fd61772560903b0b639d1ca17afb828cd657
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8 d595638d6a1b007e7c951670a83c1f39561cd118 && git checkout -B drift-repro f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8 && git merge --no-ff d595638d6a1b007e7c951670a83c1f39561cd118
node scripts/docs-audit/affected-docs.mjs --json f116b8f8d1b43ec2bfd64fbf0ebb0cf3c301b1c8
|
…orrect three docblock claims Contract-review conditions 2, 4(A), 5 and 7 on #14541. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…or drift, pin the refusal families Contract-review conditions 3, 4, 5 and 6 on #14541. The extended drift guard found a real tenth instance on its first run (RECORD_NOT_FOUND), recorded as a known-gap entry citing #14725 rather than silently excused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…ror-response-passthrough-ordering
…asured wrong Conditions 1, 2, 5, 6 and 7 on #14541: the added keys are not ApiErrorSchema members; the row/door vocabulary fork is disclosed and carried by #14723; the two status moves are stated with per-door measurements; the analytics and record-share families are enumerated; the driver-memory direction is corrected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…ror-response-passthrough-ordering
|
Red-first ablation, re-run in patch round 2 — belongs to the Verification section above. The previous seat's ablation was taken against the pre-condition test file and its report was voided with its container, so it could not speak for §5 and §6 — the two sections conditions 3 and 6 added. This one was taken against the test as it now stands, on head Discipline: the implementation was already committed, so the restore leg points at a Why no rebuild is involved, stated rather than assumed. The test imports its subject relatively — The reading that matters, and the reason this was re-run rather than inherited: §5 and §6 go red too. A drift guard and a key-level pin that stayed green against the pre-fix file would have been cover rather than a check — the mutation removes Predicted direction was RED, and RED is what was observed; nothing here reversed or produced more diagnostics than expected. Generated by Claude Code |
Fixes #14541
resolveErrorResponse— the door behindhandleRouteError/sendThrownError, which every bulk, import/export, metadata and UI route reports through — took its own declared-status passthrough before delegating tomapDataError, the door the single-record/dataroutes call directly. An engine envelope that DECLARESstatustherefore short-circuited, and every bespoke structured arm behind the delegation was unreachable from those routes. One refusal, two bodies, decided by which route caught it.The project had already ruled on this exact shape, for one code. From
error-response.tsitself:This PR generalises that accepted rule rather than proposing a new one. The exclusion never grew past its first case, and the measurements below are the bill for that.
The fix
Per triage's scope guard 2 — "prefer lifting the structured arms into a classification both doors ask first, over adding a second exclusion list":
classifyDataError— every arm ahead of its first message-derived read — move into onestructuredCodeAnswer(error, object).classifyDataErrornow begins by asking it: same arms, same order, same position.resolveErrorResponseasks the same classification before its passthrough, and answers a match by delegating tomapDataError. The two doors are therefore identical by construction — same arm, samewithDeclaredUserMessagewrapper — not by coincidence.code, or the errornamewhere a class is the contract). Nothing in it reads message TEXT to decide which condition this is. That is why thePERMISSION_DENIEDarm (whose third limb sniffs a[Security] Access deniedprefix) and the sandbox unwrap door stay where they are.fiveXxArmDisplacesDeclared4xx, added in this patch round under condition 4): an arm that answers a 5xx never displaces a status the producer declared in the 4xx band. The review measured the two doors answering503and400for one synthesised error, because that guard existed only inresolveErrorResponse. It is now asked at both, which is the honest fix — no producer declares a status on the only 5xx-answering arm, so nothing moves on the wire.Wire-contract delta — measured, not predicted
Both doors driven in process on the same error object, one producer at a time, before and after.
BULKissendThrownError(identical tohandleRouteError; pinned in §3 of the new test).SINGLEismapDataError.The producers whose bodies change
Every field that appears where it did not before
developerMessage,dependentObject,dependentCount,object(DELETE_RESTRICTED) ·currentVersion,currentRecord,object(CONCURRENT_UPDATE) ·developerMessage,field,object(DUPLICATE_RECORD) ·object(FEEDS_DISABLED, FILES_DISABLED, ATTACHMENT_PARENT_ACCESS, ATTACHMENT_DELETE_DENIED, RECORD_NOT_ACCESSIBLE) ·field,object(INVALID_FIELD).Where these keys come from, corrected under condition 1. An earlier revision of this body said they are "already declared on
ApiErrorSchema". ⛔ That was false and is withdrawn.packages/spec/src/api/contract.zod.tsdeclares exactlycode,declaredCode,message,userMessage,category,httpStatus,details,requestId— and none of these keys (re-measured on this tree: zero declarations for all seven). What is true is narrower: every one of them is a key the single-record/datadoor already ships for the same refusal, and this change puts it on the bulk doors too. Their published status, measured onorigin/main:field,objectcontent/docs/protocol/kernel/http-protocol.mdx, the 409 "Constraint Violations" bodydeveloperMessage,dependentObject,dependentCountcontent/docs/protocol/objectql/types.mdx("Required foreign keys") andcontent/docs/api/data-api.mdxcurrentVersioncontent/docs/api/wire-format.mdx§7, "Concurrent Update — 409 Conflict"currentRecordcontent/docs/**page describes it (the only textual match is the unrelatedcurrentRecordCounttenant quota)currentVersionandcurrentRecordbe named shipped-but-undocumented. Re-measured,currentVersionis documented —content/docs/api/wire-format.mdx:327names it in the envelope sentence and:376shows it in the "Concurrent Update — 409 Conflict" example body — and that file was present, with that section, at the reviewer's own base3c1bbd2a87(last touched 2026-08-31 by #13870, before the review). Writing it up as undocumented would put a new falsehood into the changeset in order to satisfy a condition whose stated purpose is "the sentence must be corrected to what is true". SocurrentRecordalone is named as undocumented. The reviewer's grep is otherwise confirmed:currentRecordhas zero hits.No key is removed anywhere. ⛔ "No status moves anywhere" — an earlier claim in this body — is not strictly true; see "Two status moves" below.
Every field whose VALUE changes
Exactly two, both on
DuplicateRecordError:code:DUPLICATE_RECORD→UNIQUE_VIOLATION.error: the engine's sentence → the curated one ("A record with this email already exists").Both are restorations, and this is the one row a reviewer should weigh most:
isUniqueViolationErroris not reachable from an app #14095,engine.insertrethrew the driver's error, which declares nostatus. It therefore fell past this passthrough intomapDataError'sisUniqueViolationErrorarm, and the bulk doors answered409 UNIQUE_VIOLATIONwith the curated sentence andfield.isUniqueViolationErroris not reachable from an app #14095 wrapped that error inDuplicateRecordError, which DOES declarestatus: 409. From that point the bulk doors started answering the engine spellingDUPLICATE_RECORDwith the engine's sentence, while the single-record door was restored explicitly bymapDataErrorhas noDUPLICATE_RECORDarm: the engine's insert conflict envelope reaches the wire through the generic passthrough, dropping thefieldkey and the user-facing conflict sentence #14389.UNIQUE_VIOLATIONback on the bulk doors.DUPLICATE_RECORDremains the in-process code on the thrown envelope — as every dialect code (SQLITE_CONSTRAINT_UNIQUE,23505,ER_DUP_ENTRY) always has.driver-memory, corrected under condition 7. An earlier revision of this body said that on
driver-memory"UNIQUE_VIOLATIONon the bulk doors is new rather than restored". ⛔ That was backwards and is withdrawn.packages/drivers/driver-memory/src/memory-unique-constraint.tsconflictRefusalsetserr.code = UNIQUE_VIOLATION_CODEanderr.status = UNIQUE_VIOLATION_STATUSon the raw refusal itself, so it took the passthrough — and the passthrough relays a registered code verbatim throughthrownCodeFields. The bulk-door code was therefore alreadyUNIQUE_VIOLATIONon that driver before #14095, and it never moved. What changes for that driver is the sentence: its raw message quotes the offending values as JSON and the curated one does not, so the change there is a withheld value, not a new code.The
DUPLICATE_RECORDarm answers the wire spellingUNIQUE_VIOLATION(#14389's ruling). A batch or import row does not go through this classification at all:metadata-protocol'stoRowApiErrorputs a thrown registered code on the row verbatim (ErrorCode.safeParse(err.code).success ? …,protocol.ts:1987), andimport-runner.ts:293(first?.code ?? e?.code ?? 'IMPORT_ROW_FAILED') does the same for row reports — deliberately, per #14095's changeset ("Import row reports improve … nowDUPLICATE_RECORD").⇒ After this change, a whole-request failure on
POST /data/:object/batchorPOST /data/:object/importanswersUNIQUE_VIOLATION, while a row-level failure on the same route answersDUPLICATE_RECORD.Neither half is new and neither is a regression — the fork has existed between the single door and the rows since #14389. What this PR does is move the
UNIQUE_VIOLATIONside onto the routes whose rows speak the other spelling, and the "one condition, one wire code" framing above is true of the DOORS, not of the rows beside them. Stating that plainly is the point of this section.⛔ This PR does not pick a winner. The ledger's rule (
packages/spec/src/api/error-code-ledger.zod.tsheader — "a code either IS the standard member for its condition, or it is registered here — and if it merely re-spells a standard member, that registration is a recorded waiver") and ADR-0112's one-name-per-concept both bear on it, and moving either spelling is a published-contract change rather than a door's call. #14723 carries the decision.Two status moves, stated under condition 5
Both are synthesised shapes that no producer in this repo actually throws, so no wire in service moves — but they are real, they were measured rather than reasoned, and they are pinned in §4 rather than described:
OBJECT_NOT_FOUNDorINVALID_FIELDnow keeps that 5xx. Onorigin/mainsuch an error fell PAST the sandbox unwrap door (declared ≥ 500) into the arm below it and answered404/400. The arms now carry an explicit!isSandboxOriginclause, so it answers the declared status with the prose withheld — mapDataError 的显式状态直通只覆盖 4xx,数据路由上一个声明了 502/503 的生产者拿不回自己的状态码(与 resolveErrorResponse 不对等) #5582's rule, and the better answer. Measured:OBJECT_NOT_FOUND+ declared503answered404 {"error":"Object 'nope' is not registered",…}on both doors and now answers503 {"error":"Internal server error","code":"OBJECT_NOT_FOUND"}on both;INVALID_FIELD+ declared502moves on the single-record door only (the bulk door already answered502).ERR_DATASOURCE_UNAVAILABLE's 503 answers a 5xx and its producer declares nostatus, so nothing moves on the wire; the two doors now agree where they answered503and400.⇒ "
classifyDataErroris behaviour-identical" is therefore not claimed. It is behaviour-identical except for exception 1, which is a deliberate consequence of the arms moving above the unwrap door.The
OBJECT_NOT_FOUNDarm's docblock is corrected too (condition 5's second half): it justified its!isSandboxOriginclause by "the sentence this arm ships iserror.message", which is wrong for that arm — it ships the fixed sentenceObject 'NAME' is not registered. The clause is justified by position only, and the docblock now says so and explicitly disclaims the sibling arm's reason.Is any of this a breaking change on a published surface?
Stated plainly rather than argued away — the review decides, not me:
codeVALUE change onDUPLICATE_RECORDis the one that could break a consumer, and it is genuinely two-directional: a client written againstmainin the window since An application told to rely on a unique index cannot recognise the violation: ObjectQL rethrows the raw driver error andisUniqueViolationErroris not reachable from an app #14095 and branching oncode === 'DUPLICATE_RECORD'at a bulk door will stop matching. A client written before An application told to rely on a unique index cannot recognise the violation: ObjectQL rethrows the raw driver error andisUniqueViolationErroris not reachable from an app #14095, or against the single-record door, or againstobjectui, matchesUNIQUE_VIOLATIONand starts working again.developerMessageon theDUPLICATE_RECORDbody carries the engine's own sentence, which names the object and the column and carries no value;causenever reaches the wire. That ismapDataErrorhas noDUPLICATE_RECORDarm: the engine's insert conflict envelope reaches the wire through the generic passthrough, dropping thefieldkey and the user-facing conflict sentence #14389's ruling for the single-record door, unchanged here.errorsentence on these bodies is no longer passed throughtruncateClientMessageon the bulk doors — the arms ship their own sentence, exactly as they always have on the single-record door. For the measured producers no sentence is anywhere near the 500-character bound.Which doors, and which are published
Every route whose catch calls
handleRouteError/sendThrownError, enumerated fromrest-server.tsby brace-matching each call site to its enclosing route registration (read only — the file is fenced out of this PR), plus each environment-scoped twin:POST /api/v1/data/:object/createMany,/updateMany,/deleteMany,/batch, andPOST /api/v1/batchPOST /api/v1/data/:object/:id/cloneGET /api/v1/data/:object/exportPOST /api/v1/data/:object/import,/import/jobs, and the five/api/v1/data/import/jobs/:jobId…routesGET /api/v1/discovery,GET /api/v1/openapi.json/api/v1/meta/**family —types,diagnostics,_drafts,_migrate-stored,:type,:type/:name(GET/PUT/DELETE),references,layers,history,audit,publish,rollback,diff,published,book/:name/tree,object/:name/state/:field/api/v1surface; first-party Console/Studio is the intended consumerGET /api/v1/ui/view/:object/:type/api/v1surface; first-party Console/Studio is the intended consumerTwo further families reach this same door through
classifiedRefusalAnswerrather than through a route catch — added under condition 6, which the earlier table omitted:POST /api/v1/analytics/dataset/query(and its environment-scoped twin)const { error: refusalText, ...refusalFields } = refusal.body), andservice-analyticsthrowsINVALID_FIELDwithstatus,fieldandobjectat three sites (analytics-service.ts:1710-1713, 1824-1827, 1954-1957), so that route's error body gainsfieldandobjectcode/declaredCode/userMessage/errorinto the nested ADR-0112 D5 envelope. ItserrorSENTENCE does change for aDuplicateRecordErrorThe widened analytics body is newly pinned at key level (§6 of the new test asserts the exact key set
['code','error','field','object'], and thatparamis not relayed), because its own envelope tests —analytics-dataset-unlisted-refusal-envelope.test.tsandanalytics-filter-refusal-envelope.test.ts— assertcodeand a message regex only, so nothing held the keys.Not on these lists, and deliberately so: the single-record
/dataCRUD routes andGET /api/v1/data/:objectcallmapDataErrordirectly — they are the reference behaviour this change converges the doors ON, and none of their bodies move.Which affected code can actually reach which door:
DUPLICATE_RECORDand the403capability/access gates throughcreateMany/updateMany/batch/clone/import;DELETE_RESTRICTEDandCONCURRENT_UPDATEthroughdeleteMany/updateMany/batch;INVALID_FIELDthroughexport(which lists) and the write-path unknown-field refusal on the bulk writes.packages/rest/src/package-routes.tsis not affected: it defines its own localsendThrownErrorand never calls this one.What deliberately does NOT change
Unsupported aggregate function两面都是裸Error(code/status 皆 undefined),且三个 spec 已声明的聚合函数无任何 SQL 后端编译 #5907). Measured after the change: aDELETE_RESTRICTEDdeclaringstatus: 503answers503 {"error":"Internal server error","code":"DELETE_RESTRICTED"}on the bulk door, exactly as before — while the single door keeps reaching the arm and answers409. That divergence is named in §4 rather than pinned on one side: converging it would move a status on a published door, which is outside this card's fence.error.message, which for a QuickJS body is the DEBUG WRAPPER rest: the sandbox debug wrapperhook '<name>' threw: Error: …reaches the client on every write route that exits abovemapDataError's unwrap (batch, createMany, updateMany, deleteMany, clone, analytics) #11588 exists to keep off this wire, so the consult declines an error carryinginnerMessageand the passthrough'ssandboxBusinessMessageread answers it as before (measured:409 {"error":"Opportunity is closed.",…}, neverhook 'guard' threw: …). The single door still ships the wrapper — the mirror defect, filed as rest:classifyDataError's structured-code arms ship the QuickJS debug wrapper aserrorfor a sandboxed producer — the mirror of #11588, on the door #11588 did not touch #14704 and pinned in §4 as a named accepted divergence rather than left unasserted.OBJECT_NOT_FOUND— its 曝露 gate 对未知对象放行所依赖的「data path 会 404」并不成立 —— findData 无存在性校验(#3545 同类前提) #3770 exclusion already gave it door parity; its body is pinned byte-identical, and the exclusion clause survives for exactly one residue (a sandboxed body throwing that code, which the consult declines and which losesobjectwithout the clause).VALIDATION_FAILED,ERR_DATASOURCE_UNAVAILABLE,PERMISSION_DENIED— measured: their producers declare nostatus(PermissionDeniedErrordeclaresstatusCode), so the passthrough never fired for them and both doors already agreed.The measurement triage left open
Triage asked for
CONCURRENT_UPDATE's producer to be measured rather than assumed from the other two. Done, and posted on the issue:packages/metadata-protocol/src/protocol.ts—class ConcurrentUpdateErrordeclaresreadonly code = 'CONCURRENT_UPDATE'andreadonly status = 409, pluscurrentVersionandcurrentRecord. So the passthrough fires and all three of those, plusobject, were dropped on the bulk doors. It behaves exactly asDELETE_RESTRICTEDandDUPLICATE_RECORDdo.The pin triage asked for (guard 3)
packages/rest/src/error-response-structured-arm-door-parity.test.ts:handleRouteErroragainstsendThrownError.CONVERGED,ACCEPTED DIVERGENCE(with the card that owns it) orMOVED (stated). The two shapes the review found pinned on one side only — the declared-4xx-vs-5xx-arm case and the sandboxed producer — are now asserted on both doors, and the two status moves above have their own cases.classifyDataError, not just the lifted one. A new bespoke arm added below the consult — exactly whereOBJECT_NOT_FOUNDandINVALID_FIELDsat before this card — is the shape that reproduces the defect, and it is now caught. Everyerror?.code === '…'literal in the whole function must be either a §1 parity case or an entry in an explicit allowlist, and the allowlist distinguishes two kinds:by-design(today:PERMISSION_DENIED, whose third limb is message-text-gated) andknown-gap(an entry that MUST cite the card carrying it). A third test refuses stale cover: every entry must name a live arm, carry a reason, and — if a gap — cite a card.RECORD_NOT_FOUNDis a genuine remaining instance of this card's defect (recordNotFoundErrordeclaresstatus = 404andobject, so the bulk doors dropobject). It is left open on purpose — its second limb is a message-text gate, and an eleventh body change after the verdict would be an unreviewed delta — and recorded as aknown-gapciting rest: the GENERIC declared-status passthrough still disagrees onobjectbetween the two error doors — plus one bespoke arm (RECORD_NOT_FOUND) still reachable from one door only #14725 rather than quietly excused.classifiedRefusalAnswerfamilies (condition 6).Verification
9c87f281a5, by a third seat. The reviewer explicitly did not run the suite, typecheck, lint or the gates, and the previous seat's report was voided when its container was restarted mid-run. No reading here is inherited.Every exit captured before any pipe (
cmd > log 2>&1; EXIT=$?). All heavy runs went throughscripts/pm/os-verify-lock.sh, verdicts read from itsVERDICTline.Is the new test file actually type-checked? Measured with
--listFilesrather than asserted, and reported either way:tsconfig.jsonexcludes**/*.test.tsby design (ci.yml gates that no test file reaches the published artifact), so the build program legitimately does not see it.typecheckruns both programs, and the test-layer one does — so the green typecheck genuinely covers the new file.Gate union, re-derived on the final tree after
git merge origin/main:The two that did not measure — ⛔ neither a red, neither a pass, each quoted in its own words:
node scripts/check-test-completeness.mjs— exit 3: "That list names this script with NO argument, which is this branch. There is no local log to hand it, so the local reading for this gate is NOT MEASURED. ⛔ It is not a red, and there is nothing here to fix." CI tees theturbo run testlog and passes the path, so CI behaviour is unchanged.node scripts/pm/check-half-states.mjs— exit 3: "Nothing was swept: no issue was listed, no predicate (H1–H16) ran … It is not a clean board and it is not a dirty one — it is no reading at all." This seat's egress denies repo-scoped REST (probed directly:GET /repos/objectstack-ai/objectstack/issues/14541answers403), which is the prerequisite it is refusing on.pnpm check:type-check-debt(--self-test && --re-measure) is a repo-scale scan, and on its first attempt the container's 10-minute foreground ceiling killed the--re-measurephase with SIGTERM (exit 143) — a resource kill, ⛔ not a finding. Re-run detached from that ceiling it passed, exit 0 in 179s under the verify lock, and that is the reading counted above. Its siblingpnpm check:type-check-coverage— the same script without--re-measure— passed, exit 0, on the first attempt. Both readings are stated so neither is hidden. (The previous seat hit the same ceiling on this same one gate, with SIGKILL137.)Changeset
patchon@objectstack/rest, by the precedent in this repo for response-contract fixes on shipped public routes (analytics-dataset-query-declared-5xx-relay.md,analytics-dataset-query-user-message.md— bothpatch, both response-body changes on published doors; the latter's own framing is "exactly one optional key is ADDED"). Nothing here is removed, the onecodeVALUE change restores the spelling those same doors answered before #14095, and the two status moves are on synthesised shapes no producer in this repo throws.Notes
needs:contract-reviewstays hung — it is triage's label and this PR does not touch it, on either carrier. This PR is draft and is not to be flipped ready or auto-merged; the PM seat lands it after reading this patched head.packages/rest/src/rest-server.tswas read but not edited (fenced: held byclaude/issue-13753-meta-read-doors-org-forwarding).packages/objectql/src/engine.ts,package-routes.tsandexecctx-consumer-census.test.tswere read to measure, never edited. Nothing undercontent/docs/releases/**,docs/adr/**,.claude/**,skills/**,AGENTS.mdorCLAUDE.md.classifyDataError's structured-code arms ship the QuickJS debug wrapper aserrorfor a sandboxed producer — the mirror of #11588, on the door #11588 did not touch #14704 (the mirror defect on the single door — a sandboxed producer's QuickJS wrapper), rest/metadata-protocol: one unique conflict, two wire spellings on ONE route — a batch/import ROW reportsDUPLICATE_RECORDwhile the whole-request failure on the same route reportsUNIQUE_VIOLATION#14723 (which spelling wins theUNIQUE_VIOLATION/DUPLICATE_RECORDfork), rest: the GENERIC declared-status passthrough still disagrees onobjectbetween the two error doors — plus one bespoke arm (RECORD_NOT_FOUND) still reachable from one door only #14725 (the generic passthrough'sobjectdisagreement, plus theRECORD_NOT_FOUNDarm). All three exist and are cited from the changeset, the docblock and the test allowlist.Co-Authored-Bynaming the model, the repo rule forbids model identifiers in pushed artifacts #14663.🤖 Generated with Claude Code
https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza