From a51eeb348ce7e79d49077f149753ed47aa072456 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 20:12:31 +0000 Subject: [PATCH 1/5] Initial plan From 8d1164855b08523bedeeae9d5a05f4001445ea51 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 20:27:33 +0000 Subject: [PATCH 2/5] feat: add reply_to_id support in add-comment for discussion threading When an agent outputs a reply_to_id field in the add_comment message, the handler now uses it to post a threaded comment to a Discussion. This works for both: - discussion/discussion_comment events with explicit item_number - workflow_dispatch events where item_number refers to a discussion (via the existing 404-fallback path to the GraphQL discussion API) resolveTopLevelDiscussionCommentId is applied to reply_to_id to handle the case where the caller passes a nested reply node ID rather than a top-level comment node ID." Agent-Logs-Url: https://github.com/github/gh-aw/sessions/53f201fe-1abf-4a8a-9541-6de36a26c892 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/add_comment.cjs | 21 ++- actions/setup/js/add_comment.test.cjs | 181 ++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 2 deletions(-) diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index 72ed50ff224..3abde1ae8cf 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -590,7 +590,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 (message.reply_to_id) { + // 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, message.reply_to_id); + } else { + replyToId = null; + } if (replyToId) { core.info(`Replying as threaded comment to discussion comment node ID: ${replyToId}`); } @@ -621,7 +632,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 an explicit reply_to_id from the message. + // Apply resolveTopLevelDiscussionCommentId to handle nested reply node IDs. + const fallbackReplyToId = message.reply_to_id ? await resolveTopLevelDiscussionCommentId(githubClient, message.reply_to_id) : 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); diff --git a/actions/setup/js/add_comment.test.cjs b/actions/setup/js/add_comment.test.cjs index 25438cef186..5b27c2922f3 100644 --- a/actions/setup/js/add_comment.test.cjs +++ b/actions/setup/js/add_comment.test.cjs @@ -594,6 +594,187 @@ 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"); + }); }); describe("regression test for wrong PR bug", () => { From 5ba9725f9691bf8f096bc8d49d40b3100ce6f5f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 21:10:05 +0000 Subject: [PATCH 3/5] feat: add comment_node_id to aw_context for discussion threading Add comment_node_id (GraphQL node ID) to the aw_context object built by buildAwContext() and resolveItemContext(). For discussion and discussion_comment events, this captures payload.comment.node_id so that dispatched specialist workflows receive the GraphQL node ID of the triggering discussion comment via aw_context.comment_node_id. The specialist can pass this value as reply_to_id in an add_comment output to thread its reply under the triggering comment, completing the CentralRepoOps dispatch chain end-to-end. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2ac8e162-7a35-4423-a934-4c4d4c91c3c2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/aw_context.cjs | 23 +++++++++++--- actions/setup/js/aw_context.test.cjs | 35 +++++++++++++++------- actions/setup/js/generate_aw_info.test.cjs | 24 +++++++++++++++ 3 files changed, 67 insertions(+), 15 deletions(-) diff --git a/actions/setup/js/aw_context.cjs b/actions/setup/js/aw_context.cjs index d300bf40b30..b9b1e848da1 100644 --- a/actions/setup/js/aw_context.cjs +++ b/actions/setup/js/aw_context.cjs @@ -23,7 +23,7 @@ * `pull_request` rather than `issue`. * * @param {object | null | undefined} payload - GitHub Actions context.payload - * @returns {{ item_type: string, item_number: string, comment_id: string }} + * @returns {{ item_type: string, item_number: string, comment_id: string, comment_node_id: string }} */ function resolveItemContext(payload) { if (payload?.issue != null) { @@ -35,12 +35,14 @@ function resolveItemContext(payload) { item_type: "pull_request", item_number: payload.issue.number != null ? String(payload.issue.number) : "", comment_id: payload.comment?.id != null ? String(payload.comment.id) : "", + comment_node_id: "", }; } return { item_type: "issue", item_number: payload.issue.number != null ? String(payload.issue.number) : "", comment_id: payload.comment?.id != null ? String(payload.comment.id) : "", + comment_node_id: "", }; } if (payload?.pull_request != null) { @@ -50,6 +52,7 @@ function resolveItemContext(payload) { // pull_request_review events carry a review object; pull_request_review_comment // events carry a comment object. Both are reported as comment_id. comment_id: payload.comment?.id != null ? String(payload.comment.id) : payload.review?.id != null ? String(payload.review.id) : "", + comment_node_id: "", }; } if (payload?.discussion != null) { @@ -57,6 +60,10 @@ function resolveItemContext(payload) { item_type: "discussion", item_number: payload.discussion.number != null ? String(payload.discussion.number) : "", comment_id: payload.comment?.id != null ? String(payload.comment.id) : "", + // comment_node_id is the GraphQL node ID of the triggering discussion comment. + // It can be used as reply_to_id in add_comment to thread responses under + // the triggering comment when dispatching specialist workflows. + comment_node_id: payload.comment?.node_id != null ? String(payload.comment.node_id) : "", }; } if (payload?.check_run != null) { @@ -64,6 +71,7 @@ function resolveItemContext(payload) { item_type: "check_run", item_number: payload.check_run.id != null ? String(payload.check_run.id) : "", comment_id: "", + comment_node_id: "", }; } if (payload?.check_suite != null) { @@ -71,9 +79,10 @@ function resolveItemContext(payload) { item_type: "check_suite", item_number: payload.check_suite.id != null ? String(payload.check_suite.id) : "", comment_id: "", + comment_node_id: "", }; } - return { item_type: "", item_number: "", comment_id: "" }; + return { item_type: "", item_number: "", comment_id: "", comment_node_id: "" }; } /** @@ -93,7 +102,8 @@ function resolveItemContext(payload) { * event_type: string, * item_type: string, * item_number: string, - * comment_id: string + * comment_id: string, + * comment_node_id: string * }} * Properties: * - item_type: Kind of entity that triggered the workflow (issue, pull_request, @@ -103,9 +113,13 @@ function resolveItemContext(payload) { * id (check_run/check_suite). Empty string when item_type is empty. * - comment_id: ID of the triggering comment or review. Empty string when the * event is not a comment/review event. + * - comment_node_id: GraphQL node ID of the triggering discussion comment. + * Only populated for discussion/discussion_comment events. Can be passed + * as reply_to_id in add_comment to thread responses under the triggering + * comment when a dispatched specialist workflow replies to a discussion. */ function buildAwContext() { - const { item_type, item_number, comment_id } = resolveItemContext(context.payload); + const { item_type, item_number, comment_id, comment_node_id } = resolveItemContext(context.payload); return { repo: `${context.repo.owner}/${context.repo.repo}`, @@ -122,6 +136,7 @@ function buildAwContext() { item_type, item_number, comment_id, + comment_node_id, }; } diff --git a/actions/setup/js/aw_context.test.cjs b/actions/setup/js/aw_context.test.cjs index 6d9d9a8691b..10f4e285356 100644 --- a/actions/setup/js/aw_context.test.cjs +++ b/actions/setup/js/aw_context.test.cjs @@ -7,23 +7,23 @@ const { resolveItemContext } = await import("./aw_context.cjs"); describe("resolveItemContext", () => { it("returns issue type and number for issues events", () => { const payload = { issue: { number: 42 } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "issue", item_number: "42", comment_id: "" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "issue", item_number: "42", comment_id: "", comment_node_id: "" }); }); it("returns issue type with comment_id for issue_comment events", () => { const payload = { issue: { number: 7 }, comment: { id: 99001122 } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "issue", item_number: "7", comment_id: "99001122" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "issue", item_number: "7", comment_id: "99001122", comment_node_id: "" }); }); it("returns pull_request type with comment_id for issue_comment events on pull requests", () => { // GitHub sends issue_comment events for PR comments with payload.issue.pull_request set const payload = { issue: { number: 7, pull_request: {} }, comment: { id: 99001122 } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "pull_request", item_number: "7", comment_id: "99001122" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "pull_request", item_number: "7", comment_id: "99001122", comment_node_id: "" }); }); it("returns pull_request type and number for pull_request events", () => { const payload = { pull_request: { number: 100 } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "pull_request", item_number: "100", comment_id: "" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "pull_request", item_number: "100", comment_id: "", comment_node_id: "" }); }); it("returns pull_request type with review id for pull_request_review events", () => { @@ -32,6 +32,7 @@ describe("resolveItemContext", () => { item_type: "pull_request", item_number: "100", comment_id: "55667788", + comment_node_id: "", }); }); @@ -42,12 +43,13 @@ describe("resolveItemContext", () => { item_type: "pull_request", item_number: "100", comment_id: "11223344", + comment_node_id: "", }); }); it("returns discussion type and number for discussion events", () => { const payload = { discussion: { number: 5 } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "discussion", item_number: "5", comment_id: "" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "discussion", item_number: "5", comment_id: "", comment_node_id: "" }); }); it("returns discussion type with comment_id for discussion_comment events", () => { @@ -56,27 +58,38 @@ describe("resolveItemContext", () => { item_type: "discussion", item_number: "5", comment_id: "77889900", + comment_node_id: "", + }); + }); + + it("returns discussion type with comment_node_id for discussion_comment events with node_id", () => { + const payload = { discussion: { number: 5 }, comment: { id: 77889900, node_id: "DC_kwDOParentComment456" } }; + expect(resolveItemContext(payload)).toEqual({ + item_type: "discussion", + item_number: "5", + comment_id: "77889900", + comment_node_id: "DC_kwDOParentComment456", }); }); it("returns check_run type and id for check_run events", () => { const payload = { check_run: { id: 7654321 } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "check_run", item_number: "7654321", comment_id: "" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "check_run", item_number: "7654321", comment_id: "", comment_node_id: "" }); }); it("returns check_suite type and id for check_suite events", () => { const payload = { check_suite: { id: 9988776 } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "check_suite", item_number: "9988776", comment_id: "" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "check_suite", item_number: "9988776", comment_id: "", comment_node_id: "" }); }); it("returns empty strings for push/workflow_dispatch events (no item payload)", () => { - expect(resolveItemContext({})).toEqual({ item_type: "", item_number: "", comment_id: "" }); - expect(resolveItemContext(null)).toEqual({ item_type: "", item_number: "", comment_id: "" }); - expect(resolveItemContext(undefined)).toEqual({ item_type: "", item_number: "", comment_id: "" }); + expect(resolveItemContext({})).toEqual({ item_type: "", item_number: "", comment_id: "", comment_node_id: "" }); + expect(resolveItemContext(null)).toEqual({ item_type: "", item_number: "", comment_id: "", comment_node_id: "" }); + expect(resolveItemContext(undefined)).toEqual({ item_type: "", item_number: "", comment_id: "", comment_node_id: "" }); }); it("returns empty item_number when number is null", () => { const payload = { issue: { number: null } }; - expect(resolveItemContext(payload)).toEqual({ item_type: "issue", item_number: "", comment_id: "" }); + expect(resolveItemContext(payload)).toEqual({ item_type: "issue", item_number: "", comment_id: "", comment_node_id: "" }); }); }); diff --git a/actions/setup/js/generate_aw_info.test.cjs b/actions/setup/js/generate_aw_info.test.cjs index ac570fc1792..83b3f23093d 100644 --- a/actions/setup/js/generate_aw_info.test.cjs +++ b/actions/setup/js/generate_aw_info.test.cjs @@ -352,4 +352,28 @@ describe("generate_aw_info.cjs", () => { expect(awInfo.context).toEqual(validContext); expect(mockCore.warning).not.toHaveBeenCalledWith(expect.stringContaining("aw_context")); }); + + it("should accept valid aw_context with comment_node_id for discussion_comment events", async () => { + const validContext = { + repo: "org/repo", + run_id: "12345", + workflow_id: "org/repo/.github/workflows/dispatcher.yml@refs/heads/main", + workflow_call_id: "12345-1", + time: new Date().toISOString(), + actor: "octocat", + event_type: "discussion_comment", + item_type: "discussion", + item_number: "240", + comment_id: "77889900", + comment_node_id: "DC_kwDOParentComment456", + }; + const contextWithDiscussion = { + ...mockContext, + payload: { inputs: { aw_context: JSON.stringify(validContext) } }, + }; + await main(mockCore, contextWithDiscussion); + const awInfo = JSON.parse(fs.readFileSync(awInfoPath, "utf8")); + expect(awInfo.context).toEqual(validContext); + expect(mockCore.warning).not.toHaveBeenCalledWith(expect.stringContaining("aw_context")); + }); }); From c4ef6062c5ca8a3b7a67ca692746a5fc01fcdda0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 21:11:14 +0000 Subject: [PATCH 4/5] docs: clarify comment_node_id is only populated for discussion events Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2ac8e162-7a35-4423-a934-4c4d4c91c3c2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/aw_context.cjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/actions/setup/js/aw_context.cjs b/actions/setup/js/aw_context.cjs index b9b1e848da1..6c24f68adeb 100644 --- a/actions/setup/js/aw_context.cjs +++ b/actions/setup/js/aw_context.cjs @@ -24,6 +24,9 @@ * * @param {object | null | undefined} payload - GitHub Actions context.payload * @returns {{ item_type: string, item_number: string, comment_id: string, comment_node_id: string }} + * comment_node_id is only populated for discussion/discussion_comment events where + * payload.comment.node_id is present (GraphQL node ID needed for reply threading). + * It is intentionally empty for all other event types (issues, PRs, checks). */ function resolveItemContext(payload) { if (payload?.issue != null) { From d08ee3560fbeb732f14f5c5808686de096f02d5e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Apr 2026 21:49:27 +0000 Subject: [PATCH 5/5] fix: normalize reply_to_id before use in discussion threading Centralize normalization of message.reply_to_id: coerce to String, trim whitespace, and warn+ignore when the result is empty. The normalized value (normalizedExplicitReplyToId) is computed once before the try/catch so both the main discussion path and the 404 discussion fallback path share the same validated value. Add two new tests: whitespace-only reply_to_id (warns and posts top-level) and numeric reply_to_id (coerced to string "12345"). Agent-Logs-Url: https://github.com/github/gh-aw/sessions/52b7c846-0f5e-4b30-9ce4-9358a3d5bc75 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/add_comment.cjs | 15 +++- actions/setup/js/add_comment.test.cjs | 124 ++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 4 deletions(-) diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index 3abde1ae8cf..6ce90421cc4 100644 --- a/actions/setup/js/add_comment.cjs +++ b/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 @@ -594,11 +601,11 @@ async function main(config = {}) { 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 (message.reply_to_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, message.reply_to_id); + replyToId = await resolveTopLevelDiscussionCommentId(githubClient, normalizedExplicitReplyToId); } else { replyToId = null; } @@ -632,9 +639,9 @@ async function main(config = {}) { try { core.info(`Trying #${itemNumber} as discussion...`); - // When retrying as discussion, honour an explicit reply_to_id from the message. + // When retrying as discussion, honour the normalized reply_to_id from the message. // Apply resolveTopLevelDiscussionCommentId to handle nested reply node IDs. - const fallbackReplyToId = message.reply_to_id ? await resolveTopLevelDiscussionCommentId(githubClient, message.reply_to_id) : null; + const fallbackReplyToId = normalizedExplicitReplyToId ? await resolveTopLevelDiscussionCommentId(githubClient, normalizedExplicitReplyToId) : null; if (fallbackReplyToId) { core.info(`Replying as threaded comment to discussion comment node ID: ${fallbackReplyToId}`); } diff --git a/actions/setup/js/add_comment.test.cjs b/actions/setup/js/add_comment.test.cjs index 5b27c2922f3..412af1ddb57 100644 --- a/actions/setup/js/add_comment.test.cjs +++ b/actions/setup/js/add_comment.test.cjs @@ -775,6 +775,130 @@ describe("add_comment", () => { // 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", () => {