From 90926c1d11a47c5c358c32956660fb1c53d82835 Mon Sep 17 00:00:00 2001 From: sacha Date: Fri, 24 Jul 2026 14:15:32 +0200 Subject: [PATCH] feat(git): resolve a single app installation by id on the credential broker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ResolveAppInstallationByIdAsync to IGitCredentialBroker: an O(1) point lookup of a single platform-app installation by its provider-side id. The Nexus setup-complete verifier currently confirms an installation by listing every installation of the app and filtering client-side — O(N) per connect. This gives it a direct by-id lookup instead. - GitHub adapter: GET /app/installations/{id} with the App JWT; a 404 maps to the new Git.InstallationNotFound error (Error.NotFound), routed via a GitRestErrorContext.ForInstallation so other statuses still propagate. - InMemoryGitServer + NullGitServer implement the new method. - New GitErrors.InstallationNotFound(installationId) factory. Claude-Session: https://claude.ai/code/session_01VFDn7gCH7vdvT4eoNhsMLr --- .../Connections/IGitCredentialBroker.cs | 17 ++++++ .../Compendium.Abstractions.Git/GitErrors.cs | 10 ++++ .../Stubs/NullGitServer.cs | 5 ++ .../Auth/GitHubCredentialBroker.cs | 28 +++++++++ .../Http/GitHubErrorMapper.cs | 10 ++++ .../Http/GitRestErrorContext.cs | 7 +++ .../Git/InMemoryGitServer.cs | 14 +++++ .../GitErrorsTests.cs | 12 ++++ .../InMemoryGitServerTests.cs | 25 ++++++++ .../NullGitServerTests.cs | 1 + .../GitHubCredentialBrokerTests.cs | 57 +++++++++++++++++++ 11 files changed, 186 insertions(+) diff --git a/src/Abstractions/Compendium.Abstractions.Git/Connections/IGitCredentialBroker.cs b/src/Abstractions/Compendium.Abstractions.Git/Connections/IGitCredentialBroker.cs index ba3a4f1..de4bd4f 100644 --- a/src/Abstractions/Compendium.Abstractions.Git/Connections/IGitCredentialBroker.cs +++ b/src/Abstractions/Compendium.Abstractions.Git/Connections/IGitCredentialBroker.cs @@ -47,6 +47,23 @@ Task> ResolveAppInstallationAsync( string? appKey = null, CancellationToken cancellationToken = default); + /// + /// App-installation mode only: resolves a single installation of the platform + /// app by its provider-side id. Unlike + /// (which probes by account) and (which + /// pages the whole list), this is an O(1) point lookup — use it when a caller + /// already holds an installation id and only needs to confirm it still belongs + /// to the app. Fails with Git.InstallationNotFound when the id does not + /// belong to the app (never installed, or the installation was deleted). + /// + /// The provider-side installation id to resolve. + /// Selects an app registration; = default app. + /// A cancellation token. + Task> ResolveAppInstallationByIdAsync( + string installationId, + string? appKey = null, + CancellationToken cancellationToken = default); + /// /// App-installation mode only: lists every installation of the platform app /// across all accounts. Adapters page through the provider API internally diff --git a/src/Abstractions/Compendium.Abstractions.Git/GitErrors.cs b/src/Abstractions/Compendium.Abstractions.Git/GitErrors.cs index 7c193e5..79f043e 100644 --- a/src/Abstractions/Compendium.Abstractions.Git/GitErrors.cs +++ b/src/Abstractions/Compendium.Abstractions.Git/GitErrors.cs @@ -88,6 +88,16 @@ public static Error AppNotInstalled(string @namespace, string? installUrl = null metadata); } + /// + /// The provider-side installation id does not belong to the platform app + /// (never installed, or the installation was deleted). Returned by the + /// by-id installation lookup on the credential broker. + /// + public static Error InstallationNotFound(string installationId) => + Error.NotFound( + $"{Prefix}.InstallationNotFound", + $"Installation '{installationId}' was not found for the platform app."); + /// /// The requested repository does not exist or is not visible to the credential. /// diff --git a/src/Abstractions/Compendium.Abstractions.Git/Stubs/NullGitServer.cs b/src/Abstractions/Compendium.Abstractions.Git/Stubs/NullGitServer.cs index b7b8347..f2e38f6 100644 --- a/src/Abstractions/Compendium.Abstractions.Git/Stubs/NullGitServer.cs +++ b/src/Abstractions/Compendium.Abstractions.Git/Stubs/NullGitServer.cs @@ -105,6 +105,11 @@ public Task> ResolveAppInstallationAsync( string @namespace, string? appKey = null, CancellationToken cancellationToken = default) => NotConfiguredTask(); + /// + public Task> ResolveAppInstallationByIdAsync( + string installationId, string? appKey = null, CancellationToken cancellationToken = default) + => NotConfiguredTask(); + /// public Task>> ListAppInstallationsAsync( string? appKey = null, CancellationToken cancellationToken = default) diff --git a/src/Adapters/Compendium.Adapters.GitHub/Auth/GitHubCredentialBroker.cs b/src/Adapters/Compendium.Adapters.GitHub/Auth/GitHubCredentialBroker.cs index a2089cd..be26bc0 100644 --- a/src/Adapters/Compendium.Adapters.GitHub/Auth/GitHubCredentialBroker.cs +++ b/src/Adapters/Compendium.Adapters.GitHub/Auth/GitHubCredentialBroker.cs @@ -172,6 +172,34 @@ public async Task> ResolveAppInstallationAsync( return Result.Failure(GitErrors.AppNotInstalled(@namespace, BuildInstallUrl(registration))); } + /// + public async Task> ResolveAppInstallationByIdAsync( + string installationId, string? appKey = null, CancellationToken cancellationToken = default) + { + ArgumentException.ThrowIfNullOrWhiteSpace(installationId); + + if (_options.ResolveApp(appKey) is not { } registration) + { + return Result.Failure(GitErrors.NotConfigured(GitHubDefaults.Provider)); + } + + var jwt = _tokenService.CreateAppJwt(registration); + if (jwt.IsFailure) + { + return Result.Failure(jwt.Error); + } + + var apiBase = GitHubDefaults.EnsureTrailingSlash(_options.ApiBaseUrl); + var installation = await _rest.GetAsync( + apiBase, jwt.Value, + $"app/installations/{Uri.EscapeDataString(installationId)}", + GitRestErrorContext.ForInstallation(installationId), cancellationToken).ConfigureAwait(false); + + return installation.IsFailure + ? Result.Failure(installation.Error) + : Result.Success(installation.Value.ToInstallationInfo()); + } + /// public async Task>> ListAppInstallationsAsync( string? appKey = null, CancellationToken cancellationToken = default) diff --git a/src/Adapters/Compendium.Adapters.GitHub/Http/GitHubErrorMapper.cs b/src/Adapters/Compendium.Adapters.GitHub/Http/GitHubErrorMapper.cs index 6ed2566..c2260ce 100644 --- a/src/Adapters/Compendium.Adapters.GitHub/Http/GitHubErrorMapper.cs +++ b/src/Adapters/Compendium.Adapters.GitHub/Http/GitHubErrorMapper.cs @@ -51,6 +51,11 @@ public static Error FromStatus( return GitErrors.NamespaceNotFound(ns); } + if (context.InstallationId is { } installationId) + { + return GitErrors.InstallationNotFound(installationId); + } + return GitErrors.ProviderRejected(Provider, statusCode, detail); case 409: @@ -94,6 +99,11 @@ public static Error FromException(Exception exception, GitRestErrorContext conte return GitErrors.NamespaceNotFound(ns); } + if (context.InstallationId is { } installationId) + { + return GitErrors.InstallationNotFound(installationId); + } + return GitErrors.ProviderRejected(Provider, 404, exception.Message); case ApiValidationException validation when LooksLikeConflict(validation.Message) diff --git a/src/Adapters/Compendium.Adapters.GitHub/Http/GitRestErrorContext.cs b/src/Adapters/Compendium.Adapters.GitHub/Http/GitRestErrorContext.cs index 1b681d3..a3ca5d3 100644 --- a/src/Adapters/Compendium.Adapters.GitHub/Http/GitRestErrorContext.cs +++ b/src/Adapters/Compendium.Adapters.GitHub/Http/GitRestErrorContext.cs @@ -20,6 +20,9 @@ internal sealed record GitRestErrorContext /// Gets the namespace this operation targeted; a 404 maps to Git.NamespaceNotFound. public string? Namespace { get; init; } + /// Gets the installation id this operation targeted; a 404 maps to Git.InstallationNotFound. + public string? InstallationId { get; init; } + /// Gets the resource name a conflict (409/422 "already exists") refers to. public string? ConflictResource { get; init; } @@ -33,4 +36,8 @@ public static GitRestErrorContext ForRepository(GitRepositoryRef repository) => /// Builds a context for a namespace-scoped operation. public static GitRestErrorContext ForNamespace(string @namespace) => new() { Namespace = @namespace, ConflictResource = @namespace }; + + /// Builds a context for a lookup of a single installation by its id. + public static GitRestErrorContext ForInstallation(string installationId) => + new() { InstallationId = installationId }; } diff --git a/src/Testing/Compendium.Testing/Git/InMemoryGitServer.cs b/src/Testing/Compendium.Testing/Git/InMemoryGitServer.cs index 784406a..ef1cafc 100644 --- a/src/Testing/Compendium.Testing/Git/InMemoryGitServer.cs +++ b/src/Testing/Compendium.Testing/Git/InMemoryGitServer.cs @@ -255,6 +255,20 @@ public Task> ResolveAppInstallationAsync( } } + /// + public Task> ResolveAppInstallationByIdAsync( + string installationId, string? appKey = null, CancellationToken cancellationToken = default) + { + Log("ResolveAppInstallationById", installationId); + lock (_gate) + { + var install = _installations.FirstOrDefault(i => i.InstallationId == installationId); + return Task.FromResult(install is null + ? Result.Failure(GitErrors.InstallationNotFound(installationId)) + : Result.Success(install)); + } + } + /// public Task>> ListAppInstallationsAsync( string? appKey = null, CancellationToken cancellationToken = default) diff --git a/tests/Unit/Compendium.Abstractions.Git.Tests/GitErrorsTests.cs b/tests/Unit/Compendium.Abstractions.Git.Tests/GitErrorsTests.cs index b9ad237..f48e087 100644 --- a/tests/Unit/Compendium.Abstractions.Git.Tests/GitErrorsTests.cs +++ b/tests/Unit/Compendium.Abstractions.Git.Tests/GitErrorsTests.cs @@ -126,6 +126,18 @@ public void AppNotInstalled_WithInstallUrl_CarriesTheInstallUrlMetadata() error.Metadata.Should().ContainKey("installUrl").WhoseValue.Should().Be("https://example.invalid/install"); } + [Fact] + public void InstallationNotFound_MapsToNotFound() + { + // Arrange / Act + var error = GitErrors.InstallationNotFound("555"); + + // Assert + error.Code.Should().Be("Git.InstallationNotFound"); + error.Type.Should().Be(ErrorType.NotFound); + error.Message.Should().Be("Installation '555' was not found for the platform app."); + } + [Fact] public void RepositoryNotFound_MapsToNotFound() { diff --git a/tests/Unit/Compendium.Abstractions.Git.Tests/InMemoryGitServerTests.cs b/tests/Unit/Compendium.Abstractions.Git.Tests/InMemoryGitServerTests.cs index 50bc041..8a0311d 100644 --- a/tests/Unit/Compendium.Abstractions.Git.Tests/InMemoryGitServerTests.cs +++ b/tests/Unit/Compendium.Abstractions.Git.Tests/InMemoryGitServerTests.cs @@ -303,6 +303,31 @@ public async Task ResolveAppInstallation_WhenSeeded_ReturnsInstallation() result.Value.InstallationId.Should().Be("inst-1"); } + [Fact] + public async Task ResolveAppInstallationById_WhenSeeded_ReturnsInstallation() + { + // Arrange + _server.SeedInstallation(new GitInstallationInfo("inst-1", "acme", GitAccountType.Organization)); + + // Act + var result = await _server.Credentials.ResolveAppInstallationByIdAsync("inst-1"); + + // Assert + result.IsSuccess.Should().BeTrue(); + result.Value.AccountLogin.Should().Be("acme"); + } + + [Fact] + public async Task ResolveAppInstallationById_WhenAbsent_FailsInstallationNotFound() + { + // Arrange / Act + var result = await _server.Credentials.ResolveAppInstallationByIdAsync("missing"); + + // Assert + result.IsFailure.Should().BeTrue(); + result.Error.Code.Should().Be("Git.InstallationNotFound"); + } + [Fact] public async Task ListAppInstallations_ReturnsSeededInstallations() { diff --git a/tests/Unit/Compendium.Abstractions.Git.Tests/NullGitServerTests.cs b/tests/Unit/Compendium.Abstractions.Git.Tests/NullGitServerTests.cs index 0f7e0b5..e2d1a0d 100644 --- a/tests/Unit/Compendium.Abstractions.Git.Tests/NullGitServerTests.cs +++ b/tests/Unit/Compendium.Abstractions.Git.Tests/NullGitServerTests.cs @@ -95,6 +95,7 @@ public async Task Credentials_EveryMethod_ReturnsNotConfigured() AssertNotConfigured(await broker.MintAsync(Connection, new GitAccessTokenScope())); AssertNotConfigured(await broker.ValidateAsync(Connection)); AssertNotConfigured(await broker.ResolveAppInstallationAsync("acme")); + AssertNotConfigured(await broker.ResolveAppInstallationByIdAsync("inst-1")); AssertNotConfigured(await broker.ListAppInstallationsAsync()); } diff --git a/tests/Unit/Compendium.Adapters.GitHub.Tests/GitHubCredentialBrokerTests.cs b/tests/Unit/Compendium.Adapters.GitHub.Tests/GitHubCredentialBrokerTests.cs index c7049d4..a1633f0 100644 --- a/tests/Unit/Compendium.Adapters.GitHub.Tests/GitHubCredentialBrokerTests.cs +++ b/tests/Unit/Compendium.Adapters.GitHub.Tests/GitHubCredentialBrokerTests.cs @@ -156,6 +156,63 @@ public async Task ResolveAppInstallation_NotInstalled_ReturnsAnInstallUrl() result.Error.Metadata["installUrl"].Should().Be("https://github.com/apps/compendium-app/installations/new"); } + [Fact] + public async Task ResolveAppInstallationById_FindsTheInstallation() + { + using var harness = new GitHubTestHarness(); + harness.Server.Given(Request.Create().WithPath("/app/installations/555").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200).WithBodyAsJson(new + { + id = 555, + account = new { login = "acme", type = "Organization" }, + suspended_at = "2026-01-02T03:04:05Z", + })); + + var result = await harness.Broker.ResolveAppInstallationByIdAsync("555"); + + result.IsSuccess.Should().BeTrue(); + result.Value.InstallationId.Should().Be("555"); + result.Value.AccountLogin.Should().Be("acme"); + result.Value.AccountType.Should().Be(GitAccountType.Organization); + result.Value.Suspended.Should().BeTrue(); + } + + [Fact] + public async Task ResolveAppInstallationById_MapsAUserAccount_AndReportsNotSuspended() + { + using var harness = new GitHubTestHarness(); + harness.Server.Given(Request.Create().WithPath("/app/installations/42").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(200) + .WithBodyAsJson(new { id = 42, account = new { login = "octocat", type = "User" } })); + + var result = await harness.Broker.ResolveAppInstallationByIdAsync("42"); + + result.Value.AccountType.Should().Be(GitAccountType.User); + result.Value.Suspended.Should().BeFalse(); + } + + [Fact] + public async Task ResolveAppInstallationById_WhenUnknown_FailsInstallationNotFound() + { + using var harness = new GitHubTestHarness(); + harness.Server.Given(Request.Create().WithPath("/app/installations/999").UsingGet()) + .RespondWith(Response.Create().WithStatusCode(404)); + + var result = await harness.Broker.ResolveAppInstallationByIdAsync("999"); + + result.Error.Code.Should().Be("Git.InstallationNotFound"); + result.Error.Type.Should().Be(ErrorType.NotFound); + } + + [Fact] + public async Task ResolveAppInstallationById_UnknownAppKey_FailsNotConfigured() + { + using var harness = new GitHubTestHarness(); + + (await harness.Broker.ResolveAppInstallationByIdAsync("555", "missing")) + .Error.Code.Should().Be("Git.NotConfigured"); + } + [Fact] public async Task ListAppInstallations_PagesThroughEveryInstallation() {