Skip to content

Hook metrics (1) a runner records every hook run - #503

Merged
mergify[bot] merged 9 commits into
mainfrom
plan/hook-metrics-1-a-runner-records-every-hook-run
Sep 13, 2026
Merged

Hook metrics (1) a runner records every hook run#503
mergify[bot] merged 9 commits into
mainfrom
plan/hook-metrics-1-a-runner-records-every-hook-run

Conversation

@EdbertChan

@EdbertChan EdbertChan commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Summary

This change adds a dormant wrapper that records every automated check while preserving its output and exit status.

Today, direct starts provide no record of which automated checks speak, stay silent, block, or fail.

The wrapper starts each check separately, forwards its standard streams, classifies the result, and appends one JSONL metrics row.

Installation wiring is intentionally deferred to the next step.

Review Claim

Approve a wrapper that records each check result without changing its visible output or exit status.

Review Lane

behavior

Review Unit

engine-runtime

Safety Invariant

A wrapped hook produces byte-identical stdout and the same exit code as a direct run; a failed metrics write leaves both unchanged and adds one stderr line.

Slice Rationale

The runner and its outcome rules form one behavior claim. Documentation describes the same contract, while installation wiring remains a later slice.

Non-goals

  • No install.sh, settings, or hook fragment changes.
  • No report CLI or metrics reader.
  • No changes to existing hooks.

Architecture

Before

graph TD
    A["Harness"] --> B["Hook script"]
    B --> C["stdout, stderr, exit code"]
Loading

After

graph TD
    A["Harness"] --> B["Hook runner"]
    B --> C["Hook subprocess"]
    C --> D["stdout, stderr, exit code"]
    B --> E["Outcome classifier"]
    B --> F["JSONL metrics row"]
Loading

Test Plan

Test Plan
  • python3 -m unittest discover -s engine/hooks/_runner/tests -v
  • bash scripts/run_all_tests.sh
  • python3 scripts/check_hook_test_coverage.py
  • python3 engine/skills/make-pr/scripts/preflight.py --base main
  • python3 scripts/check_no_new_comments.py
  • bash scripts/scrub-handoff-artifacts.sh

Revert Plan

Revert Plan
  • Safe to revert? Yes. The runner is dormant until installation wiring lands.
  • Revert command: git revert <merge-commit-sha>
  • Post-revert steps: None.
  • Data migration? No.

Note

Low Risk
New isolated runtime code with no install wiring; behavior is test-guarded to preserve hook stdout and exit codes when eventually enabled.

Overview
Adds a dormant hook runner under engine/hooks/_runner/ that wraps hook scripts in a subprocess, forwards stdout/stderr and the exit code unchanged, and appends one JSONL metrics row per run (default ~/.cache/catstack-hook-metrics/runs.jsonl, overridable via CATSTACK_HOOK_METRICS_DIR).

Each row records harness (inferred from path), hook/script, stdin-derived event and session_id (with conversation_id fallback), timing, stdout size, stderr tail, and an outcome from outcome.classify (timeout, exit 2 / JSON deny signals → blocked, crashes, catstack-hook-error lines, spoke vs silent). Optional --timeout kills slow hooks and records timed_out. Metrics or classification failures add a single catstack-hook-metrics: stderr line without altering hook stdout/exit code.

README documents the CLI and outcome precedence; unit tests lock the passthrough invariant and classification rules. Install wiring is explicitly deferred—nothing in the repo invokes the runner yet.

Reviewed by Cursor Bugbot for commit 6a83264. Bugbot is set up for automated code reviews on this repo. Configure here.

Invoker and others added 8 commits September 12, 2026 19:26
…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
…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.
@cursor

cursor Bot commented Sep 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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_0722b5cf-6ea5-4d3a-a405-f2859b23a7a4)

…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
@cursor

cursor Bot commented Sep 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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_82413a95-721c-4568-9c8f-05d5c7157784)

@EdbertChan

Copy link
Copy Markdown
Owner Author

@Mergifyio queue

@mergify

mergify Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

This pull request spent 5 minutes 30 seconds in the queue, including 4 minutes 5 seconds running CI.

Required conditions to merge
  • check-success = lint
  • check-success = test

@mergify mergify Bot added the queued label Sep 13, 2026
@mergify
mergify Bot merged commit d33b03b into main Sep 13, 2026
4 checks passed
@mergify
mergify Bot deleted the plan/hook-metrics-1-a-runner-records-every-hook-run branch September 13, 2026 02:53
@mergify mergify Bot removed the queued label Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant