Merge pull request 'Let a proposed record be withdrawn (issue 1081)' (#355) from fix/1081-withdraw into main

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).

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 {

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
@@ -606,9 +607,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 +639,60 @@ 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 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 == "" {
+		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
+	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)
+	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)
+	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
+}

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) {

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)
+			}},
 	}
 }
 

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) {

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})

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

@@ -0,0 +1,123 @@
+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 !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)
+		}
+		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")
+		}
+	})
+}

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"
       ]
     }
   ],