CI & Build / Python lint (push) Successful in 4s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / TypeScript typecheck (push) Successful in 34s
CI & Build / integration (push) Successful in 32s
CI & Build / Python tests (push) Successful in 1m8s
CI & Build / Build & push image (push) Successful in 35s
The query the last two steps were storage for. `rules_due_for_verification` returns every rule carrying a `verify_with`, ordered by `verified_at` ASC NULLS FIRST, each row carrying the check IN FULL — the opposite call from rule_brief, because the reader is about to go and run it. NULLS FIRST is the ordering this turns on. Postgres sorts NULLs last on an ASC ordering, which would put the rules nobody has ever confirmed BEHIND every rule someone once looked at. Exactly backwards: a claim with no evidence at all outranks an old one. Rules with no check never appear, and that is the property that keeps the list worth reading. Most rules are decisions — no truth value, nothing to go and check. If they appeared here the sweep would be the rulebook. `mark_rule_verified(rule_id, still_true)` closes the loop, asymmetrically: passing writes a stamp, FAILING WRITES NOTHING. There is no "verified false" state because a rule whose check failed is not in a special condition, it is wrong — and recording the failure as a flag would let it sit there being false with the sweep satisfied that someone had looked. So it stays at the top until someone corrects or retires it, and the response says so. An unrecognised `tier` filter raises rather than falling back. _valid_tier's silent always_on default is right for a WRITE — a typo should leave a rule binding — and wrong for a FILTER, where the same fallback quietly answers a different question and returns a short list that reads as good news. Deliberately NOT filterable by project: a project reaches rules through project scope, subscriptions, always-on rulebooks and exclusions, and a filter missing one of those paths would UNDER-report — the exact failure this surface exists to prevent. Said so in the docstring rather than shipping a half-correct filter. Ownership-scoped like every other rule read (owned rulebook, or owned project), in ONE statement with an OR across the XOR rather than two queries merged in Python, so the ordering is the database's and cannot disagree with itself. Note that rules have no sharing ACL in this schema — no rule_shares, no rulebook_shares — so there is no wider set for access.py to consult here. Also fixes a test title that had been lying for ten tools: "all sixteen tools" asserted 26. The number now lives only in the assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
276 lines
11 KiB
Python
276 lines
11 KiB
Python
"""Real-Postgres tests for a rule's CHECK — the write half (milestone 312).
|
|
|
|
What mocks cannot prove, and what the staleness sweep depends on:
|
|
|
|
1. **Empty means NULL.** The sweep asks for rules where `verify_with` is set.
|
|
A write that stored "" would leave a column that is neither null nor
|
|
content, and every rule ever touched through the REST door would answer
|
|
"yes, I have a check" — the sweep would list the whole rulebook and mean
|
|
nothing. Only a real column can show the difference; `to_dict`'s `or ""`
|
|
renders both the same.
|
|
|
|
2. **Clearing is possible at all.** "" means "leave unchanged" at the MCP
|
|
door, so without an explicit clear there is no way to retire a check.
|
|
|
|
3. **A stamp does not outlive the check it certifies.** Reword the check and
|
|
the old `verified_at` vouches for something that no longer exists.
|
|
"""
|
|
from datetime import datetime, timezone
|
|
|
|
import pytest
|
|
import pytest_asyncio
|
|
|
|
from scribe.models import async_session
|
|
from scribe.models.rulebook import Rule
|
|
from scribe.services import rulebooks as rulebooks_svc
|
|
from tests.helpers import ensure_user
|
|
|
|
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures("_dispose_engine")]
|
|
|
|
|
|
@pytest_asyncio.fixture
|
|
async def constraint():
|
|
"""One rule carrying a check, already verified.
|
|
|
|
Verified at creation time rather than left null, because every assertion
|
|
here is about what happens to an EXISTING stamp — a fixture that started
|
|
null could pass all of them by doing nothing.
|
|
"""
|
|
async with async_session() as s:
|
|
owner = await ensure_user(s, "verification_owner")
|
|
uid = owner.id
|
|
await s.commit()
|
|
|
|
book = await rulebooks_svc.create_rulebook(uid, "Environment facts")
|
|
topic = await rulebooks_svc.create_topic(book.id, uid, "ci")
|
|
rule = await rulebooks_svc.create_rule(
|
|
topic.id, uid, "The runner has no bash",
|
|
"Write every `run:` step in POSIX sh.",
|
|
verify_with="read the workflow's shell setting",
|
|
expires_when="the runner can be given a bash shell",
|
|
)
|
|
async with async_session() as s:
|
|
row = await s.get(Rule, rule.id)
|
|
row.verified_at = datetime(2026, 8, 1, tzinfo=timezone.utc)
|
|
await s.commit()
|
|
return {"uid": uid, "rule": rule.id}
|
|
|
|
|
|
async def _row(rule_id: int) -> Rule:
|
|
async with async_session() as s:
|
|
return await s.get(Rule, rule_id)
|
|
|
|
|
|
async def test_the_check_and_its_expiry_persist(constraint):
|
|
row = await _row(constraint["rule"])
|
|
assert row.verify_with == "read the workflow's shell setting"
|
|
assert row.expires_when == "the runner can be given a bash shell"
|
|
assert row.verified_at is not None
|
|
|
|
|
|
async def test_an_empty_string_becomes_null_not_an_empty_column(constraint):
|
|
"""The REST door's idiom: a cleared form input arrives as "".
|
|
|
|
NULL is asserted directly against the column rather than through to_dict,
|
|
which renders `None` and `""` identically — the difference this test
|
|
exists for would be invisible one layer up.
|
|
"""
|
|
await rulebooks_svc.update_rule(
|
|
constraint["rule"], constraint["uid"], verify_with="", expires_when="",
|
|
)
|
|
row = await _row(constraint["rule"])
|
|
assert row.verify_with is None
|
|
assert row.expires_when is None
|
|
|
|
|
|
async def test_naming_a_field_in_clear_empties_it(constraint):
|
|
"""The MCP door's idiom, where "" already means "leave this alone"."""
|
|
await rulebooks_svc.update_rule(
|
|
constraint["rule"], constraint["uid"], clear=["verify_with"],
|
|
)
|
|
row = await _row(constraint["rule"])
|
|
assert row.verify_with is None
|
|
# expires_when was NOT named, so it survives — clearing is per-field, and
|
|
# a caller retiring one field must not lose the others.
|
|
assert row.expires_when == "the runner can be given a bash shell"
|
|
|
|
|
|
async def test_rewording_the_check_drops_the_stamp(constraint):
|
|
"""A stamp certifies a check, not a rule.
|
|
|
|
The safe direction, for the same reason _valid_tier falls back to
|
|
always_on: a rule wrongly listed as due costs one look, a rule wrongly
|
|
vouched for costs exactly what the sweep exists to catch.
|
|
"""
|
|
await rulebooks_svc.update_rule(
|
|
constraint["rule"], constraint["uid"],
|
|
verify_with="read the runner's container shell, not the image's",
|
|
)
|
|
row = await _row(constraint["rule"])
|
|
assert row.verified_at is None
|
|
|
|
|
|
async def test_clearing_the_check_drops_the_stamp(constraint):
|
|
await rulebooks_svc.update_rule(
|
|
constraint["rule"], constraint["uid"], clear=["verify_with"],
|
|
)
|
|
row = await _row(constraint["rule"])
|
|
assert row.verified_at is None
|
|
|
|
|
|
async def test_editing_anything_else_leaves_the_stamp_alone(constraint):
|
|
"""The other half of the rule above, and the one that keeps it useful.
|
|
|
|
If any edit reset the stamp, a rulebook tidy-up would put every constraint
|
|
back at the top of the sweep and the ordering would carry no information.
|
|
Only the check's own text invalidates its verification.
|
|
"""
|
|
await rulebooks_svc.update_rule(
|
|
constraint["rule"], constraint["uid"],
|
|
why="act_runner picks the shell, and the image's SHELL directive "
|
|
"applies to the build, not to `run:`.",
|
|
expires_when="the runner grows a shell setting",
|
|
)
|
|
row = await _row(constraint["rule"])
|
|
assert row.verified_at is not None
|
|
assert row.why.startswith("act_runner picks the shell")
|
|
|
|
|
|
# ── the sweep itself (step 3) ──────────────────────────────────────────
|
|
|
|
@pytest_asyncio.fixture
|
|
async def rulebook_of_three():
|
|
"""A decision, a never-checked constraint, and a long-ago-checked one.
|
|
|
|
Three rows because the sweep's whole value is an ORDER, and an order
|
|
cannot be asserted with fewer.
|
|
"""
|
|
async with async_session() as s:
|
|
owner = await ensure_user(s, "sweep_owner")
|
|
uid = owner.id
|
|
await s.commit()
|
|
|
|
book = await rulebooks_svc.create_rulebook(uid, "Sweep fixture")
|
|
topic = await rulebooks_svc.create_topic(book.id, uid, "mixed")
|
|
decision = await rulebooks_svc.create_rule(
|
|
topic.id, uid, "dev is home", "Work directly on dev.",
|
|
)
|
|
never = await rulebooks_svc.create_rule(
|
|
topic.id, uid, "The runner has no bash", "Use POSIX sh.",
|
|
verify_with="read the workflow's shell setting",
|
|
)
|
|
stale = await rulebooks_svc.create_rule(
|
|
topic.id, uid, "Bumps need a dashboard tick", "Tick it first.",
|
|
verify_with="cat CI-runner/renovate/config.js",
|
|
tier="conditional",
|
|
)
|
|
async with async_session() as s:
|
|
row = await s.get(Rule, stale.id)
|
|
row.verified_at = datetime(2026, 1, 1, tzinfo=timezone.utc)
|
|
await s.commit()
|
|
return {
|
|
"uid": uid, "decision": decision.id,
|
|
"never": never.id, "stale": stale.id,
|
|
}
|
|
|
|
|
|
async def test_a_rule_with_no_check_is_never_in_the_sweep(rulebook_of_three):
|
|
"""The common case, and the one that keeps the list worth reading.
|
|
|
|
Most rules are decisions. If they appeared here the sweep would be the
|
|
rulebook, and nobody would read it twice.
|
|
"""
|
|
rules = await rulebooks_svc.rules_due_for_verification(rulebook_of_three["uid"])
|
|
assert rulebook_of_three["decision"] not in [r.id for r in rules]
|
|
|
|
|
|
async def test_never_checked_outranks_checked_long_ago(rulebook_of_three):
|
|
"""NULLS FIRST is the ordering decision this surface turns on.
|
|
|
|
Postgres sorts NULLs LAST by default on an ASC ordering, which would put
|
|
the rules nobody has ever confirmed at the BOTTOM — behind every rule
|
|
that at least once had someone look at it. That is exactly backwards: a
|
|
claim with no evidence at all outranks an old one.
|
|
"""
|
|
ids = [r.id for r in await rulebooks_svc.rules_due_for_verification(
|
|
rulebook_of_three["uid"]
|
|
)]
|
|
assert ids.index(rulebook_of_three["never"]) < ids.index(rulebook_of_three["stale"])
|
|
|
|
|
|
async def test_verifying_a_rule_moves_it_off_the_top(rulebook_of_three):
|
|
"""The loop closing: check it, stamp it, and it stops being the question."""
|
|
await rulebooks_svc.mark_rule_verified(
|
|
rulebook_of_three["never"], rulebook_of_three["uid"], still_true=True,
|
|
)
|
|
ids = [r.id for r in await rulebooks_svc.rules_due_for_verification(
|
|
rulebook_of_three["uid"]
|
|
)]
|
|
# Still present — verified is not retired, and it will come due again.
|
|
assert rulebook_of_three["never"] in ids
|
|
assert ids.index(rulebook_of_three["stale"]) < ids.index(rulebook_of_three["never"])
|
|
|
|
|
|
async def test_a_failed_check_writes_nothing(rulebook_of_three):
|
|
"""The asymmetry that keeps the sweep honest.
|
|
|
|
There is no "verified false" state, because a rule whose check failed is
|
|
not in a special condition — it is WRONG. Recording the failure would let
|
|
it sit there being false with the sweep satisfied that someone looked.
|
|
"""
|
|
before = await _row(rulebook_of_three["stale"])
|
|
await rulebooks_svc.mark_rule_verified(
|
|
rulebook_of_three["stale"], rulebook_of_three["uid"], still_true=False,
|
|
)
|
|
after = await _row(rulebook_of_three["stale"])
|
|
assert after.verified_at == before.verified_at
|
|
|
|
|
|
async def test_a_rule_with_no_check_cannot_be_verified(rulebook_of_three):
|
|
"""Nothing to verify is a different answer from verified — and stamping
|
|
one would put a decision into a sweep it has no business being in."""
|
|
assert await rulebooks_svc.mark_rule_verified(
|
|
rulebook_of_three["decision"], rulebook_of_three["uid"],
|
|
) is None
|
|
|
|
|
|
async def test_never_only_and_the_age_filter_narrow_to_what_they_say(rulebook_of_three):
|
|
uid = rulebook_of_three["uid"]
|
|
# Membership, not equality: the integration lane shares one database for
|
|
# the whole run and this fixture is function-scoped, so this owner has
|
|
# accumulated rules from earlier tests. Asserting the exact list would
|
|
# pass alone and fail in the suite.
|
|
never_ids = [r.id for r in await rulebooks_svc.rules_due_for_verification(
|
|
uid, never_only=True,
|
|
)]
|
|
assert rulebook_of_three["never"] in never_ids
|
|
assert rulebook_of_three["stale"] not in never_ids
|
|
assert rulebook_of_three["decision"] not in never_ids
|
|
|
|
# A rule checked in January is well past any sane window; one never
|
|
# checked always qualifies, because it is the most overdue thing there is.
|
|
aged = [r.id for r in await rulebooks_svc.rules_due_for_verification(
|
|
uid, older_than_days=30,
|
|
)]
|
|
assert rulebook_of_three["stale"] in aged
|
|
assert rulebook_of_three["never"] in aged
|
|
|
|
|
|
async def test_the_tier_filter_narrows_to_one_tier(rulebook_of_three):
|
|
ids = [r.id for r in await rulebooks_svc.rules_due_for_verification(
|
|
rulebook_of_three["uid"], tier="conditional",
|
|
)]
|
|
assert rulebook_of_three["stale"] in ids
|
|
assert rulebook_of_three["never"] not in ids
|
|
|
|
|
|
async def test_another_users_rules_are_not_in_your_sweep(rulebook_of_three):
|
|
"""Rules are ownership-scoped: there is no rule-sharing ACL in this
|
|
schema, so the only correct answer is your own rules."""
|
|
async with async_session() as s:
|
|
stranger = await ensure_user(s, "sweep_stranger")
|
|
sid = stranger.id
|
|
await s.commit()
|
|
|
|
assert await rulebooks_svc.rules_due_for_verification(sid) == []
|