Skip to content

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

Merged
os-zhuang merged 6 commits into
mainfrom
claude/issue-12509-demoted-code-withhold
Aug 29, 2026
Merged

fix(types): withhold a demoted declaredCode on an undeclared 5xx — one rule, every door inherits (#12509)#12946
os-zhuang merged 6 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 fa9018a29bcee1481d98461a89719e14665ca41cpackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json fa9018a29bcee1481d98461a89719e14665ca41c

⚠️ 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
@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
@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
@os-litant
os-litant marked this pull request as draft August 29, 2026 11:49

Copy link
Copy Markdown
Collaborator Author

⛔ 诊断完成 —— 是 governed surface 门在 merge_group 里拒绝,不是 CI、不是 base、不是队列抖动。已按门规定的首选补救转回 DRAFT

domain:cli 执行席(#6024) · 会话 session_01UjujZN219uFzBhSYfMykCd

门自己的输出(Governed Surface Queue Guard,merge_group run 33249790797,11:18:03Z)

merge group on refs/heads/main — 1 commit(s) in range
Governed Surface Queue Guard — merge_group — 1 governed pull request(s), …

  #12946 — governed:
        docs/adr/** ×1 — architecture decision records
          - docs/adr/0112-error-code-vocabulary-and-ledger.md
        ⛔ NO authorized APPROVED review pinned to head b04362571c99
           (0 review(s) read; authorized: os-zhuang, hotlong)

  ⛔  REFUSED — this merge group must not land.
      … the merge queue would have been the entire review — the shape of #9550, #10580 and #9319.

⇒ 本 PR 动了 docs/adr/0112-error-code-vocabulary-and-ledger.md ⇒ 它是一张 governed surface PR。每次入队,这道门都会拒绝,队列随即把它丢出去。五次尝试、五次同一原因。


⛔ 我此前两次诊断都错了,逐条纠正

错误一(00:2xZ):我读作「merge_group CI 成功过两次」。 那是假读数 —— merge_group 有四个 workflow(CI / Lint & Type Check / Spec Liveness Check / Governed Surface Guard),我只读了 CI 一个就下了「merge_group 通过」的结论。实测本次:前三个 success,第四个 failure

⭐ 这正是我今天一整天在对别人执行的那条规则 —— 拉完整列表、读分片不读汇总 —— 而我自己在 merge_group 那一侧违反了它。

错误二(今天上午):我用 mergeable_state: "clean" 去削弱「卡在人工 review」的判断。 也错了 —— mergeable_state 看不到只在 merge_group 上下文里跑的门。PR 页面 32/32 全绿,因为 PR 侧跑的是另一个 workflow;拒绝它的那道门根本不在 PR 的 check 列表里。

⇒ 我最初那条被我自己推翻的判断(「需要人工 review」)是对的,只是我当时给的机制是错的(不是 GitHub 的 required review,是仓库自己的 governed-surface 门),而我随后用一条看不见该门的字段把正确结论推翻了。


⚠️ 更严重的一条:我做的动作本身就是这道门存在的原因

门的 institutional memory 里逐字记着:

2026-08-12, #8012: an AI seat ENABLED AUTO-MERGE on a live docs/adr/** PR at ~11:15Z. Arming is not merging — it is a standing instruction to merge later, and the next approving review would have merged the PR unattended with every check green. … Disarming alone does not dequeue: converting the PR back to draft is what removes it from the merge queue.

⇒ 我今天对这张 docs/adr/** PR 武装了五次。它没有落地,唯一的原因是这道门每次都拦住了 —— 不是因为我判断正确

已执行的补救(门列的第 1 条,PREFERRED)

⭐ PREFERRED — take the pull request out of the queue: convert it back to DRAFT (disarming auto-merge alone does NOT dequeue it), and leave the merge to the maintainer. A human merge IS the review record for a governed surface; that is the regime, not a workaround of it.

#12946 已转回 DRAFT。 ⛔ 我走第 2 条(取一个 authorized approver 的 APPROVED review)—— 门原文:「⛔ An agent seat never submits that approval, under any account」。⛔ 也不动这道门本身:「Neither of those is "edit this check"」。

波及面核查(已实测,不是推断)

范围 结果
本席今天 arm 过的 9 条 PR(#13113 #13120 #13123 #13124 #13125 #13148 #13149 #13153 #12946) 只有 #12946 是 governed,其余 8 条对 docs/adr/**.claude/**skills/**AGENTS.mdCLAUDE.md 命中数为 0
本席今天 已合并的 14 条 PR 全部 clean —— 逐条查合并提交的文件列表,无一触碰 governed surface

没有 governed surface 在无人评审的情况下落地。门起作用了。


这张卡现在的状态与需要什么

  • DRAFT,不在队列,不会再自行入队。
  • CI 在最新 base(head b04362571)上 32/32 全绿,代码侧没有任何待办。
  • 需要的是:os-zhuanghotlong 之一人工合并(人工合并本身就是 governed surface 的评审记录),或由其中一位提交一个钉在当前 head 的 APPROVED review 后由人重新入队。⚠️ 任何后续 push 都会让该 approval 失效并重新触发拒绝。

⚠️ 连带影响:#12281 的裁定要求它「继承 #12509 的裁决」,而那条规则只存在于本分支上 ⇒ 本 PR 不落地,#12281 一直不可派发。这条依赖现在明确挂在一次人工动作上。


Generated by Claude Code

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

3 participants