Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
95 changes: 80 additions & 15 deletions actions/setup/js/safe_output_handler_manager.test.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @ts-check

import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import path from "path";
import fs from "fs";
import { createRequire } from "module";
import {
Expand Down Expand Up @@ -928,23 +929,26 @@ describe("Safe Output Handler Manager", () => {
expect(result.results[1].success).toBe(true);
});

it.each(["set_issue_type", "set_issue_field", "jira_add_label", "dispatch_repository", "call_workflow", "upload_artifact"])("should abort %s in detection warning mode", async messageType => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
const handler = vi.fn().mockResolvedValue({ success: true });
const handlers = new Map([[messageType, handler]]);
const messages = [{ type: messageType }];
it.each(["set_issue_type", "set_issue_field", "jira_add_label", "add_labels", "remove_labels", "replace_label", "dispatch_repository", "call_workflow", "upload_artifact"])(
"should abort %s in detection warning mode",
async messageType => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
const handler = vi.fn().mockResolvedValue({ success: true });
const handlers = new Map([[messageType, handler]]);
const messages = [{ type: messageType }];

const result = await processMessages(handlers, messages);
const result = await processMessages(handlers, messages);

expect(handler).not.toHaveBeenCalled();
expect(result.results[0]).toMatchObject({
type: messageType,
success: false,
cancelled: true,
threatDetected: true,
errorCode: "threat_detected_abort_policy",
});
});
expect(handler).not.toHaveBeenCalled();
expect(result.results[0]).toMatchObject({
type: messageType,
success: false,
cancelled: true,
threatDetected: true,
errorCode: "threat_detected_abort_policy",
});
}
);

it("should log conversion requirement for push_to_pull_request_branch in detection warning mode", async () => {
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
Expand Down Expand Up @@ -2371,6 +2375,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 +2435,66 @@ 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("maps key 'replace_label' directly to './replace_label.cjs' in HANDLER_MAP", () => {
const managerPath = path.join(typeof __dirname !== "undefined" ? __dirname : path.dirname(new URL(import.meta.url).pathname), "safe_output_handler_manager.cjs");
const managerSource = fs.readFileSync(managerPath, "utf8");
const handlerMapBlock = managerSource.match(/const HANDLER_MAP = \{([\s\S]*?)\n\};/);
expect(handlerMapBlock).not.toBeNull();

const pairs = Object.fromEntries([...handlerMapBlock[1].matchAll(/(\w+):\s*["'](\.\/[^"']+\.cjs)["']/g)].map(m => [m[1], m[2]]));
expect(pairs["replace_label"]).toBe("./replace_label.cjs");
});

it("loads replace_label handler from HANDLER_MAP via loadHandlers when enabled in config", async () => {
global.github = {};
try {
const handlers = await loadHandlers({ replace_label: {} });
expect(handlers.has("replace_label")).toBe(true);
expect(typeof handlers.get("replace_label")).toBe("function");
} finally {
delete global.github;
}
});

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