Compare commits

...
Author SHA1 Message Date
dev_agentandClaude Opus 5.5 3dda308f30 Document the pre-deploy duplicate position check for migration 0018 (Maestro 75bb8cc1)
CI / ci (pull_request) Successful in 8m11s
CI / ci (push) Successful in 8m10s
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 11:52:51 -04:00
dev_agent 161365fade Merge remote-tracking branch 'origin/feature/7383c2cb-3179-4b45-b558-bf1bc1833fad-launch-readiness' into task/recipe-item-positions_75bb8cc1-fd30-4050-90d2-a1627a865b22
CI / ci (push) Successful in 8m14s
CI / ci (pull_request) Successful in 8m13s
2026-10-11 10:35:27 -04:00
orchestrator-agent d182349646 Plan draft schedule tests: assert the plan_ready title and channel (Maestro ece60217) (#195)
CI / ci (push) Successful in 8m11s
Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>
2026-10-11 09:38:12 -04:00
orchestrator-agent 72755a8b61 Assert Home tonight dinner exact id and photo URL (Maestro 9d81c325) (#196)
CI / ci (push) Successful in 9m42s
Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>
2026-10-11 09:38:06 -04:00
dev_agentandClaude Opus 5.5 da4f048f85 Enforce unique recipe item positions per recipe
CI / ci (push) Successful in 9m17s
CI / ci (pull_request) Successful in 8m10s
Add migration 0018 with unique indexes on recipe_ingredients and
recipe_steps (recipe_id, position), so a recipe can never hold two
ingredients or two steps at one position. The dinner detail read now
orders by position alone and drops its read-time duplicate check; a
database already holding a duplicate fails the migration.

Maestro task 75bb8cc1-fd30-4050-90d2-a1627a865b22

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 08:50:11 -04:00
orchestrator-agent 791b321b71 Expose the Supply run title job and cover the replacement race (#194)
CI / ci (push) Successful in 8m9s
Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>
2026-10-11 08:24:51 -04:00
orchestrator-agent be41b56690 Make login OAuth state single-use (#193)
CI / ci (push) Successful in 8m6s
Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com>
2026-10-11 02:13:34 -04:00
27 changed files with 579 additions and 140 deletions
+1 -1
View File
@@ -158,7 +158,7 @@ func run(cfg config.Config, logger *slog.Logger) error {
return err return err
} }
planSvc, err := plan.NewService(db, weekDrafter(ai, guard), runner, googleSvc, notifier, sched.Wake, cfg.PublicURL, logger) planSvc, err := plan.NewService(db, weekDrafter(ai, guard), runner, googleSvc, notifier, sched.Wake, cfg.PublicURL, sched.Now, logger)
if err != nil { if err != nil {
return err return err
} }
+25 -9
View File
@@ -95,8 +95,8 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`).
| --- | --- | | --- | --- |
| `household_settings` | One row: timezone, dinner_time (17:30), reminder_time (16:30), review_time (09:00), plan_draft day/time (Fri 09:00), suggestions day/time (Sun 08:00), chat_autosend day/time (Sat 08:00), onboarding_completed_at, updates_seen_at. | | `household_settings` | One row: timezone, dinner_time (17:30), reminder_time (16:30), review_time (09:00), plan_draft day/time (Fri 09:00), suggestions day/time (Sun 08:00), chat_autosend day/time (Sat 08:00), onboarding_completed_at, updates_seen_at. |
| `recipes` | A real recipe page: source_url (unique), site_name, title, total/prep/cook minutes (the page's JSON-LD), yield, raw_jsonld, fetched_at; and its grounding: photo_id, method, difficulty, minutes_total (normalized prep to table). | | `recipes` | A real recipe page: source_url (unique), site_name, title, total/prep/cook minutes (the page's JSON-LD), yield, raw_jsonld, fetched_at; and its grounding: photo_id, method, difficulty, minutes_total (normalized prep to table). |
| `recipe_ingredients` | recipe_id, position, raw_text, name, qty, unit, note. | | `recipe_ingredients` | recipe_id, position (unique per recipe: the recipe order), raw_text, name, qty, unit, note. |
| `recipe_steps` | recipe_id, position, text. | | `recipe_steps` | recipe_id, position (unique per recipe: the recipe order), text. |
| `photos` | source_url, local_path, width, height, sha256, fetched_at. | | `photos` | source_url, local_path, width, height, sha256, fetched_at. |
| `dinners` | The meal record and its lifecycle: name, normalized_name, status (`suggested` / `quest_log` / `tried` / `regular` / `archived`), origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, recipe_url (a pasted recipe link awaiting grounding), photo_id, grounding_status (`pending` / `grounded` / `failed` / `manual`), grounding_error (why grounding failed), grounding_revision (raised whenever a dinner is renamed, so only the latest `ground_dinner` job may write it and no candidate offered before the rename grounds it), updated_by. | | `dinners` | The meal record and its lifecycle: name, normalized_name, status (`suggested` / `quest_log` / `tried` / `regular` / `archived`), origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, recipe_url (a pasted recipe link awaiting grounding), photo_id, grounding_status (`pending` / `grounded` / `failed` / `manual`), grounding_error (why grounding failed), grounding_revision (raised whenever a dinner is renamed, so only the latest `ground_dinner` job may write it and no candidate offered before the rename grounds it), updated_by. |
| `suggestion_batches` | trigger, status, ai_run_id. | | `suggestion_batches` | trigger, status, ai_run_id. |
@@ -121,6 +121,16 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`).
`recipes` holds a fetched recipe page; `dinners` holds the household's meal `recipes` holds a fetched recipe page; `dinners` holds the household's meal
record and its lifecycle, optionally linked to a recipe and a photo. record and its lifecycle, optionally linked to a recipe and a photo.
Migration `0018_recipe_item_position_unique` makes position unique per
recipe and fails, so the service refuses to start, on a database already
holding a duplicate. Before deploying it, check the database; both queries
must return no rows:
```sql
SELECT recipe_id, position FROM recipe_ingredients GROUP BY 1, 2 HAVING count(*) > 1;
SELECT recipe_id, position FROM recipe_steps GROUP BY 1, 2 HAVING count(*) > 1;
```
## Dinner lifecycle ## Dinner lifecycle
``` ```
@@ -215,10 +225,9 @@ automatic** (D-021).
raw_text}]` and `steps` the step texts, both in recipe position order and raw_text}]` and `steps` the step texts, both in recipe position order and
empty (`[]`) with no recipe, as for a dinner whose grounding is empty (`[]`) with no recipe, as for a dinner whose grounding is
`pending` or `failed`; a recipe entered by hand has only `raw_text`. `pending` or `failed`; a recipe entered by hand has only `raw_text`.
Two items at one position leave recipe order unknown, so such a recipe `source` is `{site_name}` of the page the recipe was fetched from, null
is 500 rather than shown out of order. `source` is `{site_name}` of the for a manual recipe or none; it has no `url`, as the page's link may
page the recipe was fetched from, null for a manual recipe or none; it carry a personal token.
has no `url`, as the page's link may carry a personal token.
`site_name` is the name the page gives its site (publisher or `site_name` is the name the page gives its site (publisher or
`og:site_name`) and null otherwise: a page naming no site is stored `og:site_name`) and null otherwise: a page naming no site is stored
under its host, which may be the pasted link's — a token in a under its host, which may be the pasted link's — a token in a
@@ -1120,7 +1129,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 +1155,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
+17 -2
View File
@@ -79,6 +79,7 @@ type Auth struct {
provider *oidc.Provider provider *oidc.Provider
client *http.Client client *http.Client
cookie loginCookie cookie loginCookie
pending *PendingStates
sessions Sessions sessions Sessions
allowed []string allowed []string
// endSessionURL is the provider's end_session_endpoint; logout ends // endSessionURL is the provider's end_session_endpoint; logout ends
@@ -157,6 +158,7 @@ func New(ctx context.Context, cfg Config, sessions Sessions, logger *slog.Logger
client: client, client: client,
allowed: slices.Clone(cfg.AllowedGroups), allowed: slices.Clone(cfg.AllowedGroups),
cookie: newLoginCookie(cfg.SecretKey), cookie: newLoginCookie(cfg.SecretKey),
pending: NewPendingStates(loginCookieTTL),
sessions: sessions, sessions: sessions,
endSessionURL: discovered.EndSessionURI, endSessionURL: discovered.EndSessionURI,
logger: logger, logger: logger,
@@ -250,7 +252,13 @@ func (a *Auth) login(w http.ResponseWriter, r *http.Request) {
Verifier: oauth2.GenerateVerifier(), Verifier: oauth2.GenerateVerifier(),
Return: returnTo, Return: returnTo,
} }
if err := a.cookie.set(w, state, time.Now()); err != nil { now := time.Now()
if err := a.pending.Add(state.State, now); err != nil {
a.logger.WarnContext(r.Context(), "login refused", "error", err)
http.Error(w, "too many sign-ins in progress; try again later", http.StatusServiceUnavailable)
return
}
if err := a.cookie.set(w, state, now); err != nil {
a.logger.ErrorContext(r.Context(), "set login cookie", "error", err) a.logger.ErrorContext(r.Context(), "set login cookie", "error", err)
http.Error(w, "internal error", http.StatusInternalServerError) http.Error(w, "internal error", http.StatusInternalServerError)
return return
@@ -262,7 +270,7 @@ func (a *Auth) login(w http.ResponseWriter, r *http.Request) {
} }
// callback completes the flow: it checks state against the login cookie, // callback completes the flow: it checks state against the login cookie,
// exchanges the code, verifies the ID token and nonce, then starts a session // consumes it, exchanges the code, verifies the ID token and nonce, then starts a session
// if the account is in an allowed group. Membership is checked on every // if the account is in an allowed group. Membership is checked on every
// login, so an account removed from the group is refused at its next one. // login, so an account removed from the group is refused at its next one.
func (a *Auth) callback(w http.ResponseWriter, r *http.Request) { func (a *Auth) callback(w http.ResponseWriter, r *http.Request) {
@@ -283,6 +291,13 @@ func (a *Auth) callback(w http.ResponseWriter, r *http.Request) {
http.Error(w, "sign-in state mismatch; start again", http.StatusBadRequest) http.Error(w, "sign-in state mismatch; start again", http.StatusBadRequest)
return return
} }
// A copy of the cookie verifies until it expires, so the state is
// single-use only because it is consumed here.
if !a.pending.Consume(state.State) {
a.logger.WarnContext(ctx, "login callback state already used")
http.Error(w, "sign-in state mismatch; start again", http.StatusBadRequest)
return
}
if q.Get("error") != "" { if q.Get("error") != "" {
// error and error_description are provider-supplied text; only the // error and error_description are provider-supplied text; only the
// fixed category is logged. // fixed category is logged.
+91
View File
@@ -497,6 +497,59 @@ func TestCallbackDefaultReturnPath(t *testing.T) {
} }
} }
// A saved copy of the login cookie still verifies until it expires, so a
// replay with a second valid code is refused only because the server
// consumed the state on first use.
func TestCallbackRefusesReplayedLoginState(t *testing.T) {
t.Parallel()
h := newHarness(t)
l := h.login(t, LoginPath)
idToken := h.provider.signRS256(h.provider.claims(l.query.Get("nonce")))
if resp := h.callback(l, l.query.Get("state"), idToken); resp.StatusCode != http.StatusSeeOther {
t.Fatalf("first callback status = %d, want 303", resp.StatusCode)
}
resp := h.callback(l, l.query.Get("state"), idToken)
if resp.StatusCode != http.StatusBadRequest {
t.Errorf("replayed callback status = %d, want 400", resp.StatusCode)
}
if n := len(h.sessions.started()); n != 1 {
t.Errorf("%d sessions started, want 1", n)
}
assertLoginCookieCleared(t, resp)
}
// Pending login states live in memory, so a login started before a restart
// fails closed after it even though its cookie still verifies.
func TestCallbackRefusesLoginStartedBeforeRestart(t *testing.T) {
t.Parallel()
h := newHarness(t)
l := h.login(t, LoginPath)
restarted, err := New(context.Background(), Config{
Issuer: h.provider.issuer(),
ClientID: testClientID,
ClientSecret: testClientSecret,
PublicURL: testPublicURL,
SecretKey: []byte("server-secret-key"),
AllowedGroups: testAllowedGroups,
}, h.sessions, slog.New(slog.DiscardHandler))
if err != nil {
t.Fatal(err)
}
r := chi.NewRouter()
restarted.Mount(r)
h.router = r
resp := h.callback(l, l.query.Get("state"), h.provider.signRS256(h.provider.claims(l.query.Get("nonce"))))
if resp.StatusCode != http.StatusBadRequest {
t.Errorf("callback after restart status = %d, want 400", resp.StatusCode)
}
if n := len(h.sessions.started()); n != 0 {
t.Errorf("%d sessions started, want none", n)
}
}
func TestCallbackRejected(t *testing.T) { func TestCallbackRejected(t *testing.T) {
t.Parallel() t.Parallel()
@@ -819,6 +872,44 @@ func TestLoginCookieExpires(t *testing.T) {
} }
} }
func TestPendingStates(t *testing.T) {
t.Parallel()
p := NewPendingStates(time.Minute)
now := time.Now()
if err := p.Add("first", now); err != nil {
t.Fatal(err)
}
if !p.Consume("first") {
t.Error("Consume of a pending state = false")
}
if p.Consume("first") {
t.Error("second Consume of a state = true")
}
if p.Consume("never-added") {
t.Error("Consume of an unknown state = true")
}
for i := range maxPendingStates - 1 {
if err := p.Add(fmt.Sprint("s", i), now); err != nil {
t.Fatalf("Add %d: %v", i, err)
}
}
// "first" was consumed but has not expired, so it still counts.
if err := p.Add("over", now); !errors.Is(err, ErrTooManyPending) {
t.Errorf("Add past the bound = %v, want ErrTooManyPending", err)
}
// Once the earlier states expire, they are forgotten and a new one fits.
if err := p.Add("later", now.Add(time.Minute)); err != nil {
t.Errorf("Add after expiry: %v", err)
}
if p.Consume("s0") {
t.Error("an expired state was still pending")
}
if !p.Consume("later") {
t.Error("the state added after expiry is not pending")
}
}
func TestLoginRejectsOffOriginReturn(t *testing.T) { func TestLoginRejectsOffOriginReturn(t *testing.T) {
t.Parallel() t.Parallel()
h := newHarness(t) h := newHarness(t)
+68
View File
@@ -10,6 +10,7 @@ import (
"net/http" "net/http"
"net/url" "net/url"
"strings" "strings"
"sync"
"time" "time"
) )
@@ -153,6 +154,73 @@ func (c FlowCookie) Read(r *http.Request, v flowValue, now time.Time) error {
return nil return nil
} }
// maxPendingStates bounds each flow's pending states. The login route is
// unauthenticated, so without a bound anyone could grow the set without
// limit; one household never has close to this many flows in progress.
const maxPendingStates = 10000
// ErrTooManyPending means a flow's pending states are at maxPendingStates,
// so a new flow is refused until older ones expire.
var ErrTooManyPending = errors.New("too many sign-in flows in progress")
// PendingStates holds the state of every redirect flow started and not yet
// completed, until its flow cookie expires. A copy of a flow cookie verifies
// until it expires, so a flow is single-use only because its callback
// consumes the state here. The states live in memory — MealQuest runs as one
// process — so a flow started before a restart fails closed after it.
type PendingStates struct {
ttl time.Duration
mu sync.Mutex
states map[string]time.Time
// order is every added state in the order added. Every state is pending
// for the same ttl, so it is also expiry order and Add forgets expired
// states from its front.
order []pendingState
}
type pendingState struct {
state string
expires time.Time
}
// NewPendingStates returns an empty set whose states are pending for ttl,
// the flow cookie's lifetime.
func NewPendingStates(ttl time.Duration) *PendingStates {
return &PendingStates{ttl: ttl, states: map[string]time.Time{}}
}
// Add records state, started at now, as pending and forgets expired ones.
// It fails with ErrTooManyPending when maxPendingStates are pending.
func (p *PendingStates) Add(state string, now time.Time) error {
p.mu.Lock()
defer p.mu.Unlock()
expired := 0
for expired < len(p.order) && !now.Before(p.order[expired].expires) {
delete(p.states, p.order[expired].state)
expired++
}
p.order = p.order[expired:]
// The bound counts every unexpired state, consumed or not: a consumed
// state leaves states at once but order only when it expires.
if len(p.order) >= maxPendingStates {
return ErrTooManyPending
}
expires := now.Add(p.ttl)
p.states[state] = expires
p.order = append(p.order, pendingState{state: state, expires: expires})
return nil
}
// Consume removes state from the pending states, reporting whether it was
// there.
func (p *PendingStates) Consume(state string) bool {
p.mu.Lock()
defer p.mu.Unlock()
_, ok := p.states[state]
delete(p.states, state)
return ok
}
// returnPath validates the optional post-login return path. Empty means "/". // returnPath validates the optional post-login return path. Empty means "/".
// Only same-origin absolute paths are allowed: no scheme, host, protocol- // Only same-origin absolute paths are allowed: no scheme, host, protocol-
// relative "//" prefix, backslashes or control characters. // relative "//" prefix, backslashes or control characters.
+2 -2
View File
@@ -220,7 +220,7 @@ func (q *Queries) GetRecipeBySourceURL(ctx context.Context, sourceUrl string) (R
const listRecipeIngredients = `-- name: ListRecipeIngredients :many const listRecipeIngredients = `-- name: ListRecipeIngredients :many
SELECT id, recipe_id, position, raw_text, name, qty, unit, note, created_at, updated_at FROM recipe_ingredients SELECT id, recipe_id, position, raw_text, name, qty, unit, note, created_at, updated_at FROM recipe_ingredients
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id ORDER BY position
` `
func (q *Queries) ListRecipeIngredients(ctx context.Context, recipeID string) ([]RecipeIngredient, error) { func (q *Queries) ListRecipeIngredients(ctx context.Context, recipeID string) ([]RecipeIngredient, error) {
@@ -260,7 +260,7 @@ func (q *Queries) ListRecipeIngredients(ctx context.Context, recipeID string) ([
const listRecipeSteps = `-- name: ListRecipeSteps :many const listRecipeSteps = `-- name: ListRecipeSteps :many
SELECT id, recipe_id, position, text, created_at, updated_at FROM recipe_steps SELECT id, recipe_id, position, text, created_at, updated_at FROM recipe_steps
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id ORDER BY position
` `
func (q *Queries) ListRecipeSteps(ctx context.Context, recipeID string) ([]RecipeStep, error) { func (q *Queries) ListRecipeSteps(ctx context.Context, recipeID string) ([]RecipeStep, error) {
+2 -16
View File
@@ -157,10 +157,7 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail)
if err != nil { if err != nil {
return fmt.Errorf("ingredients: %w", err) return fmt.Errorf("ingredients: %w", err)
} }
for i, ing := range ings { for _, ing := range ings {
if i > 0 && ing.Position == ings[i-1].Position {
return duplicatePosition("ingredient", ing.Position)
}
d.Ingredients = append(d.Ingredients, Ingredient{ d.Ingredients = append(d.Ingredients, Ingredient{
Name: nullString(ing.Name), Name: nullString(ing.Name),
Qty: nullString(ing.Qty), Qty: nullString(ing.Qty),
@@ -173,10 +170,7 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail)
if err != nil { if err != nil {
return fmt.Errorf("steps: %w", err) return fmt.Errorf("steps: %w", err)
} }
for i, step := range steps { for _, step := range steps {
if i > 0 && step.Position == steps[i-1].Position {
return duplicatePosition("step", step.Position)
}
d.Steps = append(d.Steps, step.Text) d.Steps = append(d.Steps, step.Text)
} }
return nil return nil
@@ -283,11 +277,3 @@ func browserHost(u *url.URL) bool {
} }
return true return true
} }
// duplicatePosition is the error for two recipe items of kind at position.
// Position is the only record of recipe order, and the id that breaks the
// tie says nothing about it, so a recipe with a duplicate is refused rather
// than shown in an order nobody can trust.
func duplicatePosition(kind string, position int64) error {
return fmt.Errorf("two %ss at position %d: recipe order is unknown", kind, position)
}
+8 -30
View File
@@ -137,10 +137,9 @@ type Service struct {
account sync.Mutex account sync.Mutex
// pending holds the state of every connect started and not yet // pending holds the state of every connect started and not yet
// completed, with when it expires. Complete consumes a state once, so a // completed. Complete consumes a state once, so a replayed cookie finds
// replayed cookie finds it gone. // it gone.
pendingMu sync.Mutex pending *auth.PendingStates
pending map[string]time.Time
} }
// NewService returns the service over st, sealing refresh tokens with box. // NewService returns the service over st, sealing refresh tokens with box.
@@ -175,7 +174,7 @@ func NewService(st *store.Store, box *secretbox.Box, cfg Config) (*Service, erro
cookie: auth.NewFlowCookie(connectCookieName, connectCookieKeyLabel, connectCookieTTL, cfg.SecretKey), cookie: auth.NewFlowCookie(connectCookieName, connectCookieKeyLabel, connectCookieTTL, cfg.SecretKey),
retryDelay: firstRetryDelay, retryDelay: firstRetryDelay,
now: time.Now, now: time.Now,
pending: map[string]time.Time{}, pending: auth.NewPendingStates(connectCookieTTL),
}, nil }, nil
} }
@@ -210,10 +209,12 @@ func (s *Service) Connect(w http.ResponseWriter, session auth.Session) (string,
Session: sessionBinding(session), Session: sessionBinding(session),
} }
now := s.now() now := s.now()
if err := s.pending.Add(state.State, now); err != nil {
return "", fmt.Errorf("gcal: %w", err)
}
if err := s.cookie.Set(w, &state, now); err != nil { if err := s.cookie.Set(w, &state, now); err != nil {
return "", fmt.Errorf("gcal: set connect cookie: %w", err) return "", fmt.Errorf("gcal: set connect cookie: %w", err)
} }
s.addPending(state.State, now)
return s.oauth.AuthCodeURL(state.State, return s.oauth.AuthCodeURL(state.State,
oauth2.AccessTypeOffline, oauth2.AccessTypeOffline,
oauth2.ApprovalForce, oauth2.ApprovalForce,
@@ -243,7 +244,7 @@ func (s *Service) Complete(w http.ResponseWriter, r *http.Request, session auth.
} }
// A copy of the cookie verifies until it expires, so the state is // A copy of the cookie verifies until it expires, so the state is
// single-use only because it is consumed here. // single-use only because it is consumed here.
if !s.consumePending(state.State) { if !s.pending.Consume(state.State) {
return fmt.Errorf("%w: already used", ErrBadState) return fmt.Errorf("%w: already used", ErrBadState)
} }
if q.Get("error") != "" { if q.Get("error") != "" {
@@ -276,29 +277,6 @@ func (s *Service) Complete(w http.ResponseWriter, r *http.Request, session auth.
return s.store(ctx, sealed) return s.store(ctx, sealed)
} }
// addPending records state, started at now, as pending until the connect
// cookie expires, and forgets expired ones.
func (s *Service) addPending(state string, now time.Time) {
s.pendingMu.Lock()
defer s.pendingMu.Unlock()
for k, expires := range s.pending {
if !now.Before(expires) {
delete(s.pending, k)
}
}
s.pending[state] = now.Add(connectCookieTTL)
}
// consumePending removes state from the pending states, reporting whether
// it was there.
func (s *Service) consumePending(state string) bool {
s.pendingMu.Lock()
defer s.pendingMu.Unlock()
_, ok := s.pending[state]
delete(s.pending, state)
return ok
}
// checkScopes fails with ErrScopes unless token's granted scopes include // checkScopes fails with ErrScopes unless token's granted scopes include
// every scope in Scopes. Google lets the household untick a permission on // every scope in Scopes. Google lets the household untick a permission on
// the consent screen and reports what was granted in "scope". // the consent screen and reports what was granted in "scope".
+34 -6
View File
@@ -94,7 +94,7 @@ func newEnv(t *testing.T) *env {
draft := func(context.Context, string, skills.WeekRequest) (skills.Week, error) { draft := func(context.Context, string, skills.WeekRequest) (skills.Week, error) {
return skills.Week{}, errors.New("no model in this test") return skills.Week{}, errors.New("no model in this test")
} }
plans, err := plan.NewService(st, draft, runner, googleSvc, notifier, func() {}, pub, logger) plans, err := plan.NewService(st, draft, runner, googleSvc, notifier, func() {}, pub, time.Now, logger)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -222,16 +222,44 @@ func TestTonightSwitchesAtLocalMidnight(t *testing.T) {
func TestTonightDinner(t *testing.T) { func TestTonightDinner(t *testing.T) {
t.Parallel() t.Parallel()
e := newEnv(t) e := newEnv(t)
e.planWeek(t) ctx := t.Context()
planID := e.planWeek(t)
planned := func(date string) string {
t.Helper()
var id string
if err := e.st.Reader.QueryRowContext(ctx, `SELECT dinner_id FROM plan_days WHERE plan_id = ? AND date = ?`, planID, date).Scan(&id); err != nil {
t.Fatal(err)
}
return id
}
h := e.at(t, "2026-10-13 18:00") h := e.at(t, "2026-10-13 18:00")
if h.Tonight == nil || h.Tonight.ID == "" || h.Tonight.Origin != "regular" || h.Tonight.Photo != nil || if h.Tonight == nil || h.Tonight.ID != planned("2026-10-13") || h.Tonight.Origin != "regular" || h.Tonight.Photo != nil ||
h.Tonight.MinutesTotal == nil || *h.Tonight.MinutesTotal != 30 { h.Tonight.MinutesTotal == nil || *h.Tonight.MinutesTotal != 30 {
t.Errorf("tonight = %+v, want Tuesday's 30-minute Regular with no photo", h.Tonight) t.Errorf("tonight = %+v, want Tuesday's 30-minute Regular %s with no photo", h.Tonight, planned("2026-10-13"))
} }
h = e.at(t, "2026-10-14 18:00") h = e.at(t, "2026-10-14 18:00")
if h.Tonight == nil || h.Tonight.Origin != "side_quest" || h.Tonight.MinutesTotal != nil { if h.Tonight == nil || h.Tonight.ID != planned("2026-10-14") || h.Tonight.Origin != "side_quest" || h.Tonight.MinutesTotal != nil {
t.Errorf("tonight = %+v, want Wednesday's Side quest of unknown minutes", h.Tonight) t.Errorf("tonight = %+v, want Wednesday's Side quest %s of unknown minutes", h.Tonight, planned("2026-10-14"))
}
// Thursday's dinner has a photo: Home serves its list-sized copy.
now := store.Now().String()
photo, err := db.New(e.st.Writer).CreatePhoto(ctx, db.CreatePhotoParams{
ID: store.NewID(), SourceUrl: "https://example.com/thursday.jpg", LocalPath: "photos/thursday.jpg",
Width: 800, Height: 600, Sha256: "thursday", FetchedAt: now, CreatedAt: now, UpdatedAt: now,
})
if err != nil {
t.Fatal(err)
}
thursday := planned("2026-10-15")
if _, err := e.st.Writer.ExecContext(ctx, `UPDATE dinners SET photo_id = ? WHERE id = ?`, photo.ID, thursday); err != nil {
t.Fatal(err)
}
h = e.at(t, "2026-10-15 18:00")
wantPhoto := "/media/photos/" + photo.ID + "-400.jpg"
if h.Tonight == nil || h.Tonight.ID != thursday || h.Tonight.Photo == nil || *h.Tonight.Photo != wantPhoto {
t.Errorf("tonight = %+v, want Thursday's dinner %s with photo %s", h.Tonight, thursday, wantPhoto)
} }
} }
+1 -1
View File
@@ -96,7 +96,7 @@ func (s *Service) revision(ctx context.Context, weekStart store.LocalDate) (int6
// the household context, the pool, last week's dinners and the dinner // the household context, the pool, last week's dinners and the dinner
// time. // time.
func (s *Service) request(ctx context.Context, weekStart store.LocalDate) (skills.WeekRequest, error) { func (s *Service) request(ctx context.Context, weekStart store.LocalDate) (skills.WeekRequest, error) {
hctx, err := harness.BuildContext(ctx, s.st, time.Now(), ContextBudget) hctx, err := harness.BuildContext(ctx, s.st, s.now(), ContextBudget)
if err != nil { if err != nil {
return skills.WeekRequest{}, fmt.Errorf("plan: %w", err) return skills.WeekRequest{}, fmt.Errorf("plan: %w", err)
} }
+10 -5
View File
@@ -149,16 +149,19 @@ type Service struct {
// publicURL is MEALQUEST_PUBLIC_URL, under which a pushed event links // publicURL is MEALQUEST_PUBLIC_URL, under which a pushed event links
// to its dinner. // to its dinner.
publicURL *url.URL publicURL *url.URL
// now is the clock today is read from: the scheduler's.
now func() time.Time
} }
// NewService returns a service over st that drafts with draft, pushes to // NewService returns a service over st that drafts with draft, pushes to
// cal with links under publicURL, tells the household through notifier, // cal with links under publicURL, tells the household through notifier,
// which must be over st, calls wake (the scheduler's Wake) after every // which must be over st, calls wake (the scheduler's Wake) after every
// committed change to a plan's days, and registers the KindDraftWeek job on // committed change to a plan's days, reads today from now (the scheduler's
// runner, which must not have started yet. The kind is service-only: // Now) and registers the KindDraftWeek job on runner, which must not have
// started yet. The kind is service-only:
// StartDraft and StartScheduledDraft, which keep one draft in flight per // StartDraft and StartScheduledDraft, which keep one draft in flight per
// week, are the only way to enqueue it. // week, are the only way to enqueue it.
func NewService(st *store.Store, draft Drafter, runner *jobs.Runner, cal Calendar, notifier *notify.Service, wake func(), publicURL *url.URL, logger *slog.Logger) (*Service, error) { func NewService(st *store.Store, draft Drafter, runner *jobs.Runner, cal Calendar, notifier *notify.Service, wake func(), publicURL *url.URL, now func() time.Time, logger *slog.Logger) (*Service, error) {
switch { switch {
case st == nil: case st == nil:
return nil, errors.New("plan: nil store") return nil, errors.New("plan: nil store")
@@ -174,10 +177,12 @@ func NewService(st *store.Store, draft Drafter, runner *jobs.Runner, cal Calenda
return nil, errors.New("plan: nil wake") return nil, errors.New("plan: nil wake")
case publicURL == nil: case publicURL == nil:
return nil, errors.New("plan: nil public URL") return nil, errors.New("plan: nil public URL")
case now == nil:
return nil, errors.New("plan: nil clock")
case logger == nil: case logger == nil:
return nil, errors.New("plan: nil logger") return nil, errors.New("plan: nil logger")
} }
s := &Service{st: st, draft: draft, cal: cal, notifier: notifier, wake: wake, logger: logger, publicURL: publicURL} s := &Service{st: st, draft: draft, cal: cal, notifier: notifier, wake: wake, logger: logger, publicURL: publicURL, now: now}
kind, err := jobs.Register(runner, KindDraftWeek, s.runDraft, Reasons...) kind, err := jobs.Register(runner, KindDraftWeek, s.runDraft, Reasons...)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -501,7 +506,7 @@ func (s *Service) Tonight(ctx context.Context) (week, today store.LocalDate, err
if err != nil { if err != nil {
return store.LocalDate{}, store.LocalDate{}, fmt.Errorf("plan: household settings: %w", err) return store.LocalDate{}, store.LocalDate{}, fmt.Errorf("plan: household settings: %w", err)
} }
today = household.Today(time.Now(), zone) today = household.Today(s.now(), zone)
week = household.WeekStart(today) week = household.WeekStart(today)
_, err = q.GetWeekPlan(ctx, week.String()) _, err = q.GetWeekPlan(ctx, week.String())
if errors.Is(err, sql.ErrNoRows) { if errors.Is(err, sql.ErrNoRows) {
+1 -1
View File
@@ -146,7 +146,7 @@ func newEnv(t *testing.T) *env {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
e.svc, err = plan.NewService(st, drafter, e.runner, e.gcal, notifytest.New(t, st), func() {}, publicURL, logger) e.svc, err = plan.NewService(st, drafter, e.runner, e.gcal, notifytest.New(t, st), func() {}, publicURL, time.Now, logger)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
+73 -13
View File
@@ -7,6 +7,7 @@ import (
"fmt" "fmt"
"log/slog" "log/slog"
"net/url" "net/url"
"slices"
"strings" "strings"
"sync" "sync"
"testing" "testing"
@@ -85,7 +86,21 @@ func (c noCalendar) PushEvents(context.Context, []gcal.Event, func() error) erro
// pass the fake clock's waits do not expect. // pass the fake clock's waits do not expect.
func (e *env) plans(ids map[string]string) *planEnv { func (e *env) plans(ids map[string]string) *planEnv {
e.t.Helper() e.t.Helper()
return e.newPlans(ids, func() {}) return e.newPlans(ids, func() {}, time.Now)
}
// scheduledPlans is plans whose today is read from the clock of the
// scheduler start last started, as in the server, so a draft's plan_ready
// title follows the fake clock. It drafts only once a scheduler started.
func (e *env) scheduledPlans(ids map[string]string) *planEnv {
e.t.Helper()
return e.newPlans(ids, func() {}, func() time.Time {
s := e.sched.Load()
if s == nil {
panic("plan service read the clock before a scheduler started")
}
return s.Now()
})
} }
// wakingPlans is plans whose changes wake the scheduler, as in the server. // wakingPlans is plans whose changes wake the scheduler, as in the server.
@@ -93,11 +108,12 @@ func (e *env) plans(ids map[string]string) *planEnv {
// the wake's pass with changed. // the wake's pass with changed.
func (e *env) wakingPlans(ids map[string]string) *planEnv { func (e *env) wakingPlans(ids map[string]string) *planEnv {
e.t.Helper() e.t.Helper()
return e.newPlans(ids, e.wake) return e.newPlans(ids, e.wake, time.Now)
} }
// newPlans is plans whose service calls wake after a plan change. // newPlans is plans whose service calls wake after a plan change and reads
func (e *env) newPlans(ids map[string]string, wake func()) *planEnv { // today from now.
func (e *env) newPlans(ids map[string]string, wake func(), now func() time.Time) *planEnv {
e.t.Helper() e.t.Helper()
p := &planEnv{e: e, model: &llm.Fake{}, ids: ids, logs: &syncBuffer{}} p := &planEnv{e: e, model: &llm.Fake{}, ids: ids, logs: &syncBuffer{}}
guard, err := harness.NewGuard(harness.DefaultMaxCalls) guard, err := harness.NewGuard(harness.DefaultMaxCalls)
@@ -118,7 +134,7 @@ func (e *env) newPlans(ids map[string]string, wake func()) *planEnv {
if err != nil { if err != nil {
e.t.Fatal(err) e.t.Fatal(err)
} }
p.svc, err = plan.NewService(e.st, drafter, runner, noCalendar{e.t}, notifytest.New(e.t, e.st), wake, publicURL, logger) p.svc, err = plan.NewService(e.st, drafter, runner, noCalendar{e.t}, notifytest.New(e.t, e.st), wake, publicURL, now, logger)
if err != nil { if err != nil {
e.t.Fatal(err) e.t.Fatal(err)
} }
@@ -235,6 +251,41 @@ func (e *env) planReady() []string {
return keys return keys
} }
// readyRow is a stored plan_ready notification as these tests check it.
type readyRow struct {
Key string
Channel notify.Channel
Title string
}
// planReadyRows returns every plan_ready notification stored, oldest
// first, each week's Google Chat row before its push row.
func (e *env) planReadyRows() []readyRow {
e.t.Helper()
var out []readyRow
for _, r := range notifytest.Rows(e.t, e.st, notify.PlanReady) {
out = append(out, readyRow{Key: r.Key, Channel: r.Channel, Title: r.Payload.Title})
}
return out
}
// nextWeekReady is the plan_ready rows of a draft of the week starting
// week, drafted the week before: one to Google Chat and one to push.
func nextWeekReady(week string) []readyRow {
const title = "Next week's plan is ready"
key := string(notify.PlanReady) + ":" + week
return []readyRow{{key, notify.ChannelChat, title}, {key, notify.ChannelPush, title}}
}
// wantReady fails unless the plan_ready rows are got.
func wantReady(t *testing.T, what string, got []readyRow, want ...[]readyRow) {
t.Helper()
all := slices.Concat(want...)
if !slices.Equal(got, all) {
t.Fatalf("%s: plan_ready rows = %+v, want %+v", what, got, all)
}
}
var ( var (
succeeded = draftJob{Status: string(jobs.StatusSucceeded)} succeeded = draftJob{Status: string(jobs.StatusSucceeded)}
aiFailed = draftJob{Status: string(jobs.StatusFailed), Error: plan.ReasonAIFailed} aiFailed = draftJob{Status: string(jobs.StatusFailed), Error: plan.ReasonAIFailed}
@@ -257,7 +308,7 @@ func mustLocalDate(s string) store.LocalDate {
// by its Monday once. // by its Monday once.
func TestPlanDraftDraftsNextWeek(t *testing.T) { func TestPlanDraftDraftsNextWeek(t *testing.T) {
e := newEnv(t) e := newEnv(t)
p := e.plans(e.pool()) p := e.scheduledPlans(e.pool())
p.reply(w43) p.reply(w43)
p.reply(w44) p.reply(w44)
@@ -271,6 +322,7 @@ func TestPlanDraftDraftsNextWeek(t *testing.T) {
wantDrafts(t, "at Friday 09:00", e.drafts(), succeeded) wantDrafts(t, "at Friday 09:00", e.drafts(), succeeded)
wantKeys(t, "plans at Friday 09:00", e.weekPlans(), []string{"2026-10-19"}) wantKeys(t, "plans at Friday 09:00", e.weekPlans(), []string{"2026-10-19"})
wantKeys(t, "plan_ready of W43", e.planReady(), []string{"plan_ready:2026-10-19"}) wantKeys(t, "plan_ready of W43", e.planReady(), []string{"plan_ready:2026-10-19"})
wantReady(t, "plan_ready rows of W43", e.planReadyRows(), nextWeekReady("2026-10-19"))
got, err := p.svc.Get(context.Background(), w43) got, err := p.svc.Get(context.Background(), w43)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@@ -283,6 +335,7 @@ func TestPlanDraftDraftsNextWeek(t *testing.T) {
wantDrafts(t, "W44", e.drafts(), succeeded, succeeded) wantDrafts(t, "W44", e.drafts(), succeeded, succeeded)
wantKeys(t, "plans", e.weekPlans(), []string{"2026-10-19", "2026-10-26"}) wantKeys(t, "plans", e.weekPlans(), []string{"2026-10-19", "2026-10-26"})
wantKeys(t, "plan_ready of W43 and W44", e.planReady(), []string{"plan_ready:2026-10-19", "plan_ready:2026-10-26"}) wantKeys(t, "plan_ready of W43 and W44", e.planReady(), []string{"plan_ready:2026-10-19", "plan_ready:2026-10-26"})
wantReady(t, "plan_ready rows of W43 and W44", e.planReadyRows(), nextWeekReady("2026-10-19"), nextWeekReady("2026-10-26"))
wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43", "2026-W44"}) wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43", "2026-W44"})
} }
@@ -291,23 +344,25 @@ func TestPlanDraftDraftsNextWeek(t *testing.T) {
// notifies no one and says so. // notifies no one and says so.
func TestPlanDraftLeavesAnExistingPlan(t *testing.T) { func TestPlanDraftLeavesAnExistingPlan(t *testing.T) {
e := newEnv(t) e := newEnv(t)
p := e.plans(e.pool()) p := e.scheduledPlans(e.pool())
p.reply(w43) p.reply(w43)
p.reply(w43) // a second draft would succeed too p.reply(w43) // a second draft would succeed too
r := e.start(at(2026, 10, 16, 8, 0), p.svc.DraftSchedule())
if _, err := p.svc.StartDraft(context.Background(), w43, false); err != nil { if _, err := p.svc.StartDraft(context.Background(), w43, false); err != nil {
t.Fatal(err) t.Fatal(err)
} }
wantDrafts(t, "manual draft", e.drafts(), succeeded) wantDrafts(t, "manual draft", e.drafts(), succeeded)
wantKeys(t, "plan_ready of the manual draft", e.planReady(), []string{"plan_ready:2026-10-19"}) wantKeys(t, "plan_ready of the manual draft", e.planReady(), []string{"plan_ready:2026-10-19"})
wantReady(t, "plan_ready rows of the manual draft", e.planReadyRows(), nextWeekReady("2026-10-19"))
before, err := p.svc.Get(context.Background(), w43) before, err := p.svc.Get(context.Background(), w43)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
r := e.start(at(2026, 10, 16, 8, 0), p.svc.DraftSchedule())
r.runUntil(at(2026, 10, 16, 9, 0)) r.runUntil(at(2026, 10, 16, 9, 0))
wantDrafts(t, "with a plan", e.drafts(), succeeded) wantDrafts(t, "with a plan", e.drafts(), succeeded)
wantKeys(t, "plan_ready with a plan", e.planReady(), []string{"plan_ready:2026-10-19"}) wantKeys(t, "plan_ready with a plan", e.planReady(), []string{"plan_ready:2026-10-19"})
wantReady(t, "plan_ready rows with a plan", e.planReadyRows(), nextWeekReady("2026-10-19"))
wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43"}) wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43"})
after, err := p.svc.Get(context.Background(), w43) after, err := p.svc.Get(context.Background(), w43)
if err != nil { if err != nil {
@@ -327,23 +382,25 @@ func TestPlanDraftLeavesAnExistingPlan(t *testing.T) {
func TestPlanDraftNoDuplicateAfterRestart(t *testing.T) { func TestPlanDraftNoDuplicateAfterRestart(t *testing.T) {
e := newEnv(t) e := newEnv(t)
ids := e.pool() ids := e.pool()
p := e.plans(ids) p := e.scheduledPlans(ids)
p.reply(w43) p.reply(w43)
r := e.start(at(2026, 10, 16, 8, 0), p.svc.DraftSchedule()) r := e.start(at(2026, 10, 16, 8, 0), p.svc.DraftSchedule())
r.runUntil(at(2026, 10, 16, 9, 0)) r.runUntil(at(2026, 10, 16, 9, 0))
wantDrafts(t, "first process", e.drafts(), succeeded) wantDrafts(t, "first process", e.drafts(), succeeded)
wantReady(t, "plan_ready rows of the first process", e.planReadyRows(), nextWeekReady("2026-10-19"))
r.stop() r.stop()
p.stop() p.stop()
// The restarted process has a new job runner, plan service and // The restarted process has a new job runner, plan service and
// scheduler; it would answer a second draft if one were started. // scheduler; it would answer a second draft if one were started.
p2 := e.plans(ids) p2 := e.scheduledPlans(ids)
p2.reply(w43) p2.reply(w43)
r2 := e.start(at(2026, 10, 16, 10, 0), p2.svc.DraftSchedule()) r2 := e.start(at(2026, 10, 16, 10, 0), p2.svc.DraftSchedule())
r2.runUntil(at(2026, 10, 23, 8, 59)) r2.runUntil(at(2026, 10, 23, 8, 59))
wantDrafts(t, "after restart", e.drafts(), succeeded) wantDrafts(t, "after restart", e.drafts(), succeeded)
wantKeys(t, "plans after restart", e.weekPlans(), []string{"2026-10-19"}) wantKeys(t, "plans after restart", e.weekPlans(), []string{"2026-10-19"})
wantKeys(t, "plan_ready after restart", e.planReady(), []string{"plan_ready:2026-10-19"}) wantKeys(t, "plan_ready after restart", e.planReady(), []string{"plan_ready:2026-10-19"})
wantReady(t, "plan_ready rows after restart", e.planReadyRows(), nextWeekReady("2026-10-19"))
wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43"}) wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43"})
} }
@@ -354,7 +411,7 @@ func TestPlanDraftNoDuplicateAfterRestart(t *testing.T) {
func TestPlanDraftAIFailureEmitsNothing(t *testing.T) { func TestPlanDraftAIFailureEmitsNothing(t *testing.T) {
e := newEnv(t) e := newEnv(t)
ids := e.pool() ids := e.pool()
p := e.plans(ids) p := e.scheduledPlans(ids)
p.model.QueueGenerate(llm.GenerateResult{Err: errors.New("provider down")}) p.model.QueueGenerate(llm.GenerateResult{Err: errors.New("provider down")})
r := e.start(at(2026, 10, 16, 8, 0), p.svc.DraftSchedule()) r := e.start(at(2026, 10, 16, 8, 0), p.svc.DraftSchedule())
@@ -362,6 +419,7 @@ func TestPlanDraftAIFailureEmitsNothing(t *testing.T) {
wantDrafts(t, "after the AI failed", e.drafts(), aiFailed) wantDrafts(t, "after the AI failed", e.drafts(), aiFailed)
wantKeys(t, "plans after the AI failed", e.weekPlans(), nil) wantKeys(t, "plans after the AI failed", e.weekPlans(), nil)
wantKeys(t, "plan_ready after the AI failed", e.planReady(), nil) wantKeys(t, "plan_ready after the AI failed", e.planReady(), nil)
wantReady(t, "plan_ready rows after the AI failed", e.planReadyRows())
wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43"}) wantKeys(t, "schedule_runs", e.claimed(scheduler.PlanDraft), []string{"2026-W42", "2026-W43"})
var status, msg string var status, msg string
@@ -377,11 +435,12 @@ func TestPlanDraftAIFailureEmitsNothing(t *testing.T) {
r.stop() r.stop()
p.stop() p.stop()
p2 := e.plans(ids) p2 := e.scheduledPlans(ids)
p2.reply(w43) p2.reply(w43)
e.start(at(2026, 10, 16, 10, 0), p2.svc.DraftSchedule()) e.start(at(2026, 10, 16, 10, 0), p2.svc.DraftSchedule())
wantDrafts(t, "after restart", e.drafts(), aiFailed) wantDrafts(t, "after restart", e.drafts(), aiFailed)
wantKeys(t, "plan_ready after restart", e.planReady(), nil) wantKeys(t, "plan_ready after restart", e.planReady(), nil)
wantReady(t, "plan_ready rows after restart", e.planReadyRows())
} }
// TestPlanDraftFollowsSettingsWithoutRestart: a plan_draft_dow and // TestPlanDraftFollowsSettingsWithoutRestart: a plan_draft_dow and
@@ -390,7 +449,7 @@ func TestPlanDraftAIFailureEmitsNothing(t *testing.T) {
// week. // week.
func TestPlanDraftFollowsSettingsWithoutRestart(t *testing.T) { func TestPlanDraftFollowsSettingsWithoutRestart(t *testing.T) {
e := newEnv(t) e := newEnv(t)
p := e.plans(e.pool()) p := e.scheduledPlans(e.pool())
p.reply(w43) p.reply(w43)
r := e.start(at(2026, 10, 12, 12, 0), p.svc.DraftSchedule()) // Monday r := e.start(at(2026, 10, 12, 12, 0), p.svc.DraftSchedule()) // Monday
@@ -401,6 +460,7 @@ func TestPlanDraftFollowsSettingsWithoutRestart(t *testing.T) {
r.runUntil(at(2026, 10, 14, 18, 30)) r.runUntil(at(2026, 10, 14, 18, 30))
wantDrafts(t, "at Wednesday 18:30", e.drafts(), succeeded) wantDrafts(t, "at Wednesday 18:30", e.drafts(), succeeded)
wantKeys(t, "plan_ready", e.planReady(), []string{"plan_ready:2026-10-19"}) wantKeys(t, "plan_ready", e.planReady(), []string{"plan_ready:2026-10-19"})
wantReady(t, "plan_ready rows", e.planReadyRows(), nextWeekReady("2026-10-19"))
// Friday 09:00, the old fire time, drafts nothing more. // Friday 09:00, the old fire time, drafts nothing more.
r.runUntil(at(2026, 10, 16, 9, 0)) r.runUntil(at(2026, 10, 16, 9, 0))
+5
View File
@@ -378,6 +378,11 @@ func (s *Scheduler) Wake() {
s.signal() s.signal()
} }
// Now is the time on the scheduler's clock, by which its schedules fire.
func (s *Scheduler) Now() time.Time {
return s.now()
}
// signal leaves one token in wake for the loop. // signal leaves one token in wake for the loop.
func (s *Scheduler) signal() { func (s *Scheduler) signal() {
select { select {
+2 -24
View File
@@ -70,8 +70,8 @@ func TestGetDinnerGroundedSideQuest(t *testing.T) {
} }
for _, stmt := range []string{ for _, stmt := range []string{
`UPDATE dinners SET status = 'quest_log', origin = 'side_quest', is_favorite = 1 WHERE id = ?`, `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_ingredients SET position = 9 - 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_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 { if _, err := e.st.Writer.Exec(stmt, id); err != nil {
t.Fatal(err) t.Fatal(err)
@@ -445,25 +445,3 @@ func TestGetDinnerWhyPickedIsCreatingSuggestion(t *testing.T) {
t.Errorf("why_picked = %v, want %q", got.WhyPicked, reason) 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)
}
}
}
+1 -1
View File
@@ -73,7 +73,7 @@ func newIdeaActionsRouter(t *testing.T, model, normalize *llm.Fake, search *reci
t.Error("draft_week ran: an idea action must never draft a plan") t.Error("draft_week ran: an idea action must never draft a plan")
return skills.Week{}, context.Canceled return skills.Week{}, context.Canceled
} }
planSvc, err := plan.NewService(st, draft, runner, newTestGoogle(t), notifytest.New(t, st), func() {}, &url.URL{Scheme: "https", Host: "mealquest.example"}, logger) planSvc, err := plan.NewService(st, draft, runner, newTestGoogle(t), notifytest.New(t, st), func() {}, &url.URL{Scheme: "https", Host: "mealquest.example"}, time.Now, logger)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
+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",
+2 -1
View File
@@ -14,6 +14,7 @@ import (
"sync" "sync"
"testing" "testing"
"testing/fstest" "testing/fstest"
"time"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/skills" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/skills"
@@ -93,7 +94,7 @@ func newPlansRouterTitled(t *testing.T, title shopping.Titler) plansEnv {
t.Fatal(err) t.Fatal(err)
} }
notifier := notifytest.New(t, st) notifier := notifytest.New(t, st)
svc, err := plan.NewService(st, fail, runner, googleSvc, notifier, func() {}, pub, logger) svc, err := plan.NewService(st, fail, runner, googleSvc, notifier, func() {}, pub, time.Now, logger)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
+3 -2
View File
@@ -15,6 +15,7 @@ import (
"strings" "strings"
"testing" "testing"
"testing/fstest" "testing/fstest"
"time"
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"golang.org/x/oauth2" "golang.org/x/oauth2"
@@ -240,7 +241,7 @@ func newTestPlan(t *testing.T) *plan.Service {
fail := func(context.Context, string, skills.WeekRequest) (skills.Week, error) { fail := func(context.Context, string, skills.WeekRequest) (skills.Week, error) {
return skills.Week{}, errors.New("no model in this test") return skills.Week{}, errors.New("no model in this test")
} }
svc, err := plan.NewService(st, fail, runner, newTestGoogle(t), notifytest.New(t, st), func() {}, &url.URL{Scheme: "https", Host: "mealquest.example"}, logger) svc, err := plan.NewService(st, fail, runner, newTestGoogle(t), notifytest.New(t, st), func() {}, &url.URL{Scheme: "https", Host: "mealquest.example"}, time.Now, logger)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -303,7 +304,7 @@ func newTestHomeOn(t *testing.T, st *store.Store) *home.Service {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
planSvc, err := plan.NewService(st, draft, runner, googleSvc, notifier, func() {}, pub, logger) planSvc, err := plan.NewService(st, draft, runner, googleSvc, notifier, func() {}, pub, time.Now, logger)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
+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())
}
}
+93 -4
View File
@@ -176,8 +176,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) {
if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed" { if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique" {
t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed", got) t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique", got)
} }
} }
@@ -197,8 +197,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) {
if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed" { if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique" {
t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed", got) t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique", got)
} }
if !tableExists(t, s, "dinners") { if !tableExists(t, s, "dinners") {
t.Error("table from 0001_init was not created") t.Error("table from 0001_init was not created")
@@ -922,3 +922,92 @@ func TestMigrateShoppingListChatSendUnconfirmed(t *testing.T) {
t.Errorf("shopping list after 0017 = %s, want %s", got, want) t.Errorf("shopping list after 0017 = %s, want %s", got, want)
} }
} }
// seedRecipeItems inserts two recipes, each with an ingredient and a step
// at position 0, then the extra statements.
func seedRecipeItems(t *testing.T, s *Store, extra ...string) {
t.Helper()
stmts := []string{
`INSERT INTO recipes (id, source_url, title, raw_jsonld, fetched_at, created_at, updated_at)
VALUES ('recipe-1', 'https://example.com/chili', 'Chili', '{}', 'f', 'c', 'u')`,
`INSERT INTO recipes (id, source_url, title, raw_jsonld, fetched_at, created_at, updated_at)
VALUES ('recipe-2', 'https://example.com/curry', 'Curry', '{}', 'f', 'c', 'u')`,
`INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-1', 'recipe-1', 0, '1 onion', 'c', 'u'), ('ing-2', 'recipe-2', 0, '1 lime', 'c', 'u')`,
`INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-1', 'recipe-1', 0, 'Chop.', 'c', 'u'), ('step-2', 'recipe-2', 0, 'Squeeze.', 'c', 'u')`,
}
for _, stmt := range append(stmts, extra...) {
if _, err := s.Writer.Exec(stmt); err != nil {
t.Fatalf("seed: %v", err)
}
}
}
// TestMigrateRecipeItemPositionUnique applies 0018 over two recipes with
// items at the same position: each recipe keeps its items, and a second
// ingredient or step at a position its recipe already uses is refused.
func TestMigrateRecipeItemPositionUnique(t *testing.T) {
ctx := context.Background()
s := openTestStore(t)
migrateBefore(t, s, "0018")
seedRecipeItems(t, s)
if err := Migrate(ctx, s.Writer, migrations.FS); err != nil {
t.Fatal(err)
}
var got string
if err := s.Reader.QueryRowContext(ctx, `SELECT
(SELECT group_concat(id || '@' || recipe_id || ':' || position, ',') FROM (SELECT * FROM recipe_ingredients ORDER BY id)) || ' / ' ||
(SELECT group_concat(id || '@' || recipe_id || ':' || position, ',') FROM (SELECT * FROM recipe_steps ORDER BY id))`).Scan(&got); err != nil {
t.Fatal(err)
}
if want := "ing-1@recipe-1:0,ing-2@recipe-2:0 / step-1@recipe-1:0,step-2@recipe-2:0"; got != want {
t.Errorf("recipe items after 0018 = %s, want %s", got, want)
}
for _, tc := range []struct{ name, stmt string }{
{"second ingredient at a position", `INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-3', 'recipe-1', 0, '2 onions', 'c', 'u')`},
{"second step at a position", `INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-3', 'recipe-1', 0, 'Dice.', 'c', 'u')`},
{"ingredient moved onto another's position", `UPDATE recipe_ingredients SET recipe_id = 'recipe-1' WHERE id = 'ing-2'`},
{"step moved onto another's position", `UPDATE recipe_steps SET recipe_id = 'recipe-1' WHERE id = 'step-2'`},
} {
if _, err := s.Writer.Exec(tc.stmt); err == nil || !strings.Contains(err.Error(), "UNIQUE constraint failed") {
t.Errorf("%s: error %v, want UNIQUE constraint failed", tc.name, err)
}
}
}
// TestMigrateRecipeItemPositionUniqueRefusesDuplicate applies 0018 over a
// recipe already holding two ingredients, or two steps, at one position:
// its order is unknown, so the migration fails and is not recorded.
func TestMigrateRecipeItemPositionUniqueRefusesDuplicate(t *testing.T) {
for _, tc := range []struct{ table, stmt string }{
{"recipe_ingredients", `INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-3', 'recipe-1', 0, '2 onions', 'c', 'u')`},
{"recipe_steps", `INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-3', 'recipe-1', 0, 'Dice.', 'c', 'u')`},
} {
t.Run(tc.table, func(t *testing.T) {
ctx := context.Background()
s := openTestStore(t)
migrateBefore(t, s, "0018")
seedRecipeItems(t, s, tc.stmt)
err := Migrate(ctx, s.Writer, migrations.FS)
if err == nil || !strings.Contains(err.Error(), "UNIQUE constraint failed: "+tc.table+".recipe_id, "+tc.table+".position") {
t.Fatalf("Migrate = %v, want the duplicate position refused", err)
}
var n int
if err := s.Reader.QueryRowContext(ctx, `SELECT count(*) FROM schema_migrations WHERE version = '0018_recipe_item_position_unique'`).Scan(&n); err != nil {
t.Fatal(err)
}
if n != 0 {
t.Error("0018 recorded as applied")
}
})
}
}
+4
View File
@@ -408,6 +408,8 @@ func TestSchemaUniqueConstraintsRejectDuplicates(t *testing.T) {
{"notification_outbox", []string{"event_key", "channel"}}, {"notification_outbox", []string{"event_key", "channel"}},
{"schedule_runs", []string{"schedule", "period_key"}}, {"schedule_runs", []string{"schedule", "period_key"}},
{"notification_prefs", []string{"event"}}, {"notification_prefs", []string{"event"}},
{"recipe_ingredients", []string{"recipe_id", "position"}},
{"recipe_steps", []string{"recipe_id", "position"}},
} }
s := seededStore(t) s := seededStore(t)
for _, c := range cases { for _, c := range cases {
@@ -445,6 +447,8 @@ func TestSchemaCompositeUniqueAllowsPartialOverlap(t *testing.T) {
{"same plan_date, different dinner", "rotation_reviews", "plan_date", "dinners", map[string]any{"dinner_id": rowID("dinners", 1)}}, {"same plan_date, different dinner", "rotation_reviews", "plan_date", "dinners", map[string]any{"dinner_id": rowID("dinners", 1)}},
{"same event_key, different channel", "notification_outbox", "event_key", "", map[string]any{"channel": "chat"}}, {"same event_key, different channel", "notification_outbox", "event_key", "", map[string]any{"channel": "chat"}},
{"same period_key, different schedule", "schedule_runs", "period_key", "", map[string]any{"schedule": "week_rollover"}}, {"same period_key, different schedule", "schedule_runs", "period_key", "", map[string]any{"schedule": "week_rollover"}},
{"same position, different recipe", "recipe_ingredients", "position", "recipes", map[string]any{"recipe_id": rowID("recipes", 1)}},
{"same position, different recipe", "recipe_steps", "position", "recipes", map[string]any{"recipe_id": rowID("recipes", 1)}},
} }
s := seededStore(t) s := seededStore(t)
for _, c := range cases { for _, c := range cases {
+1 -1
View File
@@ -1157,7 +1157,7 @@ func TestLaterBatchNeverResuggestsRenamedDrop(t *testing.T) {
fail := func(context.Context, string, skills.WeekRequest) (skills.Week, error) { fail := func(context.Context, string, skills.WeekRequest) (skills.Week, error) {
return skills.Week{}, errors.New("no draft in this test") return skills.Week{}, errors.New("no draft in this test")
} }
planSvc, err := plan.NewService(e.st, fail, e.runner, noCalendar{}, notifytest.New(t, e.st), func() {}, &url.URL{Scheme: "https", Host: "meals.example.com"}, logger) planSvc, err := plan.NewService(e.st, fail, e.runner, noCalendar{}, notifytest.New(t, e.st), func() {}, &url.URL{Scheme: "https", Host: "meals.example.com"}, time.Now, logger)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -0,0 +1,9 @@
-- 0018_recipe_item_position_unique: position is the only record of recipe
-- order, so a recipe never has two ingredients or two steps at one
-- position. Writers replace a recipe's items whole, numbered from 0, and
-- the detail read orders by position alone. A database already holding a
-- duplicate fails this migration rather than keep an order nobody can
-- trust.
CREATE UNIQUE INDEX recipe_ingredients_recipe_position ON recipe_ingredients (recipe_id, position);
CREATE UNIQUE INDEX recipe_steps_recipe_position ON recipe_steps (recipe_id, position);
+2 -2
View File
@@ -48,7 +48,7 @@ INSERT INTO recipe_ingredients (
-- name: ListRecipeIngredients :many -- name: ListRecipeIngredients :many
SELECT * FROM recipe_ingredients SELECT * FROM recipe_ingredients
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id; ORDER BY position;
-- name: DeleteRecipeIngredients :exec -- name: DeleteRecipeIngredients :exec
DELETE FROM recipe_ingredients DELETE FROM recipe_ingredients
@@ -64,7 +64,7 @@ INSERT INTO recipe_steps (
-- name: ListRecipeSteps :many -- name: ListRecipeSteps :many
SELECT * FROM recipe_steps SELECT * FROM recipe_steps
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id; ORDER BY position;
-- name: DeleteRecipeSteps :exec -- name: DeleteRecipeSteps :exec
DELETE FROM recipe_steps DELETE FROM recipe_steps