fix: route team slugs to team_reviewers API parameter and reflect failures in progress comment#423
Conversation
…lures in progress comment STAKEHOLDERS entries like `warpdotdev/oss-maintainers` are GitHub teams, not individual users. The review workflow was passing them to the `reviewers` parameter of `create_review_request`, which only accepts user logins. This caused a silent GithubException, and the progress comment still claimed the review was requested. Changes: - Add `_is_team_slug()`, `_team_slug_only()`, and `_split_reviewers()` helpers to detect and partition team slugs from user logins - Update `apply_review_result` to pass teams via `team_reviewers` and users via `reviewers` - Only report successfully requested reviewers in the progress comment - Add 12 new tests covering team slug detection, splitting, API routing, and progress comment accuracy on failure Fixes the bug observed on warpdotdev/warp#9691 where `@warpdotdev/oss-maintainers` was never actually requested. Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and posted feedback on this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR routes team stakeholder entries through GitHub's team_reviewers request parameter and avoids claiming reviewer-request success after GithubException failures.
Concerns
- The new failure path still uses the generic no-reviewer completion copy, which can claim feedback was posted even when the reviewer request failed before any review body or inline comments were created.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Co-Authored-By: Safia Abdalla <captainsafia@users.noreply.github.com> Co-Authored-By: Oz <oz-agent@warp.dev>
|
/oz-review |
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and posted feedback on this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR updates the review workflow to route GitHub team references through team_reviewers, preserve user logins in reviewers, and make the progress comment reflect whether the reviewer-request API call actually succeeded. The added tests cover slug parsing, request routing, and success/failure progress messaging.
Concerns
- No blocking correctness or security concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
There was a problem hiding this comment.
Overview
This PR updates the review workflow to route GitHub team references through team_reviewers, preserve user logins in reviewers, and make the progress comment reflect whether the reviewer-request API call actually succeeded. The added tests cover slug parsing, request routing, and success/failure progress messaging.
Concerns
- No blocking correctness or security concerns found in the changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Merge activity
|
3 participants
Problem
The review workflow in oz-for-oss fails to actually request team reviews when a
.github/STAKEHOLDERSentry references a GitHub team (e.g.@warpdotdev/oss-maintainers). The bot's progress comment claims the review was requested, but the GitHub API call silently fails because:/) are passed to thereviewersparameter, which only accepts individual user logins. Teams must go throughteam_reviewers.GithubExceptionis caught and logged, but the progress comment is updated unconditionally — always claiming success.Observed on warpdotdev/warp#9691.
Changes
core/workflows/review_pr.py_is_team_slug()— detectsorg/teamformat (presence of/)_team_slug_only()— strips theorg/prefix for the GitHub API_split_reviewers()— partitions a reviewer list into(user_logins, team_slugs)apply_review_resultto:reviewersvsteam_reviewers)tests/test_review_pr_reviewer_sampling.pyTesting
All 328 tests pass (including 12 new ones).
Conversation: https://staging.warp.dev/conversation/ef40de82-ea29-4fa3-aeab-b5ec896017d9
Run: https://oz.staging.warp.dev/runs/019de41d-84ed-77b8-930a-88afee1cc598
This PR was generated with Oz.