fix(openai): keep Codex manifest display names in account model lists

This commit is contained in:
Zero Clover
2026-09-11 03:18:00 +08:00
parent 98d86915be
commit 6feefb7aec
4 changed files with 58 additions and 11 deletions
@@ -9,7 +9,12 @@ import (
)
func TestFetchOpenAIAccountModelsOAuthPopulatesPickerFields(t *testing.T) {
_, calls := newCodexModelsOAuthCacheServer(t, `{"models":[{"slug":"new-oauth-model"},{"slug":"gpt-6-astra"}]}`)
_, calls := newCodexModelsOAuthCacheServer(t, `{"models":[
{"slug":"new-oauth-model","display_name":"New OAuth Model"},
{"slug":"gpt-5.6-sol"},
{"slug":"blank-display-name","display_name":" "},
{"slug":"gpt-6-astra"}
]}`)
gateway := &OpenAIGatewayService{}
svc := &AccountTestService{openaiGatewayService: gateway}
account := newCodexModelsTestAccount()
@@ -20,9 +25,16 @@ func TestFetchOpenAIAccountModelsOAuthPopulatesPickerFields(t *testing.T) {
models, err := svc.FetchOpenAIAccountModels(ctx, account)
require.NoError(t, err)
require.Greater(t, len(models), 2)
for i, id := range []string{"new-oauth-model", "gpt-6-astra"} {
require.Equal(t, id, models[i].ID)
require.Equal(t, id, models[i].DisplayName)
// Upstream display name wins; a missing one falls back to the local catalog name,
// then to the raw slug.
for i, expected := range []struct{ id, displayName string }{
{id: "new-oauth-model", displayName: "New OAuth Model"},
{id: "gpt-5.6-sol", displayName: "GPT-5.6 Sol"},
{id: "blank-display-name", displayName: "blank-display-name"},
{id: "gpt-6-astra", displayName: "GPT-6 Astra"},
} {
require.Equal(t, expected.id, models[i].ID)
require.Equal(t, expected.displayName, models[i].DisplayName)
require.Equal(t, "model", models[i].Type)
}
ids := make([]string, 0, len(models))
@@ -35,7 +47,7 @@ func TestFetchOpenAIAccountModelsOAuthPopulatesPickerFields(t *testing.T) {
after, err := gateway.FetchOpenAIModelsList(ctx, account)
require.NoError(t, err)
require.Equal(t, before.Body, after.Body, "picker fields must not change the shared catalog")
require.NotContains(t, string(after.Body), "display_name")
require.Contains(t, string(after.Body), `"display_name":"New OAuth Model"`, "the shared catalog keeps the upstream display name")
require.EqualValues(t, 1, calls.Load(), "picker must reuse the shared discovery cache")
}
@@ -70,6 +82,21 @@ func TestFetchOpenAIAccountModelsPreservesEmptyCatalog(t *testing.T) {
require.Empty(t, models, "an empty upstream catalog must not become a static model list")
}
func TestFetchOpenAIAccountModelsOAuthLabelsLocalImageModelsLikeUpstream(t *testing.T) {
newCodexModelsOAuthCacheServer(t, `{"models":[{"slug":"gpt-5.6-sol"}]}`)
svc := &AccountTestService{openaiGatewayService: &OpenAIGatewayService{}}
account := newCodexModelsTestAccount()
account.Credentials["model_mapping"] = map[string]any{"gpt-image-2.5-flare": "gpt-image-2.5-flare"}
models, err := svc.FetchOpenAIAccountModels(context.Background(), account)
require.NoError(t, err)
byID := make(map[string]string, len(models))
for _, model := range models {
byID[model.ID] = model.DisplayName
}
require.Equal(t, "GPT-5.6 Sol", byID["gpt-5.6-sol"], "upstream slug must not be the only label source")
require.Equal(t, "GPT Image 2.5 Flare", byID["gpt-image-2.5-flare"], "locally added models use the same naming rule")
}
func TestFetchOpenAIAccountModelsOAuthRespectsImageAllowlist(t *testing.T) {
newCodexModelsOAuthCacheServer(t, `{"models":[{"slug":"gpt-6-astra"}]}`)
svc := &AccountTestService{openaiGatewayService: &OpenAIGatewayService{}}
@@ -178,6 +178,8 @@ func (s *AccountTestService) SetOpenAIGatewayService(gateway *OpenAIGatewayServi
}
// FetchOpenAIAccountModels uses the shared cached discovery path for the test picker.
// It only fills picker-only gaps (local display-name fallbacks, OAuth image choices)
// on its own copy; the shared catalog and its cache stay untouched.
func (s *AccountTestService) FetchOpenAIAccountModels(ctx context.Context, account *Account) ([]openai.Model, error) {
if s == nil || s.openaiGatewayService == nil {
return nil, errors.New("OpenAI model discovery service is unavailable")
@@ -192,12 +194,14 @@ func (s *AccountTestService) FetchOpenAIAccountModels(ctx context.Context, accou
if err := json.Unmarshal(response.Body, &payload); err != nil {
return nil, fmt.Errorf("decode OpenAI account models: %w", err)
}
// Standard model catalogs do not require the fields used by the admin picker.
// Populate them here without changing the shared discovery response or cache.
// Every entry in the picker is labelled by the same rule: the upstream display
// name when the catalog has one, otherwise the local catalog name for that model
// ID, otherwise the raw ID. Without this the picker mixes "GPT-5.6 Sol" with
// "gpt-5.6-sol" for the same catalog.
for i := range payload.Data {
model := &payload.Data[i]
if strings.TrimSpace(model.DisplayName) == "" {
model.DisplayName = model.ID
model.DisplayName = openaiCodexDisplayName(model.ID)
}
if strings.TrimSpace(model.Type) == "" {
model.Type = "model"
@@ -219,7 +223,7 @@ func (s *AccountTestService) FetchOpenAIAccountModels(ctx context.Context, accou
}
for model := range account.GetModelMapping() {
if IsGPTImageGenerationModel(model) && !strings.Contains(model, "*") && !seen[model] {
payload.Data = append(payload.Data, openai.Model{ID: model, Object: "model", Type: "model", OwnedBy: "openai", DisplayName: model})
payload.Data = append(payload.Data, openai.Model{ID: model, Object: "model", Type: "model", OwnedBy: "openai", DisplayName: openaiCodexDisplayName(model)})
}
}
}
@@ -96,6 +96,10 @@ func modelCatalogEntries(body []byte, field string) (map[string]json.RawMessage,
return envelope, entries, nil
}
// standardOpenAIModelsBody projects either representation onto the OpenAI /v1/models
// shape. Manifest entries are rebuilt to a minimal entry set; plain OpenAI lists keep
// their upstream fields. Both keep display_name, which the admin picker and alias
// projection read.
func standardOpenAIModelsBody(body []byte, fromManifest bool) ([]byte, error) {
field, idField := "data", "id"
if fromManifest {
@@ -122,7 +126,19 @@ func standardOpenAIModelsBody(body []byte, fromManifest bool) ([]byte, error) {
}
seen[id] = struct{}{}
if fromManifest {
// Codex manifest entries carry dozens of client-only fields (instructions,
// model_messages, reasoning levels) that must not reach the public catalog,
// so the entry is rebuilt from scratch. The display name is the one manifest
// field user-facing surfaces need (admin test picker, mapping aliases), so it
// is carried over explicitly instead of being dropped with the rest.
var displayName string
if err := json.Unmarshal(entry["display_name"], &displayName); err == nil {
displayName = strings.TrimSpace(displayName)
}
entry = make(map[string]json.RawMessage)
if displayName != "" {
entry["display_name"], _ = json.Marshal(displayName)
}
}
entry["id"], _ = json.Marshal(id)
entry["object"] = json.RawMessage(`"model"`)
@@ -58,12 +58,12 @@ func TestFetchOpenAIModelsListUsesStandardRequestAndIsolatesCodexCache(t *testin
}
func TestFetchOpenAIModelsListOAuthSharesManifestCache(t *testing.T) {
_, calls := newCodexModelsOAuthCacheServer(t, `{"models":[{"slug":"special-oauth-model"},{"slug":"gpt-image-1"}]}`)
_, calls := newCodexModelsOAuthCacheServer(t, `{"models":[{"slug":"special-oauth-model","display_name":"Special OAuth Model","description":"manifest only"},{"slug":"gpt-image-1"}]}`)
s := &OpenAIGatewayService{}
account := newCodexModelsTestAccount()
response, err := s.FetchOpenAIModelsList(context.Background(), account)
require.NoError(t, err)
require.JSONEq(t, `{"object":"list","data":[{"id":"special-oauth-model","object":"model","owned_by":"openai","created":0},{"id":"gpt-image-1","object":"model","owned_by":"openai","created":0}]}`, string(response.Body))
require.JSONEq(t, `{"object":"list","data":[{"id":"special-oauth-model","object":"model","owned_by":"openai","created":0,"display_name":"Special OAuth Model"},{"id":"gpt-image-1","object":"model","owned_by":"openai","created":0}]}`, string(response.Body))
manifest, err := s.FetchCodexModelsManifest(context.Background(), account, CodexCanonicalClientVersion(), "")
require.NoError(t, err)
require.Contains(t, string(manifest.Body), `"slug":"special-oauth-model"`)