test(ingestion): /mcp's JWT validator and resource_metadata; narrow comments

PR #183 review (comment 15474):
- TestBuildMuxAnswers503ToAJWTOnMCPWhileTheIssuerIsUnresolved: building
  /mcp's middleware with a nil validator turns it red.
- TestBuildMuxMCP401CarriesResourceMetadataInWWWAuthenticate: building it
  with resourceMetadataURL "" turns it red.
- main.go, routes.go and the testMux comment now say exactly what is
  untested: main's restAuth fields and public map passed to buildMux, and
  ListenAndServe (hyperguild#182). Stale public-route list in the
  TestBuildMuxDoesNotWrapPublicRoutes comment corrected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

ingestion/cmd/server/jwt_validator_test.go

@@ -81,3 +81,31 @@ func TestBuildMuxAnswers503ToAJWTWhileTheIssuerIsUnresolved(t *testing.T) {
 	code, _ := do(t, mux, "GET", "/pending", "Bearer aaa.bbb.ccc")
 	assert.Equal(t, http.StatusServiceUnavailable, code)
 }
+
+// PR #183 review: /mcp shares REST's bearer, but only the static token was
+// asserted on it. With the issuer unresolved, a JWT on /mcp answers 503 too,
+// so /mcp's middleware was built with the JWT validator.
+func TestBuildMuxAnswers503ToAJWTOnMCPWhileTheIssuerIsUnresolved(t *testing.T) {
+	srv, _ := fakeIssuer(t, 1000)
+	v, err := buildJWTValidator(context.Background(), slog.New(slog.DiscardHandler), srv.URL, "brain")
+	require.NoError(t, err)
+	mux, err := buildMux(restAuth{token: routesTok, validator: v, realm: "brain"}, testHandler(t),
+		okHandler("mcp"), metrics.New(), nil)
+	require.NoError(t, err)
+	code, _ := do(t, mux, "POST", "/mcp", "Bearer aaa.bbb.ccc")
+	assert.Equal(t, http.StatusServiceUnavailable, code)
+}
+
+// PR #183 review: a 401 on /mcp names the protected-resource metadata in
+// WWW-Authenticate (what the claude.ai OAuth connector discovers from), so
+// /mcp's middleware was built with resourceMetadataURL.
+func TestBuildMuxMCP401CarriesResourceMetadataInWWWAuthenticate(t *testing.T) {
+	const rm = "https://brain.example/.well-known/oauth-protected-resource"
+	mux, err := buildMux(restAuth{token: routesTok, realm: "brain", resourceMetadataURL: rm}, testHandler(t),
+		okHandler("mcp"), metrics.New(), nil)
+	require.NoError(t, err)
+	rec := httptest.NewRecorder()
+	mux.ServeHTTP(rec, httptest.NewRequest(http.MethodPost, "/mcp", nil))
+	require.Equal(t, http.StatusUnauthorized, rec.Code)
+	assert.Contains(t, rec.Header().Get("WWW-Authenticate"), `resource_metadata="`+rm+`"`)
+}

ingestion/cmd/server/main.go

@@ -587,8 +587,10 @@ func main() {
 	//
 	// The whole router, /mcp's bearer included, is built in buildMux so its
 	// tests are tests of production wiring (PR #152 review, hyperguild#154).
-	// The REST API's credential is the same as /mcp's. ListenAndServe below
-	// must serve its result unchanged: that one line is the untested part.
+	// The REST API's credential is the same as /mcp's. Untested here: the
+	// restAuth fields and the public map this call passes in (e.g. a nil
+	// validator would drop JWT auth on /mcp and REST and the suite stays
+	// green), and ListenAndServe serving the result unchanged (hyperguild#182).
 	handler, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain",
 		resourceMetadataURL: resourceMetadataURL}, h, mcpSrv, metrics.New(), public)
 	if err != nil {

ingestion/cmd/server/routes.go

@@ -41,8 +41,9 @@ type restAuth struct {
 // wrapped in reg's middleware. Other public handlers are registered as given,
 // and only for patterns on the publicRoutes allowlist. It returns an
 // http.Handler, not the *ServeMux, so nothing can register a route on the
-// result. What stays untested is main's own line: a handler main built around
-// this one before ListenAndServe would not be seen by these tests.
+// result. What stays untested is main's call site: the restAuth fields and
+// the public map main passes in, and ListenAndServe serving this handler
+// unchanged (hyperguild#182). The tests build their own arguments.
 func buildMux(
 	auth restAuth, h *api.Handler, mcpHandler http.Handler, reg *metrics.Registry, public map[string]http.Handler,
 ) (http.Handler, error) {
@@ -92,8 +93,12 @@ func mountREST(mux *http.ServeMux, h *api.Handler, auth func(http.Handler) http.
 // labelRESTPattern sets the request's Pattern to the REST route it addresses
 // before auth runs, so the metrics middleware (which reads Pattern from the
 // same *http.Request) labels a refused request by its route rather than by
-// the "/" mount (hyperguild#155). A path no route matches keeps "/", so the
-// label set stays bounded by the registered routes.
+// the "/" mount (hyperguild#155). A refused request to a path no route
+// matches keeps "/", so for refused requests the label set stays bounded by
+// the registered routes. That bound does not hold for authenticated
+// requests: an unregistered path reaches the REST mux, which 404s with an
+// empty Pattern, and the metrics middleware falls back to the raw URL path
+// (hyperguild#185).
 func labelRESTPattern(rest *http.ServeMux, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if _, pattern := rest.Handler(r); pattern != "" {

ingestion/cmd/server/routes_observability_test.go

@@ -11,20 +11,41 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// wrongTokSentinel is a presented credential distinctive enough that any
+// occurrence in a log line is the raw token, not a coincidence.
+const wrongTokSentinel = "wrong-tok-sentinel-q7zx"
+
 // hyperguild#155: since #149 every REST route sits behind BearerMiddleware,
 // and chassis v0.2.0 refused without logging, so an unknown caller got
-// silent 401s. The chassis now writes an `mcp auth rejected` line.
-func TestBuildMuxLogsARESTAuthRejection(t *testing.T) {
+// silent 401s. The chassis now writes an `mcp auth rejected` line, and the
+// presented token appears nowhere in the log as a bare value (PR #181
+// review: asserting on the "Bearer " header form could never fail, since
+// the chassis logs fields, not headers).
+func TestBuildMuxRejectionLogLineOmitsTheBarePresentedToken(t *testing.T) {
 	var buf bytes.Buffer
 	prev := slog.Default()
 	slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
 	t.Cleanup(func() { slog.SetDefault(prev) })
 
-	code, _ := do(t, testMux(t), "POST", "/query", "Bearer wrong")
+	code, _ := do(t, testMux(t), "POST", "/query", "Bearer "+wrongTokSentinel)
 	require.Equal(t, http.StatusUnauthorized, code)
 	assert.Contains(t, buf.String(), "mcp auth rejected")
 	assert.Contains(t, buf.String(), "path=/query")
-	assert.NotContains(t, buf.String(), "Bearer wrong", "the credential itself is never logged")
+	assert.NotContains(t, buf.String(), wrongTokSentinel, "the rejected token is logged as a bare value")
+	assert.NotContains(t, buf.String(), routesTok, "the valid static token is logged")
+}
+
+// The `mcp auth accepted` line leaves out the valid static token.
+func TestBuildMuxAcceptedLogLineOmitsTheBareStaticToken(t *testing.T) {
+	var buf bytes.Buffer
+	prev := slog.Default()
+	slog.SetDefault(slog.New(slog.NewTextHandler(&buf, nil)))
+	t.Cleanup(func() { slog.SetDefault(prev) })
+
+	code, _ := do(t, testMux(t), "GET", "/pending", "Bearer "+routesTok)
+	require.Equal(t, http.StatusOK, code)
+	assert.Contains(t, buf.String(), "mcp auth accepted")
+	assert.NotContains(t, buf.String(), routesTok, "the accepted static token is logged as a bare value")
 }
 
 // An authenticated REST request is labelled with the REST mux's pattern:

ingestion/cmd/server/routes_test.go

@@ -22,11 +22,13 @@ func okHandler(body string) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) })
 }
 
-// testMux builds the router exactly as main does: buildMux with the full
-// public set main registers (stub handlers), a stub MCP server, and real
-// bearer config. hyperguild#149: the REST API is authenticated as a whole;
-// the PR #152 review showed a test of a COPY of the router let main.go
-// regress unseen.
+// testMux builds the router through buildMux, the function main calls, with
+// every allowlisted public pattern (stub handlers), a stub MCP server and a
+// static token only: no JWT validator and no resourceMetadataURL (the tests
+// in jwt_validator_test.go pass those). It does not use main's own restAuth
+// or public map (hyperguild#182). hyperguild#149: the REST API is
+// authenticated as a whole; the PR #152 review showed a test of a COPY of
+// the router let main.go regress unseen.
 func testMux(t *testing.T) http.Handler {
 	t.Helper()
 	mux, _ := testMuxAndRegistry(t)
@@ -87,8 +89,9 @@ func TestBuildMuxAuthenticatesAnUnregisteredPath(t *testing.T) {
 }
 
 // buildMux registers public handlers as given, without adding auth: each
-// public route owns its own (bearer on /mcp, HMAC on the webhook, client
-// secret on /oauth/token) or is meant to be open (/metrics, OAuth metadata).
+// public route owns its own (bearer/JWT on /capture, HMAC on the webhook,
+// client secret on /oauth/token) or is meant to be open (OAuth metadata).
+// /mcp and /metrics are not public routes: buildMux builds them itself.
 func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) {
 	r := testMux(t)
 	for p := range publicRoutes {

ingestion/internal/startupretry/startupretry.go

@@ -1,75 +0,0 @@
-// Package startupretry retries a startup dependency that may not be serving
-// yet, instead of exiting on its first failure.
-//
-// It exists for the cold-boot ordering race (#83): on a cluster restart the
-// ingestion pod can reach OIDC discovery before dex is answering, gets a 502
-// from ingress, and dies — after having already connected postgres, backfilled
-// the graph and ingested session batches, all of which the restart redoes.
-// Nothing is misconfigured in that scenario; the pod simply started first.
-//
-// The budget is bounded on purpose. Retrying forever would turn a genuine
-// misconfiguration (wrong issuer URL, bad audience) into a process that looks
-// healthy and never serves, which is harder to diagnose than a crash loop.
-package startupretry
-
-import (
-	"context"
-	"fmt"
-	"time"
-)
-
-// Policy bounds the retry: at most Attempts calls, with the delay doubling
-// from Base and capped at Max.
-type Policy struct {
-	Attempts int           // total calls, including the first; < 1 means 1
-	Base     time.Duration // delay before the second attempt
-	Max      time.Duration // ceiling for the doubling delay
-}
-
-// Do calls fn until it succeeds, the attempt budget is spent, or ctx ends.
-//
-// The returned error wraps the last failure, so callers keep the diagnosis
-// they would have had without the retry.
-func Do[T any](ctx context.Context, p Policy, fn func(context.Context) (T, error)) (T, error) {
-	var zero T
-
-	attempts := p.Attempts
-	if attempts < 1 {
-		attempts = 1
-	}
-
-	delay := p.Base
-	var lastErr error
-
-	for i := range attempts {
-		if err := ctx.Err(); err != nil {
-			return zero, fmt.Errorf("startupretry: %w", err)
-		}
-
-		v, err := fn(ctx)
-		if err == nil {
-			return v, nil
-		}
-		lastErr = err
-
-		if i == attempts-1 {
-			break
-		}
-
-		if delay <= 0 {
-			delay = time.Second
-		}
-		if p.Max > 0 && delay > p.Max {
-			delay = p.Max
-		}
-
-		select {
-		case <-ctx.Done():
-			return zero, fmt.Errorf("startupretry: %w", ctx.Err())
-		case <-time.After(delay):
-		}
-		delay *= 2
-	}
-
-	return zero, fmt.Errorf("startupretry: gave up after %d attempts: %w", attempts, lastErr)
-}

ingestion/internal/startupretry/startupretry_test.go

@@ -1,142 +0,0 @@
-// ingestion/internal/startupretry/startupretry_test.go
-package startupretry
-
-import (
-	"context"
-	"errors"
-	"testing"
-	"time"
-)
-
-func fastPolicy(attempts int) Policy {
-	return Policy{Attempts: attempts, Base: time.Millisecond, Max: 2 * time.Millisecond}
-}
-
-func TestDo_ReturnsImmediatelyOnFirstSuccess(t *testing.T) {
-	calls := 0
-	got, err := Do(context.Background(), fastPolicy(5), func(context.Context) (string, error) {
-		calls++
-		return "ready", nil
-	})
-	if err != nil {
-		t.Fatalf("Do: %v", err)
-	}
-	if got != "ready" {
-		t.Errorf("got %q, want %q", got, "ready")
-	}
-	if calls != 1 {
-		t.Errorf("calls = %d, want 1", calls)
-	}
-}
-
-func TestDo_SurvivesTransientFailuresThenSucceeds(t *testing.T) {
-	// The observed cold-boot race: dex is not serving yet, ingress answers
-	// 502 for the first few attempts, then discovery works.
-	calls := 0
-	got, err := Do(context.Background(), fastPolicy(5), func(context.Context) (string, error) {
-		calls++
-		if calls < 4 {
-			return "", errors.New("oidc discovery: status 502")
-		}
-		return "ready", nil
-	})
-	if err != nil {
-		t.Fatalf("Do returned %v — a transient 502 must not be fatal", err)
-	}
-	if got != "ready" {
-		t.Errorf("got %q, want %q", got, "ready")
-	}
-	if calls != 4 {
-		t.Errorf("calls = %d, want 4", calls)
-	}
-}
-
-func TestDo_GivesUpAfterTheAttemptBudgetAndReportsTheLastError(t *testing.T) {
-	// A genuinely broken issuer must still fail — retrying forever would turn
-	// a config error into a silent hang, which is worse than crash-looping.
-	last := errors.New("oidc discovery: status 404")
-	calls := 0
-
-	_, err := Do(context.Background(), fastPolicy(3), func(context.Context) (string, error) {
-		calls++
-		return "", last
-	})
-
-	if !errors.Is(err, last) {
-		t.Fatalf("err = %v, want it to wrap the last failure", err)
-	}
-	if calls != 3 {
-		t.Errorf("calls = %d, want 3 (the attempt budget)", calls)
-	}
-}
-
-func TestDo_StopsPromptlyWhenTheContextIsCancelled(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	calls := 0
-
-	done := make(chan struct{})
-	var err error
-	go func() {
-		defer close(done)
-		_, err = Do(ctx, Policy{Attempts: 100, Base: 50 * time.Millisecond, Max: time.Second},
-			func(context.Context) (string, error) {
-				calls++
-				if calls == 1 {
-					cancel()
-				}
-				return "", errors.New("still down")
-			})
-	}()
-
-	select {
-	case <-done:
-	case <-time.After(2 * time.Second):
-		t.Fatal("Do did not return after context cancellation")
-	}
-	if !errors.Is(err, context.Canceled) {
-		t.Errorf("err = %v, want context.Canceled", err)
-	}
-}
-
-func TestDo_BacksOffAndIsCappedByMax(t *testing.T) {
-	var delays []time.Duration
-	p := Policy{Attempts: 5, Base: 10 * time.Millisecond, Max: 25 * time.Millisecond}
-
-	last := time.Now()
-	_, _ = Do(context.Background(), p, func(context.Context) (string, error) {
-		now := time.Now()
-		delays = append(delays, now.Sub(last))
-		last = now
-		return "", errors.New("down")
-	})
-
-	// delays[0] is the first call (no wait); the rest are the sleeps.
-	if len(delays) != 5 {
-		t.Fatalf("attempts = %d, want 5", len(delays))
-	}
-	if delays[1] >= delays[2] {
-		t.Errorf("backoff is not growing: %v then %v", delays[1], delays[2])
-	}
-	for i, d := range delays[1:] {
-		if d > 2*p.Max {
-			t.Errorf("delay %d = %v, exceeds the Max cap %v with slack", i+1, d, p.Max)
-		}
-	}
-}
-
-func TestPolicy_ZeroValueStillMakesOneAttempt(t *testing.T) {
-	// A caller that forgets to configure the policy must not silently get a
-	// no-op that returns a nil validator and a nil error.
-	calls := 0
-	_, err := Do(context.Background(), Policy{}, func(context.Context) (string, error) {
-		calls++
-		return "", errors.New("down")
-	})
-
-	if calls != 1 {
-		t.Errorf("calls = %d, want 1", calls)
-	}
-	if err == nil {
-		t.Error("err = nil, want the failure surfaced")
-	}
-}