Repository navigation
Add per-tab remote indicator and Disconnect SSH tab context action - #162
Conversation
📝 WalkthroughWalkthroughAdds a ChangesRemote SSH indicator and disconnect action
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant BonsplitController
participant TabBarView
participant TabContextMenuBuilder
Host->>BonsplitController: createTab(showsRemoteIndicator: true)
BonsplitController-->>Host: TabID
TabBarView->>BonsplitController: tabContextDisconnectRemoteAvailabilityProvider(tabID, paneID)
BonsplitController-->>TabBarView: canDisconnectRemote
TabBarView->>TabContextMenuBuilder: makeMenu(state)
TabContextMenuBuilder-->>TabBarView: menu with "Disconnect SSH" item (if available)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Bonsplit/Public/BonsplitController.swift (1)
232-235: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
requestTabContextActionstill references.toggleFullWidthTab— ensure the merge conflict inTabContextAction.swiftis resolved to keep this case.Line 232 checks
if action == .toggleFullWidthTaband routes torequestTabFullWidthToggle. If the unresolved merge conflict inTabContextAction.swift(lines 24–28) is resolved by removingtoggleFullWidthTab, this line will fail to compile. The full-width-tab infrastructure (lines 107–109, 779–792) also depends on it.🤖 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/Bonsplit/Public/BonsplitController.swift` around lines 232 - 235, The `requestTabContextAction` flow in `BonsplitController` still depends on `.toggleFullWidthTab`, so make sure the merge conflict in `TabContextAction.swift` is resolved by keeping that enum case and any related API surface intact. Update the `TabContextAction` definition and any associated switch handling so the `.toggleFullWidthTab` branch remains valid for `requestTabFullWidthToggle`, and verify the full-width-tab support used elsewhere in the controller continues to compile and route correctly.
🤖 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 `@Sources/Bonsplit/Public/Types/TabContextAction.swift`:
- Around line 24-28: Resolve the unresolved merge conflict in TabContextAction
by removing the literal conflict markers and keeping both enum cases,
toggleFullWidthTab and disconnectRemote, so the file compiles cleanly. Make sure
TabContextAction still includes toggleFullWidthTab because BonsplitController
uses it in its action handling and full-width-tab flow, and add disconnectRemote
alongside it rather than replacing it.
---
Outside diff comments:
In `@Sources/Bonsplit/Public/BonsplitController.swift`:
- Around line 232-235: The `requestTabContextAction` flow in
`BonsplitController` still depends on `.toggleFullWidthTab`, so make sure the
merge conflict in `TabContextAction.swift` is resolved by keeping that enum case
and any related API surface intact. Update the `TabContextAction` definition and
any associated switch handling so the `.toggleFullWidthTab` branch remains valid
for `requestTabFullWidthToggle`, and verify the full-width-tab support used
elsewhere in the controller continues to compile and route correctly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 868d8dfc-ae9c-4ead-aad2-61cc94a34c76
📒 Files selected for processing (9)
Sources/Bonsplit/Internal/Models/TabItem.swiftSources/Bonsplit/Internal/Views/TabBarView.swiftSources/Bonsplit/Internal/Views/TabItemView.swiftSources/Bonsplit/Public/BonsplitController.swiftSources/Bonsplit/Public/Types/Tab.swiftSources/Bonsplit/Public/Types/TabContextAction.swiftSources/Bonsplit/Resources/en.lproj/Localizable.stringsSources/Bonsplit/Resources/ja.lproj/Localizable.stringsTests/BonsplitTests/BonsplitTests.swift
| <<<<<<< HEAD | ||
| case toggleFullWidthTab | ||
| ======= | ||
| case disconnectRemote | ||
| >>>>>>> 57f8850 (Add per-tab remote indicator and Disconnect SSH tab context action) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Unresolved git merge conflict markers will prevent compilation.
The file contains literal <<<<<<< HEAD, =======, and >>>>>>> markers at lines 24–28. This will cause a Swift compilation failure. The conflict must be resolved before merge.
Additionally, if the resolution removes case toggleFullWidthTab (as the PR summary suggests), BonsplitController.swift line 232 (if action == .toggleFullWidthTab) and the full-width-tab infrastructure at lines 107–109 and 779–792 will fail to compile. The controller still actively uses toggleFullWidthTab, so the resolution should keep both cases.
🔧 Proposed resolution: keep both cases
case toggleZoom
-<<<<<<< HEAD
- case toggleFullWidthTab
-=======
case disconnectRemote
->>>>>>> 57f8850 (Add per-tab remote indicator and Disconnect SSH tab context action)
+ case toggleFullWidthTab
+ case disconnectRemote🧰 Tools
🪛 GitHub Actions: CI / 0_tests.txt
[error] 24-26: Swift compilation error: source control conflict marker in source file (found '<<<<<<< HEAD' / '=======').
🪛 GitHub Actions: CI / tests
[error] 24-24: Swift compilation failed during swift test/emit-module. Error: source control conflict marker in source file (<<<<<<< HEAD / =======).
🤖 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/Bonsplit/Public/Types/TabContextAction.swift` around lines 24 - 28,
Resolve the unresolved merge conflict in TabContextAction by removing the
literal conflict markers and keeping both enum cases, toggleFullWidthTab and
disconnectRemote, so the file compiles cleanly. Make sure TabContextAction still
includes toggleFullWidthTab because BonsplitController uses it in its action
handling and full-width-tab flow, and add disconnectRemote alongside it rather
than replacing it.
Greptile SummaryThis PR adds remote-connection state and context-menu support for SSH tabs. The main changes are:
Confidence Score: 2/5The package is not safe to merge while
Sources/Bonsplit/Public/Types/TabContextAction.swift Important Files Changed
Reviews (1): Last reviewed commit: "Add per-tab remote indicator and Disconn..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <<<<<<< HEAD | ||
| case toggleFullWidthTab | ||
| ======= | ||
| case disconnectRemote | ||
| >>>>>>> 57f8850 (Add per-tab remote indicator and Disconnect SSH tab context action) |
There was a problem hiding this comment.
The enum body still contains merge-conflict markers. Any target that compiles Bonsplit will fail to parse this file, so neither toggleFullWidthTab nor the new disconnectRemote action can be used until both cases are kept without the markers.
| <<<<<<< HEAD | |
| case toggleFullWidthTab | |
| ======= | |
| case disconnectRemote | |
| >>>>>>> 57f8850 (Add per-tab remote indicator and Disconnect SSH tab context action) | |
| case toggleFullWidthTab | |
| case disconnectRemote |
- Tab/TabItem gain showsRemoteIndicator (rendered as a small network glyph after the tab title, with accessibility text) - BonsplitController.updateTab/createTab/splitPane carry the new field - New TabContextAction.disconnectRemote, shown only when the host's tabContextDisconnectRemoteAvailabilityProvider returns true
5cbe2bf to
7312bce
Compare
2 participants
Re-lands the never-merged
ssh-pane-remote-indicatorbranch (f3b3658, built for manaflow-ai/cmux#5949), rebased onto current main:Tab/TabItemgainshowsRemoteIndicator(rendered as a smallnetworkglyph after the tab title, with accessibility text), threaded throughcreateTab/updateTab/splitPane.TabContextAction.disconnectRemote, shown only when the host'stabContextDisconnectRemoteAvailabilityProviderreturns true. Localized en + ja.Rebase-specific integration on top of the original commit:
updateTab's no-op guard (Skip no-op tab metadata updates #160) now also comparesshowsRemoteIndicator, so an indicator-only update is not skipped.TabContextMenuState(post-Render full-width tabs through normal tab bar #161 shape) carriescanDisconnectRemoteas a defaulted init parameter, and the@MainActorconvenience init feeds it from the provider.Consumed by pane-scoped
cmux ssh --pane(manaflow-ai/cmux#7521). The cmux submodule pointer will reference this commit once it lands on main.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Add a per-tab remote connection indicator and an optional “Disconnect SSH” tab context action so hosts can signal remote tabs and let users disconnect from SSH.
New Features
showsRemoteIndicator; shown as a small “network” icon next to the title with an accessibility label. Available viacreateTab,updateTab, andsplitPane.disconnectRemote(“Disconnect SSH”), shown only whentabContextDisconnectRemoteAvailabilityProviderreturns true. Localized in en and ja.updateTabnow updates when onlyshowsRemoteIndicatorchanges.Migration
showsRemoteIndicatorwhen creating or updating tabs that are connected remotely.tabContextDisconnectRemoteAvailabilityProviderto surface the “Disconnect SSH” menu item.TabContextAction.disconnectRemotein your context menu action handler.Written for commit 7312bce. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation / Localization