From 56c68be7fae3d866f43fd19382800aedeb55487f Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 17:59:10 +0200 Subject: [PATCH 1/6] Keep a session's wait list in the claude-code module (issue 1629) A session can end its turn while an Approve it expects is still open and nothing then watches it. claude_code_wait (add/watched/done/said/list) keeps each session's waits in the module's waits state, keyed by its session id and node, and mirrors them whole and atomically to a file under the agent's home that the plugin's coming hooks (1743, 1744) will read without calling back into this module. --- modules/claude-code/cmd/claude-code/main.go | 2 + modules/claude-code/cmd/claude-code/waits.go | 483 ++++++++++++++++++ .../claude-code/cmd/claude-code/waits_test.go | 266 ++++++++++ modules/claude-code/module.json | 4 +- 4 files changed, 754 insertions(+), 1 deletion(-) create mode 100644 modules/claude-code/cmd/claude-code/waits.go create mode 100644 modules/claude-code/cmd/claude-code/waits_test.go diff --git a/modules/claude-code/cmd/claude-code/main.go b/modules/claude-code/cmd/claude-code/main.go index 88250936..fe0b702e 100644 --- a/modules/claude-code/cmd/claude-code/main.go +++ b/modules/claude-code/cmd/claude-code/main.go @@ -388,6 +388,7 @@ func tools(p Paths, servers ServerState, view *ServerView, config ConfigState, c Description: "Read-only: what this node carries of the agent's memory between nodes (novox/hq ADR 0343) — the home kind it carries for (the operator's own homes, or an agent account's), each project's memory files with their fingerprint and the node and time of their last version, the files not carried and why, the conflicts still open (a file changed on two nodes, kept twice in one file, the newer first), and how many replaced or removed versions are kept. Fingerprints only, never the text; it changes nothing.", Run: func(map[string]any) (any, error) { return memoryCarrier.Status(), nil }}) out = append(out, proposalTools(p)...) + out = append(out, waitTools(p)...) judge := NewJudge(p) out = append(out, judgeTool(judge), judgedTool(judge)) return append(out, configTools(p, config, configView)...) @@ -463,6 +464,7 @@ func main() { proposing = &Proposing{Store: stateOf{stdio.State(ProposalsState)}, Bus: sdkAskBus{}, Record: recordWarrant, Now: time.Now, Write: writeManaged} go proposing.watch(p) + waiting = &Waiting{Store: keptWaits{stdio.State(WaitsState)}, Now: time.Now} memoryCarrier = NewMemoryCarrier(p, memoryStateOf{stdio.State(MemoryState)}, notifyOperator) go memoryCarrier.Run(func(handle func(key, op string, value json.RawMessage, revision uint64) error) error { return stdio.State(MemoryState).Watch("", func(c stdio.StateChange) error { diff --git a/modules/claude-code/cmd/claude-code/waits.go b/modules/claude-code/cmd/claude-code/waits.go new file mode 100644 index 00000000..3c7e7639 --- /dev/null +++ b/modules/claude-code/cmd/claude-code/waits.go @@ -0,0 +1,483 @@ +package main + +// A session's wait list (novox/hq ADR 0489): what a session still expects an answer to — an Approve, a +// delivery's end, anything it started and must act on when it ends (follow-what-you-start) — so the +// plugin's Stop hook can refuse to end its turn while one is open and nothing watches it. +// +// Kept in the module's `waits` state under the session's id and this node (so the same session reads its +// waits again after a /clear, and after the module restarts), and mirrored whole, atomically, to a file the +// hooks read without calling back into this module: /.claude/nox-mesh/waits/.json. That +// path is under the agent's home (the manifest's `agent-home` resource), which this process — launched as +// the operator's account (novox/hq ADR 0193) — cannot itself write when the node names an agent account of +// its own (novox/hq ADR 0266): every write below goes through `fsys` (homefs.go), which already carries such +// a write through the sudo helper the guard's binary also serves, exactly as grant.go's WriteCredentials +// does for the agent's own credentials file. Where the node has no agent account of its own, fsys is the +// plain filesystem and the write reaches the operator's own home directly. + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" +) + +// WaitsState is the module's state the wait lists are kept in. +const WaitsState = "waits" + +// WaitsDir is where the mirrored files live under the agent's home. +const WaitsDir = "nox-mesh/waits" + +// WaitPrefix starts every wait's id. +const WaitPrefix = "w-" + +// MostWaits is how many open waits one session may hold at once (novox/hq ADR 0489, 1629/AC5). +const MostWaits = 50 + +// The bounds of a wait's bound, in minutes (1629/AC5). +const ( + ShortestWaitBound = 1 + LongestWaitBound = 1440 +) + +// mostWaitAttempts bounds the compare-and-set retry loop over the `waits` state. +const mostWaitAttempts = 20 + +var waitSessionID = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$`) + +// WaitRead is the verb a session reads to see where one wait stands. +type WaitRead struct { + Address string `json:"address"` + Arguments map[string]any `json:"arguments,omitempty"` +} + +// Wait is one answer a session wrote down that it expects (novox/hq ADR 0489 §1-§2). +type Wait struct { + ID string `json:"id"` + By string `json:"by"` + What string `json:"what"` + Read WaitRead `json:"read"` + Family string `json:"family,omitempty"` + Bound int `json:"bound"` // minutes + At time.Time `json:"at"` + Watcher string `json:"watcher,omitempty"` + Said bool `json:"said,omitempty"` +} + +// PastBound says whether now is past this wait's bound. +func (w Wait) PastBound(now time.Time) bool { + return now.Sub(w.At) > time.Duration(w.Bound)*time.Minute +} + +// SessionWaits is what the `waits` state keeps for one session: its open waits. +type SessionWaits struct { + Session string `json:"session"` + Node string `json:"node"` + Waits []Wait `json:"waits"` +} + +// WaitsFile is what the mirrored file holds, whole, on every change (1629/AC3): the session's open waits and +// the time they were written. +type WaitsFile struct { + Session string `json:"session"` + At time.Time `json:"at"` + Waits []Wait `json:"waits"` +} + +// WaitStore is the `waits` state: a value with its revision, created only when absent, updated only at the +// revision read. +type WaitStore interface { + Get(key string) (json.RawMessage, uint64, bool, error) + Create(key string, value any) error + Update(key string, value any, revision uint64) error +} + +// keptWaits is the `waits` state on the bus. +type keptWaits struct{ s stdio.KeptState } + +func (k keptWaits) Get(key string) (json.RawMessage, uint64, bool, error) { + e, err := k.s.Get(key) + if err != nil || e == nil { + return nil, 0, false, err + } + return e.Value, e.Revision, true, nil +} +func (k keptWaits) Create(key string, value any) error { + _, err := k.s.Create(key, value) + return err +} +func (k keptWaits) Update(key string, value any, revision uint64) error { + _, err := k.s.Update(key, value, revision) + return err +} + +// Waiting is what claude_code_wait reaches: the `waits` state and the clock. Nil outside a launch. +type Waiting struct { + Store WaitStore + Now func() time.Time +} + +var waiting *Waiting + +// waitsKey is the state's key for one session's waits: its id and this node (novox/hq ADR 0489 §1), so two +// nodes never collide over the same session id. +func waitsKey(p Paths, session string) string { return p.Node + "/" + session } + +// waitsFilePath is where one session's waits are mirrored, under the agent's home. +func waitsFilePath(p Paths, session string) string { + return filepath.Join(p.Home, ".claude", filepath.FromSlash(WaitsDir), session+".json") +} + +func newWaitID() string { + var b [8]byte + _, _ = rand.Read(b[:]) + return WaitPrefix + hex.EncodeToString(b[:]) +} + +// addressShaped says whether s could be an address `mesh_call` takes: not blank, no whitespace, and a dot +// between two names (`.`, `/.`, `/.`). +func addressShaped(s string) bool { + s = strings.TrimSpace(s) + if s == "" || strings.ContainsAny(s, " \t\n\r") { + return false + } + dot := strings.LastIndex(s, ".") + return dot > 0 && dot < len(s)-1 +} + +func readSessionWaits(store WaitStore, key string) (SessionWaits, uint64, bool, error) { + raw, rev, found, err := store.Get(key) + if err != nil || !found { + return SessionWaits{}, 0, found, err + } + var sw SessionWaits + _ = json.Unmarshal(raw, &sw) + return sw, rev, true, nil +} + +// updateWaits reads one session's waits, lets mutate change them, and writes the result back at the +// revision read — created when there was none yet — retried when another write won the race. +func updateWaits(store WaitStore, key string, mutate func(SessionWaits) (SessionWaits, error)) (SessionWaits, error) { + for i := 0; i < mostWaitAttempts; i++ { + sw, rev, found, err := readSessionWaits(store, key) + if err != nil { + return SessionWaits{}, fmt.Errorf("the wait list could not be read: %w", err) + } + next, err := mutate(sw) + if err != nil { + return SessionWaits{}, err + } + if !found { + err = store.Create(key, next) + } else { + err = store.Update(key, next, rev) + } + if err == nil { + return next, nil + } + if errors.Is(err, stdio.ErrChanged) { + continue + } + return SessionWaits{}, fmt.Errorf("the wait list could not be written: %w", err) + } + return SessionWaits{}, errors.New("the wait list kept changing elsewhere; try again") +} + +// writeWaitsFile rewrites the session's mirrored file whole, atomically (temp file, then rename): it holds +// exactly the open waits and the time of this write (1629/AC3). +func writeWaitsFile(p Paths, session string, sw SessionWaits, now time.Time) error { + path := waitsFilePath(p, session) + if err := fsys.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return err + } + waits := sw.Waits + if waits == nil { + waits = []Wait{} + } + raw, err := indented(WaitsFile{Session: session, At: now.UTC(), Waits: waits}) + if err != nil { + return err + } + tmp := path + ".mesh-tmp" + if err := fsys.WriteFile(tmp, raw, 0o600); err != nil { + return err + } + return fsys.Rename(tmp, path) +} + +// sessionAndNode reads `by` as claude_code_ask does: who@node, where node is this one. +func sessionAndNode(p Paths, by string) (string, error) { + by = strings.TrimSpace(by) + if who, node, found := strings.Cut(by, "@"); found { + if node != p.Node { + return "", fmt.Errorf("by names the node %q, and this is %s: add the wait on the node the session runs on", node, p.Node) + } + return who, nil + } + if by == "" { + return "", errors.New("by is required: your session's name, as who@node or who") + } + return by, nil +} + +// WaitInput is what claude_code_wait's add takes. +type WaitInput struct { + Session string + By string + What string + Read WaitRead + Family string + Bound int +} + +// AddWait keeps one more wait for a session, refusing outside ADR 0489's bounds (1629/AC5), and rewrites the +// session's mirrored file (1629/AC3). +func AddWait(p Paths, in WaitInput, store WaitStore, now time.Time) (map[string]any, error) { + session := strings.TrimSpace(in.Session) + if !waitSessionID.MatchString(session) { + return nil, errors.New("session is required: the id the SessionStart hook named") + } + by, err := sessionAndNode(p, in.By) + if err != nil { + return nil, err + } + what := strings.TrimSpace(in.What) + if what == "" { + return nil, errors.New("what is required: say what you are waiting for") + } + if in.Bound < ShortestWaitBound || in.Bound > LongestWaitBound { + return nil, fmt.Errorf("bound_minutes is %d; it is %d to %d", in.Bound, ShortestWaitBound, LongestWaitBound) + } + address := strings.TrimSpace(in.Read.Address) + if !addressShaped(address) { + return nil, fmt.Errorf("read.address %q is not an address: the verb that reads where this wait stands, as mesh_call takes it", in.Read.Address) + } + w := Wait{ID: newWaitID(), By: by, What: what, Read: WaitRead{Address: address, Arguments: in.Read.Arguments}, + Family: strings.TrimSpace(in.Family), Bound: in.Bound, At: now.UTC()} + key := waitsKey(p, session) + sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { + if len(sw.Waits) >= MostWaits { + return sw, fmt.Errorf("%d waits already wait for session %s; at most %d open waits per session", len(sw.Waits), session, MostWaits) + } + sw.Session, sw.Node = session, p.Node + sw.Waits = append(append([]Wait(nil), sw.Waits...), w) + return sw, nil + }) + if err != nil { + return nil, err + } + if err := writeWaitsFile(p, session, sw, now); err != nil { + return nil, fmt.Errorf("the wait was kept, but its file could not be written: %w", err) + } + say("wait %s added for session %s: %s", w.ID, session, what) + return map[string]any{"id": w.ID, "session": session, "open": len(sw.Waits)}, nil +} + +// findWait is the index of id in sw.Waits, or -1. +func findWait(sw SessionWaits, id string) int { + for i, w := range sw.Waits { + if w.ID == id { + return i + } + } + return -1 +} + +// WatchedWait marks a wait as followed: by mesh_wait on its family, or a watcher started for it (1629/AC2). +func WatchedWait(p Paths, session, id, watcher string, store WaitStore, now time.Time) (map[string]any, error) { + session, id, watcher = strings.TrimSpace(session), strings.TrimSpace(id), strings.TrimSpace(watcher) + if id == "" { + return nil, errors.New("id is required: the one add answered") + } + if watcher == "" { + return nil, errors.New("watcher is required: what follows this wait (mesh_wait on its family, or a watcher agent)") + } + key := waitsKey(p, session) + sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { + i := findWait(sw, id) + if i < 0 { + return sw, fmt.Errorf("no open wait %s for session %s", id, session) + } + sw.Waits[i].Watcher = watcher + return sw, nil + }) + if err != nil { + return nil, err + } + if err := writeWaitsFile(p, session, sw, now); err != nil { + return nil, fmt.Errorf("the wait was watched, but its file could not be written: %w", err) + } + return map[string]any{"id": id, "watcher": watcher}, nil +} + +// DoneWait removes a wait from the open list: it ended, with outcome (1629/AC2). +func DoneWait(p Paths, session, id, outcome string, store WaitStore, now time.Time) (map[string]any, error) { + session, id, outcome = strings.TrimSpace(session), strings.TrimSpace(id), strings.TrimSpace(outcome) + if id == "" { + return nil, errors.New("id is required: the one add answered") + } + key := waitsKey(p, session) + sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { + i := findWait(sw, id) + if i < 0 { + return sw, fmt.Errorf("no open wait %s for session %s", id, session) + } + sw.Waits = append(append([]Wait(nil), sw.Waits[:i]...), sw.Waits[i+1:]...) + return sw, nil + }) + if err != nil { + return nil, err + } + if err := writeWaitsFile(p, session, sw, now); err != nil { + return nil, fmt.Errorf("the wait was done, but its file could not be written: %w", err) + } + say("wait %s for session %s done: %s", id, session, outcome) + return map[string]any{"id": id, "done": true, "outcome": outcome}, nil +} + +// SaidWait marks a wait past its bound as told to the operator: refused while it is still within its bound +// (1629/AC2). +func SaidWait(p Paths, session, id string, store WaitStore, now time.Time) (map[string]any, error) { + session, id = strings.TrimSpace(session), strings.TrimSpace(id) + if id == "" { + return nil, errors.New("id is required: the one add answered") + } + key := waitsKey(p, session) + sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { + i := findWait(sw, id) + if i < 0 { + return sw, fmt.Errorf("no open wait %s for session %s", id, session) + } + if !sw.Waits[i].PastBound(now) { + return sw, fmt.Errorf("wait %s is not past its bound yet: said is for telling the operator about one that is", id) + } + sw.Waits[i].Said = true + return sw, nil + }) + if err != nil { + return nil, err + } + if err := writeWaitsFile(p, session, sw, now); err != nil { + return nil, fmt.Errorf("the wait was said, but its file could not be written: %w", err) + } + return map[string]any{"id": id, "said": true}, nil +} + +// ListWaits answers a session's open waits, with their age and whether they are past their bound (1629/AC1). +func ListWaits(p Paths, session string, store WaitStore, now time.Time) (map[string]any, error) { + session = strings.TrimSpace(session) + if session == "" { + return nil, errors.New("session is required: the id the SessionStart hook named") + } + sw, _, _, err := readSessionWaits(store, waitsKey(p, session)) + if err != nil { + return nil, fmt.Errorf("the wait list could not be read: %w", err) + } + out := []map[string]any{} + for _, w := range sw.Waits { + entry := map[string]any{"id": w.ID, "what": w.What, "by": w.By, "address": w.Read.Address, + "bound_minutes": w.Bound, "at": w.At.Format(time.RFC3339), "age_seconds": int(now.Sub(w.At).Seconds()), + "past_bound": w.PastBound(now), "watcher": w.Watcher, "said": w.Said} + if len(w.Read.Arguments) > 0 { + entry["arguments"] = w.Read.Arguments + } + if w.Family != "" { + entry["family"] = w.Family + } + out = append(out, entry) + } + return map[string]any{"session": session, "open": out}, nil +} + +// readWaitsFile reads one session's mirrored file, as the hooks do: a plain local read, as the session's own +// account (never through fsys or the module's state), and never calling back into this module. +func readWaitsFile(path string) (WaitsFile, error) { + raw, err := os.ReadFile(path) + if err != nil { + return WaitsFile{}, err + } + var f WaitsFile + if err := json.Unmarshal(raw, &f); err != nil { + return WaitsFile{}, err + } + return f, nil +} + +// waitTools is claude_code_wait: add, watched, done, said and list, over one session's wait list (novox/hq +// ADR 0489). +func waitTools(p Paths) []stdio.Tool { + notLaunched := errors.New("this module was not launched with the bus; no wait can be kept") + return []stdio.Tool{ + {Name: "claude_code_wait", + Description: "Your session's wait list (novox/hq ADR 0489): what you still expect an answer to — an Approve, a " + + "delivery's end, anything you started and must act on when it ends. The plugin's Stop hook refuses to end " + + "your turn while a wait here is open and nothing follows it. op is add, watched, done, said or list; session " + + "is always required (the id the SessionStart hook named, or printed again after a /clear). add takes by " + + "(your session's name, as who@node or who), what (what you are waiting for), read (the verb that answers " + + "where it stands: {address, arguments}), family (optional, the event mesh_wait can wait on) and bound_minutes " + + "(1 to 1440) and answers an id; at most 50 waits are open per session. watched takes id and watcher (what " + + "follows it: mesh_wait on its family, or a watcher agent you started). done takes id and outcome, and removes " + + "it from the open list. said takes id and marks a wait past its bound as told to the operator — refused " + + "while it is still within its bound. list answers the open waits, with their age and whether each is past " + + "its bound.", + Input: map[string]any{ + "op": str("add, watched, done, said or list"), + "session": str("required: the session's id"), + "by": str("add: your session's name, as who@node or who"), + "what": str("add: what you are waiting for, in plain words"), + "read": map[string]any{"type": "object", "description": "add: the verb that reads where this wait stands", + "properties": map[string]any{ + "address": str("the address, as mesh_call takes it"), + "arguments": map[string]any{"type": "object", "description": "its arguments"}, + }}, + "family": str("add: the event family mesh_wait can wait on, when there is one"), + "bound_minutes": map[string]any{"type": "integer", "description": "add: minutes until this wait's bound, 1 to 1440"}, + "id": str("watched, done or said: the id add answered"), + "watcher": str("watched: what follows this wait"), + "outcome": str("done: what the wait ended with"), + }, + Run: func(a map[string]any) (any, error) { + if waiting == nil { + return nil, notLaunched + } + session := strArg(a, "session") + now := time.Now() + if waiting.Now != nil { + now = waiting.Now() + } + switch op := strArg(a, "op"); op { + case "add": + read := WaitRead{} + if m, ok := a["read"].(map[string]any); ok { + read.Address = strArg(m, "address") + if args, ok := m["arguments"].(map[string]any); ok { + read.Arguments = args + } + } + bound := 0 + if n, ok := a["bound_minutes"].(float64); ok { + bound = int(n) + } + return AddWait(p, WaitInput{Session: session, By: strArg(a, "by"), What: strArg(a, "what"), Read: read, + Family: strArg(a, "family"), Bound: bound}, waiting.Store, now) + case "watched": + return WatchedWait(p, session, strArg(a, "id"), strArg(a, "watcher"), waiting.Store, now) + case "done": + return DoneWait(p, session, strArg(a, "id"), strArg(a, "outcome"), waiting.Store, now) + case "said": + return SaidWait(p, session, strArg(a, "id"), waiting.Store, now) + case "list": + return ListWaits(p, session, waiting.Store, now) + default: + return nil, fmt.Errorf("op is add, watched, done, said or list, not %q", op) + } + }}, + } +} diff --git a/modules/claude-code/cmd/claude-code/waits_test.go b/modules/claude-code/cmd/claude-code/waits_test.go new file mode 100644 index 00000000..59f4f0a8 --- /dev/null +++ b/modules/claude-code/cmd/claude-code/waits_test.go @@ -0,0 +1,266 @@ +package main + +// A session's wait list (novox/hq ADR 0489, issue 1629). + +import ( + "encoding/json" + "os" + "strings" + "sync" + "testing" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" +) + +// memWaits is the `waits` state: values with revisions, create-only and compare-and-set, as the bus keeps it. +type memWaits struct { + mu sync.Mutex + m map[string]json.RawMessage + rev map[string]uint64 + n uint64 +} + +func newMemWaits() *memWaits { + return &memWaits{m: map[string]json.RawMessage{}, rev: map[string]uint64{}} +} + +func (s *memWaits) Get(key string) (json.RawMessage, uint64, bool, error) { + s.mu.Lock() + defer s.mu.Unlock() + raw, ok := s.m[key] + return raw, s.rev[key], ok, nil +} +func (s *memWaits) put(key string, value any) error { + raw, err := json.Marshal(value) + s.n++ + s.m[key], s.rev[key] = raw, s.n + return err +} +func (s *memWaits) Create(key string, value any) error { + s.mu.Lock() + defer s.mu.Unlock() + if _, ok := s.m[key]; ok { + return stdio.ErrChanged + } + return s.put(key, value) +} +func (s *memWaits) Update(key string, value any, revision uint64) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.rev[key] != revision { + return stdio.ErrChanged + } + return s.put(key, value) +} + +// 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 { + t.Helper() + out, err := AddWait(p, WaitInput{Session: session, By: "fix-1629@" + 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 { + t.Fatalf("add: %v", err) + } + return out["id"].(string) +} + +// 1629/AC1: add answers an id; list answers the open waits with their age and whether each is past its bound. +func TestAddAnswersAnIDAndListShowsItsAgeAndPastBound(t *testing.T) { + p, _ := node(t, "g14") + store := newMemWaits() + id := addApproveWait(t, p, store, "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)) + if err != nil { + t.Fatal(err) + } + open := out["open"].([]map[string]any) + 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)) + if err != nil { + t.Fatal(err) + } + open = out["open"].([]map[string]any) + if open[0]["past_bound"] != true { + t.Fatalf("past its 10-minute bound after 11 minutes: %+v", open[0]) + } +} + +// 1629/AC2: watched, done and said change the wait; said is refused for a wait not past its bound; done removes +// it from the open list. +func TestWatchedDoneAndSaidChangeTheWait(t *testing.T) { + p, _ := node(t, "g14") + store := newMemWaits() + id := addApproveWait(t, p, store, "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") { + 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 { + t.Fatal(err) + } + out, _ := ListWaits(p, "sess-2", 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 { + t.Fatalf("said past the bound: %v", err) + } + out, _ = ListWaits(p, "sess-2", 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 { + t.Fatal(err) + } + out, _ = ListWaits(p, "sess-2", 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 { + t.Fatal("done on an already-done wait was not refused") + } +} + +// 1629/AC3: every change rewrites the session's waits file whole, atomically; it holds exactly the open waits +// and the write time. +func TestEveryChangeRewritesTheWaitsFileWhole(t *testing.T) { + p, _ := node(t, "g14") + store := newMemWaits() + id1 := addApproveWait(t, p, store, "sess-3", t0) + f, err := readWaitsFile(waitsFilePath(p, "sess-3")) + if err != nil { + t.Fatal(err) + } + if f.Session != "sess-3" || len(f.Waits) != 1 || f.Waits[0].ID != id1 || !f.At.Equal(t0.UTC()) { + t.Fatalf("%+v", f) + } + // Written whole: no .mesh-tmp left beside it. + 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", + Read: WaitRead{Address: "mesh-delivery.recheck"}, Bound: 5}, store, t0.Add(time.Minute)) + if err != nil { + t.Fatal(err) + } + f, err = readWaitsFile(waitsFilePath(p, "sess-3")) + 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 { + t.Fatal(err) + } + f, err = readWaitsFile(waitsFilePath(p, "sess-3")) + if err != nil || len(f.Waits) != 1 || f.Waits[0].ID != id2["id"] { + t.Fatalf("the file does not hold exactly the open waits: %+v %v", f, err) + } +} + +// 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) { + p, _ := node(t, "g14") + store := newMemWaits() + addApproveWait(t, p, store, "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)) + 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)) + if err != nil || len(out["open"].([]map[string]any)) != 1 { + t.Fatalf("not read again for the same session: %v %v", out, err) + } + + // A different session id: none of them. + out, err = ListWaits(p, "sess-other", 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) + } +} + +// 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. +func TestAddRefusesOutsideItsBounds(t *testing.T) { + p, _ := node(t, "g14") + base := func() WaitInput { + return WaitInput{Session: "sess-5", By: "fix-1629@" + p.Node, What: "something", Read: WaitRead{Address: "mesh-delivery.recheck"}, Bound: 10} + } + for name, change := range map[string]struct { + mutate func(*WaitInput) + want string + }{ + "empty what": {func(in *WaitInput) { in.What = "" }, "what is required"}, + "bound zero": {func(in *WaitInput) { in.Bound = 0 }, "bound_minutes"}, + "bound under 1": {func(in *WaitInput) { in.Bound = 0 }, "bound_minutes"}, + "bound over 1440": {func(in *WaitInput) { in.Bound = 1441 }, "bound_minutes"}, + "empty address": {func(in *WaitInput) { in.Read.Address = "" }, "read.address"}, + "address no dot": {func(in *WaitInput) { in.Read.Address = "notanaddress" }, "read.address"}, + "address has spaces": {func(in *WaitInput) { in.Read.Address = "not an address" }, "read.address"}, + } { + in := base() + change.mutate(&in) + store := newMemWaits() + if _, err := AddWait(p, in, store, t0); err == nil || !strings.Contains(err.Error(), change.want) { + t.Errorf("%s: %v", name, err) + } + } + + // More than 50 open waits per session. + store := newMemWaits() + for i := 0; i < MostWaits; i++ { + if _, err := AddWait(p, base(), store, t0); err != nil { + t.Fatalf("wait %d: %v", i, err) + } + } + if _, err := AddWait(p, base(), store, t0); err == nil || !strings.Contains(err.Error(), "at most 50 open waits") { + t.Fatalf("the 51st wait was not refused: %v", err) + } +} + +// 1629/AC6: the tool is served through the module's own dispatch. +func TestTheWaitToolIsServedThroughTheModulesDispatch(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-6", "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) + } + var added map[string]any + if err := json.Unmarshal([]byte(text), &added); err != nil { + t.Fatal(err) + } + id, _ := added["id"].(string) + if id == "" { + t.Fatalf("no id answered: %s", text) + } + + text, isErr = s.call(t, "claude_code_wait", map[string]any{"op": "list", "session": "sess-6"}) + 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"}) + if isErr { + t.Fatalf("done: %s", text) + } +} diff --git a/modules/claude-code/module.json b/modules/claude-code/module.json index b869d012..3b62d219 100644 --- a/modules/claude-code/module.json +++ b/modules/claude-code/module.json @@ -28,7 +28,8 @@ "config", "proposals", "questions", - "memory" + "memory", + "waits" ], "reads": [ "claude-licence-manager.bindings" @@ -67,6 +68,7 @@ "claude_code_proposals", "claude_code_ask", "claude_code_answer", + "claude_code_wait", "claude_code_settings_get", "claude_code_settings_set", "claude_code_settings_clear", -- 2.54.0 From 37628f4bceb5f11d04f68425f515c55a78bb9288 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 18:07:36 +0200 Subject: [PATCH 2/6] Add the plugin's SessionStart hook so a session learns its own id (issue 1744) A session is never told its session id by any tool, only by a hook's stdin, so it cannot name itself to claude_code_wait. The plugin's SessionStart hook (claude-code session-start ) prints the id and how to give it, plus the session's still-open waits after a /clear, reading the waits file claude_code_wait mirrors directly rather than calling back into this module. Carries the Stop hook's command and the GuardFiles.WaitsDir plumbing MarketplaceWith needs for both (issue 1743 wires the Stop hook's dispatch next). --- modules/claude-code/cmd/claude-code/config.go | 31 ++++++- modules/claude-code/cmd/claude-code/main.go | 4 + modules/claude-code/cmd/claude-code/mesh.go | 2 +- .../cmd/claude-code/session_start.go | 68 +++++++++++++++ .../cmd/claude-code/session_start_test.go | 85 +++++++++++++++++++ modules/claude-code/cmd/claude-code/waits.go | 14 ++- 6 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 modules/claude-code/cmd/claude-code/session_start.go create mode 100644 modules/claude-code/cmd/claude-code/session_start_test.go diff --git a/modules/claude-code/cmd/claude-code/config.go b/modules/claude-code/cmd/claude-code/config.go index 5c5e7721..a2993df5 100644 --- a/modules/claude-code/cmd/claude-code/config.go +++ b/modules/claude-code/cmd/claude-code/config.go @@ -476,10 +476,13 @@ type PluginFile struct { // what it is given. Outside hooks/, so no registered hook's name can be it. const GuardDir = "guard" -// GuardFiles are the guard as the plugin carries it: the binary to copy and the data it judges with. +// GuardFiles are the guard as the plugin carries it: the binary to copy, what it judges with, and — the same +// binary again — the waits directory the SessionStart and Stop hooks are given (novox/hq ADR 0489, issues +// 1744, 1743). Empty WaitsDir places no such hook. type GuardFiles struct { - Exe string - Data GuardData + Exe string + Data GuardData + WaitsDir string } // GuardMatcher is what the guard is asked about: every tool. Its own rules judge the shell and the agent's file @@ -495,6 +498,20 @@ func GuardCommand() string { return `"` + filepath.Join(dir, "guard") + `" guard "` + filepath.Join(dir, "guard.json") + `"` } +// SessionStartCommand is the plugin's SessionStart hook: the guard's own binary, told the session's open +// waits live under waitsDir (novox/hq ADR 0489 §3, issue 1744). +func SessionStartCommand(waitsDir string) string { + dir := filepath.Join(ManagedDir, MarketplaceDir, "plugins", Plugin, GuardDir) + return `"` + filepath.Join(dir, "guard") + `" session-start "` + waitsDir + `"` +} + +// StopCommand is the plugin's Stop hook: the guard's own binary, refusing to end the turn while a wait under +// waitsDir is open and unwatched, or past its bound and untold (novox/hq ADR 0489 §3-§6, issue 1743). +func StopCommand(waitsDir string) string { + dir := filepath.Join(ManagedDir, MarketplaceDir, "plugins", Plugin, GuardDir) + return `"` + filepath.Join(dir, "guard") + `" stop "` + waitsDir + `"` +} + // Marketplace is the marketplace directory's whole content, by path inside it. A node item of a name laid // over a mesh item of the same kind and name: the node's wins. func Marketplace(c Config) map[string]PluginFile { return MarketplaceWith(c, nil) } @@ -533,6 +550,14 @@ func MarketplaceWith(c Config, guard *GuardFiles) map[string]PluginFile { out[root+GuardDir+"/guard.json"] = PluginFile{Content: jsonFile(guard.Data)} hooks["PreToolUse"] = append(hooks["PreToolUse"], map[string]any{"matcher": GuardMatcher, "hooks": []any{map[string]any{"type": "command", "command": GuardCommand(), "timeout": 15}}}) + if guard.WaitsDir != "" { + // A session's wait list (novox/hq ADR 0489): the SessionStart hook names the session's id and its open + // waits (issue 1744); the Stop hook keeps the turn open while one is unwatched or untold (issue 1743). + hooks["SessionStart"] = append(hooks["SessionStart"], map[string]any{"matcher": "startup|resume|clear|compact", + "hooks": []any{map[string]any{"type": "command", "command": SessionStartCommand(guard.WaitsDir), "timeout": 10}}}) + hooks["Stop"] = append(hooks["Stop"], map[string]any{ + "hooks": []any{map[string]any{"type": "command", "command": StopCommand(guard.WaitsDir), "timeout": 5}}}) + } } for _, k := range keys { it := chosen[k] diff --git a/modules/claude-code/cmd/claude-code/main.go b/modules/claude-code/cmd/claude-code/main.go index fe0b702e..04e81ff8 100644 --- a/modules/claude-code/cmd/claude-code/main.go +++ b/modules/claude-code/cmd/claude-code/main.go @@ -417,6 +417,10 @@ func main() { if len(os.Args) == 3 && os.Args[1] == "guard" { os.Exit(runGuard(os.Args[2], os.Stdin, os.Stderr)) } + // The plugin's SessionStart hook (novox/hq ADR 0489 §1, issue 1744): names the session's id and its open waits. + if len(os.Args) == 3 && os.Args[1] == "session-start" { + os.Exit(runSessionStart(os.Args[2], os.Stdin, os.Stdout)) + } // The helper that touches the agent's home as the agent's account (novox/hq ADR 0266), started by this // module through sudo: one answer per request on stdin, until it ends. if len(os.Args) == 2 && os.Args[1] == HelperArg { diff --git a/modules/claude-code/cmd/claude-code/mesh.go b/modules/claude-code/cmd/claude-code/mesh.go index 71b01b93..91229d23 100644 --- a/modules/claude-code/cmd/claude-code/mesh.go +++ b/modules/claude-code/cmd/claude-code/mesh.go @@ -278,7 +278,7 @@ func guardFiles(p Paths, m *MeshTools) *GuardFiles { if resolved, err := filepath.EvalSymlinks(exe); err == nil { exe = resolved } - return &GuardFiles{Exe: exe, Data: GuardDataOf(p, m)} + return &GuardFiles{Exe: exe, Data: GuardDataOf(p, m), WaitsDir: WaitsDirPath(p)} } // MeshToolsEvery is how often the controller is asked again. diff --git a/modules/claude-code/cmd/claude-code/session_start.go b/modules/claude-code/cmd/claude-code/session_start.go new file mode 100644 index 00000000..2fbee58b --- /dev/null +++ b/modules/claude-code/cmd/claude-code/session_start.go @@ -0,0 +1,68 @@ +package main + +// The plugin's SessionStart hook (novox/hq ADR 0489 §1, issue 1744): `claude-code session-start `, +// run by the agent's own account on every start, resume, /clear and /compact. The tool runner never tells a +// session its own id (only a hook's stdin carries session_id), so a session cannot name itself to +// claude_code_wait until this hook has told it. It also lists the session's still-open waits, so a session +// that /clear-ed sees what it was watching before. +// +// It reads the session's mirrored waits file directly — the same local read the Stop hook makes +// (readWaitsFile, waits.go) — never calling back into this module: this hook runs on every start, and must +// not wait on the bus to do it. A missing or malformed file is not an error here: there is simply nothing +// open to report. + +import ( + "encoding/json" + "fmt" + "io" + "strings" + "time" +) + +// SessionStartInput is what the hook is given on stdin. +type SessionStartInput struct { + SessionID string `json:"session_id"` +} + +// SessionStartMessage is the line the hook prints: the session's id and how to give it to claude_code_wait, +// then each still-open wait from before (1744/AC1, AC2). Pure of IO beyond the one file read. +func SessionStartMessage(sessionID, waitsDir string, now time.Time) string { + var b strings.Builder + fmt.Fprintf(&b, "This session's id is %s; give it as `session` to claude_code_wait.\n", sessionID) + f, err := readWaitsFile(waitsFileIn(waitsDir, sessionID)) + + if err != nil || len(f.Waits) == 0 { + return b.String() + } + b.WriteString("Waits still open from before this session's last start, resume, /clear or /compact:\n") + for _, w := range f.Waits { + age := now.Sub(w.At).Round(time.Second) + state := "no watcher yet" + if w.Watcher != "" { + state = "watched" + } + if w.PastBound(now) { + state += ", past its bound" + if w.Said { + state += ", said" + } + } + fmt.Fprintf(&b, " %s — %s (%s ago, %s)\n", w.ID, w.What, age, state) + } + return b.String() +} + +// runSessionStart is the hook: the session's id and its open waits out, exit 0 always (1744/AC3). +func runSessionStart(waitsDir string, stdin io.Reader, stdout io.Writer) (code int) { + defer func() { + if recover() != nil { + code = 0 // never fails the session's start + } + }() + var in SessionStartInput + if err := json.NewDecoder(stdin).Decode(&in); err != nil || strings.TrimSpace(in.SessionID) == "" { + return 0 + } + fmt.Fprint(stdout, SessionStartMessage(in.SessionID, waitsDir, time.Now())) + return 0 +} diff --git a/modules/claude-code/cmd/claude-code/session_start_test.go b/modules/claude-code/cmd/claude-code/session_start_test.go new file mode 100644 index 00000000..a6289166 --- /dev/null +++ b/modules/claude-code/cmd/claude-code/session_start_test.go @@ -0,0 +1,85 @@ +package main + +// The plugin's SessionStart hook (novox/hq ADR 0489 §1, issue 1744). + +import ( + "bytes" + "path/filepath" + "strings" + "testing" + "time" +) + +// 1744/AC1: the hook prints the session's id and how to give it to claude_code_wait. +func TestSessionStartPrintsTheSessionsIDAndHowToGiveIt(t *testing.T) { + waitsDir := t.TempDir() + stdin := strings.NewReader(`{"session_id":"sess-9","hook_event_name":"SessionStart","source":"startup"}`) + var out bytes.Buffer + if code := runSessionStart(waitsDir, stdin, &out); code != 0 { + t.Fatalf("code %d, stdout %s", code, out.String()) + } + if !strings.Contains(out.String(), "sess-9") || !strings.Contains(out.String(), "claude_code_wait") { + t.Fatalf("stdout does not say how to name the session: %s", out.String()) + } +} + +// 1744/AC2: when the waits file holds open waits, it prints each (what, age, past bound or not). +func TestSessionStartListsOpenWaitsAfterAClear(t *testing.T) { + waitsDir := t.TempDir() + at := time.Date(2026, 10, 21, 10, 0, 0, 0, time.UTC) + f := WaitsFile{Session: "sess-10", At: at, Waits: []Wait{ + {ID: "w-open", What: "the operator's Approve", Bound: 10, At: at}, + {ID: "w-past", What: "a delivery to finish", Bound: 1, At: at}, + }} + writeWaitsFixture(t, filepath.Join(waitsDir, "sess-10.json"), f) + + now := at.Add(5 * time.Minute) + msg := SessionStartMessage("sess-10", waitsDir, now) + if !strings.Contains(msg, "w-open") || !strings.Contains(msg, "the operator's Approve") || !strings.Contains(msg, "no watcher") { + t.Fatalf("the open, unwatched wait is not described:\n%s", msg) + } + if !strings.Contains(msg, "w-past") || !strings.Contains(msg, "past its bound") { + t.Fatalf("the past-bound wait is not described:\n%s", msg) + } + if !strings.Contains(msg, "5m0s ago") { + t.Fatalf("the age is not said:\n%s", msg) + } +} + +// 1744/AC3: a missing or malformed file prints only the id line, and the hook never fails the session start. +func TestSessionStartNeverFailsOnAMissingOrBadFile(t *testing.T) { + waitsDir := t.TempDir() + if msg := SessionStartMessage("sess-11", waitsDir, time.Now()); !strings.Contains(msg, "sess-11") || strings.Contains(msg, "open") { + t.Fatalf("a missing file printed more than the id line: %q", msg) + } + writeFile(t, filepath.Join(waitsDir, "sess-12.json"), "{not json") + if msg := SessionStartMessage("sess-12", waitsDir, time.Now()); !strings.Contains(msg, "sess-12") || strings.Contains(msg, "open") { + t.Fatalf("a malformed file printed more than the id line: %q", msg) + } + + var out bytes.Buffer + if code := runSessionStart(waitsDir, strings.NewReader("not json at all"), &out); code != 0 { + t.Fatalf("unreadable stdin failed the session start: code %d", code) + } + var out2 bytes.Buffer + if code := runSessionStart(waitsDir, panicReader{}, &out2); code != 0 { + t.Fatalf("a panic was not recovered: code %d", code) + } +} + +// panicReader panics on Read, as a stuck pipe or a bug reading stdin might. +type panicReader struct{} + +func (panicReader) Read([]byte) (int, error) { panic("boom") } + +// 1744/AC4 is checked together with 1743/AC7 in terminal_only_test.go's +// TestThePluginsOnlyHooksAreTheGuardTheSessionStartHookAndTheStopHook. + +func writeWaitsFixture(t *testing.T, path string, f WaitsFile) { + t.Helper() + raw, err := indented(f) + if err != nil { + t.Fatal(err) + } + writeFile(t, path, string(raw)) +} diff --git a/modules/claude-code/cmd/claude-code/waits.go b/modules/claude-code/cmd/claude-code/waits.go index 3c7e7639..23db64da 100644 --- a/modules/claude-code/cmd/claude-code/waits.go +++ b/modules/claude-code/cmd/claude-code/waits.go @@ -130,9 +130,15 @@ var waiting *Waiting // nodes never collide over the same session id. func waitsKey(p Paths, session string) string { return p.Node + "/" + session } -// waitsFilePath is where one session's waits are mirrored, under the agent's home. +// WaitsDirPath is where this node's sessions' waits are mirrored, under the agent's home: the directory the +// plugin's SessionStart and Stop hooks (1744, 1743) are given on their command line. +func WaitsDirPath(p Paths) string { + return filepath.Join(p.Home, ".claude", filepath.FromSlash(WaitsDir)) +} + +// waitsFilePath is where one session's waits are mirrored. func waitsFilePath(p Paths, session string) string { - return filepath.Join(p.Home, ".claude", filepath.FromSlash(WaitsDir), session+".json") + return filepath.Join(WaitsDirPath(p), session+".json") } func newWaitID() string { @@ -396,6 +402,10 @@ func ListWaits(p Paths, session string, store WaitStore, now time.Time) (map[str return map[string]any{"session": session, "open": out}, nil } +// waitsFileIn is where one session's mirrored waits are, given a hook's own waits-dir argument: the hooks +// have no launch environment, only their command line, so they never recompute this from Paths. +func waitsFileIn(waitsDir, session string) string { return filepath.Join(waitsDir, session+".json") } + // readWaitsFile reads one session's mirrored file, as the hooks do: a plain local read, as the session's own // account (never through fsys or the module's state), and never calling back into this module. func readWaitsFile(path string) (WaitsFile, error) { -- 2.54.0 From 0fa427dad302885bff5afc86d6cdd8c95026a6a7 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 18:08:13 +0200 Subject: [PATCH 3/6] Add the plugin's Stop hook so a written-down wait is never left unwatched (issue 1743) A session could end its turn while a wait it wrote down with claude_code_wait had nothing following it. The plugin's Stop hook (claude-code stop ) refuses the stop while a wait is within its bound and unwatched, or past its bound and not said, reading the waits file directly with no network and a 5s bound; it fails open on a missing or malformed file or a panic, and lets a wait through after three refusals in a row rather than holding the session forever. Renames TestThePluginsOnlyHookIsTheGuard to cover the session-start and Stop hooks too. --- modules/claude-code/cmd/claude-code/main.go | 5 + modules/claude-code/cmd/claude-code/stop.go | 137 +++++++++++++++ .../claude-code/cmd/claude-code/stop_test.go | 160 ++++++++++++++++++ .../cmd/claude-code/terminal_only_test.go | 19 ++- 4 files changed, 315 insertions(+), 6 deletions(-) create mode 100644 modules/claude-code/cmd/claude-code/stop.go create mode 100644 modules/claude-code/cmd/claude-code/stop_test.go diff --git a/modules/claude-code/cmd/claude-code/main.go b/modules/claude-code/cmd/claude-code/main.go index 04e81ff8..00b0943c 100644 --- a/modules/claude-code/cmd/claude-code/main.go +++ b/modules/claude-code/cmd/claude-code/main.go @@ -421,6 +421,11 @@ func main() { if len(os.Args) == 3 && os.Args[1] == "session-start" { os.Exit(runSessionStart(os.Args[2], os.Stdin, os.Stdout)) } + // The plugin's Stop hook (novox/hq ADR 0489 §3-§6, issue 1743): keeps the turn open while a wait is unwatched + // or untold. + if len(os.Args) == 3 && os.Args[1] == "stop" { + os.Exit(runStop(os.Args[2], os.Stdin, os.Stderr)) + } // The helper that touches the agent's home as the agent's account (novox/hq ADR 0266), started by this // module through sudo: one answer per request on stdin, until it ends. if len(os.Args) == 2 && os.Args[1] == HelperArg { diff --git a/modules/claude-code/cmd/claude-code/stop.go b/modules/claude-code/cmd/claude-code/stop.go new file mode 100644 index 00000000..2457c132 --- /dev/null +++ b/modules/claude-code/cmd/claude-code/stop.go @@ -0,0 +1,137 @@ +package main + +// The plugin's Stop hook (novox/hq ADR 0489 §3-§6, issue 1743): `claude-code stop `, run by the +// agent's own account before a session's turn is let end. It reads the session's mirrored waits file +// directly (readWaitsFile, waits.go) — never the bus, never calling back into this module — so the decision +// is local, under 5 s, and needs no network. +// +// Within its bound, a wait needs a watcher: refused while one is open and nothing follows it yet. Past its +// bound, a wait needs a word to the operator, watched or not: refused until it is marked said. Refused at +// most three times running for the same wait (a counter file beside the waits file, reset once the blocking +// wait changes or clears); the fourth time it is let through, saying the wait was left unwatched. A missing, +// unreadable or malformed waits file, or a panic, always allows. + +import ( + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "time" +) + +// StopInput is what the hook is given on stdin. +type StopInput struct { + SessionID string `json:"session_id"` + TranscriptPath string `json:"transcript_path"` + StopHookActive bool `json:"stop_hook_active"` +} + +// MostStopRefusals is how many times in a row the Stop hook refuses for the same wait before it lets the +// turn end anyway (1743/AC5). +const MostStopRefusals = 3 + +// StopDecision is the Stop hook's verdict, and the refusal counters it leaves behind for the next call. +type StopDecision struct { + Refuse bool + Reason string + Counters map[string]int +} + +// waitBlocksStop says whether w, on its own, keeps the turn from ending, and why (ADR 0489 §3-§4). +func waitBlocksStop(session string, w Wait, now time.Time) (bool, string) { + if w.Said { + return false, "" + } + if !w.PastBound(now) { + if w.Watcher != "" { + return false, "" + } + return true, fmt.Sprintf("wait %s (%s) is open and nothing follows it yet. Read where it stands with %s, then "+ + "either mesh_wait on %s or start a watcher that rereads it, and mark it watched: claude_code_wait "+ + "{\"op\":\"watched\",\"session\":%q,\"id\":%q,\"watcher\":\"...\"}.", w.ID, w.What, w.Read.Address, familyOrAddress(w), + session, w.ID) + } + return true, fmt.Sprintf("wait %s (%s) is past its bound and the operator has not been told. Tell the operator what "+ + "waits, since when, and what you are doing about it, then mark it: claude_code_wait "+ + "{\"op\":\"said\",\"session\":%q,\"id\":%q}.", w.ID, w.What, session, w.ID) +} + +func familyOrAddress(w Wait) string { + if w.Family != "" { + return w.Family + } + return w.Read.Address +} + +// DecideStop is the Stop hook's decision, pure of IO: f and readErr are what reading the session's waits +// file gave, previous the refusal counters read beside it, and now the time. Only the first wait that still +// blocks the stop is named; the others are read again next time. +func DecideStop(session string, f WaitsFile, readErr error, previous map[string]int, now time.Time) StopDecision { + if readErr != nil { + return StopDecision{Reason: fmt.Sprintf("the wait list could not be read (%v); allowed", readErr), Counters: map[string]int{}} + } + for _, w := range f.Waits { + blocks, reason := waitBlocksStop(session, w, now) + if !blocks { + continue + } + count := previous[w.ID] + 1 + if count > MostStopRefusals { + return StopDecision{Reason: fmt.Sprintf("wait %s was refused %d times running and is let through: it was left "+ + "unwatched (%s)", w.ID, MostStopRefusals, reason), Counters: map[string]int{}} + } + return StopDecision{Refuse: true, Reason: reason, Counters: map[string]int{w.ID: count}} + } + return StopDecision{Counters: map[string]int{}} +} + +func refusalCountersPath(waitsDir, session string) string { + return filepath.Join(waitsDir, session+".refusals.json") +} + +func readRefusalCounters(waitsDir, session string) map[string]int { + raw, err := os.ReadFile(refusalCountersPath(waitsDir, session)) + if err != nil { + return map[string]int{} + } + var m map[string]int + if json.Unmarshal(raw, &m) != nil { + return map[string]int{} + } + return m +} + +// writeRefusalCounters is best-effort: a counter file that cannot be written only means the three-strikes +// cap may be judged again from zero, never that the hook fails. +func writeRefusalCounters(waitsDir, session string, m map[string]int) { + raw, err := json.Marshal(m) + if err != nil { + return + } + _ = os.WriteFile(refusalCountersPath(waitsDir, session), raw, 0o600) +} + +// runStop is the hook: exit 2 with the reason on stderr refuses the stop, exit 0 allows (1743). +func runStop(waitsDir string, stdin io.Reader, stderr io.Writer) (code int) { + defer func() { + if recover() != nil { + code = 0 // a panic always allows (1743/AC6) + } + }() + var in StopInput + if err := json.NewDecoder(stdin).Decode(&in); err != nil || strings.TrimSpace(in.SessionID) == "" { + return 0 + } + f, err := readWaitsFile(waitsFileIn(waitsDir, in.SessionID)) + d := DecideStop(in.SessionID, f, err, readRefusalCounters(waitsDir, in.SessionID), time.Now()) + writeRefusalCounters(waitsDir, in.SessionID, d.Counters) + if d.Reason != "" { + fmt.Fprintln(stderr, d.Reason) + } + if d.Refuse { + return 2 + } + return 0 +} diff --git a/modules/claude-code/cmd/claude-code/stop_test.go b/modules/claude-code/cmd/claude-code/stop_test.go new file mode 100644 index 00000000..9bb4a121 --- /dev/null +++ b/modules/claude-code/cmd/claude-code/stop_test.go @@ -0,0 +1,160 @@ +package main + +// The plugin's Stop hook (novox/hq ADR 0489 §3-§6, issue 1743). + +import ( + "bytes" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +var stopT0 = time.Date(2026, 10, 21, 12, 0, 0, 0, time.UTC) + +func openWait(id string, bound int) Wait { + return Wait{ID: id, What: "the operator's Approve", Bound: bound, At: stopT0} +} + +// 1743/AC1: a wait within its bound and unwatched refuses the stop, naming the wait, its read address and +// what the session does about it. +func TestStopRefusesAnUnwatchedWaitWithinItsBound(t *testing.T) { + w := openWait("w-1", 10) + w.Read = WaitRead{Address: "mesh-delivery.recheck"} + d := DecideStop("sess-1", WaitsFile{Waits: []Wait{w}}, nil, map[string]int{}, stopT0.Add(time.Minute)) + if !d.Refuse || !strings.Contains(d.Reason, "w-1") || !strings.Contains(d.Reason, "mesh-delivery.recheck") || + !strings.Contains(d.Reason, "mesh_wait") || !strings.Contains(d.Reason, "watcher") { + t.Fatalf("%+v", d) + } +} + +// 1743/AC2: a watched wait within its bound, a done wait (absent from the file), or no file entry: allowed. +func TestStopAllowsAWatchedOrDoneOrAbsentWait(t *testing.T) { + watched := openWait("w-2", 10) + watched.Watcher = "mesh_wait on mesh-delivery.decided" + if d := DecideStop("sess-2", WaitsFile{Waits: []Wait{watched}}, nil, map[string]int{}, stopT0.Add(time.Minute)); d.Refuse { + t.Fatalf("a watched wait within its bound was refused: %+v", d) + } + if d := DecideStop("sess-2", WaitsFile{Waits: []Wait{}}, nil, map[string]int{}, stopT0); d.Refuse { + t.Fatalf("no open waits was refused: %+v", d) + } +} + +// 1743/AC3: a wait past its bound, watched or not, is refused until said, then allowed. +func TestStopRefusesPastBoundUntilSaidWatchedOrNot(t *testing.T) { + unwatched := openWait("w-3", 5) + if d := DecideStop("sess-3", WaitsFile{Waits: []Wait{unwatched}}, nil, map[string]int{}, stopT0.Add(10*time.Minute)); !d.Refuse || + !strings.Contains(d.Reason, "w-3") || !strings.Contains(d.Reason, "said") { + t.Fatalf("unwatched past bound: %+v", d) + } + watched := unwatched + watched.Watcher = "mesh_wait on mesh-delivery.decided" + if d := DecideStop("sess-3", WaitsFile{Waits: []Wait{watched}}, nil, map[string]int{}, stopT0.Add(10*time.Minute)); !d.Refuse { + t.Fatalf("watched past bound was allowed: %+v", d) + } + said := watched + said.Said = true + if d := DecideStop("sess-3", WaitsFile{Waits: []Wait{said}}, nil, map[string]int{}, stopT0.Add(10*time.Minute)); d.Refuse { + t.Fatalf("said past bound was still refused: %+v", d) + } +} + +// 1743/AC4: a missing, unreadable or malformed waits file is allowed, saying so on stderr; the decision does +// not touch the network and answers at once. +func TestStopAllowsWhenTheWaitsFileCannotBeRead(t *testing.T) { + d := DecideStop("sess-4", WaitsFile{}, errors.New("no such file"), map[string]int{}, stopT0) + if d.Refuse || !strings.Contains(d.Reason, "could not be read") { + t.Fatalf("%+v", d) + } + waitsDir := t.TempDir() // no file at all for sess-4b + var stderr bytes.Buffer + stdin := strings.NewReader(`{"session_id":"sess-4b","stop_hook_active":false}`) + start := time.Now() + code := runStop(waitsDir, stdin, &stderr) + if code != 0 || !strings.Contains(stderr.String(), "could not be read") { + t.Fatalf("code %d, stderr %s", code, stderr.String()) + } + if took := time.Since(start); took > 5*time.Second { + t.Fatalf("took %s; the Stop hook has no network and is bounded under 5s", took) + } +} + +// 1743/AC5: the same stop refused three times running for the same wait is allowed the fourth time, saying +// it was left unwatched; the counter resets when the wait changes. +func TestStopAllowsTheFourthRefusalInARow(t *testing.T) { + w := openWait("w-5", 10) + counters := map[string]int{} + for i := 1; i <= MostStopRefusals; i++ { + d := DecideStop("sess-5", WaitsFile{Waits: []Wait{w}}, nil, counters, stopT0.Add(time.Minute)) + if !d.Refuse { + t.Fatalf("refusal %d was allowed: %+v", i, d) + } + counters = d.Counters + } + d := DecideStop("sess-5", WaitsFile{Waits: []Wait{w}}, nil, counters, stopT0.Add(time.Minute)) + if d.Refuse || !strings.Contains(d.Reason, "unwatched") { + t.Fatalf("the fourth refusal was not let through: %+v", d) + } + if len(d.Counters) != 0 { + t.Fatalf("the counter was not reset once let through: %+v", d.Counters) + } + + // A different wait starts its own count from zero. + w2 := openWait("w-5b", 10) + d2 := DecideStop("sess-5", WaitsFile{Waits: []Wait{w2}}, nil, counters, stopT0.Add(time.Minute)) + if !d2.Refuse || d2.Counters["w-5b"] != 1 { + t.Fatalf("a different wait did not start its own count: %+v", d2) + } +} + +// 1743/AC6: a panic answers allow. +func TestStopAPanicAllows(t *testing.T) { + waitsDir := t.TempDir() + var stderr bytes.Buffer + if code := runStop(waitsDir, panicReader{}, &stderr); code != 0 { + t.Fatalf("a panic was not recovered: code %d", code) + } +} + +// runStop through its counter file: three refusals in a row for the one open wait, then let through. +func TestRunStopKeepsItsCounterBesideTheWaitsFile(t *testing.T) { + waitsDir := t.TempDir() + f := WaitsFile{Session: "sess-6", At: stopT0, Waits: []Wait{openWait("w-6", 10)}} + writeWaitsFixture(t, filepath.Join(waitsDir, "sess-6.json"), f) + stdin := func() *strings.Reader { return strings.NewReader(`{"session_id":"sess-6","stop_hook_active":false}`) } + + for i := 1; i <= MostStopRefusals; i++ { + var stderr bytes.Buffer + if code := runStop(waitsDir, stdin(), &stderr); code != 2 { + t.Fatalf("refusal %d: code %d, stderr %s", i, code, stderr.String()) + } + } + var stderr bytes.Buffer + if code := runStop(waitsDir, stdin(), &stderr); code != 0 || !strings.Contains(stderr.String(), "unwatched") { + t.Fatalf("the fourth call: code %d, stderr %s", code, stderr.String()) + } + raw, err := os.ReadFile(refusalCountersPath(waitsDir, "sess-6")) + if err != nil { + t.Fatal(err) + } + var counters map[string]int + if json.Unmarshal(raw, &counters) != nil || len(counters) != 0 { + t.Fatalf("the counter file was not reset: %s", raw) + } +} + +// 1743/AC7's rendering half (hooks.json holds the guard's PreToolUse entry and the Stop entry) is checked +// together with 1744/AC4 in terminal_only_test.go's +// TestThePluginsOnlyHooksAreTheGuardTheSessionStartHookAndTheStopHook. claude_code_hook_register still +// refuses a Stop hook (or any other) at every scope. +func TestHookRegisterStillRefusesEveryHook(t *testing.T) { + p, _ := node(t, "laptop") + s := serve(t, p, &bus{kept: map[string]map[string]any{}}, memConfig{}) + text, isError := s.call(t, "claude_code_hook_register", map[string]any{"name": "x", "event": "Stop", "command": "true"}) + if !isError || !strings.Contains(text, "issue 340") { + t.Fatalf("a Stop hook was not refused: %v %s", isError, text) + } +} diff --git a/modules/claude-code/cmd/claude-code/terminal_only_test.go b/modules/claude-code/cmd/claude-code/terminal_only_test.go index b376ab73..5c41587b 100644 --- a/modules/claude-code/cmd/claude-code/terminal_only_test.go +++ b/modules/claude-code/cmd/claude-code/terminal_only_test.go @@ -261,12 +261,12 @@ func TestTheOperatorsManagedSettingsAreWhatSessionsObey(t *testing.T) { } } -// The plugin's hooks are the mesh's own guard on the shell (novox/hq ADR 0245) and nothing else, whatever the -// configuration given holds (novox/hq issue 340). -func TestThePluginsOnlyHookIsTheGuard(t *testing.T) { +// The plugin's only hooks are the guard, the session-start hook and the Stop hook (novox/hq ADR 0245, ADR +// 0489) — never a hook a registration named, whatever the configuration given holds (novox/hq issue 340). +func TestThePluginsOnlyHooksAreTheGuardTheSessionStartHookAndTheStopHook(t *testing.T) { c := Config{Mesh: []Item{{Kind: KindHook, Name: "planted", Scope: ScopeMesh, Event: "PreToolUse", Matcher: "Bash", Command: "${HOOK_DIR}/x.sh", Files: map[string]string{"x.sh": "#!/bin/sh\n"}}}} - plugin := MarketplaceWith(c, &GuardFiles{Exe: "/bin/true"}) + plugin := MarketplaceWith(c, &GuardFiles{Exe: "/bin/true", WaitsDir: "/home/agent/.claude/nox-mesh/waits"}) root := "plugins/" + Plugin + "/" var hooks struct { Hooks map[string][]struct { @@ -279,9 +279,16 @@ func TestThePluginsOnlyHookIsTheGuard(t *testing.T) { if err := json.Unmarshal([]byte(plugin[root+"hooks/hooks.json"].Content), &hooks); err != nil { t.Fatal(err) } - if len(hooks.Hooks) != 1 || len(hooks.Hooks["PreToolUse"]) != 1 || hooks.Hooks["PreToolUse"][0].Matcher != GuardMatcher || + if len(hooks.Hooks) != 3 || len(hooks.Hooks["PreToolUse"]) != 1 || hooks.Hooks["PreToolUse"][0].Matcher != GuardMatcher || hooks.Hooks["PreToolUse"][0].Hooks[0].Command != GuardCommand() { - t.Fatalf("the plugin's hooks: %s", plugin[root+"hooks/hooks.json"].Content) + t.Fatalf("the guard's hook: %s", plugin[root+"hooks/hooks.json"].Content) + } + if len(hooks.Hooks["SessionStart"]) != 1 || hooks.Hooks["SessionStart"][0].Matcher != "startup|resume|clear|compact" || + hooks.Hooks["SessionStart"][0].Hooks[0].Command != SessionStartCommand("/home/agent/.claude/nox-mesh/waits") { + t.Fatalf("the session-start hook: %s", plugin[root+"hooks/hooks.json"].Content) + } + if len(hooks.Hooks["Stop"]) != 1 || hooks.Hooks["Stop"][0].Hooks[0].Command != StopCommand("/home/agent/.claude/nox-mesh/waits") { + t.Fatalf("the Stop hook: %s", plugin[root+"hooks/hooks.json"].Content) } for path := range plugin { if strings.HasPrefix(path, root+"hooks/planted") { -- 2.54.0 From 02a8837348198195fd122b36cdf1e9bfc8004617 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 18:23:42 +0200 Subject: [PATCH 4/6] Fix review round 1 of the wait list and its hooks (issues 1629, 1743, 1744) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session id reaching the Stop and SessionStart hooks was only checked for blank, so "../../x" or "a/b" could read or write outside the waits directory; both hooks now hold every session id to waitSessionID before building any path, and allow (saying why) rather than fail on a bad one. claude_code_wait's list/watched/done/said held only add to that same check, letting a malformed id through as an ordinary, always-empty key; they all refuse it now. The Stop hook's three-strikes cap could be bypassed forever if its counter file could not be written on a refusal: it now allows instead, saying so, so a wait can never hold a turn open indefinitely. Its advice offered mesh_wait on a wait's event family unconditionally, ignoring that the session's tool runner may not be granted it (ADR 0367 §2); it now offers mesh_wait only "if this session's tool runner is granted it, otherwise start a watcher", and drops the family entirely when a wait has none. Both hooks' fail-open and let-through notices were said only on stderr, which Claude Code does not show the operator on an allowed (exit 0) outcome; they now also print the same words as a "systemMessage" JSON object on stdout, which Claude Code does show. --- .../cmd/claude-code/hook_notice.go | 23 ++++ modules/claude-code/cmd/claude-code/main.go | 2 +- .../cmd/claude-code/session_start.go | 15 ++- .../cmd/claude-code/session_start_test.go | 41 +++++++ modules/claude-code/cmd/claude-code/stop.go | 63 +++++++--- .../claude-code/cmd/claude-code/stop_test.go | 110 ++++++++++++++++-- modules/claude-code/cmd/claude-code/waits.go | 41 +++++-- .../claude-code/cmd/claude-code/waits_test.go | 21 ++++ 8 files changed, 280 insertions(+), 36 deletions(-) create mode 100644 modules/claude-code/cmd/claude-code/hook_notice.go diff --git a/modules/claude-code/cmd/claude-code/hook_notice.go b/modules/claude-code/cmd/claude-code/hook_notice.go new file mode 100644 index 00000000..ec7a7ad2 --- /dev/null +++ b/modules/claude-code/cmd/claude-code/hook_notice.go @@ -0,0 +1,23 @@ +package main + +// Shared by the plugin's hooks (novox/hq ADR 0489 §5): a notice on exit 0 — the stop hook failing open or +// letting a wait through, the session-start hook unable to use a session id — reaches the operator's screen, +// not only the log. Claude Code shows a hook's top-level "systemMessage" field to the person; stderr alone, +// on an allowed (exit 0) outcome, is never shown there. + +import ( + "encoding/json" + "fmt" + "io" +) + +// writeSystemMessage prints message as the one JSON object Claude Code shows the operator, falling back to +// the plain words on the one error that cannot happen (a string always marshals). +func writeSystemMessage(w io.Writer, message string) { + raw, err := json.Marshal(map[string]string{"systemMessage": message}) + if err != nil { + fmt.Fprintln(w, message) + return + } + fmt.Fprintln(w, string(raw)) +} diff --git a/modules/claude-code/cmd/claude-code/main.go b/modules/claude-code/cmd/claude-code/main.go index 00b0943c..d879bbae 100644 --- a/modules/claude-code/cmd/claude-code/main.go +++ b/modules/claude-code/cmd/claude-code/main.go @@ -424,7 +424,7 @@ func main() { // The plugin's Stop hook (novox/hq ADR 0489 §3-§6, issue 1743): keeps the turn open while a wait is unwatched // or untold. if len(os.Args) == 3 && os.Args[1] == "stop" { - os.Exit(runStop(os.Args[2], os.Stdin, os.Stderr)) + os.Exit(runStop(os.Args[2], os.Stdin, os.Stdout, os.Stderr)) } // The helper that touches the agent's home as the agent's account (novox/hq ADR 0266), started by this // module through sudo: one answer per request on stdin, until it ends. diff --git a/modules/claude-code/cmd/claude-code/session_start.go b/modules/claude-code/cmd/claude-code/session_start.go index 2fbee58b..84326dd5 100644 --- a/modules/claude-code/cmd/claude-code/session_start.go +++ b/modules/claude-code/cmd/claude-code/session_start.go @@ -60,9 +60,20 @@ func runSessionStart(waitsDir string, stdin io.Reader, stdout io.Writer) (code i } }() var in SessionStartInput - if err := json.NewDecoder(stdin).Decode(&in); err != nil || strings.TrimSpace(in.SessionID) == "" { + if err := json.NewDecoder(stdin).Decode(&in); err != nil { return 0 } - fmt.Fprint(stdout, SessionStartMessage(in.SessionID, waitsDir, time.Now())) + sessionID := strings.TrimSpace(in.SessionID) + if sessionID == "" { + return 0 + } + // Review round 1, item 1: a session id is never used to build a path until it is checked against + // waitSessionID — no "/" in it reaches the filesystem, so it can never name a file outside waitsDir. + if !waitSessionID.MatchString(sessionID) { + writeSystemMessage(stdout, fmt.Sprintf("this session's id (%q) could not be used to find its waits "+ + "(letters, digits, '.', '_' and '-' only)", sessionID)) + return 0 + } + fmt.Fprint(stdout, SessionStartMessage(sessionID, waitsDir, time.Now())) return 0 } diff --git a/modules/claude-code/cmd/claude-code/session_start_test.go b/modules/claude-code/cmd/claude-code/session_start_test.go index a6289166..45379776 100644 --- a/modules/claude-code/cmd/claude-code/session_start_test.go +++ b/modules/claude-code/cmd/claude-code/session_start_test.go @@ -4,6 +4,8 @@ package main import ( "bytes" + "fmt" + "os" "path/filepath" "strings" "testing" @@ -72,6 +74,45 @@ type panicReader struct{} func (panicReader) Read([]byte) (int, error) { panic("boom") } +// Review round 1, item 1 (1743/1744, ADR 0489 §5): a session id that is not a plain one is never used to +// build a file path — reading outside the waits dir never happens — and the hook prints only a line saying +// the id could not be used. +func TestSessionStartRejectsAnUnsafeSessionID(t *testing.T) { + root := t.TempDir() + waitsDir := filepath.Join(root, "agent", "waits") + if err := os.MkdirAll(waitsDir, 0o700); err != nil { + t.Fatal(err) + } + // What "../../x" resolves to from waitsDir: root/x.json, outside waitsDir, with content that must never be + // read through it. + outside := filepath.Join(waitsDir, "../../x.json") + writeWaitsFixture(t, outside, WaitsFile{Session: "x", At: time.Now(), + Waits: []Wait{{ID: "w-secret", What: "SECRET-DATA", Bound: 10, At: time.Now()}}}) + t.Cleanup(func() { _ = os.Remove(outside) }) + + for _, id := range []string{"../../x", "a/b"} { + var out bytes.Buffer + stdin := strings.NewReader(fmt.Sprintf(`{"session_id":%q}`, id)) + if code := runSessionStart(waitsDir, stdin, &out); code != 0 { + t.Fatalf("id %q: code %d", id, code) + } + if strings.Contains(out.String(), "SECRET-DATA") { + t.Fatalf("id %q: read a file outside the waits dir: %s", id, out.String()) + } + if !strings.Contains(out.String(), "could not be used") { + t.Fatalf("id %q: did not say the id could not be used: %s", id, out.String()) + } + // Item (a): the notice reaches the screen as the one JSON object Claude Code shows. + if !strings.Contains(out.String(), "systemMessage") { + t.Fatalf("id %q: the notice is not a systemMessage: %s", id, out.String()) + } + } + var out bytes.Buffer + if code := runSessionStart(waitsDir, strings.NewReader(`{"session_id":""}`), &out); code != 0 || out.Len() != 0 { + t.Fatalf("blank id: code %d, out %q", code, out.String()) + } +} + // 1744/AC4 is checked together with 1743/AC7 in terminal_only_test.go's // TestThePluginsOnlyHooksAreTheGuardTheSessionStartHookAndTheStopHook. diff --git a/modules/claude-code/cmd/claude-code/stop.go b/modules/claude-code/cmd/claude-code/stop.go index 2457c132..5e2c5db6 100644 --- a/modules/claude-code/cmd/claude-code/stop.go +++ b/modules/claude-code/cmd/claude-code/stop.go @@ -49,8 +49,8 @@ func waitBlocksStop(session string, w Wait, now time.Time) (bool, string) { return false, "" } return true, fmt.Sprintf("wait %s (%s) is open and nothing follows it yet. Read where it stands with %s, then "+ - "either mesh_wait on %s or start a watcher that rereads it, and mark it watched: claude_code_wait "+ - "{\"op\":\"watched\",\"session\":%q,\"id\":%q,\"watcher\":\"...\"}.", w.ID, w.What, w.Read.Address, familyOrAddress(w), + "%s, and mark it watched: claude_code_wait "+ + "{\"op\":\"watched\",\"session\":%q,\"id\":%q,\"watcher\":\"...\"}.", w.ID, w.What, w.Read.Address, watcherAdvice(w), session, w.ID) } return true, fmt.Sprintf("wait %s (%s) is past its bound and the operator has not been told. Tell the operator what "+ @@ -58,11 +58,15 @@ func waitBlocksStop(session string, w Wait, now time.Time) (bool, string) { "{\"op\":\"said\",\"session\":%q,\"id\":%q}.", w.ID, w.What, session, w.ID) } -func familyOrAddress(w Wait) string { +// watcherAdvice is what a session is told to do to follow a wait (ADR 0489 §3, review round 1 item 3): a +// family is a candidate for mesh_wait only when the tool runner is granted it (ADR 0367 §2), which this hook +// cannot know, so it is offered conditionally; a watcher that rereads the read address always works. +func watcherAdvice(w Wait) string { if w.Family != "" { - return w.Family + return fmt.Sprintf("mesh_wait on %s if this session's tool runner is granted it, otherwise start a watcher "+ + "that rereads %s", w.Family, w.Read.Address) } - return w.Read.Address + return fmt.Sprintf("start a watcher that rereads %s", w.Read.Address) } // DecideStop is the Stop hook's decision, pure of IO: f and readErr are what reading the session's waits @@ -103,32 +107,61 @@ func readRefusalCounters(waitsDir, session string) map[string]int { return m } -// writeRefusalCounters is best-effort: a counter file that cannot be written only means the three-strikes -// cap may be judged again from zero, never that the hook fails. -func writeRefusalCounters(waitsDir, session string, m map[string]int) { +// writeRefusalCounters writes the counter file beside the waits file. Its caller decides what an error here +// means: harmless when the stop is already allowed, but when the stop was about to be refused, a cap that +// cannot be written is a cap that cannot take effect (ADR 0489 §5 "never loops", review round 1 item 2). +func writeRefusalCounters(waitsDir, session string, m map[string]int) error { raw, err := json.Marshal(m) if err != nil { - return + return err } - _ = os.WriteFile(refusalCountersPath(waitsDir, session), raw, 0o600) + return os.WriteFile(refusalCountersPath(waitsDir, session), raw, 0o600) +} + +// notice says a fail-open or let-through reason on both the log (stderr) and the screen: Claude Code shows a +// hook's "systemMessage" on stdout, but not stderr, when the hook allows (review round 1, item a). +func notice(stdout, stderr io.Writer, reason string) { + fmt.Fprintln(stderr, reason) + writeSystemMessage(stdout, reason) } // runStop is the hook: exit 2 with the reason on stderr refuses the stop, exit 0 allows (1743). -func runStop(waitsDir string, stdin io.Reader, stderr io.Writer) (code int) { +func runStop(waitsDir string, stdin io.Reader, stdout, stderr io.Writer) (code int) { defer func() { if recover() != nil { code = 0 // a panic always allows (1743/AC6) } }() var in StopInput - if err := json.NewDecoder(stdin).Decode(&in); err != nil || strings.TrimSpace(in.SessionID) == "" { + if err := json.NewDecoder(stdin).Decode(&in); err != nil { + return 0 + } + sessionID := strings.TrimSpace(in.SessionID) + if sessionID == "" { + return 0 + } + // Review round 1, item 1: a session id is never used to build a path until it is checked against + // waitSessionID — no "/" in it reaches the filesystem, so it can never name a file outside waitsDir. + if !waitSessionID.MatchString(sessionID) { + notice(stdout, stderr, fmt.Sprintf("the session id %q could not be used to find its waits (letters, digits, "+ + "'.', '_' and '-' only); this stop is allowed", sessionID)) + return 0 + } + f, err := readWaitsFile(waitsFileIn(waitsDir, sessionID)) + counters := readRefusalCounters(waitsDir, sessionID) + d := DecideStop(sessionID, f, err, counters, time.Now()) + if werr := writeRefusalCounters(waitsDir, sessionID, d.Counters); werr != nil && d.Refuse { + // Item 2: writing the counter failed on the very call that would have incremented it toward the cap; + // refusing anyway could hold the turn forever, so this call is let through instead. + notice(stdout, stderr, fmt.Sprintf("this wait's refusal counter could not be written (%v); allowed so it "+ + "cannot hold the turn forever", werr)) return 0 } - f, err := readWaitsFile(waitsFileIn(waitsDir, in.SessionID)) - d := DecideStop(in.SessionID, f, err, readRefusalCounters(waitsDir, in.SessionID), time.Now()) - writeRefusalCounters(waitsDir, in.SessionID, d.Counters) if d.Reason != "" { fmt.Fprintln(stderr, d.Reason) + if !d.Refuse { + writeSystemMessage(stdout, d.Reason) // item a: a fail-open or let-through notice also reaches the screen + } } if d.Refuse { return 2 diff --git a/modules/claude-code/cmd/claude-code/stop_test.go b/modules/claude-code/cmd/claude-code/stop_test.go index 9bb4a121..a9dfcb82 100644 --- a/modules/claude-code/cmd/claude-code/stop_test.go +++ b/modules/claude-code/cmd/claude-code/stop_test.go @@ -6,6 +6,7 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "os" "path/filepath" "strings" @@ -20,10 +21,12 @@ func openWait(id string, bound int) Wait { } // 1743/AC1: a wait within its bound and unwatched refuses the stop, naming the wait, its read address and -// what the session does about it. +// what the session does about it (mesh_wait on its family, conditionally — review round 1 item 3 — or a +// watcher that rereads the address). func TestStopRefusesAnUnwatchedWaitWithinItsBound(t *testing.T) { w := openWait("w-1", 10) w.Read = WaitRead{Address: "mesh-delivery.recheck"} + w.Family = "mesh-delivery.decided" d := DecideStop("sess-1", WaitsFile{Waits: []Wait{w}}, nil, map[string]int{}, stopT0.Add(time.Minute)) if !d.Refuse || !strings.Contains(d.Reason, "w-1") || !strings.Contains(d.Reason, "mesh-delivery.recheck") || !strings.Contains(d.Reason, "mesh_wait") || !strings.Contains(d.Reason, "watcher") { @@ -70,13 +73,17 @@ func TestStopAllowsWhenTheWaitsFileCannotBeRead(t *testing.T) { t.Fatalf("%+v", d) } waitsDir := t.TempDir() // no file at all for sess-4b - var stderr bytes.Buffer + var stdout, stderr bytes.Buffer stdin := strings.NewReader(`{"session_id":"sess-4b","stop_hook_active":false}`) start := time.Now() - code := runStop(waitsDir, stdin, &stderr) + code := runStop(waitsDir, stdin, &stdout, &stderr) if code != 0 || !strings.Contains(stderr.String(), "could not be read") { t.Fatalf("code %d, stderr %s", code, stderr.String()) } + // Item (a): a fail-open notice also reaches the screen, as the one JSON object Claude Code shows. + if !strings.Contains(stdout.String(), "systemMessage") || !strings.Contains(stdout.String(), "could not be read") { + t.Fatalf("the fail-open notice did not reach stdout: %s", stdout.String()) + } if took := time.Since(start); took > 5*time.Second { t.Fatalf("took %s; the Stop hook has no network and is bounded under 5s", took) } @@ -113,8 +120,8 @@ func TestStopAllowsTheFourthRefusalInARow(t *testing.T) { // 1743/AC6: a panic answers allow. func TestStopAPanicAllows(t *testing.T) { waitsDir := t.TempDir() - var stderr bytes.Buffer - if code := runStop(waitsDir, panicReader{}, &stderr); code != 0 { + var stdout, stderr bytes.Buffer + if code := runStop(waitsDir, panicReader{}, &stdout, &stderr); code != 0 { t.Fatalf("a panic was not recovered: code %d", code) } } @@ -127,15 +134,19 @@ func TestRunStopKeepsItsCounterBesideTheWaitsFile(t *testing.T) { stdin := func() *strings.Reader { return strings.NewReader(`{"session_id":"sess-6","stop_hook_active":false}`) } for i := 1; i <= MostStopRefusals; i++ { - var stderr bytes.Buffer - if code := runStop(waitsDir, stdin(), &stderr); code != 2 { + var stdout, stderr bytes.Buffer + if code := runStop(waitsDir, stdin(), &stdout, &stderr); code != 2 { t.Fatalf("refusal %d: code %d, stderr %s", i, code, stderr.String()) } } - var stderr bytes.Buffer - if code := runStop(waitsDir, stdin(), &stderr); code != 0 || !strings.Contains(stderr.String(), "unwatched") { + var stdout, stderr bytes.Buffer + if code := runStop(waitsDir, stdin(), &stdout, &stderr); code != 0 || !strings.Contains(stderr.String(), "unwatched") { t.Fatalf("the fourth call: code %d, stderr %s", code, stderr.String()) } + // Item (a): the let-through notice reaches the screen too, not only the log. + if !strings.Contains(stdout.String(), "systemMessage") || !strings.Contains(stdout.String(), "unwatched") { + t.Fatalf("the let-through notice did not reach stdout: %s", stdout.String()) + } raw, err := os.ReadFile(refusalCountersPath(waitsDir, "sess-6")) if err != nil { t.Fatal(err) @@ -146,6 +157,87 @@ func TestRunStopKeepsItsCounterBesideTheWaitsFile(t *testing.T) { } } +// Review round 1, item 1 (1743/1744, ADR 0489 §5): a session id that is not a plain one (path separators, +// ".."), is never used to build a file path — the counter file stays inside the waits directory — and the +// Stop hook allows, saying why. +func TestStopRejectsAnUnsafeSessionID(t *testing.T) { + for _, id := range []string{"../../x", "a/b"} { + root := t.TempDir() + waitsDir := filepath.Join(root, "agent", "waits") + if err := os.MkdirAll(waitsDir, 0o700); err != nil { + t.Fatal(err) + } + var stdout, stderr bytes.Buffer + stdin := strings.NewReader(fmt.Sprintf(`{"session_id":%q,"stop_hook_active":false}`, id)) + if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 0 { + t.Fatalf("id %q: code %d, stderr %s", id, code, stderr.String()) + } + if !strings.Contains(stderr.String(), "could not be used") { + t.Fatalf("id %q: did not say why it allowed: %s", id, stderr.String()) + } + if !strings.Contains(stdout.String(), "systemMessage") { + t.Fatalf("id %q: the notice did not reach stdout: %s", id, stdout.String()) + } + escaped := filepath.Join(waitsDir, id+".refusals.json") + if _, err := os.Stat(escaped); !os.IsNotExist(err) { + t.Fatalf("id %q: a counter file was written outside the waits dir at %s", id, escaped) + } + } + // A blank id still just allows, quietly: it never reaches the point of building a path at all. + waitsDir := t.TempDir() + var stdout, stderr bytes.Buffer + if code := runStop(waitsDir, strings.NewReader(`{"session_id":""}`), &stdout, &stderr); code != 0 { + t.Fatalf("blank id: code %d", code) + } +} + +// Review round 1, item 2 (ADR 0489 §5 "never loops", 1743/AC5): when the decision is to refuse and the +// refusal counter cannot be written, the hook allows rather than risk refusing forever with a cap that +// never takes effect. +func TestStopAllowsWhenTheRefusalCounterCannotBeWritten(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("running as root: a directory's write bit cannot be taken away") + } + waitsDir := t.TempDir() + writeWaitsFixture(t, filepath.Join(waitsDir, "sess-7.json"), + WaitsFile{Session: "sess-7", At: stopT0, Waits: []Wait{openWait("w-7", 10)}}) + if err := os.Chmod(waitsDir, 0o500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(waitsDir, 0o700) }) + var stdout, stderr bytes.Buffer + stdin := strings.NewReader(`{"session_id":"sess-7","stop_hook_active":false}`) + if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 0 { + t.Fatalf("code %d, stderr %s", code, stderr.String()) + } + if !strings.Contains(stderr.String(), "counter") { + t.Fatalf("did not say the counter could not be written: %s", stderr.String()) + } + if !strings.Contains(stdout.String(), "systemMessage") { + t.Fatalf("the notice did not reach stdout: %s", stdout.String()) + } +} + +// Review round 1, item 3 (1743/AC1, §3 with ADR 0367 §2): a family is offered only conditionally (the tool +// runner may not be granted it); without one, the advice is only to start a watcher. +func TestWaitBlocksStopAdvisesMeshWaitOnlyConditionallyOtherwiseAWatcher(t *testing.T) { + withFamily := openWait("w-f", 10) + withFamily.Read = WaitRead{Address: "mesh-delivery.recheck"} + withFamily.Family = "mesh-delivery.decided" + _, reason := waitBlocksStop("sess-f", withFamily, stopT0.Add(time.Minute)) + if !strings.Contains(reason, "mesh_wait on mesh-delivery.decided if this session's tool runner is granted it") || + !strings.Contains(reason, "otherwise start a watcher that rereads mesh-delivery.recheck") { + t.Fatalf("%s", reason) + } + + noFamily := openWait("w-nf", 10) + noFamily.Read = WaitRead{Address: "mesh-delivery.recheck"} + _, reason2 := waitBlocksStop("sess-nf", noFamily, stopT0.Add(time.Minute)) + if strings.Contains(reason2, "mesh_wait") || !strings.Contains(reason2, "start a watcher that rereads mesh-delivery.recheck") { + t.Fatalf("%s", reason2) + } +} + // 1743/AC7's rendering half (hooks.json holds the guard's PreToolUse entry and the Stop entry) is checked // together with 1744/AC4 in terminal_only_test.go's // TestThePluginsOnlyHooksAreTheGuardTheSessionStartHookAndTheStopHook. claude_code_hook_register still diff --git a/modules/claude-code/cmd/claude-code/waits.go b/modules/claude-code/cmd/claude-code/waits.go index 23db64da..8f0e24a1 100644 --- a/modules/claude-code/cmd/claude-code/waits.go +++ b/modules/claude-code/cmd/claude-code/waits.go @@ -52,6 +52,17 @@ const mostWaitAttempts = 20 var waitSessionID = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$`) +// requireSessionID trims session and refuses it unless it is a plain id matching waitSessionID — every op +// holds to this, not only add (review round 1, item b), so a session id is never used to build a path from +// anything but a safe name (no "/", so it cannot name a file outside the waits directory). +func requireSessionID(session string) (string, error) { + session = strings.TrimSpace(session) + if !waitSessionID.MatchString(session) { + return "", errors.New("session is required: the id the SessionStart hook named") + } + return session, nil +} + // WaitRead is the verb a session reads to see where one wait stands. type WaitRead struct { Address string `json:"address"` @@ -246,9 +257,9 @@ type WaitInput struct { // AddWait keeps one more wait for a session, refusing outside ADR 0489's bounds (1629/AC5), and rewrites the // session's mirrored file (1629/AC3). func AddWait(p Paths, in WaitInput, store WaitStore, now time.Time) (map[string]any, error) { - session := strings.TrimSpace(in.Session) - if !waitSessionID.MatchString(session) { - return nil, errors.New("session is required: the id the SessionStart hook named") + session, err := requireSessionID(in.Session) + if err != nil { + return nil, err } by, err := sessionAndNode(p, in.By) if err != nil { @@ -298,7 +309,11 @@ func findWait(sw SessionWaits, id string) int { // WatchedWait marks a wait as followed: by mesh_wait on its family, or a watcher started for it (1629/AC2). func WatchedWait(p Paths, session, id, watcher string, store WaitStore, now time.Time) (map[string]any, error) { - session, id, watcher = strings.TrimSpace(session), strings.TrimSpace(id), strings.TrimSpace(watcher) + session, err := requireSessionID(session) + if err != nil { + return nil, err + } + id, watcher = strings.TrimSpace(id), strings.TrimSpace(watcher) if id == "" { return nil, errors.New("id is required: the one add answered") } @@ -325,7 +340,11 @@ func WatchedWait(p Paths, session, id, watcher string, store WaitStore, now time // DoneWait removes a wait from the open list: it ended, with outcome (1629/AC2). func DoneWait(p Paths, session, id, outcome string, store WaitStore, now time.Time) (map[string]any, error) { - session, id, outcome = strings.TrimSpace(session), strings.TrimSpace(id), strings.TrimSpace(outcome) + session, err := requireSessionID(session) + if err != nil { + return nil, err + } + id, outcome = strings.TrimSpace(id), strings.TrimSpace(outcome) if id == "" { return nil, errors.New("id is required: the one add answered") } @@ -351,7 +370,11 @@ func DoneWait(p Paths, session, id, outcome string, store WaitStore, now time.Ti // SaidWait marks a wait past its bound as told to the operator: refused while it is still within its bound // (1629/AC2). func SaidWait(p Paths, session, id string, store WaitStore, now time.Time) (map[string]any, error) { - session, id = strings.TrimSpace(session), strings.TrimSpace(id) + session, err := requireSessionID(session) + if err != nil { + return nil, err + } + id = strings.TrimSpace(id) if id == "" { return nil, errors.New("id is required: the one add answered") } @@ -378,9 +401,9 @@ func SaidWait(p Paths, session, id string, store WaitStore, now time.Time) (map[ // ListWaits answers a session's open waits, with their age and whether they are past their bound (1629/AC1). func ListWaits(p Paths, session string, store WaitStore, now time.Time) (map[string]any, error) { - session = strings.TrimSpace(session) - if session == "" { - return nil, errors.New("session is required: the id the SessionStart hook named") + session, err := requireSessionID(session) + if err != nil { + return nil, err } sw, _, _, err := readSessionWaits(store, waitsKey(p, session)) if err != nil { diff --git a/modules/claude-code/cmd/claude-code/waits_test.go b/modules/claude-code/cmd/claude-code/waits_test.go index 59f4f0a8..195c71dd 100644 --- a/modules/claude-code/cmd/claude-code/waits_test.go +++ b/modules/claude-code/cmd/claude-code/waits_test.go @@ -232,6 +232,27 @@ func TestAddRefusesOutsideItsBounds(t *testing.T) { } } +// Review round 1 item (b): list, watched, done and said refuse a session id that is not a plain one (the +// 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") + for _, bad := range []string{"", "a/b", "../x"} { + store := newMemWaits() + if _, err := ListWaits(p, bad, 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") { + t.Errorf("watched %q: %v", bad, err) + } + if _, err := DoneWait(p, bad, "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") { + t.Errorf("said %q: %v", bad, err) + } + } +} + // 1629/AC6: the tool is served through the module's own dispatch. func TestTheWaitToolIsServedThroughTheModulesDispatch(t *testing.T) { p, _ := node(t, "g14") -- 2.54.0 From f0beea153e12b561cdacd0c5a88e0cec1b6b4d8e Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 18:32:50 +0200 Subject: [PATCH 5/6] Key a wait list by name, not session id, so it survives a /clear (issues 1629, 1744, ADR 0490) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../cmd/claude-code/session_start.go | 61 ++- .../cmd/claude-code/session_start_test.go | 25 + modules/claude-code/cmd/claude-code/waits.go | 464 +++++++++++++----- .../claude-code/cmd/claude-code/waits_test.go | 248 ++++++++-- 4 files changed, 602 insertions(+), 196 deletions(-) diff --git a/modules/claude-code/cmd/claude-code/session_start.go b/modules/claude-code/cmd/claude-code/session_start.go index 84326dd5..3685ef1f 100644 --- a/modules/claude-code/cmd/claude-code/session_start.go +++ b/modules/claude-code/cmd/claude-code/session_start.go @@ -1,15 +1,16 @@ package main -// The plugin's SessionStart hook (novox/hq ADR 0489 §1, issue 1744): `claude-code session-start `, -// run by the agent's own account on every start, resume, /clear and /compact. The tool runner never tells a -// session its own id (only a hook's stdin carries session_id), so a session cannot name itself to -// claude_code_wait until this hook has told it. It also lists the session's still-open waits, so a session -// that /clear-ed sees what it was watching before. +// The plugin's SessionStart hook (novox/hq ADR 0489 §1, ADR 0490 §2, issue 1744): `claude-code session-start +// `, run by the agent's own account on every start, resume, /clear and /compact. The tool runner +// never tells a session its own id (only a hook's stdin carries session_id), so a session cannot name itself +// to claude_code_wait until this hook has told it. It also lists this session id's own still-open waits (if +// any name is already bound to it) and, always, every name on this node that still holds an open wait, so a +// session with a brand new id (a /clear gives one) can see a name it may want to adopt. // -// It reads the session's mirrored waits file directly — the same local read the Stop hook makes -// (readWaitsFile, waits.go) — never calling back into this module: this hook runs on every start, and must -// not wait on the bus to do it. A missing or malformed file is not an error here: there is simply nothing -// open to report. +// It reads the session's mirrored waits file and the node-wide index directly — the same local reads the +// Stop hook and claude_code_wait make (readWaitsFile, readNodeIndex, waits.go) — never calling back into +// this module: this hook runs on every start, and must not wait on the bus to do it. A missing or malformed +// file is not an error here: there is simply nothing open to report. import ( "encoding/json" @@ -25,29 +26,35 @@ type SessionStartInput struct { } // SessionStartMessage is the line the hook prints: the session's id and how to give it to claude_code_wait, -// then each still-open wait from before (1744/AC1, AC2). Pure of IO beyond the one file read. +// then each still-open wait already bound to this session id (1744/AC1, AC2), then every name on this node +// that still holds an open wait, with how to adopt it (ADR 0490 §2). Pure of IO beyond the two file reads. func SessionStartMessage(sessionID, waitsDir string, now time.Time) string { var b strings.Builder fmt.Fprintf(&b, "This session's id is %s; give it as `session` to claude_code_wait.\n", sessionID) - f, err := readWaitsFile(waitsFileIn(waitsDir, sessionID)) - - if err != nil || len(f.Waits) == 0 { - return b.String() - } - b.WriteString("Waits still open from before this session's last start, resume, /clear or /compact:\n") - for _, w := range f.Waits { - age := now.Sub(w.At).Round(time.Second) - state := "no watcher yet" - if w.Watcher != "" { - state = "watched" - } - if w.PastBound(now) { - state += ", past its bound" - if w.Said { - state += ", said" + if f, err := readWaitsFile(waitsFileIn(waitsDir, sessionID)); err == nil && len(f.Waits) > 0 { + b.WriteString("Waits already bound to this session's own id:\n") + for _, w := range f.Waits { + age := now.Sub(w.At).Round(time.Second) + state := "no watcher yet" + if w.Watcher != "" { + state = "watched" } + if w.PastBound(now) { + state += ", past its bound" + if w.Said { + state += ", said" + } + } + fmt.Fprintf(&b, " %s — %s (%s ago, %s)\n", w.ID, w.What, age, state) + } + } + if idx, err := readNodeIndex(nodeIndexPathIn(waitsDir)); err == nil && len(idx.Names) > 0 { + b.WriteString("Open waits on this node, by name:\n") + for _, n := range idx.Names { + fmt.Fprintf(&b, " %s: %d open wait(s), last called %s. If this is you: claude_code_wait "+ + "{\"op\":\"adopt\",\"by\":%q,\"session\":%q}\n", n.Name, n.Open, n.LastCall.Round(time.Second).Format(time.RFC3339), + n.Name+"@"+idx.Node, sessionID) } - fmt.Fprintf(&b, " %s — %s (%s ago, %s)\n", w.ID, w.What, age, state) } return b.String() } diff --git a/modules/claude-code/cmd/claude-code/session_start_test.go b/modules/claude-code/cmd/claude-code/session_start_test.go index 45379776..a1082709 100644 --- a/modules/claude-code/cmd/claude-code/session_start_test.go +++ b/modules/claude-code/cmd/claude-code/session_start_test.go @@ -116,6 +116,31 @@ func TestSessionStartRejectsAnUnsafeSessionID(t *testing.T) { // 1744/AC4 is checked together with 1743/AC7 in terminal_only_test.go's // TestThePluginsOnlyHooksAreTheGuardTheSessionStartHookAndTheStopHook. +// ADR 0490 §2: every session start lists every name on this node with an open wait, from the node-wide +// index, with the exact adopt call to make if it is yours — read directly, never by calling back into this +// module. +func TestSessionStartListsEveryNameOnTheNodeWithOpenWaits(t *testing.T) { + waitsDir := t.TempDir() + lastCall := time.Date(2026, 10, 21, 9, 55, 0, 0, time.UTC) + idx := NodeIndex{Node: "g14", At: lastCall, Names: []NodeIndexEntry{ + {Name: "fix-1629", Open: 2, CurrentSession: "sess-old", LastCall: lastCall}, + }} + raw, err := indented(idx) + if err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(waitsDir, "_index.json"), string(raw)) + + msg := SessionStartMessage("sess-brand-new", waitsDir, lastCall.Add(5*time.Minute)) + if !strings.Contains(msg, "fix-1629") || !strings.Contains(msg, "2 open wait") { + t.Fatalf("the name and its count are not listed:\n%s", msg) + } + want := `claude_code_wait {"op":"adopt","by":"fix-1629@g14","session":"sess-brand-new"}` + if !strings.Contains(msg, want) { + t.Fatalf("the exact adopt call is not given:\n%s\nwant it to contain:\n%s", msg, want) + } +} + func writeWaitsFixture(t *testing.T, path string, f WaitsFile) { t.Helper() raw, err := indented(f) diff --git a/modules/claude-code/cmd/claude-code/waits.go b/modules/claude-code/cmd/claude-code/waits.go index 8f0e24a1..f44711ad 100644 --- a/modules/claude-code/cmd/claude-code/waits.go +++ b/modules/claude-code/cmd/claude-code/waits.go @@ -1,18 +1,28 @@ package main -// A session's wait list (novox/hq ADR 0489): what a session still expects an answer to — an Approve, a -// delivery's end, anything it started and must act on when it ends (follow-what-you-start) — so the -// plugin's Stop hook can refuse to end its turn while one is open and nothing watches it. +// A session's wait list (novox/hq ADR 0489, as changed by ADR 0490): what a session still expects an answer +// to — an Approve, a delivery's end, anything it started and must act on when it ends +// (follow-what-you-start) — so the plugin's Stop hook can refuse to end its turn while one is open and +// nothing watches it. // -// Kept in the module's `waits` state under the session's id and this node (so the same session reads its -// waits again after a /clear, and after the module restarts), and mirrored whole, atomically, to a file the -// hooks read without calling back into this module: /.claude/nox-mesh/waits/.json. That -// path is under the agent's home (the manifest's `agent-home` resource), which this process — launched as -// the operator's account (novox/hq ADR 0193) — cannot itself write when the node names an agent account of -// its own (novox/hq ADR 0266): every write below goes through `fsys` (homefs.go), which already carries such -// a write through the sudo helper the guard's binary also serves, exactly as grant.go's WriteCredentials -// does for the agent's own credentials file. Where the node has no agent account of its own, fsys is the -// plain filesystem and the write reaches the operator's own home directly. +// Kept in the module's `waits` state under the session's name and this node (ADR 0490 §1) — `by`, as +// claude_code_ask takes it, never the session id, which Claude Code hands a session a new one of at a +// /clear. Every call also gives `session`, the caller's current id; the module keeps the binding of a name +// to the session that last called for it, with that call's time, so the same name's waits are found again +// across a /clear once the new session has adopted it. +// +// The waits file a hook reads is kept per session id, not per name, and holds the waits of whichever name is +// currently bound to that id (empty for an id nothing is bound to): written whole, atomically, on every +// change, and rewritten for both ids when a binding moves (AdoptName). A second file, the node-wide index, +// lists every name on this node that still holds an open wait, so the SessionStart hook can tell a session +// with a brand new id what it might want to adopt, without calling back into this module. +// +// Every file lives under the agent's home (the manifest's `agent-home` resource), which this process — +// launched as the operator's account (novox/hq ADR 0193) — cannot itself write when the node names an agent +// account of its own (novox/hq ADR 0266): every write below goes through `fsys` (homefs.go), which already +// carries such a write through the sudo helper the guard's binary also serves, exactly as grant.go's +// WriteCredentials does for the agent's own credentials file. Where the node has no agent account of its +// own, fsys is the plain filesystem and the write reaches the operator's own home directly. import ( "crypto/rand" @@ -23,6 +33,7 @@ import ( "os" "path/filepath" "regexp" + "sort" "strings" "time" @@ -38,7 +49,7 @@ const WaitsDir = "nox-mesh/waits" // WaitPrefix starts every wait's id. const WaitPrefix = "w-" -// MostWaits is how many open waits one session may hold at once (novox/hq ADR 0489, 1629/AC5). +// MostWaits is how many open waits one name may hold at once (novox/hq ADR 0489, 1629/AC5). const MostWaits = 50 // The bounds of a wait's bound, in minutes (1629/AC5). @@ -53,8 +64,8 @@ const mostWaitAttempts = 20 var waitSessionID = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$`) // requireSessionID trims session and refuses it unless it is a plain id matching waitSessionID — every op -// holds to this, not only add (review round 1, item b), so a session id is never used to build a path from -// anything but a safe name (no "/", so it cannot name a file outside the waits directory). +// holds to this (review round 1, item b), so a session id is never used to build a path from anything but a +// safe name (no "/", so it cannot name a file outside the waits directory). func requireSessionID(session string) (string, error) { session = strings.TrimSpace(session) if !waitSessionID.MatchString(session) { @@ -69,7 +80,8 @@ type WaitRead struct { Arguments map[string]any `json:"arguments,omitempty"` } -// Wait is one answer a session wrote down that it expects (novox/hq ADR 0489 §1-§2). +// Wait is one answer a session wrote down that it expects (novox/hq ADR 0489 §1-§2). By is the name it was +// added under. type Wait struct { ID string `json:"id"` By string `json:"by"` @@ -87,27 +99,50 @@ func (w Wait) PastBound(now time.Time) bool { return now.Sub(w.At) > time.Duration(w.Bound)*time.Minute } -// SessionWaits is what the `waits` state keeps for one session: its open waits. -type SessionWaits struct { - Session string `json:"session"` - Node string `json:"node"` - Waits []Wait `json:"waits"` +// NamedWaits is what the `waits` state keeps for one name on this node (novox/hq ADR 0490 §1): its open +// waits, and the binding to whichever session currently answers to that name — empty until the first call +// names it, by add or by adopt. +type NamedWaits struct { + Name string `json:"name"` + Node string `json:"node"` + CurrentSession string `json:"currentSession,omitempty"` + LastCall time.Time `json:"lastCall,omitempty"` + Waits []Wait `json:"waits"` } -// WaitsFile is what the mirrored file holds, whole, on every change (1629/AC3): the session's open waits and -// the time they were written. +// WaitsFile is what one session id's mirrored file holds, whole, on every change (1629/AC3): the open waits +// of whichever name is currently bound to this session, and the time they were written. type WaitsFile struct { Session string `json:"session"` At time.Time `json:"at"` Waits []Wait `json:"waits"` } +// NodeIndexEntry is one name's open waits, as the node-wide index lists it for the SessionStart hook +// (novox/hq ADR 0490 §2). +type NodeIndexEntry struct { + Name string `json:"name"` + Open int `json:"open"` + CurrentSession string `json:"currentSession,omitempty"` + LastCall time.Time `json:"lastCall"` +} + +// NodeIndex is the node-wide index file's whole content (novox/hq ADR 0490 §2): every name on this node that +// still holds an open wait, written by the tool on every change a name's waits or binding takes, so the +// SessionStart hook can say "if this is you, adopt" without calling back into this module. +type NodeIndex struct { + Node string `json:"node"` + At time.Time `json:"at"` + Names []NodeIndexEntry `json:"names"` +} + // WaitStore is the `waits` state: a value with its revision, created only when absent, updated only at the -// revision read. +// revision read; Keys lists every name kept, for the node-wide index. type WaitStore interface { Get(key string) (json.RawMessage, uint64, bool, error) Create(key string, value any) error Update(key string, value any, revision uint64) error + Keys() ([]string, error) } // keptWaits is the `waits` state on the bus. @@ -128,6 +163,7 @@ func (k keptWaits) Update(key string, value any, revision uint64) error { _, err := k.s.Update(key, value, revision) return err } +func (k keptWaits) Keys() ([]string, error) { return k.s.Keys() } // Waiting is what claude_code_wait reaches: the `waits` state and the clock. Nil outside a launch. type Waiting struct { @@ -137,9 +173,9 @@ type Waiting struct { var waiting *Waiting -// waitsKey is the state's key for one session's waits: its id and this node (novox/hq ADR 0489 §1), so two -// nodes never collide over the same session id. -func waitsKey(p Paths, session string) string { return p.Node + "/" + session } +// waitsKey is the state's key for one name's waits: its name and this node (novox/hq ADR 0490 §1), so two +// nodes never collide over the same name. +func waitsKey(p Paths, name string) string { return p.Node + "/" + name } // WaitsDirPath is where this node's sessions' waits are mirrored, under the agent's home: the directory the // plugin's SessionStart and Stop hooks (1744, 1743) are given on their command line. @@ -147,11 +183,15 @@ func WaitsDirPath(p Paths) string { return filepath.Join(p.Home, ".claude", filepath.FromSlash(WaitsDir)) } -// waitsFilePath is where one session's waits are mirrored. +// waitsFilePath is where one session id's mirrored waits are. func waitsFilePath(p Paths, session string) string { return filepath.Join(WaitsDirPath(p), session+".json") } +// nodeIndexPath is the node-wide index's file, beside the per-session ones; its name can never collide with +// a session id, since waitSessionID never allows a leading "_". +func nodeIndexPath(p Paths) string { return filepath.Join(WaitsDirPath(p), "_index.json") } + func newWaitID() string { var b [8]byte _, _ = rand.Read(b[:]) @@ -169,27 +209,27 @@ func addressShaped(s string) bool { return dot > 0 && dot < len(s)-1 } -func readSessionWaits(store WaitStore, key string) (SessionWaits, uint64, bool, error) { +func readNamedWaits(store WaitStore, key string) (NamedWaits, uint64, bool, error) { raw, rev, found, err := store.Get(key) if err != nil || !found { - return SessionWaits{}, 0, found, err + return NamedWaits{}, 0, found, err } - var sw SessionWaits - _ = json.Unmarshal(raw, &sw) - return sw, rev, true, nil + var nw NamedWaits + _ = json.Unmarshal(raw, &nw) + return nw, rev, true, nil } -// updateWaits reads one session's waits, lets mutate change them, and writes the result back at the +// updateNamedWaits reads one name's waits, lets mutate change them, and writes the result back at the // revision read — created when there was none yet — retried when another write won the race. -func updateWaits(store WaitStore, key string, mutate func(SessionWaits) (SessionWaits, error)) (SessionWaits, error) { +func updateNamedWaits(store WaitStore, key string, mutate func(NamedWaits) (NamedWaits, error)) (NamedWaits, error) { for i := 0; i < mostWaitAttempts; i++ { - sw, rev, found, err := readSessionWaits(store, key) + nw, rev, found, err := readNamedWaits(store, key) if err != nil { - return SessionWaits{}, fmt.Errorf("the wait list could not be read: %w", err) + return NamedWaits{}, fmt.Errorf("the wait list could not be read: %w", err) } - next, err := mutate(sw) + next, err := mutate(nw) if err != nil { - return SessionWaits{}, err + return NamedWaits{}, err } if !found { err = store.Create(key, next) @@ -202,23 +242,22 @@ func updateWaits(store WaitStore, key string, mutate func(SessionWaits) (Session if errors.Is(err, stdio.ErrChanged) { continue } - return SessionWaits{}, fmt.Errorf("the wait list could not be written: %w", err) + return NamedWaits{}, fmt.Errorf("the wait list could not be written: %w", err) } - return SessionWaits{}, errors.New("the wait list kept changing elsewhere; try again") + return NamedWaits{}, errors.New("the wait list kept changing elsewhere; try again") } -// writeWaitsFile rewrites the session's mirrored file whole, atomically (temp file, then rename): it holds -// exactly the open waits and the time of this write (1629/AC3). -func writeWaitsFile(p Paths, session string, sw SessionWaits, now time.Time) error { - path := waitsFilePath(p, session) +// writeSessionFile rewrites one session id's mirrored file whole, atomically (temp file, then rename): it +// holds exactly the open waits given and the time of this write (1629/AC3). +func writeSessionFile(p Paths, sessionID string, waits []Wait, now time.Time) error { + path := waitsFilePath(p, sessionID) if err := fsys.MkdirAll(filepath.Dir(path), 0o700); err != nil { return err } - waits := sw.Waits if waits == nil { waits = []Wait{} } - raw, err := indented(WaitsFile{Session: session, At: now.UTC(), Waits: waits}) + raw, err := indented(WaitsFile{Session: sessionID, At: now.UTC(), Waits: waits}) if err != nil { return err } @@ -229,12 +268,51 @@ func writeWaitsFile(p Paths, session string, sw SessionWaits, now time.Time) err return fsys.Rename(tmp, path) } -// sessionAndNode reads `by` as claude_code_ask does: who@node, where node is this one. -func sessionAndNode(p Paths, by string) (string, error) { +// writeNodeIndex rewrites the node-wide index whole, atomically, from every name's current state on this +// node (novox/hq ADR 0490 §2): a full scan of the `waits` state, bounded by how many names are ever used on +// one node at once. Best-effort for its caller: a name's own file is already correct by the time this runs. +func writeNodeIndex(p Paths, store WaitStore, now time.Time) error { + keys, err := store.Keys() + if err != nil { + return fmt.Errorf("the node-wide wait index could not be read: %w", err) + } + prefix := p.Node + "/" + idx := NodeIndex{Node: p.Node, At: now.UTC(), Names: []NodeIndexEntry{}} + for _, k := range keys { + name, ok := strings.CutPrefix(k, prefix) + if !ok { + continue + } + nw, _, found, err := readNamedWaits(store, k) + if err != nil || !found || len(nw.Waits) == 0 { + continue + } + idx.Names = append(idx.Names, NodeIndexEntry{Name: name, Open: len(nw.Waits), + CurrentSession: nw.CurrentSession, LastCall: nw.LastCall.UTC()}) + } + sort.Slice(idx.Names, func(i, j int) bool { return idx.Names[i].Name < idx.Names[j].Name }) + path := nodeIndexPath(p) + if err := fsys.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return err + } + raw, err := indented(idx) + if err != nil { + return err + } + tmp := path + ".mesh-tmp" + if err := fsys.WriteFile(tmp, raw, 0o600); err != nil { + return err + } + return fsys.Rename(tmp, path) +} + +// callerName reads `by` as claude_code_ask does: who@node, where node is this one — the name a wait list is +// kept under (novox/hq ADR 0490 §1), never the session id. +func callerName(p Paths, by string) (string, error) { by = strings.TrimSpace(by) if who, node, found := strings.Cut(by, "@"); found { if node != p.Node { - return "", fmt.Errorf("by names the node %q, and this is %s: add the wait on the node the session runs on", node, p.Node) + return "", fmt.Errorf("by names the node %q, and this is %s: call on the node the session runs on", node, p.Node) } return who, nil } @@ -244,6 +322,30 @@ func sessionAndNode(p Paths, by string) (string, error) { return by, nil } +// bindCaller checks the binding for name against session, refusing when another, more recently bound +// session holds it (novox/hq ADR 0490 §3) — naming that session and when it last called, and how to take the +// name back with adopt — and otherwise (re)binds it to session at now, so a name nothing has bound yet is +// claimed by the first session that calls for it, add included. +func bindCaller(nw *NamedWaits, name, node, by, session string, now time.Time) error { + if nw.CurrentSession != "" && nw.CurrentSession != session { + return fmt.Errorf("the name %q now answers to session %s (it last called at %s); adopt it again if this "+ + "is you: claude_code_wait {\"op\":\"adopt\",\"by\":%q,\"session\":%q}", name, nw.CurrentSession, + nw.LastCall.UTC().Format(time.RFC3339), by, session) + } + nw.Name, nw.Node, nw.CurrentSession, nw.LastCall = name, node, session, now.UTC() + return nil +} + +// findWait is the index of id in waits, or -1. +func findWait(waits []Wait, id string) int { + for i, w := range waits { + if w.ID == id { + return i + } + } + return -1 +} + // WaitInput is what claude_code_wait's add takes. type WaitInput struct { Session string @@ -254,14 +356,14 @@ type WaitInput struct { Bound int } -// AddWait keeps one more wait for a session, refusing outside ADR 0489's bounds (1629/AC5), and rewrites the -// session's mirrored file (1629/AC3). +// AddWait keeps one more wait for a name, refusing outside ADR 0489's bounds (1629/AC5) or when another +// session has since taken the name over (ADR 0490 §3), and rewrites the caller's mirrored file (1629/AC3). func AddWait(p Paths, in WaitInput, store WaitStore, now time.Time) (map[string]any, error) { session, err := requireSessionID(in.Session) if err != nil { return nil, err } - by, err := sessionAndNode(p, in.By) + name, err := callerName(p, in.By) if err != nil { return nil, err } @@ -276,43 +378,42 @@ func AddWait(p Paths, in WaitInput, store WaitStore, now time.Time) (map[string] if !addressShaped(address) { return nil, fmt.Errorf("read.address %q is not an address: the verb that reads where this wait stands, as mesh_call takes it", in.Read.Address) } - w := Wait{ID: newWaitID(), By: by, What: what, Read: WaitRead{Address: address, Arguments: in.Read.Arguments}, + w := Wait{ID: newWaitID(), By: name, What: what, Read: WaitRead{Address: address, Arguments: in.Read.Arguments}, Family: strings.TrimSpace(in.Family), Bound: in.Bound, At: now.UTC()} - key := waitsKey(p, session) - sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { - if len(sw.Waits) >= MostWaits { - return sw, fmt.Errorf("%d waits already wait for session %s; at most %d open waits per session", len(sw.Waits), session, MostWaits) + key := waitsKey(p, name) + nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + if err := bindCaller(&nw, name, p.Node, in.By, session, now); err != nil { + return nw, err } - sw.Session, sw.Node = session, p.Node - sw.Waits = append(append([]Wait(nil), sw.Waits...), w) - return sw, nil + if len(nw.Waits) >= MostWaits { + return nw, fmt.Errorf("%d waits already wait for %s; at most %d open waits per name", len(nw.Waits), name, MostWaits) + } + nw.Waits = append(append([]Wait(nil), nw.Waits...), w) + return nw, nil }) if err != nil { return nil, err } - if err := writeWaitsFile(p, session, sw, now); err != nil { + if err := writeSessionFile(p, session, nw.Waits, now); err != nil { return nil, fmt.Errorf("the wait was kept, but its file could not be written: %w", err) } - say("wait %s added for session %s: %s", w.ID, session, what) - return map[string]any{"id": w.ID, "session": session, "open": len(sw.Waits)}, nil -} - -// findWait is the index of id in sw.Waits, or -1. -func findWait(sw SessionWaits, id string) int { - for i, w := range sw.Waits { - if w.ID == id { - return i - } + if err := writeNodeIndex(p, store, now); err != nil { + say("writing the node-wide wait index after %s added for %s: %v", w.ID, name, err) } - return -1 + say("wait %s added for %s (session %s): %s", w.ID, name, session, what) + return map[string]any{"id": w.ID, "name": name, "session": session, "open": len(nw.Waits)}, nil } // WatchedWait marks a wait as followed: by mesh_wait on its family, or a watcher started for it (1629/AC2). -func WatchedWait(p Paths, session, id, watcher string, store WaitStore, now time.Time) (map[string]any, error) { +func WatchedWait(p Paths, session, by, id, watcher string, store WaitStore, now time.Time) (map[string]any, error) { session, err := requireSessionID(session) if err != nil { return nil, err } + name, err := callerName(p, by) + if err != nil { + return nil, err + } id, watcher = strings.TrimSpace(id), strings.TrimSpace(watcher) if id == "" { return nil, errors.New("id is required: the one add answered") @@ -320,97 +421,127 @@ func WatchedWait(p Paths, session, id, watcher string, store WaitStore, now time if watcher == "" { return nil, errors.New("watcher is required: what follows this wait (mesh_wait on its family, or a watcher agent)") } - key := waitsKey(p, session) - sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { - i := findWait(sw, id) - if i < 0 { - return sw, fmt.Errorf("no open wait %s for session %s", id, session) + key := waitsKey(p, name) + nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { + return nw, err } - sw.Waits[i].Watcher = watcher - return sw, nil + i := findWait(nw.Waits, id) + if i < 0 { + return nw, fmt.Errorf("no open wait %s for %s", id, name) + } + nw.Waits[i].Watcher = watcher + return nw, nil }) if err != nil { return nil, err } - if err := writeWaitsFile(p, session, sw, now); err != nil { + if err := writeSessionFile(p, session, nw.Waits, now); err != nil { return nil, fmt.Errorf("the wait was watched, but its file could not be written: %w", err) } + _ = writeNodeIndex(p, store, now) return map[string]any{"id": id, "watcher": watcher}, nil } // DoneWait removes a wait from the open list: it ended, with outcome (1629/AC2). -func DoneWait(p Paths, session, id, outcome string, store WaitStore, now time.Time) (map[string]any, error) { +func DoneWait(p Paths, session, by, id, outcome string, store WaitStore, now time.Time) (map[string]any, error) { session, err := requireSessionID(session) if err != nil { return nil, err } + name, err := callerName(p, by) + if err != nil { + return nil, err + } id, outcome = strings.TrimSpace(id), strings.TrimSpace(outcome) if id == "" { return nil, errors.New("id is required: the one add answered") } - key := waitsKey(p, session) - sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { - i := findWait(sw, id) - if i < 0 { - return sw, fmt.Errorf("no open wait %s for session %s", id, session) + key := waitsKey(p, name) + nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { + return nw, err } - sw.Waits = append(append([]Wait(nil), sw.Waits[:i]...), sw.Waits[i+1:]...) - return sw, nil + i := findWait(nw.Waits, id) + if i < 0 { + return nw, fmt.Errorf("no open wait %s for %s", id, name) + } + nw.Waits = append(append([]Wait(nil), nw.Waits[:i]...), nw.Waits[i+1:]...) + return nw, nil }) if err != nil { return nil, err } - if err := writeWaitsFile(p, session, sw, now); err != nil { + if err := writeSessionFile(p, session, nw.Waits, now); err != nil { return nil, fmt.Errorf("the wait was done, but its file could not be written: %w", err) } - say("wait %s for session %s done: %s", id, session, outcome) + _ = writeNodeIndex(p, store, now) + say("wait %s for %s (session %s) done: %s", id, name, session, outcome) return map[string]any{"id": id, "done": true, "outcome": outcome}, nil } // SaidWait marks a wait past its bound as told to the operator: refused while it is still within its bound // (1629/AC2). -func SaidWait(p Paths, session, id string, store WaitStore, now time.Time) (map[string]any, error) { +func SaidWait(p Paths, session, by, id string, store WaitStore, now time.Time) (map[string]any, error) { session, err := requireSessionID(session) if err != nil { return nil, err } + name, err := callerName(p, by) + if err != nil { + return nil, err + } id = strings.TrimSpace(id) if id == "" { return nil, errors.New("id is required: the one add answered") } - key := waitsKey(p, session) - sw, err := updateWaits(store, key, func(sw SessionWaits) (SessionWaits, error) { - i := findWait(sw, id) + key := waitsKey(p, name) + nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { + return nw, err + } + i := findWait(nw.Waits, id) if i < 0 { - return sw, fmt.Errorf("no open wait %s for session %s", id, session) + return nw, fmt.Errorf("no open wait %s for %s", id, name) } - if !sw.Waits[i].PastBound(now) { - return sw, fmt.Errorf("wait %s is not past its bound yet: said is for telling the operator about one that is", id) + if !nw.Waits[i].PastBound(now) { + return nw, fmt.Errorf("wait %s is not past its bound yet: said is for telling the operator about one that is", id) } - sw.Waits[i].Said = true - return sw, nil + nw.Waits[i].Said = true + return nw, nil }) if err != nil { return nil, err } - if err := writeWaitsFile(p, session, sw, now); err != nil { + if err := writeSessionFile(p, session, nw.Waits, now); err != nil { return nil, fmt.Errorf("the wait was said, but its file could not be written: %w", err) } + _ = writeNodeIndex(p, store, now) return map[string]any{"id": id, "said": true}, nil } -// ListWaits answers a session's open waits, with their age and whether they are past their bound (1629/AC1). -func ListWaits(p Paths, session string, store WaitStore, now time.Time) (map[string]any, error) { +// ListWaits answers a name's open waits, with their age and whether they are past their bound (1629/AC1). +func ListWaits(p Paths, session, by string, store WaitStore, now time.Time) (map[string]any, error) { session, err := requireSessionID(session) if err != nil { return nil, err } - sw, _, _, err := readSessionWaits(store, waitsKey(p, session)) + name, err := callerName(p, by) if err != nil { - return nil, fmt.Errorf("the wait list could not be read: %w", err) + return nil, err + } + key := waitsKey(p, name) + nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { + return nw, err + } + return nw, nil + }) + if err != nil { + return nil, err } out := []map[string]any{} - for _, w := range sw.Waits { + for _, w := range nw.Waits { entry := map[string]any{"id": w.ID, "what": w.What, "by": w.By, "address": w.Read.Address, "bound_minutes": w.Bound, "at": w.At.Format(time.RFC3339), "age_seconds": int(now.Sub(w.At).Seconds()), "past_bound": w.PastBound(now), "watcher": w.Watcher, "said": w.Said} @@ -422,13 +553,63 @@ func ListWaits(p Paths, session string, store WaitStore, now time.Time) (map[str } out = append(out, entry) } - return map[string]any{"session": session, "open": out}, nil + return map[string]any{"session": session, "name": name, "open": out}, nil +} + +// AdoptName binds name to session, always taking it over from whoever held it (novox/hq ADR 0490 §3): the +// mirrored files of both the session it is taken from and the one adopting it are rewritten at once, so a +// session that no longer owns a name never finds its old waits in its own file, and the new one does right +// away — no waiting on a bound, unlike the Stop and SessionStart hooks' own. +func AdoptName(p Paths, by, session string, store WaitStore, now time.Time) (map[string]any, error) { + session, err := requireSessionID(session) + if err != nil { + return nil, err + } + name, err := callerName(p, by) + if err != nil { + return nil, err + } + key := waitsKey(p, name) + var oldSession string + var oldLastCall time.Time + nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + oldSession, oldLastCall = nw.CurrentSession, nw.LastCall + nw.Name, nw.Node, nw.CurrentSession, nw.LastCall = name, p.Node, session, now.UTC() + return nw, nil + }) + if err != nil { + return nil, err + } + tookOver := oldSession != "" && oldSession != session + if tookOver { + if err := writeSessionFile(p, oldSession, nil, now); err != nil { + return nil, fmt.Errorf("%s was adopted, but the session it was taken from could not be told: %w", name, err) + } + } + if err := writeSessionFile(p, session, nw.Waits, now); err != nil { + return nil, fmt.Errorf("the name was adopted, but its file could not be written: %w", err) + } + if err := writeNodeIndex(p, store, now); err != nil { + say("writing the node-wide wait index after %s was adopted: %v", name, err) + } + answer := map[string]any{"name": name, "session": session, "open": len(nw.Waits)} + if tookOver { + answer["took-over-from"] = map[string]any{"session": oldSession, "last-called": oldLastCall.UTC().Format(time.RFC3339)} + say("%s adopted by session %s, taken over from %s (last called %s)", name, session, oldSession, + oldLastCall.UTC().Format(time.RFC3339)) + } else { + say("%s adopted by session %s", name, session) + } + return answer, nil } // waitsFileIn is where one session's mirrored waits are, given a hook's own waits-dir argument: the hooks // have no launch environment, only their command line, so they never recompute this from Paths. func waitsFileIn(waitsDir, session string) string { return filepath.Join(waitsDir, session+".json") } +// nodeIndexPathIn is the node-wide index file, given a hook's own waits-dir argument. +func nodeIndexPathIn(waitsDir string) string { return filepath.Join(waitsDir, "_index.json") } + // readWaitsFile reads one session's mirrored file, as the hooks do: a plain local read, as the session's own // account (never through fsys or the module's state), and never calling back into this module. func readWaitsFile(path string) (WaitsFile, error) { @@ -443,27 +624,46 @@ func readWaitsFile(path string) (WaitsFile, error) { return f, nil } -// waitTools is claude_code_wait: add, watched, done, said and list, over one session's wait list (novox/hq -// ADR 0489). +// readNodeIndex reads the node-wide index, as the SessionStart hook does: a plain local read, never through +// fsys or the module's state, and never calling back into this module. +func readNodeIndex(path string) (NodeIndex, error) { + raw, err := os.ReadFile(path) + if err != nil { + return NodeIndex{}, err + } + var idx NodeIndex + if err := json.Unmarshal(raw, &idx); err != nil { + return NodeIndex{}, err + } + return idx, nil +} + +// waitTools is claude_code_wait: add, watched, done, said, list and adopt, over a name's wait list (novox/hq +// ADR 0489, ADR 0490). func waitTools(p Paths) []stdio.Tool { notLaunched := errors.New("this module was not launched with the bus; no wait can be kept") return []stdio.Tool{ {Name: "claude_code_wait", - Description: "Your session's wait list (novox/hq ADR 0489): what you still expect an answer to — an Approve, a " + - "delivery's end, anything you started and must act on when it ends. The plugin's Stop hook refuses to end " + - "your turn while a wait here is open and nothing follows it. op is add, watched, done, said or list; session " + - "is always required (the id the SessionStart hook named, or printed again after a /clear). add takes by " + - "(your session's name, as who@node or who), what (what you are waiting for), read (the verb that answers " + - "where it stands: {address, arguments}), family (optional, the event mesh_wait can wait on) and bound_minutes " + - "(1 to 1440) and answers an id; at most 50 waits are open per session. watched takes id and watcher (what " + - "follows it: mesh_wait on its family, or a watcher agent you started). done takes id and outcome, and removes " + - "it from the open list. said takes id and marks a wait past its bound as told to the operator — refused " + - "while it is still within its bound. list answers the open waits, with their age and whether each is past " + - "its bound.", + Description: "Your wait list, kept under your session's name (novox/hq ADR 0489, ADR 0490): what you " + + "still expect an answer to — an Approve, a delivery's end, anything you started and must act on when " + + "it ends. The plugin's Stop hook refuses to end your turn while a wait here is open and nothing " + + "follows it. op is add, watched, done, said, list or adopt; session (your current session id, named " + + "by the SessionStart hook) and by (your session's name, as who@node or who) are always required. A " + + "name is bound to whichever session last called for it; if your own id changed (a /clear) you must " + + "adopt your name again before it answers to you — the SessionStart hook lists every name on this " + + "node with an open wait and how. adopt takes the name over at once, however recently another " + + "session called for it; that session is told so, naming yours, the next time it calls. add takes " + + "what (what you are waiting for), read (the verb that answers where it stands: {address, " + + "arguments}), family (optional, the event mesh_wait can wait on) and bound_minutes (1 to 1440), and " + + "answers an id; at most 50 waits are open per name. watched takes id and watcher (what follows it: " + + "mesh_wait on its family, or a watcher agent you started). done takes id and outcome, and removes it " + + "from the open list. said takes id and marks a wait past its bound as told to the operator — " + + "refused while it is still within its bound. list answers the open waits, with their age and " + + "whether each is past its bound.", Input: map[string]any{ - "op": str("add, watched, done, said or list"), - "session": str("required: the session's id"), - "by": str("add: your session's name, as who@node or who"), + "op": str("add, watched, done, said, list or adopt"), + "session": str("required: your session's current id"), + "by": str("required: your session's name, as who@node or who"), "what": str("add: what you are waiting for, in plain words"), "read": map[string]any{"type": "object", "description": "add: the verb that reads where this wait stands", "properties": map[string]any{ @@ -480,7 +680,7 @@ func waitTools(p Paths) []stdio.Tool { if waiting == nil { return nil, notLaunched } - session := strArg(a, "session") + session, by := strArg(a, "session"), strArg(a, "by") now := time.Now() if waiting.Now != nil { now = waiting.Now() @@ -498,18 +698,20 @@ func waitTools(p Paths) []stdio.Tool { if n, ok := a["bound_minutes"].(float64); ok { bound = int(n) } - return AddWait(p, WaitInput{Session: session, By: strArg(a, "by"), What: strArg(a, "what"), Read: read, + return AddWait(p, WaitInput{Session: session, By: by, What: strArg(a, "what"), Read: read, Family: strArg(a, "family"), Bound: bound}, waiting.Store, now) case "watched": - return WatchedWait(p, session, strArg(a, "id"), strArg(a, "watcher"), waiting.Store, now) + return WatchedWait(p, session, by, strArg(a, "id"), strArg(a, "watcher"), waiting.Store, now) case "done": - return DoneWait(p, session, strArg(a, "id"), strArg(a, "outcome"), waiting.Store, now) + return DoneWait(p, session, by, strArg(a, "id"), strArg(a, "outcome"), waiting.Store, now) case "said": - return SaidWait(p, session, strArg(a, "id"), waiting.Store, now) + return SaidWait(p, session, by, strArg(a, "id"), waiting.Store, now) case "list": - return ListWaits(p, session, waiting.Store, now) + return ListWaits(p, session, by, waiting.Store, now) + case "adopt": + return AdoptName(p, by, session, waiting.Store, now) default: - return nil, fmt.Errorf("op is add, watched, done, said or list, not %q", op) + return nil, fmt.Errorf("op is add, watched, done, said, list or adopt, not %q", op) } }}, } diff --git a/modules/claude-code/cmd/claude-code/waits_test.go b/modules/claude-code/cmd/claude-code/waits_test.go index 195c71dd..cfcafe26 100644 --- a/modules/claude-code/cmd/claude-code/waits_test.go +++ b/modules/claude-code/cmd/claude-code/waits_test.go @@ -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) + } +} -- 2.54.0 From 0f159a166200b45d196da09d8ae96ce6d4bb9d5b Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 18:43:03 +0200 Subject: [PATCH 6/6] Fix review round 2 of the wait list and its hooks (issues 1629, 1743, 1744, ADR 0490) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- modules/claude-code/cmd/claude-code/stop.go | 8 +- .../claude-code/cmd/claude-code/stop_test.go | 31 ++++- modules/claude-code/cmd/claude-code/waits.go | 104 ++++++++++---- .../claude-code/cmd/claude-code/waits_test.go | 128 +++++++++++++++++- 4 files changed, 238 insertions(+), 33 deletions(-) diff --git a/modules/claude-code/cmd/claude-code/stop.go b/modules/claude-code/cmd/claude-code/stop.go index 5e2c5db6..774b9e9a 100644 --- a/modules/claude-code/cmd/claude-code/stop.go +++ b/modules/claude-code/cmd/claude-code/stop.go @@ -13,8 +13,10 @@ package main import ( "encoding/json" + "errors" "fmt" "io" + "io/fs" "os" "path/filepath" "strings" @@ -72,8 +74,12 @@ func watcherAdvice(w Wait) string { // DecideStop is the Stop hook's decision, pure of IO: f and readErr are what reading the session's waits // file gave, previous the refusal counters read beside it, and now the time. Only the first wait that still // blocks the stop is named; the others are read again next time. +// +// A missing file (review round 2, item 1) is the normal state for a session with no waits — every stop of +// every such session, not an exceptional one — so it says nothing and simply allows, the same as a file that +// reads back with no open waits. Only a file that exists but cannot be read, or does not parse, says so. func DecideStop(session string, f WaitsFile, readErr error, previous map[string]int, now time.Time) StopDecision { - if readErr != nil { + if readErr != nil && !errors.Is(readErr, fs.ErrNotExist) { return StopDecision{Reason: fmt.Sprintf("the wait list could not be read (%v); allowed", readErr), Counters: map[string]int{}} } for _, w := range f.Waits { diff --git a/modules/claude-code/cmd/claude-code/stop_test.go b/modules/claude-code/cmd/claude-code/stop_test.go index a9dfcb82..c975cf30 100644 --- a/modules/claude-code/cmd/claude-code/stop_test.go +++ b/modules/claude-code/cmd/claude-code/stop_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -65,14 +66,17 @@ func TestStopRefusesPastBoundUntilSaidWatchedOrNot(t *testing.T) { } } -// 1743/AC4: a missing, unreadable or malformed waits file is allowed, saying so on stderr; the decision does -// not touch the network and answers at once. -func TestStopAllowsWhenTheWaitsFileCannotBeRead(t *testing.T) { +// 1743/AC4: an unreadable or malformed waits file is allowed, saying so on stderr and as a systemMessage on +// stdout; the decision does not touch the network and answers at once. +func TestStopAllowsAndSaysSoWhenTheWaitsFileIsUnreadableOrMalformed(t *testing.T) { d := DecideStop("sess-4", WaitsFile{}, errors.New("no such file"), map[string]int{}, stopT0) if d.Refuse || !strings.Contains(d.Reason, "could not be read") { t.Fatalf("%+v", d) } - waitsDir := t.TempDir() // no file at all for sess-4b + waitsDir := t.TempDir() + if err := os.WriteFile(filepath.Join(waitsDir, "sess-4b.json"), []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } var stdout, stderr bytes.Buffer stdin := strings.NewReader(`{"session_id":"sess-4b","stop_hook_active":false}`) start := time.Now() @@ -89,6 +93,25 @@ func TestStopAllowsWhenTheWaitsFileCannotBeRead(t *testing.T) { } } +// Review round 2, item 1 (1743/AC2, ADR 0490 §2): a missing waits file is the normal state for a session +// with no waits — every stop of every such session — so it must print nothing, on either stream, and +// simply allow; a notice there would fire constantly for nothing wrong. +func TestStopPrintsNothingWhenTheWaitsFileIsSimplyMissing(t *testing.T) { + d := DecideStop("sess-4c", WaitsFile{}, fs.ErrNotExist, map[string]int{}, stopT0) + if d.Refuse || d.Reason != "" { + t.Fatalf("a missing file (fs.ErrNotExist) said something: %+v", d) + } + waitsDir := t.TempDir() // no file at all for sess-4d + var stdout, stderr bytes.Buffer + stdin := strings.NewReader(`{"session_id":"sess-4d","stop_hook_active":false}`) + if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 0 { + t.Fatalf("code %d", code) + } + if stdout.Len() != 0 || stderr.Len() != 0 { + t.Fatalf("a missing waits file printed something: stdout %q, stderr %q", stdout.String(), stderr.String()) + } +} + // 1743/AC5: the same stop refused three times running for the same wait is allowed the fourth time, saying // it was left unwatched; the counter resets when the wait changes. func TestStopAllowsTheFourthRefusalInARow(t *testing.T) { diff --git a/modules/claude-code/cmd/claude-code/waits.go b/modules/claude-code/cmd/claude-code/waits.go index f44711ad..3929fc2a 100644 --- a/modules/claude-code/cmd/claude-code/waits.go +++ b/modules/claude-code/cmd/claude-code/waits.go @@ -247,8 +247,18 @@ func updateNamedWaits(store WaitStore, key string, mutate func(NamedWaits) (Name return NamedWaits{}, errors.New("the wait list kept changing elsewhere; try again") } -// writeSessionFile rewrites one session id's mirrored file whole, atomically (temp file, then rename): it -// holds exactly the open waits given and the time of this write (1629/AC3). +// tempSuffix is a short random name for a temp file, so two writes racing for the same path never share one +// (review round 2, item a): each gets its own, and only the rename is a single atomic step. +func tempSuffix() string { + var b [8]byte + _, _ = rand.Read(b[:]) + return hex.EncodeToString(b[:]) +} + +// writeSessionFile rewrites one session id's mirrored file whole, atomically (its own temp file, then +// rename): it holds exactly the open waits given and the time of this write (1629/AC3). waits is always the +// union of every name currently bound to sessionID (sessionWaitsUnion), never only the name an op acted on: +// a second name bound to the same id must stay enforced (ADR 0490 §1, review round 2 item 2). func writeSessionFile(p Paths, sessionID string, waits []Wait, now time.Time) error { path := waitsFilePath(p, sessionID) if err := fsys.MkdirAll(filepath.Dir(path), 0o700); err != nil { @@ -261,13 +271,51 @@ func writeSessionFile(p Paths, sessionID string, waits []Wait, now time.Time) er if err != nil { return err } - tmp := path + ".mesh-tmp" + tmp := path + ".mesh-tmp-" + tempSuffix() if err := fsys.WriteFile(tmp, raw, 0o600); err != nil { return err } return fsys.Rename(tmp, path) } +// sessionWaitsUnion is every open wait of every name on this node currently bound to session (ADR 0490 §1): +// what that session id's mirrored file must hold, whole — never only the one name an op just acted on, or a +// second name bound to the same id would stop being enforced the moment any op touched the first. +func sessionWaitsUnion(store WaitStore, node, session string) ([]Wait, error) { + keys, err := store.Keys() + if err != nil { + return nil, err + } + prefix := node + "/" + var out []Wait + names := make([]string, 0, len(keys)) + for _, k := range keys { + if strings.HasPrefix(k, prefix) { + names = append(names, k) + } + } + sort.Strings(names) // deterministic order: tests, and a session reading its own file, see the same thing + for _, k := range names { + nw, _, found, err := readNamedWaits(store, k) + if err != nil || !found || nw.CurrentSession != session { + continue + } + out = append(out, nw.Waits...) + } + return out, nil +} + +// writeSessionWaits rewrites session's mirrored file with sessionWaitsUnion: every call that changes a +// name's waits or its binding goes through this, never writeSessionFile directly with one name's waits +// alone. +func writeSessionWaits(p Paths, store WaitStore, session string, now time.Time) error { + union, err := sessionWaitsUnion(store, p.Node, session) + if err != nil { + return fmt.Errorf("the session's other names could not be read: %w", err) + } + return writeSessionFile(p, session, union, now) +} + // writeNodeIndex rewrites the node-wide index whole, atomically, from every name's current state on this // node (novox/hq ADR 0490 §2): a full scan of the `waits` state, bounded by how many names are ever used on // one node at once. Best-effort for its caller: a name's own file is already correct by the time this runs. @@ -299,7 +347,7 @@ func writeNodeIndex(p Paths, store WaitStore, now time.Time) error { if err != nil { return err } - tmp := path + ".mesh-tmp" + tmp := path + ".mesh-tmp-" + tempSuffix() if err := fsys.WriteFile(tmp, raw, 0o600); err != nil { return err } @@ -394,7 +442,7 @@ func AddWait(p Paths, in WaitInput, store WaitStore, now time.Time) (map[string] if err != nil { return nil, err } - if err := writeSessionFile(p, session, nw.Waits, now); err != nil { + if err := writeSessionWaits(p, store, session, now); err != nil { return nil, fmt.Errorf("the wait was kept, but its file could not be written: %w", err) } if err := writeNodeIndex(p, store, now); err != nil { @@ -422,7 +470,7 @@ func WatchedWait(p Paths, session, by, id, watcher string, store WaitStore, now return nil, errors.New("watcher is required: what follows this wait (mesh_wait on its family, or a watcher agent)") } key := waitsKey(p, name) - nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + _, err = updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { return nw, err } @@ -436,7 +484,7 @@ func WatchedWait(p Paths, session, by, id, watcher string, store WaitStore, now if err != nil { return nil, err } - if err := writeSessionFile(p, session, nw.Waits, now); err != nil { + if err := writeSessionWaits(p, store, session, now); err != nil { return nil, fmt.Errorf("the wait was watched, but its file could not be written: %w", err) } _ = writeNodeIndex(p, store, now) @@ -458,7 +506,7 @@ func DoneWait(p Paths, session, by, id, outcome string, store WaitStore, now tim return nil, errors.New("id is required: the one add answered") } key := waitsKey(p, name) - nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + _, err = updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { return nw, err } @@ -472,7 +520,7 @@ func DoneWait(p Paths, session, by, id, outcome string, store WaitStore, now tim if err != nil { return nil, err } - if err := writeSessionFile(p, session, nw.Waits, now); err != nil { + if err := writeSessionWaits(p, store, session, now); err != nil { return nil, fmt.Errorf("the wait was done, but its file could not be written: %w", err) } _ = writeNodeIndex(p, store, now) @@ -496,7 +544,7 @@ func SaidWait(p Paths, session, by, id string, store WaitStore, now time.Time) ( return nil, errors.New("id is required: the one add answered") } key := waitsKey(p, name) - nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { + _, err = updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { return nw, err } @@ -513,14 +561,17 @@ func SaidWait(p Paths, session, by, id string, store WaitStore, now time.Time) ( if err != nil { return nil, err } - if err := writeSessionFile(p, session, nw.Waits, now); err != nil { + if err := writeSessionWaits(p, store, session, now); err != nil { return nil, fmt.Errorf("the wait was said, but its file could not be written: %w", err) } _ = writeNodeIndex(p, store, now) return map[string]any{"id": id, "said": true}, nil } -// ListWaits answers a name's open waits, with their age and whether they are past their bound (1629/AC1). +// ListWaits answers a name's open waits, with their age and whether they are past their bound (1629/AC1), +// and who currently holds the name. Read-only (review round 2, item c): unlike every other op it never +// claims an unbound name, and never refuses a session that is not (or not yet) the one bound to it — a +// session may want to look before adopting. func ListWaits(p Paths, session, by string, store WaitStore, now time.Time) (map[string]any, error) { session, err := requireSessionID(session) if err != nil { @@ -530,15 +581,9 @@ func ListWaits(p Paths, session, by string, store WaitStore, now time.Time) (map if err != nil { return nil, err } - key := waitsKey(p, name) - nw, err := updateNamedWaits(store, key, func(nw NamedWaits) (NamedWaits, error) { - if err := bindCaller(&nw, name, p.Node, by, session, now); err != nil { - return nw, err - } - return nw, nil - }) + nw, _, _, err := readNamedWaits(store, waitsKey(p, name)) if err != nil { - return nil, err + return nil, fmt.Errorf("the wait list could not be read: %w", err) } out := []map[string]any{} for _, w := range nw.Waits { @@ -553,7 +598,11 @@ func ListWaits(p Paths, session, by string, store WaitStore, now time.Time) (map } out = append(out, entry) } - return map[string]any{"session": session, "name": name, "open": out}, nil + answer := map[string]any{"session": session, "name": name, "open": out} + if nw.CurrentSession != "" { + answer["held-by"] = map[string]any{"session": nw.CurrentSession, "last-called": nw.LastCall.UTC().Format(time.RFC3339)} + } + return answer, nil } // AdoptName binds name to session, always taking it over from whoever held it (novox/hq ADR 0490 §3): the @@ -582,16 +631,25 @@ func AdoptName(p Paths, by, session string, store WaitStore, now time.Time) (map } tookOver := oldSession != "" && oldSession != session if tookOver { - if err := writeSessionFile(p, oldSession, nil, now); err != nil { + // oldSession's file is rewritten from the union of whatever names remain bound to it (ADR 0490 §1, + // review round 2 item 2): name no longer is one of them, but another might still be — never simply + // emptied. + if err := writeSessionWaits(p, store, oldSession, now); err != nil { return nil, fmt.Errorf("%s was adopted, but the session it was taken from could not be told: %w", name, err) } } - if err := writeSessionFile(p, session, nw.Waits, now); err != nil { + if err := writeSessionWaits(p, store, session, now); err != nil { return nil, fmt.Errorf("the name was adopted, but its file could not be written: %w", err) } if err := writeNodeIndex(p, store, now); err != nil { say("writing the node-wide wait index after %s was adopted: %v", name, err) } + // Review round 2, item b: a fresh three-strikes count starts under the new binding, on both ids. + waitsDir := WaitsDirPath(p) + _ = writeRefusalCounters(waitsDir, session, map[string]int{}) + if tookOver { + _ = writeRefusalCounters(waitsDir, oldSession, map[string]int{}) + } answer := map[string]any{"name": name, "session": session, "open": len(nw.Waits)} if tookOver { answer["took-over-from"] = map[string]any{"session": oldSession, "last-called": oldLastCall.UTC().Format(time.RFC3339)} diff --git a/modules/claude-code/cmd/claude-code/waits_test.go b/modules/claude-code/cmd/claude-code/waits_test.go index cfcafe26..a86f6f3b 100644 --- a/modules/claude-code/cmd/claude-code/waits_test.go +++ b/modules/claude-code/cmd/claude-code/waits_test.go @@ -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 +} -- 2.54.0