feat: add rename window support with keyboard shortcut, CLI, and command palette#2634
Open
lucaspizzo wants to merge 1 commit intomanaflow-ai:mainfrom
Open
feat: add rename window support with keyboard shortcut, CLI, and command palette#2634lucaspizzo wants to merge 1 commit intomanaflow-ai:mainfrom
lucaspizzo wants to merge 1 commit intomanaflow-ai:mainfrom
Conversation
|
@lucaspizzo is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
|
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 persistent per-window custom titles with UI, keyboard shortcut, command-palette integration, CLI and JSON‑RPC support; persists titles in session snapshots; exposes Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as User
participant UI as ContentView\n(Command Palette / Shortcut)
participant App as AppDelegate
participant Session as SessionPersistence
participant Socket as TerminalController
User->>UI: trigger rename (shortcut / palette)
UI->>App: requestCommandPaletteRenameWindow(preferredWindow)
App->>App: customWindowTitle(for: windowId)
App-->>UI: prefill title
UI->>App: setCustomWindowTitle(windowId, title)
App->>Session: include customWindowTitle in snapshot
App->>UI: notifyMainWindowContextsDidChange
UI-->>User: update titlebar / switcher
Note over Socket,App: remote rename via JSON-RPC v2
Socket->>App: v2: window.rename(params)
App->>App: validate & resolve window_id
App->>App: setCustomWindowTitle(windowId,title)
App-->>Socket: respond ok + window payload (includes custom_title)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR adds first-class window renaming to cmux via keyboard shortcut ( Confidence Score: 4/5Safe to merge after correcting the wrong --workspace flag in the global CLI usage synopsis. One P1 issue: the global help string for rename-window shows --workspace instead of --window, producing misleading output for anyone reading the top-level CLI usage. All other paths — socket handler, UI command palette flow, session persistence, keyboard shortcut — are implemented correctly and follow established patterns. CLI/cmux.swift line 14406 — global usage synopsis shows wrong flag name. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant KeyboardShortcut as Keyboard Shortcut
participant CommandPalette as Command Palette
participant CLI as cmux CLI
participant AppDelegate
participant SessionPersistence
User->>KeyboardShortcut: Cmd+Shift+Option+R
KeyboardShortcut->>AppDelegate: requestRenameWindowViaCommandPalette()
AppDelegate->>CommandPalette: post(.commandPaletteRenameWindowRequested)
CommandPalette->>User: Show rename input (beginRenameWindowFlow)
User->>CommandPalette: Enter window name + Enter
CommandPalette->>AppDelegate: setCustomWindowTitle(windowId, title)
User->>CommandPalette: Open palette → "Rename Window…"
CommandPalette->>AppDelegate: setCustomWindowTitle(windowId, title)
User->>CLI: cmux rename-window "Gymatch"
CLI->>AppDelegate: socket window.rename {window_id, title}
AppDelegate->>AppDelegate: setCustomWindowTitle(windowId, title)
AppDelegate->>SessionPersistence: snapshot includes customWindowTitle
SessionPersistence-->>AppDelegate: restore customWindowTitle on launch
Greploops — Automatically fix all review issues by running Reviews (2): Last reviewed commit: "feat: add rename window support with key..." | 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLI/cmux.swift (1)
14405-14407:⚠️ Potential issue | 🟡 MinorFix incorrect top-level usage flag for
rename-window.The usage line currently documents
--workspace, but the implementation expects--window. This will mislead users.📝 Proposed fix
- rename-window [--workspace <id|ref>] <title> + rename-window [--window <id|ref|index>] <title>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 14405 - 14407, The usage/help line for the top-level command incorrectly lists "--workspace" for the "rename-window" action; update the help/usage text to use "--window" instead so it matches the implementation that parses the --window flag. Locate the "rename-window" usage string (the help/usage block that contains "rename-workspace ... rename-window [--workspace <id|ref>] <title>") and replace the "--workspace" token with "--window" so the documented flag name matches the CLI option parsed by the rename-window handler.
🧹 Nitpick comments (2)
Sources/TerminalController.swift (2)
3342-3345: Add an explicit main-thread rationale comment for this new socket command.Line 3342 uses
v2MainSyncin a new v2 method; add a short inline reason explaining why main-thread execution is required for this mutation path.As per coding guidelines: “If adding a new socket command, default to off-main handling; require an explicit reason in code comments when main-thread execution is necessary.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalController.swift` around lines 3342 - 3345, Add an inline comment above the v2MainSync call explaining why this socket command must run on the main thread: mention that it touches UI/state in AppDelegate (calling tabManagerFor(windowId:) and setCustomWindowTitle(windowId:title:)) and therefore must be dispatched synchronously to avoid main-thread race conditions and ensure UI consistency; place the comment immediately before the v2MainSync block in TerminalController.swift near the new v2 method so reviewers can see the rationale next to the v2MainSync usage.
3355-3359: Usecustom_titlein rename response for API consistency.Line 3358 returns
"title"whilewindow.list/window.currentexpose"custom_title". This creates unnecessary client special-casing.Proposed response-shape alignment
return .ok([ "window_id": windowId.uuidString, "window_ref": v2Ref(kind: .window, uuid: windowId), - "title": title + "custom_title": title ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalController.swift` around lines 3355 - 3359, The rename response currently returns the key "title" which is inconsistent with other endpoints; change the returned dictionary to use "custom_title" instead of "title" in the response (keep values the same, e.g., use title as the value), i.e. update the return in the function that builds the response containing windowId.uuidString and v2Ref(kind: .window, uuid: windowId) to emit "custom_title": title so it matches window.list/window.current consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 2381-2383: The code path forwards a raw global windowId into winId
and then sends window.rename, which can be invalid if the global --window was
passed as an index; instead resolve/normalize the global window identifier to
the canonical window_id before assigning winId. Update the branch that currently
does "else if let windowId { winId = windowId }" to detect numeric/index-style
windowId and call the existing resolver function (e.g., normalizeWindowId /
resolveWindowId / getWindowIdForIndex used elsewhere) to convert the index to
the real window_id, assign that normalized value to winId, and then use winId
when calling window.rename.
In `@Sources/AppDelegate.swift`:
- Around line 4688-4690: sessionAutosaveFingerprint() currently omits
customWindowTitle so rename-only title changes don't affect
runSessionAutosaveTick; update the fingerprint computation to incorporate
context.customWindowTitle (or the persisted snapshot's customWindowTitle) so
title changes alter the autosave fingerprint, and apply the same addition in the
other fingerprint location referenced (the similar block around the other
snapshot code paths) to ensure title-only edits trigger autosave.
- Line 3899: The restore logic only sets context.customWindowTitle for the
primary window; ensure secondary windows also get their saved title by applying
sessionWindowSnapshot.customWindowTitle inside
createMainWindow(sessionWindowSnapshot:), e.g. when configuring the
NSWindow/WindowGroup context for a restored secondary window set its
customWindowTitle from sessionWindowSnapshot.customWindowTitle (use the same
property used in the primary restore) so renamed non-primary windows keep their
titles after relaunch.
In `@Sources/ContentView.swift`:
- Around line 2810-2818: ContentView's updateTitlebarText() uses
AppDelegate.shared?.customWindowTitle(for:) but ContentView never observes
.mainWindowContextsDidChange (posted by setCustomWindowTitle()), so inline
titlebars can become stale; add a NotificationCenter observer in ContentView
(e.g., during init or onAppear) for .mainWindowContextsDidChange and call
scheduleTitlebarTextRefresh() when received, and remove the observer on
deinit/onDisappear; update any relevant lifecycle methods in the ContentView
class to register/unregister the observer so programmatic renames and the "Clear
Window Name" command trigger a refresh.
In `@Sources/KeyboardShortcutSettings.swift`:
- Line 109: The .renameWindow case in KeyboardShortcutSettings.swift references
the localization key "shortcut.renameWindow.label" which is missing from
Resources/Localizable.xcstrings; add an entry for "shortcut.renameWindow.label"
with the English value "Rename Window" and a corresponding Japanese translation
(e.g., "ウィンドウ名を変更") into the xcstrings catalog so the String(localized:
"shortcut.renameWindow.label", defaultValue: "Rename Window") lookup resolves
correctly.
---
Outside diff comments:
In `@CLI/cmux.swift`:
- Around line 14405-14407: The usage/help line for the top-level command
incorrectly lists "--workspace" for the "rename-window" action; update the
help/usage text to use "--window" instead so it matches the implementation that
parses the --window flag. Locate the "rename-window" usage string (the
help/usage block that contains "rename-workspace ... rename-window [--workspace
<id|ref>] <title>") and replace the "--workspace" token with "--window" so the
documented flag name matches the CLI option parsed by the rename-window handler.
---
Nitpick comments:
In `@Sources/TerminalController.swift`:
- Around line 3342-3345: Add an inline comment above the v2MainSync call
explaining why this socket command must run on the main thread: mention that it
touches UI/state in AppDelegate (calling tabManagerFor(windowId:) and
setCustomWindowTitle(windowId:title:)) and therefore must be dispatched
synchronously to avoid main-thread race conditions and ensure UI consistency;
place the comment immediately before the v2MainSync block in
TerminalController.swift near the new v2 method so reviewers can see the
rationale next to the v2MainSync usage.
- Around line 3355-3359: The rename response currently returns the key "title"
which is inconsistent with other endpoints; change the returned dictionary to
use "custom_title" instead of "title" in the response (keep values the same,
e.g., use title as the value), i.e. update the return in the function that
builds the response containing windowId.uuidString and v2Ref(kind: .window,
uuid: windowId) to emit "custom_title": title so it matches
window.list/window.current consumers.
🪄 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: 3304fc34-3038-4bcf-9d6d-ef1a618aede7
📒 Files selected for processing (7)
CLI/cmux.swiftSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/TerminalController.swift
lucaspizzo
force-pushed
the
feat/rename-window
branch
from
6eb5e55 to
d93ba41
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
Sources/TerminalController.swift (1)
3331-3360: Consider supporting title clearing via the API.The current validation rejects empty/whitespace-only titles, which means clients cannot clear a custom window title through this endpoint. While "Clear Window Name" exists as a separate command palette action, API consumers may expect parity.
One approach: treat a missing or
nulltitleparameter as a clear operation (distinct from the current invalid empty-string case).This is optional since the UI flow uses a separate action, but worth considering for API completeness.
💡 Potential approach to support clearing
private func v2WindowRename(params: [String: Any]) -> V2CallResult { guard let windowId = v2UUID(params, "window_id") else { return .err(code: "invalid_params", message: "Missing or invalid window_id", data: nil) } - guard let titleRaw = v2String(params, "title"), - !titleRaw.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { - return .err(code: "invalid_params", message: "Missing or invalid title", data: nil) - } - - let title = titleRaw.trimmingCharacters(in: .whitespacesAndNewlines) + // If title is missing or null, treat as clear; if present but empty/whitespace, reject + let titleRaw = v2String(params, "title") + let title: String? + if let raw = titleRaw { + let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { + return .err(code: "invalid_params", message: "Title cannot be empty; omit to clear", data: nil) + } + title = trimmed + } else { + title = nil // clear + } + var renamed = false v2MainSync { guard AppDelegate.shared?.tabManagerFor(windowId: windowId) != nil else { return } AppDelegate.shared?.setCustomWindowTitle(windowId: windowId, title: title) renamed = true } // ... rest unchanged, adjust response to handle nil title🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalController.swift` around lines 3331 - 3360, v2WindowRename currently rejects empty/whitespace titles; change validation so a missing/null title or a title that trims to empty is treated as a "clear" operation rather than invalid. Specifically, update v2WindowRename to accept title being absent or null (use v2String optional) and treat titleRaw?.trimmingCharacters(...) == "" as a clear request; then call the window title clearer (e.g., AppDelegate.shared?.setCustomWindowTitle(windowId: windowId, title: nil) or the existing clear method) instead of returning .err, keeping the same renamed flag and success response (return the window_ref and window_id, with title either omitted or set to nil/empty per API contract). Ensure the not_found path still triggers if the window lookup fails.Sources/ContentView.swift (1)
3083-3085: Refresh switcher state explicitly on window-context changes.Now that switcher rows can depend on
summary.customWindowTitle, this hook is doing double duty. Refreshing switcher results here too would avoid relying on incidental rerenders from the titlebar path when another window is renamed while this window’s palette is open.💡 Suggested change
view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .mainWindowContextsDidChange)) { _ in scheduleTitlebarTextRefresh() + if isCommandPalettePresented, commandPaletteListScope == .switcher { + scheduleCommandPaletteResultsRefresh(forceSearchCorpusRefresh: true) + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 3083 - 3085, The onReceive handler for NotificationCenter.default.publisher(for: .mainWindowContextsDidChange) currently only calls scheduleTitlebarTextRefresh(), but switcher rows can depend on summary.customWindowTitle so we must also explicitly refresh the switcher; update the closure created in view.onReceive(...) to invoke the switcher-refresh routine (for example, call the function that recalculates or reloads switcher results / state) in addition to scheduleTitlebarTextRefresh() so window-context changes trigger both titlebar and switcher updates.Sources/AppDelegate.swift (1)
4863-4868: Avoid no-op context-change broadcasts insetCustomWindowTitle.
notifyMainWindowContextsDidChange()is emitted even when the normalized value is unchanged, causing avoidable UI refresh churn.♻️ Suggested refinement
func setCustomWindowTitle(windowId: UUID, title: String?) { guard let context = mainWindowContexts.values.first(where: { $0.windowId == windowId }) else { return } let trimmed = title?.trimmingCharacters(in: .whitespacesAndNewlines) - context.customWindowTitle = (trimmed?.isEmpty ?? true) ? nil : trimmed + let normalized = (trimmed?.isEmpty ?? true) ? nil : trimmed + guard context.customWindowTitle != normalized else { return } + context.customWindowTitle = normalized notifyMainWindowContextsDidChange() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 4863 - 4868, In setCustomWindowTitle, avoid broadcasting unchanged context updates by computing the normalized title (let trimmed = title?.trimmingCharacters(in: .whitespacesAndNewlines) then let normalized = (trimmed?.isEmpty ?? true) ? nil : trimmed) and comparing it to context.customWindowTitle; only assign context.customWindowTitle = normalized and call notifyMainWindowContextsDidChange() when they differ. Use an equality check that handles optionals (compare normalized to context.customWindowTitle) so no-op calls are skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 2371-2393: The rename-window branch can inherit a global windowId
and cause unintended focus changes; change the logic in the "rename-window" case
so it only honors an explicit --window argument (the winArg from parseOption)
and does not fall back to the inherited variable windowId; if no --window is
provided, resolve the target via client.sendV2("window.current") as now, but do
not use the outer-scope windowId variable. Update the conditional that sets
winId (and any use of normalizeWindowHandle) to only accept the local winArg or
the current window lookup so that global/inherited windowId cannot trigger
pre-dispatch focus/activation.
- Around line 7426-7451: The top-level usage text for the "rename-window"
command is inconsistent with the subcommand help: update the usage() entry that
documents "rename-window" so it advertises the --window <id|ref|index> flag (and
matching default text) instead of --workspace <id|ref>, and ensure the example
and flag description match the subcommand's string literal for "rename-window";
keep "rename-workspace" unchanged. Target the usage() logic that builds help for
the "rename-window" subcommand and replace the incorrect flag string and default
wording to match the subcommand block shown ("--window <id|ref|index>", default:
current window).
In `@Sources/ContentView.swift`:
- Around line 2810-2815: The title assembly currently injects a hard-coded
separator by using "\(windowCustomTitle) - \(workspaceName)"; replace this with
a localized format string: fetch a localized format via String(localized:
"window.title.format", defaultValue: "%@ - %@") and then build the title with
String(format: localizedFormat, windowCustomTitle, workspaceName). Update the
branch that assigns title (the code referencing workspaceName, windowCustomTitle
and title) to use this localized formatted string instead of the inline "\( ...
- ... )" interpolation.
---
Nitpick comments:
In `@Sources/AppDelegate.swift`:
- Around line 4863-4868: In setCustomWindowTitle, avoid broadcasting unchanged
context updates by computing the normalized title (let trimmed =
title?.trimmingCharacters(in: .whitespacesAndNewlines) then let normalized =
(trimmed?.isEmpty ?? true) ? nil : trimmed) and comparing it to
context.customWindowTitle; only assign context.customWindowTitle = normalized
and call notifyMainWindowContextsDidChange() when they differ. Use an equality
check that handles optionals (compare normalized to context.customWindowTitle)
so no-op calls are skipped.
In `@Sources/ContentView.swift`:
- Around line 3083-3085: The onReceive handler for
NotificationCenter.default.publisher(for: .mainWindowContextsDidChange)
currently only calls scheduleTitlebarTextRefresh(), but switcher rows can depend
on summary.customWindowTitle so we must also explicitly refresh the switcher;
update the closure created in view.onReceive(...) to invoke the switcher-refresh
routine (for example, call the function that recalculates or reloads switcher
results / state) in addition to scheduleTitlebarTextRefresh() so window-context
changes trigger both titlebar and switcher updates.
In `@Sources/TerminalController.swift`:
- Around line 3331-3360: v2WindowRename currently rejects empty/whitespace
titles; change validation so a missing/null title or a title that trims to empty
is treated as a "clear" operation rather than invalid. Specifically, update
v2WindowRename to accept title being absent or null (use v2String optional) and
treat titleRaw?.trimmingCharacters(...) == "" as a clear request; then call the
window title clearer (e.g., AppDelegate.shared?.setCustomWindowTitle(windowId:
windowId, title: nil) or the existing clear method) instead of returning .err,
keeping the same renamed flag and success response (return the window_ref and
window_id, with title either omitted or set to nil/empty per API contract).
Ensure the not_found path still triggers if the window lookup fails.
🪄 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: 85a05463-11c8-451a-84de-cecb4c817840
📒 Files selected for processing (8)
CLI/cmux.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/TerminalController.swift
✅ Files skipped from review due to trivial changes (2)
- Resources/Localizable.xcstrings
- Sources/TabManager.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/KeyboardShortcutSettings.swift
- Sources/SessionPersistence.swift
lucaspizzo
force-pushed
the
feat/rename-window
branch
from
d93ba41 to
904b864
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Sources/AppDelegate.swift (1)
3899-3899:⚠️ Potential issue | 🟠 MajorUse the normalized setter during restore paths.
Line 3899 and Line 7159 write
customWindowTitledirectly, which bypasses normalization andnotifyMainWindowContextsDidChange(). That can leave restore-time title updates stale until another context change occurs.🛠️ Suggested fix
@@ - context.customWindowTitle = snapshot.customWindowTitle + setCustomWindowTitle(windowId: context.windowId, title: snapshot.customWindowTitle) @@ - if let customTitle = sessionWindowSnapshot?.customWindowTitle { - mainWindowContexts[ObjectIdentifier(window)]?.customWindowTitle = customTitle - } + if let sessionWindowSnapshot { + setCustomWindowTitle(windowId: windowId, title: sessionWindowSnapshot.customWindowTitle) + }Also applies to: 7159-7161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` at line 3899, Replace direct assignments to context.customWindowTitle in the restore paths with the normalized setter so normalization runs and notifyMainWindowContextsDidChange() is triggered; specifically, change the write that sets snapshot.customWindowTitle and the similar writes later in the restore code to call the normalized setter (e.g., context.setCustomWindowTitle(snapshot.customWindowTitle) or the equivalent normalized property setter used elsewhere) instead of assigning context.customWindowTitle directly, and remove any manual notify calls if the normalized setter already invokes notifyMainWindowContextsDidChange().
🧹 Nitpick comments (1)
Sources/TerminalController.swift (1)
3342-3346: Add explicit main-thread rationale and tighten AppDelegate capture.For a newly added socket command, please document why this block must run on main and avoid double optional lookups while marking success.
Suggested patch
let title = titleRaw.trimmingCharacters(in: .whitespacesAndNewlines) var renamed = false + // Main-thread required: mutates AppKit-backed window title state. v2MainSync { - guard AppDelegate.shared?.tabManagerFor(windowId: windowId) != nil else { return } - AppDelegate.shared?.setCustomWindowTitle(windowId: windowId, title: title) + guard let appDelegate = AppDelegate.shared, + appDelegate.tabManagerFor(windowId: windowId) != nil else { return } + appDelegate.setCustomWindowTitle(windowId: windowId, title: title) renamed = true }As per coding guidelines: “If adding a new socket command, default to off-main handling; require an explicit reason in code comments when main-thread execution is necessary.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalController.swift` around lines 3342 - 3346, This block must run on the main thread for UI/window title updates: add a short comment above v2MainSync explaining that setCustomWindowTitle touches UI state and must be performed on the main thread; tighten AppDelegate capture by binding let app = AppDelegate.shared once and use app to call tabManagerFor(windowId:) and setCustomWindowTitle to avoid repeated optional lookups; only set renamed = true after a successful setCustomWindowTitle call (i.e., after the guard confirms the tab manager exists and the title was applied).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/ContentView.swift`:
- Around line 3088-3090: The handler for .mainWindowContextsDidChange only calls
scheduleTitlebarTextRefresh(), leaving the command-palette cache stale; update
the same closure to also invalidate or refresh the palette cache by calling the
command-palette refresh routine (e.g. refreshCommandPaletteCache() or
CommandPalette.shared.refreshCache()), so when contexts change the palette rows
are rebuilt and stale switcher commands are removed; add this call alongside
scheduleTitlebarTextRefresh() inside the onReceive closure.
---
Duplicate comments:
In `@Sources/AppDelegate.swift`:
- Line 3899: Replace direct assignments to context.customWindowTitle in the
restore paths with the normalized setter so normalization runs and
notifyMainWindowContextsDidChange() is triggered; specifically, change the write
that sets snapshot.customWindowTitle and the similar writes later in the restore
code to call the normalized setter (e.g.,
context.setCustomWindowTitle(snapshot.customWindowTitle) or the equivalent
normalized property setter used elsewhere) instead of assigning
context.customWindowTitle directly, and remove any manual notify calls if the
normalized setter already invokes notifyMainWindowContextsDidChange().
---
Nitpick comments:
In `@Sources/TerminalController.swift`:
- Around line 3342-3346: This block must run on the main thread for UI/window
title updates: add a short comment above v2MainSync explaining that
setCustomWindowTitle touches UI state and must be performed on the main thread;
tighten AppDelegate capture by binding let app = AppDelegate.shared once and use
app to call tabManagerFor(windowId:) and setCustomWindowTitle to avoid repeated
optional lookups; only set renamed = true after a successful
setCustomWindowTitle call (i.e., after the guard confirms the tab manager exists
and the title was applied).
🪄 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: e0a69cae-cc4a-4c2f-bfd1-b4072ddcb0cc
📒 Files selected for processing (8)
CLI/cmux.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/TerminalController.swift
✅ Files skipped from review due to trivial changes (1)
- Resources/Localizable.xcstrings
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/SessionPersistence.swift
- Sources/KeyboardShortcutSettings.swift
…and palette Add the ability to rename windows via: - Keyboard shortcut (Cmd+Shift+Option+R, configurable in Settings) - CLI command: `cmux rename-window "title"` - Command palette - Socket command: `window.rename` The custom window title is displayed in the title bar (with workspace name) and persisted across sessions. Includes "Clear Window Name" command to reset. Fixes from review: - Normalize global --window ID before sending window.rename - Restore customWindowTitle for secondary windows on session restore - Include customWindowTitle in autosave fingerprint - Add .mainWindowContextsDidChange observer to refresh titlebar immediately - Add shortcut.renameWindow.label localization key with all languages - Prevent rename-window from stealing focus via global --window pre-dispatch - Fix help text: rename-window uses --window not --workspace - Localize titlebar format string (titlebar.windowWithWorkspace) - Add all 9 missing localization keys for command palette entries
lucaspizzo
force-pushed
the
feat/rename-window
branch
from
904b864 to
88b1180
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/ContentView.swift (1)
2811-2822: Normalize custom window title reads before emptiness checks.These read sites treat non-empty-but-whitespace titles as valid. A shared trimmed/normalized accessor would prevent blank titlebar/switcher labels and keep
windowHasCustomTitleconsistent.♻️ Suggested hardening diff
+ private func normalizedCustomWindowTitle(for windowId: UUID) -> String? { + let raw = AppDelegate.shared?.customWindowTitle(for: windowId) ?? "" + let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) + return trimmed.isEmpty ? nil : trimmed + } - let windowCustomTitle = AppDelegate.shared?.customWindowTitle(for: windowId) + let windowCustomTitle = normalizedCustomWindowTitle(for: windowId) let title: String - if let windowCustomTitle, !windowCustomTitle.isEmpty { + if let windowCustomTitle { title = workspaceName.isEmpty ? windowCustomTitle : String( localized: "titlebar.windowWithWorkspace", defaultValue: "\(windowCustomTitle) - \(workspaceName)" ) } else { title = workspaceName } - if let customTitle = summary.customWindowTitle { + if let customTitle = normalizedCustomWindowTitle(for: summary.windowId) { windowLabelById[summary.windowId] = customTitle } else { windowLabelById[summary.windowId] = String(localized: "commandPalette.switcher.windowLabel", defaultValue: "Window \(index + 1)") } snapshot.setBool( CommandPaletteContextKeys.windowHasCustomTitle, - AppDelegate.shared?.customWindowTitle(for: windowId) != nil + normalizedCustomWindowTitle(for: windowId) != nil )Also applies to: 5956-5960, 6354-6357
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 2811 - 2822, Normalize and trim the custom window title before emptiness checks: replace direct reads of AppDelegate.shared?.customWindowTitle(for:) (and uses of windowCustomTitle) with a single normalized/trimmed accessor (or call a new AppDelegate method like customWindowTitleNormalized(for:)) that trims whitespace/newlines and returns nil or empty if only whitespace; then use that normalized value for title assignment and for the windowHasCustomTitle check so whitespace-only titles are treated as empty across the codepaths referenced (including the other occurrences at the locations noted).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 1866-1869: The current pre-dispatch always calls window.focus for
any command with a windowId except "rename-window", which lets non-focus
commands like "rename-workspace" trigger focus; update the condition to only
call normalizeWindowHandle and client.sendV2("window.focus", ...) when the
command is an explicit focus-intent command (e.g. check command against a
whitelist such as ["focus-window", "activate-window", ...] or use a switch)
rather than excluding specific commands; reference the windowId variable,
command string, normalizeWindowHandle(windowId, client:), and
client.sendV2(method: "window.focus", params:) when implementing the whitelist
to ensure non-focus commands (including "rename-workspace") never cause
window.focus to be invoked.
---
Nitpick comments:
In `@Sources/ContentView.swift`:
- Around line 2811-2822: Normalize and trim the custom window title before
emptiness checks: replace direct reads of
AppDelegate.shared?.customWindowTitle(for:) (and uses of windowCustomTitle) with
a single normalized/trimmed accessor (or call a new AppDelegate method like
customWindowTitleNormalized(for:)) that trims whitespace/newlines and returns
nil or empty if only whitespace; then use that normalized value for title
assignment and for the windowHasCustomTitle check so whitespace-only titles are
treated as empty across the codepaths referenced (including the other
occurrences at the locations noted).
🪄 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: b3dcb4be-f306-4745-acf6-03b143fd4727
📒 Files selected for processing (8)
CLI/cmux.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/KeyboardShortcutSettings.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/TerminalController.swift
✅ Files skipped from review due to trivial changes (4)
- Sources/SessionPersistence.swift
- Sources/TabManager.swift
- Resources/Localizable.xcstrings
- Sources/TerminalController.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/KeyboardShortcutSettings.swift
- Sources/AppDelegate.swift
Comment on lines
+1866
to
1869
| // Exclude non-focus-intent commands that route by explicit window_id. | ||
| if let windowId, command != "rename-window" { | ||
| let normalizedWindow = try normalizeWindowHandle(windowId, client: client) ?? windowId | ||
| _ = try client.sendV2(method: "window.focus", params: ["window_id": normalizedWindow]) |
There was a problem hiding this comment.
Prevent non-focus commands from triggering pre-dispatch window.focus.
This condition now lets rename-workspace go through the pre-focus path, which can raise/focus a window even though it is not a focus-intent command.
🔧 Suggested fix
- if let windowId, command != "rename-window" {
+ if let windowId,
+ command != "rename-window",
+ command != "rename-workspace" {
let normalizedWindow = try normalizeWindowHandle(windowId, client: client) ?? windowId
_ = try client.sendV2(method: "window.focus", params: ["window_id": normalizedWindow])
}As per coding guidelines: "Socket/CLI commands must not steal macOS app focus (no app activation/window raising side effects). Only explicit focus-intent commands may mutate in-app focus/selection."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 1866 - 1869, The current pre-dispatch always
calls window.focus for any command with a windowId except "rename-window", which
lets non-focus commands like "rename-workspace" trigger focus; update the
condition to only call normalizeWindowHandle and client.sendV2("window.focus",
...) when the command is an explicit focus-intent command (e.g. check command
against a whitelist such as ["focus-window", "activate-window", ...] or use a
switch) rather than excluding specific commands; reference the windowId
variable, command string, normalizeWindowHandle(windowId, client:), and
client.sendV2(method: "window.focus", params:) when implementing the whitelist
to ensure non-focus commands (including "rename-workspace") never cause
window.focus to be invoked.
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
Adds first-class window renaming to cmux. Windows can now have persistent custom names that appear in the title bar, Move Workspace picker, command palette switcher, and window list API.
Cmd+Shift+Option+Rto rename the current windowcmux rename-window "Gymatch"renames the current windowwindow.renamemethod;window.listandwindow.currentnow returncustom_titleWindowName - WorkspaceNamewhen a custom title is setBreaking change
cmux rename-windownow renames the window, not the workspace. Usecmux rename-workspacefor workspaces. The tmux-compat mode (cmux tmux rename-window) remains mapped to workspace rename for backward compatibility.Files changed (7)
Sources/AppDelegate.swiftcustomWindowTitleonMainWindowContext+MainWindowSummary, accessor methods,promptRenameWindow()dialog, shortcut handler, session snapshot build/restore,windowLabelsById()prefers custom titlesSources/ContentView.swift.windowcase inCommandPaletteRenameTarget, notification observer,beginRenameWindowFlow(), command registration,palette.clearWindowName,windowHasCustomTitlecontext key, titlebarWindowName - WorkspaceName, switcher labelsSources/KeyboardShortcutSettings.swiftrenameWindowaction withCmd+Shift+Option+RdefaultSources/TabManager.swift.commandPaletteRenameWindowRequestednotification nameSources/SessionPersistence.swiftcustomWindowTitleonSessionWindowSnapshot(optional, backward compatible)Sources/TerminalController.swiftwindow.renamesocket method,custom_titleinwindow.list/window.currentresponsesCLI/cmux.swiftrename-windowcommand (separate fromrename-workspace), new help textDesign decisions
-(space-hyphen-space) between window and workspace namescustomWindowTitleis optional inSessionWindowSnapshot, so old snapshots decode cleanly (nil)rename-windowstill maps to workspace rename, only the native CLI command changes semanticsTest plan
Cmd+Shift+Option+R— rename dialog appears, enter name, title bar updatesWindowName - WorkspaceNameformatcmux rename-window "Gymatch"— renames the current windowcmux rename-workspace "test"— still renames the workspacecmux window list --json— returnscustom_titlefieldcmux tmux rename-window "test"still renames workspaceCloses #2249
Summary by cubic
Adds first-class window renaming to
cmuxwith keyboard, command palette, CLI, and socket support; custom names persist and appear in the title bar, pickers, and API.cmux rename-windownow targets windows (usecmux rename-workspacefor workspaces); tmux-compatcmux tmux rename-windowstill renames the workspace.New Features
cmux rename-window [--window <id|ref|index>] [--] <title>.window.rename;window.list/window.currentincludecustom_title.WindowName - WorkspaceName; Move Workspace picker and the switcher show custom window names.Bug Fixes
--windowbeforewindow.rename; do not pre-focus forrename-window; help now documents--window; clearer window vs. workspace errors.customWindowTitlefor all windows; included in autosave fingerprints..mainWindowContextsDidChange; titlebar format string is localized.shortcut.renameWindow.labeland command palette strings for rename/clear window in all supported languages.Written for commit 88b1180. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes