diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index c3fe04c..59a9fdd 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe system-of-record for Claude Code: MCP tools over your notes/tasks/projects/rules, a session-start push channel that surfaces your always-on rules + active-project context, process-skills (writing-plans, systematic-debugging, verification, brainstorming, reusing-code), and your saved Scribe Processes auto-surfaced as skills (/scribe:sync). Replaces superpowers + file-memory with one app-backed plugin.", - "version": "2026.09.09.0408", + "version": "2026.09.10.0221", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index 93a2b39..9da0088 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -16,6 +16,9 @@ # scribe_reached STATE SID the server answered: the next outage speaks again # scribe_config sets `url` + `token` from the env, returns 0 # only if BOTH are usable (#2278) +# scribe_rules_live FILE live rule ids from the exclusion ledger, +# comma-joined; entries age out (#3751) +# scribe_rules_append FILE stdin ids -> the ledger, timestamped # # Sourced, not executed: `. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh"`. @@ -175,3 +178,103 @@ scribe_unreached() { scribe_reached() { rm -f "$1/$2.unreached" 2>/dev/null || true } + +# --------------------------------------------------------------------------- +# THE RULE EXCLUSION LEDGER, and how entries in it AGE (#3751). +# +# Two hooks write this file and two read it, which is the whole reason the +# parsing lives here: `scribe_prior_art.sh` and `scribe_tool_rules.sh` share +# one ledger so a rule named by one arm is not re-offered by the other, and a +# format only one of them understood would break that on the first read. +# +# WHAT PROBLEM AGEING SOLVES. #3749 clears the ledger when an EVENT destroys +# context — a compaction or a /clear. This is the case with no event at all: a +# long session where the rule was named two hundred turns ago and has simply +# fallen out of attention. It is #3702's argument at the tier level (present in +# context and salient at the moment are different properties) applied to time +# instead of to tier. +# +# PER-ENTRY TIMESTAMPS, NOT A FILE MTIME. Clearing the whole ledger when the +# file is old is one line of shell and wrong in exactly the session that needs +# it: a single recent write keeps every stale id alive, and the ids that go +# stale first are the ones from the rules that fire most. +# +# WALL TIME, NOT A TURN COUNT, and the trade is real rather than dismissed. A +# turn count is a truer model of salience — an idle session does not forget — +# but a hook has no turn number without keeping its own counter, which is a +# second piece of session state to write, read, clear on compaction and get +# wrong. Wall time is available from `date` and costs nothing. The failure mode +# it accepts is a session left idle over lunch treating its rules as forgotten, +# which produces one extra full line per rule and no other harm. +_SCRIBE_RULE_TTL=2700 + +# 45 MINUTES, and the reasoning rather than the number (rule 32). +# +# There is no data on this yet, so it is a judgement made to be revised — the +# telemetry that would settle it is the one #3807 just built, and a reading of +# how often an aged-out rule gets PULLED after it returns is what should move +# this. +# +# Too short and the exclusion stops existing and the repetition it prevents +# comes back. Too long and it never fires at all in a session short enough to +# matter. 45 minutes is about one working stretch on a single task: long enough +# that a rule does not re-announce itself while you are still doing the thing +# it governs, short enough that a multi-hour session gets a genuine refresh +# rather than one 9am mention. +# +# Being wrong on the short side is now the cheaper error, which is why this +# leans short. Since #3750 an excluded rule is REFERENCED rather than withheld, +# so the ledger is no longer the only thing standing between a session and a +# rule it has forgotten — an expired entry costs one full line instead of one +# short one, and the exclusion re-arms the moment it is spent. + +# Live ids from a ledger, comma-joined for `exclude_rule_ids`. Empty output for +# a missing, empty or fully-aged file — the callers already treat "" as "send +# no exclusions". +# +# THE LAST ENTRY FOR AN ID WINS, and this is what stops a rule ping-ponging. +# The file is append-only, so a rule that ages out, gets surfaced fresh and is +# appended again has TWO lines. Reading the first would leave it permanently +# expired and it would re-announce itself on every single call from then on — +# the loudest possible failure, from the mechanism meant to quieten things. +# Appends are chronological, so the last line for an id is its most recent. +# +# A BARE ID — no tab, no timestamp — IS LIVE. That is the pre-#3751 format, and +# a session in flight when this ships has a ledger full of them. Treating +# unknown as expired would make every one of those sessions re-announce every +# rule it had already been told, all at once, which is precisely the noise this +# exists to prevent. Unknown means "not measured" and never "old" — the same +# null discipline the retrieval_logs columns use. Those entries simply never +# age, which is bounded: the session ends. +scribe_rules_live() { + local f="$1" now + [ -n "$f" ] && [ -f "$f" ] || return 0 + now=$(date +%s 2>/dev/null) || now=0 + awk -F'\t' -v now="$now" -v ttl="$_SCRIBE_RULE_TTL" ' + { + id = $1 + gsub(/[^0-9]/, "", id) + if (id == "") next + if (!(id in seen)) { seen[id] = 1; seq[++n] = id } + stamp[id] = ($2 ~ /^[0-9]+$/) ? $2 : "" + } + END { + out = "" + for (i = 1; i <= n; i++) { + id = seq[i] + if (stamp[id] != "" && now > 0 && (now - stamp[id]) > ttl) continue + out = out (out == "" ? "" : ",") id + } + print out + } + ' "$f" 2>/dev/null || true +} + +# Append surfaced ids, stamped. Reads ids on stdin, one per line — the shape +# `jq -r '(.rule_ids // [])[]?'` already produces at both call sites. +scribe_rules_append() { + local f="$1" now + [ -n "$f" ] || return 0 + now=$(date +%s 2>/dev/null) || now=0 + awk -v ts="$now" 'NF { print $1 "\t" ts }' >> "$f" 2>/dev/null || true +} diff --git a/plugin/hooks/scribe_prior_art.sh b/plugin/hooks/scribe_prior_art.sh index 67242be..04f2cd7 100755 --- a/plugin/hooks/scribe_prior_art.sh +++ b/plugin/hooks/scribe_prior_art.sh @@ -204,10 +204,11 @@ if [ -n "$session_id" ]; then 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 - if [ -f "$rulefile" ]; then - rule_seen=$(tr '\n' ',' < "$rulefile" 2>/dev/null | sed 's/,$//') - [ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}" - fi + # Ageing, not a flat read (#3751), and the ONLY ledger here that ages: the + # note channels above are a different question with a different answer, and + # this arm's sibling hook reads the same rule file through the same helper. + rule_seen=$(scribe_rules_live "$rulefile") + [ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}" fi # Not `|| exit 0`: an unreachable instance must not discard a local finding @@ -239,7 +240,8 @@ if [ -n "$body" ]; then printf '%s' "$body" | jq -r '(.sync_note_ids // [])[]?' 2>/dev/null >> "$syncfile" || true fi if [ -n "$rulefile" ]; then - printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null >> "$rulefile" || true + printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null \ + | scribe_rules_append "$rulefile" fi if [ -n "$derivefile" ]; then printf '%s' "$body" | jq -r '(.derive_keys // [])[]?' 2>/dev/null >> "$derivefile" || true diff --git a/plugin/hooks/scribe_tool_rules.sh b/plugin/hooks/scribe_tool_rules.sh index 6803a78..2d68977 100644 --- a/plugin/hooks/scribe_tool_rules.sh +++ b/plugin/hooks/scribe_tool_rules.sh @@ -86,10 +86,10 @@ rule_exclude_q="" if [ -n "$session_id" ]; then safe_sid=$(printf '%s' "$session_id" | tr -c 'A-Za-z0-9._-' '_') rulefile="$state_dir/${safe_sid}.rules.ids" - if [ -f "$rulefile" ]; then - rule_seen=$(tr '\n' ',' < "$rulefile" 2>/dev/null | sed 's/,$//') - [ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}" - fi + # Ageing, not a flat read (#3751): an id named two hours ago is not one the + # session is still holding. scribe_rules_live carries the reasoning. + rule_seen=$(scribe_rules_live "$rulefile") + [ -n "$rule_seen" ] && rule_exclude_q="&exclude_rule_ids=${rule_seen}" fi # `|| exit 0` here, unlike the prior-art hook: there is no local arm whose @@ -104,7 +104,8 @@ context=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) || exit 0 # Remember what was named so it is not repeated this session. if [ -n "$rulefile" ]; then - printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null >> "$rulefile" || true + printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null \ + | scribe_rules_append "$rulefile" fi jq -cn --arg ctx "$context" '{ diff --git a/tests/test_rule_ledger_ageing.py b/tests/test_rule_ledger_ageing.py new file mode 100644 index 0000000..a0e3807 --- /dev/null +++ b/tests/test_rule_ledger_ageing.py @@ -0,0 +1,216 @@ +"""Entries in the rule exclusion ledger age out (#3751). + +WHY THIS EXISTS + +#3749 clears the ledger when an EVENT destroys context — a compaction, a +/clear. This covers the case with no event at all: a long session where a rule +was named two hundred turns ago and has simply fallen out of attention. Same +argument #3702 made at the tier level (present in context and salient at the +moment are different properties), applied to time instead of to tier. + +WHAT IS PINNED, AND WHAT DELIBERATELY IS NOT + +Not the TTL's value. 2700 is a judgement made to be revised once there is +telemetry to revise it with, and a test asserting `== 2700` would turn a +legitimate tuning change into a red build — which is how a number nobody may +touch gets one. The tests read the constant out of the shell and assert the +PROPERTY around it, so any TTL works and only a broken comparison fails. + +Runs the real shell, like `test_session_context_ledger` and the after-write +hook's tests, and deliberately with no SCRIBE_URL/SCRIBE_TOKEN: ageing is +local, keyless and networkless, and must work on an instance that is +unreachable or was never configured. +""" +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import time +from pathlib import Path + +import pytest + +PLUGIN = Path(__file__).resolve().parents[1] / "plugin" / "hooks" +DEFS = PLUGIN / "scribe_defs.sh" + + +def _ttl() -> int: + """The TTL as the shell defines it — never a copy of the number.""" + m = re.search(r"^_SCRIBE_RULE_TTL=(\d+)", DEFS.read_text(), re.M) + assert m, "_SCRIBE_RULE_TTL is gone from scribe_defs.sh" + return int(m.group(1)) + + +def _live(ledger: Path) -> list[str]: + """`scribe_rules_live` against a real ledger, as the hooks call it.""" + for tool in ("bash", "awk"): + if shutil.which(tool) is None: + pytest.skip(f"hook runtime tool {tool!r} not installed") + proc = subprocess.run( + ["bash", "-c", f'. "{DEFS}"; scribe_rules_live "{ledger}"'], + capture_output=True, text=True, timeout=30, + env={"PATH": os.environ["PATH"]}, # no credentials — see the docstring + ) + assert proc.returncode == 0, proc.stderr + out = proc.stdout.strip() + return out.split(",") if out else [] + + +def _write_ledger(ledger: Path, entries: list[tuple[str, int | None]]) -> None: + """A ledger written in AGES rather than absolute stamps. + + Named for what it is: `tests/helpers._now` is a datetime for record + fixtures and this file wants the shell's epoch, so they are deliberately + not the same helper and deliberately not the same name. `time.time()` + rather than shelling out to `date` — both read the one system clock, so + the subprocess bought nothing. + """ + now = int(time.time()) + ledger.write_text("".join( + f"{rid}\n" if ago is None else f"{rid}\t{now - ago}\n" + for rid, ago in entries + )) + + +def test_an_old_entry_ages_out_and_a_recent_one_does_not(tmp_path): + """BOTH DIRECTIONS IN ONE ASSERTION (rule 167), because either half alone + passes against a broken implementation. + + A test that only checks the old id is gone passes against a helper that + returns nothing at all — which would drop every exclusion and bring back + the repetition the ledger exists to prevent. A test that only checks the + recent id survives passes against the flat `tr '\\n' ','` read this + replaces, i.e. against the defect itself. + """ + ttl = _ttl() + ledger = tmp_path / "s.rules.ids" + _write_ledger(ledger, [("156", ttl + 600), ("168", 60)]) + + assert _live(ledger) == ["168"], ( + f"expected the entry {ttl + 600}s old to age out and the 60s-old one " + f"to survive (TTL {ttl}s). An empty list means nothing is excluded any " + f"more; both ids means nothing ages." + ) + + +def test_a_repeatedly_surfaced_rule_appears_once_in_the_exclusion_list(tmp_path): + """List hygiene, and it is not cosmetic. + + The ledger is append-only, so a rule surfaced five times has five lines. + The output is spliced straight into `&exclude_rule_ids=` and parsed by the + server as a list; duplicates make that list grow without bound over a long + session, and a trailing or doubled comma is an empty element the parser has + to decide what to do with. Neither is visible from the hint. + + (There was a boundary test here — `ttl` vs `ttl + 1`. It was racy by + construction: a ledger written at T is read at T+n, so the entry ages by + however long the subprocess took and the two cases swap. A one-second + distinction on a 45-minute window is also not a behaviour anyone can + observe, so it was pinning a flake rather than a property.) + """ + ledger = tmp_path / "s.rules.ids" + _write_ledger(ledger, [("156", 60), ("168", 30), ("156", 10), ("156", 5)]) + + live = _live(ledger) + assert live == ["156", "168"], ( + f"got {live}: an id repeated in the ledger must appear once in the " + f"exclusion list, in first-seen order" + ) + raw = subprocess.run( + ["bash", "-c", f'. "{DEFS}"; scribe_rules_live "{ledger}"'], + capture_output=True, text=True, timeout=30, + env={"PATH": os.environ["PATH"]}, + ).stdout.strip() + assert ",," not in raw and not raw.endswith(","), ( + f"the exclusion list has an empty element: {raw!r}" + ) + + +def test_a_rule_that_ages_out_and_returns_does_not_ping_pong(tmp_path): + """THE FAILURE THIS COSTS THE MOST TO GET WRONG. + + The ledger is append-only, so a rule that ages out, gets surfaced fresh and + is appended again has TWO lines. An implementation reading the FIRST leaves + it permanently expired — and it then re-announces itself on every single + call for the rest of the session. The mechanism meant to quieten things + becomes the loudest thing in the hint. + """ + ttl = _ttl() + ledger = tmp_path / "s.rules.ids" + _write_ledger(ledger, [("156", ttl + 600)]) + + subprocess.run( + ["bash", "-c", f'. "{DEFS}"; printf \'156\\n\' | scribe_rules_append "{ledger}"'], + capture_output=True, text=True, timeout=30, + env={"PATH": os.environ["PATH"]}, + ) + + assert _live(ledger) == ["156"], ( + "a re-surfaced rule read as still expired; the most recent entry for " + "an id must win, or it re-announces itself on every call from here on" + ) + + +def test_a_bare_id_from_the_old_format_is_treated_as_live(tmp_path): + """The compatibility decision, asserted rather than described. + + Every session in flight when this ships has a ledger of bare ids. Reading + unknown as EXPIRED would make all of them re-announce every rule they had + already been told, all at once — the exact noise this feature exists to + prevent, delivered by the feature itself on the day it ships. + + Unknown means "not measured", never "old" — the same discipline the + nullable retrieval_logs columns use. Those entries never age, which is + bounded, because the session ends. + """ + ttl = _ttl() + ledger = tmp_path / "s.rules.ids" + ledger.write_text("156\n168\n") + assert _live(ledger) == ["156", "168"] + + # And a bare id keeps its own meaning next to a stale stamped one, rather + # than the file falling back to one format or the other wholesale. + _write_ledger(ledger, [("9", None), ("156", ttl + 600)]) + assert _live(ledger) == ["9"] + + +def test_a_missing_or_empty_ledger_excludes_nothing(tmp_path): + """Fail-open, like every other path in these hooks: no ledger means no + exclusions, not an error and not a stray comma the server would parse as + an id.""" + assert _live(tmp_path / "never-written.rules.ids") == [] + empty = tmp_path / "empty.rules.ids" + empty.write_text("") + assert _live(empty) == [] + + +def test_both_hooks_read_the_ledger_through_the_shared_helper(): + """Structural, because the defect this would reintroduce is invisible. + + Two hooks share one ledger so a rule named by one arm is not re-offered by + the other. If either goes back to reading the file flat, that hook stops + ageing while its sibling keeps ageing — the two disagree about which rules + are live, and nothing in the output says so. + + Pinned on the flat-read SHAPE rather than on a helper name: a hook that + stops calling the helper but still ages correctly some other way is not a + defect, and a rename of the helper is not one either. + """ + for name in ("scribe_prior_art.sh", "scribe_tool_rules.sh"): + src = (PLUGIN / name).read_text() + rule_reads = [ + ln for ln in src.splitlines() + if "rulefile" in ln and "tr " in ln and r"'\n' ','" in ln + ] + assert not rule_reads, ( + f"{name} reads the rule ledger flat again: {rule_reads}. That " + f"hook's exclusions would stop ageing while its sibling's keep " + f"ageing, and the two would disagree silently about which rules " + f"this session still holds." + ) + assert "scribe_rules_live" in src, ( + f"{name} no longer reads the rule ledger through the shared " + f"helper, so the two hooks can drift on the ledger format" + )