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")