From a2817d7b4dbf26801d7c90a48477dd508d2fd499 Mon Sep 17 00:00:00 2001 From: Hex Sullivan Date: Tue, 5 May 2026 11:16:30 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20trailing=20boundary=20rege?= =?UTF-8?q?x=20killing=20space-terminated=20patterns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The completion regex applied `([^a-zA-Z]|$)` uniformly to all alternatives. For space-ending patterns like `implemented `, `created the `, `wrote the `, `added the `, `saved the `, the boundary required a non-letter after the trailing space — which in English is almost always a letter (the next word). These five patterns were effectively dead code. Fix: split into two grep calls — one for patterns needing a trailing boundary (e.g., "fixed it" vs "fixed itself") and one for space-terminated patterns where the space IS the boundary. Adds test case confirming "Implemented the fix in src/x.ts" now correctly triggers the advisory. Addresses review feedback from PR #54 (Claude review #2, Cursor). Co-Authored-By: Claude Opus 4.6 --- plugins/core/hooks/tests/test-verify-deliverables.sh | 7 +++++++ plugins/core/hooks/verify-deliverables.sh | 10 ++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/plugins/core/hooks/tests/test-verify-deliverables.sh b/plugins/core/hooks/tests/test-verify-deliverables.sh index 01863d0..f17e446 100755 --- a/plugins/core/hooks/tests/test-verify-deliverables.sh +++ b/plugins/core/hooks/tests/test-verify-deliverables.sh @@ -155,6 +155,12 @@ TX_NEGATION=$(write_transcript "negated" \ '{"type":"assistant","message":{"content":[{"type":"text","text":"Investigated src/foo.ts — not all tests pass yet."}]}}' ) +# Space-terminated pattern: "Implemented the fix in src/x.ts" should trigger. +TX_IMPLEMENTED=$(write_transcript "implemented-space" \ + '{"type":"assistant","message":{"content":[{"type":"tool_use","name":"Read","input":{"file_path":"src/x.ts"}}]}}' \ + '{"type":"assistant","message":{"content":[{"type":"text","text":"Implemented the fix in src/x.ts to resolve the issue."}]}}' +) + # Trailing boundary: "fixed itself" should not match "fixed it". TX_FIXED_ITSELF=$(write_transcript "fixed-itself" \ '{"type":"assistant","message":{"content":[{"type":"tool_use","name":"Read","input":{"file_path":"src/x.ts"}}]}}' \ @@ -197,6 +203,7 @@ run_case "alt payload shape (.tool_input.file_path)" "$(make_payload "$TX_ALT_SH run_case "pure analyst (no claim, no mutations)" "$(make_payload "$TX_ANALYST")" silent run_case "trivial chatter (claim word but no file token)" "$(make_payload "$TX_TRIVIAL")" silent run_case "negated 'not all tests pass' (no flag)" "$(make_payload "$TX_NEGATION")" silent +run_case "space-terminated 'implemented' triggers advisory" "$(make_payload "$TX_IMPLEMENTED")" "made no file changes" run_case "'fixed itself' does not match 'fixed it'" "$(make_payload "$TX_FIXED_ITSELF")" silent run_case "broken symlink counts as present" "$(make_payload "$TX_BROKEN_SYMLINK")" silent run_case "long transcript (Write at line 1, msg after 60 Reads)" "$(make_payload "$TX_LONG")" silent diff --git a/plugins/core/hooks/verify-deliverables.sh b/plugins/core/hooks/verify-deliverables.sh index 0e31257..66f3f50 100755 --- a/plugins/core/hooks/verify-deliverables.sh +++ b/plugins/core/hooks/verify-deliverables.sh @@ -84,10 +84,12 @@ ISSUES="" if [ -n "$LAST_MSG" ] && [ "$MUTATING_CALLS" = "0" ]; then # Conservative completion patterns. False positives are noisier than - # false negatives, so we only fire on strong signals. Trailing - # boundary keeps "fixed it" from matching inside "fixed itself" - # and "all tests pass" from matching inside "passenger". - if echo "$LAST_MSG" | grep -qiE '(^|[^a-z])(done\.|complete\.|completed\.|finished\.|implemented |fixed it|created the |wrote the |added the |saved the |all tests pass(ed)?)([^a-zA-Z]|$)'; then + # false negatives, so we only fire on strong signals. + # Two groups: (1) patterns needing a trailing boundary to avoid + # substring matches ("fixed it" vs "fixed itself"), and + # (2) space-terminated patterns where the space IS the boundary. + if echo "$LAST_MSG" | grep -qiE '(^|[^a-z])(done\.|complete\.|completed\.|finished\.|fixed it|all tests pass(ed)?)([^a-zA-Z]|$)' || + echo "$LAST_MSG" | grep -qiE '(^|[^a-z])(implemented |created the |wrote the |added the |saved the )'; then # Reject negated phrasing — "not all tests pass" should not trigger. if ! echo "$LAST_MSG" | grep -qiE "(not|n't|cannot|couldn't|didn't|haven't|hasn't|wasn't)([[:space:]]+(yet|fully|quite|all|even))?[[:space:]]+(done|complete|completed|finished|implemented|fixed|added|wrote|created|saved|all tests pass)"; then # Require a file-path-shaped token. Pure analysis output