feat(ci): close five ungated seams found by the coupling pass by kyle-sexton · Pull Request #3464 · melodic-software/claude-code-plugins

kyle-sexton · 2026-08-28T07:23:43Z

No linked issue

Summary

Five independent route-lane findings from the /coupling:reduce pass merged in #3457. Each is a rule the repo already states somewhere and nothing enforces; three had already drifted. One atomic commit per item.

Fix

1. Vendored intra-plugin source had no version-bump gate. ADR 0019 states that editing a plugin's shared vendor/ source obligates a version bump, because the version is the update cache key — and that this invariant replaces the byte-drift gate for the intra-plugin case. Seven --check-bump lanes covered only cross-plugin clusters. Both claimed drifts reproduce: b3445bc2 changed vendor/video-digestion/frames/scene-detect.js with plugin.json at 0.10.9 before and after (its release note was folded into an already-released section; the commit message's claimed 0.10.7→0.10.8 bump is not in the commit), and b01dace3 changed vtt-parser.js at 0.11.0 with no changelog edit. New scripts/check-vendor-version-bump.sh, general over plugins/*/vendor/, with a new-plugin carve-out and a deleted-manifest case that fails rather than skips.

2. The review skip-actors list was restated at five sites and had drifted. scripts/verify-security-review-evidence.sh's default was missing cursor[bot], which 2b4d8abf added to both workflow lines only. Now one owned data file, .github/claude-skip-actors, with a single fail-closed parser. The reusable workflow's with: skip-actors cannot read a file, so a prep job reads the PR's base copy (mirroring the reusable's own paths-file discipline) and passes it across needs, with a fail-closed backstop if the read job did not succeed.

3. python-version: '3.14' was a bare literal at five sites while every Node lane resolved through .node-version. Adds .python-version, switches all five setup-python steps to python-version-file, and makes .github/requirements-ci.txt state the interpreter↔cp314-hash pairing that was previously only implied.

4. Two live check scripts were wired into no workflow. check-rename-sweep.sh (git grep matched only its own two files) and check-discriminating-test-skips.sh (cited in a shipped SKILL.md as "this marketplace repo's own CI gate"). Both confirmed exit-0 on the current tree before wiring, so neither breaks main. Now rename-sweep-gate and discriminating-test-skip-gate, self-test first then check, both in ci-status.needs.

5. CATEGORY_ORDER restated the taxonomy document's vocabulary and order in a document that claims consumers "never restate its definitions". The generator now parses the doc's Vocabulary tables and asserts order-equality on every run, failing closed on an unparsable shape. (The vocabulary is 18 values, not the 19 the finding said.)

No plugin manifest was touched, so no plugin version bumps were required — confirmed by --check-bump.

Verification

Every new gate was watched failing before it was trusted.

Vendor gate: unbumped edit to vendor/.../vtt-parser.js → STALE VERSION, exit 1; clean tree → exit 0. Suite 12/12.
Skip-actors: malformed list → reader exit 2, empty stdout; missing/empty → exit 2 (suite 8/8). Behavior fix shown end to end: a cursor[bot] actor under the old drifted default → guard exit 1; under the file-sourced default → skip-listed, exit 0.
Orphaned checks: a temp file carrying the hunted token and a skip_case "fixture did not pair" → rename-sweep exit 1 and disc-skips exit 1, each naming the file/line; removed → both exit 0.
CATEGORY_ORDER: array reorder → --check exit 1 with both lists printed; doc-side added row → exit 1; reverted → in sync, exit 0.

Suites: affected-tests.sh selected 133. All 126 shell suites pass, 0 fail. The 7 non-shell selections run directly: test_hygiene.py 317, test_observer.py 72, test_prune_babysit_worktrees.py 45, contract-clause + manifest-duplicate-keys 41, plus both Node suites.

Gates: check-lane-coverage.sh (48 lanes reachable), check-changelog-parity.sh --check / --check-bump, check-vendor-version-bump.sh --check-bump, validate-plugins.sh, actionlint on all three changed workflows, shellcheck on all six changed shell files, check-shell-portability.sh, markdownlint-cli2, typos, editorconfig-checker, zizmor (0 findings), runner-policy, check-silent-skips — all pass.

origin/main moved three times during this work (168a45e, 7f83ffb, daa9e14); the branch carries all three via conflict-free merges, and every gate above was re-run after the final merge.

Stated limit, not glossed: the full affected-tests.sh --run end-to-end line was never observed from a single process, because the run exceeds this environment's 10-minute command ceiling. Equivalence was preserved by executing the exact selected list sequentially in five batches, all green.

ADR 0019's intra-plugin sharing keeps one committed copy at the plugin root (vendor/), so no byte-drift sync gate applies; the replacing invariant is delivery-by-version, and nothing enforced it. Two unbumped vendor edits already shipped (b3445bc folded its note into the released 0.10.9 section with no bump; b01dace edited vtt-parser.js with none). scripts/check-vendor-version-bump.sh --check-bump <base-ref> fails a PR when a tracked file under any plugins/*/vendor/ tree changes vs the base ref and that plugin's manifest version does not, with the same new-plugin carve-out as the sync-cluster gates. Wired as vendor-version-bump-gate in ci.yml and added to the ci-status aggregate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

The literal skip-actors list was restated at five sites across both review workflows and both evidence guards, and the restatement drifted: 2b4d8ab added cursor[bot] to both workflow lines and left verify-security-review-evidence.sh's default behind, so the guard judged cursor[bot] PRs in scope while the lane skipped them. .github/claude-skip-actors now carries the one statement of ADR 0002's skip-actor exception (same repo-owned-data shape as .github/claude-skip-actors' sibling .github/claude-security-paths), and scripts/read-skip-actors.sh owns the one parse, fail-closed on a missing file, an empty active set, or a malformed entry. The claude-security-review caller reads the PR's BASE copy in a prep job (mirroring the reusable's paths-file discipline) and hands it to the lane's skip-actors input across needs, with the evidence job failing closed when that read did not succeed; the claude-review caller reads it in a step; both evidence guards default from it when SKIP_ACTORS is not in the environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

python-version: '3.14' was a bare literal at five setup-python steps in ci.yml while every Node lane already resolves through node-version-file: .node-version. All five steps now read python-version-file: .python-version, so the interpreter is stated once. The requirements-ci.txt pyyaml note now names the pairing outright: its cpXY wheel-hash sets are ABI-pinned against this interpreter, so a bump in .python-version and the hash sets move together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

scripts/check-rename-sweep.sh promised to break CI loudly when the video-digest skill's pre-rename name resurfaces, and scripts/check-discriminating-test-skips.sh is cited by plugins/testing's audit skill as this repo's own CI gate — but neither ran in any workflow, so both enforced nothing. Both run clean on the current tree. Each gets its own lane (self-test first, then the check), added to the ci-status aggregate so check-lane-coverage keeps them gating. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

docs/CATALOG-TAXONOMY.md claims sole ownership of the category vocabulary, yet scripts/generate-catalog.mjs restated all 18 values and their order in CATEGORY_ORDER with a one-way gate only: an unknown category in marketplace.json hard-errors, but nothing compared the array back to the document. The generator now parses the document's two Vocabulary tables (the backticked first-column values, in order — a narrow parse of a stable shape that fails closed when the shape stops matching) and throws on every run, generate and --check alike, when CATEGORY_ORDER disagrees. Already enforced in CI through scripts/validate-plugins.sh's generate-catalog --check step. The document's Generation contract section names the new coupling so a table reshaping is a known, loud-failure edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

…ed-seams

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ef09aed4f6

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

git's default rename detection collapses a byte-identical move from plugins/a/vendor/ to plugins/b/vendor/ into one R100 record whose --name-only line is the destination only, so the gate checked plugin b alone: bumping b while leaving a unbumped passed even though a's installed vendor source was deleted. Reproduced on a fixture before the change. The gate now diffs with --no-renames, so a move reports as a delete plus an add, one path per side, and both plugins get checked. Two regression cases cover the move shape: source plugin unbumped fails as STALE VERSION (fails without the fix), both plugins bumped passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

…ed file" This reverts commit 6a22f0b. Editing .github/workflows/claude-security-review.yml on a PR trips the reusable workflow's caller-drift fail-closed tier: the caller no longer matches the default branch's copy, the action skips itself without reviewing, and the required security-review check reports a failure no re-run can clear. The skip-actors single-sourcing therefore cannot ride this branch; it moves whole to claude/coupling-skip-actors-ssot, which carries the same change against origin/main on its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

Two fault-injected fail-opens, found in adversarial review, contradicted the gate's own fail-closed framing: - A git diff failure drained through the filter pipeline to an empty plugin list, which reads as "nothing changed" — the gate passed over a tree carrying a real unbumped vendor edit. The diff's status is now checked before its output is consumed; a failed diff exits 2. - With jq unusable, every manifest read came back empty and the loop misread each plugin as the new-plugin carve-out — a full-open gate. jq is now asserted up front; absent or broken tooling exits 2, distinct from "nothing changed". Both shapes carry PATH-shim fault-injection regression cases that exit 0 against the previous script and 2 now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m

claude · 2026-08-28T08:00:45Z

Claude encountered an error after 0s —— View job

I'll analyze this and get back to you.

claude · 2026-08-28T08:00:48Z

Claude encountered an error after 0s —— View job

I'll analyze this and get back to you.

github-actions · 2026-08-28T08:00:56Z

Warning

Automated review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/33153482945
Failure class: rate-limit
Last SDK result: {"subtype":"success","is_error":true,"num_turns":1,"duration_ms":369,"total_cost_usd":0,"api_error_status":429,"class":"rate-limit"}

Re-run the job, or workflow_dispatch this workflow with the PR number, to retry the review. A new push re-triggers this lane only if the caller's pull_request triggers include synchronize (the canonical caller omits it).
An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator (auth).

github-actions · 2026-08-28T08:00:59Z

Warning

Automated security review did not complete — this is an infrastructure failure, not a review verdict.

Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."

Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/33153482835
Failure class: rate-limit
Last SDK result: {"subtype":"success","is_error":true,"num_turns":1,"duration_ms":342,"total_cost_usd":0,"api_error_status":429,"class":"rate-limit"}

The check is green on purpose, and it is not evidence. It certifies that a security pass ran, and this one did not complete — but the cause is outside this PR's control, so merging is deliberately left unblocked rather than locking every merge for the length of the outage. Nothing was reviewed at this head. Where this check is required, it is satisfied without that evidence; a human should review security-sensitive changes here before merging.

Re-run the job, or workflow_dispatch this workflow with the PR number, to retry the review (ci-workflows#227). A new push also retries it only if the caller's pull_request triggers include synchronize (the canonical security caller keeps it). An automatic retry may already have run — it is skipped when a partial review could duplicate comments, or when the failure class needs an operator.

Re-running does NOT help for every class:

rate-limit that persists across re-runs, or auth — the credential or usage budget needs an operator; retrying will not clear it.
a run that exhausted its turn budget ("subtype":"error_max_turns" above) will exhaust it again. As the PR author, split the change into smaller PRs; raising --max-turns is a change to the caller workflow, not something you can set on this PR.

… read (#3470) scripts/check-vendor-version-bump.sh merged in #3464 with its own header stating the principle it did not fully hold: "a gate that cannot see must refuse to pass." Adversarial verification found six paths where it exits 0 while blind. All six were reproduced by fault injection before being fixed, and none is reachable in this repo's current state, which is why they are worth closing now rather than after they bite. Closed: a malformed base manifest, one with no version key, a failing git show, a failing sort in the plugin-list substitution, a non-ASCII vendor path under the default core.quotePath, and a base manifest carrying a raw NUL byte, which Bash silently strips during a command substitution so jq receives repaired JSON and the gate trusts a version the base tree does not hold. Structurally, "absent at base" is now an observation rather than a fallback: a status-checked git ls-tree probes existence, and only then must git show and jq each succeed. Base manifests are staged through a file so jq sees the bytes git stored. Also adds an opt-in --no-renames to changed_files::into, because the library's default collapses a byte-identical move to its destination and would re-open the cross-plugin rename hole #3464 closed. All other callers produce byte-identical git argv, verified through a shim. Fail-then-pass by suite: PASS=16 FAIL=5 against the pre-fix script (the five fail-opens, each "got status 0"), PASS=22 FAIL=0 after. 121 shell suites pass, exit 0.

No linked issue > **This PR cannot make its own `security-review` check green, by design — a human needs to merge it.** The reusable security-review workflow requires the caller's workflow file to match the default branch's copy. This PR edits that caller, so the action self-skips and the required check fails. The job log states it plainly: *"merging the caller-workflow change is what clears this shape — a re-run cannot."* I split this out of #3464 rather than bypass that certification. ## Summary The review skip-actors bot list was restated at five sites and had already drifted. Route-lane finding from the `/coupling:reduce` pass merged in #3457, split out of #3464 for the reason above. `scripts/verify-security-review-evidence.sh`'s default was missing `cursor[bot]`, which `2b4d8abf` added to both workflow lines only — so the two guards silently disagreed about that actor on any invocation that does not set the env. ## Fix One owned data file, `.github/claude-skip-actors`, and one fail-closed parser, `scripts/read-skip-actors.sh`, following the pattern `.github/claude-security-paths` already establishes. Both workflows and both `verify-*.sh` defaults now resolve through it. The reusable's `with: skip-actors` cannot read a file, so a prep job reads the list and passes it across `needs`, mirroring the reusable's own `paths-file` discipline. ## Verification Parser behavior, every shape — it can never succeed with a partial or empty list: ``` empty file rc=2 stdout=[] "names no actors; an empty exception must be an explicit consumer decision" comments/blanks only rc=2 stdout=[] missing file rc=2 stdout=[] embedded space or comma rc=2 stdout=[] "malformed entry" comments + entries rc=0 [dependabot[bot],claude[bot]] trailing whitespace, CRLF rc=0 (trimmed / CR stripped) ``` Suite 8/8. Mutation-tested: removing the empty-set check → 1 failure; removing the malformed-entry check → 2 failures. **Fail direction traced at every consumer, which is what matters here.** An independent adversarial verifier confirmed that a malformed or empty list can never cause a security review to be *skipped*: the security lane's plain assignment aborts the prep job under `bash -e`; `security-review` then reports `skipped`; and `security-review-evidence` still runs under `always()` and exits 1 on its first step (`needs.skip-actors.result != 'success'`) — correctly ordered *before* the guard, which would otherwise exit 0 on `LANE_RESULT=skipped`. Even if an empty list did reach a guard, `",,"` cannot match `*",actor,"*`, so the review runs rather than being skipped. Behavior fix shown end to end: a `cursor[bot]` actor under the old drifted default → guard exit 1; under the file-sourced default → skip-listed, exit 0. Two verifier findings were fixed before this PR was opened: - `claude-review.yml`'s read step used `echo "list=$(...)"`, which returns `echo`'s status and so did not fail the step on a parser error. It is now a plain assignment, matching the security caller's fail-closed form. - The prep job's `git show ... 2>/dev/null` swallowed every error, silently falling back to the head copy on a failed base fetch. An unresolvable base ref is now a hard error; an absent file at a resolvable base is an explicit notice. The job comment now states the real property rather than a stronger one: reading the base copy protects the list's *content*, but the workflow, parser, and `checkout-with-base` composite are all head-controlled on `pull_request` — not a regression (on main the literal lived in the head-controlled workflow file), and bounded by the reusable's `allowed_bots`. Gates: `actionlint` on both workflows, `shellcheck`, `check-shell-portability.sh`, `zizmor` (0 findings), `typos`, `editorconfig-checker`, runner-policy, `affected-tests.sh --run` ("All 3 selected suites passed") — all pass. ## Related Split from #3464. Follow-up to #3457. --- _Generated by [Claude Code](https://claude.ai/code/session_01QkMx972GYEQ3vCb98pLw8m)_ --------- Co-authored-by: Claude <noreply@anthropic.com>

claude added 7 commits August 28, 2026 06:48

Merge remote-tracking branch 'origin/main' into claude/coupling-ungat…

c5fa0ad

…ed-seams

Merge remote-tracking branch 'origin/main' into claude/coupling-ungat…

ef09aed

…ed-seams

chatgpt-codex-connector Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread scripts/check-vendor-version-bump.sh Outdated

claude added 3 commits August 28, 2026 07:36

github-actions Bot deleted a comment from claude Bot Aug 28, 2026

kyle-sexton mentioned this pull request Aug 28, 2026

refactor(ci): give the review skip-actors list one owner #3466

Merged

kyle-sexton merged commit 9cd59fe into main Aug 28, 2026
59 checks passed

kyle-sexton deleted the claude/coupling-ungated-seams branch August 28, 2026 08:45

kyle-sexton mentioned this pull request Aug 28, 2026

fix(ci): make the vendor version-bump gate fail closed on every blind read #3470

Merged

github-actions Bot mentioned this pull request Aug 28, 2026

[Incident] Claude lane infrastructure failure melodic-software/ci-workflows#525

Closed

Navigation Menu

Uh oh!

feat(ci): close five ungated seams found by the coupling pass - #3464

feat(ci): close five ungated seams found by the coupling pass#3464
kyle-sexton merged 10 commits into
mainfrom
claude/coupling-ungated-seams

kyle-sexton commented Aug 28, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

claude Bot commented Aug 28, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kyle-sexton commented Aug 28, 2026

Summary

Fix

Verification

Related

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

claude Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

github-actions Bot commented Aug 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Aug 28, 2026 •

edited

Loading

claude Bot commented Aug 28, 2026 •

edited

Loading