fix(server): a slow or broken hosting CLI is no longer told to install itself

apps/server/src/sourceControl/SourceControlDiscovery.test.ts

@@ -1,10 +1,20 @@
-import { SourceControlProviderKind, VcsProcessSpawnError } from "@t3tools/contracts";
+import {
+  SourceControlProviderKind,
+  type VcsError,
+  VcsProcessExitError,
+  VcsProcessSpawnError,
+  VcsProcessTimeoutError,
+} from "@t3tools/contracts";
 import { assert, it } from "@effect/vitest";
 import * as NodeServices from "@effect/platform-node/NodeServices";
 import * as Effect from "effect/Effect";
 import * as Layer from "effect/Layer";
 import * as Option from "effect/Option";
-import { providerAuth } from "@t3tools/source-control-core/server/discovery";
+import * as PlatformError from "effect/PlatformError";
+import {
+  providerAuth,
+  type SourceControlProviderDiscoverySpec,
+} from "@t3tools/source-control-core/server/discovery";
 import type * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost";
 import {
   fakeDiscovery,
@@ -27,7 +37,29 @@ const run = (
     SourceControlHost.SourceControlProcessInput,
     "operation" | "command" | "args" | "cwd"
   >,
-): Effect.Effect<SourceControlHost.SourceControlProcessOutput, VcsProcessSpawnError> => {
+): Effect.Effect<SourceControlHost.SourceControlProcessOutput, VcsError> => {
+  const context = { operation: input.operation, command: input.command, cwd: input.cwd };
+  if (input.command === "slow-cli") {
+    return Effect.fail(new VcsProcessTimeoutError({ ...context, timeoutMs: 5_000 }));
+  }
+  if (input.command === "broken-cli") {
+    return Effect.fail(
+      new VcsProcessExitError({ ...context, exitCode: 1, detail: "broken-cli: bad install" }),
+    );
+  }
+  if (input.command === "absent-cli") {
+    return Effect.fail(
+      new VcsProcessSpawnError({
+        ...context,
+        cause: PlatformError.systemError({
+          _tag: "NotFound",
+          module: "ChildProcess",
+          method: "spawn",
+          pathOrDescriptor: input.command,
+        }),
+      }),
+    );
+  }
   if (input.args[0] === "--version") {
     return input.command === "missing-cli"
       ? Effect.fail(
@@ -45,43 +77,49 @@ const run = (
   );
 };
 
-const cliDiscovery = (name: string) => ({
+const cliDiscovery = (
+  name: string,
+  executable = name === "beta" ? "missing-cli" : `${name}-cli`,
+) => ({
   type: "cli" as const,
   kind: kind(name),
   label: name,
   installHint: `Install ${name}.`,
-  executable: name === "beta" ? "missing-cli" : `${name}-cli`,
+  executable,
   versionArgs: ["--version"],
   authArgs: ["auth", "status"],
   parseAuth: ({ stdout }: { readonly stdout: string }) =>
     providerAuth({ status: "authenticated", account: /as (\S+)/.exec(stdout)?.[1] }),
 });
 
-const layerDiscovery = Layer.effect(
-  SourceControlProviderRegistry.SourceControlProviderRegistry,
-  SourceControlProviderRegistry.makeWithProviders(
-    [
-      cliDiscovery("alpha"),
-      cliDiscovery("beta"),
-      fakeDiscovery("gamma", {
-        probeAuth: Effect.succeed(providerAuth({ status: "authenticated", account: "api-user" })),
-      }),
-    ].map((discovery) => ({
-      kind: discovery.kind,
-      provider: fakeSourceControlProvider(discovery.kind),
-      discovery,
-    })),
-  ),
-).pipe(
-  Layer.provideMerge(TestSourceControlHost.layer({ process: { run } })),
-  Layer.provideMerge(ServerSettings.ServerSettingsService.layerTest()),
-  Layer.provideMerge(Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({})),
-  Layer.provideMerge(Layer.mock(VcsProcess.VcsProcess)({ run })),
-  Layer.provideMerge(
-    ServerConfig.layerTest(process.cwd(), { prefix: "t3-source-control-discovery-" }),
-  ),
-  Layer.provideMerge(NodeServices.layer),
-);
+const defaultDiscoveries = [
+  cliDiscovery("alpha"),
+  cliDiscovery("beta"),
+  fakeDiscovery("gamma", {
+    probeAuth: Effect.succeed(providerAuth({ status: "authenticated", account: "api-user" })),
+  }),
+];
+
+const layerDiscovery = (discoveries: ReadonlyArray<SourceControlProviderDiscoverySpec>) =>
+  Layer.effect(
+    SourceControlProviderRegistry.SourceControlProviderRegistry,
+    SourceControlProviderRegistry.makeWithProviders(
+      discoveries.map((discovery) => ({
+        kind: discovery.kind,
+        provider: fakeSourceControlProvider(discovery.kind),
+        discovery,
+      })),
+    ),
+  ).pipe(
+    Layer.provideMerge(TestSourceControlHost.layer({ process: { run } })),
+    Layer.provideMerge(ServerSettings.ServerSettingsService.layerTest()),
+    Layer.provideMerge(Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({})),
+    Layer.provideMerge(Layer.mock(VcsProcess.VcsProcess)({ run })),
+    Layer.provideMerge(
+      ServerConfig.layerTest(process.cwd(), { prefix: "t3-source-control-discovery-" }),
+    ),
+    Layer.provideMerge(NodeServices.layer),
+  );
 
 it.effect("reports every hosting integration in registry order with its sign-in", () =>
   Effect.gen(function* () {
@@ -127,5 +165,44 @@ it.effect("reports every hosting integration in registry order with its sign-in"
         ["jj", "available"],
       ],
     );
-  }).pipe(Effect.provide(SourceControlDiscovery.layer.pipe(Layer.provide(layerDiscovery)))),
+  }).pipe(
+    Effect.provide(
+      SourceControlDiscovery.layer.pipe(Layer.provide(layerDiscovery(defaultDiscoveries))),
+    ),
+  ),
+);
+
+it.effect("explains a hosting CLI whose version probe failed instead of advising an install", () =>
+  Effect.gen(function* () {
+    const discovery = yield* SourceControlDiscovery.SourceControlDiscovery;
+    const result = yield* discovery.discover;
+    const item = (name: string) => {
+      const found = result.sourceControlProviders.find((entry) => entry.kind === kind(name));
+      assert.isDefined(found);
+      return found;
+    };
+    const authDetail = (name: string) => Option.getOrElse(item(name).auth.detail, () => "");
+
+    assert.notInclude(item("slow").installHint, "Install slow.");
+    assert.include(item("slow").installHint, "timed out after 5000ms");
+    assert.strictEqual(authDetail("slow"), item("slow").installHint);
+
+    assert.notInclude(item("broken").installHint, "Install broken.");
+    assert.include(item("broken").installHint, "exited with status 1");
+    assert.strictEqual(authDetail("broken"), item("broken").installHint);
+
+    assert.strictEqual(item("absent").installHint, "Install absent.");
+  }).pipe(
+    Effect.provide(
+      SourceControlDiscovery.layer.pipe(
+        Layer.provide(
+          layerDiscovery([
+            cliDiscovery("slow", "slow-cli"),
+            cliDiscovery("broken", "broken-cli"),
+            cliDiscovery("absent", "absent-cli"),
+          ]),
+        ),
+      ),
+    ),
+  ),
 );

packages/source-control-core/src/server/discovery.ts

@@ -1,6 +1,8 @@
 import * as NodeUtil from "node:util";
 import {
+  VcsProcessExitError,
   VcsProcessSpawnError,
+  VcsProcessTimeoutError,
   type SourceControlProviderAuth,
   type SourceControlProviderDiscoveryItem,
   type SourceControlProviderInfo,
@@ -184,6 +186,20 @@ function isCliRemoteRefinementSpec(
   return spec.type === "cli" && spec.refineUnknownRemote !== undefined;
 }
 
+const isVcsProcessTimeoutError = Schema.is(VcsProcessTimeoutError);
+const isVcsProcessExitError = Schema.is(VcsProcessExitError);
+
+/** An installed CLI whose version probe failed needs a repair, not install advice. */
+function probeFailureHint(spec: SourceControlCliDiscoverySpec, cause: unknown): string {
+  const command = `\`${[spec.executable, ...spec.versionArgs].join(" ")}\``;
+  const outcome = isVcsProcessTimeoutError(cause)
+    ? `timed out after ${cause.timeoutMs}ms`
+    : isVcsProcessExitError(cause)
+      ? `exited with status ${cause.exitCode}`
+      : "failed";
+  return `${command} ${outcome}. Check the \`${spec.executable}\` that the server's PATH resolves.`;
+}
+
 function probeCli(input: {
   readonly spec: SourceControlCliDiscoverySpec;
   readonly process: SourceControlHost.SourceControlHost["Service"]["process"];
@@ -221,7 +237,9 @@ function probeCli(input: {
           executable: input.spec.executable,
           status: "missing" as const,
           version: Option.none<string>(),
-          installHint: input.spec.installHint,
+          installHint: isMissingExecutable(cause)
+            ? input.spec.installHint
+            : probeFailureHint(input.spec, cause),
           detail: detailFromCause(cause),
         } satisfies DiscoveryProbeResult),
       ),
@@ -262,7 +280,7 @@ export function probeSourceControlProvider(input: {
       if (item.status !== "available") {
         return Effect.succeed({
           ...item,
-          auth: unknownAuth("Hosting integration command was not found on the server PATH."),
+          auth: unknownAuth(item.installHint),
         } satisfies SourceControlProviderDiscoveryItem);
       }
 

packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts

@@ -5,12 +5,13 @@ import * as Exit from "effect/Exit";
 import * as FileSystem from "effect/FileSystem";
 import * as Layer from "effect/Layer";
 import * as Option from "effect/Option";
+import * as PlatformError from "effect/PlatformError";
 import * as Schema from "effect/Schema";
 import * as ChildProcessSpawner from "effect/process/ChildProcessSpawner";
 import * as HttpClient from "effect/http/HttpClient";
 import * as HttpClientRequest from "effect/http/HttpClientRequest";
 import * as HttpClientResponse from "effect/http/HttpClientResponse";
-import { VcsProcessSpawnError } from "@t3tools/contracts";
+import { VcsProcessSpawnError, VcsProcessTimeoutError } from "@t3tools/contracts";
 import { firstNonEmptyLine } from "@t3tools/source-control-core/server/discovery";
 import * as TestSourceControlHost from "@t3tools/source-control-testing/TestSourceControlHost";
 
@@ -1689,3 +1690,47 @@ it.effect(
       }
     }),
 );
+
+it.effect("explains a slow fj instead of advising an install when tea is absent", () =>
+  Effect.gen(function* () {
+    const spec = yield* ForgejoSourceControlProvider.makeDiscovery;
+    assert.strictEqual(spec.type, "managed-cli");
+    if (spec.type !== "managed-cli") return;
+    const result = yield* spec.probe("/repo");
+    assert.strictEqual(result.status, "missing");
+    assert.strictEqual(result.executable, "fj");
+    assert.include(result.installHint, "`fj version` timed out");
+    assert.notInclude(result.installHint, "Install");
+  }).pipe(
+    Effect.provide(
+      Layer.mergeAll(
+        Layer.mock(ForgejoCli.ForgejoCli)({ listLogins: () => Effect.succeed([]) }),
+        TestSourceControlHost.layer({
+          process: {
+            run: (input) => {
+              const context = {
+                operation: input.operation,
+                command: input.command,
+                cwd: input.cwd,
+              };
+              if (input.command === "git") return Effect.succeed(processOutput(""));
+              if (input.command === "fj")
+                return Effect.fail(new VcsProcessTimeoutError({ ...context, timeoutMs: 5_000 }));
+              return Effect.fail(
+                new VcsProcessSpawnError({
+                  ...context,
+                  cause: PlatformError.systemError({
+                    _tag: "NotFound",
+                    module: "ChildProcess",
+                    method: "spawn",
+                    pathOrDescriptor: input.command,
+                  }),
+                }),
+              );
+            },
+          },
+        }),
+      ),
+    ),
+  ),
+);

packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.ts

@@ -141,7 +141,10 @@ export const makeDiscovery = Effect.gen(function* () {
         return fj;
       }
       const tea = yield* probeSourceControlProvider({ cwd, process, spec: discovery });
-      return tea.status === "available" || fj.status === "missing" ? tea : fj;
+      // Discovery keeps the install hint only for an absent executable; a slow or broken fj
+      // explains itself better than tea's install advice.
+      const fjAbsent = fj.status === "missing" && fj.installHint === discovery.installHint;
+      return tea.status === "available" || fjAbsent ? tea : fj;
     }),
     refineUnknownRemote: Effect.fn("ForgejoSourceControlProvider.refineUnknownRemote")(
       function* (input: {