From 046b6da14beb076d8636f102ab873db81981275c Mon Sep 17 00:00:00 2001 From: winjer Date: Tue, 21 Jul 2026 09:37:07 +0100 Subject: [PATCH 1/2] fix: register the SessionStart hook so the tracing reminder fires The --on-start reminder code existed in cli.rs but setup only ever registered the Stop hook, so Claude Code never invoked --on-start on real installs and the 'tracing ENABLED/PAUSED' warning never appeared. The test harness hand-wired both hooks, which masked the gap. Add register_session_start_hook (mirrors register_stop_hook: idempotent, self-healing, preserves unrelated hooks) and compose it in setup run(). The reminder must fire for every session-start source, so the canonical hook is registered under an empty match-all matcher in its own group. Verified contract: plain stdout from a SessionStart hook is injected into Claude's context (Claude Code hooks docs). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/setup.rs | 118 ++++++++++++++++++++++++++++++++++++++++-- tests/install_test.rs | 39 ++++++++++++++ 2 files changed, 154 insertions(+), 3 deletions(-) diff --git a/src/setup.rs b/src/setup.rs index 3a27de2..cea9e33 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -77,6 +77,59 @@ pub fn register_stop_hook(mut settings: Value) -> Value { settings } +/// Register (or migrate) the canonical code-trace SessionStart hook, which runs +/// `code-trace --on-start` to print the tracing-status reminder Claude Code +/// injects as session context. +/// +/// Mirrors [`register_stop_hook`] — idempotent, self-healing, and preserving of +/// unrelated hooks. Unlike Stop, the SessionStart matcher is meaningful, and the +/// reminder must fire for *every* session-start source (startup, resume, clear, +/// compact, fork). So the canonical hook goes in its own group under an empty +/// (match-all) matcher rather than being merged into a foreign matcher group. +pub fn register_session_start_hook(mut settings: Value) -> Value { + if !settings.is_object() { + settings = json!({}); + } + let obj = settings.as_object_mut().expect("settings is an object"); + + if !obj.get("hooks").is_some_and(Value::is_object) { + obj.insert("hooks".to_string(), json!({})); + } + let hooks = obj["hooks"].as_object_mut().expect("hooks is an object"); + + if !hooks.get("SessionStart").is_some_and(Value::is_array) { + hooks.insert("SessionStart".to_string(), json!([])); + } + let ss = hooks["SessionStart"] + .as_array_mut() + .expect("SessionStart is an array"); + + // Strip every existing code-trace hook from all groups (migration + dedup). + for entry in ss.iter_mut() { + if let Some(inner) = entry.get_mut("hooks").and_then(Value::as_array_mut) { + inner.retain(|h| match h.get("command").and_then(Value::as_str) { + Some(cmd) => !is_code_trace_command(cmd), + None => true, + }); + } + } + // Drop any group left with no hooks — this removes our own prior canonical + // group so re-running does not accumulate empty match-all entries. + ss.retain(|entry| { + entry + .get("hooks") + .and_then(Value::as_array) + .is_none_or(|inner| !inner.is_empty()) + }); + + ss.push(json!({ + "matcher": "", + "hooks": [{"type": "command", "command": "code-trace --on-start"}], + })); + + settings +} + fn default_settings_path() -> PathBuf { let home = std::env::var("HOME").unwrap_or_else(|_| "~".to_string()); PathBuf::from(home).join(".claude").join("settings.json") @@ -205,7 +258,7 @@ pub fn register_hook(settings_path: &Path) -> Result<(), String> { Err(e) => return Err(format!("could not read {}: {e}", settings_path.display())), }; - let updated = register_stop_hook(settings); + let updated = register_session_start_hook(register_stop_hook(settings)); let mut out = serde_json::to_string_pretty(&updated) .map_err(|e| format!("could not serialize settings: {e}"))?; out.push('\n'); @@ -493,9 +546,17 @@ mod tests { /// Commands of every code-trace Stop hook in the document, in order. fn code_trace_commands(settings: &Value) -> Vec { + event_code_trace_commands(settings, "Stop") + } + + /// Commands of every code-trace hook under `hooks/`, in order. + fn event_code_trace_commands(settings: &Value, event: &str) -> Vec { let mut cmds = Vec::new(); - if let Some(stop) = settings.pointer("/hooks/Stop").and_then(|v| v.as_array()) { - for entry in stop { + if let Some(entries) = settings + .pointer(&format!("/hooks/{event}")) + .and_then(|v| v.as_array()) + { + for entry in entries { if let Some(hooks) = entry.get("hooks").and_then(|v| v.as_array()) { for h in hooks { if let Some(cmd) = h.get("command").and_then(|c| c.as_str()) { @@ -510,6 +571,57 @@ mod tests { cmds } + #[test] + fn session_start_hook_is_registered_with_match_all_matcher() { + // Regression: setup previously registered only the Stop hook, so the + // SessionStart tracing reminder never fired on real installs. + let out = register_session_start_hook(register_stop_hook(json!({}))); + assert_eq!( + event_code_trace_commands(&out, "SessionStart"), + vec!["code-trace --on-start"] + ); + // Must match every session-start source (startup/resume/clear/...). + let group = out.pointer("/hooks/SessionStart/0").unwrap(); + assert_eq!(group["matcher"], ""); + } + + #[test] + fn session_start_registration_is_idempotent() { + let once = register_session_start_hook(json!({})); + let twice = register_session_start_hook(once); + assert_eq!( + event_code_trace_commands(&twice, "SessionStart"), + vec!["code-trace --on-start"] + ); + // No accumulation of empty leftover groups. + assert_eq!(twice.pointer("/hooks/SessionStart").unwrap().as_array().unwrap().len(), 1); + } + + #[test] + fn session_start_migrates_legacy_and_preserves_unrelated() { + let input = json!({ + "hooks": {"SessionStart": [ + {"matcher": "startup", "hooks": [{"type": "command", "command": "~/.claude/hooks/code-trace --on-start"}]}, + {"matcher": "", "hooks": [{"type": "command", "command": "some-other-tool"}]} + ]} + }); + let out = register_session_start_hook(input); + // Legacy code-trace hook collapsed to a single canonical entry. + assert_eq!( + event_code_trace_commands(&out, "SessionStart"), + vec!["code-trace --on-start"] + ); + // Unrelated hook preserved. + let ss = out.pointer("/hooks/SessionStart").unwrap().as_array().unwrap(); + let has_other = ss.iter().any(|e| { + e.get("hooks").and_then(|v| v.as_array()).is_some_and(|hs| { + hs.iter() + .any(|h| h.get("command").and_then(|c| c.as_str()) == Some("some-other-tool")) + }) + }); + assert!(has_other, "unrelated SessionStart hook must be preserved"); + } + #[test] fn recognises_bare_command() { assert!(is_code_trace_command("code-trace")); diff --git a/tests/install_test.rs b/tests/install_test.rs index a1f3a97..f925294 100644 --- a/tests/install_test.rs +++ b/tests/install_test.rs @@ -52,6 +52,33 @@ fn code_trace_commands(settings_file: &Path) -> Vec { cmds } +/// Commands of every code-trace hook under `hooks/` in the written file. +fn event_commands(settings_file: &Path, event: &str) -> Vec { + let contents = std::fs::read_to_string(settings_file).expect("read settings file"); + let settings: Value = serde_json::from_str(&contents).expect("settings file is valid JSON"); + let mut cmds = Vec::new(); + if let Some(entries) = settings + .pointer(&format!("/hooks/{event}")) + .and_then(|v| v.as_array()) + { + for entry in entries { + if let Some(hooks) = entry.get("hooks").and_then(|v| v.as_array()) { + for h in hooks { + if let Some(cmd) = h.get("command").and_then(|c| c.as_str()) { + let base = Path::new(cmd.split_whitespace().next().unwrap_or("")) + .file_name() + .and_then(|s| s.to_str()); + if base == Some("code-trace") { + cmds.push(cmd.to_string()); + } + } + } + } + } + } + cmds +} + #[test] fn fresh_install_registers_one_canonical_hook() { let file = scratch("fresh").join("settings.json"); @@ -60,6 +87,18 @@ fn fresh_install_registers_one_canonical_hook() { assert_eq!(code_trace_commands(&file), vec!["code-trace"]); } +#[test] +fn fresh_install_registers_session_start_reminder_hook() { + // Regression: the SessionStart hook (which prints the tracing reminder) was + // never written, so the warning never fired on real installs. + let file = scratch("fresh-onstart").join("settings.json"); + assert!(register(&file).status.success()); + assert_eq!( + event_commands(&file, "SessionStart"), + vec!["code-trace --on-start"] + ); +} + #[test] fn legacy_absolute_path_hook_is_migrated_and_settings_preserved() { let file = scratch("legacy").join("settings.json"); From 55098356121771dcc5640161e3f8da6210c6a2d5 Mon Sep 17 00:00:00 2001 From: winjer Date: Tue, 21 Jul 2026 09:57:00 +0100 Subject: [PATCH 2/2] test(harness): register hooks via the real installer Previously the harness hand-wrote settings.json with both hooks, which masked the missing SessionStart registration in setup. Register hooks through 'code-trace setup --register-hook' instead, so scenario c (the tracing reminder) genuinely validates the installer's wiring. Also make the image build work behind a private npm registry via an optional NPMRC_FILE BuildKit secret (no-op with direct registry access, e.g. CI); documented in harness/README. Co-Authored-By: Claude Opus 4.8 (1M context) --- harness/Dockerfile | 8 +++++++- harness/README.md | 19 +++++++++++++++++++ harness/docker-compose.yml | 14 ++++++++++++++ harness/run-scenarios.sh | 27 +++++++++++++-------------- 4 files changed, 53 insertions(+), 15 deletions(-) diff --git a/harness/Dockerfile b/harness/Dockerfile index 5c3538f..e2b7fa0 100644 --- a/harness/Dockerfile +++ b/harness/Dockerfile @@ -19,7 +19,13 @@ RUN cargo build --release --bin code-trace \ FROM node:22-slim # Pinned CLI: bumping this is a deliberate contract re-verification (NOTES.md). ARG CLAUDE_CODE_VERSION=2.1.198 -RUN apt-get update \ +# Optional `npmrc` build secret: on networks that only reach npm through a +# private registry (e.g. a corporate mirror), pass your ~/.npmrc so the install +# routes through it. The secret is mounted only for this step — never baked into +# a layer. When absent (e.g. CI, with clean access to registry.npmjs.org) npm +# falls back to the default registry, so the build is unaffected. See README. +RUN --mount=type=secret,id=npmrc,target=/root/.npmrc \ + apt-get update \ && apt-get install -y --no-install-recommends python3 curl ca-certificates procps \ && rm -rf /var/lib/apt/lists/* \ && npm install -g @anthropic-ai/claude-code@${CLAUDE_CODE_VERSION} diff --git a/harness/README.md b/harness/README.md index 9691b1e..2b3997c 100644 --- a/harness/README.md +++ b/harness/README.md @@ -20,6 +20,25 @@ docker compose -f harness/docker-compose.yml up --build \ The `runner` service executes `run-scenarios.sh` and exits non-zero on the first failing scenario, dumping the fake Langfuse event log. +Hooks are registered by the **real installer** (`code-trace setup +--register-hook`), not hand-written JSON, so the scenarios exercise the wiring +users actually get — including the SessionStart reminder hook. + +### Behind a private npm registry + +The image build runs `npm install -g @anthropic-ai/claude-code`. On networks +that only reach npm through a private registry (so a clean container cannot hit +`registry.npmjs.org` directly), point `NPMRC_FILE` at an `~/.npmrc` that routes +through it — passed to the build as a BuildKit secret, never baked into a layer: + +```bash +NPMRC_FILE="$HOME/.npmrc" docker compose -f harness/docker-compose.yml up --build \ + --exit-code-from runner --abort-on-container-exit +``` + +Leave `NPMRC_FILE` unset (the default) where the registry is directly reachable, +such as CI. + ## Run scenarios without Docker The runner script only needs `claude`, the two binaries, and python3 on PATH: diff --git a/harness/docker-compose.yml b/harness/docker-compose.yml index d5f37d9..b602069 100644 --- a/harness/docker-compose.yml +++ b/harness/docker-compose.yml @@ -3,6 +3,8 @@ services: build: context: .. dockerfile: harness/Dockerfile + secrets: + - npmrc command: fake-langfuse environment: FAKE_LANGFUSE_ADDR: 0.0.0.0:3080 @@ -11,6 +13,8 @@ services: build: context: .. dockerfile: harness/Dockerfile + secrets: + - npmrc command: python3 /harness/stub-model/server.py environment: STUB_MODEL_PORT: "3081" @@ -19,6 +23,8 @@ services: build: context: .. dockerfile: harness/Dockerfile + secrets: + - npmrc command: /harness/run-scenarios.sh depends_on: - fake-langfuse @@ -30,3 +36,11 @@ services: CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC: "1" DISABLE_TELEMETRY: "1" DISABLE_ERROR_REPORTING: "1" + +# Optional npm credentials for the image build. NPMRC_FILE points at an ~/.npmrc +# to route `npm install` through a private registry on restricted networks; +# unset it (the default empty file) for environments with direct registry +# access, such as CI. Consumed as a BuildKit build secret — see harness/Dockerfile. +secrets: + npmrc: + file: ${NPMRC_FILE:-/dev/null} diff --git a/harness/run-scenarios.sh b/harness/run-scenarios.sh index 55fbd1f..c776b97 100755 --- a/harness/run-scenarios.sh +++ b/harness/run-scenarios.sh @@ -44,7 +44,16 @@ new_home() { rm -rf "$HOME" && mkdir -p "$HOME/.claude" "$WORK" } -# Hook wiring with tracing configured via the settings env block. +# Register the hooks through the REAL installer (`code-trace setup +# --register-hook`), not hand-written JSON, so the scenarios exercise the same +# wiring users get — including the SessionStart reminder hook. Hand-wiring both +# hooks previously masked a bug where setup registered only the Stop hook. +register_hooks() { + code-trace setup --register-hook --settings-file "$HOME/.claude/settings.json" \ + || fail "setup --register-hook failed" +} + +# Tracing configured via the settings env block; hooks via the installer. write_settings_env_mode() { cat > "$HOME/.claude/settings.json" < "$HOME/.claude/settings.json" <<'EOF' -{ - "hooks": { - "SessionStart": [{"hooks": [{"type": "command", "command": "code-trace --on-start"}]}], - "Stop": [{"hooks": [{"type": "command", "command": "code-trace"}]}] - } -} -EOF + register_hooks mkdir -p "$HOME/.config/code-trace" cat > "$HOME/.config/code-trace/config" <