fix(coding-agent): show models.json auth in login selector
This commit is contained in:
@@ -35,7 +35,7 @@ export type AuthStorageData = Record<string, AuthCredential>;
|
||||
|
||||
export type AuthStatus = {
|
||||
configured: boolean;
|
||||
source?: "stored" | "runtime" | "environment" | "fallback";
|
||||
source?: "stored" | "runtime" | "environment" | "fallback" | "models_json_key" | "models_json_command";
|
||||
label?: string;
|
||||
};
|
||||
|
||||
|
||||
@@ -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.
|
||||
*/
|
||||
|
||||
@@ -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" };
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)}`);
|
||||
|
||||
@@ -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" });
|
||||
});
|
||||
});
|
||||
@@ -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];
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user