fix(cliproxy): keep original model ids routable with aliases#4125
fix(cliproxy): keep original model ids routable with aliases#4125daiaji wants to merge 6 commits into
Conversation
|
This pull request targeted The base branch has been automatically changed to |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the model registration logic to ensure that both the original model name and its configured alias are registered and remain routable. It introduces a helper function appendModelInfoIfUnique to handle deduplication. A new test is added to verify this behavior for Codex and OpenAI compatibility configurations. Feedback was provided to optimize appendModelInfoIfUnique by avoiding unnecessary cloning of the ModelInfo struct, which can be modified and appended directly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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: 8b42bc309b
ℹ️ 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".
| if modelID == "" { | ||
| return models | ||
| } | ||
| key := strings.ToLower(modelID) |
There was a problem hiding this comment.
When a configured alias differs from the upstream name only by capitalization, this case-folded de-dupe (together with the EqualFold checks in the two builders) still registers only the alias. Provider lookup is an exact map lookup by the requested model ID, so Name: "gpt-5" with Alias: "GPT-5" leaves requests for the real upstream gpt-5 failing with unknown provider for model, even though this change is intended to keep original IDs routable.
Useful? React with 👍 / 👎.
appendModelInfoIfUnique receives freshly allocated ModelInfo values from its callers, so cloning the struct before appending is unnecessary. Trim and assign the normalized ID directly on the provided value, then append it to the model list. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28f8f0acda
ℹ️ 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".
| if name != "" && alias != name { | ||
| nameInfo := &ModelInfo{ | ||
| ID: name, |
There was a problem hiding this comment.
When an API-key config uses an alias and excludes that alias (the only registered ID before this change), registerModelsForAuth still runs applyExcludedModels on the expanded slice. This new branch adds the upstream name as another ID, so a config like models: [{name: "gpt-5", alias: "my-gpt"}] with excluded-models: ["my-gpt"] now registers gpt-5 and leaves the same credential selectable for the excluded upstream model. Please make exclusions apply to the whole alias/name pair or filter before adding the extra routable ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2a2cb295d
ℹ️ 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".
| nameInfo := &ModelInfo{ | ||
| ID: name, | ||
| Object: "model", |
There was a problem hiding this comment.
When one API-key config aliases a built-in model while another credential for the same provider registers the normal catalog, this new original-ID entry can be the last RegisterClient writer for that provider/model. The registry stores a single ModelInfo per provider, so the minimal nameInfo replaces the static entry for e.g. gpt-5.4-mini, dropping context length/supported parameters and marking the built-in as user-defined in lookups/model listings. Build the original-ID entry from the static ModelInfo when it exists (or avoid overwriting it), rather than constructing a fresh minimal record.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9df43e970
ℹ️ 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".
| if name == "" { | ||
| return nil | ||
| } | ||
| if info := registry.LookupStaticModelInfo(name); info != nil { |
There was a problem hiding this comment.
Fresh issue in the static-metadata fix: when an OpenAI-compatible entry aliases a static native model (for example name: "gemini-2.5-pro", alias: "gemini-pro"), this branch copies the native static Thinking field and ignores the OpenAI-compatible thinking value passed in from buildOpenAICompatibilityConfigModels. The alias keeps level-based OpenAI-compatible reasoning, but the newly registered original ID is looked up with Gemini/Claude budget metadata, so direct requests such as gemini-2.5-pro(high) do not have the suffix applied through the OpenAI-compatible executor. Please overlay the provider/config Thinking on top of the static metadata for these original-ID entries.
Useful? React with 👍 / 👎.
1 participant
Summary
codex-api-keyandopenai-compatibilityproviders when aliases are configured.Why
When a config model had an alias, only the alias was registered in the global model registry. Requests using the original upstream model ID failed at provider lookup with
unknown provider for modelbefore executor-level alias resolution could mapname -> name.Tests
go test ./sdk/cliproxy ./sdk/api/handlers -count=1 -timeout 120sgo build -o /tmp/opencode/cli-proxy-api-pr-test ./cmd/server