amadan

carlos / platform Public

Merge pull request #268 from carlosframework/ses-ingest-fix

email status names a verified domain that emits no SES events, and fixes a latent nil-deref on a nil RegionState.

Merged with the required check unable to run (Actions billing-dead); gate reproduced locally: 31 packages, vet and gofmt clean, plus the GOOS=darwin build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev ab58c524b8a1d9ad9feba4502af8e32c2657f798 parent c25cd5fparent fa1b73e
2 files changed, +115 −0
  • mail_cli.go +33 −0
  • mail_cli_test.go +82 −0
diff --git a/mail_cli.go b/mail_cli.go
index afcaec980..fcfbde984 100644
--- a/mail_cli.go
+++ b/mail_cli.go
@@ -823,11 +823,44 @@ func printEmailStatus(w io.Writer, res apiEmailReply) {
}
sort.Strings(regions)
var parts []string
+ // Regions whose identity is verified but carries NO configuration
+ // set. See the eventless warning below for why only those count.
+ var eventless []string
for _, region := range regions {
st := d.Regions[region]
+ if st == nil {
+ continue
+ }
parts = append(parts, fmt.Sprintf("%s:%s", region, st.Status))
+ if st.Status == "verified" && st.ConfigSet == "" {
+ eventless = append(eventless, region)
+ }
}
fmt.Fprintf(w, " domain %s (%s): %s\n", d.Domain, d.Kind, strings.Join(parts, ", "))
+ // A VERIFIED region with an empty config_set sends mail perfectly
+ // well and emits no SES events at all — so every count below stays
+ // zero forever, and "today: 0/50 sends" is indistinguishable from a
+ // quiet day. That silence is the whole of spec §9 not working: no
+ // daily cap, no bounce/complaint threshold, no automatic pause.
+ //
+ // It is not a hypothetical state. A deployment binds a configuration
+ // set only inside mail.EnsureDomain's `SNSTopicARN != ""` branch, so
+ // every identity verified BEFORE the deployment's
+ // CARLOS_MAIL_SNS_TOPIC_ARN was set stays unbound permanently —
+ // setting the variable later binds nothing retroactively, and
+ // nothing re-walks existing domains on its own.
+ //
+ // Only VERIFIED regions are named. A pending or failed region has no
+ // configuration set either, but the reason is that it has not
+ // finished verifying, which the status line above already says;
+ // warning there would put a second, misleading diagnosis on a state
+ // the member is already acting on.
+ if len(eventless) > 0 {
+ warnf(w, " no SES event destination in %s — sends, bounces and complaints are never counted for this domain, so the daily cap and the automatic pause cannot fire",
+ strings.Join(eventless, ", "))
+ mutedf(w, " re-run `carlos email domains add -app %s %s` to bind one — it is idempotent and leaves a verified identity verified",
+ res.App, d.Domain)
+ }
}
if res.Provisioned != nil {
diff --git a/mail_cli_test.go b/mail_cli_test.go
index 2251b0662..6dfa6687a 100644
--- a/mail_cli_test.go
+++ b/mail_cli_test.go
@@ -1026,3 +1026,85 @@ func TestEmailVerbDispatchAndRefusals(t *testing.T) {
t.Fatalf("bad app name = %q, want the name gate", out)
}
}
+
+// TestPrintEmailStatusNamesADomainWithNoEventDestination: a verified domain
+// whose region carries no configuration set emits NO SES events at all, so
+// the daily cap, the bounce/complaint thresholds and the automatic pause are
+// all inert for it — and the "today: 0/50 sends" line reads exactly the same
+// as a quiet day. Every identity verified before a deployment's
+// CARLOS_MAIL_SNS_TOPIC_ARN was set is in that state permanently, because
+// setting the variable binds nothing retroactively. The record already
+// carries config_set and the status door already returns it; this is the one
+// view a member with no AWS console has, so it has to say so, and name the
+// verb that repairs it.
+func TestPrintEmailStatusNamesADomainWithNoEventDestination(t *testing.T) {
+ var buf bytes.Buffer
+ printEmailStatus(&buf, apiEmailReply{
+ App: "seapointish", DailyCap: 50, Active: true,
+ Provisioned: &apiEmailProvisioned{KeyID: "AKIA1"},
+ Domains: []apiEmailDomain{{
+ Domain: "seapointish.bag.oncarlos.com", Kind: "platform",
+ Regions: map[string]*apiEmailRegionState{
+ "eu-west-1": {Status: "verified", ConfigSet: ""},
+ },
+ }},
+ })
+ out := buf.String()
+ if !strings.Contains(out, "no SES event destination") {
+ t.Fatalf("status output does not say the domain emits no events:\n%s", out)
+ }
+ // The consequence, not just the condition: a member reading "no event
+ // destination" learns nothing unless it also says what stops working.
+ if !strings.Contains(out, "cap") || !strings.Contains(out, "pause") {
+ t.Fatalf("status output does not name what is inert without events:\n%s", out)
+ }
+ // The no-infra rule (CLAUDE.md): the fix must be a carlos verb, never
+ // "go look in the AWS console".
+ if !strings.Contains(out, "carlos email domains add -app seapointish seapointish.bag.oncarlos.com") {
+ t.Fatalf("status output does not name the verb that repairs it:\n%s", out)
+ }
+}
+
+// TestPrintEmailStatusStaysQuietWhenEventsAreBound: the symmetric case. A
+// domain bound to a configuration set is the healthy shape, and the status
+// view must not nag about it — a warning printed on every healthy domain is
+// a warning nobody reads on the unhealthy one.
+func TestPrintEmailStatusStaysQuietWhenEventsAreBound(t *testing.T) {
+ var buf bytes.Buffer
+ printEmailStatus(&buf, apiEmailReply{
+ App: "seapointish", DailyCap: 50, Active: true,
+ Provisioned: &apiEmailProvisioned{KeyID: "AKIA1"},
+ Domains: []apiEmailDomain{{
+ Domain: "seapointish.bag.oncarlos.com", Kind: "platform",
+ Regions: map[string]*apiEmailRegionState{
+ "eu-west-1": {Status: "verified", ConfigSet: "carlos-bag"},
+ },
+ }},
+ })
+ out := buf.String()
+ if strings.Contains(out, "no SES event destination") {
+ t.Fatalf("status output warns about a domain that IS bound to a config set:\n%s", out)
+ }
+}
+
+// TestPrintEmailStatusDoesNotWarnOnAnUnverifiedDomain: a pending or failed
+// region has no configuration set yet either, but the reason is that it is
+// not finished verifying — telling a member their events are broken while
+// the identity is still converging is noise on top of a state they are
+// already being told about.
+func TestPrintEmailStatusDoesNotWarnOnAnUnverifiedDomain(t *testing.T) {
+ var buf bytes.Buffer
+ printEmailStatus(&buf, apiEmailReply{
+ App: "seapointish", DailyCap: 50,
+ Domains: []apiEmailDomain{{
+ Domain: "seapointish.bag.oncarlos.com", Kind: "platform",
+ Regions: map[string]*apiEmailRegionState{
+ "eu-west-1": {Status: "pending", ConfigSet: ""},
+ },
+ }},
+ })
+ out := buf.String()
+ if strings.Contains(out, "no SES event destination") {
+ t.Fatalf("status output warns about events on a still-verifying domain:\n%s", out)
+ }
+}