Revalidate the draft pool on save, refuse duplicate swaps, refresh the Quest log after a draft

Feature 14 integration fix (Maestro 857e4c33-e250-46ec-b5ea-7dd4f3b719d9). - F14-01: storeDraft re-reads the draft pool (ListDraftPool, the same eligibility rule draft_week was given) inside the save transaction. A day whose dinner was archived, moved to tried, or planned on another current or later week while draft_week ran fails the job with the new retryable reason pool_changed and stores nothing. - F14-02: Swap refuses a dinner already on another day of the week inside the write transaction; the API answers 409 dinner_already_planned. - F14-03: a finished draft invalidates the Quest log query, as the swap path does, so the tablet strip and desktop panel drop dinners it planned. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-07 16:13:22 -04:00
parent 955613941f
commit b3b342ef55
8 changed files with 168 additions and 22 deletions
@@ -36,9 +36,9 @@ func (r draftResult) PublicJSON() ([]byte, error) { return json.Marshal(r) }
 // Regulars and the Quest log dinners no other current or later plan has,
 // given the household context and last week's dinners, and the draft is
 // stored as the week's plan if the plan is still as it was before
-// draft_week ran. A pool under seven dinners fails with
-// ReasonNotEnoughDinners and draft_week failing with ReasonAIFailed; no
-// week is made up instead.
+// draft_week ran and every dinner it planned is still in the pool. A pool
+// under seven dinners fails with ReasonNotEnoughDinners and draft_week
+// failing with ReasonAIFailed; no week is made up instead.
 func (s *Service) runDraft(ctx context.Context, p DraftJob, _ func(draftResult) error) (draftResult, error) {
 	weekStart, err := ParseWeekStart(p.WeekStart)
 	if err != nil {
@@ -68,7 +68,7 @@ func (s *Service) runDraft(ctx context.Context, p DraftJob, _ func(draftResult)
 	for _, w := range week.Warnings {
 		s.logger.InfoContext(ctx, "week draft warning", "week_start", p.WeekStart, "warning", w)
 	}
-	planID, err := s.storeDraft(ctx, weekStart, p.Confirm, base, week)
+	planID, err := s.storeDraft(ctx, weekStart, household.WeekStart(in.Context.Today), p.Confirm, base, week)
 	if err != nil {
 		return draftResult{}, err
 	}
@@ -142,11 +142,28 @@ func (s *Service) request(ctx context.Context, weekStart store.LocalDate) (skill
 // draft was asked for without confirm is left as it is, failing with
 // ReasonConfirmRequired, and so is a plan no longer at revision base (the
 // revision before draft_week ran, or noPlan), failing with
-// ReasonPlanChanged.
-func (s *Service) storeDraft(ctx context.Context, weekStart store.LocalDate, confirm bool, base int64, week skills.Week) (string, error) {
+// ReasonPlanChanged. The pool is read again, as of thisWeek like the pool
+// draft_week was given: a day whose dinner is no longer in it, or is in it
+// as a Regular rather than the Quest log or the other way round, leaves
+// everything as it is, failing with ReasonPoolChanged.
+func (s *Service) storeDraft(ctx context.Context, weekStart, thisWeek store.LocalDate, confirm bool, base int64, week skills.Week) (string, error) {
 	var planID string
 	err := s.st.WithTx(ctx, func(tx *sql.Tx) error {
 		q := db.New(tx)
+		rows, err := q.ListDraftPool(ctx, db.ListDraftPoolParams{FromWeekStart: thisWeek.String(), WeekStart: weekStart.String()})
+		if err != nil {
+			return fmt.Errorf("plan: list draft pool: %w", err)
+		}
+		pool := make(map[string]string, len(rows))
+		for _, d := range rows {
+			pool[d.ID] = d.Status
+		}
+		for _, d := range week.Days {
+			status, ok := pool[d.DinnerID]
+			if !ok || (status == dinners.StatusQuestLog) != d.IsNew {
+				return &jobs.Failure{Code: ReasonPoolChanged}
+			}
+		}
 		now := store.Now().String()
 		note := sql.NullString{String: week.Note, Valid: week.Note != ""}
 		cur, err := q.GetWeekPlan(ctx, weekStart.String())
@@ -53,10 +53,15 @@ const (
 	// ReasonPlanChanged: the plan was edited while draft_week ran, so the
 	// draft does not replace the edit.
 	ReasonPlanChanged = "plan_changed"
+	// ReasonPoolChanged: a dinner the draft planned left the pool while
+	// draft_week ran (archived, tried, or its Quest log dinner planned on
+	// another week), so the draft is not stored; drafting again uses the
+	// pool as it is now.
+	ReasonPoolChanged = "pool_changed"
 )

 // Reasons are every draft failure reason.
-var Reasons = []string{ReasonNotEnoughDinners, ReasonAIFailed, ReasonConfirmRequired, ReasonPlanChanged}
+var Reasons = []string{ReasonNotEnoughDinners, ReasonAIFailed, ReasonConfirmRequired, ReasonPlanChanged, ReasonPoolChanged}

 // ContextBudget is the character budget of the household context
 // draft_week is given (see harness.BuildContext).
@@ -71,6 +76,10 @@ var ErrNotFound = errors.New("plan not found")
 // ErrDinnerNotFound is a swap to an unknown dinner.
 var ErrDinnerNotFound = errors.New("dinner not found")

+// ErrDinnerAlreadyPlanned is a swap to a dinner already on another day of
+// the week.
+var ErrDinnerAlreadyPlanned = errors.New("the dinner is already on another day of this week")
+
 // ErrConfirmRequired is a draft over a synced or changed plan asked for
 // without confirm.
 var ErrConfirmRequired = errors.New("the plan was pushed to Calendar; drafting over it needs confirm")
@@ -337,9 +346,10 @@ func (s *Service) StartDraft(ctx context.Context, weekStart store.LocalDate, con
 // Swap plans dinnerID on date of the week starting weekStart, marking the
 // day new when the dinner is in the Quest log. Swapping a synced plan
 // makes it changed. The week must have a plan (else ErrNotFound), date
-// must be one of its days (else *InvalidDayError), and the dinner must be
-// a Regular or in the Quest log (else ErrDinnerNotFound or
-// *DinnerNotPlannableError). It returns the plan as swapped.
+// must be one of its days (else *InvalidDayError), the dinner must be a
+// Regular or in the Quest log (else ErrDinnerNotFound or
+// *DinnerNotPlannableError), and it must not be on another day of the
+// week (else ErrDinnerAlreadyPlanned). It returns the plan as swapped.
 func (s *Service) Swap(ctx context.Context, weekStart store.LocalDate, date, dinnerID string) (Plan, error) {
 	dates, err := household.WeekDays(weekStart)
 	if err != nil {
@@ -369,6 +379,13 @@ func (s *Service) Swap(ctx context.Context, weekStart store.LocalDate, date, din
 		if !slices.Contains(plannable, d.Status) {
 			return &DinnerNotPlannableError{Status: d.Status}
 		}
+		days, err := q.ListPlanDays(ctx, cur.ID)
+		if err != nil {
+			return fmt.Errorf("plan: read days of %s: %w", cur.ID, err)
+		}
+		if slices.ContainsFunc(days, func(r db.ListPlanDaysRow) bool { return r.Date != day.String() && r.Dinner.ID == d.ID }) {
+			return ErrDinnerAlreadyPlanned
+		}
 		now := store.Now().String()
 		n, err := q.SwapPlanDay(ctx, db.SwapPlanDayParams{
 			DinnerID:  sql.NullString{String: d.ID, Valid: true},
@@ -174,9 +174,15 @@ func (e *env) reply(note string, names ...string) {
 // weekReply is the draft_week reply planning the dinners named names on
 // weekDates, in order.
 func (e *env) weekReply(note string, names ...string) llm.GenerateResult {
+	return e.weekReplyOn(weekDates, note, names...)
+}
+
+// weekReplyOn is the draft_week reply planning the dinners named names on
+// dates, in order.
+func (e *env) weekReplyOn(dates []string, note string, names ...string) llm.GenerateResult {
 	var days []string
 	for i, name := range names {
-		days = append(days, fmt.Sprintf(`{"date":%q,"dinner_id":%q}`, weekDates[i], e.ids[name]))
+		days = append(days, fmt.Sprintf(`{"date":%q,"dinner_id":%q}`, dates[i], e.ids[name]))
 	}
 	js := fmt.Sprintf(`{"days":[%s],"note":%q}`, strings.Join(days, ","), note)
 	return llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}}
@@ -398,15 +404,16 @@ func TestStartByAcrossDaylightSavingTimeEnd(t *testing.T) {
 			t.Errorf("%s start_by = %s, %v; want %s", dates[tc.day], b, err, tc.want)
 		}
 	}
-	// Swapped to Beef Chili, Sunday starts at 13:30 EST.
-	if _, err := e.svc.Swap(t.Context(), dst, "2026-11-01", e.ids["Beef Chili"]); err != nil {
+	// Swapped to Short Ribs (240 minutes), Sunday starts at 13:30 EST.
+	ribs := e.dinner(t, "Short Ribs", "regular", "crockpot", 240)
+	if _, err := e.svc.Swap(t.Context(), dst, "2026-11-01", ribs); err != nil {
 		t.Fatal(err)
 	}
 	if p, err = e.svc.Get(t.Context(), dst); err != nil {
 		t.Fatal(err)
 	}
 	if b, err := json.Marshal(p.Days[6].StartBy); err != nil || string(b) != `"2026-11-01T13:30:00-05:00"` {
-		t.Errorf("Sunday Beef Chili start_by = %s, %v; want 13:30 EST", b, err)
+		t.Errorf("Sunday Short Ribs start_by = %s, %v; want 13:30 EST", b, err)
 	}
 }

@@ -466,6 +473,70 @@ func TestDraftPoolLeavesOutQuestLogPlannedOnAnotherWeek(t *testing.T) {
 	}
 }

+func TestDraftFailsWhenPoolDinnerLeavesWhileDrafting(t *testing.T) {
+	for _, status := range []string{"archived", "tried"} {
+		t.Run(status, func(t *testing.T) {
+			e := newEnv(t)
+			e.start(t)
+			// Pad Thai leaves the Quest log while draft_week plans it.
+			started, release := make(chan struct{}), make(chan struct{})
+			r := e.weekReply("", fullWeek...)
+			r.Started, r.Release = started, release
+			e.model.QueueGenerate(r)
+			d, err := e.svc.StartDraft(t.Context(), week, false)
+			if err != nil {
+				t.Fatal(err)
+			}
+			<-started
+			if _, err := e.st.Writer.ExecContext(t.Context(), `UPDATE dinners SET status = ? WHERE id = ?`, status, e.ids["Pad Thai"]); err != nil {
+				t.Fatal(err)
+			}
+			close(release)
+			job := e.waitJob(t, d.JobID)
+			if job.Status != jobs.StatusFailed || *job.Error != plan.ReasonPoolChanged {
+				t.Fatalf("job %s %v, want failed %s", job.Status, job.Error, plan.ReasonPoolChanged)
+			}
+			if _, err := e.svc.Get(t.Context(), week); !errors.Is(err, plan.ErrNotFound) {
+				t.Errorf("Get = %v, want ErrNotFound: no partial plan", err)
+			}
+		})
+	}
+}
+
+func TestDraftsOfTwoWeeksDoNotShareAQuestLogDinner(t *testing.T) {
+	e := newEnv(t)
+	e.start(t)
+	// This week's draft plans Pad Thai; while draft_week runs, the other
+	// worker drafts a later week with Pad Thai too and stores it first.
+	started, release := make(chan struct{}), make(chan struct{})
+	r := e.weekReply("", fullWeek...)
+	r.Started, r.Release = started, release
+	e.model.QueueGenerate(r)
+	d, err := e.svc.StartDraft(t.Context(), week, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	<-started
+	later := mustDate("2099-01-05")
+	laterDates := []string{"2099-01-05", "2099-01-06", "2099-01-07", "2099-01-08", "2099-01-09", "2099-01-10", "2099-01-11"}
+	e.model.QueueGenerate(e.weekReplyOn(laterDates, "", fullWeek...))
+	other, err := e.svc.StartDraft(t.Context(), later, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if job := e.waitJob(t, other.JobID); job.Status != jobs.StatusSucceeded {
+		t.Fatalf("later week's draft %s %v", job.Status, job.Error)
+	}
+	close(release)
+	job := e.waitJob(t, d.JobID)
+	if job.Status != jobs.StatusFailed || *job.Error != plan.ReasonPoolChanged {
+		t.Fatalf("job %s %v, want failed %s", job.Status, job.Error, plan.ReasonPoolChanged)
+	}
+	if _, err := e.svc.Get(t.Context(), week); !errors.Is(err, plan.ErrNotFound) {
+		t.Errorf("Get = %v, want ErrNotFound: no partial plan", err)
+	}
+}
+
 func TestDraftFailsWithNotEnoughDinners(t *testing.T) {
 	e := newEnv(t)
 	e.start(t)
@@ -633,7 +704,8 @@ func TestDraftFailsWhenPlanSwappedWhileDrafting(t *testing.T) {
 		t.Fatal(err)
 	}
 	<-started
-	if _, err := e.svc.Swap(t.Context(), week, "2026-10-14", e.ids["Sheet Pan Salmon"]); err != nil {
+	e.ids["Fish Tacos"] = e.dinner(t, "Fish Tacos", "regular", "stovetop", 25)
+	if _, err := e.svc.Swap(t.Context(), week, "2026-10-14", e.ids["Fish Tacos"]); err != nil {
 		t.Fatal(err)
 	}
 	close(release)
@@ -646,7 +718,7 @@ func TestDraftFailsWhenPlanSwappedWhileDrafting(t *testing.T) {
 		t.Fatal(err)
 	}
 	want := slices.Clone(fullWeek)
-	want[2] = "Sheet Pan Salmon"
+	want[2] = "Fish Tacos"
 	if *p.AINote != "first" || !slices.Equal(dayNames(p), want) {
 		t.Errorf("plan = %q %v, want the first draft with the swap %v", *p.AINote, dayNames(p), want)
 	}
@@ -822,7 +894,8 @@ func TestSwapStatusTransitions(t *testing.T) {
 			e.reply("", fullWeek...)
 			e.mustDraft(t, false)
 			e.setStatus(t, tc.from)
-			p, err := e.svc.Swap(t.Context(), week, "2026-10-12", e.ids["Pad Thai"])
+			fishTacos := e.dinner(t, "Fish Tacos", "regular", "stovetop", 25)
+			p, err := e.svc.Swap(t.Context(), week, "2026-10-12", fishTacos)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -852,6 +925,7 @@ func TestSwapRefusals(t *testing.T) {
 	}{
 		{"no plan", mustDate("2026-10-19"), "2026-10-19", e.ids["Pad Thai"], func(err error) bool { return errors.Is(err, plan.ErrNotFound) }},
 		{"unknown dinner", week, "2026-10-12", "no-such-dinner", func(err error) bool { return errors.Is(err, plan.ErrDinnerNotFound) }},
+		{"dinner on another day", week, "2026-10-12", e.ids["Pot Roast"], func(err error) bool { return errors.Is(err, plan.ErrDinnerAlreadyPlanned) }},
 		{"suggested dinner", week, "2026-10-12", suggested, func(err error) bool { return errors.As(err, &notPlannable) }},
 		{"archived dinner", week, "2026-10-12", archived, func(err error) bool { return errors.As(err, &notPlannable) }},
 		{"day of another week", week, "2026-10-19", e.ids["Pad Thai"], func(err error) bool { return errors.As(err, &badDay) }},