From e34b32faac2bbf68fae0c769031515be5c937f29 Mon Sep 17 00:00:00 2001 From: Edbert Chan Date: Fri, 11 Sep 2026 02:24:05 -0700 Subject: [PATCH] hooks: a retry stops the word count, not the evidence checks diu-stop and prove-it-ship-gate both returned on stop_hook_active before any check ran. For diu-stop the reason was real -- trimming words reveals more words to trim, and nine consecutive blocks on one 150-word message were observed -- but returning early passed the whole message. The first block of a turn bought a free pass for whatever the rewrite said next, including a claim that was never checked. That is what happened in the session behind this change. The same message, sent as a retry, on 509b9cd and on this stack: before (509b9cd) exit=0 (silent) after exit=2 `UNVERIFIED:` is no longer an escape hatch -- it reads as ordinary prose and the claim beside it is judged on its own... A diu-stop retry now skips only the word count. prove-it-ship-gate has no word count, so its bypass was a pure free pass and is gone. These checks cannot loop the way the word count did: a well-formed tag always passes and every block message names it, so there is always a move that ends the turn. test_naming_the_blocker_ends_the_turn and test_naming_the_blocker_ends_the_turn_on_retry pin that. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VKsvxJk65w6q7KnPSRYvNg --- engine/hooks/diu-stop/claude_stop_check.py | 18 +++++--- .../hooks/diu-stop/tests/test_fix_matrix.py | 33 +++++++++---- engine/hooks/diu-stop/tests/test_hooks.py | 46 +++++++++++++++++++ engine/hooks/prove-it-ship-gate/detect.py | 2 - .../prove-it-ship-gate/tests/test_hooks.py | 13 +++++- 5 files changed, 94 insertions(+), 18 deletions(-) diff --git a/engine/hooks/diu-stop/claude_stop_check.py b/engine/hooks/diu-stop/claude_stop_check.py index 079f8f7..f6c04da 100755 --- a/engine/hooks/diu-stop/claude_stop_check.py +++ b/engine/hooks/diu-stop/claude_stop_check.py @@ -32,6 +32,16 @@ cheapest way to end a turn, and it was used that way. A tag that names no blocker is the same move wearing the new syntax, so it does not excuse either. + +`stop_hook_active` marks a rewrite after this hook already fired once this +turn. It used to return before every check, which made the first block of a +turn a free pass for whatever the rewrite said next -- a brand-new claim, +never checked. It now skips only the word-count check, which is the one +that actually loops: trimming words reveals more words to trim, and nine +consecutive blocks on one 150-word message were observed. The evidence +checks cannot loop that way, because a well-formed +{{CAT-UNVERIFIED: ... -- cannot verify: }} always passes and every +block message names it. There is always a legal move that ends the turn. """ import json import os @@ -174,18 +184,14 @@ def main(): if data.get("agent_id"): return - if data.get("stop_hook_active"): - # This block already fired once this turn and the agent has rewritten. - # Let the rewrite through: a second block starts a shave-a-few-words - # loop (observed: nine consecutive blocks on one 150-word message). - return + retry = bool(data.get("stop_hook_active")) message = data.get("last_assistant_message") or "" plain_words_note = try_check_reply(data) word_count = counted_words(message) - over_limit = word_count > WORD_LIMIT + over_limit = word_count > WORD_LIMIT and not retry claim = find_unverified_claim(message) marker_problems = find_marker_problems(message) diff --git a/engine/hooks/diu-stop/tests/test_fix_matrix.py b/engine/hooks/diu-stop/tests/test_fix_matrix.py index 0dd1761..2ebd6c0 100644 --- a/engine/hooks/diu-stop/tests/test_fix_matrix.py +++ b/engine/hooks/diu-stop/tests/test_fix_matrix.py @@ -55,6 +55,10 @@ "I think the deploy happened around 2am, so that's why the build is stale. " "{{CAT-UNVERIFIED: the 2am deploy}}" ), + "resolved": ( + "I think the deploy happened around 2am, so that's why the build is stale. " + "{{CAT-UNVERIFIED: the 2am deploy -- cannot verify: the deploy log is rotated out}}" + ), "reason": ( "A tag that names no blocker does not excuse its paragraph, so the " "hedge check still fires and the malformed-tag check fires beside " @@ -87,11 +91,14 @@ def test_fixed_fixtures_are_not_blocked_by_any_check(self): ) -class TestKnownDoubleBlocksResolveOnRetry(unittest.TestCase): +class TestKnownDoubleBlocksResolveByNamingTheBlocker(unittest.TestCase): """Some fixes deliberately still trip a second check (see - KNOWN_DOUBLE_BLOCKS). That's fine as long as the shared - `stop_hook_active` escape still releases it on the very next attempt -- - otherwise it's a real two-hook cycle, not a documented double-block.""" + KNOWN_DOUBLE_BLOCKS). `stop_hook_active` no longer releases those -- it + only stops the word-count check, so a rewrite cannot smuggle a new + unchecked claim through on the strength of the first block. What has to + exist instead is a move that ends the turn: naming the blocker in the + tag. Without one of these passing, the evidence checks would be an + unbounded loop.""" def test_hedged_message_still_blocks_once(self): for case in KNOWN_DOUBLE_BLOCKS: @@ -100,15 +107,25 @@ def test_hedged_message_still_blocks_once(self): self.assertTrue(blocked, f"{case['name']}: expected the documented double-block to fire") self.assertIn("CAT-UNVERIFIED", err) - def test_hedged_message_passes_on_stop_hook_active_retry(self): + def test_hedged_message_still_blocks_on_retry(self): for case in KNOWN_DOUBLE_BLOCKS: with self.subTest(case=case["name"]): - blocked, err = run_claude_check({ + blocked, _ = run_claude_check({ "last_assistant_message": case["hedged"], "stop_hook_active": True, }) - self.assertFalse(blocked, f"{case['name']}: retry did not escape via stop_hook_active") - self.assertEqual(err, "") + self.assertTrue(blocked, f"{case['name']}: retry must not excuse an unchecked claim") + + def test_naming_the_blocker_ends_the_turn(self): + for case in KNOWN_DOUBLE_BLOCKS: + with self.subTest(case=case["name"]): + for retry in (False, True): + blocked, err = run_claude_check({ + "last_assistant_message": case["resolved"], + "stop_hook_active": retry, + }) + self.assertFalse(blocked, f"{case['name']}: no legal move ends the turn: {err}") + self.assertEqual(err, "") if __name__ == "__main__": diff --git a/engine/hooks/diu-stop/tests/test_hooks.py b/engine/hooks/diu-stop/tests/test_hooks.py index d18ae41..3307275 100644 --- a/engine/hooks/diu-stop/tests/test_hooks.py +++ b/engine/hooks/diu-stop/tests/test_hooks.py @@ -258,6 +258,52 @@ 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_retry_still_checks_a_new_claim(self): + message = ( + "Correction on scope: that only covers the root chain. " + "UNVERIFIED: whether any other CI job picks up the test by glob." + ) + blocked, err = run_claude_check({ + "last_assistant_message": message, + "stop_hook_active": True, + }) + self.assertTrue(blocked) + self.assertIn("no longer an escape hatch", err) + + def test_retry_stops_checking_the_word_count(self): + message = " ".join(["word"] * (claude_stop_check.WORD_LIMIT + 40)) + blocked, err = run_claude_check({ + "last_assistant_message": message, + "stop_hook_active": True, + }) + self.assertFalse(blocked) + self.assertEqual(err, "") + + def test_retry_reports_only_the_claim_when_also_over_the_limit(self): + message = ( + "The owner crashed because the lock never released. " + + " ".join(["word"] * (claude_stop_check.WORD_LIMIT + 40)) + ) + blocked, err = run_claude_check({ + "last_assistant_message": message, + "stop_hook_active": True, + }) + self.assertTrue(blocked) + self.assertIn("unverified-shaped claim", err.lower()) + self.assertNotIn("over the", err) + + def test_retry_releases_once_the_claim_carries_a_well_formed_tag(self): + message = ( + "The owner crashed because the lock never released. " + "{{CAT-UNVERIFIED: the lock never released -- cannot verify: the host is powered down}}" + ) + blocked, err = run_claude_check({ + "last_assistant_message": message, + "stop_hook_active": True, + }) + self.assertFalse(blocked) + self.assertEqual(err, "") + def test_tag_excuses_only_its_own_paragraph(self): message = ( "{{CAT-UNVERIFIED: the runner is green -- cannot verify: CI is unreachable}}\n\n" diff --git a/engine/hooks/prove-it-ship-gate/detect.py b/engine/hooks/prove-it-ship-gate/detect.py index 42a674d..45fc0f3 100644 --- a/engine/hooks/prove-it-ship-gate/detect.py +++ b/engine/hooks/prove-it-ship-gate/detect.py @@ -165,8 +165,6 @@ def bash_commands_this_turn(transcript_path: str) -> list[str] | None: def decide(payload: dict) -> str | None: """Return blocking feedback, or None to let the turn finish.""" - if payload.get("stop_hook_active"): - return None message = payload.get("last_assistant_message") or "" if not claims_live_ship(message): return None 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 0666bea..587c648 100644 --- a/engine/hooks/prove-it-ship-gate/tests/test_hooks.py +++ b/engine/hooks/prove-it-ship-gate/tests/test_hooks.py @@ -144,11 +144,20 @@ def test_silent_when_a_live_command_ran_this_turn(self): finally: os.unlink(path) - def test_silent_when_stop_hook_active(self): - self.assertIsNone(detect.decide({ + def test_retry_still_fires_on_an_unproven_ship_claim(self): + self.assertIsNotNone(detect.decide({ "last_assistant_message": REAL_FIRE[0], "stop_hook_active": True, })) + def test_naming_the_blocker_ends_the_turn_on_retry(self): + tagged = REAL_FIRE[0] + ( + " {{CAT-UNVERIFIED: the live path -- cannot verify: the deploy host is unreachable}}" + ) + for retry in (False, True): + self.assertIsNone(detect.decide({ + "last_assistant_message": tagged, "stop_hook_active": retry, + })) + def test_fails_open_on_unreadable_transcript(self): self.assertIsNone(detect.decide({ "last_assistant_message": REAL_FIRE[0], "transcript_path": "/nonexistent/x.jsonl",