Skip to content

validations[] script rules: the case that DEFINES them as invariants (a stored violation refused on an unrelated edit) has no pin #14891

Closed

Description

@claude

Observation, filed unassigned. Found while writing the spec docblocks for #13879 (transition gate vs invariant). Not a behaviour defect — the engine does the right thing; the pin for the case that defines the behaviour is missing.

What the engine does

checkPredicate (packages/objectql/src/validation/rule-validator.ts) evaluates a script (and cross_field) condition against ctx.merged — the prior row overlaid with the incoming patch — on every write, with no exemption for a violation that was already stored. So a row that already violates is refused on any edit until a repairing write lands: frozen, not bricked. That is exactly what makes it an invariant rather than a transition gate, and as of #13879 it is stated as contract text in packages/spec/src/data/validation.zod.ts (the script rule's condition describe string and the module docblock, both of which ship as the JSON Schema description and the generated reference page).

The gap

packages/objectql/src/validation/rule-validator.test.ts, block script / cross_field predicates, pins:

  • a write that CARRIES the offending value ("rejects when the failure predicate is true (end before start)"),
  • a write that makes the predicate false ("allows when the predicate is false (merged record honours unchanged fields)"),
  • an unevaluable predicate (fail-closed).

Nothing pins the write that touches an unrelated field while the stored row violates. That case is the whole difference from Field.requiredWhen, whose corresponding case IS pinned on the other side of the boundary ("legacy rows rest: a pre-existing violation does not block an unrelated write"). So the two mechanisms' distinguishing behaviours have asymmetric coverage: the gate's exemption is pinned, the invariant's absence of one is not.

Measured

2026-09-03 at 2263ca4d6, scratch vitest over evaluateValidationRules with a rule condition: record.discount is greater than 60:

write previous row result
touch an unrelated field only discount 90 ValidationError (frozen)
repairing write, discount 50 discount 90 passes (not bricked)

Both behave as the new spec text now claims. The scratch file was deleted rather than landed, to keep that PR inside packages/spec.

Suggested fix

Two it cases in the existing script / cross_field predicates block, one per row above. Note PR #14801 is open on rule-validator.ts itself; the test file is a separate path, but whoever picks this up should check that PR's state first.

Generated by Claude Code


Generated by Claude Code

Activity

  1. huangyiirene commented on Sep 3, 2026

    @huangyiirene
    Collaborator

    Triage — graded. tests · priority:p3 · pm:queue · domain:engine; finding removed. ⭐ finding box → 0.

    Verified: the asymmetry is exactly as described. rule-validator.test.ts:65 pins the gate side — "legacy rows rest: a pre-existing violation does not block an unrelated write" — while the script / cross_field predicates block pins only :1813 (write carries the offending value) and :1824 (predicate made false). ⇒ No third case, and the one missing is the one that distinguishes the two mechanisms.

    ⭐ That framing is what makes this worth a card rather than a note: the contract text shipped by #13879 now states the invariant behaviour in validation.zod.ts's condition describe string — which ships as the JSON Schema description and the generated reference page. So the published claim and the pinned behaviour have diverged in coverage, not in truth. ⛔ Nothing is broken today; what is missing is the thing that would notice if it broke.

    Scope — terminal. Two it cases in the existing block, one per measured row: unrelated-field write against a violating stored row ⇒ ValidationError (frozen); repairing write ⇒ passes (not bricked).

    ⛔ Do not change checkPredicate or any behaviour. If either case does not reproduce as the card measured, ⛔ do not adjust the engine to make the test pass — stop and report, because that would mean the contract text #13879 shipped is wrong, which is a spec-lane card and a different conversation.

    ⚠️ PR #14801 is open on rule-validator.ts. The test file is a separate path so this need not block, but ⛔ check that PR's state first — if it touches predicate evaluation, these pins should land against its result, not before it.

    ✅ The scratch vitest was deleted rather than landed to keep #13879's PR inside packages/spec — correct, and the reason this arrives as a measured card instead of an unreviewed rider.


    Generated by Claude Code

  2. github-actions commented on Sep 6, 2026

    @github-actions
    Contributor

    os-closed-card-sweep — machine-findable marker for this generated comment.

    Removed the pm-loop state label(s) this closed card no longer claims: pm:dispatched.

    A state label claims work is in flight. This card is closed on a merged delivery, so the claim
    is stale; every other label is left exactly as it was found. Nothing here is a judgement about
    the card, and no verdict-bearing label is ever touched by this sweep.

    posted by half-state-patrol run 34005012908 · trigger schedule

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions