Say max_retries is the total attempt count, not a retry count (closes #316)

The delivery core makes max_retries attempts in total: a fresh delivery
starts at attempt one and gives up once the attempt number reaches
max_retries, so 3 is three attempts, not four, and 0 is special-cased to
a single fire-and-forget attempt with no retries and no circuit breaker.
The create and edit target forms called it "Max retries" with no total,
and the README data-model row called it "maximum retry attempts", so an
operator wanting "try, then retry twice" would enter the wrong number.

Both forms and the README rows now state the number is the total number
of delivery attempts, with the 0 case spelled out. The delivery
arithmetic is unchanged. A UI copy test renders both forms and pins the
shared wording so it cannot drift back to a retry count.

Model: opus-4-8

README.md

@@ -92,8 +92,7 @@ them at once. A variable already present in the real environment wins
 over the file's value for the same name.
 
 The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev`
-or `prod` (default: `prod`; `dev` must be set explicitly). The setting
+or `prod` (default: `dev`). The setting controls exactly one behavior:
-controls exactly one behavior:
 
 | Behavior | `dev`                   | `prod`           |
 | -------- | ----------------------- | ---------------- |
@@ -135,7 +134,7 @@ TTY detection, and security headers are always applied.
 
 | Variable                | Description                         | Default  |
 | ----------------------- | ----------------------------------- | -------- |
-| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod`                     | `prod`   |
+| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod`                     | `dev`    |
 | `PORT`                  | HTTP listen port                    | `8080`   |
 | `BIND_ADDRESS`          | IP address the HTTP listener binds. Loopback by default, so the cleartext listener is not published on every interface. The Docker image ships `0.0.0.0` instead. See [Bind address](#bind-address) | `127.0.0.1` (image: `0.0.0.0`) |
 | `DATA_DIR`              | Directory for all SQLite databases  | `/var/lib/webhooker` |
@@ -401,9 +400,9 @@ the bucket is. See [Rate Limiting](#rate-limiting).
 The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
 address, which restores per-client buckets. webhooker logs a warning
 at startup whenever `TRUSTED_PROXIES` is empty, in every environment —
-the warning does not depend on `WEBHOOKER_ENVIRONMENT`, because an
+not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable
-operator who never configured the deployment is precisely the one at
+defaults to `dev` and an operator who never set it is precisely the
-risk. The warning is informational when nothing proxies to the
+one at risk. The warning is informational when nothing proxies to the
 process: with no proxy in front, the peer address is the client's own
 and the buckets are already per-client. See
 [Rate Limiting](#rate-limiting) for what each limit shares.
@@ -755,15 +754,15 @@ reports.
    that.
 2. **Set `WEBHOOKER_ENVIRONMENT=prod`, and make sure the proxy sends
    `X-Forwarded-Proto`.** These are two requirements, not one. The
-   environment setting decides CORS and nothing else: `dev` answers
+   environment setting decides CORS and nothing else: the default
-   every origin with `Access-Control-Allow-Origin: *` (without
+   `dev` answers every origin with `Access-Control-Allow-Origin: *`
-   credentials), which a server-rendered production deployment has no
+   (without credentials), which a server-rendered production
-   use for, and `prod` — the default — disables it. Cookie `Secure`
+   deployment has no use for. Cookie `Secure` and the strict
-   and the strict Origin/Referer mode are **not** tied to it — they
+   Origin/Referer mode are **not** tied to it — they are decided per
-   are decided per request from the transport, which behind a proxy
+   request from the transport, which behind a proxy means the
-   means the `X-Forwarded-Proto` header. The block below sets it;
+   `X-Forwarded-Proto` header. The block below sets it; without it
-   without it every request is read as plaintext and cookies ship
+   every request is read as plaintext and cookies ship without
-   without `Secure`. See [Configuration](#configuration).
+   `Secure`. See [Configuration](#configuration).
 3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate
    limiter keys on the connecting peer, which behind a proxy is the
    proxy on every request: all clients collapse into one global bucket
@@ -1508,7 +1507,7 @@ events should be forwarded.
 | `type`           | TargetType | One of: `http`, `slack`, `database`, `log` |
 | `active`         | boolean    | Whether deliveries are enabled (default: true) |
 | `config`         | JSON text  | Type-specific configuration |
-| `max_retries`    | integer    | Maximum retry attempts for `http` and `slack` targets (0 = fire-and-forget, >0 = retries with backoff and a circuit breaker). Ignored by `database` and `log` targets |
+| `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 |
 | `max_queue_size` | integer    | Stored and shown on the target's detail view, but not enforced anywhere yet: nothing in the delivery engine consults it. Queue depth is set by the two fixed 10,000-entry channels |
 
 **Relations:** Belongs to Webhook. Has many Deliveries.
@@ -1516,12 +1515,12 @@ events should be forwarded.
 **Target types:**
 
 - **`http`** — Forward the event as an HTTP POST to a configured URL.
-  Behavior depends on `max_retries`: when `max_retries` is 0 (the
+  `max_retries` is the total number of delivery attempts, not retries on
-  default), the target operates in fire-and-forget mode — a single
+  top of the first: when `max_retries` is 0 (the default), the target
-  attempt with no retries and no circuit breaker. When `max_retries` is
+  operates in fire-and-forget mode, a single attempt with no retries and
-  greater than 0, failed deliveries are retried with exponential backoff
+  no circuit breaker; a value of N makes up to N attempts in all,
-  up to `max_retries` attempts, protected by a per-target circuit
+  retrying failed deliveries with exponential backoff and protecting them
-  breaker.
+  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`

internal/config/config.go

@@ -585,14 +585,12 @@ func resolveMetricsAuth() (string, string, error) {
 	)
 }
 
-// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to prod
+// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
-// when it is unset so a deployment that forgets the variable is not
+// dev, and rejects unrecognised values.
-// silently permissive; dev must be set explicitly. It rejects
-// unrecognised values.
 func resolveEnvironment() (string, error) {
 	environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
 	if environment == "" {
-		environment = EnvironmentProd
+		environment = EnvironmentDev
 	}
 
 	if environment != EnvironmentDev &&
@@ -774,10 +772,10 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
 // everyone else's wrong passwords, and the receiver's limits become
 // service-wide ceilings.
 //
-// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT: an
+// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That
-// operator who never configured the deployment is exactly the case it
+// variable defaults to dev, so gating on it would silence the warning
-// exists to catch, so the exposure it announces is independent of the
+// for exactly the operator who forgot to configure the deployment —
-// environment setting.
+// the case it exists to catch.
 //
 // The default of trusting nobody is deliberate — trusting forwarded
 // headers from arbitrary peers lets any client choose its own bucket —

internal/config/config_test.go

@@ -44,9 +44,9 @@ func TestEnvironmentConfig(t *testing.T) {
 		isProd      bool
 	}{
 		{
-			name:   "default is prod",
+			name:   "default is dev",
-			isDev:  false,
+			isDev:  true,
-			isProd: true,
+			isProd: false,
 		},
 		{
 			name:     "explicit dev",
@@ -848,10 +848,10 @@ func TestEgressAllowlistWarning(t *testing.T) {
 // tells an operator a deployment behind a reverse proxy shares one
 // rate-limit bucket between every client, which turns the receiver
 // limits into service-wide ceilings and collapses login failure
-// counting. It must fire whenever TRUSTED_PROXIES is empty, in any
+// counting. It must fire whenever TRUSTED_PROXIES is empty,
-// environment: the warning does not depend on WEBHOOKER_ENVIRONMENT,
+// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating
-// since an operator who never configured the deployment is exactly the
+// on it would silence the warning for exactly the operator who never
-// one it exists to catch. It stays quiet once proxies are named.
+// configured the deployment. It stays quiet once proxies are named.
 func TestSharedRateLimitBucketWarning(t *testing.T) {
 	tests := []struct {
 		name           string
@@ -871,9 +871,10 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
 			expectWarning:  false,
 		},
 		{
-			// An internet-exposed deployment whose operator set
+			// The default environment. An internet-exposed
-			// WEBHOOKER_ENVIRONMENT=dev has exactly the exposure
+			// deployment whose operator never set
-			// the warning announces.
+			// WEBHOOKER_ENVIRONMENT lands here and has exactly
+			// the exposure the warning announces.
 			name:          "dev without trusted proxies warns",
 			environment:   config.EnvironmentDev,
 			expectWarning: true,

internal/database/database.go

@@ -17,12 +17,12 @@ import (
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/banner"
 	"sneak.berlin/go/webhooker/internal/config"
-	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/gormlog"
 	"sneak.berlin/go/webhooker/internal/logger"
 )
 
 const (
+	dataDirPerm       = 0750
 	randomPasswordLen = 16
 	sessionKeyLen     = 32
 )
@@ -185,9 +185,7 @@ func (d *Database) connect() error {
 // caller's decision.
 func (d *Database) connectTo(dataDir string) error {
 	// Ensure the data directory exists before opening the database.
-	// datadir.DirPerm is the single source of the directory mode; this
+	err := os.MkdirAll(dataDir, dataDirPerm)
-	// package creates the directory too, since either may run first.
-	err := os.MkdirAll(dataDir, datadir.DirPerm)
 	if err != nil {
 		return fmt.Errorf(
 			"creating data directory %s: %w",

internal/database/webhook_db_manager.go

@@ -13,7 +13,6 @@ import (
 	"gorm.io/driver/sqlite"
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/config"
-	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/gormlog"
 	"sneak.berlin/go/webhooker/internal/logger"
 )
@@ -54,9 +53,8 @@ func NewWebhookDBManager(
 		log:     params.Logger.Get(),
 	}
 
-	// Create data directory if it doesn't exist. datadir.DirPerm is the
+	// Create data directory if it doesn't exist
-	// single source of the directory mode; either package may run first.
+	err := os.MkdirAll(m.dataDir, dataDirPerm)
-	err := os.MkdirAll(m.dataDir, datadir.DirPerm)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"creating data directory %s: %w",

internal/datadir/lock.go

@@ -29,11 +29,9 @@ import (
 // process that was killed with SIGKILL blocks nothing.
 const LockFileName = "webhooker.lock"
 
-// DirPerm is the mode DATA_DIR is created with. It is the single
+// dirPerm is the mode Acquire creates DATA_DIR with. It matches what
-// source of that mode: internal/database consumes it rather than
+// internal/database uses, since whichever runs first creates it.
-// keeping its own copy, so the two packages that both create the
+const dirPerm = 0o750
-// directory cannot drift into disagreeing about its permissions.
-const DirPerm = 0o750
 
 // ErrLocked reports that another live process holds the data
 // directory. Callers that need to know whether a deployment is running
@@ -66,7 +64,7 @@ func Acquire(dir string) (*Lock, error) {
 		return nil, ErrNoDir
 	}
 
-	err := os.MkdirAll(dir, DirPerm)
+	err := os.MkdirAll(dir, dirPerm)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"creating data directory %s: %w", dir, err,

internal/handlers/ui_copy_test.go

@@ -300,3 +300,80 @@ func TestEntrypointCopyButtonIsProgressiveEnhancement(t *testing.T) {
 		"the page must render to completion, not abort partway",
 	)
 }
+
+// maxRetriesHelp is the wording both target forms must carry. The
+// delivery core makes max_retries 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
+// attempt with no circuit breaker.
+const maxRetriesHelp = "This is the total number of delivery attempts, " +
+	"not retries on top of the first: a value of 3 makes three attempts " +
+	"in all. 0 means a single attempt with no retries and no circuit " +
+	"breaker."
+
+// TestTargetFormMaxRetriesCopyMatchesBehaviour pins the max_retries
+// 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) {
+	t.Parallel()
+
+	var h *handlers.Handlers
+
+	var sess *session.Session
+
+	app := newTestApp(t, &h, &sess)
+	app.RequireStart()
+
+	t.Cleanup(app.RequireStop)
+
+	webhook := &database.Webhook{Name: "wh", RetentionDays: 14}
+	webhook.ID = testWebhookID
+
+	entrypoint := database.Entrypoint{Path: "abc123"}
+	entrypoint.ID = "ep-1"
+
+	createBody := renderPage(
+		t, h, sess, "source_detail.html", map[string]any{
+			dataKeyWebhook: webhook,
+			"Entrypoints": handlers.NewEntrypointViews(
+				[]database.Entrypoint{entrypoint},
+			),
+			"Targets": delivery.NewTargetViews(nil),
+			"Events":  []database.Event{},
+			"BaseURL": "https://hooks.example.com",
+		},
+	)
+
+	assert.Contains(
+		t, createBody, maxRetriesHelp,
+		"the add-target form must explain max_retries as total attempts",
+	)
+
+	// A slack target exercises the same max_retries field while needing
+	// only Config.URL from the edit template, so the test data stays
+	// minimal. The Target key mirrors the field names the template reads
+	// off the handler's view value.
+	editBody := renderPage(
+		t, h, sess, "target_edit.html", map[string]any{
+			dataKeyWebhook: webhook,
+			"Target": map[string]any{
+				"ID":         "tg-1",
+				"Name":       "t",
+				"Type":       "slack",
+				"Active":     true,
+				"MaxRetries": 3,
+				"Config": map[string]any{
+					"URL": "https://hooks.slack.com/services/x",
+				},
+			},
+			dataKeyError: "",
+		},
+	)
+
+	assert.Contains(
+		t, editBody, maxRetriesHelp,
+		"the target edit form must explain max_retries as total attempts",
+	)
+}

internal/middleware/csrf_test.go

@@ -380,8 +380,9 @@ func csrfTookStrictPath(
 
 // TestCSRF_ForwardedProtoSpellingsTakeStrictPath runs the header
 // spellings a real proxy emits through the middleware. The environment
-// is set to dev -- the permissive setting -- to pin that the routing is
+// is dev -- the DEFAULT when WEBHOOKER_ENVIRONMENT is unset -- to pin
-// a per-request transport decision and owes nothing to configuration.
+// that the routing is a per-request transport decision and owes
+// nothing to configuration.
 func TestCSRF_ForwardedProtoSpellingsTakeStrictPath(t *testing.T) {
 	t.Parallel()
 

templates/source_detail.html

@@ -120,9 +120,12 @@
                         <label class="text-sm text-gray-700">Timeout (seconds, blank = default):</label>
                         <input type="number" name="timeout" min="0" max="300" :disabled="targetType !== 'http'" class="input text-sm w-24">
                     </div>
-                    <div x-show="targetType === 'http'" class="flex gap-2 items-center">
+                    <div x-show="targetType === 'http'">
-                        <label class="text-sm text-gray-700">Max retries (0 = fire-and-forget):</label>
+                        <div class="flex gap-2 items-center">
-                        <input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
+                            <label class="text-sm text-gray-700">Max retries:</label>
+                            <input type="number" name="max_retries" value="0" min="0" max="20" class="input text-sm w-24">
+                        </div>
+                        <p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
                     </div>
                     <div x-show="targetType === 'slack'">
                         <input type="url" name="url" placeholder="https://hooks.slack.com/services/..." :disabled="targetType !== 'slack'" class="input text-sm">

templates/target_edit.html

@@ -69,7 +69,7 @@
             <div class="form-group">
                 <label for="max_retries" class="label">Max retries</label>
                 <input type="number" id="max_retries" name="max_retries" value="{{.Target.MaxRetries}}" min="0" max="20" class="input">
-                <p class="text-xs text-gray-500 mt-1">0 is fire-and-forget: one attempt, no circuit breaker.</p>
+                <p class="text-xs text-gray-500 mt-1">This is the total number of delivery attempts, not retries on top of the first: a value of 3 makes three attempts in all. 0 means a single attempt with no retries and no circuit breaker.</p>
             </div>
             {{end}}