amadan

carlos / platform Public

status: review fixes — snake_case tags, CAS-retry coverage, label bias

Four small fixes from review round 1:

- RouteCounts now carries explicit snake_case json tags (static,
  resident, hibernating, awake, asleep) matching the design spec's
  fleet.json example, instead of Go's default capitalized field
  names.
- Added tests exercising WriteBox's CAS-conflict retry branch
  directly via a flakyPutStore double: one transient conflict retries
  and succeeds, two consecutive conflicts return the error rather
  than retrying forever.
- Deduplicated "ada" in labelNames (appeared at index 0 and 32);
  replaced the second occurrence with "nadia".
- Label now combines 4 digest bytes into a uint32 per word index
  instead of a single raw byte, flattening the 256%40 modulo bias.
  Labels for existing machine ids change as a result — nothing is
  persisted keyed by label yet (Task 2 just landed), so this is safe.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell d1530d7bdcaff96ece44c049004612c11940c83f parent 7cd8b7c
2 files changed, +107 −6
  • internal/status/snapshot.go +15 −6
  • internal/status/status_test.go +92 −0
diff --git a/internal/status/snapshot.go b/internal/status/snapshot.go
index e048cef86..acc6242f4 100644
--- a/internal/status/snapshot.go
+++ b/internal/status/snapshot.go
@@ -9,6 +9,7 @@ package status
import (
"context"
"crypto/sha256"
+ "encoding/binary"
"encoding/json"
"errors"
"os"
@@ -34,8 +35,11 @@ type Probes struct {
// full — partial knowledge published as a count would read as more certain
// than it is.
type RouteCounts struct {
- Static, Resident, Hibernating int
- Awake, Asleep *int // null when the socket table is unreadable
+ Static int `json:"static"`
+ Resident int `json:"resident"`
+ Hibernating int `json:"hibernating"`
+ Awake *int `json:"awake"` // null when the socket table is unreadable
+ Asleep *int `json:"asleep"` // null when the socket table is unreadable
}
// Replication summarizes litestream's furthest-behind lag across this box's
@@ -113,7 +117,7 @@ var labelNames = [40]string{
"radia", "vint", "tim", "hedy", "kathleen", "edsger", "donald", "john",
"claude", "marvin", "shafi", "leslie", "whitfield", "adi", "ron", "sophie",
"joan", "frances", "betty", "jean", "mary", "annie", "katherine", "dorothy",
- "ada", "elena", "priya", "yuki", "noor", "amara", "leo", "iris",
+ "nadia", "elena", "priya", "yuki", "noor", "amara", "leo", "iris",
}
var labelActions = [40]string{
"stargazing", "birdwatching", "gardening", "sketching", "kayaking",
@@ -131,11 +135,16 @@ var labelActions = [40]string{
// weaker hash: the machine id must never be recoverable from the label by
// brute force over any plausible id space (hostnames, instance ids), and the
// label is public.
+//
+// Each index is drawn from a 4-byte (32-bit) chunk of the digest rather than
+// a single byte: 256 % 40 == 16, so indexing with one raw byte would leave
+// names 0-15 roughly 1.4x likelier than names 16-39. A uint32 % 40 has a bias
+// under 1 in 100 million — small enough to treat the label as uniform.
func Label(machineID string) string {
sum := sha256.Sum256([]byte(machineID))
- name := labelNames[int(sum[0])%len(labelNames)]
- action := labelActions[int(sum[1])%len(labelActions)]
- return name + "-" + action
+ nameIdx := binary.BigEndian.Uint32(sum[0:4]) % uint32(len(labelNames))
+ actionIdx := binary.BigEndian.Uint32(sum[4:8]) % uint32(len(labelActions))
+ return labelNames[nameIdx] + "-" + labelActions[actionIdx]
}
// CollectBox turns a box's live registry rows and probe results into the
diff --git a/internal/status/status_test.go b/internal/status/status_test.go
index 75ca2163d..ca8195b51 100644
--- a/internal/status/status_test.go
+++ b/internal/status/status_test.go
@@ -3,6 +3,7 @@ package status_test
import (
"context"
"encoding/json"
+ "errors"
"strings"
"testing"
"time"
@@ -12,6 +13,24 @@ import (
"github.com/carlosframework/platform/internal/status"
)
+// flakyPutStore wraps a real release.Store and makes its first N Put calls
+// return release.ErrCASConflict before delegating — a stand-in for another
+// writer racing in between WriteBox's Get-etag and Put. Every other method
+// (Get, List) is the embedded Store's real behavior.
+type flakyPutStore struct {
+ release.Store
+ failCount int // Put calls left to fail before delegating
+ putCalls int
+}
+
+func (f *flakyPutStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) {
+ f.putCalls++
+ if f.putCalls <= f.failCount {
+ return "", release.ErrCASConflict
+ }
+ return f.Store.Put(ctx, key, body, ifMatch)
+}
+
func TestBandBoundaries(t *testing.T) {
f := func(v float64) *float64 { return &v }
cases := []struct {
@@ -58,6 +77,35 @@ func TestSnapshotJSONCarriesNoHostnames(t *testing.T) {
}
}
+// TestRouteCountsJSONKeysAreSnakeCase locks RouteCounts' wire shape to the
+// design spec's fleet.json example (docs/superpowers/specs/2026-08-01-fleet-
+// status-page-design.md §2: {"static":2,"resident":3,"hibernating":4,
+// "awake":1,"asleep":3}) — lowercase snake_case, not Go's default
+// capitalized field names.
+func TestRouteCountsJSONKeysAreSnakeCase(t *testing.T) {
+ routes := []registry.Route{
+ {Host: "a", Kind: "static"},
+ {Host: "b", Kind: "instance"},
+ {Host: "c", Kind: "instance", Hibernate: true, Socket: "/c.sock"},
+ }
+ listening := func(string) *bool { v := true; return &v }
+ b := status.CollectBox(routes, status.Probes{MachineID: "m", Listening: listening}, time.Now())
+ j, err := json.Marshal(b.Routes)
+ if err != nil {
+ t.Fatalf("marshal RouteCounts: %v", err)
+ }
+ for _, key := range []string{`"static"`, `"resident"`, `"hibernating"`, `"awake"`, `"asleep"`} {
+ if !strings.Contains(string(j), key) {
+ t.Errorf("RouteCounts JSON %s missing key %s", j, key)
+ }
+ }
+ for _, key := range []string{`"Static"`, `"Resident"`, `"Hibernating"`, `"Awake"`, `"Asleep"`} {
+ if strings.Contains(string(j), key) {
+ t.Errorf("RouteCounts JSON %s carries capitalized Go field name %s", j, key)
+ }
+ }
+}
+
// TestWriteBoxPutsUnderLabeledKey isn't in the plan's Step-1 listing, but
// WriteBox is part of the exported contract Task 3 relies on — worth locking
// down that it actually lands the box at the key CollectBox's label implies.
@@ -117,3 +165,47 @@ func TestWriteBoxOverwritesOnSecondTick(t *testing.T) {
t.Errorf("stored box has GeneratedAt %v, want the second write's %v (second write must win)", round.GeneratedAt, second.GeneratedAt)
}
}
+
+// TestWriteBoxRetriesOnceOnCASConflict exercises the retry branch directly:
+// the first Put loses the CAS race (something else wrote between WriteBox's
+// Get and its Put), so WriteBox must re-read the etag and retry exactly
+// once — and that retry must succeed.
+func TestWriteBoxRetriesOnceOnCASConflict(t *testing.T) {
+ dir := t.TempDir()
+ real := release.NewDirStore(dir)
+ flaky := &flakyPutStore{Store: real, failCount: 1}
+ b := status.CollectBox(nil, status.Probes{MachineID: "m"}, time.Now())
+
+ if err := status.WriteBox(context.Background(), flaky, b); err != nil {
+ t.Fatalf("WriteBox with one transient conflict: %v", err)
+ }
+ if flaky.putCalls != 2 {
+ t.Errorf("Put called %d times, want exactly 2 (fail once, retry once)", flaky.putCalls)
+ }
+
+ key := "status/boxes/" + b.Label + ".json"
+ if _, _, err := real.Get(context.Background(), key); err != nil {
+ t.Fatalf("Get %s after retry: %v", key, err)
+ }
+}
+
+// TestWriteBoxReturnsErrorOnDoubleCASConflict proves WriteBox does NOT retry
+// a second time: if the retried Put also loses the race, that error must
+// reach the caller rather than being swallowed or retried forever.
+func TestWriteBoxReturnsErrorOnDoubleCASConflict(t *testing.T) {
+ dir := t.TempDir()
+ real := release.NewDirStore(dir)
+ flaky := &flakyPutStore{Store: real, failCount: 2}
+ b := status.CollectBox(nil, status.Probes{MachineID: "m"}, time.Now())
+
+ err := status.WriteBox(context.Background(), flaky, b)
+ if err == nil {
+ t.Fatal("WriteBox: want an error after two consecutive CAS conflicts, got nil")
+ }
+ if !errors.Is(err, release.ErrCASConflict) {
+ t.Errorf("WriteBox error = %v, want it to wrap release.ErrCASConflict", err)
+ }
+ if flaky.putCalls != 2 {
+ t.Errorf("Put called %d times, want exactly 2 (no third attempt)", flaky.putCalls)
+ }
+}