diff --git a/frontend/src/api/rulebooks.ts b/frontend/src/api/rulebooks.ts index 2c3eb8c..01215e0 100644 --- a/frontend/src/api/rulebooks.ts +++ b/frontend/src/api/rulebooks.ts @@ -55,6 +55,16 @@ export interface Rule { tier: RuleTier; why: string; how_to_apply: string; + /** + * How to check the rule is still true, and the state that ends it. Set + * only on a rule that asserts a fact about something outside the + * operator's control; empty on a rule that is a decision, which is most + * of them. Empty is meaningful, not missing. + */ + verify_with: string; + expires_when: string; + /** When the check last passed. Null means never checked. */ + verified_at: string | null; /** The note or task that caused this rule, if one was recorded. */ arose_from_id: number | null; order_index: number; @@ -80,6 +90,12 @@ export interface RuleHeader { updated_at: string | null; when_to_apply?: string; arose_from_id?: number; + /** + * Present ONLY on a rule that carries a check — the presence of the key + * is itself the signal that this rule asserts a fact that can go false. + * A date (YYYY-MM-DD), or the literal "never". + */ + last_verified?: string; } export interface ApplicableRules { @@ -170,7 +186,14 @@ export async function getRule(id: number): Promise { return apiGet(`/api/rules/${id}`); } -/** The fields both write paths accept. `system_ids` REPLACES a rule's areas. */ +/** + * The fields both write paths accept. `system_ids` REPLACES a rule's areas. + * + * Sending "" for a nullable text field CLEARS it here — the server maps an + * empty string to NULL, so an emptied form input does what it looks like it + * does. (The MCP door reads "" as "leave unchanged" and needs an explicit + * clear_fields list instead; the two idioms reach the same state.) + */ export interface RuleWrite { title: string; statement: string; @@ -181,6 +204,8 @@ export interface RuleWrite { order_index: number; system_ids: number[]; arose_from_id: number | null; + verify_with: string; + expires_when: string; } export async function createRule(topicId: number, data: Partial & { title: string; statement: string }): Promise { diff --git a/src/scribe/mcp/tools/rulebooks.py b/src/scribe/mcp/tools/rulebooks.py index 6d3c094..0e9fae8 100644 --- a/src/scribe/mcp/tools/rulebooks.py +++ b/src/scribe/mcp/tools/rulebooks.py @@ -246,6 +246,14 @@ async def list_always_on_rules(project_id: int = 0) -> dict: Pair with get_project(id).applicable_rules when working on a specific project to also load that project's subscription-derived rules. + A rule carrying `last_verified` asserts a FACT about something outside the + operator's control — a runner's shell, a tool's existence, a setting + somewhere. It is still binding; the field says how long ago anyone + confirmed it, and "never" means nobody has. Follow the rule, and if you + are already standing where the check could be made, make it: get_rule + gives you its `verify_with`. Most rules have no such field, which means + they are decisions and there is nothing to check. + Args: project_id: 0 (default) = the user-wide set. Inside a project, pass its id: an always-on rulebook the project EXCLUDED at inception @@ -276,7 +284,8 @@ async def create_rule( topic_id: int, title: str, statement: str, when_to_apply: str = "", why: str = "", how_to_apply: str = "", order_index: int = 0, tier: str = "always_on", system_ids: list[int] | None = None, - arose_from_id: int = 0, force: bool = False, + arose_from_id: int = 0, verify_with: str = "", expires_when: str = "", + force: bool = False, ) -> dict: """Create a new rule in a rulebook (a SHARED rule — keep it general). @@ -314,6 +323,15 @@ async def create_rule( optional: it decides the tier below, it is how the rule is found when it matters, and a rule nobody can place is a rule nobody applies. + This field is also the rule's RETRIEVAL SURFACE — it and the + statement are what a search is matched against, so it should + carry the SYMPTOM, not just the situation: the words someone + would actually type while stuck. Measured (note 3078): a rule + whose trigger named only its situation did not surface at all + for the problem it solves; adding the symptom to the same field + brought it back as the top hit. Where a rule prevents a specific + failure, put that failure's vocabulary here — the error text, + the wrong behaviour, the dead end. tier: "always_on" (default) or "conditional". The test: can you name the trigger WITHOUT naming a system, an artifact type or a moment? If the honest answer is "whenever you @@ -328,6 +346,23 @@ async def create_rule( cannot be followed and does not survive a rewording. why: Optional rationale — the reason the rule exists. how_to_apply: Optional operationalization — when / where it kicks in. + verify_with: How to CHECK this rule is still true. Set it only when + the rule asserts a fact about something outside your control — a + runner's shell, a bot's config, whether a tool exists. Those go + false silently, with nobody present. Give a command, a path, a + URL or a query; something runnable beats prose, because prose + has to be re-interpreted by whoever finds it. + LEAVE IT EMPTY for a rule that is a DECISION — a preference, a + standard, a way of working. A decision has no truth value: it + changes when you change it, and you know that you did. An empty + verify_with is not a gap, it is the marker for "there is nothing + to go and check," and the whole signal is worthless the moment + it is filled in out of tidiness. + expires_when: The STATE under which this rule stops being true — + "when the runner can be given a bash shell", "when the dashboard + approval setting is turned off". Deliberately not a date: a + constraint expires when the ground under it moves, not on a + schedule. Pairs with verify_with; both empty is the normal case. order_index: Display order within the topic (default 0). force: Bypass the near-duplicate gate. By default, a title-identical rule already in this topic BLOCKS creation and returns its id so you update @@ -343,6 +378,7 @@ async def create_rule( title=title, statement=statement, when_to_apply=when_to_apply, tier=tier, arose_from_id=arose_from_id, why=why, how_to_apply=how_to_apply, order_index=order_index, + verify_with=verify_with, expires_when=expires_when, ) return await rulebooks_svc.rule_detail(uid, rule, system_ids) @@ -351,7 +387,8 @@ async def create_project_rule( project_id: int, statement: str, title: str = "", when_to_apply: str = "", why: str = "", how_to_apply: str = "", order_index: int = 0, tier: str = "always_on", system_ids: list[int] | None = None, - arose_from_id: int = 0, force: bool = False, + arose_from_id: int = 0, verify_with: str = "", expires_when: str = "", + force: bool = False, ) -> dict: """Create a rule scoped to a single project (no rulebook needed). @@ -383,6 +420,13 @@ async def create_project_rule( arose_from_id: The note or task that CAUSED this rule. why: Optional rationale — the reason the rule exists. how_to_apply: Optional operationalization — when / where it kicks in. + verify_with: How to check this rule is still true — see create_rule. + Set it when the rule asserts a fact about someone else's software; + leave it empty when the rule is a decision. Project rules are the + likelier home for a real check: they name this project's files, + paths and quirks, which is exactly the kind of claim that rots. + expires_when: The state under which the rule stops being true — see + create_rule. A state, not a date. order_index: Display order within the project's rule list (default 0). force: Bypass the near-duplicate gate. By default, a title-identical rule already on this project BLOCKS creation and returns its id so you @@ -399,6 +443,7 @@ async def create_project_rule( title=derived_title, statement=statement, when_to_apply=when_to_apply, tier=tier, arose_from_id=arose_from_id, why=why, how_to_apply=how_to_apply, order_index=order_index, + verify_with=verify_with, expires_when=expires_when, ) return await rulebooks_svc.rule_detail(uid, rule, system_ids) @@ -407,12 +452,33 @@ async def update_rule( rule_id: int, title: str = "", statement: str = "", when_to_apply: str = "", why: str = "", how_to_apply: str = "", order_index: int = -1, tier: str = "", system_ids: list[int] | None = None, arose_from_id: int = 0, + verify_with: str = "", expires_when: str = "", + clear_fields: list[str] | None = None, ) -> dict: """Update a rule. Empty strings / order_index=-1 leave fields unchanged. Adding `when_to_apply` and a `tier` to an existing rule is the ordinary way a rule stops being preloaded into every session and starts arriving when it is relevant. `system_ids` REPLACES the rule's areas (pass [] to clear). + + TO EMPTY A FIELD, NAME IT: clear_fields=["verify_with"]. Passing "" cannot + do it — "" means "leave this alone" here, which is what lets you update + two fields without wiping the other six. Clearable: why, how_to_apply, + when_to_apply, verify_with, expires_when, arose_from_id. Clearing and + setting the same field in one call clears it first, so the new value wins. + + Editing `verify_with` DROPS the rule's verification stamp. The stamp + certifies a check, not a rule; once the check is reworded the old stamp + vouches for something that no longer exists, so the rule re-enters the + staleness sweep as never-verified. + + Args: + verify_with: How to check the rule is still true — set it when the + rule asserts a fact about someone else's software, leave it empty + when the rule is a decision. See create_rule. + expires_when: The state under which the rule stops being true. A + state, not a date. See create_rule. + clear_fields: Names of fields to empty, as above. """ uid = current_user_id() fields: dict = {} @@ -430,9 +496,15 @@ async def update_rule( fields["why"] = why if how_to_apply: fields["how_to_apply"] = how_to_apply + if verify_with: + fields["verify_with"] = verify_with + if expires_when: + fields["expires_when"] = expires_when if order_index >= 0: fields["order_index"] = order_index - rule = await rulebooks_svc.update_rule(rule_id, uid, **fields) + rule = await rulebooks_svc.update_rule( + rule_id, uid, clear=clear_fields or (), **fields, + ) if rule is None: raise ValueError(f"rule {rule_id} not found") return await rulebooks_svc.rule_detail(uid, rule, system_ids) diff --git a/src/scribe/mcp/tools/search.py b/src/scribe/mcp/tools/search.py index e7eb1c4..c6db75d 100644 --- a/src/scribe/mcp/tools/search.py +++ b/src/scribe/mcp/tools/search.py @@ -14,6 +14,7 @@ from scribe.services.access import owner_names_for from scribe.services.embeddings import ( DEFAULT_SIMILARITY_THRESHOLD, semantic_search_notes, semantic_search_rules, ) +from scribe.services import rulebooks as rulebooks_svc from scribe.services.retrieval_telemetry import record_retrieval, retrieval_summary @@ -23,7 +24,10 @@ async def _search_rules(uid: int, q: str, limit: int) -> dict: A rule hit carries `why` and `how_to_apply`: they are the operational half of a rule and the session-start payload never includes them, so a caller who went looking should get the whole thing rather than a summary they then - have to re-fetch. + have to re-fetch. It also carries the rule's check (`verify_with`, + `expires_when`, `last_verified`) when it has one — a search hit is exactly + the moment someone is about to act on a rule, and "this asserts a fact + nobody has confirmed" is part of what the rule says. Rules are not project-scoped the way notes are (a family rule belongs to no project), so `project_id` and `system_id` do not apply here. @@ -39,6 +43,14 @@ async def _search_rules(uid: int, q: str, limit: int) -> dict: "tier": rule.tier, "why": rule.why or "", "how_to_apply": rule.how_to_apply or "", + "verify_with": rule.verify_with or "", + "expires_when": rule.expires_when or "", + # Only on a rule that carries a check; its absence means the + # rule is a decision, not that nobody has looked. + **( + {"last_verified": rulebooks_svc.last_verified_label(rule)} + if rule.verify_with else {} + ), "topic_id": rule.topic_id, "project_id": rule.project_id, "similarity": float(score), diff --git a/src/scribe/routes/rulebooks.py b/src/scribe/routes/rulebooks.py index 08d0d9e..e402584 100644 --- a/src/scribe/routes/rulebooks.py +++ b/src/scribe/routes/rulebooks.py @@ -165,6 +165,8 @@ async def create_rule(topic_id: int): when_to_apply=data.get("when_to_apply", ""), tier=data.get("tier", "always_on"), arose_from_id=data.get("arose_from_id", 0) or 0, + verify_with=data.get("verify_with", ""), + expires_when=data.get("expires_when", ""), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 @@ -191,8 +193,13 @@ async def update_rule(rule_id: int): fields = { k: v for k, v in data.items() if k in ("title", "statement", "why", "how_to_apply", "order_index", - "when_to_apply", "tier", "arose_from_id") + "when_to_apply", "tier", "arose_from_id", + "verify_with", "expires_when") } + # No clear_fields here: a form sends "" for an emptied input, and the + # service normalises "" to NULL for every nullable text column. The MCP + # door needs the explicit list only because "" already means "unchanged" + # there — two idioms, one outcome. rule = await rulebooks_svc.update_rule(rule_id, uid, **fields) if rule is None: return jsonify({"error": "rule not found"}), 404 @@ -375,6 +382,8 @@ async def create_project_rule(project_id: int): when_to_apply=data.get("when_to_apply", ""), tier=data.get("tier", "always_on"), arose_from_id=data.get("arose_from_id", 0) or 0, + verify_with=data.get("verify_with", ""), + expires_when=data.get("expires_when", ""), ) except ValueError as exc: return jsonify({"error": str(exc)}), 404 diff --git a/src/scribe/services/rulebooks.py b/src/scribe/services/rulebooks.py index aece902..a6c1c8b 100644 --- a/src/scribe/services/rulebooks.py +++ b/src/scribe/services/rulebooks.py @@ -8,6 +8,7 @@ depending on the caller's needs (mirroring services/events.py pattern). from __future__ import annotations import logging +from collections.abc import Iterable from typing import Optional from sqlalchemy import delete as sql_delete, insert, or_, select @@ -288,6 +289,17 @@ TIERS = ("always_on", "conditional") RELATION_KINDS = ("co_surfaces", "overrides", "elaborates") +# The rule columns that are nullable, and therefore the ones where EMPTY has +# to mean empty. A write that stores "" leaves a column that is not NULL and +# not content — `verify_with IS NOT NULL` would then be true for a rule with +# no check, and the staleness sweep would list rules it should never see. +# Normalising here, at the one service seam, is what makes "unset" a single +# state instead of two that read alike through to_dict's `or ""`. +NULLABLE_RULE_TEXT = ( + "why", "how_to_apply", "when_to_apply", "verify_with", "expires_when", +) + + def _valid_tier(tier: str) -> str: """An unrecognised tier falls back to always_on — the SAFE direction. @@ -299,6 +311,21 @@ def _valid_tier(tier: str) -> str: return tier if tier in TIERS else "always_on" +def last_verified_label(rule: Rule) -> str | None: + """How long ago the rule's check passed — None when it carries no check. + + One helper because two surfaces need the same answer and the brief-dict + lesson in rule_brief's docstring is what happens otherwise: three copies + that had already drifted. `None` means "this rule is a decision, the + question does not apply"; "never" means "it is a fact and nobody has + confirmed it" — a distinction worth keeping, because the second is the + one worth acting on. + """ + if not rule.verify_with: + return None + return rule.verified_at.date().isoformat() if rule.verified_at else "never" + + def rule_brief(rule: Rule, **extra) -> dict: """The shape a rule takes when it is SURFACED rather than opened. @@ -331,6 +358,16 @@ def rule_brief(rule: Rule, **extra) -> dict: out["when_to_apply"] = rule.when_to_apply if rule.arose_from_id: out["arose_from_id"] = rule.arose_from_id + # Present ONLY on a rule that carries a check — its presence is the + # signal, and it says two things at once: this rule asserts a fact that + # can go false, and here is how long ago anyone confirmed it. The check + # text itself stays in get_rule; a listing needs to know WHICH rules can + # rot, not how to test them. "never" rather than null, per #2483: a key + # that reads as an unused capability is a different claim from a rule + # nobody has ever verified. + stamp = last_verified_label(rule) + if stamp: + out["last_verified"] = stamp out.update({k: v for k, v in extra.items() if v is not None}) return out @@ -436,6 +473,7 @@ async def create_rule( topic_id: int, user_id: int, title: str, statement: str, why: str = "", how_to_apply: str = "", order_index: int = 0, when_to_apply: str = "", tier: str = "always_on", arose_from_id: int = 0, + verify_with: str = "", expires_when: str = "", ) -> Rule: async with async_session() as session: await _assert_topic_owned(session, topic_id, user_id) @@ -447,6 +485,8 @@ async def create_rule( tier=_valid_tier(tier), why=why or None, how_to_apply=how_to_apply or None, + verify_with=verify_with or None, + expires_when=expires_when or None, arose_from_id=arose_from_id or None, order_index=order_index, ) @@ -461,6 +501,7 @@ async def create_project_rule( project_id: int, user_id: int, title: str, statement: str, why: str = "", how_to_apply: str = "", order_index: int = 0, when_to_apply: str = "", tier: str = "always_on", arose_from_id: int = 0, + verify_with: str = "", expires_when: str = "", ) -> Rule: """Create a rule scoped to a single project (no rulebook ceremony). @@ -478,6 +519,8 @@ async def create_project_rule( tier=_valid_tier(tier), why=why or None, how_to_apply=how_to_apply or None, + verify_with=verify_with or None, + expires_when=expires_when or None, arose_from_id=arose_from_id or None, order_index=order_index, ) @@ -681,7 +724,23 @@ async def get_rule(rule_id: int, user_id: int) -> Optional[Rule]: return await _fetch_owned_rule(session, rule_id, user_id) -async def update_rule(rule_id: int, user_id: int, **fields) -> Optional[Rule]: +async def update_rule( + rule_id: int, user_id: int, clear: Iterable[str] = (), **fields, +) -> Optional[Rule]: + """Patch a rule. `clear` names fields to unset; **fields carries new values. + + Clearing is EXPLICIT and separate because a nullable field cannot be + emptied by passing it. The MCP door reads "" as "leave this alone" — an + agent filling three fields must not wipe the other five — so a caller + there has no value that means "remove it", and a rule that stops being a + constraint genuinely needs its check removed. Naming the field is the one + form that cannot happen by accident. + + Callers that DO have a meaningful empty value (the REST door, where a + cleared form input arrives as "") get the same outcome through + NULLABLE_RULE_TEXT normalisation below, so the two doors keep their own + idiom and agree about the result. + """ async with async_session() as session: rule = await _fetch_owned_rule(session, rule_id, user_id) if rule is None: @@ -689,10 +748,32 @@ async def update_rule(rule_id: int, user_id: int, **fields) -> Optional[Rule]: allowed = { "title", "statement", "why", "how_to_apply", "order_index", "when_to_apply", "tier", "arose_from_id", + "verify_with", "expires_when", } + check_before = rule.verify_with + for key in clear: + if key in allowed and key in NULLABLE_RULE_TEXT: + setattr(rule, key, None) + elif key == "arose_from_id": + setattr(rule, key, None) for key, value in fields.items(): - if key in allowed and value is not None: - setattr(rule, key, _valid_tier(value) if key == "tier" else value) + if key not in allowed or value is None: + continue + if key == "tier": + value = _valid_tier(value) + elif key in NULLABLE_RULE_TEXT: + value = value or None + elif key == "arose_from_id": + value = value or None + setattr(rule, key, value) + # A verification stamp certifies A CHECK, not a rule. Rewrite or + # remove the check and the old stamp certifies something that no + # longer exists — so it is dropped, and the rule re-enters the sweep. + # 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 the thing the sweep exists to catch. + if rule.verify_with != check_before: + rule.verified_at = None await session.commit() await session.refresh(rule) _refresh_rule_embedding(rule) diff --git a/tests/helpers.py b/tests/helpers.py index e263e61..df77475 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -159,6 +159,11 @@ def fake_rule(**attrs) -> MagicMock: # `when_to_apply` and `arose_from_id` would be truthy MagicMocks and # rule_brief would attach both keys on every stand-in. "when_to_apply": None, "tier": "always_on", "arose_from_id": None, + # Same reason, and the same trap one field further on: an unnamed + # `verify_with` is a truthy MagicMock, so every stand-in rule would + # claim to carry a check and rule_brief would stamp a MagicMock date + # onto all of them. Most rules have none — that is the default here. + "verify_with": None, "expires_when": None, "verified_at": None, "order_index": 0, "created_at": _now(), "updated_at": _now(), }, attrs) diff --git a/tests/test_integration_rule_verification.py b/tests/test_integration_rule_verification.py new file mode 100644 index 0000000..381a245 --- /dev/null +++ b/tests/test_integration_rule_verification.py @@ -0,0 +1,136 @@ +"""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") diff --git a/tests/test_services_rulebooks.py b/tests/test_services_rulebooks.py index b7de1ad..69aba3b 100644 --- a/tests/test_services_rulebooks.py +++ b/tests/test_services_rulebooks.py @@ -392,3 +392,57 @@ def test_an_unknown_tier_falls_back_to_binding(): assert _valid_tier("Conditional") == "always_on" assert _valid_tier("") == "always_on" assert _valid_tier("occasionally") == "always_on" + + +# ── verify_with / expires_when (milestone 312) ────────────────────────── + +def test_a_rule_with_no_check_says_nothing_about_verification(): + """The empty case is the COMMON case, and it must stay silent. + + Most rules are decisions: they have no truth value and there is nothing to + go and check. If a brief carried `last_verified` for those too, the signal + would be worthless — every rule would look like something someone ought to + be verifying, and the handful that genuinely rot would stop standing out. + """ + from scribe.services.rulebooks import last_verified_label, rule_brief + + rule = fake_rule() + assert last_verified_label(rule) is None + assert "last_verified" not in rule_brief(rule) + + +def test_an_unverified_constraint_reads_never_rather_than_null(): + """#2483 again: a null key reads as a capability going unused. "never" is + a different and much stronger claim — this rule asserts a fact about + someone else's software and nobody has ever confirmed it.""" + from scribe.services.rulebooks import last_verified_label, rule_brief + + rule = fake_rule(verify_with="cat CI-runner/renovate/config.js") + assert last_verified_label(rule) == "never" + assert rule_brief(rule)["last_verified"] == "never" + + +def test_a_verified_constraint_reports_the_date_it_was_checked(): + """A date, not a stamp — the question is "how old is this", the same call + rule_brief makes for updated_at.""" + from scribe.services.rulebooks import last_verified_label + + rule = fake_rule( + verify_with="cat CI-runner/renovate/config.js", + verified_at=datetime(2026, 8, 27, 11, 46, tzinfo=timezone.utc), + ) + assert last_verified_label(rule) == "2026-08-27" + + +def test_the_check_text_itself_never_enters_a_listing(): + """A listing says WHICH rules can rot, not how to test them. The check can + be a long command; multiplied across an always-on set it is the same bloat + `why` and `how_to_apply` are kept out of a brief to avoid.""" + from scribe.services.rulebooks import rule_brief + + out = rule_brief(fake_rule( + verify_with="a very long command " * 20, + expires_when="the runner learns a new shell", + )) + assert "verify_with" not in out + assert "expires_when" not in out