From 03cb1443e5185d130b22d792a322cf7000eb4694 Mon Sep 17 00:00:00 2001 From: zw-xysk Date: Sun, 9 Aug 2026 19:53:24 +0800 Subject: [PATCH] fix(agents): canonicalize auth profile aliases (#119505) Persist inherited per-agent order references, collapse equivalent auth-provider state keys onto the canonical provider, and clear every equivalent persisted key. Make auth-order validation and display alias-aware while leaving separate OAuth runtime-cache behavior unchanged. Fixes #119233 Co-authored-by: Peter Steinberger --- src/agents/auth-profiles/profiles.test.ts | 198 +++++++++++++++++++++- src/agents/auth-profiles/profiles.ts | 114 +++++++------ src/commands/models/auth-order.test.ts | 24 +++ src/commands/models/auth-order.ts | 27 +-- 4 files changed, 302 insertions(+), 61 deletions(-) diff --git a/src/agents/auth-profiles/profiles.test.ts b/src/agents/auth-profiles/profiles.test.ts index aee35a28905a..b7050c689774 100644 --- a/src/agents/auth-profiles/profiles.test.ts +++ b/src/agents/auth-profiles/profiles.test.ts @@ -6,7 +6,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { resolveOAuthDir } from "../../config/paths.js"; import { closeOpenClawAgentDatabasesForTest, @@ -19,9 +19,12 @@ import { testing as externalAuthTesting } from "./external-auth.test-support.js" import { loadPersistedAuthProfileStore } from "./persisted.js"; import { clearLastGoodProfileWithLock, + markAuthProfileSuccess, promoteAuthProfileInOrder, removeAuthProfilesAcrossOwnerStores, removeAuthProfilesWithLock, + removeProviderAuthProfilesWithLock, + setAuthProfileOrder, upsertAuthProfileWithLock, } from "./profiles.js"; import { @@ -46,6 +49,19 @@ import { import { testing as storeTesting } from "./store.test-support.js"; import type { AuthProfileStore, RuntimeAuthProfileStore } from "./types.js"; +vi.mock("../provider-auth-aliases.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + resolveProviderIdForAuth: (...args: Parameters) => { + const provider = args[0].trim().toLowerCase(); + return provider === "gmi-cloud" || provider === "gmicloud" + ? "gmi" + : actual.resolveProviderIdForAuth(...args); + }, + }; +}); + type ExpectedOAuthCredentialFields = { provider: string; access?: string; @@ -1398,4 +1414,184 @@ describe("promoteAuthProfileInOrder", () => { }); }); }); + +describe("setAuthProfileOrder", () => { + it("canonicalizes every alias-equivalent provider state mutation", async () => { + await withAuthProfileTestState("openclaw-auth-alias-state-", async ({ agentDir }) => { + fs.mkdirSync(agentDir, { recursive: true }); + const primary = "gmi:primary"; + const secondary = "gmi:secondary"; + const profiles = { + [primary]: { type: "api_key" as const, provider: "gmi", key: "primary" }, + [secondary]: { type: "api_key" as const, provider: "gmi", key: "secondary" }, + "openai:other": { type: "api_key" as const, provider: "openai", key: "other" }, + }; + const seeded = (): AuthProfileStore => ({ + version: AUTH_STORE_VERSION, + profiles, + order: { + "gmi-cloud": [primary], + openai: ["openai:other"], + gmicloud: [secondary], + }, + lastGood: { gmicloud: secondary, openai: "openai:other", "gmi-cloud": primary }, + }); + + saveAuthProfileStore(seeded(), agentDir); + clearRuntimeAuthProfileStoreSnapshots(); + await setAuthProfileOrder({ agentDir, provider: "gmi-cloud", order: [secondary] }); + expect(loadPersistedAuthProfileStore(agentDir)?.order).toEqual({ + openai: ["openai:other"], + gmi: [secondary], + }); + saveAuthProfileStore( + { + ...seeded(), + order: { ...seeded().order, gmi: [primary] }, + }, + agentDir, + ); + clearRuntimeAuthProfileStoreSnapshots(); + await setAuthProfileOrder({ agentDir, provider: "gmi-cloud", order: null }); + expect(loadPersistedAuthProfileStore(agentDir)?.order).toEqual({ + openai: ["openai:other"], + }); + + saveAuthProfileStore( + { + ...seeded(), + order: { ...seeded().order, "gmi-cloud": [secondary, primary] }, + }, + agentDir, + ); + clearRuntimeAuthProfileStoreSnapshots(); + await promoteAuthProfileInOrder({ agentDir, provider: "gmi-cloud", profileId: secondary }); + expect(loadPersistedAuthProfileStore(agentDir)?.order).toEqual({ + openai: ["openai:other"], + gmi: [secondary, primary], + }); + + saveAuthProfileStore(seeded(), agentDir); + clearRuntimeAuthProfileStoreSnapshots(); + await clearLastGoodProfileWithLock({ agentDir, provider: "gmi-cloud", profileId: secondary }); + expect(loadPersistedAuthProfileStore(agentDir)?.lastGood).toEqual({ + openai: "openai:other", + }); + + saveAuthProfileStore(seeded(), agentDir); + clearRuntimeAuthProfileStoreSnapshots(); + const runtimeStore = loadAuthProfileStoreForRuntime(agentDir); + await markAuthProfileSuccess({ + agentDir, + profileId: secondary, + provider: "gmi-cloud", + store: runtimeStore, + }); + expect(loadPersistedAuthProfileStore(agentDir)?.lastGood).toEqual({ + openai: "openai:other", + gmi: secondary, + }); + + saveAuthProfileStore(seeded(), agentDir); + clearRuntimeAuthProfileStoreSnapshots(); + await removeProviderAuthProfilesWithLock({ agentDir, provider: "gmi-cloud" }); + expect(loadPersistedAuthProfileStore(agentDir)).toMatchObject({ + profiles: { "openai:other": expect.any(Object) }, + order: { openai: ["openai:other"] }, + lastGood: { openai: "openai:other" }, + }); + }); + }); + + it("preserves inherited main OAuth profile IDs in a secondary agent order without copying credentials", async () => { + await withAuthProfileTestState( + "openclaw-auth-order-set-inherited-", + async ({ agentDirFor }) => { + const mainAgentDir = agentDirFor("main"); + const customAgentDir = agentDirFor("custom"); + fs.mkdirSync(mainAgentDir, { recursive: true }); + fs.mkdirSync(customAgentDir, { recursive: true }); + // Main agent owns two OAuth profiles; the secondary agent inherits them + // at runtime and has no local credential copies. + const mainStore = (): AuthProfileStore => ({ + version: AUTH_STORE_VERSION, + profiles: { + "openai:profile-a": { + type: "oauth", + provider: "openai", + access: "access-a", + refresh: "refresh-a", + expires: Date.now() + 60_000, + }, + "openai:profile-b": { + type: "oauth", + provider: "openai", + access: "access-b", + refresh: "refresh-b", + expires: Date.now() + 60_000, + }, + }, + order: { openai: ["openai:profile-a"] }, + }); + saveAuthProfileStore(mainStore()); + + // The secondary agent selects the other inherited profile ID. Before the + // fix, the local save pruned this ID because the secondary store does + // not own the OAuth credential, so `order get` fell back to main's + // profile-a (issue #119233). + const updated = await setAuthProfileOrder({ + agentDir: customAgentDir, + provider: "openai", + order: ["openai:profile-b"], + }); + + expect(updated?.order?.openai).toEqual(["openai:profile-b"]); + // Reload from persistence: the inherited ID must survive, not be pruned. + expect(loadPersistedAuthProfileStore(customAgentDir)?.order?.openai).toEqual([ + "openai:profile-b", + ]); + // The runtime store for the secondary agent reflects the local override. + expect(loadAuthProfileStoreForRuntime(customAgentDir).order?.openai).toEqual([ + "openai:profile-b", + ]); + // The secondary agent must not gain a local copy of the inherited OAuth + // credential — only the order reference is preserved. + const persistedCustom = loadPersistedAuthProfileStore(customAgentDir); + expect(persistedCustom?.profiles["openai:profile-b"]).toBeUndefined(); + expect(persistedCustom?.profiles["openai:profile-a"]).toBeUndefined(); + }, + { clearOAuthDir: true }, + ); + }); + + it("clears a provider order without preserving any profile IDs", async () => { + await withAuthProfileTestState( + "openclaw-auth-order-set-clear-", + async ({ agentDir }) => { + fs.mkdirSync(agentDir, { recursive: true }); + saveAuthProfileStore({ + version: AUTH_STORE_VERSION, + profiles: { + "openai:local": { + type: "api_key", + provider: "openai", + key: "sk-local", + }, + }, + order: { openai: ["openai:local"] }, + }); + + const updated = await setAuthProfileOrder({ + agentDir, + provider: "openai", + order: null, + }); + + expect(updated?.order?.openai ?? null).toBeNull(); + expect(loadPersistedAuthProfileStore(agentDir)?.order?.openai ?? null).toBeNull(); + }, + { clearOAuthDir: true }, + ); + }); +}); /* oxlint-disable max-lines -- TODO: split this grandfathered oversized file. */ diff --git a/src/agents/auth-profiles/profiles.ts b/src/agents/auth-profiles/profiles.ts index 64781726c915..715f53429ca4 100644 --- a/src/agents/auth-profiles/profiles.ts +++ b/src/agents/auth-profiles/profiles.ts @@ -3,10 +3,7 @@ * Updates profile order, last-good state, usage stats, and provider profile * records through locked or immediate store writes. */ -import { - findNormalizedProviderKey, - normalizeProviderId, -} from "@openclaw/model-catalog-core/provider-id"; +import { normalizeProviderId } from "@openclaw/model-catalog-core/provider-id"; import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { resolveProviderIdForAuth } from "../provider-auth-aliases.js"; @@ -28,21 +25,44 @@ export { upsertAuthProfileWithLock, upsertAuthProfileWithLockOrThrow } from "./u const authProfileProfilesLog = createSubsystemLogger("agent/embedded"); -// Auth profile order/lastGood keys may be stored as aliases. Resolve through -// auth provider normalization before updating per-provider state. -function findProviderAuthStateKey( - entries: Record | undefined, - providerKey: string, -): string | undefined { - if (!entries) { - return undefined; - } - const normalizedProviderKey = resolveProviderIdForAuth(providerKey); - return Object.keys(entries).find( - (key) => resolveProviderIdForAuth(key) === normalizedProviderKey, +function listProviderAuthStateEntries( + entries: Record | undefined, + provider: string, +): Array<[string, T]> { + const canonicalProvider = resolveProviderIdForAuth(provider); + return Object.entries(entries ?? {}) + .filter(([key]) => resolveProviderIdForAuth(key) === canonicalProvider) + .toSorted(([left], [right]) => left.localeCompare(right)); +} + +function readProviderAuthState( + entries: Record | undefined, + provider: string, +): T | undefined { + const canonicalProvider = resolveProviderIdForAuth(provider); + const matches = listProviderAuthStateEntries(entries, canonicalProvider); + return ( + matches.find(([key]) => normalizeProviderId(key) === canonicalProvider)?.[1] ?? matches[0]?.[1] ); } +function replaceProviderAuthState( + entries: Record | undefined, + provider: string, + value?: T, +): Record | undefined { + const canonicalProvider = resolveProviderIdForAuth(provider); + const next = Object.fromEntries( + Object.entries(entries ?? {}).filter( + ([key]) => resolveProviderIdForAuth(key) !== canonicalProvider, + ), + ) as Record; + if (value !== undefined) { + next[canonicalProvider] = value; + } + return Object.keys(next).length > 0 ? next : undefined; +} + // Successful auth clears transient failure/cooldown/disable state while keeping // unrelated metadata and updating lastUsed for round-robin ordering. function resetSuccessfulUsageStats( @@ -81,26 +101,31 @@ export async function setAuthProfileOrder(params: { provider: string; order?: string[] | null; }): Promise { - const providerKey = normalizeProviderId(params.provider); + const providerKey = resolveProviderIdForAuth(params.provider); const sanitized = params.order && Array.isArray(params.order) ? normalizeStringEntries(params.order) : []; const deduped = dedupeProfileIds(sanitized); return await updateAuthProfileStoreWithLock({ agentDir: params.agentDir, + // Preserve requested IDs that the agent inherits (not owns) so the local + // save path does not prune them from the order. Without this, a secondary + // agent's `models auth order set --agent` accepts an inherited profile ID + // (validated against the merged store) but drops it while persisting, so + // `order get` falls back to the inherited main order — the CLI reports a + // switch that never happened (issue #119233). Mirrors the adjacent + // promoteAuthProfileInOrder preservation contract; the clear-order path + // (deduped.length === 0) must not preserve anything. + ...(deduped.length > 0 ? { saveOptions: { preserveOrderProfileIds: deduped } } : {}), updater: (store) => { - store.order = store.order ?? {}; if (deduped.length === 0) { - if (!store.order[providerKey]) { + if (listProviderAuthStateEntries(store.order, providerKey).length === 0) { return false; } - delete store.order[providerKey]; - if (Object.keys(store.order).length === 0) { - store.order = undefined; - } + store.order = replaceProviderAuthState(store.order, providerKey); return true; } - store.order[providerKey] = deduped; + store.order = replaceProviderAuthState(store.order, providerKey, deduped); return true; }, }); @@ -125,11 +150,8 @@ export async function promoteAuthProfileInOrder(params: { if (!profile || resolveProviderIdForAuth(profile.provider) !== providerKey) { return false; } - const orderKey = - findProviderAuthStateKey(store.order, providerKey) ?? - findNormalizedProviderKey(store.order, providerKey) ?? - normalizeProviderId(providerKey); - const existing = store.order?.[orderKey]; + const matchingOrderEntries = listProviderAuthStateEntries(store.order, providerKey); + const existing = readProviderAuthState(store.order, providerKey); if (!existing || existing.length === 0) { if (!params.createIfMissing) { return false; @@ -143,7 +165,7 @@ export async function promoteAuthProfileInOrder(params: { params.profileId, ...providerProfiles.filter((profileId) => profileId !== params.profileId), ]); - store.order = { ...store.order, [orderKey]: next }; + store.order = replaceProviderAuthState(store.order, providerKey, next); return true; } const next = dedupeProfileIds([ @@ -152,11 +174,13 @@ export async function promoteAuthProfileInOrder(params: { ]); if ( next.length === existing.length && - next.every((profileId, idx) => profileId === existing[idx]) + next.every((profileId, idx) => profileId === existing[idx]) && + matchingOrderEntries.length === 1 && + matchingOrderEntries[0]?.[0] === providerKey ) { return false; } - store.order = { ...store.order, [orderKey]: next }; + store.order = replaceProviderAuthState(store.order, providerKey, next); return true; }, }); @@ -183,7 +207,6 @@ export async function removeProviderAuthProfilesWithLock(params: { agentDir?: string; }): Promise { const providerKey = resolveProviderIdForAuth(params.provider); - const storeOrderKey = normalizeProviderId(params.provider); return await updateAuthProfileStoreWithLock({ agentDir: params.agentDir, updater: (store) => { @@ -199,19 +222,13 @@ export async function removeProviderAuthProfilesWithLock(params: { changed = true; } } - if (store.order?.[storeOrderKey]) { - delete store.order[storeOrderKey]; + if (listProviderAuthStateEntries(store.order, providerKey).length > 0) { + store.order = replaceProviderAuthState(store.order, providerKey); changed = true; - if (Object.keys(store.order).length === 0) { - store.order = undefined; - } } - if (store.lastGood?.[providerKey]) { - delete store.lastGood[providerKey]; + if (listProviderAuthStateEntries(store.lastGood, providerKey).length > 0) { + store.lastGood = replaceProviderAuthState(store.lastGood, providerKey); changed = true; - if (Object.keys(store.lastGood).length === 0) { - store.lastGood = undefined; - } } if (store.usageStats && Object.keys(store.usageStats).length === 0) { store.usageStats = undefined; @@ -316,14 +333,11 @@ export async function clearLastGoodProfileWithLock(params: { return await updateAuthProfileStoreWithLock({ agentDir: params.agentDir, updater: (store) => { - const lastGoodKey = findProviderAuthStateKey(store.lastGood, providerKey); - if (!lastGoodKey || store.lastGood?.[lastGoodKey] !== params.profileId) { + const matches = listProviderAuthStateEntries(store.lastGood, providerKey); + if (!matches.some(([, profileId]) => profileId === params.profileId)) { return false; } - delete store.lastGood[lastGoodKey]; - if (Object.keys(store.lastGood).length === 0) { - store.lastGood = undefined; - } + store.lastGood = replaceProviderAuthState(store.lastGood, providerKey); return true; }, }); @@ -346,7 +360,7 @@ export async function markAuthProfileSuccess(params: { if (!profile || resolveProviderIdForAuth(profile.provider) !== providerKey) { return false; } - freshStore.lastGood = { ...freshStore.lastGood, [providerKey]: profileId }; + freshStore.lastGood = replaceProviderAuthState(freshStore.lastGood, providerKey, profileId); updateSuccessfulUsageStatsEntry(freshStore, profileId, lastUsed); return true; }, diff --git a/src/commands/models/auth-order.test.ts b/src/commands/models/auth-order.test.ts index 54dde55130f2..d599a584f6db 100644 --- a/src/commands/models/auth-order.test.ts +++ b/src/commands/models/auth-order.test.ts @@ -91,6 +91,30 @@ describe("models auth order", () => { expect(runtime.logs).toContain("Auth profile order override: anthropic:b, anthropic:a"); }); + it("accepts alias-provider profiles and reports the canonical stored order", async () => { + mocks.ensureAuthProfileStore.mockReturnValue({ + version: 1, + profiles: { + "xai:a": { type: "oauth", provider: "xai", access: "tok" }, + }, + }); + mocks.setAuthProfileOrder.mockResolvedValue({ + version: 1, + profiles: {}, + order: { xai: ["xai:a"] }, + }); + const runtime = createRuntime(); + + await modelsAuthOrderSetCommand({ provider: "x-ai", order: ["xai:a"] }, runtime); + + expect(mocks.setAuthProfileOrder).toHaveBeenCalledWith({ + agentDir: "/tmp/agent-main", + provider: "xai", + order: ["xai:a"], + }); + expect(runtime.logs).toContain("Auth profile order override: xai:a"); + }); + it("clear removes the store order and refreshes a running gateway", async () => { const runtime = createRuntime(); await modelsAuthOrderClearCommand({ provider: "anthropic" }, runtime); diff --git a/src/commands/models/auth-order.ts b/src/commands/models/auth-order.ts index 71def0b545dd..ae2c7350debb 100644 --- a/src/commands/models/auth-order.ts +++ b/src/commands/models/auth-order.ts @@ -17,10 +17,17 @@ import { refreshRunningGatewayAuthState } from "./auth-refresh.js"; import { loadModelsConfig } from "./load-config.js"; import { resolveModelsTargetAgent } from "./shared.js"; -function describeOrder(store: AuthProfileStore, provider: string): string[] { - const providerKey = normalizeProviderId(provider); - const order = store.order?.[providerKey]; - return Array.isArray(order) ? order : []; +function describeOrder(store: AuthProfileStore, provider: string, cfg: OpenClawConfig): string[] { + const authProvider = resolveProviderIdForAuth(provider, { config: cfg }); + const canonical = findNormalizedProviderValue(store.order, authProvider); + if (canonical !== undefined) { + return canonical; + } + return ( + Object.entries(store.order ?? {}) + .filter(([key]) => resolveProviderIdForAuth(key, { config: cfg }) === authProvider) + .toSorted(([left], [right]) => left.localeCompare(right))[0]?.[1] ?? [] + ); } function describeOrderFallback(cfg: OpenClawConfig, provider: string): string { @@ -61,7 +68,7 @@ export async function modelsAuthOrderGetCommand( const store = ensureAuthProfileStore(agentDir, { externalCli: externalCliDiscoveryForProviderAuth({ cfg, provider }), }); - const order = describeOrder(store, provider); + const order = describeOrder(store, provider, cfg); if (opts.json) { writeRuntimeJson(runtime, { @@ -92,7 +99,7 @@ export async function modelsAuthOrderClearCommand( const { cfg, agentId, agentDir, provider } = await resolveAuthOrderContext(opts, runtime); const updated = await setAuthProfileOrder({ agentDir, - provider, + provider: resolveProviderIdForAuth(provider, { config: cfg }), order: null, }); if (!updated) { @@ -117,7 +124,7 @@ export async function modelsAuthOrderSetCommand( const store = ensureAuthProfileStore(agentDir, { externalCli: externalCliDiscoveryForProviderAuth({ cfg, provider }), }); - const providerKey = provider; + const providerKey = resolveProviderIdForAuth(provider, { config: cfg }); const requested = normalizeStringEntries(opts.order ?? []); if (requested.length === 0) { throw new Error( @@ -132,14 +139,14 @@ export async function modelsAuthOrderSetCommand( `Auth profile "${profileId}" not found in ${shortenHomePath(agentDir)}. Run ${formatCliCommand("openclaw models auth list --provider " + provider)} to see saved profiles.`, ); } - if (normalizeProviderId(cred.provider) !== providerKey) { + if (resolveProviderIdForAuth(cred.provider, { config: cfg }) !== providerKey) { throw new Error(`Auth profile "${profileId}" is for ${cred.provider}, not ${provider}.`); } } const updated = await setAuthProfileOrder({ agentDir, - provider, + provider: providerKey, order: requested, }); if (!updated) { @@ -150,6 +157,6 @@ export async function modelsAuthOrderSetCommand( runtime.log(`Agent: ${agentId}`); runtime.log(`Provider: ${provider}`); - runtime.log(`Auth profile order override: ${describeOrder(updated, provider).join(", ")}`); + runtime.log(`Auth profile order override: ${describeOrder(updated, provider, cfg).join(", ")}`); await refreshRunningGatewayAuthState(); }