From d2ecb839230d7053068aac450c5f2ff748071ef2 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Sat, 3 Oct 2026 02:13:17 +0200 Subject: [PATCH] Offer no Replay for a delivery to a deleted target (closes #387) In the event log, a delivery to a deleted target still offered Replay, and pressing it answered "Recreate the target, then replay", advice that cannot work: a recreated target is a new one, and the old delivery still names the deleted one. Such a delivery now has no Replay button, and its row still names the target marked "(deleted)". The refusal, which a page loaded before the delete can still reach, now tells the operator to use Resubmit to send the event to the webhook's currently active targets. Model: opus-5-5 --- README.md | 6 ++- internal/handlers/delivery_replay.go | 4 +- internal/handlers/delivery_replay_test.go | 6 ++- internal/handlers/notice.go | 3 +- .../source_logs_deleted_target_test.go | 38 +++++++++++++++++++ templates/source_logs.html | 2 +- 6 files changed, 54 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index f618fe5..05ac3fd 100644 --- a/README.md +++ b/README.md @@ -1856,7 +1856,11 @@ deliver where the destination has since been fixed. A target that has been deleted or deactivated therefore refuses the replay with a message on the event log rather than delivering from stale configuration, and a replay is refused while an earlier one for the -same event and target is still pending or retrying. +same event and target is still pending or retrying. A delivery whose +target has been deleted shows no **Replay** action at all: recreating +the target makes a new one that the old delivery does not name, so +**Resubmit** is how that event reaches the webhook's currently active +targets. **Resubmit.** Replay recovers one delivery; **resubmit** re-injects one EVENT. The event log offers a per-event **Resubmit** action that stores diff --git a/internal/handlers/delivery_replay.go b/internal/handlers/delivery_replay.go index 4ef278b..be4c6d6 100644 --- a/internal/handlers/delivery_replay.go +++ b/internal/handlers/delivery_replay.go @@ -21,7 +21,9 @@ const ( // replayTargetDeleted reports a target that once existed and has // since been deleted. Deletes are soft and deliveries carry no // foreign key to the target row, so the history survives its - // target and this is the ordinary case for an old event. + // target and this is the ordinary case for an old event. The + // event log shows no Replay button for such a delivery, so only + // a page loaded before the delete reaches this. replayTargetDeleted noticeCode = "replay-target-deleted" // replayTargetMissing reports a target id that names no row at diff --git a/internal/handlers/delivery_replay_test.go b/internal/handlers/delivery_replay_test.go index f155c17..332aa59 100644 --- a/internal/handlers/delivery_replay_test.go +++ b/internal/handlers/delivery_replay_test.go @@ -513,7 +513,11 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) { ) assert.Contains(t, refused, "alert-error") - assert.Contains(t, refused, "has been deleted") + assert.Contains( + t, refused, + "has been deleted. Use Resubmit to send the event "+ + "to the webhook", + ) // An outcome code nobody issued renders no banner at all. unknown := renderSourceLogsPageWithQuery( diff --git a/internal/handlers/notice.go b/internal/handlers/notice.go index 5a20d5c..69bb88e 100644 --- a/internal/handlers/notice.go +++ b/internal/handlers/notice.go @@ -66,7 +66,8 @@ func noticeFor(r *http.Request) *notice { }, replayTargetDeleted: { Text: "Not replayed: the target this delivery was for " + - "has been deleted. Recreate the target, then replay.", + "has been deleted. Use Resubmit to send the event " + + "to the webhook's currently active targets.", Failed: true, }, replayTargetMissing: { diff --git a/internal/handlers/source_logs_deleted_target_test.go b/internal/handlers/source_logs_deleted_target_test.go index 1f1f520..e38da68 100644 --- a/internal/handlers/source_logs_deleted_target_test.go +++ b/internal/handlers/source_logs_deleted_target_test.go @@ -94,6 +94,44 @@ func TestHandleSourceLogs_NamesDeletedTarget(t *testing.T) { ) } +// TestHandleSourceLogs_OffersNoReplayForDeletedTarget proves a +// finished delivery offers Replay while its target lives and not +// once the target is deleted. A replay to a deleted target is always +// refused, and recreating the target makes a new one that the old +// delivery does not name. +func TestHandleSourceLogs_OffersNoReplayForDeletedTarget(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := seedWebhook(t, db) + tgt := seedTarget(t, db, wh.ID, database.TargetTypeLog) + + _, failed := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID) + replayForm := `action="/hook/` + wh.ID + `/deliveries/` + + failed.ID + `/replay"` + + before := renderSourceLogsPage(t, h, sess, wh.ID) + assert.Contains(t, before, replayForm) + + deleteTargetThroughHandler(t, h, sess, wh.ID, tgt.ID) + + after := renderSourceLogsPage(t, h, sess, wh.ID) + assert.NotContains(t, after, replayForm) + assert.NotContains(t, after, ">Replay<") + assert.Contains(t, after, tgt.Name+deletedMarker) +} + // TestHandleSourceLogs_MasksDeletedTargetConfig proves that // naming a deleted target does not widen what the page shows of // it: its stored configuration stays masked by exactly the rules diff --git a/templates/source_logs.html b/templates/source_logs.html index edc0bd3..08ce136 100644 --- a/templates/source_logs.html +++ b/templates/source_logs.html @@ -74,7 +74,7 @@ - {{if .Status.Terminal}} + {{if and .Status.Terminal (not .Target.Deleted)}}