diff --git a/README.md b/README.md index 8abe8854..e89eaa73 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,65 @@ The following parameters can be used for additional control over when it is safe - `override_filter_paths`: These are the file paths that, if out of date on a PR, will prevent merge no matter what files the PR is changing - example: `override_filter_paths: package.json,package-lock.json` - `override_filter_globs`: These are glob patterns for `override_filter_paths` +- `match_comment_paths`: When set to `'true'`, enables comment path matching. This reads paths from a specially formatted PR comment and checks if the branch is behind on any of those paths. This is useful when a PR's scope extends beyond the files it directly modifies (e.g., due to transitive dependencies in a monorepo triggering tests in other packages). + +#### Comment Path Matching + +When `match_comment_paths` is enabled, the helper looks for a PR comment containing the marker `` followed by a JSON array of paths in a fenced code block: + +```markdown + +```json +["path/to/package1", "path/to/package2"] +``` +``` + +This is useful for monorepos with selective testing, where changing one package (e.g., a shared data model) triggers tests for dependent packages. Without this feature, merging could introduce bugs if the dependent packages were updated on main after the PR's tests ran. + +Example workflow integration: + +```yaml +# After determining affected paths, post them as a comment +- name: Post affected paths + uses: actions/github-script@v7 + with: + script: | + const marker = ''; + const paths = ${{ steps.get-affected-paths.outputs.paths }}; + const body = `${marker}\n\`\`\`json\n${JSON.stringify(paths)}\n\`\`\``; + + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number + }); + const existing = comments.find(c => c.body.includes(marker)); + + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body + }); + } + +# Then run check-merge-safety with comment path matching enabled +- uses: ExpediaGroup/github-helpers@v1 + with: + helper: check-merge-safety + paths: | + packages/package-1 + packages/package-2 + match_comment_paths: 'true' +``` ### [close-pr](.github/workflows/close-pr.yml) diff --git a/action.yml b/action.yml index 5b68ffd5..3ebf24e4 100644 --- a/action.yml +++ b/action.yml @@ -167,6 +167,9 @@ inputs: description: 'The commit message to use' required: false default: 'Automated PR creation' + match_comment_paths: + description: 'Enable comment path matching for check-merge-safety. When enabled, reads paths from a specially formatted PR comment to determine if the branch needs rebasing.' + required: false outputs: output: description: 'The output of the helper' diff --git a/src/helpers/check-merge-safety.ts b/src/helpers/check-merge-safety.ts index 88935cbc..f71ea108 100644 --- a/src/helpers/check-merge-safety.ts +++ b/src/helpers/check-merge-safety.ts @@ -25,12 +25,15 @@ import * as core from '@actions/core'; const git = simpleGit(); const maxBranchNameLength = 50; +const COMMENT_PATHS_MARKER = ''; + export class CheckMergeSafety extends HelperInputs { declare context?: string; declare paths?: string; declare ignore_globs?: string; declare override_filter_paths?: string; declare override_filter_globs?: string; + declare match_comment_paths?: string; } export const checkMergeSafety = async (inputs: CheckMergeSafety) => { @@ -147,7 +150,7 @@ const getDiff = async (compareBase: DiffRefs, compareHead: DiffRefs, basehead: s const getMergeSafetyStateAndMessage = async ( pullRequest: PullRequest, - { paths, ignore_globs, override_filter_paths, override_filter_globs }: CheckMergeSafety + { paths, ignore_globs, override_filter_paths, override_filter_globs, match_comment_paths }: CheckMergeSafety ) => { const { base: { @@ -175,6 +178,31 @@ const getMergeSafetyStateAndMessage = async ( const truncatedRef = ref.length > maxBranchNameLength ? `${ref.substring(0, maxBranchNameLength)}...` : ref; const truncatedBranchName = `${username}:${truncatedRef}`; + + if (match_comment_paths === 'true') { + const commentPaths = await getPathsFromComment(pullRequest.number); + + if (commentPaths.length) { + core.info(`Found ${commentPaths.length} paths from PR comment`); + + const outdatedCommentPaths = commentPaths.filter(commentPath => + fileNamesWhichBranchIsBehindOn.some(file => file.startsWith(commentPath + '/') || file === commentPath) + ); + + if (outdatedCommentPaths.length) { + core.error(buildErrorMessage(outdatedCommentPaths, 'comment paths', truncatedBranchName)); + const displayPaths = outdatedCommentPaths.slice(0, 3).join(', '); + const suffix = outdatedCommentPaths.length > 3 ? '...' : ''; + return { + state: 'failure', + message: `Branch is behind on paths from comment: ${displayPaths}${suffix}. Please update with ${default_branch}.` + } as const; + } + } else { + core.info('No paths found in PR comment, skipping comment path matching check'); + } + } + const globalFilesOutdatedOnBranch = override_filter_globs ? micromatch(fileNamesWhichBranchIsBehindOn, override_filter_globs.split(/[\n,]/)) : override_filter_paths @@ -223,7 +251,7 @@ const getMergeSafetyStateAndMessage = async ( } as const; }; -const buildErrorMessage = (paths: string[], pathType: 'projects' | 'global files', branchName: string) => +const buildErrorMessage = (paths: string[], pathType: 'projects' | 'global files' | 'comment paths', branchName: string) => ` The following ${pathType} are outdated on branch ${branchName} @@ -234,3 +262,27 @@ const diffErrorMessage = (basehead: string, message = '') => `Failed to generate diff for ${basehead}. Please verify SHAs are valid and try again.${message ? `\nError: ${message}` : ''}`; const buildSuccessMessage = (branchName: string) => `Branch ${branchName} is safe to merge!`; + +const getPathsFromComment = async (pullNumber: number): Promise => { + const { data: comments } = await octokit.issues.listComments({ + ...githubContext.repo, + issue_number: pullNumber + }); + + const pathsComment = comments.find(c => c.body?.includes(COMMENT_PATHS_MARKER)); + if (!pathsComment?.body) { + return []; + } + + const jsonMatch = pathsComment.body.match(/```json\n([\s\S]*?)\n```/); + if (!jsonMatch) { + return []; + } + + try { + return JSON.parse(jsonMatch[1]); + } catch { + core.warning(`Failed to parse paths from PR #${pullNumber} comment`); + return []; + } +}; diff --git a/src/types/generated.ts b/src/types/generated.ts index 7d91f6f1..a6013668 100644 --- a/src/types/generated.ts +++ b/src/types/generated.ts @@ -67,4 +67,5 @@ export class HelperInputs { declare packages?: string; declare branch_name?: string; declare commit_message?: string; + declare match_comment_paths?: string; } diff --git a/test/helpers/check-merge-safety.test.ts b/test/helpers/check-merge-safety.test.ts index 33283a20..43a314b1 100644 --- a/test/helpers/check-merge-safety.test.ts +++ b/test/helpers/check-merge-safety.test.ts @@ -515,4 +515,201 @@ describe('checkMergeSafety', () => { ...context.repo }); }); + + describe('comment path matching', () => { + const mockListCommentsWithPaths = (paths: string[]) => { + const commentBody = `\n\`\`\`json\n${JSON.stringify(paths)}\n\`\`\``; + (octokit.issues.listComments as unknown as Mock).mockResolvedValue({ + data: [{ body: commentBody }] + }); + }; + + const mockListCommentsEmpty = () => { + (octokit.issues.listComments as unknown as Mock).mockResolvedValue({ + data: [] + }); + }; + + const mockListCommentsNoMarker = () => { + (octokit.issues.listComments as unknown as Mock).mockResolvedValue({ + data: [{ body: 'Some other comment' }] + }); + }; + + it('should prevent merge when branch is behind on comment paths', async () => { + const filesOutOfDate = ['Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout/src/file.swift']; + const changedFilesOnPr = ['Modules/EGSharedUI/EGSharedUI_DataModel/src/schema.swift']; + mockGithubRequests(filesOutOfDate, changedFilesOnPr); + mockListCommentsWithPaths([ + 'Modules/EGSharedUI/EGSharedUI_DataModel', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout' + ]); + + await checkMergeSafety({ + paths: allProjectPaths, + match_comment_paths: 'true', + ...context.repo + }); + + expect(octokit.repos.createCommitStatus).toHaveBeenCalledWith({ + sha, + state: 'failure', + context: 'Merge Safety', + description: 'Branch is behind on paths from comment: Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout. Please update with main.', + repo: 'repo', + owner: 'owner' + }); + expect(core.setFailed).toHaveBeenCalled(); + }); + + it('should allow merge when branch is up to date on all comment paths', async () => { + const filesOutOfDate = ['Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Trips/src/file.swift']; + const changedFilesOnPr = ['Modules/EGSharedUI/EGSharedUI_DataModel/src/schema.swift']; + mockGithubRequests(filesOutOfDate, changedFilesOnPr); + mockListCommentsWithPaths([ + 'Modules/EGSharedUI/EGSharedUI_DataModel', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout' + ]); + + await checkMergeSafety({ + paths: allProjectPaths, + match_comment_paths: 'true', + ...context.repo + }); + + expect(octokit.repos.createCommitStatus).toHaveBeenCalledWith({ + sha, + state: 'success', + context: 'Merge Safety', + description: 'Branch username:some-branch-name is safe to merge!', + repo: 'repo', + owner: 'owner' + }); + expect(core.setFailed).not.toHaveBeenCalled(); + }); + + it('should skip comment path check when no comment with marker is found', async () => { + const filesOutOfDate = ['Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout/src/file.swift']; + const changedFilesOnPr = ['Modules/EGSharedUI/EGSharedUI_DataModel/src/schema.swift']; + mockGithubRequests(filesOutOfDate, changedFilesOnPr); + mockListCommentsNoMarker(); + + await checkMergeSafety({ + paths: allProjectPaths, + match_comment_paths: 'true', + ...context.repo + }); + + expect(octokit.repos.createCommitStatus).toHaveBeenCalledWith({ + sha, + state: 'success', + context: 'Merge Safety', + description: 'Branch username:some-branch-name is safe to merge!', + repo: 'repo', + owner: 'owner' + }); + }); + + it('should skip comment path check when no comments exist', async () => { + const filesOutOfDate = ['Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout/src/file.swift']; + const changedFilesOnPr = ['Modules/EGSharedUI/EGSharedUI_DataModel/src/schema.swift']; + mockGithubRequests(filesOutOfDate, changedFilesOnPr); + mockListCommentsEmpty(); + + await checkMergeSafety({ + paths: allProjectPaths, + match_comment_paths: 'true', + ...context.repo + }); + + expect(octokit.repos.createCommitStatus).toHaveBeenCalledWith({ + sha, + state: 'success', + context: 'Merge Safety', + description: 'Branch username:some-branch-name is safe to merge!', + repo: 'repo', + owner: 'owner' + }); + }); + + it('should not perform comment path check when match_comment_paths is not set', async () => { + const filesOutOfDate = ['Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout/src/file.swift']; + const changedFilesOnPr = ['Modules/EGSharedUI/EGSharedUI_DataModel/src/schema.swift']; + mockGithubRequests(filesOutOfDate, changedFilesOnPr); + mockListCommentsWithPaths([ + 'Modules/EGSharedUI/EGSharedUI_DataModel', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout' + ]); + + await checkMergeSafety({ + paths: allProjectPaths, + ...context.repo + }); + + expect(octokit.issues.listComments).not.toHaveBeenCalled(); + expect(octokit.repos.createCommitStatus).toHaveBeenCalledWith({ + sha, + state: 'success', + context: 'Merge Safety', + description: 'Branch username:some-branch-name is safe to merge!', + repo: 'repo', + owner: 'owner' + }); + }); + + it('should still check existing path overlap when comment path matching passes', async () => { + const filesOutOfDate = ['packages/package-1/src/another-file.ts']; + const changedFilesOnPr = ['packages/package-1/src/some-file.ts']; + mockGithubRequests(filesOutOfDate, changedFilesOnPr); + mockListCommentsWithPaths(['Modules/EGSharedUI/EGSharedUI_DataModel']); + + await checkMergeSafety({ + paths: allProjectPaths, + match_comment_paths: 'true', + ...context.repo + }); + + expect(octokit.repos.createCommitStatus).toHaveBeenCalledWith({ + sha, + state: 'failure', + context: 'Merge Safety', + description: 'This branch has one or more outdated projects. Please update with main.', + repo: 'repo', + owner: 'owner' + }); + }); + + it('should truncate long list of outdated comment paths in status message', async () => { + const filesOutOfDate = [ + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout/src/file.swift', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Trips/src/file.swift', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Shopping/src/file.swift', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Deals/src/file.swift' + ]; + const changedFilesOnPr = ['Modules/EGSharedUI/EGSharedUI_DataModel/src/schema.swift']; + mockGithubRequests(filesOutOfDate, changedFilesOnPr); + mockListCommentsWithPaths([ + 'Modules/EGSharedUI/EGSharedUI_DataModel', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Checkout', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Trips', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Shopping', + 'Modules/EGSharedUI/EGSharedUI_Retail/EGSharedUI_Deals' + ]); + + await checkMergeSafety({ + paths: allProjectPaths, + match_comment_paths: 'true', + ...context.repo + }); + + expect(octokit.repos.createCommitStatus).toHaveBeenCalledWith({ + sha, + state: 'failure', + context: 'Merge Safety', + description: expect.stringContaining('...'), + repo: 'repo', + owner: 'owner' + }); + }); + }); });