Skip to content

chore(lint): bump golangci-lint timeout to 10m - #102

Closed
mpjunior92 wants to merge 1 commit into
masterfrom
chore/lint-timeout-10m
Closed

chore(lint): bump golangci-lint timeout to 10m#102
mpjunior92 wants to merge 1 commit into
masterfrom
chore/lint-timeout-10m

Conversation

@mpjunior92

Copy link
Copy Markdown
Contributor

Summary

Lint hit its 5-minute ceiling on PR #92 with:

```
level=error msg="Running error: context loading failed: failed to load packages: failed to load packages: failed to load with go/packages: context deadline exceeded"
level=error msg="Timeout exceeded: try increasing it by passing --timeout option"
```

The package-load step alone consumed the full 5 minutes on the self-hosted runner, leaving no budget for the actual analysis pass. Bumping to 10m gives reliable headroom — typical successful runs take 2-3 minutes when the runner isn't contending.

Diff

```diff

  • golangci-lint run --timeout "5m"
  • golangci-lint run --timeout "10m"
    ```

Why not also a workflow-level timeout-minutes

Checked `.github/workflows/main.yaml` — the `lint` job has no step-level `timeout-minutes`, so the GitHub Actions default (6h step ceiling) applies. The Makefile's `--timeout` is the only practical ceiling, and that's what we just bumped.

Test plan

  • CI lint job completes successfully on this PR (~2-3 min expected; the 10m flag is the new effective ceiling).

Lint hit the 5-minute timeout on the test/tpm-verifier-error-paths PR
with `context loading failed: context deadline exceeded`. The job's
package load alone took >5m on the self-hosted runner, leaving no
budget for the actual analysis pass. 10m gives reliable headroom; the
job typically completes in 2-3 minutes when not contending for runner
cycles.

The workflow step has no step-level timeout-minutes, so the
golangci-lint --timeout flag in the Makefile is the only ceiling.
@claude

claude Bot commented May 28, 2026

Copy link
Copy Markdown

Claude finished @mpjunior92's task in 1m 5s —— View job


Code Review

  • Gather context (read Makefile and workflow files)
  • Review the change
  • Post final feedback

Summary

This is a clean, well-scoped fix. The change is correct and the PR description is thorough. Here are my observations:

The change (Makefile:64)

-	golangci-lint run --timeout "5m"
+	golangci-lint run --timeout "10m"

Verdict: LGTM. The motivation is sound — the 5m ceiling was consumed entirely by package loading, leaving no budget for analysis. Bumping to 10m is appropriately sized given typical 2-3 minute successful runs on this runner.


Observations

1. No .golangci.yml config file exists

There's no project-level .golangci.yml, so the --timeout CLI flag is the sole timeout control. This is fine, but worth knowing: if a config file is ever added, a run.timeout entry there would take precedence over the CLI flag, so the two should be kept in sync.

2. Workflow-level timeout is absent (as noted in the PR)

The lint job at .github/workflows/main.yaml:30 runs on self-hosted with no timeout-minutes at the job or step level:

lint:
  runs-on: self-hosted
  steps:
    # no timeout-minutes here
    - name: Run linter
      run: |
        make deps
        ...
        make lint

GitHub's default of 6 hours applies. The PR author correctly identifies this. A complementary hardening would be adding timeout-minutes: 15 to the job — this guards against golangci-lint hanging (e.g., deadlock, infinite loop during analysis) rather than just timing out gracefully. That said, this is beyond the scope of this PR and I note the constraint that workflow files cannot be modified by this review tooling.

3. No security, performance, or testing concerns

This change touches only the linter invocation timeout. There are no security, correctness, or test coverage implications.


Code Quality: ✅ Clean

Security: ✅ No issues

Performance: ✅ Not applicable

Testing: ✅ No tests needed for this change

Documentation: ✅ PR body is thorough and self-contained


This is ready to merge as-is. The only optional follow-up would be adding a timeout-minutes: 15 to the lint job in the workflow to cap runaway executions — but that is a separate, incremental improvement and not a blocker. Fix this →

@mpjunior92

Copy link
Copy Markdown
Contributor Author

Folding into #92 instead of a separate PR.

@mpjunior92 mpjunior92 closed this May 28, 2026
@mpjunior92
mpjunior92 deleted the chore/lint-timeout-10m branch May 28, 2026 17:04
Sign up for free to 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.

1 participant