From c2a0a772b58112744648c3c87d712577a5992eed Mon Sep 17 00:00:00 2001 From: Josh Johanning Date: Fri, 17 Apr 2026 14:38:06 -0500 Subject: [PATCH 1/5] feat: update draft PR comment when release is published (#94) When draft_release_pr_reminder is enabled and the workflow includes a release: [published] trigger, the action now automatically updates the original draft reminder comment on the merged PR to show the published state with checked-off next steps and a working release URL. Changes: - Add handleReleasePublished() to detect and update draft comments - Route release.published events in run() with early return - Refactor draft reminder to use buildDraftReminderBody() with marker - Add version-specific HTML marker for reliable comment matching - Support legacy comments via fallback matcher - Use predictable tag URL (draft URLs 404 after publishing) - Author-filter comments to only update our own - Skip already-published comments (idempotency) - Add 13 tests covering all paths - Update README with new section and trigger example - Bump version to 3.2.0 --- README.md | 24 +++- __tests__/index.test.js | 241 +++++++++++++++++++++++++++++++++++++++- action.yml | 2 +- badges/coverage.svg | 2 +- package-lock.json | 4 +- package.json | 2 +- src/index.js | 169 ++++++++++++++++++++++++++-- 7 files changed, 427 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 8851691..db569c8 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ Please refer to the [release page](https://github.com/joshjohanning/publish-gith | `publish_minor_version` | Whether to publish minor version tag (e.g., `v1.2`) | No | `false` | | `publish_release_branch` | Whether to publish release branch (e.g., `releases/v1.2.3`) | No | `false` | | `create_release_as_draft` | Whether to create release as draft to allow review of the release before publishing; useful with [immutable releases](https://docs.github.com/en/actions/how-tos/create-and-publish-actions/using-immutable-releases-and-tags-to-manage-your-actions-releases) where changes cannot be made after publishing | No | `false` | -| `draft_release_pr_reminder` | Post a reminder comment on the merged PR when creating a draft release | No | `false` | +| `draft_release_pr_reminder` | Post a reminder comment on the merged PR when creating a draft release. When the release is published (requires `release: [published]` trigger), the comment is updated to show the published state with checked-off steps. | No | `false` | | `comment_on_linked_issues` | Comment on closed issues linked to PRs in the release notes to notify followers of the release (uses GraphQL `closingIssuesReferences`; idempotent — updates existing comment on rerun). Comments are posted for both published and draft releases; the comment is updated if the version changes on a subsequent run. | No | `false` | ### Commit Signing Behavior @@ -50,6 +50,26 @@ The action automatically handles clean builds and file management: - **Dist folder cleaning**: When `commit_dist_folder: true` and `npm_package_command` is specified, the `dist/` folder is cleaned before building to ensure no stale files persist - **Automatic file deletion**: The action removes `.github/` files from release commits and properly handles renamed/deleted files in the `dist/` folder +### Draft Release PR Reminder + +When `draft_release_pr_reminder: true` is enabled, the action: + +1. **On PR merge** — Posts a reminder comment on the merged PR with a link to the draft release and a next-steps checklist +2. **On release publish** — Automatically updates the comment to show "✅ Release Published" with checked-off steps and a working link + +To enable comment updates when a draft release is published, add the `release: [published]` trigger to your workflow: + +```yml +on: + push: + branches: + - main + release: + types: [published] +``` + +> **Note:** Legacy draft comments (created before this update feature was added) are also detected and updated via a fallback matcher. + ## Permissions The action requires specific permissions depending on features used: @@ -80,6 +100,8 @@ on: push: branches: - main + release: + types: [published] # Required for updating PR comment after draft release is published jobs: publish: diff --git a/__tests__/index.test.js b/__tests__/index.test.js index 4e1bce4..43b0c5c 100644 --- a/__tests__/index.test.js +++ b/__tests__/index.test.js @@ -20,10 +20,16 @@ const mockExec = { }; // Mock the @actions/github module +const mockGithubContext = { + eventName: 'push', + repo: { owner: 'test-owner', repo: 'test-repo' }, + sha: 'abc123def456', + payload: {} +}; + const mockGithub = { - context: { - repo: { owner: 'test-owner', repo: 'test-repo' }, - sha: 'abc123def456' + get context() { + return mockGithubContext; }, getOctokit: jest.fn() }; @@ -55,6 +61,9 @@ const mockOctokit = { }, users: { getAuthenticated: jest.fn() + }, + pulls: { + list: jest.fn() } }, paginate: jest.fn(), @@ -117,6 +126,12 @@ describe('Publish GitHub Action', () => { beforeEach(() => { jest.clearAllMocks(); + // Reset context to default push event + mockGithubContext.eventName = 'push'; + mockGithubContext.repo = { owner: 'test-owner', repo: 'test-repo' }; + mockGithubContext.sha = 'abc123def456'; + mockGithubContext.payload = {}; + // Set up default mocks mockGithub.getOctokit.mockReturnValue(mockOctokit); @@ -166,6 +181,7 @@ describe('Publish GitHub Action', () => { mockOctokit.rest.issues.createComment.mockResolvedValue({ data: { id: 456 } }); mockOctokit.rest.issues.updateComment.mockResolvedValue({ data: { id: 456 } }); mockOctokit.rest.users.getAuthenticated.mockResolvedValue({ data: { login: 'test-bot' } }); + mockOctokit.rest.pulls.list.mockResolvedValue({ data: [] }); mockOctokit.graphql.mockResolvedValue({ repository: { pullRequest: { @@ -2066,4 +2082,223 @@ describe('Publish GitHub Action', () => { expect(mockCore.setFailed).toHaveBeenCalledWith('Object does not exist'); }); }); + + describe('release.published event (update draft PR comment)', () => { + const releasePayload = { + action: 'published', + release: { + tag_name: 'v1.2.3', + html_url: 'https://github.com/test-owner/test-repo/releases/tag/untagged-abc123def456' + } + }; + + beforeEach(() => { + mockGithubContext.eventName = 'release'; + mockGithubContext.payload = releasePayload; + }); + + it('should route release.published event to handleReleasePublished', async () => { + mockOctokit.rest.pulls.list.mockResolvedValue({ data: [] }); + + await run(); + + expect(mockOctokit.rest.pulls.list).toHaveBeenCalledWith( + expect.objectContaining({ + state: 'closed', + sort: 'updated', + per_page: 100 + }) + ); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('Release published: v1.2.3')); + expect(mockCore.info).toHaveBeenCalledWith('✅ Release published event handled!'); + }); + + it('should not read package.json or create releases on release event', async () => { + mockOctokit.rest.pulls.list.mockResolvedValue({ data: [] }); + + await run(); + + // These are part of the normal publish flow — should NOT be called + expect(mockOctokit.rest.repos.listTags).not.toHaveBeenCalled(); + expect(mockOctokit.rest.repos.createRelease).not.toHaveBeenCalled(); + expect(mockExec.exec).not.toHaveBeenCalled(); + }); + + it('should skip when draft_release_pr_reminder is false', async () => { + mockCore.getInput.mockImplementation(name => { + if (name === 'draft_release_pr_reminder') return 'false'; + if (name === 'github_token') return 'test-token'; + if (name === 'github_api_url') return 'https://api.github.com'; + return ''; + }); + + await run(); + + expect(mockOctokit.rest.pulls.list).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith('Skipping PR comment update (draft_release_pr_reminder is disabled)'); + }); + + it('should warn and return on missing release payload', async () => { + mockGithubContext.payload = { action: 'published', release: {} }; + + await run(); + + expect(mockCore.warning).toHaveBeenCalledWith( + 'Release event missing expected payload data; cannot update PR comments.' + ); + expect(mockOctokit.rest.pulls.list).not.toHaveBeenCalled(); + }); + + it('should find and update a draft comment by version-specific marker', async () => { + const draftBody = + '\n## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; + + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] + }); + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 100, body: draftBody, user: { login: 'test-bot' } }] + }); + + await run(); + + expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith( + expect.objectContaining({ + comment_id: 100, + body: expect.stringContaining('## ✅ Release Published') + }) + ); + expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.stringContaining('https://github.com/test-owner/test-repo/releases/tag/v1.2.3') + }) + ); + expect(mockCore.info).toHaveBeenCalledWith('✅ Updated PR comment on PR #42'); + }); + + it('should find and update a legacy comment without marker', async () => { + // Legacy comments don't have the HTML marker + const legacyBody = '## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; + + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 55, merged_at: '2024-01-01T00:00:00Z' }] + }); + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 200, body: legacyBody, user: { login: 'test-bot' } }] + }); + + await run(); + + expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith( + expect.objectContaining({ + comment_id: 200, + body: expect.stringContaining('## ✅ Release Published') + }) + ); + }); + + it('should skip unmerged PRs', async () => { + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 10, merged_at: null }] + }); + + await run(); + + expect(mockOctokit.rest.issues.listComments).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('No PR comment found')); + }); + + it('should skip comments from other users', async () => { + const draftBody = + '\n## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; + + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] + }); + // Comment is from a different user + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 100, body: draftBody, user: { login: 'other-user' } }] + }); + + await run(); + + expect(mockOctokit.rest.issues.updateComment).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('No PR comment found')); + }); + + it('should still match comments when auth fails (no author filter)', async () => { + mockOctokit.rest.users.getAuthenticated.mockRejectedValue(new Error('Auth failed')); + + const draftBody = + '\n## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; + + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] + }); + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 100, body: draftBody, user: { login: 'unknown-bot' } }] + }); + + await run(); + + // Should still update since author filter is disabled + expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith(expect.objectContaining({ comment_id: 100 })); + }); + + it('should skip if comment already shows published state (idempotency)', async () => { + const publishedBody = '\n## ✅ Release Published\n\nAlready done.'; + + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] + }); + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 100, body: publishedBody, user: { login: 'test-bot' } }] + }); + + await run(); + + expect(mockOctokit.rest.issues.updateComment).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('already shows published state')); + }); + + it('should log info when no matching comment is found', async () => { + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] + }); + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 100, body: 'Unrelated comment', user: { login: 'test-bot' } }] + }); + + await run(); + + expect(mockOctokit.rest.issues.updateComment).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('No PR comment found')); + }); + + it('should use predictable tag URL instead of draft URL', async () => { + const draftBody = '\n## 📦 Draft Release Created'; + + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] + }); + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 100, body: draftBody, user: { login: 'test-bot' } }] + }); + + await run(); + + // The URL in the updated comment should be the predictable tag URL, not the untagged draft URL + const updateCall = mockOctokit.rest.issues.updateComment.mock.calls[0][0]; + expect(updateCall.body).toContain('https://github.com/test-owner/test-repo/releases/tag/v1.2.3'); + expect(updateCall.body).not.toContain('untagged-'); + }); + + it('should warn on API error when listing PRs', async () => { + mockOctokit.rest.pulls.list.mockRejectedValue(new Error('API rate limit exceeded')); + + await run(); + + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining('Could not update PR comment')); + }); + }); }); diff --git a/action.yml b/action.yml index d233306..e70ee2e 100644 --- a/action.yml +++ b/action.yml @@ -37,7 +37,7 @@ inputs: default: 'false' required: false draft_release_pr_reminder: - description: 'Post a reminder comment on the merged PR when creating a draft release' + description: 'Post a reminder comment on the merged PR when creating a draft release. When the release is later published (requires `release: [published]` trigger), the comment is updated to show the published state with checked-off steps.' default: 'false' required: false comment_on_linked_issues: diff --git a/badges/coverage.svg b/badges/coverage.svg index 63077f6..a7b84f4 100644 --- a/badges/coverage.svg +++ b/badges/coverage.svg @@ -1 +1 @@ -Coverage: 97.42%Coverage97.42% \ No newline at end of file +Coverage: 97.55%Coverage97.55% \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index e84c927..28ce943 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "publish-github-action", - "version": "3.1.2", + "version": "3.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "publish-github-action", - "version": "3.1.2", + "version": "3.2.0", "license": "MIT", "dependencies": { "@actions/core": "^3.0.0", diff --git a/package.json b/package.json index 3444c1c..104d31a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "publish-github-action", - "version": "3.1.2", + "version": "3.2.0", "type": "module", "private": true, "description": "Publish your GitHub Action", diff --git a/src/index.js b/src/index.js index db83399..a950b7c 100644 --- a/src/index.js +++ b/src/index.js @@ -291,6 +291,9 @@ export function parsePullRequestNumbers(text) { const RELEASE_COMMENT_MARKER = ''; +/** Version-specific marker prefix for draft release PR reminder comments */ +const DRAFT_COMMENT_MARKER_PREFIX = '\n` + + `## 📦 Draft Release Created\n\n` + + `A draft release **${version}** has been created for this PR.\n\n` + + `🔗 **[View Draft Release](${releaseUrl})**\n\n` + + `### Next Steps\n` + + `- [ ] Review the release notes\n` + + `- [ ] Publish the release to make it permanent\n\n` + + `> _This is an automated reminder from the publish-github-action workflow._` + ); +} + +/** + * Build the published release PR comment body (replaces draft reminder). + * @param {string} version - Version tag (e.g. v1.2.3) + * @param {string} releaseUrl - URL to the published release + * @returns {string} Comment body + */ +function buildPublishedReminderBody(version, releaseUrl) { + return ( + `${DRAFT_COMMENT_MARKER_PREFIX}${version} -->\n` + + `## ✅ Release Published\n\n` + + `Release **${version}** has been published!\n\n` + + `🔗 **[View Published Release](${releaseUrl})**\n\n` + + `### Next Steps\n` + + `- [x] Review the release notes\n` + + `- [x] Publish the release to make it permanent\n\n` + + `> _This comment was updated by the publish-github-action workflow._` + ); +} + +/** + * Handle release.published event by updating the draft PR reminder comment. + * @param {object} octokit - GitHub API client + * @param {object} context - GitHub Actions context + */ +async function handleReleasePublished(octokit, context) { + const release = context.payload?.release; + + if (!release || !release.tag_name || !release.html_url) { + core.warning('Release event missing expected payload data; cannot update PR comments.'); + return; + } + + const version = release.tag_name; + // Construct predictable tag-based URL (draft html_url contains untagged-... that 404s) + const repoUrl = release.html_url.replace(/\/releases\/tag\/.*$/, ''); + const releaseUrl = `${repoUrl}/releases/tag/${version}`; + const marker = `${DRAFT_COMMENT_MARKER_PREFIX}${version} -->`; + + core.info(`Release published: ${version}`); + core.info(`Searching for PR comment with marker: ${marker}`); + + // Get authenticated user for author filtering + let authenticatedLogin = null; + try { + const { data: authUser } = await retryWithBackoff(() => octokit.rest.users.getAuthenticated(), { + retries: 2, + baseDelay: 1000, + description: 'Get authenticated user' + }); + authenticatedLogin = authUser.login; + core.debug(`Authenticated as: ${authenticatedLogin}`); + } catch (error) { + core.debug(`Could not determine authenticated user: ${error.message}`); + } + + try { + // Search recent closed PRs for the draft comment + const { data: pulls } = await retryWithBackoff( + () => + octokit.rest.pulls.list({ + owner: context.repo.owner, + repo: context.repo.repo, + state: 'closed', + sort: 'updated', + direction: 'desc', + per_page: 100 + }), + { retries: 2, baseDelay: 1000, description: 'List recent closed PRs' } + ); + + for (const pr of pulls) { + if (!pr.merged_at) continue; + + const { data: comments } = await retryWithBackoff( + () => + octokit.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: pr.number, + per_page: 100 + }), + { retries: 2, baseDelay: 1000, description: `List comments on PR #${pr.number}` } + ); + + // Match by version-specific marker, or fallback to legacy comment shape + const markerComment = comments.find(c => { + if (!c.body) return false; + // Skip if we know the author and it's not us + if (authenticatedLogin && c.user?.login !== authenticatedLogin) return false; + // Primary: version-specific marker + if (c.body.includes(marker)) return true; + // Fallback: legacy comments without marker (created before this feature) + if (c.body.includes('## 📦 Draft Release Created') && c.body.includes(`**${version}**`)) return true; + return false; + }); + + if (markerComment) { + // Check if already updated + if (markerComment.body.includes('## ✅ Release Published')) { + core.info(`PR #${pr.number} comment already shows published state, skipping`); + return; + } + + const updatedBody = buildPublishedReminderBody(version, releaseUrl); + + await retryWithBackoff( + () => + octokit.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: markerComment.id, + body: updatedBody + }), + { retries: 2, baseDelay: 1000, description: `Update comment on PR #${pr.number}` } + ); + + core.info(`✅ Updated PR comment on PR #${pr.number}`); + return; + } + } + + core.info(`No PR comment found with marker for ${version}`); + } catch (error) { + core.warning(`Could not update PR comment: ${error.message}`); + } +} + /** * Main action logic */ @@ -321,6 +470,17 @@ export async function run() { const opts = githubApiUrl ? { baseUrl: githubApiUrl } : {}; const octokit = github.getOctokit(githubToken, opts); + // Handle release.published event — update the draft PR reminder comment + if (context.eventName === 'release' && context.payload?.action === 'published') { + if (draftReleasePrReminder !== 'false') { + await handleReleasePublished(octokit, context); + } else { + core.info('Skipping PR comment update (draft_release_pr_reminder is disabled)'); + } + core.info('✅ Release published event handled!'); + return; + } + const json = JSON.parse(readFileSync('package.json', 'utf8')); const version = `v${json.version}`; const minorVersion = `v${semver.major(json.version)}.${semver.minor(json.version)}`; @@ -536,14 +696,7 @@ export async function run() { const mergedPr = mergedPrs[0]; const releaseUrl = release.data.html_url; - const commentBody = - `## 📦 Draft Release Created\n\n` + - `A draft release **${version}** has been created for this PR.\n\n` + - `🔗 **[View Draft Release](${releaseUrl})**\n\n` + - `### Next Steps\n` + - `- [ ] Review the release notes\n` + - `- [ ] Publish the release to make it permanent\n\n` + - `> _This is an automated reminder from the publish-github-action workflow._`; + const commentBody = buildDraftReminderBody(version, releaseUrl); await octokit.rest.issues.createComment({ owner: context.repo.owner, From ebd1ff56e14405dbd218c58e39924698e1366af0 Mon Sep 17 00:00:00 2001 From: Josh Johanning Date: Fri, 17 Apr 2026 20:59:21 -0500 Subject: [PATCH 2/5] fix: address CCR feedback on release published handler - Use strict === 'true' check for draft_release_pr_reminder (consistent) - Add direction: 'desc' to listComments for newest-first ordering - Disable legacy fallback when auth fails (require confirmed author) - Add test for legacy fallback skipped without auth --- __tests__/index.test.js | 22 +++++++++++++++++++++- src/index.js | 11 +++++++---- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/__tests__/index.test.js b/__tests__/index.test.js index 43b0c5c..4b5a848 100644 --- a/__tests__/index.test.js +++ b/__tests__/index.test.js @@ -2241,10 +2241,30 @@ describe('Publish GitHub Action', () => { await run(); - // Should still update since author filter is disabled + // Should still update since marker match doesn't require author expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledWith(expect.objectContaining({ comment_id: 100 })); }); + it('should skip legacy fallback when auth fails (no author confirmation)', async () => { + mockOctokit.rest.users.getAuthenticated.mockRejectedValue(new Error('Auth failed')); + + // Legacy comment has no marker — only heading + version + const legacyBody = '## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; + + mockOctokit.rest.pulls.list.mockResolvedValue({ + data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] + }); + mockOctokit.rest.issues.listComments.mockResolvedValue({ + data: [{ id: 100, body: legacyBody, user: { login: 'unknown-bot' } }] + }); + + await run(); + + // Should NOT update — legacy fallback requires confirmed author + expect(mockOctokit.rest.issues.updateComment).not.toHaveBeenCalled(); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('No PR comment found')); + }); + it('should skip if comment already shows published state (idempotency)', async () => { const publishedBody = '\n## ✅ Release Published\n\nAlready done.'; diff --git a/src/index.js b/src/index.js index a950b7c..bea5650 100644 --- a/src/index.js +++ b/src/index.js @@ -402,7 +402,8 @@ async function handleReleasePublished(octokit, context) { owner: context.repo.owner, repo: context.repo.repo, issue_number: pr.number, - per_page: 100 + per_page: 100, + direction: 'desc' }), { retries: 2, baseDelay: 1000, description: `List comments on PR #${pr.number}` } ); @@ -415,7 +416,9 @@ async function handleReleasePublished(octokit, context) { // Primary: version-specific marker if (c.body.includes(marker)) return true; // Fallback: legacy comments without marker (created before this feature) - if (c.body.includes('## 📦 Draft Release Created') && c.body.includes(`**${version}**`)) return true; + // Only use fallback when author is confirmed to avoid updating someone else's comment + if (authenticatedLogin && c.body.includes('## 📦 Draft Release Created') && c.body.includes(`**${version}**`)) + return true; return false; }); @@ -472,7 +475,7 @@ export async function run() { // Handle release.published event — update the draft PR reminder comment if (context.eventName === 'release' && context.payload?.action === 'published') { - if (draftReleasePrReminder !== 'false') { + if (draftReleasePrReminder === 'true') { await handleReleasePublished(octokit, context); } else { core.info('Skipping PR comment update (draft_release_pr_reminder is disabled)'); @@ -674,7 +677,7 @@ export async function run() { }); // Post reminder comment on merged PR if draft release was created - if (createReleaseAsDraft === 'true' && draftReleasePrReminder !== 'false') { + if (createReleaseAsDraft === 'true' && draftReleasePrReminder === 'true') { try { // Find the PR associated with the current commit (the merge commit) const commitShaForPr = context.sha; From cbf7524726184c8766d5104f807ef9559e68becf Mon Sep 17 00:00:00 2001 From: Josh Johanning Date: Fri, 17 Apr 2026 21:11:31 -0500 Subject: [PATCH 3/5] fix: use tag ref lookup for deterministic PR resolution - Replace pulls.list scan with git.getRef + listPullRequestsAssociatedWithCommit - Fix workflow syntax references in action.yml and README descriptions - Update tests to use new API approach --- README.md | 2 +- __tests__/index.test.js | 43 +++++++++++++++++++---------------------- action.yml | 2 +- badges/coverage.svg | 2 +- src/index.js | 33 +++++++++++++++++++++---------- 5 files changed, 46 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index db569c8..0417dba 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ Please refer to the [release page](https://github.com/joshjohanning/publish-gith | `publish_minor_version` | Whether to publish minor version tag (e.g., `v1.2`) | No | `false` | | `publish_release_branch` | Whether to publish release branch (e.g., `releases/v1.2.3`) | No | `false` | | `create_release_as_draft` | Whether to create release as draft to allow review of the release before publishing; useful with [immutable releases](https://docs.github.com/en/actions/how-tos/create-and-publish-actions/using-immutable-releases-and-tags-to-manage-your-actions-releases) where changes cannot be made after publishing | No | `false` | -| `draft_release_pr_reminder` | Post a reminder comment on the merged PR when creating a draft release. When the release is published (requires `release: [published]` trigger), the comment is updated to show the published state with checked-off steps. | No | `false` | +| `draft_release_pr_reminder` | Post a reminder comment on the merged PR when creating a draft release. When the release is published (requires `on: release: types: [published]` trigger), the comment is updated to show the published state with checked-off steps. | No | `false` | | `comment_on_linked_issues` | Comment on closed issues linked to PRs in the release notes to notify followers of the release (uses GraphQL `closingIssuesReferences`; idempotent — updates existing comment on rerun). Comments are posted for both published and draft releases; the comment is updated if the version changes on a subsequent run. | No | `false` | ### Commit Signing Behavior diff --git a/__tests__/index.test.js b/__tests__/index.test.js index 4b5a848..02082f5 100644 --- a/__tests__/index.test.js +++ b/__tests__/index.test.js @@ -2095,27 +2095,24 @@ describe('Publish GitHub Action', () => { beforeEach(() => { mockGithubContext.eventName = 'release'; mockGithubContext.payload = releasePayload; + + // Default: tag resolves to a commit, commit is associated with a merged PR + mockOctokit.rest.git.getRef.mockResolvedValue({ data: { object: { sha: 'tag-commit-sha' } } }); + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [] }); }); it('should route release.published event to handleReleasePublished', async () => { - mockOctokit.rest.pulls.list.mockResolvedValue({ data: [] }); - await run(); - expect(mockOctokit.rest.pulls.list).toHaveBeenCalledWith( - expect.objectContaining({ - state: 'closed', - sort: 'updated', - per_page: 100 - }) + expect(mockOctokit.rest.git.getRef).toHaveBeenCalledWith(expect.objectContaining({ ref: 'tags/v1.2.3' })); + expect(mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit).toHaveBeenCalledWith( + expect.objectContaining({ commit_sha: 'tag-commit-sha' }) ); expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('Release published: v1.2.3')); expect(mockCore.info).toHaveBeenCalledWith('✅ Release published event handled!'); }); it('should not read package.json or create releases on release event', async () => { - mockOctokit.rest.pulls.list.mockResolvedValue({ data: [] }); - await run(); // These are part of the normal publish flow — should NOT be called @@ -2134,7 +2131,7 @@ describe('Publish GitHub Action', () => { await run(); - expect(mockOctokit.rest.pulls.list).not.toHaveBeenCalled(); + expect(mockOctokit.rest.git.getRef).not.toHaveBeenCalledWith(expect.objectContaining({ ref: 'tags/v1.2.3' })); expect(mockCore.info).toHaveBeenCalledWith('Skipping PR comment update (draft_release_pr_reminder is disabled)'); }); @@ -2146,14 +2143,14 @@ describe('Publish GitHub Action', () => { expect(mockCore.warning).toHaveBeenCalledWith( 'Release event missing expected payload data; cannot update PR comments.' ); - expect(mockOctokit.rest.pulls.list).not.toHaveBeenCalled(); + expect(mockOctokit.rest.git.getRef).not.toHaveBeenCalledWith(expect.objectContaining({ ref: 'tags/v1.2.3' })); }); it('should find and update a draft comment by version-specific marker', async () => { const draftBody = '\n## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] }); mockOctokit.rest.issues.listComments.mockResolvedValue({ @@ -2180,7 +2177,7 @@ describe('Publish GitHub Action', () => { // Legacy comments don't have the HTML marker const legacyBody = '## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 55, merged_at: '2024-01-01T00:00:00Z' }] }); mockOctokit.rest.issues.listComments.mockResolvedValue({ @@ -2198,7 +2195,7 @@ describe('Publish GitHub Action', () => { }); it('should skip unmerged PRs', async () => { - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 10, merged_at: null }] }); @@ -2212,7 +2209,7 @@ describe('Publish GitHub Action', () => { const draftBody = '\n## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] }); // Comment is from a different user @@ -2232,7 +2229,7 @@ describe('Publish GitHub Action', () => { const draftBody = '\n## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] }); mockOctokit.rest.issues.listComments.mockResolvedValue({ @@ -2251,7 +2248,7 @@ describe('Publish GitHub Action', () => { // Legacy comment has no marker — only heading + version const legacyBody = '## 📦 Draft Release Created\n\nA draft release **v1.2.3** has been created.'; - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] }); mockOctokit.rest.issues.listComments.mockResolvedValue({ @@ -2268,7 +2265,7 @@ describe('Publish GitHub Action', () => { it('should skip if comment already shows published state (idempotency)', async () => { const publishedBody = '\n## ✅ Release Published\n\nAlready done.'; - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] }); mockOctokit.rest.issues.listComments.mockResolvedValue({ @@ -2282,7 +2279,7 @@ describe('Publish GitHub Action', () => { }); it('should log info when no matching comment is found', async () => { - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] }); mockOctokit.rest.issues.listComments.mockResolvedValue({ @@ -2298,7 +2295,7 @@ describe('Publish GitHub Action', () => { it('should use predictable tag URL instead of draft URL', async () => { const draftBody = '\n## 📦 Draft Release Created'; - mockOctokit.rest.pulls.list.mockResolvedValue({ + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ data: [{ number: 42, merged_at: '2024-01-01T00:00:00Z' }] }); mockOctokit.rest.issues.listComments.mockResolvedValue({ @@ -2313,8 +2310,8 @@ describe('Publish GitHub Action', () => { expect(updateCall.body).not.toContain('untagged-'); }); - it('should warn on API error when listing PRs', async () => { - mockOctokit.rest.pulls.list.mockRejectedValue(new Error('API rate limit exceeded')); + it('should warn on API error when resolving tag', async () => { + mockOctokit.rest.git.getRef.mockRejectedValue(new Error('API rate limit exceeded')); await run(); diff --git a/action.yml b/action.yml index e70ee2e..0d0333d 100644 --- a/action.yml +++ b/action.yml @@ -37,7 +37,7 @@ inputs: default: 'false' required: false draft_release_pr_reminder: - description: 'Post a reminder comment on the merged PR when creating a draft release. When the release is later published (requires `release: [published]` trigger), the comment is updated to show the published state with checked-off steps.' + description: 'Post a reminder comment on the merged PR when creating a draft release. When the release is later published (requires `on: release: types: [published]` trigger), the comment is updated to show the published state with checked-off steps.' default: 'false' required: false comment_on_linked_issues: diff --git a/badges/coverage.svg b/badges/coverage.svg index a7b84f4..3fc8429 100644 --- a/badges/coverage.svg +++ b/badges/coverage.svg @@ -1 +1 @@ -Coverage: 97.55%Coverage97.55% \ No newline at end of file +Coverage: 97.59%Coverage97.59% \ No newline at end of file diff --git a/src/index.js b/src/index.js index bea5650..fdbc157 100644 --- a/src/index.js +++ b/src/index.js @@ -379,23 +379,36 @@ async function handleReleasePublished(octokit, context) { } try { - // Search recent closed PRs for the draft comment - const { data: pulls } = await retryWithBackoff( + // Resolve the tag to its commit SHA, then find associated PRs deterministically + const { data: tagRef } = await retryWithBackoff( () => - octokit.rest.pulls.list({ + octokit.rest.git.getRef({ owner: context.repo.owner, repo: context.repo.repo, - state: 'closed', - sort: 'updated', - direction: 'desc', - per_page: 100 + ref: `tags/${version}` }), - { retries: 2, baseDelay: 1000, description: 'List recent closed PRs' } + { retries: 2, baseDelay: 1000, description: `Get tag ref for ${version}` } ); - for (const pr of pulls) { - if (!pr.merged_at) continue; + const commitSha = tagRef.object.sha; + core.debug(`Tag ${version} points to commit ${commitSha}`); + const { data: associatedPrs } = await retryWithBackoff( + () => + octokit.rest.repos.listPullRequestsAssociatedWithCommit({ + owner: context.repo.owner, + repo: context.repo.repo, + commit_sha: commitSha, + per_page: 30 + }), + { retries: 2, baseDelay: 1000, description: 'List PRs associated with tag commit' } + ); + + // Filter to merged PRs only + const mergedPrs = associatedPrs.filter(pr => pr.merged_at); + core.debug(`Found ${mergedPrs.length} merged PR(s) associated with tag commit`); + + for (const pr of mergedPrs) { const { data: comments } = await retryWithBackoff( () => octokit.rest.issues.listComments({ From d1ec8e3594c3cc9ede4caeb97a385de951c99da4 Mon Sep 17 00:00:00 2001 From: Josh Johanning Date: Sat, 18 Apr 2026 14:37:51 -0500 Subject: [PATCH 4/5] fix: use continue instead of return for multi-PR support, fix syntax refs - Replace return with continue in loop so all associated PRs get updated - Fix remaining release: [published] reference in README prose - Stricter test assertions using .not.toHaveBeenCalled() --- README.md | 2 +- __tests__/index.test.js | 4 ++-- badges/coverage.svg | 2 +- src/index.js | 3 +-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 0417dba..4ab5ab1 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ When `draft_release_pr_reminder: true` is enabled, the action: 1. **On PR merge** — Posts a reminder comment on the merged PR with a link to the draft release and a next-steps checklist 2. **On release publish** — Automatically updates the comment to show "✅ Release Published" with checked-off steps and a working link -To enable comment updates when a draft release is published, add the `release: [published]` trigger to your workflow: +To enable comment updates when a draft release is published, add the `on: release: types: [published]` trigger to your workflow: ```yml on: diff --git a/__tests__/index.test.js b/__tests__/index.test.js index 02082f5..3dd373c 100644 --- a/__tests__/index.test.js +++ b/__tests__/index.test.js @@ -2131,7 +2131,7 @@ describe('Publish GitHub Action', () => { await run(); - expect(mockOctokit.rest.git.getRef).not.toHaveBeenCalledWith(expect.objectContaining({ ref: 'tags/v1.2.3' })); + expect(mockOctokit.rest.git.getRef).not.toHaveBeenCalled(); expect(mockCore.info).toHaveBeenCalledWith('Skipping PR comment update (draft_release_pr_reminder is disabled)'); }); @@ -2143,7 +2143,7 @@ describe('Publish GitHub Action', () => { expect(mockCore.warning).toHaveBeenCalledWith( 'Release event missing expected payload data; cannot update PR comments.' ); - expect(mockOctokit.rest.git.getRef).not.toHaveBeenCalledWith(expect.objectContaining({ ref: 'tags/v1.2.3' })); + expect(mockOctokit.rest.git.getRef).not.toHaveBeenCalled(); }); it('should find and update a draft comment by version-specific marker', async () => { diff --git a/badges/coverage.svg b/badges/coverage.svg index 3fc8429..429869a 100644 --- a/badges/coverage.svg +++ b/badges/coverage.svg @@ -1 +1 @@ -Coverage: 97.59%Coverage97.59% \ No newline at end of file +Coverage: 97.58%Coverage97.58% \ No newline at end of file diff --git a/src/index.js b/src/index.js index fdbc157..18dbfed 100644 --- a/src/index.js +++ b/src/index.js @@ -439,7 +439,7 @@ async function handleReleasePublished(octokit, context) { // Check if already updated if (markerComment.body.includes('## ✅ Release Published')) { core.info(`PR #${pr.number} comment already shows published state, skipping`); - return; + continue; } const updatedBody = buildPublishedReminderBody(version, releaseUrl); @@ -456,7 +456,6 @@ async function handleReleasePublished(octokit, context) { ); core.info(`✅ Updated PR comment on PR #${pr.number}`); - return; } } From 1ed81e462366b58ffa2f5483121f8067cc60d18b Mon Sep 17 00:00:00 2001 From: Josh Johanning Date: Sat, 18 Apr 2026 17:41:21 -0500 Subject: [PATCH 5/5] fix: per-PR error handling in handleReleasePublished Wrap per-PR logic in its own try/catch so a failed updateComment (e.g., permission/ownership mismatch) logs a warning and continues to the next PR instead of aborting the entire scan. Also fix the 'no comment found' log to only appear when no comments were actually updated (track updatedCount). --- __tests__/index.test.js | 32 ++++++++++++++ badges/coverage.svg | 2 +- src/index.js | 92 ++++++++++++++++++++++------------------- 3 files changed, 83 insertions(+), 43 deletions(-) diff --git a/__tests__/index.test.js b/__tests__/index.test.js index 3dd373c..15259a5 100644 --- a/__tests__/index.test.js +++ b/__tests__/index.test.js @@ -2317,5 +2317,37 @@ describe('Publish GitHub Action', () => { expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining('Could not update PR comment')); }); + + it('should continue to next PR when updateComment fails on one', async () => { + // Two merged PRs associated with the tag commit + mockOctokit.rest.repos.listPullRequestsAssociatedWithCommit.mockResolvedValue({ + data: [ + { number: 50, merged_at: '2024-01-01T00:00:00Z' }, + { number: 51, merged_at: '2024-01-01T00:00:00Z' } + ] + }); + + const marker = ``; + // PR #50 has the marker but updateComment will fail; PR #51 also has the marker + mockOctokit.rest.issues.listComments + .mockResolvedValueOnce({ + data: [{ id: 600, body: `${marker}\n## 📦 Draft Release Created`, user: { login: 'test-bot' } }] + }) + .mockResolvedValueOnce({ + data: [{ id: 601, body: `${marker}\n## 📦 Draft Release Created`, user: { login: 'test-bot' } }] + }); + + // First PR fails immediately (non-retryable, no retry attempts), second PR succeeds + mockOctokit.rest.issues.updateComment + .mockRejectedValueOnce(new Error('Resource not accessible by integration')) + .mockResolvedValueOnce({}); + + await run(); + + // Should warn about PR #50 but still update PR #51 + expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining('Could not update comment on PR #50')); + expect(mockOctokit.rest.issues.updateComment).toHaveBeenCalledTimes(2); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining('Updated PR comment on PR #51')); + }); }); }); diff --git a/badges/coverage.svg b/badges/coverage.svg index 429869a..625ab0e 100644 --- a/badges/coverage.svg +++ b/badges/coverage.svg @@ -1 +1 @@ -Coverage: 97.58%Coverage97.58% \ No newline at end of file +Coverage: 97.61%Coverage97.61% \ No newline at end of file diff --git a/src/index.js b/src/index.js index 18dbfed..e3b88f4 100644 --- a/src/index.js +++ b/src/index.js @@ -408,58 +408,66 @@ async function handleReleasePublished(octokit, context) { const mergedPrs = associatedPrs.filter(pr => pr.merged_at); core.debug(`Found ${mergedPrs.length} merged PR(s) associated with tag commit`); + let updatedCount = 0; for (const pr of mergedPrs) { - const { data: comments } = await retryWithBackoff( - () => - octokit.rest.issues.listComments({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: pr.number, - per_page: 100, - direction: 'desc' - }), - { retries: 2, baseDelay: 1000, description: `List comments on PR #${pr.number}` } - ); - - // Match by version-specific marker, or fallback to legacy comment shape - const markerComment = comments.find(c => { - if (!c.body) return false; - // Skip if we know the author and it's not us - if (authenticatedLogin && c.user?.login !== authenticatedLogin) return false; - // Primary: version-specific marker - if (c.body.includes(marker)) return true; - // Fallback: legacy comments without marker (created before this feature) - // Only use fallback when author is confirmed to avoid updating someone else's comment - if (authenticatedLogin && c.body.includes('## 📦 Draft Release Created') && c.body.includes(`**${version}**`)) - return true; - return false; - }); - - if (markerComment) { - // Check if already updated - if (markerComment.body.includes('## ✅ Release Published')) { - core.info(`PR #${pr.number} comment already shows published state, skipping`); - continue; - } - - const updatedBody = buildPublishedReminderBody(version, releaseUrl); - - await retryWithBackoff( + try { + const { data: comments } = await retryWithBackoff( () => - octokit.rest.issues.updateComment({ + octokit.rest.issues.listComments({ owner: context.repo.owner, repo: context.repo.repo, - comment_id: markerComment.id, - body: updatedBody + issue_number: pr.number, + per_page: 100, + direction: 'desc' }), - { retries: 2, baseDelay: 1000, description: `Update comment on PR #${pr.number}` } + { retries: 2, baseDelay: 1000, description: `List comments on PR #${pr.number}` } ); - core.info(`✅ Updated PR comment on PR #${pr.number}`); + // Match by version-specific marker, or fallback to legacy comment shape + const markerComment = comments.find(c => { + if (!c.body) return false; + // Skip if we know the author and it's not us + if (authenticatedLogin && c.user?.login !== authenticatedLogin) return false; + // Primary: version-specific marker + if (c.body.includes(marker)) return true; + // Fallback: legacy comments without marker (created before this feature) + // Only use fallback when author is confirmed to avoid updating someone else's comment + if (authenticatedLogin && c.body.includes('## 📦 Draft Release Created') && c.body.includes(`**${version}**`)) + return true; + return false; + }); + + if (markerComment) { + // Check if already updated + if (markerComment.body.includes('## ✅ Release Published')) { + core.info(`PR #${pr.number} comment already shows published state, skipping`); + continue; + } + + const updatedBody = buildPublishedReminderBody(version, releaseUrl); + + await retryWithBackoff( + () => + octokit.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: markerComment.id, + body: updatedBody + }), + { retries: 2, baseDelay: 1000, description: `Update comment on PR #${pr.number}` } + ); + + core.info(`✅ Updated PR comment on PR #${pr.number}`); + updatedCount++; + } + } catch (error) { + core.warning(`Could not update comment on PR #${pr.number}: ${error.message}`); } } - core.info(`No PR comment found with marker for ${version}`); + if (updatedCount === 0) { + core.info(`No PR comment found with marker for ${version}`); + } } catch (error) { core.warning(`Could not update PR comment: ${error.message}`); }