diff --git a/corpus/skills/cat-mode/SKILL.md b/corpus/skills/cat-mode/SKILL.md index 251f8737..f687f704 100644 --- a/corpus/skills/cat-mode/SKILL.md +++ b/corpus/skills/cat-mode/SKILL.md @@ -265,7 +265,7 @@ agent switch, or resubmit is a fix, and none comes before the repro. **A factual or technical claim gets a real repro script, not a history search.** Judging an old comment or a "probably confabulated" suspicion needs an actual attempt under the claimed conditions, not a `git log` sweep. No citation means "never verified," not "false." -**Unhedged root-cause or fix claims about live system behavior need instrument-level proof in the same message, or a `{{CAT-UNVERIFIED}}` tag naming the blocker.** The gate is the claim type, not a hedge word. Invoking `/prove-it` once does not arm it for later claims. Any hedge auto-runs prove-it in the same turn — a hedge is a trigger to verify, never a place to stop. +**Unhedged root-cause or fix claims about live system behavior need instrument-level proof in the same message, or a `{{CAT-UNVERIFIED: -- cannot verify: }}` tag naming the blocker.** The gate is the claim type, not a hedge word. Invoking `/prove-it` once does not arm it for later claims. Any hedge auto-runs prove-it in the same turn — a hedge is a trigger to verify, never a place to stop. Outputs carry failures explicitly (a status column, an error row), never dropped — [[principle-explicit-errors]]. diff --git a/corpus/skills/principle-subagent-inherits-scope/SKILL.md b/corpus/skills/principle-subagent-inherits-scope/SKILL.md index 3ef54457..042a9601 100644 --- a/corpus/skills/principle-subagent-inherits-scope/SKILL.md +++ b/corpus/skills/principle-subagent-inherits-scope/SKILL.md @@ -61,8 +61,33 @@ the work in a single round trip. until verified — `engine/hooks/agent-relay-attribution` flags the shape. - **State the scope in the prompt, not in your head.** An unstated boundary is not inherited. Name the files, the write authority, and the question. +- **A brief carries three things, not two: facts, the question, and + decisions.** A decision the user already made is neither a fact to weigh + nor a question to answer. Relay it as a constraint sentence that says it is + settled — "the toggle gates the re-injection, not the emission; that is + decided, do not re-open it." Filed under the question, it reads as + something to work out, and a subagent that re-opens it looks rigorous while + discarding the only part the user owned outright. Repeating the user's + words is not enough on its own: a brief can carry the requirement verbatim + and still lose it by appending one open question beside it. - **Don't prime the answer.** Hand over the facts and the question. A - subagent told what you expect finds roughly that. + subagent told what you expect finds roughly that. This governs findings, + never decisions. Leaving out a decision the user already made is not + neutrality — it is dropping a constraint, and the anti-priming rule then + rewards re-opening it. + +Prior art for the third slot: Orlena Gotel and Anthony Finkelstein, "An +analysis of the requirements traceability problem", Proc. IEEE International +Conference on Requirements Engineering, https://doi.org/10.1109/ICRE.1994.292398 +— pre-requirements-specification traceability exists so a requirement keeps +its link to the stakeholder who set it; without that link it gets +renegotiated by people who do not own it. Read this citation as +single-source: Crossref's `issued` field for the record is null, so the 1994 +date is inferred from the DOI string and the conference rather than confirmed +by metadata, and Crossref renders the second author as "C.W. Finkelstein" +while the paper is normally cited as Anthony Finkelstein. IEEE Xplore +returned an empty body and ACM DL returned 403, so no publisher page was +read. ## Related diff --git a/corpus/skills/principle-subagent-inherits-scope/tests/fires_example.md b/corpus/skills/principle-subagent-inherits-scope/tests/fires_example.md index 2652aec9..cb7368de 100644 --- a/corpus/skills/principle-subagent-inherits-scope/tests/fires_example.md +++ b/corpus/skills/principle-subagent-inherits-scope/tests/fires_example.md @@ -20,3 +20,18 @@ grounds that it was told not to. The link to and conventions are not concurrency control, so a read-only brief is not filesystem isolation. A subagent that may write gets its own worktree, and the parent that omitted one has not stated the boundary at all. + +A third shape, the one the decisions slot exists for. The user has already +decided which stage a new toggle gates. The parent relays that requirement +word for word and then appends "work out what a toggle would actually gate." +A mechanical containment check on the delegation passes — the requirement is +verbatim-contained and every content word is present — and the subagent still +ranks the user's own requirement fourth of six and argues its premise away. +The skill fires here because the decision was relayed as part of the +question instead of as a settled constraint, and because the anti-priming +rule reads re-opening it as rigour. + +No mechanical catch is claimed for that third shape, and the obvious one is +known not to work: the containment check returns PASS on the exact +delegation that drifted, because the words were all there. The gate is the +parent's wording, so this stays an `unchecked` case pinned by prose. diff --git a/corpus/skills/principle-subagent-inherits-scope/tests/stays_silent_example.md b/corpus/skills/principle-subagent-inherits-scope/tests/stays_silent_example.md index 4dcd585d..0dbd6090 100644 --- a/corpus/skills/principle-subagent-inherits-scope/tests/stays_silent_example.md +++ b/corpus/skills/principle-subagent-inherits-scope/tests/stays_silent_example.md @@ -8,3 +8,11 @@ whose authority could exceed the parent's. The principle has no target: its four limits all describe what a delegate may do, and the contradiction contract describes how a delegate reports back. A single agent editing one file on its own behalf is the case this principle is not about. + +A second silent shape, against the decisions slot. A parent hands an explorer +a read-only brief that names the files, the question, and one settled +decision relayed as a constraint the explorer may not re-open. The explorer +reads those files, answers the question, and reports one contradiction with a +`file:line` and the ref it was read at, leaving the decision alone. Nothing +here is a widening: the boundary was stated, the constraint travelled as a +constraint, and the contradiction went back up rather than being acted on. diff --git a/engine/hooks/diu-stop/README.md b/engine/hooks/diu-stop/README.md index d9fef8ae..9e27abfe 100644 --- a/engine/hooks/diu-stop/README.md +++ b/engine/hooks/diu-stop/README.md @@ -13,6 +13,26 @@ last appeared before however many compactions have happened since. The claim check reads only the main agent's turn-final message: of 337 unproven claims found in stored transcripts, 196 were mid-turn or subagent text it never saw. See [`COVERAGE.md`](COVERAGE.md) before reading its silence as clearance. +## What buys a paragraph its silence + +A fenced block of output, inline code that looks like output, a well-formed +`{{CAT-UNVERIFIED: ... -- cannot verify: ...}}` tag, or a file citation -- +and a citation has to be backed. `path:line` on its own used to silence a +paragraph with no check that the file existed, that anyone read it, or at +what ref; a made-up path silenced the gate exactly as well as a real one. A +citation now counts when it names the ref it was read at (`path:line @ +origin/main`, the form `corpus/CLAUDE.learned.md` already asks for in prose), +or when the session's transcript shows a tool call that named that path. + +Three outcomes, not two. When the transcript cannot be read, whether the path +was read is *unchecked*: the citation does not buy silence, and the block says +which path it could not check and that the ref would settle it. + +The marker check reads prose only. A marker inside a fence or a pair of +backticks is being shown, not used, so explaining the tag, quoting the rule +that defines it, or relaying this gate's own refusal word for word all stay +silent. A marker in running prose is a use and still counts. + Not one file per harness, because there is no single "stop" mechanism shared by every harness -- each one has a genuinely different amount of power at that point: diff --git a/engine/hooks/diu-stop/claude_stop_check.py b/engine/hooks/diu-stop/claude_stop_check.py index 125ec520..840eded1 100755 --- a/engine/hooks/diu-stop/claude_stop_check.py +++ b/engine/hooks/diu-stop/claude_stop_check.py @@ -45,6 +45,7 @@ Every block names every flagged sentence, so one rewrite that fixes them all gets through. """ +import json import os import re import sys @@ -90,6 +91,9 @@ FENCED_BODY_RE = re.compile(r"```[^\n]*\n(.*?)```", re.DOTALL) INLINE_CODE_RE = re.compile(r"`([^`\n]+)`") FILE_LINE_RE = re.compile(r"(? |\+\+\+ |--- |@@ |diff --git|commit [0-9a-f]{7,}|[0-9a-f]{7,10} )" r"|Traceback|^\s*at [\w.$<>]+ \(.*:\d+:\d+\)" @@ -144,15 +148,37 @@ def _opening_word(message): return match.group(0).lower() if match else "" +def prose_only(message): + """`message` with fenced blocks and inline code removed. + + A marker inside a fence or a pair of backticks is being shown, not used: + explaining the tag, quoting the rule that defines it, or pasting a gate's + own message back to the user all put the token on screen without claiming + anything. `find_unverified_claims` has stripped both for a while; the + marker check read the raw message, so the gate fired on the sentence that + taught the reader how not to trip it. + + An unterminated fence leaves a `\u0060\u0060\u0060` behind after the + substitution. Everything from that marker on is inside a code block that + never closed, so it is dropped too. + """ + prose = INLINE_CODE_RE.sub("", FENCED_BODY_RE.sub("", message or "")) + if FENCE_MARKER in prose: + prose = prose[:prose.rindex(FENCE_MARKER)] + return prose + + def find_marker_problems(message): """Return the marker complaints this message earns, in report order. A tag that names no blocker, and the retired bare `UNVERIFIED:`, each - draw their own message. Both can be present at once.""" + draw their own message. Both can be present at once. Only prose counts -- + see `prose_only`.""" + prose = prose_only(message) problems = [] - if markers.malformed_tags(message): + if markers.malformed_tags(prose): problems.append(markers.MALFORMED_TAG_MESSAGE) - if markers.has_legacy_marker(message): + if markers.has_legacy_marker(prose): problems.append(markers.LEGACY_MARKER_MESSAGE) return problems @@ -189,7 +215,76 @@ def _paragraph_claim(para): return None -def find_unverified_claims(message): +def cited_paths(text): + """The path part of every file:line in `text`, longest first.""" + seen = [] + for match in FILE_LINE_RE.finditer(text): + path = LINE_SUFFIX_RE.split(match.group(0))[0] + if path and path not in seen: + seen.append(path) + return sorted(seen, key=len, reverse=True) + + +def read_evidence(event): + """Everything this session handed a tool, as one string, or None. + + None means the check could not run -- no transcript to read, or the file + would not open. That is a third outcome, not a clean one: a citation whose + read cannot be checked does not buy silence, and the finding says why. + """ + path = event.get("transcript_path") if isinstance(event, dict) else None + if not isinstance(path, str) or not path: + return None + parts = [] + try: + with open(path, encoding="utf-8") as handle: + for line in handle: + line = line.strip() + if not line: + continue + try: + entry = json.loads(line) + except json.JSONDecodeError: + continue + if not isinstance(entry, dict): + continue + inner = entry.get("message") + content = inner.get("content") if isinstance(inner, dict) else entry.get("content") + if not isinstance(content, list): + continue + for block in content: + if isinstance(block, dict) and block.get("type") == "tool_use": + parts.append(json.dumps(block.get("input"), default=str)) + except (OSError, UnicodeError) as exc: + print( + f"catstack-hook-error diu-stop: cannot read {path}, so which files " + f"were read this session is unchecked: {exc}", + file=sys.stderr, + ) + return None + return "\n".join(parts) + + +def citation_earns_silence(para, read_blob): + """(exempt, unchecked) for the file:line citations in one paragraph. + + A bare `file.ts:99` used to silence a paragraph on its own, with no check + that the file exists, that anyone read it, or at what ref. A fabricated + path silenced the gate exactly as well as a real one. It now has to carry + the ref it was read at (`path:line @ origin/main`), which is what + corpus/CLAUDE.learned.md already asks for in prose, or the session has to + show a tool call that named that path. + """ + if not FILE_LINE_RE.search(para): + return False, False + if CITATION_REF_RE.search(para): + return True, False + if read_blob is None: + return False, True + return any(path in read_blob for path in cited_paths(para)), False + + +def find_unverified_claims(message, read_blob=None): """Return one (trigger phrase, sentence) pair for every paragraph that makes an unverified-shaped claim with no evidence marker in that same paragraph, in message order. @@ -209,7 +304,8 @@ def find_unverified_claims(message): continue if markers.excuses_paragraph(para): continue - if FILE_LINE_RE.search(para): + exempt, _unchecked = citation_earns_silence(para, read_blob) + if exempt: continue inline = INLINE_CODE_RE.findall(para) if inline and (fenced_output or any(OUTPUT_SHAPE_RE.search(code) for code in inline)): @@ -221,13 +317,26 @@ def find_unverified_claims(message): return claims -def find_unverified_claim(message): +def find_unverified_claim(message, read_blob=None): """Return the first offending phrase find_unverified_claims reports, or None.""" - claims = find_unverified_claims(message) + claims = find_unverified_claims(message, read_blob) return claims[0][0] if claims else None +def unchecked_citations(message, read_blob): + """Paths cited in a flagged paragraph whose read could not be checked.""" + if read_blob is not None: + return [] + found = [] + for para in re.split(r"\n\s*\n", message): + para = FENCED_BODY_RE.sub("", para) + _exempt, unchecked = citation_earns_silence(para, read_blob) + if unchecked: + found.extend(path for path in cited_paths(para) if path not in found) + return found + + def detect(event): if event.get("agent_id"): return [] @@ -239,7 +348,8 @@ def detect(event): word_count = counted_words(message) over_limit = word_count > WORD_LIMIT and not retry - claims = find_unverified_claims(message) + read_blob = read_evidence(event) + claims = find_unverified_claims(message, read_blob) marker_problems = find_marker_problems(message) findings = [] @@ -260,11 +370,18 @@ def detect(event): for number, (phrase, sentence) in enumerate(claims, 1): lines.append(f"{number}. \"{sentence}\" (trigger: \"{' '.join(phrase.split())}\")") lines.append( - "A backticked name or command alone is not output. Per " - "skills/prove-it/SKILL.md: for each one, either paste the output " - "of what was actually run/checked in its paragraph, or -- only if " - "the check cannot run -- tag the claim there and say why." + "A backticked name or command alone is not output, and neither is a " + "bare file:line. Per skills/prove-it/SKILL.md: for each one, either " + "paste the output of what was actually run/checked in its paragraph, " + "cite it as `path:line @ `, or -- only if the check cannot run " + "-- tag the claim there and say why." ) + unchecked = unchecked_citations(message, read_blob) + if unchecked: + lines.append( + "This turn's transcript could not be read, so whether " + f"{', '.join(unchecked)} was read this session is UNCHECKED, not " + "clear. Add the ref it was read at to the citation.") claim_message = "\n".join(lines) findings.append(Finding( rule_id=RULE_UNVERIFIED_CLAIM, diff --git a/engine/hooks/diu-stop/tests/test_hooks.py b/engine/hooks/diu-stop/tests/test_hooks.py index 0e6dd831..6dbd6166 100644 --- a/engine/hooks/diu-stop/tests/test_hooks.py +++ b/engine/hooks/diu-stop/tests/test_hooks.py @@ -23,9 +23,11 @@ FIXTURES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "fixtures") LLM_JUDGE_DIR = os.path.join(os.path.dirname(HOOKS_DIR), "llm-judge") sys.path.insert(0, LLM_JUDGE_DIR) +sys.path.insert(0, os.path.join(os.path.dirname(HOOKS_DIR), "_markers")) sys.path.insert(0, HOOKS_DIR) import claude_prompt_reminder # noqa: E402 +import markers # noqa: E402 import claude_stop_check # noqa: E402 import codex_notify # noqa: E402 import install_claude_hook # noqa: E402 @@ -270,6 +272,53 @@ def test_legacy_bare_marker_is_blocked_and_names_the_new_tag(self): self.assertIn("CAT-UNVERIFIED", err) self.assertIn("prove-it", err.lower()) + def test_a_backticked_mention_of_the_tag_is_silent(self): + """Explaining the mechanism is not using it.""" + message = ( + "The escape hatch is `{{CAT-UNVERIFIED}}` and it has to name a blocker " + "after the colon, or the gate rejects it.") + self.assertEqual(claude_stop_check.find_marker_problems(message), []) + + def test_a_mention_inside_a_fence_is_silent(self): + message = ( + "Here is the shape the gate wants:\n" + "```\n" + "{{CAT-UNVERIFIED}}\n" + "UNVERIFIED: the old one\n" + "```\n" + "Use the first form and name the blocker.") + self.assertEqual(claude_stop_check.find_marker_problems(message), []) + + def test_quoting_the_cat_mode_rule_verbatim_is_silent(self): + """The line that defines the rule must not trip the gate enforcing it.""" + message = ( + "The rule is: **Unhedged root-cause or fix claims about live system " + "behavior need instrument-level proof in the same message, or a " + "`{{CAT-UNVERIFIED}}` tag naming the blocker.**") + self.assertEqual(claude_stop_check.find_marker_problems(message), []) + blocked, err = run_claude_check({"last_assistant_message": message}) + self.assertNotIn("names no blocker", err) + self.assertFalse(blocked) + + def test_relaying_the_gates_own_refusal_is_silent(self): + """cat-mode asks for a gate's message word for word; that must be safe.""" + message = "The gate said:\n\n" + markers.MALFORMED_TAG_MESSAGE + self.assertEqual(claude_stop_check.find_marker_problems(message), []) + + def test_an_unclosed_fence_does_not_leak_a_mention_back_into_prose(self): + message = "Example:\n```\n{{CAT-UNVERIFIED}}\n" + self.assertEqual(claude_stop_check.find_marker_problems(message), []) + + def test_a_real_tag_with_no_blocker_in_prose_still_fires(self): + message = "Confirmed the crash loop. {{CAT-UNVERIFIED: the loop is real}}" + self.assertIn( + markers.MALFORMED_TAG_MESSAGE, claude_stop_check.find_marker_problems(message)) + + def test_a_bare_legacy_marker_in_prose_still_fires(self): + message = "UNVERIFIED: the crash loop is real." + self.assertIn( + markers.LEGACY_MARKER_MESSAGE, claude_stop_check.find_marker_problems(message)) + def test_retry_still_checks_a_new_claim(self): message = ( "Correction on scope: that only covers the root chain. " @@ -409,7 +458,14 @@ def test_fenced_block_does_not_silence_later_prose_in_same_paragraph(self): self.assertTrue(blocked) self.assertIn("this fixes it", err.lower()) - def test_file_line_citation_still_silences_claim_after_fence_normalization(self): + def test_a_cited_file_that_was_read_still_silences_after_fence_normalization(self): + """PR #479's invariant, kept: fence normalization leaves a real citation alone. + + #479 carried the file:line exemption over untouched as a Non-goal; it + did not decide that a path nobody read should silence anything. The + citation here is now backed the way the exemption always claimed to be + -- the session read that file. + """ message = ( "I checked the relevant snippet.\n" "```ts\n" @@ -417,10 +473,64 @@ def test_file_line_citation_still_silences_claim_after_fence_normalization(self) "```\n" "The issue was the stale guard at file.ts:1276." ) - self.assertIsNone(claude_stop_check.find_unverified_claim(message)) + read = json.dumps({"file_path": "/repo/src/file.ts"}) + self.assertIsNone(claude_stop_check.find_unverified_claim(message, read)) + transcript = self.transcript_reading("/repo/src/file.ts") + blocked, err = run_claude_check( + {"last_assistant_message": message, "transcript_path": transcript}) + self.assertFalse(blocked) + self.assertNotIn("unverified-shaped", err) + + def transcript_reading(self, *paths): + """A transcript whose tool calls name `paths`, written to a tempdir.""" + import tempfile + directory = tempfile.mkdtemp() + self.addCleanup(__import__("shutil").rmtree, directory, True) + target = os.path.join(directory, "session.jsonl") + with open(target, "w", encoding="utf-8") as handle: + for path in paths: + handle.write(json.dumps({ + "type": "assistant", + "message": {"role": "assistant", "content": [ + {"type": "tool_use", "name": "Read", "input": {"file_path": path}}]}, + }) + "\n") + return target + + def test_a_fabricated_path_no_longer_silences_the_claim(self): + """The A/B/C sweep's C case: the path exists nowhere and was never read.""" + message = ( + "The issue was the stale guard at " + "totally-made-up-file-that-does-not-exist.ts:99999." + ) + read = json.dumps({"file_path": "/repo/src/file.ts"}) + self.assertIsNotNone(claude_stop_check.find_unverified_claim(message, read)) + transcript = self.transcript_reading("/repo/src/file.ts") + blocked, err = run_claude_check( + {"last_assistant_message": message, "transcript_path": transcript}) + self.assertTrue(blocked) + self.assertIn("bare file:line", err) + + def test_a_citation_carrying_its_ref_silences_without_any_transcript(self): + """The B case: the citation says where it was read, so it stands alone.""" + message = "The issue was the stale guard at src/file.ts:1276 @ origin/main." + self.assertIsNone(claude_stop_check.find_unverified_claim(message, "")) blocked, err = run_claude_check({"last_assistant_message": message}) self.assertFalse(blocked) - self.assertEqual(err, "") + self.assertNotIn("unverified-shaped", err) + + def test_an_unreadable_transcript_makes_a_citation_unchecked_not_clear(self): + message = "The issue was the stale guard at src/file.ts:1276." + self.assertIsNotNone(claude_stop_check.find_unverified_claim(message, None)) + blocked, err = run_claude_check( + {"last_assistant_message": message, "transcript_path": "/no/such/transcript.jsonl"}) + self.assertTrue(blocked) + self.assertIn("UNCHECKED", err) + self.assertIn("src/file.ts", err) + + def test_a_read_path_named_only_by_a_grep_still_silences(self): + message = "The issue was the stale guard at src/file.ts:1276." + read = json.dumps({"pattern": "guard", "path": "src/file.ts"}) + self.assertIsNone(claude_stop_check.find_unverified_claim(message, read)) def test_hedge_i_think_it_happened_without_evidence_is_flagged(self): message = "I think the deploy happened around 2am, so that's why the build is stale." diff --git a/engine/hooks/hooks.toml b/engine/hooks/hooks.toml index 6b371efd..5d727c17 100644 --- a/engine/hooks/hooks.toml +++ b/engine/hooks/hooks.toml @@ -220,6 +220,7 @@ summary = "Stops driving the user's keyboard or screen." mode = "stop" why_mode = "attention" summary = "Stops when unchecked claims pile up." +enabled_by = "CATSTACK_UNVERIFIED_TAG_REMINDER" [hooks.verdict-flip-watch] mode = "warn" diff --git a/engine/hooks/llm-judge/phrases/wrong-check-reflect.json b/engine/hooks/llm-judge/phrases/wrong-check-reflect.json index 0aaf4200..ee2730b7 100644 --- a/engine/hooks/llm-judge/phrases/wrong-check-reflect.json +++ b/engine/hooks/llm-judge/phrases/wrong-check-reflect.json @@ -1,7 +1,7 @@ { "checker": "wrong-check-reflect", "reads": "reply", - "meaning": "The latest assistant reply admits that something it told the user earlier was wrong, misread, or answered the wrong question.", + "meaning": "The latest assistant reply admits that something it told the user earlier was wrong, misread, or answered the wrong question, OR that an earlier claim of its own went out before the check backing it had run. A reply saying the claim turned out correct but was stated before it was checked IS a match: the failure is the order, not the answer, so no word in the reply has to say anything was wrong.", "match": [ "my earlier check was wrong", "You're right, I misread that", @@ -9,7 +9,9 @@ "my mistake", "I was wrong about the path", "Correction: the file I pointed you to earlier is not the one in use; the real one is src/b.py.", - "Good catch. The earlier number was off; the real count is 12." + "Good catch. The earlier number was off; the real count is 12.", + "Correcting one claim and arming the check I implied:", + "I was right - but I said it a turn before I checked it" ], "not_match": [ "You're right. Let's go with option B.", diff --git a/engine/hooks/unverified-tag-ledger/README.md b/engine/hooks/unverified-tag-ledger/README.md index ccfa1d8c..243db7d7 100644 --- a/engine/hooks/unverified-tag-ledger/README.md +++ b/engine/hooks/unverified-tag-ledger/README.md @@ -40,7 +40,28 @@ fixtures in `tests/test_hooks.py`. - **UserPromptSubmit** (`claude_prompt_reminder.py`) — lists outstanding claims on the next prompt, quoting the rule and naming each claim plus what it is blocked on. The next prompt is the earliest point a reminder can change - behaviour without preventing the turn from ending at all. + behaviour without preventing the turn from ending at all. How much it lists + is `CATSTACK_UNVERIFIED_TAG_REMINDER` (see Env). + +## Env + +| Var | Effect | +|-----|--------| +| `CATSTACK_UNVERIFIED_TAG_REMINDER=stale` | Default, and what an unset flag means. Re-inject only claims that have already survived `ESCALATE_AFTER_TURNS` (3) turns. | +| `CATSTACK_UNVERIFIED_TAG_REMINDER=all` | Re-inject every outstanding claim on every prompt. | +| `CATSTACK_UNVERIFIED_TAG_REMINDER=off` | No re-injection at all. Rows are still recorded and the Stop refusal still runs. | +| `CATSTACK_TAG_LEDGER_DIR` | Where the per-session ledger lives (the tests use a tempdir). | + +The gate sits on the injection and nowhere else. Gating the tag itself would +hide the unverified claim rather than stop it, which is the opposite of what +the ledger is for, and `off` would then also empty the ledger it is mined +from. Any value other than the three above is named on stderr and falls back +to `stale`; an `.env` candidate that exists and cannot be read is reported the +same way rather than passing as "not set". + +The hook resolves this flag itself through `engine/hooks/_flags/flags.py`. +The `enabled_by` line in `engine/hooks/hooks.toml` records which flag the hook +answers to; nothing reads that field, so it is documentation, not the gate. - **Discharge** — a claim is resolved when a later turn runs a verification tool (`Bash`, `Read`, `Grep`, `Glob`, `NotebookRead`) and stops re-emitting it. - **Where the turn's tool list comes from** — the transcript named by @@ -53,6 +74,12 @@ fixtures in `tests/test_hooks.py`. reason is written to stderr. - **Escalation** — a claim outstanding `ESCALATE_AFTER_TURNS` (3) turns or more is reported as a reflect trigger rather than accumulating quietly. +- **Discharge is itself a reflect trigger** — a row going outstanding -> + discharged is the record of a claim that went out first and was checked + after. That is an evidence-order miss, and it carries no wrongness word, so + the phrase scanners (`engine/skills/reflect/scripts/self_retraction_scan.py`, + and the `wrong-check-reflect` dictionary) cannot see it from the text. This + hook sees it from state instead, and says so on the Stop that discharges. Malformed tags are deliberately ignored here; `diu-stop` already rejects those. diff --git a/engine/hooks/unverified-tag-ledger/claude_prompt_reminder.py b/engine/hooks/unverified-tag-ledger/claude_prompt_reminder.py index 6e95a101..7de7f625 100644 --- a/engine/hooks/unverified-tag-ledger/claude_prompt_reminder.py +++ b/engine/hooks/unverified-tag-ledger/claude_prompt_reminder.py @@ -3,13 +3,18 @@ earlier turns deferred and never settled. This is where cat-mode/SKILL.md:269 gets teeth -- the Stop hook cannot block the turn that emits a tag without deadlocking, so the reminder lands on the next prompt instead. + +How much it says is CATSTACK_UNVERIFIED_TAG_REMINDER: off, stale (default), or +all. The gate is here, on the injection, and nowhere else -- rows keep being +recorded on every setting, because hiding the tag would hide the unverified +claim instead of stopping it. """ from __future__ import annotations import json import sys -from detect import reminder +from detect import reminder, reminder_mode def main() -> None: @@ -18,8 +23,12 @@ def main() -> None: except (json.JSONDecodeError, OSError) as exc: sys.stderr.write(f"unverified-tag-ledger: unreadable payload, no reminder: {exc!r}\n") return + payload = payload if isinstance(payload, dict) else {} try: - text = reminder(str((payload or {}).get("session_id") or "")) + mode, note = reminder_mode(cwd=payload.get("cwd")) + if note: + sys.stderr.write(note + "\n") + text = reminder(str(payload.get("session_id") or ""), mode) except Exception as exc: sys.stderr.write(f"unverified-tag-ledger: reminder error, continuing: {exc!r}\n") return diff --git a/engine/hooks/unverified-tag-ledger/detect.py b/engine/hooks/unverified-tag-ledger/detect.py index 8a47c2ef..0fa280a3 100644 --- a/engine/hooks/unverified-tag-ledger/detect.py +++ b/engine/hooks/unverified-tag-ledger/detect.py @@ -27,13 +27,29 @@ sys.path.insert(0, os.path.join( os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "_markers")) +sys.path.insert(0, os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "_flags")) +import flags # noqa: E402 import markers # noqa: E402 VERIFY_TOOLS = {"Bash", "Read", "Grep", "Glob", "NotebookRead"} ESCALATE_AFTER_TURNS = 3 MAX_LISTED = 5 +REMINDER_FLAG = "CATSTACK_UNVERIFIED_TAG_REMINDER" +REMINDER_MODES = ("off", "stale", "all") +DEFAULT_REMINDER_MODE = "stale" + +DISCHARGE_REFLECT = ( + "unverified-tag-ledger: {count} claim(s) went from unverified to checked this turn: " + "{claims}. That transition is the whole event: the claim went out first and the check " + "ran after. No wording has to admit anything for this to be true, which is why the " + "phrase scanners miss it -- an evidence-order miss carries no wrongness word. " + "Treat it as a reflect trigger, not a milestone: run reflect on this transcript, or " + "say plainly why this one does not need it." +) + CLAIM_RE = re.compile( r"\{\{\s*CAT-UNVERIFIED\s*:?\s*(?P.*?)(?:--|—)\s*cannot\s+verify\s*:\s*(?P[^}]*)\}\}", re.IGNORECASE | re.DOTALL, @@ -135,9 +151,41 @@ def record_turn(session_id: str, message: str, tools_used: set[str] | None, now= return rows -def reminder(session_id: str) -> str: - """Text for UserPromptSubmit, or empty when nothing is outstanding.""" +def reminder_mode(environ=None, cwd=None, home=None) -> tuple[str, str]: + """(mode, note). mode is off, stale, or all; note names what could not be read. + + Three settings, not two, because the complaint is volume and not the + ledger. `off` silences the next-prompt reminder and keeps recording rows, + so the ledger stays minable either way. `stale` -- the default -- reminds + only about claims that have already survived ESCALATE_AFTER_TURNS turns, + which is the subset this hook already singles out as a reflect trigger. + `all` is the older behaviour, every outstanding claim every prompt. + + Unset means `stale`, deliberately. A flag whose unset value is the old + behaviour changes nothing for the person who asked for less. + """ + found = flags.resolve_flag( + REMINDER_FLAG, os.environ if environ is None else environ, cwd, home) + note = found.unreadable_note(REMINDER_FLAG) + raw = (found.value or "").strip().lower() + if raw in REMINDER_MODES: + return raw, note + if raw: + extra = ( + f"unverified-tag-ledger: {REMINDER_FLAG}={found.value!r} is not " + f"{', '.join(REMINDER_MODES)}; using {DEFAULT_REMINDER_MODE}.") + note = f"{note}\n{extra}" if note else extra + return DEFAULT_REMINDER_MODE, note + + +def reminder(session_id: str, mode: str = DEFAULT_REMINDER_MODE) -> str: + """Text for UserPromptSubmit, or empty when nothing is due.""" + if mode == "off": + return "" open_rows = outstanding(read_ledger(session_id)) + if mode != "all": + open_rows = [row for row in open_rows + if row.get("turns", 0) >= ESCALATE_AFTER_TURNS] if not open_rows: return "" stale = [row for row in open_rows if row.get("turns", 0) >= ESCALATE_AFTER_TURNS] @@ -177,18 +225,26 @@ def evaluate(payload: dict) -> dict: session_id = str(payload.get("session_id") or "") message = _last_assistant_text(payload) tools = tools_used_this_turn(payload) + was_open = {row["claim"] for row in outstanding(read_ledger(session_id))} rows = record_turn(session_id, message, tools) + notes = [] + discharged = sorted( + row["claim"] for row in rows if row.get("resolved") and row["claim"] in was_open) + if discharged: + notes.append(DISCHARGE_REFLECT.format( + count=len(discharged), claims="; ".join(discharged[:MAX_LISTED]))) new_claims = {tag["claim"] for tag in parse_tags(message)} if not new_claims: - return {"note": "", "block": ""} + return {"note": "\n".join(notes), "block": ""} if tools is None: - return {"note": ( + notes.append( f"unverified-tag-ledger: logged {len(new_claims)} CAT-UNVERIFIED claim(s), but this " "turn's tool calls could not be read from transcript_path (see the line above), so " "whether a check was attempted is UNCHECKED, not clean. Nothing was discharged and " - "the turn was not refused."), "block": ""} + "the turn was not refused.") + return {"note": "\n".join(notes), "block": ""} if not tools & VERIFY_TOOLS and not payload.get("stop_hook_active"): claims = "; ".join(sorted(new_claims)[:MAX_LISTED]) @@ -201,12 +257,12 @@ def evaluate(payload: dict) -> dict: fresh = [row for row in rows if row["claim"] in new_claims and not row.get("resolved") and row.get("turns", 0) == 0] - if not fresh: - return {"note": "", "block": ""} - return {"note": ( - f"unverified-tag-ledger: logged {len(fresh)} CAT-UNVERIFIED claim(s) against this session. " - "They are deferred, not discharged, and will be raised again next turn " - "(cat-mode/SKILL.md:269)."), "block": ""} + if fresh: + notes.append( + f"unverified-tag-ledger: logged {len(fresh)} CAT-UNVERIFIED claim(s) against this " + "session. They are deferred, not discharged, and will be raised again next turn " + "(cat-mode/SKILL.md:269).") + return {"note": "\n".join(notes), "block": ""} def decide_stop(payload: dict) -> str: diff --git a/engine/hooks/unverified-tag-ledger/tests/test_hooks.py b/engine/hooks/unverified-tag-ledger/tests/test_hooks.py index 12ab9931..7752ef19 100644 --- a/engine/hooks/unverified-tag-ledger/tests/test_hooks.py +++ b/engine/hooks/unverified-tag-ledger/tests/test_hooks.py @@ -38,6 +38,9 @@ "-- cannot verify: my own reasoning isn't observable by any command}}") MALFORMED = "{{CAT-UNVERIFIED: something I did not check}}" +EVIDENCE_ORDER_1 = "Correcting one claim and arming the check I implied:" +EVIDENCE_ORDER_2 = "I was right - but I said it a turn before I checked it" + def _real_transcript_lines() -> list[str]: with open(REAL_TRANSCRIPT, encoding="utf-8") as handle: @@ -191,11 +194,63 @@ def test_an_unreadable_turn_is_neither_blocked_nor_called_clean(self) -> None: def test_reminder_names_the_claim_and_cites_the_rule(self) -> None: self.detect.record_turn("s1", REAL_TAG_1, set()) - text = self.detect.reminder("s1") + text = self.detect.reminder("s1", "all") self.assertIn("widened scope", text) self.assertIn("cat-mode/SKILL.md:269", text) self.assertIn("never a place to stop", text) + def test_reminder_is_silent_on_off(self) -> None: + self.detect.record_turn("s1", REAL_TAG_1, set()) + for _ in range(self.detect.ESCALATE_AFTER_TURNS): + self.detect.record_turn("s1", REAL_TAG_1, set()) + self.assertEqual(self.detect.reminder("s1", "off"), "") + + def test_a_young_claim_is_not_reinjected_by_default(self) -> None: + self.detect.record_turn("s1", REAL_TAG_1, set()) + self.detect.record_turn("s1", REAL_TAG_1, set()) + rows = self.detect.read_ledger("s1") + self.assertEqual(rows[0]["turns"], 1) + self.assertEqual(self.detect.reminder("s1"), "") + + def test_a_claim_that_survives_three_turns_is_reinjected_by_default(self) -> None: + self.detect.record_turn("s1", REAL_TAG_1, set()) + for _ in range(self.detect.ESCALATE_AFTER_TURNS): + self.detect.record_turn("s1", REAL_TAG_1, set()) + text = self.detect.reminder("s1") + self.assertIn("widened scope", text) + self.assertIn("reflect trigger", text) + + def test_an_unset_flag_resolves_to_stale_not_to_the_old_behaviour(self) -> None: + mode, note = self.detect.reminder_mode(environ={}, cwd=None, home=self.tmp.name) + self.assertEqual(mode, "stale") + self.assertEqual(note, "") + + def test_each_flag_value_is_honoured(self) -> None: + for value in ("off", "stale", "all"): + mode, _note = self.detect.reminder_mode( + environ={self.detect.REMINDER_FLAG: value}, cwd=None, home=self.tmp.name) + self.assertEqual(mode, value) + + def test_a_flag_value_nobody_understands_says_so_and_falls_back(self) -> None: + mode, note = self.detect.reminder_mode( + environ={self.detect.REMINDER_FLAG: "quiet"}, cwd=None, home=self.tmp.name) + self.assertEqual(mode, "stale") + self.assertIn("is not off, stale, all", note) + + def test_an_unreadable_env_file_is_reported_as_unchecked(self) -> None: + unreadable = os.path.join(self.tmp.name, "env-is-a-directory") + os.makedirs(unreadable, exist_ok=True) + mode, note = self.detect.reminder_mode( + environ={"CATSTACK_ENV_FILE": unreadable}, cwd=None, home=self.tmp.name) + self.assertEqual(mode, "stale") + self.assertIn("could not read", note) + + def test_recording_keeps_happening_while_the_reminder_is_off(self) -> None: + """off is about the injection, never about the ledger.""" + self.detect.evaluate(self.payload(REAL_TAG_1, tools=True)) + self.assertEqual(len(self.detect.outstanding(self.detect.read_ledger("s1"))), 1) + self.assertEqual(self.detect.reminder("s1", "off"), "") + def test_two_tags_in_one_session_both_tracked(self) -> None: self.detect.record_turn("s1", REAL_TAG_1, set()) self.detect.record_turn("s1", REAL_TAG_2, set()) @@ -209,6 +264,33 @@ def test_verified_and_dropped_tag_is_discharged_through_the_real_payload(self) - self.assertEqual(self.detect.outstanding(self.detect.read_ledger("s1")), []) self.assertEqual(self.detect.reminder("s1"), "") + def test_a_discharged_claim_fires_the_reflect_trigger(self) -> None: + self.detect.evaluate(self.payload(REAL_TAG_1, tools=True)) + verdict = self.detect.evaluate( + self.payload("Here is the pasted output proving it.", tools=True)) + self.assertIn("reflect trigger", verdict["note"]) + self.assertIn("widened scope", verdict["note"]) + + def test_an_evidence_order_correction_triggers_with_no_wrongness_word(self) -> None: + """The transition fires; the reply's wording is not consulted at all.""" + for index, reply in enumerate((EVIDENCE_ORDER_1, EVIDENCE_ORDER_2)): + session = f"evidence-order-{index}" + self.detect.evaluate(self.payload(REAL_TAG_1, tools=True, session_id=session)) + verdict = self.detect.evaluate( + self.payload(reply, tools=True, session_id=session)) + self.assertIn("reflect trigger", verdict["note"]) + self.assertIn("carries no wrongness word", verdict["note"]) + + def test_a_turn_that_discharges_nothing_stays_silent_about_reflect(self) -> None: + verdict = self.detect.evaluate( + self.payload("Ran the tests, all green.", tools=True)) + self.assertEqual(verdict["note"], "") + + def test_a_reemitted_tag_is_not_reported_as_discharged(self) -> None: + self.detect.evaluate(self.payload(REAL_TAG_1, tools=True)) + verdict = self.detect.evaluate(self.payload(REAL_TAG_1, tools=True)) + self.assertNotIn("reflect trigger", verdict["note"]) + def test_unchecked_turn_does_not_discharge_a_row(self) -> None: self.detect.evaluate(self.payload(REAL_TAG_1, tools=True)) self.detect.evaluate({ diff --git a/engine/hooks/wrong-check-reflect/README.md b/engine/hooks/wrong-check-reflect/README.md index 02db976a..c9b0dfc8 100644 --- a/engine/hooks/wrong-check-reflect/README.md +++ b/engine/hooks/wrong-check-reflect/README.md @@ -12,7 +12,8 @@ up. If the judge result was unchecked, the inbox reports "could not judge" instead of treating the reply as clean. Finish the live correction first. Fail-open. -Once per transcript. Skip if the user already said `/reflect`. +Once per reply. Skip if the user asked for `/reflect` in the same turn that +produced that reply. Not word-count (`diu-stop`). Not token_audit thrash (`reflect-on-thrash`). Assistant text only - user messages and fenced code stay silent. @@ -24,9 +25,20 @@ builds a phrase-dictionary job, and sends it to `llm-judge`. The dictionary defines the meaning with `match` and `not_match` examples and supplies the static `on_hit` follow-up text. -No job is sent when `stop_hook_active` is set, when this transcript or reply -was already prompted, when the reply is empty, or when the user already asked -for `/reflect`. Inside a judge run (`CATSTACK_LLM_JUDGE_CHILD=1`) `llm-judge` +No job is sent when `stop_hook_active` is set, when this exact reply was +already prompted, when the reply is empty, or when the user asked for +`/reflect` in the turn that produced this reply. Only the person counts: the +harness files its own injections as `type: "user"` rows carrying `isMeta`, so +a Stop hook's own feedback and a skill's injected body are read as harness +text, not as the user asking. Before that, `diu-stop`'s block text and the +reflect skill's own body both said "reflect" and switched this hook off. + +The one-shot key is the +transcript path plus a hash of the reply text: keyed on the transcript alone, +the Stop of the reply *before* a correction spent the key, and the correction +a minute later found itself already prompted. The `/reflect` scan is scoped to +the current turn for the same reason -- scanning the whole transcript let one +`/reflect` switch the hook off for the rest of the session. Inside a judge run (`CATSTACK_LLM_JUDGE_CHILD=1`) `llm-judge` refuses the job. The model call runs in a detached background process, so the reply is never @@ -50,7 +62,7 @@ pattern to this hook; the prose meaning belongs in the phrase dictionary. ## Files -- `detect.py` - judge enqueue + once-per-transcript state +- `detect.py` - judge enqueue + once-per-reply state - `claude_stop_check.py` — Claude `Stop` (stderr + exit 2) - `cursor_session.py` — Cursor `stop` / `sessionEnd` (`followup_message`) - `codex_notify.py` — Codex `notify` (advisory print + chain) diff --git a/engine/hooks/wrong-check-reflect/detect.py b/engine/hooks/wrong-check-reflect/detect.py index e49ab3b1..786db3a6 100644 --- a/engine/hooks/wrong-check-reflect/detect.py +++ b/engine/hooks/wrong-check-reflect/detect.py @@ -24,8 +24,11 @@ os.path.join(os.path.expanduser("~"), ".cache", "catstack-wrong-check-reflect"), ) -ALREADY_REFLECT_RE = re.compile(r"(?i)\b/?reflect\b|\b/?automate-me\b|\bautomate me\b") -META_USER_PREFIXES = ("\s*/?(?:reflect|automate-me)\b" + r"|\s*(?:reflect|automate-me)\s*" +) +META_USER_PREFIXES = (" str: - key = transcript_path or "no-transcript" - digest = hashlib.sha1(os.path.abspath(key).encode()).hexdigest()[:16] +def reply_key(transcript_path: str, text: str) -> str: + """One-shot key for a single reply, not for a whole session. + + Keying on the transcript alone made the hook fire at most once per + session, and the Stop that spent the key was the Stop of the reply + BEFORE the correction -- so the correction itself, a minute later, was + already marked as prompted. A reply is the thing being judged, so the + reply's text is what the key is made of. The transcript stays in the key + so the same sentence in two sessions is two chances, not one. + """ + base = os.path.abspath(transcript_path) if transcript_path else "no-transcript" + return base + "\n" + hashlib.sha256((text or "").encode("utf-8")).hexdigest() + + +def _state_file(key: str) -> str: + digest = hashlib.sha1((key or "no-transcript").encode()).hexdigest()[:16] return os.path.join(STATE_DIR, f"{digest}.prompted") -def already_prompted(transcript_path: str) -> bool: - return os.path.isfile(_state_file(transcript_path or "no-transcript")) +def already_prompted(key: str) -> bool: + return os.path.isfile(_state_file(key or "no-transcript")) -def mark_prompted(transcript_path: str) -> None: - path = _state_file(transcript_path or "no-transcript") +def mark_prompted(key: str) -> None: + path = _state_file(key or "no-transcript") os.makedirs(os.path.dirname(path), exist_ok=True) with open(path, "w", encoding="utf-8") as handle: - handle.write((transcript_path or "") + "\n") + handle.write((key or "") + "\n") def _is_user_line(data: dict) -> bool: @@ -60,6 +76,22 @@ def _is_user_line(data: dict) -> bool: return isinstance(message, dict) and message.get("role") == "user" +def _is_meta_line(data: dict) -> bool: + """True for a user-shaped row that is not the person speaking. + + The harness files its own injections as `type: "user"`: a Stop hook's + feedback, a skill's body, a subagent's transcript. All of them carry + `isMeta`, which is what `engine/skills/reflect/scripts/token_audit.py:313` + keys off, so this reads the record instead of the prose. A prose prefix + could only ever catch the wordings someone had already seen -- and it + missed both the hook feedback and the reflect skill's own body, which is + how running `/reflect` disarmed this hook. + """ + if data.get("isMeta") or data.get("agentId") or data.get("isSidechain"): + return True + return _message_text(data).lstrip().startswith(META_USER_PREFIXES) + + def _message_text(data: dict) -> str: message = data.get("message") content = message.get("content") if isinstance(message, dict) else data.get("content") @@ -76,9 +108,9 @@ def _message_text(data: dict) -> str: return "" -def user_already_asked_reflect(path: str) -> bool: - if not path or not os.path.isfile(path): - return False +def _transcript_roles(path: str) -> list[tuple[str, str]] | None: + """(role, text) per transcript line, or None when the file cannot be read.""" + rows: list[tuple[str, str]] = [] try: with open(path, encoding="utf-8") as handle: for line in handle: @@ -86,15 +118,63 @@ def user_already_asked_reflect(path: str) -> bool: data = json.loads(line) except json.JSONDecodeError: continue - if not isinstance(data, dict) or not _is_user_line(data): - continue - text = _message_text(data) - if not text or text.lstrip().startswith(META_USER_PREFIXES): + if not isinstance(data, dict): continue - if ALREADY_REFLECT_RE.search(text): - return True - except OSError: + if _is_user_line(data): + rows.append(("meta" if _is_meta_line(data) else "user", _message_text(data))) + elif _is_assistant_line(data): + rows.append(("assistant", _message_text(data))) + except OSError as exc: + print( + f"catstack-hook-error wrong-check-reflect: cannot read {path}, " + f"the user's own reflect request is unchecked: {exc}", + file=sys.stderr, + ) + return None + return rows + + +def user_already_asked_reflect(path: str) -> bool: + """True when the user invoked reflect in the turn that produced this reply. + + Only a real invocation counts, which the harness records as a + `/reflect` envelope. Prose that merely says + the word does not: `"Claim I made was wrong" is a trigger for /reflect` + describes the rule, it does not ask for anything, and suppressing on it + let a sentence about the hook switch the hook off. Erring toward asking + is the safe direction for a detector that spoke 0 times in 1,682 runs. + + Scoped to that one turn on purpose. Scanning the whole transcript meant a + single `/reflect` typed at the start of a session switched the detector + off for every reply after it, however many hours later. + + The turn's window ends at the last assistant row, or at the end of the + file when there is none. A Stop payload can carry the reply before its + row lands, and the session's first turn has no earlier assistant row at + all; in both cases every row belongs to this turn. Treating a missing + row as "no request found" would make the hook ignore a reflect the + person did ask for. + """ + if not path or not os.path.isfile(path): + return False + rows = _transcript_roles(path) + if rows is None: return False + reply_at = len(rows) + for index in range(len(rows) - 1, -1, -1): + if rows[index][0] == "assistant" and rows[index][1].strip(): + reply_at = index + break + start = 0 + for index in range(reply_at - 1, -1, -1): + if rows[index][0] == "assistant" and rows[index][1].strip(): + start = index + 1 + break + for role, text in rows[start:reply_at]: + if role == "assistant" or not text: + continue + if ALREADY_REFLECT_RE.search(text): + return True return False @@ -192,7 +272,7 @@ def enqueue_judge(payload: dict) -> str | None: return None path = resolve_transcript(payload) text = last_assistant_text(payload, path) - key = path or text[:200] + key = reply_key(path, text) if not text.strip() or already_prompted(key): return None if path and user_already_asked_reflect(path): diff --git a/engine/hooks/wrong-check-reflect/eval_dictionary.py b/engine/hooks/wrong-check-reflect/eval_dictionary.py index 258dabb2..9a9e8927 100644 --- a/engine/hooks/wrong-check-reflect/eval_dictionary.py +++ b/engine/hooks/wrong-check-reflect/eval_dictionary.py @@ -17,6 +17,9 @@ (HIT_TEXT, True), ("You're right. Let's go with option B.", False), ("I double-checked my earlier count and it holds; nothing in it was wrong.", False), + ("Correcting one claim and arming the check I implied:", True), + ("I was right - but I said it a turn before I checked it", True), + ("I ran the check first and then said it, so the order was right.", False), ) diff --git a/engine/hooks/wrong-check-reflect/tests/test_hooks.py b/engine/hooks/wrong-check-reflect/tests/test_hooks.py index e3d44630..061d1fcb 100644 --- a/engine/hooks/wrong-check-reflect/tests/test_hooks.py +++ b/engine/hooks/wrong-check-reflect/tests/test_hooks.py @@ -31,6 +31,7 @@ PY = sys.executable +REFLECT_COMMAND = "reflect/reflect" HIT_TEXT = "Correction: the file I pointed you to earlier is not the one in use; the real one is src/b.py." OPTION_TEXT = "You're right. Let's go with option B." COUNT_TEXT = "I double-checked my earlier count and it holds; nothing in it was wrong." @@ -71,7 +72,19 @@ def run_codex_notify(argv: list[str]) -> str: def transcript_line(role: str, text: str) -> str: - return json.dumps({"type": role, "message": {"role": role, "content": [{"type": "text", "text": text}]}}) + """One transcript row. A role of "meta" is the harness talking, not the user. + + The shape of a meta row is taken from a real Claude Code transcript: the + harness files its Stop-hook feedback and a skill's injected body as + `type: "user"` rows carrying `isMeta: true`. + """ + meta = role == "meta" + kind = "user" if meta else role + row = {"type": kind, "message": {"role": kind, "content": [{"type": "text", "text": text}]}} + if meta: + row["isMeta"] = True + row["isSidechain"] = False + return json.dumps(row) class TestWrongCheckReflect(JudgeTestCase): @@ -198,15 +211,130 @@ def test_judge_not_enqueued_when_stop_hook_active(self): def test_judge_not_enqueued_when_already_prompted(self): path = self.write_transcript(("assistant", HIT_TEXT)) - detect.mark_prompted(path) + detect.mark_prompted(detect.reply_key(path, HIT_TEXT)) self.assertIsNone(detect.enqueue_judge({"transcript_path": path})) self.assertEqual(self.jobs(), []) def test_judge_not_enqueued_when_user_already_asked_reflect(self): - path = self.write_transcript(("user", "please /reflect"), ("assistant", HIT_TEXT)) + path = self.write_transcript(("user", REFLECT_COMMAND), ("assistant", HIT_TEXT)) + self.assertIsNone(detect.enqueue_judge({"transcript_path": path})) + self.assertEqual(self.jobs(), []) + + def test_prose_about_reflect_does_not_count_as_asking_for_one(self): + """A sentence naming the command is not an invocation of it. + + `"Claim I made was wrong" is a trigger for /reflect` describes when + the hook fires. Treating that as a request let a sentence about the + hook switch the hook off for the rest of the turn. + """ + path = self.write_transcript( + ("user", '"Claim I made was wrong" is a trigger for /reflect'), + ("assistant", HIT_TEXT), + name="prose-mention.jsonl", + ) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + + def test_the_later_correction_still_fires_after_the_reply_before_it(self): + """Lockout one: the Stop of the pre-correction reply spent the key.""" + self.use_runners(SLOW_CLEAN) + turn_one = (("user", "check the path"), ("assistant", "The live file is src/a.py.")) + path = self.write_transcript(*turn_one, name="turn.jsonl") + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + self.write_transcript( + *turn_one, + ("user", "are you sure?"), + ("assistant", HIT_TEXT), + name="turn.jsonl", + ) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + + def test_reflect_asked_earlier_in_the_session_does_not_silence_a_later_reply(self): + """Lockout two: one /reflect used to switch the hook off for good.""" + self.use_runners(SLOW_CLEAN) + path = self.write_transcript( + ("user", "please /reflect on the last hour"), + ("assistant", "Here is the reflect write-up."), + ("user", "now fix the import"), + ("assistant", HIT_TEXT), + name="long.jsonl", + ) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + + def test_reflect_asked_in_this_same_turn_is_still_not_doubled_up(self): + path = self.write_transcript( + ("user", "fix the import"), + ("assistant", "Done."), + ("user", "that was wrong. " + REFLECT_COMMAND), + ("assistant", HIT_TEXT), + name="same-turn.jsonl", + ) self.assertIsNone(detect.enqueue_judge({"transcript_path": path})) self.assertEqual(self.jobs(), []) + def test_the_same_reply_is_never_queued_twice(self): + self.use_runners(SLOW_CLEAN) + path = self.write_transcript(("assistant", HIT_TEXT), name="dedup.jsonl") + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + self.assertIsNone(detect.enqueue_judge({"transcript_path": path})) + + def test_the_same_reply_is_judged_under_both_spellings_of_the_stop_event(self): + """Harness parity: no branch may turn on the case of the event name.""" + self.use_runners(SLOW_CLEAN) + queued = {} + for spelling in ("Stop", "stop"): + path = self.write_transcript( + ("assistant", HIT_TEXT), name=f"parity-{spelling}.jsonl") + queued[spelling] = detect.enqueue_judge( + {"transcript_path": path, "hook_event_name": spelling}) + self.assertIsNotNone(queued["Stop"]) + self.assertIsNotNone(queued["stop"]) + + def test_a_stop_hook_feedback_line_does_not_suppress_the_hook(self): + """The ecosystem used to silence itself: diu-stop's own block says reflect.""" + self.use_runners(SLOW_CLEAN) + path = self.write_transcript( + ("user", "fix the import"), + ("meta", "Stop hook feedback: [diu-stop/claude_stop_check.py]: read the " + "reflect skill and say why"), + ("assistant", HIT_TEXT), + name="hook-feedback.jsonl", + ) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + + def test_the_reflect_skills_own_body_does_not_suppress_the_hook(self): + """Running /reflect used to disarm the detector that asks for it.""" + self.use_runners(SLOW_CLEAN) + path = self.write_transcript( + ("user", "fix the import"), + ("meta", "Base directory for this skill: ~/.claude/skills/reflect\n# Reflect"), + ("assistant", HIT_TEXT), + name="skill-body.jsonl", + ) + self.assertIsNotNone(detect.enqueue_judge({"transcript_path": path})) + + def test_a_real_user_reflect_request_in_the_same_turn_still_suppresses(self): + path = self.write_transcript( + ("user", "fix the import"), + ("assistant", "Done."), + ("user", "that was wrong. " + REFLECT_COMMAND), + ("meta", "Stop hook feedback: unrelated"), + ("assistant", HIT_TEXT), + name="real-request.jsonl", + ) + self.assertIsNone(detect.enqueue_judge({"transcript_path": path})) + self.assertEqual(self.jobs(), []) + + def test_unreadable_transcript_reports_unchecked_instead_of_going_quiet(self): + missing = os.path.join(self.reflect_state.name, "does-not-exist.jsonl") + self.assertFalse(detect.user_already_asked_reflect(missing)) + directory = os.path.join(self.reflect_state.name, "a-directory.jsonl") + os.makedirs(directory, exist_ok=True) + err = io.StringIO() + with redirect_stderr(err): + rows = detect._transcript_roles(directory) + self.assertIsNone(rows) + self.assertIn("unchecked", err.getvalue()) + def test_claude_malformed_stdin_fail_open(self): err = io.StringIO() with patch.object(sys, "stdin", io.StringIO("not-json")): diff --git a/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py b/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py index fb68c79d..dc4476cd 100644 --- a/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py +++ b/engine/skills/reflect/scripts/tests/test_self_retraction_scan.py @@ -46,6 +46,30 @@ def test_false_claim_still_matches(self): self.assertIsNotNone(self_retraction_scan.find_admission(text)) +class TestEvidenceOrderIsOutOfReach(unittest.TestCase): + """Two real corrections this scan cannot see, and the reason it cannot. + + Both are corrections about evidence ORDER: the claim was true, and it was + asserted before the check ran. Nothing in either sentence says anything was + wrong, so every pattern here misses them by construction. Pinned so the + next author widens the regex knowingly rather than by accident: the catch + for this class is the unverified-tag-ledger discharge transition, which + reads state rather than wording. + """ + + def test_arming_the_implied_check_is_not_reachable_by_wording(self): + text = "Correcting one claim and arming the check I implied:" + self.assertIsNone(self_retraction_scan.find_admission(text)) + + def test_right_but_asserted_early_is_not_reachable_by_wording(self): + text = "I was right - but I said it a turn before I checked it" + self.assertIsNone(self_retraction_scan.find_admission(text)) + + def test_the_same_sentence_with_a_wrongness_word_does_fire(self): + text = "I was wrong about the path; I said it a turn before I checked it." + self.assertIsNotNone(self_retraction_scan.find_admission(text)) + + class TestScanAssistantTexts(unittest.TestCase): def test_collects_one_hit_per_admission(self): hits = self_retraction_scan.scan_assistant_texts( diff --git a/tests/scenarios/self-correction.json b/tests/scenarios/self-correction.json index 8d41bd49..d5886be4 100644 --- a/tests/scenarios/self-correction.json +++ b/tests/scenarios/self-correction.json @@ -19,11 +19,20 @@ }, { "name": "admission-skipped-when-user-already-said-reflect", - "situation": "Pins the documented skip: ALREADY_REFLECT_RE suppresses the follow-up when the user's own message already asked for /reflect, so the hook does not nag for something already in flight. Caught by writing the scenario above with the user's literal message, which contained /reflect.", - "user": "\"Claim I made was wrong\" is a trigger for /reflect", + "situation": "Pins the documented skip: the hook does not nag for a reflect the user has already asked for. The user's message must be an actual request. An earlier version of this scenario used a message that only mentioned /reflect while describing the rule, so it pinned the suppression against a sentence that never asked for anything; the mention case is now its own scenario below.", + "user": "reflect/reflecton this", "reply": "Also: a claim I made earlier was wrong. The coverage run compared against origin/main rather than the slice, so the pass was vacuous.", "expect_no_enqueue": [ "wrong-check-reflect" ] + }, + { + "name": "admission-still-caught-when-reflect-is-only-mentioned", + "situation": "The mention-is-not-a-use half of the same rule. Quoting the word /reflect while describing when it fires is not a request for one, so an admission in the same turn must still reach the judge. This is the exact text the previous scenario used to suppress on.", + "user": "\"Claim I made was wrong\" is a trigger for /reflect", + "reply": "Also: a claim I made earlier was wrong. The coverage run compared against origin/main rather than the slice, so the pass was vacuous.", + "expect_enqueue": [ + "wrong-check-reflect" + ] } ] diff --git a/tests/test_cat_mode.py b/tests/test_cat_mode.py index 5fc75a49..53fe4337 100644 --- a/tests/test_cat_mode.py +++ b/tests/test_cat_mode.py @@ -873,5 +873,34 @@ def test_hook_blocks_the_same_estimate_once_the_job_already_exited(self): self.assertIsNotNone(detect.decide_stop_from_lines(ESTIMATE_REPLY, lines)) +class TestEscapeHatchTemplateIsWellFormed(unittest.TestCase): + """Every tag this skill shows a reader must be one the gate accepts. + + cat-mode carried a bare `{{CAT-UNVERIFIED}}` in the sentence that tells + the reader to use the tag, so quoting the rule tripped the gate that + enforces it. The template has to name a blocker, the same one + engine/CLAUDE.core.md already shows. + """ + + def markers(self): + import importlib.util as util + path = os.path.join(REPO_ROOT, "engine", "hooks", "_markers", "markers.py") + spec = util.spec_from_file_location("markers_for_test", path) + module = util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + def test_no_tag_in_the_skill_names_no_blocker(self): + with open(SKILL_PATH, encoding="utf-8") as handle: + text = handle.read() + malformed = self.markers().malformed_tags(text) + self.assertEqual(malformed, [], f"tags naming no blocker: {malformed}") + + def test_the_skill_still_shows_the_tag_at_all(self): + with open(SKILL_PATH, encoding="utf-8") as handle: + text = handle.read() + self.assertTrue(self.markers().well_formed_tags(text)) + + if __name__ == "__main__": unittest.main()