mesh-issues: an open whose write may have landed never says nothing was opened, and a module must itself be registered
mesh/merge-gate pass: builds new: modules/mesh-issues, sent nowhere; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery delivered

From the review: a taken key that cannot be read back, or a write never
confirmed, now tells the caller to get the number before opening again; an
owner entry naming a module is owned only by that module registered; link
targets get the secret guard; line separators are refused in one-line fields;
the acceptance test now proves the counter's own updates were contended.
This commit is contained in:
jochen
2026-10-10 12:56:16 +02:00
parent 2493e3e048
commit 6f2c76ee71
5 changed files with 100 additions and 23 deletions
@@ -43,9 +43,11 @@ func TestMain(m *testing.M) {
type bus struct {
// lost counts the compare-and-sets another writer won: proof the openers collided.
lost atomic.Int64
nc *nats.Conn
js jetstream.JetStream
kvs map[string]jetstream.KeyValue
// counterLost counts only the counter's updates another opener won: proof the counter itself was contended.
counterLost atomic.Int64
nc *nats.Conn
js jetstream.JetStream
kvs map[string]jetstream.KeyValue
}
func newBus(t *testing.T) *bus {
@@ -243,6 +245,9 @@ func (i *instance) answer(m rpc) {
errors.As(err, &api) && api.ErrorCode == jetstream.JSErrCodeStreamWrongLastSequence {
code = -32010
i.b.lost.Add(1)
if m.Method == "mesh/state.update" && p.State == CountersState {
i.b.counterLost.Add(1)
}
}
i.say(map[string]any{"id": m.ID, "error": map[string]any{"code": code, "message": err.Error()}})
return
@@ -336,9 +341,10 @@ func TestFiftyOpensAtOnceFromTwoInstancesGetFiftyDistinctConsecutiveNumbers(t *t
}
}
// The openers collided: writes were refused because another opener had moved the counter first.
t.Logf("%d writes lost to another opener and tried again", b.lost.Load())
if b.lost.Load() == 0 {
t.Fatal("no opener ever lost a write: the test did not make them collide")
t.Logf("%d writes lost to another opener and tried again, %d of them the counter's updates", b.lost.Load(), b.counterLost.Load())
if b.counterLost.Load() < opens {
t.Fatalf("only %d of the counter's updates were lost to another opener: the test did not make the openers "+
"contend for the counter", b.counterLost.Load())
}
ctx := context.Background()
e, err := b.kvs[CountersState].Get(ctx, IssueCounter)
+15 -4
View File
@@ -58,6 +58,9 @@ func plainText(field, s string, max int, lines bool) error {
if lines && (r == '\n' || r == '\t') {
continue
}
if r == '\u2028' || r == '\u2029' {
return fmt.Errorf("%s carries the line separator %U; break lines with a newline", field, r)
}
if unicode.IsControl(r) || unicode.Is(unicode.Cf, r) && r != '‍' {
if lines {
return fmt.Errorf("%s carries the unprintable character %U; only line breaks and tabs are kept", field, r)
@@ -138,15 +141,20 @@ func entries(field string, list []string, one func(string) (string, error)) ([]s
// `<repository> modules/<module>` (to-be 53 §3).
type Owners []string
// owns is the registered owner an entry names — the longest one it is or starts with, followed by a space
// and where in it — or "".
// owns is the registered owner an entry names — the longest one it is, or starts with followed by a space
// or a slash and where in it — or "". An entry that names a module of a repository (`<repository>
// modules/<module>`) is owned only by that module registered, never by the repository alone, so a module
// not on the list is not taken as a place in the repository.
func (o Owners) owns(entry string) string {
best := ""
for _, owner := range o {
if (entry == owner || strings.HasPrefix(entry, owner+" ")) && len(owner) > len(best) {
if (entry == owner || strings.HasPrefix(entry, owner+" ") || strings.HasPrefix(entry, owner+"/")) && len(owner) > len(best) {
best = owner
}
}
if repo, rest, ok := strings.Cut(entry, " "); ok && strings.HasPrefix(rest, "modules/") && best == repo {
return ""
}
return best
}
@@ -180,7 +188,7 @@ func IsCore(locatedIn []string) bool {
}
// replayID is a replay's id in mesh-lab's replays register, optionally followed by words.
var replayID = regexp.MustCompile(`^R[0-9]{1,6}( .*)?$`)
var replayID = regexp.MustCompile(`^R[0-9]{1,6}\b`)
// designPath is a design document in hq, as `amended-design` names one.
var designPath = regexp.MustCompile(`^03-DESIGN/(00-as-is|01-to-be)/[0-9]{2}-[a-z0-9]+(-[a-z0-9]+)*\.md$`)
@@ -240,5 +248,8 @@ func checkTarget(kind, target string) (string, error) {
if !ok {
return "", fmt.Errorf("a %s link's target is %s, not %q", kind, shape, t)
}
if what := secretShape(t); what != "" {
return "", fmt.Errorf("the target looks like it carries %s; a link names a thing, never a secret", what)
}
return t, nil
}
+18 -8
View File
@@ -40,7 +40,7 @@ const (
// caller is told, and nothing was written.
const (
// counterRounds × counterAttempts is how many times one open tries to advance the counter.
counterRounds = 12
counterRounds = 20
counterAttempts = 8
// issueAttempts is how many times a change to one issue is read and written again.
issueAttempts = 16
@@ -129,9 +129,6 @@ func (t *Tracker) Open(a OpenAsk) (*Opened, error) {
if record == "" {
is.Status = StatusOpen
}
if size, _ := json.Marshal(is); len(size) > MostIssueBytes-1024 {
return nil, fmt.Errorf("the issue is %d bytes; one issue is kept in at most %d", len(size), MostIssueBytes)
}
n, err := t.next()
if err != nil {
@@ -161,13 +158,22 @@ func (t *Tracker) create(is *Issue) error {
key := keyOf(is.Number)
var err error
for i := 0; i < createAttempts; i++ {
if i > 0 {
t.Sleep(jitter(time.Duration(i) * 50 * time.Millisecond))
}
_, err = t.Store.Create(IssuesState, key, is)
if err == nil {
return nil
}
if errors.Is(err, stdio.ErrChanged) {
// Ours, written by an attempt whose answer was lost?
if there, _, rerr := read(t.Store, is.Number); rerr == nil && there.Opening == is.Opening {
there, _, rerr := read(t.Store, is.Number)
if rerr != nil {
t.Logf("[mesh-issues] issue %d is kept and could not be read back: %v", is.Number, rerr)
return fmt.Errorf("issue %d may have been written by this open: the key is taken and could not be read "+
"back (%v). Call get %d before opening it again, so it is not opened twice", is.Number, rerr, is.Number)
}
if there.Opening == is.Opening {
return nil
}
t.Logf("[mesh-issues] the counter handed out %d and an issue %d is already kept: not overwritten", is.Number, is.Number)
@@ -176,9 +182,13 @@ func (t *Tracker) create(is *Issue) error {
"and tell the operator the counter is behind the issues kept", is.Number, is.Number, is.Number)
}
}
t.Logf("[mesh-issues] number %d was taken and the issue could not be written: %v", is.Number, err)
return fmt.Errorf("number %d was taken and the issue could not be written (%v); nothing was opened and %d "+
"stays unused. Open it again", is.Number, err, is.Number)
// A write that timed out may still have landed: say so, never "nothing was opened".
t.Logf("[mesh-issues] number %d was taken and the issue's write was not confirmed: %v", is.Number, err)
if there, _, rerr := read(t.Store, is.Number); rerr == nil && there.Opening == is.Opening {
return nil
}
return fmt.Errorf("number %d was taken and the issue's write was not confirmed (%v): it may have been written. "+
"Call get %d before opening it again; if there is no issue %d, it stays unused", is.Number, err, is.Number, is.Number)
}
// next takes the next issue number: the counter advanced only at the revision read, so two openers — in
@@ -3,11 +3,14 @@ package main
import (
"encoding/json"
"errors"
"fmt"
"reflect"
"strings"
"sync"
"testing"
"time"
stdio "git.novox.be/novox/mesh-sdk/go"
)
func TestTheFirstIssueGetsTheSettingsNumberAndEachNextOneMore(t *testing.T) {
@@ -113,7 +116,7 @@ func TestANumberTakenAndNotWrittenIsSaidAndNeverHandedOutAgain(t *testing.T) {
failing := &failingStore{memory: h.mem, failCreates: createAttempts}
h.Store = failing
_, err := h.Open(OpenAsk{Title: "third", Report: "r"})
if err == nil || !strings.Contains(err.Error(), "367 stays unused") {
if err == nil || !strings.Contains(err.Error(), "may have been written") || !strings.Contains(err.Error(), "get 367") {
t.Fatalf("a write that never landed: %v", err)
}
h.Store = h.mem
@@ -160,6 +163,47 @@ func TestAWriteWhoseAnswerWasLostIsKnownAsItsOwn(t *testing.T) {
}
}
// lostEvery writes the issue on its first create and then answers every create with a failure.
type lostEvery struct{ *memory }
func (l lostEvery) Create(state, key string, value any) (uint64, error) {
if state == IssuesState {
_, _ = l.memory.Create(state, key, value)
return 0, errors.New("no answer in 30 s")
}
return l.memory.Create(state, key, value)
}
// unreadable answers a taken key with a refusal and cannot read it back.
type unreadable struct{ *memory }
func (u unreadable) Create(state, key string, value any) (uint64, error) {
if state == IssuesState {
return 0, fmt.Errorf("%s: %w", key, stdio.ErrChanged)
}
return u.memory.Create(state, key, value)
}
func (u unreadable) Get(state, key string) (*Entry, error) {
if state == IssuesState {
return nil, errors.New("no answer in 30 s")
}
return u.memory.Get(state, key)
}
// An open whose write may have landed never says nothing was opened (the review of 2026-10-10).
func TestAnOpenWhoseWriteMayHaveLandedNeverSaysNothingWasOpened(t *testing.T) {
h := newHarness(t)
h.Store = lostEvery{h.mem}
if o, err := h.Open(OpenAsk{Title: "t", Report: "r"}); err != nil || o.Number != 365 {
t.Fatalf("a write that landed, every answer lost: %v %v", o, err)
}
h.Store = unreadable{h.mem}
_, err := h.Open(OpenAsk{Title: "t", Report: "r"})
if err == nil || strings.Contains(err.Error(), "nothing was opened") || !strings.Contains(err.Error(), "get 366") {
t.Fatalf("a taken key that cannot be read back: %v", err)
}
}
func TestOpenSaysAnEventWithWhatChangedAndWhoChangedIt(t *testing.T) {
h := newHarness(t)
o, err := h.Open(OpenAsk{Title: "a title", Report: "a report", LocatedIn: []string{"mesh-controller internal/broker"}})
@@ -407,6 +451,7 @@ func TestLinksAreTypedAddedOnceAndRemoved(t *testing.T) {
{LinkAsk{Kind: LinkDecision, Target: "275"}, "four-digit"},
{LinkAsk{Kind: LinkDesign, Target: "03-DESIGN/../../etc/passwd"}, "design document"},
{LinkAsk{Kind: LinkCondition, Target: "a key\nwith a line"}, "condition"},
{LinkAsk{Kind: LinkCondition, Target: "token=abcdefgh123"}, "secret"},
{LinkAsk{Kind: LinkRelatesTo, Target: keyOf(b), Remove: true}, ""},
{LinkAsk{Kind: LinkRelatesTo, Target: keyOf(b), Remove: true}, "no relates-to link"},
}
@@ -549,7 +594,9 @@ func TestAnOwnersEntryIsTheRegisteredOneOrStartsWithIt(t *testing.T) {
for entry, want := range map[string]string{
"mesh-catalog": "mesh-catalog",
"mesh-catalog modules/nats": "mesh-catalog modules/nats",
"mesh-catalog modules/nats/module.go": "mesh-catalog",
"mesh-catalog modules/nats/module.go": "mesh-catalog modules/nats",
"mesh-catalog modules/does-not-exist": "",
"mesh-catalog checks/words": "mesh-catalog",
"mesh-catalog modules/nats (the gate)": "mesh-catalog modules/nats",
"mesh-controller cmd/mesh-controller": "mesh-controller",
"mesh-controllers": "",