From 4bd7310d34dc43bd6c42b7e18d2c25695a98b9db Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 19:27:19 +0000 Subject: [PATCH 1/3] fix(server): provider refresh finds CLIs added to the login shell PATH after startup The server read the login shell PATH only once at startup, and provider drivers copy the environment when they are built. A provider CLI installed later into a directory the login shell puts on PATH stayed "not found on PATH" until the server restarted. A provider refresh (manual, per instance, or a full refresh pass) now re-reads the login shell PATH asynchronously when an enabled instance reports its CLI as missing. If the shell adds PATH entries, they are merged into the server environment and only the missing instances are rebuilt, so installed instances keep their sessions. Fixes pingdotgg/t3code#34354 AI-assisted; verified with the tests and tools listed above. --- apps/server/src/os-jank.ts | 22 ++++ .../src/provider/ProviderInstanceRegistry.ts | 17 +++ .../ProviderInstanceRegistryHydration.test.ts | 1 + .../ProviderInstanceRegistryMutator.ts | 3 + .../src/provider/ProviderRegistry.test.ts | 101 ++++++++++++++++++ apps/server/src/provider/ProviderRegistry.ts | 23 +++- packages/shared/src/shell.ts | 63 +++++++++++ 7 files changed, 229 insertions(+), 1 deletion(-) diff --git a/apps/server/src/os-jank.ts b/apps/server/src/os-jank.ts index dece291aa1ca4..48e4f533a54ec 100644 --- a/apps/server/src/os-jank.ts +++ b/apps/server/src/os-jank.ts @@ -1,6 +1,7 @@ import * as HostProcess from "@t3tools/shared/HostProcess"; import { listLoginShellCandidates, + LoginShellEnvironment, mergePathEntries, readEnvironmentFromLoginShell, readPathFromLaunchctl, @@ -108,6 +109,27 @@ export const fixPath = Effect.fn("fixPath")(function* (options?: { ); }); +/** Re-reads the login shell PATH after startup; true when it added entries to PATH. */ +export const rereadLoginShellPath = Effect.fn("rereadLoginShellPath")(function* () { + const platform = yield* HostProcess.Platform; + if (platform !== "darwin" && platform !== "linux") return false; + const env = yield* HostProcess.Environment; + const readEnvironment = yield* LoginShellEnvironment; + for (const shell of listLoginShellCandidates(platform, env.SHELL)) { + const shellEnvironment = yield* readEnvironment(shell, ["PATH", "__MISE_DIFF"]); + const shellPath = shellEnvironment.PATH; + if (!shellPath) continue; + const current = new Set((env.PATH ?? "").split(":")); + if (shellPath.split(":").every((entry) => !entry.trim() || current.has(entry.trim()))) { + return false; + } + env.PATH = mergePathEntries(shellPath, env.PATH, platform); + if (shellEnvironment.__MISE_DIFF) env.__MISE_DIFF = shellEnvironment.__MISE_DIFF; + return true; + } + return false; +}); + export const expandHomePath = Effect.fn(function* (input: string) { const { join } = yield* Path.Path; const home = yield* HostProcess.HomeDirectory; diff --git a/apps/server/src/provider/ProviderInstanceRegistry.ts b/apps/server/src/provider/ProviderInstanceRegistry.ts index 3f4df0eb35d18..b00c5b41e296d 100644 --- a/apps/server/src/provider/ProviderInstanceRegistry.ts +++ b/apps/server/src/provider/ProviderInstanceRegistry.ts @@ -542,6 +542,22 @@ export const makeProviderInstanceRegistry = Effect.fn( Effect.provideContext(driverContext), ); + const rebuild: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["rebuild"] = + Effect.fn("ProviderInstanceRegistry.rebuild")( + function* (instances) { + const live = yield* Ref.get(entries); + const ids = new Set( + instances + .filter((instance) => live.get(instance.instanceId)?.instance === instance) + .map((instance) => instance.instanceId), + ); + if (ids.size === 0) return; + yield* reconcileWithR(yield* Ref.get(configMapRef), ids); + }, + (effect) => semaphore.withPermits(1)(effect), + Effect.provideContext(driverContext), + ); + // Reading secrets can wait on a vault unlock, so the server does not wait for it to start. yield* semaphore.withPermits(1)( Ref.set(configMapRef, input.configMap).pipe( @@ -592,6 +608,7 @@ export const makeProviderInstanceRegistry = Effect.fn( const mutator: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape = { reconcile, reloadSecrets, + rebuild, }; return { registry, mutator }; diff --git a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts b/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts index dc44f0324a6b0..0ba6245e98290 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts +++ b/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts @@ -165,6 +165,7 @@ describe("layerSettingsWatcher", () => { const mutator = Layer.succeed(ProviderInstanceRegistryMutator, { reconcile: () => Queue.offer(reconciled, undefined), reloadSecrets: () => Effect.sync(() => void reloads++), + rebuild: () => Effect.void, }); yield* Layer.build( layerSettingsWatcher(Stream.fromQueue(changes), initial).pipe(Layer.provide(mutator)), diff --git a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts index 35eb52d3f2906..2a5659cf6a3d6 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts +++ b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts @@ -1,5 +1,6 @@ /** Reconcile closes removed/replaced scopes before creating replacements: at most one live instance per id. */ import type { ProviderInstanceConfigMap, ProviderInstanceId } from "@t3tools/contracts"; +import type { ProviderInstance } from "@t3tools/provider-core/server/driver"; import * as Context from "effect/Context"; import type * as Effect from "effect/Effect"; @@ -7,6 +8,8 @@ export interface ProviderInstanceRegistryMutatorShape { readonly reconcile: (configMap: ProviderInstanceConfigMap) => Effect.Effect; /** Re-reads the secrets of every instance, or one, and rebuilds those whose secrets changed. */ readonly reloadSecrets: (instanceId?: ProviderInstanceId) => Effect.Effect; + /** Rebuilds these instances from their current config, so their drivers read the environment again. */ + readonly rebuild: (instances: ReadonlyArray) => Effect.Effect; } export class ProviderInstanceRegistryMutator extends Context.Service< diff --git a/apps/server/src/provider/ProviderRegistry.test.ts b/apps/server/src/provider/ProviderRegistry.test.ts index 5c53f76237b6a..902604731bf59 100644 --- a/apps/server/src/provider/ProviderRegistry.test.ts +++ b/apps/server/src/provider/ProviderRegistry.test.ts @@ -43,6 +43,7 @@ import * as HostProcess from "@t3tools/shared/HostProcess"; import { deepMerge } from "@t3tools/shared/Struct"; import { createModelCapabilities } from "@t3tools/shared/model"; import { applyServerSettingsPatch } from "@t3tools/shared/serverSettings"; +import { LoginShellEnvironment } from "@t3tools/shared/shell"; import { checkCodexProviderStatus, type CodexAppServerProviderSnapshot } from "./CodexProvider.ts"; import { BUNDLED_CLAUDE_MODEL_CATALOG } from "./ClaudeModelCatalog.ts"; @@ -345,6 +346,8 @@ const layerTestNodeServices = Layer.mergeAll( Layer.succeed(ServerEnvironment.ServerEnvironmentIdentity, { getEnvironmentId: Effect.succeed(EnvironmentId.make("00000000-0000-4000-8000-000000000001")), }), + // Refreshes of uninstalled providers re-read the login shell; never spawn the developer's. + Layer.succeed(LoginShellEnvironment, () => Effect.succeed({})), // An empty home, so the developer's own Claude settings never reach provider probes. Layer.effect( HostProcess.HomeDirectory, @@ -1296,6 +1299,104 @@ it.layer( ); }); + it.effect("rebuilds uninstalled instances when the login shell PATH gained entries", () => + Effect.gen(function* () { + const makeInstance = (id: string, installed: boolean) => { + const provider = { + instanceId: ProviderInstanceId.make(id), + driver: ProviderDriverKind.make("claudeAgent"), + status: installed ? "ready" : "error", + enabled: true, + installed, + auth: { status: "unknown" }, + checkedAt: "2026-11-11T00:00:00.000Z", + version: null, + models: [], + slashCommands: [], + skills: [], + } satisfies ServerProvider; + return { + instanceId: provider.instanceId, + driverKind: provider.driver, + continuationIdentity: { driverKind: provider.driver, continuationKey: id }, + displayName: undefined, + enabled: true, + snapshot: { + resolveMaintenance: () => + Effect.succeed( + makeManualOnlyProviderMaintenanceCapabilities({ + provider: provider.driver, + packageName: null, + }), + ), + getSnapshot: Effect.succeed(provider), + refresh: Effect.succeed(provider), + streamChanges: Stream.empty, + applyUsageLimits: () => Effect.void, + }, + orchestrationAdapter: {} as ProviderInstance["orchestrationAdapter"], + textGeneration: {} as ProviderInstance["textGeneration"], + } satisfies ProviderInstance; + }; + const missing = makeInstance("claude-missing", false); + const installed = makeInstance("claude-installed", true); + const rebuilt: Array> = []; + const env: NodeJS.ProcessEnv = { SHELL: "/bin/bash", PATH: "/usr/bin" }; + const shellPath = yield* Ref.make("/usr/bin"); + + yield* Effect.gen(function* () { + const registry = yield* ProviderRegistry.ProviderRegistry; + yield* registry.refresh(); + assert.deepStrictEqual(rebuilt, []); + + yield* Ref.set(shellPath, "/home/test/.local/bin:/usr/bin"); + yield* registry.refreshInstance(installed.instanceId); + assert.deepStrictEqual(rebuilt, []); + yield* registry.refresh(); + assert.deepStrictEqual(rebuilt, [[missing.instanceId]]); + assert.equal(env.PATH, "/home/test/.local/bin:/usr/bin"); + + yield* registry.refreshInstance(missing.instanceId); + assert.deepStrictEqual(rebuilt, [[missing.instanceId]]); + }).pipe( + Effect.provide( + ProviderRegistry.layer.pipe( + Layer.provide( + Layer.succeed(ProviderInstanceRegistry.ProviderInstanceRegistry, { + getInstance: (id) => + Effect.succeed([missing, installed].find((i) => i.instanceId === id)), + listInstances: Effect.succeed([missing, installed]), + listUnavailable: Effect.succeed([]), + streamChanges: Stream.empty, + subscribeChanges: Effect.flatMap(PubSub.unbounded(), PubSub.subscribe), + }), + ), + Layer.provide( + Layer.mock(ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutator)({ + rebuild: (instances) => + Effect.sync(() => { + rebuilt.push(instances.map((instance) => instance.instanceId)); + }), + }), + ), + ), + ), + Effect.provideService(HostProcess.Platform, "linux"), + Effect.provideService(HostProcess.Environment, env), + Effect.provideService(LoginShellEnvironment, () => + Ref.get(shellPath).pipe(Effect.map((PATH) => ({ PATH }))), + ), + Effect.scoped, + ); + }).pipe( + Effect.provide( + ServerConfig.layerTest(process.cwd(), { + prefix: "t3-login-shell-path-", + }).pipe(Layer.provideMerge(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))), + ), + ), + ); + describe("Antigravity model inventories", () => { const previousProvider = { instanceId: ProviderInstanceId.make("antigravity-personal"), diff --git a/apps/server/src/provider/ProviderRegistry.ts b/apps/server/src/provider/ProviderRegistry.ts index a0ea5c78fbc61..8406e6655ff56 100644 --- a/apps/server/src/provider/ProviderRegistry.ts +++ b/apps/server/src/provider/ProviderRegistry.ts @@ -27,6 +27,7 @@ import * as ProviderLatestVersions from "@t3tools/provider-core/server/ProviderL import * as ModelManifest from "./ModelManifest.ts"; import { applyProviderCompatibility } from "./providerCompatibility.ts"; import * as ServerConfig from "../config.ts"; +import { rereadLoginShellPath } from "../os-jank.ts"; import * as ProviderInstanceRegistry from "./ProviderInstanceRegistry.ts"; import * as ProviderInstanceRegistryMutator from "./ProviderInstanceRegistryMutator.ts"; import { @@ -627,6 +628,24 @@ export const layer = Layer.effect( ); }); + // PATH is read from the login shell at startup and drivers copy it when built, so a CLI + // installed later needs a fresh read and a rebuild. Installed instances keep their sessions. + const rebuildMissingAfterPathChange = Effect.fn("rebuildMissingAfterPathChange")(function* ( + instanceId?: ProviderInstanceId, + ) { + const instances = yield* instanceRegistry.listInstances; + const missing = yield* Effect.filter(instances, (instance) => + // ACP also reports installed:false for setup states unrelated to executable discovery. + !instance.enabled || + instance.driverKind === "acpRegistry" || + (instanceId !== undefined && instance.instanceId !== instanceId) + ? Effect.succeed(false) + : instance.snapshot.getSnapshot.pipe(Effect.map((snapshot) => !snapshot.installed)), + ); + if (missing.length === 0 || !(yield* rereadLoginShellPath())) return; + yield* instanceRegistryMutator.rebuild(missing); + }); + type RefreshAllCompletion = Deferred.Deferred>; const refreshAllInFlightRef = yield* Ref.make>( Option.none(), @@ -656,7 +675,8 @@ export const layer = Layer.effect( // Syncing first keeps a refresh right after a rebuild from probing the replaced instance. yield* manifestService.current.pipe( Effect.flatMap(({ updatedAt }) => - syncLiveSourcesAndContinue.pipe( + rebuildMissingAfterPathChange().pipe( + Effect.andThen(syncLiveSourcesAndContinue), Effect.andThen(Ref.get(liveSubsRef)), Effect.flatMap((instances) => Effect.forEach(instances.values(), (instance) => refreshOneSource(instance), { @@ -693,6 +713,7 @@ export const layer = Layer.effect( const refreshInstance = Effect.fn("refreshInstance")(function* ( instanceId: ProviderInstanceId, ) { + yield* rebuildMissingAfterPathChange(instanceId); yield* syncLiveSourcesAndContinue; const instance = (yield* Ref.get(liveSubsRef)).get(instanceId); if (!instance) { diff --git a/packages/shared/src/shell.ts b/packages/shared/src/shell.ts index 2b4dd9b96493b..a7134273e50a4 100644 --- a/packages/shared/src/shell.ts +++ b/packages/shared/src/shell.ts @@ -21,6 +21,8 @@ const WINDOWS_SHELL_CANDIDATES = ["pwsh.exe", "powershell.exe"] as const; const WINDOWS_SHELL_PROBE_TIMEOUT_MS = 5000; // A shell stuck in the kernel survives the kill and holds stdout, so execFile's callback never fires. const WINDOWS_SHELL_PROBE_DEADLINE = Duration.millis(WINDOWS_SHELL_PROBE_TIMEOUT_MS + 1000); +const LOGIN_SHELL_PROBE_TIMEOUT_MS = 5000; +const LOGIN_SHELL_PROBE_DEADLINE = Duration.millis(LOGIN_SHELL_PROBE_TIMEOUT_MS + 1000); type ExecFileSyncLike = ( file: string, @@ -331,6 +333,67 @@ export const readEnvironmentFromLoginShell: ShellEnvironmentReader = ( return environment; }; +const execLoginShellProbe = (shell: string, args: ReadonlyArray) => + Effect.callback>((resume) => { + let child: NodeChildProcess.ChildProcess; + try { + child = NodeChildProcess.spawn(shell, args, { + detached: true, + stdio: ["ignore", "pipe", "ignore"], + timeout: LOGIN_SHELL_PROBE_TIMEOUT_MS, + killSignal: "SIGKILL", + }); + } catch { + resume(Effect.succeedNone); + return; + } + let output = ""; + child.stdout?.setEncoding("utf8"); + child.stdout?.on("data", (chunk: string) => { + output += chunk; + }); + child.once("error", () => resume(Effect.succeedNone)); + child.once("close", (code) => + resume(Effect.succeed(code === 0 ? Option.some(output) : Option.none())), + ); + return Effect.sync(() => { + child.kill("SIGKILL"); + child.stdout?.destroy(); + child.unref(); + }); + }); + +export type LoginShellEnvironmentReader = ( + shell: string, + names: ReadonlyArray, +) => Effect.Effect>>; + +/** `readEnvironmentFromLoginShell` without blocking the event loop, for reads after startup. */ +const readEnvironmentFromLoginShellAsync: LoginShellEnvironmentReader = Effect.fn( + "shell.readEnvironmentFromLoginShellAsync", +)(function* (shell, names) { + if (names.length === 0) return {}; + const output = yield* execLoginShellProbe(shell, [ + "-ilc", + buildEnvironmentCaptureCommand(shell, names), + ]).pipe(Effect.timeoutOption(LOGIN_SHELL_PROBE_DEADLINE), Effect.map(Option.flatten)); + if (Option.isNone(output)) return {}; + + const environment: Partial> = {}; + for (const name of names) { + const value = extractEnvironmentValue(output.value, name); + if (value !== undefined) environment[name] = value; + } + return environment; +}); + +export const LoginShellEnvironment = Context.Reference( + "@t3tools/shared/shell/LoginShellEnvironment", + { + defaultValue: () => readEnvironmentFromLoginShellAsync, + }, +); + export type WindowsShellEnvironmentReader = ( names: ReadonlyArray, options?: WindowsEnvironmentProbeOptions, From d874fd0e85a74ccea1bd3eaa14e5d7fbc339b339 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 19:46:33 +0000 Subject: [PATCH 2/3] fix(server): recheck login shell PATH for every missing provider, including on health checks An instance refresh merged the new login shell PATH into the server environment but rebuilt only the refreshed instance, so other missing providers kept their stale environment copy and later refreshes saw no new PATH entries. The periodic health check never reread PATH at all. A PATH change now rebuilds every enabled missing instance, and a health-check probe that still finds a CLI missing rereads the login shell PATH, at most once a minute. AI-assisted; verified with the tests and tools listed above. --- .../src/provider/ProviderRegistry.test.ts | 97 +++++++++++-------- apps/server/src/provider/ProviderRegistry.ts | 39 ++++++-- 2 files changed, 87 insertions(+), 49 deletions(-) diff --git a/apps/server/src/provider/ProviderRegistry.test.ts b/apps/server/src/provider/ProviderRegistry.test.ts index 902604731bf59..74e1233eea074 100644 --- a/apps/server/src/provider/ProviderRegistry.test.ts +++ b/apps/server/src/provider/ProviderRegistry.test.ts @@ -17,6 +17,7 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as PubSub from "effect/PubSub"; +import * as Queue from "effect/Queue"; import * as Ref from "effect/Ref"; import * as Schema from "effect/Schema"; import * as Scope from "effect/Scope"; @@ -1299,9 +1300,13 @@ it.layer( ); }); - it.effect("rebuilds uninstalled instances when the login shell PATH gained entries", () => + it.effect("rebuilds every uninstalled instance when the login shell PATH gained entries", () => Effect.gen(function* () { - const makeInstance = (id: string, installed: boolean) => { + const makeInstance = ( + id: string, + installed: boolean, + streamChanges: Stream.Stream = Stream.empty, + ) => { const provider = { instanceId: ProviderInstanceId.make(id), driver: ProviderDriverKind.make("claudeAgent"), @@ -1316,56 +1321,67 @@ it.layer( skills: [], } satisfies ServerProvider; return { - instanceId: provider.instanceId, - driverKind: provider.driver, - continuationIdentity: { driverKind: provider.driver, continuationKey: id }, - displayName: undefined, - enabled: true, - snapshot: { - resolveMaintenance: () => - Effect.succeed( - makeManualOnlyProviderMaintenanceCapabilities({ - provider: provider.driver, - packageName: null, - }), - ), - getSnapshot: Effect.succeed(provider), - refresh: Effect.succeed(provider), - streamChanges: Stream.empty, - applyUsageLimits: () => Effect.void, - }, - orchestrationAdapter: {} as ProviderInstance["orchestrationAdapter"], - textGeneration: {} as ProviderInstance["textGeneration"], - } satisfies ProviderInstance; + provider, + instance: { + instanceId: provider.instanceId, + driverKind: provider.driver, + continuationIdentity: { driverKind: provider.driver, continuationKey: id }, + displayName: undefined, + enabled: true, + snapshot: { + resolveMaintenance: () => + Effect.succeed( + makeManualOnlyProviderMaintenanceCapabilities({ + provider: provider.driver, + packageName: null, + }), + ), + getSnapshot: Effect.succeed(provider), + refresh: Effect.succeed(provider), + streamChanges, + applyUsageLimits: () => Effect.void, + }, + orchestrationAdapter: {} as ProviderInstance["orchestrationAdapter"], + textGeneration: {} as ProviderInstance["textGeneration"], + } satisfies ProviderInstance, + }; }; - const missing = makeInstance("claude-missing", false); + const healthChecks = yield* PubSub.unbounded(); + const missing = makeInstance("claude-missing", false, Stream.fromPubSub(healthChecks)); + const otherMissing = makeInstance("codex-missing", false); const installed = makeInstance("claude-installed", true); - const rebuilt: Array> = []; + const instances = [missing.instance, otherMissing.instance, installed.instance]; + const missingIds = [missing.instance.instanceId, otherMissing.instance.instanceId]; + const rebuilds = yield* Queue.unbounded>(); const env: NodeJS.ProcessEnv = { SHELL: "/bin/bash", PATH: "/usr/bin" }; const shellPath = yield* Ref.make("/usr/bin"); yield* Effect.gen(function* () { const registry = yield* ProviderRegistry.ProviderRegistry; yield* registry.refresh(); - assert.deepStrictEqual(rebuilt, []); + assert.equal(yield* Queue.size(rebuilds), 0); + // Refreshing one instance merges PATH globally, so it rebuilds every missing one. yield* Ref.set(shellPath, "/home/test/.local/bin:/usr/bin"); - yield* registry.refreshInstance(installed.instanceId); - assert.deepStrictEqual(rebuilt, []); - yield* registry.refresh(); - assert.deepStrictEqual(rebuilt, [[missing.instanceId]]); + yield* registry.refreshInstance(missing.instance.instanceId); + assert.deepStrictEqual(yield* Queue.take(rebuilds), missingIds); assert.equal(env.PATH, "/home/test/.local/bin:/usr/bin"); - - yield* registry.refreshInstance(missing.instanceId); - assert.deepStrictEqual(rebuilt, [[missing.instanceId]]); + yield* registry.refresh(); + assert.equal(yield* Queue.size(rebuilds), 0); + + // A periodic health check that still finds the CLI missing rereads PATH too. + yield* Ref.set(shellPath, "/opt/tools/bin:/home/test/.local/bin:/usr/bin"); + yield* TestClock.adjust("5 minutes"); + yield* PubSub.publish(healthChecks, missing.provider); + assert.deepStrictEqual(yield* Queue.take(rebuilds), missingIds); + assert.equal(env.PATH, "/opt/tools/bin:/home/test/.local/bin:/usr/bin"); }).pipe( Effect.provide( ProviderRegistry.layer.pipe( Layer.provide( Layer.succeed(ProviderInstanceRegistry.ProviderInstanceRegistry, { - getInstance: (id) => - Effect.succeed([missing, installed].find((i) => i.instanceId === id)), - listInstances: Effect.succeed([missing, installed]), + getInstance: (id) => Effect.succeed(instances.find((i) => i.instanceId === id)), + listInstances: Effect.succeed(instances), listUnavailable: Effect.succeed([]), streamChanges: Stream.empty, subscribeChanges: Effect.flatMap(PubSub.unbounded(), PubSub.subscribe), @@ -1373,10 +1389,11 @@ it.layer( ), Layer.provide( Layer.mock(ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutator)({ - rebuild: (instances) => - Effect.sync(() => { - rebuilt.push(instances.map((instance) => instance.instanceId)); - }), + rebuild: (rebuilt) => + Queue.offer( + rebuilds, + rebuilt.map((instance) => instance.instanceId), + ).pipe(Effect.asVoid), }), ), ), diff --git a/apps/server/src/provider/ProviderRegistry.ts b/apps/server/src/provider/ProviderRegistry.ts index 8406e6655ff56..c0b9601ddb3c7 100644 --- a/apps/server/src/provider/ProviderRegistry.ts +++ b/apps/server/src/provider/ProviderRegistry.ts @@ -7,6 +7,7 @@ import { type ServerProviderUpdateState, } from "@t3tools/contracts"; import * as Cause from "effect/Cause"; +import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; @@ -630,21 +631,38 @@ export const layer = Layer.effect( // PATH is read from the login shell at startup and drivers copy it when built, so a CLI // installed later needs a fresh read and a rebuild. Installed instances keep their sessions. - const rebuildMissingAfterPathChange = Effect.fn("rebuildMissingAfterPathChange")(function* ( - instanceId?: ProviderInstanceId, - ) { + // The PATH merge is global, so every missing instance is rebuilt, not just a refreshed one. + const lastPathRereadAt = yield* Ref.make(yield* Clock.currentTimeMillis); + const rebuildMissingAfterPathChange = Effect.fn("rebuildMissingAfterPathChange")(function* () { const instances = yield* instanceRegistry.listInstances; const missing = yield* Effect.filter(instances, (instance) => // ACP also reports installed:false for setup states unrelated to executable discovery. - !instance.enabled || - instance.driverKind === "acpRegistry" || - (instanceId !== undefined && instance.instanceId !== instanceId) + !instance.enabled || instance.driverKind === "acpRegistry" ? Effect.succeed(false) : instance.snapshot.getSnapshot.pipe(Effect.map((snapshot) => !snapshot.installed)), ); - if (missing.length === 0 || !(yield* rereadLoginShellPath())) return; + if (missing.length === 0) return; + yield* Ref.set(lastPathRereadAt, yield* Clock.currentTimeMillis); + if (!(yield* rereadLoginShellPath())) return; yield* instanceRegistryMutator.rebuild(missing); }); + // Periodic health checks publish each probe; a missing CLI there rechecks PATH at most once + // a minute (the shortest health interval), forked so the rebuild outlives the old stream. + const pathRecheckRunning = yield* Ref.make(false); + const recheckPathAfterMissingProbe = Effect.fn("recheckPathAfterMissingProbe")(function* ( + instance: ProviderInstance, + provider: ServerProvider, + ) { + if (!provider.enabled || provider.installed || instance.driverKind === "acpRegistry") return; + const now = yield* Clock.currentTimeMillis; + if (now - (yield* Ref.get(lastPathRereadAt)) < 60_000) return; + if (yield* Ref.getAndSet(pathRecheckRunning, true)) return; + yield* rebuildMissingAfterPathChange().pipe( + Effect.ignoreCause({ log: true }), + Effect.ensuring(Ref.set(pathRecheckRunning, false)), + Effect.forkIn(serviceScope), + ); + }); type RefreshAllCompletion = Deferred.Deferred>; const refreshAllInFlightRef = yield* Ref.make>( @@ -713,7 +731,7 @@ export const layer = Layer.effect( const refreshInstance = Effect.fn("refreshInstance")(function* ( instanceId: ProviderInstanceId, ) { - yield* rebuildMissingAfterPathChange(instanceId); + yield* rebuildMissingAfterPathChange(); yield* syncLiveSourcesAndContinue; const instance = (yield* Ref.get(liveSubsRef)).get(instanceId); if (!instance) { @@ -868,7 +886,10 @@ export const layer = Layer.effect( for (const [, instance] of newlyAdded) { const source = buildSnapshotSource(instance); yield* Stream.runForEach(source.streamChanges, (provider) => - correlateSnapshotWithSource(source, provider).pipe(Effect.flatMap(syncProvider)), + correlateSnapshotWithSource(source, provider).pipe( + Effect.flatMap(syncProvider), + Effect.andThen(recheckPathAfterMissingProbe(instance, provider)), + ), ).pipe(Effect.forkScoped); } yield* Effect.yieldNow; From 90643b593ab30c8f15c5ed3c837352138d475dfa Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 11 Nov 2026 19:59:07 +0000 Subject: [PATCH 3/3] fix(server): reread login shell PATH only on provider refreshes, not health checks The previous change reread the login shell PATH from periodic health checks whenever an enabled provider was missing. Codex and Claude are enabled by default, so many users would spawn an interactive login shell (running their startup files) about every five minutes, indefinitely. Drop the health-check hook and its throttle state. PATH is reread only when a provider refresh runs while an enabled provider is missing, and every missing instance is still rebuilt when the PATH gained entries. AI-assisted; verified with the tests and tools listed above. --- .../src/provider/ProviderRegistry.test.ts | 18 ++---------- apps/server/src/provider/ProviderRegistry.ts | 28 ++----------------- 2 files changed, 5 insertions(+), 41 deletions(-) diff --git a/apps/server/src/provider/ProviderRegistry.test.ts b/apps/server/src/provider/ProviderRegistry.test.ts index 74e1233eea074..78a7ea492fe61 100644 --- a/apps/server/src/provider/ProviderRegistry.test.ts +++ b/apps/server/src/provider/ProviderRegistry.test.ts @@ -1302,11 +1302,7 @@ it.layer( it.effect("rebuilds every uninstalled instance when the login shell PATH gained entries", () => Effect.gen(function* () { - const makeInstance = ( - id: string, - installed: boolean, - streamChanges: Stream.Stream = Stream.empty, - ) => { + const makeInstance = (id: string, installed: boolean) => { const provider = { instanceId: ProviderInstanceId.make(id), driver: ProviderDriverKind.make("claudeAgent"), @@ -1338,7 +1334,7 @@ it.layer( ), getSnapshot: Effect.succeed(provider), refresh: Effect.succeed(provider), - streamChanges, + streamChanges: Stream.empty, applyUsageLimits: () => Effect.void, }, orchestrationAdapter: {} as ProviderInstance["orchestrationAdapter"], @@ -1346,8 +1342,7 @@ it.layer( } satisfies ProviderInstance, }; }; - const healthChecks = yield* PubSub.unbounded(); - const missing = makeInstance("claude-missing", false, Stream.fromPubSub(healthChecks)); + const missing = makeInstance("claude-missing", false); const otherMissing = makeInstance("codex-missing", false); const installed = makeInstance("claude-installed", true); const instances = [missing.instance, otherMissing.instance, installed.instance]; @@ -1368,13 +1363,6 @@ it.layer( assert.equal(env.PATH, "/home/test/.local/bin:/usr/bin"); yield* registry.refresh(); assert.equal(yield* Queue.size(rebuilds), 0); - - // A periodic health check that still finds the CLI missing rereads PATH too. - yield* Ref.set(shellPath, "/opt/tools/bin:/home/test/.local/bin:/usr/bin"); - yield* TestClock.adjust("5 minutes"); - yield* PubSub.publish(healthChecks, missing.provider); - assert.deepStrictEqual(yield* Queue.take(rebuilds), missingIds); - assert.equal(env.PATH, "/opt/tools/bin:/home/test/.local/bin:/usr/bin"); }).pipe( Effect.provide( ProviderRegistry.layer.pipe( diff --git a/apps/server/src/provider/ProviderRegistry.ts b/apps/server/src/provider/ProviderRegistry.ts index c0b9601ddb3c7..49fae77a1c1f8 100644 --- a/apps/server/src/provider/ProviderRegistry.ts +++ b/apps/server/src/provider/ProviderRegistry.ts @@ -7,7 +7,6 @@ import { type ServerProviderUpdateState, } from "@t3tools/contracts"; import * as Cause from "effect/Cause"; -import * as Clock from "effect/Clock"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; import * as Deferred from "effect/Deferred"; @@ -632,7 +631,6 @@ export const layer = Layer.effect( // PATH is read from the login shell at startup and drivers copy it when built, so a CLI // installed later needs a fresh read and a rebuild. Installed instances keep their sessions. // The PATH merge is global, so every missing instance is rebuilt, not just a refreshed one. - const lastPathRereadAt = yield* Ref.make(yield* Clock.currentTimeMillis); const rebuildMissingAfterPathChange = Effect.fn("rebuildMissingAfterPathChange")(function* () { const instances = yield* instanceRegistry.listInstances; const missing = yield* Effect.filter(instances, (instance) => @@ -641,28 +639,9 @@ export const layer = Layer.effect( ? Effect.succeed(false) : instance.snapshot.getSnapshot.pipe(Effect.map((snapshot) => !snapshot.installed)), ); - if (missing.length === 0) return; - yield* Ref.set(lastPathRereadAt, yield* Clock.currentTimeMillis); - if (!(yield* rereadLoginShellPath())) return; + if (missing.length === 0 || !(yield* rereadLoginShellPath())) return; yield* instanceRegistryMutator.rebuild(missing); }); - // Periodic health checks publish each probe; a missing CLI there rechecks PATH at most once - // a minute (the shortest health interval), forked so the rebuild outlives the old stream. - const pathRecheckRunning = yield* Ref.make(false); - const recheckPathAfterMissingProbe = Effect.fn("recheckPathAfterMissingProbe")(function* ( - instance: ProviderInstance, - provider: ServerProvider, - ) { - if (!provider.enabled || provider.installed || instance.driverKind === "acpRegistry") return; - const now = yield* Clock.currentTimeMillis; - if (now - (yield* Ref.get(lastPathRereadAt)) < 60_000) return; - if (yield* Ref.getAndSet(pathRecheckRunning, true)) return; - yield* rebuildMissingAfterPathChange().pipe( - Effect.ignoreCause({ log: true }), - Effect.ensuring(Ref.set(pathRecheckRunning, false)), - Effect.forkIn(serviceScope), - ); - }); type RefreshAllCompletion = Deferred.Deferred>; const refreshAllInFlightRef = yield* Ref.make>( @@ -886,10 +865,7 @@ export const layer = Layer.effect( for (const [, instance] of newlyAdded) { const source = buildSnapshotSource(instance); yield* Stream.runForEach(source.streamChanges, (provider) => - correlateSnapshotWithSource(source, provider).pipe( - Effect.flatMap(syncProvider), - Effect.andThen(recheckPathAfterMissingProbe(instance, provider)), - ), + correlateSnapshotWithSource(source, provider).pipe(Effect.flatMap(syncProvider)), ).pipe(Effect.forkScoped); } yield* Effect.yieldNow;