Skip to content

Add manual fork-PR CI dispatch to ci.yml - #405

Merged
gavindidrichsen merged 3 commits into
mainfrom
add-manual-fork-ci-dispatch-v2
Oct 28, 2026
Merged

gavindidrichsen merged 3 commits into
mainfrom
add-manual-fork-ci-dispatch-v2

Conversation

@LukasAud

Copy link
Copy Markdown
Contributor

Summary

Replaces #402 with the same end result and a clean 2-commit history (identical final diff, verified — #402 accumulated some since-corrected mistakes across iterative review rounds that don't need to stay in permanent history). The Puppet-8-nightly-CI fixes that were bundled into #402 are split out to #404, since they're an unrelated, pre-existing problem.

  • Fork PRs don't receive secrets (e.g. PUPPET_FORGE_TOKEN, CODECOV_TOKEN) via the normal pull_request trigger, so CI silently skips secret-dependent steps for external contributors.
  • Adds pr_number/ref inputs to the existing workflow_dispatch trigger, plus a resolve-ref job that resolves either into an immutable commit SHA (or empty string, meaning "no override, behave exactly as before").
  • For pr_number, resolves the PR's merge commit SHA (not the mutable refs/pull/<n>/merge ref, and not the raw head SHA) via the GitHub API, failing explicitly if the PR isn't mergeable yet or has conflicts — this matches the normal pull_request trigger's test semantics while staying immutable across job boundaries, since spec and acceptance check out independently and acceptance only starts once spec finishes (a mutable ref could let a fork author push a new, unreviewed commit in that window and have it silently execute with secrets in scope).
  • Threads that SHA into the spec job (gem_ci.yml@main, which gained a ref input in Add optional ref input to gem_ci and gem_acceptance cat-github-actions#191) and the acceptance job (the repo-local custom_acceptance.yml, which needed the same ref input added since it isn't one of the shared cat-github-actions workflows).
  • workflow_dispatch is already gated to users with repo write access — that's the trust boundary. No label/approval system layered on top, intentionally lightweight for this low-traffic tool repo.

Test plan

  • Dispatch ci manually with ref set to a branch/SHA and confirm spec/acceptance check out that ref
  • Dispatch ci manually with pr_number set to a fork PR and confirm both jobs run against the resolved merge commit SHA with secrets available
  • Confirm normal push/pull_request runs on main are unaffected (ref input is empty, behaving exactly as before)

🤖 Generated with Claude Code

LukasAud and others added 2 commits October 27, 2026 14:09
Fork PRs don't get secrets (e.g. PUPPET_FORGE_TOKEN, CODECOV_TOKEN) via
the normal pull_request trigger, so CI silently skips secret-dependent
steps for external contributors. Add pr_number/ref inputs to the
existing workflow_dispatch trigger and a resolve-ref job that resolves
either into a ref, threaded into both the spec job (gem_ci.yml@main,
which gained a ref input in puppetlabs/cat-github-actions#191) and the
acceptance job (the repo-local custom_acceptance.yml, which needed the
same ref input added since it's not one of the shared cat-github-actions
workflows).

workflow_dispatch is already gated to users with repo write access,
which is the trust boundary — no label/approval system is layered on
top, intentionally lightweight for this low-traffic tool repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
refs/pull/<n>/merge is mutable: GitHub repoints it on every new push
to the PR, and spec/acceptance check it out independently, with
acceptance only starting after spec finishes. A fork author could
push a malicious commit in that window and have acceptance silently
execute it with PUPPET_FORGE_TOKEN in scope, instead of the commit a
maintainer actually reviewed before dispatching.

Resolve pr_number/ref to a concrete, immutable commit SHA once via the
GitHub API instead — specifically the PR's merge commit (matching the
module-track fork-PR precedent and the semantics of the normal
pull_request trigger, which also tests the merge result rather than
the raw head), failing explicitly if the PR isn't mergeable yet or has
conflicts rather than silently testing the wrong commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@LukasAud
LukasAud requested a review from a team as a code owner October 27, 2026 14:09
@codecov

codecov Bot commented Oct 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.98%. Comparing base (7d33bd2) to head (683c8fb).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #405   +/-   ##
=======================================
  Coverage   88.98%   88.98%           
=======================================
  Files           6        6           
  Lines         853      853           
=======================================
  Hits          759      759           
  Misses         94       94           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gavindidrichsen gavindidrichsen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gavindidrichsen
gavindidrichsen merged commit da86617 into main Oct 28, 2026
13 of 14 checks passed
@gavindidrichsen
gavindidrichsen deleted the add-manual-fork-ci-dispatch-v2 branch October 28, 2026 16:43
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.

2 participants