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

feat: clear pending review requests and reviewer-assignees when a PR is converted to draft

Open
#2,542 8 comments 0 reactions 0 assignees View on GitHub
approved skill: intermediate
Dominant language
Python
Stars
63
Forks
298
Avg merge
3d 18h
Merged PRs (30d)
38

Description

## Description

When a PR is converted to draft, its pending review requests and the assignees the reviewer-to-assignee bot added for them are left in place. Nothing in the repo reacts to `converted_to_draft`.

`.github/workflows/on-review.yml` currently has exactly two flows:

- **Add** — `pull_request_target: review_requested` → `addReviewersAsAssignees` assigns each requested individual reviewer.
- **Remove** — `workflow_run` from `Bot - Capture PR Review` (fires on `pull_request_review: submitted`) → `removeReviewerFromAssignees` removes that one reviewer.

So the only way a reviewer-assignee is ever cleared is by submitting a review. If the author converts the PR to draft while requests are pending, the reviewers stay requested and stay assigned indefinitely — the PR sits in their review queue and their "assigned to me" list even though it is explicitly not ready for review.

## Files containing the relevant logic

- `.github/workflows/on-review.yml`
- `.github/scripts/bot-pr-add-reviewers-as-assignees.js`
- `.github/scripts/__tests__/jest/bot-pr-add-reviewers-as-assignees.test.js`

## Expected behavior

When a PR is converted to draft:

1. Pending review requests (individuals and teams) are withdrawn via `pulls.removeRequestedReviewers`.
2. The individual users who had pending requests are also removed from the PR's assignees (the bot added them there when the request was made).

When the PR is later marked ready for review, **nothing is restored automatically — by design**:

- GitHub auto-re-requests code owners when a draft is marked ready, and `.github/CODEOWNERS` in this repo is entirely team-based, so baseline team review coverage returns on its own.
- Manually-requested individual reviewers must be deliberately re-requested by the author or a triager. Each fresh `review_requested` event then flows through the existing add-assignee bot unchanged.

This keeps the whole mechanism stateless: no snapshot of "who was requested before drafting" is stored anywhere.

## Actual behavior

Converting a PR to draft leaves review requests and reviewer-assignees untouched. They are only cleared if the reviewer submits a review on the draft PR, or if someone cleans them up by hand.

## Why is this important

Draft is the author's signal that the PR is not ready for review. Leaving requests and assignees in place:

- keeps the PR in reviewers' review-requested and assigned queues while no review is expected;
- makes assignee-based dashboards and any future routing read stale state;
- means a reviewer can spend time reviewing a PR the author is actively rewriting.

## Steps to reproduce

1. Open a PR and request a review from an individual user. The bot assigns them.
2. Before the reviewer submits anything, convert the PR to draft.
3. Observe that no workflow runs, the user is still a requested reviewer, and they are still an assignee.

## Suggested implementation

Add a job to `.github/workflows/on-review.yml` on `pull_request_target: [converted_to_draft]` that calls a new named export (e.g. `clearReviewStateOnDraft`) in `bot-pr-add-reviewers-as-assignees.js`:

1. Fetch the live PR (do not trust the event payload snapshot — see #2230 for why).
2. Collect `requested_reviewers` (individuals) and `requested_teams`.
3. Call `pulls.removeRequestedReviewers` with both lists.
4. Remove from assignees **only** the intersection of current assignees and the individual requested reviewers. The PR author and anyone manually assigned must never be touched.
5. No handler on `ready_for_review` — restoration is intentionally manual/CODEOWNERS-driven, per Expected behavior.

Keep the existing conventions from the current script: validate logins against `VALID_LOGIN_REGEX`, skip bot actors, log via the shared logger, tolerate 403s without failing the run, and use a `concurrency` group keyed on the PR number.

Add Jest coverage in `bot-pr-add-reviewers-as-assignees.test.js` for at least:

- pending individual + team requests → both un-requested, individual removed from assignees;
- requested reviewer who is not an assignee → request removed, no assignee call;
- author is an assignee → author untouched;
- no pending requests → no-op, no API mutation calls.

### Additional context

Enhancement to #2391 (reviewer-to-assignee bot with removal support) — the bot works as designed; draft conversion was simply not part of the original design. Sibling issue: #2541 (clearing review-queue labels on draft conversion), tracked separately since it lives in the review-sync label bot rather than this workflow.

Contributor guide

Open the contributing guide

Research direction

Start with .github/workflows/on-review.yml and .github/scripts/bot-pr-add-reviewers-as-assignees.js, then run the existing Jest suite in .github/scripts/__tests__/jest/bot-pr-add-reviewers-as-assignees.test.js. Trace the current reviewer add/remove flows and GitHub API calls. Done means draft conversion clears pending individual and team requests, removes only matching reviewer-assignees, preserves authors and manual assignees, and performs no calls when there are no requests.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, javascript
Domain
ci-cd, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.