From 231fbea2a85c3ad65954de1185798ee489ee3fc8 Mon Sep 17 00:00:00 2001 From: Matt Coles Date: Tue, 20 Oct 2026 10:14:44 +0000 Subject: [PATCH] chore(hooks): remove automatic test runs --- .../personas/quality/references/checks.conf | 6 +-- dist/macols-personas-claude-plugin.zip | Bin 162830 -> 162785 bytes docs/hooks.md | 38 ++++++---------- hooks/checks/lang_checks.sh | 14 +----- hooks/checks/post_task.sh | 16 +++---- hooks/post_task_hook.sh | 10 ++--- hooks/pre_commit_check.sh | 7 +-- .../remove-hook-test-runs/.openspec.yaml | 2 + .../changes/remove-hook-test-runs/design.md | 9 ++++ .../changes/remove-hook-test-runs/evidence.md | 20 +++++++++ .../changes/remove-hook-test-runs/proposal.md | 32 +++++++++++++ .../specs/hook-wiring/spec.md | 42 ++++++++++++++++++ .../changes/remove-hook-test-runs/tasks.md | 6 +++ tests/test_hook_loops.sh | 35 +++++++++++++-- 14 files changed, 172 insertions(+), 65 deletions(-) create mode 100644 openspec/changes/remove-hook-test-runs/.openspec.yaml create mode 100644 openspec/changes/remove-hook-test-runs/design.md create mode 100644 openspec/changes/remove-hook-test-runs/evidence.md create mode 100644 openspec/changes/remove-hook-test-runs/proposal.md create mode 100644 openspec/changes/remove-hook-test-runs/specs/hook-wiring/spec.md create mode 100644 openspec/changes/remove-hook-test-runs/tasks.md diff --git a/config/personas/quality/references/checks.conf b/config/personas/quality/references/checks.conf index 8e4f0d8..e81ca7b 100644 --- a/config/personas/quality/references/checks.conf +++ b/config/personas/quality/references/checks.conf @@ -8,10 +8,8 @@ IMMEDIATE="uv run python scripts/validate_schemas.py" # Before each commit: affected integration tests and a small eval smoke set. CHECKPOINT="uv run pytest -m integration -q && uv run python -m evals.smoke" -# Tests the checkpoint runs: related (default) runs only the tests the commit's -# changes reach, leaving the full suite to CI; full runs each affected -# package's whole suite. Use full only when the suite takes seconds. -# CHECKPOINT_TESTS=full +# Hooks do not automatically run tests. Set CHECKPOINT explicitly for any +# tests this repo needs before an agent commit, or use your own pre-commit hook. # Nightly CI (quality.yml schedule): E2E, broad model matrix, load tests, the # full eval suite. diff --git a/dist/macols-personas-claude-plugin.zip b/dist/macols-personas-claude-plugin.zip index 0108bfd821ddc1a1fd062474e0876c1481b9b84d..80298be14a4a513bd1bc16e6bc0ef5482a9239c4 100644 GIT binary patch delta 2389 zcmZ8idsvg_70>q)s2ETXMM*M&P=S?8xtIwl5~xn7OfKGF1c4X`0tqBRv|JQY3${`n znChvs0*WZ=9#$sK$EJ8`wG5Ri-oa6XiViERPSk3l?tE_$L-Rb}ALn;|=klKOd*5vP z*}C_cb!7o}_Em*<)dp!v@VUxkJFkfvWH+BoS-|)IO!=;R;{M^G19tn;SJu3Izh;+Z z>2MIgfC=8`-ug?b$Pls9cueU1#^##a<$9mT-?eQ|DY}05$d-GTt7X^oJ@UQ(@QtLa za&os~v1U%j;ST1FIv>e}LyepljQf^L#=1kQKCk(n%C^t1_BcrEFP;-wI6ks8G%ew8 zN??vBFwB*9+s5<%dNjarIP)zy9I?Swz=z6y#-T*9CS8%8=c~xgQEpfom3!OI=H&hO z;z)fXBecBbA2wB%<$dOc>a=bkGgc+JA*yJcojLN~h|cah=uR_?B;U1Ne$eZm@)gIH z@7Z+tmN;SGvk}ALkG2@2jgngV)GI40${Ru-Jg~X&Z!clu2ccCJM+MCn?{i|lT%`Ja zq-*VsxRM{f>nIB$9w~7xr7mB4fKf8H+BmZPOm*)rg=@m1!=Yu7PjY>jhYw4Qb=o8E zJ=Q%;@C>P}k97M;yp32zH*~gCXhiYt+%KkEe)HxG|AOO->Nd?1s(Euo9?u)j>6TbV z?wS6<$baVgcJUv{Zyo9!del^WYeUed&mOOJS~9#qRgxaE{QF`5za{S;Xq_uuF1;A| zGGhCr%=3N+v-IPo!UeTgJP!4a(i>6VFEmf6OWQ zIsU^>oS6k(d#l?|JQQDQMdQ`w4?h)J>|9Hftb(R z7A`$HI&}5vPyGWMhhBIU9@#vz#=7;;%QVp5JDL(3Wx<`!;c$*`?O>cfwipfPpd=jN zt3=|7hA(SWCD{W{My3c@Nmn5EE`D#}$No@AoMwHti8mEi%19u_g+hmn@Zo@rcrv4K zNk;x;(X9W11w{~+MEnrcCXqO9v;G~PA7ErUxmk2YZNE9b)Zg=7+r z&UZC=4;e8sgdbPa{=3E@F%r59XbZ80)=;6KenBA(xvU_; z=yxoqmL?4oc%@N7>^Q_G$lUvyAWVO?oW3-=oW_aHg*Y8~I!VW{;Dd&$gD&tOou&Kg=U#~d@Rp|iT_JkE1qi?hpib{WT_amIhoq?*-1QzmNG zC5vh{E{jAU^}fG}DhyzFa5@XaN5esC!Y)NHR=~@qE4*%w*@YpQN~tuPHv89z@rY?) z%%-B~d5zh@6csHhRgn~&W{rxdu(o@WDeBY;MgZypG7DsCBEp6@s!25ZZRD)q4IZoM zNqe;>dRPweXAh|xH?3inj+q2CIkf##Tg@DUIVN@Ik~EB&vD?fwx)o26yqmF|Fn70V zsM6qlx7LuiF(P%$T&W@Ru(NF%5{1`a4wV|>3Zoi&Dqk&~?_TiJ;(Tkg6uhh@@fbg| z9sIROeR%_mrL;3MK&2yg!U=y|rmHpf{0J|Z1)SB9KunU*VTQGLFpe-kk1o3Snweuq z9tpG(!WI~^q23z|CkGX3d6+rQ8%o_UF(H%NkfW8n_9k8DGU^}s6X7PH$ z-pbjwCU2=Nv4U&sX_olc7#CV>@iL(BSd}j3vt(z;d7n00Hf9d;O_cFxuEmGty=8iS zJy2y>*TUC?q39%No((XW^8&H8p#qvOE69X(2N8S)3+ZvELunyVV(qdp)+qqXB6>Ma hjhQh;xUSLg)6vjTgq_v1jW+9lq2@fRxI}`R{y(Romc{@8 delta 2350 zcmY*bX;_n27S4A8ixMG<>`|Z(M3zSycHA&jJFI1?1%a@Hodie-2nkrgC^#L8ChGF6 z^3?T)p#^JujsBzkmx5wabqVeyIQVnCX_*ke&oR)u4s^{HNA9 zl8f~BCJ>hs2;#PCc@Jl*%pB2Z4SF=1x7^;w8Y{)fv?47#BQ2-UElniMEIXtUj<6Y( zQ{~k%JqI72C+C3M7ul;%x)1-u;1siTnI(T_`*uZl>*R#`s$!+$$}3h~qbRF*7M&TW z^86yv>vLwD+3lFvmf7bfW<=Gm8}dT`G`=awV!6wy7r_>RmQPBnuGvo<%RQ3VEPj4- zS`k{gFV?NXvenqyD0J-$p6{DIKRQ+#Zn^vB%+~Ql3+1-E8>LUWLt@X~o0I7s7A0BU zKDOek_MxPR6L+5WeD-i3-5`W95T9Bj-febOz`7pBtdU1oc8x4|QLSY6n0xK9 z`EPsuO~Lr>&A`ImD=)x`IGUp5+zy_R((+YUT- z$Q(TIGWHMGU?cvslu3b&PhYfSmm)JqZ_xiR$FAX<^PgDXmwu%Yy^H#qH9J&M<>2!pQ`lrFl}k+`Xu=9ny}cm zm;9AcjQg$50~KaQ9oZMpI_G`d{|zswDW2QDW6-Wdn*I19v-n-_K=7Wnp=+Jn*x%+J z{pr_gv9R%EaNG3tjw9)h?nMr_`2N*DB!w0C_|Rd_{arIhvS#1@?h}$Wv6br^K)brx zymv2sU5#Ke>{!Wq%Xs0zvL9olFT!76k+-rZc229ma;Oe;o^x~=GLQH0QaN8~T>W}X z_|jH+xo||;>|46&T`c?byl4H*qrLO!a%?|g{}(-V01b>|iVwsiCjf3FP{ku#()xHh z9<9<-$a3D(CfJ#aJVEG>f8!!+T*4*O3N`3Md;<`nz*AiGxxPB!?J{higjhH)0XgX_ zWE+hrs7yd?LXD>rPy(>qZ8WUAyxGPG*Cdj{&ul5G$(D|1fP#u^l8_ia4kjTHp>9Y< zA&}|Kl1H)D;!-hU;&W0&hngpnQ8etfA2%l>SGk3cG5&~0mgH@~CzFwjyluk@+yE44 zl{^#yB~4P$4%qAYPv!1hYh0OvT!B27f+9geq@rM0D1r;IH8?jFu|e4^!2RjSTF%JP z$KR!r)rYkd9W&B^QU{EdkSZLL21yE;O(_KxX~++v=KC~o4W8GC6|%?$7?h6K&{{t3 zOGhsFRx)DX&U9o68@ip2GC-(QX!4vfF9WTIGV9yrb?tWe+YGWwkp?;8cYMT@@9xp3 z)5sTtrVhyLA$5tF$P;SSWuhebn9f8gP_p{Y`{JRfxd8##`B*M^QjMeX zP&lCGJQM@_dnl&LFhpo6W(vtB*%s(yWkf{wED$^NJ0qfkgnxj-eA<)aU99~C0RC=B)*E+%{4 z`;$gtk8K}7%Sg95st5+@8v?qbkXfh}1OcsMWG;7{ z;*yI{AdF-0FB)itjf>F|`~xNVZjti=NzM!<=_rPY35v-orx!WRJ=>~>`z7$Oi.py` by name; `vitest related` / `jest - --findRelatedTests` by import graph (any other `npm test` waits for the - checkpoint, since it may be E2E); `go test` on the changed packages. When +- post-task: at the end of a turn, lint and strict types, golangci-lint, + jscpd duplication, CDK synthesis, and layer rules (import-linter, + dependency-cruiser) when the project configures them. Built-in Python, + Node, Flutter and Go tests do not run. When something fails, the agent gets one more step with the findings and a fix instruction. It only re-runs when the tree changed since its last run, so it can't loop and Q&A turns stay fast (under a second on small projects). @@ -142,9 +140,7 @@ first run). Pass `--no-cli` to skip the whole step. Every turn-end and checkpoint run appends one JSON line per check to `.git/macols-checks.jsonl` (shared by worktrees, capped at 10k lines): -duration, pass or fail, and which loop ran it. At the checkpoint, failing -Python tests that the name-based turn-end selection wouldn't have run are -recorded as escapes. +duration, pass or fail, and which loop ran it. Historical test escapes remain readable in older logs. ```bash macols-check-stats # per loop and check: runs, total/p95 time, fail %, fail streak, escapes @@ -152,10 +148,8 @@ macols-check-stats --days 7 macols-check-stats --escapes # which tests escaped the fast path ``` -Read it as: a slow check that never fails belongs in CI, a long fail streak -means the agent is going round in circles, and escapes mean the fast path -needs pytest-testmon or better-named tests. `MACOLS_CHECK_LOG=off` stops the -logging. +A slow check that never fails is a candidate for CI. A long fail streak +means the agent is going round in circles. `MACOLS_CHECK_LOG=off` stops logging. ## Trusted Projects @@ -190,16 +184,12 @@ workflow. Existing layer breaks are approved explicitly: a | Variable | Effect | |---|---| -| `MACOLS_PYTEST_SCOPE=changed\|full\|off` | Python test scope at turn end (default `changed`) | -| `MACOLS_TESTMON=off` | Ignore pytest-testmon and select Python tests by name | -| `MACOLS_GO_TEST_SCOPE=changed\|module` | Go tests at turn end: only changed packages (default) or the whole changed module via the test cache | | `MACOLS_DEPLOY_DIFF=off` | Don't run `cdk diff` before a deploy prompt | | `MACOLS_DEPLOY_DIFF_TIMEOUT` | Seconds the pre-deploy `cdk diff` may take (default 240) | | `MACOLS_CHECK_LOG=off` | Don't log check timings | | `MACOLS_DUPLICATION=off` | Skip the jscpd duplication check | | `MACOLS_DUP_MIN_LINES` | Minimum clone size jscpd reports (default 10) | | `MACOLS_MAX_FILE_LINES` | File-length limit (default 500, `0` to turn off) | -| `MACOLS_GO_RACE=1` | Run Go tests with the race detector | | `MACOLS_SEMGREP=1` | Run semgrep locally | | `MACOLS_CHECKS_VERBOSE=1` | Also print notes such as "pytest not installed" | | `MACOLS_HOOKS_DIR` | Where the hook scripts live | diff --git a/hooks/checks/lang_checks.sh b/hooks/checks/lang_checks.sh index 977fa98..8e87817 100644 --- a/hooks/checks/lang_checks.sh +++ b/hooks/checks/lang_checks.sh @@ -1,7 +1,7 @@ #!/bin/bash # # lang_checks.sh — the remaining per-language batteries: cdk synth, Flutter -# (scoped flutter test + dart analyze), Go (golangci-lint/go vet + go test), +# (dart analyze), Go (golangci-lint/go vet), # and shellcheck for scripts. # # Sourced by post_task.sh AFTER common.sh and its helpers (add_critical_issue, @@ -100,10 +100,7 @@ _changed_go_packages() { } # golangci-lint (the project's .golangci.yml: dupl, gocyclo, funlen, depguard…) -# or go vet when it isn't installed, on the changed packages; then go test on -# the changed packages (./... per module at the checkpoint). -# MACOLS_GO_TEST_SCOPE=module widens turn-end tests to the whole module, which -# also catches dependents (cheap when the test cache can replay them). +# or go vet when it isn't installed, on the changed packages. run_go_checks() { command -v go &> /dev/null || { add_warning "go not installed - skipping Go checks"; return 0; } local pkgs @@ -122,13 +119,6 @@ run_go_checks() { out=$(cd "$mod" && go vet "${dirs[@]}" 2>&1) || ec=$? report_check_result "go vet ($label)" "$ec" "$out" "^.+\.go:[0-9]+(:[0-9]+)?: |^vet: " "issues" fi - ec=0 - local -a race=() - [ "${MACOLS_GO_RACE:-0}" = "1" ] && race=(-race) - local -a test_targets=(./...) - [ "${MACOLS_GO_TEST_SCOPE:-changed}" = "changed" ] && ! full_suite && test_targets=("${dirs[@]}") - out=$(cd "$mod" && ${TIMEOUT_CMD:+$TIMEOUT_CMD $MAX_TEST_TIME} go test ${race[@]+"${race[@]}"} "${test_targets[@]}" 2>&1) || ec=$? - [ "$ec" -ne 0 ] && report_test_failure "go test ($label)" "$ec" "$out" done return 0 } diff --git a/hooks/checks/post_task.sh b/hooks/checks/post_task.sh index 9c86e88..16ce6f3 100755 --- a/hooks/checks/post_task.sh +++ b/hooks/checks/post_task.sh @@ -25,10 +25,7 @@ # per-turn loop (semgrep, pip-audit/npm audit/govulncheck — see the `quality` # skill's CI template). # -# Tests pytest (scoped, or pytest-testmon when installed), -# jest/vitest related tests, flutter test (scoped), go test -# over changed modules (the test cache skips the rest), -# cdk synth +# Synthesis cdk synth # Lint ruff, eslint, dart analyze, golangci-lint (or go vet), # and shellcheck, each with the project's own limits # Types pyright or mypy (whichever the project configures), @@ -42,9 +39,8 @@ # Only findings are recorded as CRITICAL_ISSUES, each with a fix instruction. # WARNINGS hold notes (tool missing, nothing to test) for verbose runs. # -# Switches: MACOLS_PYTEST_SCOPE=changed|full|off, MACOLS_TESTMON=off, -# MACOLS_DUPLICATION=off, MACOLS_SEMGREP=1 (opt back in to a local semgrep -# scan), MACOLS_GO_RACE=1, MACOLS_GO_TEST_SCOPE=changed|module, +# Tests run only through explicit project commands, never automatically. +# Switches: MACOLS_DUPLICATION=off, MACOLS_SEMGREP=1, # MACOLS_CHECK_LOG=off (see log_check_run in common.sh), # MACOLS_LGREVIEW=off (and its BLOCK/TIMEOUT knobs — lgtmaybe_review.sh). # @@ -208,14 +204,14 @@ run_post_task_checks() { local -a checks=() if [ "$has_python" = "true" ]; then - checks+=(run_python_tests run_ruff_check run_python_typecheck run_import_linter) + checks+=(run_ruff_check run_python_typecheck run_import_linter) fi if [ "$has_node" = "true" ]; then - checks+=(run_node_tests run_eslint_check run_tsc_check run_dependency_cruiser) + checks+=(run_eslint_check run_tsc_check run_dependency_cruiser) fi [ "$has_cdk" = "true" ] && checks+=(run_cdk_tests) if [ "$has_flutter" = "true" ]; then - checks+=(run_flutter_tests run_dart_analyze) + checks+=(run_dart_analyze) fi [ "$has_go" = "true" ] && checks+=(run_go_checks) checks+=(run_shellcheck run_duplication_check run_file_length_check run_semgrep_scan) diff --git a/hooks/post_task_hook.sh b/hooks/post_task_hook.sh index c998fd6..c20344d 100755 --- a/hooks/post_task_hook.sh +++ b/hooks/post_task_hook.sh @@ -17,14 +17,12 @@ # changed nothing is allowed to stop, so the hook cannot loop. # - Never when the tool says this stop is already a hook-driven continuation # (stop_hook_active) — one nudge per turn, then the user decides. -# - pytest/jest/vitest/go test are scoped to what the changed files reach -# (pytest-testmon or file names for Python, the test cache for Go). The full -# suite is the commit checkpoint's and CI's job. -# MACOLS_PYTEST_SCOPE=full|changed|off overrides the Python scope. +# - Built-in tests never run. Repos own test execution through explicit +# project commands or their own pre-commit/CI workflow. # - Every run appends per-check timings to .git/macols-checks.jsonl # (bin/macols-check-stats summarises them). # -# Only critical findings (failing tests, lint/type/gate errors) are sent to the +# Only critical findings (failing project commands, lint/type/gate errors) are sent to the # model; notes such as "pytest not installed" print only with # MACOLS_CHECKS_VERBOSE=1. # @@ -78,7 +76,7 @@ fi TRUST_NOTE="" if [ "${UNTRUSTED_SKIPPED:-0}" = "1" ]; then - TRUST_NOTE="Tests, eslint, tsc, mypy, cdk synth, go and layer checks were skipped: $(project_root) is not a trusted project, and those run code the repo controls. To enable them, run $(dirname "$SHARED_HOOKS_ROOT")/bin/macols-trust in the repo, or add its path to $MACOLS_TRUST_FILE." + TRUST_NOTE="Project commands, eslint, tsc, mypy, cdk synth, go and layer checks were skipped: $(project_root) is not a trusted project, and those run code the repo controls. To enable them, run $(dirname "$SHARED_HOOKS_ROOT")/bin/macols-trust in the repo, or add its path to $MACOLS_TRUST_FILE." fi if [ ${#CRITICAL_ISSUES[@]} -eq 0 ]; then diff --git a/hooks/pre_commit_check.sh b/hooks/pre_commit_check.sh index 1f17dc9..3d2c19d 100755 --- a/hooks/pre_commit_check.sh +++ b/hooks/pre_commit_check.sh @@ -2,8 +2,7 @@ # # pre_commit_check.sh — protocol-neutral core of the local # checkpoint loop: when the agent runs `git commit`, run the checkpoint -# battery (the tests the change reaches, or whole suites with -# CHECKPOINT_TESTS=full; lint, types, layer rules, and the project's +# battery (lint, types, layer rules, and the project's explicit # CHECKPOINT command) before the commit lands. # # Prints the findings when the checkpoint fails and nothing otherwise, and @@ -49,9 +48,7 @@ fp="" review_ready || fp=$(_check_fingerprint || true) [ -n "$fp" ] && [ -f "$fp_file" ] && [ "$(cat "$fp_file")" = "$fp" ] && exit 0 -CHECKPOINT_MODE=1 MACOLS_PYTEST_SCOPE=full -full_suite || MACOLS_PYTEST_SCOPE=changed -export CHECKPOINT_MODE MACOLS_PYTEST_SCOPE +export CHECKPOINT_MODE=1 # shellcheck disable=SC2034 # read by the sourced post_task.sh checks MAX_TEST_TIME="${MACOLS_CHECKPOINT_TIMEOUT:-900}" run_post_task_checks || exit 0 diff --git a/openspec/changes/remove-hook-test-runs/.openspec.yaml b/openspec/changes/remove-hook-test-runs/.openspec.yaml new file mode 100644 index 0000000..0ee45e4 --- /dev/null +++ b/openspec/changes/remove-hook-test-runs/.openspec.yaml @@ -0,0 +1,2 @@ +schema: macols +created: 2026-10-20 diff --git a/openspec/changes/remove-hook-test-runs/design.md b/openspec/changes/remove-hook-test-runs/design.md new file mode 100644 index 0000000..4f85781 --- /dev/null +++ b/openspec/changes/remove-hook-test-runs/design.md @@ -0,0 +1,9 @@ +# Design +## Context +The shared orchestrator dispatches Python, Node and Flutter test checks; Go combines lint and tests in one check. Both turn-end and pre-commit use this battery. +## Goals / Non-Goals +Remove automatic test execution. Preserve static checks, synthesis, review, trust boundaries and explicit project commands. Do not provision repo-specific test hooks. +## Decisions +Remove test dispatch from the shared orchestrator and the test portion of Go checks. Keep existing standalone Python/Node/Flutter selection helpers outside the automatic battery to avoid unrelated helper and fixture churn. Remove checkpoint scope overrides because the battery no longer selects tests. +## Risks / Trade-offs +Tests no longer block completion or commits unless a repo explicitly configures them. Existing IMMEDIATE or CHECKPOINT commands can still run tests by design. diff --git a/openspec/changes/remove-hook-test-runs/evidence.md b/openspec/changes/remove-hook-test-runs/evidence.md new file mode 100644 index 0000000..e6a38f0 --- /dev/null +++ b/openspec/changes/remove-hook-test-runs/evidence.md @@ -0,0 +1,20 @@ +# Evidence + +## E1. No automatic tests; static checks and configured commands survive +- **Claim**: Both batteries skip built-in tests, including legacy full-scope settings; Go lint and explicit checkpoint failures remain effective. Trust gates and model-visible findings survive. +- **Command**: `bash tests/test_hook_loops.sh` +- **Expected**: All assertions pass using scratch repos and boundary stubs. +- **Fails before**: The new battery regression detects automatic test dispatch; Go assertions detect go test. +- **Recorded**: pass, 41/41 assertions; base HEAD 95a50bdf609186b7b0014593bdff3c24bfcdeca3, working-tree changes tested (2026-10-20). + +## E2. Shell and anchor constraints +- **Claim**: C-SHELLCHECK-CLEAN, C-SPEC-ANCHORS-HEALTHY and source-only changes hold. +- **Command**: `bash -n hooks/checks/post_task.sh hooks/checks/lang_checks.sh hooks/post_task_hook.sh hooks/pre_commit_check.sh tests/test_hook_loops.sh && shellcheck -S warning hooks/checks/post_task.sh hooks/checks/lang_checks.sh hooks/post_task_hook.sh hooks/pre_commit_check.sh tests/test_hook_loops.sh && ./scripts/spec_drift_gate.sh --check` +- **Expected**: All commands pass; installer and per-edit code unchanged. +- **Recorded**: pass; base HEAD 95a50bdf609186b7b0014593bdff3c24bfcdeca3, working-tree changes tested (2026-10-20). + +## E3. Installer, bundle and change validation +- **Claim**: Codex wiring remains valid, the quality template is bundled, and spec deltas validate. +- **Command**: `./tests/verify_install.sh codex && ./scripts/package_claude_desktop_personas.sh --check && openspec validate remove-hook-test-runs --strict && git diff --check` +- **Expected**: All hard checks pass. +- **Recorded**: pass; base HEAD 95a50bdf609186b7b0014593bdff3c24bfcdeca3, working-tree changes tested (2026-10-20). Other agents' installer verifiers were not run; shared hook wiring is unchanged. diff --git a/openspec/changes/remove-hook-test-runs/proposal.md b/openspec/changes/remove-hook-test-runs/proposal.md new file mode 100644 index 0000000..0e5e26e --- /dev/null +++ b/openspec/changes/remove-hook-test-runs/proposal.md @@ -0,0 +1,32 @@ +## Why +Automatic tests can take minutes in large repos and delay every agent reply. Test execution belongs to each repo's chosen pre-commit workflow. + +## What Changes +- Remove built-in Python, Node, Flutter and Go test execution from turn-end and commit batteries. +- Keep lint, types, structure checks, CDK synthesis, AI review and explicitly configured project commands. + +## Capabilities +### New Capabilities +None. +### Modified Capabilities +- `hook-wiring`: batteries validate code without automatically running tests. + +## Impact +Shared batteries affect every installed agent in place. Repos can explicitly configure CHECKPOINT commands for tests. + +## Scope +### May change +- `hooks/checks/post_task.sh`, `hooks/checks/lang_checks.sh`, `hooks/post_task_hook.sh`, `hooks/pre_commit_check.sh` +- `tests/test_hook_loops.sh`, `docs/hooks.md`, `config/personas/quality/references/checks.conf` +- `openspec/changes/remove-hook-test-runs/**` +- `dist/macols-personas-claude-plugin.zip`: regenerate the bundled quality template. +### Must not change +- Installer wiring, trust gates, per-edit checks, user-owned configuration and unrelated plugins. + +## Constraints +- C-EDIT-SOURCE-NOT-OUTPUT: hooks run from this source checkout. +- C-TRUSTED-REPO-CODE: explicit project commands retain trust checks. +- C-HOOKS-MODEL-VISIBLE: preserve finding delivery. +- C-SHELLCHECK-CLEAN: verify touched shell scripts. +- C-SPEC-ANCHORS-HEALTHY: keep anchors resolving and propose specification deltas. +- No new opt-in setting: automatic tests are removed for all repos. diff --git a/openspec/changes/remove-hook-test-runs/specs/hook-wiring/spec.md b/openspec/changes/remove-hook-test-runs/specs/hook-wiring/spec.md new file mode 100644 index 0000000..e5deebf --- /dev/null +++ b/openspec/changes/remove-hook-test-runs/specs/hook-wiring/spec.md @@ -0,0 +1,42 @@ +## ADDED Requirements + +### Requirement: Hook batteries do not automatically execute tests +Turn-end and commit batteries SHALL retain lint, type, layer, duplication, file-length and synthesis checks without automatically running Python, Node, Flutter or Go tests. Tests SHALL run only when explicitly requested by a repository command or another repository-owned workflow. Legacy test-scope settings SHALL NOT enable automatic test execution. + +#### Scenario: Code changes without configured test commands +- **WHEN** either battery runs in a trusted project with changed Python, Node, Flutter or Go code +- **THEN** no built-in test runner executes and applicable static checks still run + +## MODIFIED Requirements + +### Requirement: A local checkpoint runs before commits +Every tool's PreToolUse(Bash) wiring SHALL pass the command to +`hooks/pre_commit_check.sh`, which on `git commit` (not `--no-verify`, not +`MACOLS_CHECKPOINT=off`, trusted projects only) runs lint/type/layer checks, +synthesis, review and the project's explicit `CHECKPOINT` command from +`.macols/checks.conf`. A failure SHALL deny the commit with the findings as +the reason; a pass is remembered by tree fingerprint so an unchanged tree +is not re-checked. The turn-end battery SHALL run the project's `IMMEDIATE` +command. No tests SHALL run automatically in either battery. + + +#### Scenario: The agent commits a failing change +- **WHEN** the agent runs `git commit` in a trusted repo whose CHECKPOINT command fails +- **THEN** the commit is denied and the command's findings reach the model + +#### Scenario: A commit runs only the tests it reaches +- **WHEN** the agent runs `git commit` without an explicit test command in CHECKPOINT +- **THEN** the checkpoint runs its applicable validation checks without invoking tests, including when CHECKPOINT_TESTS=full was previously configured + +### Requirement: Every battery run is logged per check +`run_post_task_checks` SHALL append one JSON line per check, plus a `total` +line, to `macols-checks.jsonl` in the repo's common git dir (shared by +worktrees): time, repo, loop (`turn` or `checkpoint`), check, duration in ms +and `pass`/`fail`. The log SHALL stay bounded (newest 5000 lines once it passes +10000), `MACOLS_CHECK_LOG=off` SHALL turn it off, and `bin/macols-check-stats` +SHALL summarise time, failure rate, repeated failures and historical escapes per check. + + +#### Scenario: The user asks whether the hooks are worth their time +- **WHEN** the user runs `macols-check-stats` in a repo after some agent turns +- **THEN** it prints, per loop and check, runs, total and p95 time, failure rate and historical escaped tests diff --git a/openspec/changes/remove-hook-test-runs/tasks.md b/openspec/changes/remove-hook-test-runs/tasks.md new file mode 100644 index 0000000..5411065 --- /dev/null +++ b/openspec/changes/remove-hook-test-runs/tasks.md @@ -0,0 +1,6 @@ +## 1. Implementation +- [x] 1.1 Remove automatic test dispatch and Go test execution, keeping validation and project commands. +- [x] 1.2 Update hook documentation and project command template. +## 2. Verification +- [x] 2.1 Exercise both batteries and Go lint without automatic tests; verify explicit checkpoint failure and trust behaviour. +- [x] 2.2 Run syntax, shellcheck, installer verification and spec hygiene; record evidence. diff --git a/tests/test_hook_loops.sh b/tests/test_hook_loops.sh index 6d278d5..3400258 100755 --- a/tests/test_hook_loops.sh +++ b/tests/test_hook_loops.sh @@ -7,7 +7,7 @@ # --testmon-noselect at the checkpoint) # - a checkpoint failure the name-based selection would have missed is # recorded as an escape -# - go test covers the whole changed module at turn end, and an edit gets +# - batteries skip automatic tests; Go lint remains and an edit gets # go vet on its package # - the pre-deploy guard summarises `cdk diff` for a deploy, and says why # when it doesn't run it @@ -29,7 +29,7 @@ check() { if eval "$2"; then green "$1"; else red "$1"; fi; } FIXTURE="$(mktemp -d "${TMPDIR:-/tmp}/hook_loops.XXXXXX")" || exit 1 trap 'rm -rf "$FIXTURE"' EXIT -export MACOLS_TRUST_ALL=1 MACOLS_TRUST_FILE="$FIXTURE/trusted" MACOLS_DUPLICATION=off +export MACOLS_TRUST_ALL=1 MACOLS_TRUST_FILE="$FIXTURE/trusted" MACOLS_DUPLICATION=off MACOLS_LGREVIEW=off mkdir -p "$FIXTURE/stubbin" export PATH="$FIXTURE/stubbin:$PATH" @@ -127,6 +127,33 @@ esc=$(source "$LIB"; CHECKPOINT_MODE=1 PYTEST_FAIL=1 run_python_tests; printf '% check "checkpoint failure outside the selection is an escape" "printf '%s' \"\$esc\" | grep -qx 'root: tests/test_beta.py::test_beta'" check "checkpoint failure inside the selection is not" "! printf '%s' \"\$esc\" | grep -q test_alpha" +# Public hook entry points must not dispatch even installed test runners. +echo "batteries without automatic tests" +printf '{"scripts":{"test":"vitest"}}\n' > package.json +printf 'name: fixture\n' > pubspec.yaml +printf 'export const changed = 1;\n' > changed.ts +printf 'void main() {}\n' > changed.dart +mkdir -p node_modules/.bin test +TESTARGV="$FIXTURE/test-runners.argv" +for runner in vitest jest flutter; do + cat > "$FIXTURE/stubbin/$runner" <> '$TESTARGV' +exit 0 +STUB + chmod +x "$FIXTURE/stubbin/$runner" +done +ln -s "$FIXTURE/stubbin/vitest" node_modules/.bin/vitest +: > "$ARGV" +: > "$TESTARGV" +MACOLS_PYTEST_SCOPE=full bash "$REPO/hooks/post_task_hook.sh" --format text /dev/null +mkdir -p .macols +printf 'CHECKPOINT_TESTS=full\n' > .macols/checks.conf +MACOLS_LGREVIEW=off bash "$REPO/hooks/pre_commit_check.sh" 'git commit -am fixture' >/dev/null +check "turn and commit hooks never run pytest" "[ ! -s '$ARGV' ]" +check "turn and commit hooks never run Node or Flutter tests" "[ ! -s '$TESTARGV' ]" +rm "$FIXTURE/stubbin/vitest" "$FIXTURE/stubbin/jest" "$FIXTURE/stubbin/flutter" + # ── Go test scope ──────────────────────────────────────────────────────────── echo "go scope" new_repo "$FIXTURE/go" @@ -146,11 +173,11 @@ sed 's/"\$\*"/"golangci-lint \$*"/' "$FIXTURE/stubbin/go" > "$FIXTURE/stubbin/go chmod +x "$FIXTURE/stubbin/go" "$FIXTURE/stubbin/golangci-lint" : > "$GOARGV" ( source "$LIB"; run_go_checks ) -check "turn end tests the changed package" "grep -qx 'test ./a' '$GOARGV'" +check "turn end does not run Go tests" "! grep -q '^test ' '$GOARGV'" check "lint stays on the changed package" "grep -qx 'golangci-lint run ./a' '$GOARGV'" : > "$GOARGV" ( source "$LIB"; MACOLS_GO_TEST_SCOPE=module run_go_checks ) -check "MACOLS_GO_TEST_SCOPE=module tests the whole module" "grep -qx 'test ./...' '$GOARGV'" +check "legacy Go scope does not enable tests" "! grep -q '^test ' '$GOARGV'" # Edit time: go vet on the edited package, reported with its type errors # (which go vet prefixes with "vet: "), in trusted projects only. cat > "$FIXTURE/stubbin/go" <<'STUB'