diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index 50776dd..96abf38 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -41,6 +41,14 @@ either way. **Only rows that actually carry an identity blob count**: the pre-v0 retain nothing the screen could be talking about. It is a boolean and a timestamp; it grants **no read path**, which is still unbuilt (R-199's inventory). +**And the hold REASON now matches the hold CAUSE.** Adding a second reason and leaving both surfaces +printing the first is `CLAUDE.md`'s corollary, and it happened here: the freshly deployed hub logged +`managed floor HELD for c11: agent "0.125.0" < MinAgent 0.113.0` — a comparison that is FALSE +(0.125.0 is above 0.113.0) for a box held because its floor sat above the golden. Both the ACK log +line and the Hosts-dashboard `HeldReason` now come from one `ManagedFloorDecision.HoldReason()`, and a +test pins each reason to its cause. A true alarm that misdescribes itself is one the operator learns +to dismiss. + Tests: `managed_floor_test.go` — the campaign's exact numbers, held-regardless-of-agent, Scenario C (uncoupled untouched), floor==golden unchanged, unparseable golden. **Red-proof: removing the floor-above-golden branch reproduces the campaign's measurement exactly.** diff --git a/hub/internal/api/handler.go b/hub/internal/api/handler.go index d5ca696..9ceb117 100644 --- a/hub/internal/api/handler.go +++ b/hub/internal/api/handler.go @@ -540,8 +540,10 @@ func (h *Handler) handleReport(w http.ResponseWriter, r *http.Request) { if fd := h.store.ResolveManagedFloor(payload.CustomerID); fd.Floor != "" { resp["min_controller_version"] = fd.Floor } else if fd.Held { - h.logger.Printf("[INFO] managed floor HELD for %s: agent %q < MinAgent %s (controller floor withheld)", - payload.CustomerID, fd.AgentVersion, fd.MinAgent) + // ONE sentence, from the decision itself — the two hold reasons must never drift apart + // across this line and the dashboard (see ManagedFloorDecision.HoldReason). + h.logger.Printf("[INFO] managed floor HELD for %s: %s (controller floor withheld)", + payload.CustomerID, fd.HoldReason()) } if h.latestVersion != nil { if latest := h.latestVersion.LatestVersion(); latest != "" { diff --git a/hub/internal/store/managed_floor_test.go b/hub/internal/store/managed_floor_test.go index 17d360b..1d1ae8d 100644 --- a/hub/internal/store/managed_floor_test.go +++ b/hub/internal/store/managed_floor_test.go @@ -1,6 +1,9 @@ package store -import "testing" +import ( + "strings" + "testing" +) // setHostAgent enrolls a host for customerID and records its agent version (the reality column). func setHostAgent(t *testing.T, s *Store, customerID, hostID, agentVer string) { @@ -180,3 +183,49 @@ func TestResolveManagedFloor_R216_FloorAboveGolden(t *testing.T) { } }) } + +// The hold REASON must match the hold CAUSE. This is `CLAUDE.md`'s corollary, and it caught a real +// slip in this very session: v0.97.0 added a second hold reason, and for one deploy the live hub +// logged `managed floor HELD for c11: agent "0.125.0" < MinAgent 0.113.0` — a comparison that is +// FALSE, for a box held because its floor sat above the golden. A true alarm that misdescribes itself +// is one the operator learns to dismiss. +func TestResolveManagedFloor_HoldReasonMatchesTheCause(t *testing.T) { + t.Run("beyond-golden hold names the floor and the golden, not the agent", func(t *testing.T) { + s := newTestStore(t) + _ = s.SetGlobalMinControllerVersion("0.200.0") + _ = s.SetArtifactManifest(ArtifactManifest{GoldenVersion: "0.192.0", MinAgent: "0.113.0"}) + setHostAgent(t, s, "c11", "c11-36d660", "0.125.0") + + reason := s.ResolveManagedFloor("c11").HoldReason() + if strings.Contains(reason, "< MinAgent") { + t.Fatalf("the beyond-golden hold claims an agent comparison that is FALSE here (0.125.0 > 0.113.0): %q", reason) + } + for _, want := range []string{"0.200.0", "0.192.0", "ABOVE"} { + if !strings.Contains(reason, want) { + t.Errorf("hold reason must name %q; got %q", want, reason) + } + } + }) + + t.Run("below-MinAgent hold still names the agent comparison", func(t *testing.T) { + s := newTestStore(t) + _ = s.SetGlobalMinControllerVersion("0.113.0") + _ = s.SetArtifactManifest(ArtifactManifest{GoldenVersion: "0.113.0", MinAgent: "0.81.0"}) + setHostAgent(t, s, "c1", "h1", "0.79.0") + + reason := s.ResolveManagedFloor("c1").HoldReason() + if !strings.Contains(reason, "0.79.0") || !strings.Contains(reason, "MinAgent 0.81.0") { + t.Fatalf("the below-MinAgent hold must name the agent comparison; got %q", reason) + } + }) + + t.Run("not held → no reason", func(t *testing.T) { + s := newTestStore(t) + _ = s.SetGlobalMinControllerVersion("0.113.0") + _ = s.SetArtifactManifest(ArtifactManifest{GoldenVersion: "0.113.0", MinAgent: ""}) + setHostAgent(t, s, "c1", "h1", "0.70.0") + if r := s.ResolveManagedFloor("c1").HoldReason(); r != "" { + t.Fatalf("a served floor must have no hold reason, got %q", r) + } + }) +} diff --git a/hub/internal/store/store.go b/hub/internal/store/store.go index bf57929..f496dcb 100644 --- a/hub/internal/store/store.go +++ b/hub/internal/store/store.go @@ -1971,6 +1971,9 @@ type ManagedFloorDecision struct { Floor string // the controller-version floor to SERVE ("" = serve none) Held bool // true = the floor is withheld (see HeldBeyondGolden for which of the two reasons) AgentVersion string // the box's reported agent version ("" = unknown → held when MinAgent is set) + // RequestedFloor is the floor that was asked for, kept for the message: Floor is cleared to "" on + // a hold, and a reason naming an empty floor tells the operator nothing. + RequestedFloor string // MinAgent is the manifest's vouched MinAgent — the agent requirement of the GOLDEN's controller // (`ArtifactManifest.MinAgent`). ⚠ It describes the golden, NOT necessarily the version this // decision is about; that gap is what HeldBeyondGolden closes. Empty = uncoupled, no gating. @@ -1984,6 +1987,50 @@ type ManagedFloorDecision struct { HeldBeyondGolden bool } +// HoldReason is the ONE operator-facing sentence for a held floor, so the two hold reasons can never +// drift apart across the log line and the dashboard again. +// +// ⚠ It exists because v0.97.0 introduced a second reason and, for one deploy, both surfaces still +// printed the first: the live hub logged `managed floor HELD for c11: agent "0.125.0" < MinAgent +// 0.113.0` — a comparison that is FALSE (0.125.0 is above 0.113.0) for a box held because its floor +// sat above the golden. That is `CLAUDE.md`'s corollary exactly: when a verdict changes which field it +// counts from, the alarm text has to change with it, or a true alarm reads as one to dismiss. +// Returns "" when not held. +func (d ManagedFloorDecision) HoldReason() string { + switch { + case !d.Held: + return "" + case d.HeldBeyondGolden: + return fmt.Sprintf("held: floor %s is ABOVE the vouched golden %s, so its agent requirement is unknown — vouch a golden carrying the floor's controller (publish-train rule 1)", + d.EffectiveFloorForMessage(), goldenOrUnknown(d.GoldenVersion)) + default: + return fmt.Sprintf("held: agent %s < MinAgent %s", agentOrUnknownVersion(d.AgentVersion), d.MinAgent) + } +} + +// EffectiveFloorForMessage reports the floor that was REQUESTED, for the message — Floor is cleared to +// "" when held, and a message naming an empty floor tells the operator nothing. +func (d ManagedFloorDecision) EffectiveFloorForMessage() string { + if d.RequestedFloor != "" { + return d.RequestedFloor + } + return d.Floor +} + +func goldenOrUnknown(v string) string { + if v == "" { + return "(none vouched)" + } + return v +} + +func agentOrUnknownVersion(v string) string { + if v == "" { + return "(unknown)" + } + return v +} + // ResolveManagedFloor decides the controller-version floor to serve a customer, HOLDING it rather // than pushing a controller past the agent it depends on (Part D — the hub-enforced "agent BEFORE // controller floor" rule). @@ -2020,6 +2067,7 @@ type ManagedFloorDecision struct { // A held box is VISIBLE (the dashboard renders the reason), never silently stale. func (s *Store) ResolveManagedFloor(customerID string) ManagedFloorDecision { d := ManagedFloorDecision{Floor: s.EffectiveMinControllerVersion(customerID)} + d.RequestedFloor = d.Floor if d.Floor == "" { return d } diff --git a/hub/internal/web/hosts.go b/hub/internal/web/hosts.go index 235b3cc..85d4664 100644 --- a/hub/internal/web/hosts.go +++ b/hub/internal/web/hosts.go @@ -4,7 +4,6 @@ import ( "database/sql" "encoding/json" "errors" - "fmt" "net/http" "sort" "strings" @@ -401,7 +400,7 @@ func (s *Server) handleHostsList(w http.ResponseWriter, r *http.Request) { // never silently stale. if fd := s.store.ResolveManagedFloor(h.CustomerID); fd.Held { row.FloorHeld = true - row.HeldReason = fmt.Sprintf("held: agent %s < MinAgent %s", agentOrUnknown(fd.AgentVersion), fd.MinAgent) + row.HeldReason = fd.HoldReason() } // Guest counts from the reality table (per-host accurate).