Key a wait list by name, not session id, so it survives a /clear (issues 1629, 1744, ADR 0490)

Claude Code gives a session a new session_id at /clear, with nothing
naming the earlier one, so keying claude_code_wait's state by session id
lost a session's waits the moment it cleared. ADR 0490 changes ADR 0489
§1: waits are now kept under the caller's name and node (by, who@node);
every op also takes session, the caller's current id, and the module
keeps a binding of the name to the session that last called for it. An
unbound name is claimed by the first call for it; a session whose own
id no longer matches the binding is refused on every op, naming the
session that holds it, until it calls the new op adopt, which always
takes the name over at once (no bound protects a recent call) and tells
the earlier session so on its own next call.

The mirrored file a hook reads stays keyed by session id (a hook only
ever has that), holding whichever name's waits are currently bound to
it; adopting rewrites both the old id's file (emptied) and the new id's
(filled) in the same call. A second mirrored file, the node-wide index,
lists every name on the node with an open wait; the SessionStart hook
now also reads it, on every start, and prints the exact adopt call for
each name, so a session with a brand new id can find and reclaim its
own waits without calling back into the module.
This commit is contained in:
jochen
2026-10-21 18:32:50 +02:00
parent 02a8837348
commit f0beea153e
4 changed files with 602 additions and 196 deletions
+210 -38
View File
@@ -1,6 +1,6 @@
package main
// A session's wait list (novox/hq ADR 0489, issue 1629).
// A session's wait list (novox/hq ADR 0489, ADR 0490, issues 1629, 1744).
import (
"encoding/json"
@@ -53,11 +53,20 @@ func (s *memWaits) Update(key string, value any, revision uint64) error {
}
return s.put(key, value)
}
func (s *memWaits) Keys() ([]string, error) {
s.mu.Lock()
defer s.mu.Unlock()
out := make([]string, 0, len(s.m))
for k := range s.m {
out = append(out, k)
}
return out, nil
}
// addApproveWait adds the wait fixture the operator's Approve is kept under.
func addApproveWait(t *testing.T, p Paths, store WaitStore, session string, at time.Time) string {
// addApproveWait adds the wait fixture the operator's Approve is kept under, for name on session.
func addApproveWait(t *testing.T, p Paths, store WaitStore, name, session string, at time.Time) string {
t.Helper()
out, err := AddWait(p, WaitInput{Session: session, By: "fix-1629@" + p.Node, What: "the operator's Approve of pull request 42",
out, err := AddWait(p, WaitInput{Session: session, By: name + "@" + p.Node, What: "the operator's Approve of pull request 42",
Read: WaitRead{Address: "mesh-delivery.recheck", Arguments: map[string]any{"pr": 42}}, Family: "mesh-delivery.decided",
Bound: 10}, store, at)
if err != nil {
@@ -70,11 +79,11 @@ func addApproveWait(t *testing.T, p Paths, store WaitStore, session string, at t
func TestAddAnswersAnIDAndListShowsItsAgeAndPastBound(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
id := addApproveWait(t, p, store, "sess-1", t0)
id := addApproveWait(t, p, store, "fix-1629", "sess-1", t0)
if !strings.HasPrefix(id, WaitPrefix) {
t.Fatalf("id %q does not start with %q", id, WaitPrefix)
}
out, err := ListWaits(p, "sess-1", store, t0.Add(time.Minute))
out, err := ListWaits(p, "sess-1", "fix-1629@g14", store, t0.Add(time.Minute))
if err != nil {
t.Fatal(err)
}
@@ -82,7 +91,7 @@ func TestAddAnswersAnIDAndListShowsItsAgeAndPastBound(t *testing.T) {
if len(open) != 1 || open[0]["id"] != id || open[0]["past_bound"] != false || open[0]["age_seconds"] != 60 {
t.Fatalf("%+v", open)
}
out, err = ListWaits(p, "sess-1", store, t0.Add(11*time.Minute))
out, err = ListWaits(p, "sess-1", "fix-1629@g14", store, t0.Add(11*time.Minute))
if err != nil {
t.Fatal(err)
}
@@ -97,35 +106,36 @@ func TestAddAnswersAnIDAndListShowsItsAgeAndPastBound(t *testing.T) {
func TestWatchedDoneAndSaidChangeTheWait(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
id := addApproveWait(t, p, store, "sess-2", t0)
by := "fix-1629@g14"
id := addApproveWait(t, p, store, "fix-1629", "sess-2", t0)
if _, err := SaidWait(p, "sess-2", id, store, t0.Add(time.Minute)); err == nil || !strings.Contains(err.Error(), "not past its bound") {
if _, err := SaidWait(p, "sess-2", by, id, store, t0.Add(time.Minute)); err == nil || !strings.Contains(err.Error(), "not past its bound") {
t.Fatalf("said before the bound: %v", err)
}
if _, err := WatchedWait(p, "sess-2", id, "mesh_wait on mesh-delivery.decided", store, t0.Add(time.Minute)); err != nil {
if _, err := WatchedWait(p, "sess-2", by, id, "mesh_wait on mesh-delivery.decided", store, t0.Add(time.Minute)); err != nil {
t.Fatal(err)
}
out, _ := ListWaits(p, "sess-2", store, t0.Add(2*time.Minute))
out, _ := ListWaits(p, "sess-2", by, store, t0.Add(2*time.Minute))
open := out["open"].([]map[string]any)
if open[0]["watcher"] != "mesh_wait on mesh-delivery.decided" {
t.Fatalf("watcher not kept: %+v", open[0])
}
if _, err := SaidWait(p, "sess-2", id, store, t0.Add(11*time.Minute)); err != nil {
if _, err := SaidWait(p, "sess-2", by, id, store, t0.Add(11*time.Minute)); err != nil {
t.Fatalf("said past the bound: %v", err)
}
out, _ = ListWaits(p, "sess-2", store, t0.Add(12*time.Minute))
out, _ = ListWaits(p, "sess-2", by, store, t0.Add(12*time.Minute))
open = out["open"].([]map[string]any)
if open[0]["said"] != true {
t.Fatalf("said not kept: %+v", open[0])
}
if _, err := DoneWait(p, "sess-2", id, "approved", store, t0.Add(13*time.Minute)); err != nil {
if _, err := DoneWait(p, "sess-2", by, id, "approved", store, t0.Add(13*time.Minute)); err != nil {
t.Fatal(err)
}
out, _ = ListWaits(p, "sess-2", store, t0.Add(14*time.Minute))
out, _ = ListWaits(p, "sess-2", by, store, t0.Add(14*time.Minute))
if len(out["open"].([]map[string]any)) != 0 {
t.Fatalf("done did not remove it from the open list: %v", out)
}
if _, err := DoneWait(p, "sess-2", id, "approved", store, t0.Add(15*time.Minute)); err == nil {
if _, err := DoneWait(p, "sess-2", by, id, "approved", store, t0.Add(15*time.Minute)); err == nil {
t.Fatal("done on an already-done wait was not refused")
}
}
@@ -135,7 +145,8 @@ func TestWatchedDoneAndSaidChangeTheWait(t *testing.T) {
func TestEveryChangeRewritesTheWaitsFileWhole(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
id1 := addApproveWait(t, p, store, "sess-3", t0)
by := "fix-1629@g14"
id1 := addApproveWait(t, p, store, "fix-1629", "sess-3", t0)
f, err := readWaitsFile(waitsFilePath(p, "sess-3"))
if err != nil {
t.Fatal(err)
@@ -147,7 +158,7 @@ func TestEveryChangeRewritesTheWaitsFileWhole(t *testing.T) {
if _, err := os.Stat(waitsFilePath(p, "sess-3") + ".mesh-tmp"); !os.IsNotExist(err) {
t.Fatalf("a temp file was left beside it: %v", err)
}
id2, err := AddWait(p, WaitInput{Session: "sess-3", By: "fix-1629@" + p.Node, What: "a second wait",
id2, err := AddWait(p, WaitInput{Session: "sess-3", By: by, What: "a second wait",
Read: WaitRead{Address: "mesh-delivery.recheck"}, Bound: 5}, store, t0.Add(time.Minute))
if err != nil {
t.Fatal(err)
@@ -156,7 +167,7 @@ func TestEveryChangeRewritesTheWaitsFileWhole(t *testing.T) {
if err != nil || len(f.Waits) != 2 || !f.At.Equal(t0.Add(time.Minute).UTC()) {
t.Fatalf("%+v %v", f, err)
}
if _, err := DoneWait(p, "sess-3", id1, "approved", store, t0.Add(2*time.Minute)); err != nil {
if _, err := DoneWait(p, "sess-3", by, id1, "approved", store, t0.Add(2*time.Minute)); err != nil {
t.Fatal(err)
}
f, err = readWaitsFile(waitsFilePath(p, "sess-3"))
@@ -165,36 +176,37 @@ func TestEveryChangeRewritesTheWaitsFileWhole(t *testing.T) {
}
}
// 1629/AC4: the waits are read again after the module "restarts" (the state is kept) and for the same session
// after a /clear (the same session id lists them); a different session id lists none of them.
func TestWaitsSurviveARestartAndAClearButNotCrossSessions(t *testing.T) {
// 1629/AC4: the waits are read again after the module "restarts" (the state is kept) and for the same name
// after a /clear that keeps calling with the same session id; a different name lists none of them.
func TestWaitsSurviveARestartAndAreNeverSeenByAnotherName(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
addApproveWait(t, p, store, "sess-4", t0)
by := "fix-1629@g14"
addApproveWait(t, p, store, "fix-1629", "sess-4", t0)
// A restart: a fresh Waiting over the same store reads the same waits.
restarted := &Waiting{Store: store, Now: func() time.Time { return t0.Add(time.Minute) }}
_ = restarted
out, err := ListWaits(p, "sess-4", store, t0.Add(time.Minute))
out, err := ListWaits(p, "sess-4", by, store, t0.Add(time.Minute))
if err != nil || len(out["open"].([]map[string]any)) != 1 {
t.Fatalf("not read again after a restart: %v %v", out, err)
}
// A /clear: the same session id still lists it.
out, err = ListWaits(p, "sess-4", store, t0.Add(2*time.Minute))
// The same session id, called again: still lists it.
out, err = ListWaits(p, "sess-4", by, store, t0.Add(2*time.Minute))
if err != nil || len(out["open"].([]map[string]any)) != 1 {
t.Fatalf("not read again for the same session: %v %v", out, err)
t.Fatalf("not read again for the same name: %v %v", out, err)
}
// A different session id: none of them.
out, err = ListWaits(p, "sess-other", store, t0.Add(2*time.Minute))
// A different name, same node: none of them.
out, err = ListWaits(p, "sess-other", "fix-other@g14", store, t0.Add(2*time.Minute))
if err != nil || len(out["open"].([]map[string]any)) != 0 {
t.Fatalf("another session id saw them: %v %v", out, err)
t.Fatalf("another name saw them: %v %v", out, err)
}
}
// 1629/AC5: add refuses an empty what, a bound under 1 or over 1440 minutes, a read address that is not an
// address, and more than 50 open waits per session, naming the argument.
// address, and more than 50 open waits per name, naming the argument.
func TestAddRefusesOutsideItsBounds(t *testing.T) {
p, _ := node(t, "g14")
base := func() WaitInput {
@@ -220,7 +232,7 @@ func TestAddRefusesOutsideItsBounds(t *testing.T) {
}
}
// More than 50 open waits per session.
// More than 50 open waits per name.
store := newMemWaits()
for i := 0; i < MostWaits; i++ {
if _, err := AddWait(p, base(), store, t0); err != nil {
@@ -236,18 +248,19 @@ func TestAddRefusesOutsideItsBounds(t *testing.T) {
// same waitSessionID add already holds to), naming "session" — not silently treating it as just-another-key.
func TestListWatchedDoneAndSaidRefuseABadSessionID(t *testing.T) {
p, _ := node(t, "g14")
by := "fix-1629@g14"
for _, bad := range []string{"", "a/b", "../x"} {
store := newMemWaits()
if _, err := ListWaits(p, bad, store, t0); err == nil || !strings.Contains(err.Error(), "session") {
if _, err := ListWaits(p, bad, by, store, t0); err == nil || !strings.Contains(err.Error(), "session") {
t.Errorf("list %q: %v", bad, err)
}
if _, err := WatchedWait(p, bad, "w-1", "watcher", store, t0); err == nil || !strings.Contains(err.Error(), "session") {
if _, err := WatchedWait(p, bad, by, "w-1", "watcher", store, t0); err == nil || !strings.Contains(err.Error(), "session") {
t.Errorf("watched %q: %v", bad, err)
}
if _, err := DoneWait(p, bad, "w-1", "outcome", store, t0); err == nil || !strings.Contains(err.Error(), "session") {
if _, err := DoneWait(p, bad, by, "w-1", "outcome", store, t0); err == nil || !strings.Contains(err.Error(), "session") {
t.Errorf("done %q: %v", bad, err)
}
if _, err := SaidWait(p, bad, "w-1", store, t0); err == nil || !strings.Contains(err.Error(), "session") {
if _, err := SaidWait(p, bad, by, "w-1", store, t0); err == nil || !strings.Contains(err.Error(), "session") {
t.Errorf("said %q: %v", bad, err)
}
}
@@ -275,13 +288,172 @@ func TestTheWaitToolIsServedThroughTheModulesDispatch(t *testing.T) {
t.Fatalf("no id answered: %s", text)
}
text, isErr = s.call(t, "claude_code_wait", map[string]any{"op": "list", "session": "sess-6"})
text, isErr = s.call(t, "claude_code_wait", map[string]any{"op": "list", "session": "sess-6", "by": "fix-1629@g14"})
if isErr || !strings.Contains(text, id) {
t.Fatalf("list: %v %s", isErr, text)
}
text, isErr = s.call(t, "claude_code_wait", map[string]any{"op": "done", "session": "sess-6", "id": id, "outcome": "approved"})
text, isErr = s.call(t, "claude_code_wait", map[string]any{"op": "done", "session": "sess-6", "by": "fix-1629@g14",
"id": id, "outcome": "approved"})
if isErr {
t.Fatalf("done: %s", text)
}
}
// ---- ADR 0490: a name's waits survive a session's id changing, by adopting -----------------------------
// ADR 0490 §1: a name nothing has bound yet is claimed by the first session that calls for it (add), without
// a separate adopt first.
func TestAFirstAddBindsAnUnboundNameAtOnce(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
id := addApproveWait(t, p, store, "fix-1629", "sess-a", t0)
out, err := ListWaits(p, "sess-a", "fix-1629@g14", store, t0)
if err != nil || len(out["open"].([]map[string]any)) != 1 || out["open"].([]map[string]any)[0]["id"] != id {
t.Fatalf("%v %v", out, err)
}
}
// ADR 0490 §1/§3: once a name is bound to a session, a different, non-adopted session calling for the same
// name (any op) is refused, naming the session that holds it and when it last called.
func TestADifferentSessionIsRefusedWithoutAdopting(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
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)
}
}
// ADR 0490 §2/§3: adopt right after a simulated /clear binds the name to the new session id at once; the
// mirrored file of the old session id is emptied and the new one holds the waits, with no delay.
func TestAdoptRightAfterAClearBindsAtOnce(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
by := "fix-1629@g14"
id := addApproveWait(t, p, store, "fix-1629", "sess-old", t0)
out, err := AdoptName(p, by, "sess-new", store, t0.Add(time.Minute))
if err != nil {
t.Fatal(err)
}
if out["session"] != "sess-new" || out["open"] != 1 {
t.Fatalf("%v", out)
}
took, _ := out["took-over-from"].(map[string]any)
if took == nil || took["session"] != "sess-old" {
t.Fatalf("the answer did not name the earlier id: %v", out)
}
oldFile, err := readWaitsFile(waitsFilePath(p, "sess-old"))
if err != nil || len(oldFile.Waits) != 0 {
t.Fatalf("the old session's file still holds waits: %+v %v", oldFile, err)
}
newFile, err := readWaitsFile(waitsFilePath(p, "sess-new"))
if err != nil || len(newFile.Waits) != 1 || newFile.Waits[0].ID != id {
t.Fatalf("the new session's file does not hold the waits: %+v %v", newFile, err)
}
// The new session can act on the name at once: no 10-minute rule.
if _, err := ListWaits(p, "sess-new", by, store, t0.Add(time.Minute)); err != nil {
t.Fatalf("the new session could not act right after adopting: %v", err)
}
}
// ADR 0490 §3: adopt of a name bound to another id always takes it over, however recently that id called —
// no bound protects it.
func TestAdoptAlwaysTakesOverEvenJustCalled(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
by := "fix-1629@g14"
addApproveWait(t, p, store, "fix-1629", "sess-fresh", t0)
// sess-fresh calls again right before being adopted away from.
if _, err := ListWaits(p, "sess-fresh", by, store, t0.Add(10*time.Second)); err != nil {
t.Fatal(err)
}
out, err := AdoptName(p, by, "sess-taker", store, t0.Add(11*time.Second))
if err != nil {
t.Fatalf("adopt was refused even though it always takes over: %v", err)
}
if out["session"] != "sess-taker" {
t.Fatalf("%v", out)
}
}
// ADR 0490 §3: the earlier id's next claude_code_wait call (any op) answers that its name now answers to
// another session, naming it.
func TestTheEarlierIDIsToldOnItsNextCall(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
by := "fix-1629@g14"
addApproveWait(t, p, store, "fix-1629", "sess-old", t0)
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))
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)
}
}
// ADR 0490 §2: the node-wide index lists every name on this node with an open wait, for the SessionStart
// hook to read directly.
func TestTheNodeWideIndexListsEveryNameWithOpenWaits(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
addApproveWait(t, p, store, "fix-1629", "sess-x", t0)
addApproveWait(t, p, store, "fix-1744", "sess-y", t0)
if _, err := DoneWait(p, "sess-x", "fix-1629@g14", wantsOnlyWait(t, store, p, "fix-1629", "sess-x"), "done",
store, t0.Add(time.Minute)); err != nil {
t.Fatal(err)
}
idx, err := readNodeIndex(nodeIndexPath(p))
if err != nil {
t.Fatal(err)
}
if idx.Node != "g14" || len(idx.Names) != 1 || idx.Names[0].Name != "fix-1744" || idx.Names[0].Open != 1 ||
idx.Names[0].CurrentSession != "sess-y" {
t.Fatalf("%+v", idx)
}
}
// wantsOnlyWait is the id of the one wait a fixture added, read back through list.
func wantsOnlyWait(t *testing.T, store WaitStore, p Paths, name, session string) string {
t.Helper()
out, err := ListWaits(p, session, name+"@"+p.Node, store, t0)
if err != nil {
t.Fatal(err)
}
open := out["open"].([]map[string]any)
if len(open) != 1 {
t.Fatalf("%v", open)
}
return open[0]["id"].(string)
}
// claude_code_wait op=adopt, served through the module's own dispatch (1629/AC6, ADR 0490).
func TestAdoptIsServedThroughTheModulesDispatch(t *testing.T) {
p, _ := node(t, "g14")
store := newMemWaits()
waiting = &Waiting{Store: store, Now: func() time.Time { return t0 }}
t.Cleanup(func() { waiting = nil })
s := serve(t, p, &bus{kept: map[string]map[string]any{}}, memConfig{})
text, isErr := s.call(t, "claude_code_wait", map[string]any{"op": "add", "session": "sess-old", "by": "fix-1629@g14",
"what": "the operator's Approve", "read": map[string]any{"address": "mesh-delivery.recheck"}, "bound_minutes": 10})
if isErr {
t.Fatalf("add: %s", text)
}
text, isErr = s.call(t, "claude_code_wait", map[string]any{"op": "adopt", "session": "sess-new", "by": "fix-1629@g14"})
if isErr || !strings.Contains(text, "sess-old") {
t.Fatalf("adopt: %v %s", isErr, text)
}
}