diff --git a/engine/hooks/_runner/tests/test_machine_deliverable.py b/engine/hooks/_runner/tests/test_machine_deliverable.py index 4d77ed1cc..c99ce0dda 100644 --- a/engine/hooks/_runner/tests/test_machine_deliverable.py +++ b/engine/hooks/_runner/tests/test_machine_deliverable.py @@ -113,7 +113,7 @@ def _cases(self) -> list[tuple[str, str, str, str, bool]]: return [ ("diu-stop/claude_stop_check.py", str(_fixture("diu-stop", "claims_fires.json")["reply"]), "", "root cause", True), ("wait-needs-wakeup/claude_stop_check.py", str(wait["reply"]), - self._transcript(list(wait["transcript"])), "wait-needs-wakeup", True), + self._transcript(list(wait["transcript"])), "wait-needs-wakeup", False), ("hedge-runs-prove-it/claude_stop_check.py", "It's probably still on the branch and I don't think the build ran.", self._transcript([]), "probably", False), diff --git a/engine/hooks/_sdk/runtime.py b/engine/hooks/_sdk/runtime.py index f41b6e795..7635bdfe2 100644 --- a/engine/hooks/_sdk/runtime.py +++ b/engine/hooks/_sdk/runtime.py @@ -101,7 +101,7 @@ def run_hook( _write_detector_stderr(event) duration_ms = _duration_ms(started) mode, mode_source, finding_modes = effective_finding_modes(hook, event, findings) - _write_findings_file(findings) + _write_findings_file(findings + _unchecked_findings(event)) if event.get("_payload_error") and not findings: if json_error_stderr_prefix is not None: print( diff --git a/engine/hooks/diu-stop/README.md b/engine/hooks/diu-stop/README.md index d8f912d47..91323e278 100644 --- a/engine/hooks/diu-stop/README.md +++ b/engine/hooks/diu-stop/README.md @@ -104,7 +104,7 @@ into warnings, the Stop hook's included. - `comment_reply.py` -- reads the reply body out of an `ArtifactComments` reply or a Claude Docs utterance and applies the comment cap and the mechanical plain-words checks. No model. - `cursor_stop_check.py` -- Cursor's `stop` hook command. It uses reply text from the payload when that text is present. Otherwise it reads the last assistant text from `transcript_path`, then runs the same detector through the shared `_sdk` runtime. A missing or unreadable transcript is a finding, not an empty reply. - `diu_limit.py` -- the word limit and what it does not count. The reminder's wording and the Stop hook's check both read it, so they cannot disagree; `tests/test_limit_agreement.py` pins that. -- `plain_words.py` -- turns every `phrases/` word list into one question about the user's last message and the finished reply, hands it to the background judge, and waits for the answer so a hit blocks the same turn. A verdict delivered on the next prompt is one the user may never see. No answer in time means the turn ends unblocked. +- `plain_words.py` -- turns every `phrases/` word list into one question about the user's last message and the finished reply, hands it to the background judge, and waits for the answer so a hit blocks the same turn. A verdict delivered on the next prompt is one the user may never see. No answer in time means the turn ends unblocked. The judge reads the last five human turns, but the words the user has written anywhere in the chat go along as an allow-list, and a hit whose every content word (plurals and tenses folded, stopwords dropped) the user has already used is dropped before it blocks: a word the user said six turns ago is still the user's word. - `phrases/` -- the word lists themselves, one file per kind of wording to avoid, in the format `engine/hooks/llm-judge/phrases.py` loads. - `install_claude_hook.py` -- merges the three Claude fragments above into `~/.claude/settings.json`, idempotently, without touching anything else there. - `cursor.hooks.json` -- the whole file to install as `~/.cursor/hooks.json`. diff --git a/engine/hooks/diu-stop/plain_words.py b/engine/hooks/diu-stop/plain_words.py index 942907cd5..0fdd555ea 100644 --- a/engine/hooks/diu-stop/plain_words.py +++ b/engine/hooks/diu-stop/plain_words.py @@ -11,6 +11,7 @@ import json import os +import re import sys import time import uuid @@ -26,6 +27,12 @@ TEXT_LIMIT = 4000 RECENT_USER_TURNS = 5 META_USER_PREFIXES = (" str: return "" -def recent_user_messages(path: str, limit: int = RECENT_USER_TURNS) -> str: - """Join the last few human turns so the judge sees names the user already used.""" - if not path or not os.path.isfile(path) or limit < 1: - return "" +def human_turns(path: str) -> list[str]: + """Every human turn in the transcript, oldest first, with harness and hook rows left out.""" + if not path or not os.path.isfile(path): + return [] found: list[str] = [] with open(path, encoding="utf-8", errors="replace") as handle: for line in handle: @@ -91,17 +98,53 @@ def recent_user_messages(path: str, limit: int = RECENT_USER_TURNS) -> str: text = _message_text(data).strip() if text and not text.startswith(META_USER_PREFIXES): found.append(text) + return found + + +def recent_user_messages(path: str, limit: int = RECENT_USER_TURNS) -> str: + """Join the last few human turns so the judge sees names the user already used.""" + if limit < 1: + return "" + found = human_turns(path) if not found: return "" return "\n\n".join(found[-limit:]) +def _stem(word: str) -> str: + """Fold plurals and tenses so "MRs" and "MR", "force-pushed" and "force-push" compare equal.""" + word = word.strip("'.-/") + for ending, shortest_stem in (("ing", 3), ("ed", 3), ("s", 2)): + if word.endswith(ending) and len(word) - len(ending) >= shortest_stem and not word.endswith("ss"): + return word[: -len(ending)] + return word + + +def content_words(text: str) -> set[str]: + """Lower-cased, lightly stemmed words with the stopwords dropped.""" + return {_stem(word) for word in WORD_RE.findall((text or "").lower()) if word not in STOPWORDS} + + +def user_vocabulary(path: str) -> set[str]: + """Content words from every human turn, not only the recent ones the judge reads.""" + words: set[str] = set() + for text in human_turns(path): + words |= content_words(text) + return words + + +def used_by_user(phrase: str, vocabulary: set[str]) -> bool: + """True when every content word of the flagged phrase is one the user has written.""" + words = content_words(phrase) + return bool(words) and words <= set(vocabulary) + + def last_user_message(path: str) -> str: recent = recent_user_messages(path, limit=1) return recent -def prompt(dictionaries: list[dict], asked: str, reply: str) -> str: +def prompt(dictionaries: list[dict], asked: str, reply: str, vocabulary: set[str] | None = None) -> str: lines = [ f"Return exactly one line of JSON: {ANSWER_SHAPE}", "Each list below names a kind of wording to avoid when writing to this user.", @@ -121,6 +164,9 @@ def prompt(dictionaries: list[dict], asked: str, reply: str) -> str: "or uses a specialized word that is not in the USER text and is not defined in the same ASSISTANT sentence.", "Prefer category plain-words-vocabulary-drift for inventing or rotating names without a definition.", "", + "WORDS THE USER HAS ALREADY USED, in any form, anywhere in this chat (never count these): " + + ", ".join(sorted(vocabulary or ()))[:TEXT_LIMIT], + "", f"USER:\n{asked[-TEXT_LIMIT:]}", "", f"ASSISTANT:\n{reply[-TEXT_LIMIT:]}", @@ -138,23 +184,27 @@ def job(payload: dict) -> dict | None: dictionaries = [phrases.load(name, directory=PHRASES_DIR) for name in list_names()] if not dictionaries: return None + vocabulary = user_vocabulary(transcript) return { "id": uuid.uuid4().hex, "hook": HOOK_NAME, "transcript": transcript, - "prompt": prompt(dictionaries, recent_user_messages(transcript), reply), + "prompt": prompt(dictionaries, recent_user_messages(transcript), reply, vocabulary), + "user_vocabulary": sorted(vocabulary), "hit_if_all_true": ["match"], "on_hit": "diu: the last reply used wording the user has had to ask about; say it in everyday words.", } -def message_for(verdict: dict, reply: str) -> str: +def message_for(verdict: dict, reply: str, vocabulary: set[str] | None = None) -> str: if verdict.get("outcome") != "hit": return "" answer = verdict.get("answer") or {} closest = str(answer.get("closest") or "").strip() if not closest or closest not in reply: return "" + if vocabulary and used_by_user(closest, vocabulary): + return "" return MESSAGE.format(category=str(answer.get("category") or "unnamed list"), closest=closest) @@ -181,10 +231,11 @@ def check_reply(payload: dict) -> str: return "" deadline = time.monotonic() + wait_seconds() transcript = built["transcript"] + vocabulary = set(built.get("user_vocabulary") or ()) while time.monotonic() < deadline: for verdict in judge.drain(transcript): if verdict.get("id") == built["id"]: - return message_for(verdict, payload.get("last_assistant_message") or "") + return message_for(verdict, payload.get("last_assistant_message") or "", vocabulary) time.sleep(POLL_SECONDS) return "" diff --git a/engine/hooks/diu-stop/tests/test_plain_words_check.py b/engine/hooks/diu-stop/tests/test_plain_words_check.py index d31e45fdf..0fc22ac83 100644 --- a/engine/hooks/diu-stop/tests/test_plain_words_check.py +++ b/engine/hooks/diu-stop/tests/test_plain_words_check.py @@ -131,7 +131,66 @@ def test_a_clean_or_unchecked_verdict_says_nothing(self): self.assertEqual(plain_words.message_for({"id": "j1", "outcome": outcome}, REPLY), "") +def _long_ago_rows(first_user_text, reply): + rows = [_user(first_user_text), _assistant("ok")] + for turn in range(6): + rows.append(_user(f"next question {turn}")) + rows.append(_assistant(f"answer {turn}")) + rows.append(_assistant(reply)) + return rows + + +class TestUserVocabulary(PlainWordsCase): + def test_user_vocabulary_reads_every_human_turn_not_just_the_last_five(self): + rows = _long_ago_rows("lets review the MRs in the database", "The MR database has 3 rows.") + path = _transcript(self.tmp, rows) + vocabulary = plain_words.user_vocabulary(path) + self.assertIn("mr", vocabulary) + self.assertIn("database", vocabulary) + self.assertNotIn("the", vocabulary) + self.assertNotIn("MRs", plain_words.recent_user_messages(path)) + + def test_used_by_user_matches_plural_and_tense_forms(self): + vocabulary = plain_words.user_vocabulary(_transcript(self.tmp, [_user("force-push with lease, then look at the MRs")])) + self.assertTrue(plain_words.used_by_user("MR database", vocabulary | {"database"})) + self.assertTrue(plain_words.used_by_user("force-pushed with lease", vocabulary)) + self.assertFalse(plain_words.used_by_user("thrash vector", vocabulary)) + self.assertFalse(plain_words.used_by_user("", vocabulary)) + + def test_the_job_prompt_lists_the_users_words_as_an_allow_list(self): + rows = _long_ago_rows("lets review the MRs in the database", "The MR database has 3 rows.") + built = plain_words.job(self.payload(rows=rows, last_assistant_message="The MR database has 3 rows.")) + self.assertIn("WORDS THE USER HAS ALREADY USED", built["prompt"]) + self.assertIn("mr", built["user_vocabulary"]) + + class TestCheckReply(PlainWordsCase): + def test_plain_words_skips_a_term_the_user_used_six_turns_ago(self): + reply = "The MR database has 3 rows." + rows = _long_ago_rows("lets review the MRs in the database", reply) + payload = self.payload(rows=rows, last_assistant_message=reply) + with patch.object(self.judge, "enqueue", side_effect=lambda job: job["id"]): + with patch.object(self.judge, "drain", side_effect=lambda transcript: [_hit(self._queued_id, "MR database")]): + with patch.object(plain_words, "job", side_effect=self._remember(plain_words.job)): + self.assertEqual(plain_words.check_reply(payload), "") + + def _remember(self, real_job): + def wrapped(payload): + built = real_job(payload) + self._queued_id = built["id"] + return built + return wrapped + + def test_a_term_the_user_never_used_still_blocks(self): + reply = "The MR database has 3 rows." + rows = _long_ago_rows("lets review the database", reply) + payload = self.payload(rows=rows, last_assistant_message=reply) + with patch.object(self.judge, "enqueue", side_effect=lambda job: job["id"]): + with patch.object(self.judge, "drain", side_effect=lambda transcript: [_hit(self._queued_id, "MR database")]): + with patch.object(plain_words, "job", side_effect=self._remember(plain_words.job)): + text = plain_words.check_reply(payload) + self.assertIn("MR database", text) + def test_the_hit_comes_back_in_the_same_turn(self): payload = self.payload() built = { diff --git a/engine/hooks/hedge-runs-prove-it/README.md b/engine/hooks/hedge-runs-prove-it/README.md index 2343315a5..8d081eeff 100644 --- a/engine/hooks/hedge-runs-prove-it/README.md +++ b/engine/hooks/hedge-runs-prove-it/README.md @@ -52,8 +52,12 @@ past-tense narration of an old incident ("the task was a zombie"). An error message can expose a local fallback or hardcoded enumeration without proving the capabilities of the system named in that error. The hook records -values in bracketed or comma-joined lists from error-shaped tool results. It -blocks an outgoing reply that repeats at least two of those values within 200 +values in bracketed or comma-joined lists from error-shaped tool results, and +only when the error text itself introduces the list ("Known models:", "valid +values:", "one of") and only from this turn's tool results. Only +identifier-shaped values count (a letter plus an inner `-_./:@+`, like +`gpt-5.5`); plain words, numbers, and `p95` between commas in an error log are +prose, not a capability list. It blocks an outgoing reply that repeats at least two of those values within 200 characters of `accepts`, `supports`, `known models`, `valid`, or `only` when those values appeared nowhere in a non-error tool result. diff --git a/engine/hooks/hedge-runs-prove-it/detect.py b/engine/hooks/hedge-runs-prove-it/detect.py index b8a70c6ee..6e1294656 100644 --- a/engine/hooks/hedge-runs-prove-it/detect.py +++ b/engine/hooks/hedge-runs-prove-it/detect.py @@ -22,6 +22,11 @@ what another system accepts or supports. A reply that repeats two or more of those values beside a capability verb is blocked unless a non-error tool result also supplied them, or the reply attributes the list as fallback/error data. +Only a list the error output itself introduces ("Known models: [a-b, c-d]", +"valid values: ...") counts, only from this turn's tool results, and only +identifier-shaped values (a letter plus an inner `-_./:@+`): ordinary words +and numbers that merely sit between commas in an error log are not a list of +what a system supports. Hedges about things that are not code or state (a company's motive, a filing date) are out of scope, and so is either shape quoted rather than @@ -132,13 +137,19 @@ r"\bnot\s+supported\b", re.IGNORECASE, ) -BRACKETED_ENUM_RE = re.compile(r"\[([^\[\]\n]+)\]") COMMA_ENUM_RE = re.compile( r"(? bool: + """A letter plus an inner separator: `gpt-5.5`, `a/b`, `x_y`. Plain words, + bare numbers and `95th` are prose, not capability values.""" + return ( + len(value) >= 2 + and bool(ENUM_VALUE_RE.fullmatch(value)) + and any(ch.isalpha() for ch in value) + and bool(IDENTIFIER_SEPARATOR_RE.search(value)) + ) + + def _enumerated_values(text: str) -> set[str]: - runs = [match.group(1) for match in BRACKETED_ENUM_RE.finditer(text or "")] - runs.extend(match.group(1) for match in COMMA_ENUM_RE.finditer(text or "")) + """Identifier-shaped values in a list the text itself introduces. + + The list is the bracketed or comma run that starts right after a label + such as "Known models:" or "one of", on the same line. A comma run with + no such label is prose or log text and yields nothing. + """ values: set[str] = set() - for run in runs: + for intro in LIST_INTRO_RE.finditer(text or ""): + line_end = text.find("\n", intro.end()) + tail = text[intro.end(): line_end if line_end != -1 else len(text)] + if intro.group(0).rstrip().endswith("[") or tail.startswith("["): + run = tail.lstrip("[").split("]", 1)[0] + else: + match = COMMA_ENUM_RE.match(tail) + run = match.group(1) if match else "" for raw_value in run.split(","): - value = raw_value.strip().strip("'\"`") - if len(value) >= 2 and ENUM_VALUE_RE.fullmatch(value): + value = raw_value.strip().strip("'\"`").rstrip(".") + if _identifier_shaped(value): values.add(value.lower()) return values @@ -308,16 +341,32 @@ def _value_occurs(text: str, value: str) -> bool: )) +def _turn_start(lines: list[dict]) -> int: + start = 0 + for i, data in enumerate(lines): + if _is_human_user_line(data): + start = i + return start + + def error_only_capability_values(lines: list[dict]) -> set[str]: - """Enumerated values whose tool-result sources are all error-shaped.""" + """Enumerated values whose tool-result sources are all error-shaped. + + Error-shaped sources count only from the current turn, so an error log + from earlier in the session cannot seed a value. Non-error sources count + from the whole transcript, so any earlier clean result still clears it. + """ + turn_start = _turn_start(lines) error_values: set[str] = set() non_error_results: list[str] = [] - for block, text in _tool_results(lines): - error_shaped = bool(block.get("is_error") or ERROR_OUTPUT_RE.search(text)) - if error_shaped: - error_values.update(_enumerated_values(text)) - else: - non_error_results.append(text) + for index, data in enumerate(lines): + for block, text in _tool_results([data]): + error_shaped = bool(block.get("is_error") or ERROR_OUTPUT_RE.search(text)) + if error_shaped: + if index >= turn_start: + error_values.update(_enumerated_values(text)) + else: + non_error_results.append(text) return { value for value in error_values if not any(_value_occurs(text, value) for text in non_error_results) diff --git a/engine/hooks/hedge-runs-prove-it/tests/test_hooks.py b/engine/hooks/hedge-runs-prove-it/tests/test_hooks.py index 363f8324b..13b935403 100644 --- a/engine/hooks/hedge-runs-prove-it/tests/test_hooks.py +++ b/engine/hooks/hedge-runs-prove-it/tests/test_hooks.py @@ -283,5 +283,100 @@ def test_allows_a_single_repeated_value(self): )) +PROSE_ERROR_OUTPUT = ( + "Error: request failed after 3 tries, so I will tell you what I can, say the claim, " + "get the files, stop, wait minutes, bypass the hook, turn it off. The p95 is 52.1." +) + + +def error_lines(error_text, previous_turn=False): + result = [ + {"type": "assistant", "message": {"role": "assistant", "content": [ + {"type": "tool_use", "id": "err-1", "name": "Bash", "input": {"command": "curl x"}}]}}, + {"type": "user", "message": {"role": "user", "content": [ + {"type": "tool_result", "tool_use_id": "err-1", "content": error_text, "is_error": True}]}}, + ] + human = {"type": "user", "message": {"role": "user", "content": "next question"}} + if previous_turn: + return result + [human] + return [human] + result + + +class TestCapabilityIgnoresProseInErrorOutput(unittest.TestCase): + """Reported false positives: ordinary words between commas in an error log.""" + + def test_silent_on_dictionary_words_harvested_from_error_prose(self): + reply = ( + "The log only says what it did, so I can tell, say, claim, files, get, stop, " + "minutes, p95, bypass, hook and turn apart." + ) + self.assertIsNone(detect.decide_from_lines(reply, error_lines(PROSE_ERROR_OUTPUT))) + + def test_silent_on_comma_run_with_no_list_label(self): + text = "Error: failed for gpt-5.5, gpt-5.5-pro, gpt-5.4 in the last run" + self.assertIsNone(detect.decide_from_lines( + "codex only accepts [gpt-5.5, gpt-5.5-pro].", error_lines(text))) + + def test_silent_when_the_error_is_from_an_earlier_turn(self): + self.assertIsNone(detect.decide_from_lines( + "codex only accepts [gpt-5.5, gpt-5.5-pro].", + error_lines(ERROR_CAPABILITY_LIST, previous_turn=True), + )) + + def test_silent_on_plain_word_values_in_a_labelled_list(self): + text = "Error: invalid option. Valid values: fast, slow, medium." + self.assertIsNone(detect.decide_from_lines( + "The tool only accepts fast, slow and medium.", error_lines(text))) + + def test_still_blocks_an_unbracketed_labelled_list_from_this_turns_error(self): + text = "Error: bad model. Supported models: gpt-5.5, gpt-5.5-pro, gpt-5.4." + feedback = detect.decide_from_lines( + "codex only supports gpt-5.5 and gpt-5.5-pro here", error_lines(text)) + self.assertIn("error-shaped tool output", feedback) + + def test_still_blocks_a_bracketed_list_from_this_turns_error(self): + feedback = detect.decide_from_lines( + "codex only accepts [gpt-5.5, gpt-5.5-pro].", error_lines(ERROR_CAPABILITY_LIST)) + self.assertIn("gpt-5.5", feedback) + + +SKILL_READ_WITH_ERROR_WORD = ( + "# ci-job-timing\n\nStep times live in the GitLab job logs. The parser can show ci_step rows " + "per job. If a fetch fails with an error, check the logs, parser, plus the job id, then retry." +) + + +def read_result_lines(text): + return [ + {"type": "user", "message": {"role": "user", "content": "where do the step times come from?"}}, + {"type": "assistant", "message": {"role": "assistant", "content": [ + {"type": "tool_use", "id": "read-1", "name": "Read", "input": {"file_path": "SKILL.md"}}]}}, + {"type": "user", "message": {"role": "user", "content": [ + {"type": "tool_result", "tool_use_id": "read-1", "content": SKILL_READ_WITH_ERROR_WORD}]}}, + ] + + +class TestCapabilityIgnoresOrdinaryProseWithOnly(unittest.TestCase): + """Reported false positives: the word "only" in ordinary prose beside words from a Read.""" + + def test_silent_on_only_live_in_logs_after_a_skill_read_that_mentions_error(self): + reply = "The step times only live in GitLab job logs; the parser shows ci_step rows." + self.assertIsNone(detect.decide_from_lines(reply, read_result_lines(SKILL_READ_WITH_ERROR_WORD))) + self.assertEqual(detect.detect({ + "last_assistant_message": reply, + "transcript_path": transcript_file(read_result_lines(SKILL_READ_WITH_ERROR_WORD)), + }), []) + + def test_silent_on_read_only_question_after_an_error_with_label_we(self): + text = "Error: could not render the diagram: label, we, arrow are not valid node names." + reply = "Want me to label that arrow read-only?" + self.assertIsNone(detect.decide_from_lines(reply, error_lines(text))) + + def test_still_blocks_known_models_after_a_real_error(self): + feedback = detect.decide_from_lines( + "codex only accepts [gpt-5.5, gpt-5.5-pro].", capability_lines()) + self.assertIn("gpt-5.5, gpt-5.5-pro", feedback) + + if __name__ == "__main__": unittest.main() diff --git a/engine/hooks/hook-health/README.md b/engine/hooks/hook-health/README.md index 07073ddab..d8f18e03b 100644 --- a/engine/hooks/hook-health/README.md +++ b/engine/hooks/hook-health/README.md @@ -49,6 +49,20 @@ At most five failed rows are named. If there are more than five, the notice ends the row list with `and more`. The stderr suffix uses the first non-empty line from `stderr_tail`; it is omitted when there is no such line. +A hook that could not check anything is reported too. When a hook has three +or more rows since the last prompt and every one of them is unchecked (each +`rule_ids` entry ends in `.unchecked`, or the row's `action` is `unchecked`), +the notice gets this line, once per hook until the stderr line changes: + +```text +hook-health: reported unchecked on all runs since the last prompt: ; it is not checking anything. +``` + +Those rows are not counted as "blocked on purpose". One or two unchecked rows +stay silent, and so does a hook that checked something on any run. The SDK +runner writes unchecked rule ids into `rule_ids` even when the hook let the +turn through, so a fail-open check shows up here. + An unreadable metrics log emits this unchecked notice instead: ```text diff --git a/engine/hooks/hook-health/detect.py b/engine/hooks/hook-health/detect.py index 87e5869c7..665c2430b 100644 --- a/engine/hooks/hook-health/detect.py +++ b/engine/hooks/hook-health/detect.py @@ -16,6 +16,7 @@ from finding import Finding # noqa: E402 FAILURES = {"crashed", "timed_out", "caught_error"} +UNCHECKED_RUN_MINIMUM = 3 STALE_SCAN_SECONDS = 120 RULE_FAILED_RUNS = "hook-health.failed-runs" @@ -71,18 +72,65 @@ def failures( ] +def is_unchecked(row: dict) -> bool: + if row.get("action") == "unchecked": + return True + rule_ids = row.get("rule_ids") + if not isinstance(rule_ids, list) or not rule_ids: + return False + return all(isinstance(rule_id, str) and rule_id.endswith(".unchecked") for rule_id in rule_ids) + + +def unchecked_signature(row: dict) -> str: + return json.dumps(["unchecked", row.get("hook"), first_stderr_line(row)]) + + +def always_unchecked( + rows: list[dict], harness: str, seen: frozenset[str] = frozenset(), session: str | None = None, +) -> dict[str, list[dict]]: + """Hooks whose every run since the last prompt was unchecked, with those runs.""" + by_hook: dict[str, list[dict]] = {} + for row in rows: + hook = row.get("hook") + if row.get("harness") != harness or not isinstance(hook, str) or hook == "hook-health" or not in_session(row, session): + continue + by_hook.setdefault(hook, []).append(row) + return { + hook: runs + for hook, runs in by_hook.items() + if len(runs) >= UNCHECKED_RUN_MINIMUM + and all(is_unchecked(row) for row in runs) + and unchecked_signature(runs[-1]) not in seen + } + + +def unchecked_line(hook: str, runs: list[dict]) -> str: + stderr = first_stderr_line(runs[-1]) + suffix = f": {stderr}" if stderr else "" + return ( + f"hook-health: {hook} reported unchecked on all {len(runs)} runs since the last prompt{suffix}; " + "it is not checking anything." + ) + + def notice( rows: list[dict], harness: str, seen: frozenset[str] = frozenset(), session: str | None = None, ) -> str | None: failed = failures(rows, harness, seen, session) + unchecked = always_unchecked(rows, harness, seen, session) blocked = sum( 1 for row in rows - if row.get("harness") == harness and row.get("hook") != "hook-health" - and row.get("outcome") == "blocked" and in_session(row, session) + if row.get("harness") == harness + and row.get("hook") != "hook-health" + and row.get("outcome") == "blocked" + and in_session(row, session) + and not is_unchecked(row) ) if not failed: - return None + if not unchecked: + return None + return "\n".join(unchecked_line(hook, runs) for hook, runs in unchecked.items()) distinct: dict[str, dict] = {} for row in failed: distinct.setdefault(signature(row), row) @@ -101,6 +149,8 @@ def notice( f"hook-health: {len(failed)} hook run(s) failed since the last prompt: " f"{'; '.join(parts)} -- run python3 ~/.claude/hooks/_runner/report.py for the table." ) + for hook, runs in unchecked.items(): + text += "\n" + unchecked_line(hook, runs) if blocked: text += ( f"\nhook-health: {blocked} hook run(s) blocked on purpose " @@ -331,7 +381,9 @@ def scan(harness: str, session: str) -> None: if error is not None: text = unreadable_notice(str(log), error) else: - fresh = frozenset(signature(row) for row in failures(rows, harness, seen, session)) + fresh = frozenset(signature(row) for row in failures(rows, harness, seen, session)) | frozenset( + unchecked_signature(runs[-1]) for runs in always_unchecked(rows, harness, seen, session).values() + ) if new_offset != offset or fresh: write_offset(state, new_offset, seen | fresh) text = notice(rows, harness, seen, session) diff --git a/engine/hooks/hook-health/tests/test_hooks.py b/engine/hooks/hook-health/tests/test_hooks.py index 547a4735f..6b44a3bd4 100644 --- a/engine/hooks/hook-health/tests/test_hooks.py +++ b/engine/hooks/hook-health/tests/test_hooks.py @@ -13,6 +13,20 @@ HOOK_DIR = Path(__file__).resolve().parents[1] RUNNER_DIR = HOOK_DIR.parent / "_runner" +sys.path.insert(0, str(HOOK_DIR)) + +import detect # noqa: E402 + +UNCHECKED_ROW = { + "hook": "technical-writing-guard", + "script": "claude_stop_check.py", + "outcome": "blocked", + "exit_code": 2, + "rule_ids": ["technical-writing-guard.unchecked"], + "stderr_tail": "The document check could not load. Leave the reply unchecked.\n", + "harness": "claude", + "event": "Stop", +} HARNESS_SCRIPTS = { "claude": "claude_prompt_submit.py", "cursor": "cursor_before_submit.py", @@ -219,5 +233,36 @@ def test_malformed_payload_logs_caught_error_and_exits_zero(self) -> None: self.assertIn("catstack-hook-error hook-health: JSONDecodeError", result.stderr) +class UncheckedOnEveryRun(unittest.TestCase): + def test_hook_unchecked_on_every_run_is_reported(self) -> None: + rows = [dict(UNCHECKED_ROW) for _ in range(3)] + text = detect.notice(rows, "claude") + self.assertIsNotNone(text) + self.assertIn("technical-writing-guard reported unchecked on all 3 runs since the last prompt", text) + self.assertIn("The document check could not load. Leave the reply unchecked.", text) + self.assertIn("it is not checking anything", text) + self.assertNotIn("blocked on purpose", text) + + def test_unchecked_rows_logged_as_action_are_reported(self) -> None: + row = {**UNCHECKED_ROW, "outcome": "silent", "exit_code": 0, "rule_ids": [], "action": "unchecked"} + text = detect.notice([dict(row) for _ in range(4)], "claude") + self.assertIsNotNone(text) + self.assertIn("reported unchecked on all 4 runs", text) + + def test_single_unchecked_run_stays_quiet(self) -> None: + self.assertIsNone(detect.notice([dict(UNCHECKED_ROW)], "claude")) + self.assertIsNone(detect.notice([dict(UNCHECKED_ROW) for _ in range(2)], "claude")) + + def test_hook_that_also_checked_something_stays_quiet(self) -> None: + rows = [dict(UNCHECKED_ROW) for _ in range(3)] + rows.append({**UNCHECKED_ROW, "rule_ids": ["technical-writing-guard.sentence"]}) + self.assertIsNone(detect.notice(rows, "claude")) + + def test_unchecked_notice_is_not_repeated_once_seen(self) -> None: + rows = [dict(UNCHECKED_ROW) for _ in range(3)] + seen = frozenset(detect.unchecked_signature(row) for row in rows) + self.assertIsNone(detect.notice(rows, "claude", seen)) + + if __name__ == "__main__": unittest.main() diff --git a/engine/hooks/no-comments/README.md b/engine/hooks/no-comments/README.md index 1dbd9cf65..500f7a983 100644 --- a/engine/hooks/no-comments/README.md +++ b/engine/hooks/no-comments/README.md @@ -16,6 +16,13 @@ a `*/` with no opener before it marks the lines above it as inside. An edit made only of `*` lines, with neither `/*` nor `*/`, is treated as outside and passes. +Only a file inside a git work tree is judged: the hook walks up from the +file's nearest existing parent looking for a `.git` entry, with no +subprocess. A scratchpad or `/tmp` file has no commit message or blame to +carry the story, so an edit there passes. A path whose nearest existing +parent is the filesystem root says nothing about where it lives, so it is +still judged (fail closed on an unknown location). + `scripts/ci/check_no_new_comments.py` is the CI twin: it fails when a diff against the base adds comment lines to code, using the same detector. diff --git a/engine/hooks/no-comments/detect.py b/engine/hooks/no-comments/detect.py index fc4ee4e28..93a75ec2c 100644 --- a/engine/hooks/no-comments/detect.py +++ b/engine/hooks/no-comments/detect.py @@ -11,6 +11,11 @@ A leading * counts only while a /* block comment is open. Outside one it is code, such as the CSS universal selector or quoted text in HTML. + +Only files inside a git work tree are judged. A scratchpad or /tmp file +has no commit message or blame to carry the story, so the ban does not +apply there. A path whose nearest existing ancestor is the filesystem +root tells nothing about where it lives, and is still judged. """ from __future__ import annotations @@ -63,6 +68,25 @@ def is_code_file(path: str) -> bool: return path.lower().endswith(CODE_SUFFIXES) +def inside_git_work_tree(path: str) -> bool | None: + """True when a .git entry sits at or above the file's nearest existing + ancestor, False when none does, None when that ancestor is the root.""" + if not path: + return None + folder = Path(path).expanduser() + if not folder.is_absolute(): + folder = Path.cwd() / folder + folder = folder.parent + while not folder.exists() and folder.parent != folder: + folder = folder.parent + if folder.parent == folder: + return None + for ancestor in (folder, *folder.parents): + if (ancestor / ".git").exists(): + return True + return False + + def _strip_strings(line: str) -> str: return STRING_RE.sub('""', line) @@ -167,6 +191,8 @@ def detect(event: dict[str, object]) -> list[Finding]: found: list[str] = [] path = "" for path, text in added_text(tool_name, tool_input): + if inside_git_work_tree(path) is False: + continue found.extend(comment_lines(path, text)) if not found: return [] diff --git a/engine/hooks/no-comments/tests/test_hooks.py b/engine/hooks/no-comments/tests/test_hooks.py index 0de7181dc..38f93c6ba 100644 --- a/engine/hooks/no-comments/tests/test_hooks.py +++ b/engine/hooks/no-comments/tests/test_hooks.py @@ -9,6 +9,7 @@ import json import os import sys +import tempfile import unittest from contextlib import redirect_stderr from unittest.mock import patch @@ -177,5 +178,36 @@ def test_fails_open_on_garbage_stdin(self): self.assertEqual(err.getvalue(), "") +SCRATCHPAD_HTML = "\n\n\n
\n" + + +class TestGitWorkTreeScope(unittest.TestCase): + def _tmp(self) -> str: + folder = tempfile.TemporaryDirectory() + self.addCleanup(folder.cleanup) + return folder.name + + def test_scratchpad_html_outside_a_repo_is_not_checked(self): + root = self._tmp() + path = os.path.join(root, "scratchpad", "arch", "ci-whatif-architecture.html") + payload = {"tool_name": "Write", "tool_input": {"file_path": path, "content": SCRATCHPAD_HTML}} + self.assertEqual(detect.detect(payload), []) + code, err = run_hook(payload) + self.assertEqual((code, err), (0, "")) + self.assertIs(detect.inside_git_work_tree(path), False) + + def test_html_inside_a_repo_still_fires(self): + root = self._tmp() + os.mkdir(os.path.join(root, ".git")) + path = os.path.join(root, "docs", "page.html") + payload = {"tool_name": "Write", "tool_input": {"file_path": path, "content": SCRATCHPAD_HTML}} + self.assertEqual(len(detect.detect(payload)), 1) + self.assertIs(detect.inside_git_work_tree(path), True) + + def test_file_at_an_unknown_location_is_still_checked(self): + self.assertIsNone(detect.inside_git_work_tree("/repo/hook.py")) + self.assertIsNotNone(detect.decide(edit("/repo/hook.py", REAL_PY_COMMENT))) + + if __name__ == "__main__": unittest.main() diff --git a/engine/hooks/prove-it-ship-gate/README.md b/engine/hooks/prove-it-ship-gate/README.md index 89eebe2a0..5a4f5c810 100644 --- a/engine/hooks/prove-it-ship-gate/README.md +++ b/engine/hooks/prove-it-ship-gate/README.md @@ -63,6 +63,8 @@ that command, and `apply_patch` counts as the edit. A later `vitest` / `pnpm tes `pytest` run does not replace it. The hook names the missing program and subcommand and blocks. Unix pipelines, `node -e` scripts, and a session that never ran a product command stay silent. A quoted program-and-subcommand pair inside a script counts as that command. +A shell variable assignment at the start of a segment (`K=~/path; ls $K`, `INVOKER_HOME=/tmp/x invoker-cli doctor`) +is skipped like `sudo` or `env`: the assignment's value is never read as a program, and the program after it still counts. The transcript read fails open. Mechanical half of `corpus/skills/prove-it-ship-gate`. Judgment (is this work diff --git a/engine/hooks/prove-it-ship-gate/detect.py b/engine/hooks/prove-it-ship-gate/detect.py index 917115b8c..ce7a4f77e 100644 --- a/engine/hooks/prove-it-ship-gate/detect.py +++ b/engine/hooks/prove-it-ship-gate/detect.py @@ -55,6 +55,7 @@ "MultiEdit", "NotebookEdit", "edit_file", }) COMMAND_PREFIXES = frozenset({"sudo", "env", "time", "command", "nohup", "nice"}) +SHELL_ASSIGNMENT_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*=") GENERIC_PROGRAMS = frozenset({ "git", "node", "python", "python3", "bash", "zsh", "sh", "ls", "cat", "rg", "grep", "find", "mkdir", "rm", "cp", "mv", "ln", "sed", "awk", "jq", "gh", @@ -306,7 +307,7 @@ def surface_keys(command: str) -> list[str]: words = shlex.split(segment, posix=True) except ValueError: continue - while words and words[0] in COMMAND_PREFIXES: + while words and (words[0] in COMMAND_PREFIXES or SHELL_ASSIGNMENT_RE.match(words[0])): words = words[1:] if not words: continue diff --git a/engine/hooks/prove-it-ship-gate/tests/test_hooks.py b/engine/hooks/prove-it-ship-gate/tests/test_hooks.py index 5dbd187dd..3f2248836 100644 --- a/engine/hooks/prove-it-ship-gate/tests/test_hooks.py +++ b/engine/hooks/prove-it-ship-gate/tests/test_hooks.py @@ -501,6 +501,26 @@ def write_cursor_transcript(directory, name, events): class TestSameRepro(unittest.TestCase): + def test_surface_keys_skip_shell_variable_assignments(self): + self.assertEqual( + detect.surface_keys( + 'K=~/.claude/skills/invoker-plan-to-invoker; ls $K; echo "--- scripts ---"; ' + "ls $K/scripts 2>/dev/null | head -30" + ), + [], + ) + self.assertEqual( + detect.surface_keys( + 'F=~/.claude/skills/invoker-plan-to-invoker/formulas/implementation-stack; ' + 'ls "$F"; sed -n 1,80p "$F"/README.md' + ), + [], + ) + self.assertEqual( + detect.surface_keys("INVOKER_HOME=/tmp/x invoker-cli doctor"), + ["invoker-cli doctor"], + ) + def test_surface_keys_keep_the_real_command_and_drop_scripts(self): self.assertEqual(detect.surface_keys("invoker-cli --help"), ["invoker-cli --help"]) self.assertEqual(detect.surface_keys("invoker-cli doctor"), ["invoker-cli doctor"]) diff --git a/engine/hooks/technical-writing-guard/README.md b/engine/hooks/technical-writing-guard/README.md index b1d8b6ebb..2c213b84d 100644 --- a/engine/hooks/technical-writing-guard/README.md +++ b/engine/hooks/technical-writing-guard/README.md @@ -4,6 +4,13 @@ Stop hook for a document the user asked for. It runs `product/skills/technical-writing/scripts/grade_prose.py` `sentence_findings` on the reply, after code fences and inline code are removed. +The grader is looked up in this order: the catstack checkout named on +line 1 of the hook snapshot's `.catstack-source`, then the installed skill +at `~/.claude/skills/technical-writing/scripts` (and the `.cursor` and +`.codex` twins), then three directories above the hook file. The installed +copy of this hook lives under `~/.cache`, so the last walk alone never +finds the grader there. + A hit blocks the turn. Each finding is its own result, so one reply can report every failed check. diff --git a/engine/hooks/technical-writing-guard/detect.py b/engine/hooks/technical-writing-guard/detect.py index f5bfb3533..26d8df84f 100644 --- a/engine/hooks/technical-writing-guard/detect.py +++ b/engine/hooks/technical-writing-guard/detect.py @@ -169,8 +169,8 @@ def detect(event: dict) -> list[Finding]: return [] if event.get("stop_hook_active"): return [] - event = enrich(event) - asked = user_text(event) + enriched = enrich(event) + asked = user_text(enriched) if asked is None: return [] try: @@ -184,6 +184,7 @@ def detect(event: dict) -> list[Finding]: evidence=f"grade_prose import failed: {error}", ) ] + event = enriched if not grader.applies_to(asked): return [] message = event.get("last_assistant_message") diff --git a/engine/hooks/technical-writing-guard/tests/test_hooks.py b/engine/hooks/technical-writing-guard/tests/test_hooks.py index 73d240211..9ddbd7f89 100644 --- a/engine/hooks/technical-writing-guard/tests/test_hooks.py +++ b/engine/hooks/technical-writing-guard/tests/test_hooks.py @@ -12,6 +12,8 @@ import unittest HOOK_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +HOOKS_ROOT = os.path.dirname(HOOK_DIR) +REPO_DIR = os.path.dirname(os.path.dirname(HOOKS_ROOT)) REPO_ROOT = os.path.abspath(os.path.join(HOOK_DIR, "..", "..", "..")) SKILL_FIXTURES = os.path.join( HOOK_DIR, "..", "..", "..", "product", "skills", "technical-writing", "tests", "fixtures" @@ -177,6 +179,49 @@ def test_stop_hook_active_passes(self): }) self.assertEqual(findings, []) + def _snapshot_outside_the_repo(self) -> tuple[str, str]: + folder = tempfile.TemporaryDirectory() + self.addCleanup(folder.cleanup) + snapshot = os.path.join(folder.name, ".cache", "catstack-hooks-snapshot") + os.makedirs(snapshot) + for name in ("_sdk", "_flags", "technical-writing-guard"): + shutil.copytree(os.path.join(HOOKS_ROOT, name), os.path.join(snapshot, name)) + shutil.copy(os.path.join(HOOKS_ROOT, "hooks.toml"), snapshot) + with open(os.path.join(snapshot, ".catstack-source"), "w", encoding="utf-8") as handle: + handle.write(REPO_DIR + "\nunknown\nmain\n") + return folder.name, snapshot + + def _run_snapshot_stop(self, home: str, snapshot: str, reply: str) -> subprocess.CompletedProcess: + env = { + "PATH": os.environ.get("PATH", ""), + "HOME": home, + "CATSTACK_HOOK_METRICS_DIR": os.path.join(home, "metrics"), + } + payload = json.dumps({ + "hook_event_name": "Stop", + "user_prompt": "Rewrite this README section so a tired engineer gets it on the first read.", + "last_assistant_message": reply, + }) + return subprocess.run( + [sys.executable, os.path.join(snapshot, "technical-writing-guard", "claude_stop_check.py")], + input=payload, + capture_output=True, + text=True, + env=env, + cwd=home, + timeout=30, + ) + + def test_runs_from_a_snapshot_outside_the_repo(self): + home, snapshot = self._snapshot_outside_the_repo() + bad = self._run_snapshot_stop(home, snapshot, BEFORE) + self.assertNotIn("could not load", bad.stderr, bad.stderr) + self.assertEqual(bad.returncode, 2, bad.stderr) + self.assertIn("passive setup", bad.stderr) + good = self._run_snapshot_stop(home, snapshot, AFTER) + self.assertNotIn("could not load", good.stderr, good.stderr) + self.assertEqual(good.returncode, 0, good.stderr) + SNAPSHOT_PROBE = """ import json, sys diff --git a/engine/hooks/wait-needs-wakeup/README.md b/engine/hooks/wait-needs-wakeup/README.md index de4d9653e..831c934f5 100644 --- a/engine/hooks/wait-needs-wakeup/README.md +++ b/engine/hooks/wait-needs-wakeup/README.md @@ -20,7 +20,10 @@ wait to the harness and names a clock-time ETA. Two entrypoints, one rule: identical call). Equality of the typed command (or empty session poke) is the signal; status-check regexes are not extended for this rule. Hand the wait to a `run_in_background` exit-on-condition command or a - `Monitor` / `Agent` instead of reissuing the same command. + `Monitor` / `Agent` instead of reissuing the same command. A read-only + clock read (`date ...`, with or without a leading `TZ=...`) is never a + repeat: the Stop message asks for exactly that command, and a stale clock + is worse than a repeated one. - **PreToolUse on ScheduleWakeup (blocks, exit 2):** a wake budget (`WAIT_NEEDS_WAKEUP_BUDGET`, default 10). Every ScheduleWakeup resumes this same transcript, so wake count x context size is the session's @@ -41,6 +44,10 @@ wait to the harness and names a clock-time ETA. Two entrypoints, one rule: minutes", "about 100 min", "in 10 minutes") is a measured time, not wait language; it counts only after a forward cue such as "check back in", "wake in" or "again in". "not waiting on" is a negation and passes. + Wait words inside a + double-quoted, backticked or single-quoted span are someone citing them, + not waiting, and a reply that names `wait-needs-wakeup` itself is about + the hook, so both stay silent. The guidance ordering is deliberate: detached wakeups (background command that exits on its condition, Monitor/Agent) are named before diff --git a/engine/hooks/wait-needs-wakeup/detect.py b/engine/hooks/wait-needs-wakeup/detect.py index 422ee4200..0040e7cd0 100644 --- a/engine/hooks/wait-needs-wakeup/detect.py +++ b/engine/hooks/wait-needs-wakeup/detect.py @@ -81,6 +81,12 @@ re.IGNORECASE, ) QUOTED_BEFORE = ('"', "'", "`") +DOUBLE_QUOTE_RE = re.compile(r'"[^"]*"', re.DOTALL) +BACKTICK_RE = re.compile(r"`[^`]*`", re.DOTALL) +SINGLE_QUOTE_RE = re.compile(r"(? str | None: command = _command_from_input(inp) if not isinstance(command, str) or not command: return None + if is_clock_command(command): + return None priors = count_identical_shell_commands(transcript_path, command) if priors < 0 or priors < repeat_budget(environ): return None @@ -502,6 +510,23 @@ def _pretooluse_rule_id(reason: str) -> str: return RULE_FOREGROUND_POLL_LOOP +def is_clock_command(command: str) -> bool: + """A read-only clock read, the command the Stop message itself asks for.""" + return bool(CLOCK_COMMAND_RE.match(command or "")) + + +def quoted_spans(text: str) -> list[tuple[int, int]]: + """Character ranges of double-quoted, backticked and single-quoted runs.""" + spans: list[tuple[int, int]] = [] + for pattern in QUOTE_SPAN_RES: + spans.extend((m.start(), m.end()) for m in pattern.finditer(text or "")) + return spans + + +def _inside_quoted_span(position: int, spans: list[tuple[int, int]]) -> bool: + return any(start <= position < end for start, end in spans) + + def closing_question_start(text: str) -> int: """Offset of a final sentence that asks the user something, else -1. @@ -515,11 +540,17 @@ def closing_question_start(text: str) -> int: def is_wait_reply(text: str) -> bool: - """Wait language outside quotes (a reply that quotes the rule is not waiting) - and not superseded by a closing question to the user.""" + """Wait language outside quotes and not superseded by a closing question + to the user. A reply that quotes the wait words, or that talks about this + hook by name, is describing a wait, not waiting.""" text = text or "" + if HOOK_NAME in text.lower(): + return False + spans = quoted_spans(text) question_at = closing_question_start(text) for match in WAIT_RE.finditer(text): + if _inside_quoted_span(match.start(), spans): + continue if match.start() and text[match.start() - 1] in QUOTED_BEFORE: continue if question_at >= 0 and match.end() <= question_at: diff --git a/engine/hooks/wait-needs-wakeup/tests/test_hooks.py b/engine/hooks/wait-needs-wakeup/tests/test_hooks.py index 98ccd5cf7..fae2a4458 100644 --- a/engine/hooks/wait-needs-wakeup/tests/test_hooks.py +++ b/engine/hooks/wait-needs-wakeup/tests/test_hooks.py @@ -264,6 +264,23 @@ def test_allows_ten_minute_reply_with_schedule_wakeup_and_eta_via_hook(self): self.assertEqual(code, 0) self.assertEqual(err, "") + def test_describing_a_blocked_reply_is_not_waiting(self): + quoted = ( + 'The Stop hook rejected the last message because the reply said "it would report back" ' + "with no clock time. That message is now rewritten." + ) + self.assertIsNone(detect.decide_stop_from_lines(quoted, [])) + self.assertFalse(detect.is_wait_reply(quoted)) + about_the_hook = ( + "wait-needs-wakeup blocked the reply: it said it will report back and gave no ETA. " + "The review of the hook found that a reply about waiting is not itself a wait." + ) + self.assertIsNone(detect.decide_stop_from_lines(about_the_hook, [])) + self.assertFalse(detect.is_wait_reply(about_the_hook)) + + def test_unquoted_report_back_is_still_waiting(self): + self.assertTrue(detect.is_wait_reply("I will report back once the job lands.")) + def test_allows_when_stop_hook_active(self): case = load("wait_replies_fires.json")[0] self.assertIsNone(detect.decide_stop({ @@ -457,6 +474,38 @@ def test_fails_open_without_transcript_on_sleepless_cat(self): "transcript_path": "/nonexistent/x.jsonl", })) + def test_third_date_command_is_not_a_repeat(self): + clock = "date +%H:%M\\ %Z" + path = transcript_file([bash_tool_use(clock, "a"), bash_tool_use(clock, "b")]) + try: + code, err = run_entry(claude_pretooluse, { + "tool_name": "Bash", + "tool_input": {"command": clock, "run_in_background": False}, + "transcript_path": path, + }) + finally: + os.unlink(path) + self.assertEqual((code, err), (0, "")) + self.assertIsNone(detect.decide_repeat_command({ + "tool_name": "Bash", + "tool_input": {"command": "TZ=America/Los_Angeles date +%H:%M"}, + "transcript_path": "/nonexistent/never-read.jsonl", + })) + + def test_third_identical_curl_is_still_a_repeat(self): + curl = "curl -s https://ci.example/api/status" + path = transcript_file([bash_tool_use(curl, "a"), bash_tool_use(curl, "b")]) + try: + code, err = run_entry(claude_pretooluse, { + "tool_name": "Bash", + "tool_input": {"command": curl, "run_in_background": False}, + "transcript_path": path, + }) + finally: + os.unlink(path) + self.assertEqual(code, 2) + self.assertIn("repeat command", err) + def test_sleep_loop_rule_wins_over_repeat(self): command = "until grep -q '^exit=' out; do sleep 3; done" path = transcript_file([bash_tool_use(command, "a"), bash_tool_use(command, "b")]) diff --git a/engine/skills/reflect/references/cost-audit.md b/engine/skills/reflect/references/cost-audit.md index e419a36ce..cb0b15d9f 100644 --- a/engine/skills/reflect/references/cost-audit.md +++ b/engine/skills/reflect/references/cost-audit.md @@ -63,6 +63,10 @@ It sweeps the newest N (default 5) local Claude/OMP transcripts, or explicit pat A session can be stuck on one problem while every tool call is technically distinct — each Edit is different text, so the exact-duplicate detector above sees nothing. `token_audit.py` (Claude mode) also flags two shapes of this, both heuristic: (1) **recurring failure signatures** — tool errors whose text repeats (numbers normalized out) across more than one attempt, meaning the same failure keeps recurring rather than getting fixed; genuine user tool-rejections are excluded, since those are the user redirecting, not the agent failing; (2) **edit streaks without a verification run in between** — three or more `Edit`/`Write` calls to the same file with no intervening `Bash` call shaped like a test/build/lint/typecheck command, meaning the agent kept changing code without checking any of the changes. Both point at the same root cause: no fast feedback loop. Feed these counts to the Judgment lens (step 3) alongside the cost numbers. +## Hooks that could not check + +`token_audit.py` (Claude mode) also flags `hook-unchecked-blocks`: Stop-hook feedback rows whose text says the check could not run ("Leave the reply unchecked", "could not load"), with the hook name read from the runner's `--timeout N /` string. Three or more in one session is `yes`: that hook stopped replies without judging any of them, which is a broken install, not a working gate. + ## Cross-machine remotes Other machines this user runs agents on are listed (by name only, never by host/IP) in `~/.invoker/config.json` under `remoteTargets`, if that file exists. `token_audit.py remotes` surfaces which target names *could* be scanned over SSH. It never SSHes itself — actually reaching into a remote machine is a separate, explicitly-confirmed step. A confirmation to scan remote hosts, given before the exact command exists, authorizes the *scope*, not the *payload* — before fanning a script out over SSH to N hosts, show the exact command or script once so the user has actually seen what ran. diff --git a/engine/skills/reflect/scripts/tests/test_e2e_sample_conversations.py b/engine/skills/reflect/scripts/tests/test_e2e_sample_conversations.py index b55759abd..c9f0df74a 100644 --- a/engine/skills/reflect/scripts/tests/test_e2e_sample_conversations.py +++ b/engine/skills/reflect/scripts/tests/test_e2e_sample_conversations.py @@ -35,6 +35,7 @@ "intervention-must-automate", "brevity-follow-ups", "brevity-hook-blocks", + "hook-unchecked-blocks", "self-retraction", "subagent-thrash", ) @@ -42,6 +43,7 @@ name for name in FLAG_NAMES if name not in ( "self-retraction", "subagent-thrash", "brevity-follow-ups", "brevity-hook-blocks", + "hook-unchecked-blocks", ) ) diff --git a/engine/skills/reflect/scripts/tests/test_token_audit.py b/engine/skills/reflect/scripts/tests/test_token_audit.py index 707b337cd..17916e301 100644 --- a/engine/skills/reflect/scripts/tests/test_token_audit.py +++ b/engine/skills/reflect/scripts/tests/test_token_audit.py @@ -1337,6 +1337,7 @@ class TestOutFlags(unittest.TestCase): "brevity-hook-blocks", "self-retraction", "subagent-thrash", + "hook-unchecked-blocks", } def _flag_by_name(self, report, name): @@ -2174,6 +2175,150 @@ def test_same_text_sent_before_the_queue_row_is_a_real_repeat(self): self.assertEqual(fr["kinds"].get("verbatim-repeat"), 1) +class TestMidTurnQueuedHumanSend(unittest.TestCase): + """A message typed mid-turn is an enqueue row with a sessionId, then a + remove row with reason absorbed_mid_turn, and never its own user row.""" + + def test_absorbed_mid_turn_send_is_counted_once(self): + usage = {"input_tokens": 1, "output_tokens": 1} + text = "lets review this code. also lets rename this from ci_whatif to ci_projection_time_savings" + lines = [ + claude_user_text_line("start with the parser", ts="2026-10-13T14:40:00.000Z"), + claude_assistant_line("m1", "u1", [{"type": "text", "text": "working"}], usage), + {"type": "queue-operation", "operation": "enqueue", "timestamp": "2026-10-13T14:50:01.179Z", + "sessionId": "c80f0218", "content": text}, + {"type": "queue-operation", "operation": "remove", "timestamp": "2026-10-13T14:56:12.297Z", + "sessionId": "c80f0218", "reason": "absorbed_mid_turn", "commandUuid": "9015", "deliveryId": "9d3a", + "content": text}, + claude_assistant_line("m2", "u2", [{"type": "text", "text": "renamed"}], usage), + ] + path = write_jsonl(lines) + try: + with redirect_stdout(io.StringIO()): + result = token_audit.audit_claude(path, include_subagents=False) + finally: + os.unlink(path) + self.assertEqual(result["frustration"]["n_user_messages"], 2) + + +class TestArtifactPublishIsVerification(unittest.TestCase): + def _audit(self, lines): + path = write_jsonl(lines) + try: + with redirect_stdout(io.StringIO()): + result = token_audit.audit_claude(path, include_subagents=False) + finally: + os.unlink(path) + return result, {f["name"]: f for f in result["flags"]} + + def test_artifact_publish_of_the_edited_page_ends_the_streak(self): + u = {"input_tokens": 1, "output_tokens": 1, "cache_read_input_tokens": 0, "cache_creation_input_tokens": 0} + page = "/private/tmp/scratchpad/arch/ci-whatif-architecture.html" + edit = {"type": "tool_use", "name": "Edit", "input": {"file_path": page, "old_string": "a", "new_string": "b"}} + publish = {"type": "tool_use", "name": "Artifact", "input": {"action": "publish", "file_path": page}} + lines = [ + claude_assistant_line("m1", "u1", [{**edit, "id": "t1"}], u), + claude_assistant_line("m2", "u2", [{**edit, "id": "t2"}], u), + claude_assistant_line("m3", "u3", [{**publish, "id": "t3"}], u), + claude_assistant_line("m4", "u4", [{**edit, "id": "t4"}], u), + claude_assistant_line("m5", "u5", [{**edit, "id": "t5"}], u), + claude_assistant_line("m6", "u6", [{**publish, "id": "t6"}], u), + ] + result, flags = self._audit(lines) + self.assertEqual(result["longest_edit_streak_no_verify"], 2) + self.assertEqual(flags["no-verify-edit-streak"]["value"], "no") + self.assertIn("artifact publishes=2", flags["no-verify-edit-streak"]["rationale"]) + + def test_artifact_publish_of_another_file_does_not_end_the_streak(self): + u = {"input_tokens": 1, "output_tokens": 1, "cache_read_input_tokens": 0, "cache_creation_input_tokens": 0} + edit = {"type": "tool_use", "name": "Edit", "input": {"file_path": "/r/a.py"}} + publish = {"type": "tool_use", "name": "Artifact", "input": {"action": "publish", "file_path": "/r/other.html"}} + lines = [ + claude_assistant_line("m1", "u1", [{**edit, "id": "t1"}], u), + claude_assistant_line("m2", "u2", [{**edit, "id": "t2"}], u), + claude_assistant_line("m3", "u3", [{**publish, "id": "t3"}], u), + claude_assistant_line("m4", "u4", [{**edit, "id": "t4"}], u), + ] + result, _ = self._audit(lines) + self.assertEqual(result["longest_edit_streak_no_verify"], 3) + + +class TestCacheSpikeDetails(unittest.TestCase): + def test_each_spike_names_the_idle_gap_and_the_preceding_user_row_size(self): + small = {"input_tokens": 1, "output_tokens": 1, "cache_read_input_tokens": 0, "cache_creation_input_tokens": 1000} + big = {"input_tokens": 1, "output_tokens": 1, "cache_read_input_tokens": 0, "cache_creation_input_tokens": 100_000} + def at(line, ts): + return {**line, "timestamp": ts} + lines = [ + at(claude_user_text_line("short"), "2026-10-13T10:00:00Z"), + at(claude_assistant_line("m1", "u1", [{"type": "text", "text": "a"}], small), "2026-10-13T10:00:05Z"), + at(claude_assistant_line("m2", "u2", [{"type": "text", "text": "b"}], small), "2026-10-13T10:00:10Z"), + at(claude_assistant_line("m3", "u3", [{"type": "text", "text": "c"}], small), "2026-10-13T10:00:15Z"), + at(claude_user_text_line("x" * 500), "2026-10-13T12:13:15Z"), + at(claude_assistant_line("m4", "u4", [{"type": "text", "text": "d"}], big), "2026-10-13T12:13:20Z"), + ] + path = write_jsonl(lines) + try: + with redirect_stdout(io.StringIO()): + result = token_audit.audit_claude(path, include_subagents=False) + finally: + os.unlink(path) + flag = {f["name"]: f for f in result["flags"]}["cache-creation-spikes"] + self.assertEqual(flag["value"], "yes") + self.assertEqual(len(flag["details"]), 1) + detail = flag["details"][0] + self.assertEqual(detail["cache_creation"], 100_000) + self.assertEqual(detail["gap_seconds_since_previous_assistant"], 2 * 3600 + 13 * 60 + 5) + self.assertEqual(detail["preceding_user_chars"], 500) + + +def unchecked_hook_block(hook="technical-writing-guard", text="The document check could not load. Leave the reply unchecked."): + return claude_stop_hook_line( + "Stop hook feedback:\n[/Users/me/.local/bin/python3 $HOME/.claude/hooks/_runner/run.py " + f"--timeout 9.5 {hook}/claude_stop_check.py]: {text}\n" + ) + + +class TestHookUncheckedBlocks(unittest.TestCase): + def _flags(self, lines): + path = write_jsonl(lines) + try: + with redirect_stdout(io.StringIO()): + result = token_audit.audit_claude(path, include_subagents=False) + finally: + os.unlink(path) + return {f["name"]: f for f in result["flags"]} + + def test_three_unchecked_blocks_from_one_hook_fire(self): + usage = {"input_tokens": 1, "output_tokens": 1} + flags = self._flags([ + claude_user_text_line("write the README", ts="2026-10-13T10:00:00Z"), + claude_assistant_line("m1", "u1", [{"type": "text", "text": "draft"}], usage), + unchecked_hook_block(), + claude_assistant_line("m2", "u2", [{"type": "text", "text": "draft"}], usage), + unchecked_hook_block(), + claude_assistant_line("m3", "u3", [{"type": "text", "text": "draft"}], usage), + unchecked_hook_block(), + claude_stop_hook_line(diu_stop_block()), + ]) + flag = flags["hook-unchecked-blocks"] + self.assertEqual(flag["value"], "yes") + self.assertEqual(flag["count"], 3) + self.assertIn("technical-writing-guard=3", flag["rationale"]) + + def test_two_unchecked_blocks_stay_no(self): + usage = {"input_tokens": 1, "output_tokens": 1} + flags = self._flags([ + claude_user_text_line("write the README", ts="2026-10-13T10:00:00Z"), + claude_assistant_line("m1", "u1", [{"type": "text", "text": "draft"}], usage), + unchecked_hook_block(), + unchecked_hook_block(hook="other-guard", text="The transcript could not load."), + ]) + flag = flags["hook-unchecked-blocks"] + self.assertEqual(flag["value"], "no") + self.assertEqual(flag["count"], 2) + + def claude_queued_command_line(prompt, origin="human", command_mode="prompt", ts=None): attachment = {"type": "queued_command", "prompt": prompt, "commandMode": command_mode} if origin: diff --git a/engine/skills/reflect/scripts/token_audit.py b/engine/skills/reflect/scripts/token_audit.py index 4863f0e50..789320286 100644 --- a/engine/skills/reflect/scripts/token_audit.py +++ b/engine/skills/reflect/scripts/token_audit.py @@ -201,6 +201,10 @@ def _direct_run_targets(command): BREVITY_HOOK_NAME = "diu-stop" _STOP_HOOK_BLOCK_PREFIX = "Stop hook feedback" _STOP_HOOK_SCRIPT_RE = re.compile(r"([A-Za-z0-9_-]+)/[a-z0-9_]+\.py") +_STOP_HOOK_RUNNER_RE = re.compile(r"--timeout\s+[\d.]+\s+([A-Za-z0-9_-]+)/") +_UNCHECKED_BLOCK_MARKERS = ("Leave the reply unchecked", "could not load") +HOOK_UNCHECKED_BLOCKS_THRESHOLD = 3 +ARTIFACT_TOOL_NAME = "Artifact" _CLAUDE_COMMAND_NAME_RE = re.compile(r"\s*(?P[^<]+?)\s*", re.DOTALL) _CLAUDE_COMMAND_BLOCK_RE = re.compile(r".*?", re.DOTALL) _INLINE_COMMAND_RE = re.compile(r"(?:^|\s)(?P/[a-z][\w-]*)\b", re.I) @@ -395,6 +399,67 @@ def _claude_hook_block_names(path, rows): return names +def _claude_unchecked_hook_blocks(path, rows): + """Hook names from each Stop-hook block that said its own check could not + run. Such a block stops the reply without judging it, so three or more in + one session mean a hook is failing closed, not doing its job.""" + names = Counter() + if "/subagents/" in path.replace("\\", "/"): + return names + for row in rows: + if row.get("type") != "user" or row.get("agentId") or row.get("isSidechain"): + continue + message = row.get("message") + if not isinstance(message, dict): + continue + text = transcript_provenance._text_from_content(message.get("content")) + if not text.lstrip().startswith(_STOP_HOOK_BLOCK_PREFIX): + continue + if not any(marker in text for marker in _UNCHECKED_BLOCK_MARKERS): + continue + hooks = set(_STOP_HOOK_RUNNER_RE.findall(text)) or set(_STOP_HOOK_SCRIPT_RE.findall(text)) or {"unknown-hook"} + for name in hooks: + names[name] += 1 + return names + + +def _hook_unchecked_blocks_flag(counts): + total = sum(counts.values()) + if not total: + return _flag("hook-unchecked-blocks", "no", 0, "no Stop-hook block said its check could not run") + per_hook = ", ".join(f"{name}={n}" for name, n in counts.most_common()) + return _flag( + "hook-unchecked-blocks", + "yes" if total >= HOOK_UNCHECKED_BLOCKS_THRESHOLD else "no", + total, + f"{total} Stop-hook block(s) said the check could not run ({per_hook}); " + "each one stopped a reply without checking it", + ) + + +def _string_values(obj, depth=0): + if isinstance(obj, str): + yield obj + elif depth < 3 and isinstance(obj, dict): + for value in obj.values(): + yield from _string_values(value, depth + 1) + elif depth < 3 and isinstance(obj, list): + for value in obj: + yield from _string_values(value, depth + 1) + + +def _artifact_names_an_edited_file(inp, edited_paths): + """An Artifact call that publishes a file just edited is that file's run step.""" + edited = {fp for fp in edited_paths if fp} + edited |= {os.path.basename(fp) for fp in edited} + if not edited: + return False + for value in _string_values(inp): + if value in edited or os.path.basename(value) in edited: + return True + return False + + def _brevity_hook_blocks_flag(blocks, requests): """Times the brevity checker stopped a reply, next to the times the person had to ask. Both counts read the same session; only the second one means @@ -1075,8 +1140,12 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): intervention_commands = _claude_intervention_command_counts(path, lines) brevity_diu_commands, brevity_eli5_only = _claude_brevity_follow_ups(path, lines) brevity_hook_blocks = _claude_hook_block_names(path, lines)[BREVITY_HOOK_NAME] + unchecked_hook_blocks = _claude_unchecked_hook_blocks(path, lines) msg_usage = {} msg_first_seq = {} + msg_context = {} + last_user_chars = 0 + prev_assistant_ts = None models = Counter() tool_use = {} tool_calls_seq = [] @@ -1105,6 +1174,11 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): msg_usage[mid] = {k: 0 for k in USAGE_FIELDS} msg_first_seq[mid] = seq models[msg.get("model", "?")] += 1 + row_ts = _ts_seconds(d.get("timestamp")) + gap = row_ts - prev_assistant_ts if row_ts is not None and prev_assistant_ts is not None else None + msg_context[mid] = {"gap": gap, "user_chars": last_user_chars} + if row_ts is not None: + prev_assistant_ts = row_ts peak = msg_usage[mid] for k in USAGE_FIELDS: peak[k] = max(peak[k], u.get(k, 0)) @@ -1121,6 +1195,7 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): assistant_texts.append(text) elif d.get("type") == "user": content = d.get("message", {}).get("content") + last_user_chars = len(content) if isinstance(content, str) else len(json.dumps(content)) if content is not None else 0 human_text = content if isinstance(content, str) else None if isinstance(content, list): has_tool_result = False @@ -1151,7 +1226,7 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): total_cache_read = sum(u["cache_read_input_tokens"] for u in msg_usage.values()) total_cache_creation = sum(u["cache_creation_input_tokens"] for u in msg_usage.values()) cache_points = [ - (msg_first_seq[mid], u["cache_creation_input_tokens"], u["cache_read_input_tokens"]) + (msg_first_seq[mid], u["cache_creation_input_tokens"], u["cache_read_input_tokens"], mid) for mid, u in msg_usage.items() ] @@ -1212,7 +1287,7 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): recurring = {k: v for k, v in sig_groups.items() if len(v) > 1} edits_since_verify, file_streak_max = {}, {} - global_streak = global_streak_max = verify_count = direct_run_verify_count = 0 + global_streak = global_streak_max = verify_count = direct_run_verify_count = artifact_verify_count = 0 THRESH = 3 for s, name, inp, ident in sorted(tool_calls_seq, key=lambda x: x[0] or 0): if name == "Bash" and isinstance(inp, dict) and VERIFY_RE.search(inp.get("command") or ""): @@ -1227,6 +1302,10 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): direct_run_verify_count += 1 edits_since_verify.clear() global_streak = 0 + elif name == ARTIFACT_TOOL_NAME and isinstance(inp, dict) and _artifact_names_an_edited_file(inp, edits_since_verify): + artifact_verify_count += 1 + edits_since_verify.clear() + global_streak = 0 elif name in ("Edit", "Write") and isinstance(inp, dict): fp = _claude_tool_path(inp) edits_since_verify[fp] = edits_since_verify.get(fp, 0) + 1 @@ -1235,17 +1314,28 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): global_streak_max = max(global_streak_max, global_streak) flagged_files = {fp: n for fp, n in file_streak_max.items() if n >= THRESH} - creations = sorted(c for _, c, r in cache_points if c > 0) + creations = sorted(c for _, c, r, _mid in cache_points if c > 0) spikes = [] median = 0 if creations: median = creations[len(creations)//2] threshold = max(50_000, median * 5) seen = set() - for s, c, r in cache_points: + for s, c, r, mid in cache_points: if c >= threshold and c not in seen: seen.add(c) - spikes.append((s, c, r)) + spikes.append((s, c, r, mid)) + spike_details = [] + for s, c, r, mid in spikes: + context = msg_context.get(mid) or {} + gap = context.get("gap") + spike_details.append({ + "seq": s, + "cache_creation": c, + "cache_read": r, + "gap_seconds_since_previous_assistant": int(round(gap)) if gap is not None else None, + "preceding_user_chars": context.get("user_chars"), + }) frustration = frustration_signals( user_msgs, n_interruptions, failed_turn_indices=failed_turn_indices, @@ -1295,24 +1385,31 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): ( f"longest edit streak with zero verification: {global_streak_max}; " f"{len(flagged_files)} file(s) at or above threshold {THRESH}; " - f"verify Bash calls={verify_count}, direct-run verifies={direct_run_verify_count}" + f"verify Bash calls={verify_count}, direct-run verifies={direct_run_verify_count}, " + f"artifact publishes={artifact_verify_count}" ), ), - _flag( - "cache-creation-spikes", - "yes" if spikes else "no", - len(spikes), - ( - f"{len(spikes)} cache-creation spike(s) at/above max(50k, 5x median={median:,})" - if creations else "no cache-creation events in session" + { + **_flag( + "cache-creation-spikes", + "yes" if spikes else "no", + len(spikes), + ( + f"{len(spikes)} cache-creation spike(s) at/above max(50k, 5x median={median:,}); " + "each detail row carries the idle gap since the previous assistant turn and the size of the " + "user row before it, so an idle-gap rebuild reads apart from a big payload" + if creations else "no cache-creation events in session" + ), ), - ), + "details": spike_details, + }, ] flags.extend(_frustration_flags(frustration)) flags.append(_brevity_follow_ups_flag(brevity_eli5_only, brevity_diu_commands)) flags.append(_brevity_hook_blocks_flag( brevity_hook_blocks, brevity_eli5_only + brevity_diu_commands, )) + flags.append(_hook_unchecked_blocks_flag(unchecked_hook_blocks)) retraction_hits = self_retraction_hits(assistant_texts) flags.append(_self_retraction_flag(retraction_hits)) subagents = audit_subagents(path, audit_started_at=audit_started_at) if include_subagents else None @@ -1445,8 +1542,15 @@ def audit_claude(path, out_path=None, include_subagents=True, judge=False): print(f"longest edit streak with zero verification in between: {global_streak_max}") print("-- cache-creation spikes (fresh write, not cache read - expensive path) --") - for s, c, r in spikes: - print(f" seq~{s}: cache_creation={c:,} cache_read={r:,} (session median creation={median:,})") + for detail in spike_details: + gap = detail["gap_seconds_since_previous_assistant"] + gap_text = f"{gap:,}s" if gap is not None else "unknown" + print( + f" seq~{detail['seq']}: cache_creation={detail['cache_creation']:,} " + f"cache_read={detail['cache_read']:,} (session median creation={median:,}); " + f"gap since previous assistant turn={gap_text}; " + f"preceding user row={detail['preceding_user_chars']} chars" + ) _print_frustration(frustration) @@ -1628,7 +1732,7 @@ def audit_codex(path, out_path=None, judge=False): recurring = {k: v for k, v in sig_groups.items() if len(v) > 1} edits_since_verify, file_streak_max = {}, {} - global_streak = global_streak_max = verify_count = direct_run_verify_count = 0 + global_streak = global_streak_max = verify_count = direct_run_verify_count = artifact_verify_count = 0 for s, name, inp, ident in sorted(tool_calls_seq, key=lambda x: x[0] or 0): if name == "Bash" and isinstance(inp, dict) and VERIFY_RE.search(inp.get("command") or ""): verify_count += 1 @@ -1641,6 +1745,10 @@ def audit_codex(path, out_path=None, judge=False): direct_run_verify_count += 1 edits_since_verify.clear() global_streak = 0 + elif name == ARTIFACT_TOOL_NAME and isinstance(inp, dict) and _artifact_names_an_edited_file(inp, edits_since_verify): + artifact_verify_count += 1 + edits_since_verify.clear() + global_streak = 0 elif name == "Edit" and isinstance(inp, dict): fp = inp.get("file_path") edits_since_verify[fp] = edits_since_verify.get(fp, 0) + 1 @@ -1687,7 +1795,8 @@ def audit_codex(path, out_path=None, judge=False): ( f"longest edit streak with zero verification: {global_streak_max}; " f"{len(flagged_files)} file(s) at or above threshold 3; " - f"verify Bash calls={verify_count}, direct-run verifies={direct_run_verify_count}" + f"verify Bash calls={verify_count}, direct-run verifies={direct_run_verify_count}, " + f"artifact publishes={artifact_verify_count}" ), ), ])