Repository navigation
Fix stale group name in title bar after rename (#5404) - #5408
Conversation
Renaming a workspace group updates the sidebar header (reads group.name) but the window title bar keeps the old name, because the title bar reads the anchor workspace's own (stale) title. Introduce the resolvedWorkspaceDisplayTitle(for:) seam stubbed at the current buggy behavior, plus a behavioral test that fails on rename. The fix follows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make group.name the single source of truth for a workspace group anchor's displayed name across all window chrome. resolvedWorkspaceDisplayTitle(for:) returns the group's name when the workspace is its group's anchor (which the sidebar already represents exclusively by the group header), otherwise the workspace's own title. The custom title bar, NSWindow.title, and the toolbar command label all route through it, and renameWorkspaceGroup refreshes the NSWindow title inline plus posts workspaceGroupNameDidChange so the two imperatively-cached chrome surfaces re-derive. A rename now propagates to the title bar consistently with the sidebar, eliminating the stale-name class. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughTabManager adds resolvedWorkspaceDisplayTitle(for:) and a workspaceGroupNameDidChange notification. renameWorkspaceGroup posts the notification and refreshes window title; ContentView and WindowToolbarController use the resolved title and observe scoped notifications. Tests cover anchor vs non-anchor rename behavior. ChangesWorkspace group display name propagation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 17 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (17 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryFixes #5404: after renaming a workspace group, window chrome (custom title bar,
Confidence Score: 5/5Safe to merge — the change is narrowly scoped to deriving a display title and wiring notification observers, with no data mutations or auth paths affected. The fix eliminates a dual-source-of-truth bug with a single derivation function. Notification scoping to the posting No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Sidebar
participant TabManager
participant ContentView
participant WindowToolbarController
participant NSWindow
User->>Sidebar: Rename group
Sidebar->>TabManager: renameWorkspaceGroup(groupId:name:)
TabManager->>TabManager: "workspaceGroups[index].name = trimmed"
TabManager->>TabManager: updateWindowTitleForSelectedTab()
TabManager->>TabManager: resolvedWorkspaceDisplayTitle(for: anchor) → group.name
TabManager->>NSWindow: "title = resolvedName"
TabManager->>NotificationCenter: post(.workspaceGroupNameDidChange, object: self)
NotificationCenter->>ContentView: (scoped to this tabManager)
ContentView->>ContentView: scheduleTitlebarTextRefresh()
ContentView->>TabManager: resolvedWorkspaceDisplayTitle(for: selectedTab)
TabManager-->>ContentView: group.name
ContentView->>ContentView: "titlebarText = group.name"
NotificationCenter->>WindowToolbarController: (scoped to this tabManager)
WindowToolbarController->>WindowToolbarController: scheduleFocusedCommandTextUpdate()
WindowToolbarController->>TabManager: resolvedWorkspaceDisplayTitle(for: selectedTab)
TabManager-->>WindowToolbarController: group.name
WindowToolbarController->>WindowToolbarController: "toolbar label = Cmd: group.name"
Reviews (4): Last reviewed commit: "Merge branch 'main' of https://github.co..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| observers.append(center.addObserver( | ||
| forName: .workspaceGroupNameDidChange, | ||
| object: nil, | ||
| queue: .main | ||
| ) { [weak self] _ in | ||
| Task { @MainActor [weak self] in | ||
| self?.scheduleFocusedCommandTextUpdate() | ||
| } | ||
| }) |
There was a problem hiding this comment.
The observer is registered with object: nil, so every WindowToolbarController in every open window refreshes its toolbar label whenever any window renames a group. Because WindowToolbarController.start(tabManager:) already captures the owning tabManager before installObservers() runs, the observer can be scoped to the specific TabManager instance that posts the notification, eliminating spurious cross-window refreshes. The same consideration applies to the parallel observer in ContentView, though there the tabManager reference would need to be passed through the closure capture.
There was a problem hiding this comment.
Good catch — scoped both observers (WindowToolbarController and ContentView) to the notification's object (the posting TabManager, already passed as object: self from renameWorkspaceGroup), so a rename in one window no longer triggers spurious refreshes in others. Fixed in the latest commit.
— Claude Code
Address Greptile P2: the workspaceGroupNameDidChange observers used object: nil, so in a multi-window setup every window's title bar and toolbar refreshed on any window's rename. Scope both observers to the notification's object (the posting TabManager), which the post already identifies via object: self, eliminating spurious cross-window refreshes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…-5404-group-rename-titlebar
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 89a8c91. Configure here.
…-5404-group-rename-titlebar
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/TabManager.swift`:
- Around line 20-88: The WorkspaceGitMetadataProbeLimiter and related types
(WorkspaceGitMetadataProbeWaiter, cancelledWaiterIds logic) should be moved out
of TabManager into their own service boundary (e.g., WorkspaceGitProbeService or
WorkspaceGitMetadataProbeLimiter in a new file) and exposed via a protocol that
TabManager depends on; create a protocol (e.g., WorkspaceGitProbeLimiting) that
declares acquire()/release() async methods, implement it with the existing
WorkspaceGitMetadataProbeLimiter logic in a new source file, update TabManager
to accept the protocol as an injected dependency (initializer/property) and stop
declaring these types in Sources/TabManager.swift, and update callers/tests to
pass a concrete or fake implementation for isolated unit testing.
- Around line 80-86: cancelWaiter can leak UUIDs because you insert into
cancelledWaiterIds when the waiter isn’t in waiters, but later the waiter may be
removed/resumed elsewhere leaving that id forever; fix by removing the id from
cancelledWaiterIds whenever you actually remove/resume a waiter. Concretely, in
cancelWaiter and in the code paths that remove a waiter (the block that does
waiters.remove(at:) and calls waiter.continuation.resume(...)), call
cancelledWaiterIds.remove(id) after/resolution so the set cannot grow unbounded;
keep the existing insert branch for races but ensure every successful
removal/resume of a waiter calls cancelledWaiterIds.remove(id).
🪄 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: 60e581fa-e02f-48d3-b8fe-ee62a8d95bc8
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/TabManager.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/TabManager.swift`:
- Around line 20-88: The WorkspaceGitMetadataProbeLimiter and related types
(WorkspaceGitMetadataProbeWaiter, cancelledWaiterIds logic) should be moved out
of TabManager into their own service boundary (e.g., WorkspaceGitProbeService or
WorkspaceGitMetadataProbeLimiter in a new file) and exposed via a protocol that
TabManager depends on; create a protocol (e.g., WorkspaceGitProbeLimiting) that
declares acquire()/release() async methods, implement it with the existing
WorkspaceGitMetadataProbeLimiter logic in a new source file, update TabManager
to accept the protocol as an injected dependency (initializer/property) and stop
declaring these types in Sources/TabManager.swift, and update callers/tests to
pass a concrete or fake implementation for isolated unit testing.
- Around line 80-86: cancelWaiter can leak UUIDs because you insert into
cancelledWaiterIds when the waiter isn’t in waiters, but later the waiter may be
removed/resumed elsewhere leaving that id forever; fix by removing the id from
cancelledWaiterIds whenever you actually remove/resume a waiter. Concretely, in
cancelWaiter and in the code paths that remove a waiter (the block that does
waiters.remove(at:) and calls waiter.continuation.resume(...)), call
cancelledWaiterIds.remove(id) after/resolution so the set cannot grow unbounded;
keep the existing insert branch for races but ensure every successful
removal/resume of a waiter calls cancelledWaiterIds.remove(id).
🪄 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: 60e581fa-e02f-48d3-b8fe-ee62a8d95bc8
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/TabManager.swift
🛑 Comments failed to post (2)
Sources/TabManager.swift (2)
20-88: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
New git probe orchestration should not keep expanding
TabManager.The new limiter + batched snapshot scheduling logic adds more provider/workstream behavior directly into
Sources/TabManager.swift. This increases coupling in an already very large app-target god file and makes isolated testing harder. Please extract this into a dedicated service/package boundary (with injected dependency intoTabManager) before adding more logic here.As per coding guidelines: “
Sources/**/*.swift: Flag Provider, auth, protocol, parsing, persistence, logging, or workstream logic that needs isolated fakes, fixtures, or unit tests but is hidden behind app-target globals.”Also applies to: 2803-2905
🤖 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/TabManager.swift` around lines 20 - 88, The WorkspaceGitMetadataProbeLimiter and related types (WorkspaceGitMetadataProbeWaiter, cancelledWaiterIds logic) should be moved out of TabManager into their own service boundary (e.g., WorkspaceGitProbeService or WorkspaceGitMetadataProbeLimiter in a new file) and exposed via a protocol that TabManager depends on; create a protocol (e.g., WorkspaceGitProbeLimiting) that declares acquire()/release() async methods, implement it with the existing WorkspaceGitMetadataProbeLimiter logic in a new source file, update TabManager to accept the protocol as an injected dependency (initializer/property) and stop declaring these types in Sources/TabManager.swift, and update callers/tests to pass a concrete or fake implementation for isolated unit testing.
80-86:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftCancellation bookkeeping can leak stale waiter IDs.
At Line 85,
cancelledWaiterIds.insert(id)runs when the waiter is no longer inwaiters. In a cancellation-vs-release race, that can happen after the continuation was already resumedtrue, leaving a UUID that is never removed. Over time this can grow unbounded under churny cancellation.🤖 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/TabManager.swift` around lines 80 - 86, cancelWaiter can leak UUIDs because you insert into cancelledWaiterIds when the waiter isn’t in waiters, but later the waiter may be removed/resumed elsewhere leaving that id forever; fix by removing the id from cancelledWaiterIds whenever you actually remove/resume a waiter. Concretely, in cancelWaiter and in the code paths that remove a waiter (the block that does waiters.remove(at:) and calls waiter.continuation.resume(...)), call cancelledWaiterIds.remove(id) after/resolution so the set cannot grow unbounded; keep the existing insert branch for races but ensure every successful removal/resume of a waiter calls cancelledWaiterIds.remove(id).
This branch was successfully deployed
1 participant
Closes #5404.
Bug
Renaming a workspace group updated the name in the sidebar header but the window title bar kept showing the old name (e.g. stayed
Group 1).Root cause — two sources of truth for the group's display name
At group creation,
createWorkspaceGroupseeds both the anchor workspace'scustomTitle/titleandWorkspaceGroup.nameto the same string (TabManager.swift:5864-5883→addWorkspace(title:)→setCustomTitle). The sidebar header readsgroup.name; the window chrome (custom title bar,NSWindow.title, toolbar command label) reads the selected anchor'stab.title. The anchor is represented exclusively by the group header in the sidebar (SidebarWorkspaceRenderItem.swift:51-53), so its own title is never otherwise shown.renameWorkspaceGroupmutated onlygroup.name. The@Published workspaceGroupschange re-rendered the sidebar, but nothing updated the anchor'stitleand nothing refreshed the imperatively-cached title-bar text — so the two copies drifted.Fix — one observed source
group.namebecomes the single source of truth for an anchor's displayed name, via one derivation on the owner that holds both stores:TabManager.resolvedWorkspaceDisplayTitle(for:)— returns the group'snamewhen the workspace is its group's anchor, otherwise the workspace's owntitle.ContentView.updateTitlebarText),NSWindow.title(TabManager.windowTitle(for:)), and the toolbar command label (WindowToolbarController) all route through it.renameWorkspaceGrouprefreshes theNSWindowtitle inline and postsNotification.Name.workspaceGroupNameDidChange, which the two imperatively-cached chrome surfaces observe to re-derive.This eliminates the whole class of "a window-chrome surface shows a grouped anchor's stale name", not just the title bar.
Tests
Two-commit red/green in
cmuxTests/WorkspaceGroupTests.swift(already wired into the test target):renamingGroupUpdatesAnchorDisplayTitleasserts the anchor's resolved display title follows a rename;renamingGroupLeavesNonAnchorMemberTitleAloneguards the derivation against over-reaching to non-anchor members. The pure title-bar rendering is only reachable via XCUITest; the derivation seam is the cleanly testable behavioral boundary, so the unit tests target that.Localization
No new user-facing strings — the displayed name flows from the already-localized
Group %llddefault (or user input); the new notification is internal. NoLocalizable.xcstringschanges required.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Localized UI title derivation and notification-driven refresh; no auth, data, or persistence changes.
Overview
Fixes #5404: after renaming a workspace group, the sidebar showed the new name but window chrome (custom title bar,
NSWindow.title, toolbar “Cmd:” label) could still show the old anchor title.Window chrome now uses
TabManager.resolvedWorkspaceDisplayTitle(for:), which returns the group’snamewhen the selected tab is a group anchor, and the workspace’s owntitleotherwise.renameWorkspaceGroupupdates the window title immediately and posts.workspaceGroupNameDidChange;ContentViewandWindowToolbarControllerlisten with the notificationobjectset to theirtabManagerso only that window refreshes. Unit tests cover anchor rename and non-anchor members unchanged.Reviewed by Cursor Bugbot for commit 01ca8d4. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #5404: window title bar and toolbar label kept the old group name after rename. Anchors now use the group’s name across window chrome, scoped to only the posting window.
TabManager.resolvedWorkspaceDisplayTitle(for:)to returngroup.namefor anchors; otherwise the workspace’stitle.NSWindow.title, and the toolbar command label through this resolver.NSWindowtitle and post.workspaceGroupNameDidChange;ContentViewandWindowToolbarControllerobserve it withobject: tabManagerto prevent cross-window refresh.Written for commit 01ca8d4. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests