Skip to content

fix(workflows): allow fork PRs in pr-review-panel#826

Merged
danielmeppiel merged 1 commit intomainfrom
fix/pr-review-panel-allow-forks
Apr 21, 2026
Merged

fix(workflows): allow fork PRs in pr-review-panel#826
danielmeppiel merged 1 commit intomainfrom
fix/pr-review-panel-allow-forks

Conversation

@danielmeppiel
Copy link
Copy Markdown
Collaborator

Problem

PR #815 (from edenfunf/apm fork) had the panel-review label applied but every job skipped: https://github.com/microsoft/apm/actions/runs/24751987642

Root cause: gh-aw's default fork policy on pull_request injects a fork-blocking gate (github.event.pull_request.head.repo.id == github.repository_id) on every job, so fork-originated runs skip entirely.

Fix

Add forks: ["*"] to the pull_request trigger in .github/workflows/pr-review-panel.md. Recompiled with gh aw compile --strict.

Trust model

The panel-review label is the trust gate: only write-access maintainers can apply labels, so a malicious fork PR cannot self-trigger the workflow. gh-aw's safe-outputs isolation handles the fork-token problem -- the write-scoped publisher job runs in base-repo context, separate from the read-only agent job.

Test plan

After merge, re-apply (or remove + re-apply) panel-review to PR #815 and confirm the panel runs and posts a verdict comment.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

PR #815 (from edenfunf/apm fork) had the panel-review label applied
but every job skipped because gh-aw's default fork policy on
pull_request blocks fork-originated runs entirely
(condition: github.event.pull_request.head.repo.id == github.repository_id).

Add forks: ["*"] to remove the repo-id gate. The trust gate is the
panel-review label itself -- only write-access maintainers can apply
labels, so a malicious fork PR cannot self-trigger the workflow.
gh-aw's safe-outputs isolation handles the fork-token problem (the
write-scoped publisher job runs in base-repo context, separate from
the read-only agent job).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 21, 2026 23:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Enables the pr-review-panel agentic workflow to run for fork-originated pull requests by explicitly allowing forks in the gh-aw source workflow, and recompiling the locked workflow output accordingly.

Changes:

  • Add forks: ["*"] to the pull_request trigger in .github/workflows/pr-review-panel.md.
  • Recompile and update .github/workflows/pr-review-panel.lock.yml, removing the default same-repo fork gate from job if: conditions.
Show a summary per file
File Description
.github/workflows/pr-review-panel.md Allows fork PRs to trigger the review-panel workflow via gh-aw frontmatter.
.github/workflows/pr-review-panel.lock.yml Updates compiled workflow to reflect fork allowance and associated condition changes.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

if: >
(github.event_name != 'pull_request' || github.event.pull_request.head.repo.id == github.repository_id) &&
(github.event_name != 'pull_request' || github.event.action != 'labeled' || github.event.label.name == 'panel-review')
if: github.event_name != 'pull_request' || github.event.action != 'labeled' || github.event.label.name == 'panel-review'
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

pre_activation runs on every pull_request synchronize event (because the if only filters labeled events). With forks: ["*"], any fork PR push will now trigger at least this job, which undermines the stated cost gate and increases risk of abuse/CI noise. Consider adding a label-present check for synchronize (e.g., guard on the PR labels array) so the workflow only proceeds when panel-review is already applied, and keep labeled as the activation path.

Suggested change
if: github.event_name != 'pull_request' || github.event.action != 'labeled' || github.event.label.name == 'panel-review'
if: >
github.event_name != 'pull_request' ||
(github.event.action == 'labeled' && github.event.label.name == 'panel-review') ||
(github.event.action == 'synchronize' && contains(toJson(github.event.pull_request.labels.*.name), '"panel-review"'))

Copilot uses AI. Check for mistakes.
Comment on lines 5 to +9
# Trigger: pull_request (NOT pull_request_target -- gh-aw blocks the latter on
# public repos). Cost-gate: only runs when a maintainer applies `panel-review`.
# `synchronize` re-runs on new pushes once the PR carries the label.
# Default fork policy (no `forks:` field) restricts to same-repo PRs only.
# `forks: ["*"]` allows fork PRs to be reviewed; the trust gate is the label
# itself, which only write-access maintainers can apply.
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

The header comment says the workflow "only runs when a maintainer applies panel-review", but the trigger includes pull_request.synchronize. In the compiled workflow, synchronize currently runs at least the pre_activation job even when the PR is not labeled. Either tighten the synchronize path to require the label (recommended, especially now that forks are allowed) or update the comment to match the actual behavior.

See below for a potential fix:

# public repos). A maintainer applying `panel-review` is the intent gate for
# the actual panel review, but the workflow also listens to `synchronize`.
# That means new pushes can start the workflow even before the label is
# present. `forks: ["*"]` allows fork PRs to be reviewed; the trust gate for
# the review itself remains the label, which only write-access maintainers can
# apply.

Copilot uses AI. Check for mistakes.
@danielmeppiel danielmeppiel merged commit 6b3a79d into main Apr 21, 2026
19 checks passed
@danielmeppiel danielmeppiel deleted the fix/pr-review-panel-allow-forks branch April 21, 2026 23:42
danielmeppiel added a commit that referenced this pull request Apr 22, 2026
- Bump version to 0.9.1 in pyproject.toml and uv.lock
- Move CHANGELOG [Unreleased] entries into [0.9.1] - 2026-04-22
- Audit pass: 1 PR = 1 entry, no bloat
  - Consolidate the seven scattered entries from #832 into a single
    Added line that names the closed issues (#827/#829/#831/#834)
    and keeps the migration warning inline
  - Combine the three fork-PR fixes (#826, #836, #837) into one
    Fixed entry per the multi-PR convention
  - Drop the doc-only consolidation commits from the log

Highlights of 0.9.1:
- Install-time enforcement of org apm-policy.yml (#832)
- pr-review-panel automation now usable from fork PRs (#824, #826,
  #836, #837)
- Docs site publish gated on stable releases only (#822)
- Repository dogfooding via .apm/ as primitive source of truth (#823)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel added a commit that referenced this pull request Apr 22, 2026
- Bump version to 0.9.1 in pyproject.toml and uv.lock
- Move CHANGELOG [Unreleased] entries into [0.9.1] - 2026-04-22
- Audit pass: 1 PR = 1 entry, no bloat
  - Consolidate the seven scattered entries from #832 into a single
    Added line that names the closed issues (#827/#829/#831/#834)
    and keeps the migration warning inline
  - Combine the three fork-PR fixes (#826, #836, #837) into one
    Fixed entry per the multi-PR convention
  - Drop the doc-only consolidation commits from the log

Highlights of 0.9.1:
- Install-time enforcement of org apm-policy.yml (#832)
- pr-review-panel automation now usable from fork PRs (#824, #826,
  #836, #837)
- Docs site publish gated on stable releases only (#822)
- Repository dogfooding via .apm/ as primitive source of truth (#823)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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.

2 participants