fix(server): stop re-probing hosting CLIs for unrecognized hosts

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,20 @@ 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;
+// A login made in a terminal (`gh auth login`, `tea login add`) sends the server no signal, so
+// each host is asked again after this long.
+const UNKNOWN_REMOTE_CACHE_TTL = Duration.minutes(5);
 
 export interface SourceControlProviderRegistration {
   readonly kind: SourceControlProviderKind;
@@ -150,6 +159,53 @@ function selectProviderContext(
   );
 }
 
+// What the refiners read, minus `owner/repo`: the provider name and base URL (GitHub, GitLab),
+// the remote's scheme, host, port and mount (Forgejo), and a requested host. The mount is the
+// parsed URL path minus its last two segments, the rule the Forgejo refiner uses for its mount,
+// so the URL is parsed as given and not edited first. Scheme and host ignore case; the mount does
+// not, as Forgejo login matching compares it as is. A Forgejo login below the mount serves some
+// remotes of the key and not others, so the Forgejo refiner leaves that verdict unsettled.
+function unknownRemoteKey(context: SourceControlProvider.SourceControlProviderContext): string {
+  const remote = context.remoteUrl;
+  const url = /^[a-z][a-z0-9+.-]*:\/\//iu.test(remote) ? URL.parse(remote) : null;
+  const mount = url?.pathname
+    .replace(/^\/+|\/+$/gu, "")
+    .split("/")
+    .slice(0, -2)
+    .join("/");
+  // SCP form: the host alone, read as the Forgejo refiner reads it. A remote it cannot read, such
+  // as a path from the root, gets no Forgejo claim, so it keys by itself.
+  const server = url
+    ? `${url.protocol}//${url.host.toLowerCase()}/${mount}`
+    : (/^(?:[^@/]+@)?([^:/]+):[^/].*$/u.exec(remote)?.[1]?.toLowerCase() ?? remote);
+  return [
+    context.provider.name,
+    context.provider.baseUrl,
+    server,
+    context.requestedHost?.toLowerCase() ?? "",
+  ].join("\u0000");
+}
+
+/** Checkouts of one server share a lookup; the first to ask supplies the cwd and context. */
+class UnknownRemoteRequest {
+  readonly key: string;
+  readonly cwd: string;
+  readonly context: SourceControlProvider.SourceControlProviderContext;
+
+  constructor(cwd: string, context: SourceControlProvider.SourceControlProviderContext) {
+    this.key = unknownRemoteKey(context);
+    this.cwd = cwd;
+    this.context = context;
+  }
+
+  [Equal.symbol](that: unknown): boolean {
+    return that instanceof UnknownRemoteRequest && that.key === this.key;
+  }
+  [Hash.symbol](): number {
+    return Hash.string(this.key);
+  }
+}
+
 function bindProviderContext(
   provider: SourceControlProvider.SourceControlProvider["Service"],
   context: SourceControlProvider.SourceControlProviderContext | null,
@@ -198,6 +254,7 @@ function bindProviderContext(
 export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWithProviders")(
   function* (registrations: ReadonlyArray<SourceControlProviderRegistration>) {
     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 +266,41 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit
     const get: SourceControlProviderRegistry["Service"]["get"] = (kind) =>
       Effect.succeed(providers.get(kind) ?? unsupportedProvider(kind));
 
+    // Only a settled verdict is kept. One a checkout could not reach is shared with the lookups
+    // already waiting on it, then dropped.
+    const unknownRemoteCache = yield* Cache.makeWith<UnknownRemoteRequest, UnknownRemoteVerdict>(
+      (request) =>
+        refineUnknownRemoteProvider({
+          specs: discoverySpecs,
+          process,
+          cwd: request.cwd,
+          context: request.context,
+        }),
+      {
+        capacity: UNKNOWN_REMOTE_CACHE_CAPACITY,
+        timeToLive: (exit) =>
+          Exit.isSuccess(exit) && exit.value.settled ? UNKNOWN_REMOTE_CACHE_TTL : Duration.zero,
+      },
+    );
+    // A token, account or host switch saved in Settings changes what GitHub claims.
+    const settingsChanges = yield* serverSettings.subscribeChanges;
+    yield* Stream.runForEach(settingsChanges, () => Cache.invalidateAll(unknownRemoteCache)).pipe(
+      Effect.forkScoped,
+    );
+
+    // The verdict for the remote's server, applied to this caller's own context.
+    const refineRemote = (
+      cwd: string,
+      context: SourceControlProvider.SourceControlProviderContext | null,
+    ): Effect.Effect<SourceControlProvider.SourceControlProviderContext | null> =>
+      context === null || context.provider.kind !== "unknown"
+        ? Effect.succeed(context)
+        : Cache.get(unknownRemoteCache, new UnknownRemoteRequest(cwd, context)).pipe(
+            Effect.map((verdict) =>
+              verdict.provider ? { ...context, provider: verdict.provider } : context,
+            ),
+          );
+
     const detectProviderContext = Effect.fn("SourceControlProviderRegistry.detectProviderContext")(
       function* (cwd: string) {
         const handle = yield* vcsRegistry.resolve({ cwd }).pipe(
@@ -237,12 +329,7 @@ export const makeWithProviders = Effect.fn("makeSourceControlProviderRegistryWit
         );
         const context = selectProviderContext(remotes.remotes);
 
-        return yield* refineUnknownRemoteProvider({
-          specs: discoverySpecs,
-          process,
-          cwd,
-          context,
-        });
+        return yield* refineRemote(cwd, context);
       },
     );
 
@@ -258,12 +345,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,
-          })
+        : refineRemote(input.cwd, input.context)
       ).pipe(
         Effect.map((context) => {
           const kind = context?.provider.kind ?? SourceControlProviderKind.make("unknown");

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";
@@ -42,9 +45,10 @@ export type SourceControlCliDiscoverySpec = SourceControlDiscoverySpecBase & {
   readonly remoteRefinementArgs?: ReadonlyArray<string>;
   readonly probeTimeoutMs?: number;
   readonly parseAuth: (input: SourceControlAuthProbeInput) => SourceControlProviderAuth;
+  /** Unsettled when the CLI's output does not answer for this remote, as opposed to answering no. */
   readonly refineUnknownRemote?: (
     input: SourceControlUnknownRemoteRefinementInput,
-  ) => SourceControlProviderInfo | null;
+  ) => UnknownRemoteVerdict;
 };
 
 export type SourceControlApiDiscoverySpec = SourceControlDiscoverySpecBase & {
@@ -55,10 +59,11 @@ export type SourceControlApiDiscoverySpec = SourceControlDiscoverySpecBase & {
 export type SourceControlManagedCliDiscoverySpec = SourceControlDiscoverySpecBase & {
   readonly type: "managed-cli";
   readonly probe: (cwd: string) => Effect.Effect<SourceControlProviderDiscoveryItem>;
+  /** Unsettled when a command could not answer for this checkout, as opposed to answering no. */
   readonly refineUnknownRemote: (input: {
     readonly cwd: string;
     readonly context: SourceControlProvider.SourceControlProviderContext;
-  }) => Effect.Effect<SourceControlProviderInfo | null>;
+  }) => Effect.Effect<UnknownRemoteVerdict>;
 };
 
 export type SourceControlProviderDiscoverySpec =
@@ -294,23 +299,47 @@ export function probeSourceControlProvider(input: {
   );
 }
 
+const isVcsProcessSpawnError = Schema.is(VcsProcessSpawnError);
+
+/**
+ * A hosting CLI that is not installed claims no remote for any checkout. The spawner checks the
+ * cwd with `FileSystem.access` first, so a missing checkout fails there instead; only the
+ * executable's own ENOENT comes from `ChildProcess`. Any other failure says nothing of the host.
+ */
+export function isMissingExecutable(error: unknown): boolean {
+  return (
+    isVcsProcessSpawnError(error) &&
+    error.cause instanceof PlatformError.PlatformError &&
+    error.cause.reason._tag === "NotFound" &&
+    error.cause.reason.module === "ChildProcess"
+  );
+}
+
+export interface UnknownRemoteVerdict {
+  /** The first integration that claims the remote, or null when none does. */
+  readonly provider: SourceControlProviderInfo | null;
+  /**
+   * Every integration up to that answer could ask, so another checkout of the remote would hear
+   * the same. False when one could not ask from this checkout (it is missing, a CLI timed out).
+   */
+  readonly settled: boolean;
+}
+
 export const refineUnknownRemoteProvider = Effect.fn("refineUnknownRemoteProvider")(
   function* (input: {
     readonly specs: ReadonlyArray<SourceControlProviderDiscoverySpec>;
     readonly process: SourceControlHost.SourceControlHost["Service"]["process"];
     readonly cwd: string;
-    readonly context: SourceControlProvider.SourceControlProviderContext | null;
-  }): Effect.fn.Return<SourceControlProvider.SourceControlProviderContext | null> {
-    if (input.context === null || input.context.provider.kind !== "unknown") {
-      return input.context;
-    }
+    readonly context: SourceControlProvider.SourceControlProviderContext;
+  }): Effect.fn.Return<UnknownRemoteVerdict> {
     const context = input.context;
 
-    const providers = yield* Effect.forEach(input.specs, (spec) => {
+    const ask = (spec: SourceControlProviderDiscoverySpec): Effect.Effect<UnknownRemoteVerdict> => {
       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",
@@ -323,18 +352,31 @@ export const refineUnknownRemoteProvider = Effect.fn("refineUnknownRemoteProvide
           appendTruncationMarker: true,
         })
         .pipe(
-          Effect.map((auth) =>
-            spec.refineUnknownRemote({
+          Effect.map((auth) => {
+            const verdict = spec.refineUnknownRemote({
               cwd: input.cwd,
               context,
               auth,
-            }),
+            });
+            // Output cut at the limit may have lost the part that answers for this remote.
+            const complete = !auth.stdoutTruncated && !auth.stderrTruncated;
+            return { provider: verdict.provider, settled: verdict.settled && complete };
+          }),
+          // A CLI that is not installed answers no for every checkout.
+          Effect.catchIf(isMissingExecutable, () =>
+            Effect.succeed({ provider: null, settled: true }),
           ),
-          Effect.orElseSucceed(() => null),
+          Effect.orElseSucceed(() => ({ provider: null, settled: false })),
         );
-    });
-    const provider = providers.find((candidate) => candidate !== null);
-
-    return provider ? { ...context, provider } : context;
+    };
+    // The first claim wins, so integrations after it are not asked. It is settled only if every
+    // integration before it answered, since one that could not ask might have claimed first.
+    let settled = true;
+    for (const spec of input.specs) {
+      const verdict = yield* ask(spec);
+      settled &&= verdict.settled;
+      if (verdict.provider !== null) return { provider: verdict.provider, settled };
+    }
+    return { provider: null, settled };
   },
 );