Skip to content

Commit 4b67a21

Browse files
authored
Register replace_label handler in safe-output collect job dispatch map (#58917)
1 parent 70bd59b commit 4b67a21

2 files changed

Lines changed: 82 additions & 15 deletions

File tree

actions/setup/js/safe_output_handler_manager.cjs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ const HANDLER_MAP = {
5252
close_discussion: "./close_discussion.cjs",
5353
add_labels: "./add_labels.cjs",
5454
remove_labels: "./remove_labels.cjs",
55+
replace_label: "./replace_label.cjs",
5556
update_issue: "./update_issue.cjs",
5657
update_discussion: "./update_discussion.cjs",
5758
link_sub_issue: "./link_sub_issue.cjs",
@@ -198,6 +199,7 @@ const THREAT_WARNING_ABORT_TYPES = new Set([
198199
"add_labels",
199200
"jira_add_label",
200201
"remove_labels",
202+
"replace_label",
201203
"add_reviewer",
202204
"assign_milestone",
203205
"assign_to_agent",

actions/setup/js/safe_output_handler_manager.test.cjs

Lines changed: 80 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// @ts-check
22

33
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
4+
import path from "path";
45
import fs from "fs";
56
import { createRequire } from "module";
67
import {
@@ -928,23 +929,26 @@ describe("Safe Output Handler Manager", () => {
928929
expect(result.results[1].success).toBe(true);
929930
});
930931

931-
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 => {
932-
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
933-
const handler = vi.fn().mockResolvedValue({ success: true });
934-
const handlers = new Map([[messageType, handler]]);
935-
const messages = [{ type: messageType }];
932+
it.each(["set_issue_type", "set_issue_field", "jira_add_label", "add_labels", "remove_labels", "replace_label", "dispatch_repository", "call_workflow", "upload_artifact"])(
933+
"should abort %s in detection warning mode",
934+
async messageType => {
935+
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
936+
const handler = vi.fn().mockResolvedValue({ success: true });
937+
const handlers = new Map([[messageType, handler]]);
938+
const messages = [{ type: messageType }];
936939

937-
const result = await processMessages(handlers, messages);
940+
const result = await processMessages(handlers, messages);
938941

939-
expect(handler).not.toHaveBeenCalled();
940-
expect(result.results[0]).toMatchObject({
941-
type: messageType,
942-
success: false,
943-
cancelled: true,
944-
threatDetected: true,
945-
errorCode: "threat_detected_abort_policy",
946-
});
947-
});
942+
expect(handler).not.toHaveBeenCalled();
943+
expect(result.results[0]).toMatchObject({
944+
type: messageType,
945+
success: false,
946+
cancelled: true,
947+
threatDetected: true,
948+
errorCode: "threat_detected_abort_policy",
949+
});
950+
}
951+
);
948952

949953
it("should log conversion requirement for push_to_pull_request_branch in detection warning mode", async () => {
950954
process.env.GH_AW_DETECTION_CONCLUSION = "warning";
@@ -2371,6 +2375,7 @@ describe("Safe Output Handler Manager", () => {
23712375

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

23752380
const builtinModules = new Set(require("module").builtinModules);
23762381
const visited = new Set();
@@ -2430,6 +2435,66 @@ describe("Safe Output Handler Manager", () => {
24302435
});
24312436
});
24322437

2438+
describe("replace_label handler registration", () => {
2439+
// Regression test for the replace-label portion of
2440+
// https://github.com/github/gh-aw/issues/54811: replace_label had a
2441+
// dedicated handler module (replace_label.cjs) but was missing from
2442+
// HANDLER_MAP, so the collect job never loaded it and label
2443+
// transitions silently did nothing even though the sample/agent output
2444+
// was recorded successfully.
2445+
it("maps key 'replace_label' directly to './replace_label.cjs' in HANDLER_MAP", () => {
2446+
const managerPath = path.join(typeof __dirname !== "undefined" ? __dirname : path.dirname(new URL(import.meta.url).pathname), "safe_output_handler_manager.cjs");
2447+
const managerSource = fs.readFileSync(managerPath, "utf8");
2448+
const handlerMapBlock = managerSource.match(/const HANDLER_MAP = \{([\s\S]*?)\n\};/);
2449+
expect(handlerMapBlock).not.toBeNull();
2450+
2451+
const pairs = Object.fromEntries([...handlerMapBlock[1].matchAll(/(\w+):\s*["'](\.\/[^"']+\.cjs)["']/g)].map(m => [m[1], m[2]]));
2452+
expect(pairs["replace_label"]).toBe("./replace_label.cjs");
2453+
});
2454+
2455+
it("loads replace_label handler from HANDLER_MAP via loadHandlers when enabled in config", async () => {
2456+
global.github = {};
2457+
try {
2458+
const handlers = await loadHandlers({ replace_label: {} });
2459+
expect(handlers.has("replace_label")).toBe(true);
2460+
expect(typeof handlers.get("replace_label")).toBe("function");
2461+
} finally {
2462+
delete global.github;
2463+
}
2464+
});
2465+
2466+
it("processes replace_label messages without no-handler warnings when handler is registered", async () => {
2467+
const messages = [{ type: "replace_label", item_number: 1, label_to_remove: "old", label_to_add: "new" }];
2468+
const mockHandler = vi.fn().mockResolvedValue({
2469+
success: true,
2470+
item_number: 1,
2471+
label_to_remove: "old",
2472+
label_to_add: "new",
2473+
});
2474+
const handlers = new Map([["replace_label", mockHandler]]);
2475+
2476+
const result = await processMessages(handlers, messages);
2477+
2478+
expect(result.results).toHaveLength(1);
2479+
expect(result.results[0].type).toBe("replace_label");
2480+
// Handler was invoked, so no "no handler loaded" error
2481+
expect(result.results[0].error).toBeUndefined();
2482+
expect(mockHandler).toHaveBeenCalledTimes(1);
2483+
});
2484+
2485+
it("records no-handler error for replace_label when handler map is missing entry", async () => {
2486+
const messages = [{ type: "replace_label", item_number: 1, label_to_remove: "old", label_to_add: "new" }];
2487+
// Empty handler map - simulates the bug where replace_label was not in HANDLER_MAP
2488+
const handlers = new Map();
2489+
2490+
const result = await processMessages(handlers, messages);
2491+
2492+
expect(result.results).toHaveLength(1);
2493+
expect(result.results[0].success).toBe(false);
2494+
expect(result.results[0].error).toContain("No handler loaded for type 'replace_label'");
2495+
});
2496+
});
2497+
24332498
describe("rollbackReviewResults", () => {
24342499
it("flips success:true to success:false for submit_pull_request_review results", () => {
24352500
const results = [{ type: "submit_pull_request_review", success: true }];

0 commit comments

Comments
 (0)