Fix sidebar badges not refreshing on workspace state change (#2046)
* Add regression test for stale sidebar PR refresh * Refresh sidebar badges when workspace metadata changes * Resolve gh for app-side PR probes * Coalesce sidebar redraws during prompt updates
This commit is contained in:
Austin Wang
GitHub
parent
9b3a6ba28b
commit
7ffa447708
3 changed files with 312 additions and 4 deletions
|
|
@ -10893,9 +10893,15 @@ enum SidebarTrailingAccessoryWidthPolicy {
|
|||
// the parent rebuilds with unchanged values. Without this, every TabManager
|
||||
// or NotificationStore publish causes ALL tab items to re-evaluate (~18% of
|
||||
// main thread during typing). If you add new properties, update == below.
|
||||
// Reactive workspace state inside the row must not rely on parent diffs alone:
|
||||
// `.equatable()` can otherwise leave sidebar badges/details stale until an
|
||||
// unrelated parent change sneaks through. Keep the workspace reference plain
|
||||
// and bridge its objectWillChange into local state instead.
|
||||
// Do NOT add @EnvironmentObject or new @Binding without updating ==.
|
||||
// Do NOT remove .equatable() from the ForEach call site in VerticalTabsSidebar.
|
||||
private struct TabItemView: View, Equatable {
|
||||
private static let workspaceObservationCoalesceInterval: RunLoop.SchedulerTimeType.Stride = .milliseconds(40)
|
||||
|
||||
// Closures, Bindings, and object references are excluded from ==
|
||||
// because they're recreated every parent eval but don't affect rendering.
|
||||
nonisolated static func == (lhs: TabItemView, rhs: TabItemView) -> Bool {
|
||||
|
|
@ -10921,7 +10927,7 @@ private struct TabItemView: View, Equatable {
|
|||
let tabManager: TabManager
|
||||
let notificationStore: TerminalNotificationStore
|
||||
@Environment(\.colorScheme) private var colorScheme
|
||||
@ObservedObject var tab: Tab
|
||||
let tab: Tab
|
||||
let index: Int
|
||||
let isActive: Bool
|
||||
let workspaceShortcutDigit: Int?
|
||||
|
|
@ -10941,6 +10947,7 @@ private struct TabItemView: View, Equatable {
|
|||
let remoteContextMenuWorkspaceIds: [UUID]
|
||||
let allRemoteContextMenuTargetsConnecting: Bool
|
||||
let allRemoteContextMenuTargetsDisconnected: Bool
|
||||
@State private var workspaceObservationGeneration: UInt64 = 0
|
||||
@State private var isHovering = false
|
||||
@State private var rowHeight: CGFloat = 1
|
||||
@AppStorage(ShortcutHintDebugSettings.sidebarHintXKey) private var sidebarShortcutHintXOffset = ShortcutHintDebugSettings.defaultSidebarHintX
|
||||
|
|
@ -11148,6 +11155,7 @@ private struct TabItemView: View, Equatable {
|
|||
}
|
||||
|
||||
var body: some View {
|
||||
let _ = workspaceObservationGeneration
|
||||
let closeWorkspaceTooltip = String(localized: "sidebar.closeWorkspace.tooltip", defaultValue: "Close Workspace")
|
||||
let protectedWorkspaceTooltip = String(
|
||||
localized: "sidebar.pinnedWorkspaceProtected.tooltip",
|
||||
|
|
@ -11484,6 +11492,16 @@ private struct TabItemView: View, Equatable {
|
|||
.offset(y: index == 0 ? 0 : -(rowSpacing / 2))
|
||||
}
|
||||
}
|
||||
.onReceive(
|
||||
tab.objectWillChange
|
||||
.receive(on: RunLoop.main)
|
||||
// Prompt-time sidebar telemetry can arrive as a short burst
|
||||
// (pwd, branch, PR, shell state). Coalesce that burst so the
|
||||
// row redraws once with the settled state instead of blinking.
|
||||
.debounce(for: Self.workspaceObservationCoalesceInterval, scheduler: RunLoop.main)
|
||||
) { _ in
|
||||
workspaceObservationGeneration &+= 1
|
||||
}
|
||||
.onDrag {
|
||||
#if DEBUG
|
||||
dlog("sidebar.onDrag tab=\(tab.id.uuidString.prefix(5))")
|
||||
|
|
|
|||
|
|
@ -698,6 +698,7 @@ 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 nonisolated static let workspacePullRequestProbeTimeout: TimeInterval = 5.0
|
||||
@Published var selectedTabId: UUID? {
|
||||
willSet {
|
||||
|
|
@ -809,6 +810,7 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
}
|
||||
private var agentPIDSweepTimer: DispatchSourceTimer?
|
||||
private var workspaceGitMetadataPollTimer: DispatchSourceTimer?
|
||||
#if DEBUG
|
||||
private var debugWorkspaceSwitchCounter: UInt64 = 0
|
||||
private var debugWorkspaceSwitchId: UInt64 = 0
|
||||
|
|
@ -855,7 +857,7 @@ class TabManager: ObservableObject {
|
|||
})
|
||||
|
||||
startAgentPIDSweepTimer()
|
||||
|
||||
startWorkspaceGitMetadataPollTimer()
|
||||
#if DEBUG
|
||||
setupUITestFocusShortcutsIfNeeded()
|
||||
setupSplitCloseRightUITestIfNeeded()
|
||||
|
|
@ -867,6 +869,7 @@ class TabManager: ObservableObject {
|
|||
deinit {
|
||||
workspaceCycleCooldownTask?.cancel()
|
||||
agentPIDSweepTimer?.cancel()
|
||||
workspaceGitMetadataPollTimer?.cancel()
|
||||
}
|
||||
|
||||
// MARK: - Agent PID Sweep
|
||||
|
|
@ -888,6 +891,53 @@ class TabManager: ObservableObject {
|
|||
agentPIDSweepTimer = timer
|
||||
}
|
||||
|
||||
/// Periodically refreshes git/PR metadata for tracked workspace branches so
|
||||
/// remote GitHub state changes (e.g. PR open -> merged) reach sidebar state
|
||||
/// even when the local branch/directory does not change.
|
||||
private func startWorkspaceGitMetadataPollTimer() {
|
||||
let timer = DispatchSource.makeTimerSource(queue: .global(qos: .utility))
|
||||
let interval = Self.workspaceGitMetadataPollInterval
|
||||
timer.schedule(deadline: .now() + interval, repeating: interval)
|
||||
timer.setEventHandler { [weak self] in
|
||||
guard let self else { return }
|
||||
DispatchQueue.main.async { [weak self] in
|
||||
guard let self else { return }
|
||||
self.refreshTrackedWorkspaceGitMetadata()
|
||||
}
|
||||
}
|
||||
timer.resume()
|
||||
workspaceGitMetadataPollTimer = timer
|
||||
}
|
||||
|
||||
private func refreshTrackedWorkspaceGitMetadata() {
|
||||
let activeProbeKeys = Set(workspaceGitProbeGenerationByKey.keys)
|
||||
|
||||
for workspace in tabs {
|
||||
var candidatePanelIds = Set(workspace.panelGitBranches.keys)
|
||||
candidatePanelIds.formUnion(workspace.panelPullRequests.keys)
|
||||
|
||||
if candidatePanelIds.isEmpty,
|
||||
let focusedPanelId = workspace.focusedPanelId,
|
||||
workspace.gitBranch != nil || workspace.pullRequest != nil {
|
||||
candidatePanelIds.insert(focusedPanelId)
|
||||
}
|
||||
|
||||
for panelId in candidatePanelIds {
|
||||
let probeKey = WorkspaceGitProbeKey(workspaceId: workspace.id, panelId: panelId)
|
||||
guard !activeProbeKeys.contains(probeKey) else { continue }
|
||||
scheduleWorkspaceGitMetadataRefreshIfPossible(
|
||||
workspaceId: workspace.id,
|
||||
panelId: panelId,
|
||||
reason: "periodicPoll"
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func refreshTrackedWorkspaceGitMetadataForTesting() {
|
||||
refreshTrackedWorkspaceGitMetadata()
|
||||
}
|
||||
|
||||
private func sweepStaleAgentPIDs() {
|
||||
for tab in tabs {
|
||||
var keysToRemove: [String] = []
|
||||
|
|
@ -1699,6 +1749,69 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
}
|
||||
|
||||
private nonisolated static let fallbackCommandSearchDirectories: [String] = [
|
||||
"/opt/homebrew/bin",
|
||||
"/usr/local/bin",
|
||||
"/opt/local/bin",
|
||||
]
|
||||
|
||||
nonisolated static func resolvedCommandPathForTesting(
|
||||
executable: String,
|
||||
environment: [String: String],
|
||||
fallbackDirectories: [String]
|
||||
) -> String? {
|
||||
resolvedCommandPath(
|
||||
executable: executable,
|
||||
environment: environment,
|
||||
fallbackDirectories: fallbackDirectories
|
||||
)
|
||||
}
|
||||
|
||||
private nonisolated static func resolvedCommandPath(
|
||||
executable: String,
|
||||
environment: [String: String] = ProcessInfo.processInfo.environment,
|
||||
fallbackDirectories: [String] = fallbackCommandSearchDirectories
|
||||
) -> String? {
|
||||
guard !executable.isEmpty else { return nil }
|
||||
let fileManager = FileManager.default
|
||||
if executable.contains("/") {
|
||||
return fileManager.isExecutableFile(atPath: executable) ? executable : nil
|
||||
}
|
||||
|
||||
var searchDirectories: [String] = []
|
||||
var seenDirectories: Set<String> = []
|
||||
|
||||
func appendSearchPath(_ path: String?) {
|
||||
guard let path else { return }
|
||||
for rawComponent in path.split(separator: ":") {
|
||||
let component = String(rawComponent).trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !component.isEmpty,
|
||||
seenDirectories.insert(component).inserted else {
|
||||
continue
|
||||
}
|
||||
searchDirectories.append(component)
|
||||
}
|
||||
}
|
||||
|
||||
appendSearchPath(environment["PATH"])
|
||||
appendSearchPath(getenv("PATH").map { String(cString: $0) })
|
||||
if let bundledBinPath = Bundle.main.resourceURL?.appendingPathComponent("bin").path {
|
||||
appendSearchPath(bundledBinPath)
|
||||
}
|
||||
fallbackDirectories.forEach { appendSearchPath($0) }
|
||||
appendSearchPath("/usr/bin:/bin:/usr/sbin:/sbin")
|
||||
|
||||
for directory in searchDirectories {
|
||||
let candidate = URL(fileURLWithPath: directory, isDirectory: true)
|
||||
.appendingPathComponent(executable)
|
||||
.path
|
||||
if fileManager.isExecutableFile(atPath: candidate) {
|
||||
return candidate
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
private nonisolated static func runCommand(
|
||||
directory: String,
|
||||
executable: String,
|
||||
|
|
@ -1728,8 +1841,13 @@ class TabManager: ObservableObject {
|
|||
let process = Process()
|
||||
let stdout = Pipe()
|
||||
let stderr = Pipe()
|
||||
process.executableURL = URL(fileURLWithPath: "/usr/bin/env")
|
||||
process.arguments = [executable] + arguments
|
||||
if let resolvedExecutable = resolvedCommandPath(executable: executable) {
|
||||
process.executableURL = URL(fileURLWithPath: resolvedExecutable)
|
||||
process.arguments = arguments
|
||||
} else {
|
||||
process.executableURL = URL(fileURLWithPath: "/usr/bin/env")
|
||||
process.arguments = [executable] + arguments
|
||||
}
|
||||
process.currentDirectoryURL = URL(fileURLWithPath: directory)
|
||||
process.standardOutput = stdout
|
||||
process.standardError = stderr
|
||||
|
|
|
|||
|
|
@ -23,6 +23,96 @@ func drainMainQueue() {
|
|||
XCTWaiter().wait(for: [expectation], timeout: 1.0)
|
||||
}
|
||||
|
||||
@discardableResult
|
||||
private func waitForCondition(
|
||||
timeout: TimeInterval = 3.0,
|
||||
pollInterval: TimeInterval = 0.05,
|
||||
file: StaticString = #filePath,
|
||||
line: UInt = #line,
|
||||
_ condition: @escaping () -> Bool
|
||||
) -> Bool {
|
||||
if condition() {
|
||||
return true
|
||||
}
|
||||
|
||||
let expectation = XCTestExpectation(description: "wait for condition")
|
||||
let deadline = Date().addingTimeInterval(timeout)
|
||||
|
||||
func poll() {
|
||||
if condition() {
|
||||
expectation.fulfill()
|
||||
return
|
||||
}
|
||||
guard Date() < deadline else { return }
|
||||
DispatchQueue.main.asyncAfter(deadline: .now() + pollInterval) {
|
||||
poll()
|
||||
}
|
||||
}
|
||||
|
||||
DispatchQueue.main.async {
|
||||
poll()
|
||||
}
|
||||
|
||||
let result = XCTWaiter().wait(for: [expectation], timeout: timeout + pollInterval + 0.1)
|
||||
if result != .completed {
|
||||
XCTFail("Timed out waiting for condition", file: file, line: line)
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
private struct ProcessRunResult {
|
||||
let status: Int32
|
||||
let stdout: String
|
||||
let stderr: String
|
||||
}
|
||||
|
||||
private func runProcess(
|
||||
executablePath: String,
|
||||
arguments: [String],
|
||||
environment: [String: String]? = nil,
|
||||
currentDirectoryURL: URL? = nil
|
||||
) throws -> ProcessRunResult {
|
||||
let process = Process()
|
||||
let stdoutPipe = Pipe()
|
||||
let stderrPipe = Pipe()
|
||||
process.executableURL = URL(fileURLWithPath: executablePath)
|
||||
process.arguments = arguments
|
||||
process.environment = environment
|
||||
process.currentDirectoryURL = currentDirectoryURL
|
||||
process.standardInput = FileHandle.nullDevice
|
||||
process.standardOutput = stdoutPipe
|
||||
process.standardError = stderrPipe
|
||||
try process.run()
|
||||
process.waitUntilExit()
|
||||
return ProcessRunResult(
|
||||
status: process.terminationStatus,
|
||||
stdout: String(data: stdoutPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? "",
|
||||
stderr: String(data: stderrPipe.fileHandleForReading.readDataToEndOfFile(), encoding: .utf8) ?? ""
|
||||
)
|
||||
}
|
||||
|
||||
private func runGit(
|
||||
_ arguments: [String],
|
||||
in directoryURL: URL,
|
||||
file: StaticString = #filePath,
|
||||
line: UInt = #line
|
||||
) throws -> String {
|
||||
let result = try runProcess(
|
||||
executablePath: "/usr/bin/env",
|
||||
arguments: ["git"] + arguments,
|
||||
currentDirectoryURL: directoryURL
|
||||
)
|
||||
XCTAssertEqual(
|
||||
result.status,
|
||||
0,
|
||||
"git \(arguments.joined(separator: " ")) failed: \(result.stderr)",
|
||||
file: file,
|
||||
line: line
|
||||
)
|
||||
return result.stdout
|
||||
}
|
||||
|
||||
@MainActor
|
||||
final class TabManagerChildExitCloseTests: XCTestCase {
|
||||
func testChildExitOnLastPanelClosesSelectedWorkspaceAndKeepsIndexStable() {
|
||||
|
|
@ -210,6 +300,88 @@ final class TabManagerPullRequestProbeTests: XCTestCase {
|
|||
valid
|
||||
)
|
||||
}
|
||||
|
||||
func testResolvedCommandPathFallsBackOutsideAppPATH() throws {
|
||||
let fileManager = FileManager.default
|
||||
let tempDir = fileManager.temporaryDirectory.appendingPathComponent(
|
||||
"cmux-command-path-\(UUID().uuidString)",
|
||||
isDirectory: true
|
||||
)
|
||||
try fileManager.createDirectory(at: tempDir, withIntermediateDirectories: true)
|
||||
defer { try? fileManager.removeItem(at: tempDir) }
|
||||
|
||||
let executableName = "cmux-gh-test-\(UUID().uuidString)"
|
||||
let executableURL = tempDir.appendingPathComponent(executableName)
|
||||
try """
|
||||
#!/bin/sh
|
||||
exit 0
|
||||
""".write(to: executableURL, atomically: true, encoding: .utf8)
|
||||
try fileManager.setAttributes([.posixPermissions: 0o755], ofItemAtPath: executableURL.path)
|
||||
|
||||
XCTAssertEqual(
|
||||
TabManager.resolvedCommandPathForTesting(
|
||||
executable: executableName,
|
||||
environment: ["PATH": "/usr/bin:/bin"],
|
||||
fallbackDirectories: [tempDir.path]
|
||||
),
|
||||
executableURL.path
|
||||
)
|
||||
}
|
||||
|
||||
func testPeriodicWorkspaceGitMetadataRefreshClearsStalePullRequestAfterBranchReset() throws {
|
||||
let fileManager = FileManager.default
|
||||
let repoURL = fileManager.temporaryDirectory.appendingPathComponent("cmux-git-refresh-\(UUID().uuidString)")
|
||||
try fileManager.createDirectory(at: repoURL, withIntermediateDirectories: true)
|
||||
defer { try? fileManager.removeItem(at: repoURL) }
|
||||
|
||||
try runGit(["init", "-b", "main"], in: repoURL)
|
||||
try runGit(["config", "user.name", "cmux tests"], in: repoURL)
|
||||
try runGit(["config", "user.email", "cmux@example.invalid"], in: repoURL)
|
||||
try "seed\n".write(
|
||||
to: repoURL.appendingPathComponent("README.md"),
|
||||
atomically: true,
|
||||
encoding: .utf8
|
||||
)
|
||||
try runGit(["add", "README.md"], in: repoURL)
|
||||
try runGit(["commit", "-m", "Initial commit"], in: repoURL)
|
||||
try runGit(["checkout", "-b", "feature/sidebar-pr"], in: repoURL)
|
||||
|
||||
let manager = TabManager()
|
||||
guard let workspace = manager.selectedWorkspace,
|
||||
let panelId = workspace.focusedPanelId else {
|
||||
XCTFail("Expected selected workspace with focused panel")
|
||||
return
|
||||
}
|
||||
|
||||
workspace.updatePanelDirectory(panelId: panelId, directory: repoURL.path)
|
||||
workspace.updatePanelGitBranch(panelId: panelId, branch: "feature/sidebar-pr", isDirty: false)
|
||||
workspace.updatePanelPullRequest(
|
||||
panelId: panelId,
|
||||
number: 1052,
|
||||
label: "PR",
|
||||
url: try XCTUnwrap(URL(string: "https://github.com/manaflow-ai/cmux/pull/1052")),
|
||||
status: .open,
|
||||
branch: "feature/sidebar-pr"
|
||||
)
|
||||
|
||||
XCTAssertEqual(workspace.panelGitBranches[panelId]?.branch, "feature/sidebar-pr")
|
||||
XCTAssertEqual(workspace.panelPullRequests[panelId]?.number, 1052)
|
||||
XCTAssertEqual(workspace.sidebarPullRequestsInDisplayOrder().map(\.number), [1052])
|
||||
|
||||
try runGit(["checkout", "main"], in: repoURL)
|
||||
|
||||
manager.refreshTrackedWorkspaceGitMetadataForTesting()
|
||||
|
||||
XCTAssertTrue(
|
||||
waitForCondition {
|
||||
workspace.panelGitBranches[panelId]?.branch == "main"
|
||||
&& workspace.panelPullRequests[panelId] == nil
|
||||
}
|
||||
)
|
||||
XCTAssertEqual(workspace.gitBranch?.branch, "main")
|
||||
XCTAssertNil(workspace.pullRequest)
|
||||
XCTAssertTrue(workspace.sidebarPullRequestsInDisplayOrder().isEmpty)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue