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 <waits-dir>) 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.
This commit is contained in:
jochen
@@ -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 {
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
package main
|
||||
|
||||
// The plugin's Stop hook (novox/hq ADR 0489 §3-§6, issue 1743): `claude-code stop <waits-dir>`, 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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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") {
|
||||
|
||||
Reference in New Issue
Block a user