From d9bab7ce8b034a35f6db679cc693994cbb32a063 Mon Sep 17 00:00:00 2001 From: roboomp Date: Wed, 22 Jul 2026 10:57:47 +0000 Subject: [PATCH] fix(catalog): restore cached request-model variants via requestModelId Copilot -1m long-context variants are synthesized with transport headers and a requestModelId to a bundled base. The v10 cache omits headers; the writer only matched a same-id static entry, so these variants were flagged unrestorable and dropped on the next offline read, vanishing from the picker with a "Could not restore model" warning. The startup registry loader dropped them the same way. Restore/match headers through requestModelId in the cache writer, the model-manager restore path, and the coding-agent startup loader, and bypass a stale unrestorable marker written by the old id-only writer. Fixes #6284 --- packages/catalog/CHANGELOG.md | 4 + packages/catalog/src/model-cache.ts | 8 +- packages/catalog/src/model-manager.ts | 23 ++++-- packages/catalog/test/build.test.ts | 76 +++++++++++++++++++ packages/coding-agent/CHANGELOG.md | 4 + .../coding-agent/src/config/model-registry.ts | 12 ++- .../coding-agent/test/model-discovery.test.ts | 32 +++++++- 7 files changed, 147 insertions(+), 12 deletions(-) diff --git a/packages/catalog/CHANGELOG.md b/packages/catalog/CHANGELOG.md index 50018ad2e..fbbddaa50 100644 --- a/packages/catalog/CHANGELOG.md +++ b/packages/catalog/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed cached models that reuse a bundled request model — including GitHub Copilot `-1m` long-context variants — being flagged unrestorable and dropped after a restart. Header restoration now matches the `requestModelId` base and bypasses a stale `unrestorable` marker written by the old id-only cache writer. ([#6037](https://github.com/can1357/oh-my-pi/issues/6037), [#6284](https://github.com/can1357/oh-my-pi/issues/6284)) + ## [17.0.6] - 2026-07-20 ### Added diff --git a/packages/catalog/src/model-cache.ts b/packages/catalog/src/model-cache.ts index b6587e75e..cb68c24f5 100644 --- a/packages/catalog/src/model-cache.ts +++ b/packages/catalog/src/model-cache.ts @@ -226,7 +226,13 @@ export function writeModelCache( for (const model of models) { if (hasModelHeaders(model)) { headerOmittedModelIds.push(model.id); - if (!headersEqual(model.headers, staticById.get(model.id)?.headers)) { + // Synthesized variants (e.g. Copilot `-1m`) have no same-id static + // entry; their headers come from the `requestModelId` base. Match + // against that source too, else they are wrongly flagged + // unrestorable and dropped on the next offline read (#6037, #6284). + const staticHeaderSource = + staticById.get(model.id) ?? (model.requestModelId ? staticById.get(model.requestModelId) : undefined); + if (!headersEqual(model.headers, staticHeaderSource?.headers)) { unrestorableHeaderModelIds.push(model.id); } } diff --git a/packages/catalog/src/model-manager.ts b/packages/catalog/src/model-manager.ts index f9ff67017..0792bc688 100644 --- a/packages/catalog/src/model-manager.ts +++ b/packages/catalog/src/model-manager.ts @@ -108,9 +108,15 @@ interface CachedHeaderRestoreResult { /** * Restore cache-omitted headers from the current static source. * - * Dynamic-only header-bearing models cannot be reconstructed safely without - * persisting arbitrary credential values; callers must refetch them online or - * omit them from an offline result rather than return a broken model. + * A same-id static match is trusted only when the row did not flag the model + * unrestorable (its live headers matched static when cached). Synthesized + * variants (e.g. Copilot `-1m`) instead recover headers through + * `requestModelId`, whose static source is where their headers came from — + * honoured even past a stale `unrestorable` marker written by the old id-only + * writer (#6037, #6284). Header-bearing models without either source cannot be + * reconstructed safely without persisting arbitrary credential values; + * callers must refetch them online or omit them rather than return a broken + * model. */ function restoreCachedModelHeaders( cachedModels: readonly ModelSpec[], @@ -128,11 +134,12 @@ function restoreCachedModelHeaders( const unresolvedModelIds = new Set(); const restored = models.map(model => { if (!omittedIds.has(model.id)) return model; - if (unrestorableIds.has(model.id)) { - unresolvedModelIds.add(model.id); - return model; - } - const staticModel = staticById.get(model.id); + // A same-id static match is trusted only when the row did not flag the + // model unrestorable. A `requestModelId` source is always trusted: it is + // where a synthesized variant's headers came from. + const staticModel = + (unrestorableIds.has(model.id) ? undefined : staticById.get(model.id)) ?? + (model.requestModelId ? staticById.get(model.requestModelId) : undefined); if (!staticModel?.headers) { unresolvedModelIds.add(model.id); return model; diff --git a/packages/catalog/test/build.test.ts b/packages/catalog/test/build.test.ts index 4e89e2269..0fa790c46 100644 --- a/packages/catalog/test/build.test.ts +++ b/packages/catalog/test/build.test.ts @@ -664,6 +664,82 @@ describe("model cache spec round trip", () => { await fs.rm(tempDir, { recursive: true, force: true }); } }); + + it("keeps a synthesized request-model variant across an offline restart", async () => { + // Regression for #6037/#6284: Copilot `-1m` long-context variants are + // synthesized dynamically with transport headers and a `requestModelId` + // pointing at a same-provider base. Their headers are omitted from the + // cache but recoverable from the base's static headers, so they must NOT + // be flagged unrestorable and dropped on the next offline read. + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-catalog-request-model-variant-")); + const dbPath = path.join(tempDir, "models.db"); + const headers = { "X-GitHub-Api-Version": "2026-06-01" }; + const base = completionsSpec({ id: "sol", provider: "variant-cache-test", headers }); + const variant = completionsSpec({ + id: "sol-1m", + provider: "variant-cache-test", + requestModelId: "sol", + headers, + contextWindow: 1_000_000, + }); + const options = { + providerId: "variant-cache-test", + staticModels: [base], + cacheDbPath: dbPath, + }; + try { + const online = await resolveProviderModels<"openai-completions">( + { ...options, fetchDynamicModels: async () => [base, variant] }, + "online", + ); + expect(online.models.find(candidate => candidate.id === "sol-1m")).toBeDefined(); + + const offline = await resolveProviderModels<"openai-completions">( + { ...options, fetchDynamicModels: async () => null }, + "offline", + ); + const restored = offline.models.find(candidate => candidate.id === "sol-1m"); + expect(restored).toBeDefined(); + expect(restored?.headers).toEqual(headers); + expect(offline.models.find(candidate => candidate.id === "sol")?.headers).toEqual(headers); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + + it("recovers a legacy stale-marked request-model variant via requestModelId", async () => { + // Legacy cache rows (written by the old id-only writer) flag `-1m` + // variants unrestorable because it never matched their base's headers. + // The restore path must still recover them through `requestModelId`. + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-catalog-legacy-variant-")); + const dbPath = path.join(tempDir, "models.db"); + const headers = { "X-GitHub-Api-Version": "2026-06-01" }; + const base = completionsSpec({ id: "sol", provider: "variant-cache-test", headers }); + const variant = buildModel( + completionsSpec({ + id: "sol-1m", + provider: "variant-cache-test", + requestModelId: "sol", + headers, + contextWindow: 1_000_000, + }), + ); + try { + // Emulate a legacy write: no static header source, so the variant is + // flagged unrestorable even though its base carries the headers. + writeModelCache("variant-cache-test", Date.now(), [variant], true, "", dbPath); + + const offline = await resolveProviderModels<"openai-completions">( + { providerId: "variant-cache-test", staticModels: [base], cacheDbPath: dbPath }, + "offline", + ); + const restored = offline.models.find(candidate => candidate.id === "sol-1m"); + expect(restored).toBeDefined(); + expect(restored?.headers).toEqual(headers); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); }); describe("isOfficialAnthropicApiUrl", () => { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 945a4ed1c..a3c15c736 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed GitHub Copilot 1M-context models (e.g. `github-copilot/gpt-5.6-sol-1m`) disappearing from the model picker on restart, with a `Could not restore model` warning, until discovery was manually refreshed. The startup cache loader now restores their transport headers from the bundled base via `requestModelId`. ([#6037](https://github.com/can1357/oh-my-pi/issues/6037), [#6284](https://github.com/can1357/oh-my-pi/issues/6284)) + ## [17.0.7] - 2026-07-21 ### Fixed diff --git a/packages/coding-agent/src/config/model-registry.ts b/packages/coding-agent/src/config/model-registry.ts index bec1c4ca9..af2c809e8 100644 --- a/packages/coding-agent/src/config/model-registry.ts +++ b/packages/coding-agent/src/config/model-registry.ts @@ -1143,8 +1143,16 @@ export class ModelRegistry { models.push(spec); continue; } - if (unrestorableHeaderIds.has(spec.id)) continue; - const bundledHeaders = bundledById?.get(spec.id)?.headers; + // A same-id bundled match is trusted only when the row did not flag + // the model unrestorable. Synthesized variants (Copilot `-1m`) + // recover through `requestModelId`, whose bundled source is where + // their headers came from — honoured even past a stale + // `unrestorable` marker written before the fallback existed + // (#6037, #6284). + const bundledHeaders = ( + (unrestorableHeaderIds.has(spec.id) ? undefined : bundledById?.get(spec.id)) ?? + (spec.requestModelId ? bundledById?.get(spec.requestModelId) : undefined) + )?.headers; if (!bundledHeaders) continue; models.push({ ...spec, headers: bundledHeaders }); } diff --git a/packages/coding-agent/test/model-discovery.test.ts b/packages/coding-agent/test/model-discovery.test.ts index 310c550d5..9fa9a7ac2 100644 --- a/packages/coding-agent/test/model-discovery.test.ts +++ b/packages/coding-agent/test/model-discovery.test.ts @@ -6,7 +6,8 @@ import { Effort, type FetchImpl, type Model } from "@oh-my-pi/pi-ai"; import type { OAuthCredentials } from "@oh-my-pi/pi-ai/oauth/types"; import { buildModel } from "@oh-my-pi/pi-catalog/build"; import { writeModelCache } from "@oh-my-pi/pi-catalog/model-cache"; -import type { OpenAICompat } from "@oh-my-pi/pi-catalog/types"; +import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; +import type { ModelSpec, OpenAICompat } from "@oh-my-pi/pi-catalog/types"; import { applyLlamaCppQwenThinking } from "@oh-my-pi/pi-coding-agent/config/model-discovery"; import { kNoAuth, ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { resetSettingsForTest } from "@oh-my-pi/pi-coding-agent/config/settings"; @@ -2133,4 +2134,33 @@ describe("ModelRegistry runtime discovery", () => { expect(registry.find("litellm-test", "team-gpt")?.contextWindow).toBe(200_000); expect(registry.find("litellm-test", "deployment-id")).toBeUndefined(); }); + + test("startup restores a legacy stale-marked Copilot -1m variant via requestModelId", () => { + // Regression for #6037/#6284: a synthesized Copilot `-1m` long-context + // variant keeps the base model's transport headers via `requestModelId`. + // The v10 cache omits headers, and legacy rows written by the old id-only + // writer flag the variant unrestorable (its base is a different id). The + // startup loader must still recover the headers from the bundled base and + // keep the model selectable instead of dropping it. + const bundledBase = getBundledModel("github-copilot", "gpt-5.6-sol"); + if (!bundledBase?.headers) { + throw new Error("Expected bundled Copilot base to carry transport headers"); + } + const cachedVariant = buildModel({ + ...(bundledBase as ModelSpec<"openai-responses">), + id: "gpt-5.6-sol-1m", + name: "GPT-5.6 Sol (1M)", + requestModelId: "gpt-5.6-sol", + contextWindow: 1_050_000, + }); + // Emulate a legacy write: the variant has no same-id static header source, + // so it is flagged unrestorable even though its base carries the headers. + writeModelCache("github-copilot", Date.now(), [cachedVariant], true, "", cacheDbPath); + + const registry = new ModelRegistry(authStorage, modelsJsonPath); + + const restored = registry.find("github-copilot", "gpt-5.6-sol-1m"); + expect(restored).toBeDefined(); + expect(restored?.headers).toEqual(bundledBase.headers); + }); });