Store a webhook's retention as a plain day count, 0 meaning forever (closes #539)

retention_days loses its column default of 30, and Webhook.RetentionDays
its default tag, so a 0 is stored as 0. The 365000-day stand-in
RetentionForeverDays and the BeforeSave hook that wrote it are gone. The
reaper and the "forever" label read 0 as forever, and the forms accept 0
or 1 to MaxFiniteRetentionDays; a blank field still gives 30 on the new
webhook form and keeps the stored value on the edit form. A negative
value, which the forms refuse, is still read as forever so it cannot put
the reaper's cutoff in the future.

Model: opus-5-5

internal/database/model_webhook.go

@@ -4,27 +4,13 @@ import (
 	"math"
 	"strconv"
 	"time"
-
-	"gorm.io/gorm"
 )
 
 const (
-	// DefaultRetentionDays is the event retention period applied to a
-	// webhook created without an explicit retention value. It is the
-	// single source of truth for that policy and must stay in sync
-	// with the `gorm:"default:30"` column default on
-	// Webhook.RetentionDays below; a struct tag cannot reference a
-	// constant, so a test asserts the two agree.
+	// DefaultRetentionDays is the event retention period the new
+	// webhook form applies when its retention field is left blank.
 	DefaultRetentionDays = 30
 
-	// RetentionForeverDays is the sentinel RetentionDays value meaning
-	// "retain events forever". Users express that intent as 0, which
-	// Webhook.BeforeSave rewrites to this value: the column default
-	// substitutes DefaultRetentionDays for a zero value at insert
-	// time, so a zero can never survive a round trip to the database.
-	// Nothing outside this file may hardcode the number.
-	RetentionForeverDays = 365 * 1000
-
 	// MaxFiniteRetentionDays is the largest finite retention period the
 	// reaper's cutoff arithmetic can represent, and therefore the
 	// largest one a caller may request. It is derived from that
@@ -38,9 +24,7 @@ const (
 	// turns the cutoff into a timestamp in the far future that matches
 	// every row in the webhook's database. That is why this bound is
 	// enforced on input and why retentionCutoff saturates underneath
-	// it. Note that RetentionForeverDays deliberately sits above this
-	// ceiling: such webhooks are skipped before any cutoff is
-	// computed, and never reach the arithmetic at all.
+	// it.
 	MaxFiniteRetentionDays = int(
 		math.MaxInt64 / int64(time.Hour) / hoursPerDay,
 	)
@@ -48,12 +32,10 @@ const (
 
 // Webhook represents a webhook processing unit that groups entrypoints and targets
 //
-// Every method below takes a pointer receiver. BeforeSave has to,
-// because it mutates the record and GORM only invokes hooks declared
-// that way; the display helpers follow suit so the receiver kinds do
-// not mix. Handlers therefore put a *Webhook into template data:
-// html/template cannot call a pointer method on a value held in a map,
-// because a map element is not addressable.
+// Every method below takes a pointer receiver, so handlers put a
+// *Webhook into template data: html/template cannot call a pointer
+// method on a value held in a map, because a map element is not
+// addressable.
 type Webhook struct {
 	BaseModel
 
@@ -61,10 +43,9 @@ type Webhook struct {
 	Name        string `gorm:"not null"           json:"name"`
 	Description string `json:"description"`
 
-	// RetentionDays is the number of days to retain events. A value of
-	// RetentionForeverDays means retain forever. The column default
-	// must equal DefaultRetentionDays.
-	RetentionDays int `gorm:"default:30" json:"retentionDays"`
+	// RetentionDays is the number of days to retain events, and 0
+	// means retain them forever.
+	RetentionDays int `json:"retentionDays"`
 
 	// Relations. No model marshals the record it belongs to:
 	// User.Webhooks leads back here, and the JSON could loop.
@@ -73,36 +54,17 @@ type Webhook struct {
 	Targets     []Target     `json:"targets,omitempty"`
 }
 
-// BeforeSave normalises RetentionDays on every insert and update. A
-// non-positive value is the user's way of asking for "retain forever",
-// which is stored as the RetentionForeverDays sentinel.
-//
-// This has to happen in a hook rather than at the call sites. GORM
-// substitutes the column default (DefaultRetentionDays) for a zero
-// value while building the insert statement, which runs after
-// BeforeSave; rewriting any later than this loses that race and the
-// row lands at 30 days. Living on the model also means a future call
-// site, such as a REST API or a fixture, cannot bypass it.
-func (w *Webhook) BeforeSave(_ *gorm.DB) error {
-	if w.RetentionDays <= 0 {
-		w.RetentionDays = RetentionForeverDays
-	}
-
-	return nil
-}
-
-// retainsForever reports whether a stored RetentionDays value means
-// "keep events indefinitely". It is the single definition of that
-// question, shared by Webhook.RetainsForever and by the reaper's
+// retainsForever reports whether a RetentionDays value means "keep
+// events indefinitely", which 0 does. It is the single definition of
+// that question, shared by Webhook.RetainsForever and by the reaper's
 // cutoff computation so the two cannot disagree about which webhooks
 // are exempt from reaping.
 //
-// It accepts the RetentionForeverDays sentinel written by BeforeSave
-// and, defensively, any non-positive value, which BeforeSave treats
-// as a request to retain forever.
+// A negative value, which the forms refuse, is read the same way: as a
+// day count it would put the cutoff in the future, where it matches
+// every event the webhook has.
 func retainsForever(retentionDays int) bool {
-	return retentionDays <= 0 ||
-		retentionDays >= RetentionForeverDays
+	return retentionDays <= 0
 }
 
 // RetainsForever reports whether this webhook's events are kept
@@ -119,7 +81,8 @@ func (w *Webhook) RetentionCutoff(now time.Time) (time.Time, bool) {
 }
 
 // RetentionLabel returns the webhook's retention policy as display
-// text, so that no template has to know about the sentinel value.
+// text: "forever" rather than "0 days" for a webhook that keeps its
+// events.
 func (w *Webhook) RetentionLabel() string {
 	if w.RetainsForever() {
 		return "forever"

internal/database/model_webhook_test.go

@@ -2,8 +2,6 @@ package database_test
 
 import (
 	"context"
-	"reflect"
-	"strconv"
 	"testing"
 	"time"
 
@@ -47,8 +45,7 @@ func storedRetention(t *testing.T, db *gorm.DB, id string) int {
 }
 
 // newWebhookWithRetention creates a webhook through the ordinary Create
-// path, so the BeforeSave hook and the GORM column default both apply
-// exactly as they do in production.
+// path, as the new webhook form does.
 func newWebhookWithRetention(
 	t *testing.T,
 	db *gorm.DB,
@@ -67,7 +64,7 @@ func newWebhookWithRetention(
 	return wh.ID
 }
 
-func TestWebhookBeforeSave_ZeroBecomesForeverSentinel(t *testing.T) {
+func TestWebhookRetentionDays_ZeroIsStoredAsZero(t *testing.T) {
 	t.Parallel()
 
 	db := startedTestDB(t)
@@ -75,31 +72,10 @@ func TestWebhookBeforeSave_ZeroBecomesForeverSentinel(t *testing.T) {
 	wh := &database.Webhook{RetentionDays: 0}
 	id := newWebhookWithRetention(t, db, wh)
 
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetention(t, db, id),
-		"a zero retention must be stored as the sentinel, "+
-			"not replaced by the column default",
-	)
+	assert.Equal(t, 0, storedRetention(t, db, id))
 }
 
-func TestWebhookBeforeSave_NegativeBecomesForeverSentinel(t *testing.T) {
-	t.Parallel()
-
-	db := startedTestDB(t)
-
-	wh := &database.Webhook{RetentionDays: -5}
-	id := newWebhookWithRetention(t, db, wh)
-
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetention(t, db, id),
-	)
-}
-
-func TestWebhookBeforeSave_PositiveIsPreserved(t *testing.T) {
+func TestWebhookRetentionDays_DayCountIsStored(t *testing.T) {
 	t.Parallel()
 
 	db := startedTestDB(t)
@@ -110,10 +86,9 @@ func TestWebhookBeforeSave_PositiveIsPreserved(t *testing.T) {
 	assert.Equal(t, 7, storedRetention(t, db, id))
 }
 
-// TestWebhookBeforeSave_UpdateToZeroBecomesSentinel proves the hook
-// fires on update as well as insert, via the same Save call the edit
-// handler makes.
-func TestWebhookBeforeSave_UpdateToZeroBecomesSentinel(t *testing.T) {
+// TestWebhookRetentionDays_UpdateToZeroIsStoredAsZero changes a day
+// count to 0 through the same Save call the edit handler makes.
+func TestWebhookRetentionDays_UpdateToZeroIsStoredAsZero(t *testing.T) {
 	t.Parallel()
 
 	db := startedTestDB(t)
@@ -125,28 +100,7 @@ func TestWebhookBeforeSave_UpdateToZeroBecomesSentinel(t *testing.T) {
 	wh.RetentionDays = 0
 	require.NoError(t, db.Omit(clause.Associations).Save(wh).Error)
 
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetention(t, db, id),
-	)
-}
-
-// TestWebhookRetentionColumnDefaultMatchesConstant guards the one place
-// the default lives twice: a struct tag cannot reference a constant, so
-// this asserts the tag and DefaultRetentionDays agree.
-func TestWebhookRetentionColumnDefaultMatchesConstant(t *testing.T) {
-	t.Parallel()
-
-	field, ok := reflect.TypeFor[database.Webhook]().
-		FieldByName("RetentionDays")
-	require.True(t, ok, "Webhook.RetentionDays must exist")
-
-	assert.Equal(
-		t,
-		"default:"+strconv.Itoa(database.DefaultRetentionDays),
-		field.Tag.Get("gorm"),
-	)
+	assert.Equal(t, 0, storedRetention(t, db, id))
 }
 
 // TestMaxFiniteRetentionDaysIsTheOverflowCeiling asserts that the
@@ -176,14 +130,6 @@ func TestMaxFiniteRetentionDaysIsTheOverflowCeiling(t *testing.T) {
 		time.Duration(overCeiling*hoursPerDay)*time.Hour,
 		"one day past the ceiling must overflow",
 	)
-
-	assert.Less(
-		t,
-		database.MaxFiniteRetentionDays,
-		database.RetentionForeverDays,
-		"the sentinel sits above the ceiling and is only safe "+
-			"because retain-forever webhooks skip the arithmetic",
-	)
 }
 
 func TestWebhookRetainsForeverAndLabel(t *testing.T) {
@@ -195,16 +141,8 @@ func TestWebhookRetainsForeverAndLabel(t *testing.T) {
 		forever bool
 		label   string
 	}{
-		{
-			"sentinel",
-			database.RetentionForeverDays, true, testForeverLabel,
-		},
-		{
-			"above sentinel",
-			database.RetentionForeverDays + 1, true, testForeverLabel,
-		},
-		{"legacy zero", 0, true, testForeverLabel},
-		{"legacy negative", -1, true, testForeverLabel},
+		{"zero", 0, true, testForeverLabel},
+		{"negative", -1, true, testForeverLabel},
 		{"default", database.DefaultRetentionDays, false, "30 days"},
 		{"one day", 1, false, "1 day"},
 	}

internal/database/retention.go

@@ -243,7 +243,8 @@ func (r *RetentionReaper) reapWebhook(
 
 // retentionCutoff returns the timestamp before which a webhook's
 // events have expired, and whether any cutoff applies at all. It
-// reports false for a retain-forever policy, so no DELETE is issued.
+// reports false for a retention of 0, which means retain forever, so
+// no DELETE is issued.
 //
 // The day count is clamped to MaxFiniteRetentionDays first. This is
 // defense in depth rather than decoration: a time.Duration is an int64

internal/database/retention_test.go

@@ -85,41 +85,6 @@ func createWebhook(
 		db.Omit(clause.Associations).Create(wh).Error,
 	)
 
-	// Webhook.BeforeSave rewrites a non-positive RetentionDays to the
-	// retain-forever sentinel, and the column's GORM default would
-	// otherwise substitute 30. Force the requested value with a
-	// column-level update so tests can plant legacy rows that predate
-	// the sentinel and still carry a literal 0 or negative value.
-	require.NoError(
-		t,
-		db.Model(wh).
-			Update("retention_days", retentionDays).Error,
-	)
-
-	return wh.ID
-}
-
-// createWebhookNormally inserts a webhook through the ordinary Create
-// path, with no column-level forcing, so Webhook.BeforeSave applies
-// exactly as it does in production. Passing 0 therefore yields a row
-// holding the RetentionForeverDays sentinel.
-func createWebhookNormally(
-	t *testing.T,
-	db *gorm.DB,
-	retentionDays int,
-) string {
-	t.Helper()
-
-	wh := &database.Webhook{
-		UserID:        uuid.New().String(),
-		Name:          testWebhookName,
-		RetentionDays: retentionDays,
-	}
-	require.NoError(
-		t,
-		db.Omit(clause.Associations).Create(wh).Error,
-	)
-
 	return wh.ID
 }
 
@@ -281,19 +246,19 @@ func TestRetentionReaper_ReapsExpiredKeepsRecent(t *testing.T) {
 	assertChainPresent(t, db, recent)
 }
 
-// TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep covers the
-// end-to-end retain-forever path: a webhook created the normal way with
-// a requested retention of 0 lands on the RetentionForeverDays
-// sentinel, and the reaper leaves its ancient events alone while still
-// reaping a finite-retention webhook in the very same sweep.
-func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
+// TestRetentionReaper_SkipsZeroReapsFiniteInSameSweep covers the
+// end-to-end retain-forever path: a webhook created with a retention
+// of 0 reads back 0, and the reaper leaves its ancient events alone
+// while still reaping a finite-retention webhook in the very same
+// sweep.
+func TestRetentionReaper_SkipsZeroReapsFiniteInSameSweep(
 	t *testing.T,
 ) {
 	t.Parallel()
 
 	env := setupRetentionTest(t)
 
-	foreverID := createWebhookNormally(t, env.mainDB.DB(), 0)
+	foreverID := createWebhook(t, env.mainDB.DB(), 0)
 
 	var stored database.Webhook
 
@@ -302,14 +267,12 @@ func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
 		env.mainDB.DB().Where("id = ?", foreverID).
 			First(&stored).Error,
 	)
-	require.Equal(
-		t,
-		database.RetentionForeverDays,
-		stored.RetentionDays,
-		"a requested retention of 0 must persist as the sentinel",
+	require.Zero(
+		t, stored.RetentionDays,
+		"a retention of 0 must be stored as 0",
 	)
 
-	finiteID := createWebhookNormally(t, env.mainDB.DB(), 30)
+	finiteID := createWebhook(t, env.mainDB.DB(), 30)
 
 	foreverDB, err := env.mgr.GetDB(foreverID)
 	require.NoError(t, err)
@@ -335,14 +298,10 @@ func TestRetentionReaper_SkipsSentinelReapsFiniteInSameSweep(
 // negative, so subtracting it moves the cutoff into the far future,
 // where "created_at < cutoff" matches every row: an event created a
 // moment ago, and its delivery and delivery result, were all deleted
-// on the first sweep. 200000 is inside that band and below the
-// retain-forever sentinel, so it is treated as a finite policy and
-// really does reach the arithmetic.
+// on the first sweep. 200000 is inside that band.
 //
-// The row is planted at the column level because such a value can no
-// longer be submitted through the form; the point of the test is that
-// a row from an older version, or a future call site, still cannot
-// trigger the wipe.
+// The form refuses such a value; the point of the test is that a row
+// written by any other path still cannot trigger the wipe.
 func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
 	t *testing.T,
 ) {
@@ -358,12 +317,6 @@ func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
 		database.MaxFiniteRetentionDays,
 		"the test value must exceed what the cutoff can represent",
 	)
-	require.Less(
-		t,
-		overflowingRetentionDays,
-		database.RetentionForeverDays,
-		"the test value must not be treated as retain-forever",
-	)
 
 	webhookID := createWebhook(
 		t, env.mainDB.DB(), overflowingRetentionDays,
@@ -379,14 +332,15 @@ func TestRetentionReaper_HugeFiniteRetentionRetainsRecentEvents(
 	assertChainPresent(t, db, fresh)
 }
 
-func TestRetentionReaper_RetainsForeverWhenNonPositive(t *testing.T) {
+// TestRetentionReaper_RetainsForeverWhenNegative checks that a negative
+// retention, which the forms refuse, keeps every event rather than
+// putting the cutoff in the future, where it would match them all.
+func TestRetentionReaper_RetainsForeverWhenNegative(t *testing.T) {
 	t.Parallel()
 
 	env := setupRetentionTest(t)
 
-	// A legacy row written before the sentinel existed still carries a
-	// literal 0; the <= 0 guard must keep honouring it.
-	webhookID := createWebhook(t, env.mainDB.DB(), 0)
+	webhookID := createWebhook(t, env.mainDB.DB(), -1)
 
 	db, err := env.mgr.GetDB(webhookID)
 	require.NoError(t, err)

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

@@ -46,7 +46,7 @@ CREATE TABLE webhooks (
     user_id uuid NOT NULL,
     name text NOT NULL,
     description text,
-    retention_days integer DEFAULT 30,
+    retention_days integer,
     PRIMARY KEY (id),
     CONSTRAINT fk_users_webhooks FOREIGN KEY (user_id) REFERENCES users (id)
 );

internal/handlers/shared.go

@@ -16,21 +16,16 @@ import (
 //
 // An empty value yields fallback, which lets the create path apply the
 // default and the edit path leave the stored value unchanged. A value
-// of 0 is returned as 0 and is rewritten to the retain-forever
-// sentinel by database.Webhook's BeforeSave hook. Anything unparseable
-// or negative is refused rather than silently given a default.
+// of 0 means retain events forever and is returned as it is. Anything
+// unparseable or negative is refused rather than silently given a
+// default.
 //
 // The upper bound is not cosmetic. The reaper computes its cutoff as a
 // time.Duration, an int64 nanosecond count, so a day count above
 // database.MaxFiniteRetentionDays overflows, puts the cutoff in the
-// future, and deletes every event the webhook has. A finite value
-// above that ceiling is therefore refused, and the message names the
-// ceiling rather than implying the input was not a number.
-//
-// A value at or above the retain-forever sentinel is not out of range:
-// it is what the edit form pre-fills for a retain-forever webhook, so
-// submitting the form back unchanged has to keep meaning "forever"
-// rather than being rejected.
+// future, and deletes every event the webhook has. A value above that
+// ceiling is therefore refused, and the message names the ceiling
+// rather than implying the input was not a number.
 func parseRetentionDays(raw string, fallback int) (int, string) {
 	raw = strings.TrimSpace(raw)
 	if raw == "" {
@@ -43,10 +38,6 @@ func parseRetentionDays(raw string, fallback int) (int, string) {
 			"retain events forever."
 	}
 
-	if v >= database.RetentionForeverDays {
-		return database.RetentionForeverDays, ""
-	}
-
 	if v > database.MaxFiniteRetentionDays {
 		return 0, "Retention must be at most " +
 			strconv.Itoa(database.MaxFiniteRetentionDays) +

internal/handlers/source_management_test.go

@@ -137,8 +137,7 @@ func onlyWebhook(
 }
 
 // seedWebhookWithRetention inserts a webhook owned by the test user
-// with an exact stored retention value, bypassing Webhook.BeforeSave
-// via a column-level update so that legacy rows can be planted too.
+// with the given retention.
 func seedWebhookWithRetention(
 	t *testing.T,
 	db *database.Database,
@@ -155,13 +154,6 @@ func seedWebhookWithRetention(
 		t,
 		db.DB().Omit(clause.Associations).Create(wh).Error,
 	)
-	require.NoError(
-		t,
-		db.DB().Model(wh).
-			Update("retention_days", retentionDays).Error,
-	)
-
-	wh.RetentionDays = retentionDays
 
 	return *wh
 }
@@ -225,10 +217,9 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
 	}
 }
 
-// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever is the core
-// regression test for the bug: the create form's 0 must reach the
-// database as the retain-forever sentinel rather than being replaced by
-// the column's default of 30.
+// TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever checks
+// that the create form's 0 reaches the database as 0, which means
+// retain forever.
 func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
 	t *testing.T,
 ) {
@@ -241,30 +232,55 @@ func TestHandleSourceCreateSubmit_ZeroRetentionPersistsForever(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 
 	wh := onlyWebhook(t, env.db)
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+	assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
 	assert.True(t, wh.RetainsForever())
 }
 
-func TestHandleSourceCreateSubmit_OmittedRetentionUsesDefault(
+func TestHandleSourceCreateSubmit_DayCountIsStored(t *testing.T) {
+	t.Parallel()
+
+	env := setupSourceTest(t)
+	days := "45"
+
+	w := submitCreate(t, env.handlers, env.cookies, "finite", &days)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+
+	wh := onlyWebhook(t, env.db)
+	assert.Equal(t, 45, storedRetentionDays(t, env.db, wh.ID))
+}
+
+// TestHandleSourceCreateSubmit_BlankRetentionUsesDefault covers a
+// retention field left empty and one missing from the form: both
+// give the default, not forever.
+func TestHandleSourceCreateSubmit_BlankRetentionUsesDefault(
 	t *testing.T,
 ) {
 	t.Parallel()
 
-	env := setupSourceTest(t)
+	blank := ""
 
-	w := submitCreate(t, env.handlers, env.cookies, "defaulted", nil)
-	require.Equal(t, http.StatusSeeOther, w.Code)
+	for name, retention := range map[string]*string{
+		"blank":   &blank,
+		"omitted": nil,
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
 
-	wh := onlyWebhook(t, env.db)
-	assert.Equal(
-		t,
-		database.DefaultRetentionDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+			env := setupSourceTest(t)
+
+			w := submitCreate(
+				t, env.handlers, env.cookies, "defaulted", retention,
+			)
+			require.Equal(t, http.StatusSeeOther, w.Code)
+
+			wh := onlyWebhook(t, env.db)
+			assert.Equal(
+				t,
+				database.DefaultRetentionDays,
+				storedRetentionDays(t, env.db, wh.ID),
+			)
+		})
+	}
 }
 
 // TestHandleSourceCreate_PrefillsDefaultFromConstant keeps the create
@@ -290,7 +306,7 @@ func TestHandleSourceCreate_PrefillsDefaultFromConstant(t *testing.T) {
 	)
 	assert.NotContains(
 		t, body, `max="365"`,
-		"a max below the sentinel would block retain-forever",
+		"a max of 365 would block any longer retention",
 	)
 	assert.Contains(t, body, `min="0"`)
 }
@@ -368,40 +384,6 @@ func TestHandleSourceCreateSubmit_OverflowingRetentionIsRejected(
 	)
 }
 
-// TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever guards the
-// boundary between "too large to represent" and "retain forever": the
-// sentinel is above MaxFiniteRetentionDays, but it is the value the
-// edit form pre-fills, so it must be accepted rather than rejected as
-// out of range. A value above the sentinel is stored as the sentinel.
-func TestHandleSourceCreateSubmit_SentinelIsAcceptedAsForever(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	for _, days := range []int{
-		database.RetentionForeverDays,
-		database.RetentionForeverDays + 1,
-	} {
-		raw := strconv.Itoa(days)
-
-		t.Run(raw, func(t *testing.T) {
-			t.Parallel()
-
-			env := setupSourceTest(t)
-
-			w := submitCreate(t, env.handlers, env.cookies, "forever", &raw)
-			require.Equal(t, http.StatusSeeOther, w.Code)
-
-			wh := onlyWebhook(t, env.db)
-			assert.Equal(
-				t,
-				database.RetentionForeverDays,
-				storedRetentionDays(t, env.db, wh.ID),
-			)
-		})
-	}
-}
-
 // TestHandleSourceCreateSubmit_RejectedFormKeepsUserInput checks that a
 // validation failure hands the user's typing back, matching what the
 // edit form already does. Losing a long description to a mistyped
@@ -480,11 +462,7 @@ func TestHandleSourceEditSubmit_ZeroRetentionPersistsForever(
 	w := submitEdit(t, env, wh, "0")
 	require.Equal(t, http.StatusSeeOther, w.Code)
 
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+	assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
 }
 
 func TestHandleSourceEditSubmit_InvalidRetentionIsRejected(
@@ -867,18 +845,15 @@ func TestHandleSourceEditSubmit_ArchiveNameTaken(t *testing.T) {
 	assert.Equal(t, oldName, stored.Name)
 }
 
-// TestSourceEditForm_ForeverWebhookRoundTrips walks the exact path that
-// the removed max="365" cap used to break: render the edit form for a
-// retain-forever webhook, confirm the pre-filled sentinel is not capped
-// by browser validation, then submit that pre-filled value straight
-// back and confirm the retention policy survives untouched.
+// TestSourceEditForm_ForeverWebhookRoundTrips renders the edit form for
+// a retain-forever webhook, confirms it pre-fills 0, then submits that
+// pre-filled value straight back and confirms the retention policy
+// survives untouched.
 func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
 	t.Parallel()
 
 	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(
-		t, env.db, database.RetentionForeverDays,
-	)
+	wh := seedWebhookWithRetention(t, env.db, 0)
 
 	req := getRequest(
 		t, "/hook/"+wh.ID+"/edit", env.cookies,
@@ -889,17 +864,12 @@ func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
 
 	require.Equal(t, http.StatusOK, w.Code)
 
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
 	body := w.Body.String()
 
 	assert.Contains(
-		t, body, `value="`+sentinel+`"`,
+		t, body, `name="retention_days" value="0"`,
 		"the edit form pre-fills the stored retention",
 	)
-	assert.NotContains(
-		t, body, `max="365"`,
-		"a max below the sentinel would block saving any edit",
-	)
 	// "Currently forever." is the rendered RetentionLabel, not the
 	// static hint below the input, which says "Enter 0 to retain events
 	// forever." A bare Contains of "forever" would pass for any
@@ -910,29 +880,20 @@ func TestSourceEditForm_ForeverWebhookRoundTrips(t *testing.T) {
 	)
 
 	// Submit the pre-filled value back, exactly as a browser would.
-	post := submitEdit(t, env, wh, sentinel)
+	post := submitEdit(t, env, wh, "0")
 	require.Equal(t, http.StatusSeeOther, post.Code)
 
-	assert.Equal(
-		t,
-		database.RetentionForeverDays,
-		storedRetentionDays(t, env.db, wh.ID),
-	)
+	assert.Equal(t, 0, storedRetentionDays(t, env.db, wh.ID))
 }
 
-// TestSourceListAndDetail_ShowForeverNotTheSentinelNumber checks that
-// the retain-forever value is never rendered to the user as a raw day
-// count on either read-only view.
-func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
-	t *testing.T,
-) {
+// TestSourceListAndDetail_ShowForeverNotZeroDays checks that a
+// retention of 0 is shown as forever, never as a day count, on either
+// read-only view.
+func TestSourceListAndDetail_ShowForeverNotZeroDays(t *testing.T) {
 	t.Parallel()
 
 	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(
-		t, env.db, database.RetentionForeverDays,
-	)
-	sentinel := strconv.Itoa(database.RetentionForeverDays)
+	wh := seedWebhookWithRetention(t, env.db, 0)
 
 	listW := httptest.NewRecorder()
 	env.handlers.HandleSourceList().ServeHTTP(
@@ -941,7 +902,7 @@ func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
 
 	require.Equal(t, http.StatusOK, listW.Code)
 	assert.Contains(t, listW.Body.String(), "Retention: forever")
-	assert.NotContains(t, listW.Body.String(), sentinel)
+	assert.NotContains(t, listW.Body.String(), "Retention: 0 days")
 
 	detailW := httptest.NewRecorder()
 	env.handlers.HandleSourceDetail().ServeHTTP(
@@ -954,5 +915,5 @@ func TestSourceListAndDetail_ShowForeverNotTheSentinelNumber(
 
 	require.Equal(t, http.StatusOK, detailW.Code)
 	assert.Contains(t, detailW.Body.String(), "Retention: forever")
-	assert.NotContains(t, detailW.Body.String(), sentinel)
+	assert.NotContains(t, detailW.Body.String(), "Retention: 0 days")
 }

internal/handlers/ui_copy_test.go

@@ -184,8 +184,7 @@ func TestEventLogPageIsCalledFullEventLog(t *testing.T) {
 // TestCreateFormRetentionCopyMatchesBehaviour pins the create form's
 // retention copy to what the code does: the reaper permanently deletes
 // events past the cutoff, an empty field falls back to
-// DefaultRetentionDays, and 0 is rewritten to the retain-forever
-// sentinel by Webhook.BeforeSave.
+// DefaultRetentionDays, and 0 means retain forever.
 func TestCreateFormRetentionCopyMatchesBehaviour(t *testing.T) {
 	t.Parallel()
 
@@ -253,10 +252,7 @@ func TestEditFormRetentionCopyMatchesBehaviour(t *testing.T) {
 		"blank means unchanged, not forever",
 	)
 
-	forever := &database.Webhook{
-		Name:          "wh",
-		RetentionDays: database.RetentionForeverDays,
-	}
+	forever := &database.Webhook{Name: "wh", RetentionDays: 0}
 	forever.ID = "wh-2"
 
 	foreverBody := renderPage(

internal/handlers/webhook_stats_test.go

@@ -500,7 +500,7 @@ func TestWebhookStats_PaneShowsRetentionPeriod(t *testing.T) {
 		want          string
 	}{
 		{30, "30 days"},
-		{database.RetentionForeverDays, "forever"},
+		{0, "forever"},
 	}
 
 	for _, tt := range tests {