Skip to content

Make the lockstep check catch stale agreement and .yaml workflows - #628

Merged
jeremy merged 1 commit into
mainfrom
fix/lockstep-floor-and-yaml
Aug 6, 2026
Merged

Make the lockstep check catch stale agreement and .yaml workflows#628
jeremy merged 1 commit into
mainfrom
fix/lockstep-floor-and-yaml

Conversation

@jeremy

@jeremyjeremy commented Aug 6, 2026

Copy link
Copy Markdown
Member

Two holes in the check that shipped in #625. Neither is a live failure — all 15 workflows are .yml and all three linting steps sit at v2.11.1 — but both are the same shape as the drift the check exists to catch.

It globbed *.yml only. GitHub honours .yaml just as well, and a .yaml workflow running a linter this check never opened is precisely the invisible drift it was written for. Now scans both, and fails on an empty scan rather than passing a check that opened no files.

It enforced agreement, not currency. Verified: setting all three workflows to v2.9.0 passes cleanly and reproduces the release failure that motivated #625 — a gosec G115 false positive that blocked a tag after every PR check had gone green. So there is now a floor, MIN_VERSION, next to WORKFLOW_DIR, commented to say that raising it is deliberate and must happen in the same commit as the pins it constrains.

Compared with sort -V, and that is the point, not a detail. As strings v2.9.0 sorts abovev2.11.1 because 9 > 1, so a lexical test would wave through the exact version that broke the release tag; [ -gt ] parses neither. aur-publish.yml's pkgrel guard carries the same note for the same reason.

Existing shape is untouched: the awk step-window parser, the UNPINNED failure mode added after the bot's find on #625, and the error output that names offending files.

Verification

Ran the script directly in each state, asserting exit codes and restoring the tree between:

StateExpectGot
uniform v2.11.1 (today's tree)pass0 — "3 steps across 15 workflows"
uniform v2.9.0fail, floor1 — floor message
uniform v2.12.0pass0 — sort -V sanity above the floor
drift (one workflow differs)fail1 — disagree message
step with no version:fail1 — UNPINNED
.yaml fixture with a matching pincounted0 — 4 steps across 16 workflows
.yaml fixture with a drifted pinfail1 — names the .yaml file
.yaml fixture unpinnedfail1 — UNPINNED on the .yaml file
empty workflow dirfail1 — empty-scan message

The .yaml case is asserted as counted (3 → 4 steps, 15 → 16 workflows), not merely as a run that happened to pass; 5b and 5c confirm it is actually parsed, not just globbed.

bin/ci green (exit 0).


Summary by cubic

Updates the golangci-lint lockstep check to catch stale pins and include .yaml workflows, preventing invisible drift and release breakage. It now scans both .yml and .yaml files and enforces a version floor.

  • Bug Fixes
    • Scan *.yml and *.yaml; fail on an empty workflow scan.
    • Enforce a MIN_VERSION floor for the pinned golangci-lint; fail if below.
    • Compare versions with sort -V to avoid incorrect lexical ordering.
    • Preserve disagreement and UNPINNED checks; output now includes step and workflow counts.

Written for commit 6dc9163. Summary will update on new commits.

Review in cubic

Two holes in what shipped, both the same shape as the drift the check
was written for.
It globbed *.yml only. Every workflow here is .yml today so nothing was
skipped, but GitHub honours .yaml just as well, and a .yaml workflow
running a linter this check never opened is exactly the invisible drift
it exists to catch. Scan both, and fail on an empty scan rather than
passing a check that opened no files.
It enforced agreement, not currency. Setting all three workflows to
v2.9.0 passes cleanly and reproduces the release failure that motivated
the check in the first place. So: a floor, next to WORKFLOW_DIR, moved
in the same commit as the pins it constrains.
Compare with sort -V. As strings v2.9.0 sorts above v2.11.1, because
9 > 1, so a lexical test would wave through the precise version that
broke the release tag — and [ -gt ] parses neither. aur-publish.yml's
pkgrel guard carries the same note for the same reason.
CopilotAI balanced review requested due to automatic review settings August 6, 2026 00:37
CopilotAI reviewed Aug 6, 2026

CopilotAI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jeremy
jeremy merged commit 7fc5212 into mainAug 6, 2026
24 of 25 checks passed
@jeremy
jeremy deleted the fix/lockstep-floor-and-yaml branch August 6, 2026 00:40
jeremy added a commit that referenced this pull request Aug 22, 2026
…or-roundtrip
* origin/main: (96 commits)
ci: bump the github-actions group with 6 updates (#639)
Reject three more doomed invocations before draining stdin (#645)
Stdin `-` support everywhere sensible; usage error for stray `-` elsewhere (#641)
Add hey-cli Windows signing secrets to the release env manifest (#642)
deps: bump the go-dependencies group with 5 updates (#638)
Update nix flake and plugin version for v0.9.1
ci: bump the github-actions group with 4 updates (#633)
Add basecamp files replace: publish a new version of an uploaded file (#634)
Add basecamp files versions — HELD, blocked on the SDK (#622)
Update nix flake and plugin version for v0.9.0
Make the Codex probe's timeout actually bound doctor (#629)
Make the lockstep check catch stale agreement and .yaml workflows (#628)
Keep refreshing opencode's other spelling (#627)
Lint the release the same way we lint everything else (#625)
Install the skill where opencode actually looks (#624)
Take the communiques out of the source tree (#623)
Correct the API coverage claim: 183/184, not 100% (#621)
Stop echoing back step fields the caller never changed (#620)
Drive the circuit breaker's clock from tests, not sleep() (#619)
Tell agents the truth about card column moves (#618)
...
Sign up for freeto 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.

2 participants

@jeremy