From e9f2241cb312fd989ad0cdb5ae89363108903366 Mon Sep 17 00:00:00 2001 From: Vikrant Puppala Date: Thu, 9 Jul 2026 16:04:00 +0000 Subject: [PATCH] ci: stop secondary fork-PR checks from failing (#831) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fork verification (#849) showed 3 checks still red on fork PRs — pre-existing (also red on the original #671), separate from the cache fix, all rooted in GitHub's fork restrictions: - skip-integration-tests-pr / skip-kernel-e2e-pr post synthetic-success check-runs, needing checks:write. On fork PRs GitHub forces GITHUB_TOKEN to read-only regardless of the declared permission, so checks.create 403s ('Resource not accessible by integration'). Swallow the 403 ONLY for forks (github.event.pull_request.head.repo.fork) so the poster doesn't show a spurious failure; the REAL required checks are posted by the merge_group run (base context, full perms) when a maintainer queues the PR — the fork's code is genuinely integration-tested there. Non-fork/other errors still fail loudly. - test-with-coverage runs the e2e suite against a live warehouse and needs the azure-prod secrets, which forks never get (Secret source: None) — so it could only fail on a fork. Skip it on fork PRs; the real coverage gate runs on the merge_group transient branch (secrets present), which is what protects main. Same-repo PRs and merge_group are unaffected by all three changes. Refs #831 Co-authored-by: Isaac Signed-off-by: Vikrant Puppala --- .github/workflows/code-coverage.yml | 9 +++++ .github/workflows/kernel-e2e.yml | 40 +++++++++++++------ .../workflows/trigger-integration-tests.yml | 40 +++++++++++++------ 3 files changed, 65 insertions(+), 24 deletions(-) diff --git a/.github/workflows/code-coverage.yml b/.github/workflows/code-coverage.yml index 91ccb3c40..14c682742 100644 --- a/.github/workflows/code-coverage.yml +++ b/.github/workflows/code-coverage.yml @@ -37,6 +37,15 @@ concurrency: jobs: test-with-coverage: + # This job runs the e2e suite against a live warehouse, so it needs the + # azure-prod secrets. FORK PRs never receive those secrets (Secret source: + # None), so on a fork the job can only fail. Skip it on fork PRs — the real + # coverage gate runs on the merge_group's transient branch (base context, + # secrets present), which is what actually protects main. Same-repo PRs and + # merge_group are unaffected. + if: >- + github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository runs-on: group: databricks-protected-runner-group labels: linux-ubuntu-latest diff --git a/.github/workflows/kernel-e2e.yml b/.github/workflows/kernel-e2e.yml index e5ceabb3c..3bc5d7579 100644 --- a/.github/workflows/kernel-e2e.yml +++ b/.github/workflows/kernel-e2e.yml @@ -106,19 +106,35 @@ jobs: with: github-token: ${{ github.token }} script: | - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: 'Kernel E2E', - head_sha: context.payload.pull_request.head.sha, - status: 'completed', - conclusion: 'success', - completed_at: new Date().toISOString(), - output: { - title: 'Skipped on PR — runs in merge queue', - summary: 'Kernel E2E is skipped on PRs and runs as a required gate in the merge queue. Add the `kernel-e2e` label to preview on this PR.' + // On FORK PRs GitHub forces GITHUB_TOKEN to read-only regardless of + // the declared `checks: write`, so checks.create 403s ("Resource + // not accessible by integration"). That's expected — a fork can't + // post check-runs on the base repo. Swallow the 403 for forks so + // this poster doesn't show a spurious failure; the real Kernel E2E + // required check is posted by the merge_group run (full perms) when + // a maintainer queues the PR. Any other error still fails loudly. + const isFork = context.payload.pull_request.head.repo.fork; + try { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: 'Kernel E2E', + head_sha: context.payload.pull_request.head.sha, + status: 'completed', + conclusion: 'success', + completed_at: new Date().toISOString(), + output: { + title: 'Skipped on PR — runs in merge queue', + summary: 'Kernel E2E is skipped on PRs and runs as a required gate in the merge queue. Add the `kernel-e2e` label to preview on this PR.' + } + }); + } catch (e) { + if (isFork && e.status === 403) { + core.notice('Fork PR: cannot post the Kernel E2E check-run (read-only token). It will be posted by the merge queue at merge time.'); + } else { + throw e; } - }); + } # ─────────────────────────────────────────────────────────────── # Detect whether kernel-relevant files changed. Used by both the diff --git a/.github/workflows/trigger-integration-tests.yml b/.github/workflows/trigger-integration-tests.yml index c36fe5afe..9954d1539 100644 --- a/.github/workflows/trigger-integration-tests.yml +++ b/.github/workflows/trigger-integration-tests.yml @@ -137,21 +137,37 @@ jobs: with: github-token: ${{ github.token }} script: | + // On FORK PRs GitHub forces GITHUB_TOKEN to read-only regardless of + // the declared `checks: write`, so checks.create 403s ("Resource + // not accessible by integration"). Expected — a fork can't post + // check-runs on the base repo. Swallow the 403 for forks so this + // poster doesn't show a spurious failure; the real Python Proxy + // Tests required checks are posted by the merge_group run (full + // perms) when a maintainer queues the PR. Other errors fail loudly. + const isFork = context.payload.pull_request.head.repo.fork; const MODES = ['thrift', 'kernel']; for (const mode of MODES) { - await github.rest.checks.create({ - owner: context.repo.owner, - repo: context.repo.repo, - name: `Python Proxy Tests / ${mode}`, - head_sha: context.payload.pull_request.head.sha, - status: 'completed', - conclusion: 'success', - completed_at: new Date().toISOString(), - output: { - title: 'Skipped on PR — runs in merge queue', - summary: `Python Proxy Tests (${mode}) are skipped on PRs and run as a required gate in the merge queue. Add the \`integration-test\` label to preview them on this PR.` + try { + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: `Python Proxy Tests / ${mode}`, + head_sha: context.payload.pull_request.head.sha, + status: 'completed', + conclusion: 'success', + completed_at: new Date().toISOString(), + output: { + title: 'Skipped on PR — runs in merge queue', + summary: `Python Proxy Tests (${mode}) are skipped on PRs and run as a required gate in the merge queue. Add the \`integration-test\` label to preview them on this PR.` + } + }); + } catch (e) { + if (isFork && e.status === 403) { + core.notice(`Fork PR: cannot post the Python Proxy Tests / ${mode} check-run (read-only token). It will be posted by the merge queue at merge time.`); + } else { + throw e; } - }); + } } # =============================================================================