From 5e115dd88d2d69d88445bcfcc0b4923a363b027f Mon Sep 17 00:00:00 2001 From: Erik Demaine Date: Tue, 8 Sep 2026 03:44:08 -0400 Subject: [PATCH] fix(server): preserve discovered editors when scanning times out --- .../src/process/externalLauncher.test.ts | 66 ++++++++++++++++- apps/server/src/process/externalLauncher.ts | 71 +++++++++++-------- apps/server/src/server.test.ts | 23 +----- apps/server/src/ws.ts | 14 ++-- 4 files changed, 110 insertions(+), 64 deletions(-) diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 2ae1684b67d4..df711c47f4f6 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -754,8 +754,8 @@ it.effect.skipIf(windowsHost)( ); // 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 +// timeout in server.getConfig returns editors found so far: 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* () { @@ -893,6 +893,68 @@ it.effect("memoizes editor discovery and refreshes after the cache window", () = ); }); +it.effect("returns editors found before discovery times out", () => { + const fileInfo = { type: "File" } as FileSystem.File.Info; + let blockScan = true; + let statCalls = 0; + const launcherLayer = ExternalLauncher.layer.pipe( + Layer.provide( + Layer.mergeAll( + FileSystem.layerNoop({ + stat: () => + Effect.gen(function* () { + statCalls += 1; + if (blockScan && statCalls > 1) { + return yield* Effect.never; + } + return fileInfo; + }), + }), + Path.layer, + Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => Effect.sync(() => makeMockDetachedHandle())), + ), + ), + ), + ); + + return Effect.gen(function* () { + const launcher = yield* ExternalLauncher.ExternalLauncher; + const fiber = yield* launcher + .resolveAvailableEditors({ timeout: "1 second" }) + .pipe(Effect.forkChild); + + yield* Effect.yieldNow; + yield* TestClock.adjust("1 second"); + + const partialEditors = yield* Fiber.join(fiber); + assert.lengthOf(partialEditors, 1); + + blockScan = false; + statCalls = 0; + const completeEditors = yield* launcher.resolveAvailableEditors(); + assert.isAbove(completeEditors.length, partialEditors.length); + assert.isAbove(statCalls, 0); + }).pipe( + Effect.provide( + Layer.mergeAll( + launcherLayer, + Layer.succeed(HostProcessPlatform, "win32"), + ConfigProvider.layer( + ConfigProvider.fromEnv({ + env: { + PATH: "C:\\t3-editor-discovery-timeout-test", + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }, + }), + ), + TestClock.layer(), + ), + ), + ); +}); + // A client that disconnects mid-scan interrupts the shared discovery effect on // the connection fiber. The cache must not retain that interrupt: doing so // replayed it to every later connect for the whole TTL, so `server.getConfig` diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index 184c8b519a96..c1ad3f4ef3a5 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -23,6 +23,7 @@ import { isCommandAvailable, resolveSpawnCommand } from "@t3tools/shared/shell"; import * as Clock from "effect/Clock"; import * as Config from "effect/Config"; import * as Context from "effect/Context"; +import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Encoding from "effect/Encoding"; import * as FileSystem from "effect/FileSystem"; @@ -261,10 +262,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's shorter timeout leaves time for the remaining editor probes +// before the overall scan deadline, even when `xdg-mime` hangs on a broken +// D-Bus or desktop session. const LINUX_DIRECTORY_HANDLER_PROBE_TIMEOUT = "2 seconds"; const hasUsableLinuxDirectoryHandler = Effect.fn("externalLauncher.hasUsableLinuxDirectoryHandler")( @@ -417,15 +417,15 @@ function buildBrowserLaunch( }; } -const buildAvailableEditors = Effect.fn("externalLauncher.buildAvailableEditors")(function* ( - platform: NodeJS.Platform, - env: NodeJS.ProcessEnv, -): Effect.fn.Return< - ReadonlyArray, - never, - FileSystem.FileSystem | Path.Path | ChildProcessSpawner.ChildProcessSpawner -> { - const available: EditorId[] = []; +/** + * Appends editors to `available` as their sequential probes succeed. The caller + * can return this accumulator when a timeout interrupts the remaining probes. + */ +const resolveAvailableEditors = Effect.fn("externalLauncher.resolveAvailableEditors")(function* ( + available: EditorId[], +) { + const platform = yield* HostProcessPlatform; + const env = { ...(yield* readBrowserLaunchEnv), ...(yield* readCommandLookupEnv) }; for (const editor of EDITORS) { if (editor.commands === null) { @@ -452,12 +452,6 @@ const resolveBrowserLaunch = Effect.fn("externalLauncher.resolveBrowserLaunch")( return buildBrowserLaunch(target, platform, env); }); -const resolveAvailableEditors = Effect.fn("externalLauncher.resolveAvailableEditors")(function* () { - const platform = yield* HostProcessPlatform; - const env = { ...(yield* readBrowserLaunchEnv), ...(yield* readCommandLookupEnv) }; - return yield* buildAvailableEditors(platform, env); -}); - const resolveFileManagerRevealKind = Effect.fn("externalLauncher.resolveFileManagerRevealKind")( function* () { const platform = yield* HostProcessPlatform; @@ -472,12 +466,10 @@ const resolveFileManagerRevealKind = Effect.fn("externalLauncher.resolveFileMana // per-command cache lookups in @t3tools/shared/shell. // // This deliberately does not use `Effect.cachedWithTTL`: that memoizes the -// first caller's Exit whatever it is, including an interrupt. Callers run this -// 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. +// first caller's Exit whatever it is, including an interrupt. Editor discovery +// can be interrupted by its timeout or by a client disconnecting mid-scan. +// 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. @@ -494,7 +486,9 @@ interface EditorDiscoveryCacheEntry { export class ExternalLauncher extends Context.Service< ExternalLauncher, { - readonly resolveAvailableEditors: () => Effect.Effect>; + readonly resolveAvailableEditors: (options?: { + readonly timeout?: Duration.Input; + }) => Effect.Effect>; /** * Reveal kind for the host, or undefined when the executable a reveal * actually spawns is unavailable. Only meaningful when @@ -768,15 +762,17 @@ export const make = Effect.gen(function* () { const editorDiscoveryCache = yield* Ref.make>( Option.none(), ); - const cachedAvailableEditors = Effect.gen(function* () { + const cachedAvailableEditors = Effect.fn("externalLauncher.cachedAvailableEditors")(function* ( + available: EditorId[], + ) { const nowNanos = yield* Clock.currentTimeNanos; const entry = yield* Ref.get(editorDiscoveryCache); if (Option.isSome(entry) && entry.value.expiresAtNanos > nowNanos) { return entry.value.editors; } - const editors = yield* provideCommandResolutionServices(resolveAvailableEditors()).pipe( - Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), - ); + const editors = yield* provideCommandResolutionServices( + resolveAvailableEditors(available), + ).pipe(Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner)); yield* Ref.set( editorDiscoveryCache, Option.some({ @@ -788,7 +784,20 @@ export const make = Effect.gen(function* () { }); return ExternalLauncher.of({ - resolveAvailableEditors: () => cachedAvailableEditors, + resolveAvailableEditors: (options) => + Effect.gen(function* () { + const available: EditorId[] = []; + const discovery = cachedAvailableEditors(available); + if (options?.timeout === undefined) { + return yield* discovery; + } + return yield* discovery.pipe( + Effect.timeoutOrElse({ + duration: options.timeout, + orElse: () => Effect.succeed(available), + }), + ); + }), resolveFileManagerRevealKind: () => provideCommandResolutionServices(resolveFileManagerRevealKind()).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), diff --git a/apps/server/src/server.test.ts b/apps/server/src/server.test.ts index 2f32b6524d7b..f3c2760e15e6 100644 --- a/apps/server/src/server.test.ts +++ b/apps/server/src/server.test.ts @@ -98,11 +98,7 @@ const encodeTestJson = Schema.encodeUnknownSync(Schema.fromJsonString(Schema.Unk import * as BackgroundPolicy from "./background/BackgroundPolicy.ts"; import * as ServerConfig from "./config.ts"; import { HTTP_ROUTER_CONFIG, makeRoutesLayer } from "./server.ts"; -import { - isThreadDetailEvent, - resolveAvailableEditorsForConfig, - resolveFileManagerRevealKindForConfig, -} from "./ws.ts"; +import { isThreadDetailEvent, resolveFileManagerRevealKindForConfig } from "./ws.ts"; import * as CheckpointDiffQuery from "./checkpointing/CheckpointDiffQuery.ts"; import * as GitManager from "./git/GitManager.ts"; import * as EnvironmentTheme from "./environmentTheme.ts"; @@ -4908,23 +4904,6 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); - it.effect("does not block server config when editor discovery never resolves", () => - Effect.gen(function* () { - const discoveryInterrupted = yield* Deferred.make(); - const responseFiber = yield* resolveAvailableEditorsForConfig( - Effect.never.pipe( - Effect.onInterrupt(() => Deferred.succeed(discoveryInterrupted, undefined)), - ), - ).pipe(Effect.forkChild); - - yield* TestClock.adjust(Duration.seconds(5)); - - const availableEditors = yield* Fiber.join(responseFiber); - yield* Deferred.await(discoveryInterrupted); - assert.deepEqual(availableEditors, []); - }), - ); - it.effect("does not block server config when file manager reveal discovery never resolves", () => Effect.gen(function* () { const discoveryInterrupted = yield* Deferred.make(); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 740be1330817..86c3d5afa3d6 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -28,7 +28,6 @@ import { CommandId, type DiscoveredLocalServerList, EventId, - type EditorId, type FileManagerRevealKind, type OrchestrationClientOrigin, type OrchestrationCommand, @@ -170,10 +169,6 @@ const resolveDiscoveryForConfig = ( Effect.map(Option.getOrElse(onTimeout)), ); -export const resolveAvailableEditorsForConfig = ( - discovery: Effect.Effect, E, R>, -) => resolveDiscoveryForConfig(discovery, () => []); - export const resolveFileManagerRevealKindForConfig = ( discovery: Effect.Effect, ) => resolveDiscoveryForConfig(discovery, () => undefined); @@ -1242,9 +1237,9 @@ const makeWsRpcLayer = ( ); const environment = yield* serverEnvironment.getDescriptor; const auth = yield* serverAuth.getDescriptor(); - const availableEditors: ReadonlyArray = yield* resolveAvailableEditorsForConfig( - externalLauncher.resolveAvailableEditors(), - ); + const availableEditors = yield* externalLauncher.resolveAvailableEditors({ + timeout: CONFIG_DISCOVERY_TIMEOUT, + }); const fileManagerRevealKind = availableEditors.includes("file-manager") ? yield* resolveFileManagerRevealKindForConfig( externalLauncher.resolveFileManagerRevealKind(), @@ -1262,8 +1257,9 @@ const makeWsRpcLayer = ( availableEditors, // Same discovery-with-timeout treatment as editors: a slow probe // must not stall server.getConfig, so it degrades to no targets. - remoteOpenTargets: yield* resolveAvailableEditorsForConfig( + remoteOpenTargets: yield* resolveDiscoveryForConfig( remoteOpenTargets.resolveTargets(), + () => [], ), observability: { logsDirectoryPath: config.logsDir,