Skip to content

ci(lint): retry golangci-lint config verify on transient failures - #78

Merged
cristim merged 1 commit into
mainfrom
ci/72-lint-verify-retry
Oct 17, 2026
Merged

cristim merged 1 commit into
mainfrom
ci/72-lint-verify-retry

Conversation

@cristim

@cristim cristim commented Oct 17, 2026

Copy link
Copy Markdown
Member

Summary

The Lint Code job runs golangci-lint config verify, which downloads its JSON schema from golangci-lint.run at run time. When that site is slow, the required lint gate fails on clean code, as it did on #71 (run 38836492087, context deadline exceeded).

Now config verify runs inside a small verify_config function in the "Run golangci-lint" step. It tries up to 3 times, waiting 10s and then 20s between attempts, and logs each failed attempt to stderr. It fails the step only if all 3 attempts fail. golangci-lint run is unchanged and still runs only after verify passes.

I chose option 2 (bounded retry) from the issue:

  • Option 1 (local schema): the pinned v2.10.1 accepts a local schema only through a hidden debug --schema flag (pkg/commands/config_verify.go). That flag is not a supported interface, and a vendored schema would need updating on every lint bump.
  • Option 3 (drop verify): this would lose the schema check. It was not needed.

Closes #72

Verification (local, macOS, fake-binary harness plus the real pinned linter)

I extracted the step's run: block and ran it under bash --noprofile --norc -eo pipefail, with a fake golangci-lint first on PATH:

Case Exit code run invoked?
Verify fails twice, then passes 0 yes, after 3 verify calls
Verify always fails 1 no, after exactly 3 verify calls (30.6s with the real sleeps)
Verify passes, run fails 1 yes
Module cd fails 1 no golangci-lint call at all
  • Mutation probe: changing the function's final return 1 to return 0 makes the always-fail case pass, so the harness catches it.
  • Invalid config: the real golangci-lint@v2.10.1 config verify on a deliberately invalid config exits 3 with schema errors. An invalid .golangci.yml therefore still fails CI, about 30s later than before.
  • Linters: actionlint is clean, and all pre-commit hooks pass.

Review

An independent non-author reviewer (Sonnet, high effort) did two passes and had no actionable findings. Pass 2 was on the exact staged tree ddf124e. It checked:

  • errexit inside an && list: the function returns explicit statuses;
  • the subshell inherits the function;
  • the default run: shell; this step sets its own set -euo pipefail;
  • scope: one file, +15/-1;
  • whether anything could be deleted.

CodeRabbit was not pinged because the org's CR quota is exhausted.

🤖 Generated with Claude Code

config verify downloads its JSON schema from golangci-lint.run at run
time, so a slow or unreachable site failed the required lint gate even
on clean code. Retry it up to 3 times (10s, then 20s apart) and fail
only if every attempt fails. An invalid .golangci.yml still fails the
step, after the retries.

The pinned v2.10.1 only offers a hidden debug flag for a local schema,
so vendoring the schema was not used.

Closes #72

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim cristim added impact/internal Team-internal only type/chore Maintenance / non-user-visible priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline effort/xs Trivial / one-liner triaged Item has been triaged labels Oct 17, 2026
@coderabbitai

coderabbitai Bot commented Oct 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 1 billable file and costs up to $0.25.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 36 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 74 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-mcp/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: f3570c16-e5cb-4d7c-a315-d96b042496cf
📥 Commits

Reviewing files that changed from the base of the PR and between a14f10c and 6e336d1.

📒 Files selected for processing: 1
  • .github/workflows/ci.yml
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Oct 17, 2026

Copy link
Copy Markdown
Member Author

Merge-driver verdict: CLEAN at 6e336d109db62654e7d5b19984c29cbfe8c7c039

An independent adversarial reviewer (Sonnet, high effort, cold brief, not the author) reviewed the full diff. It found no blocking findings.

Shell semantics. The step sets set -euo pipefail (ci.yml:57). Errexit is suppressed inside verify_config because it is called in an && list, but this is harmless:

  • the function returns explicit statuses;
  • && gates golangci-lint run on that status;
  • the status of run is the subshell's exit code, so a lint failure still fails the step.

Local verification (fixture, macOS). I extracted the run block and ran it under bash --noprofile --norc -eo pipefail with a fake golangci-lint:

Case Exit verify calls run invoked?
Verify fails twice, then passes 0 3 yes
Verify always fails 1 3 no; verify stderr and an attempt line are logged each time
Verify passes, run fails 1 1 yes

Real tool checks:

  • actionlint on ci.yml is clean.
  • shellcheck reports only extraction or pre-existing notes: SC2148, plus SC2043 on the unchanged for mod in ..
  • The CI Lint Code job (run 39046644141) ran the new step and exercised the success path. The retry path is covered only by the fixture.

Non-blocking notes:

  • A genuinely invalid config now fails about 30s later, plus per-attempt time. This is documented in the PR body.
  • Each verify attempt can take up to its HTTP timeout, so the real worst case is about 2 minutes. That is well below the job's default 360-minute timeout.
  • No timeout wraps config verify, which was also true before this PR.

Gate items:

@cristim
cristim merged commit 8c9b39f into main Oct 17, 2026
13 checks passed
@cristim
cristim deleted the ci/72-lint-verify-retry branch October 17, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/eventually No deadline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci(lint): config verify fetches its schema over the network and flakes the lint gate

1 participant