Skip to content

fix(ci): change detection compares a pull request with current main, not its stale base - #5145

Closed
miguel-heygen wants to merge 2 commits into
mainfrom
fix/ci-paths-filter-merge-parent
Closed

miguel-heygen wants to merge 2 commits into
mainfrom
fix/ci-paths-filter-merge-parent

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

What

A pull request that is behind main runs gates for code it never touched. #5139 changes only docs, registry and scripts files, yet its CI run ran "Studio: timeline viewport gate", which then failed on a 0.1 ms timing difference.

With this change, change detection on a pull request compares the PR's merge commit with the current tip of main, so only the PR's own files count.

Why

The five workflows that gate jobs on changed paths use dorny/paths-filter with token: "". For a pull_request event, that action diffs pull_request.base.sha..HEAD with two dots (src/main.ts and src/git.ts at the pinned ceb8a2b). HEAD is the merge commit refs/pull/N/merge, which contains current main. base.sha is where the PR's base stood when GitHub last recorded it, which goes stale as main moves. Every commit that landed on main since then is counted as the PR's change.

For #5139 at the time of its run:

  • git diff --name-only e6daed3df0 refs/pull/5139/merge lists packages/studio/** and packages/core/** files.
  • git diff --name-only refs/pull/5139/merge^1 refs/pull/5139/merge lists none.

How

  • In ci.yml, player-perf.yml, preview-regression.yml, regression.yml and windows-render.yml, a step before the filter reads git rev-parse HEAD^1 on pull requests. The merge commit's first parent is the current tip of main.
  • That SHA is passed as the filter's base. With token: "", the action uses base ahead of base.sha for pull requests.
  • This is the same rule catalog-previews.yml already uses for its changed-items diff.
  • Push, merge-queue and scheduled runs leave base empty, so they behave exactly as before.

Test plan

  • actionlint reports nothing new in the five workflows.
  • Proof PR test(ci): docs-only proof for #5145, do not merge #5146 (closed): a docs-only change whose recorded base.sha stayed at a commit 52 packages/ files behind its base branch, even after a new push. Player perf, regression and preview-regression each logged Using base '<merge^1>' instead of the pull request base, Detected 1 changed files, and their filter as false. Under the old rule, the same merge commit diffs 60 files.
  • This PR's own "Detect changes" in CI logged Using base '0ca99b30…' (the merge commit's first parent) and Detected 5 changed files: the five workflows.
  • After merge: a docs-only PR behind main skips the Studio jobs. Before merge this cannot be shown on main, because the studio filter lists .github/workflows/ci.yml, so any PR carrying this change runs it.
  • This PR's own CI is green. It changes ci.yml, which several filters list, so it runs those gates itself.

dorny/paths-filter with token "" diffs pull_request.base.sha..merge with two dots; base.sha goes stale as main moves, so a PR behind main counted main's newer commits as its own and ran unrelated gates. Pass the merge commit's first parent as base on pull requests.
…mmit

echo "sha=$(git rev-parse HEAD^1)" exited 0 when rev-parse failed, and on a non-merge checkout HEAD^1 would be the PR's previous commit, silently skipping gates. Verify HEAD^2 exists and assign the SHA so a failure stops the step.
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown

Edit accuracy: accurate 2059 (base branch 2059), smooth 1518 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Folded into #5151, which carries this change with the timeline overscan fix that makes the Studio gate pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant