From 9e9fc7947871a913946f727854ae0a57fbce1863 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Sat, 13 Jun 2026 21:24:22 +0200 Subject: [PATCH] fix(coding-agent): treat uppercase config values as literals closes #5661 --- packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/docs/models.md | 4 +- packages/coding-agent/docs/providers.md | 4 +- .../coding-agent/src/core/model-registry.ts | 80 +--------- .../src/core/resolve-config-value.ts | 5 - packages/coding-agent/src/migrations.ts | 139 +--------------- .../test/config-value-migration.test.ts | 150 ++++++++++-------- .../coding-agent/test/model-registry.test.ts | 52 ++++-- .../5661-uppercase-header-values.test.ts | 91 +++++++++++ 9 files changed, 226 insertions(+), 303 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/5661-uppercase-header-values.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7f677ad9..6b17d85a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed custom provider config so plain uppercase API key and header values remain literals instead of being treated as legacy environment references; use explicit `$ENV_VAR` syntax for environment variables ([#5661](https://github.com/earendil-works/pi/issues/5661)). + ## [0.79.3] - 2026-06-13 ### Fixed diff --git a/packages/coding-agent/docs/models.md b/packages/coding-agent/docs/models.md index 39b56aa1..830eaacd 100644 --- a/packages/coding-agent/docs/models.md +++ b/packages/coding-agent/docs/models.md @@ -161,13 +161,11 @@ The `apiKey` and `headers` fields support command execution, environment interpo "apiKey": "$$literal-dollar-prefix" "apiKey": "$!literal-bang-prefix" ``` -- **Literal value:** Used directly +- **Literal value:** Used directly. Plain uppercase strings such as `MY_API_KEY` are literals; use `$MY_API_KEY` for environment variables. ```json "apiKey": "sk-..." ``` -Legacy uppercase env-var-like values such as `MY_API_KEY` are migrated to `$MY_API_KEY` on startup. - For `models.json`, shell commands are resolved at request time. pi intentionally does not apply built-in TTL, stale reuse, or recovery logic for arbitrary commands. Different commands need different caching and failure strategies, and pi cannot infer the right one. If your command is slow, expensive, rate-limited, or should keep using a previous value on transient failures, wrap it in your own script or command that implements the caching or TTL behavior you want. diff --git a/packages/coding-agent/docs/providers.md b/packages/coding-agent/docs/providers.md index 185405cd..f139aa30 100644 --- a/packages/coding-agent/docs/providers.md +++ b/packages/coding-agent/docs/providers.md @@ -124,13 +124,13 @@ The `key` field supports command execution, environment interpolation, and liter { "type": "api_key", "key": "$$literal-dollar-prefix" } { "type": "api_key", "key": "$!literal-bang-prefix" } ``` -- **Literal value:** Used directly +- **Literal value:** Used directly. Plain uppercase strings such as `MY_API_KEY` are literals; use `$MY_API_KEY` for environment variables. ```json { "type": "api_key", "key": "sk-ant-..." } { "type": "api_key", "key": "public" } ``` -Legacy uppercase env-var-like values such as `MY_API_KEY` are migrated to `$MY_API_KEY` on startup. OAuth credentials are also stored here after `/login` and managed automatically. +OAuth credentials are also stored here after `/login` and managed automatically. ## Cloud Providers diff --git a/packages/coding-agent/src/core/model-registry.ts b/packages/coding-agent/src/core/model-registry.ts index 00c81576..ff172966 100644 --- a/packages/coding-agent/src/core/model-registry.ts +++ b/packages/coding-agent/src/core/model-registry.ts @@ -25,7 +25,6 @@ import { type Static, Type } from "typebox"; import { Compile } from "typebox/compile"; import type { TLocalizedValidationError } from "typebox/error"; import { getAgentDir } from "../config.ts"; -import { warnDeprecation } from "../utils/deprecation.ts"; import { stripJsonComments } from "../utils/json.ts"; import { normalizePath } from "../utils/paths.ts"; import type { AuthStatus, AuthStorage } from "./auth-storage.ts"; @@ -35,7 +34,6 @@ import { getConfigValueEnvVarNames, isCommandConfigValue, isConfigValueConfigured, - isLegacyEnvVarNameConfigValue, resolveConfigValueOrThrow, resolveConfigValueUncached, resolveHeadersOrThrow, @@ -237,77 +235,6 @@ interface ProviderRequestConfig { authHeader?: boolean; } -function migrateLegacyRegisterProviderConfigValue(providerName: string, field: string, value: string): string { - if (!isLegacyEnvVarNameConfigValue(value)) return value; - warnDeprecation( - `registerProvider("${providerName}") ${field} value "${value}" is treated as a legacy environment variable reference. This will no longer be detected as an environment variable reference in a future release. Pass "$${value}" instead.`, - ); - return `$${value}`; -} - -function migrateLegacyRegisterProviderHeaders( - providerName: string, - field: string, - headers: Record | undefined, -): Record | undefined { - if (!headers) return undefined; - let migratedHeaders: Record | undefined; - for (const [key, value] of Object.entries(headers)) { - const migratedValue = migrateLegacyRegisterProviderConfigValue(providerName, `${field} header "${key}"`, value); - if (migratedValue === value) continue; - migratedHeaders ??= { ...headers }; - migratedHeaders[key] = migratedValue; - } - return migratedHeaders ?? headers; -} - -function migrateLegacyRegisterProviderConfigValues( - providerName: string, - config: ProviderConfigInput, -): ProviderConfigInput { - let migratedConfig: ProviderConfigInput | undefined; - - const setMigratedConfigValue = ( - key: TKey, - value: ProviderConfigInput[TKey], - ) => { - migratedConfig ??= { ...config }; - migratedConfig[key] = value; - }; - - if (config.apiKey) { - const apiKey = migrateLegacyRegisterProviderConfigValue(providerName, "apiKey", config.apiKey); - if (apiKey !== config.apiKey) { - setMigratedConfigValue("apiKey", apiKey); - } - } - - const headers = migrateLegacyRegisterProviderHeaders(providerName, "headers", config.headers); - if (headers !== config.headers) { - setMigratedConfigValue("headers", headers); - } - - if (config.models) { - let models: ProviderConfigInput["models"] | undefined; - for (let index = 0; index < config.models.length; index++) { - const model = config.models[index]; - const modelHeaders = migrateLegacyRegisterProviderHeaders( - providerName, - `model "${model.id}" headers`, - model.headers, - ); - if (modelHeaders === model.headers) continue; - models ??= [...config.models]; - models[index] = { ...model, headers: modelHeaders }; - } - if (models) { - setMigratedConfigValue("models", models); - } - } - - return migratedConfig ?? config; -} - export type ResolvedRequestAuth = | { ok: true; @@ -869,10 +796,9 @@ export class ModelRegistry { * If provider has oauth: registers OAuth provider for /login support. */ registerProvider(providerName: string, config: ProviderConfigInput): void { - const migratedConfig = migrateLegacyRegisterProviderConfigValues(providerName, config); - this.validateProviderConfig(providerName, migratedConfig); - this.applyProviderConfig(providerName, migratedConfig); - this.upsertRegisteredProvider(providerName, migratedConfig); + this.validateProviderConfig(providerName, config); + this.applyProviderConfig(providerName, config); + this.upsertRegisteredProvider(providerName, config); } /** diff --git a/packages/coding-agent/src/core/resolve-config-value.ts b/packages/coding-agent/src/core/resolve-config-value.ts index 119e379d..6c2263f6 100644 --- a/packages/coding-agent/src/core/resolve-config-value.ts +++ b/packages/coding-agent/src/core/resolve-config-value.ts @@ -10,7 +10,6 @@ import { getShellConfig } from "../utils/shell.ts"; const commandResultCache = new Map(); const ENV_VAR_NAME_RE = /^[A-Za-z_][A-Za-z0-9_]*$/; const ENV_VAR_NAME_PREFIX_RE = /^[A-Za-z_][A-Za-z0-9_]*/; -const LEGACY_ENV_VAR_NAME_RE = /^[A-Z_][A-Z0-9_]*$/; type TemplatePart = { type: "literal"; value: string } | { type: "env"; name: string }; @@ -136,10 +135,6 @@ export function isConfigValueConfigured(config: string): boolean { return getMissingConfigValueEnvVarNames(config).length === 0; } -export function isLegacyEnvVarNameConfigValue(config: string): boolean { - return LEGACY_ENV_VAR_NAME_RE.test(config); -} - /** * Resolve a config value (API key, header value, etc.) to an actual value. * - If starts with "!", executes the rest as a shell command and uses stdout (cached) diff --git a/packages/coding-agent/src/migrations.ts b/packages/coding-agent/src/migrations.ts index 5cce43b8..39aeea04 100644 --- a/packages/coding-agent/src/migrations.ts +++ b/packages/coding-agent/src/migrations.ts @@ -3,12 +3,10 @@ */ import chalk from "chalk"; -import { chmodSync, existsSync, mkdirSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "fs"; +import { existsSync, mkdirSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from "fs"; import { dirname, join } from "path"; import { CONFIG_DIR_NAME, getAgentDir, getBinDir } from "./config.ts"; import { migrateKeybindingsConfig } from "./core/keybindings.ts"; -import { isLegacyEnvVarNameConfigValue } from "./core/resolve-config-value.ts"; -import { stripJsonComments } from "./utils/json.ts"; const MIGRATION_GUIDE_URL = "https://github.com/earendil-works/pi-mono/blob/main/packages/coding-agent/CHANGELOG.md#extensions-migration"; @@ -74,140 +72,6 @@ export function migrateAuthToAuthJson(): string[] { return providers; } -interface ConfigValueMigration { - location: string; - from: string; - to: string; -} - -function migrateLegacyEnvVarString(value: string): string | undefined { - return isLegacyEnvVarNameConfigValue(value) ? `$${value}` : undefined; -} - -function migrateStringProperty( - record: Record, - key: string, - location: string, - migrations: ConfigValueMigration[], -): boolean { - const value = record[key]; - if (typeof value !== "string") return false; - const migrated = migrateLegacyEnvVarString(value); - if (migrated === undefined) return false; - record[key] = migrated; - migrations.push({ location, from: value, to: migrated }); - return true; -} - -function migrateHeadersConfig(headers: unknown, location: string, migrations: ConfigValueMigration[]): boolean { - if (typeof headers !== "object" || headers === null || Array.isArray(headers)) return false; - const headerRecord = headers as Record; - let migrated = false; - for (const [key, value] of Object.entries(headerRecord)) { - if (typeof value !== "string") continue; - const migratedValue = migrateLegacyEnvVarString(value); - if (migratedValue === undefined) continue; - headerRecord[key] = migratedValue; - migrations.push({ location: `${location}[${JSON.stringify(key)}]`, from: value, to: migratedValue }); - migrated = true; - } - return migrated; -} - -function migrateAuthJsonConfigValues(agentDir: string): ConfigValueMigration[] { - const authPath = join(agentDir, "auth.json"); - if (!existsSync(authPath)) return []; - - try { - const parsed = JSON.parse(readFileSync(authPath, "utf-8")) as unknown; - if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return []; - const authData = parsed as Record; - - const migrations: ConfigValueMigration[] = []; - for (const [provider, credential] of Object.entries(authData)) { - if (typeof credential !== "object" || credential === null || Array.isArray(credential)) continue; - const credentialRecord = credential as Record; - if (credentialRecord.type !== "api_key") continue; - migrateStringProperty(credentialRecord, "key", `auth.json[${JSON.stringify(provider)}].key`, migrations); - } - - if (migrations.length === 0) return []; - writeFileSync(authPath, `${JSON.stringify(parsed, null, 2)}\n`, "utf-8"); - chmodSync(authPath, 0o600); - return migrations; - } catch { - return []; - } -} - -function migrateModelsJsonConfigValues(agentDir: string): ConfigValueMigration[] { - const modelsPath = join(agentDir, "models.json"); - if (!existsSync(modelsPath)) 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); - - 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, - ); - } - } - } - - if (migrations.length === 0) return []; - writeFileSync(modelsPath, `${JSON.stringify(parsed, null, 2)}\n`, "utf-8"); - return migrations; - } catch { - return []; - } -} - -function migrateExplicitEnvVarConfigValues(): void { - const agentDir = getAgentDir(); - const migrations = [...migrateAuthJsonConfigValues(agentDir), ...migrateModelsJsonConfigValues(agentDir)]; - if (migrations.length === 0) return; - - const details = migrations.map((migration) => ` - ${migration.location}: ${migration.from} -> ${migration.to}`); - console.log( - chalk.yellow( - [ - "Warning: Migrated API key/header environment references to explicit $ENV_VAR syntax. Plain strings will be treated as literals.", - ...details, - ].join("\n"), - ), - ); -} - /** * Migrate sessions from ~/.pi/agent/*.jsonl to proper session directories. * @@ -443,7 +307,6 @@ export function runMigrations(cwd: string): { deprecationWarnings: string[]; } { const migratedAuthProviders = migrateAuthToAuthJson(); - migrateExplicitEnvVarConfigValues(); migrateSessionsFromAgentRoot(); migrateToolsToBin(); migrateKeybindingsConfigFile(); diff --git a/packages/coding-agent/test/config-value-migration.test.ts b/packages/coding-agent/test/config-value-migration.test.ts index d0bb8125..8a273b53 100644 --- a/packages/coding-agent/test/config-value-migration.test.ts +++ b/packages/coding-agent/test/config-value-migration.test.ts @@ -37,7 +37,7 @@ describe("config value env var syntax migration", () => { } } - it("rewrites legacy uppercase auth.json API key values to explicit env references", () => { + it("leaves uppercase auth.json API key values unchanged", () => { const agentDir = createAgentDir(); fs.writeFileSync( path.join(agentDir, "auth.json"), @@ -61,19 +61,17 @@ describe("config value env var syntax migration", () => { string, Record >; - expect(migrated.anthropic.key).toBe("$ANTHROPIC_API_KEY"); + expect(migrated.anthropic.key).toBe("ANTHROPIC_API_KEY"); expect(migrated.openai.key).toBe("$OPENAI_API_KEY"); expect(migrated.opencode.key).toBe("public"); expect(migrated.github.access).toBe("ACCESS_TOKEN"); - const logMessage = String(logSpy.mock.calls[0]?.[0] ?? ""); - expect(logMessage).toContain("explicit $ENV_VAR syntax"); - expect(logMessage).toContain('auth.json["anthropic"].key: ANTHROPIC_API_KEY -> $ANTHROPIC_API_KEY'); + expect(logSpy).not.toHaveBeenCalled(); }); it.each([ ["malformed", '{\n "providers": {\n'], ["blank", ""], - ])("does not throw on %s models.json during config migration", (_name, content) => { + ])("does not throw on %s models.json during migrations", (_name, content) => { const agentDir = createAgentDir(); const modelsPath = path.join(agentDir, "models.json"); fs.writeFileSync(modelsPath, content, "utf-8"); @@ -87,71 +85,93 @@ describe("config value env var syntax migration", () => { expect(loadError).toContain(`File: ${modelsPath}`); }); - it("rewrites legacy uppercase models.json API key and header values", () => { + it("leaves uppercase models.json API key and header values unchanged", async () => { const agentDir = createAgentDir(); - fs.writeFileSync( - path.join(agentDir, "models.json"), - `${JSON.stringify( - { - providers: { - "custom-provider": { - baseUrl: "https://example.com/v1", - apiKey: "CUSTOM_API_KEY", - api: "openai-completions", - headers: { - "x-api-key": "HEADER_API_KEY", - "x-literal": "literal", - }, - models: [ - { - id: "model-a", - headers: { "x-model-key": "MODEL_API_KEY" }, + const envKeys = ["CUSTOM_API_KEY", "HEADER_API_KEY", "MODEL_API_KEY", "OVERRIDE_API_KEY"]; + const savedEnv: Record = {}; + for (const key of envKeys) { + savedEnv[key] = process.env[key]; + process.env[key] = `env-${key}`; + } + + try { + fs.writeFileSync( + path.join(agentDir, "models.json"), + `${JSON.stringify( + { + providers: { + "custom-provider": { + baseUrl: "https://example.com/v1", + apiKey: "CUSTOM_API_KEY", + api: "openai-completions", + headers: { + "x-api-key": "HEADER_API_KEY", + "x-literal": "literal", + }, + models: [ + { + id: "model-a", + headers: { "x-model-key": "MODEL_API_KEY" }, + }, + ], + modelOverrides: { + "model-b": { headers: { "x-override-key": "OVERRIDE_API_KEY" } }, }, - ], - modelOverrides: { - "model-b": { headers: { "x-override-key": "OVERRIDE_API_KEY" } }, }, }, }, + null, + 2, + )}\n`, + "utf-8", + ); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + withAgentDir(agentDir, () => runMigrations(agentDir)); + + const migrated = JSON.parse(fs.readFileSync(path.join(agentDir, "models.json"), "utf-8")) as { + providers: Record< + string, + { + apiKey?: string; + headers?: Record; + models?: Array<{ headers?: Record }>; + modelOverrides?: Record }>; + } + >; + }; + const provider = migrated.providers["custom-provider"]!; + expect(provider.apiKey).toBe("CUSTOM_API_KEY"); + expect(provider.headers?.["x-api-key"]).toBe("HEADER_API_KEY"); + expect(provider.headers?.["x-literal"]).toBe("literal"); + expect(provider.models?.[0]?.headers?.["x-model-key"]).toBe("MODEL_API_KEY"); + expect(provider.modelOverrides?.["model-b"]?.headers?.["x-override-key"]).toBe("OVERRIDE_API_KEY"); + expect(logSpy).not.toHaveBeenCalled(); + + const registry = ModelRegistry.create( + AuthStorage.create(path.join(agentDir, "auth.json")), + path.join(agentDir, "models.json"), + ); + const model = registry.find("custom-provider", "model-a"); + expect(model).toBeDefined(); + expect(await registry.getApiKeyForProvider("custom-provider")).toBe("CUSTOM_API_KEY"); + expect(await registry.getApiKeyAndHeaders(model!)).toMatchObject({ + ok: true, + apiKey: "CUSTOM_API_KEY", + headers: { + "x-api-key": "HEADER_API_KEY", + "x-literal": "literal", + "x-model-key": "MODEL_API_KEY", }, - null, - 2, - )}\n`, - "utf-8", - ); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - - withAgentDir(agentDir, () => runMigrations(agentDir)); - - const migrated = JSON.parse(fs.readFileSync(path.join(agentDir, "models.json"), "utf-8")) as { - providers: Record< - string, - { - apiKey?: string; - headers?: Record; - models?: Array<{ headers?: Record }>; - modelOverrides?: Record }>; + }); + } finally { + for (const key of envKeys) { + if (savedEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedEnv[key]; } - >; - }; - const provider = migrated.providers["custom-provider"]!; - expect(provider.apiKey).toBe("$CUSTOM_API_KEY"); - expect(provider.headers?.["x-api-key"]).toBe("$HEADER_API_KEY"); - expect(provider.headers?.["x-literal"]).toBe("literal"); - expect(provider.models?.[0]?.headers?.["x-model-key"]).toBe("$MODEL_API_KEY"); - expect(provider.modelOverrides?.["model-b"]?.headers?.["x-override-key"]).toBe("$OVERRIDE_API_KEY"); - const logMessage = String(logSpy.mock.calls[0]?.[0] ?? ""); - expect(logMessage).toContain( - 'models.json.providers["custom-provider"].apiKey: CUSTOM_API_KEY -> $CUSTOM_API_KEY', - ); - expect(logMessage).toContain( - 'models.json.providers["custom-provider"].headers["x-api-key"]: HEADER_API_KEY -> $HEADER_API_KEY', - ); - expect(logMessage).toContain( - 'models.json.providers["custom-provider"].models["model-a"].headers["x-model-key"]: MODEL_API_KEY -> $MODEL_API_KEY', - ); - expect(logMessage).toContain( - 'models.json.providers["custom-provider"].modelOverrides["model-b"].headers["x-override-key"]: OVERRIDE_API_KEY -> $OVERRIDE_API_KEY', - ); + } + } }); }); diff --git a/packages/coding-agent/test/model-registry.test.ts b/packages/coding-agent/test/model-registry.test.ts index 95b70fbd..3cba5264 100644 --- a/packages/coding-agent/test/model-registry.test.ts +++ b/packages/coding-agent/test/model-registry.test.ts @@ -7,7 +7,6 @@ import { getOAuthProvider } from "@earendil-works/pi-ai/oauth"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { AuthStorage } from "../src/core/auth-storage.ts"; import { clearApiKeyCache, ModelRegistry, type ProviderConfigInput } from "../src/core/model-registry.ts"; -import { clearDeprecationWarningsForTests } from "../src/utils/deprecation.ts"; describe("ModelRegistry", () => { let tempDir: string; @@ -19,7 +18,6 @@ describe("ModelRegistry", () => { mkdirSync(tempDir, { recursive: true }); modelsJsonPath = join(tempDir, "models.json"); authStorage = AuthStorage.create(join(tempDir, "auth.json")); - clearDeprecationWarningsForTests(); }); afterEach(() => { @@ -27,7 +25,6 @@ describe("ModelRegistry", () => { rmSync(tempDir, { recursive: true }); } clearApiKeyCache(); - clearDeprecationWarningsForTests(); vi.restoreAllMocks(); }); @@ -896,26 +893,55 @@ describe("ModelRegistry", () => { expect(registry.getProviderDisplayName("oauth-provider")).toBe("OAuth Provider"); }); - test("registerProvider warns and temporarily treats uppercase apiKey as an env reference", async () => { - const originalEnv = process.env.CUSTOM_NAME; - process.env.CUSTOM_NAME = "legacy-env-key"; + test("registerProvider treats uppercase apiKey and headers as literals", async () => { + const envKeys = ["CUSTOM_NAME", "BEARER", "MODEL_TOKEN"]; + const savedEnv: Record = {}; + for (const key of envKeys) { + savedEnv[key] = process.env[key]; + process.env[key] = `env-${key}`; + } const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); try { const registry = ModelRegistry.create(authStorage, modelsJsonPath); - registry.registerProvider("legacy-provider", { + registry.registerProvider("literal-provider", { ...providerConfig("https://provider.test/v1", [{ id: "demo-model" }], "openai-completions"), apiKey: "CUSTOM_NAME", + headers: { Authorization: "BEARER" }, + models: [ + { + id: "demo-model", + name: "demo-model", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 100000, + maxTokens: 8000, + headers: { "x-model-token": "MODEL_TOKEN" }, + }, + ], }); - expect(await registry.getApiKeyForProvider("legacy-provider")).toBe("legacy-env-key"); - expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('Pass "$CUSTOM_NAME" instead')); + expect(await registry.getApiKeyForProvider("literal-provider")).toBe("CUSTOM_NAME"); + const model = registry.find("literal-provider", "demo-model"); + expect(model).toBeDefined(); + expect(await registry.getApiKeyAndHeaders(model!)).toMatchObject({ + ok: true, + apiKey: "CUSTOM_NAME", + headers: { + Authorization: "BEARER", + "x-model-token": "MODEL_TOKEN", + }, + }); + expect(warnSpy).not.toHaveBeenCalled(); } finally { - if (originalEnv === undefined) { - delete process.env.CUSTOM_NAME; - } else { - process.env.CUSTOM_NAME = originalEnv; + for (const key of envKeys) { + if (savedEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedEnv[key]; + } } } }); diff --git a/packages/coding-agent/test/suite/regressions/5661-uppercase-header-values.test.ts b/packages/coding-agent/test/suite/regressions/5661-uppercase-header-values.test.ts new file mode 100644 index 00000000..e625a3c6 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/5661-uppercase-header-values.test.ts @@ -0,0 +1,91 @@ +import { readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { afterEach, describe, expect, it } 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"; +import { createHarness } from "../harness.ts"; + +describe("regression #5661: uppercase models.json header values", () => { + const cleanups: Array<() => void> = []; + + afterEach(() => { + while (cleanups.length > 0) { + cleanups.pop()?.(); + } + }); + + function withAgentDir(agentDir: string, fn: () => void): void { + const previousAgentDir = process.env[ENV_AGENT_DIR]; + process.env[ENV_AGENT_DIR] = agentDir; + try { + fn(); + } finally { + if (previousAgentDir === undefined) { + delete process.env[ENV_AGENT_DIR]; + } else { + process.env[ENV_AGENT_DIR] = previousAgentDir; + } + } + } + + it("keeps uppercase header strings as literals during startup migrations", async () => { + const harness = await createHarness({ withConfiguredAuth: false }); + cleanups.push(harness.cleanup); + + const envKeys = ["CUSTOM_API_KEY", "BEARER"]; + const savedEnv: Record = {}; + for (const key of envKeys) { + savedEnv[key] = process.env[key]; + process.env[key] = `env-${key}`; + } + cleanups.push(() => { + for (const key of envKeys) { + if (savedEnv[key] === undefined) { + delete process.env[key]; + } else { + process.env[key] = savedEnv[key]; + } + } + }); + + const modelsPath = join(harness.tempDir, "models.json"); + writeFileSync( + modelsPath, + `${JSON.stringify( + { + providers: { + "my-provider": { + baseUrl: "https://example.com/v1", + apiKey: "CUSTOM_API_KEY", + api: "openai-completions", + headers: { Authorization: "BEARER" }, + models: [{ id: "my-model" }], + }, + }, + }, + null, + 2, + )}\n`, + "utf-8", + ); + + withAgentDir(harness.tempDir, () => runMigrations(harness.tempDir)); + + const migrated = JSON.parse(readFileSync(modelsPath, "utf-8")) as { + providers: Record }>; + }; + expect(migrated.providers["my-provider"]?.apiKey).toBe("CUSTOM_API_KEY"); + expect(migrated.providers["my-provider"]?.headers?.Authorization).toBe("BEARER"); + + const registry = ModelRegistry.create(AuthStorage.create(join(harness.tempDir, "auth.json")), modelsPath); + const model = registry.find("my-provider", "my-model"); + expect(model).toBeDefined(); + expect(await registry.getApiKeyAndHeaders(model!)).toMatchObject({ + ok: true, + apiKey: "CUSTOM_API_KEY", + headers: { Authorization: "BEARER" }, + }); + }); +});