amadan

carlos / platform Public

status: fix round 1 — exercise the real CLI, tighten fleet.json checks

Three review findings addressed:

- TestStatusCLI replaces the direct status.DryRun call with a subprocess
  test that builds the real carlos binary and runs `carlos status -dry-run
  <dir>` (asserts exit 0 + "dry-run: no leaks" on stdout) and bare `carlos
  status` (asserts non-zero exit) — the CLI wrapper's wiring and exit codes
  are now what's under test, not a duplicate of internal/status's own
  TestDryRunSelfChecks.
- The e2e's fleet.json check now decodes with json.Decoder +
  DisallowUnknownFields (stray top-level keys fail the test) and asserts
  GeneratedAt is non-zero, exactly one box labeled status.Label("e2e-box"),
  and instances.total equals the fixture's route count.
- README's config-surface header no longer claims all five env vars are
  read by the agent's status loop; STATUS_HOST/STATUS_PUBLIC's own bullet
  already said otherwise. Reworded to a plain header with per-var readers
  stated in each bullet.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 687c54bfc6f458597990ecb66cd103687297dc50 parent 2ca2550
2 files changed, +72 −19
  • README.md +11 −9
  • e2e_test.go +61 −10
diff --git a/README.md b/README.md
index 234606688..300b564df 100644
--- a/README.md
+++ b/README.md
@@ -44,17 +44,19 @@ never silently dropped. App names are opt-in only, via `carlos add
-status-named`, and everything else folds into one honest `apps_unlisted`
count, so the aggregate never lies by omission.
-**Config surface**, all read by `carlos agent`'s status loop; an unset
-`CARLOS_STATUS_BUCKET` disables the loop entirely — inert until configured,
-the same convention litestream replication uses:
+**Config surface** — every env var this feature reads, gathered here since
+no single reader owns the whole contract:
- `CARLOS_STATUS_BUCKET` / `CARLOS_STATUS_PREFIX` / `CARLOS_STATUS_REGION`
- — the bucket each box writes its snapshot to and, when it wins the lease,
- publishes the merged feed to; prefix and region also feed the cross-box
- status-lease election.
-- `STATUS_BOX_REGION` — an operator-set, coarse region word (e.g. `"EU ·
- Ireland"`) carried onto this box's snapshot verbatim; unset omits the
- field. Free text the operator chooses, never derived from cloud metadata.
+ — read by `carlos agent`'s status loop: the bucket each box writes its
+ snapshot to and, when it wins the lease, publishes the merged feed to;
+ prefix and region also feed the cross-box status-lease election. An
+ unset `CARLOS_STATUS_BUCKET` disables the whole loop — inert until
+ configured, the same convention litestream replication uses.
+- `STATUS_BOX_REGION` — also read by `carlos agent`'s status loop: an
+ operator-set, coarse region word (e.g. `"EU · Ireland"`) carried onto
+ this box's snapshot verbatim; unset omits the field. Free text the
+ operator chooses, never derived from cloud metadata.
- `STATUS_HOST` / `STATUS_PUBLIC` — not read by the agent process itself;
consumed by ops/tofu to decide whether the edge fronts the bucket's
`status/` prefix with a public hostname. `STATUS_PUBLIC=1` publishes the
diff --git a/e2e_test.go b/e2e_test.go
index 3607ebc8e..98e014010 100644
--- a/e2e_test.go
+++ b/e2e_test.go
@@ -358,10 +358,29 @@ func TestEndToEndSmoke(t *testing.T) {
if code != 200 {
t.Fatalf("GET status.local /fleet.json = %d, want 200", code)
}
+ // DisallowUnknownFields turns this decode into a schema check too: a
+ // stray top-level key (one Fleet doesn't declare) fails the test instead
+ // of silently vanishing into an unused field.
var decodedFleet status.Fleet
- if err := json.Unmarshal([]byte(body), &decodedFleet); err != nil {
+ dec := json.NewDecoder(strings.NewReader(body))
+ dec.DisallowUnknownFields()
+ if err := dec.Decode(&decodedFleet); err != nil {
t.Fatalf("decode status.local /fleet.json: %v\nbody: %s", err, body)
}
+ if decodedFleet.GeneratedAt.IsZero() {
+ t.Errorf("fleet.json generated_at is zero, want the publish tick's timestamp")
+ }
+ wantLabel := status.Label("e2e-box") // deterministic from the fixture MachineID above
+ if len(decodedFleet.Boxes) != 1 || decodedFleet.Boxes[0].Label != wantLabel {
+ t.Errorf("fleet.json boxes = %+v, want exactly one box labeled %q", decodedFleet.Boxes, wantLabel)
+ }
+ // Every route CollectBox saw falls into exactly one of static/resident/
+ // hibernating, so instances.total is always the route count — app.local
+ // (service), site.local (static), status.local (status): 3 routes, one box.
+ wantTotal := len(statusRoutes)
+ if got := decodedFleet.Instances["total"]; got == nil || *got != wantTotal {
+ t.Errorf("fleet.json instances.total = %v, want %d (one box over %d routes)", got, wantTotal, wantTotal)
+ }
// Binary channel pointer flow: ship v2 (same bytes — a rebuild), promote,
// then roll back; the pointer must return to v1 with v2 as the rollback
@@ -389,14 +408,46 @@ func TestEndToEndSmoke(t *testing.T) {
}
}
-// TestStatusDryRun exercises the CLI's `carlos status -dry-run <dir>` path.
-// RunStatus (cli.go) is three lines of flag parsing around status.DryRun —
-// the substance under test is DryRun itself, run the way an operator would
-// run it against a scratch directory before ever pointing the binary at a
-// real bucket: build a fixture fleet stuffed with needles, publish it, read
-// every object back, and fail loudly if anything leaked.
-func TestStatusDryRun(t *testing.T) {
- if err := status.DryRun(t.TempDir(), io.Discard); err != nil {
- t.Fatalf("status.DryRun: %v", err)
+// TestStatusCLI exercises the actual `carlos status` command line, not just
+// the function underneath it: builds the real carlos binary the same way
+// this file already builds testdata/echoapp, then runs it as a subprocess
+// exactly the way an operator would from a shell.
+//
+// - `carlos status -dry-run <dir>` must exit 0 and its stdout must carry
+// DryRun's "dry-run: no leaks" attestation — proving cli.go's RunStatus
+// is wired to internal/status.DryRun correctly. DryRun's own substance
+// (the fixture-needle leak scan) is already exercised directly by
+// TestDryRunSelfChecks in internal/status/status_test.go; duplicating
+// that here would just be the same assertion via a slower path, so this
+// test's job is only the wrapper's wiring and exit code.
+// - a bare `carlos status` (no -dry-run) must exit non-zero — RunStatus's
+// own input validation, not the pipeline it wraps.
+func TestStatusCLI(t *testing.T) {
+ goBin, err := exec.LookPath("go")
+ if err != nil {
+ t.Skip("go toolchain not found; skipping status CLI test")
+ }
+ tmp := t.TempDir()
+ carlosBin := filepath.Join(tmp, "carlos")
+ build := exec.Command(goBin, "build", "-o", carlosBin, ".")
+ build.Env = os.Environ()
+ if out, err := build.CombinedOutput(); err != nil {
+ t.Fatalf("build carlos: %v\n%s", err, out)
+ }
+
+ dryRunDir := filepath.Join(tmp, "dryrun")
+ if err := os.MkdirAll(dryRunDir, 0o755); err != nil {
+ t.Fatalf("mkdir dry-run dir: %v", err)
+ }
+ out, err := exec.Command(carlosBin, "status", "-dry-run", dryRunDir).CombinedOutput()
+ if err != nil {
+ t.Fatalf("carlos status -dry-run %s: %v\n%s", dryRunDir, err, out)
+ }
+ if !strings.Contains(string(out), "dry-run: no leaks") {
+ t.Errorf("carlos status -dry-run output = %q, want it to contain %q", out, "dry-run: no leaks")
+ }
+
+ if err := exec.Command(carlosBin, "status").Run(); err == nil {
+ t.Error("carlos status with no -dry-run flag: want a non-zero exit, got 0")
}
}