From 791b321b71530b43ac19873f23e80116b90d4c06 Mon Sep 17 00:00:00 2001 From: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com> Date: Sun, 11 Oct 2026 08:24:51 -0400 Subject: [PATCH] Expose the Supply run title job and cover the replacement race (#194) Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com> --- docs/ARCHITECTURE.md | 13 +++-- internal/server/plans.go | 15 ++++-- internal/server/shopping.go | 11 +++-- internal/server/shopping_test.go | 82 ++++++++++++++++++++++++++++---- internal/shopping/title_test.go | 35 ++++++++++++++ 5 files changed, 134 insertions(+), 22 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0472353..539863e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -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 a successful `Push`, the handler runs `shopping.GeneratePushed` for 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 left as it was; or `failed` — error `plan_changed` (retryable, logged 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`. - `POST /api/v1/shopping/{weekStart}/generate` makes or remakes the list through `shopping.Generate` and answers with the same shape plus - `skipped: [{date, dinner_id, dinner_name, reason}]`. 404 when the week - has no plan, 409 `archived` when its list is in the Supply archive. + `skipped: [{date, dinner_id, dinner_name, reason}]` and + `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 `{}` or `{"resend": true}` and posts the Supply run to Google Chat through `chat.PostSupplyRun`, in the week's thread: "Supply run · " and diff --git a/internal/server/plans.go b/internal/server/plans.go index 73940c2..66ac660 100644 --- a/internal/server/plans.go +++ b/internal/server/plans.go @@ -111,10 +111,13 @@ type pushedPlan struct { // plan Calendar does not show and pushing again makes it; or // supply_run_failed when it could not be made, and POST // /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 { - Status string `json:"status"` - Error *apiError `json:"error"` + Status string `json:"status"` + Error *apiError `json:"error"` + TitleJobID *string `json:"title_job_id"` } // 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. ctx := context.WithoutCancel(r.Context()) 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()) run = pushedSupplyRun{Status: "archived", Error: &apiError{Code: "archived", Message: err.Error()}} } 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", Retryable: true, }} - } else if err != nil { + } else { logger.ErrorContext(ctx, "generate Supply run after Push to Calendar", "week", week.String(), "error", err) run = pushedSupplyRun{Status: "failed", Error: &apiError{ Code: "supply_run_failed", diff --git a/internal/server/shopping.go b/internal/server/shopping.go index c5bb173..27cfd9e 100644 --- a/internal/server/shopping.go +++ b/internal/server/shopping.go @@ -60,10 +60,13 @@ type supplyRunItem struct { } // 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 { 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: @@ -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 -// 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 { return func(w http.ResponseWriter, r *http.Request) { week, ok := weekStartOrFail(w, r, logger) @@ -129,7 +132,7 @@ func postSupplyRunGenerate(logger *slog.Logger, svc *shopping.Service) http.Hand if skipped == nil { 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}) } } diff --git a/internal/server/shopping_test.go b/internal/server/shopping_test.go index 7128449..739d44a 100644 --- a/internal/server/shopping_test.go +++ b/internal/server/shopping_test.go @@ -186,7 +186,7 @@ func TestSupplyRunShape(t *testing.T) { if status != http.StatusOK { 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) { t.Errorf("generate fields = %v, want %v", got, wantTop) } @@ -199,7 +199,7 @@ func TestSupplyRunShape(t *testing.T) { if status != http.StatusOK { 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) } 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" { t.Fatalf("push = %d %s", rec.Code, rec.Body) } - if want := map[string]any{"status": "generated", "error": nil}; !reflect.DeepEqual(body["supply_run"], want) { - t.Errorf("supply_run = %v, want %v", body["supply_run"], want) + run, _ := body["supply_run"].(map[string]any) + 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 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" { t.Fatalf("push = %d %s", rec.Code, rec.Body) } - if want := map[string]any{"status": "generated", "error": nil}; !reflect.DeepEqual(body["supply_run"], want) { - t.Errorf("supply_run = %v, want %v", body["supply_run"], want) + run, _ := body["supply_run"].(map[string]any) + 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) var l supplyRunJSON callShopping(t, e, http.MethodGet, shoppingPath+"2026-10-12", "", &l) @@ -478,6 +484,7 @@ func TestPushPlanDoesNotWaitForTitle(t *testing.T) { } close(held.release) + wantTitleJobSucceeded(t, e, titleJobID) if got := e.waitTitled(t); got != "Chili and Friends" { t.Errorf("title = %q, want the title job's", got) } @@ -493,7 +500,8 @@ func TestSupplyRunGenerateDoesNotWaitForTitle(t *testing.T) { e.startRunner(t) 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) } if l.Title != nil { @@ -502,12 +510,66 @@ func TestSupplyRunGenerateDoesNotWaitForTitle(t *testing.T) { if !reflect.DeepEqual(l.render(), weekLines) { t.Errorf("generated = %v, want %v", l.render(), weekLines) } + titleJobID, _ := body["title_job_id"].(string) held.waitEntered(t) close(held.release) + wantTitleJobSucceeded(t, e, titleJobID) if got := e.waitTitled(t); got != "Chili and Friends" { 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 @@ -546,7 +608,7 @@ func TestPushPlanSupplyRunFails(t *testing.T) { "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", "retryable": true, - }} + }, "title_job_id": nil} if !reflect.DeepEqual(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", "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, - }} + }, "title_job_id": nil} if !reflect.DeepEqual(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) } 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"]) } var l supplyRunJSON diff --git a/internal/shopping/title_test.go b/internal/shopping/title_test.go index 00cf399..701bfa7 100644 --- a/internal/shopping/title_test.go +++ b/internal/shopping/title_test.go @@ -311,3 +311,38 @@ func TestGenerateTitleOverlapAcrossRename(t *testing.T) { 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()) + } +}