Expose the Supply run title job and cover the replacement race
CI / ci (push) Successful in 8m1s
CI / ci (pull_request) Successful in 7m52s

POST /shopping/{weekStart}/generate now answers with title_job_id, and
Push to Calendar's supply_run carries it too, so a client can follow the
title_supply_run job through GET /jobs/{id}, including a title_failed
failure. It is null when the list already has its title or was not
generated.

Add a regression test that replaces the week's list after archive_title
answers but before the title is stored: the old job fails title_failed
and the replacement stays untitled.

Maestro task a55adc28-4d8b-4517-b1d7-dcff39ea5a09.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-11 07:41:10 -04:00
co-authored by Claude Opus 5.5
parent be41b56690
commit a2ae76b5d4
5 changed files with 134 additions and 22 deletions
+10 -3
View File
@@ -1120,7 +1120,10 @@ Fake-clock tests of a schedule live in `internal/scheduler`, where
- The week's Supply run (Feature 17) is made from a pushed plan: after - The week's Supply run (Feature 17) is made from a pushed plan: after
a successful `Push`, the handler runs `shopping.GeneratePushed` for a successful `Push`, the handler runs `shopping.GeneratePushed` for
the week at the plan revision `Push` pushed, and the 200 answer the week at the plan revision `Push` pushed, and the 200 answer
carries `supply_run: {status, error}`. `status` is `generated`; carries `supply_run: {status, error, title_job_id}`. `title_job_id` is
the `title_supply_run` job titling a generated list, as for `POST
/shopping/{weekStart}/generate`; null when the list already has its
title or was not generated. `status` is `generated`;
`archived` when the week's Supply run is in the Supply archive and was `archived` when the week's Supply run is in the Supply archive and was
left as it was; or `failed` — error `plan_changed` (retryable, logged left as it was; or `failed` — error `plan_changed` (retryable, logged
as a warning) when the plan was edited between the push and as a warning) when the plan was edited between the push and
@@ -1143,8 +1146,12 @@ Fake-clock tests of a schedule live in `internal/scheduler`, where
when the week has no Supply run, 400 `invalid_week`. when the week has no Supply run, 400 `invalid_week`.
- `POST /api/v1/shopping/{weekStart}/generate` makes or remakes the list - `POST /api/v1/shopping/{weekStart}/generate` makes or remakes the list
through `shopping.Generate` and answers with the same shape plus through `shopping.Generate` and answers with the same shape plus
`skipped: [{date, dinner_id, dinner_name, reason}]`. 404 when the week `skipped: [{date, dinner_id, dinner_name, reason}]` and
has no plan, 409 `archived` when its list is in the Supply archive. `title_job_id`, the `title_supply_run` job titling the list, which
`GET /api/v1/jobs/{id}` reports on (failed `title_failed` when the title
could not be written); null when the list already has its title. 404
when the week has no plan, 409 `archived` when its list is in the Supply
archive.
- `POST /api/v1/shopping/{weekStart}/send-chat` (CSRF-checked) takes - `POST /api/v1/shopping/{weekStart}/send-chat` (CSRF-checked) takes
`{}` or `{"resend": true}` and posts the Supply run to Google Chat `{}` or `{"resend": true}` and posts the Supply run to Google Chat
through `chat.PostSupplyRun`, in the week's thread: "Supply run · " and through `chat.PostSupplyRun`, in the week's thread: "Supply run · " and
+10 -5
View File
@@ -111,10 +111,13 @@ type pushedPlan struct {
// plan Calendar does not show and pushing again makes it; or // plan Calendar does not show and pushing again makes it; or
// supply_run_failed when it could not be made, and POST // supply_run_failed when it could not be made, and POST
// /shopping/{weekStart}/generate makes it again. error is null when status // /shopping/{weekStart}/generate makes it again. error is null when status
// is generated. // is generated. title_job_id is the job titling a generated Supply run,
// which GET /jobs/{id} reports on; null when it already has its title or
// was not generated.
type pushedSupplyRun struct { type pushedSupplyRun struct {
Status string `json:"status"` Status string `json:"status"`
Error *apiError `json:"error"` Error *apiError `json:"error"`
TitleJobID *string `json:"title_job_id"`
} }
// postPlanPush pushes the week's plan to the chosen Google calendar // postPlanPush pushes the week's plan to the chosen Google calendar
@@ -143,7 +146,9 @@ func postPlanPush(logger *slog.Logger, svc *plan.Service, shop *shopping.Service
// gone. // gone.
ctx := context.WithoutCancel(r.Context()) ctx := context.WithoutCancel(r.Context())
run := pushedSupplyRun{Status: "generated"} run := pushedSupplyRun{Status: "generated"}
if _, err := shop.GeneratePushed(ctx, week, revision); errors.Is(err, shopping.ErrFrozen) { if g, err := shop.GeneratePushed(ctx, week, revision); err == nil {
run.TitleJobID = g.TitleJobID
} else if errors.Is(err, shopping.ErrFrozen) {
logger.InfoContext(ctx, "Supply run in the Supply archive left as it was after Push to Calendar", "week", week.String()) logger.InfoContext(ctx, "Supply run in the Supply archive left as it was after Push to Calendar", "week", week.String())
run = pushedSupplyRun{Status: "archived", Error: &apiError{Code: "archived", Message: err.Error()}} run = pushedSupplyRun{Status: "archived", Error: &apiError{Code: "archived", Message: err.Error()}}
} else if errors.Is(err, shopping.ErrPlanChanged) { } else if errors.Is(err, shopping.ErrPlanChanged) {
@@ -153,7 +158,7 @@ func postPlanPush(logger *slog.Logger, svc *plan.Service, shop *shopping.Service
Message: "the plan is on Calendar as pushed, but it was edited before its Supply run was made, so no Supply run was made from it; push again", Message: "the plan is on Calendar as pushed, but it was edited before its Supply run was made, so no Supply run was made from it; push again",
Retryable: true, Retryable: true,
}} }}
} else if err != nil { } else {
logger.ErrorContext(ctx, "generate Supply run after Push to Calendar", "week", week.String(), "error", err) logger.ErrorContext(ctx, "generate Supply run after Push to Calendar", "week", week.String(), "error", err)
run = pushedSupplyRun{Status: "failed", Error: &apiError{ run = pushedSupplyRun{Status: "failed", Error: &apiError{
Code: "supply_run_failed", Code: "supply_run_failed",
+7 -4
View File
@@ -60,10 +60,13 @@ type supplyRunItem struct {
} }
// generatedSupplyRun is the POST /shopping/{weekStart}/generate answer: // generatedSupplyRun is the POST /shopping/{weekStart}/generate answer:
// the Supply run as made, and the plan days with nothing to shop for. // the Supply run as made, the plan days with nothing to shop for, and the
// title_job_id of the job titling it, which GET /jobs/{id} reports on;
// null when the Supply run already has its title.
type generatedSupplyRun struct { type generatedSupplyRun struct {
supplyRun supplyRun
Skipped []shopping.SkippedDay `json:"skipped"` Skipped []shopping.SkippedDay `json:"skipped"`
TitleJobID *string `json:"title_job_id"`
} }
// sendChatRequest is the POST /shopping/{weekStart}/send-chat body: // sendChatRequest is the POST /shopping/{weekStart}/send-chat body:
@@ -108,7 +111,7 @@ func getSupplyRun(logger *slog.Logger, svc *shopping.Service) http.HandlerFunc {
} }
// postSupplyRunGenerate makes, or remakes, the week's Supply run from its // postSupplyRunGenerate makes, or remakes, the week's Supply run from its
// plan and answers with it and the plan days it skipped. // plan and answers with it, the plan days it skipped and its title job.
func postSupplyRunGenerate(logger *slog.Logger, svc *shopping.Service) http.HandlerFunc { func postSupplyRunGenerate(logger *slog.Logger, svc *shopping.Service) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
week, ok := weekStartOrFail(w, r, logger) week, ok := weekStartOrFail(w, r, logger)
@@ -129,7 +132,7 @@ func postSupplyRunGenerate(logger *slog.Logger, svc *shopping.Service) http.Hand
if skipped == nil { if skipped == nil {
skipped = []shopping.SkippedDay{} skipped = []shopping.SkippedDay{}
} }
writeJSON(w, r, logger, http.StatusOK, generatedSupplyRun{supplyRun: out, Skipped: skipped}) writeJSON(w, r, logger, http.StatusOK, generatedSupplyRun{supplyRun: out, Skipped: skipped, TitleJobID: g.TitleJobID})
} }
} }
+72 -10
View File
@@ -186,7 +186,7 @@ func TestSupplyRunShape(t *testing.T) {
if status != http.StatusOK { if status != http.StatusOK {
t.Fatalf("generate = %d %v", status, raw) t.Fatalf("generate = %d %v", status, raw)
} }
wantTop := []string{"chat_send_unconfirmed_at", "groups", "label", "needed_count", "sent_to_chat_at", "skipped", "status", "title", "total_count", "weekStart"} wantTop := []string{"chat_send_unconfirmed_at", "groups", "label", "needed_count", "sent_to_chat_at", "skipped", "status", "title", "title_job_id", "total_count", "weekStart"}
if got := keys(raw); !reflect.DeepEqual(got, wantTop) { if got := keys(raw); !reflect.DeepEqual(got, wantTop) {
t.Errorf("generate fields = %v, want %v", got, wantTop) t.Errorf("generate fields = %v, want %v", got, wantTop)
} }
@@ -199,7 +199,7 @@ func TestSupplyRunShape(t *testing.T) {
if status != http.StatusOK { if status != http.StatusOK {
t.Fatalf("GET = %d %v", status, raw) t.Fatalf("GET = %d %v", status, raw)
} }
if k := keys(raw); !reflect.DeepEqual(k, slices.DeleteFunc(slices.Clone(wantTop), func(s string) bool { return s == "skipped" })) { if k := keys(raw); !reflect.DeepEqual(k, slices.DeleteFunc(slices.Clone(wantTop), func(s string) bool { return s == "skipped" || s == "title_job_id" })) {
t.Errorf("GET fields = %v", k) t.Errorf("GET fields = %v", k)
} }
group := raw["groups"].([]any)[0].(map[string]any) group := raw["groups"].([]any)[0].(map[string]any)
@@ -376,8 +376,12 @@ func TestPushPlanMakesSupplyRun(t *testing.T) {
if rec.Code != http.StatusOK || body["status"] != "synced" { if rec.Code != http.StatusOK || body["status"] != "synced" {
t.Fatalf("push = %d %s", rec.Code, rec.Body) t.Fatalf("push = %d %s", rec.Code, rec.Body)
} }
if want := map[string]any{"status": "generated", "error": nil}; !reflect.DeepEqual(body["supply_run"], want) { run, _ := body["supply_run"].(map[string]any)
t.Errorf("supply_run = %v, want %v", body["supply_run"], want) if k := keys(run); run["status"] != "generated" || run["error"] != nil || !reflect.DeepEqual(k, []string{"error", "status", "title_job_id"}) {
t.Errorf("supply_run = %v, want generated with no error", run)
}
if job := titleJob(t, e, run["title_job_id"]); job["status"] != "queued" {
t.Errorf("title job = %v, want queued: no runner in this test", job)
} }
var l supplyRunJSON var l supplyRunJSON
if status, got := callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l); status != http.StatusOK { if status, got := callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l); status != http.StatusOK {
@@ -467,9 +471,11 @@ func TestPushPlanDoesNotWaitForTitle(t *testing.T) {
if rec.Code != http.StatusOK || body["status"] != "synced" { if rec.Code != http.StatusOK || body["status"] != "synced" {
t.Fatalf("push = %d %s", rec.Code, rec.Body) t.Fatalf("push = %d %s", rec.Code, rec.Body)
} }
if want := map[string]any{"status": "generated", "error": nil}; !reflect.DeepEqual(body["supply_run"], want) { run, _ := body["supply_run"].(map[string]any)
t.Errorf("supply_run = %v, want %v", body["supply_run"], want) if run["status"] != "generated" || run["error"] != nil {
t.Errorf("supply_run = %v, want generated with no error", run)
} }
titleJobID, _ := run["title_job_id"].(string)
held.waitEntered(t) held.waitEntered(t)
var l supplyRunJSON var l supplyRunJSON
callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l) callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l)
@@ -478,6 +484,7 @@ func TestPushPlanDoesNotWaitForTitle(t *testing.T) {
} }
close(held.release) close(held.release)
wantTitleJobSucceeded(t, e, titleJobID)
if got := e.waitTitled(t); got != "Chili and Friends" { if got := e.waitTitled(t); got != "Chili and Friends" {
t.Errorf("title = %q, want the title job's", got) t.Errorf("title = %q, want the title job's", got)
} }
@@ -493,7 +500,8 @@ func TestSupplyRunGenerateDoesNotWaitForTitle(t *testing.T) {
e.startRunner(t) e.startRunner(t)
var l supplyRunJSON var l supplyRunJSON
if status, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", &l); status != http.StatusOK { status, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", &l)
if status != http.StatusOK {
t.Fatalf("generate = %d %v", status, body) t.Fatalf("generate = %d %v", status, body)
} }
if l.Title != nil { if l.Title != nil {
@@ -502,12 +510,66 @@ func TestSupplyRunGenerateDoesNotWaitForTitle(t *testing.T) {
if !reflect.DeepEqual(l.render(), weekLines) { if !reflect.DeepEqual(l.render(), weekLines) {
t.Errorf("generated = %v, want %v", l.render(), weekLines) t.Errorf("generated = %v, want %v", l.render(), weekLines)
} }
titleJobID, _ := body["title_job_id"].(string)
held.waitEntered(t) held.waitEntered(t)
close(held.release) close(held.release)
wantTitleJobSucceeded(t, e, titleJobID)
if got := e.waitTitled(t); got != "Chili and Friends" { if got := e.waitTitled(t); got != "Chili and Friends" {
t.Errorf("title = %q, want the title job's", got) t.Errorf("title = %q, want the title job's", got)
} }
if _, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", nil); body["title_job_id"] != nil {
t.Errorf("remake title_job_id = %v, want null: the Supply run has its title", body["title_job_id"])
}
}
// TestSupplyRunGenerateTitleJobFails: a title job that cannot title the
// Supply run is reported failed with title_failed by GET /jobs/{id}, and
// the week's next generation answers with a new title job.
func TestSupplyRunGenerateTitleJobFails(t *testing.T) {
t.Parallel()
e := newPlansRouter(t)
e.groundWeek(t)
e.startRunner(t)
_, body := callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", nil)
first := titleJob(t, e, body["title_job_id"])
job := waitJob(t, e.h, first["id"].(string))
if job["status"] != "failed" || job["error"] != shopping.ReasonTitleFailed {
t.Errorf("title job = %v, want failed %s", job, shopping.ReasonTitleFailed)
}
_, body = callShopping(t, e, http.MethodPost, shoppingPath+"2026-10-12/generate", "", nil)
if second := titleJob(t, e, body["title_job_id"]); second["id"] == first["id"] {
t.Errorf("retry title_job_id = %v, want a new job", second["id"])
}
}
// titleJob returns the title_supply_run job id, as GET /jobs/{id} reports
// it; id must be a job id string.
func titleJob(t *testing.T, e plansEnv, id any) map[string]any {
t.Helper()
s, ok := id.(string)
if !ok || s == "" {
t.Fatalf("title_job_id = %v, want a job id", id)
}
rec, job := callDinners(t, e.h, http.MethodGet, "/api/v1/jobs/"+s, "")
if rec.Code != http.StatusOK || job["kind"] != shopping.KindTitleSupplyRun {
t.Fatalf("GET title job = %d %s, want a %s job", rec.Code, rec.Body, shopping.KindTitleSupplyRun)
}
return job
}
// wantTitleJobSucceeded waits for title job id to finish and fails t
// unless GET /jobs/{id} reports it succeeded with the title written.
func wantTitleJobSucceeded(t *testing.T, e plansEnv, id string) {
t.Helper()
titleJob(t, e, id)
job := waitJob(t, e.h, id)
result, _ := job["result"].(map[string]any)
if job["status"] != "succeeded" || result["title"] != "Chili and Friends" {
t.Errorf("title job = %v, want succeeded with the title", job)
}
} }
// TestFailedPushMakesNoSupplyRun: a push Calendar refused makes no Supply // TestFailedPushMakesNoSupplyRun: a push Calendar refused makes no Supply
@@ -546,7 +608,7 @@ func TestPushPlanSupplyRunFails(t *testing.T) {
"code": "supply_run_failed", "code": "supply_run_failed",
"message": "the plan is on Calendar, but its Supply run could not be made; make it again from the Supply run", "message": "the plan is on Calendar, but its Supply run could not be made; make it again from the Supply run",
"retryable": true, "retryable": true,
}} }, "title_job_id": nil}
if !reflect.DeepEqual(body["supply_run"], want) { if !reflect.DeepEqual(body["supply_run"], want) {
t.Errorf("supply_run = %v, want %v", body["supply_run"], want) t.Errorf("supply_run = %v, want %v", body["supply_run"], want)
} }
@@ -599,7 +661,7 @@ func TestPushPlanEditedBeforeSupplyRun(t *testing.T) {
"code": "plan_changed", "code": "plan_changed",
"message": "the plan is on Calendar as pushed, but it was edited before its Supply run was made, so no Supply run was made from it; push again", "message": "the plan is on Calendar as pushed, but it was edited before its Supply run was made, so no Supply run was made from it; push again",
"retryable": true, "retryable": true,
}} }, "title_job_id": nil}
if !reflect.DeepEqual(body["supply_run"], want) { if !reflect.DeepEqual(body["supply_run"], want) {
t.Errorf("supply_run = %v, want %v", body["supply_run"], want) t.Errorf("supply_run = %v, want %v", body["supply_run"], want)
} }
@@ -632,7 +694,7 @@ func TestPushPlanLeavesArchivedSupplyRun(t *testing.T) {
t.Fatalf("push = %d %s", rec.Code, rec.Body) t.Fatalf("push = %d %s", rec.Code, rec.Body)
} }
run, _ := body["supply_run"].(map[string]any) run, _ := body["supply_run"].(map[string]any)
if apiErr, _ := run["error"].(map[string]any); run["status"] != "archived" || apiErr["code"] != "archived" { if apiErr, _ := run["error"].(map[string]any); run["status"] != "archived" || apiErr["code"] != "archived" || run["title_job_id"] != nil {
t.Errorf("supply_run = %v, want archived", body["supply_run"]) t.Errorf("supply_run = %v, want archived", body["supply_run"])
} }
var l supplyRunJSON var l supplyRunJSON
+35
View File
@@ -311,3 +311,38 @@ func TestGenerateTitleOverlapAcrossRename(t *testing.T) {
t.Errorf("stored title = %v, want one of the two titles", deref(got)) t.Errorf("stored title = %v, want one of the two titles", deref(got))
} }
} }
// TestGenerateTitleNotStoredOnReplacedList: a list replaced by a new
// untitled list of the same week once archive_title has answered, before
// its title is stored, keeps the replacement untitled; the old list's
// title job fails, and the failure is logged.
func TestGenerateTitleNotStoredOnReplacedList(t *testing.T) {
e := titleWeek(t)
e.model.QueueGenerate(titled("Noodles, Tacos and Chili"))
e.afterTitle = func() {
for _, stmt := range []string{
`DELETE FROM shopping_items WHERE group_id IN (SELECT id FROM shopping_groups WHERE list_id IN (SELECT id FROM shopping_lists WHERE week_start = ?1))`,
`DELETE FROM shopping_groups WHERE list_id IN (SELECT id FROM shopping_lists WHERE week_start = ?1)`,
`UPDATE shopping_lists SET id = 'replacement' WHERE week_start = ?1`,
} {
if _, err := e.st.Writer.Exec(stmt, week.String()); err != nil {
t.Error(err)
}
}
}
g := e.generate(t)
wantJob(t, e.waitTitle(t, g), jobs.StatusFailed, shopping.ReasonTitleFailed)
var id string
if err := e.st.Reader.QueryRow(`SELECT id FROM shopping_lists WHERE week_start = ?`, week.String()).Scan(&id); err != nil {
t.Fatal(err)
}
if id != "replacement" || id == g.List.ID {
t.Fatalf("week's list = %s, want the replacement of %s", id, g.List.ID)
}
wantUntitled(t, "replacement's title", e.storedTitle(t))
if !strings.Contains(e.log.String(), "the Supply run was frozen or replaced before its title was stored") {
t.Errorf("log lacks the failure:\n%s", e.log.String())
}
}