fix: request human reviewer on bot-authored and external-contributor PRs#469
Conversation
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for 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 broadens reviewer escalation so bot-authored and external-contributor PRs ask for a human reviewer on approve, while preserving the existing non-member request-changes behavior.
Concerns
- The reviewer-selection prompt is now shown for automation/spec cases where
REJECTstill results in a comment, but the prompt continues to tell the agent thatREJECTpostsREQUEST_CHANGES. - The new gather-context tests construct files with
patch=None, butgather_review_contextformats every file patch before the_build_diff_mapsmock is reached, so those tests fail before exercising the new escalation behavior.
Verdict
Found: 0 critical, 2 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
| ownership_areas_serialized: list[dict[str, Any]] = [] | ||
| ownership_areas_loaded = False | ||
| if is_non_member: | ||
| if requires_human_reviewer: |
There was a problem hiding this comment.
REJECT will post REQUEST_CHANGES; apply_review_result only does that for is_non_member, so these runs get misleading review instructions. Parameterize the reject-event wording or use a separate prompt for reviewer-only escalation.
There was a problem hiding this comment.
I think we want to make this consistent and the desired behavior is that Oz-authored PRs never get the request changes action so they don't appear blocked.
captainsafia
left a comment
captainsafia
left a comment
There was a problem hiding this comment.
Seems sensible overall. Left some comments inline.
| ownership_areas_serialized: list[dict[str, Any]] = [] | ||
| ownership_areas_loaded = False | ||
| if is_non_member: | ||
| if requires_human_reviewer: |
There was a problem hiding this comment.
I think we want to make this consistent and the desired behavior is that Oz-authored PRs never get the request changes action so they don't appear blocked.
| is_non_member = _is_non_member_pr(pr) and not spec_only | ||
| # Bot-authored and external-contributor PRs require a human reviewer. | ||
| requires_human_reviewer = ( | ||
| is_automation_user(getattr(pr, "user", None)) or _is_non_member_pr(pr) |
There was a problem hiding this comment.
I think it's fine to do this for all bot-authored PRs but we should monitor if this becomes chaotic for the other bot-authored (dependnecy update bot) PRs in the repo.
Decouples reviewer selection from is_non_member by introducing requires_human_reviewer = is_automation_user(pr.user) or _is_non_member_pr(pr), so /oz-review escalates every Oz-authored PR (spec or implementation) and every external-contributor PR to a human reviewer. is_non_member still gates the REJECT -> REQUEST_CHANGES decision. Internal member PRs are unaffected. Co-Authored-By: Oz <oz-agent@warp.dev>
Narrow human-reviewer escalation so only the configured Oz bot (via GH_APP_SLUG) and external-contributor PRs trigger it; other automation accounts like dependabot keep their own reviewer conventions. Parameterize the reviewer-selection prompt's REJECT outcome so it matches apply_review_result: REQUEST_CHANGES for non-members, COMMENT for the Oz bot's own PRs. Co-Authored-By: Oz <oz-agent@warp.dev>
vkodithala
force-pushed
the
varoon/spec-pr-reviewer-escalation
branch
from
3594776 to
076749c
Compare
2 participants
Summary
/oz-reviewnow escalates to a human reviewer for every Oz-authored PR (spec or implementation) and every external-contributor PR, instead of silently self-approving. Previously reviewer selection was disabled for spec PRs and skipped for automation accounts, so agent-authored PRs could sit unreviewed.Changes
core/workflows/review_pr.py: introducerequires_human_reviewer = is_automation_user(pr.user) or _is_non_member_pr(pr)(author identity, not spec-vs-implementation). It gates the reviewer-selection prompt section ingather_review_contextand the reviewer-request branch inapply_review_result, and is persisted onReviewContext(with a backward-compat fallback tois_non_memberfor in-flight serialized contexts).is_non_memberis unchanged for theREJECT→REQUEST_CHANGESdecision. Prompt heading renamed "Non-Member Reviewer Selection" → "Reviewer Selection"; the agent-facing contract (recommended_area/recommended_reviewers, verdict gating, empty-string fallback) is unchanged.tests/test_review_pr_reviewer_sampling.py: cover Oz-authored spec/impl and external-contributor spec/impl escalation, the member-PR negative case, and that an Oz-authoredREJECTstays aCOMMENT.Behavior
Validation
python -m pytest tests→ 392 passed.Plan: https://staging.warp.dev/drive/notebook/0E2GIBz7PG49b3XXhB5ahs
Conversation: https://staging.warp.dev/conversation/6e6abd3f-0b79-40a6-8128-12862b792588
Co-Authored-By: Oz oz-agent@warp.dev