Conversation
📝 WalkthroughWalkthroughThis PR: removes a documentation step in refactor-clean.md; adds a ralph-status command doc and script; changes session-start hook to always overwrite the session env file; adds cleanup in cancel-ralph.sh to remove RALPH_SESSION_ID; and adds extensive unit and tmux integration tests for ralph-loop. Changes
Sequence DiagramsequenceDiagram
participant User
participant ralph-status.sh
participant SessionEnv as "session-env.sh"
participant StateLib as "lib/state.sh"
participant StateFile as "state file"
User->>ralph-status.sh: run ralph-status
ralph-status.sh->>SessionEnv: source $HOME/.claude/ralph-loop/session-env.sh
SessionEnv-->>ralph-status.sh: RALPH_SESSION_ID (or none)
alt no session ID
ralph-status.sh->>User: print "no active session" guidance
else has session ID
ralph-status.sh->>ralph-status.sh: validate session ID
alt invalid ID
ralph-status.sh->>User: error and exit
else valid ID
ralph-status.sh->>StateFile: check path based on session ID
alt state file missing
ralph-status.sh->>User: indicate stale/mismatch
else state file exists
ralph-status.sh->>StateLib: parse_frontmatter(state file)
StateLib-->>ralph-status.sh: iteration, max, completion_promise
ralph-status.sh->>User: display session id, iteration/max, promise
ralph-status.sh->>User: show state file path and prompt excerpt
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/me/commands/refactor-clean.md (1)
16-30:⚠️ Potential issue | 🟡 MinorFix step numbering to avoid the missing “3”.
The list now jumps from 2 to 4, which is confusing in a command guide. Please renumber steps to be sequential.
🛠️ Suggested doc fix
-4. Propose safe deletions only +3. Propose safe deletions only -5. Before each deletion: +4. Before each deletion: -6. Show summary of cleaned items +5. Show summary of cleaned items
🤖 Fix all issues with AI agents
In `@plugins/ralph-loop/commands/ralph-status.md`:
- Around line 5-10: Add the missing blank lines required by markdownlint: insert
one blank line immediately after the heading "Ralph Loop Status" and ensure
there is a blank line before and after the fenced code block containing the
command `!"${CLAUDE_PLUGIN_ROOT}/scripts/ralph-status.sh"` so the heading and
the code block are each separated by blank lines (fixes MD022 and MD031).
In `@plugins/ralph-loop/tests/tmux-test-ralph-loop.bats`:
- Around line 106-157: The tests call skip_if_no_tmux unnecessarily and include
dead tmux setup; in the `@test` "ralph-loop: SessionStart hook overwrites
RALPH_SESSION_ID (bug fix)" (and other tests that do not run tmux commands)
remove the skip_if_no_tmux guard and any unused local SESSION_NAME and trap
lines (e.g., SESSION_NAME and the tmux kill-session trap), or alternatively move
these pure-hook tests into a separate ralph-loop-specific.bats file; ensure only
tests that actually invoke tmux (use tmux new-session, send-keys, capture-pane)
keep skip_if_no_tmux and tmux session setup/teardown.
🧹 Nitpick comments (2)
plugins/ralph-loop/scripts/cancel-ralph.sh (1)
47-58:SESSION_ENV_FILEis declared twice with the same value.Line 11 and line 48 both set
SESSION_ENV_FILE="$HOME/.claude/ralph-loop/session-env.sh". If either path changes in the future, the other could easily be missed. Consider removing the re-declaration on line 48 and reusing the variable from line 11.♻️ Remove redundant declaration
-# Remove RALPH_SESSION_ID from session-env.sh to prevent stale session IDs -SESSION_ENV_FILE="$HOME/.claude/ralph-loop/session-env.sh" -if [[ -f "$SESSION_ENV_FILE" ]]; then +# Remove RALPH_SESSION_ID from session-env.sh to prevent stale session IDs +if [[ -f "$SESSION_ENV_FILE" ]]; thenplugins/ralph-loop/tests/tmux-test-ralph-loop.bats (1)
19-38:wait_for_outputhelper is defined but never called.This function is dead code — none of the tests in the file use it. Consider removing it or adding tests that actually exercise tmux pane output capture.
| --- | ||
| # Ralph Loop Status | ||
| Execute the status script: | ||
| ```! | ||
| "${CLAUDE_PLUGIN_ROOT}/scripts/ralph-status.sh" | ||
| ``` |
There was a problem hiding this comment.
Missing blank lines flagged by markdownlint.
Static analysis reports MD022 (blank line needed below heading at line 6) and MD031 (blank line needed around fenced code block at line 8). Adding blank lines would fix both:
📝 Proposed fix
---
# Ralph Loop Status
+
Execute the status script:
+
```!
"${CLAUDE_PLUGIN_ROOT}/scripts/ralph-status.sh"
```📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --- | |
| # Ralph Loop Status | |
| Execute the status script: | |
| ```! | |
| "${CLAUDE_PLUGIN_ROOT}/scripts/ralph-status.sh" | |
| ``` | |
| --- | |
| # Ralph Loop Status | |
| Execute the status script: | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 8-8: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@plugins/ralph-loop/commands/ralph-status.md` around lines 5 - 10, Add the
missing blank lines required by markdownlint: insert one blank line immediately
after the heading "Ralph Loop Status" and ensure there is a blank line before
and after the fenced code block containing the command
`!"${CLAUDE_PLUGIN_ROOT}/scripts/ralph-status.sh"` so the heading and the code
block are each separated by blank lines (fixes MD022 and MD031).
| @test "ralph-loop: SessionStart hook overwrites RALPH_SESSION_ID (bug fix)" { | ||
| skip_if_no_tmux | ||
|
|
||
| local SESSION_NAME="ralph-overwrite-$$-$RANDOM" | ||
|
|
||
| trap "tmux kill-session -t '$SESSION_NAME' 2>/dev/null || true" EXIT | ||
|
|
||
| # Setup: Create initial state file | ||
| local TEST_STATE_FILE1="$HOME/.claude/ralph-loop/ralph-loop-session-alpha.local.md" | ||
| cat > "$TEST_STATE_FILE1" <<'EOF' | ||
| --- | ||
| iteration: 0 | ||
| max_iterations: 10 | ||
| completion_promise: "DONE" | ||
| session_id: session-alpha | ||
| --- | ||
| Test prompt | ||
| EOF | ||
|
|
||
| # First SessionStart hook call | ||
| export CLAUDE_ENV_FILE="$SESSION_ENV_FILE" | ||
| echo "{\"session_id\": \"session-alpha\"}" | "$PLUGIN_DIR/hooks/session-start-hook.sh" | ||
|
|
||
| # Verify first session ID | ||
| grep -q "export RALPH_SESSION_ID=session-alpha" "$SESSION_ENV_FILE" | ||
|
|
||
| # Create second state file (simulating new session) | ||
| local TEST_STATE_FILE2="$HOME/.claude/ralph-loop/ralph-loop-session-beta.local.md" | ||
| cat > "$TEST_STATE_FILE2" <<'EOF' | ||
| --- | ||
| iteration: 0 | ||
| max_iterations: 10 | ||
| completion_promise: "DONE" | ||
| session_id: session-beta | ||
| --- | ||
| Test prompt | ||
| EOF | ||
|
|
||
| # Second SessionStart hook call | ||
| echo "{\"session_id\": \"session-beta\"}" | "$PLUGIN_DIR/hooks/session-start-hook.sh" | ||
|
|
||
| # Verify second session ID OVERWROTE (not appended to) the first | ||
| grep -q "export RALPH_SESSION_ID=session-beta" "$SESSION_ENV_FILE" | ||
|
|
||
| # Verify only ONE line exists (the bug was that it would append) | ||
| local count | ||
| count=$(grep -c "RALPH_SESSION_ID" "$SESSION_ENV_FILE" || echo "0") | ||
| [ "$count" -eq 1 ] | ||
|
|
||
| # Verify old session ID is NOT present | ||
| ! grep -q "session-alpha" "$SESSION_ENV_FILE" | ||
| } |
There was a problem hiding this comment.
Most tests in this file skip on skip_if_no_tmux but never actually use tmux.
Only the first test (line 75) creates a tmux session. Tests 2–11 invoke hooks/scripts directly yet all call skip_if_no_tmux, meaning they'll be skipped unnecessarily in CI environments that lack tmux. This creates a test coverage gap.
Consider either:
- Removing
skip_if_no_tmuxfrom tests that don't use tmux, or - Moving the non-tmux tests into
ralph-loop-specific.bats.
Additionally, several tests declare SESSION_NAME and set traps to kill tmux sessions that are never created — this is dead code that could confuse future contributors.
#!/bin/bash
# Verify which tests actually invoke tmux commands (new-session, send-keys, capture-pane)
echo "=== Tests calling skip_if_no_tmux ==="
rg -n 'skip_if_no_tmux' plugins/ralph-loop/tests/tmux-test-ralph-loop.bats
echo ""
echo "=== Lines with actual tmux usage (new-session, send-keys, capture-pane) ==="
rg -n 'tmux (new-session|send-keys|capture-pane)' plugins/ralph-loop/tests/tmux-test-ralph-loop.bats
echo ""
echo "=== Tests declaring SESSION_NAME ==="
rg -n 'local SESSION_NAME=' plugins/ralph-loop/tests/tmux-test-ralph-loop.bats🤖 Prompt for AI Agents
In `@plugins/ralph-loop/tests/tmux-test-ralph-loop.bats` around lines 106 - 157,
The tests call skip_if_no_tmux unnecessarily and include dead tmux setup; in the
`@test` "ralph-loop: SessionStart hook overwrites RALPH_SESSION_ID (bug fix)" (and
other tests that do not run tmux commands) remove the skip_if_no_tmux guard and
any unused local SESSION_NAME and trap lines (e.g., SESSION_NAME and the tmux
kill-session trap), or alternatively move these pure-hook tests into a separate
ralph-loop-specific.bats file; ensure only tests that actually invoke tmux (use
tmux new-session, send-keys, capture-pane) keep skip_if_no_tmux and tmux session
setup/teardown.
The SessionStart hook was appending to session-env.sh instead of overwriting, causing stale session IDs to accumulate over time. Changes: - session-start-hook.sh: Always overwrite (>) instead of append (>>) - cancel-ralph.sh: Remove RALPH_SESSION_ID from session-env.sh on cancel - Add tmux integration tests to verify fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a new /ralph-status command that displays the current Ralph Loop status including iteration count, max iterations, completion promise, state file location, and the current prompt. Changes: - Add ralph-status.md command file - Add ralph-status.sh script with status display - Add tests for the new command Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add functional tests for the Perl regex that extracts completion promises from assistant output. Tests verify: - Basic promise extraction - Multiline promise handling with whitespace normalization - Non-greedy matching (takes FIRST promise) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change script paths from ${CLAUDE_PLUGIN_ROOT}/scripts/ to scripts/
- Add default fallback to 'main' if gh repo view fails
- Use bash -c for verify-pr-status.sh to ensure proper argument passing
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@plugins/ralph-loop/tests/tmux-test-ralph-loop.bats`:
- Around line 40-49: The setup and teardown functions currently remove
$HOME/.claude/ralph-loop and can clobber a developer's real state; change setup
to create a temporary directory (using mktemp -d), set HOME or the plugin's
STATE_DIR environment variable to that temp dir before creating the test state,
and change teardown to remove that temp dir (not $HOME/.claude/ralph-loop).
Update the setup and teardown functions in tmux-test-ralph-loop.bats (functions
named setup and teardown) to export the temp path for tests, create any required
subdirs there, and always cleanup the same temp directory to avoid deleting real
user data.
🧹 Nitpick comments (2)
plugins/me/commands/create-pr.md (1)
49-53: Prefer portable${CLAUDE_PLUGIN_ROOT}paths in documented commands.The docs now assume the working directory is the plugin root. To keep the instructions portable (e.g., when invoked from elsewhere), consider using
${CLAUDE_PLUGIN_ROOT}in the command examples.♻️ Proposed doc tweak
-BASE=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name || echo "main") -# Conflict check (REQUIRED) -bash scripts/check-conflicts.sh "$BASE" +BASE=$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name || echo "main") +# Conflict check (REQUIRED) +bash "${CLAUDE_PLUGIN_ROOT}/scripts/check-conflicts.sh" "$BASE"-# Run in bash subshell to ensure proper argument passing -bash -c "bash scripts/verify-pr-status.sh \"$BASE\"" +# Run in bash subshell to ensure proper argument passing +bash -c "bash \"${CLAUDE_PLUGIN_ROOT}/scripts/verify-pr-status.sh\" \"$BASE\""Also applies to: 71-72
plugins/ralph-loop/scripts/cancel-ralph.sh (1)
47-58: DuplicateSESSION_ENV_FILEdeclaration.
SESSION_ENV_FILEis already defined on line 11 with the same value. The redeclaration on line 48 is redundant.♻️ Remove the duplicate declaration
# Remove RALPH_SESSION_ID from session-env.sh to prevent stale session IDs -SESSION_ENV_FILE="$HOME/.claude/ralph-loop/session-env.sh" if [[ -f "$SESSION_ENV_FILE" ]]; then
| setup() { | ||
| # Clean up any existing Ralph Loop state | ||
| rm -rf "$HOME/.claude/ralph-loop" | ||
| mkdir -p "$HOME/.claude/ralph-loop" | ||
| } | ||
|
|
||
| teardown() { | ||
| # Clean up Ralph Loop state after each test | ||
| rm -rf "$HOME/.claude/ralph-loop" | ||
| } |
There was a problem hiding this comment.
setup/teardown destructively wipes $HOME/.claude/ralph-loop — risks destroying real session state.
Running these tests on a developer machine with an active Ralph Loop will delete all state files. Consider using a temporary directory via mktemp -d and overriding HOME or STATE_DIR for test isolation:
🛡️ Proposed fix — isolate tests from real home directory
+TEST_HOME=""
+
setup() {
- # Clean up any existing Ralph Loop state
- rm -rf "$HOME/.claude/ralph-loop"
- mkdir -p "$HOME/.claude/ralph-loop"
+ TEST_HOME=$(mktemp -d)
+ export HOME="$TEST_HOME"
+ SESSION_ENV_FILE="$HOME/.claude/ralph-loop/session-env.sh"
+ mkdir -p "$HOME/.claude/ralph-loop"
}
teardown() {
- # Clean up Ralph Loop state after each test
- rm -rf "$HOME/.claude/ralph-loop"
+ rm -rf "$TEST_HOME"
}🤖 Prompt for AI Agents
In `@plugins/ralph-loop/tests/tmux-test-ralph-loop.bats` around lines 40 - 49, The
setup and teardown functions currently remove $HOME/.claude/ralph-loop and can
clobber a developer's real state; change setup to create a temporary directory
(using mktemp -d), set HOME or the plugin's STATE_DIR environment variable to
that temp dir before creating the test state, and change teardown to remove that
temp dir (not $HOME/.claude/ralph-loop). Update the setup and teardown functions
in tmux-test-ralph-loop.bats (functions named setup and teardown) to export the
temp path for tests, create any required subdirs there, and always cleanup the
same temp directory to avoid deleting real user data.
Pull request was closed
Summary
/ralph-statuscommand to display current Ralph Loop statusChanges
Bug Fix
session-start-hook.sh: Always overwrite (>) instead of append (>>) RALPH_SESSION_IDcancel-ralph.sh: Clean up RALPH_SESSION_ID from session-env.sh when cancellingNew
/ralph-statusCommandTest Coverage
Test plan
/ralph-statuscommand🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests