hiero-ledger / hiero-ledger/hiero-sdk-python

Bug: Assignment bots should refetch issue state before final assignment decision

Open
#2,230 14 comments 0 reactions 1 assignee Assigned to @chaitanyamedidar View on GitHub
Good First Issue
Dominant language
Python
Stars
63
Forks
298
Avg merge
3d 18h
Merged PRs (30d)
38

Description

## Description

The GFI and beginner `/assign` bots appear to use `context.payload.issue` from the original `issue_comment.created` webhook payload for the final assignment checks.

Because the webhook payload is a snapshot, a queued workflow run may not reflect issue state changes made by an earlier workflow run. For example, a later `/assign` run may still see `payload.issue.assignees` as empty even if the issue has already been assigned by the time that run executes.

This means the assignment decision may be based on stale issue state rather than the latest GitHub issue state.

## Files containing the relevant logic

- `.github/scripts/bot-gfi-assign-on-comment.js`
- `.github/scripts/bot-beginner-assign-on-comment.js`

Related workflows:

- `.github/workflows/bot-gfi-assign-on-comment.yml`
- `.github/workflows/bot-beginner-assign-on-comment.yml`

## Expected behavior

Before making the final assignment decision, the bot should verify the current issue state from the GitHub API, especially:

- current assignees
- current labels
- current issue state

If the issue is already assigned, the bot should not call `addAssignees`.

## Actual behavior

The current scripts appear to check `issue.assignees` from the webhook payload before calling `github.rest.issues.addAssignees(...)`.

In a close timing assignment scenario, this could lead to duplicate assignment attempts or follow-up automation running from stale assumptions.

## Why is this important

Both workflows use concurrency, which helps serialize workflow execution. However, concurrency does not refresh the original webhook payload.

So a later queued job can run after an earlier job has assigned the issue, while still seeing the original payload state:

```js
context.payload.issue.assignees = []
```
Refetching the issue before the final assignment decision would make the assignment bots safer against stale event data.

## Steps to reproduce

### Live timing scenario

1. Create or use an unassigned issue labeled `Good First Issue` or `skill: beginner`.
2. Have two different users comment `/assign` close together before the first workflow finishes.
3. Let the first workflow assign the issue.
4. The second queued workflow may still have the original `issue_comment.created` payload where `payload.issue.assignees` is empty.
5. The second workflow can proceed as if the issue is still unassigned unless it refetches the current issue state.

### Local mocked reproduction

1. Create a mocked `issue_comment.created` payload where `payload.issue.assignees` is empty.
2. Mock the current GitHub issue state so `github.rest.issues.get(...)` would return the same issue with an existing assignee.
3. Run the current GFI or beginner assignment handler.
4. Observe that the handler still calls `addAssignees`.

### Additional context

I verified this locally with a mocked event replay.

The reproduction test creates a stale `issue_comment.created` payload where `payload.issue.assignees` is empty, while the mocked current GitHub issue state has an existing assignee. When run against the current scripts, both the GFI and beginner assignment bots still call `addAssignees`.

This test is not intended as the final implementation, only as a minimal reproduction.

Command used:

```powershell
node --test .github/scripts/bot-assignment-fresh-issue.test.js
```
Observed result:
```
Expected values to be strictly equal:
1 !== 0
```

This means addAssignees was called once when the test expected it not to be called.

Minimal local reproduction test

```js
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/);
});
});
```

Run with:

```powershell
node --test .github/scripts/bot-assignment-fresh-issue.test.js
```

Observed on current code:

```text
Expected values to be strictly equal:

1 !== 0
```

This means `addAssignees` was called once even though the mocked fresh issue state already had an assignee.

### Hedera network

_No response_

### Version

main branch, commit c9c2d27

### Operating system

Windows

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.