host: metrics loop honors the fleet prefix; release.NewPrefixStore exported
Fix round from Opus review of the Task 5 prefixStore addition. Two Important findings: 1. metricsLoop shared the status bucket but ignored CARLOS_STATUS_PREFIX, writing metrics/boxes/<label>/... and alerts/boxes/<label>.json unprefixed. A fleet box's raw metrics would surface on the platform operator's monitoring page and a threshold breach would page the platform's own alert webhook for customer hardware - contradicting the isolation the status-side wrapper exists for. Wired the same release.NewPrefixStore at the metrics loop's store construction; empty prefix (every platform box today) stays a no-op. 2. Moved prefixStore/newPrefixStore from internal/host to internal/release as exported NewPrefixStore(s Store, prefix string) Store, beside the Store interface - its natural home, and reachable by Task 9's console fleet page, which needs the same rebasing and can't reach an unexported type in internal/host. Same behavior: empty prefix returns s unchanged, List/ListDirs round-trip through the same wrapper's Get without doubling the prefix. Wrapper tests moved/adapted to internal/release/store_test.go.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7 files changed,
+270
−136
README.md+11 −6internal/host/agent.go+17 −83internal/host/host_test.go+5 −46internal/host/metrics_loop.go+12 −1internal/host/metrics_loop_test.go+71 −0internal/release/store.go+84 −0internal/release/store_test.go+70 −0
diff --git a/README.md b/README.md| index 3ec23b5eb..eaeea0d05 100644 |
| --- a/README.md |
| +++ b/README.md |
| @@ -349,12 +349,17 @@ no single reader owns the whole contract: |
| does not publish fleet status yet; give it its own `CARLOS_STATUS_BUCKET` |
| if it wants the page. `CARLOS_STATUS_PREFIX` does double duty: it |
| namespaces the status-lease key, AND (customer-fleets plan, Task 5) |
| - rebases every key the tick's store touches — a fleet box set to |
| - `CARLOS_STATUS_PREFIX=fleets/<name>/` writes its own snapshot at |
| - `fleets/<name>/status/boxes/<label>.json` instead of the shared |
| - `status/boxes/` path, so it never collides with (or leaks into) the |
| - platform's own `fleet.json`. Empty (every platform box today) is a pure |
| - passthrough. |
| + rebases every key the status loop's AND the metrics loop's store |
| + touches — a fleet box set to `CARLOS_STATUS_PREFIX=fleets/<name>/` writes |
| + its own status snapshot at `fleets/<name>/status/boxes/<label>.json` and |
| + its raw/rollup metrics and alert state under `fleets/<name>/metrics/boxes/…` |
| + and `fleets/<name>/alerts/boxes/…`, instead of the shared paths a platform |
| + box uses — so neither leaks into the platform's own `fleet.json`, |
| + monitoring page, or alert webhook. Empty (every platform box today) is a |
| + pure passthrough. A **publishing** box with a prefix set would relocate |
| + `status/fleet.json` itself under that prefix too — moot today, since |
| + nothing sets a prefix on a publishing (non-fleet) box, and a fleet box |
| + never publishes at all (see `CARLOS_FLEET`, below). |
| - `CARLOS_FLEET` — read by `carlos agent`'s status loop (customer-fleets |
| plan, Task 5). Set (to the fleet's name) marks this agent as a fleet box: |
| it still self-reports its own snapshot every tick, but never contends for |
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 2327766fe..d71df4650 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -32,12 +32,19 @@ |
| // may be empty (falls back to the AWS default chain). CARLOS_STATUS_PREFIX |
| // does double duty: it namespaces that same lease key (path.Join(prefix, |
| // "status-lease")) AND, since Task 5 of the customer-fleets plan, |
| -// rebases every key statusTick's store touches (newPrefixStore) — a |
| -// fleet box set to CARLOS_STATUS_PREFIX=fleets/<name>/ writes its own |
| -// snapshot at fleets/<name>/status/boxes/<label>.json instead of the |
| -// shared status/boxes/ path every platform box also writes, so its |
| -// heartbeat never collides with (or leaks into) the platform's own |
| -// fleet.json. Empty (every platform box today) is a pure passthrough. |
| +// rebases every key statusLoop's AND metricsLoop's store touches |
| +// (release.NewPrefixStore) — a fleet box set to |
| +// CARLOS_STATUS_PREFIX=fleets/<name>/ writes its own status snapshot at |
| +// fleets/<name>/status/boxes/<label>.json and its metrics/alerts under |
| +// fleets/<name>/metrics/boxes/... and fleets/<name>/alerts/boxes/... |
| +// instead of the shared paths every platform box also writes — so |
| +// neither its status heartbeat nor its metrics/alerts ever collide with |
| +// (or leak into) the platform's own fleet.json, metrics view, or alert |
| +// webhook. Empty (every platform box today) is a pure passthrough. A |
| +// PUBLISHING box with a prefix set would relocate status/fleet.json |
| +// itself under that prefix too — moot today since nothing sets a prefix |
| +// on a publishing (non-fleet) box, and a fleet box never publishes at |
| +// all (CARLOS_FLEET's guard, below). |
| // - CARLOS_FLEET — read by statusLoop (Task 5, customer-fleets plan). Set |
| // (to the fleet's name — its value is otherwise unused here) marks this |
| // agent as a fleet box: shouldSkipPublisherElection short-circuits |
| @@ -973,80 +980,6 @@ func shouldSkipPublisherElection(getenv func(string) string) bool { |
| return getenv("CARLOS_FLEET") != "" |
| } |
| -// prefixStore rebases every key a caller works with onto a fixed base |
| -// (CARLOS_STATUS_PREFIX's actual enforcement point, Task 5 of the |
| -// customer-fleets plan) before it ever reaches the underlying store. A |
| -// fleet box's own snapshot is written through one of these with prefix |
| -// "fleets/<name>/", so status.WriteBox's hardcoded "status/boxes/<label>.json" |
| -// key lands at "fleets/<name>/status/boxes/<label>.json" — outside the exact |
| -// "status/boxes/" prefix status.BuildFleet's loadBoxes lists, and so |
| -// invisible to the platform's own fleet.json by construction (pinned by |
| -// internal/status's contract test), with no change needed to the status |
| -// package itself. List/ListDirs strip the base back off their results, so a |
| -// key List hands back still round-trips through this SAME wrapper's Get/Put |
| -// without doubling the prefix. |
| -type prefixStore struct { |
| - release.Store |
| - base string // always "" or ends in "/" |
| -} |
| - |
| -// newPrefixStore wraps s so every key is rebased under prefix. An empty |
| -// prefix returns s unchanged — every platform box today leaves |
| -// CARLOS_STATUS_PREFIX unset, and this must be a pure no-op for them. |
| -func newPrefixStore(s release.Store, prefix string) release.Store { |
| - if prefix == "" { |
| - return s |
| - } |
| - if !strings.HasSuffix(prefix, "/") { |
| - prefix += "/" |
| - } |
| - return prefixStore{Store: s, base: prefix} |
| -} |
| - |
| -func (p prefixStore) full(key string) string { return p.base + key } |
| - |
| -func (p prefixStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| - return p.Store.Get(ctx, p.full(key)) |
| -} |
| - |
| -func (p prefixStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) { |
| - return p.Store.Put(ctx, p.full(key), body, ifMatch) |
| -} |
| - |
| -func (p prefixStore) Delete(ctx context.Context, key string) error { |
| - return p.Store.Delete(ctx, p.full(key)) |
| -} |
| - |
| -// List rebases the query prefix the same way Get/Put rebase a key, then |
| -// strips its own base back off every result — the underlying store's keys |
| -// always start with p.base (that's what "queried under p.full(prefix)" |
| -// guarantees), so TrimPrefix here exactly undoes full() above, and the |
| -// returned keys round-trip through this wrapper's Get unchanged. |
| -func (p prefixStore) List(ctx context.Context, prefix string) ([]string, error) { |
| - keys, err := p.Store.List(ctx, p.full(prefix)) |
| - if err != nil { |
| - return nil, err |
| - } |
| - out := make([]string, len(keys)) |
| - for i, k := range keys { |
| - out[i] = strings.TrimPrefix(k, p.base) |
| - } |
| - return out, nil |
| -} |
| - |
| -// ListDirs mirrors List's rebase-then-strip, for the delimiter-aware listing. |
| -func (p prefixStore) ListDirs(ctx context.Context, prefix string) ([]string, error) { |
| - dirs, err := p.Store.ListDirs(ctx, p.full(prefix)) |
| - if err != nil { |
| - return nil, err |
| - } |
| - out := make([]string, len(dirs)) |
| - for i, d := range dirs { |
| - out[i] = strings.TrimPrefix(d, p.base) |
| - } |
| - return out, nil |
| -} |
| - |
| // statusLoop runs the fleet status page's 5-minute tick, forever, once armed |
| // — see the package doc comment above for the full env contract. Missing |
| // CARLOS_STATUS_BUCKET disables the loop with one log line, the same |
| @@ -1086,9 +1019,10 @@ func statusLoop(reg *registry.Registry) { |
| } |
| // store is what statusTick actually reads/writes through: identical to s |
| // (a pure passthrough) unless CARLOS_STATUS_PREFIX is set, in which case |
| - // every key it touches is rebased under that prefix — see newPrefixStore |
| - // and the package doc comment's CARLOS_STATUS_PREFIX entry. |
| - store := newPrefixStore(s, prefix) |
| + // every key it touches is rebased under that prefix — see |
| + // release.NewPrefixStore and the package doc comment's |
| + // CARLOS_STATUS_PREFIX entry. |
| + store := release.NewPrefixStore(s, prefix) |
| // The effective bucket is worth a log line on its own: it's easy to set |
| // CARLOS_DEPLOYMENT_BUCKET and CARLOS_STATUS_BUCKET to different values |
| // by accident (or on purpose) and end up with the agent writing status |
diff --git a/internal/host/host_test.go b/internal/host/host_test.go| index 68e9343aa..a74e98e9f 100644 |
| --- a/internal/host/host_test.go |
| +++ b/internal/host/host_test.go |
| @@ -334,52 +334,11 @@ func TestShouldSkipPublisherElection(t *testing.T) { |
| } |
| } |
| -// TestPrefixStoreRebasesKeys pins newPrefixStore's own contract in |
| -// isolation, before the fleet-heartbeat integration test below composes it |
| -// with statusTick: Put/Get/List/ListDirs all operate in "unprefixed" key |
| -// space from the caller's side, while the underlying store actually holds |
| -// everything under the fixed base — and a key List hands back must |
| -// round-trip through the SAME wrapper's Get without doubling the prefix. |
| -func TestPrefixStoreRebasesKeys(t *testing.T) { |
| - ctx := context.Background() |
| - under := release.NewDirStore(t.TempDir()) |
| - wrapped := newPrefixStore(under, "fleets/pi/") |
| - |
| - if _, err := wrapped.Put(ctx, "status/boxes/pi-box.json", []byte(`{"ok":true}`), ""); err != nil { |
| - t.Fatal(err) |
| - } |
| - |
| - // The underlying store must hold the REBASED key, not the caller's one. |
| - if _, _, err := under.Get(ctx, "fleets/pi/status/boxes/pi-box.json"); err != nil { |
| - t.Errorf("underlying store missing rebased key: %v", err) |
| - } |
| - if _, _, err := under.Get(ctx, "status/boxes/pi-box.json"); err == nil { |
| - t.Error("underlying store must not hold the unrebased key") |
| - } |
| - |
| - // List, queried in the caller's unprefixed space, must find it and hand |
| - // back a key that still round-trips through the SAME wrapper's Get. |
| - keys, err := wrapped.List(ctx, "status/boxes/") |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - if len(keys) != 1 || keys[0] != "status/boxes/pi-box.json" { |
| - t.Fatalf("List = %v, want exactly [status/boxes/pi-box.json]", keys) |
| - } |
| - if body, _, err := wrapped.Get(ctx, keys[0]); err != nil || string(body) != `{"ok":true}` { |
| - t.Errorf("Get(%q) via the wrapper = %q, %v", keys[0], body, err) |
| - } |
| - |
| - // An empty prefix must be a pure passthrough: the same store, unwrapped. |
| - if s := newPrefixStore(under, ""); s != release.Store(under) { |
| - t.Errorf("newPrefixStore with empty prefix = %v, want the underlying store unchanged", s) |
| - } |
| -} |
| - |
| // TestFleetBoxHeartbeatLandsUnderFleetPrefixNeverPublishes is the end-to-end |
| -// pin for Task 5's whole contract, composing the pieces the two tests above |
| -// verify in isolation: a fleet box's tick (store wrapped in |
| -// CARLOS_STATUS_PREFIX="fleets/pi/", publish forced false the way |
| +// pin for Task 5's whole contract, composing release.NewPrefixStore (whose |
| +// own contract is pinned in isolation by internal/release's |
| +// TestNewPrefixStoreRebasesKeys) with statusTick: a fleet box's tick (store |
| +// wrapped in CARLOS_STATUS_PREFIX="fleets/pi/", publish forced false the way |
| // shouldSkipPublisherElection drives statusLoop) writes ITS OWN Box |
| // snapshot at "fleets/pi/status/boxes/<label>.json" in the underlying |
| // bucket — never at the shared "status/boxes/<label>.json" a platform box |
| @@ -394,7 +353,7 @@ func TestFleetBoxHeartbeatLandsUnderFleetPrefixNeverPublishes(t *testing.T) { |
| } |
| under := release.NewDirStore(t.TempDir()) |
| - fleetStore := newPrefixStore(under, "fleets/pi/") |
| + fleetStore := release.NewPrefixStore(under, "fleets/pi/") |
| ctx := context.Background() |
| probes := status.Probes{MachineID: "pi-box"} |
diff --git a/internal/host/metrics_loop.go b/internal/host/metrics_loop.go| index a0de7bd66..9a07cebae 100644 |
| --- a/internal/host/metrics_loop.go |
| +++ b/internal/host/metrics_loop.go |
| @@ -716,11 +716,22 @@ func metricsLoop() { |
| log.Printf("agent: metrics loop disabled: neither CARLOS_STATUS_BUCKET nor CARLOS_DEPLOYMENT_BUCKET is set") |
| return |
| } |
| - s, err := release.NewS3Store(ctx, bucket) |
| + raw, err := release.NewS3Store(ctx, bucket) |
| if err != nil { |
| log.Printf("agent: metrics loop disabled: %v", err) |
| return |
| } |
| + // s is what metricsFlush/alertTick actually read/write through: the same |
| + // CARLOS_STATUS_PREFIX rebase statusLoop applies (release.NewPrefixStore), |
| + // so a fleet box's metrics/alerts land under its own fleets/<name>/ |
| + // namespace instead of the shared metrics/boxes/ + alerts/boxes/ paths a |
| + // platform box uses — without this, a customer's raw metrics would |
| + // surface on the platform operator's monitoring page (console |
| + // metricsview lists metrics/boxes/) and an alert would page the |
| + // platform's own webhook for customer hardware. Empty prefix (every |
| + // platform box today) is a pure passthrough — see the package doc |
| + // comment's CARLOS_STATUS_PREFIX entry. |
| + s := release.NewPrefixStore(raw, os.Getenv("CARLOS_STATUS_PREFIX")) |
| box := status.Label(readMachineID()) |
| statePath := filepath.Join(dataDir(), "monitor", "alerts.json") |
diff --git a/internal/host/metrics_loop_test.go b/internal/host/metrics_loop_test.go| index 4e51b8c0b..c016b58d4 100644 |
| --- a/internal/host/metrics_loop_test.go |
| +++ b/internal/host/metrics_loop_test.go |
| @@ -88,6 +88,77 @@ func TestSamplerTickAndFlush(t *testing.T) { |
| } |
| } |
| +// ---- Task 5 (customer-fleets plan) fix round: metrics loop honors the fleet prefix ---- |
| + |
| +// TestMetricsLoopStoreHonorsFleetPrefix pins the review fix: metricsLoop's |
| +// store must be wrapped in release.NewPrefixStore(CARLOS_STATUS_PREFIX) |
| +// exactly like statusLoop's, so a fleet box's raw samples, day rollup, and |
| +// alert state land under its own fleets/<name>/ namespace — not the shared |
| +// metrics/boxes/ + alerts/boxes/ paths a platform box uses. Without this, a |
| +// customer's Pi would surface on the platform operator's monitoring page |
| +// (console metricsview lists metrics/boxes/) and a threshold breach would |
| +// page the platform's own alert webhook for customer hardware. Exercises |
| +// metricsFlush and alertTick through a release.NewPrefixStore-wrapped |
| +// store — the same composition metricsLoop's store-construction line now |
| +// produces — rather than reimplementing the wrapping. |
| +func TestMetricsLoopStoreHonorsFleetPrefix(t *testing.T) { |
| + ctx := context.Background() |
| + under := release.NewDirStore(t.TempDir()) |
| + s := release.NewPrefixStore(under, "fleets/pi/") |
| + box := "pi-box" |
| + |
| + // --- metricsFlush: raw chunk + day rollup land under the prefix ------- |
| + sm := &sampler{} |
| + at := time.Date(2026, 8, 2, 10, 59, 30, 0, time.UTC) |
| + mem, disk := 0.5, 0.3 |
| + probes := func(now time.Time) sampleProbes { |
| + return sampleProbes{ |
| + procStat: func() *metrics.CPUCounters { return nil }, |
| + loadavg: func() (*float64, *float64, *float64) { return nil, nil, nil }, |
| + memFrac: func() *float64 { return &mem }, |
| + diskFrac: func() *float64 { return &disk }, |
| + units: func() map[string]unitRaw { return nil }, |
| + now: func() time.Time { return now }, |
| + } |
| + } |
| + sm.tick(probes(at)) |
| + sm.tick(probes(at.Add(2 * time.Minute))) // crosses the 10 -> 11 hour boundary |
| + if err := metricsFlush(ctx, s, box, sm, at.Add(2*time.Minute)); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, _, err := under.Get(ctx, "fleets/pi/metrics/boxes/pi-box/raw/2026-08-02-10.json"); err != nil { |
| + t.Errorf("raw chunk not under the fleet prefix: %v", err) |
| + } |
| + if _, _, err := under.Get(ctx, "metrics/boxes/pi-box/raw/2026-08-02-10.json"); err == nil { |
| + t.Error("raw chunk leaked into the shared (unprefixed) metrics namespace") |
| + } |
| + if _, _, err := under.Get(ctx, "fleets/pi/metrics/boxes/pi-box/days/2026-08-02.json"); err != nil { |
| + t.Errorf("day rollup not under the fleet prefix: %v", err) |
| + } |
| + |
| + // --- alertTick: published alert state lands under the prefix too ------ |
| + rules := metrics.DefaultRules() |
| + st := &metrics.AlertState{V: 1, Box: box, Rules: map[string]metrics.RuleState{}} |
| + firing := 0.95 |
| + latest := metrics.Sample{At: at, Host: metrics.HostSample{DiskFrac: &firing}} |
| + statePath := filepath.Join(t.TempDir(), "monitor", "alerts.json") |
| + noPost := func(string, []byte) error { return nil } |
| + if err := alertTick(ctx, s, box, rules, st, latest, statePath, "", noPost); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, _, err := under.Get(ctx, "fleets/pi/"+metrics.AlertKey(box)); err != nil { |
| + t.Errorf("alert state not under the fleet prefix: %v", err) |
| + } |
| + if _, _, err := under.Get(ctx, metrics.AlertKey(box)); err == nil { |
| + t.Error("alert state leaked into the shared (unprefixed) alerts namespace — would page the platform webhook for customer hardware") |
| + } |
| + |
| + // --- empty prefix (every platform box today) is a pure passthrough ---- |
| + if got := release.NewPrefixStore(under, ""); got != release.Store(under) { |
| + t.Errorf("NewPrefixStore with empty prefix = %v, want the underlying store unchanged", got) |
| + } |
| +} |
| + |
| // TestMetricsFlushPrunesOldChunks pins the 48h prune: a raw chunk seeded |
| // well past the retention window must be gone after a flush that crosses an |
| // hour boundary (the only time pruning runs), while a fresh chunk survives. |
diff --git a/internal/release/store.go b/internal/release/store.go| index f8b5cbd7d..f7e2fd108 100644 |
| --- a/internal/release/store.go |
| +++ b/internal/release/store.go |
| @@ -51,6 +51,90 @@ type Store interface { |
| Delete(ctx context.Context, key string) error |
| } |
| +// ---- prefixStore ------------------------------------------------------------- |
| + |
| +// prefixStore rebases every key a caller works with onto a fixed base before |
| +// it ever reaches the underlying store — the actual enforcement point for a |
| +// namespacing env var like CARLOS_STATUS_PREFIX (customer-fleets plan, Task |
| +// 5: a fleet box's own heartbeat, written through one of these with base |
| +// "fleets/<name>/", lands at "fleets/<name>/status/boxes/<label>.json" |
| +// instead of the shared "status/boxes/<label>.json" every platform box also |
| +// writes — outside the exact prefix status.BuildFleet's loadBoxes lists, and |
| +// so invisible to the platform's own fleet.json by construction, with no |
| +// change needed to the status package itself). List/ListDirs strip the base |
| +// back off their results, so a key one of these hands back still round-trips |
| +// through the SAME wrapper's Get/Put without doubling the prefix. |
| +// |
| +// Exported from internal/release (rather than living next to its first |
| +// caller in internal/host) because it is a property of Store itself, not of |
| +// any one reader/writer of one: the console's fleet page (customer-fleets |
| +// plan, Task 9) needs to read a fleet box's snapshots through this same |
| +// rebasing, and a console-side package cannot reach an unexported type in |
| +// internal/host. |
| +type prefixStore struct { |
| + Store |
| + base string // always "" or ends in "/" |
| +} |
| + |
| +// NewPrefixStore wraps s so every key is rebased under prefix. An empty |
| +// prefix returns s UNCHANGED (not a prefixStore with an empty base) — every |
| +// deployment that never sets the namespacing env var (every platform box |
| +// today) must get back the exact same store, a pure no-op, verifiable by |
| +// interface equality. |
| +func NewPrefixStore(s Store, prefix string) Store { |
| + if prefix == "" { |
| + return s |
| + } |
| + if !strings.HasSuffix(prefix, "/") { |
| + prefix += "/" |
| + } |
| + return prefixStore{Store: s, base: prefix} |
| +} |
| + |
| +func (p prefixStore) full(key string) string { return p.base + key } |
| + |
| +func (p prefixStore) Get(ctx context.Context, key string) ([]byte, string, error) { |
| + return p.Store.Get(ctx, p.full(key)) |
| +} |
| + |
| +func (p prefixStore) Put(ctx context.Context, key string, body []byte, ifMatch string) (string, error) { |
| + return p.Store.Put(ctx, p.full(key), body, ifMatch) |
| +} |
| + |
| +func (p prefixStore) Delete(ctx context.Context, key string) error { |
| + return p.Store.Delete(ctx, p.full(key)) |
| +} |
| + |
| +// List rebases the query prefix the same way Get/Put rebase a key, then |
| +// strips its own base back off every result — the underlying store's keys |
| +// always start with p.base (that's what querying under p.full(prefix) |
| +// guarantees), so TrimPrefix here exactly undoes full() above, and the |
| +// returned keys round-trip through this wrapper's Get unchanged. |
| +func (p prefixStore) List(ctx context.Context, prefix string) ([]string, error) { |
| + keys, err := p.Store.List(ctx, p.full(prefix)) |
| + if err != nil { |
| + return nil, err |
| + } |
| + out := make([]string, len(keys)) |
| + for i, k := range keys { |
| + out[i] = strings.TrimPrefix(k, p.base) |
| + } |
| + return out, nil |
| +} |
| + |
| +// ListDirs mirrors List's rebase-then-strip, for the delimiter-aware listing. |
| +func (p prefixStore) ListDirs(ctx context.Context, prefix string) ([]string, error) { |
| + dirs, err := p.Store.ListDirs(ctx, p.full(prefix)) |
| + if err != nil { |
| + return nil, err |
| + } |
| + out := make([]string, len(dirs)) |
| + for i, d := range dirs { |
| + out[i] = strings.TrimPrefix(d, p.base) |
| + } |
| + return out, nil |
| +} |
| + |
| // ---- DirStore --------------------------------------------------------------- |
| // DirStore keeps objects as files under a root directory — the test double |
diff --git a/internal/release/store_test.go b/internal/release/store_test.go| index a7a5f7726..92a7ad66d 100644 |
| --- a/internal/release/store_test.go |
| +++ b/internal/release/store_test.go |
| @@ -240,3 +240,73 @@ func TestS3StoreLive(t *testing.T) { |
| t.Fatalf("ListDirs(%q) = %v, %v; want empty (the only child is an object)", parent, leaf, err) |
| } |
| } |
| + |
| +// ---- prefixStore ------------------------------------------------------------- |
| + |
| +// TestNewPrefixStoreRebasesKeys pins NewPrefixStore's own contract in |
| +// isolation (customer-fleets plan, Task 5 — moved here from internal/host, |
| +// its first caller, on review: it's a property of Store itself, needed by |
| +// any package that must read a namespaced store, not just statusLoop): |
| +// Put/Get/List/ListDirs all operate in "unprefixed" key space from the |
| +// caller's side, while the underlying store actually holds everything under |
| +// the fixed base — and a key List hands back must round-trip through the |
| +// SAME wrapper's Get without doubling the prefix. |
| +func TestNewPrefixStoreRebasesKeys(t *testing.T) { |
| + ctx := context.Background() |
| + under := NewDirStore(t.TempDir()) |
| + wrapped := NewPrefixStore(under, "fleets/pi/") |
| + |
| + if _, err := wrapped.Put(ctx, "status/boxes/pi-box.json", []byte(`{"ok":true}`), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // The underlying store must hold the REBASED key, not the caller's one. |
| + if _, _, err := under.Get(ctx, "fleets/pi/status/boxes/pi-box.json"); err != nil { |
| + t.Errorf("underlying store missing rebased key: %v", err) |
| + } |
| + if _, _, err := under.Get(ctx, "status/boxes/pi-box.json"); err == nil { |
| + t.Error("underlying store must not hold the unrebased key") |
| + } |
| + |
| + // List, queried in the caller's unprefixed space, must find it and hand |
| + // back a key that still round-trips through the SAME wrapper's Get. |
| + keys, err := wrapped.List(ctx, "status/boxes/") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(keys) != 1 || keys[0] != "status/boxes/pi-box.json" { |
| + t.Fatalf("List = %v, want exactly [status/boxes/pi-box.json]", keys) |
| + } |
| + if body, _, err := wrapped.Get(ctx, keys[0]); err != nil || string(body) != `{"ok":true}` { |
| + t.Errorf("Get(%q) via the wrapper = %q, %v", keys[0], body, err) |
| + } |
| + |
| + // An empty prefix must be a pure passthrough: the same store, unwrapped |
| + // — not a prefixStore with an empty base. Verified by interface |
| + // equality, not merely by observing identical behavior. |
| + if s := NewPrefixStore(under, ""); s != Store(under) { |
| + t.Errorf("NewPrefixStore with empty prefix = %v, want the underlying store unchanged", s) |
| + } |
| +} |
| + |
| +// TestPrefixStoreListDirsRebasesAndStrips is List's sibling test for the |
| +// delimiter-aware listing: ListDirs must see child prefixes queried under |
| +// the rebased key space, and hand results back stripped of the base, same |
| +// as List. |
| +func TestPrefixStoreListDirsRebasesAndStrips(t *testing.T) { |
| + ctx := context.Background() |
| + under := NewDirStore(t.TempDir()) |
| + wrapped := NewPrefixStore(under, "fleets/pi/") |
| + |
| + if _, err := wrapped.Put(ctx, "status/boxes/pi-box.json", []byte(`{}`), ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + dirs, err := wrapped.ListDirs(ctx, "status/") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if len(dirs) != 1 || dirs[0] != "status/boxes/" { |
| + t.Fatalf("ListDirs = %v, want exactly [status/boxes/]", dirs) |
| + } |
| +} |