feat(webhook): honor retry_after as a pending not-before time

[Kiran01bm]

Pending inbox rows now honor a caller-set retry_after as a not-before time: the delivery is durable immediately but invisible to dispatch — and not counted as backlog — until the time passes.

Why

An upcoming redundant convergence trigger (check_suite.requested) must be enqueued with a grace delay so the organic pull_request delivery normally wins the race — the redundant signal should only be processed if the primary delivery never arrived. That requires a deferred-dispatch primitive in the inbox itself, so the deferral is as durable as the row. The retry_after column already exists (used by retryable rows); this extends its meaning to pending rows.

What

• Create persists WebhookEvent.RetryAfter (previously ignored on insert).
• The shared claimable predicate now requires retry_after IS NULL OR retry_after <= now for pending rows, matching the existing retryable-row condition. Because FindNext and InboxStats derive from the same predicate, a deferred row is not counted as claimable backlog.
• Both time-basis metrics measure from when the row became dispatchable, not from receipt, so a deferral's grace period never reads as delay: the dispatch-lag histogram uses ClaimableSince (derived by FindNext from the consumed retry_after), and the oldest-claimable-age gauge measures from GREATEST(received_at, retry_after).
• Nil RetryAfter is byte-for-byte the previous behavior, so all existing producers are unaffected. A terminal-redelivery reopen still clears retry_after — and discards a not-before time on the incoming duplicate — keeping GitHub's "Redeliver" button an immediate recovery lever.
• No schema change: retry_after already exists.

Before / after for a pending row created with a future retry_after:

before:
  Create ──▶ pending ──────────────▶ claimable by FindNext immediately
                                      (RetryAfter silently dropped)

after:
  Create ──▶ pending (deferred) ─┬─▶ invisible to FindNext + backlog gauge
                                 │        while retry_after > now
                                 └─▶ claimable once retry_after <= now
                                      (claim consumes retry_after;
                                       lag + age measured from due time)

[Copilot]

Pull request overview

This PR extends the webhook inbox semantics so a caller-provided WebhookEvent.RetryAfter is durably stored on pending rows and treated as a not-before time, keeping the row invisible to dispatch (and the claimable backlog metric) until the timestamp is reached.

Changes:

• Persist WebhookEvent.RetryAfter on insert for pending webhook deliveries.
• Update the shared “claimable” predicate so pending rows are only claimable when retry_after is unset or has elapsed (aligning pending with retryable behavior).
• Add an integration test verifying deferred pending rows are not claimable (and not counted as claimable backlog) until the not-before time passes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
pkg/storage/storage.go	Updates interface docs to describe RetryAfter as a not-before time for pending deliveries.
pkg/storage/internal/sqlstore/webhook_events.go	Persists retry_after on insert and gates pending claimability on (retry_after IS NULL OR retry_after <= now).
pkg/storage/internal/sqlstore/webhook_events_test.go	Adds coverage for deferred pending events (dispatch invisibility + InboxStats alignment).
pkg/storage/internal/sqlstore/sql_helpers.go	Adds nullTimePtr helper to bind nullable *time.Time values in SQL statements.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[morgo]

🤖 Approving on Morgan's behalf (automated review, escalation rules apply).

Verified the mechanics: Create's column/placeholder/arg lists all line up at 13 with retry_after in position; the pending-arm not-before check lives in the shared claimable predicate, so FindNext and the backlog gauge cannot drift; the claim consumes the persisted retry_after (integration test asserts the row, not just the returned struct); and MarkFailed's nullTimePtr wrap is behavior-equivalent. Effective risk is low because nothing in production sets RetryAfter on pending rows yet — the producer (#1002) is still draft — so the only live change is the metrics basis (age/lag measured from due time), which is intentional, documented, and covered by both integration and metrics tests.

Two non-blocking notes:

• This edits the same InboxStats query that #1007 rewrote for dialect portability (TIMESTAMPDIFF → Go-side subtraction). Whichever merges second needs a small semantic rebase, and the merged expression should keep #1007's portable form — GREATEST/COALESCE port fine, TIMESTAMPDIFF doesn't.
• The FIFO caveat (a deferred row re-enters at its original insertion position, ahead of rows created during its deferral) is documented honestly; fine for second-scale grace delays.

[Kiran01bm]

Review response from Kiran's (@Kiran01bm) AI code review assessment agent (Amp / Claude Opus 4.5)

Summary: Both non-blocking notes are resolved at head 3a4b3c1f — the #1007 semantic-rebase note is fixed on the branch, and the FIFO caveat needs no code change; the review's verification section raised no findings.

#	Finding	Status	Explanation
1	InboxStats edits collide with #1007's dialect-portable rewrite; whichever merges second needs a semantic rebase keeping the portable form	fixed	Branch rebased onto main past #1007; the merged oldest-unprocessed expression is MIN(GREATEST(received_at, COALESCE(retry_after, received_at))) with the dialect's CurrentTimestamp() — no TIMESTAMPDIFF remains. 3a4b3c1f adapts the retry_after tests to #1007's NewMySQL/NewPostgres constructor split.
2	FIFO caveat: a deferred row re-enters at its original insertion position, ahead of rows created during its deferral	rejected	Agreed with the review's own conclusion — the behavior is documented and acceptable for second-scale grace delays; no change made.