From 0fa427dad302885bff5afc86d6cdd8c95026a6a7 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 18:08:13 +0200 Subject: [PATCH] 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") {