diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs index 1375e9de5..7429c43b4 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs @@ -377,10 +377,11 @@ public async Task MergePullRequestAsync(ProviderCo // GitHub doesn't delete the head branch on merge — do it as a follow-up when asked, whether this attempt // merged or an earlier one did. - if (input.DeleteSourceBranch && result.Merged) - await DeleteSourceBranchAsync(context, client, repository, number, cancellationToken).ConfigureAwait(false); + if (!input.DeleteSourceBranch || !result.Merged) return result; - return result; + var (deletion, detail) = await DeleteSourceBranchAsync(context, client, repository, number, cancellationToken).ConfigureAwait(false); + + return result with { SourceBranchDeletion = deletion, SourceBranchDetail = detail }; } private static RemotePullRequestMergeResult ToMergeResult(PullRequestMerge merge) => new() { Merged = merge.Merged, Sha = merge.Sha, Message = merge.Message }; @@ -393,18 +394,73 @@ public async Task MergePullRequestAsync(ProviderCo return pr.Merged ? new RemotePullRequestMergeResult { Merged = true, Sha = pr.MergeCommitSha } : null; } - /// Its own retried step, so a blip here re-runs the cleanup — never the merge. Needs the PR's head ref, so fetch it; a delete failure (already gone / protected) is swallowed so it never fails an otherwise-successful merge. - private async Task DeleteSourceBranchAsync(ProviderContext context, GitHubClient client, RemoteRepository repository, int number, CancellationToken cancellationToken) + /// + /// Its own retried step, so a blip here re-runs the cleanup — never the merge. The merge already stands, so a cleanup + /// that cannot be done — refused, failed, or cancelled — is reported in the result, never thrown: a throw would read + /// as a failed merge. A cancel stops the cleanup at its next wait; a delete in flight then is not confirmed either way. + /// + private async Task<(SourceBranchDeletion Deletion, string Detail)> DeleteSourceBranchAsync(ProviderContext context, GitHubClient client, RemoteRepository repository, int number, CancellationToken cancellationToken) { - await _resilience.ExecuteAsync(context.Instance, nameof(MergePullRequestAsync) + "/delete-source-branch", async _ => + try { - var pr = await client.PullRequest.Get(repository.NamespacePath, repository.Name, number).ConfigureAwait(false); + return await _resilience.ExecuteAsync(context.Instance, nameof(MergePullRequestAsync) + "/delete-source-branch", _ => DeleteOwnHeadBranchAsync(client, repository, number), cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + return (SourceBranchDeletion.Failed, "The merge stands, but deleting its source branch was cancelled before GitHub confirmed it."); + } + catch (Exception ex) + { + return (SourceBranchDeletion.Failed, $"The merge stands, but its source branch was not deleted: {ex.Message}"); + } + } - if (string.IsNullOrEmpty(pr.Head?.Ref)) return; + /// + /// The credential is the base repository's, and heads/{head.ref} names a branch in the base repository. That is + /// the pull request's branch only when the head lives here: a fork's head names a branch in the fork, and the base's + /// branch of the same name belongs to someone else, so a fork's head is kept. + /// + private static async Task<(SourceBranchDeletion Deletion, string Detail)> DeleteOwnHeadBranchAsync(GitHubClient client, RemoteRepository repository, int number) + { + var pr = await client.PullRequest.Get(repository.NamespacePath, repository.Name, number).ConfigureAwait(false); - try { await client.Git.Reference.Delete(repository.NamespacePath, repository.Name, $"heads/{pr.Head.Ref}").ConfigureAwait(false); } - catch (ApiException) { /* branch already deleted / protected — the merge still succeeded */ } - }, cancellationToken).ConfigureAwait(false); + if (!IsHeadInBaseRepository(pr)) return (SourceBranchDeletion.SkippedFork, ForkHeadKeptDetail(pr, repository)); + + await DeleteBranchAsync(client, repository, pr.Head.Ref).ConfigureAwait(false); + + return (SourceBranchDeletion.Deleted, $"Deleted '{pr.Head.Ref}' from {repository.FullPath}."); + } + + /// One repository = one GitHub repository id on both ends. GitHub reports a fork deleted since the pull request was opened as no head repository, which is not this one either. + private static bool IsHeadInBaseRepository(PullRequest pr) => pr.Head?.Repository is { } head && pr.Base?.Repository is { } baseRepository && head.Id == baseRepository.Id; + + private static string ForkHeadKeptDetail(PullRequest pr, RemoteRepository repository) => + $"Kept '{pr.Head?.Ref}': the pull request's head is in {pr.Head?.Repository?.FullName ?? "a repository GitHub no longer reports"}, not {repository.FullPath}, and a source branch is deleted only from its own repository."; + + /// A refused delete of a branch that is already gone (an earlier attempt's delete landed and its answer was lost, or the repository deletes head branches itself) leaves what was asked for; a refusal while the branch is still there stands. + private static async Task DeleteBranchAsync(GitHubClient client, RemoteRepository repository, string branch) + { + try + { + await client.Git.Reference.Delete(repository.NamespacePath, repository.Name, $"heads/{branch}").ConfigureAwait(false); + } + catch (ApiException) + { + if (await BranchExistsAsync(client, repository, branch).ConfigureAwait(false)) throw; + } + } + + private static async Task BranchExistsAsync(GitHubClient client, RemoteRepository repository, string branch) + { + try + { + await client.Repository.Branch.Get(repository.NamespacePath, repository.Name, branch).ConfigureAwait(false); + return true; + } + catch (NotFoundException) + { + return false; + } } public async Task> ListIssuesAsync(ProviderContext context, RemoteRepository repository, IssueState? stateFilter, int page, int perPage, CancellationToken cancellationToken) diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs index 2f023d9c1..a411b40a4 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs @@ -338,9 +338,17 @@ public async Task MergePullRequestAsync(ProviderCo cancellationToken).ConfigureAwait(false); var merged = string.Equals(accepted.State, "merged", StringComparison.OrdinalIgnoreCase) || accepted.MergeCommitSha != null; - return new RemotePullRequestMergeResult { Merged = merged, Sha = accepted.MergeCommitSha }; + var result = new RemotePullRequestMergeResult { Merged = merged, Sha = accepted.MergeCommitSha }; + + if (!input.DeleteSourceBranch || !merged) return result; + + return result with { SourceBranchDeletion = SourceBranchDeletion.Requested, SourceBranchDetail = SourceBranchRequestedDetail(accepted) }; } + /// GitLab deletes the source branch itself, from the merge request's own source project (a fork's branch in the fork, never a same-named branch of the target), and only when the merging identity may push there. + private static string SourceBranchRequestedDetail(MergeRequest accepted) => + $"Asked GitLab to delete '{accepted.SourceBranch}' from the merge request's own source project once the merge completes; GitLab does so when the merging identity may push there."; + /// The merge an earlier attempt landed, read back from the merge request. private static async Task FindMergedAsync(IMergeRequestClient mergeRequests, int iid, CancellationToken cancellationToken) { diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitMergePullRequestNode.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitMergePullRequestNode.cs index 2f68ddb2c..81dc20694 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitMergePullRequestNode.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitMergePullRequestNode.cs @@ -14,7 +14,7 @@ namespace CodeSpace.Core.Services.Workflows.Nodes.Builtin; /// completion half of the Git write surface (open → review → merge). Inputs: repositoryId, /// number, optional method (merge / squash / rebase) / commitTitle / /// commitMessage / deleteSourceBranch / actAsUserId. Outputs merged, sha, -/// message. +/// message, and what became of the source branch (sourceBranchDeletion, sourceBranchDetail). /// /// Wire number from upstream (e.g. an auto-merge-after-approval workflow). The provider translates /// the neutral input to its own API (GitHub merge; GitLab accept). @@ -71,7 +71,7 @@ public GitMergePullRequestNode(IPullRequestService prService) "method": { "type": "string", "enum": ["merge","squash","rebase"], "x-control": "segmented", "x-enumLabels": { "merge": "Merge commit", "squash": "Squash", "rebase": "Rebase" }, "description": "How to integrate the commits. Default: merge commit.", "x-spotlight": 2 }, "commitTitle": { "type": "string", "description": "Optional merge-commit title (squash/merge). Provider default when empty." }, "commitMessage": { "type": "string", "x-long": true, "description": "Optional merge-commit message body." }, - "deleteSourceBranch": { "type": "boolean", "description": "Delete the source branch after a successful merge.", "x-spotlight": 3 }, + "deleteSourceBranch": { "type": "boolean", "description": "Delete the source branch after a successful merge, only from the pull request's own repository: a fork's branch is never matched to a same-named branch of the base. The sourceBranchDeletion output says what happened.", "x-spotlight": 3 }, "actAsUserId": { "type": "string", "format": "uuid", "x-selector": "actorUser", "description": "Merge AS this CodeSpace user's own linked GitHub/GitLab identity. Omit to use the repository's connection credential." } }, "required": ["repositoryId","number"] @@ -83,7 +83,9 @@ public GitMergePullRequestNode(IPullRequestService prService) "properties": { "merged": { "type": "boolean" }, "sha": { "type": ["string","null"] }, - "message": { "type": ["string","null"] } + "message": { "type": ["string","null"] }, + "sourceBranchDeletion": { "type": "string", "enum": ["NotRequested","Deleted","Requested","SkippedFork","Failed"], "description": "What became of the source branch. Requested: left to the provider (GitLab). SkippedFork: the head lives in a fork, so nothing was deleted. Failed: the merge stands but the branch was not deleted, or its delete was cancelled before it was confirmed." }, + "sourceBranchDetail": { "type": ["string","null"], "description": "The same in words: which branch, where, and why it was kept or not deleted." } } } """) @@ -115,20 +117,22 @@ public async Task RunAsync(NodeRunContext context, CancellationToken action: ct => _prService.MergePullRequestAsync(repoId, teamId, number, input, actAsUserId, ct), completionExtractor: r => new ExternalCallCompletion { - ResponsePayload = JsonSerializer.SerializeToElement(new { merged = r.Merged, sha = r.Sha }) + ResponsePayload = JsonSerializer.SerializeToElement(new { merged = r.Merged, sha = r.Sha, source_branch_deletion = r.SourceBranchDeletion.ToString() }) }, cancellationToken: cancellationToken).ConfigureAwait(false); } catch (ProviderInsufficientScopeException ex) { return NodeResult.Fail(DescribeMergeFailure(ex, number)); } catch (ProviderApiException ex) { return NodeResult.Fail(DescribeMergeFailure(ex, number)); } - context.Logger.LogInformation("Merged PR #{Num} on repo {RepoId} (merged={Merged}, method {Method})", number, repoId, result.Merged, method); + context.Logger.LogInformation("Merged PR #{Num} on repo {RepoId} (merged={Merged}, method {Method}, source branch {SourceBranchDeletion})", number, repoId, result.Merged, method, result.SourceBranchDeletion); var outputs = new Dictionary { ["merged"] = JsonSerializer.SerializeToElement(result.Merged), ["sha"] = JsonSerializer.SerializeToElement(result.Sha), - ["message"] = JsonSerializer.SerializeToElement(result.Message) + ["message"] = JsonSerializer.SerializeToElement(result.Message), + ["sourceBranchDeletion"] = JsonSerializer.SerializeToElement(result.SourceBranchDeletion.ToString()), + ["sourceBranchDetail"] = JsonSerializer.SerializeToElement(result.SourceBranchDetail) }; return NodeResult.Ok(outputs); diff --git a/backend/src/CodeSpace.Messages/Dtos/Providers/MergePullRequestInput.cs b/backend/src/CodeSpace.Messages/Dtos/Providers/MergePullRequestInput.cs index 624cbcdac..c70b5e4ae 100644 --- a/backend/src/CodeSpace.Messages/Dtos/Providers/MergePullRequestInput.cs +++ b/backend/src/CodeSpace.Messages/Dtos/Providers/MergePullRequestInput.cs @@ -29,14 +29,39 @@ public sealed record MergePullRequestInput /// Optional merge-commit message body. Provider default when null. public string? CommitMessage { get; init; } - /// Delete the source branch after a successful merge. Default false. + /// Delete the source branch after a successful merge — only from the pull request's own repository, never a same-named branch of the base for a fork's pull request. Default false. public bool DeleteSourceBranch { get; init; } } +/// What became of a merged pull request's source branch. Provider-neutral. +public enum SourceBranchDeletion +{ + /// No delete was attempted: the merge did not ask for one, or nothing merged. + NotRequested, + + /// The branch is gone from the pull request's own repository — this merge deleted it, or it was already gone when asked. + Deleted, + + /// Handed to the provider, which removes the branch from the request's own source project after the merge when the merging identity may (GitLab). + Requested, + + /// Kept: the head branch lives in another repository (a fork). The base repository's ref of the same name is a different branch, so nothing is deleted. + SkippedFork, + + /// The delete was refused, could not be made, or was cancelled before it was confirmed. The merge still stands; the detail says why. + Failed +} + /// Outcome of a merge: whether it merged, and (when available) the resulting commit sha + a provider message. public sealed record RemotePullRequestMergeResult { public required bool Merged { get; init; } public string? Sha { get; init; } public string? Message { get; init; } + + /// What became of the source branch. unless the merge asked for it to go. + public SourceBranchDeletion SourceBranchDeletion { get; init; } + + /// The same in words: which branch, where, and why it was kept or not deleted. Null when no delete was asked for. + public string? SourceBranchDetail { get; init; } } diff --git a/backend/tests/CodeSpace.IntegrationTests/Providers/PullRequestMergeSourceBranchFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Providers/PullRequestMergeSourceBranchFlowTests.cs new file mode 100644 index 000000000..e3c86785b --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Providers/PullRequestMergeSourceBranchFlowTests.cs @@ -0,0 +1,121 @@ +using System.Text.Json; +using Autofac; +using CodeSpace.Core.Persistence.Db; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Workflows.Nodes; +using CodeSpace.Core.Services.Workflows.Runtime; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Enums; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Providers; + +/// +/// git.merge_pr with deleteSourceBranch, taken from the node registry and run through +/// IPullRequestService, the provider registry, the real GitHub provider and Octokit, on real Postgres, against a +/// loopback GitHub. This is the chain an outsider's fork pull request reaches, whether a workflow merges it or an agent +/// tool does. The head branch is named release, like acme/api's own release branch: the pull request's branch only +/// when its head lives in acme/api. +/// +/// Fidelity: high for everything CodeSpace runs; GitHub is the loopback. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class PullRequestMergeSourceBranchFlowTests +{ + private readonly PostgresFixture _fixture; + + public PullRequestMergeSourceBranchFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Theory] + [InlineData(false, "Deleted", 1)] + [InlineData(true, "SkippedFork", 0)] + public async Task A_merged_pull_request_s_branch_is_deleted_only_in_its_own_repository(bool headInFork, string expectedOutcome, int expectedDeletes) + { + using var github = new StubProviderHost() + .Answer("PUT", "/repos/acme/api/pulls/77/merge", 200, """{"sha":"9f8e7d6c5b4a","merged":true,"message":"Pull Request successfully merged"}""") + .Answer("GET", "/repos/acme/api/pulls/77", 200, PullRequestJson(headInFork ? Outsider : AcmeApi)) + .Answer("DELETE", "/repos/acme/api/git/refs/heads/release", 204, string.Empty); + + var seed = await SeedGitHubRepositoryAsync(github.BaseUrl); + + using var scope = _fixture.BeginScope(); + var result = await scope.Resolve().Resolve("git.merge_pr").RunAsync(Context(seed), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Success, result.Error); + result.Outputs["merged"].GetBoolean().ShouldBeTrue(); + result.Outputs["sourceBranchDeletion"].GetString().ShouldBe(expectedOutcome, result.Outputs["sourceBranchDetail"].GetString()); + github.Requests.Count(r => r.Method == "DELETE").ShouldBe(expectedDeletes, "acme/api's release is deleted only when it is the pull request's own head branch"); + } + + private static readonly object AcmeApi = new { id = 4242, name = "api", full_name = "acme/api", owner = new { login = "acme" } }; + + private static readonly object Outsider = new { id = 9090, name = "api", full_name = "outsider/api", owner = new { login = "outsider" }, fork = true }; + + private static string PullRequestJson(object headRepository) => JsonSerializer.Serialize(new + { + id = 7077, + number = 77, + title = "Release fixes", + state = "closed", + merged = true, + merged_at = "2026-09-24T08:00:00Z", + merge_commit_sha = "9f8e7d6c5b4a", + head = new { @ref = "release", sha = "0a1b2c3d", repo = headRepository }, + @base = new { @ref = "main", sha = "4e5f6a7b", repo = AcmeApi }, + user = new { login = "outsider" }, + html_url = "https://github.test/acme/api/pull/77" + }); + + private static NodeRunContext Context(SeedResult seed) => new() + { + Inputs = new Dictionary + { + ["repositoryId"] = JsonSerializer.SerializeToElement(seed.RepositoryId.ToString()), + ["number"] = JsonSerializer.SerializeToElement(77), + ["deleteSourceBranch"] = JsonSerializer.SerializeToElement(true), + }, + Config = new Dictionary(), + RawInputs = JsonDocument.Parse("{}").RootElement, + RawConfig = JsonDocument.Parse("{}").RootElement, + Scope = new NodeRunScope { Trigger = new Dictionary(), Sys = new Dictionary { [SystemScopeKeys.TeamId] = JsonSerializer.SerializeToElement(seed.TeamId.ToString()) } }, + Logger = NullLogger.Instance, + Observability = NodeObservability.NoOp, + }; + + private async Task SeedGitHubRepositoryAsync(string baseUrl) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var encryptor = scope.Resolve(); + + var suffix = Guid.NewGuid().ToString("N")[..8]; + var team = new Team { Id = Guid.NewGuid(), Slug = $"t-{suffix}", Name = "Team" }; + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = team.Id, Provider = ProviderKind.GitHub, DisplayName = "loopback", BaseUrl = baseUrl, ApiUrl = baseUrl }; + var credential = new Credential + { + Id = Guid.NewGuid(), TeamId = team.Id, ProviderInstanceId = instance.Id, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.Pat, DisplayName = "connection", + EncryptedPayload = encryptor.Encrypt(scope.Resolve().Serialize(new PatPayload { Token = "fake-loopback-token" })), Status = CredentialStatus.Active + }; + var repository = new Repository + { + Id = Guid.NewGuid(), TeamId = team.Id, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ExternalId = "4242", NamespacePath = "acme", Name = "api", FullPath = "acme/api", + DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "https://github.test/acme/api", Status = RepositoryStatus.Active + }; + + db.Team.Add(team); + db.ProviderInstance.Add(instance); + db.Credential.Add(credential); + db.Repository.Add(repository); + + await db.SaveChangesAsync().ConfigureAwait(false); + + return new SeedResult(team.Id, repository.Id); + } + + private sealed record SeedResult(Guid TeamId, Guid RepositoryId); +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubWriteRetryTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubWriteRetryTests.cs index 68ef7cfde..7ec042162 100644 --- a/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubWriteRetryTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubWriteRetryTests.cs @@ -142,6 +142,7 @@ public async Task Merge_merges_exactly_once_and_still_deletes_the_source_branch( result.Sha.ShouldBe(ForgeMergeablePullRequest.MergeSha); _github.Sent("PUT", "/repos/acme/api/pulls/7/merge").ShouldBe(expectedMerges); pull.BranchDeletes.ShouldBe(1, "the caller asked for the source branch to go; finding the merge already done must not skip that"); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.Deleted); } [Fact] @@ -436,12 +437,14 @@ public StubReply Merge(RecordedRequest _) merged = _merged, merged_at = _merged ? "2026-09-24T08:00:00Z" : null, // Octokit derives PullRequest.Merged from merged_at merge_commit_sha = _merged ? MergeSha : null, - head = new { @ref = "feature/retry", sha = "0a1b2c3d" }, - @base = new { @ref = "main", sha = "4e5f6a7b" }, + head = new { @ref = "feature/retry", sha = "0a1b2c3d", repo = AcmeApi }, // GitHub names each end's repository; a same-repository head is the base's + @base = new { @ref = "main", sha = "4e5f6a7b", repo = AcmeApi }, user = new { login = "codespace-bot" }, html_url = "https://github.test/acme/api/pull/7" })); + private static readonly object AcmeApi = new { id = 4242, name = "api", full_name = "acme/api", owner = new { login = "acme" } }; + public StubReply DeleteBranch(RecordedRequest _) { BranchDeletes++; diff --git a/backend/tests/CodeSpace.UnitTests/Providers/MergeSourceBranchTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/MergeSourceBranchTests.cs new file mode 100644 index 000000000..da88be240 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/MergeSourceBranchTests.cs @@ -0,0 +1,324 @@ +using System.Text.Json; +using CodeSpace.Core.Persistence.Entities; +using CodeSpace.Core.Services.Providers; +using CodeSpace.Core.Services.Providers.Auth; +using CodeSpace.Core.Services.Providers.Errors; +using CodeSpace.Core.Services.Providers.Events; +using CodeSpace.Core.Services.Providers.GitHub; +using CodeSpace.Core.Services.Providers.GitLab; +using CodeSpace.Core.Services.Providers.Resilience; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using static CodeSpace.IntegrationTests.Webhooks.StubProviderHost; + +namespace CodeSpace.UnitTests.Providers; + +/// +/// Merging with deleteSourceBranch through the real providers (Octokit and NGitLab, the wire, the resilience wrapper) +/// against a loopback forge. GitHub never deletes a head branch on merge, so the provider deletes heads/{head.ref} +/// itself, in the BASE repository, the only one its credential is for. That ref is the pull request's branch only when +/// the head lives in the base repository. A fork's head names a branch in the fork, and the base's branch of the same name +/// (release, a teammate's feature) belongs to someone else. GitLab deletes server-side, from the merge request's own +/// source project, so the provider only asks. +/// +[Trait("Category", "Unit")] +public sealed class MergeSourceBranchTests : IDisposable +{ + private readonly StubProviderHost _forge = new(); + + public void Dispose() => _forge.Dispose(); + + [Theory] + [InlineData(DeleteAnswer.Deletes, 1)] + [InlineData(DeleteAnswer.DeletesThenGatewayError, 1)] + [InlineData(DeleteAnswer.DeletesThenConnectionDrops, 2)] + [InlineData(DeleteAnswer.AlreadyGone, 1)] + public async Task GitHub_deletes_a_head_branch_that_lives_in_the_base_repository(DeleteAnswer answer, int expectedDeletes) + { + // However GitHub answers the DELETE, what is reported is what is left: a delete whose answer was lost after it + // landed, or a branch the repository's own auto-delete already removed, leaves the branch gone, as asked. + var github = new LoopbackGitHub(_forge, "feature/retry", Acme, answer); + + var result = await MergeOnGitHubAsync(deleteSourceBranch: true); + + result.Merged.ShouldBeTrue(); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.Deleted, result.SourceBranchDetail); + result.SourceBranchDetail.ShouldBe("Deleted 'feature/retry' from acme/api."); + github.BranchExists.ShouldBeFalse(); + _forge.Sent("DELETE", "/repos/acme/api/git/refs/heads/feature/retry").ShouldBe(expectedDeletes); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task GitHub_keeps_a_fork_s_head_and_never_deletes_the_base_branch_of_the_same_name(bool forkStillExists) + { + // An outsider named their fork branch after acme/api's real release branch. GitHub reports a fork deleted since the + // pull request was opened as head.repo = null, and an unknown head repository is not the base repository either. + var github = new LoopbackGitHub(_forge, "release", forkStillExists ? Outsider : null, DeleteAnswer.Deletes); + + var result = await MergeOnGitHubAsync(deleteSourceBranch: true); + + result.Merged.ShouldBeTrue("keeping the branch never undoes the merge"); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.SkippedFork); + result.SourceBranchDetail.ShouldBe(forkStillExists ? "Kept 'release': the pull request's head is in outsider/api, not acme/api, and a source branch is deleted only from its own repository." : "Kept 'release': the pull request's head is in a repository GitHub no longer reports, not acme/api, and a source branch is deleted only from its own repository."); + github.BranchExists.ShouldBeTrue("acme/api's release is not the pull request's branch"); + _forge.Requests.ShouldNotContain(r => r.Method == "DELETE", "the base repository's credential deletes nothing for a fork's pull request"); + } + + [Fact] + public async Task GitHub_reports_a_refused_delete_and_the_merge_still_stands() + { + var github = new LoopbackGitHub(_forge, "feature/retry", Acme, DeleteAnswer.RefusesProtected); + + var result = await MergeOnGitHubAsync(deleteSourceBranch: true); + + result.Merged.ShouldBeTrue("the merge landed; a refused cleanup must not turn it into a failed merge"); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.Failed); + result.SourceBranchDetail.ShouldNotBeNull().ShouldContain("HTTP 422"); + result.SourceBranchDetail.ShouldContain("Cannot delete this protected branch", Case.Sensitive, "GitHub's own words say why"); + github.BranchExists.ShouldBeTrue(); + } + + [Theory] + [InlineData(502)] + [InlineData(403)] + public async Task GitHub_reports_a_refused_delete_as_failed_when_the_branch_cannot_be_read_back(int branchReadStatus) + { + // Only a 404 shows that a refused delete left nothing behind. A read that fails says nothing about the branch, so + // the refusal stands: taken as "gone", it would report Deleted for a branch that is still there. + var github = new LoopbackGitHub(_forge, "feature/retry", Acme, DeleteAnswer.RefusesProtected) { BranchReadFailure = branchReadStatus }; + + var result = await MergeOnGitHubAsync(deleteSourceBranch: true); + + result.Merged.ShouldBeTrue(); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.Failed, result.SourceBranchDetail); + result.SourceBranchDetail.ShouldNotBeNull().ShouldContain($"HTTP {branchReadStatus}"); + result.SourceBranchDetail.ShouldNotContain("Deleted 'feature/retry'", Case.Sensitive); + github.BranchExists.ShouldBeTrue(); + } + + [Theory] + [InlineData(DeleteAnswer.DeletesThenConnectionDrops, false, "The merge stands, but deleting its source branch was cancelled before GitHub confirmed it.")] + [InlineData(DeleteAnswer.RefusesProtected, true, "Cannot delete this protected branch")] + public async Task GitHub_reports_a_cleanup_cancelled_mid_delete_and_the_merge_still_stands(DeleteAnswer answer, bool branchLeft, string expectedDetail) + { + // The caller cancels while the DELETE is in flight, after the merge landed. The cancel stops the cleanup (no + // second DELETE) and what the cleanup knew is reported like any cleanup that could not finish: a throw here would + // read as a failed merge. A delete whose answer was lost is not confirmed either way, so it is not called deleted. + using var cancel = new CancellationTokenSource(); + var github = new LoopbackGitHub(_forge, "feature/retry", Acme, answer) { OnDelete = cancel.Cancel }; + + var result = await MergeOnGitHubAsync(deleteSourceBranch: true, cancel.Token); + + result.Merged.ShouldBeTrue("the merge landed before the cancel"); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.Failed, result.SourceBranchDetail); + result.SourceBranchDetail.ShouldNotBeNull().ShouldContain(expectedDetail); + github.BranchExists.ShouldBe(branchLeft); + _forge.Sent("DELETE", "/repos/acme/api/git/refs/heads/feature/retry").ShouldBe(1, "a cancelled cleanup does not try again"); + } + + [Fact] + public async Task GitHub_reports_a_cleanup_that_cannot_read_the_pull_request_and_the_merge_still_stands() + { + _forge.Answer("PUT", "/repos/acme/api/pulls/7/merge", 200, MergedJson).Answer("GET", "/repos/acme/api/pulls/7", 503, """{"message":"Service Unavailable"}"""); + + var result = await MergeOnGitHubAsync(deleteSourceBranch: true); + + result.Merged.ShouldBeTrue("the merge landed before the cleanup started"); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.Failed); + result.SourceBranchDetail.ShouldNotBeNull().ShouldContain("HTTP 503"); + _forge.Requests.ShouldNotContain(r => r.Method == "DELETE"); + } + + [Fact] + public async Task GitHub_reads_and_deletes_nothing_when_no_delete_was_asked_for() + { + var github = new LoopbackGitHub(_forge, "feature/retry", Acme, DeleteAnswer.Deletes); + + var result = await MergeOnGitHubAsync(deleteSourceBranch: false); + + result.Merged.ShouldBeTrue(); + result.SourceBranchDeletion.ShouldBe(SourceBranchDeletion.NotRequested); + result.SourceBranchDetail.ShouldBeNull(); + github.BranchExists.ShouldBeTrue(); + _forge.Requests.ShouldHaveSingleItem().Method.ShouldBe("PUT", "the merge is the only call"); + } + + [Theory] + [InlineData(true, SourceBranchDeletion.Requested)] + [InlineData(false, SourceBranchDeletion.NotRequested)] + public async Task GitLab_asks_for_the_source_branch_to_go_and_leaves_where_to_GitLab(bool deleteSourceBranch, SourceBranchDeletion expected) + { + // A fork's merge request (source project 9090, target 4242). GitLab removes the source branch from the merge + // request's source project, never from the target, so the provider sends the flag and deletes nothing itself. + // Sending false explicitly also keeps GitLab from falling back to the author's own "delete source branch" choice. + _forge.Answer("PUT", "/api/v4/projects/4242/merge_requests/7/merge", 200, GitLabForkMergeRequestJson); + + var result = await GitLabProvider().MergePullRequestAsync(GitLabContext(), Repository, 7, new MergePullRequestInput { DeleteSourceBranch = deleteSourceBranch }, CancellationToken.None); + + result.Merged.ShouldBeTrue(); + result.SourceBranchDeletion.ShouldBe(expected); + result.SourceBranchDetail.ShouldBe(deleteSourceBranch ? "Asked GitLab to delete 'release' from the merge request's own source project once the merge completes; GitLab does so when the merging identity may push there." : null); + var accept = _forge.Requests.ShouldHaveSingleItem("the accept is the only call; no branch is deleted from the target project"); + JsonDocument.Parse(accept.Body).RootElement.GetProperty("should_remove_source_branch").GetBoolean().ShouldBe(deleteSourceBranch); + } + + /// How the loopback GitHub answers the DELETE of the head branch's ref. + public enum DeleteAnswer + { + /// Deletes the branch and answers 204. + Deletes, + + /// Deletes the branch, then a gateway answers 502. + DeletesThenGatewayError, + + /// Deletes the branch, then the connection drops mid-answer. + DeletesThenConnectionDrops, + + /// The branch is already gone (the repository deletes head branches itself); GitHub answers 422. + AlreadyGone, + + /// The branch is protected; GitHub refuses with 422 and the branch stays. + RefusesProtected + } + + // ── Loopback GitHub ── + + private static readonly object Acme = new { id = 4242, name = "api", full_name = "acme/api", owner = new { login = "acme" } }; + + private static readonly object Outsider = new { id = 9090, name = "api", full_name = "outsider/api", owner = new { login = "outsider" }, fork = true }; + + private const string MergeSha = "9f8e7d6c5b4a"; + + private static readonly string MergedJson = JsonSerializer.Serialize(new { sha = MergeSha, merged = true, message = "Pull Request successfully merged" }); + + private static readonly RemoteRepository Repository = new() + { + ExternalId = "4242", + NamespacePath = "acme", + Name = "api", + FullPath = "acme/api", + DefaultBranch = "main", + Visibility = RepositoryVisibility.Private, + WebUrl = "https://forge.test/acme/api" + }; + + private Task MergeOnGitHubAsync(bool deleteSourceBranch, CancellationToken cancellationToken = default) => + GitHubProvider().MergePullRequestAsync(Context(ProviderKind.GitHub), Repository, 7, new MergePullRequestInput { Method = PullRequestMergeMethod.Squash, DeleteSourceBranch = deleteSourceBranch }, cancellationToken); + + private ProviderContext GitLabContext() => Context(ProviderKind.GitLab); + + private ProviderContext Context(ProviderKind kind) => new(new ProviderInstance { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = kind, DisplayName = "loopback", BaseUrl = _forge.BaseUrl, ApiUrl = kind == ProviderKind.GitHub ? _forge.BaseUrl : null }, new Credential { Id = Guid.NewGuid(), AuthType = AuthType.Pat, DisplayName = "pat", EncryptedPayload = "unused" }); + + private static GitHubRepositoryProvider GitHubProvider() + { + var resilience = new ExternalCallResilience(new ProviderErrorMapperRegistry(new IProviderErrorMapper[] { new GitHubErrorMapper() }), NullLogger.Instance); + + return new GitHubRepositoryProvider(new StaticTokenAuth(), resilience, new GitHubSignatureVerifier(), new GitHubEventNormalizer(new ProviderEventSubscriptionRegistry(Array.Empty())), new GitHubWebhookRepositoryIdentifier()); + } + + private static GitLabRepositoryProvider GitLabProvider() + { + var resilience = new ExternalCallResilience(new ProviderErrorMapperRegistry(new IProviderErrorMapper[] { new GitLabErrorMapper() }), NullLogger.Instance); + + return new GitLabRepositoryProvider(new StaticTokenAuth(), resilience, new GitLabSignatureVerifier(), new GitLabEventNormalizer(new ProviderEventSubscriptionRegistry(Array.Empty())), new GitLabWebhookRepositoryIdentifier()); + } + + /// + /// acme/api, with pull request #7 from headRef in headRepository and a branch of that name in acme/api. + /// The merge lands. A DELETE of the ref answers per ; a branch read answers whether it is + /// still there, or fails with when one is set. + /// + private sealed class LoopbackGitHub + { + private readonly string _headRef; + private readonly object? _headRepository; + private readonly DeleteAnswer _answer; + + public LoopbackGitHub(StubProviderHost host, string headRef, object? headRepository, DeleteAnswer answer) + { + _headRef = headRef; + _headRepository = headRepository; + _answer = answer; + BranchExists = answer != DeleteAnswer.AlreadyGone; + + host.Answer("PUT", "/repos/acme/api/pulls/7/merge", 200, MergedJson).Answer("GET", "/repos/acme/api/pulls/7", _ => new StubReply(200, PullRequestJson())).Answer("DELETE", $"/repos/acme/api/git/refs/heads/{headRef}", _ => Delete()).Answer("GET", $"/repos/acme/api/branches/{headRef}", _ => ReadBranch()); + } + + public bool BranchExists { get; private set; } + + /// The status every branch read fails with, whatever the branch's state. Null: reads answer truthfully. + public int? BranchReadFailure { get; init; } + + /// Runs when the DELETE arrives, before it is answered — where a caller's cancel lands mid-cleanup. + public Action? OnDelete { get; init; } + + private StubReply Delete() + { + OnDelete?.Invoke(); + + if (!BranchExists) return new StubReply(422, """{"message":"Reference does not exist"}"""); + if (_answer == DeleteAnswer.RefusesProtected) return new StubReply(422, """{"message":"Cannot delete this protected branch"}"""); + + BranchExists = false; + + return _answer switch + { + DeleteAnswer.DeletesThenGatewayError => new StubReply(502, """{"message":"502 Bad Gateway"}"""), + DeleteAnswer.DeletesThenConnectionDrops => StubReply.DropConnection, + _ => new StubReply(204, string.Empty) + }; + } + + private StubReply ReadBranch() + { + if (BranchReadFailure is { } status) return new StubReply(status, """{"message":"Branch read failed"}"""); + + return BranchExists ? new StubReply(200, JsonSerializer.Serialize(new { name = _headRef, commit = new { sha = "0a1b2c3d" }, @protected = _answer == DeleteAnswer.RefusesProtected })) : new StubReply(404, """{"message":"Branch not found"}"""); + } + + private string PullRequestJson() => JsonSerializer.Serialize(new + { + id = 7007, + number = 7, + title = "Retry safely", + state = "closed", + merged = true, + merged_at = "2026-09-24T08:00:00Z", + merge_commit_sha = MergeSha, + head = new { @ref = _headRef, sha = "0a1b2c3d", repo = _headRepository }, + @base = new { @ref = "main", sha = "4e5f6a7b", repo = Acme }, + user = new { login = "outsider" }, + html_url = "https://forge.test/acme/api/pull/7" + }); + } + + // ── Loopback GitLab ── + + private static readonly string GitLabForkMergeRequestJson = JsonSerializer.Serialize(new + { + id = 7007, + iid = 7, + project_id = 4242, + source_project_id = 9090, + target_project_id = 4242, + title = "Retry safely", + state = "merged", + merge_commit_sha = MergeSha, + source_branch = "release", + target_branch = "main", + author = new { id = 2, username = "outsider", name = "Outsider" }, + created_at = "2026-09-24T08:00:00.000Z", + updated_at = "2026-09-24T08:00:00.000Z", + web_url = "https://forge.test/acme/api/-/merge_requests/7" + }); + + private sealed class StaticTokenAuth : IProviderAuthResolver + { + public Task ResolveAsync(ProviderContext context, CancellationToken cancellationToken) => Task.FromResult(new ResolvedAuth { Token = "fake-loopback-token" }); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/GitMergePullRequestNodeTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/GitMergePullRequestNodeTests.cs index 3774a83ac..8eb6d63fa 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/GitMergePullRequestNodeTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/GitMergePullRequestNodeTests.cs @@ -15,7 +15,7 @@ namespace CodeSpace.UnitTests.Workflows; /// git.merge_pr — drives the real node against a stub that records the /// it was called with and returns a canned result (or throws), so input /// parsing (required repositoryId/number, method default + parse, commit title/message, deleteSourceBranch, -/// actAsUserId), the output shape (merged/sha/message), and the typed-provider-failure → actionable-message +/// actAsUserId), the output shape (merged/sha/message/sourceBranchDeletion/sourceBranchDetail), and the typed-provider-failure → actionable-message /// mapping (scope, 403, 404, 405, 409, 422) are all pinned. /// [Trait("Category", "Unit")] @@ -169,6 +169,32 @@ public async Task Outputs_merged_false_when_the_provider_reports_not_merged() result.Outputs["sha"].ValueKind.ShouldBe(JsonValueKind.Null); } + [Theory] + [InlineData(SourceBranchDeletion.NotRequested, null)] + [InlineData(SourceBranchDeletion.Deleted, "Deleted 'feature/retry' from acme/api.")] + [InlineData(SourceBranchDeletion.Requested, "Asked GitLab to delete 'feature/retry' from the merge request's own source project once the merge completes; GitLab does so when the merging identity may push there.")] + [InlineData(SourceBranchDeletion.SkippedFork, "Kept 'release': the pull request's head is in outsider/api, not acme/api, and a source branch is deleted only from its own repository.")] + [InlineData(SourceBranchDeletion.Failed, "The merge stands, but its source branch was not deleted: GitHub returned HTTP 422 for MergePullRequestAsync/delete-source-branch: Cannot delete this protected branch")] + public async Task Outputs_what_became_of_the_source_branch(SourceBranchDeletion deletion, string? detail) + { + var stub = new StubPrService { Result = new() { Merged = true, Sha = "deadbeef", SourceBranchDeletion = deletion, SourceBranchDetail = detail } }; + + var result = await new GitMergePullRequestNode(stub).RunAsync(Context(), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Success, "the merge stands whatever became of its source branch"); + result.Outputs["merged"].GetBoolean().ShouldBeTrue(); + result.Outputs["sourceBranchDeletion"].GetString().ShouldBe(deletion.ToString()); + result.Outputs["sourceBranchDetail"].Deserialize().ShouldBe(detail); + } + + [Fact] + public void Output_schema_declares_every_source_branch_outcome() + { + var declared = new GitMergePullRequestNode(new StubPrService()).Manifest.OutputSchema.GetProperty("properties").GetProperty("sourceBranchDeletion").GetProperty("enum").EnumerateArray().Select(e => e.GetString()); + + declared.ShouldBe(Enum.GetNames(), "a workflow or a model branching on the output reads the vocabulary from the schema"); + } + [Fact] public async Task Fails_when_repository_id_is_missing() {