diff --git a/.github/scripts/bot-assignment-fresh-issue.test.js b/.github/scripts/bot-assignment-fresh-issue.test.js new file mode 100644 index 000000000..1a2848cac --- /dev/null +++ b/.github/scripts/bot-assignment-fresh-issue.test.js @@ -0,0 +1,195 @@ +// Regression coverage for stale issue_comment payloads: the bots must refetch issue state before assigning. + +const { describe, it } = require('node:test'); +const assert = require('node:assert/strict'); + +const runGfiAssignBot = require('./bot-gfi-assign-on-comment.js'); +const runBeginnerAssignBot = require('./bot-beginner-assign-on-comment.js'); + +function createContext({ labelName, payloadAssignees = [], freshLabels, freshAssignees = [], freshState = 'open' }) { + const labels = [{ name: labelName }]; + + return { + repo: { + owner: 'hiero-ledger', + repo: 'hiero-sdk-python', + }, + payload: { + repository: { + owner: { login: 'hiero-ledger' }, + name: 'hiero-sdk-python', + }, + issue: { + number: 123, + labels, + assignees: payloadAssignees, + }, + comment: { + body: '/assign', + user: { + login: 'new-contributor', + type: 'User', + }, + }, + }, + freshIssue: { + number: 123, + title: 'Example issue', + state: freshState, + labels: freshLabels ?? labels, + assignees: freshAssignees, + }, + }; +} + +function createGithubMock(context, { freshIssueError } = {}) { + const calls = { + comments: [], + assignees: [], + }; + + const github = { + rest: { + issues: { + get: async () => { + if (freshIssueError) { + throw freshIssueError; + } + return { data: context.freshIssue }; + }, + createComment: async (params) => { + calls.comments.push(params); + return { data: {} }; + }, + addAssignees: async (params) => { + calls.assignees.push(params); + return { data: {} }; + }, + }, + }, + paginate: async () => [], + graphql: async () => ({ search: { issueCount: 1 } }), + }; + + return { github, calls }; +} + +describe('assignment bots use fresh issue state before assigning', () => { + it('GFI bot does not assign when webhook payload is stale but issue is now assigned', async () => { + const context = createContext({ + labelName: 'Good First Issue', + payloadAssignees: [], + freshAssignees: [{ login: 'current-assignee' }], + }); + const { github, calls } = createGithubMock(context); + + await runGfiAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 1); + assert.match(calls.comments[0].body, /already assigned/i); + assert.match(calls.comments[0].body, /@current-assignee/); + }); + + it('GFI bot exits safely when fresh issue fetch fails before assignment', async () => { + const context = createContext({ + labelName: 'Good First Issue', + payloadAssignees: [], + }); + const { github, calls } = createGithubMock(context, { + freshIssueError: Object.assign(new Error('GitHub API unavailable'), { status: 503 }), + }); + + await runGfiAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 0); + }); + + it('GFI bot exits when the fresh issue is no longer a Good First Issue', async () => { + const context = createContext({ + labelName: 'Good First Issue', + payloadAssignees: [], + freshLabels: [{ name: 'help wanted' }], + }); + const { github, calls } = createGithubMock(context); + + await runGfiAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 0); + }); + + it('GFI bot exits when the fresh issue is closed before assignment', async () => { + const context = createContext({ + labelName: 'Good First Issue', + payloadAssignees: [], + freshState: 'closed', + }); + const { github, calls } = createGithubMock(context); + + await runGfiAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 0); + }); + + it('beginner bot does not assign when webhook payload is stale but issue is now assigned', async () => { + const context = createContext({ + labelName: 'skill: beginner', + payloadAssignees: [], + freshAssignees: [{ login: 'current-assignee' }], + }); + const { github, calls } = createGithubMock(context); + + await runBeginnerAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 1); + assert.match(calls.comments[0].body, /already assigned/i); + assert.match(calls.comments[0].body, /@current-assignee/); + }); + + it('beginner bot exits safely when fresh issue fetch fails before assignment', async () => { + const context = createContext({ + labelName: 'skill: beginner', + payloadAssignees: [], + }); + const { github, calls } = createGithubMock(context, { + freshIssueError: Object.assign(new Error('GitHub API unavailable'), { status: 503 }), + }); + + await runBeginnerAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 0); + }); + + it('beginner bot exits when the fresh issue no longer has the beginner label', async () => { + const context = createContext({ + labelName: 'skill: beginner', + payloadAssignees: [], + freshLabels: [{ name: 'help wanted' }], + }); + const { github, calls } = createGithubMock(context); + + await runBeginnerAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 0); + }); + + it('beginner bot exits when the fresh issue is closed before assignment', async () => { + const context = createContext({ + labelName: 'skill: beginner', + payloadAssignees: [], + freshState: 'closed', + }); + const { github, calls } = createGithubMock(context); + + await runBeginnerAssignBot({ github, context }); + + assert.equal(calls.assignees.length, 0); + assert.equal(calls.comments.length, 0); + }); +}); diff --git a/.github/scripts/bot-beginner-assign-on-comment.js b/.github/scripts/bot-beginner-assign-on-comment.js index 866be76c3..6dd2dadb4 100644 --- a/.github/scripts/bot-beginner-assign-on-comment.js +++ b/.github/scripts/bot-beginner-assign-on-comment.js @@ -96,6 +96,27 @@ async function countCompletedGfiIssues(github, owner, repo, username) { return result?.search?.issueCount ?? 0; } +function issueHasBeginnerLabel(issue) { + const beginnerLabel = BEGINNER_LABEL.toLowerCase(); + return Array.isArray(issue?.labels) && issue.labels.some((label) => label.name?.toLowerCase() === beginnerLabel); +} + +async function getFreshIssue(github, owner, repo, issueNumber) { + const { data } = await github.rest.issues.get({ + owner, + repo, + issue_number: issueNumber, + }); + + return data; +} + +function buildAlreadyAssignedComment({ commenter, issue, repo }) { + const currentAssignee = issue.assignees?.[0]?.login ?? "another contributor"; + + return `👋 Hi @${commenter}, thanks for your interest! This issue is already assigned to @${currentAssignee}, but we'd love your help on another one. You can find more "${BEGINNER_LABEL}" issues [here](https://github.com/${repo.owner.login}/${repo.name}/issues?q=is%3Aissue+is%3Aopen+label%3A%22${encodeURIComponent(BEGINNER_LABEL)}%22+no%3Aassignee).`; +} + module.exports = async ({ github, context }) => { try { const { payload } = context; @@ -116,8 +137,7 @@ module.exports = async ({ github, context }) => { } // 2. Label Check (Fix 2: Defensive Check) - const beginnerLabel = BEGINNER_LABEL.toLowerCase(); - const hasBeginnerLabel = Array.isArray(issue.labels) && issue.labels.some((label) => label.name?.toLowerCase() === beginnerLabel); + const hasBeginnerLabel = issueHasBeginnerLabel(issue); if (!hasBeginnerLabel) { console.log(`[Beginner Bot] Issue #${issue.number} does not have '${BEGINNER_LABEL}' label. Exiting.`); return; @@ -258,12 +278,11 @@ Please try a GFI first, then come back — we’ll be happy to assign this! 😊 // --- ASSIGNMENT LOGIC --- if (issue.assignees && issue.assignees.length > 0) { try{ - const currentAssignee = issue.assignees[0]?.login ?? "another contributor"; await github.rest.issues.createComment({ owner: repo.owner.login, repo: repo.name, issue_number: issue.number, - body: `👋 Hi @${commenter}, thanks for your interest! This issue is already assigned to @${currentAssignee}, but we'd love your help on another one. You can find more "${BEGINNER_LABEL}" issues [here](https://github.com/${repo.owner.login}/${repo.name}/issues?q=is%3Aissue+is%3Aopen+label%3A%22${encodeURIComponent(BEGINNER_LABEL)}%22+no%3Aassignee).`, + body: buildAlreadyAssignedComment({ commenter, issue, repo }), }); } catch (error) { console.error("[Beginner Bot] Failed to post already-assigned message:", { @@ -325,7 +344,53 @@ Please try a GFI first, then come back — we’ll be happy to assign this! 😊 return; } - console.log(`[Beginner Bot] Assigning issue #${issue.number} to @${commenter}...`); + console.log(`[Beginner Bot] Checking current issue state before assigning #${issue.number} to @${commenter}...`); + + let currentIssue; + try { + currentIssue = await getFreshIssue( + github, + repo.owner.login, + repo.name, + issue.number + ); + } catch (error) { + console.error("[Beginner Bot] Failed to fetch current issue state before assignment:", { + issue: issue.number, + commenter, + message: error.message, + status: error.status, + }); + return; + } + + if (!issueHasBeginnerLabel(currentIssue)) { + console.log(`[Beginner Bot] Issue #${issue.number} no longer has '${BEGINNER_LABEL}' label. Exiting.`); + return; + } + + if (currentIssue.state !== "open") { + console.log(`[Beginner Bot] Issue #${issue.number} is '${currentIssue.state}'. Exiting.`); + return; + } + + if (currentIssue.assignees && currentIssue.assignees.length > 0) { + try{ + await github.rest.issues.createComment({ + owner: repo.owner.login, + repo: repo.name, + issue_number: issue.number, + body: buildAlreadyAssignedComment({ commenter, issue: currentIssue, repo }), + }); + } catch (error) { + console.error("[Beginner Bot] Failed to post already-assigned message:", { + issue: issue.number, + commenter, + message: error.message, + }); + } + return; + } // Fix 4: Granular Try/Catch for Assign API try { diff --git a/.github/scripts/bot-gfi-assign-on-comment.js b/.github/scripts/bot-gfi-assign-on-comment.js index 95328e603..533f67bec 100644 --- a/.github/scripts/bot-gfi-assign-on-comment.js +++ b/.github/scripts/bot-gfi-assign-on-comment.js @@ -86,6 +86,16 @@ function getCurrentAssigneeMention(issue) { return login ? `@${login}` : 'someone'; } +async function getFreshIssue({ github, owner, repo, issueNumber }) { + const { data } = await github.rest.issues.get({ + owner, + repo, + issue_number: issueNumber, + }); + + return data; +} + /** * Builds a comment explaining that the issue is already assigned. * Requester username is passed from main @@ -361,7 +371,54 @@ module.exports = async ({ github, context }) => { return; } - console.log('[gfi-assign] Assigning issue to requester'); + console.log('[gfi-assign] Checking current issue state before assignment'); + + let currentIssue; + try { + currentIssue = await getFreshIssue({ + github, + owner, + repo, + issueNumber, + }); + } catch (error) { + console.error('[gfi-assign] Failed to fetch current issue state before assignment:', { + message: error.message, + status: error.status, + owner, + repo, + issueNumber, + requesterUsername, + }); + return; + } + + if (!issueIsGoodFirstIssue(currentIssue)) { + console.log('[gfi-assign] Exit: current issue state is no longer a Good First Issue'); + return; + } + + if (currentIssue.state !== 'open') { + console.log('[gfi-assign] Exit: current issue state is not open', { + issueNumber, + state: currentIssue.state, + }); + return; + } + + if (currentIssue.assignees?.length > 0) { + console.log('[gfi-assign] Exit: current issue state is already assigned'); + + await github.rest.issues.createComment({ + owner, + repo, + issue_number: issueNumber, + body: commentAlreadyAssigned(requesterUsername, currentIssue), + }); + + console.log('[gfi-assign] Posted already-assigned comment from current issue state'); + return; + } // All validations passed and user has requested assignment on a GFI // Assign the issue to the requester @@ -409,7 +466,7 @@ module.exports = async ({ github, context }) => { if (planExists) { console.log('[gfi-assign] CodeRabbit plan already exists, skipping'); } else { - await triggerCodeRabbitPlan(github, owner, repo, issue); + await triggerCodeRabbitPlan(github, owner, repo, currentIssue); console.log('[gfi-assign] CodeRabbit plan chained successfully'); } } catch (error) {