fix(gate): stop treating stdout text as failure; cap finish-the-work blocks - #20
Open
trac3r00 wants to merge 1 commit into
Open
fix(gate): stop treating stdout text as failure; cap finish-the-work blocks#20trac3r00 wants to merge 1 commit into
trac3r00 wants to merge 1 commit into
Conversation
…blocks Two independent defects in the observation gate. 1. detect_failure() matched FAILURE_RE against the whole tool response, stdout included, whenever no structured exit signal was present. Command OUTPUT lands in stdout, so reading or grepping any source that merely discusses errors was reported as encountering one -- it fired 6 times in a single session from cat/grep alone, and polluted ledger["failures"], which drives repeated_failure(). Split the heuristic: structured_exit() reads only success/ok booleans and exit codes, and error_stream_text() scans only stderr/error fields. exit_success() and verification_record() are deliberately unchanged -- there the command already matched VERIFY_RE, so its stdout is a verdict. 2. finish-the-work.sh had no cumulative cap. stop_hook_active only alternates block/allow, so a phrasing that keeps tripping the promise regex blocked on every other Stop indefinitely -- confirmed 5/5 consecutive stops. Its siblings both self-limit (gate_stop MAX_STOP_BLOCKS=2, context-guard MAX_BLOCKS=2). Add MAX_FINISH_BLOCKS=2, counted per session in a finish-blocks/ sidecar keyed exactly like ledger_key (sha256(session_id|cwd)[:24]) and honoring FABLIZE_DATA. Kept out of the ledger itself so it cannot corrupt gate_stop state; counter writes fail open. tests/test_gate_stream_and_cap.py covers both (10 checks). Existing suites test_gate.py, test_gate_robustness.py and test_recovery.py all still pass. Refs fivetaku#19 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TCmGs7cjMDeZCYQzCgfHeB
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent defects in the observation gate, both found while QA'ing a fresh 2.1.1 install alongside other hook-heavy plugins. Fixes #19.
1.
detect_failure()treats stdout text as a failure signaldetect_failure()runs on every Bash/Edit/Write. When no structured exit signal is present it matchesFAILURE_RE(failed|failure|\berror:|traceback|...) againstresponse_text(), which joinsstdout, stderr, output, message, text, content, error, summary.Command output lands in stdout — so reading or grepping any source that discusses errors is classified as encountering one, and the gate emits:
This fired 6 times in one session purely from
cat/grepover plugin source — including once while writing a markdown document about the bug, because the document contains the word "failure". False failures also accumulate inledger["failures"], which drivesrepeated_failure(), so two unrelated file reads can normalize to the same signature and trigger the silent-recovery disclosure.Fix. Split the heuristic rather than weakening the regex:
structured_exit()— reads onlysuccess/okbooleans andexit_code/exitCode/returncode/status. Never guesses from text.error_stream_text()— collects onlystderr/errorfields, never stdout.detect_failure()— uses the structured signal; falls back to stderr-only matching when none exists.exit_success()andverification_record()are deliberately unchanged. The text heuristic is correct there: the command already matchedVERIFY_RE, so it is a known test/build/lint run and its stdout genuinely is a verdict.2.
finish-the-work.shhas no cumulative capstop_hook_activeonly alternates block/allow, so it prevents a tight loop but not an unbounded one. A phrasing that keeps tripping the promise regex blocks on every other Stop, indefinitely — measured at 5/5 consecutive stops before the fix.Both sibling gates already self-limit:
gate_stopatMAX_STOP_BLOCKS = 2,context-guard-stopatMAX_BLOCKS = 2. This one was the outlier, and the README acknowledges the regex can fire on declarative offers ("I'll add X if you want"), which is exactly the input that makes an uncapped blocker painful.Fix.
MAX_FINISH_BLOCKS = 2, counted per session in afinish-blocks/sidecar keyed exactly likeledger_key(sha256(session_id|cwd)[:24]) and honoringFABLIZE_DATA. Kept out of the ledger itself so it cannot corruptgate_stopstate. Counter writes fail open — a counter that can't be persisted must not break the hook.Tests
New
tests/test_gate_stream_and_cap.py(10 checks), following the existing standalone-script convention — exits non-zero on mismatch:Existing suites all still pass (
test_gate.py6/6 scenarios with S2/S3 still caught,test_gate_robustness.py12 checks,test_recovery.py).Note for anyone running these: they're standalone scripts, not pytest modules —
pytest tests/hits anINTERNALERRORbecausetest_gate_robustness.py:109callssys.exit()at import. Run them directly (python3 tests/test_gate.py). Happy to add a tiny runner script if that'd be useful.Not changed
FAILURE_REitself is untouched. Tightening the pattern would have been the smaller diff but the wrong fix — the problem isn't which words it matches, it's which stream it reads.