Uh oh!
There was an error while loading. Please reload this page.
test: conjoin $or/$and with sibling filters in twelve driver doubles - #8493
Merged
Merged
Conversation
…(part of #7620) Twelve in-memory WHERE matchers across packages/plugins/plugin-sharing, packages/plugins/plugin-security and packages/runtime returned early on $or/$and, discarding every sibling equality key in the same filter object — a real driver ANDs them. Corrected to the same conjoin-with-siblings shape already used by packages/objectql's six (#7846) and by several already-fixed siblings in these two packages. Measured live-vs-dormant per file via an fs.appendFileSync probe (with a positive control proving it would catch a live case): all twelve are dormant today, for two different reasons. plugin-sharing's six never receive $or/$and at all. plugin-security's five and the one runtime file do receive them, but always as the sole key in their filter object (no sibling ever present alongside), so early-return and conjoin produce identical results in every observed call. No test outcome changes. Re-grepped the issue's file enumeration at this branch's base ref rather than trusting it: plugin-sharing/src/sharing-rule.test.ts was already fixed, and three files this commit touches were never named in the issue (plugin-sharing/src/sharing-service.test.ts, plugin-security/src/check-only-write-scope.test.ts, plugin-security/src/select-only-write-visibility.test.ts). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDTnVvsgA6cUZ4xFVtPZRy
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckNo hand-written docs reference the 0 changed package(s). ✅ |
This was referenced Aug 13, 2026
os-zhuang
marked this pull request as ready for review
August 13, 2026 16:24
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #7620 — the last of three lanes (objectql's six landed in #7846).
What
Twelve in-memory
WHEREmatchers acrosspackages/plugins/plugin-sharing,packages/plugins/plugin-securityandpackages/runtimereturned early on$or(and usually$and), discarding every sibling equality key in thesame filter object:
A real driver ANDs them. Corrected to the same "conjoin, don't short-circuit"
shape #7846 already landed for
packages/objectql's six, and that severalsibling files in these same two packages already carried.
Re-grepped at this branch's ref — the issue's file list is stale
Per the issue thread's own correction:
plugin-sharing/src/sharing-rule.test.tswas already fixed before this PR (confirmed unchanged here). Re-grepping
$oracross
plugin-sharing/src/*.test.tsandplugin-security/src/*.test.tsturned up three files the issue never named, which this PR also fixes:
plugin-sharing/src/sharing-service.test.tsplugin-security/src/check-only-write-scope.test.tsplugin-security/src/select-only-write-visibility.test.tsFiles corrected (12 total):
packages/plugins/plugin-sharing/src/:authored-row-write-deferral.test.ts,boot-backfill.test.ts,bulk-recompute.test.ts,record-share-cascade.test.ts,sharing-service.test.ts,system-write-skip-notice.test.tspackages/plugins/plugin-security/src/:authored-row-write-verdict.test.ts,check-only-write-scope.test.ts,row-write-widener-composition.test.ts,select-only-write-visibility.test.ts,vama-write-path-convergence.test.tspackages/runtime/src/domains/:share-links-enforcement-context.test.tsFiles already correct and not touched:
plugin-sharing/src/bu-tree-recompute.test.ts,business-unit-graph.test.ts,recipient-width.test.ts,sharing-rule.test.ts,system-caller-inert-grant.test.ts.packages/runtime/src/domains/meta-published-runtime-publish.test.tsalso buildsa
$or-handling matcher (found by the same re-grep) and already uses the correctconjoining form — confirmed, not touched.
Left alone, but flagged: matchers with no combinator branch at all
Three
matchesinstances —plugin-security/src/security-plugin.test.ts(twoseparate local functions, one in the
explainAccessForCallerdescribe block, onein
ADR-0090 D10 agent intersection) andplugin-security/src/explain-engine.test.ts(one, in a helper explicitly commented
Minimal where-honouring ObjectQL stand-in: scalar equality and $in) — use anObject.entries(where).every(...)shape thatdoes not recognize
$or/$andas combinators at all. This is a differentshape from the bug this card names (there is no early-return branch to fix), so
this PR does not touch them.
Confirmed via full-file grep that none of the describe blocks using these three
instances currently passes
$or/$and— so today they are inert, not silentlywrong. But the failure mode the day someone does add one is worse than the
early-return bug, not better: with no combinator branch,
Object.entriestreats$oras an ordinary field name, comparesrow.$or(normallyundefined)against the
$orarray, gets no match, and the row is silently excluded — thesuite would then assert on an empty result set with no visible error, which is
exactly the risk the issue's own words describe: "dormant is still worth
closing, because the next test that adds an
$orinherits a double that lies."Not fixed here deliberately — a matcher that has no combinator handling at all
is a different defect shape than "conjoin instead of short-circuit," and
bundling a fix for it into this PR would blur what this PR actually measured.
Flagging it here rather than filing a separate issue, since it is speculative
(never exercised) and inert today; if a future change starts sending
$or/$andthrough either helper, this comment is the pointer to why the result would look
wrong without erroring.
Live-vs-dormant — measured, not assumed
Same
fs.appendFileSyncprobe discipline as #7846 (aconsole.logattemptfirst returned nothing and was correctly distrusted rather than read as "zero
calls"). All twelve are dormant, for two different reasons:
plugin-sharing's six: 0 combinator calls across ~1.52M matcherinvocations in the six suites. Positive control: the same probe temporarily
placed in the already-fixed
sharing-rule.test.ts(then reverted) recorded151
$or/ 3$andcalls in the same kind of run — proof the zero above isa real absence, not a dead probe.
plugin-security's five + the oneruntimefile: not all-zero —$andfires 23–71 times per file, and$orfires twice inauthored-row-write-verdict.test.ts. But every single occurrence carriedthe combinator as the only key in its filter object (no sibling ever
present alongside), so early-return and conjoin produce identical results
in every case observed. Dormant for a different reason than
objectql/sharing: invoked, but never mixed with a sibling key.
No test outcome changes anywhere.
plugin-sharing(21 files / 569 tests),plugin-security(52 files / 1037 tests) andruntime(151 files / 2317tests) are green before and after, byte-identical assertions.
Operator-support measurement
Unlike the objectql six (measured byte-identical operator support), these
twelve are not a single lowest common denominator —
plugin-sharing'smatchers vary between
$in-only and$in+$ne+$gte/$gtdepending onthe file;
plugin-security's five and theruntimeone are the mostuniform subset (
$inonly). This inverts what the objectql lane found(byte-identical across all six) and materially changes the shared-helper
question. Detail and rationale for not extracting a shared helper (same
reasoning #7846 used, plus this operator-support gap) is in the changeset.
Two questions this lane does not answer
Both are deliberately left to the PM now that all three lanes have landed:
whether a regression guard is worth adding (none exists today — reinstating
an early return would fail nothing, in any of the sixteen files across all
three lanes), and where a shared
matchesWherehelper would live.Tests
pnpm --filter @objectstack/plugin-sharing test— 21 files / 569 tests, all passpnpm --filter @objectstack/plugin-security test— 52 files / 1037 tests, all passpnpm --filter @objectstack/runtime test— 151 files / 2317 tests, all passpnpm --filter @objectstack/plugin-sharing --filter @objectstack/plugin-security --filter @objectstack/runtime typecheck— cleaneslinton the 12 changed files — cleanGates
node scripts/pm/dispatch-gates.mjsnamed:check:changeset-gate-self-tests,check:cross-package-test-inputs,check:docs-audit-scope,check:objectui-changeset,check:objectui-pin-fresh,check:route-envelope,check:test-source-alias,check:type-source-resolution,check-changeset-no-major.mjs,check-dev-prereqs.mjs,check-objectui-pin-fresh.mjs, plus convention-triggeredcheck:query-options-erasure,check:type-check-coverage/check:type-check-debt(new test files) andcheck:i18n(both touched packages own an i18n-extract config). Plus
check:nul-bytesper AGENTS.md.All green except
check:objectui-pin-fresh, which is pre-existing repo state(
.objectui-shabehindobjectuimain) unrelated to this diff — it only matchedbecause the gate derivation triggers on any
.changeset/path, and PR #8483 hitthe identical thing, which confirms this is repo state rather than either PR's
problem.
Watching for two ratchets that fire on what the change IS rather than which paths
moved —
check:slot-lookupandcheck:type-check-debt— since a sibling PR inthis same sweep saw both trigger outside their derived list; if either goes red
on these new test files in CI, the fix is correcting the errors, not raising the
ledger.
Generated by Claude Code