Fix pull_request_reviewer double-trigger on comment events

.github/workflows/agentic_commands.yml

@@ -7,20 +7,20 @@
 #     /brave -> brave [issue_comment] reaction=eyes
 #     /cloclo -> cloclo [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /craft -> craft [issues] reaction=eyes
-#     /design-decision-gate -> design-decision-gate [pull_request_comment,pull_request_review_comment] reaction=eyes
+#     /design-decision-gate -> design-decision-gate [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /grumpy -> grumpy-reviewer [pull_request_comment,pull_request_review_comment] reaction=eyes
-#     /matt -> mattpocock-skills-reviewer [pull_request_comment,pull_request_review_comment] reaction=eyes
+#     /matt -> mattpocock-skills-reviewer [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /mergefest -> mergefest [pull_request_comment] reaction=eyes
 #     /nit -> pr-nitpick-reviewer [pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /plan -> plan [discussion_comment,issue_comment] reaction=eyes
 #     /poem-bot -> poem-bot [issues] reaction=eyes
-#     /review -> design-decision-gate [pull_request_comment,pull_request_review_comment] reaction=eyes
-#     /review -> pr-code-quality-reviewer [pull_request_comment,pull_request_review_comment] reaction=eyes
-#     /review -> test-quality-sentinel [pull_request_comment,pull_request_review_comment] reaction=eyes
+#     /review -> design-decision-gate [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
+#     /review -> pr-code-quality-reviewer [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
+#     /review -> test-quality-sentinel [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /scout -> scout [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /security-review -> security-review [pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /summarize -> pdf-summary [issue_comment,issues] reaction=eyes
-#     /test-quality-sentinel -> test-quality-sentinel [pull_request_comment,pull_request_review_comment] reaction=eyes
+#     /test-quality-sentinel -> test-quality-sentinel [discussion,discussion_comment,issue_comment,issues,pull_request,pull_request_comment,pull_request_review_comment] reaction=eyes
 #     /tidy -> tidy [pull_request_comment] reaction=eyes
 #     /unbloat -> unbloat-docs [pull_request_comment] reaction=eyes
 #   labels:
@@ -92,7 +92,7 @@ jobs:
       - name: Route slash command
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
         env:
-          GH_AW_SLASH_ROUTING: '{"ace":[{"workflow":"ace-editor","events":["pull_request_comment"],"ai_reaction":"eyes"}],"approach-validator":[{"workflow":"approach-validator","events":["issue_comment","pull_request_comment"],"ai_reaction":"eyes"}],"archie":[{"workflow":"archie","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes"}],"brave":[{"workflow":"brave","events":["issue_comment"],"ai_reaction":"eyes"}],"cloclo":[{"workflow":"cloclo","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"craft":[{"workflow":"craft","events":["issues"],"ai_reaction":"eyes"}],"design-decision-gate":[{"workflow":"design-decision-gate","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"grumpy":[{"workflow":"grumpy-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"matt":[{"workflow":"mattpocock-skills-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"mergefest":[{"workflow":"mergefest","events":["pull_request_comment"],"ai_reaction":"eyes"}],"nit":[{"workflow":"pr-nitpick-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"plan":[{"workflow":"plan","events":["discussion_comment","issue_comment"],"ai_reaction":"eyes"}],"poem-bot":[{"workflow":"poem-bot","events":["issues"],"ai_reaction":"eyes"}],"review":[{"workflow":"design-decision-gate","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"},{"workflow":"pr-code-quality-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"},{"workflow":"test-quality-sentinel","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"scout":[{"workflow":"scout","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"security-review":[{"workflow":"security-review","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"summarize":[{"workflow":"pdf-summary","events":["issue_comment","issues"],"ai_reaction":"eyes"}],"test-quality-sentinel":[{"workflow":"test-quality-sentinel","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"tidy":[{"workflow":"tidy","events":["pull_request_comment"],"ai_reaction":"eyes"}],"unbloat":[{"workflow":"unbloat-docs","events":["pull_request_comment"],"ai_reaction":"eyes"}]}'
+          GH_AW_SLASH_ROUTING: '{"ace":[{"workflow":"ace-editor","events":["pull_request_comment"],"ai_reaction":"eyes"}],"approach-validator":[{"workflow":"approach-validator","events":["issue_comment","pull_request_comment"],"ai_reaction":"eyes"}],"archie":[{"workflow":"archie","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes"}],"brave":[{"workflow":"brave","events":["issue_comment"],"ai_reaction":"eyes"}],"cloclo":[{"workflow":"cloclo","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"craft":[{"workflow":"craft","events":["issues"],"ai_reaction":"eyes"}],"design-decision-gate":[{"workflow":"design-decision-gate","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"grumpy":[{"workflow":"grumpy-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"matt":[{"workflow":"mattpocock-skills-reviewer","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"mergefest":[{"workflow":"mergefest","events":["pull_request_comment"],"ai_reaction":"eyes"}],"nit":[{"workflow":"pr-nitpick-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"plan":[{"workflow":"plan","events":["discussion_comment","issue_comment"],"ai_reaction":"eyes"}],"poem-bot":[{"workflow":"poem-bot","events":["issues"],"ai_reaction":"eyes"}],"review":[{"workflow":"design-decision-gate","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"},{"workflow":"pr-code-quality-reviewer","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"},{"workflow":"test-quality-sentinel","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"scout":[{"workflow":"scout","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"security-review":[{"workflow":"security-review","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"summarize":[{"workflow":"pdf-summary","events":["issue_comment","issues"],"ai_reaction":"eyes"}],"test-quality-sentinel":[{"workflow":"test-quality-sentinel","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes"}],"tidy":[{"workflow":"tidy","events":["pull_request_comment"],"ai_reaction":"eyes"}],"unbloat":[{"workflow":"unbloat-docs","events":["pull_request_comment"],"ai_reaction":"eyes"}]}'
           GH_AW_LABEL_ROUTING: '{"approach-proposal":[{"workflow":"approach-validator","events":["issues","pull_request"],"ai_reaction":"eyes"}],"ci-doctor":[{"workflow":"ci-doctor","events":["pull_request"],"ai_reaction":"eyes"}],"cloclo":[{"workflow":"cloclo","events":["discussion","issues","pull_request"],"ai_reaction":"eyes"}],"dev":[{"workflow":"dev","events":["discussion","issues","pull_request"],"ai_reaction":"eyes"}],"necromancer":[{"workflow":"necromancer","events":["pull_request"],"ai_reaction":"eyes"}],"needs-design":[{"workflow":"approach-validator","events":["issues","pull_request"],"ai_reaction":"eyes"}]}'
         with:
           script: |

pkg/workflow/central_slash_command_workflow.go

@@ -115,6 +115,13 @@ func collectCentralSlashCommandRoutes(workflowDataList []*WorkflowData) (map[str
 			continue
 		}
 
+		// PullRequestReviewer workflows are driven by PR lifecycle events only.
+		// They must not be enrolled in comment-based slash-command routing to
+		// prevent accidental double-triggers when comments are posted.
+		if wd.PullRequestReviewer {
+			continue
+		}
+
 		filteredEvents := FilterCommentEvents(wd.CommandEvents)
 		if len(filteredEvents) == 0 {
 			continue

pkg/workflow/central_slash_command_workflow_test.go

@@ -271,6 +271,41 @@ func TestCollectCentralSlashCommandRoutes_RespectsReactionEventTargets(t *testin
 	require.Equal(t, []string{"|issue_comment"}, routeReactions["none-reaction"])
 }
 
+func TestCollectCentralSlashCommandRoutes_ExcludesPullRequestReviewerWorkflows(t *testing.T) {
+	data := []*WorkflowData{
+		{
+			WorkflowID:          "regular-slash-command",
+			Command:             []string{"review"},
+			CommandEvents:       []string{"issue_comment"},
+			CommandCentralized:  true,
+			PullRequestReviewer: false,
+		},
+		{
+			WorkflowID:  "reviewer-workflow",
+			Command:     []string{"review"},
+			CommandEvents: nil, // nil CommandEvents would normally mean all events in FilterCommentEvents,
+			// but this workflow is excluded from routing by PullRequestReviewer=true
+			// before FilterCommentEvents is called.
+			CommandCentralized:  true,
+			PullRequestReviewer: true,
+		},
+	}
+
+	routesByCommand, mergedEvents := collectCentralSlashCommandRoutes(data)
+
+	// Only the regular slash-command workflow should be registered; the reviewer
+	// workflow must be absent even though CommandEvents is nil (which FilterCommentEvents
+	// would normally treat as "all events").
+	require.Len(t, routesByCommand["review"], 1)
+	require.Equal(t, "regular-slash-command", routesByCommand["review"][0].Workflow)
+	for _, route := range routesByCommand["review"] {
+		require.NotEqual(t, "reviewer-workflow", route.Workflow,
+			"reviewer-workflow must not appear in centralized slash-command routes")
+	}
+	// The reviewer workflow must not contribute any events to the merged event set.
+	require.NotContains(t, mergedEvents, "pull_request_review")
+}
+
 func TestGenerateCentralSlashCommandWorkflow_UsesCentralizedRunsOnResolution(t *testing.T) {
 	tmpDir := testutil.TempDir(t, "central-slash-workflow-runs-on-test")
 	data := []*WorkflowData{

pkg/workflow/compiler_safe_outputs.go

@@ -283,7 +283,11 @@ func (c *Compiler) parseOnSection(frontmatter map[string]any, workflowData *Work
 				workflowData.PullRequestReviewer = true
 				workflowData.CommandCentralized = true
 				workflowData.Command = resolvePullRequestReviewerCommandNames(reviewerValue, workflowData, markdownPath)
-				workflowData.CommandEvents = []string{"pull_request_comment", "pull_request_review_comment"}
+				// Reviewer workflows are driven by PR lifecycle events only.
+				// CommandEvents is left nil here; the actual exclusion from
+				// comment-based slash-command routing is enforced in
+				// collectCentralSlashCommandRoutes via the PullRequestReviewer flag.
+				workflowData.CommandEvents = nil
 				// Populate CommandOtherEvents with reviewer lifecycle events
 				workflowData.CommandOtherEvents = map[string]any{
 					"pull_request": map[string]any{

pkg/workflow/compiler_safe_outputs_test.go

@@ -495,8 +495,12 @@ func TestParseOnSection_PullRequestReviewerDefaultsToReviewAndWorkflowID(t *test
 	err := c.parseOnSection(frontmatter, workflowData, "/path/to/reviewer.md")
 	require.NoError(t, err)
 	assert.Equal(t, []string{"review", "reviewer-workflow-id"}, workflowData.Command)
-	assert.Equal(t, []string{"pull_request_comment", "pull_request_review_comment"}, workflowData.CommandEvents)
+	// CommandEvents is nil for reviewer workflows; the actual exclusion from
+	// comment-based routing is enforced via the PullRequestReviewer flag in
+	// collectCentralSlashCommandRoutes.
+	assert.Nil(t, workflowData.CommandEvents)
 	assert.True(t, workflowData.CommandCentralized)
+	assert.True(t, workflowData.PullRequestReviewer)
 }
 
 func TestParseOnSection_PullRequestReviewerOwnsCommandAndEvents(t *testing.T) {
@@ -520,8 +524,12 @@ func TestParseOnSection_PullRequestReviewerOwnsCommandAndEvents(t *testing.T) {
 	err := c.parseOnSection(frontmatter, workflowData, "/path/to/reviewer.md")
 	require.NoError(t, err)
 	assert.Equal(t, []string{"review", "reviewer-workflow-id"}, workflowData.Command)
-	assert.Equal(t, []string{"pull_request_comment", "pull_request_review_comment"}, workflowData.CommandEvents)
+	// CommandEvents is nil for reviewer workflows; the actual exclusion from
+	// comment-based routing is enforced via the PullRequestReviewer flag in
+	// collectCentralSlashCommandRoutes.
+	assert.Nil(t, workflowData.CommandEvents)
 	assert.True(t, workflowData.CommandCentralized)
+	assert.True(t, workflowData.PullRequestReviewer)
 }
 
 func TestParseOnSection_PullRequestReviewerDoesNotInjectLifecycleTriggers(t *testing.T) {