From ddf7a5ab09a07d6c45fea3046d841bb6ef31b448 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 00:02:48 +0200 Subject: [PATCH 1/2] Let a proposed record be withdrawn (issue 1081) A dropped proposal or a stale reservation stayed proposed forever: there was no way to say the operator decided against it. knowledge-base.withdraw takes a reserved or proposed record to withdrawn as a new version naming why, ends a waiting operator's ask and clears a pending review in the same version, and refuses an already accepted, superseded or withdrawn record. The number stays spent either way. The old Withdraw (the background job for reservations never proposed) is renamed WithdrawStale to make room for the new, caller-driven WithdrawRecord. --- modules/mesh-knowledge/README.md | 1 + .../cmd/mesh-knowledge/highest_test.go | 2 +- .../mesh-knowledge/cmd/mesh-knowledge/kb.go | 54 +++++++- .../mesh-knowledge/cmd/mesh-knowledge/main.go | 27 +++- .../cmd/mesh-knowledge/manifest_test.go | 2 +- .../cmd/mesh-knowledge/review_test.go | 2 +- .../cmd/mesh-knowledge/withdraw_test.go | 120 ++++++++++++++++++ modules/mesh-knowledge/module.json | 6 +- 8 files changed, 204 insertions(+), 10 deletions(-) create mode 100644 modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go diff --git a/modules/mesh-knowledge/README.md b/modules/mesh-knowledge/README.md index 163f1c70..30e3cec5 100644 --- a/modules/mesh-knowledge/README.md +++ b/modules/mesh-knowledge/README.md @@ -26,6 +26,7 @@ operator, it is accepted only on the operator's Approve, given on a channel that | `propose` | the record whole | proposed, or refused naming every rule it breaks; an accepted record changes only by a progressive insight (`insight`), but an active research effort takes new versions freely, each reviewed as a proposal. Each version proposed is then reviewed by the knowledge base itself (below) | | `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` | 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 | `ask`, `get`, `holds` and `history` stay within `bound` bytes (4096 unless asked for, at most 65536) and say what they cut and how to read the rest. The module's own tool `knowledge_health` answers its health (below). diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/highest_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/highest_test.go index 42471309..d8132b00 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/highest_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/highest_test.go @@ -37,7 +37,7 @@ func TestTheHighestToolAnswersTheHighestNumberHeldPerKind(t *testing.T) { // A number reserved, and later withdrawn, is spent: it is the highest. w.reserved(t, "author@g14") // 300 w.clock = w.clock.Add(DefaultReservedFor + time.Hour) - if _, err := w.k.Withdraw(); err != nil { + if _, err := w.k.WithdrawStale(); err != nil { t.Fatal(err) } if h := answer(); h.Decision != 300 || h.Research != 45 { diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go index 4c469359..c21d2b52 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go @@ -606,9 +606,9 @@ func (k *KB) TellFindings() { // ---- reservations that were never proposed ------------------------------------------------------------------- -// Withdraw withdraws every number reserved longer ago than the settings allow and never proposed: it stays spent, -// with who reserved it and when. -func (k *KB) Withdraw() (int, error) { +// WithdrawStale withdraws every number reserved longer ago than the settings allow and never proposed: it stays +// spent, with who reserved it and when. +func (k *KB) WithdrawStale() (int, error) { ctx := context.Background() k.mu.Lock() defer k.mu.Unlock() @@ -638,3 +638,51 @@ func (k *KB) Withdraw() (int, error) { } return n, nil } + +// ---- withdraw, on the operator's decision ------------------------------------------------------------------ + +// Withdrawn is the answer to withdraw. +type Withdrawn struct { + Record string `json:"record"` + Status string `json:"status"` + Says string `json:"says"` +} + +// WithdrawRecord withdraws a reserved or proposed record on the caller's decision, with why in one line: the +// number stays spent (novox/hq ADR 0290 rule 5), as a new version by the caller. A pending review is cleared, and +// a waiting operator's ask is ended, both as part of that one version. Refused: a record this holds none of, an +// empty why, and a record already accepted (a research effort accepted at any status), superseded or withdrawn — +// only a reserved or a proposed record takes it. +func (k *KB) WithdrawRecord(caller string, key Key, why string) (*Withdrawn, error) { + why = strings.TrimSpace(why) + if why == "" { + return nil, errors.New("withdraw needs why: one line saying why the record is withdrawn; nothing was withdrawn") + } + ctx := context.Background() + k.mu.Lock() + defer k.mu.Unlock() + e, err := k.Store.Get(ctx, key) + if err != nil { + return nil, fmt.Errorf("the knowledge base cannot be read (%v); nothing was withdrawn", err) + } + if e == nil { + return nil, fmt.Errorf("the knowledge base holds no %s; nothing was withdrawn", key) + } + cur := e.Current + if cur.Status != StatusReserved && cur.Status != StatusProposed { + return nil, fmt.Errorf("%s is %s and takes no withdraw; only a reserved or a proposed record does. Nothing was withdrawn", key, cur.Status) + } + r := cur.Clone() + r.Status = StatusWithdrawn + r.Judging = nil + if r.Ask != nil && r.Ask.Said == AskWaiting { + r.Ask.Said = "ended: the record was withdrawn" + } + full := fmt.Sprintf("withdrawn by %s: %s", caller, why) + if err := k.Store.Save(ctx, Version{By: caller, At: k.now(), Why: full, Record: r}, true, true, e.Revision); err != nil { + return nil, fmt.Errorf("%s could not be written (%v); nothing was withdrawn", key, err) + } + k.emit(EventChanged, r, caller, full, e.Versions+1) + return &Withdrawn{Record: key.String(), Status: StatusWithdrawn, + Says: fmt.Sprintf("%s is withdrawn: %s. Its number stays spent, and is never handed out again.", key, why)}, nil +} diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/main.go b/modules/mesh-knowledge/cmd/mesh-knowledge/main.go index 288a894c..30d05229 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/main.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/main.go @@ -37,7 +37,7 @@ import ( const Seat = "knowledge-base" // Verbs are the seat's verbs, as the manifest declares and claims them (ADR 0290 rule 6). -var Verbs = []string{"ask", "get", "list", "findings", "holds", "links", "history", "reserve", "propose", "import", "return"} +var Verbs = []string{"ask", "get", "list", "findings", "holds", "links", "history", "reserve", "propose", "import", "return", "withdraw"} // HealthTool is the module's own tool the node-engine asks how it is. const HealthTool = "knowledge_health" @@ -158,7 +158,7 @@ func background(k *KB) { logf("[mesh-knowledge] %s", s) } } - if n, err := k.Withdraw(); err != nil { + if n, err := k.WithdrawStale(); err != nil { logf("[mesh-knowledge] withdrawing reservations: %v", err) } else if n > 0 { logf("[mesh-knowledge] withdrew %d reservation(s) never proposed", n) @@ -1097,6 +1097,29 @@ func Tools(k *KB, caller func(map[string]any) string) []stdio.Tool { } return k.Return(caller(m), key, reason) }}, + {Name: Seat + ".withdraw", + Description: "Withdraw a reserved or proposed record on your decision, with why in one line: its number stays spent and is " + + "never handed out again. A pending review of it is cleared, and a waiting operator's ask on it is ended, both as part of " + + "the one new version this writes, by you, naming why. Refused: an empty why, a record held of none, and a record already " + + "accepted (a research effort accepted at any status), superseded or withdrawn — only a reserved or a proposed record " + + "takes it.", + Input: schema([]string{"number", "why"}, map[string]any{"kind": kindArg(), "number": integer("the record's number"), + "why": text("one line: why the record is withdrawn")}), + Run: func(m map[string]any) (any, error) { + a, err := readArgs("withdraw", m, "kind", "number", "why") + if err != nil { + return nil, err + } + key, err := a.key() + if err != nil { + return nil, err + } + why, err := a.str("why") + if err != nil { + return nil, err + } + return k.WithdrawRecord(caller(m), key, why) + }}, } } diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/manifest_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/manifest_test.go index 9945acb0..0d94d731 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/manifest_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/manifest_test.go @@ -62,7 +62,7 @@ func TestItDeclaresAndHoldsTheKnowledgeBaseSeatOnceForTheMesh(t *testing.T) { if m.Module != Module || !reflect.DeepEqual(m.Seats, want) || !reflect.DeepEqual(m.Claims, want) { t.Fatalf("module %q, seats %+v, claims %+v", m.Module, m.Seats, m.Claims) } - if !reflect.DeepEqual(Verbs, []string{"ask", "get", "list", "findings", "holds", "links", "history", "reserve", "propose", "import", "return"}) { + if !reflect.DeepEqual(Verbs, []string{"ask", "get", "list", "findings", "holds", "links", "history", "reserve", "propose", "import", "return", "withdraw"}) { t.Fatalf("the verbs are not ADR 0290 rule 6's: %v", Verbs) } if !reflect.DeepEqual(m.Emits, Events) { diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review_test.go index ec3be005..51221ab2 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/review_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review_test.go @@ -406,7 +406,7 @@ func TestReserveAsksTheTrackerAndANumberNeverProposedIsWithdrawn(t *testing.T) { t.Fatal("a design number reserved") } w.clock = w.clock.Add(DefaultReservedFor + time.Hour) - if n, err := w.k.Withdraw(); err != nil || n != 1 { + if n, err := w.k.WithdrawStale(); err != nil || n != 1 { t.Fatalf("withdrew %d (%v)", n, err) } e := mustGet(t, w.k.Store, Key{KindResearch, 50}) diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go new file mode 100644 index 00000000..aff1d3b9 --- /dev/null +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go @@ -0,0 +1,120 @@ +package main + +// Withdraw (novox/hq issue 1081): the operator's decision that a reserved or proposed record is dropped, so its +// number stays spent and the record stops asking anything of anyone. + +import ( + "context" + "strings" + "testing" +) + +func TestAReservedRecordIsWithdrawnAtOnce(t *testing.T) { + eachStore(t, func(t *testing.T, s Store) { + w := imported(t, s) + n := w.reserved(t, "author@test") + key := Key{KindDecision, n} + out, err := w.k.WithdrawRecord("operator@shanks", key, "never proposed") + if err != nil { + t.Fatal(err) + } + if out.Status != StatusWithdrawn || out.Record != key.String() { + t.Fatalf("%+v", out) + } + e := mustGet(t, w.k.Store, key) + if e.Current.Status != StatusWithdrawn { + t.Fatalf("status %q", e.Current.Status) + } + }) +} + +func TestAProposedRecordNeedingTheOperatorIsWithdrawnWithItsAskEndedAndItsReviewCleared(t *testing.T) { + eachStore(t, func(t *testing.T, s Store) { + w := imported(t, s) + n := w.reserved(t, "author@test") + key := Key{KindDecision, n} + p := valid(n) + p.NeedsOperator = true + p.Brief = &Brief{Headline: "a test record that needs the operator", Summary: "the operator is asked, then the proposal is withdrawn."} + if _, err := w.k.Propose("author@test", p); err != nil { + t.Fatal(err) + } + w.reviewAll(t) + before := mustGet(t, w.k.Store, key) + if before.Current.Status != StatusProposed || before.Current.Ask == nil || before.Current.Ask.Said != AskWaiting { + t.Fatalf("not waiting for the operator: %+v", before.Current) + } + why := "replaced by research on moving designs into the knowledge base" + out, err := w.k.WithdrawRecord("operator@shanks", key, why) + if err != nil { + t.Fatal(err) + } + if out.Status != StatusWithdrawn { + t.Fatalf("status %q", out.Status) + } + after := mustGet(t, w.k.Store, key) + if after.Current.Status != StatusWithdrawn { + t.Fatalf("status %q", after.Current.Status) + } + if after.Versions != before.Versions+1 { + t.Fatalf("versions %d, want %d", after.Versions, before.Versions+1) + } + if after.Current.Ask == nil || after.Current.Ask.Said == AskWaiting { + t.Fatalf("the waiting ask was not ended: %+v", after.Current.Ask) + } + if after.Current.Judging != nil { + t.Fatalf("a pending review was not cleared: %+v", after.Current.Judging) + } + vs, err := w.k.Store.Versions(context.Background(), key) + if err != nil { + t.Fatal(err) + } + last := vs[len(vs)-1] + if last.By != "operator@shanks" || !strings.Contains(last.Why, why) { + t.Fatalf("the new version is not by the caller with the why: %+v", last) + } + }) +} + +func TestAnAcceptedRecordRefusesWithdrawNamingWhy(t *testing.T) { + eachStore(t, func(t *testing.T, s Store) { + w := imported(t, s) + n := w.reserved(t, "author@test") + key := Key{KindDecision, n} + w.accepted(t, "author@test", valid(n)) + if _, err := w.k.WithdrawRecord("operator@shanks", key, "no longer wanted"); err == nil || !strings.Contains(err.Error(), "accepted") { + t.Fatalf("err %v", err) + } + e := mustGet(t, w.k.Store, key) + if e.Current.Status != StatusAccepted { + t.Fatalf("an accepted record was changed: %q", e.Current.Status) + } + }) +} + +func TestAWithdrawnRecordRefusesWithdrawAgain(t *testing.T) { + eachStore(t, func(t *testing.T, s Store) { + w := imported(t, s) + n := w.reserved(t, "author@test") + key := Key{KindDecision, n} + if _, err := w.k.WithdrawRecord("operator@shanks", key, "first"); err != nil { + t.Fatal(err) + } + if _, err := w.k.WithdrawRecord("operator@shanks", key, "again"); err == nil || !strings.Contains(err.Error(), "withdrawn") { + t.Fatalf("err %v", err) + } + }) +} + +func TestWithdrawNeedsWhyAndRefusesARecordItHoldsNoneOf(t *testing.T) { + eachStore(t, func(t *testing.T, s Store) { + w := imported(t, s) + n := w.reserved(t, "author@test") + if _, err := w.k.WithdrawRecord("operator@shanks", Key{KindDecision, n}, " "); err == nil { + t.Fatal("an empty why was accepted") + } + if _, err := w.k.WithdrawRecord("operator@shanks", Key{KindDecision, 999999}, "why"); err == nil { + t.Fatal("a record it holds none of was accepted") + } + }) +} diff --git a/modules/mesh-knowledge/module.json b/modules/mesh-knowledge/module.json index 33c8c71f..92b15f2d 100644 --- a/modules/mesh-knowledge/module.json +++ b/modules/mesh-knowledge/module.json @@ -31,7 +31,8 @@ "reserve", "propose", "import", - "return" + "return", + "withdraw" ] } ], @@ -50,7 +51,8 @@ "reserve", "propose", "import", - "return" + "return", + "withdraw" ] } ], -- 2.54.0 From 684e8801c4200f64a659878bae7d0e81a245625d Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 00:09:16 +0200 Subject: [PATCH 2/2] Take the ask back on the router when withdrawing a record (issue 1081, review round 1) WithdrawRecord only ended the waiting ask locally; the router kept it open, so the operator still saw Approve/Decline for a withdrawn record and an answer to it was dropped silently. It now also publishes a cancel on the router's subject once the withdraw is saved, as the controller's asker does; a cancel the router cannot be told of is logged and the withdraw still stands. The module's existing "uses": ["operator-channel"] already grants this; no manifest change needed, since the ask and cancel subjects are both under that seat. --- .../mesh-knowledge/cmd/mesh-knowledge/kb.go | 18 +++++++++--- .../cmd/mesh-knowledge/kit_test.go | 29 ++++++++++++------- .../cmd/mesh-knowledge/withdraw_test.go | 3 ++ 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go index c21d2b52..09ccd688 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go @@ -17,6 +17,7 @@ import ( "time" stdio "git.novox.be/novox/mesh-sdk/go" + "git.novox.be/novox/mesh-sdk/go/asks" ) // Unproven is who acted, as recorded, where the runtime proves no caller (hq issue 365): the SDK's word. The caller @@ -650,9 +651,10 @@ type Withdrawn struct { // WithdrawRecord withdraws a reserved or proposed record on the caller's decision, with why in one line: the // number stays spent (novox/hq ADR 0290 rule 5), as a new version by the caller. A pending review is cleared, and -// a waiting operator's ask is ended, both as part of that one version. Refused: a record this holds none of, an -// empty why, and a record already accepted (a research effort accepted at any status), superseded or withdrawn — -// only a reserved or a proposed record takes it. +// a waiting operator's ask is ended here and taken back on the router (a cancel on its subject, novox/hq ADR +// 0259), both as part of that one version; a cancel the router cannot be told of is logged, and the withdraw +// still stands. Refused: a record this holds none of, an empty why, and a record already accepted (a research +// effort accepted at any status), superseded or withdrawn — only a reserved or a proposed record takes it. func (k *KB) WithdrawRecord(caller string, key Key, why string) (*Withdrawn, error) { why = strings.TrimSpace(why) if why == "" { @@ -675,7 +677,8 @@ func (k *KB) WithdrawRecord(caller string, key Key, why string) (*Withdrawn, err r := cur.Clone() r.Status = StatusWithdrawn r.Judging = nil - if r.Ask != nil && r.Ask.Said == AskWaiting { + waiting := r.Ask != nil && r.Ask.Said == AskWaiting + if waiting { r.Ask.Said = "ended: the record was withdrawn" } full := fmt.Sprintf("withdrawn by %s: %s", caller, why) @@ -683,6 +686,13 @@ func (k *KB) WithdrawRecord(caller string, key Key, why string) (*Withdrawn, err return nil, fmt.Errorf("%s could not be written (%v); nothing was withdrawn", key, err) } k.emit(EventChanged, r, caller, full, e.Versions+1) + if waiting && k.Asks != nil { + id := cur.Ask.ID + if err := k.Asks.Publish(asks.CancelSubject(Module), map[string]string{"id": id}, "cancel."+id); err != nil { + k.logf("[mesh-knowledge] the ask %s for %s is taken back here, and the router could not be told (%v): it expires there, "+ + "and no answer to it is acted on", id, key, err) + } + } return &Withdrawn{Record: key.String(), Status: StatusWithdrawn, Says: fmt.Sprintf("%s is withdrawn: %s. Its number stays spent, and is never handed out again.", key, why)}, nil } diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/kit_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/kit_test.go index 07915e59..ca15ee58 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/kit_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/kit_test.go @@ -320,11 +320,13 @@ func (j *judge) judge(question string, schema map[string]any, model string) (jso return raw, "the agent on test (" + model + ")", nil } -// router is the operator channel, played: the asks published, and its own record of each, which only it writes. +// router is the operator channel, played: the asks published, its own record of each, which only it writes, and +// the cancels taken back on its subject. type router struct { - mu sync.Mutex - asked []asks.Ask - records map[string]routerRecord + mu sync.Mutex + asked []asks.Ask + records map[string]routerRecord + cancelled []string } func newRouter() *router { return &router{records: map[string]routerRecord{}} } @@ -332,14 +334,19 @@ func newRouter() *router { return &router{records: map[string]routerRecord{}} } func (r *router) Publish(subject string, body any, id string) error { r.mu.Lock() defer r.mu.Unlock() - if subject != asks.AskSubject(Module) { - return fmt.Errorf("published on %s", subject) + switch subject { + case asks.AskSubject(Module): + a := body.(asks.Ask) + r.asked = append(r.asked, a) + cp := a + r.records[a.ID] = routerRecord{State: "open", Ask: &cp} + return nil + case asks.CancelSubject(Module): + m := body.(map[string]string) + r.cancelled = append(r.cancelled, m["id"]) + return nil } - a := body.(asks.Ask) - r.asked = append(r.asked, a) - cp := a - r.records[a.ID] = routerRecord{State: "open", Ask: &cp} - return nil + return fmt.Errorf("published on %s", subject) } func (r *router) Record(id string) (*stdio.StateEntry, error) { diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go index aff1d3b9..26a59d21 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/withdraw_test.go @@ -62,6 +62,9 @@ func TestAProposedRecordNeedingTheOperatorIsWithdrawnWithItsAskEndedAndItsReview if after.Current.Ask == nil || after.Current.Ask.Said == AskWaiting { t.Fatalf("the waiting ask was not ended: %+v", after.Current.Ask) } + if !contains(w.router.cancelled, before.Current.Ask.ID) { + t.Fatalf("the router was not told to take back the ask %s: cancelled %v", before.Current.Ask.ID, w.router.cancelled) + } if after.Current.Judging != nil { t.Fatalf("a pending review was not cleared: %+v", after.Current.Judging) } -- 2.54.0