Hook metrics (2) install wraps every hook with the runner - #512
Merged
mergify[bot] merged 22 commits intoSep 13, 2026
Merged
Conversation
…ooks/_runner/run.py runs one hook script, passes its stdout, stderr and exit code through unchanged, and appends one metrics row with a classified outcome. Review lane: behavior Safety invariant: A hook run through the runner produces byte-identical stdout and the same exit code as running it directly; a failed metrics write changes neither and adds one stderr line. Effectiveness measurement: Fixture hooks run directly and through the runner give identical stdout bytes and exit codes, and each row carries the expected outcome. Slice rationale: The runner and its outcome rules are one claim, reviewable before any install wiring. Architectural effect: Adds a shared, harness-agnostic hook runner under engine/hooks/_runner/ next to engine/hooks/_markers/. Dormant until installed. Goal: Create the runner, the pure outcome classifier, and tests. Motivation: No record exists today of which hooks fire, stay silent, or crash. Alternative considerations: Editing all 82 entrypoints to import a logging decorator was rejected: it cannot record import errors, syntax errors, or timeouts, and touches every hook. An in-process runpy runner was rejected because a harness-killed or hanging hook would take the recorder down with it. Implementation details: A subprocess wrapper plus a pure classifier in outcome.py. Non-goals: No install.sh, settings, or hook fragment change; no report CLI; no change to any existing hook. Layer: domain Feature state: dormant Files: - engine/hooks/_runner/run.py - engine/hooks/_runner/outcome.py - engine/hooks/_runner/tests/test_outcome.py - engine/hooks/_runner/tests/test_run.py - engine/hooks/_runner/tests/fixtures/ Change types: - engine/hooks/_runner/run.py: create - engine/hooks/_runner/outcome.py: create - engine/hooks/_runner/tests/test_outcome.py: create - engine/hooks/_runner/tests/test_run.py: create - engine/hooks/_runner/tests/fixtures/: create Acceptance criteria: - `python3 -m unittest discover -s engine/hooks/_runner/tests -v` exits 0. - `bash scripts/run_all_tests.sh` exits 0. - `python3 scripts/check_hook_test_coverage.py` exits 0. Exit code: 0 Invoker-Finalize-Id: e5ba51e8-de9e-430f-8672-e0eef3e3bafc
…ine/hooks/_runner/README.md states what the runner records, where the rows go, and the outcome precedence. Review lane: docs Safety invariant: Only engine/hooks/_runner/README.md changes; no code, test, or config file is edited. Effectiveness measurement: Every row field and outcome named in the README appears in run.py and outcome.py, checked by reading both. Slice rationale: Prose in its own commit so the code commit stays one claim. Architectural effect: None; prose only. Goal: Create engine/hooks/_runner/README.md. Motivation: Readers of the hook directory need the row format without reading code. Alternative considerations: Code comments were rejected; the repo forbids new comments. Implementation details: One new Markdown file. Non-goals: No code, test, or config edits. Layer: docs Feature state: dormant Files: - engine/hooks/_runner/README.md Change types: - engine/hooks/_runner/README.md: create Acceptance criteria: - `test -f engine/hooks/_runner/README.md` exits 0. Exit code: 0 Invoker-Finalize-Id: 917f28d4-7b67-447f-bfd1-7a26a20061da
…unner tests, the repo test suite, and the hook coverage gate pass. Review lane: proof Safety invariant: Verification is read-only and does not alter any repository file. Effectiveness measurement: The three commands are the direct measurement. Slice rationale: One focused proof before review. Architectural effect: None; verification only. Goal: Prove pass-through and outcome classification. Motivation: Running the tests is the proof. Alternative considerations: A live-harness run is deferred to step 2, where the runner is installed. Implementation details: Run the three commands. Non-goals: No mutations. Layer: app_regression Feature state: active Acceptance criteria: - Exits 0 only when all pass. Exit code: 0 Invoker-Finalize-Id: e73029a9-e2ea-4d00-8c66-f727763900a1
….t0.a-ae5fdfe43-637dd362
…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: cb09f6f2-66d1-4564-adb3-38e63423dbfa
…af591618b-0e93f775 — 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.
…n recording fails datetime.UTC exists only on 3.11+, so on CI's Python 3.9 the runner raised after the hook ran and dropped the hook's stdout and exit code. Use datetime.timezone.utc, and catch any failure while classifying or writing the metrics row so the hook's output is still forwarded, with one stderr line naming the error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G95BG4NxDsW4NA6fcipHrv Change-Id: Ie823ca268e85e83dd57d3b6f1b026946daeb4c99
…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) |
Base automatically changed from
plan/hook-metrics-1-a-runner-records-every-hook-run
to
main
September 13, 2026 02:53
Contributor
|
Queued — the merge queue status continues in this comment ↓. |
Change-Id: I906b9b5539dc190c313086551bc68a88d97d5a4d # Conflicts: # engine/hooks/_runner/README.md
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_96e24481-a335-4ece-8ee8-6e41c1e5550f) |
Owner
Author
|
@Mergifyio queue |
Contributor
Merge Queue Status
This pull request spent 5 minutes 56 seconds in the queue, including 4 minutes 48 seconds running CI. Required conditions to merge
|
5 tasks
mergify
Bot
deleted the
plan/hook-metrics-2-install-wraps-every-hook-with-the-runner
branch
September 13, 2026 03:07
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 hook JSON on every install, so a bug in wrapping or deduplication could break or drop hook invocations across Claude, Cursor, and Codex.
Overview
Install now routes every catstack hook through the metrics runner for Claude, Cursor, and Codex after the normal hook installers finish.
install.shsymlinksengine/hooks/_runnerinto each harness hooks directory and runswrap_installed.py, which rewrites directpython3 $HOME/.{claude,cursor,codex}/hooks/<hook>/<script>.pycommands to.../_runner/run.py --timeout <n> <hook>/<script>.py(timeout is harness value minus 0.5s, or 59.5s when unset). Already-wrapped commands are left alone; duplicate direct+wrapped entries collapse on rewrite. Non-catstack commands are untouched.Verification and maintenance follow the same rules:
check_install_effective.pyflags any remaining direct catstack hook as bypassing the runner;mirror_stop_hooks_to_subagent_stop.pyandprune_dead_hook_entries.pyunderstand runner-shaped commands; README documents the install pass. Tests cover wrapping, idempotency, install expectations, and checker reporting.Reviewed by Cursor Bugbot for commit d6e69fd. Bugbot is set up for automated code reviews on this repo. Configure here.