Fix review round 2 of the wait list and its hooks (issues 1629, 1743, 1744, ADR 0490)
mesh/merge-gate head 0f159a166200 merged into trunk 7a7a4034852a — pass: builds claude-code → ace, g14, novox, shanks; no bus step; every machine composes …
mesh/repo-check head 0f159a166200 merged into trunk 7a7a4034852a — pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-…
mesh/delivery delivered
mesh/merge-gate head 0f159a166200 merged into trunk 7a7a4034852a — pass: builds claude-code → ace, g14, novox, shanks; no bus step; every machine composes …
mesh/repo-check head 0f159a166200 merged into trunk 7a7a4034852a — pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-…
mesh/delivery delivered
The Stop hook treated a missing waits file exactly like one that could not be read: a notice on every stop of every session with no waits — the normal state, not an exceptional one. DecideStop now treats os.ErrNotExist as the ordinary case of no open waits, silently; the notice is still said for a file that exists but cannot be read or does not parse. ADR 0490 §1 keeps waits under a name, but the file a hook reads is kept per session id: writing it from only the name an op just acted on meant a second name bound to the same session id had its waits silently drop out of enforcement the moment the first name was touched, and adopting one name away emptied the id's file outright even when another name was still bound to it. Every write now rewrites a session's file from the union of every name currently bound to it (sessionWaitsUnion), adopt's rewrite of the superseded id included. Also: each atomic write gets its own temp file name, so two writes to the same path can never interleave on one; adopt clears the refusal counter of both the superseded and the adopting session, so a fresh three-strikes count starts under the new binding; and list is read-only again — it no longer claims an unbound name or refuses a session that is not (yet) the one bound to it, and instead says who currently holds the name.
This commit is contained in:
@@ -322,14 +322,20 @@ func TestADifferentSessionIsRefusedWithoutAdopting(t *testing.T) {
|
||||
by := "fix-1629@g14"
|
||||
addApproveWait(t, p, store, "fix-1629", "sess-old", t0)
|
||||
|
||||
if _, err := ListWaits(p, "sess-new", by, store, t0.Add(time.Minute)); err == nil ||
|
||||
!strings.Contains(err.Error(), "sess-old") || !strings.Contains(err.Error(), "adopt") {
|
||||
t.Fatalf("a different session was not refused, or did not name the one it holds to: %v", err)
|
||||
}
|
||||
if _, err := AddWait(p, WaitInput{Session: "sess-new", By: by, What: "another", Read: WaitRead{Address: "mesh-delivery.recheck"},
|
||||
Bound: 10}, store, t0.Add(time.Minute)); err == nil || !strings.Contains(err.Error(), "sess-old") {
|
||||
t.Fatalf("add from a different session was not refused: %v", err)
|
||||
}
|
||||
|
||||
// list, read-only (review round 2 item c), never refuses: it just says who holds the name.
|
||||
out, err := ListWaits(p, "sess-new", by, store, t0.Add(time.Minute))
|
||||
if err != nil {
|
||||
t.Fatalf("list from a different session was refused: %v", err)
|
||||
}
|
||||
held, _ := out["held-by"].(map[string]any)
|
||||
if held == nil || held["session"] != "sess-old" {
|
||||
t.Fatalf("list did not say who holds the name: %v", out)
|
||||
}
|
||||
}
|
||||
|
||||
// ADR 0490 §2/§3: adopt right after a simulated /clear binds the name to the new session id at once; the
|
||||
@@ -397,7 +403,8 @@ func TestTheEarlierIDIsToldOnItsNextCall(t *testing.T) {
|
||||
if _, err := AdoptName(p, by, "sess-new", store, t0.Add(time.Minute)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, err := ListWaits(p, "sess-old", by, store, t0.Add(2*time.Minute))
|
||||
_, err := AddWait(p, WaitInput{Session: "sess-old", By: by, What: "another", Read: WaitRead{Address: "mesh-delivery.recheck"},
|
||||
Bound: 10}, store, t0.Add(2*time.Minute))
|
||||
if err == nil || !strings.Contains(err.Error(), "sess-new") || !strings.Contains(err.Error(), "adopt") {
|
||||
t.Fatalf("the earlier id's next call did not name the new session: %v", err)
|
||||
}
|
||||
@@ -457,3 +464,114 @@ func TestAdoptIsServedThroughTheModulesDispatch(t *testing.T) {
|
||||
t.Fatalf("adopt: %v %s", isErr, text)
|
||||
}
|
||||
}
|
||||
|
||||
// ---- Review round 2: a session id's mirrored file is the union of every name bound to it -----------------
|
||||
|
||||
// Review round 2, item 2 (ADR 0490 §1): one session id with two different names bound to it holds both
|
||||
// names' waits in its one mirrored file, not only the name an op last acted on.
|
||||
func TestASessionsFileHoldsEveryNameBoundToIt(t *testing.T) {
|
||||
p, _ := node(t, "g14")
|
||||
store := newMemWaits()
|
||||
idX := addApproveWait(t, p, store, "fix-x", "sess-shared", t0)
|
||||
idY := addApproveWait(t, p, store, "fix-y", "sess-shared", t0.Add(time.Minute))
|
||||
|
||||
f, err := readWaitsFile(waitsFilePath(p, "sess-shared"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ids := map[string]bool{}
|
||||
for _, w := range f.Waits {
|
||||
ids[w.ID] = true
|
||||
}
|
||||
if len(f.Waits) != 2 || !ids[idX] || !ids[idY] {
|
||||
t.Fatalf("the session's file does not hold both names' waits: %+v", f)
|
||||
}
|
||||
}
|
||||
|
||||
// Review round 2, item 2: adopting one of two names bound to the same session away keeps that session's
|
||||
// file holding the other name's waits — it is never emptied wholesale.
|
||||
func TestAdoptingOneNameAwayKeepsTheSessionsOtherNamesWaits(t *testing.T) {
|
||||
p, _ := node(t, "g14")
|
||||
store := newMemWaits()
|
||||
idX := addApproveWait(t, p, store, "fix-x", "sess-shared", t0)
|
||||
idY := addApproveWait(t, p, store, "fix-y", "sess-shared", t0.Add(time.Minute))
|
||||
|
||||
if _, err := AdoptName(p, "fix-y@g14", "sess-y-new", store, t0.Add(2*time.Minute)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
sharedFile, err := readWaitsFile(waitsFilePath(p, "sess-shared"))
|
||||
if err != nil || len(sharedFile.Waits) != 1 || sharedFile.Waits[0].ID != idX {
|
||||
t.Fatalf("sess-shared should still hold fix-x's wait: %+v %v", sharedFile, err)
|
||||
}
|
||||
newFile, err := readWaitsFile(waitsFilePath(p, "sess-y-new"))
|
||||
if err != nil || len(newFile.Waits) != 1 || newFile.Waits[0].ID != idY {
|
||||
t.Fatalf("sess-y-new should hold fix-y's wait: %+v %v", newFile, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Review round 2, item b: adopt clears the refusal counter of both the earlier id and the new one, so a
|
||||
// fresh three-strikes count starts under the new binding.
|
||||
func TestAdoptClearsTheRefusalCounterOfBothIDs(t *testing.T) {
|
||||
p, _ := node(t, "g14")
|
||||
store := newMemWaits()
|
||||
by := "fix-1629@g14"
|
||||
addApproveWait(t, p, store, "fix-1629", "sess-old", t0)
|
||||
waitsDir := WaitsDirPath(p)
|
||||
if err := writeRefusalCounters(waitsDir, "sess-old", map[string]int{"w-1": 2}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := writeRefusalCounters(waitsDir, "sess-new", map[string]int{"w-2": 3}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := AdoptName(p, by, "sess-new", store, t0.Add(time.Minute)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if c := readRefusalCounters(waitsDir, "sess-old"); len(c) != 0 {
|
||||
t.Errorf("sess-old's counter was not cleared: %v", c)
|
||||
}
|
||||
if c := readRefusalCounters(waitsDir, "sess-new"); len(c) != 0 {
|
||||
t.Errorf("sess-new's counter was not cleared: %v", c)
|
||||
}
|
||||
}
|
||||
|
||||
// Review round 2, item c: list is read-only — it never binds an unbound name, and never refuses a
|
||||
// mismatched session; it answers the name's waits and who currently holds it.
|
||||
func TestListNeverBindsAndAnswersWhoHoldsTheName(t *testing.T) {
|
||||
p, _ := node(t, "g14")
|
||||
store := newMemWaits()
|
||||
by := "fix-1629@g14"
|
||||
|
||||
// An unbound name: list answers no waits, and binds nothing (a later add from a different session still
|
||||
// succeeds, as the first caller for an unbound name).
|
||||
out, err := ListWaits(p, "sess-reader", by, store, t0)
|
||||
if err != nil || len(out["open"].([]map[string]any)) != 0 {
|
||||
t.Fatalf("an unbound name: %v %v", out, err)
|
||||
}
|
||||
if _, found, _ := readSessionBinding(store, waitsKey(p, "fix-1629")); found {
|
||||
t.Fatal("list bound an unbound name")
|
||||
}
|
||||
if _, err := AddWait(p, WaitInput{Session: "sess-first", By: by, What: "x", Read: WaitRead{Address: "mesh-delivery.recheck"},
|
||||
Bound: 10}, store, t0); err != nil {
|
||||
t.Fatalf("add after a read-only list: %v", err)
|
||||
}
|
||||
|
||||
// A name bound to another session: list still answers (never refused), and says who holds it.
|
||||
out, err = ListWaits(p, "sess-other", by, store, t0.Add(time.Minute))
|
||||
if err != nil {
|
||||
t.Fatalf("list for a name held by another session was refused: %v", err)
|
||||
}
|
||||
if len(out["open"].([]map[string]any)) != 1 {
|
||||
t.Fatalf("list did not answer the name's waits: %v", out)
|
||||
}
|
||||
held, _ := out["held-by"].(map[string]any)
|
||||
if held == nil || held["session"] != "sess-first" {
|
||||
t.Fatalf("list did not say who holds the name: %v", out)
|
||||
}
|
||||
}
|
||||
|
||||
// readSessionBinding is a thin test helper reading the raw NamedWaits at key, without claiming it.
|
||||
func readSessionBinding(store WaitStore, key string) (NamedWaits, bool, error) {
|
||||
nw, _, found, err := readNamedWaits(store, key)
|
||||
return nw, found, err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user