diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs index 1efed4c60..1375e9de5 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitHub/GitHubRepositoryProvider.cs @@ -235,19 +235,14 @@ public async Task> ListChecksAsync(Provide // caller pass the SHA in, but that leaks Octokit detail into the service layer. var pr = await client.PullRequest.Get(repository.NamespacePath, repository.Name, number).ConfigureAwait(false); var headSha = pr.Head?.Sha; - if (string.IsNullOrEmpty(headSha)) return (IReadOnlyList)Array.Empty(); - try - { - var response = await client.Check.Run.GetAllForReference(repository.NamespacePath, repository.Name, headSha).ConfigureAwait(false); - return (IReadOnlyList)response.CheckRuns.Select(ToRemoteCheck).ToList(); - } - catch (AuthorizationException) - { - // Token lacks `repo` or Checks: Read — graceful empty rather than failing - // the whole PR detail view. - return (IReadOnlyList)Array.Empty(); - } + // Workflows gate merges on this list, so neither a missing head nor a refused read may become "no checks" — + // an empty list reads as green CI. + if (string.IsNullOrEmpty(headSha)) + throw new InvalidOperationException($"GitHub returned pull request #{number} without a head commit, so its checks cannot be read"); + + var response = await client.Check.Run.GetAllForReference(repository.NamespacePath, repository.Name, headSha).ConfigureAwait(false); + return (IReadOnlyList)response.CheckRuns.Select(ToRemoteCheck).ToList(); }, cancellationToken).ConfigureAwait(false); } diff --git a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs index 291b22527..2f023d9c1 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/GitLab/GitLabRepositoryProvider.cs @@ -814,38 +814,67 @@ private static async Task FetchStateCountAsync(string host, int projectId, public async Task> ListChecksAsync(ProviderContext context, RemoteRepository repository, int number, CancellationToken cancellationToken) { var client = await BuildClientAsync(context, cancellationToken).ConfigureAwait(false); + var projectId = int.Parse(repository.ExternalId); - return await _resilience.ExecuteAsync(context.Instance, nameof(ListChecksAsync), _ => - { - try - { - var projectId = int.Parse(repository.ExternalId); - var mrClient = client.GetMergeRequest(projectId); + // No catch: workflows gate merges on this list, so a read that fails — rate limit, outage, refused token, dropped + // connection, a payload NGitLab cannot parse — leaves as a failure. Turned into "no checks" it reads as green CI. + return await _resilience.ExecuteAsync(context.Instance, nameof(ListChecksAsync), _ => Task.FromResult(ReadLatestPipelineChecks(client, projectId, number)), cancellationToken).ConfigureAwait(false); + } - // MR pipelines are returned newest-first. We only render checks from the - // LATEST pipeline — older pipelines belong in a "history" view the SPA - // doesn't have yet, and showing all of them at once would be noise. - var pipelines = mrClient.GetPipelines(number).ToList(); - if (pipelines.Count == 0) return Task.FromResult>(Array.Empty()); + /// + /// The jobs of the merge request's latest pipeline (GitLab lists them newest-first; older pipelines are history), floored + /// by that pipeline's own status. Empty only when GitLab confirms the merge request has no pipeline to show. + /// + private static IReadOnlyList ReadLatestPipelineChecks(IGitLabClient client, int projectId, int number) + { + var latest = client.GetMergeRequest(projectId).GetPipelines(number).FirstOrDefault(); + if (latest == null) return ConfirmNoPipeline(client, projectId, number); - var latest = pipelines[0]; + var jobs = client.GetPipelines(projectId).GetJobs(latest.Id).Select(ToRemoteCheck).ToList(); - // IPipelineClient.GetJobs(pipelineId) is the canonical "list jobs in this - // pipeline" endpoint — one round-trip, returns the typed Job[] directly. - var jobs = client.GetPipelines(projectId).GetJobs(latest.Id); + return FloorByPipelineStatus(jobs, ToPipelineCheck(latest)); + } - var checks = jobs.Select(ToRemoteCheck).ToList(); - return Task.FromResult>(checks); - } - catch - { - // Token without read_api / pipelines scope, or pipelines simply disabled - // on the project — render no checks rather than failing the PR detail view. - return Task.FromResult>(Array.Empty()); - } - }, cancellationToken).ConfigureAwait(false); + /// + /// GitLab answers a merge request's pipeline list with [] both when no pipeline ran and when this credential may read none + /// of them: the list is filtered by read_pipeline, not refused. The read here can use a different credential from the + /// merge (the connection's versus the actor's), so a blind read must not open the gate. [] stands as "no checks" only when + /// CI is off for the project, or when the credential may read the project's pipelines and the merge request names no + /// head pipeline the list left out (one in a fork the credential cannot see). + /// + private static IReadOnlyList ConfirmNoPipeline(IGitLabClient client, int projectId, int number) + { + if (IsCiDisabled(client, projectId)) return Array.Empty(); + + EnsurePipelinesReadable(client, projectId); + EnsureNoUnlistedHeadPipeline(client, projectId, number); + + return Array.Empty(); } + /// CI/CD turned off for the project: no pipeline can run, and the project's own pipeline list refuses every credential. + private static bool IsCiDisabled(IGitLabClient client, int projectId) => client.Projects.GetById(projectId, new SingleProjectQuery()).BuildsAccessLevel == "disabled"; + + /// + /// The project's own pipeline list refuses with 403 a credential that may not read pipelines (or jobs), where the merge + /// request's list answers it []. Reading one page is the check; a refusal leaves as one. + /// + private static void EnsurePipelinesReadable(IGitLabClient client, int projectId) => _ = client.GetPipelines(projectId).Search(new PipelineQuery { PerPage = 1 }).FirstOrDefault(); + + private static void EnsureNoUnlistedHeadPipeline(IGitLabClient client, int projectId, int number) + { + if (client.GetMergeRequest(projectId)[number].HeadPipeline is { } head) + throw new InvalidOperationException($"GitLab lists no pipeline for merge request !{number}, yet names its head pipeline #{head.Id} ({head.Status.ToString().ToLowerInvariant()}) — a pipeline this credential may not read, so the merge request's checks cannot be read"); + } + + /// + /// The pipeline's verdict is a floor under its jobs: when no job carries it — a downstream pipeline failed, a job is + /// missing from the list, the job holding a blocked pipeline reads as an optional manual one — the pipeline joins the list + /// as a check of its own, so a pipeline GitLab does not call success never reads green. + /// + private static IReadOnlyList FloorByPipelineStatus(List jobs, RemotePullRequestCheck pipeline) => + jobs.Any(job => job.Status == pipeline.Status) ? jobs : jobs.Append(pipeline).ToList(); + public async Task PostCommentAsync(ProviderContext context, RemoteRepository repository, int number, string body, CancellationToken cancellationToken) { var client = await BuildClientAsync(context, cancellationToken).ConfigureAwait(false); @@ -1016,6 +1045,30 @@ private static RemotePullRequestCheck ToRemoteCheck(NGitLab.Models.Job job) }; } + /// The pipeline itself as a check — what adds when no job carries the pipeline's verdict. + private static RemotePullRequestCheck ToPipelineCheck(PipelineBasic pipeline) => new() + { + Name = "pipeline", + Status = MapPipelineStatus(pipeline.Status), + Conclusion = pipeline.Status.ToString().ToLowerInvariant(), + DetailsUrl = pipeline.WebUrl + }; + + /// + /// A pipeline passes only when GitLab calls it success. NGitLab types its status as a job status, but the two read + /// differently: a manual or delayed job is optional (Skipped), while a pipeline whose status is manual is blocked on a + /// manual job and one that is scheduled waits on a delayed job — neither is done, so both are Pending. A skipped pipeline + /// ran nothing, and GitLab's own merge check does not count it as success unless the project opts in, so it is + /// Cancelled. Anything else that is not success or failed (created, pending, running, an unknown status) is Pending. + /// + private static PullRequestCheckStatus MapPipelineStatus(JobStatus status) => status switch + { + JobStatus.Success => PullRequestCheckStatus.Success, + JobStatus.Failed => PullRequestCheckStatus.Failure, + JobStatus.Canceled or JobStatus.Canceling or JobStatus.Skipped => PullRequestCheckStatus.Cancelled, + _ => PullRequestCheckStatus.Pending + }; + // GitLab JobStatus: Unknown, Running, Pending, Failed, Success, Created, Canceled, Skipped, // Manual, NoBuild, Preparing, WaitingForResource, Scheduled, Canceling. Note `Canceled` — // single L — not `Cancelled`. diff --git a/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs b/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs index 33da47974..da2ee3ce9 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Resilience/ExternalCallResilience.cs @@ -148,10 +148,15 @@ private async Task AcquireTokenAsync(ProviderInstance instance, string operation /// 4xx (auth, not-found) do not — retrying just wastes quota. SDK exception types /// (Octokit.ApiException, NGitLab.GitLabException) all expose a StatusCode property /// so duck-typed reflection covers every provider without per-SDK exception mapping. + /// A network blip looks different per transport: an HttpClient SDK (Octokit) throws + /// HttpRequestException, while NGitLab speaks HttpWebRequest — a connection it never got + /// an answer on is a WebException without a response, and an answer cut short mid-body is + /// an HttpIOException. /// public static bool IsTransient(Exception exception) { - if (exception is HttpRequestException) return true; + if (exception is HttpRequestException or HttpIOException) return true; + if (exception is WebException { Response: null }) return true; if (exception is TaskCanceledException) return true; var status = ExtractStatusCode(exception); diff --git a/backend/src/CodeSpace.Core/Services/Providers/Resilience/IExternalCallResilience.cs b/backend/src/CodeSpace.Core/Services/Providers/Resilience/IExternalCallResilience.cs index f5774762c..2582bc2ac 100644 --- a/backend/src/CodeSpace.Core/Services/Providers/Resilience/IExternalCallResilience.cs +++ b/backend/src/CodeSpace.Core/Services/Providers/Resilience/IExternalCallResilience.cs @@ -5,7 +5,7 @@ namespace CodeSpace.Core.Services.Providers.Resilience; /// /// Wraps every external SDK call (Octokit, NGitLab, future Bitbucket SDK) with two layers: /// per-ProviderInstance token-bucket rate limiting and exponential-backoff retry on transient -/// failures (HttpRequestException, TaskCanceledException, 5xx HTTP status). +/// failures (HttpRequestException, NGitLab's WebException / HttpIOException, TaskCanceledException, 5xx HTTP status). /// Provider classes call this for every method that hits the wire. Streaming methods /// (IAsyncEnumerable) are intentionally NOT wrapped here — caller handles per-page retry. /// A write that must not land twice (a create, a merge) goes through diff --git a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitFetchPrChecksNode.cs b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitFetchPrChecksNode.cs index 5ec02f692..90817a7a4 100644 --- a/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitFetchPrChecksNode.cs +++ b/backend/src/CodeSpace.Core/Services/Workflows/Nodes/Builtin/GitFetchPrChecksNode.cs @@ -17,7 +17,9 @@ namespace CodeSpace.Core.Services.Workflows.Nodes.Builtin; /// state == "success") into an If/else so a workflow only merges / proceeds once CI is green. /// Read-only (not side-effecting). A PR with NO checks reports state = "success" / /// allPassed = true (vacuously — nothing is pending or failing), mirroring how providers treat a -/// PR with no required checks as mergeable. +/// PR with no required checks as mergeable. "No checks" is only ever the provider's own answer: a checks +/// list the provider could not read (rate limit, outage, refused token, dropped connection) throws out of +/// the read and fails this node, so the gate never branches on a vacuous allPassed. /// public sealed class GitFetchPrChecksNode : INodeRuntime { diff --git a/backend/tests/CodeSpace.IntegrationTests/Workflows/PullRequestChecksGateFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Workflows/PullRequestChecksGateFlowTests.cs new file mode 100644 index 000000000..57ddbb4fc --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Workflows/PullRequestChecksGateFlowTests.cs @@ -0,0 +1,252 @@ +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.Engine; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.IntegrationTests.Workflows.Infrastructure; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Commands.Workflows; +using CodeSpace.Messages.Credentials; +using CodeSpace.Messages.Dtos.Workflows; +using CodeSpace.Messages.Enums; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Shouldly; +using static CodeSpace.IntegrationTests.Webhooks.StubProviderHost; + +namespace CodeSpace.IntegrationTests.Workflows; + +/// +/// The CI gate git.fetch_pr_checks advertises, built the way its manifest says to build it and run by the real +/// engine against a loopback GitLab: trigger → git.fetch_pr_checks → logic.if on allPassed → +/// git.merge_pr. Everything between the workflow definition and the wire is production code — the engine, the +/// node, PullRequestService over Postgres, the container's provider and resilience wrapper, NGitLab. +/// +/// A checks read GitLab does not answer has to stop the run before the merge. Read as "no checks", it reaches +/// the merge with allPassed = true and merges a merge request whose CI nobody saw. An empty pipeline list the +/// credential cannot vouch for is such a read. And only a pipeline GitLab calls success opens the gate: one blocked on a +/// manual job, waiting on a delayed one or skipped is held like a failed one. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public class PullRequestChecksGateFlowTests +{ + private const string PipelinesPath = "/api/v4/projects/4242/merge_requests/7/pipelines"; + private const string JobsPath = "/api/v4/projects/4242/pipelines/77/jobs"; + private const string MergePath = "/api/v4/projects/4242/merge_requests/7/merge"; + private const string ProjectPipelinesPath = "/api/v4/projects/4242/pipelines?"; + private const string MergeRequestPath = "/api/v4/projects/4242/merge_requests/7?"; + private const string ProjectPath = "/api/v4/projects/4242"; + + private readonly PostgresFixture _fixture; + + public PullRequestChecksGateFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + /// What the loopback GitLab answers about merge request !7's CI. + public enum CiAnswer + { + PipelinesRateLimited, + PipelinesUnavailable, + PipelinesForbidden, + PipelinesNotFound, + PipelinesConnectionDropped, + FailedPipelineJobsRateLimited, + PipelinesHiddenFromCredential, + ForkHeadPipelineHidden, + NoPipeline, + GreenPipeline, + FailedJob, + FailedPipelineGreenJobs, + BlockedPipeline, + DelayedPipeline, + SkippedPipeline, + NewestPipelineFailed + } + + [Theory] + [InlineData(CiAnswer.PipelinesRateLimited)] + [InlineData(CiAnswer.PipelinesUnavailable)] + [InlineData(CiAnswer.PipelinesForbidden)] + [InlineData(CiAnswer.PipelinesNotFound)] + [InlineData(CiAnswer.PipelinesConnectionDropped)] + [InlineData(CiAnswer.FailedPipelineJobsRateLimited)] + [InlineData(CiAnswer.PipelinesHiddenFromCredential)] + [InlineData(CiAnswer.ForkHeadPipelineHidden)] + public async Task A_checks_read_GitLab_does_not_answer_fails_the_run_before_the_merge(CiAnswer answer) + { + using var gitlab = GitLabAnswering(answer); + + var (runId, nodes) = await RunGateAsync(gitlab.BaseUrl); + + nodes["checks"].Status.ShouldBe(NodeStatus.Failure, $"GitLab never told the checks node what CI said ({answer}); error={nodes["checks"].Error}"); + nodes.Values.Where(n => n.Status == NodeStatus.Success).Select(n => n.NodeId).ShouldBe(new[] { "start" }, "an unread checks list must leave the gate nothing to branch on"); + MergesSent(gitlab).ShouldBe(0, "a merge request whose CI was never read must not be merged"); + (await LoadRunAsync(runId)).Status.ShouldBe(WorkflowRunStatus.Failure); + } + + [Theory] + [InlineData(CiAnswer.GreenPipeline, true)] + [InlineData(CiAnswer.NoPipeline, true)] + [InlineData(CiAnswer.FailedJob, false)] + [InlineData(CiAnswer.FailedPipelineGreenJobs, false)] + [InlineData(CiAnswer.BlockedPipeline, false)] + [InlineData(CiAnswer.DelayedPipeline, false)] + [InlineData(CiAnswer.SkippedPipeline, false)] + [InlineData(CiAnswer.NewestPipelineFailed, false)] + public async Task The_gate_merges_only_a_merge_request_whose_CI_passed(CiAnswer answer, bool merges) + { + using var gitlab = GitLabAnswering(answer); + + var (runId, nodes) = await RunGateAsync(gitlab.BaseUrl); + + nodes["checks"].Status.ShouldBe(NodeStatus.Success, $"error={nodes["checks"].Error}"); + nodes["merge"].Status.ShouldBe(merges ? NodeStatus.Success : NodeStatus.Skipped, $"{answer}: allPassed={JsonDocument.Parse(nodes["checks"].OutputsJson).RootElement.GetProperty("allPassed")}"); + MergesSent(gitlab).ShouldBe(merges ? 1 : 0); + (await LoadRunAsync(runId)).Status.ShouldBe(WorkflowRunStatus.Success); + } + + private static int MergesSent(StubProviderHost gitlab) => gitlab.Requests.Count(r => r.Method == "PUT" && r.PathAndQuery.Contains(MergePath, StringComparison.Ordinal)); + + private static StubProviderHost GitLabAnswering(CiAnswer answer) + { + var gitlab = new StubProviderHost().Answer("PUT", MergePath, 200, MergedMergeRequestJson); + + return answer switch + { + CiAnswer.PipelinesRateLimited => gitlab.Answer("GET", PipelinesPath, 429, """{"message":"429 Too Many Requests"}"""), + CiAnswer.PipelinesUnavailable => gitlab.Answer("GET", PipelinesPath, 503, """{"message":"503 Service Unavailable"}"""), + CiAnswer.PipelinesForbidden => gitlab.Answer("GET", PipelinesPath, 403, """{"message":"403 Forbidden"}"""), + CiAnswer.PipelinesNotFound => gitlab.Answer("GET", PipelinesPath, 404, """{"message":"404 Not Found"}"""), + CiAnswer.PipelinesConnectionDropped => gitlab.Answer("GET", PipelinesPath, _ => StubReply.DropConnection), + CiAnswer.FailedPipelineJobsRateLimited => gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("failed")).Answer("GET", JobsPath, 429, """{"message":"429 Too Many Requests"}"""), + CiAnswer.PipelinesHiddenFromCredential => NoPipelineListed(gitlab, projectPipelinesStatus: 403), + CiAnswer.ForkHeadPipelineHidden => NoPipelineListed(gitlab, headPipelineStatus: "failed"), + CiAnswer.NoPipeline => NoPipelineListed(gitlab), + CiAnswer.GreenPipeline => gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("success")).Answer("GET", JobsPath, 200, JobsJson("success", "success")), + CiAnswer.FailedJob => gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("failed")).Answer("GET", JobsPath, 200, JobsJson("success", "failed")), + CiAnswer.FailedPipelineGreenJobs => gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("failed")).Answer("GET", JobsPath, 200, JobsJson("success", "success")), + CiAnswer.BlockedPipeline => gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("manual")).Answer("GET", JobsPath, 200, JobsJson("success", "manual")), + CiAnswer.DelayedPipeline => gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("scheduled")).Answer("GET", JobsPath, 200, JobsJson("success", "scheduled")), + CiAnswer.SkippedPipeline => gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("skipped")).Answer("GET", JobsPath, 200, JobsJson("skipped")), + CiAnswer.NewestPipelineFailed => gitlab.Answer("GET", PipelinesPath, 200, $"[{PipelineJson(78, "failed")},{PipelineJson(77, "success")}]").Answer("GET", "/api/v4/projects/4242/pipelines/78/jobs", 200, JobsJson("success", "failed")).Answer("GET", JobsPath, 200, JobsJson("success", "success")), + _ => throw new ArgumentOutOfRangeException(nameof(answer), answer, null) + }; + } + + /// + /// The merge request lists no pipeline, and GitLab answers what an empty list is checked against: the project's CI + /// setting, its own pipeline list (403 when the credential may not read pipelines) and the merge request's head + /// pipeline. The project route answers only its exact path — it is a prefix of every other route here. + /// + private static StubProviderHost NoPipelineListed(StubProviderHost gitlab, int projectPipelinesStatus = 200, string? headPipelineStatus = null) => + gitlab.Answer("GET", PipelinesPath, 200, "[]") + .Answer("GET", ProjectPipelinesPath, projectPipelinesStatus, projectPipelinesStatus == 200 ? "[]" : """{"message":"403 Forbidden"}""") + .Answer("GET", MergeRequestPath, 200, OpenMergeRequestJson(headPipelineStatus)) + .Answer("GET", ProjectPath, request => request.PathAndQuery == ProjectPath ? new StubReply(200, ProjectJson) : new StubReply(501, """{"message":"no stub configured for this route"}""")); + + private async Task<(Guid RunId, Dictionary Nodes)> RunGateAsync(string gitlabBaseUrl) + { + var (teamId, userId) = await WorkflowsTestSeed.SeedTeamAsync(_fixture); + var repositoryId = await SeedGitLabRepositoryAsync(teamId, gitlabBaseUrl); + var workflowId = await CreateGateWorkflowAsync(teamId, userId); + var runId = await WorkflowsTestSeed.SeedManualRunAsync(_fixture, workflowId, teamId, payloadJson: JsonSerializer.Serialize(new { repositoryId, number = 7 })); + + using (var scope = _fixture.BeginScope()) + await scope.Resolve().ExecuteRunAsync(runId, CancellationToken.None); + + using var verify = _fixture.BeginScope(); + var nodes = await verify.Resolve().WorkflowRunNode.AsNoTracking().Where(n => n.RunId == runId).ToDictionaryAsync(n => n.NodeId); + + return (runId, nodes); + } + + private async Task CreateGateWorkflowAsync(Guid teamId, Guid userId) + { + const string pullRequest = """{"repositoryId":"{{trigger.repositoryId}}","number":"{{trigger.number}}"}"""; + + var definition = new WorkflowDefinition + { + SchemaVersion = 1, + Nodes = new List + { + new() { Id = "start", TypeKey = "trigger.pr.opened", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "checks", TypeKey = "git.fetch_pr_checks", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.Json(pullRequest) }, + new() { Id = "gate", TypeKey = "logic.if", Config = WorkflowsTestSeed.Json("""{"condition":"{{nodes.checks.outputs.allPassed}} == true"}"""), Inputs = WorkflowsTestSeed.EmptyJson() }, + new() { Id = "merge", TypeKey = "git.merge_pr", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.Json(pullRequest) }, + new() { Id = "hold", TypeKey = "builtin.terminal", Config = WorkflowsTestSeed.EmptyJson(), Inputs = WorkflowsTestSeed.EmptyJson() } + }, + Edges = new List + { + new() { From = "start", To = "checks" }, + new() { From = "checks", To = "gate" }, + new() { From = "gate", To = "merge", SourceHandle = "true" }, + new() { From = "gate", To = "hold", SourceHandle = "false" } + } + }; + + using var scope = _fixture.BeginScopeAs(userId, teamId, Roles.Admin); + + return await scope.Resolve().Send(new CreateWorkflowCommand { Name = "ci-gate-" + Guid.NewGuid().ToString("N")[..8], Definition = definition, Activations = new List(), Enabled = true }); + } + + private async Task SeedGitLabRepositoryAsync(Guid teamId, string baseUrl) + { + using var scope = _fixture.BeginScope(); + var db = scope.Resolve(); + var payload = scope.Resolve().Serialize(new PatPayload { Token = "glpat-loopback" }); + + var instance = new ProviderInstance { Id = Guid.NewGuid(), TeamId = teamId, Provider = ProviderKind.GitLab, DisplayName = "loopback", BaseUrl = baseUrl, ApiUrl = baseUrl }; + var credential = new Credential { Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instance.Id, Ownership = CredentialOwnership.TeamService, AuthType = AuthType.Pat, DisplayName = "connection", EncryptedPayload = scope.Resolve().Encrypt(payload), Status = CredentialStatus.Active }; + var repository = new Repository + { + Id = Guid.NewGuid(), TeamId = teamId, ProviderInstanceId = instance.Id, CredentialId = credential.Id, ExternalId = "4242", NamespacePath = "acme", Name = "api", FullPath = "acme/api", + DefaultBranch = "main", Visibility = RepositoryVisibility.Private, WebUrl = "https://gitlab.test/acme/api", Status = RepositoryStatus.Active + }; + + db.ProviderInstance.Add(instance); + db.Credential.Add(credential); + db.Repository.Add(repository); + + await db.SaveChangesAsync(); + + return repository.Id; + } + + private async Task LoadRunAsync(Guid runId) + { + using var scope = _fixture.BeginScope(); + return await scope.Resolve().WorkflowRun.AsNoTracking().SingleAsync(r => r.Id == runId); + } + + private static string PipelinesJson(string status) => $"[{PipelineJson(77, status)}]"; + + private static string PipelineJson(long id, string status) => + $$"""{"id":{{id}},"iid":3,"project_id":4242,"status":"{{status}}","ref":"feature/ci","sha":"0123456789abcdef0123456789abcdef01234567","web_url":"https://gitlab.test/acme/api/-/pipelines/{{id}}"}"""; + + private static string OpenMergeRequestJson(string? headPipelineStatus) => + $$"""{"id":7007,"iid":7,"project_id":4242,"title":"Gate on CI","state":"opened","source_branch":"feature/ci","target_branch":"main","sha":"0123456789abcdef0123456789abcdef01234567","head_pipeline":{{(headPipelineStatus == null ? "null" : PipelineJson(91, headPipelineStatus))}},"web_url":"https://gitlab.test/acme/api/-/merge_requests/7"}"""; + + private const string ProjectJson = """{"id":4242,"name":"api","path":"api","path_with_namespace":"acme/api","default_branch":"main","builds_access_level":"enabled","web_url":"https://gitlab.test/acme/api"}"""; + + private static string JobsJson(params string[] statuses) => + "[" + string.Join(",", statuses.Select((status, i) => $$"""{"id":{{i + 1}},"name":"job-{{i + 1}}","stage":"test","status":"{{status}}","web_url":"https://gitlab.test/acme/api/-/jobs/{{i + 1}}"}""")) + "]"; + + private static readonly string MergedMergeRequestJson = JsonSerializer.Serialize(new + { + id = 7007, + iid = 7, + project_id = 4242, + title = "Gate on CI", + state = "merged", + merge_commit_sha = "9f8e7d6c5b4a", + source_branch = "feature/ci", + target_branch = "main", + author = new { id = 1, username = "codespace-bot", name = "CodeSpace" }, + created_at = "2026-09-24T08:00:00.000Z", + updated_at = "2026-09-24T08:00:00.000Z", + web_url = "https://gitlab.test/acme/api/-/merge_requests/7" + }); +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubListChecksTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubListChecksTests.cs new file mode 100644 index 000000000..328b75e40 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/GitHub/GitHubListChecksTests.cs @@ -0,0 +1,120 @@ +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.Resilience; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using static CodeSpace.IntegrationTests.Webhooks.StubProviderHost; + +namespace CodeSpace.UnitTests.Providers.GitHub; + +/// +/// The real reading a pull request's check runs — Octokit, the wire, the resilience +/// wrapper — against a loopback GitHub. Workflows gate merges on this read, so a read that fails has to fail. An empty +/// list means only that GitHub answered "no check runs on the head commit". A refused token, a rate limit or a dropped +/// connection is not an empty list, and neither is a pull request whose head commit GitHub did not report. +/// +[Trait("Category", "Unit")] +public sealed class GitHubListChecksTests : IDisposable +{ + private const string PullPath = "/repos/acme/api/pulls/7"; + private const string CheckRunsPath = "/repos/acme/api/commits/0a1b2c3d4e5f/check-runs"; + + private readonly StubProviderHost _github = new(); + + public void Dispose() => _github.Dispose(); + + [Theory] + [InlineData(401, 1)] + [InlineData(403, 1)] + [InlineData(404, 1)] + [InlineData(429, 1)] + [InlineData(503, ExternalCallResilience.MaxAttempts)] + public async Task A_check_runs_read_GitHub_refuses_fails_with_its_status(int status, int expectedAttempts) + { + _github.Answer("GET", PullPath, 200, PullRequestJson(head: new { @ref = "feature/ci", sha = "0a1b2c3d4e5f" })).Answer("GET", CheckRunsPath, status, $$"""{"message":"{{status}}"}"""); + + var thrown = await Should.ThrowAsync(ListChecksAsync); + + thrown.StatusCode.ShouldBe(status); + _github.Sent("GET", CheckRunsPath).ShouldBe(expectedAttempts, "a 5xx is retried; any other refusal is the answer"); + } + + [Fact] + public async Task A_dropped_connection_is_retried_then_fails_the_read() + { + _github.Answer("GET", PullPath, 200, PullRequestJson(head: new { @ref = "feature/ci", sha = "0a1b2c3d4e5f" })).Answer("GET", CheckRunsPath, _ => StubReply.DropConnection); + + await Should.ThrowAsync(ListChecksAsync); + + _github.Sent("GET", CheckRunsPath).ShouldBe(ExternalCallResilience.MaxAttempts, "a dropped connection is transient — retried, then the read fails rather than reading as no checks"); + } + + [Fact] + public async Task A_pull_request_without_a_head_commit_fails_the_read() + { + _github.Answer("GET", PullPath, 200, PullRequestJson(head: null)); + + await Should.ThrowAsync(ListChecksAsync); + + _github.Sent("GET", "/check-runs").ShouldBe(0); + } + + [Fact] + public async Task A_head_commit_with_no_check_runs_has_no_checks() + { + _github.Answer("GET", PullPath, 200, PullRequestJson(head: new { @ref = "feature/ci", sha = "0a1b2c3d4e5f" })).Answer("GET", CheckRunsPath, 200, """{"total_count":0,"check_runs":[]}"""); + + var checks = await ListChecksAsync(); + + checks.ShouldBeEmpty("GitHub answered that no check ran on the head commit — the one empty list this read may return"); + } + + private Task> ListChecksAsync() => Provider().ListChecksAsync(Context(), Repository, 7, CancellationToken.None); + + private static string PullRequestJson(object? head) => JsonSerializer.Serialize(new + { + id = 7007, + number = 7, + title = "Gate on CI", + state = "open", + head, + @base = new { @ref = "main", sha = "4e5f6a7b8c9d" }, + user = new { login = "codespace-bot" }, + html_url = "https://github.test/acme/api/pull/7" + }); + + private static readonly RemoteRepository Repository = new() + { + ExternalId = "4242", + NamespacePath = "acme", + Name = "api", + FullPath = "acme/api", + DefaultBranch = "main", + Visibility = RepositoryVisibility.Private, + WebUrl = "https://github.test/acme/api" + }; + + private ProviderContext Context() => new(new ProviderInstance { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = ProviderKind.GitHub, DisplayName = "loopback", BaseUrl = _github.BaseUrl, ApiUrl = _github.BaseUrl }, new Credential { Id = Guid.NewGuid(), AuthType = AuthType.Pat, DisplayName = "pat", EncryptedPayload = "unused" }); + + private static GitHubRepositoryProvider Provider() + { + var resilience = new ExternalCallResilience(new ProviderErrorMapperRegistry(new IProviderErrorMapper[] { new GitHubErrorMapper() }), NullLogger.Instance); + var normalizer = new GitHubEventNormalizer(new ProviderEventSubscriptionRegistry(Array.Empty())); + + return new GitHubRepositoryProvider(new StaticTokenAuth(), resilience, new GitHubSignatureVerifier(), normalizer, new GitHubWebhookRepositoryIdentifier()); + } + + private sealed class StaticTokenAuth : IProviderAuthResolver + { + public Task ResolveAsync(ProviderContext context, CancellationToken cancellationToken) => Task.FromResult(new ResolvedAuth { Token = "ghp_loopback" }); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabListChecksTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabListChecksTests.cs new file mode 100644 index 000000000..4ded508fb --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabListChecksTests.cs @@ -0,0 +1,277 @@ +using System.Diagnostics; +using System.Net; +using System.Net.Sockets; +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.GitLab; +using CodeSpace.Core.Services.Providers.Resilience; +using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.IntegrationTests.Webhooks; +using CodeSpace.Messages.Dtos.Providers; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using static CodeSpace.IntegrationTests.Webhooks.StubProviderHost; + +namespace CodeSpace.UnitTests.Providers.GitLab; + +/// +/// The real reading a merge request's CI — NGitLab, the wire, the resilience +/// wrapper — against a loopback GitLab. Workflows gate merges on this read, so a read that fails has to fail. A rate +/// limit, an outage, a refused token, a dropped connection or a payload NGitLab cannot parse is not an empty list. GitLab +/// answers the merge request's pipeline list with [] both when no pipeline ran and when this credential may read none of +/// them, so an empty list stands only once GitLab confirms it. The latest pipeline's own status is a floor under its jobs, +/// and only a pipeline GitLab calls success passes: one that failed, is blocked on a manual job, is waiting on a delayed +/// job or was skipped never reads green because its job list looks clean. +/// +[Trait("Category", "Unit")] +public sealed class GitLabListChecksTests : IDisposable +{ + private const string PipelinesPath = "/api/v4/projects/4242/merge_requests/7/pipelines"; + private const string JobsPath = "/api/v4/projects/4242/pipelines/77/jobs"; + private const string ProjectPipelinesPath = "/api/v4/projects/4242/pipelines?"; + private const string MergeRequestPath = "/api/v4/projects/4242/merge_requests/7?"; + private const string ProjectPath = "/api/v4/projects/4242"; + + private readonly StubProviderHost _gitlab = new(); + + public void Dispose() => _gitlab.Dispose(); + + [Theory] + [InlineData(429, 1)] + [InlineData(503, ExternalCallResilience.MaxAttempts)] + [InlineData(403, 1)] + [InlineData(404, 1)] + public async Task A_pipelines_read_GitLab_refuses_fails_with_its_status(int status, int expectedAttempts) + { + _gitlab.Answer("GET", PipelinesPath, status, $$"""{"message":"{{status}}"}"""); + + var thrown = await Should.ThrowAsync(ListChecksAsync); + + thrown.StatusCode.ShouldBe(status); + _gitlab.Sent("GET", PipelinesPath).ShouldBe(expectedAttempts, "a 5xx is retried; any other refusal is the answer"); + } + + [Fact] + public async Task A_dropped_connection_is_retried_then_fails_the_read() + { + // NGitLab reads the answer through HttpWebRequest: a body cut short surfaces as an HttpIOException, not the + // HttpRequestException an HttpClient SDK throws. It is the same network blip, so it gets the same retries. + _gitlab.Answer("GET", PipelinesPath, _ => StubReply.DropConnection); + + await Should.ThrowAsync(ListChecksAsync); + + _gitlab.Sent("GET", PipelinesPath).ShouldBe(ExternalCallResilience.MaxAttempts, "a dropped connection is transient — one TCP reset must not fail the gate run"); + } + + [Fact] + public async Task A_refused_connection_is_retried_then_fails_the_read() + { + // Nothing listens on the port, so no attempt can be counted on the wire — the backoff between attempts is the + // evidence they happened. NGitLab reports a connection it never got an answer on as a WebException without one. + var stopwatch = Stopwatch.StartNew(); + + await Should.ThrowAsync(() => Provider().ListChecksAsync(Context($"http://127.0.0.1:{UnusedLoopbackPort()}"), Repository, 7, CancellationToken.None)); + + var retriedBackoff = ExternalCallResilience.ComputeBackoff(1) + ExternalCallResilience.ComputeBackoff(2); + stopwatch.Elapsed.ShouldBeGreaterThanOrEqualTo(retriedBackoff, $"a refused connection must be tried {ExternalCallResilience.MaxAttempts} times, with {retriedBackoff.TotalMilliseconds}ms of backoff between the attempts — check ExternalCallResilience.IsTransient for WebException"); + } + + [Fact] + public async Task A_failed_pipeline_whose_jobs_cannot_be_read_fails_the_read() + { + _gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("failed")).Answer("GET", JobsPath, 429, """{"message":"429 Too Many Requests"}"""); + + var thrown = await Should.ThrowAsync(ListChecksAsync); + + thrown.StatusCode.ShouldBe(429); + } + + [Fact] + public async Task A_status_NGitLab_cannot_parse_fails_the_read() + { + // GitLab added waiting_for_callback after NGitLab 11.7's JobStatus; the parse throws rather than guess. + _gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson("waiting_for_callback")); + + await Should.ThrowAsync(ListChecksAsync); + } + + [Fact] + public async Task The_checks_come_from_the_newest_pipeline() + { + // GitLab lists a merge request's pipelines newest-first. The older one went green; the push after it went red. + _gitlab.Answer("GET", PipelinesPath, 200, $"[{PipelineJson(78, "failed")},{PipelineJson(77, "success")}]") + .Answer("GET", "/api/v4/projects/4242/pipelines/78/jobs", 200, JobsJson(new[] { "success", "failed" })) + .Answer("GET", JobsPath, 200, JobsJson(new[] { "success", "success" })); + + var checks = await ListChecksAsync(); + + GitFetchPrChecksNode.SummarizeChecks(checks).State.ShouldBe("failure", "an older green pipeline must not stand in for the newest red one"); + _gitlab.Sent("GET", "/api/v4/projects/4242/pipelines/78/jobs").ShouldBe(1); + _gitlab.Sent("GET", JobsPath).ShouldBe(0); + } + + [Fact] + public async Task A_merge_request_with_no_pipeline_has_no_checks() + { + _gitlab.Answer("GET", PipelinesPath, 200, "[]"); + AnswerNoPipeline(); + + var checks = await ListChecksAsync(); + + checks.ShouldBeEmpty("GitLab answered that no pipeline ran, and the credential may read the project's pipelines — the one empty list this read may return"); + _gitlab.Sent("GET", ProjectPipelinesPath).ShouldBe(1); + _gitlab.Sent("GET", MergeRequestPath).ShouldBe(1); + } + + [Fact] + public async Task A_project_with_CI_turned_off_has_no_checks() + { + // No pipeline can run, and GitLab refuses every pipeline read on such a project — so it is not asked. + _gitlab.Answer("GET", PipelinesPath, 200, "[]"); + AnswerNoPipeline(buildsAccessLevel: "disabled", projectPipelinesStatus: 403); + + var checks = await ListChecksAsync(); + + checks.ShouldBeEmpty(); + _gitlab.Sent("GET", ProjectPipelinesPath).ShouldBe(0); + } + + [Fact] + public async Task An_empty_pipeline_list_from_a_credential_that_may_not_read_pipelines_fails_the_read() + { + // CI is visible to project members only and the connection's identity is not one: the merge request's list hides + // every pipeline behind 200 [], and the project's list refuses the same credential with 403. + _gitlab.Answer("GET", PipelinesPath, 200, "[]"); + AnswerNoPipeline(projectPipelinesStatus: 403); + + var thrown = await Should.ThrowAsync(ListChecksAsync); + + thrown.StatusCode.ShouldBe(403, "a credential that cannot see the pipelines cannot vouch that none ran"); + } + + [Fact] + public async Task An_empty_pipeline_list_beside_a_head_pipeline_fails_the_read() + { + // A merge request from a fork whose pipelines this credential may not read: its list shows only the target + // project's pipelines (none), while the merge request still names the fork's red head pipeline. + _gitlab.Answer("GET", PipelinesPath, 200, "[]"); + AnswerNoPipeline(headPipelineStatus: "failed"); + + var thrown = await Should.ThrowAsync(ListChecksAsync); + + thrown.Message.ShouldContain("head pipeline"); + } + + [Theory] + [InlineData("failed", "failed", "Failure")] + [InlineData("success", "success", "Success")] + [InlineData("running", "success,running", "Pending,Success")] + [InlineData("running", "success", "Success,pipeline=Pending")] + [InlineData("failed", "success,success", "Success,Success,pipeline=Failure")] + [InlineData("canceled", "success,skipped", "Skipped,Success,pipeline=Cancelled")] + [InlineData("manual", "success,manual", "Skipped,Success,pipeline=Pending")] // blocked: a manual job holds the pipeline + [InlineData("scheduled", "success,scheduled", "Skipped,Success,pipeline=Pending")] // a delayed job has not run yet + [InlineData("skipped", "skipped", "Skipped,pipeline=Cancelled")] // nothing ran; GitLab's merge check does not count it as success + public async Task The_pipeline_status_is_a_floor_under_its_jobs(string pipelineStatus, string jobStatuses, string expectedStatuses) + { + _gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson(pipelineStatus)).Answer("GET", JobsPath, 200, JobsJson(jobStatuses.Split(','))); + + var checks = await ListChecksAsync(); + + var statuses = checks.Select(c => c.Name == "pipeline" ? $"pipeline={c.Status}" : c.Status.ToString()).Order(StringComparer.Ordinal); + + string.Join(",", statuses).ShouldBe(expectedStatuses, "the pipeline joins its jobs only when no job already carries its verdict"); + checks.Where(c => c.Name == "pipeline").ShouldAllBe(c => c.DetailsUrl == "https://gitlab.test/acme/api/-/pipelines/77"); + } + + [Theory] + [InlineData("success", true)] + [InlineData("failed", false)] + [InlineData("canceled", false)] + [InlineData("canceling", false)] + [InlineData("skipped", false)] + [InlineData("created", false)] + [InlineData("waiting_for_resource", false)] + [InlineData("preparing", false)] + [InlineData("pending", false)] + [InlineData("running", false)] + [InlineData("manual", false)] + [InlineData("scheduled", false)] + public async Task Only_a_pipeline_GitLab_calls_success_passes_the_gate(string pipelineStatus, bool passes) + { + _gitlab.Answer("GET", PipelinesPath, 200, PipelinesJson(pipelineStatus)).Answer("GET", JobsPath, 200, JobsJson(new[] { "success" })); + + var checks = await ListChecksAsync(); + + GitFetchPrChecksNode.SummarizeChecks(checks).AllPassed.ShouldBe(passes, $"a pipeline GitLab calls {pipelineStatus} over one green job"); + } + + private Task> ListChecksAsync() => Provider().ListChecksAsync(Context(_gitlab.BaseUrl), Repository, 7, CancellationToken.None); + + /// + /// What GitLab says once the merge request lists no pipeline: the project's CI setting, the project's own pipeline list + /// (403 when the credential may not read pipelines) and the merge request's head pipeline. Registered after every + /// narrower route — the project's path is a prefix of all of them — and answering only its exact path, so a route the + /// test did not stub still reads as 501. + /// + private void AnswerNoPipeline(string buildsAccessLevel = "enabled", int projectPipelinesStatus = 200, string? headPipelineStatus = null) + { + _gitlab.Answer("GET", ProjectPipelinesPath, projectPipelinesStatus, projectPipelinesStatus == 200 ? "[]" : $$"""{"message":"{{projectPipelinesStatus}} Forbidden"}""") + .Answer("GET", MergeRequestPath, 200, MergeRequestJson(headPipelineStatus)) + .Answer("GET", ProjectPath, request => request.PathAndQuery == ProjectPath ? new StubReply(200, ProjectJson(buildsAccessLevel)) : new StubReply(501, """{"message":"no stub configured for this route"}""")); + } + + private static string PipelinesJson(string status) => $"[{PipelineJson(77, status)}]"; + + private static string PipelineJson(long id, string status) => + $$"""{"id":{{id}},"iid":3,"project_id":4242,"status":"{{status}}","ref":"feature/ci","sha":"0123456789abcdef0123456789abcdef01234567","web_url":"https://gitlab.test/acme/api/-/pipelines/{{id}}"}"""; + + private static string JobsJson(IEnumerable statuses) => + "[" + string.Join(",", statuses.Select((status, i) => $$"""{"id":{{i + 1}},"name":"job-{{i + 1}}","stage":"test","status":"{{status}}","web_url":"https://gitlab.test/acme/api/-/jobs/{{i + 1}}"}""")) + "]"; + + private static string MergeRequestJson(string? headPipelineStatus) => + $$"""{"id":7007,"iid":7,"project_id":4242,"title":"Gate on CI","state":"opened","source_branch":"feature/ci","target_branch":"main","sha":"0123456789abcdef0123456789abcdef01234567","head_pipeline":{{(headPipelineStatus == null ? "null" : PipelineJson(91, headPipelineStatus))}},"web_url":"https://gitlab.test/acme/api/-/merge_requests/7"}"""; + + private static string ProjectJson(string buildsAccessLevel) => + $$"""{"id":4242,"name":"api","path":"api","path_with_namespace":"acme/api","default_branch":"main","builds_access_level":"{{buildsAccessLevel}}","web_url":"https://gitlab.test/acme/api"}"""; + + private static int UnusedLoopbackPort() + { + using var probe = new TcpListener(IPAddress.Loopback, 0); + probe.Start(); + var port = ((IPEndPoint)probe.LocalEndpoint).Port; + probe.Stop(); + return port; + } + + private static readonly RemoteRepository Repository = new() + { + ExternalId = "4242", + NamespacePath = "acme", + Name = "api", + FullPath = "acme/api", + DefaultBranch = "main", + Visibility = RepositoryVisibility.Private, + WebUrl = "https://gitlab.test/acme/api" + }; + + private static ProviderContext Context(string baseUrl) => new(new ProviderInstance { Id = Guid.NewGuid(), TeamId = Guid.NewGuid(), Provider = ProviderKind.GitLab, DisplayName = "loopback", BaseUrl = baseUrl }, new Credential { Id = Guid.NewGuid(), AuthType = AuthType.Pat, DisplayName = "pat", EncryptedPayload = "unused" }); + + private static GitLabRepositoryProvider Provider() + { + var resilience = new ExternalCallResilience(new ProviderErrorMapperRegistry(new IProviderErrorMapper[] { new GitLabErrorMapper() }), NullLogger.Instance); + var normalizer = new GitLabEventNormalizer(new ProviderEventSubscriptionRegistry(Array.Empty())); + + return new GitLabRepositoryProvider(new StaticTokenAuth(), resilience, new GitLabSignatureVerifier(), normalizer, new GitLabWebhookRepositoryIdentifier()); + } + + private sealed class StaticTokenAuth : IProviderAuthResolver + { + public Task ResolveAsync(ProviderContext context, CancellationToken cancellationToken) => Task.FromResult(new ResolvedAuth { Token = "glpat-loopback" }); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabWriteRetryTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabWriteRetryTests.cs index c73c670e7..7d4662a40 100644 --- a/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabWriteRetryTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Providers/GitLab/GitLabWriteRetryTests.cs @@ -19,10 +19,10 @@ namespace CodeSpace.UnitTests.Providers.GitLab; /// /// The real — NGitLab, the wire, the resilience wrapper — against a loopback -/// GitLab that applies each write before it answers. A gateway 502 after GitLab committed must not make the retry -/// apply the write again. (NGitLab surfaces a dropped connection as a WebException, which the wrapper never -/// retries, so the 5xx is the ambiguous failure that reaches a retry here. The two hook creates are the exception: -/// they post raw HTTP, where a dropped connection or a timeout is the retried failure and a 5xx answer is not.) +/// GitLab that applies each write before it answers. A gateway 502 or a dropped connection after GitLab committed must +/// not make the retry apply the write again. (NGitLab surfaces a dropped connection as an HttpIOException, or a +/// WebException when no answer arrived; the wrapper retries both, as it retries an HttpClient SDK's +/// HttpRequestException. The two hook creates post raw HTTP, where a 5xx answer is a refusal, not a retry.) /// [Trait("Category", "Unit")] public sealed class GitLabWriteRetryTests : IDisposable @@ -33,6 +33,7 @@ public sealed class GitLabWriteRetryTests : IDisposable [Theory] [InlineData(WriteScenario.LandsThenGatewayError, 1)] + [InlineData(WriteScenario.LandsThenConnectionDrops, 1)] [InlineData(WriteScenario.RefusedBeforeLanding, 2)] [InlineData(WriteScenario.RefusedThenLandsThenGatewayError, 2)] public async Task PostComment_lands_exactly_once(WriteScenario scenario, int expectedCreates) @@ -51,6 +52,7 @@ public async Task PostComment_lands_exactly_once(WriteScenario scenario, int exp [Theory] [InlineData(WriteScenario.LandsThenGatewayError, 1)] + [InlineData(WriteScenario.LandsThenConnectionDrops, 1)] [InlineData(WriteScenario.RefusedBeforeLanding, 2)] [InlineData(WriteScenario.RefusedThenLandsThenGatewayError, 2)] public async Task CommentIssue_lands_exactly_once(WriteScenario scenario, int expectedCreates) @@ -69,6 +71,7 @@ public async Task CommentIssue_lands_exactly_once(WriteScenario scenario, int ex [Theory] [InlineData(WriteScenario.LandsThenGatewayError, 1)] + [InlineData(WriteScenario.LandsThenConnectionDrops, 1)] [InlineData(WriteScenario.RefusedBeforeLanding, 2)] [InlineData(WriteScenario.RefusedThenLandsThenGatewayError, 2)] public async Task CreateIssue_lands_exactly_once(WriteScenario scenario, int expectedCreates) @@ -87,6 +90,7 @@ public async Task CreateIssue_lands_exactly_once(WriteScenario scenario, int exp [Theory] [InlineData(WriteScenario.LandsThenGatewayError, 1)] + [InlineData(WriteScenario.LandsThenConnectionDrops, 1)] [InlineData(WriteScenario.RefusedBeforeLanding, 2)] [InlineData(WriteScenario.RefusedThenLandsThenGatewayError, 2)] public async Task OpenPullRequest_lands_exactly_once(WriteScenario scenario, int expectedCreates) @@ -107,6 +111,7 @@ public async Task OpenPullRequest_lands_exactly_once(WriteScenario scenario, int [Theory] [InlineData(WriteScenario.LandsThenGatewayError, 1)] + [InlineData(WriteScenario.LandsThenConnectionDrops, 1)] [InlineData(WriteScenario.RefusedBeforeLanding, 2)] [InlineData(WriteScenario.RefusedThenLandsThenGatewayError, 2)] public async Task Merge_merges_exactly_once(WriteScenario scenario, int expectedMerges) @@ -123,6 +128,7 @@ public async Task Merge_merges_exactly_once(WriteScenario scenario, int expected [Theory] [InlineData(WriteScenario.LandsThenGatewayError, 1)] + [InlineData(WriteScenario.LandsThenConnectionDrops, 1)] [InlineData(WriteScenario.RefusedBeforeLanding, 2)] [InlineData(WriteScenario.RefusedThenLandsThenGatewayError, 2)] public async Task SubmitReview_approves_exactly_once(WriteScenario scenario, int expectedApproves) @@ -214,15 +220,15 @@ public async Task A_5xx_answer_to_a_hook_create_that_landed_leaves_one_hook(stri [Theory] [InlineData(ProjectHooks, false, 2)] - [InlineData(ProjectHooks, true, 1)] + [InlineData(ProjectHooks, true, 2)] [InlineData(GroupHooks, false, 1)] [InlineData(GroupHooks, true, 2)] public async Task A_probe_that_cannot_read_the_hooks_never_sends_the_create_again(string hooksPath, bool probeConnectionDrops, int expectedProbes) { // The create landed and its answer was lost; then the probe cannot read the hooks either. The failed probe is - // asked again only where the wrapper counts its failure as transient — NGitLab's 5xx on the project list, a - // dropped connection (or a timeout) on the raw group list. NGitLab loses a connection as a WebException, and - // the group list reads a 5xx as a refusal; both fail the call at once. Never a second create. + // asked again only where the wrapper counts its failure as transient — a 5xx or a dropped connection on NGitLab's + // project list, a dropped connection (or a timeout) on the raw group list. The group list reads a 5xx as a + // refusal, which fails the call at once. Never a second create. var hooks = new ForgeCollection(WriteScenario.LandsThenConnectionDrops, HookJson); _gitlab.Answer("POST", hooksPath, hooks.Create).Answer("GET", hooksPath, _ => probeConnectionDrops ? StubReply.DropConnection : new StubReply(502, """{"message":"502 Bad Gateway"}""")); diff --git a/backend/tests/CodeSpace.UnitTests/Providers/Resilience/ExternalCallResilienceTests.cs b/backend/tests/CodeSpace.UnitTests/Providers/Resilience/ExternalCallResilienceTests.cs index 760458b45..7872938b9 100644 --- a/backend/tests/CodeSpace.UnitTests/Providers/Resilience/ExternalCallResilienceTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Providers/Resilience/ExternalCallResilienceTests.cs @@ -54,6 +54,18 @@ public void ComputeBackoff_doubles_per_attempt(int attempt, double expectedMs) [Fact] public void IsTransient_TaskCanceledException_is_transient() => ExternalCallResilience.IsTransient(new TaskCanceledException("timeout")).ShouldBeTrue(); + // NGitLab speaks HttpWebRequest, so its network blips are not HttpRequestException: a connection it never got an + // answer on is a WebException without a response, and an answer cut short mid-body is an HttpIOException. + + [Theory] + [InlineData(WebExceptionStatus.ConnectFailure)] + [InlineData(WebExceptionStatus.Timeout)] + [InlineData(WebExceptionStatus.ConnectionClosed)] + public void IsTransient_WebException_without_a_response_is_transient(WebExceptionStatus status) => ExternalCallResilience.IsTransient(new WebException("no answer", status)).ShouldBeTrue(); + + [Fact] + public void IsTransient_HttpIOException_is_transient() => ExternalCallResilience.IsTransient(new HttpIOException(HttpRequestError.ResponseEnded, "The response ended prematurely.")).ShouldBeTrue(); + [Theory] [InlineData(500)] [InlineData(502)] diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/GitFetchPrChecksNodeTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/GitFetchPrChecksNodeTests.cs index 29c5772eb..4b6afe55d 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/GitFetchPrChecksNodeTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/GitFetchPrChecksNodeTests.cs @@ -1,6 +1,12 @@ +using System.Text.Json; +using CodeSpace.Core.Services.PullRequests; +using CodeSpace.Core.Services.Workflows.Nodes; using CodeSpace.Core.Services.Workflows.Nodes.Builtin; +using CodeSpace.Core.Services.Workflows.Runtime; using CodeSpace.Messages.Dtos.Providers; using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Exceptions; +using Microsoft.Extensions.Logging.Abstractions; using Shouldly; namespace CodeSpace.UnitTests.Workflows; @@ -9,8 +15,9 @@ namespace CodeSpace.UnitTests.Workflows; /// The PURE gate rollup for git.fetch_pr_checks () — /// the logic a workflow gates merges on. Exhaustively pins the state machine: "pending" wins over "failure" /// wins over "success"; failure ∪ cancelled both block; skipped / neutral never block; an EMPTY set passes -/// vacuously (a PR with no required checks is mergeable). The real provider fetch is covered by the -/// PullRequest integration suite; here we pin the branch-ready derivation in isolation. +/// vacuously (a PR with no required checks is mergeable). An empty set is only ever the provider's answer — a checks +/// list the provider could not read fails the node, so the gate has no allPassed to branch on. The real provider +/// reads are pinned by the GitLab / GitHub ListChecks suites and the CI-gate flow test. /// [Trait("Category", "Unit")] public sealed class GitFetchPrChecksNodeTests @@ -104,4 +111,55 @@ public void Counts_are_accurate_across_a_mixed_set() s.State.ShouldBe("pending"); s.AllPassed.ShouldBeFalse(); } + + [Fact] + public async Task A_checks_list_the_provider_could_not_read_fails_the_node_instead_of_passing_the_gate() + { + var unreadable = new ProviderApiException(ProviderKind.GitLab, 429, "ListChecksAsync", "429 Too Many Requests", new HttpRequestException("rate limited")); + var node = new GitFetchPrChecksNode(new ChecksReadingPrService(() => throw unreadable)); + + var thrown = await Should.ThrowAsync(() => node.RunAsync(Context(), CancellationToken.None)); + + thrown.ShouldBeSameAs(unreadable, "the engine fails the node on the provider's own error — never an allPassed the gate could read as green"); + } + + [Fact] + public async Task A_checks_list_the_provider_read_is_summarised_into_the_gate_outputs() + { + var node = new GitFetchPrChecksNode(new ChecksReadingPrService(() => Checks(PullRequestCheckStatus.Success, PullRequestCheckStatus.Failure))); + + var result = await node.RunAsync(Context(), CancellationToken.None); + + result.Status.ShouldBe(NodeStatus.Success); + result.Outputs["state"].GetString().ShouldBe("failure"); + result.Outputs["allPassed"].GetBoolean().ShouldBeFalse(); + result.Outputs["checks"].GetArrayLength().ShouldBe(2); + } + + private static NodeRunContext Context() => new() + { + Inputs = new Dictionary { ["repositoryId"] = JsonSerializer.SerializeToElement(Guid.NewGuid()), ["number"] = JsonSerializer.SerializeToElement(7) }, + Config = new Dictionary(), + RawInputs = JsonDocument.Parse("{}").RootElement, + RawConfig = JsonDocument.Parse("{}").RootElement, + Scope = new NodeRunScope { Trigger = new Dictionary(), Sys = new Dictionary { [SystemScopeKeys.TeamId] = JsonSerializer.SerializeToElement(Guid.NewGuid()) } }, + Logger = NullLogger.Instance, + Observability = NodeObservability.NoOp, + }; + + /// Answers the checks read with read; every other member throws (this node only reads checks). + private sealed class ChecksReadingPrService(Func> read) : IPullRequestService + { + public Task> ListChecksAsync(Guid r, Guid t, int n, CancellationToken c) => Task.FromResult(read()); + + public Task> ListAsync(Guid r, Guid t, PullRequestState? s, int p, int pp, CancellationToken c) => throw new NotImplementedException(); + public Task GetAsync(Guid r, Guid t, int n, CancellationToken c) => throw new NotImplementedException(); + public Task> ListCommitsAsync(Guid r, Guid t, int n, CancellationToken c) => throw new NotImplementedException(); + public Task> ListFilesAsync(Guid r, Guid t, int n, CancellationToken c) => throw new NotImplementedException(); + public Task GetCountsAsync(Guid r, Guid t, CancellationToken c) => throw new NotImplementedException(); + public Task PostCommentAsync(Guid r, Guid t, int n, string b, CancellationToken c) => throw new NotImplementedException(); + public Task SubmitReviewAsync(Guid r, Guid t, int n, PullRequestReviewVerdict v, string? b, Guid? a, CancellationToken c) => throw new NotImplementedException(); + public Task OpenPullRequestAsync(Guid r, Guid t, OpenPullRequestInput i, Guid? a, CancellationToken c) => throw new NotImplementedException(); + public Task MergePullRequestAsync(Guid r, Guid t, int n, MergePullRequestInput i, Guid? a, CancellationToken c) => throw new NotImplementedException(); + } } diff --git a/frontend/src/api/repositories.ts b/frontend/src/api/repositories.ts index 7d3afc52a..77020352f 100644 --- a/frontend/src/api/repositories.ts +++ b/frontend/src/api/repositories.ts @@ -125,9 +125,9 @@ export const repositoriesApi = { fetchJson(`/api/repositories/${encodeURIComponent(repositoryId)}/pull-requests/${number}/files`), // CI / checks for the PR's HEAD commit. Normalised across GitHub Actions check_runs - // and GitLab pipeline jobs. Empty list when the provider has no checks configured - // or the token lacks the required scope — the backend swallows the error and - // returns empty rather than failing the whole PR detail view. + // and GitLab pipeline jobs. Empty list only when the provider reports no checks; a + // read the provider refuses (scope, rate limit, outage) is an error — workflows gate + // merges on this list. The PR detail view hides the checks card either way. listPullRequestChecks: (repositoryId: string, number: number) => fetchJson(`/api/repositories/${encodeURIComponent(repositoryId)}/pull-requests/${number}/checks`),