F6b: merge main (#647) into the transport retry: unknownRevision reads the transport words, the row says passing (issue 1748 review 1)

Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ
This commit is contained in:
2026-11-09 23:33:33 +01:00
19 changed files with 449 additions and 36 deletions
@@ -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 {
@@ -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) {
@@ -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
}
@@ -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.
@@ -334,7 +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.
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)
}
@@ -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)
}
@@ -96,7 +96,6 @@ func saidSuffix(said string) string {
return ": " + said
}
// takeVersion takes one asked version into dir's modfile (takeVersions).
// downloadFailure is a failed download said in go's own words; one whose words are a network's (a lookup, a timeout, a reset
// connection) is a sdkstep.Passing, so the build module gives the ask again after a delay instead of failing it (ADR 0664 §1).
func downloadFailure(failure, format string, args ...any) error {
@@ -107,6 +106,7 @@ func downloadFailure(failure, format string, args ...any) error {
return err
}
// takeVersion takes one asked version into dir's modfile (takeVersions).
func (m *Module) takeVersion(ctx context.Context, root, dir string, tv *TakenVersion, pins *sync.Mutex) error {
modfile, err := m.ensureModfile(dir)
if err != nil {
@@ -304,5 +304,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 sdkstep.PassingCause(failure) {
return false
}
return strings.Contains(failure, "unknown revision") || strings.Contains(failure, "invalid version")
}
@@ -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", passing: true, downloads: 1,
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", passing: true,
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", passing: true,
@@ -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 {
@@ -172,7 +172,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"`
@@ -180,7 +181,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"`
}
@@ -283,9 +284,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)
}
}
@@ -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)
}
@@ -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 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 {
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 {
@@ -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")
@@ -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
@@ -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")
@@ -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[:])
@@ -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)
@@ -159,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)
@@ -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", "")
@@ -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)
@@ -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()
}
@@ -0,0 +1,191 @@
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)
}
}
// 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 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)
}
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)
}
}