Fix CI to test merge result instead of fork branch in isolation#562
Conversation
|
Warning Review limit reached
More reviews will be available in 35 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces blog pull request automation, documentation, and CI improvements. It adds a new GitHub Actions workflow to automatically assign blog reviewer teams based on front matter ChangesBlog Review Automation and CI
Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| .github/workflows/zola-deploy.yml | Removed explicit fork ref/repository overrides from the Checkout step; for PR events the ref resolves to an empty string, letting actions/checkout fall back to its default GITHUB_SHA (refs/pull/N/merge) |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Workflow Triggered] --> B{github.event_name}
B -->|pull_request| C["ref = '' (empty string)"]
B -->|push / workflow_dispatch / repository_dispatch| D["ref = 'main'"]
C --> E["actions/checkout falls back to GITHUB_SHA\n= refs/pull/N/merge (merge commit)"]
D --> F["actions/checkout checks out main branch"]
E --> G[Build & test merged result]
F --> H[Build & deploy main]
Reviews (4): Last reviewed commit: "Fix CI to test merge result instead of f..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
madolson
force-pushed
the
fix-ci-merge-ref
branch
from
8a961b5 to
3cf89a7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/assign-blog-reviewers.yml:
- Line 17: The workflow uses a moving tag "actions/github-script@v7" which can
drift; replace that tag with a pinned commit SHA for the actions/github-script
action (e.g., "actions/github-script@<commit_sha>") and optionally keep the
human-readable tag in a trailing comment (e.g., "# v7") so the step remains
reproducible and auditable; update the uses line in the assign-blog-reviewers
workflow to reference the specific commit SHA of actions/github-script at the v7
release.
- Around line 9-10: The workflow permissions only grant pull-requests: write but
the job calls github.rest.repos.getContent which requires contents: read; update
the workflow's permissions block to include contents: read alongside
pull-requests: write so github.rest.repos.getContent(...) can succeed before
calling github.rest.pulls.requestReviewers(...). Ensure the permissions YAML
includes the contents: read key at the same level as pull-requests.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2f72a0b6-26f7-48db-a5bb-e12e9dc927c4
📒 Files selected for processing (3)
.github/workflows/assign-blog-reviewers.yml.github/workflows/zola-deploy.ymlcontent/blog/README
💤 Files with no reviewable changes (1)
- .github/workflows/zola-deploy.yml
| permissions: | ||
| pull-requests: write |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What GitHub Actions GITHUB_TOKEN permission is required for the REST API endpoint used by github.rest.repos.getContent, and when a workflow declares permissions, do unspecified scopes default to none?
💡 Result:
To use the REST API endpoint associated with github.rest.repos.getContent (which corresponds to the "Get repository content" endpoint), the GITHUB_TOKEN requires read access to the contents scope [1][2][3]. Regarding your question about the permissions key in GitHub Actions workflows: Yes, when a workflow (or individual job) declares a permissions block, all unspecified scopes default to none [3][4][5]. The only exception to this rule is the metadata scope, which always retains read access [6]. This behavior effectively restricts the token to only the permissions explicitly granted, overriding any broader default permissions inherited from the enterprise, organization, or repository levels [3][4][5].
Citations:
- 1: https://docs.github.com/en/rest/repos/contents
- 2: https://docs.github.com/enterprise-server@3.20/rest/repos/contents
- 3: https://docs.github.com/actions/using-workflows/workflow-syntax-for-github-actions
- 4: https://www.kenmuse.com/blog/github-actions-workflow-permissions/
- 5: https://arinco.com.au/blog/changes-to-the-default-github_token-permissions/
- 6: "Modifying the permissions for the
GITHUB_TOKEN" should use a callout box for "When thepermissionskey is used, all unspecified permissions are set to no access" github/docs#35810
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the workflow file with line numbers around permissions and any getContent usage.
FILE=".github/workflows/assign-blog-reviewers.yml"
echo "=== File header / permissions block (lines 1-60) ==="
nl -ba "$FILE" | sed -n '1,80p'
echo "=== Search for repos.getContent and pull request review assignment logic ==="
rg -n "repos\.getContent|pulls\.requestReviewers|pulls\.listFiles|pull_request_target" "$FILE" || true
echo "=== Show full relevant sections (lines 1-140) ==="
nl -ba "$FILE" | sed -n '1,160p'Repository: valkey-io/valkey-io.github.io
Length of output: 187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/assign-blog-reviewers.yml"
python3 - <<'PY'
import itertools
path = ".github/workflows/assign-blog-reviewers.yml"
with open(path,"r",encoding="utf-8") as f:
lines = list(f)
def show(a,b):
for i in range(a-1,b):
if i<0 or i>=len(lines): continue
print(f"{i+1:4d}: {lines[i].rstrip()}")
print("=== lines 1-120 ===")
show(1,120)
PY
echo "=== rg for key APIs ==="
rg -n "repos\.getContent|pulls\.requestReviewers|pulls\.listFiles|pull_request_target|permissions:" "$FILE" || true
echo "=== Show lines 40-120 (likely where scripts call APIs) ==="
python3 - <<'PY'
path = ".github/workflows/assign-blog-reviewers.yml"
with open(path,"r",encoding="utf-8") as f:
lines = list(f)
for i in range(40,121):
if i-1>=len(lines): break
print(f"{i:4d}: {lines[i-1].rstrip()}")
PYRepository: valkey-io/valkey-io.github.io
Length of output: 6538
Add contents: read to workflow permissions
The workflow grants only pull-requests: write, but the job’s script calls github.rest.repos.getContent(...) (lines ~46-51). repos.getContent needs contents: read; with permissions set, unspecified scopes are disabled, so this can 403 before github.rest.pulls.requestReviewers(...) runs.
Suggested fix
permissions:
+ contents: read
pull-requests: write📝 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: | |
| pull-requests: write | |
| permissions: | |
| contents: read | |
| pull-requests: write |
🤖 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/assign-blog-reviewers.yml around lines 9 - 10, The
workflow permissions only grant pull-requests: write but the job calls
github.rest.repos.getContent which requires contents: read; update the
workflow's permissions block to include contents: read alongside pull-requests:
write so github.rest.repos.getContent(...) can succeed before calling
github.rest.pulls.requestReviewers(...). Ensure the permissions YAML includes
the contents: read key at the same level as pull-requests.
7276653 to
7529d17
Compare
|
This is correct — testing the merge result instead of the fork branch in isolation is the right fix. Minor nit: since ref: ${{ github.event_name != 'pull_request' && 'main' || '' }}An empty string makes checkout fall back to its default ( |
The checkout step was explicitly checking out the PR head ref from the fork repository. This means PRs from stale forks fail if they depend on config changes in main, even though the merge result builds fine. Use refs/pull/N/merge instead, which is GitHub's pre-computed merge of the PR into the base branch. On push/dispatch, check out main directly. Fixes valkey-io#561 Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
madolson
force-pushed
the
fix-ci-merge-ref
branch
from
7529d17 to
05335bb
Compare
3 participants
The checkout step in
zola-deploy.ymlwas explicitly checking out the PR's head ref from the fork repository. This means PRs from stale forks would fail if they depended on any config changes in main, even though the actual merge result would build fine.This removes the explicit
ref/repositoryoverrides and letsactions/checkoutuse its default behavior, which checks outrefs/pull/N/merge(the merge commit) forpull_requestevents. This is the standard pattern for CI workflows.Fixes #561