Skip to content

Fix CI checkout and coverage upload for fork PRs#737

Merged
matt-ramotar merged 1 commit into
mainfrom
fix/ci-fork-pr-checkout
Jun 7, 2026
Merged

Fix CI checkout and coverage upload for fork PRs#737
matt-ramotar merged 1 commit into
mainfrom
fix/ci-fork-pr-checkout

Conversation

@matt-ramotar

Copy link
Copy Markdown
Collaborator

Problem

CI fails at the Checkout step (~7s, before any build/test) for fork PRs. Example: #735 (run) died with:

A branch or tag with the name 'fix/mutable-store-write-queue-race' could not be found

The build-and-test checkout pins ref: ${{ github.head_ref || github.ref }} but never sets repository, so for a cross-repository (fork) PR actions/checkout looks for the head branch in the base repo (MobileNativeFoundation/Store), where it doesn't exist — the branch lives in the contributor's fork. This is a latent bug introduced in #673; it never surfaced because every PR since has come from a same-repo branch.

A second, downstream blocker: the Codecov step uses secrets.CODECOV_TOKEN, which GitHub does not expose to fork PRs, combined with fail_ci_if_error: true — so even after fixing checkout, fork PRs would fail at coverage upload.

Fix

  • Checkout — resolve repository and ref from the PR head when present (github.event.pull_request.head.repo.full_name / head.sha), falling back to github.repository / github.ref for push builds. Fork PRs now check out the contributor's head commit; same-repo PRs and pushes to main are unchanged (still build the head commit, as before — not the merge ref).
  • Codecov — skip the upload on fork PRs (where the token is unavailable). Coverage is still uploaded and enforced for same-repo PRs and pushes to main.

Notes / review call-outs

  • Security: this is a pull_request trigger (not pull_request_target), so fork builds run with a read-only GITHUB_TOKEN and no secrets; persist-credentials: false is retained. Building untrusted fork code under pull_request is the standard, safe configuration.
  • Codecov policy: skipping coverage upload for forks is a deliberate trade-off (forks can't authenticate to Codecov). If you'd rather keep attempting a tokenless upload, the alternative is fail_ci_if_error: false on that step instead of the if: guard — happy to switch.
  • This workflow change only takes effect once merged to main; Fix RealMutableStore write-queue data race (inverted lock polarity) #735 will pass after it rebases on top of this (forks run the base branch's workflow definition for pull_request).

🤖 Generated with Claude Code

The `build-and-test` checkout pinned `ref: ${{ github.head_ref || github.ref }}`
without a matching `repository`, so for cross-repository (fork) PRs Actions
looked for the head branch in the base repo and failed at checkout in ~7s with
"a branch or tag with the name '<head-branch>' could not be found" — before any
build or test ran. This affected all external/fork contributions (e.g. #735).

- Checkout: resolve `repository` and `ref` from the PR head when present
  (`github.event.pull_request.head.*`), falling back to `github.repository` /
  `github.ref` for push builds. The `ref` uses the head branch *name* (not the
  head SHA) so HEAD stays attached to a branch — the KMMBridge plugin runs
  `git pull --tags`, which fails on a detached HEAD ("you are not currently on a
  branch"). Fork PRs now check out the contributor's head branch; same-repo PRs
  and pushes to main are unchanged.
- Codecov: skip the upload on fork PRs, where `CODECOV_TOKEN` is unavailable and
  `fail_ci_if_error: true` would otherwise fail the job. Coverage is still
  uploaded and enforced for same-repo PRs and pushes to main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@matt-ramotar matt-ramotar force-pushed the fix/ci-fork-pr-checkout branch from e8cae98 to 50a7416 Compare June 7, 2026 11:13
@codecov

codecov Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.17%. Comparing base (213e574) to head (50a7416).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #737   +/-   ##
=======================================
  Coverage   80.17%   80.17%           
=======================================
  Files          42       42           
  Lines         928      928           
  Branches      177      177           
=======================================
  Hits          744      744           
  Misses        110      110           
  Partials       74       74           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@matt-ramotar matt-ramotar merged commit 80db967 into main Jun 7, 2026
5 checks passed
@github-project-automation github-project-automation Bot moved this from 🆕 Triage to ✅ Done in Store Roadmap Jun 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant