perf(ci): collapse ci.yml into six jobs and stop the AI review lanes re-running on every push - #3696
Merged
Conversation
This was referenced Sep 3, 2026
Contributor
Author
The probe pull requests, and what four real runs provedThe two filter fixtures from item 2.7 are #3697 (lockfile-only) and #3698
Every run shows exactly six jobs, so the job count is verified live rather than The filter groups matched for real, not fail-open. The action fails OPEN to The affected-suite selector, both halves. On #3698 the step printed The stale-base detector reads the head, not the merge commit. Its log reads The AI-review draft filter, proven by run. Wall time. Known red, and why this stays a draft
|
kyle-sexton
marked this pull request as ready for review
September 3, 2026 21:58
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eed193173b
ℹ️ 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".
kyle-sexton
added a commit
to melodic-software/standards
that referenced
this pull request
Sep 3, 2026
…e-review caller contract (#521) No related issue: CI/CD performance program, Phase 2.4 (melodic-software/github-iac#385, program melodic-software/github-iac#378). ## Summary The approved caller contract for the ci-workflows `claude-review.yml` reusable at `62bef7ba` (v0.14.0) admits only the `runner` input, but the reusable declares `pr-number` and `timeout-minutes`. claude-code-plugins' once-per-PR review caller (melodic-software/claude-code-plugins#3696, Phase 2 of the CI/CD performance program) passes `timeout-minutes: 15` and, on `workflow_dispatch`, `pr-number`, so the runner-policy lane rejects the call with "inputs absent from its reviewed contract: pr-number, timeout-minutes". ## Fix - `components/runner-policy/policy.json`: the `approvedReusableWorkflowContracts` entry for `claude-review.yml@62bef7bab01e8532fedfa739879034a210e9e67d` gains `pr-number` and `timeout-minutes` in `allowedInputs`; routing, secrets and caller permissions are unchanged. Both inputs exist on the reusable at that SHA (`workflow_call.inputs.timeout-minutes` and `workflow_call.inputs.pr-number`). - The policy file is standards-distributed (byte-exact in consumers under ADR-0007), so this is the source change; the sync lane carries it to claude-code-plugins, and that sync pull request must merge before claude-code-plugins#3696's lint row goes green. ## Verification - `node --test components/runner-policy/runner-policy.test.mjs`: 274 passed, 0 failed. - `npm run lint:runner-policy`: "Runner policy passed." - lefthook pre-commit (editorconfig, typos, gitleaks, biome) green on the commit. - Reproduction of the rejection: claude-code-plugins#3696, `lint` job, `runner-policy` row on its three probe runs. ## Related - melodic-software/claude-code-plugins#3696 (PR C of Phase 2); melodic-software/github-iac#385, #378; github-iac `docs/topics/ci-perf/PLAN.md` Phase 2.4 and Phase 3b (the wider runner-policy grammar change, which this does not pre-empt). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…ane closure First half of the ci.yml consolidation. The gate that pins ci.yml's shape has to learn the new shape before the YAML can be written, and the two land together in the end; this commit is the gate alone so the work is durable, and the branch is red until the workflow catches up. Three changes. The resolver is named `changes` rather than `scope`, which the gate hardcoded with no override. The resolver publishes a TABLE of boolean-string outputs rather than one. Every polarity decision in the workflow lives in that table, pinned row by row to an exact expression, and consumers compare a table entry against 'true' or 'false' and nothing else. An output the resolver publishes that the table does not name is now a defect: it is a polarity decision made where this gate cannot check it, and consumers reading it would be invisible to the consumer-form rule. The narrowing rows read the change detection results through fromJSON with an empty-object default and compare against 'false', never 'true', so an unset filter group runs its lane. The gate computes the required-lane closure from the file: everything reachable from the aggregate's needs, transitively. Two rules now turn on it. A job-level condition is a defect on a lane inside the closure, where it can report success to branch protection having run nothing, and is the intended shape outside it, where skipping the whole job is cheaper than running it to report nothing. And a lane-coverage opt-out marker on a job inside the closure now FAILS rather than exempting it: the marker asserts the lane does not gate merges, and on an aggregated lane that assertion is false while the sibling coverage gate stops asking once it sees one. Refs melodic-software/github-iac#378 Refs melodic-software/github-iac#385 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Re-pins the known-good fixture to the resolver's new name and its seven published outputs, and adds the cases the new rules need. The suite is 56 assertions and every one passes except the live-file case, which asserts the shipped ci.yml satisfies the contract and cannot until the workflow rewrite lands in the next commit. The closure cases are written as a pair that FLIPS rather than as two separate fixtures, because a verdict that only ever points one way would not show the closure is computed at all. The base fixture carries a lane outside the aggregate's needs that legitimately gates at job level under a coverage opt-out, and a lane inside it that may not; each case moves one job across the boundary and asserts the verdict changes. One case reaches the lane transitively through a required consumer rather than through the aggregate's own list, so a closure computed one level deep fails it. A first draft of the transitive case used a multi-line fixture transform. The transforms compare one line at a time, so it silently matched nothing and the case passed without testing anything. It is now anchored on a single line spelled that way nowhere else in the fixture, and the base fixture's other jobs were respelled to keep that anchor unique. The table cases cover both directions of the same idea: a resolver output the table does not name, and a step gated on one. Plus a narrowing row that compares a filter group against 'true' instead of 'false', which would skip a lane when detection is unset, and a feed override paired with a different output than its step's gate, which maps a skip that never happened. Refs melodic-software/github-iac#378 Refs melodic-software/github-iac#385 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… defect Without the aggregate job the required-lane closure is empty, and an empty closure makes both rules that depend on it pass every workflow. Reporting a single defect line while the job-level-condition rule and the lane opt-out rule quietly stopped asking is the nominal closure this gate exists to deny, so a missing aggregate is now inconclusive and exits 2 beside the missing-resolver case rather than joining the defect list. The parser also now over-matches on every output the table names rather than on the root one alone. Its header promises that a bare mention of the output in an env value or a run script reaches the exact-shape check instead of being invisible to it, and that promise was being kept for one row out of seven. Refs melodic-software/github-iac#378 Refs melodic-software/github-iac#385 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… tag All seventeen ci-workflows references inside ci.yml now resolve to 85e140fa2fe9ebe96c41156055e7676de208090a, which is the v0.19.0 tag cut after the ShellCheck fan-out merged. Sixteen are composite actions and the seventeenth is the zizmor reusable workflow, which a later commit in this branch replaces with a hash-pinned step. Before this commit the repository pinned two different ci-workflows revisions inside this one file: the composite actions at c2654182bc2d78f7909795df78304d482aa69226, dated 2026-07-13, and the zizmor reusable at 31a5b76c4a0b663023dc1c944e2bcfc01d6f6c46, dated 2026-07-21. The gitleaks action was a third, f2d5e06757201f2fce187096a2c6fa805836c3d2. One SHA per repository was never the status quo here, and this commit collapses the three inside ci.yml to one. The action surface moved by 272 commits, 51 action files, and +3257 and -353 lines between c2654182 and the wave tag. The change-detection composite action the consolidated workflow needs does not exist at c2654182; it first appears at 3b2f4eab and is present at the tag. Other workflow files still carry other ci-workflows SHAs. Converging those is Phase 6b's work, where the program plan verifies its acceptance criterion per wave, and widening the repin here would mix a dependency bump into a performance change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…olver Every one of the fifty-odd lanes this workflow carried paid for its own runner, its own checkout and its own toolchain install to run a few seconds of work. They are now steps of four working jobs, lint, test-linux, test-windows and hook-utils, behind one resolver named changes and one aggregate named ci-status. Every step keeps the name its lane had, so a check is still findable by the name it always had. The resolver publishes a table of seven boolean outputs instead of the single run_full it published before. run_full is the root, derived from the docs-only detector; the other six narrow it by the pull request's draft state and by the change-detection action's filter groups. Every narrowing clause compares against 'false' rather than 'true', so an unset group runs its lane. That table is the contract scripts/check-docs-only-gate.sh pins row by row, and the commit that taught the gate the table is the commit this one makes green. test-windows is the one job-level gate in the file, and it is legal because the job sits outside the required-lane closure the aggregate defines. A lane the aggregate cannot see costs a skipped informational run rather than a false green. It carries a lane-coverage opt-out with its reason. The zizmor reusable-workflow job becomes a step running a hash-pinned wheel from .github/requirements-ci.txt at 1.27.0, the version that reusable takes by default at the SHA this repository pinned it to, with the reusable's own persona and the job's read-only workflow token for the online audits. A like-for-like replacement: no security-lane behaviour changes inside a performance change. The pull_request trigger gains a types list including ready_for_review, without which a draft flipped to ready triggered no run at all and the draft filter would have had no way to deliver a real answer. Two departures from the written spec, both forced and both recorded in the report. A step cannot carry both a resolver gate and an event gate: the gate reads a step condition whole, so a compound condition is unsanctioned, and an event-only condition leaves the step skipped on push, which the fail-closed aggregator reds. Every diff-scoped gate therefore runs on each event and returns early from its own script when there is no base ref. And the stale-base overlap detector runs in a detached worktree at the pull request head: this job checks out the synthetic merge commit, whose merge-base with the target is the target tip, so folding the detector in naively would have reported every pull request fresh and proved nothing. scripts/check-rename-sweep.sh and its suite are deleted with their lane. The gate ledger rules them a permanent lane for a finished rename's residue, and nothing else in the tree names either file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…afts Both review callers now trigger on opened, ready_for_review and reopened only. A push to an open pull request no longer starts a review, and both lanes skip a draft outright. Between them those two changes are where this repository's Actions bill actually goes: an active branch used to buy a full LLM review, and a full LLM security review, on every commit it pushed, most of them superseded before they finished. claude-review.yml gains a workflow_dispatch trigger with a pr-number input, and its header now names the three re-review paths the reusable documents and no others: reopen the pull request, flip it to draft and back to ready, or dispatch with the number. An @claude mention is not wired here and the header says so. The review job's condition admits a dispatch explicitly, because a dispatched run carries no github.event.pull_request and a bare draft comparison would never run the review an operator asked for. Its timeout is capped at fifteen minutes, below the reusable's thirty-minute default, which is sized for two eleven-minute attempts plus backoff; this lane is advisory and a review that has not landed by then is retired rather than paid for. claude-security-review.yml takes the same trigger set and the same draft filter on all three of its jobs, with no dispatch trigger and no timeout input: the reusable at the pinned v0.14.2 exposes none, and the skip-actors job reads the ratified list from the pull request's base branch, which a dispatch has no way to supply. Recorded in that file's header, because it is a real consequence rather than an oversight: this lane's required check certifies execution, the reusable's own canonical caller keeps the push trigger for exactly that reason, and without it the check can certify a head the branch has since moved past. dependabot-miro-bundle.yml gains a path filter on the two miro manifests and a per-pull-request concurrency group. The filter is safe here and would not be on a required lane: this workflow gates no merge, so a pull request it never starts leaves no check Pending. It used to start a runner on every pull request in the repository only for its own actor check to skip the one job. AGENTS.md gains the instruction that pays for all of this: open a pull request as a draft and flip it to ready when the work is done. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every step that declines on a push now says so as a ::notice:: naming the event it declined, rather than a plain echo. Twenty-one sites. A push log shows the step ran and chose not to act, which reads differently from a step that passed without doing anything, and that difference was invisible before. The stale-base overlap step removes its detached worktree on every path, through an EXIT trap rather than a trailing command, so a failing detector cannot leave a registered worktree behind for every later git command in the job to trip over. Removal is best-effort and never changes the step's own verdict. The machine-specific-paths lane excludes five files. The v0.19.0 action detects a Windows user path written with forward slashes, a class the previously pinned SHA did not catch, so the finding arrived with the repin rather than with any content change. Four of the five are comments illustrating the very path form the surrounding code parses; the fifth is a Pester fixture whose subject IS the escaped path. Each carries its own reason and cites #3699, which tracks rewording them to the placeholder form the action documents as allowed and dropping these lines. Deferred there rather than done here because changelog parity would charge four plugin version bumps to a change that touches nothing else in those plugins. Both invocations of run-plugin-tests.sh were already at --jobs 3, the push build and the unmapped-path fallback, and are confirmed so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kyle-sexton
force-pushed
the
ci-perf/phase-2-ci
branch
from
September 4, 2026 00:35
eed1931 to
566e696
Compare
…usions Sixteen of this file's seventeen ci-workflows references are at the v0.19.0 wave tag. This one is not, deliberately, and the comment above it says why. At the wave tag the action detects five path classes, and on this repository all five fire on content that is not a machine-specific path: 40 hits across 22 files in ten plugins, every one of them prose naming the form the surrounding code parses, published CHANGELOG entries, or a fixture whose assertion subject IS the path. Three of those categories cannot be reworded. Released history is released, one of the flagged changelog entries is itself documenting a previous fix to this very detector and necessarily quotes the forms it began catching, and changing a fixture's literal changes what the test proves. The action's only lever is a file-level exclude, so silencing 40 phrases would take 22 files and ten plugins out of the scan. The five exclusions an earlier commit added are removed with this change; the lane's eight pre-existing entries are untouched and stand on their own reasons. melodic-software/ci-workflows#549 carries the detection-precision report upstream. The earlier scope estimate for this was wrong: it named five files in one class where the truth was 22 in five, because the first investigation read a truncated log tail. That error is why the remedy moved from an exclude list to holding the pin. Phase 6b's convergence work now has this reference in its input set alongside the eight in other workflow files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ave them through Exit 3 from scripts/affected-tests.sh means the selector ran every shell suite it chose and NAMED suites in other ecosystems whose runner it deliberately will not guess. This step converted that to a notice and success, on the assumption that the Node and Python steps further down the job cover them. That assumption is false, and the numbers are not close: those steps cover four Node sub-projects and one Python module, which is 2 of this tree's 40 Python suites. A change to any of the other 38 selected its suite, ran nothing, and reported success. Twelve Node suites outside the four sub-projects are in the same position. The selector's own header is explicit that this is the failure it refuses to produce: it names the suites and sets an exit code precisely so a caller cannot report success over suites that never executed. This caller was doing exactly that. Exit 3 now parses the NOT RUN list and runs the Python suites with the pinned pytest, which collects unittest.TestCase suites as well as pytest-style ones. No per-suite runner is guessed: each of these files already puts its own module directory on sys.path and carries a unittest entry point, verified against plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_state.py, which passes 23 cases under the pinned pytest 9.1.1. Anything left, Node suites outside the four sub-projects and the Pester suites no lane has ever run, is named in a warning and counted in the run summary instead of being reported as executed. The step's contract as the docs-only gate reads it is unchanged: same gating condition, still no step id, still absent from the CHECK_RESULTS feed. Only the shell body moved, so the gate needs no new fixture case, and its 57 assertions still pass. Found by Codex review on #3696. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The warning that names suites no step executes now carries #3703, and the comment above it says why the branch warns instead of failing: neither path has a runner for the Node suites outside the four sub-projects or for Pester, since run-plugin-tests.sh is shell-only as well, so failing here would red pull requests for a gap the push path shares. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
kyle-sexton
pushed a commit
that referenced
this pull request
Sep 4, 2026
…nt sweeps origin/main advanced 10 commits during this branch's run and absorbed a SEPARATE repo-wide tidy sweep (#3635, #3700, #3702, #3706). Measured overlap before touching anything: this branch changes 144 non-version files, main changed 231, and 69 files are changed by both. A non-mutating `git merge-tree` trial predicted 51 conflicted paths; the real merge produced 52. Resolution policy, applied in priority order rather than side-by-side: 1. A rename on main wins, because main's other call sites are already merged in and keeping our identifier leaves dangling references. This covered lock_uint -> lock_uint_file, assert_clean -> report_clean, need_optarg -> require_value, and the youtube- -> video- temp-dir prefix rename. 2. Content one side has and the other lacks is a judgment call, not a formatting one: decided per case on whether the missing thing still exists post-merge and whether it is load-bearing. 3. Where both sides are equivalent restatements, main's form wins. It is the published base, and preferring it keeps this branch's diff honest. 4. No third form invented unless taking either side alone leaves the file incoherent. Version and changelog conflicts (23 CHANGELOG.md, 7 plugin.json) resolved to main's side wholesale. Verified lossless rather than assumed: `git diff <merge-base> HEAD` over every plugins/*/.claude-plugin/plugin.json shows only "version" lines changed on this branch, so main keeps every description and userConfig edit it made, and our only contribution there was a version number that the new base invalidates anyway. Our changelog text is preserved in a165c45 and is re-applied at corrected versions in the following commit. scripts/check-rename-sweep.test.sh: deletion accepted. Main removed it in #3696 along with its subject script scripts/check-rename-sweep.sh, so the test was orphaned. package.json: this branch's only change here is REVERTED, restoring main's allowScripts pin of @anthropic-ai/claude-code@2.1.246. G01 had set it to 2.1.251 to restore lockstep with the then-current devDependency, correctly and citing an earlier sweep's precedent. But main has since moved that devDependency twice (#3500 to 2.1.251, #3560 to 2.1.258) and left the allow entry at 2.1.246 both times, so post-merge 2.1.251 matches nothing: not the installed version, not main's deliberate value. allowScripts is a version-keyed allowlist for package install scripts, so a key that does not match the installed version fails CLOSED; moving it to 2.1.258 would be the only change that opens anything, and widening a script-execution allowlist is a deliberate security decision rather than a side effect of a simplification sweep. Confirmed the blast radius first: allowScripts occurs exactly once in the repository, in package.json itself, and no script, workflow, gate or lavamoat/allow-scripts tooling reads it. One resolution required synthesis and it is called out because a naive take would not have compiled: in lib/players/hotmart.js, main MOVED SUBTITLE_BATCH_SIZE into the top constant block while our side added a captureMasterUrl helper whose two call sites had already merged cleanly. Taking our block whole would have declared SUBTITLE_BATCH_SIZE twice (a SyntaxError); dropping it would have dangled two calls. The helper is kept, the now-redundant constant line dropped. One premise in the resolution brief was wrong and is corrected here: I flagged adapters/registry-conformance.test.js as two competing assertions over two different collections. Reading all three merge stages shows they are orthogonal edits that collided on one line - ours hoisted `const adapters = sourceAdapters()`, main reworded the comment above it - and sourceAdapters() is pure over a frozen static map, so both sides describe the same single check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XsxC7nPL8mhm3JXL1rrjNJ
kyle-sexton
added a commit
that referenced
this pull request
Sep 7, 2026
…(Phase 9a) (#3935) No related issue: melodic-software/github-iac#378 tracks the ci-perf program (Phase 9a) ## Summary Phase 9 of the ci-perf program codifies what the earlier phases settled. This is the claude-code-plugins half of its startable-now part: two ADR amendments recording shapes this repository already ships. Neither decision changes; both records were describing a world that moved on without them. The github-iac half of the phase (ADR 0001, ADR 0011 and a new `docs/topics/ci-perf/POSTURE.md`) is a sibling pull request. Not merged by this worker. ## Fix **ADR 0002, addendum.** The record kept its default-on advisory posture while the lane triggers still ran as if a blocking gate might return. Since #3696 both callers run once per pull request, on `opened`, `ready_for_review` and `reopened`, with a draft filter whose `event_name` clause exists so a dispatched review is not filtered out by a payload that is not there, and with three named ways to get a review of a newer head. The addendum also records what the file never stated at all: the code-review job queues repository-wide with `queue: max` rather than cancelling in progress, which is a deliberate exception to the cancel posture every other lane in `ci.yml` follows, and is affordable only because a queued advisory review blocks no merge. The security caller takes the exception differently, grouping per pull request with `cancel-in-progress: false` and no `queue` key, and the two are deliberately not normalized to each other. **ADR 0019, addendum.** The record said a shared source gets a sync script and a CI gate. The shape that grew from it was one CI job per library, thirteen of them, each a runner, a checkout, a full-history unshallow and a base fetch for a few shell scripts. #3696 made them steps: twelve in `test-linux`, which already does that unshallow for its own `--check-bump` steps, and `sync-hook-utils.sh` in `hook-utils` beside the contract tests it covers. The invariant is untouched, so the addendum records the placement, the cost that was actually paid (runner startups, unshallows and concurrency slots here; the metered private minute pool on the fleet), the one thing the consolidation lost, and the recheck trigger that says when a sync gate earns a job of its own again. **What the consolidation lost, recorded rather than glossed.** The old job name was the discriminator that said which library failed. The step names were never carrying that load: seven of the twelve drift checks name their library, none of the `--check-bump` steps do, and seven of those share one string. So a red bump check now needs its log read. The addendum says so and asks a new sync step to name its library. ## Verification A fresh-context verifier (no access to the reasoning that produced the change) audited the diff against six criteria and returned **PASS on all six** after three fix rounds. It found five real defects, every one of them mine, and confirmed each fix against the files rather than against my description of the fix: 1. "A fresh toolchain install" was factually inverted. None of the thirteen retired jobs installed a toolchain; the toolchains belong to `test-linux`, the job the twelve steps moved into. 2. "Every step keeps the name it had, so a failure still says which library drifted" was a non sequitur. The verifier ran a job-attributing pass over the old and new `ci.yml` and produced the 7-of-12 and 0-of-12 counts now in the record. 3. ADR 0002 claimed a workflow-level cancel group that `claude-review.yml` does not declare. 4. My fix for (3) introduced a new false claim, that `cancel-in-progress: false` beside a `queue` key is what the security lane uses. It has no `queue` key at all, and the legality half rested on the docs page saying nothing about that pairing rather than on it saying anything. Deleted outright; nothing depended on it. The same inference was in the sibling github-iac records and is dropped there too, so the two repositories stay consistent. 5. The billing conclusion was wrong for this repository: it is public and the jobs ran on a standard runner, so nothing was billed. Split into the cost that is real here (runner startups, unshallows and concurrency slots) and the metered pool the same shape spends on the private repositories the program covers, where the organization caps spend at `$0` and the failure mode is a hard stop. The verifier also checked the trigger set, the draft filter, both concurrency blocks and the attribution SHA against the workflow files line by line, verified both external citations live, and confirmed the repository has no ADR index needing a row. **Two disclosures.** The commit at `2a9a79ee4` carries the wrong toolchain claim in its message. Correcting it needs a force-push, which this task forbids, so the correction is written into the message of the commit that fixed it and repeated here. The verifier was asked whether that disposition is adequate and said yes: the history stays honest. The brief specified `git diff origin/main | grep -c $'—'` printing 0. That form counts unchanged context and git's own hunk headers, and ADR 0002's pre-existing Contents entries contain em-dashes, so it cannot reach 0 without rewriting text this change does not touch. The added-line form is the one that measures authored prose and it prints 0. The verifier ran both, classified all six whole-diff hits (four unchanged context lines, two synthesized hunk headers), and confirmed none is in new prose. ## Related - melodic-software/github-iac#378, the ci-perf program (Phase 9). - melodic-software/github-iac#450, the sibling pull request amending ADR 0001 and ADR 0011 and adding `docs/topics/ci-perf/POSTURE.md`. - #3696, the change both addenda record. --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No related issue: Phase 2 of melodic-software/github-iac#385 (cross-repository)
Refs melodic-software/github-iac#378
Refs melodic-software/github-iac#385
Summary
ci.ymlran about fifty-five jobs. Each paid for its own runner, its owncheckout and its own toolchain install so it could run a few seconds of work,
and every one of them ran on every pull request whatever the diff touched.
Meanwhile both AI review lanes re-ran on every push to an open branch, so an
active branch bought a full LLM code review and a full LLM security review per
commit, most of them superseded before they finished.
This collapses
ci.ymlinto six jobs behind one resolver, narrows what eachlane runs to what the diff can actually affect, and stops both review lanes
re-running on push and running on drafts at all. Phase 2 of the CI/CD
performance program.
Fix
Six jobs.
changes,lint,test-linux,test-windows,hook-utils,ci-status. Every former lane is a step of one of the four working jobs andkeeps the name it had, so a check is still findable by the name it always had.
yq '.jobs | keys | length'prints6.One resolver publishing a table of seven outputs.
run_fullis the root,derived from the docs-only detector. The other six narrow it by the pull
request's draft state and by the ci-workflows
change-detectionaction'sfilter groups:
shell,node,python,powershell,docsandworkflows.Every narrowing clause compares a group against
'false'and never'true', soan unset group runs its lane, which is the action's own documented rule and the
same fail-open direction the detector already takes. Every filter group repeats
the toolchain and configuration paths, so a pull request touching only a
lockfile, only a workflow file or only the ShellCheck configuration runs every
lane rather than none.
docsandpowershellare computed and read by nothingtoday; they exist so the group set matches the program plan and a consumer can
be added without touching the resolver.
One job-level gate, outside the required-lane closure.
test-windowscarries
if: needs.changes.outputs.run_windows == 'true'and a# lane-coverage-ok:annotation, and it is deliberately not inci-status.needs. A lane the aggregate cannot see costs a skippedinformational run rather than a false green; inside the closure the same
condition would let a lane report success having run nothing, and both topology
gates reject it there. Every other gate in the file is at step level.
The repin: sixteen of seventeen. Sixteen of this file's seventeen
ci-workflows references now resolve to
85e140fa2fe9ebe96c41156055e7676de208090a, thev0.19.0tag cut after theShellCheck fan-out merged. Before this,
ci.ymlalone carried threeci-workflows revisions: the composite actions at
c2654182, the gitleaksaction at
f2d5e067, and the zizmor reusable at31a5b76c. One SHA perrepository was never the status quo. Between
c2654182and the wave tag theaction surface moved by 272 commits, 51 action files, and +3257 and -353 lines;
the
change-detectionaction this rewrite needs does not exist atc2654182at all.
The seventeenth is held at the old SHA, deliberately.
machine-specific-pathsstays atc2654182. At the wave tag it detects fivepath classes, and on this repository all five fire on content that is not a
machine-specific path: 40 hits across 22 files in ten plugins, every one of
them prose naming the form the surrounding code parses, a published CHANGELOG
entry, or a fixture whose assertion subject IS the path. Three of those
categories cannot be reworded. Released history is released, one flagged
changelog entry is itself documenting a previous fix to this very detector and
necessarily quotes the forms it began catching, and changing a fixture's
literal changes what the test proves. The action's only lever is a file-level
exclude, so silencing 40 phrases would take 22 files and ten plugins out of the
scan entirely. melodic-software/ci-workflows#549 carries the detection-precision
report upstream and #3699 tracks repinning this one line once that lands.
Other workflow files in this repository still carry other ci-workflows SHAs.
Converging them is Phase 6b's work, where the program plan verifies its
acceptance criterion per wave, and widening the repin here would mix a
dependency bump into a performance change. Phase 6b's input set now includes
this held reference too, alongside the eight in other files.
The zizmor correction. The program's decision 8 specified a hash-pinned
PyPI wheel at
zizmor==1.29.0, "the version the reusable pins". Both halveswere wrong. The reusable downloads a native release archive
(
zizmor-x86_64-unknown-linux-gnu.tar.gz) verified against a SHA-256 input;there is no PyPI involvement. And this repository passes no
versioninput, soit takes the reviewed default at its pinned SHA
31a5b76c, which is v1.27.0.1.29.0 was a worker's local binary version. The wheel pinned here is therefore
zizmor==1.27.0, run with the reusable's own persona and withGH_TOKENfromthe job's read-only workflow token for the online audits, so the step is a
like-for-like replacement and no security-lane behaviour changes inside a
performance change. One consequence, stated because it is real: like the
existing
pyyamlpin this is a platform-specific wheel, so a host outsidemanylinux_2_28_x86_64cannot install from.github/requirements-ci.txtunder--require-hashes.The draft filter and its event guard. Both AI review callers now trigger on
opened,ready_for_reviewandreopened, and both skip drafts.claude-review.yml's condition isgithub.event_name == 'workflow_dispatch' || github.event.pull_request.draft == false:the event clause is load-bearing, because a dispatched run carries no
github.event.pull_requestand a bare draft comparison would never run thereview an operator asked for. That caller also gains a
workflow_dispatchtrigger with a
pr-numberinput and a fifteen-minute timeout, and its headernow names the three re-review paths the reusable documents and no others:
reopen, flip to draft and back to ready, or dispatch with the number.
@claudeis not wired here and the header says so.
Dropping the push trigger cannot block a merge, and the live ruleset is the
evidence: the required status-check set on
mainisci-status,do-not-merge / do-not-merge,pr-issue-linkage / pr-issue-linkageandpr-title / pr-title, under three active organization rulesets (base,ci-gate,signing). ADR 0002 lines 202-205 say otherwise and are stale onthree counts: they name
security-review / security-reviewas required via asecurity-review-gateruleset that is not active, they omitpr-issue-linkage / pr-issue-linkagewhich is required, and theOrganizationAdminbypass they attribute to that ruleset therefore does notapply. The program plan already schedules the ADR 0002 amendment for Phase 9,
so that sentence is flagged stale here rather than edited inside a performance
change. If
security-review-gateis ever re-enabled, the caller regains itspush trigger in that same change.
--jobs 3, not 4. The contract corpus runs three suites at a time. Therunner has four vCPUs and the suites are spawn-bound, so four looked like the
shape that pays, and it is faster, but at four three separate suites failed
across two runs by producing empty output from an external command on a path
with no clock in it. Serialising each one in turn only moves the symptom to the
next suite, so the job count is the lever rather than the allowlist. Measured:
572 s serial, 312 s green at three, two red runs at four. See #3694.
--jobs 4--jobs 4--jobs 3Two departures from the written specification, both forced.
A step cannot carry both a resolver gate and an event gate. The docs-only gate
reads a step's condition whole, so
needs.changes.outputs.X == 'true' && github.event_name == 'pull_request'is anunsanctioned consumer form; and a step gated on the event alone is
skippedona push, which the fail-closed aggregator turns red. Every diff-scoped gate
therefore runs on each event and returns early from its own script when there
is no base ref to diff against.
The stale-base overlap detector runs in a detached worktree at the pull request
head. On a
pull_requestevent this job checks out the synthetic merge commit,whose merge-base with the target IS the target tip, so the detector would have
reported every pull request fresh and proved nothing. Its own former job carried
ref: head.shafor exactly that reason.Deleted with its lane:
scripts/check-rename-sweep.shand its suite. Thegate ledger rules them a permanent lane for a finished rename's residue, and
nothing else in the tree names either file.
Also:
dependabot-miro-bundle.ymlgains a path filter on the two miromanifests and a per-pull-request concurrency group, and
AGENTS.mdgains theinstruction that pays for the draft filter: open a pull request as a draft and
flip it to ready when the work is done.
Verification
Every command below was run locally on the branch.
bash scripts/check-docs-only-gate.sh --checkchanges; 91 references across 4 consumer jobs, all sanctionedbash scripts/check-docs-only-gate.test.shok, 0not ok, including the live-file case that was red before this rewritebash scripts/check-lane-coverage.sh --checkci-status.needsbash scripts/check-lane-coverage.test.shyq '.jobs | keys | length' .github/workflows/ci.yml6yq '.on.pull_request.types' .github/workflows/ci.yml[opened, synchronize, reopened, ready_for_review]actionlint -config-file .github/actionlint.yamlon all four changed workflowszizmor --persona regularon all four changed workflowstyposon every changed filemarkdownlint-cli2 AGENTS.mdgrep -c "synchronize"on both review callers0and0grep -c "timeout-minutes: 15" .github/workflows/claude-review.yml1grep -c "draft == false"on both review callers2and3grep -c "affected-tests.sh --run" .github/workflows/ci.yml1bash scripts/check-changelog-parity.sh--check,--check-bump,--check-preserved,--check-orderbash scripts/check-contract-slice-prune.sh--checkand--check-diff origin/mainbash scripts/check-vendor-version-bump.sh --check-bump origin/mainbash scripts/check-orphaned-fixtures.sh --check,check-fixture-git-isolation.sh --checkbash scripts/check-hook-wiring-liveness.sh,check-silent-skips.shbash scripts/check-plugin-manifest-presence.sh,check-plugin-catalog-enablement.shbash scripts/check-skill-leaf-names.sh --check,check-loop-lane-floor-drift.sh --checkbash scripts/affected-tests.sh --base origin/maincontinue-on-errorstep inlintagainst theCHECK_RESULTSfeedzizmor-1.27.0-py3-none-manylinux_2_28_x86_64.whlsha256:afb28123882d2b8248f1e480bf6cc6d1af102e0d3fbe22a40f7f795b1aa9d435Two probe pull requests exercised the filter groups against this branch and are
now closed unmerged: #3697 (lockfile-only, opened as a draft and flipped to
ready) and #3698 (workflow-only). Four real runs; their per-lane verdicts are in
the comment below. Three properties only a run could establish:
The filter groups matched for real, not fail-open. This is the property the
two probes cannot distinguish on their own, because the action fails OPEN to
relevant: a rejected pattern, a parse warning or an API hiccup produces the same
all-
trueresult a correct match does. Run33808374809's match step settlesit, printing
FILES_LIST_FAILED: falseand then six named groups alltrue,with no warning,
outcome=successin 59 ms. So the group names the resolverlooks up through
fromJSONare the names the action actually published.The detached worktree reads the head, not the merge commit. The stale-base
step's log reads
Preparing worktree (detached HEAD b9996c56), which is#3698's own head commit. Both probe heads were genuinely fresh, so the
detector's verdict proves nothing either way; the checked-out SHA is what proves
it is looking at the right tree.
Wall time, against the 25-minute timeout and the program's 12-minute p95
target.
linttest-linuxtest-windows33808259580(this PR, draft)33808363829(#3697, draft)33809595757(#3697, ready)33808374809(#3698, ready)lintis the critical path on every pull request including a docs-only one, andit is serial now. At 4.5 to 5.6 minutes the p95 target is not unreachable by
construction.
The affected-suite selector was exercised on both sides:
No suites selectedon#3698's lone workflow file, and a non-empty run on #3697's ready-flip ending
All 6 selected suites passed or were skipped.Both previously-red rows are resolved.
runner-policywas red because.github/standards/runner-policy/policy.jsongave the claude-review reusable at62bef7baanallowedInputsof exactly["runner"], and the review-triggerchange above adds
pr-numberandtimeout-minutes. That file isstandards-distributed, so under ADR-0007 the fix was a change in
melodic-software/standards(#521) and its sync back here (#3701, merged as557e1ab01); this branch is rebased onto it andrunner-policy.mjsnow passes.machine-specific-pathswas red because of the repin, and is resolved byholding that one action at its old SHA, above.
Security pass on every trigger and permission change:
pull-requests: readisgranted on
changesalone, which is the only job that needs it (thechange-detection action lists the pull request's files through the API);
no
pull_request_targetanywhere; no secret is reachable from a fork context;actionlintandzizmor --persona regularare clean on every changedworkflow; and
claude-review.ymlkeeps its repo-widequeue: maxseat groupunchanged, with no second concurrency block added.
One finding from that pass, recorded rather than silently accepted: the
security lane's required check certifies execution, and the reusable's own
canonical caller keeps the push trigger for that lane specifically. Without it
the required check can certify a head the branch has since moved past. The
program plan rules the trigger cut deliberate; the recovery path is to flip the
pull request to draft and back to ready, and that is written into the caller's
header rather than left for a reader to discover.
Related
melodic-software/github-iac#378.
02ac028, with thefollow-up in melodic-software/github-iac#393.
78349aeb3.merged as
85e140fand cut asv0.19.0.--jobs 4.machine-specific-pathsto the wave tag onceci-workflows#549 fixes its detection precision.
the Pester suites, which no step executes on either the pull-request or the
push path. The contract-suite step names them in a warning and counts them in
the run summary rather than reporting them as executed.
pr-numberandtimeout-minutesto theclaude-review reviewed contract; its sync into this repository has to merge
before the
runner-policyrow goes green.melodic-software/github-iac#385 is closed by hand after this merges.