perf(ci): gate the statusline bench lanes behind BENCH_LANES

[kyle-sexton]

No related issue: acceptance criterion 2 of the CI performance program, tracked in melodic-software/github-iac#378; no issue in this repository owns it.

Summary

Acceptance criterion 2 requires this repository's ci.yml to keep benchmarks "gated behind an env var". Nothing gated them. plugins/rate-limit-guard/bench/bench.test.sh is in the contract corpus, so both runner paths — scripts/run-plugin-tests.sh on a push and scripts/affected-tests.sh --run on a pull request — spawned all three benchmark lanes (bench-idle.sh, bench-load.sh, trace-probe.sh) whenever the suite was selected. BENCH_FLOOR_N is the spawn-count knob those lanes read, not a gate: the suite exports it itself and the lanes run either way.

Fix

BENCH_LANES gates the lane runs inside the suite, at plugins/rate-limit-guard/bench/bench.test.sh:106, after the lib-bench.sh assertions and before the first lane spawn. Unset, the suite runs its eight pure-function cases and stops. Set, it runs all thirteen.

The gate is in the test script, not in the serial list or the job step, because the script is the only place both runner paths pass through. run-plugin-tests.sh discovers the suite by glob and affected-tests.sh selects it independently; both then run bash "$suite" with the environment inherited. The serial list chooses serial-versus-parallel, not whether a suite runs, and an entry there naming no discovered suite is an error. A gate in the ci.yml step would have to carve one path out of a glob the runner owns, and would leave the pull-request selector path ungated.

No new workflow. The first commit on this branch added a weekly bench-harness.yml to run the gated lanes. #4173 deleted two workflows of exactly that shape three commits ago, under the owner direction of 2026-09-14 to delete stale and overengineered automation, and hook-utils-timing.yml went specifically for having had two runs nobody read. Adding another scheduled advisory lane would be the same thing again, so the second commit removes it. The deliberate CI run is a workflow_dispatch input on ci.yml instead: bench_lanes, threaded into the contract-test step as BENCH_LANES: ${{ inputs.bench_lanes && '1' || '' }}. Nothing runs on a schedule and nothing goes unread; the lanes run when someone asks for them, over the full corpus a dispatch already runs.

The deferral is a SKIP: line, not an ok. run-plugin-tests.sh reads ^SKIP: and names the suite under "Suites with skipped coverage (exit 0 here is NOT evidence those cases ran)", which is exactly what this is: five cases that did not run. Counting it as a pass would make the aggregate read as full coverage. --strict-skips consequently fails on the suite, which is correct: a caller declaring a fully provisioned environment is asking for every case to run, and BENCH_LANES=1 is how it gets them. No CI lane passes that flag.

What this costs. That suite is the guard that the harness still runs from a clean checkout — the defect behind the unreproducible measurements in #2521. On an ordinary pull request that guard is now the lib-helper half plus repo-wide ShellCheck; the end-to-end half runs on a dispatch. The benchmark is not deleted and loses no capability.

scripts/run-plugin-tests-serial.txt keeps its entry, with the reason rewritten: an ordinary run no longer needs a serial seat, but a BENCH_LANES=1 run does, and that is when the lanes measure exactly what contention distorts. bench/README.md describes the split, and the plugin takes a version bump with its changelog entry.

Verification

Local, in a clean worktree off origin/main:

• bash plugins/rate-limit-guard/bench/bench.test.sh: PASS=8 FAIL=0 with SKIP: bench lanes deferred, exit 0, 0.6 s.
• BENCH_LANES=1 bash plugins/rate-limit-guard/bench/bench.test.sh: PASS=13 FAIL=0, every lane case executed, 4.6 s.
• shellcheck -x and shfmt -d clean on the suite; actionlint and zizmor --persona=regular clean on ci.yml; markdownlint-cli2 clean on both changed markdown files, which are on scripts/em-dash-purged-paths.txt and gained no em dash.
• bash scripts/run-plugin-tests.test.sh: PASS=67 FAIL=0, including the case proving every serial-allowlist entry names an existing suite.
• bash scripts/check-lane-coverage.sh --check and bash scripts/check-docs-only-gate.sh --check: clean. scripts/check-changelog-parity.sh passes --check, --check-bump origin/main and --check-preserved origin/main.
• A fresh-context verifier reviewed the first commit against this scope and named seven defects; every one is answered by the second commit, including the two that this body's "no new workflow" and "SKIP: line" paragraphs record. It re-verified the branch afterwards.

Measured effect, from real runs rather than from the local numbers above. The suite always lands on test-linux (1): it is index 217 of 347 discovered suites and 217 mod 4 = 1. Across the three most recent successful push runs of ci.yml on main (34939066292, 34930491268, 34925496845) its elapsed time inside the serial phase, measured from the preceding serial suite's terminal PASS: line to its own, was 1.170 s, 1.133 s and 1.157 s: median 1.157 s. The median test-linux (N) wall time across all twelve legs of those runs is 170 s.

So the honest size of this change is about one second on one leg of a 170-second job, inside that job's run-to-run noise. It is worth having for the criterion and for what it stops — wall-clock lanes spawning on shared runners on every pull request that touches the plugin — and this body claims no throughput win. The after-number from this pull request's own run is posted as a comment once it lands, measured the same way.

Related

Refs: melodic-software/github-iac#378

Refs: #4173

Refs: #2582

Refs: #2521

🤖 Generated with Claude Code

https://claude.ai/code/session_01M98vWnd3jQJ19Pe6Uj7bV9

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-15T13:30:12.191656Z	37db04a	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @kyle-sexton's task in 4m 13s —— View job

---

Code review complete

• Read repo instructions (CLAUDE.md / AGENTS.md) and invoke /review:code-review
• Check skip gate (draft state, security-lane split, prior review) — PR is open/non-draft; .github/workflows/claude-security-review.yml exists, so security scope is correctly left to that lane
• Fetch PR diff and read changed files (git diff d4fc675 HEAD, matches the PR's reported 74/-11 across 6 files)
• Review for correctness / maintainability issues
• Post findings

Findings

No blocking or high-signal issues found.

Traced the logic the PR body describes and confirmed it holds:

• The BENCH_LANES gate in bench.test.sh sits after the lib-helper assertions and before the first lane spawn, so an unset var stops the suite at 8 cases with a SKIP: line and a set var runs all 13. The shared summary() helper (L35-40) preserves the original exit-code contract (PASS/FAIL counts, [[ $FAIL -eq 0 ]] as the return status) on both the early-exit and full-run paths.
• --strict-skips is not passed anywhere in ci.yml or in any repo hook against the real suite corpus (only against synthetic fixtures in run-plugin-tests.test.sh), so the new permanent SKIP: on ungated runs is informational only and won't red any check.
• The new workflow_dispatch trigger in ci.yml and its BENCH_LANES: ${{ inputs.bench_lanes && '1' || '' }} threading (L1508) compose safely with every existing github.event.pull_request.* / github.base_ref reference in the file — they already null-safely degrade on non-pull_request events (that's how push has always worked), and the one step that fetches a base ref (checkout-with-base) is explicitly gated on github.event_name == 'pull_request'. A dispatch takes the run-plugin-tests.sh full-corpus path, not affected-tests.sh, so BENCH_LANES=1 reaches the suite as intended.
• plugin.json version bump, CHANGELOG.md entry, bench/README.md prose, and the rewritten run-plugin-tests-serial.txt comment are all consistent with the actual code change and with each other.

Not independently verified: I was unable to execute shell commands in this sandbox (bash plugins/rate-limit-guard/bench/bench.test.sh and similar required approval I couldn't grant), so the PASS/FAIL counts, timing numbers, and lint-tool output in the PR's Verification section are author-claimed and unconfirmed by me — the static trace above is what I could independently check.

[claude]

Claude finished @kyle-sexton's task in 47s —— View job

---

Security review — PR #4178

• Gather context (diff via gh pr diff, read changed files)
• Run skip-gate check
• Report result

Result: no security-relevant surface.

Reviewed all 6 changed files (.github/workflows/ci.yml, plugins/rate-limit-guard/.claude-plugin/plugin.json, plugins/rate-limit-guard/CHANGELOG.md, plugins/rate-limit-guard/bench/README.md, plugins/rate-limit-guard/bench/bench.test.sh, scripts/run-plugin-tests-serial.txt).

• The new workflow_dispatch input (bench_lanes, type: boolean) can only be triggered manually by someone who already has write access — it's not pull_request_target/workflow_run processing untrusted input, and a boolean input can't carry an expression-injection payload into the BENCH_LANES: ${{ inputs.bench_lanes && '1' || '' }} env assignment.
• BENCH_LANES is consumed in bench.test.sh only via a plain ${BENCH_LANES:-} bash check — no eval, no command construction from the value, nothing attacker-influenced.
• No permissions: widening, no secrets touched, no new trigger type comparable to pull_request_target.
• The remaining files are a version bump, changelog entry, README prose, and a comment-only edit to the serial allowlist — no logic with security surface.

This falls under the skip gate ("change has no security-relevant surface after reading the diff"), so no findings to report. GitHub Actions hardening concerns (if any) are zizmor's lane, not this one, and none of the above rise to needing that reasoning anyway.

[github-actions]

Last security-reviewed head: 37db04a221836320e45e1d81ddf134f5c6299d2b. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[github-actions]

Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[kyle-sexton]

After-number, from this pull request's own run (34976006943, head d8c0343).

test-linux (1) is the leg that carries the suite. In its "Run plugin contract tests" step the selector printed:

13:39:18.9488284Z === plugins/rate-limit-guard/bench/bench.test.sh ===
13:39:18.9665627Z SKIP: bench lanes deferred; set BENCH_LANES=1 to run them
13:39:18.9680036Z PASS: plugins/rate-limit-guard/bench/bench.test.sh

0.019 s, against a 1.157 s median before (the three most recent successful push runs of ci.yml on main: 34939066292, 34930491268, 34925496845). The residual 19 ms is the eight lib-helper assertions, which still run on every pull request.

One caveat on the comparison, because the two numbers come off different code paths. A pull request runs affected-tests.sh --run, which prints the === suite === marker BEFORE the suite executes, so marker-to-PASS: is the suite's own elapsed. A push runs run-plugin-tests.sh, which captures each suite's output and replays the whole block after the suite finishes, so the marker there is worthless and the before-number is measured from the preceding serial suite's terminal PASS: line instead. Both measure the suite's own wall time; neither is the other's rerun.

The job totals are not a comparison and are not offered as one. This run's step ran 46 selected suites in 227 s on that leg; the main runs sharded the full corpus, median test-linux (N) 170 s. One second off one leg stays inside that job's run-to-run spread, which is what the body said to expect.