Conversation
… not alias alone A per-auth rename and a global fork sharing one client-visible alias must both survive: the fork registers the id, the rename decides the upstream body. Keyed on alias alone the per-auth entry evicted the fork and the id was never routable. Exercised live exposing Codex gpt-reserve on 2026-09-02.
![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: 3fcd0dfb7b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…enames Review found two gaps in the (name, alias) merge: a joined-string key can collide when a name or alias contains the joiner, and a global rename that shares its alias with a per-auth entry, if retained, lets the catalog be renamed from the global source while the credential routes the id to its own upstream. The key is now a struct; a shadowed global entry survives only when it is a fork, which is the case the original change exists for.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9aa0aab0a5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… is not in the catalog applyOAuthModelAliasEntries walks the catalog in order, so a global fork A -> X that survives the merge can claim X before the per-auth rename B -> X is reached, leaving B exposed under its own id while requests for X route to B. The fork is a fallback for the hidden-model case only: it is kept when B is absent from the catalog and dropped when B can supply the entry itself. The merge now receives the catalog's id set for that check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7efb11d7ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A per-auth source that oauth-excluded-models removed looks, after filtering, exactly like a hidden upstream model, so the global fork was kept and registered the alias from the global source while request-time resolution (per-auth first) sent it to the excluded model. The merge now receives the credential's exclusion list and treats an excluded source as able to supply the entry, so the fork is dropped and the alias stays unroutable through that credential.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac2af5fdee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
registerModelsForAuth filters the catalog with the global oauth-excluded-models list when a credential carries no synthesized excluded_models attribute (SDK-created or legacy credentials). Reading exclusions back from attributes missed that list, so an excluded source could still read as hidden. Both callers now pass the list they filtered with, and the attribute reader is gone.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e27241d12
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…fork Exclusions run before aliases, so a fork that survived the merge would recreate an excluded alias from the global source while requests for it routed to the per-auth upstream. The exclusion list is now checked against the alias as well as the source, wildcards included.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58f7cee905
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…fork fallback registerModelsForAuth appends static plugin models after the alias pass, so a per-auth source a plugin supplies was absent from the catalog the merge saw and read as hidden, keeping a fork that then registered the alias from the global source. The caller now passes the ids it will end up serving, plugin models included.
|
v7.3.2 adds buildAntigravityReverseAliasMap, which calls the upstream three-argument oauthModelAliasesForAuth. Combining it with this PR's five-argument helper breaks compilation, even though the PR's historical checks are green. The local compatibility fix passes nil for the catalog and exclusions on this probe path. There is also an ordering issue beyond compilation: the pair merge can return a per-auth rename and a global fork under one visible alias. The reverse map must keep the first entry, matching per-auth-first request routing, rather than overwrite it with the later global fork. Two new local tests cover per-auth precedence and an unshadowed global fork, including credential-prefix removal. The service package, alias-pair regressions, and full server build pass on the repaired v7.3.2 stack. These compatibility changes are committed in the local carry, not yet added to this PR's head. |
dev added buildAntigravityReverseAliasMap, which calls the three-argument oauthModelAliasesForAuth. The merge passes nil catalog and nil exclusions there so the tree builds: the probe maps ids that are already registered back to their upstream and has no catalog of its own. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The (name, alias) merge can return a per-auth rename and a global fork under one client-visible alias, per-auth first. buildAntigravityReverseAliasMap overwrote the first entry with the later fork, so the capability probe looked up the fork's source instead of the upstream the credential is actually sent. The first entry for an alias now wins, matching request-time resolution. Before the pair merge the table held at most one entry per alias, so existing configs map the same way. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 participant
Summary
oauth-model-aliasmerge on the(name, alias)pair instead ofaliasalone, so a global fork and a per-auth rename that share one client-visible id both surviveProblem
oauthModelAliasesForAuth(sdk/cliproxy/service_models.go) merges a credential'smodel_aliaseswith the globaloauth-model-aliastable and dedupes onalias. Two entries that share a client-visible alias but map to different upstream names cannot coexist, so the per-auth entry evicts the global one.That blocks exposing a hidden upstream model. A global fork
{name: gpt-5.6-luna, alias: gpt-5.6-luna-reserve, fork: true}registers the id in the catalog, becauseapplyOAuthModelAliasEntriescan only clone an id already in the model list. A per-auth{name: gpt-reserve, alias: gpt-5.6-luna-reserve}decides what that credential sends upstream, because request-time resolution consults per-auth aliases first. With the alias-only key the fork is dropped, the id is never registered, and every request for it fails withunknown provider. I reproduced this on a Codex Plus account withgpt-reserveon 2026-09-02.Fix
{name, alias}on the lowercased, trimmed strings, so a name or alias containing a joiner cannot collide with another pair. Identical pairs still collapse with the per-auth copy first, so existing configs are unchanged.applyOAuthModelAliasEntries, which walks the catalog in order, build the entry from the global source while the credential routes the id to its own upstream. The registered entry would carry the wrong metadata and the per-auth source would stay exposed under its own id.oauth-excluded-modelsremoved is not a hidden model, even though the filtered catalog makes it look like one.applyOAuthModelAliasForAuthtakes the effective exclusion list, and both callers pass the list they filtered the catalog with (the global table or the synthesizer's merged attribute). The merge drops the fork when either the source or the client-visible alias is excluded, wildcards included, so the id stays unroutable through that credential.registerModelsForAuthappends static plugin models after the alias pass. It now passes the ids it will end up serving, plugin models included, so a plugin-supplied source is not mistaken for a hidden one.Antigravity reverse alias map
buildAntigravityReverseAliasMapinsdk/cliproxy/antigravity_models.go, added todevafter this PR opened, maps registered ids back to upstream ids for the web-search capability probe. It called the old three-argument merge, sodevplus this PR did not compile. It now passes a nil catalog and nil exclusions. The probe only looks up ids that are already registered and has no catalog of its own, and a nil catalog keeps every global fork, so an id that only a fork registered still resolves.The merge can now return a per-auth rename and a global fork under one alias, per-auth first. The map used to overwrite the first entry with the later fork, which made the probe check the fork's source instead of the model the credential is sent. The first entry for an alias now wins. Before the pair key the merged table held at most one entry per alias, so this ordering does not change results for existing configs.
Tests
sdk/cliproxy/service_models_alias_pair_test.go. Each failed ondev@ aa36527 before the change:KeepsForkAlongsidePerAuthRename: fork and rename both survive, per-auth firstDropsShadowedGlobalRename: a shared alias on a non-fork global entry yields only the per-auth entryDropsGlobalForkWhenPerAuthSourceIsInCatalog: with both sources in the catalog, only the per-auth rename survives and the end-to-end catalog is[gpt-5.6-luna, gpt-5.6-luna-reserve]DropsGlobalForkWhenPerAuthSourceIsExcluded: with the per-auth source excluded, the fork is dropped and the alias is not registeredDropsGlobalForkWhenAliasIsExcluded: with the alias excluded by wildcard, the fork is dropped and the alias is not recreatedSeesPluginSourcesAsPresent: a per-auth source a static plugin supplies counts as present, so the fork is droppedPairKeyIsCollisionFree:{a->b, c}and{a, b->c}are two pairs, not oneStillDedupesSamePair: an identical pair collapses to the per-auth copysdk/cliproxy/antigravity_models_alias_test.go:BuildAntigravityReverseAliasMapPrefersPerAuthEntry: with a per-auth rename and a global fork on one alias, the map returns the per-auth upstream. It fails on merge commit 6f07047, before the ordering change.BuildAntigravityReverseAliasMapKeepsUnshadowedGlobalFork: a fork with no per-auth entry for its alias stays in the map, and a prefixed id resolves through it.Run on this branch after merging
dev@ aafa4e9:All pass except
TestGetDevinModelsFallback(internal/registry) andTestRegisterModelsForAuth_AntigravityFetchesWebSearchCapability(sdk/cliproxy), which fail the same way on unmodifieddev@ aafa4e9 with the embedded model catalog. Both assert catalog data (OwnedBy,SupportsWebSearch), not alias behavior. CI refreshes the catalog before building and does not run tests.Overlap
internal/runtime/executorandinternal/config, not these files.feat/per-credential-fallback-models) builds on this merge. This PR does not depend on it.🤖 Generated with Claude Code