feat: turn the 400-line diff cap into a 1000-line runaway guard excluding tests #470

Merged

igor merged 1 commit from agent/467-feat-diff-cap-1000-line-runaway-guard-excluding into master

2026-08-09 01:56:39 +00:00

igor commented

2026-08-09 01:13:32 +00:00

Collaborator

What this PR does

feat: turn the 400-line diff cap into a 1000-line runaway guard excluding tests
Add lib/scope-gate.sh: is_test_path classifier + scope_gate_sum_numstat, used by the finalize-time gate in bin/tick.sh (threshold now SCOPE_GATE_MAX_LINES=1000, counted via git diff --numstat instead of --shortstat)
Rewrite the AGENTS.md worker contract: diffs should be the size the task honestly requires (no padding, no drive-by refactors), the 1000-non-test-line hard block is a runaway guard not a target, and add explicit comment discipline (comments state a non-obvious why/invariant only; no narration, no changelog-style comments)
Add two review dimensions to bin/lib/review-directive.md: diff-appropriateness (padding/redundant abstraction/drive-by changes are findings regardless of diff size) and comment bloat (narrating/unnecessary comments are findings; pervasive bloat licenses REQUEST_CHANGES on its own)
Update the other live references to the old 400-line number for consistency: CLAUDE.md, docs/architecture.md, bin/lib/ceo-digest-directive.md, lib/security-gate.sh
Tests: new bin/test-scope-gate.sh covering the classifier (positive/negative paths for every pattern) and the numstat summer (test-file exclusion, lockfile/dist exclusion, binary files, empty input)

Test plan

make test passes (bin/check-sync.sh outcome-sentinel/helper contract check + all bin/test-*.sh, including the new suite)
make lint passes (shellcheck + mdl)
No manual verification needed beyond the above; this is pure shell logic + prompt-surface text with no UI

Closes #467

## What this PR does - [x] feat: turn the 400-line diff cap into a 1000-line runaway guard excluding tests - [x] Add `lib/scope-gate.sh`: `is_test_path` classifier + `scope_gate_sum_numstat`, used by the finalize-time gate in `bin/tick.sh` (threshold now `SCOPE_GATE_MAX_LINES=1000`, counted via `git diff --numstat` instead of `--shortstat`) - [x] Rewrite the `AGENTS.md` worker contract: diffs should be the size the task honestly requires (no padding, no drive-by refactors), the 1000-non-test-line hard block is a runaway guard not a target, and add explicit comment discipline (comments state a non-obvious why/invariant only; no narration, no changelog-style comments) - [x] Add two review dimensions to `bin/lib/review-directive.md`: diff-appropriateness (padding/redundant abstraction/drive-by changes are findings regardless of diff size) and comment bloat (narrating/unnecessary comments are findings; pervasive bloat licenses REQUEST_CHANGES on its own) - [x] Update the other live references to the old 400-line number for consistency: `CLAUDE.md`, `docs/architecture.md`, `bin/lib/ceo-digest-directive.md`, `lib/security-gate.sh` - [x] Tests: new `bin/test-scope-gate.sh` covering the classifier (positive/negative paths for every pattern) and the numstat summer (test-file exclusion, lockfile/dist exclusion, binary files, empty input) ## Test plan - [x] `make test` passes (`bin/check-sync.sh` outcome-sentinel/helper contract check + all `bin/test-*.sh`, including the new suite) - [x] `make lint` passes (`shellcheck` + `mdl`) - [x] No manual verification needed beyond the above; this is pure shell logic + prompt-surface text with no UI Closes #467

igor added 1 commit

2026-08-09 01:13:32 +00:00

feat: turn the 400-line diff cap into a 1000-line runaway guard excluding tests

Lint / check-sync (push) Successful in 6s

Details

Lint / check-sync (pull_request) Successful in 5s

Details

e2d4af54f4

igor commented

2026-08-09 01:16:54 +00:00

Author

Collaborator

🤖 Review — `APPROVE` (automated)

CI for e2d4af54: success

Replaces the finalize-time 400-line scope cap with a 1000-non-test-line runaway guard, extracting the counting logic into lib/scope-gate.sh with unit tests, and updates the worker contract / review directive / docs to match. CI green, diff ~250 lines, scope matches issue #467.

Verified against the contract

Checklist is honest: every checked item maps to real hunks (lib/scope-gate.sh, bin/tick.sh, AGENTS.md, bin/lib/review-directive.md, CLAUDE.md, docs/architecture.md, bin/lib/ceo-digest-directive.md, lib/security-gate.sh, bin/test-scope-gate.sh). No fabricated boxes found.
check-sync.sh contract preserved: the AGENTS.md rewrite doesn't touch any OUTCOME: sentinel, # OUTCOME: blocked is still on the block branch in tick.sh, and agent-block.sh is still referenced by name in AGENTS.md so the helper-existence grep still fires.
Counting semantics are equivalent to the old --shortstat path: the lockfile/dist/build exclusions are reproduced as case patterns covering both top-level and nested forms, binary (-\t-) rows contribute 0, empty input yields 0. Test arithmetic (10+5 + 3+1 = 19) checks out.
set -e safety in scope_gate_sum_numstat is fine: [ -z "$path" ] && continue and is_test_path "$path" && continue both fail on the left side of &&, which is exempt from errexit.
Classifier tests cover a positive case for each of the five patterns plus plausible negatives (latest-version.sh, contest.js, foo_test.txt).

Non-blocking notes

lib/scope-gate.sh:36 — git diff --numstat renders renames as pkg/{old => new}.go (rename detection is on by default). Such a row won't match the lockfile/dist case patterns, and a {src => test}/foo.go rename won't match (^|/)test[s]?/. Both fail conservatively (counted rather than skipped), so the guard can only over-count, but it's a latent surprise if someone later relies on the exclusions being exact. --numstat --no-renames would remove the ambiguity.
lib/scope-gate.sh:28 — (^|/)spec/ will classify non-test spec/ trees (e.g. OpenAPI specs) as test files, silently exempting them from the guard. Acceptable given the issue hardcoded this list, worth knowing.
is_test_path forks a grep per changed path; on a genuinely runaway branch (thousands of files) that's thousands of subprocesses. Fine in practice, but a bash case/[[ =~ ]] would avoid it.
The igor#411/#465 rationale now appears three times (lib header, tick.sh gate comment, AGENTS.md). The issue asked for it at the gate, so this isn't a violation, but by the comment-discipline rule this same PR introduces, the tick.sh copy could be a one-liner pointing at lib/scope-gate.sh.
No test exercises the tick.sh wiring (CHANGED -gt $SCOPE_GATE_MAX_LINES and the pipe from git diff --numstat); coverage stops at the pure functions. Consistent with how the rest of tick.sh is (not) tested, and the comparison is trivially readable in the diff.

Nothing here blocks. Approving.

_{Independent review by the harness on claude-opus-5 (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `APPROVE` _(automated)_ CI for `e2d4af54`: **success** Replaces the finalize-time 400-line scope cap with a 1000-non-test-line runaway guard, extracting the counting logic into `lib/scope-gate.sh` with unit tests, and updates the worker contract / review directive / docs to match. CI green, diff ~250 lines, scope matches issue #467. **Verified against the contract** - Checklist is honest: every checked item maps to real hunks (`lib/scope-gate.sh`, `bin/tick.sh`, `AGENTS.md`, `bin/lib/review-directive.md`, `CLAUDE.md`, `docs/architecture.md`, `bin/lib/ceo-digest-directive.md`, `lib/security-gate.sh`, `bin/test-scope-gate.sh`). No fabricated boxes found. - `check-sync.sh` contract preserved: the AGENTS.md rewrite doesn't touch any `OUTCOME:` sentinel, `# OUTCOME: blocked` is still on the block branch in `tick.sh`, and `agent-block.sh` is still referenced by name in AGENTS.md so the helper-existence grep still fires. - Counting semantics are equivalent to the old `--shortstat` path: the lockfile/`dist`/`build` exclusions are reproduced as `case` patterns covering both top-level and nested forms, binary (`-\t-`) rows contribute 0, empty input yields 0. Test arithmetic (`10+5 + 3+1 = 19`) checks out. - `set -e` safety in `scope_gate_sum_numstat` is fine: `[ -z "$path" ] && continue` and `is_test_path "$path" && continue` both fail on the *left* side of `&&`, which is exempt from `errexit`. - Classifier tests cover a positive case for each of the five patterns plus plausible negatives (`latest-version.sh`, `contest.js`, `foo_test.txt`). **Non-blocking notes** - `lib/scope-gate.sh:36` — `git diff --numstat` renders renames as `pkg/{old => new}.go` (rename detection is on by default). Such a row won't match the lockfile/`dist` `case` patterns, and a `{src => test}/foo.go` rename won't match `(^|/)test[s]?/`. Both fail *conservatively* (counted rather than skipped), so the guard can only over-count, but it's a latent surprise if someone later relies on the exclusions being exact. `--numstat --no-renames` would remove the ambiguity. - `lib/scope-gate.sh:28` — `(^|/)spec/` will classify non-test `spec/` trees (e.g. OpenAPI specs) as test files, silently exempting them from the guard. Acceptable given the issue hardcoded this list, worth knowing. - `is_test_path` forks a `grep` per changed path; on a genuinely runaway branch (thousands of files) that's thousands of subprocesses. Fine in practice, but a bash `case`/`[[ =~ ]]` would avoid it. - The igor#411/#465 rationale now appears three times (lib header, `tick.sh` gate comment, AGENTS.md). The issue asked for it at the gate, so this isn't a violation, but by the comment-discipline rule this same PR introduces, the `tick.sh` copy could be a one-liner pointing at `lib/scope-gate.sh`. - No test exercises the `tick.sh` wiring (`CHANGED -gt $SCOPE_GATE_MAX_LINES` and the pipe from `git diff --numstat`); coverage stops at the pure functions. Consistent with how the rest of `tick.sh` is (not) tested, and the comparison is trivially readable in the diff. Nothing here blocks. Approving. --- <sub>Independent review by the harness on `claude-opus-5` (effort: high). The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-08-09 01:16:54 +00:00

2 minutes 15 seconds

igor merged commit cc70c6b886 into master

2026-08-09 01:56:39 +00:00

igor deleted branch agent/467-feat-diff-cap-1000-line-runaway-guard-excluding

2026-08-09 01:56:40 +00:00

igor referenced this pull request from a commit

2026-08-09 01:56:40 +00:00

Merge pull request 'feat: turn the 400-line diff cap into a 1000-line runaway guard excluding tests' (#470) from agent/467-feat-diff-cap-1000-line-runaway-guard-excluding into master