From 2d6100934d903c13fa2d3fc7992896dc8493940f Mon Sep 17 00:00:00 2001 From: Bob Fowler Date: Fri, 25 Sep 2026 14:49:07 -0400 Subject: [PATCH 1/4] fix(server): installed editors no longer vanish when discovery is slow Editor discovery stat-ed every PATH x PATHEXT candidate for every editor, sequentially. On Windows with a long PATH that took longer than server.getConfig's five-second bound, which then reported no editors at all, so the Open menu showed "No installed editors found". - List each PATH directory once per scan and only stat names the listing contains (withPathDirectoryListings). Unlistable directories fall back to direct probes; missing ones are skipped. - Probe editors concurrently under a four-second budget that returns what it found instead of discarding it. - Never cache an incomplete scan, and never let one hide an editor the last complete scan found. Fixes #4697. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/process/externalLauncher.test.ts | 99 ++++++++++++++++++- apps/server/src/process/externalLauncher.ts | 88 ++++++++++++----- packages/shared/src/shell.test.ts | 79 +++++++++++++++ packages/shared/src/shell.ts | 40 ++++++++ 4 files changed, 275 insertions(+), 31 deletions(-) diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index f714a70f783d..f4ccca551aec 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -842,10 +842,9 @@ it.effect.skipIf(windowsHost)( }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); -// The handler probe carries its own timeout because the editor scan's outer -// timeout in server.getConfig degrades to an EMPTY editor list: a wedged -// xdg-mime must cost only the file manager, never the other editors. Runs on -// the live clock so the probe's real timeout fires. +// The handler probe carries its own timeout so a wedged xdg-mime costs only +// the file manager and the scan still completes. Runs on the live clock so +// the probe's real timeout fires. it.live.skipIf(windowsHost)("a stalled handler probe drops only the file manager", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; @@ -1082,6 +1081,7 @@ it.effect("memoizes editor discovery and refreshes after the cache window", () = Layer.provide( Layer.mergeAll( FileSystem.layerNoop({ + readDirectory: () => Effect.succeed(["code.CMD"]), stat: () => Effect.sync(() => { statCalls += 1; @@ -1147,6 +1147,7 @@ it.effect("rescans after an interrupted discovery instead of caching the interru Layer.provide( Layer.mergeAll( FileSystem.layerNoop({ + readDirectory: () => Effect.succeed(["code.CMD"]), // The first scan parks inside `stat` so the interrupt lands while // discovery is in flight, which is what a client disconnecting // mid-connect does to the shared effect. @@ -1199,6 +1200,96 @@ it.effect("rescans after an interrupted discovery instead of caching the interru ); }); +// One Windows PATH directory listing `listed`. Every stat reports a file except +// the ones `stalls` matches, which never resolve, like a probe wedged on a +// slow disk or share. Each test passes its own PATH so the process-wide +// command-resolution cache cannot leak results between tests. +const stubbedWindowsDiscoveryLayer = (input: { + readonly pathDirectory: string; + readonly listed: ReadonlyArray; + readonly stalls: (filePath: string) => boolean; + readonly onStat?: () => void; +}) => + Layer.mergeAll( + ExternalLauncher.layer.pipe( + Layer.provide( + Layer.mergeAll( + FileSystem.layerNoop({ + readDirectory: () => Effect.succeed([...input.listed]), + stat: (filePath) => { + input.onStat?.(); + return input.stalls(filePath) + ? Effect.never + : Effect.succeed({ type: "File" } as FileSystem.File.Info); + }, + }), + Path.layer, + Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => Effect.sync(() => makeMockDetachedHandle())), + ), + ), + ), + ), + Layer.succeed(HostProcessPlatform, "win32"), + ConfigProvider.layer( + ConfigProvider.fromEnv({ env: { PATH: input.pathDirectory, PATHEXT: ".EXE;.CMD" } }), + ), + ); + +it.effect("returns the editors found when one probe outlives the scan budget", () => + Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + const fiber = yield* Effect.forkChild(launcher.resolveAvailableEditors()); + yield* TestClock.adjust("4 seconds"); + + // Trae stalls, but the editors on either side of it, including the file + // manager probed last, are still reported. + assert.deepEqual(yield* Fiber.join(fiber), ["cursor", "file-manager"]); + }).pipe( + Effect.provide( + stubbedWindowsDiscoveryLayer({ + pathDirectory: "C:\\t3-editor-scan-budget-test", + listed: ["cursor.EXE", "trae.EXE", "explorer.EXE"], + stalls: (filePath) => filePath.includes("trae"), + }), + ), + ), +); + +it.effect("keeps editors from the last complete scan when a rescan runs out of time", () => { + let stallVscode = false; + let stats = 0; + return Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + assert.deepEqual(yield* launcher.resolveAvailableEditors(), ["vscode", "file-manager"]); + + // Past the discovery cache window, the rescan stalls on VS Code. + yield* TestClock.adjust("61 seconds"); + stallVscode = true; + const fiber = yield* Effect.forkChild(launcher.resolveAvailableEditors()); + yield* TestClock.adjust("4 seconds"); + assert.deepEqual(yield* Fiber.join(fiber), ["vscode", "file-manager"]); + + // The incomplete scan was not cached, so the next call scans again. + stallVscode = false; + const statsBeforeRescan = stats; + assert.deepEqual(yield* launcher.resolveAvailableEditors(), ["vscode", "file-manager"]); + assert.isAbove(stats, statsBeforeRescan); + }).pipe( + Effect.provide( + stubbedWindowsDiscoveryLayer({ + pathDirectory: "C:\\t3-editor-incomplete-rescan-test", + listed: ["code.CMD", "explorer.EXE"], + stalls: (filePath) => stallVscode && filePath.includes("code"), + onStat: () => { + stats += 1; + }, + }), + ), + ); +}); + it.effect("rejects unknown editors through the service API", () => Effect.gen(function* () { const launcher = yield* ExternalLauncher.ExternalLauncher; diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index 29c25e790c61..17aecdf1bd64 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -20,7 +20,11 @@ import { } from "@t3tools/contracts"; import { resolveEditorCommand } from "@t3tools/shared/editor"; import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; -import { isCommandAvailable, resolveSpawnCommand } from "@t3tools/shared/shell"; +import { + isCommandAvailable, + resolveSpawnCommand, + withPathDirectoryListings, +} from "@t3tools/shared/shell"; import * as Clock from "effect/Clock"; import * as Config from "effect/Config"; import * as Context from "effect/Context"; @@ -248,10 +252,9 @@ function fileManagerCommandForPlatform( // so the client would see a silent no-op. Require the handler before // advertising the file manager on Linux. // -// The probe carries its own timeout well inside the scan timeout -// `server.getConfig` applies to editor discovery: that outer timeout degrades -// to an empty editor list, so a hung `xdg-mime` (broken D-Bus or desktop -// session) must cost only the file manager, not every discovered editor. +// The probe carries its own timeout well inside the editor scan budget, so a +// hung `xdg-mime` (broken D-Bus or desktop session) costs only the file +// manager and the scan still completes and gets cached. const LINUX_DIRECTORY_HANDLER_PROBE_TIMEOUT = "2 seconds"; const hasUsableLinuxDirectoryHandler = Effect.fn("externalLauncher.hasUsableLinuxDirectoryHandler")( @@ -404,31 +407,53 @@ function buildBrowserLaunch( }; } +// Stop probing before server.getConfig's five-second discovery bound, which +// would otherwise discard everything the scan had found. +const EDITOR_SCAN_BUDGET = "4 seconds"; + +interface EditorScan { + readonly editors: ReadonlyArray; + readonly complete: boolean; +} + +const isEditorAvailable = Effect.fn("externalLauncher.isEditorAvailable")(function* ( + editor: (typeof EDITORS)[number], + platform: NodeJS.Platform, + env: NodeJS.ProcessEnv, +) { + if (editor.commands === null) { + return (yield* resolveUsableFileManagerCommand(platform, env)) !== undefined; + } + return Option.isSome(yield* resolveEditorCommand(editor, env)); +}); + +// Editors are probed concurrently, so one stalled lookup cannot hide the rest, +// and share PATH directory listings, so a long Windows PATH costs one listing +// per directory rather than a stat per PATH x PATHEXT candidate per editor. const buildAvailableEditors = Effect.fn("externalLauncher.buildAvailableEditors")(function* ( platform: NodeJS.Platform, env: NodeJS.ProcessEnv, ): Effect.fn.Return< - ReadonlyArray, + EditorScan, never, FileSystem.FileSystem | Path.Path | ChildProcessSpawner.ChildProcessSpawner > { - const available: EditorId[] = []; - - for (const editor of EDITORS) { - if (editor.commands === null) { - if ((yield* resolveUsableFileManagerCommand(platform, env)) !== undefined) { - available.push(editor.id); - } - continue; - } - - const command = yield* resolveEditorCommand(editor, env); - if (Option.isSome(command)) { - available.push(editor.id); - } - } + const found = new Set(); + const finished = yield* Effect.forEach( + EDITORS, + (editor) => + isEditorAvailable(editor, platform, env).pipe( + Effect.map((available) => { + if (available) found.add(editor.id); + }), + ), + { concurrency: "unbounded", discard: true }, + ).pipe(withPathDirectoryListings, Effect.timeoutOption(EDITOR_SCAN_BUDGET)); - return available; + return { + editors: EDITORS.flatMap((editor) => (found.has(editor.id) ? [editor.id] : [])), + complete: Option.isSome(finished), + }; }); const resolveBrowserLaunch = Effect.fn("externalLauncher.resolveBrowserLaunch")(function* ( @@ -463,8 +488,10 @@ const resolveFileManagerRevealKind = Effect.fn("externalLauncher.resolveFileMana // on the connection fiber under a timeout (`resolveAvailableEditorsForConfig`), // so one client disconnecting mid-scan would cache the interrupt and replay it // to every later connect for the whole TTL, breaking `server.getConfig` -// permanently. Storing only on success means an interrupted scan leaves the -// cache untouched and the next connect simply rescans. +// permanently. Storing only complete scans means an interrupted or +// out-of-budget scan leaves the cache untouched and the next connect simply +// rescans. An incomplete scan still never hides an editor the last complete +// scan found: running out of time is not evidence that it was uninstalled. // Expiry uses the monotonic clock (Clock.currentTimeNanos), matching the // command-resolution cache in @t3tools/shared/shell, so a backward wall-clock // adjustment cannot keep an expired entry alive. @@ -765,17 +792,24 @@ export const make = Effect.gen(function* () { if (Option.isSome(entry) && entry.value.expiresAtNanos > nowNanos) { return entry.value.editors; } - const editors = yield* provideCommandResolutionServices(resolveAvailableEditors()).pipe( + const scan = yield* provideCommandResolutionServices(resolveAvailableEditors()).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), ); + if (!scan.complete) { + const kept = new Set([ + ...scan.editors, + ...Option.match(entry, { onNone: () => [], onSome: ({ editors }) => editors }), + ]); + return EDITORS.flatMap((editor) => (kept.has(editor.id) ? [editor.id] : [])); + } yield* Ref.set( editorDiscoveryCache, Option.some({ - editors, + editors: scan.editors, expiresAtNanos: nowNanos + EDITOR_DISCOVERY_CACHE_TTL_NANOS, }), ); - return editors; + return scan.editors; }); return ExternalLauncher.of({ diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index 621fe49b3087..6755f78b9915 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -4,6 +4,7 @@ import { HostProcessEnvironment, HostProcessPlatform } from "@t3tools/shared/hos import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; +import * as PlatformError from "effect/PlatformError"; import * as TestClock from "effect/testing/TestClock"; import { describe, expect, it, vi } from "vite-plus/test"; @@ -25,6 +26,7 @@ import { resolveWindowsEnvironment, SpawnExecutableResolution, WindowsShellEnvironment, + withPathDirectoryListings, type WindowsShellEnvironmentReader, } from "./shell.ts"; @@ -473,6 +475,83 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => { expect(probed.filter((filePath) => /\.(com|exe|bat|cmd)$/.test(filePath))).toHaveLength(4); }), ); + + it.effect("lists each PATH directory once per batch and stats only listed names", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const first = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); + const second = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); + const missing = path.join(first, "missing"); + yield* fs.writeFileString(path.join(first, "cursor.CMD"), ""); + yield* fs.writeFileString(path.join(second, "cursor.EXE"), ""); + yield* fs.writeFileString(path.join(second, "explorer.EXE"), ""); + const env = { PATH: [missing, first, second].join(";"), PATHEXT: ".EXE;.CMD" }; + const listed: Array = []; + const statted: Array = []; + + yield* Effect.gen(function* () { + // PATH order still wins over PATHEXT order. + expect(yield* resolveCommandPath("cursor", { env })).toBe(path.join(first, "cursor.CMD")); + expect(yield* resolveCommandPath("explorer", { env })).toBe( + path.join(second, "explorer.EXE"), + ); + expect(yield* isCommandAvailable("absent", { env })).toBe(false); + }).pipe( + withPathDirectoryListings, + Effect.provideService(FileSystem.FileSystem, { + ...fs, + readDirectory: (directory) => { + listed.push(directory); + return fs.readDirectory(directory); + }, + stat: (filePath) => { + statted.push(filePath); + return fs.stat(filePath); + }, + }), + ); + + expect(listed).toEqual([missing, first, second]); + expect(statted).toEqual([path.join(first, "cursor.CMD"), path.join(second, "explorer.EXE")]); + }).pipe( + Effect.provideService(HostProcessPlatform, "win32"), + Effect.provideService(CommandResolutionCache, new Map()), + ), + ); + + it.effect("probes candidates directly when a PATH directory cannot be listed", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); + const executable = path.join(directory, "editor.CMD"); + yield* fs.writeFileString(executable, ""); + + const resolved = yield* resolveCommandPath("editor", { + env: { PATH: directory, PATHEXT: ".CMD" }, + }).pipe( + withPathDirectoryListings, + Effect.provideService(FileSystem.FileSystem, { + ...fs, + readDirectory: (directory) => + Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "readDirectory", + pathOrDescriptor: directory, + }), + ), + }), + ); + + expect(resolved).toBe(executable); + }).pipe( + Effect.provideService(HostProcessPlatform, "win32"), + Effect.provideService(CommandResolutionCache, new Map()), + ), + ); }); effectIt.layer(NodeServices.layer)("resolveSpawnCommand", (it) => { diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 11a45907cc1d..925eff3d12f6 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -3,6 +3,7 @@ import * as NodeOS from "node:os"; import * as NodePath from "node:path"; import * as NodeChildProcess from "node:child_process"; import * as NodeFS from "node:fs"; +import * as Cache from "effect/Cache"; import * as Clock from "effect/Clock"; import * as Data from "effect/Data"; import * as Effect from "effect/Effect"; @@ -511,6 +512,40 @@ export const CommandResolutionCache = Context.Reference + FileSystem.FileSystem.use((fileSystem) => fileSystem.readDirectory(directory)).pipe( + Effect.map( + (entries): ReadonlySet | undefined => + new Set(entries.map((entry) => entry.toLowerCase())), + ), + Effect.catch((error) => + Effect.succeed(error.reason._tag === "NotFound" ? new Set() : undefined), + ), + ); + +const PathDirectoryListings = Context.Reference< + Cache.Cache | undefined, never, FileSystem.FileSystem> | undefined +>("@t3tools/shared/shell/PathDirectoryListings", { defaultValue: () => undefined }); + +/** + * Run a batch of command lookups (such as editor discovery) that lists each + * PATH directory once and only probes names the listing contains, instead of + * stat-ing every PATH x PATHEXT candidate per command. A single lookup is + * cheaper without it. Each run lists afresh, so later batches see new installs. + */ +export const withPathDirectoryListings = (effect: Effect.Effect) => + Effect.gen(function* () { + const listings = yield* Cache.make({ + capacity: 1024, + lookup: listPathDirectory, + requireServicesAt: "lookup", + }); + return yield* effect.pipe(Effect.provideService(PathDirectoryListings, listings)); + }); + function cacheCommandResolution( cache: Map, cacheKey: string, @@ -602,8 +637,13 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat pathEntries.push(pathEntry); } + const listings = yield* PathDirectoryListings; for (const pathEntry of pathEntries) { + const names = listings === undefined ? undefined : yield* Cache.get(listings, pathEntry); for (const candidate of commandCandidates) { + // Listings are lowercased; the stat below still checks the exact + // spelling for case-sensitive directories and rejects non-files. + if (names !== undefined && !names.has(candidate.toLowerCase())) continue; const candidatePath = path.join(pathEntry, candidate); if (yield* isExecutableFile(candidatePath, platform, windowsPathExtensions)) { cacheCommandResolution(cache, cacheKey, candidatePath, nowNanos); From 491a52f69567cbf9db4f7e29939e16341d75da6b Mon Sep 17 00:00:00 2001 From: Bob Fowler Date: Fri, 25 Sep 2026 16:43:46 -0400 Subject: [PATCH 2/4] docs(shared): document the per-scan PATH listing snapshot Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/shared/src/shell.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 925eff3d12f6..651ef3fc734c 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -534,7 +534,10 @@ const PathDirectoryListings = Context.Reference< * Run a batch of command lookups (such as editor discovery) that lists each * PATH directory once and only probes names the listing contains, instead of * stat-ing every PATH x PATHEXT candidate per command. A single lookup is - * cheaper without it. Each run lists afresh, so later batches see new installs. + * cheaper without it. Listings are a snapshot for the run: a command installed + * into a directory after the run listed it is missed until the next run, and + * that miss is cached like any other. Accepted for the tens of thousands of + * stats this saves on long Windows PATHs. */ export const withPathDirectoryListings = (effect: Effect.Effect) => Effect.gen(function* () { From dce528d399b95b0c00c24ef909e648c085593f71 Mon Sep 17 00:00:00 2001 From: Bob Fowler Date: Fri, 25 Sep 2026 17:01:53 -0400 Subject: [PATCH 3/4] fix(shared): relist PATH directories that change during a batch Before reusing a listing, compare the directory's mtime with the one recorded when it was listed, and relist it if it changed. A command installed mid-scan is now found exactly as a direct probe would find it, at the cost of one directory stat per lookup instead of one per PATHEXT candidate (~1,600 stats and 275-437 ms on a 73-entry Windows PATH, vs ~34,000 stats and 7-11 s before this PR). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/process/externalLauncher.test.ts | 19 ++++-- packages/shared/src/shell.test.ts | 32 +++++++++- packages/shared/src/shell.ts | 64 +++++++++++++------ 3 files changed, 88 insertions(+), 27 deletions(-) diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index f4ccca551aec..3a7ff41feda5 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -6,10 +6,12 @@ import * as NodePath from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import * as ConfigProvider from "effect/ConfigProvider"; +import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; +import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; @@ -1074,9 +1076,15 @@ it.effect.skipIf(windowsHost)("ignores unusable app bundles and keeps PATH launc }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); +// Stubbed stats answer for PATH directories too, and discovery checks a +// directory's mtime before reusing its listing, so the stub carries one. +const stubFileInfo = { + type: "File", + mtime: Option.some(DateTime.toDateUtc(DateTime.makeUnsafe(0))), +} as FileSystem.File.Info; + it.effect("memoizes editor discovery and refreshes after the cache window", () => { let statCalls = 0; - const fileInfo = { type: "File" } as FileSystem.File.Info; const launcherLayer = ExternalLauncher.layer.pipe( Layer.provide( Layer.mergeAll( @@ -1085,7 +1093,7 @@ it.effect("memoizes editor discovery and refreshes after the cache window", () = stat: () => Effect.sync(() => { statCalls += 1; - return fileInfo; + return stubFileInfo; }), }), Path.layer, @@ -1140,7 +1148,6 @@ it.effect("memoizes editor discovery and refreshes after the cache window", () = // replayed it to every later connect for the whole TTL, so `server.getConfig` // failed and no client could reconnect until the server restarted. it.effect("rescans after an interrupted discovery instead of caching the interrupt", () => { - const fileInfo = { type: "File" } as FileSystem.File.Info; let blockFirstScan = true; let scans = 0; const launcherLayer = ExternalLauncher.layer.pipe( @@ -1157,7 +1164,7 @@ it.effect("rescans after an interrupted discovery instead of caching the interru if (blockFirstScan) { return yield* Effect.never; } - return fileInfo; + return stubFileInfo; }), }), Path.layer, @@ -1218,9 +1225,7 @@ const stubbedWindowsDiscoveryLayer = (input: { readDirectory: () => Effect.succeed([...input.listed]), stat: (filePath) => { input.onStat?.(); - return input.stalls(filePath) - ? Effect.never - : Effect.succeed({ type: "File" } as FileSystem.File.Info); + return input.stalls(filePath) ? Effect.never : Effect.succeed(stubFileInfo); }, }), Path.layer, diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index 6755f78b9915..f43c8b9c7d7e 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -512,8 +512,36 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => { }), ); - expect(listed).toEqual([missing, first, second]); - expect(statted).toEqual([path.join(first, "cursor.CMD"), path.join(second, "explorer.EXE")]); + // The missing directory is dated but never listed; directory mtime checks + // aside, only names present in a listing are probed. + expect(listed).toEqual([first, second]); + expect(statted.filter((filePath) => ![missing, first, second].includes(filePath))).toEqual([ + path.join(first, "cursor.CMD"), + path.join(second, "explorer.EXE"), + ]); + }).pipe( + Effect.provideService(HostProcessPlatform, "win32"), + Effect.provideService(CommandResolutionCache, new Map()), + ), + ); + + it.effect("relists a PATH directory that changed during the batch", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); + const env = { PATH: directory, PATHEXT: ".EXE" }; + + yield* Effect.gen(function* () { + expect(yield* isCommandAvailable("absent", { env })).toBe(false); + yield* fs.writeFileString(path.join(directory, "installed.EXE"), ""); + // mtime has millisecond precision; move it clearly past the listing + // (numeric times are seconds; this is 2100-01-01). + yield* fs.utimes(directory, 4_102_444_800, 4_102_444_800); + expect(yield* resolveCommandPath("installed", { env })).toBe( + path.join(directory, "installed.EXE"), + ); + }).pipe(withPathDirectoryListings); }).pipe( Effect.provideService(HostProcessPlatform, "win32"), Effect.provideService(CommandResolutionCache, new Map()), diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 651ef3fc734c..8a6f9c97e16e 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -8,6 +8,7 @@ import * as Clock from "effect/Clock"; import * as Data from "effect/Data"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; +import * as Option from "effect/Option"; import * as Path from "effect/Path"; import { HostProcessEnvironment, HostProcessPlatform } from "./hostProcess.ts"; @@ -512,32 +513,58 @@ export const CommandResolutionCache = Context.Reference - FileSystem.FileSystem.use((fileSystem) => fileSystem.readDirectory(directory)).pipe( - Effect.map( - (entries): ReadonlySet | undefined => - new Set(entries.map((entry) => entry.toLowerCase())), - ), - Effect.catch((error) => - Effect.succeed(error.reason._tag === "NotFound" ? new Set() : undefined), - ), +interface PathDirectoryListing { + readonly modified: number | null | undefined; + /** Lowercased entry names, or undefined to probe candidates directly. */ + readonly names: ReadonlySet | undefined; +} + +// The directory's mtime, null when it does not exist, or undefined when it +// cannot be read or dated (permissions, a busy share). +const pathDirectoryModified = (directory: string) => + FileSystem.FileSystem.use((fileSystem) => fileSystem.stat(directory)).pipe( + Effect.map((info) => Option.getOrUndefined(info.mtime)?.getTime()), + Effect.catch((error) => Effect.succeed(error.reason._tag === "NotFound" ? null : undefined)), ); +// Dated before it is read, so an entry added in between shows up as a changed +// mtime on the next check instead of hiding behind a matching one. A missing +// directory holds nothing; one that cannot be dated or listed yields undefined +// names so lookups probe its candidates directly. +const listPathDirectory = (directory: string) => + Effect.gen(function* (): Effect.fn.Return { + const modified = yield* pathDirectoryModified(directory); + if (modified === null) return { modified, names: new Set() }; + if (modified === undefined) return { modified, names: undefined }; + const entries = yield* FileSystem.FileSystem.use((fileSystem) => + fileSystem.readDirectory(directory), + ).pipe(Effect.orElseSucceed(() => undefined)); + return { modified, names: entries && new Set(entries.map((entry) => entry.toLowerCase())) }; + }); + const PathDirectoryListings = Context.Reference< - Cache.Cache | undefined, never, FileSystem.FileSystem> | undefined + Cache.Cache | undefined >("@t3tools/shared/shell/PathDirectoryListings", { defaultValue: () => undefined }); +// Every use re-checks the directory's mtime and relists it if it changed, so a +// command installed mid-batch is found exactly as a direct probe would find it. +// One directory stat per lookup still replaces a stat per PATHEXT candidate. +const readPathDirectoryNames = Effect.fn("shell.readPathDirectoryNames")(function* ( + listings: Cache.Cache, + directory: string, +) { + const listing = yield* Cache.get(listings, directory); + if (listing.names === undefined) return undefined; + if ((yield* pathDirectoryModified(directory)) === listing.modified) return listing.names; + yield* Cache.invalidate(listings, directory); + return (yield* Cache.get(listings, directory)).names; +}); + /** * Run a batch of command lookups (such as editor discovery) that lists each * PATH directory once and only probes names the listing contains, instead of * stat-ing every PATH x PATHEXT candidate per command. A single lookup is - * cheaper without it. Listings are a snapshot for the run: a command installed - * into a directory after the run listed it is missed until the next run, and - * that miss is cached like any other. Accepted for the tens of thousands of - * stats this saves on long Windows PATHs. + * cheaper without it. */ export const withPathDirectoryListings = (effect: Effect.Effect) => Effect.gen(function* () { @@ -642,7 +669,8 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat const listings = yield* PathDirectoryListings; for (const pathEntry of pathEntries) { - const names = listings === undefined ? undefined : yield* Cache.get(listings, pathEntry); + const names = + listings === undefined ? undefined : yield* readPathDirectoryNames(listings, pathEntry); for (const candidate of commandCandidates) { // Listings are lowercased; the stat below still checks the exact // spelling for case-sensitive directories and rejects non-files. From 614f15ea13509f5565e24a6b4564f67895e3a0a8 Mon Sep 17 00:00:00 2001 From: Bob Fowler Date: Fri, 25 Sep 2026 17:56:07 -0400 Subject: [PATCH 4/4] fix(shared): narrow the editor discovery fix to PATH listings Keep only the change that fixes #4697: list each PATH directory once per scan, relisting it when its mtime changes, and probe only listed names. That alone takes discovery on a 73-entry Windows PATH from 6-11 s to ~250 ms, so concurrent probes, the partial-result budget, and keeping the last complete list are dropped here; they only matter for a probe that hangs outright and can follow separately. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/process/externalLauncher.test.ts | 112 ++---------------- apps/server/src/process/externalLauncher.ts | 84 +++++-------- packages/shared/src/shell.test.ts | 98 ++------------- packages/shared/src/shell.ts | 69 +++++------ 4 files changed, 75 insertions(+), 288 deletions(-) diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 3a7ff41feda5..f714a70f783d 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -6,12 +6,10 @@ import * as NodePath from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import * as ConfigProvider from "effect/ConfigProvider"; -import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; -import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Sink from "effect/Sink"; import * as Stream from "effect/Stream"; @@ -844,9 +842,10 @@ it.effect.skipIf(windowsHost)( }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); -// The handler probe carries its own timeout so a wedged xdg-mime costs only -// the file manager and the scan still completes. Runs on the live clock so -// the probe's real timeout fires. +// The handler probe carries its own timeout because the editor scan's outer +// timeout in server.getConfig degrades to an EMPTY editor list: a wedged +// xdg-mime must cost only the file manager, never the other editors. Runs on +// the live clock so the probe's real timeout fires. it.live.skipIf(windowsHost)("a stalled handler probe drops only the file manager", () => Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; @@ -1076,24 +1075,17 @@ it.effect.skipIf(windowsHost)("ignores unusable app bundles and keeps PATH launc }).pipe(Effect.scoped, Effect.provide(NodeServices.layer)), ); -// Stubbed stats answer for PATH directories too, and discovery checks a -// directory's mtime before reusing its listing, so the stub carries one. -const stubFileInfo = { - type: "File", - mtime: Option.some(DateTime.toDateUtc(DateTime.makeUnsafe(0))), -} as FileSystem.File.Info; - it.effect("memoizes editor discovery and refreshes after the cache window", () => { let statCalls = 0; + const fileInfo = { type: "File" } as FileSystem.File.Info; const launcherLayer = ExternalLauncher.layer.pipe( Layer.provide( Layer.mergeAll( FileSystem.layerNoop({ - readDirectory: () => Effect.succeed(["code.CMD"]), stat: () => Effect.sync(() => { statCalls += 1; - return stubFileInfo; + return fileInfo; }), }), Path.layer, @@ -1148,13 +1140,13 @@ it.effect("memoizes editor discovery and refreshes after the cache window", () = // replayed it to every later connect for the whole TTL, so `server.getConfig` // failed and no client could reconnect until the server restarted. it.effect("rescans after an interrupted discovery instead of caching the interrupt", () => { + const fileInfo = { type: "File" } as FileSystem.File.Info; let blockFirstScan = true; let scans = 0; const launcherLayer = ExternalLauncher.layer.pipe( Layer.provide( Layer.mergeAll( FileSystem.layerNoop({ - readDirectory: () => Effect.succeed(["code.CMD"]), // The first scan parks inside `stat` so the interrupt lands while // discovery is in flight, which is what a client disconnecting // mid-connect does to the shared effect. @@ -1164,7 +1156,7 @@ it.effect("rescans after an interrupted discovery instead of caching the interru if (blockFirstScan) { return yield* Effect.never; } - return stubFileInfo; + return fileInfo; }), }), Path.layer, @@ -1207,94 +1199,6 @@ it.effect("rescans after an interrupted discovery instead of caching the interru ); }); -// One Windows PATH directory listing `listed`. Every stat reports a file except -// the ones `stalls` matches, which never resolve, like a probe wedged on a -// slow disk or share. Each test passes its own PATH so the process-wide -// command-resolution cache cannot leak results between tests. -const stubbedWindowsDiscoveryLayer = (input: { - readonly pathDirectory: string; - readonly listed: ReadonlyArray; - readonly stalls: (filePath: string) => boolean; - readonly onStat?: () => void; -}) => - Layer.mergeAll( - ExternalLauncher.layer.pipe( - Layer.provide( - Layer.mergeAll( - FileSystem.layerNoop({ - readDirectory: () => Effect.succeed([...input.listed]), - stat: (filePath) => { - input.onStat?.(); - return input.stalls(filePath) ? Effect.never : Effect.succeed(stubFileInfo); - }, - }), - Path.layer, - Layer.succeed( - ChildProcessSpawner.ChildProcessSpawner, - ChildProcessSpawner.make(() => Effect.sync(() => makeMockDetachedHandle())), - ), - ), - ), - ), - Layer.succeed(HostProcessPlatform, "win32"), - ConfigProvider.layer( - ConfigProvider.fromEnv({ env: { PATH: input.pathDirectory, PATHEXT: ".EXE;.CMD" } }), - ), - ); - -it.effect("returns the editors found when one probe outlives the scan budget", () => - Effect.gen(function* () { - const launcher = yield* ExternalLauncher.ExternalLauncher; - const fiber = yield* Effect.forkChild(launcher.resolveAvailableEditors()); - yield* TestClock.adjust("4 seconds"); - - // Trae stalls, but the editors on either side of it, including the file - // manager probed last, are still reported. - assert.deepEqual(yield* Fiber.join(fiber), ["cursor", "file-manager"]); - }).pipe( - Effect.provide( - stubbedWindowsDiscoveryLayer({ - pathDirectory: "C:\\t3-editor-scan-budget-test", - listed: ["cursor.EXE", "trae.EXE", "explorer.EXE"], - stalls: (filePath) => filePath.includes("trae"), - }), - ), - ), -); - -it.effect("keeps editors from the last complete scan when a rescan runs out of time", () => { - let stallVscode = false; - let stats = 0; - return Effect.gen(function* () { - const launcher = yield* ExternalLauncher.ExternalLauncher; - assert.deepEqual(yield* launcher.resolveAvailableEditors(), ["vscode", "file-manager"]); - - // Past the discovery cache window, the rescan stalls on VS Code. - yield* TestClock.adjust("61 seconds"); - stallVscode = true; - const fiber = yield* Effect.forkChild(launcher.resolveAvailableEditors()); - yield* TestClock.adjust("4 seconds"); - assert.deepEqual(yield* Fiber.join(fiber), ["vscode", "file-manager"]); - - // The incomplete scan was not cached, so the next call scans again. - stallVscode = false; - const statsBeforeRescan = stats; - assert.deepEqual(yield* launcher.resolveAvailableEditors(), ["vscode", "file-manager"]); - assert.isAbove(stats, statsBeforeRescan); - }).pipe( - Effect.provide( - stubbedWindowsDiscoveryLayer({ - pathDirectory: "C:\\t3-editor-incomplete-rescan-test", - listed: ["code.CMD", "explorer.EXE"], - stalls: (filePath) => stallVscode && filePath.includes("code"), - onStat: () => { - stats += 1; - }, - }), - ), - ); -}); - it.effect("rejects unknown editors through the service API", () => Effect.gen(function* () { const launcher = yield* ExternalLauncher.ExternalLauncher; diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index 17aecdf1bd64..ab287e4d78ee 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -252,9 +252,10 @@ function fileManagerCommandForPlatform( // so the client would see a silent no-op. Require the handler before // advertising the file manager on Linux. // -// The probe carries its own timeout well inside the editor scan budget, so a -// hung `xdg-mime` (broken D-Bus or desktop session) costs only the file -// manager and the scan still completes and gets cached. +// The probe carries its own timeout well inside the scan timeout +// `server.getConfig` applies to editor discovery: that outer timeout degrades +// to an empty editor list, so a hung `xdg-mime` (broken D-Bus or desktop +// session) must cost only the file manager, not every discovered editor. const LINUX_DIRECTORY_HANDLER_PROBE_TIMEOUT = "2 seconds"; const hasUsableLinuxDirectoryHandler = Effect.fn("externalLauncher.hasUsableLinuxDirectoryHandler")( @@ -407,53 +408,31 @@ function buildBrowserLaunch( }; } -// Stop probing before server.getConfig's five-second discovery bound, which -// would otherwise discard everything the scan had found. -const EDITOR_SCAN_BUDGET = "4 seconds"; - -interface EditorScan { - readonly editors: ReadonlyArray; - readonly complete: boolean; -} - -const isEditorAvailable = Effect.fn("externalLauncher.isEditorAvailable")(function* ( - editor: (typeof EDITORS)[number], - platform: NodeJS.Platform, - env: NodeJS.ProcessEnv, -) { - if (editor.commands === null) { - return (yield* resolveUsableFileManagerCommand(platform, env)) !== undefined; - } - return Option.isSome(yield* resolveEditorCommand(editor, env)); -}); - -// Editors are probed concurrently, so one stalled lookup cannot hide the rest, -// and share PATH directory listings, so a long Windows PATH costs one listing -// per directory rather than a stat per PATH x PATHEXT candidate per editor. const buildAvailableEditors = Effect.fn("externalLauncher.buildAvailableEditors")(function* ( platform: NodeJS.Platform, env: NodeJS.ProcessEnv, ): Effect.fn.Return< - EditorScan, + ReadonlyArray, never, FileSystem.FileSystem | Path.Path | ChildProcessSpawner.ChildProcessSpawner > { - const found = new Set(); - const finished = yield* Effect.forEach( - EDITORS, - (editor) => - isEditorAvailable(editor, platform, env).pipe( - Effect.map((available) => { - if (available) found.add(editor.id); - }), - ), - { concurrency: "unbounded", discard: true }, - ).pipe(withPathDirectoryListings, Effect.timeoutOption(EDITOR_SCAN_BUDGET)); + const available: EditorId[] = []; - return { - editors: EDITORS.flatMap((editor) => (found.has(editor.id) ? [editor.id] : [])), - complete: Option.isSome(finished), - }; + for (const editor of EDITORS) { + if (editor.commands === null) { + if ((yield* resolveUsableFileManagerCommand(platform, env)) !== undefined) { + available.push(editor.id); + } + continue; + } + + const command = yield* resolveEditorCommand(editor, env); + if (Option.isSome(command)) { + available.push(editor.id); + } + } + + return available; }); const resolveBrowserLaunch = Effect.fn("externalLauncher.resolveBrowserLaunch")(function* ( @@ -467,7 +446,7 @@ const resolveBrowserLaunch = Effect.fn("externalLauncher.resolveBrowserLaunch")( const resolveAvailableEditors = Effect.fn("externalLauncher.resolveAvailableEditors")(function* () { const platform = yield* HostProcessPlatform; const env = { ...(yield* readBrowserLaunchEnv), ...(yield* readCommandLookupEnv) }; - return yield* buildAvailableEditors(platform, env); + return yield* buildAvailableEditors(platform, env).pipe(withPathDirectoryListings); }); const resolveFileManagerRevealKind = Effect.fn("externalLauncher.resolveFileManagerRevealKind")( @@ -488,10 +467,8 @@ const resolveFileManagerRevealKind = Effect.fn("externalLauncher.resolveFileMana // on the connection fiber under a timeout (`resolveAvailableEditorsForConfig`), // so one client disconnecting mid-scan would cache the interrupt and replay it // to every later connect for the whole TTL, breaking `server.getConfig` -// permanently. Storing only complete scans means an interrupted or -// out-of-budget scan leaves the cache untouched and the next connect simply -// rescans. An incomplete scan still never hides an editor the last complete -// scan found: running out of time is not evidence that it was uninstalled. +// permanently. Storing only on success means an interrupted scan leaves the +// cache untouched and the next connect simply rescans. // Expiry uses the monotonic clock (Clock.currentTimeNanos), matching the // command-resolution cache in @t3tools/shared/shell, so a backward wall-clock // adjustment cannot keep an expired entry alive. @@ -792,24 +769,17 @@ export const make = Effect.gen(function* () { if (Option.isSome(entry) && entry.value.expiresAtNanos > nowNanos) { return entry.value.editors; } - const scan = yield* provideCommandResolutionServices(resolveAvailableEditors()).pipe( + const editors = yield* provideCommandResolutionServices(resolveAvailableEditors()).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), ); - if (!scan.complete) { - const kept = new Set([ - ...scan.editors, - ...Option.match(entry, { onNone: () => [], onSome: ({ editors }) => editors }), - ]); - return EDITORS.flatMap((editor) => (kept.has(editor.id) ? [editor.id] : [])); - } yield* Ref.set( editorDiscoveryCache, Option.some({ - editors: scan.editors, + editors, expiresAtNanos: nowNanos + EDITOR_DISCOVERY_CACHE_TTL_NANOS, }), ); - return scan.editors; + return editors; }); return ExternalLauncher.of({ diff --git a/packages/shared/src/shell.test.ts b/packages/shared/src/shell.test.ts index f43c8b9c7d7e..6c89e49d017f 100644 --- a/packages/shared/src/shell.test.ts +++ b/packages/shared/src/shell.test.ts @@ -4,7 +4,6 @@ import { HostProcessEnvironment, HostProcessPlatform } from "@t3tools/shared/hos import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; -import * as PlatformError from "effect/PlatformError"; import * as TestClock from "effect/testing/TestClock"; import { describe, expect, it, vi } from "vite-plus/test"; @@ -476,105 +475,34 @@ effectIt.layer(NodeServices.layer)("resolveCommandPath", (it) => { }), ); - it.effect("lists each PATH directory once per batch and stats only listed names", () => + it.effect("probes only listed PATH names and relists a directory that changes", () => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; - const first = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); - const second = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); - const missing = path.join(first, "missing"); + const first = yield* fs.makeTempDirectoryScoped(); + const second = yield* fs.makeTempDirectoryScoped(); yield* fs.writeFileString(path.join(first, "cursor.CMD"), ""); yield* fs.writeFileString(path.join(second, "cursor.EXE"), ""); - yield* fs.writeFileString(path.join(second, "explorer.EXE"), ""); - const env = { PATH: [missing, first, second].join(";"), PATHEXT: ".EXE;.CMD" }; - const listed: Array = []; - const statted: Array = []; - + const env = { PATH: `${first};${second}`, PATHEXT: ".EXE;.CMD" }; + const probed: Array = []; yield* Effect.gen(function* () { - // PATH order still wins over PATHEXT order. expect(yield* resolveCommandPath("cursor", { env })).toBe(path.join(first, "cursor.CMD")); - expect(yield* resolveCommandPath("explorer", { env })).toBe( - path.join(second, "explorer.EXE"), - ); expect(yield* isCommandAvailable("absent", { env })).toBe(false); + yield* fs.writeFileString(path.join(second, "late.EXE"), ""); + yield* fs.utimes(second, 4_102_444_800, 4_102_444_800); // seconds: 2100-01-01 + expect(yield* resolveCommandPath("late", { env })).toBe(path.join(second, "late.EXE")); }).pipe( withPathDirectoryListings, Effect.provideService(FileSystem.FileSystem, { ...fs, - readDirectory: (directory) => { - listed.push(directory); - return fs.readDirectory(directory); - }, - stat: (filePath) => { - statted.push(filePath); - return fs.stat(filePath); + stat: (file) => { + // Record candidate probes, not the per-lookup directory mtime checks. + if (file !== first && file !== second) probed.push(file); + return fs.stat(file); }, }), ); - - // The missing directory is dated but never listed; directory mtime checks - // aside, only names present in a listing are probed. - expect(listed).toEqual([first, second]); - expect(statted.filter((filePath) => ![missing, first, second].includes(filePath))).toEqual([ - path.join(first, "cursor.CMD"), - path.join(second, "explorer.EXE"), - ]); - }).pipe( - Effect.provideService(HostProcessPlatform, "win32"), - Effect.provideService(CommandResolutionCache, new Map()), - ), - ); - - it.effect("relists a PATH directory that changed during the batch", () => - Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); - const env = { PATH: directory, PATHEXT: ".EXE" }; - - yield* Effect.gen(function* () { - expect(yield* isCommandAvailable("absent", { env })).toBe(false); - yield* fs.writeFileString(path.join(directory, "installed.EXE"), ""); - // mtime has millisecond precision; move it clearly past the listing - // (numeric times are seconds; this is 2100-01-01). - yield* fs.utimes(directory, 4_102_444_800, 4_102_444_800); - expect(yield* resolveCommandPath("installed", { env })).toBe( - path.join(directory, "installed.EXE"), - ); - }).pipe(withPathDirectoryListings); - }).pipe( - Effect.provideService(HostProcessPlatform, "win32"), - Effect.provideService(CommandResolutionCache, new Map()), - ), - ); - - it.effect("probes candidates directly when a PATH directory cannot be listed", () => - Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; - const path = yield* Path.Path; - const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-path-listing-" }); - const executable = path.join(directory, "editor.CMD"); - yield* fs.writeFileString(executable, ""); - - const resolved = yield* resolveCommandPath("editor", { - env: { PATH: directory, PATHEXT: ".CMD" }, - }).pipe( - withPathDirectoryListings, - Effect.provideService(FileSystem.FileSystem, { - ...fs, - readDirectory: (directory) => - Effect.fail( - PlatformError.systemError({ - _tag: "PermissionDenied", - module: "FileSystem", - method: "readDirectory", - pathOrDescriptor: directory, - }), - ), - }), - ); - - expect(resolved).toBe(executable); + expect(probed).toEqual([path.join(first, "cursor.CMD"), path.join(second, "late.EXE")]); }).pipe( Effect.provideService(HostProcessPlatform, "win32"), Effect.provideService(CommandResolutionCache, new Map()), diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 8a6f9c97e16e..0a25785d916d 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -515,56 +515,39 @@ export const CommandResolutionCache = Context.Reference | undefined; } -// The directory's mtime, null when it does not exist, or undefined when it -// cannot be read or dated (permissions, a busy share). -const pathDirectoryModified = (directory: string) => +// mtime of a PATH directory; null when missing, undefined when unreadable. +const directoryMtime = (directory: string) => FileSystem.FileSystem.use((fileSystem) => fileSystem.stat(directory)).pipe( - Effect.map((info) => Option.getOrUndefined(info.mtime)?.getTime()), + Effect.map((info) => + info.type === "Directory" ? Option.getOrUndefined(info.mtime)?.getTime() : undefined, + ), Effect.catch((error) => Effect.succeed(error.reason._tag === "NotFound" ? null : undefined)), ); -// Dated before it is read, so an entry added in between shows up as a changed -// mtime on the next check instead of hiding behind a matching one. A missing -// directory holds nothing; one that cannot be dated or listed yields undefined -// names so lookups probe its candidates directly. -const listPathDirectory = (directory: string) => - Effect.gen(function* (): Effect.fn.Return { - const modified = yield* pathDirectoryModified(directory); - if (modified === null) return { modified, names: new Set() }; - if (modified === undefined) return { modified, names: undefined }; - const entries = yield* FileSystem.FileSystem.use((fileSystem) => - fileSystem.readDirectory(directory), - ).pipe(Effect.orElseSucceed(() => undefined)); - return { modified, names: entries && new Set(entries.map((entry) => entry.toLowerCase())) }; - }); +// Dated before it is read, so an entry added in between changes the mtime seen +// on the next check. Without names, lookups probe candidates directly. +const listPathDirectory = Effect.fnUntraced(function* ( + directory: string, +): Effect.fn.Return { + const modified = yield* directoryMtime(directory); + if (modified == null) return { modified, names: modified === null ? new Set() : undefined }; + const entries = yield* FileSystem.FileSystem.use((fileSystem) => + fileSystem.readDirectory(directory), + ).pipe(Effect.orElseSucceed(() => undefined)); + return { modified, names: entries && new Set(entries.map((entry) => entry.toLowerCase())) }; +}); const PathDirectoryListings = Context.Reference< Cache.Cache | undefined >("@t3tools/shared/shell/PathDirectoryListings", { defaultValue: () => undefined }); -// Every use re-checks the directory's mtime and relists it if it changed, so a -// command installed mid-batch is found exactly as a direct probe would find it. -// One directory stat per lookup still replaces a stat per PATHEXT candidate. -const readPathDirectoryNames = Effect.fn("shell.readPathDirectoryNames")(function* ( - listings: Cache.Cache, - directory: string, -) { - const listing = yield* Cache.get(listings, directory); - if (listing.names === undefined) return undefined; - if ((yield* pathDirectoryModified(directory)) === listing.modified) return listing.names; - yield* Cache.invalidate(listings, directory); - return (yield* Cache.get(listings, directory)).names; -}); - /** - * Run a batch of command lookups (such as editor discovery) that lists each - * PATH directory once and only probes names the listing contains, instead of - * stat-ing every PATH x PATHEXT candidate per command. A single lookup is - * cheaper without it. + * Run a batch of command lookups (e.g. editor discovery) that lists each PATH + * directory once, relisting it if its mtime changes, and probes only listed + * names instead of every PATH x PATHEXT candidate per command. */ export const withPathDirectoryListings = (effect: Effect.Effect) => Effect.gen(function* () { @@ -669,12 +652,14 @@ const resolveCommandPathForPlatform = Effect.fn("shell.resolveCommandPathForPlat const listings = yield* PathDirectoryListings; for (const pathEntry of pathEntries) { - const names = - listings === undefined ? undefined : yield* readPathDirectoryNames(listings, pathEntry); + let listing = listings && (yield* Cache.get(listings, pathEntry)); + if (listings && listing?.names && listing.modified !== (yield* directoryMtime(pathEntry))) { + yield* Cache.invalidate(listings, pathEntry); + listing = yield* Cache.get(listings, pathEntry); + } for (const candidate of commandCandidates) { - // Listings are lowercased; the stat below still checks the exact - // spelling for case-sensitive directories and rejects non-files. - if (names !== undefined && !names.has(candidate.toLowerCase())) continue; + // The stat below still checks exact case and rejects non-files. + if (listing?.names && !listing.names.has(candidate.toLowerCase())) continue; const candidatePath = path.join(pathEntry, candidate); if (yield* isExecutableFile(candidatePath, platform, windowsPathExtensions)) { cacheCommandResolution(cache, cacheKey, candidatePath, nowNanos);