Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions internal/jobs/customer_restore_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,16 @@ func (w *CustomerRestoreRunnerWorker) processRestore(parentCtx context.Context,
return false
}

// Finalize.
if _, updErr := w.db.ExecContext(parentCtx, `
// Finalize on a FRESH bounded context, not parentCtx — mirrors the
// customer_backup_runner fix (P2-W4, BugBash 2026-05-18). pg_restore has
// already succeeded above, so the data is durably in place; if parentCtx
// were cancelled by a worker shutdown (rolling deploy / node drain), the
// UPDATE would fail, markRestoreFailed would run, and a SUCCESSFUL restore
// would be recorded as 'failed'. A detached context lets the row reach
// 'ok' even mid-shutdown.
finalizeCtx, finalizeCancel := context.WithTimeout(context.Background(), 10*time.Second)
defer finalizeCancel()
if _, updErr := w.db.ExecContext(finalizeCtx, `
UPDATE resource_restores
SET status = 'ok', finished_at = now()
WHERE id = $1
Expand Down
22 changes: 16 additions & 6 deletions internal/jobs/deploy_success_email_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,23 @@ func TestEventEmail_BuildDeploySuccess_NoEmailReturnsFalse(t *testing.T) {
func TestLifecycle_RenderDeployHealthy_SurfacesURL(t *testing.T) {
const url = "https://my-app.deployment.instanode.dev"
subject, html, text := renderDeployHealthy(map[string]string{
"app_name": "my-app",
"app_name": "6fffcc21",
"env": "production",
"app_url": url,
"time_to_healthy_seconds": "31",
})
if !strings.Contains(subject, "my-app") {
t.Errorf("subject %q should name the app", subject)
// app_name is an opaque hex slug, so it must NOT appear in the subject as
// a prose name (bug #23) — the subject is generic and the URL identifies
// the app. The slug appears in the body only as a labeled identifier.
if strings.Contains(subject, "6fffcc21") {
t.Errorf("subject %q must not render the opaque app_id slug as a name", subject)
}
if !strings.Contains(html, url) {
t.Errorf("html body should contain the live URL %q", url)
}
if !strings.Contains(html, "6fffcc21") {
t.Errorf("html body should show the app_id as an identifier")
}
if !strings.Contains(text, url) {
t.Errorf("text body should contain the live URL %q", url)
}
Expand All @@ -145,11 +151,15 @@ func TestLifecycle_RenderDeployHealthy_NoURLFallsBackToDashboard(t *testing.T) {
// and links to the dashboard.
func TestLifecycle_RenderDeployCreated_NoURLPromise(t *testing.T) {
subject, html, _ := renderDeployCreated(map[string]string{
"app_name": "my-app",
"app_name": "6fffcc21",
"env": "production",
})
if !strings.Contains(subject, "my-app") {
t.Errorf("subject %q should name the app", subject)
// Opaque slug must not appear as a prose name in the subject (bug #23).
if strings.Contains(subject, "6fffcc21") {
t.Errorf("subject %q must not render the opaque app_id slug as a name", subject)
}
if !strings.Contains(html, "6fffcc21") {
t.Errorf("started email should still show the app_id as an identifier in the body")
}
if !strings.Contains(html, dashboardURL) {
t.Errorf("started email should link to the dashboard")
Expand Down
14 changes: 11 additions & 3 deletions internal/jobs/event_email_forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -658,10 +658,18 @@ batchLoop:
"audit_id", row.ID,
"kind", row.Kind,
"error", supErr,
"note", "fail-closed: skipping send, cursor NOT advanced — retries when DB recovers",
"note", "fail-closed: halting batch, cursor NOT advanced — this row + remainder retried when DB recovers",
)
skipped++
continue
// MUST halt the whole batch, not `continue`. The cursor is
// advanced per-row inline (and redisEventCursorStore.write is an
// unconditional Set, not a max), so a `continue` would let a LATER
// sendable row in this same batch advance the watermark past this
// held row — stranding it behind the cursor forever and silently
// dropping a legitimate transactional email. `break batchLoop`
// leaves the cursor at the last successfully-advanced row so this
// row is re-fetched next tick — mirrors the SendClassTransient halt.
transient++
break batchLoop
}
if supErr != nil {
// Bounce/spam lookup failure — fail-OPEN: treat as "not
Expand Down
47 changes: 47 additions & 0 deletions internal/jobs/event_email_forwarder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,53 @@ func TestEventForwarder_UnsubscribeCheckError_FailsClosed(t *testing.T) {
}
}

// TestEventForwarder_UnsubscribeFailClosed_HaltsBatch is the multi-row
// regression for bug #23 (bug bash 2026-06-02): a fail-CLOSED row followed by
// a sendable row in the SAME batch must NOT let the sendable row advance the
// cursor past the held row. The pre-fix code used `continue` (not `break
// batchLoop`); because the cursor is advanced per-row inline via an
// unconditional Set, the second row's send moved the watermark past row 1,
// stranding it forever — silently dropping a legitimate transactional email
// during a transient unsubscribe-lookup DB blip. The single-row test above
// can't catch this (continue and break are indistinguishable with one row).
func TestEventForwarder_UnsubscribeFailClosed_HaltsBatch(t *testing.T) {
db, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherRegexp))
if err != nil {
t.Fatalf("sqlmock.New: %v", err)
}
defer db.Close()

held := time.Date(2026, 5, 13, 19, 0, 0, 0, time.UTC)
sendable := held.Add(time.Minute) // sorts AFTER the held row (ASC order)
mock.ExpectQuery(`SELECT[\s\S]+FROM audit_log`).
WillReturnRows(sqlmock.NewRows(auditRowsCols).
AddRow("held-row", "team-h", auditKindOnboardingClaimed, "", "x", []byte(`{}`), held, "blip@example.com").
AddRow("sendable-row", "team-s", auditKindOnboardingClaimed, "", "y", []byte(`{}`), sendable, "ok@example.com"))

provider := &fakeProvider{sendFn: func(_ context.Context, _ email.EventEmail) error { return nil }}
cursor := &memCursor{}
w := newEventEmailForwarderWorkerForTest(db, cursor, provider)
// failNext fails ONLY the first hasSuppression call — i.e. the held row.
w.suppression = &memSuppression{
suppressedEmails: map[string]bool{},
failNext: fmt.Errorf("simulated unsubscribe DB blip: %w", errUnsubscribeLookupFailed),
}

if err := w.Work(context.Background(), fakeJobLocal[EventEmailForwarderArgs]()); err != nil {
t.Fatalf("Work: %v", err)
}

// The batch must HALT on the held row: the sendable row is never reached,
// so the provider is not called and the cursor never advances past (or to)
// either row. Next tick re-fetches from the un-advanced cursor.
if got := provider.callCount(); got != 0 {
t.Errorf("expected 0 SendEvent calls (batch halts on fail-closed row), got %d — a later row advanced past the held row", got)
}
if cursor.c.ID != "" {
t.Errorf("cursor.ID = %q; want \"\" — fail-closed must halt the batch, never let a later row advance the cursor past the held row", cursor.c.ID)
}
}

// TestEventForwarder_NoopProvider_AdvancesCursor — wiring a real
// email.NoopProvider through the forwarder is the integration check that
// the SendClassSkippedNoTemplate path advances cursors. If this regresses,
Expand Down
43 changes: 32 additions & 11 deletions internal/jobs/lifecycle_emails.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,15 +183,25 @@ var (
`<p style="margin:0 0 14px;">Your deployment is now <strong>permanent</strong> — it will no longer expire automatically{{ if .Source }} (changed via {{ .Source }}){{ end }}.</p>
<p style="margin:0 0 4px;color:#555;font-size:14px;">It'll keep serving traffic until you delete it explicitly.</p>`))

// NOTE: AppName here is the app_id — an opaque 8-char hex slug
// (generateAppID), NOT a human-readable name. So it is rendered ONLY as a
// labeled `App` identifier in a <code> cell, never interpolated into prose
// as if it were a name ("Your deployment 6fffcc21 is live" reads as
// gibberish). This mirrors the deploy-TTL builders, which deliberately do
// NOT copy app_id → deploy_name for the same reason.
bodyDeployCreated = template.Must(template.New("b_dcreated").Parse(
`<p style="margin:0 0 14px;">Your deployment{{ if .AppName }} <strong>{{ .AppName }}</strong>{{ end }} has started building{{ if .Env }} in the <strong>{{ .Env }}</strong> environment{{ end }}.</p>
`<p style="margin:0 0 14px;">Your deployment has started building{{ if .Env }} in the <strong>{{ .Env }}</strong> environment{{ end }}.</p>
{{ if .AppName }}<table cellpadding="6" cellspacing="0" style="background:#f7f7f8;border-radius:6px;font-size:14px;margin:4px 0 14px;width:100%;">
<tr><td style="color:#666;width:120px;">App</td><td><code>{{ .AppName }}</code></td></tr>
</table>{{ end }}
<p style="margin:0 0 4px;">We're building the image and rolling it out now — this usually takes under a minute. You'll get a second email with the live URL the moment it's serving traffic.</p>
<p style="margin:0 0 4px;color:#555;font-size:14px;">Watch the build live from your dashboard.</p>`))

bodyDeployHealthy = template.Must(template.New("b_dhealthy").Parse(
`<p style="margin:0 0 14px;">Your deployment{{ if .AppName }} <strong>{{ .AppName }}</strong>{{ end }} is <strong>live</strong>{{ if .Env }} in the <strong>{{ .Env }}</strong> environment{{ end }} and serving traffic.</p>
{{ if .AppURL }}<table cellpadding="6" cellspacing="0" style="background:#f7f7f8;border-radius:6px;font-size:14px;margin:4px 0 14px;width:100%;">
<tr><td style="color:#666;width:120px;">URL</td><td><a href="{{ .AppURL }}" style="color:#2563eb;"><strong>{{ .AppURL }}</strong></a></td></tr>
`<p style="margin:0 0 14px;">Your deployment is <strong>live</strong>{{ if .Env }} in the <strong>{{ .Env }}</strong> environment{{ end }} and serving traffic.</p>
{{ if or .AppURL .AppName }}<table cellpadding="6" cellspacing="0" style="background:#f7f7f8;border-radius:6px;font-size:14px;margin:4px 0 14px;width:100%;">
{{ if .AppURL }}<tr><td style="color:#666;width:120px;">URL</td><td><a href="{{ .AppURL }}" style="color:#2563eb;"><strong>{{ .AppURL }}</strong></a></td></tr>{{ end }}
{{ if .AppName }}<tr><td style="color:#666;">App</td><td><code>{{ .AppName }}</code></td></tr>{{ end }}
{{ if .TimeToHealthy }}<tr><td style="color:#666;">Build time</td><td>{{ .TimeToHealthy }}s</td></tr>{{ end }}
</table>{{ end }}
<p style="margin:0 0 4px;color:#555;font-size:14px;">Manage, redeploy, or set a custom domain from your dashboard.</p>`))
Expand Down Expand Up @@ -724,8 +734,9 @@ func renderDeployMadePermanent(params map[string]string) (string, string, string
// live URL yet. The "started" email links to the dashboard; the live URL
// arrives in the separate deploy.healthy email.
func renderDeployCreated(params map[string]string) (string, string, string) {
name := orDefault(params["app_name"], "your app")
subject := "Deploying " + name + " on instanode"
// app_name is the opaque app_id slug — used as an identifier, never as a
// prose name in the subject (see bodyDeployCreated NOTE).
subject := "Your instanode deployment has started"
heading := "Your deployment has started"
body := renderBody(bodyDeployCreated, viewDeployCreated{
AppName: params["app_name"], Env: params["env"],
Expand All @@ -734,9 +745,14 @@ func renderDeployCreated(params map[string]string) (string, string, string) {
Title: subject, Heading: heading, Body: body,
CTALabel: "Watch the build", CTAURL: dashboardURL,
})
textBody := "Your deployment has started building."
if id := params["app_name"]; id != "" {
textBody += " App: " + id + "."
}
textBody += " This usually takes under a minute — you'll get a follow-up email with the live URL once it's serving traffic."
text := lifecycleText(lifecycleTextView{
Heading: heading,
Body: "Your deployment " + name + " has started building. This usually takes under a minute — you'll get a follow-up email with the live URL once it's serving traffic.",
Heading: heading,
Body: textBody,
CTALabel: "Watch the build", CTAURL: dashboardURL,
})
return subject, html, text
Expand All @@ -746,8 +762,10 @@ func renderDeployCreated(params map[string]string) (string, string, string) {
// success counterpart to renderDeployFailed: the app is live, so the email
// leads with the URL and the CTA opens the running app.
func renderDeployHealthy(params map[string]string) (string, string, string) {
name := orDefault(params["app_name"], "your app")
subject := name + " is live on instanode"
// app_name is the opaque app_id slug — shown as an identifier in the body,
// never as a prose name in the subject (see bodyDeployHealthy NOTE). The
// live URL identifies the app for the reader.
subject := "Your instanode deployment is live"
heading := "Your app is live 🚀"
body := renderBody(bodyDeployHealthy, viewDeployHealthy{
AppName: params["app_name"],
Expand All @@ -765,10 +783,13 @@ func renderDeployHealthy(params map[string]string) (string, string, string) {
Title: subject, Heading: heading, Body: body,
CTALabel: ctaLabel, CTAURL: cta,
})
textBody := "Your deployment " + name + " is live and serving traffic."
textBody := "Your deployment is live and serving traffic."
if url := params["app_url"]; url != "" {
textBody += " It's available at " + url + "."
}
if id := params["app_name"]; id != "" {
textBody += " App: " + id + "."
}
text := lifecycleText(lifecycleTextView{
Heading: heading,
Body: textBody,
Expand Down
Loading