Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 55 additions & 2 deletions packages/shared/src/shell.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
resolveSpawnCommand,
resolveWindowsEnvironment,
SpawnExecutableResolution,
type SpawnExecutableResolver,
WindowsShellEnvironment,
withPathDirectoryListings,
type WindowsShellEnvironmentReader,
Expand Down Expand Up @@ -515,7 +516,10 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
Effect.gen(function* () {
const command = yield* resolveSpawnCommand("node.exe", ["script.js", "hello & goodbye"], {
env: { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" },
}).pipe(Effect.provideService(HostProcessPlatform, "win32"));
}).pipe(
Effect.provideService(HostProcessPlatform, "win32"),
Effect.provideService(CommandResolutionCache, new Map()),
);

expect(command).toEqual({
command: "node.exe",
Expand All @@ -533,6 +537,7 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
{ env: { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" } },
).pipe(
Effect.provideService(HostProcessPlatform, "win32"),
Effect.provideService(CommandResolutionCache, new Map()),
Effect.provideService(
SpawnExecutableResolution,
() => "C:\\Program Files\\npm & tools\\vp.cmd",
Expand All @@ -559,6 +564,7 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
extendEnv: true,
}).pipe(
Effect.provideService(HostProcessPlatform, "win32"),
Effect.provideService(CommandResolutionCache, new Map()),
Effect.provideService(HostProcessEnvironment, {
PATH: "C:\\Users\\tester\\AppData\\Roaming\\npm",
PATHEXT: ".COM;.EXE;.BAT;.CMD",
Expand All @@ -577,11 +583,58 @@ effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => {
}),
);

it.effect("scans PATH once per command until the search environment changes", () =>
Effect.gen(function* () {
const scans: Array<string> = [];
const scan: SpawnExecutableResolver = (name, _platform, env) => {
scans.push(`${name}@${env.PATH}`);
return name === "missing" ? undefined : `${env.PATH}\\${name}.exe`;
};
const resolve = (command: string, path: string, resolver = scan) =>
resolveSpawnCommand(command, [], { env: { PATH: path, PATHEXT: ".EXE" } }).pipe(
Effect.provideService(HostProcessPlatform, "win32"),
Effect.provideService(SpawnExecutableResolution, resolver),
);

expect((yield* resolve("git", "C:\\one")).command).toBe("C:\\one\\git.exe");
expect((yield* resolve("git", "C:\\one")).command).toBe("C:\\one\\git.exe");
// A failed spawn is how a provider reports "not installed", so a miss
// must clear the moment the binary appears.
yield* resolve("missing", "C:\\one");
yield* resolve("missing", "C:\\one");
expect((yield* resolve("git", "C:\\two")).command).toBe("C:\\two\\git.exe");
// Callers probe explicit paths they may have just written.
yield* resolve("C:\\tools\\git.exe", "C:\\one");
yield* resolve("C:\\tools\\git.exe", "C:\\one");
expect(scans).toEqual([
"git@C:\\one",
"missing@C:\\one",
"missing@C:\\one",
"git@C:\\two",
"C:\\tools\\git.exe@C:\\one",
"C:\\tools\\git.exe@C:\\one",
]);

yield* TestClock.adjust("31 seconds");
yield* resolve("git", "C:\\one");
expect(scans).toHaveLength(7);

// Another resolver sharing the cache gets its own answer, not the cached one.
const elsewhere = yield* resolve("git", "C:\\one", () => "D:\\elsewhere\\git.exe");
expect(elsewhere.command).toBe("D:\\elsewhere\\git.exe");
expect((yield* resolve("git", "C:\\one")).command).toBe("C:\\one\\git.exe");
expect(scans).toHaveLength(7);
}).pipe(Effect.provideService(CommandResolutionCache, new Map())),
);

it.effect("does not fall back to a shell for unresolved Windows commands", () =>
Effect.gen(function* () {
const command = yield* resolveSpawnCommand("missing & calc", ["unsafe & value"], {
env: { PATH: "", PATHEXT: ".COM;.EXE;.BAT;.CMD" },
}).pipe(Effect.provideService(HostProcessPlatform, "win32"));
}).pipe(
Effect.provideService(HostProcessPlatform, "win32"),
Effect.provideService(CommandResolutionCache, new Map()),
);

expect(command).toEqual({
command: "missing & calc",
Expand Down
39 changes: 38 additions & 1 deletion packages/shared/src/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,18 @@ export const withPathDirectoryListings = <A, E, R>(effect: Effect.Effect<A, E, R
return yield* effect.pipe(Effect.provideService(PathDirectoryListings, listings));
});

// An injected resolver may answer differently for the same search, so its
// entries are kept apart from every other resolver's.
let spawnResolverCacheIdCount = 0;
const spawnResolverCacheIds = new WeakMap<SpawnExecutableResolver, number>();
function spawnResolverCacheId(resolver: SpawnExecutableResolver): number {
const known = spawnResolverCacheIds.get(resolver);
if (known !== undefined) return known;
const id = spawnResolverCacheIdCount++;
spawnResolverCacheIds.set(resolver, id);
return id;
}

function cacheCommandResolution(
cache: Map<string, CommandResolutionCacheEntry>,
cacheKey: string,
Expand Down Expand Up @@ -700,7 +712,32 @@ export const resolveSpawnCommand = Effect.fnUntraced(function* (
? { ...hostEnvironment, ...options.env }
: options.env;
const resolveExecutable = yield* SpawnExecutableResolution;
const resolvedCommand = resolveExecutable(command, platform, env) ?? command;
// The scan is synchronous and runs before every child process, so it shares
// the PATH scan cache above. Explicit paths stay uncached for the same reason,
// and so do misses: a failed spawn is how providers report "not installed",
// and that has to clear the moment the binary appears.
const explicitPath = command.includes("/") || command.includes("\\");
const cache = yield* CommandResolutionCache;
const cacheKey = [
"spawn",
String(spawnResolverCacheId(resolveExecutable)),
platform,
resolvePathEnvironmentVariable(env),
resolveWindowsPathExtensions(env).join(";"),
command,
].join(COMMAND_RESOLUTION_CACHE_KEY_SEPARATOR);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const nowNanos = yield* Clock.currentTimeNanos;
const cached = explicitPath ? undefined : cache.get(cacheKey);
let resolvedExecutable: string | null;
if (cached !== undefined && cached.expiresAtNanos > nowNanos) {
resolvedExecutable = cached.resolvedPath;
} else {
resolvedExecutable = resolveExecutable(command, platform, env) ?? null;
if (!explicitPath && resolvedExecutable !== null) {
cacheCommandResolution(cache, cacheKey, resolvedExecutable, nowNanos);
}
}
const resolvedCommand = resolvedExecutable ?? command;
const extension = NodePath.win32.extname(resolvedCommand).toLowerCase();
if (extension !== ".cmd" && extension !== ".bat") {
return { command: resolvedCommand, args: [...args], shell: false };
Expand Down
Loading