Make new_suggestions and Send to Google Chat recoverable

MQ-167-01: a scheduled or manual batch now emits new_suggestions:{batchId}
in the transaction that first stores its Side quests, so the cards and
their notification commit together. A later round that fails, or a
failure recording the batch's end, no longer loses the event; a later
round's emit is a no-op by key; a notification that cannot be stored
fails that store, leaving no unannounced cards. Replenish batches still
never notify. finishBatch goes back to recording status only.

MQ-167-02: migration 0017 adds shopping_lists.chat_send_unconfirmed_at.
SendToChat records the send as begun before posting, clears it with
sent_to_chat_at once Google Chat accepts, and clears it alone when Chat
refuses. A post Chat does not answer, or one it accepts that cannot be
recorded, is ErrSendUnconfirmed (502 chat_unconfirmed, not retryable)
and leaves the marker, as does a crash mid-send; a repeat without resend
never reposts, and resend posts again. The Supply run screen re-reads
the list after a failed send, shows an unconfirmed send, offers no Retry
for it, and sends again only with resend.

Task: f0cd9d50-9c11-41be-a945-7bae9f5f28a4

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

internal/suggest/batch.go

@@ -15,7 +15,6 @@ import (
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs"
-	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store"
 )
@@ -58,12 +57,12 @@ func (r batchResult) PublicJSON() ([]byte, error) { return json.Marshal(r) }
 // shutdown, the job is left running and the batch with it, for the next
 // start to fail.
 //
-// A scheduled or manual batch that ends done having added Side quests to
-// the deck emits new_suggestions, keyed by the batch, in the transaction
-// that records it done. A replenish batch, which tops up the deck as the
-// household swipes, never notifies. A notification that cannot be stored
-// fails the done write with it, as any failure recording the batch's end
-// does: the job fails, and its Side quests stay in the deck.
+// A scheduled or manual batch emits new_suggestions, keyed by the batch,
+// in the transaction that first adds Side quests to the deck, so the
+// household is told of every Side quest stored, whether or not the batch
+// then ends done: a later round, or recording the batch's end, may fail.
+// A replenish batch, which tops up the deck as the household swipes, never
+// notifies. See store.
 func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchResult) error) (batchResult, error) {
 	batch, err := db.New(s.st.Reader).GetSuggestionBatch(ctx, p.BatchID)
 	if err != nil {
@@ -79,19 +78,14 @@ func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchRe
 	if n == 0 {
 		return batchResult{}, fmt.Errorf("suggest: batch %s is not pending", p.BatchID)
 	}
-	f, err := s.fill(ctx, p.BatchID, publish)
+	res, err := s.fill(ctx, p.BatchID, batch.Trigger != TriggerReplenish, publish)
 	if err == nil {
-		var ev *notify.Event
-		if batch.Trigger != TriggerReplenish && len(f.names) > 0 {
-			e := chat.NewSuggestions(p.BatchID, household.WeekStart(f.today), f.names)
-			ev = &e
-		}
-		if err := s.finishBatch(ctx, p.BatchID, BatchDone, ev); err != nil {
+		if err := s.finishBatch(ctx, p.BatchID, BatchDone); err != nil {
 			return batchResult{}, err
 		}
-		return f.res, nil
+		return res, nil
 	}
-	finishErr := s.finishBatch(ctx, p.BatchID, BatchFailed, nil)
+	finishErr := s.finishBatch(ctx, p.BatchID, BatchFailed)
 	if ctx.Err() != nil {
 		return batchResult{}, errors.Join(ctx.Err(), finishErr)
 	}
@@ -104,60 +98,56 @@ func (s *Service) runBatch(ctx context.Context, p BatchJob, publish func(batchRe
 	return batchResult{}, errors.Join(err, finishErr)
 }
 
-// filled is what fill did: the batch's result, the names of the Side
-// quests it added to the deck, and the household's date it ran on.
-type filled struct {
-	res   batchResult
-	names []string
-	today store.LocalDate
-}
-
 // fill does runBatch's work, returning a *jobs.Failure for a failure the
-// household may see.
-func (s *Service) fill(ctx context.Context, batchID string, publish func(batchResult) error) (filled, error) {
+// household may see. When notifies is true, store emits new_suggestions
+// with the Side quests it adds.
+func (s *Service) fill(ctx context.Context, batchID string, notifies bool, publish func(batchResult) error) (batchResult, error) {
 	res := batchResult{BatchID: batchID, Stage: StageSuggesting, SuggestionIDs: []string{}}
 	if err := publish(res); err != nil {
-		return filled{}, err
+		return batchResult{}, err
 	}
 	in, err := s.request(ctx)
 	if err != nil {
-		return filled{}, err
+		return batchResult{}, err
 	}
 	out, err := s.suggest(ctx, batchID, in)
 	if err != nil {
 		if ctx.Err() != nil {
-			return filled{}, ctx.Err()
+			return batchResult{}, ctx.Err()
 		}
-		return filled{}, &jobs.Failure{Code: ReasonAIFailed}
+		return batchResult{}, &jobs.Failure{Code: ReasonAIFailed}
 	}
 	res.Stage = StageGrounding
 	res.Dropped = len(out.Dropped)
 	if err := publish(res); err != nil {
-		return filled{}, err
+		return batchResult{}, err
+	}
+	var week *store.LocalDate
+	if notifies {
+		w := household.WeekStart(in.Context.Today)
+		week = &w
 	}
-	var names []string
 	left := out.Suggestions
 	for len(res.SuggestionIDs) < BatchSize && len(left) > 0 {
 		round := left[:min(BatchSize-len(res.SuggestionIDs), len(left))]
 		left = left[len(round):]
 		grounded, err := s.ground(ctx, round)
 		if err != nil {
-			return filled{}, err
+			return batchResult{}, err
 		}
 		res.Dropped += len(round) - len(grounded)
-		ids, stored, err := s.store(ctx, batchID, grounded)
+		ids, err := s.store(ctx, batchID, week, grounded)
 		if err != nil {
-			return filled{}, err
+			return batchResult{}, err
 		}
 		res.Dropped += len(grounded) - len(ids)
 		res.SuggestionIDs = append(res.SuggestionIDs, ids...)
-		names = append(names, stored...)
 		if err := publish(res); err != nil {
-			return filled{}, err
+			return batchResult{}, err
 		}
 	}
 	res.Stage = StageDone
-	return filled{res: res, names: names, today: in.Context.Today}, nil
+	return res, nil
 }
 
 // request reads what suggest_side_quests is given: the household context,
@@ -232,9 +222,15 @@ func (s *Service) ground(ctx context.Context, quests []skills.SideQuest) ([]grou
 // suggest_side_quests ran, a dinner it repeats was added, a suggestion it
 // repeats passed or a Rotation check dropped, or a Regular it cites renamed
 // or archived. Passed names are read in the same transaction as the store,
-// so a Pass or drop committed before it is never missed. The names stored
-// are returned with the ids.
-func (s *Service) store(ctx context.Context, batchID string, quests []groundedQuest) ([]string, []string, error) {
+// so a Pass or drop committed before it is never missed.
+//
+// When week is not nil and a Side quest is stored, new_suggestions:{batchID}
+// naming those stored is emitted in the same transaction, for the week
+// starting week, so the Side quests and their notification commit together
+// or not at all: a notification that cannot be stored fails the store, and
+// the batch with it. The event's key makes the first round to store Side
+// quests the one that notifies; a later round's emit stores nothing.
+func (s *Service) store(ctx context.Context, batchID string, week *store.LocalDate, quests []groundedQuest) ([]string, error) {
 	ids := []string{}
 	var stored []string
 	err := s.st.WithTx(ctx, func(tx *sql.Tx) error {
@@ -302,12 +298,18 @@ func (s *Service) store(ctx context.Context, batchID string, quests []groundedQu
 			ids = append(ids, id)
 			stored = append(stored, d.Name)
 		}
-		return nil
+		if week == nil || len(stored) == 0 {
+			return nil
+		}
+		return s.notifier.EmitTx(ctx, tx, chat.NewSuggestions(batchID, *week, stored))
 	})
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
-	return ids, stored, nil
+	if week != nil && len(stored) > 0 {
+		s.notifier.Wake()
+	}
+	return ids, nil
 }
 
 // citedIDs returns the ids of the Regulars named cited, false when one

internal/suggest/suggest.go

@@ -257,39 +257,27 @@ func (s *Service) startBatch(ctx context.Context, tx *sql.Tx, trigger string) (B
 	return Batch{ID: row.ID, JobID: jobID, Trigger: row.Trigger, Status: row.Status, Created: true}, nil
 }
 
-// finishBatch records running batch id as status, and emits ev with it
-// when ev is not nil, with jobs.FinalWriteContext: the write is made even
-// when ctx has been cancelled, but a writer that stays busy cannot hold its
-// worker past jobs.FinalWriteTimeout. Its job's handler returns any error, so the job
+// finishBatch records running batch id as status, with
+// jobs.FinalWriteContext: the write is made even when ctx has been
+// cancelled, but a writer that stays busy cannot hold its worker past
+// jobs.FinalWriteTimeout. Its job's handler returns any error, so the job
 // never ends succeeded with the batch still running; the runner keeps only
 // a code, so the error is logged too.
-func (s *Service) finishBatch(ctx context.Context, id, status string, ev *notify.Event) error {
+func (s *Service) finishBatch(ctx context.Context, id, status string) error {
 	write, cancel := jobs.FinalWriteContext(ctx)
 	defer cancel()
-	err := s.st.WithTx(write, func(tx *sql.Tx) error {
-		n, err := db.New(tx).FinishSuggestionBatch(write, db.FinishSuggestionBatchParams{
-			Status:    status,
-			UpdatedAt: store.Now().String(),
-			ID:        id,
-		})
-		if err != nil {
-			return err
-		}
-		if n != 1 {
-			return errors.New("batch is not running")
-		}
-		if ev == nil {
-			return nil
-		}
-		return s.notifier.EmitTx(write, tx, *ev)
+	n, err := db.New(s.st.Writer).FinishSuggestionBatch(write, db.FinishSuggestionBatchParams{
+		Status:    status,
+		UpdatedAt: store.Now().String(),
+		ID:        id,
 	})
+	if err == nil && n != 1 {
+		err = errors.New("batch is not running")
+	}
 	if err != nil {
 		s.logger.ErrorContext(ctx, "record suggestion batch status", "batch_id", id, "status", status, "error", err)
 		return fmt.Errorf("suggest: record batch %s %s: %w", id, status, err)
 	}
-	if ev != nil {
-		s.notifier.Wake()
-	}
 	return nil
 }
 

internal/suggest/suggest_test.go

@@ -454,6 +454,100 @@ func TestBatchEmitsNewSuggestions(t *testing.T) {
 	}
 }
 
+// TestBatchNotifiesAsSideQuestsAreStored: new_suggestions is stored with
+// the first round to add Side quests, so a batch whose later round fails
+// still tells the household of those in the deck, once; a later round that
+// succeeds adds no second notification; a replenish batch notifies nothing.
+func TestBatchNotifiesAsSideQuestsAreStored(t *testing.T) {
+	for _, tc := range []struct {
+		trigger   string
+		lastFails bool
+		notify    bool
+	}{
+		{suggest.TriggerScheduled, true, true},
+		{suggest.TriggerManual, true, true},
+		{suggest.TriggerManual, false, true},
+		{suggest.TriggerReplenish, true, false},
+	} {
+		t.Run(fmt.Sprintf("%s last round fails %v", tc.trigger, tc.lastFails), func(t *testing.T) {
+			e := newEnv(t)
+			e.start(t)
+			// The first round grounds BatchSize Side quests but B; the
+			// second round grounds F.
+			var quests []string
+			for i := range suggest.BatchSize + 1 {
+				name := fmt.Sprintf("Fajita Dish %c", 'A'+i)
+				e.search.Results[name] = nil
+				if i != 1 {
+					e.recipe(t, name)
+				}
+				quests = append(quests, sideQuest(name, "Like your Chicken Fajitas.", "Chicken Fajitas"))
+			}
+			e.reply(quests...)
+			if tc.lastFails {
+				if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_f BEFORE INSERT ON dinners WHEN NEW.name = 'Fajita Dish F' BEGIN SELECT RAISE(ABORT, 'F refused'); END`); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			b, err := e.svc.StartBatch(t.Context(), tc.trigger)
+			if err != nil {
+				t.Fatal(err)
+			}
+			job := e.waitJob(t, b.JobID)
+			want, deck := jobs.StatusSucceeded, suggest.BatchSize
+			if tc.lastFails {
+				want, deck = jobs.StatusFailed, suggest.BatchSize-1
+			}
+			if job.Status != want {
+				t.Fatalf("batch job %s, want %s", job.Status, want)
+			}
+			if got := e.deckNames(t); len(got) != deck {
+				t.Fatalf("deck = %v, want %d Side quests", got, deck)
+			}
+			rows := notifytest.Rows(t, e.st, notify.NewSuggestions)
+			if !tc.notify {
+				if len(rows) != 0 {
+					t.Errorf("new_suggestions rows = %+v, want none", rows)
+				}
+				return
+			}
+			if len(rows) != 1 || rows[0].Key != "new_suggestions:"+b.ID ||
+				rows[0].Payload.Body != "Fajita Dish A, Fajita Dish C, Fajita Dish D + 1 more" {
+				t.Errorf("new_suggestions rows = %+v, want one keyed by batch %s naming the first round", rows, b.ID)
+			}
+		})
+	}
+}
+
+// TestBatchFailsWhenItsNotificationCannotBeStored: Side quests are stored
+// only with their notification, so a notification that cannot be stored
+// fails the batch with an empty deck rather than leaving Side quests the
+// household is never told of.
+func TestBatchFailsWhenItsNotificationCannotBeStored(t *testing.T) {
+	e := newEnv(t)
+	e.start(t)
+	e.recipe(t, "Chili Verde")
+	e.reply(sideQuest("Chili Verde", "Your Beef Chili gone green.", "Beef Chili"))
+	if _, err := e.st.Writer.ExecContext(t.Context(), `CREATE TRIGGER refuse_outbox BEFORE INSERT ON notification_outbox BEGIN SELECT RAISE(ABORT, 'outbox refused'); END`); err != nil {
+		t.Fatal(err)
+	}
+
+	b, err := e.svc.StartBatch(t.Context(), suggest.TriggerManual)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if job := e.waitJob(t, b.JobID); job.Status != jobs.StatusFailed {
+		t.Fatalf("batch job %s, want failed", job.Status)
+	}
+	if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed {
+		t.Errorf("batch status = %s, want failed", got)
+	}
+	if got := e.deckNames(t); len(got) != 0 {
+		t.Errorf("deck = %v with no notification stored, want empty", got)
+	}
+}
+
 func TestBatchKeepsAtMostBatchSize(t *testing.T) {
 	e := newEnv(t)
 	e.start(t)
@@ -1204,8 +1298,9 @@ func TestBatchJobFailsWhenRecordingItsEndFails(t *testing.T) {
 	if got := e.batchStatus(t, b.ID); got != suggest.BatchFailed {
 		t.Errorf("batch status = %s with its job failed, want failed", got)
 	}
-	if rows := notifytest.Rows(t, e.st, notify.NewSuggestions); len(rows) != 0 {
-		t.Errorf("new_suggestions rows of a batch whose end was not recorded = %+v, want none", rows)
+	// Its Side quests stay in the deck, and the household was told of them.
+	if rows := notifytest.Rows(t, e.st, notify.NewSuggestions); len(rows) != 1 || rows[0].Key != "new_suggestions:"+b.ID || rows[0].Payload.Body != "Chili Verde" {
+		t.Errorf("new_suggestions rows of a batch whose end was not recorded = %+v, want one naming Chili Verde", rows)
 	}
 }