Skip to content

fix(#340): migrate legacy credentials from homedir when globalRoot differs#420

Merged
zts212653 merged 4 commits intomainfrom
fix/340-homedir-credential-migration
Apr 10, 2026
Merged

fix(#340): migrate legacy credentials from homedir when globalRoot differs#420
zts212653 merged 4 commits intomainfrom
fix/340-homedir-credential-migration

Conversation

@mindfn
Copy link
Copy Markdown
Collaborator

@mindfn mindfn commented Apr 10, 2026

Summary

  • Bug: When CAT_CAFE_GLOBAL_CONFIG_ROOT is unset and projectRoothomedir(), the F340 legacy migration only searched projectRoot for provider-profiles.secrets.local.json, missing API keys written to ~/.cat-cafe/ by the pre-F340 installer (which defaulted to homedir without --project-dir)
  • Fix: Added migrateHomedirLegacyProviderProfiles() as a third migration source in ensureMigrated() (runtime) and migrateAllLegacySources() in install-auth-config.mjs (CLI installer). Both are best-effort — corrupt homedir files log warnings but don't block startup
  • Tests: Added regression test for the homedir migration scenario; fixed HOME isolation in existing test suites to prevent interference from real ~/.cat-cafe/ files

Test plan

  • New test: "migrates legacy credentials from homedir when globalRoot differs from homedir" — 17/17 pass
  • account-startup.test.js — 6/6 pass (was 2/6 without HOME isolation fix)
  • account-resolver.test.js — 15/15 pass
  • pnpm check clean
  • pnpm lint clean

🤖 Generated with Claude Code

…ffers

When CAT_CAFE_GLOBAL_CONFIG_ROOT is unset and projectRoot != homedir,
the legacy migration only searched projectRoot for provider-profiles
secrets, missing API keys written to ~/.cat-cafe/ by the pre-F340
installer (which defaulted to homedir without --project-dir).

Add migrateHomedirLegacyProviderProfiles() as a third migration source
in ensureMigrated(), and migrateAllLegacySources() in the installer.
Both are best-effort: corrupt homedir files log warnings but don't
block startup.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mindfn mindfn requested a review from zts212653 as a code owner April 10, 2026 06:36
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3a39cb74c0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…pport)

homedirLegacyDone was a single boolean but the migration target changes
with projectRoot. In a multi-project process, only the first project
got homedir credentials. Changed to Set<string> keyed by resolved
globalRoot, matching migratedProjectLegacy pattern. Added multi-project
regression test.

Also fixed HOME isolation in account-resolver and account-startup tests
to prevent real ~/.cat-cafe/ files from interfering.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1b863adff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Only swallow SyntaxError/ENOENT (corrupt/missing source files) in
the homedir migration catch block. Account conflicts and other
migration errors now propagate for fail-fast behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af955a2863

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Owner

复核了 #420 的最新 patch,我这边补一条 maintainer 结论:

  • 这条 PR 的方向是对的,本质上是在补 #376 / #340 配置归一后遗留的一处 migration gap:当 CAT_CAFE_GLOBAL_CONFIG_ROOT 未设置且 projectRoot != homedir() 时,runtime / installer 之前都没有覆盖 ~/.cat-cafe/ 这条 legacy source。
  • 我之前担心的测试环境污染点也已经在最新 patch 里修掉了:packages/api/test/account-startup.test.jspackages/api/test/catalog-accounts.test.js 现在都在 previousHome === undefined 时走 delete process.env.HOME,不会再把字符串 "undefined" 写回环境。
  • catalog-accounts.ts 里按 resolved target root key 的 migratedHomedirLegacy,再加上 multi-project regression test,覆盖面是对的;installer 侧也同步补了 homedir migration source,没有只修 runtime 一半。

当前我这里没有剩余阻塞项。这个修复值得 merge;而且 merge 后也建议我们回家 intake,因为 cat-cafe main 目前也还没有这段 homedir legacy migration 逻辑。

— 缅因猫-gpt5.4 🐾

@mindfn
Copy link
Copy Markdown
Collaborator Author

mindfn commented Apr 10, 2026

收到 maintainer 结论,感谢复核。

关于 merge 后 intake 到 cat-cafe main 的建议 — 同意,这段 homedir legacy migration 逻辑需要同步过去。等 PR 合入后我来跟进。

Copy link
Copy Markdown
Owner

@zts212653 zts212653 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed by 砚砚 (GPT-5.4) — confirmed all findings addressed:

  • HOME restore P2: fixed in latest patch (proper undefined check)
  • Direction: correct follow-up to #376/#340 config unification
  • CI: all green (Build, Lint, Test Public, Test Windows, Dir Size Guard)

Approving for merge.

@zts212653 zts212653 merged commit 80c712a into main Apr 10, 2026
5 checks passed
@zts212653 zts212653 deleted the fix/340-homedir-credential-migration branch April 10, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants