Skip to content

fix(runtime): withhold the message of every declared 5xx at the dispatcher exit (#12281) - #13240

Draft
os-trump wants to merge 1 commit into
mainfrom
claude/issue-12281-declared-5xx-prose-withhold-r62
Draft

fix(runtime): withhold the message of every declared 5xx at the dispatcher exit (#12281)#13240
os-trump wants to merge 1 commit into
mainfrom
claude/issue-12281-declared-5xx-prose-withhold-r62

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes#12281

Clause-②: yes — this changes an answer on a public REST door. The contract-review gate attaches to this PR.

Implements the maintainer's 2026-08-27 ruling on #12509 (option D), propagated verbatim to this card:

errorResponseBase adopts the structural withhold for every declared 5xx message, aligning to /data's rule; the author-facing text channel is userMessage (#9934), never the raw message. … the implementation inherits the #12509 ruling and its dispatch constraints rather than re-opening the question.

What was wrong

errorResponseBase (packages/runtime/src/dispatcher-plugin.ts) gated its 5xx message withhold on declaresServerFaultstatus >= 500and a non-empty string code. /data gates on declaredHttpStatus, which reads status ?? statusCode and never consults code. Two bands of declared 5xx were therefore withheld at /data and legible here:

  1. no code — the card's title case. The structural half of /analytics/query 仍把 RLS 策略字段名回显给调用方 —— read-scope 拒收的泄漏在姐妹面上没堵,#5367 只堵了 dataset 路由 #5811's withhold required a code, so the no-code half of the declared band fell back to looksLikeInternalErrorLeak alone: the phrasing heuristic /analytics/query 仍把 RLS 策略字段名回显给调用方 —— read-scope 拒收的泄漏在姐妹面上没堵,#5367 只堵了 dataset 路由 #5811's own argument found insufficient, which is why the withhold was made structural in the first place.
  2. the statusCode spelling — wider, and not named in the card body. A producer declaring { statusCode: 503, code: 'SERVICE_UNAVAILABLE' } is fully ADR-0112-compliant and was still withheld at /data and legible here, purely because declaresServerFault read the status key only.

Option D closes both. They are the same file, the same ternary and the same ruling, so they are not split across two cards.

The change

One condition, inherited rather than re-derived:

- declaresServerFault(err) || (httpStatus >= 500 && looksLikeInternalErrorLeak(raw))
+ serverFaultProvenance(thrown) === 'declared' || (httpStatus >= 500 && looksLikeInternalErrorLeak(raw))

serverFaultProvenance is the function #12946 landed in @objectstack/types for exactly this — its own docblock names this card as its 'declared' limb and says "this function is the shape it will read rather than a second copy it would have to grow". demotedDeclaredCode already reads the same function for the code channel, so both channels at this door now read one judgement. ⛔ No per-door variant was written: that is the divergence this family has been repaired for twice.

resolveThrownHttpError(err, 500) was already called in this function for the code channel; it is now resolved once, above the ternary, so the prose rule and the code rule cannot look at different errors.

FROM → TO on the wire

For any route served by the dispatcher plugin (/analytics, /auth, /i18n, /automation, /notifications, /mcp, /packages):

thrown by the producerbeforeafter
{ status: 503 }, no code"message":"PRODUCER PROSE""message":"Internal server error"
{ statusCode: 503, code: 'SERVICE_UNAVAILABLE' }"message":"PRODUCER PROSE""message":"Internal server error"
{ statusCode: 503 }, no code"message":"PRODUCER PROSE""message":"Internal server error"
{ status: 503, code: 'SERVICE_UNAVAILABLE' }"message":"Internal server error"unchanged
a bare Error (declares nothing)"message":"PRODUCER PROSE"unchanged
any declared 4xx"message":"PRODUCER PROSE"unchanged

Only message moves. code, httpStatus, declaredCode and details are untouched, so nothing a machine branches on changes, and the untouched error still reaches the operator via __obsRecordedError and the log.

⛔ The gate is the declared status, never the resolved httpStatus (which falls back to 500 for a throw that declared nothing). #5667's undeclared-5xx tiering is preserved exactly — that was the one way this change could have done real harm, and it is pinned in both directions.

This is a no-op on today's tree

The R39 measurement on this card, accepted by the PM seat, established the population reaching this door is empty: metadata-protocol's deleteMetaItem reaches only the REST /meta door (the dispatcher plugin mounts neither /meta nor /data), and action-execution.ts's seven statusCode throws are all caught before this exit. That is precisely why now was the cheapest moment: the alignment costs no legibility that exists and buys the invariant forward.

⚠️ It also means a green suite proves nothing by itself, so every new pin drives its shape through the real mounted POST /api/v1/analytics/query route rather than asserting the predicate.

Tests

New: packages/runtime/src/dispatcher-plugin.declared-5xx-prose-withhold.test.ts — 7 declared shapes (both axes, both axes together, the no-regression case, both live producer sentences, a 504) plus 4 controls.

Two sibling pins were deliberately reversed, each of which asserted the pre-ruling behaviour:

Full @objectstack/runtime suite on 32470116: 201 files / 2968 tests passed, VERDICT command-exit 0.

Reverse verification

Direction predicted before running, then observed. Restoring the old predicate turned exactly 8 tests red: the 6 declared cases whose band widened, plus the 2 reversed sibling pins. The { status: 503, code } case and all 4 controls stayed green — so the pins discriminate rather than always-fail.

Mutation proven on disk by blob hash (cce89f6166afa5ba) and by grep counts on the exact injected and removed text, never by an exit code; restore proven by blob hash returning to cce89f61 with git diff HEAD empty. The mutation script carried a trap … EXIT INT TERM with absolute paths.

Gates

Re-derived for the actual change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (it read the change set from git itself: 5 paths vs merge base a81aa9dd5). The derivation added families the dispatch list did not name — check:objectql-double-limit, release-rehearsal-clone --self-test, and six convention-triggered ones including check:engine-double-contract, check:where-matcher and check:type-check-debt. All run on 32470116, exit codes captured before any pipe:

Green (31):check:nul-bytes · check:route-envelope · check:test-source-alias · check:type-source-resolution · check:published-files · check:slot-lookup · check:page-declaration-shape · check:logger-receiver-detach · check:objectql-double-limit · check:cross-package-test-inputs · check:engine-double-contract · check:where-matcher · check:query-options-erasure · check:keyed-text-bounds · check:undeclared-dep-imports · check:changeset-gate-self-tests · check:objectui-changeset · check:pm-half-states · check:type-check-coverage · check:type-check-debt · check:dual-build-cjs-loads · check-comment-mask-adoption · check-plugin-teardown-shape · check-ci-filter-parity · check-shard-attestation · plus check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, release-rehearsal-clone --self-test, docs-audit/check-affected-docs, docs-audit/check-drift-comment.

check:type-check-debt reported "surplus: none — every entry sits exactly at its measurement, so any new error is red", i.e. the new test file added zero to the ratchet. check:dual-build-cjs-loads needed a full pnpm build (71/71 tasks green) and then passed: 102 require entry points across 66 packages load.

NOT MEASURED (2) — each refused to run and says so in its own verdict text; neither is a pass and neither is a red:

  • check-test-completeness — "PREREQUISITE NOT MET … grades a saved turbo run test log, and no log was named", exit 3.
  • scripts/pm/check-half-states.mjs — needs a real GITHUB_TOKEN; this container has the proxy placeholder, exit 3.

Lint: a declared narrowing, with its three pieces of evidence

Repo-wide pnpm lint is CI's run. Narrowed to the 4 lintable changed files: 0 errors, 0 warnings, and the narrowing is a measurement rather than a skip because —

  1. population — all 4 lintable files in the diff were actually linted (none silently ignored), read from eslint's own resolution, not from an assumption about which files count;
  2. count — 4, read from --format json, not from console prose;
  3. invarianceeslint.config.mjs declares at its line 328, with a recorded positive control, that this repo "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file, test or not" ⇒ this diff cannot move the verdict on any file it did not touch.

packages/runtimetypecheck is green, and its tsconfig excludes **/*.test.ts — verified with --listFiles (dispatcher-plugin.ts 1 hit, the new test file 0). So that green is a true statement about the product change and says nothing about the test files; those were typechecked separately under a widened config, where all four touched files contributed 0 errors.

Fences honoured

Two findings, filed not fixed

  1. errorResponseBase never populates userMessage, though ApiErrorSchema.userMessage is declared in spec and buildApiError already supports it. The ruling names userMessage as the author-facing channel that compensates for the withheld prose — at this exit that channel is absent, while its caught-path sibling (http-dispatcher.tserrorFromThrown) and /data both carry it. Not fixed here: adding a wire field is a separate additive public-door increment deserving its own review, and the empty population means nothing loses text today.
  2. This PR removed declaresServerFault's last production consumer; it now survives as a definition plus its own tests in the fenced packages/types/src/error-leak.ts.

Generated by Claude Code

…tcher exit (#12281)
`errorResponseBase` gated its 5xx message withhold on `declaresServerFault`
(`status >= 500` AND a non-empty string `code`), while `/data` gates on
`declaredHttpStatus` (`status ?? statusCode`, `code` not consulted). Two bands
of declared 5xx were withheld one door over and shipped their prose here: one
carrying no `code` at all, and one merely spelling `statusCode` despite being
fully ADR-0112-compliant.
Maintainer ruling 2026-08-27 on #12509 (option D): adopt the structural
withhold for every declared 5xx message, aligning to `/data`. The judgement is
inherited rather than re-derived -- the door now reads `serverFaultProvenance`
from `@objectstack/types`, the same function `demotedDeclaredCode` already
reads for the code channel.
The gate is the DECLARED status, never the resolved `httpStatus`, so #5667's
undeclared-5xx tiering is preserved: a bare `Error` stays legible and still
goes through the `looksLikeInternalErrorLeak` heuristic alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 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 — 23 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 9e0ba21a1bf863c44e341fdad1bbecb755d450c2packageMentionDocs.

Which tree this was computed on

This run read content/docs from 7126b8045104d680c37ced0a1595abfab7ffcbbc — the merge of head 3247011623d45f18efcd5bd2ed14139502c38217 into base 9e0ba21a1bf863c44e341fdad1bbecb755d450c2, 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 7126b8045104d680c37ced0a1595abfab7ffcbbc && git checkout 7126b8045104d680c37ced0a1595abfab7ffcbbc
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e0ba21a1bf863c44e341fdad1bbecb755d450c2 3247011623d45f18efcd5bd2ed14139502c38217 && git checkout -B drift-repro 9e0ba21a1bf863c44e341fdad1bbecb755d450c2 && git merge --no-ff 3247011623d45f18efcd5bd2ed14139502c38217
node scripts/docs-audit/affected-docs.mjs --json 9e0ba21a1bf863c44e341fdad1bbecb755d450c2

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

@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

⛔ PM — barred from the merge queue by the clause-② gate (not a defect, no rework owed)

This PR is not being armed, and the reason is procedural rather than anything wrong with the change. Recording it here so the bar is visible on the card and nobody re-derives it.

The trigger, read from this PR rather than recalled. The body's second line is:

**Clause-②: yes** — this changes an answer on a public REST door. The contract-review gate attaches to this PR.

That is the declaration limb, which is path-independent — it fires on the author's own declaration regardless of whether the diff touches packages/spec/src/**. It does not, here (change set is 5 paths, packages/runtime + tests), so the path limb is silent. One limb is enough.

The fuse, measured against observed state.

valuesource
CONTRACT_REVIEW_TIERclaude-fable-5scripts/pm/dispatch-gates.mjs:5507
this seat's serving modelclaude-opus-5external_metadata.last_served_model

The comparison is against last_served_model, never the configured/session model — a session can be configured one way and served another, and the fuse cares what actually ran the turn. This seat is below tier, so with the clause-② limb lit, enqueue is forbidden and this seat may not self-clear its own gate. Both halves bind; clearing it myself is exactly the move the fuse exists to prevent.

Action taken: hung needs:contract-review. Nothing else changed.

What this is NOT

  • Not a quota or rate-limit problem. Unrelated mechanism.
  • Not a finding against the work. I am not asserting this PR is correct either — a below-tier seat declining to rule is the whole point; treat the contract question as unexamined, not as passed.
  • No rework is owed by the dev. Do not push changes in response to this comment.

What releases it

Either of, and only:

  1. a contract review from a seat served at claude-fable-5 or above, or
  2. a maintainer ruling on the clause-② question directly.

Two housekeeping notes for whoever picks this up

  • The PR is still draft: true, which is an independent bar on enqueue. Even once the gate clears, it needs undrafting.
  • mergeable_state has moved cleanunstable since 14:22Z (it was clean when I first read it this pass, unstable on the re-read minutes later). Something in CI has gone pending-or-red on 3247011. I have not diagnosed it and am not recording it as either a pass or a failure — it is simply un-measured here, and it is worth a look before this is queued for real.

Generated by Claude Code

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

Labels

documentationImprovements or additions to documentationneeds:contract-reviewsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

runtime: a declared 5xx carrying NO code keeps its prose on /analytics/query where /data withholds it unconditionally

2 participants

@os-trump@claude