From 1e7c7dd02fafd6e6dab98619edbce0759cc68aaf Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 17 Apr 2026 14:11:41 -0700 Subject: [PATCH] refactor(auth): polish external oauth bootstrap flow --- CHANGELOG.md | 1 + src/agents/auth-health.test.ts | 33 +++++++++++++++++++ .../auth-profiles.external-cli-sync.test.ts | 8 ++--- src/agents/auth-profiles/effective-oauth.ts | 27 +++++++++++---- src/agents/auth-profiles/external-cli-sync.ts | 17 +++++++++- .../auth-profiles/oauth-refresh-queue.test.ts | 11 ++++++- .../oauth.adopt-identity.test.ts | 11 ++++++- .../oauth.concurrent-agents.test.ts | 11 ++++++- .../oauth.mirror-refresh.test.ts | 11 ++++++- src/agents/auth-profiles/oauth.ts | 32 +++++++++--------- src/agents/pi-auth-json.test.ts | 2 +- 11 files changed, 132 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58fae42de28..35a2141c793 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Docs: https://docs.openclaw.ai - OpenAI Codex/OAuth: keep external CLI OAuth imports runtime-only by overlaying fresher Codex CLI credentials without mutating `auth-profiles.json`, so `.codex` stays a bootstrap/runtime input instead of becoming durable OpenClaw state. Thanks @vincentkoc. - OpenAI Codex/OAuth: drop legacy CLI-manager routing from the remaining bootstrap path so Codex and MiniMax CLI imports are matched by their canonical OpenClaw profile ids instead of stale `managedBy` metadata. Thanks @vincentkoc. - OpenAI Codex/OAuth: only bootstrap from external CLI OAuth when the local OpenClaw profile is missing or unusable, so healthy local sessions are no longer overridden by fresher `.codex` tokens. Thanks @vincentkoc. +- OpenAI Codex/OAuth: rename the external CLI bootstrap helper, reuse the same usable-oauth check across runtime fallback paths, and add debug logs plus health coverage so bootstrap decisions stay legible. Thanks @vincentkoc. - Twitch/setup: load Twitch through the bundled setup-entry discovery path and keep setup/status account detection aligned with runtime config. (#68008) Thanks @gumadeiras. - Feishu/card actions: resolve card-action chat type from the Feishu chat API when stored context is missing, preferring `chat_mode` over `chat_type`, so DM-originated card actions no longer bypass `dmPolicy` by falling through to the group handling path. (#68201) - Cron/isolated-agent: preserve `trusted: false` on isolated cron awareness events mirrored into the main session, and forward the optional `trusted` flag through the gateway cron wrapper so explicit trust downgrades survive session-key scoping. (#68210) diff --git a/src/agents/auth-health.test.ts b/src/agents/auth-health.test.ts index 232b3e23069..bd7420ed540 100644 --- a/src/agents/auth-health.test.ts +++ b/src/agents/auth-health.test.ts @@ -140,6 +140,39 @@ describe("buildAuthHealthSummary", () => { expect(statuses["openai-codex:default"]).toBe("ok"); }); + it("keeps healthy local oauth over fresher imported Codex CLI credentials in health status", () => { + vi.spyOn(Date, "now").mockReturnValue(now); + readCodexCliCredentialsCachedMock.mockReturnValue({ + type: "oauth", + provider: "openai-codex", + access: "fresh-cli-access", + refresh: "fresh-cli-refresh", + expires: now + 7 * DEFAULT_OAUTH_WARN_MS, + accountId: "acct-cli", + }); + const store = { + version: 1, + profiles: { + "openai-codex:default": { + type: "oauth" as const, + provider: "openai-codex", + access: "healthy-local-access", + refresh: "healthy-local-refresh", + expires: now + DEFAULT_OAUTH_WARN_MS + 10_000, + }, + }, + }; + + const summary = buildAuthHealthSummary({ + store, + warnAfterMs: DEFAULT_OAUTH_WARN_MS, + }); + + const profile = summary.profiles.find((entry) => entry.profileId === "openai-codex:default"); + expect(profile?.status).toBe("ok"); + expect(profile?.expiresAt).toBe(now + DEFAULT_OAUTH_WARN_MS + 10_000); + }); + it("marks token profiles with invalid expires as missing with reason code", () => { vi.spyOn(Date, "now").mockReturnValue(now); const store = { diff --git a/src/agents/auth-profiles.external-cli-sync.test.ts b/src/agents/auth-profiles.external-cli-sync.test.ts index 2c58ebf561a..4e738eef80d 100644 --- a/src/agents/auth-profiles.external-cli-sync.test.ts +++ b/src/agents/auth-profiles.external-cli-sync.test.ts @@ -6,7 +6,7 @@ const mocks = vi.hoisted(() => ({ readMiniMaxCliCredentialsCached: vi.fn<() => OAuthCredential | null>(() => null), })); -let readManagedExternalCliCredential: typeof import("./auth-profiles/external-cli-sync.js").readManagedExternalCliCredential; +let readExternalCliBootstrapCredential: typeof import("./auth-profiles/external-cli-sync.js").readExternalCliBootstrapCredential; let resolveExternalCliAuthProfiles: typeof import("./auth-profiles/external-cli-sync.js").resolveExternalCliAuthProfiles; let hasUsableOAuthCredential: typeof import("./auth-profiles/external-cli-sync.js").hasUsableOAuthCredential; let shouldBootstrapFromExternalCliCredential: typeof import("./auth-profiles/external-cli-sync.js").shouldBootstrapFromExternalCliCredential; @@ -48,7 +48,7 @@ describe("external cli oauth resolution", () => { mocks.readMiniMaxCliCredentialsCached.mockReset().mockReturnValue(null); ({ hasUsableOAuthCredential, - readManagedExternalCliCredential, + readExternalCliBootstrapCredential, resolveExternalCliAuthProfiles, shouldBootstrapFromExternalCliCredential, shouldReplaceStoredOAuthCredential, @@ -181,7 +181,7 @@ describe("external cli oauth resolution", () => { }), ); - const credential = readManagedExternalCliCredential({ + const credential = readExternalCliBootstrapCredential({ profileId: OPENAI_CODEX_DEFAULT_PROFILE_ID, credential: makeOAuthCredential({ provider: "openai-codex" }), }); @@ -197,7 +197,7 @@ describe("external cli oauth resolution", () => { makeOAuthCredential({ provider: "openai-codex" }), ); - const credential = readManagedExternalCliCredential({ + const credential = readExternalCliBootstrapCredential({ profileId: OPENAI_CODEX_DEFAULT_PROFILE_ID, credential: makeOAuthCredential({ provider: "anthropic" }), }); diff --git a/src/agents/auth-profiles/effective-oauth.ts b/src/agents/auth-profiles/effective-oauth.ts index 02743e5204d..0624c1908c2 100644 --- a/src/agents/auth-profiles/effective-oauth.ts +++ b/src/agents/auth-profiles/effective-oauth.ts @@ -1,6 +1,7 @@ +import { log } from "./constants.js"; import { hasUsableOAuthCredential, - readManagedExternalCliCredential, + readExternalCliBootstrapCredential, shouldBootstrapFromExternalCliCredential, } from "./external-cli-sync.js"; import type { OAuthCredential } from "./types.js"; @@ -9,7 +10,7 @@ export function resolveEffectiveOAuthCredential(params: { profileId: string; credential: OAuthCredential; }): OAuthCredential { - const imported = readManagedExternalCliCredential({ + const imported = readExternalCliBootstrapCredential({ profileId: params.profileId, credential: params.credential, }); @@ -17,12 +18,26 @@ export function resolveEffectiveOAuthCredential(params: { return params.credential; } if (hasUsableOAuthCredential(params.credential)) { + log.debug("resolved oauth credential from canonical local store", { + profileId: params.profileId, + provider: params.credential.provider, + localExpires: params.credential.expires, + externalExpires: imported.expires, + }); return params.credential; } - return shouldBootstrapFromExternalCliCredential({ + const shouldBootstrap = shouldBootstrapFromExternalCliCredential({ existing: params.credential, imported, - }) - ? imported - : params.credential; + }); + if (shouldBootstrap) { + log.debug("resolved oauth credential from external cli bootstrap", { + profileId: params.profileId, + provider: imported.provider, + localExpires: params.credential.expires, + externalExpires: imported.expires, + }); + return imported; + } + return params.credential; } diff --git a/src/agents/auth-profiles/external-cli-sync.ts b/src/agents/auth-profiles/external-cli-sync.ts index 23930189be6..0ce36dcbe3a 100644 --- a/src/agents/auth-profiles/external-cli-sync.ts +++ b/src/agents/auth-profiles/external-cli-sync.ts @@ -6,6 +6,7 @@ import { EXTERNAL_CLI_SYNC_TTL_MS, MINIMAX_CLI_PROFILE_ID, OPENAI_CODEX_DEFAULT_PROFILE_ID, + log, } from "./constants.js"; import { resolveTokenExpiryState } from "./credential-state.js"; import type { AuthProfileStore, OAuthCredential } from "./types.js"; @@ -122,7 +123,7 @@ function resolveExternalCliSyncProvider(params: { return provider; } -export function readManagedExternalCliCredential(params: { +export function readExternalCliBootstrapCredential(params: { profileId: string; credential: OAuthCredential; }): OAuthCredential | null { @@ -152,8 +153,22 @@ export function resolveExternalCliAuthProfiles( now, }) ) { + if (existingOAuth) { + log.debug("kept usable local oauth over external cli bootstrap", { + profileId: providerConfig.profileId, + provider: providerConfig.provider, + localExpires: existingOAuth.expires, + externalExpires: creds.expires, + }); + } continue; } + log.debug("used external cli oauth bootstrap because local oauth was missing or unusable", { + profileId: providerConfig.profileId, + provider: providerConfig.provider, + localExpires: existingOAuth?.expires, + externalExpires: creds.expires, + }); profiles.push({ profileId: providerConfig.profileId, credential: creds, diff --git a/src/agents/auth-profiles/oauth-refresh-queue.test.ts b/src/agents/auth-profiles/oauth-refresh-queue.test.ts index bbb9991d49d..65f8f2c09dd 100644 --- a/src/agents/auth-profiles/oauth-refresh-queue.test.ts +++ b/src/agents/auth-profiles/oauth-refresh-queue.test.ts @@ -72,9 +72,18 @@ vi.mock("./external-auth.js", () => ({ })); vi.mock("./external-cli-sync.js", () => ({ - readManagedExternalCliCredential: () => null, + readExternalCliBootstrapCredential: () => null, resolveExternalCliAuthProfiles: () => [], areOAuthCredentialsEquivalent: (a: unknown, b: unknown) => a === b, + hasUsableOAuthCredential: (credential: { access?: string; expires?: number } | undefined) => + Boolean( + credential && + typeof credential.access === "string" && + credential.access.length > 0 && + typeof credential.expires === "number" && + Number.isFinite(credential.expires) && + Date.now() < credential.expires, + ), })); function createExpiredOauthStore(params: { diff --git a/src/agents/auth-profiles/oauth.adopt-identity.test.ts b/src/agents/auth-profiles/oauth.adopt-identity.test.ts index e6e4f94c10a..5f5e2e4d269 100644 --- a/src/agents/auth-profiles/oauth.adopt-identity.test.ts +++ b/src/agents/auth-profiles/oauth.adopt-identity.test.ts @@ -79,9 +79,18 @@ vi.mock("./doctor.js", () => ({ })); vi.mock("./external-cli-sync.js", () => ({ - readManagedExternalCliCredential: () => null, + readExternalCliBootstrapCredential: () => null, resolveExternalCliAuthProfiles: () => [], areOAuthCredentialsEquivalent: (a: unknown, b: unknown) => a === b, + hasUsableOAuthCredential: (credential: { access?: string; expires?: number } | undefined) => + Boolean( + credential && + typeof credential.access === "string" && + credential.access.length > 0 && + typeof credential.expires === "number" && + Number.isFinite(credential.expires) && + Date.now() < credential.expires, + ), })); function oauthCred(params: { diff --git a/src/agents/auth-profiles/oauth.concurrent-agents.test.ts b/src/agents/auth-profiles/oauth.concurrent-agents.test.ts index 22b49dca6a8..5ed1b40ad9d 100644 --- a/src/agents/auth-profiles/oauth.concurrent-agents.test.ts +++ b/src/agents/auth-profiles/oauth.concurrent-agents.test.ts @@ -65,9 +65,18 @@ vi.mock("./doctor.js", () => ({ // credential files; it is slow and can pollute test state. Stub it to a no-op // so the suite only exercises in-repo auth-profile logic. vi.mock("./external-cli-sync.js", () => ({ - readManagedExternalCliCredential: () => null, + readExternalCliBootstrapCredential: () => null, resolveExternalCliAuthProfiles: () => [], areOAuthCredentialsEquivalent: (a: unknown, b: unknown) => a === b, + hasUsableOAuthCredential: (credential: { access?: string; expires?: number } | undefined) => + Boolean( + credential && + typeof credential.access === "string" && + credential.access.length > 0 && + typeof credential.expires === "number" && + Number.isFinite(credential.expires) && + Date.now() < credential.expires, + ), })); function createExpiredOauthStore(params: { diff --git a/src/agents/auth-profiles/oauth.mirror-refresh.test.ts b/src/agents/auth-profiles/oauth.mirror-refresh.test.ts index 81a68275d29..181badeadce 100644 --- a/src/agents/auth-profiles/oauth.mirror-refresh.test.ts +++ b/src/agents/auth-profiles/oauth.mirror-refresh.test.ts @@ -76,9 +76,18 @@ vi.mock("./doctor.js", () => ({ })); vi.mock("./external-cli-sync.js", () => ({ - readManagedExternalCliCredential: () => null, + readExternalCliBootstrapCredential: () => null, resolveExternalCliAuthProfiles: () => [], areOAuthCredentialsEquivalent: (a: unknown, b: unknown) => a === b, + hasUsableOAuthCredential: (credential: { access?: string; expires?: number } | undefined) => + Boolean( + credential && + typeof credential.access === "string" && + credential.access.length > 0 && + typeof credential.expires === "number" && + Number.isFinite(credential.expires) && + Date.now() < credential.expires, + ), })); function createExpiredOauthStore(params: { diff --git a/src/agents/auth-profiles/oauth.ts b/src/agents/auth-profiles/oauth.ts index f1c8878930a..fb920ab81fc 100644 --- a/src/agents/auth-profiles/oauth.ts +++ b/src/agents/auth-profiles/oauth.ts @@ -27,7 +27,8 @@ import { formatAuthDoctorHint } from "./doctor.js"; import { resolveEffectiveOAuthCredential } from "./effective-oauth.js"; import { areOAuthCredentialsEquivalent, - readManagedExternalCliCredential, + hasUsableOAuthCredential, + readExternalCliBootstrapCredential, shouldReplaceStoredOAuthCredential, } from "./external-cli-sync.js"; import { ensureAuthStoreFile, resolveAuthStorePath, resolveOAuthRefreshLockPath } from "./paths.js"; @@ -161,10 +162,11 @@ async function loadFreshStoredOAuthCredential(params: { }): Promise { const reloadedStore = loadAuthProfileStoreForSecretsRuntime(params.agentDir); const reloaded = reloadedStore.profiles[params.profileId]; - if (reloaded?.type !== "oauth" || reloaded.provider !== params.provider) { - return null; - } - if (!Number.isFinite(reloaded.expires) || Date.now() >= reloaded.expires) { + if ( + reloaded?.type !== "oauth" || + reloaded.provider !== params.provider || + !hasUsableOAuthCredential(reloaded) + ) { return null; } if ( @@ -557,7 +559,7 @@ async function doRefreshOAuthTokenWithLock(params: { return null; } - if (Date.now() < cred.expires) { + if (hasUsableOAuthCredential(cred)) { return { apiKey: await buildOAuthApiKey(cred.provider, cred), newCredentials: cred, @@ -577,8 +579,7 @@ async function doRefreshOAuthTokenWithLock(params: { if ( mainCred?.type === "oauth" && mainCred.provider === cred.provider && - Number.isFinite(mainCred.expires) && - Date.now() < mainCred.expires && + hasUsableOAuthCredential(mainCred) && // Defense-in-depth identity gate. Tolerates the pure upgrade // case (sub predates identity capture) but refuses positive // mismatch, identity regression, and non-overlapping fields. @@ -598,8 +599,7 @@ async function doRefreshOAuthTokenWithLock(params: { } else if ( mainCred?.type === "oauth" && mainCred.provider === cred.provider && - Number.isFinite(mainCred.expires) && - Date.now() < mainCred.expires && + hasUsableOAuthCredential(mainCred) && !isSafeToCopyOAuthIdentity(cred, mainCred) ) { // Main has fresh creds but they belong to a DIFFERENT account — @@ -618,7 +618,7 @@ async function doRefreshOAuthTokenWithLock(params: { } } - const externallyManaged = readManagedExternalCliCredential({ + const externallyManaged = readExternalCliBootstrapCredential({ profileId: params.profileId, credential: cred, }); @@ -630,7 +630,7 @@ async function doRefreshOAuthTokenWithLock(params: { store.profiles[params.profileId] = externallyManaged; saveAuthProfileStore(store, params.agentDir); } - if (Date.now() < externallyManaged.expires) { + if (hasUsableOAuthCredential(externallyManaged)) { return { apiKey: await buildOAuthApiKey(externallyManaged.provider, externallyManaged), newCredentials: externallyManaged, @@ -752,7 +752,7 @@ async function tryResolveOAuthProfile( credential: cred, }); - if (Date.now() < effectiveCred.expires) { + if (hasUsableOAuthCredential(effectiveCred)) { return await buildOAuthProfileResult({ provider: effectiveCred.provider, credentials: effectiveCred, @@ -908,7 +908,7 @@ export async function resolveApiKeyForProfile( credential: oauthCred, }); - if (Date.now() < effectiveOAuthCred.expires) { + if (hasUsableOAuthCredential(effectiveOAuthCred)) { return await buildOAuthProfileResult({ provider: effectiveOAuthCred.provider, credentials: effectiveOAuthCred, @@ -933,7 +933,7 @@ export async function resolveApiKeyForProfile( } catch (error) { const refreshedStore = loadAuthProfileStoreForSecretsRuntime(params.agentDir); const refreshed = refreshedStore.profiles[profileId]; - if (refreshed?.type === "oauth" && Date.now() < refreshed.expires) { + if (refreshed?.type === "oauth" && hasUsableOAuthCredential(refreshed)) { return await buildOAuthProfileResult({ provider: refreshed.provider, credentials: refreshed, @@ -1003,7 +1003,7 @@ export async function resolveApiKeyForProfile( if ( mainCred?.type === "oauth" && mainCred.provider === cred.provider && - Date.now() < mainCred.expires && + hasUsableOAuthCredential(mainCred) && // Defense-in-depth identity gate — refuse to inherit credentials // from a different account even under refresh failure. Tolerates // pre-capture credentials but refuses regression/non-overlap. diff --git a/src/agents/pi-auth-json.test.ts b/src/agents/pi-auth-json.test.ts index 692fdc852cf..dd9642c81a6 100644 --- a/src/agents/pi-auth-json.test.ts +++ b/src/agents/pi-auth-json.test.ts @@ -10,7 +10,7 @@ vi.mock("../plugins/provider-runtime.js", () => ({ })); vi.mock("./auth-profiles/external-cli-sync.js", () => ({ - readManagedExternalCliCredential: () => null, + readExternalCliBootstrapCredential: () => null, resolveExternalCliAuthProfiles: () => [], }));