Hook metrics (2) install wraps every hook with the runner - #512
Open
EdbertChan wants to merge 12 commits into
Conversation
…nstall.sh wraps every installed catstack hook entry in Claude, Cursor and Codex with the runner, and a second install changes nothing. Review lane: behavior Safety invariant: Every hook registered for Claude, Cursor and Codex goes through the runner; a rerun creates no duplicate entries; non-catstack entries are kept byte-for-byte. Effectiveness measurement: In a temp HOME the three files hold zero unwrapped catstack hook entries after install, and a second install leaves them byte-identical. Slice rationale: All three harnesses in one PR so hooks behave the same everywhere. Architectural effect: Every hook run in every harness now produces a metrics row. Goal: Add wrap_installed.py, link _runner into all three hook roots, and call it last in install.sh. Motivation: The runner is inert until the harness calls it. Alternative considerations: Rewriting the 69 per-hook fragments and the strings embedded in Cursor installers was rejected: it touches every hook, and a new hook added without the wrapper would bypass metrics. A post-install pass covers future hooks automatically. Implementation details: A post-install pass over the three harness config files rewrites matching hook entries to call the runner. Non-goals: No change to run.py or outcome.py, to any hook script, or to the Codex `notify` line in config.toml. Layer: app_bridge Feature state: active Files: - engine/hooks/_runner/wrap_installed.py - engine/hooks/_runner/tests/test_wrap_installed.py - install.sh Change types: - engine/hooks/_runner/wrap_installed.py: create - engine/hooks/_runner/tests/test_wrap_installed.py: create - install.sh: modify Acceptance criteria: - `python3 -m unittest discover -s engine/hooks/_runner/tests -v` exits 0. - `python3 -m unittest tests.test_install -v` exits 0. - `shellcheck install.sh` exits 0. Exit code: 0 Invoker-Finalize-Id: 026f7cd3-16c7-45af-9eb2-8c7ba1483c72
…m: scripts/check_install_effective.py reports every installed catstack hook entry that bypasses the runner. Review lane: policy Safety invariant: The check only reads the three harness config files and reports; it writes nothing. Effectiveness measurement: A test with one wrapped and one unwrapped fixture entry gets exactly one `hook bypasses the metrics runner` line. Slice rationale: The install check is tooling policy, kept apart from the install behavior it checks. Architectural effect: A missed or future hook that bypasses metrics shows up in the install check. Goal: Extend check_install_effective.py with a bypass check. Motivation: Wrapping is only trustworthy if something reports a miss. Alternative considerations: Duplicating the matcher in the check was rejected; it imports match_direct from wrap_installed.py so the two cannot disagree. Implementation details: Import match_direct and print one problem line per unwrapped entry. Non-goals: No install.sh or hook edits. Layer: app_bridge Feature state: active Files: - scripts/check_install_effective.py - tests/test_check_install_effective.py Change types: - scripts/check_install_effective.py: modify - tests/test_check_install_effective.py: create Acceptance criteria: - `python3 -m unittest tests.test_check_install_effective -v` exits 0. Exit code: 0 Invoker-Finalize-Id: 1c706729-4212-4fdc-9c6a-dd8c11051a81
…gine/hooks/_runner/README.md states how install wraps hook entries and how the install check reports a bypass. Review lane: docs Safety invariant: Only engine/hooks/_runner/README.md changes; no code, test, or config file is edited. Effectiveness measurement: The README's entry format and printed messages match wrap_installed.py, checked by reading both. Slice rationale: Prose in its own commit so the code commits stay one claim each. Architectural effect: None; prose only. Goal: Add an Install section to engine/hooks/_runner/README.md. Motivation: Readers need the install shape without reading code. Alternative considerations: Code comments were rejected; the repo forbids new comments. Implementation details: One Markdown section. Non-goals: No code, test, or config edits. Layer: docs Feature state: active Files: - engine/hooks/_runner/README.md Change types: - engine/hooks/_runner/README.md: docs-only Acceptance criteria: - `grep -n "wrap_installed.py" engine/hooks/_runner/README.md` prints at least one line. Exit code: 0 Invoker-Finalize-Id: a272c6e0-1997-4c03-8e14-324d8cf5bdce
…e repo suite, which includes the runner, install, and install-check tests, passes, and install.sh is shellcheck-clean. Review lane: proof Safety invariant: Verification is read-only for the repository; tests write only into temp directories. Effectiveness measurement: The two commands are the direct measurement. Slice rationale: One focused proof before review. Architectural effect: None; verification only. Goal: Prove wrapping is complete and a rerun changes nothing. Motivation: Running the tests is the proof. Alternative considerations: The live claude -p row check needs model access and is run by the parent session after merge. Implementation details: Run the suite and shellcheck. Non-goals: No mutations. Layer: app_regression Feature state: active Acceptance criteria: - Exits 0 only when all pass. Exit code: 127 Invoker-Finalize-Id: 354b2bdb-010c-4742-91ee-4ba090cd0747
…e repo suite, which includes the runner, install, and install-check tests, passes, and install.sh is shellcheck-clean. Review lane: proof Safety invariant: Verification is read-only for the repository; tests write only into temp directories. Effectiveness measurement: The two commands are the direct measurement. Slice rationale: One focused proof before review. Architectural effect: None; verification only. Goal: Prove wrapping is complete and a rerun changes nothing. Motivation: Running the tests is the proof. Alternative considerations: The live claude -p row check needs model access and is run by the parent session after merge. Implementation details: Run the suite and shellcheck. Non-goals: No mutations. Layer: app_regression Feature state: active Acceptance criteria: - Exits 0 only when all pass. Exit code: 0 Invoker-Finalize-Id: publish-approved-fix
…0.t0.a-ab927feb4-e722238b
…No ephemeral inter-task handoff files remain in the worktree before the merge gate. Review lane: cleanup Safety invariant: The scrub script only checks for known handoff artifact names and never touches source, tests, or other repository files. Effectiveness measurement: The script exits non-zero if any handoff artifact remains. Slice rationale: Required terminal scrub for every implementation workflow. Architectural effect: None; hygiene only. Goal: Leave the branch free of handoff artifacts. Motivation: Handoff files must not reach the PR. Alternative considerations: Manual cleanup was rejected as non-deterministic. Implementation details: Run scripts/scrub-handoff-artifacts.sh. Layer exception: allowed -- the terminal scrub must run after every task in the workflow, including the docs task. Non-goals: No product edits. Layer: app_regression Feature state: active Acceptance criteria: - `bash scripts/scrub-handoff-artifacts.sh` exits 0. Exit code: 0 Invoker-Finalize-Id: abcc8dd1-19ae-4cdc-b9a6-8bec5bce3173
…af2175b3f-597cac02 — Review claim: No ephemeral inter-task handoff files remain in the worktree before the merge gate. Review lane: cleanup Safety invariant: The scrub script only checks for known handoff artifact names and never touches source, tests, or other repository files. Effectiveness measurement: The script exits non-zero if any handoff artifact remains. Slice rationale: Required terminal scrub for every implementation workflow. Architectural effect: None; hygiene only. Goal: Leave the branch free of handoff artifacts. Motivation: Handoff files must not reach the PR. Alternative considerations: Manual cleanup was rejected as non-deterministic. Implementation details: Run scripts/scrub-handoff-artifacts.sh. Layer exception: allowed -- the terminal scrub must run after every task in the workflow, including the docs task. Non-goals: No product edits. Layer: app_regression Feature state: active Acceptance criteria: - `bash scripts/scrub-handoff-artifacts.sh` exits 0.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_71e248bb-4241-4aff-b033-59952ba45219) |
…unner Once wrap_installed.py rewrites an entry to `_runner/run.py --timeout T <hook>/<script>.py`, the only $HOME/.claude/hooks/ path in the command is the runner itself, which exists, so a deleted hook's entry was never pruned. prune_dead_hook_entries.py now also checks the hook script named after the runner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G95BG4NxDsW4NA6fcipHrv Change-Id: I2d2f61c86f55a52697a191aeffc2049d81247650
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_ee33fadb-8943-4d17-8875-3b28b67a97d6) |
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.
Summary
The installer now links the metrics runner into Claude, Cursor, and Codex hook roots.
After each installer runs, a post-install pass rewrites direct catstack hook commands to invoke the metrics runner.
The pass preserves hook arguments and unrelated entries, and repeated installs remain byte-identical.
The installation check reports any catstack hook command that bypasses the metrics runner.
Review Claim
Every installed catstack hook in Claude, Cursor, and Codex invokes the metrics runner after installation, preserving unrelated configuration.
Review Lane
behavior
Review Unit
engine-runtime
Safety Invariant
Every catstack hook entry goes through the runner; rerunning
install.shcreates no duplicates, and non-catstack entries remain byte-for-byte unchanged.Slice Rationale
This slice installs the runner at the common post-install boundary, where all three harness configuration files are visible together.
The install-effective check, README, and focused tests document and protect the same wrapping behavior.
Non-goals
run.py,outcome.py, or individual hook scripts.notifyline inconfig.toml.claude -pmetrics-row measurement; that requires model access and runs after merge.Architecture
Before
graph TD A["install.sh"] --> B["Claude/Cursor/Codex installers"] B --> C["direct hook commands"]After
graph TD A["install.sh"] --> B["Claude/Cursor/Codex installers"] B --> C["wrap_installed.py"] C --> D["runner-backed hook commands"] D --> E["_runner/run.py"] E --> F["metrics rows"]Test Plan
Test Plan
bash scripts/run_all_tests.sh && shellcheck install.shpython3 -m unittest discover -s engine/hooks/_runner/tests -vpython3 -m unittest tests.test_install -vpython3 -m unittest tests.test_check_install_effective -vbash scripts/scrub-handoff-artifacts.shclaude -pverification remains a post-merge measurement because it requires model access.Revert Plan
Revert Plan
git revert <merge-sha>bash scripts/run_all_tests.sh && shellcheck install.sh.Note
Medium Risk
Rewrites live harness JSON under
$HOMEon every install; a bug could break hook invocation or drop entries, though tests and idempotent design limit blast radius.Overview
Install now routes Claude, Cursor, and Codex catstack hooks through the metrics runner instead of invoking hook scripts directly.
After per-harness hook installers finish,
install.shsymlinksengine/hooks/_runnerinto each hooks tree and runswrap_installed.py, which rewrites commands of the formpython3 $HOME/.<harness>/hooks/<hook>/<script>.pytopython3 .../_runner/run.py --timeout <n> <hook>/<script>.py. Harness timeout minus 0.5s is used when present; otherwise 59.5. Already-wrapped commands are left alone; duplicate direct/wrapped entries for the same hook are collapsed. Non-catstack commands are unchanged.Verification and maintenance follow the same rules:
check_install_effective.pyflags any remaining direct catstack command as bypassing the runner;mirror_stop_hooks_to_subagent_stop.pyandprune_dead_hook_entries.pyunderstand runner-wrapped commands. README documents install behavior; unit and install tests cover wrapping, idempotency, bypass reporting, and pruning.Reviewed by Cursor Bugbot for commit 36ef709. Bugbot is set up for automated code reviews on this repo. Configure here.