Skip to content

fix(rest): the generic declared-status passthrough names its object on both error doors - #15452

Draft
os-litant wants to merge 5 commits into
mainfrom
claude/issue-14725-declared-status-passthrough-object-key
Draft

fix(rest): the generic declared-status passthrough names its object on both error doors#15452
os-litant wants to merge 5 commits into
mainfrom
claude/issue-14725-declared-status-passthrough-object-key

Conversation

@os-litant

@os-litant os-litant commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Fixes #14725

resolveErrorResponse's 4xx arm now ends on the same ...(object ? { object } : {})
limb classifyDataError's generic declared-status passthrough has always carried, so
the two copies of one passthrough no longer differ by a key. Direction ruled by triage
(comment 5518039240): option yes, following #14541's precedent.

1. Re-measured on today's main before writing anything

The card's disagreement table was measured on the #14541 branch, so the first
question was whether there is still a card. There is. Both doors, one error object,
origin/main @ a12b15e394 (mapDataError = the single-record /data door,
sendThrownError = the bulk / metadata / UI door):

probe single door bulk door agree?
{ code: 'DUPLICATE_RECORD', status: 409 } (no name, so no bespoke arm) 409 {"error":"…","code":"DUPLICATE_RECORD","object":"duly_note"} 409 {"error":"…","code":"DUPLICATE_RECORD"} no
{ code: 'RECORD_LOCKED', status: 409 } …,"object":"duly_note"} …} no
residue 2 — recordNotFoundError('duly_note','abc') (code + status:404 + object) 404 {…,"code":"RECORD_NOT_FOUND","object":"duly_note"} 404 {…,"code":"RECORD_NOT_FOUND"} no
boundary — sniff-matching message, no declared code/status 404 {…,"object":"duly_note"} same yes
control — OBJECT_NOT_FOUND (bespoke arm, #14541) 404 {…,"object":"duly_note"} same yes

After the change all five agree, including the two controls, which stay byte-identical.

Residue 2's producer chain verified at the throw site: recordNotFoundError
(packages/core/src/utils/record-not-found.ts) sets code = 'RECORD_NOT_FOUND',
status = 404 and object — so its declared status carries it past the
RECORD_NOT_FOUND arm into this same generic passthrough. That is why one limb closes
both residues, and why no arm needed lifting.

2. The census the contract review needs to price this

Method. The affected door is resolveErrorResponse. Its only callers are the two
exported senders in error-response.tssendThrownError and handleRouteError
plus classifiedRefusalAnswer, which calls it with no object argument at all.
Neither sender is re-exported from packages/rest/src/index.ts, so the call-site set is
closed inside this package. Enumerated over every tracked non-test .ts file
(git ls-files), excluding comment lines, error-response.ts's own internal
delegations, and package-routes.ts's local sendThrownError (a different function
of the same name, declared at package-routes.ts:273, which answers the nested envelope
through resolveThrownHttpError and never reaches this door). Each site's route was
resolved by walking back to its nearest path: / method: declaration.

Result — 35 route call sites; 9 of them can widen a body:

sites argument body
9 req.params?.object (6) or String(req.params?.object || '') (3) gains object
5 the literal '' (falsy) unchanged
21 none passed unchanged

The 9 widened routes: POST /data/:object/batch, /createMany, /updateMany,
/deleteMany, POST /data/:object/:id/clone, POST /data/:object/import,
POST /data/:object/import/jobs, GET /data/:object/export,
GET /ui/view/:object/:type. The 5 literal-'' sites are the
/data/import/jobs/* family; the 21 bare sites are the metadata / UI / discovery /
docs / cross-object-batch families. So the card's "bounded above by the data routes" is
now exact: 9 of 35, and the name is read from the door's argument, never from
error.object — pinned, so a future edit cannot quietly promote the producer's field.

3. Census of the passthrough itself — is there a third copy?

There is not. declaredHttpStatus has four call sites in error-response.ts
(declaredServerFaultAnswer, classifyDataError's sandbox-unwrap door,
classifyDataError's generic passthrough, classifiedRefusalAnswer) plus one inline
400-599 band read in resolveErrorResponse. Of those, exactly two are the generic
declared-status passthrough
— the two the card names. Repo-wide, the only other
...(object ? { object } : {}) sites outside error-response.ts are
rest-server.ts's hand-built BATCH_TOO_LARGE 400 and
runtime's permission-denied-envelope.ts; neither is a status passthrough.

4. Boundaries kept — and pinned, not described

5. Fixture triage — two existing pins had to be re-judged, and why

Both are in error-response-structured-arm-door-parity.test.ts, #14541's own file.

  1. §4's ERR_DATASOURCE_UNAVAILABLE case went red, exactly as it should: it
    asserted expect(bulk.body).not.toHaveProperty('object') under a comment naming
    this card as the owner of that residue and pinning it "so the residue is visible
    rather than implied". The case is labelled CONVERGED, and §4's own contract says
    CONVERGED means "both doors answer the same status AND body". With the limb added
    that label now holds for the body too, so the assertion became
    expect(bulk.body).toEqual(single.body).
  2. §5's drift-guard entry for RECORD_NOT_FOUND moved from known-gap to
    by-design.
    It is the guard Routes exiting through handleRouteError answer the engine's DUPLICATE_RECORD envelope from resolveErrorResponse's .status passthrough — field and object dropped, mapDataError's structured 409 arms never consulted #14541's contract review condition 3 required, and it
    was green by disclosure of a gap that this PR closes. Leaving it would leave the
    guard green by disclosing a gap that no longer exists, citing a card that is closed —
    the exact "stale cover" the guard's own third test exists to prevent. The arm still
    stays out of the shared classification (its second limb is a message-TEXT gate,
    outside the declared-code boundary — unchanged), but it no longer needs lifting:
    both limbs now converge measurably, and the new entry says so and names the file that
    pins it. No entry was removed and the allowlist did not grow.

Recorded and not repaired here: with no known-gap entries left, that guard's
if (entry.kind === 'known-gap') expect(typeof entry.card).toBe('number') limb no
longer evaluates. It is a validation rule for a list expected to gain entries, not a
check that lost its subject — flagged rather than silently worked around.

6. Tests

New: packages/rest/src/error-response-generic-passthrough-object-parity.test.ts — 17
cases (Tests 17 passed (17) on its own). The pins are the two doors agreeing (same
status, same body), never the limb existing, so a refactor that keeps the limb and moves
the agreement still goes red. Every refusal assertion states code and status
(ADR-0112); no bare toThrow(). §5 asserts key absence ('object' in body === false)
rather than relying on toEqual, which cannot tell {} from { object: undefined }
and emitting "object": undefined is a different published body from omitting the key.

Ablation, both legs proved on disk before the run (see the Verification section for
the exact confirmations). The test imports ./error-response.js relatively, inside its
own package, so vitest resolves it to the source — no dist is involved and the red
proves the run read the mutated bytes.

7. Changeset — minor, by a rule I first failed to find

minor on @objectstack/rest. I originally shipped patch and flagged the question,
because I could not find the "additive widening is at least minor" rule in AGENTS.md
or .claude/skills/pm-dispatch/references. It is not in either — it lives in
.github/workflows/pr-automation.yml
(the Check Changeset step's "WHICH LEVEL" prose,
i.e. the text an author who is told a changeset is missing actually reads), and is cited
from scripts/check-changeset-no-major.mjs so that declared and enforced stay in one
place. Landed 2026-09-04, maintainer decision batch #35 on #15294. Read at origin/main
and quoted:

A purely additive widening of a published package's public surface (a new exported
symbol on an index, a new accepted key or value) takes at least minor. The commit
type may raise a bump but never lower it below what the act requires; a fix( that
widens an index is therefore minor, and a fix( that changes no public surface
stays patch. … The commit type is not the discriminator it looked like: it
correlates with the act, and when they disagree the act wins. The 64 historical
patch precedents are pre-rule and nothing is retro-fixed.

That last sentence settles the argument this section used to make: the precedent I cited
#14541's own patch for a strictly larger instance of this delta — is named by the
ruling as pre-rule, and the ruling exists precisely because two contract reviews read
that history to opposite bumps for the same additive act. Citing the history cites the
ambiguity the ruling ended.

This PR adds a new accepted key to a published 4xx body on 9 routes, so the act is that
widening whatever the fix( type says — and it would be incoherent to call the same act
a contract widening for the reviewer (§8) and a no-public-surface-change for the
changeset. The bump is the only thing that moved: the diff is otherwise unchanged.

8. Clause-② — re-declared from the DELIVERED diff

Clause-②: yes, unchanged from the claim declaration and mechanical under #14696's
floor: a new key on a published payload. The delivered diff is narrower than a
reader of the card might expect — one limb, no arm lifted, no status moved, no code
value changed, nothing removed — but "narrower" does not lower the clause: the key is
still new on a published 4xx body for 9 routes. ⇒ This PR parks: draft, not flipped,
not armed, not enqueued, needs:contract-review applied to the card and this PR
together now that the reviewable increment exists (director ruling 5536344304).

9. Verification — final HEAD e141d14adb, on a clean tree

Heavy runs went through scripts/pm/os-verify-lock.sh; every verdict below is quoted
from that script's own VERDICT line or the gate's own output, never from a bare $?,
and every exit code was captured before any pipe.

⚠️ Two HEADs appear below, deliberately. Everything was first measured at 19b2470401.
The bump ruling then arrived, so the changeset's one line changed and origin/main was
merged again (11 commits) to keep the gate derivation off a stale tree — final HEAD
e141d14adb. The code diff is byte-identical between the two, so each row says
which HEAD produced it rather than implying one sweep.

Tests.

run result
pnpm --filter @objectstack/rest test VERDICT command-exit 0 · Test Files 179 passed (179) · Tests 3040 passed (3040) — run at both HEADs, same numbers
pnpm --filter @objectstack/rest typecheck (tsc --noEmit + check:test-typecheck) VERDICT command-exit 0 · 0 file(s) / 0 error(s) — at 19b2470401
downstream consumer sweep pnpm --filter '...@objectstack/rest' test (prefix = dependents), at 19b2470401 VERDICT command-exit 0 — 12 packages: rest 3040, runtime 3208, cli 2876 (+6 expected fail), plugin-auth 1986, client 431, cloud-connection 362, http-conformance 86, hono 74, service-sms 74, plugin-dev 72, verify 50, client-react 34

The typecheck green is measured, not assumed: tsc -p tsconfig.test.json --listFiles
names both edited test files (1 hit each), so the check compiled them rather than
excluding them.

Ablation — two legs, direction predicted first, mutation proved on disk before each
run, restore proved after.
Measured at 19b2470401; the mutated file is
byte-identical at e141d14adb, so it was not re-run. The implementation was committed
first, so both restore legs point at HEAD; the script carries a trap on
EXIT/INT/TERM with an absolute path, and each restore is proved by blob-hash equality
against the HEAD blob plus an empty git diff HEAD — never by an exit code.

leg mutation, confirmed on disk predicted measured
baseline blob e9824e26 == HEAD blob, git diff HEAD empty green 63 passed (63)
1 — delete the limb guarded-limb occurrences 13 to 12; occurrences inside the 4xx arm = 0 the door-agreement pins go red, code/status assertions stay green 7 failed | 56 passed. Every failure is a body-agreement assertion (toEqual / toHaveProperty('object', …)); the status and .code assertions on the very same cases all passed, and the boundary case went red only on its body-equality line
2 — make the limb unconditional (...{ object }) injected form present exactly once; guarded form 13 to 12 the key-absence pins go red, the agreement pins stay green (toEqual cannot tell {} from { object: undefined }) 4 failed | 59 passed — exactly the four key-set pins: §2's "door supplies NO object", §4's classifiedRefusalAnswer case, and both §5 cases
restore blob e9824e26 == HEAD blob, git diff HEAD empty green 63 passed (63)

Leg 2 is the one worth keeping: it is what makes "object": undefined a different
published body from an omitted key, and nothing in the file caught it before.

The ablation ran against source — the test imports ./error-response.js relatively,
inside its own package, so vitest resolves it to error-response.ts; that both legs
went red with no rebuild is itself the proof the run read the mutated bytes.

Gates. Derived, not reused: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, on the clean tree at final HEAD, no stale-tree
warning: 4 paths, 46 commands — all 46 run, all exit 0.

The family grew from 44 to 46 across the two HEADs, and the growth is not the
changeset edit: derived on my own tree before merging, the list was byte-identical to
the 44. The two new members arrived with origin/main
packages/lint/scripts/check-reference-carrier-shape.mjs and its --self-test — and
both are green.

Two members return exit 3 (PREREQUISITE NOT MET — NOT MEASURED, neither pass nor red)
on a tree with no dist: check:dual-build-cjs-loads and check:type-check-debt. The
prerequisite was satisfied at each HEAD with a full
turbo run build --filter='./packages/*' --filter='./packages/*/*' (71/71 successful
both times) and both re-run: exit 0 (103/66/619/1 against floors 90/58/520/1;
14 ledger entries re-measured, none above its recorded number).

The five artifact-roster families whose roster sits under a directory these paths are in
— silence there is evidence in neither direction — were run at both HEADs:
check-changeset-fixed, check:authz-resolver, check:error-code-casing,
check:filter-alias-parity, check:swallow-census-controls, all exit 0.

The Check Changeset job's own three base-scoped checks were run directly against the
real merge base (6ed4b811af), since the derived family only carries their
--self-test halves: check-empty-changeset --base (1 declaring changeset(s) added),
check-adr-0087-registration --base (1 non-breaking changeset(s) seen),
check-changeset-no-major --base (introduces no major bump) — all exit 0, self-tests
included. Control-character scan run beyond check:nul-bytes over every edited file and
this body: no hits.

⚠️ CI is still the authority on the gate farm; this is the cheap half, deliberately run
before reporting rather than after waiting for it.

…n both doors (#14725)

`resolveErrorResponse`'s 4xx arm now ends on the same
`...(object ? { object } : {})` limb `classifyDataError`'s generic
declared-status passthrough has always carried, so the two copies of one
passthrough no longer differ by a key.

Measured on `main` @ `a12b15e394`, one error object, both doors:

    { code: 'DUPLICATE_RECORD', status: 409 }   // no `name`, so no bespoke arm
      mapDataError(err, 'duly_note')
        409 {"error":"…","code":"DUPLICATE_RECORD","object":"duly_note"}
      sendThrownError(res, err, 'duly_note')
        409 {"error":"…","code":"DUPLICATE_RECORD"}

The 5xx arm deliberately gains nothing: its sibling
`declaredServerFaultAnswer` names no object either, so that band already
agreed and adding the limb there would create the divergence this one
removes. The message-text sniff is not lifted above the passthrough.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
`error-response-structured-arm-door-parity.test.ts` §4's
`ERR_DATASOURCE_UNAVAILABLE` case pinned the bodies differing by one key
and named this card as the owner of that residue. With the limb added the
case's own CONVERGED label holds for the body too, so the pin asserts
door-to-door body equality instead of describing a gap that is closed.

The §5 drift guard's `RECORD_NOT_FOUND` entry moves from `known-gap`
(citing this card) to `by-design` for the same reason: the arm still stays
out of the shared classification — its second limb is a message-TEXT gate,
outside the declared-code boundary — but it no longer needs lifting,
because both limbs now converge measurably. Leaving a `known-gap` citing a
closed card would leave the guard green by disclosing a gap that no longer
exists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
@github-actions github-actions Bot added the size/m label Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest, touching 3 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx (via DUPLICATE_RECORD (literal, a string literal in resolveErrorResponse))
  • content/docs/automation/flows.mdx (via DUPLICATE_RECORD (literal, a string literal in resolveErrorResponse))
  • content/docs/protocol/kernel/error-handling.mdx (via DUPLICATE_RECORD (literal, a string literal in resolveErrorResponse))
What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 6ed4b811af30080854eb94c6bb292e1bd137d356packageMentionDocs.

Which tree this was computed on

This run read content/docs from e02c2a8e63dc7184d2d422457fe064ef9b9a2351 — the merge of head e141d14adbfcf8659c17c97f113018e44ca981a8 into base 6ed4b811af30080854eb94c6bb292e1bd137d356, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e02c2a8e63dc7184d2d422457fe064ef9b9a2351 && git checkout e02c2a8e63dc7184d2d422457fe064ef9b9a2351
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6ed4b811af30080854eb94c6bb292e1bd137d356 e141d14adbfcf8659c17c97f113018e44ca981a8 && git checkout -B drift-repro 6ed4b811af30080854eb94c6bb292e1bd137d356 && git merge --no-ff e141d14adbfcf8659c17c97f113018e44ca981a8

node scripts/docs-audit/affected-docs.mjs --json 6ed4b811af30080854eb94c6bb292e1bd137d356

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 6ed4b811af30080854eb94c6bb292e1bd137d356 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… of a published surface

The bump rule lives in `.github/workflows/pr-automation.yml`'s Check Changeset
prose (and is cited from `scripts/check-changeset-no-major.mjs`), not in
AGENTS.md: "A purely additive widening of a published package's public surface
(a new exported symbol on an `index`, a new accepted key or value) takes at
least `minor`. The commit type may raise a bump but never lower it below what
the act requires." Maintainer ruling 2026-09-04, decision batch #35.

This PR adds a new accepted key to a published 4xx body, so the act is that
widening whatever the `fix(` type says. The historical `patch` precedents this
changeset previously cited are named in the same ruling as pre-rule and are
deliberately not retro-fixed, so they cannot settle it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N

Copy link
Copy Markdown
Collaborator Author

Bump raised to minor per the PM ruling on #14725, and one platform reading worth recording.

Head is now e141d14adb. The only diff change is one line in
.changeset/rest-generic-passthrough-object-key.md (patch to minor); the code diff
is byte-identical. origin/main was merged again (11 commits) so the gate derivation
would not run on a stale tree. Section 7 of the description is rewritten to record the
rule and where it actually lives — .github/workflows/pr-automation.yml's Check
Changeset "WHICH LEVEL" prose, cited from scripts/check-changeset-no-major.mjs — and
why the historical patch precedents cannot settle it: the same ruling names them
pre-rule and retro-fixes nothing.

Verification at the new head: the derived gate family (46 commands, all green — it grew
from 44 because origin/main added check-reference-carrier-shape.mjs and its
self-test, not because of this edit), the five flagged artifact-roster families, and the
Check Changeset job's own three base-scoped checks run directly against the real merge
base 6ed4b811afcheck-empty-changeset --base (1 declaring changeset added),
check-adr-0087-registration --base (1 non-breaking changeset seen),
check-changeset-no-major --base (no major introduced) — all exit 0, self-tests
included. pnpm --filter @objectstack/rest test re-run on the merged tree:
Tests 3040 passed (3040).

⚠️ Platform reading, for whoever edits a PR description next. Editing this body
through update_pull_request removed the attribution footer block entirely — not
the documented downgrade from the session-URL form to the bare one, but the whole block
(rule line included) gone. Measured on two independent page fetches after the edit
landed, with the new section 7 confirmed present in the same fetch, so it is not a stale
render. Per the standing rule not to loop re-posting a footer, the attribution lives
here instead of being re-appended to the body.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants