From ed9c2f65bbffc476832d4a3e0c6ff8324f4f9f33 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Fri, 24 Jul 2026 23:44:35 +0200 Subject: [PATCH 1/6] fix(sessions): move provider catalog network refreshes off request paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared ModelRuntime was constructed with network refreshes enabled, so reloadConfig()/login()/logout() — called on the model picker, session model changes, and auth dialogs — performed unbounded provider-catalog fetches. A single stalled fetch blocked those requests for minutes and, through pi's coalesced per-provider refresh, dragged session creation along with it. Construct the runtime with PI_OFFLINE forced so every runtime-driven refresh stays local, and add ModelCatalogRefresher as the single deliberate network path: bounded by an abort timeout, serialized through one in-flight run, scheduled in the background, and triggered after provider auth changes. --- .changeset/bounded-model-catalog-refresh.md | 5 + src/server/sessiond.ts | 12 +- src/server/sessions/authService.test.ts | 37 +++- src/server/sessions/authService.ts | 32 +++- .../sessions/modelCatalogRefresher.test.ts | 173 ++++++++++++++++++ src/server/sessions/modelCatalogRefresher.ts | 108 +++++++++++ 6 files changed, 358 insertions(+), 9 deletions(-) create mode 100644 .changeset/bounded-model-catalog-refresh.md create mode 100644 src/server/sessions/modelCatalogRefresher.test.ts create mode 100644 src/server/sessions/modelCatalogRefresher.ts diff --git a/.changeset/bounded-model-catalog-refresh.md b/.changeset/bounded-model-catalog-refresh.md new file mode 100644 index 0000000..d6c9e45 --- /dev/null +++ b/.changeset/bounded-model-catalog-refresh.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Fix multi-minute stalls when opening the model selector, starting sessions, or using the auth dialogs. Provider catalog refreshes no longer run unbounded network fetches on request paths: the shared model runtime now operates offline and pi-web refreshes provider catalogs itself on a bounded, background schedule and after provider logins. diff --git a/src/server/sessiond.ts b/src/server/sessiond.ts index 8703dba..949eeb0 100644 --- a/src/server/sessiond.ts +++ b/src/server/sessiond.ts @@ -9,6 +9,7 @@ import { SessionEventHub } from "./realtime/sessionEventHub.js"; import { AuthService } from "./sessions/authService.js"; import { bootstrapAndFreezeGlobalExtensionProviders } from "./sessions/globalProviderPolicy.js"; import { registerAuthRoutes } from "./sessions/authRoutes.js"; +import { ModelCatalogRefresher } from "./sessions/modelCatalogRefresher.js"; import { PiSessionService } from "./sessions/piSessionService.js"; import { createPiSessionManagerGateway } from "./sessions/piSessionManagerGateway.js"; import { registerSessionRoutes } from "./sessions/sessionRoutes.js"; @@ -55,6 +56,12 @@ await runSessionDaemonStartup({ // still mutable, then freeze every later extension-provider mutation before // any real session can load project resources. await bootstrapAndFreezeGlobalExtensionProviders(auth.runtime, activeAgentProfile.dir, app.log); + // The shared model runtime is constructed offline so request paths never + // wait on provider-catalog fetches; this is the single bounded network + // refresher, and auth changes (login/logout) ask it for a prompt run. + const catalogRefresher = new ModelCatalogRefresher({ runtime: auth.runtime, logger: app.log }); + catalogRefresher.start(); + auth.subscribe(() => { catalogRefresher.requestRefresh(); }); const spawnTargets = config.spawnSessions ? new ProjectScopedSpawnTargetResolver({ projects: new ProjectService(new ProjectStore()), workspaces: new WorkspaceService() }) : undefined; @@ -79,7 +86,7 @@ await runSessionDaemonStartup({ ...getPiWebRuntimeComponent("sessiond", SESSIOND_RUNTIME_CAPABILITIES), activeAgentProfile, }); - return { eventHub, workspaceActivity, auth, sessions, terminals, unreadStore, activeAgentProfile, runtimeComponent }; + return { eventHub, workspaceActivity, auth, sessions, terminals, unreadStore, activeAgentProfile, runtimeComponent, catalogRefresher }; }, registerRoutes({ eventHub, workspaceActivity, auth, sessions, terminals, runtimeComponent }) { registerWorkspaceActivityRoutes(app, workspaceActivity); @@ -102,7 +109,7 @@ await runSessionDaemonStartup({ app.get("/runtime", () => runtimeComponent); }, - async listen({ auth, sessions, terminals, unreadStore }) { + async listen({ auth, sessions, terminals, unreadStore, catalogRefresher }) { let shuttingDown = false; async function shutdown(signal: NodeJS.Signals): Promise { if (shuttingDown) return; @@ -117,6 +124,7 @@ await runSessionDaemonStartup({ } }; await attempt("dispose terminals", () => { terminals.dispose(); }); + await attempt("dispose catalog refresher", () => { catalogRefresher.dispose(); }); await attempt("dispose auth", () => { auth.dispose(); }); await attempt("dispose sessions", () => sessions.dispose()); await attempt("flush session unread state", () => unreadStore.flush()); diff --git a/src/server/sessions/authService.test.ts b/src/server/sessions/authService.test.ts index 8a07d0a..7779966 100644 --- a/src/server/sessions/authService.test.ts +++ b/src/server/sessions/authService.test.ts @@ -362,7 +362,7 @@ describe("AuthService", () => { it("stores credentials in the configured agent directory", async () => { const agentDir = await tempAgentDir(); - const runtime = await createModelRuntimeForAgentDir(agentDir, false); + const runtime = await createModelRuntimeForAgentDir(agentDir); const auth = await AuthService.create({ runtime }); await auth.saveApiKey("anthropic", "sk-test"); @@ -470,6 +470,39 @@ describe("AuthService", () => { }); }); +describe("createModelRuntimeForAgentDir", () => { + it("disables runtime-owned network refreshes so request paths stay local", async () => { + // The stall fix relies on this construction-time flag: reloadConfig(), + // login(), and logout() refresh with allowNetwork = modelNetworkEnabled. + // Asserting the private field is proportionate here because the flag is + // exactly the contract this change depends on. + const agentDir = await tempAgentDir(); + const runtime = await createModelRuntimeForAgentDir(agentDir); + expect(Reflect.get(runtime, "modelNetworkEnabled")).toBe(false); + }); + + it("restores a previously set PI_OFFLINE after runtime creation", async () => { + // The file-level beforeEach stubs PI_OFFLINE=1. + const agentDir = await tempAgentDir(); + await createModelRuntimeForAgentDir(agentDir); + expect(process.env["PI_OFFLINE"]).toBe("1"); + }); + + it("restores a previously unset PI_OFFLINE after runtime creation", async () => { + vi.unstubAllEnvs(); + const previous = process.env["PI_OFFLINE"]; + delete process.env["PI_OFFLINE"]; + try { + const agentDir = await tempAgentDir(); + const runtime = await createModelRuntimeForAgentDir(agentDir); + expect(Reflect.get(runtime, "modelNetworkEnabled")).toBe(false); + expect(process.env["PI_OFFLINE"]).toBeUndefined(); + } finally { + if (previous !== undefined) process.env["PI_OFFLINE"] = previous; + } + }); +}); + async function createAuthService(seed: Record = {}, logger?: AuthServiceLogger) { const credentials = new InMemoryCredentialStore(); for (const [providerId, credential] of Object.entries(seed)) { @@ -486,7 +519,7 @@ async function createFileBackedAuthService(seed: Record) { const agentDir = await tempAgentDir(); const authPath = join(agentDir, "auth.json"); await writeFile(authPath, JSON.stringify(seed, null, 2)); - const runtime = await createModelRuntimeForAgentDir(agentDir, false); + const runtime = await createModelRuntimeForAgentDir(agentDir); const auth = await AuthService.create({ runtime }); const changes: AuthChange[] = []; auth.subscribe((change) => { changes.push(change); }); diff --git a/src/server/sessions/authService.ts b/src/server/sessions/authService.ts index 5a237ff..6e7eaca 100644 --- a/src/server/sessions/authService.ts +++ b/src/server/sessions/authService.ts @@ -1,5 +1,5 @@ import { join } from "node:path"; -import { ModelRuntime } from "@earendil-works/pi-coding-agent"; +import { ModelRuntime, type CreateModelRuntimeOptions } from "@earendil-works/pi-coding-agent"; import type { AuthInteraction } from "@earendil-works/pi-ai"; import type { AuthProvidersResponse, AuthType, OAuthFlowState } from "../../shared/apiTypes.js"; import { getLoginProviderOptions, getLogoutProviderOptions } from "./authProviderOptions.js"; @@ -31,11 +31,33 @@ interface AuthChangeContext { const noopLogger: AuthServiceLogger = { error() { /* no-op */ } }; -export function createModelRuntimeForAgentDir(agentDir: string, allowModelNetwork?: boolean): Promise { - return ModelRuntime.create({ +/** + * Create the shared model runtime with runtime-owned network refreshes disabled. + * + * Upstream `ModelRuntime.reloadConfig()`, `login()`, and `logout()` always refresh + * with `allowNetwork: modelNetworkEnabled` and accept no abort signal. With the + * default (`PI_OFFLINE` unset) a single stalled provider-catalog fetch can block + * those call paths for minutes — and, because pi-web shares one runtime and pi + * coalesces per-provider refreshes, session creation joins the same stalled + * fetch. Forcing `PI_OFFLINE` during construction makes every runtime-driven + * refresh local-only; pi-web performs its own bounded catalog refreshes in the + * background instead (see modelCatalogRefresher.ts). + */ +async function createOfflineModelRuntime(options: CreateModelRuntimeOptions): Promise { + const previous = process.env["PI_OFFLINE"]; + process.env["PI_OFFLINE"] = "1"; + try { + return await ModelRuntime.create(options); + } finally { + if (previous === undefined) delete process.env["PI_OFFLINE"]; + else process.env["PI_OFFLINE"] = previous; + } +} + +export function createModelRuntimeForAgentDir(agentDir: string): Promise { + return createOfflineModelRuntime({ authPath: join(agentDir, "auth.json"), modelsPath: join(agentDir, "models.json"), - ...(allowModelNetwork === undefined ? {} : { allowModelNetwork }), }); } @@ -52,7 +74,7 @@ export class AuthService { } static async create(deps: AuthServiceDependencies = {}): Promise { - const runtime = deps.runtime ?? (deps.agentDir === undefined ? await ModelRuntime.create({}) : await createModelRuntimeForAgentDir(deps.agentDir)); + const runtime = deps.runtime ?? (deps.agentDir === undefined ? await createOfflineModelRuntime({}) : await createModelRuntimeForAgentDir(deps.agentDir)); const logger = deps.logger ?? noopLogger; const authFlows = deps.authFlows ?? new OAuthLoginFlowService({ logger }); return new AuthService(runtime, authFlows, logger); diff --git a/src/server/sessions/modelCatalogRefresher.test.ts b/src/server/sessions/modelCatalogRefresher.test.ts new file mode 100644 index 0000000..c311bb8 --- /dev/null +++ b/src/server/sessions/modelCatalogRefresher.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ModelCatalogRefresher } from "./modelCatalogRefresher.js"; + +interface RefreshCall { + allowNetwork?: boolean; + signal?: AbortSignal; +} + +interface RefreshResult { + aborted: boolean; + errors: Map; +} + +const okResult = (): RefreshResult => ({ aborted: false, errors: new Map() }); + +function deferred() { + let resolveValue: (value: T) => void = () => undefined; + const promise = new Promise((resolve) => { + resolveValue = resolve; + }); + return { promise, resolve: resolveValue }; +} + +function createRuntime() { + const calls: RefreshCall[] = []; + const refresh = vi.fn((options?: RefreshCall) => { + calls.push(options ?? {}); + return Promise.resolve(okResult()); + }); + return { refresh, calls }; +} + +function createLogger() { + const warn = vi.fn(); + const error = vi.fn(); + return { logger: { warn, error }, warn, error }; +} + +/** Let a started refresh run to completion, including the finally-queue bookkeeping. */ +async function flushMicrotasks(rounds = 5): Promise { + for (let index = 0; index < rounds; index++) await Promise.resolve(); +} + +describe("ModelCatalogRefresher", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("runs a bounded network refresh when one is requested", async () => { + const runtime = createRuntime(); + const refresher = new ModelCatalogRefresher({ runtime }); + + refresher.requestRefresh(); + await flushMicrotasks(); + + expect(runtime.refresh).toHaveBeenCalledOnce(); + const call = runtime.calls.at(0); + expect(call?.allowNetwork).toBe(true); + expect(call?.signal).toBeInstanceOf(AbortSignal); + refresher.dispose(); + }); + + it("coalesces overlapping requests into a single follow-up run", async () => { + const gate = deferred(); + const refresh = vi.fn() + .mockImplementationOnce(() => gate.promise) + .mockResolvedValue(okResult()); + const refresher = new ModelCatalogRefresher({ runtime: { refresh } }); + + refresher.requestRefresh(); + refresher.requestRefresh(); + refresher.requestRefresh(); + expect(refresh).toHaveBeenCalledOnce(); + + gate.resolve(okResult()); + await flushMicrotasks(); + + expect(refresh).toHaveBeenCalledTimes(2); + refresher.dispose(); + }); + + it("refreshes after the initial delay and then on the interval", async () => { + const runtime = createRuntime(); + const refresher = new ModelCatalogRefresher({ runtime, initialDelayMs: 1_000, intervalMs: 60_000 }); + refresher.start(); + + await vi.advanceTimersByTimeAsync(999); + expect(runtime.refresh).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); + expect(runtime.refresh).toHaveBeenCalledOnce(); + + await vi.advanceTimersByTimeAsync(60_000); + expect(runtime.refresh).toHaveBeenCalledTimes(2); + refresher.dispose(); + }); + + it("stops scheduling refreshes after dispose", async () => { + const runtime = createRuntime(); + const refresher = new ModelCatalogRefresher({ runtime, initialDelayMs: 1_000, intervalMs: 60_000 }); + refresher.start(); + refresher.dispose(); + + await vi.advanceTimersByTimeAsync(120_000); + expect(runtime.refresh).not.toHaveBeenCalled(); + }); + + it("does not run a queued follow-up after dispose", async () => { + const gate = deferred(); + const refresh = vi.fn().mockImplementation(() => gate.promise); + const refresher = new ModelCatalogRefresher({ runtime: { refresh } }); + + refresher.requestRefresh(); + refresher.requestRefresh(); + refresher.dispose(); + gate.resolve(okResult()); + await flushMicrotasks(); + + expect(refresh).toHaveBeenCalledOnce(); + }); + + it("warns and keeps going when a refresh is aborted by its timeout", async () => { + const { logger, warn, error } = createLogger(); + const refresh = vi.fn(() => Promise.resolve({ aborted: true, errors: new Map() })); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, logger }); + + refresher.requestRefresh(); + await flushMicrotasks(); + + expect(warn).toHaveBeenCalledOnce(); + expect(error).not.toHaveBeenCalled(); + refresher.dispose(); + }); + + it("warns with provider details when a refresh reports provider errors", async () => { + const { logger, warn } = createLogger(); + const errors = new Map([["openrouter", new Error("boom")]]); + const refresh = vi.fn(() => Promise.resolve({ aborted: false, errors })); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, logger }); + + refresher.requestRefresh(); + await flushMicrotasks(); + + expect(warn).toHaveBeenCalledWith( + { providers: ["openrouter: boom"] }, + "model catalog refresh failed for some providers; keeping cached catalogs", + ); + refresher.dispose(); + }); + + it("logs and swallows a rejecting refresh so timers stay alive", async () => { + const { logger, error } = createLogger(); + const failure = new Error("refresh exploded"); + const refresh = vi.fn() + .mockRejectedValueOnce(failure) + .mockResolvedValue(okResult()); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, logger, initialDelayMs: 1_000, intervalMs: 60_000 }); + + refresher.requestRefresh(); + await flushMicrotasks(); + + expect(error).toHaveBeenCalledWith({ err: failure }, "model catalog refresh failed; keeping cached catalogs"); + + refresher.start(); + await vi.advanceTimersByTimeAsync(61_000); + expect(refresh).toHaveBeenCalledTimes(3); + refresher.dispose(); + }); +}); diff --git a/src/server/sessions/modelCatalogRefresher.ts b/src/server/sessions/modelCatalogRefresher.ts new file mode 100644 index 0000000..7d4ad54 --- /dev/null +++ b/src/server/sessions/modelCatalogRefresher.ts @@ -0,0 +1,108 @@ +import type { ModelRuntime } from "@earendil-works/pi-coding-agent"; + +/** + * Matches pi's REMOTE_CATALOG_REFRESH_INTERVAL_MS: provider catalog entries in + * models-store.json are treated as fresh for four hours. + */ +const DEFAULT_INTERVAL_MS = 4 * 60 * 60 * 1000; +/** Give the daemon a moment to finish startup before the first network refresh. */ +const DEFAULT_INITIAL_DELAY_MS = 15_000; +/** Bound every catalog refresh so a stalled provider fetch can never block for minutes. */ +const DEFAULT_TIMEOUT_MS = 15_000; + +/** Minimal structured-logging seam for non-fatal refresh failures. */ +export interface ModelCatalogRefresherLogger { + warn(details: Record, message: string): void; + error(details: Record, message: string): void; +} + +export interface ModelCatalogRefresherOptions { + runtime: Pick; + logger?: ModelCatalogRefresherLogger; + intervalMs?: number; + initialDelayMs?: number; + timeoutMs?: number; +} + +const noopLogger: ModelCatalogRefresherLogger = { + warn() { /* no-op */ }, + error() { /* no-op */ }, +}; + +/** + * Refreshes provider model catalogs over the network on a background schedule. + * + * The shared ModelRuntime is constructed offline (see authService.ts), so its + * own refreshes never touch the network and stay fast on request paths. This + * refresher is the single place that deliberately performs network refreshes — + * bounded by an abort timeout, serialized through one in-flight run, and off + * any request path. `requestRefresh()` additionally asks for a prompt refresh + * after events that change what should be listed, such as provider logins. + */ +export class ModelCatalogRefresher { + private readonly runtime: Pick; + private readonly logger: ModelCatalogRefresherLogger; + private readonly intervalMs: number; + private readonly initialDelayMs: number; + private readonly timeoutMs: number; + private initialTimer?: NodeJS.Timeout; + private intervalTimer?: NodeJS.Timeout; + private inflight: Promise | undefined; + private queued = false; + private disposed = false; + + constructor(options: ModelCatalogRefresherOptions) { + this.runtime = options.runtime; + this.logger = options.logger ?? noopLogger; + this.intervalMs = options.intervalMs ?? DEFAULT_INTERVAL_MS; + this.initialDelayMs = options.initialDelayMs ?? DEFAULT_INITIAL_DELAY_MS; + this.timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + } + + start(): void { + if (this.disposed) return; + this.initialTimer = setTimeout(() => { this.requestRefresh(); }, this.initialDelayMs); + this.initialTimer.unref(); + this.intervalTimer = setInterval(() => { this.requestRefresh(); }, this.intervalMs); + this.intervalTimer.unref(); + } + + /** Ask for a refresh, coalescing concurrent and overlapping requests. */ + requestRefresh(): void { + if (this.disposed) return; + if (this.inflight !== undefined) { + this.queued = true; + return; + } + const run = this.run(); + this.inflight = run; + this.inflight.finally(() => { + this.inflight = undefined; + if (this.queued && !this.disposed) { + this.queued = false; + this.requestRefresh(); + } + }).catch(() => { /* run() never rejects; finally() re-throws otherwise */ }); + } + + dispose(): void { + this.disposed = true; + if (this.initialTimer !== undefined) clearTimeout(this.initialTimer); + if (this.intervalTimer !== undefined) clearInterval(this.intervalTimer); + } + + private async run(): Promise { + try { + const result = await this.runtime.refresh({ allowNetwork: true, signal: AbortSignal.timeout(this.timeoutMs) }); + if (result.aborted) { + this.logger.warn({ timeoutMs: this.timeoutMs }, "model catalog refresh timed out; keeping cached catalogs"); + } + if (result.errors.size > 0) { + const providers = [...result.errors.entries()].map(([providerId, error]) => `${providerId}: ${error.message}`); + this.logger.warn({ providers }, "model catalog refresh failed for some providers; keeping cached catalogs"); + } + } catch (error: unknown) { + this.logger.error({ err: error }, "model catalog refresh failed; keeping cached catalogs"); + } + } +} From acda1cc0be7e70249d55214203530e4ac0726c36 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sat, 25 Jul 2026 12:40:33 +0200 Subject: [PATCH 2/6] fix(sessions): honor offline settings in background catalog refresher The background model catalog refresher always requested a network refresh, so sessiond fetched provider catalogs on a schedule even when the operator set PI_OFFLINE or PI_WEB_OFFLINE. Before the refresher existed, those settings made every runtime refresh local-only. Add `offlineModeEnabled()` to the config module and inject the resulting flag from sessiond's frozen daemon environment, so the refresher schedules nothing and ignores auth-triggered requests in offline mode. The narrower PI_SKIP_VERSION_CHECK / PI_WEB_SKIP_VERSION_CHECK keys are deliberately not included: they only suppress release lookups. --- src/config.test.ts | 21 ++++++++++++++- src/config.ts | 15 +++++++++++ src/server/sessiond.ts | 11 +++++--- .../sessions/modelCatalogRefresher.test.ts | 18 ++++++++++++- src/server/sessions/modelCatalogRefresher.ts | 27 ++++++++++++++++--- 5 files changed, 84 insertions(+), 8 deletions(-) diff --git a/src/config.test.ts b/src/config.test.ts index be5c0d1..b2ae4cf 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -2,7 +2,7 @@ import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { DEFAULT_MAX_UPLOAD_BYTES, DEFAULT_UPLOADS_FOLDER, agentDirEnvSource, agentSessionDirEnvKeys, effectiveAgentConfig, effectivePiWebConfig, hasAgentDirEnvOverride, hasAgentSessionDirEnvOverride, loadPiWebConfig, maxUploadBytes, savePiWebConfig, spawnSessionsEnabled, subsessionsEnabled } from "./config.js"; +import { DEFAULT_MAX_UPLOAD_BYTES, DEFAULT_UPLOADS_FOLDER, agentDirEnvSource, agentSessionDirEnvKeys, effectiveAgentConfig, effectivePiWebConfig, hasAgentDirEnvOverride, hasAgentSessionDirEnvOverride, loadPiWebConfig, maxUploadBytes, offlineModeEnabled, savePiWebConfig, spawnSessionsEnabled, subsessionsEnabled } from "./config.js"; let tempDir: string; let configPath: string; @@ -233,6 +233,25 @@ describe("subsessionsEnabled", () => { }); }); +describe("offlineModeEnabled", () => { + it("is off when no offline env var is set", () => { + expect(offlineModeEnabled({})).toBe(false); + }); + + it("treats an empty value as unset", () => { + expect(offlineModeEnabled({ PI_OFFLINE: "", PI_WEB_OFFLINE: "" })).toBe(false); + }); + + it("is on when either offline key has a value", () => { + expect(offlineModeEnabled({ PI_OFFLINE: "1" })).toBe(true); + expect(offlineModeEnabled({ PI_WEB_OFFLINE: "anything" })).toBe(true); + }); + + it("ignores the narrower skip-version-check keys", () => { + expect(offlineModeEnabled({ PI_SKIP_VERSION_CHECK: "1", PI_WEB_SKIP_VERSION_CHECK: "1" })).toBe(false); + }); +}); + function testOptions(): { env: NodeJS.ProcessEnv } { return { env: { PI_WEB_CONFIG: configPath } }; } diff --git a/src/config.ts b/src/config.ts index 8ad0920..5f766b5 100644 --- a/src/config.ts +++ b/src/config.ts @@ -266,6 +266,21 @@ export function subsessionsEnabled(env: NodeJS.ProcessEnv = process.env, config: return config.subsessions ?? false; } +const OFFLINE_ENV_KEYS = ["PI_WEB_OFFLINE", "PI_OFFLINE"] as const; + +/** + * Whether the operator asked PI WEB (or pi itself) to stay offline, meaning + * background network access must be skipped. Matches the "set and non-empty" + * semantics used for the other runtime-only env switches. + * + * Deliberately narrower than `piWebStatus`'s update-check suppression: the + * `*_SKIP_VERSION_CHECK` keys only silence release lookups, while these keys ask + * for no background network at all. + */ +export function offlineModeEnabled(env: NodeJS.ProcessEnv = process.env): boolean { + return OFFLINE_ENV_KEYS.some((key) => isEnvSet(env[key])); +} + function parseString(value: unknown, key: string, path: string): string { if (typeof value !== "string" || value === "") throw new Error(`PI WEB config ${key} must be a non-empty string: ${path}`); return value; diff --git a/src/server/sessiond.ts b/src/server/sessiond.ts index 949eeb0..fe4fb44 100644 --- a/src/server/sessiond.ts +++ b/src/server/sessiond.ts @@ -24,7 +24,7 @@ import { TerminalService } from "./terminals/terminalService.js"; import { registerTerminalRoutes } from "./terminals/terminalRoutes.js"; import { getPiWebRuntimeComponent } from "./piWebStatus.js"; import { SESSIOND_RUNTIME_CAPABILITIES } from "../shared/capabilities.js"; -import { agentSessionDirEnvKeys, effectivePiWebConfig, maxUploadBytes } from "../config.js"; +import { agentSessionDirEnvKeys, effectivePiWebConfig, maxUploadBytes, offlineModeEnabled } from "../config.js"; import { createActiveAgentProfileDescriptor } from "../sessiond/activeAgentProfile.js"; import { runSessionDaemonStartup } from "./sessiond/sessionDaemonStartup.js"; @@ -58,8 +58,13 @@ await runSessionDaemonStartup({ await bootstrapAndFreezeGlobalExtensionProviders(auth.runtime, activeAgentProfile.dir, app.log); // The shared model runtime is constructed offline so request paths never // wait on provider-catalog fetches; this is the single bounded network - // refresher, and auth changes (login/logout) ask it for a prompt run. - const catalogRefresher = new ModelCatalogRefresher({ runtime: auth.runtime, logger: app.log }); + // refresher, and auth changes (login/logout) ask it for a prompt run. It + // stays fully inert when the operator asked for offline behavior. + const catalogRefresher = new ModelCatalogRefresher({ + runtime: auth.runtime, + logger: app.log, + offline: offlineModeEnabled(daemonEnvironment), + }); catalogRefresher.start(); auth.subscribe(() => { catalogRefresher.requestRefresh(); }); const spawnTargets = config.spawnSessions diff --git a/src/server/sessions/modelCatalogRefresher.test.ts b/src/server/sessions/modelCatalogRefresher.test.ts index c311bb8..0b2aed8 100644 --- a/src/server/sessions/modelCatalogRefresher.test.ts +++ b/src/server/sessions/modelCatalogRefresher.test.ts @@ -31,9 +31,10 @@ function createRuntime() { } function createLogger() { + const info = vi.fn(); const warn = vi.fn(); const error = vi.fn(); - return { logger: { warn, error }, warn, error }; + return { logger: { info, warn, error }, info, warn, error }; } /** Let a started refresh run to completion, including the finally-queue bookkeeping. */ @@ -123,6 +124,21 @@ describe("ModelCatalogRefresher", () => { expect(refresh).toHaveBeenCalledOnce(); }); + it("never touches the network when offline mode is enabled", async () => { + const runtime = createRuntime(); + const { logger, info } = createLogger(); + const refresher = new ModelCatalogRefresher({ runtime, logger, offline: true, initialDelayMs: 1_000, intervalMs: 60_000 }); + + refresher.start(); + await vi.advanceTimersByTimeAsync(300_000); + refresher.requestRefresh(); + await flushMicrotasks(); + + expect(runtime.refresh).not.toHaveBeenCalled(); + expect(info).toHaveBeenCalledWith({}, "offline mode is enabled; skipping background model catalog refreshes"); + refresher.dispose(); + }); + it("warns and keeps going when a refresh is aborted by its timeout", async () => { const { logger, warn, error } = createLogger(); const refresh = vi.fn(() => Promise.resolve({ aborted: true, errors: new Map() })); diff --git a/src/server/sessions/modelCatalogRefresher.ts b/src/server/sessions/modelCatalogRefresher.ts index 7d4ad54..b38bd5a 100644 --- a/src/server/sessions/modelCatalogRefresher.ts +++ b/src/server/sessions/modelCatalogRefresher.ts @@ -10,8 +10,9 @@ const DEFAULT_INITIAL_DELAY_MS = 15_000; /** Bound every catalog refresh so a stalled provider fetch can never block for minutes. */ const DEFAULT_TIMEOUT_MS = 15_000; -/** Minimal structured-logging seam for non-fatal refresh failures. */ +/** Minimal structured-logging seam for refresh lifecycle and non-fatal failures. */ export interface ModelCatalogRefresherLogger { + info(details: Record, message: string): void; warn(details: Record, message: string): void; error(details: Record, message: string): void; } @@ -19,12 +20,19 @@ export interface ModelCatalogRefresherLogger { export interface ModelCatalogRefresherOptions { runtime: Pick; logger?: ModelCatalogRefresherLogger; + /** + * When true the operator asked for offline behavior, so no network refresh is + * ever attempted. Injected from the daemon environment instead of read from + * `process.env` here, so the decision stays explicit and testable. + */ + offline?: boolean; intervalMs?: number; initialDelayMs?: number; timeoutMs?: number; } const noopLogger: ModelCatalogRefresherLogger = { + info() { /* no-op */ }, warn() { /* no-op */ }, error() { /* no-op */ }, }; @@ -38,10 +46,15 @@ const noopLogger: ModelCatalogRefresherLogger = { * bounded by an abort timeout, serialized through one in-flight run, and off * any request path. `requestRefresh()` additionally asks for a prompt refresh * after events that change what should be listed, such as provider logins. + * + * When the operator asked for offline behavior (`PI_OFFLINE` / `PI_WEB_OFFLINE`), + * the refresher performs no network I/O at all and the cached catalogs in + * models-store.json are used as they are. */ export class ModelCatalogRefresher { private readonly runtime: Pick; private readonly logger: ModelCatalogRefresherLogger; + private readonly offline: boolean; private readonly intervalMs: number; private readonly initialDelayMs: number; private readonly timeoutMs: number; @@ -54,6 +67,7 @@ export class ModelCatalogRefresher { constructor(options: ModelCatalogRefresherOptions) { this.runtime = options.runtime; this.logger = options.logger ?? noopLogger; + this.offline = options.offline ?? false; this.intervalMs = options.intervalMs ?? DEFAULT_INTERVAL_MS; this.initialDelayMs = options.initialDelayMs ?? DEFAULT_INITIAL_DELAY_MS; this.timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; @@ -61,15 +75,22 @@ export class ModelCatalogRefresher { start(): void { if (this.disposed) return; + if (this.offline) { + this.logger.info({}, "offline mode is enabled; skipping background model catalog refreshes"); + return; + } this.initialTimer = setTimeout(() => { this.requestRefresh(); }, this.initialDelayMs); this.initialTimer.unref(); this.intervalTimer = setInterval(() => { this.requestRefresh(); }, this.intervalMs); this.intervalTimer.unref(); } - /** Ask for a refresh, coalescing concurrent and overlapping requests. */ + /** + * Ask for a refresh, coalescing concurrent and overlapping requests. A no-op + * in offline mode, so auth changes never trigger network I/O either. + */ requestRefresh(): void { - if (this.disposed) return; + if (this.disposed || this.offline) return; if (this.inflight !== undefined) { this.queued = true; return; From 3d3538c76bd12c0688323f4f897fd2590f141748 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sat, 25 Jul 2026 12:48:15 +0200 Subject: [PATCH 3/6] fix(sessions): size catalog refresh cadence, force, and retries Tick the background catalog refresher hourly instead of every four hours: pi stamps `checkedAt` after a fetch completes, so a tick at exactly its 4h freshness window always landed a few seconds short and only fetched on every other tick (~8h effective). Scheduled runs stay unforced, so the extra ticks are nearly free and pi's gate keeps deciding when to fetch. Auth-triggered refreshes now pass `force: true` so a re-login of a provider refreshed within the last four hours actually reaches the network. A request queued behind an in-flight run keeps the strongest mode asked for, so a forced request is never downgraded. Raise the whole-cycle timeout to 60s, since one run covers every refreshable provider and a background job has no startup budget, and give a timed-out or errored run exactly one bounded retry. Retries never earn retries, are superseded by any fresh request, and are cleared by `dispose()`. --- .../sessions/modelCatalogRefresher.test.ts | 144 ++++++++++++++++++ src/server/sessions/modelCatalogRefresher.ts | 135 ++++++++++++---- 2 files changed, 250 insertions(+), 29 deletions(-) diff --git a/src/server/sessions/modelCatalogRefresher.test.ts b/src/server/sessions/modelCatalogRefresher.test.ts index 0b2aed8..f91a0a3 100644 --- a/src/server/sessions/modelCatalogRefresher.test.ts +++ b/src/server/sessions/modelCatalogRefresher.test.ts @@ -3,6 +3,7 @@ import { ModelCatalogRefresher } from "./modelCatalogRefresher.js"; interface RefreshCall { allowNetwork?: boolean; + force?: boolean; signal?: AbortSignal; } @@ -12,6 +13,7 @@ interface RefreshResult { } const okResult = (): RefreshResult => ({ aborted: false, errors: new Map() }); +const abortedResult = (): RefreshResult => ({ aborted: true, errors: new Map() }); function deferred() { let resolveValue: (value: T) => void = () => undefined; @@ -65,6 +67,67 @@ describe("ModelCatalogRefresher", () => { refresher.dispose(); }); + it("forces auth-triggered refreshes past pi's freshness gate", async () => { + const runtime = createRuntime(); + const refresher = new ModelCatalogRefresher({ runtime }); + + refresher.requestRefresh(); + await flushMicrotasks(); + + expect(runtime.calls.at(0)?.force).toBe(true); + refresher.dispose(); + }); + + it("leaves scheduled refreshes unforced so pi's freshness gate stays in charge", async () => { + const runtime = createRuntime(); + const refresher = new ModelCatalogRefresher({ runtime, initialDelayMs: 1_000, intervalMs: 60_000 }); + refresher.start(); + + await vi.advanceTimersByTimeAsync(61_000); + + expect(runtime.refresh).toHaveBeenCalledTimes(2); + expect(runtime.calls.map((call) => call.force)).toEqual([false, false]); + refresher.dispose(); + }); + + it("keeps a forced request forced when it is queued behind a scheduled run", async () => { + const gate = deferred(); + const calls: RefreshCall[] = []; + const refresh = vi.fn((options?: RefreshCall) => { + calls.push(options ?? {}); + return calls.length === 1 ? gate.promise : Promise.resolve(okResult()); + }); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, initialDelayMs: 1_000, intervalMs: 60_000 }); + refresher.start(); + + await vi.advanceTimersByTimeAsync(1_000); + expect(calls.at(0)?.force).toBe(false); + + refresher.requestRefresh(); + gate.resolve(okResult()); + await flushMicrotasks(); + + expect(refresh).toHaveBeenCalledTimes(2); + expect(calls.at(1)?.force).toBe(true); + refresher.dispose(); + }); + + it("refreshes well within pi's four-hour freshness window", async () => { + const fourHoursMs = 4 * 60 * 60 * 1000; + const runtime = createRuntime(); + // Defaults matter here: the bug this pins is a scheduled interval that lands + // just short of pi's TTL and therefore only fetches on every other tick. + const refresher = new ModelCatalogRefresher({ runtime }); + refresher.start(); + + await vi.advanceTimersByTimeAsync(fourHoursMs); + + // Ticks are cheap because scheduled runs never force, so several land inside + // one TTL window and at least one is guaranteed to be past the gate. + expect(runtime.refresh.mock.calls.length).toBeGreaterThan(2); + refresher.dispose(); + }); + it("coalesces overlapping requests into a single follow-up run", async () => { const gate = deferred(); const refresh = vi.fn() @@ -124,6 +187,87 @@ describe("ModelCatalogRefresher", () => { expect(refresh).toHaveBeenCalledOnce(); }); + it("retries once after an aborted run and then waits for the schedule", async () => { + const { logger, info } = createLogger(); + const refresh = vi.fn() + .mockResolvedValueOnce(abortedResult()) + .mockResolvedValueOnce(abortedResult()) + .mockResolvedValue(okResult()); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, logger, retryDelayMs: 30_000, intervalMs: 3_600_000 }); + + refresher.requestRefresh(); + await flushMicrotasks(); + expect(refresh).toHaveBeenCalledOnce(); + expect(info).toHaveBeenCalledWith({ retryDelayMs: 30_000, mode: "forced" }, "scheduling one model catalog refresh retry"); + + await vi.advanceTimersByTimeAsync(30_000); + expect(refresh).toHaveBeenCalledTimes(2); + + // The retry also failed, but a retry never earns another retry. + await vi.advanceTimersByTimeAsync(300_000); + expect(refresh).toHaveBeenCalledTimes(2); + refresher.dispose(); + }); + + it("retries once after a run reports provider errors", async () => { + const errors = new Map([["openrouter", new Error("boom")]]); + const refresh = vi.fn() + .mockResolvedValueOnce({ aborted: false, errors }) + .mockResolvedValue(okResult()); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, retryDelayMs: 30_000 }); + + refresher.requestRefresh(); + await flushMicrotasks(); + + await vi.advanceTimersByTimeAsync(30_000); + expect(refresh).toHaveBeenCalledTimes(2); + refresher.dispose(); + }); + + it("does not schedule a retry after a successful run", async () => { + const runtime = createRuntime(); + const refresher = new ModelCatalogRefresher({ runtime, retryDelayMs: 30_000, intervalMs: 3_600_000 }); + + refresher.requestRefresh(); + await flushMicrotasks(); + await vi.advanceTimersByTimeAsync(300_000); + + expect(runtime.refresh).toHaveBeenCalledOnce(); + refresher.dispose(); + }); + + it("drops a pending retry when dispose happens first", async () => { + const refresh = vi.fn() + .mockResolvedValueOnce(abortedResult()) + .mockResolvedValue(okResult()); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, retryDelayMs: 30_000 }); + + refresher.requestRefresh(); + await flushMicrotasks(); + refresher.dispose(); + + await vi.advanceTimersByTimeAsync(300_000); + expect(refresh).toHaveBeenCalledOnce(); + }); + + it("lets a new request supersede a pending retry instead of running both", async () => { + const refresh = vi.fn() + .mockResolvedValueOnce(abortedResult()) + .mockResolvedValue(okResult()); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, retryDelayMs: 30_000 }); + + refresher.requestRefresh(); + await flushMicrotasks(); + + refresher.requestRefresh(); + await flushMicrotasks(); + expect(refresh).toHaveBeenCalledTimes(2); + + await vi.advanceTimersByTimeAsync(300_000); + expect(refresh).toHaveBeenCalledTimes(2); + refresher.dispose(); + }); + it("never touches the network when offline mode is enabled", async () => { const runtime = createRuntime(); const { logger, info } = createLogger(); diff --git a/src/server/sessions/modelCatalogRefresher.ts b/src/server/sessions/modelCatalogRefresher.ts index b38bd5a..6f6ac7d 100644 --- a/src/server/sessions/modelCatalogRefresher.ts +++ b/src/server/sessions/modelCatalogRefresher.ts @@ -1,14 +1,42 @@ import type { ModelRuntime } from "@earendil-works/pi-coding-agent"; /** - * Matches pi's REMOTE_CATALOG_REFRESH_INTERVAL_MS: provider catalog entries in - * models-store.json are treated as fresh for four hours. + * Tick hourly and let pi decide when a fetch is actually due: pi treats stored + * catalogs as fresh for REMOTE_CATALOG_REFRESH_INTERVAL_MS (4h) and skips the + * network entirely on an unforced refresh inside that window. Ticking at + * exactly 4h would land a few seconds short of the window every time, because + * pi stamps `checkedAt` only after a fetch completes, so every other tick would + * be skipped and the real cadence would be ~8h. A shorter interval removes that + * off-by-one-latency skip, tolerates clock skew, and costs nothing when the + * cache is fresh, because scheduled runs never set `force`. */ -const DEFAULT_INTERVAL_MS = 4 * 60 * 60 * 1000; +const DEFAULT_INTERVAL_MS = 60 * 60 * 1000; /** Give the daemon a moment to finish startup before the first network refresh. */ const DEFAULT_INITIAL_DELAY_MS = 15_000; -/** Bound every catalog refresh so a stalled provider fetch can never block for minutes. */ -const DEFAULT_TIMEOUT_MS = 15_000; +/** + * Bound every catalog refresh so a stalled provider fetch cannot run forever. + * One run covers every refreshable provider under a single signal, so this is a + * whole-cycle budget for a background job, not pi's 15s startup budget. + */ +const DEFAULT_TIMEOUT_MS = 60_000; +/** + * Wait this long before the single follow-up attempt that a timed-out or failed + * run earns, so a transient network problem does not cost a whole interval. + */ +const DEFAULT_RETRY_DELAY_MS = 5 * 60 * 1000; + +/** + * Scheduled runs defer to pi's freshness gate; forced runs bypass it because + * the caller knows the cached catalog is wrong (for example after a login). + */ +type RefreshMode = "scheduled" | "forced"; + +/** A queued follow-up must keep the strongest mode requested while a run was in flight. */ +const strongestMode = (left: RefreshMode | undefined, right: RefreshMode): RefreshMode => + left === "forced" || right === "forced" ? "forced" : "scheduled"; + +/** Whether a run finished with a complete picture, or earned a retry. */ +type RunOutcome = "complete" | "incomplete"; /** Minimal structured-logging seam for refresh lifecycle and non-fatal failures. */ export interface ModelCatalogRefresherLogger { @@ -29,6 +57,7 @@ export interface ModelCatalogRefresherOptions { intervalMs?: number; initialDelayMs?: number; timeoutMs?: number; + retryDelayMs?: number; } const noopLogger: ModelCatalogRefresherLogger = { @@ -44,8 +73,9 @@ const noopLogger: ModelCatalogRefresherLogger = { * own refreshes never touch the network and stay fast on request paths. This * refresher is the single place that deliberately performs network refreshes — * bounded by an abort timeout, serialized through one in-flight run, and off - * any request path. `requestRefresh()` additionally asks for a prompt refresh - * after events that change what should be listed, such as provider logins. + * any request path. `requestRefresh()` additionally asks for a prompt forced + * refresh after events that change what should be listed, such as provider + * logins, where the cached catalog is known to be wrong. * * When the operator asked for offline behavior (`PI_OFFLINE` / `PI_WEB_OFFLINE`), * the refresher performs no network I/O at all and the cached catalogs in @@ -58,10 +88,12 @@ export class ModelCatalogRefresher { private readonly intervalMs: number; private readonly initialDelayMs: number; private readonly timeoutMs: number; + private readonly retryDelayMs: number; private initialTimer?: NodeJS.Timeout; private intervalTimer?: NodeJS.Timeout; - private inflight: Promise | undefined; - private queued = false; + private retryTimer: NodeJS.Timeout | undefined; + private inflight: Promise | undefined; + private queuedMode: RefreshMode | undefined; private disposed = false; constructor(options: ModelCatalogRefresherOptions) { @@ -71,6 +103,7 @@ export class ModelCatalogRefresher { this.intervalMs = options.intervalMs ?? DEFAULT_INTERVAL_MS; this.initialDelayMs = options.initialDelayMs ?? DEFAULT_INITIAL_DELAY_MS; this.timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + this.retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; } start(): void { @@ -79,42 +112,84 @@ export class ModelCatalogRefresher { this.logger.info({}, "offline mode is enabled; skipping background model catalog refreshes"); return; } - this.initialTimer = setTimeout(() => { this.requestRefresh(); }, this.initialDelayMs); + this.initialTimer = setTimeout(() => { this.queueRefresh("scheduled"); }, this.initialDelayMs); this.initialTimer.unref(); - this.intervalTimer = setInterval(() => { this.requestRefresh(); }, this.intervalMs); + this.intervalTimer = setInterval(() => { this.queueRefresh("scheduled"); }, this.intervalMs); this.intervalTimer.unref(); } /** - * Ask for a refresh, coalescing concurrent and overlapping requests. A no-op - * in offline mode, so auth changes never trigger network I/O either. + * Ask for an immediate refresh that bypasses pi's freshness gate, for callers + * that know the cached catalog is stale (auth changes). Coalesces with + * concurrent and overlapping requests, and is a no-op in offline mode so auth + * changes never trigger network I/O either. */ requestRefresh(): void { - if (this.disposed || this.offline) return; - if (this.inflight !== undefined) { - this.queued = true; - return; - } - const run = this.run(); - this.inflight = run; - this.inflight.finally(() => { - this.inflight = undefined; - if (this.queued && !this.disposed) { - this.queued = false; - this.requestRefresh(); - } - }).catch(() => { /* run() never rejects; finally() re-throws otherwise */ }); + this.queueRefresh("forced"); } dispose(): void { this.disposed = true; if (this.initialTimer !== undefined) clearTimeout(this.initialTimer); if (this.intervalTimer !== undefined) clearInterval(this.intervalTimer); + this.clearRetryTimer(); } - private async run(): Promise { + /** + * Single entry point for every refresh trigger. Only one run is ever in + * flight; overlapping requests collapse into one follow-up that keeps the + * strongest mode asked for, so a forced request is never downgraded. + */ + private queueRefresh(mode: RefreshMode, isRetry = false): void { + if (this.disposed || this.offline) return; + // Any fresh trigger supersedes a pending retry, which keeps retries from + // stacking up behind normal activity. + if (!isRetry) this.clearRetryTimer(); + if (this.inflight !== undefined) { + this.queuedMode = strongestMode(this.queuedMode, mode); + return; + } + const run = this.run(mode); + this.inflight = run; + run.then((outcome) => { + this.inflight = undefined; + const queued = this.queuedMode; + this.queuedMode = undefined; + if (queued !== undefined) { + // A request that arrived during the run replaces any retry this run + // would have earned; it runs now instead. + this.queueRefresh(queued); + return; + } + // Only a first attempt earns a retry, so a failing provider can never + // turn into a retry loop. + if (outcome === "incomplete" && !isRetry) this.scheduleRetry(mode); + }).catch(() => { /* run() reports failures through its logger and never rejects */ }); + } + + private scheduleRetry(mode: RefreshMode): void { + if (this.disposed) return; + this.logger.info({ retryDelayMs: this.retryDelayMs, mode }, "scheduling one model catalog refresh retry"); + this.retryTimer = setTimeout(() => { + this.retryTimer = undefined; + this.queueRefresh(mode, true); + }, this.retryDelayMs); + this.retryTimer.unref(); + } + + private clearRetryTimer(): void { + if (this.retryTimer === undefined) return; + clearTimeout(this.retryTimer); + this.retryTimer = undefined; + } + + private async run(mode: RefreshMode): Promise { try { - const result = await this.runtime.refresh({ allowNetwork: true, signal: AbortSignal.timeout(this.timeoutMs) }); + const result = await this.runtime.refresh({ + allowNetwork: true, + force: mode === "forced", + signal: AbortSignal.timeout(this.timeoutMs), + }); if (result.aborted) { this.logger.warn({ timeoutMs: this.timeoutMs }, "model catalog refresh timed out; keeping cached catalogs"); } @@ -122,8 +197,10 @@ export class ModelCatalogRefresher { const providers = [...result.errors.entries()].map(([providerId, error]) => `${providerId}: ${error.message}`); this.logger.warn({ providers }, "model catalog refresh failed for some providers; keeping cached catalogs"); } + return result.aborted || result.errors.size > 0 ? "incomplete" : "complete"; } catch (error: unknown) { this.logger.error({ err: error }, "model catalog refresh failed; keeping cached catalogs"); + return "incomplete"; } } } From c5af390ab9ec30d414a351740f46175d4d16dbc4 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sat, 25 Jul 2026 12:56:31 +0200 Subject: [PATCH 4/6] fix(sessions): abort in-flight catalog refresh on dispose `dispose()` only cleared timers, so a refresh already in flight kept its provider fetch alive for the rest of the timeout budget and could delay daemon shutdown, which is exactly when sessiond disposes the refresher. A refresher-lifetime AbortController is now combined with the per-run timeout via `AbortSignal.any`, and `dispose()` aborts it. A dispose-triggered abort logs as expected shutdown info rather than a timeout warning or an error, whether the runtime resolves as aborted or rejects. `start()` is now idempotent: a second call previously overwrote both timer handles and leaked the first pair, which kept firing. Also replaces the `then().catch()` bookkeeping chain in `queueRefresh()` with an awaited private `runCycle()`, keeping the coalescing, retry, and dispose semantics unchanged. --- .../sessions/modelCatalogRefresher.test.ts | 61 +++++++++++++++++ src/server/sessions/modelCatalogRefresher.ts | 68 +++++++++++++------ 2 files changed, 107 insertions(+), 22 deletions(-) diff --git a/src/server/sessions/modelCatalogRefresher.test.ts b/src/server/sessions/modelCatalogRefresher.test.ts index f91a0a3..85c95a4 100644 --- a/src/server/sessions/modelCatalogRefresher.test.ts +++ b/src/server/sessions/modelCatalogRefresher.test.ts @@ -173,6 +173,67 @@ describe("ModelCatalogRefresher", () => { expect(runtime.refresh).not.toHaveBeenCalled(); }); + it("aborts the in-flight refresh when disposed", async () => { + const { logger, info, warn, error } = createLogger(); + let observed: AbortSignal | undefined; + // Stand in for a provider fetch that only settles when its signal aborts, + // which is what makes an unaborted run delay daemon shutdown. + const refresh = vi.fn((options?: RefreshCall) => { + observed = options?.signal; + return new Promise((resolve) => { + options?.signal?.addEventListener("abort", () => { resolve(abortedResult()); }); + }); + }); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, logger }); + + refresher.requestRefresh(); + await flushMicrotasks(); + expect(observed?.aborted).toBe(false); + + refresher.dispose(); + await flushMicrotasks(); + + expect(observed?.aborted).toBe(true); + // A deliberate shutdown abort is expected, not a timeout or a fault. + expect(info).toHaveBeenCalledWith({}, "model catalog refresh aborted by dispose; keeping cached catalogs"); + expect(warn).not.toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); + }); + + it("does not report a refresh that rejects because dispose aborted it as a failure", async () => { + const { logger, info, warn, error } = createLogger(); + const refresh = vi.fn((options?: RefreshCall) => new Promise((_resolve, reject) => { + options?.signal?.addEventListener("abort", () => { reject(new Error("This operation was aborted")); }); + })); + const refresher = new ModelCatalogRefresher({ runtime: { refresh }, logger }); + + refresher.requestRefresh(); + await flushMicrotasks(); + refresher.dispose(); + await flushMicrotasks(); + + expect(info).toHaveBeenCalledWith({}, "model catalog refresh aborted by dispose; keeping cached catalogs"); + expect(warn).not.toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); + }); + + it("keeps the timers of the first start when start is called again", async () => { + const runtime = createRuntime(); + const refresher = new ModelCatalogRefresher({ runtime, initialDelayMs: 1_000, intervalMs: 60_000 }); + + refresher.start(); + refresher.start(); + + await vi.advanceTimersByTimeAsync(61_000); + + // A leaked second timer pair would double every scheduled refresh. + expect(runtime.refresh).toHaveBeenCalledTimes(2); + + refresher.dispose(); + await vi.advanceTimersByTimeAsync(120_000); + expect(runtime.refresh).toHaveBeenCalledTimes(2); + }); + it("does not run a queued follow-up after dispose", async () => { const gate = deferred(); const refresh = vi.fn().mockImplementation(() => gate.promise); diff --git a/src/server/sessions/modelCatalogRefresher.ts b/src/server/sessions/modelCatalogRefresher.ts index 6f6ac7d..a3c191e 100644 --- a/src/server/sessions/modelCatalogRefresher.ts +++ b/src/server/sessions/modelCatalogRefresher.ts @@ -72,8 +72,8 @@ const noopLogger: ModelCatalogRefresherLogger = { * The shared ModelRuntime is constructed offline (see authService.ts), so its * own refreshes never touch the network and stay fast on request paths. This * refresher is the single place that deliberately performs network refreshes — - * bounded by an abort timeout, serialized through one in-flight run, and off - * any request path. `requestRefresh()` additionally asks for a prompt forced + * bounded by an abort timeout, serialized through one in-flight run, stopped by + * `dispose()` even mid-flight, and off any request path. `requestRefresh()` additionally asks for a prompt forced * refresh after events that change what should be listed, such as provider * logins, where the cached catalog is known to be wrong. * @@ -92,9 +92,16 @@ export class ModelCatalogRefresher { private initialTimer?: NodeJS.Timeout; private intervalTimer?: NodeJS.Timeout; private retryTimer: NodeJS.Timeout | undefined; - private inflight: Promise | undefined; + private inflight: Promise | undefined; private queuedMode: RefreshMode | undefined; + private started = false; private disposed = false; + /** + * Aborted by `dispose()` so a refresh already in flight stops with the + * refresher instead of holding its fetch open for the rest of the timeout + * budget, which would delay daemon shutdown. + */ + private readonly lifetime = new AbortController(); constructor(options: ModelCatalogRefresherOptions) { this.runtime = options.runtime; @@ -106,8 +113,10 @@ export class ModelCatalogRefresher { this.retryDelayMs = options.retryDelayMs ?? DEFAULT_RETRY_DELAY_MS; } + /** Idempotent: a second call keeps the timers the first call installed. */ start(): void { - if (this.disposed) return; + if (this.disposed || this.started) return; + this.started = true; if (this.offline) { this.logger.info({}, "offline mode is enabled; skipping background model catalog refreshes"); return; @@ -128,11 +137,13 @@ export class ModelCatalogRefresher { this.queueRefresh("forced"); } + /** Terminal: stops the schedule, drops any queued follow-up, and aborts an in-flight run. */ dispose(): void { this.disposed = true; if (this.initialTimer !== undefined) clearTimeout(this.initialTimer); if (this.intervalTimer !== undefined) clearInterval(this.intervalTimer); this.clearRetryTimer(); + this.lifetime.abort(); } /** @@ -149,22 +160,25 @@ export class ModelCatalogRefresher { this.queuedMode = strongestMode(this.queuedMode, mode); return; } - const run = this.run(mode); - this.inflight = run; - run.then((outcome) => { - this.inflight = undefined; - const queued = this.queuedMode; - this.queuedMode = undefined; - if (queued !== undefined) { - // A request that arrived during the run replaces any retry this run - // would have earned; it runs now instead. - this.queueRefresh(queued); - return; - } - // Only a first attempt earns a retry, so a failing provider can never - // turn into a retry loop. - if (outcome === "incomplete" && !isRetry) this.scheduleRetry(mode); - }).catch(() => { /* run() reports failures through its logger and never rejects */ }); + // runCycle() reports every failure through the logger and never rejects. + this.inflight = this.runCycle(mode, isRetry); + } + + /** One run plus the follow-up it leads to: a queued request, a single retry, or nothing. */ + private async runCycle(mode: RefreshMode, isRetry: boolean): Promise { + const outcome = await this.run(mode); + this.inflight = undefined; + const queued = this.queuedMode; + this.queuedMode = undefined; + if (queued !== undefined) { + // A request that arrived during the run replaces any retry this run would + // have earned; it runs now instead. + this.queueRefresh(queued); + return; + } + // Only a first attempt earns a retry, so a failing provider can never turn + // into a retry loop. + if (outcome === "incomplete" && !isRetry) this.scheduleRetry(mode); } private scheduleRetry(mode: RefreshMode): void { @@ -188,8 +202,12 @@ export class ModelCatalogRefresher { const result = await this.runtime.refresh({ allowNetwork: true, force: mode === "forced", - signal: AbortSignal.timeout(this.timeoutMs), + signal: AbortSignal.any([this.lifetime.signal, AbortSignal.timeout(this.timeoutMs)]), }); + if (result.aborted && this.lifetime.signal.aborted) { + this.logStoppedByDispose(); + return "incomplete"; + } if (result.aborted) { this.logger.warn({ timeoutMs: this.timeoutMs }, "model catalog refresh timed out; keeping cached catalogs"); } @@ -199,8 +217,14 @@ export class ModelCatalogRefresher { } return result.aborted || result.errors.size > 0 ? "incomplete" : "complete"; } catch (error: unknown) { - this.logger.error({ err: error }, "model catalog refresh failed; keeping cached catalogs"); + if (this.lifetime.signal.aborted) this.logStoppedByDispose(); + else this.logger.error({ err: error }, "model catalog refresh failed; keeping cached catalogs"); return "incomplete"; } } + + /** A shutdown abort is expected, so it must not be reported as a timeout or a fault. */ + private logStoppedByDispose(): void { + this.logger.info({}, "model catalog refresh aborted by dispose; keeping cached catalogs"); + } } From 90dd7ce401ebb7d0f71cfff36d7f87294595ff37 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sat, 25 Jul 2026 13:07:02 +0200 Subject: [PATCH 5/6] docs(sessions): document background catalog refresh and contain offline env window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding 6: serialize createOfflineModelRuntime so overlapping calls cannot interleave their PI_OFFLINE save/restore pairs and leave the process offline, and name the process-wide visibility of that window in the docblock. Finding 7: assert the offline construction through the public refresh seam via reloadConfig() — the request path that regressed — instead of reading upstream's private modelNetworkEnabled field. Finding 8.4/8.5: document the background provider-catalog refresh in docs/config.md and docs/config.html (cadence, timeout, single retry, offline opt-out via PI_WEB_OFFLINE / PI_OFFLINE only), and update the changeset to match the behavior after the earlier fixes. --- .changeset/bounded-model-catalog-refresh.md | 2 +- docs/config.html | 51 +++++++++++++++++++++ docs/config.md | 16 +++++++ src/server/sessions/authService.test.ts | 40 +++++++++++++--- src/server/sessions/authService.ts | 33 ++++++++++++- 5 files changed, 132 insertions(+), 10 deletions(-) diff --git a/.changeset/bounded-model-catalog-refresh.md b/.changeset/bounded-model-catalog-refresh.md index d6c9e45..10e6342 100644 --- a/.changeset/bounded-model-catalog-refresh.md +++ b/.changeset/bounded-model-catalog-refresh.md @@ -2,4 +2,4 @@ "@jmfederico/pi-web": patch --- -Fix multi-minute stalls when opening the model selector, starting sessions, or using the auth dialogs. Provider catalog refreshes no longer run unbounded network fetches on request paths: the shared model runtime now operates offline and pi-web refreshes provider catalogs itself on a bounded, background schedule and after provider logins. +Fix multi-minute stalls when opening the model selector, starting sessions, or using the auth dialogs. Provider model catalogs are no longer fetched on request paths: the session daemon now refreshes them in the background on a bounded schedule — shortly after startup and hourly, plus immediately after a provider login or logout — with a per-run timeout and a single retry, keeping the stored catalogs when a provider fails. Setting `PI_WEB_OFFLINE` or `PI_OFFLINE` disables these background refreshes entirely. See the configuration reference for details. diff --git a/docs/config.html b/docs/config.html index f814d69..2c327da 100644 --- a/docs/config.html +++ b/docs/config.html @@ -100,6 +100,7 @@ Manual uploads Agent profile and companion CLI Pi extension providers + Model catalog refresh Session tools Completion tools @@ -479,6 +480,17 @@ Not supported locally Restart web/API after env changes + + Offline mode + — + PI_WEB_OFFLINE, PI_OFFLINE + Web/API + session daemon env + Not supported locally + + Restart session daemon and web/API after env changes; also disables the + background model catalog refresh + + @@ -642,6 +654,45 @@ +
+

Background model catalog refresh

+

+ PI WEB shares one model runtime across all sessions, and provider model catalogs are refreshed over the + network only on the session daemon's own background schedule. Nothing a browser or API request triggers + waits on a provider catalog fetch, so a slow or unreachable provider cannot stall opening the model + selector, starting a session, or the auth dialogs. +

+

The session daemon runs the refresh:

+
    +
  • + 15 seconds after the daemon starts, then hourly. Pi treats stored + catalogs as fresh for four hours, so most hourly ticks make no network request at all; the shorter tick + only makes sure a due refresh is not delayed to the next tick. +
    • +
  • + Immediately after a provider login or logout, bypassing that freshness window, because + the cached catalog is known to be wrong. +
    • +
+

+ Each run is bounded: it is aborted after 60 seconds, and a run that times out or fails + earns one retry after five minutes. Failures never clear the stored catalogs — the last + successfully fetched models stay in use and the daemon log records what failed. A refresh in flight is + also aborted when the daemon shuts down. +

+

+ Models fetched by a background refresh appear the next time a client asks for the model list, so a model + selector left open across a refresh may need to be reopened. +

+

+ To turn the background refresh off entirely, set PI_WEB_OFFLINE or PI_OFFLINE in + the session daemon's environment and restart it. In offline mode PI WEB performs no provider catalog + network requests, including after logins, and sessions use the catalogs already stored in the agent + profile. The PI_WEB_SKIP_VERSION_CHECK and PI_SKIP_VERSION_CHECK keys do + not affect this refresh; they only suppress PI WEB release checks. +

+
+

Session daemon tools

spawnSessions

diff --git a/docs/config.md b/docs/config.md index 90b9d85..efc68d1 100644 --- a/docs/config.md +++ b/docs/config.md @@ -131,6 +131,7 @@ Rows with JSON key `—` are runtime-only environment variables, not config-file | Agent profile session storage directory | — | `PI_WEB_AGENT_SESSION_DIR` (`PI_CODING_AGENT_SESSION_DIR` for Pi compatibility) | Session daemon env | Not supported locally | Restart session daemon; env-only session storage override | | Agent profile state directory | — | `PI_WEB_AGENT_DIR` (`PI_CODING_AGENT_DIR` for Pi compatibility) | Web/API + session daemon env | Not supported locally | Restart services | | Skip update checks | — | `PI_WEB_SKIP_VERSION_CHECK`, `PI_WEB_OFFLINE`, `PI_SKIP_VERSION_CHECK`, `PI_OFFLINE` | Web/API env | Not supported locally | Restart web/API after env changes | +| Offline mode | — | `PI_WEB_OFFLINE`, `PI_OFFLINE` | Web/API + session daemon env | Not supported locally | Restart session daemon and web/API after env changes; also disables the [background model catalog refresh](#background-model-catalog-refresh) | ## Key details @@ -218,6 +219,21 @@ Ignored mutations are written to the session-daemon log once per operation and p Configure providers before the daemon starts: use the active agent directory's `models.json`, or install the Pi extension globally in that agent profile. Project Pi extensions and project-level `models.json` files cannot add providers to PI WEB's shared baseline. After updating PI WEB—or after installing, removing, or updating a global Pi extension that registers providers—manually restart `pi-web-sessiond.service` (`systemctl --user restart pi-web-sessiond`). Restarting only the web/API service and running `/reload` do not rebuild the baseline. +### Background model catalog refresh + +PI WEB shares one model runtime across all sessions, and provider model catalogs are refreshed over the network only on the session daemon's own background schedule. Nothing a browser or API request triggers waits on a provider catalog fetch, so a slow or unreachable provider cannot stall opening the model selector, starting a session, or the auth dialogs. + +The session daemon runs the refresh: + +- **15 seconds after the daemon starts**, then **hourly**. Pi treats stored catalogs as fresh for four hours, so most hourly ticks make no network request at all; the shorter tick only makes sure a due refresh is not delayed to the next tick. +- **Immediately after a provider login or logout**, bypassing that freshness window, because the cached catalog is known to be wrong. + +Each run is bounded: it is aborted after **60 seconds**, and a run that times out or fails earns **one retry after five minutes**. Failures never clear the stored catalogs — the last successfully fetched models stay in use and the daemon log records what failed. A refresh in flight is also aborted when the daemon shuts down. + +Models fetched by a background refresh appear the next time a client asks for the model list, so a model selector left open across a refresh may need to be reopened. + +To turn the background refresh off entirely, set `PI_WEB_OFFLINE` or `PI_OFFLINE` in the session daemon's environment and restart it. In offline mode PI WEB performs no provider catalog network requests, including after logins, and sessions use the catalogs already stored in the agent profile. The `PI_WEB_SKIP_VERSION_CHECK` and `PI_SKIP_VERSION_CHECK` keys do **not** affect this refresh; they only suppress PI WEB release checks. + ### Session daemon tools `spawnSessions` controls whether agents receive the `spawn_session` tool. It defaults to `true`; set it to `false` if you do not want an agent to start independent PI WEB sessions. diff --git a/src/server/sessions/authService.test.ts b/src/server/sessions/authService.test.ts index 7779966..f4fb9bc 100644 --- a/src/server/sessions/authService.test.ts +++ b/src/server/sessions/authService.test.ts @@ -471,14 +471,16 @@ describe("AuthService", () => { }); describe("createModelRuntimeForAgentDir", () => { - it("disables runtime-owned network refreshes so request paths stay local", async () => { - // The stall fix relies on this construction-time flag: reloadConfig(), - // login(), and logout() refresh with allowNetwork = modelNetworkEnabled. - // Asserting the private field is proportionate here because the flag is - // exactly the contract this change depends on. + it("keeps runtime-owned refreshes local so request paths cannot stall", async () => { + // reloadConfig() is the request-path call site that regressed: it refreshes + // with allowNetwork = the construction-time network flag and no abort signal. const agentDir = await tempAgentDir(); const runtime = await createModelRuntimeForAgentDir(agentDir); - expect(Reflect.get(runtime, "modelNetworkEnabled")).toBe(false); + const refresh = vi.spyOn(runtime, "refresh"); + + await runtime.reloadConfig(); + + expect(refresh).toHaveBeenCalledWith({ allowNetwork: false }); }); it("restores a previously set PI_OFFLINE after runtime creation", async () => { @@ -495,12 +497,36 @@ describe("createModelRuntimeForAgentDir", () => { try { const agentDir = await tempAgentDir(); const runtime = await createModelRuntimeForAgentDir(agentDir); - expect(Reflect.get(runtime, "modelNetworkEnabled")).toBe(false); + const refresh = vi.spyOn(runtime, "refresh"); + + await runtime.reloadConfig(); + + expect(refresh).toHaveBeenCalledWith({ allowNetwork: false }); expect(process.env["PI_OFFLINE"]).toBeUndefined(); } finally { if (previous !== undefined) process.env["PI_OFFLINE"] = previous; } }); + + it("restores PI_OFFLINE when creations overlap, because the env windows are serialized", async () => { + vi.unstubAllEnvs(); + const previous = process.env["PI_OFFLINE"]; + delete process.env["PI_OFFLINE"]; + try { + const dirs = await Promise.all([tempAgentDir(), tempAgentDir(), tempAgentDir()]); + const runtimes = await Promise.all(dirs.map((dir) => createModelRuntimeForAgentDir(dir))); + + // Interleaved save/restore pairs would leave PI_OFFLINE set process-wide. + expect(process.env["PI_OFFLINE"]).toBeUndefined(); + for (const runtime of runtimes) { + const refresh = vi.spyOn(runtime, "refresh"); + await runtime.reloadConfig(); + expect(refresh).toHaveBeenCalledWith({ allowNetwork: false }); + } + } finally { + if (previous !== undefined) process.env["PI_OFFLINE"] = previous; + } + }); }); async function createAuthService(seed: Record = {}, logger?: AuthServiceLogger) { diff --git a/src/server/sessions/authService.ts b/src/server/sessions/authService.ts index 6e7eaca..81e726d 100644 --- a/src/server/sessions/authService.ts +++ b/src/server/sessions/authService.ts @@ -31,6 +31,14 @@ interface AuthChangeContext { const noopLogger: AuthServiceLogger = { error() { /* no-op */ } }; +/** + * Serializes the `PI_OFFLINE` windows below, which is what keeps their + * save/restore pairs properly nested. Two overlapping calls would otherwise both + * capture the forced `"1"` and restore it, leaving the whole process offline + * permanently. Owned by this module only; nothing else may mutate it. + */ +let offlineRuntimeCreations: Promise = Promise.resolve(); + /** * Create the shared model runtime with runtime-owned network refreshes disabled. * @@ -42,12 +50,33 @@ const noopLogger: AuthServiceLogger = { error() { /* no-op */ } }; * fetch. Forcing `PI_OFFLINE` during construction makes every runtime-driven * refresh local-only; pi-web performs its own bounded catalog refreshes in the * background instead (see modelCatalogRefresher.ts). + * + * `modelNetworkEnabled` is computed once from the environment inside + * `ModelRuntime.create()`, so the env var is the only lever upstream exposes. + * Calls are queued so their env windows never overlap. */ -async function createOfflineModelRuntime(options: CreateModelRuntimeOptions): Promise { +function createOfflineModelRuntime(options: CreateModelRuntimeOptions): Promise { + const created = offlineRuntimeCreations.then(() => forceOfflineWhile(() => ModelRuntime.create(options))); + // A failed creation must not poison the queue; the caller still sees the rejection. + offlineRuntimeCreations = created.then(() => undefined, () => undefined); + return created; +} + +/** + * Force `PI_OFFLINE` for the duration of `create`, then restore what was there. + * + * `process.env` is process-wide, so this window is a real global side effect: + * anything reading `PI_OFFLINE` while `create` awaits observes offline mode, + * including upstream's package manager, tools manager, and version check. That + * is acceptable because pi-web only builds runtimes during daemon startup and in + * tests, and the window is one `ModelRuntime.create()` call — but it is why + * callers must stay serialized rather than run concurrently. + */ +async function forceOfflineWhile(create: () => Promise): Promise { const previous = process.env["PI_OFFLINE"]; process.env["PI_OFFLINE"] = "1"; try { - return await ModelRuntime.create(options); + return await create(); } finally { if (previous === undefined) delete process.env["PI_OFFLINE"]; else process.env["PI_OFFLINE"] = previous; From 5d632ef11db3667133fd794f2f1885ca7ef24949 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sat, 25 Jul 2026 13:32:21 +0200 Subject: [PATCH 6/6] test(sessions): make the offline-runtime regression test discriminating The reworked assertion ran under the file-level PI_OFFLINE=1 stub, so the runtime was offline whether or not createOfflineModelRuntime forced it and the test passed with the fix fully removed. Clear the stub for that case, and rewrap a docblock line. --- src/server/sessions/authService.test.ts | 3 +++ src/server/sessions/modelCatalogRefresher.ts | 7 ++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/server/sessions/authService.test.ts b/src/server/sessions/authService.test.ts index f4fb9bc..2cb00ce 100644 --- a/src/server/sessions/authService.test.ts +++ b/src/server/sessions/authService.test.ts @@ -474,6 +474,9 @@ describe("createModelRuntimeForAgentDir", () => { it("keeps runtime-owned refreshes local so request paths cannot stall", async () => { // reloadConfig() is the request-path call site that regressed: it refreshes // with allowNetwork = the construction-time network flag and no abort signal. + // The ambient PI_OFFLINE=1 stub has to go, or the runtime would be offline + // whether or not the helper forces it and this would assert nothing. + vi.stubEnv("PI_OFFLINE", undefined); const agentDir = await tempAgentDir(); const runtime = await createModelRuntimeForAgentDir(agentDir); const refresh = vi.spyOn(runtime, "refresh"); diff --git a/src/server/sessions/modelCatalogRefresher.ts b/src/server/sessions/modelCatalogRefresher.ts index a3c191e..b3d9f29 100644 --- a/src/server/sessions/modelCatalogRefresher.ts +++ b/src/server/sessions/modelCatalogRefresher.ts @@ -73,9 +73,10 @@ const noopLogger: ModelCatalogRefresherLogger = { * own refreshes never touch the network and stay fast on request paths. This * refresher is the single place that deliberately performs network refreshes — * bounded by an abort timeout, serialized through one in-flight run, stopped by - * `dispose()` even mid-flight, and off any request path. `requestRefresh()` additionally asks for a prompt forced - * refresh after events that change what should be listed, such as provider - * logins, where the cached catalog is known to be wrong. + * `dispose()` even mid-flight, and off any request path. `requestRefresh()` + * additionally asks for a prompt forced refresh after events that change what + * should be listed, such as provider logins, where the cached catalog is known + * to be wrong. * * When the operator asked for offline behavior (`PI_OFFLINE` / `PI_WEB_OFFLINE`), * the refresher performs no network I/O at all and the cached catalogs in