Make recipe repairs conditional and keep pasted links out of logs

F1: A candidate or link repair now writes the dinner only if it is still unarchived and as observed before grounding (normalized_name, grounding_status, grounding_revision and recipe_id). A dinner renamed or grounded some other way meanwhile is ErrGroundConflict (409 ground_conflict); one archived, before or during the repair, is ErrArchived (409 dinner_archived). Manual repairs reject archived dinners too. F2: Any failure grounding a candidate or link by hand wraps ErrGroundFailed, which the API logs as reason=ground_failed with the dinner id only, never the pasted link or dependency error text. GroundURL no longer formats the link into its error. Maestro task 8741cedb-1849-4e3f-8d05-f5648c77c7ee. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-06 20:03:05 -04:00
parent cc7875c76c
commit 5c4c507ee6
9 changed files with 291 additions and 55 deletions
@@ -76,6 +76,18 @@ var groundTimeout = GroundTimeout
 // runs past GroundTimeout; the dinner is left as it was.
 var ErrGroundTimeout = errors.New("dinners: grounding the recipe took too long")

+// ErrGroundFailed wraps any failure grounding a candidate or link by hand.
+// Its text may quote the link and dependency errors, so it is never
+// logged.
+var ErrGroundFailed = errors.New("dinners: grounding the recipe failed")
+
+// ErrArchived is grounding an archived dinner by hand.
+var ErrArchived = errors.New("dinner archived")
+
+// ErrGroundConflict is a candidate or link grounded for a dinner that was
+// renamed or grounded some other way while it was grounding.
+var ErrGroundConflict = errors.New("dinner changed while its recipe was grounding")
+
 // GroundJob is the KindGroundDinner payload: the dinner to ground and its
 // grounding_revision when the job was enqueued.
 type GroundJob struct {
@@ -326,8 +338,11 @@ type ManualRecipe struct {
 // difficulty. A manual recipe is stored as typed — no model runs — and
 // sets grounding_status manual with its method and minutes; it has no
 // photo, and the dinner keeps its difficulty. An invalid f is a
-// *ValidationError; a page that cannot ground the dinner is a
-// *recipes.ErrUngroundable and changes nothing.
+// *ValidationError and an archived dinner ErrArchived. A failure grounding
+// a candidate or link wraps ErrGroundFailed; a page that cannot ground the
+// dinner is also a *recipes.ErrUngroundable. A dinner renamed, archived or
+// grounded some other way while a candidate or link was grounding is
+// ErrGroundConflict or ErrArchived. None of these changes the dinner.
 func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string) (Dinner, error) {
 	if updatedBy == "" {
 		return Dinner{}, errors.New("dinners: ground: no member to record as updated_by")
@@ -339,9 +354,13 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
 	if manual != nil {
 		return s.groundManual(ctx, id, *manual, updatedBy)
 	}
-	if _, err := s.get(ctx, id); err != nil {
+	cur, err := s.get(ctx, id)
+	if err != nil {
 		return Dinner{}, err
 	}
+	if cur.Status == StatusArchived {
+		return Dinner{}, ErrArchived
+	}
 	groundCtx, cancel := context.WithTimeout(ctx, groundTimeout)
 	defer cancel()
 	gr, err := s.grounder.GroundURL(groundCtx, link)
@@ -349,21 +368,25 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
 		if ctx.Err() == nil && errors.Is(groundCtx.Err(), context.DeadlineExceeded) {
 			return Dinner{}, ErrGroundTimeout
 		}
-		return Dinner{}, fmt.Errorf("dinners: ground %s: %w", id, err)
+		return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
 	}
 	row, err := db.New(s.st.Writer).GroundDinner(ctx, db.GroundDinnerParams{
-		RecipeID:        sql.NullString{String: gr.RecipeID, Valid: true},
-		PhotoID:         sql.NullString{String: gr.PhotoID, Valid: true},
-		Method:          sql.NullString{String: gr.Method, Valid: true},
-		MinutesTotal:    nullInt(gr.MinutesTotal),
-		Difficulty:      sql.NullString{String: gr.Difficulty, Valid: true},
-		GroundingStatus: GroundingGrounded,
-		UpdatedBy:       sql.NullString{String: updatedBy, Valid: true},
-		UpdatedAt:       store.Now().String(),
-		ID:              id,
+		RecipeID:                  sql.NullString{String: gr.RecipeID, Valid: true},
+		PhotoID:                   sql.NullString{String: gr.PhotoID, Valid: true},
+		Method:                    sql.NullString{String: gr.Method, Valid: true},
+		MinutesTotal:              nullInt(gr.MinutesTotal),
+		Difficulty:                sql.NullString{String: gr.Difficulty, Valid: true},
+		GroundingStatus:           GroundingGrounded,
+		UpdatedBy:                 sql.NullString{String: updatedBy, Valid: true},
+		UpdatedAt:                 store.Now().String(),
+		ID:                        cur.ID,
+		ObservedNormalizedName:    cur.NormalizedName,
+		ObservedGroundingStatus:   cur.GroundingStatus,
+		ObservedGroundingRevision: cur.GroundingRevision,
+		ObservedRecipeID:          cur.RecipeID,
 	})
 	if errors.Is(err, sql.ErrNoRows) {
-		return Dinner{}, ErrNotFound
+		return Dinner{}, s.groundRejected(ctx, id)
 	}
 	if err != nil {
 		return Dinner{}, fmt.Errorf("dinners: ground %s: %w", id, err)
@@ -371,6 +394,19 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
 	return fromRow(row), nil
 }

+// groundRejected is the error for dinner id when it was not as observed
+// before grounding: ErrNotFound, ErrArchived or ErrGroundConflict.
+func (s *Service) groundRejected(ctx context.Context, id string) error {
+	cur, err := s.get(ctx, id)
+	if err != nil {
+		return err
+	}
+	if cur.Status == StatusArchived {
+		return ErrArchived
+	}
+	return ErrGroundConflict
+}
+
 // groundManual stores m as dinner id's recipe, with its ingredients and
 // steps, and records it on the dinner, all in one transaction.
 func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, updatedBy string) (Dinner, error) {
@@ -384,6 +420,9 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u
 		if err != nil {
 			return fmt.Errorf("dinners: read %s: %w", id, err)
 		}
+		if cur.Status == StatusArchived {
+			return ErrArchived
+		}
 		ld, err := json.Marshal(manualJSONLD{
 			Context:      "https://schema.org",
 			Type:         "Recipe",
@@ -439,14 +478,18 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u
 			}
 		}
 		row, err = q.GroundDinner(ctx, db.GroundDinnerParams{
-			RecipeID:        sql.NullString{String: rec.ID, Valid: true},
-			Method:          method,
-			MinutesTotal:    minutes,
-			Difficulty:      cur.Difficulty,
-			GroundingStatus: GroundingManual,
-			UpdatedBy:       sql.NullString{String: updatedBy, Valid: true},
-			UpdatedAt:       now,
-			ID:              cur.ID,
+			RecipeID:                  sql.NullString{String: rec.ID, Valid: true},
+			Method:                    method,
+			MinutesTotal:              minutes,
+			Difficulty:                cur.Difficulty,
+			GroundingStatus:           GroundingManual,
+			UpdatedBy:                 sql.NullString{String: updatedBy, Valid: true},
+			UpdatedAt:                 now,
+			ID:                        cur.ID,
+			ObservedNormalizedName:    cur.NormalizedName,
+			ObservedGroundingStatus:   cur.GroundingStatus,
+			ObservedGroundingRevision: cur.GroundingRevision,
+			ObservedRecipeID:          cur.RecipeID,
 		})
 		if err != nil {
 			return fmt.Errorf("dinners: ground %s: %w", id, err)
@@ -1106,3 +1106,103 @@ func TestGroundTimesOut(t *testing.T) {
 		t.Errorf("grounding_status = %s, want failed as before", row.GroundingStatus)
 	}
 }
+
+// TestFixInFlightWhenDinnerChanges: a link still grounding when its dinner
+// is renamed, archived or grounded some other way writes nothing; the
+// change made meanwhile is kept.
+func TestFixInFlightWhenDinnerChanges(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name   string
+		change func(t *testing.T, e *env, id string)
+		want   error
+		// kept checks the dinner shows the change, not the link's recipe.
+		kept func(row db.Dinner) bool
+	}{
+		{"renamed", func(t *testing.T, e *env, id string) {
+			if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Name: ptr("Chicken curry")}, member); err != nil {
+				t.Fatal(err)
+			}
+		}, dinners.ErrGroundConflict, func(row db.Dinner) bool {
+			return row.Name == "Chicken curry" && row.GroundingStatus == dinners.GroundingFailed && !row.RecipeID.Valid
+		}},
+		{"archived", func(t *testing.T, e *env, id string) {
+			if _, err := e.svc.Archive(t.Context(), id, member); err != nil {
+				t.Fatal(err)
+			}
+		}, dinners.ErrArchived, func(row db.Dinner) bool {
+			return row.Status == dinners.StatusArchived && row.GroundingStatus == dinners.GroundingFailed && !row.RecipeID.Valid
+		}},
+		{"grounded by hand", func(t *testing.T, e *env, id string) {
+			if _, err := e.svc.Ground(t.Context(), id, dinners.Fix{Manual: &dinners.ManualRecipe{
+				Ingredients: []string{"1 lb beef"}, Minutes: ptr(240), Method: ptr("crockpot"),
+			}}, member); err != nil {
+				t.Fatal(err)
+			}
+		}, dinners.ErrGroundConflict, func(row db.Dinner) bool {
+			return row.GroundingStatus == dinners.GroundingManual && row.Method.String == "crockpot"
+		}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			e := newEnv(t)
+			id := failedDinner(t, e, "Grandma's stew")
+			e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
+			e.pages.Gate = make(chan struct{})
+
+			done := make(chan error, 1)
+			go func() {
+				_, err := e.svc.Ground(context.Background(), id, dinners.Fix{RecipeURL: ptr(stew)}, member)
+				done <- err
+			}()
+			e.waitFetched(t, stew)
+			tt.change(t, e, id)
+			close(e.pages.Gate)
+
+			if err := <-done; !errors.Is(err, tt.want) {
+				t.Fatalf("err = %v, want %v", err, tt.want)
+			}
+			if row := e.dinner(t, id); !tt.kept(row) {
+				t.Errorf("dinner = %+v, want the %s change kept and the link's recipe not written", row, tt.name)
+			}
+		})
+	}
+}
+
+// TestFixArchivedDinner: an archived dinner cannot be grounded by hand.
+func TestFixArchivedDinner(t *testing.T) {
+	t.Parallel()
+	e := newEnv(t)
+	id := failedDinner(t, e, "Grandma's stew")
+	if _, err := e.svc.Archive(t.Context(), id, member); err != nil {
+		t.Fatal(err)
+	}
+	before := e.dinner(t, id)
+	for _, f := range []dinners.Fix{
+		{RecipeURL: ptr(stew)},
+		{Manual: &dinners.ManualRecipe{Ingredients: []string{"1 lb beef"}, Minutes: ptr(240), Method: ptr("crockpot")}},
+	} {
+		if _, err := e.svc.Ground(t.Context(), id, f, member); !errors.Is(err, dinners.ErrArchived) {
+			t.Errorf("fix %+v: err = %v, want ErrArchived", f, err)
+		}
+	}
+	if got := e.pages.Fetched(); len(got) != 0 {
+		t.Errorf("fetched %q for an archived dinner", got)
+	}
+	if after := e.dinner(t, id); after != before {
+		t.Errorf("archived dinner changed: %+v, was %+v", after, before)
+	}
+}
+
+// waitFetched waits until rawURL has been fetched.
+func (e *env) waitFetched(t *testing.T, rawURL string) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for !slices.Contains(e.pages.Fetched(), rawURL) {
+		if time.Now().After(deadline) {
+			t.Fatalf("%s never fetched", rawURL)
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+}