ci: delete unread advisory workflows and the canary PR trigger - #4173
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 2m 48s —— View job
|
|
Claude finished @kyle-sexton's task in 55s —— View job Security review
No security issues found. This PR only deletes dead automation and tightens existing workflows — it adds no new attack surface:
Deferring GitHub Actions hardening findings (triggers, permissions, pinning) to zizmor per this lane's scope — none apply here regardless, since this diff only removes triggers and permissions, never adds them. |
|
Last security-reviewed head: |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15a427862b
ℹ️ 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".
15a4278 to
56c1335
Compare
video-digest-source-liveness.yml ran red on its schedule for five weeks outside ci-status; delete it with its liveness harness, its requirements file, the package.json aliases and the em-dash allowlist glob. Nothing outside the workflow used the harness. hook-utils-timing.yml ran twice with nobody reading the result; delete it with the HOOK_UTILS_TIMING-gated clock comparisons in lib/hook-utils.test.sh and the pairing and estimator helpers only they used. The load-independent engagement and read-count probes stay as the regression guards. silent-revert-canary.yml keeps its push-time canary and loses only the pull_request self-test trigger, its now-dead event gates and comments. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019mo9xaZ53JHHy4aBjgd8wK
The FIFO handshake note and the no-FIFO fallback hold still referred to the repeated sampling and the stall comparison's slow arm; the worst case is now the stall read-count probe's unsliced run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019mo9xaZ53JHHy4aBjgd8wK
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XwY46aTKXzw9wrLhVC1Azz
56c1335 to
9778847
Compare
The first commit here added a weekly bench-harness workflow to run the gated lanes. #4173 deleted the two workflows of exactly that shape three commits ago, under the owner direction to delete stale and overengineered automation, and hook-utils-timing was deleted for having two runs nobody read. A new scheduled advisory lane would be the same thing again. So ci.yml gains a workflow_dispatch `bench_lanes` input instead, threaded into the contract-test step as BENCH_LANES. Nothing runs on a schedule, nothing goes unread, and the lanes still have a deliberate CI run. The deferral now prints a SKIP line rather than counting an ok, so the runner's aggregate names the coverage that did not run instead of reading as a full pass. bench/README.md describes the split, and the plugin takes a version bump with its changelog entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M98vWnd3jQJ19Pe6Uj7bV9
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.com/claude-code) https://claude.ai/code/session_01M98vWnd3jQJ19Pe6Uj7bV9 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
No linked issue: owner direction 2026-09-14 to delete stale and overengineered automation.
Summary
This deletes two advisory workflows whose output nobody reads, removes the timing branches and helpers that existed only for one of them, and removes the pull-request self-test trigger from the silent-revert canary. None of these lanes is part of
ci-status.Fix
video-digest-source-liveness.yml(scheduled runs red 5 weeks running, not inci-status): deleted, together with:plugins/knowledge/skills/video-digest/extraction/liveness/**(the harness, its manifest, fixtures, test andLIVENESS.md). A repository grep found no user of the harness outside that directory and the workflow: no skill, no other test, no import. Its ownrun-source-liveness.test.jswas picked up by the extraction package's vitest glob, but it tested only the harness..github/requirements-video-digest-liveness.txt, which only the workflow read.liveness:offlineandliveness:livescripts in the extractionpackage.json.scripts/em-dash-purged-paths.txt. The purge gate fails on stale globs.hook-utils-timing.yml(2 runs, results unread): deleted.lib/hook-utils.test.sh, removed bothHOOK_UTILS_TIMING-gated clock comparisons: late-EOF and stall overshoot. Also removed the helpers only they used (bs_median,bs_paired_estimate,bs_paired_verdict,bs_samples,bs_time_late_eof,bs_reads_on) and Test 18b(i), which exercised only that estimator.silent-revert-canary.yml: removed only thepull_requesttrigger, the twoif: github.event_name != 'pull_request'gates it made necessary, and the comments that explained it. The push-to-main canary is unchanged: self-test, incident replay, range scan and restoration assertion.ci.yml: removed only thehook-utils-timing.ymlentry from the workflow-schema file list.docs/ci-runner-routing.mdnow names one Windows lane.docs/architecture/landscape.jsonandlandscape.mdare updated toreference-edges.shoutput for the edges this changes:actions/checkout15 → 11,actions/setup-node5 → 3, and theactions/upload-artifactedge removed. The rest of the record is left as generated.docs/specs/provenance-convention-engagement.mddoes not reference these workflows.LIVENESS.mdindocs/specs/docs-hygiene-sweep-unapplied-remediations.md, and theplugins/knowledge/CHANGELOG.mdhistory.Verification
bash lib/hook-utils.test.sh:PASS=462 FAIL=0.bash -nandshellcheck lib/hook-utils.test.sh: clean.actionlint: exit 0, including after rebasing onto ci: hand ShellCheck the raw pull-request diff so non-shell diffs skip the whole-repo scan #4167.markdownlint-cli2ondocs/ci-runner-routing.mdanddocs/architecture/landscape.md: 0 issues.jq emptyon the changed JSON: OK.reference-edges.shonorigin/mainand on this branch: they differ only in the three workflow edges above.scripts/check-purged-em-dashes.shhangs on this Windows host, so CI runs it. A script check confirmed that every remaining allowlist glob matches a tracked file.lib/hook-utils.test.sh; both are fixed. It passed on re-verification after the rebase.Related
link-check.yml)🤖 Generated with Claude Code
https://claude.ai/code/session_019mo9xaZ53JHHy4aBjgd8wK