Store a target's number of delivery attempts, at least one (closes #540)

Target.MaxRetries (max_retries) becomes Target.MaxAttempts
(max_attempts), and the form field, parser and ceiling are renamed to
match. 1 is a single attempt with no circuit breaker; above 1 retries
behind the breaker up to that many attempts. Both target forms refuse
0 and less and default a blank field to 1. The target page shows the
plain number, and the README's two paragraphs on 0 against 1 become
one sentence.

Model: opus-5-5

README.md

@@ -1568,29 +1568,27 @@ did not arrive on the URL and counts in neither.
 A delivery destination for events. Each target defines where and how events
 should be forwarded.
 
-| Field         | Type       | Description                                                                                                                                                                                                                                                                                                                                                 |
+| Field          | Type       | Description                                                                                                                                                                             |
-| ------------- | ---------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| -------------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `id`          | UUID       | Primary key                                                                                                                                                                                                                                                                                                                                                 |
+| `id`           | UUID       | Primary key                                                                                                                                                                             |
-| `webhook_id`  | UUID       | Foreign key → Webhook                                                                                                                                                                                                                                                                                                                                       |
+| `webhook_id`   | UUID       | Foreign key → Webhook                                                                                                                                                                   |
-| `name`        | string     | Human-readable name                                                                                                                                                                                                                                                                                                                                         |
+| `name`         | string     | Human-readable name                                                                                                                                                                     |
-| `type`        | TargetType | One of: `http`, `slack`, `database`, `log`                                                                                                                                                                                                                                                                                                                  |
+| `type`         | TargetType | One of: `http`, `slack`, `database`, `log`                                                                                                                                              |
-| `active`      | boolean    | Whether deliveries are enabled (default: true)                                                                                                                                                                                                                                                                                                              |
+| `active`       | boolean    | Whether deliveries are enabled (default: true)                                                                                                                                          |
-| `config`      | JSON text  | Type-specific configuration                                                                                                                                                                                                                                                                                                                                 |
+| `config`       | JSON text  | Type-specific configuration                                                                                                                                                             |
-| `max_retries` | integer    | Total delivery attempts for `http` and `slack` targets, not retries on top of the first: 0 is a single fire-and-forget attempt with no retries and no circuit breaker, and a value of N makes N attempts in all, with exponential backoff and a per-target circuit breaker. Ignored by `database` and `log` targets. The web UI labels it Delivery attempts |
+| `max_attempts` | integer    | How many times an `http` or `slack` target attempts each delivery in all: at least 1, and 1 by default. Ignored by `database` and `log` targets. The web UI labels it Delivery attempts |
 
 **Relations:** Belongs to Webhook. Has many Deliveries.
 
 **Target types:**
 
-- **`http`** — Forward the event as an HTTP POST to a configured URL.
+- **`http`** — Forward the event as an HTTP POST to a configured URL. With
-  `max_retries` is the total number of delivery attempts, not retries on top of
+  `max_attempts` at 1 the target makes a single attempt; above 1 it retries a
-  the first: when `max_retries` is 0 (the default), the target operates in
+  failed delivery with exponential backoff, up to that many attempts in all,
-  fire-and-forget mode, a single attempt with no retries and no circuit breaker;
+  behind a per-target circuit breaker.
-  a value of N makes up to N attempts in all, retrying failed deliveries with
-  exponential backoff and protecting them with a per-target circuit breaker.
 - **`slack`** — Post the event as a formatted message to a Slack-compatible
   incoming webhook URL (`webhookUrl` in `config`). It is built on the same HTTP
-  core as `http` and honours `max_retries` identically, circuit breaker
+  core as `http` and honours `max_attempts` identically, circuit breaker
   included. See the Slack target section under "Per-Webhook Event Databases" for
   the message format.
 - **`database`** — Archive the full event as a row into the target's own archive
@@ -2165,11 +2163,11 @@ External Service
                               ▼                ▼                    ▼
                        ┌────────────┐   ┌────────────┐      ┌────────────┐
                        │ HTTP Target│   │ HTTP Target│      │ Log Target │
-                       │(max_retries│   │(max_retries│      │ (stdout)   │
+                       │max_attempts│   │max_attempts│      │ (stdout)   │
-                       │   == 0)    │   │   > 0,     │      └────────────┘
+                       │   == 1,    │   │   > 1,     │      └────────────┘
-                       │ fire+forget│   │ backoff +  │
+                       │ one attempt│   │ backoff +  │
                        └────────────┘   │ circuit    │
-                                        │ breaker)   │
+                                        │ breaker    │
                                         └────────────┘
 ```
 
@@ -2246,7 +2244,7 @@ re-inject it to every currently active target.
 
 ### Circuit Breaker (HTTP and Slack Targets with Retries)
 
-`http` and `slack` targets with `max_retries` > 0 are protected by a
+`http` and `slack` targets with `max_attempts` > 1 are protected by a
 **per-target circuit breaker** that prevents hammering a down target with
 repeated failed delivery attempts. The circuit breaker is in-memory only and
 resets on restart (which is fine — startup recovery rescans the database
@@ -2287,10 +2285,10 @@ anyway).
 - **Cooldown:** 30 seconds in open state before probing
 
 **Scope:** Circuit breakers apply to **`http` and `slack` targets with
-`max_retries` > 0**. The Slack target is built on the same HTTP core and hands
+`max_attempts` > 1**. The Slack target is built on the same HTTP core and hands
-its own `max_retries` to the same retry path, so it gets a breaker with the same
+its own `max_attempts` to the same retry path, so it gets a breaker with the
-5-failure / 30-second defaults. Fire-and-forget targets of either type
+same 5-failure / 30-second defaults. Single-attempt targets of either type
-(`max_retries` == 0), database targets (local operations), and log targets
+(`max_attempts` == 1), database targets (local operations), and log targets
 (stdout) do not use circuit breakers.
 
 When a circuit is open and a new delivery arrives, the engine marks the delivery

internal/database/model_target.go

@@ -29,9 +29,11 @@ type Target struct {
 	// handler that marshals the model itself.
 	Config string `gorm:"type:text" json:"-"` // JSON configuration
 
-	// For HTTP targets (max_retries=0 means fire-and-forget,
+	// MaxAttempts is how many times an HTTP or Slack target attempts
-	// >0 enables retries with backoff)
+	// each delivery in all, at least 1. 1 is a single attempt with no
-	MaxRetries int `json:"maxRetries,omitempty"`
+	// circuit breaker; above 1, a failed attempt is retried with
+	// backoff behind the breaker.
+	MaxAttempts int `json:"maxAttempts,omitempty"`
 
 	// Relations. No model marshals the record it belongs to:
 	// Webhook.Targets leads back here, and the JSON could loop.

internal/database/schema_test.go

@@ -87,13 +87,13 @@ func TestMainSchemaHoldsEveryModel(t *testing.T) {
 		Active:      true,
 	})
 	assertRoundTrip(t, db, &database.Target{
-		BaseModel:  newBaseModel(),
+		BaseModel:   newBaseModel(),
-		WebhookID:  uuid.New().String(),
+		WebhookID:   uuid.New().String(),
-		Name:       "target",
+		Name:        "target",
-		Type:       database.TargetTypeHTTP,
+		Type:        database.TargetTypeHTTP,
-		Active:     true,
+		Active:      true,
-		Config:     `{"url":"https://example.com/"}`,
+		Config:      `{"url":"https://example.com/"}`,
-		MaxRetries: 3,
+		MaxAttempts: 3,
 	})
 }
 

internal/db/migrations/main/001_schema.sql

@@ -80,7 +80,7 @@ CREATE TABLE targets (
     type text NOT NULL,
     active numeric DEFAULT true,
     config text,
-    max_retries integer,
+    max_attempts integer,
     PRIMARY KEY (id),
     CONSTRAINT fk_webhooks_targets
         FOREIGN KEY (webhook_id) REFERENCES webhooks (id)

internal/delivery/engine.go

@@ -107,7 +107,7 @@ type Task struct {
 	TargetName   string
 	TargetType   database.TargetType
 	TargetConfig string
-	MaxRetries   int
+	MaxAttempts  int
 
 	Method      string
 	RawQuery    string
@@ -1769,10 +1769,10 @@ func buildEventFromTask(task *Task) database.Event {
 
 func buildTargetFromTask(task *Task) database.Target {
 	target := database.Target{
-		Name:       task.TargetName,
+		Name:        task.TargetName,
-		Type:       task.TargetType,
+		Type:        task.TargetType,
-		Config:     task.TargetConfig,
+		Config:      task.TargetConfig,
-		MaxRetries: task.MaxRetries,
+		MaxAttempts: task.MaxAttempts,
 	}
 
 	target.ID = task.TargetID
@@ -2028,7 +2028,7 @@ func (e *Engine) countAttemptsBatch(
 	// One delivery id per recorded attempt, tallied here rather than
 	// grouped in SQL: internal/gormlog forbids (*gorm.DB).Scan, which
 	// a GROUP BY into a struct would need, and an attempt row per
-	// delivery is bounded by the target's MaxRetries.
+	// delivery is bounded by the target's MaxAttempts.
 	var attemptIDs []string
 
 	err := webhookDB.
@@ -2106,7 +2106,7 @@ func buildRecoveryTask(
 		TargetName:   target.Name,
 		TargetType:   target.Type,
 		TargetConfig: target.Config,
-		MaxRetries:   target.MaxRetries,
+		MaxAttempts:  target.MaxAttempts,
 		Method:       event.Method,
 		RawQuery:     event.RawQuery,
 		Headers:      event.Headers,

internal/delivery/engine_integration_test.go

@@ -170,7 +170,7 @@ func iTask(
 	d database.Delivery,
 	event database.Event,
 	webhookID, targetID, name, config string,
-	maxRetries, attemptNum int,
+	maxAttempts, attemptNum int,
 	body *string,
 ) delivery.Task {
 	return delivery.Task{
@@ -181,7 +181,7 @@ func iTask(
 		TargetName:   name,
 		TargetType:   database.TargetTypeHTTP,
 		TargetConfig: config,
-		MaxRetries:   maxRetries,
+		MaxAttempts:  maxAttempts,
 		Method:       event.Method,
 		Headers:      event.Headers,
 		ContentType:  event.ContentType,
@@ -242,7 +242,7 @@ func TestProcessNewTask_InlineBody(t *testing.T) {
 	cfg := iHTTPConfig(ts.URL)
 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"test-target", cfg, 0, 1, &bodyStr,
+		"test-target", cfg, 1, 1, &bodyStr,
 	)
 
 	s.Engine.ExportProcessNewTask(
@@ -293,7 +293,7 @@ func TestProcessNewTask_LargeBody_FetchFromDB(
 	cfg := iHTTPConfig(ts.URL)
 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"test-large", cfg, 0, 1, nil,
+		"test-large", cfg, 1, 1, nil,
 	)
 
 	s.Engine.ExportProcessNewTask(
@@ -320,7 +320,7 @@ func TestProcessNewTask_InvalidWebhookID(t *testing.T) {
 		TargetName:   "test",
 		TargetType:   database.TargetTypeHTTP,
 		TargetConfig: iHTTPConfig("http://localhost:9999"),
-		MaxRetries:   0,
+		MaxAttempts:  1,
 		Body:         nil,
 		AttemptNum:   1,
 	}
@@ -645,7 +645,7 @@ func TestRecoverPendingDeliveries(t *testing.T) {
 
 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "recovery-target",
-		database.TargetTypeLog, "", 0,
+		database.TargetTypeLog, "", 1,
 	)
 
 	iSeedPendingDeliveries(
@@ -686,17 +686,17 @@ func iCreateTarget(
 	targetID, webhookID, name string,
 	targetType database.TargetType,
 	config string,
-	maxRetries int,
+	maxAttempts int,
 ) {
 	t.Helper()
 
 	target := database.Target{
-		WebhookID:  webhookID,
+		WebhookID:   webhookID,
-		Name:       name,
+		Name:        name,
-		Type:       targetType,
+		Type:        targetType,
-		Active:     true,
+		Active:      true,
-		Config:     config,
+		Config:      config,
-		MaxRetries: maxRetries,
+		MaxAttempts: maxAttempts,
 	}
 	target.ID = targetID
 
@@ -1077,7 +1077,7 @@ func TestRecoverInFlight_WithPendingDeliveries(
 
 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "recover-target",
-		database.TargetTypeLog, "", 0,
+		database.TargetTypeLog, "", 1,
 	)
 
 	event := iSeedEvent(
@@ -1232,7 +1232,7 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
 	task := iTask(
 		d, event, s.WebhookID, targetID,
 		"auth-target", string(cfgJSON),
-		0, 1, &bodyStr,
+		1, 1, &bodyStr,
 	)
 
 	s.Engine.ExportProcessNewTask(
@@ -1277,7 +1277,7 @@ func TestDeliverHTTP_TargetTimeout(t *testing.T) {
 
 	task, d := iHTTPTaskAndDelivery(
 		event, del, "timeout-target",
-		string(cfgJSON), 0, 1,
+		string(cfgJSON), 1, 1,
 	)
 
 	e.ExportDeliverHTTP(context.TODO(), db, d, task)
@@ -1339,7 +1339,7 @@ func TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog(
 	bodyStr := event.Body
 	task := iTask(
 		del, event, s.WebhookID, targetID,
-		"cut-target", cfgJSON, 0, 1, &bodyStr,
+		"cut-target", cfgJSON, 1, 1, &bodyStr,
 	)
 
 	s.Engine.ExportProcessNewTask(context.TODO(), &task)
@@ -1403,7 +1403,7 @@ func iHTTPTaskAndDelivery(
 	event database.Event,
 	del database.Delivery,
 	name, config string,
-	maxRetries, attemptNum int,
+	maxAttempts, attemptNum int,
 ) (*delivery.Task, *database.Delivery) {
 	task := &delivery.Task{
 		DeliveryID:   del.ID,
@@ -1413,7 +1413,7 @@ func iHTTPTaskAndDelivery(
 		TargetName:   name,
 		TargetType:   database.TargetTypeHTTP,
 		TargetConfig: config,
-		MaxRetries:   maxRetries,
+		MaxAttempts:  maxAttempts,
 		AttemptNum:   attemptNum,
 	}
 
@@ -1466,7 +1466,7 @@ func TestDeliverHTTP_InvalidConfig(t *testing.T) {
 	)
 
 	task, d := iHTTPTaskAndDelivery(
-		event, del, "bad-config", `not-json`, 0, 1,
+		event, del, "bad-config", `not-json`, 1, 1,
 	)
 
 	e.ExportDeliverHTTP(context.TODO(), db, d, task)
@@ -1490,7 +1490,7 @@ func TestDeliverHTTP_InvalidConfigUnrecordedStaysPending(t *testing.T) {
 	)
 
 	task, d := iHTTPTaskAndDelivery(
-		event, del, "bad-config", `not-json`, 0, 1,
+		event, del, "bad-config", `not-json`, 1, 1,
 	)
 
 	require.NoError(t, db.Exec("drop table delivery_results").Error)

internal/delivery/engine_lifecycle_test.go

@@ -278,7 +278,7 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
 }
 
 // seedHeldDelivery gives the setup's webhook an http target with
-// maxRetries, pointed at a receiver that keeps every request open until
+// maxAttempts, pointed at a receiver that keeps every request open until
 // the client gives up on it, and seeds one delivery to that target at
 // status. It returns the task that sends attempt attemptNum of the
 // delivery, and a channel that is closed when a request reaches the
@@ -286,7 +286,7 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
 func seedHeldDelivery(
 	t *testing.T,
 	s iSetup,
-	maxRetries int,
+	maxAttempts int,
 	status database.DeliveryStatus,
 	attemptNum int,
 ) (delivery.Task, <-chan struct{}) {
@@ -315,7 +315,7 @@ func seedHeldDelivery(
 	iCreateWebhook(t, s.MainDB, s.WebhookID, "hook")
 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "held",
-		database.TargetTypeHTTP, cfg, maxRetries,
+		database.TargetTypeHTTP, cfg, maxAttempts,
 	)
 
 	event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"held":true}`)
@@ -325,7 +325,7 @@ func seedHeldDelivery(
 
 	return iTask(
 		d, event, s.WebhookID, targetID, "held", cfg,
-		maxRetries, attemptNum, &bodyStr,
+		maxAttempts, attemptNum, &bodyStr,
 	), arrived
 }
 
@@ -380,7 +380,7 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
 	lc := startEngineViaHook(t, s.Engine)
 
 	task, arrived := seedHeldDelivery(
-		t, s, 0, database.DeliveryStatusPending, 1,
+		t, s, 1, database.DeliveryStatusPending, 1,
 	)
 
 	s.Engine.Notify([]delivery.Task{task})
@@ -404,13 +404,13 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
 func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) {
 	t.Parallel()
 
-	const maxRetries = 2
+	const maxAttempts = 2
 
 	s := newISetup(t)
 	lc := startEngineViaHook(t, s.Engine)
 
 	task, arrived := seedHeldDelivery(
-		t, s, maxRetries, database.DeliveryStatusRetrying, maxRetries,
+		t, s, maxAttempts, database.DeliveryStatusRetrying, maxAttempts,
 	)
 	iSeedFailedResult(t, s.WebhookDB, task.DeliveryID)
 

internal/delivery/engine_test.go

@@ -142,7 +142,7 @@ func buildHTTPFixture(
 	dlv database.Delivery,
 	event database.Event,
 	targetID, name, config string,
-	maxRetries, attemptNum int,
+	maxAttempts, attemptNum int,
 ) httpDeliveryFixture {
 	task := &delivery.Task{
 		DeliveryID:   dlv.ID,
@@ -152,7 +152,7 @@ func buildHTTPFixture(
 		TargetName:   name,
 		TargetType:   database.TargetTypeHTTP,
 		TargetConfig: config,
-		MaxRetries:   maxRetries,
+		MaxAttempts:  maxAttempts,
 		AttemptNum:   attemptNum,
 	}
 
@@ -162,10 +162,10 @@ func buildHTTPFixture(
 		Status:   dlv.Status,
 		Event:    event,
 		Target: database.Target{
-			Name:       name,
+			Name:        name,
-			Type:       database.TargetTypeHTTP,
+			Type:        database.TargetTypeHTTP,
-			Config:     config,
+			Config:      config,
-			MaxRetries: maxRetries,
+			MaxAttempts: maxAttempts,
 		},
 	}
 	d.ID = dlv.ID
@@ -277,7 +277,7 @@ func TestDeliverHTTP_Success(t *testing.T) {
 
 	cfg := newHTTPTargetConfig(ts.URL)
 	fix := buildHTTPFixture(
-		dlv, event, targetID, "test-http", cfg, 0, 1,
+		dlv, event, targetID, "test-http", cfg, 1, 1,
 	)
 
 	e.ExportDeliverHTTP(
@@ -324,7 +324,7 @@ func TestDeliverHTTP_Failure(t *testing.T) {
 	cfg := newHTTPTargetConfig(ts.URL)
 	fix := buildHTTPFixture(
 		dlv, event, targetID,
-		"test-http-fail", cfg, 0, 1,
+		"test-http-fail", cfg, 1, 1,
 	)
 
 	e.ExportDeliverHTTP(
@@ -473,14 +473,23 @@ func TestDeliverHTTP_WithRetries_Success(t *testing.T) {
 	)
 }
 
-func TestDeliverHTTP_MaxRetriesExhausted(t *testing.T) {
+// TestDeliverHTTP_MaxAttemptsExhausted proves a target with three
+// delivery attempts makes at most three: the first two failures are
+// retried, the third is final, and each attempt goes through the
+// target's circuit breaker.
+func TestDeliverHTTP_MaxAttemptsExhausted(t *testing.T) {
 	t.Parallel()
 
+	const maxAttempts = 3
+
 	db := testWebhookDB(t)
 
+	var requests atomic.Int32
+
 	ts := httptest.NewServer(
 		http.HandlerFunc(
 			func(w http.ResponseWriter, _ *http.Request) {
+				requests.Add(1)
 				w.WriteHeader(http.StatusBadGateway)
 			},
 		),
@@ -489,26 +498,104 @@ func TestDeliverHTTP_MaxRetriesExhausted(t *testing.T) {
 
 	e := testEngine(t, 1)
 	targetID := uuid.New().String()
+
+	// It opens on the last failure only if every attempt reached it.
+	cb := delivery.NewTestCircuitBreaker(maxAttempts, time.Minute)
+	e.ExportSetCircuitBreaker(targetID, cb)
+
 	event := seedEvent(t, db, `{"retry":"exhaust"}`)
 
 	dlv := seedDelivery(
 		t, db, event.ID, targetID,
-		database.DeliveryStatusRetrying,
+		database.DeliveryStatusPending,
 	)
 
 	cfg := newHTTPTargetConfig(ts.URL)
 	fix := buildHTTPFixture(
 		dlv, event, targetID,
-		"test-http-exhaust", cfg, 3, 3,
+		"test-http-exhaust", cfg, maxAttempts, 1,
 	)
+	sched := &recordingScheduler{}
 
-	e.ExportDeliverHTTP(
+	for attempt := 1; attempt <= maxAttempts; attempt++ {
-		context.TODO(), db, fix.Delivery, fix.Task,
+		fix.Task.AttemptNum = attempt
+
+		e.ExportDeliverHTTPWithScheduler(
+			context.TODO(), db, fix.Delivery, fix.Task, sched,
+		)
+	}
+
+	assert.Equal(t, int32(maxAttempts), requests.Load())
+	assert.Len(t, sched.delays, maxAttempts-1,
+		"every failure but the last should be retried",
 	)
-
 	assertDeliveryStatus(t, db, dlv.ID,
 		database.DeliveryStatusFailed,
 	)
+	assert.Equal(t, delivery.CircuitOpen, cb.State(),
+		"every attempt should be counted by the circuit breaker",
+	)
+}
+
+// TestDeliverHTTP_SingleAttemptSkipsCircuitBreaker proves a target
+// with one delivery attempt makes exactly that attempt, even while
+// its circuit breaker is open, and that a failure is final.
+func TestDeliverHTTP_SingleAttemptSkipsCircuitBreaker(t *testing.T) {
+	t.Parallel()
+
+	db := testWebhookDB(t)
+
+	var requests atomic.Int32
+
+	ts := httptest.NewServer(
+		http.HandlerFunc(
+			func(w http.ResponseWriter, _ *http.Request) {
+				requests.Add(1)
+				w.WriteHeader(http.StatusBadGateway)
+			},
+		),
+	)
+	defer ts.Close()
+
+	e := testEngine(t, 1)
+	targetID := uuid.New().String()
+
+	cb := e.ExportGetCircuitBreaker(targetID)
+
+	for range delivery.ExportDefaultFailureThreshold {
+		cb.RecordFailure()
+	}
+
+	require.Equal(t, delivery.CircuitOpen, cb.State())
+
+	event := seedEvent(t, db, `{"retry":"none"}`)
+
+	dlv := seedDelivery(
+		t, db, event.ID, targetID,
+		database.DeliveryStatusPending,
+	)
+
+	cfg := newHTTPTargetConfig(ts.URL)
+	fix := buildHTTPFixture(
+		dlv, event, targetID,
+		"test-http-single", cfg, 1, 1,
+	)
+	sched := &recordingScheduler{}
+
+	e.ExportDeliverHTTPWithScheduler(
+		context.TODO(), db, fix.Delivery, fix.Task, sched,
+	)
+
+	assert.Equal(t, int32(1), requests.Load(),
+		"the open circuit breaker should not hold the attempt back",
+	)
+	assert.Empty(t, sched.delays, "no retry should be scheduled")
+	assertDeliveryStatus(t, db, dlv.ID,
+		database.DeliveryStatusFailed,
+	)
+	assertDeliveryResult(
+		t, db, dlv.ID, false, http.StatusBadGateway,
+	)
 }
 
 func TestDeliverHTTP_SchedulesRetryOnFailure(
@@ -741,7 +828,7 @@ func seedConcurrencyTasks(
 			TargetName:   tasks[i].Target.Name,
 			TargetType:   tasks[i].Target.Type,
 			TargetConfig: tasks[i].Target.Config,
-			MaxRetries:   0,
+			MaxAttempts:  1,
 			AttemptNum:   1,
 		}
 	}
@@ -2051,10 +2138,10 @@ func buildSlackRetryDelivery(
 		Status:   database.DeliveryStatusPending,
 		Event:    event,
 		Target: database.Target{
-			Name:       "test-slack-retry",
+			Name:        "test-slack-retry",
-			Type:       database.TargetTypeSlack,
+			Type:        database.TargetTypeSlack,
-			Config:     cfg,
+			Config:      cfg,
-			MaxRetries: 5,
+			MaxAttempts: 5,
 		},
 	}
 	d.ID = dlv.ID
@@ -2089,11 +2176,11 @@ func TestDeliverSlack_WithRetries_SchedulesRetry(
 	)
 
 	task := &delivery.Task{
-		DeliveryID: dlv.ID,
+		DeliveryID:  dlv.ID,
-		TargetID:   targetID,
+		TargetID:    targetID,
-		TargetType: database.TargetTypeSlack,
+		TargetType:  database.TargetTypeSlack,
-		MaxRetries: 5,
+		MaxAttempts: 5,
-		AttemptNum: 1,
+		AttemptNum:  1,
 	}
 
 	e.ExportProcessDelivery(context.TODO(), db, d, task)

internal/delivery/event_db_isolation_test.go

@@ -152,7 +152,7 @@ func TestEventDBHoldsNoTargetRowsOnFailedDelivery(t *testing.T) {
 	)
 	task := iTask(
 		d, event, s.WebhookID, targetID,
-		"leaky-target", cfg, 0, 1, &body,
+		"leaky-target", cfg, 1, 1, &body,
 	)
 
 	s.Engine.ExportProcessNewTask(context.TODO(), &task)

internal/delivery/event_timestamp_test.go

@@ -176,7 +176,7 @@ func tsTask(
 		TargetName:   target.Name,
 		TargetType:   database.TargetTypeSlack,
 		TargetConfig: target.Config,
-		MaxRetries:   0,
+		MaxAttempts:  1,
 		Method:       event.Method,
 		Headers:      event.Headers,
 		ContentType:  event.ContentType,

internal/delivery/inflight_test.go

@@ -229,7 +229,7 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
 
 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "released",
-		database.TargetTypeLog, "", 0,
+		database.TargetTypeLog, "", 1,
 	)
 
 	event := iSeedEvent(

internal/delivery/metrics_test.go

@@ -291,20 +291,20 @@ func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
 	body := event.Body
 	cfg := iHTTPConfig(ts.URL)
 
-	// A retry budget above the failure threshold, so the breaker
+	// An attempt budget above the failure threshold, so the breaker
 	// rather than the budget is what stops the delivery.
-	maxRetries := delivery.ExportDefaultFailureThreshold + 5
+	maxAttempts := delivery.ExportDefaultFailureThreshold + 5
 
 	// The retries below are only run if their target still exists;
 	// see https://git.eeqj.de/sneak/webhooker/issues/107.
 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "metrics-trip",
-		database.TargetTypeHTTP, cfg, maxRetries,
+		database.TargetTypeHTTP, cfg, maxAttempts,
 	)
 
 	first := iTask(
 		d, event, s.WebhookID, targetID,
-		"metrics-trip", cfg, maxRetries, 1, &body,
+		"metrics-trip", cfg, maxAttempts, 1, &body,
 	)
 
 	s.Engine.ExportProcessNewTask(context.TODO(), &first)
@@ -316,7 +316,7 @@ func TestDeliveryMetrics_CircuitBreakerGauge(t *testing.T) {
 		ExportDefaultFailureThreshold; attempt++ {
 		task := iTask(
 			d, event, s.WebhookID, targetID,
-			"metrics-trip", cfg, maxRetries, attempt, &body,
+			"metrics-trip", cfg, maxAttempts, attempt, &body,
 		)
 
 		s.Engine.ExportProcessRetryTask(
@@ -364,16 +364,16 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
 
 	body := event.Body
 	cfg := iHTTPConfig(ts.URL)
-	maxRetries := delivery.ExportDefaultFailureThreshold + 5
+	maxAttempts := delivery.ExportDefaultFailureThreshold + 5
 
 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "metrics-blocked",
-		database.TargetTypeHTTP, cfg, maxRetries,
+		database.TargetTypeHTTP, cfg, maxAttempts,
 	)
 
 	first := iTask(
 		d, event, s.WebhookID, targetID,
-		"metrics-blocked", cfg, maxRetries, 1, &body,
+		"metrics-blocked", cfg, maxAttempts, 1, &body,
 	)
 
 	s.Engine.ExportProcessNewTask(context.TODO(), &first)
@@ -382,7 +382,7 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
 		ExportDefaultFailureThreshold; attempt++ {
 		task := iTask(
 			d, event, s.WebhookID, targetID,
-			"metrics-blocked", cfg, maxRetries, attempt, &body,
+			"metrics-blocked", cfg, maxAttempts, attempt, &body,
 		)
 
 		s.Engine.ExportProcessRetryTask(context.TODO(), &task)
@@ -405,7 +405,7 @@ func TestDeliveryMetrics_BreakerBlockedIsNotAnAttempt(
 
 	blocked := iTask(
 		d, event, s.WebhookID, targetID,
-		"metrics-blocked", cfg, maxRetries,
+		"metrics-blocked", cfg, maxAttempts,
 		delivery.ExportDefaultFailureThreshold+1, &body,
 	)
 

internal/delivery/recovery_durability_test.go

@@ -66,7 +66,7 @@ func TestRecoverySkipsPendingWithSuccessfulResult(
 
 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "already-delivered",
-		database.TargetTypeLog, "", 0,
+		database.TargetTypeLog, "", 1,
 	)
 
 	event := iSeedEvent(
@@ -129,7 +129,7 @@ func TestRecoveryContinuesTheAttemptNumbering(t *testing.T) {
 
 	iCreateTarget(t, s.MainDB, targetID,
 		s.WebhookID, "numbering",
-		database.TargetTypeLog, "", 0,
+		database.TargetTypeLog, "", 1,
 	)
 
 	event := iSeedEvent(
@@ -444,10 +444,10 @@ func TestFailedResultWriteWithRetriesLeavesDeliveryRecoverable(
 				Status:   database.DeliveryStatusPending,
 				Event:    event,
 				Target: database.Target{
-					Name:       "unwritable",
+					Name:        "unwritable",
-					Type:       database.TargetTypeHTTP,
+					Type:        database.TargetTypeHTTP,
-					Config:     iHTTPConfig(ts.URL),
+					Config:      iHTTPConfig(ts.URL),
-					MaxRetries: 3,
+					MaxAttempts: 3,
 				},
 			}
 			full.ID = d.ID

internal/delivery/target_config_view.go

@@ -120,8 +120,8 @@ func unavailableConfigFields() []ConfigField {
 }
 
 // slackConfigFields describes a Slack target: its masked
-// webhook URL and its retry count. Only the masked URL is
+// webhook URL and its number of delivery attempts. Only the
-// shown; the full URL is the credential.
+// masked URL is shown; the full URL is the credential.
 func slackConfigFields(t *database.Target) []ConfigField {
 	cfg, err := parseSlackConfig(t.Config)
 	if err != nil {
@@ -131,7 +131,7 @@ func slackConfigFields(t *database.Target) []ConfigField {
 	return []ConfigField{{
 		Label: "Webhook URL",
 		Value: cfg.MaskedWebhookURL(),
-	}, maxRetriesField(t)}
+	}, maxAttemptsField(t)}
 }
 
 // httpConfigFields describes an HTTP target: its destination
@@ -178,23 +178,18 @@ func httpConfigFields(t *database.Target) []ConfigField {
 		})
 	}
 
-	fields = append(fields, maxRetriesField(t))
+	fields = append(fields, maxAttemptsField(t))
 
 	return fields
 }
 
-// maxRetriesField describes a target's retry count, which lives
+// maxAttemptsField describes a target's number of delivery
-// on the target row rather than in its configuration blob. A
+// attempts, which lives on the target row rather than in its
-// stored 0 makes a single attempt, so it is shown as 1.
+// configuration blob.
-func maxRetriesField(t *database.Target) ConfigField {
+func maxAttemptsField(t *database.Target) ConfigField {
-	attempts := strconv.Itoa(t.MaxRetries)
-	if t.MaxRetries == 0 {
-		attempts = "1 (fire-and-forget: no retries, no circuit breaker)"
-	}
-
 	return ConfigField{
 		Label: "Delivery attempts",
-		Value: attempts,
+		Value: strconv.Itoa(t.MaxAttempts),
 	}
 }
 

internal/delivery/target_config_view_test.go

@@ -32,7 +32,7 @@ const (
 	viewMaskedOrigin  = viewExampleOrigin + "/..."
 	viewUnavailable   = "(unavailable)"
 	viewExpiryNever   = "never"
-	viewMaxRetries    = "Delivery attempts"
+	viewMaxAttempts   = "Delivery attempts"
 )
 
 func TestMaskedWebhookURL(t *testing.T) {
@@ -171,6 +171,7 @@ func slackTarget() database.Target {
 		Active: true,
 		Config: `{"webhookUrl":"` +
 			slackWebhookURL + `"}`,
+		MaxAttempts: 1,
 	}
 }
 
@@ -189,28 +190,29 @@ func TestNewTargetViews_Slack(t *testing.T) {
 	assert.Equal(
 		t,
 		map[string]string{
-			"Webhook URL":  slackMaskedURL,
+			"Webhook URL":   slackMaskedURL,
-			viewMaxRetries: "1 (fire-and-forget: no retries, no circuit breaker)",
+			viewMaxAttempts: "1",
 		},
 		fieldMap(view.Config),
 	)
 }
 
-// TestNewTargetViews_SlackRetries proves a Slack target shows
+// TestNewTargetViews_SlackAttempts proves a Slack target shows
-// its retry count the same way an HTTP target does.
+// its number of delivery attempts the same way an HTTP target
-func TestNewTargetViews_SlackRetries(t *testing.T) {
+// does.
+func TestNewTargetViews_SlackAttempts(t *testing.T) {
 	t.Parallel()
 
 	target := slackTarget()
-	target.MaxRetries = 2
+	target.MaxAttempts = 2
 
 	view := viewFor(t, target)
 
 	assert.Equal(
 		t,
 		map[string]string{
-			"Webhook URL":  slackMaskedURL,
+			"Webhook URL":   slackMaskedURL,
-			viewMaxRetries: "2",
+			viewMaxAttempts: "2",
 		},
 		fieldMap(view.Config),
 	)
@@ -225,7 +227,7 @@ func TestNewTargetViews_HTTP(t *testing.T) {
 			`"timeout":30,` +
 			`"headers":{"Authorization":"Bearer sekrit"},` +
 			`"forwardQuery":true}`,
-		MaxRetries: 5,
+		MaxAttempts: 5,
 	})
 
 	fields := fieldMap(view.Config)
@@ -237,7 +239,7 @@ func TestNewTargetViews_HTTP(t *testing.T) {
 			"Timeout":         "30s",
 			"Headers":         "1 configured",
 			"Query string":    "passed on to this target",
-			viewMaxRetries:    "5",
+			viewMaxAttempts:   "5",
 		},
 		fields,
 	)
@@ -248,19 +250,22 @@ func TestNewTargetViews_HTTP(t *testing.T) {
 	}
 }
 
-func TestNewTargetViews_HTTPFireAndForget(t *testing.T) {
+// TestNewTargetViews_HTTPSingleAttempt proves a single attempt is
+// shown as the plain number, like any other.
+func TestNewTargetViews_HTTPSingleAttempt(t *testing.T) {
 	t.Parallel()
 
 	view := viewFor(t, database.Target{
-		Type:   database.TargetTypeHTTP,
+		Type:        database.TargetTypeHTTP,
-		Config: `{"url":"` + viewExampleHook + `"}`,
+		Config:      `{"url":"` + viewExampleHook + `"}`,
+		MaxAttempts: 1,
 	})
 
 	assert.Equal(
 		t,
 		map[string]string{
 			"Destination URL": viewMaskedOrigin,
-			viewMaxRetries:    "1 (fire-and-forget: no retries, no circuit breaker)",
+			viewMaxAttempts:   "1",
 		},
 		fieldMap(view.Config),
 	)

internal/delivery/target_http.go

@@ -53,10 +53,10 @@ type httpCore struct {
 }
 
 // deliver runs one delivery attempt through the retry core.
-// A maxRetries of 0 is fire-and-forget: a single attempt is
+// A maxAttempts of 1 is a single attempt, recorded without
-// recorded and no circuit breaker is consulted. A positive
+// consulting a circuit breaker. A higher maxAttempts gates the
-// maxRetries gates the attempt on the circuit breaker and
+// attempt on the circuit breaker and schedules a backed-off retry
-// schedules a backed-off retry on failure.
+// on failure, until maxAttempts attempts have been made.
 //
 // ctx is the context attempt sends its request with. Stopping the
 // engine cancels it; see interruptedByStop.
@@ -66,17 +66,17 @@ func (c *httpCore) deliver(
 	d *database.Delivery,
 	task *Task,
 	sched Scheduler,
-	maxRetries int,
+	maxAttempts int,
 	attempt func() attemptResult,
 ) {
-	if maxRetries == 0 {
+	if maxAttempts <= 1 {
 		c.fireAndForget(ctx, webhookDB, d, attempt())
 
 		return
 	}
 
 	c.withRetry(
-		ctx, webhookDB, d, task, sched, maxRetries, attempt,
+		ctx, webhookDB, d, task, sched, maxAttempts, attempt,
 	)
 }
 
@@ -153,7 +153,7 @@ func (c *httpCore) withRetry(
 	d *database.Delivery,
 	task *Task,
 	sched Scheduler,
-	maxRetries int,
+	maxAttempts int,
 	attempt func() attemptResult,
 ) {
 	cb := c.getCircuitBreaker(task.TargetID)
@@ -205,7 +205,7 @@ func (c *httpCore) withRetry(
 	cb.RecordFailure()
 
 	c.handleRetry(
-		webhookDB, d, task, sched, maxRetries, attemptNum,
+		webhookDB, d, task, sched, maxAttempts, attemptNum,
 	)
 }
 
@@ -266,10 +266,10 @@ func (c *httpCore) handleRetry(
 	d *database.Delivery,
 	task *Task,
 	sched Scheduler,
-	maxRetries int,
+	maxAttempts int,
 	attemptNum int,
 ) {
-	if attemptNum >= maxRetries {
+	if attemptNum >= maxAttempts {
 		c.eng.settleStatus(
 			webhookDB, d, d.Target.Type,
 			database.DeliveryStatusFailed,
@@ -439,7 +439,7 @@ func (t *httpTarget) Deliver(
 
 	t.deliver(
 		ctx, webhookDB, d, task, sched,
-		d.Target.MaxRetries, attempt,
+		d.Target.MaxAttempts, attempt,
 	)
 }
 

internal/delivery/target_slack.go

@@ -30,9 +30,8 @@ type SlackTargetConfig struct {
 // slackTarget delivers events to Slack incoming webhooks. It
 // formats the event into a Slack message and posts it as
 // JSON. It shares the retry core with the HTTP target: a
-// MaxRetries of 0 stays single-attempt fire-and-forget
+// MaxAttempts of 1 is a single attempt, while a higher one
-// (preserving existing Slack targets), while a positive
+// adds backoff and circuit breaking.
-// MaxRetries adds backoff and circuit breaking.
 type slackTarget struct {
 	*httpCore
 
@@ -83,7 +82,7 @@ func (t *slackTarget) Deliver(
 
 	t.deliver(
 		ctx, webhookDB, d, task, sched,
-		d.Target.MaxRetries, attempt,
+		d.Target.MaxAttempts, attempt,
 	)
 }
 

internal/delivery/terminal_state_test.go

@@ -743,7 +743,7 @@ func TestSendRecoveredDeliveries_TargetMissingFromMap(
 
 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "found-on-lookup",
-		database.TargetTypeLog, "", 0,
+		database.TargetTypeLog, "", 1,
 	)
 
 	event := iSeedEvent(
@@ -790,7 +790,7 @@ func TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone(
 
 	iCreateTarget(
 		t, s.MainDB, targetID, s.WebhookID, "healthy",
-		database.TargetTypeLog, "", 0,
+		database.TargetTypeLog, "", 1,
 	)
 
 	event := iSeedEvent(

internal/handlers/delivery_replay.go

@@ -308,7 +308,7 @@ func createReplayDelivery(
 		TargetName:   target.Name,
 		TargetType:   target.Type,
 		TargetConfig: target.Config,
-		MaxRetries:   target.MaxRetries,
+		MaxAttempts:  target.MaxAttempts,
 		Method:       event.Method,
 		RawQuery:     event.RawQuery,
 		Headers:      event.Headers,

internal/handlers/event_log.go

@@ -496,7 +496,7 @@ const deliveryIDChunkSize = 500
 //
 // Each response body is cut by SQLite rather than in Go, for
 // the reason deliveryResultColumns gives. How many attempts a
-// delivery has is the target's MaxRetries, which the
+// delivery has is bounded by the target's MaxAttempts, which the
 // authenticated operator sets; how many of them reach the page
 // is bounded again by maxRenderedAttempts.
 func (h *Handlers) loadDeliveryResults(

internal/handlers/export_test.go

@@ -37,10 +37,10 @@ const MaxRenderedResponseBytesForTest = maxRenderedResponseBytes
 // per-delivery attempt ceiling to the handlers_test package.
 const MaxRenderedAttemptsForTest = maxRenderedAttempts
 
-// MaxTargetRetriesForTest exposes the target max_retries ceiling to
+// MaxTargetAttemptsForTest exposes the target max_attempts ceiling to
 // the handlers_test package, so the tests assert against the constant
 // the handlers enforce rather than a number copied beside it.
-const MaxTargetRetriesForTest = maxTargetRetries
+const MaxTargetAttemptsForTest = maxTargetAttempts
 
 // EventDBLeftMsgForTest and SidecarLeftMsgForTest expose the two
 // messages the webhook delete handler logs when a file of the event

internal/handlers/target_attempts.go

@@ -0,0 +1,81 @@
+package handlers
+
+import (
+	"errors"
+	"strconv"
+	"strings"
+)
+
+// maxTargetAttempts bounds a target's max_attempts.
+//
+// Both target forms already declare max="20" on the input, so this
+// enforces server-side what the UI has always advertised rather than
+// introducing a new limit.
+//
+// The number is not cosmetic. Every attempt writes a delivery_results
+// row that the event log then loads and renders, and the engine backs
+// off by 2^(n-1) seconds, so attempt 20 is already about six days
+// after the first. A value beyond this buys no additional durability
+// and only costs rows.
+const maxTargetAttempts = 20
+
+// Errors returned when a max_attempts form value cannot be turned into
+// a number of attempts.
+var (
+	// errAttemptsInvalid signals a max_attempts form value that is not
+	// a whole number of at least 1.
+	errAttemptsInvalid = errors.New(
+		"must be a whole number, at least 1",
+	)
+
+	// errAttemptsTooLarge signals a max_attempts form value that is a
+	// whole number but above maxTargetAttempts. It is distinguished
+	// from errAttemptsInvalid so the message can name the ceiling
+	// instead of implying the input was not a number.
+	errAttemptsTooLarge = errors.New("out of range")
+)
+
+// parseMaxAttempts interprets a max_attempts form value.
+//
+// An ABSENT value — the field empty or not submitted — yields
+// fallback, which lets the create path apply its default and the edit
+// path leave the stored value alone. A value that is SET BUT INVALID
+// is an error: unparseable, below 1, or above maxTargetAttempts.
+//
+// The distinction is the whole point of this function. Returning a
+// default for input the operator typed but that did not parse would
+// silently turn retries off on a store-and-forward proxy — and on the
+// edit path destroy a working configuration over a typo. A default
+// answers a question that was not asked; it never answers one that
+// was asked badly.
+func parseMaxAttempts(raw string, fallback int) (int, error) {
+	raw = strings.TrimSpace(raw)
+	if raw == "" {
+		return fallback, nil
+	}
+
+	v, err := strconv.Atoi(raw)
+	if err != nil || v < 1 {
+		return 0, errAttemptsInvalid
+	}
+
+	if v > maxTargetAttempts {
+		return 0, errAttemptsTooLarge
+	}
+
+	return v, nil
+}
+
+// attemptsErrorMessage returns the message the create and edit forms
+// show for a rejected max_attempts value. Any error other than
+// errAttemptsTooLarge falls back to the generic wording, so an
+// unrecognised parse failure still produces a sensible 400.
+func attemptsErrorMessage(err error) string {
+	if errors.Is(err, errAttemptsTooLarge) {
+		return errAttemptsTooLarge.Error() +
+			": at most " + strconv.Itoa(maxTargetAttempts) +
+			" attempts"
+	}
+
+	return errAttemptsInvalid.Error()
+}

internal/handlers/target_attempts_test.go

@@ -0,0 +1,429 @@
+package handlers_test
+
+import (
+	"net/http"
+	"net/url"
+	"strconv"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/database"
+	"sneak.berlin/go/webhooker/internal/handlers"
+)
+
+// attemptsTargetURL is the destination the attempts-validation targets
+// point at. It is a literal public address rather than a hostname so
+// the SSRF check resolves nothing and a sandbox without DNS cannot
+// make these cases pass or fail for the wrong reason.
+const attemptsTargetURL = "https://93.184.216.34/hooks/attempts"
+
+const (
+	// wayAboveCeiling is the typo'd-extra-zero case from the report.
+	wayAboveCeiling = "999999999"
+
+	// notANumber is the plainest garbage an operator can type, and
+	// the value the report submitted on the edit form.
+	notANumber = "abc"
+
+	// workingAttempts is the number of attempts a seeded target is
+	// already delivering with, which a rejected submission must not
+	// disturb.
+	workingAttempts = 2
+)
+
+// aboveCeiling is the smallest rejected whole number.
+func aboveCeiling() string {
+	return strconv.Itoa(handlers.MaxTargetAttemptsForTest + 1)
+}
+
+// overCeilingAttempts is whole-number input past the limit, which is
+// rejected with the limit named.
+func overCeilingAttempts() []string {
+	return []string{aboveCeiling(), wayAboveCeiling}
+}
+
+// belowOneAttempts is whole-number input under the floor of one
+// attempt, which is rejected with the floor named.
+func belowOneAttempts() []string {
+	return []string{"0", "-5"}
+}
+
+// unparseableAttempts is input an operator can type into the field
+// that is not a number of attempts. Each must be REJECTED: silently
+// reading any of them as a default turns retries off on a
+// store-and-forward proxy without saying so.
+//
+// The twenty-digit case is here because it parses as digits but
+// overflows int, which is the one failure the field's own min/max
+// attributes cannot describe.
+func unparseableAttempts() []string {
+	return []string{
+		notANumber,
+		"2.7",
+		"12345678901234567890",
+		"1e3",
+	}
+}
+
+// createAttemptsForm is a complete, otherwise-valid HTTP target
+// creation, so the only thing any case below varies is max_attempts.
+func createAttemptsForm(attempts string) url.Values {
+	form := url.Values{}
+	form.Set("name", "attempts-target")
+	form.Set("type", string(database.TargetTypeHTTP))
+	form.Set("url", attemptsTargetURL)
+
+	if attempts != absentField {
+		form.Set("max_attempts", attempts)
+	}
+
+	return form
+}
+
+// absentField marks a field the form does not submit at all, which is
+// the case that legitimately takes a default and must stay distinct
+// from a field submitted with garbage in it.
+const absentField = "\x00absent"
+
+// absentAttempts is every way of saying "the operator did not set
+// this", each of which takes the default rather than a 400. Blank and
+// whitespace-only count as absent here because they do in the timeout
+// and retention controls on the same forms; a rule the fields do not
+// share would be its own surprise.
+func absentAttempts() []string {
+	return []string{absentField, "", "  "}
+}
+
+// createWithAttempts posts the target create form for a fresh webhook
+// and returns the webhook and the response.
+func createWithAttempts(
+	t *testing.T,
+	env *sourceTestEnv,
+	attempts string,
+) (database.Webhook, int, string) {
+	t.Helper()
+
+	webhook := seedWebhookWithRetention(t, env.db, 30)
+
+	w := serveTarget(
+		env, http.MethodPost,
+		"/hook/"+webhook.ID+"/targets",
+		createAttemptsForm(attempts),
+	)
+
+	return webhook, w.Code, w.Body.String()
+}
+
+// TestTargetCreate_AttemptsAboveCeilingRejected proves the create form
+// enforces a ceiling at all, and that the 400 names it — a rejection
+// that does not say what the limit is leaves the operator guessing.
+func TestTargetCreate_AttemptsAboveCeilingRejected(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	ceiling := strconv.Itoa(handlers.MaxTargetAttemptsForTest)
+
+	for _, attempts := range overCeilingAttempts() {
+		webhook, code, body := createWithAttempts(t, env, attempts)
+
+		assert.Equal(t, http.StatusBadRequest, code,
+			"max_attempts=%s should be rejected", attempts)
+		assert.Contains(t, body, ceiling,
+			"the rejection for %s should name the ceiling",
+			attempts)
+		assert.Empty(t,
+			targetsForWebhook(t, env.db, webhook.ID),
+			"no target should be created for %s", attempts)
+	}
+}
+
+// TestTargetCreate_AttemptsBelowOneRejected proves the create form
+// refuses 0 and less, naming the floor: every delivery is attempted
+// at least once.
+func TestTargetCreate_AttemptsBelowOneRejected(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, attempts := range belowOneAttempts() {
+		webhook, code, body := createWithAttempts(t, env, attempts)
+
+		assert.Equal(t, http.StatusBadRequest, code,
+			"max_attempts=%s should be rejected", attempts)
+		assert.Contains(t, body, "at least 1",
+			"the rejection for %s should name the floor",
+			attempts)
+		assert.Empty(t,
+			targetsForWebhook(t, env.db, webhook.ID),
+			"no target should be created for %s", attempts)
+	}
+}
+
+// TestTargetCreate_UnparseableAttemptsRejected is the core of the
+// defect: each of these was accepted with HTTP 200 and stored as a
+// default.
+func TestTargetCreate_UnparseableAttemptsRejected(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, attempts := range unparseableAttempts() {
+		webhook, code, body := createWithAttempts(t, env, attempts)
+
+		assert.Equal(t, http.StatusBadRequest, code,
+			"max_attempts=%q should be rejected, not coerced",
+			attempts)
+		assert.Contains(t, body, "whole number",
+			"the rejection for %q should say why", attempts)
+		assert.Empty(t,
+			targetsForWebhook(t, env.db, webhook.ID),
+			"no target should be created for %q", attempts)
+	}
+}
+
+// TestTargetCreate_ValidAttemptsStored covers the accepting half,
+// including both bounds: a bound that rejects its own limit would
+// make the advertised range unreachable.
+func TestTargetCreate_ValidAttemptsStored(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, want := range []int{1, 3, handlers.MaxTargetAttemptsForTest} {
+		webhook, code, body := createWithAttempts(
+			t, env, strconv.Itoa(want),
+		)
+
+		require.Equal(t, http.StatusSeeOther, code, body)
+
+		targets := targetsForWebhook(t, env.db, webhook.ID)
+		require.Len(t, targets, 1)
+		assert.Equal(t, want, targets[0].MaxAttempts)
+	}
+}
+
+// TestTargetCreate_AbsentAttemptsTakesDefault keeps the two cases
+// distinct. An omitted field is not an operator asking for something
+// invalid, so it still gets the default of one attempt rather than a
+// 400 — otherwise the fix above would make the form unusable.
+func TestTargetCreate_AbsentAttemptsTakesDefault(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, attempts := range absentAttempts() {
+		webhook, code, body := createWithAttempts(t, env, attempts)
+
+		require.Equal(t, http.StatusSeeOther, code, body)
+
+		targets := targetsForWebhook(t, env.db, webhook.ID)
+		require.Len(t, targets, 1)
+		assert.Equal(t, 1, targets[0].MaxAttempts,
+			"an absent max_attempts should take the default")
+	}
+}
+
+// seedAttemptsTarget creates an HTTP target already delivering with
+// workingAttempts attempts, through the real create handler.
+func seedAttemptsTarget(
+	t *testing.T,
+	env *sourceTestEnv,
+) (database.Webhook, database.Target) {
+	t.Helper()
+
+	webhook, code, body := createWithAttempts(
+		t, env, strconv.Itoa(workingAttempts),
+	)
+	require.Equal(t, http.StatusSeeOther, code, body)
+
+	targets := targetsForWebhook(t, env.db, webhook.ID)
+	require.Len(t, targets, 1)
+	require.Equal(t, workingAttempts, targets[0].MaxAttempts)
+
+	return webhook, targets[0]
+}
+
+// editAttemptsForm is a complete edit submission that changes the
+// target's name as well, so a rejected submission can be shown to
+// have written nothing at all rather than merely to have left
+// max_attempts alone.
+func editAttemptsForm(attempts string) url.Values {
+	form := url.Values{}
+	form.Set("name", "renamed-by-edit")
+	form.Set("url", attemptsTargetURL)
+
+	if attempts != absentField {
+		form.Set("max_attempts", attempts)
+	}
+
+	return form
+}
+
+// assertEditRejectedAndUnchanged submits an edit expected to fail and
+// checks both halves of the requirement: the 400 explains itself, and
+// the target it was submitted against is untouched.
+func assertEditRejectedAndUnchanged(
+	t *testing.T,
+	env *sourceTestEnv,
+	attempts, wantReason string,
+) {
+	t.Helper()
+
+	webhook, target := seedAttemptsTarget(t, env)
+
+	w := submitTargetEdit(
+		env, webhook.ID, target.ID, editAttemptsForm(attempts),
+	)
+
+	assert.Equal(t, http.StatusBadRequest, w.Code,
+		"max_attempts=%q should be rejected on edit", attempts)
+	assert.Contains(t, w.Body.String(), wantReason,
+		"the rejection for %q should say why", attempts)
+
+	stored := storedTarget(t, env, target.ID)
+	assert.Equal(t, workingAttempts, stored.MaxAttempts,
+		"a rejected edit must not destroy the working number "+
+			"of attempts with %q", attempts)
+	assert.Equal(t, "attempts-target", stored.Name,
+		"a rejected edit must write nothing at all")
+}
+
+// TestTargetEdit_UnparseableAttemptsRejected is the damaging half of
+// the defect. A target delivering with retries, re-saved with a typo
+// in the field, returned 200 and was left with retries disabled and
+// nothing said.
+func TestTargetEdit_UnparseableAttemptsRejected(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, attempts := range unparseableAttempts() {
+		assertEditRejectedAndUnchanged(
+			t, env, attempts, "whole number",
+		)
+	}
+}
+
+// TestTargetEdit_AttemptsBelowOneRejected proves the floor applies
+// to the edit path too.
+func TestTargetEdit_AttemptsBelowOneRejected(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, attempts := range belowOneAttempts() {
+		assertEditRejectedAndUnchanged(t, env, attempts, "at least 1")
+	}
+}
+
+// TestTargetEdit_AttemptsAboveCeilingRejected proves the ceiling
+// applies to the edit path too, naming itself, so the two paths
+// cannot disagree about what is storable.
+func TestTargetEdit_AttemptsAboveCeilingRejected(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	ceiling := strconv.Itoa(handlers.MaxTargetAttemptsForTest)
+
+	for _, attempts := range overCeilingAttempts() {
+		assertEditRejectedAndUnchanged(t, env, attempts, ceiling)
+	}
+}
+
+// TestTargetEdit_ValidAttemptsStored covers the accepting half of the
+// edit path, so the bounds cannot be enforced by simply refusing
+// every submission.
+func TestTargetEdit_ValidAttemptsStored(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, want := range []int{1, 9, handlers.MaxTargetAttemptsForTest} {
+		webhook, target := seedAttemptsTarget(t, env)
+
+		w := submitTargetEdit(
+			env, webhook.ID, target.ID,
+			editAttemptsForm(strconv.Itoa(want)),
+		)
+		require.Equal(t,
+			http.StatusSeeOther, w.Code, w.Body.String(),
+		)
+
+		assert.Equal(t, want,
+			storedTarget(t, env, target.ID).MaxAttempts)
+	}
+}
+
+// TestTargetEdit_AbsentAttemptsLeavesStoredValue is the edit path's
+// absent-versus-invalid case. The field is only offered by the forms
+// for types that retry, so a submission without it must leave the
+// stored count alone rather than be rejected or reset.
+func TestTargetEdit_AbsentAttemptsLeavesStoredValue(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	for _, attempts := range absentAttempts() {
+		webhook, target := seedAttemptsTarget(t, env)
+
+		w := submitTargetEdit(
+			env, webhook.ID, target.ID,
+			editAttemptsForm(attempts),
+		)
+		require.Equal(t,
+			http.StatusSeeOther, w.Code, w.Body.String(),
+		)
+
+		assert.Equal(t, workingAttempts,
+			storedTarget(t, env, target.ID).MaxAttempts,
+			"an absent max_attempts must leave the stored "+
+				"count alone (%q)", attempts)
+	}
+}
+
+// TestTargetAttempts_CreateAndEditAgreeOnEveryCase proves the two
+// paths cannot disagree, which is what let the create form and the
+// edit form drift apart in the first place. Every input is submitted
+// to both and the accept/reject verdicts are compared.
+func TestTargetAttempts_CreateAndEditAgreeOnEveryCase(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+
+	accepted := []string{
+		"1", "2",
+		strconv.Itoa(handlers.MaxTargetAttemptsForTest),
+	}
+	overCeiling := overCeilingAttempts()
+	belowOne := belowOneAttempts()
+	unparseable := unparseableAttempts()
+
+	cases := make(
+		[]string, 0,
+		len(accepted)+len(overCeiling)+len(belowOne)+
+			len(unparseable),
+	)
+	cases = append(cases, accepted...)
+	cases = append(cases, overCeiling...)
+	cases = append(cases, belowOne...)
+	cases = append(cases, unparseable...)
+
+	for _, attempts := range cases {
+		_, createCode, _ := createWithAttempts(t, env, attempts)
+
+		webhook, target := seedAttemptsTarget(t, env)
+		editCode := submitTargetEdit(
+			env, webhook.ID, target.ID,
+			editAttemptsForm(attempts),
+		).Code
+
+		assert.Equal(t,
+			createCode == http.StatusBadRequest,
+			editCode == http.StatusBadRequest,
+			"create and edit must agree on max_attempts=%q "+
+				"(create %d, edit %d)",
+			attempts, createCode, editCode,
+		)
+	}
+}

internal/handlers/target_create.go

@@ -102,9 +102,10 @@ func (h *Handlers) newTarget(
 	in targetFormInput,
 ) (*database.Target, string, error) {
 	target := &database.Target{
-		WebhookID: webhookID,
+		WebhookID:   webhookID,
-		Type:      in.Type,
+		Type:        in.Type,
-		Active:    true,
+		Active:      true,
+		MaxAttempts: 1,
 	}
 
 	errMsg, err := h.setTargetFromForm(ctx, target, in)
@@ -117,9 +118,10 @@ func (h *Handlers) newTarget(
 
 // setTargetFromForm validates a target form against the target's type
 // and, when it accepts it, sets the target's name, configuration and
-// retry count from it. It returns the message the form shows for
+// number of delivery attempts from it. It returns the message the form
-// anything it refuses, an unknown type among them, and then leaves the
+// shows for anything it refuses, an unknown type among them, and then
-// target unchanged; an error is the server's fault, as for newTarget.
+// leaves the target unchanged; an error is the server's fault, as for
+// newTarget.
 // The add target form and the target edit form both go through here,
 // so the two cannot come to disagree about what a target may be.
 func (h *Handlers) setTargetFromForm(
@@ -136,20 +138,20 @@ func (h *Handlers) setTargetFromForm(
 		return errMsg, err
 	}
 
-	// An empty max_retries keeps the target's count: the
+	// An empty max_attempts keeps the target's count: the default of
-	// fire-and-forget default of 0 for a new target, and the stored
+	// 1 for a new target, and the stored count for an edited one,
-	// count for an edited one, since the forms for target types that
+	// since the forms for target types that do not retry have no such
-	// do not retry have no such field. A value that is filled in but
+	// field. A value that is filled in but invalid is refused rather
-	// invalid is refused rather than becoming that count, so a typo
+	// than becoming that count, so a typo cannot destroy the count a
-	// cannot destroy the count a target is delivering with.
+	// target is delivering with.
-	maxRetries, err := parseMaxRetries(in.MaxRetries, target.MaxRetries)
+	maxAttempts, err := parseMaxAttempts(in.MaxAttempts, target.MaxAttempts)
 	if err != nil {
-		return "Invalid delivery attempts: " + retriesErrorMessage(err), nil
+		return "Invalid delivery attempts: " + attemptsErrorMessage(err), nil
 	}
 
 	target.Name = in.Name
 	target.Config = configJSON
-	target.MaxRetries = maxRetries
+	target.MaxAttempts = maxAttempts
 
 	return "", nil
 }
@@ -176,8 +178,8 @@ type targetFormInput struct {
 	// ForwardQuery is an HTTP target's checkbox that passes each
 	// event's query string on to it.
 	ForwardQuery bool
-	// MaxRetries is an HTTP or Slack target's max_retries.
+	// MaxAttempts is an HTTP or Slack target's max_attempts.
-	MaxRetries string
+	MaxAttempts string
 	// Expiry is a database (archive) target's row expiry.
 	Expiry string
 	// Rotation is a database (archive) target's rotation.
@@ -204,7 +206,7 @@ func targetFormInputFrom(r *http.Request) targetFormInput {
 		Headers:      r.PostFormValue("headers"),
 		Timeout:      r.PostFormValue("timeout"),
 		ForwardQuery: r.PostFormValue("forward_query") != "",
-		MaxRetries:   r.PostFormValue("max_retries"),
+		MaxAttempts:  r.PostFormValue("max_attempts"),
 		Expiry:       r.PostFormValue("expiry"),
 		Rotation:     r.PostFormValue("rotation"),
 	}

internal/handlers/target_create_query_test.go

@@ -196,7 +196,7 @@ func TestHandleTargetCreate_QueryStringCannotSupplyNameOrType(
 
 	w, _ := postTargetCreate(
 		t, env, webhook.ID,
-		"name=leaky&type=slack&max_retries=9&expiry=30d&url="+
+		"name=leaky&type=slack&max_attempts=9&expiry=30d&url="+
 			url.QueryEscape(targetSecretURL),
 		url.Values{},
 	)

internal/handlers/target_create_test.go

@@ -21,24 +21,24 @@ func TestHandleTargetCreate_EveryType(t *testing.T) {
 
 	// fields is the rest of each submission, as a query string.
 	cases := []struct {
-		targetType  database.TargetType
+		targetType   database.TargetType
-		fields      string
+		fields       string
-		wantConfig  string
+		wantConfig   string
-		wantRetries int
+		wantAttempts int
 	}{
 		{
-			database.TargetTypeHTTP, "timeout=12&max_retries=3",
+			database.TargetTypeHTTP, "timeout=12&max_attempts=3",
 			`{"url":"` + editOriginalURL + `","timeout":12}`, 3,
 		},
 		{
-			database.TargetTypeSlack, "max_retries=4",
+			database.TargetTypeSlack, "max_attempts=4",
 			`{"webhookUrl":"` + editOriginalURL + `"}`, 4,
 		},
 		{
 			database.TargetTypeDatabase, "expiry=720h",
-			`{"expiry":"720h"}`, 0,
+			`{"expiry":"720h"}`, 1,
 		},
-		{database.TargetTypeLog, "", "", 0},
+		{database.TargetTypeLog, "", "", 1},
 	}
 
 	for _, tc := range cases {
@@ -62,7 +62,7 @@ func TestHandleTargetCreate_EveryType(t *testing.T) {
 			targets := targetsForWebhook(t, env.db, webhook.ID)
 			require.Len(t, targets, 1)
 			assert.Equal(t, tc.targetType, targets[0].Type)
-			assert.Equal(t, tc.wantRetries, targets[0].MaxRetries)
+			assert.Equal(t, tc.wantAttempts, targets[0].MaxAttempts)
 
 			if tc.wantConfig == "" {
 				assert.Empty(t, targets[0].Config)
@@ -90,11 +90,11 @@ func TestHandleTargetCreate_RefusedFormComesBack(t *testing.T) {
 		{
 			database.TargetTypeHTTP,
 			"name=private&url=" + editBlockedURL +
-				"&timeout=12&max_retries=3",
+				"&timeout=12&max_attempts=3",
 			"Invalid target URL",
 		},
 		{
-			database.TargetTypeSlack, "name=no-url&max_retries=4",
+			database.TargetTypeSlack, "name=no-url&max_attempts=4",
 			"Webhook URL is required for Slack targets",
 		},
 		{
@@ -133,8 +133,8 @@ func TestHandleTargetCreate_RefusedFormComesBack(t *testing.T) {
 			assert.Contains(t, page, html.EscapeString(tc.reason))
 
 			// Each value comes back in a data attribute of the targets
-			// section named after its field (max_retries as
+			// section named after its field (max_attempts as
-			// data-max-retries), except url, which comes back in
+			// data-max-attempts), except url, which comes back in
 			// data-destination; templates/source_detail.html says why.
 			for field := range typed {
 				attr := "data-" + strings.ReplaceAll(field, "_", "-")

internal/handlers/target_edit.go

@@ -82,7 +82,7 @@ func (h *Handlers) HandleTargetEdit() http.HandlerFunc {
 			Headers:      cfg.Headers,
 			Timeout:      cfg.Timeout,
 			ForwardQuery: cfg.ForwardQuery,
-			MaxRetries:   strconv.Itoa(target.MaxRetries),
+			MaxAttempts:  strconv.Itoa(target.MaxAttempts),
 			Expiry:       cfg.Expiry,
 			Rotation:     cfg.Rotation,
 		}

internal/handlers/target_edit_test.go

@@ -117,7 +117,7 @@ func seedHTTPTarget(
 	form.Set("url", editOriginalURL)
 	form.Set("headers", headers)
 	form.Set("timeout", timeout)
-	form.Set("max_retries", "3")
+	form.Set("max_attempts", "3")
 
 	w := serveTarget(
 		env, http.MethodPost,
@@ -179,7 +179,7 @@ func editForm(targetURL, headers, timeout string) url.Values {
 	form.Set("url", targetURL)
 	form.Set("headers", headers)
 	form.Set("timeout", timeout)
-	form.Set("max_retries", "5")
+	form.Set("max_attempts", "5")
 
 	return form
 }
@@ -280,9 +280,9 @@ func TestHandleTargetEditSubmit_Saves(t *testing.T) {
 		assertEditClearingRemovesThem(t, env)
 	})
 
-	t.Run("absent max_retries is not zeroed", func(t *testing.T) {
+	t.Run("absent max_attempts is kept", func(t *testing.T) {
 		t.Parallel()
-		assertEditKeepsAbsentMaxRetries(t, env)
+		assertEditKeepsAbsentMaxAttempts(t, env)
 	})
 }
 
@@ -307,7 +307,7 @@ func assertEditChangesDestination(
 
 	reloaded := storedTarget(t, env, target.ID)
 	assert.Equal(t, "edited-name", reloaded.Name)
-	assert.Equal(t, 5, reloaded.MaxRetries)
+	assert.Equal(t, 5, reloaded.MaxAttempts)
 	assert.Equal(
 		t, database.TargetTypeHTTP, reloaded.Type,
 		"the edit form must not change a target's type",
@@ -368,25 +368,25 @@ func assertEditClearingRemovesThem(
 	assert.Zero(t, cfg.Timeout)
 }
 
-// Retries are offered only by the forms for target types that retry.
+// Delivery attempts are offered only by the forms for target types
-// An absent field means the form does not edit retries, not that
+// that retry. An absent field means the form does not edit them, not
-// they should be turned off.
+// that retries should be turned off.
-func assertEditKeepsAbsentMaxRetries(
+func assertEditKeepsAbsentMaxAttempts(
 	t *testing.T, env *sourceTestEnv,
 ) {
 	t.Helper()
 
 	webhook, target := seedHTTPTarget(t, env, "", "")
-	require.Equal(t, 3, target.MaxRetries)
+	require.Equal(t, 3, target.MaxAttempts)
 
 	form := editForm(editOriginalURL, "", "")
-	form.Del("max_retries")
+	form.Del("max_attempts")
 
 	w := submitTargetEdit(env, webhook.ID, target.ID, form)
 	require.Equal(t, http.StatusSeeOther, w.Code, w.Body.String())
 
 	assert.Equal(
-		t, 3, storedTarget(t, env, target.ID).MaxRetries,
+		t, 3, storedTarget(t, env, target.ID).MaxAttempts,
 	)
 }
 
@@ -659,12 +659,12 @@ func TestHandleTargetEditSubmit_RefusedFormComesBack(t *testing.T) {
 		{
 			database.TargetTypeHTTP,
 			"name=edited&url=" + editBlockedURL +
-				"&headers=X-Edited:+kept&timeout=12&max_retries=3",
+				"&headers=X-Edited:+kept&timeout=12&max_attempts=3",
 			"Invalid target URL",
 		},
 		{
 			database.TargetTypeSlack,
-			"name=edited&url=" + editOriginalURL + "&max_retries=25",
+			"name=edited&url=" + editOriginalURL + "&max_attempts=25",
 			"Invalid delivery attempts",
 		},
 		{

internal/handlers/target_retries.go

@@ -1,83 +0,0 @@
-package handlers
-
-import (
-	"errors"
-	"strconv"
-	"strings"
-)
-
-// maxTargetRetries bounds a target's max_retries.
-//
-// Both target forms already declare max="20" on the input, so this
-// enforces server-side what the UI has always advertised rather than
-// introducing a new limit.
-//
-// The number is not cosmetic. Every attempt writes a delivery_results
-// row that the event log then loads and renders, and the engine backs
-// off by 2^(n-1) seconds, so attempt 20 is already about six days
-// after the first. A value beyond this buys no additional durability
-// and only costs rows.
-const maxTargetRetries = 20
-
-// Errors returned when a max_retries form value cannot be turned into
-// a retry count.
-var (
-	// errRetriesInvalid signals a max_retries form value that is not
-	// a non-negative whole number.
-	errRetriesInvalid = errors.New(
-		"must be a whole number",
-	)
-
-	// errRetriesTooLarge signals a max_retries form value that is a
-	// whole number but above maxTargetRetries. It is distinguished
-	// from errRetriesInvalid so the message can name the ceiling
-	// instead of implying the input was not a number.
-	errRetriesTooLarge = errors.New("out of range")
-)
-
-// parseMaxRetries interprets a max_retries form value.
-//
-// An ABSENT value — the field empty or not submitted — yields
-// fallback, which lets the create path apply its default and the edit
-// path leave the stored value alone. A value that is SET BUT INVALID
-// is an error: unparseable, negative, or above maxTargetRetries.
-//
-// The distinction is the whole point of this function. max_retries=0
-// means fire-and-forget, so returning 0 for input the operator typed
-// but that did not parse silently disables retries on a
-// store-and-forward proxy — and on the edit path it destroys a
-// working retry configuration over a typo. A default answers a
-// question that was not asked; it never answers one that was asked
-// badly.
-func parseMaxRetries(raw string, fallback int) (int, error) {
-	raw = strings.TrimSpace(raw)
-	if raw == "" {
-		return fallback, nil
-	}
-
-	v, err := strconv.Atoi(raw)
-	if err != nil || v < 0 {
-		return 0, errRetriesInvalid
-	}
-
-	if v > maxTargetRetries {
-		return 0, errRetriesTooLarge
-	}
-
-	return v, nil
-}
-
-// retriesErrorMessage returns the message the create and edit forms
-// show for a rejected max_retries value. Any error other than
-// errRetriesTooLarge falls back to the generic wording, so an
-// unrecognised parse failure still produces a sensible 400.
-func retriesErrorMessage(err error) string {
-	if errors.Is(err, errRetriesTooLarge) {
-		return errRetriesTooLarge.Error() +
-			": at most " + strconv.Itoa(maxTargetRetries) +
-			" attempts"
-	}
-
-	return errRetriesInvalid.Error() +
-		", or 0 for fire-and-forget"
-}

internal/handlers/target_retries_test.go

@@ -1,385 +0,0 @@
-package handlers_test
-
-import (
-	"net/http"
-	"net/url"
-	"strconv"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-)
-
-// retriesTargetURL is the destination the retry-validation targets
-// point at. It is a literal public address rather than a hostname so
-// the SSRF check resolves nothing and a sandbox without DNS cannot
-// make these cases pass or fail for the wrong reason.
-const retriesTargetURL = "https://93.184.216.34/hooks/retries"
-
-const (
-	// wayAboveCeiling is the typo'd-extra-zero case from the report.
-	wayAboveCeiling = "999999999"
-
-	// notANumber is the plainest garbage an operator can type, and
-	// the value the report submitted on the edit form.
-	notANumber = "abc"
-
-	// workingRetries is the retry count a seeded target is already
-	// delivering with, which a rejected submission must not disturb.
-	workingRetries = 2
-)
-
-// aboveCeiling is the smallest rejected whole number.
-func aboveCeiling() string {
-	return strconv.Itoa(handlers.MaxTargetRetriesForTest + 1)
-}
-
-// overCeilingRetries is whole-number input past the limit, which is
-// rejected with the limit named.
-func overCeilingRetries() []string {
-	return []string{aboveCeiling(), wayAboveCeiling}
-}
-
-// unparseableRetries is input an operator can type into the field
-// that is not a retry count. Each must be REJECTED: silently reading
-// any of them as 0 turns a store-and-forward proxy into
-// fire-and-forget without saying so.
-//
-// The twenty-digit case is here because it parses as digits but
-// overflows int, which is the one failure the field's own min/max
-// attributes cannot describe.
-func unparseableRetries() []string {
-	return []string{
-		notANumber,
-		"2.7",
-		"-5",
-		"12345678901234567890",
-		"1e3",
-	}
-}
-
-// createRetriesForm is a complete, otherwise-valid HTTP target
-// creation, so the only thing any case below varies is max_retries.
-func createRetriesForm(retries string) url.Values {
-	form := url.Values{}
-	form.Set("name", "retries-target")
-	form.Set("type", string(database.TargetTypeHTTP))
-	form.Set("url", retriesTargetURL)
-
-	if retries != absentField {
-		form.Set("max_retries", retries)
-	}
-
-	return form
-}
-
-// absentField marks a field the form does not submit at all, which is
-// the case that legitimately takes a default and must stay distinct
-// from a field submitted with garbage in it.
-const absentField = "\x00absent"
-
-// absentRetries is every way of saying "the operator did not set
-// this", each of which takes the default rather than a 400. Blank and
-// whitespace-only count as absent here because they do in the timeout
-// and retention controls on the same forms; a rule the fields do not
-// share would be its own surprise.
-func absentRetries() []string {
-	return []string{absentField, "", "  "}
-}
-
-// createWithRetries posts the target create form for a fresh webhook
-// and returns the webhook and the response.
-func createWithRetries(
-	t *testing.T,
-	env *sourceTestEnv,
-	retries string,
-) (database.Webhook, int, string) {
-	t.Helper()
-
-	webhook := seedWebhookWithRetention(t, env.db, 30)
-
-	w := serveTarget(
-		env, http.MethodPost,
-		"/hook/"+webhook.ID+"/targets",
-		createRetriesForm(retries),
-	)
-
-	return webhook, w.Code, w.Body.String()
-}
-
-// TestTargetCreate_RetriesAboveCeilingRejected proves the create form
-// enforces a ceiling at all, and that the 400 names it — a rejection
-// that does not say what the limit is leaves the operator guessing.
-func TestTargetCreate_RetriesAboveCeilingRejected(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	ceiling := strconv.Itoa(handlers.MaxTargetRetriesForTest)
-
-	for _, retries := range overCeilingRetries() {
-		webhook, code, body := createWithRetries(t, env, retries)
-
-		assert.Equal(t, http.StatusBadRequest, code,
-			"max_retries=%s should be rejected", retries)
-		assert.Contains(t, body, ceiling,
-			"the rejection for %s should name the ceiling",
-			retries)
-		assert.Empty(t,
-			targetsForWebhook(t, env.db, webhook.ID),
-			"no target should be created for %s", retries)
-	}
-}
-
-// TestTargetCreate_UnparseableRetriesRejected is the core of the
-// defect: each of these was accepted with HTTP 200 and stored as 0.
-func TestTargetCreate_UnparseableRetriesRejected(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	for _, retries := range unparseableRetries() {
-		webhook, code, body := createWithRetries(t, env, retries)
-
-		assert.Equal(t, http.StatusBadRequest, code,
-			"max_retries=%q should be rejected, not coerced",
-			retries)
-		assert.Contains(t, body, "whole number",
-			"the rejection for %q should say why", retries)
-		assert.Empty(t,
-			targetsForWebhook(t, env.db, webhook.ID),
-			"no target should be created for %q", retries)
-	}
-}
-
-// TestTargetCreate_ValidRetriesStored covers the accepting half,
-// including the ceiling itself: a bound that rejects its own limit
-// would make the advertised maximum unreachable.
-func TestTargetCreate_ValidRetriesStored(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	for _, want := range []int{0, 3, handlers.MaxTargetRetriesForTest} {
-		webhook, code, body := createWithRetries(
-			t, env, strconv.Itoa(want),
-		)
-
-		require.Equal(t, http.StatusSeeOther, code, body)
-
-		targets := targetsForWebhook(t, env.db, webhook.ID)
-		require.Len(t, targets, 1)
-		assert.Equal(t, want, targets[0].MaxRetries)
-	}
-}
-
-// TestTargetCreate_AbsentRetriesTakesDefault keeps the two cases
-// distinct. An omitted field is not an operator asking for something
-// invalid, so it still gets the fire-and-forget default rather than a
-// 400 — otherwise the fix above would make the form unusable.
-func TestTargetCreate_AbsentRetriesTakesDefault(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	for _, retries := range absentRetries() {
-		webhook, code, body := createWithRetries(t, env, retries)
-
-		require.Equal(t, http.StatusSeeOther, code, body)
-
-		targets := targetsForWebhook(t, env.db, webhook.ID)
-		require.Len(t, targets, 1)
-		assert.Equal(t, 0, targets[0].MaxRetries,
-			"an absent max_retries should take the default")
-	}
-}
-
-// seedRetriesTarget creates an HTTP target already delivering with
-// workingRetries retries, through the real create handler.
-func seedRetriesTarget(
-	t *testing.T,
-	env *sourceTestEnv,
-) (database.Webhook, database.Target) {
-	t.Helper()
-
-	webhook, code, body := createWithRetries(
-		t, env, strconv.Itoa(workingRetries),
-	)
-	require.Equal(t, http.StatusSeeOther, code, body)
-
-	targets := targetsForWebhook(t, env.db, webhook.ID)
-	require.Len(t, targets, 1)
-	require.Equal(t, workingRetries, targets[0].MaxRetries)
-
-	return webhook, targets[0]
-}
-
-// editRetriesForm is a complete edit submission that changes the
-// target's name as well, so a rejected submission can be shown to
-// have written nothing at all rather than merely to have left
-// max_retries alone.
-func editRetriesForm(retries string) url.Values {
-	form := url.Values{}
-	form.Set("name", "renamed-by-edit")
-	form.Set("url", retriesTargetURL)
-
-	if retries != absentField {
-		form.Set("max_retries", retries)
-	}
-
-	return form
-}
-
-// assertEditRejectedAndUnchanged submits an edit expected to fail and
-// checks both halves of the requirement: the 400 explains itself, and
-// the target it was submitted against is untouched.
-func assertEditRejectedAndUnchanged(
-	t *testing.T,
-	env *sourceTestEnv,
-	retries, wantReason string,
-) {
-	t.Helper()
-
-	webhook, target := seedRetriesTarget(t, env)
-
-	w := submitTargetEdit(
-		env, webhook.ID, target.ID, editRetriesForm(retries),
-	)
-
-	assert.Equal(t, http.StatusBadRequest, w.Code,
-		"max_retries=%q should be rejected on edit", retries)
-	assert.Contains(t, w.Body.String(), wantReason,
-		"the rejection for %q should say why", retries)
-
-	stored := storedTarget(t, env, target.ID)
-	assert.Equal(t, workingRetries, stored.MaxRetries,
-		"a rejected edit must not destroy the working retry "+
-			"count with %q", retries)
-	assert.Equal(t, "retries-target", stored.Name,
-		"a rejected edit must write nothing at all")
-}
-
-// TestTargetEdit_UnparseableRetriesRejected is the damaging half of
-// the defect. A target delivering with two retries, re-saved with a
-// typo in the field, returned 200 and was left with retries disabled
-// and nothing said.
-func TestTargetEdit_UnparseableRetriesRejected(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	for _, retries := range unparseableRetries() {
-		assertEditRejectedAndUnchanged(
-			t, env, retries, "whole number",
-		)
-	}
-}
-
-// TestTargetEdit_RetriesAboveCeilingRejected proves the ceiling
-// applies to the edit path too, naming itself, so the two paths
-// cannot disagree about what is storable.
-func TestTargetEdit_RetriesAboveCeilingRejected(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	ceiling := strconv.Itoa(handlers.MaxTargetRetriesForTest)
-
-	for _, retries := range overCeilingRetries() {
-		assertEditRejectedAndUnchanged(t, env, retries, ceiling)
-	}
-}
-
-// TestTargetEdit_ValidRetriesStored covers the accepting half of the
-// edit path, so the ceiling cannot be enforced by simply refusing
-// every submission.
-func TestTargetEdit_ValidRetriesStored(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	for _, want := range []int{0, 9, handlers.MaxTargetRetriesForTest} {
-		webhook, target := seedRetriesTarget(t, env)
-
-		w := submitTargetEdit(
-			env, webhook.ID, target.ID,
-			editRetriesForm(strconv.Itoa(want)),
-		)
-		require.Equal(t,
-			http.StatusSeeOther, w.Code, w.Body.String(),
-		)
-
-		assert.Equal(t, want,
-			storedTarget(t, env, target.ID).MaxRetries)
-	}
-}
-
-// TestTargetEdit_AbsentRetriesLeavesStoredValue is the edit path's
-// absent-versus-invalid case. Retries are only offered by the forms
-// for types that retry, so a submission without the field must leave
-// the stored count alone rather than be rejected or zeroed.
-func TestTargetEdit_AbsentRetriesLeavesStoredValue(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	for _, retries := range absentRetries() {
-		webhook, target := seedRetriesTarget(t, env)
-
-		w := submitTargetEdit(
-			env, webhook.ID, target.ID,
-			editRetriesForm(retries),
-		)
-		require.Equal(t,
-			http.StatusSeeOther, w.Code, w.Body.String(),
-		)
-
-		assert.Equal(t, workingRetries,
-			storedTarget(t, env, target.ID).MaxRetries,
-			"an absent max_retries must leave the stored "+
-				"count alone (%q)", retries)
-	}
-}
-
-// TestTargetRetries_CreateAndEditAgreeOnEveryCase proves the two
-// paths cannot disagree, which is what let the create form and the
-// edit form drift apart in the first place. Every input is submitted
-// to both and the accept/reject verdicts are compared.
-func TestTargetRetries_CreateAndEditAgreeOnEveryCase(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-
-	accepted := []string{
-		"0", "1",
-		strconv.Itoa(handlers.MaxTargetRetriesForTest),
-	}
-	overCeiling := overCeilingRetries()
-	unparseable := unparseableRetries()
-
-	cases := make(
-		[]string, 0,
-		len(accepted)+len(overCeiling)+len(unparseable),
-	)
-	cases = append(cases, accepted...)
-	cases = append(cases, overCeiling...)
-	cases = append(cases, unparseable...)
-
-	for _, retries := range cases {
-		_, createCode, _ := createWithRetries(t, env, retries)
-
-		webhook, target := seedRetriesTarget(t, env)
-		editCode := submitTargetEdit(
-			env, webhook.ID, target.ID,
-			editRetriesForm(retries),
-		).Code
-
-		assert.Equal(t,
-			createCode == http.StatusBadRequest,
-			editCode == http.StatusBadRequest,
-			"create and edit must agree on max_retries=%q "+
-				"(create %d, edit %d)",
-			retries, createCode, editCode,
-		)
-	}
-}

internal/handlers/target_toggle_test.go

@@ -59,7 +59,7 @@ func TestHandleTargetToggle_DoesNotUndoAnEdit(t *testing.T) {
 	stored := storedTarget(t, env, tgt.ID)
 	assert.False(t, stored.Active)
 	assert.Equal(t, "edited-name", stored.Name)
-	assert.Equal(t, 5, stored.MaxRetries)
+	assert.Equal(t, 5, stored.MaxAttempts)
 	assert.Equal(
 		t, editReplacedURL, storedHTTPConfig(t, env, tgt.ID).URL,
 	)

internal/handlers/ui_copy_test.go

@@ -348,21 +348,19 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
 	)
 }
 
-// maxRetriesHelp is the wording both target forms must carry. The
+// maxAttemptsHelp is the wording both target forms must carry. The
-// delivery core makes max_retries attempts in total, not that many
+// delivery core makes max_attempts attempts in total, not that many
 // retries on top of a first try (a fresh delivery starts at attempt 1
 // and target_http gives up once the attempt number reaches
-// max_retries), and 0 is special-cased to a single fire-and-forget
+// max_attempts).
-// attempt with no circuit breaker.
+const maxAttemptsHelp = "How many times each delivery is attempted in " +
-const maxRetriesHelp = "How many times each delivery is attempted in " +
+	"all, the first attempt included."
-	"all, the first attempt included. 0 means a single attempt with no " +
-	"retries and no circuit breaker."
 
-// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries
+// TestTargetFormMaxAttemptsCopyMatchesBehaviour pins the max_attempts
 // label, "Delivery attempts", and help text on both the create form
 // (the add-target form on the webhook detail page) and the edit form,
 // so the copy cannot drift back to calling the number a retry count.
-func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
+func TestTargetFormMaxAttemptsCopyMatchesBehaviour(t *testing.T) {
 	t.Parallel()
 
 	var h *handlers.Handlers
@@ -394,11 +392,11 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
 
 	assert.Contains(t, createBody, "Delivery attempts:</label>")
 	assert.Contains(
-		t, createBody, maxRetriesHelp,
+		t, createBody, maxAttemptsHelp,
-		"the add-target form must explain max_retries as total attempts",
+		"the add-target form must explain max_attempts as total attempts",
 	)
 
-	// A slack target exercises the same max_retries field while needing
+	// A slack target exercises the same max_attempts field while needing
 	// only a URL from the edit template, so the test data stays
 	// minimal. The Target and TargetForm keys mirror the field names
 	// the template reads off the handler's values.
@@ -412,8 +410,8 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
 				"Active": true,
 			},
 			"TargetForm": map[string]any{
-				"URL":        "https://hooks.slack.com/services/x",
+				"URL":         "https://hooks.slack.com/services/x",
-				"MaxRetries": "3",
+				"MaxAttempts": "3",
 			},
 			dataKeyError: "",
 		},
@@ -421,8 +419,8 @@ func TestTargetFormMaxRetriesCopyMatchesBehaviour(t *testing.T) {
 
 	assert.Contains(t, editBody, `class="label">Delivery attempts</label>`)
 	assert.Contains(
-		t, editBody, maxRetriesHelp,
+		t, editBody, maxAttemptsHelp,
-		"the target edit form must explain max_retries as total attempts",
+		"the target edit form must explain max_attempts as total attempts",
 	)
 }
 

internal/handlers/webhook.go

@@ -442,7 +442,7 @@ func buildDeliveryTasks(
 			TargetName:   targets[i].Name,
 			TargetType:   targets[i].Type,
 			TargetConfig: targets[i].Config,
-			MaxRetries:   targets[i].MaxRetries,
+			MaxAttempts:  targets[i].MaxAttempts,
 			Method:       event.Method,
 			RawQuery:     event.RawQuery,
 			Headers:      event.Headers,

internal/server/alpine_browser_test.go

@@ -354,11 +354,11 @@ func checkAddEachTargetType(ctx context.Context, t *testing.T, url string) {
 	}{
 		{
 			"http", "http",
-			"csrf_token name type url headers timeout max_retries",
+			"csrf_token name type url headers timeout max_attempts",
 			map[string]string{"url": publicTargetURL},
 		},
 		{
-			"slack", "slack", "csrf_token name type url max_retries",
+			"slack", "slack", "csrf_token name type url max_attempts",
 			map[string]string{"url": publicTargetURL},
 		},
 		{
@@ -630,11 +630,11 @@ func checkRefusedEdits(
 		values map[string]string
 	}{
 		{page + "/targets/" + targetID + "/edit", map[string]string{
-			"#name":        "edited-target",
+			"#name":         "edited-target",
-			"#url":         "http://127.0.0.1/hook",
+			"#url":          "http://127.0.0.1/hook",
-			"#headers":     "X-Edited: kept",
+			"#headers":      "X-Edited: kept",
-			"#timeout":     "12",
+			"#timeout":      "12",
-			"#max_retries": "3",
+			"#max_attempts": "3",
 		}},
 		{page + "/edit", map[string]string{
 			"#name":           "edited-webhook",

static/js/app.js

@@ -139,7 +139,7 @@ document.addEventListener("alpine:init", function () {
             headers: "",
             timeout: "",
             forwardQuery: false,
-            maxRetries: "",
+            maxAttempts: "",
             expiry: "",
             rotation: "",
             init() {
@@ -153,7 +153,7 @@ document.addEventListener("alpine:init", function () {
                 this.timeout = refused.timeout;
                 this.forwardQuery =
                     this.$root.hasAttribute("data-forward-query");
-                this.maxRetries = refused.maxRetries;
+                this.maxAttempts = refused.maxAttempts;
                 this.expiry = refused.expiry;
                 this.rotation = refused.rotation;
             },
@@ -173,7 +173,7 @@ document.addEventListener("alpine:init", function () {
                 this.headers = "";
                 this.timeout = "";
                 this.forwardQuery = false;
-                this.maxRetries = "";
+                this.maxAttempts = "";
                 this.expiry = "";
                 this.rotation = "";
                 this.$refs.form.reset();

templates/source_detail.html

@@ -136,7 +136,7 @@
              data-headers="{{.TargetForm.Headers}}"
              data-timeout="{{.TargetForm.Timeout}}"
              {{if .TargetForm.ForwardQuery}}data-forward-query{{end}}
-             data-max-retries="{{.TargetForm.MaxRetries}}"
+             data-max-attempts="{{.TargetForm.MaxAttempts}}"
              data-expiry="{{.TargetForm.Expiry}}"
              data-rotation="{{.TargetForm.Rotation}}">
             <div class="p-4 border-b border-gray-200 flex justify-between items-center">
@@ -194,9 +194,9 @@
                             <div>
                                 <div class="flex gap-2 items-center">
                                     <label class="text-sm text-gray-700">Delivery attempts:</label>
-                                    <input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
+                                    <input type="number" name="max_attempts" :value="maxAttempts" placeholder="1" min="1" max="20" class="input text-sm w-24">
                                 </div>
-                                <p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included. 0 means a single attempt with no retries and no circuit breaker.</p>
+                                <p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included.</p>
                             </div>
                         </div>
                     </template>
@@ -210,9 +210,9 @@
                             <div>
                                 <div class="flex gap-2 items-center">
                                     <label class="text-sm text-gray-700">Delivery attempts:</label>
-                                    <input type="number" name="max_retries" :value="maxRetries" placeholder="0" min="0" max="20" class="input text-sm w-24">
+                                    <input type="number" name="max_attempts" :value="maxAttempts" placeholder="1" min="1" max="20" class="input text-sm w-24">
                                 </div>
-                                <p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included. 0 means a single attempt with no retries and no circuit breaker.</p>
+                                <p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included.</p>
                             </div>
                         </div>
                     </template>

templates/target_edit.html

@@ -89,9 +89,9 @@
 
             {{if or (eq .Target.Type "http") (eq .Target.Type "slack")}}
             <div class="form-group">
-                <label for="max_retries" class="label">Delivery attempts</label>
+                <label for="max_attempts" class="label">Delivery attempts</label>
-                <input type="number" id="max_retries" name="max_retries" value="{{.TargetForm.MaxRetries}}" min="0" max="20" class="input">
+                <input type="number" id="max_attempts" name="max_attempts" value="{{.TargetForm.MaxAttempts}}" min="1" max="20" class="input">
-                <p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included. 0 means a single attempt with no retries and no circuit breaker.</p>
+                <p class="text-xs text-gray-500 mt-1">How many times each delivery is attempted in all, the first attempt included.</p>
             </div>
             {{end}}