Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 26 additions & 2 deletions lib/response_analyzer.sh
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,25 @@ parse_json_response() {

# Bug #1 Fix: If exit_signal is still false, check for RALPH_STATUS block in .result field
# Claude CLI JSON format embeds the RALPH_STATUS block within the .result text field
#
# Supported emit formats (both must be recognized; do not regress to single-format match):
# 1) Canonical separator-marker format (lib/enable_core.sh:560-568):
# ---RALPH_STATUS---
# EXIT_SIGNAL: true
# ---END_RALPH_STATUS---
# 2) YAML colon-block format emitted by some agent prompts:
# RALPH_STATUS:
# EXIT_SIGNAL: true
# reason: "..."
# The regex anchors at start-of-line (with optional leading whitespace) so prose
# mentions of "RALPH_STATUS:" inline (e.g. in commit messages, doc strings, or
# tool-result echoes) DO NOT trigger the gate; only block-header lines do.
# The downstream EXIT_SIGNAL extraction (grep "EXIT_SIGNAL:" | cut | xargs) handles
# both layouts uniformly because both emit one EXIT_SIGNAL: <bool> line.
# See also the parallel check at the structured-output text-parsing fallback below.
if [[ "$exit_signal" == "false" && "$has_result_field" == "true" ]]; then
local result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

SC2155: Declare and assign result_text separately.

When local and command substitution are combined, the local builtin's exit code (always 0) masks the subcommand's exit code. Shellcheck flags this on line 176.

🛠 Proposed fix
-    local result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null)
+    local result_text
+    result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null)
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 176-176: Declare and assign separately to avoid masking return values.

(SC2155)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/response_analyzer.sh` at line 176, The issue is that combining "local"
with command substitution masks the command's exit code for result_text; change
the code in the response parsing logic so you first declare the variable (local
result_text) and then assign it with the jq command substitution
(result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null)), and after
assignment check the jq exit status ($?) or handle empty/error cases; reference
the variable result_text and input variable output_file and ensure any failure
from jq is propagated or logged instead of being hidden.

if [[ -n "$result_text" ]] && echo "$result_text" | grep -q -- "---RALPH_STATUS---"; then
if [[ -n "$result_text" ]] && echo "$result_text" | grep -qE -- "^[[:space:]]*(---RALPH_STATUS---|RALPH_STATUS:)"; then
# Extract EXIT_SIGNAL value from RALPH_STATUS block within result text
local embedded_exit_sig
embedded_exit_sig=$(echo "$result_text" | grep "EXIT_SIGNAL:" | cut -d: -f2 | xargs)
Expand Down Expand Up @@ -486,7 +502,15 @@ analyze_response() {
local explicit_exit_signal_found=false

# 1. Check for explicit structured output (if Claude follows schema)
if grep -q -- "---RALPH_STATUS---" "$output_file"; then
# Match both the canonical "---RALPH_STATUS---" separator-marker format AND
# the YAML colon-block format ("RALPH_STATUS:" with indented keys) some agent
# prompts emit. The regex anchors at start-of-line (with optional leading
# whitespace) so prose mentions of these tokens inline DO NOT trigger the
# gate; only block-header lines do. The downstream EXIT_SIGNAL/STATUS
# extraction handles both layouts uniformly because each emits one
# "EXIT_SIGNAL: <bool>" line.
# See also the parallel check in the JSON-mode .result text path above.
if grep -qE -- "^[[:space:]]*(---RALPH_STATUS---|RALPH_STATUS:)" "$output_file"; then
# Parse structured output
local status=$(grep "STATUS:" "$output_file" | cut -d: -f2 | xargs)
local exit_sig=$(grep "EXIT_SIGNAL:" "$output_file" | cut -d: -f2 | xargs)
Expand Down
142 changes: 142 additions & 0 deletions tests/unit/test_json_parsing.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1155,3 +1155,145 @@ EOF
exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "false"
}

# =============================================================================
# YAML COLON-BLOCK RALPH_STATUS FORMAT TESTS
# =============================================================================
# Some agent prompts emit the structured-status block as YAML (colon heading +
# indented keys) instead of the canonical "---RALPH_STATUS---" separator
# markers. The detection regex extends to match either form so EXIT_SIGNAL is
# read out of both layouts; the downstream `grep "EXIT_SIGNAL:" | cut | xargs`
# extraction is layout-agnostic.

@test "text mode: YAML colon-block RALPH_STATUS with EXIT_SIGNAL=true triggers exit" {
local output_file="$LOG_DIR/test_output.log"

cat > "$output_file" << 'EOF'
Investigation complete.

RALPH_STATUS:
EXIT_SIGNAL: true
reason: "All tasks complete"

End of response.
EOF

analyze_response "$output_file" 1

local exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "true"
}

@test "text mode: YAML colon-block RALPH_STATUS with EXIT_SIGNAL=false does not trigger exit" {
local output_file="$LOG_DIR/test_output.log"

cat > "$output_file" << 'EOF'
Made progress on the auth module.

RALPH_STATUS:
EXIT_SIGNAL: false
reason: "More work needed on the session layer"
EOF

analyze_response "$output_file" 1

local exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "false"
}

@test "JSON mode .result field with YAML colon-block RALPH_STATUS triggers exit" {
# Claude CLI JSON wrapper format: structured response with .result string
# containing a YAML colon-block RALPH_STATUS section. Mirrors the
# ".result + ---RALPH_STATUS---" path tested elsewhere; verifies the
# extended regex catches the YAML form too.
local output_file="$LOG_DIR/test_output.log"

cat > "$output_file" << 'EOF'
{
"result": "Implementation done.\n\nRALPH_STATUS:\n EXIT_SIGNAL: true\n reason: \"Project complete\"\n",
"sessionId": "test-session-001"
}
EOF

analyze_response "$output_file" 1

local exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "true"
}

@test "JSON mode .result field with YAML colon-block EXIT_SIGNAL=false respects continue intent" {
# YAML emit can also signal "continue working" — explicit EXIT_SIGNAL: false
# must be respected the same way the separator-marker form is.
local output_file="$LOG_DIR/test_output.log"

cat > "$output_file" << 'EOF'
{
"result": "Phase 1 done.\n\nRALPH_STATUS:\n EXIT_SIGNAL: false\n reason: \"Phase 2 still pending\"\n",
"sessionId": "test-session-002"
}
EOF

analyze_response "$output_file" 1

local exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "false"
}

@test "text mode: inline RALPH_STATUS: in prose does NOT trigger gate" {
# Regression: the regex must anchor to start-of-line so prose mentions of
# "RALPH_STATUS:" don't accidentally enter the EXIT_SIGNAL extraction block.
# Without anchoring, a sentence like "see the RALPH_STATUS: emit format
# documented above" would match the gate. With anchoring, only a block
# header at start-of-line matches.
local output_file="$LOG_DIR/test_output.log"

cat > "$output_file" << 'EOF'
Working on the documentation update.

I added a paragraph that mentions the RALPH_STATUS: emit format inline,
referencing the canonical layout. No actual block was emitted.

EXIT_SIGNAL: true should also not trigger by itself without a header.
EOF

analyze_response "$output_file" 1

local exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "false"
}

@test "JSON mode .result field with inline RALPH_STATUS: in prose does NOT trigger gate" {
# Regression: same line-anchor concern but in the JSON-mode .result path.
local output_file="$LOG_DIR/test_output.log"

cat > "$output_file" << 'EOF'
{
"result": "Updating the README. The RALPH_STATUS: section now describes both formats. EXIT_SIGNAL: true would be a block-emit not an inline reference, so no gate trigger here.",
"sessionId": "test-session-prose"
}
EOF

analyze_response "$output_file" 1

local exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "false"
}

@test "text mode: leading-whitespace RALPH_STATUS: header DOES trigger gate (indented YAML block)" {
# The anchor allows optional leading whitespace so indented blocks (e.g.,
# nested YAML or quoted in tool output) still match the gate.
local output_file="$LOG_DIR/test_output.log"

cat > "$output_file" << 'EOF'
Some context lines.

RALPH_STATUS:
EXIT_SIGNAL: true
reason: "Done"
EOF

analyze_response "$output_file" 1

local exit_signal=$(jq -r '.analysis.exit_signal' "$RALPH_DIR/.response_analysis")
assert_equal "$exit_signal" "true"
}
Loading