[andygrove]

Which issue does this PR close?

Closes #4289. Supersedes #4570.

Rationale for this change

Nearly every pull request comes from a fork (194 of the 198 open ones), and the pull request tier plus the run-* label runs were about 60% of the roughly 2,900 runner-hours that ci.yml and ci_label.yml spent in the week to 2026-10-28. The merge queue was about 30% and the nightly about 8%.

apache/datafusion (apache/datafusion#25591) and apache/arrow-rs (apache/arrow-rs#11788) moved this work onto the contributor's fork, using the fork's own push trigger rather than a separate fork-only wrapper workflow, and treat the fork's result as informational. This applies the same arrangement to Comet. The merge queue stays on ASF runners and still runs the queue tier on the merge result (the PR tier plus Spark SQL on 4.1, Iceberg 1.11, macOS and the other queue-tier suites; the nightly-only and run-* suites are not part of it), so nothing reaches main on the strength of a fork run alone.

What changes are included in this PR?

• compute-changes.py: a push to a fork's branch gets the pr tier, and a pull request from a fork into main loses it upstream. A pull request opened inside a fork loses it too, since its branch push already ran the tier. The run-* label suites and the labeled run still run here, as do a pull request from a branch of this repository and a pull request to a release branch. One effect on a fork pull request: run-all-spark-profiles now brings forward only the four non-default profiles here, since the fork's run covers the default profile and the lints; ci.yml's profile table and the policy cases say so.
• ci_fork.yml (new): calls ci.yml on a push to any fork branch, the default branch included. It is a separate workflow, as ci_label.yml is, so a push to a branch of this repository cannot publish a skipped Required Checks next to a pull request's real one. Upstream its only job is skipped. ci.yml also triggers on push to main, so its Preflight skips a fork push unless ci_fork.yml called it (told apart by the caller's github.workflow), which leaves the bridge one workflow to look up. A newer push to a fork branch cancels the run in flight through a supersede job that compares the branch name with case, because Actions compares group names, == and != without regard to case and a fork can hold both feature and Feature.
• Scheduled workflows stay upstream. A fork that enables them would run the nightly on its owner's minutes, and with no earlier scheduled run to diff against it runs the whole nightly tier. Preflight in ci.yml, miri.yml, stale.yml, codeql.yml and the bridge skip a scheduled run outside this repository. required_checks skips itself when Preflight does, so a fork's schedule or sync starts no runner, and nightly_report reads a skipped aggregator as nothing to report.
• ci.yml: a fork push is diffed against upstream main by dev/ci/fork-push-changed-files.sh, which gives the diff its pull request will have. Diffing against the fork's own main would omit a file that a branch reverts relative to newer upstream code when the fork is behind. The script lists every file when the fetch fails or there is no merge base, and nothing when the branch adds nothing to upstream, and dev/ci/test-fork-push-changed-files.py covers those cases on real repositories. A fork push is keyed on its run id in the concurrency group, so a backup branch at the same commit cannot cancel a pull request's build. Preflight runs the bridge tests.
• fork_ci_status.yml (new): mirrors the fork's run onto the pull request as an informational CI (fork) check, on workflow_run and every 15 minutes. It is adapted from arrow-rs, never checks out pull request code, and needs only checks: write. dev/ci/test-fork-ci-status.cjs covers it against mocked GitHub APIs (38 tests).
• check-ci-config.py: policy cases for fork pushes and fork pull requests, the new environment wiring, and checks that the caller, the bridge and ci.yml's push trigger agree. The guards on Preflight, ci_fork.yml's job and every root job of a scheduled workflow are checked by evaluating their if: for concrete events (a fork's schedule, a push through ci_fork.yml, a push to a fork's main), not by matching text. Each guard was mutation-tested.
• ci.yml's concurrency group is keyed on the pull request number, not github.head_ref. The branch name alone let two pull requests from different forks with the same branch name cancel each other's run. This is part of ci: concurrency group is keyed on the branch name, so same-named fork branches can cancel each other's CI #7315; codeql.yml, miri.yml and pr_title_check.yml have the same pattern, so that issue stays open. The tests pin that same-named branches get separate groups.
• Docs: a "Pull requests from forks" section in the CI guide, a pointer in the development guide, and a "Fork runs" section in the workflows README.

CI (fork) is advisory, not required, because a contributor controls the workflow that produced it. A green fork run that never ran the tests costs one queue slot. The earlier attempts mirrored the fork's result into a required check; arrow-rs and DataFusion decided against that, and so does this PR.

How are these changes tested?

• python3 dev/ci/check-ci-config.py, node --test dev/ci/test-fork-ci-status.cjs and actionlint --shellcheck=off pass locally. I removed each new guard in turn and confirmed the check fails.
• End to end on a fork (the author's), with this branch's workflows: a push to a branch started only Comet CI (fork run), which ran the full PR tier and passed. With cold caches it took 59 minutes of wall time and 240 runner-minutes, with the native build at 23 minutes. With warm caches (from the fork's main run by the old ci.yml) it took 50 minutes and 213 runner-minutes, with the native build at under 5. A push that adds nothing to upstream main, such as a sync of the fork, selects no build or test suite and writes to no build cache. For comparison, successful full PR runs on ASF runners over the previous week had a median wall time of 81 minutes. No job waited for a runner. This is one account and two runs.
• On this pull request: it touches ci.yml and dev/ci/**, which select the full Linux build, and its upstream run ran only Preflight, Detect changes and Required Checks. PR Build (Linux) and every other job were skipped, and Required Checks passed.
• Not tested on GitHub: fork_ci_status.yml. workflow_run and schedule use the copy on the default branch, so it cannot run until this merges. The bridge is covered by its mocked tests, as in the arrow-rs and DataFusion changes, and the first fork pull request after merge is its real test.

Contributors need to enable Actions on their fork and rebase onto main; the CI guide says so, and CI (fork) says "Enable CI on your fork" when it finds no run.

[ErikBPF]

Reviewed f2e5fa60c8d943af8daa9384a1db1baa7a9d8015.

P2 — ci.yml:341 — Preserve cumulative coverage for pushes to fork main

A PR from the fork’s default main takes the incremental before..after path. A native-code push followed by a README-only push therefore sets build_linux_full=false, although the PR still changes native code. Upstream now skips that tier too, and branch-keyed concurrency can cancel the unfinished code run. The actual workflow shell reproduced the selection failure.

Evidence

elif [[ "$REPOSITORY" != "apache/datafusion-comet" && "$PUSH_REF" != "refs/heads/$DEFAULT_BRANCH" ]]; then

Action: Use cumulative coverage or full PR-tier selection for fork-default-branch pushes. Add a code-then-docs regression test.

P2 — ci.yml:350 — Preserve upstream changes when the fork baseline is stale

Let fork main remain at A while upstream advances to B with a native-file change. A feature branch based on B that reverts that file now matches A, so this diff omits it. The reproduction selected only README.md and returned build_linux_full=false, while the upstream PR diff also contained native/core/src/lib.rs. Syncing fork main avoids this case, but the implementation promises conservative handling of stale baselines.

Evidence

git diff --name-only "origin/$DEFAULT_BRANCH"..."$PUSH_AFTER" > changed_files.txt

Action: Preserve upstream-relative changed paths or fall back conservatively. Add a stale-baseline/native-revert regression test. Correct the “never fewer” claim alongside the implementation.

P2 — docs/source/contributor-guide/ci.md:123 — Describe the queue coverage actually provided

The new documentation presents complete queue coverage as the reason fork results can remain advisory. The queue still selects only applicable queue-tier suites. Nightly-only and label-only suites remain excluded, and path filters still apply.

Evidence

The merge queue, which also stays here, runs every suite on the merge result before anything lands.

Action: Replace “every suite” with “the applicable queue-tier suites” here and in the matching workflow and README explanations.

Inherited security findings

These predate this PR. Track the repairs separately and link the issues.

P1 — dev/release/publish-to-maven.sh:165 — Stop logging publishing authentication

The Nexus close request combines curl -u with verbose output. Curl writes the recoverable Basic Authorization header to stderr, which the surrounding command substitution does not capture. A loopback test with dummy credentials reproduced the disclosure. Historical log exposure remains unverified.

Evidence

-H "Content-Type:application/xml" -v \

Action: Remove -v. Have the release owner privately inspect retained stderr or combined transcripts and revoke credentials if exposure is confirmed.

P2 — publish_snapshot.yml:219–220 — Keep publishing credentials out of dry runs

dry_run skips Maven settings creation and changes the goal to install, but both Nexus secrets still enter the build environment. Maven and its plugins can read them before any publication is needed. A dummy Maven stub confirmed both variables remained accessible with GOAL=install.

Evidence

NEXUS_USER: ${{ secrets.NEXUS_USER }}

NEXUS_PW: ${{ secrets.NEXUS_PW }}

Action: Make dry runs credential-free. Build without publishing secrets and perform real publication in a fresh restricted job.

P2 — docs.yaml:89 — Separate dependency execution from repository-write access

The docs caller grants contents: write, and both checkouts persist authentication before dependency installation and Sphinx/Maven execution. A compromised selected dependency can use the same writer intended for the final website push. The main-ref guard blocks direct fork execution, but does not contain dependency compromise during an authorized build.

Evidence

pip install -r docs/requirements.txt

Action: Build docs in a read-only job with non-persisted checkout credentials. Publish static output from a separate minimal write-scoped job. This isolates credentials, but does not establish that a compromised builder produced safe content.

[andygrove]

cc @blaginin

[andygrove]

Thanks for the careful review. It was of the first commit, so some of this has moved; the current head is f1a3040.

The fork main finding no longer has a code path to get wrong. A push to a fork's default branch runs nothing now (Preflight skips it, since it is a sync of commits the queue already tested), and a pull request opened from it gets a neutral "open the pull request from a branch" rather than a mirrored run.

The stale baseline finding is right, and the "never fewer" claim was wrong. A fork push is now diffed against upstream main by dev/ci/fork-push-changed-files.sh, which is the diff the pull request will show. It lists every file when the fetch fails, there is no merge base, or the diff is empty. dev/ci/test-fork-push-changed-files.py builds real repositories for your native-revert case, code followed by docs, git push fork main:my-fix, an unreachable upstream and an empty diff. I checked that the revert test fails if the script goes back to diffing against the fork's own main, and that the fetch works on a runner with the checkout credentials removed.

On the queue coverage, the docs, the workflows README and the workflow comments now say the queue runs the queue tier with path filters, and that the nightly-only and run-* suites are not part of it.

The three inherited findings are outside this diff, so I have not touched publish-to-maven.sh, publish_snapshot.yml or docs.yaml here.

[sunchao]

The upstream-main diff fixes the stale-fork case from the earlier review. The CI policy check, all seven real-Git diff tests, and all 36 bridge tests passed for f1a3040d.

I’m requesting changes because the advisory check can tell a contributor that CI is missing when their branch has a run, and the new schedule validator passes even when the guard that keeps the full nightly off forks is removed. I’ve also marked where the sync job and rerun cadence differ from the contributor guide. The status bridge itself has only been exercised with mocked APIs on this revision.

[andygrove]

The two CI findings are now tracked as #7319 (the snapshot dry run) and #7320 (the docs deploy). The release script finding is for the release manager and will be handled separately from this PR.

[sunchao]

Thanks for following up on all four comments. I verified the fixes in 40eb6cf2: a fork with default develop now tests a branch named main through the workflow the bridge reads, removing just the fork-schedule guard now fails validation, and the aggregator and reporter both skip the intended events. The new rerun-cadence explanation matches the bridge.

I’m requesting changes for a remaining branch-identity issue. Distinct Git branch names can differ only in capitalization. One such branch can lose its build at the default-branch check, and two feature branches can cancel each other. I’ve also pointed out a regression test that still passes if the previous main trigger exclusion is restored.

One small wording change would finish the guide: the two bullets at ci.md lines 157 and 162 still say a fork’s main gets no CI. They should say that the fork’s default branch skips the Comet PR tier, consistent with the develop case you just fixed.

The policy check, all 37 bridge/concurrency tests, and all seven Git diff tests passed locally. Upstream Required Checks passed and the fork build was still running when I reviewed.

[andygrove]

On the two guide bullets at the end of your review: they now say the fork's default branch does not run the Comet PR tier, rather than that nothing runs on main, and the 'Open the pull request from a branch' bullet notes that a case variant such as Main counts as the default. Also in 86f1bae.

[comphead]

This follows the pattern DataFusion and arrow-rs adopted and that Spark's notify_test_workflow.yml/update_build_status.yml have used for years. The new workflows check out nothing, interpolate no event values into scripts, and the mirrored check stays advisory, so a fork controlling its own run cannot gate a merge. Four comments.

[andygrove]

A change of direction on fork pushes, in dd828c0: a push to a fork's default branch is no longer skipped. It runs like any other branch push, so ci_fork.yml's jobs skip only upstream, the bridge looks up the same workflow for every pull request, and the default-branch and case-variant handling from the earlier rounds is gone. A push that adds nothing to upstream main, such as a sync of the fork, still selects no suite because its diff against upstream is empty, so it starts Preflight and Detect changes and writes no cache. ci.yml's Preflight still skips a fork push that did not come through ci_fork.yml, since ci.yml also triggers on push to main. The threads about the default branch and its case variants no longer apply.

[sunchao]

I rechecked dd828c0 against Andy’s updated fork-push design and resolved my two default-branch threads. Changes made directly on a fork’s main select the expected suites, an empty sync selects none, and restoring main to branches-ignore now fails validation.

The remaining CI issue is that two branches in the same fork can cancel each other when they point to the same commit. I followed up in the existing concurrency discussion.

The config check, seven Git diff tests, and all 38 Node tests passed locally. The upstream and fork runs passed on this head. The advisory bridge still awaits its first hosted run from the default branch. Upstream run · Fork run

[sunchao]

Approved at 6dc32f21. Fork pushes now get separate concurrency groups even when two branches point to the same commit. Restoring the old SHA-based key made the new regression test fail. The empty-sync documentation is accurate, and I verified and resolved the six remaining review threads.

I traced the fork CI failure to an existing order-sensitive assertion in CometParquetWriterSuite. Both partition files were created and closed, but they closed in the opposite order. The same assertion failed upstream on an unrelated PR before this fork run. The test, its recording filesystem, and the Linux build workflow are identical to the PR base. A separate test fix is open. Fork failure · Independent upstream failure · Test fix #7335

The CI config check, all 38 Node tests, and all seven Git diff tests passed on this head. Upstream CI passed. The informational bridge still needs its first hosted run after the workflow reaches the default branch. Upstream CI

[andygrove]

Thank you @sunchao, @comphead and @ErikBPF for the thorough reviews. Between you they found a stale-baseline diff, the hole in the schedule guard, a runner started on skipped runs, the case-insensitive branch and group comparisons, the same-commit collision, the over-wide bridge permissions and a wrong example of mine in the empty-diff case, and each one changed the code. This is now in the merge queue.

Two things to watch after it lands. The CI (fork) bridge has only run against mocked APIs, so the first fork pull request after the merge is its real test, and the first scheduled nightly on main should still run. If either looks wrong, please say so here or on #7315.