fix!: order privilege events by sequence number and merge snapshots by position

The four privilege keys arrive on separate transient queues, so a late
Privilege.Added or User.Added could resurrect a revoked grant. Services also
fetched /authz before binding their queues, losing revocations published in
between.

Process now orders events by authz-service's global sequenceNo per
(email, company): an event only overrides older facts, User.Removed stamps
every privilege, and events without a sequence number fail closed (additions
dropped, removals held until the next snapshot). Fetch checks the status,
retries 503 while authz-service's read view is behind, reads the
X-Authz-Sequence header and merges the snapshot as facts at that position,
ignoring snapshots older than one already merged. CompaniesByUser returns []
instead of nil.

BREAKING CHANGE: events without SequenceNo no longer grant anything; tests that
seed the handler through Process must set SequenceNo. Call Fetch() after
conn.Start (ADR-0015).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DVGsVQ8AMFR4NZoxyCoEqS

client.go

@@ -3,16 +3,29 @@ package client
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"log/slog"
+	"math"
 	"net/http"
 	"reflect"
+	"strconv"
 	"sync"
+	"time"
 
 	goamqp "codeberg.org/messaging/go-messaging-amqp"
 	spec "codeberg.org/messaging/messaging"
 )
 
+// SequenceHeader carries the global sequence number of the authz-service read view the
+// /authz snapshot was read at.
+const SequenceHeader = "X-Authz-Sequence"
+
+// pending marks a fact cleared by an event without a sequence number. No event can
+// override it; only a snapshot can.
+const pending = math.MaxInt
+
 // CompanyPrivileges contains the privileges for a combination of email address and company id
 type CompanyPrivileges struct {
 	Admin      bool `json:"admin"`
@@ -25,13 +38,84 @@ type CompanyPrivileges struct {
 	Salary     bool `json:"salary"`
 }
 
+// field returns the flag for privilege, or nil for an unknown privilege.
+func (c *CompanyPrivileges) field(privilege Privilege) *bool {
+	switch privilege {
+	case PrivilegeAdmin:
+		return &c.Admin
+	case PrivilegeCompany:
+		return &c.Company
+	case PrivilegeConsumer:
+		return &c.Consumer
+	case PrivilegeTime:
+		return &c.Time
+	case PrivilegeInvoicing:
+		return &c.Invoicing
+	case PrivilegeAccounting:
+		return &c.Accounting
+	case PrivilegeSupplier:
+		return &c.Supplier
+	case PrivilegeSalary:
+		return &c.Salary
+	}
+	return nil
+}
+
+// companyState is what the handler knows about one email in one company. Every
+// privilege event for a company comes from authz-service's Company aggregate, so
+// its global sequence number orders the events for the pair. The four routing
+// keys arrive on separate queues in any order; a fact is only overwritten by a
+// fact with a higher sequence number.
+type companyState struct {
+	privileges CompanyPrivileges
+	// privilegeSeq is the sequence number of the last fact about each privilege.
+	privilegeSeq map[Privilege]int
+	// member is whether the user belongs to the company, as of memberSeq. A
+	// User.Removed stamps every privilege with its sequence number, so a grant
+	// older than the removal can't come back.
+	member    bool
+	memberSeq int
+}
+
+func newCompanyState() *companyState {
+	return &companyState{privilegeSeq: map[Privilege]int{}}
+}
+
+// clearPending turns facts cleared without a sequence number into unordered facts, so
+// the snapshot being merged replaces them.
+func (s *companyState) clearPending() {
+	if s.memberSeq == pending {
+		s.memberSeq = 0
+	}
+	for p, seq := range s.privilegeSeq {
+		if seq == pending {
+			s.privilegeSeq[p] = 0
+		}
+	}
+}
+
+// maxSeq is the highest sequence number of any fact in the state.
+func (s *companyState) maxSeq() int {
+	m := s.memberSeq
+	for _, seq := range s.privilegeSeq {
+		m = max(m, seq)
+	}
+	return m
+}
+
 // PrivilegeHandler processes PrivilegeAdded-events and fetches the initial set of privileges from an authz-service
 type PrivilegeHandler struct {
 	*sync.RWMutex
-	client     *http.Client
-	baseURL    string
-	apiKey     string
-	privileges map[string]map[string]*CompanyPrivileges
+	client  *http.Client
+	baseURL string
+	apiKey  string
+	state   map[string]map[string]*companyState
+	// floor is the sequence number of the newest snapshot. Its effects are all in
+	// the state, so an event at or below it is stale.
+	floor int
+	// retries is how many times Fetch retries a snapshot authz-service isn't ready to serve.
+	retries    int
+	retryDelay time.Duration
 }
 
 // OptsFunc is used to configure the PrivilegeHandler
@@ -55,9 +139,11 @@ func WithAPIKey(key string) OptsFunc {
 func New(opts ...OptsFunc) *PrivilegeHandler {
 	handler := &PrivilegeHandler{
 		RWMutex:    &sync.RWMutex{},
-		client:     &http.Client{},
+		client:     &http.Client{Timeout: 30 * time.Second},
 		baseURL:    "http://authz-service",
-		privileges: map[string]map[string]*CompanyPrivileges{},
+		state:      map[string]map[string]*companyState{},
+		retries:    60,
+		retryDelay: time.Second,
 	}
 	for _, opt := range opts {
 		opt(handler)
@@ -65,8 +151,27 @@ func New(opts ...OptsFunc) *PrivilegeHandler {
 	return handler
 }
 
-// Fetch the initial set of privileges from an authz-service
+// Fetch a snapshot of all privileges from an authz-service and merge it into the state.
+//
+// Call it after the AMQP connection has started: authz-service only serves a snapshot
+// that includes every stored event, so together with the bound queues no event is
+// missed. While its read view is behind it answers 503, and Fetch retries. The snapshot
+// is applied as facts at its sequence number: a fact from an event newer than the
+// snapshot is kept, everything else is replaced, and a pair missing from the snapshot is
+// removed. A snapshot older than one already merged is ignored.
 func (h *PrivilegeHandler) Fetch() error {
+	for attempt := 0; ; attempt++ {
+		err := h.fetch()
+		if !errors.Is(err, errNotReady) || attempt >= h.retries {
+			return err
+		}
+		time.Sleep(h.retryDelay)
+	}
+}
+
+var errNotReady = errors.New("fetch privileges: authz-service read view not ready")
+
+func (h *PrivilegeHandler) fetch() error {
 	req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/authz", h.baseURL), nil)
 	if err != nil {
 		return err
@@ -80,21 +185,83 @@ func (h *PrivilegeHandler) Fetch() error {
 	if err != nil {
 		return err
 	}
+	defer func() { _ = resp.Body.Close() }()
+
+	if resp.StatusCode == http.StatusServiceUnavailable {
+		return errNotReady
+	}
+	if resp.StatusCode != http.StatusOK {
+		return fmt.Errorf("fetch privileges: unexpected status %s", resp.Status)
+	}
 
 	buff, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return err
 	}
 
-	h.Lock()
-	defer h.Unlock()
-	err = json.Unmarshal(buff, &h.privileges)
-	if err != nil {
+	var snapshot map[string]map[string]CompanyPrivileges
+	if err := json.Unmarshal(buff, &snapshot); err != nil {
 		return err
 	}
+
+	seq := 0
+	if v := resp.Header.Get(SequenceHeader); v != "" {
+		seq, err = strconv.Atoi(v)
+		if err != nil || seq < 0 {
+			return fmt.Errorf("fetch privileges: invalid %s header %q", SequenceHeader, v)
+		}
+	} else {
+		// ponytail: only during the rollout window, before authz-service sends the header.
+		slog.Warn("authz snapshot has no sequence number; events older than it can revert it", "header", SequenceHeader)
+	}
+
+	h.Lock()
+	defer h.Unlock()
+	h.merge(snapshot, seq)
 	return nil
 }
 
+func (h *PrivilegeHandler) merge(snapshot map[string]map[string]CompanyPrivileges, seq int) {
+	if seq < h.floor {
+		// A newer snapshot is already merged; this one could only bring back removed state.
+		slog.Warn("ignoring authz snapshot older than the one already merged", "seq", seq, "floor", h.floor)
+		return
+	}
+	for email, companies := range h.state {
+		for companyID, s := range companies {
+			s.clearPending()
+			if _, exists := snapshot[email][companyID]; exists {
+				continue
+			}
+			if s.maxSeq() <= seq {
+				// Nothing newer than the snapshot; the floor keeps stale events out.
+				delete(companies, companyID)
+				continue
+			}
+			removeUser(s, seq, false)
+		}
+		if len(companies) == 0 {
+			delete(h.state, email)
+		}
+	}
+	for email, companies := range snapshot {
+		for companyID, privileges := range companies {
+			s := h.company(email, companyID)
+			if s.memberSeq <= seq {
+				s.member = true
+				s.memberSeq = seq
+			}
+			for _, p := range AllPrivilege {
+				if s.privilegeSeq[p] <= seq {
+					*s.privileges.field(p) = *privileges.field(p)
+					s.privilegeSeq[p] = seq
+				}
+			}
+		}
+	}
+	h.floor = max(h.floor, seq)
+}
+
 func (h *PrivilegeHandler) Setup() []goamqp.Setup {
 	return []goamqp.Setup{
 		goamqp.TransientEventStreamConsumer("User.Added", process[UserAdded](h)),
@@ -116,71 +283,118 @@ func process[T privilegeEvent](h *PrivilegeHandler) spec.EventHandler[T] {
 	}
 }
 
-// Process privilege-related events and update the internal state
+// Process privilege-related events and update the internal state.
+//
+// Events are applied by sequence number, not by arrival order. An event without a
+// sequence number can't be ordered, so it fails closed: a removal is applied and an
+// addition is dropped.
 func (h *PrivilegeHandler) Process(msg any) error {
 	h.Lock()
 	defer h.Unlock()
 
 	switch ev := msg.(type) {
 	case *UserAdded:
-		// Keep the privileges already recorded for the company. Each routing key has
-		// its own transient queue, so a Privilege.Added published after this event can
-		// be processed before it; overwriting the entry here would drop that privilege
-		// until the next Fetch, which only runs at start.
-		priv, exists := h.privileges[ev.Email]
-		if !exists {
-			priv = map[string]*CompanyPrivileges{}
-			h.privileges[ev.Email] = priv
+		if h.stale(ev.SequenceNo, true, ev) {
+			return nil
 		}
-		if _, exists := priv[ev.CompanyID]; !exists {
-			priv[ev.CompanyID] = &CompanyPrivileges{}
+		s := h.company(ev.Email, ev.CompanyID)
+		if ev.SequenceNo > s.memberSeq {
+			s.member = true
+			s.memberSeq = ev.SequenceNo
 		}
 		return nil
 	case *UserRemoved:
-		if priv, exists := h.privileges[ev.Email]; exists {
-			delete(priv, ev.CompanyID)
+		if h.stale(ev.SequenceNo, false, ev) {
+			return nil
 		}
+		removeUser(h.company(ev.Email, ev.CompanyID), ev.SequenceNo, ev.SequenceNo == 0)
 		return nil
 	case *PrivilegeAdded:
-		h.setPrivileges(ev.Email, ev.CompanyID, ev.Privilege, true)
+		if h.stale(ev.SequenceNo, true, ev) {
+			return nil
+		}
+		h.setPrivilege(ev.Email, ev.CompanyID, ev.Privilege, ev.SequenceNo, true)
 		return nil
 	case *PrivilegeRemoved:
-		h.setPrivileges(ev.Email, ev.CompanyID, ev.Privilege, false)
+		if h.stale(ev.SequenceNo, false, ev) {
+			return nil
+		}
+		h.setPrivilege(ev.Email, ev.CompanyID, ev.Privilege, ev.SequenceNo, false)
 		return nil
 	default:
-		fmt.Printf("Got unexpected message type (%s): '%+v'\n", reflect.TypeOf(msg).String(), msg)
+		slog.Error("unexpected privilege message type", "type", reflect.TypeOf(msg).String())
 		return fmt.Errorf("unexpected event type: '%s'", reflect.TypeOf(msg))
 	}
 }
 
-func (h *PrivilegeHandler) setPrivileges(email, companyId string, privilege Privilege, set bool) {
-	if priv, exists := h.privileges[email]; exists {
-		if c, exists := priv[companyId]; exists {
-			switch privilege {
-			case PrivilegeAdmin:
-				c.Admin = set
-			case PrivilegeCompany:
-				c.Company = set
-			case PrivilegeConsumer:
-				c.Consumer = set
-			case PrivilegeTime:
-				c.Time = set
-			case PrivilegeInvoicing:
-				c.Invoicing = set
-			case PrivilegeAccounting:
-				c.Accounting = set
-			case PrivilegeSupplier:
-				c.Supplier = set
-			case PrivilegeSalary:
-				c.Salary = set
-			}
-		} else {
-			priv[companyId] = &CompanyPrivileges{}
-			h.setPrivileges(email, companyId, privilege, set)
+// stale reports whether an event must be skipped: an addition without a sequence
+// number, or any event already covered by the latest snapshot.
+func (h *PrivilegeHandler) stale(seq int, addition bool, ev any) bool {
+	if seq < 0 || seq >= pending {
+		slog.Error("dropping privilege event with invalid sequence number", "type", reflect.TypeOf(ev).String(), "seq", seq)
+		return true
+	}
+	if seq == 0 {
+		if addition {
+			slog.Warn("dropping privilege addition without sequence number", "type", reflect.TypeOf(ev).String())
 		}
-	} else {
-		h.privileges[email] = map[string]*CompanyPrivileges{}
-		h.setPrivileges(email, companyId, privilege, set)
+		return addition
+	}
+	return seq <= h.floor
+}
+
+func (h *PrivilegeHandler) company(email, companyID string) *companyState {
+	companies, exists := h.state[email]
+	if !exists {
+		companies = map[string]*companyState{}
+		h.state[email] = companies
+	}
+	s, exists := companies[companyID]
+	if !exists {
+		s = newCompanyState()
+		companies[companyID] = s
+	}
+	return s
+}
+
+// removeUser removes the user at seq, overriding every fact older than seq. An
+// unordered removal (all) overrides every fact and marks them pending, so no event can
+// bring them back before the next snapshot.
+func removeUser(s *companyState, seq int, all bool) {
+	stamp := seq
+	if all {
+		stamp = pending
+	}
+	if all || s.memberSeq < seq {
+		s.member = false
+		s.memberSeq = max(s.memberSeq, stamp)
+	}
+	for _, p := range AllPrivilege {
+		if all || s.privilegeSeq[p] < seq {
+			*s.privileges.field(p) = false
+			s.privilegeSeq[p] = max(s.privilegeSeq[p], stamp)
+		}
+	}
+}
+
+func (h *PrivilegeHandler) setPrivilege(email, companyID string, privilege Privilege, seq int, set bool) {
+	if !privilege.IsValid() {
+		return
+	}
+	s := h.company(email, companyID)
+	if seq != 0 && seq <= s.privilegeSeq[privilege] {
+		return
+	}
+	*s.privileges.field(privilege) = set
+	if seq == 0 {
+		// Only an unordered removal gets here; keep it until the next snapshot.
+		seq = pending
+	}
+	s.privilegeSeq[privilege] = max(s.privilegeSeq[privilege], seq)
+	// authz-service's aggregate adds the user when a privilege is granted.
+	if set && seq > s.memberSeq {
+		s.member = true
+		s.memberSeq = seq
 	}
 }
 
@@ -188,12 +402,10 @@ func (h *PrivilegeHandler) setPrivileges(email, companyId string, privilege Priv
 func (h *PrivilegeHandler) CompaniesByUser(email string, predicate func(privileges CompanyPrivileges) bool) []string {
 	h.RLock()
 	defer h.RUnlock()
-	var result []string
-	if p, exists := h.privileges[email]; exists {
-		for k, v := range p {
-			if predicate(*v) {
-				result = append(result, k)
-			}
+	result := []string{}
+	for k, s := range h.state[email] {
+		if s.member && predicate(s.privileges) {
+			result = append(result, k)
 		}
 	}
 	return result
@@ -203,11 +415,8 @@ func (h *PrivilegeHandler) CompaniesByUser(email string, predicate func(privileg
 func (h *PrivilegeHandler) IsAllowed(email, companyID string, predicate func(privileges CompanyPrivileges) bool) bool {
 	h.RLock()
 	defer h.RUnlock()
-	if p, exists := h.privileges[email]; exists {
-		if v, exists := p[companyID]; exists {
-			return predicate(*v)
-		}
+	if s, exists := h.state[email][companyID]; exists && s.member {
+		return predicate(s.privileges)
 	}
-
 	return false
 }