Skip to content

fix(service-analytics): refuse a cross-object dataset-level filter on both ObjectQL doors (#10861) - #11066

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-10861-dataset-scope-crossobject-refusal
Aug 22, 2026
Merged

os-warren merged 1 commit into
mainfrom
claude/issue-10861-dataset-scope-crossobject-refusal

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes #10861

Parent card: #10413 — its phase-2 per-measure filter work is untouched here and stays where it is.

What was wrong

PR #10758 (#10413 phase 1) gave a compiled dataset's definition-level filter a route onto the ObjectQL door for the first time. That route was outside the member view ObjectQLStrategy.planCrossObject judges — execute() lowers the scope onto conjuncts after the view is built, and generateSql() renders it from a separate getDatasetScope(...)?.filter read — so nothing ever saw it.

Reproduced on the merged tree (072d072d2, i.e. after PR #10862 landed), one fixture, both doors in one run. This is byte-for-byte the measurement the card recorded at 5f2e54cc6, so the defect survived every move of the tree in between:

dataset: object 'opportunity', include: ['account'],
         filter: { 'account.region': 'West' }
capabilities: { nativeSql: false, objectqlAggregate: true, inMemory: false }

BEFORE  execute()      ACCEPTED
        generateSql()  ACCEPTED
        engine.aggregate calls: 1  filter: {"$and":[{"account.region":"West"}]}

engine.aggregate cannot join. account.region is not a column of opportunity, so on any driver that evaluates the predicate honestly it matches nothing, and the widget answered a number that was neither the scoped number nor the unscoped one — with no error anywhere. That is the silent mis-bucket #3654's loud refusal exists to prevent, arriving through a producer #3654 predates.

#10759 was not this card: both doors agreed in accepting it, so there was no preview/execution divergence to restore.

What changed

Maintainer ruling, 2026-08-22 decision-inbox digest (46 cards, accepted verbatim 「接受所有」): Option A — refuse at query time.

filterMemberView now folds the dataset scope's leaves into the one member view both doors are judged on, so the refusal fires where driver capability is known. Compile-time rejection in dataset-compiler.ts (B) was not taken — the compiler cannot see which driver will serve the dataset, and the same definition is legal on a native-SQL deployment. Serving the shape (C) is deferred; no customer dataset depending on it was found in the course of this work.

AFTER   execute()      REFUSED  code=INVALID_FIELD status=400 member=account.region
        generateSql()  REFUSED  code=INVALID_FIELD status=400 member=account.region
        engine.aggregate calls: 0

The member view is re-derived on the merged tree rather than taken from the card's description of it (triage's note: whichever lands second re-checks the shape). As #10862 left it, it was collectFilterLeaves(normalizeAnalyticsFilterTree(query)) keyed by resolveFieldName(..., 'any') and valued true. It is now keyed the same way and valued by provenance ('where' | 'dataset-filter') — the value slot was already documented as a placeholder planCrossObject never read, and provenance is the only thing the refusal needs it for. Dataset leaves are inserted first so a member named by both producers keeps the caller's provenance, which is the actionable one.

One deliberate envelope choice, flagged for contract review

The new refusal carries code: INVALID_FIELD, status: 400, member, cube — and no param. That is a decision, not an omission, and it is pinned as such:

  • AnalyticsRequestKey is documented as "which request key named the member". No request key named this one.
  • AnalyticsQuerySchema is strictObject and has no filter key (its own guidance sends filters → where), so a widened param: 'filter' would name a key the schema rejects — the "never suggest a key the schema cannot accept" trap strict-object.ts documents.
  • param: 'where' would send the caller to a where this request does not even carry.

cube is the locator that is actionable, and the message names the dataset and says the same definition is valid on a native-SQL deployment. It stays INVALID_FIELD rather than becoming DATASET_INVALID because it is the same physical verdict as its neighbour — this engine cannot join this member — and splitting the code by provenance would make a caller branch on two wire shapes for one capability limit. The four refusals that predate this card keep their messages byte-identical (#5923's tests read them); this one is a fifth, separately worded.

The as-is pin

crossobject-conjunct-refusal.test.ts's last test was written to the behaviour as it was, deliberately, so it would go red the day the decision landed. It did. It is flipped, not deleted, and the explanatory paragraph above it is rewritten so the next reader sees the refusal is intended rather than a leftover explanation of a defect that no longer exists. The file header's "FOUR directions" section is rewritten to six.

Six pinned directions

direction
①–④ #10759's: nested cross-object member refused on both doors; clean combinators still served carrying their disjunction; generateSql() unchanged; the ordinary dataset conjunct not misread. Re-run unchanged — 15 of the 18 tests, all green.
⑤ a cross-object dataset-level filter is refused on both doors, with code and status, the member, the cube, the absent param, and engine.aggregate never reached
⑥ load-bearing: an ordinary dataset scope (is_deleted: false) still passes both doors and still reaches the engine carrying its predicate

Two-door agreement is asserted as agreement over one query — [execute === undefined, generateSql === undefined] compared to [false, false] — not as two independent expectations, which is the shape #10862 used. A mixed scope (one base leaf + one cross-object leaf) is pinned refused, and the caller's-where case is kept as the known-present control that still carries param: 'where'.

Proof

Ablation, signature predicted in writing before mutating. Mutation: drop the dataset-scope arm from filterMemberView, leaving the new refusal arm starved of input. Predicted 3 failed | 15 passed (18), naming the three tests and their first-failure assertions, with ⑥ and the known-present control staying green (that asymmetry is the point). Observed: exactly that — expected undefined to be an instance of Error, then expected [true,true] to deeply equal [false,false] twice. Restore proved byte-identical (git hash-object dd7d329f… both sides) and re-run to a real verdict: 18 passed (18).

src/ vs dist/, argued from the files and then measured. The test imports '../analytics-service.js', which imports './strategies/objectql-strategy.js' — both relative, so the package exports map is never consulted for the subject. Re-verified rather than inherited from #10862's seat, in the stronger form: with a built, correct dist/index.js present, a src-only mutation flipped the verdict 18 passed → 3 failed, while dist/index.js stayed byte-identical (md5 2ed2099a… both sides) and never contained the mutation marker.

Zero-hit counter-check, positive control first. filterMemberView(cube, query → 2 hits, both the 3-arg form (the two doors: the pattern demonstrably matches). filterMemberView(cube, query) (the 2-arg form) → 0. All four conjuncts.push sites were then read, not counted: two are the caller's where via applyFilterNode, one is the dataset scope (now in the view), one is the time-dimension window (refused by planCrossObject's own timeDimensions loop, which runs first). The platform read scope is applied after the plan and is base-object by construction.

Gates — derived on the final commit d24441362, clean tree

node scripts/pm/dispatch-gates.mjs with no path arguments (3 committed paths, 0 working tree, 0 untracked, three-dot vs merge base 072d072d2). It named 11 path-derived families plus 5 convention-triggered ones. Re-derived after the amend; the list was identical. Exit codes captured before any pipe.

gate verdict
check:changeset-gate-self-tests ✓ 118 + 212 assertions over real temp git repos
check:objectui-changeset ✓ releasing list derived from changesets
check:slot-lookup ✓ ratchet holds: 107 unswept sites, none new
check:test-source-alias ✓ 72 packages scanned; 45 published subpaths resolved
check:type-source-resolution ✓ 77 packages with a tsconfig scanned
check-adr-0087-registration ✓ 1 declared-breaking changeset, carrying a disposition
check-changeset-no-major ✓ no major bump
check-ci-filter-parity ✓ all 83 declared cross-package globs covered
check-empty-changeset ✓ no empty-frontmatter changeset
check-plugin-teardown-shape ✓ 63 Plugin implementations, baseline fully burned down
check-affected-docs ✓ self-test 339 cases pass
check:query-options-erasure ✓ ratchet holds: 67 unswept sites, none new
check:type-check-coverage ✓ self-test 47+59+29+28+19 cases hold
check:type-check-debt ✓ 33 ledger entries re-measured in 248.9s, 1908 raw errors, none above its recorded number
check:engine-double-contract ✓ 377 pinned, 133 in DEBT, 2 exempt
check:where-matcher ✓ 277 matchers, all conform
check-nul-bytes ✓ 6394 files, no raw ASCII control bytes

check:type-check-debt first refused (--re-measure cannot run: 47 workspace dependencies … no built type entry point). A refusal is NOT MEASURED, so the closure was built exactly as lint.yml does (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful) and it was re-run to the real verdict above. check-adr-0087-registration was genuinely red first — this changeset declares BREAKING and carried no disposition marker — and is green on the argued not-required (no-migration-prescription) line now in the changeset.

Tests. @objectstack/service-analytics: 80 files / 1766 tests passed. Consumer door: @objectstack/rest's 7 analytics suites, 85 tests passed, run against a freshly built dist that demonstrably carried the new message.

Breaking-vs-not, argued in the open

Called breaking in the changeset and minor in the bump. A query that returns 200 with rows today starts answering 400, and it does so because of a saved dataset rather than anything in the request — a dashboard that renders today can start showing an error. That is the strongest reading of breaking and it is why it is stated rather than filed quietly. What is not lost is any correct answer: the rows that stop being served were already wrong, and wrong in the way that hides itself. An ordinary dataset scope is unaffected and pinned so.

Posture

Stays draft. needs:contract-review is hung on #10861 and this seat does not clear it, mark ready, arm auto-merge, or merge. The absent-param choice above is the specific thing worth a reviewer's eye.


Generated by Claude Code

… both ObjectQL doors (#10861)

PR #10758 gave a compiled dataset's definition-level `filter` a route onto the
ObjectQL door for the first time. That route was outside the member view
`planCrossObject` judges, so a dataset declaring `filter: { 'account.region':
'West' }` was accepted by BOTH doors and `engine.aggregate` received
`{"$and":[{"account.region":"West"}]}` — a predicate it cannot join.

`account.region` is not a column of `opportunity`, so on any driver that
evaluates it honestly the predicate matches nothing and the widget answered a
number that was neither the scoped number nor the unscoped one, with no error
anywhere. #10759 was not this card: the two doors AGREED in accepting it, so
there was no preview/execution divergence to restore, and refusing it is a new
decision about the refusal set.

Ruled by the maintainer on 2026-08-22 (decision-inbox digest, accepted verbatim
「接受所有」): Option A — refuse at query time. `filterMemberView` now folds the
dataset scope's leaves into the one view both doors are judged on, so the
refusal fires where driver capability is known. Compile-time rejection in
`dataset-compiler.ts` was not taken: the compiler cannot see which driver will
serve the dataset, and the same definition is legal on a native-SQL deployment.

The refusal keeps the `invalidMemberError` family (INVALID_FIELD / 400) — the
same physical verdict as its neighbour, so a caller need not branch on two wire
shapes for one capability limit — and carries `member` + `cube`. `param` is
deliberately absent: `AnalyticsRequestKey` is the request vocabulary,
`AnalyticsQuerySchema` is strict and has no `filter` key, and the request may
carry no `where` at all, so both `'where'` and a widened `'filter'` would name a
key the caller cannot go and edit.

The as-is pin #10759 left in `crossobject-conjunct-refusal.test.ts` went red as
designed; it is flipped to the new behaviour and its explanatory paragraph is
rewritten, so the next reader sees the refusal is intended rather than a
leftover explanation of a defect that no longer exists.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/data-modeling/analytics.mdx (via account.region (literal))
  • content/docs/permissions/rls.mdx (via account.region (literal))
  • content/docs/ui/dashboards.mdx (via account.region (literal))

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

  • content/docs/releases/v14.mdx (via generateSql (symbol))
  • content/docs/releases/v17.mdx (via ObjectQLStrategy (symbol), generateSql (symbol))

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
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 8 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 3e26359a7d87fbe0e03c7236346166c71a821ec0 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 3e26359a7d87fbe0e03c7236346166c71a821ec0

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

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32581945280 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 5.80s
    

↳ 失败原因 是判读的关键:超时(Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言(AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️ 本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 82 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Merged via the queue into main with commit 13a3dca Aug 22, 2026
32 checks passed
@os-warren
os-warren deleted the claude/issue-10861-dataset-scope-crossobject-refusal branch August 22, 2026 15:47
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A cross-object dataset-level filter reaches engine.aggregate unrefused on the ObjectQL path — both doors accept what the engine cannot join

2 participants