Skip to content

[jsweep] Clean add_comment.cjs #20900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 2 commits into from
Mar 14, 2026
Merged

[jsweep] Clean add_comment.cjs #20900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25a954b
jsweep: clean add_comment.cjs
github-actions[bot] Mar 14, 2026
14c7b1b
chore: recompile lock files
Copilot Mar 14, 2026
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
20 changes: 19 additions & 1 deletion .github/workflows/draft-pr-cleanup.lock.yml
View file
Open in desktop

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

20 changes: 19 additions & 1 deletion .github/workflows/poem-bot.lock.yml
View file
Open in desktop

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

20 changes: 19 additions & 1 deletion .github/workflows/smoke-claude.lock.yml
View file
Open in desktop

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

124 changes: 21 additions & 103 deletions actions/setup/js/add_comment.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ async function hideOlderComments(github, owner, repo, itemNumber, workflowId, is
const nodeId = isDiscussion ? String(comment.id) : comment.node_id;
core.info(`Hiding comment: ${nodeId}`);

const result = await minimizeComment(github, nodeId, normalizedReason);
await minimizeComment(github, nodeId, normalizedReason);
hiddenCount++;
core.info(`✓ Hidden comment: ${nodeId}`);
}
Expand Down Expand Up @@ -357,8 +357,6 @@ async function main(config = {}) {

processedCount++;

const item = message;

// Merge resolved temp IDs
if (resolvedTemporaryIds) {
for (const [tempId, resolved] of Object.entries(resolvedTemporaryIds)) {
Expand All @@ -369,7 +367,7 @@ async function main(config = {}) {
}

// Resolve and validate target repository
const repoResult = resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, "comment");
const repoResult = resolveAndValidateRepo(message, defaultTargetRepo, allowedRepos, "comment");
if (!repoResult.success) {
core.warning(`Skipping comment: ${repoResult.error}`);
return {
Expand All @@ -385,26 +383,26 @@ async function main(config = {}) {
let isDiscussion = false;

// Check if item_number was explicitly provided in the message
if (item.item_number !== undefined && item.item_number !== null) {
if (message.item_number !== undefined && message.item_number !== null) {
// Resolve temporary IDs if present
const resolvedTarget = resolveRepoIssueTarget(item.item_number, temporaryIdMap, repoParts.owner, repoParts.repo);
const resolvedTarget = resolveRepoIssueTarget(message.item_number, temporaryIdMap, repoParts.owner, repoParts.repo);

// Check if this is an unresolved temporary ID
if (resolvedTarget.wasTemporaryId && !resolvedTarget.resolved) {
core.info(`Deferring add_comment: unresolved temporary ID (${item.item_number})`);
core.info(`Deferring add_comment: unresolved temporary ID (${message.item_number})`);
return {
success: false,
deferred: true,
error: resolvedTarget.errorMessage || `Unresolved temporary ID: ${item.item_number}`,
error: resolvedTarget.errorMessage || `Unresolved temporary ID: ${message.item_number}`,
};
}

// Check for other resolution errors (including null resolved)
if (resolvedTarget.errorMessage || !resolvedTarget.resolved) {
core.warning(`Invalid item_number specified: ${item.item_number}`);
core.warning(`Invalid item_number specified: ${message.item_number}`);
return {
success: false,
error: `Invalid item_number specified: ${item.item_number}`,
error: `Invalid item_number specified: ${message.item_number}`,
};
}

Expand Down Expand Up @@ -433,7 +431,7 @@ async function main(config = {}) {
// For issues/PRs, use the resolveTarget helper which respects target configuration
const targetResult = resolveTarget({
targetConfig: commentTarget,
item: item,
item: message,
context: context,
itemType: "add_comment",
supportsPR: true, // add_comment supports both issues and PRs
Expand All @@ -460,7 +458,7 @@ async function main(config = {}) {
// author so the second sanitization pass does not accidentally strip them.
const parentAuthors = [];
if (!isDiscussion) {
if (item.item_number !== undefined && item.item_number !== null) {
if (message.item_number !== undefined && message.item_number !== null) {
// Explicit item_number: fetch the issue/PR to get its author
try {
const { data: issueData } = await githubClient.rest.issues.get({
Expand Down Expand Up @@ -494,7 +492,7 @@ async function main(config = {}) {
}

// Replace temporary ID references in body
let processedBody = replaceTemporaryIdReferences(item.body || "", temporaryIdMap, itemRepo);
let processedBody = replaceTemporaryIdReferences(message.body || "", temporaryIdMap, itemRepo);

// Sanitize content to prevent injection attacks, allowing parent issue/PR/discussion authors
// so they can be @mentioned in the generated comment.
Expand Down Expand Up @@ -583,6 +581,12 @@ async function main(config = {}) {
};
}

// Records a created comment in createdComments and returns the success result.
const recordComment = (/** @type {{ id: string | number, html_url: string }} */ comment, /** @type {boolean} */ isDiscussionFlag) => {
createdComments.push({ id: comment.id, html_url: comment.html_url, _tracking: { commentId: comment.id, itemNumber, repo: itemRepo, isDiscussion: isDiscussionFlag } });
return { success: true, commentId: comment.id, url: comment.html_url, itemNumber, repo: itemRepo, isDiscussion: isDiscussionFlag };
};

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
Expand All @@ -595,27 +599,6 @@ async function main(config = {}) {
/** @type {{ id: string | number, html_url: string }} */
let comment;
if (isDiscussion) {
// Use GraphQL for discussions
const discussionQuery = `
query($owner: String!, $repo: String!, $number: Int!) {
repository(owner: $owner, name: $repo) {
discussion(number: $number) {
id
}
}
}
`;
const queryResult = await githubClient.graphql(discussionQuery, {
owner: repoParts.owner,
repo: repoParts.repo,
number: itemNumber,
});

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

comment = await commentOnDiscussion(githubClient, repoParts.owner, repoParts.repo, itemNumber, processedBody, null);
} else {
// Use REST API for issues/PRs
Expand All @@ -629,29 +612,7 @@ async function main(config = {}) {
}

core.info(`Created comment: ${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: isDiscussion,
},
};

createdComments.push(commentResult);

return {
success: true,
commentId: comment.id,
url: comment.html_url,
itemNumber: itemNumber,
repo: itemRepo,
isDiscussion: isDiscussion,
};
return recordComment(comment, isDiscussion);
} catch (error) {
const errorMessage = getErrorMessage(error);

Expand All @@ -661,58 +622,15 @@ async function main(config = {}) {

// 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) {
if (is404 && !isDiscussion && message.item_number !== undefined && message.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 githubClient.graphql(discussionQuery, {
owner: repoParts.owner,
repo: repoParts.repo,
number: itemNumber,
});

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

core.info(`Found discussion #${itemNumber}, adding comment...`);
core.info(`Trying #${itemNumber} as discussion...`);
const comment = await commentOnDiscussion(githubClient, repoParts.owner, repoParts.repo, itemNumber, processedBody, null);

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,
};
return recordComment(comment, true);
} catch (discussionError) {
const discussionErrorMessage = getErrorMessage(discussionError);
// @ts-expect-error - Error handling with optional chaining
Expand Down
4 changes: 2 additions & 2 deletions actions/setup/js/add_comment.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,8 +879,8 @@ describe("add_comment", () => {
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();
const createdLog = infoCalls.find(msg => msg.includes("Created comment on discussion"));
expect(createdLog).toBeTruthy();
});

it("should return skipped when item_number not found as issue/PR or discussion", async () => {
Expand Down
Loading