scripts/pr_review.py's wait requests a Copilot review before its first poll, and that request is a GraphQL mutation against the repository named on the command line. It does not pass through in_scope(), so the in-process owner check that comment and reply refuse on does not cover it.
The behavior
At main (6e859734ddd159a4718a420e5762ae1db73722e2), in_scope() has exactly two call sites, one in comment_on_pr and one in reply_to_thread. The request path has none:
# scripts/pr_review.py, request_copilot_review()
gh_graphql(M_REQUEST_REVIEWS, pr=pr_node_id, bot=bot_id)# scripts/pr_review.py, the wait loop's auto-request
if not done and not answer and not drift and not reviewer_requested(pr):
print(f"auto-request: {request_copilot_review(pr['id'], copilot_bot_id(history))}")M_REQUEST_REVIEWS is a requestReviews mutation, so wait on a repository under another owner issues a state-changing call under the logged-in identity, on a pull request in that repository. The pull request node id is read live from that same repository in the same run, so the id rule is satisfied and the target is exactly the one named by --repo. What is missing is the owner comparison, not a live id.
Reaching it takes no unusual invocation. python3 scripts/pr_review.py wait 1 --repo globex/other-thing is the ordinary shape, and the refusal that would stop the same target on reply never fires.
Why it matters
Requesting a review is externally visible on the target pull request and attributable to the account. It is the class of write the owner check was added for, and it is the one write path in this script that the check does not reach.
scripts/README.md also states the check more broadly than the code implements it:
The owner check is also enforced in-process.
That sentence sits in the paragraph about what the script trades away by performing mutations the gh-write-guard hook cannot read, which is where a reader goes to find out what still protects them.
Suggested fix
Run the same in_scope() refusal on the wait path before the auto-request, exiting 64 and writing nothing, so all three write paths refuse identically. Then either narrow the README sentence to the subcommands it describes or leave it as written once it is true.
Worth deciding whether the refusal should stop wait outright or only suppress its auto-request and keep polling, since wait is otherwise read-only and a cross-owner read is allowed. Suppressing the request and continuing to poll keeps the read useful while removing the write, and stopping outright is the simpler rule to state.
Scope
Found while fixing #1557, whose stated scope was the refusal message wording only, so this was left out of that pull request rather than folded into it.
scripts/pr_review.py'swaitrequests a Copilot review before its first poll, and that request is a GraphQL mutation against the repository named on the command line. It does not pass throughin_scope(), so the in-process owner check thatcommentandreplyrefuse on does not cover it.The behavior
At
main(6e859734ddd159a4718a420e5762ae1db73722e2),in_scope()has exactly two call sites, one incomment_on_prand one inreply_to_thread. The request path has none:M_REQUEST_REVIEWSis arequestReviewsmutation, sowaiton a repository under another owner issues a state-changing call under the logged-in identity, on a pull request in that repository. The pull request node id is read live from that same repository in the same run, so the id rule is satisfied and the target is exactly the one named by--repo. What is missing is the owner comparison, not a live id.Reaching it takes no unusual invocation.
python3 scripts/pr_review.py wait 1 --repo globex/other-thingis the ordinary shape, and the refusal that would stop the same target onreplynever fires.Why it matters
Requesting a review is externally visible on the target pull request and attributable to the account. It is the class of write the owner check was added for, and it is the one write path in this script that the check does not reach.
scripts/README.mdalso states the check more broadly than the code implements it:That sentence sits in the paragraph about what the script trades away by performing mutations the
gh-write-guardhook cannot read, which is where a reader goes to find out what still protects them.Suggested fix
Run the same
in_scope()refusal on thewaitpath before the auto-request, exiting64and writing nothing, so all three write paths refuse identically. Then either narrow the README sentence to the subcommands it describes or leave it as written once it is true.Worth deciding whether the refusal should stop
waitoutright or only suppress its auto-request and keep polling, sincewaitis otherwise read-only and a cross-owner read is allowed. Suppressing the request and continuing to poll keeps the read useful while removing the write, and stopping outright is the simpler rule to state.Scope
Found while fixing #1557, whose stated scope was the refusal message wording only, so this was left out of that pull request rather than folded into it.