Skip automatic docs previews for fork PRs and drop the setup-uv retry steps (#3445)

Author: maxisbey

.github/scripts/docs_preview.js

@@ -23,15 +23,24 @@ async function authorize({ github, context, core }) {
   let slashAttempt = false;
 
   if (context.eventName === 'pull_request_target') {
-    // Gate on the *sender* (whoever caused this run — on synchronize that
-    // is the pusher), not the PR author, so a non-admin pushing to an
-    // admin-opened branch does not get an automatic build.
-    const actor = context.payload.sender.login;
-    prNumber = String(context.payload.pull_request.number);
-    headSha = context.payload.pull_request.head.sha;
-    const perm = await permissionFor(actor);
-    authorized = perm.level === 'admin';
-    core.info(`pull_request_target by ${actor} (level=${perm.level}, role=${perm.role}) → authorized=${authorized}`);
+    const pr = context.payload.pull_request;
+    prNumber = String(pr.number);
+    headSha = pr.head.sha;
+    // No automatic preview for fork PRs: actions/checkout refuses to fetch a
+    // fork's head in a pull_request_target run. A maintainer can still request
+    // one with /preview-docs. (head.repo is null once the fork is deleted.)
+    const headRepo = pr.head.repo;
+    if (!headRepo || headRepo.id !== context.payload.repository.id) {
+      core.info(`PR #${prNumber} head is on ${headRepo ? headRepo.full_name : 'a deleted fork'}; fork PRs are previewed via /preview-docs only.`);
+    } else {
+      // Gate on the *sender* (whoever caused this run — on synchronize that
+      // is the pusher), not the PR author, so a non-admin pushing to an
+      // admin-opened branch does not get an automatic build.
+      const actor = context.payload.sender.login;
+      const perm = await permissionFor(actor);
+      authorized = perm.level === 'admin';
+      core.info(`pull_request_target by ${actor} (level=${perm.level}, role=${perm.role}) → authorized=${authorized}`);
+    }
   } else {
     // issue_comment: the job-level `if:` already guarantees this is a PR
     // comment starting with /preview-docs.

.github/scripts/docs_preview.test.js

@@ -12,6 +12,7 @@ const assert = require('node:assert/strict');
 const { authorize, comment } = require('./docs_preview.js');
 
 const REPO = { owner: 'modelcontextprotocol', repo: 'python-sdk' };
+const BASE_REPO = { id: 1, full_name: 'modelcontextprotocol/python-sdk' };
 const HEAD = 'e4dfda7baa127ab00ebcd1d5324560cbe3cdfe42';
 const MARKER = '<!-- docs-preview -->';
 
@@ -37,6 +38,26 @@ const authorizeScenarios = [
     event: pushed(7, 'writer'),
     expect: { authorized: 'false', pr_number: '7', head_sha: HEAD, slash_attempt: 'false' },
   },
+  {
+    // actions/checkout refuses a fork's head under pull_request_target, so
+    // the run stops here instead of failing in `build`; /preview-docs still works.
+    name: 'admin pushes to or reopens a fork PR → no automatic preview',
+    event: pushed(7, 'admin', { fork: 'someone/python-sdk' }),
+    expect: { authorized: 'false', pr_number: '7', head_sha: HEAD, slash_attempt: 'false' },
+    permissionLookups: 0,
+  },
+  {
+    name: 'fork PR whose fork has since been deleted → no automatic preview',
+    event: pushed(7, 'admin', { fork: null }),
+    expect: { authorized: 'false', pr_number: '7', head_sha: HEAD, slash_attempt: 'false' },
+    permissionLookups: 0,
+  },
+  {
+    name: 'maintainer comments /preview-docs on a fork PR → previewed like any other',
+    pr: { fork: 'someone/python-sdk' },
+    event: slash(7, 'maintainer'),
+    expect: { authorized: 'true', pr_number: '7', head_sha: HEAD, slash_attempt: 'true' },
+  },
   {
     name: 'maintainer comments /preview-docs on an open PR → preview of its current head',
     event: slash(7, 'maintainer'),
@@ -70,6 +91,7 @@ for (const s of authorizeScenarios) {
     const world = makeWorld({ pr: { number: 7, ...s.pr } });
     assert.deepEqual(await runAuthorize(world, s.event), s.expect);
     assert.equal(world.writes.length, 0);
+    if (s.permissionLookups !== undefined) assert.equal(world.permissionLookups, s.permissionLookups);
   });
 }
 
@@ -146,8 +168,10 @@ test('comment: a build or deploy that did not succeed is reported with the short
 
 // ── Harness ────────────────────────────────────────────────────────────────
 
-function pushed(number, sender) {
-  return { eventName: 'pull_request_target', actor: sender, payload: { action: 'synchronize', pull_request: { number, head: { sha: HEAD } }, sender: { login: sender } } };
+// `fork`: full name of the fork the head lives on; null for a deleted fork; omitted for a same-repo branch.
+function pushed(number, sender, { fork } = {}) {
+  const repo = fork === undefined ? BASE_REPO : fork === null ? null : { id: 2, full_name: fork };
+  return { eventName: 'pull_request_target', actor: sender, payload: { action: 'synchronize', repository: BASE_REPO, pull_request: { number, head: { sha: HEAD, repo } }, sender: { login: sender } } };
 }
 function slash(number, commenter) {
   return { eventName: 'issue_comment', actor: commenter, payload: { action: 'created', issue: { number, pull_request: {} }, comment: { body: '/preview-docs', user: { login: commenter } } } };
@@ -173,7 +197,7 @@ async function runComment(world, env, actor) {
 // ── A tiny in-memory GitHub ────────────────────────────────────────────────
 
 function makeWorld({ pr, comments = [] }) {
-  const world = { pr: { state: 'open', ...pr }, comments: [], writes: [], failPermissionLookup: false, nextCommentId: 100 };
+  const world = { pr: { state: 'open', ...pr }, comments: [], writes: [], failPermissionLookup: false, permissionLookups: 0, nextCommentId: 100 };
   for (const c of comments) world.comments.push({ id: world.nextCommentId++, ...c });
 
   const err = (status, message = 'fake error') => Object.assign(new Error(message), { status });
@@ -183,14 +207,19 @@ function makeWorld({ pr, comments = [] }) {
   const rest = {
     repos: {
       getCollaboratorPermissionLevel: async ({ username }) => {
+        world.permissionLookups++;
         if (world.failPermissionLookup) throw err(500, 'boom');
         const person = PEOPLE[username];
         if (!person) throw err(404, 'not a user');
         return { data: { ...person, user: { login: username } } };
       },
     },
     pulls: {
-      get: async ({ pull_number }) => { checkPr(pull_number); return { data: { number: pull_number, state: world.pr.state, head: { sha: HEAD } } }; },
+      get: async ({ pull_number }) => {
+        checkPr(pull_number);
+        const repo = world.pr.fork ? { id: 2, full_name: world.pr.fork } : BASE_REPO;
+        return { data: { number: pull_number, state: world.pr.state, head: { sha: HEAD, repo } } };
+      },
     },
     issues: {
       listComments: async ({ issue_number }) => { checkPr(issue_number); return { data: world.comments.map((c) => ({ id: c.id, body: c.body, user: { login: c.user } })) }; },

.github/workflows/docs-preview.yml

@@ -4,12 +4,16 @@ name: Docs Preview
 #
 # Security: the build executes Python from the PR (mkdocstrings imports
 # src/mcp, `!!python/name:` config directives run, and heads may ship their
-# own build scripts). The build is gated by `authorize` (admin sender for
-# auto-preview, admin/maintainer commenter for /preview-docs) and isolated
-# from Cloudflare secrets — `build` runs PR code with no secrets and hands
-# the static site to `deploy` via an artifact, so PR code never shares a
-# runner with the Cloudflare token. `authorize` and `comment` run
-# .github/scripts/docs_preview.js, checked out from the default branch only.
+# own build scripts). The build is gated by `authorize` (admin sender on a
+# same-repo branch for auto-preview, admin/maintainer commenter for
+# /preview-docs) and isolated from Cloudflare secrets — `build` runs PR code
+# with no secrets and hands the static site to `deploy` via an artifact, so
+# PR code never shares a runner with the Cloudflare token. Fork PRs get no
+# automatic preview: actions/checkout refuses to fetch a fork's head in a
+# pull_request_target run, so a maintainer requests one with /preview-docs,
+# which runs under issue_comment with the same gating and isolation.
+# `authorize` and `comment` run .github/scripts/docs_preview.js, checked out
+# from the default branch only; those two checkouts must never take a `ref:`.
 #
 # Required configuration:
 #   - secrets.CLOUDFLARE_API_TOKEN   (scope: Account → Cloudflare Pages → Edit)
@@ -62,6 +66,7 @@ jobs:
       - name: Check out the scripts (default branch)
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
+          # No `ref:` here, ever: this job trusts what it checks out (see header).
           persist-credentials: false
           sparse-checkout: .github/scripts
 
@@ -88,19 +93,20 @@ jobs:
       - name: Install uv
         uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
         with:
-          # pull_request_target runs share the base-branch Actions cache; saving
-          # a cache populated while untrusted PR code ran would let it poison
-          # later trusted workflows. Mirrors publish-pypi.yml.
+          # Keep the untrusted build away from the shared Actions cache. GitHub
+          # already limits pull_request_target and issue_comment runs to
+          # read-only access in the default branch's cache scope, so this is
+          # defence in depth (and avoids a refused save in the post step).
+          # Mirrors publish-pypi.yml.
           enable-cache: false
           version: 0.9.5
 
-      # pull_request_target runs this workflow file from the base branch, so
-      # the whole recipe — dependency sync included — must come from the
-      # checkout itself: heads that ship scripts/docs/build.sh (the Zensical
-      # toolchain) build with it; older heads, and v1.x heads previewed via
-      # /preview-docs, still build with MkDocs. Both arms must write the site
-      # to site/. Keep the detection in sync with build_site() in
-      # scripts/build-docs.sh.
+      # Both triggers run this workflow file from the default branch (whatever
+      # the PR targets), so the whole recipe — dependency sync included — must
+      # come from the checkout itself: heads that ship scripts/docs/build.sh
+      # (the Zensical toolchain) build with it; older heads and v1.x heads still
+      # build with MkDocs. Both arms must write the site to site/. Keep the
+      # detection in sync with build_site() in scripts/build-docs.sh.
       - run: |
           if [ -f scripts/docs/build.sh ]; then
             bash scripts/docs/build.sh
@@ -161,6 +167,7 @@ jobs:
       - name: Check out the scripts (default branch)
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
+          # No `ref:` here, ever: this job trusts what it checks out (see header).
           persist-credentials: false
           sparse-checkout: .github/scripts
 

.github/workflows/shared.yml

@@ -17,18 +17,7 @@ jobs:
         with:
           persist-credentials: false
 
-      # setup-uv's manifest fetch is a single request with a hard 5s timeout
-      # (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
       - name: Install uv
-        id: setup-uv
-        continue-on-error: true
-        uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
-        with:
-          enable-cache: true
-          version: 0.9.5
-
-      - name: Install uv (retry)
-        if: steps.setup-uv.outcome == 'failure'
         uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
         with:
           enable-cache: true
@@ -79,18 +68,7 @@ jobs:
         with:
           persist-credentials: false
 
-      # setup-uv's manifest fetch is a single request with a hard 5s timeout
-      # (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
       - name: Install uv
-        id: setup-uv
-        continue-on-error: true
-        uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
-        with:
-          enable-cache: true
-          version: 0.9.5
-
-      - name: Install uv (retry)
-        if: steps.setup-uv.outcome == 'failure'
         uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
         with:
           enable-cache: true
@@ -125,18 +103,7 @@ jobs:
         with:
           persist-credentials: false
 
-      # setup-uv's manifest fetch is a single request with a hard 5s timeout
-      # (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
       - name: Install uv
-        id: setup-uv
-        continue-on-error: true
-        uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
-        with:
-          enable-cache: true
-          version: 0.9.5
-
-      - name: Install uv (retry)
-        if: steps.setup-uv.outcome == 'failure'
         uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
         with:
           enable-cache: true
@@ -165,18 +132,7 @@ jobs:
         with:
           persist-credentials: false
 
-      # setup-uv's manifest fetch is a single request with a hard 5s timeout
-      # (astral-sh/setup-uv#869); retry once. Drop when upstream adds a retry.
       - name: Install uv
-        id: setup-uv
-        continue-on-error: true
-        uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
-        with:
-          enable-cache: true
-          version: 0.9.5
-
-      - name: Install uv (retry)
-        if: steps.setup-uv.outcome == 'failure'
         uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d # v10.0.1
         with:
           enable-cache: true