Repository navigation
Issue #3866: Add terminal WebView sidekick drawer - #3968
austinywang wants to merge 58 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 a Terminal Sidekick (state, coordinator, toggle shortcut, split UI, divider, persistence, restore), rewrites omnibar/shortcut routing and selection-repeat, ties terminal runtime/input to real window attachment with socket injection changes, simplifies notifications/unread syncing, updates AgentLaunch sanitization, renames Xcode project/CI/CLI to GhosttyTabs, and adjusts/extends tests and docs accordingly. ChangesTerminal Sidekick & UI /
Shortcuts & Omnibar routing /
Terminal surface/input & controller /
BrowserPanel / Omnibar & WebView injection /
Notifications & unread sync /
Agent launch sanitizer & restorable sessions /
Tests, CI/workflows, CLI, docs, project rename / various files
Estimated code review effort: 🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryAdds a per-terminal
Confidence Score: 5/5Safe to merge; all previously identified correctness concerns are addressed and the new code is well-structured. The coordinator lifecycle, availability gating, and session restore are all exercised by the new test suite. No correctness or data-loss issues were found; the only feedback is a file-organisation nit on TerminalPanelView.swift having grown to 460 lines. TerminalPanelView.swift — worth splitting the sidekick types into their own file before the file grows further, but no blocking issue in the current state. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppDelegate
participant TerminalPanel
participant Coordinator as TerminalSidekickCoordinator
participant BrowserPanel
participant View as TerminalPanelView
User->>AppDelegate: opt+cmd+B keyEquivalent
AppDelegate->>AppDelegate: canToggleTerminalSidekickInActiveMainWindow()
AppDelegate->>AppDelegate: "Task @MainActor (escape animation context)"
AppDelegate->>TerminalPanel: toggleSidekick()
TerminalPanel->>Coordinator: toggleSidekick()
Coordinator->>Coordinator: ensureBrowserPanel()
Coordinator->>BrowserPanel: init(initialURL:, renderInitialNavigation:)
Coordinator->>Coordinator: replaceState(isOpen: true)
Coordinator->>TerminalPanel: notifyChanged() objectWillChange.send()
TerminalPanel-->>View: "re-render sidekickState.isOpen=true"
User->>View: drag divider
View->>View: sidekickResizePreviewRatio local State
View->>TerminalPanel: setSidekickSplitRatio() on drag end
TerminalPanel->>Coordinator: setSidekickSplitRatio()
User->>View: submit address bar
View->>TerminalPanel: navigateSidekick(input:)
TerminalPanel->>Coordinator: navigateSidekick(input:)
Coordinator->>BrowserPanel: navigateSmart(trimmed)
Note over AppDelegate,View: On session save
TerminalPanel->>Coordinator: sessionSnapshot()
Coordinator-->>TerminalPanel: SessionTerminalSidekickSnapshot?
Note over AppDelegate,View: On session restore
TerminalPanel->>Coordinator: restoreSidekick(snapshot)
Coordinator->>Coordinator: BrowserAvailabilitySettings.isEnabled() guard
Coordinator->>BrowserPanel: ensureBrowserPanel() navigate(to:)
Reviews (10): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Panels/TerminalPanel.swift`:
- Around line 212-217: The closeSidekick() currently only flips
sidekickState.isOpen to false but leaves sidekickBrowserPanel alive; modify
closeSidekick() to call sidekickBrowserPanel?.close() and then set
sidekickBrowserPanel = nil to release the BrowserPanel/WKWebView resources, and
ensure openSidekick() lazily recreates sidekickBrowserPanel when needed (or
document this as an intentional trade-off if you prefer keeping it for faster
reopen). Keep references to sidekickState and currentSidekickURLString()
unchanged and only add the close() + nil assignment to properly release
resources.
In `@Sources/Panels/TerminalPanelView.swift`:
- Around line 235-238: commitAddress currently calls onNavigate(addressText)
unconditionally, allowing empty/whitespace submissions via Enter; add a guard
that trims addressText and returns early if empty (matching the "Open in
Sidekick" button disable logic), only calling onNavigate(trimmed) and then
setting addressFocused = false when the trimmed value is non-empty; reference
commitAddress, addressText, onNavigate, and addressFocused when making the
change.
🪄 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: 065dcf39-8895-4ece-ae46-bed7fc1df251
📒 Files selected for processing (13)
Resources/Localizable.xcstringsSources/AppDelegate.swiftSources/KeyboardShortcutContext.swiftSources/KeyboardShortcutSettings.swiftSources/Panels/TerminalPanel.swiftSources/Panels/TerminalPanelView.swiftSources/SessionPersistence.swiftSources/Workspace.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftcmuxTests/KeyboardShortcutContextTests.swiftcmuxTests/TabManagerSessionSnapshotTests.swiftcmuxTests/WorkspaceUnitTests.swiftweb/data/cmux-shortcuts.ts
|
Follow-up review fixes are now on head
@cubic-dev-ai review |
@lawrencecchen I have started the AI code review. It will take a few minutes to complete. |
|
Follow-up compile fix is now on head @cubic-dev-ai review |
@lawrencecchen I have started the AI code review. It will take a few minutes to complete. |
|
Follow-up unit compile fix is now on head @cubic-dev-ai review |
@lawrencecchen I have started the AI code review. It will take a few minutes to complete. |
|
Follow-up unit compile fix is now on head @cubic-dev-ai review |
@lawrencecchen I have started the AI code review. It will take a few minutes to complete. |
|
Follow-up unit compile fix is now on head @cubic-dev-ai review |
@lawrencecchen I have started the AI code review. It will take a few minutes to complete. |
…-3866-webview-sidekick-drawer # Conflicts: # .circleci/config.yml # .github/workflows/ci.yml # .github/workflows/perf-activation.yml # Sources/AppDelegate.swift # Sources/TabManager.swift # cmuxTests/AppDelegateShortcutRoutingTests.swift # cmuxTests/SessionPersistenceTests.swift # docs/internal/skills-customization-ideas.md
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 12f266c. Configure here.
| func browserOmnibarShouldContinueControlNavigationRepeat(flags: NSEvent.ModifierFlags) -> Bool { | ||
| browserOmnibarNormalizedModifierFlags(flags) == [.control] | ||
| let normalizedFlags = browserOmnibarNormalizedModifierFlags(flags) | ||
| return normalizedFlags == [.control] || normalizedFlags == [.command] |
There was a problem hiding this comment.
Production-unused function after inline replacement
Low Severity
browserOmnibarShouldContinueControlNavigationRepeat was updated to support both Command and Control modifiers, but its only production call site in handleBrowserOmnibarSelectionRepeatLifecycleEvent was replaced with inline logic checking browserOmnibarRepeatModifierFlags. The function is now only referenced by tests, making it dead code in production.
Reviewed by Cursor Bugbot for commit 12f266c. Configure here.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Closes #3866
Summary
⌥⌘Bshortcut, settings/config/docs metadata, localization, and behavior coverage.Scope
This is a P1 first pass: manual toggle, manual URL entry, inline WKWebView rendering, and session persistence. URL detection from terminal output, detachable sidekick windows, and search indexing remain follow-up scope.
Verification
git diff --checkjq empty Resources/Localizable.xcstringsNote
Medium Risk
Medium risk: adds new shortcut-driven UI behavior (terminal sidekick + omnibar navigation/repeat logic) and removes Rust-based command palette search/CI Rust installs, which could affect keyboard routing and build pipelines.
Overview
Adds a focused-terminal WebView “sidekick” drawer that can be toggled via a new
toggleTerminalSidekickshortcut, including new localization strings and updated shortcut documentation.Refactors browser omnibar keyboard navigation to support Cmd/Ctrl
n/prepeat behavior, simplifies address-bar focus tracking, and prevents focus-stealing after file drops via anallowsPanelFocusAfterFileDropgate.Removes the Rust
Native/CommandPaletteNucleoFFIimplementation (and its Swift glue/overlay) and drops Rust installation steps across GitHub Actions workflows; also tweaks agent launch argument sanitization for Codex session subcommands and adjusts notification menu unread counting/removes the toggle-unread shortcut/strings.Reviewed by Cursor Bugbot for commit 12f266c. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a per-terminal WebView “sidekick” beside terminals for quick lookups, with drag‑resize and per‑session persistence, addressing issue #3866. ⌥⌘B now toggles the sidekick when a terminal is focused and the right sidebar otherwise; browser shortcuts and Find route to the sidekick when it’s focused.
New Features
BrowserPanelView; routes browser shortcuts/Find from sidekick focus.Bug Fixes
BrowserPaneDropContextaddsallowsPanelFocusAfterFileDropto avoid focus steal.--add-diratresumeandfork; supports--fork-session;topProcessLabelprints as reason=; skips launcher script when Claude auth selection env is preserved; preserves hook trust.CommandPaletteNucleoFFIlibrary/tests/scripts; activation workflow uses a distinct SPM cache key; docs updated (shortcut clarifications and CLI cleanup).Written for commit 12f266c. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Changes