From c71e516352cb41541c1d433c5723a604b7a32cf7 Mon Sep 17 00:00:00 2001 From: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> Date: Wed, 6 May 2026 01:28:28 +0530 Subject: [PATCH 1/4] fix: refetch issue state before assignment Signed-off-by: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> --- .../scripts/bot-beginner-assign-on-comment.js | 62 ++++++++++++++++++- .github/scripts/bot-gfi-assign-on-comment.js | 53 +++++++++++++++- 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/.github/scripts/bot-beginner-assign-on-comment.js b/.github/scripts/bot-beginner-assign-on-comment.js index 866be76c3..7f5556cac 100644 --- a/.github/scripts/bot-beginner-assign-on-comment.js +++ b/.github/scripts/bot-beginner-assign-on-comment.js @@ -96,6 +96,21 @@ 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; +} + module.exports = async ({ github, context }) => { try { const { payload } = context; @@ -116,8 +131,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; @@ -325,7 +339,49 @@ 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.assignees && currentIssue.assignees.length > 0) { + try{ + const currentAssignee = currentIssue.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).`, + }); + } 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..660f735e2 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,46 @@ 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.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 +458,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) { From b7e4bef4996c8b7bf037b03659351ac2b151994e Mon Sep 17 00:00:00 2001 From: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> Date: Wed, 6 May 2026 20:38:28 +0530 Subject: [PATCH 2/4] Added assignment issue test Signed-off-by: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> --- .../bot-assignment-fresh-issue.test.js | 101 ++++++++++++++++++ .../scripts/bot-beginner-assign-on-comment.js | 12 ++- 2 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 .github/scripts/bot-assignment-fresh-issue.test.js 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..7f82f6f98 --- /dev/null +++ b/.github/scripts/bot-assignment-fresh-issue.test.js @@ -0,0 +1,101 @@ +// 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 = [], freshAssignees = [] }) { + return { + repo: { + owner: 'hiero-ledger', + repo: 'hiero-sdk-python', + }, + payload: { + repository: { + owner: { login: 'hiero-ledger' }, + name: 'hiero-sdk-python', + }, + issue: { + number: 123, + labels: [{ name: labelName }], + assignees: payloadAssignees, + }, + comment: { + body: '/assign', + user: { + login: 'new-contributor', + type: 'User', + }, + }, + }, + freshIssue: { + number: 123, + title: 'Example issue', + labels: [{ name: labelName }], + assignees: freshAssignees, + }, + }; +} + +function createGithubMock(context) { + const calls = { + comments: [], + assignees: [], + }; + + const github = { + rest: { + issues: { + get: async () => ({ 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('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/); + }); +}); diff --git a/.github/scripts/bot-beginner-assign-on-comment.js b/.github/scripts/bot-beginner-assign-on-comment.js index 7f5556cac..8d9df7ff2 100644 --- a/.github/scripts/bot-beginner-assign-on-comment.js +++ b/.github/scripts/bot-beginner-assign-on-comment.js @@ -111,6 +111,12 @@ async function getFreshIssue(github, owner, repo, 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; @@ -272,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:", { @@ -366,12 +371,11 @@ Please try a GFI first, then come back — we’ll be happy to assign this! 😊 if (currentIssue.assignees && currentIssue.assignees.length > 0) { try{ - const currentAssignee = currentIssue.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: currentIssue, repo }), }); } catch (error) { console.error("[Beginner Bot] Failed to post already-assigned message:", { From ae81c8120acf32f6d50b4c31d8ba2961bba18220 Mon Sep 17 00:00:00 2001 From: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> Date: Fri, 8 May 2026 01:14:49 +0530 Subject: [PATCH 3/4] test: cover assignment bot fresh issue defensive paths Signed-off-by: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> --- .../bot-assignment-fresh-issue.test.js | 75 +++++++++++++++++-- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/.github/scripts/bot-assignment-fresh-issue.test.js b/.github/scripts/bot-assignment-fresh-issue.test.js index 7f82f6f98..4f6e64aed 100644 --- a/.github/scripts/bot-assignment-fresh-issue.test.js +++ b/.github/scripts/bot-assignment-fresh-issue.test.js @@ -6,7 +6,9 @@ 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 = [], freshAssignees = [] }) { +function createContext({ labelName, payloadAssignees = [], freshLabels, freshAssignees = [] }) { + const labels = [{ name: labelName }]; + return { repo: { owner: 'hiero-ledger', @@ -19,7 +21,7 @@ function createContext({ labelName, payloadAssignees = [], freshAssignees = [] } }, issue: { number: 123, - labels: [{ name: labelName }], + labels, assignees: payloadAssignees, }, comment: { @@ -33,13 +35,13 @@ function createContext({ labelName, payloadAssignees = [], freshAssignees = [] } freshIssue: { number: 123, title: 'Example issue', - labels: [{ name: labelName }], + labels: freshLabels ?? labels, assignees: freshAssignees, }, }; } -function createGithubMock(context) { +function createGithubMock(context, { freshIssueError } = {}) { const calls = { comments: [], assignees: [], @@ -48,7 +50,12 @@ function createGithubMock(context) { const github = { rest: { issues: { - get: async () => ({ data: context.freshIssue }), + get: async () => { + if (freshIssueError) { + throw freshIssueError; + } + return { data: context.freshIssue }; + }, createComment: async (params) => { calls.comments.push(params); return { data: {} }; @@ -83,6 +90,35 @@ describe('assignment bots use fresh issue state before assigning', () => { 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('beginner bot does not assign when webhook payload is stale but issue is now assigned', async () => { const context = createContext({ labelName: 'skill: beginner', @@ -98,4 +134,33 @@ describe('assignment bots use fresh issue state before assigning', () => { 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); + }); }); From 2ee160bc4e31811e30deef55c06cc65d0d3fec2e Mon Sep 17 00:00:00 2001 From: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> Date: Fri, 8 May 2026 01:26:19 +0530 Subject: [PATCH 4/4] test: cover closed issue assignment guard Signed-off-by: chaitanyamedidar <175808937+chaitanyamedidar@users.noreply.github.com> --- .../bot-assignment-fresh-issue.test.js | 31 ++++++++++++++++++- .../scripts/bot-beginner-assign-on-comment.js | 5 +++ .github/scripts/bot-gfi-assign-on-comment.js | 8 +++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/.github/scripts/bot-assignment-fresh-issue.test.js b/.github/scripts/bot-assignment-fresh-issue.test.js index 4f6e64aed..1a2848cac 100644 --- a/.github/scripts/bot-assignment-fresh-issue.test.js +++ b/.github/scripts/bot-assignment-fresh-issue.test.js @@ -6,7 +6,7 @@ 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 = [] }) { +function createContext({ labelName, payloadAssignees = [], freshLabels, freshAssignees = [], freshState = 'open' }) { const labels = [{ name: labelName }]; return { @@ -35,6 +35,7 @@ function createContext({ labelName, payloadAssignees = [], freshLabels, freshAss freshIssue: { number: 123, title: 'Example issue', + state: freshState, labels: freshLabels ?? labels, assignees: freshAssignees, }, @@ -119,6 +120,20 @@ describe('assignment bots use fresh issue state before assigning', () => { 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', @@ -163,4 +178,18 @@ describe('assignment bots use fresh issue state before assigning', () => { 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 8d9df7ff2..6dd2dadb4 100644 --- a/.github/scripts/bot-beginner-assign-on-comment.js +++ b/.github/scripts/bot-beginner-assign-on-comment.js @@ -369,6 +369,11 @@ Please try a GFI first, then come back — we’ll be happy to assign this! 😊 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({ diff --git a/.github/scripts/bot-gfi-assign-on-comment.js b/.github/scripts/bot-gfi-assign-on-comment.js index 660f735e2..533f67bec 100644 --- a/.github/scripts/bot-gfi-assign-on-comment.js +++ b/.github/scripts/bot-gfi-assign-on-comment.js @@ -398,6 +398,14 @@ module.exports = async ({ github, context }) => { 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');