fix: use distinct XML marker for threat detection engine errors vs real threats

actions/setup/js/generate_footer.test.cjs

@@ -487,7 +487,8 @@ describe("generate_footer.cjs", () => {
 
         expect(result).toContain("> [!WARNING]");
         expect(result).toContain("threat detection engine error");
-        expect(result).toContain("<!-- gh-aw-threat-detected -->");
+        expect(result).toContain("<!-- gh-aw-threat-engine-error -->");
+        expect(result).not.toContain("<!-- gh-aw-threat-detected -->");
         expect(result).not.toContain("> [!CAUTION]");
         expect(result).not.toContain("agentic threat detected");
         expect(result).toContain("failed to produce results");
@@ -501,6 +502,8 @@ describe("generate_footer.cjs", () => {
 
         expect(result).toContain("> [!WARNING]");
         expect(result).toContain("threat detection engine error");
+        expect(result).toContain("<!-- gh-aw-threat-engine-error -->");
+        expect(result).not.toContain("<!-- gh-aw-threat-detected -->");
         expect(result).not.toContain("> [!CAUTION]");
         expect(result).not.toContain("agentic threat detected");
         expect(result).toContain("could not be parsed");

actions/setup/js/messages.test.cjs

@@ -1412,6 +1412,8 @@ describe("messages.cjs", () => {
 
       expect(result).toContain("> [!WARNING]");
       expect(result).toContain("threat detection engine error");
+      expect(result).toContain("<!-- gh-aw-threat-engine-error -->");
+      expect(result).not.toContain("<!-- gh-aw-threat-detected -->");
       expect(result).not.toContain("> [!CAUTION]");
       expect(result).not.toContain("agentic threat detected");
       expect(result).toContain("threat detection engine failed");

actions/setup/js/messages_run_status.cjs

@@ -8,7 +8,7 @@
  */
 
 const { getMessages, renderTemplate, toSnakeCase } = require("./messages_core.cjs");
-const { getDetectionReasonText, getThreatDetectedMarkerTemplate, normalizeThreatKinds, isToolingFailureReason } = require("./threat_detection_warning.cjs");
+const { getDetectionReasonText, getThreatWarningPresentation, normalizeThreatKinds, isToolingFailureReason } = require("./threat_detection_warning.cjs");
 
 /**
  * Renders a message using a custom template from config or a default template.
@@ -152,12 +152,13 @@ function getCommitPushedMessage(ctx) {
  */
 function getDetectionWarningMessage(ctx) {
   const reasonText = getDetectionReasonText(ctx.reason);
+  const presentation = getThreatWarningPresentation(ctx.reason);
   const isEngineError = isToolingFailureReason(ctx.reason);
   if (isEngineError) {
-    const defaultTemplate = `> [!WARNING]\n> threat detection engine error\n> The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.\n> ${getThreatDetectedMarkerTemplate()}\n>\n> <details>\n> <summary>Details</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;
+    const defaultTemplate = `> [!${presentation.admonition}]\n> ${presentation.title}\n> ${presentation.summary}\n> ${presentation.marker}\n>\n> <details>\n> <summary>Details</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;
     return renderConfiguredMessage("detectionEngineError", defaultTemplate, { ...ctx, reasonText, threatKinds: normalizeThreatKinds(ctx.reason) });
   }
-  const defaultTemplate = `> [!CAUTION]\n> agentic threat detected\n> Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.\n> ${getThreatDetectedMarkerTemplate()}\n>\n> <details>\n> <summary>Details</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;
+  const defaultTemplate = `> [!${presentation.admonition}]\n> ${presentation.title}\n> ${presentation.summary}\n> ${presentation.marker}\n>\n> <details>\n> <summary>Details</summary>\n>\n> {reason_text}\n>\n> Review the [workflow run logs]({run_url}) for details.\n> </details>`;
   return renderConfiguredMessage("detectionWarning", defaultTemplate, { ...ctx, reasonText, threatKinds: normalizeThreatKinds(ctx.reason) });
 }
 

actions/setup/js/push_to_pull_request_branch.cjs

@@ -21,7 +21,7 @@ const { withGitHubHostToken } = require("./git_auth_helpers.cjs");
 const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits, isShallowOrSparseCheckout, linearizeRangeAsCommit, ensureSafeDirectoryTrust } = require("./git_helpers.cjs");
 const { normalizeCommitSHA } = require("./commit_sha_helpers.cjs");
 const { findRepoCheckout } = require("./find_repo_checkout.cjs");
-const { getThreatDetectedMarker } = require("./threat_detection_warning.cjs");
+const { getThreatWarningPresentation } = require("./threat_detection_warning.cjs");
 const { attachExecutionState } = require("./safe_output_execution_metadata.cjs");
 const { resolveTransportPaths } = require("./resolve_transport_paths.cjs");
 
@@ -1304,11 +1304,12 @@ async function main(config = {}) {
           // For fork-backed PRs, use an owner-qualified head reference.
           const reviewHeadRef = pushRemoteUrl ? `${pushRepoParts.owner}:${reviewBranchName}` : reviewBranchName;
           const detectionReasonEnv = process.env.GH_AW_DETECTION_REASON || "unknown";
+          const warning = getThreatWarningPresentation(detectionReasonEnv);
           const prBody = [
-            "> [!CAUTION]",
-            "> agentic threat detected",
-            "> Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.",
-            `> ${getThreatDetectedMarker(detectionReasonEnv)}`,
+            `> [!${warning.admonition}]`,
+            `> ${warning.title}`,
+            `> ${warning.summary}`,
+            `> ${warning.marker}`,
             ">",
             `> **Reason:** ${detectionReasonEnv}`,
             ">",

actions/setup/js/push_to_pull_request_branch.test.cjs

@@ -253,6 +253,57 @@ describe("push_to_pull_request_branch.cjs", () => {
     return module;
   }
 
+  async function runFallbackPullRequestScenario(branch, detectionReason = "unknown") {
+    createPatchFile(branch);
+    process.env.GH_AW_DETECTION_REASON = detectionReason;
+
+    mockExec.exec.mockResolvedValueOnce(0); // fetch
+    mockExec.exec.mockResolvedValueOnce(0); // rev-parse
+    mockExec.exec.mockResolvedValueOnce(0); // checkout
+
+    mockExec.getExecOutput.mockResolvedValueOnce({ exitCode: 0, stdout: "before-sha\n", stderr: "" }); // git rev-parse HEAD (before patch)
+
+    mockExec.exec.mockResolvedValueOnce(0); // git am
+
+    const originalGetExecOutput = mockExec.getExecOutput;
+    mockExec.getExecOutput = vi.fn().mockImplementation(async (cmd, args) => {
+      const argList = Array.isArray(args) ? args : [];
+      if (argList[0] === "rev-parse" && argList[1] === "origin/feature-branch^{commit}") {
+        return { exitCode: 0, stdout: "1111111111111111111111111111111111111111\n", stderr: "" };
+      }
+      if (argList[0] === "rev-list" && argList[1] === "--merges") {
+        return { exitCode: 0, stdout: "0\n", stderr: "" };
+      }
+      if (argList[0] === "rev-list" && argList[1] === "--parents") {
+        return {
+          exitCode: 0,
+          stdout: "2222222222222222222222222222222222222222 1111111111111111111111111111111111111111\n",
+          stderr: "",
+        };
+      }
+      if (argList[0] === "ls-remote" && argList[2] === "refs/heads/feature-branch") {
+        return { exitCode: 0, stdout: "1111111111111111111111111111111111111111\trefs/heads/feature-branch\n", stderr: "" };
+      }
+      if (argList[0] === "log") {
+        if (argList.includes(".github/workflows/")) {
+          return { exitCode: 0, stdout: "", stderr: "" };
+        }
+        return { exitCode: 0, stdout: "Test commit\n", stderr: "" };
+      }
+      if (argList[0] === "diff-tree") {
+        return { exitCode: 0, stdout: "", stderr: "" };
+      }
+      return originalGetExecOutput(cmd, args);
+    });
+
+    mockGithub.graphql.mockRejectedValueOnce(new Error("GraphQL error: branch protection"));
+    mockExec.exec.mockRejectedValueOnce(new Error("! [rejected] feature-branch -> feature-branch (non-fast-forward)"));
+
+    const module = await loadModule();
+    const handler = await module.main({});
+    return handler({ branch }, {});
+  }
+
   /**
    * Helper to create a valid patch file at the canonical path derived from the
    * message branch. The privileged handler always re-derives the patch path
@@ -1171,59 +1222,7 @@ index 0000000..abc1234
     });
 
     it("should create fallback pull request on non-fast-forward push rejection by default", async () => {
-      const patchPath = createPatchFile("should-create-fallback-pull-request-on-non-fast-forward-push");
-
-      // Set up successful operations until push
-      mockExec.exec.mockResolvedValueOnce(0); // fetch
-      mockExec.exec.mockResolvedValueOnce(0); // rev-parse
-      mockExec.exec.mockResolvedValueOnce(0); // checkout
-
-      mockExec.getExecOutput.mockResolvedValueOnce({ exitCode: 0, stdout: "before-sha\n", stderr: "" }); // git rev-parse HEAD (before patch)
-
-      mockExec.exec.mockResolvedValueOnce(0); // git am
-
-      const originalGetExecOutput = mockExec.getExecOutput;
-      mockExec.getExecOutput = vi.fn().mockImplementation(async (cmd, args) => {
-        const argList = Array.isArray(args) ? args : [];
-        if (argList[0] === "rev-parse" && argList[1] === "origin/feature-branch^{commit}") {
-          return { exitCode: 0, stdout: "1111111111111111111111111111111111111111\n", stderr: "" };
-        }
-        if (argList[0] === "rev-list" && argList[1] === "--merges") {
-          return { exitCode: 0, stdout: "0\n", stderr: "" };
-        }
-        if (argList[0] === "rev-list" && argList[1] === "--parents") {
-          return {
-            exitCode: 0,
-            stdout: "2222222222222222222222222222222222222222 1111111111111111111111111111111111111111\n",
-            stderr: "",
-          };
-        }
-        if (argList[0] === "ls-remote" && argList[2] === "refs/heads/feature-branch") {
-          return { exitCode: 0, stdout: "1111111111111111111111111111111111111111\trefs/heads/feature-branch\n", stderr: "" };
-        }
-        if (argList[0] === "log") {
-          // Pre-flight workflow check targets .github/workflows/; return empty to avoid
-          // short-circuiting the fallback path with a workflows_scope_required error.
-          if (argList.includes(".github/workflows/")) {
-            return { exitCode: 0, stdout: "", stderr: "" };
-          }
-          return { exitCode: 0, stdout: "Test commit\n", stderr: "" };
-        }
-        if (argList[0] === "diff-tree") {
-          return { exitCode: 0, stdout: "", stderr: "" };
-        }
-        return originalGetExecOutput(cmd, args);
-      });
-
-      // GraphQL call fails, triggering fallback to git push
-      mockGithub.graphql.mockRejectedValueOnce(new Error("GraphQL error: branch protection"));
-
-      // Fallback git push also fails with non-fast-forward
-      mockExec.exec.mockRejectedValueOnce(new Error("! [rejected] feature-branch -> feature-branch (non-fast-forward)"));
-
-      const module = await loadModule();
-      const handler = await module.main({});
-      const result = await handler({ branch: "should-create-fallback-pull-request-on-non-fast-forward-push" }, {});
+      const result = await runFallbackPullRequestScenario("should-create-fallback-pull-request-on-non-fast-forward-push");
 
       expect(result.success).toBe(true);
       expect(result.fallback_used).toBe(true);
@@ -1466,6 +1465,27 @@ index 0000000..abc1234
       mockExec.getExecOutput = savedGetExecOutput;
     });
 
+    it.each([
+      ["agent_failure", "> [!WARNING]", "> threat detection engine error", "<!-- gh-aw-threat-engine-error -->", "<!-- gh-aw-threat-detected -->"],
+      ["threat_detected", "> [!CAUTION]", "> agentic threat detected", "<!-- gh-aw-threat-detected -->", "<!-- gh-aw-threat-engine-error -->"],
+    ])("should create a review PR body for %s with the correct admonition and marker", async (reason, admonition, title, expectedMarker, unexpectedMarker) => {
+      process.env.GH_AW_DETECTION_CONCLUSION = "warning";
+      process.env.GH_AW_DETECTION_REASON = reason;
+      mockContext.runId = 12345;
+      createPatchFile(`review-branch-body-${reason}`);
+
+      const module = await loadModule();
+      const handler = await module.main({});
+      const result = await handler({ branch: `review-branch-body-${reason}` }, {});
+
+      expect(result.success).toBe(true);
+      const [params] = mockGithub.rest.pulls.create.mock.calls.at(-1);
+      expect(params.body).toContain(admonition);
+      expect(params.body).toContain(title);
+      expect(params.body).toContain(expectedMarker);
+      expect(params.body).not.toContain(unexpectedMarker);
+    });
+
     it("should skip non-fatally when review branch is rejected for workflows scope (timeout variant, agent has none)", async () => {
       process.env.GH_AW_DETECTION_CONCLUSION = "warning";
       createPatchFile("review-branch-workflows-scope-timeout");

actions/setup/js/threat_detection_warning.cjs

@@ -20,25 +20,6 @@ function normalizeThreatKinds(reason) {
   return kinds.length > 0 ? Array.from(new Set(kinds)).join(",") : "unknown";
 }
 
-/**
- * Returns the XML marker used to identify threat-detected output.
- *
- * @param {string | undefined | null} reason
- * @returns {string}
- */
-function getThreatDetectedMarker(reason) {
-  return "<!-- gh-aw-threat-detected -->";
-}
-
-/**
- * Returns the marker template for configured message rendering.
- *
- * @returns {string}
- */
-function getThreatDetectedMarkerTemplate() {
-  return "<!-- gh-aw-threat-detected -->";
-}
-
 /**
  * Returns a human-readable reason text for detection warnings.
  *
@@ -69,10 +50,85 @@ function isToolingFailureReason(reason) {
   return normalized === "agent_failure" || normalized === "parse_error";
 }
 
+/**
+ * Returns the XML marker used to identify threat-engine-error output.
+ * This marker is distinct from the real-threat marker so that automated tools
+ * can distinguish a tooling failure from an actual security finding.
+ *
+ * @returns {string}
+ */
+function getThreatEngineErrorMarker() {
+  return "<!-- gh-aw-threat-engine-error -->";
+}
+
+/**
+ * Returns the marker template for configured engine-error message rendering.
+ *
+ * @returns {string}
+ */
+function getThreatEngineErrorMarkerTemplate() {
+  return "<!-- gh-aw-threat-engine-error -->";
+}
+
+/**
+ * Returns the review-warning presentation associated with a detection reason.
+ * Centralizing these fields keeps admonition copy and marker routing in sync
+ * across status messages, footers, and fallback pull request bodies.
+ *
+ * @param {string | undefined | null} reason
+ * @returns {{admonition: string, title: string, summary: string, marker: string}}
+ */
+function getThreatWarningPresentation(reason) {
+  if (isToolingFailureReason(reason)) {
+    return {
+      admonition: "WARNING",
+      title: "threat detection engine error",
+      summary: "The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.",
+      marker: getThreatEngineErrorMarker(),
+    };
+  }
+  return {
+    admonition: "CAUTION",
+    title: "agentic threat detected",
+    summary: "Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.",
+    marker: getThreatDetectedMarker(reason),
+  };
+}
+
+/**
+ * Returns the XML marker used to identify threat-detected output.
+ * When the reason indicates a tooling failure (agent_failure, parse_error) a
+ * distinct engine-error marker is returned so automated tools can distinguish
+ * "detection engine crashed" from "detection engine found something".
+ *
+ * @param {string | undefined | null} reason
+ * @returns {string}
+ */
+function getThreatDetectedMarker(reason) {
+  if (isToolingFailureReason(reason)) {
+    return getThreatEngineErrorMarker();
+  }
+  return "<!-- gh-aw-threat-detected -->";
+}
+
+/**
+ * Returns the marker template for configured message rendering.
+ * Always returns the real-threat marker; use getThreatEngineErrorMarkerTemplate()
+ * for tooling-failure templates where the reason is known at template-build time.
+ *
+ * @returns {string}
+ */
+function getThreatDetectedMarkerTemplate() {
+  return "<!-- gh-aw-threat-detected -->";
+}
+
 module.exports = {
   normalizeThreatKinds,
+  getThreatWarningPresentation,
   getThreatDetectedMarker,
   getThreatDetectedMarkerTemplate,
+  getThreatEngineErrorMarker,
+  getThreatEngineErrorMarkerTemplate,
   getDetectionReasonText,
   isToolingFailureReason,
 };

actions/setup/js/threat_detection_warning.test.cjs

@@ -1,5 +1,14 @@
 import { describe, it, expect } from "vitest";
-import { normalizeThreatKinds, getThreatDetectedMarker, getThreatDetectedMarkerTemplate, getDetectionReasonText, isToolingFailureReason } from "./threat_detection_warning.cjs";
+import {
+  normalizeThreatKinds,
+  getThreatWarningPresentation,
+  getThreatDetectedMarker,
+  getThreatDetectedMarkerTemplate,
+  getThreatEngineErrorMarker,
+  getThreatEngineErrorMarkerTemplate,
+  getDetectionReasonText,
+  isToolingFailureReason,
+} from "./threat_detection_warning.cjs";
 
 describe("threat_detection_warning", () => {
   describe("normalizeThreatKinds", () => {
@@ -15,10 +24,38 @@ describe("threat_detection_warning", () => {
   });
 
   describe("marker helpers", () => {
-    it("emits the normative threat marker", () => {
-      expect(getThreatDetectedMarker("threat_detected,parse_error")).toBe("<!-- gh-aw-threat-detected -->");
+    it("emits the normative threat marker for real threats", () => {
+      expect(getThreatDetectedMarker("threat_detected")).toBe("<!-- gh-aw-threat-detected -->");
+      expect(getThreatDetectedMarker(null)).toBe("<!-- gh-aw-threat-detected -->");
+      expect(getThreatDetectedMarker(undefined)).toBe("<!-- gh-aw-threat-detected -->");
+      expect(getThreatDetectedMarker("")).toBe("<!-- gh-aw-threat-detected -->");
       expect(getThreatDetectedMarkerTemplate()).toBe("<!-- gh-aw-threat-detected -->");
     });
+
+    it("emits the engine-error marker for tooling failures", () => {
+      expect(getThreatDetectedMarker("agent_failure")).toBe("<!-- gh-aw-threat-engine-error -->");
+      expect(getThreatDetectedMarker("parse_error")).toBe("<!-- gh-aw-threat-engine-error -->");
+    });
+
+    it("getThreatEngineErrorMarker always returns the engine-error marker", () => {
+      expect(getThreatEngineErrorMarker()).toBe("<!-- gh-aw-threat-engine-error -->");
+      expect(getThreatEngineErrorMarkerTemplate()).toBe("<!-- gh-aw-threat-engine-error -->");
+    });
+
+    it("returns a centralized warning presentation for tooling failures and threats", () => {
+      expect(getThreatWarningPresentation("agent_failure")).toEqual({
+        admonition: "WARNING",
+        title: "threat detection engine error",
+        summary: "The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.",
+        marker: "<!-- gh-aw-threat-engine-error -->",
+      });
+      expect(getThreatWarningPresentation("threat_detected")).toEqual({
+        admonition: "CAUTION",
+        title: "agentic threat detected",
+        summary: "Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.",
+        marker: "<!-- gh-aw-threat-detected -->",
+      });
+    });
   });
 
   describe("getDetectionReasonText", () => {