chore: add pre-commit linters and CI lint workflow - #29
Conversation
PR Summary by QodoAdd pre-commit linters and CI workflow for file + commit linting
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
|
🤖 Review · |
|
🤖 Review · |
Code Review by Qodo
Context used✅ Compliance rules (platform):
55 rules 1. Unsanitized sha in ::error::
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| for sha in $(git rev-list --no-merges "${RANGE}"); do | ||
| git log --format='%s' -1 "${sha}" > /tmp/commit-msg.txt | ||
| if ! uvx --from gitlint-core gitlint --config .gitlint --ignore B6 --msg-filename /tmp/commit-msg.txt; then | ||
| echo "::error::Commit ${sha} does not follow Conventional Commits format" | ||
| FAILED=true |
There was a problem hiding this comment.
1. Unsanitized sha in ::error:: 📜 Skill insight ⛨ Security
The workflow emits a GitHub Actions workflow command using echo "::error::...${sha}..." without
sanitizing the interpolated sha value. This can enable workflow-command injection if the
interpolated value contains ::, encoded newlines, or control characters.
Agent Prompt
## Issue description
A GitHub Actions workflow command (`::error::...`) is emitted with an interpolated variable (`${sha}`) that is not sanitized, violating the requirement that *all* interpolated values in workflow commands be sanitized individually.
## Issue Context
The command is printed inside the `Lint commits` step. Even if `sha` is expected to be a commit hash, the compliance requirement is to sanitize each interpolated value regardless of perceived risk.
## Fix Focus Areas
- .github/workflows/lint.yml[74-78]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| name: CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| pull_request: | ||
| branches: [main] | ||
| merge_group: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
There was a problem hiding this comment.
2. Protected paths modified 📜 Skill insight § Compliance
This PR modifies protected governance/infrastructure paths (.github/workflows/lint.yml and .pre-commit-config.yaml), which must be explicitly flagged for required human review. Changes in these paths must not be auto-approved.
Agent Prompt
## Issue description
Protected governance/infrastructure paths are modified in this PR, which requires explicit justification and mandatory human review.
## Issue Context
Protected paths include `.github/` and `.pre-commit-config.yaml`. Provide an explicit issue/ADR link or justification in-repo (e.g., header comments) to document authorization for these governance changes.
## Fix Focus Areas
- .github/workflows/lint.yml[1-12]
- .pre-commit-config.yaml[1-48]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| name: CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| pull_request: | ||
| branches: [main] | ||
| merge_group: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
|
|
||
| - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 | ||
| with: | ||
| python-version: "3.12" | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 | ||
|
|
||
| - name: Install pre-commit | ||
| run: uv pip install --system pre-commit | ||
|
|
||
| - name: Install pinact | ||
| run: | | ||
| curl -sSfL "https://github.com/suzuki-shunsuke/pinact/releases/download/v4.1.0/pinact_linux_amd64.tar.gz" -o /tmp/pinact.tar.gz | ||
| echo "8fcbf1b3e95551c82fd995535e3c1defa70e23299ce36eb3afd6c98778de6ca0 /tmp/pinact.tar.gz" | sha256sum -c | ||
| tar xzf /tmp/pinact.tar.gz -C /usr/local/bin pinact | ||
|
|
||
| - name: Run pre-commit on all files | ||
| run: pre-commit run --all-files | ||
|
|
||
| commit-lint: | ||
| runs-on: ubuntu-24.04 | ||
| steps: | ||
| - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 | ||
|
|
||
| - name: Lint PR title | ||
| if: github.event_name == 'pull_request' | ||
| env: | ||
| PR_TITLE: ${{ github.event.pull_request.title }} | ||
| run: | | ||
| echo "${PR_TITLE}" > /tmp/pr-title.txt | ||
| uvx --from gitlint-core gitlint --config .gitlint --ignore B6 --msg-filename /tmp/pr-title.txt | ||
|
|
||
| - name: Lint commits | ||
| env: | ||
| EVENT_NAME: ${{ github.event_name }} | ||
| PUSH_BEFORE: ${{ github.event.before }} | ||
| PUSH_AFTER: ${{ github.sha }} | ||
| MQ_BASE: ${{ github.event.merge_group.base_sha }} | ||
| MQ_HEAD: ${{ github.event.merge_group.head_sha }} | ||
| PR_BASE: ${{ github.event.pull_request.base.sha }} | ||
| PR_HEAD: ${{ github.event.pull_request.head.sha }} | ||
| run: | | ||
| case "${EVENT_NAME}" in | ||
| push) RANGE="${PUSH_BEFORE}..${PUSH_AFTER}" ;; | ||
| merge_group) RANGE="${MQ_BASE}..${MQ_HEAD}" ;; | ||
| pull_request) RANGE="${PR_BASE}..${PR_HEAD}" ;; | ||
| *) echo "Unknown event: ${EVENT_NAME}"; exit 1 ;; | ||
| esac | ||
|
|
||
| FAILED=false | ||
| for sha in $(git rev-list --no-merges "${RANGE}"); do | ||
| git log --format='%s' -1 "${sha}" > /tmp/commit-msg.txt | ||
| if ! uvx --from gitlint-core gitlint --config .gitlint --ignore B6 --msg-filename /tmp/commit-msg.txt; then | ||
| echo "::error::Commit ${sha} does not follow Conventional Commits format" | ||
| FAILED=true | ||
| fi | ||
| done | ||
| if ${FAILED}; then | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
3. No linked issue authorization 📜 Skill insight § Compliance
This PR introduces non-trivial CI/linting infrastructure changes but does not include an explicit linked issue authorizing the work. Non-trivial changes require a linked issue for authorization.
Agent Prompt
## Issue description
Non-trivial changes require explicit authorization via a linked issue.
## Issue Context
This PR adds substantial new CI and lint infrastructure (workflow + pre-commit + gitlint config). Add an issue/ADR reference (and ideally link it in the PR description) that authorizes introducing these governance changes.
## Fix Focus Areas
- .github/workflows/lint.yml[1-83]
- .pre-commit-config.yaml[1-48]
- .gitlint[1-10]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| - name: Lint PR title | ||
| if: github.event_name == 'pull_request' | ||
| env: | ||
| PR_TITLE: ${{ github.event.pull_request.title }} | ||
| run: | | ||
| echo "${PR_TITLE}" > /tmp/pr-title.txt | ||
| uvx --from gitlint-core gitlint --config .gitlint --ignore B6 --msg-filename /tmp/pr-title.txt | ||
|
|
There was a problem hiding this comment.
5. Gitlint b6 mismatch 🐞 Bug ⚙ Maintainability
CI runs gitlint with --ignore B6, but the local commit-msg pre-commit gitlint hook is configured without that ignore, so commits/PR titles can be rejected locally while still passing CI.
Agent Prompt
## Issue description
CI and local pre-commit enforce different gitlint rules: CI ignores rule `B6` while the pre-commit `commit-msg` hook does not. This creates inconsistent behavior for contributors.
## Issue Context
CI invokes: `gitlint --ignore B6 ...`. The pre-commit hook configuration for gitlint does not pass `--ignore B6`.
## Fix Focus Areas
- .github/workflows/lint.yml[48-55]
- .pre-commit-config.yaml[19-24]
## Suggested fixes
Pick one and apply consistently:
- Option A (match CI): add `args: [--ignore, B6]` to the pre-commit `gitlint` hook.
- Option B (match local): remove `--ignore B6` from CI so CI enforces the same rules as the hook.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| run: | | ||
| echo "${PR_TITLE}" > /tmp/pr-title.txt | ||
| uvx --from gitlint-core gitlint --config .gitlint --ignore B6 --msg-filename /tmp/pr-title.txt | ||
|
|
There was a problem hiding this comment.
6. Unpinned gitlint-core 🐞 Bug ☼ Reliability
The commit-lint job runs uvx --from gitlint-core gitlint without pinning a version, so CI behavior can drift over time and diverge from the pre-commit hook’s pinned gitlint version.
Agent Prompt
## Issue description
`uvx --from gitlint-core gitlint` does not pin a specific version, so CI lint results can change when `gitlint-core` releases new versions.
## Issue Context
Local pre-commit pins gitlint via the pre-commit repo `rev: v0.19.1`, but CI resolves `gitlint-core` dynamically.
## Fix Focus Areas
- .github/workflows/lint.yml[52-55]
- .pre-commit-config.yaml[19-21]
## Suggested fixes
- Pin the version used by `uvx`, e.g. `uvx --from 'gitlint-core==0.19.1' gitlint ...` (or the exact version you want to standardize on).
- Alternatively, standardize by running gitlint via pre-commit in CI (so CI uses the same pinned hook).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
🤖 Review · |
Add .pre-commit-config.yaml with applicable linters from fullsend repo: - Syntax checks: check-yaml, check-json, check-toml - Hygiene: trailing-whitespace, end-of-file-fixer, mixed-line-ending - Security: detect-private-key, gitleaks, check-added-large-files - Shell: shellcheck (with SC1091, SC2001, SC2016 ignored) - GitHub Actions: actionlint, pinact (scoped to lint.yml) - Commits: gitlint (conventional commits, commit-msg hook) Add .gitlint config enforcing conventional commit format. Add .github/workflows/lint.yml CI workflow with two jobs: - test: runs pre-commit on all files - commit-lint: lints PR title and individual commit messages Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
291e098 to
16f645d
Compare
|
🤖 Finished Review · ✅ Success · Started 5:28 PM UTC · Completed 5:39 PM UTC |
ReviewVerdict: Approve Clean addition of pre-commit linting infrastructure and CI workflow. The three new files are well-structured, follow existing repo conventions (SHA-pinned actions, A few low-severity observations for follow-up: Low findings1. pinact scope limited to 2. Workflow/job naming ( 3. Commit-lint range edge cases ( 4. README documentation gap (
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| hooks: | ||
| - id: pinact | ||
| name: pinact (SHA-pin check) | ||
| entry: pinact run --fix=false --no-api .github/workflows/lint.yml |
There was a problem hiding this comment.
[low] incomplete-coverage
The pinact hook hardcodes the entry command and files regex to only check .github/workflows/lint.yml. The repo also has release.yml (already SHA-pinned) and fullsend.yaml (externally managed). Future workflow files added to the repo would silently bypass the SHA-pin enforcement.
Suggested fix: Broaden the hook: files: ^.github/workflows/.*.ya?ml$ with an exclude for fullsend.yaml, and remove the hardcoded path from the entry command.
| @@ -0,0 +1,83 @@ | |||
| name: CI | |||
There was a problem hiding this comment.
[low] naming-convention
Workflow name 'CI' is generic compared to existing descriptive names ('fullsend', 'Release'). Job name 'test' (line 14) is misleading for a job that runs pre-commit linters.
Suggested fix: Rename the workflow to 'Lint' and the first job to 'lint' or 'pre-commit' for clarity.
| merge_group) RANGE="${MQ_BASE}..${MQ_HEAD}" ;; | ||
| pull_request) RANGE="${PR_BASE}..${PR_HEAD}" ;; | ||
| *) echo "Unknown event: ${EVENT_NAME}"; exit 1 ;; | ||
| esac |
There was a problem hiding this comment.
[low] edge-case
The push-event commit range does not handle the null SHA (force-push or initial push to main). The pull_request range uses base branch tip rather than merge-base, which may enumerate upstream commits if the PR branch has merged main.
Suggested fix: Guard the push case against null SHA (0{40}). For pull_request, consider using git merge-base to compute the fork point.
fullsend-ai-review
Bot
added
the
requires-manual-review
|
🤖 Finished Retro · ✅ Success · Started 10:30 PM UTC · Completed 10:35 PM UTC |
Retro: PR #29 — Initial repo bootstrap with linters and CIPR #29 bootstrapped the The retro focused on the linting and CI setup — the stated scope of the PR. Two concrete gaps were identified:
Note: Could not verify whether open issues already cover these proposals because the GitHub token was not available for API access. If duplicates exist, these proposals should be closed as duplicates. Proposals filed
|
2 participants
Summary
.pre-commit-config.yamlwith applicable linters from fullsend repo:.gitlintconfig matching fullsend repo conventions.github/workflows/lint.ymlCI workflow with two jobs:test— runspre-commit run --all-filescommit-lint— lints PR title and individual commit messagesLinters from fullsend that are not applicable (no Go, Python, TypeScript, or frontend code in this repo): golangci-lint, gofmt, go-vet, ruff, ty, bandit, ESLint, Prettier, Stylelint, svelte-check, lint-staged, and the custom
hack/lint-*scripts.Test plan
🤖 Generated with Claude Code