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
80 changes: 79 additions & 1 deletion actions/setup/js/add_comment.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,88 @@ async function main(config = {}) {
} catch (error) {
const errorMessage = getErrorMessage(error);

// Check if this is a 404 error (discussion/issue was deleted)
// Check if this is a 404 error (discussion/issue was deleted or wrong type)
// @ts-expect-error - Error handling with optional chaining
const is404 = error?.status === 404 || errorMessage.includes("404") || errorMessage.toLowerCase().includes("not found");

// If 404 and item_number was explicitly provided and we tried as issue/PR,
// retry as a discussion (the user may have provided a discussion number)
if (is404 && !isDiscussion && item.item_number !== undefined && item.item_number !== null) {
core.info(`Item #${itemNumber} not found as issue/PR, retrying as discussion...`);

try {
// Try to find and comment on the discussion
const discussionQuery = `
query($owner: String!, $repo: String!, $number: Int!) {
repository(owner: $owner, name: $repo) {
discussion(number: $number) {
id
}
}
}
`;
const queryResult = await github.graphql(discussionQuery, {
owner: repoParts.owner,
repo: repoParts.repo,
number: itemNumber,
});

const discussionId = queryResult?.repository?.discussion?.id;
if (!discussionId) {
throw new Error(`Discussion #${itemNumber} not found in ${itemRepo}`);
}

core.info(`Found discussion #${itemNumber}, adding comment...`);
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The fallback logic does not respect the hide-older-comments configuration. When a 404 occurs and the code retries as a discussion, it directly adds the comment without calling hideOlderComments. This means that if hide-older-comments is enabled and the fallback succeeds, older comments won't be hidden as expected. Consider calling hideOlderComments before adding the comment in the fallback path, similar to the primary flow at lines 433-437.

Suggested change
core.info(`Found discussion #${itemNumber}, adding comment...`);
core.info(`Found discussion #${itemNumber}, adding comment...`);
// Respect hide-older-comments configuration in the fallback-to-discussion path
if (config["hide-older-comments"]) {
await hideOlderComments(github, {
owner: repoParts.owner,
repo: repoParts.repo,
number: itemNumber,
isDiscussion: true,
});
}

Copilot uses AI. Check for mistakes.
const comment = await commentOnDiscussion(github, repoParts.owner, repoParts.repo, itemNumber, processedBody, null);

Comment on lines +512 to +535
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

The GraphQL query to check if the discussion exists (lines 513-521) duplicates logic that already exists in the commentOnDiscussion function (lines 215-226). The commentOnDiscussion function already retrieves the discussion ID and throws an error if the discussion is not found. This duplicated query adds an unnecessary API call. Consider removing this duplicate check and directly calling commentOnDiscussion, then handling the error from that function.

Suggested change
// Try to find and comment on the discussion
const discussionQuery = `
query($owner: String!, $repo: String!, $number: Int!) {
repository(owner: $owner, name: $repo) {
discussion(number: $number) {
id
}
}
}
`;
const queryResult = await github.graphql(discussionQuery, {
owner: repoParts.owner,
repo: repoParts.repo,
number: itemNumber,
});
const discussionId = queryResult?.repository?.discussion?.id;
if (!discussionId) {
throw new Error(`Discussion #${itemNumber} not found in ${itemRepo}`);
}
core.info(`Found discussion #${itemNumber}, adding comment...`);
const comment = await commentOnDiscussion(github, repoParts.owner, repoParts.repo, itemNumber, processedBody, null);
core.info(`Attempting to add comment to discussion #${itemNumber}...`);
const comment = await commentOnDiscussion(
github,
repoParts.owner,
repoParts.repo,
itemNumber,
processedBody,
null
);

Copilot uses AI. Check for mistakes.
core.info(`Created comment on discussion: ${comment.html_url}`);

// Add tracking metadata
const commentResult = {
id: comment.id,
html_url: comment.html_url,
_tracking: {
commentId: comment.id,
itemNumber: itemNumber,
repo: itemRepo,
isDiscussion: true,
},
};

createdComments.push(commentResult);

return {
success: true,
commentId: comment.id,
url: comment.html_url,
itemNumber: itemNumber,
repo: itemRepo,
isDiscussion: true,
};
} catch (discussionError) {
const discussionErrorMessage = getErrorMessage(discussionError);
// @ts-expect-error - Error handling with optional chaining
const isDiscussion404 = discussionError?.status === 404 || discussionErrorMessage.toLowerCase().includes("not found");

if (isDiscussion404) {
// Neither issue/PR nor discussion found - truly doesn't exist
core.warning(`Target #${itemNumber} was not found as issue, PR, or discussion (may have been deleted): ${discussionErrorMessage}`);
return {
success: true,
warning: `Target not found: ${discussionErrorMessage}`,
skipped: true,
};
}

// Other error when trying as discussion
core.error(`Failed to add comment to discussion: ${discussionErrorMessage}`);
return {
success: false,
error: discussionErrorMessage,
};
}
}

if (is404) {
// Treat 404s as warnings - the target was deleted between execution and safe output processing
core.warning(`Target was not found (may have been deleted): ${errorMessage}`);
Expand Down
207 changes: 207 additions & 0 deletions actions/setup/js/add_comment.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -745,4 +745,211 @@ describe("add_comment", () => {
expect(errorCalls.length).toBeGreaterThan(0);
});
});

describe("discussion fallback", () => {
it("should retry as discussion when item_number returns 404 as issue/PR", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

let infoCalls = [];
mockCore.info = msg => {
infoCalls.push(msg);
};

// Mock REST API to return 404 (not found as issue/PR)
mockGithub.rest.issues.createComment = async () => {
const error = new Error("Not Found");
// @ts-ignore
error.status = 404;
throw error;
};

// Mock GraphQL to return discussion
let graphqlCalls = [];
mockGithub.graphql = async (query, vars) => {
graphqlCalls.push({ query, vars });

// First call is to check if discussion exists
if (query.includes("query") && query.includes("discussion(number:")) {
return {
repository: {
discussion: {
id: "D_kwDOTest789",
url: "https://github.com/owner/repo/discussions/14117",
},
},
};
}

// Second call is to add comment
if (query.includes("mutation") && query.includes("addDiscussionComment")) {
return {
addDiscussionComment: {
comment: {
id: "DC_kwDOTest999",
body: "Test comment",
createdAt: "2026-02-06T12:00:00Z",
url: "https://github.com/owner/repo/discussions/14117#discussioncomment-999",
},
},
};
}
};

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ target: 'triggering' }); })()`);

const message = {
type: "add_comment",
item_number: 14117,
body: "Test comment on discussion",
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.isDiscussion).toBe(true);
expect(result.itemNumber).toBe(14117);
expect(result.url).toContain("discussions/14117");

// Verify it logged the retry
const retryLog = infoCalls.find(msg => msg.includes("retrying as discussion"));
expect(retryLog).toBeTruthy();

const foundLog = infoCalls.find(msg => msg.includes("Found discussion"));
expect(foundLog).toBeTruthy();
});

it("should return skipped when item_number not found as issue/PR or discussion", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

let warningCalls = [];
mockCore.warning = msg => {
warningCalls.push(msg);
};

// Mock REST API to return 404
mockGithub.rest.issues.createComment = async () => {
const error = new Error("Not Found");
// @ts-ignore
error.status = 404;
throw error;
};

// Mock GraphQL to also return 404 (discussion doesn't exist either)
mockGithub.graphql = async (query, vars) => {
if (query.includes("query") && query.includes("discussion(number:")) {
return {
repository: {
discussion: null,
},
};
}
};

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ target: 'triggering' }); })()`);

const message = {
type: "add_comment",
item_number: 99999,
body: "Test comment",
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.skipped).toBe(true);
expect(result.warning).toContain("not found");

// Verify warning was logged
const notFoundWarning = warningCalls.find(msg => msg.includes("not found"));
expect(notFoundWarning).toBeTruthy();
});

it("should not retry as discussion when 404 occurs without explicit item_number", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

let warningCalls = [];
mockCore.warning = msg => {
warningCalls.push(msg);
};

// Mock REST API to return 404
mockGithub.rest.issues.createComment = async () => {
const error = new Error("Not Found");
// @ts-ignore
error.status = 404;
throw error;
};

// GraphQL should not be called
let graphqlCalled = false;
mockGithub.graphql = async () => {
graphqlCalled = true;
throw new Error("GraphQL should not be called");
};

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ target: 'triggering' }); })()`);

const message = {
type: "add_comment",
// No item_number - using target resolution
body: "Test comment",
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.skipped).toBe(true);
expect(graphqlCalled).toBe(false);

// Verify warning was logged
const notFoundWarning = warningCalls.find(msg => msg.includes("not found"));
expect(notFoundWarning).toBeTruthy();
});

it("should not retry as discussion when already detected as discussion context", async () => {
const addCommentScript = fs.readFileSync(path.join(__dirname, "add_comment.cjs"), "utf8");

// Set discussion context
mockContext.eventName = "discussion";
mockContext.payload = {
discussion: {
number: 100,
},
};

let warningCalls = [];
mockCore.warning = msg => {
warningCalls.push(msg);
};

// Mock GraphQL to return 404 for discussion
let graphqlCallCount = 0;
mockGithub.graphql = async (query, vars) => {
graphqlCallCount++;

if (query.includes("query") && query.includes("discussion(number:")) {
return {
repository: {
discussion: null,
},
};
}
};

const handler = await eval(`(async () => { ${addCommentScript}; return await main({ target: 'triggering' }); })()`);

const message = {
type: "add_comment",
body: "Test comment",
};

const result = await handler(message, {});

expect(result.success).toBe(true);
expect(result.skipped).toBe(true);

// Should only call GraphQL once (not retry)
expect(graphqlCallCount).toBe(1);
});
});
});
Loading