From 55cb5c6763ccd07cb302c85554bbd44457fab35b Mon Sep 17 00:00:00 2001 From: Austin Wang Date: Tue, 17 Mar 2026 22:57:12 -0700 Subject: [PATCH] Fix sidebar workspace PR status display and false positives (#1636) * test(sidebar): add failing PR checks regressions * fix(sidebar): show workspace PR status * refactor(sidebar): restore PR icon style * refactor(sidebar): simplify PR check labels * test(sidebar): cover focused workspace PR selection * fix(sidebar): scope workspace PRs to current branch * test(sidebar): cover stale PR after branch change * fix(sidebar): clear stale PRs on branch changes * test(sidebar): cover workspace PR false positives * fix(sidebar): stop false-positive workspace PR badges * test(cmuxTests): remove duplicate sidebar PR regressions --- .../cmux-bash-integration.bash | 62 +-- .../cmux-zsh-integration.zsh | 62 +-- Sources/ContentView.swift | 11 +- Sources/TabManager.swift | 501 ++++++++++++++---- Sources/TerminalController.swift | 78 ++- Sources/Workspace.swift | 107 +++- cmuxTests/CmuxWebViewKeyEquivalentTests.swift | 143 ++++- .../WorkspacePullRequestSidebarTests.swift | 82 +++ 8 files changed, 827 insertions(+), 219 deletions(-) create mode 100644 cmuxTests/WorkspacePullRequestSidebarTests.swift diff --git a/Resources/shell-integration/cmux-bash-integration.bash b/Resources/shell-integration/cmux-bash-integration.bash index 46afa112..1981b9f2 100644 --- a/Resources/shell-integration/cmux-bash-integration.bash +++ b/Resources/shell-integration/cmux-bash-integration.bash @@ -194,8 +194,6 @@ _cmux_report_pr_for_path() { [[ -n "$CMUX_PANEL_ID" ]] || return 0 local branch repo_slug="" gh_output="" gh_error="" err_file="" gh_status number state url status_opt="" - local explicit_branch_output="" explicit_branch_error="" explicit_branch_status=0 - local implicit_probe_indicates_no_pr=0 explicit_probe_indicates_no_pr=0 local -a gh_repo_args=() branch="$(git -C "$repo_path" branch --show-current 2>/dev/null)" if [[ -z "$branch" ]] || ! command -v gh >/dev/null 2>&1; then @@ -211,7 +209,7 @@ _cmux_report_pr_for_path() { [[ -n "$err_file" ]] || return 1 gh_output="$( builtin cd "$repo_path" 2>/dev/null \ - && gh pr view \ + && gh pr view "$branch" \ "${gh_repo_args[@]}" \ --json number,state,url \ --jq '[.number, .state, .url] | @tsv' \ @@ -223,53 +221,20 @@ _cmux_report_pr_for_path() { /bin/rm -f -- "$err_file" >/dev/null 2>&1 || true fi - if (( gh_status == 0 )) && [[ -n "$gh_output" ]]; then - : - else + if (( gh_status != 0 )) || [[ -z "$gh_output" ]]; then if (( gh_status == 0 )) && [[ -z "$gh_output" ]]; then - implicit_probe_indicates_no_pr=1 - elif _cmux_pr_output_indicates_no_pull_request "$gh_error"; then - implicit_probe_indicates_no_pr=1 + _cmux_clear_pr_for_panel + return 0 + fi + if _cmux_pr_output_indicates_no_pull_request "$gh_error"; then + _cmux_clear_pr_for_panel + return 0 fi - # `gh pr view` without an explicit branch can fail to resolve the - # current worktree branch even when the branch has a PR. Fall back to - # the explicit branch name before concluding there is no PR. - err_file="$(/usr/bin/mktemp "${TMPDIR:-/tmp}/cmux-gh-pr-view.XXXXXX" 2>/dev/null || true)" - [[ -n "$err_file" ]] || return 1 - explicit_branch_output="$( - builtin cd "$repo_path" 2>/dev/null \ - && gh pr view "$branch" \ - "${gh_repo_args[@]}" \ - --json number,state,url \ - --jq '[.number, .state, .url] | @tsv' \ - 2>"$err_file" - )" - explicit_branch_status=$? - if [[ -f "$err_file" ]]; then - explicit_branch_error="$("/bin/cat" -- "$err_file" 2>/dev/null || true)" - /bin/rm -f -- "$err_file" >/dev/null 2>&1 || true - fi - - if (( explicit_branch_status == 0 )) && [[ -n "$explicit_branch_output" ]]; then - gh_output="$explicit_branch_output" - gh_status=0 - else - if (( explicit_branch_status == 0 )) && [[ -z "$explicit_branch_output" ]]; then - explicit_probe_indicates_no_pr=1 - elif _cmux_pr_output_indicates_no_pull_request "$explicit_branch_error"; then - explicit_probe_indicates_no_pr=1 - fi - - if (( implicit_probe_indicates_no_pr )) && (( explicit_probe_indicates_no_pr )); then - _cmux_clear_pr_for_panel - return 0 - fi - - # Preserve the last-known PR badge when gh fails transiently, then retry - # on the next background poll instead of clearing visible state. - return 1 - fi + # Always scope PR detection to the exact current branch. Preserve the + # last-known PR badge when gh fails transiently, then retry on the next + # background poll instead of showing a mismatched PR. + return 1 fi IFS=$'\t' read -r number state url <<< "$gh_output" @@ -284,7 +249,8 @@ _cmux_report_pr_for_path() { *) return 1 ;; esac - _cmux_send "report_pr $number $url $status_opt --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" + local quoted_branch="${branch//\"/\\\"}" + _cmux_send "report_pr $number $url $status_opt --branch=\"$quoted_branch\" --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" } _cmux_child_pids() { diff --git a/Resources/shell-integration/cmux-zsh-integration.zsh b/Resources/shell-integration/cmux-zsh-integration.zsh index 1bcf084f..3026ae95 100644 --- a/Resources/shell-integration/cmux-zsh-integration.zsh +++ b/Resources/shell-integration/cmux-zsh-integration.zsh @@ -312,8 +312,6 @@ _cmux_report_pr_for_path() { [[ -n "$CMUX_PANEL_ID" ]] || return 0 local branch repo_slug="" gh_output="" gh_error="" err_file="" number state url status_opt="" gh_status - local explicit_branch_output="" explicit_branch_error="" explicit_branch_status=0 - local implicit_probe_indicates_no_pr=0 explicit_probe_indicates_no_pr=0 local -a gh_repo_args gh_repo_args=() branch="$(git -C "$repo_path" branch --show-current 2>/dev/null)" @@ -330,7 +328,7 @@ _cmux_report_pr_for_path() { [[ -n "$err_file" ]] || return 1 gh_output="$( builtin cd "$repo_path" 2>/dev/null \ - && gh pr view \ + && gh pr view "$branch" \ "${gh_repo_args[@]}" \ --json number,state,url \ --jq '[.number, .state, .url] | @tsv' \ @@ -342,53 +340,20 @@ _cmux_report_pr_for_path() { /bin/rm -f -- "$err_file" >/dev/null 2>&1 || true fi - if (( gh_status == 0 )) && [[ -n "$gh_output" ]]; then - : - else + if (( gh_status != 0 )) || [[ -z "$gh_output" ]]; then if (( gh_status == 0 )) && [[ -z "$gh_output" ]]; then - implicit_probe_indicates_no_pr=1 - elif _cmux_pr_output_indicates_no_pull_request "$gh_error"; then - implicit_probe_indicates_no_pr=1 + _cmux_clear_pr_for_panel + return 0 + fi + if _cmux_pr_output_indicates_no_pull_request "$gh_error"; then + _cmux_clear_pr_for_panel + return 0 fi - # `gh pr view` without an explicit branch can fail to resolve the - # current worktree branch even when the branch has a PR. Fall back to - # the explicit branch name before concluding there is no PR. - err_file="$(/usr/bin/mktemp "${TMPDIR:-/tmp}/cmux-gh-pr-view.XXXXXX" 2>/dev/null || true)" - [[ -n "$err_file" ]] || return 1 - explicit_branch_output="$( - builtin cd "$repo_path" 2>/dev/null \ - && gh pr view "$branch" \ - "${gh_repo_args[@]}" \ - --json number,state,url \ - --jq '[.number, .state, .url] | @tsv' \ - 2>"$err_file" - )" - explicit_branch_status=$? - if [[ -f "$err_file" ]]; then - explicit_branch_error="$("/bin/cat" -- "$err_file" 2>/dev/null || true)" - /bin/rm -f -- "$err_file" >/dev/null 2>&1 || true - fi - - if (( explicit_branch_status == 0 )) && [[ -n "$explicit_branch_output" ]]; then - gh_output="$explicit_branch_output" - gh_status=0 - else - if (( explicit_branch_status == 0 )) && [[ -z "$explicit_branch_output" ]]; then - explicit_probe_indicates_no_pr=1 - elif _cmux_pr_output_indicates_no_pull_request "$explicit_branch_error"; then - explicit_probe_indicates_no_pr=1 - fi - - if (( implicit_probe_indicates_no_pr )) && (( explicit_probe_indicates_no_pr )); then - _cmux_clear_pr_for_panel - return 0 - fi - - # Keep the last-known PR badge on transient gh failures (auth hiccups, - # API lag after creation, or rate limiting) and retry on the next poll. - return 1 - fi + # Always scope PR detection to the exact current branch. When gh fails + # transiently (auth hiccups, API lag, rate limiting), keep the last-known + # badge and retry on the next poll instead of showing a mismatched PR. + return 1 fi local IFS=$'\t' @@ -404,7 +369,8 @@ _cmux_report_pr_for_path() { *) return 1 ;; esac - _cmux_send "report_pr $number $url $status_opt --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" + local quoted_branch="${branch//\"/\\\"}" + _cmux_send "report_pr $number $url $status_opt --branch=\"$quoted_branch\" --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" } _cmux_child_pids() { diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index 8242588b..fca38cd5 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10882,7 +10882,7 @@ private struct TabItemView: View, Equatable { .underline() .lineLimit(1) .truncationMode(.tail) - Text(pullRequestStatusLabel(pullRequest.status)) + Text(pullRequestStatusLabel(pullRequest.status, checks: pullRequest.checks)) .lineLimit(1) Spacer(minLength: 0) } @@ -11578,6 +11578,7 @@ private struct TabItemView: View, Equatable { let label: String let url: URL let status: SidebarPullRequestStatus + let checks: SidebarPullRequestChecksStatus? } private func pullRequestDisplays(orderedPanelIds: [UUID]) -> [PullRequestDisplay] { @@ -11587,7 +11588,8 @@ private struct TabItemView: View, Equatable { number: pullRequest.number, label: pullRequest.label, url: pullRequest.url, - status: pullRequest.status + status: pullRequest.status, + checks: pullRequest.checks ) } } @@ -11612,7 +11614,10 @@ private struct TabItemView: View, Equatable { NSWorkspace.shared.open(url) } - private func pullRequestStatusLabel(_ status: SidebarPullRequestStatus) -> String { + private func pullRequestStatusLabel( + _ status: SidebarPullRequestStatus, + checks _: SidebarPullRequestChecksStatus? + ) -> String { switch status { case .open: return String(localized: "sidebar.pullRequest.statusOpen", defaultValue: "open") case .merged: return String(localized: "sidebar.pullRequest.statusMerged", defaultValue: "merged") diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index 783f0e7c..6df6abaf 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -647,10 +647,17 @@ fileprivate func cmuxVsyncIOSurfaceTimelineCallback( @MainActor class TabManager: ObservableObject { + private enum WorkspacePullRequestSnapshot: Equatable { + case unsupportedRepository + case notFound + case resolved(SidebarPullRequestState) + case transientFailure + } + private struct InitialWorkspaceGitMetadataSnapshot: Equatable { let branch: String? let isDirty: Bool - let pullRequest: SidebarPullRequestState? + let pullRequest: WorkspacePullRequestSnapshot } private struct CommandResult { @@ -661,6 +668,22 @@ class TabManager: ObservableObject { let executionError: String? } + private struct WorkspaceGitProbeKey: Hashable { + let workspaceId: UUID + let panelId: UUID + } + + private struct GitHubPullRequestViewItem: Decodable { + let number: Int + let state: String + let url: String + } + + private struct GitHubPullRequestCheckItem: Decodable { + let bucket: String? + let state: String? + } + /// The window that owns this TabManager. Set by AppDelegate.registerMainWindow(). /// Used to apply title updates to the correct window instead of NSApp.keyWindow. weak var window: NSWindow? @@ -674,7 +697,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 nonisolated static let initialWorkspacePullRequestProbeTimeout: TimeInterval = 5.0 + private nonisolated static let workspacePullRequestProbeTimeout: TimeInterval = 5.0 @Published var selectedTabId: UUID? { willSet { #if DEBUG @@ -761,8 +784,8 @@ class TabManager: ObservableObject { label: "com.cmux.initial-workspace-git-probe", qos: .utility ) - private var initialWorkspaceGitProbeGenerationByWorkspace: [UUID: UUID] = [:] - private var initialWorkspaceGitProbeTimersByWorkspace: [UUID: [DispatchSourceTimer]] = [:] + private var workspaceGitProbeGenerationByKey: [WorkspaceGitProbeKey: UUID] = [:] + private var workspaceGitProbeTimersByKey: [WorkspaceGitProbeKey: [DispatchSourceTimer]] = [:] // Recent tab history for back/forward navigation (like browser history) private var tabHistory: [UUID] = [] @@ -892,6 +915,33 @@ class TabManager: ObservableObject { } } + private func gitProbeDirectory(for workspace: Workspace, panelId: UUID) -> String? { + let rawDirectory = workspace.panelDirectories[panelId] + ?? (workspace.focusedPanelId == panelId ? workspace.currentDirectory : nil) + return rawDirectory.flatMap(normalizedWorkingDirectory) + } + + private func scheduleWorkspaceGitMetadataRefreshIfPossible( + workspaceId: UUID, + panelId: UUID, + reason: String, + delays: [TimeInterval] = [0] + ) { + guard let workspace = tabs.first(where: { $0.id == workspaceId }), + workspace.panels[panelId] != nil, + let directory = gitProbeDirectory(for: workspace, panelId: panelId) else { + return + } + + scheduleWorkspaceGitMetadataRefresh( + workspaceId: workspaceId, + panelId: panelId, + directory: directory, + delays: delays, + reason: reason + ) + } + private func wireClosedBrowserTracking(for workspace: Workspace) { workspace.onClosedBrowserPanel = { [weak self] snapshot in self?.recentlyClosedBrowsers.push(snapshot) @@ -1147,20 +1197,36 @@ class TabManager: ObservableObject { workspaceId: UUID, panelId: UUID, directory: String + ) { + scheduleWorkspaceGitMetadataRefresh( + workspaceId: workspaceId, + panelId: panelId, + directory: directory, + delays: Self.initialWorkspaceGitProbeDelays, + reason: "initial" + ) + } + + private func scheduleWorkspaceGitMetadataRefresh( + workspaceId: UUID, + panelId: UUID, + directory: String, + delays: [TimeInterval], + reason: String ) { let normalizedDirectory = normalizeDirectory(directory) + let key = WorkspaceGitProbeKey(workspaceId: workspaceId, panelId: panelId) let generation = UUID() - cancelInitialWorkspaceGitProbeTimers(workspaceId: workspaceId) - initialWorkspaceGitProbeGenerationByWorkspace[workspaceId] = generation + cancelWorkspaceGitProbeTimers(for: key) + workspaceGitProbeGenerationByKey[key] = generation #if DEBUG dlog( "workspace.gitProbe.schedule workspace=\(workspaceId.uuidString.prefix(5)) " + - "panel=\(panelId.uuidString.prefix(5)) dir=\(normalizedDirectory)" + "panel=\(panelId.uuidString.prefix(5)) dir=\(normalizedDirectory) reason=\(reason)" ) #endif - let delays = Self.initialWorkspaceGitProbeDelays var timers: [DispatchSourceTimer] = [] for (index, delay) in delays.enumerated() { let isLastAttempt = index == delays.count - 1 @@ -1169,11 +1235,10 @@ class TabManager: ObservableObject { timer.setEventHandler { [weak self] in let snapshot = Self.initialWorkspaceGitMetadataSnapshot(for: normalizedDirectory) Task { @MainActor [weak self] in - self?.applyInitialWorkspaceGitMetadataSnapshot( + self?.applyWorkspaceGitMetadataSnapshot( snapshot, generation: generation, - workspaceId: workspaceId, - panelId: panelId, + probeKey: key, expectedDirectory: normalizedDirectory, isLastAttempt: isLastAttempt ) @@ -1182,11 +1247,11 @@ class TabManager: ObservableObject { timers.append(timer) timer.resume() } - initialWorkspaceGitProbeTimersByWorkspace[workspaceId] = timers + workspaceGitProbeTimersByKey[key] = timers } - private func cancelInitialWorkspaceGitProbeTimers(workspaceId: UUID) { - guard let timers = initialWorkspaceGitProbeTimersByWorkspace.removeValue(forKey: workspaceId) else { + private func cancelWorkspaceGitProbeTimers(for key: WorkspaceGitProbeKey) { + guard let timers = workspaceGitProbeTimersByKey.removeValue(forKey: key) else { return } for timer in timers { @@ -1195,95 +1260,139 @@ class TabManager: ObservableObject { } } - private func clearInitialWorkspaceGitProbe(workspaceId: UUID) { - initialWorkspaceGitProbeGenerationByWorkspace.removeValue(forKey: workspaceId) - cancelInitialWorkspaceGitProbeTimers(workspaceId: workspaceId) + private func clearWorkspaceGitProbe(_ key: WorkspaceGitProbeKey) { + workspaceGitProbeGenerationByKey.removeValue(forKey: key) + cancelWorkspaceGitProbeTimers(for: key) } - private func applyInitialWorkspaceGitMetadataSnapshot( + private func clearWorkspaceGitProbes(workspaceId: UUID) { + let keys = Set(workspaceGitProbeGenerationByKey.keys.filter { $0.workspaceId == workspaceId }) + .union(workspaceGitProbeTimersByKey.keys.filter { $0.workspaceId == workspaceId }) + for key in keys { + clearWorkspaceGitProbe(key) + } + } + + private func applyWorkspaceGitMetadataSnapshot( _ snapshot: InitialWorkspaceGitMetadataSnapshot, generation: UUID, - workspaceId: UUID, - panelId: UUID, + probeKey: WorkspaceGitProbeKey, expectedDirectory: String, isLastAttempt: Bool ) { defer { - if isLastAttempt, - initialWorkspaceGitProbeGenerationByWorkspace[workspaceId] == generation { - clearInitialWorkspaceGitProbe(workspaceId: workspaceId) + if shouldStopWorkspaceGitMetadataRefresh(snapshot) || isLastAttempt, + workspaceGitProbeGenerationByKey[probeKey] == generation { + clearWorkspaceGitProbe(probeKey) } } - guard initialWorkspaceGitProbeGenerationByWorkspace[workspaceId] == generation else { return } - guard let workspace = tabs.first(where: { $0.id == workspaceId }) else { - clearInitialWorkspaceGitProbe(workspaceId: workspaceId) + guard workspaceGitProbeGenerationByKey[probeKey] == generation else { return } + guard let workspace = tabs.first(where: { $0.id == probeKey.workspaceId }) else { + clearWorkspaceGitProbe(probeKey) return } - guard workspace.panels[panelId] != nil else { - clearInitialWorkspaceGitProbe(workspaceId: workspaceId) + guard workspace.panels[probeKey.panelId] != nil else { + clearWorkspaceGitProbe(probeKey) return } - let currentDirectory = normalizedWorkingDirectory( - workspace.panelDirectories[panelId] ?? workspace.currentDirectory - ) - if let currentDirectory, currentDirectory != expectedDirectory { - clearInitialWorkspaceGitProbe(workspaceId: workspaceId) + guard let currentDirectory = gitProbeDirectory(for: workspace, panelId: probeKey.panelId) else { + clearWorkspaceGitProbe(probeKey) + return + } + if currentDirectory != expectedDirectory { + clearWorkspaceGitProbe(probeKey) #if DEBUG dlog( - "workspace.gitProbe.skip workspace=\(workspaceId.uuidString.prefix(5)) " + - "panel=\(panelId.uuidString.prefix(5)) reason=directoryChanged " + + "workspace.gitProbe.skip workspace=\(probeKey.workspaceId.uuidString.prefix(5)) " + + "panel=\(probeKey.panelId.uuidString.prefix(5)) reason=directoryChanged " + "expected=\(expectedDirectory) current=\(currentDirectory)" ) #endif return } - workspace.updatePanelDirectory(panelId: panelId, directory: expectedDirectory) + workspace.updatePanelDirectory(panelId: probeKey.panelId, directory: expectedDirectory) - let previousBranch = Self.normalizedBranchName(workspace.panelGitBranches[panelId]?.branch) let nextBranch = snapshot.branch if let nextBranch { - workspace.updatePanelGitBranch(panelId: panelId, branch: nextBranch, isDirty: snapshot.isDirty) + workspace.updatePanelGitBranch( + panelId: probeKey.panelId, + branch: nextBranch, + isDirty: snapshot.isDirty + ) } else { - workspace.clearPanelGitBranch(panelId: panelId) + workspace.clearPanelGitBranch(panelId: probeKey.panelId) } - if let pullRequest = snapshot.pullRequest { + switch snapshot.pullRequest { + case .resolved(let pullRequest): workspace.updatePanelPullRequest( - panelId: panelId, + panelId: probeKey.panelId, number: pullRequest.number, label: pullRequest.label, url: pullRequest.url, - status: pullRequest.status + status: pullRequest.status, + checks: pullRequest.checks ) - } else if previousBranch != nextBranch || (nextBranch == nil && workspace.panelPullRequests[panelId] != nil) { - workspace.clearPanelPullRequest(panelId: panelId) + case .notFound: + if workspace.panelPullRequests[probeKey.panelId] != nil { + workspace.clearPanelPullRequest(panelId: probeKey.panelId) + } + case .unsupportedRepository, .transientFailure: + break } #if DEBUG let branchLabel = snapshot.branch ?? "none" - let prLabel = snapshot.pullRequest.map { "#\($0.number):\($0.status.rawValue)" } ?? "none" + let prLabel: String = { + switch snapshot.pullRequest { + case .unsupportedRepository: + return "unsupported" + case .notFound: + return "none" + case .transientFailure: + return "transientFailure" + case .resolved(let pullRequest): + let checks = pullRequest.checks?.rawValue ?? "none" + return "#\(pullRequest.number):\(pullRequest.status.rawValue):\(checks)" + } + }() dlog( - "workspace.gitProbe.apply workspace=\(workspaceId.uuidString.prefix(5)) " + - "panel=\(panelId.uuidString.prefix(5)) branch=\(branchLabel) dirty=\(snapshot.isDirty ? 1 : 0) " + + "workspace.gitProbe.apply workspace=\(probeKey.workspaceId.uuidString.prefix(5)) " + + "panel=\(probeKey.panelId.uuidString.prefix(5)) branch=\(branchLabel) dirty=\(snapshot.isDirty ? 1 : 0) " + "pr=\(prLabel)" ) #endif } + private func shouldStopWorkspaceGitMetadataRefresh( + _ snapshot: InitialWorkspaceGitMetadataSnapshot + ) -> Bool { + switch snapshot.pullRequest { + case .transientFailure: + return false + case .unsupportedRepository, .notFound, .resolved: + return true + } + } + private nonisolated static func initialWorkspaceGitMetadataSnapshot( for directory: String ) -> InitialWorkspaceGitMetadataSnapshot { let branch = normalizedBranchName(runGitCommand(directory: directory, arguments: ["branch", "--show-current"])) guard let branch else { - return InitialWorkspaceGitMetadataSnapshot(branch: nil, isDirty: false, pullRequest: nil) + return InitialWorkspaceGitMetadataSnapshot( + branch: nil, + isDirty: false, + pullRequest: .notFound + ) } let statusOutput = runGitCommand(directory: directory, arguments: ["status", "--porcelain", "-uno"]) let isDirty = !(statusOutput?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty ?? true) - let pullRequest = initialWorkspacePullRequestSnapshot(directory: directory, branch: branch) + let pullRequest = workspacePullRequestSnapshot(directory: directory, branch: branch) return InitialWorkspaceGitMetadataSnapshot(branch: branch, isDirty: isDirty, pullRequest: pullRequest) } @@ -1295,34 +1404,42 @@ class TabManager: ObservableObject { ) } - private nonisolated static func initialWorkspacePullRequestSnapshot( + private nonisolated static func workspacePullRequestSnapshot( directory: String, branch: String - ) -> SidebarPullRequestState? { - let repoSlug = githubRepositorySlug(directory: directory) - let repoArguments = repoSlug.map { ["--repo", $0] } ?? [] + ) -> WorkspacePullRequestSnapshot { + guard let repoSlug = githubRepositorySlug(directory: directory) else { + return .unsupportedRepository + } + let result = runCommandResult( directory: directory, executable: "gh", arguments: [ "pr", "view", branch, - ] + repoArguments + [ + "--repo", repoSlug, "--json", "number,state,url", - "--jq", "[.number, .state, .url] | @tsv", ], - timeout: initialWorkspacePullRequestProbeTimeout + timeout: workspacePullRequestProbeTimeout ) - guard let result else { return nil } - guard let output = result.stdout, - result.exitStatus == 0, - !output.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { + guard let result else { +#if DEBUG + dlog( + "workspace.gitProbe.pr.fail dir=\(directory) branch=\(branch) " + + "repo=\(repoSlug) status=nil" + ) +#endif + return .transientFailure + } + + guard !result.timedOut, + result.executionError == nil, + let exitStatus = result.exitStatus else { #if DEBUG let statusText: String if result.timedOut { statusText = "timeout" - } else if let exitStatus = result.exitStatus { - statusText = "exit=\(exitStatus)" } else if let executionError = result.executionError { statusText = "error=\(executionError)" } else { @@ -1331,47 +1448,188 @@ class TabManager: ObservableObject { let stderr = debugLogSnippet(result.stderr) ?? "none" dlog( "workspace.gitProbe.pr.fail dir=\(directory) branch=\(branch) " + - "repo=\(repoSlug ?? "none") status=\(statusText) stderr=\(stderr)" + "repo=\(repoSlug) status=\(statusText) stderr=\(stderr)" ) #endif - return nil + return .transientFailure } - let trimmedOutput = output.trimmingCharacters(in: .whitespacesAndNewlines) - let fields = trimmedOutput - .trimmingCharacters(in: .whitespacesAndNewlines) - .split(separator: "\t", maxSplits: 2, omittingEmptySubsequences: false) - guard fields.count == 3, - let number = Int(fields[0]), - let url = URL(string: String(fields[2])) else { + if exitStatus != 0 { + let stderr = result.stderr ?? "" + if prErrorIndicatesNoPullRequest(stderr) { +#if DEBUG + dlog( + "workspace.gitProbe.pr.none dir=\(directory) branch=\(branch) " + + "repo=\(repoSlug) stderr=\(debugLogSnippet(stderr) ?? "none")" + ) +#endif + return .notFound + } +#if DEBUG + dlog( + "workspace.gitProbe.pr.fail dir=\(directory) branch=\(branch) " + + "repo=\(repoSlug) status=exit=\(exitStatus) stderr=\(debugLogSnippet(stderr) ?? "none")" + ) +#endif + return .transientFailure + } + + let output = result.stdout ?? "" + guard !output.isEmpty, + let pullRequest = decodeJSON(GitHubPullRequestViewItem.self, from: output) else { #if DEBUG dlog( "workspace.gitProbe.pr.parseFail dir=\(directory) branch=\(branch) " + - "repo=\(repoSlug ?? "none") output=\(debugLogSnippet(trimmedOutput) ?? "none")" + "repo=\(repoSlug) output=\(debugLogSnippet(output) ?? "none")" ) #endif - return nil + return .transientFailure } - let status: SidebarPullRequestStatus - switch fields[1].uppercased() { - case "OPEN": - status = .open - case "MERGED": - status = .merged - case "CLOSED": - status = .closed - default: - return nil + guard let status = pullRequestStatus(from: pullRequest.state), + let url = URL(string: pullRequest.url) else { +#if DEBUG + dlog( + "workspace.gitProbe.pr.parseFail dir=\(directory) branch=\(branch) " + + "repo=\(repoSlug) output=\(debugLogSnippet(output) ?? "none")" + ) +#endif + return .transientFailure } + let checks = status == .open + ? pullRequestChecksStatus(number: pullRequest.number, directory: directory, repoSlug: repoSlug) + : nil + #if DEBUG dlog( "workspace.gitProbe.pr.success dir=\(directory) branch=\(branch) " + - "repo=\(repoSlug ?? "none") number=\(number) state=\(status.rawValue)" + "repo=\(repoSlug) number=\(pullRequest.number) state=\(status.rawValue) checks=\(checks?.rawValue ?? "none")" ) #endif - return SidebarPullRequestState(number: number, label: "PR", url: url, status: status) + return .resolved( + SidebarPullRequestState( + number: pullRequest.number, + label: "PR", + url: url, + status: status, + branch: branch, + checks: checks + ) + ) + } + + private nonisolated static func pullRequestChecksStatus( + number: Int, + directory: String, + repoSlug: String + ) -> SidebarPullRequestChecksStatus? { + let result = runCommandResult( + directory: directory, + executable: "gh", + arguments: [ + "pr", "checks", String(number), + "--repo", repoSlug, + "--json", "bucket,state" + ], + timeout: workspacePullRequestProbeTimeout + ) + + guard let result, + !result.timedOut, + result.executionError == nil, + let output = result.stdout, + let exitStatus = result.exitStatus, + exitStatus == 0 || exitStatus == 8, + let checks = decodeJSON([GitHubPullRequestCheckItem].self, from: output) else { + return nil + } + + var sawPending = false + var sawPass = false + + for check in checks { + let bucket = check.bucket?.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + let state = check.state?.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + + if isFailingCheckState(bucket: bucket, state: state) { + return .fail + } + if isPendingCheckState(bucket: bucket, state: state) { + sawPending = true + continue + } + if isPassingCheckState(bucket: bucket, state: state) { + sawPass = true + } + } + + if sawPending { + return .pending + } + if sawPass { + return .pass + } + return nil + } + + private nonisolated static func pullRequestStatus( + from rawState: String + ) -> SidebarPullRequestStatus? { + switch rawState.trimmingCharacters(in: .whitespacesAndNewlines).uppercased() { + case "OPEN": + return .open + case "MERGED": + return .merged + case "CLOSED": + return .closed + default: + return nil + } + } + + private nonisolated static func decodeJSON(_ type: T.Type, from text: String) -> T? { + guard let data = text.data(using: .utf8) else { return nil } + return try? JSONDecoder().decode(T.self, from: data) + } + + private nonisolated static func prErrorIndicatesNoPullRequest(_ text: String?) -> Bool { + let normalized = text? + .trimmingCharacters(in: .whitespacesAndNewlines) + .lowercased() ?? "" + guard !normalized.isEmpty else { return false } + return normalized.contains("no pull requests found") + || normalized.contains("no pull request found") + || normalized.contains("no pull requests associated") + || normalized.contains("no pull request associated") + } + + private nonisolated static func isFailingCheckState(bucket: String?, state: String?) -> Bool { + switch bucket ?? state ?? "" { + case "fail", "failure", "failed", "error", "timed_out", "timedout", + "cancel", "cancelled", "canceled", "action_required", "startup_failure": + return true + default: + return false + } + } + + private nonisolated static func isPendingCheckState(bucket: String?, state: String?) -> Bool { + switch bucket ?? state ?? "" { + case "pending", "queued", "in_progress", "requested", "waiting", "expected": + return true + default: + return false + } + } + + private nonisolated static func isPassingCheckState(bucket: String?, state: String?) -> Bool { + switch bucket ?? state ?? "" { + case "pass", "success", "successful", "completed", "neutral", "skipping", "skipped": + return true + default: + return false + } } private nonisolated static func runCommand( @@ -1765,8 +2023,49 @@ class TabManager: ObservableObject { func updateSurfaceDirectory(tabId: UUID, surfaceId: UUID, directory: String) { guard let tab = tabs.first(where: { $0.id == tabId }) else { return } + let previousDirectory = gitProbeDirectory(for: tab, panelId: surfaceId) let normalized = normalizeDirectory(directory) tab.updatePanelDirectory(panelId: surfaceId, directory: normalized) + let nextDirectory = normalizedWorkingDirectory(normalized) + if previousDirectory != nextDirectory { + scheduleWorkspaceGitMetadataRefreshIfPossible( + workspaceId: tabId, + panelId: surfaceId, + reason: "directoryChange" + ) + } + } + + func updateSurfaceGitBranch( + tabId: UUID, + surfaceId: UUID, + branch: String, + isDirty: Bool + ) { + guard let tab = tabs.first(where: { $0.id == tabId }) else { return } + let current = tab.panelGitBranches[surfaceId] + let normalizedBranch = Self.normalizedBranchName(branch) ?? branch + guard current?.branch != normalizedBranch || current?.isDirty != isDirty else { return } + tab.updatePanelGitBranch(panelId: surfaceId, branch: normalizedBranch, isDirty: isDirty) + scheduleWorkspaceGitMetadataRefreshIfPossible( + workspaceId: tabId, + panelId: surfaceId, + reason: "branchChange" + ) + } + + func clearSurfaceGitBranch(tabId: UUID, surfaceId: UUID) { + guard let tab = tabs.first(where: { $0.id == tabId }) else { return } + let hadBranch = tab.panelGitBranches[surfaceId] != nil + let hadPullRequest = tab.panelPullRequests[surfaceId] != nil + guard hadBranch || hadPullRequest else { return } + tab.clearPanelGitBranch(panelId: surfaceId) + tab.clearPanelPullRequest(panelId: surfaceId) + scheduleWorkspaceGitMetadataRefreshIfPossible( + workspaceId: tabId, + panelId: surfaceId, + reason: "branchCleared" + ) } func updateSurfaceShellActivity( @@ -1792,7 +2091,7 @@ class TabManager: ObservableObject { func closeWorkspace(_ workspace: Workspace) { guard tabs.count > 1 else { return } sentryBreadcrumb("workspace.close", data: ["tabCount": tabs.count - 1]) - clearInitialWorkspaceGitProbe(workspaceId: workspace.id) + clearWorkspaceGitProbes(workspaceId: workspace.id) sidebarSelectedWorkspaceIds.remove(workspace.id) AppDelegate.shared?.notificationStore?.clearNotifications(forTabId: workspace.id) @@ -1819,7 +2118,7 @@ class TabManager: ObservableObject { @discardableResult func detachWorkspace(tabId: UUID) -> Workspace? { guard let index = tabs.firstIndex(where: { $0.id == tabId }) else { return nil } - clearInitialWorkspaceGitProbe(workspaceId: tabId) + clearWorkspaceGitProbes(workspaceId: tabId) sidebarSelectedWorkspaceIds.remove(tabId) let removed = tabs.remove(at: index) @@ -4607,8 +4906,10 @@ extension TabManager { for tab in tabs { unwireClosedBrowserTracking(for: tab) } - for workspaceId in Array(initialWorkspaceGitProbeGenerationByWorkspace.keys) { - clearInitialWorkspaceGitProbe(workspaceId: workspaceId) + let existingProbeKeys = Set(workspaceGitProbeGenerationByKey.keys) + .union(workspaceGitProbeTimersByKey.keys) + for key in existingProbeKeys { + clearWorkspaceGitProbe(key) } // Clear non-@Published state without touching tabs/selectedTabId yet. @@ -4667,19 +4968,17 @@ extension TabManager { tabs = newTabs selectedTabId = newSelectedId for workspace in newTabs { - guard let terminalPanel = workspace.focusedTerminalPanel ?? workspace.panels.values - .compactMap({ $0 as? TerminalPanel }) - .first, - let directory = normalizedWorkingDirectory( - workspace.panelDirectories[terminalPanel.id] ?? workspace.currentDirectory - ) else { - continue + let terminalPanels = workspace.panels.values.compactMap { $0 as? TerminalPanel } + for terminalPanel in terminalPanels { + guard let directory = gitProbeDirectory(for: workspace, panelId: terminalPanel.id) else { + continue + } + scheduleInitialWorkspaceGitMetadataRefresh( + workspaceId: workspace.id, + panelId: terminalPanel.id, + directory: directory + ) } - scheduleInitialWorkspaceGitMetadataRefresh( - workspaceId: workspace.id, - panelId: terminalPanel.id, - directory: directory - ) } if let selectedTabId { diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index de1e96f3..6227cbaf 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -387,10 +387,42 @@ class TerminalController { number: Int, label: String, url: URL, - status: SidebarPullRequestStatus + status: SidebarPullRequestStatus, + branch: String?, + checks: SidebarPullRequestChecksStatus? ) -> Bool { guard let current else { return true } - return current.number != number || current.label != label || current.url != url || current.status != status + let normalizedBranch = branch?.trimmingCharacters(in: .whitespacesAndNewlines) + let effectiveBranch: String? = { + if let normalizedBranch, !normalizedBranch.isEmpty { + return normalizedBranch + } + guard current.number == number, + current.label == label, + current.url == url, + current.status == status else { + return nil + } + return current.branch + }() + let effectiveChecks: SidebarPullRequestChecksStatus? = { + if let checks { + return checks + } + guard current.number == number, + current.label == label, + current.url == url, + current.status == status else { + return nil + } + return current.checks + }() + return current.number != number + || current.label != label + || current.url != url + || current.status != status + || current.branch != effectiveBranch + || current.checks != effectiveChecks } nonisolated static func shouldReplacePorts(current: [Int]?, next: [Int]) -> Bool { @@ -10844,8 +10876,8 @@ class TerminalController { clear_progress [--tab=X] - Clear progress bar report_git_branch [--status=dirty] [--tab=X] [--panel=Y] - Report git branch clear_git_branch [--tab=X] [--panel=Y] - Clear git branch - report_pr [--label=PR] [--state=open|merged|closed] [--tab=X] [--panel=Y] - Report pull request / review item - report_review [--label=MR] [--state=open|merged|closed] [--tab=X] [--panel=Y] - Alias for provider-specific review item + report_pr [--label=PR] [--state=open|merged|closed] [--branch=] [--checks=pass|fail|pending] [--tab=X] [--panel=Y] - Report pull request / review item + report_review [--label=MR] [--state=open|merged|closed] [--checks=pass|fail|pending] [--tab=X] [--panel=Y] - Alias for provider-specific review item clear_pr [--tab=X] [--panel=Y] - Clear pull request report_ports [port2...] [--tab=X] [--panel=Y] - Report listening ports report_tty [--tab=X] [--panel=Y] - Register TTY for batched port scanning @@ -14492,7 +14524,12 @@ class TerminalController { let validSurfaceIds = Set(tab.panels.keys) tab.pruneSurfaceMetadata(validSurfaceIds: validSurfaceIds) guard validSurfaceIds.contains(scope.panelId) else { return } - tab.updatePanelGitBranch(panelId: scope.panelId, branch: branch, isDirty: isDirty) + tabManager.updateSurfaceGitBranch( + tabId: scope.workspaceId, + surfaceId: scope.panelId, + branch: branch, + isDirty: isDirty + ) } return "OK" } @@ -14523,7 +14560,7 @@ class TerminalController { let validSurfaceIds = Set(tab.panels.keys) tab.pruneSurfaceMetadata(validSurfaceIds: validSurfaceIds) guard validSurfaceIds.contains(scope.panelId) else { return } - tab.clearPanelGitBranch(panelId: scope.panelId) + tabManager.clearSurfaceGitBranch(tabId: scope.workspaceId, surfaceId: scope.panelId) } return "OK" } @@ -14541,7 +14578,7 @@ class TerminalController { private func reportPullRequest(_ args: String) -> String { let parsed = parseOptions(args) guard parsed.positional.count >= 2 else { - return "ERROR: Missing pull request number or URL — usage: report_pr [--label=PR] [--state=open|merged|closed] [--tab=X] [--panel=Y]" + return "ERROR: Missing pull request number or URL — usage: report_pr [--label=PR] [--state=open|merged|closed] [--branch=] [--checks=pass|fail|pending] [--tab=X] [--panel=Y]" } let rawNumber = parsed.positional[0].trimmingCharacters(in: .whitespacesAndNewlines) @@ -14561,10 +14598,21 @@ class TerminalController { guard let status = SidebarPullRequestStatus(rawValue: statusRaw) else { return "ERROR: Invalid pull request state '\(statusRaw)' — use: open, merged, closed" } + let branch = normalizedOptionValue(parsed.options["branch"]) + + let checks: SidebarPullRequestChecksStatus? + if let rawChecks = normalizedOptionValue(parsed.options["checks"]) { + guard let parsedChecks = SidebarPullRequestChecksStatus(rawValue: rawChecks.lowercased()) else { + return "ERROR: Invalid pull request checks '\(rawChecks)' — use: pass, fail, pending" + } + checks = parsedChecks + } else { + checks = nil + } let labelRaw = normalizedOptionValue(parsed.options["label"]) ?? "PR" guard !labelRaw.isEmpty else { - return "ERROR: Invalid review label — usage: report_pr [--label=PR] [--state=open|merged|closed] [--tab=X] [--panel=Y]" + return "ERROR: Invalid review label — usage: report_pr [--label=PR] [--state=open|merged|closed] [--branch=] [--checks=pass|fail|pending] [--tab=X] [--panel=Y]" } let label = String(labelRaw.prefix(16)) @@ -14573,14 +14621,16 @@ class TerminalController { return schedulePanelMetadataMutation( args: args, options: parsed.options, - missingPanelUsage: "report_pr [--label=PR] [--state=open|merged|closed] [--tab=X] [--panel=Y]" + missingPanelUsage: "report_pr [--label=PR] [--state=open|merged|closed] [--branch=] [--checks=pass|fail|pending] [--tab=X] [--panel=Y]" ) { tab, surfaceId in guard Self.shouldReplacePullRequest( current: tab.panelPullRequests[surfaceId], number: number, label: label, url: url, - status: status + status: status, + branch: branch, + checks: checks ) else { return } @@ -14590,7 +14640,9 @@ class TerminalController { number: number, label: label, url: url, - status: status + status: status, + branch: branch, + checks: checks ) } } @@ -14958,12 +15010,14 @@ class TerminalController { lines.append("git_branch=none") } - if let pr = tab.pullRequest { + if let pr = tab.sidebarPullRequestsInDisplayOrder().first { lines.append("pr=#\(pr.number) \(pr.status.rawValue) \(pr.url.absoluteString)") lines.append("pr_label=\(pr.label)") + lines.append("pr_checks=\(pr.checks?.rawValue ?? "none")") } else { lines.append("pr=none") lines.append("pr_label=none") + lines.append("pr_checks=none") } if tab.listeningPorts.isEmpty { diff --git a/Sources/Workspace.swift b/Sources/Workspace.swift index 444f7170..4557f122 100644 --- a/Sources/Workspace.swift +++ b/Sources/Workspace.swift @@ -4506,11 +4506,41 @@ enum SidebarPullRequestStatus: String { case closed } +enum SidebarPullRequestChecksStatus: String { + case pass + case fail + case pending +} + +private func normalizedSidebarBranchName(_ branch: String?) -> String? { + guard let branch else { return nil } + let trimmed = branch.trimmingCharacters(in: .whitespacesAndNewlines) + return trimmed.isEmpty ? nil : trimmed +} + struct SidebarPullRequestState: Equatable { let number: Int let label: String let url: URL let status: SidebarPullRequestStatus + let branch: String? + let checks: SidebarPullRequestChecksStatus? + + init( + number: Int, + label: String, + url: URL, + status: SidebarPullRequestStatus, + branch: String? = nil, + checks: SidebarPullRequestChecksStatus? = nil + ) { + self.number = number + self.label = label + self.url = url + self.status = status + self.branch = normalizedSidebarBranchName(branch) + self.checks = checks + } } enum SidebarBranchOrdering { @@ -4606,6 +4636,15 @@ enum SidebarBranchOrdering { } } + func checksPriority(_ checks: SidebarPullRequestChecksStatus?) -> Int { + switch checks { + case .fail: return 3 + case .pending: return 2 + case .pass: return 1 + case nil: return 0 + } + } + func normalizedReviewURLKey(for url: URL) -> String { guard var components = URLComponents(url: url, resolvingAgainstBaseURL: false) else { return url.absoluteString @@ -4642,6 +4681,9 @@ enum SidebarBranchOrdering { guard let existing = pullRequestsByKey[key] else { continue } if statusPriority(state.status) > statusPriority(existing.status) { pullRequestsByKey[key] = state + } else if state.status == existing.status, + checksPriority(state.checks) > checksPriority(existing.checks) { + pullRequestsByKey[key] = state } } @@ -5671,9 +5713,16 @@ final class Workspace: Identifiable, ObservableObject { func updatePanelGitBranch(panelId: UUID, branch: String, isDirty: Bool) { let state = SidebarGitBranchState(branch: branch, isDirty: isDirty) let existing = panelGitBranches[panelId] + let branchChanged = existing?.branch != nil && existing?.branch != branch if existing?.branch != branch || existing?.isDirty != isDirty { panelGitBranches[panelId] = state } + if branchChanged { + panelPullRequests.removeValue(forKey: panelId) + if panelId == focusedPanelId { + pullRequest = nil + } + } if panelId == focusedPanelId { gitBranch = state } @@ -5681,8 +5730,10 @@ final class Workspace: Identifiable, ObservableObject { func clearPanelGitBranch(panelId: UUID) { panelGitBranches.removeValue(forKey: panelId) + panelPullRequests.removeValue(forKey: panelId) if panelId == focusedPanelId { gitBranch = nil + pullRequest = nil } } @@ -5691,10 +5742,50 @@ final class Workspace: Identifiable, ObservableObject { number: Int, label: String, url: URL, - status: SidebarPullRequestStatus + status: SidebarPullRequestStatus, + branch: String? = nil, + checks: SidebarPullRequestChecksStatus? = nil ) { - let state = SidebarPullRequestState(number: number, label: label, url: url, status: status) let existing = panelPullRequests[panelId] + let normalizedBranch = normalizedSidebarBranchName(branch) + let currentPanelBranch = normalizedSidebarBranchName(panelGitBranches[panelId]?.branch) + let resolvedBranch: String? = { + if let normalizedBranch { + return normalizedBranch + } + if let currentPanelBranch { + return currentPanelBranch + } + guard let existing, + existing.number == number, + existing.label == label, + existing.url == url, + existing.status == status else { + return nil + } + return existing.branch + }() + let resolvedChecks: SidebarPullRequestChecksStatus? = { + if let checks { + return checks + } + guard let existing, + existing.number == number, + existing.label == label, + existing.url == url, + existing.status == status else { + return nil + } + return existing.checks + }() + let state = SidebarPullRequestState( + number: number, + label: label, + url: url, + status: status, + branch: resolvedBranch, + checks: resolvedChecks + ) if existing != state { panelPullRequests[panelId] = state } @@ -5873,10 +5964,16 @@ final class Workspace: Identifiable, ObservableObject { } func sidebarPullRequestsInDisplayOrder(orderedPanelIds: [UUID]) -> [SidebarPullRequestState] { - SidebarBranchOrdering.orderedUniquePullRequests( + let validPanelPullRequests = panelPullRequests.filter { panelId, state in + guard let pullRequestBranch = normalizedSidebarBranchName(state.branch) else { + return true + } + return normalizedSidebarBranchName(panelGitBranches[panelId]?.branch) == pullRequestBranch + } + return SidebarBranchOrdering.orderedUniquePullRequests( orderedPanelIds: orderedPanelIds, - panelPullRequests: panelPullRequests, - fallbackPullRequest: pullRequest + panelPullRequests: validPanelPullRequests, + fallbackPullRequest: nil ) } diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index 9f0d08f9..9f343720 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -7294,6 +7294,40 @@ final class WorkspacePanelGitBranchTests: XCTestCase { XCTAssertEqual(ordered.map(\.isDirty), [false, true]) } + @MainActor + func testSidebarPullRequestsTrackFocusedPanelOnly() { + let workspace = Workspace() + guard let firstPanelId = workspace.focusedPanelId, + let paneId = workspace.paneId(forPanelId: firstPanelId), + let secondPanel = workspace.newTerminalSurface(inPane: paneId, focus: false) else { + XCTFail("Expected focused panel and a second panel") + return + } + + workspace.updatePanelGitBranch(panelId: firstPanelId, branch: "main", isDirty: false) + workspace.updatePanelGitBranch(panelId: secondPanel.id, branch: "feature/sidebar-pr", isDirty: false) + workspace.updatePanelPullRequest( + panelId: secondPanel.id, + number: 1629, + label: "PR", + url: URL(string: "https://github.com/manaflow-ai/cmux/pull/1629")!, + status: .open + ) + + XCTAssertNil(workspace.pullRequest) + XCTAssertTrue( + workspace.sidebarPullRequestsInDisplayOrder().isEmpty, + "Expected background panel PRs to stay hidden while the focused panel has no PR" + ) + + workspace.focusPanel(secondPanel.id) + + XCTAssertEqual( + workspace.sidebarPullRequestsInDisplayOrder().map(\.number), + [1629] + ) + } + func testSidebarOrderingUsesPaneOrderThenTabOrderWithBranchDeduping() { let workspace = Workspace() guard let leftFirstPanelId = workspace.focusedPanelId, @@ -7624,6 +7658,62 @@ final class SidebarBranchOrderingTests: XCTestCase { ) } + func testOrderedUniquePullRequestsPrefersEntryWithChecksWhenStatusesMatch() { + let first = UUID() + let second = UUID() + + let pullRequests = SidebarBranchOrdering.orderedUniquePullRequests( + orderedPanelIds: [first, second], + panelPullRequests: [ + first: pullRequestState( + number: 42, + label: "PR", + url: "https://github.com/manaflow-ai/cmux/pull/42", + status: .open + ), + second: pullRequestState( + number: 42, + label: "PR", + url: "https://github.com/manaflow-ai/cmux/pull/42", + status: .open, + checks: .pass + ) + ], + fallbackPullRequest: nil + ) + + XCTAssertEqual(pullRequests.count, 1) + XCTAssertEqual(pullRequests.first?.checks, .pass) + } + + @MainActor + func testUpdatePanelPullRequestPreservesExistingChecksWhenUpdateOmitsThem() { + let workspace = Workspace(title: "Tests", workingDirectory: FileManager.default.currentDirectoryPath, portOrdinal: 0) + guard let panelId = workspace.focusedPanelId else { + XCTFail("Expected focused panel for new workspace") + return + } + + workspace.updatePanelPullRequest( + panelId: panelId, + number: 42, + label: "PR", + url: URL(string: "https://github.com/manaflow-ai/cmux/pull/42")!, + status: .open, + checks: .pass + ) + workspace.updatePanelPullRequest( + panelId: panelId, + number: 42, + label: "PR", + url: URL(string: "https://github.com/manaflow-ai/cmux/pull/42")!, + status: .open + ) + + XCTAssertEqual(workspace.panelPullRequests[panelId]?.checks, .pass) + XCTAssertEqual(workspace.pullRequest?.checks, .pass) + } + func testOrderedUniquePullRequestsUsesFallbackWhenNoPanelPullRequestsExist() { let fallback = pullRequestState( number: 11, @@ -7640,17 +7730,66 @@ final class SidebarBranchOrderingTests: XCTestCase { XCTAssertEqual(pullRequests, [fallback]) } + @MainActor + func testUpdatePanelGitBranchClearsFocusedPullRequestWhenBranchChanges() { + let workspace = Workspace(title: "Tests", workingDirectory: FileManager.default.currentDirectoryPath, portOrdinal: 0) + guard let panelId = workspace.focusedPanelId else { + XCTFail("Expected focused panel for new workspace") + return + } + + workspace.updatePanelGitBranch(panelId: panelId, branch: "feature/sidebar-pr", isDirty: false) + workspace.updatePanelPullRequest( + panelId: panelId, + number: 1629, + label: "PR", + url: URL(string: "https://github.com/manaflow-ai/cmux/pull/1629")!, + status: .open + ) + + workspace.updatePanelGitBranch(panelId: panelId, branch: "main", isDirty: false) + + XCTAssertNil(workspace.pullRequest) + XCTAssertNil(workspace.panelPullRequests[panelId]) + XCTAssertTrue(workspace.sidebarPullRequestsInDisplayOrder().isEmpty) + } + + @MainActor + func testSidebarPullRequestsHideBranchMismatches() { + let workspace = Workspace(title: "Tests", workingDirectory: FileManager.default.currentDirectoryPath, portOrdinal: 0) + guard let panelId = workspace.focusedPanelId else { + XCTFail("Expected focused panel for new workspace") + return + } + + workspace.updatePanelGitBranch(panelId: panelId, branch: "main", isDirty: false) + workspace.updatePanelPullRequest( + panelId: panelId, + number: 1629, + label: "PR", + url: URL(string: "https://github.com/manaflow-ai/cmux/pull/1629")!, + status: .open, + branch: "feature/sidebar-pr" + ) + + XCTAssertTrue(workspace.sidebarPullRequestsInDisplayOrder().isEmpty) + } + private func pullRequestState( number: Int, label: String, url: String, - status: SidebarPullRequestStatus + status: SidebarPullRequestStatus, + branch: String? = nil, + checks: SidebarPullRequestChecksStatus? = nil ) -> SidebarPullRequestState { SidebarPullRequestState( number: number, label: label, url: URL(string: url)!, - status: status + status: status, + branch: branch, + checks: checks ) } } diff --git a/cmuxTests/WorkspacePullRequestSidebarTests.swift b/cmuxTests/WorkspacePullRequestSidebarTests.swift new file mode 100644 index 00000000..e3cc26e1 --- /dev/null +++ b/cmuxTests/WorkspacePullRequestSidebarTests.swift @@ -0,0 +1,82 @@ +import XCTest + +#if canImport(cmux_DEV) +@testable import cmux_DEV +#elseif canImport(cmux) +@testable import cmux +#endif + +@MainActor +final class WorkspacePullRequestSidebarTests: XCTestCase { + func testSidebarPullRequestsIgnoreStaleWorkspaceLevelCacheWithoutPanelState() throws { + let workspace = Workspace(title: "Test") + let panelId = UUID() + let staleURL = try XCTUnwrap(URL(string: "https://github.com/manaflow-ai/cmux/pull/1640")) + + workspace.pullRequest = SidebarPullRequestState( + number: 1640, + label: "PR", + url: staleURL, + status: .open, + branch: "main" + ) + workspace.gitBranch = SidebarGitBranchState(branch: "main", isDirty: false) + + XCTAssertEqual(workspace.sidebarPullRequestsInDisplayOrder(orderedPanelIds: [panelId]), []) + } + + func testSidebarPullRequestsFilterBranchMismatchPerPanel() throws { + let workspace = Workspace(title: "Test") + let panelId = UUID() + let staleURL = try XCTUnwrap(URL(string: "https://github.com/manaflow-ai/cmux/pull/1640")) + + workspace.panelGitBranches[panelId] = SidebarGitBranchState(branch: "main", isDirty: false) + workspace.panelPullRequests[panelId] = SidebarPullRequestState( + number: 1640, + label: "PR", + url: staleURL, + status: .open, + branch: "feature/old" + ) + + XCTAssertEqual(workspace.sidebarPullRequestsInDisplayOrder(orderedPanelIds: [panelId]), []) + } + + func testSidebarPullRequestsPreferBestStateAcrossPanels() throws { + let workspace = Workspace(title: "Test") + let firstPanelId = UUID() + let secondPanelId = UUID() + let url = try XCTUnwrap(URL(string: "https://github.com/manaflow-ai/cmux/pull/1640")) + + workspace.panelGitBranches[firstPanelId] = SidebarGitBranchState(branch: "feature/work", isDirty: false) + workspace.panelGitBranches[secondPanelId] = SidebarGitBranchState(branch: "feature/work", isDirty: false) + workspace.panelPullRequests[firstPanelId] = SidebarPullRequestState( + number: 1640, + label: "PR", + url: url, + status: .open, + branch: "feature/work", + checks: .pass + ) + workspace.panelPullRequests[secondPanelId] = SidebarPullRequestState( + number: 1640, + label: "PR", + url: url, + status: .merged, + branch: "feature/work" + ) + + XCTAssertEqual( + workspace.sidebarPullRequestsInDisplayOrder(orderedPanelIds: [firstPanelId, secondPanelId]), + [ + SidebarPullRequestState( + number: 1640, + label: "PR", + url: url, + status: .merged, + branch: "feature/work" + ) + ] + ) + } +}