Skip to content

Add Home workspace webview - #6788

Closed
lawrencecchen wants to merge 1 commit into
mainfrom
task-home-button-gui
Closed

lawrencecchen wants to merge 1 commit into
mainfrom
task-home-button-gui

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Makes the titlebar Home button create or select a pinned Home workspace at the top of the workspace list.
  • Renders Home as a bundled BrowserPanel webview at cmux-webview://home/home.html, using the same webviews build pipeline as the diff/agent surfaces.
  • Adds a trusted cmuxHome webview bridge for New Workspace, Browser, Command Palette, and Settings actions.
  • Places Home immediately to the right of the plus/New Workspace titlebar button.
  • Removes the prior SwiftUI Home overlay path while keeping old .home session decode compatibility by mapping it back to tabs.

Testing

  • git diff --check
  • ./scripts/build-webviews-app.sh --check
  • node -e 'JSON.parse(require("fs").readFileSync("Resources/Localizable.xcstrings", "utf8")); console.log("Localizable OK")'\n- ./scripts/check-pbxproj.sh\n- ./scripts/reload-cloud.sh --tag homewv passed in https://github.com/manaflow-ai/cmux/actions/runs/28213731201.\n- Follow-up placement rebuild ./scripts/reload-cloud.sh --tag homewv passed in https://github.com/manaflow-ai/cmux/actions/runs/28214785551.\n- Launched tagged app homewv, verified /tmp/cmux-debug-homewv.sock identifies com.cmuxterm.app.debug.homewv, sent show_home, confirmed Home is selected, pinned, and index 0.\n- Verified Home DOM text and direct browser screenshot at /var/folders/rr/vmfx6xh12dz2tlvgtmyvjmf80000gn/T/cmux-browser-screenshots/surface-401255A5-1782442115517-F540CF61.png.\n- Verified titlebar order with screenshot /var/folders/rr/vmfx6xh12dz2tlvgtmyvjmf80000gn/T/cmux-screenshots/2026-06-26T03-19-20Z_16FB98FD.png: sidebar, notifications, plus, Home, back.\n- Invoked window.webkit.messageHandlers.cmuxHome.postMessage({ action: "newBrowser" }) from the Home page and confirmed it created/focused a browser workspace.\n\n## Issues\n- Related: user task from cmuxterm-hq.

Note

Low Risk
String catalog-only changes with no logic or security impact.

Overview
Adds manual English and Japanese strings in Localizable.xcstrings for the Home experience: titlebar.home.accessibilityLabel and titlebar.home.tooltip for the titlebar Home control, and home.workspace.title for the pinned Home workspace label.

Reviewed by Cursor Bugbot for commit 3f8c6d0. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • New Features
    • Added a Home dashboard with quick actions (New Workspace, Browser), Command Palette, and Settings.
    • Added a Home control to the title bar, including full support in minimal/hidden titlebar modes.
    • Made Home a first-class sidebar selection option for consistent behavior across windows.
  • Bug Fixes
    • Improved Home opening to reliably activate the best available main window and load the Home workspace/surface.
    • Refined workspace/tab focus and minimal-mode sidebar action/layout handling for more consistent interaction.
  • Documentation
    • Expanded Home-related localization (including accessibility labels and tooltips) in English and Japanese.

@vercel

vercel Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 26, 2026 3:43am
cmux-staging Building Building Preview, Comment Jun 26, 2026 3:43am

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a Home webview, native handling for Home actions, UI entry points that open Home, and workspace startup settings for Home browser surfaces.

Changes

Home screen and action wiring

Layer / File(s) Summary
Webview bundle and bootstrap
Resources/Localizable.xcstrings, Resources/markdown-viewer/webviews-app/*, scripts/build-webviews-app.sh, webviews/src/agent-session/shared/bridge.ts, webviews/src/main.tsx, webviews/src/router.tsx
Adds Home localization, bundled webview entry files, router/bootstrap support, build-script output generation, and the Window.webkit.cmuxHome bridge typing.
Home surface UI
Resources/markdown-viewer/webviews-app/chunks/homeSurface.mjs, webviews/src/home.css, webviews/src/surfaces/homeSurface.tsx
Adds the Home surface action UI, bridge calls, icon rendering, and Home-specific styling for the webview.
Home webview bridge and scheme handling
Sources/Panels/CmuxBundledWebViewURLSchemeHandler.swift, Sources/Panels/HomeWebViewBridge.swift, Sources/Panels/BrowserPanel.swift, cmux.xcodeproj/project.pbxproj
Adds the bundled Home scheme handler, the Home message bridge, browser-panel wiring for the new bridge and scheme, and project inclusion for both native files.
Minimal-mode control contract
Resources/Localizable.xcstrings, Sources/WindowDragHandleView.swift, Sources/Update/UpdateTitlebarAccessory.swift, Sources/Update/MinimalModeSidebarControls.swift, Sources/WindowDecorationsController.swift
Adds the .home minimal-mode slot, localized Home labels and workspace title, updated titlebar control geometry and slot mapping, and right-click handling for the new control.
Home workspace bootstrap
Sources/AppDelegate.swift, Sources/ContentView.swift, Sources/SessionPersistence.swift, Sources/TabManager.swift, Sources/Workspace.swift, Sources/TerminalController.swift
Adds showHomeInActiveMainWindow(preferredWindow:), threads Home through titlebar/window control paths, updates session mapping, extends browser workspace creation parameters, and adds the debug show_home command.

Sequence Diagram(s)

sequenceDiagram
  participant HomeSurface
  participant HomeWebViewBridge
  participant AppDelegate
  participant BrowserPanel
  participant CmuxBundledWebViewURLSchemeHandler

  HomeSurface->>HomeWebViewBridge: postMessage({ action })
  HomeWebViewBridge->>AppDelegate: dispatch Home action
  AppDelegate->>BrowserPanel: activate Home browser surface
  BrowserPanel->>CmuxBundledWebViewURLSchemeHandler: load cmux-webview://home/home.html
Loading

Estimated Code Review Effort

🎯 5 (Critical) | ⏱️ ~90 minutes

Possibly related PRs

  • manaflow-ai/cmux#4160: Touches the same minimal-mode titlebar action infrastructure and MinimalModeSidebarControlActionSlot paths used here.
  • manaflow-ai/cmux#5017: Also changes Sources/Update/UpdateTitlebarAccessory.swift, which this PR extends with a new Home control.
  • manaflow-ai/cmux#5613: Also modifies the webview dispatcher entry points in webviews/src/main.tsx and webviews/src/main.mjs.

Poem

🐇 I hopped to the Home with a tap and a grin,
A new little doorway now opens within.
Buttons, bridges, and browsers align,
And the cozy new workspace says “Home” just in time.


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (4 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Algorithmic Complexity ❌ Error FAIL: Sources/Panels/HomeWebViewBridge.swift:140-149 does nested scans over all tabs and panel keys per bridge message; no bound/benchmark is given for ~1000 workspaces. Replace the tabs.contains/keys.contains join with indexed lookup (e.g. workspaceById plus workspace.panels[panelId] or a panel→workspace map) before resolving the window context.
Cmux Full Internationalization ❌ Error Fail: the new Home webview hardcodes English UI text instead of next-intl, and the new xcstrings keys are only en/ja while the catalog already has 20 locales. Move Home surface copy into web/messages/*.json for every locale in web/i18n/routing.ts, and add translations for the new Resources/Localizable.xcstrings keys in all catalog locales.
Cmux No Test Or Debug Seam In Production Source ❌ Error Sources/TerminalController.swift adds a DEBUG-only show_home socket command plus showHome(), a new debug seam in production source with no production caller. Move the debug-only command into a dedicated debug file/folder, or remove it from production and reach needed state from tests via @testable import.
Cmux No Ambient Global State ❌ Error FAIL: Sources/Panels/HomeWebViewBridge.swift:9 and Sources/Panels/CmuxBundledWebViewURLSchemeHandler.swift:6 add new shared singletons, creating ambient global surfaces. Instantiate the bridge and URL-scheme handler per BrowserPanel/webview config and inject them from the app seam instead of exposing shared.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (20 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding a Home workspace webview.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed PASS: New Home path stays on MainActor (AppDelegate, BrowserPanel, TerminalController, UpdateTitlebarAccessory, HomeWebViewBridge), with no new nonisolated mutable UI-store access.
Cmux Swift Blocking Runtime ✅ Passed New runtime paths use async/MainActor code; the only sync wait is v2MainSync in a #if DEBUG socket command, so no production blocking was added.
Cmux Browser Automation Off-Main ✅ Passed show_home is a direct main-hop UI action; wait-heavy browser.* methods still live in socketWorkerMethods and are covered by ControlCommandExecutionPolicyTests.
Cmux Expensive Synchronous Load ✅ Passed PASS: New Home UI/socket paths only route to showHomeInActiveMainWindow; I found no added RestorableAgentSessionIndex.load or other agent-history sync parsing on main/interactive paths.
Cmux Cache Substitution Correctness ✅ Passed No offending cache substitution: the new Home flow uses live window/workspace state, and browser history fallback is freshness-checked via live URL before restored history.
Cmux No Hacky Sleeps ✅ Passed No changed TS/JS/shell/runtime file adds fixed sleeps, timers, polling, or wait-based sync; the new Home flow is event-driven.
Cmux Swift Concurrency ✅ Passed The diff only adds UI/OS-boundary hops (Task { @mainactor }, v2MainSync) and no new background queues, Combine state, or internal completion-handler APIs.
Cmux Swift @Concurrent ✅ Passed No new @concurrent or problematic nonisolated async work was added; the new Home actions are sync or explicitly hop to @MainActor.
Cmux Swift File And Package Boundaries ✅ Passed No new oversized Swift files; added Home code is focused AppKit/UI glue, while the touched huge files got only small, related additions.
Cmux Swiftpm Lockfiles ✅ Passed Diff only changes submodule pointers/ghostty Zig files; no .gitignore, Package.resolved, or pbxproj package-reference changes, so the rule isn’t triggered.
Cmux Swift Logging ✅ Passed The only new Home-path log is cmuxDebugLog inside #if DEBUG; no added print/NSLog/debugPrint or new Logger misuse appears in the diff.
Cmux User-Facing Error Privacy ✅ Passed New user-facing errors are generic/sanitized ('Home bridge is unavailable', not_allowed/invalid_request/action_failed, 'ERROR: No main window'); no vendor/provider/raw upstream details.
Cmux Swiftui State Layout ✅ Passed The SwiftUI changes only add Home action closures and enum cases; they don’t introduce new ObservableObject/@published state, GeometryReader measurement, or render-time state writes.
Cmux Architecture Rethink ✅ Passed Home is centralized in AppDelegate.showHomeInActiveMainWindow; UI surfaces and socket commands just call it, and the new WK bridge/scheme handler are required platform glue.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: Home stays inside existing workspace/BrowserPanel paths; no new standalone WindowGroup/NSPanel/NSWindowController or cmux.* identifier changes were added.
Cmux Source Artifacts ✅ Passed Changed paths are intentional source/config/assets or submodule pointers; no logs, screenshots, temp dirs, caches, or other artifact paths appear.
Description check ✅ Passed The description includes solid Summary and Testing sections, and the missing template sections are non-critical.
✨ 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 task-home-button-gui

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

Comment @coderabbitai help to get the list of available commands.

Comment thread Sources/ContentView.swift Outdated
@greptile-apps

greptile-apps Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces the Home workspace webview: a pinned, index-0 BrowserPanel loaded from the bundled cmux-webview://home/home.html surface, with a new cmuxHome bridge for New Workspace, Browser, Command Palette, and Settings actions, and a corresponding titlebar Home button wired into both the standard and minimal-mode control bars.

  • New infrastructure: CmuxBundledWebViewURLSchemeHandler serves bundled webview assets under the cmux-webview:// custom scheme with a symlink-resolving path-traversal guard; HomeWebViewBridge handles bridge messages with strict frame-trust validation (cmux-webview://home/home.html main-frame only), following the existing DiffCommentsBridge pattern.
  • Workspace lifecycle: ensureHomeWorkspace in AppDelegate creates or selects the Home workspace on every showHome call, pins it, and reorders it to index 0; the lookup uses the localized home.workspace.title string as the identity key — a title-based heuristic that produces duplicate Home workspaces if the system locale changes between sessions.
  • Session compatibility: SessionSidebarSelection.home is decoded to .tabs for backwards compatibility; Localizable.xcstrings adds English and Japanese entries for all three new UI strings.

Confidence Score: 4/5

Safe to merge with awareness of the workspace-identity issue: all other new infrastructure (bridge trust validation, path-traversal guard, session-compat decode) looks correct.

The workspace lookup in ensureHomeWorkspace uses the session's current localized string for "Home" as the only identity key. A user who changes their system language between sessions will accumulate duplicate pinned Home workspaces instead of returning to the original one — a present, reproducible misbehavior on the changed code path. Everything else in the PR (bridge frame-trust gating, URL scheme handler path guards, titlebar layout math, persistence decode) is well-constructed.

Sources/AppDelegate.swift — the ensureHomeWorkspace title-based lookup.

Important Files Changed

Filename Overview
Sources/AppDelegate.swift Adds showHomeInActiveMainWindow, ensureHomeWorkspace, and ensureHomeBrowserSurface. The workspace lookup uses a localized title string as the sole identity key, causing duplicate Home workspaces when the system locale changes.
Sources/Panels/HomeWebViewBridge.swift New WKScriptMessageHandlerWithReply for the cmuxHome bridge; follows the existing DiffCommentsBridge singleton pattern, with correct frame-trust validation (isTrustedHomeFrame) and associated-object context lookup.
Sources/Panels/CmuxBundledWebViewURLSchemeHandler.swift New WKURLSchemeHandler serving bundled webview assets under the cmux-webview:// custom scheme; path traversal is guarded by component filtering plus symlink-resolving root-prefix check.
Sources/Update/UpdateTitlebarAccessory.swift Adds Home button to TitlebarControlsView and HiddenTitlebarSidebarControlsView; fixes buttonRowWidth/xOffset to count against MinimalModeSidebarControlActionSlot (which includes .home) via the new titlebarControlSlot mapping.
webviews/src/surfaces/homeSurface.tsx New Home webview surface built with native <button> elements and a frame-trust-gated bridge call; handles pending/error state correctly.
Sources/SessionPersistence.swift Adds SessionSidebarSelection.home decoded to .tabs for backwards compatibility with persisted .home sessions.
Sources/WindowDragHandleView.swift Adds .home to MinimalModeSidebarControlActionSlot and widens hostWidth by 30 pt; .home is listed in acceptsContextMenu = true but the right-click handler falls through to super.
Resources/Localizable.xcstrings Adds titlebar.home.accessibilityLabel, titlebar.home.tooltip, and home.workspace.title with both English and Japanese translations, matching all existing catalog locales.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant User
    participant TitlebarButton as Titlebar Home Button
    participant AppDelegate
    participant TabManager
    participant Workspace
    participant BrowserPanel
    participant SchemeHandler as CmuxBundledWebViewURLSchemeHandler
    participant Bridge as HomeWebViewBridge

    User->>TitlebarButton: click
    TitlebarButton->>AppDelegate: showHomeInActiveMainWindow()
    AppDelegate->>AppDelegate: resolve active MainWindowContext
    AppDelegate->>TabManager: ensureHomeWorkspace(homeURL:)
    TabManager->>TabManager: "tabs.first { customTitle == title }"
    alt workspace not found
        TabManager->>Workspace: addWorkspace(initialSurface: .browser, initialBrowserURL: homeURL)
        Workspace->>BrowserPanel: init(initialURL: homeURL, transparentBackground: true)
    end
    AppDelegate->>Workspace: setPinned(true), reorderToIndex(0)
    AppDelegate->>BrowserPanel: navigate(to: cmux-webview://home/home.html)
    BrowserPanel->>SchemeHandler: webView(_:start:urlSchemeTask:)
    SchemeHandler->>SchemeHandler: bundledFileURL → Data(contentsOf:)
    SchemeHandler-->>BrowserPanel: home.html + homeSurface.mjs
    BrowserPanel-->>User: Home webview rendered

    User->>Bridge: "cmuxHome.postMessage({ action: newWorkspace })"
    Bridge->>Bridge: isTrustedHomeFrame check
    Bridge->>AppDelegate: performNewWorkspaceAction()
    AppDelegate-->>User: new workspace created
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant User
    participant TitlebarButton as Titlebar Home Button
    participant AppDelegate
    participant TabManager
    participant Workspace
    participant BrowserPanel
    participant SchemeHandler as CmuxBundledWebViewURLSchemeHandler
    participant Bridge as HomeWebViewBridge

    User->>TitlebarButton: click
    TitlebarButton->>AppDelegate: showHomeInActiveMainWindow()
    AppDelegate->>AppDelegate: resolve active MainWindowContext
    AppDelegate->>TabManager: ensureHomeWorkspace(homeURL:)
    TabManager->>TabManager: "tabs.first { customTitle == title }"
    alt workspace not found
        TabManager->>Workspace: addWorkspace(initialSurface: .browser, initialBrowserURL: homeURL)
        Workspace->>BrowserPanel: init(initialURL: homeURL, transparentBackground: true)
    end
    AppDelegate->>Workspace: setPinned(true), reorderToIndex(0)
    AppDelegate->>BrowserPanel: navigate(to: cmux-webview://home/home.html)
    BrowserPanel->>SchemeHandler: webView(_:start:urlSchemeTask:)
    SchemeHandler->>SchemeHandler: bundledFileURL → Data(contentsOf:)
    SchemeHandler-->>BrowserPanel: home.html + homeSurface.mjs
    BrowserPanel-->>User: Home webview rendered

    User->>Bridge: "cmuxHome.postMessage({ action: newWorkspace })"
    Bridge->>Bridge: isTrustedHomeFrame check
    Bridge->>AppDelegate: performNewWorkspaceAction()
    AppDelegate-->>User: new workspace created
Loading

Reviews (4): Last reviewed commit: "Add home button dashboard" | Re-trigger Greptile

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1846d0fa29

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/ContentView.swift Outdated
AppDelegate.shared?.openPreferencesWindow(debugSource: "home.settings")
}
)
.opacity(sidebarSelectionState.selection == .home ? 1 : 0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Deactivate portal workspaces before showing Home

When Home is opened while a portal-backed panel is selected, this only fades the SwiftUI workspace container out; the WorkspaceContentView above still receives isWorkspaceVisible: presentation.isPanelVisible and isWorkspaceInputActive: isSelectedWorkspace, so terminal/browser AppKit portal views can remain visible and first-responder-capable above this dashboard. In a normal terminal or browser workspace, the Home button can therefore appear to do nothing or leave keystrokes going to the workspace; derive portal visibility/input from sidebarSelectionState.selection == .tabs (or explicitly hide/deactivate portals) before showing Home.

Useful? React with 👍 / 👎.

Comment on lines 10240 to +10247
return "OK"
}

func showHome() -> String {
var didShow = false
v2MainSync {
didShow = AppDelegate.shared?.showHomeInActiveMainWindow() == true
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Debug seam left in production source

showHome() is compiled into release builds but its only caller is the case "show_home" inside #if DEBUG. Any method whose only call site is stripped from shipping builds is dead production code and a test/debug seam. The PR's own testing description confirms this was used purely as a debug verification step ("ran show_home through the tagged socket"). Either guard the method with #if DEBUG to match its call site, or promote it to a genuine production socket command and remove the #if DEBUG guard from the case.

Rule Used: Flag Swift files under a production Sources path (... (source)

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!

Comment thread Sources/ContentView.swift
@@ -16122,7 +16169,270 @@ private struct ExtensionSidebarBrowserStackEndDropDelegate: DropDelegate {
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 270-line home dashboard appended to already massively oversized ContentView.swift

ContentView.swift is already well over 16 000 lines at this diff's context. The cmux file-boundary rule flags large additions to existing oversized files and independently-testable UI logic kept in the app target instead of a small package. The seven private structs here (HomeDashboardView, HomeHeaderView, HomeAction, HomeActionGridView, HomeActionCardView, HomeWorkspaceSectionView, HomeWorkspaceRowView) form a self-contained, independently testable feature that would be a natural candidate for its own file or a HomeUI mini-package.

Rule Used: Flag Swift changes that add too much unrelated res... (source)

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!

@lawrencecchen
lawrencecchen force-pushed the task-home-button-gui branch from 1846d0f to 163f707 Compare June 26, 2026 02:11

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 163f707. Configure here.

Comment thread Sources/ContentView.swift

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 163f707b86

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/ContentView.swift Outdated
Comment on lines +1832 to +1833
tabManager.selectedTabId = workspaceId
sidebarSelectionState.selection = .tabs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale multi-selection when selecting from Home

When a user has multiple workspace rows selected in the sidebar, then opens Home and chooses a workspace, this only updates tabManager.selectedTabId. The existing onChange(of: tabManager.selectedTabId) path only syncs selectedTabIds when its count is already <= 1, so the old multi-selection survives after returning to .tabs; subsequent sidebar context-menu or close actions can still operate on that stale selected set instead of the workspace just chosen. Mirror the sidebar/extension selection paths by resetting selectedTabIds and lastSidebarSelectionIndex and selecting the workspace through selectWorkspace here.

Useful? React with 👍 / 👎.

Comment thread Sources/ContentView.swift Outdated
tabManager: tabManager,
debugSource: "home.newWorkspace"
) == true {
sidebarSelectionState.selection = .tabs

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Move Home exit into shared workspace creation

This reset runs only for the Home card, but the same new-workspace behavior is also exposed through the titlebar button and menu/keyboard shortcuts, which call performNewWorkspaceAction without switching sidebarSelectionState back to .tabs. When Home is active, those entrypoints create/select a workspace behind the dashboard, so the action appears to do nothing; put the Home-to-tabs transition in the shared creation path or in every entrypoint that can run while Home is selected.

Useful? React with 👍 / 👎.

Comment thread Sources/ContentView.swift Outdated
.foregroundStyle(.secondary)

VStack(spacing: 8) {
ForEach(workspaces) { workspace in

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Snapshot Home workspace rows before the ForEach

The root AGENTS.md snapshot-boundary rule says rows below a ForEach must receive immutable value snapshots plus closures only; this ForEach iterates live Workspace ObservableObject references and the row closure reads/captures workspace directly. That bypasses the snapshot discipline used to avoid sidebar/list invalidation churn, so build a value row model such as { id, title, directory } before this boundary and pass only that snapshot into the row/action.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/ContentView.swift`:
- Around line 16174-16437: The Home dashboard UI has been added as a large block
inside ContentView, pushing an already oversized production file further past
the repo guideline. Move HomeDashboardView and its helper types (HomeHeaderView,
HomeAction, HomeActionGridView, HomeActionCardView, HomeWorkspaceSectionView,
and HomeWorkspaceRowView) into a dedicated Swift file under Sources, and update
ContentView to reference the extracted views without changing behavior.
- Around line 1831-1834: Update the Home workspace चयन path so it also clears
any existing sidebar multi-selection when `onSelectWorkspace` switches
`tabManager.selectedTabId` and `sidebarSelectionState.selection` back to
`.tabs`. The issue is that this closure only changes the active workspace,
leaving stale `selectedTabIds` intact; fix it by routing this entrypoint through
the same shared selection update path used elsewhere or by explicitly collapsing
the sidebar selection state in the `onSelectWorkspace` handler. Reference
`onSelectWorkspace`, `tabManager.selectedTabId`, and
`sidebarSelectionState.selection` to keep the behavior consistent across
entrypoints.
🪄 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: 325effc4-f9d1-4140-80da-4c368f653ddd

📥 Commits

Reviewing files that changed from the base of the PR and between 1846d0f and 163f707.

📒 Files selected for processing (9)
  • Resources/Localizable.xcstrings
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/SessionPersistence.swift
  • Sources/TerminalController.swift
  • Sources/Update/MinimalModeSidebarControls.swift
  • Sources/Update/UpdateTitlebarAccessory.swift
  • Sources/WindowDecorationsController.swift
  • Sources/WindowDragHandleView.swift

Comment thread Sources/ContentView.swift Outdated
Comment on lines +1831 to +1834
onSelectWorkspace: { workspaceId in
tabManager.selectedTabId = workspaceId
sidebarSelectionState.selection = .tabs
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Collapse sidebar multi-selection when Home picks a workspace.

This path only updates selectedTabId, so an existing multi-selection survives the jump back to .tabs. onChange(of: tabManager.selectedTabId) intentionally skips rewriting selectedTabIds when count > 1, which means later sidebar batch actions can still target the stale range even though the user explicitly chose one workspace from Home.

Suggested fix
                 onSelectWorkspace: { workspaceId in
-                    tabManager.selectedTabId = workspaceId
+                    selectedTabIds = [workspaceId]
+                    lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == workspaceId }
+                    if let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) {
+                        tabManager.selectWorkspace(workspace)
+                    } else {
+                        tabManager.selectedTabId = workspaceId
+                    }
                     sidebarSelectionState.selection = .tabs
                 },

As per coding guidelines, "When a behavior is exposed through multiple entrypoints ... implement one shared action/model path and verify every entrypoint that should invoke it."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onSelectWorkspace: { workspaceId in
tabManager.selectedTabId = workspaceId
sidebarSelectionState.selection = .tabs
},
onSelectWorkspace: { workspaceId in
selectedTabIds = [workspaceId]
lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == workspaceId }
if let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) {
tabManager.selectWorkspace(workspace)
} else {
tabManager.selectedTabId = workspaceId
}
sidebarSelectionState.selection = .tabs
},
🤖 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 1831 - 1834, Update the Home
workspace चयन path so it also clears any existing sidebar multi-selection when
`onSelectWorkspace` switches `tabManager.selectedTabId` and
`sidebarSelectionState.selection` back to `.tabs`. The issue is that this
closure only changes the active workspace, leaving stale `selectedTabIds`
intact; fix it by routing this entrypoint through the same shared selection
update path used elsewhere or by explicitly collapsing the sidebar selection
state in the `onSelectWorkspace` handler. Reference `onSelectWorkspace`,
`tabManager.selectedTabId`, and `sidebarSelectionState.selection` to keep the
behavior consistent across entrypoints.

Source: Coding guidelines

Comment thread Sources/ContentView.swift Outdated
Comment on lines +16174 to +16437
private struct HomeDashboardView: View {
let workspaces: [Workspace]
let onSelectWorkspace: (UUID) -> Void
let onNewWorkspace: () -> Void
let onNewBrowser: () -> Void
let onOpenCommandPalette: () -> Void
let onOpenSettings: () -> Void

private var recentWorkspaces: ArraySlice<Workspace> {
workspaces.prefix(6)
}

var body: some View {
ScrollView {
VStack(alignment: .leading, spacing: 24) {
HomeHeaderView(
title: String(localized: "home.title", defaultValue: "Home"),
subtitle: String(localized: "home.subtitle", defaultValue: "Graphical overview")
)
HomeActionGridView(
actions: [
HomeAction(
id: "newWorkspace",
title: String(localized: "home.action.newWorkspace.title", defaultValue: "New Workspace"),
subtitle: String(localized: "home.action.newWorkspace.subtitle", defaultValue: "Terminal workspace"),
systemImage: "terminal",
tint: Color(nsColor: .systemGreen),
action: onNewWorkspace
),
HomeAction(
id: "newBrowser",
title: String(localized: "home.action.newBrowser.title", defaultValue: "Browser"),
subtitle: String(localized: "home.action.newBrowser.subtitle", defaultValue: "Browser workspace"),
systemImage: "globe",
tint: Color(nsColor: .systemBlue),
action: onNewBrowser
),
HomeAction(
id: "commandPalette",
title: String(localized: "home.action.commandPalette.title", defaultValue: "Command Palette"),
subtitle: String(localized: "home.action.commandPalette.subtitle", defaultValue: "Commands and workspaces"),
systemImage: "command",
tint: Color(nsColor: .systemPurple),
action: onOpenCommandPalette
),
HomeAction(
id: "settings",
title: String(localized: "home.action.settings.title", defaultValue: "Settings"),
subtitle: String(localized: "home.action.settings.subtitle", defaultValue: "Preferences"),
systemImage: "gearshape",
tint: Color(nsColor: .systemOrange),
action: onOpenSettings
),
]
)

if !recentWorkspaces.isEmpty {
HomeWorkspaceSectionView(
title: String(localized: "home.section.workspaces", defaultValue: "Workspaces"),
workspaces: recentWorkspaces,
onSelectWorkspace: onSelectWorkspace
)
}
}
.frame(maxWidth: 900, alignment: .leading)
.padding(.horizontal, 32)
.padding(.vertical, 34)
}
.frame(maxWidth: .infinity, maxHeight: .infinity)
.background {
LinearGradient(
colors: [
Color(nsColor: .windowBackgroundColor),
Color(nsColor: .controlBackgroundColor).opacity(0.65),
],
startPoint: .topLeading,
endPoint: .bottomTrailing
)
.ignoresSafeArea()
}
.accessibilityIdentifier("HomeDashboardView")
}
}

private struct HomeHeaderView: View {
let title: String
let subtitle: String

var body: some View {
HStack(spacing: 14) {
Image(systemName: "house.fill")
.font(.system(size: 24, weight: .semibold))
.foregroundStyle(.white)
.frame(width: 48, height: 48)
.background(
RoundedRectangle(cornerRadius: 8, style: .continuous)
.fill(Color(nsColor: .systemBlue))
)

VStack(alignment: .leading, spacing: 3) {
Text(title)
.cmuxFont(size: 28, weight: .bold)
.foregroundStyle(.primary)
Text(subtitle)
.cmuxFont(size: 13, weight: .medium)
.foregroundStyle(.secondary)
}
}
}
}

private struct HomeAction: Identifiable {
let id: String
let title: String
let subtitle: String
let systemImage: String
let tint: Color
let action: () -> Void
}

private struct HomeActionGridView: View {
let actions: [HomeAction]

private let columns = [
GridItem(.adaptive(minimum: 190, maximum: 260), spacing: 12, alignment: .top)
]

var body: some View {
LazyVGrid(columns: columns, alignment: .leading, spacing: 12) {
ForEach(actions) { action in
Button(action: action.action) {
HomeActionCardView(action: action)
}
.buttonStyle(.plain)
}
}
}
}

private struct HomeActionCardView: View {
let action: HomeAction
@State private var isHovering = false

var body: some View {
HStack(spacing: 12) {
Image(systemName: action.systemImage)
.font(.system(size: 17, weight: .semibold))
.foregroundStyle(action.tint)
.frame(width: 34, height: 34)
.background(
RoundedRectangle(cornerRadius: 7, style: .continuous)
.fill(action.tint.opacity(0.14))
)

VStack(alignment: .leading, spacing: 3) {
Text(action.title)
.cmuxFont(size: 13, weight: .semibold)
.foregroundStyle(.primary)
.lineLimit(1)
Text(action.subtitle)
.cmuxFont(size: 11, weight: .regular)
.foregroundStyle(.secondary)
.lineLimit(2)
.fixedSize(horizontal: false, vertical: true)
}

Spacer(minLength: 4)
}
.padding(12)
.frame(minHeight: 70)
.background(
RoundedRectangle(cornerRadius: 8, style: .continuous)
.fill(Color(nsColor: .controlBackgroundColor).opacity(isHovering ? 0.88 : 0.66))
)
.overlay(
RoundedRectangle(cornerRadius: 8, style: .continuous)
.stroke(Color(nsColor: .separatorColor).opacity(isHovering ? 0.85 : 0.45), lineWidth: 1)
)
.contentShape(RoundedRectangle(cornerRadius: 8, style: .continuous))
.onHover { isHovering = $0 }
}
}

private struct HomeWorkspaceSectionView: View {
let title: String
let workspaces: ArraySlice<Workspace>
let onSelectWorkspace: (UUID) -> Void

var body: some View {
VStack(alignment: .leading, spacing: 10) {
Text(title)
.cmuxFont(size: 13, weight: .semibold)
.foregroundStyle(.secondary)

VStack(spacing: 8) {
ForEach(workspaces) { workspace in
Button {
onSelectWorkspace(workspace.id)
} label: {
HomeWorkspaceRowView(
title: workspace.customTitle ?? workspace.title,
directory: workspace.currentDirectory
)
}
.buttonStyle(.plain)
}
}
}
}
}

private struct HomeWorkspaceRowView: View {
let title: String
let directory: String
@State private var isHovering = false

var body: some View {
HStack(spacing: 10) {
Image(systemName: "rectangle.stack")
.font(.system(size: 14, weight: .semibold))
.foregroundStyle(Color(nsColor: .systemTeal))
.frame(width: 28, height: 28)
.background(
RoundedRectangle(cornerRadius: 7, style: .continuous)
.fill(Color(nsColor: .systemTeal).opacity(0.13))
)

VStack(alignment: .leading, spacing: 2) {
Text(title)
.cmuxFont(size: 12, weight: .semibold)
.foregroundStyle(.primary)
.lineLimit(1)
if !directory.isEmpty {
Text(directory)
.cmuxFont(size: 11, weight: .regular)
.foregroundStyle(.secondary)
.lineLimit(1)
.truncationMode(.middle)
}
}

Spacer()

Image(systemName: "chevron.right")
.font(.system(size: 11, weight: .semibold))
.foregroundStyle(.tertiary)
}
.padding(.horizontal, 12)
.padding(.vertical, 10)
.background(
RoundedRectangle(cornerRadius: 8, style: .continuous)
.fill(Color(nsColor: .controlBackgroundColor).opacity(isHovering ? 0.78 : 0.48))
)
.overlay(
RoundedRectangle(cornerRadius: 8, style: .continuous)
.stroke(Color(nsColor: .separatorColor).opacity(isHovering ? 0.75 : 0.32), lineWidth: 1)
)
.contentShape(RoundedRectangle(cornerRadius: 8, style: .continuous))
.onHover { isHovering = $0 }
}
}

enum SidebarSelection {
case home

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Extract the Home dashboard views into their own file.

This adds a few hundred more production lines to Sources/ContentView.swift, which is already far beyond the repo’s file-size budget. Move HomeDashboardView and its helper views into a dedicated Sources/*.swift file before merge.

As per coding guidelines, "Flag when more than 250 lines are added to an existing production Swift file already over 800 lines"; based on learnings, "avoid bloating the existing file: extract the subview into a dedicated Swift file under Sources."

🤖 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 16174 - 16437, The Home dashboard UI
has been added as a large block inside ContentView, pushing an already oversized
production file further past the repo guideline. Move HomeDashboardView and its
helper types (HomeHeaderView, HomeAction, HomeActionGridView,
HomeActionCardView, HomeWorkspaceSectionView, and HomeWorkspaceRowView) into a
dedicated Swift file under Sources, and update ContentView to reference the
extracted views without changing behavior.

Sources: Coding guidelines, Learnings

@blacksmith-sh

This comment has been minimized.

@lawrencecchen
lawrencecchen force-pushed the task-home-button-gui branch from 163f707 to bdb6320 Compare June 26, 2026 02:50
@lawrencecchen lawrencecchen changed the title Add home button dashboard Add Home workspace webview Jun 26, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bdb632036e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/AppDelegate.swift
Comment on lines +6691 to +6693
let workspace = tabManager.tabs.first { workspace in
workspace.customTitle == title || workspace.title == title
} ?? tabManager.addWorkspace(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Use a stable identifier for Home workspace

When any existing workspace has the localized display title “Home” (for example a user-renamed project or a browser page whose title is Home), this treats that user workspace as the dashboard and the following code then pins/reorders it and navigates or adds a browser surface. That can overwrite the user's current workspace instead of opening the Home dashboard; match a stable marker such as the bundled Home URL or dedicated metadata rather than the mutable display title.

Useful? React with 👍 / 👎.

Comment on lines +15 to +18
id: "newWorkspace",
title: "New Workspace",
subtitle: "Start a terminal workspace",
icon: "terminal",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Localize the Home webview copy

In Japanese or any non-English locale, the new Home webview still renders these hardcoded English labels because this React surface never reads the new Localizable.xcstrings entries or any locale payload. The root AGENTS.md localization rule requires user-facing strings to be localized for supported locales, so the Home header, action titles/subtitles, HTML title/lang, and errors need to come from an en/ja catalog or native-provided localized data before shipping.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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/AppDelegate.swift`:
- Around line 6689-6694: The Home workspace lookup in ensureHomeWorkspace
currently relies on comparing the localized title, which can collide with
user-renamed workspaces and fail across locale changes. Update the workspace
identity logic to use a stable, non-user-editable identifier or persisted flag
on the Workspace created via tabManager.addWorkspace, and have the
tabManager.tabs.first lookup match that identifier instead of title/customTitle.
Keep the pin/reorder behavior unchanged, but ensure the Home workspace is
uniquely recognized regardless of localization or user edits.
- Around line 6647-6650: Remove the NSSound.beep() call from the shared helper
that guards BrowserAvailabilitySettings.isEnabled() and Self.homeWebViewURL(),
since the false return is already handled by callers like ContentView.showHome()
and the minimal-titlebar path. Keep the guard’s return value behavior unchanged,
and leave the beep responsibility to those caller methods so the failure only
alerts once.

In `@Sources/TerminalController.swift`:
- Around line 10243-10248: The showHome command in showHome() is returning a
misleading specific error when AppDelegate.shared?.showHomeInActiveMainWindow()
returns false for non-window failures. Update the failure branch in showHome()
to return a generic error message instead of "ERROR: No main window", keeping
the success path unchanged and using the showHomeInActiveMainWindow() result
only to decide OK vs failure.

In `@webviews/src/home.css`:
- Line 14: Fix the Stylelint violations in home.css by adjusting the declaration
order/spacing around the relevant rule blocks and changing the `currentColor`
value in the affected style to lowercase `currentcolor`. Update the CSS near the
`background` declaration and the rule using `currentColor` so they comply with
`declaration-empty-line-before` and `value-keyword-case`, ensuring the
stylesheet passes CI.

In `@webviews/src/surfaces/homeSurface.tsx`:
- Around line 45-66: The Home UI is currently exposing internal bridge failure
details from runHomeAction and HomeSurface, including reply.error.code and raw
Error.message text. Update HomeSurface/handleAction to translate all native or
thrown failures into stable user-facing copy, and keep the underlying
code/details only for diagnostics or logging. Preserve the bridge-specific error
handling in runHomeAction, but do not pass internal codes like invalid_request
or not_allowed through to setError or the page.
🪄 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: ba94699c-63fb-47de-ba2a-afd51a788cb0

📥 Commits

Reviewing files that changed from the base of the PR and between 163f707 and bdb6320.

📒 Files selected for processing (28)
  • Resources/Localizable.xcstrings
  • Resources/markdown-viewer/webviews-app/chunks/agentSessionSurface.mjs
  • Resources/markdown-viewer/webviews-app/chunks/diffSurface.mjs
  • Resources/markdown-viewer/webviews-app/chunks/homeSurface.mjs
  • Resources/markdown-viewer/webviews-app/chunks/installWebviewStyles.mjs
  • Resources/markdown-viewer/webviews-app/chunks/router.mjs
  • Resources/markdown-viewer/webviews-app/home.html
  • Resources/markdown-viewer/webviews-app/main.mjs
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/Panels/CmuxBundledWebViewURLSchemeHandler.swift
  • Sources/Panels/HomeWebViewBridge.swift
  • Sources/SessionPersistence.swift
  • Sources/TabManager.swift
  • Sources/TerminalController.swift
  • Sources/Update/MinimalModeSidebarControls.swift
  • Sources/Update/UpdateTitlebarAccessory.swift
  • Sources/WindowDecorationsController.swift
  • Sources/WindowDragHandleView.swift
  • Sources/Workspace.swift
  • cmux.xcodeproj/project.pbxproj
  • scripts/build-webviews-app.sh
  • webviews/src/agent-session/shared/bridge.ts
  • webviews/src/home.css
  • webviews/src/main.tsx
  • webviews/src/router.tsx
  • webviews/src/surfaces/homeSurface.tsx

Comment thread Sources/AppDelegate.swift
Comment on lines +6647 to +6650
guard BrowserAvailabilitySettings.isEnabled(),
let homeURL = Self.homeWebViewURL() else {
NSSound.beep()
return false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the beep from this shared helper.

ContentView.showHome() and the minimal-titlebar callers already beep when this returns false, so this path produces two alert sounds when Home cannot open.

Suggested fix
             guard BrowserAvailabilitySettings.isEnabled(),
                   let homeURL = Self.homeWebViewURL() else {
-                NSSound.beep()
                 return false
             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
guard BrowserAvailabilitySettings.isEnabled(),
let homeURL = Self.homeWebViewURL() else {
NSSound.beep()
return false
guard BrowserAvailabilitySettings.isEnabled(),
let homeURL = Self.homeWebViewURL() else {
return 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 6647 - 6650, Remove the
NSSound.beep() call from the shared helper that guards
BrowserAvailabilitySettings.isEnabled() and Self.homeWebViewURL(), since the
false return is already handled by callers like ContentView.showHome() and the
minimal-titlebar path. Keep the guard’s return value behavior unchanged, and
leave the beep responsibility to those caller methods so the failure only alerts
once.

Comment thread Sources/AppDelegate.swift
Comment on lines +6689 to +6694
func ensureHomeWorkspace(in tabManager: TabManager, homeURL: URL) -> Workspace {
let title = String(localized: "home.workspace.title", defaultValue: "Home")
let workspace = tabManager.tabs.first { workspace in
workspace.customTitle == title || workspace.title == title
} ?? tabManager.addWorkspace(
title: title,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Use a stable Home workspace identifier instead of the localized title.

This lookup can hijack any user workspace renamed to the localized Home title, and it will also miss the existing Home workspace after a locale change, creating or mutating the wrong tab before Lines 6707-6712 pin/reorder it. Home needs a non-user-editable identifier/flag persisted with the workspace, not a title comparison.

As per path instructions, "flag deriving the value from a window/pane/terminal title, name, or process-argv heuristic" for correctness-critical detection/identity.

🤖 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 6689 - 6694, The Home workspace
lookup in ensureHomeWorkspace currently relies on comparing the localized title,
which can collide with user-renamed workspaces and fail across locale changes.
Update the workspace identity logic to use a stable, non-user-editable
identifier or persisted flag on the Workspace created via
tabManager.addWorkspace, and have the tabManager.tabs.first lookup match that
identifier instead of title/customTitle. Keep the pin/reorder behavior
unchanged, but ensure the Home workspace is uniquely recognized regardless of
localization or user edits.

Source: Path instructions

Comment on lines +10243 to +10248
func showHome() -> String {
var didShow = false
v2MainSync {
didShow = AppDelegate.shared?.showHomeInActiveMainWindow() == true
}
return didShow ? "OK" : "ERROR: No main window"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Return a generic failure message here.

showHomeInActiveMainWindow() also returns false when Home is unavailable (for example Browser is disabled or the Home webview URL cannot be resolved), so "ERROR: No main window" is incorrect for reachable failure paths and will mislead socket debugging.

Suggested fix
 func showHome() -> String {
     var didShow = false
     v2MainSync {
         didShow = AppDelegate.shared?.showHomeInActiveMainWindow() == true
     }
-    return didShow ? "OK" : "ERROR: No main window"
+    return didShow ? "OK" : "ERROR: Unable to show Home"
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func showHome() -> String {
var didShow = false
v2MainSync {
didShow = AppDelegate.shared?.showHomeInActiveMainWindow() == true
}
return didShow ? "OK" : "ERROR: No main window"
func showHome() -> String {
var didShow = false
v2MainSync {
didShow = AppDelegate.shared?.showHomeInActiveMainWindow() == true
}
return didShow ? "OK" : "ERROR: Unable to show Home"
🤖 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/TerminalController.swift` around lines 10243 - 10248, The showHome
command in showHome() is returning a misleading specific error when
AppDelegate.shared?.showHomeInActiveMainWindow() returns false for non-window
failures. Update the failure branch in showHome() to return a generic error
message instead of "ERROR: No main window", keeping the success path unchanged
and using the showHomeInActiveMainWindow() result only to decide OK vs failure.

Comment thread webviews/src/home.css
--home-green: #25a244;
--home-violet: #9b5cff;
--home-orange: #ff9500;
background: transparent;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix Stylelint errors that will fail CI.

Two lint violations were flagged in this new file:

  • Line 167: value-keyword-case expects currentcolor (lowercase) instead of currentColor.
  • Line 14: declaration-empty-line-before expects an empty line before the background: transparent; declaration.
🎨 Proposed fixes
   --home-orange: `#ff9500`;
+
   background: transparent;
 .home-action svg {
   fill: none;
-  stroke: currentColor;
+  stroke: currentcolor;

Also applies to: 167-167

🧰 Tools
🪛 Stylelint (17.13.0)

[error] 14-14: Expected empty line before declaration (declaration-empty-line-before)

(declaration-empty-line-before)

🤖 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 `@webviews/src/home.css` at line 14, Fix the Stylelint violations in home.css
by adjusting the declaration order/spacing around the relevant rule blocks and
changing the `currentColor` value in the affected style to lowercase
`currentcolor`. Update the CSS near the `background` declaration and the rule
using `currentColor` so they comply with `declaration-empty-line-before` and
`value-keyword-case`, ensuring the stylesheet passes CI.

Source: Linters/SAST tools

Comment on lines +45 to +66
async function runHomeAction(action: HomeAction): Promise<void> {
const handler = nativeHandler();
if (!handler) {
throw new Error("Home bridge is unavailable.");
}
const reply = await handler.postMessage({ action });
if (!reply?.ok) {
throw new Error(reply?.error?.code ?? "action_failed");
}
}

function HomeSurface() {
const [pendingAction, setPendingAction] = React.useState<HomeAction | null>(null);
const [error, setError] = React.useState<string | null>(null);

async function handleAction(action: HomeAction) {
setPendingAction(action);
setError(null);
try {
await runHomeAction(action);
} catch (err) {
setError(err instanceof Error ? err.message : "Action failed.");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not surface bridge error codes in the Home UI.

Lines 50-52 and Lines 65-66 can render internal bridge failures like invalid_request/not_allowed, or any thrown Error.message, directly into the page. Please map native failures to stable user-facing copy and keep the detailed code only for diagnostics.

Suggested fix
+const HOME_ACTION_ERROR = "Couldn't complete that action. Try again.";
+
 async function runHomeAction(action: HomeAction): Promise<void> {
   const handler = nativeHandler();
   if (!handler) {
-    throw new Error("Home bridge is unavailable.");
+    throw new Error("home_action_failed");
   }
   const reply = await handler.postMessage({ action });
   if (!reply?.ok) {
-    throw new Error(reply?.error?.code ?? "action_failed");
+    throw new Error("home_action_failed");
   }
 }
@@
-    } catch (err) {
-      setError(err instanceof Error ? err.message : "Action failed.");
+    } catch {
+      setError(HOME_ACTION_ERROR);
     } finally {
       setPendingAction(null);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async function runHomeAction(action: HomeAction): Promise<void> {
const handler = nativeHandler();
if (!handler) {
throw new Error("Home bridge is unavailable.");
}
const reply = await handler.postMessage({ action });
if (!reply?.ok) {
throw new Error(reply?.error?.code ?? "action_failed");
}
}
function HomeSurface() {
const [pendingAction, setPendingAction] = React.useState<HomeAction | null>(null);
const [error, setError] = React.useState<string | null>(null);
async function handleAction(action: HomeAction) {
setPendingAction(action);
setError(null);
try {
await runHomeAction(action);
} catch (err) {
setError(err instanceof Error ? err.message : "Action failed.");
const HOME_ACTION_ERROR = "Couldn't complete that action. Try again.";
async function runHomeAction(action: HomeAction): Promise<void> {
const handler = nativeHandler();
if (!handler) {
throw new Error("home_action_failed");
}
const reply = await handler.postMessage({ action });
if (!reply?.ok) {
throw new Error("home_action_failed");
}
}
function HomeSurface() {
const [pendingAction, setPendingAction] = React.useState<HomeAction | null>(null);
const [error, setError] = React.useState<string | null>(null);
async function handleAction(action: HomeAction) {
setPendingAction(action);
setError(null);
try {
await runHomeAction(action);
} catch {
setError(HOME_ACTION_ERROR);
} finally {
setPendingAction(null);
}
}
}
🤖 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 `@webviews/src/surfaces/homeSurface.tsx` around lines 45 - 66, The Home UI is
currently exposing internal bridge failure details from runHomeAction and
HomeSurface, including reply.error.code and raw Error.message text. Update
HomeSurface/handleAction to translate all native or thrown failures into stable
user-facing copy, and keep the underlying code/details only for diagnostics or
logging. Preserve the bridge-specific error handling in runHomeAction, but do
not pass internal codes like invalid_request or not_allowed through to setError
or the page.

@lawrencecchen
lawrencecchen force-pushed the task-home-button-gui branch from bdb6320 to 3f8c6d0 Compare June 26, 2026 03:19

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f8c6d0bfe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/AppDelegate.swift
}
_ = tabManager.reorderWorkspace(tabId: workspace.id, toIndex: 0)
ensureHomeBrowserSurface(in: workspace, homeURL: homeURL)
tabManager.selectedTabId = workspace.id

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Collapse sidebar multi-selection when showing Home

Fresh evidence in this revision is that Home now selects its pinned workspace here by assigning selectedTabId directly. When the sidebar currently has multiple selected workspaces, ContentView's onChange(of: tabManager.selectedTabId) deliberately skips syncing selectedTabIds while the set size is greater than 1, so clicking the Home button leaves the old multi-selection active and subsequent sidebar batch/context-menu actions can still target those stale workspaces instead of Home. Route this through the shared workspace selection/collapse path or post the same collapse event used by keyboard navigation.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
Sources/TerminalController.swift (1)

10243-10249: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Misleading failure message. showHomeInActiveMainWindow() returns false for reachable non-window failures (Browser disabled, Home webview URL unresolvable), so "ERROR: No main window" will misattribute those cases. Return a generic message instead.

🤖 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/TerminalController.swift` around lines 10243 - 10249, The failure
message in showHome is too specific because showHomeInActiveMainWindow can
return false for reasons other than a missing main window. Update showHome in
TerminalController to return a generic ERROR message when didShow is false, and
keep the current success path unchanged so the result no longer misattributes
Browser-disabled or unresolved Home URL cases.
🤖 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.

Duplicate comments:
In `@Sources/TerminalController.swift`:
- Around line 10243-10249: The failure message in showHome is too specific
because showHomeInActiveMainWindow can return false for reasons other than a
missing main window. Update showHome in TerminalController to return a generic
ERROR message when didShow is false, and keep the current success path unchanged
so the result no longer misattributes Browser-disabled or unresolved Home URL
cases.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e6c5cb7c-cb76-4250-b0d8-47436337ca72

📥 Commits

Reviewing files that changed from the base of the PR and between bdb6320 and 3f8c6d0.

📒 Files selected for processing (28)
  • Resources/Localizable.xcstrings
  • Resources/markdown-viewer/webviews-app/chunks/agentSessionSurface.mjs
  • Resources/markdown-viewer/webviews-app/chunks/diffSurface.mjs
  • Resources/markdown-viewer/webviews-app/chunks/homeSurface.mjs
  • Resources/markdown-viewer/webviews-app/chunks/installWebviewStyles.mjs
  • Resources/markdown-viewer/webviews-app/chunks/router.mjs
  • Resources/markdown-viewer/webviews-app/home.html
  • Resources/markdown-viewer/webviews-app/main.mjs
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/Panels/CmuxBundledWebViewURLSchemeHandler.swift
  • Sources/Panels/HomeWebViewBridge.swift
  • Sources/SessionPersistence.swift
  • Sources/TabManager.swift
  • Sources/TerminalController.swift
  • Sources/Update/MinimalModeSidebarControls.swift
  • Sources/Update/UpdateTitlebarAccessory.swift
  • Sources/WindowDecorationsController.swift
  • Sources/WindowDragHandleView.swift
  • Sources/Workspace.swift
  • cmux.xcodeproj/project.pbxproj
  • scripts/build-webviews-app.sh
  • webviews/src/agent-session/shared/bridge.ts
  • webviews/src/home.css
  • webviews/src/main.tsx
  • webviews/src/router.tsx
  • webviews/src/surfaces/homeSurface.tsx

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026

This branch was successfully deployed

1 active deployment
Preview – cmux — 3f8c6d0b Deployed Jun 26, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants