Repository navigation
Fix restored panel unread sidebar badges - #4550
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a per-panel restored-unread indicator (visualOnly vs workspaceUnread) persisted in session snapshots and carried on detached transfers; Workspace replaces the restored-unread ID set with per-panel indicators, updates restore/clear/mark flows and pruning, updates autosave fingerprint hashing, and adds tests for restore/fingerprint behavior. ChangesRestored Unread Contribution State
Sequence Diagram(s)sequenceDiagram
participant Session
participant Workspace
participant Panel
participant TerminalNotificationStore
Session->>Workspace: buildSessionSnapshot()
Workspace->>Session: SessionPanelSnapshot(restoredUnreadContributesToWorkspace)
Session->>Workspace: restoreSessionSnapshot()
Workspace->>Panel: restorePanelUnreadIndicator(panelId, contributesToWorkspaceUnread)
Panel->>Workspace: set restoredUnreadPanelIndicators[panelId] = indicator
Workspace->>Workspace: recompute derived panel-derived workspace unread
Workspace->>TerminalNotificationStore: sync derived workspace/tab unread (unreadCount)
Workspace->>Workspace: detachPanel(panelId) -> DetachedSurfaceTransfer(restoredUnreadIndicator)
Workspace->>Workspace: attachDetachedPanel(transfer) -> restore or remove indicator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (16 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 SummaryThis PR fixes workspace sidebar unread badges being absent after session restore by persisting a per-panel
Confidence Score: 5/5Safe to merge — all restore, detach/attach, and autosave paths are updated consistently, legacy nil snapshots default correctly to contributing, and the fix is covered by new focused tests. The change is narrowly scoped to session restore unread state. The typed enum replaces a plain Bool throughout all code paths, the legacy-nil fallback expression is correct, and four new unit tests exercise exactly the edge cases that previously caused the badge to be missing. No files require special attention. Important Files Changed
Reviews (5): Last reviewed commit: "fix: keep read restored indicators visua..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| restorePanelUnreadIndicator( | ||
| panelId, | ||
| contributesToWorkspaceUnread: snapshot.restoredUnreadContributesToWorkspace == true | ||
| ) |
There was a problem hiding this comment.
restoredUnreadContributesToWorkspace = nil (the field didn't exist). Because nil == true evaluates to false, those restored panels get the visual unread indicator but never contribute to the workspace sidebar badge — the exact bug this PR is fixing. On first boot after upgrading, every user with a pre-existing unread panel in their session will silently skip the badge. The fix only kicks in after they clear the unread and go through another save/restore cycle.
Changing to != false treats nil (legacy, no recorded preference) as "should contribute", which is the correct migration default: every panel that was unread when the session was saved should light up the sidebar badge after restore.
| restorePanelUnreadIndicator( | |
| panelId, | |
| contributesToWorkspaceUnread: snapshot.restoredUnreadContributesToWorkspace == true | |
| ) | |
| restorePanelUnreadIndicator( | |
| panelId, | |
| contributesToWorkspaceUnread: snapshot.restoredUnreadContributesToWorkspace != false | |
| ) |
There was a problem hiding this comment.
Addressed with explicit restoredUnreadContributesToWorkspace persistence and a more precise legacy fallback: nil plus no notification rows contributes to workspace unread, while nil plus read-only notification rows remains visual-only. Covered by testLegacyRestoredPanelUnreadIndicatorMarksWorkspaceUnreadForSidebar and testSessionRestorePreservesFocusedReadIndicatorWithReadNotificationsAsVisualOnly.
— Claude Code
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5b609403-a063-43f8-844a-d848b3eb69e3
📒 Files selected for processing (5)
Sources/SessionPersistence.swiftSources/Workspace+DetachedSurfaceTransfer.swiftSources/Workspace.swiftcmuxTests/WorkspaceManualUnreadTests.swiftcmuxTests/WorkspaceUnitTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
5846f4b to
625ae8b
Compare
625ae8b to
1aec237
Compare
![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 1aec237. Configure here.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/Workspace.swift (1)
1108-1114:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefault legacy
nilrestore flags to contributing.Lines 1110-1111 still infer
falsefrom non-emptynotifications, so an older snapshot withrestoredUnreadContributesToWorkspace == nilcan lose its workspace sidebar badge after restore. Only an explicitfalseshould come back as visual-only here.💡 Proposed fix
- let contributesToWorkspaceUnread = snapshot.restoredUnreadContributesToWorkspace - ?? (snapshot.notifications?.isEmpty ?? true) + let contributesToWorkspaceUnread = snapshot.restoredUnreadContributesToWorkspace != 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/Workspace.swift` around lines 1108 - 1114, The code currently falls back from snapshot.restoredUnreadContributesToWorkspace to inferring from snapshot.notifications, which causes nil (legacy) to be treated as non-contributing; change the fallback so that only an explicit false makes it visual-only. In the block where contributesToWorkspaceUnread is computed (referencing snapshot.restoredUnreadContributesToWorkspace, snapshot.notifications, and passed into restorePanelUnreadIndicator along with panelId), replace the fallback expression with a default of true (e.g. snapshot.restoredUnreadContributesToWorkspace ?? true) so nil => contributes to workspace unread.
🤖 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/Workspace.swift`:
- Around line 1108-1114: The code currently falls back from
snapshot.restoredUnreadContributesToWorkspace to inferring from
snapshot.notifications, which causes nil (legacy) to be treated as
non-contributing; change the fallback so that only an explicit false makes it
visual-only. In the block where contributesToWorkspaceUnread is computed
(referencing snapshot.restoredUnreadContributesToWorkspace,
snapshot.notifications, and passed into restorePanelUnreadIndicator along with
panelId), replace the fallback expression with a default of true (e.g.
snapshot.restoredUnreadContributesToWorkspace ?? true) so nil => contributes to
workspace unread.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 70239ca1-d051-48c7-a162-f2c2bf3bdb71
📒 Files selected for processing (2)
Sources/Workspace.swiftcmuxTests/WorkspaceManualUnreadTests.swift
|
Closeout note for the remaining review-bot rollup: I verified the fallback suggestion against the final restore behavior. Applying a blanket nil => true fallback would reintroduce the read-only/focused visual indicator bug that Cursor flagged. The final restore logic intentionally distinguishes legacy nil with no notification rows, which contributes to workspace unread, from legacy nil with read-only notification rows, which remains visual-only. Covered by testLegacyRestoredPanelUnreadIndicatorMarksWorkspaceUnreadForSidebar and testSessionRestorePreservesFocusedReadIndicatorWithReadNotificationsAsVisualOnly. |
This branch was successfully deployed
1 participant
Summary:
Tests:
Dogfood:
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Adjusts session snapshot/restore and workspace unread projection logic, which can affect unread badge counts across restores and autosave fingerprinting. Risk is moderate due to state-migration/legacy handling and multiple code paths (restore, detach/attach, notification store sync).
Overview
Fixes incorrect workspace sidebar unread badges after session restore by persisting whether a restored panel unread indicator should contribute to workspace unread.
SessionPanelSnapshotnow carriesrestoredUnreadContributesToWorkspace, andWorkspacereplaces the boolean restored-unread set with a typedRestoredPanelUnreadIndicatormap that distinguishes visual-only vs workspace-unread indicators; this is propagated through snapshot restore, detached surface transfer, pruning, and unread syncing.TabManager’s autosave fingerprint now hashes this per-panel contribution, and new unit tests cover restored/legacy snapshots and visual-only restored indicators not affecting workspace unread counts.Reviewed by Cursor Bugbot for commit 75d49c9. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes sidebar unread badges after session restore by persisting whether a restored panel’s indicator contributes to workspace unread, and treating read restored badges as visual‑only. Meets the session restore sidebar notifications requirement and keeps visual‑only indicators out.
restoredUnreadContributesToWorkspaceinSessionPanelSnapshot; nil defaults to contributing for legacy snapshots.restoredUnreadwith typedRestoredPanelUnreadIndicator; use it across restore/detach/attach/prune, panel‑derived workspace unread, andTabManagerautosave fingerprints.Written for commit 75d49c9. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Tests