From 0080e80c00a3e333e3dcafd85b2615d56f84e3b8 Mon Sep 17 00:00:00 2001 From: Bob Fowler Date: Sat, 3 Oct 2026 11:45:10 -0400 Subject: [PATCH] fix(server): editors appear once a slow discovery scan finishes (#13917) --- .../src/process/externalLauncher.test.ts | 41 ++-- apps/server/src/process/externalLauncher.ts | 79 +++++--- apps/server/src/ws.test.ts | 188 +++++++++++++++++- apps/server/src/ws.ts | 110 ++++++++-- 4 files changed, 359 insertions(+), 59 deletions(-) diff --git a/apps/server/src/process/externalLauncher.test.ts b/apps/server/src/process/externalLauncher.test.ts index aab4d78f4d7145d4b889ec5c2583bfdd7b7c6684..9a5c8b99ce7d0fd3980c3afa21e136ba6f5d94fa 100644 --- a/apps/server/src/process/externalLauncher.test.ts +++ b/apps/server/src/process/externalLauncher.test.ts @@ -6,6 +6,7 @@ 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 Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; @@ -1186,26 +1187,24 @@ 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", () => { +// Connects run discovery under a timeout and may disconnect mid-scan. Neither +// may cancel the scan: on a busy host every connect would time out partway +// through, cache nothing, and leave every client without editors. +it.effect("keeps scanning after the caller is interrupted and shares that scan", () => { const fileInfo = { type: "File" } as FileSystem.File.Info; - let blockFirstScan = true; - let scans = 0; + const release = Deferred.makeUnsafe(); + let parkedStats = 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. + // Scans park inside `stat` until released, so the interrupt lands + // while discovery is in flight. stat: () => Effect.gen(function* () { - scans += 1; - if (blockFirstScan) { - return yield* Effect.never; + if (!Deferred.isDoneUnsafe(release)) { + parkedStats += 1; + yield* Deferred.await(release); } return fileInfo; }), @@ -1222,16 +1221,18 @@ it.effect("rescans after an interrupted discovery instead of caching the interru return Effect.gen(function* () { const launcher = yield* ExternalLauncher.ExternalLauncher; - const fiber = yield* Effect.forkChild(launcher.resolveAvailableEditors()); + const interrupted = yield* Effect.forkChild(launcher.resolveAvailableEditors()); yield* Effect.yieldNow; - yield* Fiber.interrupt(fiber); + yield* Fiber.interrupt(interrupted); - // The next connect must still get a real answer well inside the TTL. - blockFirstScan = false; - scans = 0; - const editors = yield* launcher.resolveAvailableEditors(); + // The next connect joins the running scan instead of starting its own. + const next = yield* Effect.forkChild(launcher.resolveAvailableEditors()); + yield* Effect.yieldNow; + assert.equal(parkedStats, 1); + + yield* Deferred.succeed(release, undefined); + const editors = yield* Fiber.join(next); assert.equal(editors.includes("vscode"), true); - assert.isAbove(scans, 0); }).pipe( Effect.provide( Layer.mergeAll( diff --git a/apps/server/src/process/externalLauncher.ts b/apps/server/src/process/externalLauncher.ts index ab287e4d78ee4ec62e5624b4686a575ed62856e4..058318ea9d55d6d484f1cf1d385e4b4c69873df9 100644 --- a/apps/server/src/process/externalLauncher.ts +++ b/apps/server/src/process/externalLauncher.ts @@ -28,13 +28,16 @@ import { import * as Clock from "effect/Clock"; import * as Config from "effect/Config"; import * as Context from "effect/Context"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Encoding from "effect/Encoding"; +import * as Exit from "effect/Exit"; 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 Scope from "effect/Scope"; import * as Stream from "effect/Stream"; import * as ChildProcess from "effect/unstable/process/ChildProcess"; import * as ChildProcessSpawner from "effect/unstable/process/ChildProcessSpawner"; @@ -462,21 +465,21 @@ const resolveFileManagerRevealKind = Effect.fn("externalLauncher.resolveFileMana // the discovered set for a bounded window so repeat connects skip even the // 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. +// The scan runs on its own fiber in the service scope, and every caller awaits +// that one scan. Callers apply a timeout (`resolveAvailableEditorsForConfig`) +// and disconnect mid-connect; neither may cancel a scan other connects are +// waiting on, or throw away work a slow host (a busy server at startup, a +// long PATH) needs more than one connect to finish. A failed scan clears the +// entry so the next caller starts over rather than replaying the failure. // 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 expiresAtNanos: bigint; + readonly scan: Deferred.Deferred>; + /** Undefined while the scan is still running. */ + readonly expiresAtNanos: bigint | undefined; } /** @@ -760,27 +763,57 @@ export const make = Effect.gen(function* () { Effect.provideService(Path.Path, path), ); + const scope = yield* Scope.Scope; const editorDiscoveryCache = yield* Ref.make>( Option.none(), ); - const cachedAvailableEditors = Effect.gen(function* () { - 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( + const runEditorDiscovery = (scan: Deferred.Deferred>) => + provideCommandResolutionServices(resolveAvailableEditors()).pipe( Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + Effect.onExit((exit) => + Effect.gen(function* () { + const expiresAtNanos = (yield* Clock.currentTimeNanos) + EDITOR_DISCOVERY_CACHE_TTL_NANOS; + yield* Ref.update(editorDiscoveryCache, (current) => + Option.isNone(current) || current.value.scan !== scan + ? current + : Exit.isSuccess(exit) + ? Option.some({ scan, expiresAtNanos }) + : Option.none(), + ); + yield* Deferred.done(scan, exit); + }), + ), + Effect.interruptible, + Effect.forkIn(scope), ); - yield* Ref.set( + // Claiming the cache entry and starting its scan must not be split by an + // interrupt, or the entry would wait on a scan that never runs. + const acquireEditorDiscovery = Effect.gen(function* () { + const nowNanos = yield* Clock.currentTimeNanos; + const [scan, isNewScan] = yield* Ref.modify( editorDiscoveryCache, - Option.some({ - editors, - expiresAtNanos: nowNanos + EDITOR_DISCOVERY_CACHE_TTL_NANOS, - }), + ( + current, + ): [ + [EditorDiscoveryCacheEntry["scan"], boolean], + Option.Option, + ] => { + if ( + Option.isSome(current) && + (current.value.expiresAtNanos === undefined || current.value.expiresAtNanos > nowNanos) + ) { + return [[current.value.scan, false], current]; + } + const scan = Deferred.makeUnsafe>(); + return [[scan, true], Option.some({ scan, expiresAtNanos: undefined })]; + }, ); - return editors; - }); + if (isNewScan) { + yield* runEditorDiscovery(scan); + } + return scan; + }).pipe(Effect.uninterruptible); + const cachedAvailableEditors = Effect.flatMap(acquireEditorDiscovery, Deferred.await); return ExternalLauncher.of({ resolveAvailableEditors: () => cachedAvailableEditors, diff --git a/apps/server/src/ws.test.ts b/apps/server/src/ws.test.ts index e280f0750cef016c78b44030546034dc44eb31db..630d64c49b6c18bd4ea237517b9ea9331f82c001 100644 --- a/apps/server/src/ws.test.ts +++ b/apps/server/src/ws.test.ts @@ -1,15 +1,29 @@ import { assert, it } from "@effect/vitest"; -import { ORCHESTRATION_PROTOCOL_VERSION } from "@t3tools/contracts"; +import { + ORCHESTRATION_PROTOCOL_VERSION, + type ServerConfig, + type ServerConfigStreamEvent, +} from "@t3tools/contracts"; +import { HostProcessPlatform } from "@t3tools/shared/hostProcess"; +import * as ConfigProvider from "effect/ConfigProvider"; import * as Deferred from "effect/Deferred"; import * as Duration from "effect/Duration"; 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"; +import * as Queue from "effect/Queue"; +import * as Stream from "effect/Stream"; import * as TestClock from "effect/testing/TestClock"; +import { ChildProcessSpawner } from "effect/unstable/process"; +import * as ExternalLauncher from "./process/externalLauncher.ts"; import { hasCompatibleOrchestrationProtocol, resolveAvailableEditorsForConfig, shouldUseBoundedThreadSnapshot, + withLateEditorConfig, } from "./ws.ts"; it("accepts only the current orchestration protocol before websocket RPC setup", () => { @@ -48,3 +62,175 @@ it.effect("does not block server config when editor discovery never resolves", ( assert.deepEqual(availableEditors, []); }), ); + +// Only the fields the late-editor fold reads or rewrites. +const snapshotConfig = (fields: Partial) => + ({ availableEditors: [], settings: {}, ...fields }) as unknown as ServerConfig; + +const settingsUpdated = (settings: object): ServerConfigStreamEvent => ({ + version: 1, + type: "settingsUpdated", + payload: { settings: settings as ServerConfig["settings"] }, +}); + +it.effect("resends late editors without rolling back updates already sent", () => + Effect.gen(function* () { + const settingsSent = yield* Deferred.make(); + const events = yield* withLateEditorConfig( + snapshotConfig({ settings: { enableProviderUpdateChecks: true } as never }), + Stream.make(settingsUpdated({ enableProviderUpdateChecks: false })), + { + resolveAvailableEditors: () => Effect.succeed(["file-manager"]), + // Holds the late snapshot until the settings change has gone out. + resolveFileManagerRevealKind: () => + Deferred.await(settingsSent).pipe(Effect.as("file-explorer" as const)), + }, + ).pipe( + Stream.tap((event) => + event.type === "settingsUpdated" ? Deferred.succeed(settingsSent, undefined) : Effect.void, + ), + Stream.runCollect, + ); + + const [first, second] = Array.from(events); + assert.equal(events.length, 2); + assert.equal(first?.type, "settingsUpdated"); + assert.equal(second?.type, "snapshot"); + if (second?.type === "snapshot") { + assert.deepEqual(second.config.availableEditors, ["file-manager"]); + assert.equal(second.config.shellRevealInFileManagerKind, "file-explorer"); + assert.deepEqual(second.config.settings, { enableProviderUpdateChecks: false } as never); + } + }), +); + +it.effect("sends no late snapshot when the scan matches the snapshot", () => + Effect.gen(function* () { + const events = yield* withLateEditorConfig( + snapshotConfig({ availableEditors: ["vscode"] }), + Stream.empty, + { + resolveAvailableEditors: () => Effect.succeed(["vscode"]), + resolveFileManagerRevealKind: () => Effect.succeed(undefined), + }, + ).pipe(Stream.runCollect); + + assert.equal(events.length, 0); + }), +); + +it.effect("resends a file manager reveal kind that missed the snapshot", () => + Effect.gen(function* () { + const events = yield* withLateEditorConfig( + snapshotConfig({ availableEditors: ["file-manager"] }), + Stream.empty, + { + resolveAvailableEditors: () => Effect.succeed(["file-manager"]), + resolveFileManagerRevealKind: () => Effect.succeed("file-explorer"), + }, + ).pipe(Stream.runCollect); + + const [late] = Array.from(events); + assert.equal(events.length, 1); + assert.equal(late?.type, "snapshot"); + if (late?.type === "snapshot") { + assert.equal(late.config.shellRevealInFileManagerKind, "file-explorer"); + } + }), +); + +// The real launcher on Windows over a filesystem whose probes park until +// released, like a host too busy to finish discovery inside the snapshot timeout. +const makeParkedWindowsLauncher = Effect.gen(function* () { + const parkedProbes = yield* Queue.unbounded(); + const release = yield* Deferred.make(); + const launcher = yield* ExternalLauncher.make.pipe( + Effect.provide( + Layer.mergeAll( + FileSystem.layerNoop({ + stat: () => + Queue.offer(parkedProbes, undefined).pipe( + Effect.andThen(Deferred.await(release)), + Effect.as({ type: "File" } as FileSystem.File.Info), + ), + }), + Path.layer, + Layer.succeed( + ChildProcessSpawner.ChildProcessSpawner, + ChildProcessSpawner.make(() => Effect.die("unexpected spawn")), + ), + ), + ), + ); + const onWindows = (effect: Effect.Effect) => + effect.pipe( + Effect.provideService(HostProcessPlatform, "win32"), + Effect.provide( + ConfigProvider.layer( + ConfigProvider.fromEnv({ + env: { PATH: "C:\\t3-late-editors-test", PATHEXT: ".EXE" }, + }), + ), + ), + ); + return { + editors: { + resolveAvailableEditors: () => onWindows(launcher.resolveAvailableEditors()), + resolveFileManagerRevealKind: () => onWindows(launcher.resolveFileManagerRevealKind()), + }, + probeParked: Queue.take(parkedProbes), + releaseProbes: Deferred.succeed(release, undefined), + }; +}); + +it.effect("recovers editors after a real scan outlasts the config timeout", () => + Effect.gen(function* () { + const { editors, probeParked, releaseProbes } = yield* makeParkedWindowsLauncher; + + const snapshotFiber = yield* resolveAvailableEditorsForConfig( + editors.resolveAvailableEditors(), + ).pipe(Effect.forkChild); + yield* probeParked; + yield* TestClock.adjust(Duration.seconds(5)); + const snapshotEditors = yield* Fiber.join(snapshotFiber); + assert.deepEqual(snapshotEditors, []); + + const lateFiber = yield* withLateEditorConfig( + snapshotConfig({ availableEditors: snapshotEditors }), + Stream.empty, + editors, + ).pipe(Stream.runCollect, Effect.forkChild); + yield* releaseProbes; + + const [late] = Array.from(yield* Fiber.join(lateFiber)); + assert.equal(late?.type, "snapshot"); + if (late?.type === "snapshot") { + assert.equal(late.config.availableEditors.includes("vscode"), true); + } + }).pipe(Effect.scoped), +); + +it.effect("recovers a reveal kind whose real probe outlasts the config timeout", () => + Effect.gen(function* () { + const { editors, probeParked, releaseProbes } = yield* makeParkedWindowsLauncher; + + // The snapshot's bounded probe timed out: file manager, but no reveal kind. + const lateFiber = yield* withLateEditorConfig( + snapshotConfig({ availableEditors: ["file-manager"] }), + Stream.empty, + { + resolveAvailableEditors: () => Effect.succeed(["file-manager"]), + resolveFileManagerRevealKind: editors.resolveFileManagerRevealKind, + }, + ).pipe(Stream.runCollect, Effect.forkChild); + yield* probeParked; + yield* TestClock.adjust(Duration.seconds(6)); + yield* releaseProbes; + + const [late] = Array.from(yield* Fiber.join(lateFiber)); + assert.equal(late?.type, "snapshot"); + if (late?.type === "snapshot") { + assert.equal(late.config.shellRevealInFileManagerKind, "file-explorer"); + } + }).pipe(Effect.scoped), +); diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 2a0006ddb24b4b1fc96ded5dbf5474113f2e61a6..783377666f147c14ce6622dee4181c22b77d54b8 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -76,6 +76,8 @@ import { type RelayClientInstallProgressEvent, type ServerSelfUpdateError, type ServerSelfUpdateProgressEvent, + type ServerConfig as ClientServerConfig, + type ServerConfigStreamEvent, type ServerLifecycleStreamEvent, type FilesystemBrowseFailure, FilesystemBrowseError, @@ -261,6 +263,94 @@ const resolveFileManagerRevealKindForConfig = ( discovery: Effect.Effect, ) => resolveDiscoveryForConfig(discovery, () => undefined); +type EditorDiscovery = Pick< + ExternalLauncher.ExternalLauncher["Service"], + "resolveAvailableEditors" | "resolveFileManagerRevealKind" +>; + +// The config fields that follow from which editors are installed. +const resolveEditorConfig = ( + availableEditors: ReadonlyArray, + revealKind: Effect.Effect, +) => + Effect.gen(function* () { + const fileManagerRevealKind = availableEditors.includes("file-manager") + ? yield* revealKind + : undefined; + return { + availableEditors, + ...(fileManagerRevealKind === undefined + ? {} + : { + shellRevealInFileManager: true, + shellRevealInFileManagerKind: fileManagerRevealKind, + }), + }; + }); + +/** + * Live config updates that follow a snapshot of `config`. A busy host can + * outlast the snapshot's discovery timeouts, which send no editors, or no + * reveal kind for the file manager. The scan keeps running, so once it lands + * this resends the config: clients replace theirs on any snapshot. The resent + * config is folded from the live updates already sent, so it cannot roll back + * a change that landed while the scan ran. + */ +export const withLateEditorConfig = ( + config: ClientServerConfig, + liveUpdates: Stream.Stream, + launcher: EditorDiscovery, +) => { + const lateEditorConfig = Stream.fromEffect(launcher.resolveAvailableEditors()).pipe( + Stream.filter( + (editors) => + editors.join() !== config.availableEditors.join() || + (editors.includes("file-manager") && config.shellRevealInFileManagerKind === undefined), + ), + // Unbounded, unlike the snapshot: the reveal-kind probe is not shared, so + // a timeout here would cancel a probe that outlasts it every time. + Stream.mapEffect((editors) => + resolveEditorConfig(editors, launcher.resolveFileManagerRevealKind()), + ), + Stream.filter( + (editorConfig) => + editorConfig.availableEditors.join() !== config.availableEditors.join() || + editorConfig.shellRevealInFileManagerKind !== config.shellRevealInFileManagerKind, + ), + Stream.map((editorConfig) => ({ type: "editorsResolved" as const, editorConfig })), + ); + + return Stream.merge(liveUpdates, lateEditorConfig).pipe( + Stream.mapAccum( + (): ClientServerConfig => config, + (current, event): readonly [ClientServerConfig, ReadonlyArray] => { + switch (event.type) { + case "editorsResolved": { + const { + availableEditors: _editors, + shellRevealInFileManager: _reveal, + shellRevealInFileManagerKind: _revealKind, + ...rest + } = current; + const next = { ...rest, ...event.editorConfig }; + return [next, [{ version: 1, type: "snapshot", config: next }]]; + } + case "keybindingsUpdated": + return [{ ...current, ...event.payload }, [event]]; + case "providerStatuses": + return [{ ...current, providers: event.payload.providers }, [event]]; + case "settingsUpdated": + return [{ ...current, settings: event.payload.settings }, [event]]; + // Themes and usage-limit sources never ride in a snapshot; clients + // carry their projected values across one. + default: + return [current, [event]]; + } + }, + ), + ); +}; + function unexpectedCompatibilityError(error: never): never { throw new Error(`Unhandled compatibility error: ${String(error)}`); } @@ -1620,14 +1710,10 @@ const makeWsRpcLayer = ( const environment = yield* serverEnvironment.getDescriptor; const auth = yield* serverAuth.getDescriptor(); const scratchWorkspaceRoot = yield* managedFolders.scratchRoot; - const availableEditors: ReadonlyArray = yield* resolveAvailableEditorsForConfig( - externalLauncher.resolveAvailableEditors(), + const editorConfig = yield* resolveEditorConfig( + yield* resolveAvailableEditorsForConfig(externalLauncher.resolveAvailableEditors()), + resolveFileManagerRevealKindForConfig(externalLauncher.resolveFileManagerRevealKind()), ); - const fileManagerRevealKind = availableEditors.includes("file-manager") - ? yield* resolveFileManagerRevealKindForConfig( - externalLauncher.resolveFileManagerRevealKind(), - ) - : undefined; return { environment, @@ -1637,7 +1723,7 @@ const makeWsRpcLayer = ( keybindings: keybindingsConfig.keybindings, issues: keybindingsConfig.issues, providers, - availableEditors, + ...editorConfig, // Same discovery-with-timeout treatment as editors: a slow probe // must not stall server.getConfig, so it degrades to no targets. remoteOpenTargets: yield* resolveAvailableEditorsForConfig( @@ -1659,12 +1745,6 @@ const makeWsRpcLayer = ( }, settings, shellResumeCompletionMarker: true, - ...(fileManagerRevealKind === undefined - ? {} - : { - shellRevealInFileManager: true, - shellRevealInFileManagerKind: fileManagerRevealKind, - }), threadResumeCompletionMarker: true, threadSnapshotPagination: true, ...Option.match(scratchWorkspaceRoot, { @@ -3632,7 +3712,7 @@ const makeWsRpcLayer = ( return Stream.concat( rpcInitialItems([{ version: 1 as const, type: "snapshot" as const, config }]), - liveUpdates, + withLateEditorConfig(config, liveUpdates, externalLauncher), ); }), { "rpc.aggregate": "server" }, -- 2.54.0