fix(plugin): the prompt boundary retrieves against prompts, not plumbing (#4200)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / Python tests (push) Successful in 1m30s
CI & Build / Build & push image (push) Successful in 17s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 10s
CI & Build / integration (push) Successful in 51s
CI & Build / TypeScript typecheck (push) Successful in 54s
CI & Build / Python tests (push) Successful in 1m30s
CI & Build / Build & push image (push) Successful in 17s
Claude Code submits more than typed words through UserPromptSubmit. A task notification, a slash-command echo and the caveat banner a local command prints all arrive as user turns, reaching `.prompt` indistinguishable from something the operator wrote. scribe_autoinject.sh believed all of them. The cost that matters is not the wasted embedding — it is the log row. Every such call counts in the denominator of every prompt-boundary surface, so delivery rate reads low for a reason unrelated to retrieval; and each refusal lands in `near_misses`, where a later tuning decision reads it as demand. Measured while taking milestone 399's acceptance (#3898): 15 of the top 20 `preference_slot` near-misses were `<task-notification>` blocks, all matching ONE record — #140 "Let each action land before starting the next" — all within thousandths of the 0.70 floor. A notification that an action finished really does resemble a preference about letting actions land. Lowering the floor to serve that apparent demand would have injected that record into every notification: the instrument arguing for the wrong fix, which is #379 again. scribe_skip_prompt is a PREFIX test, not a substring one, and that is the whole safety argument. A real prompt may contain one of these tags — an operator pasting a transcript, or a `<system-reminder>` after typed words — and must still be retrieved against. Nothing an operator types begins with a client envelope. The compaction-resume injection is deliberately NOT filtered: it is machine-written, but it summarises real work, and a resumed session is where recalling a rule earns its keep. Client-side only, so no server contract moves and lagging plugin caches keep working. The tags are Claude Code protocol constructs, identical on every install — instance-agnostic under rule 115. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy
This commit is contained in:
@@ -0,0 +1,117 @@
|
||||
"""The prompt boundary retrieves against PROMPTS, not against plumbing (#4200).
|
||||
|
||||
Claude Code submits more than typed words through `UserPromptSubmit`. A task
|
||||
notification, the echo of a slash command and the caveat banner a local command
|
||||
prints all arrive as user turns, and all reach `.prompt` looking exactly like
|
||||
something the operator wrote. `scribe_autoinject.sh` believed them.
|
||||
|
||||
WHY THIS IS TESTED AT ALL, given that retrieving against a notification almost
|
||||
never returns anything. The cost that matters is not the wasted embedding, it
|
||||
is the LOG ROW. Every one of those calls counts in the denominator of every
|
||||
prompt-boundary surface, so delivery rate reads low for a reason unrelated to
|
||||
retrieval quality; and each refusal lands in `near_misses`, where a later
|
||||
tuning decision reads it as demand. Measured on #3898: 15 of the top 20
|
||||
preference near-misses were `<task-notification>` blocks, all matching one
|
||||
record, all within thousandths of the floor. A floor lowered to serve that
|
||||
apparent demand would have injected that record into every notification — an
|
||||
instrument arguing for the wrong fix, which is exactly #379.
|
||||
|
||||
So the assertions split in two, and the SECOND half is the one guarding
|
||||
against the obvious wrong implementation:
|
||||
|
||||
* the client's envelopes are skipped;
|
||||
* a real prompt that merely CONTAINS one of those tags is NOT — because the
|
||||
cheap version of this filter is a substring search, and a substring search
|
||||
silences an operator who pastes a transcript or asks a question about a
|
||||
tag by name. The filter reads the FIRST token only.
|
||||
|
||||
The corpus below is drawn from real transcript shapes rather than invented,
|
||||
which is the reason the whitespace and trailing-`<system-reminder>` cases are
|
||||
here: both occur.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
DEFS = ROOT / "plugin" / "hooks" / "scribe_defs.sh"
|
||||
HOOK = ROOT / "plugin" / "hooks" / "scribe_autoinject.sh"
|
||||
|
||||
# Written by the client. None of these is a question anyone asked.
|
||||
SYNTHETIC = [
|
||||
"<task-notification>\n<task-id>b1oojuqu3</task-id>\n<output-file>/tmp/x</output-file>",
|
||||
"<command-name>/compact</command-name>",
|
||||
"<command-message>compact</command-message>",
|
||||
"<command-args>focus on the hooks</command-args>",
|
||||
"<local-command-stdout>Reconnected to plugin:scribe</local-command-stdout>",
|
||||
"<local-command-stderr>Failed to reconnect</local-command-stderr>",
|
||||
"<local-command-caveat>Caveat: The messages below were generated…</local-command-caveat>",
|
||||
" \n<task-notification>\n<task-id>abc</task-id>", # whitespace must not walk the filter
|
||||
]
|
||||
|
||||
# Typed by a person. Every one of these must still be retrieved against.
|
||||
REAL = [
|
||||
"please merge to main",
|
||||
"awesome go for 3898",
|
||||
"why does <task-notification> show up in the telemetry?", # names the tag
|
||||
"fix the hook<system-reminder>injected</system-reminder>", # tag, but trailing
|
||||
"This session is being continued from a previous conversation.", # see module docstring
|
||||
"<taskbar> is rendering wrong", # a tag, but not one of the client's
|
||||
"here is the transcript you asked for:\n<local-command-stdout>hi</local-command-stdout>",
|
||||
]
|
||||
|
||||
|
||||
def _skip(prompt: str) -> bool:
|
||||
"""Run the real predicate in the real shell — not a reimplementation."""
|
||||
if shutil.which("bash") is None: # pragma: no cover
|
||||
pytest.skip("hook runtime tool 'bash' not installed")
|
||||
script = f'set -uo pipefail\n. "{DEFS}"\nscribe_skip_prompt "$1" && echo SKIP || echo KEEP\n'
|
||||
out = subprocess.run(
|
||||
["bash", "-c", script, "_", prompt],
|
||||
capture_output=True, text=True, timeout=30,
|
||||
env={"PATH": os.environ["PATH"], "HOME": os.environ.get("HOME", "/tmp")},
|
||||
)
|
||||
assert out.returncode == 0, out.stderr
|
||||
verdict = out.stdout.strip()
|
||||
assert verdict in {"SKIP", "KEEP"}, out.stdout
|
||||
return verdict == "SKIP"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("prompt", SYNTHETIC, ids=lambda p: p[:28])
|
||||
def test_client_written_turns_are_skipped(prompt: str) -> None:
|
||||
assert _skip(prompt), f"retrieval would fire on a machine-written turn: {prompt[:60]!r}"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("prompt", REAL, ids=lambda p: p[:28])
|
||||
def test_operator_prompts_survive(prompt: str) -> None:
|
||||
assert not _skip(prompt), f"a real prompt was silenced: {prompt[:60]!r}"
|
||||
|
||||
|
||||
def test_the_filter_is_a_prefix_test_not_a_substring_test() -> None:
|
||||
"""The distinction the whole design rests on, asserted as one fact.
|
||||
|
||||
Stated separately from the parametrised cases because a future rewrite
|
||||
that reaches for `grep -q` or `case *"<tag>"*` passes nothing here, and
|
||||
the parametrised failure would read as "one odd input" rather than "the
|
||||
implementation changed shape".
|
||||
"""
|
||||
tag = "<task-notification>"
|
||||
assert _skip(tag + " trailing")
|
||||
assert not _skip("what is " + tag + "?")
|
||||
|
||||
|
||||
def test_the_hook_consults_the_filter_before_spending_a_request() -> None:
|
||||
"""Placement, not merely presence.
|
||||
|
||||
The guard is worthless below the `curl`: the point is that no row is
|
||||
logged, so it has to sit ahead of the request that would log one.
|
||||
"""
|
||||
body = HOOK.read_text()
|
||||
assert "scribe_skip_prompt" in body, "the hook does not consult the filter"
|
||||
assert body.index("scribe_skip_prompt") < body.index("curl -fsS"), \
|
||||
"the filter runs after the request it exists to prevent"
|
||||
Reference in New Issue
Block a user