Skip to content

fix(tmux): respect pane-base-index in setup_tmux_session#259

Open
edlsh wants to merge 1 commit intofrankbria:mainfrom
edlsh:fix/tmux-pane-base-index
Open

fix(tmux): respect pane-base-index in setup_tmux_session#259
edlsh wants to merge 1 commit intofrankbria:mainfrom
edlsh:fix/tmux-pane-base-index

Conversation

@edlsh
Copy link
Copy Markdown

@edlsh edlsh commented Apr 17, 2026

Summary

Ralph's --monitor mode silently fails to start the loop when the user's ~/.tmux.conf sets setw -g pane-base-index 1. The tmux session opens with two empty shell prompts and a stray tail -f in a third pane — the Ralph loop itself never runs.

Root cause: setup_tmux_session handles base-index (for windows) via get_tmux_base_index but hardcodes pane targets as .0, .1, .2. With pane-base-index 1, tmux numbers panes from 1 so:

  • send-keys -t session:0.0 "$ralph_cmd" targets a non-existent pane and silently drops the input → the Ralph loop never starts
  • send-keys -t session:0.1 "tail -f live.log" lands in the actual left-most pane (which was supposed to be the loop)
  • send-keys -t session:0.2 "ralph-monitor" targets a pane that may or may not exist depending on split order

This is common because many popular dotfiles / Oh My Zsh setups ship with setw -g pane-base-index 1 enabled.

Repro

# In ~/.tmux.conf:
set -g base-index 1
setw -g pane-base-index 1

# Then:
cd my-ralph-project
ralph --monitor
# tmux opens with empty panes, loop never runs

Fix

Add get_tmux_pane_base_index mirroring get_tmux_base_index, then compute pane indices as base_pane + {0,1,2} throughout setup_tmux_session. Default behavior (pane-base-index 0) is unchanged.

Tests

  • Updated the inline mirror of setup_tmux_session in test_tmux_integration.bats (per the file's "keep in sync" convention)
  • Parameterised the tmux show-options mock to return per-option values via MOCK_TMUX_BASE_INDEX / MOCK_TMUX_PANE_BASE_INDEX — previously it always returned 0
  • Added 3 regression tests:
    • setup_tmux_session respects pane-base-index 1 for all pane targets — verifies .1/.2/.3 are used and .0 is never touched
    • setup_tmux_session respects both base-index and pane-base-index set to 1 — double non-zero case
    • get_tmux_pane_base_index returns 0 as default — mirrors existing get_tmux_base_index default test

All 680 tests pass (16 original tmux + 3 new + 1 renumbered concurrent test, plus the rest of the suite).

Checklist

  • Bug reproduces on main with a minimal .tmux.conf snippet
  • Fix preserves existing behavior for pane-base-index 0 (all 16 original tmux tests pass unchanged)
  • New regression tests added
  • Full test suite (npm test) passes — 680/680
  • Syntax check (bash -n ralph_loop.sh) passes

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Ralph Loop now correctly respects tmux's pane-base-index configuration instead of assuming fixed pane positions, improving compatibility with custom tmux setups.
  • Tests

    • Added tests to verify pane indexing behavior across different tmux configurations.

When a user's ~/.tmux.conf sets `setw -g pane-base-index 1` (common in
popular dotfiles and Oh My Zsh setups), tmux panes are numbered starting
at 1, not 0. Ralph previously handled `base-index` (windows) but
hardcoded pane targets as .0/.1/.2. With pane-base-index=1:

- send-keys -t session:0.0 targets a non-existent pane (silently fails)
- The Ralph loop never starts in the left pane
- The live-log tail and ralph-monitor land in the wrong panes

Visible symptom: running `ralph --monitor` opens a tmux session with
two empty shell prompts and a stray `tail -f` in a third pane. The
loop never runs.

Fix: add `get_tmux_pane_base_index` mirroring `get_tmux_base_index`,
then compute pane indices as `base_pane + {0,1,2}` throughout
`setup_tmux_session`. Default behavior (pane-base-index=0) is
unchanged.

Tests (test_tmux_integration.bats):
- Updated inline mirror of setup_tmux_session to match
- Parameterised the tmux show-options mock to return per-option values
  via MOCK_TMUX_BASE_INDEX / MOCK_TMUX_PANE_BASE_INDEX
- Added 3 regression tests:
  * pane-base-index 1 targets panes .1/.2/.3 (no .0 touches)
  * base-index 1 + pane-base-index 1 targets 1.1/1.2/1.3
  * get_tmux_pane_base_index defaults to 0

All 680 tests pass.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 17, 2026

Walkthrough

This PR updates tmux pane targeting to respect the pane-base-index configuration setting instead of using hardcoded indices. A new helper function queries tmux for the pane base index value, and setup_tmux_session() now computes dynamic pane targets accordingly. Tests are expanded to verify the functionality across different configurations.

Changes

Cohort / File(s) Summary
Tmux pane-base-index support
ralph_loop.sh
Added get_tmux_pane_base_index() function to query tmux's pane-base-index setting with a default of 0. Updated setup_tmux_session() to compute dynamic pane targets (pane0, pane1, pane2) using this value instead of hardcoded pane indices (.0, .1, .2), affecting pane splitting, command delivery, pane selection, and title assignments.
Test coverage expansion
tests/integration/test_tmux_integration.bats
Expanded tmux mock to support tmux show-options queries for both base-index and pane-base-index via configurable environment variables. Added regression tests verifying correct pane targeting when pane-base-index is non-zero, edge cases with combined base indices, and default fallback behavior. Updated test numbering following new test additions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through panes so keen,
No hardcoded numbers to be seen!
Dynamic indices, tmux knows best,
Base-index queries pass the test!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main fix: making setup_tmux_session respect the pane-base-index configuration option.
Docstring Coverage ✅ Passed Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%.

✏️ 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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
ralph_loop.sh (1)

293-302: Pane target derivation looks correct.

Computing pane0/pane1/pane2 from base_pane + {0,1,2} is the right fix for the pane-base-index=1 case, and preserves existing behavior when it's 0.

Optional nit: local pane0=$((base_pane + 0)) — the + 0 is a no-op; local pane0=$base_pane would be equivalent. The current form does keep visual symmetry with pane1/pane2, so feel free to leave as-is.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ralph_loop.sh` around lines 293 - 302, Replace the no-op arithmetic for pane0
with a simple assignment to avoid unnecessary expansion: change the declaration
local pane0=$((base_pane + 0)) to local pane0=$base_pane while keeping pane1 and
pane2 as local pane1=$((base_pane + 1)) and local pane2=$((base_pane + 2)); this
touches the pane target derivation that uses get_tmux_pane_base_index and
preserves behavior for pane0/pane1/pane2.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ralph_loop.sh`:
- Around line 293-302: Replace the no-op arithmetic for pane0 with a simple
assignment to avoid unnecessary expansion: change the declaration local
pane0=$((base_pane + 0)) to local pane0=$base_pane while keeping pane1 and pane2
as local pane1=$((base_pane + 1)) and local pane2=$((base_pane + 2)); this
touches the pane target derivation that uses get_tmux_pane_base_index and
preserves behavior for pane0/pane1/pane2.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2346a55a-57f4-4777-ab70-95533e2f4981

📥 Commits

Reviewing files that changed from the base of the PR and between 24b8969 and 6f05e96.

📒 Files selected for processing (2)
  • ralph_loop.sh
  • tests/integration/test_tmux_integration.bats

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