feat: route non-member PR reviews through warpdotdev/warp-ownership#459
Draft
vkodithala wants to merge 3 commits into
Draft
feat: route non-member PR reviews through warpdotdev/warp-ownership#459vkodithala wants to merge 3 commits into
vkodithala wants to merge 3 commits into
Conversation
For non-member PRs the review workflow now picks the human reviewer from the canonical ownership-areas list in warpdotdev/warp-ownership instead of guessing logins out of .github/STAKEHOLDERS. The cloud review agent picks a single area name, the validator enforces it against the parsed list, and the apply step maps area -> eligible owner with a uniform random pick. STAKEHOLDERS remains the deterministic last-resort fallback when the agent can't commit to one area or the org-wide GitHub App install doesn't cover the ownership repo. Flow: - Dispatch: gather_review_context reads ownership-areas/*.md via the payload installation token, inlines the prose block plus a JSON list of canonical names in the cloud prompt, and serializes the list into RunState.payload_subset. - Agent: returns 'recommended_area' (or '') alongside verdict/body/ comments. validate_review_json.py rejects unknown areas on APPROVE and any non-empty area on REJECT, gated by --ownership-areas ownership_areas.json (missing file = no ownership areas in scope). - Apply: _resolve_recommended_reviewers picks one eligible owner via random.SystemRandom.choice for the chosen area, excluding the PR author. Falls back lazily to STAKEHOLDERS only when ownership-areas resolution yields nothing. The webhook reuses the payload-keyed GitHub client for ownership-repo access; failures fail open so consumers outside the warpdotdev org keep the legacy STAKEHOLDERS behavior automatically. Co-Authored-By: Oz <oz-agent@warp.dev>
Contributor
|
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}
Contributor
There was a problem hiding this comment.
Overview
This PR moves non-member reviewer selection from agent-supplied reviewer handles to ownership-area selection, adds ownership-area parsing, threads the dispatch/apply context, and extends validation to check recommended_area.
Concerns
- The new ownership-area validator option is always populated with a default file path, so omitted and explicit
--ownership-areasusage cannot be distinguished. That can make local/non-ownership validations depend on a stale ambient file and can also let ownership-scoped dispatches silently skip the required gate when the file was not written.
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
| ) | ||
| parser.add_argument( | ||
| "--ownership-areas", | ||
| default="ownership_areas.json", |
Contributor
There was a problem hiding this comment.
--ownership-areas calls indistinguishable: local runs that intentionally omit the flag can be affected by a stale ownership_areas.json, while ownership-scoped dispatches that pass the flag still silently disable the gate if the file was not written. Default this to None and only tolerate a missing file when the flag was omitted.
For non-member PRs reviewed via the review-pull-request workflow, the
control plane now enumerates other PRs that reference the same issue
(closing keyword, manual link, or plain #N mention) and prefers reusing
a same-issue reviewer before falling back to the existing ownership-area
or STAKEHOLDERS resolution.
- Add 'Issue.timelineItems(CROSS_REFERENCED_EVENT)' GraphQL discovery
in 'oz/helpers.py' plus a paginated 'find_related_prs_for_issue'
helper that enriches each sibling PR with reviewer signals via
PullRequest.get_review_requests and PullRequest.get_reviews. Bots,
the current PR's author, and cross-repo sources are filtered out.
- Add 'build_related_pr_reviewer_candidates' which produces an
ordered, deterministic candidate list with two tiers:
- 'requested-open': human reviewers currently requested on an open
sibling PR (strongest signal).
- 'prior-review': humans who already submitted a non-PENDING review
on a sibling PR (weaker fallback signal).
- Extend ReviewContext with 'linked_issue_number', 'related_prs', and
'related_pr_reviewer_candidates'. These are populated only for
non-member, non-spec PRs that resolve to a single linked issue.
- Render a 'Related same-issue PRs' block in the dispatched review
prompt so the agent sees the same context the control plane uses.
No review.json schema change.
- Insert a same-issue reviewer-reuse step ahead of the existing
ownership-area / STAKEHOLDERS resolution. Eligibility is filtered
through the union of ownership-area owners when ownership areas are
loaded; otherwise the STAKEHOLDERS owner set is used. Ambiguous
tiers log and fall through rather than picking randomly.
Refs #456.
Co-Authored-By: Oz <oz-agent@warp.dev>
vkodithala
force-pushed
the
varoon/integrate-ownership-areas
branch
from
b52a22f to
3aacab2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Non-member PR reviews currently guess a reviewer login out of
.github/STAKEHOLDERSfrom agent free-form output. This PR moves the source of truth towarpdotdev/warp-ownership/ownership-areas/*.mdso reviewer selection is bounded by a Warp-curated allowlist, distributed evenly across owners of the matched area, and auditable as PR → area → owner.Flow
gather_review_contextreadsownership-areas/*.mdvia the payload installation token (org-wide install onwarpdotdevalready coverswarp-ownership; outside orgs fail open). It inlines the prose block and a JSON list of canonical area names into the cloud prompt and serializes the parsed list ontoRunState.payload_subset.recommended_area— exactly one area name copied verbatim, or""to defer — alongsideverdict/body/comments. The bundledvalidate_review_json.pyreadsownership_areas.json(also inlined for the agent to write) via--ownership-areas; it rejects unknown names onAPPROVEand any non-emptyrecommended_areaonREJECT.recommended_area→ eligible owners, excludes the PR author, and usesrandom.SystemRandom.choicefor uniform load-balancing across area owners. STAKEHOLDERS is loaded lazily as the deterministic last-resort fallback when the agent can't commit or ownership-areas resolution yields nothing.Reason
SystemRandom.choiceinstead of always pinging the first listed owner.--ownership-areas) so invalidrecommended_areavalues are caught beforereview.jsonis uploaded, not silently dropped at apply time.warpdotdevkeep the existing behavior automatically.Validation
python3 -m unittest tests.test_ownership tests.test_review_pr_reviewer_sampling tests.test_prompts tests.test_webhook_dispatch tests.test_builders→ 103 tests pass.validate_review_json.pyexercised manually across 6 cases (valid area / unknown area / REJECT-with-area / empty fallback / missing ownership file / REJECT-empty); each produced the expected pass/fail outcome.Artifacts
Co-Authored-By: Oz oz-agent@warp.dev