fix: register the SessionStart hook so the tracing reminder fires - #17
Merged
Conversation
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
--on-startreminder code existed incli.rs, butcode-trace setuponly ever registered the Stop hook — never the SessionStart hook. So Claude Code never invoked--on-starton real installs, and thetracing ENABLED / PAUSEDwarning never appeared. The test harness hand-wired both hooks, which masked the gap.Fix: add
register_session_start_hook(mirrorsregister_stop_hook— idempotent, self-healing, preserves unrelated hooks) and compose it insetuprun(). 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. Confirmed against the Claude Code hooks docs that plain stdout from a SessionStart hook is injected into Claude's context, so the existingprintln!is correct.Harness hardening: the harness now registers hooks via the real installer (
code-trace setup --register-hook) instead of hand-written JSON, so scenario (c) genuinely validates the installer's wiring — this bug would have been caught. Also added an optionalNPMRC_FILEBuildKit secret so the image build works behind a private npm registry (no-op with direct registry access, e.g. CI).Verified:
cargo test+ clippy clean; the container harness passes all 5 scenarios end-to-end, including the reminder scenario now driven by the real installer.Re-running the installer on an existing install repairs it: the SessionStart hook is added additively, preserving the existing Stop hook and other settings.