Skip to content

ci(nixos): allow fork PR checkout under pull_request_target (+ ARM-primary) - #545

Merged
mrosseel merged 1 commit into
releasefrom
fix/nixos-pr-fork-checkout
Jul 21, 2026
Merged

ci(nixos): allow fork PR checkout under pull_request_target (+ ARM-primary)#545
mrosseel merged 1 commit into
releasefrom
fix/nixos-pr-fork-checkout

Conversation

@mrosseel

Copy link
Copy Markdown
Collaborator

Problem

Testable fork PRs (e.g. #534 "Asteroids!", from mrosseel:feature/observable-asteroids) fail immediately at checkout:

Refusing to check out fork pull request code from a 'pull_request_target' workflow...
set 'allow-unsafe-pr-checkout: true' on the actions/checkout step.

actions/checkout recently began refusing to check out a fork PR's head when the trigger is pull_request_target. Because pull_request_target runs the base repo's default-branch copy of the workflow, this can only be fixed here on release — a contributor's fork branch can't change it. Same-repo PRs are unaffected, which is why it only bites fork PRs.

Fix

  • Add allow-unsafe-pr-checkout: true to the two head-checkout steps only (build-hosted, build-pi5). The trusted update-manifest checkout (base branch, holds the write token) is left untouched.
  • This is safe here: the build job runs only after a maintainer applies the testable/preview label (the existing review gate for running contributor code) and checks out a pinned head SHA.
  • Also flips the primary runner to the free ubuntu-24.04-arm hosted runner, with the self-hosted Pi5 demoted to a last-resort fallback (matches the ci-nixos-arm-primary change).

Effect

Once merged into release, re-triggering a labeled fork PR (toggle its testable label) builds on GitHub's ARM runner instead of dying at checkout.

…mary

actions/checkout now refuses to check out a fork PR's head under
pull_request_target unless allow-unsafe-pr-checkout is set, so every
testable fork PR (e.g. the Asteroids PR) fails at checkout before
building. Add the opt-in on the two head-checkout steps only; the
trusted update-manifest checkout is left untouched. Also flips the
build to the free ubuntu-24.04-arm hosted runner first, with the
self-hosted Pi5 as last-resort fallback.
@mrosseel
mrosseel merged commit 571bde8 into release Jul 21, 2026
5 checks passed
brickbots added a commit that referenced this pull request Jul 31, 2026
release had accumulated CI work that was never on main (#514, #515,
#516, #519, #522, #545), and both branches had independently applied the
same focus-technique docs (#546 on main, #547 on release) and the same
manifest/PR-builder changes. Merging release in first makes the coming
main -> release promotion clean instead of carrying four conflicts into
the release cut.

Four conflicts, each resolved by taking one side wholesale -- none was a
semantic disagreement, only the same work committed twice:

  .github/scripts/update_manifest.py    -> release
      Strict superset: newest-first entry ordering plus the
      --migration-url / --migration-sha256-url arguments from #516.
      main had nothing release lacked.

  .github/workflows/nixos-pr-build.yml  -> release
      Genuinely divergent designs. main (#493) polls a self-hosted Pi5
      first via a native-wait job, falling back to a hosted arm64
      runner; release (#495 + #545) was re-architected ARM-primary with
      the Pi5 as last-resort fallback, drops the polling job, and adds
      allow-unsafe-pr-checkout for fork PRs under pull_request_target.
      release's is the newer and hardened design.

  docs/source/quick_start.rst           -> main
  docs/source/troubleshooting.rst       -> main
      Both branches carry the same small-turn focus technique, but
      release applied it to the old Focus screen's prose (+/- zoom to
      2x/4x). main describes the rebuilt screen from #531 -- magnified
      star tiles, SQUARE cycling to the Image view, the central HFD --
      which is what 2.6.1 actually ships.

Net effect: main gains the release-only CI (release.yml, workflow_dispatch
only, so nothing fires on merge) and keeps its own newer docs. No source
changes. 1100 unit+smoke tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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