fix: run the app-ci path filter on pull requests only - #1
Merged
Conversation
dorny/paths-filter uses the GitHub API on a pull_request event, but on a push it has to diff against github.event.before with git. That commit is not in the changes job's default depth-1 checkout, so it fetches it — and the checkout persists no credential (zizmor requires that), so on a private repository the fetch dies with "could not read Username for 'https://github.com'". Every push to main after the first one therefore failed the changes job, which skipped ci and commitlint: the merge gate existed on pull requests only. The first push to a branch survived just because github.event.before is the all-zero sha there. Gate build-filters and the filter on github.event_name == 'pull_request' and fall the job's roots output back to inputs.roots otherwise. The filter is a pull-request time saver — most roots are untouched there — while on the default branch every root should be verified anyway, so the non-pull-request branch wants the full list regardless. Both branches emit a JSON array string, so fromJSON and the ci job's != '[]' guard keep working, no fetch-depth change is needed, and persist-credentials: false stays. Claude-Session: https://claude.ai/code/session_01J4HB8qJjdZwaAv42k6HMpv
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
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.
What this changes
app-ci.yml'schangesjob failed on every push tomainafter the very first one, in every project generated by the scaffold toolbox.ciandcommitlintare gated on its output, so both wereskipped— meaning no config root'sci-unithas ever run onmainin any generated project. The merge gate existed only on pull requests.Found independently by four end-to-end runs on four fresh private repositories, always the same three lines:
The cause: the
changesjob checks out at the defaultfetch-depth: 1withpersist-credentials: false, then runsdorny/paths-filter. On apull_requestevent paths-filter uses the GitHub API and is fine. On apushit must diff againstgithub.event.before, which a depth-1 checkout does not have, so it tries to fetch it — and there is no credential left to authenticate with on a private repository. The first push to a branch works only becausegithub.event.beforeis the all-zero SHA. This is the known interaction betweenpersist-credentials: falseand actions that shell out to git (reviewdog/reviewdog#2187, EndBug/add-and-commit#385).The fix runs the filter on
pull_requestonly, and falls back to the fullinputs.rootson every other event. The filter is a pull-request time saver — most roots are untouched there. Onmainthe opposite is wanted: a push to the default branch is exactly where every root should be verified, not a subset. That also removes the git dependency entirely, sopersist-credentials: falsestays and nofetch-depthchange is needed.How it was verified
mise run ci(this repo's own CI:zizmor .github/workflows/thenactionlint, thencheck) —EXIT=0,No findings to report.tests/workflows.batsshape tests — 5/5 pass, includingevery checkout disables credential persistence.${{ steps.filter.outputs.changes || inputs.roots }}: a skipped step yields an empty string (falsy) so the input passes through, while"[]"is a non-empty string (truthy) and survives. Both produce a JSON array string thatfromJSONaccepts, and a pull request touching no root still yields exactly[], so theif: needs.changes.outputs.roots != '[]'guard still skipsci.["apps/api","apps/web"][]["apps/api","apps/web"]Note for the reviewer
Merging this is not the whole delivery. The
v1tag has to move for it to reach any generated project, and that reaches all of them at once (ADR-0005). That is a separate, deliberate step.https://claude.ai/code/session_01J4HB8qJjdZwaAv42k6HMpv