From 47c5c5bdbfea128fd1fb038a3ebbdca5c5a23916 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Thu, 29 Oct 2026 08:40:31 -0700 Subject: [PATCH] perf(server): source control discovery no longer waits on Bitbucket Bitbucket probes skip /user without a credential, API probes time out after 3s, and CLI version and auth probes spawn concurrently. Co-Authored-By: Claude Opus 5.5 --- .../SourceControlProviderRegistry.test.ts | 32 +++++++++- .../src/server/BitbucketApi.test.ts | 3 +- .../src/server/BitbucketApi.ts | 31 ++++++---- .../src/server/discovery.ts | 60 ++++++++++--------- 4 files changed, 85 insertions(+), 41 deletions(-) diff --git a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts index 249bc07b3ad3f..05b6a1a8d0e42 100644 --- a/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts +++ b/apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts @@ -118,7 +118,12 @@ function makeRegistry(input: { kind: provider.kind, label: provider.kind, installHint: "unused", - probeAuth: Effect.die("unused"), + probeAuth: Effect.succeed({ + status: "unknown", + account: Option.none(), + host: Option.none(), + detail: Option.none(), + }), }, })), ).pipe( @@ -440,3 +445,28 @@ it.effect("asks again once Settings change", () => assert.strictEqual((yield* resolveUnknown(registry, "c")).provider.kind, "github"); }), ); + +it.effect("answers discovery without waiting on a hosting API that stalls", () => + Effect.gen(function* () { + const registry = yield* makeRegistry({ + remotes: [], + discovery: [ + claimingCli("gitlab"), + { + type: "api", + kind: kind("bitbucket"), + label: "Bitbucket", + installHint: "unused", + probeAuth: Effect.never, + }, + ], + }); + const discovering = yield* registry.discover.pipe(Effect.forkChild); + yield* TestClock.adjust("3 seconds"); + const items = yield* Fiber.join(discovering); + const bitbucket = items.find((item) => item.kind === "bitbucket"); + assert.strictEqual(bitbucket?.status, "available"); + assert.strictEqual(bitbucket?.auth.status, "unknown"); + assert.strictEqual(items.find((item) => item.kind === "gitlab")?.status, "available"); + }), +); diff --git a/packages/source-control-bitbucket/src/server/BitbucketApi.test.ts b/packages/source-control-bitbucket/src/server/BitbucketApi.test.ts index 26891c3548974..c0464276abf24 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketApi.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketApi.test.ts @@ -513,7 +513,7 @@ it.effect("never puts a saved token that is unsafe for an HTTP header on the wir }); it.effect("reports saved credentials as configured when Bitbucket cannot confirm them", () => { - const { layer } = makeLayer({ + const { execute, layer } = makeLayer({ response: () => new Response(null, { status: 401 }), env: { T3CODE_BITBUCKET_API_BASE_URL: "https://api.test.local/2.0" }, }); @@ -523,6 +523,7 @@ it.effect("reports saved credentials as configured when Bitbucket cannot confirm const settings = yield* TestSourceControlHost.TestSourceControlHostSettings; assert.strictEqual((yield* bitbucket.probeAuth).status, "unauthenticated"); + assert.strictEqual(execute.mock.calls.length, 0); yield* settings.update({ sourceControlHosts: { bitbucket: { accessToken: "saved-access-token" } }, diff --git a/packages/source-control-bitbucket/src/server/BitbucketApi.ts b/packages/source-control-bitbucket/src/server/BitbucketApi.ts index 72077040aa870..863b206d95578 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketApi.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketApi.ts @@ -986,18 +986,25 @@ export const make = Effect.gen(function* () { return BitbucketApi.of({ request, - probeAuth: executeJson( - "probeAuth", - HttpClientRequest.get(apiUrl("/user")), - BitbucketUserSchema, - ).pipe( - Effect.map((user) => ({ - status: "authenticated" as const, - account: nonEmpty(user.username ?? user.display_name ?? user.account_id), - host: Option.some("bitbucket.org"), - detail: Option.none(), - })), - Effect.catch(() => currentCredential.pipe(Effect.map(authFromCredential))), + // Bitbucket answers anonymous /user with 401, so without a credential there is nothing to ask. + probeAuth: currentCredential.pipe( + Effect.flatMap((credential) => + credential === null + ? Effect.succeed(authFromCredential(null)) + : executeJson( + "probeAuth", + HttpClientRequest.get(apiUrl("/user")), + BitbucketUserSchema, + ).pipe( + Effect.map((user) => ({ + status: "authenticated" as const, + account: nonEmpty(user.username ?? user.display_name ?? user.account_id), + host: Option.some("bitbucket.org"), + detail: Option.none(), + })), + Effect.catch(() => currentCredential.pipe(Effect.map(authFromCredential))), + ), + ), ), listPullRequests: Effect.fn("BitbucketApi.listPullRequests")(function (input) { return resolveRepository(input).pipe( diff --git a/packages/source-control-core/src/server/discovery.ts b/packages/source-control-core/src/server/discovery.ts index 66889ddc4f2ab..2f018bfa10ca3 100644 --- a/packages/source-control-core/src/server/discovery.ts +++ b/packages/source-control-core/src/server/discovery.ts @@ -75,6 +75,8 @@ type SourceControlCliRemoteRefinementSpec = SourceControlCliDiscoverySpec & { }; const DEFAULT_PROBE_TIMEOUT_MS = 5_000; +/** An API host that stalls must not hold the whole discovery answer behind it. */ +const API_PROBE_TIMEOUT = "3 seconds"; function probeTimeoutMs(spec: SourceControlCliDiscoverySpec): number { return spec.probeTimeoutMs ?? DEFAULT_PROBE_TIMEOUT_MS; @@ -236,6 +238,12 @@ export function probeSourceControlProvider(input: { if (input.spec.type === "managed-cli") return input.spec.probe(input.cwd); if (input.spec.type === "api") { return input.spec.probeAuth.pipe( + Effect.timeoutOption(API_PROBE_TIMEOUT), + Effect.map( + Option.getOrElse(() => + unknownAuth("The hosting service did not answer in time. Refresh to try again."), + ), + ), Effect.map( (auth) => ({ @@ -253,20 +261,15 @@ export function probeSourceControlProvider(input: { const spec = input.spec; - return probeCli({ - spec, - process: input.process, - cwd: input.cwd, - }).pipe( - Effect.flatMap((item) => { - if (item.status !== "available") { - return Effect.succeed({ - ...item, - auth: unknownAuth("Hosting integration command was not found on the server PATH."), - } satisfies SourceControlProviderDiscoveryItem); - } - - return input.process + // The auth check does not depend on the version output, so both spawn at once. + return Effect.all( + [ + probeCli({ + spec, + process: input.process, + cwd: input.cwd, + }), + input.process .run({ operation: "source-control.discovery.auth", command: spec.executable, @@ -278,21 +281,24 @@ export function probeSourceControlProvider(input: { appendTruncationMarker: true, }) .pipe( - Effect.map( - (result) => - ({ - ...item, - auth: spec.parseAuth(result), - }) satisfies SourceControlProviderDiscoveryItem, - ), + Effect.map(spec.parseAuth), Effect.catch((cause) => - Effect.succeed({ - ...item, - auth: unknownAuth(Option.getOrUndefined(detailFromCause(cause))), - } satisfies SourceControlProviderDiscoveryItem), + Effect.succeed(unknownAuth(Option.getOrUndefined(detailFromCause(cause)))), ), - ); - }), + ), + ], + { concurrency: "unbounded" }, + ).pipe( + Effect.map( + ([item, auth]) => + ({ + ...item, + auth: + item.status === "available" + ? auth + : unknownAuth("Hosting integration command was not found on the server PATH."), + }) satisfies SourceControlProviderDiscoveryItem, + ), ); }