Skip to content

fix(analyzer): respect explicit EXIT_SIGNAL:false with STATUS:COMPLETE#147

Closed
linuxkd wants to merge 1 commit into
frankbria:mainfrom
linuxkd:fix/exit-signal-override-146
Closed

fix(analyzer): respect explicit EXIT_SIGNAL:false with STATUS:COMPLETE#147
linuxkd wants to merge 1 commit into
frankbria:mainfrom
linuxkd:fix/exit-signal-override-146

Conversation

@linuxkd
Copy link
Copy Markdown

@linuxkd linuxkd commented Feb 1, 2026

Summary

Fixes #146

When Claude outputs STATUS: COMPLETE with EXIT_SIGNAL: false, it means "this task is complete, but there are more tasks to do". The previous code incorrectly treated STATUS: COMPLETE as overriding EXIT_SIGNAL, causing Ralph to prematurely exit after completing a single task.

Problem

In lib/response_analyzer.sh lines 139-146, the condition:

if [[ "$embedded_status" == "COMPLETE" && "$exit_signal" != "true" ]]; then

Was TRUE when exit_signal is "false" (not equal to "true"), so it overrode Claude's explicit EXIT_SIGNAL: false and set exit_signal="true".

The condition $exit_signal != "true" doesn't distinguish between:

  1. EXIT_SIGNAL was not specified (should use STATUS as fallback)
  2. EXIT_SIGNAL was explicitly set to "false" (should respect Claude's intent)

Solution

Change the condition to -z "$embedded_exit_sig" which only triggers when EXIT_SIGNAL was not extracted from the RALPH_STATUS block at all:

if [[ "$embedded_status" == "COMPLETE" && -z "$embedded_exit_sig" ]]; then

Test Plan

  • Test 53: Verifies exit_signal stays false when Claude explicitly sets EXIT_SIGNAL: false with STATUS: COMPLETE
  • Test 54: Verifies STATUS: COMPLETE is still used as fallback when EXIT_SIGNAL is absent
  • All 54 tests in test_json_parsing.bats pass
  • All 40 tests in test_exit_detection.bats pass

Summary by CodeRabbit

  • Bug Fixes
    • Fixed handling so explicit exit signals are respected and not overridden by status indicators; status is now used only as a fallback when no explicit signal exists. Debug output updated to reflect this behavior.
  • Tests
    • Added unit tests that verify explicit exit signals override status and that status is used only as a fallback.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 1, 2026

Walkthrough

This PR ensures explicit EXIT_SIGNAL values emitted in RALPH_STATUS blocks are preserved during parsing in lib/response_analyzer.sh; STATUS: COMPLETE is now used to infer an exit signal only when no explicit EXIT_SIGNAL was provided.

Changes

Cohort / File(s) Summary
Response analyzer
lib/response_analyzer.sh
Add tracking (explicit_exit_signal_found) for an explicitly provided EXIT_SIGNAL, adjust parsing to preserve explicit true/false values, and change fallback so STATUS: COMPLETE only sets exit when no explicit EXIT_SIGNAL was present; update debug messages and comments.
Unit tests
tests/unit/test_json_parsing.bats
Add tests verifying explicit EXIT_SIGNAL: false is respected even when STATUS: COMPLETE is present, and that STATUS: COMPLETE acts as a fallback only when EXIT_SIGNAL is absent.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 Hopping through logs with a carrot and grin,
I found an explicit signal and kept it within.
No STATUS shall shout when false was declared,
Claude's intent honored — the parser prepared! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary fix: respecting explicit EXIT_SIGNAL:false even when STATUS:COMPLETE is present, which matches the core issue being resolved.
Linked Issues check ✅ Passed The PR implements all core coding requirements from issue #146: correcting the parsing logic, replacing the condition with a presence check, and adding tests verifying both explicit EXIT_SIGNAL respect and STATUS fallback behavior.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the EXIT_SIGNAL override bug in lib/response_analyzer.sh and adding corresponding unit tests, with no unrelated modifications present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@macroscopeapp
Copy link
Copy Markdown
Contributor

macroscopeapp Bot commented Feb 1, 2026

Respect explicit EXIT_SIGNAL:false when STATUS: COMPLETE in parse_json_response within response_analyzer.sh

Update parse_json_response to track explicit EXIT_SIGNAL presence and preserve false values from top-level JSON or RALPH_STATUS; infer exit_signal=true from STATUS: COMPLETE only when EXIT_SIGNAL is absent. Add unit tests covering precedence.

📍Where to Start

Start with parse_json_response in response_analyzer.sh, then review new tests in tests/unit/test_json_parsing.bats.


Macroscope summarized afec773.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@lib/response_analyzer.sh`:
- Around line 139-146: The STATUS-based inference currently unconditionally sets
exit_signal="true" when embedded_status=="COMPLETE" even if an explicit
EXIT_SIGNAL:false was provided elsewhere; change the logic in
lib/response_analyzer.sh so that you only set exit_signal from STATUS when no
explicit EXIT_SIGNAL was provided. Concretely, ensure you track whether an
explicit EXIT_SIGNAL was found (e.g., introduce/use a flag alongside
embedded_exit_sig) and modify the block that inspects embedded_status (the code
referencing embedded_status, embedded_exit_sig, and exit_signal) to only assign
exit_signal="true" when the explicit-exit flag is unset/empty; preserve the
DEBUG echo behavior when you infer the signal.

Comment thread lib/response_analyzer.sh
frankbria#146)

When Claude outputs STATUS: COMPLETE with EXIT_SIGNAL: false, it means
"this task is complete, but there are more tasks to do". The previous
code incorrectly treated STATUS: COMPLETE as overriding EXIT_SIGNAL.

Root cause: Two locations in parse_json_response() could override an
explicit EXIT_SIGNAL:false:
1. Lines 139-146: STATUS:COMPLETE fallback logic
2. Lines 199-205: Normalize values block

Fix: Introduce `explicit_exit_signal_found` flag to track whether an
EXIT_SIGNAL was explicitly provided. Only use STATUS/completion_status
as fallback when no explicit EXIT_SIGNAL was found.

Changes:
- lib/response_analyzer.sh: Track explicit EXIT_SIGNAL, respect it in
  both the RALPH_STATUS extraction and normalization blocks
- tests/unit/test_json_parsing.bats: Add 2 tests for the fix (53, 54)

Also fixes SC2155 shellcheck warning by separating declare and assign.

Test count: 54 tests in test_json_parsing.bats (100% pass rate)
@linuxkd linuxkd force-pushed the fix/exit-signal-override-146 branch from 91c4b80 to afec773 Compare February 1, 2026 01:02
@linuxkd
Copy link
Copy Markdown
Author

linuxkd commented Feb 1, 2026

Updated based on CodeRabbit's review feedback:

  • Added explicit_exit_signal_found flag to properly track when EXIT_SIGNAL is explicitly provided
  • Fixed the second override location at line 199-205 (normalize values block) that was also overriding explicit EXIT_SIGNAL:false
  • Fixed SC2155 shellcheck warning by separating declare and assign

All 94 tests passing (54 json_parsing + 40 exit_detection).

@frankbria
Copy link
Copy Markdown
Owner

Thank you @linuxkd for this excellent fix! 🙏

Applied in commit 43bba1b.

The fix properly tracks whether EXIT_SIGNAL was explicitly provided vs inferred from STATUS. Now when Claude outputs:

STATUS: COMPLETE
EXIT_SIGNAL: false

...it correctly continues working ("task complete, but more tasks remain") instead of exiting prematurely.

This closes issue #146. Credit given in the commit message!

@frankbria frankbria closed this Feb 2, 2026
frankbria pushed a commit that referenced this pull request Feb 2, 2026
When Claude outputs STATUS: COMPLETE with EXIT_SIGNAL: false, it means
"this task is complete, but there are more tasks to do". Previously,
STATUS: COMPLETE incorrectly overrode EXIT_SIGNAL, causing premature exit.

The fix:
- Track whether EXIT_SIGNAL was explicitly provided (vs inferred)
- Only use STATUS: COMPLETE as fallback when no EXIT_SIGNAL was specified
- Explicit EXIT_SIGNAL: false is now respected (continues working)

Fixes #146
Credit: @linuxkd (PR #147)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: STATUS: COMPLETE overrides explicit EXIT_SIGNAL: false in parse_json_response

2 participants