From 80a47d850e718502431bd03e3ec1279d025b31ab Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:59:34 +0100 Subject: [PATCH] fix(multiplayer): wire the ResultSubmitter adapter; pin the deeper gap it exposed Investigating the fleet.yaml wiring task found something more fundamental than a manifest problem: cmd/control-plane/main.go never wires WorkloadVerify, and store.PostgresResults (a ready-made, already-correct ResultSubmitter adapter matching the interface exactly) was referenced from nowhere outside its own file -- not even a test. Both server-authenticated routes this session built (/v1/servers/{id}/register and the pre-existing /result) are completely unreachable in the actual running control-plane binary today: Service.serverMutation treats a nil WorkloadVerify as fatal for both routes regardless of ServerRegistrar/ResultSubmitter being present, so every real request 503s. Wire the safe, obviously-correct half: ResultSubmitter now uses store.PostgresResults{DB: db}, same pattern as ServerRegistrar. Deliberately NOT attempting a WorkloadVerify implementation here. server/workload/jwt.go's ParseAndValidate needs a pre-known "expected" WorkloadBinding to construct its policy against (itself needing a durable per-allocation lookup that doesn't exist yet) plus a real cryptographic SignatureVerifier -- which for a Kubernetes projected service account token means either fetching/caching the cluster's own JWKS or delegating to the API server's TokenReview endpoint, a different verification model that doesn't fit ParseAndValidate's signature-callback shape at all and would need its own domain-level adapter. This is authentication-critical code with no existing wiring example anywhere in the codebase to follow, and the actual trust boundary (a live cluster's key material) can't be validated from this sandbox regardless of how carefully the client code is written. Building it fast under this session's already-heavy pace risked a subtle, dangerous mistake far more costly than leaving the gap named precisely, which is what this commit does instead. Added TestServerRoutesRequireWorkloadVerifyToBeWired: pins the current 503-on-every-request behavior as an explicit, visible regression trip-wire rather than a silent gap -- it's designed to start failing (and be updated, not deleted) the day WorkloadVerify is actually wired. --- server/cmd/control-plane/main.go | 1 + server/cmd/control-plane/main_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/server/cmd/control-plane/main.go b/server/cmd/control-plane/main.go index 2493944d..71bed570 100644 --- a/server/cmd/control-plane/main.go +++ b/server/cmd/control-plane/main.go @@ -87,6 +87,7 @@ func newAPIHandler(db *sql.DB, indexes ...api.CandidateIndex) http.Handler { ProposalBackend: api.ProposalProviderFromStore(db), ProposalPromoter: api.ProposalPromoterFromStore(db), ServerRegistrar: api.ServerRegistrarFromStore(db), + ResultSubmitter: store.PostgresResults{DB: db}, Assignment: api.AssignmentProviderFromStore(db), CandidateIndex: candidateIndex, ProbeRecorder: store.PostgresQueue{DB: db}, diff --git a/server/cmd/control-plane/main_test.go b/server/cmd/control-plane/main_test.go index 6248922f..de7115c3 100644 --- a/server/cmd/control-plane/main_test.go +++ b/server/cmd/control-plane/main_test.go @@ -14,3 +14,27 @@ func TestAPIHandlerExposesHealthWithoutDatabase(t *testing.T) { t.Fatalf("health status = %d", rec.Code) } } + +// TestServerRoutesRequireWorkloadVerifyToBeWired pins a real, known gap +// rather than leaving it silent: newAPIHandler wires ServerRegistrar and +// ResultSubmitter, but never a WorkloadVerify -- and Service.serverMutation +// treats a nil WorkloadVerify as fatal for BOTH the register and result +// routes, regardless of whether their own dependency is present. So today, +// in the actual running binary, POST /v1/servers/{id}/register and +// /v1/servers/{id}/result both always 503, independent of a real database or +// real request. This test should start failing (and be updated, not +// deleted) the day a real WorkloadVerify is wired -- that's the intended +// signal, not a bug in the test. +func TestServerRoutesRequireWorkloadVerifyToBeWired(t *testing.T) { + handler := newAPIHandler(nil) + for _, path := range []string{"/v1/servers/server-1/register", "/v1/servers/server-1/result"} { + req := httptest.NewRequest(http.MethodPost, path, nil) + req.Header.Set("Idempotency-Key", "regression-pin-key-123456") + req.Header.Set("Authorization", "Bearer anything") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusServiceUnavailable { + t.Fatalf("%s status = %d, want 503 (WorkloadVerify still unwired) -- if this changed, update this test rather than deleting it", path, rec.Code) + } + } +}