cleanup(#340): config architecture cleanup — protocol removal + accounts rename#376
cleanup(#340): config architecture cleanup — protocol removal + accounts rename#376
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: cbd0375918
ℹ️ 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: 795cd580e9
ℹ️ 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
worktree-f340-config-cleanup-impl
branch
from
795cd58 to
7b2ec52
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b2ec522a6
ℹ️ 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: eaf264b34f
ℹ️ 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: 4fe4f46bcf
ℹ️ 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: ae35d0fae9
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e408c63910
ℹ️ 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: d7ac065574
ℹ️ 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: 0d338360e1
ℹ️ 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: 415705178e
ℹ️ 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: 32d20bf6f1
ℹ️ 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: d3aae974e4
ℹ️ 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: 496c1f1b7a
ℹ️ 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
worktree-f340-config-cleanup-impl
branch
from
496c1f1 to
1ca7712
Compare
|
@codex review Please review latest commit df6d271 for P1/P2 only. 规则：任何 P1/P2 必须给"可执行复现"：
审查标准（详见 AGENTS.md "Review guidelines" section）：
|
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ 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". |
ZephaniaCN
left a comment
ZephaniaCN
left a comment
There was a problem hiding this comment.
P1 Blocker — DELETE 语义升级风险
问题
全局 accounts.json 迁移后，DELETE /api/provider-profiles/:profileId 的语义从项目级删除升级为全局账户删除。当前缺少两层保护：
- 引用检查：如果某个 breed/variant 的 accountRef 仍指向该 profile，删除后会导致猫猫调用失败（静默 500）
- Force 确认：全局删除应要求显式 ?force=true，防止误删
最小修复方案
DELETE /api/provider-profiles/:id
→ 扫描 cat-config breeds[].variants[].accountRef
→ 如有引用 → 400 { referencedBy: [...] }
→ 加 ?force=true 才允许跳过检查
影响范围
- packages/api/src/routes/provider-profiles.ts DELETE handler
- 需新增 findAccountRefUsages(profileId) 工具函数
PR 其余部分（CRUD、前端面板）质量没问题，仅 DELETE 语义需补丁。修复后可合并。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 901e2c02b3
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 901e2c02b3
ℹ️ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 901e2c02b3
ℹ️ 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".
…l consumer Delete PROTOCOL_ENV_KEY_MAP, resolveEnvFallbackKey(), and the env fallback in accountToRuntimeProfile. Credentials must come from stored credentials.json; process.env fallback based on account.protocol is removed as part of protocol退場. #376 will handle .env API key consolidation separately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mindfn
force-pushed
the
worktree-f340-config-cleanup-impl
branch
from
901e2c0 to
9c99718
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c9971840a
ℹ️ 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".
Change `--project-dir DIR` to `--project-dir <your-project-dir>` and add full command path so it's clear this is a placeholder, not a copy-paste-ready command. [宪宪/Opus-46🐾]
P1: migrateLegacyFrom() now correctly flattens v1 nested providers.<client>.profiles[] into individual account entries, instead of creating a shell account keyed by the client name. Also handles v1 nested secrets format (providers.<client>.<id>). P2: Secret import no longer gates on mergedIds (only populated on first run). Uses `id in accounts` instead, so credentials are imported on retry even when accounts already exist in global from a previous partial migration. Both issues confirmed by Codex review on c0821f7 with reproduction evidence. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous fix (8428a67) replaced mergedIds gate with `id in accounts` to make retry-safe, but also allowed a legacy API key to be silently attached to a pre-existing OAuth account with the same ID. Fix: two-tier credential import gate: - First run (mergedSet): account just merged → always import secret - Retry (skipped): account already existed → only import if the global account is api_key-typed, preventing cross-contamination to OAuth Regression test added for the collision path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previous fix only checked authType, allowing same-typed api_key accounts from different sources to get cross-contaminated. Now the retry path compares authType + baseUrl + displayName between global and legacy source — only imports the credential when the global account matches what this migration would have produced, proving it came from a previous partial run of the same source (not a collision with a different-origin account). Regression test added for api_key collision with different baseUrl. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rtup fail-fast Blocker 1: resolveGlobalRoot() now defaults to projectRoot instead of homedir() when CAT_CAFE_GLOBAL_CONFIG_ROOT is unset. Threaded projectRoot through catalog-accounts.ts (14 call sites), credentials.ts (5 public APIs), account-resolver.ts, and routes/accounts.ts. Blocker 2: account-startup.ts restored with LL-043 fail-fast contract — migration errors with legacy source → wrapped hard error, credentials read failure → hard error, legacy present + no accounts → hard error. Tests: 40/40 pass (16 catalog-accounts + 5 startup + 19 installer). [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
P1-1: index.ts no longer wraps accountStartupHook in try/catch+warn. Errors now propagate to main().catch → process.exit(1), enforcing the LL-043 fail-fast contract end-to-end. P1-2: DELETE /api/accounts/:id now scans BOTH cat-catalog.json AND cat-template.json for accountRef bindings. Template-only projects (pre-bootstrap) no longer silently allow deletion of bound accounts. Review: 缅因猫(gpt52) + ZephaniaCN [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Timer precision can cause setTimeout to fire ~1ms early, producing idleSinceMs < threshold. A threshold event must never report a duration smaller than its own trigger point — clamp with Math.max. Fixes: F149 idle watchdog CI flake (49ms < 50ms assertion) [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… homedir) The installer script resolveGlobalRoot() fell back to homedir() when CAT_CAFE_GLOBAL_CONFIG_ROOT was unset, while runtime catalog-accounts.ts falls back to projectRoot. This caused accounts written by the installer to land in ~/.cat-cafe/ instead of the project's .cat-cafe/ directory. Add _activeProjectDir module-level state set at all 4 CLI entry points so resolveGlobalRoot() uses the same env → projectRoot → homedir cascade as the runtime. Adds a test that exercises this path without the env override that previously masked the bug in all existing tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…er default The delete guard in accounts.ts resolved to homedir() when CAT_CAFE_GLOBAL_CONFIG_ROOT was unset, while the storage layer (catalog-accounts.ts) correctly defaulted to projectRoot. This caused spurious 409 "shared global store" errors on non-force DELETE when accounts were actually project-local. Now uses the same env → projectRoot → homedir cascade. Regression test covers the default-path scenario without env override. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adapt #375's resolveTargetAccountRef/resolveNextCli/resolveEffectiveAccountRefForUpdate to #340 naming (ClientId, body.clientId, currentCat.clientId). Add defense-in-depth effort validation in getCatEffort to reject stale cross-provider effort values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mindfn
force-pushed
the
worktree-f340-config-cleanup-impl
branch
from
50be6c1 to
d33ebc6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d33ebc646e
ℹ️ 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".
✅ Merge Gate passed — merging nowBoth maintainer cats (布偶猫/Opus-46 + 缅因猫/GPT-5.4) have aligned:
Thank you @mindfn for your patience and thorough work on this — 45 commits, 11 rounds of Codex review, and every P1 we raised was addressed with evidence. This is a model community contribution. We'll proceed with squash merge now, followed by our internal intake process. — 布偶猫/Opus-46 🐾 |
zts212653
left a comment
zts212653
left a comment
There was a problem hiding this comment.
All blockers resolved. Protocol runtime-only ✓, project-local default root ✓, startup fail-fast ✓, route-layer alignment ✓. Approving on behalf of maintainer team (布偶猫/Opus-46 + 缅因猫/GPT-5.4).
— 布偶猫/Opus-46 🐾
3 participants
Summary
Issue #340: 配置架构冗余梳理 — cat-config/template/catalog/provider-profiles 四源归一
Core cleanup of the configuration architecture that eliminates redundant code. This PR completes the new architecture landing + naming unification + protocol retirement, with two migration bridges intentionally preserved.
What changed
AccountConfig.protocolfield removed from shared types. No longer written during migration, account creation, or edit. Resolver derives protocol solely from well-known account IDs at runtime (BUILTIN_ACCOUNT_MAP). Legacy protocol-matching fallback deleted./api/accounts/:profileId/test(incomplete feature, no frontend entry point) deleted along with all probe/heuristic protocol inference (accounts-probe.ts,inferProbeProtocol).provider-profilesroutes, types, components, and tests renamed toaccountssemantics.LlmAIProvider,index.tsabstractive summary,resolveAnthropicRuntimeProfilemigrated from protocol-based discovery toresolveForClient()full discovery chain (well-known →builtin_→installer-).CatProvider→ClientId,provider→clientId(on CatVariant),ocProviderName→providernaming cleanup.cli,color,contextBudget,voiceConfigare atomic-replaced during config overlay.resolveForClient()fails closed when explicitpreferredAccountRefmissesinstaller-${client}API key accounts in default resolution pathLlmAIProvider) use full discovery chain, not hardcoded builtin refsforce: trueon account delete — server-side reference guard effective.client-auth removeexits non-zero when--forceomitted.CAT_CAFE_DIR→CONFIG_SUBDIRacross all config modules.account-startup.tsrewritten with three-path fail-fast — migration failure, credential corruption, and empty-migration-with-legacy all throw hard errors. Errors propagate tomain().catch → process.exit(1).findBoundCatIds()scans bothcat-catalog.jsonandcat-template.jsonfor variant→account bindings before allowing account deletion.install-auth-config.mjsresolveGlobalRoot()now uses the same env → projectRoot → homedir cascade as runtimecatalog-accounts.ts, preventing accounts from landing in~/.cat-cafe/instead of project-local.cat-cafe/.Key design decisions
BUILTIN_ACCOUNT_MAP). Custom accounts don't get protocol — they are reached via explicitaccountRef. No protocol is read from or written toaccounts.json.accountRefbinding orinstaller-${client}naming convention. Protocol-based discovery is fully removed.${ROOT}/.cat-cafe/). Only renamed constants and updated descriptions.projectRoot/.cat-cafe/), consistent between runtime and installer.CAT_CAFE_GLOBAL_CONFIG_ROOTenv overrides for shared/global use cases.Intentionally preserved legacy compat (follow-up scope)
providerProfileId → accountRefread-time migration incat-catalog-store.tsprovider-profiles.json → accounts.jsonmigration incatalog-accounts.tsandinstall-auth-config.mjsThese two migration bridges will be removed once all instances have migrated.
Test plan
pnpm lint— TypeScript type check passespnpm check— Biome 0 errorspnpm -F @cat-cafe/api buildpassesContributes to #340
🤖 Generated with Claude Code