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) + } +}