From 5d50b952d4cbec213f6311859d4c3fb60aaa8a03 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 31 Oct 2026 10:16:56 +0100 Subject: [PATCH] knowledge-base: keep a retraction from being undone by a later acceptance or a redraft A record proposed before a retraction and accepted after it marked the retracted clause changed or the retracted record superseded, serving it again; and returning a drafted record numbered its clauses anew, bringing retracted ones back in force or making a waiting Approve mark another clause. accept now leaves what is retracted alone, Return refuses a record with a retraction, and retract waits until a record's drafting is done. --- modules/mesh-knowledge/README.md | 2 +- .../cmd/mesh-knowledge/draft.go | 10 +++ .../mesh-knowledge/cmd/mesh-knowledge/kb.go | 13 ++++ .../cmd/mesh-knowledge/retract.go | 3 + .../cmd/mesh-knowledge/withdraw_test.go | 65 +++++++++++++++++-- 5 files changed, 88 insertions(+), 5 deletions(-) diff --git a/modules/mesh-knowledge/README.md b/modules/mesh-knowledge/README.md index b8955ebf..0dde291c 100644 --- a/modules/mesh-knowledge/README.md +++ b/modules/mesh-knowledge/README.md @@ -27,7 +27,7 @@ operator, it is accepted only on the operator's Approve, given on a channel that | `import` | `dry-run`, `commit` | hq's decision records and research efforts at the commit the records module stands at, each whole at its own number | | `return` | `number`, `reason` | drafted clauses sent back to be drafted again; refused for a record whose clauses another record changed or links by number | | `withdraw` | `number`, `kind`, `why`, `retraction` | a reserved or proposed record withdrawn, as a new version naming why; its number stays spent. A pending review is cleared and a waiting operator's ask is ended. Refused for a record already accepted, superseded or withdrawn. With `retraction` true, the retraction waiting on the record is withdrawn instead, and the record holds as it did | -| `retract` | `number`, `kind`, `clauses`, `why` | an accepted or superseded record retracted whole, or the clauses listed, on the operator's Approve only (asked on the router like a record that needs the operator). Until then, and on Decline or expiry, nothing changes. A retracted record or clause no longer holds: `ask` and `holds` leave it out, `get` marks it with by whom, when, why and the warrant, `links` say so both ways, history keeps every version; what it changed or superseded stays so, marked retracted, and the whole-set rules still read its links | +| `retract` | `number`, `kind`, `clauses`, `why` | an accepted or superseded record retracted whole, or the clauses listed, on the operator's Approve only (asked on the router like a record that needs the operator). Until then, and on Decline or expiry, nothing changes. A retracted record or clause no longer holds: `ask` and `holds` leave it out, `get` marks it with by whom, when, why and the warrant, `links` say so both ways, history keeps every version; what it changed or superseded stays so, marked retracted, and the whole-set rules still read its links. A record accepted after it does not supersede or change what is retracted; a record still being drafted is retracted only once its drafting is done, and a record with a retraction (waiting or approved) is never returned to be drafted again. The caller need not be proven by the runtime, as for `withdraw`: it is recorded, and the operator's Approve is what authorises the retraction | | `progress` | `of` (`mesh`, `design `, `milestone /`, `issue `, `decision ` or `ADR `, `research `), `detail` (`minimal` or `detailed`), `bound`, `since` (a window such as `60m`, at most `24h`) | how far it has come (hq issue 1947): each child done ✓, moving ▶, waiting ⏸ or stuck ✗ with when it last moved, worked out from the issue tracker's rows and mesh-delivery's deliveries and groups, never written (ADR 0424 §1). `minimal` is one line per milestone, for a report every five minutes (two or three bus calls); `detailed` the report's tree with each issue's pull requests, the groups holding them and what it depends on. A design's title and state are read through the records module, cached by hq's commit; its milestones and their issues are the issue tracker's milestone records (`issue-tracker.milestones`, issue 1781), asked once per answer; a design with none is refused, naming the seed (`issue-tracker.import` with `milestones`). With `since`, only what changed in the window under `of`, headed "N changes in the last 60 min": milestones moved (from what to what, from `issue-tracker.milestones` `one`), issues opened or moved from what to what (from `issue-tracker.changes`, the tracker's log), noted or linked (from `issue-tracker.get`), pull requests opened, checked, merged, held or closed (from `mesh-delivery.show`'s transitions and check). It keeps no state; its warnings say when the tracker cannot answer `changes` (a move is then said by the status it reached, not from what) and that mesh-delivery keeps no time a merge reached its machines | `ask`, `get`, `holds` and `history` stay within `bound` bytes (4096 unless asked for, at most 65536) and say what they cut diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go b/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go index 2bc37c8e..1f87b83d 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go @@ -500,6 +500,16 @@ func (k *KB) Return(caller string, key Key, reason string) (map[string]any, erro return nil, fmt.Errorf("the knowledge base holds no %s", key) } r := e.Current.Clone() + if r.Retracting != nil || len(r.Retractions) > 0 { + // Drafting again numbers the clauses anew and brings them back in force: a retraction waiting would mark + // another clause than the operator was shown, and one approved would be undone. + which := "approved" + if r.Retracting != nil { + which = "waiting" + } + return nil, fmt.Errorf("%s is not returned: it has a retraction %s, which drafting it again would undo or point at other "+ + "clauses. Withdraw a waiting retraction first; a retracted record or clause stays so", key, which) + } if r.Drafting == nil { return nil, fmt.Errorf("%s's clauses were written by its author, not drafted: a written clause is changed by a new record", key) } diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go index 0773db51..e1458459 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go @@ -490,6 +490,12 @@ func (k *KB) accept(ctx context.Context, held *Entry, w *Record, by, why, warran k.logf("[mesh-knowledge] %s %s %s, which cannot be read now (%v): the whole-set rules will say so", r.Key(), l.Kind, to, err) continue } + if t.Current.Status == StatusRetracted { + // What the operator retracted stays retracted: a record proposed before it and accepted after does not + // bring it back as superseded or changed. + k.logf("[mesh-knowledge] %s %s %s, which is retracted: it stays retracted", r.Key(), l.Kind, to) + continue + } c := t.Current.Clone() var what string switch l.Kind { @@ -497,6 +503,9 @@ func (k *KB) accept(ctx context.Context, held *Entry, w *Record, by, why, warran c.Status = StatusSuperseded c.Extra = addExtra(c.Extra, "superseded-by", r.Key().String()) for i := range c.Clauses { + if c.Clauses[i].Status == ClauseRetracted { + continue + } c.Clauses[i].Status = ClauseSuperseded c.Clauses[i].ChangedBy = append(c.Clauses[i].ChangedBy, r.Key().String()) } @@ -506,6 +515,10 @@ func (k *KB) accept(ctx context.Context, held *Entry, w *Record, by, why, warran if cl == nil || cl.Status == ClauseSuperseded { continue } + if cl.Status == ClauseRetracted { + k.logf("[mesh-knowledge] %s changes %s §%d, which is retracted: it stays retracted", r.Key(), to, l.To.Clause) + continue + } cl.Status = ClauseChanged from := r.Key().String() if l.From > 0 { diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/retract.go b/modules/mesh-knowledge/cmd/mesh-knowledge/retract.go index 6a1494ce..e4537b84 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/retract.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/retract.go @@ -97,6 +97,9 @@ func (k *KB) Retract(caller string, key Key, clauses []int, why string) (*Retrac return nil, fmt.Errorf("%s is %s and takes no retraction; nothing was asked", key, cur.Status) case cur.Retracting != nil: return nil, fmt.Errorf("%s has a retraction waiting for the operator (%s): withdraw it first. Nothing was asked", key, cur.Retracting.Says()) + case cur.Drafting != nil && cur.Drafting.State != DraftDone: + return nil, fmt.Errorf("%s's clauses are being drafted (%s): retract it once its drafting is done, so the clauses keep the "+ + "numbers the operator is shown. Nothing was asked", key, cur.Drafting.State) case e.Pending != nil: return nil, fmt.Errorf("%s has version %d waiting for its review: let it be decided first. Nothing was asked", key, e.PendingVersion) } diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go index 750d12c3..ce847dbd 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go @@ -133,12 +133,17 @@ func TestARetractionTakesEffectOnlyOnTheOperatorsApprove(t *testing.T) { name string clauses []int answer string // approve, decline, or withdraw (the pending retraction withdrawn, then approved too late) + // then is a record proposed before the retraction and accepted after it, changing §1 or superseding the record: + // its acceptance never brings back what was retracted. + then string } for _, c := range []tc{ - {"the record whole, approved", nil, OptionApprove}, - {"one clause, approved", []int{1}, OptionApprove}, - {"the record whole, declined", nil, OptionDecline}, - {"one clause, withdrawn before the answer", []int{1}, "withdraw"}, + {"the record whole, approved", nil, OptionApprove, ""}, + {"one clause, approved", []int{1}, OptionApprove, ""}, + {"the record whole, declined", nil, OptionDecline, ""}, + {"one clause, withdrawn before the answer", []int{1}, "withdraw", ""}, + {"one clause, then a change of it proposed before and accepted after", []int{1}, OptionApprove, LinkChanges}, + {"the record whole, then a supersession proposed before and accepted after", nil, OptionApprove, LinkSupersedes}, } { t.Run(c.name, func(t *testing.T) { eachStore(t, func(t *testing.T, s Store) { @@ -149,6 +154,18 @@ func TestARetractionTakesEffectOnlyOnTheOperatorsApprove(t *testing.T) { w.accepted(t, "author@test", valid(n)) w.k.judgeSet(ctx) findings, _ := w.k.Store.Findings(ctx) + later := 0 + if c.then != "" { + later = w.reserved(t, "author@test") + p := valid(later) + p.Links = []LinkAsk{{Kind: LinkExtends, To: itoa4(n)}, {Kind: LinkChanges, To: itoa4(n) + " §1", From: 1}} + if c.then == LinkSupersedes { + p.Links = []LinkAsk{{Kind: LinkSupersedes, To: itoa4(n)}} + } + if _, err := w.k.Propose("author@test", p); err != nil { + t.Fatal(err) + } + } before := mustGet(t, w.k.Store, key) asked := len(w.router.asked) @@ -198,6 +215,21 @@ func TestARetractionTakesEffectOnlyOnTheOperatorsApprove(t *testing.T) { if _, err := w.k.Reconcile(); err != nil { t.Fatal(err) } + if c.then != "" { + w.reviewAll(t) + if l := mustGet(t, w.k.Store, Key{KindDecision, later}).Current; !l.Accepted() { + t.Fatalf("the later record was not accepted: %s", l.Status) + } + r := mustGet(t, w.k.Store, key).Current + h, _ := w.k.Holds("", &key, 0) + // Whole: retracted, nothing held. One clause: §1 retracted, and §2 the only one held. + whole := c.clauses == nil + if (whole && (r.Status != StatusRetracted || len(h.Clauses) != 0)) || + (!whole && (r.Clause(1).Status != ClauseRetracted || len(h.Clauses) != 1)) { + t.Fatalf("an acceptance brought back what was retracted: %s, %+v, holds %+v", r.Status, r.Clauses, h.Clauses) + } + return + } e := mustGet(t, w.k.Store, key) vs, _ := w.k.Store.Versions(ctx, key) if len(vs) <= before.Versions || e.Current.Retracting != nil { @@ -303,5 +335,30 @@ func TestARetractionTakesEffectOnlyOnTheOperatorsApprove(t *testing.T) { if _, err := w.k.WithdrawRetraction("lead@shanks", key, "nothing waits"); err == nil { t.Fatal("a retraction was withdrawn where none waits") } + // A drafted record: retracted only once its drafting is done, and never returned to be drafted again while a + // retraction waits or after one, which would number its clauses anew and bring them back in force. + drafted := Key{KindDecision, 240} + draftUntil(t, w, drafted, DraftDrafted) + if _, err := w.k.Retract("lead@shanks", drafted, []int{1}, why); err == nil || !strings.Contains(err.Error(), "draft") { + t.Fatalf("a record still being drafted: %v", err) + } + draftUntil(t, w, drafted, DraftDone) + out, err := w.k.Retract("lead@shanks", drafted, []int{1}, why) + if err != nil { + t.Fatal(err) + } + if _, err := w.k.Return("operator@shanks", drafted, "redraft"); err == nil || !strings.Contains(err.Error(), "retract") { + t.Fatalf("returned while a retraction waits: %v", err) + } + w.router.answer(out.Ask, OptionApprove, w.clock) + if _, err := w.k.Reconcile(); err != nil { + t.Fatal(err) + } + if _, err := w.k.Return("operator@shanks", drafted, "redraft"); err == nil || !strings.Contains(err.Error(), "retract") { + t.Fatalf("returned after a retraction: %v", err) + } + if c := mustGet(t, w.k.Store, drafted).Current.Clause(1); c == nil || c.Status != ClauseRetracted { + t.Fatalf("§1: %+v", c) + } }) }