From 8cebe98d2fe890416302618d82220473bc4769de Mon Sep 17 00:00:00 2001 From: roboomp Date: Mon, 22 Jun 2026 13:30:01 +0000 Subject: [PATCH] fix(coding-agent): evicted openai-completions provider session state on backend switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AgentSession.#closeProviderSessionsForModelSwitch` only handled `openai-codex-responses` and `openai-responses:` keys. The `openai-completions:::` entries — which cache strict-tools disable scopes and reasoning-effort fallbacks tied to the upstream backend — survived /model switches between different providers or base URLs, so the next request to that backend (e.g. on /model toggle back) replayed stale decisions made against an entirely different transport. Switching to a model whose `(provider, baseUrl)` differs from the current openai-completions model now evicts every cached entry sharing the old prefix. Same-backend model toggles keep their cached state, matching the existing codex/responses semantics. Fixes #3260 --- packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/session/agent-session.ts | 32 ++++ ...on-openai-completions-model-switch.test.ts | 165 ++++++++++++++++++ 3 files changed, 201 insertions(+) create mode 100644 packages/coding-agent/test/agent-session-openai-completions-model-switch.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 2e70c2bd9..fb2d4f07c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `openai-completions` provider session state surviving `/model` switches across different providers or base URLs. `AgentSession.#closeProviderSessionsForModelSwitch` only evicted `openai-codex-responses` and `openai-responses:` keys; entries keyed `openai-completions:::` (cached strict-tools disable scopes and reasoning-effort fallbacks for the old transport) lingered indefinitely. Moving away from an `openai-completions` backend now evicts every cached entry sharing the previous `(provider, baseUrl)` pair, while same-backend model toggles keep their cached state ([#3260](https://github.com/can1357/oh-my-pi/issues/3260)) + ## [16.1.14] - 2026-06-22 ### Added diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index b25b30507..d594a2f14 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -9026,6 +9026,23 @@ export class AgentSession { providerKeys.add(`openai-responses:${nextModel.provider}`); } + // `openai-completions` sessions are keyed `openai-completions:::` + // and cache backend-specific decisions (strict-tools disable scopes, reasoning-effort + // fallbacks). When the user moves away from that backend — different `api`, `provider`, + // or `baseUrl` — the cached decisions no longer track the active transport. Evict the + // full `(provider, baseUrl)` prefix; same-backend model toggles keep their state. + let completionsPrefixToEvict: string | undefined; + if (currentModel.api === "openai-completions") { + const currentScope = `${currentModel.provider}:${currentModel.baseUrl ?? ""}`; + const nextScope = + nextModel.api === "openai-completions" + ? `${nextModel.provider}:${nextModel.baseUrl ?? ""}` + : undefined; + if (currentScope !== nextScope) { + completionsPrefixToEvict = `openai-completions:${currentScope}:`; + } + } + for (const providerKey of providerKeys) { const state = this.#providerSessionState.get(providerKey); if (!state) continue; @@ -9041,6 +9058,21 @@ export class AgentSession { this.#providerSessionState.delete(providerKey); } + + if (completionsPrefixToEvict !== undefined) { + for (const [key, state] of this.#providerSessionState) { + if (!key.startsWith(completionsPrefixToEvict)) continue; + try { + state.close(); + } catch (error) { + logger.warn("Failed to close provider session state during model switch", { + providerKey: key, + error: String(error), + }); + } + this.#providerSessionState.delete(key); + } + } } #normalizeProviderReplayValue(value: unknown): unknown { diff --git a/packages/coding-agent/test/agent-session-openai-completions-model-switch.test.ts b/packages/coding-agent/test/agent-session-openai-completions-model-switch.test.ts new file mode 100644 index 000000000..9b71f2f65 --- /dev/null +++ b/packages/coding-agent/test/agent-session-openai-completions-model-switch.test.ts @@ -0,0 +1,165 @@ +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import * as path from "node:path"; +import { Agent } from "@oh-my-pi/pi-agent-core"; +import type { Model, ProviderSessionState } from "@oh-my-pi/pi-ai"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; +import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { TempDir } from "@oh-my-pi/pi-utils"; + +// Regression: `#closeProviderSessionsForModelSwitch` historically only handled +// the `openai-codex-responses` / `openai-responses` keys and left +// `openai-completions:::` entries behind on a +// /model switch. The cached strict-tools disable scopes and reasoning-effort +// fallbacks for the old backend then survived indefinitely — repro reported +// in #3260 (PR #3236). + +describe("AgentSession openai-completions provider session eviction", () => { + let tempDir: TempDir; + let session: AgentSession; + let modelRegistry: ModelRegistry; + let authStorage: AuthStorage; + + beforeAll(async () => { + tempDir = TempDir.createSync("@pi-completions-eviction-"); + authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); + modelRegistry = new ModelRegistry(authStorage); + }); + + afterAll(() => { + authStorage.close(); + tempDir.removeSync(); + }); + + afterEach(async () => { + if (session) { + await session.dispose(); + } + }); + + function completionsModel(provider: string, id: string): Model { + const model = modelRegistry.find(provider, id); + if (!model) { + throw new Error(`expected bundled openai-completions model ${provider}/${id}`); + } + if (model.api !== "openai-completions") { + throw new Error(`expected ${provider}/${id} to use openai-completions, got ${model.api}`); + } + return model; + } + + function completionsSessionKey(model: Model): string { + return `openai-completions:${model.provider}:${model.baseUrl ?? ""}:${model.id}`; + } + + function buildSession(model: Model): AgentSession { + authStorage.setRuntimeApiKey(model.provider, "test-key"); + const agent = new Agent({ + initialState: { + model, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + }); + return new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated({ "compaction.enabled": false }), + modelRegistry, + }); + } + + it("evicts stale openai-completions state on provider/baseUrl switch", async () => { + const deepseek = completionsModel("deepseek", "deepseek-v4-pro"); + const cerebras = completionsModel("cerebras", "llama3.1-8b"); + authStorage.setRuntimeApiKey(cerebras.provider, "cerebras-test-key"); + + session = buildSession(deepseek); + + const oldCloseSpy = vi.fn(); + session.providerSessionState.set(completionsSessionKey(deepseek), { + close: oldCloseSpy, + } satisfies ProviderSessionState); + + await session.setModel(cerebras); + + expect(session.model?.provider).toBe(cerebras.provider); + expect(session.model?.id).toBe(cerebras.id); + expect(oldCloseSpy).toHaveBeenCalledTimes(1); + expect(session.providerSessionState.has(completionsSessionKey(deepseek))).toBe(false); + }); + + it("evicts every cached entry under the old (provider, baseUrl) prefix", async () => { + const deepseekPro = completionsModel("deepseek", "deepseek-v4-pro"); + const deepseekFlash = completionsModel("deepseek", "deepseek-v4-flash"); + const cerebras = completionsModel("cerebras", "llama3.1-8b"); + authStorage.setRuntimeApiKey(cerebras.provider, "cerebras-test-key"); + + session = buildSession(deepseekPro); + + const proCloseSpy = vi.fn(); + const flashCloseSpy = vi.fn(); + session.providerSessionState.set(completionsSessionKey(deepseekPro), { + close: proCloseSpy, + } satisfies ProviderSessionState); + // A sibling model on the same backend — set up earlier in the session but + // not currently active. Same `(provider, baseUrl)` prefix; must also go. + session.providerSessionState.set(completionsSessionKey(deepseekFlash), { + close: flashCloseSpy, + } satisfies ProviderSessionState); + + await session.setModel(cerebras); + + expect(proCloseSpy).toHaveBeenCalledTimes(1); + expect(flashCloseSpy).toHaveBeenCalledTimes(1); + expect(session.providerSessionState.has(completionsSessionKey(deepseekPro))).toBe(false); + expect(session.providerSessionState.has(completionsSessionKey(deepseekFlash))).toBe(false); + }); + + it("leaves unrelated provider session state untouched", async () => { + const deepseek = completionsModel("deepseek", "deepseek-v4-pro"); + const cerebras = completionsModel("cerebras", "llama3.1-8b"); + authStorage.setRuntimeApiKey(cerebras.provider, "cerebras-test-key"); + + session = buildSession(deepseek); + + const oldCloseSpy = vi.fn(); + const unrelatedCloseSpy = vi.fn(); + const unrelatedKey = "openai-completions:other-provider:https://other.example/v1:other-model"; + session.providerSessionState.set(completionsSessionKey(deepseek), { + close: oldCloseSpy, + } satisfies ProviderSessionState); + session.providerSessionState.set(unrelatedKey, { + close: unrelatedCloseSpy, + } satisfies ProviderSessionState); + + await session.setModel(cerebras); + + expect(oldCloseSpy).toHaveBeenCalledTimes(1); + expect(unrelatedCloseSpy).not.toHaveBeenCalled(); + expect(session.providerSessionState.has(unrelatedKey)).toBe(true); + }); + + it("keeps cached state when switching models on the same (provider, baseUrl)", async () => { + const deepseekPro = completionsModel("deepseek", "deepseek-v4-pro"); + const deepseekFlash = completionsModel("deepseek", "deepseek-v4-flash"); + expect(deepseekPro.provider).toBe(deepseekFlash.provider); + expect(deepseekPro.baseUrl).toBe(deepseekFlash.baseUrl); + + session = buildSession(deepseekPro); + + const proCloseSpy = vi.fn(); + session.providerSessionState.set(completionsSessionKey(deepseekPro), { + close: proCloseSpy, + } satisfies ProviderSessionState); + + await session.setModel(deepseekFlash); + + expect(session.model?.id).toBe(deepseekFlash.id); + expect(proCloseSpy).not.toHaveBeenCalled(); + expect(session.providerSessionState.has(completionsSessionKey(deepseekPro))).toBe(true); + }); +});