Fix safe-output summary outcome classification

actions/setup/js/add_comment.cjs

@@ -565,6 +565,8 @@ async function main(config = {}) {
       return {
         success: false,
         skipped: true,
+        reasonCode: "MAX_COUNT_REACHED",
+        reason: "Max count reached",
         error: `Max count of ${maxCount} reached`,
       };
     }
@@ -682,6 +684,8 @@ async function main(config = {}) {
               return {
                 success: false,
                 skipped: true,
+                reasonCode: "NO_CONTEXT",
+                reason: "No target context available",
                 error: targetResult.error,
               };
             }
@@ -696,6 +700,8 @@ async function main(config = {}) {
             return {
               success: false,
               skipped: true,
+              reasonCode: "NO_CONTEXT",
+              reason: "No target context available",
               error: targetResult.error,
             };
           }
@@ -716,14 +722,31 @@ async function main(config = {}) {
         });
         if (requiredLabels.length > 0) {
           const itemLabels = (filterItem.labels || []).map(/** @param {any} l */ l => (typeof l === "string" ? l : l.name || ""));
-          if (!requiredLabels.every(r => itemLabels.includes(r))) {
+          const missingLabels = requiredLabels.filter(r => !itemLabels.includes(r));
+          if (missingLabels.length > 0) {
             core.info(`Skipping add_comment for #${itemNumber}: does not match required-labels filter (${requiredLabels.join(", ")})`);
-            return { success: false, skipped: true, error: `Item does not match required-labels filter` };
+            return {
+              success: false,
+              skipped: true,
+              reasonCode: "REQUIRED_LABELS_MISMATCH",
+              reason: "Required labels missing",
+              error: "Item does not match required-labels filter",
+              target: { repo: itemRepo, number: itemNumber },
+              safeDetails: { requiredLabels, missingLabels },
+            };
           }
         }
         if (requiredTitlePrefix && !filterItem.title?.startsWith(requiredTitlePrefix)) {
           core.info(`Skipping add_comment for #${itemNumber}: title does not start with required prefix "${requiredTitlePrefix}"`);
-          return { success: false, skipped: true, error: `Item title does not start with required prefix` };
+          return {
+            success: false,
+            skipped: true,
+            reasonCode: "REQUIRED_TITLE_PREFIX_MISMATCH",
+            reason: "Required title prefix missing",
+            error: "Item title does not start with required prefix",
+            target: { repo: itemRepo, number: itemNumber },
+            safeDetails: { requiredTitlePrefix },
+          };
         }
       } catch (err) {
         core.warning(`Could not fetch item #${itemNumber} to check filters: ${getErrorMessage(err)}`);
@@ -1049,6 +1072,9 @@ async function main(config = {}) {
               success: true,
               warning: `Target not found: ${discussionErrorMessage}`,
               skipped: true,
+              reasonCode: "TARGET_NOT_FOUND",
+              reason: "Target not found",
+              target: { repo: itemRepo, number: itemNumber },
             };
           }
 
@@ -1063,6 +1089,9 @@ async function main(config = {}) {
             return {
               success: false,
               skipped: true,
+              reasonCode: "DISCUSSIONS_TOKEN_SCOPE_MISMATCH",
+              reason: "GitHub token cannot add comments to discussions",
+              target: { repo: itemRepo, number: itemNumber },
               error: warningMessage,
             };
           }
@@ -1083,6 +1112,9 @@ async function main(config = {}) {
           success: true,
           warning: `Target not found: ${errorMessage}`,
           skipped: true,
+          reasonCode: "TARGET_NOT_FOUND",
+          reason: "Target not found",
+          target: { repo: itemRepo, number: itemNumber },
         };
       }
 
@@ -1093,6 +1125,9 @@ async function main(config = {}) {
           success: true,
           warning: `Target is locked: ${errorMessage}`,
           skipped: true,
+          reasonCode: "TARGET_LOCKED",
+          reason: "Target is locked",
+          target: { repo: itemRepo, number: itemNumber },
         };
       }
 

actions/setup/js/add_labels.cjs

@@ -232,14 +232,31 @@
         });
         if (requiredLabels.length > 0) {
           const itemLabels = (item.labels || []).map(/** @param {any} l */ l => (typeof l === "string" ? l : l.name || ""));
-          if (!requiredLabels.every(r => itemLabels.includes(r))) {
+          const missingLabels = requiredLabels.filter(r => !itemLabels.includes(r));
+          if (missingLabels.length > 0) {
             core.info(`Skipping add_labels for ${contextType} #${itemNumber}: does not match required-labels filter (${requiredLabels.join(", ")})`);
-            return { success: false, skipped: true, error: `Item does not match required-labels filter` };
+            return {
+              success: false,
+              skipped: true,
+              reasonCode: "REQUIRED_LABELS_MISMATCH",
+              reason: "Required labels missing",
+              error: "Item does not match required-labels filter",
+              target: { repo: itemRepo, number: itemNumber },
+              safeDetails: { requiredLabels, missingLabels },
+            };
           }
         }
         if (requiredTitlePrefix && !item.title?.startsWith(requiredTitlePrefix)) {
           core.info(`Skipping add_labels for ${contextType} #${itemNumber}: title does not start with required prefix "${requiredTitlePrefix}"`);
-          return { success: false, skipped: true, error: `Item title does not start with required prefix` };
+          return {
+            success: false,
+            skipped: true,
+            reasonCode: "REQUIRED_TITLE_PREFIX_MISMATCH",
+            reason: "Required title prefix missing",
+            error: "Item title does not start with required prefix",
+            target: { repo: itemRepo, number: itemNumber },
+            safeDetails: { requiredTitlePrefix },
+          };
         }
       }
 
@@ -345,7 +362,7 @@
 
           const issueNodeId = issueData?.node_id;
           if (!issueNodeId) {
             throw new Error(`${SAFE_OUTPUT_E099}: Failed to resolve GraphQL node ID for ${contextType} #${itemNumber}`);
           }
 
           // The GraphQL updateIssue mutation only accepts Issue node IDs, and

actions/setup/js/safe_output_handler_manager.cjs

@@ -691,10 +691,15 @@ function partitionFailureResults(results) {
 /**
  * Export item-level safe-output status as GitHub Actions outputs.
  *
- * @param {{itemsSucceeded: number, itemsFailed: number, status: string}} status
+ * @param {{itemsSucceeded: number, itemsApplied?: number, itemsSkipped?: number, itemsWarnings?: number, itemsCancelled?: number, itemsDeferred?: number, itemsFailed: number, status: string}} status
  */
 function setSafeOutputsStatusOutputs(status) {
   core.setOutput("items_succeeded", String(status.itemsSucceeded));
+  core.setOutput("items_applied", String(status.itemsApplied ?? status.itemsSucceeded));
+  core.setOutput("items_skipped", String(status.itemsSkipped ?? 0));
+  core.setOutput("items_warnings", String(status.itemsWarnings ?? 0));
+  core.setOutput("items_cancelled", String(status.itemsCancelled ?? 0));
+  core.setOutput("items_deferred", String(status.itemsDeferred ?? 0));
   core.setOutput("items_failed", String(status.itemsFailed));
   core.setOutput("status", status.status);
 }
@@ -908,17 +913,24 @@ async function processMessages(messageHandlers, messages, onItemCreated = null)
       // Call the message handler with the individual message and resolved temp IDs
       const result = await messageHandler(effectiveMessage, resolvedTemporaryIds, temporaryIdMap);
 
-      // Check if the handler explicitly returned a skipped result (e.g. if_no_changes: warn/ignore).
-      // Skipped results should NOT trigger fail-fast cancellation of subsequent messages.
-      if (result && result.success === false && result.skipped === true && !result.deferred) {
-        const msg = result.error || "Handler returned success: false with skipped: true";
+      // Check if the handler explicitly returned a skipped result (e.g. policy filters,
+      // no-op warnings, or if_no_changes: warn/ignore). Skipped results should NOT
+      // trigger fail-fast cancellation of subsequent messages, and any summary-safe
+      // diagnostics supplied by the handler must be preserved.
+      if (result && result.skipped === true && !result.deferred) {
+        const msg = result.reason || result.warning || result.error || "Handler returned skipped: true";
         core.info(`⏭ Message ${i + 1} (${messageType}) skipped — ${msg}`);
         results.push({
           type: messageType,
           messageIndex: i,
-          success: false,
+          success: result.success === true,
           skipped: true,
+          ...(result.warning ? { warning: result.warning } : {}),
+          ...(result.reason ? { reason: result.reason } : {}),
+          ...(result.reasonCode ? { reasonCode: result.reasonCode } : {}),
+          ...(result.errorCode ? { errorCode: result.errorCode } : {}),
           error: msg,
+          result,
         });
         continue;
       }
@@ -1093,6 +1105,27 @@ async function processMessages(messageHandlers, messages, onItemCreated = null)
         // Call the handler again with updated temp ID map
         const result = await deferred.handler(deferred.message, resolvedTemporaryIds, temporaryIdMap);
 
+        if (result && result.skipped === true && !result.deferred) {
+          const msg = result.reason || result.warning || result.error || "Handler returned skipped: true";
+          core.info(`⏭ Retry of message ${deferred.messageIndex + 1} (${deferred.type}) skipped — ${msg}`);
+          const resultIndex = results.findIndex(r => r.messageIndex === deferred.messageIndex);
+          if (resultIndex >= 0) {
+            results[resultIndex] = {
+              type: deferred.type,
+              messageIndex: deferred.messageIndex,
+              success: result.success === true,
+              skipped: true,
+              ...(result.warning ? { warning: result.warning } : {}),
+              ...(result.reason ? { reason: result.reason } : {}),
+              ...(result.reasonCode ? { reasonCode: result.reasonCode } : {}),
+              ...(result.errorCode ? { errorCode: result.errorCode } : {}),
+              error: msg,
+              result,
+            };
+          }
+          continue;
+        }
+
         // Check if the handler explicitly returned a failure
         if (result && result.success === false && !result.deferred) {
           const errorMsg = result.error || "Handler returned success: false";

actions/setup/js/safe_output_handler_manager.test.cjs

@@ -219,11 +219,35 @@ describe("Safe Output Handler Manager", () => {
 
       expect(status).toEqual({
         itemsSucceeded: 2,
+        itemsApplied: 2,
+        itemsSkipped: 1,
+        itemsWarnings: 0,
+        itemsCancelled: 1,
+        itemsDeferred: 1,
         itemsFailed: 1,
         status: "partial_success",
       });
     });
 
+    it("computes skipped and warning counts without counting them as applied mutations", () => {
+      expect(
+        computeSafeOutputsStatus([
+          { type: "add_comment", success: true, result: { success: true, skipped: true, warning: "Target locked" } },
+          { type: "add_labels", success: false, skipped: true, result: { success: false, skipped: true, reasonCode: "REQUIRED_LABELS_MISMATCH" } },
+          { type: "create_issue", success: true },
+        ])
+      ).toEqual({
+        itemsSucceeded: 1,
+        itemsApplied: 1,
+        itemsSkipped: 2,
+        itemsWarnings: 0,
+        itemsCancelled: 0,
+        itemsDeferred: 0,
+        itemsFailed: 0,
+        status: "completed_with_skips",
+      });
+    });
+
     it("computes failure item status when all active results failed", () => {
       expect(
         computeSafeOutputsStatus([
@@ -232,6 +256,11 @@ describe("Safe Output Handler Manager", () => {
         ])
       ).toEqual({
         itemsSucceeded: 0,
+        itemsApplied: 0,
+        itemsSkipped: 0,
+        itemsWarnings: 0,
+        itemsCancelled: 0,
+        itemsDeferred: 0,
         itemsFailed: 2,
         status: "failure",
       });
@@ -245,6 +274,11 @@ describe("Safe Output Handler Manager", () => {
       });
 
       expect(core.setOutput).toHaveBeenCalledWith("items_succeeded", "10");
+      expect(core.setOutput).toHaveBeenCalledWith("items_applied", "10");
+      expect(core.setOutput).toHaveBeenCalledWith("items_skipped", "0");
+      expect(core.setOutput).toHaveBeenCalledWith("items_warnings", "0");
+      expect(core.setOutput).toHaveBeenCalledWith("items_cancelled", "0");
+      expect(core.setOutput).toHaveBeenCalledWith("items_deferred", "0");
       expect(core.setOutput).toHaveBeenCalledWith("items_failed", "5");
       expect(core.setOutput).toHaveBeenCalledWith("status", "partial_success");
     });
@@ -614,6 +648,36 @@ describe("Safe Output Handler Manager", () => {
       expect(result.results[1].messageIndex).toBe(1);
     });
 
+    it("preserves summary-safe diagnostics from skipped handler results", async () => {
+      const messages = [{ type: "add_comment", item_number: 123 }];
+      const skippedResult = {
+        success: false,
+        skipped: true,
+        reasonCode: "REQUIRED_LABELS_MISMATCH",
+        reason: "Required labels missing",
+        error: "Item does not match required-labels filter",
+        target: { repo: "owner/repo", number: 123 },
+        safeDetails: {
+          requiredLabels: ["automation", "n-plus-1"],
+          missingLabels: ["automation"],
+        },
+      };
+      const handler = vi.fn().mockResolvedValue(skippedResult);
+
+      const result = await processMessages(new Map([["add_comment", handler]]), messages);
+
+      expect(result.success).toBe(true);
+      expect(result.results[0]).toMatchObject({
+        type: "add_comment",
+        messageIndex: 0,
+        success: false,
+        skipped: true,
+        reasonCode: "REQUIRED_LABELS_MISMATCH",
+        reason: "Required labels missing",
+        result: skippedResult,
+      });
+    });
+
     it("should abort non-reviewable outputs in detection warning mode", async () => {
       process.env.GH_AW_DETECTION_CONCLUSION = "warning";
       const messages = [{ type: "merge_pull_request" }, { type: "create_issue", title: "Review this", body: "Body" }];