amadan

carlos / platform Public

fix: no-op attach neither audits nor bumps the serial (#151)

AttachClaim returned created=false both for a route converge (a real Put)
and for the pure no-op where the stored claim already said what the caller
asked for (no Put at all — claims.go returns before the write). The door
could not tell them apart, so it audited and bumped control/serial on
every attach. Spec §5 is "every successful WRITE audits and bumps", and
titogo's router re-attaches each hostname on every reload pass: no-op
attaches are the steady state, so each pass was burying the tenant's audit
log and sending the whole fleet to converge onto nothing.

AttachClaim now answers (created, changed, err): changed is true on a
create and on a converge, false on the no-op and every refusal. The door
gates audit + bumpSerial on changed, exactly as detach gates them on
removed. `created` — and therefore the wire response — is unchanged.

TestAPIDomainSerialBump walks attach, no-op re-attach, converge, effective
detach and no-op detach, asserting the serial AND the audit-row count at
each step; Task 1's claims tests pin changed=false on the idempotent case
and changed=true on the converge.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell cc37b37f2d7bca9281ae3f752e73af4539cfa02d parent 0d7ae2e
4 files changed, +120 −47
  • internal/console/apidomains.go +17 −8
  • internal/console/apidomains_test.go +58 −9
  • internal/domains/claims.go +20 −10
  • internal/domains/claims_test.go +25 −20
diff --git a/internal/console/apidomains.go b/internal/console/apidomains.go
index 1174720b2..c957f1fcd 100644
--- a/internal/console/apidomains.go
+++ b/internal/console/apidomains.go
@@ -154,7 +154,7 @@ func (s *Server) handleAPIDomainAttach(w http.ResponseWriter, r *http.Request) {
if !ok {
return
}
- created, err := domains.AttachClaim(ctx, s.cfg.Releases, domains.Claim{
+ created, changed, err := domains.AttachClaim(ctx, s.cfg.Releases, domains.Claim{
Host: host, Account: acct.Sqid, App: app, Route: route,
// UTC, like every other timestamp written to the bucket: these claims
// are read on boxes in other regions.
@@ -171,13 +171,22 @@ func (s *Server) handleAPIDomainAttach(w http.ResponseWriter, r *http.Request) {
jsonError(w, http.StatusBadGateway, "could not attach the hostname — try again")
return
}
- s.audit(acct, app, principal, "domain-attach", "attached hostname "+host+" -> "+route+" (cli)")
- // The claim is what a box's domains pass mints an alias row from, so this
- // is a box-consumed write (carlos-deploy §3): announce it, and the
- // hostname is serving seconds after the call instead of on the hour.
- // Strictly after the write and the audit — a serial that moved for a
- // write that failed would send every box looking for nothing.
- s.bumpSerial(ctx)
+ // Only an actual WRITE audits and announces (spec §5), the same rule
+ // detach observes below. `created` is the caller's answer; `changed` is
+ // this one, and they differ on the converge — a re-attach that repointed
+ // the route created nothing but must reach the fleet. The no-op case is
+ // not hypothetical: titogo's router re-attaches every hostname on each
+ // reload pass, so a bump per call would keep every box converging onto
+ // nothing and an audit row per call would bury the tenant's log.
+ if changed {
+ s.audit(acct, app, principal, "domain-attach", "attached hostname "+host+" -> "+route+" (cli)")
+ // The claim is what a box's domains pass mints an alias row from, so
+ // this is a box-consumed write (carlos-deploy §3): announce it, and
+ // the hostname is serving seconds after the call instead of on the
+ // hour. Strictly after the write and the audit — a serial that moved
+ // for a write that failed would send every box looking for nothing.
+ s.bumpSerial(ctx)
+ }
writeJSON(w, http.StatusOK, map[string]any{"host": host, "route": route, "created": created})
}
diff --git a/internal/console/apidomains_test.go b/internal/console/apidomains_test.go
index b67acfe0f..38316c041 100644
--- a/internal/console/apidomains_test.go
+++ b/internal/console/apidomains_test.go
@@ -185,7 +185,7 @@ func TestAPIDomainAttachIdempotent(t *testing.T) {
// the holder is in this deployment at all.
func TestAPIDomainAttachConflictNeutral(t *testing.T) {
fx, member := instanceFixture(t, "code-dom-conflict")
- if _, err := domains.AttachClaim(context.Background(), fx.rel, domains.Claim{
+ if _, _, err := domains.AttachClaim(context.Background(), fx.rel, domains.Claim{
Host: "tickets.example.com", Account: "cdef", App: "widgets",
Route: "widgets.example.net", CreatedAt: time.Now().UTC().Format(time.RFC3339),
}); err != nil {
@@ -278,7 +278,7 @@ func TestAPIDomainAttachRouteDefault(t *testing.T) {
func TestAPIDomainDetachQuiet(t *testing.T) {
fx, member := instanceFixture(t, "code-dom-detach")
attachDomain(t, fx, member, "tickets.example.com", "go.tito.io")
- if _, err := domains.AttachClaim(context.Background(), fx.rel, domains.Claim{
+ if _, _, err := domains.AttachClaim(context.Background(), fx.rel, domains.Claim{
Host: "theirs.example.com", Account: "cdef", App: "widgets",
Route: "widgets.example.net", CreatedAt: time.Now().UTC().Format(time.RFC3339),
}); err != nil {
@@ -337,7 +337,7 @@ func TestAPIDomainList(t *testing.T) {
attachDomain(t, fx, member, "live.example.com", "go.tito.io")
attachDomain(t, fx, member, "pending.example.com", "go.tito.io")
attachDomain(t, fx, member, "stale.example.com", "go.tito.io")
- if _, err := domains.AttachClaim(ctx, fx.rel, domains.Claim{
+ if _, _, err := domains.AttachClaim(ctx, fx.rel, domains.Claim{
Host: "elsewhere.example.com", Account: "bab", App: "other",
Route: "other.go.tito.io", CreatedAt: now.UTC().Format(time.RFC3339),
}); err != nil {
@@ -398,9 +398,29 @@ func TestAPIDomainList(t *testing.T) {
}
}
+// domainEvents counts this app's audit rows whose action starts "domain-".
+func domainEvents(t *testing.T, fx *consoleFixture) int {
+ t.Helper()
+ events, err := fx.st.Events(OpsAccountID, "titogo", 50)
+ if err != nil {
+ t.Fatal(err)
+ }
+ n := 0
+ for _, e := range events {
+ if strings.HasPrefix(e.Action, "domain-") {
+ n++
+ }
+ }
+ return n
+}
+
// TestAPIDomainSerialBump: boxes converge off control/serial, so every
-// EFFECTIVE write announces itself — and a no-op detach, which changed
-// nothing a box reads, does not.
+// EFFECTIVE write announces itself — and the two no-ops, a re-attach that
+// changed nothing and a detach that removed nothing, announce nothing and
+// write no audit row. That matters more than it looks: titogo's router
+// re-attaches its hostnames on every reload pass, so redundant attaches are
+// the STEADY STATE — a bump per pass would keep the whole fleet converging
+// onto nothing, and an audit row per pass would bury the tenant's log.
func TestAPIDomainSerialBump(t *testing.T) {
fx, member := instanceFixture(t, "code-dom-serial")
if got := serial(t, fx); got != 0 {
@@ -410,18 +430,47 @@ func TestAPIDomainSerialBump(t *testing.T) {
if got := serial(t, fx); got != 1 {
t.Fatalf("serial after an attach = %d, want 1", got)
}
+ if got := domainEvents(t, fx); got != 1 {
+ t.Fatalf("audit rows after an attach = %d, want 1", got)
+ }
+ // The router's steady state: the same attach again, changing nothing.
+ if got := attachDomain(t, fx, member, "tickets.example.com", "go.tito.io"); got.Created {
+ t.Fatalf("no-op re-attach = %+v, want created false", got)
+ }
+ if got := serial(t, fx); got != 1 {
+ t.Fatalf("serial after a no-op re-attach = %d, want it unmoved at 1", got)
+ }
+ if got := domainEvents(t, fx); got != 1 {
+ t.Fatalf("audit rows after a no-op re-attach = %d, want it unmoved at 1", got)
+ }
+ // A converge IS a write: new route, so the fleet hears about it.
+ if got := attachDomain(t, fx, member, "tickets.example.com", "two.go.tito.io"); got.Created {
+ t.Fatalf("converge attach = %+v, want created false", got)
+ }
+ if got := serial(t, fx); got != 2 {
+ t.Fatalf("serial after a converge = %d, want 2", got)
+ }
+ if got := domainEvents(t, fx); got != 2 {
+ t.Fatalf("audit rows after a converge = %d, want 2", got)
+ }
if code, _, out := rawReq(t, http.MethodDelete,
domainsURL(fx, "bab", "titogo")+"/tickets.example.com", member, ""); code != http.StatusOK {
t.Fatalf("detach = %d %s, want 200", code, out)
}
- if got := serial(t, fx); got != 2 {
- t.Fatalf("serial after an effective detach = %d, want 2", got)
+ if got := serial(t, fx); got != 3 {
+ t.Fatalf("serial after an effective detach = %d, want 3", got)
+ }
+ if got := domainEvents(t, fx); got != 3 {
+ t.Fatalf("audit rows after an effective detach = %d, want 3", got)
}
if code, _, out := rawReq(t, http.MethodDelete,
domainsURL(fx, "bab", "titogo")+"/never.example.com", member, ""); code != http.StatusOK {
t.Fatalf("no-op detach = %d %s, want 200", code, out)
}
- if got := serial(t, fx); got != 2 {
- t.Fatalf("serial after a no-op detach = %d, want it unmoved at 2", got)
+ if got := serial(t, fx); got != 3 {
+ t.Fatalf("serial after a no-op detach = %d, want it unmoved at 3", got)
+ }
+ if got := domainEvents(t, fx); got != 3 {
+ t.Fatalf("audit rows after a no-op detach = %d, want it unmoved at 3", got)
}
}
diff --git a/internal/domains/claims.go b/internal/domains/claims.go
index d2689d59a..7b00329c4 100644
--- a/internal/domains/claims.go
+++ b/internal/domains/claims.go
@@ -92,30 +92,40 @@ func GetClaim(ctx context.Context, s release.Store, host string) (*Claim, error)
// key IS the cross-account uniqueness gate. Same owner re-attaching is
// idempotent (converging Route in place); any other owner gets
// ErrHostUnavailable — deliberately neutral, never naming the holder.
-func AttachClaim(ctx context.Context, s release.Store, c Claim) (bool, error) {
+//
+// created reports that this call brought the claim into existence; changed
+// reports that it WROTE — a create or a route converge, and false for the
+// pure no-op where the stored claim already said what the caller asked for.
+// The two are separate answers because they drive different things: created
+// is what the door tells the caller, changed is what the door audits and
+// bumps control/serial on. Callers that re-attach on a loop (titogo's router
+// re-attaches every hostname on each reload pass) make no-op attaches the
+// steady state, and a serial that moved for a call that wrote nothing would
+// send every box in the fleet looking for a change that isn't there.
+func AttachClaim(ctx context.Context, s release.Store, c Claim) (created, changed bool, err error) {
c.Schema = 1
body, err := json.Marshal(c)
if err != nil {
- return false, err
+ return false, false, err
}
for attempt := 0; ; attempt++ {
cur, etag, err := getClaimETag(ctx, s, c.Host)
if errors.Is(err, os.ErrNotExist) {
if _, err := s.Put(ctx, ClaimKey(c.Host), body, ""); err == nil {
- return true, nil
+ return true, true, nil
} else if errors.Is(err, release.ErrCASConflict) && attempt == 0 {
continue // lost the create race — re-read and judge the winner
} else {
- return false, err
+ return false, false, err
}
} else if err != nil {
- return false, err
+ return false, false, err
}
if cur.Account != c.Account || cur.App != c.App {
- return false, ErrHostUnavailable
+ return false, false, ErrHostUnavailable
}
if cur.Route == c.Route {
- return false, nil
+ return false, false, nil // nothing to write: this call is a no-op
}
// Converge repoints Route and nothing else: the stored claim keeps
// its CreatedAt (the claim's age) and Wildcard (TLS opt-in state a
@@ -125,14 +135,14 @@ func AttachClaim(ctx context.Context, s release.Store, c Claim) (bool, error) {
next.Route = c.Route
nbody, err := json.Marshal(next)
if err != nil {
- return false, err
+ return false, false, err
}
if _, err := s.Put(ctx, ClaimKey(c.Host), nbody, etag); err == nil {
- return false, nil
+ return false, true, nil // not a create, but a real write
} else if errors.Is(err, release.ErrCASConflict) && attempt == 0 {
continue // converge race — one retry, then surface
} else {
- return false, err
+ return false, false, err
}
}
}
diff --git a/internal/domains/claims_test.go b/internal/domains/claims_test.go
index e4f08a64f..078f10ed4 100644
--- a/internal/domains/claims_test.go
+++ b/internal/domains/claims_test.go
@@ -13,12 +13,12 @@ import (
func TestAttachClaimCreates(t *testing.T) {
s := release.NewDirStore(t.TempDir())
ctx := context.Background()
- created, err := AttachClaim(ctx, s, Claim{
+ created, changed, err := AttachClaim(ctx, s, Claim{
Host: "tickets.example.com", Account: "bab", App: "kass",
Route: "canary/live", CreatedAt: "2026-08-12T00:00:00Z", CreatedBy: "paul@tito.io",
})
- if err != nil || !created {
- t.Fatalf("AttachClaim: created=%v err=%v, want true nil", created, err)
+ if err != nil || !created || !changed {
+ t.Fatalf("AttachClaim: created=%v changed=%v err=%v, want true true nil", created, changed, err)
}
got, err := GetClaim(ctx, s, "tickets.example.com")
if err != nil {
@@ -35,16 +35,19 @@ func TestAttachClaimIdempotent(t *testing.T) {
s := release.NewDirStore(t.TempDir())
ctx := context.Background()
claim := Claim{Host: "tickets.example.com", Account: "bab", App: "kass", Route: "canary/live"}
- if _, err := AttachClaim(ctx, s, claim); err != nil {
+ if _, _, err := AttachClaim(ctx, s, claim); err != nil {
t.Fatal(err)
}
before, err := GetClaim(ctx, s, claim.Host)
if err != nil {
t.Fatal(err)
}
- created, err := AttachClaim(ctx, s, claim)
- if err != nil || created {
- t.Fatalf("AttachClaim re-attach: created=%v err=%v, want false nil", created, err)
+ // changed=false is what makes this a NO-OP rather than a write: the door
+ // audits and bumps control/serial off that answer, and a re-attach on
+ // every router reload pass is the steady state, not the exception.
+ created, changed, err := AttachClaim(ctx, s, claim)
+ if err != nil || created || changed {
+ t.Fatalf("AttachClaim re-attach: created=%v changed=%v err=%v, want false false nil", created, changed, err)
}
after, err := GetClaim(ctx, s, claim.Host)
if err != nil {
@@ -60,16 +63,18 @@ func TestAttachClaimConvergesRoute(t *testing.T) {
ctx := context.Background()
claim := Claim{Host: "tickets.example.com", Account: "bab", App: "kass", Route: "canary/live",
CreatedAt: "2026-08-12T00:00:00Z", Wildcard: true, Catchall: true}
- if _, err := AttachClaim(ctx, s, claim); err != nil {
+ if _, _, err := AttachClaim(ctx, s, claim); err != nil {
t.Fatal(err)
}
// A converge repoints Route and nothing else: the caller's claim here
// deliberately carries no CreatedAt and no Wildcard, and both stored
// values must survive — a route repoint must never strip another
- // branch's TLS opt-in or restamp the claim's age.
- created, err := AttachClaim(ctx, s, Claim{Host: claim.Host, Account: "bab", App: "kass", Route: "canary/next"})
- if err != nil || created {
- t.Fatalf("AttachClaim route change: created=%v err=%v, want false nil", created, err)
+ // branch's TLS opt-in or restamp the claim's age. created=false but
+ // changed=TRUE: a converge IS a store write, so the door must audit it
+ // and announce it to the fleet.
+ created, changed, err := AttachClaim(ctx, s, Claim{Host: claim.Host, Account: "bab", App: "kass", Route: "canary/next"})
+ if err != nil || created || !changed {
+ t.Fatalf("AttachClaim route change: created=%v changed=%v err=%v, want false true nil", created, changed, err)
}
got, err := GetClaim(ctx, s, claim.Host)
if err != nil {
@@ -88,7 +93,7 @@ func TestAttachClaimOtherOwnerRefused(t *testing.T) {
s := release.NewDirStore(t.TempDir())
ctx := context.Background()
claim := Claim{Host: "tickets.example.com", Account: "bab", App: "kass", Route: "canary/live"}
- if _, err := AttachClaim(ctx, s, claim); err != nil {
+ if _, _, err := AttachClaim(ctx, s, claim); err != nil {
t.Fatal(err)
}
before, err := GetClaim(ctx, s, claim.Host)
@@ -98,13 +103,13 @@ func TestAttachClaimOtherOwnerRefused(t *testing.T) {
other := claim
other.Account = "bad"
- if _, err := AttachClaim(ctx, s, other); !errors.Is(err, ErrHostUnavailable) {
+ if _, _, err := AttachClaim(ctx, s, other); !errors.Is(err, ErrHostUnavailable) {
t.Fatalf("other account: err = %v, want ErrHostUnavailable", err)
}
otherApp := claim
otherApp.App = "console"
- if _, err := AttachClaim(ctx, s, otherApp); !errors.Is(err, ErrHostUnavailable) {
+ if _, _, err := AttachClaim(ctx, s, otherApp); !errors.Is(err, ErrHostUnavailable) {
t.Fatalf("other app: err = %v, want ErrHostUnavailable", err)
}
@@ -124,7 +129,7 @@ func TestAttachClaimNewerSchemaRefused(t *testing.T) {
if _, err := s.Put(ctx, ClaimKey("tickets.example.com"), body, ""); err != nil {
t.Fatal(err)
}
- _, err := AttachClaim(ctx, s, Claim{Host: "tickets.example.com", Account: "bab", App: "kass"})
+ _, _, err := AttachClaim(ctx, s, Claim{Host: "tickets.example.com", Account: "bab", App: "kass"})
if err == nil || !strings.Contains(err.Error(), "schema") {
t.Fatalf("AttachClaim over newer schema: err = %v, want error mentioning schema", err)
}
@@ -141,7 +146,7 @@ func TestDetachClaimRemoves(t *testing.T) {
s := release.NewDirStore(t.TempDir())
ctx := context.Background()
claim := Claim{Host: "tickets.example.com", Account: "bab", App: "kass"}
- if _, err := AttachClaim(ctx, s, claim); err != nil {
+ if _, _, err := AttachClaim(ctx, s, claim); err != nil {
t.Fatal(err)
}
removed, err := DetachClaim(ctx, s, claim.Host, claim.Account, claim.App)
@@ -166,7 +171,7 @@ func TestDetachClaimOtherOwnerQuiet(t *testing.T) {
s := release.NewDirStore(t.TempDir())
ctx := context.Background()
claim := Claim{Host: "tickets.example.com", Account: "bab", App: "kass"}
- if _, err := AttachClaim(ctx, s, claim); err != nil {
+ if _, _, err := AttachClaim(ctx, s, claim); err != nil {
t.Fatal(err)
}
removed, err := DetachClaim(ctx, s, claim.Host, "bad", "console")
@@ -181,10 +186,10 @@ func TestDetachClaimOtherOwnerQuiet(t *testing.T) {
func TestListClaimsSkipsGarbage(t *testing.T) {
s := release.NewDirStore(t.TempDir())
ctx := context.Background()
- if _, err := AttachClaim(ctx, s, Claim{Host: "a.example.com", Account: "bab", App: "kass"}); err != nil {
+ if _, _, err := AttachClaim(ctx, s, Claim{Host: "a.example.com", Account: "bab", App: "kass"}); err != nil {
t.Fatal(err)
}
- if _, err := AttachClaim(ctx, s, Claim{Host: "b.example.com", Account: "bad", App: "console"}); err != nil {
+ if _, _, err := AttachClaim(ctx, s, Claim{Host: "b.example.com", Account: "bad", App: "console"}); err != nil {
t.Fatal(err)
}
if _, err := s.Put(ctx, ClaimKey("garbage.example.com"), []byte("not json"), ""); err != nil {