feat(analyzer): opt-in TRUST_INTURN_RECOVERY flag for hook-retry denials - #348
Open
nitrocode wants to merge 1 commit into
Open
feat(analyzer): opt-in TRUST_INTURN_RECOVERY flag for hook-retry denials#348nitrocode wants to merge 1 commit into
nitrocode wants to merge 1 commit into
Conversation
A PreToolUse hook can deny a Write/Edit/NotebookEdit/MultiEdit tool call with a corrective message. Claude Code retries the tool call within the same turn and succeeds (is_error=false, stop_reason=end_turn), but the next loop halts anyway because the denial still appears in the previous transcript. Issue frankbria#243's exception only covers Bash compound-command denials with an already-permitted base; hook denials on file-mutation tools have no equivalent carve-out. Adds an opt-in TRUST_INTURN_RECOVERY flag (default false, matching the approach suggested in PR frankbria#264's closing comment) to .ralphrc. When true, and every denial in a completed turn is for a file-mutation tool AND the turn ended cleanly, should_exit_gracefully logs an advisory and continues instead of halting. Default behavior is unchanged. is_error/stop_reason are outcome signals, not authorization signals like Issue frankbria#243's ALLOWED_TOOLS-coverage check, so this stays opt-in and off by default: a clean turn end does not prove the specific denied write was resolved, only that the turn did not crash afterward. Refs: frankbria#264
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.
Problem
should_exit_gracefullyhalts the next loop when the previous transcript'spermission_denialsarray is non-empty. This is a false positive when a PreToolUse hook denies a Write/Edit tool call with a corrective message and Claude retries the tool call successfully within the same turn — the turn ends withis_error: falseandstop_reason: end_turn, work completes, but the next loop halts onpermission_deniedanyway. Issue #243's exception only covers Bash compound-command denials; hook denials on file-mutation tools have no equivalent carve-out.Repro
Write/Editwhen the input matches some rule (e.g. a style hook that rewritestool_inputand returns a correctivedenydecision).ralphon a task that produces a matching Write/Edit.is_error: false,stop_reason: end_turn).should_exit_gracefullyhalts withPermission denied for N command(s): Write, Edit ... Update ALLOWED_TOOLS in .ralphrc, even though nothing is actually blocked and the previous loop's work already landed.Observed 3 times in one session on this exact pattern.
Fix (opt-in, off by default)
Following the guidance in #264's closing comment — that a broader denial-recovery fix "would be welcome" but "gated behind a config flag, so the default keeps the strict #101 halt" — this PR adds an opt-in
TRUST_INTURN_RECOVERYflag (defaultfalse) to.ralphrc/templates/ralphrc.template. When true, and every denial in a completed turn is forWrite/Edit/NotebookEdit/MultiEditAND the CLI reportsis_error: false+stop_reason: end_turn,should_exit_gracefullylogs an advisory and continues instead of halting. Structurally parallel to the Issue #243 compound-command exception (#268).Default behavior (flag unset or
false) is unchanged. All 1007 pre-existing unit tests pass (2 unrelatedtest_sandbox_e2b.batsfailures reproduce on a clean checkout ofmaintoo — local Python environment issue on my machine, not caused by this change).Residual limitation (documented in the ralphrc template with a
WARNING:comment, matching the existingCB_AUTO_RESETconvention)is_error: falseandstop_reason: end_turnare outcome signals ("the turn didn't crash after the denial") not authorization signals ("the specific denied call was resolved"). A hook that denies a Write to a sensitive path (e.g..ralphrcitself) which the agent then abandons and moves past would also produce a clean turn end. EnablingTRUST_INTURN_RECOVERYweakens the Issue #101 silent-loop guard for this class of case, which is why it defaults tofalseand requires explicit opt-in.Stronger signals (walking assistant messages for a subsequent successful
tool_useof the same tool with matching input) were considered and left out to keep this PR minimally scoped, consistent with #268's surgical style.Verification
tests/unit/test_hook_retry_recovery.bats(new, 9 tests): flag gating (unset/false/true), coverage of all four file-mutation tools,is_error: truestill halts, Bash denials stay out of scope, mixed denials still halt, and two end-to-end tests that exercise the fullanalyze_responsepath (not justparse_json_response) to confirm the new fields actually propagate into the.response_analysisfile thatshould_exit_gracefullyreads from.analysis.*—parse_json_response's result is re-extracted and re-packaged insideanalyze_response, so a wiring gap there would otherwise pass the parse-level tests while still no-op'ing in production.tests/unit/test_compound_command_detection.bats(existing, 27 tests): all pass unchanged, confirming the Issue Permission denied but already configured in ALLOWED_TOOLS #243 path is untouched.tests/unit/*.batssuite: 1007/1009 pass; the 2 failures are pre-existing and unrelated (see above).References