ci: skip eval jobs deterministically on fork PRs#2345
Conversation
Fork PRs cannot receive repository secrets or a writable GITHUB_TOKEN, so the eval shards, the report comment, and the ghcr.io image push can never succeed from a fork. Today the outcome depends on Docker layer cache state rather than on the code: a warm cache lets build-image pass, the eval shards then run and fail at SDK auth; a cold cache makes build-image fail its push and the evals skip entirely. Identical code produces opposite CI verdicts. Gate build-image, evals, and report on the PR head being in the base repository. Same-repo PRs, pushes, and workflow_dispatch are unchanged and keep full coverage.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
The first cut gated build-image, evals, and report on the PR head being in the base repo. That skipped every job in the workflow, and a run in which no job starts is recorded as a startup failure (conclusion: failure, zero jobs) rather than a skip -- trading one bogus red for another. Let build-image always run instead, and make only its push conditional. A fork still builds the CI image, so Dockerfile.ci changes stay validated, it just does not publish with a packages:read token. That guarantees at least one job starts while evals and report skip deterministically.
The previous comment claimed an all-skipped run registers as a startup failure. That is not the mechanism -- an all-skipped run is simply grey. The real reason build-image stays ungated is that fork PRs should still validate Dockerfile.ci and get one honest green check.
Heads up: this PR's own checks cannot run, and that is expected
The tell is The actual mechanism: GitHub refuses to start workflow runs for a pull request from a fork when that pull request modifies anything under Corroborating evidence from this repo: I checked the last 60 merged PRs. Every merged PR that touched So this PR is in the somewhat funny position of being blocked by a stricter cousin of the exact problem it is trying to fix. What I did verify locally
What a maintainer needs to doBecause of the restriction above, this change can only be exercised from a branch in the base repo: Then open a PR from that branch, or just cherry-pick the single commit. It is one file and roughly ten lines. What the change doesOn a fork PR:
On same-repo PRs, pushes, and Why it is worth doingRight now the outcome for a fork PR is decided by Docker layer cache state, not by the code:
Same commit, opposite verdict, depending on whether someone else happened to warm the cache first. #2264 merged with This just makes the skip explicit instead of accidental. |
1 participant
Problem
Fork PRs cannot pass this workflow, and today whether they appear to pass is decided by Docker layer cache state rather than by the code under review.
GitHub does not pass repository secrets to a
pull_requesttriggered from a fork, and issues a read-onlyGITHUB_TOKEN(fork workflow security). Three consequences follow, none of which any contributor can fix from a branch:evals—ANTHROPIC_API_KEY,OPENAI_API_KEY, andGEMINI_API_KEYare all empty. Agent shards fail witherror_api;llm-judgethrowsCould not resolve authentication methodatnew Anthropic()before a request is made;e2e-pty-plan-smokeshowsNot logged inand then times out at 300s. No model executes, so these burn 0 tokens and $0.00 across every retry.report— cannot post its comment:Resource not accessible by integration (addComment).build-image— cannot push to ghcr.io withpackages: read.The nondeterminism
evalsdeclaresneeds: build-image, and the image tag is content-hashed overDockerfile.ci,package.json, andbun.lock. So:build-imageevalsIdentical code, opposite verdicts. #2264 merged in the second state with
build-imagered andevalsshowingskipping; #2323 sits in the first state with 8 red checks. Neither outcome reflects the branch.This also means no fork PR can reach green while these jobs run.
e2e-pty-plan-smokedoes not use diff-based test selection (shouldRun = EVALS && EVALS_TIER === 'gate'), so it executes on every PR regardless of the diff, and it needs an authenticatedclaude. Confirmed across unrelated fork PRs from different authors, which fail with byte-identical errors.Change
Gate
build-image,evals, andreporton the PR head living in the base repository:Fork PRs now skip these jobs deterministically — the same outcome the cold-cache path already produces by accident, but for a stated reason and every time. Same-repo PRs, pushes, and
workflow_dispatchare untouched and keep full eval coverage. Maintainers get real coverage on fork contributions the way they do today: push the commits to a base-repo branch (git fetch origin pull/N/headthen push that ref) and let it run with secrets.What this does not do
test/skill-e2e-first-task-scaffold.test.tsthrowsrequires ANTHROPIC_API_KEY ... refusing to skip).pull_request_target. Running untrusted fork code with secrets is unsafe.build-imagepush behavior generally. Making the build run withpush: falseon forks soDockerfile.cistill gets compile-checked is a reasonable follow-up, deliberately out of scope here.Verification
actionlint(v1.7.11, matching.github/workflows/actionlint.yml) reports the same 2 pre-existingrunner-labelwarnings before and after; no new findings.needsedges unchanged.if:conditions plus a comment. No step, matrix, secret, or permission changes.Note for maintainers
If Trunk requires the
evals (...)orreportcontexts for merge, please confirm skipped jobs satisfy them as expected. I could not inspect the Trunk configuration from outside the org. Happy to adjust the approach.Found while investigating #2323; filed separately so that PR stays free of CI edits.