Include canonical issue IDs in duplicate close intents

actions/setup/js/close_issue.cjs

@@ -13,6 +13,7 @@ const { loadTemporaryIdMapFromResolved, resolveRepoIssueTarget } = require("./te
 const { getErrorMessage } = require("./error_helpers.cjs");
 const { normalizeIssueIntentMetadata } = require("./issue_intents.cjs");
 const { resolveTarget } = require("./safe_output_helpers.cjs");
+const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
 
 /**
  * Parse a `duplicate_of` value into { owner, repo, issueNumber }.
@@ -67,22 +68,31 @@ function parseDuplicateOf(value, defaultOwner, defaultRepo) {
  * This creates a native "marked this as a duplicate of #X" timeline event.
  *
  * @param {any} github - Authenticated GitHub client (must support graphql)
- * @param {string} duplicateNodeId - GraphQL node ID of the issue being marked as a duplicate
+ * @param {string|undefined} duplicateNodeId - GraphQL node ID of the issue being marked as a duplicate
  * @param {string} canonicalOwner - Owner of the canonical issue's repo
  * @param {string} canonicalRepo - Repo of the canonical issue
  * @param {number} canonicalNumber - Issue number that is the canonical original
  * @returns {Promise<void>}
  */
-async function markIssueAsDuplicate(github, duplicateNodeId, canonicalOwner, canonicalRepo, canonicalNumber) {
+async function markIssueAsDuplicate(github, duplicateNodeId, canonicalOwner, canonicalRepo, canonicalNumber, canonicalIssueData) {
   if (!duplicateNodeId) {
     throw new Error(`${ERR_NOT_FOUND}: node_id missing for duplicate issue`);
   }
 
-  const { data: canonicalData } = await github.rest.issues.get({
-    owner: canonicalOwner,
-    repo: canonicalRepo,
-    issue_number: canonicalNumber,
-  });
+  let canonicalData = canonicalIssueData;
+  if (!canonicalData) {
+    const response = await withRetry(
+      () =>
+        github.rest.issues.get({
+          owner: canonicalOwner,
+          repo: canonicalRepo,
+          issue_number: canonicalNumber,
+        }),
+      RATE_LIMIT_RETRY_CONFIG,
+      `get canonical issue ${canonicalOwner}/${canonicalRepo}#${canonicalNumber}`
+    );
+    canonicalData = response.data;
+  }
 
   const canonicalNodeId = canonicalData.node_id;
   if (!canonicalNodeId) {
@@ -108,6 +118,23 @@ async function markIssueAsDuplicate(github, duplicateNodeId, canonicalOwner, can
   );
 }
 
+function resolveEligibleCanonicalIssue(parsed, owner, repo, issueNumber, defaultTargetRepo, allowedRepos) {
+  if (!parsed) {
+    return { canonicalIssue: undefined, reason: "duplicate_of value could not be parsed" };
+  }
+
+  const canonicalRepoSlug = `${parsed.owner}/${parsed.repo}`;
+  if (!validateRepo(canonicalRepoSlug, defaultTargetRepo, allowedRepos).valid) {
+    return { canonicalIssue: undefined, reason: `canonical repo "${canonicalRepoSlug}" is not in the allowed-repos list` };
+  }
+
+  if (parsed.owner.toLowerCase() === owner.toLowerCase() && parsed.repo.toLowerCase() === repo.toLowerCase() && parsed.issueNumber === issueNumber) {
+    return { canonicalIssue: undefined, reason: `issue #${issueNumber} cannot be a duplicate of itself` };
+  }
+
+  return { canonicalIssue: parsed, reason: undefined };
+}
+
 /**
  * Get issue details using REST API
  * @param {any} github - GitHub REST API instance
@@ -157,9 +184,9 @@ async function addIssueComment(github, owner, repo, issueNumber, message) {
  * @param {string} repo - Repository name
  * @param {number} issueNumber - Issue number
  * @param {string} stateReason - The reason for closing: "COMPLETED", "NOT_PLANNED", or "DUPLICATE"
- * @returns {Promise<{number: number, html_url: string, title: string, node_id: string}>} Issue details
+ * @returns {Promise<{issue: {number: number, html_url: string, title: string, node_id?: string}, canonicalIssueData?: {id?: number, node_id?: string}}>} Close result and fetched canonical issue details
  */
-async function closeIssue(github, owner, repo, issueNumber, stateReason, intentMetadata, useIssueIntent) {
+async function closeIssue(github, owner, repo, issueNumber, stateReason, intentMetadata, useIssueIntent, canonicalIssue) {
   const baseParams = {
     owner,
     repo,
@@ -169,7 +196,30 @@ async function closeIssue(github, owner, repo, issueNumber, stateReason, intentM
   };
 
   const hasIntentMetadata = Boolean(intentMetadata && Object.keys(intentMetadata).length > 0);
+  let canonicalIssueData;
   if (useIssueIntent && hasIntentMetadata) {
+    let duplicateIssueId;
+    if (baseParams.state_reason === "duplicate" && !canonicalIssue) {
+      throw new Error(`${ERR_VALIDATION}: duplicate close intent requires a valid, allowed, non-self duplicate_of reference`);
+    }
+    if (canonicalIssue) {
+      const { data } = await withRetry(
+        () =>
+          github.rest.issues.get({
+            owner: canonicalIssue.owner,
+            repo: canonicalIssue.repo,
+            issue_number: canonicalIssue.issueNumber,
+          }),
+        RATE_LIMIT_RETRY_CONFIG,
+        `get canonical issue ${canonicalIssue.owner}/${canonicalIssue.repo}#${canonicalIssue.issueNumber}`
+      );
+      canonicalIssueData = data;
+      duplicateIssueId = data.id;
+      if (!duplicateIssueId) {
+        throw new Error(`${ERR_NOT_FOUND}: id missing for canonical issue ${canonicalIssue.owner}/${canonicalIssue.repo}#${canonicalIssue.issueNumber}`);
+      }
+    }
+
     try {
       core.debug("Attempting issue-intent close request");
       const { data: issue } = await github.request("PATCH /repos/{owner}/{repo}/issues/{issue_number}", {
@@ -178,9 +228,10 @@ async function closeIssue(github, owner, repo, issueNumber, stateReason, intentM
         issue_number: issueNumber,
         state: { value: "closed", ...intentMetadata },
         state_reason: baseParams.state_reason,
+        ...(duplicateIssueId !== undefined ? { duplicate_issue_id: duplicateIssueId } : {}),
       });
       core.debug("Issue-intent close request succeeded");
-      return issue;
+      return { issue, canonicalIssueData };
     } catch (error) {
       const errorRecord = error && typeof error === "object" ? /** @type {Record<string, unknown>} */ error : undefined;
       const response = errorRecord?.response;
@@ -196,7 +247,7 @@ async function closeIssue(github, owner, repo, issueNumber, stateReason, intentM
 
   const { data: issue } = await github.rest.issues.update(baseParams);
 
-  return issue;
+  return { issue, canonicalIssueData: undefined };
 }
 
 /**
@@ -351,46 +402,35 @@ async function main(config = {}) {
         const intentMetadata = issueIntentEnabled ? normalizeIssueIntentMetadata(item) : {};
         core.info(`Closing issue #${entityNumber} with state_reason=${stateReason}`);
 
-        const closePromise = closeIssue(github, owner, repo, entityNumber, stateReason, intentMetadata, issueIntentEnabled);
+        const stateReasonUpper = stateReason.toUpperCase();
+        const parsed = stateReasonUpper === "DUPLICATE" ? parseDuplicateOf(item.duplicate_of, owner, repo) : null;
+        const canonicalIssueResolution = resolveEligibleCanonicalIssue(parsed, owner, repo, entityNumber, defaultTargetRepo, allowedRepos);
+        const canonicalIssue = canonicalIssueResolution.canonicalIssue;
+        const closePromise = closeIssue(github, owner, repo, entityNumber, stateReason, intentMetadata, issueIntentEnabled, canonicalIssue);
 
         // When duplicate_of is provided and state_reason is DUPLICATE, create the native duplicate relationship
-        const stateReasonUpper = stateReason.toUpperCase();
         if (item.duplicate_of !== undefined && item.duplicate_of !== null && stateReasonUpper !== "DUPLICATE") {
           core.warning(`duplicate_of is set but state_reason is ${stateReason} (not DUPLICATE); native duplicate marking will be skipped`);
         }
         if (item.duplicate_of !== undefined && item.duplicate_of !== null && stateReasonUpper === "DUPLICATE") {
-          const parsed = parseDuplicateOf(item.duplicate_of, owner, repo);
-          if (parsed) {
-            // Validate canonical repo against the same allowedRepos policy
-            const canonicalRepoSlug = `${parsed.owner}/${parsed.repo}`;
-            const repoValidation = validateRepo(canonicalRepoSlug, defaultTargetRepo, allowedRepos);
-            if (!repoValidation.valid) {
-              core.warning(`Skipping native duplicate marking: canonical repo "${canonicalRepoSlug}" is not in the allowed-repos list`);
-              return closePromise;
-            }
-
-            // Guard against marking an issue as a duplicate of itself
-            if (parsed.owner === owner && parsed.repo === repo && parsed.issueNumber === entityNumber) {
-              core.warning(`Skipping native duplicate marking: issue #${entityNumber} cannot be a duplicate of itself`);
-              return closePromise;
-            }
-
-            core.info(`Marking issue #${entityNumber} as duplicate of ${parsed.owner}/${parsed.repo}#${parsed.issueNumber}`);
-            return closePromise.then(async closedEntity => {
-              try {
-                await markIssueAsDuplicate(github, closedEntity.node_id, parsed.owner, parsed.repo, parsed.issueNumber);
-                core.info(`✓ Marked issue #${entityNumber} as duplicate of ${parsed.owner}/${parsed.repo}#${parsed.issueNumber}`);
-              } catch (dupError) {
-                core.warning(`Failed to mark native duplicate relationship for #${entityNumber}: ${getErrorMessage(dupError)}`);
-              }
-              return closedEntity;
-            });
+          if (!canonicalIssue) {
+            core.warning(`Skipping native duplicate marking: ${canonicalIssueResolution.reason}`);
           } else {
-            core.warning(`duplicate_of value "${item.duplicate_of}" could not be parsed; skipping native duplicate marking`);
+            core.info(`Marking issue #${entityNumber} as duplicate of ${canonicalIssue.owner}/${canonicalIssue.repo}#${canonicalIssue.issueNumber}`);
           }
         }
 
-        return closePromise;
+        return closePromise.then(async ({ issue: closedEntity, canonicalIssueData }) => {
+          if (canonicalIssue) {
+            try {
+              await markIssueAsDuplicate(github, closedEntity.node_id, canonicalIssue.owner, canonicalIssue.repo, canonicalIssue.issueNumber, canonicalIssueData);
+              core.info(`✓ Marked issue #${entityNumber} as duplicate of ${canonicalIssue.owner}/${canonicalIssue.repo}#${canonicalIssue.issueNumber}`);
+            } catch (dupError) {
+              core.warning(`Failed to mark native duplicate relationship for #${entityNumber}: ${getErrorMessage(dupError)}`);
+            }
+          }
+          return closedEntity;
+        });
       },
 
       continueOnCommentError: false,

actions/setup/js/close_issue.test.cjs

@@ -1189,6 +1189,107 @@ describe("close_issue", () => {
   });
 
   describe("duplicate_of native marking", () => {
+    it.each([50, "50", "#50", "test-owner/test-repo#50", "https://github.com/test-owner/test-repo/issues/50", "other-org/other-repo#50"])("should resolve %s to a database ID before sending a duplicate close intent", async duplicateOf => {
+      const handler = await main({ allowed_repos: ["other-org/other-repo"] });
+      const getCalls = [];
+      const requestCalls = [];
+      const graphqlCalls = [];
+      mockGithub.rest.issues.get = async params => {
+        getCalls.push(params);
+        return { data: { id: 100000 + params.issue_number, node_id: `node_${params.issue_number}`, number: params.issue_number, title: "Test Issue", labels: [], state: "open" } };
+      };
+      mockGithub.request = async (route, params) => {
+        expect(getCalls).toHaveLength(2);
+        requestCalls.push({ route, params });
+        return { data: { number: params.issue_number, node_id: `node_${params.issue_number}`, title: "Test Issue", state: "open" } };
+      };
+      mockGithub.graphql = async (mutation, variables) => graphqlCalls.push(variables);
+
+      const result = await handler({ issue_number: 100, body: "Duplicate", state_reason: "duplicate", duplicate_of: duplicateOf, suggest: true, rationale: "Same defect", confidence: "high" }, {});
+
+      expect(result.success).toBe(true);
+      expect(getCalls[1]).toEqual({
+        owner: String(duplicateOf).startsWith("other-org/") ? "other-org" : "test-owner",
+        repo: String(duplicateOf).startsWith("other-org/") ? "other-repo" : "test-repo",
+        issue_number: 50,
+      });
+      expect(requestCalls).toEqual([
+        {
+          route: "PATCH /repos/{owner}/{repo}/issues/{issue_number}",
+          params: {
+            owner: "test-owner",
+            repo: "test-repo",
+            issue_number: 100,
+            state: { value: "closed", suggest: true, rationale: "Same defect", confidence: "HIGH" },
+            state_reason: "duplicate",
+            duplicate_issue_id: 100050,
+          },
+        },
+      ]);
+      expect(getCalls).toHaveLength(2);
+      expect(graphqlCalls).toHaveLength(1);
+      expect(graphqlCalls[0].canonicalId).toBe("node_50");
+    });
+
+    it.each([403, 404, "missing ID"])("should fail closed when canonical issue lookup fails with %s", async failure => {
+      const handler = await main();
+      const mutations = [];
+      mockGithub.rest.issues.get = async ({ issue_number }) => {
+        if (issue_number === 50 && typeof failure === "number") {
+          throw Object.assign(new Error("Canonical issue unavailable"), { status: failure });
+        }
+        return { data: { number: issue_number, title: "Test Issue", labels: [], state: "open" } };
+      };
+      mockGithub.request = async () => mutations.push("intent");
+      mockGithub.rest.issues.update = async () => mutations.push("legacy close");
+      mockGithub.graphql = async () => mutations.push("duplicate marking");
+
+      const result = await handler({ issue_number: 100, body: "Duplicate", state_reason: "duplicate", duplicate_of: 50, suggest: true }, {});
+
+      expect(result.success).toBe(false);
+      expect(result.error).toContain(failure === "missing ID" ? "id missing for canonical issue" : "Canonical issue unavailable");
+      expect(mutations).toEqual([]);
+    });
+
+    it.each([
+      { fields: { duplicate_of: "invalid" }, rejected: true },
+      { fields: { duplicate_of: 100 }, rejected: true },
+      { fields: { duplicate_of: "Test-Owner/Test-Repo#100" }, rejected: true },
+      { fields: { duplicate_of: "other-org/other-repo#50" }, rejected: true },
+      { fields: { duplicate_of: 50, state_reason: "completed" }, rejected: false },
+    ])("should handle an ineligible duplicate reference safely: %j", async ({ fields, rejected }) => {
+      const handler = await main();
+      const getCalls = [];
+      const requestCalls = [];
+      const updateCalls = [];
+      mockGithub.rest.issues.get = async params => {
+        getCalls.push(params);
+        return { data: { number: params.issue_number, title: "Test Issue", labels: [], state: "open" } };
+      };
+      mockGithub.request = async (route, params) => {
+        requestCalls.push(params);
+        return { data: { number: params.issue_number, title: "Test Issue" } };
+      };
+      mockGithub.rest.issues.update = async params => {
+        updateCalls.push(params);
+        return { data: { number: params.issue_number, title: "Test Issue" } };
+      };
+
+      const result = await handler({ issue_number: 100, body: "Close", state_reason: "duplicate", suggest: true, ...fields }, {});
+
+      expect(getCalls).toHaveLength(1);
+      if (rejected) {
+        expect(result.success).toBe(false);
+        expect(result.error).toContain("duplicate close intent requires a valid");
+        expect(requestCalls).toHaveLength(0);
+        expect(updateCalls).toHaveLength(0);
+      } else {
+        expect(result.success).toBe(true);
+        expect(requestCalls).toHaveLength(1);
+        expect(requestCalls[0].duplicate_issue_id).toBeUndefined();
+      }
+    });
+
     it("should call markAsDuplicate mutation when duplicate_of and state_reason DUPLICATE are set", async () => {
       const handler = await main({ max: 10, state_reason: "DUPLICATE" });
       const graphqlCalls = [];
@@ -1520,6 +1621,7 @@ describe("close_issue", () => {
 
       mockGithub.rest.issues.get = async ({ owner, repo, issue_number }) => ({
         data: {
+          id: 100000 + issue_number,
           number: issue_number,
           title: "Test Issue",
           labels: [],
@@ -1563,6 +1665,7 @@ describe("close_issue", () => {
       expect(requestCalls).toHaveLength(1);
       expect(requestCalls[0].route).toBe("PATCH /repos/{owner}/{repo}/issues/{issue_number}");
       expect(requestCalls[0].params.state).toMatchObject({ value: "closed", suggest: true, rationale: "Same CSV defect.", confidence: "HIGH" });
+      expect(requestCalls[0].params.duplicate_issue_id).toBe(100050);
 
       // Verify the native duplicate GraphQL mutation was also executed
       expect(graphqlCalls.length).toBeGreaterThan(0);