From 1906074369594ce55454cf35fa1006a50ae08743 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Tue, 9 Jun 2026 13:14:24 +0200 Subject: [PATCH] fix(coding-agent): handle invalid models json during migration closes #5418 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/src/migrations.ts | 75 ++++++++++--------- .../test/config-value-migration.test.ts | 19 +++++ 3 files changed, 60 insertions(+), 35 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a36f356d..95c7561e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixed +- Fixed invalid `models.json` syntax to skip startup config migrations and report the normal file-path-aware models error instead of a raw JSON parse stack trace ([#5418](https://github.com/earendil-works/pi/issues/5418)). - Fixed GitHub release notes and interactive changelog links to resolve package-relative documentation URLs correctly ([#5516](https://github.com/earendil-works/pi/issues/5516)). ## [0.79.0] - 2026-06-08 diff --git a/packages/coding-agent/src/migrations.ts b/packages/coding-agent/src/migrations.ts index eb07f220..5cce43b8 100644 --- a/packages/coding-agent/src/migrations.ts +++ b/packages/coding-agent/src/migrations.ts @@ -144,47 +144,52 @@ function migrateModelsJsonConfigValues(agentDir: string): ConfigValueMigration[] const modelsPath = join(agentDir, "models.json"); if (!existsSync(modelsPath)) return []; - const parsed = JSON.parse(stripJsonComments(readFileSync(modelsPath, "utf-8"))) as unknown; - if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return []; - const modelsData = parsed as Record; - const providers = modelsData.providers; - if (typeof providers !== "object" || providers === null || Array.isArray(providers)) return []; + try { + const parsed = JSON.parse(stripJsonComments(readFileSync(modelsPath, "utf-8"))) as unknown; + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return []; + const modelsData = parsed as Record; + const providers = modelsData.providers; + if (typeof providers !== "object" || providers === null || Array.isArray(providers)) return []; - const migrations: ConfigValueMigration[] = []; - for (const [provider, providerConfig] of Object.entries(providers)) { - if (typeof providerConfig !== "object" || providerConfig === null || Array.isArray(providerConfig)) continue; - const providerRecord = providerConfig as Record; - const providerLocation = `models.json.providers[${JSON.stringify(provider)}]`; - migrateStringProperty(providerRecord, "apiKey", `${providerLocation}.apiKey`, migrations); - migrateHeadersConfig(providerRecord.headers, `${providerLocation}.headers`, migrations); + const migrations: ConfigValueMigration[] = []; + for (const [provider, providerConfig] of Object.entries(providers)) { + if (typeof providerConfig !== "object" || providerConfig === null || Array.isArray(providerConfig)) continue; + const providerRecord = providerConfig as Record; + const providerLocation = `models.json.providers[${JSON.stringify(provider)}]`; + migrateStringProperty(providerRecord, "apiKey", `${providerLocation}.apiKey`, migrations); + migrateHeadersConfig(providerRecord.headers, `${providerLocation}.headers`, migrations); - if (Array.isArray(providerRecord.models)) { - for (let index = 0; index < providerRecord.models.length; index++) { - const modelConfig = providerRecord.models[index]; - if (typeof modelConfig !== "object" || modelConfig === null || Array.isArray(modelConfig)) continue; - const modelRecord = modelConfig as Record; - const modelKey = typeof modelRecord.id === "string" ? JSON.stringify(modelRecord.id) : String(index); - migrateHeadersConfig(modelRecord.headers, `${providerLocation}.models[${modelKey}].headers`, migrations); + if (Array.isArray(providerRecord.models)) { + for (let index = 0; index < providerRecord.models.length; index++) { + const modelConfig = providerRecord.models[index]; + if (typeof modelConfig !== "object" || modelConfig === null || Array.isArray(modelConfig)) continue; + const modelRecord = modelConfig as Record; + const modelKey = typeof modelRecord.id === "string" ? JSON.stringify(modelRecord.id) : String(index); + migrateHeadersConfig(modelRecord.headers, `${providerLocation}.models[${modelKey}].headers`, migrations); + } + } + + const modelOverrides = providerRecord.modelOverrides; + if (typeof modelOverrides === "object" && modelOverrides !== null && !Array.isArray(modelOverrides)) { + for (const [modelId, modelOverride] of Object.entries(modelOverrides)) { + if (typeof modelOverride !== "object" || modelOverride === null || Array.isArray(modelOverride)) + continue; + const modelOverrideRecord = modelOverride as Record; + migrateHeadersConfig( + modelOverrideRecord.headers, + `${providerLocation}.modelOverrides[${JSON.stringify(modelId)}].headers`, + migrations, + ); + } } } - const modelOverrides = providerRecord.modelOverrides; - if (typeof modelOverrides === "object" && modelOverrides !== null && !Array.isArray(modelOverrides)) { - for (const [modelId, modelOverride] of Object.entries(modelOverrides)) { - if (typeof modelOverride !== "object" || modelOverride === null || Array.isArray(modelOverride)) continue; - const modelOverrideRecord = modelOverride as Record; - migrateHeadersConfig( - modelOverrideRecord.headers, - `${providerLocation}.modelOverrides[${JSON.stringify(modelId)}].headers`, - migrations, - ); - } - } + if (migrations.length === 0) return []; + writeFileSync(modelsPath, `${JSON.stringify(parsed, null, 2)}\n`, "utf-8"); + return migrations; + } catch { + return []; } - - if (migrations.length === 0) return []; - writeFileSync(modelsPath, `${JSON.stringify(parsed, null, 2)}\n`, "utf-8"); - return migrations; } function migrateExplicitEnvVarConfigValues(): void { diff --git a/packages/coding-agent/test/config-value-migration.test.ts b/packages/coding-agent/test/config-value-migration.test.ts index 35d4f155..d0bb8125 100644 --- a/packages/coding-agent/test/config-value-migration.test.ts +++ b/packages/coding-agent/test/config-value-migration.test.ts @@ -3,6 +3,8 @@ import * as os from "node:os"; import * as path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; import { ENV_AGENT_DIR } from "../src/config.ts"; +import { AuthStorage } from "../src/core/auth-storage.ts"; +import { ModelRegistry } from "../src/core/model-registry.ts"; import { runMigrations } from "../src/migrations.ts"; describe("config value env var syntax migration", () => { @@ -68,6 +70,23 @@ describe("config value env var syntax migration", () => { expect(logMessage).toContain('auth.json["anthropic"].key: ANTHROPIC_API_KEY -> $ANTHROPIC_API_KEY'); }); + it.each([ + ["malformed", '{\n "providers": {\n'], + ["blank", ""], + ])("does not throw on %s models.json during config migration", (_name, content) => { + const agentDir = createAgentDir(); + const modelsPath = path.join(agentDir, "models.json"); + fs.writeFileSync(modelsPath, content, "utf-8"); + + withAgentDir(agentDir, () => expect(() => runMigrations(agentDir)).not.toThrow()); + + expect(fs.readFileSync(modelsPath, "utf-8")).toBe(content); + const registry = ModelRegistry.create(AuthStorage.create(path.join(agentDir, "auth.json")), modelsPath); + const loadError = registry.getError(); + expect(loadError).toContain("Failed to parse models.json"); + expect(loadError).toContain(`File: ${modelsPath}`); + }); + it("rewrites legacy uppercase models.json API key and header values", () => { const agentDir = createAgentDir(); fs.writeFileSync(