Merge pull request 'mesh-knowledge: a review owed says when it is due, survives a restart, and its last try is told to the operator (hq issue 497)' (#219) from fix/497-a-review-owed-outlives-a-restart into main

modules/mesh-knowledge/README.md

@@ -104,8 +104,12 @@ as text. `get` serves each, and any other field a record of hq wrote under `fiel
      Its author revises it by proposing it again, and that version is reviewed again.
    - A judgement that cannot be had (the judge unreachable, or an answer that is not `approve`, or `changes` naming
      no finding) leaves the record proposed and says so on it (`get`'s `review`). It is never taken as an
-     approval. It is asked again after 10, then 20 minutes; after 3 tries the record says its author proposes it
-     again to have it reviewed again, and the module is unhealthy, naming it, so the mesh raises a condition.
+     approval. It is owed: its try count and when it is asked again (10, then 20 minutes later) are kept with the
+     version in the store and said in its history, so the background work takes it up at that time, also after a
+     restart of the tool runner (hq issue 497). After 3 tries the record says it was the last and its author
+     proposes it again to have it reviewed again; the operator is told once on the operator's channel (keyed by
+     record and version, and cleared once the record is proposed again, accepted, withdrawn or superseded), and
+     the module is unhealthy, naming it, so the mesh raises a condition.
      An ask of the operator that cannot be made counts as a try the same way.
    - A review whose outcome the database refused to keep is held back for that record alone (10, then 20
      minutes, 3 tries, logged as given up), so it never blocks the records behind it.

modules/mesh-knowledge/cmd/mesh-knowledge/answers.go

@@ -770,11 +770,6 @@ func reviewSays(r *Record) string {
 	case JudgingUnproven:
 		return "not reviewed: " + j.Reason
 	}
-	s := fmt.Sprintf("not reviewed: %s (try %d of %d); it stays proposed, and no judgement that could not be had counts as an approval", j.Reason, j.Tries, MostReviewTries)
-	if j.Tries >= MostReviewTries {
-		s += "; its author proposes it again to have it reviewed again"
-	} else {
-		s += "; asked again after a pause"
-	}
-	return s
+	// When it is asked again is said, so a pause is not taken for a review lost (novox/hq issue 497).
+	return fmt.Sprintf("not reviewed: %s (%s; no judgement that could not be had counts as an approval", j.Reason, j.owedSays())
 }

modules/mesh-knowledge/cmd/mesh-knowledge/draft.go

@@ -356,7 +356,7 @@ func (k *KB) tellTopic(ctx context.Context, topic string) {
 	summary := fmt.Sprintf("The clauses of %d prose decisions on %s are drafted and checked: %d confirmed, %d still drafted.", records, topic, confirmed, drafted)
 	explanation := "A second judgement confirmed each clause against its record; a clause still drafted is never served as in force. " +
 		"You may send any record back with the knowledge base's return, and its clauses are drafted again. Nothing needs you otherwise."
-	if err := k.Notify("knowledge.clauses."+strings.ReplaceAll(topic, " ", "-"), summary, "Clauses drafted for "+topic, explanation); err != nil {
+	if err := k.Notify("knowledge.clauses."+strings.ReplaceAll(topic, " ", "-"), summary, "Clauses drafted for "+topic, explanation, ""); err != nil {
 		k.logf("[mesh-knowledge] the operator could not be told about %s's clauses: %v", topic, err)
 	}
 }

modules/mesh-knowledge/cmd/mesh-knowledge/kb.go

@@ -64,8 +64,12 @@ type Reserver func(kind, title string) (int, error)
 // the structured answer and who judged.
 type Judgement func(question string, schema map[string]any, model string) (json.RawMessage, string, error)
 
-// Notifier tells the operator's channel something (operator-channel.notify).
-type Notifier func(key, summary, headline, explanation string) error
+// Notifier tells the operator's channel something (operator-channel.notify). Resolved is the line its message says
+// once it is cleared: the channel keeps the one given when it is raised (novox/hq issue 497); "" for none.
+type Notifier func(key, summary, headline, explanation, resolved string) error
+
+// Clearer tells the operator's channel that what it was told under key is over, in one resolved line.
+type Clearer func(key, resolved string) error
 
 // KB is the knowledge base.
 type KB struct {
@@ -75,6 +79,7 @@ type KB struct {
 	Judge    Judgement
 	Asks     AskBus
 	Notify   Notifier
+	Clear    Clearer
 	Emit     func(event string, body any) error
 	Settings func() (Settings, error)
 	Now      func() time.Time

modules/mesh-knowledge/cmd/mesh-knowledge/kit_test.go

@@ -397,7 +397,11 @@ type world struct {
 	router  *router
 	said    *said
 	told    []string
-	clock   time.Time
+	// toldKeys are the keys told under, and cleared what was cleared: key, then its resolved line.
+	toldKeys []string
+	cleared  [][2]string
+	resolved []string // the resolved line given with each notice raised
+	clock    time.Time
 }
 
 func newWorld(t *testing.T, s Store, records Records) *world {
@@ -406,8 +410,12 @@ func newWorld(t *testing.T, s Store, records Records) *world {
 		clock: time.Date(2026, 10, 11, 9, 0, 0, 0, time.UTC)}
 	w.k = &KB{Store: s, Records: records, Reserve: w.tracker.reserve, Judge: w.judge.judge, Asks: w.router,
 		Emit: w.said.emit, Settings: func() (Settings, error) { return Settings{}, nil }, Now: func() time.Time { return w.clock },
-		Logf:   func(f string, a ...any) { t.Logf(f, a...) },
-		Notify: func(key, summary, headline, explanation string) error { w.told = append(w.told, summary); return nil }}
+		Logf: func(f string, a ...any) { t.Logf(f, a...) },
+		Notify: func(key, summary, headline, explanation, resolved string) error {
+			w.told, w.toldKeys, w.resolved = append(w.told, summary), append(w.toldKeys, key), append(w.resolved, resolved)
+			return nil
+		},
+		Clear: func(key, resolved string) error { w.cleared = append(w.cleared, [2]string{key, resolved}); return nil }}
 	return w
 }
 

modules/mesh-knowledge/cmd/mesh-knowledge/main.go

@@ -87,6 +87,7 @@ func newKB(store Store) *KB {
 		Judge:    busJudgement(os.Getenv("MESH_NODE")),
 		Asks:     sdkAskBus{},
 		Notify:   busNotifier,
+		Clear:    busClearer,
 		Emit:     func(event string, body any) error { return stdio.Emit(event, body) },
 		Settings: func() (Settings, error) { return readSettings(file) },
 		Logf:     logf,
@@ -224,9 +225,21 @@ func busJudgement(node string) Judgement {
 }
 
 // busNotifier tells the operator's channel, as a warning: it is the one severity below urgent.
-func busNotifier(key, summary, headline, explanation string) error {
+func busNotifier(key, summary, headline, explanation, resolved string) error {
+	body := map[string]any{"key": key, "subject": "the knowledge base",
+		"severity": "warning", "summary": summary, "headline": headline, "explanation": explanation}
+	if resolved != "" {
+		body["resolved"] = resolved
+	}
+	_, err := stdio.Ask("seat:operator-channel.notify", body)
+	return err
+}
+
+// busClearer tells the operator's channel that what it was told under key is over: the message is edited to say
+// resolved (novox/hq issue 497).
+func busClearer(key, resolved string) error {
 	_, err := stdio.Ask("seat:operator-channel.notify", map[string]any{"key": key, "subject": "the knowledge base",
-		"severity": "warning", "summary": summary, "headline": headline, "explanation": explanation})
+		"clear": true, "resolved": resolved})
 	return err
 }
 
@@ -329,6 +342,20 @@ func (l *lazyStore) Told(ctx context.Context, topic string) (bool, error) {
 	}
 	return s.Told(ctx, topic)
 }
+func (l *lazyStore) ToldOf(ctx context.Context, prefix string) ([]string, error) {
+	s, err := l.store(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return s.ToldOf(ctx, prefix)
+}
+func (l *lazyStore) Untold(ctx context.Context, key string) error {
+	s, err := l.store(ctx)
+	if err != nil {
+		return err
+	}
+	return s.Untold(ctx, key)
+}
 
 // ---- prepare, check, dry-run --------------------------------------------------------------------------------
 

modules/mesh-knowledge/cmd/mesh-knowledge/operator.go

@@ -141,13 +141,8 @@ func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string,
 	}
 	unasked := func(why string) (string, error) {
 		w.Ask = nil
-		w.Judging = &Judging{State: JudgingFailed, Tries: tries + 1, Reason: why, At: k.now()}
-		said := fmt.Sprintf("approved by the knowledge base's review, and %s (try %d of %d); it stays proposed", why, tries+1, MostReviewTries)
-		if tries+1 >= MostReviewTries {
-			said += ", and its author proposes it again to have it reviewed again"
-		} else {
-			said += " and is reviewed again after a pause"
-		}
+		w.Judging = failedJudging(tries+1, why, k.now())
+		said := fmt.Sprintf("approved by the knowledge base's review, and %s (%s", why, w.Judging.owedSays())
 		e, err := k.Store.Get(ctx, w.Key())
 		if err != nil || e == nil {
 			return "", fmt.Errorf("%s: %s, and that could not be kept (%v)", w.Key(), said, err)

modules/mesh-knowledge/cmd/mesh-knowledge/pgstore.go

@@ -549,6 +549,29 @@ func (s *PgStore) Findings(ctx context.Context) ([]Finding, error) {
 	return out, rows.Err()
 }
 
+func (s *PgStore) ToldOf(ctx context.Context, prefix string) ([]string, error) {
+	// left() and not like: a prefix may hold an underscore, which like would take for any character.
+	rows, err := s.pool.Query(ctx, `select topic from told where left(topic, length($1)) = $1 order by topic`, prefix)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	var out []string
+	for rows.Next() {
+		var k string
+		if err := rows.Scan(&k); err != nil {
+			return nil, err
+		}
+		out = append(out, k)
+	}
+	return out, rows.Err()
+}
+
+func (s *PgStore) Untold(ctx context.Context, key string) error {
+	_, err := s.pool.Exec(ctx, `delete from told where topic = $1`, key)
+	return err
+}
+
 func (s *PgStore) Told(ctx context.Context, topic string) (bool, error) {
 	tag, err := s.pool.Exec(ctx, `insert into told (topic) values ($1) on conflict do nothing`, topic)
 	if err != nil {

modules/mesh-knowledge/cmd/mesh-knowledge/review.go

@@ -9,8 +9,11 @@ package main
 //   - On changes the record stays proposed, the findings kept with the version they judged, and its author revises
 //     it with a new version, which is reviewed again.
 //   - A judgement that cannot be had — the judge unreachable, or its answer unreadable — leaves the record proposed
-//     and says so on it. It is never taken as an approval. It is asked again after a pause, up to MostReviewTries;
-//     after that the record says its author proposes it again to have it reviewed again.
+//     and says so on it. It is never taken as an approval. It is owed: the try count and when the next try is due
+//     are kept with the version in the store, so the background work takes it up at that time, also after a restart
+//     of the tool runner (novox/hq issue 497), up to MostReviewTries. After the last try the record says so, its
+//     author proposes it again to have it reviewed again, the module's health names it, and the operator is told
+//     once on the operator's channel.
 //   - A record with a version whose writer the runtime did not prove is not reviewed: who wrote it cannot be told,
 //     and no one but its proven author revises it, so its author proposes it again under a new number.
 //   - On approve, a record that does not need the operator is accepted at once; one its author marked
@@ -35,6 +38,41 @@ type Judging struct {
 	Tries  int       `json:"tries,omitempty"`
 	Reason string    `json:"reason,omitempty"`
 	At     time.Time `json:"at,omitempty"`
+	// Due is when a failed review is asked again; zero after its last try. It is kept in the store with the
+	// version, so a restart of the tool runner neither loses the try nor asks it early (novox/hq issue 497).
+	Due time.Time `json:"due,omitempty"`
+}
+
+// ReviewPause is the pause after a failed try of a review, times the tries so far.
+const ReviewPause = 10 * time.Minute
+
+// failedJudging is a review that could not be had, the tries'th time: owed again at a time it keeps, unless that
+// was its last try.
+func failedJudging(tries int, reason string, now time.Time) *Judging {
+	j := &Judging{State: JudgingFailed, Tries: tries, Reason: reason, At: now}
+	if tries < MostReviewTries {
+		j.Due = now.Add(time.Duration(tries) * ReviewPause)
+	}
+	return j
+}
+
+// due is when a failed review is asked again. A version kept before the time was kept (novox/hq issue 497) has
+// none, and is due after the same pause from its last try.
+func (j *Judging) due() time.Time {
+	if !j.Due.IsZero() {
+		return j.Due
+	}
+	return j.At.Add(time.Duration(j.Tries) * ReviewPause)
+}
+
+// owedSays is what a failed try says of what follows it: when it is asked again, or that it was the last.
+func (j *Judging) owedSays() string {
+	if j.Tries >= MostReviewTries {
+		return fmt.Sprintf("try %d of %d, the last); it stays proposed and nothing of it is in force, the operator is told, "+
+			"and its author proposes it again to have it reviewed again", j.Tries, MostReviewTries)
+	}
+	return fmt.Sprintf("try %d of %d); it stays proposed, and it is asked again at %s", j.Tries, MostReviewTries,
+		j.due().Local().Format("2006-01-02 15:04 MST"))
 }
 
 // The states of Judging.
@@ -162,7 +200,7 @@ func (k *KB) reviewDue(e *Entry) bool {
 	case JudgingDue:
 		return true
 	case JudgingFailed:
-		return w.Judging.Tries < MostReviewTries && k.now().Sub(w.Judging.At) >= time.Duration(w.Judging.Tries)*10*time.Minute
+		return w.Judging.Tries < MostReviewTries && !k.now().Before(w.Judging.due())
 	}
 	return false
 }
@@ -217,7 +255,12 @@ func (k *KB) ReviewStep() (string, error) {
 	if err != nil {
 		return "", err
 	}
+	k.clearUnreviewed(all)
 	for _, e := range all {
+		if ranOutOfTries(e.Working()) {
+			k.tellUnreviewed(e)
+			continue
+		}
 		if !k.reviewDue(e) || k.heldBack(e) {
 			continue
 		}
@@ -240,13 +283,119 @@ func Unreviewed(all []*Entry) []string {
 	var out []string
 	for _, e := range all {
 		w := e.Working()
-		if w.Status == StatusProposed && w.Judging != nil && w.Judging.State == JudgingFailed && w.Judging.Tries >= MostReviewTries {
+		if ranOutOfTries(w) {
 			out = append(out, fmt.Sprintf("%s (%s)", w.Key(), w.Judging.Reason))
 		}
 	}
 	return out
 }
 
+func ranOutOfTries(w *Record) bool {
+	return w.Status == StatusProposed && w.Judging != nil && w.Judging.State == JudgingFailed && w.Judging.Tries >= MostReviewTries
+}
+
+// unreviewedPrefix starts the key of every notice of a proposal that ran out of review tries. The notice's key is
+// unreviewedPrefix, the record and the version that recorded its last try (knowledge.unreviewed.decision-294.v9), so
+// a later version's notice is its own; what the store keeps as told is that key, a space and the last failed try's
+// time, which names the run of tries whatever versions are written after it (a link change writes one).
+const unreviewedPrefix = "knowledge.unreviewed."
+
+func unreviewedOf(k Key) string { return fmt.Sprintf("%s%s-%d.", unreviewedPrefix, k.Kind, k.Number) }
+
+// tellUnreviewed tells the operator, once for each run of tries, that a proposal ran out of review tries: a history
+// line alone is read by nobody, and a proposal never reviewed must not wait in silence (novox/hq issue 497).
+func (k *KB) tellUnreviewed(e *Entry) {
+	if k.Notify == nil {
+		return
+	}
+	ctx := context.Background()
+	w := e.Working()
+	at := w.Judging.At.UTC().Format(time.RFC3339Nano)
+	told, err := k.Store.ToldOf(ctx, unreviewedOf(w.Key()))
+	if err != nil {
+		k.logf("[mesh-knowledge] whether the operator was told that %s was never reviewed cannot be read: %v", w.Key(), err)
+		return
+	}
+	for _, t := range told {
+		if strings.HasSuffix(t, " "+at) {
+			return
+		}
+	}
+	version := e.CurrentVersion
+	if e.Pending != nil {
+		version = e.PendingVersion
+	}
+	key := fmt.Sprintf("%sv%d", unreviewedOf(w.Key()), version)
+	// Kept as told before the notice is sent, as for drafted clauses: a notice that fails is logged and not sent
+	// again, and the module's health still names the record, so the mesh's condition stands for it.
+	if first, err := k.Store.Told(ctx, key+" "+at); err != nil || !first {
+		return
+	}
+	summary := fmt.Sprintf("%s was never reviewed: its review could not be had %d times (last: %s). It stays proposed, and nothing of it is in force.",
+		w.Key(), w.Judging.Tries, w.Judging.Reason)
+	explanation := "No review that could not be had counts as an approval. Its author proposes it again to have it reviewed again; " +
+		"until then the knowledge base's health names it. The knowledge base's get and history show it."
+	// The channel edits a cleared message with the resolved line given here, not the one sent with the clear.
+	resolved := w.Key().String() + " no longer waits for its review"
+	if err := k.Notify(key, summary, "A proposal was never reviewed: "+w.Key().String(), explanation, resolved); err != nil {
+		k.logf("[mesh-knowledge] the operator could not be told that %s was never reviewed: %v; the module's health still names it", w.Key(), err)
+	}
+}
+
+// clearUnreviewed clears every notice of a proposal that ran out of review tries once the record has left that
+// state — proposed again, accepted, withdrawn or superseded — so the operator's message never keeps saying it waits
+// (novox/hq issue 497). A clear that fails is kept as told and sent again at the next step.
+func (k *KB) clearUnreviewed(all []*Entry) {
+	if k.Clear == nil {
+		return
+	}
+	ctx := context.Background()
+	told, err := k.Store.ToldOf(ctx, unreviewedPrefix)
+	if err != nil {
+		k.logf("[mesh-knowledge] the notices of proposals never reviewed cannot be read: %v", err)
+		return
+	}
+	byKey := map[string]*Entry{}
+	for _, e := range all {
+		byKey[unreviewedOf(e.Current.Key())] = e
+	}
+	for _, t := range told {
+		key, at, ok := strings.Cut(t, " ")
+		if !ok {
+			continue
+		}
+		var e *Entry
+		for prefix, x := range byKey {
+			if strings.HasPrefix(key, prefix) {
+				e = x
+				break
+			}
+		}
+		resolved := strings.TrimPrefix(key, unreviewedPrefix) + " is no longer held by the knowledge base"
+		if e != nil {
+			w := e.Working()
+			if ranOutOfTries(w) && w.Judging.At.UTC().Format(time.RFC3339Nano) == at {
+				continue
+			}
+			switch {
+			case w.Status == StatusProposed:
+				resolved = w.Key().String() + " was proposed again and is reviewed again"
+			case w.Accepted():
+				resolved = w.Key().String() + " was reviewed and is " + w.Status
+			default:
+				resolved = w.Key().String() + " no longer waits for its review: it is " + w.Status
+			}
+		}
+		if err := k.Clear(key, resolved); err != nil {
+			k.logf("[mesh-knowledge] the notice that %s was never reviewed could not be cleared: %v; it is cleared again at the next step", key, err)
+			continue
+		}
+		if err := k.Store.Untold(ctx, t); err != nil {
+			k.logf("[mesh-knowledge] the notice %s was cleared, and that could not be kept: %v", key, err)
+		}
+	}
+}
+
 // reviewOne asks the judgement on one record's working version, and acts on its verdict. The write lock is held
 // only to read and to write, never across the judgement; a record revised while it was judged is judged again.
 func (k *KB) reviewOne(key Key) (string, error) {
@@ -312,12 +461,8 @@ func (k *KB) reviewOne(key Key) (string, error) {
 	}
 	if err != nil {
 		// No verdict: the record stays proposed and says so. Never an approval.
-		w.Judging.State, w.Judging.Reason, w.Judging.At = JudgingFailed, err.Error(), k.now()
-		w.Judging.Tries++
-		why := fmt.Sprintf("not reviewed: %v (try %d of %d); it stays proposed", err, w.Judging.Tries, MostReviewTries)
-		if w.Judging.Tries >= MostReviewTries {
-			why += ", and its author proposes it again to have it reviewed again"
-		}
+		w.Judging = failedJudging(w.Judging.Tries+1, err.Error(), k.now())
+		why := fmt.Sprintf("not reviewed: %v (%s", err, w.Judging.owedSays())
 		by = ReviewBy
 		if serr := save(w, why); serr != nil {
 			return "", fmt.Errorf("%s: %s, and that could not be kept (%v)", key, why, serr)

modules/mesh-knowledge/cmd/mesh-knowledge/review_owed_test.go

@@ -0,0 +1,199 @@
+package main
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+	"time"
+)
+
+// novox/hq issue 497: a review that could not be had is owed. What is owed is kept in the knowledge base's store,
+// with its try count and when its next try is due, so the background work takes it up after a restart of the tool
+// runner as it would have without one; the history says when it is asked again, so nobody takes a pause for a
+// loss; and after its last try the history says so and the operator is told once. It is never an approval.
+func TestAReviewOwedOutlivesARestartAndItsLastTryIsSaid(t *testing.T) {
+	eachStore(t, func(t *testing.T, s Store) {
+		inZone(t)
+		w := imported(t, s)
+		n := proposed(t, w, "author@g14", false)
+		key := Key{KindDecision, n}
+		lastWhy := func() string {
+			vs, err := s.Versions(context.Background(), key)
+			if err != nil || len(vs) == 0 {
+				t.Fatalf("versions of %s: %v", key, err)
+			}
+			return vs[len(vs)-1].Why
+		}
+		var due time.Time
+		for try := 1; try <= MostReviewTries; try++ {
+			if try > 1 {
+				// The tool runner restarts: a new knowledge base over the same store, nothing of the old one's memory.
+				w = newWorld(t, s, fixture())
+				w.clock = due.Add(-time.Second)
+				if said := w.reviewAll(t); len(said) != 0 {
+					t.Fatalf("try %d asked before it was due: %v", try, said)
+				}
+				w.clock = due
+			}
+			w.judge.reviewFail = true
+			if said := w.reviewAll(t); len(said) != 1 || !strings.Contains(said[0], "not reviewed") {
+				t.Fatalf("try %d after a restart was not taken up: %v", try, said)
+			}
+			e := mustGet(t, s, key)
+			if e.Current.Status != StatusProposed || e.Current.Judging == nil || e.Current.Judging.State != JudgingFailed ||
+				e.Current.Judging.Tries != try || len(e.Current.Reviews) != 0 {
+				t.Fatalf("try %d is not counted in the store: %s, %+v", try, e.Current.Status, e.Current.Judging)
+			}
+			why := lastWhy()
+			if !strings.Contains(why, fmt.Sprintf("try %d of %d", try, MostReviewTries)) {
+				t.Fatalf("try %d: the history does not count it: %q", try, why)
+			}
+			if try < MostReviewTries {
+				due = w.clock.Add(time.Duration(try) * 10 * time.Minute)
+				when := "asked again at " + due.Local().Format("2006-01-02 15:04 MST")
+				if utc := due.UTC().Format("15:04"); strings.Contains(when, utc) || !strings.Contains(when, "CEST") {
+					t.Fatalf("the time is not said locally: %q (UTC %s)", when, utc)
+				}
+				if !strings.Contains(why, when) {
+					t.Fatalf("try %d: the history does not say when it is asked again (%q): %q", try, when, why)
+				}
+				if g, _ := w.k.Get(GetAsk{Number: n}); !strings.Contains(g.Review, when) {
+					t.Fatalf("try %d: get does not say when it is asked again (%q): %q", try, when, g.Review)
+				}
+				if len(w.told) != 0 {
+					t.Fatalf("try %d: the operator was told before the last try: %v", try, w.told)
+				}
+			}
+		}
+		// After its last try: the history says it was the last, and the operator is told, once.
+		if why := lastWhy(); !strings.Contains(why, "the last") || !strings.Contains(why, "the operator is told") {
+			t.Fatalf("the last try is not said in the history: %q", why)
+		}
+		if len(w.told) != 1 || !strings.Contains(w.told[0], key.String()) || !strings.Contains(w.told[0], "never reviewed") {
+			t.Fatalf("the operator is not told once of the last try: %v", w.told)
+		}
+		w.clock = w.clock.Add(24 * time.Hour)
+		if said := w.reviewAll(t); len(said) != 0 || len(w.told) != 1 {
+			t.Fatalf("after the last try: asked %v, told %v", said, w.told)
+		}
+		// Another restart neither asks it again nor tells the operator again.
+		clock := w.clock
+		w = newWorld(t, s, fixture())
+		w.clock = clock
+		if said := w.reviewAll(t); len(said) != 0 || len(w.told) != 0 {
+			t.Fatalf("after a restart past the last try: asked %v, told %v", said, w.told)
+		}
+		// Never an approval.
+		if e := mustGet(t, s, key); e.Current.Status != StatusProposed || len(w.said.of(EventAccepted)) != 0 {
+			t.Fatalf("a review never had approved it: %s", e.Current.Status)
+		}
+	})
+}
+
+// novox/hq issue 497: the notice of a proposal never reviewed says it stays proposed, so it is cleared once that is
+// no longer so; and a later version's run of tries is a notice of its own, told once whatever else is written.
+func TestProposedAgainAfterTheLastTryClearsTheNotice(t *testing.T) {
+	eachStore(t, func(t *testing.T, s Store) {
+		w := imported(t, s)
+		n := proposed(t, w, "author@g14", false)
+		key := Key{KindDecision, n}
+		runOut := func(from int) {
+			t.Helper()
+			w.judge.reviewFail = true
+			for try := from; try <= MostReviewTries; try++ {
+				if said := w.reviewAll(t); len(said) != 1 {
+					t.Fatalf("try %d: %v", try, said)
+				}
+				w.clock = w.clock.Add(time.Duration(try) * ReviewPause)
+			}
+		}
+		runOut(1)
+		e := mustGet(t, s, key)
+		first := fmt.Sprintf("knowledge.unreviewed.decision-%d.v%d", n, e.CurrentVersion)
+		if len(w.toldKeys) != 1 || w.toldKeys[0] != first || len(w.cleared) != 0 ||
+			w.resolved[0] != key.String()+" no longer waits for its review" {
+			t.Fatalf("after the last try: told %v, cleared %v", w.toldKeys, w.cleared)
+		}
+		// Another version written while it waits (as a link change on it writes one) tells nothing again.
+		r := e.Current.Clone()
+		if err := s.Save(context.Background(), Version{By: "test", At: w.clock, Why: "a link changed", Record: r}, true, false, e.Revision); err != nil {
+			t.Fatal(err)
+		}
+		if w.reviewAll(t); len(w.toldKeys) != 1 || len(w.cleared) != 0 {
+			t.Fatalf("a version written while it waits: told %v, cleared %v", w.toldKeys, w.cleared)
+		}
+		// Proposed again: the notice is cleared, saying so.
+		if _, err := w.k.Propose("author@g14", valid(n)); err != nil {
+			t.Fatal(err)
+		}
+		w.clock = w.clock.Add(time.Second)
+		w.reviewAll(t) // its try 1, which fails, and the clearing
+		w.clock = w.clock.Add(ReviewPause)
+		if len(w.cleared) != 1 || w.cleared[0][0] != first || !strings.Contains(w.cleared[0][1], "proposed again") {
+			t.Fatalf("proposed again: cleared %v", w.cleared)
+		}
+		// Its own run of tries out is a notice of its own, under its own version.
+		runOut(2)
+		if len(w.toldKeys) != 2 || w.toldKeys[1] == first || len(w.cleared) != 1 {
+			t.Fatalf("a later version ran out: told %v, cleared %v", w.toldKeys, w.cleared)
+		}
+		// Proposed again and accepted at last: that notice is cleared too, and nothing is cleared twice, also after
+		// a restart.
+		w.judge.reviewFail = false
+		w.accepted(t, "author@g14", valid(n))
+		if len(w.cleared) != 2 || w.cleared[1][0] != w.toldKeys[1] || !strings.Contains(w.cleared[1][1], "proposed again") {
+			t.Fatalf("accepted: cleared %v", w.cleared)
+		}
+		clock := w.clock
+		w = newWorld(t, s, fixture())
+		w.clock = clock
+		if w.reviewAll(t); len(w.cleared) != 0 || len(w.told) != 0 {
+			t.Fatalf("after a restart: told %v, cleared %v", w.told, w.cleared)
+		}
+	})
+}
+
+// inZone sets the local zone to one that is not UTC for one test, so a time said locally is proven to be, wherever
+// the test runs.
+func inZone(t *testing.T) {
+	t.Helper()
+	was := time.Local
+	time.Local = time.FixedZone("CEST", 2*60*60)
+	t.Cleanup(func() { time.Local = was })
+}
+
+// novox/hq issue 497: a clear the operator's channel refused keeps its notice as told, and is sent again at the next
+// step until it is taken.
+func TestAClearThatFailsIsSentAgain(t *testing.T) {
+	eachStore(t, func(t *testing.T, s Store) {
+		w := imported(t, s)
+		n := proposed(t, w, "author@g14", false)
+		w.judge.reviewFail = true
+		for try := 1; try <= MostReviewTries; try++ {
+			w.reviewAll(t)
+			w.clock = w.clock.Add(time.Duration(try) * ReviewPause)
+		}
+		if len(w.toldKeys) != 1 {
+			t.Fatalf("after the last try: told %v", w.toldKeys)
+		}
+		prefix := fmt.Sprintf("knowledge.unreviewed.decision-%d.", n)
+		refused := 0
+		w.k.Clear = func(key, resolved string) error { refused++; return errors.New("the channel is down") }
+		if _, err := w.k.Propose("author@g14", valid(n)); err != nil {
+			t.Fatal(err)
+		}
+		w.reviewAll(t)
+		if told, err := s.ToldOf(context.Background(), prefix); err != nil || len(told) != 1 || refused == 0 {
+			t.Fatalf("a refused clear: told %v (%v), refused %d", told, err, refused)
+		}
+		w.k.Clear = func(key, resolved string) error { w.cleared = append(w.cleared, [2]string{key, resolved}); return nil }
+		w.clock = w.clock.Add(time.Second)
+		w.reviewAll(t)
+		if told, err := s.ToldOf(context.Background(), prefix); err != nil || len(told) != 0 || len(w.cleared) != 1 ||
+			w.cleared[0][0] != w.toldKeys[0] {
+			t.Fatalf("the next step: told %v (%v), cleared %v", told, err, w.cleared)
+		}
+	})
+}

modules/mesh-knowledge/cmd/mesh-knowledge/store.go

@@ -63,8 +63,12 @@ type Store interface {
 	// SetFindings replaces the whole-set findings; Findings reads them.
 	SetFindings(ctx context.Context, f []Finding) error
 	Findings(ctx context.Context) ([]Finding, error)
-	// Told records that the operator was told about a topic's clauses, once; false when it was already.
+	// Told records that the operator was told of something, once, by its key: a topic's clauses drafted, or a
+	// version that ran out of review tries (novox/hq issue 497); false when it was already.
 	Told(ctx context.Context, topic string) (bool, error)
+	// ToldOf is every key told of that starts with prefix; Untold takes one away once what it told of is over.
+	ToldOf(ctx context.Context, prefix string) ([]string, error)
+	Untold(ctx context.Context, key string) error
 }
 
 // memStore is the Store in memory.
@@ -271,6 +275,26 @@ func (s *memStore) Findings(context.Context) ([]Finding, error) {
 	return append([]Finding(nil), s.findings...), nil
 }
 
+func (s *memStore) ToldOf(_ context.Context, prefix string) ([]string, error) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	var out []string
+	for k := range s.told {
+		if strings.HasPrefix(k, prefix) {
+			out = append(out, k)
+		}
+	}
+	sort.Strings(out)
+	return out, nil
+}
+
+func (s *memStore) Untold(_ context.Context, key string) error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	delete(s.told, key)
+	return nil
+}
+
 func (s *memStore) Told(_ context.Context, topic string) (bool, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()