Repository navigation
Reopen closed workspaces with sticky repo identity - #8841
Conversation
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds reopening of closed workspaces through history, command palette, menu, and keyboard shortcuts. Persists workspace title/color customizations by normalized directory across creation and session restoration, with capacity policies and recovery tests. ChangesWorkspace recovery and customization
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR adds a "Reopen Closed Workspace" action (⌘⇧T default) surfaced in the History menu, command palette, and settings. It introduces an independent 100-record workspace-only capacity policy for the shared closed-item history store, and a new
Confidence Score: 4/5The core reopen/restore/sticky-identity flows are well-structured and the previous issues were addressed. One existing concern about the remote tmux workspace label not being applied remains unresolved in the diff. The Files Needing Attention: Sources/RemoteTmuxController.swift — the sessionName title passed to addWorkspace is never applied to the workspace when shouldApplyWorkspaceDirectoryCustomization is false and no compensating setCustomTitle call follows. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (⌘⇧T)
participant AD as AppDelegate
participant CIHS as ClosedItemHistoryStore
participant TM as TabManager
participant WS as Workspace
participant WDCS as WorkspaceDirectoryCustomizationStore
U->>AD: reopenMostRecentlyClosedWorkspace()
AD->>CIHS: restoreMostRecentlyClosedWorkspace()
CIHS-->>CIHS: restoreFirstRestorable(matching: .workspace)
CIHS->>AD: workspaceEntry (restore closure)
AD->>TM: restoreClosedWorkspace(workspaceEntry)
TM->>TM: addWorkspace(shouldApplyWorkspaceDirectoryCustomization: false)
TM->>WS: restoreSessionSnapshot(snapshot)
WS-->>TM: panelIds restored
TM->>WDCS: customizations(forDirectories:[directoryKey])
WDCS-->>TM: stored or nil
alt stored record exists
TM->>WS: setCustomTitle(stored.customTitle)
TM->>WS: setCustomColor(stored.customColor)
else no record + snapshot has user-owned title/color
TM->>WDCS: updateCustomization(for: directoryKey)
end
TM-->>AD: "restored = true"
AD->>AD: focusMainWindow(windowId)
Reviews (11): Last reviewed commit: "Construct customization store on main ac..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| static let defaultCapacity = 100 | ||
| static let shared = ClosedItemHistoryStore( | ||
| capacity: nil, | ||
| capacity: defaultCapacity, | ||
| fileURL: defaultHistoryFileURL() | ||
| ) |
There was a problem hiding this comment.
ClosedItemHistoryStore.shared previously had capacity: nil (unbounded). Bounding it to 100 affects the existing "Reopen Last Closed" action for browser panels and tabs — items beyond the 100-item budget are permanently dropped. A user who closes many browser panels in a session could find their older "Reopen Last Closed" history gone, with no warning. The workspace-only filter inside reopenMostRecentlyClosedWorkspace makes the cap feel even tighter: every closed panel or tab eats into the same 100-item budget.
There was a problem hiding this comment.
Fixed in bc8f3bc. The shared store now has an independent 100-record workspace capacity while the pre-existing panel/window history remains unbounded. Coverage pushes a panel plus three workspaces through a two-workspace bound and verifies the panel survives.
— Claude Code
| if historyStore === ClosedItemHistoryStore.shared, | ||
| let appDelegate = AppDelegate.shared { | ||
| return appDelegate.reopenMostRecentlyClosedWorkspace(preferredTabManager: self) | ||
| } | ||
|
|
||
| return historyStore.restoreFirstRestorable( | ||
| newerThan: nil, | ||
| matching: { entry in | ||
| if case .workspace = entry { | ||
| return true | ||
| } | ||
| return false | ||
| }, | ||
| using: { entry in | ||
| guard case .workspace(let workspaceEntry) = entry else { return false } | ||
| return self.restoreClosedWorkspace(workspaceEntry) | ||
| } | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
TabManager.reopenMostRecentlyClosedWorkspace delegates to AppDelegate (which includes window focus) when historyStore === .shared, but falls through to a local implementation — without focus — when an injected non-shared store is used. Any test driving the fallback path validates restoration but never exercises the window-focus logic, so coverage of the production flow is incomplete. The workspace-matching lambda is also duplicated between this fallback and AppDelegate.reopenMostRecentlyClosedWorkspace. Extracting a shared helper for the predicate and moving the focus responsibility to the call site (rather than the store-identity check) would make both paths exercise the same code.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Fixed in bc8f3bc. AppDelegate and TabManager now share one workspace-only history helper, TabManager no longer branches on singleton identity, and behavior coverage exercises active-destination routing while proving the source window remains unchanged.
— Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swift`:
- Around line 194-198: Move reopenClosedWorkspace out of the switch branch
returning .navigation and into the preceding .workspace branch alongside the
other workspace actions, so its shortcut is categorized as .workspace while all
other cases retain their existing categories.
In `@Sources/ClosedWorkspaceHistory.swift`:
- Around line 20-23: Update the tab-manager selection in the workspace
restoration flow to prioritize preferredTabManager before the
workspaceEntry.windowId lookup. Preserve self.tabManager as the final fallback
so history-menu and shortcut callers restore additively in the selected
destination window.
In `@web/data/cmux-shortcuts.ts`:
- Line 252: Update the reopenClosedWorkspace shortcut entry’s localized
description to include translations for all supported next-intl locales, rather
than only en and ja. Use the existing LocalizedText locale keys and preserve
localizedShortcutText() fallback behavior; do not change fallback logic unless
intentionally expanding it for the supported locales.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 998ce2d8-d789-4e88-a5fc-f93b61c68290
📒 Files selected for processing (26)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction+Defaults.swiftPackages/macOS/CmuxSettings/Sources/CmuxSettings/Values/ShortcutAction.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/ClosedItemHistory.swiftSources/ClosedWorkspaceHistory.swiftSources/CmuxConfigExecutor+WorkspaceLaunch.swiftSources/ContentView+RightSidebarCommandPalette.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/SessionPersistence.swiftSources/TabManager+DetachedWorkspace.swiftSources/TabManager+SavedLayouts.swiftSources/TabManager+WorkspaceCustomTitle.swiftSources/TabManager+WorkspaceDirectoryCustomization.swiftSources/TabManager.swiftSources/Workspace.swiftSources/WorkspaceDirectoryCustomization.swiftSources/WorkspaceDirectoryCustomizationStore.swiftSources/cmuxApp+HistoryMenu.swiftSources/cmuxApp.swiftcmux.xcodeproj/project.pbxprojcmuxTests/WorkspaceRecoveryTests.swiftskills/cmux-settings/references/shortcut-actions.mdweb/data/cmux-shortcuts.tsweb/data/cmux.schema.json
…sed-workspace # Conflicts: # Sources/TabManager.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
7015-7022: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the palette submenu translation coverage.
menu.history.reopenClosedWorkspaceis localized only forenandja, while the nearby history palette items cover the full supported catalog. Add matching translations for the remaining supported locales inResources/Localizable.xcstrings.🤖 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 `@Sources/ContentView.swift` around lines 7015 - 7022, Extend the menu.history.reopenClosedWorkspace entry in Resources/Localizable.xcstrings with translations for every supported locale currently missing, matching the coverage and established wording of the nearby history palette items while preserving the existing en and ja translations.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 7015-7022: Extend the menu.history.reopenClosedWorkspace entry in
Resources/Localizable.xcstrings with translations for every supported locale
currently missing, matching the coverage and established wording of the nearby
history palette items while preserving the existing en and ja translations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 145f92d7-7562-4f40-91ad-6aa5fa93bcf8
📒 Files selected for processing (7)
Resources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxproj
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/AppDelegate.swift (1)
7469-7512: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable directory customization for the Cloud VM synthetic workspace.
context.tabManager.addWorkspace(...)for.cloudVMLoadingstill uses the defaultshouldApplyWorkspaceDirectoryCustomization: true. WithinheritWorkingDirectory: falseand no explicitworkingDirectory, this workspace can resolve to a fallback directory and inherit stored workspace customization meant for a real directory-based workspace, like the Remote tmux mirror workspace does.🐛 Proposed fix
workspace = context.tabManager.addWorkspace( title: workspaceTitle, titleSource: .auto, initialSurface: .cloudVMLoading, inheritWorkingDirectory: false, select: true, - autoWelcomeIfNeeded: false + autoWelcomeIfNeeded: false, + shouldApplyWorkspaceDirectoryCustomization: false )🤖 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 `@Sources/AppDelegate.swift` around lines 7469 - 7512, Update the addWorkspace call in performCloudVMAction for the .cloudVMLoading synthetic workspace to disable directory customization by passing shouldApplyWorkspaceDirectoryCustomization: false. Preserve the existing inheritWorkingDirectory: false behavior and all other workspace setup options.
🤖 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.
Outside diff comments:
In `@Sources/AppDelegate.swift`:
- Around line 7469-7512: Update the addWorkspace call in performCloudVMAction
for the .cloudVMLoading synthetic workspace to disable directory customization
by passing shouldApplyWorkspaceDirectoryCustomization: false. Preserve the
existing inheritWorkingDirectory: false behavior and all other workspace setup
options.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ae7e6847-2a21-479d-86f0-21cff75ec51e
📒 Files selected for processing (12)
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Customization/WorkspaceDirectoryCustomization.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Customization/WorkspaceDirectoryCustomizationStore.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/Customization/WorkspaceDirectoryCustomizationStoreTests.swiftSources/AppDelegate+MoveTabToNewWorkspace.swiftSources/AppDelegate.swiftSources/ClosedWorkspaceHistory.swiftSources/ContentView.swiftSources/RemoteTmuxController.swiftSources/TabManager+DetachedWorkspace.swiftSources/TabManager+WorkspaceDirectoryCustomization.swiftSources/TabManager.swiftcmuxTests/WorkspaceRecoveryTests.swift
💤 Files with no reviewable changes (1)
- Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/Customization/WorkspaceDirectoryCustomization.swift
| let workspace = tabManager.addWorkspace( | ||
| title: sessionName, | ||
| title: sessionName, titleSource: .auto, | ||
| select: false, | ||
| autoWelcomeIfNeeded: false | ||
| autoWelcomeIfNeeded: false, shouldApplyWorkspaceDirectoryCustomization: false | ||
| ) |
There was a problem hiding this comment.
The title: sessionName argument is now a no-op. In addWorkspace, the title is only applied inside applyWorkspaceDirectoryCustomization, which is gated on shouldApplyWorkspaceDirectoryCustomization. When that flag is false, the workspace's customTitle is never set and the session name never appears in the sidebar. The CmuxConfigExecutor path that also passes shouldApplyWorkspaceDirectoryCustomization: false works around this by calling tabManager.setCustomTitle(tabId: newWorkspace.id, title: workspaceName, source: .auto) immediately after — there is no equivalent compensating call here.
1 participant
Summary
⌘⇧T, is editable in Settings andcmux.json, and repeated invocations walk backward through workspace-only closed history.Persistence design
Closed workspace recovery reuses the existing persisted
ClosedItemHistoryStoresnapshot pipeline, with an independent workspace-only capacity policy. Capacity trimming uses one-pass removal and is persisted after load, so oversized history is repaired once.Sticky identity lives in
CmuxWorkspacesas an injectedWorkspaceDirectoryCustomizationStore.UserDefaultsremains the sole source of truth, while a versioned envelope retains the 512 most recently mutated directory records (including explicit-clear tombstones) and migrates the earlier dictionary payload. Session restore batch-reads the map once instead of decoding it per workspace.Sticky customization semantics
cdchanges do not move identity to another directory.TabManagermutation path.The older generalized Reopen Last Closed action remains customizable. Its prior explicit
⌘⇧Tbinding is preserved for existing profiles; fresh profiles give the familiar browser-style gesture to workspace reopen.CLI
Recently-closed listing/reopen was skipped. It is not a cheap alias in the current architecture: it would require a new socket protocol operation, cross-window routing contract, CLI parsing/output/help, and localized documentation. Existing CLI workspace creation and customization paths already participate in sticky identity.
Testing and validation
Behavior coverage includes:
Local static validation covered pbxproj normalization/check, test-target wiring, package lockfile policy, workspace package grouping, deterministic-test lint, warnings-as-errors focused typechecking, Swift parser checks, package manifest parsing, and the English/Japanese localization audit. Per issue instructions, no local app build,
xcodebuild, or test execution was run; CI is the execution gate.python3 scripts/swift_file_length_budget.pyis unavailable because the script and.github/swift-file-length-budget.tsvare absent from currentorigin/main. Every large touched Swift file has zero net line growth, all new Swift files remain under 500 lines, and.github/swift-warning-budget.tsvis unchanged.Closes #8772