Repository navigation
Conversation
Adds an isMuted flag to Workspace with full session persistence. The context menu now shows Mute/Unmute Notifications next to Pin/Unpin. addNotification() silently discards notifications for muted workspaces.
|
@anthhub is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds per-workspace notification mute: a context-menu toggle and localized labels, a published Changes
Sequence DiagramsequenceDiagram
participant User
participant ContentView as "ContentView\n(Context Menu)"
participant Workspace
participant SessionPersistence as "Session\nPersistence"
participant NotificationStore as "TerminalNotificationStore"
participant NotificationSystem as "Notification\nSystem"
User->>ContentView: Toggle "Mute/Unmute Notifications"
ContentView->>Workspace: set isMuted = true/false
ContentView->>SessionPersistence: persist snapshot (includes isMuted)
NotificationSystem->>NotificationStore: addNotification(event, tabId)
NotificationStore->>Workspace: lookup workspace by tabId (tabManagerFor)
alt workspace.isMuted == true
NotificationStore--x NotificationSystem: return (suppressed)
else workspace.isMuted == false
NotificationStore->>NotificationSystem: deliver/schedule notification
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 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 implements per-workspace notification muting as requested in #2038, and also bundles an unrelated Key observations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant TabItemView
participant Workspace
participant SessionPersistence
participant TerminalNotificationStore
User->>TabItemView: Right-click workspace tab
TabItemView->>Workspace: read isMuted
TabItemView-->>User: Show "Mute / Unmute Notifications"
User->>TabItemView: Click "Mute Notifications"
TabItemView->>Workspace: isMuted = true
Workspace-->>TabItemView: objectWillChange fires
TabItemView->>TabItemView: workspaceObservationGeneration += 1 (debounced)
Note over Workspace,SessionPersistence: On app quit / session save
Workspace->>SessionPersistence: makeSessionSnapshot() → isMuted: true
Note over SessionPersistence,Workspace: On app relaunch / session restore
SessionPersistence->>Workspace: restoreSessionSnapshot() → isMuted = snapshot.isMuted ?? false
Note over TerminalNotificationStore: During noisy command output
TerminalNotificationStore->>Workspace: tabs.first(id==tabId)?.isMuted
alt isMuted == true
TerminalNotificationStore-->>TerminalNotificationStore: early return (no sound, flash, desktop notification)
else isMuted == false
TerminalNotificationStore->>TerminalNotificationStore: normal notification processing
end
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
10855
to
10896
| enum SidebarPathFormatter { | ||
| static let homeDirectoryPath: String = FileManager.default.homeDirectoryForCurrentUser.path | ||
|
|
||
| /// Maximum number of path segments shown before adding a leading ellipsis. | ||
| /// e.g. `~/a/b/c/d` → `…/c/d` when maxSegments == 2. | ||
| static let maxDisplaySegments: Int = 3 | ||
|
|
||
| static func shortenedPath( | ||
| _ path: String, | ||
| homeDirectoryPath: String = Self.homeDirectoryPath | ||
| ) -> String { | ||
| let trimmed = path.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !trimmed.isEmpty else { return path } | ||
|
|
||
| // Replace home directory prefix with ~ | ||
| let tildeReplaced: String | ||
| if trimmed == homeDirectoryPath { | ||
| return "~" | ||
| } else if trimmed.hasPrefix(homeDirectoryPath + "/") { | ||
| tildeReplaced = "~" + trimmed.dropFirst(homeDirectoryPath.count) | ||
| } else { | ||
| tildeReplaced = trimmed | ||
| } | ||
| if trimmed.hasPrefix(homeDirectoryPath + "/") { | ||
| return "~" + trimmed.dropFirst(homeDirectoryPath.count) | ||
|
|
||
| // Apply smart truncation: keep only the last maxDisplaySegments segments | ||
| // so paths with a long common prefix remain distinguishable in the sidebar. | ||
| // e.g. `~/Desktop/YOKE/Claude Code/Projects/Athlete Merch/nilclub` | ||
| // → `…/Projects/Athlete Merch/nilclub` | ||
| let segments = tildeReplaced.split(separator: "/", omittingEmptySubsequences: false) | ||
| // For a tilde-replaced path like `~/a/b/c`, split on "/" yields | ||
| // ["~", "a", "b", "c"]. The tilde token is always a single leading segment. | ||
| // For an absolute path like `/tmp/a/b`, split yields ["", "tmp", "a", "b"]. | ||
| // We only truncate when there are strictly more segments than allowed. | ||
| let effectiveMax = maxDisplaySegments + 1 // +1 for the leading "~" or "" token | ||
| guard segments.count > effectiveMax else { | ||
| return tildeReplaced | ||
| } | ||
| return trimmed | ||
| let tail = segments.suffix(maxDisplaySegments).joined(separator: "/") | ||
| return "…/" + tail | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
The SidebarPathFormatter.shortenedPath smart-truncation rewrite (adding maxDisplaySegments, path ellipsis logic) is a completely separate UX feature from the per-workspace notification mute described in the PR title and description. Bundling it makes:
- The PR scope harder to review in isolation
- Bisecting harder if either feature regresses (a bisect lands on this commit for both changes simultaneously)
- The changelog entry for the path-truncation feature effectively missing
The change is logically correct on its own — the segment-count math handles ~-prefixed and absolute paths properly — but it belongs in its own PR and PR description entry.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/ContentView.swift (1)
11660-11693:⚠️ Potential issue | 🟠 MajorAdd a persistent mute indicator in the row.
This wires the toggle, but
tab.isMutedis only used here inContentView.swift, so after the menu closes there is still no visible sidebar state for muted workspaces. That misses the linked issue’s requested tab-level affordance.🔕 Suggested follow-up near the pin/title cluster
if tab.isPinned { Image(systemName: "pin.fill") .font(.system(size: 9, weight: .semibold)) .foregroundColor(activeSecondaryColor(0.8)) .safeHelp(protectedWorkspaceTooltip) } + if tab.isMuted { + Image(systemName: "bell.slash.fill") + .font(.system(size: 9, weight: .semibold)) + .foregroundColor(activeSecondaryColor(0.8)) + } Text(tab.title) .font(.system(size: 12.5, weight: titleFontWeight)) .foregroundColor(activePrimaryTextColor) + .opacity(tab.isMuted ? 0.72 : 1.0) .lineLimit(1) .truncationMode(.tail) .layoutPriority(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 11660 - 11693, The context menu toggles mute by directly setting workspace.isMuted inside the Button(muteLabel) closure but never updates the tab model/controller or the row UI, so the sidebar shows no persistent mute indicator; change the mutation to use the tab manager API (mirror setPinned pattern) — e.g., add/update a method like tabManager.setMuted(_ tab: Tab, muted: Bool) and call that for each target id instead of assigning workspace.isMuted directly, and ensure the row view that renders the sidebar (the Tab/Workspace row component used by ContentView.swift) reads Tab.isMuted (observable) and shows a mute icon/affordance so the muted state persists and is visible after the menu closes.
🤖 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/TerminalNotificationStore.swift`:
- Around line 895-896: The mute check uses AppDelegate.shared?.tabManager which
only reflects the active window and can miss muted tabs in other windows; change
the lookup to use AppDelegate.shared?.workspaceFor(tabId: tabId) (or equivalent
global workspace resolver) to find the workspace/tab by tabId across all
mainWindowContexts and then check its isMuted property before returning; update
the conditional that references
AppDelegate.shared?.tabManager?.tabs.first(where: { $0.id == tabId })?.isMuted
to resolve the workspace/tab via workspaceFor(tabId:) and use that result's
isMuted.
---
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 11660-11693: The context menu toggles mute by directly setting
workspace.isMuted inside the Button(muteLabel) closure but never updates the tab
model/controller or the row UI, so the sidebar shows no persistent mute
indicator; change the mutation to use the tab manager API (mirror setPinned
pattern) — e.g., add/update a method like tabManager.setMuted(_ tab: Tab, muted:
Bool) and call that for each target id instead of assigning workspace.isMuted
directly, and ensure the row view that renders the sidebar (the Tab/Workspace
row component used by ContentView.swift) reads Tab.isMuted (observable) and
shows a mute icon/affordance so the muted state persists and is visible after
the menu closes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e635ce85-f2fc-4652-a084-b819cae802d7
📒 Files selected for processing (5)
Resources/Localizable.xcstringsSources/ContentView.swiftSources/SessionPersistence.swiftSources/TerminalNotificationStore.swiftSources/Workspace.swift
added 3 commits
March 25, 2026 20:55
- TerminalNotificationStore: use tabManagerFor(tabId:) to search all windows instead of the global key-window tabManager, so mute takes effect for tabs in non-key windows - ContentView TabItemView ==: add lhs.tab.isMuted == rhs.tab.isMuted so that toggling mute invalidates the .equatable() guard and the context menu label updates correctly
Add lhs.tab.isMuted == rhs.tab.isMuted to TabItemView == so that toggling mute on a workspace invalidates the .equatable() guard and forces the context menu label (Mute / Unmute Notifications) to re-render.
…ai#2148 PR manaflow-ai#2145 inadvertently included SidebarPathFormatter maxDisplaySegments and shortenedPath truncation logic that belongs to PR manaflow-ai#2148 (sidebar path optimisation). This commit reverts ContentView.swift to origin/main for SidebarPathFormatter while keeping only the mute-related changes: TabItemView == guard, shouldMute/muteLabel vars, mute Button.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 11661-11664: The toggle wired in ContentView sets tab.isMuted but
TabItemView never reads or draws that state; update TabItemView (the view that
renders each tab row) to accept and/or observe tab.isMuted (or the Tab model)
and render a visible muted affordance in the row body (for example a speaker /
muted icon, badge, or dimmed text) when isMuted is true; ensure the view updates
when the property changes and update the row's accessibility label/tooltip to
reflect muted/unmuted so users can discover the status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c907bd10-8d20-4794-86b7-19c87fce6075
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/TerminalNotificationStore.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/TerminalNotificationStore.swift
…s and add muted visual indicator - TerminalNotificationStore.addNotification: replace tabManager? with tabManagerFor(tabId:) for isActiveTab, focusedSurfaceId, and moveTabToTopForNotification so muted workspaces in non-key windows are correctly suppressed - TabItemView: render bell.slash.fill icon when tab.isMuted is true, giving users a persistent visual indication of muted state after the context menu closes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
|
Closing this stale feature PR for now. The underlying idea may still be useful, but this version likely needs to come back as a smaller follow-up after maintainer guidance on mute-state semantics and cross-window behavior. |
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
Implements per-workspace notification mute as requested in #2038.
@Published var isMuted: Bool = falsetoWorkspacevar isMuted: Bool?toSessionWorkspaceSnapshotfor session persistence (optional/backward-compatible — old sessions decodenil→false)isMutedinrestoreSessionSnapshot(_:)so mute state survives app restartTerminalNotificationStore.addNotification()when the workspace is muted — no sound, no tab flash, no desktop notificationenandjaTest plan
while true; do echo hello; sleep 0.5; done)Fixes #2038
🤖 Generated with Claude Code
Summary by cubic
Add per-workspace notification mute with session persistence, a context menu toggle, and a bell-slash icon on muted tabs. Works across windows and fully silences notifications (fixes #2038).
New Features
isMutedonWorkspacewith session persistence (SessionWorkspaceSnapshot.isMutedoptional, default false).TerminalNotificationStore.addNotification(); localized strings for en/ja.Bug Fixes
tabManagerFor(tabId:)for active/focus checks and reordering.TabItemViewEquatable includesisMuted, so the Mute/Unmute label updates after toggling.Written for commit 4058343. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization