Repository navigation
fix(service-analytics): refuse a cross-object dataset-level filter on both ObjectQL doors (#10861) - #11066
Conversation
… 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
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 2 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 32581945280 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
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
filtera route onto the ObjectQL door for the first time. That route was outside the member viewObjectQLStrategy.planCrossObjectjudges —execute()lowers the scope ontoconjunctsafter the view is built, andgenerateSql()renders it from a separategetDatasetScope(...)?.filterread — 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 at5f2e54cc6, so the defect survived every move of the tree in between:engine.aggregatecannot join.account.regionis not a column ofopportunity, 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.
filterMemberViewnow 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 indataset-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.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 byresolveFieldName(..., 'any')and valuedtrue. It is now keyed the same way and valued by provenance ('where' | 'dataset-filter') — the value slot was already documented as a placeholderplanCrossObjectnever 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 noparam. That is a decision, not an omission, and it is pinned as such:AnalyticsRequestKeyis documented as "which request key named the member". No request key named this one.AnalyticsQuerySchemaisstrictObjectand has nofilterkey (its ownguidancesendsfilters→where), so a widenedparam: 'filter'would name a key the schema rejects — the "never suggest a key the schema cannot accept" trapstrict-object.tsdocuments.param: 'where'would send the caller to awherethis request does not even carry.cubeis the locator that is actionable, and the message names the dataset and says the same definition is valid on a native-SQL deployment. It staysINVALID_FIELDrather than becomingDATASET_INVALIDbecause 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
generateSql()unchanged; the ordinary dataset conjunct not misread. Re-run unchanged — 15 of the 18 tests, all green.filteris refused on both doors, withcodeandstatus, the member, thecube, the absentparam, andengine.aggregatenever reachedis_deleted: false) still passes both doors and still reaches the engine carrying its predicateTwo-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-wherecase is kept as the known-present control that still carriesparam: '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. Predicted3 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, thenexpected [true,true] to deeply equal [false,false]twice. Restore proved byte-identical (git hash-objectdd7d329f…both sides) and re-run to a real verdict:18 passed (18).src/vsdist/, argued from the files and then measured. The test imports'../analytics-service.js', which imports'./strategies/objectql-strategy.js'— both relative, so the packageexportsmap is never consulted for the subject. Re-verified rather than inherited from #10862's seat, in the stronger form: with a built, correctdist/index.jspresent, asrc-only mutation flipped the verdict18 passed→3 failed, whiledist/index.jsstayed 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 fourconjuncts.pushsites were then read, not counted: two are the caller'swhereviaapplyFilterNode, one is the dataset scope (now in the view), one is the time-dimension window (refused byplanCrossObject's owntimeDimensionsloop, 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 treenode scripts/pm/dispatch-gates.mjswith no path arguments (3 committed paths, 0 working tree, 0 untracked, three-dot vs merge base072d072d2). 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.check:changeset-gate-self-testscheck:objectui-changesetcheck:slot-lookupcheck:test-source-aliascheck:type-source-resolutioncheck-adr-0087-registrationcheck-changeset-no-majormajorbumpcheck-ci-filter-paritycheck-empty-changesetcheck-plugin-teardown-shapecheck-affected-docscheck:query-options-erasurecheck:type-check-coveragecheck:type-check-debtcheck:engine-double-contractcheck:where-matchercheck-nul-bytescheck:type-check-debtfirst refused (--re-measure cannot run: 47 workspace dependencies … no built type entry point). A refusal is NOT MEASURED, so the closure was built exactly aslint.ymldoes (turbo run build --filter='./packages/*' --filter='./packages/*/*', 70/70 successful) and it was re-run to the real verdict above.check-adr-0087-registrationwas genuinely red first — this changeset declares BREAKING and carried no disposition marker — and is green on the arguednot-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 builtdistthat demonstrably carried the new message.Breaking-vs-not, argued in the open
Called breaking in the changeset and
minorin the bump. A query that returns200with rows today starts answering400, 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-reviewis hung on #10861 and this seat does not clear it, mark ready, arm auto-merge, or merge. The absent-paramchoice above is the specific thing worth a reviewer's eye.Generated by Claude Code