Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1546,6 +1546,9 @@ private final class WindowTmuxWorkspacePaneOverlayController: NSObject {
)
)
super.init()
model.onStateChange = { [weak self] in
self?.renderModelState()
}
containerView.translatesAutoresizingMaskIntoConstraints = false
containerView.wantsLayer = true
containerView.layer?.backgroundColor = NSColor.clear.cgColor
Expand Down Expand Up @@ -1592,25 +1595,22 @@ private final class WindowTmuxWorkspacePaneOverlayController: NSObject {
guard ensureInstalled() else { return }
if let state {
model.apply(state)
hostingView.rootView = TmuxWorkspacePaneOverlayView(
unreadRects: model.unreadRects,
flashRect: model.flashRect,
flashStartedAt: model.flashStartedAt,
flashReason: model.flashReason
)
containerView.alphaValue = 1
containerView.isHidden = false
} else {
model.clear()
hostingView.rootView = TmuxWorkspacePaneOverlayView(
unreadRects: [],
flashRect: nil,
flashStartedAt: nil,
flashReason: nil
)
containerView.alphaValue = 0
containerView.isHidden = true
}
renderModelState()
}

private func renderModelState() {
guard ensureInstalled() else { return }
hostingView.rootView = TmuxWorkspacePaneOverlayView(
unreadRects: model.unreadRects,
flashRect: model.flashRect,
flashStartedAt: model.flashStartedAt,
flashReason: model.flashReason
)
containerView.alphaValue = model.hasVisibleContent ? 1 : 0
containerView.isHidden = !model.hasVisibleContent
}
}

Expand Down
35 changes: 33 additions & 2 deletions Sources/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -766,8 +766,13 @@ class TabManager: ObservableObject {
/// Static so port ranges don't overlap across multiple windows (each window has its own TabManager).
private static var nextPortOrdinal: Int = 0
private static let initialWorkspaceGitProbeDelays: [TimeInterval] = [0, 0.5, 1.5, 3.0, 6.0, 10.0]
private static let workspaceGitMetadataPollInterval: TimeInterval = 30
private static let selectedWorkspaceGitMetadataPollInterval: TimeInterval = 5
// Background PR metadata is best-effort. Keep it infrequent so idle windows
// don't repeatedly spawn `gh` when nothing is changing.
private static let workspaceGitMetadataPollInterval: TimeInterval = 5 * 60
private static let selectedWorkspaceGitMetadataPollInterval: TimeInterval = 2 * 60
// Keep individual `gh` probes long enough for cold launches and slower
// VPN/GitHub Enterprise responses. The lower poll frequency already limits
// idle process churn, so an overly aggressive timeout hurts correctness.
private nonisolated static let workspacePullRequestProbeTimeout: TimeInterval = 5.0
@Published var selectedTabId: UUID? {
willSet {
Expand Down Expand Up @@ -829,6 +834,7 @@ class TabManager: ObservableObject {
if let selectedTabId = self.selectedTabId {
self.dismissFocusedPanelNotificationIfActive(tabId: selectedTabId)
}
self.scheduleSelectedWorkspaceGitMetadataRefresh(reason: "selectionChange")
#if DEBUG
let dtMs = self.debugWorkspaceSwitchStartTime > 0
? (CACurrentMediaTime() - self.debugWorkspaceSwitchStartTime) * 1000
Expand Down Expand Up @@ -1046,6 +1052,26 @@ class TabManager: ObservableObject {
)
}

private func scheduleSelectedWorkspaceGitMetadataRefresh(reason: String) {
guard let workspace = selectedWorkspace,
let focusedPanelId = workspace.focusedPanelId else {
return
}

// Preserve any in-flight bootstrap retry sequence for this workspace.
let probeKey = WorkspaceGitProbeKey(workspaceId: workspace.id, panelId: focusedPanelId)
guard workspaceGitProbeGenerationByKey[probeKey] == nil,
workspaceGitProbeTimersByKey[probeKey] == nil else {
return
}

scheduleWorkspaceGitMetadataRefreshIfPossible(
workspaceId: workspace.id,
panelId: focusedPanelId,
reason: reason
)
}

func refreshTrackedWorkspaceGitMetadataForTesting() {
refreshTrackedWorkspaceGitMetadata()
}
Expand All @@ -1061,6 +1087,11 @@ class TabManager: ObservableObject {
)
}

func workspaceGitProbeAttemptCountForTesting(workspaceId: UUID, panelId: UUID) -> Int {
let probeKey = WorkspaceGitProbeKey(workspaceId: workspaceId, panelId: panelId)
return workspaceGitProbeTimersByKey[probeKey]?.count ?? 0
}

func activeWorkspaceGitProbePanelIdsForTesting(workspaceId: UUID) -> Set<UUID> {
let probeKeys = Set(workspaceGitProbeGenerationByKey.keys.filter { $0.workspaceId == workspaceId })
.union(workspaceGitProbeTimersByKey.keys.filter { $0.workspaceId == workspaceId })
Expand Down
150 changes: 122 additions & 28 deletions Sources/WorkspaceContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,25 @@ struct TmuxWorkspacePaneOverlayRenderState: Equatable {
}

@MainActor
final class TmuxWorkspacePaneOverlayModel: ObservableObject {
@Published private(set) var unreadRects: [CGRect] = []
@Published private(set) var flashRect: CGRect?
@Published private(set) var flashStartedAt: Date?
@Published private(set) var flashReason: WorkspaceAttentionFlashReason?
final class TmuxWorkspacePaneOverlayModel {
private(set) var unreadRects: [CGRect] = []
private(set) var flashRect: CGRect?
private(set) var flashStartedAt: Date?
private(set) var flashReason: WorkspaceAttentionFlashReason?

private var lastWorkspaceId: UUID?
private var lastFlashToken: UInt64?
private var lastObservedFlashToken: UInt64?
private var runningFlashToken: UInt64?
private var flashResetWorkItem: DispatchWorkItem?
var onStateChange: (() -> Void)?

var showsAnimatedFlash: Bool {
flashRect != nil && flashStartedAt != nil
}

var hasVisibleContent: Bool {
!unreadRects.isEmpty || showsAnimatedFlash
}

func apply(
_ state: TmuxWorkspacePaneOverlayRenderState,
Expand All @@ -122,26 +133,88 @@ final class TmuxWorkspacePaneOverlayModel: ObservableObject {
let didChangeWorkspace = lastWorkspaceId != state.workspaceId
if didChangeWorkspace {
lastWorkspaceId = state.workspaceId
lastFlashToken = state.flashToken
lastObservedFlashToken = state.flashToken
runningFlashToken = nil
cancelFlashReset()
flashStartedAt = nil
return
}

if let lastFlashToken,
state.flashToken != lastFlashToken,
let didChangeFlashToken = lastObservedFlashToken != state.flashToken
if didChangeFlashToken {
lastObservedFlashToken = state.flashToken

if state.flashRect != nil {
startFlash(
workspaceId: state.workspaceId,
flashToken: state.flashToken,
startedAt: now()
)
} else {
// A new token supersedes any in-flight flash even when layout churn
// temporarily leaves the pane without a window-space rect.
cancelFlashReset()
flashStartedAt = nil
runningFlashToken = nil
}
return
}

// Preserve an in-flight flash if the pane rect is temporarily missing.
// Once the same token has a rect again, resume from the original start time.
if runningFlashToken == nil,
state.flashRect != nil {
flashStartedAt = now()
startFlash(
workspaceId: state.workspaceId,
flashToken: state.flashToken,
startedAt: now()
)
Comment on lines +165 to +171

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Latch the token after auto-clear.

Because Lines 202-203 clear runningFlashToken, the same flashToken falls back into the Lines 165-171 start path on the next apply(...) with a non-nil rect. Any later layout/unread update can replay the flash without a new attention event, which risks repeated animation work after the flash was supposed to be finished.

🛠️ Minimal fix
         let workItem = DispatchWorkItem { [weak self] in
             guard let self else { return }
             guard self.lastWorkspaceId == workspaceId,
                   self.runningFlashToken == flashToken else {
                 return
             }

             self.flashStartedAt = nil
-            self.runningFlashToken = nil
             self.onStateChange?()
         }

Also applies to: 202-203

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/WorkspaceContentView.swift` around lines 165 - 171, The flash can
restart after auto-clear because the cleared runningFlashToken allows startFlash
to run again with the same state.flashToken; add a small latch: introduce a
lastLatchedFlashToken (or reuse a similarly-scoped var) and when you clear
runningFlashToken in apply(...) also set lastLatchedFlashToken =
state.flashToken, then change the start condition (the block that calls
startFlash with workspaceId/state.flashToken/startedAt) to require that
state.flashToken != lastLatchedFlashToken (in addition to state.flashRect != nil
and runningFlashToken == nil) so the same token cannot immediately re-enter the
start path.

}
self.lastFlashToken = state.flashToken
}

func clear() {
cancelFlashReset()
unreadRects = []
flashRect = nil
flashStartedAt = nil
flashReason = nil
lastWorkspaceId = nil
lastFlashToken = nil
lastObservedFlashToken = nil
runningFlashToken = nil
}

private func startFlash(workspaceId: UUID, flashToken: UInt64, startedAt: Date) {
flashStartedAt = startedAt
runningFlashToken = flashToken
scheduleFlashReset(workspaceId: workspaceId, flashToken: flashToken)
}

private func scheduleFlashReset(workspaceId: UUID, flashToken: UInt64) {
cancelFlashReset()

let workItem = DispatchWorkItem { [weak self] in
guard let self else { return }
guard self.lastWorkspaceId == workspaceId,
self.runningFlashToken == flashToken else {
return
}

self.flashStartedAt = nil
// Keep the handled token latched so later layout churn with the same
// token cannot replay a completed flash. A new token or workspace
// change will reset the latch.
self.onStateChange?()
}
flashResetWorkItem = workItem
DispatchQueue.main.asyncAfter(
deadline: .now() + FocusFlashPattern.duration,
execute: workItem
)
Comment on lines +195 to +212

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 @MainActor-isolated property access in @Sendable closure

DispatchWorkItem takes a @Sendable closure, so Swift Concurrency's type checker sees access to self.lastWorkspaceId, self.lastFlashToken, and self.flashStartedAt (all @MainActor-isolated) as potential data races. At runtime this is safe because the work item is dispatched to DispatchQueue.main, but the compiler cannot verify the equivalence between DispatchQueue.main and @MainActor in a @Sendable closure without an explicit annotation.

Wrapping the body with MainActor.assumeIsolated documents the intent and avoids warnings under -strict-concurrency=complete:

Suggested change
let workItem = DispatchWorkItem { [weak self] in
guard let self else { return }
guard self.lastWorkspaceId == workspaceId,
self.lastFlashToken == flashToken else {
return
}
self.flashStartedAt = nil
}
flashResetWorkItem = workItem
DispatchQueue.main.asyncAfter(
deadline: .now() + FocusFlashPattern.duration,
execute: workItem
)
let workItem = DispatchWorkItem { [weak self] in
MainActor.assumeIsolated {
guard let self else { return }
guard self.lastWorkspaceId == workspaceId,
self.lastFlashToken == flashToken else {
return
}
self.flashStartedAt = nil
}
}

}

private func cancelFlashReset() {
flashResetWorkItem?.cancel()
flashResetWorkItem = nil
}
}

Expand All @@ -152,29 +225,50 @@ struct TmuxWorkspacePaneOverlayView: View {
let flashReason: WorkspaceAttentionFlashReason?

var body: some View {
TimelineView(.animation) { timeline in
Canvas { context, _ in
for rect in unreadRects {
drawUnreadRing(in: &context, rect: rect)
Group {
if flashRect != nil && flashStartedAt != nil {
TimelineView(.animation) { timeline in
Canvas { context, _ in
renderOverlay(
in: &context,
currentDate: timeline.date
)
}
}

guard let flashRect,
let flashStartedAt else { return }
let elapsed = timeline.date.timeIntervalSince(flashStartedAt)
let opacity = FocusFlashPattern.opacity(at: elapsed)
guard opacity > 0.001 else { return }
drawFlashRing(
in: &context,
rect: flashRect,
opacity: opacity,
reason: flashReason ?? .notificationArrival
)
} else if !unreadRects.isEmpty {
Canvas { context, _ in
renderOverlay(in: &context, currentDate: nil)
}
} else {
Color.clear
}
}
.allowsHitTesting(false)
.frame(maxWidth: .infinity, maxHeight: .infinity)
}

private func renderOverlay(
in context: inout GraphicsContext,
currentDate: Date?
) {
for rect in unreadRects {
drawUnreadRing(in: &context, rect: rect)
}

guard let flashRect,
let flashStartedAt,
let currentDate else { return }
let elapsed = currentDate.timeIntervalSince(flashStartedAt)
let opacity = FocusFlashPattern.opacity(at: elapsed)
guard opacity > 0.001 else { return }
drawFlashRing(
in: &context,
rect: flashRect,
opacity: opacity,
reason: flashReason ?? .notificationArrival
)
}

private func drawUnreadRing(in context: inout GraphicsContext, rect: CGRect) {
guard let path = ringPath(for: rect) else { return }
var glowContext = context
Expand Down
Loading
Loading