Give reminder cancellation its own store and IPC path (V-719)

CancelReminder replaces the cancelled half of MarkReminder, which stays
delivery-only. Cancellation has to win against the start of an external
send, so it refuses when the occurrence has a pending, sent or unknown
outbox row, and clears the delivery group inside the same transaction.
BeginDeliveryAttempt takes the mirror lock for reminder sends, so no
interleaving lets both operations report success.

Cancelling one member of a collapsed catch-up bundle invalidates the
cached phrase on every pending sibling; a later retry would otherwise keep
saying "three reminders" after one was removed.

Legacy rows carry the empty delivery group from migration 25, so they only
count as this occurrence when they began at or after its next-fire
boundary. Without that bound one old success would make a recurring series
permanently uncancellable.

ListPendingReminders returns cancellable rows in firing order, with no
limit by default, because spoken resolution must not miss an old reminder
that newer fired history pushed out of ListReminders' window.

Cancellation is ordinary authenticated write authority: it prevents a
future send and cannot create one. cmd/e2eprobe drives both from outside.

--no-verify: master is the working branch this session by the owner's call.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-15 17:19:13 +04:00
parent 5b0b29dfad
commit 0b057df2a3
17 changed files with 846 additions and 38 deletions
+56
View File
@@ -2,6 +2,7 @@ package store
import (
"context"
"database/sql"
"fmt"
"log"
"time"
@@ -34,6 +35,9 @@ const (
// stored for post-crash operator triage, not enforced as a uniqueness
// constraint (a rule/reminder legitimately re-sends across ticks).
func (s *Store) BeginDeliveryAttempt(ctx context.Context, kind, rule string, reminderID int64, deliveryGroup, channel, bodyHash string, now time.Time) (int64, error) {
if kind == "reminder" && deliveryGroup != "" {
return s.beginReminderDeliveryAttempt(ctx, rule, reminderID, deliveryGroup, channel, bodyHash, now)
}
res, err := s.db.ExecContext(ctx,
`INSERT INTO delivery_attempts (kind, rule, reminder_id, delivery_group, channel, body_hash, status, created_ts)
VALUES (?, ?, ?, ?, ?, ?, 'pending', ?)`,
@@ -48,6 +52,58 @@ func (s *Store) BeginDeliveryAttempt(ctx context.Context, kind, rule string, rem
return id, nil
}
// beginReminderDeliveryAttempt serializes the last local cancellation point
// with the first externally ambiguous delivery point. CancelReminder clears a
// group's identity when it wins first; when this insert wins first it leaves a
// pending attempt that makes cancellation refuse. There is therefore no state
// in which both operations report success.
func (s *Store) beginReminderDeliveryAttempt(ctx context.Context, rule string, reminderID int64, deliveryGroup, channel, bodyHash string, now time.Time) (int64, error) {
tx, err := s.db.BeginTx(ctx, nil)
if err != nil {
return 0, fmt.Errorf("begin reminder delivery attempt: begin: %w", err)
}
defer tx.Rollback()
var status, currentGroup string
err = tx.QueryRowContext(ctx,
`SELECT status, delivery_group FROM reminders WHERE id = ?`, reminderID,
).Scan(&status, &currentGroup)
if err != nil {
if err == sql.ErrNoRows {
return 0, ErrReminderNotFound
}
return 0, fmt.Errorf("begin reminder delivery attempt: read reminder %d: %w", reminderID, err)
}
if status != ReminderPending || currentGroup != deliveryGroup {
return 0, fmt.Errorf("%w: reminder %d no longer owns delivery group", ErrReminderState, reminderID)
}
var live int
if err := tx.QueryRowContext(ctx, `
SELECT COUNT(*) FROM delivery_attempts
WHERE kind = 'reminder' AND delivery_group = ?
AND status IN ('pending', 'sent', 'unknown')`, deliveryGroup).Scan(&live); err != nil {
return 0, fmt.Errorf("begin reminder delivery attempt: inspect group: %w", err)
}
if live > 0 {
return 0, ErrReminderInFlight
}
res, err := tx.ExecContext(ctx, `
INSERT INTO delivery_attempts (kind, rule, reminder_id, delivery_group, channel, body_hash, status, created_ts)
VALUES ('reminder', ?, ?, ?, ?, ?, 'pending', ?)`,
rule, reminderID, deliveryGroup, channel, bodyHash, now.UnixMilli())
if err != nil {
return 0, fmt.Errorf("begin reminder delivery attempt: %w", err)
}
id, err := res.LastInsertId()
if err != nil {
return 0, fmt.Errorf("begin reminder delivery attempt: last insert id: %w", err)
}
if err := tx.Commit(); err != nil {
return 0, fmt.Errorf("begin reminder delivery attempt: commit: %w", err)
}
return id, nil
}
// CompleteDeliveryAttempt records the sink's outcome for a prior
// BeginDeliveryAttempt. status is "sent", "failed" or "dropped" — never
// "pending" or "unknown" (those are set only by Begin and reconciliation
+13 -2
View File
@@ -54,7 +54,18 @@ func TestListDeliveryAttempts(t *testing.T) {
if err := s.CompleteDeliveryAttempt(ctx, dropped, DeliveryDropped, base.Add(time.Minute)); err != nil {
t.Fatal(err)
}
if _, err := s.BeginDeliveryAttempt(ctx, "reminder", "", 7, "reminder:test", "voice", "h3", base.Add(2*time.Minute)); err != nil {
reminderID, err := s.CreateReminder(ctx, base.Add(time.Hour), `{"text":"test"}`, "")
if err != nil {
t.Fatal(err)
}
reminders, err := s.ListReminders(ctx, 1)
if err != nil || len(reminders) != 1 {
t.Fatalf("list reminder: %+v, %v", reminders, err)
}
if err := s.CacheReminderPhrase(ctx, reminders, "reminder:test", "test", "test", "neutral"); err != nil {
t.Fatal(err)
}
if _, err := s.BeginDeliveryAttempt(ctx, "reminder", "", reminderID, "reminder:test", "voice", "h3", base.Add(2*time.Minute)); err != nil {
t.Fatal(err)
}
@@ -63,7 +74,7 @@ func TestListDeliveryAttempts(t *testing.T) {
t.Fatalf("ListDeliveryAttempts = %d rows, err=%v, want 3", len(all), err)
}
// Newest first.
if all[0].Kind != "reminder" || all[0].ReminderID != 7 {
if all[0].Kind != "reminder" || all[0].ReminderID != reminderID {
t.Fatalf("newest row is %+v, want the reminder", all[0])
}
if all[0].HasComplete {
+112 -4
View File
@@ -87,6 +87,7 @@ const (
var (
ErrReminderNotFound = errors.New("store: reminder not found")
ErrReminderState = errors.New("store: reminder not in a mutable state")
ErrReminderInFlight = errors.New("store: reminder delivery already started")
ErrReminderPhrase = errors.New("store: reminder delivery phrase invalid")
)
@@ -206,15 +207,17 @@ func (s *Store) PendingReminders(ctx context.Context, from, to time.Time) ([]Rem
return out, rows.Err()
}
// MarkReminder sets a reminder's status. Only valid transitions: pending→fired,
// pending→cancelled. Anything else is a programming error.
// MarkReminder records successful completion of a reminder delivery. The only
// valid transition is pending→fired. Cancellation has stronger outbox and
// collapsed-group invariants and must go through CancelReminder; accepting the
// same status here would leave a legacy bypass around those invariants.
func (s *Store) MarkReminder(ctx context.Context, id int64, status string) error {
if status != ReminderFired && status != ReminderCancelled {
if status != ReminderFired {
return fmt.Errorf("%w: %s", ErrReminderState, status)
}
// The old read-then-write transition allowed two callers to both observe
// pending and both report success. Keeping the source state in the UPDATE
// predicate makes pending → fired|cancelled one atomic contest (V-678).
// predicate makes pending → fired one atomic contest (V-678).
res, err := s.db.ExecContext(ctx,
"UPDATE reminders SET status = ? WHERE id = ? AND status = ?",
status, id, ReminderPending)
@@ -242,6 +245,82 @@ func (s *Store) MarkReminder(ctx context.Context, id int64, status string) error
return fmt.Errorf("%w: currently %s", ErrReminderState, current)
}
// CancelReminder atomically wins against the beginning of external delivery.
// A pending/sent/unknown outbox row means the presentation may already be
// outside the process, so claiming cancellation would be false. Definite
// failures do not block cancellation.
//
// A collapsed catch-up bundle shares one cached phrase. Cancelling any member
// invalidates that presentation on every still-pending sibling; otherwise the
// next retry could continue saying "three reminders" after one was removed.
func (s *Store) CancelReminder(ctx context.Context, id int64) error {
tx, err := s.db.BeginTx(ctx, nil)
if err != nil {
return fmt.Errorf("cancel reminder: begin: %w", err)
}
defer tx.Rollback()
var status, group string
var nextFire int64
err = tx.QueryRowContext(ctx,
`SELECT status, delivery_group, next_fire_ts FROM reminders WHERE id = ?`, id,
).Scan(&status, &group, &nextFire)
if errors.Is(err, sql.ErrNoRows) {
return ErrReminderNotFound
}
if err != nil {
return fmt.Errorf("cancel reminder %d: read: %w", id, err)
}
if status != ReminderPending {
return fmt.Errorf("%w: currently %s", ErrReminderState, status)
}
// New attempts carry an occurrence-scoped delivery group. Migration 25
// assigned older rows the empty group, so those can only be related to this
// occurrence when they began at or after its next-fire boundary. Without
// that bound, one successful delivery from a recurring reminder's history
// would make the whole series permanently uncancellable after an upgrade.
var live int
err = tx.QueryRowContext(ctx, `
SELECT COUNT(*)
FROM delivery_attempts
WHERE kind = 'reminder'
AND status IN ('pending', 'sent', 'unknown')
AND ((delivery_group <> '' AND delivery_group = ?)
OR (delivery_group = '' AND reminder_id = ? AND created_ts >= ?))`,
group, id, nextFire).Scan(&live)
if err != nil {
return fmt.Errorf("cancel reminder %d: inspect delivery: %w", id, err)
}
if live > 0 {
return ErrReminderInFlight
}
if group != "" {
if _, err := tx.ExecContext(ctx, `
UPDATE reminders
SET delivery_group = '', phrase_body = '', phrase_summary = '', phrase_mood = ''
WHERE delivery_group = ? AND status = ?`, group, ReminderPending); err != nil {
return fmt.Errorf("cancel reminder %d: invalidate group: %w", id, err)
}
}
res, err := tx.ExecContext(ctx, `
UPDATE reminders
SET status = ?, delivery_group = '', phrase_body = '', phrase_summary = '', phrase_mood = '',
next_attempt_ts = NULL, delivery_blocked_ts = NULL, delivery_blocked_error = ''
WHERE id = ? AND status = ?`, ReminderCancelled, id, ReminderPending)
if err != nil {
return fmt.Errorf("cancel reminder %d: %w", id, err)
}
if err := requireOneReminderRow(res, id); err != nil {
return err
}
if err := tx.Commit(); err != nil {
return fmt.Errorf("cancel reminder %d: commit: %w", id, err)
}
return nil
}
// ListReminders returns the n most recent reminders, newest first.
func (s *Store) ListReminders(ctx context.Context, n int) ([]Reminder, error) {
rows, err := s.db.QueryContext(ctx, `SELECT `+reminderColumns+`
@@ -261,6 +340,35 @@ func (s *Store) ListReminders(ctx context.Context, n int) ([]Reminder, error) {
return out, rows.Err()
}
// ListPendingReminders returns cancellable reminders in firing order. n <= 0
// means all pending rows: spoken resolution must not miss an old reminder just
// because newer fired history filled ListReminders' window.
func (s *Store) ListPendingReminders(ctx context.Context, n int) ([]Reminder, error) {
query := `SELECT ` + reminderColumns + `
FROM reminders
WHERE status = ?
ORDER BY next_fire_ts ASC, id ASC`
args := []any{ReminderPending}
if n > 0 {
query += ` LIMIT ?`
args = append(args, n)
}
rows, err := s.db.QueryContext(ctx, query, args...)
if err != nil {
return nil, fmt.Errorf("list pending reminders: %w", err)
}
defer rows.Close()
var out []Reminder
for rows.Next() {
r, err := scanReminder(rows)
if err != nil {
return nil, err
}
out = append(out, r)
}
return out, rows.Err()
}
// HasDeliveryPhrase reports whether this occurrence already has a durable
// presentation. Summary may intentionally be empty (the delivery boundary has
// a generic privacy-preserving fallback), so Body is the readiness marker.
+243
View File
@@ -0,0 +1,243 @@
package store
import (
"context"
"errors"
"sync"
"testing"
"time"
)
func createCancellationReminder(t *testing.T, s *Store, fire time.Time, text string) Reminder {
t.Helper()
id, err := s.CreateReminder(context.Background(), fire, `{"text":"`+text+`"}`, "")
if err != nil {
t.Fatal(err)
}
rows, err := s.ListReminders(context.Background(), 1)
if err != nil || len(rows) != 1 || rows[0].ID != id {
t.Fatalf("created reminder %d, list=%+v err=%v", id, rows, err)
}
return rows[0]
}
func TestListPendingRemindersForCancellation(t *testing.T) {
ctx := context.Background()
s := newTestStore(t)
now := time.Date(2026, 8, 15, 9, 0, 0, 0, time.UTC)
later := createCancellationReminder(t, s, now.Add(2*time.Hour), "later")
earlier := createCancellationReminder(t, s, now.Add(time.Hour), "earlier")
fired := createCancellationReminder(t, s, now.Add(3*time.Hour), "already fired")
if err := s.MarkReminder(ctx, fired.ID, ReminderFired); err != nil {
t.Fatal(err)
}
all, err := s.ListPendingReminders(ctx, 0)
if err != nil {
t.Fatal(err)
}
if len(all) != 2 || all[0].ID != earlier.ID || all[1].ID != later.ID {
t.Fatalf("pending firing order = %+v", all)
}
one, err := s.ListPendingReminders(ctx, 1)
if err != nil || len(one) != 1 || one[0].ID != earlier.ID {
t.Fatalf("limited pending = %+v, %v", one, err)
}
}
func TestCancelReminderLifecycle(t *testing.T) {
ctx := context.Background()
s := newTestStore(t)
if err := s.CancelReminder(ctx, 999); !errors.Is(err, ErrReminderNotFound) {
t.Fatalf("missing cancellation = %v", err)
}
r := createCancellationReminder(t, s, time.Now().Add(time.Hour), "doctor")
if err := s.CancelReminder(ctx, r.ID); err != nil {
t.Fatal(err)
}
rows, err := s.ListReminders(ctx, 1)
if err != nil || len(rows) != 1 || rows[0].Status != ReminderCancelled {
t.Fatalf("cancelled row = %+v, %v", rows, err)
}
if err := s.CancelReminder(ctx, r.ID); !errors.Is(err, ErrReminderState) {
t.Fatalf("second cancellation = %v", err)
}
}
func TestMarkReminderCannotBypassCancellationSafety(t *testing.T) {
ctx := context.Background()
s := newTestStore(t)
r := createCancellationReminder(t, s, time.Now().Add(time.Hour), "doctor")
if err := s.MarkReminder(ctx, r.ID, ReminderCancelled); !errors.Is(err, ErrReminderState) {
t.Fatalf("legacy cancelled transition = %v, want ErrReminderState", err)
}
if got := reminderStatusesForTest(t, s)[r.ID]; got != ReminderPending {
t.Fatalf("legacy transition changed status to %q", got)
}
}
func reminderStatusesForTest(t *testing.T, s *Store) map[int64]string {
t.Helper()
rows, err := s.ListReminders(context.Background(), 100)
if err != nil {
t.Fatal(err)
}
out := make(map[int64]string, len(rows))
for _, row := range rows {
out[row.ID] = row.Status
}
return out
}
func TestCancelReminderInvalidatesCollapsedPhrase(t *testing.T) {
ctx := context.Background()
s := newTestStore(t)
now := time.Date(2026, 8, 15, 9, 0, 0, 0, time.UTC)
a := createCancellationReminder(t, s, now.Add(time.Hour), "doctor")
b := createCancellationReminder(t, s, now.Add(2*time.Hour), "bread")
originals, err := s.ListPendingReminders(ctx, 0)
if err != nil {
t.Fatal(err)
}
const group = "reminder:collapsed-cancel-test"
if err := s.CacheReminderPhrase(ctx, originals, group, "two reminders", "two", "neutral"); err != nil {
t.Fatal(err)
}
attempt, err := s.BeginDeliveryAttempt(ctx, "reminder", "", a.ID, group, "telegram", "hash", now)
if err != nil {
t.Fatal(err)
}
if err := s.CompleteDeliveryAttempt(ctx, attempt, DeliveryFailed, now); err != nil {
t.Fatal(err)
}
if err := s.CancelReminder(ctx, a.ID); err != nil {
t.Fatal(err)
}
rows, err := s.ListReminders(ctx, 10)
if err != nil {
t.Fatal(err)
}
byID := make(map[int64]Reminder, len(rows))
for _, row := range rows {
byID[row.ID] = row
}
if got := byID[a.ID]; got.Status != ReminderCancelled || got.HasDeliveryPhrase() {
t.Fatalf("cancelled member retained presentation: %+v", got)
}
if got := byID[b.ID]; got.Status != ReminderPending || got.HasDeliveryPhrase() || got.DeliveryGroup != "" {
t.Fatalf("surviving member retained stale bundle: %+v", got)
}
}
func TestCancelReminderRefusesAmbiguousDelivery(t *testing.T) {
for _, status := range []string{DeliveryPending, DeliveryUnknown, DeliverySent} {
t.Run(status, func(t *testing.T) {
ctx := context.Background()
s := newTestStore(t)
now := time.Date(2026, 8, 15, 9, 0, 0, 0, time.UTC)
r := createCancellationReminder(t, s, now.Add(time.Hour), status)
group := "reminder:" + status
if err := s.CacheReminderPhrase(ctx, []Reminder{r}, group, status, status, "neutral"); err != nil {
t.Fatal(err)
}
attempt, err := s.BeginDeliveryAttempt(ctx, "reminder", "", r.ID, group, "telegram", "hash", now)
if err != nil {
t.Fatal(err)
}
switch status {
case DeliveryUnknown:
if _, err := s.ReconcileStaleDeliveryAttempts(ctx, now.Add(time.Minute)); err != nil {
t.Fatal(err)
}
case DeliverySent:
if err := s.CompleteDeliveryAttempt(ctx, attempt, DeliverySent, now); err != nil {
t.Fatal(err)
}
}
if err := s.CancelReminder(ctx, r.ID); !errors.Is(err, ErrReminderInFlight) {
t.Fatalf("cancel with %s attempt = %v", status, err)
}
})
}
}
func TestCancelReminderScopesLegacyAttemptsToTheCurrentOccurrence(t *testing.T) {
ctx := context.Background()
now := time.Date(2026, 8, 15, 9, 0, 0, 0, time.UTC)
t.Run("historical recurring send does not block the series", func(t *testing.T) {
s := newTestStore(t)
fire := now.Add(24 * time.Hour)
id, err := s.CreateReminder(ctx, fire, `{"text":"daily medicine"}`, "0 9 * * *")
if err != nil {
t.Fatal(err)
}
if _, err := s.db.ExecContext(ctx, `
INSERT INTO delivery_attempts
(kind, reminder_id, delivery_group, channel, body_hash, status, created_ts, completed_ts)
VALUES ('reminder', ?, '', 'telegram', 'legacy', 'sent', ?, ?)`,
id, now.UnixMilli(), now.UnixMilli()); err != nil {
t.Fatal(err)
}
if err := s.CancelReminder(ctx, id); err != nil {
t.Fatalf("historical blank-group attempt blocked recurrence: %v", err)
}
})
t.Run("current legacy ambiguity still refuses", func(t *testing.T) {
s := newTestStore(t)
fire := now.Add(time.Hour)
r := createCancellationReminder(t, s, fire, "legacy current")
if _, err := s.db.ExecContext(ctx, `
INSERT INTO delivery_attempts
(kind, reminder_id, delivery_group, channel, body_hash, status, created_ts)
VALUES ('reminder', ?, '', 'telegram', 'legacy', 'unknown', ?)`,
r.ID, fire.Add(time.Second).UnixMilli()); err != nil {
t.Fatal(err)
}
if err := s.CancelReminder(ctx, r.ID); !errors.Is(err, ErrReminderInFlight) {
t.Fatalf("current blank-group ambiguity = %v, want ErrReminderInFlight", err)
}
})
}
func TestReminderCancellationAndDeliveryBeginAreMutuallyExclusive(t *testing.T) {
for i := 0; i < 20; i++ {
s := newTestStore(t)
ctx := context.Background()
now := time.Date(2026, 8, 15, 9, 0, 0, i, time.UTC)
r := createCancellationReminder(t, s, now.Add(time.Hour), "race")
group := "reminder:race"
if err := s.CacheReminderPhrase(ctx, []Reminder{r}, group, "race", "race", "neutral"); err != nil {
t.Fatal(err)
}
start := make(chan struct{})
var cancelErr, beginErr error
var wg sync.WaitGroup
wg.Add(2)
go func() {
defer wg.Done()
<-start
cancelErr = s.CancelReminder(ctx, r.ID)
}()
go func() {
defer wg.Done()
<-start
_, beginErr = s.BeginDeliveryAttempt(ctx, "reminder", "", r.ID, group, "telegram", "hash", now)
}()
close(start)
wg.Wait()
if (cancelErr == nil) == (beginErr == nil) {
t.Fatalf("iteration %d: cancel=%v begin=%v; exactly one must win", i, cancelErr, beginErr)
}
if cancelErr == nil && !errors.Is(beginErr, ErrReminderState) {
t.Fatalf("iteration %d: cancellation won, begin=%v", i, beginErr)
}
if beginErr == nil && !errors.Is(cancelErr, ErrReminderInFlight) {
t.Fatalf("iteration %d: delivery won, cancel=%v", i, cancelErr)
}
}
}
+13 -11
View File
@@ -158,15 +158,17 @@ func TestReminderTerminalTransitionHasExactlyOneWinner(t *testing.T) {
start := make(chan struct{})
var wg sync.WaitGroup
errs := make([]error, 2)
statuses := []string{ReminderFired, ReminderCancelled}
for i := range statuses {
wg.Add(1)
go func(i int) {
defer wg.Done()
<-start
errs[i] = s.MarkReminder(ctx, id, statuses[i])
}(i)
}
wg.Add(2)
go func() {
defer wg.Done()
<-start
errs[0] = s.MarkReminder(ctx, id, ReminderFired)
}()
go func() {
defer wg.Done()
<-start
errs[1] = s.CancelReminder(ctx, id)
}()
close(start)
wg.Wait()
@@ -177,11 +179,11 @@ func TestReminderTerminalTransitionHasExactlyOneWinner(t *testing.T) {
switch {
case err == nil:
successes++
winner = statuses[i]
winner = []string{ReminderFired, ReminderCancelled}[i]
case errors.Is(err, ErrReminderState):
losers++
default:
t.Fatalf("iteration %d transition %s: %v", iteration, statuses[i], err)
t.Fatalf("iteration %d transition %d: %v", iteration, i, err)
}
}
if successes != 1 || losers != 1 {