Avoid crash when creating a new workspace

[lawrencecchen]

Summary

• add a regression test for the after-current insertion path when the selected workspace is already last
• simplify TabManager.newTabInsertIndex(snapshot:placementOverride:) to compute pinned count and selected state in one pass without the crash-prone optional/index lookup path

Testing

• xcodebuild build-for-testing -project /Users/lawrence/fun/cmuxterm-hq/worktrees/task-fix-add-workspace-insert-index-crash/GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -only-testing:cmuxTests/WorkspaceCreationPlacementTests/testAddWorkspaceAfterCurrentOverrideAppendsAfterLastSelectedWorkspace -derivedDataPath /tmp/cmux-task-fix-add-workspace-insert-index-crash-tests (passed)
• CMUX_SOCKET_PATH=/tmp/cmux-debug-fix-insert-index-crash.sock /tmp/cmux-cli new-workspace ... with repeated tagged-socket workspace creation and list-workspaces checks (no crash, tagged app stayed responsive)
• xcodebuild test -project /Users/lawrence/fun/cmuxterm-hq/worktrees/task-fix-add-workspace-insert-index-crash/GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -only-testing:cmuxTests/WorkspaceCreationPlacementTests/testAddWorkspaceAfterCurrentOverrideAppendsAfterLastSelectedWorkspace -derivedDataPath /tmp/cmux-task-fix-add-workspace-insert-index-crash-tests (runner crashed before establishing connection, unrelated AppKit/test-host failure)

Issues

• Closes Crash: SIGSEGV in TabManager.newTabInsertIndex when creating new workspace #1966

---

Summary by cubic

Prevent a crash when creating a new workspace with 'after current' placement when the selected workspace is last. Simplifies insert-index logic to append safely and preserve tab order.

• Bug Fixes
  • Compute pinned count, selected index, and selected pinned state in one pass in TabManager.newTabInsertIndex, removing optional/index lookup crash.
  • Clarify regression test testAddWorkspaceAfterCurrentOverrideAppendsAfterLastSelectedWorkspace to assert order is preserved and the new workspace is appended last.

^{Written for commit 4f5a317. Summary will update on new commits.}

Summary by CodeRabbit

• Performance Improvements

  • Improved tab insertion logic for smoother, more responsive tab handling.

• Tests

  • Added a test validating workspace creation and placement when inserting a new workspace after the current one.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Mar 24, 2026 2:17am

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f16d683-675e-469a-b429-30467120ec98

📥 Commits

Reviewing files that changed from the base of the PR and between 1c45915 and 4f5a317.

📒 Files selected for processing (1)

• cmuxTests/WorkspaceUnitTests.swift

✅ Files skipped from review due to trivial changes (1)

• cmuxTests/WorkspaceUnitTests.swift

---

📝 Walkthrough

Walkthrough

Refactored TabManager.newTabInsertIndex to compute pinned count, selected index, and selected-is-pinned in a single pass over a snapshot of tabs; added a unit test verifying workspace insertion with placementOverride: .afterCurrent.

Changes

Cohort / File(s)	Summary
Tab Manager Loop Optimization Sources/TabManager.swift	Rewrote newTabInsertIndex(snapshot:placementOverride:) to iterate snapshot.tabs once using enumerated(); captures snapshot.selectedTabId locally, computes pinnedCount, selectedIndex, and selectedIsPinned during the loop, and uses local tabs.count for total. Replaces prior filter/count/firstIndex(where:) pattern.
Workspace Placement Test cmuxTests/WorkspaceUnitTests.swift	Added WorkspaceCreationPlacementTests.testAddWorkspaceAfterCurrentOverrideAppendsAfterLastSelectedWorkspace to assert that adding a workspace with placementOverride: .afterCurrent inserts the new workspace after the selected workspace and preserves existing tab order.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Fix crash when creating a new workspace #1391 — Directly related changes to newTabInsertIndex and the snapshot approach; likely overlaps with this refactor.
• Keep pinned workspaces above the sidebar pin boundary #1503 — Related edits to tab insertion/reordering logic that also consider pinned tabs and insertion indices.

Poem

🐰 One loop hopped through the slate,
counting pins and picking the mate.
No stale pointers left to bite,
new tabs land just right—
a cheerful hop into the night ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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
Title check	✅ Passed	The title accurately describes the primary fix: preventing a crash when creating a new workspace with 'after current' placement.
Description check	✅ Passed	The description covers the summary of changes and testing approach, though some testing details are verbose and the demo video section is missing.
Linked Issues check	✅ Passed	The PR directly addresses issue #1966 by fixing the crash in TabManager.newTabInsertIndex through single-pass computation and adding a regression test. However, it does not implement any changes related to issue #39 (Release v1.31.0), which lists unrelated feature work (shortcuts, browser zoom, update menu).
Out of Scope Changes check	✅ Passed	All changes are directly scoped to fixing the TabManager.newTabInsertIndex crash and adding a regression test; no unrelated changes detected.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch task-fix-add-workspace-insert-index-crash

---

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.}

[cubic-dev-ai]

No issues found across 2 files

[greptile-apps]

Greptile Summary

This PR fixes a crash that could occur when creating a new workspace with the .afterCurrent placement policy and the currently-selected workspace is already the last tab. The fix simplifies TabManager.newTabInsertIndex(snapshot:placementOverride:) from a two-pass approach (separate filter, firstIndex, and index-based map calls) into a single enumeration loop that accumulates pinnedCount, selectedIndex, and selectedIsPinned together, removing the intermediate subscript lookup that was considered crash-prone.

• Sources/TabManager.swift — replaces three separate functional-style passes with one for (index, tab) in tabs.enumerated() loop; the logic and output are equivalent but the code is simpler and avoids the snapshot.tabs[$0] subscript step.
• cmuxTests/WorkspaceUnitTests.swift — adds testAddWorkspaceAfterCurrentOverrideAppendsAfterLastSelectedWorkspace to cover the specific regression: select the last workspace, call addWorkspace(.afterCurrent), verify the new tab is appended at the end and no existing tab is reordered.
• Commit structure follows the two-commit policy from CLAUDE.md (test added first on commit 202a6997, fix applied second on 1c45915a).
• Minor: _ = manager.tabs[0] on line 339 of the new test is a no-op that could silently crash if TabManager ever starts with zero tabs; see inline comment.

Confidence Score: 5/5

• Safe to merge — the fix is a clean, correct simplification with a properly structured regression test and no logic changes to the insertion policy.
• The refactoring is straightforward: the new single-pass loop produces identical outputs to the original two-pass approach, and the insertionIndex helper it delegates to is unchanged. The regression test directly exercises the crash path. Commit structure correctly follows the CLAUDE.md two-commit policy (failing test first, fix second). The only open item is a minor P2 style issue in the test body (_ = manager.tabs[0]), which does not affect correctness.
• No files require special attention.

Important Files Changed

Filename	Overview
Sources/TabManager.swift	Refactors newTabInsertIndex from a two-pass approach (filter+firstIndex+map) to a single enumeration loop that captures pinnedCount, selectedIndex, and selectedIsPinned together, eliminating the intermediate index-based lookup on snapshot.tabs.
cmuxTests/WorkspaceUnitTests.swift	Adds a regression test (testAddWorkspaceAfterCurrentOverrideAppendsAfterLastSelectedWorkspace) covering the crash path where the selected workspace is already last and .afterCurrent placement should append at the end.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[newTabInsertIndex called] --> B[Get placement & tabs]
    B --> C[Single-pass loop over tabs]
    C --> D{tab.isPinned?}
    D -- Yes --> E[pinnedCount += 1]
    D -- No --> F[no-op]
    E --> G{selectedIndex == nil\n& tab.id == selectedTabId?}
    F --> G
    G -- Yes --> H[selectedIndex = index\nselectedIsPinned = tab.isPinned]
    G -- No --> I[continue]
    H --> I
    I --> C
    C -- done --> J[insertionIndex placement]
    J --> K{placement}
    K -- .top --> L[return pinnedCount]
    K -- .end --> M[return totalCount]
    K -- .afterCurrent --> N{selectedIndex set?}
    N -- No --> O[return totalCount]
    N -- Yes --> P{selectedIsPinned?}
    P -- Yes --> Q[return pinnedCount]
    P -- No --> R["return min(selectedIndex+1, totalCount)"]

Loading

_{Reviews (1): Last reviewed commit: "Avoid crash in workspace insert index" | Re-trigger Greptile}