The #35 data gate could never fill: a real review call routed cleanly to qwen36 but /pass-rate stayed total:0 under every key. Root cause was three independent defects in the session_log path, each alone fatal: - A: a successful routed call logged final_status "skip", never "pass". /pass-rate computes pass/(pass+fail) and skips count as neither, so the >=0.90 gate was mathematically unreachable. Success now logs "pass". - B: every record was written under skill "_routing", so /pass-rate?skill= review|debug (what #35 measures) always read zero. Now uses the real e.Skill; routing decisions stay groupable via session_id "_routing". - C: the session_log POST to the bearer-gated ingestion /mcp carried no Authorization header → silent 401, swallowed by best-effort logging (the documented mcpclient-empty-token-silent-401 footgun). Logger now takes a token (BRAIN_MCP_TOKEN) and sets the bearer when non-empty. Tests rewritten to assert correct behavior (they had encoded the bugs: "skip" on success, "_routing" skill). New test covers the auth header and the empty-token path. Infra (BRAIN_MCP_TOKEN ExternalSecret + env on the routing deployment) and redeploy follow separately. Refs #73, #35. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -53,7 +53,7 @@ func main() {
|
||||
|
||||
router := &routing.Router{
|
||||
Fetcher: routing.NewFetcher(cfg.BrainURL, "7d", time.Duration(cfg.PassRateTTLSeconds)*time.Second),
|
||||
Logger: routing.NewLogger(cfg.BrainURL),
|
||||
Logger: routing.NewLogger(cfg.BrainURL, cfg.BrainMCPToken),
|
||||
Policy: routing.Policy{Floor: cfg.RouteLocalFloor, Ceil: cfg.RouteLocalCeil},
|
||||
FastModel: cfg.FastModel,
|
||||
ThinkingModel: cfg.ThinkingModel,
|
||||
|
||||
@@ -14,6 +14,7 @@ type RoutingConfig struct {
|
||||
LiteLLMBaseURL string // LITELLM_BASE_URL, default https://llm-api.d-ma.be
|
||||
LiteLLMAPIKey string // LITELLM_API_KEY
|
||||
BrainURL string // BRAIN_URL, default http://ingestion.supervisor:3300
|
||||
BrainMCPToken string // BRAIN_MCP_TOKEN, bearer for the auth-gated ingestion /mcp (session_log)
|
||||
FastModel string // HYPERGUILD_FAST_MODEL, default koala/qwen35-9b-fast
|
||||
ThinkingModel string // HYPERGUILD_THINKING_MODEL, default iguana/gemma4-26b
|
||||
// RouteLocalFloor and RouteLocalCeil intentionally invert the usual
|
||||
@@ -44,6 +45,7 @@ func LoadRouting() (RoutingConfig, error) {
|
||||
LiteLLMBaseURL: envOr("LITELLM_BASE_URL", "https://llm-api.d-ma.be"),
|
||||
LiteLLMAPIKey: os.Getenv("LITELLM_API_KEY"),
|
||||
BrainURL: envOr("BRAIN_URL", "http://ingestion.supervisor:3300"),
|
||||
BrainMCPToken: os.Getenv("BRAIN_MCP_TOKEN"),
|
||||
FastModel: envOr("HYPERGUILD_FAST_MODEL", "koala/qwen35-9b-fast"),
|
||||
ThinkingModel: envOr("HYPERGUILD_THINKING_MODEL", "iguana/gemma4-26b"),
|
||||
}
|
||||
|
||||
+19
-6
@@ -17,19 +17,24 @@ type LogEntry struct {
|
||||
Message string // free-form, e.g. "model=qwen35, pass_rate=0.94"
|
||||
ProjectRoot string
|
||||
DurationMs int64
|
||||
Failed bool // true → final_status: "fail"; false → "skip"
|
||||
Failed bool // true → final_status: "fail"; false → "pass"
|
||||
}
|
||||
|
||||
// Logger posts session_log entries to a brain MCP at BrainURL + /mcp.
|
||||
type Logger struct {
|
||||
BrainURL string
|
||||
Token string // bearer for the (auth-gated) ingestion /mcp; empty = no header
|
||||
HTTP *http.Client
|
||||
}
|
||||
|
||||
// NewLogger creates a Logger with a 2-second HTTP timeout.
|
||||
func NewLogger(brainURL string) *Logger {
|
||||
// NewLogger creates a Logger with a 2-second HTTP timeout. token authenticates
|
||||
// to the bearer-gated ingestion /mcp; an empty token sends no Authorization
|
||||
// header (and silently 401s against a gated server — see brain
|
||||
// mcpclient-empty-token-silent-401-envfrom-missing-key).
|
||||
func NewLogger(brainURL, token string) *Logger {
|
||||
return &Logger{
|
||||
BrainURL: brainURL,
|
||||
Token: token,
|
||||
HTTP: &http.Client{Timeout: 2 * time.Second},
|
||||
}
|
||||
}
|
||||
@@ -37,7 +42,10 @@ func NewLogger(brainURL string) *Logger {
|
||||
// LogDecision posts a session_log MCP call. Errors are returned but the caller
|
||||
// MUST NOT block real work on them — logging is best-effort.
|
||||
func (l *Logger) LogDecision(ctx context.Context, e LogEntry) error {
|
||||
status := "skip"
|
||||
// A completed routed call is a pass (liveness); only an execution error is a
|
||||
// fail. There is no "skip" for routing — the prior default-to-"skip" meant a
|
||||
// successful call never counted toward pass_rate, so the gate was unreachable.
|
||||
status := "pass"
|
||||
if e.Failed {
|
||||
status = "fail"
|
||||
}
|
||||
@@ -48,8 +56,10 @@ func (l *Logger) LogDecision(ctx context.Context, e LogEntry) error {
|
||||
"params": map[string]any{
|
||||
"name": "session_log",
|
||||
"arguments": map[string]any{
|
||||
"session_id": e.SessionID,
|
||||
"skill": "_routing",
|
||||
"session_id": e.SessionID,
|
||||
// The real skill, so /pass-rate?skill=review|debug sees these
|
||||
// records; routing decisions stay groupable via session_id "_routing".
|
||||
"skill": e.Skill,
|
||||
"phase": "decide",
|
||||
"final_status": status,
|
||||
"message": fmt.Sprintf("%s: %s — %s", e.Skill, e.Decision, e.Message),
|
||||
@@ -67,6 +77,9 @@ func (l *Logger) LogDecision(ctx context.Context, e LogEntry) error {
|
||||
return fmt.Errorf("log: build request: %w", err)
|
||||
}
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
if l.Token != "" {
|
||||
req.Header.Set("Authorization", "Bearer "+l.Token)
|
||||
}
|
||||
resp, err := l.HTTP.Do(req)
|
||||
if err != nil {
|
||||
return fmt.Errorf("log: request: %w", err)
|
||||
|
||||
@@ -15,36 +15,44 @@ import (
|
||||
|
||||
func TestLoggerLogDecision(t *testing.T) {
|
||||
var captured map[string]any
|
||||
var authHeader string
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
assert.Equal(t, http.MethodPost, r.Method)
|
||||
assert.Equal(t, "/mcp", r.URL.Path)
|
||||
authHeader = r.Header.Get("Authorization")
|
||||
body, _ := io.ReadAll(r.Body)
|
||||
require.NoError(t, json.Unmarshal(body, &captured))
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"jsonrpc": "2.0", "id": 1, "result": map[string]any{"content": []map[string]any{{"type": "text", "text": "ok"}}}})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
l := routing.NewLogger(srv.URL)
|
||||
l := routing.NewLogger(srv.URL, "test-token")
|
||||
err := l.LogDecision(context.Background(), routing.LogEntry{
|
||||
SessionID: "sess-1",
|
||||
Skill: "review",
|
||||
Decision: "local",
|
||||
Message: "model=qwen35, pass_rate=0.94",
|
||||
Message: "model=qwen36, pass_rate=0.94",
|
||||
ProjectRoot: "/home/x/proj",
|
||||
DurationMs: 1234,
|
||||
Failed: false,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Bug C fix: the POST authenticates to the bearer-gated ingestion /mcp.
|
||||
assert.Equal(t, "Bearer test-token", authHeader)
|
||||
|
||||
params := captured["params"].(map[string]any)
|
||||
assert.Equal(t, "tools/call", captured["method"])
|
||||
assert.Equal(t, "session_log", params["name"])
|
||||
|
||||
args := params["arguments"].(map[string]any)
|
||||
assert.Equal(t, "_routing", args["skill"])
|
||||
// Bug B fix: the record carries the real skill so /pass-rate?skill=review sees it.
|
||||
assert.Equal(t, "review", args["skill"])
|
||||
assert.Equal(t, "decide", args["phase"])
|
||||
assert.Equal(t, "skip", args["final_status"])
|
||||
// Bug A fix: a successful routed call logs "pass", not "skip".
|
||||
assert.Equal(t, "pass", args["final_status"])
|
||||
assert.Contains(t, args["message"].(string), "review: local")
|
||||
// session grouping is preserved via session_id.
|
||||
assert.Equal(t, "sess-1", args["session_id"])
|
||||
assert.Equal(t, "/home/x/proj", args["project_root"])
|
||||
assert.Equal(t, float64(1234), args["duration_ms"])
|
||||
@@ -59,23 +67,40 @@ func TestLoggerLogFailure(t *testing.T) {
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
l := routing.NewLogger(srv.URL)
|
||||
l := routing.NewLogger(srv.URL, "test-token")
|
||||
err := l.LogDecision(context.Background(), routing.LogEntry{
|
||||
SessionID: "s", Skill: "debug", Decision: "local", Message: "litellm down", Failed: true,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
args := captured["params"].(map[string]any)["arguments"].(map[string]any)
|
||||
assert.Equal(t, "debug", args["skill"])
|
||||
assert.Equal(t, "fail", args["final_status"])
|
||||
}
|
||||
|
||||
func TestLoggerOmitsAuthWhenTokenEmpty(t *testing.T) {
|
||||
var authHeader string
|
||||
hasAuth := false
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
authHeader = r.Header.Get("Authorization")
|
||||
_, hasAuth = r.Header["Authorization"]
|
||||
_ = json.NewEncoder(w).Encode(map[string]any{"jsonrpc": "2.0", "id": 1, "result": map[string]any{}})
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
l := routing.NewLogger(srv.URL, "")
|
||||
require.NoError(t, l.LogDecision(context.Background(), routing.LogEntry{Skill: "review", SessionID: "_routing", Decision: "local"}))
|
||||
assert.False(t, hasAuth, "no Authorization header should be set when token is empty")
|
||||
assert.Equal(t, "", authHeader)
|
||||
}
|
||||
|
||||
func TestLoggerSurfacesUpstreamError(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
http.Error(w, "down", http.StatusBadGateway)
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
l := routing.NewLogger(srv.URL)
|
||||
l := routing.NewLogger(srv.URL, "test-token")
|
||||
err := l.LogDecision(context.Background(), routing.LogEntry{Skill: "x", SessionID: "y", Decision: "local"})
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
@@ -50,7 +50,7 @@ func newRouter(t *testing.T, llm *fakeLLM, passRate float64) (*routing.Router, *
|
||||
|
||||
r := &routing.Router{
|
||||
Fetcher: routing.NewFetcher(brain.URL, "7d", time.Minute),
|
||||
Logger: routing.NewLogger(brain.URL),
|
||||
Logger: routing.NewLogger(brain.URL, ""),
|
||||
Policy: routing.Policy{Floor: 0.9, Ceil: 0.7},
|
||||
FastModel: "koala/qwen35-9b-fast",
|
||||
ThinkingModel: "iguana/gemma4-26b",
|
||||
@@ -117,7 +117,7 @@ func TestRouterDefaultsToFastWhenBrainUnreachable(t *testing.T) {
|
||||
llm := &fakeLLM{resp: "ok"}
|
||||
r := &routing.Router{
|
||||
Fetcher: routing.NewFetcher(brain.URL, "7d", time.Minute),
|
||||
Logger: routing.NewLogger(brain.URL),
|
||||
Logger: routing.NewLogger(brain.URL, ""),
|
||||
Policy: routing.Policy{Floor: 0.9, Ceil: 0.7},
|
||||
FastModel: "koala/qwen35-9b-fast",
|
||||
ThinkingModel: "iguana/gemma4-26b",
|
||||
|
||||
Reference in New Issue
Block a user