From c85b5964f193d422ec7705301ca434c3b6eb1172 Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Thu, 6 Aug 2026 23:05:43 -0700 Subject: [PATCH 1/2] fix(server): stop a disconnecting client from blocking all later reconnects Editor discovery is memoized with Effect.cachedWithTTL, which stores the first caller's Exit whatever it is. Callers run it on the connection fiber under a timeout, so a client that disconnects mid-scan cached the resulting interrupt and every later connect replayed it: server.getConfig died on the first RPC and no client could reconnect for the 60s TTL, refreshed on each failed attempt. Cache the discovered set only on success, so an interrupted scan leaves the cache untouched and the next connect rescans. Co-Authored-By: Claude --- .../src/process/externalLauncher.test.ts | 65 +++++++++++++++++++ apps/server/src/process/externalLauncher.ts | 39 +++++++++-- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index 36ef8264328..1ab6166e92a 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -2,6 +2,7 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import * as ConfigProvider from "effect/ConfigProvider"; 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 Path from "effect/Path"; @@ -216,6 +217,70 @@ it.effect("memoizes editor discovery and refreshes after the cache window", () = ); }); +// 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` +// 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({ + // 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. + stat: () => + Effect.gen(function* () { + scans += 1; + if (blockFirstScan) { + 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* Effect.forkChild(launcher.resolveAvailableEditors()); + yield* Effect.yieldNow; + yield* Fiber.interrupt(fiber); + + // The next connect must still get a real answer well inside the TTL. + blockFirstScan = false; + scans = 0; + const editors = yield* launcher.resolveAvailableEditors(); + assert.equal(editors.includes("vscode"), true); + assert.isAbove(scans, 0); + }).pipe( + Effect.provide( + Layer.mergeAll( + launcherLayer, + Layer.succeed(HostProcessPlatform, "win32"), + ConfigProvider.layer( + ConfigProvider.fromEnv({ + env: { + PATH: "C:\\t3-editor-discovery-interrupt-test", + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }, + }), + ), + ), + ), + ); +}); + 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 2cac42f0fec..f020ae075fc 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -19,14 +19,17 @@ import { } from "@t3tools/contracts"; import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; 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"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Path from "effect/Path"; +import * as Ref from "effect/Ref"; import * as ChildProcess from "effect/unstable/process/ChildProcess"; import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; @@ -302,7 +305,20 @@ const resolveAvailableEditors = Effect.fn("externalLauncher.resolveAvailableEdit // client connect (the server config embeds the available editors). Memoize // the discovered set for a bounded window so repeat connects skip even the // per-command cache lookups in @t3tools/shared/shell. -const EDITOR_DISCOVERY_CACHE_TTL = "60 seconds"; +// +// 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. +const EDITOR_DISCOVERY_CACHE_TTL = Duration.seconds(60); + +interface EditorDiscoveryCacheEntry { + readonly editors: ReadonlyArray; + readonly expiresAtMillis: number; +} /** * ExternalLauncher - Service tag for browser/editor launch operations. @@ -449,10 +465,25 @@ export const make = Effect.gen(function* () { Effect.provideService(Path.Path, path), ); - const cachedAvailableEditors = yield* Effect.cachedWithTTL( - provideCommandResolutionServices(resolveAvailableEditors()), - EDITOR_DISCOVERY_CACHE_TTL, + const editorDiscoveryCache = yield* Ref.make>( + Option.none(), ); + const cachedAvailableEditors = Effect.gen(function* () { + const nowMillis = yield* Clock.currentTimeMillis; + const entry = yield* Ref.get(editorDiscoveryCache); + if (Option.isSome(entry) && entry.value.expiresAtMillis > nowMillis) { + return entry.value.editors; + } + const editors = yield* provideCommandResolutionServices(resolveAvailableEditors()); + yield* Ref.set( + editorDiscoveryCache, + Option.some({ + editors, + expiresAtMillis: nowMillis + Duration.toMillis(EDITOR_DISCOVERY_CACHE_TTL), + }), + ); + return editors; + }); return ExternalLauncher.of({ resolveAvailableEditors: () => cachedAvailableEditors, From 76d0847b01ac5dfdb98e91dfbd54f7563121625c Mon Sep 17 00:00:00 2001 From: Theo Browne Date: Thu, 6 Aug 2026 23:11:11 -0700 Subject: [PATCH 2/2] fix(server): expire the editor discovery cache on the monotonic clock Matches the command-resolution cache in @t3tools/shared/shell so a backward wall-clock adjustment cannot keep an expired entry alive. Co-Authored-By: Claude --- apps/server/src/process/externalLauncher.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index f020ae075fc..8ec928f26fc 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -22,7 +22,6 @@ 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"; @@ -313,11 +312,14 @@ const resolveAvailableEditors = Effect.fn("externalLauncher.resolveAvailableEdit // 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. -const EDITOR_DISCOVERY_CACHE_TTL = Duration.seconds(60); +// 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. +const EDITOR_DISCOVERY_CACHE_TTL_NANOS = 60_000_000_000n; interface EditorDiscoveryCacheEntry { readonly editors: ReadonlyArray; - readonly expiresAtMillis: number; + readonly expiresAtNanos: bigint; } /** @@ -469,9 +471,9 @@ export const make = Effect.gen(function* () { Option.none(), ); const cachedAvailableEditors = Effect.gen(function* () { - const nowMillis = yield* Clock.currentTimeMillis; + const nowNanos = yield* Clock.currentTimeNanos; const entry = yield* Ref.get(editorDiscoveryCache); - if (Option.isSome(entry) && entry.value.expiresAtMillis > nowMillis) { + if (Option.isSome(entry) && entry.value.expiresAtNanos > nowNanos) { return entry.value.editors; } const editors = yield* provideCommandResolutionServices(resolveAvailableEditors()); @@ -479,7 +481,7 @@ export const make = Effect.gen(function* () { editorDiscoveryCache, Option.some({ editors, - expiresAtMillis: nowMillis + Duration.toMillis(EDITOR_DISCOVERY_CACHE_TTL), + expiresAtNanos: nowNanos + EDITOR_DISCOVERY_CACHE_TTL_NANOS, }), ); return editors;