Uh oh!
There was an error while loading. Please reload this page.
fix(rest): narrow the flat door's thrown code to the declared ADR-0112 vocabulary (#9232) - #9459
Conversation
…ulary (#9232) `packages/rest`'s flat `sendThrownError` passed a caught error's `code` to the wire verbatim, so the ADR-0112 invariant that #9106 made absolute — `code` is a closed vocabulary at every door — was contradicted by an observable door. That is the reader ambiguity #9106 was filed to remove, one door over. Per the maintainer ruling of 2026-08-17: body POSITION is not a carve-out. An unregistered thrown spelling is demoted to a top-level `declaredCode` sibling in the flat body, and `code` carries the member the status derives — computed by the shared `resolveThrownHttpError` / `demotedDeclaredCode` pair rather than by a fourth open-coded restatement of the rule. All four flat passthrough arms now ask one question, which also closes the non-string limb that let a numeric driver errno land in `code`. Pinned in `rest-thrown-code-vocabulary.test.ts`, both halves: an unregistered code demotes, and a registered one still arrives verbatim in `code`. The second half is what stops an over-eager narrowing from swallowing valid codes. Fixture triage, recorded at each site: three suites drove invented spellings (`UPSTREAM_UNAVAILABLE`, `HOOK_REFUSED`, `X_FAULT`) that no producer emits and the ledger never carried; their subject is the status/message rule, so each moved to a registered vehicle that is still not the status-derived code. The OWD posture gate is a REAL producer of a lowercase, unregistered `owd_widening_forbidden`, so its suite asserts the demote instead. Also repoints stale `#7035` citations in this package's prose: that card closed 2026-08-10 having converged three `/meta` 501 handlers, and was being cited as an open finding owning the envelope-position line. The live owner is the `check:route-envelope` ratchet. Same for a stale `#8111` citation beside one of them. The parity sentence in `packages/runtime` is updated in the same change, as the ruling required — it claimed a symmetry #9106 had already removed. Co-Authored-By: Claude <noreply@anthropic.com>
…aredCode` (#9232) Found by the reverse verification, not by review. Restoring one of the four verbatim passthroughs reddened the sibling case but left this one GREEN: every assertion it made is vacuously true of `undefined` — an absent field is not a vocabulary member and is not equal to `code`. A pin that cannot fail for the defect it names is worse than no pin, because it reads as coverage. Co-Authored-By: Claude <noreply@anthropic.com>
⚠️ ADR-PATH COMMIT — this is the commit that makes the PR maintainer-merge-only. It is deliberately separate so it can be reviewed, or dropped, on its own. The #9106 amendment's Scope paragraph carved `packages/rest`'s flat dialect OUT of the closure and pointed the reader at #9232 for "the remaining question about the flat top-level `code`". Once #9232 lands there is no remaining question, and leaving the paragraph would move the declared-≠-actual contradiction rather than remove it: the ADR would tell a reader the flat door is exempt while the code narrows. That is the same defect class the ruling required fixing for the parity sentence. Note for the record: the ruling asked for "the stale #7035 citations in #9098's prose and the ADR-0112 amendment" to be fixed. ADR-0112 contains ZERO #7035 citations (control: 13 `ADR` hits in the same file), so the literal reading finds nothing to do here. What the ADR actually needed was this — the Scope carve-out the ruling itself overturned. Records the ruling, what moves on the wire, the measured consumer sweep, the one live producer the demote is observable on, and where it is pinned. Keeps the envelope-position line explicitly separate and still open. Co-Authored-By: Claude <noreply@anthropic.com>
`check:type-check-debt --re-measure` reported @objectstack/rest's TEST_DEBT at 156 against a frozen 155: TS2835, this package resolves `nodenext` and the new file's relative import had no extension. The neighbouring suites omit it and are part of that frozen pile; a new file must not add to a shrink-only ratchet, and raising the entry is maintainer-only and not a way to get CI green. Co-Authored-By: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
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):
|
Status for the maintainer: this PR is ACCEPTED by the PM
Full review is on the card: #9232, comment Generated by Claude Code |
…t-door-declared-code
os-zhuang
commented
Aug 18, 2026
|
| reading | value |
|---|---|
ADR maintainer approval | completed / failure (since 06:00:08Z) |
| reviews on this PR | 0 (get_reviews → empty) |
| enqueued | 11:56:28Z, actor os-steve |
This merges regardless, because ADR maintainer approval is not one of the six required contexts in ruleset 12119582. That asymmetry is recorded in this seat's post as: "arming auto-merge bypasses it; the rule stops it, not the mechanism." The mechanism is now doing what the mechanism does.
⛔ I have not dequeued it and will not. Two reasons, and I want both on the record rather than just the conclusion:
- It may be authorized.
os-steveis a collaborator seat that could be acting on a maintainer instruction I cannot see — the same way this seat acted on a one-time model-tier downgrade earlier today that would look unauthorized to anyone reading only the labels. ⛔ I do not reverse another seat's action on an inference about its authority. - The rule I have been enforcing is scoped to me."ADR/skills-path PRs are never merged or enqueued by this seat" — it governs my hands, not the repo's mechanism. I held this PR for ~9 hours on exactly that rule and declined to arm auto-merge on it three separate times; that stands as the right call for me, and it is not authority to override someone else.
What I am doing instead: stating it plainly, now, while it is still actionable — the queue takes minutes, and a merge is revertible but noisier than a dequeue. Escalated to the maintainer in this round's report in the same breath.
ADR maintainer approval being advisory means every ADR change in this repo is one enable_pr_auto_merge away from landing unreviewed, by any seat, at any time. That is a mechanism-level observation and it is the maintainer's to rule on — ⛔ I am not proposing a ruleset change, and repo settings are the manual floor. (It would also be the second Settings-vs-workflow ordering hazard this repo has hit today; the first was the Lint & Repo Gates rename that deadlocked every open PR for an hour.)
📌 For completeness, since it decides nothing but is worth having beside the above: the six required checks are all green on this head, and mergeable_state was unstable rather than dirty, so there is no conflict and no merge-main was needed. The only red thing on this PR is the approval gate.
Generated by Claude Code
os-zhuang
commented
Aug 18, 2026
✅ Withdrawn — the approval landed 28 seconds after my reading. My comment above was accurate when taken and wrong by the time it posted.
⭐ Worth keeping, because it is a method failure and not just bad luck: I took three readings ( ⇒ The concrete fix, which I will apply rather than merely note: when a reading is the basis for raising an alarm about someone else's in-progress action, re-take it immediately before posting, not just before deciding. The cost of the extra read is one API call; the cost of skipping it was a public accusation-shaped comment about a colleague's action that was already resolved. 📌 What stands from that comment, because it is independent of the false alarm: Also unchanged and still true: six required checks green, Landing this unblocks #9426, #9488 and #9487 — all three land in Generated by Claude Code |
Fixes#9232
docs/adr/**, so it is maintainer-merge-onlyThe dispatch asked me to establish, by reading, whether
docs/adr/0112-*.mdgenuinely needs an edit, because the answer decides the landing path. It does. The ADR edit is isolated in its own commit (ad0beb32) so it can be reviewed — or dropped — on its own.But not for the reason anyone expected, and the PM's measurement was right.
docs/adr/0112-error-code-vocabulary-and-ledger.mdcontains ZERO7035citations (control: 13ADRhits in the same file, so the search works and the zero is real). The ruling's third bullet asks for the stale#7035citations to be repointed — naming #9098's prose and the ADR-0112 amendment. Taken literally, that finds nothing to do in the ADR.What the ADR actually needed is a different sentence. The #9106 amendment's Scope paragraph carves this exact door OUT of the closure:
Once this lands there is no remaining question, and the flat door is no longer exempt. Leaving that paragraph would move the declared-≠-actual contradiction rather than remove it: the ADR would tell a reader the flat door is carved out while the code narrows. That is the same defect class the ruling required fixing for the parity sentence, pointing the other way.
What changed
packages/rest's flat responder passed a caught error'scodeto the wire verbatim, at four arms —resolveErrorResponse's 4xx and 5xx passthroughs andmapDataError's two declared-status arms. All four now resolve the code through the sharedresolveThrownHttpError/demotedDeclaredCodepair, which is the ONE definition the three dispatcher exits already read:code, nothing beside it.codecarries the member the status derives; the producer's spelling moves to a top-leveldeclaredCodesibling. Presence means demotion.Delegating rather than open-coding a fourth
ErrorCode.safeParseis deliberate: a second definition of one rule is the shape that let two doors answer differently in the first place.One bounded fix taken in place, named here because it was not in the dispatch
The two
resolveErrorResponsearms gated on bare truthiness, so a non-stringcode— a numeric driver errno — could reach the flat body ascode: 1062. The other two arms already asked "non-empty string". Unifying them on the shared resolver's question closes that limb. Evidence that this is the settled shape, not my invention:resolveThrownHttpErroralready classes a non-stringcodeas context, with the reasoning in its own source — "promoting it would put a number in the field callers branch on, which is the drift #3842 removed". Same defect class as the card, same file, same gate family, mechanical. Pinned in §4 of the new suite.The pin
packages/rest/src/rest-thrown-code-vocabulary.test.ts— 24 cases over all four arms, in both directions. §2 (a registered code still arrives verbatim) is the half a regression does not redden, which is exactly why it is written down: a door answeringINTERNAL_ERRORfor everything would satisfy §1 on every case and destroy the vocabulary silently.Reverse verification, direction predicted before running. Restoring the
resolveErrorResponse5xx passthrough: 3 RED, 21 green — §1's 5xx demote case, §4's numeric errno, §5's status-derivation — and every §2 registered case stayed green, the predicted asymmetry.That ablation also earned its keep by finding a weak case of my own: §1's "presence means demotion" stayed green under it, because every assertion it made is vacuously true of
undefined. Fixed in896cac00with atoBeTypeOf('string')guard. A pin that cannot fail for the defect it names is worse than no pin.Fixture triage — three dispositions, not one batch rename
Nine existing cases moved. Each was judged separately and the reason is recorded at the site:
UPSTREAM_UNAVAILABLE(3 files)CONNECTOR_UPSTREAM_UNAVAILABLEHOOK_REFUSEDERR_HOOK_TARGET_REBINDX_FAULTowd_widening_forbiddenEvery replacement is deliberately not the status-derived code for any status in its loop, so "the producer's own code survives" can still fail.
The observable case, and a real finding underneath it
plugin-security'sobjectPostureGatethrows a lowercaseowd_widening_forbidden. It violates ADR-0112 on two counts at once — D1 rules the value space^[A-Z][A-Z0-9_]*$, and the ledger carries it in no casing — so that body could never satisfy the schema it claimed to. It now answersPERMISSION_DENIEDwith its own spelling preserved indeclaredCode.It is invisible to
check:dispatcher-error-vocabulary, whose scan matches SCREAMING_SNAKE shapes only. Filed unassigned rather than fixed here: registering a code is thepackages/speclane's call, and that gate's own scope note says nothing there edits the ledger.Consumer sweep — re-run, not inherited
The card assumed zero measured pull. Corrected, in the direction that matters: there IS a live read of the flat top-level
code—packages/client'sasSemanticCode(errorBody?.code), which ADR-0112's D5 amendment already records as one of the two live envelopes' declared spots. It is a generic pass-through normalisation, not a branch.What matters is whether anything branches on an unregistered spelling. Nothing does. Every code any consumer compares against was classified against the live union:
DESTRUCTIVE_CHANGE,INVALID_METADATA,ITEM_LOCKED,CONCURRENT_UPDATE,BATCH_UNRESOLVED_REF,BATCH_NOT_ATOMIC,UPLOAD_SESSION_EXPIRED,FLOW_FAILED,VALIDATION_FAILED,PERMISSION_DENIED,RECORD_LOCKED,FORBIDDEN,NOT_IMPLEMENTED— all REGISTERED, so all unaffected.INVALID_PAYLOAD, has zero producers in this repo, and both of its call sites are||-guarded byerr?.status === 422anyway.The sweep covered
packages/client,client-react,examples,packages/qaand the objectui checkout, including the helper-mediatederrorCodeIs/errorCodeIsAnyOfshapes a comparison-shape scan cannot see.Stale citations repointed
#7035reached statecompletedon 2026-08-10 via PR #7293, having converged three/meta501 handlers and nothing else, and was still being cited as an open finding owning the envelope-position line. Repointed where it was stale, inerror-response.ts,rest-server.tsandrest-field-visibility-fault-envelope.test.ts. A stale#8111citation beside one of them got the same treatment — it reached statecompletedon 2026-08-12 via PR #8212, having converged the family it named.Citations that describe #7035 accurately in the past tense were left alone; they are history, not drift.
check:route-enveloperatchet, not a card — because there is no live card. #7035, #8111 and #3843 have all reached statecompleted, and no open issue owns the envelope-position line (verified against the 27 opendomain:cliissues). Rather than point live prose at a card that closes with this very PR, the prose now names the ratchet, which is a mechanism CI actually enforces. The gate script's ownratchet: '#7035 …'fields still name the closed card; that file is outside this PR's declared surface and is filed separately.The envelope position is untouched and still open. It was explicitly not a precondition.
Merged
origin/mainfor the renamed required checkThe repository's required-check set changed after the first push: ruleset
12119582now requiresLint & Repo Gatesinstead ofESLint, and the workflow PR that produces that context (#9421) merged afterwards. The old head could not emit it, soorigin/mainwas merged (never rebased) — head is now047457ca3.Re-verified rather than assumed, because
mainmoved 18 commits:FLOW_DISABLEDandFLOW_NO_START_NODE; theSTORED_TYPE_NOT_CANONICALchange is comment-only. Every vehicle this PR's suites depend on was re-checked against the rebuilt union:PACKAGE_IS_HAUNTED,owd_widening_forbidden,UPSTREAM_UNAVAILABLE,HOOK_REFUSEDandX_FAULTare still unregistered (so no §1 case went vacuous), andRECORD_LOCKED,CONNECTOR_UPSTREAM_UNAVAILABLE,ERR_HOOK_TARGET_REBINDandFIELD_VISIBILITY_UNRESOLVEDare still registered.check-route-envelope.mjs's 420-line rewrite left this PR green, and — worth noting for the finding filed alongside this PR — it left both#7035ratchet:fields (lines 245 and 309) untouched. The rewrite added a new plugin-route audit with#9364owners and did not repoint the two stale ones.Nothing was adjusted to accommodate main.
Verification
All at
047457ca3, the current head — a merge oforigin/main(e374b4d74). Every figure below was re-measured on that merged head, not carried over:pnpm install --frozen-lockfile, a full closure rebuild (70/70 tasks, 68 of them not cached), then the suites and the gate union.mainhad moved 18 commits / 64 files, including a 420-line rewrite ofcheck-route-envelope.mjsand two new ledger codes.@objectstack/restsuite@objectstack/runtimesuite@objectstack/http-conformance@objectstack/dogfood(on rebuiltdist/)@objectstack/resttsc --noEmittypecheck@objectstack/rest(--filter '...^@objectstack/rest', the direction a contract tightening lands in), 14 declare atypecheckscript and all 14 ran and passed. Counted rather than inferred from the exit code, because a filter matching no script exits 0eslint --no-inline-configon the changed files047457ca3check:type-check-debt --re-measure047457ca3— 33 ledger entries re-measured in 221.6s, none above its recorded numberThe workspace closure was built before the ratchet and dogfood runs (
turbo run build, 70 tasks) — a throw on an unbuilt tree is not measured.check:type-check-debtinitially went red (@objectstack/restTEST_DEBT 155 → 156, TS2835). Fixed at the source ineb70f7f4aby giving the new file's import an explicit.jsextension. The ratchet was not touched, and it is still green on the merged head.ℹ️ The only card this PR closes is #9232. Every other number above — #7035, #8111, #3843, #9098, #9106 — is cited as history or as a live neighbour and is not addressed here.
Required checks
TypeScript Type Check·Lint & Repo Gates·Test Core·Dogfood Regression Gate·Build Core·Temporal Conformance (live PG + MySQL)(
Lint & Repo GatesreplacedESLintin ruleset12119582; the merge above is what lets this head emit it.)⛔ Auto-merge is deliberately not armed.
ADR maintainer approvalis not one of the six required contexts, so arming it would carry this ADR change into the merge queue past the approval that check exists to demand.Generated by Claude Code