Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions actions/setup/js/safe_output_handler_manager.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const HANDLER_MAP = {
close_discussion: "./close_discussion.cjs",
add_labels: "./add_labels.cjs",
remove_labels: "./remove_labels.cjs",
replace_label: "./replace_label.cjs",
update_issue: "./update_issue.cjs",
update_discussion: "./update_discussion.cjs",
link_sub_issue: "./link_sub_issue.cjs",
Expand Down Expand Up @@ -198,6 +199,7 @@ const THREAT_WARNING_ABORT_TYPES = new Set([
"add_labels",
"jira_add_label",
"remove_labels",
"replace_label",
"add_reviewer",
"assign_milestone",
"assign_to_agent",
Expand Down
40 changes: 40 additions & 0 deletions actions/setup/js/safe_output_handler_manager.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2371,6 +2371,7 @@ describe("Safe Output Handler Manager", () => {

const handlerFiles = [...handlerMapBlock[1].matchAll(/["'](\.\/[^"']+\.cjs)["']/g)].map(match => match[1].slice(2));
expect(handlerFiles).toContain("approve_workflow_run.cjs");
expect(handlerFiles).toContain("replace_label.cjs");

const builtinModules = new Set(require("module").builtinModules);
const visited = new Set();
Expand Down Expand Up @@ -2430,6 +2431,45 @@ describe("Safe Output Handler Manager", () => {
});
});

describe("replace_label handler registration", () => {
// Regression test for the replace-label portion of
// https://github.com/github/gh-aw/issues/54811: replace_label had a
// dedicated handler module (replace_label.cjs) but was missing from
// HANDLER_MAP, so the collect job never loaded it and label
// transitions silently did nothing even though the sample/agent output
// was recorded successfully.
it("processes replace_label messages without no-handler warnings when handler is registered", async () => {
const messages = [{ type: "replace_label", item_number: 1, label_to_remove: "old", label_to_add: "new" }];
const mockHandler = vi.fn().mockResolvedValue({
success: true,
item_number: 1,
label_to_remove: "old",
label_to_add: "new",
});
const handlers = new Map([["replace_label", mockHandler]]);

const result = await processMessages(handlers, messages);

expect(result.results).toHaveLength(1);
expect(result.results[0].type).toBe("replace_label");
// Handler was invoked, so no "no handler loaded" error
expect(result.results[0].error).toBeUndefined();
expect(mockHandler).toHaveBeenCalledTimes(1);
});

it("records no-handler error for replace_label when handler map is missing entry", async () => {
const messages = [{ type: "replace_label", item_number: 1, label_to_remove: "old", label_to_add: "new" }];
// Empty handler map - simulates the bug where replace_label was not in HANDLER_MAP
const handlers = new Map();

const result = await processMessages(handlers, messages);

expect(result.results).toHaveLength(1);
expect(result.results[0].success).toBe(false);
expect(result.results[0].error).toContain("No handler loaded for type 'replace_label'");
});
});

describe("rollbackReviewResults", () => {
it("flips success:true to success:false for submit_pull_request_review results", () => {
const results = [{ type: "submit_pull_request_review", success: true }];
Expand Down
Loading