feat(rotation): Phase 5 drift detection in agent.Client (#72) (#132)

When the dashboard signs an outbound request with kid X but the agent
matches kid Y instead (because rotation propagated on one side but
not the other), drift detection logs the disagreement so an operator
can resync. Closes the observability loop on the install→use→remove
cycle: Phase 2 added the request header, the agent's auth middleware
already echoes the matched kid, and this commit wires the
dashboard-side comparison.

What's here
-----------

`agent.HeaderKID = "X-Gearbox-Kid"` is the shared constant the
dashboard sends and the agent echoes back. The agent's middleware
sets it on every authenticated response (see Phase 1).

`agent.Client`

- `SetDriftHandler(DriftHandler)` installs an optional callback
  invoked when `resp.Header.Get(HeaderKID)` differs from the kid the
  client was built with. Reads c.onDrift at RoundTrip time, so the
  handler can be installed AFTER construction (typical for long-
  lived per-box clients held by the dashboard).
- Transport wrap: `kidObservingTransport` sits between the http
  client and the underlying TLS transport, calling `c.checkDrift` on
  every successful response. One central point of inspection — no
  invasive edits to every `doRequest*` method.
- `LogDriftHandler(logger, boxID)` builds a ready-to-use DriftHandler
  that emits a structured warn-level log. Lowest-friction wiring for
  the long-lived clients held in the WebSocketManager.

Test fix
--------

`TestClientTimeout` was a flaky pre-existing test whose substring
check was case-sensitive — Go's net/http error message capitalises
"Timeout" sometimes and emits "context deadline exceeded" other
times. Fixed by lower-casing the error message before substring
matching. Verified stable across 5 runs.

Tests
-----

`client_drift_test.go` exercises the four corners of the matrix:

- Drift handler fires when kid mismatches.
- Doesn't fire when kid matches.
- Doesn't fire when the agent omits the header (older agents).
- Doesn't fire when the dashboard client has no kid.

Not yet wired into production code
----------------------------------

Adding `agent.LogDriftHandler` is the small API surface; deciding
*where* to call SetDriftHandler is a separate design choice
(WebSocketManager? capability poller? every short-lived
handler.agentClient()?). Deferring that integration so this PR
stays focused on the observability primitive itself. A follow-up
can install LogDriftHandler at every site that constructs a kid-
bearing client.

Refs: Phase 5 of the implementation plan posted to #72.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: sarg3nt

gearbox/internal/framework/agent/client.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"log/slog"
 	"net"
 	"net/http"
 	"net/url"
@@ -33,17 +34,43 @@ type Client struct {
 	baseURL    string
 	apiKey     string
 	kid        string // optional; when set, sent as X-Gearbox-Kid header
+	onDrift    DriftHandler
 	httpClient *http.Client
 }
 
 // HeaderKID is the request/response header name carrying the keyring
 // entry id. The agent's middleware echoes the matched kid on every
 // authenticated response (see middleware.ResponseHeaderKID); the
 // dashboard sends this kid on outbound requests so the agent + audit
-// log can correlate keys. Phase 5 (drift detection) compares the
+// log can correlate keys. Drift detection (Phase 5) compares the
 // request-time kid with the echoed response kid.
 const HeaderKID = "X-Gearbox-Kid"
 
+// DriftHandler is invoked when the agent's echoed kid differs from
+// the kid the client sent. Implementations typically log + surface a
+// "rotation drift" banner; the default is to do nothing (set via
+// SetDriftHandler).
+//
+// expected = what the client sent on the request
+// actual   = what the agent echoed back on the response
+//
+// The handler is called on the request goroutine; cheap operations
+// only (logging is fine, network calls are not).
+type DriftHandler func(expected, actual string)
+
+// LogDriftHandler returns a DriftHandler that emits a structured
+// warn-level log line — the lowest-friction wiring for a long-lived
+// agent.Client. The "box_id" tag lets the operator filter the journal
+// to a single box.
+func LogDriftHandler(logger *slog.Logger, boxID int64) DriftHandler {
+	return func(expected, actual string) {
+		logger.Warn("rotation drift: agent matched a different kid than expected",
+			"box_id", boxID,
+			"expected_kid", expected,
+			"actual_kid", actual)
+	}
+}
+
 // NewClient creates a new HAProxy Agent API client.
 func NewClient(baseURL, apiKey string) *Client {
 	return NewClientWithTimeout(baseURL, apiKey, DefaultTimeout)
@@ -74,6 +101,29 @@ func (c *Client) WithKID(kid string) *Client {
 // paths.
 func (c *Client) KID() string { return c.kid }
 
+// SetDriftHandler installs a callback invoked when the agent's
+// echoed X-Gearbox-Kid response header differs from the kid the
+// client sent. Nil disables drift detection (the default).
+//
+// Use this on long-lived Client instances cached per box; the rotator
+// builds short-lived clients per call and wouldn't benefit from
+// installing a handler.
+func (c *Client) SetDriftHandler(h DriftHandler) { c.onDrift = h }
+
+// checkDrift inspects resp's X-Gearbox-Kid header against the kid the
+// client sent. Called from each doRequest* path on success. No-op when
+// the client has no kid or no handler is installed.
+func (c *Client) checkDrift(resp *http.Response) {
+	if resp == nil || c.kid == "" || c.onDrift == nil {
+		return
+	}
+	actual := resp.Header.Get(HeaderKID)
+	if actual == "" || actual == c.kid {
+		return
+	}
+	c.onDrift(c.kid, actual)
+}
+
 // setAuthHeaders applies the standard auth + Accept headers and, when
 // the client was built with a kid, the X-Gearbox-Kid request header.
 // Callers must call this BEFORE setting Content-Type so their override
@@ -135,14 +185,37 @@ func NewClientWithTimeout(baseURL, apiKey string, timeout time.Duration) *Client
 		},
 	}
 
-	return &Client{
+	c := &Client{
 		baseURL: baseURL,
 		apiKey:  apiKey,
-		httpClient: &http.Client{
-			Timeout:   timeout,
-			Transport: transport,
-		},
 	}
+	c.httpClient = &http.Client{
+		Timeout: timeout,
+		// Wrap the base transport so every response is inspected for the
+		// X-Gearbox-Kid header against c.kid. The transport reads c.kid
+		// and c.onDrift at RoundTrip time, so SetDriftHandler is reflected
+		// immediately without rebuilding the client.
+		Transport: &kidObservingTransport{base: transport, c: c},
+	}
+	return c
+}
+
+// kidObservingTransport wraps an http.RoundTripper and invokes the
+// owning Client's checkDrift on every successful response. Implements
+// the drift-detection half of Phase 5 (issue #72) — the dashboard
+// learns immediately when an agent's matched kid disagrees with the
+// kid the dashboard sent, signalling a partial rotation.
+type kidObservingTransport struct {
+	base http.RoundTripper
+	c    *Client
+}
+
+func (t *kidObservingTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	resp, err := t.base.RoundTrip(req)
+	if err == nil {
+		t.c.checkDrift(resp)
+	}
+	return resp, err
 }
 
 // BuildTLSConfig is the exported alias for createTLSConfig — used by

gearbox/internal/framework/agent/client_drift_test.go

@@ -0,0 +1,98 @@
+package agent
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"sync/atomic"
+	"testing"
+)
+
+func TestDriftHandler_FiresOnKIDMismatch(t *testing.T) {
+	// Mock agent that always echoes a hard-coded kid in the response.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set(HeaderKID, "actual-kid")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("{}"))
+	}))
+	defer srv.Close()
+
+	c := NewClientWithKID(srv.URL, "ignored", "expected-kid")
+	var fired atomic.Bool
+	var seenExpected, seenActual string
+	c.SetDriftHandler(func(expected, actual string) {
+		fired.Store(true)
+		seenExpected, seenActual = expected, actual
+	})
+
+	if _, err := c.doRequest("GET", "/anything", url.Values{}); err != nil {
+		t.Fatalf("doRequest: %v", err)
+	}
+	if !fired.Load() {
+		t.Errorf("drift handler not invoked")
+	}
+	if seenExpected != "expected-kid" || seenActual != "actual-kid" {
+		t.Errorf("drift handler args: expected=%q actual=%q", seenExpected, seenActual)
+	}
+}
+
+func TestDriftHandler_DoesNotFireOnMatch(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set(HeaderKID, "matched")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("{}"))
+	}))
+	defer srv.Close()
+
+	c := NewClientWithKID(srv.URL, "ignored", "matched")
+	var fired atomic.Bool
+	c.SetDriftHandler(func(_, _ string) { fired.Store(true) })
+
+	if _, err := c.doRequest("GET", "/anything", url.Values{}); err != nil {
+		t.Fatalf("doRequest: %v", err)
+	}
+	if fired.Load() {
+		t.Errorf("drift handler incorrectly invoked on matching kid")
+	}
+}
+
+func TestDriftHandler_DoesNotFireWhenAgentDoesNotEchoKID(t *testing.T) {
+	// Older agents won't set X-Gearbox-Kid. The drift handler should
+	// stay silent rather than flagging every response as "drift".
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("{}"))
+	}))
+	defer srv.Close()
+
+	c := NewClientWithKID(srv.URL, "ignored", "expected-kid")
+	var fired atomic.Bool
+	c.SetDriftHandler(func(_, _ string) { fired.Store(true) })
+
+	if _, err := c.doRequest("GET", "/anything", url.Values{}); err != nil {
+		t.Fatalf("doRequest: %v", err)
+	}
+	if fired.Load() {
+		t.Errorf("drift handler incorrectly invoked when agent sent no kid header")
+	}
+}
+
+func TestDriftHandler_DoesNotFireWhenClientHasNoKID(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set(HeaderKID, "something")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte("{}"))
+	}))
+	defer srv.Close()
+
+	c := NewClient(srv.URL, "ignored") // no kid
+	var fired atomic.Bool
+	c.SetDriftHandler(func(_, _ string) { fired.Store(true) })
+
+	if _, err := c.doRequest("GET", "/anything", url.Values{}); err != nil {
+		t.Fatalf("doRequest: %v", err)
+	}
+	if fired.Load() {
+		t.Errorf("drift handler invoked when client has no kid; should no-op")
+	}
+}

gearbox/internal/framework/agent/client_test.go

@@ -579,7 +579,11 @@ func TestClientTimeout(t *testing.T) {
 		t.Fatal("expected timeout error")
 	}
 
-	if !strings.Contains(err.Error(), "context deadline exceeded") && !strings.Contains(err.Error(), "timeout") {
+	// The error message format varies by Go version + timing: sometimes
+	// "context deadline exceeded", sometimes "Client.Timeout exceeded
+	// while awaiting headers" (capital T). Match either by lower-casing.
+	msg := strings.ToLower(err.Error())
+	if !strings.Contains(msg, "context deadline exceeded") && !strings.Contains(msg, "timeout") {
 		t.Errorf("expected timeout error, got %v", err)
 	}
 }