Skip to content

Read the Reviewer's Node Id Repo-Wide Instead of From the Pull Request - #628

Merged
ptr727 merged 2 commits into
developfrom
fix/copilot-bot-node-id-repo-wide
Aug 8, 2026
Merged

Read the Reviewer's Node Id Repo-Wide Instead of From the Pull Request#628
ptr727 merged 2 commits into
developfrom
fix/copilot-bot-node-id-repo-wide

Conversation

@ptr727

Copy link
Copy Markdown
Owner

DispositionAmends "Re-vendor the changed \verbatim` content, which is one sweep covering seven files."` under "Fleet Sweeps". No cluster under "Work Clusters" covered this, and a cluster created and deleted in the same change records nothing, so the observation lands on the entry that already reasons about what each downstream copy owes.

No closing keyword, since a feature to develop pull request never fires one.

What was wrong

"Triggering and Polling" in the runbook said the reviewer bot's node id is read from an existing Copilot review on the pull request being reviewed. Three statements asserted it:

  • The round-1 paragraph: "The requestReviews mutation below is for re-requesting on later pushes (a new head SHA). By then a prior review exists, so its bot node id is readable."
  • The step-1 code comment, whose query was pullRequest(number: <N>) { reviews }.
  • The paragraph after the recipe: "step 1 needs at least one prior formal review on the PR".

A "Cold start" paragraph added later says the opposite in the same section, that the id is the reviewer account's and is stable across every pull request in the repo. The three statements above were never swept, so the section contradicted itself and the strict reading is the one a reader hits first. Acting on it, a pull request that auto-review-on-open never fired on has no way to reach round 1 and gets handed to the maintainer to seed through the UI, which is the hand-off the mutation exists to remove.

What the tree says

Read across the newest eight pull requests in this repo on 2026-08-08, every Copilot review carries the same bot node id. It is an account id, so nothing has to land on a pull request before it can be read.

The change

  • The round-1 paragraph drops the two claims above and says a round 1 carrying no review means wait, then request it yourself.
  • Step 1 queries the 20 newest pull requests with the first // empty guard and the [ -z ] stop, so it resolves on a pull request with no review of its own.
  • The per-pull-request claim and the cold-start exception collapse into one statement of the id's scope, which drops the duplicated query block. The pull-request-scoped read stays as a note that it buys nothing over the repo-wide read and fails on exactly the round the repo-wide read handles.
  • TODO.md records the re-vendor. This propagates a procedure rather than refreshing a hash, so a downstream repo left on the old copy is wrong on the rule. The #578 bullet claimed to be the sole rule-propagating half of that sweep, which a second one falsifies, so it is widened to two.

Verification

Both corrected blocks were run verbatim against this repo before the prose was written, reads only, and both returned the id. scripts/prose_lint.py exit 0, markdownlint-cli2 0 issues, editorconfig-checker exit 0, and both files were checked line by line for CRLF, .github/copilot-instructions.md at 353 of 353 and TODO.md at 489 of 489.

🤖 Generated with Claude Code

The runbook's "Triggering and Polling" said the bot node id is read from
an existing Copilot review on the pull request being reviewed, so step 1
needed a prior formal review there, and a pull request with none was sent
to the maintainer to seed one through the UI. A "Cold start" paragraph
added later contradicted that in the same section, and the three earlier
statements asserting the strict reading were never swept.
The id belongs to the reviewer bot account, not to a pull request, and is
identical on every pull request in the repo. Read across the eight newest
here on 2026-08-08, all eight carry the same value, so a pull request that
auto-review-on-open never fired on can still drive round 1 itself.
- The round-1 paragraph drops "the mutation below is for re-requesting on
later pushes" and "by then a prior review exists, so its bot node id is
readable". A round 1 with no review means wait, then request it.
- Step 1 queries the 20 newest pull requests with the empty-result guard,
so it resolves before this pull request has a review of its own.
- The per-pull-request claim and the cold-start exception collapse into one
statement of the id's scope, dropping the duplicate query block. The
pull-request-scoped read stays as a note that it buys nothing and fails
on exactly the round the repo-wide read handles.
TODO.md records the re-vendor: this propagates a procedure rather than
refreshing a hash, so a downstream repo left on the old copy still hands
round 1 back to the maintainer. The #578 bullet claimed to be the sole
rule-propagating half of that sweep and is widened to two.
Both corrected blocks were run verbatim before this was written, and the
change was checked with prose_lint.py, markdownlint-cli2, and
editorconfig-checker, all clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI lite review requested due to automatic review settings August 8, 2026 19:16

CopilotAI 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.

Pull request overview

Updates the Copilot review runbook to clarify that the Copilot reviewer bot’s GraphQL node id is repo-wide (account-scoped) and can be discovered without requiring a prior review on the current pull request, reducing “cold start” friction when auto-review-on-open doesn’t fire.

Changes:

  • Adjusts “Round 1” guidance to explicitly allow driving the first review via requestReviews when auto-review does not land.
  • Replaces the bot-id discovery query to scan recent pull requests repo-wide, with an explicit empty-result guard.
  • Records the re-vendor/procedure-propagation note in TODO.md.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
.github/copilot-instructions.mdUpdates the runbook’s bot-id discovery and round-1 workflow to avoid requiring a prior review on the PR.
TODO.mdUpdates the fleet sweep notes to record the runbook procedure change as rule-propagating content.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread.github/copilot-instructions.md
Copilot's round-1 finding on #628 says the GraphQL `pullRequests`
connection defaults to `states: [OPEN]`, which would leave the repo-wide
bot node id read empty in a repository whose Copilot-reviewed pull
requests have all merged, the cold start it exists for.
Run both ways against this repository while exactly one pull request was
open, the omitted default answers one OPEN and four MERGED, and
`states: [OPEN]` on the same call answers the one OPEN alone. The default
is every state, so the finding is false and the query stands.
It goes in "Disproved Claims" rather than only in the thread, because the
query is new, ships fleet-wide, and the claim is the kind a later round
re-raises against a maintainer with no way to tell it from a first.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CopilotAI review requested due to automatic review settings August 8, 2026 19:21

CopilotAI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit 92b9fc5 into developAug 8, 2026
7 checks passed
@ptr727
ptr727 deleted the fix/copilot-bot-node-id-repo-wide branch August 8, 2026 19:25
ptr727 added a commit that referenced this pull request Aug 8, 2026
Closes#578.
Promotion of `develop` at `6df14c0`, three squashes since the last one.
Merge commit only, no squash and no rebase, and **no
`--delete-branch`**, since this pull request's head is `develop` itself.
## What is being promoted
| Squash | Change |
| --- | --- |
| [`fdd1e25`](fdd1e25)
(#627) | Widens three carried `GOVERNANCE.md` rules to state the
recurring case, which is
[#578](#578), plus the
`WORKFLOW.md` cross-reference the same change took. |
| [`92b9fc5`](92b9fc5)
(#628) | Corrects the Copilot runbook to read the reviewer bot's node id
repo-wide rather than from the pull request under review, and records
the disproof of the claim raised against it. |
| [`6df14c0`](6df14c0)
(#629) | Records the backlog cluster for documenting a cross-owner write
grant. |
Four files, `.github/copilot-instructions.md`, `GOVERNANCE.md`,
`TODO.md` and `WORKFLOW.md`, at 49 insertions against 73 deletions.
## The closing keyword
`Closes#578` is on this pull request rather than on #627, because
GitHub fires a closing keyword only when it merges into the default
branch, and #627 merged into `develop`. Merging this closes that issue.
The work itself landed in `fdd1e25`.
## What this owes downstream
Two of the three carry a rule rather than a hash refresh, so a
repository left on the old copy is wrong on the rule and not only on the
bytes. Both are recorded in `TODO.md` under "Re-vendor the changed
`verbatim` content":
- The [#578](#578)
widening touches three `verbatim` `GOVERNANCE.md` sections, and
"Operational Repositories" matters most on the two `operational`
repositories that can act on it.
- The runbook correction is `intent` fidelity, so **no audit reports
it**. A repository on the old copy reads its own runbook as requiring a
review on the pull request before the reviewer bot's node id can be
read, and hands round 1 back to the maintainer whenever
auto-review-on-open does not fire.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for freeto 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

@ptr727