fix(monitor): detect tmux base-index after starting the server (+ macOS bash 3.2 test portability)

[JamesFincher]

What

Two related macOS-support fixes — both verified on Darwin (bash 3.2.57) and unchanged on Linux CI.

1. fix(monitor): detect tmux base-index AFTER starting the server

setup_tmux_session() detected base-index/pane-base-index before creating the tmux session. tmux show-options does not auto-start a server, so on the first ralph --monitor run (no server yet) detection silently errored and defaulted to 0. With a base-index 1 / pane-base-index 1 config this made every window.pane target off-by-one (e.g. pane 1.0, window 0), so the ralph --live loop command was sent to a nonexistent pane — tmux opened to empty idle panes and the loop never started.

Fix: detect after tmux new-session starts the server (which sources the user's config), so detection is reliable even on first run. Mirrors the reorder in the test's inline setup_tmux_session and adds a regression test asserting new-session precedes show-options in the call log.

2. test: make the bats suite portable to macOS bash 3.2

15 tests passed on Linux CI but failed on macOS. Root causes:

• source <(process-substitution) does not define functions on bash 3.2 (they silently stay undefined → "command not found"). Broke 11 test_monitor.bats + 2 test_ralph_enable.bats tests. Fix: write to a temp file and source the file.
• head -n -1 is GNU-only (BSD head rejects it: "illegal line count"). Fix: sed '$d'.
• test_log_rotation BSD-stat stub delegated to GNU stat -c%s (fails on BSD). Fix: portable wc -c.
• test_notifications notify-send branch is unreachable on macOS (osascript always present; /bin symlinks to /usr/bin). Skip on Darwin — still runs on Linux CI.

Verification

• npm test: 1275/1275 green on macOS (was 1260/1274); tmux integration suite 32/32.
• Reproduced the original failure (no server → detection 0 → empty panes) and confirmed the fix end-to-end on a real project with a base-index 1 config on a first --monitor run — the Ralph loop now starts correctly and runs Claude iterations.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Fixed pane targeting issue in tmux sessions that occurred during the first ralph --monitor run when no tmux server exists.

• Tests

  • Improved test compatibility for macOS bash environments and process substitution handling.

[coderabbitai]

Walkthrough

setup_tmux_session in ralph_loop.sh is reordered so tmux new-session -d runs before querying base-index/pane-base-index. Tests in test_tmux_integration.bats mirror this and add a regression test. Multiple test files replace source <(...) process substitution with temp-file sourcing for macOS bash 3.2 compatibility, and minor platform guards are added.

Changes

tmux base-index ordering fix

Layer / File(s)	Summary
setup_tmux_session: detect indices after new-session ralph_loop.sh	Reorders the function body so tmux new-session -d starts the server before get_tmux_base_index/get_tmux_pane_base_index are called and pane0/pane1/pane2 are computed.
Mirror reorder in integration test helper + regression test tests/integration/test_tmux_integration.bats	Applies the same ordering to the test helper's setup_tmux_session, and adds a new test asserting the tmux mock call log records new-session -d before show-options.

macOS bash 3.2 test portability

Layer / File(s)	Summary
Replace process substitution in monitor tests tests/integration/test_monitor.bats	setup() and the EXIT-trap test now write a stripped copy of ralph_monitor.sh to a temp file and source that file, replacing source <(head -n -1 | grep -v ...) process substitution.
Replace process substitution in phase_verification tests tests/unit/test_ralph_enable.bats	Both phase_verification extraction tests write the function body to .pv.sh and source it, replacing the prior source <(sed ...) approach.
Fix stat stub and add notify-send macOS skip tests/unit/test_log_rotation.bats, tests/unit/test_notifications.bats	The BSD stat fallback stub uses wc -c instead of delegating to real stat; the Linux notify-send test skips on macOS where osascript is always present.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• frankbria/ralph-claude-code#245: Introduced setup_tmux_session logic in the tmux integration tests that this PR directly reorders for the base-index/pane-base-index detection timing fix.
• frankbria/ralph-claude-code#259: Also modifies setup_tmux_session in ralph_loop.sh and tmux integration tests around how base-index/pane-base-index are used when targeting panes.
• frankbria/ralph-claude-code#246: Modifies the same test_monitor.bats file for sourcing ralph_monitor.sh without running main and asserting the EXIT trap calls cleanup.

Poem

🐇 Hop, hop — the server must wake before we peek,
No index-guessing while tmux is still asleep!
A temp file here, a sed and source there,
macOS bash 3.2 no longer pulls its hair.
The call log sings: new-session first, then show,
A tidy rabbit checks the order high and low! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main changes: fixing tmux base-index detection timing and addressing macOS bash 3.2 test portability issues.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/unit/test_log_rotation.bats`:
- Around line 84-86: The BSD stat stub branch prints the byte count using wc -c
but then falls through to the real_stat exec call on the following line,
appending extra output. Add a return statement immediately after the wc -c line
to terminate that branch and prevent fall-through to the exec statement,
ensuring the stub outputs only the byte count without additional output from the
real stat command.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3473e6ba-83e8-4c43-821c-aa845c7df9d5

📥 Commits

Reviewing files that changed from the base of the PR and between 86ad619 and 5919e0c.

📒 Files selected for processing (6)

• ralph_loop.sh
• tests/integration/test_monitor.bats
• tests/integration/test_tmux_integration.bats
• tests/unit/test_log_rotation.bats
• tests/unit/test_notifications.bats
• tests/unit/test_ralph_enable.bats

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Terminate the BSD stat stub branch after printing size.

On Line 86, the stub prints byte count via wc -c, but then falls through to Line 88 (exec "$real_stat" "$@"). That can append extra output and make the fallback simulation inaccurate.

Proposed fix

 if [[ "$1" == "-f%z" ]]; then
   shift
   # Portable byte count — real stat's -c%s is GNU-only and fails on BSD/macOS,
   # which silently defeated this fallback simulation there. wc -c works everywhere.
   wc -c < "$1"
+  exit 0
 fi

📝 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

	# Portable byte count — real stat's -c%s is GNU-only and fails on BSD/macOS,
	# which silently defeated this fallback simulation there. wc -c works everywhere.
	wc -c < "\$1"
	if [[ "$1" == "-f%z" ]]; then
	shift
	# Portable byte count — real stat's -c%s is GNU-only and fails on BSD/macOS,
	# which silently defeated this fallback simulation there. wc -c works everywhere.
	wc -c < "$1"
	exit 0
	fi

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/test_log_rotation.bats` around lines 84 - 86, The BSD stat stub
branch prints the byte count using wc -c but then falls through to the real_stat
exec call on the following line, appending extra output. Add a return statement
immediately after the wc -c line to terminate that branch and prevent
fall-through to the exec statement, ensuring the stub outputs only the byte
count without additional output from the real stat command.

[frankbria]

Cross-family review — opencode (zai/glm-5.2)

Review Findings

---

1. Major — tests/unit/test_log_rotation.bats:86

Description: The wc -c < "$1" replacement for the BSD stat stub's -f%z branch lacks an exit 0 (or exec), causing fall-through to the unconditional exec "$real_stat" "$@" on the next line — which runs the real stat with no format flag, appending multi-line unformatted output after the byte count.

Failure scenario: log_utils.sh:21 captures the stub's full stdout into file_size via file_size=$(stat -f%z "$log_file" 2>/dev/null || echo "0"). The result is "11534336\n File: /path/ralph.log\n Size: 11534336 ..." — not a clean integer. The subsequent [[ "$file_size" -lt "$max_size" ]] in rotate_logs() errors with "integer expression expected" and returns 1 (false), so && return 0 never fires and rotation proceeds unconditionally regardless of file size. The current 11MB test passes by accident (rotation should happen), but any future test with a sub-10MB file using this stub path would see incorrect rotation. The old code used exec "$real_stat" -c%s "$@" (process replacement — clean exit, no fall-through), so this is also a behavioral regression on Linux. The acceptance criterion explicitly requires "BSD stat stub correctness" — the stub does not produce a correct byte count. Fix: add exit 0 after wc -c < "$1".

---

2. Suggestion — tests/integration/test_tmux_integration.bats:369-389

Description: TEST 3a guards the new-session → show-options call ordering, but the mock tmux always succeeds for show-options regardless of server state, so it does not reproduce the actual first-run failure mode (tmux show-options fails when no server exists).

Failure scenario: A future refactor could re-break detection in a way the ordering check doesn't catch — e.g., adding a retry wrapper around show-options that silently succeeds even before the server starts, or moving detection into a subshell that masks the failure. Consider having the mock fail show-options before the first new-session and succeed after, which would directly reproduce the bug class rather than just guarding the fix's surface symptom.

---

3. Nitpick — tests/unit/test_log_rotation.bats:86

Description: wc -c < "$1" handles only a single file argument, whereas stat -f%z accepts multiple.

Failure scenario: Harmless today since log_utils.sh always calls stat with exactly one file, but if a future caller passes multiple files the stub would silently report only the first file's size.

---

Additional observations (no action required)

• ralph_loop.sh:485-510 — Core fix is correct: all base_win/base_pane/paneN uses (lines 516+) are after their post-new-session assignment (lines 506-510). No use-before-assignment. The comment explanation is accurate. Acceptance criterion 1 is met.
• test_monitor.bats:30-33 — sed '$d' is a correct portable replacement for head -n -1; temp-file→source correctly avoids the bash 3.2 process-substitution function-definition bug. No behavior change on Linux.
• test_ralph_enable.bats:297-299, 326-328 — Temp-file→source conversions are correct and portable.
• test_notifications.bats:158 — Skipping the notify-send branch on macOS is correct (osascript is always present, making the branch unreachable). No Linux behavior change.

---

Verdict: REQUEST_CHANGES

[frankbria]

Final Triage Summary (PR #330)

Cutoff: 2026-07-10T18:21:42Z — no new findings since cutoff.

Fixed

• [CodeRabbit + opencode/GLM cross-family review, Major] BSD stat stub fell through to exec real stat after wc -c, polluting the captured size → fixed in e92a8e4 (added exit 0); mutation-verified (breaking the -f%z fallback in log_utils.sh now fails the test).

Skipped (with justification)

• [opencode/GLM, Suggestion] Make the tmux mock fail show-options pre-server to reproduce the bug class — skipped: the new-session→show-options ordering assertion already guards the regression; reworking the shared mock's state machine is out of scope for this PR.
• [opencode/GLM, Nitpick] Stub handles only one file argument for -f%z — skipped: log_utils.sh always calls stat with exactly one file.

CI note

• review / review (GLM Review workflow) fails on fork PRs by design: Secret source: None — fork runs receive no repo secrets, so it cannot authenticate to z.ai. Not a defect of this PR; its purpose is covered by the manually posted cross-family review above.

Demo

• Both acceptance criteria verified with outcome evidence (real tmux server on isolated socket with base-index 1: pre-server detection reproduces the 0/0 bug and send-keys to :0.0 fails; PR order detects 1/1 and the loop command demonstrably executes in the correct pane via capture-pane. bash 3.2.57 container reproduces the source <(...) function-definition failure; temp-file pattern works; sed '$d' byte-identical to head -n -1 on Linux; all 5 touched test files pass, 80/80).