diff --git a/internal/gateway/chat.go b/internal/gateway/chat.go index 5a70914..fa5ceee 100644 --- a/internal/gateway/chat.go +++ b/internal/gateway/chat.go @@ -256,7 +256,17 @@ func (g *Gateway) checkQuota(ctx context.Context, model string) *quotaRejection } return nil } - return "aRejection{msg: fmt.Sprintf("model %q is not allowed for this key", model)} +// A scope that simply does not list the model is a different problem from +// a quota being exhausted, and the message has to say which: telling an +// operator "not allowed for this key" when they configured a quota sends +// them to the wrong setting. AUTO is checked here before the chain is even +// consulted, so this is the message that surfaces for AUTO. +if isAuto(model) { +return "aRejection{msg: fmt.Sprintf( +"model %q is not in this key's model scope, so it cannot use AUTO; "+ +"add %q to the key's models or remove the scope restriction", model, model)} +} +return "aRejection{msg: fmt.Sprintf("model %q is not allowed for this key", model)} } // quotaWindowSuffix describes a quota's reset window for an error message, so @@ -452,19 +462,11 @@ func (g *Gateway) handleChat(w http.ResponseWriter, r *http.Request) { return } } - // A key whose model scope does not include AUTO cannot use AUTO at all, - // even when it has its own AUTO chain configured. That combination is - // easy to set up by accident and produced a misleading error: the request - // fell through to the generic "model %q is not configured" below, which - // claims AUTO does not exist — it does, this key just may not use it. Name - // the actual reason so the admin can fix the scope. - if isAuto(model) { - if allow := g.allowedModels(r.Context()); allow != nil && !g.hasScopeModel(allow, model) { - writeError(w, http.StatusForbidden, "model_not_allowed", - fmt.Sprintf("model %q is not in this key's model scope, so it cannot use AUTO; add %q to the key's models or remove the scope restriction", model, model)) - return - } - } + // Note: a key whose scope excludes AUTO is rejected earlier, by + // checkQuota(ctx, "AUTO") above, which produces the same message. That + // ordering matters — checkQuota runs before the chain is consulted, so it + // is the check an AUTO request actually hits. Duplicating it here would be + // dead code. cands, effective := g.resolveCands(r.Context(), &req) if len(cands) == 0 { writeError(w, http.StatusNotFound, "model_not_found", fmt.Sprintf("model %q is not configured", model)) diff --git a/internal/gateway/ui_key_auto_test.go b/internal/gateway/ui_key_auto_test.go index c83c70e..851a6b1 100644 --- a/internal/gateway/ui_key_auto_test.go +++ b/internal/gateway/ui_key_auto_test.go @@ -1,6 +1,7 @@ package gateway import ( + "os" "strings" "testing" ) @@ -362,3 +363,33 @@ func TestUIKeyAutoWarnsOnScopeConflict(t *testing.T) { t.Fatalf("i18n key kAutoChainScopeWarn appears %d time(s), want zh+en", n) } } + +// The scope conflict for AUTO must be reported by checkQuota, because that is +// the check an AUTO request actually reaches first (it runs before the chain is +// consulted). Putting the message on the later path made it dead code — the +// request was rejected one check earlier with the generic "not allowed for this +// key" wording, which sends an operator looking for a quota that isn't the +// problem. +func TestAutoScopeConflictMessageComesFromCheckQuota(t *testing.T) { + b, err := os.ReadFile("chat.go") + if err != nil { + t.Fatalf("read chat.go: %v", err) + } + src := string(b) + i := strings.Index(src, "func (g *Gateway) checkQuota(") + if i < 0 { + t.Fatal("checkQuota not found") + } + // Body = up to the next top-level func. + rest := src[i:] + if j := strings.Index(rest[1:], "\nfunc "); j > 0 { + rest = rest[:j+1] + } + if !strings.Contains(rest, "isAuto(model)") { + t.Fatal("checkQuota must special-case AUTO: a scope that omits AUTO is " + + "rejected there before anything else looks at the chain") + } + if !strings.Contains(rest, "not in this key's model scope") { + t.Fatal("checkQuota's AUTO branch must name the scope as the cause") + } +}