Repository navigation
Hook backlog (1c-b) both-layout install tests - #1332
Conversation
tests/test_install.py and engine/hooks/_runner/tests/test_wrap_installed.py asserted one settings entry per hook, so every per-hook guarantee they check failed once CATSTACK_HOOK_DISPATCHER (#1122) collapsed an event to a single _runner/dispatch.py entry: 70 failures and 1 error on #1122's head, all in these two files. load_config() in tests/test_install.py now reads an installed config and expands a collapsed event back into one entry per hook, resolving each hook's registration the way dispatch.py itself does (dispatch.load_event_hooks: the cursor registry, or the hook's own manifest for claude and codex). Each assertion therefore sees the identical shape in both layouts and keeps checking the same guarantee -- right event, right harness, Stop mirrored to SubagentStop or opted out, dead entries pruned, non-catstack entries untouched. test_wrap_installed.py's fixture pins the flag off, since it asserts the settings bytes install writes rather than a hook's registration. #1122's own test_default_is_off_and_byte_identical_to_no_flag_at_all now compares the default install against an install that sets the flag to whatever flags.py resolves as the default, and clears the key from its own environment first, so the flag-on run of the full suite cannot leak into the "default" side of the comparison. The CATSTACK_HOOK_DISPATCHER default is left off. Flipping it on is gated on both full-suite runs passing, and the source branch plan/hook-backlog-1c-install-tests-accept-the-one-entry-per-event-layout-default-on-only-if-both-suites-pass never changed engine/hooks/_flags/flags.py, so that gate was not met there and this commit does not claim it. Cherry-picked from 6330abf on that branch. The branch's two other content commits (engine/hooks/_runner/doctor.py, scripts/test/run_skill_scenarios.py) are unrelated to this claim and are left behind.
…ew claim: tests.test_install and test_wrap_installed accept both install layouts (one entry per hook with CATSTACK_HOOK_DISPATCHER off, one dispatcher entry per event with it on), and the flag defaults to on only if bash scripts/test/run_all_tests.sh passes with the flag on and off. Review lane: behavior Safety invariant: If either suite run fails, the default stays off. Per-hook guarantees stay checked in the dispatcher layout. Files: tests/test_install.py, tests/test_wrap_installed.py Change types: test Acceptance criteria: This workflow publishes one pull request for the install-layout claim only. Exit code: 0 Invoker-Finalize-Id: f459f9c9-a623-4fd7-9708-21cc3ae4cbda
…s/g0.t0.a-a464d403f-4908b6a4 — Review claim: tests.test_install and test_wrap_installed accept both install layouts (one entry per hook with CATSTACK_HOOK_DISPATCHER off, one dispatcher entry per event with it on), and the flag defaults to on only if bash scripts/test/run_all_tests.sh passes with the flag on and off. Review lane: behavior Safety invariant: If either suite run fails, the default stays off. Per-hook guarantees stay checked in the dispatcher layout. Files: tests/test_install.py, tests/test_wrap_installed.py Change types: test Acceptance criteria: This workflow publishes one pull request for the install-layout claim only.
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: 629059e5-3f90-49e5-80fb-cf0a0e81cd61) |
|
Queued — the merge queue status continues in this comment ↓. |
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 1 hour 1 minute 19 seconds in the queue, including 1 hour 20 seconds running CI. ReasonThe merge conditions cannot be satisfied: the checks did not pass within the checks timeout of 1 hour These checks were still running:
Failing checks: HintYou may have to fix your CI before adding the pull request to the queue again. If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again. Waiting for
All conditions
Requeued — the merge queue status continues in this comment ↓. |
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 2 hours 57 minutes 4 seconds in the queue, including 2 hours 54 seconds running CI. ReasonThe merge conditions cannot be satisfied: the checks did not pass within the checks timeout of 1 hour These checks were still running:
Failing checks: HintYou may have to fix your CI before adding the pull request to the queue again. If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again. Waiting for
All conditions
Requeued — the merge queue status continues in this comment ↓. |
|
@Mergifyio queue |
Merge Queue Status
This pull request spent 2 hours 49 minutes 48 seconds in the queue, including 1 hour 41 minutes 21 seconds running CI. Required conditions to merge
|
1 participant
Summary
The install checks now accept both supported hook layouts: one entry per hook or one shared entry per event.
The checks expand the shared form before comparing each hook, so both layouts keep the same per-hook rules.
Review Claim
The install and wrapping checks cover both layouts, and the shared form stays off unless both full-suite modes pass.
Review Lane
behavior
Review Unit
engine-runtime
Safety Invariant
If either full-suite run fails, the dispatcher default stays off. Per-hook guarantees remain checked in the dispatcher layout.
Slice Rationale
This slice adds the install-layout proof requested by the dispatcher-defect workflow without changing production hook behavior.
Non-goals
Measured
none: This slice adds proof for install layouts and has no product metric.
Test Plan
Test Plan
python3 engine/skills/make-pr/scripts/preflight.py --base origin/main --commit-only --dry-runruff check --select E9,F engine/hooks/_runner/tests/test_wrap_installed.py tests/test_install.pypython3 -m unittest tests.test_install.TestHookDispatcherFlagInstall -vpython3 -m unittest test_wrap_installed.DispatcherFlag -vRevert Plan
Revert Plan
git revert <sha>Note
Low Risk
Test-only changes to install and wrap_installed suites; no production hook install or routing logic is modified.
Overview
Install and wrap tests now treat both hook config shapes as equivalent for assertions: one command per hook (dispatcher off) versus one
_runner/dispatch.pyentry per event (dispatcher on).tests/test_install.pyaddsload_config(), which reads Claude/Cursor/Codex hook JSON and, when it seesdispatch.py, expands events back to per-hook groups viadispatch.load_event_hooksand the same runner command formatting aswrap_installed. Most install wiring checks switch from rawjson.loadtoload_config, so per-hook matchers and script paths are validated regardless of layout.TestHookDispatcherFlagInstall.test_default_is_off_and_byte_identical_to_no_flag_at_allnow stripsCATSTACK_HOOK_DISPATCHERfrom the environment and compares a default install to one that explicitly sets the flag to whateverflags.hook_dispatcher_onresolves—so suite-wide flag leakage cannot fake a pass.test_wrap_installed.pypinsCATSTACK_HOOK_DISPATCHER=0in the shared fixture so non-dispatcher wrap tests stay on the per-hook baseline unlessDispatcherFlagturns the flag on.No production install or dispatcher behavior changes—test-only proof that both layouts satisfy the same per-hook guarantees.
Reviewed by Cursor Bugbot for commit 0e6a50c. Bugbot is set up for automated code reviews on this repo. Configure here.