Skip to content

feat(analyzer): opt-in TRUST_INTURN_RECOVERY flag for hook-retry denials #348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nitrocode wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(analyzer): opt-in TRUST_INTURN_RECOVERY flag for hook-retry denials #348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions lib/response_analyzer.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,43 @@ parse_json_response() {
fi
fi

# Hook-retry recovery detection (opt-in via TRUST_INTURN_RECOVERY).
# PreToolUse hooks can deny a Write/Edit/NotebookEdit/MultiEdit tool call
# with a corrective message (e.g. a style-rule hook rewriting a Write's
# tool_input). Claude Code then retries the tool call within the SAME
# turn and succeeds; the overall turn ends with is_error=false and
# stop_reason=end_turn. Without this exception the next loop halts on
# the previous turn's denial array — a false positive.
# This exception is OPT-IN because is_error/stop_reason are outcome
# signals (turn ended cleanly) not authorization signals (the specific
# denial was resolved). A hook that denies a Write to a sensitive path
# which the agent then abandons would also produce is_error=false /
# stop_reason=end_turn, defeating the Issue #101 silent-loop guard.
# Users must explicitly opt in via TRUST_INTURN_RECOVERY=true in .ralphrc.
local has_hook_recovered_denials="false"
local hook_recovered_denial_count=0
if [[ $permission_denial_count -gt 0 && "$has_compound_command_limitation" == "false" && "${TRUST_INTURN_RECOVERY:-false}" == "true" ]]; then
local turn_is_error
turn_is_error=$(jq -r '.is_error // false' "$output_file" 2>/dev/null)
local turn_stop_reason
turn_stop_reason=$(jq -r '.stop_reason // ""' "$output_file" 2>/dev/null)

if [[ "$turn_is_error" == "false" && ( "$turn_stop_reason" == "end_turn" || -z "$turn_stop_reason" ) ]]; then
# Count file-mutation-tool denials. Anything else is a real gap.
local mutation_denial_count
mutation_denial_count=$(jq -r '[.permission_denials[] | select(.tool_name == "Write" or .tool_name == "Edit" or .tool_name == "NotebookEdit" or .tool_name == "MultiEdit")] | length' "$output_file" 2>/dev/null || echo "0")
mutation_denial_count=$((mutation_denial_count + 0))

if [[ $mutation_denial_count -gt 0 && $mutation_denial_count -eq $permission_denial_count ]]; then
has_hook_recovered_denials="true"
hook_recovered_denial_count=$mutation_denial_count
# Downgrade has_permission_denials — user opted into
# trusting in-turn recovery on file-mutation tools.
has_permission_denials="false"
fi
fi
fi

# Normalize values
# Convert exit_signal to boolean string
# Only infer from status/completion_status if no explicit EXIT_SIGNAL was provided
Expand Down Expand Up @@ -463,6 +500,8 @@ parse_json_response() {
--argjson denied_commands "$denied_commands_json" \
--argjson has_compound_command_limitation "$has_compound_command_limitation" \
--argjson compound_command_count "$compound_command_count" \
--argjson has_hook_recovered_denials "$has_hook_recovered_denials" \
--argjson hook_recovered_denial_count "$hook_recovered_denial_count" \
'{
status: $status,
exit_signal: $exit_signal,
Expand All @@ -480,6 +519,8 @@ parse_json_response() {
denied_commands: $denied_commands,
has_compound_command_limitation: $has_compound_command_limitation,
compound_command_count: $compound_command_count,
has_hook_recovered_denials: $has_hook_recovered_denials,
hook_recovered_denial_count: $hook_recovered_denial_count,
metadata: {
loop_number: $loop_number,
session_id: $session_id
Expand Down Expand Up @@ -542,6 +583,9 @@ analyze_response() {
# Compound-command limitation flag (Issue #243)
local has_compound_command_limitation=$(jq -r '.has_compound_command_limitation // false' $RALPH_DIR/.json_parse_result 2>/dev/null || echo "false")
local compound_command_count=$(jq -r '.compound_command_count // 0' $RALPH_DIR/.json_parse_result 2>/dev/null || echo "0")
# Hook-retry recovery flag (opt-in, TRUST_INTURN_RECOVERY)
local has_hook_recovered_denials=$(jq -r '.has_hook_recovered_denials // false' $RALPH_DIR/.json_parse_result 2>/dev/null || echo "false")
local hook_recovered_denial_count=$(jq -r '.hook_recovered_denial_count // 0' $RALPH_DIR/.json_parse_result 2>/dev/null || echo "0")

# Persist session ID if present (for session continuity across loop iterations)
if [[ -n "$session_id" && "$session_id" != "null" ]]; then
Expand Down Expand Up @@ -621,6 +665,8 @@ analyze_response() {
--argjson denied_commands "$denied_commands_json" \
--argjson has_compound_command_limitation "$has_compound_command_limitation" \
--argjson compound_command_count "$compound_command_count" \
--argjson has_hook_recovered_denials "$has_hook_recovered_denials" \
--argjson hook_recovered_denial_count "$hook_recovered_denial_count" \
--argjson asking_questions "$asking_questions" \
--argjson question_count "$question_count" \
'{
Expand All @@ -643,6 +689,8 @@ analyze_response() {
denied_commands: $denied_commands,
has_compound_command_limitation: $has_compound_command_limitation,
compound_command_count: $compound_command_count,
has_hook_recovered_denials: $has_hook_recovered_denials,
hook_recovered_denial_count: $hook_recovered_denial_count,
asking_questions: $asking_questions,
question_count: $question_count
}
Expand Down
11 changes: 11 additions & 0 deletions ralph_loop.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,16 +931,27 @@ should_exit_gracefully() {
# Exception (Issue #243): if every denial is a Bash compound command whose
# base is already covered by ALLOWED_TOOLS, the denial is a Claude CLI
# limitation rather than a real permission gap — log an advisory and continue.
# Exception (opt-in, TRUST_INTURN_RECOVERY): if every denial is a file-
# mutation tool call (Write/Edit/NotebookEdit/MultiEdit) AND the turn ended
# cleanly, treat as advisory. Off by default because this is an outcome
# signal not an authorization signal — see WARNING in the ralphrc template.
if [[ -f "$RESPONSE_ANALYSIS_FILE" ]]; then
local has_permission_denials=$(jq -r '.analysis.has_permission_denials // false' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "false")
local has_compound_limitation=$(jq -r '.analysis.has_compound_command_limitation // false' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "false")
local has_hook_recovered=$(jq -r '.analysis.has_hook_recovered_denials // false' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "false")

if [[ "$has_compound_limitation" == "true" ]]; then
local compound_count=$(jq -r '.analysis.compound_command_count // 0' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "0")
local denied_cmds=$(jq -r '.analysis.denied_commands | join(", ")' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "unknown")
log_status "WARN" "⚠️ Claude CLI denied $compound_count compound command(s) but the base command is already in ALLOWED_TOOLS: $denied_cmds"
log_status "WARN" "This is a Claude CLI matching limitation (pipes/redirects bypass Bash(cmd *) patterns). Loop continues."
# Fall through — do not halt
elif [[ "$has_hook_recovered" == "true" ]]; then
local recovered_count=$(jq -r '.analysis.hook_recovered_denial_count // 0' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "0")
local denied_cmds=$(jq -r '.analysis.denied_commands | join(", ")' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "unknown")
log_status "WARN" "⚠️ $recovered_count file-mutation tool call(s) denied by a PreToolUse hook; turn ended cleanly (TRUST_INTURN_RECOVERY=true): $denied_cmds"
log_status "WARN" "Loop continues. Note: this is an outcome signal, not proof the specific denial was resolved — see .ralphrc TRUST_INTURN_RECOVERY warning."
# Fall through — do not halt
elif [[ "$has_permission_denials" == "true" ]]; then
local denied_count=$(jq -r '.analysis.permission_denial_count // 0' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "0")
local denied_cmds=$(jq -r '.analysis.denied_commands | join(", ")' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "unknown")
Expand Down
20 changes: 20 additions & 0 deletions templates/ralphrc.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,26 @@ CB_COOLDOWN_MINUTES=30
# WARNING: Reduces circuit breaker safety for unattended operation
CB_AUTO_RESET=false

# =============================================================================
# PERMISSION-DENIAL HANDLING
# =============================================================================

# Trust in-turn hook-retry recovery for file-mutation tool denials.
# When true, and every permission denial in a completed turn is for a
# Write/Edit/NotebookEdit/MultiEdit tool AND the turn ended cleanly
# (is_error=false, stop_reason=end_turn), the next loop continues with
# an advisory instead of halting. Targets the PreToolUse-hook-denies-
# then-Claude-retries-in-turn pattern (e.g. a style-rule hook that
# rewrites the tool_input and denies with a corrective message).
#
# WARNING: This weakens the Issue #101 silent-loop protection. A clean
# turn end does not prove the specific denied write was resolved — the
# agent may have abandoned the write (e.g. against a protected path
# like .ralphrc) and moved on to unrelated work. Leave false unless
# your hook configuration is known-safe and you have observed the
# false-positive halt described in the linked issue.
TRUST_INTURN_RECOVERY=false

# =============================================================================
# ADVANCED SETTINGS
# =============================================================================
Expand Down
210 changes: 210 additions & 0 deletions tests/unit/test_hook_retry_recovery.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
#!/usr/bin/env bats
# Unit tests for opt-in hook-retry-recovery permission-denial detection
# (lib/response_analyzer.sh, TRUST_INTURN_RECOVERY).
#
# Covers a false positive where a PreToolUse hook denies a Write/Edit tool
# call with a corrective message, Claude retries and succeeds within the
# same turn (is_error=false, stop_reason=end_turn), but the next loop still
# halted because the denial appeared somewhere in the transcript. Off by
# default: enabling TRUST_INTURN_RECOVERY trades some of the Issue #101
# silent-loop protection for fewer false-positive halts on this pattern.

load '../helpers/test_helper'

RESPONSE_ANALYZER="${BATS_TEST_DIRNAME}/../../lib/response_analyzer.sh"

setup() {
TEST_DIR="$(mktemp -d)"
cd "$TEST_DIR"
source "$RESPONSE_ANALYZER"
unset TRUST_INTURN_RECOVERY
}

teardown() {
cd /
[[ -n "$TEST_DIR" && -d "$TEST_DIR" ]] && rm -rf "$TEST_DIR"
}

# -----------------------------------------------------------------------------
# parse_json_response: flag gating
# -----------------------------------------------------------------------------

_write_clean_turn_denial() {
local file="$1"
local tool_name="$2"
cat > "$file" <<EOF
{
"status": "IN_PROGRESS",
"session_id": "test-session",
"is_error": false,
"stop_reason": "end_turn",
"permission_denials": [
{"tool_name": "$tool_name", "tool_input": {"file_path": "/tmp/x"}}
]
}
EOF
}

@test "parse_json_response: flag unset (default) keeps halt on Write denial with clean turn" {
local output_file="$TEST_DIR/output.json"
local result_file="$TEST_DIR/result.json"
_write_clean_turn_denial "$output_file" "Write"

unset TRUST_INTURN_RECOVERY
parse_json_response "$output_file" "$result_file"

assert_equal "$(jq -r '.has_hook_recovered_denials' "$result_file")" "false"
assert_equal "$(jq -r '.has_permission_denials' "$result_file")" "true"
}

@test "parse_json_response: flag explicitly false keeps halt on Write denial with clean turn" {
local output_file="$TEST_DIR/output.json"
local result_file="$TEST_DIR/result.json"
_write_clean_turn_denial "$output_file" "Write"

export TRUST_INTURN_RECOVERY="false"
parse_json_response "$output_file" "$result_file"

assert_equal "$(jq -r '.has_hook_recovered_denials' "$result_file")" "false"
assert_equal "$(jq -r '.has_permission_denials' "$result_file")" "true"
}

@test "parse_json_response: flag ON downgrades Write denial with clean turn to advisory" {
local output_file="$TEST_DIR/output.json"
local result_file="$TEST_DIR/result.json"
_write_clean_turn_denial "$output_file" "Write"

export TRUST_INTURN_RECOVERY="true"
parse_json_response "$output_file" "$result_file"

assert_equal "$(jq -r '.has_hook_recovered_denials' "$result_file")" "true"
assert_equal "$(jq -r '.has_permission_denials' "$result_file")" "false"
assert_equal "$(jq -r '.hook_recovered_denial_count' "$result_file")" "1"
}

@test "parse_json_response: flag ON covers Edit, NotebookEdit, MultiEdit the same as Write" {
for tool in Edit NotebookEdit MultiEdit; do
local output_file="$TEST_DIR/output_$tool.json"
local result_file="$TEST_DIR/result_$tool.json"
_write_clean_turn_denial "$output_file" "$tool"

export TRUST_INTURN_RECOVERY="true"
parse_json_response "$output_file" "$result_file"

assert_equal "$(jq -r '.has_hook_recovered_denials' "$result_file")" "true"
done
}

# -----------------------------------------------------------------------------
# parse_json_response: flag ON, but the turn did NOT end cleanly (real denial)
# -----------------------------------------------------------------------------

@test "parse_json_response: flag ON keeps halt when is_error is true (turn failed)" {
local output_file="$TEST_DIR/output.json"
local result_file="$TEST_DIR/result.json"
cat > "$output_file" <<'EOF'
{
"status": "IN_PROGRESS",
"session_id": "test-session",
"is_error": true,
"stop_reason": "end_turn",
"permission_denials": [
{"tool_name": "Write", "tool_input": {"file_path": "/tmp/x"}}
]
}
EOF
export TRUST_INTURN_RECOVERY="true"
parse_json_response "$output_file" "$result_file"

assert_equal "$(jq -r '.has_hook_recovered_denials' "$result_file")" "false"
assert_equal "$(jq -r '.has_permission_denials' "$result_file")" "true"
}

# -----------------------------------------------------------------------------
# parse_json_response: flag ON, but the denial is out of scope for this flag
# -----------------------------------------------------------------------------

@test "parse_json_response: flag ON does not cover Bash denials (Issue #243's own path handles those)" {
local output_file="$TEST_DIR/output.json"
local result_file="$TEST_DIR/result.json"
cat > "$output_file" <<'EOF'
{
"status": "IN_PROGRESS",
"session_id": "test-session",
"is_error": false,
"stop_reason": "end_turn",
"permission_denials": [
{"tool_name": "Bash", "tool_input": {"command": "rm -rf /tmp/x"}}
]
}
EOF
export TRUST_INTURN_RECOVERY="true"
export CLAUDE_ALLOWED_TOOLS="Write,Read"
parse_json_response "$output_file" "$result_file"

assert_equal "$(jq -r '.has_hook_recovered_denials' "$result_file")" "false"
assert_equal "$(jq -r '.has_permission_denials' "$result_file")" "true"
}

@test "parse_json_response: flag ON keeps halt on mixed Write + AskUserQuestion denials (real gap)" {
local output_file="$TEST_DIR/output.json"
local result_file="$TEST_DIR/result.json"
cat > "$output_file" <<'EOF'
{
"status": "IN_PROGRESS",
"session_id": "test-session",
"is_error": false,
"stop_reason": "end_turn",
"permission_denials": [
{"tool_name": "Write", "tool_input": {"file_path": "/tmp/x"}},
{"tool_name": "AskUserQuestion", "tool_input": {}}
]
}
EOF
export TRUST_INTURN_RECOVERY="true"
parse_json_response "$output_file" "$result_file"

assert_equal "$(jq -r '.has_hook_recovered_denials' "$result_file")" "false"
assert_equal "$(jq -r '.has_permission_denials' "$result_file")" "true"
}

# -----------------------------------------------------------------------------
# analyze_response: end-to-end wiring through to .response_analysis
# -----------------------------------------------------------------------------
#
# parse_json_response is called BY analyze_response and its result is
# re-extracted and re-packaged into the final RESPONSE_ANALYSIS_FILE that
# ralph_loop.sh's should_exit_gracefully() actually reads (under .analysis.*).
# These tests exercise that full path so a wiring gap between the two
# jq-construction sites (parse_json_response's own output vs. the
# re-packaged analysis: {...} block inside analyze_response) would fail
# here even if the parse_json_response-only tests above pass.

@test "analyze_response: flag ON propagates has_hook_recovered_denials into .analysis" {
export RALPH_DIR="$TEST_DIR/.ralph"
mkdir -p "$RALPH_DIR"

local output_file="$TEST_DIR/claude_output.json"
_write_clean_turn_denial "$output_file" "Write"

export TRUST_INTURN_RECOVERY="true"
analyze_response "$output_file" 1 "$RALPH_DIR/.response_analysis"

assert_equal "$(jq -r '.analysis.has_hook_recovered_denials' "$RALPH_DIR/.response_analysis")" "true"
assert_equal "$(jq -r '.analysis.has_permission_denials' "$RALPH_DIR/.response_analysis")" "false"
assert_equal "$(jq -r '.analysis.hook_recovered_denial_count' "$RALPH_DIR/.response_analysis")" "1"
}

@test "analyze_response: flag unset (default) propagates halt-preserving false into .analysis" {
export RALPH_DIR="$TEST_DIR/.ralph"
mkdir -p "$RALPH_DIR"

local output_file="$TEST_DIR/claude_output.json"
_write_clean_turn_denial "$output_file" "Write"

unset TRUST_INTURN_RECOVERY
analyze_response "$output_file" 1 "$RALPH_DIR/.response_analysis"

assert_equal "$(jq -r '.analysis.has_hook_recovered_denials' "$RALPH_DIR/.response_analysis")" "false"
assert_equal "$(jq -r '.analysis.has_permission_denials' "$RALPH_DIR/.response_analysis")" "true"
}