Skip to content

Commit 2553c55

Browse files
ci: post PR-open skip check inline for internal PRs (fix waiting-for-status hang)
Follow-up to the previous commit. Removing the inline PR-open poster in favour of the workflow_run reporter created a bootstrap gap: workflow_run workflows only run from the DEFAULT-branch copy, so on this PR (and any introducing PR) the reporter can't fire and NOTHING posts 'Python Integration Tests' on the PR head — the required check sits 'Expected — Waiting for status to be reported' forever. Fix: split the placeholder by PR origin, both posting as the driver-test app (the identity the ruleset pins the required check to): - Internal PRs: restore skip-integration-tests-pr in trigger-integration-tests.yml, now minting the INTEGRATION_TEST_APP token (not github.token) and self-guarded to head repo == base repo. Posts from the PR branch immediately — no label, no workflow_run dependency. This is what unblocks internal PRs like this one. - Fork PRs: skip-checks-reporter.yml (workflow_run) now self-guards to fork runs (head repo != base repo), since a fork's pull_request run can't mint secrets inline. The two guards are mutually exclusive — no double-post. Behaviour matches the intent: skipped placeholder on PR open, real run in the merge queue (and label preview). The merge-queue app-token posts from the prior commit are unchanged. Co-authored-by: Isaac
1 parent 28dfd58 commit 2553c55

2 files changed

Lines changed: 88 additions & 49 deletions

File tree

.github/workflows/skip-checks-reporter.yml

Lines changed: 33 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,24 @@
11
name: Report Integration Test Skip
22

33
# Posts the PR-open "skipped" placeholder for the required
4-
# `Python Integration Tests` check, as the driver-test GitHub App.
4+
# `Python Integration Tests` check on FORK PRs, as the driver-test GitHub App.
55
#
6-
# Why a separate workflow (and not an inline job in trigger-integration-tests.yml):
7-
# - The branch ruleset pins the required `Python Integration Tests` check to the
8-
# driver-test app's integration id (driver-integration-test). Only a check
9-
# posted BY that app satisfies the gate — a `github.token` (github-actions)
10-
# check of the same name does NOT (it is a different check context).
11-
# - A PR from a fork runs its `pull_request` workflows with a READ-ONLY GITHUB_TOKEN
12-
# and no access to secrets, so it cannot mint the app token and cannot post any
13-
# check on its own head. That would leave fork PRs unable to enter the merge queue
14-
# without a maintainer label.
15-
# - `workflow_run` workflows always execute in THIS (base) repo's context using the
16-
# workflow definition from the default branch, with full secret access — even when
17-
# the run that triggered them came from a fork. That lets us post the app-attributed
18-
# placeholder on any PR head, fork or not, so every PR can auto-enqueue.
6+
# Division of labour:
7+
# - INTERNAL PRs: the placeholder is posted inline by trigger-integration-tests.yml's
8+
# `skip-integration-tests-pr` job (it can mint the app token directly). Posting it
9+
# there means it works from the PR branch immediately, without waiting for this
10+
# workflow_run file to reach the default branch.
11+
# - FORK PRs: a fork's `pull_request` run has a READ-ONLY GITHUB_TOKEN and no secrets,
12+
# so it cannot mint the app token or post any check on its own head. This
13+
# workflow_run workflow instead runs in THIS (base) repo's context with full secret
14+
# access even for fork-triggered runs, so it can post the app-attributed placeholder
15+
# on a fork PR's head. This job self-guards to fork runs to avoid double-posting on
16+
# internal PRs (which the inline job already covers).
17+
#
18+
# Why the app (not github.token): the ruleset pins the required
19+
# `Python Integration Tests` check to the driver-test app's integration id. Only a
20+
# check posted BY that app satisfies the gate — a github.token (github-actions) check
21+
# of the same name is a different context and does NOT.
1922
#
2023
# SECURITY: this workflow runs with secrets in a privileged context. It MUST NOT check
2124
# out or execute any PR/fork-controlled content. It only calls checks.create with a
@@ -25,8 +28,7 @@ name: Report Integration Test Skip
2528
#
2629
# The real integration suite is unaffected: it runs as the required gate on the
2730
# `merge_group` commit (and as a label preview on internal PRs), dispatched by
28-
# trigger-integration-tests.yml. This workflow only posts the pre-merge placeholder.
29-
# Mirrors databricks-sql-go / databricks-sql-nodejs.
31+
# trigger-integration-tests.yml. Mirrors databricks-sql-go / databricks-sql-nodejs.
3032

3133
on:
3234
workflow_run:
@@ -35,8 +37,11 @@ on:
3537

3638
jobs:
3739
report-skip:
38-
# Only for PR-triggered runs; the merge_group run posts the real required check.
39-
if: github.event.workflow_run.event == 'pull_request'
40+
# Fork PR-triggered runs only. Internal PRs are posted inline by
41+
# trigger-integration-tests.yml; the merge_group run posts the real required check.
42+
if: >-
43+
github.event.workflow_run.event == 'pull_request' &&
44+
github.event.workflow_run.head_repository.full_name != github.event.workflow_run.repository.full_name
4045
runs-on:
4146
group: databricks-protected-runner-group
4247
labels: linux-ubuntu-latest
@@ -45,22 +50,14 @@ jobs:
4550
contents: read
4651
pull-requests: read
4752
steps:
48-
# The reporter triggers on EVERY pull_request run of "Trigger Integration
49-
# Tests" (opened/synchronize/reopened/labeled/closed), but the
50-
# workflow_run payload does NOT expose the originating sub-action, so we
51-
# cannot filter on it directly. Recover the needed context from the PR
52-
# and post the synthetic success only for the opened/synchronize/reopened
53-
# scope the old inline job covered:
54-
# - skip when the PR is closed (outside the old job's scope), and
55-
# - skip when a real label preview is running for this head (an internal
56-
# PR carrying the integration-test label): trigger-tests-pr dispatches
57-
# the real suite and driver-test posts the real check for the same
58-
# app + name + head_sha, so a premature success placeholder would race
59-
# that check and could mask a real preview failure.
60-
# Fork PRs KEEP the placeholder even when labeled: their label preview
61-
# cannot dispatch (no secret access), so nothing else posts the required
62-
# check for them. This is a read-only lookup (no checkout / no execution
63-
# of PR-controlled content) and does not weaken the SECURITY note above.
53+
# This job is fork-only (see the job `if:`), so the placeholder is always
54+
# appropriate EXCEPT when the fork PR is already closed. A fork PR's label
55+
# preview cannot dispatch the real suite (no secret access), so nothing
56+
# else posts the required check for it — keep the placeholder even when
57+
# labeled. Skip only closed PRs. Resolve the PR by SHA because a fork's
58+
# workflow_run payload has an empty pull_requests array. Read-only lookup
59+
# (no checkout / no execution of PR content) — does not weaken the SECURITY
60+
# note above.
6461
- name: Decide whether to post the placeholder
6562
id: gate
6663
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0
@@ -90,12 +87,9 @@ jobs:
9087
repo: context.repo.repo,
9188
pull_number: number,
9289
});
93-
const isFork = pr.head.repo.full_name !== `${context.repo.owner}/${context.repo.repo}`;
94-
const hasLabel = pr.labels.some(l => l.name === 'integration-test');
9590
const isClosed = pr.state === 'closed';
96-
const skip = isClosed || (hasLabel && !isFork);
97-
console.log(`PR #${number} fork=${isFork} label=${hasLabel} closed=${isClosed} -> post=${!skip}`);
98-
core.setOutput('post', (!skip).toString());
91+
console.log(`PR #${number} closed=${isClosed} -> post=${!isClosed}`);
92+
core.setOutput('post', (!isClosed).toString());
9993
10094
- name: Generate GitHub App token (this repo)
10195
id: app-token

.github/workflows/trigger-integration-tests.yml

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,61 @@ jobs:
116116
body: body
117117
});
118118
119-
# NOTE: the PR-open "skipped" placeholder for the required
120-
# `Python Integration Tests` check is NOT posted here. It is posted by the
121-
# companion workflow `skip-checks-reporter.yml`, which runs on `workflow_run`
122-
# in the base-repo context so it can post the check as the driver-test app —
123-
# the app the branch ruleset PINS the required check to — on EVERY PR head,
124-
# including fork PRs (whose own `pull_request` run gets a read-only token and
125-
# cannot post checks at all). A `pull_request`-triggered `github.token` stub
126-
# here posted the check as `github-actions`, which is a DIFFERENT check
127-
# context and did NOT satisfy the app-pinned gate — so the required check sat
128-
# unfulfilled and PRs stayed blocked. See skip-checks-reporter.yml.
119+
# =============================================================================
120+
# For internal PRs: post the "skipped" placeholder for the required
121+
# `Python Integration Tests` check on non-label events. The real run happens in
122+
# the merge queue (or via explicit label preview).
123+
#
124+
# CRITICAL: post as the driver-test APP, not github.token. The ruleset pins the
125+
# required check to that app's integration id; a github.token (github-actions)
126+
# check of the same name is a DIFFERENT context and does NOT satisfy the gate,
127+
# so the required check would sit "waiting for status" forever.
128+
#
129+
# Fork PRs are handled by the companion `skip-checks-reporter.yml` (workflow_run):
130+
# a fork's `pull_request` run has a read-only token and no secrets, so it can
131+
# neither mint the app token nor post any check here. This job self-guards to
132+
# internal PRs (head repo == base repo); the reporter covers forks from the
133+
# base-repo context. Internal PRs are posted HERE (not via the reporter) so the
134+
# placeholder appears without depending on the workflow_run copy being on the
135+
# default branch.
136+
# =============================================================================
137+
skip-integration-tests-pr:
138+
if: |
139+
github.event_name == 'pull_request' &&
140+
github.event.action != 'labeled' &&
141+
github.event.action != 'closed' &&
142+
github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name
143+
runs-on:
144+
group: databricks-protected-runner-group
145+
labels: linux-ubuntu-latest
146+
steps:
147+
- name: Generate GitHub App token (this repo)
148+
id: app-token
149+
uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v3.0.0
150+
with:
151+
app-id: ${{ secrets.INTEGRATION_TEST_APP_ID }}
152+
private-key: ${{ secrets.INTEGRATION_TEST_PRIVATE_KEY }}
153+
owner: databricks
154+
repositories: databricks-sql-python
155+
156+
- name: Skip Python Integration Tests
157+
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b # v7.1.0
158+
with:
159+
github-token: ${{ steps.app-token.outputs.token }}
160+
script: |
161+
await github.rest.checks.create({
162+
owner: context.repo.owner,
163+
repo: context.repo.repo,
164+
name: 'Python Integration Tests',
165+
head_sha: context.payload.pull_request.head.sha,
166+
status: 'completed',
167+
conclusion: 'success',
168+
completed_at: new Date().toISOString(),
169+
output: {
170+
title: 'Skipped on PR — runs in merge queue',
171+
summary: 'Python Integration Tests are skipped on PRs and run as the required gate in the merge queue. Add the `integration-test` label to preview them on this PR.'
172+
}
173+
});
129174
130175
# =============================================================================
131176
# For PRs: Dispatch real tests when integration-test label is added.

0 commit comments

Comments
 (0)