ci: add golangci-lint and coverage gate - #2
Conversation
Match the quality bar used by prairie-server and the client repos. Co-authored-by: Jonah May <JonahMMay@users.noreply.github.com>
Co-authored-by: Jonah May <JonahMMay@users.noreply.github.com>
Co-authored-by: Jonah May <JonahMMay@users.noreply.github.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
📝 WalkthroughWalkthroughThe pull request updates CI triggers and execution controls, adds a configured ChangesCI quality gates
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
37-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin a golangci-lint minor version.
latestmakes lint results change without a repository change. Pin a supported minor (for example,v2.12) while retaining patch updates. (github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml at line 37, Update the golangci-lint action configuration’s version value from latest to a supported minor version such as v2.12, preserving automatic patch updates while making lint behavior stable across repository changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 9-10: Update the workflow-level permissions alongside contents:
read to grant pull-requests: read, ensuring only-new-issues can access pull
request data during PR runs.
- Around line 54-57: Update the “Run tests with coverage” workflow step to
exclude generated .pb.go files from coverage.out and make package discovery fail
closed: enable pipefail so go list or grep failures stop the step rather than
producing a partial package list. Preserve the existing runtimedefault exclusion
and coverage test behavior.
In `@scripts/check-coverage.sh`:
- Around line 2-4: Update the COVER_MIN fallback in the coverage-check script
from 70 to the documented 65 percent floor, while preserving the ability for an
explicitly set COVER_MIN value to override it.
- Around line 15-20: Validate COVER_MIN before invoking the awk comparison,
requiring a numeric value between 0 and 100 and rejecting malformed or negative
thresholds with a nonzero exit. Update the coverage-check flow around the awk
BEGIN block so only validated COVER_MIN values reach the existing
total-versus-min comparison.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 37: Update the golangci-lint action configuration’s version value from
latest to a supported minor version such as v2.12, preserving automatic patch
updates while making lint behavior stable across repository changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2f41fda-6a5e-4676-809e-61d7ceb295c9
📒 Files selected for processing (3)
.github/workflows/ci.yml.golangci.ymlscripts/check-coverage.sh
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- .github/workflows/ci.yml (relevant lines) ---\n'
nl -ba .github/workflows/ci.yml | sed -n '1,140p'
printf '\n--- search for golangci-lint action usage ---\n'
rg -n "golangci|only-new-issues|pull-requests|permissions:" .github/workflows -SRepository: Prairie-Server/prairie-plugin-sdk
Length of output: 262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- .github/workflows/ci.yml (relevant lines) ---'
nl -ba .github/workflows/ci.yml | sed -n '1,140p'
echo
echo '--- golangci-lint action references ---'
rg -n "golangci|only-new-issues|pull-requests|permissions:" .github/workflows -S || trueRepository: Prairie-Server/prairie-plugin-sdk
Length of output: 261
🌐 Web query:
golangci-lint-action only-new-issues pull-requests permission GitHub Actions
💡 Result:
When using the only-new-issues option in the golangci-lint-action, you should grant read access to pull requests in your GitHub Actions workflow permissions [1][2]. Specifically, the action requires the pull-requests: read permission to fetch the necessary diff information from the GitHub API [1][3]. The recommended configuration in your workflow file is as follows [1][2]: permissions: contents: read # Optional: allow read access to pull requests. Use with only-new-issues option. pull-requests: read The action uses this permission to retrieve the diff of the pull request content (or push content) via the GitHub API, which it then processes using the --new-from-patch option to report only newly introduced issues [1][2]. If you do not provide this permission, the action may fail to correctly identify or filter for only the new issues [1].
Citations:
- 1: https://github.com/golangci/golangci-lint-action/blob/main/README.md
- 2: https://github.com/golangci/golangci-lint-action
- 3: https://github.com/golangci/golangci-lint-action/tree/v6.5.0
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- .github/workflows/ci.yml (relevant lines) ---'
sed -n '1,140p' .github/workflows/ci.yml | cat -n
echo
echo '--- golangci-lint references ---'
rg -n "golangci|only-new-issues|pull-requests|permissions:" .github/workflows -S || trueRepository: Prairie-Server/prairie-plugin-sdk
Length of output: 2656
Add pull-requests: read for only-new-issues. contents: read leaves PR access at none, and this mode needs the pull request API on PR runs or linting can fail before it starts.
Proposed fix
permissions:
contents: read
+ pull-requests: read📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: read | |
| permissions: | |
| contents: read | |
| pull-requests: read |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 9 - 10, Update the workflow-level
permissions alongside contents: read to grant pull-requests: read, ensuring
only-new-issues can access pull request data during PR runs.
| - name: Run tests with coverage | ||
| env: | ||
| GOWORK: off | ||
| run: go test $(go list ./pkg/pluginsdk/... | grep -v '/runtimedefault$') -count=1 -covermode=atomic -coverprofile=coverage.out |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ci.yml relevant section ==\n'
nl -ba .github/workflows/ci.yml | sed -n '1,140p'
printf '\n== search for coverage/protobuf filtering ==\n'
rg -n --hidden -S "coverage\.raw|coverage\.out|pb\.go|runtimedefault|coverprofile|go test .*cover" .github/workflows . || trueRepository: Prairie-Server/prairie-plugin-sdk
Length of output: 242
🌐 Web query:
GitHub Actions default shell Linux bash pipefail documentation run step
💡 Result:
In GitHub Actions, the shell used for run steps depends on whether you explicitly specify a shell and whether the runner is on a Linux/macOS or Windows platform [1]. The default behavior on Linux and macOS is as follows: 1. Unspecified shell: If no shell is specified for a run step on a Linux or macOS runner, GitHub Actions uses a default command equivalent to bash -e {0} [1]. This default mode does not include the pipefail option [1][2]. If bash is not found on the system path, it falls back to sh -e {0} [1]. 2. Explicit bash shell: If you explicitly specify shell: bash in your workflow, GitHub Actions uses the command bash --noprofile --norc -eo pipefail {0} [1]. This explicitly includes the pipefail option, which ensures that a pipeline returns a non-zero exit code if any command in the pipeline fails [1][2]. To ensure that your run steps utilize pipefail behavior, it is recommended to explicitly set the shell to bash [2]: - name: Example step shell: bash run: | command_that_might_fail | command_that_succeeds If you require custom shell behavior beyond the defaults provided, you can specify a custom template string for the shell option, such as shell: bash -eo pipefail {0} [3][2]. Note that for jobs running inside a container, the default shell is typically sh rather than bash unless otherwise configured [4].
Citations:
- 1: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax
- 2: Default
bashshell doesn't seem to run with-o pipefailactions/runner-images#4459 - 3: https://github.com/actions/runner/blob/main/docs/adrs/0277-run-action-shell-options.md
- 4: https://docs.github.com/en/actions/how-tos/write-workflows/choose-where-workflows-run/run-jobs-in-a-container
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ci.yml section ==\n'
sed -n '45,70p' .github/workflows/ci.yml
printf '\n== search for coverage/protobuf filtering ==\n'
grep -RIn --exclude-dir=.git -E 'coverage\.raw|coverage\.out|pb\.go|runtimedefault|coverprofile|go test .*cover' .github/workflows .Repository: Prairie-Server/prairie-plugin-sdk
Length of output: 3461
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ci.yml section ==\n'
sed -n '45,70p' .github/workflows/ci.yml
printf '\n== coverage/protobuf references under repository ==\n'
grep -RIn --exclude-dir=.git -E 'coverage\.raw|coverage\.out|pb\.go|runtimedefault|coverprofile|go test .*cover' .Repository: Prairie-Server/prairie-plugin-sdk
Length of output: 3166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== scripts/check-coverage.sh ==\n'
sed -n '1,160p' scripts/check-coverage.sh
printf '\n== coverage/raw post-processing references ==\n'
grep -RIn --exclude-dir=.git -E 'coverage\.raw|awk .*pb\.go|sed .*pb\.go|filter.*pb\.go' scripts .github/workflows || trueRepository: Prairie-Server/prairie-plugin-sdk
Length of output: 919
Keep coverage generation fail-closed and drop generated protobufs. The current step still writes .pb.go entries into coverage.out, and the go list | grep substitution runs under the default bash -e shell without pipefail, so a go list failure can still fall through to go test with a partial package set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 54 - 57, Update the “Run tests with
coverage” workflow step to exclude generated .pb.go files from coverage.out and
make package discovery fail closed: enable pipefail so go list or grep failures
stop the step rather than producing a partial package list. Preserve the
existing runtimedefault exclusion and coverage test behavior.
| # Fail if total statement coverage is below COVER_MIN (percent). | ||
| set -euo pipefail | ||
| COVER_MIN="${COVER_MIN:-70}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the fallback with the documented 65% floor.
CI sets COVER_MIN=65, but standalone execution defaults to 70, so local runs can reject coverage that passes CI. Use 65 as the fallback or centralize the threshold.
Proposed fix
-COVER_MIN="${COVER_MIN:-70}"
+COVER_MIN="${COVER_MIN:-65}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Fail if total statement coverage is below COVER_MIN (percent). | |
| set -euo pipefail | |
| COVER_MIN="${COVER_MIN:-70}" | |
| # Fail if total statement coverage is below COVER_MIN (percent). | |
| set -euo pipefail | |
| COVER_MIN="${COVER_MIN:-65}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check-coverage.sh` around lines 2 - 4, Update the COVER_MIN fallback
in the coverage-check script from 70 to the documented 65 percent floor, while
preserving the ability for an explicitly set COVER_MIN value to override it.
| awk -v total="$total" -v min="$COVER_MIN" 'BEGIN { | ||
| if (total+0 < min+0) { | ||
| printf "coverage %.1f%% is below required %.1f%%\n", total, min > "/dev/stderr" | ||
| exit 1 | ||
| } | ||
| printf "coverage %.1f%% (min %.1f%%)\n", total, min |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
awk -v min='abc' 'BEGIN { print min+0 < 65+0 ? "fail" : "pass" }'Repository: Prairie-Server/prairie-plugin-sdk
Length of output: 176
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
wc -l scripts/check-coverage.sh
cat -n scripts/check-coverage.sh | sed -n '1,80p'Repository: Prairie-Server/prairie-plugin-sdk
Length of output: 1015
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
wc -l scripts/check-coverage.sh
cat -n scripts/check-coverage.sh | sed -n '1,120p'Repository: Prairie-Server/prairie-plugin-sdk
Length of output: 1015
🏁 Script executed:
python3 - <<'PY'
import subprocess, textwrap
cases = [
("80", "abc"),
("80", "-5"),
("80", "70"),
("50", "70"),
]
for total, minv in cases:
script = textwrap.dedent(f"""\
awk -v total='{total}' -v min='{minv}' 'BEGIN {{
if (total+0 < min+0) {{
print "reject"
}} else {{
print "accept"
}}
}}'
""")
out = subprocess.check_output(["bash", "-lc", script], text=True).strip()
print(total, minv, "=>", out)
PYRepository: Prairie-Server/prairie-plugin-sdk
Length of output: 236
Reject malformed COVER_MIN values before the compare. min+0 coerces abc and negative inputs to 0, so invalid thresholds can pass the gate. Validate COVER_MIN is numeric and within 0..100 first.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/check-coverage.sh` around lines 15 - 20, Validate COVER_MIN before
invoking the awk comparison, requiring a numeric value between 0 and 100 and
rejecting malformed or negative thresholds with a nonzero exit. Update the
coverage-check flow around the awk BEGIN block so only validated COVER_MIN
values reach the existing total-versus-min comparison.
2 participants
Summary
Adds the same class of CI quality gates used by prairie-server / client repos.
Changes
.golangci.yml(gofmt/goimports, govet, staticcheck, unused, …)./pkg/pluginsdk/...(excludes generated proto +runtimedefault)scripts/check-coverage.shTest plan
go test+ coverage scriptSummary by CodeRabbit