authorgravatar for niklas@jadeva.seNiklas Westman <niklas@jadeva.se> 2026-09-05 03:10:46+02:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-09-04 18:10:46-07:00
logc3caceade14ccda0686db0031aca79d7d38a71d0
treef629351d8b03b3789ee91342af1de4dbe94fe3a6
parent110bbe6b55d684627a2481aebfd7c1ae3396011c
signature Signed by PGP key B5690EEEBB952194

perf(shared): skip duplicate PATH entries and per-probe tracing (#9618)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>

2 files changed, 129 insertions(+), 4 deletions(-)

packages/shared/src/shell.test.ts+121
......@@ -2,11 +2,15 @@ import * as NodeServices from "@effect/platform-node/NodeServices";
22import { it as effectIt } from "@effect/vitest";
33import { HostProcessEnvironment, HostProcessPlatform } from "@t3tools/shared/hostProcess";
44import * as Effect from "effect/Effect";
5import * as FileSystem from "effect/FileSystem";
6import * as Path from "effect/Path";
7import * as TestClock from "effect/testing/TestClock";
58import { describe, expect, it, vi } from "vite-plus/test";
69
710import {
811 extractPathFromShellOutput,
912 CommandAvailability,
13 CommandResolutionCache,
1014 type CommandAvailabilityChecker,
1115 isCommandAvailable,
1216 listLoginShellCandidates,
......@@ -375,6 +379,123 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => {
375379 expect(result._tag).toBe("Failure");
376380 }),
377381 );
382
383 // Records every path the scan stats, without ever reporting a match, so the
384 // walk runs to exhaustion and the probe set can be inspected. Assertions
385 // below count probes rather than naming paths: `Path` is the host's, so the
386 // separator differs between a Windows and a Linux CI runner.
387 const recordProbes = (env: NodeJS.ProcessEnv) =>
388 Effect.gen(function* () {
389 const probed: Array<string> = [];
390 const result = yield* resolveCommandPath("definitely-not-installed", { env }).pipe(
391 Effect.provideService(HostProcessPlatform, "win32"),
392 Effect.provideService(CommandResolutionCache, new Map()),
393 Effect.provide(
394 FileSystem.layerNoop({
395 stat: (filePath) =>
396 Effect.sync(() => {
397 probed.push(filePath);
398 return { type: "Directory" } as FileSystem.File.Info;
399 }),
400 }),
401 ),
402 Effect.result,
403 );
404
405 expect(result._tag).toBe("Failure");
406 return probed;
407 });
408
409 it.effect("visits a repeated PATH directory only once", () =>
410 Effect.gen(function* () {
411 const probed = yield* recordProbes({
412 PATH: "C:\\bin;C:\\other;C:\\bin;C:\\other",
413 PATHEXT: ".COM;.EXE",
414 });
415
416 // Two directories, two extensions, upper and lowercase spellings.
417 expect(probed).toHaveLength(8);
418 expect(new Set(probed).size).toBe(probed.length);
419 }),
420 );
421
422 it.effect("still visits a PATH entry that differs only in case", () =>
423 Effect.gen(function* () {
424 const probed = yield* recordProbes({
425 PATH: "C:\\bin;C:\\BIN",
426 PATHEXT: ".COM;.EXE",
427 });
428
429 // Deliberately not folded together. Windows 10+ can mark a directory
430 // case-sensitive, so the two spellings are not provably one directory and
431 // skipping the second could hide a command that is really there.
432 expect(probed).toHaveLength(8);
433 }),
434 );
435
436 it.effect.each(["audit-command", "audit-command.CMD"])(
437 "resolves lowercase executable files for %s in a case-sensitive Windows PATH directory",
438 (command) =>
439 Effect.gen(function* () {
440 const fs = yield* FileSystem.FileSystem;
441 const path = yield* Path.Path;
442 const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-case-sensitive-path-" });
443 const executable = path.join(cwd, "audit-command.cmd");
444 yield* fs.writeFileString(executable, "@echo off\n");
445
446 const resolved = yield* resolveCommandPath(command, {
447 env: { PATH: cwd, PATHEXT: ".CMD" },
448 }).pipe(
449 Effect.provideService(HostProcessPlatform, "win32"),
450 Effect.provideService(CommandResolutionCache, new Map()),
451 Effect.provideService(FileSystem.FileSystem, {
452 ...fs,
453 // Keep this case-sensitive fixture portable to case-insensitive hosts.
454 stat: (filePath) =>
455 fs.stat(filePath === executable ? filePath : path.join(cwd, "missing")),
456 }),
457 );
458
459 expect(resolved).toBe(executable);
460 }),
461 );
462
463 it.effect("keeps cached misses until expiry while allowing explicit paths", () =>
464 Effect.gen(function* () {
465 const fs = yield* FileSystem.FileSystem;
466 const path = yield* Path.Path;
467 const cwd = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-cache-" });
468 const executable = path.join(cwd, "appeared.CMD");
469 const options = { env: { PATH: `${cwd};${cwd}`, PATHEXT: ".CMD" } };
470
471 expect((yield* resolveCommandPath("appeared", options).pipe(Effect.result))._tag).toBe(
472 "Failure",
473 );
474 yield* fs.writeFileString(executable, "@echo off\n");
475 expect((yield* resolveCommandPath("appeared", options).pipe(Effect.result))._tag).toBe(
476 "Failure",
477 );
478 expect(yield* resolveCommandPath(executable, options)).toBe(executable);
479 yield* TestClock.adjust("30 seconds");
480 expect(yield* resolveCommandPath("appeared", options)).toBe(executable);
481 }).pipe(
482 Effect.provideService(HostProcessPlatform, "win32"),
483 Effect.provideService(CommandResolutionCache, new Map()),
484 ),
485 );
486
487 it.effect("keeps upper and lowercase PATHEXT candidates", () =>
488 Effect.gen(function* () {
489 const probed = yield* recordProbes({
490 PATH: "C:\\bin",
491 PATHEXT: ".COM;.EXE;.BAT;.CMD",
492 });
493
494 expect(probed).toHaveLength(8);
495 expect(probed.filter((filePath) => /\.(COM|EXE|BAT|CMD)$/.test(filePath))).toHaveLength(4);
496 expect(probed.filter((filePath) => /\.(com|exe|bat|cmd)$/.test(filePath))).toHaveLength(4);
497 }),
498 );
378499});
379500
380501effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
packages/shared/src/shell.ts+8-4
......@@ -544,7 +544,8 @@ function cacheCommandResolution(
544544 });
545545}
546546
547const isExecutableFile = Effect.fn("shell.isExecutableFile")(function* (
547// Trace each command lookup, not every candidate file it probes.
548const isExecutableFile = Effect.fnUntraced(function* (
548549 filePath: string,
549550 platform: NodeJS.Platform,
550551 windowsPathExtensions: ReadonlyArray<string>,
......@@ -605,12 +606,15 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat
605606 return cached.resolvedPath;
606607 }
607608
609 // Keep case variants: Windows can make PATH directories case-sensitive.
608610 const pathEntries: string[] = [];
611 const seenPathEntries = new Set<string>();
609612 for (const entry of pathValue.split(pathDelimiterForPlatform(platform))) {
610613 const pathEntry = stripWrappingQuotes(entry.trim());
611 if (pathEntry.length > 0) {
612 pathEntries.push(pathEntry);
613 }
614 if (pathEntry.length === 0 || seenPathEntries.has(pathEntry)) continue;
615
616 seenPathEntries.add(pathEntry);
617 pathEntries.push(pathEntry);
614618 }
615619
616620 for (const pathEntry of pathEntries) {