diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index 587aba5..4bbe45b 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -87,6 +87,7 @@ const kbWritePathEnabled = ref(true); // unrelated code through (#2223). Shares top-k, not the threshold. const kbWritePathThreshold = ref("0.68"); const kbRuleHintThreshold = ref("0.72"); +const kbCheckpointThreshold = ref("0.80"); // The two ACT arms no longer share a bar (#3853). A write-path query is a code // payload; a pre-tool query is a shell command, often under a dozen words — // less text, less signal, lower scores for the same relevance. Measured at one @@ -275,6 +276,10 @@ async function saveKbInject() { // Bash call, so a fallback of 0 would put a rule in front of every command. const trT = Math.min(1, Math.max(0, Number(kbToolRuleThreshold.value) || 0.68)); const prT = Math.min(1, Math.max(0, Number(kbPromptRuleThreshold.value) || 0.72)); + // The checkpoint bar, and the `|| default` guard matters most here of all: + // this is the only number that can STOP a call, so a fallback of 0 would + // hold the first command of every session behind whatever ranked first. + const cpT = Math.min(1, Math.max(0, Number(kbCheckpointThreshold.value) || 0.8)); const rpT = Math.min(1, Math.max(0, Number(kbReportPrefThreshold.value) || 0.72)); // The budgets, clamped the way the server clamps them: a whole number in // [1, 10]. Never 0 — an arm turned off is turned off by its switch, and a @@ -300,6 +305,7 @@ async function saveKbInject() { kbPlanMatchThreshold.value = String(planT); kbWritePathThreshold.value = String(wpT); kbRuleHintThreshold.value = String(rhT); + kbCheckpointThreshold.value = String(cpT); kbToolRuleThreshold.value = String(trT); kbPromptRuleThreshold.value = String(prT); kbReportPrefThreshold.value = String(rpT); @@ -319,6 +325,12 @@ async function saveKbInject() { // in services/plugin_context.py for why rules cannot share the // code threshold any more than code could share the prose one. kb_rulehint_threshold: String(rhT), + // The one bar that stops a call rather than annotating it. Its own key, + // never derived from the two rule bars above: it answers a different + // question of the same scores — not "is this worth showing" but "is the + // corpus confident enough to be read first" — and a number that moves + // when another moves is a number nobody can reason about. + kb_checkpoint_threshold: String(cpT), // A FOURTH and FIFTH bar, and they are separate keys on purpose: the // whole finding of #3853 is that one number cannot serve arms whose // queries are different shapes. Moving one must not move the others. @@ -790,6 +802,9 @@ onMounted(async () => { kbInjectTopK.value = allSettings.kb_autoinject_top_k; } kbWritePathEnabled.value = allSettings.kb_writepath_enabled !== "false"; + if (allSettings.kb_checkpoint_threshold !== undefined) { + kbCheckpointThreshold.value = allSettings.kb_checkpoint_threshold; + } if (allSettings.kb_rulehint_threshold !== undefined) { kbRuleHintThreshold.value = allSettings.kb_rulehint_threshold; } @@ -1723,6 +1738,32 @@ async function deleteUser(userId: number) { />

How many standing rules one edit may be shown (1–10).

+
+ + +

+ Every hint above arrives beside the result of the action it + was about — useful to read afterwards, too late to change what the + action was. Above this bar a standing rule is instead put + in front of the command: Claude is asked to read the rule + first, and then runs the command anyway if the rule does not apply. + Set well above the thresholds above, because it costs a round trip + rather than a line. Only standing rules can hold a command, never + preferences; only a rule this session has not already read; and + never more than once per rule or five times per session, so a bar + set too low is a chatty session rather than a stuck one. Commands + only — file edits are never held. +

+
/dev/null && return 1 + n=$(grep -c '^[0-9][0-9]*$' "$f" 2>/dev/null || printf '0') + # `grep -c` over a missing file can print nothing; a bare arithmetic test + # on an empty string is a syntax error in some shells and silently true in + # others, which is how a cap comes to cap nothing (#3191's shape). + n=$(printf '%s' "$n" | tr -cd '0-9') + [ -n "$n" ] || n=0 + [ "$n" -ge "$_SCRIBE_CHECKPOINT_CAP" ] && return 1 + fi + printf '%s\n' "$id" >> "$f" 2>/dev/null || return 1 + return 0 +} + +scribe_json_deny() { + # $1 hook event name, $2 the reason the agent reads INSTEAD of the result. + # + # The one place this plugin emits a decision on a tool call. `deny` returns + # the reason to the MODEL and the call does not run — it is not a prompt to + # the operator, costs them nothing, and is undone by the model simply + # submitting the call again. That is the whole difference from `ask`, which + # would hand a judgement that is the agent's to make to the person who asked + # for the work. + local esc + esc=$(printf '%s' "$2" | scribe_json_escape) || return 0 + printf '{"hookSpecificOutput":{"hookEventName":"%s","permissionDecision":"deny","permissionDecisionReason":"%s"}}\n' \ + "$1" "$esc" +} + scribe_held_query() { local ids ids=$(scribe_rules_live "$1") diff --git a/plugin/hooks/scribe_tool_rules.sh b/plugin/hooks/scribe_tool_rules.sh index 44d1a4d..0556393 100644 --- a/plugin/hooks/scribe_tool_rules.sh +++ b/plugin/hooks/scribe_tool_rules.sh @@ -102,12 +102,46 @@ body=$(curl -fsS --max-time 5 \ body_flat=$(printf '%s' "$body" | scribe_json_flat) context=$(scribe_json_pick "$body_flat" '.context') -[ -n "$context" ] || exit 0 -# Remember what was named so it is not repeated this session. +# Remember what was named so it is not repeated this session. BEFORE the +# checkpoint branch and before the empty-context exit: the ledger records what +# the server chose to surface, which happened whichever way this hook then +# renders it. Doing it inside one branch is how the two arms' ledgers came to +# disagree once already. if [ -n "$rulefile" ]; then scribe_json_list "$body_flat" '.rule_ids' | scribe_rules_append "$rulefile" fi +# ── The pre-act checkpoint (#4214, milestone 419) ──────────────────────── +# +# WHY THIS ARM AND NOT THE WRITE PATH. scribe_prior_art.sh carries an explicit, +# tested property that it never returns a permissionDecision — a recall aid may +# not stand in the way of a write, which is the operator's decision and is +# guarded by test_hook_never_returns_a_permission_decision. No equivalent +# decision covers this arm, and the misses that motivated the milestone on the +# command side are the ones a stop actually reaches: a commit message asserting +# what CI said, a verification script that checks nothing. +# +# WHAT THE STOP IS FOR. Everything else this plugin emits is additionalContext, +# which Claude Code delivers alongside the tool RESULT — so the rule is read +# after the call is written and reads as commentary on a decision already made. +# That is milestone 419's central finding. A deny returns the reason to the +# model with the call unrun, so the rule's own text can be read before the act +# exists. It costs the operator nothing: no prompt reaches them, and the model +# clears it by reading one record and re-submitting. +# +# It cannot recur. `scribe_checkpoint_allowed` holds at most one act per rule +# and at most five per session, so the worst case of a mis-set floor is a noisy +# session rather than one that cannot move. +cp_rule=$(scribe_json_pick "$body_flat" '.checkpoint.rule_id') +cp_reason=$(scribe_json_pick "$body_flat" '.checkpoint.reason') +if [ -n "$cp_rule" ] && [ -n "$cp_reason" ] && [ -n "$session_id" ]; then + if scribe_checkpoint_allowed "$state_dir/${safe_sid}.checkpoint.ids" "$cp_rule"; then + scribe_json_deny PreToolUse "$cp_reason" + exit 0 + fi +fi + +[ -n "$context" ] || exit 0 scribe_json_out PreToolUse "$context" exit 0 diff --git a/src/scribe/routes/plugin.py b/src/scribe/routes/plugin.py index d5d58c0..0d63961 100644 --- a/src/scribe/routes/plugin.py +++ b/src/scribe/routes/plugin.py @@ -170,6 +170,24 @@ async def pre_tool_rules(): different claims about the reader's context, so they get different lines (#4100). + + Returns `context`, `rule_ids`, and `checkpoint` (#4214, milestone 419). + + `checkpoint` IS THE ONE PART OF THIS RESPONSE THAT IS NOT A HINT. It is + empty on almost every call. When present it carries `rule_id`, `title`, + `trigger`, `score` and a rendered `reason`, and it means the hook should + DENY the call rather than annotate it — because every other line this + endpoint returns is delivered by Claude Code alongside the tool RESULT, + so it reaches the reader after the act is composed and reads as + commentary on a decision already made. + + It is raised only for a rule (never a preference, which claims no such + force), only for the ranker's top hit, only above the checkpoint + threshold — well above this arm's own floor — and only when the session + has NOT opened that rule. The remedy is one `get_rule` call, and the act + may then be re-submitted unchanged; the hook caps stops at one per rule + and five per session so a mis-set floor degrades to noise rather than to + a session that cannot proceed. """ tool = (request.args.get("tool") or "tool").strip() command = request.args.get("command") or "" @@ -223,6 +241,14 @@ async def write_path_prior_art(): or `canon:`) already named this session by the ledger arm (#2900); its own channel, like the two above. + (Returns a `checkpoint` block on the same contract as /tool-rules — + see that endpoint. The write-path HOOK deliberately does not act on + it: scribe_prior_art.sh carries a tested property that it never + returns a permissionDecision, on the operator's decision that a recall + aid may not stand in the way of a write. The block is computed and + returned so that decision can be revisited with evidence rather than + re-argued, and so a change of mind is a hook edit and not a feature.) + 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 diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index f9d048f..5f7ed2b 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -271,6 +271,66 @@ RULEHINT_LIMIT = SURFACES["write_path_rule"].budget_default # stops working and the answer is a reranker (#1038), not a smaller number. _RULEHINT_BAND = 0.05 +# ── The pre-act checkpoint (#4214, milestone 419) ─────────────────────── +# +# WHAT A CHECKPOINT IS, AND WHY IT IS NOT A LOUDER HINT. Every arm above +# appends text to an act the agent has already composed. Claude Code delivers +# `additionalContext` alongside the tool result, so by the time the line is +# read the call is written and the rule reads as commentary on a decision +# already made. That is milestone 419's central finding, measured over a +# session where three misses were caught by the operator and none by this +# system. +# +# A checkpoint is the same retrieval, spent differently: the hook returns a +# `deny` and the act does not run, so the rule's own text is read BEFORE the +# call exists. The remedy is `get_rule(id)` and nothing else — which is why +# the condition below is "the session has not opened it" rather than "the +# session has not recorded an outcome for it". An outcome can be satisfied +# with one cheap call that asserts compliance without producing any, and a +# checkpoint that can be dismissed that way manufactures exactly the +# compliance data step 2 exists to measure. Reading a rule cannot be faked in +# that direction: after `get_rule` the statement is in context, which is the +# whole of what was wanted. +# +# WHAT "CONSEQUENTIAL" MEANS HERE, AND WHY IT IS NOT A LIST. The obvious +# implementation enumerates act kinds — a write to product code, a schema +# change, a bulk classification, a merge. Every one of those is consequential +# because THIS operator wrote rules about it, and a shipped list would be this +# instance's corpus hard-coded into the product (rule 115). So the corpus +# decides: an act is consequential when the install's own rules speak to it +# with high confidence. A fresh install with no rules never stops anything, +# and an install whose rules are about something else entirely stops on that +# instead. +_CHECKPOINT_THRESHOLD_KEY = "kb_checkpoint_threshold" + +# MEASURED, not chosen (2026-09-21, `retrieval_telemetry(days=30)` on the +# instance this was built on — recorded as provenance for the number, not as +# a defence of it under rule 115): +# +# write_path_rule floor 0.72 p10 0.6984 p50 0.7319 p90 0.7628 max 0.8817 +# pre_tool_rule floor 0.68 p10 0.6838 p50 0.7018 p90 0.7373 max 0.8293 +# +# 0.80 sits above p90 on BOTH arms and below max on both, so it selects from +# the top decile of an already-selective arm rather than from its bulk — and +# it is reachable, which a bar above 0.8293 would not be for the busier arm. +# Over that window the two arms returned ~3,584 non-empty calls between them, +# so an upper bound of one tenth of those is ~12 a day across a very heavy +# install, and the true rate is lower because 0.80 is not p90 but above it. +# +# A SETTING, because the number above is a distance in one embedding model's +# geometry over one corpus and cannot transfer (retrieval_surfaces' opening +# argument). The default ships as a starting point with the means to correct +# it, which is the only honest form for a number like this. +_CHECKPOINT_DEFAULT = 0.80 + +# A session cannot be stopped more than this many times, however the corpus +# scores. Not a tuning value — a guard on the worst case, like MAX_BUDGET: a +# mis-set floor or a corpus that suddenly resembles everything must degrade to +# a noisy session, never to one that cannot make progress. The hook enforces +# it, because only the hook knows what a session is. +CHECKPOINT_SESSION_CAP = 5 + + # AND A REPEAT COMPETES ON RANK ALONE (#3750), WHICH SURVIVES THE BAND. # # Since #3750 a hit already on the session's exclusion ledger is RENDERED @@ -1524,8 +1584,35 @@ async def get_writepath_config(user_id: int) -> dict: "rule_top_k": await budget_for(user_id, "write_path_rule"), "tool_rule_threshold": await floor_for(user_id, "pre_tool_rule"), "tool_rule_top_k": await budget_for(user_id, "pre_tool_rule"), + # NOT from the surfaces registry, and deliberately so. Everything in + # that table is a pair belonging to one QUERY — a floor saying what is + # worth ranking and a budget saying how many lines it may spend. The + # checkpoint runs no query of its own; it re-reads hits the two rule + # arms already produced and asks a different question of them. Putting + # it in the registry would give it a phantom budget and make the + # tuning tool offer to change how many checkpoints an act may raise, + # which is not a number anyone should have. + "checkpoint_threshold": await _checkpoint_floor(user_id), } + +async def _checkpoint_floor(user_id: int) -> float: + """The confidence at which a hint becomes a stop. Clamped, never trusted. + + A floor read out of settings reaches here as operator-typed text. Below + zero it would stop every act with a rule anywhere near it; above one it + can never fire and the feature is silently dead, which is the failure mode + #3430 found and the reason this clamps rather than validating at the door. + """ + raw = await get_setting( + user_id, _CHECKPOINT_THRESHOLD_KEY, str(_CHECKPOINT_DEFAULT), + ) + try: + value = float(str(raw).strip()) + except (TypeError, ValueError): + return _CHECKPOINT_DEFAULT + return min(1.0, max(0.0, value)) + def _rule_band(hits: list) -> list: """The top hit, plus every hit within `_RULEHINT_BAND` of it (#3851). @@ -1547,6 +1634,104 @@ def _rule_band(hits: list) -> list: return [(s, r) for s, r in hits if s >= top - _RULEHINT_BAND] +def checkpoint_for( + kept: list, *, held: set[int], floor: float, where: str, +) -> dict: + """The one rule, if any, that should STOP this act rather than annotate it. + + Sync and pure so it can be read against a fixed list of hits without a + database — the two arms share it for the reason `_rule_band` is shared: + #3497 is the record of these two drifting apart by being modelled on each + other instead of sharing one function. + + FOUR CONDITIONS, AND EACH IS A DIFFERENT KIND OF WRONG IT PREVENTS. + + 1. THE SCORE CLEARS `floor`. Not the arm's own floor — a much higher bar, + measured at `_CHECKPOINT_DEFAULT`. The hint arms keep nudging at their + floor; only a hit the corpus is confident about is allowed to stop + anything. + + 2. IT IS A RULE, NEVER A PREFERENCE. A preference says how something has + been done before and following it is what keeps work consistent; a rule + says what happens if you do not. Stopping an act over a preference + would assert a force the record explicitly does not claim, and + `_rule_hint_line` already keeps that distinction in the one word that + names it. + + 3. THE SESSION HAS NOT OPENED IT. `held` is observable — a PostToolUse + hook watches for the `get_rule` call (#4100) — so this is a recorded + event and not a model's self-report about its own context. A session + that read the rule has already had the thing the checkpoint exists to + produce, and stopping it again would be punishing the behaviour being + asked for. + + 4. IT IS THE TOP HIT. `kept` is a band, and a band's tail is there to let + an act surface a SET; the ranker's confidence claim attaches to its + first element only. A checkpoint raised on the fourth line of a band is + a stop justified by a score nobody claimed. + + Returns a dict rather than a rule or a tuple. Widening a tuple is an + interface change to every unpack site that the compiler does not report + (#4207, learned the expensive way in this same milestone's first week), and + this value crosses a JSON boundary into a shell script where a missing + field is a silently empty variable. + """ + if not kept or floor <= 0: + return {} + score, rule = kept[0] + if score < floor: + return {} + if getattr(rule, "kind", "") == "preference": + return {} + if rule.id in held: + return {} + found = { + "rule_id": rule.id, + "title": rule.title, + "trigger": (rule.when_to_apply or "").strip(), + "score": round(float(score), 4), + "where": where, + } + # RENDERED HERE, at the one call site, rather than by each arm. Two arms + # that each remember to render it are two arms that can stop agreeing on + # what a stop says — which is #3497's history for this exact pair. The + # text stays a separate function so it can be read and tested without a + # rule object, but nothing outside this line decides whether to call it. + found["reason"] = checkpoint_reason(found) + return found + + +def checkpoint_reason(checkpoint: dict) -> str: + """The text the agent reads INSTEAD of running the act. + + Written as a practice rather than a prohibition (rule 165): it says what + to do and why it is worth doing, not what is forbidden. The act is not + wrong — nothing here knows whether it is — and saying so plainly is what + keeps the stop from reading as an accusation the system is in no position + to make. + + It names the remedy as ONE call, because a stop whose remedy is vague + costs more than the miss it prevents. And it says the act may simply be + re-submitted afterwards, so a reader who finds the rule irrelevant is out + in two calls rather than negotiating with a hook. + """ + if not checkpoint: + return "" + trigger = checkpoint.get("trigger") or "" + return ( + f"Held for one read. “{checkpoint['title']}” is a standing rule " + f"this session has not opened, and it scores {checkpoint['score']} " + f"against what you are about to do" + + (f" ({trigger})" if trigger else "") + + f". Read it with get_rule({checkpoint['rule_id']}), then go ahead — " + f"re-submit this call unchanged if the rule does not apply, which is a " + f"judgement only you can make. Nothing here has decided the act is " + f"wrong; the rule is being put in front of it rather than beside it, " + f"because a rule delivered alongside a result arrives after the " + f"decision it was meant to inform." + ) + + def _rule_hint_line( rule, *, where: str, seen: bool, held: bool = False, compact: bool = False, ) -> str: @@ -2203,6 +2388,7 @@ async def build_write_path_hint( # # Fails open like every other arm: a rule hint must never break a write. rule_ids: list[int] = [] + checkpoint: dict = {} try: already = set(exclude_rule_ids or []) held = set(held_rule_ids or []) @@ -2242,6 +2428,14 @@ async def build_write_path_hint( # would inflate pull_through's denominator with a choice this arm never # made. A reference is a RENDERING decision, not a retrieval outcome. rule_ids.extend(rule.id for _score, rule in fresh) + # The stop, beside the lines rather than instead of them — see + # `checkpoint_for`. `kept` is passed, not `fresh`: whether a rule + # was named earlier this session says nothing about whether this + # act should wait for it to be READ, and those are the two axes + # #3750 exists to keep apart. + checkpoint = checkpoint_for( + kept, held=held, floor=cfg["checkpoint_threshold"], where="here", + ) # TWO tables, and the split is not arbitrary. retrieval_logs is one # row per CALL, keyed on the score distribution a threshold is tuned # from. rule_usage_events is one row per RULE per event, which is the @@ -2312,6 +2506,7 @@ async def build_write_path_hint( "derive": derive, "derive_keys": [d["key"] for d in derive], "rule_ids": rule_ids, + "checkpoint": checkpoint, } @@ -2351,7 +2546,14 @@ async def build_tool_rule_hint( Fails open and returns an empty context on any error: a recall aid may never break the operator's action. """ - out: dict = {"context": "", "rule_ids": []} + # `checkpoint` is present on EVERY return, including the early ones. The + # two arms feed one shell reader, and a key that exists on some responses + # and not others is read there as an empty variable either way — so the + # difference is invisible at the point it would bite and only shows up in + # a test that asserts the contract. Same reason `warnings` is always a + # list in the telemetry readout: an absent key and an empty one must not + # be two ways of saying nothing. + out: dict = {"context": "", "rule_ids": [], "checkpoint": {}} command = (command or "").strip() if not command: return out @@ -2440,6 +2642,14 @@ async def build_tool_rule_hint( ) out["context"] = "\n".join(lines) out["rule_ids"] = rule_ids + # AFTER the lines, never instead of them. A checkpoint stops the act; + # it does not decide what the act should be told, and a reader who + # reads the rule and re-submits must find the same hint waiting. The + # two are independent renderings of one retrieval. + out["checkpoint"] = checkpoint_for( + kept, held=held, floor=cfg["checkpoint_threshold"], + where=f"this {tool_name} call", + ) except Exception: logger.debug("pre-tool rule arm failed", exc_info=True) return out diff --git a/tests/test_pre_act_checkpoint.py b/tests/test_pre_act_checkpoint.py new file mode 100644 index 0000000..c8325af --- /dev/null +++ b/tests/test_pre_act_checkpoint.py @@ -0,0 +1,307 @@ +"""A rule put IN FRONT of an act, not beside its result (#4214, milestone 419). + +WHY THIS EXISTS + +Every other rule surface in this plugin returns `additionalContext`, which +Claude Code delivers alongside the tool RESULT. By the time the line is read +the call is written, so the rule reads as commentary on a decision already +made. Milestone 419 measured the cost over one session: seven misses, three +caught by the operator and none by this system, five of them the same move — +acting on the thing in hand without reading the contract around it. + +A checkpoint spends the same retrieval differently. The hook returns a `deny`, +the act does not run, and the rule's own text can be read before the call +exists. The remedy is one `get_rule` call and the act may then be re-submitted +unchanged. + +WHAT THIS PINS, AND WHAT IT DELIBERATELY DOES NOT + +Pinned: the four conditions under which an act may be held, the two guards +that stop it recurring, and the shape of the envelope. Not pinned: the bar +itself (a tuning value, and a test asserting 0.80 would fail on every retune +while proving nothing) or the wording of the reason (prose, and it will be +rewritten). The cases below express scores relative to the constant. + +THE ONE BOUNDARY THAT IS A RECORDED DECISION, NOT A DESIGN CHOICE. Only the +ACTION arm can hold. `scribe_prior_art.sh` carries a tested property that it +never returns a permissionDecision — the operator's decision that a recall aid +may not stand in the way of a write — and this milestone does not get to +quietly overturn it. The write-path arm computes the same block and returns +it, so the decision can be revisited with evidence; the hook ignores it. The +last test here asserts that boundary holds, because it is exactly the kind of +property that erodes when someone extends the feature later. +""" +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +from scribe.services.plugin_context import ( + _CHECKPOINT_DEFAULT, + CHECKPOINT_SESSION_CAP, + _rule_band, + checkpoint_for, +) +from tests.helpers import fake_rule + +ROOT = Path(__file__).resolve().parents[1] +HOOKS = ROOT / "plugin" / "hooks" +DEFS = HOOKS / "scribe_defs.sh" +ACTION_HOOK = HOOKS / "scribe_tool_rules.sh" +WRITE_HOOK = HOOKS / "scribe_prior_art.sh" + +WHERE = "this Bash call" +TRIGGER = "about to assert in a commit message what CI said" + + +def hit(score: float, rule_id: int, **attrs): + # `attrs` overrides rather than duplicates: passing `when_to_apply=""` for + # the no-trigger case alongside a hard-coded default is a duplicate keyword + # and a TypeError, not a test. + fields = {"id": rule_id, "title": f"rule {rule_id}", "when_to_apply": TRIGGER} + fields.update(attrs) + return (score, fake_rule(**fields)) + + +def hold(kept, held=(), floor=None): + return checkpoint_for( + kept, held=set(held), floor=_CHECKPOINT_DEFAULT if floor is None else floor, + where=WHERE, + ) + + +# ── The four conditions ─────────────────────────────────────────────────── + +def test_nothing_retrieved_holds_nothing(): + """The common case by a wide margin, and the one that must be cheapest.""" + assert hold([]) == {} + + +def test_a_hit_under_the_bar_is_a_hint_and_not_a_stop(): + """The hint arms keep working at their own floor; only a hit the corpus is + confident about is allowed to cost a round trip.""" + assert hold([hit(_CHECKPOINT_DEFAULT - 0.001, 1)]) == {} + + +def test_a_hit_on_the_bar_holds_the_act(): + got = hold([hit(_CHECKPOINT_DEFAULT, 1)]) + assert got["rule_id"] == 1 + assert got["where"] == WHERE + + +def test_a_preference_never_holds_an_act(): + """A preference says how something has been done and following it is what + keeps work consistent; a rule says what happens if you do not. Stopping an + act over a preference would assert a force the record does not claim, and + the renderer already keeps that distinction in the word that names it.""" + assert hold([hit(0.99, 2, kind="preference")]) == {} + + +def test_a_rule_the_session_already_opened_never_holds_an_act(): + """`held` is observable — a PostToolUse hook watches for the `get_rule` + call (#4100) — so this is a recorded event, not a model's self-report + about its own context. A session that read the rule has already had the + thing the checkpoint exists to produce, and holding it again would punish + the behaviour being asked for.""" + assert hold([hit(0.99, 3)], held={3}) == {} + + +def test_holding_one_rule_does_not_excuse_another(): + assert hold([hit(0.99, 4)], held={3})["rule_id"] == 4 + + +def test_only_the_bands_top_hit_may_hold(): + """`_rule_band` keeps a SET so an act can surface several rules, but the + ranker's confidence claim attaches to its first element only. A stop + raised on the fourth line of a band is a stop justified by a score nobody + claimed — so a preference on top ends the question rather than deferring + to the rule behind it.""" + band = [hit(0.99, 5, kind="preference"), hit(0.985, 6)] + assert hold(band) == {} + + +def test_position_decides_not_score(): + """Deliberately falsifiable from the other side: hits arrive ordered, and + this reads `kept[0]` rather than re-maximising. A version that took the + highest score would pick 8 here.""" + assert hold([hit(0.985, 7), hit(0.99, 8)])["rule_id"] == 7 + + +def test_the_band_trims_before_the_checkpoint_sees_it(): + """The two instruments compose in one direction only: the band decides + what is close enough to show, and the checkpoint reads what survived.""" + kept = _rule_band([hit(0.83, 11), hit(0.81, 12), hit(0.70, 13)]) + assert len(kept) == 2 + assert hold(kept, floor=0.80)["rule_id"] == 11 + + +def test_a_floor_of_zero_disables_rather_than_holding_everything(): + """The failure direction that matters. A bar read as 0 — a cleared + setting, a bad parse — must turn the feature OFF, never hold the first + command of every session behind whatever ranked first.""" + assert hold([hit(0.99, 9)], floor=0.0) == {} + + +# ── What the held act is told ───────────────────────────────────────────── + +def test_the_reason_names_the_one_call_that_clears_it(): + """A stop whose remedy is vague costs more than the miss it prevents.""" + reason = hold([hit(0.99, 42)])["reason"] + assert "get_rule(42)" in reason + assert "rule 42" in reason + assert TRIGGER in reason + + +def test_the_reason_says_the_act_may_proceed_unchanged(): + """Nothing here knows whether the act is wrong, and saying so is what + keeps the stop from reading as an accusation the system cannot support.""" + assert "re-submit" in hold([hit(0.99, 42)])["reason"] + + +def test_a_rule_with_no_trigger_renders_without_an_empty_bracket(): + assert "()" not in hold([hit(0.99, 10, when_to_apply="")])["reason"] + + +def test_every_held_act_carries_a_rendered_reason(): + """Rendered at the one call site inside `checkpoint_for`, never by each + arm: two arms that each remember to render it are two arms that can stop + agreeing on what a stop says, which is #3497's history for this pair.""" + assert hold([hit(0.99, 1)])["reason"].strip() + + +# ── The two guards, in the shell that enforces them ─────────────────────── + +def sh(script: str) -> subprocess.CompletedProcess: + for tool in ("bash", "awk"): + if shutil.which(tool) is None: + pytest.skip(f"hook runtime tool {tool!r} not installed") + return subprocess.run( + ["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'], + capture_output=True, text=True, + ) + + +@pytest.fixture() +def ledger(tmp_path): + return tmp_path / "sid.checkpoint.ids" + + +def allowed(ledger, rule_id) -> bool: + r = sh(f'scribe_checkpoint_allowed "{ledger}" "{rule_id}" && echo YES || echo NO') + assert r.returncode == 0, r.stderr + return r.stdout.strip().endswith("YES") + + +def test_a_rule_may_hold_at_most_one_act_per_session(ledger): + """Once the remedy has been offered, repeating it turns a reader who + decided the rule does not apply into a reader who cannot proceed.""" + assert allowed(ledger, 101) + assert not allowed(ledger, 101) + assert allowed(ledger, 102), "a different rule is a different claim" + + +def test_a_session_cannot_be_held_more_than_the_cap(ledger): + """The guard on the worst case, not a tuning value: a mis-set floor or a + corpus that suddenly resembles everything must degrade to a noisy session, + never to one that cannot make progress.""" + for n in range(CHECKPOINT_SESSION_CAP): + assert allowed(ledger, 200 + n) + assert not allowed(ledger, 999) + + +def test_a_refused_hold_is_not_written_to_the_ledger(ledger): + """Otherwise the cap eats itself: refusals would count toward it and the + ledger would grow without a single act ever being held.""" + for n in range(CHECKPOINT_SESSION_CAP): + allowed(ledger, 200 + n) + allowed(ledger, 999) + assert len(ledger.read_text().split()) == CHECKPOINT_SESSION_CAP + + +@pytest.mark.parametrize("rule_id", ["", "abc", " "]) +def test_an_unreadable_rule_id_refuses_rather_than_holding(ledger, rule_id): + """Fails CLOSED in the direction that costs nothing. A garbled id cannot + be written to the ledger, so allowing it would be a stop that recurs + forever with no way to clear it.""" + assert not allowed(ledger, rule_id) + + +def test_no_ledger_file_refuses_rather_than_holding(tmp_path): + """A session with no id gets no ledger, and a stop that cannot be recorded + is a stop that cannot be capped.""" + assert not allowed("", 101) + + +def test_the_deny_envelope_is_valid_json_carrying_the_reason(): + r = sh('scribe_json_deny PreToolUse "read \\"rule 9\\" first — then re-submit"') + assert r.returncode == 0, r.stderr + out = json.loads(r.stdout)["hookSpecificOutput"] + assert out["hookEventName"] == "PreToolUse" + assert out["permissionDecision"] == "deny" + # Quotes and an em dash survive the escaper — the reason is prose and will + # contain both, and a broken envelope is silently ignored by the harness. + assert '"rule 9"' in out["permissionDecisionReason"] + assert "re-submit" in out["permissionDecisionReason"] + + +# ── The boundary that is a recorded decision ────────────────────────────── + +def test_only_the_action_hook_can_hold_an_act(): + """THE GUARD ON THE RECORDED DECISION, and the reason it is here rather + than left to memory. + + `scribe_prior_art.sh` runs before every Write and Edit and has never been + able to stop one. That is the operator's decision — a recall aid may not + stand in the way of the work — and `test_hook_never_returns_a_permission_ + decision` in test_write_path_trigger.py holds the other half of it. + + This milestone has a live argument for extending the checkpoint to writes: + three of its seven misses were file edits and none of them are reachable + from the command side. That argument is exactly why this assertion exists. + A feature with a good reason to spread is the kind that spreads without + anyone deciding to, and the decision here is the operator's to revisit. + """ + assert "scribe_json_deny" in ACTION_HOOK.read_text() + write_code = [ + ln for ln in WRITE_HOOK.read_text().splitlines() + if not ln.lstrip().startswith("#") + ] + assert not any("scribe_json_deny" in ln for ln in write_code) + assert not any("permissionDecision" in ln for ln in write_code) + + +def test_the_action_hook_caps_every_hold_it_emits(): + """Structural, and able to fail (rule 167): the deny and the ledger call + must appear together. A deny emitted outside the guard is a session that + can be held by the same rule on every command, which is the one outcome + both guards exist to prevent.""" + text = ACTION_HOOK.read_text() + code = [ln for ln in text.splitlines() if not ln.lstrip().startswith("#")] + denies = [i for i, ln in enumerate(code) if "scribe_json_deny" in ln] + assert denies, "the action arm no longer emits a hold at all" + for i in denies: + window = "\n".join(code[max(0, i - 6):i]) + assert "scribe_checkpoint_allowed" in window, ( + "a hold is emitted without passing the per-rule and per-session " + "guards first" + ) + + +def test_the_action_hook_records_what_was_surfaced_whichever_way_it_renders(): + """The ledger append sits BEFORE the checkpoint branch. What the server + chose to surface happened whichever way this hook then renders it, and + doing the bookkeeping inside one branch is how the two arms' ledgers came + to disagree once already.""" + code = [ + ln for ln in ACTION_HOOK.read_text().splitlines() + if not ln.lstrip().startswith("#") + ] + append = next(i for i, ln in enumerate(code) if "scribe_rules_append" in ln) + deny = next(i for i, ln in enumerate(code) if "scribe_json_deny" in ln) + assert append < deny + + +def test_the_action_hook_is_still_shell_valid(): + subprocess.run(["bash", "-n", str(ACTION_HOOK)], check=True)