Skip to content

Name the Directory the Scope Refusal Probed, Not the Checkout - #1563

Merged
ptr727 merged 1 commit into
developfrom
fix/scope-refusal-names-script-dir
Sep 12, 2026
Merged

Name the Directory the Scope Refusal Probed, Not the Checkout#1563
ptr727 merged 1 commit into
developfrom
fix/scope-refusal-names-script-dir

Conversation

@ptr727

@ptr727 ptr727 commented Sep 12, 2026

Copy link
Copy Markdown
Owner

scripts/pr_review.py resolves the owner a write may stay within from the script's own directory, which is deliberate: the helper is hub-hosted rather than carried, so it is reached from a hub checkout while the repository being answered is named on the command line, and the working directory says nothing about who owns either. Its refusals called that directory "this checkout", which an operator reads as the working directory. A copy of the script outside any checkout therefore produced a refusal the operator could disprove on the spot with git remote get-url origin in the tree they were standing in, with nothing on screen explaining the disagreement.

Message only, per the issue. The anchoring, the refusal, and the refusal writing nothing are unchanged.

What changed

  • Both refusals name the directory that was actually probed, and describe the probe rather than claiming which repository encloses that directory.
  • The unreadable-owner refusal names a remedy, scoped to the cause it actually fixes rather than stated as the diagnosis.
  • HERE is a module constant, the single reader for the probe and for the messages that name it, matching the shape six sibling scripts already use.
  • reply's exit-64 gloss and origin_owner's docstring each said less than the code does, and scripts/README.md carried the same "this checkout" claim. All three now agree with the messages.

Before, from a copy outside any checkout:

status=OUT_OF_SCOPE nothing was written: this checkout has no readable `origin`, so the owner a
write would stay within cannot be established, and an unverified scope is not a scope

After, against the same reproduction:

status=OUT_OF_SCOPE nothing was written: this script got no owner from `git remote get-url
origin` against the directory it sits in, /tmp/scratch, so the owner a write would stay within
cannot be established, and an unverified scope is not a scope. That directory is this script's
own rather than the working directory, so a readable `origin` in the working directory says
nothing about this, and the remedy for a copy of this script sitting outside a checkout is to
run the copy that lives inside a hub checkout instead of this one

Verification

Reproduced the issue's own scenario against the fix, a copy of the script outside any checkout and a cross-owner target from a fleet checkout, and read both messages back. 322 cases in the module and 1403 under discovery, plus prose_lint, repo_gate --check eol, spec/validate.py, ruff format --check and ruff check, all clean.

The new probe case was proved by mutation rather than by passing: against a copy whose probe reads Path.cwd() instead of HERE, test_the_probe_reads_the_directory_the_refusal_names fails. Without that, the assertion passed on a path coincidence whenever the working directory happened to equal the anchor.

Five local review rounds ran over this diff. Each message clause was checked against every state that reaches its branch, meaning a missing git, a timeout, no remote named origin, a directory under no repository, an origin URL the owner regex cannot parse, and an inherited GIT_DIR. Three earlier wordings were rejected for failing one of those: one asserted the probe had executed when a missing git means it never did, one named a remedy that read as an instruction to change directory and so reproduced the same refusal, and one claimed a repository encloses the probed directory, which an inherited GIT_DIR falsifies.

Raised separately rather than folded in

Two real defects surfaced during review that are code changes rather than wording, so they are filed instead of carried here:

Addresses #1557. The closing reference goes on the promotion pull request, since a develop-targeted one never registers.

`pr_review.py` reads the owner a write may stay within from the script's own
directory, which is deliberate, since the helper is reached from a hub checkout
while the repository being answered is named on the command line. The refusal
called that directory "this checkout", which an operator reads as the working
directory, so a copy of the script outside any checkout produced a refusal the
operator could immediately disprove with `git remote get-url origin` in the tree
they were standing in, with nothing on screen explaining the disagreement.

Both refusals now name the directory that was actually probed, and the
unreadable-origin one names the remedy as well, so the message is
self-diagnosing. A `script_dir()` helper is the single reader for the probe and
for the messages that name it, so the two can never drift apart.

The anchoring, the refusal, and the refusal writing nothing are unchanged.
Message only.

Fixes #1557
Copilot AI lite review requested due to automatic review settings September 12, 2026 04:02
@coderabbitai

coderabbitai Bot commented Sep 12, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 88fe1492-70a4-4647-b653-8e9a3f6838b4


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The changes are scoped to messaging/documentation plus targeted regression tests, and the updated wording matches the actual control flow (scope refusal occurs before any GitHub reads).

Pull request overview

This PR updates scripts/pr_review.py scope-refusal messaging and supporting documentation/tests so the refusal names the directory actually probed (the script's own directory) rather than ambiguously referring to “this checkout”, which can be misread as the operator’s working directory.

Changes:

  • Introduces a HERE module constant and uses it consistently for the git -C ... remote get-url origin probe and for refusal messages that name the probed directory.
  • Updates the scope refusal strings (including the exit-64 gloss) to describe the probe and provide a concrete remedy for the “script copied outside a checkout” scenario.
  • Adds regression tests ensuring the probe and refusal text stay aligned, and updates scripts/README.md to match the corrected behavior.
File summaries
File Description
scripts/pr_review.py Unifies scope anchor via HERE and updates exit-64/refusal wording to name the probed directory and remedy.
scripts/README.md Updates the reply/comment exit-64 documentation to match the clarified probe behavior and refusal wording.
scripts/tests/test_pr_review.py Adds tests that assert the probe uses HERE and the refusal messages name the probed directory and avoid “this checkout”.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ptr727
ptr727 merged commit 1c7e1f1 into develop Sep 12, 2026
9 checks passed
@ptr727
ptr727 deleted the fix/scope-refusal-names-script-dir branch September 12, 2026 04:06
ptr727 added a commit that referenced this pull request Sep 12, 2026
…1564)

Promotes one commit from `develop` to `main`.

- `1c7e1f1` Name the Directory the Scope Refusal Probed, Not the
Checkout (#1563)

`scripts/pr_review.py` reads the owner a write may stay within from the
script's own directory rather than the working directory, deliberately,
since the helper is hub-hosted and reached from a hub checkout while the
repository being answered is named on the command line. Its out-of-scope
refusals called that directory "this checkout", which an operator reads
as the working directory and can disprove on the spot with `git remote
get-url origin` in the tree they are standing in. Both refusals now
describe the probe and name the directory it ran against, and `reply`'s
exit-64 gloss, `origin_owner`'s docstring and the matching
`scripts/README.md` sentence agree with them.

Message only. The anchoring, the refusal, and the refusal writing
nothing are unchanged.

Copilot reviewed the feature pull request at its merged head, 3 of 3
changed files, no findings and none suppressed.

Closes #1557
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