From aa92be3d062ac3d1162863cb6ca62cfe5d8e8fc5 Mon Sep 17 00:00:00 2001 From: Mike Olson Date: Wed, 14 Oct 2026 19:41:25 -0400 Subject: [PATCH 1/5] fix(server): Stop re-probing hosting CLIs for unrecognized hosts Remember the unknown-remote refinement verdict for five minutes, shared by checkouts whose remotes have the same server and path prefix. Callers that arrive while a probe runs wait on it. A missing CLI, a missing GitHub login, or glab reporting no login for the host counts as an answer. A timeout, an API or keyring failure, or a claim reached after an earlier check failed is not cached. glab is asked about the remote's own host, spelled as the remote spells it, and a Settings change clears the cache. Fixes #12060 Co-authored-by: Victor Vogel <263261067+only21mil@users.noreply.github.com> Co-authored-by: Jorrin <43169049+JorrinKievit@users.noreply.github.com> --- .../SourceControlProviderRegistry.test.ts | 313 +++++++++++++++++- .../SourceControlProviderRegistry.ts | 84 ++++- .../src/server/discovery.ts | 72 ++-- .../server/ForgejoSourceControlProvider.ts | 30 +- .../server/GitCafeSourceControlProvider.ts | 2 +- .../src/server/GitHubSourceControlProvider.ts | 18 +- .../src/server/GitLabSourceControlProvider.ts | 15 +- 7 files changed, 482 insertions(+), 52 deletions(-) diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index 2ff7fdb0d702..c7a307ae4c3f 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -2,12 +2,23 @@ import { SourceControlProviderKind } from "@t3tools/contracts"; import { assert, it } from "@effect/vitest"; import * as NodeServices from "@effect/platform-node/NodeServices"; import * as DateTime from "effect/DateTime"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import * as PlatformError from "effect/PlatformError"; +import * as PubSub from "effect/PubSub"; import * as Redacted from "effect/Redacted"; +import * as Stream from "effect/Stream"; +import * as TestClock from "effect/testing/TestClock"; import { ChildProcessSpawner } from "effect/process"; -import { VcsRepositoryDetectionError } from "@t3tools/contracts"; +import { + DEFAULT_SERVER_SETTINGS, + VcsProcessSpawnError, + VcsProcessTimeoutError, + VcsRepositoryDetectionError, +} from "@t3tools/contracts"; import * as ServerSettings from "../serverSettings.ts"; import * as ServerConfig from "../config.ts"; @@ -54,6 +65,9 @@ function makeRegistry(input: { readonly process?: Partial; readonly githubApi?: Partial; readonly gitlab?: Partial; + readonly forgejo?: Partial; + readonly settings?: Partial; + readonly started?: Effect.Effect; readonly resolve?: VcsDriverRegistry.VcsDriverRegistry["Service"]["resolve"]; }) { const driver = { @@ -99,6 +113,7 @@ function makeRegistry(input: { }); return SourceControlProviderRegistry.make.pipe( + Effect.tap(input.started ?? Effect.void), Effect.provide( Layer.mergeAll( NodeServices.layer, @@ -118,7 +133,9 @@ function makeRegistry(input: { Layer.mock(AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli)({}), Layer.mock(BitbucketApi.BitbucketApi)({}), Layer.mock(BitbucketPullRequestApi.BitbucketPullRequestApi)({}), - ServerSettings.ServerSettingsService.layerTest(), + input.settings + ? Layer.mock(ServerSettings.ServerSettingsService)(input.settings) + : ServerSettings.ServerSettingsService.layerTest(), Layer.mock(GitHubPullRequestApi.GitHubPullRequestApi)({}), Layer.mock(GitHubApi.GitHubApi)({ // No GitHub credential unless a test supplies one, so custom hosts stay unclaimed. @@ -128,7 +145,10 @@ function makeRegistry(input: { Layer.mock(GitVcsDriver.GitVcsDriver)({}), Layer.mock(GitLabCli.GitLabCli)(input.gitlab ?? {}), Layer.mock(GitLabPullRequestCli.GitLabPullRequestCli)({}), - Layer.mock(ForgejoCli.ForgejoCli)({ listLogins: () => Effect.succeed([]) }), + Layer.mock(ForgejoCli.ForgejoCli)({ + listLogins: () => Effect.succeed([]), + ...input.forgejo, + }), Layer.mock(GitCafeApi.GitCafeApi)({}), Layer.mock(GitCafeCredentials.GitCafeCredentials)({ cliEnv: () => Effect.succeed({}), @@ -459,3 +479,290 @@ it.effect("skips GitHub discovery for the identity resolver's empty base URL", ( assert.deepStrictEqual(hosts, ["githubenterprise.dev.example.com"]); }), ); + +const notFound = (command: string, cwd: string, module = "ChildProcess", method = "spawn") => + new VcsProcessSpawnError({ + operation: "test", + command, + cwd, + cause: PlatformError.systemError({ _tag: "NotFound", module, method }), + }); + +const resolveUnknown = ( + registry: SourceControlProviderRegistry.SourceControlProviderRegistry["Service"], + repository: string, + host = "git.example.internal", +) => + registry.resolveHandle({ + cwd: `/${repository}`, + context: { + provider: { + kind: SourceControlProviderKind.make("unknown"), + name: host.toLowerCase(), + baseUrl: `https://${host.toLowerCase()}`, + }, + remoteName: "origin", + remoteUrl: `git@${host}:team/${repository}.git`, + }, + }); + +it.effect("asks once for every checkout of a server no integration claims", () => + Effect.gen(function* () { + const asked: Array = []; + const release = yield* Deferred.make(); + const registry = yield* makeRegistry({ + remotes: [], + process: { + run: ({ command, cwd }) => { + asked.push(command); + return Deferred.await(release).pipe(Effect.andThen(Effect.fail(notFound(command, cwd)))); + }, + }, + forgejo: { + listLogins: ({ command = "tea", cwd }) => { + asked.push(command); + const cause = notFound(command, cwd); + return Effect.fail(new ForgejoCli.ForgejoCliError({ command, cwd, detail: "", cause })); + }, + }, + }); + + // Each caller runs until it waits, so once all have started the first probe is still open. + const started = yield* Effect.forEach(["a", "b", "c"], () => Deferred.make()); + const callers = yield* Effect.forEach( + ["a", "b", "c"], + (repository, index) => + Deferred.succeed(started[index]!, undefined).pipe( + Effect.andThen(resolveUnknown(registry, repository)), + ), + { concurrency: "unbounded" }, + ).pipe(Effect.forkChild); + yield* Effect.forEach(started, Deferred.await); + assert.deepStrictEqual(asked, ["glab"]); + yield* Deferred.succeed(release, undefined); + const handles = yield* Fiber.join(callers); + yield* resolveUnknown(registry, "d"); + assert.deepStrictEqual( + handles.map((handle) => [handle.provider.kind, handle.context?.remoteUrl]), + ["a", "b", "c"].map((name) => ["unknown", `git@git.example.internal:team/${name}.git`]), + ); + assert.deepStrictEqual(asked, ["glab", "fj", "tea"]); + + // A login made in a terminal sends no signal, so the server is asked again later. + yield* TestClock.adjust("5 minutes"); + yield* resolveUnknown(registry, "a"); + assert.strictEqual(asked.length, 6); + }), +); + +it.effect.each(["tea", "gh"] as const)("asks again after %s could not run", (failing) => + Effect.gen(function* () { + let attempts = 0; + const fail = (error: E) => { + attempts += 1; + return Effect.fail(error); + }; + const registry = yield* makeRegistry({ + remotes: [], + process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, + githubApi: { + credential: (host) => + failing === "gh" + ? fail(new GitHubCredentials.GitHubCliFailedError({ host, cause: "timeout" })) + : Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })), + }, + forgejo: { + listLogins: ({ command, cwd }) => + failing === "tea" && command !== "fj" + ? fail(new ForgejoCli.ForgejoCliError({ command: "tea", cwd, detail: "Failed." })) + : Effect.succeed([]), + }, + }); + for (const repository of ["a", "b"]) { + const handle = yield* resolveUnknown(registry, repository); + assert.strictEqual(handle.provider.kind, "unknown"); + } + assert.strictEqual(attempts, 2); + }), +); + +const glabReply = (stderr: string, exitCode = 1) => + Effect.succeed(processOutput("", { stderr, exitCode: ChildProcessSpawner.ExitCode(exitCode) })); +const glabLogin = (host = "git.example.internal") => + glabReply(`${host}\n ✓ Logged in to ${host} as maria (keyring)\n`, 0); + +it.effect.each([ + { name: "a login", replies: [glabLogin()], found: ["gitlab", "gitlab"], asks: 1 }, + { + name: "a login under the remote's spelling", + host: "Git.Example.Internal", + replies: [glabLogin("Git.Example.Internal")], + found: ["gitlab", "gitlab"], + asks: 1, + }, + ...[ + ["no login", "x git.example.internal has not been authenticated with glab"], + ["no login on glab 1.22", "x git.example.internal not authenticated with glab"], + ["no instances", "no GitLab instances have been authenticated with glab"], + ["no instance on glab 1.22", "No GitLab instance has been authenticated with glab"], + ].map(([name, message]) => ({ + name, + host: undefined, + replies: [glabReply(message!)], + found: ["unknown", "unknown"], + asks: 1, + })), + { + name: "a failed check", + replies: [glabReply("git.example.internal\n x git.example.internal: API call failed\n")], + found: ["unknown", "unknown"], + asks: 2, + }, + { + name: "a timeout", + replies: [ + Effect.fail( + new VcsProcessTimeoutError({ operation: "test", command: "glab", cwd: "/a", timeoutMs: 1 }), + ), + ], + found: ["unknown", "unknown"], + asks: 2, + }, + { + name: "a check failed in a missing checkout", + replies: [Effect.fail(notFound("glab", "/a", "FileSystem", "access")), glabLogin()], + found: ["unknown", "gitlab"], + asks: 2, + }, +])("asks glab about the remote's own host and keeps $name", (scenario) => + Effect.gen(function* () { + const host = scenario.host ?? "git.example.internal"; + const asked: Array> = []; + const registry = yield* makeRegistry({ + remotes: [], + process: { + // glab holds a login only under the host's exact spelling. + run: ({ args, cwd }) => { + asked.push([cwd, ...args]); + return args[3] === host + ? (scenario.replies[asked.length - 1] ?? scenario.replies[0]!) + : glabReply("x git.example.internal has not been authenticated with glab"); + }, + }, + }); + const found = yield* Effect.forEach(["a", "b"], (repository) => + resolveUnknown(registry, repository, host).pipe(Effect.map(({ provider }) => provider.kind)), + ); + assert.deepStrictEqual(found, scenario.found); + assert.deepStrictEqual( + asked, + ["/a", "/b"] + .slice(0, scenario.asks) + .map((cwd) => [cwd, "auth", "status", "--hostname", host]), + ); + }), +); + +it.effect("does not keep a tea login found while fj could not be read", () => + Effect.gen(function* () { + let fjReadable = false; + const loginAt = (port: number) => ({ + name: "login", + url: `https://git.example.internal:${port}`, + ssh_host: "git.example.internal", + user: "maria", + default: "true", + }); + const registry = yield* makeRegistry({ + remotes: [], + process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, + forgejo: { + listLogins: ({ command, cwd }) => + command === "fj" && !fjReadable + ? Effect.fail(new ForgejoCli.ForgejoCliError({ command, cwd, detail: "Unreadable." })) + : Effect.succeed([loginAt(command === "fj" ? 4000 : 3000)]), + }, + }); + const fallback = yield* resolveUnknown(registry, "a"); + assert.strictEqual(fallback.context?.provider.baseUrl, "https://git.example.internal:3000"); + fjReadable = true; + const recovered = yield* resolveUnknown(registry, "b"); + assert.strictEqual(recovered.context?.provider.baseUrl, "https://git.example.internal:4000"); + }), +); + +it.effect.each([ + { first: "https://host-a.example/repo.git", second: "https://host-b.example/repo.git" }, + { first: "ssh://git@host-a.example/repo.git", second: "ssh://git@host-b.example/repo.git" }, + { first: "https://host.example:8443/repo.git", second: "https://host.example:9443/repo.git" }, +])("keeps the lookups for $first and $second apart", ({ first, second }) => + Effect.gen(function* () { + const asked: Array = []; + const registry = yield* makeRegistry({ + remotes: [], + process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, + forgejo: { + listLogins: ({ command, remoteUrl }) => { + if (command === "tea") asked.push(remoteUrl); + return Effect.succeed([]); + }, + }, + }); + for (const remoteUrl of [first, second]) { + // The identity resolver's context, where the remote alone names the server. + const provider = { + kind: SourceControlProviderKind.make("unknown"), + name: "Unknown", + baseUrl: "", + }; + yield* registry.resolveHandle({ + cwd: "/repo", + context: { provider, remoteName: "origin", remoteUrl }, + }); + } + assert.deepStrictEqual(asked, [first, second]); + }), +); + +it.effect("asks again once Settings change", () => + Effect.gen(function* () { + const changes = yield* PubSub.unbounded(); + const handled = yield* Effect.forEach([0, 1], () => Deferred.make()); + let changeCount = 0; + const markHandled = Effect.suspend(() => Deferred.succeed(handled[changeCount++]!, undefined)); + let signedIn = false; + const registry = yield* makeRegistry({ + remotes: [], + settings: { + subscribeChanges: PubSub.subscribe(changes).pipe( + Effect.map((subscription) => + // A change is marked handled once the registry asks for the next one. + Stream.fromSubscription(subscription).pipe( + Stream.flatMap((settings) => + Stream.concat(Stream.make(settings), Stream.fromEffectDrain(markHandled)), + ), + ), + ), + ), + }, + // Saved as the registry finishes starting, before its consumer has run. + started: PubSub.publish(changes, DEFAULT_SERVER_SETTINGS), + process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, + githubApi: { + credential: (host) => + signedIn + ? Effect.succeed({ token: Redacted.make("token"), fingerprint: `${host}:fingerprint` }) + : Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })), + }, + }); + yield* Deferred.await(handled[0]!); + assert.strictEqual((yield* resolveUnknown(registry, "a")).provider.kind, "unknown"); + signedIn = true; + assert.strictEqual((yield* resolveUnknown(registry, "b")).provider.kind, "unknown"); + + // A token saved for the host in Settings. + yield* PubSub.publish(changes, DEFAULT_SERVER_SETTINGS); + yield* Deferred.await(handled[1]!); + assert.strictEqual((yield* resolveUnknown(registry, "c")).provider.kind, "github"); + }), +); diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts index 821e24bf7bc3..95d3b6093d31 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts @@ -3,8 +3,11 @@ import * as Cache from "effect/Cache"; import * as Context from "effect/Context"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; +import * as Equal from "effect/Equal"; import * as Exit from "effect/Exit"; +import * as Hash from "effect/Hash"; import * as Layer from "effect/Layer"; +import * as Stream from "effect/Stream"; import { SourceControlProviderError, type SourceControlProviderDiscoveryItem, @@ -18,14 +21,19 @@ import { probeSourceControlProvider, refineUnknownRemoteProvider, type SourceControlProviderDiscoverySpec, + type UnknownRemoteVerdict, } from "@t3tools/source-control-core/server/discovery"; import * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; import * as ServerConfig from "../config.ts"; +import * as ServerSettings from "../serverSettings.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; const PROVIDER_DETECTION_CACHE_CAPACITY = 2_048; const PROVIDER_DETECTION_CACHE_TTL = Duration.seconds(5); +const UNKNOWN_REMOTE_CACHE_CAPACITY = 512; +// Outlasts the one-minute pull request sweep. A login made since is noticed after this. +const UNKNOWN_REMOTE_CACHE_TTL = Duration.minutes(5); export interface SourceControlProviderRegistration { readonly kind: SourceControlProviderKind; @@ -150,6 +158,35 @@ function selectProviderContext( ); } +/** + * Checkouts of one server share a lookup. The key omits `cwd`, which matters only when a check fails + * there, and such a verdict is not kept. Equality ignores the lookup, so this is not a `Data.Class`. + */ +class UnknownRemoteLookup { + readonly key: string; + readonly lookup: Effect.Effect; + + constructor( + context: SourceControlProvider.SourceControlProviderContext, + lookup: Effect.Effect, + ) { + this.lookup = lookup; + this.key = [ + context.provider.name, + context.provider.baseUrl, + context.remoteUrl.trim().replace(/(?) { const config = yield* ServerConfig.ServerConfig; + const serverSettings = yield* ServerSettings.ServerSettingsService; const { process } = yield* SourceControlHost.SourceControlHost; const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry; const providers = new Map< @@ -209,6 +247,36 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit const get: SourceControlProviderRegistry["Service"]["get"] = (kind) => Effect.succeed(providers.get(kind) ?? unsupportedProvider(kind)); + // A verdict a check could not give is shared with callers already waiting on it, then dropped. + const unknownRemoteCache = yield* Cache.makeWith( + (request: UnknownRemoteLookup) => request.lookup, + { + capacity: UNKNOWN_REMOTE_CACHE_CAPACITY, + timeToLive: (exit) => + Exit.isSuccess(exit) && exit.value.settled ? UNKNOWN_REMOTE_CACHE_TTL : Duration.zero, + }, + ); + // A credential saved or a host turned on or off in Settings clears the kept verdicts. The + // subscription is taken here so a change made before the consumer starts is not lost. + const settingsChanges = yield* serverSettings.subscribeChanges; + yield* Stream.runForEach(settingsChanges, () => Cache.invalidateAll(unknownRemoteCache)).pipe( + Effect.forkScoped, + ); + + const refineContext = ( + cwd: string, + context: SourceControlProvider.SourceControlProviderContext | null, + ): Effect.Effect => + context === null || context.provider.kind !== "unknown" + ? Effect.succeed(context) + : Cache.get( + unknownRemoteCache, + new UnknownRemoteLookup( + context, + refineUnknownRemoteProvider({ specs: discoverySpecs, process, cwd, context }), + ), + ).pipe(Effect.map(({ provider }) => (provider ? { ...context, provider } : context))); + const detectProviderContext = Effect.fn("SourceControlProviderRegistry.detectProviderContext")( function* (cwd: string) { const handle = yield* vcsRegistry.resolve({ cwd }).pipe( @@ -235,14 +303,7 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit }), ), ); - const context = selectProviderContext(remotes.remotes); - - return yield* refineUnknownRemoteProvider({ - specs: discoverySpecs, - process, - cwd, - context, - }); + return yield* refineContext(cwd, selectProviderContext(remotes.remotes)); }, ); @@ -258,12 +319,7 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit const resolveHandle: SourceControlProviderRegistry["Service"]["resolveHandle"] = (input) => (input.context === undefined ? Cache.get(providerContextCache, input.cwd) - : refineUnknownRemoteProvider({ - specs: discoverySpecs, - process, - cwd: input.cwd, - context: input.context, - }) + : refineContext(input.cwd, input.context) ).pipe( Effect.map((context) => { const kind = context?.provider.kind ?? SourceControlProviderKind.make("unknown"); diff --git a/packages/source-control-core/src/server/discovery.ts b/packages/source-control-core/src/server/discovery.ts index 70d26b4a963c..8c69375aee57 100644 --- a/packages/source-control-core/src/server/discovery.ts +++ b/packages/source-control-core/src/server/discovery.ts @@ -4,14 +4,17 @@ * @module source-control-core/server/discovery */ import * as NodeUtil from "node:util"; -import type { - SourceControlProviderAuth, - SourceControlProviderDiscoveryItem, - SourceControlProviderInfo, - SourceControlProviderKind, +import { + VcsProcessSpawnError, + type SourceControlProviderAuth, + type SourceControlProviderDiscoveryItem, + type SourceControlProviderInfo, + type SourceControlProviderKind, } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; +import * as PlatformError from "effect/PlatformError"; +import * as Schema from "effect/Schema"; import type * as SourceControlProvider from "./SourceControlProvider.ts"; import type * as SourceControlHost from "./SourceControlHost.ts"; @@ -39,12 +42,15 @@ export type SourceControlCliDiscoverySpec = SourceControlDiscoverySpecBase & { readonly executable: string; readonly versionArgs: ReadonlyArray; readonly authArgs: ReadonlyArray; - readonly remoteRefinementArgs?: ReadonlyArray; + readonly remoteRefinementArgs?: ( + context: SourceControlProvider.SourceControlProviderContext, + ) => ReadonlyArray; readonly probeTimeoutMs?: number; readonly parseAuth: (input: SourceControlAuthProbeInput) => SourceControlProviderAuth; + /** `null` when nothing claims the remote; `undefined` when the output cannot say. */ readonly refineUnknownRemote?: ( input: SourceControlUnknownRemoteRefinementInput, - ) => SourceControlProviderInfo | null; + ) => SourceControlProviderInfo | null | undefined; }; export type SourceControlApiDiscoverySpec = SourceControlDiscoverySpecBase & { @@ -58,7 +64,7 @@ export type SourceControlManagedCliDiscoverySpec = SourceControlDiscoverySpecBas readonly refineUnknownRemote: (input: { readonly cwd: string; readonly context: SourceControlProvider.SourceControlProviderContext; - }) => Effect.Effect; + }) => Effect.Effect; }; export type SourceControlProviderDiscoverySpec = @@ -294,28 +300,47 @@ export function probeSourceControlProvider(input: { ); } +const isVcsProcessSpawnError = Schema.is(VcsProcessSpawnError); + +/** + * A hosting CLI that is not installed claims no remote. The spawner checks the cwd first, so only + * the executable's own ENOENT comes from `ChildProcess`; a missing checkout fails elsewhere. + */ +export function isMissingExecutable(error: unknown): boolean { + return ( + isVcsProcessSpawnError(error) && + PlatformError.isPlatformError(error.cause) && + error.cause.reason._tag === "NotFound" && + error.cause.reason.module === "ChildProcess" + ); +} + +export interface UnknownRemoteVerdict { + readonly provider: SourceControlProviderInfo | null; + /** Every check ahead of the answer ran, so another checkout of the server would hear the same. */ + readonly settled: boolean; +} + export const refineUnknownRemoteProvider = Effect.fn("refineUnknownRemoteProvider")( function* (input: { readonly specs: ReadonlyArray; readonly process: SourceControlHost.SourceControlHost["Service"]["process"]; readonly cwd: string; - readonly context: SourceControlProvider.SourceControlProviderContext | null; - }): Effect.fn.Return { - if (input.context === null || input.context.provider.kind !== "unknown") { - return input.context; - } + readonly context: SourceControlProvider.SourceControlProviderContext; + }): Effect.fn.Return { const context = input.context; - const providers = yield* Effect.forEach(input.specs, (spec) => { + const verdicts = yield* Effect.forEach(input.specs, (spec) => { if (spec.type === "managed-cli") { return spec.refineUnknownRemote({ cwd: input.cwd, context }); } - if (!isCliRemoteRefinementSpec(spec)) return Effect.succeed(null); + if (!isCliRemoteRefinementSpec(spec)) + return Effect.succeed({ provider: null, settled: true }); return input.process .run({ operation: "source-control.discovery.refine-unknown-remote", command: spec.executable, - args: spec.remoteRefinementArgs ?? spec.authArgs, + args: spec.remoteRefinementArgs?.(context) ?? spec.authArgs, cwd: input.cwd, allowNonZeroExit: true, timeoutMs: probeTimeoutMs(spec), @@ -330,11 +355,20 @@ export const refineUnknownRemoteProvider = Effect.fn("refineUnknownRemoteProvide auth, }), ), - Effect.orElseSucceed(() => null), + Effect.catchIf(isMissingExecutable, () => Effect.succeed(null)), + Effect.orElseSucceed(() => undefined), + Effect.map((provider) => ({ + provider: provider ?? null, + settled: provider !== undefined, + })), ); }); - const provider = providers.find((candidate) => candidate !== null); - return provider ? { ...context, provider } : context; + let settled = true; + for (const verdict of verdicts) { + settled &&= verdict.settled; + if (verdict.provider) return { provider: verdict.provider, settled }; + } + return { provider: null, settled }; }, ); diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts index 4ea8e16ed35f..8ebe91bcf08b 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts @@ -8,6 +8,7 @@ import * as SourceControlHost from "@t3tools/source-control-core/server/SourceCo import * as ForgejoCli from "./ForgejoCli.ts"; import * as SourceControlProvider from "@t3tools/source-control-core/server/SourceControlProvider"; import { + isMissingExecutable, providerAuth, probeSourceControlProvider, type SourceControlCliDiscoverySpec, @@ -24,7 +25,7 @@ export const discovery = { executable: "tea", versionArgs: ["--version"], authArgs: ["login", "status", "--output", "json"], - remoteRefinementArgs: ["login", "list", "--output", "json"], + remoteRefinementArgs: () => ["login", "list", "--output", "json"], parseAuth: (input) => { const logins = ForgejoCli.parseForgejoLogins(input.stdout); const login = logins.find((entry) => entry.default === "true") ?? logins[0]; @@ -149,22 +150,37 @@ export const makeDiscovery = Effect.gen(function* () { readonly context: SourceControlProvider.SourceControlProviderContext; }) { const remote = ForgejoCli.parseForgejoRemote(input.context.remoteUrl); - if (!remote) return null; + if (!remote) return { provider: null, settled: true }; + let settled = true; for (const command of ["fj", "tea"] as const) { const logins = yield* listLogins({ cwd: input.cwd, command, remoteUrl: input.context.remoteUrl, - }).pipe(Effect.orElseSucceed(() => [])); + }).pipe( + // A CLI that is not installed holds no login; any other failure leaves it unasked. + Effect.catchIf( + (error) => isMissingExecutable(error.cause), + () => Effect.succeed([]), + ), + Effect.orElseSucceed(() => undefined), + ); + if (logins === undefined) { + settled = false; + continue; + } const login = ForgejoCli.matchForgejoLogin(logins, remote, input.context.requestedHost); if (login) return { - kind: SourceControlProviderKind.make("forgejo"), - name: discovery.label, - baseUrl: login.url, + provider: { + kind: SourceControlProviderKind.make("forgejo"), + name: discovery.label, + baseUrl: login.url, + }, + settled, }; } - return null; + return { provider: null, settled }; }, ), } satisfies SourceControlManagedCliDiscoverySpec; diff --git a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts index db4bea8dbf4c..8258e4aaff1b 100644 --- a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts @@ -118,7 +118,7 @@ export const makeDiscovery = Effect.gen(function* () { }), ), ), - refineUnknownRemote: () => Effect.succeed(null), + refineUnknownRemote: () => Effect.succeed({ provider: null, settled: true }), } satisfies SourceControlManagedCliDiscoverySpec; }); diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index c52b3c2dd2c8..803f1af714e3 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -268,20 +268,26 @@ export const makeDiscovery = Effect.gen(function* () { refineUnknownRemote: ({ context }) => { // Identity resolution also probes providers before a web base URL is known. const url = URL.parse(context.provider.baseUrl); - if (!url?.host) return Effect.succeed(null); + if (!url?.host) return Effect.succeed({ provider: null, settled: true }); return api.credential(url.host).pipe( Effect.as(true), - Effect.catchTags({ GitHubHostDisabledError: () => Effect.succeed(true) }), - Effect.orElseSucceed(() => false), - Effect.map((known) => - known + Effect.catchTags({ + GitHubCliMissingError: () => Effect.succeed(false), + GitHubHostDisabledError: () => Effect.succeed(true), + GitHubNotSignedInError: () => Effect.succeed(false), + }), + Effect.map((known) => ({ + provider: known ? ({ kind: SourceControlProviderKind.make("github"), name: "GitHub Self-Hosted", baseUrl: context.provider.baseUrl, } as const) : null, - ), + settled: true, + })), + // gh timed out or failed, so it is unknown whether it holds a login for the host. + Effect.orElseSucceed(() => ({ provider: null, settled: false })), ); }, } satisfies SourceControlManagedCliDiscoverySpec; diff --git a/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.ts b/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.ts index 6c15029d0e3f..e7d62ab8e8c4 100644 --- a/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.ts +++ b/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.ts @@ -80,14 +80,19 @@ function parseGitLabAuth(input: SourceControlAuthProbeInput) { }); } +// What glab prints when it holds no login for the host, across its 1.x releases. +const GITLAB_HOST_NOT_LOGGED_IN_PATTERN = + /\bnot (?:been )?authenticated with glab\b|\bno gitlab instances? ha(?:s|ve) been authenticated\b/iu; + function refineUnknownGitLabRemote(input: SourceControlUnknownRemoteRefinementInput) { const host = input.context.provider.name.toLowerCase(); - const authenticated = parseGitLabAuthStatusHosts(combinedAuthOutput(input.auth)).some( + const output = combinedAuthOutput(input.auth); + const authenticated = parseGitLabAuthStatusHosts(output).some( (entry) => entry.account !== null && entry.host === host, ); if (!authenticated) { - return null; + return GITLAB_HOST_NOT_LOGGED_IN_PATTERN.test(output) ? null : undefined; } return { @@ -104,6 +109,12 @@ export const discovery = { executable: "glab", versionArgs: ["--version"], authArgs: ["auth", "status"], + // glab matches its configured hosts by exact name, so ask with the remote's own spelling. + remoteRefinementArgs: ({ provider, remoteUrl }) => { + const at = remoteUrl.toLowerCase().indexOf(provider.name.toLowerCase()); + const host = at < 0 ? provider.name : remoteUrl.slice(at, at + provider.name.length); + return ["auth", "status", "--hostname", host]; + }, parseAuth: parseGitLabAuth, refineUnknownRemote: refineUnknownGitLabRemote, installHint: From bfed68569df86d1bfac5593aaa414c73a7400464 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Thu, 15 Oct 2026 09:52:38 -0700 Subject: [PATCH 2/5] test(server): trim unknown-host cache tests Keep three registry tests for the unknown-remote refinement cache: checkouts of one server share a probe until the TTL lapses (with glab asked by --hostname), an unanswered check is probed again, and a Settings change clears kept verdicts. Co-Authored-By: Claude Opus 5.5 --- .../SourceControlProviderRegistry.test.ts | 226 ++++-------------- 1 file changed, 42 insertions(+), 184 deletions(-) diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index c7a307ae4c3f..3e7430157554 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -67,7 +67,6 @@ function makeRegistry(input: { readonly gitlab?: Partial; readonly forgejo?: Partial; readonly settings?: Partial; - readonly started?: Effect.Effect; readonly resolve?: VcsDriverRegistry.VcsDriverRegistry["Service"]["resolve"]; }) { const driver = { @@ -113,7 +112,6 @@ function makeRegistry(input: { }); return SourceControlProviderRegistry.make.pipe( - Effect.tap(input.started ?? Effect.void), Effect.provide( Layer.mergeAll( NodeServices.layer, @@ -480,29 +478,28 @@ it.effect("skips GitHub discovery for the identity resolver's empty base URL", ( }), ); -const notFound = (command: string, cwd: string, module = "ChildProcess", method = "spawn") => +const notFound = (command: string, cwd: string) => new VcsProcessSpawnError({ operation: "test", command, cwd, - cause: PlatformError.systemError({ _tag: "NotFound", module, method }), + cause: PlatformError.systemError({ _tag: "NotFound", module: "ChildProcess", method: "spawn" }), }); const resolveUnknown = ( registry: SourceControlProviderRegistry.SourceControlProviderRegistry["Service"], repository: string, - host = "git.example.internal", ) => registry.resolveHandle({ cwd: `/${repository}`, context: { provider: { kind: SourceControlProviderKind.make("unknown"), - name: host.toLowerCase(), - baseUrl: `https://${host.toLowerCase()}`, + name: "git.example.internal", + baseUrl: "https://git.example.internal", }, remoteName: "origin", - remoteUrl: `git@${host}:team/${repository}.git`, + remoteUrl: `git@Git.Example.Internal:team/${repository}.git`, }, }); @@ -513,11 +510,17 @@ it.effect("asks once for every checkout of a server no integration claims", () = const registry = yield* makeRegistry({ remotes: [], process: { - run: ({ command, cwd }) => { - asked.push(command); + run: ({ command, args, cwd }) => { + asked.push([command, ...args].join(" ")); return Deferred.await(release).pipe(Effect.andThen(Effect.fail(notFound(command, cwd)))); }, }, + githubApi: { + credential: (host) => { + asked.push(`credential ${host}`); + return Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })); + }, + }, forgejo: { listLogins: ({ command = "tea", cwd }) => { asked.push(command); @@ -527,7 +530,7 @@ it.effect("asks once for every checkout of a server no integration claims", () = }, }); - // Each caller runs until it waits, so once all have started the first probe is still open. + // Three checkouts read while the first probe is still running share it. const started = yield* Effect.forEach(["a", "b", "c"], () => Deferred.make()); const callers = yield* Effect.forEach( ["a", "b", "c"], @@ -538,215 +541,71 @@ it.effect("asks once for every checkout of a server no integration claims", () = { concurrency: "unbounded" }, ).pipe(Effect.forkChild); yield* Effect.forEach(started, Deferred.await); - assert.deepStrictEqual(asked, ["glab"]); yield* Deferred.succeed(release, undefined); const handles = yield* Fiber.join(callers); yield* resolveUnknown(registry, "d"); assert.deepStrictEqual( - handles.map((handle) => [handle.provider.kind, handle.context?.remoteUrl]), - ["a", "b", "c"].map((name) => ["unknown", `git@git.example.internal:team/${name}.git`]), + handles.map((handle) => handle.provider.kind), + ["unknown", "unknown", "unknown"], ); - assert.deepStrictEqual(asked, ["glab", "fj", "tea"]); + const once = asked.slice(); + // glab is asked about the remote's host as the remote spells it. + assert.sameMembers(once, [ + "credential git.example.internal", + "glab auth status --hostname Git.Example.Internal", + "fj", + "tea", + ]); // A login made in a terminal sends no signal, so the server is asked again later. yield* TestClock.adjust("5 minutes"); yield* resolveUnknown(registry, "a"); - assert.strictEqual(asked.length, 6); + assert.deepStrictEqual(asked, [...once, ...once]); }), ); -it.effect.each(["tea", "gh"] as const)("asks again after %s could not run", (failing) => +it.effect("asks again after a check that could not answer", () => Effect.gen(function* () { - let attempts = 0; - const fail = (error: E) => { - attempts += 1; - return Effect.fail(error); - }; - const registry = yield* makeRegistry({ - remotes: [], - process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, - githubApi: { - credential: (host) => - failing === "gh" - ? fail(new GitHubCredentials.GitHubCliFailedError({ host, cause: "timeout" })) - : Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })), - }, - forgejo: { - listLogins: ({ command, cwd }) => - failing === "tea" && command !== "fj" - ? fail(new ForgejoCli.ForgejoCliError({ command: "tea", cwd, detail: "Failed." })) - : Effect.succeed([]), - }, - }); - for (const repository of ["a", "b"]) { - const handle = yield* resolveUnknown(registry, repository); - assert.strictEqual(handle.provider.kind, "unknown"); - } - assert.strictEqual(attempts, 2); - }), -); - -const glabReply = (stderr: string, exitCode = 1) => - Effect.succeed(processOutput("", { stderr, exitCode: ChildProcessSpawner.ExitCode(exitCode) })); -const glabLogin = (host = "git.example.internal") => - glabReply(`${host}\n ✓ Logged in to ${host} as maria (keyring)\n`, 0); - -it.effect.each([ - { name: "a login", replies: [glabLogin()], found: ["gitlab", "gitlab"], asks: 1 }, - { - name: "a login under the remote's spelling", - host: "Git.Example.Internal", - replies: [glabLogin("Git.Example.Internal")], - found: ["gitlab", "gitlab"], - asks: 1, - }, - ...[ - ["no login", "x git.example.internal has not been authenticated with glab"], - ["no login on glab 1.22", "x git.example.internal not authenticated with glab"], - ["no instances", "no GitLab instances have been authenticated with glab"], - ["no instance on glab 1.22", "No GitLab instance has been authenticated with glab"], - ].map(([name, message]) => ({ - name, - host: undefined, - replies: [glabReply(message!)], - found: ["unknown", "unknown"], - asks: 1, - })), - { - name: "a failed check", - replies: [glabReply("git.example.internal\n x git.example.internal: API call failed\n")], - found: ["unknown", "unknown"], - asks: 2, - }, - { - name: "a timeout", - replies: [ - Effect.fail( - new VcsProcessTimeoutError({ operation: "test", command: "glab", cwd: "/a", timeoutMs: 1 }), - ), - ], - found: ["unknown", "unknown"], - asks: 2, - }, - { - name: "a check failed in a missing checkout", - replies: [Effect.fail(notFound("glab", "/a", "FileSystem", "access")), glabLogin()], - found: ["unknown", "gitlab"], - asks: 2, - }, -])("asks glab about the remote's own host and keeps $name", (scenario) => - Effect.gen(function* () { - const host = scenario.host ?? "git.example.internal"; - const asked: Array> = []; + let asks = 0; const registry = yield* makeRegistry({ remotes: [], process: { - // glab holds a login only under the host's exact spelling. - run: ({ args, cwd }) => { - asked.push([cwd, ...args]); - return args[3] === host - ? (scenario.replies[asked.length - 1] ?? scenario.replies[0]!) - : glabReply("x git.example.internal has not been authenticated with glab"); - }, - }, - }); - const found = yield* Effect.forEach(["a", "b"], (repository) => - resolveUnknown(registry, repository, host).pipe(Effect.map(({ provider }) => provider.kind)), - ); - assert.deepStrictEqual(found, scenario.found); - assert.deepStrictEqual( - asked, - ["/a", "/b"] - .slice(0, scenario.asks) - .map((cwd) => [cwd, "auth", "status", "--hostname", host]), - ); - }), -); - -it.effect("does not keep a tea login found while fj could not be read", () => - Effect.gen(function* () { - let fjReadable = false; - const loginAt = (port: number) => ({ - name: "login", - url: `https://git.example.internal:${port}`, - ssh_host: "git.example.internal", - user: "maria", - default: "true", - }); - const registry = yield* makeRegistry({ - remotes: [], - process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, - forgejo: { - listLogins: ({ command, cwd }) => - command === "fj" && !fjReadable - ? Effect.fail(new ForgejoCli.ForgejoCliError({ command, cwd, detail: "Unreadable." })) - : Effect.succeed([loginAt(command === "fj" ? 4000 : 3000)]), - }, - }); - const fallback = yield* resolveUnknown(registry, "a"); - assert.strictEqual(fallback.context?.provider.baseUrl, "https://git.example.internal:3000"); - fjReadable = true; - const recovered = yield* resolveUnknown(registry, "b"); - assert.strictEqual(recovered.context?.provider.baseUrl, "https://git.example.internal:4000"); - }), -); - -it.effect.each([ - { first: "https://host-a.example/repo.git", second: "https://host-b.example/repo.git" }, - { first: "ssh://git@host-a.example/repo.git", second: "ssh://git@host-b.example/repo.git" }, - { first: "https://host.example:8443/repo.git", second: "https://host.example:9443/repo.git" }, -])("keeps the lookups for $first and $second apart", ({ first, second }) => - Effect.gen(function* () { - const asked: Array = []; - const registry = yield* makeRegistry({ - remotes: [], - process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, - forgejo: { - listLogins: ({ command, remoteUrl }) => { - if (command === "tea") asked.push(remoteUrl); - return Effect.succeed([]); + run: ({ command, cwd }) => { + asks += 1; + return Effect.fail( + new VcsProcessTimeoutError({ operation: "test", command, cwd, timeoutMs: 1 }), + ); }, }, }); - for (const remoteUrl of [first, second]) { - // The identity resolver's context, where the remote alone names the server. - const provider = { - kind: SourceControlProviderKind.make("unknown"), - name: "Unknown", - baseUrl: "", - }; - yield* registry.resolveHandle({ - cwd: "/repo", - context: { provider, remoteName: "origin", remoteUrl }, - }); - } - assert.deepStrictEqual(asked, [first, second]); + yield* resolveUnknown(registry, "a"); + yield* resolveUnknown(registry, "b"); + assert.strictEqual(asks, 2); }), ); it.effect("asks again once Settings change", () => Effect.gen(function* () { const changes = yield* PubSub.unbounded(); - const handled = yield* Effect.forEach([0, 1], () => Deferred.make()); - let changeCount = 0; - const markHandled = Effect.suspend(() => Deferred.succeed(handled[changeCount++]!, undefined)); + const handled = yield* Deferred.make(); let signedIn = false; const registry = yield* makeRegistry({ remotes: [], settings: { subscribeChanges: PubSub.subscribe(changes).pipe( Effect.map((subscription) => - // A change is marked handled once the registry asks for the next one. + // A change is handled once the registry asks for the next one. Stream.fromSubscription(subscription).pipe( Stream.flatMap((settings) => - Stream.concat(Stream.make(settings), Stream.fromEffectDrain(markHandled)), + Stream.concat( + Stream.make(settings), + Stream.fromEffectDrain(Deferred.succeed(handled, undefined)), + ), ), ), ), ), }, - // Saved as the registry finishes starting, before its consumer has run. - started: PubSub.publish(changes, DEFAULT_SERVER_SETTINGS), process: { run: ({ command, cwd }) => Effect.fail(notFound(command, cwd)) }, githubApi: { credential: (host) => @@ -755,14 +614,13 @@ it.effect("asks again once Settings change", () => : Effect.fail(new GitHubCredentials.GitHubNotSignedInError({ host })), }, }); - yield* Deferred.await(handled[0]!); assert.strictEqual((yield* resolveUnknown(registry, "a")).provider.kind, "unknown"); signedIn = true; assert.strictEqual((yield* resolveUnknown(registry, "b")).provider.kind, "unknown"); // A token saved for the host in Settings. yield* PubSub.publish(changes, DEFAULT_SERVER_SETTINGS); - yield* Deferred.await(handled[1]!); + yield* Deferred.await(handled); assert.strictEqual((yield* resolveUnknown(registry, "c")).provider.kind, "github"); }), ); From f10e13188036f23c5e424df61ba961b76f4ff680 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:52:00 -0700 Subject: [PATCH 3/5] refactor(server): simplify unknown-host refinement cache Every read of a project whose remote points at an unrecognised host asked glab, fj, tea and the GitHub credential layer again, once per project per minute in the pull request sweep. The registry now keeps the answer per server for five minutes, shares one in-flight probe between checkouts, and asks again on a Settings change. The refinement contract stays as it was for adapters: a refiner returns the provider it claims or null. The core treats any failure as "could not tell" and does not keep that verdict, so a missing CLI (the common case) is the one failure adapters translate to null, through a shared isMissingExecutable check. glab is asked about the remote's own host, spelled as the remote spells it, and its "not logged in" messages count as an answer. Co-Authored-By: Claude Opus 5.5 --- .../SourceControlProviderRegistry.ts | 13 ++- .../src/server/discovery.ts | 102 ++++++++++-------- .../server/ForgejoSourceControlProvider.ts | 21 ++-- .../server/GitCafeSourceControlProvider.ts | 2 +- .../src/server/GitHubSourceControlProvider.ts | 12 +-- 5 files changed, 78 insertions(+), 72 deletions(-) diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts index 95d3b6093d31..fd306a986d10 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.ts @@ -159,8 +159,10 @@ function selectProviderContext( } /** - * Checkouts of one server share a lookup. The key omits `cwd`, which matters only when a check fails - * there, and such a verdict is not kept. Equality ignores the lookup, so this is not a `Data.Class`. + * One lookup per server: remotes that differ only in `owner/repo` share a verdict, and callers + * that arrive while it runs wait on it. `cwd` is left out because it only matters when a probe + * fails there, and a failed probe is not kept. Equality ignores `lookup`, so this is not a + * `Data.Class`. */ class UnknownRemoteLookup { readonly key: string; @@ -174,6 +176,7 @@ class UnknownRemoteLookup { this.key = [ context.provider.name, context.provider.baseUrl, + // Strip a trailing `owner/repo`, never the host after `//`, so one-segment remotes stay apart. context.remoteUrl.trim().replace(/(? Effect.succeed(providers.get(kind) ?? unsupportedProvider(kind)); - // A verdict a check could not give is shared with callers already waiting on it, then dropped. + // "No integration holds this host" is the common verdict and the one worth keeping. A verdict + // reached while some probe failed is handed to the callers waiting on it, then dropped. const unknownRemoteCache = yield* Cache.makeWith( (request: UnknownRemoteLookup) => request.lookup, { @@ -256,8 +260,7 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit Exit.isSuccess(exit) && exit.value.settled ? UNKNOWN_REMOTE_CACHE_TTL : Duration.zero, }, ); - // A credential saved or a host turned on or off in Settings clears the kept verdicts. The - // subscription is taken here so a change made before the consumer starts is not lost. + // A token saved or a host toggled in Settings must show on the next read, not after the TTL. const settingsChanges = yield* serverSettings.subscribeChanges; yield* Stream.runForEach(settingsChanges, () => Cache.invalidateAll(unknownRemoteCache)).pipe( Effect.forkScoped, diff --git a/packages/source-control-core/src/server/discovery.ts b/packages/source-control-core/src/server/discovery.ts index 8c69375aee57..512e9bdcc790 100644 --- a/packages/source-control-core/src/server/discovery.ts +++ b/packages/source-control-core/src/server/discovery.ts @@ -11,9 +11,12 @@ import { type SourceControlProviderInfo, type SourceControlProviderKind, } from "@t3tools/contracts"; +import type * as Cause from "effect/Cause"; +import * as Data from "effect/Data"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; +import * as Result from "effect/Result"; import * as Schema from "effect/Schema"; import type * as SourceControlProvider from "./SourceControlProvider.ts"; @@ -47,7 +50,7 @@ export type SourceControlCliDiscoverySpec = SourceControlDiscoverySpecBase & { ) => ReadonlyArray; readonly probeTimeoutMs?: number; readonly parseAuth: (input: SourceControlAuthProbeInput) => SourceControlProviderAuth; - /** `null` when nothing claims the remote; `undefined` when the output cannot say. */ + /** `null` when the CLI holds nothing for the remote; `undefined` when its output cannot say. */ readonly refineUnknownRemote?: ( input: SourceControlUnknownRemoteRefinementInput, ) => SourceControlProviderInfo | null | undefined; @@ -61,10 +64,14 @@ export type SourceControlApiDiscoverySpec = SourceControlDiscoverySpecBase & { export type SourceControlManagedCliDiscoverySpec = SourceControlDiscoverySpecBase & { readonly type: "managed-cli"; readonly probe: (cwd: string) => Effect.Effect; + /** + * `null` when the integration holds nothing for the remote, which includes its CLI not being + * installed. A failure means it could not tell, and the registry does not keep that answer. + */ readonly refineUnknownRemote: (input: { readonly cwd: string; readonly context: SourceControlProvider.SourceControlProviderContext; - }) => Effect.Effect; + }) => Effect.Effect; }; export type SourceControlProviderDiscoverySpec = @@ -303,8 +310,8 @@ export function probeSourceControlProvider(input: { const isVcsProcessSpawnError = Schema.is(VcsProcessSpawnError); /** - * A hosting CLI that is not installed claims no remote. The spawner checks the cwd first, so only - * the executable's own ENOENT comes from `ChildProcess`; a missing checkout fails elsewhere. + * A hosting CLI that is not installed claims no remote. The spawner checks the cwd before the + * executable, so only the executable's own ENOENT comes from `ChildProcess`. */ export function isMissingExecutable(error: unknown): boolean { return ( @@ -315,12 +322,50 @@ export function isMissingExecutable(error: unknown): boolean { ); } +/** The CLI ran, but its output does not say whether it holds the remote's host. */ +class UnknownRemoteUnanswered extends Data.TaggedError("UnknownRemoteUnanswered") {} + export interface UnknownRemoteVerdict { readonly provider: SourceControlProviderInfo | null; - /** Every check ahead of the answer ran, so another checkout of the server would hear the same. */ + /** Every integration answered, so the verdict holds for other checkouts of the same server. */ readonly settled: boolean; } +function refineWithSpec(input: { + readonly spec: SourceControlProviderDiscoverySpec; + readonly process: SourceControlHost.SourceControlHost["Service"]["process"]; + readonly cwd: string; + readonly context: SourceControlProvider.SourceControlProviderContext; +}): Effect.Effect { + const { spec, context } = input; + if (spec.type === "managed-cli") { + return spec.refineUnknownRemote({ cwd: input.cwd, context }); + } + if (!isCliRemoteRefinementSpec(spec)) return Effect.succeed(null); + return input.process + .run({ + operation: "source-control.discovery.refine-unknown-remote", + command: spec.executable, + args: spec.remoteRefinementArgs?.(context) ?? spec.authArgs, + cwd: input.cwd, + allowNonZeroExit: true, + timeoutMs: probeTimeoutMs(spec), + maxOutputBytes: 8_000, + appendTruncationMarker: true, + }) + .pipe( + Effect.catchIf(isMissingExecutable, () => Effect.void), + Effect.flatMap((auth) => { + const provider = + auth === undefined ? null : spec.refineUnknownRemote({ cwd: input.cwd, context, auth }); + return provider === undefined + ? Effect.fail(new UnknownRemoteUnanswered()) + : Effect.succeed(provider); + }), + ); +} + +/** Asks every integration whether it holds the remote's server; the first claim wins. */ export const refineUnknownRemoteProvider = Effect.fn("refineUnknownRemoteProvider")( function* (input: { readonly specs: ReadonlyArray; @@ -328,46 +373,15 @@ export const refineUnknownRemoteProvider = Effect.fn("refineUnknownRemoteProvide readonly cwd: string; readonly context: SourceControlProvider.SourceControlProviderContext; }): Effect.fn.Return { - const context = input.context; - - const verdicts = yield* Effect.forEach(input.specs, (spec) => { - if (spec.type === "managed-cli") { - return spec.refineUnknownRemote({ cwd: input.cwd, context }); - } - if (!isCliRemoteRefinementSpec(spec)) - return Effect.succeed({ provider: null, settled: true }); - return input.process - .run({ - operation: "source-control.discovery.refine-unknown-remote", - command: spec.executable, - args: spec.remoteRefinementArgs?.(context) ?? spec.authArgs, - cwd: input.cwd, - allowNonZeroExit: true, - timeoutMs: probeTimeoutMs(spec), - maxOutputBytes: 8_000, - appendTruncationMarker: true, - }) - .pipe( - Effect.map((auth) => - spec.refineUnknownRemote({ - cwd: input.cwd, - context, - auth, - }), - ), - Effect.catchIf(isMissingExecutable, () => Effect.succeed(null)), - Effect.orElseSucceed(() => undefined), - Effect.map((provider) => ({ - provider: provider ?? null, - settled: provider !== undefined, - })), - ); - }); - + const results = yield* Effect.forEach(input.specs, (spec) => + refineWithSpec({ ...input, spec }).pipe(Effect.result), + ); + // A claim reached after another integration could not answer is not settled: that one might + // have claimed the host first. let settled = true; - for (const verdict of verdicts) { - settled &&= verdict.settled; - if (verdict.provider) return { provider: verdict.provider, settled }; + for (const result of results) { + if (Result.isFailure(result)) settled = false; + else if (result.success !== null) return { provider: result.success, settled }; } return { provider: null, settled }; }, diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts index 8ebe91bcf08b..61a571b873b1 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts @@ -150,37 +150,28 @@ export const makeDiscovery = Effect.gen(function* () { readonly context: SourceControlProvider.SourceControlProviderContext; }) { const remote = ForgejoCli.parseForgejoRemote(input.context.remoteUrl); - if (!remote) return { provider: null, settled: true }; - let settled = true; + if (!remote) return null; for (const command of ["fj", "tea"] as const) { const logins = yield* listLogins({ cwd: input.cwd, command, remoteUrl: input.context.remoteUrl, }).pipe( - // A CLI that is not installed holds no login; any other failure leaves it unasked. + // A CLI that is not installed holds no login; any other failure propagates. Effect.catchIf( (error) => isMissingExecutable(error.cause), () => Effect.succeed([]), ), - Effect.orElseSucceed(() => undefined), ); - if (logins === undefined) { - settled = false; - continue; - } const login = ForgejoCli.matchForgejoLogin(logins, remote, input.context.requestedHost); if (login) return { - provider: { - kind: SourceControlProviderKind.make("forgejo"), - name: discovery.label, - baseUrl: login.url, - }, - settled, + kind: SourceControlProviderKind.make("forgejo"), + name: discovery.label, + baseUrl: login.url, }; } - return { provider: null, settled }; + return null; }, ), } satisfies SourceControlManagedCliDiscoverySpec; diff --git a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts index 8258e4aaff1b..db4bea8dbf4c 100644 --- a/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts +++ b/packages/source-control-gitcafe/src/server/GitCafeSourceControlProvider.ts @@ -118,7 +118,7 @@ export const makeDiscovery = Effect.gen(function* () { }), ), ), - refineUnknownRemote: () => Effect.succeed({ provider: null, settled: true }), + refineUnknownRemote: () => Effect.succeed(null), } satisfies SourceControlManagedCliDiscoverySpec; }); diff --git a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts index 803f1af714e3..04fe68874412 100644 --- a/packages/source-control-github/src/server/GitHubSourceControlProvider.ts +++ b/packages/source-control-github/src/server/GitHubSourceControlProvider.ts @@ -268,7 +268,8 @@ export const makeDiscovery = Effect.gen(function* () { refineUnknownRemote: ({ context }) => { // Identity resolution also probes providers before a web base URL is known. const url = URL.parse(context.provider.baseUrl); - if (!url?.host) return Effect.succeed({ provider: null, settled: true }); + if (!url?.host) return Effect.succeed(null); + // A timeout or keyring failure propagates: the registry does not keep that answer. return api.credential(url.host).pipe( Effect.as(true), Effect.catchTags({ @@ -276,18 +277,15 @@ export const makeDiscovery = Effect.gen(function* () { GitHubHostDisabledError: () => Effect.succeed(true), GitHubNotSignedInError: () => Effect.succeed(false), }), - Effect.map((known) => ({ - provider: known + Effect.map((known) => + known ? ({ kind: SourceControlProviderKind.make("github"), name: "GitHub Self-Hosted", baseUrl: context.provider.baseUrl, } as const) : null, - settled: true, - })), - // gh timed out or failed, so it is unknown whether it holds a login for the host. - Effect.orElseSucceed(() => ({ provider: null, settled: false })), + ), ); }, } satisfies SourceControlManagedCliDiscoverySpec; From bd71ff01029c8517466ca71828ce5ed569337ea2 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Thu, 15 Oct 2026 10:55:41 -0700 Subject: [PATCH 4/5] fix(forgejo): ask tea when fj cannot read its logins Co-Authored-By: Claude Opus 5.5 --- .../src/server/ForgejoSourceControlProvider.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts index 61a571b873b1..2c69f5776ebb 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts @@ -151,19 +151,30 @@ export const makeDiscovery = Effect.gen(function* () { }) { const remote = ForgejoCli.parseForgejoRemote(input.context.remoteUrl); if (!remote) return null; + // Both CLIs claim Forgejo, so one failing does not stop the other from claiming the host. + let failure: ForgejoCli.ForgejoCliError | undefined; for (const command of ["fj", "tea"] as const) { const logins = yield* listLogins({ cwd: input.cwd, command, remoteUrl: input.context.remoteUrl, }).pipe( - // A CLI that is not installed holds no login; any other failure propagates. + // A CLI that is not installed holds no login. Effect.catchIf( (error) => isMissingExecutable(error.cause), () => Effect.succeed([]), ), + Effect.result, + ); + if (Result.isFailure(logins)) { + failure ??= logins.failure; + continue; + } + const login = ForgejoCli.matchForgejoLogin( + logins.success, + remote, + input.context.requestedHost, ); - const login = ForgejoCli.matchForgejoLogin(logins, remote, input.context.requestedHost); if (login) return { kind: SourceControlProviderKind.make("forgejo"), @@ -171,6 +182,7 @@ export const makeDiscovery = Effect.gen(function* () { baseUrl: login.url, }; } + if (failure) return yield* failure; return null; }, ), From b4dfa8ef6d76943ffd6ba2711f0a8245b28f523b Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Thu, 15 Oct 2026 11:31:51 -0700 Subject: [PATCH 5/5] test(gitlab): pin the glab output that marks a host as not logged in Also declare the core's unanswered-refinement error with Schema. Co-Authored-By: Claude Opus 5.5 --- .../src/server/discovery.ts | 6 ++-- .../GitLabSourceControlProvider.test.ts | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/packages/source-control-core/src/server/discovery.ts b/packages/source-control-core/src/server/discovery.ts index 512e9bdcc790..6a50de973e3d 100644 --- a/packages/source-control-core/src/server/discovery.ts +++ b/packages/source-control-core/src/server/discovery.ts @@ -12,7 +12,6 @@ import { type SourceControlProviderKind, } from "@t3tools/contracts"; import type * as Cause from "effect/Cause"; -import * as Data from "effect/Data"; import * as Effect from "effect/Effect"; import * as Option from "effect/Option"; import * as PlatformError from "effect/PlatformError"; @@ -323,7 +322,10 @@ export function isMissingExecutable(error: unknown): boolean { } /** The CLI ran, but its output does not say whether it holds the remote's host. */ -class UnknownRemoteUnanswered extends Data.TaggedError("UnknownRemoteUnanswered") {} +class UnknownRemoteUnanswered extends Schema.TaggedError()( + "UnknownRemoteUnanswered", + {}, +) {} export interface UnknownRemoteVerdict { readonly provider: SourceControlProviderInfo | null; diff --git a/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.test.ts b/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.test.ts index ccfc49aff208..c12e4e6100d4 100644 --- a/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.test.ts +++ b/packages/source-control-gitlab/src/server/GitLabSourceControlProvider.test.ts @@ -214,6 +214,34 @@ it("refines unknown GitLab remotes with mixed-case provider hosts", () => { }); }); +it("tells a host glab has no login for apart from output that cannot say", () => { + const refine = (stderr: string) => + GitLabSourceControlProvider.discovery.refineUnknownRemote?.({ + cwd: "/repo", + context: { + provider: { + kind: SourceControlProviderKind.make("unknown"), + name: "git.example.test", + baseUrl: "https://git.example.test", + }, + remoteName: "origin", + remoteUrl: "https://git.example.test/group/project.git", + }, + auth: { exitCode: ChildProcessSpawner.ExitCode(1), stdout: "", stderr }, + }); + + // A settled "no" is cached; anything else makes the next read ask glab again. + assert.strictEqual( + refine("x git.example.test has not been authenticated with glab. Run `glab auth login`"), + null, + ); + assert.strictEqual( + refine("No GitLab instances have been authenticated with glab. Run `glab auth login`"), + null, + ); + assert.strictEqual(refine("git.example.test\n x API call failed: 503"), undefined); +}); + it("parses authenticated GitLab auth status hosts with ports and single-label names", () => { assert.deepStrictEqual( parseGitLabAuthStatusHosts(`localhost:8080