bvandeusenandClaude Opus 5
91b34619f9
verify_with / expires_when now reach a rule through both doors and come back on every read. The open question this step existed to settle was how to UNSET a nullable field, and the answer is one convention per door: - MCP: "" still means "leave unchanged" — an agent filling three fields must not wipe the other five — so clearing is explicit, clear_fields=["..."]. Naming the field is the one form that cannot happen by accident. - REST: a cleared form input arrives as "", and the service normalises "" to NULL for every nullable rule column, so an emptied input does what it looks like it does. Two idioms, one outcome, and the normalisation is what makes the step-3 sweep correct: `verify_with IS NOT NULL` would otherwise be true for every rule ever touched through the UI, and the sweep would list the whole rulebook and mean nothing. to_dict renders "" and NULL identically, so this is only visible against a real column — hence the integration module rather than a mock. Editing verify_with drops verified_at. A stamp certifies A CHECK, not a rule; reword the check and the old stamp vouches for something that no longer exists. Safe direction, same asymmetry as _valid_tier: a rule wrongly listed as due costs one look, a rule wrongly vouched for costs the thing the sweep exists to catch. Editing anything else leaves the stamp alone, or a rulebook tidy-up would reset every constraint and the ordering would carry nothing. Reads: rule_brief attaches `last_verified` ONLY to a rule that carries a check — its presence is the signal, and it says both "this asserts a fact that can go false" and "here is how long ago anyone confirmed it". "never" rather than null, per #2483. The check text itself stays in get_rule; a listing needs to know which rules can rot, not how to test them. Search hits carry the full trio, since a hit is exactly the moment someone is about to act on a rule. Also folds in the #3078 finding, which had been sitting as a note: create_rule now teaches that when_to_apply is the retrieval surface and must carry the SYMPTOM — the words you would type while stuck — not just the situation. fake_rule gains the three fields as None for the reason the helper already documents one line up: unnamed, verify_with is a truthy MagicMock and every stand-in rule would claim a check it does not have. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
137 lines
5.3 KiB
Python
137 lines
5.3 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")
|