carlos / platform Public

console: resume a stale cap pause for every sender on a status read, not just the app sender (plan 2, task 13 review fix)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by claude agent, operated by paul@keymail.dev 8be533bec7f00d0b90ab6e87510dfe8be9a84df2 parent 7ccfa8a
2 files changed, +147 −13
  • internal/console/apimail.go +31 −11
  • internal/console/apimail_test.go +116 −2
diff --git a/internal/console/apimail.go b/internal/console/apimail.go
index 899e358ab..cfee73b3f 100644
--- a/internal/console/apimail.go
+++ b/internal/console/apimail.go
@@ -374,7 +374,7 @@ func dnsRecordsOut(recs []dnsrec.Record) []dnsRecordOut {
// record differently.
//
// standalone, domains and senders are NEVER null, matching storeReply's
-// env_names — a client reads them rather than testing for absence. Neither
+// env_names. a client reads them rather than testing for absence. Neither
// carries a password: mail.Standalone stores a name, an IAM user and a key
// id, never the secret (see that type's doc comment); a domain record
// carries DKIM tokens and ARNs, not credentials.
@@ -696,21 +696,41 @@ func (s *Server) handleAPIEmailStatus(w http.ResponseWriter, r *http.Request) {
return
}
// R2 (review follow-up): resume a stale CAP pause on the status READ
- // path, not only on the SES event path. A paused app's IAM key is
+ // path, not only on the SES event path. A paused sender's IAM key is
// deactivated, so it sends nothing, so no events ever arrive to run
- // enforceMailLimits' day-roll check — a member running `carlos email
- // status` is exactly the moment a rolled-over daily budget should
- // clear. Shares resumeStaleCapPause with enforceMailLimits so the two
- // can never drift; a RATE pause, a PER-INSTANCE pause or a kind-less
+ // enforceMailLimits' day-roll check for it either. a member running
+ // `carlos email status` is exactly the moment a rolled-over daily budget
+ // should clear. Shares resumeStaleCapPause with enforceMailLimits so the
+ // two can never drift; a RATE pause, a PER-INSTANCE pause or a kind-less
// pause is left untouched by that function, fail-safe.
- sn, _, err := mail.GetSender(ctx, s.cfg.Releases, acct.Sqid, app, mail.SenderAppID)
+ //
+ // EVERY sender gets this, not only the app sender (review round 2, task
+ // 13). This reply now names every sender's active and pause_kind, and a
+ // cap-paused non-app sender sends nothing either, so it never generates
+ // the SES event that would otherwise resume it. step 5's per-customer
+ // instance senders would each pause at their own cap and stay paused
+ // forever without this.
+ senders, err := mail.ListSenders(ctx, s.cfg.Releases, acct.Sqid, app)
+ appResumed := false
if err == nil {
- if _, resumed, rerr := s.resumeStaleCapPause(ctx, acct.Sqid, app, mail.SenderAppID, sn, time.Now()); rerr != nil {
- err = rerr
- } else if resumed {
- rec, _, err = mail.Get(ctx, s.cfg.Releases, acct.Sqid, app)
+ for _, sn := range senders {
+ _, resumed, rerr := s.resumeStaleCapPause(ctx, acct.Sqid, app, sn.ID, sn, time.Now())
+ if rerr != nil {
+ err = rerr
+ break
+ }
+ if resumed && sn.ID == mail.SenderAppID {
+ appResumed = true
+ }
}
}
+ if err == nil && appResumed {
+ // The top-level fields describe the app sender's own record, so a
+ // resume of it (not a sibling) has to be re-read before emailReply
+ // renders them. resumeStaleCapPause hands back a fresh sender, not a
+ // patched copy, and rec is the Record view of that same object.
+ rec, _, err = mail.Get(ctx, s.cfg.Releases, acct.Sqid, app)
+ }
if err != nil {
log.Printf("console: api email status resume %s/%s: %v", acct.Sqid, app, err)
jsonError(w, http.StatusBadGateway, "could not read the email record — try again")
diff --git a/internal/console/apimail_test.go b/internal/console/apimail_test.go
index 706b69a65..0dad941d3 100644
--- a/internal/console/apimail_test.go
+++ b/internal/console/apimail_test.go
@@ -476,6 +476,120 @@ func TestAPIEmailStatusLeavesARatePauseAlone(t *testing.T) {
}
}
+// findSender pulls one sender's entry out of a decoded status reply's
+// senders array by id. Over HTTP the array decodes as []any of
+// map[string]any, unlike the in-process emailReply callers that see
+// []map[string]any directly.
+func findSender(t *testing.T, out map[string]any, id string) map[string]any {
+ t.Helper()
+ raw, _ := out["senders"].([]any)
+ for _, r := range raw {
+ if m, ok := r.(map[string]any); ok && m["id"] == id {
+ return m
+ }
+ }
+ t.Fatalf("no sender %q in %#v", id, out["senders"])
+ return nil
+}
+
+// putSiblingSender writes a second, non-app sender straight to its own
+// object, the way step 5's per-customer instance enable will, already
+// carrying a pause so a status-read test can seed one without going through
+// a walk that does not exist yet.
+func putSiblingSender(t *testing.T, fx *consoleFixture, app, pauseKind string, pausedAt time.Time) {
+ t.Helper()
+ if _, err := mail.PutSender(context.Background(), fx.rel, release.OpsAccount, app, &mail.Sender{
+ ID: "book.example.com", Scope: mail.ScopeInstance, Source: mail.SourceDomain, Host: "book.example.com",
+ Declared: &mail.Declared{EnvPrefix: "TITOGO_SMTP", From: "noreply@book.example.com", At: pausedAt, By: "o@example.test"},
+ Grant: &mail.GrantBlock{Domains: []string{"book.example.com"}, DailyCap: 50, Source: "tier:free", At: pausedAt, By: "tier"},
+ Provisioned: &mail.Provisioned{
+ IAMUser: "sibling-user", KeyID: "AKIA-SIB", Active: false,
+ PauseKind: pauseKind, At: pausedAt,
+ },
+ }, ""); err != nil {
+ t.Fatal(err)
+ }
+}
+
+// TestAPIEmailStatusResumesAStaleCapPauseOnANonAppSender: task 13 review,
+// round 2. The event path (enforceMailLimits) never runs for a sender that
+// is already paused, because a paused sender's IAM key sends nothing and so
+// generates no SES event to run it. The status door's own resume used to
+// look at the app sender only; it must do the same day-roll check for every
+// sender it is about to report, or a per-customer instance sender (step 5)
+// would pause at its cap and never resume, since nothing else ever reads it.
+func TestAPIEmailStatusResumesAStaleCapPauseOnANonAppSender(t *testing.T) {
+ fx, fake, _, token := emailArmed(t, "code-email-status-resume-sibling")
+ app, domain := "titogo", "titogo.bab.oncarlos.com"
+ provisionEmail(t, fx, fake, token, app, domain, "hello@"+domain)
+ putSiblingSender(t, fx, app, mail.PauseKindCap, time.Now().AddDate(0, 0, -1))
+
+ status, out := getJSON(t, emailURL(fx, app), token)
+ if status != http.StatusOK {
+ t.Fatalf("status = %d %v", status, out)
+ }
+
+ // The app sender's own top-level fields are exactly as before: the
+ // sibling's pause and resume must not touch them.
+ if out["active"] != true || out["daily_cap"] != float64(50) {
+ t.Fatalf("the top-level fields moved: active=%v daily_cap=%v", out["active"], out["daily_cap"])
+ }
+
+ sibling := findSender(t, out, "book.example.com")
+ if sibling["active"] != true {
+ t.Fatalf("sibling active = %v, want true. a cap pause from yesterday must clear on a status read, same as the app sender", sibling["active"])
+ }
+ if _, ok := sibling["pause_kind"]; ok {
+ t.Fatalf("sibling pause_kind = %v, want absent once resumed", sibling["pause_kind"])
+ }
+
+ sn, _, err := mail.GetSender(context.Background(), fx.rel, release.OpsAccount, app, "book.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if sn.Provisioned == nil || !sn.Provisioned.Active {
+ t.Fatal("the sibling sender on disk is still paused after the status read resumed it")
+ }
+ if sn.Provisioned.PauseKind != "" {
+ t.Fatalf("PauseKind = %q on disk after resume, want cleared", sn.Provisioned.PauseKind)
+ }
+}
+
+// TestAPIEmailStatusLeavesARatePauseAloneOnANonAppSender: the same door must
+// never resume a RATE pause on a sibling sender either. That pause exists
+// because the sender is abusive, not because a budget rolled over, and it
+// needs a human, whichever sender it sits on.
+func TestAPIEmailStatusLeavesARatePauseAloneOnANonAppSender(t *testing.T) {
+ fx, fake, _, token := emailArmed(t, "code-email-status-no-resume-rate-sibling")
+ app, domain := "titogo", "titogo.bab.oncarlos.com"
+ provisionEmail(t, fx, fake, token, app, domain, "hello@"+domain)
+ putSiblingSender(t, fx, app, mail.PauseKindRate, time.Now().AddDate(0, 0, -1))
+
+ status, out := getJSON(t, emailURL(fx, app), token)
+ if status != http.StatusOK {
+ t.Fatalf("status = %d %v", status, out)
+ }
+
+ sibling := findSender(t, out, "book.example.com")
+ if sibling["active"] != false {
+ t.Fatalf("sibling active = %v, want false. a rate pause must never auto-resume, sibling or not", sibling["active"])
+ }
+ if sibling["pause_kind"] != mail.PauseKindRate {
+ t.Fatalf("sibling pause_kind = %v, want %q to survive", sibling["pause_kind"], mail.PauseKindRate)
+ }
+
+ sn, _, err := mail.GetSender(context.Background(), fx.rel, release.OpsAccount, app, "book.example.com")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if sn.Provisioned == nil || sn.Provisioned.Active {
+ t.Fatal("the sibling sender on disk was resumed. a rate pause must never auto-resume")
+ }
+ if sn.Provisioned.PauseKind != mail.PauseKindRate {
+ t.Fatalf("PauseKind = %q on disk, want it to survive as %q", sn.Provisioned.PauseKind, mail.PauseKindRate)
+ }
+}
+
// TestAPIEmailDeclareRefusesAnUnaddressedFrom: a From with no "@" is refused
// before anything is written.
func TestAPIEmailDeclareRefusesAnUnaddressedFrom(t *testing.T) {
@@ -1305,8 +1419,8 @@ func TestEmailReplyCarriesEverySenderAndKeepsItsTopLevelFields(t *testing.T) {
ctx := context.Background()
// emailReply reads counts for TODAY (it computes its own day from
// time.Now(), not from a caller-supplied clock), so the events seeded
- // here have to land on the real day, not mailFixture's fixed 2026-08-19
- // — otherwise MailCounts would legitimately find nothing.
+ // here have to land on the real day, not mailFixture's fixed 2026-08-19.
+ // otherwise MailCounts would legitimately find nothing.
day := time.Now().UTC().Format("2006-01-02")
if err := fx.st.RecordMailEvent(ctx, release.OpsAccount, "harbourmail", mail.SenderAppID, day, mail.EventSend); err != nil {
t.Fatal(err)