Skip to content

ci(preview-readme): skip fork PRs to fix checkout failure - #30

Merged
CRThaze merged 1 commit into
mainfrom
CRThaze/fix-preview-readme-skip-fork-prs
Jul 27, 2026
Merged

ci(preview-readme): skip fork PRs to fix checkout failure#30
CRThaze merged 1 commit into
mainfrom
CRThaze/fix-preview-readme-skip-fork-prs

Conversation

@CRThaze

@CRThaze CRThaze commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Problem

The preview-readme check failed on #28 (a fork PR from gaul/s3proxy-chart). The actions/checkout step failed with:

git -c protocol.version=2 fetch ... origin +refs/heads/healthz-probes*:...
The process '/usr/bin/git' failed with exit code 1   (retried 3x, then errored)

Root cause: the job checks out the head branch by name but does not set repository:

- uses: actions/checkout@v4
  with:
    ref: ${{ github.event.pull_request.head.ref }}

repository therefore defaults to this repo (comet-ml/s3proxy-chart), so checkout tries to fetch refs/heads/<branch> from here. On a fork PR the branch lives only in the fork, so the fetch finds no ref and fails. Every other workflow uses the default merge-ref checkout (refs/pull/N/merge, which GitHub materializes in the base repo for forks too), so they were unaffected.

There is also a second, latent limitation: even with checkout fixed, the comment-posting steps call issues.createComment with GITHUB_TOKEN, which the pull_request event grants fork PRs as read-only, so the preview comment cannot be posted from a fork PR anyway.

Fix

Guard the job on head/base repo identity:

if: github.event.pull_request.head.repo.full_name == github.repository

Same-repo (internal) branches still get the README preview comment; fork PRs skip the job cleanly instead of reporting a spurious red check. This is the minimal, no-security-tradeoff option. Full fork support would need a workflow_run two-stage pattern (render on pull_request with a read-only token + artifact upload; comment on workflow_run with a write token), which can follow separately if we want previews on external contributions.

Validation

  • python3 -c "import yaml; yaml.safe_load(...)" on the workflow: OK.
  • The change is additive (one if: plus an explanatory comment); no step logic changed.

The preview-readme job checks out the head branch by name
(ref: github.event.pull_request.head.ref) without setting repository, so
actions/checkout fetches refs/heads/<branch> from this repo. For a fork PR
the branch lives only in the fork, so the fetch finds no ref and the step
fails with 'git failed with exit code 1' (seen on PR #28, from
gaul/s3proxy-chart). Every other workflow uses the default merge-ref
checkout and is unaffected.

Even with checkout fixed, the comment-posting steps need a write-scoped
GITHUB_TOKEN, which the pull_request event does not grant fork PRs, so the
preview feature is inherently limited to same-repo branches.

Guard the job on head.repo.full_name == github.repository: internal
branches still get the README preview; fork PRs skip it cleanly instead of
reporting a spurious failure.
@github-actions

Copy link
Copy Markdown

📊 Helm Render Diff Summary

Chart charts/s3proxy rendered with Kubernetes 1.29.0. Informational only — this check never fails the PR.

Values File Chart Path Changes Status
test-values/autoscaling.yaml charts/s3proxy - ✅ No Changes
test-values/azureblob.yaml charts/s3proxy - ✅ No Changes
test-values/b2.yaml charts/s3proxy - ✅ No Changes
test-values/filesystem.yaml charts/s3proxy - ✅ No Changes
test-values/gcs.yaml charts/s3proxy - ✅ No Changes
test-values/ingress.yaml charts/s3proxy - ✅ No Changes
test-values/multi-backend.yaml charts/s3proxy - ✅ No Changes
test-values/openstack-swift.yaml charts/s3proxy - ✅ No Changes
test-values/rackspace.yaml charts/s3proxy - ✅ No Changes
test-values/s3.yaml charts/s3proxy - ✅ No Changes
test-values/tls-existing-secret.yaml charts/s3proxy - ✅ No Changes
test-values/tls.yaml charts/s3proxy - ✅ No Changes
test-values/transient.yaml charts/s3proxy - ✅ No Changes

🎉 No changes detected in any of the tested configurations!

@CRThaze
CRThaze merged commit 5a269ef into main Jul 27, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant