fix(codex): address routed catalog review feedback

This commit is contained in:
Long Li
2026-08-23 15:43:57 +09:00
parent e39fce270d
commit fc589bce17
5 changed files with 267 additions and 35 deletions
@@ -298,6 +298,23 @@ func TestCodexModelsFailsOverFromRetryableUpstreamStatus(t *testing.T) {
}
}
// Scenario: an API-key upstream without /models is excluded only for this discovery request.
func TestCodexModelsFailsOverWhenAPIKeyModelsEndpointIsUnavailable(t *testing.T) {
for _, status := range []int{http.StatusNotFound, http.StatusMethodNotAllowed} {
t.Run(http.StatusText(status), func(t *testing.T) {
handler, upstream, groupID := newCodexModelsFailoverTestHandler(status)
recorder := performCodexModelsRequest(t, handler, groupID)
if got, want := upstream.calls(), []int64{1, 2}; !equalInt64Slices(got, want) {
t.Fatalf("upstream account calls: got %v, want %v", got, want)
}
if recorder.Code != http.StatusOK {
t.Fatalf("status: got %d, want %d; body=%s", recorder.Code, http.StatusOK, recorder.Body.String())
}
})
}
}
func TestCodexModelsFailsOverFromUpstreamTransportError(t *testing.T) {
handler, upstream, groupID := newCodexModelsFailoverTestHandler(http.StatusServiceUnavailable)
upstream.firstErr = &net.OpError{
@@ -334,7 +351,6 @@ func TestCodexModelsDoesNotFailOverFromPermanentUpstreamStatus(t *testing.T) {
http.StatusBadRequest,
http.StatusUnauthorized,
http.StatusForbidden,
http.StatusNotFound,
600,
}
for _, status := range statuses {
@@ -45,13 +45,8 @@ func groupCodexModelMetadata(
}
}
}
if explicitClaims && codexExplicitModelTargetsConflict(accounts, modelID) {
return codexModelMetadataOverride{
reasoningConflict: true,
inputModalitiesConflict: true,
}, true
}
explicitTargetsConflict := explicitClaims && codexExplicitModelTargetsConflictForPlatform(accounts, platform, modelID)
publicAlias := upstreamModel != modelID
candidates := make([]UpstreamModelMetadata, 0)
for i := range accounts {
account := &accounts[i]
@@ -70,8 +65,17 @@ func groupCodexModelMetadata(
}
lookupModel = account.GetMappedModel(upstreamModel)
}
if strings.TrimSpace(lookupModel) != modelID {
publicAlias = true
}
metadata, ok := account.GetUpstreamModelMetadata(lookupModel)
if !ok {
if explicitTargetsConflict {
return codexModelMetadataOverride{
reasoningConflict: true,
inputModalitiesConflict: true,
}, true
}
return codexModelMetadataOverride{}, false
}
candidates = append(candidates, metadata)
@@ -79,7 +83,12 @@ func groupCodexModelMetadata(
if len(candidates) == 0 {
return codexModelMetadataOverride{}, false
}
return intersectUpstreamModelMetadata(modelID, candidates), true
metadata := intersectUpstreamModelMetadata(modelID, candidates)
if publicAlias {
metadata.DisplayName = modelID
metadata.Description = configuredCodexCustomDescription
}
return metadata, true
}
func codexExplicitModelTargetsConflict(accounts []Account, modelID string) bool {
@@ -96,6 +105,23 @@ func codexExplicitModelTargetsConflict(accounts []Account, modelID string) bool
return len(targets) > 1
}
func codexExplicitModelTargetsConflictForPlatform(accounts []Account, platform, modelID string) bool {
targets := make(map[string]struct{})
for i := range accounts {
account := &accounts[i]
if account.Platform != platform {
continue
}
mappedModel, matched := account.ResolveMappedModel(modelID)
mappedModel = strings.TrimSpace(mappedModel)
if !matched || mappedModel == "" {
continue
}
targets[mappedModel] = struct{}{}
}
return len(targets) > 1
}
func intersectUpstreamModelMetadata(modelID string, candidates []UpstreamModelMetadata) codexModelMetadataOverride {
result := codexModelMetadataOverride{UpstreamModelMetadata: UpstreamModelMetadata{ID: strings.TrimSpace(modelID)}}
for _, candidate := range candidates {
@@ -135,6 +135,107 @@ func TestBuildCodexModelsManifestForGroupIntersectsSyncedAccountMetadata(t *test
require.EqualValues(t, 128_000, models[0]["context_window"])
}
// Scenario: the same public alias may target different models on one platform when complete snapshots can be intersected.
func TestBuildCodexModelsManifestForGroupIntersectsDifferentMappedTargetsWithoutLeakingAlias(t *testing.T) {
t.Parallel()
const groupID int64 = 739
reasoning := true
newAccount := func(id int64, target, displayName, description string, levels, modalities []string, contextWindow int64) Account {
account := Account{
ID: id, Platform: PlatformOpenAI, Type: AccountTypeAPIKey,
Credentials: map[string]any{
"base_url": "https://provider.example/v1",
"model_mapping": map[string]any{"my-coder": target},
},
}
account.SetUpstreamModelMetadataSnapshot(UpstreamModelMetadataSnapshot{Models: map[string]UpstreamModelMetadata{
target: {
ID: target, DisplayName: displayName, Description: description, Reasoning: &reasoning,
SupportedReasoningLevels: levels,
InputModalities: modalities,
ContextWindow: contextWindow,
},
}})
return account
}
openAIAccount := newAccount(
31,
"gpt-5.6-sol",
"GPT-5.6 Sol",
"OpenAI upstream model",
[]string{"low", "medium", "high", "xhigh"},
[]string{"text", "image"},
272_000,
)
arkAccount := newAccount(
32,
"glm-5.3",
"GLM 5.3",
"Ark upstream model",
[]string{"low", "medium", "high"},
[]string{"text"},
1_000_000,
)
for _, accounts := range [][]Account{{openAIAccount, arkAccount}, {arkAccount, openAIAccount}} {
svc := &GatewayService{accountRepo: codexModelsVisibilityAccountRepo{byGroup: map[int64][]Account{
groupID: accounts,
}}}
body, err := svc.BuildCodexModelsManifestForGroup(
context.Background(), &Group{ID: groupID, Platform: PlatformOpenAI}, "", []string{"my-coder"},
)
require.NoError(t, err)
models := decodeCodexManifestModels(t, body)
require.Len(t, models, 1)
require.Equal(t, "my-coder", models[0]["slug"])
require.Equal(t, "my-coder", models[0]["display_name"])
require.Equal(t, "Custom model routed through Sub2API.", models[0]["description"])
require.Equal(t, []string{"low", "medium", "high"}, effortsFromManifestModel(t, models[0]))
require.Equal(t, []any{"text"}, models[0]["input_modalities"])
require.EqualValues(t, 272_000, models[0]["context_window"])
}
}
// Scenario: a Composite alias claimed across platforms remains ambiguous and fails closed.
func TestBuildCodexModelsManifestForGroupKeepsCrossPlatformAliasAmbiguityClosed(t *testing.T) {
t.Parallel()
const groupID int64 = 740
reasoning := true
newAccount := func(id int64, platform, target string) Account {
account := Account{
ID: id, Platform: platform, Type: AccountTypeAPIKey,
Credentials: map[string]any{"model_mapping": map[string]any{"shared-alias": target}},
}
account.SetUpstreamModelMetadataSnapshot(UpstreamModelMetadataSnapshot{Models: map[string]UpstreamModelMetadata{
target: {
ID: target, DisplayName: target, Reasoning: &reasoning,
SupportedReasoningLevels: []string{"low", "high"},
InputModalities: []string{"text", "image"},
ContextWindow: 128_000,
},
}})
return account
}
svc := &GatewayService{accountRepo: codexModelsVisibilityAccountRepo{byGroup: map[int64][]Account{
groupID: {
newAccount(33, PlatformOpenAI, "gpt-5.6-sol"),
newAccount(34, PlatformGrok, "grok-4.6"),
},
}}}
body, err := svc.BuildCodexModelsManifestForGroup(
context.Background(), &Group{ID: groupID, Platform: PlatformComposite}, "", []string{"shared-alias"},
)
require.NoError(t, err)
models := decodeCodexManifestModels(t, body)
require.Len(t, models, 1)
require.Equal(t, "shared-alias", models[0]["display_name"])
require.Empty(t, effortsFromManifestModel(t, models[0]))
require.Equal(t, []any{"text"}, models[0]["input_modalities"])
}
func TestBuildCodexModelsManifestForGroupDoesNotAdvertiseNoneWhenAccountReasoningConflicts(t *testing.T) {
t.Parallel()
@@ -109,21 +109,28 @@ func (s *OpenAIGatewayService) BuildGroupConfiguredCodexModelsManifest(
return nil, false, nil
}
configuredModels, err := s.groupConfiguredCodexModelIDs(ctx, group.ID)
accounts, err := s.accountRepo.ListSchedulableByGroupID(ctx, group.ID)
if err != nil {
return nil, false, fmt.Errorf("load group configured Codex models: %w", err)
}
configuredModels := openAIConfiguredCodexModelIDs(accounts)
if len(configuredModels) == 0 {
return nil, false, nil
}
body, err := BuildCodexModelsManifest(nil)
body, err := buildCodexModelsManifestForAccounts(
PlatformOpenAI,
configuredModels,
accounts,
nil,
true,
)
if err != nil {
return nil, false, fmt.Errorf("initialize group configured Codex models: %w", err)
}
body, _, err = mergeConfiguredCodexModelsManifest(
body,
configuredModels,
nil,
group.ModelsListConfig.Models,
group.CustomModelsListEnabled(),
)
@@ -187,7 +194,10 @@ func (s *OpenAIGatewayService) groupConfiguredCodexModelIDs(ctx context.Context,
if err != nil {
return nil, err
}
return openAIConfiguredCodexModelIDs(accounts), nil
}
func openAIConfiguredCodexModelIDs(accounts []Account) []string {
seen := make(map[string]struct{})
models := make([]string, 0)
for i := range accounts {
@@ -208,11 +218,12 @@ func (s *OpenAIGatewayService) groupConfiguredCodexModelIDs(ctx context.Context,
}
}
sort.Strings(models)
return models, nil
return models
}
const (
configuredCodexModelPriority = 50
configuredCodexCustomDescription = "Custom model routed through Sub2API."
configuredCodexFallbackContext = 272_000
configuredCodexDeepSeekV4Context = 1_000_000
configuredCodexGrokContext = 500_000
@@ -304,7 +315,7 @@ func newConfiguredCodexModelDescriptor(modelID string) configuredCodexModelDescr
descriptor := configuredCodexModelDescriptor{
Slug: modelID,
DisplayName: modelID,
Description: "Custom model routed through Sub2API.",
Description: configuredCodexCustomDescription,
DefaultReasoningLevel: &noReasoningLevel,
SupportedReasoningLevels: []configuredCodexReasoningLevel{
{Effort: "none", Description: configuredCodexReasoningLevelDescription("none")},
@@ -652,6 +663,22 @@ func (s *GatewayService) BuildCodexModelsManifestForGroup(
compositeRoutesAvailable = false
}
}
return buildCodexModelsManifestForAccounts(
effectivePlatform,
modelIDs,
accounts,
compositeRoutes,
compositeRoutesAvailable,
)
}
func buildCodexModelsManifestForAccounts(
effectivePlatform string,
modelIDs []string,
accounts []Account,
compositeRoutes []CompositeModelRoute,
compositeRoutesAvailable bool,
) ([]byte, error) {
imageInputModels := make(map[string]bool, len(modelIDs))
metadataModels := codexCatalogMetadataModels(
effectivePlatform,
@@ -717,6 +744,10 @@ func buildCodexModelsManifest(
if metadata, ok := modelMetadata[modelID]; ok {
applyUpstreamModelMetadataToCodexDescriptor(&descriptor, metadata)
}
if metadataModelID != modelID {
descriptor.DisplayName = modelID
descriptor.Description = configuredCodexCustomDescription
}
models = append(models, descriptor)
}
return json.Marshal(struct {
@@ -1135,12 +1166,13 @@ func (e *codexModelsManifestUpstreamError) Error() string { return e.err.Error()
func (e *codexModelsManifestUpstreamError) Unwrap() error { return e.err }
// IsRetryableCodexModelsManifestError reports whether another selected account
// may succeed without changing the request. Configuration and upstream 4xx
// responses, except 429 and ChatGPT-backend 401, are intentionally not
// retried. A manifest 401 from the ChatGPT Codex backend reflects the selected
// OAuth account's upstream token rather than the client request (the client's
// own API key was already validated locally), so a different account may still
// serve the manifest. Custom API key upstreams keep the old no-failover 401
// may succeed without changing the request. API key upstream 404/405 responses
// mean that the selected account does not expose a model-discovery endpoint, so
// another account may still serve the manifest. Other upstream 4xx responses,
// except 429 and ChatGPT-backend 401, are intentionally not retried. A manifest
// 401 from the ChatGPT Codex backend reflects the selected OAuth account's
// upstream token rather than the client request (the client's own API key was
// already validated locally). Custom API key upstreams keep the no-failover 401
// behavior because their /models auth semantics are not authoritative for the
// account.
func IsRetryableCodexModelsManifestError(err error) bool {
@@ -1148,6 +1180,14 @@ func IsRetryableCodexModelsManifestError(err error) bool {
return errors.As(err, &upstreamErr) && upstreamErr.retryable
}
func isRetryableCodexModelsManifestStatus(statusCode int, useAPIKeyUpstream bool) bool {
return (useAPIKeyUpstream &&
(statusCode == http.StatusNotFound || statusCode == http.StatusMethodNotAllowed)) ||
(statusCode == http.StatusUnauthorized && !useAPIKeyUpstream) ||
statusCode == http.StatusTooManyRequests ||
(statusCode >= http.StatusInternalServerError && statusCode < 600)
}
func isRetryableCodexModelsManifestTransportError(err error) bool {
if err == nil || errors.Is(err, context.Canceled) {
return false
@@ -1575,9 +1615,7 @@ func (s *OpenAIGatewayService) fetchCodexModelsManifestUpstream(ctx context.Cont
statusCode: resp.StatusCode,
headers: resp.Header.Clone(),
body: body,
retryable: (resp.StatusCode == http.StatusUnauthorized && !request.useAPIKeyUpstream) ||
resp.StatusCode == http.StatusTooManyRequests ||
(resp.StatusCode >= http.StatusInternalServerError && resp.StatusCode < 600),
retryable: isRetryableCodexModelsManifestStatus(resp.StatusCode, request.useAPIKeyUpstream),
}
}
@@ -623,8 +623,8 @@ func TestBuildCodexModelsManifestForGroupUsesMappedTargetMetadataForCompositeAli
models := decodeCodexManifestModels(t, body)
require.Len(t, models, 1)
require.Equal(t, "reasoning-alias", models[0]["slug"])
require.Equal(t, "Claude Opus 4.8", models[0]["display_name"])
require.Equal(t, "Claude coding and reasoning model routed through Sub2API.", models[0]["description"])
require.Equal(t, "reasoning-alias", models[0]["display_name"])
require.Equal(t, "Custom model routed through Sub2API.", models[0]["description"])
require.Equal(t, []string{"low", "medium", "high", "xhigh", "max"}, effortsFromManifestModel(t, models[0]))
}
@@ -816,6 +816,29 @@ func TestBuildGroupConfiguredCodexModelsManifestUsesAdministratorConfiguration(t
t.Parallel()
const groupID int64 = 77
reasoning := true
arkAccount := Account{
Platform: PlatformOpenAI,
Type: AccountTypeAPIKey,
Credentials: map[string]any{
"model_mapping": map[string]any{
"glm-5.3": "glm-5.3",
"gpt-image-2": "gpt-image-2",
},
},
}
arkAccount.SetUpstreamModelMetadataSnapshot(UpstreamModelMetadataSnapshot{Models: map[string]UpstreamModelMetadata{
"glm-5.3": {
ID: "glm-5.3",
DisplayName: "GLM 5.3",
Description: "Ark coding model",
Reasoning: &reasoning,
DefaultReasoningLevel: "medium",
SupportedReasoningLevels: []string{"low", "medium", "high"},
InputModalities: []string{"text"},
ContextWindow: 1_000_000,
},
}})
svc := &OpenAIGatewayService{accountRepo: codexModelsVisibilityAccountRepo{
byGroup: map[int64][]Account{
groupID: {
@@ -823,15 +846,7 @@ func TestBuildGroupConfiguredCodexModelsManifestUsesAdministratorConfiguration(t
Platform: PlatformOpenAI,
Type: AccountTypeOAuth,
},
{
Platform: PlatformOpenAI,
Type: AccountTypeAPIKey,
Credentials: map[string]any{
"model_mapping": map[string]any{
"glm-5.3": "glm-5.3",
},
},
},
arkAccount,
},
},
}}
@@ -840,7 +855,14 @@ func TestBuildGroupConfiguredCodexModelsManifestUsesAdministratorConfiguration(t
manifest, configured, err := svc.BuildGroupConfiguredCodexModelsManifest(context.Background(), group, "")
require.NoError(t, err)
require.True(t, configured)
require.Equal(t, []string{"glm-5.3"}, codexManifestModelSlugs(t, manifest.Body))
models := decodeCodexManifestModels(t, manifest.Body)
require.Len(t, models, 1)
require.Equal(t, "glm-5.3", models[0]["slug"])
require.Equal(t, "GLM 5.3", models[0]["display_name"])
require.Equal(t, []string{"low", "medium", "high"}, effortsFromManifestModel(t, models[0]))
require.Equal(t, "medium", models[0]["default_reasoning_level"])
require.Equal(t, []any{"text"}, models[0]["input_modalities"])
require.EqualValues(t, 1_000_000, models[0]["context_window"])
require.Equal(t, codexModelsManifestBodyETag(manifest.Body), manifest.ETag)
notModified, configured, err := svc.BuildGroupConfiguredCodexModelsManifest(
@@ -1089,6 +1111,35 @@ func TestIsRetryableCodexModelsManifestTransportError(t *testing.T) {
}
}
func TestIsRetryableCodexModelsManifestStatus(t *testing.T) {
t.Parallel()
tests := []struct {
name string
statusCode int
useAPIKeyUpstream bool
retryable bool
}{
{name: "api key 404", statusCode: http.StatusNotFound, useAPIKeyUpstream: true, retryable: true},
{name: "api key 405", statusCode: http.StatusMethodNotAllowed, useAPIKeyUpstream: true, retryable: true},
{name: "oauth 404", statusCode: http.StatusNotFound},
{name: "oauth 405", statusCode: http.StatusMethodNotAllowed},
{name: "api key 401", statusCode: http.StatusUnauthorized, useAPIKeyUpstream: true},
{name: "oauth 401", statusCode: http.StatusUnauthorized, retryable: true},
{name: "api key 400", statusCode: http.StatusBadRequest, useAPIKeyUpstream: true},
{name: "api key 403", statusCode: http.StatusForbidden, useAPIKeyUpstream: true},
{name: "rate limited", statusCode: http.StatusTooManyRequests, retryable: true},
{name: "server error", statusCode: http.StatusServiceUnavailable, retryable: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tt.retryable, isRetryableCodexModelsManifestStatus(tt.statusCode, tt.useAPIKeyUpstream))
})
}
}
func newCodexModelsAPIKeyTestService(upstream HTTPUpstream) *OpenAIGatewayService {
return &OpenAIGatewayService{
cfg: &config.Config{Security: config.SecurityConfig{URLAllowlist: config.URLAllowlistConfig{