Fix Cmd+N crash from stale workspace creation snapshots

[austinywang]

Summary

• add regression coverage for workspace creation when a captured workspace is closed after the snapshot is taken
• stop WorkspaceCreationSnapshot from retaining live Workspace objects and other teardown-sensitive state
• insert new workspaces into the current live tabs array so post-snapshot closes/reorders are preserved instead of resurrecting stale workspaces

Context

The #2017 fix stabilized tabs/selectedTabId reads during workspace creation, but WorkspaceCreationSnapshot still retained live Workspace references and addWorkspace rebuilt tabs from the snapshotted array. That left a stale-object path open: if a workspace was closed or torn down after snapshot capture but before insertion, Cmd+N could reintroduce dead workspace objects and eventually trip the PAC failure reported in #2131.

This change snapshots only value state needed for placement/config inheritance and computes insertion against the live workspace list while preserving the pre-creation selection intent.

Closes #2131

Validation

• ./scripts/reload.sh --tag issue2131-cmdn-crash --launch
• Local unit/UI tests not run per repo policy

---

Summary by cubic

Fixes a Cmd+N crash by snapshotting only value state and inserting the new workspace into the live tabs array so closed/reordered tabs aren’t resurrected. Closes #2131.

• Bug Fixes
  • Replaced live Workspace references with WorkspaceCreationTabSnapshot (id, isPinned).
  • Snapshot now stores selectedTabId, selectedTabWasPinned, preferredWorkingDirectory, and inheritedTerminalConfig.
  • Compute insertion index against current tabs; insert into the live array to preserve post-snapshot closes/reorders and selection intent.
  • Handle .afterCurrent when the selected tab closes after snapshot (fallback based on pin state).
  • Added didCaptureWorkspaceCreationSnapshot() test seam and regression tests to ensure closed tabs aren’t reinserted and selection remains correct.

^{Written for commit 17e8bb1. Summary will update on new commits.}

Summary by CodeRabbit

• Refactor

  • Improved workspace creation snapshot handling and tab placement logic to better preserve manual changes made during workspace creation.

• Tests

  • Added test coverage for workspace insertion behavior when workspaces are closed or selection changes during creation.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Mar 25, 2026 9:39am

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Refactored workspace-creation snapshotting by replacing a full tab array with lightweight WorkspaceCreationTabSnapshot objects containing only IDs and pin status, while extending the snapshot with selected-tab-pin-state and inherited configuration. Updated insertion logic to compute placement indices from snapshot pin/grouping data, apply mutations to live tabs rather than snapshot copies, and added a test seam for capturing snapshot events.

Changes

Cohort / File(s)	Summary
Workspace Creation Snapshot Refactoring Sources/TabManager.swift	Simplified WorkspaceCreationSnapshot structure from capturing full [Workspace] tabs to lightweight WorkspaceCreationTabSnapshot with ID and pin status only; extended snapshot with selectedTabWasPinned, preferredWorkingDirectory, and inheritedTerminalConfig. Updated addWorkspace to compute insertion indices using snapshot pin/grouping data and apply insertions to live tabs array (preserving post-snapshot mutations). Changed configuration inheritance to accept Workspace? directly. Added didCaptureWorkspaceCreationSnapshot() test seam method.
Snapshot-Mutation Test Coverage cmuxTests/WorkspaceUnitTests.swift	Extended SnapshotMutatingTabManager with afterCaptureWorkspaceCreationSnapshot callback and override of new seam method. Added two test cases verifying .afterCurrent insertion behavior when workspace selection changes or workspaces close during/after snapshot capture, ensuring closed workspaces are not reinserted and correct workspace becomes selected.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Avoid crash when creating a new workspace #2023: Directly modifies workspace-creation insertion logic and tests for .afterCurrent placement behavior with snapshot-derived data.
• Add regression coverage for Cmd+N workspace creation crash #2127: Adds TabManager test seam for selection mutation during workspace creation and tests .afterCurrent insertion semantics.
• Fix NIGHTLY crash after cmux ssh cmux-macmini #2099: Refactors addWorkspace and insertion-index computation to use pre-creation snapshot data instead of live tab reads.

Poem

🐰 A snapshot captures what was true,
Before the tabs all change their view,
Now insertions use their data slight—
Just IDs, pins, and what was right!
Post-mutations leave no trace behind,
A cleaner placement logic, well-designed! 🎯

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description includes a clear Summary section explaining what changed and why, but lacks Testing section details (no manual testing results documented) and the Demo Video section is empty despite potential UI behavior changes.	Add a Testing section documenting how the changes were tested locally and what was verified, and clarify whether a demo video is needed for the behavior changes made.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the main fix: resolving a Cmd+N crash caused by stale workspace creation snapshots, which aligns perfectly with the PR's core objective.

_{✏️ 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 issue-2131-cmd-n-crash-regression

---

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

[greptile-apps]

Greptile Summary

This PR fixes a Cmd+N crash (#2131) where WorkspaceCreationSnapshot retained live Workspace references, allowing a closed or torn-down workspace to be reinserted into the tab list if it was closed between snapshot capture and the actual insertion. The fix has two parts: (1) WorkspaceCreationSnapshot now stores only value-type state (id, isPinned, computed working directory, and terminal config) via a new WorkspaceCreationTabSnapshot helper, eliminating all references to live objects; (2) addWorkspace now inserts into the current live tabs array rather than rebuilding from the snapshotted array, so post-snapshot closes and reorders are automatically preserved.

Key changes:

• WorkspaceCreationTabSnapshot replaces [Workspace] in the snapshot, storing only id and isPinned at capture time.
• newTabInsertIndex now derives pinnedCount and selectedIndex from the live tabs while reading selectedTabId/selectedTabWasPinned from the snapshot to preserve pre-creation selection intent — including a correct fallback when the snapshotted selected tab has since been closed.
• preferredWorkingDirectory and inheritedTerminalConfig are now eagerly resolved into the snapshot, removing the need for snapshot.selectedWorkspace live-object access later.
• A didCaptureWorkspaceCreationSnapshot() test seam is added to allow unit tests to inject post-snapshot mutations.
• Two regression tests cover the previously crashing scenarios: a non-selected workspace closed after snapshot, and the selected workspace itself closing after snapshot. The commits follow the required two-commit regression test structure (failing test first, fix second).

Confidence Score: 5/5

• Safe to merge — root cause is correctly addressed, regression coverage is solid, and no new correctness issues are introduced.
• The fix precisely targets the stale-object path by removing all live Workspace references from the snapshot and switching insertion to operate on the live array. The placement logic correctly mixes live tab state (for current pin counts and indices) with snapshot state (for pre-creation selection intent), including a sound fallback when the snapshotted selected tab no longer exists. Two regression tests with the required two-commit structure validate both close-race scenarios. The one flagged item (nextTabCount using snapshot count for title/breadcrumb) is cosmetic and does not affect correctness or the crash fix.
• No files require special attention.

Important Files Changed

Filename	Overview
Sources/TabManager.swift	Core fix: replaces live-object WorkspaceCreationSnapshot with a value-only snapshot struct, switches insertion to operate on the live tabs array, and refactors newTabInsertIndex to compute pinned count and selection index against live tabs while preserving pre-capture selection intent from the snapshot. Logic is sound across all placement modes.
cmuxTests/WorkspaceUnitTests.swift	Adds two regression tests covering the snapshot-close race: one for a non-selected workspace closed after snapshot, and one for the selected workspace itself closing after snapshot. Both use the new didCaptureWorkspaceCreationSnapshot seam and correctly assert final tab order, absence of the closed workspace, and selected tab identity.

Sequence Diagram

sequenceDiagram
    participant User as User (Cmd+N)
    participant TM as TabManager
    participant Snap as WorkspaceCreationSnapshot
    participant Live as Live tabs[]

    User->>TM: addWorkspace()
    TM->>Snap: workspaceCreationSnapshot()<br/>(copies IDs, isPinned, workingDir, config)
    Note over Snap: No live Workspace refs retained
    TM->>TM: didCaptureWorkspaceCreationSnapshot()
    Note over TM,Live: Post-snapshot window:<br/>workspace may be closed/reordered here

    TM->>Live: newTabInsertIndex(snapshot, liveTabs: tabs)
    Note over Live: pinnedCount, selectedIndex<br/>computed from current live tabs
    Note over Snap: selectedTabId, selectedTabWasPinned<br/>read from snapshot (pre-creation intent)

    TM->>TM: makeWorkspaceForCreation(...)

    TM->>Live: var updatedTabs = tabs (live)
    TM->>Live: updatedTabs.insert(newWorkspace, at: insertIndex)
    TM->>TM: tabs = updatedTabs
    Note over Live: Stale closed workspaces<br/>never reintroduced ✓

Loading

_{Reviews (1): Last reviewed commit: "Fix Cmd+N crash from stale workspace cre..." | Re-trigger Greptile}

[greptile-apps]

Stale tab count used for title and Sentry breadcrumb

nextTabCount is derived from snapshot.tabs.count, which reflects the tab count at snapshot capture time. In the exact race this PR is fixing — where a workspace is closed between snapshot and insertion — nextTabCount will be one higher than the actual live count, resulting in a workspace titled "Terminal N" while only N-1 tabs exist. The Sentry breadcrumb on the next line also reports the inflated count.

Since insertion now operates on tabs (live), the title and breadcrumb could use the live count at that point for accuracy:

Suggested change

	let nextTabCount = snapshot.tabs.count + 1
	let nextTabCount = tabs.count + 1

This is cosmetic-only, but it would prevent misleading Sentry breadcrumbs in the post-snapshot-close race.

[cubic-dev-ai]

No issues found across 2 files