From 5c9bb40777621882017bccd9c22a01a8af902cf7 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 30 Aug 2026 13:02:00 -0400 Subject: [PATCH] feat(rules): a session is told when its rules move under it (#3244, milestone 323 step 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rules payload carries a marker; the write-path hook hands it back; the server says which rules moved. Nothing is said when nothing moved. THE COUNT IS NOT DECORATION. max(updated_at) alone cannot see a DELETED rule — it moves no timestamp — and that is the single change that takes an instruction OUT of force, which is the one a session most needs to hear about. The marker is `|`, and a deletion is reported through the count because there is no row left to name. THE HOOK IS THE CARRIER because it already fires before a write, which is the moment acting on a stale rule costs something. One comparison, no payload, no extra round trip. WHERE THE MARKER IS CAPTURED, and it could not be anywhere else: the SessionStart hook, from /api/plugin/context. The model also receives one from list_always_on_rules, but a hook cannot see an MCP tool's result — so the value the write path compares has to be stored where a shell script can reach it. Keyed by session id in the state dir the prior-art hook already uses, so "changed since" means since THIS session loaded its rules. NOT ON rules_payload, against the task's letter. Those are applicable_rules — a different, subscription-derived set. One key name over two sets is how a comparison starts reporting phantom changes, and the write path compares against the always-on set. WHAT IT CANNOT SEE is stated in both the service and the write-path arm as a table, because a reader who finds an etag will assume it covers staleness generally: another session edits a rule mid-flight | caught the session is misremembering a rule read hours ago | caught compaction summarised the rules out of context | NOT caught The third is the most common, and the marker is blind to it — the etag was in context too and went with the rules. The SessionStart nudge is that case's only mechanism and must not be softened because this shipped. A test asserts both modules still explain that. Instance-agnostic (rule 115): an install with no rules produces a stable marker rather than an error, and "no rules" reads as a state rather than as a change. An unreadable or absent marker reports nothing — a signal that cries wolf is worse than none, because it trains a reader to skip the line that will one day be true. The arm fails open like every other arm on this hook. The delivery is tested through the real build_write_path_hint rather than the helper alone: the feature IS a line arriving in a session, and the arithmetic being right proves nothing about that. Live acceptance is deploy-gated and not yet recorded on the task. Co-Authored-By: Claude Opus 5 --- plugin/hooks/scribe_after_write.sh | 12 +- plugin/hooks/scribe_session_context.sh | 22 +++ src/scribe/mcp/tools/rulebooks.py | 11 +- src/scribe/routes/plugin.py | 7 + src/scribe/services/plugin_context.py | 73 +++++++- src/scribe/services/rulebooks.py | 84 +++++++++ tests/test_rules_etag.py | 226 +++++++++++++++++++++++++ 7 files changed, 431 insertions(+), 4 deletions(-) create mode 100644 tests/test_rules_etag.py diff --git a/plugin/hooks/scribe_after_write.sh b/plugin/hooks/scribe_after_write.sh index 52dcfd4..10857d2 100644 --- a/plugin/hooks/scribe_after_write.sh +++ b/plugin/hooks/scribe_after_write.sh @@ -175,6 +175,16 @@ while IFS= read -r rel_path; do derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | jq -sRr '@uri' 2>/dev/null) || derive_seen="" [ -n "$derive_seen" ] && derive_exclude_q="&exclude_derive=${derive_seen}" fi + # The rules marker the SessionStart hook stored, handed back so the server + # can say whether those rules moved since (milestone 323). Nothing stored + # means nothing sent, which the server reads as silence rather than as a + # mismatch — an install that never reached /api/plugin/context must not + # start claiming its rules changed. + etag_q="" + if [ -f "$state_dir/${safe_sid}.rules_etag" ]; then + held=$(jq -sRr '@uri' < "$state_dir/${safe_sid}.rules_etag" 2>/dev/null) || held="" + [ -n "$held" ] && etag_q="&rules_etag=${held}" + fi if [ -n "$path_enc" ]; then # 8s, not the pre-write hook's 5: this hook runs AFTER the tool, so it # gates nothing the session is waiting on, and the first prior-art call @@ -184,7 +194,7 @@ while IFS= read -r rel_path; do reached=1 body=$(curl -fsS --max-time 8 \ -H "Authorization: Bearer ${token}" \ - "${url%/}/api/plugin/prior-art?path=${path_enc}&code=${code_enc}${repo_q}${exclude_q}${sync_exclude_q}${derive_exclude_q}${shapes_q}" 2>/dev/null) || { body=""; reached=0; } + "${url%/}/api/plugin/prior-art?path=${path_enc}&code=${code_enc}${repo_q}${exclude_q}${sync_exclude_q}${derive_exclude_q}${shapes_q}${etag_q}" 2>/dev/null) || { body=""; reached=0; } # A call that was owed and didn't come back is said, once per outage # (#2932) — shared marker with the pre-write hook, so one outage is one # line however the code was written. diff --git a/plugin/hooks/scribe_session_context.sh b/plugin/hooks/scribe_session_context.sh index 04aad2a..77b2ab3 100755 --- a/plugin/hooks/scribe_session_context.sh +++ b/plugin/hooks/scribe_session_context.sh @@ -109,6 +109,28 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then -H "Authorization: Bearer ${token}" \ "${url%/}/api/plugin/context${q}" 2>/dev/null) || body="" [ -n "$body" ] && dyn=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) + # Stash the rules marker for the write-path hook (milestone 323). THIS is + # where it has to be captured: the model receives one from + # list_always_on_rules too, but a hook cannot see an MCP tool's result. Stored + # under the same state dir the prior-art hook already uses, keyed by session, + # so "changed since" means since THIS session loaded its rules. + # + # Written on `compact` as well as `startup`, and that is correct rather than + # convenient: a compact tells the session to re-pull its rules, so the marker + # should describe the set it is about to hold. It is also why this cannot + # cover the compaction case — see the table in services/plugin_context.py. + if [ -n "$body" ]; then + etag=$(printf '%s' "$body" | jq -r '.rules_etag // empty' 2>/dev/null) || etag="" + if [ -n "$etag" ]; then + sid=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || sid="" + safe_sid=$(printf '%s' "${sid:-nosession}" | tr -c 'A-Za-z0-9._-' '_') + etag_dir="${TMPDIR:-/tmp}/scribe-priorart" + # Best-effort throughout: a marker that cannot be stored costs a hint, + # never the session. + mkdir -p "$etag_dir" 2>/dev/null \ + && printf '%s' "$etag" > "$etag_dir/${safe_sid}.rules_etag" 2>/dev/null || true + fi + fi [ -z "$dyn" ] && status="> ⚠️ Scribe: live rules/project context could not be loaded this session (instance unreachable or request failed). The standing guidance above still applies — pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\` as needed." elif [ -n "$url" ] && [ -z "$token" ]; then status="> ⚠️ Scribe: live context disabled this session — the API key is not configured (Scribe base URL is). Set it with \`/plugin\` → Scribe → configure, or export SCRIBE_TOKEN. Tools still work; pull rules with \`list_always_on_rules()\` and project context with \`enter_project()\`." diff --git a/src/scribe/mcp/tools/rulebooks.py b/src/scribe/mcp/tools/rulebooks.py index 9795b79..cd040d7 100644 --- a/src/scribe/mcp/tools/rulebooks.py +++ b/src/scribe/mcp/tools/rulebooks.py @@ -262,7 +262,16 @@ async def list_always_on_rules(project_id: int = 0) -> dict: """ uid = current_user_id() rules = await rulebooks_svc.list_always_on_rules(uid, project_id=project_id) - return {"rules": [_rule_summary(r) for r in rules], "total": len(rules)} + return { + "rules": [_rule_summary(r) for r in rules], + "total": len(rules), + # A marker for the set you are now holding. It is not for you to read: + # the write-path hook carries it back and is told if these rules have + # moved since. Deliberately NOT on rules_payload's applicable_rules — + # that is a DIFFERENT set (subscription-derived), and one key name + # over two sets is how a comparison starts reporting phantom changes. + "rules_etag": rulebooks_svc.rules_etag(rules), + } async def get_rule(rule_id: int) -> dict: diff --git a/src/scribe/routes/plugin.py b/src/scribe/routes/plugin.py index 14e9d9b..b951413 100644 --- a/src/scribe/routes/plugin.py +++ b/src/scribe/routes/plugin.py @@ -138,6 +138,11 @@ async def write_path_prior_art(): or `canon:`) already named this session by the ledger arm (#2900); its own channel, like the two above. + rules_etag (opt) — the marker the session was given when it loaded + its always-on rules (milestone 323). Sent back + so the server can say whether those rules have + MOVED since. Absent means the hook has nothing + stored, which is silence, not a mismatch. shapes (opt) — comma-separated `kind:name` definitions the hook found in (or enclosing) the payload, kind being css|sym. The shape ledger's write-path feed @@ -157,6 +162,7 @@ async def write_path_prior_art(): p.strip() for p in (request.args.get("exclude_derive") or "").split(",") if p.strip() ] exclude_rule_ids = _int_list(request.args.get("exclude_rule_ids")) + rules_etag = (request.args.get("rules_etag") or "").strip() shapes = _parse_shapes(request.args.get("shapes") or "") api_key = getattr(g, "api_key", None) may_stamp = api_key is None or getattr(api_key, "scope", "") == "write" @@ -168,6 +174,7 @@ async def write_path_prior_art(): repo_key=repo_bindings_svc.normalize_repo_key(repo) if repo else "", exclude_derive=exclude_derive, exclude_rule_ids=exclude_rule_ids, + rules_etag=rules_etag, ) return jsonify(result) diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 2759e84..5491de4 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -708,6 +708,7 @@ async def build_write_path_hint( repo_key: str = "", exclude_derive: list[str] | None = None, exclude_rule_ids: list[int] | None = None, + rules_etag: str = "", ) -> dict: """Prior-art hint for the plugin's PreToolUse hook on Write/Edit. @@ -1038,6 +1039,63 @@ async def build_write_path_hint( for arm, ids in by_arm.items(): record_surfaced(user_id=user_id, note_ids=ids, source=arm) + # ── Have the rules moved under this session? (milestone 323) ─────── + # + # THE CARRIER IS THE POINT. This hook already fires before a write — the + # moment acting on a stale rule actually costs something — and the check + # is one comparison against a marker the session already holds. No + # payload, no extra round trip, and nothing said when nothing moved. + # + # WHAT THIS CANNOT SEE, and a reader who finds an etag here will assume + # otherwise: + # + # what goes wrong | caught? + # ---------------------------------------------------|-------- + # another session edits a rule mid-flight | yes + # the session is misremembering a rule read hours ago | yes + # compaction summarised the rules out of context | NO + # + # The third is the most common and this is blind to it: the etag was in + # context too and went with the rules. The SessionStart nudge is that + # case's only mechanism and must not be softened because this shipped. + # + # Fails open, like every other arm here: a staleness hint must never + # break a write. + if rules_etag: + try: + current = await rulebooks_svc.list_always_on_rules( + user_id, project_id=project_id or 0, + ) + if rulebooks_svc.rules_etag(current) != rules_etag: + moved = rulebooks_svc.rules_moved_since(current, rules_etag) + held = rulebooks_svc.etag_count(rules_etag) + bits = [] + if moved: + named = ", ".join( + f"#{r.id} \u201c{r.title}\u201d" for r in moved[:3] + ) + more = len(moved) - 3 + bits.append( + f"{named}" + (f", and {more} more" if more > 0 else "") + ) + # A DELETED rule moves no timestamp and leaves no row to name, + # so the count is the only thing that can report the one change + # that takes an instruction OUT of force. + if held is not None and held != len(current): + delta = len(current) - held + bits.append( + f"{abs(delta)} rule(s) {'added' if delta > 0 else 'no longer in force'}" + ) + if bits: + lines.append( + "Your loaded rules have changed since this session " + "started — " + "; ".join(bits) + ". Re-read them with " + "list_always_on_rules() before relying on the set you " + "are holding." + ) + except Exception: + logger.debug("write-path rules-etag arm failed", exc_info=True) + # ── Standing rules that may apply here (milestone 307) ────────────── # # A SUGGESTION, not a binding surface, and the distinction is the design @@ -1208,7 +1266,10 @@ async def build_session_context( its normalized key — triggers a one-line "bind this repo" hint so the binding is self-healing. - Returns {"context": str, "rule_count": int, "project": dict | None}. + Returns {"context": str, "rule_count": int, "project": dict | None, + "rules_etag": str}. The etag is for the HOOK, not for the model — the + hook stores it and hands it back on each write so the server can say + whether these rules have moved since the session loaded them. `context` is markdown ready to drop into `additionalContext`; it is capped at _MAX_CHARS with an explicit truncation note so the hook can pass it through verbatim. @@ -1317,4 +1378,12 @@ async def build_session_context( if len(context) > _MAX_CHARS: context = context[:_MAX_CHARS].rstrip() + "\n\n…(truncated — call list_always_on_rules())" - return {"context": context, "rule_count": len(rules), "project": project_dict} + return { + "context": context, + "rule_count": len(rules), + "project": project_dict, + # Computed from the rules THIS payload was built from, not re-queried: + # the marker has to describe the set the session is actually holding, + # and a second query could disagree with the first. + "rules_etag": rulebooks_svc.rules_etag(rules), + } diff --git a/src/scribe/services/rulebooks.py b/src/scribe/services/rulebooks.py index f5c9152..b133250 100644 --- a/src/scribe/services/rulebooks.py +++ b/src/scribe/services/rulebooks.py @@ -9,6 +9,7 @@ from __future__ import annotations import logging from collections.abc import Iterable +from datetime import datetime from typing import Optional from sqlalchemy import and_, delete as sql_delete, insert, or_, select @@ -1413,6 +1414,89 @@ def rules_payload(applicable: dict) -> dict: } +# ── The staleness marker (milestone 323 step 5) ──────────────────────── +# +# WHAT THIS CAN AND CANNOT SEE. An etag catches a rule that MOVED after a +# session loaded it. It is not a general staleness check, and a reader who +# finds one here will assume it is: +# +# what goes wrong | caught? +# ---------------------------------------------------|-------- +# another session edits a rule mid-flight | yes +# the session is misremembering a rule read hours ago | yes +# compaction summarised the rules out of context | NO +# +# The third is the most common and the marker is blind to it, because the +# etag was in context too and went with the rules. The SessionStart nudge is +# that case's only mechanism, and MUST NOT be softened because this exists — +# retiring something that covers the common case in favour of something that +# does not is the plausible mistake here. + +_ETAG_EMPTY = "empty|0" + + +def rules_etag(rules: list) -> str: + """A marker for "is the set you are holding still the current one?". + + `max(updated_at)` alone is not enough: DELETING a rule moves no timestamp, + and that is the single change that takes an instruction OUT of force — + the one a session most needs to hear about. The count catches it. + + Instance-agnostic (rule 115): it knows nothing about any particular + rulebook, and an install with one rule or none produces a stable marker + rather than an error. "No rules" must read as a state, not as a change, + or every session on a fresh install would be told its rules had moved. + """ + if not rules: + return _ETAG_EMPTY + newest = max(r.updated_at for r in rules) + return f"{newest.isoformat()}|{len(rules)}" + + +async def rules_etag_for(user_id: int, project_id: int = 0) -> str: + """The current marker for the set a session at this scope would hold. + + Deliberately built from `list_always_on_rules` rather than from a + `max()/count()` aggregate. An aggregate would be cheaper, and would have + to restate that function's definition of the set — the always_on flag, + the project's inception exclusions, the tier filter. Two definitions of + "the session's rules" is how the marker starts disagreeing with the + rules, which is worse than materialising a few dozen rows. + """ + rules = await list_always_on_rules(user_id, project_id=project_id) + return rules_etag(rules) + + +def rules_moved_since(rules: list, held_etag: str) -> list: + """The rules whose text changed after `held_etag` was issued. + + Returns [] when the marker matches, is unparseable, or is absent — a + caller cannot act on "something is different but I cannot say what", and + a garbled marker must not be reported as a change. + + A count difference is real news that this list cannot show: a rule + DELETED since the marker was issued has no row left to return. Callers + compare counts separately. + """ + if not held_etag or held_etag == _ETAG_EMPTY: + return [] + stamp, _, _count = held_etag.partition("|") + try: + held_at = datetime.fromisoformat(stamp) + except ValueError: + return [] + return [r for r in rules if r.updated_at and r.updated_at > held_at] + + +def etag_count(held_etag: str) -> int | None: + """How many rules the holder had. None when the marker cannot be read.""" + _stamp, _, count = (held_etag or "").partition("|") + try: + return int(count) + except ValueError: + return None + + # ── The staleness sweep (milestone 312) ──────────────────────────────── async def rules_due_for_verification( diff --git a/tests/test_rules_etag.py b/tests/test_rules_etag.py new file mode 100644 index 0000000..cfa55ab --- /dev/null +++ b/tests/test_rules_etag.py @@ -0,0 +1,226 @@ +"""The staleness marker on the rules payload (milestone 323 step 5). + +WHAT THE MARKER IS FOR: telling a session that the rules it is holding have +MOVED since it loaded them. Not general staleness — the limitation is stated +in services/rulebooks.py and in the write-path arm, and two tests here pin the +cases that would otherwise be quietly lost. + +The two that matter most are both about NOT crying wolf. A marker that reports +a change when nothing changed gets ignored within a day, and an ignored +staleness signal is worse than none: it trains a reader to skip the line that +will one day be true. +""" +from datetime import datetime, timedelta, timezone +from types import SimpleNamespace +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from scribe.services import rulebooks as svc + +NOW = datetime(2026, 8, 30, 12, 0, tzinfo=timezone.utc) +LATER = NOW + timedelta(hours=1) + + +def _rule(updated_at=NOW, rid=1, title="a rule"): + """A Rule-shaped stand-in. The marker only ever reads three attributes, + and a real model would need a session to build.""" + return SimpleNamespace(id=rid, title=title, updated_at=updated_at) + + +def test_the_same_set_produces_the_same_marker(): + """The whole mechanism rests on this. If the marker moved on its own, every + write would report a change and the line would be noise by lunchtime.""" + rules = [_rule(rid=1), _rule(rid=2, updated_at=LATER)] + assert svc.rules_etag(rules) == svc.rules_etag(list(reversed(rules))), ( + "the marker depends on the ORDER rules come back in, so any query " + "whose sort changes would look like an edit" + ) + + +def test_an_edit_moves_the_marker(): + before = svc.rules_etag([_rule(rid=1), _rule(rid=2)]) + after = svc.rules_etag([_rule(rid=1), _rule(rid=2, updated_at=LATER)]) + assert before != after + + +def test_a_DELETED_rule_moves_the_marker(): + """THE CASE max(updated_at) ALONE CANNOT SEE, and the reason the count is + in there. Deleting a rule moves no timestamp — and it is the single change + that takes an instruction OUT of force, which is the one a session most + needs to hear about.""" + before = svc.rules_etag([_rule(rid=1), _rule(rid=2)]) + after = svc.rules_etag([_rule(rid=1)]) + assert before != after, ( + "a deleted rule left the marker unchanged — the count is missing, and " + "the session would keep obeying an instruction that no longer exists" + ) + + +def test_no_rules_is_a_state_not_a_change(): + """Rule 115: this has to behave on an install with no rules at all. `max()` + over an empty set raises; a marker that raised would take the whole write + path's hint down, and one that varied would tell every session on a fresh + install that its rules had changed.""" + assert svc.rules_etag([]) == svc.rules_etag([]) + assert svc.rules_etag([]) != svc.rules_etag([_rule()]) + + +def test_moved_since_names_only_what_actually_moved(): + held = svc.rules_etag([_rule(rid=1), _rule(rid=2)]) + current = [_rule(rid=1), _rule(rid=2, updated_at=LATER, title="reworded")] + moved = svc.rules_moved_since(current, held) + assert [r.id for r in moved] == [2] + + +def test_a_matching_marker_names_nothing(): + rules = [_rule(rid=1), _rule(rid=2)] + assert svc.rules_moved_since(rules, svc.rules_etag(rules)) == [] + + +def test_an_unreadable_marker_reports_no_change(): + """A caller cannot act on "something differs but I cannot say what", and a + garbled marker must never be rendered as a change — that is the shape of a + signal that gets ignored.""" + assert svc.rules_moved_since([_rule(updated_at=LATER)], "not-an-etag") == [] + assert svc.rules_moved_since([_rule(updated_at=LATER)], "") == [] + assert svc.etag_count("garbled") is None + + +def test_the_empty_marker_reports_no_change(): + """An install that had no rules and now has some: the count says so, and + this function has no timestamp to reason from. Silence here, not a claim.""" + assert svc.rules_moved_since([_rule()], svc.rules_etag([])) == [] + + +def test_the_count_survives_the_round_trip(): + assert svc.etag_count(svc.rules_etag([_rule(rid=1), _rule(rid=2)])) == 2 + assert svc.etag_count(svc.rules_etag([])) == 0 + + +def test_the_limitation_is_stated_where_a_reader_will_be(): + """A future reader who finds an etag will assume it covers staleness + generally. It does not — it is blind to compaction, which is the most + common case — and the SessionStart nudge is that case's only mechanism. + + Pinned because the plausible mistake is retiring a nudge that works on the + strength of a signal that does not cover it, and the comment is the only + thing standing in the way. + """ + import inspect + + from scribe.services import plugin_context + + for module in (svc, plugin_context): + src = inspect.getsource(module).lower() + assert "compaction" in src and "etag" in src, ( + f"{module.__name__} no longer explains what the rules marker " + f"cannot see. Without it the next reader will treat an etag as a " + f"general staleness check and soften the SessionStart nudge." + ) + + +# ── The arm that delivers the message (milestone 323 step 5) ─────────── +# +# The marker is worth nothing until a session is actually TOLD. These drive +# the real `build_write_path_hint`, because the feature IS a line arriving in +# a hook's output — a test of the helper alone would prove the arithmetic and +# nothing about the delivery. + + +def _quiet_write_path(pc, rules): + """Every other arm stubbed to silent, so the only line that can appear is + the one under test.""" + return ( + patch.object(pc, "get_writepath_config", + AsyncMock(return_value={"enabled": True, "threshold": 0.6, + "top_k": 3})), + patch.object(pc.snippets_svc, "list_snippets", + AsyncMock(return_value=([], 0))), + patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), + patch.object(pc, "semantic_search_rules", AsyncMock(return_value=[])), + patch.object(pc, "record_retrieval", MagicMock()), + patch.object(pc, "record_surfaced", MagicMock()), + patch.object(pc, "owner_names_for", AsyncMock(return_value={})), + patch.object(pc.rulebooks_svc, "list_always_on_rules", + AsyncMock(return_value=rules)), + ) + + +async def _hint(rules, held_etag): + from scribe.services import plugin_context as pc + + patches = _quiet_write_path(pc, rules) + for p in patches: + p.start() + try: + out = await pc.build_write_path_hint( + 1, "src/scribe/services/rulebooks.py", code="x" * 400, + rules_etag=held_etag, + ) + finally: + for p in patches: + p.stop() + return out["context"] + + +@pytest.mark.asyncio +async def test_a_session_is_told_which_rule_moved(): + """The delivery, end to end through the real hint builder. Naming the rule + is the point — "something changed" sends the reader to re-read everything, + which is the cost the marker was meant to avoid.""" + held = svc.rules_etag([_rule(rid=1), _rule(rid=2, title="dev is home")]) + current = [_rule(rid=1), _rule(rid=2, title="dev is home", updated_at=LATER)] + + ctx = await _hint(current, held) + assert "changed since this session started" in ctx + assert "#2" in ctx and "dev is home" in ctx + + +@pytest.mark.asyncio +async def test_a_session_holding_the_current_rules_is_told_nothing(): + """The one that keeps the signal worth reading. A line on every write is a + line nobody reads.""" + rules = [_rule(rid=1), _rule(rid=2)] + ctx = await _hint(rules, svc.rules_etag(rules)) + assert "changed since this session started" not in ctx + + +@pytest.mark.asyncio +async def test_a_session_that_sent_no_marker_is_told_nothing(): + """An install whose hook never reached /api/plugin/context has nothing + stored. Absent must read as silence, not as a mismatch — otherwise the + first thing a new install hears is that its rules changed.""" + ctx = await _hint([_rule(updated_at=LATER)], "") + assert "changed since this session started" not in ctx + + +@pytest.mark.asyncio +async def test_a_DELETED_rule_is_reported_even_though_it_has_no_row(): + """The count arm. A deleted rule leaves nothing to name, and it is the + change that takes an instruction OUT of force — so "no longer in force" + has to be sayable without a row to say it about.""" + held = svc.rules_etag([_rule(rid=1), _rule(rid=2)]) + ctx = await _hint([_rule(rid=1)], held) + assert "no longer in force" in ctx + + +@pytest.mark.asyncio +async def test_the_arm_fails_open(): + """A staleness hint must never break a write. Every other arm here fails + open for the same reason, and this one runs a query that can fail.""" + from scribe.services import plugin_context as pc + + patches = _quiet_write_path(pc, []) + for p in patches: + p.start() + try: + with patch.object(pc.rulebooks_svc, "list_always_on_rules", + AsyncMock(side_effect=RuntimeError("database down"))): + out = await pc.build_write_path_hint( + 1, "src/x.py", code="x" * 400, rules_etag="2026-01-01T00:00:00+00:00|3", + ) + finally: + for p in patches: + p.stop() + assert "changed since this session started" not in out["context"]