Skip to content

refactor(create_pull_request): extract helpers module and add tests #33687

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 11 commits into from
May 21, 2026
Merged

refactor(create_pull_request): extract helpers module and add tests #33687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
15256f4
refactor(create_pull_request): extract helpers and add tests
Copilot May 21, 2026
a819a19
address code review feedback
Copilot May 21, 2026
331f9fb
test(helpers): fix flaky crypto test and URL-encoding test coverage
Copilot May 21, 2026
a5eba14
test(helpers): use afterEach for mock cleanup in createBundleTempRef …
Copilot May 21, 2026
23e3f24
Merge remote-tracking branch 'origin/main' into copilot/refactor-crea…
Copilot May 21, 2026
55dad84
chore: merge main and recompile workflows
Copilot May 21, 2026
d36acd5
Merge remote-tracking branch 'origin/main' into copilot/refactor-crea…
Copilot May 21, 2026
4ecbf3b
chore: merge main and recompile workflows
Copilot May 21, 2026
3508ecb
Merge branch 'main' into copilot/refactor-create-pull-request
pelikhan May 21, 2026
490cd0d
Merge branch 'main' into copilot/refactor-create-pull-request
pelikhan May 21, 2026
d07f3e1
style: format helpers files with prettier
Copilot May 21, 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
272 changes: 29 additions & 243 deletions actions/setup/js/create_pull_request.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,32 @@ const { getBaseBranch } = require("./get_base_branch.cjs");
const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
const { buildWorkflowRunUrl } = require("./workflow_metadata_helpers.cjs");
const { checkFileProtection } = require("./manifest_file_helpers.cjs");
const { renderTemplateFromFile, buildProtectedFileList, encodePathSegments, getPromptPath } = require("./messages_core.cjs");
const { COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL, MAX_ASSIGNEES } = require("./constants.cjs");
const { renderTemplateFromFile, buildProtectedFileList, getPromptPath } = require("./messages_core.cjs");
const { COPILOT_REVIEWER_BOT, FAQ_CREATE_PR_PERMISSIONS_URL } = require("./constants.cjs");
const { isStagedMode } = require("./safe_output_helpers.cjs");
const { withRetry, isTransientError, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
const { tryEnforceArrayLimit } = require("./limit_enforcement_helpers.cjs");
const { withRetry, RATE_LIMIT_RETRY_CONFIG } = require("./error_recovery.cjs");
const { findAgent, getIssueDetails, assignAgentToIssue } = require("./assign_agent_helpers.cjs");
const { globPatternToRegex } = require("./glob_pattern_helpers.cjs");
const { ensureFullHistoryForBundle, extractBundlePrerequisiteCommits } = require("./git_helpers.cjs");
const { parseDiffGitHeader: parseDiffGitHeaderPaths, extractDiffGitHeaderEntries } = require("./patch_path_helpers.cjs");
const { resolveAllowedMentionsFromPayload } = require("./resolve_mentions_from_payload.cjs");
const {
MANAGED_FALLBACK_ISSUE_LABEL,
LABEL_MAX_RETRIES,
LABEL_INITIAL_DELAY_MS,
LABEL_MAX_DELAY_MS,
summarizeListForLog,
createBundleTempRef,
isLabelTransientError,
parseAllowedBaseBranches,
isBaseBranchAllowed,
parseStringListConfig,
mergeFallbackIssueLabels,
sanitizeFallbackAssignees,
neutralizeClosingKeywordsForIssueBody,
generatePatchPreview,
buildManifestProtectionCreatePrUrl,
renderManifestProtectionFallbackBody,
} = require("./create_pull_request_helpers.cjs");

/**
* @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction
Expand Down Expand Up @@ -68,35 +84,8 @@ async function createCopilotAssignmentClient(config) {
/** @type {string} Safe output type handled by this module */
const HANDLER_TYPE = "create_pull_request";

@github-actions github-actions Bot May 21, 2026

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.

[/zoom-out] Comment clarifies where, not why.

// NOTE: MANAGED_FALLBACK_ISSUE_LABEL, createBundleTempRef, and summarizeListForLog
// are imported from create_pull_request_helpers.cjs above.

This comment states the obvious (imports are at the top). Better:

// Extracted helpers: Pure functions with no dependency on main() closure.
// See create_pull_request_helpers.cjs for implementation details.

Or remove entirely — the import statement is self-documenting.

/** @type {string} Label always added to fallback issues so the triage system can find them */
const MANAGED_FALLBACK_ISSUE_LABEL = "agentic-workflows";

/**
* Creates a temporary refs/bundles ref for applying create_pull_request bundles.
* Branch names are sanitized for ref compatibility, and a short crypto-random
* suffix avoids collisions between branches that sanitize to the same value.
*
* @param {string} branchName - Target branch name
* @returns {string} Temporary bundle ref name
*/
function createBundleTempRef(branchName) {
const suffix = crypto.randomBytes(4).toString("hex");
return `refs/bundles/create-pr-${branchName.replace(/[^a-zA-Z0-9-]/g, "-")}-${suffix}`;
}

/**
* Summarize a list for log output to avoid excessively long lines.
* @param {string[]} values
* @param {number} limit
* @returns {string}
*/
function summarizeListForLog(values, limit = 10) {
if (!Array.isArray(values) || values.length === 0) {
return "(none)";
}
const preview = values.slice(0, limit).join(", ");
return values.length > limit ? `${preview} ... and ${values.length - limit} more` : preview;
}
// NOTE: MANAGED_FALLBACK_ISSUE_LABEL, createBundleTempRef, and summarizeListForLog
// are imported from create_pull_request_helpers.cjs above.

/**
* Attempt automatic recovery for git am add/add conflicts by preferring the patch version.
Expand Down Expand Up @@ -319,131 +308,11 @@ async function rewriteBundleBranchAsSingleCommit(baseBranch, execApi) {
}
}

/**
* Determines if a label API error is transient and worth retrying.
* Returns true for:
* - The GitHub race condition where a newly-created PR's node ID is not immediately
* resolvable via the REST/GraphQL bridge (unprocessable validation error).
* - Any standard transient error matched by {@link isTransientError} (network issues,
* rate limits, 5xx gateway errors, etc.).
* @param {any} error - The error to check
* @returns {boolean} True if the error is transient and should be retried
*/
function isLabelTransientError(error) {
const msg = getErrorMessage(error);
if (msg.includes("Could not resolve to a node with the global id")) {
return true;
}
return isTransientError(error);
}

/** @type {number} Number of retry attempts for label operations */
const LABEL_MAX_RETRIES = 5;
/** @type {number} Base delay in ms used to calculate label retry backoff (3 seconds) */
const LABEL_INITIAL_DELAY_MS = 3000;
/** @type {number} Maximum delay in ms between label retries (30 seconds) */
const LABEL_MAX_DELAY_MS = 30000;

/**
* Parse allowed base branch patterns from config value (array or comma-separated string)
* @param {string[]|string|undefined} allowedBaseBranchesValue
* @returns {Set<string>}
*/
function parseAllowedBaseBranches(allowedBaseBranchesValue) {
const set = new Set();
if (Array.isArray(allowedBaseBranchesValue)) {
allowedBaseBranchesValue
.map(branch => String(branch).trim())
.filter(Boolean)
.forEach(branch => set.add(branch));
} else if (typeof allowedBaseBranchesValue === "string") {
allowedBaseBranchesValue
.split(",")
.map(branch => branch.trim())
.filter(Boolean)
.forEach(branch => set.add(branch));
}
return set;
}

/**
* Check if a base branch matches an allowed pattern.
* Supports exact matches and "*" glob patterns (e.g. "release/*").
* @param {string} baseBranch
* @param {Set<string>} allowedBaseBranches
* @returns {boolean}
*/
function isBaseBranchAllowed(baseBranch, allowedBaseBranches) {
if (allowedBaseBranches.has(baseBranch)) {
return true;
}
for (const pattern of allowedBaseBranches) {
if (pattern === "*") {
return true;
}
if (pattern.includes("*") && globPatternToRegex(pattern, { pathMode: true, caseSensitive: true }).test(baseBranch)) {
return true;
}
}
return false;
}

/**
* Parse config values that may be arrays or comma-separated strings.
* @param {string[]|string|undefined} value
* @returns {string[]}
*/
function parseStringListConfig(value) {
if (!value) {
return [];
}
const raw = Array.isArray(value) ? value : String(value).split(",");
return raw.map(item => String(item).trim()).filter(Boolean);
}

/**
* Merges the required fallback label with any workflow-configured labels,
* deduplicating and filtering empty values.
* @param {string[]} [labels]
* @returns {string[]}
*/
function mergeFallbackIssueLabels(labels = []) {
const normalizedLabels = labels
.filter(label => !!label)
.map(label => String(label).trim())
.filter(label => label);
return [...new Set([MANAGED_FALLBACK_ISSUE_LABEL, ...normalizedLabels])];
}

/**
* Sanitizes configured assignees for fallback issue creation.
* Filters invalid values, removes the special "copilot" username (not a valid GitHub user
* for issue assignment), and enforces the MAX_ASSIGNEES limit.
* Returns null (no assignees field) if the sanitized list is empty.
* @param {string[]} assignees - Raw assignees from config
* @returns {string[] | null} Sanitized assignees or null if none remain
*/
function sanitizeFallbackAssignees(assignees) {
if (!assignees || assignees.length === 0) {
return null;
}
const sanitized = assignees
.filter(a => typeof a === "string")
.map(a => a.trim())
.filter(a => a.length > 0 && a.toLowerCase() !== "copilot");

if (sanitized.length === 0) {
return null;
}

const limitResult = tryEnforceArrayLimit(sanitized, MAX_ASSIGNEES, "assignees");
if (!limitResult.success) {
core.warning(`Assignees limit exceeded for fallback issue: ${limitResult.error}. Using first ${MAX_ASSIGNEES}.`);
return sanitized.slice(0, MAX_ASSIGNEES);
}

return sanitized;
}
// NOTE: isLabelTransientError, LABEL_MAX_RETRIES, LABEL_INITIAL_DELAY_MS, LABEL_MAX_DELAY_MS,
// parseAllowedBaseBranches, isBaseBranchAllowed, parseStringListConfig, mergeFallbackIssueLabels,
// sanitizeFallbackAssignees, neutralizeClosingKeywordsForIssueBody, generatePatchPreview,
// buildManifestProtectionCreatePrUrl, and renderManifestProtectionFallbackBody
// are imported from create_pull_request_helpers.cjs above.

/**
* Creates a fallback GitHub issue, retrying on rate-limit and other transient errors
Expand Down Expand Up @@ -494,61 +363,6 @@ async function createFallbackIssue(githubClient, repoParts, title, body, labels,
);
}

/**
* Builds a compare URL used in protected-files fallback issue bodies.
* Optionally appends a prefilled PR body that closes the fallback issue.
* @param {string} githubServer
* @param {{owner: string, repo: string}} repoParts
* @param {string} baseBranch
* @param {string} branchName
* @param {string} title
* @param {number} [fallbackIssueNumber]
* @returns {string}
*/
function buildManifestProtectionCreatePrUrl(githubServer, repoParts, baseBranch, branchName, title, fallbackIssueNumber) {
const encodedBase = encodePathSegments(baseBranch);
const encodedHead = encodePathSegments(branchName);
let createPrUrl = `${githubServer}/${repoParts.owner}/${repoParts.repo}/compare/${encodedBase}...${encodedHead}?expand=1&title=${encodeURIComponent(title)}`;
if (typeof fallbackIssueNumber === "number") {
createPrUrl += `&body=${encodeURIComponent(`Closes #${fallbackIssueNumber}`)}`;
}
return createPrUrl;
}

/**
* Renders protected-files fallback issue body with a prefilled compare URL.
* @param {string} mainBodyContent
* @param {string} footerContent
* @param {string} fileList
* @param {string} createPrUrl
* @returns {string}
*/
function renderManifestProtectionFallbackBody(mainBodyContent, footerContent, fileList, createPrUrl) {
const templatePath = getPromptPath("manifest_protection_create_pr_fallback.md");
return renderTemplateFromFile(templatePath, {
main_body: mainBodyContent,
footer: footerContent,
files: fileList,
create_pr_url: createPrUrl,
});
}

/**
* Neutralizes issue-closing keywords in body text to avoid unintended cross-issue closure
* when PR content is reused in fallback issue bodies.
*
* Example: "Closes #123" -> "Closes \\#123"
*
* @param {string} content
* @returns {string}
*/
function neutralizeClosingKeywordsForIssueBody(content) {
if (!content) {
return content;
}
return String(content).replace(/\b(fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)\s+((?:[a-z0-9_.-]+\/[a-z0-9_.-]+)?#\d+)\b/gi, (_match, keyword, issueRef) => `${keyword} ${String(issueRef).replace("#", "\\#")}`);
}

/**
* Maximum limits for pull request parameters to prevent resource exhaustion.
* These limits align with GitHub's API constraints and security best practices.
Expand Down Expand Up @@ -632,35 +446,7 @@ function enforcePullRequestLimits(patchContent, maxFiles = MAX_FILES) {
}
}

/**
* Generate a patch preview with max 500 lines and 2000 chars for issue body
* @param {string} patchContent - The full patch content
* @returns {string} Formatted patch preview
*/
function generatePatchPreview(patchContent) {
if (!patchContent || !patchContent.trim()) {
return "";
}

const lines = patchContent.split("\n");
const maxLines = 500;
const maxChars = 2000;

// Apply line limit first
let preview = lines.length <= maxLines ? patchContent : lines.slice(0, maxLines).join("\n");
const lineTruncated = lines.length > maxLines;

// Apply character limit
const charTruncated = preview.length > maxChars;
if (charTruncated) {
preview = preview.slice(0, maxChars);
}

const truncated = lineTruncated || charTruncated;
const summary = truncated ? `Show patch preview (${Math.min(maxLines, lines.length)} of ${lines.length} lines)` : `Show patch (${lines.length} lines)`;

return `\n\n<details><summary>${summary}</summary>\n\n\`\`\`diff\n${preview}${truncated ? "\n... (truncated)" : ""}\n\`\`\`\n\n</details>`;
}
// NOTE: generatePatchPreview is imported from create_pull_request_helpers.cjs above.

/**
* Check whether the remote branch already exists and, if so, either reuse it
Expand Down
Loading
Loading