From 9c3a330ff5b2ce007366dd0656074bf700c5e334 Mon Sep 17 00:00:00 2001 From: "Mars.P" Date: Sun, 18 Oct 2026 12:35:43 +0800 Subject: [PATCH] Show build, role and credential plumbing in a health report An API pod and a worker pod run the same assembly from two images, and nothing let an operator see which build each one was on, whether the OAuth refresh lock was really exclusive on this deployment's Postgres path, whether the host git could carry a clone token, or whether the credential repair sweep was scheduled at all. Each of those failed quietly and read as a bad token or a flaky run. Add GET /health, a JSON report beside the probes. Anyone, signed in or not, reads the process's build and Hangfire role and each check's name and status, which is all it takes to compare an API image with a worker image. A signed-in global Admin also reads each check's description and data (git release, connection topology, repair backlog); on the internet-facing API those are reconnaissance, not for an anonymous caller. A check that threw is reported by the exception's type, never its message. /health/live and /health/ready keep their bodies and status codes; the new checks carry no "ready" tag and only ever report Degraded, so no orchestrator restarts or unroutes a pod over them. Nothing refuses to start: each finding is logged and reported instead. - refresh-lock: a self-test takes one advisory-lock key on a session and tries it from a second, several times, then reads the answer of the holder's own unlock. A second session that gets the key, or an unlock told it never held the key, means a transaction-mode pooler, RDS Proxy or a read-write split: an Error at boot and Degraded in the report. A pass is reported as "no sign of multiplexing", because a pooler that picks a server session per statement can slip past it. An unlock that missed is retried a few times so the key is not left on the pooler's server session. The finding is reused for five minutes so the endpoint cannot make the database work per request. - git: `git --version` is read once and logged in a boot posture line with the build and role. Below 2.31, the release that reads GIT_CONFIG_COUNT, the report is Degraded, and a tokened clone, push, ls-remote or pasted-credential pack clone that git refused for want of a credential now fails with a typed message naming the cause instead of "could not read Username". For workspace clones, pushes and ls-remote it is a WorkspaceException, so every existing catch site and failure code is unchanged; the pack clone stays a PackImportException carrying it. - credential-repair-sweep: whether Hangfire holds the sweep's entry, when it is next due and how its last run ended, and how many disconnected credentials it is waiting to move. The backlog is counted by the sweep's own candidate query, now shared with it. The scheduler's stored error is an exception chain that can name the storage host, so the finding says only that one was recorded; the full text is logged once per distinct error. --- backend/src/CodeSpace.Api/Startup.cs | 23 +- .../Services/Agents/PackCloneFetcher.cs | 11 +- ...GitCredentialHelperUnsupportedException.cs | 25 ++ .../Services/Agents/Workspace/HostGit.cs | 69 ++++ .../Integrators/LocalGitBranchIntegrator.cs | 4 +- .../Providers/LocalGitWorkspaceProvider.cs | 14 +- .../Agents/Workspace/RemoteTipResolver.cs | 2 +- .../Agents/Workspace/TokenedGitCommand.cs | 27 ++ .../Agents/Workspace/WorkspaceException.cs | 5 +- .../CredentialSuccessionService.cs | 36 +- .../Credentials/IStrandedCredentialCensus.cs | 16 + .../Services/Health/BootPostureReporter.cs | 64 +++ .../CredentialRepairSweepHealthCheck.cs | 175 +++++++++ .../Health/Checks/GitVersionHealthCheck.cs | 35 ++ .../Health/Checks/RefreshLockHealthCheck.cs | 30 ++ .../Services/Health/HealthResponseWriter.cs | 53 +++ .../Services/Health/HostPosture.cs | 42 ++ .../Services/Health/IAdvisoryLockSessions.cs | 29 ++ .../Services/Health/IRefreshLockSelfTest.cs | 10 + .../Services/Health/RefreshLockSelfTest.cs | 215 +++++++++++ .../Sessions/NpgsqlAdvisoryLockSessions.cs | 61 +++ .../Supervisor/SupervisorAcceptanceGrader.cs | 2 +- .../CodeSpace.Messages/Health/GitReading.cs | 4 + .../CodeSpace.Messages/Health/GitVersion.cs | 34 ++ .../Health/HealthResponse.cs | 20 + .../Health/RefreshLockVerdict.cs | 17 + .../RecurringJobWorkerHostFactory.cs | 7 +- .../Jobs/WorkerHealthReportE2ETests.cs | 129 +++++++ .../Tasks/HealthEndpointE2ETests.cs | 143 +++++++ .../Tasks/HealthReportDisclosureE2ETests.cs | 81 ++++ .../CredentialRepairSweepHealthFlowTests.cs | 124 ++++++ .../Health/RefreshLockSelfTestFlowTests.cs | 159 ++++++++ .../OAuth/ProviderReconnectFlowTests.cs | 36 ++ .../Agents/LocalGitBranchIntegratorTests.cs | 53 +++ .../Agents/PackCloneFetcherCredentialTests.cs | 29 ++ .../Agents/SupervisorAcceptanceGraderTests.cs | 18 + .../Health/BootPostureReporterTests.cs | 164 ++++++++ .../CredentialRepairSweepHealthCheckTests.cs | 290 ++++++++++++++ .../Health/GitVersionHealthCheckTests.cs | 69 ++++ .../Health/HealthResponseWriterTests.cs | 169 ++++++++ .../Health/HostPostureTests.cs | 48 +++ .../Health/NpgsqlAdvisoryLockSessionsTests.cs | 33 ++ .../Health/RefreshLockHealthCheckTests.cs | 61 +++ .../Health/RefreshLockSelfTestTests.cs | 365 ++++++++++++++++++ .../Workflows/GitVersionTests.cs | 69 ++++ .../LocalGitWorkspaceProviderTests.cs | 81 ++++ .../Workflows/RemoteTipResolverTests.cs | 27 ++ .../Workflows/TokenedGitOldGitFailureTests.cs | 75 ++++ 48 files changed, 3229 insertions(+), 24 deletions(-) create mode 100644 backend/src/CodeSpace.Core/Services/Agents/Workspace/GitCredentialHelperUnsupportedException.cs create mode 100644 backend/src/CodeSpace.Core/Services/Agents/Workspace/HostGit.cs create mode 100644 backend/src/CodeSpace.Core/Services/Credentials/IStrandedCredentialCensus.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/BootPostureReporter.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/Checks/CredentialRepairSweepHealthCheck.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/Checks/GitVersionHealthCheck.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/Checks/RefreshLockHealthCheck.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/HealthResponseWriter.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/HostPosture.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/IAdvisoryLockSessions.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/IRefreshLockSelfTest.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/RefreshLockSelfTest.cs create mode 100644 backend/src/CodeSpace.Core/Services/Health/Sessions/NpgsqlAdvisoryLockSessions.cs create mode 100644 backend/src/CodeSpace.Messages/Health/GitReading.cs create mode 100644 backend/src/CodeSpace.Messages/Health/GitVersion.cs create mode 100644 backend/src/CodeSpace.Messages/Health/HealthResponse.cs create mode 100644 backend/src/CodeSpace.Messages/Health/RefreshLockVerdict.cs create mode 100644 backend/tests/CodeSpace.E2ETests/Jobs/WorkerHealthReportE2ETests.cs create mode 100644 backend/tests/CodeSpace.E2ETests/Tasks/HealthReportDisclosureE2ETests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Health/CredentialRepairSweepHealthFlowTests.cs create mode 100644 backend/tests/CodeSpace.IntegrationTests/Health/RefreshLockSelfTestFlowTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/BootPostureReporterTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/CredentialRepairSweepHealthCheckTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/GitVersionHealthCheckTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/HealthResponseWriterTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/HostPostureTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/NpgsqlAdvisoryLockSessionsTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/RefreshLockHealthCheckTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Health/RefreshLockSelfTestTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/GitVersionTests.cs create mode 100644 backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitOldGitFailureTests.cs diff --git a/backend/src/CodeSpace.Api/Startup.cs b/backend/src/CodeSpace.Api/Startup.cs index 0584d26ad..27141be41 100644 --- a/backend/src/CodeSpace.Api/Startup.cs +++ b/backend/src/CodeSpace.Api/Startup.cs @@ -6,6 +6,8 @@ using CodeSpace.Api.Filters; using CodeSpace.Api.Logging; using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Health; +using CodeSpace.Core.Services.Health.Checks; using CodeSpace.Core.Services.OutboundHttp; using CodeSpace.Core.Services.Identity; using CodeSpace.Core.Settings; @@ -49,7 +51,17 @@ public void ConfigureServices(IServiceCollection services) // the pipeline responds, so a transient DB blip doesn't trigger a pod RESTART. /health/ready (readiness) // runs the "ready"-tagged checks — here a DbContext ping — so a pod is pulled from the Service's endpoints // (stops receiving traffic) while its DB is unreachable, without being killed. Mapped anonymous below. - services.AddHealthChecks().AddDbContextCheck("db", tags: new[] { "ready" }); + // + // /health is the report for a person or a script, not a probe: this process's build and role plus every check. The + // three checks after the database are deployment facts that otherwise take a query or a shell on the pod to learn — + // whether the credential refresh lock is exclusive across connections, whether git can carry a clone token, whether the + // credential repair sweep is scheduled. They carry NO "ready" tag, so /health/ready is exactly what it was and none + // of them can pull a pod from rotation or restart it: they degrade the report and say why. + services.AddHealthChecks() + .AddDbContextCheck("db", tags: new[] { "ready" }) + .AddCheck(RefreshLockHealthCheck.Name) + .AddCheck(GitVersionHealthCheck.Name) + .AddCheck(CredentialRepairSweepHealthCheck.Name); // CORS — only matters when the SPA calls the backend directly (VITE_API_URL=http://localhost:5099) rather // than through Vite's same-origin /api proxy. The policy, its origins setting and the Development-only @@ -134,6 +146,10 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) // so the filter can read HttpContext.User. app.UseCodeSpaceHangfire(Configuration); + // Once the host is serving: one line naming the build, role and git, and the refresh-lock self-test. Describes the pod; never + // refuses it. + app.ApplicationServices.GetRequiredService().ReportWhenStarted(); + app.UseEndpoints(endpoints => { // k8s liveness/readiness probes — AllowAnonymous because the global FallbackPolicy requires auth and a @@ -141,6 +157,11 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env) endpoints.MapHealthChecks("/health/live", new HealthCheckOptions { Predicate = _ => false }).AllowAnonymous(); endpoints.MapHealthChecks("/health/ready", new HealthCheckOptions { Predicate = check => check.Tags.Contains("ready") }).AllowAnonymous(); + // The report, as JSON. Anonymous like the probes, so what anyone reads is the build, the role and each check's status; a + // signed-in global Admin also reads each check's description and data (see HealthResponseWriter). Nothing an exception + // message could carry is printed to either. + endpoints.MapHealthChecks("/health", new HealthCheckOptions { ResponseWriter = HealthResponseWriter.WriteAsync }).AllowAnonymous(); + endpoints.MapControllers(); if (env.IsDevelopment()) endpoints.MapOpenApi(); diff --git a/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs b/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs index 02cbee064..7e47fc8cb 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/PackCloneFetcher.cs @@ -91,9 +91,18 @@ private async Task CloneAsync(string url, string? reference, string dir, Cancell var result = await _runners.Resolve(SandboxKinds.Local).RunAsync(BuildCloneSpec(url, reference, dir), cancellationToken).ConfigureAwait(false); if (result.Status != SandboxStatus.Success) - throw new PackImportException(CloneFailedMessage(url, result)); + throw OldGitFailure(url, result) ?? new PackImportException(CloneFailedMessage(url, result)); } + /// + /// The clone failure when the cause is a host git older than : a pasted credential + /// travels through GIT_CONFIG_COUNT, which that git ignores, so the remote's request for it went unanswered. The + /// message names the URL without its credential and the actionable cause, in place of git's "could not read Username"; the + /// typed exception is kept as the inner one. Null whenever an old git is not the explanation. + /// + private static PackImportException? OldGitFailure(string url, SandboxResult result) => + TokenedGitCommand.OldGitFailure(PastedSecret(url) is not null, result.Stderr) is { } oldGit ? new PackImportException($"git clone of '{WithoutPastedCredential(url)}' failed: {oldGit.Message}", oldGit) : null; + /// /// The import walks this checkout (a worker that dies mid-import leaves it on disk for the janitor), so its origin must /// hold no credential. The clone already named the URL without it; as a belt, rewrite origin to that URL through the diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/GitCredentialHelperUnsupportedException.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/GitCredentialHelperUnsupportedException.cs new file mode 100644 index 000000000..d44d97dac --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/GitCredentialHelperUnsupportedException.cs @@ -0,0 +1,25 @@ +using CodeSpace.Messages.Health; + +namespace CodeSpace.Core.Services.Agents.Workspace; + +/// +/// A tokened git command was refused for want of a credential on a host whose git is older than +/// : the helper that would have answered is configured through +/// GIT_CONFIG_COUNT, which that git ignores. Still a — the run executor, the +/// integrate nodes and the acceptance grader already handle that type — and it keeps its failure code; what changes is that +/// the message names the cause and the fix, where git says only that it "could not read Username". +/// +public sealed class GitCredentialHelperUnsupportedException : WorkspaceException +{ + public GitCredentialHelperUnsupportedException(GitVersion version) : base(Describe(version)) + { + Version = version; + } + + /// The release of git this host runs. + public GitVersion Version { get; } + + private static string Describe(GitVersion version) => + $"This host's git is {version}, older than {TokenedGitCommand.MinimumGit}. The credential helper that hands git its token is configured through GIT_CONFIG_COUNT, " + + $"which git ignores before {TokenedGitCommand.MinimumGit}, so the remote's request for a credential went unanswered. Upgrade the git in this image to {TokenedGitCommand.MinimumGit} or later."; +} diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/HostGit.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/HostGit.cs new file mode 100644 index 000000000..41bc86692 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/HostGit.cs @@ -0,0 +1,69 @@ +using CodeSpace.Messages.Health; +using System.Diagnostics; + +namespace CodeSpace.Core.Services.Agents.Workspace; + +/// +/// The git this host runs, asked once per process: every clone, fetch, push and launch-time ls-remote spawns +/// the same binary from the same PATH, so one git --version answers for all of them. The answer is what the +/// credential helper's 2.31 floor () is measured against — at boot, in the health +/// report, and when a tokened command fails for want of a credential. +/// +/// A probe that fails — no git on PATH, a hung or unreadable answer — is a with no version, +/// never an exception: this is read from a boot path and a health endpoint, neither of which may fail because of it. +/// +public static class HostGit +{ + private const int ProbeTimeoutMs = 10_000; + + private static readonly Lazy Probed = new(Probe, LazyThreadSafetyMode.ExecutionAndPublication); + + private static readonly AsyncLocal Stand = new(); + + /// The host's git as it answered the first time it was asked. + public static GitReading Current => Stand.Value ?? Probed.Value; + + /// + /// Answer as for the current async flow until the returned scope is disposed — the seam that + /// lets a test put an old git in front of a code path without installing one. Scoped to the flow, so tests running in + /// parallel never see each other's. + /// + internal static IDisposable Use(GitReading reading) + { + var previous = Stand.Value; + Stand.Value = reading; + + return new Restore(previous); + } + + private static GitReading Probe() + { + try + { + var start = new ProcessStartInfo { FileName = "git", UseShellExecute = false, RedirectStandardOutput = true, RedirectStandardError = true }; + start.ArgumentList.Add("--version"); + + using var process = Process.Start(start) ?? throw new InvalidOperationException("the process did not start"); + + if (!process.WaitForExit(ProbeTimeoutMs)) + { + try { process.Kill(entireProcessTree: true); } catch { /* best-effort */ } + + return new GitReading(null, $"git could not be run: `git --version` did not answer within {ProbeTimeoutMs / 1000}s"); + } + + var output = process.StandardOutput.ReadToEnd().Trim(); + + return process.ExitCode == 0 ? new GitReading(GitVersion.Parse(output), output) : new GitReading(null, $"git could not be run: `git --version` exited {process.ExitCode}"); + } + catch (Exception ex) + { + return new GitReading(null, $"git could not be run: {ex.GetType().Name}"); + } + } + + private sealed class Restore(GitReading? previous) : IDisposable + { + public void Dispose() => Stand.Value = previous; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs index 340ebfafe..fb207df01 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Integrators/LocalGitBranchIntegrator.cs @@ -216,7 +216,7 @@ private async Task CloneAsync(IntegrationRequest request, string directory, Canc var result = await RunTokenedGitAsync(request, new[] { "clone", url, directory }, directory, cancellationToken).ConfigureAwait(false); if (result.Status != SandboxStatus.Success) - throw new WorkspaceException($"git clone failed (exit {result.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(result.Stderr), request.Token)}"); + throw TokenedGitCommand.OldGitFailure(Remote(request).IsTokened, result.Stderr) ?? new WorkspaceException($"git clone failed (exit {result.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(result.Stderr), request.Token)}"); } /// Check out the EXACT shared base (detached) so every apply --3way resolves the pre-image against the commit the agents saw. False when the SHA is not in history (a bad base). @@ -457,7 +457,7 @@ private async Task CommitAsync(string directory, int count, CancellationToken ca return "remote integration branch advanced — refusing to clobber it"; // A genuine infrastructure failure (auth / network / repo gone) — surface it (token redacted). - throw new WorkspaceException($"git push failed (exit {result.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(result.Stderr), request.Token)}"); + throw TokenedGitCommand.OldGitFailure(Remote(request).IsTokened, result.Stderr) ?? new WorkspaceException($"git push failed (exit {result.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(result.Stderr), request.Token)}"); } private async Task ResetToBaseAsync(string directory, IntegrationRequest request, CancellationToken cancellationToken) diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs index 1555686f1..638954881 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/Providers/LocalGitWorkspaceProvider.cs @@ -341,7 +341,7 @@ private async Task CloneAsync(WorkspaceRequest request, RepositoryCommandContext var result = await RunGitAsync(args, context, cancellationToken).ConfigureAwait(false); if (result.Status != SandboxStatus.Success) - throw new WorkspaceException($"git clone failed (exit {result.ExitCode}): {Redact(Summarize(result.Stderr), request.Token)}"); + throw TokenedGitCommand.OldGitFailure(context.Remote.IsTokened, result.Stderr) ?? new WorkspaceException($"git clone failed (exit {result.ExitCode}): {Redact(Summarize(result.Stderr), request.Token)}"); if (!string.IsNullOrWhiteSpace(request.PinnedSha)) await MaterializePinAsync(request, context, cancellationToken).ConfigureAwait(false); @@ -934,7 +934,7 @@ private Task RunPublishGitAsync(MaterializedRepo repo, string pub ExecuteGitAsync(repo, args, PublishGitSpec(publishDir, args, network, timeoutSeconds), cancellationToken); private Task RunTokenedPublishGitOrThrowAsync(MaterializedRepo repo, string publishDir, TokenedGitCommand.Remote remote, IReadOnlyList args, CancellationToken cancellationToken) => - EnsureSuccessAsync(repo, args, RunTokenedPublishGitAsync(repo, publishDir, remote, args, cancellationToken)); + EnsureSuccessAsync(repo, args, RunTokenedPublishGitAsync(repo, publishDir, remote, args, cancellationToken), remote.IsTokened); /// Run a publish-repo command that reaches — the LFS upload, the push, the readback — as a , with the network and the push budget. private Task RunTokenedPublishGitAsync(MaterializedRepo repo, string publishDir, TokenedGitCommand.Remote remote, IReadOnlyList args, CancellationToken cancellationToken) => @@ -1029,13 +1029,17 @@ private async Task ExecuteGitAsync(MaterializedRepo repo, IReadOn } } - /// Await a git result and throw a redacted on a non-success status; the argv is redacted too, as a belt. - private static async Task EnsureSuccessAsync(MaterializedRepo repo, IReadOnlyList args, Task run) + /// + /// Await a git result and throw a redacted on a non-success status; the argv is redacted too, as a belt. + /// is whether the command ran against a remote that carried a credential (), + /// the only commands an old git can fail for want of one: it is the remote's own answer, not the run's clone token, so the two cannot disagree. + /// + private static async Task EnsureSuccessAsync(MaterializedRepo repo, IReadOnlyList args, Task run, bool tokened = false) { var result = await run.ConfigureAwait(false); if (result.Status != SandboxStatus.Success) - throw new WorkspaceException($"git {string.Join(' ', RedactArgs(args, repo.Token))} failed (exit {result.ExitCode}): {Redact(result.Stderr.Trim(), repo.Token)}"); + throw TokenedGitCommand.OldGitFailure(tokened, result.Stderr) ?? new WorkspaceException($"git {string.Join(' ', RedactArgs(args, repo.Token))} failed (exit {result.ExitCode}): {Redact(result.Stderr.Trim(), repo.Token)}"); SandboxOutputCompleteness.RequireStdout(result); return result.Stdout; diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs index 4526b13ee..a5ae3f0d4 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/RemoteTipResolver.cs @@ -93,7 +93,7 @@ public sealed class RemoteTipResolver : IRemoteTipResolver, ISingletonDependency } if (result.Status != SandboxStatus.Success) - throw new WorkspaceException($"git ls-remote failed (exit {result.ExitCode}) resolving the launch base for {SanitizeUrl(request.RepositoryUrl)}: {LocalGitWorkspaceProvider.Redact(Truncate(result.Stderr), request.Token)} — the launch base pin requires a reachable remote; the clone would fail the same way later"); + throw TokenedGitCommand.OldGitFailure(remote.IsTokened, result.Stderr) ?? new WorkspaceException($"git ls-remote failed (exit {result.ExitCode}) resolving the launch base for {SanitizeUrl(request.RepositoryUrl)}: {LocalGitWorkspaceProvider.Redact(Truncate(result.Stderr), request.Token)} — the launch base pin requires a reachable remote; the clone would fail the same way later"); // sha1 (40) and sha256 (64) object formats both count — a sha256 remote must not read as "ref missing". return result.Stdout diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs index cc76df687..b8e1e733b 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/TokenedGitCommand.cs @@ -1,4 +1,5 @@ using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Health; namespace CodeSpace.Core.Services.Agents.Workspace; @@ -64,6 +65,32 @@ internal static class TokenedGitCommand /// The username a token is sent under when its provider names none: GitHub's. internal const string DefaultTokenUsername = "x-access-token"; + /// The first git that reads GIT_CONFIG_COUNT, which the whole credential helper is configured through. An older git ignores the variable, so a tokened command finds no credential. + internal static readonly GitVersion MinimumGit = new(2, 31, 0); + + /// + /// What a tokened command that git refused for want of a credential becomes on a host whose git is older than + /// : a naming that cause, in place of git's own + /// "could not read Username". Null whenever the old git is not the explanation — the command was not tokened (no helper was + /// ever going to be configured), git's complaint is some other one (an old git clones a public repository fine), or the + /// host's release is unknown or new enough, in which case the credential really was missing or refused and git's words stand. + /// The caller throws it in place of its own failure, with the result's redacted detail unchanged when this is null. + /// + internal static WorkspaceException? OldGitFailure(bool tokened, string stderr) => + tokened && AskedForACredential(stderr) ? OldGitFailure(tokened, stderr, HostGit.Current) : null; + + /// against a given reading of the host's git. + internal static WorkspaceException? OldGitFailure(bool tokened, string stderr, GitReading git) + { + if (!tokened || !AskedForACredential(stderr)) return null; + + return git.Version is { } version && !version.IsAtLeast(MinimumGit) ? new GitCredentialHelperUnsupportedException(version) : null; + } + + /// git's non-interactive words for a remote that wanted a credential nothing supplied. + private static bool AskedForACredential(string stderr) => + stderr.Contains("could not read Username", StringComparison.Ordinal) || stderr.Contains("could not read Password", StringComparison.Ordinal); + /// /// The credential helper a tokened command names: a shell function that answers get from the two variables until /// an erase records in the directory that the remote refused the credential, then says diff --git a/backend/src/CodeSpace.Core/Services/Agents/Workspace/WorkspaceException.cs b/backend/src/CodeSpace.Core/Services/Agents/Workspace/WorkspaceException.cs index 676bd60e1..0333dcb5c 100644 --- a/backend/src/CodeSpace.Core/Services/Agents/Workspace/WorkspaceException.cs +++ b/backend/src/CodeSpace.Core/Services/Agents/Workspace/WorkspaceException.cs @@ -5,8 +5,11 @@ namespace CodeSpace.Core.Services.Agents.Workspace; /// /// Thrown when a workspace can't be prepared — a failed clone, a missing ref, or git being unavailable. /// The executor maps it to a clean agent-run failure (the run never starts the harness). +/// +/// Not sealed so a failure with a known cause can say so () while +/// every catch site that handles this type keeps handling it. /// -public sealed class WorkspaceException : Exception, IFailure +public class WorkspaceException : Exception, IFailure { public FailureKind Kind => FailureKind.Unprocessable; diff --git a/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs b/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs index 61e7769c2..a355d2bd1 100644 --- a/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs +++ b/backend/src/CodeSpace.Core/Services/Credentials/CredentialSuccessionService.cs @@ -20,7 +20,7 @@ namespace CodeSpace.Core.Services.Credentials; /// (), each a deliberate act rather than a side effect of connecting. The repair sweep /// stays the fallback for what none of those doors reached, and walks its backlog in bounded pages (). /// -public sealed partial class CredentialSuccessionService : ICredentialSuccessionService, IScopedDependency +public sealed partial class CredentialSuccessionService : ICredentialSuccessionService, IStrandedCredentialCensus, IScopedDependency { private const string SweepName = "credential-binding-repair"; @@ -58,6 +58,8 @@ public async Task CarryForwardAsync(Credential successor, CancellationToken canc public async Task RepairStrandedAsync(int batchSize, CancellationToken cancellationToken) => await new SweepPass(SweepName, _backoff, _logger).RunAsync(batchSize, LoadStrandedAsync, RepairAsync, cancellationToken).ConfigureAwait(false); + public async Task CountAsync(CancellationToken cancellationToken) => await StrandedWithSuccessor().CountAsync(cancellationToken).ConfigureAwait(false); + /// /// The owner's revoked credentials on the successor's instance and team. Filtered on the tracked entities, not in /// SQL: a token link revokes the credential it replaces in the same unit of work, so that row is Revoked in memory @@ -82,15 +84,29 @@ private async Task> LoadDisconnectedPredecessorIdsAsync(Credential su /// instead of re-reading its first rows every time. /// private async Task> LoadStrandedAsync(Guid? after, int take, CancellationToken cancellationToken) + { + var rows = await StrandedWithSuccessor(after) + .OrderBy(candidate => candidate.DeadId) + .Take(take) + .ToListAsync(cancellationToken).ConfigureAwait(false); + + return rows.Select(row => new StrandedCredential(row.DeadId, row.SuccessorId!.Value)).ToList(); + } + + /// + /// Every candidate after (all of them when null), unbounded: what + /// pages through and counts, so the two cannot disagree about what a candidate is. + /// + private IQueryable StrandedWithSuccessor(Guid? after = null) { var revoked = _db.Credential.AsNoTracking().Where(dead => dead.Status == CredentialStatus.Revoked && dead.OwnerUserId != null); if (after is Guid cursor) revoked = revoked.Where(dead => dead.Id.CompareTo(cursor) > 0); - var rows = await revoked + return revoked .Where(dead => _db.Repository.Any(r => r.CredentialId == dead.Id && r.TeamId == dead.TeamId && r.ProviderInstanceId == dead.ProviderInstanceId && r.DeletedDate == null) || _db.ConnectionWebhook.Any(w => w.CredentialId == dead.Id && w.ProviderInstanceId == dead.ProviderInstanceId && WebhookRegistrationLifecycle.InService.Contains(w.RegistrationStatus))) - .Select(dead => new + .Select(dead => new StrandedRow { DeadId = dead.Id, SuccessorId = _db.Credential @@ -99,12 +115,7 @@ private async Task> LoadStrandedAsync(Guid? after, int .Select(c => (Guid?)c.Id) .FirstOrDefault() }) - .Where(candidate => candidate.SuccessorId != null) - .OrderBy(candidate => candidate.DeadId) - .Take(take) - .ToListAsync(cancellationToken).ConfigureAwait(false); - - return rows.Select(row => new StrandedCredential(row.DeadId, row.SuccessorId!.Value)).ToList(); + .Where(candidate => candidate.SuccessorId != null); } /// Move one disconnected credential's bindings and save them on their own, then hand its revived hooks to the registrar. @@ -193,4 +204,11 @@ private sealed record StrandedCredential(Guid DeadId, Guid SuccessorId) : ISweep } private sealed record Adopted(int RepositoryCount, int ConnectionWebhookCount, List RevivedHookIds); + + /// The query's row: a member-initialised class because an anonymous type cannot be returned from . + private sealed class StrandedRow + { + public Guid DeadId { get; init; } + public Guid? SuccessorId { get; init; } + } } diff --git a/backend/src/CodeSpace.Core/Services/Credentials/IStrandedCredentialCensus.cs b/backend/src/CodeSpace.Core/Services/Credentials/IStrandedCredentialCensus.cs new file mode 100644 index 000000000..9b2ac269b --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Credentials/IStrandedCredentialCensus.cs @@ -0,0 +1,16 @@ +namespace CodeSpace.Core.Services.Credentials; + +/// +/// How many disconnected credentials the binding-repair sweep would move right now. A sibling of +/// rather than another member of it, so reading the backlog never widens what +/// something that only repairs it has to implement. +/// +public interface IStrandedCredentialCensus +{ + /// + /// The revoked credentials that something live is still bound to AND whose owner has since connected another — exactly + /// the rows takes, counted without its batch bound. A + /// backlog that stays above zero across passes is a sweep that is not running, or one candidate that fails every time. + /// + Task CountAsync(CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Health/BootPostureReporter.cs b/backend/src/CodeSpace.Core/Services/Health/BootPostureReporter.cs new file mode 100644 index 000000000..dd1ba6eac --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/BootPostureReporter.cs @@ -0,0 +1,64 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Core.Services.Health.Checks; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Health; + +/// +/// Once the host is serving, says what this pod is: one line naming the build, the role and the git it runs, a Warning when +/// that git is below the credential helper's floor, and the refresh-lock self-test, whose own finding is logged as it lands. +/// The log-side twin of /health, for the moment nothing can reach the pod to ask it. +/// +/// It runs off the thread that starts the host and swallows everything: the only thing a posture report may do to a +/// process is describe it. A pod whose git is too old, or whose lock is not exclusive, starts all the same and says so. +/// +public sealed class BootPostureReporter : ISingletonDependency +{ + private readonly HostPosture _posture; + private readonly IRefreshLockSelfTest _selfTest; + private readonly IHostApplicationLifetime _lifetime; + private readonly ILogger _logger; + + public BootPostureReporter(HostPosture posture, IRefreshLockSelfTest selfTest, IHostApplicationLifetime lifetime, ILogger logger) + { + _posture = posture; + _selfTest = selfTest; + _lifetime = lifetime; + _logger = logger; + } + + /// Arrange for the report when the host has started. Returns at once. + public void ReportWhenStarted() => _lifetime.ApplicationStarted.Register(() => _ = Task.Run(() => ReportAsync(_lifetime.ApplicationStopping))); + + internal async Task ReportAsync(CancellationToken cancellationToken) + { + try + { + ReportPosture(); + + await _selfTest.VerdictAsync(cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + // The host is stopping; there is nothing left to report to. + } + catch (Exception ex) + { + _logger.LogWarning(ex, "The boot posture report could not finish; the pod is unaffected"); + } + } + + private void ReportPosture() + { + var git = HostGit.Current; + + _logger.LogInformation("Boot posture: build {Build}, role {Role} ({RoleMeaning}), git {GitVersion}", _posture.Build, _posture.RoleName, _posture.RoleMeaning, git.Version?.ToString() ?? git.Detail); + + var assessment = GitVersionHealthCheck.Assess(git); + + if (assessment.Status != HealthStatus.Healthy) _logger.LogWarning("{GitPosture}", assessment.Description); + } +} diff --git a/backend/src/CodeSpace.Core/Services/Health/Checks/CredentialRepairSweepHealthCheck.cs b/backend/src/CodeSpace.Core/Services/Health/Checks/CredentialRepairSweepHealthCheck.cs new file mode 100644 index 000000000..cf9800060 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/Checks/CredentialRepairSweepHealthCheck.cs @@ -0,0 +1,175 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Jobs.RecurringJobs; +using CodeSpace.Core.Services.Credentials; +using Hangfire; +using Hangfire.Storage; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Health.Checks; + +/// +/// Whether the disconnected-credential binding-repair sweep exists on this deployment, when it last ran, and how many +/// credentials it is waiting to repair. Only a Worker-role pod registers recurring jobs, so an API-only fleet — or workers +/// still on a build from before the sweep — has no sweep at all, and nothing but a query against Hangfire's tables said so. +/// +/// The schedule is Hangfire's own record, read from the same storage from any role (the API pod's report describes the +/// worker's sweep). The backlog is counted live — the rows the sweep would take — so it stays right when the sweep is run by a +/// build that records nothing. Both are kept for : this check answers an anonymous endpoint and +/// reads the database. +/// +/// Degraded, never Unhealthy, and never a refusal: a deployment without the sweep serves requests and runs agents; what it +/// does not do is carry a repository onto its owner's reconnect. +/// +/// The scheduler's own error text is never part of the finding. Hangfire stores an exception chain there — up to a hundred +/// stack frames, and for a storage fault the server's host and port — and /health is read by callers who are not signed +/// in. The finding says an error was recorded; the full text goes to the log, once per distinct error. +/// +public sealed class CredentialRepairSweepHealthCheck : IHealthCheck, ISingletonDependency +{ + /// The check's name in /health — a contract, pinned by test. + public const string Name = "credential-repair-sweep"; + + /// The recurring-job id the Worker role registers the sweep under (). + public const string JobId = nameof(DisconnectedCredentialBindingRepairRecurringJob); + + /// How long a reading is reused. + public static readonly TimeSpan SnapshotLifetime = TimeSpan.FromSeconds(30); + + /// How late the next scheduled run may be before the schedule is called stalled: Hangfire's scheduler polls every few seconds, so a run minutes past due means no scheduler is running. + public static readonly TimeSpan OverdueAfter = TimeSpan.FromMinutes(2); + + private readonly IServiceScopeFactory _scopes; + private readonly TimeProvider _clock; + private readonly ILogger _logger; + private readonly JobStorage? _storage; + private readonly object _gate = new(); + private (DateTimeOffset At, HealthCheckResult Result)? _last; + private string? _loggedSchedulerError; + + public CredentialRepairSweepHealthCheck(IServiceScopeFactory scopes, TimeProvider clock, ILogger logger, JobStorage? storage = null) + { + _scopes = scopes; + _clock = clock; + _logger = logger; + _storage = storage; + } + + public async Task CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) + { + if (Fresh() is { } cached) return cached; + + var result = await ReadAsync(cancellationToken).ConfigureAwait(false); + + lock (_gate) _last = (_clock.GetUtcNow(), result); + + return result; + } + + private HealthCheckResult? Fresh() + { + lock (_gate) return _last is { } last && _clock.GetUtcNow() - last.At < SnapshotLifetime ? last.Result : null; + } + + private async Task ReadAsync(CancellationToken cancellationToken) + { + if (_storage is null) return HealthCheckResult.Degraded("This host has no job storage, so whether the repair sweep is scheduled cannot be read."); + + try + { + var entry = ReadSchedule(_storage); + + LogSchedulerError(entry); + + var candidates = await CountCandidatesAsync(cancellationToken).ConfigureAwait(false); + + return Assess(entry, candidates, _clock.GetUtcNow().UtcDateTime); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + return HealthCheckResult.Degraded($"The repair sweep's schedule could not be read ({ex.GetType().Name})."); + } + } + + /// + /// Where the scheduler's recorded error goes instead of the report: a Warning with its full text, the first time a disabled + /// entry carries it and again only when the text changes or returns after the entry recovered. The check is read every + /// , so without the memory one stuck entry would log twice a minute. + /// + internal void LogSchedulerError(RecurringJobDto? entry) + { + var error = entry is { Removed: false, NextExecution: null } && !string.IsNullOrWhiteSpace(entry.Error) ? entry.Error : null; + + lock (_gate) + { + if (error == _loggedSchedulerError) return; + + _loggedSchedulerError = error; + } + + if (error is not null) _logger.LogWarning("Hangfire's scheduler disabled the {JobId} schedule after an error: {SchedulerError}", JobId, error); + } + + private static RecurringJobDto? ReadSchedule(JobStorage storage) + { + using var connection = storage.GetConnection(); + + return connection.GetRecurringJobs([JobId]).SingleOrDefault(); + } + + /// A failed count is null, not an exception: the schedule is still worth reporting. A scope of its own because this check is a singleton and the census reads through a request-lifetime context. + private async Task CountCandidatesAsync(CancellationToken cancellationToken) + { + try + { + using var scope = _scopes.CreateScope(); + + return await scope.ServiceProvider.GetRequiredService().CountAsync(cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + return null; + } + } + + /// The finding for one reading of the schedule and the backlog. Pure: the clock and both reads are arguments. + public static HealthCheckResult Assess(RecurringJobDto? entry, int? candidates, DateTime nowUtc) + { + var data = new Dictionary { ["registered"] = entry is { Removed: false } }; + + if (candidates is { } count) data["strandedCandidates"] = count; + + if (entry is null or { Removed: true }) + return HealthCheckResult.Degraded($"The repair sweep ({JobId}) is not registered: only a Worker-role pod registers recurring jobs, so no worker of a build that has it has started against this database.", data: data); + + AddSchedule(data, entry); + + if (Stalled(entry, nowUtc) is { } stalled) return HealthCheckResult.Degraded(stalled, data: data); + + if (candidates is null) return HealthCheckResult.Degraded("The repair sweep is scheduled, but the credentials it has to repair could not be counted.", data: data); + + return HealthCheckResult.Healthy(entry.LastExecution is null ? "The repair sweep is scheduled and has not run yet." : "The repair sweep is scheduled and running.", data); + } + + private static void AddSchedule(Dictionary data, RecurringJobDto entry) + { + data["cron"] = entry.Cron; + + if (entry.LastExecution is { } last) data["lastExecutionUtc"] = last; + if (entry.NextExecution is { } next) data["nextExecutionUtc"] = next; + if (entry.LastJobState is { } state) data["lastJobState"] = state; + } + + /// Why the schedule is not doing its job, or null when it is: disabled by the scheduler, overdue, or its last run failed. + private static string? Stalled(RecurringJobDto entry, DateTime nowUtc) + { + if (entry.NextExecution is null) + return $"The repair sweep is registered but disabled: Hangfire's scheduler gave it no next run{(string.IsNullOrWhiteSpace(entry.Error) ? "" : " and recorded an error (its text is in this pod's log and on the recurring job in Hangfire's storage)")}. A current worker re-enables it."; + + if (nowUtc - entry.NextExecution.Value > OverdueAfter) + return $"The repair sweep is overdue by {(nowUtc - entry.NextExecution.Value).TotalMinutes:0} minutes: no live worker is running Hangfire's scheduler."; + + return entry.LastJobState == "Failed" ? "The repair sweep's last run failed." : null; + } +} diff --git a/backend/src/CodeSpace.Core/Services/Health/Checks/GitVersionHealthCheck.cs b/backend/src/CodeSpace.Core/Services/Health/Checks/GitVersionHealthCheck.cs new file mode 100644 index 000000000..f8f01c6b5 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/Checks/GitVersionHealthCheck.cs @@ -0,0 +1,35 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Diagnostics.HealthChecks; + +namespace CodeSpace.Core.Services.Health.Checks; + +/// +/// Whether this host's git can carry a clone token. The credential helper is configured through GIT_CONFIG_COUNT, which +/// git reads from 2.31; an older git ignores it, finds no credential, and every tokened clone, fetch and push fails as "could +/// not read Username" — which reads as a bad token. Degraded, never Unhealthy: the process works, the commands that need a +/// credential do not, and an orchestrator must not restart a pod over an image it cannot fix by restarting. +/// +public sealed class GitVersionHealthCheck : IHealthCheck, ISingletonDependency +{ + /// The check's name in /health — a contract, pinned by test. + public const string Name = "git"; + + public Task CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) => Task.FromResult(Assess(HostGit.Current)); + + public static HealthCheckResult Assess(GitReading git) + { + if (git.Version is not { } version) + return HealthCheckResult.Degraded($"The git release could not be read ({git.Detail}), so whether it can carry a clone credential is unknown."); + + var data = new Dictionary { ["version"] = version.ToString(), ["minimum"] = TokenedGitCommand.MinimumGit.ToString() }; + + if (version.IsAtLeast(TokenedGitCommand.MinimumGit)) return HealthCheckResult.Healthy($"git {version}", data); + + return HealthCheckResult.Degraded( + $"git {version} is older than {TokenedGitCommand.MinimumGit}: the credential helper that hands git its token is configured through GIT_CONFIG_COUNT, which git ignores before {TokenedGitCommand.MinimumGit}, " + + "so every clone, fetch and push that needs a token fails as \"could not read Username\". Upgrade the git in this image.", + data: data); + } +} diff --git a/backend/src/CodeSpace.Core/Services/Health/Checks/RefreshLockHealthCheck.cs b/backend/src/CodeSpace.Core/Services/Health/Checks/RefreshLockHealthCheck.cs new file mode 100644 index 000000000..1d003d53d --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/Checks/RefreshLockHealthCheck.cs @@ -0,0 +1,30 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Diagnostics.HealthChecks; + +namespace CodeSpace.Core.Services.Health.Checks; + +/// +/// The refresh-lock self-test's finding, as a health entry. Degraded for a lock that is not exclusive or not known to be: +/// the compare-and-set write and the re-read keep refreshed tokens correct, so nothing here is worth a restart, but a +/// deployment whose refresh is not serialized should know. +/// +public sealed class RefreshLockHealthCheck : IHealthCheck, ISingletonDependency +{ + /// The check's name in /health — a contract, pinned by test. + public const string Name = "refresh-lock"; + + private readonly IRefreshLockSelfTest _selfTest; + + public RefreshLockHealthCheck(IRefreshLockSelfTest selfTest) { _selfTest = selfTest; } + + public async Task CheckHealthAsync(HealthCheckContext context, CancellationToken cancellationToken = default) => + Assess(await _selfTest.VerdictAsync(cancellationToken).ConfigureAwait(false)); + + public static HealthCheckResult Assess(RefreshLockVerdict verdict) + { + var data = new Dictionary { ["outcome"] = verdict.Outcome.ToString(), ["observedAtUtc"] = verdict.ObservedAt.UtcDateTime }; + + return verdict.Outcome == RefreshLockOutcome.Exclusive ? HealthCheckResult.Healthy(verdict.Detail, data) : HealthCheckResult.Degraded(verdict.Detail, data: data); + } +} diff --git a/backend/src/CodeSpace.Core/Services/Health/HealthResponseWriter.cs b/backend/src/CodeSpace.Core/Services/Health/HealthResponseWriter.cs new file mode 100644 index 000000000..41cb09492 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/HealthResponseWriter.cs @@ -0,0 +1,53 @@ +using CodeSpace.Core.Services.Identity; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Health; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; + +namespace CodeSpace.Core.Services.Health; + +/// +/// Writes GET /health: this process's build and role, then every check's finding, as JSON. The endpoint is anonymous — a +/// probe cannot sign in — so by default it prints only what an operator needs to compare two images from outside: the build, +/// the role, and each check's name and status. A check's description and data (the git release, the connection topology, the +/// repair backlog) are shown to a signed-in global Admin alone: on an internet-facing API they are reconnaissance. Either way a +/// check that threw is reported by the exception's type, never its message, which can carry an address or a user name. +/// +/// Who the caller is comes from — the instance Admin role is a database fact, not a +/// token claim — and a caller whose roles cannot be read is treated as anonymous: this is the endpoint to ask when the +/// database is down, and a signed-in caller must not turn that into an error. +/// +public static class HealthResponseWriter +{ + private static readonly IReadOnlyDictionary NoData = new Dictionary(); + + public static Task WriteAsync(HttpContext context, HealthReport report) + { + var posture = context.RequestServices.GetRequiredService(); + var detailed = IsGlobalAdmin(context.RequestServices); + + var checks = report.Entries.OrderBy(entry => entry.Key, StringComparer.Ordinal).Select(entry => ToEntry(entry.Key, entry.Value, detailed)).ToList(); + + return context.Response.WriteAsJsonAsync(new HealthResponse(report.Status.ToString(), posture.Build, posture.RoleName, posture.ProcessesJobs, detailed, checks), context.RequestAborted); + } + + private static bool IsGlobalAdmin(IServiceProvider services) + { + try + { + return services.GetService()?.HasRole(Roles.Admin) == true; + } + catch (Exception) + { + return false; + } + } + + private static HealthCheckEntry ToEntry(string name, HealthReportEntry entry, bool detailed) + { + if (!detailed) return new HealthCheckEntry(name, entry.Status.ToString(), null, NoData); + + return new HealthCheckEntry(name, entry.Status.ToString(), entry.Exception is null ? entry.Description : entry.Exception.GetType().Name, entry.Data); + } +} diff --git a/backend/src/CodeSpace.Core/Services/Health/HostPosture.cs b/backend/src/CodeSpace.Core/Services/Health/HostPosture.cs new file mode 100644 index 000000000..dff90fe48 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/HostPosture.cs @@ -0,0 +1,42 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Settings; +using CodeSpace.Core.Settings.Logging; +using CodeSpace.Messages.Enums; + +namespace CodeSpace.Core.Services.Health; + +/// +/// Which build this process is and which role the deployment gave it — what /health and the boot line print so an API +/// image and a worker image can be compared from outside. Both facts are the ones the process already acts on: the build is +/// (stamped on every log line) and the role is , resolved by the +/// same rule that decides whether this process starts job servers, so the report cannot disagree with the behaviour. +/// +/// The role is Api or Worker because that is all the deployment chooses between. Every process serves HTTP, +/// so Worker is also what a single all-in-one process is: it serves the API and processes the queue. +/// +public sealed class HostPosture : ISingletonDependency +{ + public HostPosture(HangfireHostingSetting role) : this(role.Value, BuildIdentity.Value) { } + + internal HostPosture(HangfireHosting role, string build) + { + Role = role; + Build = build; + } + + /// Version plus commit sha, as stamped on every log line. + public string Build { get; } + + public HangfireHosting Role { get; } + + /// The role's name as /health and the boot line print it — a contract, pinned by test. + public string RoleName => Role.ToString(); + + /// Whether this process runs Hangfire job servers: only the Worker role does. + public bool ProcessesJobs => Role == HangfireHosting.Worker; + + /// The role in a phrase, for the boot line. + public string RoleMeaning => ProcessesJobs + ? "processes jobs and owns the recurring schedule; also serves HTTP" + : "serves HTTP and enqueues jobs; processes no jobs"; +} diff --git a/backend/src/CodeSpace.Core/Services/Health/IAdvisoryLockSessions.cs b/backend/src/CodeSpace.Core/Services/Health/IAdvisoryLockSessions.cs new file mode 100644 index 000000000..7268cd885 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/IAdvisoryLockSessions.cs @@ -0,0 +1,29 @@ +namespace CodeSpace.Core.Services.Health; + +/// +/// Opens independent Postgres sessions for the refresh-lock self-test, the way PostgresAdvisoryLock opens its own: +/// one connection per session, from the process's connection string. Narrow on purpose — the test needs to take a key, try +/// it from another session, and let go, and nothing else. +/// +public interface IAdvisoryLockSessions +{ + Task OpenAsync(CancellationToken cancellationToken); +} + +/// One Postgres session. Advisory locks belong to a server session, which is what makes them re-entrant for it and exclusive against any other. +public interface IAdvisoryLockSession : IAsyncDisposable +{ + /// pg_advisory_lock: blocks until this session holds . On a key nobody holds, it returns at once. + Task LockAsync(long key, CancellationToken cancellationToken); + + /// pg_try_advisory_lock: true when this session now holds — including when it already did, since a session may take its own lock again; false when another session holds it. + Task TryLockAsync(long key, CancellationToken cancellationToken); + + /// + /// pg_advisory_unlock for one hold of : true when this session held it and has let one hold go, + /// false when it did not hold it. Straight to a server, a session that took the key and then unlocks it always gets true; a + /// false is the server saying the statement ran on a session that never took it — the signature of a pooler that serves one + /// connection's statements from different server sessions. + /// + Task UnlockAsync(long key, CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Health/IRefreshLockSelfTest.cs b/backend/src/CodeSpace.Core/Services/Health/IRefreshLockSelfTest.cs new file mode 100644 index 000000000..679452ed4 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/IRefreshLockSelfTest.cs @@ -0,0 +1,10 @@ +using CodeSpace.Messages.Health; + +namespace CodeSpace.Core.Services.Health; + +/// Whether pg_advisory_lock is exclusive across the connections this process opens — the property OAuth token refresh serializes on. +public interface IRefreshLockSelfTest +{ + /// The latest finding, running the test first when there is none or it is older than its lifetime. Never throws for a test that cannot run (that is ); a cancelled still cancels. + Task VerdictAsync(CancellationToken cancellationToken); +} diff --git a/backend/src/CodeSpace.Core/Services/Health/RefreshLockSelfTest.cs b/backend/src/CodeSpace.Core/Services/Health/RefreshLockSelfTest.cs new file mode 100644 index 000000000..bdcd035a9 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/RefreshLockSelfTest.cs @@ -0,0 +1,215 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Logging; + +namespace CodeSpace.Core.Services.Health; + +/// +/// Takes one advisory-lock key on a session, tries it from a second, and reports whether the second was refused. +/// +/// OAuth token refresh holds pg_advisory_lock per credential so that two pods do not both spend the one +/// refresh token. The lock is only exclusive when each connection IS a server session. Behind a transaction-mode pooler +/// (PgBouncer), RDS Proxy or a read-write splitting proxy two connections can share one — where the lock is re-entrant — +/// or land on different ones, and refresh stops being serialized without any error. The compare-and-set write and the +/// re-read after the lock keep the stored tokens correct; what is lost is that a loser can still call the provider with a +/// token the winner has already spent. This test turns that from an inference into a line in the log and a field in +/// /health. +/// +/// Two observations convict a pooler, and either one is certain. The contender taking the held key means two +/// connections shared a server session. The holder's own unlock being told it never held the key means one connection's +/// statements ran on different server sessions — the lock went to one, the unlock to another. The contender tries more than +/// once because a pooler that picks a server session per statement lands it on the holder's only some of the time; the +/// unlock's answer is read because it catches that same pooler on the one statement the contender cannot be fooled about. +/// A test that passes still cannot prove the lock is exclusive — it fails to find a pooler — and says so. +/// +/// It never refuses anything: a lock that is not exclusive is logged as an Error and reported as Degraded, and the +/// process carries on. The finding is kept for and re-taken on the next ask after that, so +/// an anonymous endpoint reading it cannot make the database open connections per request, and a failover that changes the +/// topology is noticed. +/// +public sealed class RefreshLockSelfTest : IRefreshLockSelfTest, ISingletonDependency +{ + /// How long a finding is reused before the next ask runs the test again. + public static readonly TimeSpan VerdictLifetime = TimeSpan.FromMinutes(5); + + /// The bound on one run: connecting two sessions to an unreachable server must not hold a health request, or the boot path, for the driver's own minutes. + public static readonly TimeSpan ProbeTimeout = TimeSpan.FromSeconds(30); + + /// How many times the second session tries the held key before the lock is believed exclusive. + public const int ContenderAttempts = 5; + + /// How many times an unlock that missed is asked again, hoping to land where the lock is, before the test gives up on clearing it. + internal const int ReleaseAttempts = 5; + + private readonly IAdvisoryLockSessions _sessions; + private readonly TimeProvider _clock; + private readonly ILogger _logger; + private readonly SemaphoreSlim _running = new(1, 1); + private RefreshLockVerdict? _latest; + private RefreshLockOutcome? _reported; + + public RefreshLockSelfTest(IAdvisoryLockSessions sessions, TimeProvider clock, ILogger logger) + { + _sessions = sessions; + _clock = clock; + _logger = logger; + } + + public async Task VerdictAsync(CancellationToken cancellationToken) + { + if (Fresh() is { } cached) return cached; + + await _running.WaitAsync(cancellationToken).ConfigureAwait(false); + + try + { + if (Fresh() is { } raced) return raced; + + var verdict = await RunAsync(cancellationToken).ConfigureAwait(false); + + Volatile.Write(ref _latest, verdict); + Report(verdict); + + return verdict; + } + finally + { + _running.Release(); + } + } + + private RefreshLockVerdict? Fresh() => Volatile.Read(ref _latest) is { } latest && _clock.GetUtcNow() - latest.ObservedAt < VerdictLifetime ? latest : null; + + /// One run, bounded by . A failure to run is a finding of its own, not an exception: the caller is the boot path or a health request. + private async Task RunAsync(CancellationToken cancellationToken) + { + using var timeout = new CancellationTokenSource(ProbeTimeout, _clock); + using var bounded = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeout.Token); + + try + { + return await TakeTheKeyTwiceAsync(bounded.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + { + return Observe(RefreshLockOutcome.Inconclusive, $"The self-test did not finish within {ProbeTimeout.TotalSeconds:0}s, so whether pg_advisory_lock is exclusive across connections is unknown."); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogWarning(ex, "The refresh-lock self-test could not run; whether pg_advisory_lock is exclusive across connections is unknown"); + + return Observe(RefreshLockOutcome.Inconclusive, $"The self-test could not run ({ex.GetType().Name}), so whether pg_advisory_lock is exclusive across connections is unknown."); + } + } + + /// A fresh random key, so a second pod running this at the same moment cannot collide with it and no credential's refresh lock is touched. + private async Task TakeTheKeyTwiceAsync(CancellationToken cancellationToken) + { + var key = Random.Shared.NextInt64(); + + await using var holder = await _sessions.OpenAsync(cancellationToken).ConfigureAwait(false); + await using var contender = await _sessions.OpenAsync(cancellationToken).ConfigureAwait(false); + + await holder.LockAsync(key, cancellationToken).ConfigureAwait(false); + + bool contenderTookTheKey; + bool holderUnlocked; + + try + { + contenderTookTheKey = await ContendAsync(contender, key, cancellationToken).ConfigureAwait(false); + } + finally + { + holderUnlocked = await ReleaseAsync(holder, key).ConfigureAwait(false); + } + + return Conclude(contenderTookTheKey, holderUnlocked); + } + + /// True when the contender was ever granted the held key — which it releases at once; false when every try was refused. + private static async Task ContendAsync(IAdvisoryLockSession contender, long key, CancellationToken cancellationToken) + { + for (var attempt = 1; attempt <= ContenderAttempts; attempt++) + { + if (!await contender.TryLockAsync(key, cancellationToken).ConfigureAwait(false)) continue; + + await ReleaseAsync(contender, key).ConfigureAwait(false); + + return true; + } + + return false; + } + + private RefreshLockVerdict Conclude(bool contenderTookTheKey, bool holderUnlocked) + { + if (contenderTookTheKey) return Observe(RefreshLockOutcome.NotExclusive, NotExclusiveDetail); + + if (!holderUnlocked) return Observe(RefreshLockOutcome.NotExclusive, UnlockMissedDetail); + + return Observe(RefreshLockOutcome.Exclusive, ExclusiveDetail); + } + + /// + /// Lets go of and returns false when the server's FIRST answer was that this session did not hold it — + /// the unlock ran on a server session that never took the lock, which leaves the lock where it was. It then asks again, a + /// bounded number of times, hoping to land where the lock is, so a pooler does not accumulate a leaked key per test. An + /// unlock that throws says nothing about the lock (true): the session is closed next, which releases it on a direct connection. + /// + private static async Task ReleaseAsync(IAdvisoryLockSession session, long key) + { + var missed = false; + + for (var attempt = 1; attempt <= ReleaseAttempts; attempt++) + { + try + { + if (await session.UnlockAsync(key, CancellationToken.None).ConfigureAwait(false)) return !missed; + } + catch + { + return !missed; + } + + missed = true; + } + + return false; + } + + private RefreshLockVerdict Observe(RefreshLockOutcome outcome, string detail) => new(outcome, detail, _clock.GetUtcNow()); + + /// Logged on a change of finding only: a re-test every few minutes must not repeat the same Error. + private void Report(RefreshLockVerdict verdict) + { + var previous = _reported; + + if (previous == verdict.Outcome) return; + + _reported = verdict.Outcome; + + if (verdict.Outcome == RefreshLockOutcome.NotExclusive) + _logger.LogError("Refresh-lock self-test failed: {Detail}", verdict.Detail); + else if (verdict.Outcome == RefreshLockOutcome.Exclusive) + _logger.LogInformation(previous == RefreshLockOutcome.NotExclusive ? "Refresh-lock self-test: no sign of multiplexed connections any more" : "Refresh-lock self-test passed: no sign that connections are multiplexed"); + } + + /// What a passing test says: what it saw and, as plainly, what it cannot see. + internal const string ExclusiveDetail = + "No sign that connections are multiplexed: a second session was refused the advisory lock the first one held, and the first released it as its own. " + + "A pooler that picks a server session per statement can slip past this test, so it narrows the question and does not close it."; + + /// What an operator reads when the contender was GRANTED the held key: the symptom, the likely topology, what still holds, and the setting to change. + internal const string NotExclusiveDetail = + "A second Postgres session took the advisory lock the first one already held, so pg_advisory_lock does not serialize across this deployment's connections. " + WhatToCheck; + + /// What an operator reads when the holder's unlock was told it did not hold the key it had just taken. + internal const string UnlockMissedDetail = + "A Postgres session that had just taken an advisory lock was told, on unlocking it, that it did not hold it, so one connection's statements are served by different server sessions and pg_advisory_lock does not serialize across this deployment's connections. " + WhatToCheck; + + private const string WhatToCheck = + "CodeSpaceStore:ConnectionString most likely points at a transaction-mode pooler (PgBouncer), RDS Proxy or a read-write splitting proxy. " + + "OAuth token refresh then relies on its compare-and-set write and re-read alone: two refreshes of one credential can both call the provider. " + + "Point it at the primary directly, or at a session-mode pool."; +} diff --git a/backend/src/CodeSpace.Core/Services/Health/Sessions/NpgsqlAdvisoryLockSessions.cs b/backend/src/CodeSpace.Core/Services/Health/Sessions/NpgsqlAdvisoryLockSessions.cs new file mode 100644 index 000000000..857695ce5 --- /dev/null +++ b/backend/src/CodeSpace.Core/Services/Health/Sessions/NpgsqlAdvisoryLockSessions.cs @@ -0,0 +1,61 @@ +using CodeSpace.Core.DependencyInjection; +using CodeSpace.Core.Settings.Database; +using Npgsql; + +namespace CodeSpace.Core.Services.Health.Sessions; + +/// +/// over the process's own connection string — the one PostgresAdvisoryLock opens +/// its connections from, so the test meets whatever sits between this process and Postgres exactly as token refresh does. +/// +/// Pooling is off for these connections. They are two distinct client connections for as long as they live, and +/// closing one closes the server session, which releases every lock it holds — a pooled connection would go back to the +/// pool still holding one until it was next reset. +/// +public sealed class NpgsqlAdvisoryLockSessions : IAdvisoryLockSessions, ISingletonDependency +{ + private readonly CodeSpaceConnectionString _connectionString; + + public NpgsqlAdvisoryLockSessions(CodeSpaceConnectionString connectionString) { _connectionString = connectionString; } + + /// The connection string is read here, not when this is constructed: a string the driver cannot parse must surface as the self-test's "could not run", never as a failure to build the host. + public async Task OpenAsync(CancellationToken cancellationToken) + { + var connection = new NpgsqlConnection(new NpgsqlConnectionStringBuilder(_connectionString.Value) { Pooling = false }.ConnectionString); + + try + { + await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + } + catch + { + await connection.DisposeAsync().ConfigureAwait(false); + throw; + } + + return new Session(connection); + } + + private sealed class Session(NpgsqlConnection connection) : IAdvisoryLockSession + { + public async Task LockAsync(long key, CancellationToken cancellationToken) => + await ScalarAsync("SELECT pg_advisory_lock(@key)", key, cancellationToken).ConfigureAwait(false); + + public async Task TryLockAsync(long key, CancellationToken cancellationToken) => + await ScalarAsync("SELECT pg_try_advisory_lock(@key)", key, cancellationToken).ConfigureAwait(false) is true; + + public async Task UnlockAsync(long key, CancellationToken cancellationToken) => + await ScalarAsync("SELECT pg_advisory_unlock(@key)", key, cancellationToken).ConfigureAwait(false) is true; + + public ValueTask DisposeAsync() => connection.DisposeAsync(); + + private async Task ScalarAsync(string sql, long key, CancellationToken cancellationToken) + { + await using var command = connection.CreateCommand(); + command.CommandText = sql; + command.Parameters.AddWithValue("@key", key); + + return await command.ExecuteScalarAsync(cancellationToken).ConfigureAwait(false); + } + } +} diff --git a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs index 7761a9ac6..1b7a0b527 100644 --- a/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs +++ b/backend/src/CodeSpace.Core/Services/Supervisor/SupervisorAcceptanceGrader.cs @@ -363,7 +363,7 @@ private async Task CloneAtBaseAsync(WorkspaceRequest clone, string baseSha, stri TokenedGitCommand.Spec(remote, new SandboxSpec { Command = "git", Args = new[] { "clone", remote.Url, directory }, WorkingDirectory = directory, TimeoutSeconds = CloneTimeoutSeconds, AllowNetwork = true }), cancellationToken).ConfigureAwait(false); if (cloneResult.Status != SandboxStatus.Success) - throw new WorkspaceException($"git clone failed (exit {cloneResult.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(cloneResult.Stderr), clone.Token)}"); + throw TokenedGitCommand.OldGitFailure(remote.IsTokened, cloneResult.Stderr) ?? new WorkspaceException($"git clone failed (exit {cloneResult.ExitCode}): {LocalGitWorkspaceProvider.Redact(Summarize(cloneResult.Stderr), clone.Token)}"); // Model-authored setup/acceptance commands run INSIDE this clone next. The clone named the remote without its // credential, so origin holds none; the strip stays as a belt, via the SAME shared helper LocalGitWorkspaceProvider's diff --git a/backend/src/CodeSpace.Messages/Health/GitReading.cs b/backend/src/CodeSpace.Messages/Health/GitReading.cs new file mode 100644 index 000000000..5d62ce266 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Health/GitReading.cs @@ -0,0 +1,4 @@ +namespace CodeSpace.Messages.Health; + +/// What git --version said on this host: the release when it could be read, and the sentence an operator reads either way — git's own output, or why there was none. +public sealed record GitReading(GitVersion? Version, string Detail); diff --git a/backend/src/CodeSpace.Messages/Health/GitVersion.cs b/backend/src/CodeSpace.Messages/Health/GitVersion.cs new file mode 100644 index 000000000..61dba7ac9 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Health/GitVersion.cs @@ -0,0 +1,34 @@ +using System.Text.RegularExpressions; + +namespace CodeSpace.Messages.Health; + +/// +/// A git release as git --version prints it: major, minor, patch. Vendor and pre-release suffixes +/// ((Apple Git-154), .windows.1, -rc1) are dropped, a two-part release reads as patch 0, and anything +/// that is not git version N.N is unknown (null) rather than a guess — a host whose release cannot be read +/// must never be called too old. +/// +public readonly partial record struct GitVersion(int Major, int Minor, int Patch) : IComparable +{ + public static GitVersion? Parse(string? versionOutput) + { + if (string.IsNullOrWhiteSpace(versionOutput)) return null; + + var match = ReleasePattern().Match(versionOutput); + + if (!match.Success) return null; + + var patch = match.Groups["patch"].Success ? int.Parse(match.Groups["patch"].Value) : 0; + + return new GitVersion(int.Parse(match.Groups["major"].Value), int.Parse(match.Groups["minor"].Value), patch); + } + + public int CompareTo(GitVersion other) => (Major, Minor, Patch).CompareTo((other.Major, other.Minor, other.Patch)); + + public bool IsAtLeast(GitVersion minimum) => CompareTo(minimum) >= 0; + + public override string ToString() => $"{Major}.{Minor}.{Patch}"; + + [GeneratedRegex(@"^\s*git version (?\d+)\.(?\d+)(?:\.(?\d+))?", RegexOptions.CultureInvariant)] + private static partial Regex ReleasePattern(); +} diff --git a/backend/src/CodeSpace.Messages/Health/HealthResponse.cs b/backend/src/CodeSpace.Messages/Health/HealthResponse.cs new file mode 100644 index 000000000..dbe18d587 --- /dev/null +++ b/backend/src/CodeSpace.Messages/Health/HealthResponse.cs @@ -0,0 +1,20 @@ +namespace CodeSpace.Messages.Health; + +/// +/// The body of GET /health: this process's build and role, so an API image and a worker image can be compared +/// from outside, and every health check's own finding. Additive to the probe surface — /health/live and +/// /health/ready keep their bodies and status codes. +/// +/// Two tiers on one anonymous endpoint. Everyone gets the build, the role and each check's name and status. A signed-in +/// global Admin also gets each check's description and data — the git release, the connection topology, the repair backlog — +/// which on an internet-facing API is reconnaissance, not something to print to a caller who has not signed in. +/// +/// The worst check's status: Healthy, Degraded or Unhealthy. +/// Which build this process is (version plus commit sha), as stamped on every log line. +/// The Hangfire role the deployment gave this process: Api or Worker. +/// Whether this process runs Hangfire servers. Every role serves HTTP; only Worker also processes the queue. +/// Whether each check's and are filled in: true for a signed-in global Admin, false for everyone else, whose entries carry only a name and a status. +public sealed record HealthResponse(string Status, string Build, string Role, bool ProcessesJobs, bool DetailsIncluded, IReadOnlyList Checks); + +/// One health check's finding. carries the facts behind the status, never a secret; both it and are empty unless . +public sealed record HealthCheckEntry(string Name, string Status, string? Description, IReadOnlyDictionary Data); diff --git a/backend/src/CodeSpace.Messages/Health/RefreshLockVerdict.cs b/backend/src/CodeSpace.Messages/Health/RefreshLockVerdict.cs new file mode 100644 index 000000000..c7d421c9c --- /dev/null +++ b/backend/src/CodeSpace.Messages/Health/RefreshLockVerdict.cs @@ -0,0 +1,17 @@ +namespace CodeSpace.Messages.Health; + +/// What the refresh-lock self-test found out about the Postgres connections this process opens. +public enum RefreshLockOutcome +{ + /// A second session could not take the advisory lock the first one held, and the first released it as its own: no sign that connections are multiplexed. A finding of absence, not a proof — a pooler that picks a server session per statement can slip past it. + Exclusive, + + /// The connections this process opens do not map one-to-one onto server sessions — a transaction-mode pooler, RDS Proxy, or a read-write splitting proxy. Either a second session took the advisory lock the first one held, or the first, unlocking the key it had just taken, was told it did not hold it. + NotExclusive, + + /// The test could not run to its end (no connection, a timeout). Nothing is known about the lock either way. + Inconclusive, +} + +/// The result of one refresh-lock self-test, with the sentence an operator reads and when it was observed. +public sealed record RefreshLockVerdict(RefreshLockOutcome Outcome, string Detail, DateTimeOffset ObservedAt); diff --git a/backend/tests/CodeSpace.E2ETests/Infrastructure/RecurringJobWorkerHostFactory.cs b/backend/tests/CodeSpace.E2ETests/Infrastructure/RecurringJobWorkerHostFactory.cs index ef1eb8d9e..da42324ac 100644 --- a/backend/tests/CodeSpace.E2ETests/Infrastructure/RecurringJobWorkerHostFactory.cs +++ b/backend/tests/CodeSpace.E2ETests/Infrastructure/RecurringJobWorkerHostFactory.cs @@ -1,5 +1,6 @@ using CodeSpace.Core.Persistence.Db; using CodeSpace.Core.Services.Agents.Sandbox.Runners; +using CodeSpace.Core.Services.Health; using CodeSpace.Core.Settings; using CodeSpace.Messages.Enums; using Microsoft.AspNetCore.Hosting; @@ -147,12 +148,12 @@ protected override void ConfigureWebHost(IWebHostBuilder builder) /// Appended AFTER Program.CreateHostBuilder's own UseSerilog(), so this host's /// ILoggerFactory registration is the last one and wins — every ILogger<T> the pipeline /// resolves (including TransactionalBehavior's) writes to the sink. The test proves that wiring with a - /// canary line before it trusts the absence of anything. Warning and above, except - /// at Information, which is the level of the worker's boot "Sandbox posture:" line. + /// canary line before it trusts the absence of anything. Warning and above, except and + /// at Information, the level of the worker's boot "Sandbox posture:" and "Boot posture:" lines. /// protected override IHost CreateHost(IHostBuilder builder) { - builder.UseSerilog(new LoggerConfiguration().MinimumLevel.Warning().MinimumLevel.Override(typeof(LocalProcessRunner).FullName!, LogEventLevel.Information).WriteTo.Sink(_logSink).CreateLogger(), dispose: true); + builder.UseSerilog(new LoggerConfiguration().MinimumLevel.Warning().MinimumLevel.Override(typeof(LocalProcessRunner).FullName!, LogEventLevel.Information).MinimumLevel.Override(typeof(BootPostureReporter).FullName!, LogEventLevel.Information).WriteTo.Sink(_logSink).CreateLogger(), dispose: true); return StartedHost = base.CreateHost(builder); } diff --git a/backend/tests/CodeSpace.E2ETests/Jobs/WorkerHealthReportE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Jobs/WorkerHealthReportE2ETests.cs new file mode 100644 index 000000000..3074e8c22 --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Jobs/WorkerHealthReportE2ETests.cs @@ -0,0 +1,129 @@ +using System.Collections.Concurrent; +using System.Net; +using System.Net.Http.Json; +using CodeSpace.Core.Services.Health; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Health; +using Serilog.Core; +using Serilog.Events; +using Shouldly; + +namespace CodeSpace.E2ETests.Jobs; + +/// +/// What a Worker-role pod says about itself, over HTTP and in its boot log, from the real worker host: the same fixture and +/// the same WorkerHangfireRegistrar that register the recurring jobs on a deployment's worker. The API-role half is +/// HealthEndpointE2ETests; together they are the "compare the two images from outside" surface. +/// +/// Tier: 🟢 High-fidelity (Rule 12) — real host, real Postgres, real Hangfire storage, the production health checks and +/// self-test. Substituted: the host runs as Development, like every sibling fixture (see +/// ), and the refresh-lock "pooler" case is covered at the integration tier because +/// no pooler is stood up here. +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Worker")] +public sealed class WorkerHealthReportE2ETests +{ + private static readonly TimeSpan BootLineBudget = TimeSpan.FromSeconds(30); + + [Fact] + public async Task A_worker_pod_reports_its_build_and_role_a_scheduled_sweep_and_an_exclusive_refresh_lock() + { + var sink = new CapturedEvents(); + + await using var factory = new RecurringJobWorkerHostFactory(sink); + await factory.InitializeAsync(); + + using var client = factory.CreateClient(); + var response = await client.GetAsync("/health"); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, "an anonymous request must be answered: a probe cannot sign in"); + + var anonymous = (await response.Content.ReadFromJsonAsync()).ShouldNotBeNull(); + + anonymous.Build.ShouldBe(CodeSpace.Core.Settings.Logging.BuildIdentity.Value, "the build /health names is the one every log line carries"); + anonymous.Role.ShouldBe("Worker"); + anonymous.ProcessesJobs.ShouldBeTrue(); + anonymous.DetailsIncluded.ShouldBeFalse(); + Entry(anonymous, "credential-repair-sweep").Status.ShouldBe("Healthy", "the Worker role registers every recurring job when it boots, so the sweep is scheduled"); + Entry(anonymous, "refresh-lock").Status.ShouldBe("Healthy"); + anonymous.Checks.ShouldAllBe(check => check.Description == null && check.Data.Count == 0); + + var report = await AdminReportAsync(factory); + var sweep = Entry(report, "credential-repair-sweep"); + + report.DetailsIncluded.ShouldBeTrue(); + sweep.Status.ShouldBe("Healthy", sweep.Description); + Fact(sweep, "registered").GetBoolean().ShouldBeTrue("the Worker role registers every recurring job when it boots, so the sweep is scheduled"); + Fact(sweep, "strandedCandidates").GetInt32().ShouldBe(0, "a fresh database has nothing to repair"); + + var refreshLock = Entry(report, "refresh-lock"); + + refreshLock.Status.ShouldBe("Healthy", refreshLock.Description); + Fact(refreshLock, "outcome").GetString().ShouldBe("Exclusive"); + } + + [Fact] + public async Task A_worker_pod_logs_its_boot_posture_once_naming_build_and_role() + { + var sink = new CapturedEvents(); + + await using var factory = new RecurringJobWorkerHostFactory(sink); + await factory.InitializeAsync(); + + _ = factory.Services; // builds and starts the host, which fires ApplicationStarted + + var line = await WaitForBootLineAsync(sink); + + line.Level.ShouldBe(LogEventLevel.Information); + line.Properties["Build"].ToString().Trim('"').ShouldBe(CodeSpace.Core.Settings.Logging.BuildIdentity.Value); + line.Properties["Role"].ToString().Trim('"').ShouldBe("Worker"); + line.Properties.ShouldContainKey("GitVersion"); + sink.Events().Count(IsBootPostureLine).ShouldBe(1, "the host reports once, however many times it is asked about"); + } + + /// The report as a signed-in global Admin reads it — the instance Admin role is a database row, so the account is seeded with it. + private static async Task AdminReportAsync(RecurringJobWorkerHostFactory factory) + { + var harness = new TenantHarness(factory); + var admin = await harness.AccountAsync(); + + await harness.MakeGlobalAdminAsync(admin); + + var response = await harness.SendAsync(HttpMethod.Get, "/health", admin, teamId: null); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response)); + + return (await response.Content.ReadFromJsonAsync()).ShouldNotBeNull(); + } + + private static HealthCheckEntry Entry(HealthResponse report, string name) => report.Checks.Single(check => check.Name == name); + + private static System.Text.Json.JsonElement Fact(HealthCheckEntry entry, string key) => ((System.Text.Json.JsonElement)entry.Data[key]); + + private static bool IsBootPostureLine(LogEvent logEvent) => logEvent.MessageTemplate.Text.StartsWith("Boot posture:", StringComparison.Ordinal); + + /// The report is made off the thread that starts the host, so the line arrives shortly after it; the wait is bounded and names what never came. + private static async Task WaitForBootLineAsync(CapturedEvents sink) + { + var deadline = DateTime.UtcNow + BootLineBudget; + + while (DateTime.UtcNow < deadline) + { + if (sink.Events().FirstOrDefault(IsBootPostureLine) is { } line) return line; + + await Task.Delay(50); + } + + throw new ShouldAssertException($"no \"Boot posture:\" line was logged within {BootLineBudget.TotalSeconds:0}s of the host starting. BootPostureReporter.ReportWhenStarted must be called from Startup.Configure, and the host's logger must let {nameof(BootPostureReporter)} through at Information."); + } + + private sealed class CapturedEvents : ILogEventSink + { + private readonly ConcurrentQueue _events = new(); + + public void Emit(LogEvent logEvent) => _events.Enqueue(logEvent); + + public IReadOnlyList Events() => _events.ToArray(); + } +} diff --git a/backend/tests/CodeSpace.E2ETests/Tasks/HealthEndpointE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Tasks/HealthEndpointE2ETests.cs index adc4eedef..ed60a24e1 100644 --- a/backend/tests/CodeSpace.E2ETests/Tasks/HealthEndpointE2ETests.cs +++ b/backend/tests/CodeSpace.E2ETests/Tasks/HealthEndpointE2ETests.cs @@ -1,5 +1,13 @@ using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Core.Settings.Logging; using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Options; using Shouldly; namespace CodeSpace.E2ETests.Tasks; @@ -10,6 +18,11 @@ namespace CodeSpace.E2ETests.Tasks; /// header — a kubelet probe is unauthenticated, so a probe that 401'd would mark every pod perpetually unready. /// /health/ready additionally runs the DbContext check (the test DB is up), proving readiness reflects real state. /// +/// /health is the report on top of that surface — this process's build and role and every check's finding, in two tiers: +/// anyone gets the build, the role and each check's status; a signed-in global Admin also gets each check's description and +/// data — and the probes' semantics must not have moved: while /health says Degraded, /health/ready is still the plain-text 200 it was, so no +/// orchestrator restarts or unroutes a pod over a finding. +/// /// Tier: 🟢 High-fidelity — real app host + real Postgres + the real middleware pipeline (auth, fallback /// policy, endpoint routing). /// @@ -40,4 +53,134 @@ public async Task Readiness_answers_200_anonymously_when_the_database_is_reachab response.StatusCode.ShouldBe(HttpStatusCode.OK, customMessage: "/health/ready must be anonymous and 200 when the DbContext check passes (the test Postgres is up)"); } + + [Fact] + public async Task The_probes_answer_exactly_what_they_always_did() + { + // Additive proof: the new checks carry no "ready" tag and the probes' writers are the default plain-text one, so a + // client that has always read these bodies reads the same bytes. + var live = await _factory.CreateClient().GetAsync("/health/live"); + var ready = await _factory.CreateClient().GetAsync("/health/ready"); + + live.StatusCode.ShouldBe(HttpStatusCode.OK); + (await live.Content.ReadAsStringAsync()).ShouldBe("Healthy"); + live.Content.Headers.ContentType?.MediaType.ShouldBe("text/plain"); + ready.StatusCode.ShouldBe(HttpStatusCode.OK); + (await ready.Content.ReadAsStringAsync()).ShouldBe("Healthy", "readiness runs only the \"ready\"-tagged checks — the database — whatever /health says"); + ready.Content.Headers.ContentType?.MediaType.ShouldBe("text/plain"); + } + + [Fact] + public void Readiness_still_runs_only_the_database_check_and_the_report_runs_all_four() + { + // The structural half of "probes unchanged": which checks the probe predicates select. The three new checks must carry + // no "ready" tag — one that did would let a git, lock or sweep finding pull a pod out of rotation. + var registrations = _factory.Services.GetRequiredService>().Value.Registrations; + + registrations.Where(registration => registration.Tags.Contains("ready")).Select(registration => registration.Name).ShouldBe(new[] { "db" }); + registrations.Select(registration => registration.Name).OrderBy(name => name, StringComparer.Ordinal).ShouldBe(new[] { "credential-repair-sweep", "db", "git", "refresh-lock" }); + } + + [Fact] + public async Task The_report_names_this_pods_build_and_role_and_each_checks_status_anonymously_and_nothing_more() + { + var response = await _factory.CreateClient().GetAsync("/health"); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, "a Degraded report is still a 200 — only an Unhealthy one is 503"); + response.Content.Headers.ContentType?.MediaType.ShouldBe("application/json"); + + var body = await response.Content.ReadAsStringAsync(); + var report = JsonSerializer.Deserialize(body, new JsonSerializerOptions(JsonSerializerDefaults.Web)).ShouldNotBeNull(); + + report.Build.ShouldBe(BuildIdentity.Value, "the build /health names is the one every log line carries"); + report.Role.ShouldBe("Api", "this fixture pins HangfireHosting=Api"); + report.ProcessesJobs.ShouldBeFalse(); + report.DetailsIncluded.ShouldBeFalse("nobody signed in"); + report.Checks.Select(check => check.Name).ShouldBe(new[] { "credential-repair-sweep", "db", "git", "refresh-lock" }); + Entry(report, "db").Status.ShouldBe("Healthy"); + Entry(report, "credential-repair-sweep").Status.ShouldBe("Degraded", "an API-only fixture has no sweep — the status says so without saying why"); + report.Checks.ShouldAllBe(check => check.Description == null && check.Data.Count == 0, "the findings' text and facts are for a signed-in Admin"); + body.ShouldNotContain("strandedCandidates", Case.Sensitive, "an internal backlog count is not for an anonymous caller"); + body.ShouldNotContain("PgBouncer", Case.Sensitive, "nor is the connection topology"); + body.ShouldNotContain("git version", Case.Insensitive); + } + + [Fact] + public async Task A_signed_in_global_admin_also_gets_every_checks_description_and_data() + { + var report = await AdminReportAsync(); + + report.DetailsIncluded.ShouldBeTrue(); + report.Build.ShouldBe(BuildIdentity.Value); + report.Checks.Select(check => check.Name).ShouldBe(new[] { "credential-repair-sweep", "db", "git", "refresh-lock" }); + Entry(report, "refresh-lock").Description.ShouldNotBeNull(); + Entry(report, "refresh-lock").Data.ShouldContainKey("outcome"); + } + + [Fact] + public async Task A_signed_in_account_that_is_not_a_global_admin_gets_the_anonymous_report() + { + var harness = new TenantHarness(_factory); + var account = await harness.AccountAsync(); + + var response = await harness.SendAsync(HttpMethod.Get, "/health", account, teamId: null); + var report = (await response.Content.ReadFromJsonAsync()).ShouldNotBeNull(); + + response.StatusCode.ShouldBe(HttpStatusCode.OK); + report.DetailsIncluded.ShouldBeFalse("being signed in is not enough; the instance Admin role is"); + report.Checks.ShouldAllBe(check => check.Description == null && check.Data.Count == 0); + } + + [Fact] + public async Task The_report_says_what_the_hosts_git_and_database_connections_can_do() + { + var report = await AdminReportAsync(); + + var git = Entry(report, "git"); + var host = HostGit.Current; + + // Through the real reader: whatever git this machine has, the entry must agree with the floor the credential helper has. + git.Status.ShouldBe(host.Version is { } version && version.IsAtLeast(TokenedGitCommand.MinimumGit) ? "Healthy" : "Degraded", git.Description); + if (host.Version is { } installed) Fact(git, "version").GetString().ShouldBe(installed.ToString()); + + var refreshLock = Entry(report, "refresh-lock"); + + refreshLock.Status.ShouldBe("Healthy", refreshLock.Description); + Fact(refreshLock, "outcome").GetString().ShouldBe("Exclusive", "the test Postgres is reached directly, so a second session is refused the held key and the holder's unlock lands on its own session"); + } + + [Fact] + public async Task An_api_only_deployment_reports_the_repair_sweep_as_not_registered_without_touching_readiness() + { + // This fixture is the API role with no worker against its database: nothing registered the sweep, and /health says so. + var report = await AdminReportAsync(); + + var sweep = Entry(report, "credential-repair-sweep"); + + sweep.Status.ShouldBe("Degraded"); + sweep.Description.ShouldNotBeNull().ShouldContain("not registered"); + Fact(sweep, "registered").GetBoolean().ShouldBeFalse(); + report.Status.ShouldBe("Degraded", "the worst check names the report's status"); + + (await _factory.CreateClient().GetAsync("/health/ready")).StatusCode.ShouldBe(HttpStatusCode.OK, "a finding on /health must never pull a pod out of rotation"); + } + + /// The report as a signed-in global Admin reads it: the instance Admin role is a database row, so the account is seeded with it and sends its own token. + private async Task AdminReportAsync() + { + var harness = new TenantHarness(_factory); + var admin = await harness.AccountAsync(); + + await harness.MakeGlobalAdminAsync(admin); + + var response = await harness.SendAsync(HttpMethod.Get, "/health", admin, teamId: null); + + response.StatusCode.ShouldBe(HttpStatusCode.OK, await TenantHarness.DescribeAsync(response)); + + return (await response.Content.ReadFromJsonAsync()).ShouldNotBeNull(); + } + + private static HealthCheckEntry Entry(HealthResponse report, string name) => report.Checks.Single(check => check.Name == name); + + private static JsonElement Fact(HealthCheckEntry entry, string key) => (JsonElement)entry.Data[key]; } diff --git a/backend/tests/CodeSpace.E2ETests/Tasks/HealthReportDisclosureE2ETests.cs b/backend/tests/CodeSpace.E2ETests/Tasks/HealthReportDisclosureE2ETests.cs new file mode 100644 index 000000000..6d6dfe08e --- /dev/null +++ b/backend/tests/CodeSpace.E2ETests/Tasks/HealthReportDisclosureE2ETests.cs @@ -0,0 +1,81 @@ +using System.Net; +using System.Net.Http.Json; +using CodeSpace.Core.Services.Health.Checks; +using CodeSpace.E2ETests.Infrastructure; +using CodeSpace.Messages.Health; +using Hangfire; +using Microsoft.Extensions.DependencyInjection; +using Shouldly; + +namespace CodeSpace.E2ETests.Tasks; + +/// +/// What GET /health prints about a scheduler fault, over the real pipeline and from real Hangfire storage. Hangfire's +/// scheduler stores the exception chain of a failed schedule — up to a hundred stack frames, and for a storage fault the +/// server's host and port — in the recurring job's Error field. The report says an error was recorded and never repeats +/// it, for an anonymous caller and for a global Admin alike: the full text is in the pod's log. +/// +/// Own fixture, own database: the entry written here would otherwise change what every other health test sees, and the +/// check reuses a reading for thirty seconds. +/// +/// Tier: 🟢 High-fidelity — real app host, real Postgres, the real middleware pipeline and the production sweep check, +/// over a schedule entry written field for field the way Hangfire's scheduler leaves a disabled one +/// (WorkerFleetCapabilityE2ETests does the same for the whole schedule). +/// +[Trait("Category", "E2E")] +[Trait("Surface", "Http")] +public sealed class HealthReportDisclosureE2ETests : IClassFixture +{ + private const string StorageFault = + "Npgsql.NpgsqlException: Failed to connect to 10.20.30.40:5432\n ---> System.Net.Sockets.SocketException (111): Connection refused\n" + + " at Npgsql.Internal.NpgsqlConnector.Connect(NpgsqlTimeout timeout)\n at Hangfire.PostgreSql.PostgreSqlJobQueue.Enqueue(String queue, String jobId)"; + + private readonly TaskLaunchApiFactory _factory; + + public HealthReportDisclosureE2ETests(TaskLaunchApiFactory factory) { _factory = factory; } + + [Fact] + public async Task A_scheduler_error_stored_with_a_storage_address_and_stack_frames_reaches_neither_an_anonymous_caller_nor_an_admin() + { + DisableTheSweepAsAFailingSchedulerDoes(); + + var anonymous = await _factory.CreateClient().GetAsync("/health"); + var anonymousBody = await anonymous.Content.ReadAsStringAsync(); + + anonymous.StatusCode.ShouldBe(HttpStatusCode.OK); + ShouldHoldNothingOfTheFault(anonymousBody); + + var harness = new TenantHarness(_factory); + var admin = await harness.AccountAsync(); + + await harness.MakeGlobalAdminAsync(admin); + + var response = await harness.SendAsync(HttpMethod.Get, "/health", admin, teamId: null); + var adminBody = await response.Content.ReadAsStringAsync(); + var report = (await response.Content.ReadFromJsonAsync()).ShouldNotBeNull(); + + ShouldHoldNothingOfTheFault(adminBody); + report.Checks.Single(check => check.Name == "credential-repair-sweep").Description.ShouldNotBeNull().ShouldContain("recorded an error", customMessage: "the Admin is told there is one, and where its text is"); + } + + private void DisableTheSweepAsAFailingSchedulerDoes() + { + var storage = _factory.Services.GetRequiredService(); + + using var connection = storage.GetConnection(); + using var transaction = connection.CreateWriteTransaction(); + + transaction.SetRangeInHash($"recurring-job:{CredentialRepairSweepHealthCheck.JobId}", new Dictionary { ["NextExecution"] = string.Empty, ["Error"] = StorageFault, ["RetryAttempt"] = "5" }); + transaction.AddToSet("recurring-jobs", CredentialRepairSweepHealthCheck.JobId, -1.0); + transaction.Commit(); + } + + private static void ShouldHoldNothingOfTheFault(string body) + { + body.ShouldNotContain("10.20.30.40", Case.Sensitive, "a storage host and port stay in the log"); + body.ShouldNotContain("Npgsql", Case.Sensitive); + body.ShouldNotContain("PostgreSqlJobQueue", Case.Sensitive); + body.ShouldNotContain("SocketException", Case.Sensitive); + body.ShouldNotContain(" at ", Case.Sensitive, "no stack frame"); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Health/CredentialRepairSweepHealthFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Health/CredentialRepairSweepHealthFlowTests.cs new file mode 100644 index 000000000..f16e6e02f --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Health/CredentialRepairSweepHealthFlowTests.cs @@ -0,0 +1,124 @@ +using Autofac; +using Autofac.Extensions.DependencyInjection; +using CodeSpace.Api.Extensions.Hangfire; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Health.Checks; +using CodeSpace.Core.Services.Jobs; +using CodeSpace.IntegrationTests.Fleet; +using CodeSpace.IntegrationTests.Infrastructure; +using Hangfire; +using Hangfire.Common; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Health; + +/// +/// The credential repair sweep's health entry read from real Hangfire storage on Postgres, over the schedule the production +/// RecurringJobSchedule writes — so what the check reads is what a Worker pod leaves behind, field for field, and an +/// API pod's report describes it. The backlog is the production census over the fixture's database. +/// +/// Tier: 🟢 High-fidelity (Rule 12) for the schedule and the backlog. What no test here can produce is a worker's +/// scheduler actually firing the entry (that is RecurringJobWorkerSmokeE2ETests), so a "last run" is read only from +/// the unit tier; here the schedule's next run is moved the way time moves it. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class CredentialRepairSweepHealthFlowTests +{ + private readonly PostgresFixture _fixture; + + public CredentialRepairSweepHealthFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Fact] + public async Task A_schedule_a_worker_registered_reads_back_healthy_with_its_cadence_and_the_live_backlog() + { + using var fleet = new HangfireFleetHarness(_fixture.ConnectionString); + using var scope = BeginScopeOnRealStorage(fleet); + RecurringJobSchedule.Assert(new AutofacServiceProvider(scope)); + + var result = await Check(scope, fleet).CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Healthy, result.Description); + result.Data["registered"].ShouldBe(true); + result.Data["cron"].ShouldBe("*/5 * * * *"); + ((DateTime)result.Data["nextExecutionUtc"]).ShouldBeGreaterThan(DateTime.UtcNow.AddSeconds(-5), "a freshly registered entry is due in the future"); + result.Data["strandedCandidates"].ShouldBe(await scope.Resolve().CountAsync(CancellationToken.None), "the backlog is the census over this database"); + } + + [Fact] + public async Task A_database_no_worker_registered_the_sweep_in_reads_back_as_not_registered() + { + // The API pod's view of an API-only fleet, or of workers whose build predates the sweep: Hangfire holds no entry. + using var fleet = new HangfireFleetHarness(_fixture.ConnectionString); + using var scope = BeginScopeOnRealStorage(fleet); + + var result = await Check(scope, fleet).CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("not registered"); + result.Data["registered"].ShouldBe(false); + } + + [Fact] + public async Task An_entry_a_previous_version_scheduler_disabled_reads_back_as_disabled_without_the_error_text_it_left() + { + using var fleet = new HangfireFleetHarness(_fixture.ConnectionString); + using var scope = BeginScopeOnRealStorage(fleet); + RecurringJobSchedule.Assert(new AutofacServiceProvider(scope)); + + // Field for field what RecurringJobScheduler leaves when it cannot load a job type (RecurringJobScheduleKeeperFlowTests). + Write(fleet, new Dictionary { ["NextExecution"] = string.Empty, ["Error"] = "Could not load the job: no such type", ["RetryAttempt"] = "5" }); + + var result = await Check(scope, fleet).CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("disabled"); + result.Description.ShouldContain("recorded an error"); + result.Description.ShouldNotContain("Could not load the job", customMessage: "the stored error is an exception chain that can name the storage host; the report never repeats it"); + } + + [Fact] + public async Task A_schedule_whose_next_run_slipped_far_into_the_past_reads_back_as_overdue() + { + using var fleet = new HangfireFleetHarness(_fixture.ConnectionString); + using var scope = BeginScopeOnRealStorage(fleet); + RecurringJobSchedule.Assert(new AutofacServiceProvider(scope)); + + Write(fleet, new Dictionary { ["NextExecution"] = JobHelper.SerializeDateTime(DateTime.UtcNow.AddMinutes(-30)) }); + + var result = await Check(scope, fleet).CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("overdue"); + } + + private static CredentialRepairSweepHealthCheck Check(ILifetimeScope scope, HangfireFleetHarness fleet) => new(new ChildScopeFactory(scope), TimeProvider.System, NullLogger.Instance, fleet.Storage); + + /// Fields of the sweep's recurring entry, written with Hangfire's own write transaction. + private static void Write(HangfireFleetHarness fleet, Dictionary fields) + { + using var connection = fleet.Storage.GetConnection(); + using var transaction = connection.CreateWriteTransaction(); + + transaction.SetRangeInHash($"recurring-job:{CredentialRepairSweepHealthCheck.JobId}", fields); + transaction.Commit(); + } + + private ILifetimeScope BeginScopeOnRealStorage(HangfireFleetHarness fleet) => + fleet.BeginScope(_fixture, builder => builder.RegisterInstance(new CodeSpaceBackgroundJobClient(() => new BackgroundJobClient(fleet.Storage), () => new RecurringJobManager(fleet.Storage))).As()); + + private sealed class ChildScopeFactory(ILifetimeScope parent) : IServiceScopeFactory + { + public IServiceScope CreateScope() => new ChildScope(parent.BeginLifetimeScope()); + } + + private sealed class ChildScope(ILifetimeScope scope) : IServiceScope + { + public IServiceProvider ServiceProvider { get; } = new AutofacServiceProvider(scope); + + public void Dispose() => scope.Dispose(); + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/Health/RefreshLockSelfTestFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/Health/RefreshLockSelfTestFlowTests.cs new file mode 100644 index 000000000..03bd4e026 --- /dev/null +++ b/backend/tests/CodeSpace.IntegrationTests/Health/RefreshLockSelfTestFlowTests.cs @@ -0,0 +1,159 @@ +using CodeSpace.Core.Services.Health; +using CodeSpace.Core.Services.Health.Sessions; +using CodeSpace.Core.Settings.Database; +using CodeSpace.IntegrationTests.Infrastructure; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Configuration; +using Npgsql; +using Shouldly; + +namespace CodeSpace.IntegrationTests.Health; + +/// +/// The refresh-lock self-test against a real Postgres, through the production that +/// opens its connections from the same connection string PostgresAdvisoryLock does. Two real sessions must refuse each +/// other; two "clients" that share one session — what a transaction-mode pooler hands out — must be caught, because on that +/// session pg_advisory_lock is re-entrant and refresh quietly stops being serialized. +/// +/// Tier: 🟡 Medium-mock (Rule 12) for the pooler: the topology that breaks the lock cannot be stood up without +/// installing one, so "shares a session" is modelled by handing the SAME real session to both roles. Everything else — the +/// server, the connections, the lock calls, the self-test — is production. +/// +[Collection(PostgresCollection.Name)] +[Trait("Category", "Integration")] +public sealed class RefreshLockSelfTestFlowTests +{ + private readonly PostgresFixture _fixture; + + public RefreshLockSelfTestFlowTests(PostgresFixture fixture) { _fixture = fixture; } + + [Fact] + public async Task Two_real_sessions_refuse_each_other_so_the_lock_is_exclusive_and_nothing_is_left_held() + { + var before = await AdvisoryLocksHeldInThisDatabaseAsync(); + + var verdict = await NewSelfTest(RealSessions(_fixture.ConnectionString)).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Exclusive, verdict.Detail); + (await AdvisoryLocksHeldInThisDatabaseAsync()).ShouldBe(before, "the test closed its sessions, which released every key it took"); + } + + [Fact] + public async Task Two_clients_that_share_one_real_session_are_reported_as_not_exclusive() + { + var before = await AdvisoryLocksHeldInThisDatabaseAsync(); + + await using (var shared = await RealSessions(_fixture.ConnectionString).OpenAsync(CancellationToken.None)) + { + var verdict = await NewSelfTest(new OneSessionForEveryone(shared)).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.NotExclusive, "a session may take its own advisory lock again, so the second client was not refused"); + verdict.Detail.ShouldContain("PgBouncer"); + } + + (await AdvisoryLocksHeldInThisDatabaseAsync()).ShouldBe(before, "the shared session's lock was taken twice and must have been given back twice"); + } + + [Fact] + public async Task The_production_session_behaves_as_the_self_test_assumes() + { + var key = Random.Shared.NextInt64(); + var sessions = RealSessions(_fixture.ConnectionString); + + await using var first = await sessions.OpenAsync(CancellationToken.None); + await using var second = await sessions.OpenAsync(CancellationToken.None); + + await first.LockAsync(key, CancellationToken.None); + + (await second.TryLockAsync(key, CancellationToken.None)).ShouldBeFalse("another session holds it"); + (await first.TryLockAsync(key, CancellationToken.None)).ShouldBeTrue("the holder may take its own lock again — the property a shared session turns into a non-lock"); + + (await first.UnlockAsync(key, CancellationToken.None)).ShouldBeTrue("the holder released a hold it had"); + (await first.UnlockAsync(key, CancellationToken.None)).ShouldBeTrue("it held the key twice"); + (await first.UnlockAsync(key, CancellationToken.None)).ShouldBeFalse("a third unlock releases nothing: Postgres answers false, which is how a pooler's misrouted unlock shows itself"); + + (await second.TryLockAsync(key, CancellationToken.None)).ShouldBeTrue("released by both unlocks"); + + (await second.UnlockAsync(key, CancellationToken.None)).ShouldBeTrue(); + } + + [Fact] + public async Task An_unlock_that_lands_on_another_real_session_is_reported_as_not_exclusive_even_though_the_contender_was_refused() + { + // Medium-mock (Rule 12): what a statement-level pooler does to the holder's unlock, modelled by sending it down a different real session. + var before = await AdvisoryLocksHeldInThisDatabaseAsync(); + var real = RealSessions(_fixture.ConnectionString); + + await using (var holderServer = await real.OpenAsync(CancellationToken.None)) + await using (var elsewhere = await real.OpenAsync(CancellationToken.None)) + await using (var contender = await real.OpenAsync(CancellationToken.None)) + { + var verdict = await NewSelfTest(new MisroutedUnlocks(holderServer, elsewhere, contender)).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.NotExclusive, "Postgres told the holder's unlock that its session did not hold the key"); + verdict.Detail.ShouldContain("did not hold"); + } + + (await AdvisoryLocksHeldInThisDatabaseAsync()).ShouldBe(before, "closing the sessions released the lock the misrouted unlocks left behind"); + } + + [Fact] + public async Task A_server_that_cannot_be_reached_is_inconclusive_at_once_and_names_no_address() + { + var unreachable = new NpgsqlConnectionStringBuilder(_fixture.ConnectionString) { Host = "127.0.0.1", Port = 1, Timeout = 3 }.ConnectionString; + + var verdict = await NewSelfTest(RealSessions(unreachable)).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Inconclusive); + verdict.Detail.ShouldNotContain("127.0.0.1"); + } + + private static NpgsqlAdvisoryLockSessions RealSessions(string connectionString) => + new(new CodeSpaceConnectionString(new ConfigurationBuilder().AddInMemoryCollection(new Dictionary { ["CodeSpaceStore:ConnectionString"] = connectionString }).Build())); + + private static RefreshLockSelfTest NewSelfTest(IAdvisoryLockSessions sessions) => + new(sessions, TimeProvider.System, Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + + /// The advisory locks any session holds in the fixture's own database. The suite's tests in this collection run one at a time, so before and after are comparable. + private async Task AdvisoryLocksHeldInThisDatabaseAsync() + { + await using var connection = new NpgsqlConnection(_fixture.ConnectionString); + await connection.OpenAsync(); + + await using var command = new NpgsqlCommand("SELECT count(*) FROM pg_locks WHERE locktype = 'advisory' AND database = (SELECT oid FROM pg_database WHERE datname = current_database())", connection); + + return (long)(await command.ExecuteScalarAsync())!; + } + + /// The self-test's holder takes its lock on holder but every unlock it sends runs on elsewhere; the contender is its own real session. None is disposed here — the test owns them. + private sealed class MisroutedUnlocks(IAdvisoryLockSession holder, IAdvisoryLockSession elsewhere, IAdvisoryLockSession contender) : IAdvisoryLockSessions + { + private int _opened; + + public Task OpenAsync(CancellationToken cancellationToken) => + Task.FromResult(++_opened == 1 ? new Misrouted(holder, elsewhere) : new Borrowed(contender)); + + private sealed class Misrouted(IAdvisoryLockSession lockSession, IAdvisoryLockSession unlockSession) : IAdvisoryLockSession + { + public Task LockAsync(long key, CancellationToken cancellationToken) => lockSession.LockAsync(key, cancellationToken); + public Task TryLockAsync(long key, CancellationToken cancellationToken) => lockSession.TryLockAsync(key, cancellationToken); + public Task UnlockAsync(long key, CancellationToken cancellationToken) => unlockSession.UnlockAsync(key, cancellationToken); + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } + } + + /// Both roles of the self-test get the one session — the connection a transaction-mode pooler would have handed to both clients. Not disposed by either; the test owns it. + private sealed class OneSessionForEveryone(IAdvisoryLockSession session) : IAdvisoryLockSessions + { + public Task OpenAsync(CancellationToken cancellationToken) => Task.FromResult(new Borrowed(session)); + } + + /// A session lent to the self-test: disposing it leaves the real one open for its owner. + private sealed class Borrowed(IAdvisoryLockSession inner) : IAdvisoryLockSession + { + public Task LockAsync(long key, CancellationToken cancellationToken) => inner.LockAsync(key, cancellationToken); + public Task TryLockAsync(long key, CancellationToken cancellationToken) => inner.TryLockAsync(key, cancellationToken); + public Task UnlockAsync(long key, CancellationToken cancellationToken) => inner.UnlockAsync(key, cancellationToken); + public ValueTask DisposeAsync() => ValueTask.CompletedTask; + } +} diff --git a/backend/tests/CodeSpace.IntegrationTests/OAuth/ProviderReconnectFlowTests.cs b/backend/tests/CodeSpace.IntegrationTests/OAuth/ProviderReconnectFlowTests.cs index 5cd642555..1a95c5197 100644 --- a/backend/tests/CodeSpace.IntegrationTests/OAuth/ProviderReconnectFlowTests.cs +++ b/backend/tests/CodeSpace.IntegrationTests/OAuth/ProviderReconnectFlowTests.cs @@ -232,6 +232,34 @@ public async Task The_repair_sweep_moves_a_stranded_binding_only_to_its_owners_r WasDispatched(hookId).ShouldBeTrue("the revived hook is handed to the registrar, not left Pending for a bind that may never come"); } + [Theory] + [InlineData(StrandedShape.ReconnectedAfterTheDisconnect, true)] + [InlineData(StrandedShape.TokenLinkedInTheSameSave, true)] + [InlineData(StrandedShape.OnlyAnOlderLiveCredential, false)] + [InlineData(StrandedShape.ReconnectedByAnotherMember, false)] + [InlineData(StrandedShape.ReconnectedThenDisconnectedAgain, false)] + [InlineData(StrandedShape.BoundToALiveCredentialWithANewerSibling, false)] + public async Task The_census_counts_exactly_the_shapes_the_sweep_moves_and_falls_as_it_moves_them(StrandedShape shape, bool counted) + { + // The health report's backlog is only worth reading if it is the sweep's own candidate set. The shapes are the sweep + // theory's, so a rule changed for one and not the other fails here; the baseline is taken after a drain, so rows other + // tests in this shared database left cannot move it. + var world = await SeedWorldAsync(); + + using var dispatches = Jobs().ManualExecution(); + await SweepUntilDrainedAsync(); + var baseline = await CountStrandedAsync(); + + var (bound, _) = await SeedStrandedCredentialsAsync(world, shape); + await SeedRepositoryAsync(world, bound, shape is StrandedShape.TokenLinkedInTheSameSave or StrandedShape.BoundToALiveCredentialWithANewerSibling ? RepositoryStatus.Active : RepositoryStatus.Error); + + (await CountStrandedAsync()).ShouldBe(baseline + (counted ? 1 : 0), $"{shape}: counted only when the sweep would move it"); + + await SweepUntilDrainedAsync(); + + (await CountStrandedAsync()).ShouldBe(baseline, $"{shape}: a moved binding leaves the backlog, and one that stays was never in it"); + } + [Fact] public async Task The_repair_sweep_moves_a_group_hook_that_is_the_disconnected_credentials_only_binding() { @@ -314,6 +342,14 @@ private async Task SweepUntilDrainedAsync() throw new ShouldAssertException($"RepairDisconnectedCredentialBindingsCommand still repaired bindings after {MaxSweepPasses} passes — a repaired credential is not leaving the candidate set. Check that the sweep's candidate query drops a revoked credential once nothing live is bound to it."); } + /// The health report's backlog: what the sweep would take, counted without its batch bound. + private async Task CountStrandedAsync() + { + using var scope = _fixture.BeginScope(); + + return await scope.Resolve().CountAsync(CancellationToken.None); + } + /// The Personal tab's "Disconnect". private async Task DisconnectAsync(World world, Guid credentialId) { diff --git a/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs index 6837970a2..327c383dd 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/LocalGitBranchIntegratorTests.cs @@ -1,7 +1,9 @@ using CodeSpace.Core.Services.Agents.Sandbox; +using CodeSpace.Core.Services.Agents.Workspace; using CodeSpace.Core.Services.Agents.Workspace.Integrators; using CodeSpace.Core.Services.Workflows.Artifacts; using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Health; using Microsoft.Extensions.Logging.Abstractions; using Shouldly; @@ -134,6 +136,57 @@ await integrator.IntegrateAsync(new IntegrationRequest runner.Specs.Where(s => TokenedGitSpecs.ArgvCarriesACredential(s, "test-token")).Select(s => string.Join(' ', s.Args)).ShouldBeEmpty(); } + [Fact] + public async Task A_tokened_clone_refused_for_want_of_a_credential_by_a_git_before_2_31_fails_with_the_cause() + { + var runner = new RefusedCloneRunner("fatal: could not read Username for 'https://example.test': terminal prompts disabled"); + var integrator = new LocalGitBranchIntegrator(new SandboxRunnerRegistry(new ISandboxRunner[] { runner }), new InlineOffloader(), NullLogger.Instance); + using var _ = HostGit.Use(new GitReading(new GitVersion(2, 30, 2), "git version 2.30.2")); + + var failure = await Should.ThrowAsync(() => integrator.IntegrateAsync(new IntegrationRequest + { + TeamId = Guid.NewGuid(), RepositoryUrl = "https://example.test/repo.git", BaseSha = "base", Token = "test-token", IntegrationBranch = "codespace/integration/run", + Contributions = new[] { new BranchContribution { Label = "agent", BaseSha = "base", Patch = "diff --git a/f.txt b/f.txt\n--- a/f.txt\n+++ b/f.txt\n@@ -1 +1 @@\n-a\n+b\n" } }, + }, CancellationToken.None)); + + failure.Message.ShouldContain("2.30.2"); + runner.Specs.ShouldContain(spec => Subcommand(spec) == "clone", "fixture check: the refusal came from the clone"); + } + + [Fact] + public async Task A_tokened_push_refused_for_want_of_a_credential_by_a_git_before_2_31_fails_with_the_cause() + { + var runner = new RefusedCloneRunner("fatal: could not read Username for 'https://example.test': terminal prompts disabled", refuses: "push"); + var integrator = new LocalGitBranchIntegrator(new SandboxRunnerRegistry(new ISandboxRunner[] { runner }), new InlineOffloader(), NullLogger.Instance); + using var _ = HostGit.Use(new GitReading(new GitVersion(2, 30, 2), "git version 2.30.2")); + + await Should.ThrowAsync(() => integrator.IntegrateAsync(new IntegrationRequest + { + TeamId = Guid.NewGuid(), RepositoryUrl = "https://example.test/repo.git", BaseSha = "base", Token = "test-token", IntegrationBranch = "codespace/integration/run", + Contributions = new[] { new BranchContribution { Label = "agent", BaseSha = "base", Patch = "diff --git a/f.txt b/f.txt\n--- a/f.txt\n+++ b/f.txt\n@@ -1 +1 @@\n-a\n+b\n" } }, + }, CancellationToken.None)); + + runner.Specs.ShouldContain(spec => Subcommand(spec) == "push", "fixture check: the refusal came from the push, after a clean apply"); + } + + /// Refuses one git subcommand (the clone, unless told otherwise) the way git does for a remote that wanted a credential and got none; every other command answers as an integration would (the branch is new, the rest succeed). + private sealed class RefusedCloneRunner(string stderr, string refuses = "clone") : ISandboxRunner + { + public string Kind => "local"; + public List Specs { get; } = new(); + + public Task RunAsync(SandboxSpec spec, CancellationToken cancellationToken) + { + Specs.Add(spec); + + if (Subcommand(spec) == refuses) return Task.FromResult(new SandboxResult { Status = SandboxStatus.Failed, ExitCode = 128, Stdout = "", Stderr = stderr }); + + var absent = spec.Args.Contains("--quiet") || spec.Args.Contains("--verify"); + + return Task.FromResult(new SandboxResult { Status = absent ? SandboxStatus.Failed : SandboxStatus.Success, ExitCode = absent ? 1 : 0, Stdout = "", Stderr = "" }); + } + } + /// The git subcommand, past any leading -c key=value and -C dir. private static string Subcommand(SandboxSpec spec) { diff --git a/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs index 23e59cd25..da9bf70e8 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/PackCloneFetcherCredentialTests.cs @@ -3,6 +3,7 @@ using CodeSpace.Core.Services.Agents.Workspace; using CodeSpace.Core.Services.Agents.Workspace.Providers; using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Health; using Microsoft.Extensions.Logging.Abstractions; using Shouldly; @@ -114,6 +115,34 @@ public async Task A_failed_clone_throws_without_the_credential_and_leaves_no_clo Directory.Exists(runner.Specs.Single().WorkingDirectory).ShouldBeFalse("a failed clone is reclaimed before the throw"); } + [Theory] + [InlineData(Tokened, "git version 2.30.2", true)] + [InlineData("https://fake-pasted-token-0123456789@github.com/owner/repo.git", "git version 2.30.2", true)] // a token pasted as the user alone is a credential too + [InlineData(Tokened, "git version 2.39.5", false)] // a git that reads GIT_CONFIG_COUNT: the credential really was refused + [InlineData("https://github.com/owner/repo.git", "git version 2.30.2", false)] // nothing was pasted: an old git is not the explanation + public async Task A_clone_the_remote_asked_a_credential_for_names_an_old_git_only_when_a_credential_was_pasted_and_git_is_old(string url, string gitOutput, bool oldGitIsTheCause) + { + var runner = new ScriptedRunner { CloneStderr = "fatal: could not read Username for 'https://github.com': terminal prompts disabled" }; + using var _ = HostGit.Use(new GitReading(GitVersion.Parse(gitOutput), gitOutput)); + + var failure = await Should.ThrowAsync(() => Fetcher(runner).FetchAsync(url, null, CancellationToken.None)); + + (failure.InnerException is GitCredentialHelperUnsupportedException).ShouldBe(oldGitIsTheCause, failure.Message); + failure.Message.ShouldNotContain(Marker); + failure.Message.ShouldStartWith("git clone of 'https://github.com/owner/repo.git' failed", Case.Sensitive, "the URL is still named, without its userinfo"); + + if (oldGitIsTheCause) + { + failure.Message.ShouldContain("2.30.2"); + failure.Message.ShouldContain("GIT_CONFIG_COUNT"); + failure.Message.ShouldNotContain("could not read Username", Case.Sensitive, "the message names the cause in place of git's own words"); + } + else + { + failure.Message.ShouldContain("could not read Username", Case.Sensitive, "otherwise the message is exactly what it always was"); + } + } + [Theory] [InlineData(Tokened)] [InlineData("https://fake-pasted-token-0123456789@github.com/owner/repo.git")] // a token pasted as the user alone diff --git a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs index f567c9936..d5058d104 100644 --- a/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Agents/SupervisorAcceptanceGraderTests.cs @@ -140,6 +140,24 @@ public async Task A_null_clone_request_fails_closed() grade.Detail.ShouldContain("clone-failed"); } + [Fact] + public async Task A_tokened_clone_refused_for_want_of_a_credential_by_a_git_before_2_31_fails_closed_with_the_cause() + { + var runners = new RecordingRunnerRegistry(); + runners.Script(new SandboxResult { Status = SandboxStatus.Failed, ExitCode = 128, Stdout = "", Stderr = "fatal: could not read Username for 'https://gitlab.example.test': terminal prompts disabled" }); + var grader = Build(new FakeResolver(new WorkspaceRequest { RepositoryUrl = "https://gitlab.example.test/a/b.git", Token = "test-token" }), new FakeGrader(Pass), runners: runners); + using var _ = CodeSpace.Core.Services.Agents.Workspace.HostGit.Use(new CodeSpace.Messages.Health.GitReading(new CodeSpace.Messages.Health.GitVersion(2, 30, 2), "git version 2.30.2")); + + var grade = await grader.GradeBaseAsync(Guid.NewGuid(), Guid.NewGuid(), "b", Spec(), 30, CancellationToken.None); + + runners.Invocations.ShouldHaveSingleItem().Args.ShouldContain("clone", customMessage: "fixture check: the refusal came from the clone"); + grade.Passed.ShouldBeFalse(); + grade.Detail.ShouldStartWith("clone-failed:"); + grade.Detail.ShouldContain("2.30.2"); + grade.Detail.ShouldContain("GIT_CONFIG_COUNT"); + grade.Class.ShouldBe(GradeFailureClass.Environment); + } + [Fact] public async Task A_resolve_failure_fails_closed_without_throwing() { diff --git a/backend/tests/CodeSpace.UnitTests/Health/BootPostureReporterTests.cs b/backend/tests/CodeSpace.UnitTests/Health/BootPostureReporterTests.cs new file mode 100644 index 000000000..cab85e377 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/BootPostureReporterTests.cs @@ -0,0 +1,164 @@ +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Core.Services.Health; +using CodeSpace.Messages.Enums; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// +/// The line a pod prints once it is serving: build, role, git. It is the log-side twin of /health — what lets two +/// pods be compared from a log search when nothing can reach their health endpoint — and it starts the refresh-lock +/// self-test so a lock that is not exclusive is an Error at boot rather than a finding nobody asked for. +/// +[Trait("Category", "Unit")] +public sealed class BootPostureReporterTests +{ + [Fact] + public async Task It_logs_one_line_naming_the_build_the_role_and_git() + { + var logger = new CapturingLogger(); + var reporter = NewReporter(new HostPosture(HangfireHosting.Api, "1.0.0+abc123"), logger); + using var _ = HostGit.Use(Reading("git version 2.39.5")); + + await reporter.ReportAsync(CancellationToken.None); + + var line = logger.Entries.Where(e => e.Message.StartsWith("Boot posture:", StringComparison.Ordinal)).ToList().ShouldHaveSingleItem(); + + line.Level.ShouldBe(LogLevel.Information); + line.Properties["Build"].ShouldBe("1.0.0+abc123"); + line.Properties["Role"].ShouldBe("Api"); + line.Properties["GitVersion"].ShouldBe("2.39.5"); + line.Message.ShouldContain("1.0.0+abc123"); + line.Message.ShouldContain("Api"); + line.Message.ShouldContain("2.39.5"); + } + + [Fact] + public async Task A_git_below_the_floor_is_also_a_Warning_with_the_cause() + { + var logger = new CapturingLogger(); + using var _ = HostGit.Use(Reading("git version 2.30.2")); + + await NewReporter(new HostPosture(HangfireHosting.Worker, "1.0.0+abc123"), logger).ReportAsync(CancellationToken.None); + + var warning = logger.Entries.Where(e => e.Level == LogLevel.Warning).ToList().ShouldHaveSingleItem(); + + warning.Message.ShouldContain("2.30.2"); + warning.Message.ShouldContain("GIT_CONFIG_COUNT"); + } + + [Fact] + public async Task A_current_git_logs_no_Warning() + { + var logger = new CapturingLogger(); + using var _ = HostGit.Use(Reading("git version 2.39.5")); + + await NewReporter(new HostPosture(HangfireHosting.Worker, "1.0.0+abc123"), logger).ReportAsync(CancellationToken.None); + + logger.Entries.ShouldNotContain(e => e.Level >= LogLevel.Warning); + } + + [Fact] + public async Task It_runs_the_refresh_lock_self_test_so_its_finding_is_logged_at_boot() + { + var test = new CountingSelfTest(); + using var _ = HostGit.Use(Reading("git version 2.39.5")); + + await NewReporter(new HostPosture(HangfireHosting.Api, "1.0.0+abc123"), new CapturingLogger(), test).ReportAsync(CancellationToken.None); + + test.Asks.ShouldBe(1); + } + + [Fact] + public async Task A_self_test_that_throws_never_escapes_the_boot_path() + { + var logger = new CapturingLogger(); + using var _ = HostGit.Use(Reading("git version 2.39.5")); + + await Should.NotThrowAsync(() => NewReporter(new HostPosture(HangfireHosting.Api, "1.0.0+abc123"), logger, new CountingSelfTest { Throws = true }).ReportAsync(CancellationToken.None)); + + logger.Entries.ShouldContain(e => e.Level == LogLevel.Warning && e.Exception is InvalidOperationException); + } + + [Fact] + public async Task It_reports_when_the_host_has_started_and_not_before() + { + var logger = new CapturingLogger(); + var lifetime = new FakeLifetime(); + using var _ = HostGit.Use(Reading("git version 2.39.5")); + var reporter = new BootPostureReporter(new HostPosture(HangfireHosting.Api, "1.0.0+abc123"), new CountingSelfTest(), lifetime, logger); + + reporter.ReportWhenStarted(); + + logger.Entries.ShouldBeEmpty("nothing is said before the host is serving"); + + lifetime.Start(); + + await Eventually(() => logger.Entries.Any(e => e.Message.StartsWith("Boot posture:", StringComparison.Ordinal))); + } + + private static BootPostureReporter NewReporter(HostPosture posture, CapturingLogger logger, CountingSelfTest? test = null) => + new(posture, test ?? new CountingSelfTest(), new FakeLifetime(), logger); + + private static GitReading Reading(string output) => new(GitVersion.Parse(output), output); + + private static async Task Eventually(Func condition) + { + var deadline = DateTime.UtcNow.AddSeconds(10); + + while (!condition() && DateTime.UtcNow < deadline) await Task.Delay(10); + + condition().ShouldBeTrue("the report was never logged after the host started"); + } + + private sealed class CountingSelfTest : IRefreshLockSelfTest + { + public int Asks { get; private set; } + + public bool Throws { get; init; } + + public Task VerdictAsync(CancellationToken cancellationToken) + { + Asks++; + + return Throws ? throw new InvalidOperationException("boom") : Task.FromResult(new RefreshLockVerdict(RefreshLockOutcome.Exclusive, "ok", DateTimeOffset.UnixEpoch)); + } + } + + private sealed class FakeLifetime : IHostApplicationLifetime + { + private readonly CancellationTokenSource _started = new(); + private readonly CancellationTokenSource _stopping = new(); + + public CancellationToken ApplicationStarted => _started.Token; + public CancellationToken ApplicationStopping => _stopping.Token; + public CancellationToken ApplicationStopped => CancellationToken.None; + + public void Start() => _started.Cancel(); + public void StopApplication() => _stopping.Cancel(); + } + + private sealed class CapturingLogger : ILogger + { + private readonly object _gate = new(); + private readonly List _entries = new(); + + public IReadOnlyList Entries { get { lock (_gate) return _entries.ToList(); } } + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + var properties = (state as IEnumerable> ?? []).ToDictionary(p => p.Key, p => p.Value?.ToString()); + + lock (_gate) _entries.Add(new Entry(logLevel, formatter(state, exception), exception, properties)); + } + + public sealed record Entry(LogLevel Level, string Message, Exception? Exception, IReadOnlyDictionary Properties); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Health/CredentialRepairSweepHealthCheckTests.cs b/backend/tests/CodeSpace.UnitTests/Health/CredentialRepairSweepHealthCheckTests.cs new file mode 100644 index 000000000..4938a8b87 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/CredentialRepairSweepHealthCheckTests.cs @@ -0,0 +1,290 @@ +using CodeSpace.Core.Jobs.RecurringJobs; +using CodeSpace.Core.Services.Credentials; +using CodeSpace.Core.Services.Health.Checks; +using Hangfire; +using Hangfire.Storage; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Time.Testing; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// +/// Whether the credential binding-repair sweep exists on this deployment at all, and how big the backlog it should be +/// clearing is. Only a Worker-role pod registers it, so an API-only fleet — or workers still on a build from before the sweep — +/// has none, and nothing else says so. The schedule is read the way Hangfire records it; the entry here is built with the +/// fields a recurring job really carries, and the integration tier reads one back from real storage. +/// +[Trait("Category", "Unit")] +public sealed class CredentialRepairSweepHealthCheckTests +{ + private static readonly DateTime Now = new(2026, 10, 18, 9, 0, 0, DateTimeKind.Utc); + + [Fact] + public void The_check_is_named_for_the_sweep_and_looks_for_the_job_the_worker_registers() + { + CredentialRepairSweepHealthCheck.Name.ShouldBe("credential-repair-sweep"); + CredentialRepairSweepHealthCheck.JobId.ShouldBe(nameof(DisconnectedCredentialBindingRepairRecurringJob), "Hangfire indexes the schedule by this id; the registration and this lookup must agree"); + } + + [Fact] + public void A_scheduled_sweep_that_ran_a_few_minutes_ago_is_healthy_and_reports_when_and_how_many() + { + var entry = Entry(lastExecution: Now.AddMinutes(-3), nextExecution: Now.AddMinutes(2), lastJobState: "Succeeded"); + + var result = CredentialRepairSweepHealthCheck.Assess(entry, candidates: 0, Now); + + result.Status.ShouldBe(HealthStatus.Healthy); + result.Data["registered"].ShouldBe(true); + result.Data["lastExecutionUtc"].ShouldBe(Now.AddMinutes(-3)); + result.Data["nextExecutionUtc"].ShouldBe(Now.AddMinutes(2)); + result.Data["lastJobState"].ShouldBe("Succeeded"); + result.Data["cron"].ShouldBe("*/5 * * * *"); + result.Data["strandedCandidates"].ShouldBe(0); + } + + [Fact] + public void A_sweep_registered_a_moment_ago_that_has_not_run_yet_is_healthy_not_missing() + { + var result = CredentialRepairSweepHealthCheck.Assess(Entry(lastExecution: null, nextExecution: Now.AddMinutes(4), lastJobState: null), candidates: 2, Now); + + result.Status.ShouldBe(HealthStatus.Healthy); + result.Data["strandedCandidates"].ShouldBe(2); + result.Data.ShouldNotContainKey("lastExecutionUtc"); + result.Description.ShouldNotBeNull().ShouldContain("not run yet"); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void A_sweep_nothing_registered_is_degraded_and_says_which_pods_register_it(bool removedMarker) + { + // Hangfire answers a recurring id it holds no entry for with a placeholder marked Removed, not with null. + RecurringJobDto? entry = removedMarker ? new RecurringJobDto { Id = CredentialRepairSweepHealthCheck.JobId, Removed = true } : null; + + var result = CredentialRepairSweepHealthCheck.Assess(entry, candidates: 3, Now); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("not registered"); + result.Description.ShouldContain("Worker", customMessage: "only the Worker role registers recurring jobs"); + result.Data["registered"].ShouldBe(false); + result.Data["strandedCandidates"].ShouldBe(3, "the backlog is still the answer to how bad it is"); + } + + [Fact] + public void A_sweep_the_scheduler_disabled_is_degraded_and_says_an_error_was_recorded_without_repeating_it() + { + // A previous-version scheduler that cannot load a job type retries, then disables the entry: no next execution. + var entry = Entry(lastExecution: Now.AddHours(-1), nextExecution: null, lastJobState: "Succeeded"); + entry.Error = "Could not load type 'CodeSpace.Core.Jobs.IJobSafeRunner'"; + + var result = CredentialRepairSweepHealthCheck.Assess(entry, candidates: 0, Now); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("disabled"); + result.Description.ShouldContain("recorded an error", customMessage: "it says there is one, and where to read it"); + result.Description.ShouldNotContain("Could not load type", customMessage: "the scheduler's text is an exception chain: it stays in the log, not on the report"); + } + + [Fact] + public void A_scheduler_error_with_stack_frames_and_a_storage_address_never_reaches_the_description_or_the_data() + { + // What Hangfire's RecurringJobScheduler stores for a storage fault: the exception chain, up to a hundred frames, host:port included. + var entry = Entry(lastExecution: null, nextExecution: null, lastJobState: null); + entry.Error = "Npgsql.NpgsqlException: Failed to connect to 10.20.30.40:5432\n ---> System.Net.Sockets.SocketException (111): Connection refused\n at Npgsql.Internal.NpgsqlConnector.Connect(NpgsqlTimeout timeout)\n at Hangfire.PostgreSql.PostgreSqlJobQueue.Enqueue(String queue, String jobId)"; + + var result = CredentialRepairSweepHealthCheck.Assess(entry, candidates: 0, Now); + var everythingPrinted = result.Description + string.Join(' ', result.Data.Select(pair => $"{pair.Key}={pair.Value}")); + + everythingPrinted.ShouldNotContain("10.20.30.40"); + everythingPrinted.ShouldNotContain("Npgsql"); + everythingPrinted.ShouldNotContain("Hangfire.PostgreSql"); + result.Description.ShouldNotContain("\n"); + } + + [Fact] + public void A_sweep_whose_next_run_is_long_past_is_degraded_because_no_scheduler_is_running() + { + var entry = Entry(lastExecution: Now.AddMinutes(-40), nextExecution: Now.AddMinutes(-35), lastJobState: "Succeeded"); + + var result = CredentialRepairSweepHealthCheck.Assess(entry, candidates: 1, Now); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("overdue"); + result.Description.ShouldContain("35"); + } + + [Fact] + public void A_next_run_a_little_late_is_still_healthy_the_scheduler_polls_between_ticks() + { + var result = CredentialRepairSweepHealthCheck.Assess(Entry(lastExecution: Now.AddMinutes(-5), nextExecution: Now.AddSeconds(-30), lastJobState: "Succeeded"), candidates: 0, Now); + + result.Status.ShouldBe(HealthStatus.Healthy); + } + + [Fact] + public void A_sweep_whose_last_run_failed_is_degraded() + { + var result = CredentialRepairSweepHealthCheck.Assess(Entry(lastExecution: Now.AddMinutes(-1), nextExecution: Now.AddMinutes(4), lastJobState: "Failed"), candidates: 0, Now); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("failed"); + } + + [Fact] + public void A_backlog_that_could_not_be_counted_is_degraded_and_the_schedule_is_still_reported() + { + var result = CredentialRepairSweepHealthCheck.Assess(Entry(lastExecution: Now.AddMinutes(-3), nextExecution: Now.AddMinutes(2), lastJobState: "Succeeded"), candidates: null, Now); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("could not be counted"); + result.Data.ShouldNotContainKey("strandedCandidates"); + result.Data["registered"].ShouldBe(true); + } + + [Fact] + public async Task A_host_with_no_job_storage_says_so_instead_of_throwing() + { + var check = new CredentialRepairSweepHealthCheck(new NoCensusScopes(), TimeProvider.System, NullLogger.Instance, storage: null); + + var result = await check.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain("job storage"); + } + + [Fact] + public async Task Storage_that_cannot_be_read_degrades_the_report_by_exception_type_and_never_throws() + { + var storage = new DownStorage(); + var check = new CredentialRepairSweepHealthCheck(new NoCensusScopes(), TimeProvider.System, NullLogger.Instance, storage); + + var result = await check.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain(nameof(InvalidOperationException)); + result.Description.ShouldNotContain("10.9.8.7", customMessage: "/health is anonymous: an address from an exception message must stay out of it"); + } + + [Fact] + public async Task A_reading_is_reused_for_its_lifetime_so_an_anonymous_endpoint_cannot_make_the_database_work_per_request() + { + var clock = new FakeTimeProvider(Now); + var storage = new DownStorage(); + var check = new CredentialRepairSweepHealthCheck(new NoCensusScopes(), clock, NullLogger.Instance, storage); + + var first = await check.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + clock.Advance(CredentialRepairSweepHealthCheck.SnapshotLifetime - TimeSpan.FromSeconds(1)); + var reused = await check.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + reused.Description.ShouldBe(first.Description); + storage.Opened.ShouldBe(1); + + clock.Advance(TimeSpan.FromSeconds(1)); + await check.CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + storage.Opened.ShouldBe(2, "an aged-out reading is taken again"); + } + + [Fact] + public void The_schedulers_error_is_logged_in_full_the_first_time_it_is_seen_and_not_again_while_it_stays_the_same() + { + var logger = new CapturingLogger(); + var check = new CredentialRepairSweepHealthCheck(new NoCensusScopes(), TimeProvider.System, logger, storage: null); + var disabled = Entry(lastExecution: null, nextExecution: null, lastJobState: null); + disabled.Error = "Npgsql.NpgsqlException: Failed to connect to 10.20.30.40:5432\n at Hangfire.PostgreSql.PostgreSqlJobQueue.Enqueue"; + + check.LogSchedulerError(disabled); + check.LogSchedulerError(disabled); + + var warning = logger.Entries.ShouldHaveSingleItem("a read every thirty seconds must not repeat the same Warning"); + + warning.Level.ShouldBe(LogLevel.Warning); + warning.Message.ShouldContain("10.20.30.40:5432", customMessage: "the log is where the full text, address and frames included, goes"); + warning.Message.ShouldContain("PostgreSqlJobQueue.Enqueue"); + } + + [Fact] + public void A_different_error_and_an_error_that_returns_after_the_entry_recovered_are_each_logged_again() + { + var logger = new CapturingLogger(); + var check = new CredentialRepairSweepHealthCheck(new NoCensusScopes(), TimeProvider.System, logger, storage: null); + var first = Entry(lastExecution: null, nextExecution: null, lastJobState: null); + first.Error = "first failure"; + var second = Entry(lastExecution: null, nextExecution: null, lastJobState: null); + second.Error = "second failure"; + var healthy = Entry(lastExecution: Now, nextExecution: Now.AddMinutes(5), lastJobState: "Succeeded"); + + check.LogSchedulerError(first); + check.LogSchedulerError(second); + check.LogSchedulerError(healthy); + check.LogSchedulerError(second); + + logger.Entries.Select(e => e.Message).ShouldBe(new[] + { + "Hangfire's scheduler disabled the DisconnectedCredentialBindingRepairRecurringJob schedule after an error: first failure", + "Hangfire's scheduler disabled the DisconnectedCredentialBindingRepairRecurringJob schedule after an error: second failure", + "Hangfire's scheduler disabled the DisconnectedCredentialBindingRepairRecurringJob schedule after an error: second failure", + }); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void An_entry_with_no_error_text_or_none_registered_logs_nothing(bool removedMarker) + { + var logger = new CapturingLogger(); + var check = new CredentialRepairSweepHealthCheck(new NoCensusScopes(), TimeProvider.System, logger, storage: null); + + check.LogSchedulerError(removedMarker ? new RecurringJobDto { Id = CredentialRepairSweepHealthCheck.JobId, Removed = true } : Entry(lastExecution: null, nextExecution: null, lastJobState: null)); + check.LogSchedulerError(null); + + logger.Entries.ShouldBeEmpty(); + } + + private sealed class CapturingLogger : ILogger + { + public List<(LogLevel Level, string Message)> Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + Entries.Add((logLevel, formatter(state, exception))); + } + + /// A recurring entry as Hangfire materializes one: the cron, the dates it keeps in UTC, and the last job's state. + private static RecurringJobDto Entry(DateTime? lastExecution, DateTime? nextExecution, string? lastJobState) => new() + { + Id = CredentialRepairSweepHealthCheck.JobId, + Cron = "*/5 * * * *", + LastExecution = lastExecution, + NextExecution = nextExecution, + LastJobState = lastJobState, + CreatedAt = Now.AddDays(-1), + }; + + /// Job storage whose every read fails, the way an unreachable database does — with an address in the message. + private sealed class DownStorage : JobStorage + { + public int Opened { get; private set; } + + public override IMonitoringApi GetMonitoringApi() => throw new NotSupportedException(); + + public override IStorageConnection GetConnection() + { + Opened++; + + throw new InvalidOperationException("storage is down at 10.9.8.7:5432"); + } + } + + private sealed class NoCensusScopes : IServiceScopeFactory + { + public IServiceScope CreateScope() => throw new InvalidOperationException("no storage means no census is attempted"); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Health/GitVersionHealthCheckTests.cs b/backend/tests/CodeSpace.UnitTests/Health/GitVersionHealthCheckTests.cs new file mode 100644 index 000000000..354048f99 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/GitVersionHealthCheckTests.cs @@ -0,0 +1,69 @@ +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Core.Services.Health.Checks; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// +/// The git floor, reported. A git before 2.31 ignores the variable the credential helper is configured through, so every +/// tokened clone, fetch and push fails as "could not read Username" — a failure that looks like a bad token. The check turns +/// that into Degraded with the cause, and never into a refusal to start. +/// +[Trait("Category", "Unit")] +public sealed class GitVersionHealthCheckTests +{ + [Fact] + public void The_check_is_named_git_in_the_report() => GitVersionHealthCheck.Name.ShouldBe("git"); + + [Theory] + [InlineData("git version 2.39.5")] + [InlineData("git version 2.31.0")] + public void A_git_that_reads_GIT_CONFIG_COUNT_is_healthy(string output) + { + var result = GitVersionHealthCheck.Assess(Reading(output)); + + result.Status.ShouldBe(HealthStatus.Healthy); + result.Description.ShouldNotBeNull().ShouldContain(GitVersion.Parse(output)!.Value.ToString()); + result.Data["version"].ShouldBe(GitVersion.Parse(output)!.Value.ToString()); + result.Data["minimum"].ShouldBe("2.31.0"); + } + + [Fact] + public void A_git_before_2_31_is_degraded_and_says_why() + { + var result = GitVersionHealthCheck.Assess(Reading("git version 2.30.2")); + + result.Status.ShouldBe(HealthStatus.Degraded, "Degraded keeps /health/ready at 200: an orchestrator must not restart a pod over this"); + result.Description.ShouldNotBeNull().ShouldContain("2.30.2"); + result.Description.ShouldContain("2.31"); + result.Description.ShouldContain("GIT_CONFIG_COUNT"); + result.Description.ShouldContain("could not read Username", customMessage: "it names the symptom an operator would otherwise chase"); + result.Data["version"].ShouldBe("2.30.2"); + } + + [Theory] + [InlineData("git could not be run: Win32Exception")] + [InlineData("something git never prints")] + public void A_git_whose_release_cannot_be_read_is_degraded_with_what_was_seen(string detail) + { + var result = GitVersionHealthCheck.Assess(new GitReading(null, detail)); + + result.Status.ShouldBe(HealthStatus.Degraded); + result.Description.ShouldNotBeNull().ShouldContain(detail); + result.Data.ShouldNotContainKey("version"); + } + + [Fact] + public async Task The_check_reads_the_hosts_git() + { + using var _ = HostGit.Use(Reading("git version 2.30.2")); + + var result = await new GitVersionHealthCheck().CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + } + + private static GitReading Reading(string output) => new(GitVersion.Parse(output), output); +} diff --git a/backend/tests/CodeSpace.UnitTests/Health/HealthResponseWriterTests.cs b/backend/tests/CodeSpace.UnitTests/Health/HealthResponseWriterTests.cs new file mode 100644 index 000000000..8db2f6b41 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/HealthResponseWriterTests.cs @@ -0,0 +1,169 @@ +using System.Text.Json; +using CodeSpace.Core.Services.Health; +using CodeSpace.Core.Services.Identity; +using CodeSpace.Messages.Constants; +using CodeSpace.Messages.Enums; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// +/// The body of GET /health, in two tiers. The endpoint is anonymous — a probe cannot sign in, and an operator comparing an +/// API image with a worker image should not need to — so anyone gets the build, the role and each check's status. What a check +/// says about the deployment (its description, its data: the git release, the connection topology, the repair backlog) is +/// reconnaissance on an internet-facing API, and is shown to a signed-in global Admin only. A check that threw is reported by +/// the exception's type in either tier, never its message. +/// +[Trait("Category", "Unit")] +public sealed class HealthResponseWriterTests +{ + private static readonly HostPosture Worker = new(HangfireHosting.Worker, "1.0.0+abc123"); + + [Fact] + public async Task An_admin_gets_the_build_the_role_the_worst_status_and_every_checks_finding_in_full() + { + var report = Report( + ("git", new HealthReportEntry(HealthStatus.Degraded, "git is old", TimeSpan.Zero, null, new Dictionary { ["version"] = "2.30.2" })), + ("db", new HealthReportEntry(HealthStatus.Healthy, null, TimeSpan.Zero, null, new Dictionary()))); + + var (context, body) = await WriteAsync(report, Worker, new StubUser(Roles.Admin)); + + context.Response.ContentType.ShouldStartWith("application/json"); + body.GetProperty("status").GetString().ShouldBe("Degraded"); + body.GetProperty("build").GetString().ShouldBe("1.0.0+abc123"); + body.GetProperty("role").GetString().ShouldBe("Worker"); + body.GetProperty("processesJobs").GetBoolean().ShouldBeTrue(); + body.GetProperty("detailsIncluded").GetBoolean().ShouldBeTrue(); + + var checks = body.GetProperty("checks").EnumerateArray().ToList(); + + checks.Select(c => c.GetProperty("name").GetString()).ShouldBe(new[] { "db", "git" }, "ordered by name so two reports diff cleanly"); + checks[1].GetProperty("status").GetString().ShouldBe("Degraded"); + checks[1].GetProperty("description").GetString().ShouldBe("git is old"); + checks[1].GetProperty("data").GetProperty("version").GetString().ShouldBe("2.30.2"); + } + + [Fact] + public async Task Anyone_else_gets_the_build_the_role_and_each_checks_status_but_no_description_and_no_data() + { + var report = Report( + ("git", new HealthReportEntry(HealthStatus.Degraded, "git 2.30.2 is older than 2.31.0", TimeSpan.Zero, null, new Dictionary { ["version"] = "2.30.2" })), + ("credential-repair-sweep", new HealthReportEntry(HealthStatus.Degraded, "not registered", TimeSpan.Zero, null, new Dictionary { ["strandedCandidates"] = 7 }))); + + var (_, body) = await WriteAsync(report, Worker, user: null); + + body.GetProperty("status").GetString().ShouldBe("Degraded"); + body.GetProperty("build").GetString().ShouldBe("1.0.0+abc123", "comparing two images from outside is what the endpoint is for"); + body.GetProperty("role").GetString().ShouldBe("Worker"); + body.GetProperty("detailsIncluded").GetBoolean().ShouldBeFalse("so a reader who sees empty findings knows why"); + + var checks = body.GetProperty("checks").EnumerateArray().ToList(); + + checks.Select(c => c.GetProperty("name").GetString()).ShouldBe(new[] { "credential-repair-sweep", "git" }); + checks.Select(c => c.GetProperty("status").GetString()).ShouldBe(new[] { "Degraded", "Degraded" }); + checks.ShouldAllBe(c => c.GetProperty("description").ValueKind == JsonValueKind.Null && c.GetProperty("data").EnumerateObject().Count() == 0); + + var printed = body.GetRawText(); + + printed.ShouldNotContain("2.30.2"); + printed.ShouldNotContain("strandedCandidates"); + printed.ShouldNotContain("not registered"); + } + + [Theory] + [InlineData("Member")] + [InlineData("Viewer")] + public async Task A_signed_in_caller_who_is_not_a_global_admin_is_treated_as_anonymous(string role) + { + var report = Report(("git", new HealthReportEntry(HealthStatus.Degraded, "git is old", TimeSpan.Zero, null, new Dictionary { ["version"] = "2.30.2" }))); + + var (_, body) = await WriteAsync(report, Worker, new StubUser(role)); + + body.GetProperty("detailsIncluded").GetBoolean().ShouldBeFalse(); + body.GetRawText().ShouldNotContain("git is old"); + } + + [Fact] + public async Task A_caller_whose_roles_cannot_be_read_is_treated_as_anonymous_and_the_report_is_still_written() + { + // Roles come from the database; a report is most needed when it is down, and a signed-in caller must not turn that into a 500. + var report = Report(("db", new HealthReportEntry(HealthStatus.Healthy, "up", TimeSpan.Zero, null, new Dictionary()))); + + var (_, body) = await WriteAsync(report, Worker, new StubUser(throws: true)); + + body.GetProperty("status").GetString().ShouldBe("Healthy"); + body.GetProperty("detailsIncluded").GetBoolean().ShouldBeFalse(); + body.GetRawText().ShouldNotContain("up\""); + } + + [Fact] + public async Task A_host_with_no_current_user_registered_serves_the_anonymous_tier() + { + var report = Report(("git", new HealthReportEntry(HealthStatus.Degraded, "git is old", TimeSpan.Zero, null, new Dictionary()))); + + var (_, body) = await WriteAsync(report, Worker, user: null); + + body.GetProperty("detailsIncluded").GetBoolean().ShouldBeFalse(); + } + + [Fact] + public async Task An_api_role_says_it_processes_no_jobs() + { + var (_, body) = await WriteAsync(Report(), new HostPosture(HangfireHosting.Api, "1.0.0+abc123"), user: null); + + body.GetProperty("role").GetString().ShouldBe("Api"); + body.GetProperty("processesJobs").GetBoolean().ShouldBeFalse(); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task A_check_that_threw_is_reported_by_the_exception_type_and_never_its_message_in_either_tier(bool admin) + { + var thrown = new InvalidOperationException("Failed to connect to 10.20.30.40:5432 as user svc_codespace"); + var report = Report(("db", new HealthReportEntry(HealthStatus.Unhealthy, thrown.Message, TimeSpan.Zero, thrown, new Dictionary()))); + + var (_, body) = await WriteAsync(report, new HostPosture(HangfireHosting.Api, "1.0.0+abc123"), admin ? new StubUser(Roles.Admin) : null); + + var entry = body.GetProperty("checks")[0]; + + entry.GetProperty("status").GetString().ShouldBe("Unhealthy"); + if (admin) entry.GetProperty("description").GetString().ShouldBe(nameof(InvalidOperationException)); + body.GetRawText().ShouldNotContain("10.20.30.40", customMessage: "an address or a user name from an exception message must never reach /health"); + body.GetRawText().ShouldNotContain("svc_codespace"); + } + + private static HealthReport Report(params (string Name, HealthReportEntry Entry)[] entries) => + new(entries.ToDictionary(e => e.Name, e => e.Entry), TimeSpan.Zero); + + private static async Task<(DefaultHttpContext Context, JsonElement Body)> WriteAsync(HealthReport report, HostPosture posture, ICurrentUser? user) + { + var services = new ServiceCollection().AddSingleton(posture); + + if (user is not null) services.AddSingleton(user); + + var context = new DefaultHttpContext { RequestServices = services.BuildServiceProvider() }; + context.Response.Body = new MemoryStream(); + + await HealthResponseWriter.WriteAsync(context, report); + + context.Response.Body.Position = 0; + + return (context, JsonDocument.Parse(context.Response.Body).RootElement.Clone()); + } + + /// The caller as the request pipeline resolves one: no roles at all for an anonymous request, the database's roles for a signed-in one, and a throw when that database is down. + private sealed class StubUser(string? role = null, bool throws = false) : ICurrentUser + { + public Guid? Id => Guid.NewGuid(); + public string Name => "stub"; + public IReadOnlyList Roles => throws ? throw new InvalidOperationException("role lookup failed: no connection to 10.20.30.40") : role is null ? Array.Empty() : new[] { role }; + public IReadOnlyList Permissions => Array.Empty(); + public bool HasRole(string name) => Roles.Contains(name); + public bool HasPermission(string permission) => false; + public bool PasswordMustChange => false; + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Health/HostPostureTests.cs b/backend/tests/CodeSpace.UnitTests/Health/HostPostureTests.cs new file mode 100644 index 000000000..60b09e77c --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/HostPostureTests.cs @@ -0,0 +1,48 @@ +using CodeSpace.Core.Services.Health; +using CodeSpace.Core.Settings; +using CodeSpace.Core.Settings.Logging; +using CodeSpace.Messages.Enums; +using Microsoft.Extensions.Configuration; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// Which build and which role this process is — the two facts that let an API image and a worker image be compared from outside. +[Trait("Category", "Unit")] +public sealed class HostPostureTests +{ + [Theory] + [InlineData("Api", "Api", false)] + [InlineData("Worker", "Worker", true)] + [InlineData(null, "Worker", true)] + [InlineData("nonsense", "Worker", true)] + public void The_role_is_the_one_the_deployment_gave_the_process_by_the_same_rule_that_decides_what_it_runs(string? configured, string role, bool processesJobs) + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary { [HangfireHostingSetting.ConfigurationKey] = configured }).Build(); + + var posture = new HostPosture(new HangfireHostingSetting(configuration)); + + posture.Role.ShouldBe(Enum.Parse(role)); + posture.RoleName.ShouldBe(role, "the wire name /health prints — a rename is a contract change"); + posture.ProcessesJobs.ShouldBe(processesJobs); + } + + [Fact] + public void The_build_is_the_identity_stamped_on_every_log_line() + { + var posture = new HostPosture(HangfireHosting.Api, BuildIdentity.Value); + + posture.Build.ShouldBe(BuildIdentity.Value); + } + + [Theory] + [InlineData(HangfireHosting.Api, "enqueues", "no jobs")] + [InlineData(HangfireHosting.Worker, "processes jobs", "recurring")] + public void The_role_is_explained_in_a_phrase_for_the_boot_line(HangfireHosting role, string first, string second) + { + var meaning = new HostPosture(role, "1.0.0+abc").RoleMeaning; + + meaning.ShouldContain(first); + meaning.ShouldContain(second); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Health/NpgsqlAdvisoryLockSessionsTests.cs b/backend/tests/CodeSpace.UnitTests/Health/NpgsqlAdvisoryLockSessionsTests.cs new file mode 100644 index 000000000..e8c57c4e8 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/NpgsqlAdvisoryLockSessionsTests.cs @@ -0,0 +1,33 @@ +using CodeSpace.Core.Services.Health; +using CodeSpace.Core.Services.Health.Sessions; +using CodeSpace.Core.Settings.Database; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// +/// The sessions are built when the host is, from a connection string that may be anything an operator typed. A string the +/// driver cannot parse is the self-test's "could not run" — one of the findings /health reports — and never a failure to +/// build the host, which is the one thing a posture report must not cause. +/// +[Trait("Category", "Unit")] +public sealed class NpgsqlAdvisoryLockSessionsTests +{ + [Theory] + [InlineData("this is not a connection string")] + [InlineData("Host=localhost;NoSuchKeyword=1")] + [InlineData("")] + public async Task A_connection_string_the_driver_rejects_builds_the_sessions_and_makes_the_self_test_inconclusive(string connectionString) + { + var configuration = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary { ["CodeSpaceStore:ConnectionString"] = connectionString }).Build(); + + var sessions = Should.NotThrow(() => new NpgsqlAdvisoryLockSessions(new CodeSpaceConnectionString(configuration)), "construction must never fail the host"); + + var verdict = await new RefreshLockSelfTest(sessions, TimeProvider.System, NullLogger.Instance).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Inconclusive); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Health/RefreshLockHealthCheckTests.cs b/backend/tests/CodeSpace.UnitTests/Health/RefreshLockHealthCheckTests.cs new file mode 100644 index 000000000..c80be99d2 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/RefreshLockHealthCheckTests.cs @@ -0,0 +1,61 @@ +using CodeSpace.Core.Services.Health; +using CodeSpace.Core.Services.Health.Checks; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Diagnostics.HealthChecks; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// The refresh-lock self-test's finding, as a health entry: Degraded for anything but an exclusive lock, with the sentence an operator reads. +[Trait("Category", "Unit")] +public sealed class RefreshLockHealthCheckTests +{ + private static readonly DateTimeOffset At = new(2026, 10, 18, 9, 0, 0, TimeSpan.Zero); + + [Fact] + public void The_check_is_named_refresh_lock_in_the_report() => RefreshLockHealthCheck.Name.ShouldBe("refresh-lock"); + + [Fact] + public void An_exclusive_lock_is_healthy() + { + var result = RefreshLockHealthCheck.Assess(new RefreshLockVerdict(RefreshLockOutcome.Exclusive, "exclusive", At)); + + result.Status.ShouldBe(HealthStatus.Healthy); + result.Data["outcome"].ShouldBe("Exclusive"); + result.Data["observedAtUtc"].ShouldBe(At.UtcDateTime); + } + + [Theory] + [InlineData(RefreshLockOutcome.NotExclusive)] + [InlineData(RefreshLockOutcome.Inconclusive)] + public void A_lock_that_is_not_exclusive_or_not_known_to_be_is_degraded_with_the_precise_message(RefreshLockOutcome outcome) + { + var result = RefreshLockHealthCheck.Assess(new RefreshLockVerdict(outcome, "the precise message", At)); + + result.Status.ShouldBe(HealthStatus.Degraded, "Degraded leaves /health/ready at 200: the compare-and-set and the re-read keep refresh correct, so no pod is worth restarting"); + result.Description.ShouldBe("the precise message"); + result.Data["outcome"].ShouldBe(outcome.ToString()); + } + + [Fact] + public async Task The_check_asks_the_self_test() + { + var test = new ScriptedSelfTest(new RefreshLockVerdict(RefreshLockOutcome.NotExclusive, "pooler", At)); + + var result = await new RefreshLockHealthCheck(test).CheckHealthAsync(new HealthCheckContext(), CancellationToken.None); + + result.Status.ShouldBe(HealthStatus.Degraded); + test.Asks.ShouldBe(1); + } + + private sealed class ScriptedSelfTest(RefreshLockVerdict verdict) : IRefreshLockSelfTest + { + public int Asks { get; private set; } + + public Task VerdictAsync(CancellationToken cancellationToken) + { + Asks++; + return Task.FromResult(verdict); + } + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Health/RefreshLockSelfTestTests.cs b/backend/tests/CodeSpace.UnitTests/Health/RefreshLockSelfTestTests.cs new file mode 100644 index 000000000..0370c2ee2 --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Health/RefreshLockSelfTestTests.cs @@ -0,0 +1,365 @@ +using CodeSpace.Core.Services.Health; +using CodeSpace.Messages.Health; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Time.Testing; +using Shouldly; + +namespace CodeSpace.UnitTests.Health; + +/// +/// The startup self-test that takes one advisory-lock key on two connections and expects the second to be refused. OAuth +/// token refresh serializes on pg_advisory_lock; a transaction-mode pooler, RDS Proxy or a read-write splitting proxy +/// hands both connections the same server session, where the lock is re-entrant, and refresh quietly stops being +/// serialized. The test cannot stop that — the compare-and-set write and the re-read keep the stored tokens correct — it +/// names it. Driven here against a fake server that models exactly the property that matters: a lock is held by a SERVER +/// session, and a session may take its own lock again. +/// +[Trait("Category", "Unit")] +public sealed class RefreshLockSelfTestTests +{ + private static readonly DateTimeOffset Start = new(2026, 10, 18, 9, 0, 0, TimeSpan.Zero); + + [Fact] + public async Task Two_sessions_cannot_both_hold_the_key_so_the_lock_is_exclusive() + { + var server = new FakeServer(); + + var verdict = await NewTest(server).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Exclusive); + verdict.ObservedAt.ShouldBe(Start); + server.Opened.ShouldBe(2, "one holder, one contender"); + server.ContenderAttempts.ShouldBe(RefreshLockSelfTest.ContenderAttempts, "an exclusive lock is only believed after the contender has tried more than once"); + server.AssertNothingLeftHeldAndEverythingClosed(); + } + + [Fact] + public async Task A_second_session_that_takes_the_held_key_means_the_lock_is_not_exclusive() + { + var server = new FakeServer { EveryClientIsOneServerSession = true }; + + var verdict = await NewTest(server).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.NotExclusive); + verdict.Detail.ShouldContain("pg_advisory_lock", customMessage: "it names the lock that failed"); + verdict.Detail.ShouldContain("PgBouncer", customMessage: "and the topologies that cause it"); + verdict.Detail.ShouldContain("RDS Proxy"); + verdict.Detail.ShouldContain("compare-and-set", customMessage: "and what still holds when the lock does not"); + verdict.Detail.ShouldContain("CodeSpaceStore:ConnectionString", customMessage: "and which setting to change"); + server.ContenderAttempts.ShouldBe(1, "the first answer that breaks the property settles it"); + server.AssertNothingLeftHeldAndEverythingClosed(); + } + + [Fact] + public async Task A_pooler_that_only_sometimes_lands_both_sessions_on_one_server_is_still_caught() + { + // A pool of server sessions hands the contender a different one first, then the holder's. Trying once would call that lock exclusive. + var server = new FakeServer { ContenderLandsOnTheHoldersSessionAtAttempt = 4 }; + + var verdict = await NewTest(server).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.NotExclusive); + server.ContenderAttempts.ShouldBe(4); + server.AssertNothingLeftHeldAndEverythingClosed(); + } + + [Fact] + public async Task A_holder_told_it_never_held_the_lock_it_took_means_statements_are_multiplexed_even_though_the_contender_was_refused() + { + // A transaction-mode pooler serves each statement from some server session: the lock lands on one, the unlock on another. + // The contender can miss the holder's server session every time; the unlock's answer cannot be talked away. + var server = new FakeServer { UnlockMissesBeforeLanding = 1 }; + + var verdict = await NewTest(server).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.NotExclusive, "the holder's own unlock said it did not hold the key"); + verdict.Detail.ShouldContain("did not hold", customMessage: "it names what was observed"); + verdict.Detail.ShouldContain("PgBouncer", customMessage: "and the topologies that cause it"); + verdict.Detail.ShouldContain("compare-and-set"); + verdict.Detail.ShouldContain("CodeSpaceStore:ConnectionString"); + server.ContenderAttempts.ShouldBe(RefreshLockSelfTest.ContenderAttempts, "the contender was refused every time, and that alone proves nothing"); + server.AssertNothingLeftHeldAndEverythingClosed(); + } + + [Fact] + public async Task An_unlock_that_misses_is_retried_to_clear_the_lock_it_left_but_only_a_bounded_number_of_times() + { + var server = new FakeServer { UnlockMissesBeforeLanding = 100 }; + + var verdict = await NewTest(server).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.NotExclusive); + server.UnlockCalls.ShouldBe(RefreshLockSelfTest.ReleaseAttempts, "a pooler that never lands the unlock where the lock is must not keep the boot path busy"); + } + + [Fact] + public async Task An_unlock_that_throws_is_no_evidence_about_the_lock_either_way() + { + var server = new FakeServer { UnlockFails = new InvalidOperationException("connection reset by 10.20.30.40") }; + + var verdict = await NewTest(server).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Exclusive, "the contender was refused and the unlock said nothing: not a finding against the lock"); + verdict.Detail.ShouldNotContain("10.20.30.40"); + } + + [Fact] + public async Task A_lock_that_passes_says_it_found_no_sign_of_multiplexing_not_that_it_proved_exclusivity() + { + var verdict = await NewTest(new FakeServer()).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Exclusive); + verdict.Detail.ShouldContain("no sign", customMessage: "a pass fails to find a pooler; it cannot prove there is none"); + verdict.Detail.ShouldNotContain("is exclusive across connections"); + } + + [Fact] + public async Task A_test_that_cannot_run_is_inconclusive_and_does_not_echo_the_exception_message() + { + var server = new FakeServer { OpenFails = new InvalidOperationException("Failed to connect to 10.20.30.40:5432") }; + + var verdict = await NewTest(server).VerdictAsync(CancellationToken.None); + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Inconclusive); + verdict.Detail.ShouldContain(nameof(InvalidOperationException), customMessage: "the type says what kind of failure it was"); + verdict.Detail.ShouldNotContain("10.20.30.40", customMessage: "/health is anonymous: an address from an exception message must stay in the log"); + verdict.Detail.ShouldContain("unknown", customMessage: "and that nothing is known about the lock"); + } + + [Fact] + public async Task A_test_that_hangs_ends_as_inconclusive_when_the_timeout_passes() + { + var clock = new FakeTimeProvider(Start); + var opening = new TaskCompletionSource(); + var server = new FakeServer { OnOpen = async cancellationToken => { opening.TrySetResult(); await Task.Delay(Timeout.Infinite, cancellationToken); } }; + var test = NewTest(server, clock); + + var pending = test.VerdictAsync(CancellationToken.None); + await opening.Task; + clock.Advance(RefreshLockSelfTest.ProbeTimeout); + + var verdict = await pending; + + verdict.Outcome.ShouldBe(RefreshLockOutcome.Inconclusive); + verdict.Detail.ShouldContain("did not finish"); + } + + [Fact] + public async Task A_caller_that_cancels_gets_the_cancellation_and_nothing_is_remembered() + { + var server = new FakeServer { OnOpen = async cancellationToken => await Task.Delay(Timeout.Infinite, cancellationToken) }; + var test = NewTest(server); + using var cancel = new CancellationTokenSource(); + + var pending = test.VerdictAsync(cancel.Token); + await cancel.CancelAsync(); + + await Should.ThrowAsync(() => pending); + + server.OnOpen = null; + (await test.VerdictAsync(CancellationToken.None)).Outcome.ShouldBe(RefreshLockOutcome.Exclusive, "a cancelled caller must not leave a stale verdict behind"); + } + + [Fact] + public async Task The_verdict_is_reused_until_it_is_older_than_its_lifetime() + { + var clock = new FakeTimeProvider(Start); + var server = new FakeServer(); + var test = NewTest(server, clock); + + var first = await test.VerdictAsync(CancellationToken.None); + clock.Advance(RefreshLockSelfTest.VerdictLifetime - TimeSpan.FromSeconds(1)); + var reused = await test.VerdictAsync(CancellationToken.None); + + reused.ShouldBeSameAs(first); + server.Opened.ShouldBe(2, "an anonymous endpoint must not open two connections per request"); + + clock.Advance(TimeSpan.FromSeconds(1)); + var fresh = await test.VerdictAsync(CancellationToken.None); + + fresh.ShouldNotBeSameAs(first); + fresh.ObservedAt.ShouldBe(Start + RefreshLockSelfTest.VerdictLifetime); + server.Opened.ShouldBe(4); + } + + [Fact] + public async Task Askers_that_arrive_together_share_one_test() + { + var release = new TaskCompletionSource(); + var server = new FakeServer { OnOpen = _ => release.Task }; + var test = NewTest(server); + + var asks = Enumerable.Range(0, 5).Select(_ => test.VerdictAsync(CancellationToken.None)).ToList(); + release.SetResult(); + var verdicts = await Task.WhenAll(asks); + + verdicts.Distinct().Count().ShouldBe(1); + server.Opened.ShouldBe(2); + } + + [Fact] + public async Task A_lock_that_is_not_exclusive_is_logged_as_an_Error_once_and_not_again_while_it_stays_so() + { + var clock = new FakeTimeProvider(Start); + var logger = new CapturingLogger(); + var test = NewTest(new FakeServer { EveryClientIsOneServerSession = true }, clock, logger); + + await test.VerdictAsync(CancellationToken.None); + clock.Advance(RefreshLockSelfTest.VerdictLifetime); + await test.VerdictAsync(CancellationToken.None); + + var errors = logger.Entries.Where(e => e.Level == LogLevel.Error).ToList(); + + errors.ShouldHaveSingleItem("a re-test every few minutes must not repeat the same Error"); + errors[0].Message.ShouldContain("PgBouncer"); + } + + [Fact] + public async Task A_lock_that_turns_exclusive_after_not_being_so_says_so_once() + { + var clock = new FakeTimeProvider(Start); + var logger = new CapturingLogger(); + var server = new FakeServer { EveryClientIsOneServerSession = true }; + var test = NewTest(server, clock, logger); + + await test.VerdictAsync(CancellationToken.None); + server.EveryClientIsOneServerSession = false; + clock.Advance(RefreshLockSelfTest.VerdictLifetime); + await test.VerdictAsync(CancellationToken.None); + + logger.Entries.Where(e => e.Level == LogLevel.Information).ShouldHaveSingleItem().Message.ShouldContain("no sign"); + } + + [Fact] + public async Task A_test_that_cannot_run_is_a_Warning_with_the_exception_never_an_Error() + { + var logger = new CapturingLogger(); + var test = NewTest(new FakeServer { OpenFails = new InvalidOperationException("no route") }, new FakeTimeProvider(Start), logger); + + await test.VerdictAsync(CancellationToken.None); + + logger.Entries.ShouldNotContain(e => e.Level == LogLevel.Error); + logger.Entries.ShouldContain(e => e.Level == LogLevel.Warning && e.Exception is InvalidOperationException); + } + + private static RefreshLockSelfTest NewTest(FakeServer server, FakeTimeProvider? clock = null, ILogger? logger = null) => + new(server, clock ?? new FakeTimeProvider(Start), logger ?? new CapturingLogger()); + + /// + /// A Postgres server's advisory locks, as far as the self-test can tell. A lock is held by a server SESSION, a session may + /// take a lock it already holds (re-entrant), and a different session is refused. A client normally has a session of its + /// own; behind a transaction-mode pooler every client lands on one. + /// + private sealed class FakeServer : IAdvisoryLockSessions + { + private readonly Dictionary _held = new(); + private int _nextSession; + private int _opened; + private int _closed; + + public bool EveryClientIsOneServerSession { get; set; } + + /// Models a session pool: the contender's try-locks run on another server session until this attempt, and on the holder's from then on. Zero = never. + public int ContenderLandsOnTheHoldersSessionAtAttempt { get; init; } + + /// Models a statement-level pooler: this many unlocks run on a server session that does not hold the key (answering false, changing nothing) before one lands on the holder's. + public int UnlockMissesBeforeLanding { get; set; } + + /// Every unlock throws this — a connection that dropped after the contender was refused. + public Exception? UnlockFails { get; init; } + + public Exception? OpenFails { get; init; } + + public Func? OnOpen { get; set; } + + public int Opened => _opened; + + public int ContenderAttempts { get; private set; } + + public int UnlockCalls { get; private set; } + + public async Task OpenAsync(CancellationToken cancellationToken) + { + if (OnOpen is { } onOpen) await onOpen(cancellationToken); + + if (OpenFails is { } failure) throw failure; + + Interlocked.Increment(ref _opened); + + return new Session(this, EveryClientIsOneServerSession ? 1 : Interlocked.Increment(ref _nextSession), isContender: _opened % 2 == 0); + } + + public void AssertNothingLeftHeldAndEverythingClosed() + { + _held.ShouldBeEmpty("the test must never leave a key held — a real one would hold it until the connection died"); + _closed.ShouldBe(_opened, "every session the test opened is disposed"); + } + + private sealed class Session(FakeServer server, int serverSession, bool isContender) : IAdvisoryLockSession + { + public Task LockAsync(long key, CancellationToken cancellationToken) + { + if (server._held.TryGetValue(key, out var held) && held.Session != serverSession) throw new InvalidOperationException("a blocking lock on a held key would hang the test: the holder must lock a fresh key"); + + server._held[key] = (serverSession, server._held.TryGetValue(key, out var again) ? again.Depth + 1 : 1); + + return Task.CompletedTask; + } + + public Task TryLockAsync(long key, CancellationToken cancellationToken) + { + if (isContender) server.ContenderAttempts++; + + var effective = server.ContenderLandsOnTheHoldersSessionAtAttempt is > 0 && isContender + ? (server.ContenderAttempts >= server.ContenderLandsOnTheHoldersSessionAtAttempt ? 1 : 99) + : serverSession; + + if (server._held.TryGetValue(key, out var held) && held.Session != effective) return Task.FromResult(false); + + server._held[key] = (effective, held.Depth + 1); + + return Task.FromResult(true); + } + + public Task UnlockAsync(long key, CancellationToken cancellationToken) + { + server.UnlockCalls++; + + if (server.UnlockFails is { } failure) throw failure; + + if (server.UnlockMissesBeforeLanding > 0) + { + server.UnlockMissesBeforeLanding--; + + return Task.FromResult(false); + } + + if (!server._held.TryGetValue(key, out var held)) return Task.FromResult(false); + + if (held.Depth <= 1) server._held.Remove(key); + else server._held[key] = (held.Session, held.Depth - 1); + + return Task.FromResult(true); + } + + public ValueTask DisposeAsync() + { + Interlocked.Increment(ref server._closed); + + return ValueTask.CompletedTask; + } + } + } + + private sealed class CapturingLogger : ILogger + { + public List<(LogLevel Level, string Message, Exception? Exception)> Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) => + Entries.Add((logLevel, formatter(state, exception), exception)); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/GitVersionTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/GitVersionTests.cs new file mode 100644 index 000000000..db1ebc09c --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/GitVersionTests.cs @@ -0,0 +1,69 @@ +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Messages.Health; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// The release a host's git reports. It decides one thing: whether the env-only credential helper +/// (GIT_CONFIG_COUNT, git 2.31 and later) can reach this git at all, so the parse has to cope with what real +/// distributions print and answer "unknown" — never a guess — for anything else. +/// +[Trait("Category", "Unit")] +public sealed class GitVersionTests +{ + [Theory] + [InlineData("git version 2.39.5", 2, 39, 5)] + [InlineData("git version 2.39.5\n", 2, 39, 5)] + [InlineData("git version 2.31.0", 2, 31, 0)] + [InlineData("git version 2.30.2", 2, 30, 2)] + [InlineData("git version 2.31", 2, 31, 0)] + [InlineData("git version 2.39.5 (Apple Git-154)", 2, 39, 5)] + [InlineData("git version 2.43.0.windows.1", 2, 43, 0)] + [InlineData("git version 2.45.0-rc1", 2, 45, 0)] + public void It_reads_the_release_out_of_git_version_output(string output, int major, int minor, int patch) => + GitVersion.Parse(output).ShouldBe(new GitVersion(major, minor, patch)); + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + [InlineData("fatal: not a git repository")] + [InlineData("git version")] + [InlineData("git version two.thirty")] + [InlineData("2.39.5")] + public void Anything_else_is_unknown_rather_than_a_guess(string? output) => GitVersion.Parse(output).ShouldBeNull(); + + [Theory] + [InlineData("2.30.2", false)] + [InlineData("2.30.99", false)] + [InlineData("2.31.0", true)] + [InlineData("2.31.1", true)] + [InlineData("2.39.5", true)] + [InlineData("3.0.0", true)] + [InlineData("1.99.9", false)] + public void It_is_at_least_2_31_from_exactly_2_31(string release, bool expected) + { + var version = GitVersion.Parse($"git version {release}")!.Value; + + version.IsAtLeast(TokenedGitCommand.MinimumGit).ShouldBe(expected); + } + + [Fact] + public void The_minimum_is_the_release_that_introduced_GIT_CONFIG_COUNT() => TokenedGitCommand.MinimumGit.ShouldBe(new GitVersion(2, 31, 0)); + + [Fact] + public void It_prints_as_the_dotted_release() => new GitVersion(2, 39, 5).ToString().ShouldBe("2.39.5"); + + [Fact] + public void The_host_probe_parses_what_this_machines_git_actually_prints() + { + // Through the real reader: the fixtures above are what I believe distributions print; this is what one does. + var reading = HostGit.Current; + + if (reading.Version is null && reading.Detail.StartsWith("git could not be run", StringComparison.Ordinal)) return; // no git here; cross-host dotnet test stays clean + + reading.Version.ShouldNotBeNull($"git is installed but its output was not understood: '{reading.Detail}'"); + reading.Detail.ShouldStartWith("git version "); + } +} diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs index a90cba2c0..0fdce44a6 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/LocalGitWorkspaceProviderTests.cs @@ -3,6 +3,7 @@ using CodeSpace.Core.Services.Agents.Workspace; using CodeSpace.Core.Services.Agents.Workspace.Providers; using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Health; using Microsoft.Extensions.Logging.Abstractions; using Shouldly; @@ -203,6 +204,44 @@ await Should.ThrowAsync(() => provider.PrepareAsync(Workspac runner.Invocations.ShouldBeEmpty(); } + [Theory] + [InlineData(true, "git version 2.30.2", true)] + [InlineData(true, "git version 2.39.5", false)] + [InlineData(false, "git version 2.30.2", false)] + public async Task A_clone_the_remote_asked_a_credential_for_names_an_old_git_only_when_a_token_was_sent_and_git_is_old(bool tokened, string gitOutput, bool oldGitIsTheCause) + { + // The run's own clone — the failure every run on a git before 2.31 died of, reported as "could not read Username". + var runner = new RefusedCloneRunner("fatal: could not read Username for 'https://gitlab.example.test': terminal prompts disabled"); + var provider = new LocalGitWorkspaceProvider(new SandboxRunnerRegistry(new ISandboxRunner[] { runner }), NullLogger.Instance); + using var _ = HostGit.Use(new GitReading(GitVersion.Parse(gitOutput), gitOutput)); + + var failure = await Should.ThrowAsync(() => provider.PrepareAsync(WorkspaceProvisionRequest.FromSingle(new WorkspaceRequest + { + RepositoryUrl = "https://gitlab.example.test/a/b.git", Token = tokened ? "test-token" : null, + }), CancellationToken.None)); + + runner.CloneAttempts.ShouldBe(1, "fixture check: the refusal came from the clone"); + (failure is GitCredentialHelperUnsupportedException).ShouldBe(oldGitIsTheCause, failure.Message); + failure.Message.ShouldNotContain("test-token"); + if (!oldGitIsTheCause) failure.Message.ShouldContain("git clone failed", customMessage: "otherwise the message is exactly what it always was"); + } + + /// Refuses the clone the way git does for a remote that wanted a credential and got none; answers every other command as a clean success. + private sealed class RefusedCloneRunner(string cloneStderr) : ISandboxRunner + { + public string Kind => "local"; + public int CloneAttempts { get; private set; } + + public Task RunAsync(SandboxSpec spec, CancellationToken cancellationToken) + { + if (!spec.Args.Contains("clone")) return Task.FromResult(new SandboxResult { Status = SandboxStatus.Success, ExitCode = 0, Stdout = new string('a', 40), Stderr = "" }); + + CloneAttempts++; + + return Task.FromResult(new SandboxResult { Status = SandboxStatus.Failed, ExitCode = 128, Stdout = "", Stderr = cloneStderr }); + } + } + private sealed class WorkspacePathRunner : ISandboxRunner { public string Kind => "local"; @@ -589,10 +628,17 @@ private sealed class PostTurnRunner(bool agentCommittedItself) : ISandboxRunner { public string Kind => "local"; public List Specs { get; } = new(); + + /// When set, the push is refused with this stderr — the way git refuses one the remote wanted a credential for. + public string? RefusePushWith { get; init; } + public Task RunAsync(SandboxSpec spec, CancellationToken cancellationToken) { Specs.Add(spec); + if (RefusePushWith is not null && spec.Args.Contains("push")) + return Task.FromResult(new SandboxResult { Status = SandboxStatus.Failed, ExitCode = 128, Stdout = "", Stderr = RefusePushWith }); + if (agentCommittedItself && spec.Args.Contains("commit")) return Task.FromResult(new SandboxResult { Status = SandboxStatus.Failed, ExitCode = 1, Stdout = "nothing to commit, working tree clean", Stderr = "" }); @@ -642,6 +688,41 @@ public async Task Every_command_before_the_token_strip_runs_as_a_tokened_command runner.Specs.Where(s => TokenedGitSpecs.RunsTokened(s, "https://example.test/repo.git")).ShouldAllBe(s => TokenedGitSpecs.CarriesTheCredential(s, "x-access-token", "test-token")); } + [Fact] + public async Task A_push_the_remote_asked_a_credential_for_names_an_old_git_when_the_host_git_is_before_2_31() + { + // The publish is the last thing a run does, and the one failure that leaves a Succeeded run with no branch. + var runner = new PostTurnRunner(agentCommittedItself: false) { RefusePushWith = "fatal: could not read Username for 'https://example.test': terminal prompts disabled" }; + var provider = new LocalGitWorkspaceProvider(new SandboxRunnerRegistry(new[] { runner }), NullLogger.Instance); + + await using var handle = await provider.PrepareAsync(PostTurnProvision("fixture-token", multiRepo: false), CancellationToken.None); + using var _ = HostGit.Use(new GitReading(new GitVersion(2, 30, 2), "git version 2.30.2")); + + var failure = await Should.ThrowAsync(() => ((IWorkspacePushHandle)handle).PushChangesAsync("codespace/run", CancellationToken.None)); + + runner.Specs.ShouldContain(spec => spec.Args.Contains("push"), "fixture check: the refusal came from the push"); + failure.Message.ShouldNotContain("fixture-token"); + } + + [Theory] + [InlineData("https://x-access-token:stored-credential@example.test/repo.git")] // the credential rides in the repository URL + [InlineData("https://example.test/repo.git")] + public async Task A_run_with_no_clone_token_never_pushes_whatever_its_url_carries_so_the_publish_has_no_credential_of_its_own(string repositoryUrl) + { + // The premise the publish's old-git decision rests on: a command that reaches the remote there is tokened exactly when the + // run has a clone token. RemoteFor also reads a URL's userinfo as a credential, but without a token the publish returns + // before any git runs — so the remote the push runs against and the token the run holds cannot disagree. + var runner = new PostTurnRunner(agentCommittedItself: false) { RefusePushWith = "fatal: could not read Username for 'https://example.test': terminal prompts disabled" }; + var provider = new LocalGitWorkspaceProvider(new SandboxRunnerRegistry(new[] { runner }), NullLogger.Instance); + + await using var handle = await provider.PrepareAsync(WorkspaceProvisionRequest.FromSingle(new WorkspaceRequest { RepositoryUrl = repositoryUrl }), CancellationToken.None); + var prepared = runner.Specs.Count; + + (await ((IWorkspacePushHandle)handle).PushChangesAsync("codespace/run", CancellationToken.None)).ShouldBeNull(); + + runner.Specs.Skip(prepared).ShouldBeEmpty("no git runs for a run that holds no push credential"); + } + [Fact] public async Task Only_the_publish_commands_that_reach_the_remote_run_as_tokened_commands() { diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs index 3e765f72c..bae7fa462 100644 --- a/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs +++ b/backend/tests/CodeSpace.UnitTests/Workflows/RemoteTipResolverTests.cs @@ -2,6 +2,7 @@ using CodeSpace.Core.Services.Agents.Sandbox.Runners; using CodeSpace.Core.Services.Agents.Workspace; using CodeSpace.Messages.Agents; +using CodeSpace.Messages.Health; using Shouldly; namespace CodeSpace.UnitTests.Workflows; @@ -197,6 +198,25 @@ public async Task The_launch_probe_runs_as_a_tokened_command_only_when_it_carrie if (tokened) TokenedGitSpecs.CarriesTheCredential(probe, "x-access-token", "test-token").ShouldBeTrue(); } + [Theory] + [InlineData("git version 2.30.2", true)] + [InlineData("git version 2.39.5", false)] + public async Task A_tokened_probe_the_remote_refused_for_want_of_a_credential_names_an_old_git_only_when_git_is_old(string gitOutput, bool oldGitIsTheCause) + { + // The launch probe runs on the API pod, which installs git for it alone. With a git before 2.31 its token is never + // offered, and the launch failed with git's "could not read Username" — a sentence that points at the credential. + var runner = new RefusingRunner("fatal: could not read Username for 'https://gitlab.example.test': terminal prompts disabled"); + var request = new WorkspaceRequest { RepositoryUrl = "https://gitlab.example.test/a/b.git", Token = "test-token", Ref = "main" }; + using var _ = HostGit.Use(new GitReading(GitVersion.Parse(gitOutput), gitOutput)); + + var failure = await Should.ThrowAsync(() => + new RemoteTipResolver(new SandboxRunnerRegistry(new ISandboxRunner[] { runner })).ResolveTipShaAsync(request, refRequired: true, CancellationToken.None)); + + (failure is GitCredentialHelperUnsupportedException).ShouldBe(oldGitIsTheCause, failure.Message); + failure.Message.Contains("could not read Username", StringComparison.Ordinal).ShouldBe(!oldGitIsTheCause, "the typed failure replaces git's sentence; otherwise git's own words stand"); + failure.Message.ShouldNotContain("test-token"); + } + // ─── harness (the LocalGitWorkspaceProviderTests pattern) ─────────────────────── private sealed class LsRemoteRunner(string stdout) : ISandboxRunner @@ -210,6 +230,13 @@ public Task RunAsync(SandboxSpec spec, CancellationToken cancella } } + private sealed class RefusingRunner(string stderr) : ISandboxRunner + { + public string Kind => "local"; + public Task RunAsync(SandboxSpec spec, CancellationToken cancellationToken) => + Task.FromResult(new SandboxResult { Status = SandboxStatus.Failed, ExitCode = 128, Stdout = "", Stderr = stderr }); + } + private static RemoteTipResolver NewResolver() => new(new SandboxRunnerRegistry(new ISandboxRunner[] { new LocalProcessRunner() })); diff --git a/backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitOldGitFailureTests.cs b/backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitOldGitFailureTests.cs new file mode 100644 index 000000000..7af72c4ea --- /dev/null +++ b/backend/tests/CodeSpace.UnitTests/Workflows/TokenedGitOldGitFailureTests.cs @@ -0,0 +1,75 @@ +using CodeSpace.Core.Services.Agents.Workspace; +using CodeSpace.Messages.Failures; +using CodeSpace.Messages.Health; +using Shouldly; + +namespace CodeSpace.UnitTests.Workflows; + +/// +/// A tokened command carries its credential in a helper that GIT_CONFIG_COUNT configures, and a git older than 2.31 +/// ignores that variable: the remote asks for a credential, nothing answers, and git says it "could not read Username". +/// That sentence names no cause an operator can act on. When — and only when — the host's git is known to be too old +/// and the command was tokened, the failure says so. +/// +[Trait("Category", "Unit")] +public sealed class TokenedGitOldGitFailureTests +{ + private const string NoUsername = "fatal: could not read Username for 'https://gitlab.example.test': terminal prompts disabled"; + + [Theory] + [InlineData(NoUsername)] + [InlineData("fatal: could not read Username for 'https://gitlab.example.test': No such device or address")] + [InlineData("fatal: could not read Password for 'https://x-access-token@gitlab.example.test': terminal prompts disabled")] + [InlineData("remote: HTTP Basic: Access denied\nfatal: could not read Username for 'https://gitlab.example.test': terminal prompts disabled\n")] + public void A_tokened_command_refused_for_want_of_a_credential_by_a_git_before_2_31_fails_with_the_cause(string stderr) + { + var failure = TokenedGitCommand.OldGitFailure(tokened: true, stderr, Reading("git version 2.30.2")); + + var typed = failure.ShouldBeOfType(); + + typed.Version.ShouldBe(new GitVersion(2, 30, 2)); + typed.Message.ShouldContain("2.30.2", customMessage: "the message names the git the host has"); + typed.Message.ShouldContain("2.31", customMessage: "and the release that can carry the credential"); + typed.Message.ShouldContain("GIT_CONFIG_COUNT", customMessage: "and why an older one cannot"); + typed.Message.ShouldContain("Upgrade", customMessage: "and what to do about it"); + typed.Message.ShouldNotContain("could not read", customMessage: "the cryptic sentence is what this replaces"); + } + + [Fact] + public void The_typed_failure_is_still_a_workspace_failure_so_every_existing_catch_site_handles_it() + { + var failure = TokenedGitCommand.OldGitFailure(tokened: true, NoUsername, Reading("git version 2.30.2")); + + failure.ShouldBeAssignableTo("the run executor, the integrate nodes and the grader catch WorkspaceException; a sibling type would escape them"); + ((IFailure)failure!).Code.ShouldBe(FailureCodes.WorkspaceUnresolvable, "the wire code a client branches on is unchanged"); + ((IFailure)failure).Kind.ShouldBe(FailureKind.Unprocessable); + } + + public static TheoryData NotTheOldGitCase => new() + { + { true, NoUsername, "git version 2.31.0" }, // exactly the first release that reads GIT_CONFIG_COUNT: the credential really was missing or refused + { true, NoUsername, "git version 2.39.5" }, + { false, NoUsername, "git version 2.30.2" }, // untokened: no helper was ever going to be configured, so an old git explains nothing + { true, "fatal: repository 'https://gitlab.example.test/a/b.git/' not found", "git version 2.30.2" }, // a different failure; an old git clones a public repository fine + { true, "fatal: unable to access 'https://gitlab.example.test/a/b.git/': The requested URL returned error: 403", "git version 2.30.2" }, + { true, "", "git version 2.30.2" }, + { true, NoUsername, "fatal: not a git repository" }, // a git whose release is unknown cannot be blamed + }; + + [Theory] + [MemberData(nameof(NotTheOldGitCase))] + public void It_stays_out_of_the_way_unless_an_old_git_is_the_explanation(bool tokened, string stderr, string gitOutput) => + TokenedGitCommand.OldGitFailure(tokened, stderr, Reading(gitOutput)).ShouldBeNull(); + + [Fact] + public void The_host_overload_reads_the_hosts_git_and_a_test_can_stand_in_for_it() + { + using (HostGit.Use(Reading("git version 2.30.2"))) + TokenedGitCommand.OldGitFailure(tokened: true, NoUsername).ShouldBeOfType(); + + using (HostGit.Use(Reading("git version 2.39.5"))) + TokenedGitCommand.OldGitFailure(tokened: true, NoUsername).ShouldBeNull(); + } + + private static GitReading Reading(string output) => new(GitVersion.Parse(output), output); +}