Fix sidebar PR badge detection for workspace branches (#1896)
* test: cover sidebar PR probe selection * fix: detect sidebar PR badges across github remotes
This commit is contained in:
parent
39c03c9b07
commit
5c4fab1296
2 changed files with 248 additions and 34 deletions
|
|
@ -673,10 +673,11 @@ class TabManager: ObservableObject {
|
|||
let panelId: UUID
|
||||
}
|
||||
|
||||
private struct GitHubPullRequestViewItem: Decodable {
|
||||
struct GitHubPullRequestProbeItem: Decodable, Equatable {
|
||||
let number: Int
|
||||
let state: String
|
||||
let url: String
|
||||
let updatedAt: String?
|
||||
}
|
||||
|
||||
private struct GitHubPullRequestCheckItem: Decodable {
|
||||
|
|
@ -1408,17 +1409,40 @@ class TabManager: ObservableObject {
|
|||
directory: String,
|
||||
branch: String
|
||||
) -> WorkspacePullRequestSnapshot {
|
||||
guard let repoSlug = githubRepositorySlug(directory: directory) else {
|
||||
let repoSlugs = githubRepositorySlugs(directory: directory)
|
||||
guard !repoSlugs.isEmpty else {
|
||||
return .unsupportedRepository
|
||||
}
|
||||
|
||||
var sawTransientFailure = false
|
||||
for repoSlug in repoSlugs {
|
||||
switch workspacePullRequestSnapshot(directory: directory, branch: branch, repoSlug: repoSlug) {
|
||||
case .resolved(let pullRequest):
|
||||
return .resolved(pullRequest)
|
||||
case .transientFailure:
|
||||
sawTransientFailure = true
|
||||
case .notFound, .unsupportedRepository:
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
return sawTransientFailure ? .transientFailure : .notFound
|
||||
}
|
||||
|
||||
private nonisolated static func workspacePullRequestSnapshot(
|
||||
directory: String,
|
||||
branch: String,
|
||||
repoSlug: String
|
||||
) -> WorkspacePullRequestSnapshot {
|
||||
let result = runCommandResult(
|
||||
directory: directory,
|
||||
executable: "gh",
|
||||
arguments: [
|
||||
"pr", "view", branch,
|
||||
"pr", "list",
|
||||
"--repo", repoSlug,
|
||||
"--json", "number,state,url",
|
||||
"--state", "all",
|
||||
"--head", branch,
|
||||
"--json", "number,state,url,updatedAt",
|
||||
],
|
||||
timeout: workspacePullRequestProbeTimeout
|
||||
)
|
||||
|
|
@ -1455,28 +1479,17 @@ class TabManager: ObservableObject {
|
|||
}
|
||||
|
||||
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")"
|
||||
"repo=\(repoSlug) status=exit=\(exitStatus) stderr=\(debugLogSnippet(result.stderr) ?? "none")"
|
||||
)
|
||||
#endif
|
||||
return .transientFailure
|
||||
}
|
||||
|
||||
let output = result.stdout ?? ""
|
||||
guard !output.isEmpty,
|
||||
let pullRequest = decodeJSON(GitHubPullRequestViewItem.self, from: output) else {
|
||||
guard let pullRequests = decodeJSON([GitHubPullRequestProbeItem].self, from: output) else {
|
||||
#if DEBUG
|
||||
dlog(
|
||||
"workspace.gitProbe.pr.parseFail dir=\(directory) branch=\(branch) " +
|
||||
|
|
@ -1486,6 +1499,16 @@ class TabManager: ObservableObject {
|
|||
return .transientFailure
|
||||
}
|
||||
|
||||
guard let pullRequest = preferredPullRequest(from: pullRequests) else {
|
||||
#if DEBUG
|
||||
dlog(
|
||||
"workspace.gitProbe.pr.none dir=\(directory) branch=\(branch) " +
|
||||
"repo=\(repoSlug)"
|
||||
)
|
||||
#endif
|
||||
return .notFound
|
||||
}
|
||||
|
||||
guard let status = pullRequestStatus(from: pullRequest.state),
|
||||
let url = URL(string: pullRequest.url) else {
|
||||
#if DEBUG
|
||||
|
|
@ -1519,6 +1542,61 @@ class TabManager: ObservableObject {
|
|||
)
|
||||
}
|
||||
|
||||
nonisolated static func preferredPullRequest(
|
||||
from pullRequests: [GitHubPullRequestProbeItem]
|
||||
) -> GitHubPullRequestProbeItem? {
|
||||
func statusPriority(_ status: SidebarPullRequestStatus) -> Int {
|
||||
switch status {
|
||||
case .open:
|
||||
return 3
|
||||
case .merged:
|
||||
return 2
|
||||
case .closed:
|
||||
return 1
|
||||
}
|
||||
}
|
||||
|
||||
func isPreferred(
|
||||
candidate: GitHubPullRequestProbeItem,
|
||||
over current: GitHubPullRequestProbeItem
|
||||
) -> Bool {
|
||||
guard let candidateStatus = pullRequestStatus(from: candidate.state),
|
||||
let currentStatus = pullRequestStatus(from: current.state) else {
|
||||
return false
|
||||
}
|
||||
|
||||
let candidatePriority = statusPriority(candidateStatus)
|
||||
let currentPriority = statusPriority(currentStatus)
|
||||
if candidatePriority != currentPriority {
|
||||
return candidatePriority > currentPriority
|
||||
}
|
||||
|
||||
let candidateUpdatedAt = candidate.updatedAt ?? ""
|
||||
let currentUpdatedAt = current.updatedAt ?? ""
|
||||
if candidateUpdatedAt != currentUpdatedAt {
|
||||
return candidateUpdatedAt > currentUpdatedAt
|
||||
}
|
||||
|
||||
return candidate.number > current.number
|
||||
}
|
||||
|
||||
var best: GitHubPullRequestProbeItem?
|
||||
for pullRequest in pullRequests {
|
||||
guard pullRequestStatus(from: pullRequest.state) != nil,
|
||||
URL(string: pullRequest.url) != nil else {
|
||||
continue
|
||||
}
|
||||
guard let currentBest = best else {
|
||||
best = pullRequest
|
||||
continue
|
||||
}
|
||||
if isPreferred(candidate: pullRequest, over: currentBest) {
|
||||
best = pullRequest
|
||||
}
|
||||
}
|
||||
return best
|
||||
}
|
||||
|
||||
private nonisolated static func pullRequestChecksStatus(
|
||||
number: Int,
|
||||
directory: String,
|
||||
|
|
@ -1593,17 +1671,6 @@ class TabManager: ObservableObject {
|
|||
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",
|
||||
|
|
@ -1713,14 +1780,66 @@ class TabManager: ObservableObject {
|
|||
)
|
||||
}
|
||||
|
||||
private nonisolated static func githubRepositorySlug(directory: String) -> String? {
|
||||
guard let remoteURL = runGitCommand(
|
||||
directory: directory,
|
||||
arguments: ["remote", "get-url", "origin"]
|
||||
) else {
|
||||
return nil
|
||||
nonisolated static func githubRepositorySlugs(fromGitRemoteVOutput output: String) -> [String] {
|
||||
var slugByRemoteName: [String: String] = [:]
|
||||
|
||||
for line in output.split(whereSeparator: \.isNewline) {
|
||||
let parts = line.split(whereSeparator: \.isWhitespace)
|
||||
guard parts.count >= 3 else { continue }
|
||||
|
||||
let remoteName = String(parts[0])
|
||||
let remoteURL = String(parts[1])
|
||||
let remoteKind = String(parts[2])
|
||||
guard remoteKind == "(fetch)",
|
||||
let repoSlug = githubRepositorySlug(fromRemoteURL: remoteURL) else {
|
||||
continue
|
||||
}
|
||||
|
||||
if slugByRemoteName[remoteName] == nil {
|
||||
slugByRemoteName[remoteName] = repoSlug
|
||||
}
|
||||
}
|
||||
|
||||
let orderedRemoteNames = slugByRemoteName.keys.sorted { lhs, rhs in
|
||||
let lhsPriority = githubRemotePriority(lhs)
|
||||
let rhsPriority = githubRemotePriority(rhs)
|
||||
if lhsPriority != rhsPriority {
|
||||
return lhsPriority < rhsPriority
|
||||
}
|
||||
return lhs < rhs
|
||||
}
|
||||
|
||||
var orderedSlugs: [String] = []
|
||||
var seen: Set<String> = []
|
||||
for remoteName in orderedRemoteNames {
|
||||
guard let repoSlug = slugByRemoteName[remoteName],
|
||||
seen.insert(repoSlug).inserted else {
|
||||
continue
|
||||
}
|
||||
orderedSlugs.append(repoSlug)
|
||||
}
|
||||
return orderedSlugs
|
||||
}
|
||||
|
||||
private nonisolated static func githubRepositorySlugs(directory: String) -> [String] {
|
||||
guard let output = runGitCommand(directory: directory, arguments: ["remote", "-v"]) else {
|
||||
return []
|
||||
}
|
||||
return githubRepositorySlugs(fromGitRemoteVOutput: output)
|
||||
}
|
||||
|
||||
private nonisolated static func githubRemotePriority(_ remoteName: String) -> Int {
|
||||
switch remoteName.lowercased() {
|
||||
case "upstream":
|
||||
return 0
|
||||
case "origin":
|
||||
return 1
|
||||
default:
|
||||
return 2
|
||||
}
|
||||
}
|
||||
|
||||
private nonisolated static func githubRepositorySlug(fromRemoteURL remoteURL: String) -> String? {
|
||||
let trimmed = remoteURL.trimmingCharacters(in: .whitespacesAndNewlines)
|
||||
guard !trimmed.isEmpty else { return nil }
|
||||
|
||||
|
|
|
|||
|
|
@ -117,6 +117,101 @@ final class TabManagerWorkspaceOwnershipTests: XCTestCase {
|
|||
}
|
||||
}
|
||||
|
||||
@MainActor
|
||||
final class TabManagerPullRequestProbeTests: XCTestCase {
|
||||
func testGitHubRepositorySlugsPrioritizeUpstreamThenOriginAndDeduplicate() {
|
||||
let output = """
|
||||
origin https://github.com/austinwang/cmux.git (fetch)
|
||||
origin https://github.com/austinwang/cmux.git (push)
|
||||
upstream git@github.com:manaflow-ai/cmux.git (fetch)
|
||||
upstream git@github.com:manaflow-ai/cmux.git (push)
|
||||
backup ssh://git@github.com/manaflow-ai/cmux.git (fetch)
|
||||
mirror https://gitlab.com/manaflow-ai/cmux.git (fetch)
|
||||
"""
|
||||
|
||||
XCTAssertEqual(
|
||||
TabManager.githubRepositorySlugs(fromGitRemoteVOutput: output),
|
||||
["manaflow-ai/cmux", "austinwang/cmux"]
|
||||
)
|
||||
}
|
||||
|
||||
func testPreferredPullRequestPrefersOpenOverMergedAndClosed() {
|
||||
let candidates = [
|
||||
TabManager.GitHubPullRequestProbeItem(
|
||||
number: 1889,
|
||||
state: "MERGED",
|
||||
url: "https://github.com/manaflow-ai/cmux/pull/1889",
|
||||
updatedAt: "2026-03-20T18:00:00Z"
|
||||
),
|
||||
TabManager.GitHubPullRequestProbeItem(
|
||||
number: 1891,
|
||||
state: "OPEN",
|
||||
url: "https://github.com/manaflow-ai/cmux/pull/1891",
|
||||
updatedAt: "2026-03-19T18:00:00Z"
|
||||
),
|
||||
TabManager.GitHubPullRequestProbeItem(
|
||||
number: 1800,
|
||||
state: "CLOSED",
|
||||
url: "https://github.com/manaflow-ai/cmux/pull/1800",
|
||||
updatedAt: "2026-03-21T18:00:00Z"
|
||||
),
|
||||
]
|
||||
|
||||
XCTAssertEqual(
|
||||
TabManager.preferredPullRequest(from: candidates),
|
||||
candidates[1]
|
||||
)
|
||||
}
|
||||
|
||||
func testPreferredPullRequestPrefersMostRecentlyUpdatedWithinSameStatus() {
|
||||
let olderOpen = TabManager.GitHubPullRequestProbeItem(
|
||||
number: 1880,
|
||||
state: "OPEN",
|
||||
url: "https://github.com/manaflow-ai/cmux/pull/1880",
|
||||
updatedAt: "2026-03-18T18:00:00Z"
|
||||
)
|
||||
let newerOpen = TabManager.GitHubPullRequestProbeItem(
|
||||
number: 1890,
|
||||
state: "OPEN",
|
||||
url: "https://github.com/manaflow-ai/cmux/pull/1890",
|
||||
updatedAt: "2026-03-20T18:00:00Z"
|
||||
)
|
||||
|
||||
XCTAssertEqual(
|
||||
TabManager.preferredPullRequest(from: [olderOpen, newerOpen]),
|
||||
newerOpen
|
||||
)
|
||||
}
|
||||
|
||||
func testPreferredPullRequestIgnoresMalformedCandidates() {
|
||||
let valid = TabManager.GitHubPullRequestProbeItem(
|
||||
number: 1888,
|
||||
state: "OPEN",
|
||||
url: "https://github.com/manaflow-ai/cmux/pull/1888",
|
||||
updatedAt: "2026-03-20T18:00:00Z"
|
||||
)
|
||||
|
||||
XCTAssertEqual(
|
||||
TabManager.preferredPullRequest(from: [
|
||||
TabManager.GitHubPullRequestProbeItem(
|
||||
number: 9999,
|
||||
state: "WHATEVER",
|
||||
url: "https://github.com/manaflow-ai/cmux/pull/9999",
|
||||
updatedAt: "2026-03-21T18:00:00Z"
|
||||
),
|
||||
TabManager.GitHubPullRequestProbeItem(
|
||||
number: 10000,
|
||||
state: "OPEN",
|
||||
url: "not a url",
|
||||
updatedAt: "2026-03-21T18:00:00Z"
|
||||
),
|
||||
valid,
|
||||
]),
|
||||
valid
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@MainActor
|
||||
final class TabManagerCloseWorkspacesWithConfirmationTests: XCTestCase {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue