Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/aw-failure-investigator.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 59 additions & 1 deletion actions/setup/js/add_workflow_run_comment.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,54 @@ function reportCommentError(rawContext, message) {
core.setFailed(message);
}

/**
* @param {ReusableStatusComment} reusableComment
* @param {{
* source: "native" | "workflow_dispatch" | "repository_dispatch";
* eventName: string;
* eventPayload: any;
* workflowRepo: { owner: string, repo: string };
* eventRepo: { owner: string, repo: string };
* }} invocationContext
* @param {any} rawContext
* @returns {Promise<string>}
*/
async function updateReusableStatusComment(reusableComment, invocationContext, rawContext) {
const runUrl = buildWorkflowRunUrl(rawContext, invocationContext.workflowRepo);
const commentBody = buildCommentBody(invocationContext.eventName, runUrl);

// Discussion comments use GraphQL node IDs and a dedicated update mutation.
if (reusableComment.id.startsWith("DC_")) {
const result = await github.graphql(
`
mutation($commentId: ID!, $body: String!) {
updateDiscussionComment(input: { commentId: $commentId, body: $body }) {
comment { id url }
}
}`,
{ commentId: reusableComment.id, body: commentBody }
);
const updatedUrl = result?.updateDiscussionComment?.comment?.url;
return typeof updatedUrl === "string" && updatedUrl.trim() ? updatedUrl : reusableComment.url;
}

const commentRepo = reusableComment.repo || invocationContext.eventRepo;
const numericCommentId = Number(reusableComment.id);
if (!Number.isInteger(numericCommentId) || numericCommentId <= 0) {
throw new Error(`${ERR_VALIDATION}: Reusable status comment ID must be a positive integer (received "${reusableComment.id}")`);
}

const response = await github.request("PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}", {
owner: commentRepo.owner,
repo: commentRepo.repo,
comment_id: numericCommentId,
body: commentBody,
headers: { Accept: "application/vnd.github+json" },
});
const updatedUrl = response?.data?.html_url;
return typeof updatedUrl === "string" && updatedUrl.trim() ? updatedUrl : reusableComment.url;
}

/**
* Add a comment with a workflow run link to the triggering item.
* This script ONLY creates comments - it does NOT add reactions.
Expand All @@ -190,7 +238,17 @@ async function createOrReuseStatusComment(rawContext = context) {
if (!reusableComment.repo) {
core.warning("Reusable status comment repo missing; falling back to the invocation event repo.");
}
const outputs = setCommentOutputs(reusableComment.id, reusableComment.url, reusableComment.repo || invocationContext.eventRepo, { logReuse: true });
let reusableCommentUrl = reusableComment.url;
try {
reusableCommentUrl = await updateReusableStatusComment(reusableComment, invocationContext, rawContext);
core.info("Updated reusable status comment with current workflow run metadata");
} catch (error) {
core.warning(`Failed to update reusable status comment body: ${getErrorMessage(error)}`);
Comment thread
pelikhan marked this conversation as resolved.
if (!reusableCommentUrl) {
core.warning("No fallback reusable status comment URL available; comment-url output will be empty.");
}
}
const outputs = setCommentOutputs(reusableComment.id, reusableCommentUrl, reusableComment.repo || invocationContext.eventRepo, { logReuse: true });
return {
...outputs,
reused: true,
Expand Down
147 changes: 143 additions & 4 deletions actions/setup/js/add_workflow_run_comment.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,12 @@ describe("add_workflow_run_comment", () => {
});

it("reuses an existing status comment from client_payload aw_context", async () => {
mockGithub.request.mockResolvedValueOnce({
data: {
id: 67890,
html_url: "https://github.com/statusowner/statusrepo/issues/789#issuecomment-67890",
},
});
global.context = {
eventName: "repository_dispatch",
runId: 12345,
Expand All @@ -233,16 +239,30 @@ describe("add_workflow_run_comment", () => {

await runScript();

expect(mockGithub.request).not.toHaveBeenCalled();
expect(mockGithub.request).toHaveBeenCalledWith(
"PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}",
expect.objectContaining({
owner: "statusowner",
repo: "statusrepo",
comment_id: 67890,
body: expect.stringContaining("https://github.com/workflowowner/workflowrepo/actions/runs/12345"),
})
);
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-id", "67890");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", "https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", "https://github.com/statusowner/statusrepo/issues/789#issuecomment-67890");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-repo", "statusowner/statusrepo");
expect(mockCore.info).toHaveBeenCalledWith("Reusing existing status comment outputs");
});
});

describe("main() - workflow_dispatch aw_context reuse", () => {
it("reuses an existing status comment from aw_context", async () => {
mockGithub.request.mockResolvedValueOnce({
data: {
id: 67890,
html_url: "https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890",
},
});
global.context = {
eventName: "workflow_dispatch",
runId: 12345,
Expand All @@ -263,14 +283,28 @@ describe("add_workflow_run_comment", () => {

await runScript();

expect(mockGithub.request).not.toHaveBeenCalled();
expect(mockGithub.request).toHaveBeenCalledWith(
"PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}",
expect.objectContaining({
owner: "targetowner",
repo: "targetrepo",
comment_id: 67890,
body: expect.stringContaining("https://github.com/workflowowner/workflowrepo/actions/runs/12345"),
})
);
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-id", "67890");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", "https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-repo", "targetowner/targetrepo");
expect(mockCore.setFailed).not.toHaveBeenCalled();
});

it("reuses an existing status comment from camelCase awContext", async () => {
mockGithub.request.mockResolvedValueOnce({
data: {
id: 67890,
html_url: "https://github.com/statusowner/statusrepo/issues/789#issuecomment-67890",
},
});
global.context = {
eventName: "workflow_dispatch",
runId: 12345,
Expand All @@ -292,8 +326,113 @@ describe("add_workflow_run_comment", () => {

await runScript();

expect(mockGithub.request).not.toHaveBeenCalled();
expect(mockGithub.request).toHaveBeenCalledWith(
"PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}",
expect.objectContaining({
owner: "statusowner",
repo: "statusrepo",
comment_id: 67890,
body: expect.stringContaining("https://github.com/workflowowner/workflowrepo/actions/runs/12345"),
})
);
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-id", "67890");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", "https://github.com/statusowner/statusrepo/issues/789#issuecomment-67890");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-repo", "statusowner/statusrepo");
expect(mockCore.info).toHaveBeenCalledWith("Reusing existing status comment outputs");
expect(mockCore.setFailed).not.toHaveBeenCalled();
});

it("updates reusable discussion comments via GraphQL", async () => {
mockGithub.graphql.mockResolvedValue({
updateDiscussionComment: {
comment: {
id: "DC_kwDOReusable123",
url: "https://github.com/targetowner/targetrepo/discussions/789#discussioncomment-67890",
},
},
});
global.context = {
eventName: "workflow_dispatch",
runId: 12345,
repo: { owner: "workflowowner", repo: "workflowrepo" },
payload: {
inputs: {
aw_context: JSON.stringify({
repo: "targetowner/targetrepo",
event_type: "discussion_comment",
item_type: "discussion",
item_number: 789,
status_comment_id: "DC_kwDOReusable123",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/diagnose] status_comment_url is intentionally absent here, but this creates a latent bug: if github.graphql rejects, the catch block falls back to reusableComment.url, which will be "" (empty string from readReusableStatusComment). The comment-url output would then be empty.

Consider either adding a status_comment_url to this test fixture (to cover the fallback case explicitly) and adding a companion failure-path test that asserts the empty-string fallback, or documenting that empty status_comment_url is intentional for fresh DC_ comments.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in 0491705: when update fails and no fallback URL is available, the code now emits an explicit warning (comment-url output will be empty) to avoid silent behavior.

}),
},
},
};

await runScript();

expect(mockGithub.graphql).toHaveBeenCalledWith(
expect.stringContaining("updateDiscussionComment"),
expect.objectContaining({
commentId: "DC_kwDOReusable123",
body: expect.stringContaining("https://github.com/workflowowner/workflowrepo/actions/runs/12345"),
})
);
expect(mockGithub.request).not.toHaveBeenCalled();
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-id", "DC_kwDOReusable123");
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", "https://github.com/targetowner/targetrepo/discussions/789#discussioncomment-67890");
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/tdd] The non-fatal fallback path in createOrReuseStatusComment (the catch block that warns and continues with the stale URL) has no test coverage.

This is the core safety net of the non-fatal design: if updateReusableStatusComment throws, callers should see core.warning fired and the original reusableComment.url used as the output — not setFailed. Without a test, a future refactor that removes the try/catch would go undetected.

💡 Suggested sibling test
it('falls back to stale URL and warns when PATCH throws', async () => {
  mockGithub.request.mockRejectedValueOnce(new Error('network timeout'));
  global.context = {
    eventName: 'workflow_dispatch',
    runId: 12345,
    repo: { owner: 'workflowowner', repo: 'workflowrepo' },
    payload: {
      inputs: {
        aw_context: JSON.stringify({
          repo: 'targetowner/targetrepo',
          event_type: 'issue_comment',
          item_type: 'issue',
          item_number: 789,
          status_comment_id: 67890,
          status_comment_url: 'https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890',
        }),
      },
    },
  };

  await runScript();

  expect(mockCore.warning).toHaveBeenCalledWith(
    expect.stringContaining('Failed to update reusable status comment body')
  );
  expect(mockCore.setOutput).toHaveBeenCalledWith(
    'comment-url',
    'https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890'
  );
  expect(mockCore.setFailed).not.toHaveBeenCalled();
});

This makes the non-fatal contract explicit and regression-proof.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 0491705: a failure-path test now covers PATCH rejection, asserts warning emission, verifies stale URL fallback output, and confirms no setFailed.


it("warns and falls back to stale URL when reusable issue-comment update fails", async () => {
mockGithub.request.mockRejectedValueOnce(new Error("network timeout"));
global.context = {
eventName: "workflow_dispatch",
runId: 12345,
repo: { owner: "workflowowner", repo: "workflowrepo" },
payload: {
inputs: {
aw_context: JSON.stringify({
repo: "targetowner/targetrepo",
event_type: "issue_comment",
item_type: "issue",
item_number: 789,
status_comment_id: 67890,
status_comment_url: "https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890",
}),
},
},
};

await runScript();

expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to update reusable status comment body"));
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", "https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890");
expect(mockCore.setFailed).not.toHaveBeenCalled();
});

it("warns when reusable comment ID is not a positive integer", async () => {
global.context = {
eventName: "workflow_dispatch",
runId: 12345,
repo: { owner: "workflowowner", repo: "workflowrepo" },
payload: {
inputs: {
aw_context: JSON.stringify({
repo: "targetowner/targetrepo",
event_type: "issue_comment",
item_type: "issue",
item_number: 789,
status_comment_id: "67890abc",
status_comment_url: "https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890",
}),
},
},
};

await runScript();

expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining(`${ERR_VALIDATION}: Reusable status comment ID must be a positive integer (received "67890abc")`));
expect(mockCore.setOutput).toHaveBeenCalledWith("comment-url", "https://github.com/targetowner/targetrepo/issues/789#issuecomment-67890");
expect(mockCore.setFailed).not.toHaveBeenCalled();
});
});

Expand Down
Loading