From e3b4ecde8cf2c356055269f7865439a230c2cedc Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 22:42:00 +0100 Subject: [PATCH 1/7] Name the copy wait's ownership mark nonce, not token, so the state accepts it; the copy test's state refuses credential names as the bus does (issue 1966) Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ --- .../cmd/mesh-build-image/copy.go | 15 ++++---- .../cmd/mesh-build-image/helpers_test.go | 38 +++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/modules/mesh-build-image/cmd/mesh-build-image/copy.go b/modules/mesh-build-image/cmd/mesh-build-image/copy.go index 28655df7e..21fe20ecc 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/copy.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/copy.go @@ -99,7 +99,7 @@ func (m *Module) copy(ctx context.Context, args map[string]any) (any, error) { now = m.Now } asked := CopyWait{Digest: digest, Module: module, Artifact: name, From: from, Node: m.Node, - Asked: now().UTC(), Token: strconv.FormatInt(now().UnixNano(), 36) + "-" + m.Node} + Asked: now().UTC(), Nonce: strconv.FormatInt(now().UnixNano(), 36) + "-" + m.Node} // Waited for before the ask, in memory and in the state mirror-waits, so an outcome said at once, or heard by another // instance of the module (the seat's events reach one of them), is not missed. heard, stop := m.Outcomes.Expect(digest) @@ -107,7 +107,7 @@ func (m *Module) copy(ctx context.Context, args map[string]any) (any, error) { if _, err := m.Outcomes.Waits.Put(key, asked); err != nil { return nil, fmt.Errorf("%s%s: keeping the copy's wait in %s: %v", builds.NotStored, name, MirrorWaitsState, err) } - defer m.Outcomes.drop(key, asked.Token) + defer m.Outcomes.drop(key, asked.Nonce) said := buildstep.NarrateSeat(ctx, buildstep.ImageSeat, "mirror of "+from) raw, err := m.call()("seat:"+buildstep.ImageSeat+"."+reg.VerbMirror, reg.MirrorInput{Name: module + "/" + name, Source: from, Digest: digest}) if err != nil { @@ -170,7 +170,8 @@ const MirrorWaitsState = "mirror-waits" // MirrorWaitKey is a copy's key in mirror-waits: its digest, with the colon a key cannot hold as a dash. func MirrorWaitKey(digest string) string { return strings.Replace(digest, ":", "-", 1) } -// CopyWait is one value of mirror-waits: the copy asked, and its outcome once heard. +// CopyWait is one value of mirror-waits: the copy asked, and its outcome once heard. Nonce tells this copy's wait from +// a later copy's of the same digest; it is no credential, and no field here may be named as one (the state refuses it). type CopyWait struct { Digest string `json:"digest"` Module string `json:"module"` @@ -178,7 +179,7 @@ type CopyWait struct { From string `json:"from"` Node string `json:"node"` Asked time.Time `json:"asked"` - Token string `json:"token"` + Nonce string `json:"nonce"` Outcome *MirrorOutcome `json:"outcome,omitempty"` } @@ -281,9 +282,9 @@ func (o *Outcomes) kept(key string) (MirrorOutcome, bool) { return *w.Outcome, true } -// drop removes key's wait when it is still the copy's own (token): a later copy of the same digest keeps its own. -func (o *Outcomes) drop(key, token string) { - if w, err := o.read(key); err == nil && w != nil && w.Token == token { +// drop removes key's wait when it is still the copy's own (nonce): a later copy of the same digest keeps its own. +func (o *Outcomes) drop(key, nonce string) { + if w, err := o.read(key); err == nil && w != nil && w.Nonce == nonce { _ = o.Waits.Delete(key) } } diff --git a/modules/mesh-build-image/cmd/mesh-build-image/helpers_test.go b/modules/mesh-build-image/cmd/mesh-build-image/helpers_test.go index 024543477..4e01d5282 100644 --- a/modules/mesh-build-image/cmd/mesh-build-image/helpers_test.go +++ b/modules/mesh-build-image/cmd/mesh-build-image/helpers_test.go @@ -115,6 +115,9 @@ func (w *memWaits) Put(key string, value any) (uint64, error) { if err != nil { return 0, err } + if f := credentialShaped(b); f != "" { + return 0, fmt.Errorf("the state refuses a field %q, which names a credential", f) + } w.mu.Lock() if w.m == nil { w.m = map[string]json.RawMessage{} @@ -339,3 +342,38 @@ func tool(t *testing.T, m *Module, name string) func(map[string]any) (any, error t.Fatalf("no tool %s", name) return nil } + +// credentialShaped is the first field name in a JSON value that ends as the state's guard refuses (the bus's +// credentialEndings), or "": the in-memory state refuses it as the bus does, so every copy test meets the guard. +func credentialShaped(value []byte) string { + var v any + if json.Unmarshal(value, &v) != nil { + return "" + } + var walk func(any) string + walk = func(v any) string { + switch t := v.(type) { + case map[string]any: + for k, x := range t { + norm := strings.NewReplacer("-", "", "_", "", " ", "").Replace(strings.ToLower(k)) + for _, end := range []string{"password", "passwd", "secret", "token", "credential", "credentials", + "authorization", "apikey", "privatekey", "accesskey", "cookie"} { + if strings.HasSuffix(norm, end) { + return k + } + } + if f := walk(x); f != "" { + return f + } + } + case []any: + for _, x := range t { + if f := walk(x); f != "" { + return f + } + } + } + return "" + } + return walk(v) +} From 24bf59219ee5d72a1b202630d4525ab4863b9cbc Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 22:42:00 +0100 Subject: [PATCH 2/7] Distribution and the forge hold their caller list from its config watch, not an ask per call (issue 1966) Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ --- .../cmd/distribution-tools/contract_test.go | 2 +- modules/distribution/cmd/distribution-tools/main.go | 2 +- modules/distribution/internal/holder/callers.go | 12 ++++++++---- modules/gitea/internal/packageseats/holder.go | 10 ++++++---- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/modules/distribution/cmd/distribution-tools/contract_test.go b/modules/distribution/cmd/distribution-tools/contract_test.go index d2453f9ea..7bd5c1e22 100644 --- a/modules/distribution/cmd/distribution-tools/contract_test.go +++ b/modules/distribution/cmd/distribution-tools/contract_test.go @@ -57,7 +57,7 @@ func TestTheVerbsSpeakTheSeatsContract(t *testing.T) { t.Errorf("%s's body %s is not the contract's: %v", event, b, err) } } - // The caller list is read on each call: one that cannot be read answers no verb, and registry_callers says it as read. + // The caller list is read held: one never read answers no verb, and registry_callers says it as read. unread := func() (holder.Callers, error) { return nil, errors.New("config refused") } for _, tool := range tools(&holder.Verbs{}, kept[holder.Window]{}, unread) { if tool.Name == holder.ImageSeat+"."+holder.VerbVersions { diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index dc80fc85d..4270a2adb 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -248,7 +248,7 @@ func arg[T any](args map[string]any) (T, error) { func str(args map[string]any, k string) string { s, _ := args[k].(string); return strings.TrimSpace(s) } -// tools are distribution's tools over v; each call that checks a caller reads the caller list on that call (callers: +// tools are distribution's tools over v; each call that checks a caller reads the caller list held (callers: // the config value as mesh-config resolves it now), so an operator's change of it holds from the next call on. func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder.Callers, error)) []stdio.Tool { now := func() (*holder.Verbs, error) { diff --git a/modules/distribution/internal/holder/callers.go b/modules/distribution/internal/holder/callers.go index b00894c82..b3021139e 100644 --- a/modules/distribution/internal/holder/callers.go +++ b/modules/distribution/internal/holder/callers.go @@ -93,11 +93,15 @@ func ParseCallers(raw string) (Callers, error) { return selfNotReleasing(c) } -// ConfiguredCallers is the config value `callers` as mesh-config resolved it for this machine, read through the tools -// runtime's stdio.Config on each call (never an environment variable: nothing renders a config value into a -// program's environment); unset is DefaultCallers, and Self on release's list is refused as ParseCallers refuses it. +// configured holds the config value `callers`: asked of the runtime once, by a watch, and replaced on each change of +// the value, so a call never asks (issue 1966: an ask that timed out refused every verb). +var configured = stdio.HoldVerbCallers(CallersConfig, VerbsOf(ImageSeat), DefaultCallers()) + +// ConfiguredCallers is the config value `callers` as mesh-config resolved it for this machine, held from the tools +// runtime's watch of it (never an environment variable: nothing renders a config value into a program's environment); +// unset is DefaultCallers, and Self on release's list is refused as ParseCallers refuses it. func ConfiguredCallers() (Callers, error) { - c, err := stdio.ConfiguredVerbCallers(CallersConfig, VerbsOf(ImageSeat), DefaultCallers()) + c, err := configured() if err != nil { return nil, err } diff --git a/modules/gitea/internal/packageseats/holder.go b/modules/gitea/internal/packageseats/holder.go index 244f1322d..ea56b67f0 100644 --- a/modules/gitea/internal/packageseats/holder.go +++ b/modules/gitea/internal/packageseats/holder.go @@ -77,10 +77,12 @@ func DefaultCallers() Callers { VerbVersions: {stdio.AnyCaller}} } -// ConfiguredCallers is the config value `callers` as mesh-config resolved it; unset is DefaultCallers. -func ConfiguredCallers() (Callers, error) { - return stdio.ConfiguredVerbCallers("callers", Verbs, DefaultCallers()) -} +// configured holds the config value `callers`: asked of the runtime once, by a watch, and replaced on each change of +// the value, so a call never asks (issue 1966). +var configured = stdio.HoldVerbCallers("callers", Verbs, DefaultCallers()) + +// ConfiguredCallers is the config value `callers` as mesh-config resolved it, held from its watch; unset is DefaultCallers. +func ConfiguredCallers() (Callers, error) { return configured() } // Publish is a publish as the seats' contract names it (mesh-contracts npmpackageregistry and gopackageregistry share // one shape): what the tools bundle reads from either seat's PublishInput. From 6299b47f4f953b21b5df2a661a280efa980382f1 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 22:42:09 +0100 Subject: [PATCH 3/7] A mesh package the Go seat lacks is fetched from the forge's git, not stopped by an unreachable public proxy (issue 1966) GOPROXY joined the Go seat, the public proxy and direct with commas, so go went on only after a 404 or 410: a contracts package the seat does not hold, and a public proxy a node cannot reach, ended the fetch before direct. It joins them with | now, so any failure goes on to the next. go words a proxy it cannot reach as "invalid version", which the pin path read as an unknown revision and labelled "on no pull request head"; a network failure is never that now (network is unreachable and no route to host are network failures). Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ --- modules/mesh-build-go/cmd/mesh-build-go/fetch.go | 5 ++++- modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go | 2 +- modules/mesh-build-go/cmd/mesh-build-go/versions.go | 7 ++++++- modules/mesh-build-go/cmd/mesh-build-go/versions_test.go | 4 ++++ 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go index 59a06ade8..95acdde8f 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go @@ -334,7 +334,10 @@ func (m *Module) goEnv() []string { // The Go seat first, the public proxy second, the forge's git last (CAT-BG3, E11: direct goes in the first // change after mesh-sdk's and mesh-contracts' first numbered versions are in the Go seat); no GOPRIVATE, and // the checksum database off for the forge's paths, which go.sum and the builder's h1: digest guard instead. - env = append(env, "GOPROXY="+proxy+","+PublicGoProxy+",direct", "GONOSUMDB="+ForgeGoPaths, "GOPRIVATE=") + // The separator is "|", not ",": with "," go goes on only after a 404 or 410, so a seat that does not hold a + // package (a contracts package not published yet) and a public proxy the node cannot reach stopped the fetch + // before "direct" (issue 1966); with "|" any failure goes on to the next, and direct reaches the forge's git. + env = append(env, "GOPROXY="+proxy+"|"+PublicGoProxy+"|direct", "GONOSUMDB="+ForgeGoPaths, "GOPRIVATE=") } else { env = append(env, "GOPRIVATE="+ForgeGoPaths) } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go index e01fd9b45..f1387604c 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go @@ -153,7 +153,7 @@ func TestFetchAsksGitForTheSshRemoteAndDownloadsEachModule(t *testing.T) { } m.GoSeatFile = seat env := strings.Join(m.goEnv(), " ") - for _, want := range []string{"GOPROXY=http://forge.mesh:3000/api/packages/novox/go,https://proxy.golang.org,direct", "GONOSUMDB=git.novox.be", "GOPRIVATE= "} { + for _, want := range []string{"GOPROXY=http://forge.mesh:3000/api/packages/novox/go|https://proxy.golang.org|direct", "GONOSUMDB=git.novox.be", "GOPRIVATE= "} { if !strings.Contains(env+" ", want) { t.Errorf("with the Go seat bound the environment lacks %q: %s", want, env) } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/versions.go b/modules/mesh-build-go/cmd/mesh-build-go/versions.go index 4d182addd..694ed1e7c 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/versions.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/versions.go @@ -108,7 +108,7 @@ const ( // reset connection), not of a version or a checksum that is wrong. func networkFailure(s string) bool { s = strings.ToLower(s) - for _, w := range []string{"i/o timeout", "no such host", "temporary failure in name resolution", "connection refused", "connection reset", "tls handshake timeout", "lookup "} { + for _, w := range []string{"i/o timeout", "no such host", "temporary failure in name resolution", "connection refused", "connection reset", "tls handshake timeout", "network is unreachable", "no route to host", "lookup "} { if strings.Contains(s, w) { return true } @@ -348,5 +348,10 @@ func comparePre(a, b string) int { // "invalid version" it wraps it in): it labels a commit the forge holds on no pull request head (placePin), and decides // nothing; every other failure is the network's or the forge's. func unknownRevision(failure string) bool { + // go words any failure of a proxy as "invalid version: ..." (issue 1966): a failure of the network is not go knowing + // no such revision, whatever else the text says. + if networkFailure(failure) { + return false + } return strings.Contains(failure, "unknown revision") || strings.Contains(failure, "invalid version") } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go b/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go index e24c6415b..865510cc1 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/versions_test.go @@ -49,6 +49,10 @@ func TestAnAskedVersionIsWrittenIntoTheModfileWhateverGoModNames(t *testing.T) { built: commitSHA, said: "placed from the pull request's head"}, {name: "a lookup that times out is no pin", asked: offHead, failing: true, failsWith: "i/o timeout", answer: `{"Path":"` + sdk + `","Error":"git.novox.be/novox/mesh-sdk/go@` + offHead + `: dial tcp: lookup git.novox.be on 10.10.0.3:53: read udp 10.10.0.3:42292->10.10.0.3:53: i/o timeout"}`}, + // Issue 1966: go words a public proxy it cannot reach as "invalid version"; that is the network's failure, not an + // unknown revision, so a trunk commit on no pull request head is not labelled "on no pull request head". + {name: "an unreachable proxy worded invalid version is no pin", asked: offHead, failing: true, failsWith: "network is unreachable", + answer: `{"Path":"` + sdk + `","Error":"invalid version: Get \"https://proxy.golang.org/` + sdk + `/@v/` + offHead + `.info\": dial tcp [2a00:1450::1]:443: connect: network is unreachable"}`}, // The forge's own listing failing is a failure that may be retaken too, with its own cause (review 3). {name: "a forge that cannot be listed is retaken", asked: commitSHA, failing: true, forgeDown: true, failsWith: "unable to access the forge", From 407fc6c9e625cd98f192208db722ba839ee18bcd Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 22:55:53 +0100 Subject: [PATCH 4/7] The Go seat goes on to the public proxy only on a 404, the public proxy to direct on any failure (issue 1966, review 1) Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ --- modules/mesh-build-go/cmd/mesh-build-go/fetch.go | 9 +++++---- modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go index 95acdde8f..1a17af061 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch.go @@ -334,10 +334,11 @@ func (m *Module) goEnv() []string { // The Go seat first, the public proxy second, the forge's git last (CAT-BG3, E11: direct goes in the first // change after mesh-sdk's and mesh-contracts' first numbered versions are in the Go seat); no GOPRIVATE, and // the checksum database off for the forge's paths, which go.sum and the builder's h1: digest guard instead. - // The separator is "|", not ",": with "," go goes on only after a 404 or 410, so a seat that does not hold a - // package (a contracts package not published yet) and a public proxy the node cannot reach stopped the fetch - // before "direct" (issue 1966); with "|" any failure goes on to the next, and direct reaches the forge's git. - env = append(env, "GOPROXY="+proxy+"|"+PublicGoProxy+"|direct", "GONOSUMDB="+ForgeGoPaths, "GOPRIVATE=") + // The seat goes on to the public proxy only on a 404 or 410 (",": a package it does not hold), so a seat that + // fails otherwise (5xx, refused, timeout) fails the fetch loudly; the public proxy goes on to direct on any + // failure ("|"): a public proxy the node cannot reach stopped the fetch before direct, and direct reaches the + // forge's git for a package the seat does not hold (issue 1966, ADR 0605 §6). + env = append(env, "GOPROXY="+proxy+","+PublicGoProxy+"|direct", "GONOSUMDB="+ForgeGoPaths, "GOPRIVATE=") } else { env = append(env, "GOPRIVATE="+ForgeGoPaths) } diff --git a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go index f1387604c..d76472a6f 100644 --- a/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go +++ b/modules/mesh-build-go/cmd/mesh-build-go/fetch_test.go @@ -153,7 +153,7 @@ func TestFetchAsksGitForTheSshRemoteAndDownloadsEachModule(t *testing.T) { } m.GoSeatFile = seat env := strings.Join(m.goEnv(), " ") - for _, want := range []string{"GOPROXY=http://forge.mesh:3000/api/packages/novox/go|https://proxy.golang.org|direct", "GONOSUMDB=git.novox.be", "GOPRIVATE= "} { + for _, want := range []string{"GOPROXY=http://forge.mesh:3000/api/packages/novox/go,https://proxy.golang.org|direct", "GONOSUMDB=git.novox.be", "GOPRIVATE= "} { if !strings.Contains(env+" ", want) { t.Errorf("with the Go seat bound the environment lacks %q: %s", want, env) } From 62644aa012fd36ea9bcdeb889c4da635bc54b673 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 23:13:55 +0100 Subject: [PATCH 5/7] The knowledge base's review knows its round: from round 3 only a wrong record asks changes, at round 5 the operator decides (issue 784, ADR 0665) Claude-Session: https://claude.ai/code/session_01JG73KK8HD1gufnqyFxMFdW --- .../mesh-knowledge/cmd/mesh-knowledge/kb.go | 10 +- .../cmd/mesh-knowledge/operator.go | 64 +++++++- .../cmd/mesh-knowledge/record.go | 8 +- .../cmd/mesh-knowledge/review.go | 10 +- .../cmd/mesh-knowledge/review_parts.go | 3 +- .../cmd/mesh-knowledge/review_round.go | 66 ++++++++ .../cmd/mesh-knowledge/review_round_test.go | 144 ++++++++++++++++++ 7 files changed, 296 insertions(+), 9 deletions(-) create mode 100644 modules/mesh-knowledge/cmd/mesh-knowledge/review_round.go create mode 100644 modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go index ec4edba72..87741c18f 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/kb.go @@ -364,6 +364,10 @@ func (k *KB) Propose(caller string, a ProposeAsk) (*Proposed, error) { current := true why := strings.TrimSpace(a.Why) if held != nil { + if w := held.Working(); w.Status == StatusProposed && w.Ask != nil && w.Ask.Said == AskWaiting && w.Ask.Round > 0 { + return nil, fmt.Errorf("%s is at round %d of its review, which asked changes: the operator decides it, and no review runs before the "+ + "answer. Nothing was proposed; propose again after the operator's answer", key, w.Ask.Round) + } switch w := held.Working(); { case held.Current.Status == StatusReserved: r.ReservedBy, r.ReservedAt, r.ReservedFor = held.Current.ReservedBy, held.Current.ReservedAt, held.Current.ReservedFor @@ -380,7 +384,7 @@ func (k *KB) Propose(caller string, a ProposeAsk) (*Proposed, error) { "Nothing was proposed", key, w.Author, caller) } r.Author, r.Reviews, r.ReservedBy, r.ReservedAt, r.ReservedFor = w.Author, w.Reviews, w.ReservedBy, w.ReservedAt, w.ReservedFor - r.Parts = w.Parts + r.Parts, r.CountFrom = w.Parts, w.CountFrom r.Writers = w.Writers if a.Insight != "" { refusals = append(refusals, Refusal{Rule: "reserved", Says: "a proposal not yet accepted is revised, not corrected by an insight"}) @@ -411,6 +415,10 @@ func (k *KB) Propose(caller string, a ProposeAsk) (*Proposed, error) { } r.AcceptedAt = held.Current.AcceptedAt r.Parts = held.Current.Parts + if held.Pending != nil { + // A revision of a pending version keeps its earlier reviews, so the round count goes on (ADR 0665 §1). + r.Reviews, r.CountFrom = held.Pending.Reviews, held.Pending.CountFrom + } r.Pending = &PendingState{Insight: Insight{Date: k.now().Format("2006-01-02"), Before: strings.TrimSpace(a.Insight), By: caller}} r.Date = held.Current.Date r.Effort = a.Status diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/operator.go b/modules/mesh-knowledge/cmd/mesh-knowledge/operator.go index 202ac029a..5bdfb7bfc 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/operator.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/operator.go @@ -92,6 +92,9 @@ const AskGlance = 1 // again to hold the warrant to it — in the form it was asked in, so an ask made before a form changed is still // held to the words the operator was shown. func composeAsk(r *Record, a *OperatorAsk) oc.Ask { + if a.Round > 0 { + return changesAsk(r, a) + } if a.Form == AskGlance { return glanceAsk(r, a) } @@ -124,6 +127,44 @@ func glanceAsk(r *Record, a *OperatorAsk) oc.Ask { return withOptions(r, a, oc.Ask{ID: a.ID, Headline: headline, Explanation: explanation, Details: details}) } +// changesAsk is the ask of a record whose round-5 review asked changes (ADR 0665 §3): the findings, read at a glance, +// and the two answers. It is composed from the record and its ask alone: the findings are the last review's. +func changesAsk(r *Record, a *OperatorAsk) oc.Ask { + name := r.Key().String() + var b strings.Builder + fmt.Fprintf(&b, "The knowledge base's review of %s asked changes at round %d, the last round: you decide. Approve accepts the version as written; "+ + "Decline leaves it proposed for its author, and its next review starts at round 1.", name, a.Round) + if t := strings.TrimSuffix(strings.TrimSpace(r.Title), "."); shown(t) { + fmt.Fprintf(&b, "\n\nIts title: %s.", t) + } + if n := len(r.Reviews); n > 0 { + b.WriteString("\n\nThe findings:") + size, hidden := 0, 0 + for _, f := range strings.Split(r.Reviews[n-1].Findings, "\n") { + if f = strings.TrimSpace(f); f == "" { + continue + } + if rs := []rune(f); len(rs) > 300 { + f = string(rs[:300]) + "..." + } + if !shown(f) || size+len(f) > 1500 { + hidden++ + continue + } + size += len(f) + fmt.Fprintf(&b, "\n- %s", f) + } + if hidden > 0 { + fmt.Fprintf(&b, "\n(%d finding(s) not shown here)", hidden) + } + } + fmt.Fprintf(&b, "\n\nRead it whole with the knowledge base's get, number %d; its fingerprint is %s.", r.Number, fingerprint(r.Digest())) + out := withOptions(r, a, oc.Ask{ID: a.ID, Headline: fmt.Sprintf("%s: accept it as written after round %d?", name, a.Round), Explanation: b.String()}) + out.Options[0].Does = "the version is accepted as written and its clauses are in force" + out.Options[1].Does = "the record stays proposed for its author; its next review is round 1" + return out +} + // listedAsk is the ask as it was composed before the brief: every clause's lead listed. Composed only to hold // the warrant of an ask made then. func listedAsk(r *Record, a *OperatorAsk) oc.Ask { @@ -180,7 +221,11 @@ func newAskID(r *Record) string { // it. The caller holds the write lock. An ask that could not be made or published is taken back, and the review is // asked again after a pause, counted with the tries before it (tries): the record stays proposed, and is reviewed // at most MostReviewTries times. -func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string, tries int) (string, error) { +// +// round is 0 for a record the review approved. For round MostReviewRounds it is that round: the review asked changes, +// and the operator is asked to accept the version as written or leave it proposed (ADR 0665 §3). An ask not made +// then takes the round's review back, so its retry is the same round. +func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string, tries int, round int) (string, error) { version := held.CurrentVersion if held.Pending != nil { version = held.PendingVersion @@ -189,6 +234,10 @@ func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string, w.Ask = nil w.Judging = failedJudging(tries+1, why, k.now()) said := fmt.Sprintf("approved by the knowledge base's review, and %s (%s", why, w.Judging.owedSays()) + if round > 0 { + w.Reviews = w.Reviews[:len(w.Reviews)-1] + said = fmt.Sprintf("round %d asked changes, and %s (%s", round, why, w.Judging.owedSays()) + } e, err := k.Store.Get(ctx, w.Key()) if err != nil || e == nil { return "", fmt.Errorf("%s: %s, and that could not be kept (%v)", w.Key(), said, err) @@ -203,17 +252,21 @@ func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string, if k.Asks == nil { return unasked("the operator cannot be asked from here") } - oa := &OperatorAsk{ID: newAskID(w), At: k.now(), Version: version, Said: AskWaiting, Form: AskGlance} + oa := &OperatorAsk{ID: newAskID(w), At: k.now(), Version: version, Said: AskWaiting, Form: AskGlance, Round: round} ask := composeAsk(w, oa) if err := occ.Check(ask, k.now()); err != nil { return unasked(fmt.Sprintf("the ask could not be made (%v)", err)) } w.Ask = oa - if err := k.Store.Save(ctx, Version{By: by, At: k.now(), Why: "reviewed: approved; the operator is asked", Record: w, Insight: held.Pending != nil && held.Pending.IsInsight()}, + asked := "approved; the operator is asked" + if round > 0 { + asked = fmt.Sprintf("round %d asked changes; the operator is asked", round) + } + if err := k.Store.Save(ctx, Version{By: by, At: k.now(), Why: "reviewed: " + asked, Record: w, Insight: held.Pending != nil && held.Pending.IsInsight()}, held.Pending == nil, false, held.Revision); err != nil { return "", fmt.Errorf("%s: the review could not be kept (%v); the operator was not asked", w.Key(), err) } - k.emit(EventReviewed, w, by, "approved; the operator is asked", held.Versions+1) + k.emit(EventReviewed, w, by, asked, held.Versions+1) if err := k.Asks.Publish(oc.AcceptSubjectAsk(Module), ask, ask.ID); err != nil { k.logf("[mesh-knowledge] the ask for %s could not be published: %v", w.Key(), err) return unasked(fmt.Sprintf("the operator could not be asked (%v)", err)) @@ -351,6 +404,9 @@ func (k *KB) decide(ctx context.Context, e *Entry, w *Record, warrant oc.Warrant case OptionDecline: w = w.Clone() w.Ask.Warrant = occ.Says(warrant) + if w.Ask.Round > 0 { + w.CountFrom = len(w.Reviews) // the revision's review starts a new count at round 1 (ADR 0665 §3) + } return k.endAsk(ctx, e, w, "declined by the operator") } return k.endAsk(ctx, e, w, "was answered with "+option.ID+", which it does not offer") diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/record.go b/modules/mesh-knowledge/cmd/mesh-knowledge/record.go index 79cf58fe4..1cc60e6e9 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/record.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/record.go @@ -264,6 +264,9 @@ type OperatorAsk struct { // Held is why the router holds the ask while it waits: no channel can carry it yet (ADR 0397, issue 926). // Empty once the router offers it, or when it ended. Held string `json:"held,omitempty"` + // Round is the review round whose changes this ask lays before the operator (ADR 0665 §3); 0 for an ask of + // the approval of a record the review approved. + Round int `json:"round,omitempty"` } // Drafting is where a prose record stands in the drafting and confirming of its clauses (to-be 55 §7). @@ -329,6 +332,9 @@ type Record struct { // AcceptedAt is when it was accepted here; zero for one imported accepted. AcceptedAt time.Time `json:"accepted-at,omitempty"` Reviews []Review `json:"reviews,omitempty"` + // CountFrom is how many of Reviews came before the round count last restarted: the operator's Decline of a + // round-5 ask restarts it (ADR 0665 §1, §3), so the next review is round 1 (review_round.go). + CountFrom int `json:"count-from,omitempty"` // Parts are the latest verdict of each part of the version (the head, each section), kept for a new version whose // part is unchanged (review_parts.go). Parts []PartReview `json:"parts,omitempty"` @@ -484,7 +490,7 @@ func (r *Record) Clone() *Record { // it. func (r *Record) Digest() string { c := r.Clone() - c.Reviews, c.Ask, c.Drafting, c.Judging, c.Parts, c.FormOnly = nil, nil, nil, nil, nil, false + c.Reviews, c.Ask, c.Drafting, c.Judging, c.Parts, c.FormOnly, c.CountFrom = nil, nil, nil, nil, nil, false, 0 raw, _ := json.Marshal(c) sum := sha256.Sum256(raw) return "sha256:" + hex.EncodeToString(sum[:]) diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review.go index c2208f5c3..bd61bc25b 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/review.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review.go @@ -121,7 +121,8 @@ func ReviewQuestion(r *Record, linked []*Record) string { "what it decides is checked; and every link fits: a record it changes says which clause and really changes that "+ "clause, a record it supersedes is really replaced whole, and no clause contradicts a clause in force of a record it "+ "links without changing or superseding it. Answer changes otherwise, with findings, each naming the section or "+ - "clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n\n", what, r.Key()) + "clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n", what, r.Key()) + b.WriteString(roundText(r) + "\n") b.WriteString("The rules every record was already checked against when it was proposed:\n") for _, rule := range PerRecordRules { fmt.Fprintf(&b, "- %s\n", rule.Holds) @@ -536,7 +537,12 @@ func (k *KB) reviewOne(key Key) (string, error) { } } } + round := w.Round() w.Reviews = append(w.Reviews, Review{By: by, At: k.now(), Verdict: verdict, Findings: strings.Join(findings, "\n"), Version: version}) + if verdict == VerdictChanges && round >= MostReviewRounds { + // Round 5's changes are the operator's to decide (ADR 0665 §3): no review runs before the answer. + return k.askOperator(ctx, now, w, by, tries, round) + } if verdict == VerdictChanges { why := fmt.Sprintf("reviewed: changes asked (%d finding(s)); it stays proposed for its author to revise", len(findings)) if err := save(w, why); err != nil { @@ -546,7 +552,7 @@ func (k *KB) reviewOne(key Key) (string, error) { return key.String() + ": " + why, nil } if w.NeedsOperator { - return k.askOperator(ctx, now, w, by, tries) + return k.askOperator(ctx, now, w, by, tries, 0) } k.emit(EventReviewed, w, by, "approved", now.Versions+1) r, err := k.accept(ctx, now, w, by, "accepted after the knowledge base's review", "") diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review_parts.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review_parts.go index e55af336d..d4fa6b428 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/review_parts.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review_parts.go @@ -71,7 +71,8 @@ func introFor(r *Record, what string) string { "what it decides is checked; and every link fits: a record it changes says which clause and really changes that "+ "clause, a record it supersedes is really replaced whole, and no clause contradicts a clause in force of a record it "+ "links without changing or superseding it. Answer changes otherwise, with findings, each naming the section or "+ - "clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n\n", about, r.Key(), what) + "clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n", about, r.Key(), what) + b.WriteString(roundText(r) + "\n") b.WriteString("The rules every record was already checked against when it was proposed:\n") for _, rule := range PerRecordRules { fmt.Fprintf(&b, "- %s\n", rule.Holds) diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review_round.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round.go new file mode 100644 index 000000000..f6e0dc608 --- /dev/null +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round.go @@ -0,0 +1,66 @@ +package main + +// The review knows its round (novox/hq ADR 0665, issue 784). A record's round is the number of its reviews since the +// last of: it was accepted, it was first proposed, the operator declined it at round MostReviewRounds, plus one. +// Reviews belong to the version being worked (a pending version starts with none) and CountFrom marks the Decline, +// so the count is what is on the record. The question names the round and lists the earlier findings of the count; +// from round OnlyWrongFrom only a finding that makes the record wrong asks changes; at round MostReviewRounds a +// review that asks changes becomes an ask of the operator (askOperator with a round), and no review runs before the +// answer (Propose refuses a revision meanwhile). + +import ( + "fmt" + "strings" +) + +const ( + // MostReviewRounds is the round at which the operator decides. + MostReviewRounds = 5 + // OnlyWrongFrom is the first round in which only a finding that makes the record wrong asks changes. + OnlyWrongFrom = 3 + // MostEarlierFindings bounds the earlier findings a question lists, the latest rounds kept. + MostEarlierFindings = 6 << 10 +) + +// Round is the round of the next review of r. +func (r *Record) Round() int { + from := r.CountFrom + if from > len(r.Reviews) { + from = len(r.Reviews) + } + return len(r.Reviews) - from + 1 +} + +// roundText is what every part's question says of the round: its number, what the round allows, and the findings +// of the earlier rounds of the count. +func roundText(r *Record) string { + round := r.Round() + var b strings.Builder + fmt.Fprintf(&b, "\nThis is round %d of its review. At round %d the operator decides, so name every finding at once; "+ + "a finding an earlier round named and the record answered is not named again.\n", round, MostReviewRounds) + if round >= OnlyWrongFrom { + b.WriteString("From round 3 only a finding that makes the record wrong asks changes: two of its parts contradict each other, " + + "or it contradicts a clause in force of a record it links without changing or superseding that clause, or it breaks one of the " + + "rules above, or a link the whole-set rule needs is missing. Anything else, such as wording, a further case, a nicer form or a " + + "detail its design can hold, is a remark: answer approve and give it as a finding under approve.\n") + } + var earlier []string + for i := r.CountFrom; i < len(r.Reviews) && i >= 0; i++ { + if rv := r.Reviews[i]; rv.Verdict == VerdictChanges && strings.TrimSpace(rv.Findings) != "" { + earlier = append(earlier, fmt.Sprintf("Round %d (version %d) asked changes:\n%s\n", i-r.CountFrom+1, rv.Version, rv.Findings)) + } + } + size, from := 0, len(earlier) + for from > 0 && size+len(earlier[from-1]) <= MostEarlierFindings { + from-- + size += len(earlier[from]) + } + if from < len(earlier) { + b.WriteString("\nThe findings of the earlier rounds:\n") + if from > 0 { + fmt.Fprintf(&b, "(%d earlier round(s) are not shown: the question has no room for them.)\n", from) + } + b.WriteString(strings.Join(earlier[from:], "\n")) + } + return b.String() +} diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go new file mode 100644 index 000000000..b98db7a92 --- /dev/null +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go @@ -0,0 +1,144 @@ +package main + +import ( + "fmt" + "strings" + "testing" + "time" + + occ "git.novox.be/novox/mesh-contracts/operator-and-conversation/go/clients/operatorchannelclient" +) + +// ADR 0665: the review knows its round; from round 3 only a record that is wrong asks changes; at round 5 changes +// become an ask of the operator, no review runs before the answer, and a Decline restarts the count. + +func changes(i int) string { + return fmt.Sprintf(`{"verdict":"changes","findings":["finding of review %d"]}`, i) +} + +// review is the i'th review of record n: its author revises it first (but for the first), then every part is +// judged with answer. It answers the questions the judge was asked. +func (w *world) review(t *testing.T, n, i int, answer string) string { + t.Helper() + p := valid(n) + p.Sections[2].Text = fmt.Sprintf("Merges land sooner; revision %d.", i) + if _, err := w.k.Propose("author@g14", p); err != nil { + t.Fatal(err) + } + w.judge.review, w.judge.reviews = answer, nil + w.reviewAll(t) + return strings.Join(w.judge.reviews, "\n") +} + +func TestTheReviewIsToldItsRoundAndRoundFiveIsTheOperators(t *testing.T) { + w := imported(t, newMemStore()) + n := w.reserved(t, "author@g14") + key := Key{KindDecision, n} + for i := 1; i <= 4; i++ { + q := w.review(t, n, i, changes(i)) + if !strings.Contains(q, fmt.Sprintf("This is round %d of its review", i)) || !strings.Contains(q, "At round 5 the operator decides") || + strings.Contains(q, "From round 3 only") != (i >= 3) { + t.Fatalf("round %d's question: %.600s", i, q) + } + for j := 1; j < i; j++ { // every earlier round's findings, none of this round's + if !strings.Contains(q, fmt.Sprintf("Round %d (version", j)) || !strings.Contains(q, fmt.Sprintf("finding of review %d", j)) { + t.Errorf("round %d's question lacks the findings of round %d", i, j) + } + } + if strings.Contains(q, fmt.Sprintf("finding of review %d", i)) { + t.Errorf("round %d's question holds its own finding", i) + } + } + if e := mustGet(t, w.k.Store, key); e.Current.Status != StatusProposed || len(e.Current.Reviews) != 4 || e.Current.Round() != 5 || len(w.router.asked) != 0 { + t.Fatalf("after four rounds: %s, %d reviews, %d asks", e.Current.Status, len(e.Current.Reviews), len(w.router.asked)) + } + + // Round 5 asks changes: the operator is asked, showing the findings; no review runs, and no revision is taken, before the answer. + if q := w.review(t, n, 5, changes(5)); !strings.Contains(q, "This is round 5 of its review") { + t.Fatalf("round 5's question: %.300s", q) + } + if len(w.router.asked) != 1 { + t.Fatalf("asks after round 5's changes: %d", len(w.router.asked)) + } + ask := w.router.asked[0] + if err := occ.Check(ask, w.clock); err != nil || !strings.Contains(ask.Explanation, "finding of review 5") || !strings.Contains(ask.Explanation, "round 5") { + t.Fatalf("the ask: %v %+v", err, ask) + } + if e := mustGet(t, w.k.Store, key); e.Current.Ask == nil || e.Current.Ask.Round != 5 || e.Current.Status != StatusProposed { + t.Fatalf("after round 5: %+v", e.Current.Ask) + } + w.judge.reviews = nil + if said := w.reviewAll(t); len(said) != 0 || len(w.judge.reviews) != 0 { + t.Fatalf("a review ran before the operator answered: %v", said) + } + p := valid(n) + p.Sections[2].Text = "Merges land sooner; revision 6." + if _, err := w.k.Propose("author@g14", p); err == nil || !strings.Contains(err.Error(), "the operator decides") { + t.Fatalf("a revision before the answer: %v", err) + } + + // Decline: it stays proposed, and the revision's review is round 1. + w.router.answer(ask.ID, OptionDecline, w.clock.Add(time.Minute)) + if _, err := w.k.Reconcile(); err != nil { + t.Fatal(err) + } + if q := w.review(t, n, 6, changes(6)); !strings.Contains(q, "This is round 1 of its review") || strings.Contains(q, "finding of review") { + t.Fatalf("after the Decline: %.400s", q) + } + if len(w.router.asked) != 1 { + t.Fatalf("round 1 asked the operator") + } +} + +func TestRoundFiveApprovedByTheOperatorAcceptsTheVersionAsWritten(t *testing.T) { + w := imported(t, newMemStore()) + n := w.reserved(t, "author@g14") + for i := 1; i <= 5; i++ { + w.review(t, n, i, changes(i)) + } + w.router.answer(w.router.asked[0].ID, OptionApprove, w.clock.Add(time.Minute)) + if _, err := w.k.Reconcile(); err != nil { + t.Fatal(err) + } + if e := mustGet(t, w.k.Store, Key{KindDecision, n}); e.Current.Status != StatusAccepted || e.Current.Ask.Said != AskApproved { + t.Fatalf("%s %+v", e.Current.Status, e.Current.Ask) + } +} + +// From round 3 a finding that is only a remark sits under approve, and the record is accepted. +func TestARemarkUnderApproveAtRoundFourAcceptsTheRecord(t *testing.T) { + w := imported(t, newMemStore()) + n := w.reserved(t, "author@g14") + for i := 1; i <= 3; i++ { + w.review(t, n, i, changes(i)) + } + q := w.review(t, n, 4, `{"verdict":"approve","findings":["wording: the second section could say it shorter"]}`) + if !strings.Contains(q, "answer approve and give it as a finding under approve") { + t.Fatalf("the question: %.400s", q) + } + if e := mustGet(t, w.k.Store, Key{KindDecision, n}); e.Current.Status != StatusAccepted || len(e.Current.Reviews) != 4 { + t.Fatalf("%s, %d reviews", e.Current.Status, len(e.Current.Reviews)) + } +} + +// The rounds of a pending version (an insight to an accepted record) go on across its revisions. +func TestTheRoundCountGoesOnAcrossRevisionsOfAPendingInsight(t *testing.T) { + w := imported(t, newMemStore()) + n := w.reserved(t, "author@g14") + w.accepted(t, "author@g14", valid(n)) + var q string + for i := 1; i <= 3; i++ { + p := valid(n) + p.Insight = "it said only that merges land sooner" + p.Sections[2].Text = fmt.Sprintf("Merges land sooner, revision %d.", i) + if _, err := w.k.Propose("author@g14", p); err != nil { + t.Fatal(err) + } + w.judge.review, w.judge.reviews = changes(i), nil + w.reviewAll(t) + q = strings.Join(w.judge.reviews, "\n") + } + if !strings.Contains(q, "This is round 3 of its review") { + t.Fatalf("the third revision of the insight: %.300s", q) + } +} From 09ea4a40bfa96f3efbd9d469519adb7d2d33b47a Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 23:20:22 +0100 Subject: [PATCH 6/7] A closing keeps the review round, get says the round, a close waits for the operator's round-5 answer (issue 784, ADR 0665) Claude-Session: https://claude.ai/code/session_01JG73KK8HD1gufnqyFxMFdW --- .../cmd/mesh-knowledge/answers.go | 20 +++++++-- .../cmd/mesh-knowledge/close.go | 13 +++++- .../cmd/mesh-knowledge/review.go | 2 +- .../cmd/mesh-knowledge/review_round_test.go | 44 +++++++++++++++++++ 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go b/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go index 4a9941fb3..84737abf4 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go @@ -311,6 +311,8 @@ type Got struct { // Idea is said of a research effort kept as an idea: it is not what holds (ADR 0629 §2). Idea string `json:"idea,omitempty"` Waiting string `json:"waiting,omitempty"` + // Round is the round of the next review of the working version (ADR 0665). + Round int `json:"round,omitempty"` // Retracted are the retractions the operator approved; Retracting, one waiting for the operator. Retracted []RetractionRow `json:"retracted,omitempty"` Retracting string `json:"retracting,omitempty"` @@ -386,11 +388,21 @@ func (k *KB) Get(a GetAsk) (*Got, error) { e.PendingVersion, k, last.Version) } } - if r.Ask != nil { - g.Waiting = "the operator's ask: " + r.Ask.Said - if r.Ask.Said == AskWaiting && r.Ask.Held != "" { + if a.Version == 0 { + g.Round = e.Working().Round() + } + ask := r.Ask + if a.Version == 0 && e.Pending != nil && e.Pending.Ask != nil { + ask = e.Pending.Ask // the ask of the version being worked + } + if ask != nil { + g.Waiting = "the operator's ask: " + ask.Said + if ask.Round > 0 { + g.Waiting = fmt.Sprintf("the operator's ask after round %d asked changes: %s", ask.Round, ask.Said) + } + if ask.Said == AskWaiting && ask.Held != "" { // Held by the router, not shown to the operator yet: said so, with why (ADR 0397, issue 926). - g.Waiting = "the operator's ask is held by the router, not yet shown to the operator: " + r.Ask.Held + g.Waiting = "the operator's ask is held by the router, not yet shown to the operator: " + ask.Held } } if a.Version == 0 { diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/close.go b/modules/mesh-knowledge/cmd/mesh-knowledge/close.go index 5d2c95085..5970de867 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/close.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/close.go @@ -85,6 +85,10 @@ func (k *KB) Close(caller string, a CloseAsk) (*Closed, error) { if cur.Retracting != nil { return nil, fmt.Errorf("%s has a retraction waiting for the operator; nothing was closed", key) } + if w := e.Working(); w.Ask != nil && w.Ask.Said == AskWaiting && w.Ask.Round > 0 { + return nil, fmt.Errorf("%s is at round %d of its review, which asked changes: the operator decides it, and no review runs before the "+ + "answer. Nothing was closed; close it after the operator's answer", key, w.Ask.Round) + } want := closeLinkKind[a.To] var added []Link var refusals []Refusal @@ -111,7 +115,11 @@ func (k *KB) Close(caller string, a CloseAsk) (*Closed, error) { name = fmt.Sprintf("%s (%d)", closeSection[a.To], n) } r.Sections = append(r.Sections, Section{Name: name, Text: note}) - r.Reviews, r.Ask, r.Retracting = nil, nil, nil + r.Reviews, r.CountFrom, r.Ask, r.Retracting = nil, 0, nil, nil + if e.Pending != nil { + // The closing keeps the round count of the version it replaces (ADR 0665 §1). + r.Reviews, r.CountFrom = e.Pending.Reviews, e.Pending.CountFrom + } r.NeedsOperator, r.Brief = false, nil r.Author = caller r.Writers = append([]string{}, cur.Writers...) @@ -243,7 +251,8 @@ func ClosingQuestion(r *Record, from string, linked []*Record) string { b.WriteString("the note says why it is parked as an idea. A closing to an idea takes no links: never ask for one.") } b.WriteString(" Answer changes otherwise, with findings, each naming the note or the link and saying what to change. " + - "Do not ask for changes of taste or wording that leave the meaning as it is.\n\n") + "Do not ask for changes of taste or wording that leave the meaning as it is.\n") + b.WriteString(roundText(r) + "\n") fmt.Fprintf(&b, "The note, section %q:\n\n%s\n\n", c.Section, c.Note) if c.To == StatusGraduated { b.WriteString("The became links the effort carries, those it carried before and those the closing adds:\n") diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review.go index bd61bc25b..12aecb349 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/review.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review.go @@ -160,7 +160,7 @@ func ReviewQuestion(r *Record, linked []*Record) string { // graduated or abandoned, which it takes once accepted) are shown apart, each named for what it is (hq issue 492). func reviewed(r *Record) *Record { c := r.Clone() - c.Reviews, c.Ask, c.Judging, c.Drafting, c.Writers, c.Author, c.Parts = nil, nil, nil, nil, nil, "", nil + c.Reviews, c.Ask, c.Judging, c.Drafting, c.Writers, c.Author, c.Parts, c.CountFrom = nil, nil, nil, nil, nil, "", nil, 0 c.Extra = addExtra(c.Extra, "state in the knowledge base", c.Status+" (every record under review is; it is not the record's own status)") if c.Kind == KindResearch && c.Effort != "" { c.Extra = addExtra(c.Extra, "research status", c.Effort) diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go index b98db7a92..c747fdb51 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go @@ -142,3 +142,47 @@ func TestTheRoundCountGoesOnAcrossRevisionsOfAPendingInsight(t *testing.T) { t.Fatalf("the third revision of the insight: %.300s", q) } } + +// A pending version of an active effort: revisions and a closing keep the count (ADR 0665 §1), `get` says the round and +// the ask after round 5, and a close is refused while that ask waits (§3). +func TestARoundCountSurvivesAClosingAndAClosingWaitsForTheOperatorsAnswer(t *testing.T) { + w := imported(t, newMemStore()) + n := w.effort(t, StatusActive) + revise := func(i int, answer string) string { + next := effortAsk(n, StatusActive, Section{Name: "00-overview.md", Text: fmt.Sprintf("What a roadmap entry would be, revision %d.", i)}) + if _, err := w.k.Propose("author@g14", next); err != nil { + t.Fatal(err) + } + w.judge.review, w.judge.reviews = answer, nil + w.reviewAll(t) + return strings.Join(w.judge.reviews, "\n") + } + for i := 1; i <= 2; i++ { + revise(i, changes(i)) + } + if g, err := w.k.Get(GetAsk{Kind: KindResearch, Number: n}); err != nil || g.Round != 3 { + t.Fatalf("get after two rounds: %+v, %v", g, err) + } + if _, err := w.k.Close("lead@shanks", CloseAsk{Number: n, To: StatusIdea, Note: "Parked: nothing carries it this month."}); err != nil { + t.Fatal(err) + } + w.judge.review, w.judge.reviews = "", nil + w.reviewAll(t) + if q := strings.Join(w.judge.reviews, "\n"); !strings.Contains(q, "This is round 3 of its review") || !strings.Contains(q, "finding of review 2") { + t.Fatalf("the closing's question: %.500s", q) + } + + m := w.effort(t, StatusActive) + n = m + for i := 1; i <= 5; i++ { + revise(i, changes(i)) + } + g, err := w.k.Get(GetAsk{Kind: KindResearch, Number: n}) + if err != nil || !strings.Contains(g.Waiting, "after round 5 asked changes") { + t.Fatalf("get after round 5: %+v, %v", g.Waiting, err) + } + if _, err := w.k.Close("lead@shanks", CloseAsk{Number: n, To: StatusIdea, Note: "Parked: nothing carries it this month."}); err == nil || + !strings.Contains(err.Error(), "the operator decides") { + t.Fatalf("a close before the answer: %v", err) + } +} From c12f1a16db83ba0cb2c33dda286e386b31464d04 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 23:22:46 +0100 Subject: [PATCH 7/7] get shows a round only for a proposed version (issue 784, ADR 0665) Claude-Session: https://claude.ai/code/session_01JG73KK8HD1gufnqyFxMFdW --- modules/mesh-knowledge/cmd/mesh-knowledge/answers.go | 4 ++-- .../mesh-knowledge/cmd/mesh-knowledge/review_round_test.go | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go b/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go index 84737abf4..9b29b439f 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go @@ -388,8 +388,8 @@ func (k *KB) Get(a GetAsk) (*Got, error) { e.PendingVersion, k, last.Version) } } - if a.Version == 0 { - g.Round = e.Working().Round() + if w := e.Working(); a.Version == 0 && w.Status == StatusProposed { + g.Round = w.Round() } ask := r.Ask if a.Version == 0 && e.Pending != nil && e.Pending.Ask != nil { diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go index c747fdb51..a87a7c693 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/review_round_test.go @@ -163,6 +163,9 @@ func TestARoundCountSurvivesAClosingAndAClosingWaitsForTheOperatorsAnswer(t *tes if g, err := w.k.Get(GetAsk{Kind: KindResearch, Number: n}); err != nil || g.Round != 3 { t.Fatalf("get after two rounds: %+v, %v", g, err) } + if g, err := w.k.Get(GetAsk{Kind: KindDecision, Number: 276}); err != nil || g.Round != 0 { + t.Fatalf("get of an accepted record shows round %d, %v", g.Round, err) + } if _, err := w.k.Close("lead@shanks", CloseAsk{Number: n, To: StatusIdea, Note: "Parked: nothing carries it this month."}); err != nil { t.Fatal(err) }