CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 49s
CI & Build / TypeScript typecheck (push) Successful in 57s
CI & Build / Python tests (push) Failing after 1m7s
CI & Build / Build & push image (push) Skipped
Every hook opened `command -v jq >/dev/null 2>&1 || exit 0`, so on a machine without jq the operator got no session context, no rules, no prior art and no process sync — and not one word saying why, because `exit 0` is indistinguishable from "ran fine, nothing to say". jq is absent by default on macOS, on the Debian/Ubuntu slim images, on Alpine and in most CI containers. That is not a prerequisite to document; it is the plugin handing its own packaging problem to whoever installs it. `tac` was worse: GNU-only, so the prior-art hook's enclosing-definition arm did nothing at all on every Mac, silently, from the day it shipped. It is not replaced but removed — scribe_defs judges each line independently, so extracting forward and taking `tail -1` is the same answer as reversing and taking the head, and it drops the early-exit `head` that #4042 was filed for. No server contract changed, so a lagging plugin cache keeps working. scribe_json.awk JSON -> IDX<TAB>PATH<TAB>VALUE. Two modes: `whole` for an event or a response body, `lines` for a transcript, where an unparseable record is dropped and the rest still read — the `map(try fromjson catch empty)` the jq program opened with. Arrays also report their LENGTH at `[#]`, which is what keeps "zero notes" distinct from "no answer" (#2932). scribe_turn.awk the turn-bounding program, replacing the thirty lines of jq in the Stop hook. scribe_defs.sh scribe_json_flat / _pick / _list / _len / _list_minus read, scribe_json_out writes the envelope (five copies of one shape, gone), scribe_urlenc replaces `jq -sRr '@uri'`. Percent-encoding goes through `od -tu1` rather than an awk character loop on purpose: awk's idea of a character follows the locale, so gawk reads an accented letter as one and mawk as two, and an encoder built on substr() would emit a different URL depending on which awk is installed. Encoding is defined on bytes. Verified byte-identical to `jq -sRr '@uri'`. Measured, not assumed. The per-event path costs 8ms against jq's 3ms. The transcript path was 70x slower until two fixes: the Stop hook now finds where the turn starts with a fixed-string grep before parsing (a needle carrying unescaped quotes cannot occur inside a JSON string, so it matches only at a record's top level — checked against a full JSON parse of a 27MB transcript: 152 prompt records, 152 matches, no misses, no extras), and the parser reads each token out of a 1024-byte window instead of copying the rest of the buffer per token, which was quadratic in line length on the 400KB tool results a transcript carries. Differential-tested against the jq program it replaces over 724 windows cut from three real transcripts — 724 identical, 0 mismatched, 45 of them exercising a real task close and a real reply. That sweep is what caught `scribe_turn.awk` never setting FS, which truncated every multi-word reply at its first space and was invisible to a test whose replies were all empty. check_plugin.py's `jq -R` lint becomes a guard against either binary coming back, and three smoke checks lose their `shutil.which("jq")` skip. jq is not in `ci-python` either, so those three announced a skip on every CI run and had never once run there: removing the dependency from the product also closed a permanent hole in its verification. They pass now across all ten hooks. tests/test_hook_json_reader.py is a differential against Python's `json` over nested objects, arrays, unicode, escapes, control characters, empty cases and a value longer than the token window, plus the envelope, the encoder and the turn analyzer. 139 cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
173 lines
7.1 KiB
Python
173 lines
7.1 KiB
Python
"""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 ("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_the_note_ledgers_are_cleared_too_now_that_a_repeat_is_shown(tmp_path):
|
|
"""The decision this test used to pin, reversed — and why it could be.
|
|
|
|
It read: the note arms were deliberately left out of #3749, whether a
|
|
surfaced NOTE should come back after a compaction is a different question,
|
|
and a glob over `<sid>.*` would decide it by accident. That was right when
|
|
it was written, and the reason was the note arms' behaviour rather than the
|
|
note arms' nature: a note repeat was WITHHELD, so clearing their ledger
|
|
meant re-injecting whole menu lines the session had already been given.
|
|
Against that, leaving them was the cheaper mistake.
|
|
|
|
#4101 removed the premise. A note repeat is now rendered again with a
|
|
`seen` marker instead of dropped from the search, so the ledger no longer
|
|
decides whether a record is reachable — only whether its line says the
|
|
session has met it before. Across a compaction that marker becomes a false
|
|
statement: it tells a freshly-summarised session it has already seen a
|
|
record that is nowhere in its context. Keeping the ledger buys nothing and
|
|
asserts something untrue.
|
|
|
|
Kept as a named test rather than deleted, because the roster it guards
|
|
moved to tests/test_session_ledger_clear.py and what is worth keeping HERE
|
|
is the record that the answer changed, with the reason attached — a
|
|
deleted test takes the old decision's reasoning with 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 not (notes.exists() or sync.exists() or derive.exists()), (
|
|
"a note ledger outlived the context it describes, so its records are "
|
|
"marked `seen` to a session that can no longer see them"
|
|
)
|
|
|
|
|
|
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"
|
|
)
|