ci: forward-only lint gate + non-blocking type/test/lock jobs + pre-commit#147
Merged
Conversation
…ommit - ci.yml: ruff on changed Python files (pinned 0.14.4, blocking); pyright, full pytest, and Modal lean/browser/vision lockfile freshness as non-blocking signal jobs (matches the existing py3.12 canary idiom) - pytest.ini: register browser/vision/live markers for the lean CI lane - .pre-commit-config.yaml: ruff + gitleaks (reuses .gitleaks.toml) + hygiene - CONTRIBUTING.md: document pre-commit setup + forward-only CI policy
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughExisting CI test jobs switch from ChangesCI and Developer Tooling Overhaul
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Codex Exhaustive Code ReviewFindings
No production API, auth, DB, migration, or backend contract changes were introduced in this PR. I did not find blocking backend correctness findings beyond the CI filename-handling issue. Validation run: |
There was a problem hiding this comment.
Pull request overview
This PR hardens the repo’s developer tooling by adding a forward-only CI lint gate (Ruff on changed Python files) plus non-blocking signal jobs (pyright, full pytest lane, Modal lockfile freshness) and a local pre-commit setup to catch issues earlier without forcing an immediate backlog cleanup.
Changes:
- Add a blocking Ruff “changed files only” lint/format gate to CI, and introduce non-blocking typecheck/test/lockfile-freshness jobs.
- Add a
.pre-commit-config.yamlwith Ruff, Gitleaks, and basic hygiene hooks. - Register
browser/vision/livepytest markers and document the forward-only policy in CONTRIBUTING.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
pytest.ini |
Registers CI-excluded markers (browser, vision, live) used by the new “full pytest” CI lane. |
CONTRIBUTING.md |
Documents pre-commit setup and the forward-only CI lint policy. |
.pre-commit-config.yaml |
Adds staged-file hooks for Ruff + formatting, Gitleaks, and YAML/whitespace hygiene. |
.github/workflows/ci.yml |
Adds Ruff forward-only lint gate and non-blocking signal jobs; standardizes installs on uv pip sync from lockfiles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| || ! git cat-file -e "$base" 2>/dev/null; then | ||
| base="$(git rev-parse HEAD~1 2>/dev/null || git rev-parse HEAD)" | ||
| fi | ||
| files="$(git diff --name-only --diff-filter=ACM "$base"...HEAD -- '*.py' | tr '\n' ' ')" |
Comment on lines
+158
to
+162
| - name: Install dependencies | ||
| run: | | ||
| uv pip sync --system requirements.lock.txt | ||
| uv pip install --system pyright | ||
|
|
Comment on lines
+36
to
+38
| Hooks run `ruff`, `gitleaks`, and basic hygiene checks on staged files before | ||
| each commit. They only touch files you actually change, so the existing | ||
| lint/format backlog will not block unrelated commits. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)
198-221: ⚡ Quick winConsider adding explicit
permissionsblock following least-privilege principle.Same as other jobs: explicitly declare minimal permissions (likely
contents: read).🔒 Proposed fix
test-full: name: Full pytest (non-blocking) runs-on: ubuntu-latest continue-on-error: true + permissions: + contents: read steps:🤖 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 198 - 221, The test-full job is missing an explicit permissions block that declares minimal required permissions following the least-privilege principle. Add a permissions block at the job level (at the same indentation as runs-on and continue-on-error) with contents: read permission, which matches the security posture of other jobs in the workflow file.Source: Linters/SAST tools
97-137: ⚡ Quick winConsider adding explicit
permissionsblock following least-privilege principle.The job inherits default write permissions. Following least-privilege principle, explicitly declare the minimal permissions needed (likely just
contents: readfor this job).🔒 Proposed fix
lint: name: Ruff (changed files, forward-only) runs-on: ubuntu-latest + permissions: + contents: read steps:🤖 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 97 - 137, The lint job is missing an explicit permissions block and inherits default write permissions, violating the least-privilege principle. Add a permissions block to the lint job that explicitly declares only contents: read permission, which is the minimal access required for this job to checkout the repository and run the linting checks.Source: Linters/SAST tools
140-164: ⚡ Quick winConsider adding explicit
permissionsblock following least-privilege principle.Same as the
lintjob: explicitly declare minimal permissions (likelycontents: read).🔒 Proposed fix
typecheck: name: Pyright (non-blocking) runs-on: ubuntu-latest continue-on-error: true + permissions: + contents: read steps:🤖 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 140 - 164, The typecheck job is missing an explicit permissions block, which violates the least-privilege security principle for GitHub Actions workflows. Add a permissions block at the job level in the typecheck job (before the runs-on field) and explicitly declare minimal required permissions. Since this job only needs to checkout and read the repository contents to run Pyright type checking, set the permissions to contents: read only, removing any unnecessary access similar to what should be done in other jobs.Source: Linters/SAST tools
167-195: ⚡ Quick winConsider adding explicit
permissionsblock following least-privilege principle.Same as other jobs: explicitly declare minimal permissions (likely
contents: read).🔒 Proposed fix
modal-locks: name: Modal lockfile freshness (non-blocking) runs-on: ubuntu-latest continue-on-error: true + permissions: + contents: read steps:🤖 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 167 - 195, The modal-locks job is missing an explicit permissions block that should follow the least-privilege principle. Add a permissions block to the modal-locks job (the job definition that starts with the name "Modal lockfile freshness (non-blocking)") and declare the minimal required permission which is likely contents: read, to allow the job to check out the repository. This ensures the job follows the same security best practices as other jobs in the workflow by explicitly limiting its access.Source: Linters/SAST tools
🤖 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 101-104: The actions/checkout@v5 action in the Checkout step is
missing the security hardening setting persist-credentials: false. Add
persist-credentials: false to the with block of the checkout action, alongside
the existing fetch-depth: 0 configuration, to prevent the GitHub token from
persisting in the workspace where it could potentially be accessed by malicious
code.
- Around line 145-146: The actions/checkout@v5 step is missing the
persist-credentials security parameter. Add a with section to the Checkout step
and set persist-credentials to false to prevent GitHub token persistence in the
git configuration, which is a security hardening best practice for CI workflows.
- Around line 203-204: The checkout action in the Checkout step is missing the
security hardening configuration. Add a with section to the actions/checkout@v5
action and set persist-credentials to false to prevent Git credentials from
being persisted in the workflow runner's Git configuration, which is a security
best practice.
- Around line 172-173: The checkout action needs security hardening by disabling
credential persistence. In the checkout action step (currently using
actions/checkout@v5), add a `with` section that sets `persist-credentials:
false` to prevent GitHub tokens from being persisted to the Git configuration,
which reduces the risk of credential leakage in the workflow.
- Around line 133-137: The ${{ steps.changed.outputs.files }} variable is
unquoted in both the ruff check and ruff format command lines, which could lead
to shell injection if filenames contain spaces or special characters. Add double
quotes around the variable in both the "ruff@0.14.4 check" and "ruff@0.14.4
format --check" run commands to properly escape and handle filenames with
special characters.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 198-221: The test-full job is missing an explicit permissions
block that declares minimal required permissions following the least-privilege
principle. Add a permissions block at the job level (at the same indentation as
runs-on and continue-on-error) with contents: read permission, which matches the
security posture of other jobs in the workflow file.
- Around line 97-137: The lint job is missing an explicit permissions block and
inherits default write permissions, violating the least-privilege principle. Add
a permissions block to the lint job that explicitly declares only contents: read
permission, which is the minimal access required for this job to checkout the
repository and run the linting checks.
- Around line 140-164: The typecheck job is missing an explicit permissions
block, which violates the least-privilege security principle for GitHub Actions
workflows. Add a permissions block at the job level in the typecheck job (before
the runs-on field) and explicitly declare minimal required permissions. Since
this job only needs to checkout and read the repository contents to run Pyright
type checking, set the permissions to contents: read only, removing any
unnecessary access similar to what should be done in other jobs.
- Around line 167-195: The modal-locks job is missing an explicit permissions
block that should follow the least-privilege principle. Add a permissions block
to the modal-locks job (the job definition that starts with the name "Modal
lockfile freshness (non-blocking)") and declare the minimal required permission
which is likely contents: read, to allow the job to check out the repository.
This ensures the job follows the same security best practices as other jobs in
the workflow by explicitly limiting its access.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 84ad673c-8a4d-49da-b7f8-5b885a61b0b3
📒 Files selected for processing (4)
.github/workflows/ci.yml.pre-commit-config.yamlCONTRIBUTING.mdpytest.ini
…entials, injection-safe ruff args) - lint: diff-filter ACM->ACMR (catch renames); pass changed files via env + xargs instead of template interpolation (no shell injection) - pin pyright==1.1.390 for reproducible non-blocking typecheck - persist-credentials: false on lint/typecheck/modal-locks/test-full checkouts - CONTRIBUTING: note pre-commit hooks auto-fix in place and require re-staging
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 1 of the tooling upgrade — CI hardening + local pre-commit. Pure tooling/config; touches no application Python.
CI (
.github/workflows/ci.yml)lint(blocking): runsruffonly on the Python files changed in the PR (pinned 0.14.4) — forward-only so the existing lint backlog doesn't block, while new code is held to standard.typecheck(pyright),test-full(full pytest minus browser/vision/live),modal-locks(lean/browser/vision lockfile freshness): non-blocking signal jobs, matching the existing py3.12continue-on-errorcanary idiom. Flip to blocking once each backlog is burned down.Other
pytest.ini: registerbrowser/vision/livemarkers used bytest-full..pre-commit-config.yaml: ruff + gitleaks (reuses existing.gitleaks.toml) + hygiene hooks, staged-files only.CONTRIBUTING.md: document setup + the forward-only policy.Notes
ci.ymlinstall-step tweak (uv pip sync --system) already in the working tree.Test
All new jobs validated locally: pre-commit configs pass
pre-commit validate-config; workflow YAML parses; pytest marker filter collects clean.Summary by CodeRabbit
browser,vision, andlivemarkers to better organize and filter test execution.