Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6a794c59f
ℹ️ 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".
|
@codex review Please review latest commit f1fb6f1 for P1/P2 only. 规则：任何 P1/P2 必须给"可执行复现"：
审查标准（详见 AGENTS.md "Review guidelines" section）：
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7faa7feb6b
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d2085cd0f
ℹ️ 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".
mindfn
force-pushed
the
fix/340-project-catalog-conflict-crash
branch
from
189b06d to
d801ba3
Compare
|
@codex review Please review latest commit d801ba3 for P1/P2 only. 规则：任何 P1/P2 必须给"可执行复现"：
审查标准（详见 AGENTS.md "Review guidelines" section）：
|
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
… conflict When global accounts.json diverges from a project's cat-catalog.json (e.g. displayName or models updated), migrateProjectAccountsToGlobal threw a fatal error crashing the app at startup. Since project catalog accounts are documented as inert stale copies and global is authoritative, conflicts should be silently skipped (global wins). - Add skipConflicts option to mergeIntoGlobal - Pass skipConflicts: true from migrateProjectAccountsToGlobal - Catch block marks project as done to avoid retry loops on persistent errors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The DELETE endpoint test previously asserted that migration conflict surfaced as a 400 error. Now that project catalog conflicts are silently skipped (global wins), update the test to assert DELETE succeeds with force:true (binding check is irrelevant to this migration test). [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pre-F340 code (PR #290) wrote API keys directly to ~/.cat-cafe/credentials.json. When globalRoot resolves to a different directory (projectRoot), those credentials were not picked up — only provider-profiles.secrets.local.json was migrated. Add migrateHomedirCredentials() that runs AFTER legacy secrets migration so homedir credentials.json wins on overlap (newer source, user-updated keys). Also handles the case where credentials.json exists at homedir without any provider-profiles files. [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Homedir credentials migration unconditionally overwrote target credentials, which could replace user-updated keys with stale homedir copies. Fixed by: 1. Skip-existing semantics: only import refs not already at target 2. Reorder ensureMigrated: homedir credentials runs FIRST, before legacy secrets — legacy's `id in existing` check naturally defers to homedir Priority chain: user-set target > homedir credentials.json > legacy secrets. [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
JSON.parse succeeds on null/string/number but `in` operator throws
TypeError on non-objects. Normalize parsed value to {} when not a
plain object.
[宪宪/Opus-46🐾]
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mindfn
force-pushed
the
fix/340-project-catalog-conflict-crash
branch
from
d801ba3 to
932eca2
Compare
zts212653
left a comment
zts212653
left a comment
There was a problem hiding this comment.
Three independent reviewers (GPT-5.4, Codex, Opus) confirmed: bug is real, fix is correct, CI green. Approved for merge.
Reviewed files: catalog-accounts.ts, catalog-accounts.test.js, accounts-route.test.js
Decision: merge + absorbed intake
[宪宪/Opus-46🐾]
2 participants
Summary
migrateProjectAccountsToGlobalthrew a fatal error on startup when globalaccounts.jsondiverged from a project's stalecat-catalog.jsoncopy (e.g. differentdisplayNameormodels)skipConflictsoption tomergeIntoGlobal— when enabled, conflicting accounts are logged and skipped (global wins) instead of throwingskipConflicts: trueand catches persistent errors to avoid retry loopsRoot cause
After PR #420 merged, global accounts could be updated (new models, display name changes) while project
cat-catalog.jsonretained the old snapshot. The strict equality check inmergeIntoGlobaltreated this divergence as a fatal conflict, crashing the app on Windows at startup.Test plan
readCatalogAccountssucceeds and returns global version (not stale project version)pnpm checkpasses (biome)pnpm lintpasses (tsc)🤖 Generated with Claude Code