diff --git a/backend/internal/service/account_test_models_test.go b/backend/internal/service/account_test_models_test.go index ba8b511a57..59af5edbd9 100644 --- a/backend/internal/service/account_test_models_test.go +++ b/backend/internal/service/account_test_models_test.go @@ -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{}} diff --git a/backend/internal/service/account_test_service.go b/backend/internal/service/account_test_service.go index 44e29dfc1e..b57491e71b 100644 --- a/backend/internal/service/account_test_service.go +++ b/backend/internal/service/account_test_service.go @@ -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)}) } } } diff --git a/backend/internal/service/openai_models_list.go b/backend/internal/service/openai_models_list.go index 42740c2df3..2a07f10d67 100644 --- a/backend/internal/service/openai_models_list.go +++ b/backend/internal/service/openai_models_list.go @@ -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"`) diff --git a/backend/internal/service/openai_models_list_test.go b/backend/internal/service/openai_models_list_test.go index c898c06189..08df6e249c 100644 --- a/backend/internal/service/openai_models_list_test.go +++ b/backend/internal/service/openai_models_list_test.go @@ -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"`)