feat(add-comment): support reply_to_id for discussion threading from any trigger (#24367)

Author: Copilot

actions/setup/js/add_comment.cjs

@@ -573,6 +573,13 @@ async function main(config = {}) {
       return { success: true, commentId: comment.id, url: comment.html_url, itemNumber, repo: itemRepo, isDiscussion: isDiscussionFlag };
     };
 
+    // Normalize reply_to_id once so both the main discussion path and the
+    // 404 discussion fallback path use the same validated value.
+    const normalizedExplicitReplyToId = message.reply_to_id === undefined || message.reply_to_id === null ? null : String(message.reply_to_id).trim();
+    if (message.reply_to_id !== undefined && message.reply_to_id !== null && !normalizedExplicitReplyToId) {
+      core.warning("Ignoring empty discussion reply_to_id after normalization");
+    }
+
     try {
       // Hide older comments if enabled AND append-only-comments is not enabled
       // When append-only-comments is true, we want to keep all comments visible
@@ -590,7 +597,18 @@ async function main(config = {}) {
         // GitHub Discussions only supports two nesting levels, so if the triggering comment is
         // itself a reply, we resolve the top-level parent's node ID to use as replyToId.
         const hasExplicitItemNumber = message.item_number !== undefined && message.item_number !== null;
-        const replyToId = context.eventName === "discussion_comment" && !hasExplicitItemNumber ? await resolveTopLevelDiscussionCommentId(githubClient, context.payload?.comment?.node_id) : null;
+        let replyToId;
+        if (context.eventName === "discussion_comment" && !hasExplicitItemNumber) {
+          // When triggered by a discussion_comment event, thread the reply under the triggering comment.
+          replyToId = await resolveTopLevelDiscussionCommentId(githubClient, context.payload?.comment?.node_id);
+        } else if (normalizedExplicitReplyToId) {
+          // Allow the agent to explicitly specify a reply_to_id (e.g. for workflow_dispatch-triggered
+          // workflows that know the target comment node ID). Apply resolveTopLevelDiscussionCommentId
+          // to handle cases where the caller passes a reply node ID instead of a top-level one.
+          replyToId = await resolveTopLevelDiscussionCommentId(githubClient, normalizedExplicitReplyToId);
+        } else {
+          replyToId = null;
+        }
         if (replyToId) {
           core.info(`Replying as threaded comment to discussion comment node ID: ${replyToId}`);
         }
@@ -621,7 +639,13 @@ async function main(config = {}) {
 
         try {
           core.info(`Trying #${itemNumber} as discussion...`);
-          const comment = await commentOnDiscussion(githubClient, repoParts.owner, repoParts.repo, itemNumber, processedBody, null);
+          // When retrying as discussion, honour the normalized reply_to_id from the message.
+          // Apply resolveTopLevelDiscussionCommentId to handle nested reply node IDs.
+          const fallbackReplyToId = normalizedExplicitReplyToId ? await resolveTopLevelDiscussionCommentId(githubClient, normalizedExplicitReplyToId) : null;
+          if (fallbackReplyToId) {
+            core.info(`Replying as threaded comment to discussion comment node ID: ${fallbackReplyToId}`);
+          }
+          const comment = await commentOnDiscussion(githubClient, repoParts.owner, repoParts.repo, itemNumber, processedBody, fallbackReplyToId);
 
           core.info(`Created comment on discussion: ${comment.html_url}`);
           return recordComment(comment, true);

actions/setup/js/add_comment.test.cjs

@@ -594,6 +594,311 @@ describe("add_comment", () => {
       // Should NOT be threaded since item_number was explicitly provided
       expect(capturedReplyToId).toBeUndefined();
     });
+
+    it("should use reply_to_id from message when not triggered by discussion_comment event", async () => {
+      const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
+
+      // workflow_dispatch trigger (not discussion_comment); item_number refers to a discussion
+      mockContext.eventName = "workflow_dispatch";
+      mockContext.payload = {};
+
+      // Make REST API return 404 so the code falls back to the discussion path
+      mockGithub.rest.issues.createComment = async () => {
+        const err = new Error("Not Found");
+        // @ts-expect-error - Simulating GitHub REST API error
+        err.status = 404;
+        throw err;
+      };
+
+      let capturedReplyToId = undefined;
+      mockGithub.graphql = async (query, variables) => {
+        if (query.includes("addDiscussionComment")) {
+          capturedReplyToId = variables?.replyToId;
+          return {
+            addDiscussionComment: {
+              comment: {
+                id: "DC_kwDOTest456",
+                body: variables?.body,
+                createdAt: "2024-01-01",
+                url: "https://github.com/owner/repo/discussions/10#discussioncomment-456",
+              },
+            },
+          };
+        }
+        // Mock replyTo resolution: the provided reply_to_id is already top-level (no parent)
+        if (query.includes("replyTo")) {
+          return { node: { replyTo: null } };
+        }
+        return {
+          repository: {
+            discussion: {
+              id: "D_kwDOTest123",
+              url: "https://github.com/owner/repo/discussions/10",
+            },
+          },
+        };
+      };
+
+      const handler = await eval(`(async () => { ${addCommentScript}; return await main({}); })()`);
+
+      const message = {
+        type: "add_comment",
+        item_number: 10,
+        body: "🔄 Updated finding...",
+        reply_to_id: "DC_kwDOParentComment456",
+      };
+
+      const result = await handler(message, {});
+
+      expect(result.success).toBe(true);
+      expect(result.isDiscussion).toBe(true);
+      // Should use the reply_to_id from the message
+      expect(capturedReplyToId).toBe("DC_kwDOParentComment456");
+    });
+
+    it("should resolve top-level parent when reply_to_id points to a nested reply", async () => {
+      const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
+
+      // workflow_dispatch trigger (not discussion_comment); item_number refers to a discussion
+      mockContext.eventName = "workflow_dispatch";
+      mockContext.payload = {};
+
+      // Make REST API return 404 so the code falls back to the discussion path
+      mockGithub.rest.issues.createComment = async () => {
+        const err = new Error("Not Found");
+        // @ts-expect-error - Simulating GitHub REST API error
+        err.status = 404;
+        throw err;
+      };
+
+      let capturedReplyToId = undefined;
+      mockGithub.graphql = async (query, variables) => {
+        if (query.includes("addDiscussionComment")) {
+          capturedReplyToId = variables?.replyToId;
+          return {
+            addDiscussionComment: {
+              comment: {
+                id: "DC_kwDOTest456",
+                body: variables?.body,
+                createdAt: "2024-01-01",
+                url: "https://github.com/owner/repo/discussions/10#discussioncomment-456",
+              },
+            },
+          };
+        }
+        // Mock replyTo resolution: the provided reply_to_id is itself a reply, return its parent
+        if (query.includes("replyTo")) {
+          return { node: { replyTo: { id: "DC_kwDOTopLevelComment123" } } };
+        }
+        return {
+          repository: {
+            discussion: {
+              id: "D_kwDOTest123",
+              url: "https://github.com/owner/repo/discussions/10",
+            },
+          },
+        };
+      };
+
+      const handler = await eval(`(async () => { ${addCommentScript}; return await main({}); })()`);
+
+      const message = {
+        type: "add_comment",
+        item_number: 10,
+        body: "🔄 Updated finding...",
+        reply_to_id: "DC_kwDONestedReply789",
+      };
+
+      const result = await handler(message, {});
+
+      expect(result.success).toBe(true);
+      expect(result.isDiscussion).toBe(true);
+      // Should resolve to the top-level parent, not the nested reply
+      expect(capturedReplyToId).toBe("DC_kwDOTopLevelComment123");
+    });
+
+    it("should ignore reply_to_id when triggered by discussion_comment event without explicit item_number", async () => {
+      const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
+
+      // discussion_comment trigger takes precedence over reply_to_id
+      mockContext.eventName = "discussion_comment";
+      mockContext.payload = {
+        discussion: {
+          number: 10,
+        },
+        comment: {
+          node_id: "DC_kwDOTriggeringComment123",
+        },
+      };
+
+      let capturedReplyToId = undefined;
+      mockGithub.graphql = async (query, variables) => {
+        if (query.includes("addDiscussionComment")) {
+          capturedReplyToId = variables?.replyToId;
+          return {
+            addDiscussionComment: {
+              comment: {
+                id: "DC_kwDOTest456",
+                body: variables?.body,
+                createdAt: "2024-01-01",
+                url: "https://github.com/owner/repo/discussions/10#discussioncomment-456",
+              },
+            },
+          };
+        }
+        // Mock replyTo resolution: triggering comment is top-level (no parent)
+        if (query.includes("replyTo")) {
+          return { node: { replyTo: null } };
+        }
+        return {
+          repository: {
+            discussion: {
+              id: "D_kwDOTest123",
+              url: "https://github.com/owner/repo/discussions/10",
+            },
+          },
+        };
+      };
+
+      const handler = await eval(`(async () => { ${addCommentScript}; return await main({}); })()`);
+
+      const message = {
+        type: "add_comment",
+        body: "Reply that provides reply_to_id but should use triggering comment instead",
+        reply_to_id: "DC_kwDOShouldBeIgnored999",
+      };
+
+      const result = await handler(message, {});
+
+      expect(result.success).toBe(true);
+      expect(result.isDiscussion).toBe(true);
+      // Should use the triggering comment node_id, not the reply_to_id from the message
+      expect(capturedReplyToId).toBe("DC_kwDOTriggeringComment123");
+    });
+
+    it("should ignore and warn when reply_to_id is a whitespace-only string", async () => {
+      const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
+
+      mockContext.eventName = "workflow_dispatch";
+      mockContext.payload = {};
+
+      // Capture warning calls
+      const warningCalls = [];
+      mockCore.warning = msg => {
+        warningCalls.push(msg);
+      };
+
+      // Make REST API return 404 so the code falls back to the discussion path
+      mockGithub.rest.issues.createComment = async () => {
+        const err = new Error("Not Found");
+        // @ts-expect-error - Simulating GitHub REST API error
+        err.status = 404;
+        throw err;
+      };
+
+      let capturedReplyToId = undefined;
+      mockGithub.graphql = async (query, variables) => {
+        if (query.includes("addDiscussionComment")) {
+          capturedReplyToId = variables?.replyToId;
+          return {
+            addDiscussionComment: {
+              comment: {
+                id: "DC_kwDOTest456",
+                body: variables?.body,
+                createdAt: "2024-01-01",
+                url: "https://github.com/owner/repo/discussions/10#discussioncomment-456",
+              },
+            },
+          };
+        }
+        return {
+          repository: {
+            discussion: {
+              id: "D_kwDOTest123",
+              url: "https://github.com/owner/repo/discussions/10",
+            },
+          },
+        };
+      };
+
+      const handler = await eval(`(async () => { ${addCommentScript}; return await main({}); })()`);
+
+      const message = {
+        type: "add_comment",
+        item_number: 10,
+        body: "Comment with blank reply_to_id",
+        reply_to_id: "   ",
+      };
+
+      const result = await handler(message, {});
+
+      expect(result.success).toBe(true);
+      expect(result.isDiscussion).toBe(true);
+      // Whitespace-only reply_to_id should be ignored (post top-level)
+      expect(capturedReplyToId).toBeUndefined();
+      expect(warningCalls).toContain("Ignoring empty discussion reply_to_id after normalization");
+    });
+
+    it("should coerce numeric reply_to_id to string before use", async () => {
+      const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");
+
+      mockContext.eventName = "workflow_dispatch";
+      mockContext.payload = {};
+
+      // Make REST API return 404 so the code falls back to the discussion path
+      mockGithub.rest.issues.createComment = async () => {
+        const err = new Error("Not Found");
+        // @ts-expect-error - Simulating GitHub REST API error
+        err.status = 404;
+        throw err;
+      };
+
+      let capturedReplyToId = undefined;
+      mockGithub.graphql = async (query, variables) => {
+        if (query.includes("addDiscussionComment")) {
+          capturedReplyToId = variables?.replyToId;
+          return {
+            addDiscussionComment: {
+              comment: {
+                id: "DC_kwDOTest456",
+                body: variables?.body,
+                createdAt: "2024-01-01",
+                url: "https://github.com/owner/repo/discussions/10#discussioncomment-456",
+              },
+            },
+          };
+        }
+        // Mock replyTo resolution: node ID is top-level (no parent)
+        if (query.includes("replyTo")) {
+          return { node: { replyTo: null } };
+        }
+        return {
+          repository: {
+            discussion: {
+              id: "D_kwDOTest123",
+              url: "https://github.com/owner/repo/discussions/10",
+            },
+          },
+        };
+      };
+
+      const handler = await eval(`(async () => { ${addCommentScript}; return await main({}); })()`);
+
+      const message = {
+        type: "add_comment",
+        item_number: 10,
+        body: "Comment with numeric reply_to_id",
+        // @ts-expect-error - intentionally passing a number to test coercion
+        reply_to_id: 12345,
+      };
+
+      const result = await handler(message, {});
+
+      expect(result.success).toBe(true);
+      expect(result.isDiscussion).toBe(true);
+      // Numeric reply_to_id should be coerced to "12345"
+      expect(capturedReplyToId).toBe("12345");
+    });
   });
 
   describe("regression test for wrong PR bug", () => {