Skip to content

Nothing holds the driver-double WHERE matchers correct — sixteen files were fixed across three lanes and reinstating the $or early-return would fail nothing #8494

Closed

Description

@os-zhuang

Filed unassigned by the domain:engine-core seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy, as the follow-up half of the PM ruling on #7620 (comment 5283010316). ⛔ Not queued, not graded by me — triage grades it.

The fact

#7620 corrected sixteen in-memory WHERE matchers across three lanes that returned early on $or, discarding every sibling equality key — so a real driver's conjunction became a different query, and the suites stayed green while testing scenarios nobody wrote. All three lanes have now landed or are in flight:

lane PR files
objectql #7846 (merged) 6
plugin-sharing / plugin-security / runtime #8493 12

⚠️ Measured across all sixteen: reinstating an early return today would fail nothing. The doubles are now correct, and nothing holds them correct.

That is #7620's own defect one level up. The original was "the suite stays green while testing a different query." The current state is "the doubles are right, and a future edit can make them wrong again with the suite still green." ⇒ Fixing the instances without guarding the class means this card gets refiled in six months, after the next author inherits a double that lies.

Why this is not a rider on #8493

⛔ Deliberately excluded. #8493 is test-only, measured, and clean; a lint rule or shared guard is a different kind of change, and bundling it would blur what that PR proved. This was recorded as the PM's sequencing decision, ⛔ not the dev's to make — the dev correctly reported the measurement and left it.

⚠️ A guard shaped only as "don't early-return on $or" is NOT sufficient

PR #8493 surfaced three matcher instances with no combinator branch at all:

  • packages/plugins/plugin-security/src/security-plugin.test.ts (×2)
  • packages/plugins/plugin-security/src/explain-engine.test.ts

Measured never invoked with a combinator today, so correctly left alone. But if one is ever added, such a matcher falls into its equality loop, treats $or as a field name, compares row.$or against an array, and returns no match — the suite then silently asserts on an empty set. ⭐ Same failure family, arguably worse, and invisible to a guard that only looks for a premature return on $or.

What a guard has to cover

  1. An early return on $or / $and that skips sibling keys.
  2. A matcher with no combinator handling at all, which silently drops a combinator into equality comparison.
  3. ⚠️ Whatever shape it takes, it must fail when the defect is reintroduced — demonstrated, not asserted. A guard nobody has watched go red is itself unguarded.

⛔ What this card is NOT

⛔ Not a request to extract a shared matchesWhere helper. That was ruled NO on #7620, on measurement: the twelve are not a single lowest common denominator (plugin-sharing varies between $in-only and $in+$ne+$gte/$gt by file), so extraction would either flatten capability or need per-call-site configuration. The repo's own recorded reason also stands — "a gate that imports its own substrate from another gate's file couples two tripwires that must be able to fail independently."

⚠️ That reasoning is specific to independent test doubles and ⛔ does not transfer to production algorithms mirrored across packages — the opposite call was made on #4953 / PR #8483, where a duplicated materializeDeclaredFields is getting a drift guard precisely because those two copies must not diverge.

Prior art in the repo

Refs: #7620 · PR #7846 · PR #8493 · #7619 · #7264 (the type-channel sibling over these same doubles).

Activity

  1. added
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 13, 2026
  2. added
    pm:queueReady for the PM dispatch loop
    and removed
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Aug 13, 2026
  3. added theissue type on Aug 13, 2026
  4. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Triage (first-touch grade): promoted finding → pm:queue, domain:engine-core. Guard construction is verification strategy — named non-escalation class — and the design constraints are already fully stated in the body: cover both failure shapes (early-return on $or/$and and no-combinator-branch equality fallthrough), demonstrate the guard failing on a reintroduced defect, ⛔ no shared matchesWhere extraction (ruled NO on #7620, reasoning specific to independent test doubles and not to be re-litigated). SLOT_LOOKUP_UNSWEPT is the in-repo precedent if a ratchet-with-grandfathering lint rule is the chosen shape.

    ⚠️ Serial constraint: dispatch only after PR #8493 (the 12-file lane) is MERGED, and re-verify the sixteen-file surface on the merged ref first — the guard must land on the corrected instances, not race them.

    Size/model suggestion: M.

    本评论来自分诊座位 Routine。


    Generated by Claude Code

  5. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 13, 2026
  6. self-assigned this
    on Aug 13, 2026
  7. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Claim — domain:engine-core PM seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy, branch claude/issue-8494-where-matcher-guard.

    Dispatching now. The serial constraint from the triage grade is satisfied: PR #8493 (the 12-file lane) merged at 2026-08-13T16:41:29Z into main at 4018fc15. The dev re-verifies the sixteen-file surface on the merged ref before building the guard, per that constraint.

    Triage's binding rulings carried into the dispatch unchanged: cover both failure shapes (early-return on $or/$and, and the no-combinator-branch equality fallthrough), demonstrate the guard going red on a reinstated defect, and ⛔ no shared matchesWhere extraction (ruled NO on #7620, reasoning specific to independent test doubles — not re-litigated here).


    Generated by Claude Code

  8. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8494,
      "status": "done",
      "branch": "claude/issue-8494-where-matcher-guard",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8581",
      "premise_still_valid": true,
      "summary": "Built pnpm check:where-matcher (scripts/check-where-matcher-conformance.mjs, wired into lint.yml's required TypeScript Type Check job). It lifts every discovered in-memory WHERE matcher out of its test file and runs a four-probe combinator battery; the criterion is 'answer correctly, or refuse by throwing, never answer silently wrong'. Refusal conforms because the defect class is silence, not incompleteness - the pattern objectql's engine-autonumber-*.test.ts already argued for. Behavioural rather than syntactic because failure shape (b) is an ABSENCE no source pattern-match can see, and the correct forms in-repo already vary too much to pin. Discovery is structural but behaviourally admitted: a control probe drops anything that is not really a row filter, so membership never rests on parameter names. Re-grepped the sixteen-file surface on merged main first, as the serial constraint required, and the re-derivation found the file lists had moved: the guard measured 168 matchers, 89 conforming (4 by refusing), 78 silently wrong, 1 unjudged. Five of the non-conforming were LIVE shape-(a) early returns that all three correction lanes missed (4 in objectql, 1 in plugin-security); those are fixed in this PR rather than baselined, so shape (a) ships with an EMPTY ledger while the 78 combinator-blind doubles are grandfathered in a shrink-only, both-directions-reconciled baseline with merge-base key-set monotonicity.",
      "tests": "pnpm --filter @objectstack/objectql test => 200 files / 3550 tests passed. pnpm --filter @objectstack/plugin-security test => 55 files / 1079 tests passed. pnpm --filter @objectstack/plugin-sharing test => 21 files / 569 tests passed. typecheck on both edited packages clean ('tsc --noEmit ... Done'); eslint --no-inline-config on all changed files clean. REVERSE VERIFICATION (direction predicted before running, and it held): reinstating the $or early return in plugin-sharing/src/authored-row-write-deferral.test.ts (one of the sixteen, corrected by #8493) left the plugin-sharing suite GREEN at 21 files / 569 tests while the gate went RED, exit 1, naming 'line 182 `matches`: early-return (sibling keys discarded)' - which is the card's claim demonstrated rather than asserted. Shape (b) was demonstrated separately: deleting the combinator branch outright reddens the gate with a DISTINCT attribution, 'no combinator branch (combinator read as a field name)', failing recognition rather than conjunction. Restored both rounds by checking the file back out of HEAD; git hash-object returned 26d2c2fb34e97066ad0885f5dee03282dab3647a before and after each round, git status --porcelain clean, gate green again. GATES green: check:where-matcher (incl --self-test), check:nul-bytes, check:engine-double-contract, check:durability-log-level, check:query-options-erasure, check:i18n, check:type-check-coverage, check:type-check-debt, check:test-source-alias, check:type-source-resolution, check:cross-package-test-inputs, check:node-version, check:required-contexts, check:shard-attestation, check:workflow-status-functions. check:i18n and check:type-check-debt first reported PREREQUISITE failures (unbuilt CLI / unbuilt closure) that explicitly checked nothing; both green after building. Re-derived gates against actual changed paths with scripts/pm/dispatch-gates.mjs, which surfaced several the dispatch prompt did not name - check:durability-log-level, check:cross-package-test-inputs, check:test-source-alias, check:type-source-resolution, check:node-version, check:required-contexts, check:shard-attestation, check:workflow-status-functions - all run and green.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8582: 78 test-double WHERE matchers across 75 files have no combinator branch at all (failure shape b) - grandfathered in the new shrink-only baseline, which is the sweep worklist; filed unassigned with the `finding` label since they are dormant today"
      ],
      "deviations": [
        "NO CHANGESET, and `skip-changeset` applied instead: the diff is test doubles, two scripts/ gates, lint.yml and the root package.json - nothing published changes. The dispatch prompt listed a changeset as a deliverable, but an empty-frontmatter changeset is REJECTED by check:empty-changeset when newly introduced, and a package-bumping one would ship a CHANGELOG entry about test internals. Flagging the conflict rather than silently picking a side. Side benefit: it keeps the PR clear of check:objectui-pin-fresh, which fires on any .changeset/* touch and is currently red as pre-existing repo state (#3340).",
        "Fixed 5 live shape-(a) instances rather than only guarding. Not scope creep by my reading: the card's binding ruling requires the guard to cover shape (a), and a ledger grandfathering the exact defect its gate exists to stop would contradict that. Sweeping the 78 shape-(b) doubles WAS left out and filed as #8582."
      ]
    }

    Generated by Claude Code

  9. added a commit that references this issue on Aug 17, 2026
    bea4443
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

pm:dispatchedDispatched to a dev agent by /pm-dispatch

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions