Skip to content

Lint the release the same way we lint everything else - #625

Merged
jeremy merged 2 commits into
mainfrom
ci/release-lint-lockstep
Aug 5, 2026
Merged

Lint the release the same way we lint everything else#625
jeremy merged 2 commits into
mainfrom
ci/release-lint-lockstep

Conversation

@jeremy

@jeremyjeremy commented Aug 5, 2026

Copy link
Copy Markdown
Member

v0.9.0-rc.1 failed. Nothing was wrong with the code.

What happened

release.yml pinned golangci-lint at v2.9.0. test.yml and security.yml are on v2.11.1 — with a # keep in lockstep with test.yml Lint comment on the latter. release.yml drifted and nothing noticed, because the release gate is the one job no pull request ever runs.

It surfaced as:

internal/commands/assignments.go:142:85: G115: integer overflow conversion int -> int32 (gosec)

That conversion is guarded, two lines above it:

ifposition<1||position>math.MaxInt32 {
returnoutput.ErrUsage("--position must be 1 or greater (positions are 1-based)")
}
...MyAssignments().Reorder(cmd.Context(), recordingID, int32(position))

gosec at v2.11.1 follows that bound and stays quiet. At v2.9.0 it does not. So a false positive that every PR check had already passed took down a release tag, and assignments.go needs no change — it was already correct.

The fix

Pin matched, plus scripts/check-lint-lockstep.sh to enforce it. The lockstep comments already existed and already failed; comments don't enforce.

It reads the version off every golangci-lint-action step and fails naming the files:

FAIL: golangci-lint pins disagree across workflows:
.github/workflows/release.yml:v2.9.0
.github/workflows/security.yml:v2.11.1
.github/workflows/test.yml:v2.11.1

Verified both directions: passes on this branch (3 workflows at v2.11.1), and fails as above when release.yml is reverted to v2.9.0.

Wired into three places:

  • make check — local bin/ci
  • the release quality gate — so a future drift fails as drift, ahead of the linter
  • test.yml, before the linter step — so PRs catch it, which is the whole point

Next

Once this lands, v0.9.0-rc.1 gets re-cut as v0.9.0-rc.2. rc.1 published no release and no artifacts — every job after the gate was skipped — so nothing needs unpublishing. I'd rather leave the dead tag than retag it.

bin/ci green (EXIT=0).


Summary by cubic

Align golangci-lint versions across CI and enforce lockstep to prevent release failures from version drift. release.yml now pins v2.11.1, and the check fails if any workflow drifts or has an unpinned step.

  • Bug Fixes
    • Pin golangci-lint to v2.11.1 in .github/workflows/release.yml (matching test.yml/security.yml).
    • Add scripts/check-lint-lockstep.sh to ensure all golangci/golangci-lint-action steps are pinned and use the same version; report mismatches and missing pins.
    • Run the check in make check, the release quality gate, and in test.yml before linting.

Written for commit 66e9980. Summary will update on new commits.

Review in cubic

release.yml pinned golangci-lint at v2.9.0 while test.yml and security.yml
had moved to v2.11.1. Nothing noticed, because the release gate is the one
job no pull request runs.
It surfaced as v0.9.0-rc.1 failing, on this:
internal/commands/assignments.go:142:85: G115: integer overflow
conversion int -> int32 (gosec)
That conversion is guarded two lines above it — `position < 1 || position >
math.MaxInt32` returns before the cast. gosec at v2.11.1 follows the bound
and stays quiet; at v2.9.0 it does not. So the finding was a false positive
that every PR check had already passed, and the only thing it blocked was a
release tag.
The pin is now in lockstep, and scripts/check-lint-lockstep.sh enforces
that rather than trusting the "keep in lockstep" comments that were already
there and had already failed. It reads the version off every
golangci-lint-action step and fails when they disagree, naming the files:
FAIL: golangci-lint pins disagree across workflows:
.github/workflows/release.yml:v2.9.0
.github/workflows/security.yml:v2.11.1
.github/workflows/test.yml:v2.11.1
Wired into `make check`, the release gate, and test.yml ahead of the linter
itself, so drift is reported as drift instead of as a version-specific
finding in whichever job happens to be the odd one out.
No change to assignments.go: the code was already correct.
CopilotAI balanced review requested due to automatic review settings August 5, 2026 16:32
@github-actionsgithub-actionsBot added the ci CI/CD workflows label Aug 5, 2026
@github-actions

Copy link
Copy Markdown

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

  • .github/workflows/release.yml
  • .github/workflows/test.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

@chatgpt-codex-connectorchatgpt-codex-connectorBot 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:39d2233379

ℹ️ 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 threadscripts/check-lint-lockstep.sh Outdated
CopilotAI reviewed Aug 5, 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.

The first version of this check parsed pins and compared them, so a step
that declared no version was skipped rather than caught: delete
release.yml's version line and the check reported success across the
remaining two workflows.
That is the same hole in a different shape. An unpinned step resolves to
whatever golangci-lint-action defaults to, which moves on its own
schedule and puts the release gate back on a version nothing else runs.
Now every golangci-lint-action step must carry a version. Verified in all
four states: agreeing pins pass, a drifted pin fails naming the files, a
deleted version line fails as UNPINNED, and restoring it passes again.
CopilotAI review requested due to automatic review settings August 5, 2026 16:38
@jeremy

Copy link
Copy Markdown
MemberAuthor

Good catch — fixed in 66e9980, and you were right about the mechanism.

The first version parsed pins and compared them, so a step with no version: was skipped rather than caught. Same hole, different shape: an unpinned step resolves to whatever golangci-lint-action defaults to, which moves on its own schedule and puts the release gate right back on a version nothing else runs.

Now every golangci-lint-action step must yield a version, and a missing one is its own failure mode:

FAIL: golangci-lint-action step with no version pin:
.github/workflows/release.yml
An unpinned step takes whatever the action defaults to and drifts on
its own schedule, which is the mismatch this check exists to prevent.

Verified in all four states — agreeing pins pass, a drifted pin fails naming the files, a deleted version line fails as UNPINNED, and restoring it passes again.

CopilotAI reviewed Aug 5, 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 3d4e235 into mainAug 5, 2026
25 of 26 checks passed
@jeremy
jeremy deleted the ci/release-lint-lockstep branch August 5, 2026 16:42
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

ciCI/CD workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jeremy