Fix pull_request_reviewer double-trigger on comment events

[Copilot]

Workflows using on.pull_request_reviewer were running twice — triggered correctly by PR lifecycle events and again unexpectedly when comments were posted.

Root causes

Inverted if: guard (tools.go): The compiled job guard used a denylist that short-circuited to true for every event outside pull_request/pull_request_review (including workflow_dispatch, pull_request_review_comment):

# Before — denylist: true for ANY non-PR/review event
if: (github.event_name != 'pull_request' && github.event_name != 'pull_request_review') || github.event.pull_request.state != 'closed'

# After — allowlist: only the events this workflow is meant to handle
if: github.event_name == 'workflow_dispatch' || ((github.event_name == 'pull_request' || github.event_name == 'pull_request_review') && github.event.pull_request.state != 'closed')

Reviewer workflows registered in comment routing (compiler_safe_outputs.go): CommandEvents was set to ["pull_request_comment", "pull_request_review_comment"], which enrolled reviewer workflows in the central router's slash-command table. Any comment starting with /review dispatched the workflow a second time via workflow_dispatch. Fixed by setting CommandEvents = nil for reviewer workflows.

Changes

• tools.go — Replace denylist reviewer guard with allowlist
• compiler_safe_outputs.go — Set CommandEvents = nil for pull_request_reviewer workflows
• compiler_safe_outputs_test.go — Update assertions for the now-nil CommandEvents
• pull_request_reviewer_if_guard_test.go — New test asserting the allowlist guard is emitted and the old denylist form is absent

[Copilot]

Pull request overview

Fixes unintended double-triggering of on.pull_request_reviewer workflows by tightening the compiled job if: guard logic and adjusting how reviewer workflows participate in centralized routing.

Changes:

• Replace the reviewer job if: guard from a logically-inverted denylist to an allowlist.
• Stop enrolling pull_request_reviewer workflows in comment-based centralized routing by altering CommandEvents handling.
• Update/add tests to validate the new guard and updated reviewer routing expectations.

Show a summary per file

File	Description
pkg/workflow/tools.go	Updates the compiled reviewer if: guard to an allowlist including workflow_dispatch and open-PR PR/review events.
pkg/workflow/compiler_safe_outputs.go	Changes reviewer workflow parsing to clear CommandEvents to avoid comment routing registration.
pkg/workflow/compiler_safe_outputs_test.go	Updates reviewer trigger tests to assert CommandEvents is now nil.
pkg/workflow/pull_request_reviewer_if_guard_test.go	Adds a regression test asserting the new allowlist guard is emitted and the old denylist form is absent.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

pkg/workflow/compiler_safe_outputs_test.go:529

• Same issue as the earlier reviewer CommandEvents assertion: nil is interpreted elsewhere as “all/default events”, so this test can mask unintended enrollment of reviewer workflows into centralized comment routing. Consider asserting the explicit opt-out representation used to ensure reviewer workflows are not added to GH_AW_SLASH_ROUTING.

	}

	err := c.parseOnSection(frontmatter, workflowData, "/path/to/reviewer.md")
	require.NoError(t, err)
	assert.Equal(t, []string{"review", "reviewer-workflow-id"}, workflowData.Command)
	// pull_request_reviewer takes ownership and clears comment events to prevent
	// the central router from dispatching the workflow on comment triggers.
	assert.Nil(t, workflowData.CommandEvents)
	assert.True(t, workflowData.CommandCentralized)
}

• Files reviewed: 4/4 changed files
• Comments generated: 3

[pelikhan]

@copilot review all comments

[Copilot]

@copilot review all comments

Addressed all three review comments in commit b5f2e3d:

1. compiler_safe_outputs.go:287-290 — The real fix is now in collectCentralSlashCommandRoutes (central_slash_command_workflow.go): an explicit if wd.PullRequestReviewer { continue } guard skips reviewer workflows before FilterCommentEvents is ever called, so the nil-means-all-events behavior can't accidentally enroll them. The comment in compiler_safe_outputs.go now accurately describes this.

2. compiler_safe_outputs_test.go:495-502 — Updated test assertions to document that CommandEvents = nil alone is not the enforcement mechanism, and added assert.True(t, workflowData.PullRequestReviewer) to pin the flag that drives the actual exclusion. A new dedicated test TestCollectCentralSlashCommandRoutes_ExcludesPullRequestReviewerWorkflows verifies that a reviewer workflow with CommandEvents = nil is absent from the centralized routing table even when a regular workflow with the same command name is registered.

3. pull_request_reviewer_if_guard_test.go:18-19 — Rewrote the function comment to correctly explain that the if: guard controls which events run the job, and that comment-event exclusion from routing is enforced via collectCentralSlashCommandRoutes, not via the if: expression.