From 471daec594ae01ac0d21f0b7290ea9b8cb010bf6 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Tue, 6 Oct 2026 22:58:43 -0400 Subject: [PATCH] Refuse expired candidate offers at lookup time Store each offer set's deadline and check it under offersMu in candidateURL, so an expired candidate id cannot ground a dinner while its eviction timer is delayed. The timer still evicts the set from memory. Co-Authored-By: Claude Opus 5.5 --- internal/dinners/export_test.go | 9 +++++++++ internal/dinners/grounding.go | 19 ++++++++++++------- internal/dinners/grounding_test.go | 17 +++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/internal/dinners/export_test.go b/internal/dinners/export_test.go index ef7d545..cf7b488 100644 --- a/internal/dinners/export_test.go +++ b/internal/dinners/export_test.go @@ -22,6 +22,15 @@ func SetOfferLifetime(t *testing.T, d time.Duration) { t.Cleanup(func() { offerLifetime = old }) } +// AdvanceClock moves the service's clock d ahead of real time until t +// ends, without firing any timer. A test calling it must not run in +// parallel. +func AdvanceClock(t *testing.T, d time.Duration) { + old := now + now = func() time.Time { return old().Add(d) } + t.Cleanup(func() { now = old }) +} + // HasOffers reports whether the service holds candidates offered for // dinner id. func (s *Service) HasOffers(id string) bool { diff --git a/internal/dinners/grounding.go b/internal/dinners/grounding.go index 0aaed08..60fe2f0 100644 --- a/internal/dinners/grounding.go +++ b/internal/dinners/grounding.go @@ -300,12 +300,16 @@ type Candidate struct { // ground it; tests shorten it. var offerLifetime = 30 * time.Minute +// now is the clock offer lifetimes are measured on; tests advance it. +var now = time.Now + // offerSet is the candidates last offered for a dinner: the page URL of -// each candidate id. expiry drops the set from offers once offerLifetime -// has passed. +// each candidate id. They ground the dinner until deadline; expiry drops +// the set from offers once it has passed. type offerSet struct { - urls map[string]string - expiry *time.Timer + urls map[string]string + deadline time.Time + expiry *time.Timer } // offerSearch is a Candidates search running for a dinner. It is stale @@ -368,7 +372,7 @@ func (s *Service) trackOffers(id string) (keep func(urls map[string]string), don return } s.dropOffers(id) - set := &offerSet{urls: urls} + set := &offerSet{urls: urls, deadline: now().Add(offerLifetime)} // expiry waits for offersMu, so it finds set stored even when it // fires at once, and drops set only, never offers stored later. set.expiry = time.AfterFunc(offerLifetime, func() { @@ -423,12 +427,13 @@ func (s *Service) candidateID(id, rawURL string) string { // candidateURL is the page URL of candidate cid last offered for dinner id. // A cid not among the dinner's live offers, or whose page is not a valid -// recipe link, is a *ValidationError. +// recipe link, is a *ValidationError. Offers past their deadline are not +// live, even before their expiry drops them. func (s *Service) candidateURL(id, cid string) (string, error) { var raw string var ok bool s.offersMu.Lock() - if set := s.offers[id]; set != nil { + if set := s.offers[id]; set != nil && now().Before(set.deadline) { raw, ok = set.urls[cid] } s.offersMu.Unlock() diff --git a/internal/dinners/grounding_test.go b/internal/dinners/grounding_test.go index 5c21c6d..4604419 100644 --- a/internal/dinners/grounding_test.go +++ b/internal/dinners/grounding_test.go @@ -941,6 +941,23 @@ func TestCandidatesExpire(t *testing.T) { wantStaleCandidate(t, e, id, cid) } +// TestCandidatesExpireBeforeEviction: once their lifetime passes, a +// dinner's candidates no longer ground it, even while the service still +// holds them because their eviction has not run yet. +func TestCandidatesExpireBeforeEviction(t *testing.T) { + dinners.SetOfferLifetime(t, time.Hour) + e := newEnv(t) + id := failedDinner(t, e, "Mystery curry") + e.search.Results["Mystery curry"] = []recipes.Candidate{{URL: curryA}} + cid := candidateID(t, e, id, "Chicken Curry") + + dinners.AdvanceClock(t, time.Hour) + if !e.svc.HasOffers(id) { + t.Fatal("candidates evicted already, want them held until their timer fires") + } + wantStaleCandidate(t, e, id, cid) +} + // TestExpiredCandidatesNotHeld: the service holds no dinner's candidates // past their lifetime, however many dinners were offered some. func TestExpiredCandidatesNotHeld(t *testing.T) {