Repository navigation
Move fork-PR credentialed CI off pull_request_target - #1044
Merged
Merged
Conversation
david-siqi-liu
force-pushed
the
david/ci-fork-integration-approval
branch
2 times, most recently
from
October 8, 2026 15:23
c21bf9a to
aa4658a
Compare
This was referenced Oct 8, 2026
david-siqi-liu
force-pushed
the
david/ci-fork-integration-approval
branch
from
October 8, 2026 17:02
aa4658a to
30e828f
Compare
david-siqi-liu
changed the title
david-siqi-liu
force-pushed
the
david/ci-fork-integration-approval
branch
2 times, most recently
from
October 8, 2026 17:20
ea0669a to
ef6ad0d
Compare
david-siqi-liu
changed the title
david-siqi-liu
force-pushed
the
david/ci-fork-integration-approval
branch
5 times, most recently
from
October 9, 2026 17:13
e76dce1 to
978df65
Compare
david-siqi-liu
marked this pull request as ready for review
October 9, 2026 19:12
david-siqi-liu
requested review from
AarushiShah-db,
lilly-luo and
rohita5l
as code owners
October 9, 2026 19:12
rohita5l
approved these changes
Oct 9, 2026
rohita5l
left a comment
rohita5l
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the CI trigger changes. Please address the inline P1 concerning journey-check results being attached to a different revision than the one evaluated. Focused verification: 91 tests passed; live comment-triggered workflows were not exercised.
max-rozen-oss-db
approved these changes
Oct 9, 2026
david-siqi-liu
force-pushed
the
david/ci-fork-integration-approval
branch
from
October 10, 2026 14:14
3bf1e47 to
eb7ce7c
Compare
david-siqi-liu
enabled auto-merge
October 10, 2026 14:14
Fork PRs get no secrets on pull_request, so every credentialed integration job skips for them, and GitHub is closing the pull_request_target route (default-branch-only workflows, the checkout fork guard, and the default block in public repos from 2026-11-02). Following dbt-databricks, an org member comments /integration-test after reviewing a PR. An issue_comment workflow from the default branch runs integration.yml on the PR head with secrets and posts a Fork integration status and comment. /integration-test <sha> runs only if the head is still the reviewed commit. Non-members get a reply saying only org members can run it, and in-repo PRs can use it as an on-demand rerun. integration.yml takes the commit as a ref input, pins every checkout to it without persisted credentials, skips the uv cache for these runs, and keys its concurrency group per PR for comment events. ug-review.yml moves to an org member /ug-review comment. user-journey-test-required.yml runs on pull_request for in-repo PRs and on an org member /user-journey-check comment for fork PRs, reporting a commit status back. No workflow uses pull_request_target any more. Co-authored-by: Isaac <no-reply@databricks.com>
The managed-opencode job landed on main (#896) after this branch was cut, so it lacked the pinned, credential-free checkout and the disabled uv cache that fork runs require. Co-authored-by: Isaac <no-reply@databricks.com>
The authorize step is bash that only runs on ubuntu-latest, and its fake `gh` shim cannot execute on the Windows unit-test runner. Co-authored-by: Isaac <no-reply@databricks.com>
The fork report step refetched the PR head after the judge ran, so a push during the check got the previous revision's verdict. The gate now verifies the head is unchanged across its diff reads, outputs that SHA, and the report posts only against it. Also document in AGENTS.md that workflows must not use pull_request_target. Co-authored-by: Isaac <no-reply@databricks.com>
david-siqi-liu
force-pushed
the
david/ci-fork-integration-approval
branch
from
October 12, 2026 15:00
eb7ce7c to
246cd62
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After this PR, no workflow uses
pull_request_target. Fork PRs get credentialed CI when an org member (MEMBERorOWNER) comments a command on the PR. Every comment-triggered run uses the default branch's workflow definitions. Design doc (option B): https://docs.google.com/document/d/1v-6znVBCb8YfGxr12SBd80w3i9n4U0fMEYohyz9Aoyw/editCommands
Org members comment these on a PR:
/integration-testruns the full Integration workflow with secrets on the PR's current head commit./integration-test <sha>does the same, but only if the head is still<sha>(7 to 40 hex characters). If the fork pushed since, it refuses and replies with the new head./ug-reviewruns the UG rubric review./user-journey-checkruns the user-journey gate. For in-repo PRs it also runs automatically, so the command is mostly for forks.Each command posts a commit status and a comment with the run link on the PR. The statuses are
Fork integrationandUser Journey Test Required. Each new push needs a new command. Anyone else who comments/integration-testgets a reply saying only org members can run integration tests.Before and after
Live integration on fork PRs:
pull_request. That covers the E2E CUJs, workspace validation, and the smoke, full, and managed journeys./integration-testor/integration-test <sha>. It runsintegration.ymlon the pinned commit with secrets.Live integration on in-repo PRs:
/integration-testalso works as an on-demand rerun, and it never cancels the automatic run.UG review:
ug-reviewlabel, viapull_request_target./ug-review. It still runs only default-branch code and never executes the PR.User Journey Test Required:
pull_request_targeton every PR.pull_requestfor in-repo PRs, and/user-journey-checkfor fork PRs. It still reads the PR only through the API. The gate checks the head is unchanged across its diff reads and reports the fork-PR status only on that evaluated commit, so a push during the check never inherits the old verdict.pull_request_target:ug-review.ymlanduser-journey-test-required.yml.Fork safety in
integration.ymlrefinput. When the input is empty, existing callers behave exactly as before.refand setspersist-credentials: false.AGENTS.mdnow says workflows must not usepull_request_target. The newfork-integration.ymlcall tointegration.ymlcarries the same inlinezizmor: ignore[secrets-inherit]asci.yml(#1089).Testing
gh. They also check the non-member reply, the pinned credential-free checkouts, the disabled caches, and the status reports.issue_commentworkflows only run from the default branch, so the first live run happens after merge. The plan is to comment/integration-teston a fork PR such as Memoize Databricks CLI tokens per process and skip impossible--no-browserre-auth #1017 or Cut ug startup cost: fast --version entry point and lazy heavy imports #1018, and/ug-reviewon any PR.OIDC note: under
issue_comment, the OIDC subject is the default branch, not a PR ref. If the JFrog trust policy for the Windows installation lane keys on PR refs, that one lane fails on fork runs. The other lanes use secrets, not OIDC.This pull request and its description were written by Isaac.