Skip to content

Commit fc7a080

Browse files
jsweep: clean add_workflow_run_comment.cjs (#26161)
1 parent e3bef41 commit fc7a080

File tree

2 files changed

+165
-95
lines changed

2 files changed

+165
-95
lines changed

actions/setup/js/add_workflow_run_comment.cjs

Lines changed: 69 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -84,44 +84,25 @@ async function main() {
8484

8585
try {
8686
switch (eventName) {
87-
case "issues": {
88-
const number = context.payload?.issue?.number;
89-
if (!number) {
90-
core.setFailed(`${ERR_NOT_FOUND}: Issue number not found in event payload`);
91-
return;
92-
}
93-
commentEndpoint = `/repos/${owner}/${repo}/issues/${number}/comments`;
94-
break;
95-
}
96-
87+
case "issues":
9788
case "issue_comment": {
9889
const number = context.payload?.issue?.number;
9990
if (!number) {
10091
core.setFailed(`${ERR_NOT_FOUND}: Issue number not found in event payload`);
10192
return;
10293
}
103-
// Create new comment on the issue itself, not on the comment
104-
commentEndpoint = `/repos/${owner}/${repo}/issues/${number}/comments`;
105-
break;
106-
}
107-
108-
case "pull_request": {
109-
const number = context.payload?.pull_request?.number;
110-
if (!number) {
111-
core.setFailed(`${ERR_NOT_FOUND}: Pull request number not found in event payload`);
112-
return;
113-
}
11494
commentEndpoint = `/repos/${owner}/${repo}/issues/${number}/comments`;
11595
break;
11696
}
11797

98+
case "pull_request":
11899
case "pull_request_review_comment": {
119100
const number = context.payload?.pull_request?.number;
120101
if (!number) {
121102
core.setFailed(`${ERR_NOT_FOUND}: Pull request number not found in event payload`);
122103
return;
123104
}
124-
// Create new comment on the PR itself (using issues endpoint since PRs are issues)
105+
// PRs use the issues comment endpoint
125106
commentEndpoint = `/repos/${owner}/${repo}/issues/${number}/comments`;
126107
break;
127108
}
@@ -156,123 +137,118 @@ async function main() {
156137
await addCommentWithWorkflowLink(commentEndpoint, runUrl, eventName);
157138
} catch (error) {
158139
const errorMessage = getErrorMessage(error);
159-
core.error(`Failed to create comment: ${errorMessage}`);
160140
// Don't fail the job - just warn since this is not critical
161141
core.warning(`Failed to create comment with workflow link: ${errorMessage}`);
162142
}
163143
}
164144

165145
/**
166-
* Add a comment with a workflow run link
167-
* @param {string} endpoint - The GitHub API endpoint to create the comment (or special format for discussions)
146+
* Build the comment body text for a workflow run link.
147+
* Sanitizes the content and appends all required markers.
148+
* @param {string} eventName - The event type
168149
* @param {string} runUrl - The URL of the workflow run
169-
* @param {string} eventName - The event type (to determine the comment text)
150+
* @returns {string} The assembled comment body
170151
*/
171-
async function addCommentWithWorkflowLink(endpoint, runUrl, eventName) {
172-
// Get workflow name from environment variable
152+
function buildCommentBody(eventName, runUrl) {
173153
const workflowName = process.env.GH_AW_WORKFLOW_NAME || "Workflow";
174-
175-
// Determine the event type description using lookup object
176154
const eventTypeDescription = EVENT_TYPE_DESCRIPTIONS[eventName] ?? "event";
177155

178-
// Use getRunStartedMessage for the workflow link text (supports custom messages)
179-
const workflowLinkText = getRunStartedMessage({
180-
workflowName: workflowName,
181-
runUrl: runUrl,
182-
eventType: eventTypeDescription,
183-
});
184-
185-
// Sanitize the workflow link text to prevent injection attacks (defense in depth for custom message templates)
186-
// This must happen BEFORE adding workflow markers to preserve them
187-
let commentBody = sanitizeContent(workflowLinkText);
156+
// Sanitize before adding markers (defense in depth for custom message templates)
157+
let body = sanitizeContent(getRunStartedMessage({ workflowName, runUrl, eventType: eventTypeDescription }));
188158

189159
// Add lock notice if lock-for-agent is enabled for issues or issue_comment
190-
const lockForAgent = process.env.GH_AW_LOCK_FOR_AGENT === "true";
191-
if (lockForAgent && (eventName === "issues" || eventName === "issue_comment")) {
192-
commentBody += "\n\n🔒 This issue has been locked while the workflow is running to prevent concurrent modifications.";
160+
if (process.env.GH_AW_LOCK_FOR_AGENT === "true" && (eventName === "issues" || eventName === "issue_comment")) {
161+
body += "\n\n🔒 This issue has been locked while the workflow is running to prevent concurrent modifications.";
193162
}
194163

195-
// Add workflow-id and tracker-id markers for hide-older-comments feature
164+
// Add workflow-id marker for hide-older-comments feature
196165
const workflowId = process.env.GITHUB_WORKFLOW || "";
197-
const trackerId = process.env.GH_AW_TRACKER_ID || "";
198-
199-
// Add workflow-id marker if available
200166
if (workflowId) {
201-
commentBody += `\n\n${generateWorkflowIdMarker(workflowId)}`;
167+
body += `\n\n${generateWorkflowIdMarker(workflowId)}`;
202168
}
203169

204-
// Add tracker-id marker if available (for backwards compatibility)
170+
// Add tracker-id marker for backwards compatibility
171+
const trackerId = process.env.GH_AW_TRACKER_ID || "";
205172
if (trackerId) {
206-
commentBody += `\n\n<!-- gh-aw-tracker-id: ${trackerId} -->`;
173+
body += `\n\n<!-- gh-aw-tracker-id: ${trackerId} -->`;
207174
}
208175

209-
// Add comment type marker to identify this as a reaction comment
210-
// This prevents it from being hidden by hide-older-comments
211-
commentBody += `\n\n<!-- gh-aw-comment-type: reaction -->`;
176+
// Identify this as a reaction comment (prevents it from being hidden by hide-older-comments)
177+
body += `\n\n<!-- gh-aw-comment-type: reaction -->`;
212178

213-
// Handle discussion events specially
214-
if (eventName === "discussion") {
215-
// Parse discussion number from special format: "discussion:NUMBER"
216-
const discussionNumber = parseInt(endpoint.split(":")[1], 10);
179+
return body;
180+
}
217181

218-
// Get discussion node ID using helper function
219-
const discussionId = await getDiscussionNodeId(discussionNumber);
182+
/**
183+
* Post a GraphQL comment to a discussion, optionally as a threaded reply.
184+
* @param {number} discussionNumber - The discussion number
185+
* @param {string} commentBody - The comment body
186+
* @param {string|null} replyToNodeId - Parent comment node ID for threading (null for top-level)
187+
*/
188+
async function postDiscussionComment(discussionNumber, commentBody, replyToNodeId = null) {
189+
const discussionId = await getDiscussionNodeId(discussionNumber);
220190

221-
const result = await github.graphql(
191+
/** @type {any} */
192+
let result;
193+
if (replyToNodeId) {
194+
result = await github.graphql(
195+
`
196+
mutation($dId: ID!, $body: String!, $replyToId: ID!) {
197+
addDiscussionComment(input: { discussionId: $dId, body: $body, replyToId: $replyToId }) {
198+
comment { id url }
199+
}
200+
}`,
201+
{ dId: discussionId, body: commentBody, replyToId: replyToNodeId }
202+
);
203+
} else {
204+
result = await github.graphql(
222205
`
223206
mutation($dId: ID!, $body: String!) {
224207
addDiscussionComment(input: { discussionId: $dId, body: $body }) {
225-
comment {
226-
id
227-
url
228-
}
208+
comment { id url }
229209
}
230210
}`,
231211
{ dId: discussionId, body: commentBody }
232212
);
213+
}
233214

234-
const comment = result.addDiscussionComment.comment;
235-
setCommentOutputs(comment.id, comment.url);
215+
const comment = result.addDiscussionComment.comment;
216+
setCommentOutputs(comment.id, comment.url);
217+
}
218+
219+
/**
220+
* Add a comment with a workflow run link
221+
* @param {string} endpoint - The GitHub API endpoint to create the comment (or special format for discussions)
222+
* @param {string} runUrl - The URL of the workflow run
223+
* @param {string} eventName - The event type (to determine the comment text)
224+
*/
225+
async function addCommentWithWorkflowLink(endpoint, runUrl, eventName) {
226+
const commentBody = buildCommentBody(eventName, runUrl);
227+
228+
if (eventName === "discussion") {
229+
// Parse discussion number from special format: "discussion:NUMBER"
230+
const discussionNumber = parseInt(endpoint.split(":")[1], 10);
231+
await postDiscussionComment(discussionNumber, commentBody);
236232
return;
237-
} else if (eventName === "discussion_comment") {
233+
}
234+
235+
if (eventName === "discussion_comment") {
238236
// Parse discussion number from special format: "discussion_comment:NUMBER:COMMENT_ID"
239237
const discussionNumber = parseInt(endpoint.split(":")[1], 10);
240238

241-
// Get discussion node ID using helper function
242-
const discussionId = await getDiscussionNodeId(discussionNumber);
243-
244-
// Get the comment node ID to use as the parent for threading.
245-
// GitHub Discussions only supports two nesting levels, so if the triggering comment is
246-
// itself a reply, we resolve the top-level parent's node ID.
239+
// GitHub Discussions only supports two nesting levels, so resolve the top-level parent's node ID
247240
const commentNodeId = await resolveTopLevelDiscussionCommentId(github, context.payload?.comment?.node_id);
248-
249-
const result = await github.graphql(
250-
`
251-
mutation($dId: ID!, $body: String!, $replyToId: ID!) {
252-
addDiscussionComment(input: { discussionId: $dId, body: $body, replyToId: $replyToId }) {
253-
comment {
254-
id
255-
url
256-
}
257-
}
258-
}`,
259-
{ dId: discussionId, body: commentBody, replyToId: commentNodeId }
260-
);
261-
262-
const comment = result.addDiscussionComment.comment;
263-
setCommentOutputs(comment.id, comment.url);
241+
await postDiscussionComment(discussionNumber, commentBody, commentNodeId);
264242
return;
265243
}
266244

267245
// Create a new comment for non-discussion events
268246
const createResponse = await github.request("POST " + endpoint, {
269247
body: commentBody,
270-
headers: {
271-
Accept: "application/vnd.github+json",
272-
},
248+
headers: { Accept: "application/vnd.github+json" },
273249
});
274250

275251
setCommentOutputs(createResponse.data.id, createResponse.data.html_url);
276252
}
277253

278-
module.exports = { main, addCommentWithWorkflowLink };
254+
module.exports = { main, addCommentWithWorkflowLink, buildCommentBody, postDiscussionComment };

actions/setup/js/add_workflow_run_comment.test.cjs

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,20 @@ describe("add_workflow_run_comment", () => {
230230
);
231231
expect(mockCore.setFailed).not.toHaveBeenCalled();
232232
});
233+
234+
it("should fail when PR number is missing in pull_request_review_comment event", async () => {
235+
global.context = {
236+
eventName: "pull_request_review_comment",
237+
runId: 12345,
238+
repo: { owner: "testowner", repo: "testrepo" },
239+
payload: {},
240+
};
241+
242+
await runScript();
243+
244+
expect(mockCore.setFailed).toHaveBeenCalledWith(`${ERR_NOT_FOUND}: Pull request number not found in event payload`);
245+
expect(mockGithub.request).not.toHaveBeenCalled();
246+
});
233247
});
234248

235249
describe("main() - discussion event", () => {
@@ -341,9 +355,9 @@ describe("add_workflow_run_comment", () => {
341355

342356
await runScript();
343357

344-
expect(mockCore.error).toHaveBeenCalledWith(expect.stringContaining("Failed to create comment"));
345358
expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to create comment with workflow link"));
346-
// Should NOT call setFailed - errors should only warn
359+
// Should NOT use core.error or core.setFailed for non-critical errors
360+
expect(mockCore.error).not.toHaveBeenCalled();
347361
expect(mockCore.setFailed).not.toHaveBeenCalled();
348362
});
349363
});
@@ -475,4 +489,84 @@ describe("add_workflow_run_comment", () => {
475489
);
476490
});
477491
});
492+
493+
describe("buildCommentBody()", () => {
494+
it("should include the run URL in the comment body", async () => {
495+
const { buildCommentBody } = await import("./add_workflow_run_comment.cjs?" + Date.now());
496+
const body = buildCommentBody("issues", "https://github.com/testowner/testrepo/actions/runs/99");
497+
expect(body).toContain("https://github.com/testowner/testrepo/actions/runs/99");
498+
});
499+
500+
it("should always include reaction comment type marker", async () => {
501+
const { buildCommentBody } = await import("./add_workflow_run_comment.cjs?" + Date.now());
502+
const body = buildCommentBody("issues", "https://example.com/run/1");
503+
expect(body).toContain("<!-- gh-aw-comment-type: reaction -->");
504+
});
505+
506+
it("should include workflow-id marker when GITHUB_WORKFLOW is set", async () => {
507+
process.env.GITHUB_WORKFLOW = "my-workflow.yml";
508+
const { buildCommentBody } = await import("./add_workflow_run_comment.cjs?" + Date.now());
509+
const body = buildCommentBody("issues", "https://example.com/run/1");
510+
expect(body).toContain("<!-- gh-aw-workflow-id: my-workflow.yml -->");
511+
});
512+
513+
it("should include tracker-id marker when GH_AW_TRACKER_ID is set", async () => {
514+
process.env.GH_AW_TRACKER_ID = "my-tracker";
515+
const { buildCommentBody } = await import("./add_workflow_run_comment.cjs?" + Date.now());
516+
const body = buildCommentBody("issues", "https://example.com/run/1");
517+
expect(body).toContain("<!-- gh-aw-tracker-id: my-tracker -->");
518+
});
519+
520+
it("should add lock notice for issues event when GH_AW_LOCK_FOR_AGENT=true", async () => {
521+
process.env.GH_AW_LOCK_FOR_AGENT = "true";
522+
const { buildCommentBody } = await import("./add_workflow_run_comment.cjs?" + Date.now());
523+
const body = buildCommentBody("issues", "https://example.com/run/1");
524+
expect(body).toContain("🔒 This issue has been locked");
525+
});
526+
527+
it("should not add lock notice for pull_request events", async () => {
528+
process.env.GH_AW_LOCK_FOR_AGENT = "true";
529+
const { buildCommentBody } = await import("./add_workflow_run_comment.cjs?" + Date.now());
530+
const body = buildCommentBody("pull_request", "https://example.com/run/1");
531+
expect(body).not.toContain("🔒 This issue has been locked");
532+
});
533+
534+
it("should use unknown event type description for unrecognized events", async () => {
535+
const { buildCommentBody } = await import("./add_workflow_run_comment.cjs?" + Date.now());
536+
// Should not throw for unknown event types
537+
const body = buildCommentBody("some_unknown_event", "https://example.com/run/1");
538+
expect(body).toBeTruthy();
539+
expect(body).toContain("<!-- gh-aw-comment-type: reaction -->");
540+
});
541+
});
542+
543+
describe("postDiscussionComment()", () => {
544+
it("should post a top-level discussion comment when no replyToNodeId", async () => {
545+
const { postDiscussionComment } = await import("./add_workflow_run_comment.cjs?" + Date.now());
546+
await postDiscussionComment(10, "Test body");
547+
548+
expect(mockGithub.graphql).toHaveBeenCalled();
549+
const mutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("addDiscussionComment"));
550+
expect(mutationCall).toBeDefined();
551+
expect(mutationCall[1]).toMatchObject({ body: "Test body" });
552+
expect(mutationCall[1]).not.toHaveProperty("replyToId");
553+
});
554+
555+
it("should post a threaded comment when replyToNodeId is provided", async () => {
556+
const { postDiscussionComment } = await import("./add_workflow_run_comment.cjs?" + Date.now());
557+
await postDiscussionComment(10, "Reply body", "DC_kwParent123");
558+
559+
const mutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("replyToId"));
560+
expect(mutationCall).toBeDefined();
561+
expect(mutationCall[1]).toMatchObject({ body: "Reply body", replyToId: "DC_kwParent123" });
562+
});
563+
564+
it("should set comment outputs after posting", async () => {
565+
const { postDiscussionComment } = await import("./add_workflow_run_comment.cjs?" + Date.now());
566+
await postDiscussionComment(10, "Test body");
567+
568+
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-id", "DC_kwDOTest456");
569+
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", expect.stringContaining("discussioncomment-456"));
570+
});
571+
});
478572
});

0 commit comments

Comments
 (0)