chore(rate-limit-guard): commit the #2521 statusline benchmark harness (0.7.4)

[kyle-sexton]

Summary

PR #2521's headline render-path measurement — render.sh alone 234.4 ms vs 1047.1 ms behind the pre-#2521 tee (Windows/MSYS, n=9, same-window) — was produced by a benchmark harness that lived only in an untracked local scratch directory. The merged perf claim was unreproducible, and nothing could catch a regression that quietly reintroduced the render-path cost. This PR commits the harness.

• bench/lib-bench.sh — shared helpers: the spawn-floor control (median of 11 bare bash -c exit spawns, bracketing every timed section — on MSYS the process-creation floor dominates every number, so a run whose floor moved is discarded), the canonical statusline payload fixture, now_ms/median.
• bench/bench-idle.sh — the lane that produced the headline numbers: N sequential renders, floor before/after, median + mean + raw samples.
• bench/bench-load.sh — the concurrency lane: N virtual sessions rendering once a second for M seconds, the shape that stresses the spool/drain election.
• bench/trace-probe.sh — xtrace of a non-elected render, printing everything executed before passthrough: the check that the render path stays fork-free, which is the property perf(rate-limit-guard): spool the statusline snapshot, drain it on a cadence (0.7.0) #2521 exists to protect.
• bench/README.md — records what perf(rate-limit-guard): spool the statusline snapshot, drain it on a cadence (0.7.0) #2521 measured, on what platform, with what discipline, and how to re-run each lane (including isolating a run from the machine's live ~/.claude/rate-limit-guard/ contract file).

Plugin 0.7.3 → 0.7.4 with a changelog entry, per the shipped-contents-change convention.

Adaptation from the scratch originals

• STATUSLINE_ENTRY no longer defaults to a machine-local ~/.claude/statusline/entrypoint.sh; it defaults to this repo's scripts/statusline-tee.sh in standalone mode, resolved relative to the bench dir — runnable from a clean checkout. The env override is documented for measuring a real machine entrypoint.
• trace-probe.sh defaults its tee argument to the repo copy the same way (it already isolated itself under a throwaway HOME).
• # shellcheck disable=SC2034 on the payload fixture (consumed by the sourcing scripts), matching the hook-utils precedent.
• Everything else is byte-faithful to what produced the perf(rate-limit-guard): spool the statusline snapshot, drain it on a cadence (0.7.0) #2521 numbers. Verified against the tee at head: spool path, .last-drain stamp, RLG_TEE_DRAIN_INTERVAL (default 30), and the {"e":…,"p":…} record shape all still match.

Three scratch files were not brought over: gates.sh (throwaway wrapper around repo CI gates, hardcoded to a dead worktree path), pr-body.md (byte-identical to #2521's merged body), gates.log (empty).

CI stance

Deliberately no CI wiring: a wall-clock benchmark on shared runners is noise, not a gate. None of these files use the *.test.sh suffix that scripts/run-plugin-tests.sh and the CI test lanes discover, so nothing new runs or gates in CI beyond the standard shell-lint gates. The tee's behavioural coverage remains scripts/statusline-tee.test.sh.

Verification

• Smoke-ran from the clean worktree: bench-idle.sh 3 under an isolated HOME, and trace-probe.sh (pre-passthrough trace shows builtin-only work).
• shellcheck -x --rcfile=.shellcheckrc, shfmt -d, scripts/check-shell-portability.sh --paths on all four scripts: clean.
• markdownlint-cli2, typos, editorconfig-checker on changed files: clean.
• scripts/check-changelog-parity.sh --check / --check-order / --check-bump origin/main / --check-preserved origin/main: pass.
• scripts/validate-plugins.sh: all manifests + catalog pass.
• All 15 suites scripts/affected-tests.sh origin/main selected: pass.
• rate-limit-guard's own suites (statusline-tee.test.sh 96, statusline-shim.test.sh 36, record-rate-limit-stop.test.sh 19): all pass.

Related

Closes #2582

🤖 Generated with Claude Code

https://claude.ai/code/session_01Aac8xjCjMxFsXGHCXKHY4W

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 5m 7s —— View job

---

Claude's Task List

• Read PR context and diff
• Read repo CLAUDE.md/REVIEW.md/AGENTS.md conventions
• Run /review:code-review procedure over the diff (git diff origin/main...HEAD)
• Post inline findings
• Post final summary

Review summary

This lane is code-review-only here — .github/workflows/claude-security-review.yml exists in this repo, so security findings are scoped to that lane per REVIEW.md; none were reported by this pass.

Scope: the 4 new bench/*.sh scripts, bench/README.md, the 0.7.3 → 0.7.4 version bump, and the changelog entry. I read every changed file and cross-checked the two bench scripts that depend on scripts/statusline-tee.sh (lib-bench.sh's default STATUSLINE_ENTRY, trace-probe.sh's election-priming logic) against that script's actual drain/election code (_rlg_spool_dispatch/_rlg_shard_name, lines ~672–870) to confirm the harness's assumptions still hold — they do; trace-probe.sh's non-election setup and its sed -n '1,/set +o pipefail/p' trace cut both match the tee at HEAD.

Findings posted inline (2):

1. bench/lib-bench.sh:18 — now_ms() reads $EPOCHREALTIME (Bash ≥5.0 only) under this file's set -u. On any bash lacking it — notably stock macOS /bin/bash 3.2, which scripts/statusline-tee.sh itself explicitly special-cases for compatibility — this is a fatal unbound variable error that aborts the whole harness before any sample is taken, undercutting the "runnable from a clean checkout" goal.
2. bench/bench-load.sh:27 — the per-second pacing sleep mis-renders when a render completes in <1ms (spent == 0): printf '%03d' 1000 isn't truncated to 3 digits, so the sleep argument becomes "0.1000" (0.1s) instead of the intended 1.0s. Narrow edge case in practice given real process-spawn overhead, but a genuine order-of-magnitude bug in the interval math.

Not flagged (checked, no issue found): the median() even-length index choice (upper-middle vs. interpolated) is a reasonable simplification for a benchmarking tool, not a correctness bug; bench-load.sh's concurrent per-session log files have no shared-state/atomicity requirement (REVIEW.md's atomicity check); the trace-probe.sh shard-name/election priming (sess-42) matches the tee's actual _rlg_shard_name derivation.

Verification note: the PR body's shellcheck/shfmt/portability/test-suite results are author-claimed — this sandbox's Bash execution required manual approval that wasn't available in this run, so I reviewed the scripts statically rather than re-running that tooling; I'm not asserting independent confirmation of those results.
· branch chore/2582-rlg-bench-harness

[claude]

Security review complete

• Gather context (PR diff, changed files)
• Review new shell scripts (bench/*.sh) for security-relevant issues
• Check plugin manifest / changelog changes
• Post findings

No security issues found.

This PR adds a local benchmarking harness (bench/lib-bench.sh, bench/bench-idle.sh, bench/bench-load.sh, bench/trace-probe.sh, bench/README.md) plus a version/changelog bump. It has no security-relevant surface:

• No untrusted or externally-sourced input — all payloads (BENCH_PAYLOAD, the trace-probe.sh fixture) are fixed literal JSON strings baked into the scripts.
• STATUSLINE_ENTRY/TEE path overrides are read from the local operator's own environment to point the harness at their own statusline entrypoint — this is a same-trust-level convenience, not a trust-boundary crossing (no remote or unprivileged actor can influence it).
• Temp state uses mktemp -d with proper trap ... EXIT cleanup (bench-load.sh) or a throwaway $HOME (trace-probe.sh); no predictable paths or unsafe permissions.
• No GitHub Actions workflow files are touched, no secrets/tokens are handled, and nothing here is wired into CI (bench/README.md confirms none of these scripts use the *.test.sh suffix CI discovers).
• plugin.json/CHANGELOG.md changes are a straightforward version bump with no config/permission changes.

One non-security note for the author's awareness (not reported as a finding since it requires the operator to deliberately skip the documented isolation step, not an attacker-controlled path): bench-idle.sh/bench-load.sh don't default to a throwaway HOME the way trace-probe.sh does — the README correctly instructs setting HOME="$(mktemp -d)" before running them against a machine's live tee, but nothing in the scripts enforces that, so a run against the real entrypoint without following the README could drain synthetic bench-* session data into the real ~/.claude/rate-limit-guard/rate-limits.json contract file. Worth a self-review follow-up if desired, but out of scope for this security lane.

View job run

[github-actions]

Last security-reviewed head: 56e0280c38c77cebdeb523eed43f0b893c09328a. 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 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d1dd19e8f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 51s —— View job

---

Claude's Task List

• Read PR context and diff
• Invoke /review:code-review and follow its procedure exactly
• Post inline findings (none — see below)
• Post final summary

Review summary

Code-review lane only — this repo has a claude-security-review.yml lane, so security findings are scoped there per REVIEW.md; none reported here.

This review covers the delta since the last code-review pass on this PR (8d1dd19 → 56e0280, "harden the bench harness per review, add its contract suite"). I diffed the two commits directly and re-read every changed file at HEAD rather than re-reviewing the whole PR from scratch.

All four previously-reported findings are fixed, correctly:

1. EPOCHREALTIME unbound-variable abort (my earlier finding) — lib-bench.sh now guards with [[ -z "${EPOCHREALTIME:-}" ]] before any use and exits with a clear message, documented as a deliberate hard Bash ≥5.0 requirement in bench/README.md (any fallback like date +%s%3N would put a spawn inside the very thing being timed) — lib-bench.sh#L11-L17. Verified by a new contract test asserting a loud refusal, not an unbound-variable crash.
2. bench-load.sh pacing bug (0 ms spent → 0.1s sleep instead of 1.0s) (my earlier finding) — extracted into pace_sleep_arg(), which now computes rem = 1000 - spent and formats "$((rem/1000)).$(printf '%03d' $((rem%1000)))", correctly producing "1.000" for spent=0 — lib-bench.sh#L45-L52. Covered by three new unit-test cases including the exact spent=0 regression. Confirmed the only call site (bench-load.sh) always calls it with spent < 1000, so the theoretical negative-rem branch is unreachable.
3. Codex: timer reads inside command substitutions add a dominant fork — now_ms() now assigns into a caller-provided variable via printf -v instead of $(now_ms), eliminating the extra subshell fork on every timed bracket — lib-bench.sh#L33-L36. bench/README.md adds an "Instrument note" honestly caveating that this changes absolute numbers vs. the historical perf(rate-limit-guard): spool the statusline snapshot, drain it on a cadence (0.7.0) #2521 baseline (but not the delta the claim rests on) — good call to document rather than silently reconcile.
4. Codex: failed renders reported as fast samples — render_once() now propagates the render's exit status (relies on pipefail, already set), and both bench-idle.sh and bench-load.sh check it and abort the run instead of recording a sample — bench-idle.sh#L16-L19, bench-load.sh#L27-L30.
5. Codex: scripts unmapped in affected-tests.sh coverage — the new bench/bench.test.sh (135 lines) follows the *.test.sh convention scripts/run-plugin-tests.sh discovers, and its text references lib-bench.sh, bench-idle.sh, bench-load.sh, and trace-probe.sh by basename, which satisfies affected-tests.sh's R3 "referenced" selection rule — I confirmed this by reading scripts/affected-tests.sh's rule set and grepping the new test file for those basenames rather than trusting the README's claim. bench/README.md's "CI" section is updated accordingly (was "Not CI", now documents bench.test.sh as the contract suite that runs).

New code checked, no issues found: the new bench.test.sh contract suite itself (isolates HOME, small BENCH_FLOOR_N, asserts behavior/output shape not timing, exercises both the success and failing-render path for each lane) is well-targeted and consistent with the harness it tests. trace-probe.sh's mode change (100644 → 100755) is a harmless direct-executable fix. CHANGELOG.md/plugin.json stay internally consistent (0.7.4, single version bump, changelog entry updated in place to describe the hardening).

Verification note: this sandbox's Bash execution required manual approval not available in this run (including for shellcheck/the new test suite itself), so — as in the prior pass — I reviewed statically: read every changed line in both commits, cross-checked the fixes against the specific bug mechanics originally reported, and traced scripts/affected-tests.sh's actual selection rules against the new test file's content rather than accepting the PR's/README's claims at face value. I did not independently execute bench.test.sh, shellcheck, or shfmt; the PR body's tool-run results remain author-claimed.

No inline findings to post — the two prior code-review findings are resolved and no new issues surfaced in this round.
· branch chore/2582-rlg-bench-harness

[claude]

Claude finished @kyle-sexton's task in 1m 8s —— View job

---

Claude's Task List

• Read PR context and diff
• Invoke /review:security-review procedure exactly, per custom instructions
• Determine whether new security-relevant surface exists since the last reviewed head
• Post final summary

Security review result: skip gate — no new security-relevant surface

Per /review:security-review's skip gate, the last successful security review on this PR covered head 8d1dd19 (bench harness commit) and found no issues. Since then, the only new commit is 56e0280 ("harden the bench harness per review, add its contract suite"), which is exactly what its title says: it fixes the four review findings already posted against 8d1dd19 (2 from Claude's code review, 2 from Codex) and adds bench/bench.test.sh. I diffed 8d1dd19..56e0280 directly and read every changed hunk:

• lib-bench.sh — now_ms now assigns via printf -v instead of $(...) (removes an extra fork from timings, not a trust-boundary change); adds a loud bash >= 5.0 refusal instead of an unbound-variable crash when EPOCHREALTIME is missing; new render_once/pace_sleep_arg helpers factor out existing logic and fix the sleep-arithmetic bug.
• bench-idle.sh / bench-load.sh — now check render_once's exit status and abort the lane (exit 1) instead of silently timing a failed render as a sample.
• bench.test.sh (new, 135 lines) — a contract smoke suite for the above, run under mktemp -d / isolated HOME, using only fixed literal payloads. No untrusted input, no eval, no new temp-file races (proper trap ... EXIT cleanup).
• README.md / CHANGELOG.md — documentation only.

None of this touches trust boundaries, secrets/tokens, authorization, injection surfaces, or GitHub Actions workflow config — it's purely internal hardening of a local, non-CI-gated wall-clock benchmarking tool, addressing feedback already given. Per the skip gate, no new inline findings are posted.