mathiasandClaude Opus 4.8
da9bdc4cbb
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>
107 lines
3.8 KiB
Go
107 lines
3.8 KiB
Go
package routing_test
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"io"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/mathiasbq/supervisor/internal/routing"
|
|
"github.com/stretchr/testify/assert"
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
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, "test-token")
|
|
err := l.LogDecision(context.Background(), routing.LogEntry{
|
|
SessionID: "sess-1",
|
|
Skill: "review",
|
|
Decision: "local",
|
|
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)
|
|
// 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"])
|
|
// 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"])
|
|
}
|
|
|
|
func TestLoggerLogFailure(t *testing.T) {
|
|
var captured map[string]any
|
|
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
body, _ := io.ReadAll(r.Body)
|
|
_ = json.Unmarshal(body, &captured)
|
|
_ = json.NewEncoder(w).Encode(map[string]any{"jsonrpc": "2.0", "id": 1, "result": map[string]any{}})
|
|
}))
|
|
defer srv.Close()
|
|
|
|
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, "test-token")
|
|
err := l.LogDecision(context.Background(), routing.LogEntry{Skill: "x", SessionID: "y", Decision: "local"})
|
|
require.Error(t, err)
|
|
}
|