Skip to content

fix(rest): relay a producer-declared 5xx on /analytics/dataset/query instead of collapsing it to 500 ANALYTICS_QUERY_FAILED - #12280

Merged
os-trump merged 1 commit into
mainfrom
claude/issue-11718-analytics-declared-5xx-relay
Aug 25, 2026
Merged

fix(rest): relay a producer-declared 5xx on /analytics/dataset/query instead of collapsing it to 500 ANALYTICS_QUERY_FAILED#12280
os-trump merged 1 commit into
mainfrom
claude/issue-11718-analytics-declared-5xx-relay

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes#11718

POST /api/v1/analytics/dataset/query collapsed every producer-declared 5xx onto a hand-built 500 ANALYTICS_QUERY_FAILED. POST /api/v1/data/:object relays the declared status and ADR-0112 code and withholds only the prose (#5582). Measured door-to-door, one error object, both handlers driven in process — this is §8f's own failure text from the ablation below:

registered code: analytics 500 {"code":"ANALYTICS_QUERY_FAILED","error":"Internal server error"}
vs /data 503 {"error":"Internal server error","code":"SERVICE_UNAVAILABLE"}

The reference held — and this door was the only one of three

Zone 2 asked me to verify rather than assume that /data is the correct reference. It is, and the check turned up a third door that settles it:

doordeclared 5xxrelays statusrelays code
POST /data/:object#5582 passthroughyesyes
POST /analytics/query (sibling)dispatcher-plugin.errorResponseBaseyesyes
POST /analytics/dataset/queryhand-built arm ③nono

The sibling analytics face already relays both halves — pinned end to end against a real AnalyticsService in packages/runtime/src/analytics-query-read-scope-withhold.test.ts, which asserts a read-scope refusal arrives as 500 + READ_SCOPE_COMPILE_FAILED. So /analytics/dataset/query was the sole deviant, and it deviated only because it built its 5xx body by hand. No fork to report.

The change

/data's 5xx arm is lifted into declaredServerFaultAnswer (error-response.ts) and read by both doors — the way arm ①b already imports classifiedRefusalAnswer. A third local opinion at this boundary is exactly how the two faces came to disagree, so the arm is imported, not restated. /data's behaviour does not move; its own pins prove it (2357 tests, below).

Arm ③ splits: ③a a declared 5xx → relayed; ③b everything else → unchanged 500 ANALYTICS_QUERY_FAILED with #5667's tiering.

Not a re-opening of #5352/#5367/#5811. Those rule the prose, and the prose is still withheld — byte-identically, from the same shared arm. The old pin's stated reason for the collapse was that a producer must not "slip past the logError line that makes it visible to operators"; that concern is answered, not overruledlogError runs before the relay branch and is unconditional, and the inverted pin now asserts the operator still gets the full original text.

Contract change (⚠️needs:contract-review)

  • declared { status: 503, code: 'SERVICE_UNAVAILABLE' }503 SERVICE_UNAVAILABLE (was 500 ANALYTICS_QUERY_FAILED)
  • an unregistered declared code demotes exactly as /data demotes it: 503 {"code":"SERVICE_UNAVAILABLE","declaredCode":"WAREHOUSE_UNAVAILABLE"} (finding: packages/rest's flat sendThrownError still puts a thrown error's code on the wire un-narrowed — ADR-0112's closure does not reach that door #9232) — measured, not assumed
  • ⚠️read-scope-sql's ten RLS refusals now answer 500 READ_SCOPE_COMPILE_FAILED, not 500 ANALYTICS_QUERY_FAILED. Called out because the card flagged it as load-bearing. The 2026-08-06 ruling is untouched in substance — still a SERVER fault, still 500, still with the policy content withheld from the body and intact in the log, still not the 400 DATASET_INVALID it was rescued from (all four still asserted). What changed is that this route stopped overwriting the producer's declaration with a code of its own. That overwrite was never ruled; it is what the hand-built envelope happened to emit, and it made one fault answer to two different names depending on which analytics door the caller used. This converges on the sibling face's shipped answer.
  • unchanged: an undeclared fault (no status to relay) and the whole declared-4xx band, including the half-envelope rule.

Verification

Local, at ce146ab2:

  • pnpm --filter @objectstack/rest test146 files / 2357 tests passed, os-verify-lock: VERDICT command-exit 0
  • pnpm --filter @objectstack/rest typecheckVERDICT command-exit 0
  • gate family re-derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack against the actual change set (7 paths), not the dispatch list

Ablation — the pins fail without the fix

Predicted before running: 10 RED. Measured: 10 RED, exactly that set.

Mutation was the relay branch only (source change reverted, pins kept), confirmed on disk with anchored greps in both directions before any result was read — injected text 1 → 0, ablation marker 0 → 1, git diff --stat non-empty — and restored under trap … EXIT INT TERM, with git status --porcelain empty afterwards proving byte-identical restore. No rebuild leg applies: the pins import ./rest-server.js / ./error-response.js relatively, so vitest resolves package source, never dist/.

RED without the fix: §8c, §8f, 5× read-scope envelope, 2× filter envelope, 1× dimension-gate.

Named as regression guards, not red-before evidence — green in both states: §8e (the client-band parity loop, already green before this change) and §8g (new positive control over unchanged ③b behaviour).

§8f is flipped, not deleted — it recorded a measured defect and that record is the only evidence the defect existed, so the assertion is turned around and the reasoning kept, the way §8b was handled for #11684. It now drives both doors with one refusal and asserts they agree on the values the producer declared (status and code and the declaredCode demote), plus explicitly not.toBe(500) / not.toBe('ANALYTICS_QUERY_FAILED') — because "both are 5xx with a code" was already true of the defect.

Notes

  • ⛔ Draft, and not to be marked ready or auto-merged from this seat: needs:contract-review is cleared by the review chain. Fuse measured — last_served_modelclaude-opus-5CONTRACT_REVIEW_TIERclaude-fable-5.
  • Footprint kept to the analytics arm — rest-server.ts is +52 lines, all inside the one catch, for the four cards queued behind this file.

Generated by Claude Code

`POST /api/v1/analytics/dataset/query` collapsed every producer-declared 5xx
onto a hand-built `500 ANALYTICS_QUERY_FAILED`, where `POST /data/:object`
relays the declared status and ADR-0112 code and withholds only the prose
(#5582). Measured door-to-door on one error object, a declared
`{ status: 503, code: 'SERVICE_UNAVAILABLE' }` answered `503
SERVICE_UNAVAILABLE` on `/data` and `500 ANALYTICS_QUERY_FAILED` here.
`/data` is the reference and does not move. Its 5xx arm is lifted into
`declaredServerFaultAnswer` and read by both doors, the way the 4xx arm
already imports `classifiedRefusalAnswer` — a third local opinion at this
boundary is how the two faces came to disagree. The sibling analytics face
`/analytics/query` already relayed both halves, so this door was the only
one of three overwriting a producer's declaration.
The prose withhold is untouched (#5352/#5367/#5811) and `logError` still runs
before the relay branch, so a declared status cannot buy a producer past the
operator's log. An undeclared fault keeps `500 ANALYTICS_QUERY_FAILED` and
#5667's tiering.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HbG3rGVLjZStHQxHDtzJdJ
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/client-sdk.mdx(via data.create (sdk), data.find (sdk))
  • content/docs/api/data-flow.mdx(via data.create (sdk))
  • content/docs/api/environment-routing.mdx(via data.find (sdk))
  • content/docs/api/error-catalog.mdx(via data.create (sdk))
  • content/docs/deployment/troubleshooting.mdx(via data.find (sdk))
  • content/docs/kernel/runtime-services/data-service.mdx(via data.create (sdk), data.find (sdk))
  • content/docs/permissions/authentication.mdx(via data.find (sdk))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx(via data.create (sdk), queryDataset (sdk))
  • content/docs/releases/v9.mdx(via queryDataset (sdk))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 anchor(s) matched too much of the corpus to be a work list: /api/v1/data/:object (route, 29 pages), /data/:object (route, 64 pages)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 177: 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; 107 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 33e81a5fe0edc80ea75dceb685965a193a834720packageMentionDocs.

Which tree this was computed on

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

⚠️ 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 33e81a5fe0edc80ea75dceb685965a193a834720 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

Contract-review verdict: PASS (#11718) — option A endorsed

Reviewed by the contract-review chain at CONTRACT_REVIEW_TIER. Tier fuse, machine reading this sub-round: get_session.external_metadata.last_served_model = claude-fable-5.

Reviewed: the source patches (error-response.ts, the §8 pin file) in full, the triage ruling and seat ACCEPT on #11718 — and the one question the seat correctly narrowed for this tier was measured, not argued:

"Is any consumer keyed on the literal ANALYTICS_QUERY_FAILED for a read-scope refusal?" — No, measured in both repos. objectstack's hits are the change itself, its pins, and the spec ledger row (which stays live — arm ③b still emits the code for the undeclared band). objectui has exactly one production hit, packages/data-objectstack/src/index.ts, and it is prose: the extraction reads source.code generically and its own comment states "nothing downstream classifies on it". No consumer branches on the literal; the docblock's example stays true.

On the A/B fork: A (relay status AND code, byte-identical to /data) is correct, for the seat's three reasons plus the measurement above — triage's ruling names /data's behaviour, which relays both halves; two of the three doors already shipped it, so A converges rather than invents; and B would keep the route overwriting the producer's code, leaving §8f asserting a value no producer declared. The read-scope-sql consequence (ten refusals now READ_SCOPE_COMPILE_FAILED) is the sibling face's long-shipped answer, and the 2026-08-06 ruling's actual subject — server fault, 500, policy withheld, not DATASET_INVALID — is asserted unchanged in the updated pin.

Verified in the diff: declaredServerFaultAnswer gates on declaredHttpStatus >= 500, not declaresServerFault alone — the no-code 5xx keeps its status with nothing invented, which is the load-bearing difference; /data's arm becomes the one reader of the lifted body (unchanged bytes, no third opinion); logError stays above and unconditional, answering #5352's stated reason rather than overruling it; §8f is flipped with the original assertion quoted, asserts the producer's declared values plus explicitly not-the-old-answer, and §8g pins ③b so the route provably still classifies. Prose withheld byte-identically on both faces.

Landing completed by the chain in the same stroke per the maintainer's standing instruction (2026-08-25, verbatim: 「审核通过你应该直接负责合并吧,还要等原始的项目经理吗」): clearing needs:contract-review on this PR and #11718, flipping ready, arming the queue.


Generated by Claude Code

@os-trump
os-trump marked this pull request as ready for review August 25, 2026 17:37
@os-trump
os-trump added this pull request to the merge queueAug 25, 2026
Merged via the queue into main with commit 6747718Aug 25, 2026
40 checks passed
@os-trump
os-trump deleted the claude/issue-11718-analytics-declared-5xx-relay branch August 25, 2026 18:08
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rest: a declared 5xx answers 500 ANALYTICS_QUERY_FAILED on /analytics/dataset/query where /data answers the declared status and code

2 participants

@os-trump@claude