From 81068e8cf9b5a5032aca3916656483ed5c46cb83 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Mar 2026 01:53:08 +0000 Subject: [PATCH 1/5] Initial plan From 703eef6e0e7826653a02ec50bc1e1a18e87e1f43 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Mar 2026 02:16:52 +0000 Subject: [PATCH 2/5] fix: safe-output-items.jsonl always empty and SafeItemsCount always 0 Expand LOGGED_TYPES in safe_output_manifest.cjs to include all handler types (add_labels, close_issue, update_issue, etc.), not just creation types. Update extractCreatedItemFromResult to use LOGGED_TYPES and make URL optional for modification types. Fix extractCreatedItemsFromManifest in audit_report.go to skip entries without type instead of without URL. Add formatManifestLogMessage helper. Update tests accordingly. Fixes #(issue number) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/safe_output_handler_manager.cjs | 19 +++- actions/setup/js/safe_output_manifest.cjs | 87 +++++++++++++++---- .../setup/js/safe_output_manifest.test.cjs | 80 +++++++++++++---- pkg/cli/audit_report.go | 8 +- pkg/cli/audit_report_test.go | 23 ++++- 5 files changed, 174 insertions(+), 43 deletions(-) diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 7cfc8ca8359..2b96c45456e 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -209,6 +209,19 @@ function collectMissingMessages(messages) { return { missingTools, missingData, noopMessages }; } +/** + * Format a log message for a manifest entry. + * Shows the URL for creation types and the item number for modification types. + * + * @param {{type: string, url?: string, number?: number}} item - Manifest item + * @returns {string} Formatted log message + */ +function formatManifestLogMessage(item) { + if (item.url) return `📝 Manifest: logged ${item.type} → ${item.url}`; + if (item.number != null) return `📝 Manifest: logged ${item.type} #${item.number}`; + return `📝 Manifest: logged ${item.type}`; +} + /** * Process all messages from agent output in the order they appear * Dispatches each message to the appropriate handler while maintaining shared state (temporary ID map) @@ -457,14 +470,14 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) for (const item of result) { const createdItem = extractCreatedItemFromResult(messageType, item); if (createdItem) { - core.info(`📝 Manifest: logged ${createdItem.type} → ${createdItem.url}`); + core.info(formatManifestLogMessage(createdItem)); onItemCreated(createdItem); } } } else { const createdItem = extractCreatedItemFromResult(messageType, result); if (createdItem) { - core.info(`📝 Manifest: logged ${createdItem.type} → ${createdItem.url}`); + core.info(formatManifestLogMessage(createdItem)); onItemCreated(createdItem); } } @@ -573,7 +586,7 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) if (onItemCreated) { const createdItem = extractCreatedItemFromResult(deferred.type, result); if (createdItem) { - core.info(`📝 Manifest: logged ${createdItem.type} → ${createdItem.url}`); + core.info(formatManifestLogMessage(createdItem)); onItemCreated(createdItem); } } diff --git a/actions/setup/js/safe_output_manifest.cjs b/actions/setup/js/safe_output_manifest.cjs index 8cb1b316ea7..af268309fc9 100644 --- a/actions/setup/js/safe_output_manifest.cjs +++ b/actions/setup/js/safe_output_manifest.cjs @@ -11,8 +11,8 @@ const { ERR_SYSTEM } = require("./error_codes.cjs"); const MANIFEST_FILE_PATH = "/tmp/safe-output-items.jsonl"; /** - * Safe output types that create items in GitHub. - * These are the types that should be logged to the manifest. + * Safe output types that create new items in GitHub (always return a URL). + * This is a subset of LOGGED_TYPES kept for backward compatibility. * @type {Set} */ const CREATE_ITEM_TYPES = new Set([ @@ -29,10 +29,59 @@ const CREATE_ITEM_TYPES = new Set([ "autofix_code_scanning_alert", ]); +/** + * All safe output types that should be logged to the manifest, including both + * creation operations (which return a URL) and modification operations (which + * operate on existing GitHub items and may not return a URL). + * + * Excludes purely internal types that do not represent a GitHub state change: + * - noop: no-op, produces no GitHub side effects + * - missing_tool / missing_data: meta-operations that signal missing capabilities + * @type {Set} + */ +const LOGGED_TYPES = new Set([ + // Creation types (also in CREATE_ITEM_TYPES) + "create_issue", + "add_comment", + "create_discussion", + "create_pull_request", + "create_project", + "create_project_status_update", + "create_pull_request_review_comment", + "submit_pull_request_review", + "reply_to_pull_request_review_comment", + "create_code_scanning_alert", + "autofix_code_scanning_alert", + "create_missing_tool_issue", + "create_missing_data_issue", + // Modification/action types + "close_issue", + "close_discussion", + "add_labels", + "remove_labels", + "update_issue", + "update_discussion", + "link_sub_issue", + "update_release", + "resolve_pull_request_review_thread", + "push_to_pull_request_branch", + "update_pull_request", + "close_pull_request", + "mark_pull_request_as_ready_for_review", + "hide_comment", + "set_issue_type", + "add_reviewer", + "assign_milestone", + "assign_to_user", + "unassign_from_user", + "dispatch_workflow", + "update_project", +]); + /** * @typedef {Object} ManifestEntry * @property {string} type - The safe output type (e.g., "create_issue") - * @property {string} url - URL of the created item in GitHub + * @property {string} [url] - URL of the affected item in GitHub (present for creation types; omitted for modification types that don't return a URL) * @property {number} [number] - Issue/PR/discussion number if applicable * @property {string} [repo] - Repository slug (owner/repo) if applicable * @property {string} [temporaryId] - Temporary ID assigned to this item, if any @@ -40,7 +89,7 @@ const CREATE_ITEM_TYPES = new Set([ */ /** - * Create a manifest logger function for recording created items. + * Create a manifest logger function for recording executed safe output items. * * The logger writes JSONL entries to the specified manifest file. * It is designed to be easily testable by accepting the file path as a parameter. @@ -54,18 +103,17 @@ function createManifestLogger(manifestFile = MANIFEST_FILE_PATH) { ensureManifestExists(manifestFile); /** - * Log a created item to the manifest file. - * Items without a URL are silently skipped. + * Log an executed safe output item to the manifest file. * - * @param {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string}} item - Created item details + * @param {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string}} item - Executed item details */ return function logCreatedItem(item) { - if (!item || !item.url) return; + if (!item) return; /** @type {ManifestEntry} */ const entry = { type: item.type, - url: item.url, + ...(item.url ? { url: item.url } : {}), ...(item.number != null ? { number: item.number } : {}), ...(item.repo ? { repo: item.repo } : {}), ...(item.temporaryId ? { temporaryId: item.temporaryId } : {}), @@ -99,27 +147,29 @@ function ensureManifestExists(manifestFile = MANIFEST_FILE_PATH) { } /** - * Extract created item details from a handler result for manifest logging. - * Returns null if the result does not represent a created item with a URL, - * or if the result is from a staged (preview) run where no item was actually created. + * Extract executed item details from a handler result for manifest logging. + * Returns null if the type is not tracked (not in LOGGED_TYPES) or if the + * result is from a staged (preview) run where no item was actually modified. + * + * For creation types (CREATE_ITEM_TYPES), the result URL is included when present. + * For modification types (e.g. add_labels, close_issue), the URL is optional. * * @param {string} type - The handler type (e.g., "create_issue") * @param {any} result - The handler result object - * @returns {{type: string, url: string, number?: number, repo?: string, temporaryId?: string}|null} + * @returns {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string}|null} */ function extractCreatedItemFromResult(type, result) { - if (!result || !CREATE_ITEM_TYPES.has(type)) return null; + if (!result || !LOGGED_TYPES.has(type)) return null; - // In staged mode (🎭 Staged Mode Preview), no item was actually created in GitHub — skip logging + // In staged mode (🎭 Staged Mode Preview), no item was actually modified in GitHub — skip logging if (result.staged === true) return null; - // Normalize URL from different result shapes + // Normalize URL from different result shapes (present for creation types) const url = result.url || result.projectUrl || result.html_url; - if (!url) return null; return { type, - url, + ...(url ? { url } : {}), ...(result.number != null ? { number: result.number } : {}), ...(result.repo ? { repo: result.repo } : {}), ...(result.temporaryId ? { temporaryId: result.temporaryId } : {}), @@ -129,6 +179,7 @@ function extractCreatedItemFromResult(type, result) { module.exports = { MANIFEST_FILE_PATH, CREATE_ITEM_TYPES, + LOGGED_TYPES, createManifestLogger, ensureManifestExists, extractCreatedItemFromResult, diff --git a/actions/setup/js/safe_output_manifest.test.cjs b/actions/setup/js/safe_output_manifest.test.cjs index 07c76b58e19..358a4d8b2c1 100644 --- a/actions/setup/js/safe_output_manifest.test.cjs +++ b/actions/setup/js/safe_output_manifest.test.cjs @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import fs from "fs"; import path from "path"; -import { MANIFEST_FILE_PATH, CREATE_ITEM_TYPES, createManifestLogger, ensureManifestExists, extractCreatedItemFromResult } from "./safe_output_manifest.cjs"; +import { MANIFEST_FILE_PATH, CREATE_ITEM_TYPES, LOGGED_TYPES, createManifestLogger, ensureManifestExists, extractCreatedItemFromResult } from "./safe_output_manifest.cjs"; describe("safe_output_manifest", () => { let testManifestFile; @@ -47,6 +47,30 @@ describe("safe_output_manifest", () => { }); }); + describe("LOGGED_TYPES", () => { + it("should include all CREATE_ITEM_TYPES", () => { + for (const type of CREATE_ITEM_TYPES) { + expect(LOGGED_TYPES.has(type)).toBe(true); + } + }); + + it("should include modification types not in CREATE_ITEM_TYPES", () => { + expect(LOGGED_TYPES.has("add_labels")).toBe(true); + expect(LOGGED_TYPES.has("close_issue")).toBe(true); + expect(LOGGED_TYPES.has("update_issue")).toBe(true); + expect(LOGGED_TYPES.has("remove_labels")).toBe(true); + expect(LOGGED_TYPES.has("close_discussion")).toBe(true); + expect(LOGGED_TYPES.has("update_pull_request")).toBe(true); + expect(LOGGED_TYPES.has("close_pull_request")).toBe(true); + }); + + it("should not include internal/meta types", () => { + expect(LOGGED_TYPES.has("noop")).toBe(false); + expect(LOGGED_TYPES.has("missing_tool")).toBe(false); + expect(LOGGED_TYPES.has("missing_data")).toBe(false); + }); + }); + describe("createManifestLogger", () => { it("should append a JSONL entry when called with a url", () => { const log = createManifestLogger(testManifestFile); @@ -67,13 +91,19 @@ describe("safe_output_manifest", () => { expect(Date.parse(entry.timestamp)).not.toBeNaN(); }); - it("should skip items without a url", () => { + it("should append a JSONL entry for an item without a url (modification type)", () => { const log = createManifestLogger(testManifestFile); - log({ type: "create_issue", url: undefined }); + log({ type: "add_labels", number: 20875 }); - // File is created by createManifestLogger() immediately, but should be empty - expect(fs.existsSync(testManifestFile)).toBe(true); - expect(fs.readFileSync(testManifestFile, "utf8")).toBe(""); + const content = fs.readFileSync(testManifestFile, "utf8"); + const lines = content.trim().split("\n"); + expect(lines).toHaveLength(1); + + const entry = JSON.parse(lines[0]); + expect(entry.type).toBe("add_labels"); + expect(entry.url).toBeUndefined(); + expect(entry.number).toBe(20875); + expect(entry.timestamp).toBeDefined(); }); it("should skip null/undefined items", () => { @@ -194,20 +224,37 @@ describe("safe_output_manifest", () => { expect(item.type).toBe("add_comment"); }); - it("should return null for non-create types", () => { + it("should return null for non-logged types (noop and internal meta types)", () => { const result = { success: true, url: "https://github.com/owner/repo/issues/1" }; - expect(extractCreatedItemFromResult("update_issue", result)).toBeNull(); - expect(extractCreatedItemFromResult("close_issue", result)).toBeNull(); - expect(extractCreatedItemFromResult("add_labels", result)).toBeNull(); expect(extractCreatedItemFromResult("noop", result)).toBeNull(); + expect(extractCreatedItemFromResult("missing_tool", result)).toBeNull(); + expect(extractCreatedItemFromResult("missing_data", result)).toBeNull(); + }); + + it("should extract item from add_labels result (modification type without url)", () => { + const result = { success: true, number: 20875, labelsAdded: ["bug", "cli"], contextType: "issue" }; + const item = extractCreatedItemFromResult("add_labels", result); + expect(item).not.toBeNull(); + expect(item.type).toBe("add_labels"); + expect(item.url).toBeUndefined(); + expect(item.number).toBe(20875); + }); + + it("should extract item from close_issue result (modification type with url)", () => { + const result = { success: true, number: 123, url: "https://github.com/owner/repo/issues/123", title: "Test" }; + const item = extractCreatedItemFromResult("close_issue", result); + expect(item).not.toBeNull(); + expect(item.type).toBe("close_issue"); + expect(item.url).toBe("https://github.com/owner/repo/issues/123"); + expect(item.number).toBe(123); }); - it("should return null for staged results (no item actually created)", () => { - // Staged results have staged: true and no URL — nothing was really created + it("should return null for staged results (no item actually modified)", () => { + // Staged results have staged: true — nothing was really changed const stagedResult = { success: true, staged: true, previewInfo: { repo: "owner/repo", title: "Test" } }; expect(extractCreatedItemFromResult("create_issue", stagedResult)).toBeNull(); expect(extractCreatedItemFromResult("add_comment", stagedResult)).toBeNull(); - expect(extractCreatedItemFromResult("create_discussion", stagedResult)).toBeNull(); + expect(extractCreatedItemFromResult("add_labels", stagedResult)).toBeNull(); }); it("should return null for staged results even if url is somehow present", () => { @@ -216,9 +263,12 @@ describe("safe_output_manifest", () => { expect(extractCreatedItemFromResult("create_issue", stagedResultWithUrl)).toBeNull(); }); - it("should return null when result has no URL", () => { + it("should return item without url when result has no URL (for logged types)", () => { const result = { success: true, repo: "owner/repo", number: 1 }; - expect(extractCreatedItemFromResult("create_issue", result)).toBeNull(); + const item = extractCreatedItemFromResult("create_issue", result); + expect(item).not.toBeNull(); + expect(item.url).toBeUndefined(); + expect(item.number).toBe(1); }); it("should return null for null/undefined result", () => { diff --git a/pkg/cli/audit_report.go b/pkg/cli/audit_report.go index 355f4ae8097..9c4d7bf223e 100644 --- a/pkg/cli/audit_report.go +++ b/pkg/cli/audit_report.go @@ -105,10 +105,12 @@ type FileInfo struct { Description string `json:"description"` } -// CreatedItemReport represents a single item created in GitHub by a safe output handler +// CreatedItemReport represents a single item executed in GitHub by a safe output handler. +// URL is present for creation types (e.g. create_issue, add_comment) but may be empty +// for modification types (e.g. add_labels, close_issue) that do not return a URL. type CreatedItemReport struct { Type string `json:"type" console:"header:Type"` - URL string `json:"url" console:"header:URL"` + URL string `json:"url,omitempty" console:"header:URL,omitempty"` Number int `json:"number,omitempty" console:"header:Number,omitempty"` Repo string `json:"repo,omitempty" console:"header:Repo,omitempty"` TemporaryID string `json:"temporaryId,omitempty" console:"header:Temp ID,omitempty"` @@ -409,7 +411,7 @@ func extractCreatedItemsFromManifest(logsPath string) []CreatedItemReport { auditReportLog.Printf("Skipping invalid manifest line: %v", err) continue } - if item.URL == "" { + if item.Type == "" { continue } items = append(items, item) diff --git a/pkg/cli/audit_report_test.go b/pkg/cli/audit_report_test.go index a23203d9f82..ddec7b6ddc5 100644 --- a/pkg/cli/audit_report_test.go +++ b/pkg/cli/audit_report_test.go @@ -1150,16 +1150,31 @@ func TestExtractCreatedItemsFromManifest(t *testing.T) { assert.Equal(t, "add_comment", items[1].Type) }) - t.Run("skips entries without URL", func(t *testing.T) { + t.Run("includes entries without URL (modification types like add_labels)", func(t *testing.T) { dir := t.TempDir() - content := `{"type":"create_issue","url":"","timestamp":"2024-01-01T00:00:00Z"} + content := `{"type":"add_labels","number":20875,"timestamp":"2024-01-01T00:00:00Z"} {"type":"create_issue","url":"https://github.com/owner/repo/issues/2","timestamp":"2024-01-01T00:01:00Z"} ` require.NoError(t, os.WriteFile(filepath.Join(dir, safeOutputItemsManifestFilename), []byte(content), 0600)) items := extractCreatedItemsFromManifest(dir) - require.Len(t, items, 1, "should skip entry with empty URL") - assert.Equal(t, "https://github.com/owner/repo/issues/2", items[0].URL) + require.Len(t, items, 2, "should include both entries: one without URL and one with URL") + assert.Equal(t, "add_labels", items[0].Type) + assert.Empty(t, items[0].URL) + assert.Equal(t, 20875, items[0].Number) + assert.Equal(t, "https://github.com/owner/repo/issues/2", items[1].URL) + }) + + t.Run("skips entries without type field", func(t *testing.T) { + dir := t.TempDir() + content := `{"url":"https://github.com/owner/repo/issues/1","timestamp":"2024-01-01T00:00:00Z"} +{"type":"add_comment","url":"https://github.com/owner/repo/issues/1#issuecomment-1","timestamp":"2024-01-01T00:02:00Z"} +` + require.NoError(t, os.WriteFile(filepath.Join(dir, safeOutputItemsManifestFilename), []byte(content), 0600)) + + items := extractCreatedItemsFromManifest(dir) + require.Len(t, items, 1, "should skip entry without type and parse 1 valid one") + assert.Equal(t, "add_comment", items[0].Type) }) t.Run("skips invalid JSON lines", func(t *testing.T) { From 086aafbd473f9f11b4825b9482f31fe0c7a56a51 Mon Sep 17 00:00:00 2001 From: Peli de Halleux Date: Fri, 13 Mar 2026 19:49:54 -0700 Subject: [PATCH 3/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- actions/setup/js/safe_output_handler_manager.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 2b96c45456e..47f964497fc 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -211,7 +211,7 @@ function collectMissingMessages(messages) { /** * Format a log message for a manifest entry. - * Shows the URL for creation types and the item number for modification types. + * Prefers the URL when available, otherwise falls back to the item number. * * @param {{type: string, url?: string, number?: number}} item - Manifest item * @returns {string} Formatted log message From 0034799ec66a990e3c80bfff7a4e4969c7162a78 Mon Sep 17 00:00:00 2001 From: Peli de Halleux Date: Fri, 13 Mar 2026 19:50:04 -0700 Subject: [PATCH 4/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- actions/setup/js/safe_output_manifest.cjs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/actions/setup/js/safe_output_manifest.cjs b/actions/setup/js/safe_output_manifest.cjs index af268309fc9..7e7d3f7856f 100644 --- a/actions/setup/js/safe_output_manifest.cjs +++ b/actions/setup/js/safe_output_manifest.cjs @@ -11,7 +11,8 @@ const { ERR_SYSTEM } = require("./error_codes.cjs"); const MANIFEST_FILE_PATH = "/tmp/safe-output-items.jsonl"; /** - * Safe output types that create new items in GitHub (always return a URL). + * Safe output types that create new items in GitHub (these typically return a URL, + * but the URL may be omitted in some cases). * This is a subset of LOGGED_TYPES kept for backward compatibility. * @type {Set} */ @@ -31,8 +32,9 @@ const CREATE_ITEM_TYPES = new Set([ /** * All safe output types that should be logged to the manifest, including both - * creation operations (which return a URL) and modification operations (which - * operate on existing GitHub items and may not return a URL). + * creation operations (which typically return a URL, but may omit it) and + * modification operations (which operate on existing GitHub items and may not + * return a URL). * * Excludes purely internal types that do not represent a GitHub state change: * - noop: no-op, produces no GitHub side effects From 84aa8a2e9ec55976157da7973a535ff1d1aa9e7f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 14 Mar 2026 02:57:38 +0000 Subject: [PATCH 5/5] refactor: make manifest generic with NOT_LOGGED_TYPES and support safe jobs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace hardcoded LOGGED_TYPES (30+ types) with NOT_LOGGED_TYPES exclusion set (noop, missing_tool, missing_data). Any type not in NOT_LOGGED_TYPES is automatically logged — new handlers and custom safe job types included without needing to update any list. Log custom safe output job dispatches to manifest so SafeItemsCount counts operations handled by user-defined safe jobs. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../setup/js/safe_output_handler_manager.cjs | 22 +++++- actions/setup/js/safe_output_manifest.cjs | 67 +++++-------------- .../setup/js/safe_output_manifest.test.cjs | 49 ++++++++------ 3 files changed, 67 insertions(+), 71 deletions(-) diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 47f964497fc..035c9135008 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -317,8 +317,28 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) // Check if this message type is handled by a custom safe output job if (customJobTypes.has(messageType)) { - // Silently skip - this is handled by a custom safe output job step core.debug(`Message ${i + 1} (${messageType}) will be handled by custom safe output job`); + + // Log the dispatch to the manifest so the operation is counted in SafeItemsCount. + // The custom job does the actual work; here we record the intent from the message. + if (onItemCreated) { + // Prefer item_number (explicit target), fall back to issue_number then pull_request_number. + // This mirrors the precedence order used by individual safe output handlers. + const rawNumber = message.item_number ?? message.issue_number ?? message.pull_request_number; + const itemNumber = rawNumber != null ? parseInt(String(rawNumber), 10) : undefined; + const validNumber = itemNumber != null && !isNaN(itemNumber) ? itemNumber : undefined; + + const messageResult = { + ...(validNumber != null ? { number: validNumber } : {}), + ...(message.repo ? { repo: message.repo } : {}), + }; + const createdItem = extractCreatedItemFromResult(messageType, messageResult); + if (createdItem) { + core.info(formatManifestLogMessage(createdItem)); + onItemCreated(createdItem); + } + } + results.push({ type: messageType, messageIndex: i, diff --git a/actions/setup/js/safe_output_manifest.cjs b/actions/setup/js/safe_output_manifest.cjs index 7e7d3f7856f..53a71d2a711 100644 --- a/actions/setup/js/safe_output_manifest.cjs +++ b/actions/setup/js/safe_output_manifest.cjs @@ -13,7 +13,7 @@ const MANIFEST_FILE_PATH = "/tmp/safe-output-items.jsonl"; /** * Safe output types that create new items in GitHub (these typically return a URL, * but the URL may be omitted in some cases). - * This is a subset of LOGGED_TYPES kept for backward compatibility. + * Kept for backward compatibility. * @type {Set} */ const CREATE_ITEM_TYPES = new Set([ @@ -31,54 +31,17 @@ const CREATE_ITEM_TYPES = new Set([ ]); /** - * All safe output types that should be logged to the manifest, including both - * creation operations (which typically return a URL, but may omit it) and - * modification operations (which operate on existing GitHub items and may not - * return a URL). + * Safe output types that should NEVER be logged to the manifest. + * These types represent metadata signals rather than GitHub state changes: + * - noop: no-op message, produces no GitHub side effects + * - missing_tool: records a missing tool capability (metadata only) + * - missing_data: records missing required data (metadata only) * - * Excludes purely internal types that do not represent a GitHub state change: - * - noop: no-op, produces no GitHub side effects - * - missing_tool / missing_data: meta-operations that signal missing capabilities + * All other types — built-in handler types, custom safe job types, and + * any future types — are logged automatically without needing to update this list. * @type {Set} */ -const LOGGED_TYPES = new Set([ - // Creation types (also in CREATE_ITEM_TYPES) - "create_issue", - "add_comment", - "create_discussion", - "create_pull_request", - "create_project", - "create_project_status_update", - "create_pull_request_review_comment", - "submit_pull_request_review", - "reply_to_pull_request_review_comment", - "create_code_scanning_alert", - "autofix_code_scanning_alert", - "create_missing_tool_issue", - "create_missing_data_issue", - // Modification/action types - "close_issue", - "close_discussion", - "add_labels", - "remove_labels", - "update_issue", - "update_discussion", - "link_sub_issue", - "update_release", - "resolve_pull_request_review_thread", - "push_to_pull_request_branch", - "update_pull_request", - "close_pull_request", - "mark_pull_request_as_ready_for_review", - "hide_comment", - "set_issue_type", - "add_reviewer", - "assign_milestone", - "assign_to_user", - "unassign_from_user", - "dispatch_workflow", - "update_project", -]); +const NOT_LOGGED_TYPES = new Set(["noop", "missing_tool", "missing_data"]); /** * @typedef {Object} ManifestEntry @@ -150,18 +113,20 @@ function ensureManifestExists(manifestFile = MANIFEST_FILE_PATH) { /** * Extract executed item details from a handler result for manifest logging. - * Returns null if the type is not tracked (not in LOGGED_TYPES) or if the + * Returns null if the type is explicitly excluded (NOT_LOGGED_TYPES) or if the * result is from a staged (preview) run where no item was actually modified. * - * For creation types (CREATE_ITEM_TYPES), the result URL is included when present. - * For modification types (e.g. add_labels, close_issue), the URL is optional. + * All other types — built-in handlers, custom safe job types, and future types — + * are logged automatically. For creation types (CREATE_ITEM_TYPES), the result + * URL is included when present. For modification types (e.g. add_labels, + * close_issue), the URL is optional. * * @param {string} type - The handler type (e.g., "create_issue") * @param {any} result - The handler result object * @returns {{type: string, url?: string, number?: number, repo?: string, temporaryId?: string}|null} */ function extractCreatedItemFromResult(type, result) { - if (!result || !LOGGED_TYPES.has(type)) return null; + if (!result || NOT_LOGGED_TYPES.has(type)) return null; // In staged mode (🎭 Staged Mode Preview), no item was actually modified in GitHub — skip logging if (result.staged === true) return null; @@ -181,7 +146,7 @@ function extractCreatedItemFromResult(type, result) { module.exports = { MANIFEST_FILE_PATH, CREATE_ITEM_TYPES, - LOGGED_TYPES, + NOT_LOGGED_TYPES, createManifestLogger, ensureManifestExists, extractCreatedItemFromResult, diff --git a/actions/setup/js/safe_output_manifest.test.cjs b/actions/setup/js/safe_output_manifest.test.cjs index 358a4d8b2c1..dd98cb273ce 100644 --- a/actions/setup/js/safe_output_manifest.test.cjs +++ b/actions/setup/js/safe_output_manifest.test.cjs @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import fs from "fs"; import path from "path"; -import { MANIFEST_FILE_PATH, CREATE_ITEM_TYPES, LOGGED_TYPES, createManifestLogger, ensureManifestExists, extractCreatedItemFromResult } from "./safe_output_manifest.cjs"; +import { MANIFEST_FILE_PATH, CREATE_ITEM_TYPES, NOT_LOGGED_TYPES, createManifestLogger, ensureManifestExists, extractCreatedItemFromResult } from "./safe_output_manifest.cjs"; describe("safe_output_manifest", () => { let testManifestFile; @@ -47,27 +47,30 @@ describe("safe_output_manifest", () => { }); }); - describe("LOGGED_TYPES", () => { - it("should include all CREATE_ITEM_TYPES", () => { - for (const type of CREATE_ITEM_TYPES) { - expect(LOGGED_TYPES.has(type)).toBe(true); - } + describe("NOT_LOGGED_TYPES", () => { + it("should contain only noop and internal meta types", () => { + expect(NOT_LOGGED_TYPES.has("noop")).toBe(true); + expect(NOT_LOGGED_TYPES.has("missing_tool")).toBe(true); + expect(NOT_LOGGED_TYPES.has("missing_data")).toBe(true); }); - it("should include modification types not in CREATE_ITEM_TYPES", () => { - expect(LOGGED_TYPES.has("add_labels")).toBe(true); - expect(LOGGED_TYPES.has("close_issue")).toBe(true); - expect(LOGGED_TYPES.has("update_issue")).toBe(true); - expect(LOGGED_TYPES.has("remove_labels")).toBe(true); - expect(LOGGED_TYPES.has("close_discussion")).toBe(true); - expect(LOGGED_TYPES.has("update_pull_request")).toBe(true); - expect(LOGGED_TYPES.has("close_pull_request")).toBe(true); + it("should not contain any handler or modification types (all are logged by default)", () => { + expect(NOT_LOGGED_TYPES.has("create_issue")).toBe(false); + expect(NOT_LOGGED_TYPES.has("add_labels")).toBe(false); + expect(NOT_LOGGED_TYPES.has("close_issue")).toBe(false); + expect(NOT_LOGGED_TYPES.has("update_issue")).toBe(false); }); - it("should not include internal/meta types", () => { - expect(LOGGED_TYPES.has("noop")).toBe(false); - expect(LOGGED_TYPES.has("missing_tool")).toBe(false); - expect(LOGGED_TYPES.has("missing_data")).toBe(false); + it("should not contain CREATE_ITEM_TYPES (they are logged)", () => { + for (const type of CREATE_ITEM_TYPES) { + expect(NOT_LOGGED_TYPES.has(type)).toBe(false); + } + }); + + it("should allow custom safe job types to be logged automatically (not in exclusion list)", () => { + // Custom safe job types are never added to NOT_LOGGED_TYPES so they are always logged + expect(NOT_LOGGED_TYPES.has("my_custom_job_type")).toBe(false); + expect(NOT_LOGGED_TYPES.has("deploy_to_staging")).toBe(false); }); }); @@ -224,13 +227,21 @@ describe("safe_output_manifest", () => { expect(item.type).toBe("add_comment"); }); - it("should return null for non-logged types (noop and internal meta types)", () => { + it("should return null for excluded types (noop and internal meta types)", () => { const result = { success: true, url: "https://github.com/owner/repo/issues/1" }; expect(extractCreatedItemFromResult("noop", result)).toBeNull(); expect(extractCreatedItemFromResult("missing_tool", result)).toBeNull(); expect(extractCreatedItemFromResult("missing_data", result)).toBeNull(); }); + it("should extract item from custom safe job type (generic: any type not excluded is logged)", () => { + const result = { success: true, number: 42 }; + const item = extractCreatedItemFromResult("my_custom_job_type", result); + expect(item).not.toBeNull(); + expect(item.type).toBe("my_custom_job_type"); + expect(item.number).toBe(42); + }); + it("should extract item from add_labels result (modification type without url)", () => { const result = { success: true, number: 20875, labelsAdded: ["bug", "cli"], contextType: "issue" }; const item = extractCreatedItemFromResult("add_labels", result);