ci: run PR builds in fork and mirror status to apache repo #4976
ci: run PR builds in fork and mirror status to apache repo #4976Ma77Ball wants to merge 3 commits intoapache:mainfrom
Conversation
|
@Yicong-Huang, @aglinxinyuan, and @chenlica, please review. |
There was a problem hiding this comment.
Pull request overview
This PR restructures CI so that pull request builds run in contributors’ forks (avoiding first-time-contributor approval and conserving apache/texera Actions minutes) while mirroring results back to the PR via a single required Build status.
Changes:
- Add a fork-owned CI workflow (
fork-ci.yml) plus PR-side workflows to create and continuously update aBuildcommit status that links to the fork run. - Stop running duplicate PR CI in the Apache repo by removing
pull_requesttriggers from existing workflows. - Adjust post-merge automation and ASF branch protection to key off the mirrored
Buildstatus.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/fork-ci.yml | Runs the full build matrix + header check in contributor forks (no-op in apache/texera). |
| .github/workflows/notify_test_workflow.yml | On PR updates, posts/updates a Build commit status and PR comments pointing to the fork CI run (or explaining failure). |
| .github/workflows/update_build_status.yml | Periodically/after notify completion, syncs the Build status to match fork run progress. |
| .github/workflows/required-checks.yml | Removes PR triggering and ensures the base repo build matrix runs only for non-PR events. |
| .github/workflows/check-header.yml | Stops running license-header checks on PR events in the base repo. |
| .github/workflows/automatic-email-notif-on-ddl-change.yml | Moves DDL email notification to a push: main trigger scoped to sql/updates/**. |
| .asf.yaml | Updates required status checks to Build and Validate PR title. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -23,13 +23,6 @@ on: | |||
| - 'ci-enable/**' | |||
| - 'main' | |||
| - 'release/**' | |||
| // Failure scenarios: red ✗ Build status, with target_url pointing | ||
| // somewhere useful for the committer to dig in. The detailed | ||
| // explanation lives in the postOnceComment on the PR conversation. | ||
| async function failCheck(title, target_url) { | ||
| await setBuildStatus('failure', title, target_url || notifyRunUrl) | ||
| core.setFailed(title) | ||
| } | ||
|
|
||
| // Success scenario (non-fork PR): leaves the workflow green. | ||
| async function passCheck(title, target_url) { | ||
| await setBuildStatus('success', title, target_url || notifyRunUrl) | ||
| } |
| on: | ||
| workflow_run: | ||
| workflows: ["On pull request update"] | ||
| types: [completed] | ||
| schedule: | ||
| - cron: "*/5 * * * *" | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| update: | ||
| name: Update build status | ||
| runs-on: ubuntu-latest | ||
| # workflow_run mode polls up to 60 min waiting for fork CI completion. | ||
| # Other modes (cron, workflow_dispatch) finish in seconds. | ||
| timeout-minutes: 65 | ||
| permissions: | ||
| actions: read | ||
| statuses: write | ||
| steps: | ||
| - name: "Update build status" | ||
| uses: actions/github-script@v8 | ||
| env: | ||
| TRIGGER_EVENT: ${{ github.event_name }} | ||
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| const isLongPoll = process.env.TRIGGER_EVENT === 'workflow_run' | ||
| const maxIterations = isLongPoll ? 120 : 1 // 120 * 30s = 60 min | ||
| const sleepMs = 30000 | ||
| const statusContext = 'Build' | ||
|
|
||
| console.log('=== update_build_status: ' + new Date().toISOString() + ' ===') | ||
| console.log('trigger=' + process.env.TRIGGER_EVENT + ' mode=' + (isLongPoll ? 'long-poll up to ' + maxIterations + ' iters' : 'single pass')) | ||
|
|
| - name: Find added SQL update file | ||
| if: steps.pr.outputs.skip == 'false' | ||
| id: get_sql_file | ||
| run: | | ||
| FILE=$(git diff --name-only --diff-filter=A \ | ||
| ${{ github.event.pull_request.base.sha }} \ | ||
| ${{ github.event.pull_request.merge_commit_sha }} \ | ||
| -- 'sql/updates/') | ||
| FILE=$(git diff --name-only --diff-filter=A HEAD~1 HEAD -- 'sql/updates/') | ||
| echo "sql_file=$FILE" >> $GITHUB_OUTPUT | ||
|
|
| await postOnceComment( | ||
| '<!-- fork-ci-not-applicable -->', | ||
| ':information_source: **Fork CI is not applicable to this PR.**\n' + | ||
| '\n' + | ||
| 'This PR is opened from a branch inside `' + baseRepo + '` itself, not from a fork. ' + | ||
| 'Fork CI is the system that runs the full build matrix in *contributor forks* and surfaces the result on PRs to `apache/texera`. ' + | ||
| 'Since there\'s no fork involved here, the `Build` status has been auto-passed.\n' + | ||
| '\n' + | ||
| 'In-tree branches like this one are gated by the **Required Checks** status check (which runs builds directly in `' + baseRepo + '`).\n' | ||
| ) | ||
| await passCheck( | ||
| 'Fork CI not applicable (in-tree PR)', | ||
| context.serverUrl + '/' + baseRepo + '/actions/workflows/required-checks.yml' | ||
| ) |
| # Other modes (cron, workflow_dispatch) finish in seconds. | ||
| timeout-minutes: 65 | ||
| permissions: | ||
| actions: read |
aglinxinyuan
left a comment
aglinxinyuan
left a comment
There was a problem hiding this comment.
Two findings I think aren't already covered by Copilot's review — see inline. The bigger items Copilot caught (backport regression, in-tree PR auto-pass with no CI, missing pull-requests: read, failCheck marking notify run as failed, DDL diff edge cases) are all worth addressing too.
| // points at the PR or notify run — never at the fork run. | ||
| if (current && current.state === 'failure') { | ||
| const isForkTarget = current.target_url && current.target_url.includes('/' + pr.head.repo.full_name + '/actions/runs/') | ||
| if (!isForkTarget) continue |
There was a problem hiding this comment.
Sticky-failure bug: this skip rule prevents recovery from notify's "detection failed" path.
In notify_test_workflow.yml line 337-338, the "Fork CI run not detected" path sets:
const forkActionsUrl = 'https://github.com/' + headRepo + '/actions'
await failCheck('Fork CI run not detected for ' + head_sha.substring(0, 8), forkActionsUrl)That target_url is the fork's Actions tab — it has no /runs/<id>/, so isForkTarget here is false, so the continue fires and update never re-checks this PR. Even if the fork CI run eventually registers and completes successfully, the Build status stays red until the contributor force-pushes.
This breaks the "5-min cron picks up late runs" recovery path that the 60s detection budget in notify implicitly assumes. Under heavy GitHub Actions load, queued-run registration past 60s is plausible.
Fix options:
- Use a sentinel
target_urlfor detection-failed that the updater treats as "keep looking" (e.g. point at.../actions/workflows/fork-ci.yml, whichisForkTargetwould still skip but a newisLookAgainTargetcheck would catch). - Or special-case detection-failed by description (
current.description.startsWith('Fork CI run not detected')) — distinct from the genuinely terminal failures (blocked branch, fork inaccessible) the comment is justifying.
| run_frontend: true | ||
| run_amber: true | ||
| run_amber_integration: true | ||
| run_platform: true | ||
| run_python: true | ||
| run_agent_service: true |
There was a problem hiding this comment.
Hardcoding all run_*: true bypasses the label-driven CI gating documented in AGENTS.md → "CI labels & gating" and implemented by LABEL_STACKS in required-checks.yml.
Before this PR, a docs-only PR (label union → empty) skipped every build stack; a frontend-only PR ran only frontend; etc. After this PR, fork CI runs the full matrix on every push regardless of labels. Apache's runner budget is unaffected (public-fork minutes are free), but this means slower PR feedback for contributors and wasted compute on every docs-only change.
Two ways to keep parity:
- Mirror the
LABEL_STACKSprecheck logic intofork-ci.yml(re-fetch PR labels forhead_refvia the GitHub API on push, set therun_*outputs accordingly). The fork has read access to PR labels on the upstream repo. - Accept the regression for now and update the AGENTS.md "CI labels & gating" section to reflect that label gating now applies only to push events / backports, not PR builds.
What changes were proposed in this PR?
Moves PR builds off the Apache repo and into the contributor's fork, so PRs from forks no longer need first-time-contributor approval and don't burn Apache's GitHub Actions minutes.
fork-ci.ymlthat runs the full build matrix and license header check on every push to a branch in the contributor's fork. It is a no-op when run insideapache/texera.notify_test_workflow.ymlthat posts aBuildstatus on the PR and links it to the matching fork CI run.update_build_status.ymlthat keeps theBuildstatus in sync with the fork run as it progresses.pull_requesttrigger fromrequired-checks.ymlandcheck-header.ymlso they no longer duplicate fork CI on the Apache side.automatic-email-notif-on-ddl-change.ymlfrom apull_request: closedtrigger to apush: maintrigger so it stops queuing for approval on every fork PR..asf.yamlso the required status checks areBuild(mirrored from fork CI) andValidate PR title.Any related issues, documentation, discussions?
Closes: #4290
How was this PR tested?
Tested manually on a fork:
Buildstatus appears on the PR and updates to success or failure based on the fork run.main,release/**, andci-enable/**in a fork are correctly skipped with a clear PR comment.required-checks.ymlstill runs on push tomain,release/**, andci-enable/**in the Apache repo.Was this PR authored or co-authored using generative AI tooling?
Co-Authored with Claude Opus 4.7 in compliance with ASF