fix: safe-output-items.jsonl always empty and SafeItemsCount always 0

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.
+ * 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
+ */
+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)
@@ -304,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,
@@ -457,14 +490,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 +606,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);
             }
           }

actions/setup/js/safe_output_manifest.cjs

@@ -11,8 +11,9 @@ 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 (these typically return a URL,
+ * but the URL may be omitted in some cases).
+ * Kept for backward compatibility.
  * @type {Set<string>}
  */
 const CREATE_ITEM_TYPES = new Set([
@@ -29,18 +30,31 @@ const CREATE_ITEM_TYPES = new Set([
   "autofix_code_scanning_alert",
 ]);
 
+/**
+ * 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)
+ *
+ * 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<string>}
+ */
+const NOT_LOGGED_TYPES = new Set(["noop", "missing_tool", "missing_data"]);
+
 /**
  * @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
  * @property {string} timestamp - ISO 8601 timestamp of creation
  */
 
 /**
- * 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 +68,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 +112,31 @@ 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 explicitly excluded (NOT_LOGGED_TYPES) or if the
+ * result is from a staged (preview) run where no item was actually modified.
+ *
+ * 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}
+ * @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 || NOT_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 +146,7 @@ function extractCreatedItemFromResult(type, result) {
 module.exports = {
   MANIFEST_FILE_PATH,
   CREATE_ITEM_TYPES,
+  NOT_LOGGED_TYPES,
   createManifestLogger,
   ensureManifestExists,
   extractCreatedItemFromResult,

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, NOT_LOGGED_TYPES, createManifestLogger, ensureManifestExists, extractCreatedItemFromResult } from "./safe_output_manifest.cjs";
 
 describe("safe_output_manifest", () => {
   let testManifestFile;
@@ -47,6 +47,33 @@ describe("safe_output_manifest", () => {
     });
   });
 
+  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 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 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);
+    });
+  });
+
   describe("createManifestLogger", () => {
     it("should append a JSONL entry when called with a url", () => {
       const log = createManifestLogger(testManifestFile);
@@ -67,13 +94,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 +227,45 @@ describe("safe_output_manifest", () => {
       expect(item.type).toBe("add_comment");
     });
 
-    it("should return null for non-create 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("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 return null for staged results (no item actually created)", () => {
-      // Staged results have staged: true and no URL — nothing was really created
+    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);
+      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 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 +274,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", () => {

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)

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) {