diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 0bf512a7afc..89ff792ff83 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed OpenCode Go `/login` credentials being shadowed by an existing `OPENCODE_API_KEY` env fallback after switching accounts. ([#4688](https://github.com/can1357/oh-my-pi/issues/4688)) + ## [16.3.10] - 2026-07-06 ### Fixed diff --git a/packages/ai/src/auth-storage.ts b/packages/ai/src/auth-storage.ts index a790ce5d591..d18ba42f328 100644 --- a/packages/ai/src/auth-storage.ts +++ b/packages/ai/src/auth-storage.ts @@ -66,6 +66,7 @@ const USAGE_RANKING_METRIC_EPSILON = 1e-9; export type ApiKeyCredential = { type: "api_key"; key: string; + source?: "login"; }; export type OAuthCredential = { @@ -1554,13 +1555,14 @@ export class AuthStorage { provider: string, type: T, sessionId?: string, + filter?: (credential: AuthCredential) => boolean, ): { credential: Extract; index: number } | undefined { const credentials = this.#getCredentialsForProvider(provider) .map((credential, index) => ({ credential, index })) - .filter( - (entry): entry is { credential: Extract; index: number } => - entry.credential.type === type, - ); + .filter((entry): entry is { credential: Extract; index: number } => { + if (entry.credential.type !== type) return false; + return filter?.(entry.credential) ?? true; + }); if (credentials.length === 0) return undefined; if (credentials.length === 1) return credentials[0]; @@ -1851,8 +1853,8 @@ export class AuthStorage { /** * Classify where a provider's auth comes from, following the same precedence * as {@link AuthStorage.getApiKey}: runtime override → config override → - * stored OAuth → env var → stored api_key → fallback resolver. Returns - * undefined when no auth is configured. + * stored OAuth → login-stored api_key → env var → stored api_key → + * fallback resolver. Returns undefined when no auth is configured. * * Compact, structured counterpart to {@link describeCredentialSource}. */ @@ -1861,6 +1863,9 @@ export class AuthStorage { if (this.#configOverrides.has(provider)) return { kind: "config" }; const stored = this.#getCredentialsForProvider(provider); if (stored.some(credential => credential.type === "oauth")) return { kind: "oauth" }; + if (stored.some(credential => credential.type === "api_key" && credential.source === "login")) { + return { kind: "api_key" }; + } if (getEnvApiKey(provider)) return { kind: "env", envVar: getEnvApiKeyName(provider) }; if (stored.some(credential => credential.type === "api_key")) return { kind: "api_key" }; if (this.#fallbackResolver?.(provider)) return { kind: "fallback" }; @@ -2005,7 +2010,7 @@ export class AuthStorage { if (!result) { return; } - const newCredential: ApiKeyCredential = { type: "api_key", key: result }; + const newCredential: ApiKeyCredential = { type: "api_key", key: result, source: "login" }; const stored = this.#store.upsertAuthCredentialRemote ? await this.#store.upsertAuthCredentialRemote(provider, newCredential) : this.#store.upsertAuthCredentialForProvider(provider, newCredential); @@ -3933,9 +3938,10 @@ export class AuthStorage { * 1. Runtime override (CLI --api-key) * 2. Config override (models.yml `providers..apiKey`) * 3. OAuth token from storage (auto-refreshed) - * 4. Environment variable - * 5. Stored API key (e.g. a broker-migrated copy) — last resort, so an explicit env var wins - * 6. Fallback resolver (models.yml custom providers, last-resort) + * 4. API key persisted by a successful `/login` + * 5. Environment variable + * 6. Stored API key (e.g. a broker-migrated copy) — last resort, so an explicit env var wins + * 7. Fallback resolver (models.yml custom providers, last-resort) */ async getApiKey(provider: string, sessionId?: string, options?: AuthApiKeyOptions): Promise { // Runtime override takes highest priority @@ -3954,13 +3960,24 @@ export class AuthStorage { return configKey; } - // Precedence: a deliberate OAuth login wins, then an explicit env var, then a stored - // static api_key (which may be a stale broker-migrated copy) as a last resort. + // Precedence: a deliberate OAuth/login credential wins, then an explicit env var, + // then a stored static api_key (which may be a stale broker-migrated copy) as a last resort. const oauthResolved = await this.#resolveOAuthSelection(provider, sessionId, options); if (oauthResolved) { return oauthResolved.apiKey; } + const loginApiKeySelection = this.#selectCredentialByType( + provider, + "api_key", + sessionId, + credential => credential.type === "api_key" && credential.source === "login", + ); + if (loginApiKeySelection) { + this.#recordSessionCredential(provider, sessionId, "api_key", loginApiKeySelection.index); + return this.#configValueResolver(loginApiKeySelection.credential.key); + } + // Past OAuth: the session sticky (if any) is stale — the request authenticates via // env/api_key/fallback, not OAuth, so clear it now so getOAuthAccountId() correctly // suppresses account_uuid for this session. @@ -3969,7 +3986,12 @@ export class AuthStorage { const envKey = getEnvApiKey(provider); if (envKey) return envKey; - const apiKeySelection = this.#selectCredentialByType(provider, "api_key", sessionId); + const apiKeySelection = this.#selectCredentialByType( + provider, + "api_key", + sessionId, + credential => credential.type !== "api_key" || credential.source !== "login", + ); if (apiKeySelection) { this.#recordSessionCredential(provider, sessionId, "api_key", apiKeySelection.index); return this.#configValueResolver(apiKeySelection.credential.key); @@ -4674,9 +4696,10 @@ export class AuthStorage { * 1. Runtime override (`--api-key`). * 2. Config override (`models.yml` `providers..apiKey`). * 3. Stored OAuth credential. - * 4. Env var — overrides a stored static api_key (e.g. a stale broker copy). - * 5. Stored api_key credential. - * 6. Fallback resolver. + * 4. API key persisted by a successful `/login`. + * 5. Env var — overrides a stored static api_key (e.g. a stale broker copy). + * 6. Stored api_key credential. + * 7. Fallback resolver. * * The string is purely informational; consumers must not parse it. */ @@ -4691,14 +4714,16 @@ export class AuthStorage { const baseLabel = this.#sourceLabel ?? "local store"; const stored = this.#getStoredCredentials(provider); const session = sessionId ? this.#sessionLastCredential.get(provider)?.get(sessionId) : undefined; - // Describe the stored credential of a given type, honoring the session sticky index. - const describeStored = (type: AuthCredential["type"]): string | undefined => { + const describeStored = ( + type: AuthCredential["type"], + filter?: (credential: AuthCredential) => boolean, + ): string | undefined => { const typed = stored .map((entry, index) => ({ entry, index })) - .filter(({ entry }) => entry.credential.type === type); + .filter(({ entry }) => entry.credential.type === type && (filter?.(entry.credential) ?? true)); if (typed.length === 0) return undefined; - const index = session?.type === type ? session.index : typed[0].index; - const chosen = stored[index] ?? typed[0].entry; + const sticky = session?.type === type ? typed.find(entry => entry.index === session.index) : undefined; + const chosen = sticky?.entry ?? typed[0].entry; const credential = chosen.credential; const identity = credential.type === "oauth" @@ -4707,11 +4732,19 @@ export class AuthStorage { return `${baseLabel} · ${type} #${chosen.id} (${identity})`; }; - // A deliberate OAuth login wins; then an explicit env var; then a stored static api_key. + // Deliberate login credentials win; then an explicit env var; then a stored static api_key. const oauthSource = describeStored("oauth"); if (oauthSource) return oauthSource; + const loginApiKeySource = describeStored( + "api_key", + credential => credential.type === "api_key" && credential.source === "login", + ); + if (loginApiKeySource) return loginApiKeySource; if (getEnvApiKey(provider)) return `env (over ${baseLabel})`; - const apiKeySource = describeStored("api_key"); + const apiKeySource = describeStored( + "api_key", + credential => credential.type !== "api_key" || credential.source !== "login", + ); if (apiKeySource) return apiKeySource; if (this.#fallbackResolver?.(provider) !== undefined) return "fallback resolver"; return undefined; @@ -4776,9 +4809,10 @@ function normalizeStoredIdentityKey(identityKey: string | null | undefined): str function serializeCredential(provider: string, credential: AuthCredential): SerializedCredentialRecord | null { if (credential.type === "api_key") { + const data = credential.source === "login" ? { key: credential.key, source: "login" } : { key: credential.key }; return { credentialType: "api_key", - data: JSON.stringify({ key: credential.key }), + data: JSON.stringify(data), identityKey: null, }; } @@ -4806,7 +4840,8 @@ function deserializeCredential(row: AuthRow): AuthCredential | null { if (row.credential_type === "api_key") { const data = parsed as Record; if (typeof data.key === "string") { - return { type: "api_key", key: data.key }; + const source = data.source === "login" ? "login" : undefined; + return source ? { type: "api_key", key: data.key, source } : { type: "api_key", key: data.key }; } } if (row.credential_type === "oauth") { diff --git a/packages/ai/test/auth-storage-api-key-login.test.ts b/packages/ai/test/auth-storage-api-key-login.test.ts index 879a8a9eca9..5d4fe3c398a 100644 --- a/packages/ai/test/auth-storage-api-key-login.test.ts +++ b/packages/ai/test/auth-storage-api-key-login.test.ts @@ -39,9 +39,9 @@ function countCredentialRowsByDisabledState(dbPath: string, provider: string, di } describe("AuthStorage api-key login upsert", () => { - // A live env var now (correctly) overrides a stored static api_key. These tests verify that a - // freshly stored api_key resolves through AuthStorage.getApiKey, so neutralize the env leg - // entirely — this ignores every provider's ambient env key, not just the few set locally. + // Most tests neutralize the env leg so ambient shell / ~/.env keys cannot + // hide the stored credential behavior under test. Login-persisted API keys + // have their own precedence coverage below. let tempDir = ""; let dbPath = ""; let store: SqliteAuthCredentialStore | null = null; @@ -49,9 +49,10 @@ describe("AuthStorage api-key login upsert", () => { let loginDeepSeekSpy: Mock; let loginKagiSpy: Mock; let loginOllamaCloudSpy: Mock; + let getEnvApiKeySpy: Mock; beforeEach(async () => { - vi.spyOn(aiStream, "getEnvApiKey").mockReturnValue(undefined); + getEnvApiKeySpy = vi.spyOn(aiStream, "getEnvApiKey").mockReturnValue(undefined); tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-ai-auth-api-key-login-")); dbPath = path.join(tempDir, "agent.db"); store = await SqliteAuthCredentialStore.open(dbPath); @@ -118,8 +119,8 @@ describe("AuthStorage api-key login upsert", () => { const credentials = store.listAuthCredentials("kagi"); expect(credentials.map(entry => entry.credential)).toEqual([ - { type: "api_key", key: "first-kagi-key" }, - { type: "api_key", key: "second-kagi-key" }, + { type: "api_key", key: "first-kagi-key", source: "login" }, + { type: "api_key", key: "second-kagi-key", source: "login" }, ]); const rotatedKeys = [await authStorage.getApiKey("kagi"), await authStorage.getApiKey("kagi")].sort(); expect(rotatedKeys).toEqual(["first-kagi-key", "second-kagi-key"]); @@ -188,4 +189,17 @@ describe("AuthStorage api-key login upsert", () => { expect(store.getApiKey("deepseek")).toBe("same-deepseek-key"); expect(await authStorage.getApiKey("deepseek", "session-deepseek-relogin")).toBe("same-deepseek-key"); }); + + it("uses a fresh OpenCode Go login over an existing env fallback", async () => { + if (!authStorage) throw new Error("test setup failed"); + + getEnvApiKeySpy.mockImplementation(provider => (provider === "opencode-go" ? "old-opencode-key" : undefined)); + + await authStorage.login("opencode-go", { + onAuth: () => {}, + onPrompt: async () => "new-opencode-key", + }); + + expect(await authStorage.getApiKey("opencode-go", "session-opencode-go-login")).toBe("new-opencode-key"); + }); });