Merge pull request 'mesh-knowledge: the review asks changes for a decision nothing would cite, so its acceptance never turns the module unhealthy (hq issue 504)' (#237) from fix/504-propose-refuses-uncited-record into main

This commit was merged in pull request #237.
This commit is contained in:
2026-10-12 11:44:58 +00:00
8 changed files with 284 additions and 46 deletions
+12 -4
View File
@@ -132,8 +132,16 @@ as text. `get` serves each, and any other field a record of hq wrote under `fiel
- `partial`: both ends of a change agree;
- `rests-on-live`: no accepted record extends a superseded one.
A failure is kept as a finding and makes the module unhealthy, so the mesh raises a condition naming the rule
and the records. The acceptance stands.
A failure is kept as a finding and told to the operator once, as a warning naming the rule and the records
(`operator-channel.notify`, keyed `knowledge.finding.<rule>.<records>`, sent again at the next step when it
could not be sent, and cleared once the rule holds for them again). It is a fault of the records, which their
authors repair, not of the module: the module's health names it and stays healthy, so it never holds the
module's own rollouts (the operator's decision of 2026-10-12 on hq issue 504). The acceptance stands.
`cited` is also judged ahead, in the review: a decision that names no home and that no accepted (or
superseded) record links is not judged and not accepted. Its review, by the knowledge base itself, asks changes
naming the rule (hq issue 504). It is not refused at propose, because no record can link a number only
reserved: a record citing a new one is proposed after it and accepted, and the new one is proposed again.
## The import
@@ -181,14 +189,14 @@ the consolidated schema for a fresh database is `schema.sql`. A test holds the t
- **the bundle**: the verbs, `knowledge_health`, the operator's answers heard, and once a minute (and at once when
a record is proposed) the router's record read, reservations never proposed withdrawn, the reviews of proposals,
and the drafting worker. It never changes the schema, and
and the drafting worker; and it tells the whole-set findings. It never changes the schema, and
refuses to serve while the schema is behind.
- **`prepare`** (run once): brings the schema to this version.
- **`keep`** (judged by its unit): judges the whole record set every five minutes and keeps the findings.
- **`watch`** (judged by `knowledge_health`, run as the machine's account, the one the runtime runs the bundle
as, so the tool can read what it writes): every minute, reads whether the database serves and what the
findings say, and leaves its verdict in `${dir:state}/health.json`. That is the file the health tool answers
from. A verdict older than three minutes is unhealthy.
from. A verdict older than three minutes is unhealthy; a whole-set finding is named in it but leaves it healthy.
## Settings
@@ -514,6 +514,73 @@ func (k *KB) judgeSet(ctx context.Context) []Finding {
return findings
}
// findingPrefix keys the notice of one whole-set finding: the prefix, the rule, and the records it names.
const findingPrefix = "knowledge.finding."
func findingKey(f Finding) string {
names := make([]string, len(f.Records))
for i, r := range f.Records {
names[i] = strings.ReplaceAll(strings.ToLower(r), " ", "-")
}
return findingPrefix + f.Rule + "." + strings.Join(names, ".")
}
// TellFindings, called by the background work and never under the write lock, tells the operator, once, each finding of the rules that judge the whole record set, as a warning
// naming the rule and the records, and clears the notice once the rule holds for them again. A finding is a fault
// of the records, which their authors repair; it is not the module failing, so it leaves the module's health as it
// is and never holds the module's own rollouts (the operator's decision of 2026-10-12 on novox/hq issue 504, as
// issue 508 decided for a proposal never reviewed). A notice that cannot be sent is not kept as told, and is sent
// again at the next step.
func (k *KB) TellFindings() {
if k.Notify == nil || k.Clear == nil {
return
}
ctx := context.Background()
findings, err := k.Store.Findings(ctx)
if err != nil {
k.logf("[mesh-knowledge] the whole-set findings cannot be read to tell them: %v", err)
return
}
now := map[string]bool{}
for _, f := range findings {
key := findingKey(f)
now[key] = true
first, err := k.Store.Told(ctx, key)
if err != nil || !first {
continue
}
records := strings.Join(f.Records, ", ")
summary := fmt.Sprintf("The rule %s, which judges the whole record set, fails: %s.", f.Rule, f.Says)
explanation := "The knowledge base works; its records break a rule. The acceptance stands. The author of " + records +
" repairs it: a decision cited by nothing names its home in a progressive insight (propose with homes and insight), " +
"or a record that cites it is accepted. get and links show the record."
resolved := fmt.Sprintf("The rule %s holds again for %s", f.Rule, records)
if err := k.Notify(key, summary, "A rule of the record set fails: "+records, explanation, resolved); err != nil {
k.logf("[mesh-knowledge] the operator could not be told that %s fails for %s: %v; told again at the next step", f.Rule, records, err)
if uerr := k.Store.Untold(ctx, key); uerr != nil {
k.logf("[mesh-knowledge] and that it was not told could not be kept: %v", uerr)
}
}
}
told, err := k.Store.ToldOf(ctx, findingPrefix)
if err != nil {
k.logf("[mesh-knowledge] the notices of whole-set findings cannot be read: %v", err)
return
}
for _, key := range told {
if now[key] {
continue
}
if err := k.Clear(key, ""); err != nil {
k.logf("[mesh-knowledge] the notice %s could not be cleared: %v; it is cleared again at the next step", key, err)
continue
}
if err := k.Store.Untold(ctx, key); err != nil {
k.logf("[mesh-knowledge] the notice %s was cleared, and that could not be kept: %v", key, err)
}
}
}
// ---- reservations that were never proposed -------------------------------------------------------------------
// Withdraw withdraws every number reserved longer ago than the settings allow and never proposed: it stays spent,
@@ -40,6 +40,9 @@ func TestMain(m *testing.M) {
serve(k, os.Getenv("MESH_KNOWLEDGE_STATUS"))
os.Exit(0)
}
// Every test runs in a zone that is not UTC, so a time said locally is proven to be, wherever the tests run. It
// is set once, before any goroutine starts: a test that set it while a bus test's server read it raced.
time.Local = testZone
code := m.Run()
stopPostgres()
os.Exit(code)
@@ -436,9 +439,13 @@ func valid(n int) ProposeAsk {
{Name: "Consequences", Text: "Merges land sooner."},
{Name: "How it is checked", Text: "The controller's merge-window tests."},
},
Links: []LinkAsk{{Kind: LinkExtends, To: "0276"}, {Kind: LinkChanges, To: "0276 §1", From: 1}}}
Links: []LinkAsk{{Kind: LinkExtends, To: "0276"}, {Kind: LinkChanges, To: "0276 §1", From: 1}},
Homes: []string{testHome}}
}
// testHome is the hq design valid names as its home: a decision is accepted only cited (novox/hq issue 504).
const testHome = "03-DESIGN/01-to-be/55-the-knowledge-base.md"
// imported is a world whose knowledge base holds the fixture's records.
func imported(t *testing.T, s Store) *world {
t.Helper()
@@ -164,6 +164,8 @@ func background(k *KB) {
logf("[mesh-knowledge] withdrew %d reservation(s) never proposed", n)
}
review()
// The keeper judges the record set in a process of its own, which tells nobody: its findings are told here.
k.TellFindings()
for {
said, err := k.DraftStep()
if err != nil {
@@ -552,10 +554,11 @@ type HealthAnswer struct {
At time.Time `json:"at,omitempty"`
}
// Health is whether the knowledge base works: unhealthy when its database cannot serve, or when a rule that judges
// the whole record set fails — the mesh raises a condition naming the rule and the records. A proposal that ran out
// of review tries waits on its author, not on the module: it is told to its author and the operator, and named
// here, but leaves the module healthy, so it never holds the module's own rollouts (novox/hq issue 508).
// Health is whether the knowledge base works: unhealthy when its database cannot serve. A rule that judges the whole
// record set and fails is a fault of the records, which their authors repair, not of the module: it is told to the
// operator as a warning naming the rule and the records (TellFindings), and named here, but leaves the module
// healthy (the operator's decision of 2026-10-12 on novox/hq issue 504). So does a proposal that ran out of review
// tries (novox/hq issue 508). Neither holds the module's own rollouts.
func (k *KB) Health(ready Healther) HealthAnswer {
ctx, cancel := context.WithTimeout(context.Background(), 8*time.Second)
defer cancel()
@@ -586,17 +589,14 @@ func (k *KB) Health(ready Healther) HealthAnswer {
waiting = fmt.Sprintf("%d proposed record(s) could not be reviewed in %d tries and wait for their authors to propose them again: %s",
n, MostReviewTries, strings.Join(unreviewed, "; "))
}
why := "the database serves and every rule that judges the whole record set holds"
if len(says) > 0 {
if len(says) > 8 {
says = append(says[:8], fmt.Sprintf("and %d more", len(says)-8))
}
why := fmt.Sprintf("%d rule(s) that judge the whole record set fail: %s", len(findings), strings.Join(says, "; "))
if waiting != "" {
why += ". " + waiting
}
return HealthAnswer{State: "unhealthy", Why: why}
why = fmt.Sprintf("the database serves; %d rule(s) that judge the whole record set fail, told to the operator as a warning "+
"for the records' authors to repair: %s", len(findings), strings.Join(says, "; "))
}
why := "the database serves and every rule that judges the whole record set holds"
if waiting != "" {
why += "; " + waiting
}
@@ -605,10 +605,11 @@ func (k *KB) Health(ready Healther) HealthAnswer {
func healthTool(status string, now func() time.Time) stdio.Tool {
return stdio.Tool{Name: HealthTool,
Description: "How the knowledge base is, as its watch last saw it: healthy when its database serves this version and every " +
"rule that judges the whole record set holds; unhealthy otherwise, naming the rule and the records — a decision accepted " +
"that nothing cites, a supersession or a change said on one end only, a record extending a superseded one. A " +
"proposed record that ran out of review tries is named but leaves it healthy: it waits on its author (novox/hq issue 508).",
Description: "How the knowledge base is, as its watch last saw it: healthy when its database serves this version, unhealthy " +
"otherwise. A rule that judges the whole record set and fails — a decision accepted that nothing cites, a supersession " +
"or a change said on one end only, a record extending a superseded one — is named but leaves it healthy: it is told " +
"to the operator as a warning for the records' authors (novox/hq issue 504). So is a proposed record that ran out of " +
"review tries: it waits on its author (novox/hq issue 508).",
Run: func(map[string]any) (any, error) { return ReadStatus(status, now()), nil }}
}
@@ -918,7 +919,8 @@ func Tools(k *KB, caller func(map[string]any) string) []stdio.Tool {
"link to nothing, a clause without a bold lead, a superseded record cited alone, an unknown topic, a retired word. A " +
"proposal is revised by proposing it again; an accepted record changes only by a progressive insight (insight: what " +
"stood before). The knowledge base then reviews each version itself, in a judgement that sees only the record, the " +
"records it links and the rules: on approve it is accepted (a record that needs the operator waits for the operator's " +
"records it links and the rules. A decision that names no home and that no accepted record links is not judged: " +
"its review asks changes for the whole-set rule cited, which its acceptance would break. On approve it is accepted (a record that needs the operator waits for the operator's " +
"Approve), on changes it stays proposed with the findings for its author to revise, and when no judgement can be had " +
"it stays proposed and says so. Nothing proposed is in force.",
Input: schema([]string{"number", "title"}, map[string]any{
@@ -933,7 +935,7 @@ func Tools(k *KB, caller func(map[string]any) string) []stdio.Tool {
"links": map[string]any{"type": "array", "description": "its links: kind, to (\"0276\", \"0276 §4\", \"research 045\", \"issue 365\", a design's path), from (the clause it goes out from)",
"items": map[string]any{"type": "object", "properties": map[string]any{"kind": map[string]any{"type": "string", "enum": LinkKinds},
"to": text("the target"), "from": integer("the clause of this record it goes out from")}, "required": []string{"kind", "to"}}},
"homes": texts("hq documents that cite it, such as a design's path"),
"homes": texts("hq documents that cite it, such as a design's path; a decision needs one unless an accepted record links it"),
"needs-operator": flag("it holds only once the operator approves it"),
"status": map[string]any{"type": "string", "enum": ResearchStatuses, "description": "a research effort's status"},
"insight": text("for an accepted record: a progressive insight, saying what stood before"),
@@ -408,7 +408,10 @@ func (k *KB) reviewOne(key Key) (string, error) {
k.mu.Lock()
e, err := k.Store.Get(ctx, key)
var linked []*Record
var all []*Entry
var allErr error
if err == nil && e != nil {
all, allErr = k.Store.All(ctx)
seen := map[Key]bool{key: true}
for _, l := range e.Working().Links {
to, ok := l.To.Record()
@@ -430,9 +433,20 @@ func (k *KB) reviewOne(key Key) (string, error) {
var verdict string
var findings []string
by := ReviewBy
if len(question) > MostReviewQuestion {
uncited := ""
if allErr == nil {
uncited = UncitedOnAcceptance(w, all)
}
switch {
case allErr != nil:
err = fmt.Errorf("the record set cannot be read to judge whether a record cites it: %v", allErr)
case uncited != "":
// The whole-set rule cited, judged ahead: changes, asked by the knowledge base itself without a judgement,
// so its acceptance never turns the module's health red for it (novox/hq issue 504).
verdict, findings = VerdictChanges, []string{"the rule cited, which judges the whole record set: " + uncited}
case len(question) > MostReviewQuestion:
err = fmt.Errorf("the record is %d bytes as a question, and a judgement takes at most %d: shorten it", len(question), MostReviewQuestion)
} else {
default:
var answer json.RawMessage
var judge string
answer, judge, err = k.Judge(question, reviewSchema, k.judgeModel())
@@ -155,13 +155,16 @@ func TestProposedAgainAfterTheLastTryClearsTheNotice(t *testing.T) {
})
}
// 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.
// testZone is the local zone every test runs in, set once by TestMain: not UTC.
var testZone = time.FixedZone("CEST", 2*60*60)
// inZone says the test needs the local zone not to be UTC; TestMain set it before any goroutine started. Setting it
// here, under a running bus server's goroutines, was a data race the race detector failed.
func inZone(t *testing.T) {
t.Helper()
was := time.Local
time.Local = time.FixedZone("CEST", 2*60*60)
t.Cleanup(func() { time.Local = was })
if time.Local != testZone {
t.Fatal("the local zone is not the tests' own: TestMain sets it")
}
}
// novox/hq issue 497: a clear the operator's channel refused keeps its notice as told, and is sent again at the next
@@ -546,6 +546,36 @@ func JudgeSet(records []*Record) []Finding {
return out
}
// UncitedOnAcceptance is the whole-set rule `cited` judged ahead, on a decision's working version as it would
// stand once accepted, over the records as JudgeSet reads them: "" when it names a home or a record accepted (or
// superseded) links it, else what the rule would say. hq held this rule at the merge of the record's own pull
// request, which carried the design citing it; the knowledge base holds it in the review, before the judgement, so
// an acceptance never leaves a decision cited by nothing (novox/hq issue 504). It is not refused at propose: no
// record can link a number only reserved, so a record that will cite a new one is proposed after it, accepted, and
// the new one proposed again. A citing record changed by a correction after the acceptance is what the rule, run
// after every acceptance, still says. An imported record was held to hq's checks, and its correction is not judged
// here, as JudgeSet does not judge it.
func UncitedOnAcceptance(r *Record, all []*Entry) string {
if r.Kind != KindDecision || len(r.Homes) > 0 || (r.Pending != nil && r.AcceptedAt.IsZero()) {
return ""
}
k := r.Key()
for _, e := range all {
from := e.Current
if from.Key() == k || !(from.Accepted() || from.Status == StatusSuperseded) {
continue
}
for _, l := range from.Links {
if to, ok := l.To.Record(); ok && to == k {
return ""
}
}
}
return fmt.Sprintf("%s would be accepted with nothing citing it: it names no home and no accepted record links it. Name "+
"its home (homes: the hq design or playbook that cites it), or have the record that links it accepted first; then "+
"propose this one again", k)
}
// RuleFrom is the day each dated rule began to hold a record, as hq's checks hold it: a record dated earlier was
// never held to it, and a dated correction of it (a progressive insight, which keeps the record's date) is not.
// words.py held the retired words from 2026-10-07 and the daemons' lines from 2026-10-09; cycle.py the deferrals
@@ -1,38 +1,119 @@
package main
import (
"context"
"errors"
"strings"
"testing"
)
// to-be 55 §10: "A whole-set rule that fails is loud — the module's test: an acceptance that leaves a decision
// cited by nothing raises the condition." The condition is the mesh's, raised when the module's health answers
// unhealthy; the test reads the health answer the node-engine reads.
func TestAnAcceptanceThatLeavesADecisionCitedByNothingMakesTheModuleUnhealthy(t *testing.T) {
// A decision nothing would cite is not accepted: its review asks changes itself, without a judgement, naming the
// rule cited, and the module stays healthy (novox/hq issue 504). Before, the review approved it, the acceptance
// broke the rule, and the module turned unhealthy for a record its author could only repair after the fact.
func TestADecisionNothingWouldCiteIsNotAccepted(t *testing.T) {
eachStore(t, func(t *testing.T, s Store) {
w := imported(t, s)
if h := w.k.Health(nil); !h.Healthy {
t.Fatalf("unhealthy after the import of real records: %s", h.Why)
}
n := w.reserved(t, "author@g14")
w.accepted(t, "author@g14", valid(n))
h := w.k.Health(nil)
if h.Healthy || h.State != "unhealthy" || !strings.Contains(h.Why, "the rule cited") || !strings.Contains(h.Why, itoaKey(n)) {
t.Fatalf("an accepted decision nothing cites left the module %+v", h)
p := valid(n)
p.Homes = nil
if _, err := w.k.Propose("author@g14", p); err != nil {
t.Fatal(err)
}
// The acceptance stands: the condition says it, and undoes nothing.
judged := len(w.judge.reviews)
w.reviewAll(t)
e := mustGet(t, s, Key{KindDecision, n})
if e.Current.Status != StatusProposed {
t.Fatalf("a decision nothing cites was %s by its review", e.Current.Status)
}
rv := e.Current.Reviews
if len(rv) == 0 || rv[len(rv)-1].Verdict != VerdictChanges || !strings.Contains(rv[len(rv)-1].Findings, "the rule cited") ||
rv[len(rv)-1].By != ReviewBy {
t.Fatalf("its review asked no changes for the rule cited: %+v", rv)
}
if len(w.judge.reviews) != judged {
t.Fatal("a judgement was spent on a record the rule cited refuses")
}
if h := w.k.Health(nil); !h.Healthy {
t.Fatalf("unhealthy for a record not accepted: %s", h.Why)
}
// Named with its home, it is accepted, and the module stays healthy.
p.Homes = []string{testHome}
w.accepted(t, "author@g14", p)
if h := w.k.Health(nil); !h.Healthy {
t.Fatalf("unhealthy after a decision with its home was accepted: %s", h.Why)
}
})
}
// A new decision is cited by a record proposed after it: no record can link a number only reserved. Once the
// citing record is accepted, the new one proposed again is accepted. A correction of the citing record that drops
// the link still leaves the decision cited by nothing, and that is loud: told to the operator once, as a warning
// naming the rule and the record, while the module stays healthy and its rollouts are not held (the operator's
// decision of 2026-10-12 on novox/hq issue 504; to-be 55 §10). A record that cites it clears the notice.
func TestARecordAcceptedCitingANewDecisionLetsItBeAcceptedAndDroppingTheLinkIsLoud(t *testing.T) {
eachStore(t, func(t *testing.T, s Store) {
w := imported(t, s)
n := w.reserved(t, "author@g14")
a := valid(n)
a.Homes = nil
if _, err := w.k.Propose("author@g14", a); err != nil {
t.Fatal(err)
}
w.reviewAll(t)
m := w.reserved(t, "author@g14")
b := valid(m)
b.Links = append(b.Links, LinkAsk{Kind: LinkCites, To: itoa4(n)})
w.accepted(t, "author@g14", b)
w.accepted(t, "author@g14", a) // its revision: an accepted record links it now
if h := w.k.Health(nil); !h.Healthy {
t.Fatalf("unhealthy with both accepted, one citing the other: %s", h.Why)
}
w.k.TellFindings()
if len(w.told) != 0 {
t.Fatalf("told with every rule holding: %v", w.told)
}
// A correction of the citing record drops its link: the rule, run after the acceptance, says so.
b.Links = b.Links[:len(b.Links)-1]
b.Insight = "it cited " + itoaKey(n)
w.accepted(t, "author@g14", b)
h := w.k.Health(nil)
if !h.Healthy || !strings.Contains(h.Why, "the rule cited") || !strings.Contains(h.Why, itoaKey(n)) {
t.Fatalf("a decision left cited by nothing: the health should stay healthy and name it, not %+v", h)
}
w.k.TellFindings()
w.k.TellFindings()
if len(w.told) != 1 || !strings.Contains(w.told[0], "rule cited") || !strings.Contains(w.told[0], itoaKey(n)) ||
!strings.HasPrefix(w.toldKeys[0], findingPrefix) {
t.Fatalf("the operator is not told once, naming the rule and the record: %v %v", w.told, w.toldKeys)
}
// A restart tells nothing again: what was told is kept in the store.
told := w.toldKeys[0]
restarted := newWorld(t, s, fixture())
restarted.clock = w.clock
restarted.k.TellFindings()
if len(restarted.told) != 0 {
t.Fatalf("told again after a restart: %v", restarted.told)
}
w.told = nil
if mustGet(t, s, Key{KindDecision, n}).Current.Status != StatusAccepted {
t.Fatal("the acceptance was undone")
}
// A record that cites it heals it.
m := w.reserved(t, "author@g14")
p := valid(m)
p.Links = append(p.Links, LinkAsk{Kind: LinkCites, To: itoa4(n)})
w.accepted(t, "author@g14", p)
h = w.k.Health(nil)
if strings.Contains(h.Why, itoaKey(n)+" is accepted and nothing cites it") {
k := w.reserved(t, "author@g14")
c := valid(k)
c.Links = append(c.Links, LinkAsk{Kind: LinkCites, To: itoa4(n)})
w.accepted(t, "author@g14", c)
if h := w.k.Health(nil); strings.Contains(h.Why, itoaKey(n)+" is accepted and nothing cites it") {
t.Fatalf("still said after a record cited it: %s", h.Why)
}
w.k.TellFindings()
if len(w.cleared) != 1 || w.cleared[0][0] != told {
t.Fatalf("the notice is not cleared once the rule holds again: %v", w.cleared)
}
w.k.TellFindings()
if len(w.cleared) != 1 || len(w.told) != 0 {
t.Fatalf("after the clear: cleared %v, told %v", w.cleared, w.told)
}
})
}
@@ -83,3 +164,29 @@ func TestEachWholeSetRuleFails(t *testing.T) {
t.Errorf("a consistent set fails: %+v", f)
}
}
// A finding's notice the operator's channel refused is not kept as told: it is sent again at the next step.
func TestAFindingsNoticeThatFailsIsSentAgain(t *testing.T) {
eachStore(t, func(t *testing.T, s Store) {
w := newWorld(t, s, fixture())
f := Finding{Rule: "cited", Records: []string{"ADR 0300"}, Says: "ADR 0300 is accepted and nothing cites it"}
if err := s.SetFindings(context.Background(), []Finding{f}); err != nil {
t.Fatal(err)
}
tries := 0
w.k.Notify = func(key, summary, headline, explanation, resolved string) error {
tries++
if tries == 1 {
return errors.New("nothing answers operator-channel.notify")
}
w.told = append(w.told, summary)
return nil
}
w.k.TellFindings()
w.k.TellFindings()
w.k.TellFindings()
if tries != 2 || len(w.told) != 1 {
t.Fatalf("a refused notice: %d tries, told %v", tries, w.told)
}
})
}