fix(config): make resolveUserConfig idempotent (no spurious self-collision warning) - #24
Merged
Merged
Conversation
…lision warning resolveUserConfig flagged a "user BYO name collides with admin" collision whenever a base entry shared a name with the user's built entry — looking only at the NAME, not whether the base entry was genuinely admin's. When an already-resolved config (which already contains the user's merged BYO) flows back through a second resolveUserConfig call, the user's own codex-ep / gpt-5.5 sit in `base` and got flagged as admin collisions, emitting [user-config] <u>: user BYO name(s) collide with admin (codex-ep, gpt-5.5); admin wins, dropped the colliding entries, kept the rest even on a deployment with an EMPTY admin registry. Functionally harmless (the merged result is identical) but a false alarm — and the same self-collision class as the earlier 7febede probe-base bug. The 71a6274 BYO fix added a second resolution point (runDispatchedAgent), so run-subagent / background-task/runner pre-resolve then runDispatchedAgent resolves again → double-resolve → the warning. A collision is now a base entry that DIFFERS (isDeepStrictEqual) from the user's built entry. Self-merge entries are byte-identical → not flagged → resolveUserConfig is idempotent. A real admin shadow is structurally distinct (raw apiKey / global auth vs the user's apiKeyRef + credentialIdentity) → still differs → still dropped + warned. Regression test (c3): double-resolve emits no "collide with admin" warning and equals the single resolve (fails on old code with the exact warning). Real-collision cases (c)/(c2) still drop + preserve admin. Full suite 2620/2620, typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a false-positive “user BYO name(s) collide with admin” warning by making resolveUserConfig idempotent when it receives a config that already includes the same user’s merged BYO entries (double-resolve scenario).
Changes:
- Update collision detection for user BYO endpoints/models to only treat it as a collision when the base entry exists and differs structurally from the user-built entry (
isDeepStrictEqual). - Add a regression test ensuring double-resolving does not emit the collision warning and produces identical resolved registries.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/config/user-override.ts | Makes collision detection idempotent by deep-comparing base vs user-built entries before treating them as admin collisions. |
| src/config/user-override.test.ts | Adds regression coverage for the double-resolve “self-collision warning” scenario. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
症状
纯 BYO 部署(admin 全局 config 空)下,日志反复出现:
```
[user-config] puyuclaw: user BYO name(s) collide with admin (codex-ep, gpt-5.5); admin wins, dropped the colliding entries, kept the rest
```
但 admin 根本没配同名条目。
根因
resolveUserConfig判撞名只看名字在不在 base 里,不看那条到底是不是 admin 的。当一份已 resolve 过、含用户 BYO 的 config 再次流经resolveUserConfig(双重 resolve),用户自己的codex-ep/gpt-5.5此时就在base里,被当成 admin 撞名 → 打这条 warning。双重 resolve 的来源:
71a6274(BYO 修复)加了第二个解析点runDispatchedAgent,而run-subagent/background-task/runner已经先 resolve 了一遍再传进来 → resolve 两次。功能上无害(最终注册表一致),但日志是假警告,且与7febede的 probe-base 自撞名同一类。修复
撞名判据改为「base 条目与用户 built 条目结构不同」(
isDeepStrictEqual):resolveUserConfig幂等。apiKey/全局authvs 用户的apiKeyRef+credentialIdentity)→ 仍判撞名 → 仍丢弃 + warn。测试
回归 (c3):双重 resolve 不打
collide with admin且等于单次 resolve(旧码上 fail,打出与用户一字不差的 warning)。真撞名 (c)/(c2) 仍丢弃 + 保留 admin。全量 2620/2620、typecheck 干净。🤖 Generated with Claude Code