Repository navigation
Fix iOS workspace swipe-delete confirmation crash - #6051
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughList-level state tracks a pending workspace close; rows request closure via list helpers and receive an external ChangesWorkspace Close Confirmation Flow
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 19 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (19 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
Greptile SummaryFixes the iOS crash caused by
Confidence Score: 5/5The change is safe to merge; the crash repro is addressed and both delete paths (swipe and context menu) route through the same confirmation flow. The root cause (row-owned @State surviving cell recycling) is correctly eliminated by moving state to the list. Both the swipe-action and context-menu paths funnel through requestWorkspaceClose, so no delete can bypass confirmation. The only observations are a misleading parameter signature on confirmCloseWorkspace — a design inconsistency that carries no runtime risk. No files require special attention beyond the confirmCloseWorkspace signature cleanup noted in the inline suggestions. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Row as WorkspaceNavigationRow
participant List as WorkspaceListView
participant Mac as Mac
User->>Row: "swipe trailing or context Delete"
Row->>List: closeWorkspace(workspaceID)
Note over List: workspacePendingCloseID = workspaceID
List-->>Row: isConfirmingClose binding becomes true
Row->>User: confirmationDialog appears
alt User confirms
User->>Row: tap Delete confirm
Row->>List: confirmCloseWorkspace(workspaceID)
Note over List: workspacePendingCloseID = nil
List->>Mac: closeWorkspace(workspaceID)
else User cancels
User->>Row: tap Cancel or backdrop
Row->>List: "isConfirmingClose.wrappedValue = false"
Note over List: workspacePendingCloseID = nil
end
Reviews (3): Last reviewed commit: "Keep iOS delete swipe native" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+341
to
+348
| private var requestWorkspaceClose: ((MobileWorkspacePreview.ID) -> Void)? { | ||
| guard closeWorkspace != nil else { | ||
| return nil | ||
| } | ||
| return { workspaceID in | ||
| workspacePendingCloseID = workspaceID | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
requestWorkspaceClose computed var causes broad row invalidation on every delete gesture
requestWorkspaceClose is a computed property, so each evaluation of body produces a new closure instance. Because ((MobileWorkspacePreview.ID) -> Void)? does not conform to Equatable, SwiftUI cannot short-circuit the structural comparison for WorkspaceNavigationRow, and every visible row body re-evaluates whenever workspacePendingCloseID changes. Before this PR the confirmation state lived inside each row's own @State, so only the tapped row would re-render. Now, each swipe initiation and each dialog dismissal broadcasts to all rows in the list.
Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swift (1)
61-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTurn off full-swipe for the destructive trailing action.
Line 61 still lets a full trailing swipe trigger the delete request immediately, so the confirmation dialog auto-presents instead of preserving the intended reveal-and-tap flow from this PR. Set
allowsFullSwipetofalsehere.Suggested fix
- .swipeActions(edge: .trailing, allowsFullSwipe: true) { + .swipeActions(edge: .trailing, allowsFullSwipe: 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 `@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swift` around lines 61 - 69, The trailing swipe action currently allows a full swipe to immediately invoke the destructive action; update the swipeActions modifier in WorkspaceNavigationRow (the block using .swipeActions(edge: .trailing, allowsFullSwipe: true) with the closeWorkspace Button) to set allowsFullSwipe to false so the delete is revealed and must be tapped (i.e., change allowsFullSwipe: true to allowsFullSwipe: false).
🤖 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
`@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swift`:
- Around line 61-69: The trailing swipe action currently allows a full swipe to
immediately invoke the destructive action; update the swipeActions modifier in
WorkspaceNavigationRow (the block using .swipeActions(edge: .trailing,
allowsFullSwipe: true) with the closeWorkspace Button) to set allowsFullSwipe to
false so the delete is revealed and must be tapped (i.e., change
allowsFullSwipe: true to allowsFullSwipe: false).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 34559bca-93f4-4173-a257-e399756070a4
📒 Files selected for processing (2)
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swift
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swift`:
- Line 61: The trailing swipe action in WorkspaceNavigationRow.swift currently
uses .swipeActions(edge: .trailing, allowsFullSwipe: true), which enables
full-swipe delete; change allowsFullSwipe to false so the trailing delete uses
reveal-and-tap confirmation (i.e., replace allowsFullSwipe: true with
allowsFullSwipe: false in the .swipeActions call in WorkspaceNavigationRow).
🪄 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
Run ID: f5f7cbd9-b9ae-42b1-b3db-1a0b51d40b74
📒 Files selected for processing (1)
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swift
| @@ -54,11 +60,12 @@ struct WorkspaceNavigationRow: View { | |||
| } | |||
| .swipeActions(edge: .trailing, allowsFullSwipe: true) { | |||
There was a problem hiding this comment.
Disable full-swipe on trailing delete to preserve reveal-and-tap confirmation flow.
Line 61 sets allowsFullSwipe: true, which allows a full swipe to trigger delete-request presentation directly. That conflicts with the stated behavior for this PR (“no full-swipe auto-presentation”). Set trailing delete swipe to allowsFullSwipe: false.
Proposed diff
- .swipeActions(edge: .trailing, allowsFullSwipe: true) {
+ .swipeActions(edge: .trailing, allowsFullSwipe: 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
`@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceNavigationRow.swift`
at line 61, The trailing swipe action in WorkspaceNavigationRow.swift currently
uses .swipeActions(edge: .trailing, allowsFullSwipe: true), which enables
full-swipe delete; change allowsFullSwipe to false so the trailing delete uses
reveal-and-tap confirmation (i.e., replace allowsFullSwipe: true with
allowsFullSwipe: false in the .swipeActions call in WorkspaceNavigationRow).
hhsw2015
pushed a commit
to hhsw2015/cmux
that referenced
this pull request
Jun 14, 2026
PRs included: - AppDelegate decomposition: CmuxSession session-snapshot repository (manaflow-ai#6030) - Fix Cmd+T cwd after session restore (manaflow-ai#6055) - Speed up iOS terminal scroll rendering (manaflow-ai#6035) - Preserve Pi sessions across workspace restore (manaflow-ai#5607) - Scope Biome checks to maintained JS sources (manaflow-ai#6008) - Fix manaflow-ai#5917: restore OSC 11 pane-local backgrounds (manaflow-ai#5997) - Expose stable window title templates (manaflow-ai#6059) - Honor macos-option-as-alt left/right - Fix macOS 27 SF Symbol rasterization crash (manaflow-ai#5999) - CmuxRemote* family: extract Workspace remote/cloud-VM connectivity - Fix iOS workspace swipe-delete confirmation crash (manaflow-ai#6051) - TabManager decomposition Wave 3+4 sub-models - Sidebar row cleanups: branchless frame anchor - CmuxIPCService: extract AppDelegate multi-window CLI routing - CmuxSidebarGit: extract TabManager git-metadata + PR-polling subsystem - CmuxTerminalCore: extract terminal core leaf Fork-side adjustments: - ghostty submodule: cherry-pick mouse-modifier-state fix onto our renderer-realized branch - Workspace.swift: take theirs (upstream extracted ~7700 lines into CmuxCore.Remote/CmuxRemoteSession packages); restore fork's renameTopLevelLayoutTabContaining/closeTopLevelLayoutTabContaining + surfaceTmuxClientTTYNames + WorkspaceLayoutTab integration - TabManager.swift: take theirs; re-add static allocatePortOrdinal() - BrowserPanelView, RenderableSystemSymbol: keep fork's cmuxSymbolPixelSize extension on top of upstream's cmuxSymbolRasterSize - Add CmuxWorkspaces / CMUXSessionDaemon / CmuxCommandPalette imports to TerminalController, Workspace, SessionPersistence - Sources/Workspace+P43Stubs.swift: thin shims for SplitEqualizer, WorkspaceRemoteSessionController.PortScanKickReason, WorkspaceGroupNewWorkspacePlacementSettings (legacy types fork TC still calls; replace with package APIs in P44+) - Sources/GhosttySurfaceSizeDeferralReason.swift: restore fork-only enum (deleted by upstream) - Sources/StableLayout/SessionBlueprintExportAction.swift: parked debug action (depends on legacy SessionPersistenceStore, gone) - Sources/GhosttyTerminalView.swift: stub ghostty_surface_select_cursor_line_compat (needs zig 0.15.2 xcframework rebuild) - pbxproj: keep-both, drop stale ProcessPipeReader/SplitEqualizer/Panels/BrowserProxyEndpoint refs, fix SurfaceHibernationPolicy UUID collision - Drop fork's WorkspaceRemoteConfiguration.swift + WorkspaceRemoteSSHBatchCommandBuilder.swift (extracted to CmuxCore package)
This branch was successfully deployed
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
1 participant
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.
Summary
WorkspaceListViewRepro fixed
Swipe-delete one workspace, then swipe-delete the row above it. The confirmation dialog is now owned by the list instead of a recycled row.
Verification
git diff --check HEAD~1..HEAD./scripts/lint-ios-package-conventions.shpython3 scripts/swift_file_length_budget.py --budget .github/swift-file-length-budget.tsv./scripts/reload-cloud.sh --tag wdel(cloud unavailable, local fallback succeeded)ios/scripts/reload.sh --tag wdelLocalization
No new user-facing strings. Existing localized delete confirmation strings were moved from row-level presentation to list-level presentation.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes an iOS crash when swipe-deleting workspaces by moving delete-confirmation state to
WorkspaceListViewand presenting the native confirmation dialog from the swiped row. Full-swipe now opens the confirm dialog (no auto-delete); recycled cells no longer own presentation, and swipe and context-menu deletes share the same flow.Written for commit c075ddf. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes