diff --git a/packages/coding-agent/src/core/auth-storage.ts b/packages/coding-agent/src/core/auth-storage.ts index fc2adb8b..dc8fabda 100644 --- a/packages/coding-agent/src/core/auth-storage.ts +++ b/packages/coding-agent/src/core/auth-storage.ts @@ -35,7 +35,7 @@ export type AuthStorageData = Record; export type AuthStatus = { configured: boolean; - source?: "stored" | "runtime" | "environment" | "fallback"; + source?: "stored" | "runtime" | "environment" | "fallback" | "models_json_key" | "models_json_command"; label?: string; }; diff --git a/packages/coding-agent/src/core/model-registry.ts b/packages/coding-agent/src/core/model-registry.ts index 74f1efbb..362d8df5 100644 --- a/packages/coding-agent/src/core/model-registry.ts +++ b/packages/coding-agent/src/core/model-registry.ts @@ -25,7 +25,7 @@ import { type Static, Type } from "typebox"; import { Compile } from "typebox/compile"; import type { TLocalizedValidationError } from "typebox/error"; import { getAgentDir } from "../config.js"; -import type { AuthStorage } from "./auth-storage.js"; +import type { AuthStatus, AuthStorage } from "./auth-storage.js"; import { clearConfigValueCache, resolveConfigValueOrThrow, @@ -701,6 +701,32 @@ export class ModelRegistry { } } + /** + * Return auth status for a provider, including request auth configured in models.json. + * This intentionally does not execute command-backed config values. + */ + getProviderAuthStatus(provider: string): AuthStatus { + const authStatus = this.authStorage.getAuthStatus(provider); + if (authStatus.source) { + return authStatus; + } + + const providerApiKey = this.providerRequestConfigs.get(provider)?.apiKey; + if (!providerApiKey) { + return authStatus; + } + + if (providerApiKey.startsWith("!")) { + return { configured: true, source: "models_json_command" }; + } + + if (process.env[providerApiKey]) { + return { configured: true, source: "environment", label: providerApiKey }; + } + + return { configured: true, source: "models_json_key" }; + } + /** * Get API key for a provider. */ diff --git a/packages/coding-agent/src/modes/interactive/components/auth-selector-status.ts b/packages/coding-agent/src/modes/interactive/components/auth-selector-status.ts deleted file mode 100644 index dc9ffc53..00000000 --- a/packages/coding-agent/src/modes/interactive/components/auth-selector-status.ts +++ /dev/null @@ -1,44 +0,0 @@ -import type { AuthCredential, AuthStatus } from "../../../core/auth-storage.js"; - -export type SelectorAuthType = "oauth" | "api_key"; - -export type AuthSelectorIndicator = - | { kind: "configured"; label: string } - | { kind: "configured-other"; label: string } - | { kind: "runtime" } - | { kind: "environment"; label: string } - | { kind: "fallback" } - | { kind: "unconfigured" }; - -function getConfiguredLabel(authType: SelectorAuthType): string { - return authType === "oauth" ? "subscription configured" : "api key configured"; -} - -export function getAuthSelectorIndicator( - authType: SelectorAuthType, - credential: AuthCredential | undefined, - authStatus?: AuthStatus, -): AuthSelectorIndicator { - if (credential) { - const label = getConfiguredLabel(credential.type); - return credential.type === authType ? { kind: "configured", label } : { kind: "configured-other", label }; - } - - if (authType === "oauth") { - return { kind: "unconfigured" }; - } - - if (authStatus?.source === "runtime") { - return { kind: "runtime" }; - } - - if (authStatus?.source === "environment") { - return { kind: "environment", label: authStatus.label ?? "API key" }; - } - - if (authStatus?.source === "fallback") { - return { kind: "fallback" }; - } - - return { kind: "unconfigured" }; -} diff --git a/packages/coding-agent/src/modes/interactive/components/oauth-selector.ts b/packages/coding-agent/src/modes/interactive/components/oauth-selector.ts index aeede4b3..1925e206 100644 --- a/packages/coding-agent/src/modes/interactive/components/oauth-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/oauth-selector.ts @@ -7,9 +7,8 @@ import { Spacer, TruncatedText, } from "@mariozechner/pi-tui"; -import type { AuthStorage } from "../../../core/auth-storage.js"; +import type { AuthStatus, AuthStorage } from "../../../core/auth-storage.js"; import { theme } from "../theme/theme.js"; -import { getAuthSelectorIndicator } from "./auth-selector-status.js"; import { DynamicBorder } from "./dynamic-border.js"; export type AuthSelectorProvider = { @@ -40,6 +39,7 @@ export class OAuthSelectorComponent extends Container implements Focusable { private selectedIndex: number = 0; private mode: "login" | "logout"; private authStorage: AuthStorage; + private getAuthStatus: (providerId: string) => AuthStatus; private onSelectCallback: (providerId: string) => void; private onCancelCallback: () => void; @@ -49,11 +49,13 @@ export class OAuthSelectorComponent extends Container implements Focusable { providers: AuthSelectorProvider[], onSelect: (providerId: string) => void, onCancel: () => void, + getAuthStatus?: (providerId: string) => AuthStatus, ) { super(); this.mode = mode; this.authStorage = authStorage; + this.getAuthStatus = getAuthStatus ?? ((providerId) => this.authStorage.getAuthStatus(providerId)); this.allProviders = providers; this.filteredProviders = providers; this.onSelectCallback = onSelect; @@ -147,29 +149,29 @@ export class OAuthSelectorComponent extends Container implements Focusable { } private formatStatusIndicator(provider: AuthSelectorProvider): string { - const status = provider.authType === "api_key" ? this.authStorage.getAuthStatus(provider.id) : undefined; - const indicator = getAuthSelectorIndicator(provider.authType, this.authStorage.get(provider.id), status); + const credential = this.authStorage.get(provider.id); + if (credential?.type === provider.authType) return theme.fg("success", " ✓ configured"); + if (credential) { + const label = credential.type === "oauth" ? "subscription configured" : "API key configured"; + return theme.fg("muted", " • ") + theme.fg("warning", label); + } + if (provider.authType !== "api_key") return theme.fg("muted", " • unconfigured"); - if (indicator.kind === "configured") { - return theme.fg("success", ` ✓ ${indicator.label}`); + const status = this.getAuthStatus(provider.id); + switch (status.source) { + case "environment": + return theme.fg("success", ` ✓ env: ${status.label ?? "API key"}`); + case "runtime": + return theme.fg("success", " ✓ runtime API key"); + case "fallback": + return theme.fg("success", " ✓ custom API key"); + case "models_json_key": + return theme.fg("success", " ✓ key in models.json"); + case "models_json_command": + return theme.fg("success", " ✓ command in models.json"); + default: + return theme.fg("muted", " • unconfigured"); } - - if (indicator.kind === "configured-other") { - return theme.fg("muted", " • ") + theme.fg("warning", indicator.label); - } - - const base = theme.fg("muted", " • unconfigured"); - if (indicator.kind === "environment") { - return base + theme.fg("success", ` · env: ${indicator.label}`); - } - if (indicator.kind === "runtime") { - return base + theme.fg("success", " · runtime API key"); - } - if (indicator.kind === "fallback") { - return base + theme.fg("success", " · custom API key"); - } - - return base; } handleInput(keyData: string): void { diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index fff361a1..951be6df 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -4438,6 +4438,7 @@ export class InteractiveMode { done(); this.showLoginAuthTypeSelector(); }, + (providerId) => this.session.modelRegistry.getProviderAuthStatus(providerId), ); return { component: selector, focus: selector }; }); @@ -4451,7 +4452,9 @@ export class InteractiveMode { const providerOptions = this.getLogoutProviderOptions(); if (providerOptions.length === 0) { - this.showStatus("No providers logged in. Use /login first."); + this.showStatus( + "No stored credentials to remove. /logout only removes credentials saved by /login; environment variables and models.json config are unchanged.", + ); return; } @@ -4475,7 +4478,7 @@ export class InteractiveMode { const message = providerOption.authType === "oauth" ? `Logged out of ${providerOption.name}` - : `Removed API key for ${providerOption.name}`; + : `Removed stored API key for ${providerOption.name}. Environment variables and models.json config are unchanged.`; this.showStatus(message); } catch (error: unknown) { this.showError(`Logout failed: ${error instanceof Error ? error.message : String(error)}`); diff --git a/packages/coding-agent/test/auth-selector-status.test.ts b/packages/coding-agent/test/auth-selector-status.test.ts deleted file mode 100644 index fc2fdb0c..00000000 --- a/packages/coding-agent/test/auth-selector-status.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { describe, expect, test } from "vitest"; -import { getAuthSelectorIndicator } from "../src/modes/interactive/components/auth-selector-status.js"; - -describe("getAuthSelectorIndicator", () => { - test("shows subscription configured when oauth matches the selector", () => { - expect( - getAuthSelectorIndicator("oauth", { - type: "oauth", - access: "access-token", - refresh: "refresh-token", - expires: Date.now() + 60_000, - }), - ).toEqual({ kind: "configured", label: "subscription configured" }); - }); - - test("shows api key configured in the subscription selector when an api key is stored", () => { - expect( - getAuthSelectorIndicator("oauth", { - type: "api_key", - key: "sk-test", - }), - ).toEqual({ kind: "configured-other", label: "api key configured" }); - }); - - test("shows api key configured when api key auth matches the selector", () => { - expect( - getAuthSelectorIndicator("api_key", { - type: "api_key", - key: "sk-test", - }), - ).toEqual({ kind: "configured", label: "api key configured" }); - }); - - test("shows subscription configured in the api key selector when oauth is stored", () => { - expect( - getAuthSelectorIndicator("api_key", { - type: "oauth", - access: "access-token", - refresh: "refresh-token", - expires: Date.now() + 60_000, - }), - ).toEqual({ kind: "configured-other", label: "subscription configured" }); - }); - - test("keeps env api key hints in the api key selector", () => { - expect( - getAuthSelectorIndicator("api_key", undefined, { - configured: false, - source: "environment", - label: "ANTHROPIC_API_KEY", - }), - ).toEqual({ kind: "environment", label: "ANTHROPIC_API_KEY" }); - }); - - test("ignores api key env hints in the subscription selector", () => { - expect( - getAuthSelectorIndicator("oauth", undefined, { - configured: false, - source: "environment", - label: "ANTHROPIC_API_KEY", - }), - ).toEqual({ kind: "unconfigured" }); - }); -}); diff --git a/packages/coding-agent/test/model-registry.test.ts b/packages/coding-agent/test/model-registry.test.ts index 9ce5f204..9c2a6ec7 100644 --- a/packages/coding-agent/test/model-registry.test.ts +++ b/packages/coding-agent/test/model-registry.test.ts @@ -1164,6 +1164,64 @@ describe("ModelRegistry", () => { expect(count).toBe(2); }); + test("provider auth status reports apiKey environment variables from models.json", () => { + const envVarName = "TEST_API_KEY_STATUS_TEST_98765"; + const originalEnv = process.env[envVarName]; + + try { + process.env[envVarName] = "status-test-key"; + + writeRawModelsJson({ + "custom-provider": providerWithApiKey(envVarName), + }); + + const registry = ModelRegistry.create(authStorage, modelsJsonPath); + + expect(registry.getProviderAuthStatus("custom-provider")).toEqual({ + configured: true, + source: "environment", + label: envVarName, + }); + } finally { + if (originalEnv === undefined) { + delete process.env[envVarName]; + } else { + process.env[envVarName] = originalEnv; + } + } + }); + + test("provider auth status reports non-env apiKey values from models.json as a config key", () => { + writeRawModelsJson({ + "custom-provider": providerWithApiKey("literal_api_key_value"), + }); + + const registry = ModelRegistry.create(authStorage, modelsJsonPath); + + expect(registry.getProviderAuthStatus("custom-provider")).toEqual({ + configured: true, + source: "models_json_key", + }); + }); + + test("provider auth status reports command apiKey values from models.json without executing them", () => { + const counterFile = join(tempDir, "status-counter"); + writeFileSync(counterFile, "0"); + const counterPath = toShPath(counterFile); + const command = `!sh -c 'echo 1 > "${counterPath}"; echo key-value'`; + writeRawModelsJson({ + "custom-provider": providerWithApiKey(command), + }); + + const registry = ModelRegistry.create(authStorage, modelsJsonPath); + + expect(registry.getProviderAuthStatus("custom-provider")).toEqual({ + configured: true, + source: "models_json_command", + }); + expect(readFileSync(counterFile, "utf-8")).toBe("0"); + }); + test("environment variables are not cached (changes are picked up)", async () => { const envVarName = "TEST_API_KEY_CACHE_TEST_98765"; const originalEnv = process.env[envVarName]; diff --git a/packages/coding-agent/test/oauth-selector.test.ts b/packages/coding-agent/test/oauth-selector.test.ts index c7be8665..6b2c4c10 100644 --- a/packages/coding-agent/test/oauth-selector.test.ts +++ b/packages/coding-agent/test/oauth-selector.test.ts @@ -34,12 +34,35 @@ describe("OAuthSelectorComponent", () => { expect(getApiKeyProviderDisplayName("anthropic")).toBe("Anthropic"); expect(isApiKeyLoginProvider("openai", oauthProviderIds, builtInProviderIds)).toBe(true); expect(isApiKeyLoginProvider("github-copilot", oauthProviderIds, builtInProviderIds)).toBe(false); - expect(isApiKeyLoginProvider("amazon-bedrock", oauthProviderIds, builtInProviderIds)).toBe(false); + expect(isApiKeyLoginProvider("amazon-bedrock", oauthProviderIds, builtInProviderIds)).toBe(true); expect(isApiKeyLoginProvider("custom-oauth", oauthProviderIds, builtInProviderIds)).toBe(false); expect(isApiKeyLoginProvider("custom-api", oauthProviderIds, builtInProviderIds)).toBe(true); }); - it("shows environment API key auth while keeping provider unconfigured", () => { + it("shows stored OAuth auth distinctly in the API key selector", () => { + const authStorage = AuthStorage.inMemory({ + anthropic: { + type: "oauth", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }, + }); + const selector = new OAuthSelectorComponent( + "login", + authStorage, + [{ id: "anthropic", name: "Anthropic", authType: "api_key" }], + () => {}, + () => {}, + ); + + const output = stripAnsi(selector.render(120).join("\n")); + + expect(output).toContain("Anthropic"); + expect(output).toContain("subscription configured"); + }); + + it("shows environment API key auth as configured", () => { process.env.OPENAI_API_KEY = "test-openai-key"; const authStorage = AuthStorage.inMemory(); const selector = new OAuthSelectorComponent( @@ -53,7 +76,61 @@ describe("OAuthSelectorComponent", () => { const output = stripAnsi(selector.render(120).join("\n")); expect(output).toContain("OpenAI"); - expect(output).toContain("unconfigured"); - expect(output).toContain("env: OPENAI_API_KEY"); + expect(output).toContain("✓ env: OPENAI_API_KEY"); + expect(output).not.toContain("unconfigured"); + }); + + it("shows custom provider environment API key auth from status resolver", () => { + const authStorage = AuthStorage.inMemory(); + const selector = new OAuthSelectorComponent( + "login", + authStorage, + [{ id: "ollama", name: "ollama", authType: "api_key" }], + () => {}, + () => {}, + () => ({ configured: true, source: "environment", label: "OLLAMA_API_KEY" }), + ); + + const output = stripAnsi(selector.render(120).join("\n")); + + expect(output).toContain("ollama"); + expect(output).toContain("✓ env: OLLAMA_API_KEY"); + expect(output).not.toContain("unconfigured"); + }); + + it("shows models.json API key auth as configured", () => { + const authStorage = AuthStorage.inMemory(); + const selector = new OAuthSelectorComponent( + "login", + authStorage, + [{ id: "local-proxy", name: "local-proxy", authType: "api_key" }], + () => {}, + () => {}, + () => ({ configured: true, source: "models_json_key" }), + ); + + const output = stripAnsi(selector.render(120).join("\n")); + + expect(output).toContain("local-proxy"); + expect(output).toContain("✓ key in models.json"); + expect(output).not.toContain("unconfigured"); + }); + + it("shows models.json command auth as configured", () => { + const authStorage = AuthStorage.inMemory(); + const selector = new OAuthSelectorComponent( + "login", + authStorage, + [{ id: "op-proxy", name: "op-proxy", authType: "api_key" }], + () => {}, + () => {}, + () => ({ configured: true, source: "models_json_command" }), + ); + + const output = stripAnsi(selector.render(120).join("\n")); + + expect(output).toContain("op-proxy"); + expect(output).toContain("✓ command in models.json"); + expect(output).not.toContain("unconfigured"); }); });