From d93fa809cf443c768486ac6bb2e1f85a0455b317 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 12:03:19 +0000 Subject: [PATCH 1/3] Initial plan From 721ec00c7aba514ed6649e391814d7705bcf6c64 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 12:20:21 +0000 Subject: [PATCH 2/3] refactor: centralize close-flow in createCloseEntityHandler shared factory (#close-issue-close-pr-duplication) - Add createCloseEntityHandler() to close_entity_helpers.cjs handling the common pipeline: max-count gating, comment body resolution, sanitization, label/title-prefix filtering, staged-mode preview, comment posting with configurable error handling, entity close, and success result construction - Refactor close_issue.cjs to delegate to createCloseEntityHandler with issue-specific callbacks (cross-repo, all-labels-must-match, no footer, state_reason support) - Refactor close_pull_request.cjs to delegate to createCloseEntityHandler, removing duplicate local checkLabelFilter, checkTitlePrefixFilter and buildCommentBody (now imported from close_entity_helpers.cjs) - Update close_pull_request.test.cjs title-prefix error assertion to match the aligned error message format Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d384dacd-9154-4360-9586-b607087672e9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/close_entity_helpers.cjs | 223 +++++++++++++ actions/setup/js/close_issue.cjs | 265 ++++------------ actions/setup/js/close_pull_request.cjs | 315 ++++--------------- actions/setup/js/close_pull_request.test.cjs | 2 +- 4 files changed, 351 insertions(+), 454 deletions(-) diff --git a/actions/setup/js/close_entity_helpers.cjs b/actions/setup/js/close_entity_helpers.cjs index 5de4a8e9184..b7dea509f9b 100644 --- a/actions/setup/js/close_entity_helpers.cjs +++ b/actions/setup/js/close_entity_helpers.cjs @@ -9,6 +9,7 @@ const { getErrorMessage } = require("./error_helpers.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); const { isStagedMode } = require("./safe_output_helpers.cjs"); +const { logStagedPreviewInfo } = require("./staged_preview.cjs"); /** * @typedef {'issue' | 'pull_request'} EntityType @@ -204,6 +205,227 @@ function escapeMarkdownTitle(title) { return title.replace(/[[\]()]/g, "\\$&"); } +/** + * @typedef {Object} CloseEntityHandlerCallbacks + * @property {(item: Object, config: Object) => ({success: true, entityNumber: number, owner: string, repo: string, entityRepo?: string} | {success: false, error: string})} resolveTarget + * Resolves the entity number and target repository from the message and handler config. + * The factory passes both `item` and `config`; implementations may ignore `config` if not needed. + * @property {(github: any, owner: string, repo: string, entityNumber: number) => Promise<{number: number, title: string, labels: Array<{name: string}>, html_url: string, state: string}>} getDetails + * Fetches entity details from the GitHub API. + * @property {(entity: Object, entityNumber: number, requiredLabels: string[]) => {valid: true} | {valid: false, warning?: string, error: string}} validateLabels + * Validates entity labels against the required-labels filter. + * @property {(sanitizedBody: string, item: Object) => string} buildCommentBody + * Builds the final comment body from the already-sanitized body text. + * The factory passes both `sanitizedBody` and `item`; implementations may ignore `item` + * if they retrieve context values (e.g. triggering PR number) from the global `context` directly. + * @property {(github: any, owner: string, repo: string, entityNumber: number, body: string) => Promise<{id: number, html_url: string}>} addComment + * Posts a comment to the entity. + * @property {(github: any, owner: string, repo: string, entityNumber: number, item: Object, config: Object) => Promise<{number: number, html_url: string, title: string}>} closeEntity + * Closes the entity via the GitHub API. + * The factory passes `item` and `config` for implementations that need per-item overrides + * (e.g. `state_reason`); implementations that don't need them may ignore those parameters. + * @property {(closedEntity: Object, commentResult: Object|null, wasAlreadyClosed: boolean, commentPosted: boolean) => Object} buildSuccessResult + * Builds the success result object returned to the caller. + * @property {boolean} [continueOnCommentError] + * When true, a failed comment post is logged but does not abort the close operation. + * When false/omitted, a comment failure propagates and causes the handler to return an error. + */ + +/** + * Create a message-level close-entity handler function. + * + * Centralises the common close-flow pipeline: + * 1. Max-count gating + * 2. Comment body resolution (item.body → config.comment fallback) + * 3. Content sanitization + * 4. Target repository / entity number resolution (via callbacks.resolveTarget) + * 5. Entity details fetch + already-closed detection + * 6. Label filter validation (via callbacks.validateLabels) + * 7. Title-prefix filter validation + * 8. Staged-mode preview short-circuit + * 9. Comment posting (with optional continueOnCommentError) + * 10. Entity close (skipped when already closed) + * 11. Success result construction (via callbacks.buildSuccessResult) + * + * Entity-specific behaviour (API calls, label semantics, comment body + * construction, result shape, cross-repo support) is supplied through the + * callbacks argument so that each handler only retains the code that is + * genuinely unique to it. + * + * @param {Object} config - Handler configuration object from main() + * @param {EntityConfig} entityConfig - Entity display/type configuration + * @param {CloseEntityHandlerCallbacks} callbacks - Entity-specific callbacks + * @param {any} githubClient - Authenticated GitHub client + * @returns {Function} Async handler function suitable for use as a message handler + */ +function createCloseEntityHandler(config, entityConfig, callbacks, githubClient) { + const requiredLabels = config.required_labels || []; + const requiredTitlePrefix = config.required_title_prefix || ""; + const maxCount = config.max || 10; + const comment = config.comment || ""; + const isStaged = isStagedMode(config); + + let processedCount = 0; + + return async function handleCloseEntity(message, resolvedTemporaryIds) { + // 1. Max-count gating + if (processedCount >= maxCount) { + core.warning(`Skipping ${entityConfig.itemType}: max count of ${maxCount} reached`); + return { success: false, error: `Max count of ${maxCount} reached` }; + } + processedCount++; + + const item = message; + + // Log message structure for debugging (avoid logging body content) + const logFields = { has_body: !!item.body, body_length: item.body ? item.body.length : 0 }; + if (item[entityConfig.numberField] !== undefined) { + logFields[entityConfig.numberField] = item[entityConfig.numberField]; + } + if (item.repo !== undefined) { + logFields.has_repo = true; + } + core.info(`Processing ${entityConfig.itemType} message: ${JSON.stringify(logFields)}`); + + // 2. Comment body resolution + /** @type {string} */ + let commentToPost; + /** @type {string} */ + let commentSource = "unknown"; + + if (typeof item.body === "string" && item.body.trim() !== "") { + commentToPost = item.body; + commentSource = "item.body"; + } else if (typeof comment === "string" && comment.trim() !== "") { + commentToPost = comment; + commentSource = "config.comment"; + } else { + core.warning("No comment body provided in message and no default comment configured"); + return { success: false, error: "No comment body provided" }; + } + + core.info(`Comment body determined: length=${commentToPost.length}, source=${commentSource}`); + + // 3. Content sanitization + commentToPost = sanitizeContent(commentToPost); + + // 4. Target repository / entity number resolution + const targetResult = callbacks.resolveTarget(item, config); + if (!targetResult.success) { + core.warning(`Skipping ${entityConfig.itemType}: ${targetResult.error}`); + return { success: false, error: targetResult.error }; + } + const { entityNumber, owner, repo: repoName, entityRepo } = targetResult; + if (entityRepo) { + core.info(`Target repository: ${entityRepo}`); + } + + try { + // 5. Entity details fetch + core.info(`Fetching ${entityConfig.displayName} details for #${entityNumber} in ${owner}/${repoName}`); + const entity = await callbacks.getDetails(githubClient, owner, repoName, entityNumber); + core.info(`${entityConfig.displayNameCapitalized} #${entityNumber} fetched: state=${entity.state}, title="${entity.title}", labels=[${entity.labels.map(l => l.name || l).join(", ")}]`); + + const wasAlreadyClosed = entity.state === "closed"; + if (wasAlreadyClosed) { + core.info(`${entityConfig.displayNameCapitalized} #${entityNumber} is already closed, but will still add comment`); + } + + // 6. Label filter validation + const labelResult = callbacks.validateLabels(entity, entityNumber, requiredLabels); + if (!labelResult.valid) { + core.warning(labelResult.warning || `Skipping ${entityConfig.displayName} #${entityNumber}: ${labelResult.error}`); + return { success: false, error: labelResult.error }; + } + if (requiredLabels.length > 0) { + core.info(`${entityConfig.displayNameCapitalized} #${entityNumber} has required labels: ${requiredLabels.join(", ")}`); + } + + // 7. Title-prefix filter validation + if (requiredTitlePrefix && !checkTitlePrefixFilter(entity.title, requiredTitlePrefix)) { + core.warning(`${entityConfig.displayNameCapitalized} #${entityNumber} title doesn't start with "${requiredTitlePrefix}"`); + return { success: false, error: `Title doesn't start with "${requiredTitlePrefix}"` }; + } + if (requiredTitlePrefix) { + core.info(`${entityConfig.displayNameCapitalized} #${entityNumber} has required title prefix: "${requiredTitlePrefix}"`); + } + + // 8. Staged-mode preview short-circuit + if (isStaged) { + const repoStr = entityRepo || `${owner}/${repoName}`; + logStagedPreviewInfo(`Would close ${entityConfig.displayName} #${entityNumber} in ${repoStr}`); + return { + success: true, + staged: true, + previewInfo: { + number: entityNumber, + repo: repoStr, + alreadyClosed: wasAlreadyClosed, + hasComment: !!commentToPost, + }, + }; + } + + // 9. Comment posting + const commentBody = callbacks.buildCommentBody(commentToPost, item); + core.info(`Adding comment to ${entityConfig.displayName} #${entityNumber}: length=${commentBody.length}`); + + /** @type {{id: number, html_url: string}|null} */ + let commentResult = null; + let commentPosted = false; + try { + commentResult = await callbacks.addComment(githubClient, owner, repoName, entityNumber, commentBody); + commentPosted = true; + core.info(`✓ Comment posted to ${entityConfig.displayName} #${entityNumber}: ${commentResult.html_url}`); + core.info(`Comment details: id=${commentResult.id}, body_length=${commentBody.length}`); + } catch (commentError) { + const errorMsg = getErrorMessage(commentError); + if (callbacks.continueOnCommentError) { + core.error(`Failed to add comment to ${entityConfig.displayName} #${entityNumber}: ${errorMsg}`); + core.error( + `Error details: ${JSON.stringify({ + entityNumber, + hasBody: !!item.body, + bodyLength: item.body ? item.body.length : 0, + errorMessage: errorMsg, + })}` + ); + // commentPosted stays false; close operation continues + } else { + throw commentError; + } + } + + // 10. Entity close (skipped when already closed) + let closedEntity; + if (wasAlreadyClosed) { + core.info(`${entityConfig.displayNameCapitalized} #${entityNumber} was already closed, comment ${commentPosted ? "added successfully" : "posting attempted"}`); + closedEntity = entity; + } else { + closedEntity = await callbacks.closeEntity(githubClient, owner, repoName, entityNumber, item, config); + core.info(`✓ ${entityConfig.displayNameCapitalized} #${entityNumber} closed successfully: ${closedEntity.html_url}`); + } + + core.info(`${entityConfig.itemType} completed successfully for ${entityConfig.displayName} #${entityNumber}`); + + // 11. Success result construction + return callbacks.buildSuccessResult(closedEntity, commentResult, wasAlreadyClosed, commentPosted); + } catch (error) { + const errorMessage = getErrorMessage(error); + core.error(`Failed to close ${entityConfig.displayName} #${entityNumber}: ${errorMessage}`); + core.error( + `Error details: ${JSON.stringify({ + entityNumber, + hasBody: !!item.body, + bodyLength: item.body ? item.body.length : 0, + errorMessage, + })}` + ); + return { success: false, error: errorMessage }; + } + }; +} + /** * Process close entity items from agent output * @param {EntityConfig} config - Entity configuration @@ -389,6 +611,7 @@ module.exports = { resolveEntityNumber, buildCommentBody, escapeMarkdownTitle, + createCloseEntityHandler, ISSUE_CONFIG, PULL_REQUEST_CONFIG, }; diff --git a/actions/setup/js/close_issue.cjs b/actions/setup/js/close_issue.cjs index 4c765d057c2..e8379ee3c1d 100644 --- a/actions/setup/js/close_issue.cjs +++ b/actions/setup/js/close_issue.cjs @@ -5,13 +5,10 @@ * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction */ -const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); -const { sanitizeContent } = require("./sanitize_content.cjs"); -const { logStagedPreviewInfo } = require("./staged_preview.cjs"); -const { isStagedMode } = require("./safe_output_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); +const { createCloseEntityHandler, ISSUE_CONFIG } = require("./close_entity_helpers.cjs"); /** * Get issue details using REST API @@ -82,19 +79,13 @@ async function closeIssue(github, owner, repo, issueNumber, stateReason) { * @type {HandlerFactoryFunction} */ async function main(config = {}) { - // Extract configuration + const configStateReason = config.state_reason || "COMPLETED"; const requiredLabels = config.required_labels || []; const requiredTitlePrefix = config.required_title_prefix || ""; - const maxCount = config.max || 10; - const comment = config.comment || ""; - const configStateReason = config.state_reason || "COMPLETED"; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); const githubClient = await createAuthenticatedGitHubClient(config); - // Check if we're in staged mode - const isStaged = isStagedMode(config); - - core.info(`Close issue configuration: max=${maxCount}, state_reason=${configStateReason}`); + core.info(`Close issue configuration: max=${config.max || 10}, state_reason=${configStateReason}`); if (requiredLabels.length > 0) { core.info(`Required labels: ${requiredLabels.join(", ")}`); } @@ -106,199 +97,81 @@ async function main(config = {}) { core.info(`Allowed repos: ${Array.from(allowedRepos).join(", ")}`); } - // Track how many items we've processed for max limit - let processedCount = 0; - - /** - * Message handler function that processes a single close_issue message - * @param {Object} message - The close_issue message to process - * @param {Object} resolvedTemporaryIds - Map of temporary IDs to {repo, number} - * @returns {Promise} Result with success/error status - */ - return async function handleCloseIssue(message, resolvedTemporaryIds) { - // Check if we've hit the max limit - if (processedCount >= maxCount) { - core.warning(`Skipping close_issue: max count of ${maxCount} reached`); - return { - success: false, - error: `Max count of ${maxCount} reached`, - }; - } - - processedCount++; - - const item = message; - - // Log message structure for debugging (avoid logging body content) - core.info( - `Processing close_issue message: ${JSON.stringify({ - has_body: !!item.body, - body_length: item.body ? item.body.length : 0, - issue_number: item.issue_number, - has_repo: !!item.repo, - })}` - ); - - // Determine comment body - prefer non-empty item.body over non-empty config.comment - /** @type {string} */ - let commentToPost; - /** @type {string} */ - let commentSource = "unknown"; - - if (typeof item.body === "string" && item.body.trim() !== "") { - commentToPost = item.body; - commentSource = "item.body"; - } else if (typeof comment === "string" && comment.trim() !== "") { - commentToPost = comment; - commentSource = "config.comment"; - } else { - core.warning("No comment body provided in message and no default comment configured"); - return { - success: false, - error: "No comment body provided", - }; - } - - core.info(`Comment body determined: length=${commentToPost.length}, source=${commentSource}`); - - // Sanitize content to prevent injection attacks - commentToPost = sanitizeContent(commentToPost); - - // Resolve and validate target repository - const repoResult = resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, "issue"); - if (!repoResult.success) { - core.warning(`Skipping close_issue: ${repoResult.error}`); - return { - success: false, - error: repoResult.error, - }; - } - const { repo: itemRepo, repoParts } = repoResult; - core.info(`Target repository: ${itemRepo}`); + return createCloseEntityHandler( + config, + ISSUE_CONFIG, + { + resolveTarget(item) { + // Resolve and validate target repository + const repoResult = resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, "issue"); + if (!repoResult.success) { + return { success: false, error: repoResult.error }; + } + const { repo: entityRepo, repoParts } = repoResult; + + // Determine issue number + let issueNumber; + if (item.issue_number !== undefined) { + issueNumber = parseInt(String(item.issue_number), 10); + if (isNaN(issueNumber)) { + return { success: false, error: `Invalid issue number: ${item.issue_number}` }; + } + } else { + const contextIssue = context.payload?.issue?.number; + if (!contextIssue) { + return { success: false, error: "No issue number available" }; + } + issueNumber = contextIssue; + } - // Determine issue number - let issueNumber; - if (item.issue_number !== undefined) { - issueNumber = parseInt(String(item.issue_number), 10); - if (isNaN(issueNumber)) { - core.warning(`Invalid issue number: ${item.issue_number}`); - return { - success: false, - error: `Invalid issue number: ${item.issue_number}`, - }; - } - } else { - // Use context issue if available - const contextIssue = context.payload?.issue?.number; - if (!contextIssue) { - core.warning("No issue_number provided and not in issue context"); - return { - success: false, - error: "No issue number available", - }; - } - issueNumber = contextIssue; - } + return { success: true, entityNumber: issueNumber, owner: repoParts.owner, repo: repoParts.repo, entityRepo }; + }, + + getDetails: getIssueDetails, + + validateLabels(entity, entityNumber, requiredLabels) { + if (requiredLabels.length > 0) { + const issueLabels = entity.labels.map(/** @param {any} l */ l => (typeof l === "string" ? l : l.name || "")); + const missingLabels = requiredLabels.filter(required => !issueLabels.includes(required)); + if (missingLabels.length > 0) { + return { + valid: false, + warning: `Issue #${entityNumber} missing required labels: ${missingLabels.join(", ")}`, + error: `Missing required labels: ${missingLabels.join(", ")}`, + }; + } + } + return { valid: true }; + }, - try { - // Fetch issue details - core.info(`Fetching issue details for #${issueNumber} in ${repoParts.owner}/${repoParts.repo}`); - const issue = await getIssueDetails(githubClient, repoParts.owner, repoParts.repo, issueNumber); - core.info(`Issue #${issueNumber} fetched: state=${issue.state}, title="${issue.title}", labels=[${issue.labels.map(l => l.name || l).join(", ")}]`); + buildCommentBody(sanitizedBody) { + // Issues post the sanitized body directly without a workflow footer + return sanitizedBody; + }, - // Check if already closed - but still add comment - const wasAlreadyClosed = issue.state === "closed"; - if (wasAlreadyClosed) { - core.info(`Issue #${issueNumber} is already closed, but will still add comment`); - } + addComment: addIssueComment, - // Validate required labels if configured - if (requiredLabels.length > 0) { - const issueLabels = issue.labels.map(l => (typeof l === "string" ? l : l.name || "")); - const missingLabels = requiredLabels.filter(required => !issueLabels.includes(required)); - if (missingLabels.length > 0) { - core.warning(`Issue #${issueNumber} missing required labels: ${missingLabels.join(", ")}`); - return { - success: false, - error: `Missing required labels: ${missingLabels.join(", ")}`, - }; - } - core.info(`Issue #${issueNumber} has all required labels: ${requiredLabels.join(", ")}`); - } + closeEntity(github, owner, repo, entityNumber, item) { + // Support item-level state_reason override, falling back to config-level default + const stateReason = item.state_reason || configStateReason; + core.info(`Closing issue #${entityNumber} with state_reason=${stateReason}`); + return closeIssue(github, owner, repo, entityNumber, stateReason); + }, - // Validate required title prefix if configured - if (requiredTitlePrefix && !issue.title.startsWith(requiredTitlePrefix)) { - core.warning(`Issue #${issueNumber} title doesn't start with "${requiredTitlePrefix}"`); - return { - success: false, - error: `Title doesn't start with "${requiredTitlePrefix}"`, - }; - } - if (requiredTitlePrefix) { - core.info(`Issue #${issueNumber} has required title prefix: "${requiredTitlePrefix}"`); - } + continueOnCommentError: false, - // If in staged mode, preview the close without executing it - if (isStaged) { - logStagedPreviewInfo(`Would close issue #${issueNumber} in ${itemRepo}`); + buildSuccessResult(closedEntity, commentResult, wasAlreadyClosed) { return { success: true, - staged: true, - previewInfo: { - number: issueNumber, - repo: itemRepo, - alreadyClosed: wasAlreadyClosed, - hasComment: !!commentToPost, - }, + number: closedEntity.number, + url: closedEntity.html_url, + title: closedEntity.title, + alreadyClosed: wasAlreadyClosed, }; - } - - // Add comment with the body from the message - core.info(`Adding comment to issue #${issueNumber}: length=${commentToPost.length}`); - const commentResult = await addIssueComment(githubClient, repoParts.owner, repoParts.repo, issueNumber, commentToPost); - core.info(`✓ Comment posted to issue #${issueNumber}: ${commentResult.html_url}`); - core.info(`Comment details: id=${commentResult.id}, body_length=${commentToPost.length}`); - - // Close the issue if not already closed - let closedIssue; - if (wasAlreadyClosed) { - core.info(`Issue #${issueNumber} was already closed, comment added successfully`); - closedIssue = issue; - } else { - // Use item-level state_reason if provided, otherwise fall back to config-level default - const stateReason = item.state_reason || configStateReason; - core.info(`Closing issue #${issueNumber} in ${itemRepo} with state_reason=${stateReason}`); - closedIssue = await closeIssue(githubClient, repoParts.owner, repoParts.repo, issueNumber, stateReason); - core.info(`✓ Issue #${issueNumber} closed successfully: ${closedIssue.html_url}`); - } - - core.info(`close_issue completed successfully for issue #${issueNumber}`); - - return { - success: true, - number: issueNumber, - url: closedIssue.html_url, - title: closedIssue.title, - alreadyClosed: wasAlreadyClosed, - }; - } catch (error) { - const errorMessage = getErrorMessage(error); - core.error(`Failed to close issue #${issueNumber}: ${errorMessage}`); - core.error( - `Error details: ${JSON.stringify({ - issueNumber, - repo: itemRepo, - hasBody: !!item.body, - bodyLength: item.body ? item.body.length : 0, - errorMessage, - })}` - ); - return { - success: false, - error: errorMessage, - }; - } - }; + }, + }, + githubClient + ); } module.exports = { main }; diff --git a/actions/setup/js/close_pull_request.cjs b/actions/setup/js/close_pull_request.cjs index 2483993cfb6..6429e5a79ae 100644 --- a/actions/setup/js/close_pull_request.cjs +++ b/actions/setup/js/close_pull_request.cjs @@ -1,22 +1,14 @@ // @ts-check /// -const { getErrorMessage } = require("./error_helpers.cjs"); -const { getTrackerID } = require("./get_tracker_id.cjs"); -const { generateFooterWithMessages } = require("./messages_footer.cjs"); -const { sanitizeContent } = require("./sanitize_content.cjs"); -const { logStagedPreviewInfo } = require("./staged_preview.cjs"); -const { isStagedMode } = require("./safe_output_helpers.cjs"); const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs"); const { ERR_NOT_FOUND } = require("./error_codes.cjs"); -const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs"); +const { createCloseEntityHandler, checkLabelFilter, buildCommentBody, PULL_REQUEST_CONFIG } = require("./close_entity_helpers.cjs"); /** * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction */ -const HANDLER_TYPE = "close_pull_request"; - /** * Get pull request details using REST API * @param {any} github - GitHub REST API instance @@ -83,17 +75,11 @@ async function closePullRequest(github, owner, repo, prNumber) { * @type {HandlerFactoryFunction} */ async function main(config = {}) { - // Extract configuration const requiredLabels = config.required_labels || []; const requiredTitlePrefix = config.required_title_prefix || ""; - const maxCount = config.max || 10; - const comment = config.comment || ""; const githubClient = await createAuthenticatedGitHubClient(config); - // Check if we're in staged mode (either globally or per-handler config) - const isStaged = isStagedMode(config); - - core.info(`Close pull request configuration: max=${maxCount}`); + core.info(`Close pull request configuration: max=${config.max || 10}`); if (requiredLabels.length > 0) { core.info(`Required labels: ${requiredLabels.join(", ")}`); } @@ -101,252 +87,67 @@ async function main(config = {}) { core.info(`Required title prefix: ${requiredTitlePrefix}`); } - // Track how many items we've processed for max limit - let processedCount = 0; - - /** - * Message handler function that processes a single close_pull_request message - * @param {Object} message - The close_pull_request message to process - * @param {Object} resolvedTemporaryIds - Map of temporary IDs to {repo, number} - * @returns {Promise} Result with success/error status - */ - return async function handleClosePullRequest(message, resolvedTemporaryIds) { - // Check if we've hit the max limit - if (processedCount >= maxCount) { - core.warning(`Skipping close_pull_request: max count of ${maxCount} reached`); - return { - success: false, - error: `Max count of ${maxCount} reached`, - }; - } - - processedCount++; - - const item = message; - - // Log message structure for debugging (avoid logging body content) - core.info( - `Processing close_pull_request message: ${JSON.stringify({ - has_body: !!item.body, - body_length: item.body ? item.body.length : 0, - pull_request_number: item.pull_request_number, - })}` - ); - - // Determine comment body - prefer non-empty item.body over non-empty config.comment - /** @type {string} */ - let commentToPost; - /** @type {string} */ - let commentSource = "unknown"; - - if (typeof item.body === "string" && item.body.trim() !== "") { - commentToPost = item.body; - commentSource = "item.body"; - } else if (typeof comment === "string" && comment.trim() !== "") { - commentToPost = comment; - commentSource = "config.comment"; - } else { - core.warning("No comment body provided in message and no default comment configured"); - return { - success: false, - error: "No comment body provided", - }; - } - - core.info(`Comment body determined: length=${commentToPost.length}, source=${commentSource}`); + return createCloseEntityHandler( + config, + PULL_REQUEST_CONFIG, + { + resolveTarget(item) { + let prNumber; + if (item.pull_request_number !== undefined) { + prNumber = parseInt(String(item.pull_request_number), 10); + if (isNaN(prNumber)) { + return { success: false, error: `Invalid pull request number: ${item.pull_request_number}` }; + } + } else { + const contextPR = context.payload?.pull_request?.number; + if (!contextPR) { + return { success: false, error: "No pull_request_number provided and not in pull request context" }; + } + prNumber = contextPR; + } + return { success: true, entityNumber: prNumber, owner: context.repo.owner, repo: context.repo.repo }; + }, + + getDetails: getPullRequestDetails, + + validateLabels(entity, entityNumber, requiredLabels) { + if (!checkLabelFilter(entity.labels, requiredLabels)) { + return { + valid: false, + warning: `Skipping PR #${entityNumber}: does not match label filter (required: ${requiredLabels.join(", ")})`, + error: "PR does not match required labels", + }; + } + return { valid: true }; + }, + + buildCommentBody(sanitizedBody) { + const triggeringPRNumber = context.payload?.pull_request?.number; + const triggeringIssueNumber = context.payload?.issue?.number; + return buildCommentBody(sanitizedBody, triggeringIssueNumber, triggeringPRNumber); + }, + + addComment: addPullRequestComment, + + closeEntity(github, owner, repo, prNumber) { + core.info(`Closing PR #${prNumber}`); + return closePullRequest(github, owner, repo, prNumber); + }, - // Sanitize content to prevent injection attacks - commentToPost = sanitizeContent(commentToPost); + continueOnCommentError: true, - // Determine PR number - let prNumber; - if (item.pull_request_number !== undefined) { - prNumber = parseInt(String(item.pull_request_number), 10); - if (isNaN(prNumber)) { - core.warning(`Invalid pull request number: ${item.pull_request_number}`); + buildSuccessResult(closedEntity, commentResult, wasAlreadyClosed, commentPosted) { return { - success: false, - error: `Invalid pull request number: ${item.pull_request_number}`, - }; - } - } else { - // Use context PR if available - const contextPR = context.payload?.pull_request?.number; - if (!contextPR) { - core.warning("No pull_request_number provided and not in pull request context"); - return { - success: false, - error: "No pull_request_number provided and not in pull request context", - }; - } - prNumber = contextPR; - } - - core.info(`Processing close_pull_request for PR #${prNumber}`); - - // Get PR details - const { owner, repo } = context.repo; - let pr; - try { - core.info(`Fetching PR details for #${prNumber} in ${owner}/${repo}`); - pr = await getPullRequestDetails(githubClient, owner, repo, prNumber); - core.info(`PR #${prNumber} fetched: state=${pr.state}, title="${pr.title}", labels=[${pr.labels.map(l => l.name || l).join(", ")}]`); - } catch (error) { - const errorMsg = getErrorMessage(error); - core.warning(`Failed to get PR #${prNumber} details: ${errorMsg}`); - return { - success: false, - error: `Failed to get PR #${prNumber} details: ${errorMsg}`, - }; - } - - // Check if already closed - but still add comment - const wasAlreadyClosed = pr.state === "closed"; - if (wasAlreadyClosed) { - core.info(`PR #${prNumber} is already closed, but will still add comment`); - } - - // Check label filter - if (!checkLabelFilter(pr.labels, requiredLabels)) { - core.info(`Skipping PR #${prNumber}: does not match label filter (required: ${requiredLabels.join(", ")})`); - return { - success: false, - error: `PR does not match required labels`, - }; - } - if (requiredLabels.length > 0) { - core.info(`PR #${prNumber} has all required labels: ${requiredLabels.join(", ")}`); - } - - // Check title prefix filter - if (!checkTitlePrefixFilter(pr.title, requiredTitlePrefix)) { - core.info(`Skipping PR #${prNumber}: title does not start with '${requiredTitlePrefix}'`); - return { - success: false, - error: `PR title does not start with required prefix`, - }; - } - if (requiredTitlePrefix) { - core.info(`PR #${prNumber} has required title prefix: "${requiredTitlePrefix}"`); - } - - // If in staged mode, preview the close without executing it - if (isStaged) { - logStagedPreviewInfo(`Would close PR #${prNumber}`); - return { - success: true, - staged: true, - previewInfo: { - number: prNumber, + success: true, + pull_request_number: closedEntity.number, + pull_request_url: closedEntity.html_url, alreadyClosed: wasAlreadyClosed, - hasComment: !!commentToPost, - }, - }; - } - - // Add comment with the body from the message - let commentPosted = false; - try { - const triggeringPRNumber = context.payload?.pull_request?.number; - const triggeringIssueNumber = context.payload?.issue?.number; - const commentBody = buildCommentBody(commentToPost, triggeringIssueNumber, triggeringPRNumber); - core.info(`Adding comment to PR #${prNumber}: length=${commentBody.length}`); - await addPullRequestComment(githubClient, owner, repo, prNumber, commentBody); - commentPosted = true; - core.info(`✓ Comment posted to PR #${prNumber}`); - core.info(`Comment details: body_length=${commentBody.length}`); - } catch (error) { - const errorMsg = getErrorMessage(error); - core.error(`Failed to add comment to PR #${prNumber}: ${errorMsg}`); - core.error( - `Error details: ${JSON.stringify({ - prNumber, - hasBody: !!item.body, - bodyLength: item.body ? item.body.length : 0, - errorMessage: errorMsg, - })}` - ); - // Continue with closing even if comment fails - but log the error - } - - // Close the PR if not already closed - let closedPR; - if (wasAlreadyClosed) { - core.info(`PR #${prNumber} was already closed, comment ${commentPosted ? "posted successfully" : "posting attempted"}`); - closedPR = pr; - } else { - try { - core.info(`Closing PR #${prNumber}`); - closedPR = await closePullRequest(githubClient, owner, repo, prNumber); - core.info(`✓ PR #${prNumber} closed successfully: ${closedPR.title}`); - } catch (error) { - const errorMsg = getErrorMessage(error); - core.error(`Failed to close PR #${prNumber}: ${errorMsg}`); - core.error( - `Error details: ${JSON.stringify({ - prNumber, - hasBody: !!item.body, - bodyLength: item.body ? item.body.length : 0, - errorMessage: errorMsg, - })}` - ); - return { - success: false, - error: `Failed to close PR #${prNumber}: ${errorMsg}`, + commentPosted, }; - } - } - - core.info(`close_pull_request completed for PR #${prNumber}: ${commentPosted ? "comment posted and " : ""}PR ${wasAlreadyClosed ? "was already closed" : "closed successfully"}`); - - return { - success: true, - pull_request_number: closedPR.number, - pull_request_url: closedPR.html_url, - alreadyClosed: wasAlreadyClosed, - commentPosted, - }; - }; -} - -/** - * Check if labels match the required labels filter - * @param {Array<{name: string}>} prLabels - Labels on the PR - * @param {string[]} requiredLabels - Required labels (any match) - * @returns {boolean} True if PR has at least one required label or no filter is set - */ -function checkLabelFilter(prLabels, requiredLabels) { - if (requiredLabels.length === 0) return true; - - const labelNames = prLabels.map(l => l.name); - return requiredLabels.some(required => labelNames.includes(required)); -} - -/** - * Check if title matches the required prefix filter - * @param {string} title - PR title - * @param {string} requiredTitlePrefix - Required title prefix - * @returns {boolean} True if title starts with required prefix or no filter is set - */ -function checkTitlePrefixFilter(title, requiredTitlePrefix) { - if (!requiredTitlePrefix) return true; - return title.startsWith(requiredTitlePrefix); -} - -/** - * Build comment body with tracker ID and footer - * @param {string} body - The original comment body - * @param {number|undefined} triggeringIssueNumber - Issue number that triggered this workflow - * @param {number|undefined} triggeringPRNumber - PR number that triggered this workflow - * @returns {string} The complete comment body with tracker ID and footer - */ -function buildCommentBody(body, triggeringIssueNumber, triggeringPRNumber) { - const workflowName = process.env.GH_AW_WORKFLOW_NAME || "Workflow"; - const workflowSource = process.env.GH_AW_WORKFLOW_SOURCE || ""; - const workflowSourceURL = process.env.GH_AW_WORKFLOW_SOURCE_URL || ""; - const runUrl = buildWorkflowRunUrl(context, context.repo); - - return body.trim() + getTrackerID("markdown") + generateFooterWithMessages(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, undefined); + }, + }, + githubClient + ); } module.exports = { main }; diff --git a/actions/setup/js/close_pull_request.test.cjs b/actions/setup/js/close_pull_request.test.cjs index 75199622f60..895a28cee78 100644 --- a/actions/setup/js/close_pull_request.test.cjs +++ b/actions/setup/js/close_pull_request.test.cjs @@ -407,7 +407,7 @@ describe("close_pull_request", () => { const result = await handler({ pull_request_number: 100, body: "Close" }, {}); expect(result.success).toBe(false); - expect(result.error).toContain("does not start with required prefix"); + expect(result.error).toContain("doesn't start with"); }); it("should add comment before closing when configured", async () => { From 184fcec3f93da42af8bc9129a2e0e4a2007ad719 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 14:42:08 +0000 Subject: [PATCH 3/3] fix: resolve TypeScript type error and double-sanitization in createCloseEntityHandler - Change @returns JSDoc from {Function} to {MessageHandlerFunction} so tsc accepts the return value of main() in close_issue.cjs and close_pull_request.cjs - Remove redundant sanitizeContent() call from buildCommentBody(); callers are now responsible for sanitizing before calling it - Update processCloseEntityItems to sanitize item.body before passing to buildCommentBody(), keeping behaviour unchanged for that code path Agent-Logs-Url: https://github.com/github/gh-aw/sessions/898f68c4-bd25-4d5d-9375-68a7258195ff Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/close_entity_helpers.cjs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/actions/setup/js/close_entity_helpers.cjs b/actions/setup/js/close_entity_helpers.cjs index b7dea509f9b..439c11dfe0f 100644 --- a/actions/setup/js/close_entity_helpers.cjs +++ b/actions/setup/js/close_entity_helpers.cjs @@ -51,10 +51,8 @@ function buildCommentBody(body, triggeringIssueNumber, triggeringPRNumber) { const workflowSourceURL = process.env.GH_AW_WORKFLOW_SOURCE_URL || ""; const runUrl = buildWorkflowRunUrl(context, context.repo); - // Sanitize the body content to prevent injection attacks - const sanitizedBody = sanitizeContent(body); - - return sanitizedBody.trim() + getTrackerID("markdown") + generateFooterWithMessages(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, undefined); + // Caller is responsible for sanitizing body before passing it here. + return body.trim() + getTrackerID("markdown") + generateFooterWithMessages(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, undefined); } /** @@ -256,7 +254,7 @@ function escapeMarkdownTitle(title) { * @param {EntityConfig} entityConfig - Entity display/type configuration * @param {CloseEntityHandlerCallbacks} callbacks - Entity-specific callbacks * @param {any} githubClient - Authenticated GitHub client - * @returns {Function} Async handler function suitable for use as a message handler + * @returns {import('./types/handler-factory').MessageHandlerFunction} Message handler function */ function createCloseEntityHandler(config, entityConfig, callbacks, githubClient) { const requiredLabels = config.required_labels || []; @@ -514,8 +512,9 @@ async function processCloseEntityItems(config, callbacks, handlerConfig = {}) { core.info(`${config.displayNameCapitalized} #${entityNumber} is already closed, but will still add comment`); } - // Build comment body - const commentBody = buildCommentBody(item.body, triggeringIssueNumber, triggeringPRNumber); + // Build comment body (sanitize first, then append tracker/footer) + const sanitizedItemBody = sanitizeContent(item.body); + const commentBody = buildCommentBody(sanitizedItemBody, triggeringIssueNumber, triggeringPRNumber); // Add comment before closing (or to already-closed entity) const comment = await callbacks.addComment(github, context.repo.owner, context.repo.repo, entityNumber, commentBody);