fix(loop): replace non-existent --prompt-file with -p flag#58
Conversation
The build_claude_command() function was incorrectly using --prompt-file which doesn't exist in Claude Code CLI. This fix: - Replaces --prompt-file with -p flag plus prompt content - Reads prompt content via $(cat "$prompt_file") before execution - Adds error handling for missing prompt files - Maintains shell injection safety through array-based command building - Updates comments to reflect the correct approach Adds 6 TDD tests verifying the fix: - Uses -p flag instead of --prompt-file - Reads prompt file content correctly - Handles missing prompt file - Includes all modern CLI flags - Handles multiline prompt content - Prevents shell injection Test count: 145 -> 151 (all passing)
WalkthroughThis PR bumps the version to v0.9.2 and fixes prompt file handling by replacing the non-existent Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
PR Review: This PR fixes a critical bug in the modern CLI implementation. The review is POSITIVE with minor observations. STRENGTHS:
OBSERVATIONS:
SECURITY REVIEW:
MEETS RALPH QUALITY STANDARDS: RECOMMENDATION: APPROVE with optional improvements The PR successfully fixes a critical bug and follows all Ralph quality standards. The observations above are minor optimizations rather than blocking issues. This is production-ready as-is. Optional improvements can be addressed in a future PR if desired. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (3)
4-4: Update the test badge to reflect the new test count.The badge still shows
145 passingbut should show151 passingto match the actual test count after adding 6 new tests in this PR.📝 Proposed fix
- +
363-377: Update test count references for consistency.Lines 363 and 380 still reference
145 tests, which should be updated to151 teststo match the new test count after this PR.📝 Proposed fix
-# Run all tests (145 tests) +# Run all tests (151 tests) bats tests/And on line 380:
-- **145 tests** across 7 test files -- **100% pass rate** (145/145 passing) +- **151 tests** across 7 test files +- **100% pass rate** (151/151 passing)
600-616: Update test count in development roadmap section.Line 606 references "145 comprehensive tests" which should be updated to "151 comprehensive tests". Also update the test coverage breakdown (line 614) from "Unit Tests: 105" to "Unit Tests: 111" (105 + 6 new tests).
📝 Proposed fix
**What's Delivered:** - Core loop functionality with intelligent exit detection - Rate limiting (100 calls/hour) and circuit breaker pattern - Response analyzer with semantic understanding -- 145 comprehensive tests (100% pass rate) +- 151 comprehensive tests (100% pass rate) - tmux integration and live monitoring - PRD import functionality - Installation system and project templates - Modern CLI commands with JSON output support - CI/CD pipeline with GitHub Actions **Test Coverage Breakdown:** -- Unit Tests: 105 (CLI parsing, JSON, exit detection, rate limiting) +- Unit Tests: 111 (CLI parsing, JSON, exit detection, rate limiting, CLI modern) - Integration Tests: 40 (loop execution, edge cases) - Test Files: 7
🤖 Fix all issues with AI agents
In @README.md:
- Line 34: The README has an inconsistent test count: the "Recent Improvements"
section (shows “151 passing tests”) conflicts with the test count badge on the
top of the file (shows “145”); update the badge value to match the Recent
Improvements count (or vice versa) so both references consistently show the same
number (e.g., change the badge text/number to 151), ensuring you edit the badge
markup near the top and the Recent Improvements line text.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
CLAUDE.mdREADME.mdralph_loop.shtests/unit/test_cli_modern.bats
🧰 Additional context used
📓 Path-based instructions (5)
ralph_loop.sh
📄 CodeRabbit inference engine (CLAUDE.md)
ralph_loop.sh: Use JSON output format for Claude Code responses by setting CLAUDE_OUTPUT_FORMAT="json"
Implement session continuity management with init_claude_session(), save_claude_session() functions and use --continue flag to preserve context across Claude Code CLI calls
Build loop context injection using build_loop_context() function to include loop number, remaining tasks, circuit breaker state, and previous work summary via --append-system-prompt
Implement rate limiting with default 100 API calls per hour (configurable via --calls flag) with automatic hourly reset and countdown display
Files:
ralph_loop.sh
**/*.sh
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.sh: Generate ISO timestamps for logging using cross-platform date utilities from lib/date_utils.sh
Keep all implementation documentation synchronized with codebase: update inline comments in bash scripts when implementation changes, remove outdated comments immediately, and keep CLAUDE.md current
Files:
ralph_loop.sh
+(install.sh|ralph_loop.sh|ralph_monitor.sh|setup.sh|create_files.sh|ralph_import.sh)
📄 CodeRabbit inference engine (CLAUDE.md)
Verify all Ralph main scripts (ralph_loop.sh, ralph_monitor.sh, setup.sh, create_files.sh, ralph_import.sh) are properly installed to ~/.ralph/ and global commands (ralph, ralph-monitor, ralph-setup, ralph-import) are available in ~/.local/bin/
Files:
ralph_loop.sh
**/*.bats
📄 CodeRabbit inference engine (CLAUDE.md)
All new features MUST achieve minimum 85% code coverage ratio, maintain 100% test pass rate, include unit tests, integration tests, and end-to-end tests, and validate coverage before marking features complete
Files:
tests/unit/test_cli_modern.bats
CLAUDE.md
📄 CodeRabbit inference engine (CLAUDE.md)
Update CLAUDE.md with new commands in 'Key Commands' section, update 'Exit Conditions and Thresholds' when logic changes, keep installation instructions accurate and tested, and document new Ralph loop behaviors
Files:
CLAUDE.md
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to CLAUDE.md : Update CLAUDE.md with new commands in 'Key Commands' section, update 'Exit Conditions and Thresholds' when logic changes, keep installation instructions accurate and tested, and document new Ralph loop behaviors
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to ralph_loop.sh : Implement session continuity management with init_claude_session(), save_claude_session() functions and use --continue flag to preserve context across Claude Code CLI calls
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to ralph_loop.sh : Build loop context injection using build_loop_context() function to include loop number, remaining tasks, circuit breaker state, and previous work summary via --append-system-prompt
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to ralph_loop.sh : Use JSON output format for Claude Code responses by setting CLAUDE_OUTPUT_FORMAT="json"
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to templates/**/* : Maintain template files (PROMPT.md template, AGENT.md template, fix_plan.md template) current with best practices and document new Ralph configuration options
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to CLAUDE.md : Update CLAUDE.md with new commands in 'Key Commands' section, update 'Exit Conditions and Thresholds' when logic changes, keep installation instructions accurate and tested, and document new Ralph loop behaviors
Applied to files:
ralph_loop.shtests/unit/test_cli_modern.batsREADME.mdCLAUDE.md
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to ralph_loop.sh : Use JSON output format for Claude Code responses by setting CLAUDE_OUTPUT_FORMAT="json"
Applied to files:
ralph_loop.shtests/unit/test_cli_modern.batsCLAUDE.md
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to ralph_loop.sh : Implement session continuity management with init_claude_session(), save_claude_session() functions and use --continue flag to preserve context across Claude Code CLI calls
Applied to files:
ralph_loop.shtests/unit/test_cli_modern.batsREADME.mdCLAUDE.md
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to ralph_loop.sh : Build loop context injection using build_loop_context() function to include loop number, remaining tasks, circuit breaker state, and previous work summary via --append-system-prompt
Applied to files:
ralph_loop.shtests/unit/test_cli_modern.bats
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to templates/**/* : Maintain template files (PROMPT.md template, AGENT.md template, fix_plan.md template) current with best practices and document new Ralph configuration options
Applied to files:
ralph_loop.sh
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to templates/**/* : Create PROMPT.md template with instructions for Ralph's autonomous behavior, fix_plan.md template with initial task structure, and AGENT.md template with build system instructions
Applied to files:
ralph_loop.sh
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to +(install.sh|ralph_loop.sh|ralph_monitor.sh|setup.sh|create_files.sh|ralph_import.sh) : Verify all Ralph main scripts (ralph_loop.sh, ralph_monitor.sh, setup.sh, create_files.sh, ralph_import.sh) are properly installed to ~/.ralph/ and global commands (ralph, ralph-monitor, ralph-setup, ralph-import) are available in ~/.local/bin/
Applied to files:
ralph_loop.sh
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to +(ralph_loop.sh|lib/response_analyzer.sh) : Use exit detection thresholds: MAX_CONSECUTIVE_TEST_LOOPS=3, MAX_CONSECUTIVE_DONE_SIGNALS=2, TEST_PERCENTAGE_THRESHOLD=30%
Applied to files:
ralph_loop.sh
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to **/*.sh : Keep all implementation documentation synchronized with codebase: update inline comments in bash scripts when implementation changes, remove outdated comments immediately, and keep CLAUDE.md current
Applied to files:
ralph_loop.sh
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: File naming convention: prefix Ralph-specific control files with @ (e.g., fix_plan.md, AGENT.md), use hidden files for state tracking (e.g., .call_count, .exit_signals), organize logs in logs/ directory with timestamps, and place auto-generated documentation in docs/generated/
Applied to files:
ralph_loop.sh
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to lib/response_analyzer.sh : Analyze Claude Code output for completion signals by detecting JSON output format and parsing structured fields (status, exit_signal, work_type, files_modified)
Applied to files:
ralph_loop.shCLAUDE.md
📚 Learning: 2025-12-31T19:31:02.350Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: templates/AGENT.md:0-0
Timestamp: 2025-12-31T19:31:02.350Z
Learning: Applies to templates/**/AGENT.md : Update AGENT.md with new build patterns, Key Learnings section, command examples, and testing patterns
Applied to files:
README.md
📚 Learning: 2025-12-31T19:31:02.350Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: templates/AGENT.md:0-0
Timestamp: 2025-12-31T19:31:02.350Z
Learning: Applies to templates/**/*.{test,spec}.{js,ts,jsx,tsx,py} : All tests must pass with 100% pass rate - no exceptions
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-31T19:31:02.350Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: templates/AGENT.md:0-0
Timestamp: 2025-12-31T19:31:02.350Z
Learning: Applies to templates/**/*.{integration,e2e}.{test,spec}.{js,ts,jsx,tsx,py} : Write integration tests for API endpoints or main functionality
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-31T19:31:02.350Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: templates/AGENT.md:0-0
Timestamp: 2025-12-31T19:31:02.350Z
Learning: Applies to templates/**/*.{e2e,end-to-end}.{test,spec}.{js,ts,jsx,tsx,py} : Write end-to-end tests for critical user workflows
Applied to files:
CLAUDE.md
📚 Learning: 2025-12-31T19:31:02.350Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: templates/AGENT.md:0-0
Timestamp: 2025-12-31T19:31:02.350Z
Learning: Applies to templates/**/*.{js,ts,jsx,tsx,py,rs} : Write unit tests for all business logic and services
Applied to files:
CLAUDE.md
📚 Learning: 2026-01-09T04:26:33.873Z
Learnt from: CR
Repo: frankbria/ralph-claude-code PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-09T04:26:33.873Z
Learning: Applies to **/*.bats : All new features MUST achieve minimum 85% code coverage ratio, maintain 100% test pass rate, include unit tests, integration tests, and end-to-end tests, and validate coverage before marking features complete
Applied to files:
CLAUDE.md
🧬 Code graph analysis (2)
ralph_loop.sh (1)
tests/unit/test_cli_modern.bats (2)
log_status(53-57)build_claude_command(431-479)
tests/unit/test_cli_modern.bats (2)
ralph_loop.sh (1)
build_claude_command(486-536)tests/helpers/test_helper.bash (1)
assert_failure(13-19)
🪛 markdownlint-cli2 (0.18.1)
README.md
38-38: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: coverage
- GitHub Check: claude-review
🔇 Additional comments (14)
ralph_loop.sh (2)
485-536: LGTM! Correct fix for the prompt file handling bug.The implementation correctly:
- Adds prompt file validation before building the command (lines 494-498)
- Reads the prompt file content and passes it via
-pflag instead of the non-existent--prompt-file(lines 530-535)- Maintains shell injection safety through array-based command building
- Updates comments to accurately reflect the
-pflag usageThe error handling returns early on missing files, preventing downstream failures.
566-580: Good defensive programming with fallback handling.The modern CLI path now properly validates that
build_claude_commandsucceeds before enabling modern mode (line 572). If the command build fails (e.g., missing prompt file), it falls back to legacy mode with a warning (lines 576-577). This prevents silent failures and maintains backward compatibility.CLAUDE.md (4)
9-9: Documentation correctly updated for v0.9.2.Version, test count (151), and pass rate (100%) are accurately reflected.
275-286: Test file breakdown is accurate.The table correctly shows test_cli_modern.bats with 29 tests (23 previous + 6 new = 29) and the updated description mentioning the build_claude_command fix. Total of 151 tests matches the sum: 27 + 29 + 20 + 20 + 15 + 20 + 20 = 151.
301-308: Excellent documentation of the v0.9.2 changes.The "Prompt File Fix" section clearly describes:
- The critical bug (non-existent
--prompt-fileflag)- The solution (using
-pwith prompt content)- Added error handling
- Test additions
- Safety considerations
This provides clear context for users and future maintainers.
345-345: CLI flag documentation correctly updated.The description accurately reflects that
-pis used with content read from file via command substitution, not--prompt-file.README.md (1)
38-44: Well-documented v0.9.2 improvements.The "Prompt File Fix" section clearly explains the bug fix, the solution using
-pwith command substitution, error handling additions, and test coverage increases. This matches the implementation and provides good context for users.tests/unit/test_cli_modern.bats (7)
420-479: Well-structured test helper function.The
build_claude_commandfunction in the test file is a faithful copy of the implementation for isolated testing. The function correctly:
- Validates prompt file existence (lines 439-443)
- Adds output format flag (lines 445-448)
- Parses and adds allowed tools (lines 450-463)
- Adds session continuity flag (lines 465-468)
- Adds loop context (lines 470-473)
- Reads prompt content and uses
-pflag (lines 475-478)This duplication is appropriate for unit testing and matches the actual implementation in ralph_loop.sh.
481-500: Excellent test for the core bug fix.This test validates that the command uses
-pflag instead of the non-existent--prompt-fileflag. The test checks both the negative case (should NOT contain --prompt-file) and positive case (should contain -p), which is thorough.
502-517: Good test for prompt content reading.This test verifies that the prompt file content is actually read and included in the command arguments. Using specific content ("My specific prompt content for testing") makes the assertion clear and unambiguous.
519-533: Critical test for error handling.This test validates that
build_claude_commandfails gracefully when the prompt file is missing. The test usesassert_failureand checks for error messaging, ensuring robust error handling as documented in the PR objectives.
535-558: Comprehensive integration test for modern CLI flags.This test validates that all modern CLI flags are correctly included when enabled:
--output-format json--allowedToolswith multiple tools--continue--append-system-promptwith context-pwith prompt contentThis ensures the complete modern CLI command building works end-to-end.
560-597: Excellent test for multiline prompt handling.This test creates a realistic multiline prompt with headers, paragraphs, and empty lines, then verifies that the content is preserved in the command arguments. The test properly:
- Iterates through the array to find the
-pflag- Checks that the next element contains the multiline content
- Verifies multiple distinct parts of the content
This ensures that complex prompts work correctly with the array-based approach.
599-623: Critical security test for shell injection prevention.This test creates a prompt file with various dangerous shell metacharacters:
- Command substitution:
$(dangerous)and$(command)- Backticks
- Variable expansion:
$VARand${VAR}- Shell commands:
; rm -rf /The test verifies that these are preserved literally in the array (not executed), confirming that the array-based approach maintains shell injection safety as documented in the PR objectives.
Summary
build_claude_command()used non-existent--prompt-fileCLI flag-pflagChanges
ralph_loop.sh--prompt-file "$prompt_file"with-p "$prompt_content"using command substitutiontests/unit/test_cli_modern.batsbuild_claude_commandfunction:-pflag instead of--prompt-fileDocumentation
Test plan
ralph --monitorwith default JSON output format--output-format textSummary by CodeRabbit
Bug Fixes
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.