Skip to content

ci: skip docs preview deploy for fork PRs - #2029

Merged
kevalmorabia97 merged 1 commit into
mainfrom
haoguo/fix-docs-preview-fork-prs
Jul 29, 2026
Merged

ci: skip docs preview deploy for fork PRs#2029
kevalmorabia97 merged 1 commit into
mainfrom
haoguo/fix-docs-preview-fork-prs

Conversation

@h-guo18

@h-guo18 h-guo18 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Type of change: Bug fix (CI)

The deploy-preview job in the Docs workflow fails on every pull request opened from a fork, which blocks merging for all external contributors.

Root cause. deploy-preview runs rossjrw/pr-preview-action@v1, which pushes the built HTML to the gh-pages branch. The workflow declares permissions: contents: write, but for a pull_request event originating from a forked repository GitHub caps the GITHUB_TOKEN at read-only — the permissions: block cannot elevate above that cap. The push is therefore rejected:

remote: Permission to NVIDIA/Model-Optimizer.git denied to github-actions[bot].
fatal: unable to access 'https://github.com/NVIDIA/Model-Optimizer.git/': The requested URL returned error: 403

The job's if: condition gated on github.event_name, github.event.action and the changes path filter, but never on whether the PR came from a fork — so it always ran and always failed.

Because the changes filter matches docs/**, modelopt/** and .github/workflows/pages.yml, essentially any substantive fork PR trips this.

Fix. Restrict deploy-preview to PRs whose head branch lives in this repository:

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

A skipped job is not a failed job, so fork PRs are no longer blocked by it.

Usage

N/A — CI-only change.

Testing

Behaviour by scenario:

Scenario Before After
PR from a branch in this repo preview deployed preview deployed (unchanged)
PR from a fork ❌ fails with 403 ⏭️ skipped
Fork deleted (head.repo is null) ❌ fails ⏭️ skipped
  • Confirmed against workflow history: recent Docs runs on in-repo branches (main, chenjiel/nvfp4-act-headroom, mxin/qad-skill, haoguo/dspark-ptq-script) all succeed, while fork-branch runs fail with the 403 above.
  • build-docs was already passing on the affected PRs — only the deploy step failed, so documentation builds are unaffected either way.
  • YAML parses; pre-commit run --files .github/workflows/pages.yml passes. (yamlfmt excludes ^.github/workflows/, so this file is not auto-formatted.)
  • This PR edits .github/workflows/pages.yml, which is itself in the changes filter, so it exercises deploy-preview on the in-repo path — the preview deploy on this PR passing is a self-check that the unchanged path still works.

The closed cleanup path is gated by the same condition. That is intentional: a fork PR never deployed a preview directory, so there is nothing to remove.

Before your PR is "Ready for review"

  • Is this change backward compatible?: ✅
  • If you copied code from any other sources or added a new PIP dependency, did you follow guidance in CONTRIBUTING.md: N/A
  • Did you write any new necessary tests?: N/A — workflow-condition change; not unit-testable
  • Did you update Changelog?: N/A — CI infrastructure, not user-facing
  • Did you get Claude approval on this PR?: ❌ — not yet run

Additional Information

Currently blocking #1975 (add Qwen3-VL support for DFlash training), which is approved with every other check green and sits at mergeStateStatus: BLOCKED solely because of this job.

Note that a PR only picks up this fix once its branch contains it, since workflows run from the PR branch's own definitions.

A follow-up option, if doc previews for external contributors are wanted: build in the pull_request workflow and deploy from a separate workflow_run-triggered workflow, which executes in the base-repo context and does get a write token. Deliberately not using pull_request_target here — that would run unreviewed PR code with write permissions.

Summary by CodeRabbit

  • Bug Fixes
    • Preview deployments no longer run for pull requests from forked repositories, preventing failed deployments caused by permission restrictions.

Signed-off-by: h-guo18 <67671475+h-guo18@users.noreply.github.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c6ff0afb-3274-4738-bdd0-33493e8bcb9c

📥 Commits

Reviewing files that changed from the base of the PR and between a3ac475 and 25c1d98.

📒 Files selected for processing (1)
  • .github/workflows/pages.yml

📝 Walkthrough

Walkthrough

The preview deployment workflow now skips fork-originated pull requests by requiring the pull request source repository to match the current repository, avoiding blocked preview pushes.

Changes

Preview deployment workflow

Layer / File(s) Summary
Prevent forked preview deployments
.github/workflows/pages.yml
The deploy-preview job now checks that head.repo.full_name matches github.repository, with comments documenting the read-only token behavior for forked pull requests.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: aanoosheh, achidiac-nv, ajrasane

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch haoguo/fix-docs-preview-fork-prs

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-29 09:32 UTC

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.83%. Comparing base (a3ac475) to head (25c1d98).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2029   +/-   ##
=======================================
  Coverage   66.83%   66.83%           
=======================================
  Files         519      519           
  Lines       58916    58916           
=======================================
  Hits        39376    39376           
  Misses      19540    19540           
Flag Coverage Δ
unit 54.89% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kevalmorabia97
kevalmorabia97 marked this pull request as ready for review July 29, 2026 09:31
@kevalmorabia97
kevalmorabia97 requested a review from a team as a code owner July 29, 2026 09:31
@kevalmorabia97
kevalmorabia97 enabled auto-merge (squash) July 29, 2026 09:31
@kevalmorabia97
kevalmorabia97 merged commit c2070cf into main Jul 29, 2026
34 of 35 checks passed
@kevalmorabia97
kevalmorabia97 deleted the haoguo/fix-docs-preview-fork-prs branch July 29, 2026 09:32
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.

2 participants