fix(spawn): target session not active window when creating crewmate windows#74
Open
hcho22 wants to merge 4 commits into
Open
fix(spawn): target session not active window when creating crewmate windows#74hcho22 wants to merge 4 commits into
hcho22 wants to merge 4 commits into
Conversation
new-window -t "$SES" resolved to the session's active window and tried to
create at that occupied index, failing "create window failed: index N in use"
when low window indices were occupied. The trailing colon ("$SES:") forces
session-only targeting so tmux picks the next free index.
…ior tests in README
… window-index test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
tmux new-windowtarget inbin/fm-spawn.shfrom the bare session name (-t "$SES") to the session-only form (-t "$SES:"), so tmux places each crewmate window at the next free index instead of resolving to the session's active window and failingcreate window failed: index N in usewhen low indices are already occupied.tests/fm-spawn-window-index.test.sh, a regression test that runs on a private tmux socket and asserts the source carries the session-target form, the fixed command lands the window at the next free index, and the occupied-index target reproduces theindex N in usefailure the fix avoids.README.md.Risk Assessment
✅ Low: A minimal, correct one-line tmux-targeting fix that is fully decoupled from downstream logic (the window is referenced by name, not index), backed by isolated private-socket regression tests and accurate README documentation.
Testing
Ran the new
tests/fm-spawn-window-index.test.shregression suite (passes) plus the siblingfm-spawn-batch.test.sh(passes), then reproduced the end-user-facing failure on an isolated private tmux socket: the pre-fix bare-session target resolves to the occupied active window and aborts with the bug report's exactcreate window failed: index 0 in use, while the fixed$SES:session target succeeds and places the crewmate window at the next free index (3). This is a CLI/tmux behavior change with no UI surface, so evidence is captured as CLI transcripts rather than screenshots. Note: on this host's tmux 3.6b the bare form happened to also pick a free index, confirming the crash is version-dependent as the test header states; the decisive evidence is the reproduced occupied-index failure that the fix avoids. Overall result: fix verified working, no findings.Evidence: Pre-fix vs post-fix new-window behavior on an occupied session
Evidence: Reproduced 'index 0 in use' failure and the fix avoiding it
=== The exact failure the fix avoids (bug report's 'index N in use') === $ tmux new-window -d -t "BUG:0" -n fm-crew-test exit status: 1 error: create window failed: index 0 in use === The fix (-t "$SES:") on the same occupied session: succeeds at next free index === $ tmux new-window -d -t "BUG:" -n fm-crew-test exit status: 0 error: <none> fm-crew-test landed at index: 3Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/fm-spawn-window-index.test.sh:84- test_occupied_index_target_is_rejected asserts on the exact tmux error substring 'index 0 in use' (tests/fm-spawn-window-index.test.sh:84). This couples the regression test to tmux's wording for an occupied-index failure rather than to firstmate's own code. The substring has been stable across tmux versions for years, so the risk is low, but a future tmux build that rephrases the message would break this test even though the production fix remains correct. This is a deliberate, documented choice (it mirrors the bug-report error), so no action is required - noting it as a maintenance awareness point only.✅ **Test** - passed
✅ No issues found.
bash tests/fm-spawn-window-index.test.sh- all 3 assertions pass (source uses$SES:; session-target picks next free index; occupied-index target rejected)bash tests/fm-spawn-batch.test.sh- all pass (sibling fm-spawn test documented alongside this change)Manual repro on a private tmux socket (-L, $TMUX unset): built a session with indices 0,1,2 occupied and active window on index 0, then rantmux new-window -d -t "BUG:0"(the active window the bare form resolves to) ->create window failed: index 0 in use, andtmux new-window -d -t "BUG:"(the fix) -> exit 0, window landed at index 3Verified worktree left clean and removed stray private tmux sockets/servers created during testing✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.