From 2e0a7d1cbd2d3ab958027f065b9ebabdb992e025 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 14:14:07 +0200 Subject: [PATCH] Review: an answered retry never brings its old refusal back (issue 369) A refusal stood for its part whenever no ask was open, so after the retry was taken and answered, "Questions for you not delivered" came back with the old words for an hour, then again after each answer. A refusal is now current only while no ask was made after it that the router did not refuse; the verdict wait applies only to that newer ask. Tests reconcile inside the wait and after an answer. --- cmd/mesh-controller/asker.go | 22 +++++++++++++++++----- cmd/mesh-controller/asker_test.go | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/cmd/mesh-controller/asker.go b/cmd/mesh-controller/asker.go index ce15ed26..bec3562e 100644 --- a/cmd/mesh-controller/asker.go +++ b/cmd/mesh-controller/asker.go @@ -370,8 +370,17 @@ func (a *asker) reconcile(ctx context.Context) error { } } // Refusals in a row, by part: those since the last ask that was not refused, within askRefusedCounted. - inRow := map[string]int{} + // superseded: a part asked since its last refusal, by an ask the router did not refuse — open, answered, + // expired or cancelled. Its refusal is history then, never said again (the review of PR 198: an answered + // retry brought the refusal back). + inRow, superseded := map[string]int{}, map[string]asked{} for k, last := range refused { + for _, r := range all { + if partKey(r.Condition, r.Part) == k && r.State != string(asks.OutcomeRefused) && r.State != askUnsent && + r.Opened.After(last.Opened) && r.Opened.After(superseded[k].Opened) { + superseded[k] = r + } + } var since time.Time for _, r := range all { if partKey(r.Condition, r.Part) == k && r.State != string(asks.OutcomeRefused) && r.State != askUnsent && @@ -425,10 +434,13 @@ func (a *asker) reconcile(ctx context.Context) error { if ended.IsZero() { ended = r.Opened } - waiting := !held && r.Channels == channels && now.Sub(ended) < refusedRetryAfter(inRow[key]) - // Asked again now, or lately and the router's word not in yet: said as it was until that word. - verdictDue := !held || (cur.Opened.After(r.Opened) && now.Sub(cur.Opened) < askVerdictWait) - if waiting || verdictDue { + later, asked := superseded[key] + waiting := !held && !asked && r.Channels == channels && now.Sub(ended) < refusedRetryAfter(inRow[key]) + // Asked again now (the wait over), or lately and the router's word not in yet: said as it was until + // that word, so the condition neither clears nor is raised again at each try. + retrying := !held && !asked && !waiting + verdictDue := held && asked && later.ID == cur.ID && now.Sub(cur.Opened) < askVerdictWait + if waiting || retrying || verdictDue { if !saidUnasked { unasked, saidUnasked = append(unasked, c), true } diff --git a/cmd/mesh-controller/asker_test.go b/cmd/mesh-controller/asker_test.go index dde54dab..96654618 100644 --- a/cmd/mesh-controller/asker_test.go +++ b/cmd/mesh-controller/asker_test.go @@ -760,6 +760,11 @@ func TestARefusalIsAskedAgainAndTheUndeliveredConditionClearsOnceTaken(t *testin if got := last(); len(got) != 1 { t.Fatalf("cleared while the router's word on the new ask was awaited: the condition would flap") } + r.now = r.now.Add(askVerdictWait / 2) + _ = r.a.reconcile(context.Background()) + if got := last(); len(got) != 1 { + t.Fatalf("cleared inside the verdict wait: the condition would flap") + } // Refused again, now for a reason of today: said in those words. refuse("no channel can carry any of its answers now: telegram: no account is linked") _ = r.a.reconcile(context.Background()) @@ -781,6 +786,19 @@ func TestARefusalIsAskedAgainAndTheUndeliveredConditionClearsOnceTaken(t *testin if n := len(r.asksSent(t)); n != 3 { t.Fatalf("an ask taken was asked again: %d asks", n) } + // The operator answers it; the condition stays open (its fix takes a while): the old refusal is not said again. + taken := r.asksSent(t)[2] + r.store.Change(context.Background(), taken.ID, func(x *asked) bool { + x.State, x.Ended = string(asks.OutcomeChosen), r.now + return true + }) + for i := 0; i < 5; i++ { + r.now = r.now.Add(askEvery) + _ = r.a.reconcile(context.Background()) + if got := last(); len(got) != 0 { + t.Fatalf("an answered ask brought its old refusal back: %+v", got) + } + } // Its explanation opens with one verdict, not two. r.open = append(r.open, heldCondition()) _ = r.a.reconcile(context.Background())