fix(shared): looking up the Xcode git no longer blocks the server

packages/shared/src/shell.test.ts

@@ -4,6 +4,8 @@ import { it as effectIt } from "@effect/vitest";
 import * as HostProcess from "@t3tools/shared/HostProcess";
 import { DesktopHostTelemetryMessage } from "@t3tools/contracts";
 import * as Schema from "effect/Schema";
+import * as Scheduler from "effect/Scheduler";
+import * as Deferred from "effect/Deferred";
 import * as Effect from "effect/Effect";
 import * as Fiber from "effect/Fiber";
 import * as FileSystem from "effect/FileSystem";
@@ -23,6 +25,7 @@ import {
   type CommandAvailabilityChecker,
   isCommandAvailable,
   listLoginShellCandidates,
+  locateXcodeGitWithXcrun,
   mergePathEntries,
   mergePathValues,
   npmShimExecutable,
@@ -44,14 +47,26 @@ const decodeDesktopHostMessage = Schema.decodeUnknownSync(DesktopHostTelemetryMe
 const nativeProbe = vi.hoisted(() => ({
   execFile: vi.fn<typeof import("node:child_process").execFile>(),
   original: undefined as unknown as typeof import("node:child_process").execFile,
+  spawn: vi.fn<typeof import("node:child_process").spawn>(),
 }));
 
 vi.mock("node:child_process", async (importOriginal) => {
   const original = await importOriginal<typeof import("node:child_process")>();
   nativeProbe.original = original.execFile;
-  return { ...original, execFile: nativeProbe.execFile.mockImplementation(original.execFile) };
+  return {
+    ...original,
+    execFile: nativeProbe.execFile.mockImplementation(original.execFile),
+    spawn: nativeProbe.spawn.mockImplementation(original.spawn),
+  };
 });
 
+const exitOf = (child: NodeChildProcess.ChildProcess) =>
+  child.exitCode !== null || child.signalCode !== null
+    ? Promise.resolve({ code: child.exitCode, signal: child.signalCode })
+    : new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) =>
+        child.once("exit", (code, signal) => resolve({ code, signal })),
+      );
+
 const withWindowsEnvironmentMocks = <A, E, R>(
   effect: Effect.Effect<A, E, R>,
   readEnvironment: (
@@ -860,10 +875,12 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
         Effect.provideService(SpawnExecutableResolution, (command) =>
           command === "git" ? options.gitOnPath : files.has(command) ? command : undefined,
         ),
-        Effect.provideService(XcodeGitLocation, () => {
-          host.xcrunCalls += 1;
-          return host.xcrunGit;
-        }),
+        Effect.provideService(XcodeGitLocation, () =>
+          Effect.sync(() => {
+            host.xcrunCalls += 1;
+            return host.xcrunGit;
+          }),
+        ),
       );
       return { files, host, resolve };
     };
@@ -879,6 +896,118 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
       }).pipe(Effect.provideService(CommandResolutionCache, new Map())),
     );
 
+    // A small yield budget lets the scheduler switch callers between any two operations.
+    it.effect.each([undefined, 3, 4, 5, 6] as const)(
+      "waits for xcrun without blocking, sharing one lookup between callers (yield budget %s)",
+      (budget) =>
+        Effect.gen(function* () {
+          let lookup = yield* Deferred.make<string | undefined>();
+          let xcrunCalls = 0;
+          const resolve = resolveSpawnCommand("git", ["status"], {
+            env: { PATH: "/fake/bin" },
+          }).pipe(
+            Effect.map((resolved) => resolved.command),
+            Effect.provideService(HostProcess.Platform, "darwin"),
+            Effect.provideService(SpawnExecutableResolution, (command) =>
+              command === "git" ? "/usr/bin/git" : command === XCODE_GIT ? command : undefined,
+            ),
+            Effect.provideService(XcodeGitLocation, () => {
+              xcrunCalls += 1;
+              return Deferred.await(lookup);
+            }),
+            budget === undefined
+              ? (effect) => effect
+              : Effect.provideService(Scheduler.MaxOpsBeforeYield, budget),
+          );
+          const callers = Effect.forEach([1, 2, 3], () =>
+            Effect.forkChild(resolve, { startImmediately: true }),
+          );
+          const first = yield* callers;
+          yield* Deferred.succeed(lookup, XCODE_GIT);
+          expect(yield* Fiber.joinAll(first)).toEqual([XCODE_GIT, XCODE_GIT, XCODE_GIT]);
+          expect(xcrunCalls).toBe(1);
+
+          // Once the answer expires, the next callers share one new lookup.
+          lookup = yield* Deferred.make<string | undefined>();
+          yield* TestClock.adjust("31 seconds");
+          const second = yield* callers;
+          yield* Deferred.succeed(lookup, XCODE_GIT);
+          expect(yield* Fiber.joinAll(second)).toEqual([XCODE_GIT, XCODE_GIT, XCODE_GIT]);
+          expect(xcrunCalls).toBe(2);
+        }).pipe(Effect.provideService(CommandResolutionCache, new Map())),
+    );
+
+    it.effect("the xcrun lookup resolves nothing when xcrun is missing or fails", () =>
+      Effect.gen(function* () {
+        const env = { PATH: "/usr/bin:/bin", DEVELOPER_DIR: "/nonexistent/t3-developer-dir" };
+        const locate = yield* XcodeGitLocation;
+        expect(yield* locate(env)).toBeUndefined();
+        expect(
+          yield* locateXcodeGitWithXcrun(env, { xcrun: "/nonexistent/t3-xcrun" }),
+        ).toBeUndefined();
+      }),
+    );
+
+    it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")(
+      "the xcrun lookup settles at its deadline when a descendant keeps stdout open",
+      () =>
+        Effect.gen(function* () {
+          const fs = yield* FileSystem.FileSystem;
+          const path = yield* Path.Path;
+          const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-xcrun-" });
+          const descendantPid = path.join(directory, "descendant.pid");
+          const xcrun = path.join(directory, "xcrun");
+          yield* fs.writeFileString(
+            xcrun,
+            `#!/bin/sh\nsleep 600 &\necho $! > '${descendantPid}'\necho /fake/git\n`,
+          );
+          yield* fs.chmod(xcrun, 0o755);
+          const stopDescendant = fs.readFileString(descendantPid).pipe(
+            Effect.flatMap((pid) => Effect.sync(() => process.kill(Number(pid.trim())))),
+            Effect.ignore,
+          );
+          yield* Effect.addFinalizer(() => stopDescendant);
+          const lookup = yield* locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun }).pipe(
+            Effect.forkChild({ startImmediately: true }),
+          );
+          const child = nativeProbe.spawn.mock.results.at(-1)
+            ?.value as NodeChildProcess.ChildProcess;
+          yield* Effect.promise(() => exitOf(child));
+
+          // xcrun has exited and printed a path, but its descendant still holds stdout open.
+          expect(lookup.pollUnsafe()).toBeUndefined();
+          yield* TestClock.adjust("5 seconds");
+          expect(yield* Fiber.join(lookup)).toBeUndefined();
+        }).pipe(Effect.scoped),
+    );
+
+    it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")(
+      "the xcrun lookup kills an xcrun that ignores SIGTERM at its deadline",
+      () =>
+        Effect.gen(function* () {
+          const fs = yield* FileSystem.FileSystem;
+          const path = yield* Path.Path;
+          const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-xcrun-" });
+          const xcrun = path.join(directory, "xcrun");
+          yield* fs.writeFileString(xcrun, "#!/bin/sh\ntrap '' TERM\nexec sleep 600\n");
+          yield* fs.chmod(xcrun, 0o755);
+          const lookup = yield* locateXcodeGitWithXcrun({ PATH: "/usr/bin:/bin" }, { xcrun }).pipe(
+            Effect.forkChild({ startImmediately: true }),
+          );
+          const child = nativeProbe.spawn.mock.results.at(-1)
+            ?.value as NodeChildProcess.ChildProcess;
+          const kill = vi.spyOn(child, "kill");
+          const exited = exitOf(child);
+          yield* Effect.addFinalizer(() => Effect.sync(() => child.kill("SIGKILL")));
+
+          expect(lookup.pollUnsafe()).toBeUndefined();
+          yield* TestClock.adjust("5 seconds");
+          expect(yield* Fiber.join(lookup)).toBeUndefined();
+          expect(kill).toHaveBeenCalledWith("SIGKILL");
+          expect(yield* Effect.promise(() => exited)).toEqual({ code: null, signal: "SIGKILL" });
+        }).pipe(Effect.scoped),
+    );
+
     it.effect("keeps git when PATH has another git or the host is not macOS", () =>
       Effect.gen(function* () {
         const homebrew = setup({ gitOnPath: "/opt/homebrew/bin/git", xcrunGit: XCODE_GIT });
@@ -901,18 +1030,27 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
       }).pipe(Effect.provideService(CommandResolutionCache, new Map())),
     );
 
-    it.effect("looks again when the resolved git disappears", () =>
-      Effect.gen(function* () {
-        const betaGit = "/Applications/Xcode-beta.app/Contents/Developer/usr/bin/git";
-        const { files, host, resolve } = setup({ gitOnPath: "/usr/bin/git", xcrunGit: XCODE_GIT });
-        expect(yield* resolve).toBe(XCODE_GIT);
-
-        files.delete(XCODE_GIT);
-        files.add(betaGit);
-        host.xcrunGit = betaGit;
-        expect(yield* resolve).toBe(betaGit);
-        expect(host.xcrunCalls).toBe(2);
-      }).pipe(Effect.provideService(CommandResolutionCache, new Map())),
+    it.effect.each([undefined, 3] as const)(
+      "looks again when the resolved git disappears (yield budget %s)",
+      (budget) =>
+        Effect.gen(function* () {
+          const betaGit = "/Applications/Xcode-beta.app/Contents/Developer/usr/bin/git";
+          const { files, host, resolve } = setup({
+            gitOnPath: "/usr/bin/git",
+            xcrunGit: XCODE_GIT,
+          });
+          const budgeted =
+            budget === undefined
+              ? resolve
+              : resolve.pipe(Effect.provideService(Scheduler.MaxOpsBeforeYield, budget));
+          expect(yield* budgeted).toBe(XCODE_GIT);
+
+          files.delete(XCODE_GIT);
+          files.add(betaGit);
+          host.xcrunGit = betaGit;
+          expect(yield* budgeted).toBe(betaGit);
+          expect(host.xcrunCalls).toBe(2);
+        }).pipe(Effect.provideService(CommandResolutionCache, new Map())),
     );
   });
 

packages/shared/src/shell.ts

@@ -6,7 +6,9 @@ import * as NodeFS from "node:fs";
 import * as Cache from "effect/Cache";
 import * as Clock from "effect/Clock";
 import * as Duration from "effect/Duration";
+import * as Deferred from "effect/Deferred";
 import * as Effect from "effect/Effect";
+import * as Exit from "effect/Exit";
 import * as FileSystem from "effect/FileSystem";
 import * as Option from "effect/Option";
 import * as Path from "effect/Path";
@@ -760,22 +762,63 @@ export function npmShimExecutable(
   return isFile(executable) ? executable : undefined;
 }
 
-export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => string | undefined;
+export type XcodeGitLocator = (env: NodeJS.ProcessEnv) => Effect.Effect<string | undefined>;
+
+const XCRUN_MAX_OUTPUT_BYTES = 1024 * 1024;
+const XCRUN_DEADLINE = Duration.seconds(5);
+
+// Stopping it kills xcrun with SIGKILL, which it cannot ignore, and stops reading a stdout
+// that a descendant may still hold open after xcrun exits.
+const runXcrunFindGit = (xcrun: string, env: NodeJS.ProcessEnv) =>
+  Effect.callback<Option.Option<string>>((resume) => {
+    let child: NodeChildProcess.ChildProcess;
+    try {
+      child = NodeChildProcess.spawn(xcrun, ["--find", "git"], {
+        env,
+        stdio: ["ignore", "pipe", "ignore"],
+      });
+    } catch {
+      resume(Effect.succeedNone);
+      return;
+    }
+    const stop = () => {
+      child.kill("SIGKILL");
+      child.unref();
+      child.stdout?.destroy();
+    };
+    const chunks: Array<Buffer> = [];
+    let bytes = 0;
+    child.stdout?.on("data", (chunk: Buffer) => {
+      bytes += chunk.byteLength;
+      if (bytes > XCRUN_MAX_OUTPUT_BYTES) {
+        stop();
+        resume(Effect.succeedNone);
+        return;
+      }
+      chunks.push(chunk);
+    });
+    child.once("error", () => resume(Effect.succeedNone));
+    child.once("close", (code) => {
+      const located = Buffer.concat(chunks).toString("utf8").trim();
+      resume(
+        code === 0 && NodePath.isAbsolute(located)
+          ? Effect.succeedSome(located)
+          : Effect.succeedNone,
+      );
+    });
+    return Effect.sync(stop);
+  });
 
 // xcrun honors DEVELOPER_DIR and the `xcode-select` choice, as the shim does.
-function locateXcodeGitWithXcrun(env: NodeJS.ProcessEnv): string | undefined {
-  try {
-    const located = NodeChildProcess.execFileSync("/usr/bin/xcrun", ["--find", "git"], {
-      encoding: "utf8",
-      env,
-      timeout: 5_000,
-      stdio: ["ignore", "pipe", "ignore"],
-    }).trim();
-    return NodePath.isAbsolute(located) ? located : undefined;
-  } catch {
-    return undefined;
-  }
-}
+// It takes ~15 ms and reruns when the cached answer expires, so it must not block the event loop.
+export const locateXcodeGitWithXcrun = (
+  env: NodeJS.ProcessEnv,
+  options: { readonly xcrun?: string } = {},
+): Effect.Effect<string | undefined> =>
+  runXcrunFindGit(options.xcrun ?? "/usr/bin/xcrun", env).pipe(
+    Effect.timeoutOption(XCRUN_DEADLINE),
+    Effect.map((found) => Option.getOrUndefined(Option.flatten(found))),
+  );
 
 export const XcodeGitLocation = Context.Reference<XcodeGitLocator>(
   "@t3tools/shared/shell/XcodeGitLocation",
@@ -784,6 +827,28 @@ export const XcodeGitLocation = Context.Reference<XcodeGitLocator>(
 
 const MACOS_GIT_SHIM = "/usr/bin/git";
 
+// Callers that miss the cache together wait for one xcrun run instead of starting their own.
+const xcodeGitLookups = new WeakMap<
+  Map<string, CommandResolutionCacheEntry>,
+  Map<string, Deferred.Deferred<string | null>>
+>();
+
+const lookupXcodeGit = (
+  env: NodeJS.ProcessEnv,
+  resolveExecutable: SpawnExecutableResolver,
+  locateXcodeGit: XcodeGitLocator,
+): Effect.Effect<string | null> =>
+  resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM
+    ? locateXcodeGit(env).pipe(
+        Effect.map((located) =>
+          located !== undefined && resolveExecutable(located, "darwin", env) !== undefined
+            ? located
+            : null,
+        ),
+        Effect.catchCause(() => Effect.succeed(null)),
+      )
+    : Effect.succeed(null);
+
 /**
  * Skip the xcrun shim's per-spawn lookup only when git resolves to /usr/bin/git.
  * User-selected git on PATH still wins, children keep PATH, and lookup failures use `git`.
@@ -812,13 +877,33 @@ const resolveMacosGit = Effect.fnUntraced(function* (
   ) {
     return cached.resolvedPath ?? "git";
   }
-  const located =
-    resolveExecutable("git", "darwin", env) === MACOS_GIT_SHIM ? locateXcodeGit(env) : undefined;
-  const realGit =
-    located !== undefined && resolveExecutable(located, "darwin", env) !== undefined
-      ? located
-      : null;
-  cacheCommandResolution(cache, cacheKey, realGit, nowNanos);
+  const pending = xcodeGitLookups.get(cache) ?? new Map<string, Deferred.Deferred<string | null>>();
+  xcodeGitLookups.set(cache, pending);
+  // Registering and starting the lookup cannot be split by a yield or an interrupt, so callers
+  // find it as soon as it exists. Detached, a caller that stops waiting does not stop it.
+  const lookup = yield* Effect.uninterruptible(
+    Effect.suspend(() => {
+      const existing = pending.get(cacheKey);
+      if (existing !== undefined) return Effect.succeed(existing);
+      const shared = Deferred.makeUnsafe<string | null>();
+      pending.set(cacheKey, shared);
+      return lookupXcodeGit(env, resolveExecutable, locateXcodeGit).pipe(
+        Effect.tap((realGit) =>
+          Effect.sync(() => cacheCommandResolution(cache, cacheKey, realGit, nowNanos)),
+        ),
+        Effect.onExit((exit) =>
+          Effect.sync(() => {
+            if (pending.get(cacheKey) === shared) pending.delete(cacheKey);
+          }).pipe(
+            Effect.andThen(Deferred.done(shared, Exit.isSuccess(exit) ? exit : Exit.succeed(null))),
+          ),
+        ),
+        Effect.forkDetach,
+        Effect.as(shared),
+      );
+    }),
+  );
+  const realGit = yield* Deferred.await(lookup);
   return realGit ?? "git";
 });