Fixed CI workflows for use in private repo#26686
Merged
kevinansfield merged 1 commit intomainfrom Mar 4, 2026
Merged
Conversation
Contributor
WalkthroughReplaces fork-detection gating with a required 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/actions/load-docker-image/action.yml (1)
24-30: Decouple archive filename from artifact name for safer reuse.Line 29 assumes the archive file is exactly
${artifact-name}.tar.gz. That works for the current caller, but it makes this generalized action brittle if artifact naming changes.Proposed refactor
- name: Download image artifact (artifact) if: inputs.use-artifact == 'true' uses: actions/download-artifact@v4 with: name: ${{ inputs.artifact-name }} + path: _docker_image_artifact - name: Load image from artifact (artifact) if: inputs.use-artifact == 'true' shell: bash run: | echo "Loading Docker image from artifact..." - gunzip -c ${{ inputs.artifact-name }}.tar.gz | docker load + ARCHIVE="$(find _docker_image_artifact -maxdepth 1 -type f -name '*.tar.gz' | head -n1)" + if [ -z "$ARCHIVE" ]; then + echo "❌ No Docker image archive found in _docker_image_artifact" + exit 1 + fi + gunzip -c "$ARCHIVE" | docker load echo "Available images after load:" docker images🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/load-docker-image/action.yml around lines 24 - 30, The step "Load image from artifact (artifact)" currently hardcodes the archive as `${{ inputs.artifact-name }}.tar.gz`, which couples archive filename to artifact-name; add a new input (e.g., inputs.archive-filename) to action.yml and change the run to use that input when building the gunzip/docker load pipeline (fall back to a safe search or artifact-name + extension only if archive-filename is empty), and update any docs/README to document the new inputs; update references to inputs.artifact-name in the load step to use inputs.archive-filename so callers can supply arbitrary archive names without breaking reuse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 66-68: The workflow currently assigns BASE_COMMIT from the GitHub
API and can silently set BASE_COMMIT=null; modify the step that sets BASE_COMMIT
so it validates the value after assignment (check for empty string or literal
"null"), emit a clear error message including the raw API response or status
when validation fails, and exit non‑zero to fail the job early; update the
snippet around the BASE_COMMIT assignment and the subsequent echo
"BASE_COMMIT=..." logic to perform this check and abort if BASE_COMMIT is not a
valid SHA.
---
Nitpick comments:
In @.github/actions/load-docker-image/action.yml:
- Around line 24-30: The step "Load image from artifact (artifact)" currently
hardcodes the archive as `${{ inputs.artifact-name }}.tar.gz`, which couples
archive filename to artifact-name; add a new input (e.g.,
inputs.archive-filename) to action.yml and change the run to use that input when
building the gunzip/docker load pipeline (fall back to a safe search or
artifact-name + extension only if archive-filename is empty), and update any
docs/README to document the new inputs; update references to
inputs.artifact-name in the load step to use inputs.archive-filename so callers
can supply arbitrary archive names without breaking reuse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2e0385cd-b9d1-468d-8c4d-f48e540e4f9f
📥 Commits
Reviewing files that changed from the base of the PR and between d066dba and f28d814dc872d51c119cb10aa7b946bc14eb3f4e.
📒 Files selected for processing (3)
.github/actions/load-docker-image/action.yml.github/workflows/ci.yml.github/workflows/create-release-branch.yml
Comment on lines
+66
to
68
| BASE_COMMIT=$(curl --location --request GET 'https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}' --header 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' | jq -r .base.sha) | ||
| echo "Setting BASE_COMMIT to $BASE_COMMIT" | ||
| echo "BASE_COMMIT=$BASE_COMMIT" >> $GITHUB_ENV |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/ci.yml | sed -n '60,75p'Repository: TryGhost/Ghost
Length of output: 1066
🏁 Script executed:
grep -n "BASE_COMMIT" .github/workflows/ci.ymlRepository: TryGhost/Ghost
Length of output: 891
🏁 Script executed:
cat -n .github/workflows/ci.yml | sed -n '200,210p'Repository: TryGhost/Ghost
Length of output: 742
🏁 Script executed:
# Check what happens with jq when parsing null values
echo '{"base": {"sha": null}}' | jq -r .base.sha
echo "---"
echo '{"base": {}}' | jq -r .base.sha
echo "---"
echo '{}' | jq -r .base.shaRepository: TryGhost/Ghost
Length of output: 80
🏁 Script executed:
# Simulate API failure scenario
echo "Testing curl behavior with HTTP errors:"
echo "1. Without --fail flag (current code behavior):"
curl --silent --location 'https://httpbin.org/status/401' | head -c 100
echo -e "\n---"
echo "Exit code: $?"
echo -e "\n2. With --fail flag (proposed fix):"
curl --fail --silent 'https://httpbin.org/status/401' 2>&1 | head -c 100
echo -e "\n---"
echo "Exit code: $?"Repository: TryGhost/Ghost
Length of output: 218
🏁 Script executed:
# Check the nx affected command documentation to understand what happens with --base=null
# First, let me see what version of nx is used
cat -n .github/workflows/ci.yml | sed -n '1,30p'Repository: TryGhost/Ghost
Length of output: 1218
🏁 Script executed:
# Search for any validation or error handling around BASE_COMMIT usage
grep -B5 -A5 "BASE_COMMIT" .github/workflows/ci.yml | head -50Repository: TryGhost/Ghost
Length of output: 2306
Fail fast if PR metadata lookup cannot resolve BASE_COMMIT.
Line 66 can silently produce BASE_COMMIT=null on API/auth/rate-limit failures, and downstream yarn nx affected commands then fail with misleading errors instead of catching the root cause immediately.
Proposed hardening
- BASE_COMMIT=$(curl --location --request GET 'https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}' --header 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' | jq -r .base.sha)
+ API_URL="${{ github.api_url }}/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}"
+ BASE_COMMIT=$(curl --fail --silent --show-error --location "$API_URL" \
+ --header "Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}" \
+ --header "Accept: application/vnd.github+json" | jq -r '.base.sha')
+ if [ -z "$BASE_COMMIT" ] || [ "$BASE_COMMIT" = "null" ]; then
+ echo "::error::Unable to resolve BASE_COMMIT from PR metadata"
+ exit 1
+ fi
echo "Setting BASE_COMMIT to $BASE_COMMIT"
echo "BASE_COMMIT=$BASE_COMMIT" >> $GITHUB_ENV📝 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.
Suggested change
| BASE_COMMIT=$(curl --location --request GET 'https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}' --header 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' | jq -r .base.sha) | |
| echo "Setting BASE_COMMIT to $BASE_COMMIT" | |
| echo "BASE_COMMIT=$BASE_COMMIT" >> $GITHUB_ENV | |
| API_URL="${{ github.api_url }}/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}" | |
| BASE_COMMIT=$(curl --fail --silent --show-error --location "$API_URL" \ | |
| --header "Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}" \ | |
| --header "Accept: application/vnd.github+json" | jq -r '.base.sha') | |
| if [ -z "$BASE_COMMIT" ] || [ "$BASE_COMMIT" = "null" ]; then | |
| echo "::error::Unable to resolve BASE_COMMIT from PR metadata" | |
| exit 1 | |
| fi | |
| echo "Setting BASE_COMMIT to $BASE_COMMIT" | |
| echo "BASE_COMMIT=$BASE_COMMIT" >> $GITHUB_ENV |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 66 - 68, The workflow currently
assigns BASE_COMMIT from the GitHub API and can silently set BASE_COMMIT=null;
modify the step that sets BASE_COMMIT so it validates the value after assignment
(check for empty string or literal "null"), emit a clear error message including
the raw API response or status when validation fails, and exit non‑zero to fail
the job early; update the snippet around the BASE_COMMIT assignment and the
subsequent echo "BASE_COMMIT=..." logic to perform this check and abort if
BASE_COMMIT is not a valid SHA.
Member
🤖 Velo CI Failure AnalysisClassification: 🟠 SOFT FAIL
|
Member
🤖 Velo CI Failure AnalysisClassification: 🟠 SOFT FAIL
|
rob-ghost
approved these changes
Mar 4, 2026
- Added `contents: read` permission to job_setup so checkout works in private repos - Replaced hardcoded `TryGhost/Ghost` repo reference with `github.repository` in PR metadata API call - Added `github.repository == 'TryGhost/Ghost'` guard to deploy/publish jobs (canary, publish_packages, deploy_tinybird_staging, deploy_tinybird_production, deploy_admin, trigger_cd, create-release-branch) so they only run on the main repo - Consolidated Docker push strategy: non-main repos use artifact-based image transfer (like fork PRs) so E2E tests still work without GHCR access - Renamed `is-fork-pr`/`is-fork` to `use-artifact` to accurately reflect the broadened semantics
kevinansfield
force-pushed
the
fix-ci-for-private-repo
branch
from
f28d814 to
87de8ca
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
66-68:⚠️ Potential issue | 🟠 MajorFail fast when PR metadata lookup cannot resolve
BASE_COMMIT.Line 66 can still produce empty/
nullon API/auth/rate-limit failures, then downstreamnx affected --base=...fails later with a misleading root cause.Proposed hardening
- BASE_COMMIT=$(curl --location --request GET 'https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}' --header 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' | jq -r .base.sha) + API_URL="${{ github.api_url }}/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}" + BASE_COMMIT=$(curl --fail --silent --show-error --location "$API_URL" \ + --header "Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}" \ + --header "Accept: application/vnd.github+json" | jq -r '.base.sha') + if [ -z "$BASE_COMMIT" ] || [ "$BASE_COMMIT" = "null" ]; then + echo "::error::Unable to resolve BASE_COMMIT from PR metadata" + exit 1 + fi echo "Setting BASE_COMMIT to $BASE_COMMIT" echo "BASE_COMMIT=$BASE_COMMIT" >> $GITHUB_ENV#!/bin/bash # Read-only verification: inspect whether PR metadata step fails fast on API/JSON failures. sed -n '63,75p' .github/workflows/ci.yml rg -n --type=yaml -C2 'BASE_COMMIT=.*curl|jq -r \.base\.sha|--fail|github\.api_url|Unable to resolve BASE_COMMIT' .github/workflows/ci.yml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 66 - 68, The BASE_COMMIT assignment step can yield empty/null on API/auth/rate-limit failures; update the block that sets BASE_COMMIT (the line assigning BASE_COMMIT=$(curl ... | jq -r .base.sha) and the subsequent echo lines) to make the curl call fail fast (e.g. enable --fail/--show-error) and immediately validate the resolved $BASE_COMMIT value: if it is empty/null or "null", print a clear error (e.g. "Unable to resolve BASE_COMMIT from PR metadata") to stderr and exit non‑zero instead of exporting to GITHUB_ENV and continuing to run downstream commands like nx affected --base=..., so the workflow fails fast on API/JSON errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 66-68: The BASE_COMMIT assignment step can yield empty/null on
API/auth/rate-limit failures; update the block that sets BASE_COMMIT (the line
assigning BASE_COMMIT=$(curl ... | jq -r .base.sha) and the subsequent echo
lines) to make the curl call fail fast (e.g. enable --fail/--show-error) and
immediately validate the resolved $BASE_COMMIT value: if it is empty/null or
"null", print a clear error (e.g. "Unable to resolve BASE_COMMIT from PR
metadata") to stderr and exit non‑zero instead of exporting to GITHUB_ENV and
continuing to run downstream commands like nx affected --base=..., so the
workflow fails fast on API/JSON errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9c3d74e9-1a9f-49e1-b738-b73ac01c8bba
📥 Commits
Reviewing files that changed from the base of the PR and between f28d814dc872d51c119cb10aa7b946bc14eb3f4e and 87de8ca.
📒 Files selected for processing (3)
.github/actions/load-docker-image/action.yml.github/workflows/ci.yml.github/workflows/create-release-branch.yml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26686 +/- ##
==========================================
+ Coverage 73.19% 73.21% +0.02%
==========================================
Files 1530 1532 +2
Lines 120510 120518 +8
Branches 14574 14576 +2
==========================================
+ Hits 88209 88240 +31
+ Misses 31280 31272 -8
+ Partials 1021 1006 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
3 participants
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.
Summary
contents: readpermission tojob_setupso checkout works in private repos (the implicit public repo read access doesn't apply here)TryGhost/Ghostrepo reference withgithub.repositoryin the PR metadata API callgithub.repository == 'TryGhost/Ghost'guard to all deploy/publish jobs so they skip cleanly on other repos: canary, publish_packages, deploy_tinybird_staging, deploy_tinybird_production, deploy_admin, trigger_cd, and create-release-branchis-fork-pr/is-forktouse-artifactthroughout ci.yml and load-docker-image action to accurately reflect the broadened semantics