.github/workflows/ci.yaml (2)

452-462: Job dependency chain creates significant delay before formatting feedback.

The job depends on 7 prior jobs, which means formatting comments won't appear until all of those jobs finish. Consider whether this is intentional (to avoid conflicts) or whether an earlier checkpoint would be preferable.

---

481-493: Use git add -A to capture all changes (including untracked files).

Line 487 uses git add -u, which only stages modifications to already-tracked files. While cargo fmt typically doesn't create new files, using git add -A or git add . is more defensive:

-          git add -u
+          git add -A

Additionally, consider checking the git push exit code to ensure the push succeeded before setting formatted=true.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to extra_tests/**/*.py : Use ruff to lint custom Python code

.github/workflows/ci.yaml (1)

459-459: CRITICAL: Condition fails on pull_request events.

Line 459 accesses github.event.head_commit.message, which is only available on push events. The workflow triggers on both push and pull_request (lines 2–5), so this condition is broken for PR events. For PRs, github.event.head_commit is null, causing contains(null, '[skip ci]') to behave unpredictably—the job will likely run unconditionally.

This matches the root cause identified in the previous review: the workflow works on your fork (where you have permissions) but fails here because the condition doesn't distinguish event types. Per previous review recommendations, the simplest fix is to skip auto-format for fork PRs by checking the event type:

-    if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
+    if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository) }}

This skips the job for fork PRs (where you don't have write permissions anyway) and checks the commit message only on push events where the context is available.

.github/workflows/ci.yaml (1)

459-459: Align skip condition with other jobs.

Other jobs in this workflow use !contains(github.event.pull_request.labels.*.name, 'skip:ci') to determine whether to run (lines 111, 176, 237, etc.). The new job uses a different mechanism: commit message matching. For consistency and predictability, consider aligning the auto-format job to use the same label-based skip pattern already established throughout the workflow:

-    if: ${{ !contains(github.event.head_commit.message, '[skip ci]') }}
+    if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip:ci') }}

This ensures that all jobs respond to the same skip:ci label, making the workflow behavior uniform and easier to reason about.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to extra_tests/**/*.py : Use ruff to lint custom Python code

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes

.github/workflows/pr-auto-commit.yaml (2)

16-19: ⚠️ Critical: Add fork guard to prevent workflow failure on forked PRs.

The job condition lacks a check to ensure the PR originates from the base repository. When this workflow runs on PRs from forked repositories, GitHub Actions in the base repo cannot push to the fork's branch, causing the workflow to fail. This is why the workflow succeeds in the author's fork but fails in the main repository.

Add a guard to skip auto-format for forked PRs:

  auto_format:
    if: |
      !contains(github.event.pull_request.labels.*.name, 'skip:ci') &&
      !contains(github.event.pull_request.head.sha, '[skip ci]') &&
+     github.event.pull_request.head.repo.full_name == github.repository

This ensures the job only runs for PRs opened against the main repository, not forked PRs.

---

67-76: ⚠️ Major: Mitigate script injection risk by passing ref through environment variable.

Line 76 directly interpolates github.event.pull_request.head.ref into the inline shell script. An untrusted PR ref could be crafted to inject arbitrary shell commands. Per GitHub's security guidance, pass the ref through an environment variable instead:

  - name: Commit and push formatting changes
    if: steps.check_changes.outputs.has_changes == 'true'
+   env:
+     HEAD_REF: ${{ github.event.pull_request.head.ref }}
    run: |
      git config user.name "github-actions[bot]"
      git config user.email "github-actions[bot]@users.noreply.github.com"
      
      git add -u
      git commit -m "Auto-format code [skip ci]"
      
-     git push origin HEAD:${{ github.event.pull_request.head.ref }}
+     git push origin HEAD:"$HEAD_REF"

.github/workflows/pr-auto-commit.yaml (1)

78-95: Minor: Consistent script variable handling in PR comment message.

Line 94 also interpolates ${{ github.event.pull_request.head.ref }} directly. While this is in a message/documentation context (lower risk than the git push command), apply the same mitigation for consistency and to establish a pattern:

  - name: Comment on PR
    if: steps.check_changes.outputs.has_changes == 'true'
+   env:
+     HEAD_REF: ${{ github.event.pull_request.head.ref }}
    uses: marocchino/sticky-pull-request-comment@v2
    with:
      number: ${{ github.event.pull_request.number }}
      message: |
        **Code has been automatically formatted**
        
        The code in this PR has been formatted using `cargo fmt`.
        The changes have been committed with `[skip ci]` to avoid triggering another CI run.
        
        **Triggered by commit:** `${{ github.event.pull_request.head.sha }}`  
        **Last formatted:** ${{ github.event.pull_request.updated_at }}
        
        You may need to pull the latest changes before pushing again:
        ```bash
-       git pull origin ${{ github.event.pull_request.head.ref }}
+       git pull origin "$HEAD_REF"
        ```

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Format Rust code with the default rustfmt style (run `cargo fmt`)

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-11T05:06:59.971Z
Learning: Applies to **/*.rs : Run clippy and fix any warnings or lints introduced by your changes

Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.