Skip to content

fix(types): withhold a demoted declaredCode on an undeclared 5xx — one rule, every door inherits (#12509) - #12946

Open
os-litant wants to merge 5 commits into
mainfrom
claude/issue-12509-demoted-code-withhold
Open

fix(types): withhold a demoted declaredCode on an undeclared 5xx — one rule, every door inherits (#12509)#12946
os-litant wants to merge 5 commits into
mainfrom
claude/issue-12509-demoted-code-withhold

Conversation

@os-litant

@os-litant os-litant commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12509

Maintainer ruling 2026-08-27 (adjudication session_01DKWDdUJ2XNRESVVWUvcpnh, verbatim: 「同意」), option D: in 5xx sanitisation a demoted code — one the fallback-to-500 picked up from an undeclared producer — is withheld along with the prose; an author-declared code survives. Implemented once at the shared resolver so every door inherits one rule. Options A (record the leak as a decision), B (withhold the author channel too) and C (a per-door rule) were declined by name and are not attempted here.

The three readings the ruling named as the first step

(a) Driver-errno reachability at the other two doors' seams — the premise does NOT hold at the dispatcher door.

The card's premise ("a driver errno cannot reach a producing seam") had been measured at PackageService's four seams only. Re-measured on origin/main @ aef1b7e64 with a real ObjectQL and a driver that fails every access with a coded fault:

seam route driven answer
packages door DELETE /api/v1/packages/:id via real ObjectStackProtocolImplementation 503 SERVICE_UNAVAILABLE, no declaredCode — the protocol converts the fault to a declared, registered refusal, so the premise holds even with code: 'SQLITE_ERROR' present
flat /data door GET/POST /api/v1/data/:object via real CRUD routes 500 DATABASE_ERROR / 500 INTERNAL_ERROR, fixed sanitised bodies, no declaredCode — the errno never reaches the wire
dispatcher door POST /api/v1/analytics/query 500 INTERNAL_ERROR with declaredCode: "SQLITE_ERROR" — no producer sits between the service and errorResponseBase, so the throw reaches the resolver verbatim

The premise does not hold at the dispatcher door. That is what makes this a repair rather than a tidy-up.

(b) Consumers of the demoted strings on the wire — none found in this repo. Outside the emitting doors, their tests, packages/spec's schema declaration and the generated API reference, nothing reads declaredCode, and nothing anywhere branches on a driver-errno literal (SQLITE_ERROR, 42P01). packages/client has no read of the field at all. ⚠️ Boundary stated rather than hidden: sibling repos (objectui, hotcrm, cloud) are not in this checkout and were not measured here; ADR-0112's Consequences records the two earlier consumer sweeps for #9106 / #9232, but those were about the demote generally, not about 5xx demoted strings. No consumer dependency was found, so the ruling's scope stands and nothing was escalated.

(c) #12281's population (declared-5xx producers carrying NO code) — non-empty. Seven sites, all in packages/runtime/src/action-execution.ts: six × { statusCode: 503, message: 'Data service not available' } and one { statusCode: 501, message: 'Data query fallback cannot serve …' }. declaresServerFault needs a string code, so their prose travels today; #12281's rule would replace it with the generic sentence, including the 501's self-correcting operator guidance. Every other 5xx-declaring producer found (mcp, plugin-auth, objectql/action-activation.ts) carries a code. This card changes nothing for that population — they declare no code, so they never had a declaredCode to withhold.

Reproduced first, on the real routes

Before (origin/main @ aef1b7e64), driving real routes rather than reading source:

PKG-DOOR   sqlite => 500 {"error":{"code":"INTERNAL_ERROR","message":"Internal server error","declaredCode":"SQLITE_ERROR"}}
PKG-DOOR   pg     => 500 {"error":{"code":"INTERNAL_ERROR","message":"Internal server error","declaredCode":"42P01"}}
PKG-DOOR   author => 503 {"error":{"code":"SERVICE_UNAVAILABLE","message":"…","declaredCode":"ACME_LEDGER_OFFLINE"}}
DISPATCHER sqlite => 500 {"error":{…,"declaredCode":"SQLITE_ERROR"}}
DISPATCHER-PLUGIN sqlite => 500 {"error":{…,"declaredCode":"SQLITE_ERROR"}}   (real POST /api/v1/analytics/query)
ENDPOINT-EXEC     sqlite => 500 {"error":{…,"declaredCode":"SQLITE_ERROR"}}

After, same harness, one edit:

PKG-DOOR   sqlite => 500 {"error":{"code":"INTERNAL_ERROR","message":"Internal server error"}}
PKG-DOOR   pg     => 500 {"error":{"code":"INTERNAL_ERROR","message":"Internal server error"}}
PKG-DOOR   author => 503 {"error":{"code":"SERVICE_UNAVAILABLE","message":"…","declaredCode":"ACME_LEDGER_OFFLINE"}}   ← unchanged
DISPATCHER / DISPATCHER-PLUGIN / ENDPOINT-EXEC sqlite => 500, no declaredCode

One place, and it is not the doors

serverFaultProvenance (packages/types/src/thrown-http-error.ts) is the whole judgement, applied inside demotedDeclaredCode — the read every door already makes. No door file changes in this PR. Five emitting exits inherit it: sendThrownError (packages/rest/src/package-routes.ts), thrownCodeFields (packages/rest/src/error-response.ts), HttpDispatcher.errorFromThrown, endpoint-executor.endpointErrorAnswer and dispatcher-plugin.errorResponseBase — ⭐ five, not the three the card names; the two extra are the endpoint executor and the dispatcher plugin, both measured above.

Scan of the working tree including untracked files (grep -r, never git grep, which reads tracked files only):

  • term under test serverFaultProvenanceone executable definition (packages/types/src/thrown-http-error.ts:324) and one non-test call site (:353, in demotedDeclaredCode, same file). Its other appearances are the test that imports it, the ADR anchor, the ADR and the changeset — prose, no second implementation.
  • positive control demotedDeclaredCode (not a substring of the term under test, in either direction) — 17 .ts files. A zero there would have meant a dead scan; the scan also listed the then-untracked new test files, so the tracked-only trap is falsified in both directions.

⚠️ Measured near-neighbour, reported rather than folded in. declaredServerFaultAnswer (packages/rest/src/error-response.ts:471) open-codes "is this a declared 5xx" for a different question — whether to relay the producer's declared status and code — and reads declaredHttpStatus (which also imposes a 400–599 band) rather than the resolver's declaredStatus. It is not a second implementation of demoted-vs-author-declared, and the two cannot contradict each other on the wire. Whether it folds into serverFaultProvenance is a question for #12281, which touches that limb.

⚠️ Second measured exception: packages/cloud-connection/src/cloud-connection-plugin.ts:382 emits declaredCode directly, not through the shared rule — a verbatim relay of an upstream RFC 8628 spelling on a hard-coded 400. It never emits the channel at a 5xx, so the withhold's scope is not evaded; recorded because "every emission goes through one function" would otherwise be false.

The judgement the ruling's text leaves to the implementation

The discriminator is the status channel. A driver errno and an app's own spelling both arrive on .code as a plain string, so telling them apart by inspecting the string would be a heuristic over an open channel — the consumer-side tolerance ADR-0112 exists to forbid — and unfalsifiable, since nothing stops an app from spelling SQLITE_ERROR. packages/types/src/thrown-http-error-5xx-code-withhold.test.ts §3 pins that: the same spelling survives when declared and is withheld when not.

⚠️ The cost, stated rather than discovered later: a producer that spells a code but declares no status loses that code on a 5xx. That is what "the fallback-to-500 picked it up from an undeclared producer" means once it is spelled as code, and it is pinned as a named row rather than left implicit. It is kept by declaring the status the refusal means. It is not option B: the tenant-authored limb ADR-0112's amendment protects rides a 400 (SandboxErrorVALIDATION_ERROR, the DUPLICATE witness), untouched here.

Not gated on whether looksLikeInternalErrorLeak fired. That predicate reads a different channel; gating on it would leak the errno for exactly the dialects whose prose the heuristic misses — the ceiling sendThrownError's own note records.

What #12281 still needs when it lands

The prose axis is deliberately not applied. What it needs, precisely:

  1. errorResponseBase (packages/runtime/src/dispatcher-plugin.ts:591) changes its message gate from declaresServerFault(err) || (httpStatus >= 500 && looksLikeInternalErrorLeak(raw)) to the 'declared' limb of serverFaultProvenance — the same function, other limb. No new predicate is needed; that is the shape this PR put there.
  2. The population in (c) — seven code-less declared-5xx throws in action-execution.ts — is what changes behaviour, and their messages are operator guidance, so runtime: a declared 5xx carrying NO code keeps its prose on /analytics/query where /data withholds it unconditionally #12281's measurement-first step should decide whether they move to userMessage (contract: a hook refusal has no way to mark its message user-facing — the console's 403 substitution (ruled in #3821) needs a producer-side opt-in channel #9934) rather than simply losing their text.
  3. packages/runtime/src/dispatcher-5xx-demoted-code-withhold.test.ts §4 pins today's message behaviour on both shapes, so runtime: a declared 5xx carrying NO code keeps its prose on /analytics/query where /data withholds it unconditionally #12281 lands as a visible change rather than as drift; its second expectation flips to the generic sentence.
  4. declaredServerFaultAnswer's open-coded "declared 5xx" (above) is the one place worth reconsidering in the same change.

Verification

Union re-run after the final commit, on 824007d8f:

  • pnpm --filter @objectstack/types test396 passed / 396
  • pnpm --filter @objectstack/rest test2569 passed / 2569 (155 files)
  • pnpm --filter @objectstack/runtime test2935 passed / 2935 (198 files)
  • pnpm --filter @objectstack/types --filter @objectstack/rest --filter @objectstack/runtime run typecheck — clean. ⚠️ Coverage measured, not assumed: --listFiles shows the types test is in its tsc program and the rest test is in packages/rest/tsconfig.test.json; @objectstack/runtime's tsconfig.json excludes **/*.test.ts and it has no sibling test program, so the runtime test is not compiled by that script — a standing gap for that package's whole test layer, already ledgered in TEST_DEBT.
  • pnpm lintwhole repo, exit 0, zero findings (eslint . --no-inline-config, 92s). Not a narrowed run.
  • pnpm check:type-check-debtOK, 31 ledger entries re-measured, none above its recorded number. It first went RED on this branch: the new runtime test added an unused vi import, taking @objectstack/runtime's TEST_DEBT from 217 to 218. Fixed by deleting the import (commit 2), ⛔ not by raising the entry.
  • Green: check:adr-anchors (see below), check:type-check-coverage, check:engine-double-contract, check:where-matcher, check:cross-package-test-inputs, check:test-source-alias, check:type-source-resolution, check:published-files, check:doc-authoring, check:empty-changeset, check:adr-0087-registration, check:page-declaration-shape, check:slot-lookup, check:query-options-erasure, check:nul-bytes, check-changeset-no-major, check-comment-mask-adoption, check-adr-links, check-plugin-teardown-shape.
  • ⚠️ pnpm check:adr-anchors reports 1 problem that is not from this branch: packages/spec/src/data/object.zod.ts cites ADR-0006 D4, which ADR-0006 does not declare. That file and ADR-0006 are untouched here; the failure reproduces on origin/main. Reported to the PM rather than filed — the dedup read (GET /repos/…/issues) answers 403 from this seat.

Reverse verification

Predicted before running — deleting the guard line from demotedDeclaredCode: types 7 red, rest 4 red, runtime 10 red; §1's provenance table stays green (the ablation removes the application, not the judgement), and both door files' "wire == shared rule" comparisons stay green because both sides move together, which is precisely why the literal assertions in §1 exist.

Measured: 7 / 4 / 10, exactly. Failure list matched row for row.

The ablation proved both legs on disk, not by exit code: anchor count 1 → 0 and the injected marker 1, worktree blob ded6de18ee521eb7; @objectstack/types rebuilt and ablation-dist-preflight … --absent confirmed the compiled guard gone from dist/ — load-bearing, because @objectstack/rest does not alias @objectstack/types to source and consumes that artifact (@objectstack/runtime does alias). Restore leg: git checkout HEAD -- with the ABSOLUTE file path, quoted, from an EXIT INT TERM trap, then proven by git diff HEAD empty and the worktree blob back to ded6de18, with the preflight showing the guard present in dist/index.js and dist/index.mjs again.

⚠️ Correction, recorded rather than silently repaired: the first version of this body wrote that restore command with the path as a placeholder in angle brackets, and GitHub's body sanitizer ate the placeholder — leaving a command that read as git checkout HEAD -- "$REPO_ROOT/", i.e. restoring the whole repo root. The sentence above is the corrected spelling; the command actually run named one absolute file path.

Generated by Claude Code

claude added 2 commits August 28, 2026 09:05
…DR-0112 scope, #12509)

In 5xx sanitisation a DEMOTED code — one the fallback-to-500 picked up from a
producer that declared no HTTP answer, e.g. a driver errno — is withheld along
with the prose; an AUTHOR-DECLARED code survives at every status. Maintainer
ruling 2026-08-27, option D.

The judgement is `serverFaultProvenance`, one exported function in
`packages/types/src/thrown-http-error.ts`, applied inside `demotedDeclaredCode`
— the read every door already makes — so all five emitting exits inherit it and
no registrar carries a variant. No door file changes.

The discriminator is the STATUS channel because it is the only structural one:
a driver errno and an app's own spelling both arrive on `.code` as a plain
string, so telling them apart by looking at the string would be a heuristic
over an open channel, and unfalsifiable besides.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
`@objectstack/runtime`'s tsconfig excludes `**/*.test.ts`, so no tsc program
its `typecheck` script runs reads this file — but `check:type-check-debt`
re-measures the test layer against a frozen, shrink-only ledger, and the unused
import took TEST_DEBT from 217 to 218. Fixed rather than ledgered: raising the
entry is maintainer-only and hands back what an earlier PR paid to press down.

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

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 1 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 8cb96ec41b34d987ca713cd94174af2fa45c955bpackageMentionDocs.

Which tree this was computed on

This run read content/docs from aa8d99e17f0d411087ee98a20c518ac2b8cd4a8c — the merge of head 75faa7fccfa874edfbb920684938b5fd0be76382 into base 8cb96ec41b34d987ca713cd94174af2fa45c955b, 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 aa8d99e17f0d411087ee98a20c518ac2b8cd4a8c && git checkout aa8d99e17f0d411087ee98a20c518ac2b8cd4a8c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8cb96ec41b34d987ca713cd94174af2fa45c955b 75faa7fccfa874edfbb920684938b5fd0be76382 && git checkout -B drift-repro 8cb96ec41b34d987ca713cd94174af2fa45c955b && git merge --no-ff 75faa7fccfa874edfbb920684938b5fd0be76382

node scripts/docs-audit/affected-docs.mjs --json 8cb96ec41b34d987ca713cd94174af2fa45c955b

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 28, 2026
@os-litant
os-litant marked this pull request as ready for review August 28, 2026 10:06
@os-litant
os-litant requested a review from hotlong as a code owner August 28, 2026 10:06
@os-litant
os-litant added this pull request to the merge queue Aug 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 28, 2026
@os-litant
os-litant added this pull request to the merge queue Aug 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 28, 2026
@os-litant
os-litant enabled auto-merge August 28, 2026 15:35
@os-litant
os-litant added this pull request to the merge queue Aug 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 28, 2026
@os-litant
os-litant added this pull request to the merge queue Aug 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Decision] Is ADR-0112's declaredCode channel in scope for 5xx sanitisation at all — and the answer must be applied to all three doors at once

2 participants