From 0ab94b2a0056c0cad1a1b33575e8e65bcd4b5077 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 23 Aug 2026 10:59:54 -0400 Subject: [PATCH] =?UTF-8?q?fix(plugin):=20write-path=20hooks=20say=20when?= =?UTF-8?q?=20Scribe=20did=20not=20answer=20=E2=80=94=20once=20per=20outag?= =?UTF-8?q?e,=20shared=20marker,=20record=20nudge=20withheld=20on=20an=20u?= =?UTF-8?q?nanswered=20call;=20check=5Fplugin=20allows=20exactly=20that=20?= =?UTF-8?q?line=20when=20unreachable;=20plugin=200.1.43=20(#2932)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- plugin/.claude-plugin/plugin.json | 2 +- plugin/README.md | 12 +++-- plugin/hooks/scribe_after_write.sh | 23 +++++++-- plugin/hooks/scribe_defs.sh | 31 +++++++++++ plugin/hooks/scribe_prior_art.sh | 26 +++++++--- scripts/check_plugin.py | 37 ++++++++++--- tests/test_after_write_hook.py | 34 ++++++++++-- tests/test_write_path_trigger.py | 83 ++++++++++++++++++++++++------ 8 files changed, 208 insertions(+), 40 deletions(-) diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 9249e08..577b44c 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": "0.1.42", + "version": "0.1.43", "author": { "name": "Bryan Van Deusen" }, "mcpServers": { "scribe": { diff --git a/plugin/README.md b/plugin/README.md index 49e0008..14a1fdc 100644 --- a/plugin/README.md +++ b/plugin/README.md @@ -55,15 +55,21 @@ On install you'll be asked for: the edit" — each with its own once-per-session dedup. A third, ledger-fed line names a duplicate family (no canon) or a canon recorded elsewhere for the names being written (its own dedup channel, `exclude_derive`). - Toggle in **Settings → Knowledge auto-inject**. + Fail-open but not fail-silent: a configured instance that does not answer + in time is said, once per outage ("Scribe did not answer … this write went + UNCHECKED"), so a session can tell "checked, nothing there" from "never + checked"; an answer clears the marker. The local by-name arm needs no + server and always runs. Toggle in **Settings → Knowledge auto-inject**. - `hooks/hooks.json` → PostToolUse hook on `Bash` (`hooks/scribe_after_write.sh`): code written through sed/heredocs/scripts never reaches the PreToolUse hook, so this one diffs the working tree after every Bash call (per-session path+blob snapshot; one `git status` when nothing changed) and runs the same arms on the definitions just written, through the same endpoint and the same dedup channels. `additionalContext` - only; silent on any failure. The extractor, the prose/data skip list and the - local by-name duplicate arm are shared in `hooks/scribe_defs.sh`. + only; never blocks, and shares the pre-write hook's once-per-outage "did not + answer" line (8 s budget here — it runs after the tool, so it gates + nothing). The extractor, the prose/data skip list, the local by-name + duplicate arm and the outage line are shared in `hooks/scribe_defs.sh`. - `skills/` → the universal process-skills, surfaced by description match. - `hooks/scribe_sync_processes.sh` (a 2nd SessionStart hook) + the `/scribe:sync` command → generate `~/.claude/skills/scribe-proc-*` stubs from your Scribe diff --git a/plugin/hooks/scribe_after_write.sh b/plugin/hooks/scribe_after_write.sh index 9a49ffc..cf5959f 100644 --- a/plugin/hooks/scribe_after_write.sh +++ b/plugin/hooks/scribe_after_write.sh @@ -154,6 +154,8 @@ while IFS= read -r rel_path; do context="" body="" + reached="" # "" unconfigured (no call owed) · 1 answered · 0 did not + unreached_context="" if [ -n "$url" ] && [ -n "$token" ]; then q=$(printf '%s' "$code" | head -c 1200) path_enc=$(printf '%s' "$rel_path" | jq -sRr '@uri' 2>/dev/null) || path_enc="" @@ -182,9 +184,18 @@ while IFS= read -r rel_path; do # after a redeploy is a cold start (embedding warm-up, ~4.6s observed) # that a 4s cap turned into a silent fail-open — the one write a # session most wants the ledger's word on lost it. + 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="" + "${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; } + # 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. + if [ "$reached" = 1 ]; then + scribe_reached "$state_dir" "$safe_sid" + else + unreached_context=$(scribe_unreached "$state_dir" "$safe_sid" 8 "$rel_path") + fi fi if [ -n "$body" ]; then context=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) || context="" @@ -202,8 +213,10 @@ while IFS= read -r rel_path; do fi # The record nudge (#2664), same gate as the pre-write hook: duplication - # demonstrated locally AND nothing recorded for it. - if [ -n "$local_lines" ]; then + # demonstrated locally AND nothing recorded for it — and (#2932) never on a + # call that did not answer; "nothing recorded" is a claim only an answer + # can back. + if [ -n "$local_lines" ] && [ "$reached" != 0 ]; then n_recorded=$(printf '%s' "$body" | jq -r '.note_ids | length' 2>/dev/null) || n_recorded=0 if [ "${n_recorded:-0}" = "0" ] || [ "$n_recorded" = "" ]; then local_context="${local_context}"$'\n'"> None of those existing copies is recorded in Scribe. If the version just written is the canonical one — or this edit is consolidating the copies — record it now with create_snippet so the next session is offered it instead of writing another copy." @@ -215,6 +228,10 @@ while IFS= read -r rel_path; do [ -n "$part" ] && part="${part}"$'\n' part="${part}${context}" fi + if [ -n "$unreached_context" ]; then + [ -n "$part" ] && part="${part}"$'\n' + part="${part}${unreached_context}" + fi [ -n "$part" ] || continue [ -n "$combined" ] && combined="${combined}"$'\n' combined="${combined}${part}" diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index 61991bc..67b4a32 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -11,6 +11,9 @@ # scribe_defs stdin code → "kindname" per definition # scribe_local_dups ROOT REL "kindname" lines on stdin → the by-name # local-duplicate lines (ARM 1, #2280) +# scribe_unreached STATE SID SECS REL the "Scribe didn't answer" line, once +# per outage (#2932) — or nothing, if said lately +# scribe_reached STATE SID the server answered: the next outage speaks again # # Sourced, not executed: `. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh"`. @@ -114,3 +117,31 @@ scribe_local_dups() { printf '> - `%s` is already defined in %s other file(s): %s\n' "$label" "$count" "$files" done } + +# --------------------------------------------------------------------------- +# The blind spot made visible (#2932). Both write-path hooks fail OPEN when the +# instance is slow or down — right for noise, wrong for silence: a session +# cannot tell "the ledger checked and found nothing" from "the ledger never +# answered", and a self-surfacing system cannot afford an invisible miss (the +# first write after a redeploy lost its derive line to a 4s cold start and +# nobody knew). So a failed call says so — ONCE per outage: the marker holds +# the time it last spoke; within ten minutes of that it stays quiet, and a +# successful call clears it so the next outage announces itself afresh. +# Unconfigured installs never reach this: no URL/token means no call was owed. +_SCRIBE_UNREACHED_QUIET=600 + +scribe_unreached() { + local marker="$1/$2.unreached" now last + now=$(date +%s 2>/dev/null) || now=0 + if [ -f "$marker" ]; then + last=$(cat "$marker" 2>/dev/null) || last=0 + case "$last" in ''|*[!0-9]*) last=0 ;; esac + [ $((now - last)) -lt "$_SCRIBE_UNREACHED_QUIET" ] && return 0 + fi + printf '%s' "$now" > "$marker" 2>/dev/null || true + printf '> Scribe did not answer the prior-art check for `%s` within %ss — this write went UNCHECKED against the record and the shape ledger (the local by-name arm, if it spoke above, needed no server). If the name matters, check it yourself: `search` for the concept, `list_shapes(project_id, path=…)` for the ledger. Said once per outage; if it keeps happening the instance is slow or down.' "$4" "$3" +} + +scribe_reached() { + rm -f "$1/$2.unreached" 2>/dev/null || true +} diff --git a/plugin/hooks/scribe_prior_art.sh b/plugin/hooks/scribe_prior_art.sh index 92ec8cf..a19d1b0 100755 --- a/plugin/hooks/scribe_prior_art.sh +++ b/plugin/hooks/scribe_prior_art.sh @@ -203,11 +203,20 @@ if [ -n "$session_id" ]; then fi fi -# `|| true`, not `|| exit 0`: an unreachable instance must not discard a local -# finding that needed no instance to produce. +# Not `|| exit 0`: an unreachable instance must not discard a local finding +# that needed no instance to produce. And not silence either (#2932): a call +# that was owed and didn't come back is said, once per outage, so the session +# knows this write went unchecked. +reached=1 body=$(curl -fsS --max-time 5 \ -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="" + "${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; } +unreached_context="" +if [ "$reached" = 1 ]; then + scribe_reached "$state_dir" "${safe_sid:-nosession}" +else + unreached_context=$(scribe_unreached "$state_dir" "${safe_sid:-nosession}" 5 "$rel_path") +fi context="" if [ -n "$body" ]; then @@ -234,9 +243,10 @@ fi # noise: the duplication is demonstrated, not guessed. Gated on BOTH sides so # an ordinary new helper (no other copies) and an already-recorded one (the # server spoke) stay nudge-free — a reflex that fires on everything is one -# that gets skipped. An unreachable server counts as "nothing recorded": the -# local finding needed no server, and the nudge fails open with it. -if [ -n "$local_lines" ]; then +# that gets skipped. A server that did not ANSWER earns no nudge (#2932): "none +# of those copies is recorded" is a claim only an answer can back — the +# unreached line says what actually happened instead. +if [ -n "$local_lines" ] && [ "$reached" = 1 ]; then n_recorded=$(printf '%s' "$body" | jq -r '.note_ids | length' 2>/dev/null) || n_recorded=0 if [ "${n_recorded:-0}" = "0" ] || [ "$n_recorded" = "" ]; then local_context="${local_context}"$'\n'"> None of those existing copies is recorded in Scribe. If the version being written is the canonical one — or this edit is consolidating the copies — record it now with create_snippet (name, code, when-to-reach-for-it, location) so the next session is offered it instead of writing another copy." @@ -251,6 +261,10 @@ if [ -n "$context" ]; then [ -n "$combined" ] && combined="${combined}"$'\n' combined="${combined}${context}" fi +if [ -n "$unreached_context" ]; then + [ -n "$combined" ] && combined="${combined}"$'\n' + combined="${combined}${unreached_context}" +fi [ -n "$combined" ] || exit 0 # No permissionDecision: this is a nudge, not a gate. The write goes ahead. diff --git a/scripts/check_plugin.py b/scripts/check_plugin.py index 1ff3a73..c4bdcfd 100755 --- a/scripts/check_plugin.py +++ b/scripts/check_plugin.py @@ -172,15 +172,24 @@ def check_shellcheck() -> None: # --- the fail-open contract ------------------------------------------------ # Every hook promises never to break the operator's session: unconfigured or -# unreachable, it exits 0. Three of them additionally promise SILENCE, because -# they are pure enrichment. scribe_session_context.sh is the exception by -# design — it always emits a static behavioural floor that needs no credentials -# and no network, so "silent" would be the wrong assertion for it. +# unreachable, it exits 0. Unconfigured, the enrichment hooks are SILENT — no +# call was owed. scribe_session_context.sh is the exception by design — it +# always emits a static behavioural floor that needs no credentials and no +# network, so "silent" would be the wrong assertion for it. +# +# UNREACHABLE is different for the two write-path hooks since #2932: a call +# that was owed and did not come back is SAID, once per outage ("> Scribe did +# not answer …"), so a session can tell "checked, nothing there" from "never +# checked". That line — or silence, when the once-per-outage marker in +# ${TMPDIR:-/tmp}/scribe-priorart/ was set by a run in the last ten minutes — +# is the only output allowed with no working instance; anything else is a hook +# speaking on data it cannot have. # # This is the contract that made #2198 invisible for weeks, so it is worth -# pinning: the bug and the healthy no-results case look identical from outside. -# Pinning it does NOT make the failure visible; it makes sure the fail-open -# behaviour is deliberate rather than accidental. +# pinning: the bug and the healthy no-results case looked identical from +# outside. #2932 is what finally makes the failure visible at the write; this +# check makes sure the fail-open behaviour stays deliberate rather than +# accidental. # A symbol that exists nowhere, ASSEMBLED rather than written literally. # The prior-art hook's local arm (#2280) fires with no credentials, so the # silence assertion below needs a name the repo genuinely lacks. Two traps, @@ -218,6 +227,9 @@ SMOKE_EVENTS: dict[str, str] = { # The one hook that legitimately produces output with no credentials. STATIC_FLOOR = "scribe_session_context.sh" +# The hooks that say so when a configured instance does not answer (#2932). +OUTAGE_SPEAKERS = {"scribe_prior_art.sh", "scribe_after_write.sh"} +OUTAGE_LINE = "> Scribe did not answer the prior-art check" def _run_hook(script: Path, event: str, env_extra: dict[str, str]) -> subprocess.CompletedProcess: @@ -269,6 +281,17 @@ def check_fail_open() -> None: f"behavioural floor must survive having no credentials") else: ok(f"{rel} [{label}]: exit 0, static floor present") + elif out and label == "unreachable" and script.name in OUTAGE_SPEAKERS: + # The only thing allowed here is the outage line itself. + try: + ctx = json.loads(out)["hookSpecificOutput"]["additionalContext"] + except (ValueError, KeyError, TypeError): + ctx = "" + if ctx.startswith(OUTAGE_LINE): + ok(f"{rel} [{label}]: exit 0, says the instance did not answer") + else: + fail(f"{rel} [{label}]: emitted output with no working instance " + f"that is not the outage line:\n {out[:200]}") elif out: fail(f"{rel} [{label}]: emitted output with no working instance:\n" f" {out[:200]}") diff --git a/tests/test_after_write_hook.py b/tests/test_after_write_hook.py index 5a8ec7e..01dfb11 100644 --- a/tests/test_after_write_hook.py +++ b/tests/test_after_write_hook.py @@ -108,14 +108,40 @@ def test_after_write_is_silent_where_it_has_nothing_to_say(tmp_path): assert _run(repo, env, session="s-quiet") == "" -def test_after_write_local_arm_and_record_nudge_work_without_a_server(tmp_path): - """The local by-name arm needs no instance (#2280) and the record nudge - (#2664) fails open with it — a refused connection stands in for the - instance.""" +def test_after_write_local_arm_works_without_a_server_and_says_the_server_did_not_answer(tmp_path): + """The local by-name arm needs no instance (#2280). A configured instance + that does not ANSWER (a refused connection stands in for it) is said, once + per outage (#2932) — and the record nudge, which claims "nothing recorded", + is withheld: no answer backs that claim.""" env = _env(tmp_path) repo = _repo(tmp_path, env) (repo / "d.py").write_text("def slug(t):\n return t.lower()\n") out = _run(repo, env, session="s-local") ctx = json.loads(out)["hookSpecificOutput"]["additionalContext"] assert "`slug` is already defined in 1 other file(s): c.py" in ctx + assert "Scribe did not answer the prior-art check for `d.py` within 8s" in ctx + assert "UNCHECKED" in ctx + assert "None of those existing copies is recorded" not in ctx + marker = tmp_path / "scribe-priorart" / "s-local.unreached" + assert marker.is_file() and marker.read_text().isdigit() + # Still down a moment later: the local arm speaks, the outage line does not + # repeat (once per outage, not once per write). + (repo / "e.py").write_text("def slug(t):\n return t.upper()\n") + out = _run(repo, env, session="s-local") + ctx = json.loads(out)["hookSpecificOutput"]["additionalContext"] + assert "`slug` is already defined in" in ctx + assert "did not answer" not in ctx + + +def test_after_write_unconfigured_install_owes_no_call_and_keeps_the_record_nudge(tmp_path): + """No URL/token → no call was owed, so nothing is "unreached"; the local + arm and the record nudge (#2664) stand on their own, as before.""" + env = {k: v for k, v in _env(tmp_path).items() if k not in ("SCRIBE_URL", "SCRIBE_TOKEN")} + repo = _repo(tmp_path, env) + (repo / "d.py").write_text("def slug(t):\n return t.lower()\n") + out = _run(repo, env, session="s-unconf") + ctx = json.loads(out)["hookSpecificOutput"]["additionalContext"] + assert "`slug` is already defined in 1 other file(s): c.py" in ctx assert "create_snippet" in ctx + assert "did not answer" not in ctx + assert not (tmp_path / "scribe-priorart" / "s-unconf.unreached").exists() diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index 626663f..e938f4d 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -909,29 +909,35 @@ def _hook_runtime_env(): "SCRIBE_URL": "http://127.0.0.1:9", "SCRIBE_TOKEN": "t"} -def test_hook_nudges_recording_when_copies_exist_but_nothing_is_recorded(tmp_path): - """#2664: the local arm proves duplication; when Scribe has no record of it, - the same context block must ask for create_snippet — the one moment the - recording nudge is earned rather than noise. An unreachable server counts - as "nothing recorded": the local finding needed no server, and the nudge - fails open with it (here: a refused connection stands in for the instance).""" - env = _hook_runtime_env() +def _dup_repo(tmp_path, env): repo = tmp_path / "repo" repo.mkdir() subprocess.run(["git", "init", "-q"], cwd=repo, check=True, env=env) (repo / "a.py").write_text("def debounce(fn):\n return fn\n") # git grep searches the index, so the existing copy must be staged. subprocess.run(["git", "add", "."], cwd=repo, check=True, env=env) - out = subprocess.run( - ["bash", str(HOOK)], - input=json.dumps({ - "session_id": "s-nudge", "cwd": str(repo), "tool_name": "Write", - "tool_input": {"file_path": str(repo / "b.py"), - "content": "def debounce(fn):\n return fn\n"}, - }), - capture_output=True, text=True, env=env, - ) + return repo + + +def _write_event(repo, session="s-nudge"): + return json.dumps({ + "session_id": session, "cwd": str(repo), "tool_name": "Write", + "tool_input": {"file_path": str(repo / "b.py"), + "content": "def debounce(fn):\n return fn\n"}, + }) + + +def test_hook_nudges_recording_when_copies_exist_but_nothing_is_recorded(tmp_path): + """#2664: the local arm proves duplication; when Scribe ANSWERS that it has + no record of it, the same context block must ask for create_snippet — the + one moment the recording nudge is earned rather than noise.""" + with http_sink(b'{"context":"","note_ids":[],"sync_note_ids":[]}') as (port, seen): + env = dict(_hook_runtime_env(), SCRIBE_URL=f"http://127.0.0.1:{port}") + repo = _dup_repo(tmp_path, env) + out = subprocess.run(["bash", str(HOOK)], input=_write_event(repo), + capture_output=True, text=True, env=env) assert out.returncode == 0 + assert seen and seen[0]["path"] == ["b.py"] assert out.stdout.strip(), ( "hook produced no output — the local arm should have found the " "staged duplicate and nudged" @@ -939,6 +945,51 @@ def test_hook_nudges_recording_when_copies_exist_but_nothing_is_recorded(tmp_pat ctx = json.loads(out.stdout)["hookSpecificOutput"]["additionalContext"] assert "already defined" in ctx # the duplication finding assert "create_snippet" in ctx # the recording ask riding it + assert "did not answer" not in ctx + + +def test_hook_says_when_scribe_did_not_answer_once_per_outage(tmp_path): + """#2932: a configured instance that does not answer (refused connection) + is SAID — the write went unchecked — instead of the hook failing open in + silence; the record nudge's "nothing recorded" claim is withheld. Once per + outage: a second miss is quiet, an answer clears the marker, and the next + miss speaks again. The marker is shared with the after-write hook.""" + env = _hook_runtime_env() # SCRIBE_URL → a refused port + repo = _dup_repo(tmp_path, env) + marker = tmp_path / "scribe-priorart" / "s-out.unreached" + env["TMPDIR"] = str(tmp_path) + out = subprocess.run(["bash", str(HOOK)], input=_write_event(repo, "s-out"), + capture_output=True, text=True, env=env) + assert out.returncode == 0 + ctx = json.loads(out.stdout)["hookSpecificOutput"]["additionalContext"] + assert "already defined" in ctx + assert "Scribe did not answer the prior-art check for `b.py` within 5s" in ctx + assert "UNCHECKED" in ctx and "list_shapes" in ctx + assert "None of those existing copies is recorded" not in ctx + assert marker.is_file() + # Second miss inside the quiet window: local arm only. + out = subprocess.run(["bash", str(HOOK)], input=_write_event(repo, "s-out"), + capture_output=True, text=True, env=env) + ctx = json.loads(out.stdout)["hookSpecificOutput"]["additionalContext"] + assert "already defined" in ctx and "did not answer" not in ctx + # An answer clears the marker … + with http_sink(b'{"context":"","note_ids":[],"sync_note_ids":[]}') as (port, _seen): + up = dict(env, SCRIBE_URL=f"http://127.0.0.1:{port}") + subprocess.run(["bash", str(HOOK)], input=_write_event(repo, "s-out"), + capture_output=True, text=True, env=up) + assert not marker.exists() + # … so the next outage is announced afresh. + out = subprocess.run(["bash", str(HOOK)], input=_write_event(repo, "s-out"), + capture_output=True, text=True, env=env) + assert "did not answer" in json.loads(out.stdout)["hookSpecificOutput"]["additionalContext"] + # A write the hook had nothing local to say about still carries the line + # (the line is the whole message then): a fresh session, no duplicate. + (repo / "a.py").unlink() + subprocess.run(["git", "add", "-A"], cwd=repo, check=True, env=env) + out = subprocess.run(["bash", str(HOOK)], input=_write_event(repo, "s-out-2"), + capture_output=True, text=True, env=env) + ctx = json.loads(out.stdout)["hookSpecificOutput"]["additionalContext"] + assert ctx.startswith("> Scribe did not answer") def test_hook_stays_quiet_about_recording_when_nothing_is_duplicated(tmp_path):