diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index 4f9cb0ae044..9b0a740e521 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -19,6 +19,7 @@ const { getMissingInfoSections } = require("./missing_messages_helper.cjs"); const { getMessages } = require("./messages_core.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { MAX_COMMENT_LENGTH, MAX_MENTIONS, MAX_LINKS, enforceCommentLimits } = require("./comment_limit_helpers.cjs"); +const { resolveTopLevelDiscussionCommentId } = require("./github_api_helpers.cjs"); const { logStagedPreviewInfo } = require("./staged_preview.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); const { isPayloadUserBot } = require("./resolve_mentions.cjs"); @@ -575,8 +576,10 @@ async function main(config = {}) { if (isDiscussion) { // When triggered by a discussion_comment event (without explicit item_number), // reply as a threaded comment to the triggering comment instead of posting top-level. + // 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 ? context.payload?.comment?.node_id : null; + const replyToId = context.eventName === "discussion_comment" && !hasExplicitItemNumber ? await resolveTopLevelDiscussionCommentId(githubClient, context.payload?.comment?.node_id) : null; if (replyToId) { core.info(`Replying as threaded comment to discussion comment node ID: ${replyToId}`); } diff --git a/actions/setup/js/add_comment.test.cjs b/actions/setup/js/add_comment.test.cjs index bfac328576f..3703628946f 100644 --- a/actions/setup/js/add_comment.test.cjs +++ b/actions/setup/js/add_comment.test.cjs @@ -408,6 +408,10 @@ describe("add_comment", () => { }, }; } + // Mock replyTo resolution: top-level comment has no parent + if (query.includes("replyTo")) { + return { node: { replyTo: null } }; + } return { repository: { discussion: { @@ -433,6 +437,64 @@ describe("add_comment", () => { expect(capturedReplyToId).toBe("DC_kwDOTriggeringComment123"); }); + it("should use parent comment node ID when the triggering comment is itself a threaded reply", async () => { + const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8"); + + // Change context to discussion_comment event where the triggering comment is itself a reply + mockContext.eventName = "discussion_comment"; + mockContext.payload = { + discussion: { + number: 10, + }, + comment: { + node_id: "DC_kwDOReplyComment789", // This is a reply, not a top-level comment + }, + }; + + 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 triggering comment is a reply with a parent + if (query.includes("replyTo")) { + return { node: { replyTo: { id: "DC_kwDOParentComment456" } } }; + } + 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 to a reply - should use parent node ID", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.isDiscussion).toBe(true); + // The replyToId should be the parent (top-level) comment node ID, not the triggering reply + expect(capturedReplyToId).toBe("DC_kwDOParentComment456"); + }); + it("should post a top-level comment (not threaded) when triggered by discussion_comment with explicit item_number", async () => { const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8"); diff --git a/actions/setup/js/add_reaction_and_edit_comment.cjs b/actions/setup/js/add_reaction_and_edit_comment.cjs index 11a110d6ea5..e8b27e5e343 100644 --- a/actions/setup/js/add_reaction_and_edit_comment.cjs +++ b/actions/setup/js/add_reaction_and_edit_comment.cjs @@ -7,6 +7,7 @@ const { generateWorkflowIdMarker } = require("./generate_footer.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { ERR_API, ERR_NOT_FOUND, ERR_VALIDATION } = require("./error_codes.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); +const { resolveTopLevelDiscussionCommentId } = require("./github_api_helpers.cjs"); /** * Event type descriptions for comment messages @@ -358,8 +359,10 @@ async function addCommentWithWorkflowLink(endpoint, runUrl, eventName) { const discussionNumber = parseInt(endpoint.split(":")[1], 10); const { id: discussionId } = await getDiscussionId(context.repo.owner, context.repo.repo, discussionNumber); - // Get the comment node ID to use as the parent for threading - const commentNodeId = context.payload?.comment?.node_id; + // Get the comment node ID to use as the parent for threading. + // 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. + const commentNodeId = await resolveTopLevelDiscussionCommentId(github, context.payload?.comment?.node_id); const result = await github.graphql( ` diff --git a/actions/setup/js/add_reaction_and_edit_comment.test.cjs b/actions/setup/js/add_reaction_and_edit_comment.test.cjs index e6707eca5f5..dfb4cfefd69 100644 --- a/actions/setup/js/add_reaction_and_edit_comment.test.cjs +++ b/actions/setup/js/add_reaction_and_edit_comment.test.cjs @@ -313,6 +313,7 @@ describe("add_reaction_and_edit_comment.cjs", () => { mockGithub.graphql .mockResolvedValueOnce({ addReaction: { reaction: { id: "MDg6UmVhY3Rpb24xMjM0NTY3ODk=", content: "EYES" } } }) .mockResolvedValueOnce({ repository: { discussion: { id: "D_kwDOABcD1M4AaBbC", url: "https://github.com/testowner/testrepo/discussions/10" } } }) + .mockResolvedValueOnce({ node: { replyTo: null } }) // resolveTopLevelDiscussionCommentId: top-level comment .mockResolvedValueOnce({ addDiscussionComment: { comment: { id: "DC_kwDOABcD1M4AaBbE", url: "https://github.com/testowner/testrepo/discussions/10#discussioncomment-789" }, diff --git a/actions/setup/js/add_workflow_run_comment.cjs b/actions/setup/js/add_workflow_run_comment.cjs index 8201482241e..7b766e31770 100644 --- a/actions/setup/js/add_workflow_run_comment.cjs +++ b/actions/setup/js/add_workflow_run_comment.cjs @@ -9,6 +9,7 @@ const { ERR_NOT_FOUND, ERR_VALIDATION } = require("./error_codes.cjs"); const { getMessages } = require("./messages_core.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); +const { resolveTopLevelDiscussionCommentId } = require("./github_api_helpers.cjs"); /** * Event type descriptions for comment messages @@ -240,8 +241,10 @@ async function addCommentWithWorkflowLink(endpoint, runUrl, eventName) { // Get discussion node ID using helper function const discussionId = await getDiscussionNodeId(discussionNumber); - // Get the comment node ID to use as the parent for threading - const commentNodeId = context.payload?.comment?.node_id; + // Get the comment node ID to use as the parent for threading. + // 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. + const commentNodeId = await resolveTopLevelDiscussionCommentId(github, context.payload?.comment?.node_id); const result = await github.graphql( ` diff --git a/actions/setup/js/github_api_helpers.cjs b/actions/setup/js/github_api_helpers.cjs index f9d5f54ec23..0383b01081a 100644 --- a/actions/setup/js/github_api_helpers.cjs +++ b/actions/setup/js/github_api_helpers.cjs @@ -141,8 +141,43 @@ async function fetchAllRepoLabels(githubClient, owner, repo) { return allLabels; } +/** + * Resolves the top-level parent comment node ID for GitHub Discussion replies. + * GitHub Discussions only supports two nesting levels: top-level comments and one level of replies. + * If the given comment is itself a reply (has a replyTo parent), the parent's node ID is returned + * so that replyToId always points to a top-level comment. + * + * @param {Object} github - GitHub API client (must support graphql) + * @param {string|null|undefined} commentNodeId - The node_id of the triggering comment + * @returns {Promise} The node ID to use as replyToId (parent if reply, otherwise the original) + */ +async function resolveTopLevelDiscussionCommentId(github, commentNodeId) { + if (!commentNodeId) { + return commentNodeId; + } + try { + const result = await github.graphql( + `query($nodeId: ID!) { + node(id: $nodeId) { + ... on DiscussionComment { + replyTo { + id + } + } + } + }`, + { nodeId: commentNodeId } + ); + return result?.node?.replyTo?.id ?? commentNodeId; + } catch (error) { + logGraphQLError(/** @type {Error & { errors?: Array<{ type?: string, message: string, path?: unknown, locations?: unknown }>, request?: unknown, data?: unknown, status?: number }} */ error, "resolving top-level discussion comment"); + return commentNodeId; + } +} + module.exports = { fetchAllRepoLabels, getFileContent, logGraphQLError, + resolveTopLevelDiscussionCommentId, }; diff --git a/actions/setup/js/github_api_helpers.test.cjs b/actions/setup/js/github_api_helpers.test.cjs index 85aed0d7467..e6e62b109af 100644 --- a/actions/setup/js/github_api_helpers.test.cjs +++ b/actions/setup/js/github_api_helpers.test.cjs @@ -10,6 +10,7 @@ describe("github_api_helpers.cjs", () => { let getFileContent; let logGraphQLError; let fetchAllRepoLabels; + let resolveTopLevelDiscussionCommentId; let mockGithub; beforeEach(async () => { @@ -28,6 +29,7 @@ describe("github_api_helpers.cjs", () => { getFileContent = module.getFileContent; logGraphQLError = module.logGraphQLError; fetchAllRepoLabels = module.fetchAllRepoLabels; + resolveTopLevelDiscussionCommentId = module.resolveTopLevelDiscussionCommentId; }); describe("getFileContent", () => { @@ -333,4 +335,70 @@ describe("github_api_helpers.cjs", () => { expect(result).toEqual([]); }); }); + + describe("resolveTopLevelDiscussionCommentId", () => { + it("should return the original node ID when the comment is a top-level comment (no replyTo)", async () => { + const mockGraphql = vi.fn().mockResolvedValueOnce({ + node: { + replyTo: null, + }, + }); + + const result = await resolveTopLevelDiscussionCommentId({ graphql: mockGraphql }, "DC_topLevel123"); + + expect(result).toBe("DC_topLevel123"); + expect(mockGraphql).toHaveBeenCalledWith(expect.stringContaining("replyTo"), { nodeId: "DC_topLevel123" }); + }); + + it("should return the parent node ID when the comment is itself a threaded reply", async () => { + const mockGraphql = vi.fn().mockResolvedValueOnce({ + node: { + replyTo: { + id: "DC_parentComment456", + }, + }, + }); + + const result = await resolveTopLevelDiscussionCommentId({ graphql: mockGraphql }, "DC_replyComment789"); + + expect(result).toBe("DC_parentComment456"); + }); + + it("should return the original node ID when node response has no DiscussionComment data", async () => { + const mockGraphql = vi.fn().mockResolvedValueOnce({ + node: null, + }); + + const result = await resolveTopLevelDiscussionCommentId({ graphql: mockGraphql }, "DC_unknown123"); + + expect(result).toBe("DC_unknown123"); + }); + + it("should return null without calling graphql when commentNodeId is null", async () => { + const mockGraphql = vi.fn(); + + const result = await resolveTopLevelDiscussionCommentId({ graphql: mockGraphql }, null); + + expect(result).toBeNull(); + expect(mockGraphql).not.toHaveBeenCalled(); + }); + + it("should return undefined without calling graphql when commentNodeId is undefined", async () => { + const mockGraphql = vi.fn(); + + const result = await resolveTopLevelDiscussionCommentId({ graphql: mockGraphql }, undefined); + + expect(result).toBeUndefined(); + expect(mockGraphql).not.toHaveBeenCalled(); + }); + + it("should return the original node ID and log error when graphql throws", async () => { + const mockGraphql = vi.fn().mockRejectedValueOnce(new Error("GraphQL network error")); + + const result = await resolveTopLevelDiscussionCommentId({ graphql: mockGraphql }, "DC_fallback123"); + + expect(result).toBe("DC_fallback123"); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("resolving top-level discussion comment")); + }); + }); }); diff --git a/actions/setup/js/notify_comment_error.cjs b/actions/setup/js/notify_comment_error.cjs index 551ee4a6e99..fbe4e40b017 100644 --- a/actions/setup/js/notify_comment_error.cjs +++ b/actions/setup/js/notify_comment_error.cjs @@ -12,6 +12,7 @@ const { getErrorMessage, isLockedError } = require("./error_helpers.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { ERR_VALIDATION } = require("./error_codes.cjs"); const { parseBoolTemplatable } = require("./templatable.cjs"); +const { resolveTopLevelDiscussionCommentId } = require("./github_api_helpers.cjs"); /** * Collect generated asset URLs from safe output jobs @@ -215,7 +216,9 @@ async function main() { return; } - const replyToId = eventName === "discussion_comment" ? context.payload?.comment?.node_id : null; + // 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. + const replyToId = eventName === "discussion_comment" ? await resolveTopLevelDiscussionCommentId(github, context.payload?.comment?.node_id) : null; const mutation = replyToId ? `mutation($dId: ID!, $body: String!, $replyToId: ID!) { addDiscussionComment(input: { discussionId: $dId, body: $body, replyToId: $replyToId }) { diff --git a/actions/setup/js/update_activation_comment.cjs b/actions/setup/js/update_activation_comment.cjs index 1c3744a3785..0157d7b419f 100644 --- a/actions/setup/js/update_activation_comment.cjs +++ b/actions/setup/js/update_activation_comment.cjs @@ -9,6 +9,7 @@ const { parseBoolTemplatable } = require("./templatable.cjs"); const { generateXMLMarker } = require("./generate_footer.cjs"); const { getFooterMessage } = require("./messages_footer.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); +const { resolveTopLevelDiscussionCommentId } = require("./github_api_helpers.cjs"); /** * Update the activation comment with a link to the created pull request or issue @@ -117,7 +118,9 @@ async function updateActivationCommentWithMessage(github, context, core, message return; } - const replyToId = eventName === "discussion_comment" ? context.payload?.comment?.node_id : null; + // 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. + const replyToId = eventName === "discussion_comment" ? await resolveTopLevelDiscussionCommentId(github, context.payload?.comment?.node_id) : null; const mutation = replyToId ? `mutation($dId: ID!, $body: String!, $replyToId: ID!) { addDiscussionComment(input: { discussionId: $dId, body: $body, replyToId: $replyToId }) { diff --git a/actions/setup/js/update_activation_comment.test.cjs b/actions/setup/js/update_activation_comment.test.cjs index 53747d0883a..2e6d6fc808d 100644 --- a/actions/setup/js/update_activation_comment.test.cjs +++ b/actions/setup/js/update_activation_comment.test.cjs @@ -84,6 +84,14 @@ const createTestableFunction = scriptContent => { }, }; } + if (module === "./github_api_helpers.cjs") { + return { + resolveTopLevelDiscussionCommentId: async (githubClient, commentNodeId) => { + // In tests, simulate that the triggering comment is always a top-level comment + return commentNodeId; + }, + }; + } throw new Error(`Module ${module} not mocked in test`); }; return new Function(