Merge the feature branch into task/supply-archive-unfrozen-run (Maestro 29b629aa refresh)

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

internal/server/dinnerdetail_test.go

@@ -3,6 +3,7 @@ package server
 import (
 	"database/sql"
 	"encoding/json"
+	"io/fs"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -14,6 +15,7 @@ import (
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest"
 	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store"
+	"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations"
 )
 
 // getDetail answers GET /dinners/{id} for id, failing unless it is 200, and
@@ -70,8 +72,8 @@ func TestGetDinnerGroundedSideQuest(t *testing.T) {
 	}
 	for _, stmt := range []string{
 		`UPDATE dinners SET status = 'quest_log', origin = 'side_quest', is_favorite = 1 WHERE id = ?`,
-		`UPDATE recipe_ingredients SET position = 1 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`,
-		`UPDATE recipe_steps SET position = 1 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`,
+		`UPDATE recipe_ingredients SET position = 9 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`,
+		`UPDATE recipe_steps SET position = 9 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`,
 	} {
 		if _, err := e.st.Writer.Exec(stmt, id); err != nil {
 			t.Fatal(err)
@@ -328,14 +330,33 @@ func TestGetDinnerSourceURLOnlyWhenSafe(t *testing.T) {
 	}
 }
 
-// TestGetDinnerSiteNameNeverFromLink: a page that names no site is stored
-// under its host, so a recipe grounded in a pasted link whose host carries a
-// token has site_name null and nothing from the link in its detail; a page
-// that names its site keeps that name, even on a token-bearing host; and a
-// stored name shaped like another host, as a redirect leaves, is withheld.
-// The links carry a query, so source.url is withheld too and nothing of the
-// link may appear.
-func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
+// siteNameMigration is migration 0020, which clears the host-shaped site
+// names stored before extraction stopped storing hosts.
+const siteNameMigration = "0020_site_name_from_page.sql"
+
+// applySiteNameMigration runs siteNameMigration again over rows a test
+// stored as the code before it did, as upgrading a database holding them
+// does.
+func applySiteNameMigration(t *testing.T, st *store.Store) {
+	t.Helper()
+	body, err := fs.ReadFile(migrations.FS, siteNameMigration)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := st.Writer.ExecContext(t.Context(), string(body)); err != nil {
+		t.Fatalf("%s: %v", siteNameMigration, err)
+	}
+}
+
+// TestGetDinnerSiteNameProvenance: extraction stores only a site name the
+// page gave, never the URL's host, and the detail applies one rule to what
+// is stored. A name from page metadata is shown even over a token-bearing
+// link; a page naming no site stores no name, and is named by its link's
+// host only when the link is shown — so over a link with a token nothing
+// of the link reaches the client. A name stored before extraction stopped
+// storing hosts is read by the same rule once migration 0020 has cleared
+// it if it could be a host.
+func TestGetDinnerSiteNameProvenance(t *testing.T) {
 	t.Parallel()
 	e := newDinnersEnv(t)
 	const (
@@ -343,11 +364,14 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
 		namedToken = "named-token-def456"
 		bare       = "https://" + bareToken + ".example.com/recipe?print=1"
 		named      = "https://www." + namedToken + ".example.com/recipe?print=1"
+		ordinary   = "https://www.kitchen.example/recipes/curry"
 	)
 	e.pages.Bodies[bare] = recipestest.RecipePage("Chicken Curry", "", "https://a.example/curry.jpg", 40)
 	e.pages.Bodies[named] = recipestest.RecipePage("Weeknight Curry", "Curry House", "https://b.example/curry.jpg", 25)
+	e.pages.Bodies[ordinary] = recipestest.RecipePage("Green Curry", "", "https://c.example/curry.jpg", 30)
 	e.images.JPEGs["https://a.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 1)
 	e.images.JPEGs["https://b.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 2)
+	e.images.JPEGs["https://c.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 3)
 	ground := func(dish, page string) (id, recipeID string) {
 		t.Helper()
 		id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending
@@ -360,15 +384,18 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
 		}
 		return id, recipeID
 	}
-	storedSite := func(recipeID string) string {
+	storedSite := func(recipeID string) any {
 		t.Helper()
-		var site string
+		var site sql.NullString
 		if err := e.st.Reader.QueryRow(`SELECT site_name FROM recipes WHERE id = ?`, recipeID).Scan(&site); err != nil {
 			t.Fatal(err)
 		}
-		return site
+		if !site.Valid {
+			return nil
+		}
+		return site.String
 	}
-	check := func(what, id string, leaks []string, want any) {
+	check := func(what, id string, leaks []string, want map[string]any) {
 		t.Helper()
 		rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "")
 		if rec.Code != http.StatusOK {
@@ -380,28 +407,48 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
 			}
 		}
 		raw, _ := getDetail(t, e.h, id)
-		if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": want, "url": nil}) {
-			t.Errorf("source of %s = %v, want site_name %v and url null", what, raw["source"], want)
+		if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, want) {
+			t.Errorf("source of %s = %v, want %v", what, raw["source"], want)
 		}
 	}
 
-	bareID, bareRecipe := ground("Chicken curry", bare)
-	if got := storedSite(bareRecipe); got != bareToken+".example.com" {
-		t.Fatalf("stored site_name = %q, want the link's host", got)
-	}
-	check("a page naming no site", bareID, []string{bare, bareToken, "example.com"}, nil)
-
 	namedID, namedRecipe := ground("Weeknight curry", named)
 	if got := storedSite(namedRecipe); got != "Curry House" {
-		t.Fatalf("stored site_name = %q, want the page's", got)
+		t.Fatalf("stored site_name = %v, want the page's", got)
 	}
-	check("a page naming its site", namedID, []string{named, namedToken, "example.com"}, "Curry House")
+	check("a metadata-derived name over a token-bearing link", namedID, []string{named, namedToken, "example.com"},
+		map[string]any{"site_name": "Curry House", "url": nil})
 
-	const redirectHost = "redirect-token-ghi789.example.net"
-	if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, redirectHost, namedRecipe); err != nil {
-		t.Fatal(err)
+	bareID, bareRecipe := ground("Chicken curry", bare)
+	if got := storedSite(bareRecipe); got != nil {
+		t.Fatalf("stored site_name = %v, want null: the page names no site", got)
 	}
-	check("a page named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken}, nil)
+	check("a hostname-derived name over a token-bearing link", bareID, []string{bare, bareToken, "example.com"},
+		map[string]any{"site_name": nil, "url": nil})
+
+	ordinaryID, ordinaryRecipe := ground("Green curry", ordinary)
+	if got := storedSite(ordinaryRecipe); got != nil {
+		t.Fatalf("stored site_name = %v, want null: the page names no site", got)
+	}
+	check("a hostname-derived name over an ordinary link", ordinaryID, nil,
+		map[string]any{"site_name": "kitchen.example", "url": ordinary})
+
+	// Rows as extraction stored them before it stopped storing hosts: the
+	// token-bearing link's own host, and a redirect's host under a page
+	// that named its site, are cleared by 0020; a page's own name stays.
+	const redirectHost = "redirect-token-ghi789.example.net"
+	for recipeID, site := range map[string]string{bareRecipe: bareToken + ".example.com", namedRecipe: redirectHost, ordinaryRecipe: "Green Kitchen"} {
+		if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, site, recipeID); err != nil {
+			t.Fatal(err)
+		}
+	}
+	applySiteNameMigration(t, e.st)
+	check("a row stored before, named by its token-bearing link's host", bareID, []string{bare, bareToken, "example.com"},
+		map[string]any{"site_name": nil, "url": nil})
+	check("a row stored before, named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken},
+		map[string]any{"site_name": nil, "url": nil})
+	check("a row stored before, named by its page", ordinaryID, nil,
+		map[string]any{"site_name": "Green Kitchen", "url": ordinary})
 }
 
 // TestGetDinnerWhyPickedIsCreatingSuggestion: why_picked is the reason of
@@ -445,25 +492,3 @@ func TestGetDinnerWhyPickedIsCreatingSuggestion(t *testing.T) {
 		t.Errorf("why_picked = %v, want %q", got.WhyPicked, reason)
 	}
 }
-
-// TestGetDinnerDuplicatePositionFails: a recipe with two ingredients or two
-// steps at one position has no order anyone can trust, so its detail is a
-// 500 rather than the recipe in some order.
-func TestGetDinnerDuplicatePositionFails(t *testing.T) {
-	t.Parallel()
-	e := newDinnersEnv(t)
-	for _, table := range []string{"recipe_ingredients", "recipe_steps"} {
-		id := createDinner(t, e.h, `{"name":"Family chili `+table+`"}`)
-		if rec, _ := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground",
-			`{"manual":{"ingredients":["1 lb beef","1 can beans"],"steps":["Brown.","Simmer."],"minutes":240,"method":"crockpot"}}`); rec.Code != http.StatusOK {
-			t.Fatalf("POST ground = %d: %s", rec.Code, rec.Body)
-		}
-		if _, err := e.st.Writer.Exec(`UPDATE `+table+` SET position = 0 WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`, id); err != nil {
-			t.Fatal(err)
-		}
-		rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "")
-		if rec.Code != http.StatusInternalServerError || strings.Contains(rec.Body.String(), "Simmer.") || strings.Contains(rec.Body.String(), "1 lb beef") {
-			t.Errorf("GET with duplicate %s positions = %d: %s, want 500 and no recipe", table, rec.Code, rec.Body)
-		}
-	}
-}

internal/server/dinners.go

@@ -72,11 +72,11 @@ func getDinners(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 
 // getDinner answers GET /dinners/{id} with the dinner, its recipe and the
 // reason it was suggested. An archived dinner is 404, like an unknown one,
-// not a 200 with its status: it has no detail to show. source never has the
-// recipe page's link, and source.site_name is null unless the page itself
-// named its site — a name that could be a host, the link's or a redirect's,
-// is withheld (dinners.Source) — so the detail screen shows "Recipe from
-// {site}" only when site_name is not null.
+// not a 200 with its status: it has no detail to show. source is as
+// dinners.SourceOf gives it: the recipe page's link only when it is free of
+// secrets, and site_name the name the page gave its site, else the shown
+// link's host, else null — so the detail screen shows "Recipe from {site}"
+// only when site_name is not null.
 func getDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		d, err := svc.Get(r.Context(), chi.URLParam(r, "id"))
@@ -134,7 +134,8 @@ func patchDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 }
 
 // getDinnerCandidates answers GET /dinners/{id}/recipe-candidates with up
-// to dinners.MaxCandidates recipe pages for the dinner's name.
+// to dinners.MaxCandidates recipe pages for the dinner's name, each
+// site_name as dinners.SourceOf gives it.
 func getDinnerCandidates(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		list, err := svc.Candidates(r.Context(), chi.URLParam(r, "id"))

internal/server/grounding_test.go

@@ -184,6 +184,51 @@ func TestGetRecipeCandidates(t *testing.T) {
 	}
 }
 
+// TestGetRecipeCandidatesSiteNames: a candidate's site_name follows the
+// recipe detail's rule (dinners.SourceOf), not a hostname fallback of its
+// own: a name the page gave is offered as it stands, even over a link with
+// a token; a page naming no site is named by its link's host only when
+// the link is free of secrets, and is null over a token-bearing link, so
+// nothing of that link reaches the client.
+func TestGetRecipeCandidatesSiteNames(t *testing.T) {
+	t.Parallel()
+	e := newDinnersEnv(t)
+	const (
+		token    = "offer-token-abc123"
+		named    = "https://named.example/curry?access_token=" + token
+		bare     = "https://" + token + ".example.com/curry?print=1"
+		ordinary = "https://www.kitchen.example/recipes/curry"
+	)
+	e.search.Results["Chicken curry"] = []recipes.Candidate{{URL: named}, {URL: bare}, {URL: ordinary}}
+	e.pages.Bodies[named] = recipestest.RecipePage("Named Curry", "Curry House", "https://a.example/curry.jpg", 40)
+	e.pages.Bodies[bare] = recipestest.RecipePage("Bare Curry", "", "https://b.example/curry.jpg", 30)
+	e.pages.Bodies[ordinary] = recipestest.RecipePage("Ordinary Curry", "", "https://c.example/curry.jpg", 25)
+	id := createDinner(t, e.h, `{"name":"Chicken curry"}`) // runner not started: stays pending
+
+	rec, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "")
+	if rec.Code != http.StatusOK {
+		t.Fatalf("GET = %d: %s", rec.Code, rec.Body)
+	}
+	for _, leak := range []string{token, "access_token", "example.com"} {
+		if strings.Contains(rec.Body.String(), leak) {
+			t.Errorf("candidates contain %q: %s", leak, rec.Body)
+		}
+	}
+	want := map[string]any{"Named Curry": "Curry House", "Bare Curry": nil, "Ordinary Curry": "kitchen.example"}
+	list, _ := body["candidates"].([]any)
+	if len(list) != len(want) {
+		t.Fatalf("candidates = %v, want %d", body, len(want))
+	}
+	for _, c := range list {
+		c := c.(map[string]any)
+		title, _ := c["title"].(string)
+		site, ok := c["site_name"]
+		if w, known := want[title]; !known || !ok || site != w {
+			t.Errorf("candidate %q site_name = %#v (present %v), want %#v", title, site, ok, w)
+		}
+	}
+}
+
 func TestGetRecipeCandidatesSearchFails(t *testing.T) {
 	t.Parallel()
 	e := newDinnersEnv(t)

internal/server/ideasource_test.go

@@ -16,7 +16,7 @@ import (
 )
 
 // sourceCase is a recipe page an idea grounds in: its link and the site
-// name its page gives (empty for none, so the host is stored instead).
+// name its page gives (empty for none, so no name is stored).
 type sourceCase struct {
 	dish, link, site string
 	// want is the idea's source_url and site_name; nil is null.
@@ -33,7 +33,7 @@ const (
 
 // sourceCases are a clean link, credited by its page's name, and links
 // carrying a token in the query and the userinfo whose pages name no site,
-// so the stored name is the link's host.
+// so no name is stored and, the links being unsafe, none is shown.
 var sourceCases = []sourceCase{
 	{"Chicken Curry", cleanSourceLink, "Curry House", cleanSourceLink, "Curry House"},
 	{"Beef Stew", "https://" + queryHost + "/stew?access_token=" + queryToken, "", nil, nil},
@@ -137,8 +137,9 @@ func legacyIdea(name, link, site string) map[string]any {
 }
 
 // A job of either idea kind whose result was stored before sources were
-// made safe is served with the sources a job stored now holds: neither a
-// token-bearing link nor a name taken from its host.
+// made safe, and upgraded by migration 0020, is served with the sources a
+// job stored now holds: neither a token-bearing link nor a name taken from
+// its host.
 func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
 	t.Parallel()
 	model, normalize, search, pages, images := newQuickFakes()
@@ -162,7 +163,10 @@ func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
 			id, kind, string(js)); err != nil {
 			t.Fatal(err)
 		}
-		checkIdeaSources(t, h, id)
+	}
+	applySiteNameMigration(t, st)
+	for kind := range map[string]bool{ideas.KindFromIngredients: true, ideas.KindQuick: true} {
+		checkIdeaSources(t, h, "legacy-"+kind)
 	}
 }
 

internal/server/plans_test.go

@@ -108,7 +108,7 @@ func newPlansRouterTitled(t *testing.T, title shopping.Titler) plansEnv {
 		t.Fatal(err)
 	}
 	h, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), chatSvc,
-		newTestDinners(t), newTestSuggest(t), svc, shop, googleSvc, newTestPush(t), newTestIdeas(t), newTestHome(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}})
+		newTestDinnersOn(t, st), newTestSuggest(t), svc, shop, googleSvc, newTestPush(t), newTestIdeas(t), newTestHome(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}})
 	if err != nil {
 		t.Fatal(err)
 	}

internal/server/server_test.go

@@ -168,7 +168,12 @@ func newTestSearch(t *testing.T) *recipes.SearchSettingsService {
 // with its own unstarted job runner and a grounder that finds nothing.
 func newTestDinners(t *testing.T) *dinners.Service {
 	t.Helper()
-	st := newTestStore(t)
+	return newTestDinnersOn(t, newTestStore(t))
+}
+
+// newTestDinnersOn is newTestDinners over st.
+func newTestDinnersOn(t *testing.T, st *store.Store) *dinners.Service {
+	t.Helper()
 	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
 	runner, err := jobs.NewRunner(st, logger, 1)
 	if err != nil {

internal/server/shopping_test.go

@@ -75,7 +75,7 @@ func (e plansEnv) groundWeek(t *testing.T) {
 func (e plansEnv) archive(t *testing.T) {
 	t.Helper()
 	if _, err := e.st.Writer.ExecContext(t.Context(),
-		`UPDATE shopping_lists SET status = 'archived', frozen_at = '2026-10-19T04:05:00Z'`); err != nil {
+		`UPDATE shopping_lists SET status = 'archived', frozen_at = '2026-10-19T04:05:00Z'; UPDATE shopping_groups SET frozen_dinner_name = (SELECT name FROM dinners WHERE dinners.id = dinner_id) WHERE list_id IN (SELECT id FROM shopping_lists WHERE frozen_at IS NOT NULL)`); err != nil {
 		t.Fatal(err)
 	}
 }
@@ -917,6 +917,73 @@ func TestSupplyArchiveUnfrozenPastWeek(t *testing.T) {
 	}
 }
 
+// TestSupplyArchiveKeepsFrozenNames freezes last week's Supply run, then
+// renames its dinner and this week's through the dinners API: the frozen
+// post and list keep the name the dinner had when frozen, while this
+// week's list, not frozen, shows the new name.
+func TestSupplyArchiveKeepsFrozenNames(t *testing.T) {
+	t.Parallel()
+	e := newPlansRouter(t)
+	if _, err := e.st.Writer.ExecContext(t.Context(), `DELETE FROM plan_days; DELETE FROM week_plans`); err != nil {
+		t.Fatal(err)
+	}
+	zone, err := household.LoadZone("America/New_York") // the env's household zone
+	if err != nil {
+		t.Fatal(err)
+	}
+	thisWeek := household.WeekStart(household.Today(time.Now(), zone))
+	y, m, d := thisWeek.Date()
+	lastWeek := store.LocalDateOf(time.Date(y, m, d-7, 0, 0, 0, 0, time.UTC))
+
+	last := e.weekSupplyRun(t, lastWeek, "Beef Stew", shopIng{"stewing beef", "2", "lb"})
+	current := e.weekSupplyRun(t, thisWeek, "Lemon Chicken", shopIng{"lemon", "1", ""})
+	if err := e.shop.Freeze(t.Context(), lastWeek); err != nil {
+		t.Fatal(err)
+	}
+	for id, name := range map[string]string{
+		last.Groups[0].Dinner.ID:    "Sunday Beef Stew",
+		current.Groups[0].Dinner.ID: "Lemon Garlic Chicken",
+	} {
+		if status, body := callShopping(t, e, http.MethodPatch, dinnersPath+"/"+id, `{"name":"`+name+`"}`, nil); status != http.StatusOK {
+			t.Fatalf("PATCH dinner %s = %d %v", id, status, body)
+		}
+	}
+
+	var got archiveJSON
+	if status, raw := callShopping(t, e, http.MethodGet, shoppingPath+"archive?by=week", "", &got); status != http.StatusOK {
+		t.Fatalf("GET archive = %d %v", status, raw)
+	}
+	var posts []string
+	for _, p := range got.Periods {
+		for _, post := range p.Posts {
+			posts = append(posts, fmt.Sprintf("%s %v current=%t", post.WeekStart, post.Dinners, post.Current))
+		}
+	}
+	want := []string{
+		thisWeek.String() + " [Lemon Garlic Chicken] current=true",
+		lastWeek.String() + " [Beef Stew] current=false",
+	}
+	if !reflect.DeepEqual(posts, want) {
+		t.Errorf("archive after rename =\n  %s\nwant\n  %s", strings.Join(posts, "\n  "), strings.Join(want, "\n  "))
+	}
+
+	for _, tc := range []struct {
+		week store.LocalDate
+		want string
+	}{
+		{lastWeek, "Monday " + lastWeek.String() + " Beef Stew: stewing beef [2 lb] needed"},
+		{thisWeek, "Monday " + thisWeek.String() + " Lemon Garlic Chicken: lemon [1] needed"},
+	} {
+		var l supplyRunJSON
+		if status, body := callShopping(t, e, http.MethodGet, shoppingPath+tc.week.String(), "", &l); status != http.StatusOK {
+			t.Fatalf("GET %s = %d %v", tc.week, status, body)
+		}
+		if got := l.render(); !reflect.DeepEqual(got, []string{tc.want}) {
+			t.Errorf("list %s after rename = %v, want [%s]", tc.week, got, tc.want)
+		}
+	}
+}
+
 // TestSupplyRunSendChat posts the week's Supply run to Google Chat once,
 // records sent_to_chat_at, answers a second send with the status as it
 // is, and posts again only with resend.