Skip to content

test(ralph-loop): add functional promise extraction tests - #442

Closed
baleen37 wants to merge 5 commits into
mainfrom
fix/loop
Closed

test(ralph-loop): add functional promise extraction tests#442
baleen37 wants to merge 5 commits into
mainfrom
fix/loop

Conversation

@baleen37

@baleen37 baleen37 commented Feb 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Bug fix: Prevent RALPH_SESSION_ID accumulation in session-env.sh by always overwriting instead of appending
  • Feature: Add /ralph-status command to display current Ralph Loop status
  • Tests: Add end-to-end loop tests and functional promise extraction tests

Changes

Bug Fix

  • session-start-hook.sh: Always overwrite (>) instead of append (>>) RALPH_SESSION_ID
  • cancel-ralph.sh: Clean up RALPH_SESSION_ID from session-env.sh when cancelling

New /ralph-status Command

  • Shows session ID, iteration count, max iterations, completion promise
  • Displays state file location and current prompt
  • Handles "no active loop" case gracefully

Test Coverage

  • E2E tmux tests for loop iteration, completion by promise, and max iterations
  • Functional tests for Perl promise extraction (multiline, whitespace normalization, non-greedy)
  • All 72 tests passing

Test plan

  • All existing tests pass
  • New unit tests for promise extraction
  • New tmux integration tests for loop behavior
  • Manual testing of /ralph-status command

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a ralph-status command to show active Ralph Loop session details and iteration status.
  • Bug Fixes

    • Improved cleanup when canceling a loop to remove session exports.
    • Made session start overwrite prior session-env entries to ensure single-line session IDs.
  • Documentation

    • Added usage docs for ralph-status and adjusted command docs (including a removed/renumbered step).
  • Tests

    • Added extensive unit and integration tests covering loop lifecycle and status behavior.

@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This 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

Cohort / File(s) Summary
Documentation
plugins/me/commands/refactor-clean.md, plugins/me/commands/create-pr.md, plugins/ralph-loop/commands/ralph-status.md
Removed the "Generate comprehensive report" step (renumbering) in refactor-clean.md; adjusted create-pr pre-push commands and base-branch fallback to main; added ralph-status command doc with front matter referencing the new status script.
Session start hook
plugins/ralph-loop/hooks/session-start-hook.sh
Changed ENV_FILE write from conditional append/create to unconditional overwrite of the session-env file (single > write).
Cancellation cleanup
plugins/ralph-loop/scripts/cancel-ralph.sh
Added logic to remove export RALPH_SESSION_ID=... lines from session-env.sh, cleanup temp files, and delete session-env.sh if empty after cancellation.
Status script
plugins/ralph-loop/scripts/ralph-status.sh
New script: loads session-env, validates session ID, locates/parses state file frontmatter (iteration, max iterations, completion promise), and prints concise status and prompt excerpt.
State library & parsing tests
plugins/ralph-loop/tests/..., plugins/ralph-loop/tests/ralph-loop-specific.bats
Added/extended tests for state.sh helpers (validate_session_id, parse_frontmatter, iteration/max extraction, completion promise), promise extraction edge cases, and ralph-status expectations.
Integration tests (tmux)
plugins/ralph-loop/tests/tmux-test-ralph-loop.bats
New comprehensive tmux-based E2E tests covering SessionStart/Stop behavior, env file overwrite semantics, cancel-ralph cleanup, state transitions, completion detection, max-iteration stopping, and ralph-status outputs.

Sequence Diagram

sequenceDiagram
    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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested labels

released

Poem

🐰 I hopped through loops and parsed the night,
Found session IDs and made things right,
Status shown, stale files undone,
Cleanup done — the loop's at sun,
Hooray! A tidy run. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title focuses on 'add functional promise extraction tests' but the PR includes multiple major changes: bug fix for RALPH_SESSION_ID accumulation, new /ralph-status feature, tmux E2E tests, and documentation updates. Consider a more comprehensive title that reflects the primary changes, such as 'feat(ralph-loop): add ralph-status command and promise extraction tests' or 'fix(ralph-loop): improve session tracking and add status command'.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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
  • Commit unit tests in branch fix/loop

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 | 🟡 Minor

Fix 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_FILE is 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" ]]; then
plugins/ralph-loop/tests/tmux-test-ralph-loop.bats (1)

19-38: wait_for_output helper 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.

Comment on lines +5 to +10
---
# Ralph Loop Status
Execute the status script:
```!
"${CLAUDE_PLUGIN_ROOT}/scripts/ralph-status.sh"
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
---
# 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).

Comment on lines +106 to +157
@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"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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:

  1. Removing skip_if_no_tmux from tests that don't use tmux, or
  2. 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.

baleen37 and others added 5 commits February 6, 2026 22:45
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>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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 `@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: Duplicate SESSION_ENV_FILE declaration.

SESSION_ENV_FILE is 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

Comment on lines +40 to +49
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"
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

@baleen37 baleen37 closed this Feb 8, 2026
auto-merge was automatically disabled February 8, 2026 05:03

Pull request was closed

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.

1 participant