diff --git a/apps/server/src/project/Layers/RepositoryIdentityResolver.test.ts b/apps/server/src/project/Layers/RepositoryIdentityResolver.test.ts index 1c985cd8592..a456eb36052 100644 --- a/apps/server/src/project/Layers/RepositoryIdentityResolver.test.ts +++ b/apps/server/src/project/Layers/RepositoryIdentityResolver.test.ts @@ -6,6 +6,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import { TestClock } from "effect/testing"; +import { ChildProcessSpawner } from "effect/unstable/process"; import * as ProcessRunner from "../../processRunner.ts"; import { RepositoryIdentityResolver } from "../Services/RepositoryIdentityResolver.ts"; @@ -26,6 +27,33 @@ const git = (cwd: string, args: ReadonlyArray) => }); }).pipe(Effect.provide(ProcessRunner.layer)); +const makeCallCountingProcessRunner = ( + state: { revParseCalls: number; remoteCalls: number }, + fixture: { readonly topLevel: string; readonly remotesStdout: string }, +) => + ProcessRunner.ProcessRunner.of({ + run: (input) => { + const succeed = (stdout: string) => + Effect.succeed({ + stdout, + stderr: "", + code: ChildProcessSpawner.ExitCode(0), + timedOut: false, + stdoutTruncated: false, + stderrTruncated: false, + } satisfies ProcessRunner.ProcessRunOutput); + if (input.args.includes("rev-parse") && input.args.includes("--show-toplevel")) { + state.revParseCalls += 1; + return succeed(fixture.topLevel); + } + if (input.args.includes("remote") && input.args.includes("-v")) { + state.remoteCalls += 1; + return succeed(fixture.remotesStdout); + } + return succeed(""); + }, + }); + const makeRepositoryIdentityResolverTestLayer = (options: { readonly positiveCacheTtl?: Duration.Input; readonly negativeCacheTtl?: Duration.Input; @@ -236,4 +264,102 @@ it.layer(NodeServices.layer)("RepositoryIdentityResolverLive", (it) => { ), ), ); + + it.effect( + "resolves the git top-level only once across repeated resolves of the same workspace", + () => + Effect.gen(function* () { + const state = { revParseCalls: 0, remoteCalls: 0 }; + const cwd = "/workspace/project"; + const resolverLayer = Layer.effect( + RepositoryIdentityResolver, + makeRepositoryIdentityResolver({ cacheCapacity: 16 }), + ).pipe( + Layer.provide( + Layer.succeed( + ProcessRunner.ProcessRunner, + makeCallCountingProcessRunner(state, { + topLevel: cwd, + remotesStdout: + "origin\tgit@github.com:T3Tools/t3code.git (fetch)\n" + + "origin\tgit@github.com:T3Tools/t3code.git (push)\n", + }), + ), + ), + ); + + yield* Effect.gen(function* () { + const resolver = yield* RepositoryIdentityResolver; + const first = yield* resolver.resolve(cwd); + const second = yield* resolver.resolve(cwd); + + expect(first?.canonicalKey).toBe("github.com/t3tools/t3code"); + expect(second?.canonicalKey).toBe("github.com/t3tools/t3code"); + // The top-level lookup must be memoised: replaying many events for the + // same workspace should not re-spawn `git rev-parse --show-toplevel`. + expect(state.revParseCalls).toBe(1); + expect(state.remoteCalls).toBe(1); + }).pipe(Effect.provide(resolverLayer)); + }), + ); + + it.effect( + "retries the top-level lookup after a transient git failure instead of caching the cwd fallback", + () => + Effect.gen(function* () { + let revParseCalls = 0; + const nestedWorkspace = "/workspace/repo/packages/web"; + const topLevel = "/workspace/repo"; + const succeed = (stdout: string) => + Effect.succeed({ + stdout, + stderr: "", + code: ChildProcessSpawner.ExitCode(0), + timedOut: false, + stdoutTruncated: false, + stderrTruncated: false, + } satisfies ProcessRunner.ProcessRunOutput); + const processRunner = ProcessRunner.ProcessRunner.of({ + run: (input) => { + if (input.args.includes("rev-parse") && input.args.includes("--show-toplevel")) { + revParseCalls += 1; + // First lookup times out (the #2037 failure mode); later ones resolve. + return revParseCalls === 1 + ? Effect.succeed({ + stdout: "", + stderr: "", + code: null, + timedOut: true, + stdoutTruncated: false, + stderrTruncated: false, + } satisfies ProcessRunner.ProcessRunOutput) + : succeed(topLevel); + } + if (input.args.includes("remote") && input.args.includes("-v")) { + return succeed( + "origin\tgit@github.com:T3Tools/t3code.git (fetch)\n" + + "origin\tgit@github.com:T3Tools/t3code.git (push)\n", + ); + } + return succeed(""); + }, + }); + const resolverLayer = Layer.effect( + RepositoryIdentityResolver, + makeRepositoryIdentityResolver({ cacheCapacity: 16 }), + ).pipe(Layer.provide(Layer.succeed(ProcessRunner.ProcessRunner, processRunner))); + + yield* Effect.gen(function* () { + const resolver = yield* RepositoryIdentityResolver; + yield* resolver.resolve(nestedWorkspace); + const second = yield* resolver.resolve(nestedWorkspace); + + // A failed lookup is not cached, so the next resolve retries rev-parse + // and recovers the real top-level rather than pinning rootPath to the + // nested workspace path. + expect(revParseCalls).toBe(2); + expect(second?.rootPath).toBe(topLevel); + }).pipe(Effect.provide(resolverLayer)); + }), + ); }); diff --git a/apps/server/src/project/Layers/RepositoryIdentityResolver.ts b/apps/server/src/project/Layers/RepositoryIdentityResolver.ts index d4ae073b953..6737602816b 100644 --- a/apps/server/src/project/Layers/RepositoryIdentityResolver.ts +++ b/apps/server/src/project/Layers/RepositoryIdentityResolver.ts @@ -83,11 +83,17 @@ interface RepositoryIdentityResolverOptions { readonly negativeCacheTtl?: Duration.Input; } +interface RepositoryIdentityCacheKey { + /** The git top-level path when found, otherwise the original cwd. */ + readonly cacheKey: string; + /** Whether `git rev-parse --show-toplevel` actually resolved a top-level. */ + readonly resolved: boolean; +} + const resolveRepositoryIdentityCacheKey = Effect.fn("resolveRepositoryIdentityCacheKey")(function* ( cwd: string, -) { +): Effect.fn.Return { const processRunner = yield* ProcessRunner.ProcessRunner; - let cacheKey = cwd; // git is a real executable on every platform — no cmd.exe shell mode, which // would split paths containing spaces during cmd's re-tokenization. @@ -99,15 +105,13 @@ const resolveRepositoryIdentityCacheKey = Effect.fn("resolveRepositoryIdentityCa }) .pipe(Effect.option); if (topLevelResult._tag === "None" || topLevelResult.value.code !== 0) { - return cacheKey; + return { cacheKey: cwd, resolved: false }; } const candidate = topLevelResult.value.stdout.trim(); - if (candidate.length > 0) { - cacheKey = candidate; - } - - return cacheKey; + return candidate.length > 0 + ? { cacheKey: candidate, resolved: true } + : { cacheKey: cwd, resolved: false }; }); const resolveRepositoryIdentityFromCacheKey = Effect.fn("resolveRepositoryIdentityFromCacheKey")( @@ -135,6 +139,38 @@ export const makeRepositoryIdentityResolver = Effect.fn("makeRepositoryIdentityR function* (options: RepositoryIdentityResolverOptions = {}) { const processRunner = yield* ProcessRunner.ProcessRunner; + // `git rev-parse --show-toplevel` is otherwise re-run on every resolve. During + // `replayEvents` enrichment that means one git subprocess per project event on + // every reconnect, which is the dominant cost behind the "Some requests are + // slow" toast (#2037). Memoise the cwd -> top-level mapping so a replay burst + // resolves each workspace at most once per positive TTL. + // + // Only a resolved top-level is cached. An unresolved cwd (not a repo, or git + // timed out) is intentionally not cached: a timeout is not a definitive answer, + // and caching the cwd fallback would let a nested workspace keep resolving + // `git remote -v` against the nested path, pinning the identity rootPath to the + // nested dir until the entry expired. Re-running rev-parse on the next resolve + // self-corrects, matching the pre-cache behaviour. + const repositoryIdentityCacheKeyCache = yield* Cache.makeWith< + string, + RepositoryIdentityCacheKey + >( + (cwd) => + resolveRepositoryIdentityCacheKey(cwd).pipe( + Effect.provideService(ProcessRunner.ProcessRunner, processRunner), + ), + { + capacity: options.cacheCapacity ?? DEFAULT_REPOSITORY_IDENTITY_CACHE_CAPACITY, + timeToLive: Exit.match({ + onSuccess: (value) => + value.resolved + ? (options.positiveCacheTtl ?? DEFAULT_POSITIVE_CACHE_TTL) + : Duration.zero, + onFailure: () => Duration.zero, + }), + }, + ); + const repositoryIdentityCache = yield* Cache.makeWith( (cacheKey) => resolveRepositoryIdentityFromCacheKey(cacheKey).pipe( @@ -155,9 +191,7 @@ export const makeRepositoryIdentityResolver = Effect.fn("makeRepositoryIdentityR const resolve: RepositoryIdentityResolverShape["resolve"] = Effect.fn( "RepositoryIdentityResolver.resolve", )(function* (cwd) { - const cacheKey = yield* resolveRepositoryIdentityCacheKey(cwd).pipe( - Effect.provideService(ProcessRunner.ProcessRunner, processRunner), - ); + const { cacheKey } = yield* Cache.get(repositoryIdentityCacheKeyCache, cwd); return yield* Cache.get(repositoryIdentityCache, cacheKey); });