Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions .github/workflows/agentic_commands.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
# necromancer -> necromancer [pull_request] reaction=eyes
# needs-design -> approach-validator [issues,pull_request] reaction=eyes
# pull-request reviewers:
# design-decision-gate [pull_request,pull_request_review]
# mattpocock-skills-reviewer [pull_request,pull_request_review]
# pr-code-quality-reviewer [pull_request,pull_request_review]
# test-quality-sentinel [pull_request,pull_request_review]
# design-decision-gate [pull_request]
# mattpocock-skills-reviewer [pull_request]
# pr-code-quality-reviewer [pull_request]
# test-quality-sentinel [pull_request]
# ___ _ _
# / _ \ | | (_)
# | |_| | __ _ ___ _ __ | |_ _ ___
Expand Down Expand Up @@ -64,8 +64,6 @@ on:
types: [created, edited]
pull_request:
types: [edited, labeled, opened, ready_for_review, reopened, review_requested]
pull_request_review:
types: [submitted]
pull_request_review_comment:
Comment on lines 65 to 67
types: [created, edited]
discussion:
Expand Down Expand Up @@ -99,7 +97,7 @@ jobs:
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":"pr-code-quality-reviewer","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_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"}]}'
GH_AW_REVIEWER_ROUTING: '[{"workflow":"design-decision-gate","events":["pull_request","pull_request_review"]},{"workflow":"mattpocock-skills-reviewer","events":["pull_request","pull_request_review"]},{"workflow":"pr-code-quality-reviewer","events":["pull_request","pull_request_review"]},{"workflow":"test-quality-sentinel","events":["pull_request","pull_request_review"]}]'
GH_AW_REVIEWER_ROUTING: '[{"workflow":"design-decision-gate","events":["pull_request"]},{"workflow":"mattpocock-skills-reviewer","events":["pull_request"]},{"workflow":"pr-code-quality-reviewer","events":["pull_request"]},{"workflow":"test-quality-sentinel","events":["pull_request"]}]'
with:
script: |
const { setupGlobals } = require('${{ runner.temp }}/gh-aw/actions/setup_globals.cjs');
Expand Down
22 changes: 3 additions & 19 deletions actions/setup/js/route_slash_command.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
/// <reference types="@actions/github-script" />

const { REACTION_MAP } = require("./add_reaction.cjs");
const { extractWorkflowId } = require("./generate_footer.cjs");
// Keep this aligned with the current default stable GitHub REST API version used by workflows.
// Update when GitHub advances the recommended version to avoid sunset/deprecation warnings.
const GITHUB_API_VERSION = "2022-11-28";
Expand Down Expand Up @@ -272,27 +271,12 @@ async function main() {
return;
}

if ((context.eventName === "pull_request" || context.eventName === "pull_request_review") && Array.isArray(reviewerRoutes) && reviewerRoutes.length > 0) {
if (context.eventName === "pull_request" && Array.isArray(reviewerRoutes) && reviewerRoutes.length > 0) {
const matches = reviewerRoutes.filter(route => Array.isArray(route.events) && route.events.includes(context.eventName));
if (matches.length > 0) {
let selected = matches;
if (context.eventName === "pull_request") {
if (!["ready_for_review", "review_requested"].includes(context.payload?.action ?? "")) {
selected = [];
}
} else if (context.eventName === "pull_request_review") {
const action = context.payload?.action ?? "";
if (action !== "submitted") {
selected = [];
} else {
const workflowId = extractWorkflowId(context.payload?.review?.body ?? "");
if (workflowId) {
selected = selected.filter(route => route.workflow === workflowId);
} else {
selected = [];
core.info("No workflow marker found in pull request review body; skipping reviewer dispatch.");
}
}
if (!["ready_for_review", "review_requested"].includes(context.payload?.action ?? "")) {
selected = [];
}
if (selected.length > 0) {
core.info(`Matched reviewer routes on '${context.eventName}': ${selected.map(route => route.workflow).join(", ")}.`);
Expand Down
33 changes: 6 additions & 27 deletions actions/setup/js/route_slash_command.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ describe("route_slash_command", () => {
it("dispatches reviewer lifecycle routes on pull_request ready_for_review", async () => {
globals.context.eventName = "pull_request";
globals.context.payload = { action: "ready_for_review", pull_request: { number: 12, state: "open" } };
process.env.GH_AW_REVIEWER_ROUTING = JSON.stringify([{ workflow: "pr-reviewer", events: ["pull_request", "pull_request_review"] }]);
process.env.GH_AW_REVIEWER_ROUTING = JSON.stringify([{ workflow: "pr-reviewer", events: ["pull_request"] }]);

await main();

Expand All @@ -242,7 +242,7 @@ describe("route_slash_command", () => {
it("dispatches reviewer lifecycle routes on pull_request review_requested", async () => {
globals.context.eventName = "pull_request";
globals.context.payload = { action: "review_requested", pull_request: { number: 12, state: "open" } };
process.env.GH_AW_REVIEWER_ROUTING = JSON.stringify([{ workflow: "pr-reviewer", events: ["pull_request", "pull_request_review"] }]);
process.env.GH_AW_REVIEWER_ROUTING = JSON.stringify([{ workflow: "pr-reviewer", events: ["pull_request"] }]);

await main();

Expand All @@ -264,35 +264,14 @@ describe("route_slash_command", () => {
expect(globals.core.info).toHaveBeenCalledWith(expect.stringContaining("Pull request is closed at workflow start"));
});

it("routes pull_request_review using workflow marker in review body", async () => {
globals.context.eventName = "pull_request_review";
globals.context.payload = {
action: "submitted",
pull_request: { number: 9, state: "open" },
review: { body: "Looks good\n<!-- gh-aw-workflow-id: pr-reviewer -->" },
};
process.env.GH_AW_REVIEWER_ROUTING = JSON.stringify([
{ workflow: "pr-reviewer", events: ["pull_request_review"] },
{ workflow: "other-reviewer", events: ["pull_request_review"] },
]);

await main();

expect(dispatchCalls).toHaveLength(1);
expect(dispatchCalls[0].workflow_id).toBe("pr-reviewer.lock.yml");
const awContext = JSON.parse(dispatchCalls[0].inputs.aw_context);
expect(awContext.reviewer_lifecycle_event).toBe("pull_request_review");
});

it("ignores pull_request_review edited and dismissed actions", async () => {
process.env.GH_AW_REVIEWER_ROUTING = JSON.stringify([{ workflow: "pr-reviewer", events: ["pull_request_review"] }]);
for (const action of ["edited", "dismissed"]) {
it("ignores pull_request actions outside reviewer lifecycle", async () => {
process.env.GH_AW_REVIEWER_ROUTING = JSON.stringify([{ workflow: "pr-reviewer", events: ["pull_request"] }]);
for (const action of ["opened", "synchronize"]) {
dispatchCalls.length = 0;
globals.context.eventName = "pull_request_review";
globals.context.eventName = "pull_request";
globals.context.payload = {
action,
pull_request: { number: 9, state: "open" },
review: { body: "Looks good\n<!-- gh-aw-workflow-id: pr-reviewer -->" },
};
await main();
expect(dispatchCalls).toHaveLength(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1696,8 +1696,7 @@ on:
- **PRR3**: Built-in slash command behavior for `on.pull_request_reviewer` MUST always be enabled and MUST NOT be replaced by a separate `on.slash_command` trigger definition. If both fields are present, reviewer-trigger command name and reviewer-trigger event set MUST take precedence.
- **PRR4**: Implementations MUST subscribe reviewer lifecycle routing to:
- `pull_request` actions `ready_for_review` and `review_requested`
- `pull_request_review` action `submitted` only
- **PRR5**: Implementations MUST ignore `pull_request_review` actions other than `submitted` (including `edited` and `dismissed`) for reviewer lifecycle routing.
- **PRR5**: Implementations MUST ignore `pull_request` actions outside reviewer lifecycle routing (for example `opened`, `edited`, `synchronize`, and `closed`).
- **PRR6**: When multiple `pull_request_reviewer` workflows are configured, slash command names MUST be unique case-insensitively.
- **PRR7**: Duplicate reviewer slash command names MUST fail compilation with an explicit validation error.

Expand Down
6 changes: 1 addition & 5 deletions pkg/workflow/central_slash_command_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,17 +260,13 @@ func collectPullRequestReviewerRoutes(workflowDataList []*WorkflowData, mergedEv
}
routes = append(routes, reviewerLifecycleRoute{
Workflow: wd.WorkflowID,
Events: []string{"pull_request", "pull_request_review"},
Events: []string{"pull_request"},
})
if mergedEvents["pull_request"] == nil {
mergedEvents["pull_request"] = make(map[string]bool)
}
mergedEvents["pull_request"]["ready_for_review"] = true
mergedEvents["pull_request"]["review_requested"] = true
if mergedEvents["pull_request_review"] == nil {
mergedEvents["pull_request_review"] = make(map[string]bool)
}
mergedEvents["pull_request_review"]["submitted"] = true
}
sort.Slice(routes, func(i, j int) bool {
return routes[i].Workflow < routes[j].Workflow
Expand Down
7 changes: 3 additions & 4 deletions pkg/workflow/central_slash_command_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,12 @@ func TestGenerateCentralSlashCommandWorkflow_IncludesPullRequestReviewerRoutes(t
require.NoError(t, err)
text := string(content)
require.Contains(t, text, "GH_AW_REVIEWER_ROUTING")
require.Contains(t, text, `"workflow":"pr-reviewer","events":["pull_request","pull_request_review"]`)
require.Contains(t, text, "pull_request_review:")
require.Contains(t, text, "types: [submitted]")
require.Contains(t, text, `"workflow":"pr-reviewer","events":["pull_request"]`)
require.NotContains(t, text, "pull_request_review:")
require.Contains(t, text, "ready_for_review")
require.Contains(t, text, "review_requested")
require.Contains(t, text, "# pull-request reviewers:")
require.Contains(t, text, "# pr-reviewer [pull_request,pull_request_review]")
require.Contains(t, text, "# pr-reviewer [pull_request]")
}

func TestGenerateCentralSlashCommandWorkflow_InfersReviewerCommandFromWorkflowIDWhenMissing(t *testing.T) {
Expand Down
3 changes: 0 additions & 3 deletions pkg/workflow/compiler_safe_outputs.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,6 @@ func (c *Compiler) parseOnSection(frontmatter map[string]any, workflowData *Work
"pull_request": map[string]any{
"types": []string{"ready_for_review", "review_requested"},
},
"pull_request_review": map[string]any{
"types": []string{"submitted"},
},
}
Comment on lines 281 to 285
if _, hasConcurrency := frontmatter["concurrency"]; !hasConcurrency && workflowData.Concurrency == "" {
workflowData.Concurrency = "concurrency:\n group: \"gh-aw-${{ github.workflow }}-${{ github.event.pull_request.number || github.event.issue.number || github.run_id }}-all-reviewers\"\n queue: max"
Expand Down
Loading