fix: address PR review feedback from CodeRabbit
- Disable live mode when build_claude_command fails (not just empty array) - Strengthen safety check to verify use_modern_cli=true - Fix case-sensitive sed range in test (execute→Execute) - Update CLAUDE.md: auto-switch docs, test counts (484→490)
This commit is contained in:
parent
4027ac929a
commit
131b95db38
3 changed files with 15 additions and 10 deletions
|
|
@ -169,7 +169,7 @@ tmux attach -t <session-name>
|
|||
|
||||
### Running Tests
|
||||
```bash
|
||||
# Run all tests (484 tests)
|
||||
# Run all tests (490 tests)
|
||||
npm test
|
||||
|
||||
# Run specific test suites
|
||||
|
|
@ -214,7 +214,7 @@ CLAUDE_MIN_VERSION="2.0.76" # Minimum Claude CLI version
|
|||
```
|
||||
|
||||
**CLI Options:**
|
||||
- `--output-format json|text` - Set Claude output format (default: json)
|
||||
- `--output-format json|text` - Set Claude output format (default: json). Note: `--live` mode requires JSON and will auto-switch from text to json.
|
||||
- `--allowed-tools "Write,Read,Bash(git *)"` - Restrict allowed tools
|
||||
- `--no-continue` - Disable session continuity, start fresh each loop
|
||||
|
||||
|
|
@ -445,13 +445,13 @@ Ralph uses advanced error detection with two-stage filtering to eliminate false
|
|||
|
||||
## Test Suite
|
||||
|
||||
### Test Files (484 tests total)
|
||||
### Test Files (490 tests total)
|
||||
|
||||
| File | Tests | Description |
|
||||
|------|-------|-------------|
|
||||
| `test_circuit_breaker_recovery.bats` | 19 | Cooldown timer, auto-reset, parse_iso_to_epoch, CLI flag (Issue #160) |
|
||||
| `test_cli_parsing.bats` | 35 | CLI argument parsing for all flags + monitor parameter forwarding |
|
||||
| `test_cli_modern.bats` | 33 | Modern CLI commands (Phase 1.1) + build_claude_command fix |
|
||||
| `test_cli_modern.bats` | 39 | Modern CLI commands (Phase 1.1) + build_claude_command fix + live mode text format fix (#164) |
|
||||
| `test_json_parsing.bats` | 52 | JSON output format parsing + Claude CLI format + session management + array format |
|
||||
| `test_session_continuity.bats` | 44 | Session lifecycle management + expiration + circuit breaker integration + issue #91 fix |
|
||||
| `test_exit_detection.bats` | 53 | Exit signal detection + EXIT_SIGNAL-based completion indicators + progress detection |
|
||||
|
|
|
|||
|
|
@ -1062,6 +1062,10 @@ execute_claude_code() {
|
|||
log_status "INFO" "Using modern CLI mode (${CLAUDE_OUTPUT_FORMAT} output)"
|
||||
else
|
||||
log_status "WARN" "Failed to build modern CLI command, falling back to legacy mode"
|
||||
if [[ "$LIVE_OUTPUT" == "true" ]]; then
|
||||
log_status "ERROR" "Live mode requires a built Claude command. Falling back to background mode."
|
||||
LIVE_OUTPUT=false
|
||||
fi
|
||||
fi
|
||||
|
||||
# Execute Claude Code
|
||||
|
|
@ -1091,9 +1095,9 @@ execute_claude_code() {
|
|||
fi
|
||||
|
||||
if [[ "$LIVE_OUTPUT" == "true" ]]; then
|
||||
# Safety check: CLAUDE_CMD_ARGS must be populated for live mode
|
||||
if [[ ${#CLAUDE_CMD_ARGS[@]} -eq 0 ]]; then
|
||||
log_status "ERROR" "CLAUDE_CMD_ARGS is empty — cannot build live mode command. Falling back to background mode."
|
||||
# Safety check: live mode requires a successfully built modern command
|
||||
if [[ "$use_modern_cli" != "true" || ${#CLAUDE_CMD_ARGS[@]} -eq 0 ]]; then
|
||||
log_status "ERROR" "Live mode requires a built Claude command. Falling back to background mode."
|
||||
LIVE_OUTPUT=false
|
||||
fi
|
||||
fi
|
||||
|
|
|
|||
|
|
@ -740,10 +740,11 @@ EOF
|
|||
|
||||
@test "safety check prevents live mode with empty CLAUDE_CMD_ARGS" {
|
||||
# Verify ralph_loop.sh has the safety check for empty CLAUDE_CMD_ARGS
|
||||
run grep -A3 'CLAUDE_CMD_ARGS.*-eq 0' "${BATS_TEST_DIRNAME}/../../ralph_loop.sh"
|
||||
# The check also verifies use_modern_cli is true (not just non-empty array)
|
||||
run grep -A3 'use_modern_cli.*CLAUDE_CMD_ARGS.*-eq 0' "${BATS_TEST_DIRNAME}/../../ralph_loop.sh"
|
||||
|
||||
# Should find safety check that falls back to background mode
|
||||
[[ "$output" == *"LIVE_OUTPUT"* ]] || [[ "$output" == *"empty"* ]] || [[ "$output" == *"background"* ]]
|
||||
[[ "$output" == *"LIVE_OUTPUT"* ]] || [[ "$output" == *"background"* ]]
|
||||
}
|
||||
|
||||
@test "build_claude_command is called regardless of output format in ralph_loop.sh" {
|
||||
|
|
@ -757,7 +758,7 @@ EOF
|
|||
|
||||
# The old pattern: "json" check immediately followed by build_claude_command
|
||||
# should no longer exist as a gate
|
||||
run bash -c "sed -n '/# Build the Claude CLI command/,/execute Claude Code/p' '$script' | grep -c 'CLAUDE_OUTPUT_FORMAT.*json.*build_claude_command'"
|
||||
run bash -c "sed -n '/# Build the Claude CLI command/,/# Execute Claude Code/p' '$script' | grep -c 'CLAUDE_OUTPUT_FORMAT.*json.*build_claude_command'"
|
||||
|
||||
# Should find 0 matches (the gate has been removed)
|
||||
[[ "$output" == "0" ]]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue