diff --git a/.abcd/development/brief/04-surfaces/01-ahoy.md b/.abcd/development/brief/04-surfaces/01-ahoy.md index 260070db..bdeb5120 100644 --- a/.abcd/development/brief/04-surfaces/01-ahoy.md +++ b/.abcd/development/brief/04-surfaces/01-ahoy.md @@ -164,8 +164,10 @@ Steps, run in parallel where independent: `hooks/hooks.json` is present in the plugin install AND contains the three required event entries (`UserPromptSubmit`, `SessionStart`, `PreCompact`) each referencing the expected prompt-router hook commands. The shipped - manifest also wires `abcd hook session-start` (a second `SessionStart` - command), `abcd hook session-end` (a `SessionEnd` event), and `abcd guard + manifest also wires `abcd hook session-start` (chained after the bootstrap + and ahead of `prompt-router-reset` inside the ONE `SessionStart` command — + the harness runs sibling hooks in parallel, so the event carries a single + entry), `abcd hook session-end` (a `SessionEnd` event), and `abcd guard hook` (a `PreToolUse` event, matcher `Bash`, that checks a shell command against the hazard registry before it runs) — five event types in all; verification covers only the three prompt-router commands above. A missing or diff --git a/.abcd/development/brief/05-internals/03-configuration.md b/.abcd/development/brief/05-internals/03-configuration.md index b55e47ea..00d9fb8a 100644 --- a/.abcd/development/brief/05-internals/03-configuration.md +++ b/.abcd/development/brief/05-internals/03-configuration.md @@ -356,7 +356,9 @@ abcd/ │ ├── release-changelog-composer.md / ruthless-reviewer.md / security-reviewer.md │ └── sota-researcher.md # plus per-agent fixtures/ dirs, README.md, CHANGELOG.md └── hooks/ # Claude Code event hooks — each command shells directly to the binary - └── hooks.json # UserPromptSubmit → hook prompt-router; SessionStart → prompt-router-reset + session-start; + └── hooks.json # UserPromptSubmit → hook prompt-router; SessionStart → ONE chained command: + # bootstrap.sh, then session-start + prompt-router-reset, each fed a copy of the + # payload (siblings would run in parallel and share one stdin); # PreToolUse (matcher Bash) → guard hook; PreCompact → prompt-router-reset; SessionEnd → session-end ``` diff --git a/.abcd/development/specs/closed/spc-21-a-fresh-plugin-install-just-works-the-hook-binary-survives.md b/.abcd/development/specs/closed/spc-21-a-fresh-plugin-install-just-works-the-hook-binary-survives.md index 0dfa9ecc..842386fc 100644 --- a/.abcd/development/specs/closed/spc-21-a-fresh-plugin-install-just-works-the-hook-binary-survives.md +++ b/.abcd/development/specs/closed/spc-21-a-fresh-plugin-install-just-works-the-hook-binary-survives.md @@ -18,9 +18,10 @@ plus wiring plus reporting: no `internal/core` behaviour changes. - **`hooks/bootstrap.sh`** (new, committed): the self-provisioning script. Needs no abcd binary to run — the binary is exactly what is missing. -- **`hooks/hooks.json`**: the bootstrap becomes the first `SessionStart` - entry, ahead of the binary-backed `prompt-router-reset` / `session-start` - commands. +- **`hooks/hooks.json`**: the `SessionStart` event is ONE entry whose command + runs the bootstrap and then the binary-backed `prompt-router-reset` / + `session-start` calls in the same shell (see Hook wiring, and the correction + recorded there). - **`$CLAUDE_PLUGIN_ROOT/.binary-meta`** (new, written by the bootstrap): one small key=value file recording `release_tag`, `release_sha` (when resolvable), `fetched_at`, and `plugin_sha` — the input for skew reporting. @@ -85,15 +86,41 @@ Behaviour, in order: ### Hook wiring -`hooks.json` gains, as the first `SessionStart` hook: +`hooks.json` declares ONE `SessionStart` entry, whose command runs the +bootstrap and then the two binary-backed calls in a single shell: ```json -{"type": "command", "command": "\"$CLAUDE_PLUGIN_ROOT/hooks/bootstrap.sh\""} +{"type": "command", "timeout": 240, "command": "… bootstrap.sh …; i=$(cat); … abcd hook session-start; … abcd hook prompt-router-reset"} ``` -Ordering within one event's hook list is preserved by the harness, so the -binary-backed session hooks run after the bootstrap in the same event. The -`UserPromptSubmit`/`PreToolUse`/`PreCompact`/`SessionEnd` commands are +Two properties of that chain are load-bearing and easy to lose. **The payload is +read once and piped to each call separately.** Every hook verb takes its input +with `io.ReadAll` over the whole of stdin (`readHookInput`), so two calls sharing +one stdin leave the second reading EOF — `hook session-start` would fail to +unmarshal and take its silent `return nil` path, disabling both of its notices +in every session. **`session-start` runs before `prompt-router-reset`**, because +the reset ends with an unconditional "abcd rules: reset session" diagnostic on +stderr, and whichever call runs first owns the only line the transcript renders. +The exit precedence (bootstrap, then `session-start`, then the reset) is +computed from saved status codes and does not depend on that order. + +**Correction (2026-08-12, iss-204 / iss-208).** This section wired the +bootstrap as the first of THREE sibling `SessionStart` entries on the warrant +that *"ordering within one event's hook list is preserved by the harness, so +the binary-backed session hooks run after the bootstrap in the same event"*. +That warrant is **false**, and it was load-bearing: the harness runs every hook +matching an event **in parallel** (the hooks reference says so verbatim). Both +gated entries raced the ~10.7 MB download, lost, printed "the plugin binary is +not installed", and genuinely did not run — on every fresh install and every +plugin update, since an update lands in a fresh commit-stamped cache directory +with no binary. The three entries are collapsed into the one command above, so +the sequencing is OWNED by the manifest rather than assumed of the harness; the +two gated messages become one; and the bootstrap's own message is emitted +first, so the single line the transcript renders is the success rather than a +missing-binary complaint (iss-208). Nothing else in this spec rests on hook +ordering. Do not restore the ordering warrant. + +The `UserPromptSubmit`/`PreToolUse`/`PreCompact`/`SessionEnd` commands are unchanged — on the first-ever event before any `SessionStart` completed they fail as today (fail-open guard with UNGUARDED warning, per the intent's guard-window decision). diff --git a/.abcd/work/issues/open/iss-204-hooks-hooks-json-puts-bootstrap-sh-and-the-two-binary-backed.md b/.abcd/work/issues/resolved/iss-204-hooks-hooks-json-puts-bootstrap-sh-and-the-two-binary-backed.md similarity index 84% rename from .abcd/work/issues/open/iss-204-hooks-hooks-json-puts-bootstrap-sh-and-the-two-binary-backed.md rename to .abcd/work/issues/resolved/iss-204-hooks-hooks-json-puts-bootstrap-sh-and-the-two-binary-backed.md index ee375598..35a7a47a 100644 --- a/.abcd/work/issues/open/iss-204-hooks-hooks-json-puts-bootstrap-sh-and-the-two-binary-backed.md +++ b/.abcd/work/issues/resolved/iss-204-hooks-hooks-json-puts-bootstrap-sh-and-the-two-binary-backed.md @@ -7,6 +7,8 @@ category: "bug" source: "user-observation" found_during: "first manual plugin install test (2026-08-10)" found_at: "hooks/hooks.json" +resolution: "Collapsed the three SessionStart entries into one chained command (bootstrap, then prompt-router-reset and session-start in the same shell), so sequencing is owned by hooks.json rather than assumed of a harness that runs sibling hooks in parallel. spc-21's false ordering warrant corrected in place." +impact: fix --- hooks/hooks.json puts bootstrap.sh and the two binary-backed SessionStart hooks in ONE event group and relies on list order, but the harness runs all matching hooks in PARALLEL — the Claude Code hooks reference states verbatim 'All matching hooks run in parallel.' spc-21 asserts the opposite: 'Ordering within one event's hook list is preserved by the harness, so the binary-backed session hooks run after the bootstrap in the same event.' That warrant is false, and it is load-bearing. On a fresh install the two gated hooks evaluate [ -f "$CLAUDE_PLUGIN_ROOT/abcd" ] while bootstrap.sh is still downloading the ~10.7MB release binary (timeout 240), lose the race, and both print 'the plugin binary is not installed'. Observed on the first manual marketplace install (2026-08-10): both errors printed BEFORE bootstrap's own success notice, which is direct evidence of parallel execution. Consequence is not only noise — 'hook prompt-router-reset' and 'hook session-start' genuinely do not run for that session, and itd-105 AC#1 ('every hook executes successfully — no No such file or directory, no unguarded-shell warning') fails on every fresh install AND every plugin update, since each update lands in a fresh commit-stamped cache directory with no binary. Fix direction: collapse the three entries into ONE SessionStart command that runs bootstrap.sh and then the two binary calls in a single shell, so the sequencing is owned here rather than assumed of the harness. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-208-a-successful-bootstrap-install-is-rendered-to-the-user-as-a.md b/.abcd/work/issues/resolved/iss-208-a-successful-bootstrap-install-is-rendered-to-the-user-as-a.md similarity index 81% rename from .abcd/work/issues/open/iss-208-a-successful-bootstrap-install-is-rendered-to-the-user-as-a.md rename to .abcd/work/issues/resolved/iss-208-a-successful-bootstrap-install-is-rendered-to-the-user-as-a.md index 4ed46928..44e9c520 100644 --- a/.abcd/work/issues/open/iss-208-a-successful-bootstrap-install-is-rendered-to-the-user-as-a.md +++ b/.abcd/work/issues/resolved/iss-208-a-successful-bootstrap-install-is-rendered-to-the-user-as-a.md @@ -8,6 +8,8 @@ source: "user-observation" found_during: "first manual plugin install test (2026-08-10)" found_at: "hooks/bootstrap.sh" blocked_by: [iss-204] +resolution: "The chained SessionStart command emits the bootstrap's own message first, so the single line the transcript renders on a fresh install is the checksum-verified success rather than a missing-binary complaint. The non-zero exit stays: it is the only channel that puts stderr in front of the human." +impact: fix --- A successful bootstrap install is rendered to the user as a third 'SessionStart:startup hook error', indistinguishable from the two genuine failures above it. hooks/bootstrap.sh's notice() exits 2 deliberately, and its comment gives the sound reason: a SessionStart hook's stdout becomes model context, while only a non-zero exit puts stderr in front of the human. But the harness renders ANY non-zero SessionStart exit as a ' hook error' notice — the docs confirm exit 2 'renders in the transcript as a hook error notice, the same way a non-blocking error does'. So there is no 'notice' channel distinct from 'error', and the checksum-verified happy path is labelled a fault. Observed on the first manual install (2026-08-10): three consecutive hook-error lines, of which the third was the install SUCCEEDING. Fix direction: fold this into the single chained SessionStart entry proposed in iss-204 and lead the visible first line with the success, since the transcript shows only the first line of stderr; the honest-failure posture for the genuinely-no-binary window is unaffected. \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 77ab7f0c..32bfcd18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,34 @@ called out in a **Breaking** section. ### Fixed +- **The session-start hooks run after the bootstrap that provisions their binary, + and a successful install reads as success** (iss-204, iss-208). The hook + manifest listed the bootstrap and the two binary-backed commands as three + sibling `SessionStart` entries and relied on list order; the harness runs every + hook matching an event in parallel, so both gated entries raced a ~10.7 MB + download, lost, printed "the plugin binary is not installed", and genuinely did + not run — on every fresh install and every plugin update, since an update lands + in a fresh cache directory with no binary. The three entries are now ONE + command that runs the bootstrap and then both binary calls in a single shell, + so the sequencing is owned by the manifest rather than assumed of the harness. + Chaining them makes two further properties load-bearing, and both are held + explicitly: the hook payload is read once and piped to each call separately, + because every hook verb consumes the whole of stdin and a shared stdin would + leave `session-start` reading EOF and silently disabling its notices; and + `session-start` runs ahead of `prompt-router-reset`, whose unconditional + success diagnostic would otherwise be the one line the transcript renders. + The bootstrap's own message is emitted first, which is what the transcript + renders: on a fresh install the visible line is the checksum-verified success + rather than one of two missing-binary complaints, and the two complaints + collapse into one. The honest-failure posture is unchanged — a refusal keeps + its message and its exit code, a binary that is genuinely absent is still said + out loud, and the binary calls' stdout still reaches the model untouched. The + spec that shipped the bootstrap carried the false warrant ("ordering within one + event's hook list is preserved by the harness") as a load-bearing claim; it is + corrected in place, with the brief's two descriptions of the manifest. Parallel + hook execution and the plugin cache are not present in CI, so the end-to-end + proof is the manual install gate; what CI holds is the manifest's shape and the + chained command's behaviour against fixtures. - **`ahoy install` prompts read a piped answer, in a fixed order, and `--yes` says what it does not cover** (iss-167, iss-166). The prompter attached to stdin only when stdin was a terminal, so `yes | abcd ahoy install` — the first thing an diff --git a/hooks/hooks.json b/hooks/hooks.json index 59e1a531..45f6b83d 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -5,9 +5,7 @@ ], "SessionStart": [ {"hooks": [ - {"type": "command", "timeout": 240, "command": "\"$CLAUDE_PLUGIN_ROOT/hooks/bootstrap.sh\""}, - {"type": "command", "command": "if [ -f \"$CLAUDE_PLUGIN_ROOT/abcd\" ] && [ -x \"$CLAUDE_PLUGIN_ROOT/abcd\" ]; then \"$CLAUDE_PLUGIN_ROOT/abcd\" hook prompt-router-reset; else echo \"abcd: the plugin binary is not installed, so the rule loader did not reset for this session — hooks/bootstrap.sh provisions it at session start; start a session with network access, or install per https://github.com/REPPL/abcd-cli#install\" >&2; exit 2; fi"}, - {"type": "command", "command": "if [ -f \"$CLAUDE_PLUGIN_ROOT/abcd\" ] && [ -x \"$CLAUDE_PLUGIN_ROOT/abcd\" ]; then \"$CLAUDE_PLUGIN_ROOT/abcd\" hook session-start; else echo \"abcd: the plugin binary is not installed, so the session-start checks did not run — hooks/bootstrap.sh provisions it at session start; start a session with network access, or install per https://github.com/REPPL/abcd-cli#install\" >&2; exit 2; fi"} + {"type": "command", "timeout": 240, "command": "[ -n \"${CLAUDE_PLUGIN_ROOT:-}\" ] || exit 0; b=''; s=0; if [ -x \"$CLAUDE_PLUGIN_ROOT/hooks/bootstrap.sh\" ]; then b=$(\"$CLAUDE_PLUGIN_ROOT/hooks/bootstrap.sh\" 2>&1 >/dev/null &2; if [ -f \"$CLAUDE_PLUGIN_ROOT/abcd\" ] && [ -x \"$CLAUDE_PLUGIN_ROOT/abcd\" ]; then i=$(cat); printf '%s' \"$i\" | \"$CLAUDE_PLUGIN_ROOT/abcd\" hook session-start; e=$?; printf '%s' \"$i\" | \"$CLAUDE_PLUGIN_ROOT/abcd\" hook prompt-router-reset; p=$?; [ \"$s\" -eq 0 ] || exit \"$s\"; [ \"$e\" -eq 0 ] || exit \"$e\"; exit \"$p\"; fi; [ -n \"$b\" ] || printf '%s\\n' \"abcd: the plugin binary is not installed, so the rule loader did not reset and the session-start checks did not run — hooks/bootstrap.sh provisions it at session start; start a session with network access, or install per https://github.com/REPPL/abcd-cli#install\" >&2; [ \"$s\" -eq 0 ] || exit \"$s\"; exit 2"} ]} ], "PreToolUse": [ diff --git a/internal/surface/cli/hooks_sessionstart_test.go b/internal/surface/cli/hooks_sessionstart_test.go new file mode 100644 index 00000000..9ba19fa7 --- /dev/null +++ b/internal/surface/cli/hooks_sessionstart_test.go @@ -0,0 +1,344 @@ +package cli + +import ( + "bytes" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// The SessionStart wiring these tests pin exists because the harness runs every +// hook matching an event IN PARALLEL (iss-204). The previous manifest listed the +// bootstrap and the two binary-backed commands as three sibling entries of one +// event and relied on list order; spc-21 asserted that ordering was preserved, +// which is false. Both gated entries lost the race against a ~10.7 MB download, +// printed "the plugin binary is not installed", and genuinely did not run — on +// every fresh install and every plugin update, since an update lands in a fresh +// cache directory with no binary. +// +// So sequencing is owned by ONE command string: bootstrap first, then the two +// binary calls in the same shell. These tests assert that shape and the +// behaviour that only the collapsed form can have. + +// sessionStartHooks is the decoded shape of the events these tests read. Only +// what is asserted is modelled; anything else in the manifest is ignored. +type sessionStartHooks struct { + Hooks map[string][]struct { + Hooks []struct { + Type string `json:"type"` + Timeout int `json:"timeout"` + Command string `json:"command"` + } `json:"hooks"` + } `json:"hooks"` +} + +// hooksManifest locates the committed hooks/hooks.json from this test file's own +// on-disk position, the same way bootstrapScript locates the shipped script — so +// the assertions below derive from the manifest that actually ships. +func hooksManifest(t *testing.T) string { + t.Helper() + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed to locate the test source file") + } + return filepath.Clean(filepath.Join(filepath.Dir(file), "..", "..", "..", "hooks", "hooks.json")) +} + +// sessionStartCommand returns the single SessionStart command string, failing if +// the event carries anything other than exactly one entry holding exactly one +// command. The count IS the fix: a second sibling entry is a hook the harness +// would run in parallel with the bootstrap again. +func sessionStartCommand(t *testing.T) string { + t.Helper() + data, err := os.ReadFile(hooksManifest(t)) + if err != nil { + t.Fatalf("reading the committed hooks manifest: %v", err) + } + var doc sessionStartHooks + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatalf("hooks/hooks.json does not parse: %v", err) + } + entries := doc.Hooks["SessionStart"] + if len(entries) != 1 { + t.Fatalf("SessionStart must declare exactly one entry group (the harness runs matching hooks in parallel), found %d", len(entries)) + } + if len(entries[0].Hooks) != 1 { + t.Fatalf("the SessionStart entry group must hold exactly one command (siblings race each other), found %d", len(entries[0].Hooks)) + } + return entries[0].Hooks[0].Command +} + +// sessionStartRun executes the shipped SessionStart command under `sh -c` with a +// constructed environment and stdin, returning stdout, stderr and the exit code +// SEPARATELY — the split is the point. A SessionStart hook's stdout becomes model +// context while only a non-zero exit puts its stderr in front of the human, and +// the transcript shows only the FIRST line of that stderr (iss-208). +// +// stdin is never left nil: the harness delivers the hook payload there, and a +// command that reads it must be exercised with it. +func sessionStartRun(t *testing.T, root, stdin string, extraEnv ...string) (string, string, int) { + t.Helper() + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh unavailable") + } + cmd := exec.Command("sh", "-c", sessionStartCommand(t)) + cmd.Stdin = strings.NewReader(stdin) + cmd.Env = append([]string{ + "PATH=" + os.Getenv("PATH"), + "HOME=" + t.TempDir(), + "CLAUDE_PLUGIN_ROOT=" + root, + }, extraEnv...) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + code := 0 + if err := cmd.Run(); err != nil { + e, ok := err.(*exec.ExitError) + if !ok { + t.Fatalf("running the SessionStart command: %v (stderr %s)", err, stderr.String()) + } + code = e.ExitCode() + } + return stdout.String(), stderr.String(), code +} + +// sessionStartRoot builds a plugin root holding a stub hooks/bootstrap.sh, and +// (when body is non-empty) a stub abcd that appends every invocation to a call +// log. The stubs stand in for the real script and binary so the WIRING is what +// is under test; hooks/bootstrap.sh's own behaviour is covered by bootstrap_test. +func sessionStartRoot(t *testing.T, bootstrap, binary string) (root, calls string) { + t.Helper() + root = t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "hooks"), 0o755); err != nil { + t.Fatal(err) + } + calls = filepath.Join(root, "calls.log") + if err := os.WriteFile(filepath.Join(root, "hooks", "bootstrap.sh"), []byte(bootstrap), 0o755); err != nil { + t.Fatal(err) + } + if binary != "" { + if err := os.WriteFile(filepath.Join(root, "abcd"), []byte(binary), 0o755); err != nil { + t.Fatal(err) + } + } + return root, calls +} + +// stubBinary stands in for the real binary, and it models the three things +// about it the chained command has to respect: +// +// - every hook verb CONSUMES stdin whole (readHookInput is io.ReadAll over a +// LimitReader), so two calls sharing one stdin leave the second reading EOF; +// - `hook prompt-router-reset` writes an UNCONDITIONAL success diagnostic to +// stderr, so whichever call runs first owns the line the transcript renders; +// - `hook session-start` exits 2 when it has a notice, which is the only way +// its warning reaches the human. +const stubBinary = `#!/bin/sh +in=$(cat) +printf '%s %s stdin=[%s]\n' "$1" "$2" "$in" >> "$ABCD_CALLS" +printf 'stdout from %s\n' "$2" +if [ "$2" = "prompt-router-reset" ]; then + printf 'abcd rules: reset session ("SessionStart")\n' >&2 + exit 0 +fi +printf 'abcd: a session-start notice\n' >&2 +exit 2 +` + +// sessionStartPayload is a harness SessionStart payload of the shape +// readHookInput unmarshals. Every chained call must receive it in full. +const sessionStartPayload = `{"session_id":"s1","hook_event_name":"SessionStart","source":"startup","cwd":"/tmp"}` + +// callLog returns the recorded invocations, or "" when nothing ran. +func callLog(t *testing.T, path string) string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return "" + } + t.Fatal(err) + } + return string(data) +} + +// firstLine is what the transcript renders of a hook's stderr. +func firstLine(s string) string { + s = strings.TrimLeft(s, "\n") + if i := strings.IndexByte(s, '\n'); i >= 0 { + return s[:i] + } + return s +} + +// TestSessionStartIsOneChainedCommand pins the collapse itself: one entry, one +// command, naming the bootstrap and both binary calls, and parsing as POSIX sh. +func TestSessionStartIsOneChainedCommand(t *testing.T) { + command := sessionStartCommand(t) + for _, want := range []string{"hooks/bootstrap.sh", "hook prompt-router-reset", "hook session-start"} { + if !strings.Contains(command, want) { + t.Errorf("the single SessionStart command must run %q; command = %q", want, command) + } + } + if strings.Index(command, "hooks/bootstrap.sh") > strings.Index(command, "hook prompt-router-reset") { + t.Error("the bootstrap must be invoked before the binary-backed calls in the chained command") + } + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh unavailable") + } + script := filepath.Join(t.TempDir(), "session-start.sh") + if err := os.WriteFile(script, []byte(command+"\n"), 0o644); err != nil { + t.Fatal(err) + } + if out, err := exec.Command("sh", "-n", script).CombinedOutput(); err != nil { + t.Fatalf("the SessionStart command is not valid POSIX sh: %v (%s)", err, out) + } +} + +// TestSessionStartLeadsWithTheBootstrapSuccess is iss-208's fix and iss-204's +// together: on a fresh install the bootstrap's success is the FIRST stderr line +// (the only one the transcript renders), and both binary-backed calls still run +// — against the binary the bootstrap has just installed. +func TestSessionStartLeadsWithTheBootstrapSuccess(t *testing.T) { + root, calls := sessionStartRoot(t, `#!/bin/sh +cp "$CLAUDE_PLUGIN_ROOT/staged-abcd" "$CLAUDE_PLUGIN_ROOT/abcd" +chmod 0755 "$CLAUDE_PLUGIN_ROOT/abcd" +printf 'abcd bootstrap: installed the checksum-verified abcd binary\n' >&2 +exit 2 +`, "") + // Staged beside the root rather than written by the stub: the binary the + // bootstrap installs is what the chained calls must then find. + if err := os.WriteFile(filepath.Join(root, "staged-abcd"), []byte(stubBinary), 0o755); err != nil { + t.Fatal(err) + } + stdout, stderr, code := sessionStartRun(t, root, sessionStartPayload, "ABCD_CALLS="+calls) + + if got := firstLine(stderr); !strings.HasPrefix(got, "abcd bootstrap: installed") { + t.Errorf("the first stderr line must be the bootstrap's success (it is the only line the transcript shows); got %q\nfull stderr:\n%s", got, stderr) + } + if code == 0 { + t.Error("the exit must stay non-zero: only a non-zero SessionStart exit puts stderr in front of the human") + } + if strings.Contains(stderr, "the plugin binary is not installed") { + t.Errorf("a successful bootstrap must not be followed by a missing-binary complaint; stderr:\n%s", stderr) + } + log := callLog(t, calls) + for _, want := range []string{"hook prompt-router-reset", "hook session-start"} { + if !strings.Contains(log, want) { + t.Errorf("%q must run after the bootstrap in the same shell; call log = %q", want, log) + } + } + if !strings.Contains(stdout, "stdout from prompt-router-reset") || !strings.Contains(stdout, "stdout from session-start") { + t.Errorf("the binary calls' stdout must pass through untouched (it is the model-context channel); stdout = %q", stdout) + } +} + +// TestSessionStartSteadyStateRunsBothCalls is the boring session: the binary is +// already there, the bootstrap takes its fast path and says nothing, and both +// calls run. +// +// The first-line assertion is the one with teeth. `hook prompt-router-reset` +// ends by writing an UNCONDITIONAL "abcd rules: reset session" diagnostic to +// stderr (cli.go), so if it runs first it owns the only line the transcript +// renders and `hook session-start`'s actionable notice — "transcripts will not +// be captured", the version-skew line — is never seen. The chain therefore runs +// session-start FIRST; the exit precedence is computed from the saved codes and +// does not depend on the order. +func TestSessionStartSteadyStateRunsBothCalls(t *testing.T) { + root, calls := sessionStartRoot(t, "#!/bin/sh\nexit 0\n", stubBinary) + _, stderr, code := sessionStartRun(t, root, sessionStartPayload, "ABCD_CALLS="+calls) + + log := callLog(t, calls) + if !strings.Contains(log, "hook prompt-router-reset") || !strings.Contains(log, "hook session-start") { + t.Errorf("both binary calls must run in the steady state; call log = %q", log) + } + if got := firstLine(stderr); got != "abcd: a session-start notice" { + t.Errorf("session-start's notice must be the line the transcript renders, not the reset's success diagnostic; first line = %q\nfull stderr:\n%s", got, stderr) + } + if code != 2 { + t.Errorf("the binary's notice exit must propagate; code = %d", code) + } +} + +// TestSessionStartFeedsThePayloadToEveryCall is the defect a shared stdin +// creates. Every hook verb reads its payload with io.ReadAll over the whole of +// stdin (readHookInput), so two calls chained on ONE stdin leave the second +// reading EOF: json.Unmarshal fails and `hook session-start` takes its silent +// return-nil path, permanently disabling both of its notices in every session. +// The wrapper therefore reads the payload once and feeds each call a copy. +func TestSessionStartFeedsThePayloadToEveryCall(t *testing.T) { + root, calls := sessionStartRoot(t, "#!/bin/sh\nexit 0\n", stubBinary) + sessionStartRun(t, root, sessionStartPayload, "ABCD_CALLS="+calls) + + log := callLog(t, calls) + for _, verb := range []string{"prompt-router-reset", "session-start"} { + want := "hook " + verb + " stdin=[" + sessionStartPayload + "]" + if !strings.Contains(log, want) { + t.Errorf("`hook %s` must receive the whole payload, not what a previous call left; call log = %q", verb, log) + } + } +} + +// TestSessionStartReportsAMissingBinaryOnce is the genuinely-no-binary window: +// the honest failure is unchanged, but it is now said ONCE rather than by two +// sibling hooks that raced the download. +func TestSessionStartReportsAMissingBinaryOnce(t *testing.T) { + root, calls := sessionStartRoot(t, "#!/bin/sh\nexit 0\n", "") + _, stderr, code := sessionStartRun(t, root, sessionStartPayload, "ABCD_CALLS="+calls) + + if n := strings.Count(stderr, "the plugin binary is not installed"); n != 1 { + t.Errorf("the missing binary must be reported exactly once, got %d; stderr:\n%s", n, stderr) + } + if code == 0 { + t.Error("a missing binary must exit non-zero so the message reaches the human") + } + if log := callLog(t, calls); log != "" { + t.Errorf("no binary call can have run with no binary; call log = %q", log) + } +} + +// TestSessionStartKeepsTheBootstrapRefusalFirst: when the bootstrap refuses it +// has already said what is missing and the three ways out. That message leads, +// its exit code propagates, and the wrapper adds no second complaint on top of +// it — the first line of a refusal is the one the transcript shows. +func TestSessionStartKeepsTheBootstrapRefusalFirst(t *testing.T) { + root, calls := sessionStartRoot(t, `#!/bin/sh +printf 'abcd bootstrap: the latest release tag could not be resolved\n\nThe abcd binary is not installed in the plugin root.\n' >&2 +exit 1 +`, "") + _, stderr, code := sessionStartRun(t, root, sessionStartPayload, "ABCD_CALLS="+calls) + + if got := firstLine(stderr); got != "abcd bootstrap: the latest release tag could not be resolved" { + t.Errorf("the bootstrap's refusal must lead; first line = %q", got) + } + if code != 1 { + t.Errorf("the bootstrap's refusal exit must propagate; code = %d", code) + } + if strings.Contains(stderr, "the plugin binary is not installed, so") { + t.Errorf("a refusal already names the missing binary; the wrapper must not repeat it. stderr:\n%s", stderr) + } +} + +// TestSessionStartWithoutAPluginRootDoesNothing: the manifest is only ever run +// by the harness, but a command that dereferences an unset CLAUDE_PLUGIN_ROOT +// would reach for /hooks/bootstrap.sh and report a raw shell error. +func TestSessionStartWithoutAPluginRootDoesNothing(t *testing.T) { + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh unavailable") + } + cmd := exec.Command("sh", "-c", sessionStartCommand(t)) + cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + t.TempDir()} + cmd.Stdin = strings.NewReader(sessionStartPayload) + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + t.Errorf("an unset CLAUDE_PLUGIN_ROOT must exit 0 and change nothing: %v (stderr %s)", err, stderr.String()) + } + if stderr.Len() != 0 { + t.Errorf("an unset CLAUDE_PLUGIN_ROOT must say nothing; stderr = %q", stderr.String()) + } +}