fix(orb): parse the AMS PR outcome from an anchored dedupKey, not a substring scan

src/notifications/ams-events.ts

@@ -103,6 +103,27 @@ export function buildAmsGovernorPausedEvent(input: {
   };
 }
 
+// The `ams_pr_outcome` dedupKey is the ONLY carrier of the merged/closed decision (DetectedNotificationEvent
+// has no `decision` field), so its exact format is load-bearing. Both the producer below and the reader
+// (parseAmsPrOutcomeDecision) derive from this one prefix so the format is spelled once, not twice (#9703).
+const AMS_PR_OUTCOME_DEDUP_KEY_PREFIX = "ams_pr_outcome";
+
+/** Build the decision-carrying dedupKey. The single source of the `ams_pr_outcome:...:decision:closedAt` shape. */
+function amsPrOutcomeDedupKey(repoFullName: string, pullNumber: number, decision: "merged" | "closed", closedAt: string): string {
+  return `${AMS_PR_OUTCOME_DEDUP_KEY_PREFIX}:${repoFullName}#${pullNumber}:${decision}:${closedAt}`;
+}
+
+/** Anchored matcher for a well-formed `ams_pr_outcome` dedupKey, with the decision as capture group 1. Derives
+ *  its prefix + decision segment from the same constants the producer uses, so the two cannot drift (#9703). */
+export const AMS_PR_OUTCOME_DEDUP_KEY_PATTERN = new RegExp(`^${AMS_PR_OUTCOME_DEDUP_KEY_PREFIX}:.+#\\d+:(merged|closed):.+$`);
+
+/** The single reader of AMS_PR_OUTCOME_DEDUP_KEY_PATTERN: returns the embedded decision, or null when the key is
+ *  malformed (missing/unknown decision segment). A null result must never be rendered as an unproven merge. */
+export function parseAmsPrOutcomeDecision(dedupKey: string): "merged" | "closed" | null {
+  const match = AMS_PR_OUTCOME_DEDUP_KEY_PATTERN.exec(dedupKey);
+  return match ? (match[1] as "merged" | "closed") : null;
+}
+
 /** Miner-local PR outcome change (merged or closed). */
 export function buildAmsPrOutcomeEvent(input: {
   recipientLogin: string;
@@ -120,7 +141,7 @@ export function buildAmsPrOutcomeEvent(input: {
     recipientLogin,
     repoFullName: input.repoFullName,
     pullNumber: input.pullNumber,
-    dedupKey: `ams_pr_outcome:${input.repoFullName}#${input.pullNumber}:${input.decision}:${closedAt}`,
+    dedupKey: amsPrOutcomeDedupKey(input.repoFullName, input.pullNumber, input.decision, closedAt),
     deeplink: githubPullDeeplink(input.repoFullName, input.pullNumber),
     actorLogin: recipientLogin,
     detectedAt,
@@ -140,6 +161,11 @@ export function normalizeAmsNotificationEventInput(
   if (!isAmsNotificationEventType(record.eventType)) return null;
   if (typeof record.repoFullName !== "string" || !record.repoFullName.trim()) return null;
   if (typeof record.dedupKey !== "string" || !record.dedupKey.trim()) return null;
+  // ams_pr_outcome-only: its dedupKey is the sole carrier of the merged/closed decision, so a key that does not
+  // parse would silently render as "closed without merge" (including for a real merge). Reject it at ingest --
+  // the MCP/self-host path reaches this normalizer directly, so validating only in the HTTP zod schema is not
+  // enough. Every other AMS kind keeps its existing non-empty-string check unchanged (#9703).
+  if (record.eventType === "ams_pr_outcome" && parseAmsPrOutcomeDecision(record.dedupKey.trim()) === null) return null;
   if (typeof record.deeplink !== "string" || !record.deeplink.trim()) return null;
   if (typeof record.actorLogin !== "string" || !record.actorLogin.trim()) return null;
   if (typeof record.detectedAt !== "string" || !record.detectedAt.trim()) return null;

src/notifications/service.ts

@@ -1,4 +1,5 @@
 import { sanitizePublicComment } from "../github/commands";
+import { parseAmsPrOutcomeDecision } from "./ams-events";
 import {
   countRecentNotificationDeliveries,
   getNotificationDeliveryById,
@@ -88,8 +89,10 @@ export function buildAmsGovernorPausedNotification(_event: DetectedNotificationE
 
 export function buildAmsPrOutcomeNotification(event: DetectedNotificationEvent): { title: string; body: string } {
   const ref = `${event.repoFullName}#${event.pullNumber}`;
-  // buildAmsPrOutcomeEvent embeds `:merged:` or `:closed:` in the dedupKey after the PR number.
-  const merged = event.dedupKey.includes(":merged:");
+  // The decision is carried in the dedupKey; read it through the single anchored parser rather than a loose
+  // substring scan. A null parse renders the CLOSED copy -- never claim an unproven merge -- though the ingest
+  // normalizer now rejects a malformed ams_pr_outcome key upstream, so that arm is unreachable from ingest (#9703).
+  const merged = parseAmsPrOutcomeDecision(event.dedupKey) === "merged";
   if (merged) {
     return {
       title: sanitizePublicComment(`AMS recorded merge: ${ref}`),

test/unit/miner-ams-notifications.test.ts

@@ -10,6 +10,8 @@ import {
   publishAmsNotificationEvents,
   scheduleAmsNotificationEvents,
 } from "../../packages/loopover-miner/lib/ams-notifications";
+// #9703: the SERVER's anchored decision-parser -- the consumer this producer must stay format-compatible with.
+import { parseAmsPrOutcomeDecision } from "../../src/notifications/ams-events";
 
 const roots: string[] = [];
 
@@ -240,4 +242,14 @@ describe("ams-notifications (#7657)", () => {
   it("returns sent 0 for an empty event list", async () => {
     await expect(publishAmsNotificationEvents([])).resolves.toEqual({ sent: 0 });
   });
+
+  // #9703: the miner producer here is what actually runs; the server consumer recovers the merged/closed
+  // decision by parsing this dedupKey. Pin the producer's output against the server's anchored pattern so a
+  // reorder of decision/closedAt (which would silently flip every merge to "closed without merge") fails loudly.
+  it("buildAmsPrOutcomePayload's dedupKey satisfies the server's AMS_PR_OUTCOME_DEDUP_KEY_PATTERN (drift guard)", () => {
+    for (const decision of ["merged", "closed"] as const) {
+      const payload = buildAmsPrOutcomePayload({ recipientLogin: "miner", repoFullName: "acme/widgets", pullNumber: 7, decision, closedAt: "2026-07-21T00:00:00.000Z" });
+      expect(parseAmsPrOutcomeDecision(payload.dedupKey)).toBe(decision);
+    }
+  });
 });

test/unit/notifications-ams-events.test.ts

@@ -1,11 +1,13 @@
 import { describe, expect, it } from "vitest";
 import {
+  AMS_PR_OUTCOME_DEDUP_KEY_PATTERN,
   buildAmsAttemptFailedEvent,
   buildAmsAttemptStartedEvent,
   buildAmsGovernorPausedEvent,
   buildAmsPrOutcomeEvent,
   isAmsNotificationEventType,
   normalizeAmsNotificationEventInput,
+  parseAmsPrOutcomeDecision,
 } from "../../src/notifications/ams-events";
 
 describe("AMS notification event builders (#7657)", () => {
@@ -100,6 +102,47 @@ describe("AMS notification event builders (#7657)", () => {
     expect(closed.dedupKey).toContain(":closed:");
   });
 
+  it("#9703: parseAmsPrOutcomeDecision reads the decision from a well-formed dedupKey and null otherwise", () => {
+    // The builder's own key round-trips through the parser for both decisions.
+    const merged = buildAmsPrOutcomeEvent({ recipientLogin: "miner", repoFullName: "acme/widgets", pullNumber: 7, decision: "merged" });
+    const closed = buildAmsPrOutcomeEvent({ recipientLogin: "miner", repoFullName: "acme/widgets", pullNumber: 7, decision: "closed" });
+    expect(parseAmsPrOutcomeDecision(merged.dedupKey)).toBe("merged");
+    expect(parseAmsPrOutcomeDecision(closed.dedupKey)).toBe("closed");
+    expect(AMS_PR_OUTCOME_DEDUP_KEY_PATTERN.test(merged.dedupKey)).toBe(true);
+
+    // Malformed keys: wrong prefix, no decision segment, an unknown decision, and the loose-substring trap where
+    // ":merged:" appears somewhere other than the decision segment. All must parse to null, never a false merge.
+    for (const bad of [
+      "k",
+      "ams_pr_outcome:acme/widgets#7", // truncated, no decision/closedAt
+      "ams_pr_outcome:acme/widgets#7:reopened:2026-07-21T00:00:00.000Z", // unknown decision
+      "other:acme/widgets#7:merged:2026-07-21T00:00:00.000Z", // wrong prefix
+      "ams_pr_outcome:acme/widgets:merged:#7:2026", // no #<pullNumber> before the decision
+    ]) {
+      expect(parseAmsPrOutcomeDecision(bad)).toBeNull();
+    }
+  });
+
+  it("#9703: normalizeAmsNotificationEventInput rejects an ams_pr_outcome whose dedupKey has no valid decision", () => {
+    const base = {
+      repoFullName: "acme/widgets",
+      pullNumber: 7,
+      deeplink: "https://example.com",
+      actorLogin: "miner",
+      detectedAt: "2026-07-21T00:00:00.000Z",
+    };
+    // A malformed ams_pr_outcome key would silently render as "closed without merge" -- rejected at ingest.
+    expect(normalizeAmsNotificationEventInput({ ...base, eventType: "ams_pr_outcome", dedupKey: "ams_pr_outcome:no-decision" }, "miner")).toBeNull();
+    // A well-formed one is accepted.
+    const good = normalizeAmsNotificationEventInput(
+      { ...base, eventType: "ams_pr_outcome", dedupKey: "ams_pr_outcome:acme/widgets#7:merged:2026-07-21T00:00:00.000Z" },
+      "miner",
+    );
+    expect(good).not.toBeNull();
+    // The ams_pr_outcome-only rule does not touch other AMS kinds: a loose dedupKey still passes for them.
+    expect(normalizeAmsNotificationEventInput({ ...base, eventType: "ams_attempt_started", dedupKey: "k" }, "miner")).not.toBeNull();
+  });
+
   it("normalizes ingest payloads and rejects non-AMS kinds", () => {
     expect(isAmsNotificationEventType("ams_attempt_started")).toBe(true);
     expect(isAmsNotificationEventType("pull_request_merged")).toBe(false);

test/unit/notifications-service.test.ts

@@ -156,6 +156,15 @@ describe("AMS notification event kinds (#7657)", () => {
     );
     expect(closed.title.toLowerCase()).toContain("close");
     expect(closed.body.toLowerCase()).toContain("without merge");
+
+    // #9703: a malformed dedupKey (no valid decision) renders the CLOSED copy -- never a false merge. This arm
+    // is unreachable from the ingest route (normalizeAmsNotificationEventInput rejects such a key upstream), but
+    // buildAmsPrOutcomeNotification stays fail-safe on its own regardless of how the event was constructed.
+    const malformed = buildNotificationContent(
+      event({ eventType: "ams_pr_outcome", dedupKey: "ams_pr_outcome:owner/repo#7:no-decision-here" }),
+    );
+    expect(malformed.title.toLowerCase()).toContain("close");
+    expect(malformed.body.toLowerCase()).toContain("without merge");
   });
 
   it("evaluates AMS events and enqueues notify-deliver jobs (job-dispatch handoff shape)", async () => {