fix(auth): load legacy Codex OAuth sidecars in embedded secrets-runtime loaders by RomneyDa · Pull Request #85074 · openclaw/openclaw

RomneyDa · 2026-05-21T19:21:02Z

Credit

This fix originates from @Totalsolutionsync's work in #84752 (P1, with live self-hosted-gateway proof attached there). I cherry-picked it into a single-purpose PR because #84752 bundles three orthogonal fixes (diagnostic lane, auth, Telegram polling) and the author explicitly offered to split. This PR takes them up on the auth half so the high-priority Codex-OAuth-on-upgrade fix can land on its own.

Commits cherry-picked: 85f36e8d2b4e + 4624e34c0671. Adjustments on top: stripped local-fork bookkeeping comments per repo policy, fixed a unicorn(no-useless-fallback-in-spread) lint error that surfaced on check-lint, added a focused regression test for the three flipped loaders, and added a doc note about Keychain-only legacy sidecars still requiring doctor.

What the bug is

#83312 added a runtime auto-migration for legacy oauthRef-backed openai-codex profiles: when the OAuth refresh manager touches one, it persists the rotated tokens inline. CLI inference hits that path, so CLI users self-heal. The embedded runner (Telegram, cron, sub-agent dispatch) goes through three different loaders that hardcoded resolveLegacyOAuthSidecars: false, so the credential loads token-less, gets filtered as ineligible, and the OAuth manager — and therefore the migration — is never reached. Result: No API key found for provider "openai-codex" on every embedded turn until the user runs openclaw doctor.

Users who upgrade via openclaw update are fine (it runs doctor). Users who upgrade via npm i -g openclaw@latest hit this — I did.

The fix

Flip the three embedded loaders' default to true, matching what loadStoredOAuthRefreshStore already does inside the refresh lock. The existing #83312 migration then fires on the first embedded turn and persists tokens inline.

Scope note: Keychain-only sidecars still need doctor

The embedded runtime path runs with allowKeychainPrompt: false, and readLegacyMacOAuthSecretKeychainKey early-bails when that flag is false. So this PR fully self-heals users whose legacy sidecar seed lives in OPENCLAW_AUTH_PROFILE_SECRET_KEY or the seed file under ~/.openclaw/oauth/..., but does not self-heal users whose seed lives only in macOS Keychain. Those users still need a one-time openclaw doctor --fix to migrate inline. A doc note has been added in docs/gateway/doctor.md covering this, and the Keychain follow-up is tracked in #85083.

Regression test

src/agents/auth-profiles/store.sidecar-runtime-defaults.test.ts builds a fixture with a legacy oauthRef-backed Codex profile and a sidecar file encrypted with a file/env-based seed, then asserts each of the three flipped loaders returns the credential with inline access / refresh / idToken hydrated by default — and that explicit resolveLegacyOAuthSidecars: false still opts out.

Verification

node scripts/run-vitest.mjs src/agents/auth-profiles/store.sidecar-runtime-defaults.test.ts src/agents/auth-profiles/oauth-manager.test.ts src/agents/auth-profiles/persisted-boundary.test.ts — 50 passed across 6 test-file shards.
node scripts/run-oxlint.mjs src/agents/auth-profiles/store.ts — clean after the spread fix.
Live verification credit goes to @Totalsolutionsync on fix: self-heal lane wedges + restore openai-codex OAuth on embedded path #84752 (real self-hosted gateway, Telegram + cron-isolated runs, zero recurrences after activation).

Companion

Pairs with #85028 (fail-fast when an OAuth credential has no refresh token) as defense-in-depth.

Co-authored-by: @Totalsolutionsync.

clawsweeper · 2026-05-21T19:22:56Z

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
The branch flips three embedded auth-profile loader defaults to resolve legacy Codex OAuth sidecar material, adds focused regression coverage, and documents the Keychain-only doctor path.

Reproducibility: yes. from source and linked live proof, though I did not run a local live gateway repro in this read-only review. Current main leaves embedded loaders opted out of legacy sidecar hydration, and the source PR's redacted gateway proof shows the failing embedded Codex paths recovering after this auth change.

PR rating
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Summary: Small scoped patch with strong source evidence, targeted regression coverage, linked live gateway proof, and no blocking findings.

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Real behavior proof
Sufficient (logs): The PR body points to redacted live self-hosted gateway logs from the source split showing embedded openai-codex Telegram and cron paths working after the same auth change, with exact-head targeted regression tests reported separately.

Risk before merge

Merging this PR intentionally changes embedded auth-provider loading so existing legacy Codex sidecar material is rehydrated in read-only secrets-runtime paths; maintainers should explicitly accept that upgrade behavior.
Keychain-only legacy sidecars remain out of scope and doctor-required in this PR; the separate follow-up at Legacy Codex OAuth sidecars stored only in macOS Keychain still require doctor for embedded runtime path #85083 tracks that smaller bucket.
The strongest live behavior proof is inherited from the source split at fix: self-heal lane wedges + restore openai-codex OAuth on embedded path #84752, while this exact PR head relies on the reported targeted regression tests plus source equivalence.

Maintainer options:

Land the scoped compatibility fix (recommended)
Accept the intentional read-only legacy sidecar rehydration in embedded loaders because it reuses the existing resolver and preserves allowKeychainPrompt: false.
Request exact-head live gateway proof
Hold merge until someone reruns a Telegram or cron embedded Codex OAuth turn on this exact head if maintainers want live proof separate from the source split.
Pause for the Keychain-only design
Defer this PR only if maintainers want the file/env seed bucket and Keychain-only bucket solved together instead of landing the narrower upgrade fix first.

Next step before merge
Protected maintainer handling and the auth-provider upgrade behavior need explicit maintainer merge judgment; there is no concrete repair finding for ClawSweeper to queue.

Security
Cleared: The diff reuses existing read-only legacy OAuth sidecar loading, keeps Keychain prompts disabled in embedded paths, and adds no dependencies, secret logging, lifecycle hooks, or new write paths.

Review details

Best possible solution:

Land the scoped loader-default fix if maintainers accept the auth-provider upgrade behavior, while keeping the Keychain-only self-heal design in the separate follow-up.

Do we have a high-confidence way to reproduce the issue?

Yes from source and linked live proof, though I did not run a local live gateway repro in this read-only review. Current main leaves embedded loaders opted out of legacy sidecar hydration, and the source PR's redacted gateway proof shows the failing embedded Codex paths recovering after this auth change.

Is this the best way to solve the issue?

Yes. The PR is the narrow owner-boundary fix: reuse the existing legacy sidecar resolver and refresh migration in the embedded loader defaults instead of adding a second migration or enabling Keychain prompts in headless paths.

Label changes:

add rating: 🦞 diamond lobster: Current PR rating is 🦞 diamond lobster because proof is 🦞 diamond lobster, patch quality is 🦞 diamond lobster, and Small scoped patch with strong source evidence, targeted regression coverage, linked live gateway proof, and no blocking findings.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

P1: The PR targets an upgraded embedded-agent auth regression that can break Telegram, cron, and isolated sub-agent Codex OAuth runs for real users.
merge-risk: 🚨 auth-provider: The diff changes OAuth credential loading defaults for embedded runtime paths, which affects provider credential resolution during upgrade.
rating: 🦞 diamond lobster: Current PR rating is 🦞 diamond lobster because proof is 🦞 diamond lobster, patch quality is 🦞 diamond lobster, and Small scoped patch with strong source evidence, targeted regression coverage, linked live gateway proof, and no blocking findings.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The PR body points to redacted live self-hosted gateway logs from the source split showing embedded openai-codex Telegram and cron paths working after the same auth change, with exact-head targeted regression tests reported separately.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body points to redacted live self-hosted gateway logs from the source split showing embedded openai-codex Telegram and cron paths working after the same auth change, with exact-head targeted regression tests reported separately.

Acceptance criteria:

node scripts/run-vitest.mjs src/agents/auth-profiles/store.sidecar-runtime-defaults.test.ts src/agents/auth-profiles/store.test.ts src/agents/auth-profiles/oauth-manager.test.ts
node scripts/run-oxlint.mjs src/agents/auth-profiles/store.ts

What I checked:

Current main opt-out path: Current main still has the embedded secrets-runtime loaders explicitly opt out of legacy OAuth sidecar resolution, which leaves oauthRef-backed Codex credentials without token material in those paths. (src/agents/auth-profiles/store.ts:589, 016c34ff1d2a)
Eligibility failure source path: OAuth credentials with neither access nor refresh are treated as missing credentials, and resolveAuthProfileOrder delegates to that eligibility check before provider auth resolution can proceed. (src/agents/auth-profiles/credential-state.ts:105, 016c34ff1d2a)
Embedded callers use these loaders: Embedded discovery and runner paths call loadAuthProfileStoreForSecretsRuntime, loadAuthProfileStoreWithoutExternalProfiles, or ensureAuthProfileStoreWithoutExternalProfiles with allowKeychainPrompt false, so the loader defaults directly affect Telegram, cron, and isolated sub-agent auth resolution. (src/agents/pi-auth-discovery.ts:34, 016c34ff1d2a)
PR loader change: The PR head changes loadAuthProfileStoreForSecretsRuntime, loadAuthProfileStoreWithoutExternalProfiles, and ensureAuthProfileStoreWithoutExternalProfiles so legacy sidecar resolution defaults to true while preserving an explicit opt-out. (src/agents/auth-profiles/store.ts:589, 17eb0b364428)
Regression coverage: The new test builds an env/file-seed legacy oauthRef fixture and asserts all three changed loaders hydrate access, refresh, and idToken by default while resolveLegacyOAuthSidecars: false still opts out. (src/agents/auth-profiles/store.sidecar-runtime-defaults.test.ts:107, 17eb0b364428)
Keychain scope boundary: The legacy sidecar reader only consults macOS Keychain when allowKeychainPrompt is not false, so the PR's allowKeychainPrompt: false embedded paths correctly leave Keychain-only migration to doctor and the linked follow-up. (src/agents/auth-profiles/legacy-oauth-sidecar.ts:229, 016c34ff1d2a)

Likely related people:

joshavant: Auth sidecar compatibility and the resolveLegacyOAuthSidecars contract were introduced by merged Keep legacy Codex OAuth sidecar profiles usable #83312, which is the migration path this PR extends to embedded loaders. (role: introduced legacy sidecar compatibility; confidence: high; commits: 06f4c97130c6; files: src/agents/auth-profiles/store.ts, src/agents/auth-profiles/persisted.ts, src/agents/auth-profiles/legacy-oauth-sidecar.ts)
RomneyDa: Auth-provider fail-fast behavior for token-less OAuth credentials was recently merged in the same area, and this PR is the scoped split of the embedded-loader sidecar fix. (role: recent adjacent auth contributor; confidence: high; commits: 205c595b134c, 17eb0b364428; files: src/agents/auth-profiles/oauth-manager.ts, src/agents/auth-profiles/store.ts, src/agents/auth-profiles/store.sidecar-runtime-defaults.test.ts)
steipete: Local blame in this checkout points the relevant auth-profile files at a current-main grafted commit, so this is a weak routing signal compared with the merged PR provenance. (role: current-main snapshot contributor; confidence: low; commits: f43e83c937f4; files: src/agents/auth-profiles/store.ts, src/agents/auth-profiles/persisted.ts, src/agents/auth-profiles/legacy-oauth-sidecar.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 016c34ff1d2a.

clawsweeper · 2026-05-21T19:47:04Z

ClawSweeper PR egg

✨ Hatched: 🥚 common Mossy Crabkin

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: sleeps inside passing CI.
Image traits: location status garden; accessory shell-shaped keyboard; palette coral, mint, and warm cream; mood sleepy but ready; pose sitting proudly on a smooth stone; shell smooth pearl shell; lighting soft studio lighting; background delicate sparkle particles.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Mossy Crabkin in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

RomneyDa · 2026-05-21T19:51:59Z

Filed #85083 as the follow-up for the Keychain-only sidecar case explicitly called out as out-of-scope here. Lands the same self-heal experience for the smaller bucket of users whose legacy seed is in macOS Keychain, with a safe-prompting design (interactive: migrate; headless: fail clean) instead of just flipping allowKeychainPrompt.

…me loaders The auto-migration introduced in #83312 only fires when a credential is loaded via a path that reads its sidecar tokens. The OAuth refresh manager's internal loader does (so direct CLI inference works and self-heals on first refresh). The embedded runner's secrets-runtime loaders did not: - loadAuthProfileStoreForSecretsRuntime - loadAuthProfileStoreWithoutExternalProfiles - ensureAuthProfileStoreWithoutExternalProfiles All three opted out of sidecar resolution. So for an upgraded user with a legacy oauthRef-backed openai-codex profile, the credential loaded with no access/refresh material, evaluateStoredCredentialEligibility marked it ineligible, resolveAuthProfileOrder filtered it out, and resolveApiKeyForProvider threw "No API key found for provider 'openai-codex'" before the OAuth manager (and its migration path) was ever consulted. CLI worked, Telegram/cron/embedded turns broke — only doctor-or-bust would fix it. Flip the three embedded loaders to default resolveLegacyOAuthSidecars to true (matching loadStoredOAuthRefreshStore). The existing #83312 refresh-and-rewrite then fires on the first embedded turn for these users and persists tokens inline, removing the legacy sidecar from disk on the next doctor pass. Cherry-picked and squashed from PR #84752 (commits 85f36e8 and 4624e34). Comments noting local-fork bookkeeping stripped per repo policy. Co-authored-by: Will <totalsolutionspm@gmail.com>

…first interactive CLI run Closes the remaining bucket from #85083: macOS users whose legacy `oauthRef` sidecar seed lives only in the Keychain still had to know to run `openclaw doctor --fix` after upgrade, because the embedded-runtime self-heal added in #85074 intentionally runs with `allowKeychainPrompt: false`. This change adds a tiny pre-command trigger in `runCli` that, on darwin + TTY only, reuses the existing `maybeRepairLegacyOAuthSidecarProfiles` doctor flow when a sidecar dir is present. Fast bail-outs make the cost a single `readdirSync` for users without legacy sidecars. Declining writes a 24h cool-off marker in the state dir so we never pester the user every command. Headless paths (cron, systemd, Telegram polling, embedded sub-agent dispatch) still skip the prompt, but the legacy sidecar loader now emits a one-shot warn that names `openclaw doctor --fix` and macOS Keychain, instead of letting the credential silently fall through to a downstream "No API key found". Also drops the user-confusing `sidecar-backed` phrasing from the doctor prompt/notes/changes; existing internal identifier names are left intact.

…me loaders (openclaw#85074) The auto-migration introduced in openclaw#83312 only fires when a credential is loaded via a path that reads its sidecar tokens. The OAuth refresh manager's internal loader does (so direct CLI inference works and self-heals on first refresh). The embedded runner's secrets-runtime loaders did not: - loadAuthProfileStoreForSecretsRuntime - loadAuthProfileStoreWithoutExternalProfiles - ensureAuthProfileStoreWithoutExternalProfiles All three opted out of sidecar resolution. So for an upgraded user with a legacy oauthRef-backed openai-codex profile, the credential loaded with no access/refresh material, evaluateStoredCredentialEligibility marked it ineligible, resolveAuthProfileOrder filtered it out, and resolveApiKeyForProvider threw "No API key found for provider 'openai-codex'" before the OAuth manager (and its migration path) was ever consulted. CLI worked, Telegram/cron/embedded turns broke — only doctor-or-bust would fix it. Flip the three embedded loaders to default resolveLegacyOAuthSidecars to true (matching loadStoredOAuthRefreshStore). The existing openclaw#83312 refresh-and-rewrite then fires on the first embedded turn for these users and persists tokens inline, removing the legacy sidecar from disk on the next doctor pass. Cherry-picked and squashed from PR openclaw#84752 (commits 85f36e8 and 4624e34). Comments noting local-fork bookkeeping stripped per repo policy. Co-authored-by: Will <totalsolutionspm@gmail.com>

RomneyDa requested a review from a team as a code owner May 21, 2026 19:21

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS maintainer Maintainer-authored PR labels May 21, 2026

RomneyDa force-pushed the fix-embedded-codex-oauth-sidecar-loaders branch from 474a56f to fc23cd8 Compare May 21, 2026 19:31

joshavant self-assigned this May 21, 2026

RomneyDa force-pushed the fix-embedded-codex-oauth-sidecar-loaders branch from fc23cd8 to ac608c7 Compare May 21, 2026 19:49

openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime size: S and removed size: XS labels May 21, 2026

RomneyDa mentioned this pull request May 21, 2026

Legacy Codex OAuth sidecars stored only in macOS Keychain still require doctor for embedded runtime path #85083

Closed

RomneyDa force-pushed the fix-embedded-codex-oauth-sidecar-loaders branch from ac608c7 to 17eb0b3 Compare May 21, 2026 19:54

clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels May 21, 2026

RomneyDa merged commit 4399eee into main May 21, 2026
114 of 116 checks passed

RomneyDa deleted the fix-embedded-codex-oauth-sidecar-loaders branch May 21, 2026 20:07

github-actions Bot mentioned this pull request May 21, 2026

📡 Upstream Digest — 2026-05-21 21:22 UTC curtismercier/openclaw-mods#911

Open

RomneyDa mentioned this pull request May 22, 2026

fix(auth): auto-migrate keychain-only legacy Codex OAuth profiles on first interactive CLI run #85163

Closed

RomneyDa mentioned this pull request May 24, 2026

fix(auth): point Keychain-only legacy Codex OAuth users at doctor instead of auto-prompting #86220

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(auth): load legacy Codex OAuth sidecars in embedded secrets-runtime loaders#85074

fix(auth): load legacy Codex OAuth sidecars in embedded secrets-runtime loaders#85074
RomneyDa merged 1 commit into
mainfrom
fix-embedded-codex-oauth-sidecar-loaders

RomneyDa commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 21, 2026

Uh oh!

RomneyDa commented May 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

RomneyDa commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Credit

What the bug is

The fix

Scope note: Keychain-only sidecars still need doctor

Regression test

Verification

Companion

Uh oh!

clawsweeper Bot commented May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented May 21, 2026

Hatch command

Uh oh!

RomneyDa commented May 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RomneyDa commented May 21, 2026 •

edited

Loading

clawsweeper Bot commented May 21, 2026 •

edited

Loading