diff --git a/Sources/Bloom/State/AppModel+Workspaces.swift b/Sources/Bloom/State/AppModel+Workspaces.swift index 29ed40d80..564b67cdd 100644 --- a/Sources/Bloom/State/AppModel+Workspaces.swift +++ b/Sources/Bloom/State/AppModel+Workspaces.swift @@ -650,7 +650,7 @@ extension AppModel { pullRequest: PullRequest, in model: WorkspaceModel ) async { - let template = PromptOverrides().template(for: .continueAfterMerge) + let template = PromptOverrides().template(for: .continueAfterMerge, forge: pullRequest.forge) let render = continuation.render(template: template, pullRequest: pullRequest.number) guard let session = await continuationSession(in: model) else { return } diff --git a/Sources/Bloom/State/Bridges.swift b/Sources/Bloom/State/Bridges.swift index ace900d88..a9ece9fa7 100644 --- a/Sources/Bloom/State/Bridges.swift +++ b/Sources/Bloom/State/Bridges.swift @@ -5,11 +5,13 @@ import BloomCore /// The one place the app layer touches `GitHub`. Keeping it behind a single adapter means a /// change to the gh wrapper's signature is a one-file fix rather than a sweep through views. /// -/// Pull requests can be prepared by an agent or directly through git and gh. Direct merges and -/// marking a draft ready also use the GitHub CLI without involving an agent. +/// Pull requests can be prepared by an agent or directly through git and gh, or glab on GitLab. +/// Direct merges and marking a draft ready also use the forge's CLI without involving an agent. enum GitHubBridge { static func createPullRequestDirectly(for workspace: Workspace) async throws -> PullRequest { - try await DirectPullRequestCreation.create(for: workspace) + try await DirectPullRequestCreation.create( + for: workspace, forge: await ForgeResolver.forge(for: workspace.path) + ) } static func readPullRequest(for workspace: Workspace, maxAge: Duration = .zero) async -> PullRequestRead { guard await ForgeResolver.forge(for: workspace.path) == .gitHub else { @@ -51,7 +53,9 @@ enum GitHubBridge { static func mergePullRequest( _ pullRequest: PullRequest, base: String, method: GitHub.MergeMethod, worktree: String ) async throws -> DirectMergeOutcome { - try await GitHub.mergePullRequest(pullRequest, into: base, method: method, worktree: worktree) + try await ForgeResolver.client(of: pullRequest.forge).mergePullRequest( + pullRequest, into: base, method: method, worktree: worktree + ) } /// Nil when GitHub refused this token the check runs. A failed read is still an empty list, diff --git a/Sources/Bloom/State/WorkspaceModel+PullRequest.swift b/Sources/Bloom/State/WorkspaceModel+PullRequest.swift index 6f7a28318..a029eb477 100644 --- a/Sources/Bloom/State/WorkspaceModel+PullRequest.swift +++ b/Sources/Bloom/State/WorkspaceModel+PullRequest.swift @@ -20,6 +20,12 @@ extension WorkspaceModel { forge == .gitLab ? .ready : GitHubAvailability.shared.state } + /// gh's readiness, asked afresh, or ready on GitLab. For the Checks tab's poll. + func checksReadiness() async -> GitHubAvailability.State { + forge = await ForgeResolver.forge(for: workspace.path) + return forge == .gitLab ? .ready : await GitHubAvailability.shared.check() + } + func continueAfterMerge(_ pullRequest: PullRequest) async -> AppModel.ContinuationOutcome { await app.continueAfterMerge(workspace, pullRequest: pullRequest) } diff --git a/Sources/Bloom/State/WorkspaceModel.swift b/Sources/Bloom/State/WorkspaceModel.swift index 1b107cbbc..a72d393f8 100644 --- a/Sources/Bloom/State/WorkspaceModel.swift +++ b/Sources/Bloom/State/WorkspaceModel.swift @@ -2503,7 +2503,8 @@ final class WorkspaceModel { /// /// Returns nil on success, or the sentence to put in front of the user. func requestPullRequest(overrides: PromptOverrides = PromptOverrides()) async -> String? { - let template = overrides.template(for: .createPullRequest) + let forge = await ForgeResolver.forge(for: workspace.path) + let template = overrides.template(for: .createPullRequest, forge: forge) let wanted = Set(PromptTemplate.variableNames(in: template)) // Only what this template actually asks for. The built-in one names the target branch and @@ -2532,7 +2533,7 @@ final class WorkspaceModel { // pressed a button in the inspector should be looking at the answer to it. activeSessionID = session.id isExpectingPullRequest = true - await transcript(for: session).submit(await pullRequestTurn(text: render.text)) + await transcript(for: session).submit(await pullRequestTurn(text: render.text, forge: forge)) return nil } @@ -2559,7 +2560,7 @@ final class WorkspaceModel { /// /// Returns nil on success, or the sentence to put in front of the user. func requestPush(overrides: PromptOverrides = PromptOverrides()) async -> String? { - let template = overrides.template(for: .pushLocalWork) + let template = overrides.template(for: .pushLocalWork, forge: await ForgeResolver.forge(for: workspace.path)) let wanted = Set(PromptTemplate.variableNames(in: template)) if wanted.contains(PromptRegistry.PushLocalWork.changes) { @@ -2622,11 +2623,12 @@ final class WorkspaceModel { title: pullRequest.title, branch: pullRequest.branch, baseBranch: workspace.baseBranch, - method: method + method: method, + forge: pullRequest.forge ) - let render = context.render(template: overrides.template(for: .mergePullRequest)) + let render = context.render(template: overrides.template(for: .mergePullRequest, forge: pullRequest.forge)) - let text = await turn(render.text, for: .merge) + let text = await turn(render.text, for: .merge, forge: pullRequest.forge) activeSessionID = session.id await transcript(for: session).submit(text) return nil @@ -2653,7 +2655,9 @@ final class WorkspaceModel { /// refreshed when the workspace is selected, and the sequence that has to work is typing an /// instruction in the project settings window and pressing Merge in the window behind it /// without touching the sidebar in between. - private func turn(_ text: String, for subject: ProjectInstructions.Subject) async -> String { + private func turn( + _ text: String, for subject: ProjectInstructions.Subject, forge: Forge + ) async -> String { await reloadSettings() let stated = ProjectInstructions.stated(subject, in: settings) let path = workspace.path @@ -2662,7 +2666,7 @@ final class WorkspaceModel { let extra = await Task.detached(priority: .userInitiated) { ProjectInstructions.resolve(subject, in: path, stated: stated) }.value - return ProjectInstructions.turn(text, for: subject, adding: extra) + return ProjectInstructions.turn(text, for: subject, adding: extra, forge: forge) } /// Asks the workspace's agent to bring the base branch in and resolve the conflicts. @@ -2699,15 +2703,17 @@ final class WorkspaceModel { branch: workspace.branch, baseBranch: workspace.baseBranch ) - let render = context.render(template: overrides.template(for: .fixConflicts)) + let forge = pullRequest.forge + let render = context.render(template: overrides.template(for: .fixConflicts, forge: forge)) let path = workspace.path let rendered = render.text + let contents = forge == .gitLab ? GitLabInstructions.conflictMarkdown : ConflictInstructions.defaultMarkdown // Off the main actor: it writes a file into the worktree, and this runs on a button press. let asked = await Task.detached(priority: .userInitiated) { - ConflictInstructions.asking(rendered, in: path) + ConflictInstructions.asking(rendered, in: path, contents: contents) }.value - let text = await turn(asked, for: .fixConflicts) + let text = await turn(asked, for: .fixConflicts, forge: forge) activeSessionID = session.id await transcript(for: session).submit(text) return nil @@ -2722,11 +2728,14 @@ final class WorkspaceModel { /// /// When the file cannot be written, the instructions go into the message itself. A read-only /// checkout is a reason to say it differently, not a reason for the button to stop working. - private func pullRequestTurn(text: String) async -> String { - if let path = await PullRequestInstructions.ensure(in: workspace.path) { + private func pullRequestTurn(text: String, forge: Forge) async -> String { + let (contents, scratch) = forge == .gitLab + ? (GitLabInstructions.mergeRequestMarkdown, GitLabInstructions.mergeRequestScratchPath) + : (PullRequestInstructions.defaultMarkdown, PullRequestInstructions.scratchPath) + if let path = await PullRequestInstructions.ensure(in: workspace.path, contents: contents, scratch: scratch) { return PullRequestInstructions.asking(text, toFollow: path) } - return text + "\n\n" + PullRequestInstructions.defaultMarkdown + return text + "\n\n" + contents } /// A workspace whose agent was never started still has a button to press. Rather than doing diff --git a/Sources/Bloom/Views/CreateWorkspace/CreateWorkspaceView.swift b/Sources/Bloom/Views/CreateWorkspace/CreateWorkspaceView.swift index 7693c4987..ada71edc3 100644 --- a/Sources/Bloom/Views/CreateWorkspace/CreateWorkspaceView.swift +++ b/Sources/Bloom/Views/CreateWorkspace/CreateWorkspaceView.swift @@ -927,7 +927,8 @@ struct CreateWorkspaceView: View { pullRequests: checkoutOptions.pullRequests, branches: checkoutOptions.branches, baseBranches: branchOptions, - defaultBranch: repo?.defaultBranch ?? "" + defaultBranch: repo?.defaultBranch ?? "", + forge: checkoutOptions.forge ) } diff --git a/Sources/Bloom/Views/Inspector/BranchActionsPopover.swift b/Sources/Bloom/Views/Inspector/BranchActionsPopover.swift index a53d4f5f7..456dbe3c7 100644 --- a/Sources/Bloom/Views/Inspector/BranchActionsPopover.swift +++ b/Sources/Bloom/Views/Inspector/BranchActionsPopover.swift @@ -5,10 +5,10 @@ enum PullRequestCreationRoute: String, CaseIterable { case agent case direct - var label: String { + func label(on forge: Forge) -> String { switch self { - case .agent: "Ask agent to create PR" - case .direct: "Commit and create PR with GitHub CLI" + case .agent: "Ask agent to create \(forge.shortRequest)" + case .direct: "Commit and create \(forge.shortRequest) with \(forge.name) CLI" } } } @@ -23,8 +23,8 @@ struct BranchActionsPopover: View { var body: some View { ConfirmationPopover( - title: "Create pull request?", - confirmLabel: "Create PR", + title: "Create \(model.forge.request)?", + confirmLabel: "Create \(model.forge.shortRequest)", tint: Palette.controlAccent, canConfirm: route == .agent || (model.gitHubReadiness.isUsable && !model.isRunning), onConfirm: { @@ -36,23 +36,23 @@ struct BranchActionsPopover: View { ) { Picker("Create using", selection: $route) { ForEach(PullRequestCreationRoute.allCases, id: \.self) { choice in - Text(choice.label).tag(choice) + Text(choice.label(on: model.forge)).tag(choice) } } .pickerStyle(.radioGroup) if route == .direct { - Text("Bloom stages and commits all local changes as “\(model.workspace.name)”, pushes the branch, then opens a pull request with gh.") + Text("Bloom stages and commits all local changes as “\(model.workspace.name)”, pushes the branch, then opens a \(model.forge.request) with \(model.forge.cli).") .foregroundStyle(.secondary) if !model.gitHubReadiness.isUsable { - Text("Connect GitHub to use the direct route.") + Text("Connect \(model.forge.name) to use the direct route.") .foregroundStyle(Palette.negative) } else if model.isRunning { Text("Wait for the agent to finish before committing directly.") .foregroundStyle(Palette.negative) } } else { - Text("The agent will prepare the commits, push, and create the pull request using this project's instructions.") + Text("The agent will prepare the commits, push, and create the \(model.forge.request) using this project's instructions.") .foregroundStyle(.secondary) } } diff --git a/Sources/Bloom/Views/Inspector/ChecksView.swift b/Sources/Bloom/Views/Inspector/ChecksView.swift index 3db9942ad..4e020d733 100644 --- a/Sources/Bloom/Views/Inspector/ChecksView.swift +++ b/Sources/Bloom/Views/Inspector/ChecksView.swift @@ -158,10 +158,10 @@ struct ChecksView: View { EmptyStateView( glyph: "checkmark.seal", title: "No checks", - message: "GitHub has not reported a check run for this branch." + message: "\(model.forge.name) has not reported a check run for this branch." ) } else { - LoadingView("Asking GitHub") + LoadingView("Asking \(model.forge.name)") } } @@ -220,7 +220,7 @@ struct ChecksView: View { // Asked every pass rather than once: gh can be signed in from a terminal while this // tab is open, and the answer is cached, so this costs a subprocess only when it // has actually expired. - let state = await GitHubAvailability.shared.check() + let state = await model.checksReadiness() github = state if state == .ready { diff --git a/Sources/Bloom/Views/Inspector/MergeConfirmationPopover.swift b/Sources/Bloom/Views/Inspector/MergeConfirmationPopover.swift index 8bd35e705..67616823b 100644 --- a/Sources/Bloom/Views/Inspector/MergeConfirmationPopover.swift +++ b/Sources/Bloom/Views/Inspector/MergeConfirmationPopover.swift @@ -30,7 +30,7 @@ struct MergeConfirmationPopover: View { ) { Picker("Merge using", selection: executionBinding) { ForEach(MergeExecution.allCases, id: \.self) { choice in - Text(choice.label).tag(choice) + Text(choice.label(on: pullRequest.forge)).tag(choice) } } .pickerStyle(.radioGroup) @@ -41,11 +41,16 @@ struct MergeConfirmationPopover: View { } if execution == .direct { - Text("Bloom will run gh pr merge with the selected \(method.label.lowercased()) method. GitHub may queue the merge if this repository requires it.") - Text("Bloom will not delete the branch or change this checkout. GitHub may delete the remote branch if the repository is configured to do so.") + let forge = pullRequest.forge + Text("Bloom will run \(forge.command("merge")) with the selected \(method.label.lowercased()) method. \(forge.name) may queue the merge if this repository requires it.") + if forge == .gitLab, method == .rebase { + Text("GitLab rebases the branch first, which starts a new pipeline. A project that waits for it will not merge yet.") + .foregroundStyle(.secondary) + } + Text("Bloom will not delete the branch or change this checkout. \(forge.name) may delete the remote branch if the repository is configured to do so.") .foregroundStyle(.secondary) if !canMergeDirectly { - Text("Refresh this pull request before merging directly.") + Text("Refresh this \(forge.request) before merging directly.") .foregroundStyle(Palette.negative) } } else { diff --git a/Sources/Bloom/Views/Inspector/PullRequestBadge.swift b/Sources/Bloom/Views/Inspector/PullRequestBadge.swift index 2b9a845e1..13167a0e3 100644 --- a/Sources/Bloom/Views/Inspector/PullRequestBadge.swift +++ b/Sources/Bloom/Views/Inspector/PullRequestBadge.swift @@ -26,6 +26,7 @@ struct PullRequestBadge: View { var url: String /// The state's colour, or nil for the states that carry none. var tint: Color? + var forge: Forge = .gitHub private var ink: Color { tint ?? Palette.textSecondary } @@ -37,7 +38,7 @@ struct PullRequestBadge: View { // a machine set to Dutch #2631 came out as "#2.631": a pull request number with a // thousands separator in it, which is not a number GitHub has. The chip this // replaces was handed a `String` and never had the problem. - Text(verbatim: "#\(number)") + Text(verbatim: forge.reference(number)) // Monospaced digits, so a four digit number and a five digit one are the same // shape and the badge cannot appear to jitter as a poll comes back. .font(Typo.label) @@ -71,8 +72,8 @@ struct PullRequestBadge: View { .accessibilityElement(children: .ignore) .accessibilityAddTraits(.isButton) .accessibilityLabel("Pull request \(number), \(title)") - .accessibilityHint("Opens on GitHub") - .help("Open #\(number) on GitHub: \(title)") + .accessibilityHint("Opens on \(forge.name)") + .help("Open \(forge.reference(number)) on \(forge.name): \(title)") } /// The arrow's own route, unchanged and not duplicated: there is one way this app opens a diff --git a/Sources/Bloom/Views/Inspector/PullRequestBar.swift b/Sources/Bloom/Views/Inspector/PullRequestBar.swift index bd7ef0206..a585eacbc 100644 --- a/Sources/Bloom/Views/Inspector/PullRequestBar.swift +++ b/Sources/Bloom/Views/Inspector/PullRequestBar.swift @@ -123,6 +123,7 @@ struct PullRequestBar: View { branchActions: branchActions, worktree: model.workspace.path, github: model.gitHubReadiness, + forge: model.forge, hasChanges: hasChanges, hasReadChanges: model.hasReadChanges, continued: model.continued, @@ -190,13 +191,14 @@ struct PullRequestBar: View { case .direct: do { let created = try await model.createPullRequestDirectly() + let forge = created.forge report = PullRequestNotice( - tone: .info, title: "Pull request created", - message: "Opened #\(created.number) on GitHub." + tone: .info, title: "\(forge.titledRequest) created", + message: "Opened \(forge.reference(created.number)) on \(forge.name)." ) } catch { report = PullRequestNotice( - tone: .failure, title: "Could not create the pull request", + tone: .failure, title: "Could not create the \(model.forge.request)", message: String(describing: error) ) } @@ -324,7 +326,7 @@ struct PullRequestBar: View { } /// The confirmation chose how to execute the merge. The direct path stays in the model, - /// keeping subprocesses out of the view and reporting gh's answer beside the inspector. + /// keeping subprocesses out of the view and reporting gh's or glab's answer beside the inspector. private func merge(_ method: GitHub.MergeMethod, execution: MergeExecution) { guard let pullRequest = model.pullRequest else { return } isWorking = true @@ -342,23 +344,25 @@ struct PullRequestBar: View { case .direct: do { let outcome = try await model.mergePullRequestDirectly(pullRequest, method: method) + let forge = pullRequest.forge + let reference = forge.reference(pullRequest.number) let message: String switch outcome { case .merged: - message = "#\(pullRequest.number) merged. Bloom did not delete the branch or change this checkout." + message = "\(reference) merged. Bloom did not delete the branch or change this checkout." case .acceptedButOpen: - message = "#\(pullRequest.number) is still open, possibly waiting in a merge queue." + message = "\(reference) is still open, possibly waiting in a merge queue." case .acceptedUnverified: - message = "gh accepted the command, but Bloom could not verify #\(pullRequest.number)'s final status." + message = "\(forge.cli) accepted the command, but Bloom could not verify \(reference)'s final status." } report = PullRequestNotice( tone: .info, - title: outcome == .merged ? "Merged on GitHub" : "GitHub accepted the merge", + title: outcome == .merged ? "Merged on \(forge.name)" : "\(forge.name) accepted the merge", message: message ) } catch { report = PullRequestNotice( - tone: .failure, title: "Could not merge on GitHub", + tone: .failure, title: "Could not merge on \(pullRequest.forge.name)", message: String(describing: error) ) } diff --git a/Sources/Bloom/Views/Inspector/PullRequestCreator.swift b/Sources/Bloom/Views/Inspector/PullRequestCreator.swift index 0e48ff362..a6a88e7fe 100644 --- a/Sources/Bloom/Views/Inspector/PullRequestCreator.swift +++ b/Sources/Bloom/Views/Inspector/PullRequestCreator.swift @@ -32,6 +32,7 @@ struct PullRequestCreator: View { /// workspace with no worktree on this Mac, which is also one that is never in the dark. var worktree: String var github: GitHubAvailability.State + var forge: Forge = .gitHub /// Whether this branch has anything on it at all. A worktree identical to its base has nothing /// to open a pull request for, and Bloom knows that for free, so it says so here rather than /// spending a whole agent turn on the agent finding out. @@ -152,18 +153,18 @@ struct PullRequestCreator: View { if let note = branchActions.note { return note } if !hasChanges, !hasReadChanges { return "Comparing with \(baseBranch)…" } guard hasChanges else { return ContinuedBranch.line(on: branch, continued: continued) } - return "No pull request yet. Target \(baseBranch)." + return "No \(forge.request) yet. Target \(baseBranch)." } /// A single title-bar control. Its popover chooses how to create the PR now. private var branchActionsButton: some View { - Button("Create PR") { isShowingBranchActions = true } + Button("Create \(forge.shortRequest)") { isShowingBranchActions = true } .buttonStyle(.borderedProminent) .buttonBorderShape(.roundedRectangle(radius: Metrics.corner)) .tint(Palette.controlAccent) .controlSize(.regular) .disabled(!branchActions.isAllowed) - .help("Create a pull request for \(branch)") + .help("Create a \(forge.request) for \(branch)") .popover(isPresented: $isShowingBranchActions, arrowEdge: .top) { BranchActionsPopover(model: model, onCreatePullRequest: action) } diff --git a/Sources/Bloom/Views/Inspector/PullRequestSummary.swift b/Sources/Bloom/Views/Inspector/PullRequestSummary.swift index 4fed8208a..be26071d3 100644 --- a/Sources/Bloom/Views/Inspector/PullRequestSummary.swift +++ b/Sources/Bloom/Views/Inspector/PullRequestSummary.swift @@ -80,7 +80,7 @@ struct PullRequestSummary: View { // the part the reader is trying to read. A right click costs no width at all, and it is // where a link is asked for everywhere else on this system. .contextMenu { - Button("Open on GitHub") { GitHubBridge.open(pullRequest.url) } + Button("Open on \(pullRequest.forge.name)") { GitHubBridge.open(pullRequest.url) } Button("Copy link", action: copyLink) if let url = URL(string: pullRequest.url) { // A submenu of this menu rather than a picker that replaces it, which is where @@ -111,7 +111,8 @@ struct PullRequestSummary: View { number: pullRequest.number, title: pullRequest.title, url: pullRequest.url, - tint: tint + tint: tint, + forge: pullRequest.forge ) if pullRequest.isDraft { draftChip } } @@ -128,7 +129,7 @@ struct PullRequestSummary: View { /// is the other thing about this pull request that is simply true. private var draftChip: some View { Chip(text: "Draft", tint: Palette.textSecondary, background: Palette.hover) - .help("This pull request is still a draft, so it cannot be merged.") + .help("This \(pullRequest.forge.request) is still a draft, so it cannot be merged.") .accessibilityLabel("Draft") } @@ -448,7 +449,7 @@ struct PullRequestSummary: View { .fixedSize() .help( branchActions.reason - ?? "Mark #\(pullRequest.number) ready for review on GitHub." + ?? "Mark \(pullRequest.forge.reference(pullRequest.number)) ready for review on \(pullRequest.forge.name)." ) } diff --git a/Sources/BloomCore/GitHub/DirectMergeCheck.swift b/Sources/BloomCore/GitHub/DirectMergeCheck.swift new file mode 100644 index 000000000..c8e7279cd --- /dev/null +++ b/Sources/BloomCore/GitHub/DirectMergeCheck.swift @@ -0,0 +1,56 @@ +import Foundation + +/// What a direct merge checks on either forge before running the CLI: the request as shown, this +/// checkout at its head, and a fresh read that still says the same. GitHub's words stay as they were. +enum DirectMergeCheck { + /// The confirmed head commit and base branch, once this checkout is known to be exactly that. + static func confirmed( + _ expected: PullRequest, base: String, worktree: String + ) async throws -> (head: String, base: String) { + let forge = expected.forge + guard expected.number > 0, !expected.url.isEmpty, + let head = expected.headRevision, isCommitID(head), + let expectedBase = expected.baseBranch, !expectedBase.isEmpty else { + throw GitHubError("Refresh the \(forge.request) before merging directly.") + } + if !expected.branch.isEmpty, + try await Git.currentBranch(of: worktree) != expected.branch { + throw GitHubError("This worktree is no longer on the \(forge.request)'s branch.") + } + + let local = try await Git.localWork(worktree: worktree) + guard !local.isAhead else { + throw GitHubError("This worktree has changes or commits \(forge.name) has not got. Commit and push them, or choose the agent route after reviewing the warning.") + } + guard try await Git.headSHA(of: worktree) == head else { + throw GitHubError("This checkout is not at the \(forge.request)'s head commit. Refresh or update it before merging directly.") + } + + let context = try await Git.repositoryContext(in: worktree, baseBranch: base) + guard expectedBase == context.baseBranch else { + throw GitHubError("The \(forge.request) targets \(expectedBase), not \(context.baseBranch). Refresh it before merging.") + } + return (head, expectedBase) + } + + /// A fresh read of the same request: unchanged since it was shown, and still mergeable. + static func verify(_ current: PullRequest?, against expected: PullRequest, head: String, base: String) throws { + let forge = expected.forge + guard let current else { + throw GitHubError("\(forge.name) no longer has \(forge.request) \(forge.reference(expected.number)).") + } + guard current.url == expected.url, current.branch == expected.branch, + current.baseBranch == base, current.headRevision == head else { + throw GitHubError("The \(forge.request) changed since it was shown. Refresh it before merging.") + } + guard current.isOpen, current.status.canMerge else { + throw GitHubError(current.status.blockedReason ?? "This \(forge.request) can no longer be merged.") + } + } + + static func isCommitID(_ value: String) -> Bool { + (value.count == 40 || value.count == 64) && value.utf8.allSatisfy { + (48...57).contains($0) || (65...70).contains($0) || (97...102).contains($0) + } + } +} diff --git a/Sources/BloomCore/GitHub/DirectPullRequestCreation.swift b/Sources/BloomCore/GitHub/DirectPullRequestCreation.swift index a5355e4dd..31ba86a08 100644 --- a/Sources/BloomCore/GitHub/DirectPullRequestCreation.swift +++ b/Sources/BloomCore/GitHub/DirectPullRequestCreation.swift @@ -1,9 +1,10 @@ import Foundation -/// Commits and publishes a workspace branch before asking GitHub to open its pull request. +/// Commits and publishes a workspace branch before asking the forge to open its pull request. /// Each step is explicit so a failure leaves the completed git work in place and reports it. public enum DirectPullRequestCreation { - public static func create(for workspace: Workspace) async throws -> PullRequest { + public static func create(for workspace: Workspace, forge: Forge = .gitHub) async throws -> PullRequest { + let client = ForgeResolver.client(of: forge) let worktree = workspace.path guard try await Git.currentBranch(of: worktree) == workspace.branch else { throw GitHubError("This worktree is no longer on \(workspace.branch).") @@ -12,9 +13,9 @@ public enum DirectPullRequestCreation { throw GitHubError("The target branch is not a valid Git branch name.") } - switch await GitHub.readPullRequest(for: workspace, maxAge: .zero) { + switch await client.readPullRequest(for: workspace, maxAge: .zero) { case .current(.some(let existing)): - throw GitHubError("Pull request #\(existing.number) already exists for this branch.") + throw GitHubError("\(forge.titledRequest) \(forge.reference(existing.number)) already exists for this branch.") case .current(nil): break case .unavailable(let failure): @@ -48,11 +49,11 @@ public enum DirectPullRequestCreation { throw GitHubError(prefix + String(describing: error)) } do { - return try await GitHub.createPullRequestFromCommits( + return try await client.createPullRequestFromCommits( worktree: worktree, base: workspace.baseBranch, title: title ) } catch { - throw GitHubError("The branch was pushed, but GitHub did not create the pull request: \(error)") + throw GitHubError("The branch was pushed, but \(forge.name) did not create the \(forge.request): \(error)") } } } diff --git a/Sources/BloomCore/GitHub/Forge.swift b/Sources/BloomCore/GitHub/Forge.swift index 9b0e43454..8a89c39d1 100644 --- a/Sources/BloomCore/GitHub/Forge.swift +++ b/Sources/BloomCore/GitHub/Forge.swift @@ -14,6 +14,32 @@ public enum Forge: String, Sendable, Hashable, Codable { } } + /// `request` at the start of a sentence. + public var titledRequest: String { + request.prefix(1).uppercased() + request.dropFirst() + } + + /// `PR` or `MR`, where a button has room for little else. + public var shortRequest: String { + switch self { + case .gitHub: "PR" + case .gitLab: "MR" + } + } + + /// The command line tool Bloom runs for this forge. + public var cli: String { + switch self { + case .gitHub: "gh" + case .gitLab: "glab" + } + } + + /// `gh pr merge`, `glab mr merge`. + public func command(_ verb: String) -> String { + "\(cli) \(self == .gitHub ? "pr" : "mr") \(verb)" + } + public var name: String { switch self { case .gitHub: "GitHub" diff --git a/Sources/BloomCore/GitHub/ForgeClient.swift b/Sources/BloomCore/GitHub/ForgeClient.swift index 79dcae209..500e09a30 100644 --- a/Sources/BloomCore/GitHub/ForgeClient.swift +++ b/Sources/BloomCore/GitHub/ForgeClient.swift @@ -4,6 +4,7 @@ import Foundation /// callers pass, so every default stays on `GitHub` rather than being restated here. public protocol ForgeClient { static func access(in directory: String) async -> GitHubAccess + static func readPullRequest(for workspace: Workspace, maxAge: Duration) async -> PullRequestRead static func pullRequest(for workspace: Workspace, maxAge: Duration) async throws -> PullRequest? static func pullRequest(forBranch branch: String, worktree: String) async throws -> PullRequest? static func checks(for workspace: Workspace) async throws -> [CheckRun]? @@ -13,6 +14,10 @@ public protocol ForgeClient { static func pullRequestSummary(number: Int, repoPath: String) async throws -> PullRequestListing static func repositorySlug(repoPath: String) async -> String? static func checkoutPullRequest(number: Int, into worktree: String, localBranch: String) async throws + static func createPullRequestFromCommits(worktree: String, base: String, title: String) async throws -> PullRequest + static func mergePullRequest( + _ expected: PullRequest, into base: String, method: GitHub.MergeMethod, worktree: String + ) async throws -> DirectMergeOutcome } /// `GitHub` as a `ForgeClient`. It only forwards. @@ -22,6 +27,10 @@ public enum GitHubForge: ForgeClient { await GitHub.access() } + public static func readPullRequest(for workspace: Workspace, maxAge: Duration) async -> PullRequestRead { + await GitHub.readPullRequest(for: workspace, maxAge: maxAge) + } + public static func pullRequest(for workspace: Workspace, maxAge: Duration) async throws -> PullRequest? { try await GitHub.pullRequest(for: workspace, maxAge: maxAge) } @@ -61,4 +70,16 @@ public enum GitHubForge: ForgeClient { public static func checkoutPullRequest(number: Int, into worktree: String, localBranch: String) async throws { try await GitHub.checkoutPullRequest(number: number, into: worktree, localBranch: localBranch) } + + public static func createPullRequestFromCommits( + worktree: String, base: String, title: String + ) async throws -> PullRequest { + try await GitHub.createPullRequestFromCommits(worktree: worktree, base: base, title: title) + } + + public static func mergePullRequest( + _ expected: PullRequest, into base: String, method: GitHub.MergeMethod, worktree: String + ) async throws -> DirectMergeOutcome { + try await GitHub.mergePullRequest(expected, into: base, method: method, worktree: worktree) + } } diff --git a/Sources/BloomCore/GitHub/ForgeRouting.swift b/Sources/BloomCore/GitHub/ForgeRouting.swift index 3a5b3c552..d08888260 100644 --- a/Sources/BloomCore/GitHub/ForgeRouting.swift +++ b/Sources/BloomCore/GitHub/ForgeRouting.swift @@ -36,7 +36,11 @@ public enum ForgeResolver { private static let resolutions = ForgeResolutions() public static func client(for directory: String) async -> any ForgeClient.Type { - switch await forge(for: directory) { + client(of: await forge(for: directory)) + } + + public static func client(of forge: Forge) -> any ForgeClient.Type { + switch forge { case .gitHub: GitHubForge.self case .gitLab: GitLab.self } diff --git a/Sources/BloomCore/GitHub/GitHub.swift b/Sources/BloomCore/GitHub/GitHub.swift index 040b6adf3..b49b5a840 100644 --- a/Sources/BloomCore/GitHub/GitHub.swift +++ b/Sources/BloomCore/GitHub/GitHub.swift @@ -394,40 +394,13 @@ public enum GitHub { public static func mergePullRequest( _ expected: PullRequest, into base: String, method: MergeMethod, worktree: String ) async throws -> DirectMergeOutcome { - guard expected.number > 0, !expected.url.isEmpty, - let expectedHead = expected.headRevision, Self.isCommitID(expectedHead), - let expectedBase = expected.baseBranch, !expectedBase.isEmpty else { - throw GitHubError("Refresh the pull request before merging directly.") - } - if !expected.branch.isEmpty, - try await Git.currentBranch(of: worktree) != expected.branch { - throw GitHubError("This worktree is no longer on the pull request's branch.") - } - - let local = try await Git.localWork(worktree: worktree) - guard !local.isAhead else { - throw GitHubError("This worktree has changes or commits GitHub has not got. Commit and push them, or choose the agent route after reviewing the warning.") - } - guard try await Git.headSHA(of: worktree) == expectedHead else { - throw GitHubError("This checkout is not at the pull request's head commit. Refresh or update it before merging directly.") - } - - let context = try await Git.repositoryContext(in: worktree, baseBranch: base) - guard expectedBase == context.baseBranch else { - throw GitHubError("The pull request targets \(expectedBase), not \(context.baseBranch). Refresh it before merging.") - } - guard let current = try await snapshot( + let (expectedHead, expectedBase) = try await DirectMergeCheck.confirmed( + expected, base: base, worktree: worktree + ) + let current = try await snapshot( forNumber: expected.number, worktree: worktree, maxAge: .zero - )?.pullRequest else { - throw GitHubError("GitHub no longer has pull request #\(expected.number).") - } - guard current.url == expected.url, current.branch == expected.branch, - current.baseBranch == expectedBase, current.headRevision == expectedHead else { - throw GitHubError("The pull request changed since it was shown. Refresh it before merging.") - } - guard current.isOpen, current.status.canMerge else { - throw GitHubError(current.status.blockedReason ?? "This pull request can no longer be merged.") - } + )?.pullRequest + try DirectMergeCheck.verify(current, against: expected, head: expectedHead, base: expectedBase) let arguments = [ "pr", "merge", String(expected.number), method.flag, @@ -447,12 +420,6 @@ public enum GitHub { return after.isMerged ? .merged : .acceptedButOpen } - private static func isCommitID(_ value: String) -> Bool { - (value.count == 40 || value.count == 64) && value.utf8.allSatisfy { - (48...57).contains($0) || (65...70).contains($0) || (97...102).contains($0) - } - } - /// Pushes the current HEAD to `branch` on its configured publication remote. /// /// The branch never travels as a bare argument. Git will happily create a branch called diff --git a/Sources/BloomCore/GitHub/MergeExecutionChoice.swift b/Sources/BloomCore/GitHub/MergeExecutionChoice.swift index e0a0989b0..6bcef3d5f 100644 --- a/Sources/BloomCore/GitHub/MergeExecutionChoice.swift +++ b/Sources/BloomCore/GitHub/MergeExecutionChoice.swift @@ -1,13 +1,13 @@ import Foundation -/// Whether Bloom sends a merge turn or executes the chosen gh command itself. +/// Whether Bloom sends a merge turn or executes the chosen gh or glab command itself. public enum MergeExecution: String, CaseIterable, Sendable { case direct case agent - public var label: String { + public func label(on forge: Forge) -> String { switch self { - case .direct: "Merge with GitHub CLI" + case .direct: "Merge with \(forge.name) CLI" case .agent: "Ask agent to merge" } } diff --git a/Sources/BloomCore/GitHub/MergePrompt.swift b/Sources/BloomCore/GitHub/MergePrompt.swift index e2e6efd08..b2c12d1b9 100644 --- a/Sources/BloomCore/GitHub/MergePrompt.swift +++ b/Sources/BloomCore/GitHub/MergePrompt.swift @@ -23,6 +23,7 @@ public struct MergePromptContext: Sendable, Hashable { public var branch: String public var baseBranch: String public var method: GitHub.MergeMethod + public var forge: Forge public init( workspaceName: String, @@ -30,7 +31,8 @@ public struct MergePromptContext: Sendable, Hashable { title: String, branch: String, baseBranch: String, - method: GitHub.MergeMethod + method: GitHub.MergeMethod, + forge: Forge = .gitHub ) { self.workspaceName = workspaceName self.number = number @@ -38,6 +40,7 @@ public struct MergePromptContext: Sendable, Hashable { self.branch = branch self.baseBranch = baseBranch self.method = method + self.forge = forge } public var values: [String: String] { @@ -48,7 +51,7 @@ public struct MergePromptContext: Sendable, Hashable { PromptRegistry.MergePullRequest.branch: branch.isEmpty ? Self.noBranch : branch, PromptRegistry.MergePullRequest.baseBranch: baseBranch, PromptRegistry.MergePullRequest.method: method.phrase, - PromptRegistry.MergePullRequest.methodFlag: method.flag, + PromptRegistry.MergePullRequest.methodFlag: forge == .gitLab ? GitLabInstructions.flag(for: method) : method.flag, ] } diff --git a/Sources/BloomCore/GitHub/ProjectInstructions.swift b/Sources/BloomCore/GitHub/ProjectInstructions.swift index 93710bc7f..de91766fa 100644 --- a/Sources/BloomCore/GitHub/ProjectInstructions.swift +++ b/Sources/BloomCore/GitHub/ProjectInstructions.swift @@ -167,9 +167,11 @@ public enum ProjectInstructions { /// Pure, and the only place the three are put in an order, so what an agent is about to be /// told can be asserted without a worktree, a store or GitHub. That is the same reason /// `MergePromptContext` is a value rather than a lookup. - public static func turn(_ rendered: String, for subject: Subject, adding extra: Extra) -> String { + public static func turn( + _ rendered: String, for subject: Subject, adding extra: Extra, forge: Forge = .gitHub + ) -> String { var parts = [rendered.trimmingCharacters(in: .whitespacesAndNewlines)] - if let canonical = canonical(for: subject) { parts.append(canonical) } + if let canonical = canonical(for: subject, forge: forge) { parts.append(canonical) } if let closing = sentence(for: subject, adding: extra) { parts.append(closing) } return parts.filter { !$0.isEmpty }.joined(separator: "\n\n") } @@ -188,9 +190,9 @@ public enum ProjectInstructions { /// bubble is what stopped anybody reading them. They are added before this call rather than /// inside it, by `WorkspaceModel.requestFixConflicts`, because the path they are at is a fact /// about one worktree and everything in this switch is a constant. - public static func canonical(for subject: Subject) -> String? { + public static func canonical(for subject: Subject, forge: Forge = .gitHub) -> String? { switch subject { - case .merge: MergeInstructions.canonical + case .merge: forge == .gitLab ? GitLabInstructions.merge : MergeInstructions.canonical case .fixConflicts: nil } } diff --git a/Sources/BloomCore/GitHub/PullRequestInstructions.swift b/Sources/BloomCore/GitHub/PullRequestInstructions.swift index 1dc0027ec..2554a55d1 100644 --- a/Sources/BloomCore/GitHub/PullRequestInstructions.swift +++ b/Sources/BloomCore/GitHub/PullRequestInstructions.swift @@ -50,8 +50,10 @@ public enum PullRequestInstructions { /// path in a turn is a promise to the agent that it can read what it names. `ComposerView` /// takes the same last look before it sends a prompt somebody typed, and this is the same /// look taken for the one attachment Bloom makes for itself. - public static func ensure(in worktree: String, contents: String = defaultMarkdown) async -> String? { - guard let path = await choose(in: worktree, contents: contents) else { return nil } + public static func ensure( + in worktree: String, contents: String = defaultMarkdown, scratch: String = scratchPath + ) async -> String? { + guard let path = await choose(in: worktree, contents: contents, scratch: scratch) else { return nil } // Deliberately after every branch below rather than inside them, so a case added later // cannot answer with a path nobody looked at. return InstructionFile.isFile(path, in: worktree) ? path : nil @@ -64,12 +66,13 @@ public enum PullRequestInstructions { } /// Which of the two files this worktree is going to use, writing Bloom's own if it has to. - private static func choose(in worktree: String, contents: String) async -> String? { + private static func choose(in worktree: String, contents: String, scratch scratchPath: String) async -> String? { let project = (worktree as NSString).appendingPathComponent(projectPath) if InstructionFile.isFile(projectPath, in: worktree) { - if let moved = await reclaimStrayDefault(at: project, in: worktree) { return moved } - return projectPath + guard let moved = await reclaimStrayDefault(at: project, in: worktree) else { return projectPath } + // A stray GitHub default is GitHub's scratch copy now; GitLab writes its own below. + if scratchPath == Self.scratchPath { return moved } } WorktreeScratch.shield(WorktreeScratch.generated, in: worktree) diff --git a/Sources/BloomCore/GitLab/GitLab.swift b/Sources/BloomCore/GitLab/GitLab.swift index e9f1d0419..2f729e683 100644 --- a/Sources/BloomCore/GitLab/GitLab.swift +++ b/Sources/BloomCore/GitLab/GitLab.swift @@ -11,8 +11,13 @@ public enum GitLab: ForgeClient { /// Checking out fetches objects, so it gets what `gh pr checkout` gets. private static let checkoutTimeout = Duration.seconds(120) private static let logTimeout = Duration.seconds(90) + /// What `gh pr merge` and `gh pr create` get. + private static let mergeTimeout = Duration.seconds(60) + /// `glab mr rebase` polls until GitLab is done; this caps a rebase that never is. + private static let rebaseTimeout = Duration.seconds(120) private static let candidateLimit = 20 private static let jobLimit = 100 + private static let commitLimit = 100 private static let listLimit = 30 public static func access(in directory: String) async -> GitHubAccess { @@ -113,6 +118,121 @@ public enum GitLab: ForgeClient { ) } + // MARK: Creating and merging directly + + /// The workspace's title, and the pushed commits for the description, as `gh pr create --fill` + /// does. Not glab's own `--fill`, which pushes again to a remote of its choosing. + public static func createPullRequestFromCommits( + worktree: String, base: String, title: String + ) async throws -> PullRequest { + guard Git.isValidBranchName(base) else { + throw GitHubError("refusing to open a merge request against '\(base)': not a valid branch name") + } + guard let project = await project(in: worktree) else { throw notAProject } + let context = try await Git.repositoryContext(in: worktree, baseBranch: base) + let log = try await Git.check( + ["log", "--reverse", "--format=%s%x1f%b%x1e", "\(context.baseTrackingRef ?? base)..HEAD", "--"], + in: worktree + ) + var arguments = [ + "mr", "create", "--source-branch", context.publishBranch, "--target-branch", base, + "--title", title, "--description", description(fromCommits: commits(fromLog: log.stdout)), + "--yes", "-R", project.webURL, + ] + if let fork = GitLabProject(remote: context.publishRemoteURL), fork != project { + arguments += ["--head", fork.webURL] + } + let created = try await check(arguments, cwd: worktree, timeout: mergeTimeout) + guard let number = mergeRequestNumber(in: created.stdout) else { + throw GitHubError("glab did not say which merge request it opened.") + } + return try await snapshot(number: number, in: project, worktree: worktree).pullRequest + } + + /// Merges exactly what the owner confirmed. `--sha` holds GitLab to the head that was shown, + /// and `--auto-merge=false` stops glab from merging later, once the pipeline passes. + public static func mergePullRequest( + _ expected: PullRequest, into base: String, method: GitHub.MergeMethod, worktree: String + ) async throws -> DirectMergeOutcome { + let (head, expectedBase) = try await DirectMergeCheck.confirmed(expected, base: base, worktree: worktree) + guard let project = await project(in: worktree) else { throw notAProject } + let number = expected.number + let current = try await snapshot(number: number, in: project, worktree: worktree).pullRequest + try DirectMergeCheck.verify(current, against: expected, head: head, base: expectedBase) + + var mergeHead = head + if method == .rebase { + mergeHead = try await rebase(number, confirmedHead: head, in: project, worktree: worktree) + } + var arguments = ["mr", "merge", String(number), "--yes", "--auto-merge=false", "--sha", mergeHead] + if method == .squash { arguments.append("--squash") } + arguments += ["-R", project.webURL] + let result = try await run(arguments, cwd: worktree, timeout: mergeTimeout) + let after = try? await snapshot(number: number, in: project, worktree: worktree).pullRequest + guard result.ok else { + if after?.isMerged == true { return .merged } + let refusal = result.stderr.isEmpty ? result.stdout : result.stderr + guard method != .rebase else { + throw GitHubError("GitLab rebased \(Forge.gitLab.reference(number)) but did not merge it: \(refusal)") + } + throw ShellError(command: "glab " + arguments.joined(separator: " "), status: result.status, stderr: refusal) + } + guard let after else { return .acceptedUnverified } + return after.isMerged ? .merged : .acceptedButOpen + } + + /// glab's `--rebase` rebases, then GitLab refuses the `--sha` it was given. So the rebase runs + /// first, and its new head is merged only if it replayed the confirmed commits and nothing else. + private static func rebase( + _ number: Int, confirmedHead: String, in project: GitLabProject, worktree: String + ) async throws -> String { + let confirmed = try await commits(of: number, in: project, worktree: worktree) + guard confirmed.contains(where: { $0.sha == confirmedHead }) else { + throw GitHubError("The merge request changed since it was shown. Refresh it before merging.") + } + try await check(["mr", "rebase", String(number), "-R", project.webURL], cwd: worktree, timeout: rebaseTimeout) + let rebased = try await snapshot(number: number, in: project, worktree: worktree).pullRequest + let replayed = try await commits(of: number, in: project, worktree: worktree) + guard let head = rebased.headRevision, replayed.contains(where: { $0.sha == head }), + replayed.map(\.title) == confirmed.map(\.title) else { + throw GitHubError("GitLab rebased \(Forge.gitLab.reference(number)), but its commits are not the ones confirmed. Nothing was merged.") + } + return head + } + + private static func commits( + of number: Int, in project: GitLabProject, worktree: String + ) async throws -> [GitLabDecoding.CommitPayload] { + let result = try await check( + api(project.host, "projects/\(project.apiID)/merge_requests/\(number)/commits?per_page=\(commitLimit)"), + cwd: worktree + ) + return try GitLabDecoding.commits(from: Data(result.stdout.utf8)) + } + + /// One commit's own body, or the subjects of several, oldest first: `gh pr create --fill`. + static func description(fromCommits commits: [(subject: String, body: String)]) -> String { + if commits.count == 1 { return commits[0].body.trimmingCharacters(in: .whitespacesAndNewlines) } + return commits.map { "- \($0.subject)" }.joined(separator: "\n") + } + + /// `git log --format=%s%x1f%b%x1e` read back into subjects and bodies. + static func commits(fromLog output: String) -> [(subject: String, body: String)] { + output.split(separator: "\u{1e}").compactMap { record in + let fields = record.split(separator: "\u{1f}", maxSplits: 1, omittingEmptySubsequences: false) + let subject = fields[0].trimmingCharacters(in: .whitespacesAndNewlines) + guard !subject.isEmpty else { return nil } + return (subject, fields.count > 1 ? String(fields[1]) : "") + } + } + + /// The number at the end of the merge request URL glab prints once it has opened one. + static func mergeRequestNumber(in output: String) -> Int? { + guard let range = output.range(of: #"/-/merge_requests/(\d+)"#, options: [.regularExpression, .backwards]) + else { return nil } + return Int(output[range].split(separator: "/").last ?? "") + } + // MARK: Reading one merge request struct Snapshot { diff --git a/Sources/BloomCore/GitLab/GitLabDecoding.swift b/Sources/BloomCore/GitLab/GitLabDecoding.swift index 2762505d1..da39b9109 100644 --- a/Sources/BloomCore/GitLab/GitLabDecoding.swift +++ b/Sources/BloomCore/GitLab/GitLabDecoding.swift @@ -44,6 +44,20 @@ enum GitLabDecoding { let finishedAt: String? } + struct CommitPayload: Decodable, Equatable { + let sha: String? + let title: String? + + enum CodingKeys: String, CodingKey { + case sha = "id" + case title + } + } + + static func commits(from data: Data) throws -> [CommitPayload] { + try decode([CommitPayload].self, from: data) + } + static func mergeRequest(from data: Data) throws -> MergeRequestPayload { try decode(MergeRequestPayload.self, from: data) } diff --git a/Sources/BloomCore/GitLab/GitLabInstructions.swift b/Sources/BloomCore/GitLab/GitLabInstructions.swift new file mode 100644 index 000000000..0dd060e2d --- /dev/null +++ b/Sources/BloomCore/GitLab/GitLabInstructions.swift @@ -0,0 +1,80 @@ +import Foundation + +/// What Bloom tells an agent on a GitLab project: GitHub's texts in GitLab's words, so the two +/// cannot drift, except where the commands differ. GitHub's own defaults are never edited. +public enum GitLabInstructions { + /// Not `pr-instructions.md`, which a worktree may already hold with GitHub's steps in it. + public static let mergeRequestScratchPath = "\(WorktreeScratch.generated)/mr-instructions.md" + + public static var mergeRequestMarkdown: String { + translate(PullRequestInstructions.defaultMarkdown).replacingOccurrences( + of: "`gh pr create --base --title --body\n <description>`", + with: "`glab mr create --target-branch <target branch> --title <title>\n --description <description> --yes`" + ) + } + + public static var conflictMarkdown: String { translate(ConflictInstructions.defaultMarkdown) } + + public static let merge = """ + This message names the merge request, the branch it is on, and the merge method to use. + Nothing below chooses any of those, and nothing below may change them. + + - If this project has a skill or an instruction file about merging, follow that first. It + outranks everything here. + - Read the merge request's head commit, its `sha`, from `glab mr view <number> -F json`. + - Merge with `glab mr merge <number> --yes --auto-merge=false --sha <head commit>`, plus + `--squash` if the message names it, run from this worktree. + - For a rebase, run `glab mr rebase <number>` first and wait for it, then read the new head + commit and merge with no method flag. Not glab's `--rebase`: it rebases, then GitLab + refuses the `--sha` it was given, because the head has moved. + - Pass no other flags. `--auto-merge=false` is required: without it glab sets "merge when + the pipeline succeeds", which merges later, when nobody is watching. Not + `--remove-source-branch`: the branch is deleted below, once the merge has happened. + - **If GitLab refuses the merge, stop.** Say what it said, in its own words. Do not retry it, + do not force it, and do not change an approval rule, a protected branch or any other + project setting to get round it. A refusal is an answer, and the person who pressed the + button is reading this. + - Only once the merge has actually succeeded, delete the branch on the server with + `git push --delete -- origin refs/heads/<branch>`, using the branch the message names. If + git answers that the remote ref does not exist, it was deleted on merge and there is + nothing left to do. + - Change nothing on this machine. Do not delete the local branch, do not remove or move the + worktree, and do not check out anything else. + - Do not commit and do not push. If the worktree is holding work GitLab has not got, say in + one line that it was left behind. + - Finish by saying what happened: that it merged, and whether the branch on the server is gone. + + If a step fails, stop and say what went wrong instead of working around it. + """ + + /// The built-in template for a GitLab project, or nil to use GitHub's as it stands. + public static func defaultTemplate(for id: PromptID) -> String? { + switch id { + case .mergePullRequest: + "Merge merge request !{{number}} into {{base_branch}} as a {{method}} ({{method_flag}}). " + + "It is on the branch {{branch}}." + case .createPullRequest, .pushLocalWork, .fixConflicts, .continueAfterMerge: + translate(PromptRegistry.definition(for: id).defaultTemplate) + case .carryOnArchived, .review, .nameWorkspace: + nil + } + } + + /// The merge method as the GitLab template names it. Merge is no flag at all: the project's + /// own method, whether that is a merge commit, semi-linear or fast forward. + public static func flag(for method: GitHub.MergeMethod) -> String { + switch method { + case .merge: "no method flag, the project's own merge method" + case .squash: "`--squash`" + case .rebase: "`glab mr rebase` first, then no method flag" + } + } + + static func translate(_ text: String) -> String { + [ + ("Pull request #", "Merge request !"), ("pull request #", "merge request !"), + ("Pull request", "Merge request"), ("pull request", "merge request"), + ("Pull Request", "Merge Request"), ("GitHub", "GitLab"), + ].reduce(text) { $0.replacingOccurrences(of: $1.0, with: $1.1) } + } +} diff --git a/Sources/BloomCore/Transcript/PromptOverrides.swift b/Sources/BloomCore/Transcript/PromptOverrides.swift index 7aca2f28c..16fd097d7 100644 --- a/Sources/BloomCore/Transcript/PromptOverrides.swift +++ b/Sources/BloomCore/Transcript/PromptOverrides.swift @@ -18,6 +18,12 @@ public struct PromptOverrides: @unchecked Sendable { keyPrefix + id.rawValue } + /// GitLab's overrides live apart, so a GitHub template saying `gh pr merge` never reaches a + /// GitLab project. + public static func key(for id: PromptID, forge: Forge) -> String { + forge == .gitHub ? key(for: id) : keyPrefix + "gitLab." + id.rawValue + } + private let defaults: UserDefaults public init(defaults: UserDefaults = .standard) { @@ -54,4 +60,13 @@ public struct PromptOverrides: @unchecked Sendable { } return override } + + public func template(for id: PromptID, forge: Forge) -> String { + guard forge == .gitLab else { return template(for: id) } + let override = defaults.string(forKey: Self.key(for: id, forge: forge)) ?? "" + guard !override.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { + return GitLabInstructions.defaultTemplate(for: id) ?? PromptRegistry.definition(for: id).defaultTemplate + } + return override + } } diff --git a/Sources/BloomCore/Transcript/SentTurn.swift b/Sources/BloomCore/Transcript/SentTurn.swift index 7743c4cb9..e2347dc13 100644 --- a/Sources/BloomCore/Transcript/SentTurn.swift +++ b/Sources/BloomCore/Transcript/SentTurn.swift @@ -72,6 +72,7 @@ public enum SentTurn { /// written to, because the reader pressed the button and never sees the file. public static let mergeTitle = "Merge instructions" public static let pullRequestTitle = "Pull request instructions" + public static let mergeRequestTitle = "Merge request instructions" public static let conflictTitle = "Conflict instructions" public static let projectTitle = "Project instructions" @@ -91,6 +92,8 @@ public enum SentTurn { switch path { case PullRequestInstructions.projectPath, PullRequestInstructions.scratchPath: return pullRequestTitle + case GitLabInstructions.mergeRequestScratchPath: + return mergeRequestTitle case ConflictInstructions.scratchPath: return conflictTitle default: @@ -153,6 +156,11 @@ public enum SentTurn { found += PullRequestInstructions.retiredDefaults.map { InjectedInstruction(title: pullRequestTitle, body: $0) } + found += [ + InjectedInstruction(title: mergeTitle, body: GitLabInstructions.merge), + InjectedInstruction(title: mergeRequestTitle, body: GitLabInstructions.mergeRequestMarkdown), + InjectedInstruction(title: conflictTitle, body: GitLabInstructions.conflictMarkdown), + ] return found }() diff --git a/Sources/BloomCore/Workspace/WorkspaceCheckout.swift b/Sources/BloomCore/Workspace/WorkspaceCheckout.swift index 852b7df2b..30f228cca 100644 --- a/Sources/BloomCore/Workspace/WorkspaceCheckout.swift +++ b/Sources/BloomCore/Workspace/WorkspaceCheckout.swift @@ -303,7 +303,7 @@ public enum WorkspaceCheckoutPlan { /// The URL carries the repository as well, which is not thrown away: pasting another /// repository's pull request into this project is a mistake worth naming rather than a number /// that quietly resolves to somebody else's work. - public static func parseReference(_ text: String) -> PullRequestReference? { + public static func parseReference(_ text: String, forge: Forge = .gitHub) -> PullRequestReference? { let trimmed = text.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty else { return nil } @@ -319,11 +319,26 @@ public enum WorkspaceCheckoutPlan { ) } + if forge == .gitLab, let mergeRequest = gitLabReference(trimmed) { return mergeRequest } let bare = trimmed.hasPrefix("#") ? String(trimmed.dropFirst()) : trimmed guard let number = positiveNumber(bare) else { return nil } return PullRequestReference(number: number, repository: nil) } + /// `!12`, or a merge request URL: `https://<host>/<group path>/-/merge_requests/12`. + private static func gitLabReference(_ text: String) -> PullRequestReference? { + if text.hasPrefix("!") { + return positiveNumber(String(text.dropFirst())).map { PullRequestReference(number: $0, repository: nil) } + } + guard let url = URL(string: text), url.host != nil else { return nil } + let parts = url.path.split(separator: "/").map(String.init) + guard let separator = parts.firstIndex(of: "-"), separator >= 2, + parts.indices.contains(separator + 2), parts[separator + 1] == "merge_requests", + let number = positiveNumber(parts[separator + 2]) + else { return nil } + return PullRequestReference(number: number, repository: parts[..<separator].joined(separator: "/")) + } + private static func positiveNumber(_ text: String) -> Int? { guard !text.isEmpty, text.allSatisfy(\.isNumber), let value = Int(text), value > 0 else { return nil @@ -445,17 +460,17 @@ public enum WorkspaceCheckoutResolver { /// to have as number 42. Being told the numbers belong to different repositories is a fixable /// mistake; silently reviewing the wrong pull request is not. public static func problem( - with text: String, in repository: String? + with text: String, in repository: String?, forge: Forge = .gitHub ) -> String? { guard !text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { return "Type a pull request number or paste its URL." } - guard let reference = WorkspaceCheckoutPlan.parseReference(text) else { - return "'\(text.trimmingCharacters(in: .whitespacesAndNewlines))' is not a pull request number or URL." + guard let reference = WorkspaceCheckoutPlan.parseReference(text, forge: forge) else { + return "'\(text.trimmingCharacters(in: .whitespacesAndNewlines))' is not a \(forge.request) number or URL." } if let repository, let named = reference.repository, named.lowercased() != repository.lowercased() { - return "That pull request belongs to \(named), and this project is \(repository)." + return "That \(forge.request) belongs to \(named), and this project is \(repository)." } return nil } @@ -463,10 +478,11 @@ public enum WorkspaceCheckoutResolver { /// The whole of it, including the gh call. Lives here rather than in the sheet so the sheet is /// left drawing a text field and reading an answer. public static func resolve(_ text: String, repoPath: String) async -> WorkspaceCheckoutResolution { + let kind = await ForgeResolver.forge(for: repoPath) let forge = await ForgeResolver.client(for: repoPath) let slug = await forge.repositorySlug(repoPath: repoPath) - if let problem = problem(with: text, in: slug) { return .failure(problem) } - guard let reference = WorkspaceCheckoutPlan.parseReference(text) else { + if let problem = problem(with: text, in: slug, forge: kind) { return .failure(problem) } + guard let reference = WorkspaceCheckoutPlan.parseReference(text, forge: kind) else { return .failure("That is not a pull request number or URL.") } do { diff --git a/Sources/BloomCore/Workspace/WorkspaceSource.swift b/Sources/BloomCore/Workspace/WorkspaceSource.swift index e0cd7d832..ad058a0f6 100644 --- a/Sources/BloomCore/Workspace/WorkspaceSource.swift +++ b/Sources/BloomCore/Workspace/WorkspaceSource.swift @@ -193,17 +193,21 @@ public struct WorkspaceSourceOffering: Sendable, Hashable { public let baseBranches: [String] /// The project's default branch, which a new branch is ordinarily cut from. See `carryOn`. public let defaultBranch: String + /// Which references a typed row accepts: `!12` and merge request URLs only on GitLab. + public let forge: Forge public init( pullRequests: [PullRequestListing] = [], branches: [ExistingBranch] = [], baseBranches: [String] = [], - defaultBranch: String = "" + defaultBranch: String = "", + forge: Forge = .gitHub ) { self.pullRequests = pullRequests self.branches = branches self.baseBranches = baseBranches self.defaultBranch = defaultBranch + self.forge = forge } /// The "Open, and carry on" section: what somebody else has already written. @@ -278,7 +282,7 @@ public struct WorkspaceSourceOffering: Sendable, Hashable { /// `WorkspaceCheckoutPlan.parseReference`, which already knows every shape a pull request /// arrives in. private func typedRows(for query: String) -> [WorkspaceSource] { - guard let reference = WorkspaceCheckoutPlan.parseReference(query) else { return [] } + guard let reference = WorkspaceCheckoutPlan.parseReference(query, forge: forge) else { return [] } guard !pullRequests.contains(where: { $0.number == reference.number }) else { return [] } return [.pullRequest(.typed(reference, text: query))] } diff --git a/Sources/BloomCore/Workspace/WorkspaceStartContext.swift b/Sources/BloomCore/Workspace/WorkspaceStartContext.swift index 4386f3003..f12346172 100644 --- a/Sources/BloomCore/Workspace/WorkspaceStartContext.swift +++ b/Sources/BloomCore/Workspace/WorkspaceStartContext.swift @@ -102,19 +102,22 @@ public struct WorkspaceCheckoutOptions: Sendable { /// half of the picker knew the branch was taken and the pull request half did not, so the one /// that reached `git worktree add` was the one that could not have worked. public let holders: [String: BranchHolder] + public let forge: Forge public init( pullRequests: [PullRequestListing] = [], branches: [ExistingBranch] = [], access: GitHubAccess = .ready, failure: String? = nil, - holders: [String: BranchHolder] = [:] + holders: [String: BranchHolder] = [:], + forge: Forge = .gitHub ) { self.pullRequests = pullRequests self.branches = branches self.access = access self.failure = failure self.holders = holders + self.forge = forge } /// Both branch listings are read here rather than handed in. @@ -158,6 +161,7 @@ public struct WorkspaceCheckoutOptions: Sendable { workspaceNames: BranchHolder.names(of: workspaces, in: repoID) ) + let forgeKind = await ForgeResolver.forge(for: repoPath) func options( pullRequests: [PullRequestListing] = [], access: GitHubAccess = .ready, @@ -175,7 +179,8 @@ public struct WorkspaceCheckoutOptions: Sendable { ), access: access, failure: failure, - holders: branchesInUse + holders: branchesInUse, + forge: forgeKind ) } diff --git a/Tests/BloomCoreTests/GitHubStoredValueTests.swift b/Tests/BloomCoreTests/GitHubStoredValueTests.swift index 52d81cc69..50ea38a67 100644 --- a/Tests/BloomCoreTests/GitHubStoredValueTests.swift +++ b/Tests/BloomCoreTests/GitHubStoredValueTests.swift @@ -33,7 +33,7 @@ struct GitHubStoredValueTests { ]) #expect(GitHub.MergeMethod.allCases.map(\.rawValue) == ["merge", "squash", "rebase"]) #expect(MergeExecution.allCases.map(\.rawValue) == ["direct", "agent"]) - #expect(MergeExecution.allCases.map(\.label) == ["Merge with GitHub CLI", "Ask agent to merge"]) + #expect(MergeExecution.allCases.map { $0.label(on: .gitHub) } == ["Merge with GitHub CLI", "Ask agent to merge"]) let checks: [PullRequest.Checks] = [.none, .pending, .passing, .failing, .unavailable] #expect(checks.map(\.rawValue) == ["none", "pending", "passing", "failing", "unavailable"]) } diff --git a/Tests/BloomCoreTests/GitLabDirectTests.swift b/Tests/BloomCoreTests/GitLabDirectTests.swift new file mode 100644 index 000000000..646bc2793 --- /dev/null +++ b/Tests/BloomCoreTests/GitLabDirectTests.swift @@ -0,0 +1,197 @@ +import Foundation +import Testing +@testable import BloomCore + +@Suite("Direct GitLab create and merge", .tags(.git), .scratchDirectory) +struct GitLabDirectTests { + private let project = "https://gitlab.com/group/app" + + @Test("merges the confirmed head with no auto-merge, the method's flag and the project named") + func mergesTheConfirmedHead() async throws { + let repo = try await featureRepo() + defer { repo.cleanUp() } + let head = try await Git.headSHA(of: repo.path) + + for method in [GitHub.MergeMethod.squash, .merge] { + let glab = MergeGlab(head: head) + let outcome = try await GitLab.$commandOverride.withValue({ arguments, _ in + await glab.execute(arguments) + }) { + try await GitLab.mergePullRequest(expected(head: head), into: "main", method: method, worktree: repo.path) + } + let flags: [String] = method == .squash ? ["--squash"] : [] + let merge = ["mr", "merge", "42", "--yes", "--auto-merge=false", "--sha", head] + flags + ["-R", project] + #expect(outcome == .merged) + #expect(await glab.merges == [merge]) + } + } + + @Test("a rebase runs first, and the rebased head is what gets merged") + func rebasesThenMerges() async throws { + let repo = try await featureRepo() + defer { repo.cleanUp() } + let head = try await Git.headSHA(of: repo.path) + let rebased = String(repeating: "b", count: 40) + let glab = MergeGlab(head: head, rebasedHead: rebased) + + let outcome = try await GitLab.$commandOverride.withValue({ arguments, _ in + await glab.execute(arguments) + }) { + try await GitLab.mergePullRequest(expected(head: head), into: "main", method: .rebase, worktree: repo.path) + } + + #expect(outcome == .merged) + #expect(await glab.rebases == 1) + #expect(await glab.merges == [ + ["mr", "merge", "42", "--yes", "--auto-merge=false", "--sha", rebased, "-R", project], + ]) + } + + @Test("a rebase that brought in a commit nobody confirmed is not merged") + func refusesAnUnconfirmedCommit() async throws { + let repo = try await featureRepo() + defer { repo.cleanUp() } + let head = try await Git.headSHA(of: repo.path) + let glab = MergeGlab(head: head, rebasedHead: String(repeating: "b", count: 40), extraAfterRebase: "Sneaked in") + + do { + _ = try await GitLab.$commandOverride.withValue({ arguments, _ in + await glab.execute(arguments) + }) { + try await GitLab.mergePullRequest(expected(head: head), into: "main", method: .rebase, worktree: repo.path) + } + Issue.record("A rebase with an unconfirmed commit was merged") + } catch let error as GitHubError { + #expect(error.message.contains("not the ones confirmed")) + } + #expect(await glab.merges.isEmpty) + } + + @Test("a newly pushed head refuses the direct merge before glab merges") + func refusesChangedHead() async throws { + let repo = try await featureRepo() + defer { repo.cleanUp() } + let head = try await Git.headSHA(of: repo.path) + let glab = MergeGlab(head: String(repeating: "a", count: 40)) + + do { + _ = try await GitLab.$commandOverride.withValue({ arguments, _ in + await glab.execute(arguments) + }) { + try await GitLab.mergePullRequest(expected(head: head), into: "main", method: .merge, worktree: repo.path) + } + Issue.record("A changed merge request head was merged") + } catch let error as GitHubError { + #expect(error.message == "The merge request changed since it was shown. Refresh it before merging.") + } + #expect(await glab.merges.isEmpty) + } + + @Test("opens the merge request from the pushed branch, described by its commits as gh --fill does") + func createsFromCommits() async throws { + let repo = try await featureRepo() + defer { repo.cleanUp() } + try repo.write("b.txt", "b\n") + try await Shell.check("git", ["add", "b.txt"], cwd: repo.path) + try await Shell.check("git", ["commit", "-q", "-m", "Add b"], cwd: repo.path) + let head = try await Git.headSHA(of: repo.path) + let glab = MergeGlab(head: head) + + let created = try await GitLab.$commandOverride.withValue({ arguments, _ in + await glab.execute(arguments) + }) { + try await GitLab.createPullRequestFromCommits(worktree: repo.path, base: "main", title: "Feature") + } + + #expect(created.number == 42) + #expect(created.forge == .gitLab) + #expect(await glab.creates == [[ + "mr", "create", "--source-branch", "feature", "--target-branch", "main", + "--title", "Feature", "--description", "- Add a\n- Add b", "--yes", "-R", project, + ]]) + } + + @Test("a single commit's body is the description, and glab's printed URL names the request") + func descriptionAndNumber() { + #expect(GitLab.description(fromCommits: [(subject: "Add a", body: "Why a\n")]) == "Why a") + #expect(GitLab.description(fromCommits: []) == "") + #expect(GitLab.commits(fromLog: "Add a\u{1f}Why\n\u{1e}\nAdd b\u{1f}\u{1e}\n").map { $0.subject } == ["Add a", "Add b"]) + #expect(GitLab.mergeRequestNumber(in: "Creating…\nhttps://gitlab.com/g/app/-/merge_requests/42\n") == 42) + #expect(GitLab.mergeRequestNumber(in: "nothing here") == nil) + } + + @Test("GitHub's words for the direct routes are the ones it had") + func gitHubWords() { + #expect(Forge.gitHub.command("merge") == "gh pr merge") + #expect(Forge.gitHub.titledRequest == "Pull request") + #expect(Forge.gitHub.shortRequest == "PR") + #expect(Forge.gitLab.command("merge") == "glab mr merge") + #expect(MergeExecution.direct.label(on: .gitLab) == "Merge with GitLab CLI") + } + + /// On `feature`, one commit ahead of `main`, with `origin/main` where `main` is. + private func featureRepo() async throws -> TempRepo { + let repo = try await TempRepo() + try await Shell.check("git", ["remote", "add", "origin", "git@gitlab.com:group/app.git"], cwd: repo.path) + try await Shell.check("git", ["update-ref", "refs/remotes/origin/main", "HEAD"], cwd: repo.path) + try await Shell.check("git", ["checkout", "-q", "-b", "feature"], cwd: repo.path) + try repo.write("a.txt", "a\n") + try await Shell.check("git", ["add", "a.txt"], cwd: repo.path) + try await Shell.check("git", ["commit", "-q", "-m", "Add a", "-m", "Why a"], cwd: repo.path) + return repo + } + + private func expected(head: String) -> PullRequest { + PullRequest( + number: 42, title: "Feature", url: "\(project)/-/merge_requests/42", state: "OPEN", + branch: "feature", baseBranch: "main", headRevision: head, forge: .gitLab + ) + } +} + +/// glab for one merge request, !42 on `feature`, answering the API reads and the mr commands. +private actor MergeGlab { + private var head: String + private let rebasedHead: String? + private let extraAfterRebase: String? + private var merged = false + private(set) var merges: [[String]] = [] + private(set) var creates: [[String]] = [] + private(set) var rebases = 0 + + init(head: String, rebasedHead: String? = nil, extraAfterRebase: String? = nil) { + self.head = head + self.rebasedHead = rebasedHead + self.extraAfterRebase = extraAfterRebase + } + + func execute(_ arguments: [String]) -> ShellResult { + if arguments.starts(with: ["mr", "merge"]) { + merges.append(arguments) + merged = true + return ShellResult(status: 0, stdout: "", stderr: "") + } + if arguments.starts(with: ["mr", "create"]) { + creates.append(arguments) + return ShellResult(status: 0, stdout: "https://gitlab.com/group/app/-/merge_requests/42\n", stderr: "") + } + if arguments.starts(with: ["mr", "rebase"]) { + rebases += 1 + head = rebasedHead ?? head + return ShellResult(status: 0, stdout: "", stderr: "") + } + if arguments.last?.hasSuffix("/commits?per_page=100") == true { + let extra = rebases > 0 ? extraAfterRebase.map { [(String(repeating: "c", count: 40), $0)] } ?? [] : [] + let commits = extra + [(head, "Add a")] + let listed = commits.map { #"{"id":"\#($0.0)","title":"\#($0.1)"}"# }.joined(separator: ",") + return ShellResult(status: 0, stdout: "[\(listed)]", stderr: "") + } + let state = merged ? "merged" : "opened" + return ShellResult(status: 0, stdout: """ + {"iid":42,"title":"Feature","web_url":"https://gitlab.com/group/app/-/merge_requests/42",\ + "state":"\(state)","draft":false,"has_conflicts":false,"detailed_merge_status":"mergeable",\ + "source_branch":"feature","target_branch":"main","sha":"\(head)",\ + "source_project_id":1,"target_project_id":1} + """, stderr: "") + } +} diff --git a/Tests/BloomCoreTests/GitLabInstructionsTests.swift b/Tests/BloomCoreTests/GitLabInstructionsTests.swift new file mode 100644 index 000000000..7653dca20 --- /dev/null +++ b/Tests/BloomCoreTests/GitLabInstructionsTests.swift @@ -0,0 +1,109 @@ +import Foundation +import Testing +@testable import BloomCore + +@Suite("GitLab instructions", .scratchDirectory) +struct GitLabInstructionsTests { + @Test("every GitLab text, as it stands") + func texts() { + expectCharacterised(GitLabInstructions.mergeRequestMarkdown, "gitlab-mr-instructions.md") + expectCharacterised(GitLabInstructions.merge, "gitlab-merge-instructions.md") + expectCharacterised(GitLabInstructions.conflictMarkdown, "gitlab-conflict-instructions.md") + for definition in PromptRegistry.all { + guard let template = GitLabInstructions.defaultTemplate(for: definition.id) else { continue } + expectCharacterised(template, "gitlab-prompt-\(definition.id.rawValue).txt") + } + } + + @Test("nothing sent to a GitLab project's agent names gh or GitHub's words") + func noGitHubWords() { + let texts = [GitLabInstructions.mergeRequestMarkdown, GitLabInstructions.merge, GitLabInstructions.conflictMarkdown] + + PromptRegistry.all.compactMap { GitLabInstructions.defaultTemplate(for: $0.id) } + for text in texts { + #expect(!text.contains("`gh "), "\(text.prefix(60))") + #expect(!text.contains("GitHub"), "\(text.prefix(60))") + #expect(!text.localizedCaseInsensitiveContains("pull request"), "\(text.prefix(60))") + } + #expect(GitLabInstructions.merge.contains("--auto-merge=false")) + #expect(GitLabInstructions.mergeRequestMarkdown.contains("glab mr create --target-branch")) + } + + @Test("a GitHub override never reaches a GitLab project, and GitHub's lookup is unchanged") + func overrides() throws { + let defaults = try #require(UserDefaults(suiteName: "gitlab-prompts-\(UUID().uuidString)")) + let overrides = PromptOverrides(defaults: defaults) + overrides.set("Run gh pr merge now.", for: .mergePullRequest) + + #expect(overrides.template(for: .mergePullRequest, forge: .gitHub) == "Run gh pr merge now.") + #expect(overrides.template(for: .mergePullRequest, forge: .gitLab) + == GitLabInstructions.defaultTemplate(for: .mergePullRequest)) + + defaults.set("Merge it with glab.", forKey: PromptOverrides.key(for: .mergePullRequest, forge: .gitLab)) + #expect(PromptOverrides.key(for: .mergePullRequest, forge: .gitLab) == "prompts.gitLab.mergePullRequest") + #expect(overrides.template(for: .mergePullRequest, forge: .gitLab) == "Merge it with glab.") + #expect(overrides.template(for: .review, forge: .gitLab) == overrides.template(for: .review)) + } + + @Test("the merge turn names GitLab's flags and carries GitLab's steps", arguments: [ + (method: GitHub.MergeMethod.squash, flag: "`--squash`"), + (method: .rebase, flag: "`glab mr rebase` first, then no method flag"), + (method: .merge, flag: "no method flag, the project's own merge method"), + ]) + func mergeTurn(method: GitHub.MergeMethod, flag: String) { + let context = MergePromptContext( + workspaceName: "w", number: 12, title: "t", branch: "feature", baseBranch: "main", + method: method, forge: .gitLab + ) + let template = GitLabInstructions.defaultTemplate(for: .mergePullRequest) ?? "" + let rendered = context.render(template: template).text + #expect(rendered.hasPrefix("Merge merge request !12 into main as a ")) + #expect(rendered.contains("(\(flag))")) + let turn = ProjectInstructions.turn(rendered, for: .merge, adding: .nothing, forge: .gitLab) + #expect(turn.contains(GitLabInstructions.merge)) + #expect(!turn.contains(MergeInstructions.canonical)) + } + + @Test("a GitHub merge turn is what it was") + func gitHubMergeTurn() { + let context = MergePromptContext( + workspaceName: "w", number: 12, title: "t", branch: "feature", baseBranch: "main", method: .squash + ) + #expect(context.values[PromptRegistry.MergePullRequest.methodFlag] == "--squash") + let turn = ProjectInstructions.turn("x", for: .merge, adding: .nothing) + #expect(turn == "x\n\n" + MergeInstructions.canonical) + } + + private static let referenceCases: [(text: String, number: Int, repository: String?)] = [ + ("!42", 42, nil), + ("https://gitlab.com/group/sub/app/-/merge_requests/42", 42, "group/sub/app"), + ("https://git.example.com/team/app/-/merge_requests/7/diffs", 7, "team/app"), + ("https://github.com/acme/app/pull/9", 9, "acme/app"), + ("#5", 5, nil), + ] + + @Test("merge request references", arguments: referenceCases) + func references(text: String, number: Int, repository: String?) throws { + let reference = try #require(WorkspaceCheckoutPlan.parseReference(text, forge: .gitLab)) + #expect(reference.number == number) + #expect(reference.repository == repository) + } + + @Test("a GitHub project reads !12 and merge request URLs as it always did: not at all") + func gitHubIgnoresMergeRequests() { + #expect(WorkspaceCheckoutPlan.parseReference("!12") == nil) + #expect(WorkspaceCheckoutPlan.parseReference("https://gitlab.com/group/app/-/merge_requests/12") == nil) + } + + @Test("a GitLab worktree gets its own instructions file, beside GitHub's") + func ownScratchFile() async throws { + let worktree = TestScratch.unique("gitlab-scratch") + try FileManager.default.createDirectory(atPath: worktree, withIntermediateDirectories: true) + let path = await PullRequestInstructions.ensure( + in: worktree, contents: GitLabInstructions.mergeRequestMarkdown, + scratch: GitLabInstructions.mergeRequestScratchPath + ) + #expect(path == GitLabInstructions.mergeRequestScratchPath) + let written = try String(contentsOfFile: "\(worktree)/\(GitLabInstructions.mergeRequestScratchPath)", encoding: .utf8) + #expect(written == GitLabInstructions.mergeRequestMarkdown) + } +} diff --git a/Tests/fixtures/characterisation/gitlab-conflict-instructions.md b/Tests/fixtures/characterisation/gitlab-conflict-instructions.md new file mode 100644 index 000000000..eeef0fd27 --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-conflict-instructions.md @@ -0,0 +1,39 @@ +# Resolving merge conflicts + +Bloom attaches this file when someone presses Fix merge conflicts. This copy is Bloom's own, is +rewritten on every press and is invisible to git, so an edit made here does not last. To change +what this project does about conflicts, write it in `.bloom/conflict-instructions.md` and commit +it: Bloom attaches that as well, and where the two disagree yours wins. The message these steps +arrived with is a prompt you can reword in Bloom's settings. + +That message names the merge request, the branch this worktree is on, and the branch that +conflicts with it. Call the second one this branch and the third the base branch below. + +- If this project has a skill or an instruction file about resolving conflicts, follow that + first. It outranks everything here. +- Fetch the base branch first, so you are working against what is on the server rather than a + stale copy of it. +- Bring the base branch into this branch the way this project brings it in: merge it unless + the project's own conventions say to rebase onto it. + It goes into this branch and never the other way round. +- Work through every conflicted file. Keep what this branch changed and what the base branch + changed, and where the two genuinely disagree, read enough of the code around them to work out + which is right instead of taking a side. +- Follow this project's conventions, and run whatever it uses to check itself before you call + anything resolved. +- Commit the resolution, with a message worded the way this project words one. +- Then push it. A conflict resolved only in this worktree is still a conflict to everybody else, + and the merge request goes on refusing to merge until the branch on the server carries the + resolution. If bringing the base branch in rewrote this branch's commits, which a rebase does, + the push needs `--force-with-lease`, and it may only ever go to this branch, never to the base + branch and never to any other branch. +- **Do not push if you are not sure.** Genuine uncertainty about what a resolution should be, a + check that fails for a reason neither branch explains, or anything you had to guess at: leave + the commit here, say what you are unsure about, and let a person look. A resolution nobody + believes in is worse on the server than in a worktree. +- Do not merge the merge request whatever happens. Whether the work is good is the reader's + decision, and they are the one who pressed the button. +- Finish by saying which files conflicted, what you decided in each of them, whether you pushed, + and anything you are not sure about. + +If a step fails, stop and say what went wrong instead of working around it. \ No newline at end of file diff --git a/Tests/fixtures/characterisation/gitlab-merge-instructions.md b/Tests/fixtures/characterisation/gitlab-merge-instructions.md new file mode 100644 index 000000000..889404966 --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-merge-instructions.md @@ -0,0 +1,29 @@ +This message names the merge request, the branch it is on, and the merge method to use. +Nothing below chooses any of those, and nothing below may change them. + +- If this project has a skill or an instruction file about merging, follow that first. It + outranks everything here. +- Read the merge request's head commit, its `sha`, from `glab mr view <number> -F json`. +- Merge with `glab mr merge <number> --yes --auto-merge=false --sha <head commit>`, plus + `--squash` if the message names it, run from this worktree. +- For a rebase, run `glab mr rebase <number>` first and wait for it, then read the new head + commit and merge with no method flag. Not glab's `--rebase`: it rebases, then GitLab + refuses the `--sha` it was given, because the head has moved. +- Pass no other flags. `--auto-merge=false` is required: without it glab sets "merge when + the pipeline succeeds", which merges later, when nobody is watching. Not + `--remove-source-branch`: the branch is deleted below, once the merge has happened. +- **If GitLab refuses the merge, stop.** Say what it said, in its own words. Do not retry it, + do not force it, and do not change an approval rule, a protected branch or any other + project setting to get round it. A refusal is an answer, and the person who pressed the + button is reading this. +- Only once the merge has actually succeeded, delete the branch on the server with + `git push --delete -- origin refs/heads/<branch>`, using the branch the message names. If + git answers that the remote ref does not exist, it was deleted on merge and there is + nothing left to do. +- Change nothing on this machine. Do not delete the local branch, do not remove or move the + worktree, and do not check out anything else. +- Do not commit and do not push. If the worktree is holding work GitLab has not got, say in + one line that it was left behind. +- Finish by saying what happened: that it merged, and whether the branch on the server is gone. + +If a step fails, stop and say what went wrong instead of working around it. \ No newline at end of file diff --git a/Tests/fixtures/characterisation/gitlab-mr-instructions.md b/Tests/fixtures/characterisation/gitlab-mr-instructions.md new file mode 100644 index 000000000..3a6a7e664 --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-mr-instructions.md @@ -0,0 +1,24 @@ +# Opening a merge request + +Bloom attaches this file when someone presses Create merge request. This copy is Bloom's own and +is invisible to git. To make it this project's, move it to `.bloom/pr-instructions.md`, edit it +to say how this project opens merge requests, and commit it. Bloom then uses that copy instead +and never writes over it, so everybody working here gets the change. + +The message this file came with names the branch to target. Call it the target branch below. + +- If this project has a skill or an instruction file about opening merge requests, follow that + first. It outranks everything here. +- Run `git status`. If anything is uncommitted, review it and commit it, following whatever + this project says about commit messages. +- Push the branch with `git push -u origin HEAD`. If it already tracks a different upstream, + push to that one instead. +- Read the whole branch with `git diff <target branch>...` before writing anything. The + description has to cover every change on the branch, not only what was done in this session. +- Open the merge request with `glab mr create --target-branch <target branch> --title <title> + --description <description> --yes`. If the repository has a merge request template, fill that in instead of + writing your own structure. Keep the title under 80 characters and the description under + five sentences. +- Say what the merge request URL is once it exists. + +If a step fails, stop and say what went wrong instead of working around it. \ No newline at end of file diff --git a/Tests/fixtures/characterisation/gitlab-prompt-continueAfterMerge.txt b/Tests/fixtures/characterisation/gitlab-prompt-continueAfterMerge.txt new file mode 100644 index 000000000..dfbf37632 --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-prompt-continueAfterMerge.txt @@ -0,0 +1,5 @@ +Merge request !{{pull_request}} is merged, so {{previous_branch}} is finished with. + +This worktree is now on a new branch, {{branch}}, cut from an up to date {{base_branch}}, so everything that just landed is already underneath you. Nothing else moved: same directory, same session, and anything that was uncommitted is still here. + +Do not redo what the merge request landed, and do not start anything new yet. Say in one line that you are ready, then wait for what I ask next. \ No newline at end of file diff --git a/Tests/fixtures/characterisation/gitlab-prompt-createPullRequest.txt b/Tests/fixtures/characterisation/gitlab-prompt-createPullRequest.txt new file mode 100644 index 000000000..b72ecacbd --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-prompt-createPullRequest.txt @@ -0,0 +1 @@ +Create a merge request for this workspace against {{base_branch}}. \ No newline at end of file diff --git a/Tests/fixtures/characterisation/gitlab-prompt-fixConflicts.txt b/Tests/fixtures/characterisation/gitlab-prompt-fixConflicts.txt new file mode 100644 index 000000000..46fbc92e8 --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-prompt-fixConflicts.txt @@ -0,0 +1,3 @@ +Merge request !{{number}} conflicts with {{base_branch}}, so GitLab will not merge it as it stands. Resolve that here, in this worktree, on {{branch}}: bring {{base_branch}} into this branch, work through the conflicts, commit, and push {{branch}}, so the conflict is gone for everybody rather than only here. + +Do not merge the merge request. \ No newline at end of file diff --git a/Tests/fixtures/characterisation/gitlab-prompt-mergePullRequest.txt b/Tests/fixtures/characterisation/gitlab-prompt-mergePullRequest.txt new file mode 100644 index 000000000..478dcd696 --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-prompt-mergePullRequest.txt @@ -0,0 +1 @@ +Merge merge request !{{number}} into {{base_branch}} as a {{method}} ({{method_flag}}). It is on the branch {{branch}}. \ No newline at end of file diff --git a/Tests/fixtures/characterisation/gitlab-prompt-pushLocalWork.txt b/Tests/fixtures/characterisation/gitlab-prompt-pushLocalWork.txt new file mode 100644 index 000000000..f3336a102 --- /dev/null +++ b/Tests/fixtures/characterisation/gitlab-prompt-pushLocalWork.txt @@ -0,0 +1 @@ +Commit everything outstanding in this worktree, with a message that describes the change the way this project words one, then push {{branch}} so its merge request reflects what is here. If there is nothing to commit, just push. \ No newline at end of file