diff --git a/.github/workflows/draft-pr-cleanup.lock.yml b/.github/workflows/draft-pr-cleanup.lock.yml index 2fd6c2a283..a3f96ea013 100644 --- a/.github/workflows/draft-pr-cleanup.lock.yml +++ b/.github/workflows/draft-pr-cleanup.lock.yml @@ -321,7 +321,7 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs cat > /opt/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' - {"add_comment":{"max":20},"add_labels":{"max":20},"missing_data":{},"missing_tool":{},"noop":{"max":1}} + {"add_comment":{"max":20},"add_labels":{"max":20},"close_pull_request":{"max":10,"target":"*"},"missing_data":{},"missing_tool":{},"noop":{"max":1}} GH_AW_SAFE_OUTPUTS_CONFIG_EOF - name: Write Safe Outputs Tools run: | @@ -559,6 +559,24 @@ jobs: } } }, + "close_pull_request": { + "defaultMax": 1, + "fields": { + "body": { + "required": true, + "type": "string", + "sanitize": true, + "maxLength": 65000 + }, + "pull_request_number": { + "optionalPositiveInteger": true + }, + "repo": { + "type": "string", + "maxLength": 256 + } + } + }, "missing_data": { "defaultMax": 20, "fields": { diff --git a/.github/workflows/poem-bot.lock.yml b/.github/workflows/poem-bot.lock.yml index 31ba9c75f5..d6f35096f6 100644 --- a/.github/workflows/poem-bot.lock.yml +++ b/.github/workflows/poem-bot.lock.yml @@ -417,7 +417,7 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs cat > /opt/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' - {"add_comment":{"max":3,"target":"*"},"add_labels":{"allowed":["poetry","creative","automation","ai-generated","epic","haiku","sonnet","limerick"],"max":5},"create_agent_session":{"max":1},"create_discussion":{"expires":24,"max":2},"create_issue":{"expires":48,"group":true,"max":2},"create_missing_tool_issue":{"max":1,"title_prefix":"[missing tool]"},"create_pull_request":{"expires":48,"max":1,"reviewers":["copilot"],"title_prefix":"[🎨 POETRY] "},"create_pull_request_review_comment":{"max":2},"link_sub_issue":{"max":3},"missing_data":{},"missing_tool":{},"noop":{"max":1},"push_to_pull_request_branch":{"max":1},"update_issue":{"max":2},"upload_asset":{"max":0}} + {"add_comment":{"max":3,"target":"*"},"add_labels":{"allowed":["poetry","creative","automation","ai-generated","epic","haiku","sonnet","limerick"],"max":5},"close_pull_request":{"max":2,"required_labels":["poetry","automation"],"required_title_prefix":"[🎨 POETRY]","target":"*"},"create_agent_session":{"max":1},"create_discussion":{"expires":24,"max":2},"create_issue":{"expires":48,"group":true,"max":2},"create_missing_tool_issue":{"max":1,"title_prefix":"[missing tool]"},"create_pull_request":{"expires":48,"max":1,"reviewers":["copilot"],"title_prefix":"[🎨 POETRY] "},"create_pull_request_review_comment":{"max":2},"link_sub_issue":{"max":3},"missing_data":{},"missing_tool":{},"noop":{"max":1},"push_to_pull_request_branch":{"max":1},"update_issue":{"max":2},"upload_asset":{"max":0}} GH_AW_SAFE_OUTPUTS_CONFIG_EOF - name: Write Safe Outputs Tools run: | @@ -1034,6 +1034,24 @@ jobs: } } }, + "close_pull_request": { + "defaultMax": 1, + "fields": { + "body": { + "required": true, + "type": "string", + "sanitize": true, + "maxLength": 65000 + }, + "pull_request_number": { + "optionalPositiveInteger": true + }, + "repo": { + "type": "string", + "maxLength": 256 + } + } + }, "create_agent_session": { "defaultMax": 1, "fields": { diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index bce7941e5a..569fc510b6 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -843,7 +843,7 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs cat > /opt/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' - {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"push_to_pull_request_branch":{"max":1,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} + {"add_comment":{"max":2},"add_labels":{"allowed":["smoke-claude"],"max":3},"add_reviewer":{"max":2},"close_pull_request":{"max":1,"staged":true},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1},"push_to_pull_request_branch":{"max":1,"target":"*"},"resolve_pull_request_review_thread":{"max":5},"submit_pull_request_review":{"max":1},"update_pull_request":{"max":1}} GH_AW_SAFE_OUTPUTS_CONFIG_EOF - name: Write Safe Outputs Tools run: | @@ -1375,6 +1375,24 @@ jobs: } } }, + "close_pull_request": { + "defaultMax": 1, + "fields": { + "body": { + "required": true, + "type": "string", + "sanitize": true, + "maxLength": 65000 + }, + "pull_request_number": { + "optionalPositiveInteger": true + }, + "repo": { + "type": "string", + "maxLength": 256 + } + } + }, "create_issue": { "defaultMax": 1, "fields": { diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index 9d6acfce49..bc9c59e2a4 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -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}`); } @@ -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)) { @@ -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 { @@ -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}`, }; } @@ -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 @@ -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({ @@ -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. @@ -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 @@ -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 @@ -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); @@ -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 diff --git a/actions/setup/js/add_comment.test.cjs b/actions/setup/js/add_comment.test.cjs index e2304ce393..54061e4a3c 100644 --- a/actions/setup/js/add_comment.test.cjs +++ b/actions/setup/js/add_comment.test.cjs @@ -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 () => {