Skip to content
Open
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
26 changes: 26 additions & 0 deletions Sources/Bloom/State/AppModel+Projects.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ extension AppModel {
case .alreadyRepository(let root):
guard let repo = await addKnownRepository(at: root) else { return nil }
if focusing { await focus(repo) }
// Not awaited: a glab probe to a host behind a VPN that is down takes seconds.
Task { await askForgeIfBoth(repo) }
return repo

case .refuse(let refusal):
Expand Down Expand Up @@ -113,6 +115,30 @@ extension AppModel {
))
}

/// Asks which forge `repo` uses when its remotes are on both, and returns once it is answered.
func askForgeIfBoth(_ repo: Repo) async {
guard forgeQuestion == nil, await ForgeResolver.offersBoth(repo.path), forgeQuestion == nil
else { return }
await withCheckedContinuation { answered in
forgeQuestionAnswered = answered
forgeQuestion = repo
}
}

/// The answer to `forgeQuestion`, kept for every workspace of the project. `nil` is Not now.
func answerForgeQuestion(_ forge: Forge?, for repo: Repo? = nil) {
if let forge, let repo {
do {
try ForgeResolver.remember(forge, for: repo.path)
} catch {
alert = BloomAlert(title: "Could not save the forge", message: error.readableMessage)
}
}
let answered = forgeQuestionAnswered
forgeQuestionAnswered = nil
answered?.resume()
}

/// Called by `ProjectSetupSheet` once the folder really is a repository.
func finishProjectSetup(_ path: String?) async {
ProjectSetup.shared.dismiss()
Expand Down
8 changes: 8 additions & 0 deletions Sources/Bloom/State/AppModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ final class AppModel {
/// Non-nil after an archive script failed, while the owner decides between keeping the
/// workspace and archiving it without the script. RootView presents it.
var pendingScriptFailure: ScriptFailureRequest?
/// A project just added with remotes on both GitHub and GitLab. RootView asks which one it
/// uses. See `ForgeResolver.offersBoth`.
var forgeQuestion: Repo? {
// A dialog dismissed without its buttons must still let `askForgeIfBoth` return.
didSet { if forgeQuestion == nil { Task { self.answerForgeQuestion(nil) } } }
}
/// Resumed once `forgeQuestion` is answered, so the create window can wait for it.
@ObservationIgnored var forgeQuestionAnswered: CheckedContinuation<Void, Never>?
/// The transcript half of a search, one row per workspace. Held here rather than in `HomeView`
/// for the same reason `homeFilter` is: the pane is destroyed and rebuilt every time the
/// selection leaves Home, and a result list that had to be fetched again on the way back would
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ struct WorkspaceHoverCardView: View {
// `verbatim`, for the reason `PullRequestBadge` writes down: an interpolated
// `Int` inside a `LocalizedStringKey` is formatted for the locale, and a
// machine set to Dutch drew "#2.631" for a pull request GitHub calls 2631.
Text(verbatim: "#\(pullRequest.number)")
Text(verbatim: pullRequest.forge.reference(pullRequest.number))
.font(Typo.caption)
.monospacedDigit()
.foregroundStyle(Palette.textSecondary)
Expand Down
8 changes: 4 additions & 4 deletions Sources/Bloom/Views/Inspector/PullRequestSummary.swift
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ struct PullRequestSummary: View {
}

private var archiveHelp: String {
return "Remove this workspace's worktree. #\(pullRequest.number) is merged, so "
return "Remove this workspace's worktree. \(pullRequest.forge.reference(pullRequest.number)) is merged, so "
+ "this asks first only when something here exists nowhere else."
}

Expand Down Expand Up @@ -434,7 +434,7 @@ struct PullRequestSummary: View {
branchActions.reason
?? "Ask this workspace's agent to \(pushLabel.lowercased()) branch "
+ "\(pullRequest.branch.isEmpty ? "this branch" : pullRequest.branch), so "
+ "#\(pullRequest.number) is what is on this disk."
+ "\(pullRequest.forge.reference(pullRequest.number)) is what is on this disk."
)
}

Expand Down Expand Up @@ -502,7 +502,7 @@ struct PullRequestSummary: View {
.help(
branchActions.reason
?? "Ask this workspace's agent to bring \(baseBranch) into this worktree and "
+ "resolve the conflicts here. Nothing is pushed and #\(pullRequest.number)"
+ "resolve the conflicts here. Nothing is pushed and \(pullRequest.forge.reference(pullRequest.number))"
+ " is not merged."
)
}
Expand Down Expand Up @@ -582,7 +582,7 @@ struct PullRequestSummary: View {
/// request is this" without spending any of the strip's width on an answer the reader already
/// has from the workspace name.
private var helpText: String {
var text = "#\(pullRequest.number) \(pullRequest.title)"
var text = "\(pullRequest.forge.reference(pullRequest.number)) \(pullRequest.title)"
if let detail = status.detail { text += "\n\(detail)" }
if let reason = status.blockedReason { text += "\n\(reason)" }
return text
Expand Down
14 changes: 14 additions & 0 deletions Sources/Bloom/Views/RootView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,20 @@ struct RootView: View {
// `ArchiveRequest` in the core, where it can be tested.
Text(request.message)
}
// Asked when a project with remotes on both forges is added and no forge is declared.
// Not now writes nothing: detection stays in charge, which picks GitHub, and asks again.
.confirmationDialog(
"Which forge does this project use?",
isPresented: $app.forgeQuestion.isPresent(),
titleVisibility: .visible,
presenting: app.forgeQuestion
) { repo in
Button(Forge.gitLab.name) { app.answerForgeQuestion(.gitLab, for: repo) }
Button(Forge.gitHub.name) { app.answerForgeQuestion(.gitHub, for: repo) }
Button("Not now", role: .cancel) { app.answerForgeQuestion(nil) }
} message: { repo in
Text("\(repo.name) has remotes on GitHub and GitLab. Every workspace of the project will use the one you choose, and you can change it in the project settings. Until you choose, Bloom uses GitHub.")
}
// The question asked before a session that is still working is closed. On the window for
// the reason the archive confirmation above is: it is raised from the tab strip's close
// button and from Cmd+W in the menu bar, and there is no one control both of those could
Expand Down
2 changes: 2 additions & 0 deletions Sources/Bloom/Views/Sidebar/StartProjectView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,8 @@ struct StartProjectView: View {
// asking its root view to present it.
openWindow(id: BloomApp.mainWindowID)
guard let repo else { return }
// Answered before New Workspace goes up: the main window shows one sheet at a time.
await app.askForgeIfBoth(repo)
Task { @MainActor in
await Task.yield()
NotificationCenter.default.post(name: .bloomNewWorkspace, object: repo)
Expand Down
10 changes: 6 additions & 4 deletions Sources/BloomCore/Bridge/WorkspaceMergeTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -306,14 +306,16 @@ public struct WorkspaceMergeTool: BridgeToolHandling {
}

private func note(workspace: Workspace, pullRequest: PullRequest, chat: String) -> String {
"""
let forge = pullRequest.forge
return """
Nothing is merged. A turn has begun in '\(workspace.name)', in the chat '\(chat)': its \
agent has been asked to merge #\(pullRequest.number) and it runs `gh pr merge` itself, \
agent has been asked to merge \(forge.reference(pullRequest.number)) and it runs \
`\(forge.command("merge"))` itself, \
where the owner can watch it and answer anything it asks. Bloom does not wait for that \
turn and there is no way to wait for it from here, so do not sit idle. The merge may still \
not happen: GitHub is allowed to refuse, and the agent is told to stop and report a \
not happen: \(forge.name) is allowed to refuse, and the agent is told to stop and report a \
refusal rather than force it. To find out what became of it, call workspace_list with \
include_github and read the pull request's state.
include_github and read the \(forge.request)'s state.
"""
}

Expand Down
78 changes: 62 additions & 16 deletions Sources/BloomCore/GitHub/ForgeRouting.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,17 @@ public enum ForgeRouting {
static func isGitLabCom(_ host: String) -> Bool {
host == "gitlab.com" || host.hasSuffix(".gitlab.com")
}

/// The hosts beside a GitHub remote that could make the repository GitLab's as well. Empty
/// without a GitHub remote, because then `decide` answers without anybody being asked.
static func hostsBesideGitHub(_ hosts: [String]) -> Set<String> {
guard hosts.contains(where: isGitHub) else { return [] }
return Set(hosts.filter { !isGitHub($0) })
}
}

/// `ForgeRouting` applied to a directory, remembered until git's config or a settings file changes.
/// `ForgeRouting` applied to a directory, remembered until git's config or a settings file changes,
/// or for as long as a glab refusal lasts when that refusal is what made it GitHub.
public enum ForgeResolver {
private static let resolutions = ForgeResolutions()

Expand All @@ -53,28 +61,65 @@ public enum ForgeResolver {
let fingerprint = fingerprint(of: [configPath].compactMap { $0 } + SettingsLoader.candidatePaths(repo: checkout))
if let known = await resolutions.forge(for: directory, fingerprint: fingerprint) { return known }

let forge = await resolve(directory: directory, checkout: checkout, configPath: configPath)
await resolutions.store(forge, for: directory, fingerprint: fingerprint)
let (forge, lasting) = await resolve(directory: directory, checkout: checkout, configPath: configPath)
// GitHub because glab said no is kept no longer than that no: a VPN that was down comes back.
let expiry = lasting ? nil : ContinuousClock.now + GlabSignIn.refusalLifetime
await resolutions.store(forge, for: directory, fingerprint: fingerprint, expiry: expiry)
return forge
}

private static func resolve(directory: String, checkout: String, configPath: String?) async -> Forge {
if let declared = SettingsLoader.load(repo: checkout).forge { return declared }
private static func resolve(
directory: String, checkout: String, configPath: String?
) async -> (forge: Forge, lasting: Bool) {
if let declared = SettingsLoader.load(repo: checkout).forge { return (declared, true) }
// Settled from the config file's text where possible, so a GitHub user pays no process.
let written = configPath.flatMap { try? String(contentsOfFile: $0, encoding: .utf8) } ?? ""
let hosts = remoteURLs(in: written).compactMap { GitHub.repositoryHost($0) }
if hosts.contains(where: ForgeRouting.isGitHub) { return .gitHub }
if !hosts.contains(where: ForgeRouting.isGitLabCom), GlabSignIn.isInstalled() == false { return .gitHub }
if hosts.contains(where: ForgeRouting.isGitHub) { return (.gitHub, true) }
if !hosts.contains(where: ForgeRouting.isGitLabCom), GlabSignIn.isInstalled() == false { return (.gitHub, true) }

let config = (try? await Git.repositoryConfiguration(in: directory)) ?? [:]
let context = try? await Git.repositoryContext(in: directory)
let remotes = config.filter { $0.key.hasPrefix("remote.") && $0.key.hasSuffix(".url") }.map(\.value)
let remotes = await configuredRemoteURLs(in: directory)
switch ForgeRouting.decide(declared: nil, baseRemoteURL: context?.baseRemoteURL, remoteURLs: remotes) {
case .forge(let decided): return decided
case .askGlab(let host): return await GlabSignIn.shared.isSignedIn(to: host) ? .gitLab : .gitHub
case .forge(let decided): return (decided, true)
case .askGlab(let host):
let signedIn = await GlabSignIn.shared.isSignedIn(to: host)
return (signedIn ? .gitLab : .gitHub, signedIn)
}
}

/// Whether a repository has remotes on both forges and no `git.forge` settling it. `decide`
/// would quietly pick GitHub there, which is wrong for a GitLab project mirrored to GitHub,
/// so the owner is asked instead and the answer goes through `remember`. Never without glab:
/// GitLab is no answer there, and the settings screen hides the choice for the same reason.
public static func offersBoth(_ directory: String) async -> Bool {
guard GlabSignIn.isInstalled() else { return false }
let checkout = mainCheckout(Git.repositoryPaths(in: directory)) ?? directory
guard SettingsLoader.load(repo: checkout).forge == nil else { return false }
let hosts = await configuredRemoteURLs(in: directory).compactMap { GitHub.repositoryHost($0) }
let others = ForgeRouting.hostsBesideGitHub(hosts)
if others.contains(where: ForgeRouting.isGitLabCom) { return true }
for host in others where await GlabSignIn.shared.isSignedIn(to: host) { return true }
return false
}

/// Writes the owner's answer to `.bloom/settings.local.toml` in the project's own checkout,
/// which `forge(for:)` reads for every workspace. Not the shared file: remotes are this
/// machine's git config, so a teammate with only the GitHub remote must not inherit it.
public static func remember(_ forge: Forge, for directory: String) throws {
let checkout = mainCheckout(Git.repositoryPaths(in: directory)) ?? directory
let path = (checkout as NSString).appendingPathComponent(".bloom/settings.local.toml")
var document = SettingsDocument(contentsOf: path)
SettingsWriter.apply(.forge(forge), to: &document)
SettingsWriter.prepareFolder(for: path, repo: checkout)
try document.write(to: path)
}

private static func configuredRemoteURLs(in directory: String) async -> [String] {
let config = (try? await Git.repositoryConfiguration(in: directory)) ?? [:]
return config.filter { $0.key.hasPrefix("remote.") && $0.key.hasSuffix(".url") }.map(\.value)
}

/// The `url = ` values of a git config file, without git. Includes and `insteadOf` are not
/// followed; anything they hide reaches the full resolution instead.
static func remoteURLs(in config: String) -> [String] {
Expand All @@ -97,15 +142,16 @@ public enum ForgeResolver {
}

private actor ForgeResolutions {
private var entries: [String: (fingerprint: [Date?], forge: Forge)] = [:]
private var entries: [String: (fingerprint: [Date?], forge: Forge, expiry: ContinuousClock.Instant?)] = [:]

func forge(for directory: String, fingerprint: [Date?]) -> Forge? {
guard let entry = entries[directory], entry.fingerprint == fingerprint else { return nil }
guard let entry = entries[directory], entry.fingerprint == fingerprint,
entry.expiry.map({ ContinuousClock.now < $0 }) ?? true else { return nil }
return entry.forge
}

func store(_ forge: Forge, for directory: String, fingerprint: [Date?]) {
entries[directory] = (fingerprint, forge)
func store(_ forge: Forge, for directory: String, fingerprint: [Date?], expiry: ContinuousClock.Instant?) {
entries[directory] = (fingerprint, forge, expiry)
}
}

Expand All @@ -114,7 +160,7 @@ private actor ForgeResolutions {
actor GlabSignIn {
static let shared = GlabSignIn()
private static let timeout = Duration.seconds(10)
private static let refusalLifetime = Duration.seconds(300)
static let refusalLifetime = Duration.seconds(300)

private var answers: [String: (signedIn: Bool, at: ContinuousClock.Instant)] = [:]
private var probes: [String: Task<Bool, Never>] = [:]
Expand Down
6 changes: 4 additions & 2 deletions Sources/BloomCore/Presentation/WorkspaceHoverCard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ public struct WorkspaceHoverCard: Sendable, Hashable {
public var number: Int
public var title: String
public var url: String
public var forge: Forge

public init(number: Int, title: String, url: String) {
public init(number: Int, title: String, url: String, forge: Forge = .gitHub) {
self.number = number
self.title = title
self.url = url
self.forge = forge
}
}

Expand Down Expand Up @@ -100,7 +102,7 @@ public struct WorkspaceHoverCard: Sendable, Hashable {
/// `PullRequestOwnership` drops it upstream, which is where that rule lives.
static func reference(to pullRequest: PullRequest?) -> PullRequestRef? {
pullRequest.map {
PullRequestRef(number: $0.number, title: $0.title, url: $0.url)
PullRequestRef(number: $0.number, title: $0.title, url: $0.url, forge: $0.forge)
}
}
}
29 changes: 29 additions & 0 deletions Tests/BloomCoreTests/ForgeRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,35 @@ struct ForgeRoutingTests {
#expect(await ForgeResolver.forge(for: repo.path) == .gitLab)
}

@Test("only hosts beside a GitHub remote can make a repository ambiguous", arguments: [
(hosts: ["github.com", "gitlab.com"], others: Set(["gitlab.com"])),
(hosts: ["github-work", "git.example.fr"], others: Set(["git.example.fr"])),
(hosts: ["gitlab.com", "git.example.fr"], others: Set<String>()),
(hosts: ["github.com"], others: Set<String>()),
])
func hostsBesideGitHub(hosts: [String], others: Set<String>) {
#expect(ForgeRouting.hostsBesideGitHub(hosts) == others)
}

@Test("a repository on both forges asks once, and the answer stays on this machine", .tags(.git))
func choice() async throws {
let repo = try await TempRepo()
defer { repo.cleanUp() }
try await Shell.check("git", ["remote", "add", "origin", "git@gitlab.com:group/app.git"], cwd: repo.path)
try await Shell.check("git", ["remote", "add", "mirror", "git@github.com:acme/app.git"], cwd: repo.path)
let glabInstalled: @Sendable ([String], String?) async throws -> ShellResult = { _, _ in ShellResult(status: 1, stdout: "", stderr: "") }
let asked = await GitLab.$commandOverride.withValue(glabInstalled) { await ForgeResolver.offersBoth(repo.path) }
#expect(asked)
#expect(await ForgeResolver.forge(for: repo.path) == .gitHub)

try ForgeResolver.remember(.gitLab, for: repo.path)
#expect(await ForgeResolver.forge(for: repo.path) == .gitLab)
let askedAgain = await GitLab.$commandOverride.withValue(glabInstalled) { await ForgeResolver.offersBoth(repo.path) }
#expect(askedAgain == false)
#expect(FileManager.default.fileExists(atPath: "\(repo.path)/.bloom/settings.local.toml"))
#expect(FileManager.default.fileExists(atPath: "\(repo.path)/.bloom/settings.toml") == false)
}

@Test("remote addresses are read from git's config file without git")
func remoteURLs() {
let config = "[remote \"origin\"]\n\turl = git@github.com:acme/app.git\n\tpushurl = x\n[remote \"up\"]\n\turl=\"https://gitlab.com/g/a\"\n"
Expand Down
18 changes: 18 additions & 0 deletions Tests/BloomCoreTests/WorkspaceMergeToolTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,24 @@ struct WorkspaceMergeToolTests {
#expect(!json["state"]!.stringValue!.contains("merge"))
}

@Test("on GitLab the answer names the merge request, glab and GitLab")
func theAnswerOnGitLab() async throws {
let store = try makeTestStore("merge-answer-gitlab")
let target = try await workspace(in: store)
var mergeRequest = Self.open()
mergeRequest.forge = .gitLab

let result = await tool(reading: .found(mergeRequest, local: nil)).call(
request(["workspace": .string(target.id.rawValue)]), as: .owner, store: store
)

let note = try #require(try answer(result)["note"]?.stringValue)
#expect(note.contains("asked to merge !42 and it runs `glab mr merge` itself"))
#expect(note.contains("GitLab is allowed to refuse"))
#expect(note.contains("read the merge request's state"))
#expect(!note.contains("gh "))
}

@Test("squash is what it proposes, and the other two are carried through unaltered")
func methods() async throws {
let store = try makeTestStore("merge-methods")
Expand Down
Loading
Loading