Skip to content

fix: allow adding required reviewers in update_reviewers - #1688

Closed
Vasilii Boldurean (boldurean) wants to merge 1 commit into
microsoft:mainfrom
boldurean:fix/pr-required-reviewers
Closed

Vasilii Boldurean (boldurean) wants to merge 1 commit into
microsoft:mainfrom
boldurean:fix/pr-required-reviewers

Conversation

@boldurean

@boldurean Vasilii Boldurean (boldurean) commented Oct 12, 2026 •

Copy link
Copy Markdown

repo_pull_request_write with action: "update_reviewers" always added reviewers as optional. There was no way to add a required reviewer, which is the normal case for teams whose branch policy expects named required reviewers.

This PR adds an optional isRequired boolean input:

  • When reviewerAction is add, it is passed through to createPullRequestReviewers as { id, isRequired } for each reviewer.
  • When omitted, the payload is { id } as before, so existing behaviour is unchanged.
  • The trimmed response now includes isRequired for each reviewer, so callers can confirm the result.

Example:

{
  "action": "update_reviewers",
  "reviewerAction": "add",
  "isRequired": true,
  "repositoryId": "<repo>",
  "pullRequestId": 123,
  "reviewerIds": ["<identity id>"]
}

GitHub issue number

None filed.

Associated Risks

Low. The new input is optional and the request payload is identical to before when it is omitted. Not yet confirmed: whether re-adding an existing optional reviewer with isRequired: true promotes them to required through the batch endpoint.

✅ PR Checklist

  • I have read the contribution guidelines
  • I have read the code of conduct guidelines
  • Title of the pull request is clear and informative.
  • 👌 Code hygiene
  • 🔭 Telemetry added, updated, or N/A
  • 📄 Documentation added, updated, or N/A
  • 🛡️ Automated tests added, or N/A

🧪 How did you test it?

  • Added a unit test that checks isRequired: true is sent for every reviewer and returned in the response.
  • Existing update_reviewers test still passes with the unchanged { id } payload.
  • jest test/src/tools/repositories.test.ts: 331 passed, 100% coverage on repositories.ts.
  • tsc, prettier --check and eslint are clean.

repo_pull_request_write update_reviewers always added reviewers as
optional. Add an optional isRequired input that is passed through to
createPullRequestReviewers. When omitted, behaviour is unchanged.
The response now includes isRequired for each reviewer.
@boldurean

Copy link
Copy Markdown
Author

Vasilii Boldurean (Vasilii Boldurean (@boldurean)) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

@danhellem

Copy link
Copy Markdown
Collaborator

We are closing this issue per contribution guidelines. Please create an Issue first and have it reviewed before creating a PR

@boldurean

Copy link
Copy Markdown
Author

We are closing this issue per contribution guidelines. Please create an Issue first and have it reviewed before creating a PR

Hi Dan Hellem (@danhellem), sorry missed that, will do so thanks

@boldurean

Copy link
Copy Markdown
Author

Dan Hellem (@danhellem) just seen the issue was already raised in discussions - #899 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants