Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
22 changes: 20 additions & 2 deletions lib/response_analyzer.sh
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,22 @@ 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 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 -- "---RALPH_STATUS---|RALPH_STATUS:"; then
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
# 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 +499,12 @@ 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 downstream EXIT_SIGNAL/STATUS extraction handles both
# 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 -- "---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
83 changes: 83 additions & 0 deletions tests/unit/test_json_parsing.bats
Original file line number Diff line number Diff line change
Expand Up @@ -1155,3 +1155,86 @@ 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"
}
Loading