amadan
cloud: reaper's "never claimed" signal is setup_complete, not member count (#442)
End-to-end verification on the box exposed a wrong premise in #434: a
freshly-created, NEVER-claimed space has ONE member, not zero. add-instance
pre-seeds the admin user so the setup invite has an identity to ship, and
claiming BECOMES that same user — so member count is 1 whether or not the space
was ever set up. The old `members == 0` check therefore never fired: the reaper
was armed but would never reclaim anything (safe, but non-functional).
Fix: the signal is `setup_complete` (absent until the admin finishes the setup
conversation, then "1"). instanceStatus now reads both setup_complete and the
member count; GET /instances/{host}/stats returns both; the reaper reclaims a
candidate only if it NEVER completed setup AND members <= 1 (nobody beyond the
seeded admin joined) — a belt-and-suspenders guard. Set-up / in-use / unreadable
spaces are still always skipped.
Verified against real box data: never-claimed space has setup_complete unset +
an unused setup invite; claimed spaces have setup_complete=1 + the invite
consumed.
Tests updated to the setup_complete signal (both cases now have 1 member; the
flag is the discriminator); instanceStatus + stats endpoint covered.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
4 files changed,
+108
−75
controld.go+6 −5docs/cloud-provisioner.md+8 −3reaper.go+47 −32reaper_test.go+47 −35
diff --git a/controld.go b/controld.go| index 17b8388..f4d812d 100644 |
| --- a/controld.go |
| +++ b/controld.go |
| @@ -89,16 +89,17 @@ func (c *controld) handler() http.Handler { |
| return mux |
| } |
| -// statsHandler reports a space's member count — the reaper's emptiness signal |
| -// (zero = never claimed). Read-only against the live instance DB. validHost |
| -// guards the DB path against traversal before it's joined with the data dir. |
| +// statsHandler reports whether a space was ever set up (setup_complete) plus its |
| +// member count — the reaper's "never claimed" signal. Read-only against the live |
| +// instance DB. validHost guards the DB path against traversal before it's joined |
| +// with the data dir. |
| func (c *controld) statsHandler(w http.ResponseWriter, r *http.Request) { |
| host := strings.ToLower(strings.TrimSpace(r.PathValue("host"))) |
| if !validHost(host) { |
| writeControlErr(w, http.StatusBadRequest, "invalid_host", "not a valid hostname") |
| return |
| } |
| - n, err := instanceMembers(host) |
| + members, setupComplete, err := instanceStatus(host) |
| if err != nil { |
| if os.IsNotExist(err) { |
| writeControlErr(w, http.StatusNotFound, "unknown_host", "no such instance") |
| @@ -108,7 +109,7 @@ func (c *controld) statsHandler(w http.ResponseWriter, r *http.Request) { |
| writeControlErr(w, http.StatusInternalServerError, "stats_failed", err.Error()) |
| return |
| } |
| - writeJSON(w, http.StatusOK, map[string]any{"host": host, "members": n}) |
| + writeJSON(w, http.StatusOK, map[string]any{"host": host, "members": members, "setup_complete": setupComplete}) |
| } |
| // deleteHandler deprovisions one instance (self-serve space deletion, reached |
diff --git a/docs/cloud-provisioner.md b/docs/cloud-provisioner.md| index a028d1f..ef43a07 100644 |
| --- a/docs/cloud-provisioner.md |
| +++ b/docs/cloud-provisioner.md |
| @@ -206,10 +206,15 @@ would otherwise sit there forever: empty, but holding a slot and a disk. The |
| disables it**; a value comfortably past the 7-day invite expiry, e.g. 14, is |
| the intent). |
| - For each candidate it asks the control daemon `GET /instances/{host}/stats`, |
| - which reads the instance DB **read-only** and returns its member count. Zero |
| - members = never claimed. **Anything non-empty, or whose count can't be read, is |
| + which reads the instance DB **read-only** and returns `setup_complete` + member |
| + count. The "never claimed" signal is **`setup_complete`, not member count**: |
| + `add-instance` pre-seeds the admin user so the setup invite has an identity, and |
| + claiming *becomes* that user — so a space has exactly one member whether or not |
| + it was ever set up. A candidate is reaped only if it **never completed setup** |
| + (`setup_complete` absent) AND no one beyond the seeded admin joined |
| + (`members <= 1`). **Anything set up, in use, or whose status can't be read is |
| left untouched** — a stats error is always a skip, never a delete. |
| -- An empty candidate is deprovisioned through the normal controld delete, which |
| +- A reaped candidate is deprovisioned through the normal controld delete, which |
| **archives** the DB/env to `/data/trash` (never destroys), so a false positive |
| is recoverable. Its ledger row is cleared, freeing the quota slot. |
| - `LCHAT_CLOUD_REAP_DRY_RUN=1` logs exactly what it *would* reclaim without |
diff --git a/reaper.go b/reaper.go| index 27b9d31..3a246eb 100644 |
| --- a/reaper.go |
| +++ b/reaper.go |
| @@ -18,41 +18,52 @@ import ( |
| // |
| // Self-serve creation hands out a slot against the capacity cap the moment a |
| // space is made. A space whose admin never completes setup — the setup invite |
| -// expires after 7 days — would otherwise sit there forever: empty, but holding a |
| -// slot and a disk. The reaper sweeps the cloud ledger for spaces older than a |
| -// threshold, asks the control daemon whether each is still empty (zero members = |
| -// never claimed), and deprovisions the empties. Deprovisioning ARCHIVES to |
| -// /data/trash (deleteInstance never destroys), so a false positive is |
| -// recoverable; a companion trash purge reclaims that archive after a longer |
| -// window. |
| +// expires after 7 days — would otherwise sit there forever, holding a slot and a |
| +// disk. The reaper sweeps the cloud ledger for spaces older than a threshold, |
| +// asks the control daemon whether each was ever set up (see instanceStatus — the |
| +// signal is `setup_complete`, NOT member count), and deprovisions the ones that |
| +// never were. Deprovisioning ARCHIVES to /data/trash (deleteInstance never |
| +// destroys), so a false positive is recoverable; a companion trash purge |
| +// reclaims that archive after a longer window. |
| // |
| // Both are opt-in (off unless their env is set) and only ever touch spaces IN |
| // the cloud ledger — operator-run instances (family.…, home, push, …) are never |
| // in it, so they can never be candidates. |
| -// ---- instance emptiness (controld-side, root) --------------------------------- |
| +// ---- "never claimed" signal (controld-side, root) ----------------------------- |
| -// instanceMembers counts the members in a space's DB. Opened READ-ONLY so it's |
| -// safe against the live instance — no migrations, no writes. A never-claimed |
| -// space has zero (its admin never finished setup, so no user was ever created). |
| -func instanceMembers(host string) (int, error) { |
| +// instanceStatus reads a space's claimed-ness from its DB, READ-ONLY (safe |
| +// against the live instance — no migrations, no writes). |
| +// |
| +// "Never claimed" is NOT "zero members": add-instance pre-seeds the admin user |
| +// so the setup invite has an identity to ship, and claiming BECOMES that same |
| +// user — so a space has exactly one member whether or not anyone ever set it up. |
| +// The real signal is `setup_complete`: absent until the admin finishes the setup |
| +// conversation (markSetupComplete), then "1". members is returned too, as a |
| +// belt-and-suspenders guard (don't reap if anyone beyond the seeded admin joined). |
| +func instanceStatus(host string) (members int, setupComplete bool, err error) { |
| path := filepath.Join(instanceDataDir, host+".db") |
| if _, err := os.Stat(path); err != nil { |
| - return 0, err // not-exist propagates → the caller answers unknown_host |
| + return 0, false, err // not-exist propagates → the caller answers unknown_host |
| } |
| conn, err := sql.Open("sqlite", "file:"+path+"?mode=ro&_pragma=busy_timeout(3000)") |
| if err != nil { |
| - return 0, err |
| + return 0, false, err |
| } |
| defer conn.Close() |
| conn.SetMaxOpenConns(1) |
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| defer cancel() |
| - var n int |
| - if err := conn.QueryRowContext(ctx, "SELECT COUNT(*) FROM users").Scan(&n); err != nil { |
| - return 0, err |
| + if err := conn.QueryRowContext(ctx, "SELECT COUNT(*) FROM users").Scan(&members); err != nil { |
| + return 0, false, err |
| + } |
| + var v string |
| + switch err := conn.QueryRowContext(ctx, "SELECT value FROM config WHERE key='setup_complete'").Scan(&v); err { |
| + case nil, sql.ErrNoRows: // absent = never set up |
| + default: |
| + return 0, false, err |
| } |
| - return n, nil |
| + return members, v == "1", nil |
| } |
| // ---- trash purge (controld-side, root) ---------------------------------------- |
| @@ -118,28 +129,30 @@ func runTrashPurge(keep time.Duration) { |
| // ---- the reaper (cloud-side) --------------------------------------------------- |
| -// controlStats asks the control daemon how many members a space has. |
| -func (s *cloudServer) controlStats(host string) (int, error) { |
| +// controlStatus asks the control daemon whether a space was ever set up (and its |
| +// member count, as a guard). |
| +func (s *cloudServer) controlStatus(host string) (members int, setupComplete bool, err error) { |
| req, err := http.NewRequest("GET", "http://control/instances/"+host+"/stats", nil) |
| if err != nil { |
| - return 0, err |
| + return 0, false, err |
| } |
| req.Header.Set("Authorization", "Bearer "+s.controlToken) |
| resp, err := s.control.Do(req) |
| if err != nil { |
| - return 0, err |
| + return 0, false, err |
| } |
| defer resp.Body.Close() |
| if resp.StatusCode != http.StatusOK { |
| - return 0, fmt.Errorf("control stats %s: HTTP %d", host, resp.StatusCode) |
| + return 0, false, fmt.Errorf("control stats %s: HTTP %d", host, resp.StatusCode) |
| } |
| var body struct { |
| - Members int `json:"members"` |
| + Members int `json:"members"` |
| + SetupComplete bool `json:"setup_complete"` |
| } |
| if err := json.NewDecoder(resp.Body).Decode(&body); err != nil { |
| - return 0, err |
| + return 0, false, err |
| } |
| - return body.Members, nil |
| + return body.Members, body.SetupComplete, nil |
| } |
| // reapLoop sweeps every interval; enabled only when reapAfter>0. |
| @@ -159,8 +172,10 @@ func (s *cloudServer) reapLoop(interval time.Duration) { |
| } |
| // reapOnce reclaims never-claimed spaces older than reapAfter. It deletes only a |
| -// space the control daemon confirms is EMPTY; anything unreachable or non-empty |
| -// is left untouched (a stats error is a skip, never a reap). Returns counts. |
| +// space the control daemon confirms was NEVER set up (setup_complete absent) AND |
| +// that nobody beyond the pre-seeded admin joined (members <= 1); anything set up, |
| +// in use, or whose status can't be read is left untouched (a stats error is a |
| +// skip, never a reap). Returns counts. |
| func (s *cloudServer) reapOnce(now time.Time) (reaped, kept int) { |
| cutoff := now.Add(-s.reapAfter).Unix() |
| cands, err := s.db.spacesOlderThan(cutoff) |
| @@ -169,18 +184,18 @@ func (s *cloudServer) reapOnce(now time.Time) (reaped, kept int) { |
| return 0, 0 |
| } |
| for _, c := range cands { |
| - members, err := s.controlStats(c.Host) |
| + members, setupComplete, err := s.controlStatus(c.Host) |
| if err != nil { |
| log.Printf("cloud: reaper: skip %s (stats unavailable): %v", c.Host, err) |
| continue |
| } |
| - if members > 0 { |
| + if setupComplete || members > 1 { |
| kept++ |
| - continue // claimed / in use — never reap |
| + continue // set up, or someone else joined — never reap |
| } |
| created := time.Unix(c.CreatedAt, 0).UTC().Format(time.RFC3339) |
| if s.reapDryRun { |
| - log.Printf("cloud: reaper: WOULD reclaim %s (id %d, 0 members, created %s)", c.Host, c.ID, created) |
| + log.Printf("cloud: reaper: WOULD reclaim %s (id %d, never set up, created %s)", c.Host, c.ID, created) |
| continue |
| } |
| status, body, err := s.controlDelete(c.Host) |
diff --git a/reaper_test.go b/reaper_test.go| index bc4abf7..6a41852 100644 |
| --- a/reaper_test.go |
| +++ b/reaper_test.go |
| @@ -93,26 +93,30 @@ func TestReapOnceReclaimsOnlyOldEmpties(t *testing.T) { |
| now := time.Unix(2_000_000_000, 0) |
| old := now.Add(-20 * 24 * time.Hour).Unix() |
| recent := now.Add(-1 * 24 * time.Hour).Unix() |
| - emptyID := insertSpaceRow(t, db, "empty.11ch.at", old) // old + 0 members → reaped |
| - claimedID := insertSpaceRow(t, db, "used.11ch.at", old) // old + members → kept |
| - insertSpaceRow(t, db, "young.11ch.at", recent) // too new → not a candidate |
| + // Both have the 1 pre-seeded admin (realistic); setup_complete is the signal. |
| + unclaimedID := insertSpaceRow(t, db, "unclaimed.11ch.at", old) // old + never set up → reaped |
| + claimedID := insertSpaceRow(t, db, "used.11ch.at", old) // old + set up → kept |
| + insertSpaceRow(t, db, "young.11ch.at", recent) // too new → not a candidate |
| deleted := map[string]bool{} |
| - sock := reapControl(t, map[string]int{"empty.11ch.at": 0, "used.11ch.at": 4}, deleted, nil) |
| + sock := reapControl(t, |
| + map[string]int{"unclaimed.11ch.at": 1, "used.11ch.at": 1}, |
| + map[string]bool{"used.11ch.at": true}, // only used.* completed setup |
| + deleted, nil) |
| s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour} |
| reaped, kept := s.reapOnce(now) |
| if reaped != 1 || kept != 1 { |
| t.Fatalf("reaped=%d kept=%d, want 1/1", reaped, kept) |
| } |
| - if !deleted["empty.11ch.at"] { |
| - t.Error("the old empty space should have been deleted") |
| + if !deleted["unclaimed.11ch.at"] { |
| + t.Error("the old never-set-up space should have been deleted") |
| } |
| if deleted["used.11ch.at"] { |
| - t.Error("a space with members must NEVER be reaped") |
| + t.Error("a space that completed setup must NEVER be reaped") |
| } |
| - // ledger: empty gone, used + young remain |
| - if _, ok, _ := db.spaceOwnedByID(t, emptyID); ok { |
| + // ledger: unclaimed gone, used + young remain |
| + if _, ok, _ := db.spaceOwnedByID(t, unclaimedID); ok { |
| t.Error("reaped space should be cleared from the ledger") |
| } |
| if _, ok, _ := db.spaceOwnedByID(t, claimedID); !ok { |
| @@ -124,10 +128,10 @@ func TestReapOnceDryRunDeletesNothing(t *testing.T) { |
| db := openTestCloudDB(t) |
| now := time.Unix(2_000_000_000, 0) |
| old := now.Add(-20 * 24 * time.Hour).Unix() |
| - insertSpaceRow(t, db, "empty.11ch.at", old) |
| + insertSpaceRow(t, db, "unclaimed.11ch.at", old) |
| deleted := map[string]bool{} |
| - sock := reapControl(t, map[string]int{"empty.11ch.at": 0}, deleted, nil) |
| + sock := reapControl(t, map[string]int{"unclaimed.11ch.at": 1}, nil, deleted, nil) |
| s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour, reapDryRun: true} |
| reaped, _ := s.reapOnce(now) |
| @@ -147,7 +151,7 @@ func TestReapOnceSkipsWhenStatsUnavailable(t *testing.T) { |
| deleted := map[string]bool{} |
| // stats endpoint errors for this host → the reaper must skip, never delete. |
| - sock := reapControl(t, map[string]int{}, deleted, map[string]bool{"flaky.11ch.at": true}) |
| + sock := reapControl(t, map[string]int{}, nil, deleted, map[string]bool{"flaky.11ch.at": true}) |
| s := &cloudServer{db: db, control: controlClient(sock), controlToken: "t", reapAfter: 14 * 24 * time.Hour} |
| reaped, kept := s.reapOnce(now) |
| @@ -159,45 +163,51 @@ func TestReapOnceSkipsWhenStatsUnavailable(t *testing.T) { |
| } |
| } |
| -// ---- instanceMembers + the controld stats endpoint ---------------------------- |
| +// ---- instanceStatus + the controld stats endpoint ----------------------------- |
| -func TestInstanceMembersAndStatsHandler(t *testing.T) { |
| +func TestInstanceStatusAndStatsHandler(t *testing.T) { |
| dir := t.TempDir() |
| prev := instanceDataDir |
| instanceDataDir = dir |
| defer func() { instanceDataDir = prev }() |
| - // A live instance DB with two members. |
| - db, err := openDB(filepath.Join(dir, "used.11ch.at.db")) |
| + // A set-up space: the pre-seeded admin + setup_complete='1'. |
| + used, err := openDB(filepath.Join(dir, "used.11ch.at.db")) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - defer db.sql.Close() |
| - if _, err := db.sql.Exec(`INSERT INTO users(id, created_at) VALUES('u1',1),('u2',2)`); err != nil { |
| + defer used.sql.Close() |
| + if _, err := used.sql.Exec(`INSERT INTO users(id, created_at) VALUES('admin',1)`); err != nil { |
| t.Fatal(err) |
| } |
| - // And an empty (never-claimed) one. |
| - empty, err := openDB(filepath.Join(dir, "empty.11ch.at.db")) |
| + if err := used.setSetupComplete(true); err != nil { |
| + t.Fatal(err) |
| + } |
| + // A never-set-up space: the pre-seeded admin, no setup_complete row. |
| + fresh, err := openDB(filepath.Join(dir, "fresh.11ch.at.db")) |
| if err != nil { |
| t.Fatal(err) |
| } |
| - defer empty.sql.Close() |
| + defer fresh.sql.Close() |
| + if _, err := fresh.sql.Exec(`INSERT INTO users(id, created_at) VALUES('admin',1)`); err != nil { |
| + t.Fatal(err) |
| + } |
| - if n, err := instanceMembers("used.11ch.at"); err != nil || n != 2 { |
| - t.Errorf("instanceMembers(used) = %d, %v; want 2", n, err) |
| + if m, sc, err := instanceStatus("used.11ch.at"); err != nil || m != 1 || !sc { |
| + t.Errorf("instanceStatus(used) = (%d, %v, %v); want (1, true, nil)", m, sc, err) |
| } |
| - if n, err := instanceMembers("empty.11ch.at"); err != nil || n != 0 { |
| - t.Errorf("instanceMembers(empty) = %d, %v; want 0", n, err) |
| + if m, sc, err := instanceStatus("fresh.11ch.at"); err != nil || m != 1 || sc { |
| + t.Errorf("instanceStatus(fresh) = (%d, %v, %v); want (1, false, nil)", m, sc, err) |
| } |
| - if _, err := instanceMembers("missing.11ch.at"); !os.IsNotExist(err) { |
| + if _, _, err := instanceStatus("missing.11ch.at"); !os.IsNotExist(err) { |
| t.Errorf("missing instance should be not-exist, got %v", err) |
| } |
| - // The stats endpoint, routed + auth-gated, reports the same. |
| + // The stats endpoint, routed + auth-gated, reports setup_complete. |
| c := &controld{tokenHash: hashToken("secret")} |
| srv := httptest.NewServer(c.handler()) |
| defer srv.Close() |
| - req, _ := http.NewRequest("GET", srv.URL+"/instances/used.11ch.at/stats", nil) |
| + req, _ := http.NewRequest("GET", srv.URL+"/instances/fresh.11ch.at/stats", nil) |
| req.Header.Set("Authorization", "Bearer secret") |
| resp, err := srv.Client().Do(req) |
| if err != nil { |
| @@ -208,15 +218,16 @@ func TestInstanceMembersAndStatsHandler(t *testing.T) { |
| t.Fatalf("stats HTTP %d", resp.StatusCode) |
| } |
| var body struct { |
| - Members int `json:"members"` |
| + Members int `json:"members"` |
| + SetupComplete bool `json:"setup_complete"` |
| } |
| _ = json.NewDecoder(resp.Body).Decode(&body) |
| - if body.Members != 2 { |
| - t.Errorf("stats members = %d, want 2", body.Members) |
| + if body.Members != 1 || body.SetupComplete { |
| + t.Errorf("stats = %+v, want members 1, setup_complete false", body) |
| } |
| // No token → 401. |
| - req2, _ := http.NewRequest("GET", srv.URL+"/instances/used.11ch.at/stats", nil) |
| + req2, _ := http.NewRequest("GET", srv.URL+"/instances/fresh.11ch.at/stats", nil) |
| resp2, _ := srv.Client().Do(req2) |
| if resp2.StatusCode != 401 { |
| t.Errorf("unauth stats = %d, want 401", resp2.StatusCode) |
| @@ -262,8 +273,9 @@ func (db *cloudDB) spaceOwnedByID(t *testing.T, id int64) (string, bool, error) |
| // reapControl serves a stand-in control daemon (on a socket, via the existing |
| // fakeControl helper) answering the reaper's stats + delete calls: GET stats |
| -// returns a member count (or 500 when statsErr[host]); DELETE records the host. |
| -func reapControl(t *testing.T, members map[string]int, deleted, statsErr map[string]bool) string { |
| +// returns member count + setup_complete (or 500 when statsErr[host]); DELETE |
| +// records the host. |
| +func reapControl(t *testing.T, members map[string]int, setup, deleted, statsErr map[string]bool) string { |
| t.Helper() |
| mux := http.NewServeMux() |
| mux.HandleFunc("GET /instances/{host}/stats", func(w http.ResponseWriter, r *http.Request) { |
| @@ -272,7 +284,7 @@ func reapControl(t *testing.T, members map[string]int, deleted, statsErr map[str |
| w.WriteHeader(http.StatusInternalServerError) |
| return |
| } |
| - writeJSON(w, http.StatusOK, map[string]any{"members": members[h]}) |
| + writeJSON(w, http.StatusOK, map[string]any{"members": members[h], "setup_complete": setup[h]}) |
| }) |
| mux.HandleFunc("DELETE /instances/{host}", func(w http.ResponseWriter, r *http.Request) { |
| deleted[r.PathValue("host")] = true |