From 08cec8cb1cb711c90457ed40efb45feaf0140d1b Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:35:33 +0200 Subject: [PATCH 1/2] Route adopt's refusal-counter reset through fsys, and name the wait in the Stop hook's suggestions (issues 1749, 1757) issue 1749: AdoptName reset both session ids' refusal-counter files with os.WriteFile, bypassing fsys; on a node whose agent has its own account (ADR 0266) the module runs as the operator's account and cannot write into the agent's home directly, so the reset silently failed. writeRefusalCounters now goes through fsys (MkdirAll then WriteFile), as every other write into the agent's home does; it is also called from the Stop hook itself, which already runs as the agent's own account, where fsys is its zero value and behaves exactly as the plain write did before. issue 1757: the Stop hook's suggested watched and said calls left out `by`, which claude_code_wait requires on every op, so a session following the suggestion word for word was refused. Both now carry "by" from the wait's own By field (already kept per wait since issue 1629). The SessionStart hook's suggested adopt call already carried by; added a test for it too, alongside the Stop hook's two, each extracting the exact JSON the hook prints and feeding it back to the tool's own dispatch to prove it is accepted as printed. --- .../cmd/claude-code/hook_calls_test.go | 139 ++++++++++++++++++ modules/claude-code/cmd/claude-code/stop.go | 21 ++- .../claude-code/cmd/claude-code/stop_test.go | 28 ++++ 3 files changed, 181 insertions(+), 7 deletions(-) create mode 100644 modules/claude-code/cmd/claude-code/hook_calls_test.go diff --git a/modules/claude-code/cmd/claude-code/hook_calls_test.go b/modules/claude-code/cmd/claude-code/hook_calls_test.go new file mode 100644 index 00000000..ff3a9b7b --- /dev/null +++ b/modules/claude-code/cmd/claude-code/hook_calls_test.go @@ -0,0 +1,139 @@ +package main + +// Every call a hook prints for a session to copy must be accepted by claude_code_wait's own argument check +// (issue 1757): the Stop hook's suggested watched and said calls, and the SessionStart hook's suggested +// adopt call. + +import ( + "encoding/json" + "strings" + "testing" + "time" +) + +// suggestedCall extracts the one JSON object embedded in a hook's message, right after "claude_code_wait " +// — what a session is expected to copy verbatim — and parses it into the arguments a tool call takes. +func suggestedCall(t *testing.T, msg string) map[string]any { + t.Helper() + i := strings.Index(msg, "claude_code_wait ") + if i < 0 { + t.Fatalf("no suggested call in: %s", msg) + } + rest := msg[i+len("claude_code_wait "):] + start := strings.Index(rest, "{") + if start < 0 { + t.Fatalf("no JSON object in: %s", msg) + } + depth, end := 0, -1 + for j, c := range rest[start:] { + switch c { + case '{': + depth++ + case '}': + depth-- + if depth == 0 { + end = start + j + 1 + } + } + if end >= 0 { + break + } + } + if end < 0 { + t.Fatalf("unbalanced JSON object in: %s", msg) + } + var args map[string]any + if err := json.Unmarshal([]byte(rest[start:end]), &args); err != nil { + t.Fatalf("the suggested call is not valid JSON: %v (%s)", err, rest[start:end]) + } + return args +} + +// Issue 1757: the Stop hook's suggested watched call carries `by`, and the tool accepts it as printed. +func TestTheStopHooksSuggestedWatchedCallIsAcceptedByTheTool(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": "verify-0489-test", "by": "fix-1757@g14", + "what": "the operator's Approve", "read": map[string]any{"address": "mesh-delivery.recheck"}, "bound_minutes": 10}) + if isErr { + t.Fatalf("add: %s", text) + } + + f, err := readWaitsFile(waitsFilePath(p, "verify-0489-test")) + if err != nil || len(f.Waits) != 1 { + t.Fatalf("%+v %v", f, err) + } + _, reason := waitBlocksStop("verify-0489-test", f.Waits[0], t0.Add(time.Minute)) + if !strings.Contains(reason, "\"by\":\"fix-1757\"") { + t.Fatalf("the suggested call does not carry by: %s", reason) + } + args := suggestedCall(t, reason) + + text, isErr = s.call(t, "claude_code_wait", args) + if isErr { + t.Fatalf("the suggested watched call, as printed, was refused: %s", text) + } +} + +// Issue 1757: the Stop hook's suggested said call (past its bound) carries `by` too. +func TestTheStopHooksSuggestedSaidCallIsAcceptedByTheTool(t *testing.T) { + p, _ := node(t, "g14") + store := newMemWaits() + clock := t0 + waiting = &Waiting{Store: store, Now: func() time.Time { return clock }} + 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": "verify-0489-test", "by": "fix-1757@g14", + "what": "the operator's Approve", "read": map[string]any{"address": "mesh-delivery.recheck"}, "bound_minutes": 1}) + if isErr { + t.Fatalf("add: %s", text) + } + + clock = t0.Add(2 * time.Minute) // past its 1-minute bound, for the live tool call too + f, err := readWaitsFile(waitsFilePath(p, "verify-0489-test")) + if err != nil || len(f.Waits) != 1 { + t.Fatalf("%+v %v", f, err) + } + _, reason := waitBlocksStop("verify-0489-test", f.Waits[0], clock) + if !strings.Contains(reason, "\"by\":\"fix-1757\"") { + t.Fatalf("the suggested call does not carry by: %s", reason) + } + args := suggestedCall(t, reason) + + text, isErr = s.call(t, "claude_code_wait", args) + if isErr { + t.Fatalf("the suggested said call, as printed, was refused: %s", text) + } +} + +// Issue 1757: the SessionStart hook's suggested adopt call already carries `by` — checked, as asked, so a +// regression here is caught the same way as the Stop hook's. +func TestTheSessionStartHooksSuggestedAdoptCallIsAcceptedByTheTool(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-1757@g14", + "what": "the operator's Approve", "read": map[string]any{"address": "mesh-delivery.recheck"}, "bound_minutes": 10}) + if isErr { + t.Fatalf("add: %s", text) + } + + msg := SessionStartMessage("sess-new", WaitsDirPath(p), t0.Add(time.Minute)) + if !strings.Contains(msg, "\"by\":\"fix-1757@g14\"") { + t.Fatalf("the suggested adopt call does not carry by: %s", msg) + } + args := suggestedCall(t, msg) + + text, isErr = s.call(t, "claude_code_wait", args) + if isErr { + t.Fatalf("the suggested adopt call, as printed, was refused: %s", text) + } +} diff --git a/modules/claude-code/cmd/claude-code/stop.go b/modules/claude-code/cmd/claude-code/stop.go index 774b9e9a..260dd9a1 100644 --- a/modules/claude-code/cmd/claude-code/stop.go +++ b/modules/claude-code/cmd/claude-code/stop.go @@ -52,12 +52,12 @@ func waitBlocksStop(session string, w Wait, now time.Time) (bool, string) { } return true, fmt.Sprintf("wait %s (%s) is open and nothing follows it yet. Read where it stands with %s, then "+ "%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) + "{\"op\":\"watched\",\"session\":%q,\"by\":%q,\"id\":%q,\"watcher\":\"...\"}.", w.ID, w.What, w.Read.Address, + watcherAdvice(w), session, w.By, 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) + "{\"op\":\"said\",\"session\":%q,\"by\":%q,\"id\":%q}.", w.ID, w.What, session, w.By, w.ID) } // watcherAdvice is what a session is told to do to follow a wait (ADR 0489 §3, review round 1 item 3): a @@ -113,15 +113,22 @@ func readRefusalCounters(waitsDir, session string) map[string]int { return m } -// 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). +// writeRefusalCounters writes the counter file beside the waits file, through fsys like every other write +// into the agent's home (novox/hq ADR 0266, issue 1749): called from the Stop hook, which already runs as +// the agent's own account (fsys is then its zero value, a plain write, as before), and from the tool's +// AdoptName, which runs as the operator's account and must reach a confined node's agent home through the +// sudo helper fsys carries such a write through. 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 err } - return os.WriteFile(refusalCountersPath(waitsDir, session), raw, 0o600) + if err := fsys.MkdirAll(waitsDir, 0o700); err != nil { + return err + } + return fsys.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 diff --git a/modules/claude-code/cmd/claude-code/stop_test.go b/modules/claude-code/cmd/claude-code/stop_test.go index c975cf30..a8f49cc6 100644 --- a/modules/claude-code/cmd/claude-code/stop_test.go +++ b/modules/claude-code/cmd/claude-code/stop_test.go @@ -273,3 +273,31 @@ func TestHookRegisterStillRefusesEveryHook(t *testing.T) { t.Fatalf("a Stop hook was not refused: %v %s", isError, text) } } + +// Issue 1749: adopt's refusal-counter reset writes into the agent's home (beside the waits file) and must +// go through fsys like every other such write (novox/hq ADR 0266) — on a confined node the module runs as +// the operator's account, which cannot write there directly; only the helper, as the agent's own account, +// can. The confined-node fake (agentFiles, agent_account_test.go) proves it: a write that reaches the real +// filesystem only through the helper's request/response protocol bumps its count. +func TestWriteRefusalCountersGoesThroughTheHelper(t *testing.T) { + f, home, counted := agentFiles(t) + was := fsys + fsys = f + t.Cleanup(func() { fsys = was }) + + waitsDir := filepath.Join(home, ".claude", "nox-mesh", "waits") + if err := writeRefusalCounters(waitsDir, "sess-1", map[string]int{"w-1": 2}); err != nil { + t.Fatal(err) + } + if counted.asked.Load() == 0 { + t.Fatal("the counter file was written directly, never through the helper") + } + raw, err := os.ReadFile(filepath.Join(waitsDir, "sess-1.refusals.json")) + if err != nil { + t.Fatal(err) + } + var m map[string]int + if json.Unmarshal(raw, &m) != nil || m["w-1"] != 2 { + t.Fatalf("the written counters: %s", raw) + } +} From e3a714d5f0f04ed214d56fc6c089cec293ebcf1c Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:41:05 +0200 Subject: [PATCH 2/2] Stop writing an empty refusal-counter file on every stop of every session (issue 1749) writeRefusalCounters wrote "{}" and, since it now routes through fsys (this branch's earlier commit), created the waits folder on every single stop, even for a session that never had a wait: one counter file per session id, piling up on a node that uses the wait list at all. It now removes the file instead of writing empty counters, a no-op when there was never one to begin with, so a stop with nothing to record leaves neither the file nor the folder; a refusal still writes it, and the stop right after the wait is done (or let through) removes it again. --- modules/claude-code/cmd/claude-code/stop.go | 28 +++++--- .../claude-code/cmd/claude-code/stop_test.go | 65 ++++++++++++++++++- 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/modules/claude-code/cmd/claude-code/stop.go b/modules/claude-code/cmd/claude-code/stop.go index 260dd9a1..d1328c77 100644 --- a/modules/claude-code/cmd/claude-code/stop.go +++ b/modules/claude-code/cmd/claude-code/stop.go @@ -113,14 +113,26 @@ func readRefusalCounters(waitsDir, session string) map[string]int { return m } -// writeRefusalCounters writes the counter file beside the waits file, through fsys like every other write -// into the agent's home (novox/hq ADR 0266, issue 1749): called from the Stop hook, which already runs as -// the agent's own account (fsys is then its zero value, a plain write, as before), and from the tool's -// AdoptName, which runs as the operator's account and must reach a confined node's agent home through the -// sudo helper fsys carries such a write through. 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). +// writeRefusalCounters keeps the counter file beside the waits file in step with m, through fsys like every +// other write into the agent's home (novox/hq ADR 0266, issue 1749): called from the Stop hook, which +// already runs as the agent's own account (fsys is then its zero value, a plain write or remove, as +// before), and from the tool's AdoptName, which runs as the operator's account and must reach a confined +// node's agent home through the sudo helper fsys carries such a write through. 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). +// +// Empty, m is removed rather than written as "{}" (issue 1749, follow-up): a stop with nothing to record — +// every stop of every session with no waits, the ordinary case — must leave neither the file nor, the first +// time, the waits folder itself; removing a file that was never there is not an error. func writeRefusalCounters(waitsDir, session string, m map[string]int) error { + path := refusalCountersPath(waitsDir, session) + if len(m) == 0 { + if err := fsys.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) { + return err + } + return nil + } raw, err := json.Marshal(m) if err != nil { return err @@ -128,7 +140,7 @@ func writeRefusalCounters(waitsDir, session string, m map[string]int) error { if err := fsys.MkdirAll(waitsDir, 0o700); err != nil { return err } - return fsys.WriteFile(refusalCountersPath(waitsDir, session), raw, 0o600) + return fsys.WriteFile(path, raw, 0o600) } // notice says a fail-open or let-through reason on both the log (stderr) and the screen: Claude Code shows a diff --git a/modules/claude-code/cmd/claude-code/stop_test.go b/modules/claude-code/cmd/claude-code/stop_test.go index a8f49cc6..71501a56 100644 --- a/modules/claude-code/cmd/claude-code/stop_test.go +++ b/modules/claude-code/cmd/claude-code/stop_test.go @@ -170,13 +170,72 @@ func TestRunStopKeepsItsCounterBesideTheWaitsFile(t *testing.T) { 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")) + // Once let through, the counters are empty again: the file is removed, not left holding "{}" (issue + // 1749, follow-up). + if _, err := os.Stat(refusalCountersPath(waitsDir, "sess-6")); !os.IsNotExist(err) { + t.Fatalf("the counter file was not removed once its counters emptied: %v", err) + } +} + +// Issue 1749, follow-up: a stop for a session with no waits at all leaves nothing behind — not the counter +// file, not even the waits folder a node that never added a wait would otherwise gain on every stop of +// every session. +func TestStopWithNoWaitsLeavesNoFolderAndNoFile(t *testing.T) { + waitsDir := filepath.Join(t.TempDir(), "waits") // not created: this is what the hook is handed cold + var stdout, stderr bytes.Buffer + stdin := strings.NewReader(`{"session_id":"sess-none","stop_hook_active":false}`) + if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 0 { + t.Fatalf("code %d, stderr %s", code, stderr.String()) + } + if _, err := os.Stat(waitsDir); !os.IsNotExist(err) { + t.Fatalf("a stop with no waits created the waits folder: %v", err) + } +} + +// Issue 1749, follow-up: a refusal writes the counter file (there is something to record). +func TestStopWithARefusalWritesTheCounterFile(t *testing.T) { + waitsDir := t.TempDir() + f := WaitsFile{Session: "sess-7", At: stopT0, Waits: []Wait{openWait("w-7", 10)}} + writeWaitsFixture(t, filepath.Join(waitsDir, "sess-7.json"), f) + var stdout, stderr bytes.Buffer + stdin := strings.NewReader(`{"session_id":"sess-7","stop_hook_active":false}`) + if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 2 { + t.Fatalf("code %d, stderr %s", code, stderr.String()) + } + raw, err := os.ReadFile(refusalCountersPath(waitsDir, "sess-7")) 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) + if json.Unmarshal(raw, &counters) != nil || counters["w-7"] != 1 { + t.Fatalf("the counter file: %s", raw) + } +} + +// Issue 1749, follow-up: once the wait is done (no longer in the waits file), the next stop removes the +// counter file rather than leaving a stale "{}" behind. +func TestStopAfterTheWaitIsDoneRemovesTheCounterFile(t *testing.T) { + waitsDir := t.TempDir() + path := filepath.Join(waitsDir, "sess-8.json") + writeWaitsFixture(t, path, WaitsFile{Session: "sess-8", At: stopT0, Waits: []Wait{openWait("w-8", 10)}}) + stdin := strings.NewReader(`{"session_id":"sess-8","stop_hook_active":false}`) + var stdout, stderr bytes.Buffer + if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 2 { + t.Fatalf("refusal: code %d, stderr %s", code, stderr.String()) + } + if _, err := os.Stat(refusalCountersPath(waitsDir, "sess-8")); err != nil { + t.Fatalf("the counter file was not written: %v", err) + } + + // The wait is done: it is no longer in the waits file (DoneWait removes it, waits.go). + writeWaitsFixture(t, path, WaitsFile{Session: "sess-8", At: stopT0, Waits: []Wait{}}) + var stdout2, stderr2 bytes.Buffer + stdin2 := strings.NewReader(`{"session_id":"sess-8","stop_hook_active":false}`) + if code := runStop(waitsDir, stdin2, &stdout2, &stderr2); code != 0 { + t.Fatalf("after done: code %d, stderr %s", code, stderr2.String()) + } + if _, err := os.Stat(refusalCountersPath(waitsDir, "sess-8")); !os.IsNotExist(err) { + t.Fatalf("the counter file was not removed after the wait was done: %v", err) } }