Skip to content

Route centralized command dispatches to the triggering PR branch#37187

Open
Copilot wants to merge 7 commits into
mainfrom
copilot/update-compiler-agentic-commands-runtime
Open

Route centralized command dispatches to the triggering PR branch#37187
Copilot wants to merge 7 commits into
mainfrom
copilot/update-compiler-agentic-commands-runtime

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 5, 2026

Centralized agentic_commands dispatch was triggering decentralized slash/label workflows on router context refs instead of the event’s branch context. This change makes dispatch branch-aware, including issue-comment and label events that originate from a PR thread.

  • Runtime: branch-aware dispatch ref resolution

    • route_slash_command.cjs now resolves dispatch refs in PR-first order:
      1. GITHUB_HEAD_REF
      2. payload.pull_request.head.ref
      3. issue-backed PR lookup via pulls.get (for issues / issue_comment on PRs)
      4. existing fallback refs/default branch
    • Added ref normalization so branch names and full refs/* inputs are handled consistently.
  • Runtime: issue-backed PR handling

    • For issue events that are actually PR-backed, router now fetches the PR head ref and dispatches to that branch, so slash/label workflows run against the correct PR code state.
  • Compiler: router permission alignment

    • central_slash_command_workflow.go now emits pull-requests: write when merged events include issue/issue_comment paths that may require PR head resolution.
    • Refactored permission decision into a focused helper (needsPullRequestsPermission) for readability and maintenance.
  • Compiler: workflow_dispatch PR-checkout condition alignment

    • pr.go now treats relayed workflow_dispatch runs as PR-checkout eligible when aw_context indicates an issue_comment event on a pull_request item.
    • This ensures the existing “Checkout PR branch” step still runs for dispatched issue-comment PR contexts.
  • Coverage updates

    • Extended JS routing tests with branch-ref assertions for:
      • slash command on issue_comment over PR
      • label routing on issue-backed PR label events
    • Added Go workflow generation coverage for pull-request permission emission on issue_comment routes.
    • Added unit coverage for the compiler-generated PR checkout condition with workflow_dispatch + aw_context issue-comment PR context.
// route_slash_command.cjs (simplified)
if (context.payload?.issue?.pull_request) {
  const pr = await github.rest.pulls.get({ owner, repo, pull_number });
  return `refs/heads/${pr.data.head.ref}`;
}

Copilot AI and others added 3 commits June 5, 2026 20:07
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Route decentralized command dispatches to triggering branch Route centralized command dispatches to the triggering PR branch Jun 5, 2026
Copilot AI requested a review from pelikhan June 5, 2026 20:12
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 5, 2026

/scout do a security review on this change

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🔭 Recon complete! Scout has charted the territory. Map ready! 🗺️

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

🔍 Scout Research Report

Triggered by @github-actions[bot]

Executive Summary

This PR resolves a well-known GitHub Actions limitation: issue_comment (and issues/label) events always run in the context of the default branch, never the PR head branch. The community consensus — across GitHub Docs, Security Lab, and multiple discussions — strongly validates the PR's core approach: when an issue event is PR-backed, call pulls.get to resolve head.ref and dispatch against that branch. The PR's PR-first resolution order (GITHUB_HEAD_REFpayload.pull_request.head.ref → issue-backed pulls.get → fallback) matches the recommended pattern almost exactly.

Research surfaced two correctness gotchas and one security consideration worth confirming the PR handles. They are summarized below the fold.

Click to expand detailed findings

Research Findings

1. The root cause is confirmed and intentional GitHub behavior

The issue_comment event "occurs for comments on both issues and pull requests" but runs in the context of the default branch, not the PR branch (GitHub Docs — events). There is a long-standing open feature request to add a dedicated pull_request_comment event or expose head/base ref directly (community #59389, #49628). The PR's pulls.get lookup is the canonical workaround; community examples use the identical call:

const pr = await github.rest.pulls.get({ owner, repo, pull_number: context.payload.issue.number });
// dispatch ref: pr.data.head.ref

2. ⚠️ Gotcha — workflow_dispatch requires the workflow file to exist on BOTH the target ref AND the default branch

This is the highest-risk edge case for branch-aware dispatch. A createWorkflowDispatch call fails if:

  • the workflow .yml does not exist on the repo's default branchHTTP 404: workflow X not found on the default branch ([SO 76461307]((stackoverflow.com/redacted), and
  • the workflow on the dispatched ref lacks a workflow_dispatch trigger → Workflow does not have 'workflow_dispatch' trigger (community #24657).

Implication for this PR: when dispatching to a PR head branch, if that PR itself modifies or adds the decentralized slash/label workflow file, the dispatched run executes the head-branch version of the workflow definition. Worth confirming tests cover: (a) PR that edits the target workflow, (b) PR whose head branch is missing the workflow entirely.

3. ⚠️ Gotcha — fork PR head refs are not branches in the base repo

pr.data.head.ref returns the bare branch name (e.g. patch-1), but for a fork PR that branch lives in the contributor's fork, not the base repo. Dispatching refs/heads/patch-1 against the base repo will 404 (or, worse, silently match an unrelated same-named base-repo branch). The PR description's fallback chain handles the no-PR case; recommend verifying the fork path explicitly (pr.data.head.repo.full_name !== base repo → fall back to default branch rather than dispatching a non-existent ref).

4. 🔒 Security — pwn request exposure when running PR head code with elevated permissions

The PR adds pull-requests: write to the router. GitHub Security Lab warns that dispatching workflows that check out and run untrusted PR head code with write permissions/secrets is the classic "pwn request" pattern (Preventing pwn requests, [Arctiq pitfalls]((arctiq.com/redacted) The router permission bump itself is low-risk (it only reads the PR to resolve a ref). The risk lives in the dispatched workflow if it both runs head code and holds write scope — keep the privileged router and the code-executing workflow separated (which this architecture already does via dispatch).

5. GITHUB_HEAD_REF is only populated on pull_request/pull_request_target

Confirms the PR's resolution ordering is correct: GITHUB_HEAD_REF is empty for issue_comment, issues, label, and pull_request_review events (Contexts reference, community #25773), so falling through to pulls.get for issue-backed events is necessary, not redundant.

Recommendations

  • ✅ Approach is sound and matches community/GitHub-recommended patterns — proceed.
  • Add/confirm a test for the fork PR case (head ref not present in base repo) so dispatch falls back gracefully instead of 404-ing.
  • Add/confirm a test for a PR that modifies the dispatched workflow file itself, since the head-branch definition is what executes.
  • Document (in the workflow or PR) that the decentralized target workflow must keep a workflow_dispatch trigger and remain present on the default branch, or dispatch silently breaks.
  • Keep the privileged router (pull-requests: write) strictly separate from any step that executes untrusted PR head code.

Key Sources

Suggested Next Steps

  1. Verify fork-PR dispatch behavior in JS routing tests.
  2. Verify dispatch when the PR edits the target workflow file.
  3. Note the default-branch + workflow_dispatch-trigger preconditions in maintainer docs.

🔭 Intelligence gathered by Scout · opus48 267.3K · agent 80.2 AIC · threat-detection 12.6 AIC ·

@github-actions github-actions Bot added the scout label Jun 5, 2026
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 5, 2026

@copilot the compiler already includes a step to checkout the pull-request branch. For issue_comment workflows, it needs to update the expression to support workflow_dispatch with a specific branch provided in the context.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 6, 2026

@copilot the compiler already includes a step to checkout the pull-request branch. For issue_comment workflows, it needs to update the expression to support workflow_dispatch with a specific branch pr...

Updated in f4c8810. The compiler now includes workflow_dispatch + aw_context(issue_comment + pull_request item_type) in the PR checkout step condition, and I added a unit test to cover the generated expression.

@pelikhan pelikhan marked this pull request as ready for review June 6, 2026 01:09
Copilot AI review requested due to automatic review settings June 6, 2026 01:09
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

🧠 Matt Pocock Skills Reviewer failed during the skills-based review.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

Test Quality Sentinel failed during test quality analysis.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

⚠️ PR Code Quality Reviewer failed during code quality review.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

Design Decision Gate 🏗️ failed during design decision gate check.

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

This PR fixes centralized agentic_commands workflow dispatch so decentralized slash/label workflows run against the triggering PR’s head branch (instead of the router’s ref), including issue/label events that originate from PR-backed issues.

Changes:

  • Updated JS router ref resolution to prefer PR head refs (env GITHUB_HEAD_REF, PR payload head ref, then PR lookup via pulls.get for issue-backed PR events) and normalized refs consistently.
  • Updated workflow compiler logic to treat dispatched workflow_dispatch runs with aw_context indicating issue_comment on a pull_request as eligible for “Checkout PR branch”.
  • Updated central router workflow permission emission to include pull-requests: write when issue/issue_comment routes may require PR head resolution, plus added/extended unit tests.
Show a summary per file
File Description
pkg/workflow/pr.go Extends PR checkout step condition to include dispatched issue_comment PR contexts via aw_context.
pkg/workflow/pr_test.go Adds coverage asserting the new workflow_dispatch + aw_context PR-checkout condition is emitted.
pkg/workflow/central_slash_command_workflow.go Refactors PR permission decision into needsPullRequestsPermission, expanding to issue/issue_comment-backed PR cases.
pkg/workflow/central_slash_command_workflow_test.go Adds coverage ensuring issue_comment centralized routes emit pull-requests: write.
actions/setup/js/route_slash_command.cjs Makes dispatch ref resolution branch-aware (PR-first ordering + issue-backed PR lookup) and normalizes refs.
actions/setup/js/route_slash_command.test.cjs Adds assertions that issue-comment and label events on PR-backed issues dispatch to the PR head branch.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 6/6 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

@copilot review all comments and fix the failing test/build-wasm jobs before re-evaluating.

Generated by 👨‍🍳 PR Sous Chef · agent 45.4 AIC · threat-detection 4.79 AIC ·

@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 6, 2026

@copilot merge main and recompile

Copilot AI and others added 2 commits June 6, 2026 03:41
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 6, 2026

@copilot merge main and recompile

Done in 6c9af1d: merged origin/main into this branch, resolved conflicts, and ran make recompile.

@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 6, 2026

@copilot merge main and recompile

…er-agentic-commands-runtime

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 6, 2026

@copilot merge main and recompile

Done in 9262185: merged latest origin/main and ran make recompile.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 6, 2026

@copilot review all comments and address unresolved review feedback.

Please refresh the branch and summarize any remaining blockers.

Generated by 👨‍🍳 PR Sous Chef · 43.2 AIC · ⌖ 4.94 AIC ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants