Skip to content

fix(service-analytics): a row-scope refusal carries a declared envelope, so queryDataset stops classifying refusals by their wording - #17336

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-17130-declared-envelope-on-analytics-refusals-r2
Sep 10, 2026
Merged

fix(service-analytics): a row-scope refusal carries a declared envelope, so queryDataset stops classifying refusals by their wording#17336
os-zhuang merged 4 commits into
mainfrom
claude/issue-17130-declared-envelope-on-analytics-refusals-r2

Conversation

@os-tesla

Copy link
Copy Markdown
Collaborator

Fixes #17335

Clause-②: no — no error-code-ledger row is added. The existing READ_SCOPE_COMPILE_FAILED, already owned by @objectstack/service-analytics (packages/spec/src/api/error-code-ledger.zod.ts:783) and registered for exactly this condition, is reused. No packages/spec/** edit, no new exported symbol on any barrel (isMissingSourceError is exported from its module for the guard to ask, and stays absent from index.ts).

⚠️ This pull request is a re-creation, and its branch is a re-push. The original (#17316, branch claude/issue-17130-declared-envelope-on-analytics-refusals) and its card (#17130) were removed when the os-trump account was suspended. That PR is unreachable (404) yet still holds its branch, so GitHub refuses a second PR on it — hence the -r2 name. The card has been re-filed as #17335.

The commits are byte-identical: this branch points at d1cd397d4810f50e519fe351d616ad4be42a9aa8, the original head, pushed unchanged from the local object store. ⛔ No rebase, no amend, no squash.

The original body and the os-dev-report were lost with the account; what follows is reconstructed from the delivering seat's report. Every number below was measured at d1cd397d4 before the suspension and has NOT been re-run against today's main — CI on this PR is the current reading.

The defect

queryDataset decides between re-raising and degrading to an empty chart. A bare Error carries neither status nor code, so it misses hasDeclaredErrorEnvelope and falls through to isMissingSourceError, which judges by wording — and three of its six substrings (not registered, unknown object, is not a registered object) are exactly the phrasings a registry or security refusal reaches for.

PR #17125's fail-closed row-scope refusal propagates today only because its wording happens to match none of them. ⛔ A coincidence, not a construction: the next reword turns a fail-closed gate into a 200 with no rows.

Two legs, and the cheaper one is the one that keeps working

1 — the guard (landed first, deliberately). refusal-wording-collision.test.ts derives its population from source: every throw in every non-test .ts under the package's src/, read through the TypeScript AST, with wording resolved transitively through in-package message helpers, and the verdict asked of the real isMissingSourceError. It asserts no bare refusal this package raises can match it.

⇒ 22 files, 87 throws, 71 carrying wording, 0 collisions.

⭐ This is the half that protects the next reword rather than this one message. ⛔ Its population is derived, never hand-listed — a hand-listed corpus rots the same way the thing it guards did.

2 — the fix. readScopeUnresolvedError (src/read-scope-refusal.ts) stamps READ_SCOPE_COMPILE_FAILED / 500 on both row-scope resolution refusals — plugin.ts's security bridge and AnalyticsService.resolveReadScopes — so queryDataset re-throws them at hasDeclaredErrorEnvelope and never reaches the heuristic.

isMissingSourceError's subtraction list is untouched; PR #17125's refusal is not reworded; every message is byte-unchanged; no refusal is made likelier to degrade.

Why an existing code rather than a new one

The ledger was enumerated with two independent legs that reconcile: a real JS parse of the ERROR_CODE_LEDGER object literal (lines 190..1266) and a line-anchored regex over the same byte range — both answering owners=29 rows=324 uniqueCodes=282, identical sets. READ_SCOPE_COMPILE_FAILED is registered for "RLS read-scope lowering failed fail-closed — a SERVER fault (500), never the caller's", which is this condition. ⇒ no ledger row, Clause-② stays no, and the PR is landable without a CONTRACT_REVIEW_TIER verdict.

⚠️ One thing a reviewer should weigh: that ledger comment says "lowering", and after this change a second constructor stamps the same code from the resolution stage. Resolution and lowering are two stages of one pipeline with one outcome, and two spellings for one condition class is what ADR-0112 exists to remove — but the comment is now narrower than the code's coverage. Registering a separate READ_SCOPE_UNRESOLVED would be purely additive over what lands here (swap the constant, keep the constructor, keep every message), so choosing reuse now forecloses nothing.

PERMISSION_DENIED / 403 was rejected on a measured ground, not taste: at 4xx the boundary echoes the message, so "read-scope resolution failed for X" would tell a tenant that this deployment's security service is broken. The declared 500 withholds it — the disclosure posture #5367's maintainer ruling settled for this family.

Evidence (measured at d1cd397d4, pre-suspension)

Own package — 104 files / 2243 tests green; typecheck clean, and proven to cover the new files rather than exclude them (tsc --noEmit --listFiles places both new test files in the program).

Acceptance 1 (positive) — three colliding wordings (not registered, unknown object, is not a registered object), raised through the package's own constructor, all propagate out of the real queryDataset. ⛔ Never a 200 with an empty chart.

Acceptance 2 (negative control)no such table, Postgres's real relation "x" does not exist and MySQL's doesn't exist still degrade to {rows:[],fields:[],totals:[]} with the "backing object … is unavailable" warn; an absent security service still runs unscoped; the pre-existing dataset-degradation-envelope.test.ts (11 cases) green unchanged.

Acceptance 3 (guard controls) — an in-suite synthetic control (one bare colliding throw + one enveloped one; exactly the bare one reported) and a live control on the real tree: BEFORE=0 / AFTER=1 marker counts proving the mutation hit disk, plugin.ts:1142 named in the failure, Tests 1 failed | 3 passed. Restored and proved by state (blob == HEAD blob, empty git diff HEAD, empty git status --porcelain), then Tests 4 passed.

Acceptance 4 (ablation of the fix) — prediction written first; mutate+measure in ONE shell under trap … EXIT INT TERM; on-disk proof BEFORE_CODE=1/BEFORE_STATUS=1 → AFTER_CODE=0/AFTER_STATUS=0. Result Tests 6 failed | 13 passedexactly the predicted 6, in two distinct shapes: three rows fail on code/status undefined, and three fail with "a fail-closed refusal was served as an empty chart" — i.e. queryDataset resolved, the defect reproduced. ⭐ Predicted green and green: all four wording-guard cases and all three #5033 negative controls — stripping the envelope is invisible to a wording check, which is why the envelope needed pins of its own.

Acceptance 5 (cross-package sweep) — population derived two ways and reconciled: leg A = every workspace manifest depending on @objectstack/service-analytics (6 packages); leg B = every file outside the package naming it (81 files / 18 packages), narrowed to real imports (5 packages, all inside leg A). Positive control: leg A independently contains @objectstack/runtime (owner of analytics-query-read-scope-withhold.test.ts, which pins the wire answer for this very family). Results: service-analytics 104/2243 · rest 187/3118 · runtime 251/3531 · client 41/492 · verify 14/103 · cli 190/2644 · dogfood 135 files + 1064 tests.

Repo-wide lint, not narrowed — full eslint . --no-inline-config: 6477 files, 0 errors, 0 warnings, exit 0.

Gates — 61 families derived, 61 run. --ran printed 0 NOT-MEASURED; that zero is the runner's claim and is annotated by hand instead: check:dual-build-cjs-loads and check:type-check-debt each exited 3 (PREREQUISITE NOT MET) on their first run and were re-run to exit 0 once their prerequisites existed — ⛔ neither counted as a pass on the strength of the first run. Four artifact-roster families whose roster sits under a directory this diff touches were run explicitly (silence is evidence in neither direction), plus check:error-code-provenance because this diff stamps an error code: all PASS.

Known, recorded, not fixed here

missingSourceRelation has a second arm a bare colliding refusal can land in: "… unknown object in the resolved scope" makes the extractor read the relation name as in, so queryDataset re-reports the refusal as a cross-datasource topology error — loud, but describing a JOIN that does not exist. Both arms are wrong for a security refusal; this card is about the silent one, and the envelope closes both arms for the refusals this PR touches. ⛔ Not filed separately because it is the class this card fixes.


Generated by Claude Code

… source

`queryDataset` classifies a BARE error by its words, and three of the six
limbs (`not registered`, `unknown object`, `is not a registered object`) are
the phrasings a registry or security refusal reaches for. The guard walks
every `throw` in this package's non-test sources with the TypeScript AST,
resolves each one's wording through in-package message helpers, and asserts
none of the un-enveloped ones can be read as a driver reporting an absent
table. Population and verdict are both derived: the sites from the AST, the
verdict from `isMissingSourceError` itself.

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

`queryDataset` re-throws any error whose producer declared `code` + `status`
and classifies everything else by wording — six substrings, three of which
(`not registered`, `unknown object`, `is not a registered object`) are what a
registry or security refusal reaches for. A hit is not a wrong status code, it
is `{rows: [], fields: [], totals: []}`: a fail-closed gate served as a
confident empty chart.

The row-scope RESOLUTION stage refused bare on both of its sites, and
propagated only because its text happened to miss all six. Both now carry
`READ_SCOPE_COMPILE_FAILED` / 500 — the code the sibling LOWERING stage has
owned since #5367, so no ledger row is added — through one constructor. Every
message is byte-unchanged: the fix is the declaration, not a luckier string.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/plugins/packages.mdx (via AnalyticsServicePlugin (symbol, a top-level class))
What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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 — 9 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 cca1dc0bfb1e380442670843606d6a0c7762f6bapackageMentionDocs.

Which tree this was computed on

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

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

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

@os-zhuang
os-zhuang marked this pull request as ready for review September 10, 2026 10:36
@os-zhuang
os-zhuang added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 54b3d1d Sep 10, 2026
68 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-17130-declared-envelope-on-analytics-refusals-r2 branch September 10, 2026 11:02
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/l tests tooling

Projects

None yet

3 participants