admin: drop dup RoleStore + Retry-After in forward 503 (Codex P2)

Two changes on the AdminForward leader-side dispatcher:

- Codex P2 on PR #635: forwarded 503 leader_unavailable lost
  the Retry-After: 1 header that the leader-direct HTTP path
  emits. Add retry_after_seconds to AdminForwardResponse so the
  follower's bridge can rebuild the same HTTP header (the bridge
  side wires it in the next PR; the proto + leader populate it
  here). Direct HTTP path is unaffected — the proto field is
  only consumed during forwarding.
- Drop the duplicate RoleStore/MapRoleStore declarations from
  forward_server.go. They now live in role_store.go (added on
  PR #634 for HTTP-side role revalidation) so both surfaces
  share one definition.

Tests: extend TestForwardServer_CreateTable_LeaderSteppedDownReturns503
to pin the retry_after_seconds=1 hint.

Author: bootjp

internal/admin/forward_server.go

@@ -42,30 +42,6 @@ type ForwardServer struct {
 	logger *slog.Logger
 }
 
-// RoleStore is the access-key → role lookup the leader uses to
-// re-validate the inbound principal. Implementations should mirror
-// the admin server's `Roles` map; production passes a typed wrapper
-// around that map so tests can swap in an in-memory stub.
-type RoleStore interface {
-	// LookupRole returns the role for an access key as understood
-	// by the leader's view of cluster configuration. The bool is
-	// false when the access key is not in the admin role index — a
-	// follower that forwarded the principal should not be able to
-	// "make up" an admin identity.
-	LookupRole(accessKey string) (Role, bool)
-}
-
-// MapRoleStore is the trivial in-memory implementation, sufficient
-// for tests and for the production wiring (which already keeps the
-// role map in memory).
-type MapRoleStore map[string]Role
-
-// LookupRole implements RoleStore.
-func (m MapRoleStore) LookupRole(accessKey string) (Role, bool) {
-	r, ok := m[accessKey]
-	return r, ok
-}
-
 // NewForwardServer wires a TablesSource and a RoleStore behind the
 // gRPC AdminForward service. logger may be nil; defaults to
 // slog.Default().
@@ -237,7 +213,14 @@ func forwardErrorResponse(op string, err error) *pb.AdminForwardResponse {
 		// Should never happen on the leader path — the leader
 		// just verified itself — but if a leadership transfer
 		// races with the dispatch, surface it consistently.
-		return mustForwardJSON(http.StatusServiceUnavailable, errorBody{Error: "leader_unavailable", Message: "leader stepped down mid-request"})
+		// Carry retry_after_seconds=1 so the follower's bridge
+		// translates it back into the same HTTP Retry-After
+		// header the leader-direct path emits (Codex P2 on
+		// PR #635 — without this the forwarded 503 would lose
+		// its retry timing).
+		resp := mustForwardJSON(http.StatusServiceUnavailable, errorBody{Error: "leader_unavailable", Message: "leader stepped down mid-request"})
+		resp.RetryAfterSeconds = 1
+		return resp
 	case errors.Is(err, ErrTablesNotFound):
 		return mustForwardJSON(http.StatusNotFound, errorBody{Error: "not_found", Message: "table does not exist"})
 	case errors.Is(err, ErrTablesAlreadyExists):

internal/admin/forward_server_test.go

@@ -334,6 +334,12 @@ func TestForwardServer_CreateTable_LeaderSteppedDownReturns503(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, int32(http.StatusServiceUnavailable), resp.GetStatusCode())
 	require.Contains(t, string(resp.GetPayload()), "leader_unavailable")
+	// Codex P2 on PR #635: the leader's 503 must carry the
+	// retry-after hint over the wire so the follower's bridge can
+	// rebuild the same HTTP Retry-After header a leader-direct
+	// 503 would have produced. Without this the forwarded 503
+	// silently strips the retry timing.
+	require.Equal(t, int32(1), resp.GetRetryAfterSeconds())
 }
 
 // TestForwardServer_DeleteTable_LeaderSteppedDownReturns503 mirrors

proto/admin_forward.pb.go

@@ -74,4 +74,14 @@ message AdminForwardResponse {
   // practice always "application/json; charset=utf-8" but echoed
   // explicitly so the follower does not have to guess.
   string content_type = 3;
+  // retry_after_seconds carries a Retry-After hint the follower's
+  // bridge translates back into an HTTP Retry-After header. The
+  // direct HTTP path sets Retry-After: 1 on 503 leader_unavailable
+  // (mid-dispatch leadership churn); without this field the same
+  // response shape would lose its retry timing once forwarded —
+  // clients would see 503 with no Retry-After and fall back to
+  // their default backoff. Codex P2 on PR #635 flagged the gap.
+  // Zero means "no Retry-After header"; the field is only meaningful
+  // for status codes that traditionally carry one (typically 503).
+  int32 retry_after_seconds = 4;
 }