A repeat is not a rejection, and a compaction is not knowledge #145
@@ -55,6 +55,53 @@ here=$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) || exit 0
|
||||
event=$(cat 2>/dev/null || true)
|
||||
source=$(printf '%s' "$event" | jq -r '.source // empty' 2>/dev/null) || source=""
|
||||
|
||||
# --- The rule ledger outlives the context it describes (#3749) ---
|
||||
#
|
||||
# scribe_prior_art.sh and scribe_tool_rules.sh record every rule id they have
|
||||
# named in <state>/<sid>.rules.ids and hand it back as exclude_rule_ids, so a
|
||||
# rule is named once per session and then goes quiet. That is right while the
|
||||
# session still HOLDS what it was told, and wrong the moment it does not.
|
||||
#
|
||||
# A compaction summarizes the earlier injections away and does not touch the
|
||||
# filesystem, so the rule ends up absent from context AND still excluded —
|
||||
# unreachable for the rest of the session. The banner below tells the model to
|
||||
# re-pull its ALWAYS-ON rules, but a rule an arm surfaced is conditional and is
|
||||
# not in that set, so it has no other way back. The rules most likely to be in
|
||||
# this state are the ones that fire most often, which is to say the ones that
|
||||
# apply most.
|
||||
#
|
||||
# The session id survives a compaction — the etag marker further down is
|
||||
# rewritten on `compact` and keyed by session_id, which is only meaningful if
|
||||
# the id is stable — so the stale ledger is genuinely found again, not orphaned.
|
||||
#
|
||||
# CLEARED ON THE SOURCES THAT DESTROY CONTEXT, AND ONLY THOSE:
|
||||
#
|
||||
# compact CLEAR — summarized away; the file survived.
|
||||
# clear CLEAR — context wiped.
|
||||
# startup nothing to do: a new session id means a new, empty file.
|
||||
# resume KEEP. The context was genuinely restored, so the ledger still
|
||||
# describes what the session holds. Clearing here would re-surface
|
||||
# every rule after a restore that lost nothing — the mirror error.
|
||||
# fork KEEP, and the answer is the same whichever way forks are keyed: a
|
||||
# fork carries the conversation, so if it inherits the id the ledger
|
||||
# is accurate, and if it gets a new one the file is empty anyway.
|
||||
#
|
||||
# ONLY the rules ledger. The same directory holds .ids / .sync.ids /
|
||||
# .derive.ids for the note arms. Whether a surfaced NOTE should return after a
|
||||
# compaction is a different question with a different answer, and leaving those
|
||||
# alone is a decision rather than an oversight.
|
||||
case "$source" in
|
||||
compact|clear)
|
||||
sid=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || sid=""
|
||||
if [ -n "$sid" ]; then
|
||||
safe_sid=$(printf '%s' "$sid" | tr -c 'A-Za-z0-9._-' '_')
|
||||
# Best-effort, like every other filesystem touch in these hooks: a ledger
|
||||
# that cannot be removed costs a repeated exclusion, never a session.
|
||||
rm -f "${TMPDIR:-/tmp}/scribe-priorart/${safe_sid}.rules.ids" 2>/dev/null || true
|
||||
fi
|
||||
;;
|
||||
esac
|
||||
|
||||
out=""
|
||||
# Append $1 to $out, separated by a horizontal rule when $out already has content.
|
||||
append() { if [ -n "$out" ]; then out="${out}"$'\n\n---\n\n'"$1"; else out="$1"; fi; }
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
"""The SessionStart hook clears the rule ledger exactly when context dies (#3749).
|
||||
|
||||
WHY THIS EXISTS
|
||||
|
||||
The prior-art and tool-rule hooks record every rule id they have named in
|
||||
`<state>/<sid>.rules.ids` and hand it back as `exclude_rule_ids`, so a rule is
|
||||
surfaced once per session and then goes quiet. That is correct while the
|
||||
session still holds what it was told.
|
||||
|
||||
A compaction breaks that assumption in the worst available way: it summarizes
|
||||
the earlier injections out of context and does not touch the filesystem. The
|
||||
rule ends up absent from context AND still excluded — unreachable for the rest
|
||||
of the session. The compaction banner tells the model to re-pull its
|
||||
*always-on* rules, but a rule an arm surfaced is conditional and is not in that
|
||||
set, so it has no other way back. The rules most likely to be in that state are
|
||||
the ones that fire most often.
|
||||
|
||||
WHAT THIS PINS
|
||||
|
||||
Not "the ledger is cleared" — that would pass against a hook which deletes it
|
||||
on every source, and deleting on `resume` is its own defect: the context was
|
||||
genuinely restored there, so re-surfacing every rule is the mirror error.
|
||||
|
||||
What is pinned is the DISCRIMINATION. The whole source table is asserted in one
|
||||
statement, so a blanket delete (all False) and a no-op (all True) both fail,
|
||||
and neither can be made to pass by editing one case.
|
||||
|
||||
Runs the real shell against a temp TMPDIR, like the after-write hook's tests.
|
||||
Deliberately with no SCRIBE_URL/SCRIBE_TOKEN in the environment: clearing the
|
||||
ledger is local, keyless and networkless, and must still happen on an instance
|
||||
that is unreachable or unconfigured.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
PLUGIN = Path(__file__).resolve().parents[1] / "plugin"
|
||||
HOOK = PLUGIN / "hooks" / "scribe_session_context.sh"
|
||||
|
||||
|
||||
def _env(tmp_path):
|
||||
"""Near-namesake of test_after_write_hook's `_env`, and deliberately not it:
|
||||
that one needs git and curl and SUPPLIES credentials, because the behaviour
|
||||
it tests is a network round-trip. This one must prove the opposite — that
|
||||
the clear happens with no credentials and no network at all — so sharing a
|
||||
helper would mean testing this case in an environment that cannot show it.
|
||||
"""
|
||||
for tool in ("jq", "bash"):
|
||||
if shutil.which(tool) is None:
|
||||
pytest.skip(f"hook runtime tool {tool!r} not installed")
|
||||
# No SCRIBE_URL / SCRIBE_TOKEN on purpose — see the module docstring.
|
||||
return {"PATH": os.environ["PATH"], "TMPDIR": str(tmp_path),
|
||||
"HOME": str(tmp_path)}
|
||||
|
||||
|
||||
def _ledger(tmp_path, sid: str, name: str = "rules.ids") -> Path:
|
||||
d = tmp_path / "scribe-priorart"
|
||||
d.mkdir(exist_ok=True)
|
||||
f = d / f"{sid}.{name}"
|
||||
f.write_text("156\n168\n")
|
||||
return f
|
||||
|
||||
|
||||
def _fire(source: str, sid: str, env) -> None:
|
||||
subprocess.run(
|
||||
["bash", str(HOOK)],
|
||||
input=json.dumps({"source": source, "session_id": sid}),
|
||||
capture_output=True, text=True, env=env, timeout=30,
|
||||
)
|
||||
|
||||
|
||||
def test_the_rules_ledger_survives_exactly_when_the_context_does(tmp_path):
|
||||
"""The whole source table, in one assertion, so it cannot be half-satisfied.
|
||||
|
||||
`startup` is listed even though it is a no-op against a session id that has
|
||||
never been seen: it is asserted here so that a future change which starts
|
||||
clearing indiscriminately fails on a case somebody would otherwise call
|
||||
harmless.
|
||||
"""
|
||||
env = _env(tmp_path)
|
||||
survived = {}
|
||||
for source in ("compact", "clear", "resume", "startup"):
|
||||
sid = f"sess-{source}"
|
||||
ledger = _ledger(tmp_path, sid)
|
||||
_fire(source, sid, env)
|
||||
survived[source] = ledger.exists()
|
||||
|
||||
assert survived == {
|
||||
"compact": False,
|
||||
"clear": False,
|
||||
"resume": True,
|
||||
"startup": True,
|
||||
}, (
|
||||
f"got {survived}. A rule surfaced before a compaction is summarized "
|
||||
f"out of context while its id stays on the exclusion ledger, so it "
|
||||
f"becomes unreachable for the rest of the session — that is what the "
|
||||
f"compact/clear cases prevent. The resume case is the other half: the "
|
||||
f"context came back intact there, and re-surfacing every rule after a "
|
||||
f"restore that lost nothing is the same defect from the other side. "
|
||||
f"All-False means something is deleting unconditionally; all-True "
|
||||
f"means the clear never runs."
|
||||
)
|
||||
|
||||
|
||||
def test_only_the_rule_ledger_is_cleared_and_the_note_ledgers_are_left(tmp_path):
|
||||
"""Scope, asserted rather than described.
|
||||
|
||||
The same directory holds `.ids`, `.sync.ids` and `.derive.ids` for the note
|
||||
arms. Whether a surfaced NOTE should come back after a compaction is a
|
||||
different question with a different answer, and it is not being answered.
|
||||
A `rm` glob over `<sid>.*` would pass every assertion in the test above
|
||||
while silently deciding it.
|
||||
"""
|
||||
env = _env(tmp_path)
|
||||
sid = "sess-scope"
|
||||
rules = _ledger(tmp_path, sid, "rules.ids")
|
||||
notes = _ledger(tmp_path, sid, "ids")
|
||||
sync = _ledger(tmp_path, sid, "sync.ids")
|
||||
derive = _ledger(tmp_path, sid, "derive.ids")
|
||||
|
||||
_fire("compact", sid, env)
|
||||
|
||||
assert not rules.exists(), "the rule ledger should have been cleared"
|
||||
assert notes.exists() and sync.exists() and derive.exists(), (
|
||||
"a note ledger was cleared too. The note arms were deliberately left "
|
||||
"out of #3749 — clearing them is a decision about a different surface, "
|
||||
"and a glob that takes them along makes it by accident."
|
||||
)
|
||||
|
||||
|
||||
def test_a_compact_without_a_session_id_is_survivable(tmp_path):
|
||||
"""Defensive, because this hook's contract is fail-open.
|
||||
|
||||
An event with no `session_id` names no ledger. The hook must not error, and
|
||||
must not fall back to a wildcard — clearing every session's ledger on the
|
||||
machine because this one event was malformed is the worst available
|
||||
reading of "best effort".
|
||||
"""
|
||||
env = _env(tmp_path)
|
||||
other = _ledger(tmp_path, "someone-elses-session")
|
||||
|
||||
proc = subprocess.run(
|
||||
["bash", str(HOOK)],
|
||||
input=json.dumps({"source": "compact"}),
|
||||
capture_output=True, text=True, env=env, timeout=30,
|
||||
)
|
||||
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
assert other.exists(), (
|
||||
"an event with no session id cleared a ledger belonging to a different "
|
||||
"session"
|
||||
)
|
||||
Reference in New Issue
Block a user