From e70b3d6bb9e92f78282f9f004a765397a254e00d Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Wed, 22 Jul 2026 20:58:56 +0200 Subject: [PATCH] fix(sessions): keep ignored provider mutations inert --- src/server/sessions/authService.test.ts | 8 ++++++- .../sessions/globalProviderPolicy.test.ts | 21 +++++++++++++++++++ src/server/sessions/globalProviderPolicy.ts | 12 +++++++---- .../piSessionService.promptQueue.test.ts | 14 +++++++++++-- 4 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/server/sessions/authService.test.ts b/src/server/sessions/authService.test.ts index e427e6c..8a07d0a 100644 --- a/src/server/sessions/authService.test.ts +++ b/src/server/sessions/authService.test.ts @@ -3,13 +3,19 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { ModelRuntime } from "@earendil-works/pi-coding-agent"; import { InMemoryCredentialStore, type AuthPrompt, type Credential } from "@earendil-works/pi-ai"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OAuthFlowState } from "../../shared/apiTypes.js"; import { AuthService, createModelRuntimeForAgentDir, type AuthChange, type AuthServiceLogger } from "./authService.js"; import { OAuthLoginFlowService } from "./oauthLoginFlowService.js"; const tempDirs: string[] = []; +beforeEach(() => { + // Pi 0.81 uses PI_OFFLINE for refreshes after runtime creation. Auth tests + // exercise local credential behavior and must never fetch provider catalogs. + vi.stubEnv("PI_OFFLINE", "1"); +}); + afterEach(async () => { vi.unstubAllEnvs(); await Promise.all(tempDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); diff --git a/src/server/sessions/globalProviderPolicy.test.ts b/src/server/sessions/globalProviderPolicy.test.ts index 4b536e0..0545f9b 100644 --- a/src/server/sessions/globalProviderPolicy.test.ts +++ b/src/server/sessions/globalProviderPolicy.test.ts @@ -157,6 +157,27 @@ describe("bootstrapAndFreezeGlobalExtensionProviders", () => { expect(JSON.stringify(ignoredMutations)).not.toContain("secret"); }); + it("keeps ignored mutations as no-ops when structured logging fails", async () => { + const agentDir = await tempDir("pi-web-global-provider-unit-"); + const runtime = await createTestModelRuntime(); + const { logger } = capturingLogger(); + const loggingError = new Error("provider mutation logger failed"); + const throwingLogger: GlobalProviderBootstrapLogger = { + ...logger, + info(details, message) { + if (message === "ignored provider mutation after global bootstrap") throw loggingError; + logger.info(details, message); + }, + }; + + await bootstrapAndFreezeGlobalExtensionProviders(runtime, agentDir, throwingLogger); + + expect(() => { registerProjectConfigProvider(runtime); }).not.toThrow(); + expect(() => { runtime.registerNativeProvider(nativeProvider("project-native")); }).not.toThrow(); + expect(() => { runtime.unregisterProvider("project-only"); }).not.toThrow(); + expect(runtime.getRegisteredProviderIds()).toEqual([]); + }); + 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 ebad281..c999526 100644 --- a/src/server/sessions/globalProviderPolicy.ts +++ b/src/server/sessions/globalProviderPolicy.ts @@ -62,10 +62,14 @@ function freezeProviderMutations(runtime: ModelRuntime, logger: GlobalProviderBo const loggedIds = loggedProviderIds[operation]; if (loggedIds.has(providerId)) return; loggedIds.add(providerId); - logger.info( - { context: LOG_CONTEXT, operation, providerId }, - "ignored provider mutation after global bootstrap", - ); + 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. + } }; const frozenMethods: ProviderMutationMethods = { registerProvider(providerId) { diff --git a/src/server/sessions/piSessionService.promptQueue.test.ts b/src/server/sessions/piSessionService.promptQueue.test.ts index 9b8a04f..781acce 100644 --- a/src/server/sessions/piSessionService.promptQueue.test.ts +++ b/src/server/sessions/piSessionService.promptQueue.test.ts @@ -4,12 +4,22 @@ import { join } from "node:path"; import { createAssistantMessageEventStream, InMemoryCredentialStore, type AssistantMessage } from "@earendil-works/pi-ai"; import type { StreamFn } from "@earendil-works/pi-agent-core"; import { ModelRuntime } from "@earendil-works/pi-coding-agent"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { PiSessionService } from "./piSessionService.js"; import { CapturingSessionEventHub, createTestModelRuntime, fakeRuntime, runtimeCreator, seedCredential, sessionGateway, sessionRecord, sessionRef, TEST_MODEL_ID, TEST_MODEL_PROVIDER, testModel, testModelRuntime, type RuntimeCreator } from "./piSessionService.testSupport.js"; const TEST_AGENT_DIR = "/tmp/pi-web-test-agent"; +beforeEach(() => { + // Pi 0.81 uses PI_OFFLINE for refreshes after runtime creation. These tests + // exercise local model/auth behavior and must never fetch provider catalogs. + vi.stubEnv("PI_OFFLINE", "1"); +}); + +afterEach(() => { + vi.unstubAllEnvs(); +}); + describe("PiSessionService prompt, queue, and auth warnings", () => { it("sends prompts to an injected runtime without touching the SDK runtime", async () => { const fake = fakeRuntime("prompt-session"); @@ -78,7 +88,7 @@ describe("PiSessionService prompt, queue, and auth warnings", () => { await service.dispose(); }); - it("generates a session name for the first prompt via the session's agent.streamFn", async () => { + it("generates a session name for the first prompt via the session's agent.streamFunction", async () => { const model = testModel(); const streamCalls: unknown[] = []; const streamFn: StreamFn = (streamModel, context, options) => {