mesh-knowledge: clear a never-reviewed notice once the record moves on, key it by version and run of tries, say times locally (review of #219)
mesh/merge-gate pass: builds mesh-knowledge → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery superseded: a newer head of the same pull request
mesh/merge-gate pass: builds mesh-knowledge → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery superseded: a newer head of the same pull request
A notice left open kept telling the operator a record was proposed and not in force after it was proposed again or accepted, and a later version's notice merged into it.
This commit is contained in:
@@ -107,8 +107,9 @@ as text. `get` serves each, and any other field a record of hq wrote under `fiel
|
||||
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, and the
|
||||
module is unhealthy, naming it, so the mesh raises a condition.
|
||||
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.
|
||||
|
||||
@@ -67,6 +67,9 @@ type Judgement func(question string, schema map[string]any, model string) (json.
|
||||
// Notifier tells the operator's channel something (operator-channel.notify).
|
||||
type Notifier func(key, summary, headline, explanation 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 {
|
||||
Store Store
|
||||
@@ -75,6 +78,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
|
||||
|
||||
@@ -397,7 +397,10 @@ 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
|
||||
clock time.Time
|
||||
}
|
||||
|
||||
func newWorld(t *testing.T, s Store, records Records) *world {
|
||||
@@ -406,8 +409,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 string) error {
|
||||
w.told, w.toldKeys = append(w.told, summary), append(w.toldKeys, key)
|
||||
return nil
|
||||
},
|
||||
Clear: func(key, resolved string) error { w.cleared = append(w.cleared, [2]string{key, resolved}); return nil }}
|
||||
return w
|
||||
}
|
||||
|
||||
|
||||
@@ -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,
|
||||
@@ -230,6 +231,14 @@ func busNotifier(key, summary, headline, explanation string) error {
|
||||
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",
|
||||
"clear": true, "resolved": resolved})
|
||||
return err
|
||||
}
|
||||
|
||||
// ---- the store, opened when it is first needed ------------------------------------------------------------
|
||||
|
||||
// lazyStore opens the database on the first call, and again after a failure, so a database that arrives late is
|
||||
@@ -329,6 +338,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 --------------------------------------------------------------------------------
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -72,7 +72,7 @@ func (j *Judging) owedSays() string {
|
||||
"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().UTC().Format("2006-01-02 15:04 UTC"))
|
||||
j.due().Local().Format("2006-01-02 15:04 MST"))
|
||||
}
|
||||
|
||||
// The states of Judging.
|
||||
@@ -255,6 +255,7 @@ 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)
|
||||
@@ -293,33 +294,106 @@ func ranOutOfTries(w *Record) bool {
|
||||
return w.Status == StatusProposed && w.Judging != nil && w.Judging.State == JudgingFailed && w.Judging.Tries >= MostReviewTries
|
||||
}
|
||||
|
||||
// tellUnreviewed tells the operator, once for each version, 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). The store
|
||||
// keeps that it was told, so a restart does not tell it again. A tell that fails is logged; the module's health
|
||||
// still names the record, so the mesh's condition stands.
|
||||
// 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
|
||||
}
|
||||
first, err := k.Store.Told(context.Background(), fmt.Sprintf("unreviewed %s version %d", w.Key(), version))
|
||||
if err != nil || !first {
|
||||
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."
|
||||
key := fmt.Sprintf("knowledge.unreviewed.%s-%d", w.Kind, w.Number)
|
||||
if err := k.Notify(key, summary, "A proposal was never reviewed: "+w.Key().String(), explanation); 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) {
|
||||
|
||||
@@ -50,7 +50,7 @@ func TestAReviewOwedOutlivesARestartAndItsLastTryIsSaid(t *testing.T) {
|
||||
}
|
||||
if try < MostReviewTries {
|
||||
due = w.clock.Add(time.Duration(try) * 10 * time.Minute)
|
||||
when := "asked again at " + due.Format("2006-01-02 15:04 UTC")
|
||||
when := "asked again at " + due.Local().Format("2006-01-02 15:04 MST")
|
||||
if !strings.Contains(why, when) {
|
||||
t.Fatalf("try %d: the history does not say when it is asked again (%q): %q", try, when, why)
|
||||
}
|
||||
@@ -86,3 +86,65 @@ func TestAReviewOwedOutlivesARestartAndItsLastTryIsSaid(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// 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 {
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -66,6 +66,9 @@ type Store interface {
|
||||
// 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.
|
||||
@@ -272,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()
|
||||
|
||||
Reference in New Issue
Block a user