Archived
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.
This commit is contained in:
+20
-1
@@ -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 } };
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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<string, Error>() }));
|
||||
|
||||
@@ -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<string, unknown>, message: string): void;
|
||||
warn(details: Record<string, unknown>, message: string): void;
|
||||
error(details: Record<string, unknown>, message: string): void;
|
||||
}
|
||||
@@ -19,12 +20,19 @@ export interface ModelCatalogRefresherLogger {
|
||||
export interface ModelCatalogRefresherOptions {
|
||||
runtime: Pick<ModelRuntime, "refresh">;
|
||||
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<ModelRuntime, "refresh">;
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user