Skip to content

ci: delete unread advisory workflows and the canary PR trigger - #4173

Merged
kyle-sexton merged 3 commits into
mainfrom
ci/delete-advisory-checks
Sep 15, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
ci/delete-advisory-checks

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

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 in ci-status): deleted, together with:
    • plugins/knowledge/skills/video-digest/extraction/liveness/** (the harness, its manifest, fixtures, test and LIVENESS.md). A repository grep found no user of the harness outside that directory and the workflow: no skill, no other test, no import. Its own run-source-liveness.test.js was 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.
    • The liveness:offline and liveness:live scripts in the extraction package.json.
    • Its glob in scripts/em-dash-purged-paths.txt. The purge gate fails on stale globs.
  • hook-utils-timing.yml (2 runs, results unread): deleted.
    • In lib/hook-utils.test.sh, removed both HOOK_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.
    • The load-independent chunk-boundary engagement probe and stall read-count probe stay as the regression guards (hook-utils tests: buffer_stdin sliced-vs-unsliced delta assertion flakes under machine load #2105). Comments that described the removed comparisons are updated.
  • silent-revert-canary.yml: removed only the pull_request trigger, the two if: 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 the hook-utils-timing.yml entry from the workflow-schema file list.
  • Docs:
    • docs/ci-runner-routing.md now names one Windows lane.
    • docs/architecture/landscape.json and landscape.md are updated to reference-edges.sh output for the edges this changes: actions/checkout 15 → 11, actions/setup-node 5 → 3, and the actions/upload-artifact edge removed. The rest of the record is left as generated.
  • Checked, no change needed: docs/specs/provenance-convention-engagement.md does not reference these workflows.
  • Left as dated records: the findings row naming LIVENESS.md in docs/specs/docs-hygiene-sweep-unapplied-remediations.md, and the plugins/knowledge/CHANGELOG.md history.

Verification

  • bash lib/hook-utils.test.sh: PASS=462 FAIL=0.
  • bash -n and shellcheck 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-cli2 on docs/ci-runner-routing.md and docs/architecture/landscape.md: 0 issues.
  • jq empty on the changed JSON: OK.
  • reference-edges.sh on origin/main and on this branch: they differ only in the three workflow edges above.
  • Not run locally: scripts/check-purged-em-dashes.sh hangs on this Windows host, so CI runs it. A script check confirmed that every remaining allowlist glob matches a tracked file.
  • A fresh-context verifier checked the branch against this scope. Its first pass named two stale comments in lib/hook-utils.test.sh; both are fixed. It passed on re-verification after the rebase.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_019mo9xaZ53JHHy4aBjgd8wK

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 14, 2026

Copy link
Copy Markdown

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-14T20:57:56.686710Z 15a4278 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 Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

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


/review:code-review — code-review lane

  • Gather context (PR body, diff via git diff HEAD~2 HEAD, repo instructions)
  • Invoke /review:code-review and apply its criteria/skip-gate
  • Check for dangling references left by the deletions
  • Post final summary (no inline findings)

Scope reviewed: all 18 changed files (+33/-1415), diffed directly against the PR's own two commits so the review isn't polluted by origin/main having advanced past this branch's base.

Findings: none that clear this lane's high-signal bar. Specifically checked and found clean:

  • No dangling references to the removed bs_median, bs_paired_estimate, bs_paired_verdict, bs_samples, bs_time_late_eof, bs_reads_on, bs_deltas_a_first/b_first, or HOOK_UTILS_TIMING anywhere in lib/hook-utils.test.sh or elsewhere in the repo.
  • No remaining references to video-digest-source-liveness.yml, hook-utils-timing.yml, or requirements-video-digest-liveness.txt in any workflow, script, or doc outside the two dated historical records the PR explicitly calls out (plugins/knowledge/CHANGELOG.md, docs/specs/docs-hygiene-sweep-unapplied-remediations.md).
  • No other workflow file references the two deleted workflows (no workflow_call/reusable dependency broken).
  • docs/architecture/landscape.json's file-sample swap (hook-utils-timing.ymlmanaged-files-guard.yml under the actions/checkout edge) checks out: managed-files-guard.yml does use actions/checkout, consistent with a regenerated sample list rather than a hand-edit error.
  • silent-revert-canary.yml: the removed if: github.event_name != 'pull_request' gates and the pull_request trigger are removed together consistently; the remaining push/workflow_dispatch triggers and step logic are unaffected.
  • .github/workflows/ci.yml's workflow-schema file list drops only the hook-utils-timing.yml entry, cleanly.

Not independently re-run: this lane's sandbox blocked direct script execution (bash -n, running the test suite) here, so the author's PASS=462 FAIL=0, shellcheck, actionlint, markdownlint-cli2, and reference-edges.sh results are author-claimed and unverified by me, not independently confirmed — the line-by-line diff read above is what backs this review's findings.

This is a well-scoped deletion PR; the removed code has no remaining callers and the accompanying doc/schema updates are internally consistent.

@claude

claude Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

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


Security review

  • Gather context (PR diff, changed files)
  • Apply skip gate
  • Review changed workflow files for security-relevant surface
  • Review deleted/modified scripts and docs
  • Post final findings

No security issues found.

This PR only deletes dead automation and tightens existing workflows — it adds no new attack surface:

  • video-digest-source-liveness.yml and hook-utils-timing.yml are deleted outright, along with the yt-dlp install, the VIDEO_DIGEST_YT_DLP_COOKIES_FILE secret handling, and the liveness harness/fixtures that lived under it. Removing a workflow that touched a secret and shelled out to yt-dlp reduces surface rather than adding any.
  • silent-revert-canary.yml removes the pull_request trigger and the two if: github.event_name != 'pull_request' gates it required — this trigger was a plain pull_request (not pull_request_target), scoped by paths to the canary's own files, so it never ran with elevated permissions or secrets against untrusted PR content. Its removal is cleanup, not a fix for a live issue, and does not weaken anything.
  • ci.yml only drops a now-deleted file path from a schema-validation file list.
  • The lib/hook-utils.test.sh, docs/**, package.json, and scripts/em-dash-purged-paths.txt changes are test/doc/script deletions with no security-relevant logic.

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.

@github-actions

Copy link
Copy Markdown
Contributor

Last security-reviewed head: 15a427862b4996b22bc159b48f4d136ab169f398. 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

Copy link
Copy Markdown
Contributor

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread docs/architecture/landscape.md
@kyle-sexton
kyle-sexton force-pushed the ci/delete-advisory-checks branch from 15a4278 to 56c1335 Compare September 14, 2026 21:13
kyle-sexton and others added 3 commits September 15, 2026 00:02
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
@kyle-sexton
kyle-sexton force-pushed the ci/delete-advisory-checks branch from 56c1335 to 9778847 Compare September 15, 2026 04:17
@kyle-sexton
kyle-sexton merged commit cae4980 into main Sep 15, 2026
12 checks passed
@kyle-sexton
kyle-sexton deleted the ci/delete-advisory-checks branch September 15, 2026 04:52
kyle-sexton added a commit that referenced this pull request Sep 15, 2026
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
kyle-sexton added a commit that referenced this pull request Sep 15, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant