Fix stale sidebar preview after workspace reset - #1234
Closed
austinywang wants to merge 3 commits into
Closed
austinywang wants to merge 3 commits into
austinywang wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a sidebar preview management system: shell hooks send a new clear command at prompt, TerminalController exposes Changes
Sequence Diagram(s)sequenceDiagram
participant Shell as Shell (bash/zsh)
participant Socket as CMux Socket
participant TC as TerminalController
participant NS as TerminalNotificationStore
participant WS as Workspace
rect rgba(200,220,255,0.5)
Shell->>Socket: send "clear_sidebar_preview [--tab X --panel Y]"
end
rect rgba(200,255,200,0.5)
Socket->>TC: receive command
TC->>TC: validate session/tab/panel scope
TC->>WS: clearSidebarPreviewIfSourceMatches(panelId, reason)
TC->>NS: (if needed) propagate notification-based updates
end
rect rgba(255,230,200,0.5)
NS->>WS: replace/clear preview based on notification IDs
WS->>WS: update sidebarPreviewText (set/clear)
WS->>UI: publish updated sidebarPreviewText (subscribers update)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
📝 Coding Plan
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 replaces the stale "latest notification history entry" approach for the sidebar subtitle with workspace-owned ephemeral Key observations:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Shell as Shell (bash/zsh)
participant TC as TerminalController
participant NS as TerminalNotificationStore
participant WS as Workspace
participant SB as Sidebar (TabItemView)
Note over Shell,SB: Notification arrives
NS->>NS: addNotification(tabId, surfaceId, title, body)
NS->>WS: clearSidebarPreviewIfNotificationMatches(removedIds)
NS->>WS: setSidebarPreview(notificationId, sourcePanelId, title, body)
WS-->>SB: @Published sidebarPreviewText updated → re-render
Note over Shell,SB: Shell returns to prompt
Shell->>TC: clear_sidebar_preview --tab=X --panel=Y (socket)
TC->>TC: DispatchQueue.main.sync
TC->>WS: clearSidebarPreviewIfSourceMatches(panelId, reason)
WS-->>SB: sidebarPreviewText = nil → re-render
Note over Shell,SB: Terminal history cleared
TC->>WS: clearSidebarPreviewIfSourceMatches(panelId, "surface.clear_history")
WS-->>SB: sidebarPreviewText = nil → re-render
Note over Shell,SB: Workspace renamed / title changed
WS->>WS: applyProcessTitle / setCustomTitle / restore
WS->>WS: clearSidebarPreview(reason: "workspace.*")
WS-->>SB: sidebarPreviewText = nil → re-render
Note over Shell,SB: Panel removed (pruneSurfaceMetadata)
WS->>WS: pruneSurfaceMetadata(validSurfaceIds)
WS->>WS: clearSidebarPreview("surface.pruned") if source panel gone
WS-->>SB: sidebarPreviewText = nil → re-render
Last reviewed commit: 8008abe |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1653
to
+1660
| func clearSidebarPreviewIfSourceMatches(panelId: UUID?, reason: String) { | ||
| guard let sourcePanelId = sidebarPreviewSourcePanelId else { | ||
| clearSidebarPreview(reason: reason) | ||
| return | ||
| } | ||
| guard panelId == nil || sourcePanelId == panelId else { return } | ||
| clearSidebarPreview(reason: reason) | ||
| } |
Contributor
There was a problem hiding this comment.
Workspace-level preview cleared by panel-specific events
When sidebarPreviewSourcePanelId is nil — which is valid when the active preview came from a workspace-level notification (surfaceId: nil in addNotification) — the guard falls through and unconditionally calls clearSidebarPreview. This means any panel-specific event (e.g. a git branch change on panel B) will also erase a workspace-level notification preview, because those callers pass a specific panelId:
// gitBranch.changed — a per-panel call, not workspace-wide
clearSidebarPreviewIfSourceMatches(panelId: panelId, reason: "gitBranch.changed")The intent of IfSourceMatches is to only clear when the panel causing the event matches the panel that originated the preview. When the source is nil (workspace-level) and the incoming event is for a specific panel, the two don't match and the preview should be kept, not cleared.
Suggested fix:
func clearSidebarPreviewIfSourceMatches(panelId: UUID?, reason: String) {
guard let sourcePanelId = sidebarPreviewSourcePanelId else {
// Source is workspace-level (nil). Only clear for workspace-level events.
if panelId == nil {
clearSidebarPreview(reason: reason)
}
return
}
guard panelId == nil || sourcePanelId == panelId else { return }
clearSidebarPreview(reason: reason)
}This makes the semantics consistent: panelId == nil is a workspace-wide clear (affects all previews regardless of source), while a specific panelId only clears previews that originated from that same panel.
Comment on lines
+960
to
+965
| let removedIds: Set<UUID> = updated | ||
| .filter { $0.id == id } | ||
| .map(\.id) | ||
| .reduce(into: Set<UUID>()) { partialResult, next in | ||
| partialResult.insert(next) | ||
| } |
Contributor
There was a problem hiding this comment.
Overly complex removedIds construction
Since TerminalNotification.id is a UUID and each notification is expected to have a unique ID, the filter $0.id == id will match at most one element. The entire .filter { }.map(\.id).reduce(into:) chain is equivalent to Set([id]). The simpler form also avoids iterating notifications twice (once for removedIds, once for affectedTabIds):
Suggested change
| let removedIds: Set<UUID> = updated | |
| .filter { $0.id == id } | |
| .map(\.id) | |
| .reduce(into: Set<UUID>()) { partialResult, next in | |
| partialResult.insert(next) | |
| } | |
| let removedIds: Set<UUID> = [id] |
Comment on lines
+866
to
+875
| workspace(forTabId: tabId)?.clearSidebarPreviewIfNotificationMatches( | ||
| ids: removedNotificationIds, | ||
| reason: "notification.replaced" | ||
| ) | ||
| workspace(forTabId: tabId)?.setSidebarPreview( | ||
| notificationId: notification.id, | ||
| sourcePanelId: surfaceId, | ||
| title: title, | ||
| body: body | ||
| ) |
Contributor
There was a problem hiding this comment.
Double lookup of workspace(forTabId:)
workspace(forTabId: tabId) is called twice in sequence — once for clearSidebarPreviewIfNotificationMatches and once for setSidebarPreview. Each call walks AppDelegate.shared?.tabManagerFor(tabId:) and then does a linear search through tabManager.tabs. The result can be cached in a local let:
if let ws = workspace(forTabId: tabId) {
ws.clearSidebarPreviewIfNotificationMatches(
ids: removedNotificationIds,
reason: "notification.replaced"
)
ws.setSidebarPreview(
notificationId: notification.id,
sourcePanelId: surfaceId,
title: title,
body: body
)
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
1854-1868:⚠️ Potential issue | 🟠 MajorClosed or moved panels can still leave a stale preview.
This invalidation only runs for callers that go through
pruneSurfaceMetadata(...). The normal panel removal paths insplitTabBar(_:didCloseTab:fromPane:)andsplitTabBar(_:didClosePane:)delete panel state inline without calling this helper orclearSidebarPreviewIfSourceMatches(...), so a preview sourced from that panel can survive after the panel disappears or is moved to another workspace.♻️ Suggested direction
func splitTabBar(_ controller: BonsplitController, didCloseTab tabId: TabID, fromPane pane: PaneID) { ... + clearSidebarPreviewIfSourceMatches(panelId: panelId, reason: "surface.closed") panels.removeValue(forKey: panelId) ... } func splitTabBar(_ controller: BonsplitController, didClosePane paneId: PaneID) { ... for panelId in closedPanelIds { + clearSidebarPreviewIfSourceMatches(panelId: panelId, reason: "surface.closedPane") panels[panelId]?.close() panels.removeValue(forKey: panelId) ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 1854 - 1868, The preview source (sidebarPreviewSourcePanelId) can become stale because some panel removal paths (splitTabBar(_:didCloseTab:fromPane:) and splitTabBar(_:didClosePane:)) remove panel state inline and don't call pruneSurfaceMetadata(...) or clearSidebarPreviewIfSourceMatches(...); update those close handlers to invoke clearSidebarPreviewIfSourceMatches(sourcePanelId:) or call clearSidebarPreview(reason:) when the closed panel ID equals sidebarPreviewSourcePanelId (or simply call pruneSurfaceMetadata after their removals) so the preview is cleared whenever a panel is closed or moved; modify splitTabBar(_:didCloseTab:fromPane:) and splitTabBar(_:didClosePane:) to perform this check and clear.
🤖 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/TerminalController.swift`:
- Around line 4355-4356: The current call to
ws.clearSidebarPreviewIfSourceMatches(panelId: surfaceId, reason:
"surface.clear_history") can clear previews that have no recorded source and
thus remove non-terminal previews; instead, add or use a stricter check that
only clears when the recorded preview source explicitly equals this terminal's
panel id (e.g., implement/use a helper like
ws.clearSidebarPreviewOnlyIfSourceEquals(panelId:surfaceId) or a predicate
method ws.sidebarPreviewSourceEquals(panelId:)), call that before
terminalPanel.surface.forceRefresh(reason:
"terminalController.v2SurfaceClearHistory"), and ensure it does not fall back to
clearSidebarPreview(...) when no source is recorded so only terminal-sourced
previews are removed.
- Around line 13614-13620: The code currently maps a missing/invalid scoped
panelId to nil, causing clearSidebarPreviewIfSourceMatches to perform a
workspace-wide clear; instead preserve the original scope distinction: compute
validSurfaceIds via tab.panels.keys and call pruneSurfaceMetadata as before, but
only call clearSidebarPreviewIfSourceMatches with panelId: scope.panelId if
scope.panelId is nil (meaning a true workspace clear) or if scope.panelId is
non-nil and exists in validSurfaceIds; if scope.panelId is non-nil but not in
validSurfaceIds, skip the clear entirely (do not pass nil). Reference:
validSurfaceIds, tab.pruneSurfaceMetadata(validSurfaceIds:), scope.panelId,
tab.clearSidebarPreviewIfSourceMatches(...).
---
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 1854-1868: The preview source (sidebarPreviewSourcePanelId) can
become stale because some panel removal paths
(splitTabBar(_:didCloseTab:fromPane:) and splitTabBar(_:didClosePane:)) remove
panel state inline and don't call pruneSurfaceMetadata(...) or
clearSidebarPreviewIfSourceMatches(...); update those close handlers to invoke
clearSidebarPreviewIfSourceMatches(sourcePanelId:) or call
clearSidebarPreview(reason:) when the closed panel ID equals
sidebarPreviewSourcePanelId (or simply call pruneSurfaceMetadata after their
removals) so the preview is cleared whenever a panel is closed or moved; modify
splitTabBar(_:didCloseTab:fromPane:) and splitTabBar(_:didClosePane:) to perform
this check and clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2d5fe7f-66e2-457f-a83d-41d80b3cf6ef
📒 Files selected for processing (6)
Resources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zshSources/ContentView.swiftSources/TerminalController.swiftSources/TerminalNotificationStore.swiftSources/Workspace.swift
Comment on lines
+4355
to
4356
| ws.clearSidebarPreviewIfSourceMatches(panelId: surfaceId, reason: "surface.clear_history") | ||
| terminalPanel.surface.forceRefresh(reason: "terminalController.v2SurfaceClearHistory") |
There was a problem hiding this comment.
Only clear terminal-sourced previews here.
clearSidebarPreviewIfSourceMatches is broader than its name suggests. In Sources/Workspace.swift at Lines 1652-1656, it falls back to clearSidebarPreview(...) when no source panel is recorded, so this surface.clear_history path can also erase a notification-owned preview unrelated to surfaceId. Use a stricter check/helper here so history clears only remove previews actually sourced from this terminal.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalController.swift` around lines 4355 - 4356, The current call
to ws.clearSidebarPreviewIfSourceMatches(panelId: surfaceId, reason:
"surface.clear_history") can clear previews that have no recorded source and
thus remove non-terminal previews; instead, add or use a stricter check that
only clears when the recorded preview source explicitly equals this terminal's
panel id (e.g., implement/use a helper like
ws.clearSidebarPreviewOnlyIfSourceEquals(panelId:surfaceId) or a predicate
method ws.sidebarPreviewSourceEquals(panelId:)), call that before
terminalPanel.surface.forceRefresh(reason:
"terminalController.v2SurfaceClearHistory"), and ensure it does not fall back to
clearSidebarPreview(...) when no source is recorded so only terminal-sourced
previews are removed.
Comment on lines
+13614
to
+13620
| let validSurfaceIds = Set(tab.panels.keys) | ||
| tab.pruneSurfaceMetadata(validSurfaceIds: validSurfaceIds) | ||
| let panelId = validSurfaceIds.contains(scope.panelId) ? scope.panelId : nil | ||
| tab.clearSidebarPreviewIfSourceMatches( | ||
| panelId: panelId, | ||
| reason: panelId == nil ? "sidebarPreview.clear.workspace" : "sidebarPreview.clear.surface" | ||
| ) |
There was a problem hiding this comment.
Don't downgrade an invalid scoped panel into a workspace-wide clear.
After pruneSurfaceMetadata, the vanished-source case is already handled. Turning a missing scope.panelId into nil here makes clearSidebarPreviewIfSourceMatches clear the whole workspace instead, so a late prompt hook from a closed/replaced surface can wipe a newer preview from another panel.
[suggested fix]
Diff
- let panelId = validSurfaceIds.contains(scope.panelId) ? scope.panelId : nil
- tab.clearSidebarPreviewIfSourceMatches(
- panelId: panelId,
- reason: panelId == nil ? "sidebarPreview.clear.workspace" : "sidebarPreview.clear.surface"
- )
+ guard validSurfaceIds.contains(scope.panelId) else { return }
+ tab.clearSidebarPreviewIfSourceMatches(
+ panelId: scope.panelId,
+ reason: "sidebarPreview.clear.surface"
+ )📝 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
| let validSurfaceIds = Set(tab.panels.keys) | |
| tab.pruneSurfaceMetadata(validSurfaceIds: validSurfaceIds) | |
| let panelId = validSurfaceIds.contains(scope.panelId) ? scope.panelId : nil | |
| tab.clearSidebarPreviewIfSourceMatches( | |
| panelId: panelId, | |
| reason: panelId == nil ? "sidebarPreview.clear.workspace" : "sidebarPreview.clear.surface" | |
| ) | |
| let validSurfaceIds = Set(tab.panels.keys) | |
| tab.pruneSurfaceMetadata(validSurfaceIds: validSurfaceIds) | |
| guard validSurfaceIds.contains(scope.panelId) else { return } | |
| tab.clearSidebarPreviewIfSourceMatches( | |
| panelId: scope.panelId, | |
| reason: "sidebarPreview.clear.surface" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalController.swift` around lines 13614 - 13620, The code
currently maps a missing/invalid scoped panelId to nil, causing
clearSidebarPreviewIfSourceMatches to perform a workspace-wide clear; instead
preserve the original scope distinction: compute validSurfaceIds via
tab.panels.keys and call pruneSurfaceMetadata as before, but only call
clearSidebarPreviewIfSourceMatches with panelId: scope.panelId if scope.panelId
is nil (meaning a true workspace clear) or if scope.panelId is non-nil and
exists in validSurfaceIds; if scope.panelId is non-nil but not in
validSurfaceIds, skip the clear entirely (do not pass nil). Reference:
validSurfaceIds, tab.pruneSurfaceMetadata(validSurfaceIds:), scope.panelId,
tab.clearSidebarPreviewIfSourceMatches(...).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/TerminalNotificationStore.swift (1)
1011-1017: Consider simplifying the set construction.Since
remove(id:)removes at most one notification (filtering by exact ID match), thefilter/map/reducechain is more complex than necessary. A simpler approach would be:♻️ Suggested simplification
- let removedIds: Set<UUID> = updated - .filter { $0.id == id } - .map(\.id) - .reduce(into: Set<UUID>()) { partialResult, next in - partialResult.insert(next) - } - let affectedTabIds = Set(updated.filter { $0.id == id }.map(\.tabId)) + guard let removed = updated.first(where: { $0.id == id }) else { return } + let removedIds: Set<UUID> = [removed.id] + let affectedTabIds: Set<UUID> = [removed.tabId] updated.removeAll { $0.id == id } - guard updated.count != originalCount else { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/TerminalNotificationStore.swift` around lines 1011 - 1017, The removedIds construction is overly complex for remove(id:) which matches an exact ID; replace the filter/map/reduce chain with a simpler Set creation (e.g., create a Set from the single id or from the filtered map directly) to produce the same Set<UUID> more readably—update the code around the removedIds declaration (and leave affectedTabIds as-is) in the remove(id:) logic.
🤖 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/Workspace.swift`:
- Around line 1689-1696: applyProcessTitle(_:)'s current behavior calls
clearSidebarPreview whenever the terminal OSC title changes, which causes
unwanted preview resets on normal shell churn; remove the
clearSidebarPreview(reason: "workspace.processTitle") call from
applyProcessTitle(_:) and instead invoke clearSidebarPreview only from the
explicit workspace rename/reset code paths (the existing clear_sidebar_preview
hook or the functions that perform workspace rename/reset), ensuring
applyProcessTitle(_:), processTitle, title and customTitle remain responsible
only for updating titles and not for invalidating sidebar previews.
- Around line 1885-1888: The sidebar preview invalidation is only invoked from
pruneSurfaceMetadata(validSurfaceIds:) via the check of
sidebarPreviewSourcePanelId, so when panels are closed/detached in
splitTabBar(_:didCloseTab:fromPane:) and splitTabBar(_:didClosePane:) the
preview can remain stale; update those two methods to call
clearSidebarPreviewIfSourceMatches(panelId:reason:) (or call
clearSidebarPreview(reason:) when appropriate) with the closed/detached panel ID
so the preview is immediately cleared, ensuring the same identifier check used
in the prune path is reused rather than duplicating logic.
---
Nitpick comments:
In `@Sources/TerminalNotificationStore.swift`:
- Around line 1011-1017: The removedIds construction is overly complex for
remove(id:) which matches an exact ID; replace the filter/map/reduce chain with
a simpler Set creation (e.g., create a Set from the single id or from the
filtered map directly) to produce the same Set<UUID> more readably—update the
code around the removedIds declaration (and leave affectedTabIds as-is) in the
remove(id:) logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe3072a1-0edb-49f5-9a4a-bd46353adec2
📒 Files selected for processing (3)
Sources/TerminalNotificationStore.swiftSources/Workspace.swiftcmuxTests/CmuxWebViewKeyEquivalentTests.swift
Comment on lines
1689
to
+1696
| func applyProcessTitle(_ title: String) { | ||
| let previousTitle = self.title | ||
| processTitle = title | ||
| guard customTitle == nil else { return } | ||
| self.title = title | ||
| if previousTitle != self.title { | ||
| clearSidebarPreview(reason: "workspace.processTitle") | ||
| } |
There was a problem hiding this comment.
Don't treat normal terminal title churn as a preview reset.
Sources/TerminalView.swift Lines 267-273 call applyProcessTitle(_:) for every OSC 0/2 title sequence. That makes Line 1695 fire on routine shell activity like cd and prompt redraws, so a still-relevant sidebar preview disappears even though the workspace was not actually reset or renamed. The dedicated clear_sidebar_preview hook already gives you a precise prompt-time invalidation path; this clear should stay on explicit rename/reset flows instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Workspace.swift` around lines 1689 - 1696, applyProcessTitle(_:)'s
current behavior calls clearSidebarPreview whenever the terminal OSC title
changes, which causes unwanted preview resets on normal shell churn; remove the
clearSidebarPreview(reason: "workspace.processTitle") call from
applyProcessTitle(_:) and instead invoke clearSidebarPreview only from the
explicit workspace rename/reset code paths (the existing clear_sidebar_preview
hook or the functions that perform workspace rename/reset), ensuring
applyProcessTitle(_:), processTitle, title and customTitle remain responsible
only for updating titles and not for invalidating sidebar previews.
Comment on lines
+1885
to
+1888
| if let sourcePanelId = sidebarPreviewSourcePanelId, | ||
| !validSurfaceIds.contains(sourcePanelId) { | ||
| clearSidebarPreview(reason: "surface.pruned") | ||
| } |
There was a problem hiding this comment.
Prune-only invalidation misses normal tab/pane close paths.
Line 1885 only covers panel removal when callers go through pruneSurfaceMetadata(validSurfaceIds:), but splitTabBar(_:didCloseTab:fromPane:) at Lines 4732-4746 and splitTabBar(_:didClosePane:) at Lines 4911-4925 remove panel state directly without calling this invalidation or clearSidebarPreviewIfSourceMatches(panelId:reason:). If the preview came from a panel that was closed or detached, the stale subtitle can survive until some later reset.
Minimal fix
func splitTabBar(_ controller: BonsplitController, didCloseTab tabId: TabID, fromPane pane: PaneID) {
...
let panel = panels[panelId]
+ clearSidebarPreviewIfSourceMatches(panelId: panelId, reason: "surface.closed")
`#if` DEBUG
dlog(
"surface.didCloseTab.begin tab=\(String(describing: tabId).prefix(5)) " + if !closedPanelIds.isEmpty {
for panelId in closedPanelIds {
+ clearSidebarPreviewIfSourceMatches(panelId: panelId, reason: "surface.closed")
`#if` DEBUG
dlog(
"surface.didClosePane.panel pane=\(paneId.id.uuidString.prefix(5)) " +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Workspace.swift` around lines 1885 - 1888, The sidebar preview
invalidation is only invoked from pruneSurfaceMetadata(validSurfaceIds:) via the
check of sidebarPreviewSourcePanelId, so when panels are closed/detached in
splitTabBar(_:didCloseTab:fromPane:) and splitTabBar(_:didClosePane:) the
preview can remain stale; update those two methods to call
clearSidebarPreviewIfSourceMatches(panelId:reason:) (or call
clearSidebarPreview(reason:) when appropriate) with the closed/detached panel ID
so the preview is immediately cleared, ensuring the same identifier check used
in the prune path is reused rather than duplicating logic.
3 tasks
lawrencecchen
added
the
stale-revisit
lawrencecchen
added a commit
that referenced
this pull request
Sep 30, 2026
…diffs
- A link or button with a zero-width or zero-height box is left out unless
content inside it has a box that clip/clip-path does not hide (Wikipedia's
zero-width citation backlinks go; an icon overflowing a zero-size link
stays).
- A link to another site prints [url=host/first-segment/…] (48 characters
at most); same-site links stay without URL unless { urls: true }.
- A control with its own ref keeps its name when its children carry refs
(MDN's <summary> disclosures around links).
- Name and text comparisons ignore case ("main content" / "Main content").
- Where a clipped element is left out, the brackets around it close up.
- An interactive snapshot's diff adds text an action added or changed
(from a diff of the full tree), with its ancestor lines, without repeating
lines the diff already has.
- Spec: typed values print unredacted and passwords stay masked, a
deliberate choice.
Goldens reviewed line by line: 03-states (the cross-site "peer" link now
shows [url=127.0.0.1/aria.html]) and 28-compact (the "()" after the
clipped #1234 link is gone). Green: unit 26/26, cmux-dev 29/29, oracle
15/15.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This was referenced Sep 30, 2026
teamleaderleo
added a commit
that referenced
this pull request
Sep 30, 2026
* test(agent-chat): cover GitHub references in the transcript An agent writes `#847`, `owner/repo#847`, `GH-1234` and abbreviated commit SHAs constantly, and the terminal makes all four clickable. The chat transcript does not: remark-gfm autolinks URLs and nothing else, so the same sentence is a link in one pane and plain text in the other. These tests state what the transcript should do, and they fail because the module they call does not exist yet. The rules are deliberately the same as TerminalGitHubReferenceDetector, including the parts that say no: a bare number needs a known repository, a hex run needs both a digit and a letter, and checksum lengths are not commits. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * feat(agent-chat): make GitHub references in the transcript clickable remark-gfm autolinks URLs and nothing else, so `#847`, `owner/repo#847`, `GH-1234` and `a360a95` were plain text in the chat transcript while the terminal pane made all four clickable. The same sentence behaved two different ways depending on which pane you read it in. The reading rules are the TypeScript half of TerminalGitHubReferenceDetector, refusals included: a bare number or a commit SHA needs a known repository, a hex run needs both a digit and a letter so `deadbeef` and `12345678` stay text, and checksum lengths are not commits. Only github.com remotes produce a slug, because a self-hosted host spells its issue URLs against its own domain. References become ordinary markdown links before the transcript parses the source, so the renderer is untouched and a reference gets the same styling and click handling as any other link. Code spans, fenced blocks, existing links and autolinks are left exactly as written. The repository comes from the working-directory check the session already runs, so there is no new round trip; Chat asks for the check when a session arrives without one, which is what happens to a session started outside the composer or restored on a reconnect. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(agent-chat): link GitHub references from the parsed document The first pass rewrote the markdown source before handing it to the parser, and a regular expression cannot stand in for CommonMark's grammar. It wrote `[#847](https://github.com/...)` into double-backtick code spans, `~~~` fenced blocks, indented code blocks, code spans that wrap a line, and link reference definitions, so a reader saw markdown source inside their own code. A stray ``` in prose read as an unterminated fence and silently stopped linking everything after it. Linking now happens in a remark plugin that walks the parsed document's text nodes. The parser has already decided which characters are code, which are an existing link and which are prose, so every one of those cases follows from the tree instead of being re-derived. Four other things came out of the same review: - A number that is still arriving is a prefix of the number the agent means, so `#8471` passed through `#847` on its way in and offered a link to a different issue. The token at the end of a streaming message is left as text until something follows it. - `memo` on the transcript's markdown did not hold: the repository slug was read through a context whose value is a fresh object each render, so every message re-parsed on every streamed token. The slug now has its own context holding a string. - The `git` call that reads the remote passed its timeout into `gitOutput`'s byte limit, so it used the 10 s default on the path that starts a session. It gets 2 s and a 4 KB limit. - The slug cache never expired and never shrank. Entries expire, a miss expires sooner so `git init` in a session directory is picked up, the map is bounded, and concurrent checks share one `git` run. An SSH remote written `git@GitHub.com:` is the same host as one written in lower case, and now reads the same way. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(agent-chat): add GitHub references gallery fixture Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
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
3 participants
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.
Fixes #1232.
Summary
Verification
Summary by cubic
Fixes stale sidebar preview by moving it into workspace state and keeping it in sync with workspace, terminal, and notification events. Prevents old subtitles from sticking after resets, panel changes, or notification updates.
tab.sidebarPreviewTextand render it in the sidebar (no longer derived from the notification list).clear_sidebar_preview(wired into bash/zsh).Written for commit 66e6489. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests