fix(doctor): merge legacy flat auth repair into existing SQLite store by yetval · Pull Request #98245 · openclaw/openclaw

yetval · 2026-06-30T18:54:30Z

What Problem This Solves

Resolves a problem where running openclaw doctor can silently and permanently destroy stored auth credentials. When an install carries an ancient pre-versioned flat auth-profiles.json (a provider to credential map with no {version, profiles} wrapper) alongside a populated SQLite auth profile store, the legacy-flat repair rewrites the entire SQLite store with the flat file's profiles only. Any credential that lives in SQLite but not in the flat file, for example an OAuth refresh token from a login performed after the SQLite migration, is wiped. The backup captures only the flat JSON, so the destroyed SQLite credential is unrecoverable and the affected provider must be re-authenticated. This runs on the default openclaw doctor path (the auto-fix prompt defaults to yes), so a routine maintenance run is enough to trigger it.

Why This Change Was Made

maybeRepairLegacyFlatAuthProfileStores in src/commands/doctor-auth-flat-profiles.ts built its store solely from the flat JSON and then called saveAuthProfileStore, which replaces the whole auth_profile_store SQLite row instead of merging. This path predates the SQLite auth migration: whole-store overwrite was harmless against a JSON store but became destructive once a populated SQLite store could exist, and backupAuthProfileStore only ever copied the flat JSON.

The fix mirrors the sibling SQLite migration (maybeMigrateAuthProfileJsonStoresToSqlite): load the existing SQLite store, merge the legacy flat profiles into it so existing credentials win, then verify the imported profiles persisted before removing the flat file. It reuses the in-file helpers loadPersistedAuthProfileStore, mergeImportedAuthProfiles, and formatMissingAuthProfileSqliteVerification rather than adding a second strategy. Behavior is unchanged for the intended case: when SQLite is empty the merge yields exactly the flat profiles. The aws-sdk marker branch in the same function is unaffected; it moves routing metadata into config and never touched the SQLite credential store.

This is distinct from open PR #97881, which edits the same file but only in the credential-coercion helpers (coerceLegacyFlatCredential and an import) and addresses field loss in the other migration path; it does not touch maybeRepairLegacyFlatAuthProfileStores. The two changes are in different functions and independent, needing at most a trivial rebase if both land.

// before
const backupPath = backupAuthProfileStore(entry.authPath, now);
saveAuthProfileStore(entry.store, entry.agentDir, { syncExternalCli: false });
fs.unlinkSync(entry.authPath);

// after
const existing = loadPersistedAuthProfileStore(entry.agentDir) ?? {
  version: AUTH_STORE_VERSION,
  profiles: {},
};
const importedProfileIds = new Set(Object.keys(entry.store.profiles));
const merged = mergeImportedAuthProfiles({
  store: { ...existing, version: Math.max(existing.version, entry.store.version) },
  profiles: entry.store.profiles,
  existingProfileIds: new Set(Object.keys(existing.profiles)),
});
const backupPath = backupAuthProfileStore(entry.authPath, now);
saveAuthProfileStore(merged, entry.agentDir, { syncExternalCli: false });
const verificationFailure = formatMissingAuthProfileSqliteVerification({
  expected: merged,
  importedProfileIds,
  loaded: loadPersistedAuthProfileStore(entry.agentDir),
});
if (verificationFailure) {
  result.warnings.push(/* leave the flat JSON in place */);
  continue;
}
fs.unlinkSync(entry.authPath);

User Impact

Running openclaw doctor on an install that still has a legacy flat auth-profiles.json no longer deletes SQLite-only credentials. Existing OAuth and API credentials in the SQLite store are preserved, the legacy flat profiles are merged in alongside them, and the flat file is removed only after the imported profiles are confirmed present. If verification fails the flat file is left in place with a warning instead of losing data.

Evidence

Drove the real maybeRepairLegacyFlatAuthProfileStores against a real on-disk per-agent SQLite auth store (populated through the real saveAuthProfileStore) plus a real legacy flat auth-profiles.json, on pristine main 66e676d29b and on the patched tree with identical inputs. The auth store, the repair entry point, and the SQLite read-back were all real; nothing was mocked. Steps: seed SQLite with an anthropic:default OAuth credential, write a pre-versioned flat auth-profiles.json holding only openai, run the repair with auto-fix accepted, then read the raw SQLite store back.

# BEFORE (pristine main 66e676d29b)
BEFORE doctor, SQLite store: {"anthropic:default":{"type":"oauth","provider":"anthropic","access":"sk-access-LIVE","refresh":"sk-refresh-LIVE-unrecoverable","expires":9999999999999}}
AFTER  doctor, SQLite store: {"openai:default":{"type":"api_key","provider":"openai","key":"sk-openai-flat"}}
anthropic OAuth refresh token survived: false

# AFTER (this patch, identical inputs)
BEFORE doctor, SQLite store: {"anthropic:default":{"type":"oauth","provider":"anthropic","access":"sk-access-LIVE","refresh":"sk-refresh-LIVE-unrecoverable","expires":9999999999999}}
AFTER  doctor, SQLite store: {"anthropic:default":{"type":"oauth","provider":"anthropic","access":"sk-access-LIVE","refresh":"sk-refresh-LIVE-unrecoverable","expires":9999999999999},"openai:default":{"type":"api_key","provider":"openai","key":"sk-openai-flat"}}
anthropic OAuth refresh token survived: true

On pristine main the anthropic OAuth profile and its refresh token vanish from SQLite after the repair; with the patch both the preserved anthropic OAuth credential and the migrated openai key are present.

Additional checks:

node scripts/run-vitest.mjs src/commands/doctor-auth-flat-profiles.test.ts passes 28/28. The new case preserves existing SQLite auth profiles when migrating a legacy flat store fails on pristine main and passes with the patch.
node scripts/run-oxlint.mjs and oxfmt --check on both changed files are clean.
node scripts/run-tsgo.mjs -p tsconfig.core.json and -p test/tsconfig/tsconfig.core.test.json are clean.
No live provider auth request was issued and the full build was not run; the proof exercises the on-disk SQLite store and the real repair function.

clawsweeper · 2026-06-30T18:55:42Z

Codex review: needs maintainer review before merge. Reviewed July 5, 2026, 12:33 PM ET / 16:33 UTC.

Summary
The PR changes the legacy flat auth-profile doctor repair to merge flat JSON profiles into the existing per-agent SQLite auth store, verify persistence, and add a regression test.

PR surface: Source +21, Tests +47. Total +68 across 2 files.

Reproducibility: yes. at source level. Current main writes only the legacy flat store into SQLite during the repair, and the PR body provides copied before/after output from the real repair function using on-disk SQLite state.

Review metrics: 1 noteworthy metric.

Persistent auth repair path: 1 legacy-flat doctor repair changed. This path rewrites persisted credential material during doctor repair, so preservation and cleanup ordering matter before merge.

Stored data model
Persistent data-model change detected: migration/backfill/repair: src/commands/doctor-auth-flat-profiles.test.ts, migration/backfill/repair: src/commands/doctor-auth-flat-profiles.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

[P1] The diff changes upgrade-time doctor repair for persisted credential material, so maintainers should explicitly accept that existing SQLite credentials win over legacy flat imports.
[P1] The related open auth migration PR fix(auth-profiles): preserve secret refs and OAuth fields during doctor auth migration #97881 edits the same migration area; if it lands first, this branch needs a small rebase and focused auth-doctor test rerun.
[P1] The broader first-run/update stranded JSON auth-profile problem remains tracked by [bug] 2026.6.x auth-store migration to SQLite silently strands existing JSON auth profiles — all OAuth/token providers lose auth, fallback chains die silently #100067, so this PR should not be treated as closing that work.

Maintainer options:

Accept after auth-migration owner review (recommended)
If maintainers agree that existing SQLite credentials must win over legacy flat imports, land after required CI and any rebase against related auth-migration PRs.
Add a shipped-shape fixture first
If maintainers want lower residual upgrade risk, add one regression around an observed shipped flat auth-profiles.json plus existing SQLite store before merge.
Sequence with field-preservation work
If fix(auth-profiles): preserve secret refs and OAuth fields during doctor auth migration #97881 lands first, rebase this branch and rerun the focused doctor auth-profile tests before merge.

Next step before merge

No automated repair is needed; maintainers should review the compatibility-sensitive credential migration behavior and sequencing with overlapping auth migration PRs.

Maintainer decision needed

Question: Should the legacy-flat doctor repair preserve existing SQLite auth profiles over imported flat JSON profiles and leave the flat JSON in place when SQLite verification fails?
Rationale: This is persisted credential migration behavior: automation can verify the code path, but maintainers need to own the upgrade and auth/security boundary decision before merge.
Likely owner: steipete — The SQLite auth-store refactor and earlier flat doctor migration history make this the strongest available owner signal for the upgrade boundary decision.
Options:
- Accept existing SQLite wins (recommended): Land this PR after required checks, with focused rebase/testing if related auth migration work lands first.
- Add shipped-shape fixture first: Require one more regression fixture from an observed shipped flat auth-profiles.json shape before merge.
- Pause for broader migration direction: Hold this PR until maintainers decide how it should sequence with startup/update stranded-JSON work.

Security
Cleared: No concrete security or supply-chain regression was found; the diff only changes local credential-store migration preservation logic and tests.

Review details

Best possible solution:

Land the focused merge-preserving doctor repair after auth-migration owner review accepts the upgrade-sensitive credential behavior and any needed rebase with related auth migration work.

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

Yes at source level. Current main writes only the legacy flat store into SQLite during the repair, and the PR body provides copied before/after output from the real repair function using on-disk SQLite state.

Is this the best way to solve the issue?

Yes. Reusing the existing mergeImportedAuthProfiles and SQLite verification pattern is the narrowest maintainable fix; changing saveAuthProfileStore to merge globally would blur its replacement semantics for unrelated callers.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e62cf76bd50f.

Label changes

Label justifications:

P1: The PR fixes upgrade-time auth credential loss that can break provider authentication after running doctor.
merge-risk: 🚨 compatibility: The diff changes doctor migration behavior for legacy on-disk auth stores during upgrade.
merge-risk: 🚨 auth-provider: The migrated data controls OAuth refresh credentials and provider API credentials in the SQLite auth store.
merge-risk: 🚨 security-boundary: The changed code handles credential and token material that OpenClaw treats as sensitive persisted auth state.
rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied before/after live output from the real repair function using an on-disk SQLite auth store and legacy flat JSON.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied before/after live output from the real repair function using an on-disk SQLite auth store and legacy flat JSON.

Evidence reviewed

PR surface:

Source +21, Tests +47. Total +68 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	22	1	+21
Tests	1	47	0	+47
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	69	1	+68

What I checked:

Current main overwrite path: Current main backs up the legacy flat file, saves entry.store directly, then removes the JSON, so existing SQLite-only profiles are outside the write payload. (src/commands/doctor-auth-flat-profiles.ts:1201, e62cf76bd50f)
SQLite save replaces the persisted payload: saveAuthProfileStore builds a local payload and writes it through writePersistedAuthProfileStoreRaw when different, so callers that want preservation need to merge before saving. (src/agents/auth-profiles/store.ts:1039, e62cf76bd50f)
Sibling migration precedent: The canonical JSON-to-SQLite migration already loads the existing store, merges imports with existing profile ids protected, verifies SQLite readback, then cleans up legacy files. (src/commands/doctor-auth-flat-profiles.ts:909, e62cf76bd50f)
PR merge-and-verify shape: The PR loads the existing persisted store, calls mergeImportedAuthProfiles, saves the merged store, verifies imported profiles, and only then removes the flat JSON. (src/commands/doctor-auth-flat-profiles.ts:1198, 0302f28241f3)
Regression coverage: The PR adds a test that seeds SQLite with an Anthropic OAuth profile, writes a flat OpenAI legacy file, runs the repair, and expects both profiles to remain while the legacy file is removed. (src/commands/doctor-auth-flat-profiles.test.ts:887, 0302f28241f3)
Doctor entry point: The doctor auth-profile health flow invokes maybeRepairLegacyFlatAuthProfileStores, so the changed repair is reachable from doctor. (src/flows/doctor-health-contributions.ts:300, e62cf76bd50f)

Likely related people:

steipete: GitHub commit history shows the SQLite auth-store refactor and the earlier flat auth-profile doctor migration touching the central files. (role: introduced SQLite auth-store surface and flat doctor migration history; confidence: high; commits: e16ac0433092, b5371bfd636d; files: src/commands/doctor-auth-flat-profiles.ts, src/agents/auth-profiles/store.ts, src/agents/auth-profiles/persisted.ts)
Pick-cat: Authored the merged default-agent auth-profile import into SQLite in the same doctor migration file. (role: recent adjacent auth migration contributor; confidence: high; commits: f018945eff80, 79b2987f0287; files: src/commands/doctor-auth-flat-profiles.ts, src/commands/doctor-auth-flat-profiles.test.ts)
fuller-stack-dev: Authored the merged verification-before-cleanup fix for the same doctor auth migration file and tests. (role: recent auth migration hardening contributor; confidence: high; commits: e24c3df27d28; files: src/commands/doctor-auth-flat-profiles.ts, src/commands/doctor-auth-flat-profiles.test.ts)
vincentkoc: Recent GitHub commit history shows maintenance around doctor repair helper types and auth-profile persistence exports. (role: recent auth persistence contributor; confidence: medium; commits: ee2d4e1f7923, 7186f0d65462; files: src/commands/doctor-auth-flat-profiles.ts, src/agents/auth-profiles/persisted.ts)

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.

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.

Review history (1 earlier review cycle)

reviewed 2026-06-30T18:59:54.753Z sha 2fd656e :: needs maintainer review before merge. :: none

openclaw-barnacle · 2026-07-20T04:43:09Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

maybeRepairLegacyFlatAuthProfileStores rewrote the per-agent SQLite auth profile store with a store built solely from the legacy flat auth-profiles.json, and backed up only that flat JSON. Any credential present in SQLite but absent from the flat file (for example an OAuth refresh token from a login after the SQLite migration) was destroyed and was not in the backup, so a routine openclaw doctor caused unrecoverable credential loss. Load the existing SQLite store and merge the legacy flat profiles into it, preserving credentials already present, then verify the imported profiles persisted before removing the flat file, mirroring the SQLite migration path.

clawsweeper · 2026-07-22T17:52:07Z

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(doctor): merge legacy flat auth repair into existing SQLite store This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

jalehman · 2026-07-22T18:36:13Z

Rank-up moves addressed:

fix(auth-profiles): preserve secret refs and OAuth fields during doctor auth migration #97881 is intentionally sequenced after this focused repair and will be rebased/evaluated separately.
The force-pushed head d1392de9f48 has completed exact-head CI with no failures or pending checks; focused doctor-auth proof passes 29/29 and fresh autoreview is clean.

jalehman · 2026-07-22T18:37:33Z

Merged via squash.

Prepared head SHA: d1392de9f48613ec35d1f4781a63bd3529e759a7
Landed commit: 5419a945875ba90fcbddd84a3b50866fd64716a8

…openclaw#98245) maybeRepairLegacyFlatAuthProfileStores rewrote the per-agent SQLite auth profile store with a store built solely from the legacy flat auth-profiles.json, and backed up only that flat JSON. Any credential present in SQLite but absent from the flat file (for example an OAuth refresh token from a login after the SQLite migration) was destroyed and was not in the backup, so a routine openclaw doctor caused unrecoverable credential loss. Load the existing SQLite store and merge the legacy flat profiles into it, preserving credentials already present, then verify the imported profiles persisted before removing the flat file, mirroring the SQLite migration path.

openclaw-barnacle Bot added commands Command implementations size: S labels Jun 30, 2026

yetval mentioned this pull request Jul 1, 2026

memory-wiki: transient existing-page read failure during re-ingest silently wipes the user ## Notes block #98345

Closed

zhangqueping mentioned this pull request Jul 1, 2026

fix(memory-wiki): retry transient existing-page reads to preserve human Notes blocks #98555

Closed

clawsweeper Bot mentioned this pull request Jul 10, 2026

fix(auth-profiles): import legacy auth-profiles.json into SQLite store on load #93352

Closed

openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jul 20, 2026

jalehman self-assigned this Jul 22, 2026

jalehman force-pushed the fix/doctor-flat-auth-merge branch from 0302f28 to d1392de Compare July 22, 2026 17:49

jalehman merged commit 5419a94 into openclaw:main Jul 22, 2026
117 checks passed

github-actions Bot mentioned this pull request Jul 22, 2026

📡 Upstream Digest — 2026-07-22 18:59 UTC curtismercier/openclaw-mods#1374

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(doctor): merge legacy flat auth repair into existing SQLite store#98245

fix(doctor): merge legacy flat auth repair into existing SQLite store#98245
jalehman merged 1 commit into
openclaw:mainfrom
yetval:fix/doctor-flat-auth-merge

yetval commented Jun 30, 2026

Uh oh!

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading

Uh oh!

openclaw-barnacle Bot commented Jul 20, 2026

Uh oh!

clawsweeper Bot commented Jul 22, 2026

Uh oh!

jalehman commented Jul 22, 2026

Uh oh!

Uh oh!

jalehman commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

yetval commented Jun 30, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

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

Uh oh!

Uh oh!

openclaw-barnacle Bot commented Jul 20, 2026

Uh oh!

clawsweeper Bot commented Jul 22, 2026

Uh oh!

jalehman commented Jul 22, 2026

Uh oh!

Uh oh!

jalehman commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clawsweeper Bot commented Jun 30, 2026 •

edited

Loading