diff --git a/.changeset/known-provider-model-refresh.md b/.changeset/known-provider-model-refresh.md new file mode 100644 index 0000000..c725683 --- /dev/null +++ b/.changeset/known-provider-model-refresh.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Let an already-known provider extension refresh its own model list after daemon startup. Previously every provider registration made after the global bootstrap was ignored, so a provider that fetched an updated model catalog on session start never had those models appear. A registration is now applied when it matches the provider's recorded startup configuration in every respect except the model list; anything else — a new provider, a changed provider base URL, API key, API type, headers, or auth surface, a native provider registration, or an unregistration — is still ignored to keep project-level provider configuration from leaking between workspaces. Documented the refreshed policy under Pi extension provider baseline in the configuration reference. diff --git a/src/server/sessions/globalProviderPolicy.acceptance.test.ts b/src/server/sessions/globalProviderPolicy.acceptance.test.ts index ecd2ee8..57c8954 100644 --- a/src/server/sessions/globalProviderPolicy.acceptance.test.ts +++ b/src/server/sessions/globalProviderPolicy.acceptance.test.ts @@ -76,6 +76,24 @@ function providerRegistrationSource(providerId: string, variant = "baseline"): s return `pi.registerProvider(${JSON.stringify(providerId)}, ${JSON.stringify(providerConfig(providerId, variant))});`; } +/** + * A catalog refresh as real provider extensions perform it: the *complete* + * provider config is re-sent with only `models` replaced, never a delta. + */ +function catalogRefreshSource(providerId: string, refreshedModelId: string, variant = "baseline"): string { + const config = providerConfig(providerId, variant); + const models = [{ + id: refreshedModelId, + name: `${providerId} refreshed model`, + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 1_000, + maxTokens: 100, + }]; + return `pi.registerProvider(${JSON.stringify(providerId)}, ${JSON.stringify({ ...config, models })});`; +} + function nativeProviderRegistrationSource(providerId: string, variant = "baseline"): string { const baseUrl = providerBaseUrl(providerId, variant); const model = { @@ -345,7 +363,52 @@ describe("immutable global provider bootstrap acceptance", () => { await expectNoProviderMutationFeedback(service, ref); }); - it("keeps a tensorX-style startup provider while ignoring its session_start refresh", async () => { + it("applies a tensorX-style session_start catalog refresh from a known provider", async () => { + const providerId = "tensorx-style"; + const refreshedModelId = "tensorx-style-refreshed-model"; + const agentDir = await agentDirWithExtension(` + export default function (pi) { + ${providerRegistrationSource(providerId)} + pi.on("session_start", () => { + ${catalogRefreshSource(providerId, refreshedModelId)} + }); + } + `); + const { service, runtime, logEntries } = await policyHarness({ agentDir }); + const cwd = await tempDir("pi-web-policy-project-"); + + const session = await service.start(cwd); + const ref = { id: session.id, cwd }; + + expect(runtime.getModel(providerId, refreshedModelId)).toMatchObject({ + provider: providerId, + baseUrl: providerBaseUrl(providerId, "baseline"), + }); + expect(runtime.getModel(providerId, modelId(providerId, "baseline"))).toBeUndefined(); + expect(runtime.getRegisteredProviderConfig(providerId)).toMatchObject({ + baseUrl: providerBaseUrl(providerId, "baseline"), + apiKey: `sk-${providerId}-baseline-secret`, + }); + expect(await service.availableModels(ref)).toEqual(expect.arrayContaining([ + expect.objectContaining({ provider: providerId, id: refreshedModelId }), + ])); + // The extension body replays its unchanged startup config when the session + // loads it; that is not a catalog change and stays an ignored no-op. + expectIgnoredMutations(logEntries, [{ operation: "registerProvider", providerId }]); + expect(logEntries).toContainEqual({ + level: "info", + details: { + context: "global-provider-bootstrap", + operation: "registerProvider", + providerId, + modelCount: 1, + }, + message: "applied models-only provider update after global bootstrap", + }); + await expectNoProviderMutationFeedback(service, ref); + }); + + it("keeps a tensorX-style startup provider while ignoring a session_start config replacement", async () => { const providerId = "tensorx-style"; const agentDir = await agentDirWithExtension(` export default function (pi) { diff --git a/src/server/sessions/globalProviderPolicy.test.ts b/src/server/sessions/globalProviderPolicy.test.ts index 69aae37..e524d13 100644 --- a/src/server/sessions/globalProviderPolicy.test.ts +++ b/src/server/sessions/globalProviderPolicy.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import type { Provider } from "@earendil-works/pi-ai"; +import type { ModelRuntime } from "@earendil-works/pi-coding-agent"; import { bootstrapAndFreezeGlobalExtensionProviders, type GlobalProviderBootstrapLogger, @@ -69,6 +70,52 @@ function nativeProvider(providerId: string, name = providerId): Provider { }; } +const GLOBAL_PROVIDER_SOURCE = ` + export default function (pi) { + pi.registerProvider("global-config", { + name: "Global Config", + baseUrl: "https://global.example.com", + apiKey: "$GLOBAL_PROVIDER_KEY", + api: "openai-completions", + models: [{ + id: "global-model", + name: "Global Model", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 8192, + maxTokens: 1024 + }] + }); + } +`; + +function catalogModel(modelId: string): NonNullable[number] { + return { + id: modelId, + name: modelId, + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 8_192, + maxTokens: 1_024, + }; +} + +/** + * The complete config the motivating extension re-sends when it refreshes its + * catalog: every baseline field repeated verbatim, only `models` differing. + */ +function globalConfigWithModels(modelIds: readonly string[]): ProviderConfigInput { + return { + name: "Global Config", + baseUrl: "https://global.example.com", + apiKey: "$GLOBAL_PROVIDER_KEY", + api: "openai-completions", + models: modelIds.map(catalogModel), + }; +} + function registerProjectConfigProvider(runtime: Awaited>): void { runtime.registerProvider("project-config", { name: "Project Config", @@ -87,6 +134,8 @@ function registerProjectConfigProvider(runtime: Awaited>; + describe("bootstrapAndFreezeGlobalExtensionProviders", () => { it("captures the global baseline before making every later provider mutation a no-op", async () => { const agentDir = await agentDirWithExtension(` @@ -232,6 +281,120 @@ describe("bootstrapAndFreezeGlobalExtensionProviders", () => { expect(runtime.getRegisteredProviderIds()).toEqual([]); }); + it("applies a models-only refresh from a known provider and rebases the baseline", async () => { + const agentDir = await agentDirWithExtension(GLOBAL_PROVIDER_SOURCE); + const runtime = await createTestModelRuntime(); + const { entries, logger } = capturingLogger(); + + await bootstrapAndFreezeGlobalExtensionProviders(runtime, agentDir, logger); + + // A complete config, exactly as a refreshing extension re-sends it. + runtime.registerProvider("global-config", globalConfigWithModels(["global-model", "refreshed-model"])); + + expect(runtime.getModel("global-config", "refreshed-model")).toBeDefined(); + expect(runtime.getModel("global-config", "global-model")).toBeDefined(); + expect(runtime.getRegisteredProviderConfig("global-config")).toMatchObject({ + baseUrl: "https://global.example.com", + apiKey: "$GLOBAL_PROVIDER_KEY", + }); + expect(entries).toContainEqual({ + level: "info", + details: { + context: "global-provider-bootstrap", + operation: "registerProvider", + providerId: "global-config", + modelCount: 2, + }, + message: "applied models-only provider update after global bootstrap", + }); + + // The accepted config becomes the new baseline, so the next honest refresh + // (compared against it, not the original) is still accepted. + runtime.registerProvider("global-config", globalConfigWithModels(["second-refresh-model"])); + + expect(runtime.getModel("global-config", "second-refresh-model")).toBeDefined(); + expect(runtime.getModel("global-config", "refreshed-model")).toBeUndefined(); + expect(entries.filter((entry) => entry.message === "ignored provider mutation after global bootstrap")).toEqual([]); + }); + + it("ignores registrations that change any field other than the model catalog", async () => { + const agentDir = await agentDirWithExtension(GLOBAL_PROVIDER_SOURCE); + const runtime = await createTestModelRuntime(); + const { entries, logger } = capturingLogger(); + + await bootstrapAndFreezeGlobalExtensionProviders(runtime, agentDir, logger); + const baselineConfig = runtime.getRegisteredProviderConfig("global-config"); + + const mismatches: Record = { + name: { ...globalConfigWithModels(["changed-model"]), name: "Renamed Config" }, + baseUrl: { ...globalConfigWithModels(["changed-model"]), baseUrl: "https://replacement-secret.example.com" }, + apiKey: { ...globalConfigWithModels(["changed-model"]), apiKey: "replacement-secret-api-key" }, + api: { ...globalConfigWithModels(["changed-model"]), api: "anthropic-messages" }, + headers: { ...globalConfigWithModels(["changed-model"]), headers: { Authorization: "replacement-secret-token" } }, + authHeader: { ...globalConfigWithModels(["changed-model"]), authHeader: false }, + // Function-valued fields cannot be compared by value, so any incoming + // closure is conservatively treated as a mismatch. + refreshModels: { + ...globalConfigWithModels(["changed-model"]), + refreshModels: () => Promise.resolve([catalogModel("changed-model")]), + }, + streamSimple: { + ...globalConfigWithModels(["changed-model"]), + streamSimple: () => { throw new Error("streamSimple should not be called in this test"); }, + }, + oauth: { + ...globalConfigWithModels(["changed-model"]), + oauth: { + name: "Replacement OAuth", + login: () => Promise.reject(new Error("login should not be called in this test")), + refreshToken: () => Promise.reject(new Error("refreshToken should not be called in this test")), + getApiKey: () => "replacement-secret-oauth-key", + }, + }, + }; + for (const config of Object.values(mismatches)) runtime.registerProvider("global-config", config); + // A provider absent from the baseline stays blocked even for models-only shapes. + runtime.registerProvider("unknown-config", { models: [catalogModel("unknown-model")] }); + + expect(runtime.getRegisteredProviderConfig("global-config")).toBe(baselineConfig); + expect(runtime.getModel("global-config", "changed-model")).toBeUndefined(); + expect(runtime.getRegisteredProviderIds()).toEqual(["global-config"]); + expect(entries.filter((entry) => entry.message === "applied models-only provider update after global bootstrap")) + .toEqual([]); + // Repeated ignored registrations stay de-duplicated per (operation, provider). + const ignoredDetails = entries + .filter((entry) => entry.message === "ignored provider mutation after global bootstrap") + .map((entry) => entry.details); + expect(ignoredDetails).toEqual([ + { context: "global-provider-bootstrap", operation: "registerProvider", providerId: "global-config" }, + { context: "global-provider-bootstrap", operation: "registerProvider", providerId: "unknown-config" }, + ]); + // Rejected configs carry credentials; the decision log must never echo them. + expect(JSON.stringify(ignoredDetails)).not.toContain("secret"); + }); + + it("keeps native registration and unregistration frozen for a known provider", async () => { + const agentDir = await agentDirWithExtension(GLOBAL_PROVIDER_SOURCE); + const runtime = await createTestModelRuntime(); + const { entries, logger } = capturingLogger(); + + await bootstrapAndFreezeGlobalExtensionProviders(runtime, agentDir, logger); + const baselineConfig = runtime.getRegisteredProviderConfig("global-config"); + + runtime.registerNativeProvider(nativeProvider("global-config", "native-secret-name")); + runtime.unregisterProvider("global-config"); + + expect(runtime.getRegisteredProviderConfig("global-config")).toBe(baselineConfig); + expect(runtime.getRegisteredNativeProvider("global-config")).toBeUndefined(); + expect(runtime.getModel("global-config", "global-model")).toBeDefined(); + expect(entries + .filter((entry) => entry.message === "ignored provider mutation after global bootstrap") + .map((entry) => entry.details)).toEqual([ + { context: "global-provider-bootstrap", operation: "registerNativeProvider", providerId: "global-config" }, + { context: "global-provider-bootstrap", operation: "unregisterProvider", providerId: "global-config" }, + ]); + }); + it("logs non-fatal Pi bootstrap diagnostics and still freezes the runtime", async () => { const agentDir = await agentDirWithExtension(` export default function (pi) { diff --git a/src/server/sessions/globalProviderPolicy.ts b/src/server/sessions/globalProviderPolicy.ts index c999526..5056222 100644 --- a/src/server/sessions/globalProviderPolicy.ts +++ b/src/server/sessions/globalProviderPolicy.ts @@ -1,6 +1,7 @@ import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { isDeepStrictEqual } from "node:util"; import { createAgentSessionServices, type AgentSessionRuntimeDiagnostic, @@ -17,8 +18,49 @@ export interface GlobalProviderBootstrapLogger { type ProviderMutationOperation = "registerNativeProvider" | "registerProvider" | "unregisterProvider"; type ProviderMutationMethods = Pick; +/** Pi's `ProviderConfigInput`, read from the runtime contract instead of a deep package import. */ +type RegisteredProviderConfig = NonNullable>; const LOG_CONTEXT = "global-provider-bootstrap"; +const MODELS_FIELD = "models"; + +/** + * Snapshot the merged config Pi holds for every config-registered provider. + * Pi merges defined values over the previous registration, so the runtime's + * own record — not the extension's last argument — is the accurate baseline. + * Native providers have no comparable config and are deliberately absent. + */ +function captureProviderConfigBaseline(runtime: ModelRuntime): Map { + const baseline = new Map(); + for (const providerId of runtime.getRegisteredProviderIds()) { + const config = runtime.getRegisteredProviderConfig(providerId); + if (config) baseline.set(providerId, config); + } + return baseline; +} + +/** + * True when `incoming` would change the model catalog of `baseline` and nothing + * else. + * + * Extensions that refresh their catalog re-send a complete provider config, so + * the test is "equal to the baseline except for models", not "contains only + * models". Omitted fields are not changes: Pi's merge keeps the previous value, + * which also makes an unchanged catalog a plain replay rather than an update. + * Function-valued fields (`streamSimple`, `refreshModels`, `oauth` methods) + * compare by reference under deep strict equality, so a freshly created closure + * reads as a mismatch. That conservative direction is intentional: an unclear + * comparison must fall back to the frozen no-op. + */ +function isModelsOnlyProviderUpdate(baseline: RegisteredProviderConfig, incoming: RegisteredProviderConfig): boolean { + const baselineFields = new Map(Object.entries(baseline)); + const otherFieldsMatch = Object.entries(incoming).every(([field, value]) => { + if (field === MODELS_FIELD || value === undefined) return true; + return isDeepStrictEqual(value, baselineFields.get(field)); + }); + if (!otherFieldsMatch) return false; + return incoming.models !== undefined && !isDeepStrictEqual(incoming.models, baseline.models); +} async function loadGlobalExtensionServices(runtime: ModelRuntime, agentDir: string): Promise { const scratchCwd = await mkdtemp(join(tmpdir(), "pi-web-global-ext-")); @@ -47,7 +89,11 @@ function logBootstrapDiagnostic( } } -function freezeProviderMutations(runtime: ModelRuntime, logger: GlobalProviderBootstrapLogger): void { +function freezeProviderMutations( + runtime: ModelRuntime, + logger: GlobalProviderBootstrapLogger, + configBaseline: Map, +): void { const originalMethods: ProviderMutationMethods = { registerNativeProvider: runtime.registerNativeProvider.bind(runtime), registerProvider: runtime.registerProvider.bind(runtime), @@ -58,22 +104,42 @@ function freezeProviderMutations(runtime: ModelRuntime, logger: GlobalProviderBo registerProvider: new Set(), unregisterProvider: new Set(), }; + // Logging must never turn a provider mutation into an extension failure. + const logQuietly = (details: Record, message: string): void => { + try { + logger.info(details, message); + } catch { + // Intentionally ignored; the mutation decision already stands. + } + }; const logIgnoredMutation = (operation: ProviderMutationOperation, providerId: string): void => { const loggedIds = loggedProviderIds[operation]; if (loggedIds.has(providerId)) return; loggedIds.add(providerId); - try { - logger.info( - { context: LOG_CONTEXT, operation, providerId }, - "ignored provider mutation after global bootstrap", - ); - } catch { - // Logging must not turn an ignored mutation into an extension failure. - } + logQuietly({ context: LOG_CONTEXT, operation, providerId }, "ignored provider mutation after global bootstrap"); }; const frozenMethods: ProviderMutationMethods = { - registerProvider(providerId) { - logIgnoredMutation("registerProvider", providerId); + registerProvider(providerId, config) { + const baseline = configBaseline.get(providerId); + if (!baseline || !isModelsOnlyProviderUpdate(baseline, config)) { + logIgnoredMutation("registerProvider", providerId); + return; + } + // Pi validates the registration and ends in a fire-and-forget local + // refresh, so this stays synchronous and never reaches the network. + originalMethods.registerProvider(providerId, config); + const accepted = runtime.getRegisteredProviderConfig(providerId); + // Re-read the merged record so the next comparison uses what Pi stored. + if (accepted) configBaseline.set(providerId, accepted); + logQuietly( + { + context: LOG_CONTEXT, + operation: "registerProvider", + providerId, + modelCount: accepted?.models?.length ?? 0, + }, + "applied models-only provider update after global bootstrap", + ); }, registerNativeProvider(provider) { logIgnoredMutation("registerNativeProvider", provider.id); @@ -104,6 +170,13 @@ function freezeProviderMutations(runtime: ModelRuntime, logger: GlobalProviderBo * public service factory. Pi exposes no provider-freeze hook, so the daemon * deliberately shadows the three public instance mutation methods afterward; * every registration replay or later call is then a logged no-op. + * + * The one exception is a known config provider refreshing its own model + * catalog: a `registerProvider` call whose config matches the recorded + * baseline in every field except `models` is applied, because a catalog is a + * property of the provider rather than of the project. Native registration + * stays fully frozen — it passes a whole `Provider` object with no comparable + * config — as does unregistration. */ export async function bootstrapAndFreezeGlobalExtensionProviders( runtime: ModelRuntime, @@ -113,7 +186,7 @@ export async function bootstrapAndFreezeGlobalExtensionProviders( const services = await loadGlobalExtensionServices(runtime, agentDir); const providerIds = Object.freeze([...runtime.getRegisteredProviderIds()].sort()); - freezeProviderMutations(runtime, logger); + freezeProviderMutations(runtime, logger, captureProviderConfigBaseline(runtime)); for (const diagnostic of services.diagnostics) logBootstrapDiagnostic(logger, diagnostic); for (const extensionError of services.resourceLoader.getExtensions().errors) {