Skip to content

fix(copilot): explain review request denials instead of forwarding a bare 404 - #3119

Merged
SamMorrowDrums merged 2 commits into
github:mainfrom
dylanpulver:fix/copilot-review-cross-fork
Aug 19, 2026
Merged

fix(copilot): explain review request denials instead of forwarding a bare 404#3119
SamMorrowDrums merged 2 commits into
github:mainfrom
dylanpulver:fix/copilot-review-cross-fork

Conversation

@dylanpulver

@dylanpulver dylanpulver commented Aug 19, 2026 •
edited by SamMorrowDrums
Loading

Copy link
Copy Markdown
Contributor

Summary

request_copilot_review forwarded GitHub's 404 verbatim, so a caller who simply lacked write access on the target repository got a tool result that read as if the repository or the pull request did not exist. The tool now identifies which cause applies and says so.

Why

Fixes #3027

The reporter authored a cross-fork pull request into microsoft/vscode, requested a Copilot review from the GitHub website successfully, then got 404 Not Found from the same request through this tool. Nothing in the tool result explained the difference, so the report reasonably concluded the tool was calling the wrong endpoint.

Root cause. POST /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers requires write access to the repository, and authoring the pull request does not grant it (pull request reviews reference). The endpoint documents 403 and 422 as its denial responses (review requests REST reference), but what it actually returns is a 404 carrying no permission signal. Confirmed against the live API with a token holding full repo scope on a repository where permissions.push is false:

POST /repos/github/github-mcp-server/pulls/3038/requested_reviewers
HTTP/2.0 404 Not Found
X-Oauth-Scopes: gist, read:org, repo, workflow

The reviewer login was deliberately invalid so the call could not succeed, which also shows the permission check runs before reviewer validation. Token scope is not the problem, repository write access is.

There is no fallback to add. GraphQL exposes only requestReviews and requestReviewsByLogin, and both enforce the same gate. Calling requestReviewsByLogin as the same non-write user returns FORBIDDEN: ... does not have the correct permissions to execute RequestReviewsByLogin, so GraphQL refuses the same actor for the same reason and is only more honest about it. Whatever path the website uses to offer Copilot to a fork author is not exposed as a public API endpoint, so the fix goes to the error surface.

What changed

  • copilotReviewErrMsg in pkg/github/copilot.go enriches the failure message when the response is 403 or 404. It reads the repository once, on the failure path only, and reports which cause applies: no write access (pointing at the pull request page on the website, which is what the reporter found working), a repository that cannot be read at all, or a repository the caller can write to (pointing at the pull request number and Copilot availability instead).
  • Rate limiting also surfaces as 403. *github.RateLimitError and *github.AbuseRateLimitError short-circuit to the original message, so a rate-limited caller is not told they lack write access and does not pay a second call that would be refused for the same reason.
  • The shape follows dependabotErrMsg in pkg/github/dependabot.go, the existing helper in this repo that appends a permission hint on 403 or 404. Any other status keeps the original message, so unrelated failures are not mislabelled, and the success path is untouched.
  • Scoped to request_copilot_review on purpose. The other callers of RequestReviewers hit the same platform behavior, but the remedy text here is specific to Copilot code review, so a shared helper would not carry over cleanly.

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

The tool schema, name, description and success result are untouched. Only the text of an existing error result changes, so the toolsnap is unchanged.

Prompts tested (tool changes only)

No tool schema change, so no new prompts. The behavior was exercised through the unit tests below and the underlying API behavior was verified directly against api.github.com as shown above.

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

The added repository read uses the caller's own client and runs only after a request has already failed, so it grants no access the caller did not have and adds no call to the success path. The message names only the owner and repository the caller supplied, and reports write access as a boolean rather than echoing any response body.

Tool renaming

  • I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
    • I have added the new tool aliases in deprecated_tool_aliases.go
  • I am not renaming tools as part of this PR

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

./script/lint reports 0 issues and ./script/test passes across every package. ./script/generate-docs produces no diff, confirming the tool surface is unchanged. Six cases were added to Test_RequestCopilotReview:

  • pull request author without write access, the case from the issue: 404 plus a repository whose permissions.push is false, asserting the message names the missing write access.
  • forbidden is explained the same way as not found, so the documented 403 gets the same treatment as the 404 that actually shows up.
  • write access present points at the pull request instead, asserting the message names pull request 999 rather than blaming permissions.
  • unreadable repository, where the repository read also fails.
  • server error is not explained as a permission problem, asserting a 500 keeps the original message and gains no permission text.
  • rate limited forbidden is not explained as a permission problem, asserting a rate-limited 403 keeps the rate limit message and never reads the repository.

Docs

  • Not needed
  • Updated (README / docs / examples)

The tool description and generated docs are unchanged.

@dylanpulver
dylanpulver requested a review from a team as a code owner August 19, 2026 14:06
dylanpulver and others added 2 commits August 19, 2026 16:51
@dylanpulver @SamMorrowDrums
fix(copilot): explain review request denials instead of forwarding a …
698e354 
…bare 404

The review request endpoint requires write access to the repository, and
GitHub refuses a caller without it with 404 Not Found rather than a
permission error. Authoring the pull request does not grant that access,
so a fork contributor can be offered a Copilot review by the web UI and
still be refused by request_copilot_review, with nothing in the tool
result to say why.

On 403 or 404 the tool now reads the repository once so it can name the
cause. A caller without write access is told so directly and pointed at
the web UI. When the repository cannot be read at all, or when write
access is present, the message says so and points at the likelier
cause.
@SamMorrowDrums @dylanpulver
fix(copilot): keep rate limit denials out of the review permission hint
631703f 
A 403 carrying X-RateLimit-Remaining: 0, or a secondary rate limit
documentation URL, reached copilotReviewErrMsg as a rate limit error. It
was explained as a missing repository or missing write access, and the
repository read it triggered was refused for the same reason, so the
caller paid an extra call to be told the wrong thing.

Return the base message for both rate limit error types so the rate
limit text stands on its own, and trim the helper and its tests to the
comments the code cannot state.

Co-authored-by: Dylan Pulver <dylanpulver@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@SamMorrowDrums
SamMorrowDrums force-pushed the fix/copilot-review-cross-fork branch from e9a8d45 to 631703f Compare August 19, 2026 14:53
@SamMorrowDrums
SamMorrowDrums merged commit 24dc8b0 into github:main Aug 19, 2026
13 checks passed
SamMorrowDrums
SamMorrowDrums approved these changes Aug 19, 2026
View reviewed changes

@SamMorrowDrums SamMorrowDrums left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sanity-checked the failure-path behavior, rate-limit handling, tests, and comment cleanup. CI is green and the branch is current.

@github-actions github-actions Bot mentioned this pull request Aug 20, 2026
Closed
Copilot AI mentioned this pull request Aug 21, 2026
Merged
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
HarmonybrewBot HarmonybrewBot
!17166 merge bump-github-mcp-server-1.10.0 into main
f6385a3 
github-mcp-server 1.10.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>## Highlights

v1.10.0 is a substantial security, reliability, and compatibility release for the GitHub MCP Server.

### Safer by default

- Added confirmed repository deletion with form elicitation and protected multi-round-trip state ([#3076](github/github-mcp-server#3076)).
- Restricted bearer credentials to configured GitHub authorities ([#3056](github/github-mcp-server#3056)).
- Enforced HTTPS for GitHub Enterprise hosts ([#3069](github/github-mcp-server#3069)).
- Made invalid static `--tools` configuration fail closed ([#3050](github/github-mcp-server#3050)).
- Hardened lockdown, request limits, cache isolation, URL traversal, and response sanitization ([#3109](github/github-mcp-server#3109), [#3112](github/github-mcp-server#3112), [#3113](github/github-mcp-server#3113), [#3111](github/github-mcp-server#3111), [#3108](github/github-mcp-server#3108), [#3114](github/github-mcp-server#3114), [#3110](github/github-mcp-server#3110)).

### Better GitHub Enterprise Server compatibility

- `list_issues` and `search_issues` now degrade safely when issue custom-field GraphQL types are unavailable ([#3086](github/github-mcp-server#3086), [#2897](github/github-mcp-server#2897)).
- `list_issues` now returns assignee logins and stable empty arrays for unassigned issues ([#3064](github/github-mcp-server#3064)).
- Validation failures expose safe ruleset details when supplied by the GitHub API ([#3081](github/github-mcp-server#3081)).

### Safer repository file operations

- `get_file_contents` now identifies symbolic links and clearly labels dereferenced content.
- Symbolic-link writes require explicit `allow_symlink_write: true` opt-in ([#3071](github/github-mcp-server#3071)).
- Fixed binary MCP resources being base64-encoded twice ([#3098](github/github-mcp-server#3098)).

### More reliable tool contracts

- Corrected `add_issue_comment` input modes ([#3085](github/github-mcp-server#3085)).
- `issue_write.issue_fields[].delete: false` is now a valid no-op for strict-schema clients ([#3077](github/github-mcp-server#3077)).
- Zero-parameter tools accept an omitted `arguments` property while rejecting explicit `null` and malformed JSON ([#3099](github/github-mcp-server#3099)).
- Notification subscription tools explicitly advertise destructive behavior ([#2936](github/github-mcp-server#2936)).
- Copilot review-request denials now return actionable guidance instead of a bare 404 ([#3119](github/github-mcp-server#3119)).

### Projects, Actions, and efficiency

- Added Project view lifecycle operations and visible-field configuration ([#2961](github/github-mcp-server#2961), [#2988](github/github-mcp-server#2988)).
- Reduced Actions and other successful response payloads ([#3047](github/github-mcp-server#3047), [#3055](github/github-mcp-server#3055)).
- Improved sanitization performance and refreshed UI/build dependencies ([#3120](github/github-mcp-server#3120), [#3100](github/github-mcp-server#3100)).

### Behavior changes to note

- Unknown static `--tools` names now prevent startup.
- Symbolic-link updates require explicit opt-in.
- Requests cannot relax server-enforced lockdown.
- Oversized HTTP request bodies are rejected early.
- `add_issue_comment` performs stricter mutually exclusive mode validation.
- Repository deletion requires an eligible modern MCP client, form elicitation support, and the appropriate scopes.

## What's Changed
* Reduce Actions workflow list response payloads by @tommaso-moro in github/github-mcp-server#3047
* Use minimal response types for tool results by @tommaso-moro in github/github-mcp-server#3055
* Add basic project view management by @zwick in github/github-mcp-server#2961
* Add visible fields to project views by @zwick in github/github-mcp-server#2988
* fix(actions): avoid malformed response on log download failure by @SamMorrowDrums in github/github-mcp-server#3066
* fix(security): enforce HTTPS for gh-host/GITHUB_HOST to prevent cleartext credentials by @SamMorrowDrums in github/github-mcp-server#3069
* Reject unsupported subscription streams by @SamMorrowDrums in github/github-mcp-server#3073
* build(deps): bump distroless/base-debian12 from `348dac1` to `76b3162` by @dependabot[bot] in github/github-mcp-server#3088
* build(deps): bump golang from 1.25.12-alpine to 1.25.13-alpine by @dependabot[bot] in github/github-mcp-server#3087
* build(deps): bump github.com/stretchr/testify from 1.11.1 to 1.12.0 by @dependabot[bot] in github/github-mcp-server#3089
* build(deps): bump the npm_and_yarn group across 1 directory with 2 updates by @dependabot[bot] in github/github-mcp-server#3007
* build(deps): bump docker/login-action from 4.4.0 to 4.6.0 by @dependabot[bot] in github/github-mcp-server#3005
* Add confirmed repository deletion tool by @SamMorrowDrums in github/github-mcp-server#3076
* fix(issues): validate add_issue_comment input modes by @SamMorrowDrums in github/github-mcp-server#3085
* Fix static --tools validation fallback by @Mahmoud772122777 in github/github-mcp-server#3050
* Attach GitHub token only to configured GitHub hosts by @SyedAnas01 in github/github-mcp-server#3056
* Show ruleset violation details when create_branch fails by @Hashim1999164 in github/github-mcp-server#3081
* fix(notifications): mark subscription tools destructive by @SamMorrowDrums in github/github-mcp-server#2936
* Fix binary resource blob encoding by @SamMorrowDrums in github/github-mcp-server#3098
* Return assignees from list_issues by @tgockel in github/github-mcp-server#3064
* Fix list_issues on GHES schemas without issue fields by @SamMorrowDrums in github/github-mcp-server#3086
* Fix omitted tool arguments by @SamMorrowDrums in github/github-mcp-server#3099
* Clarify symlink behavior for repository file reads and writes by @theinfosecguy in github/github-mcp-server#3071
* fix(issues): allow delete:false in issue_write issue_fields by @tgockel in github/github-mcp-server#3077
* Handle unsupported issueFieldValues enrichment in search_issues by @kerobbi in github/github-mcp-server#2897
* build(deps): clear UI dependency alerts by @SamMorrowDrums in github/github-mcp-server#3100
* fix(lockdown): harden pull_request_read get_commits handling by @SamMorrowDrums in github/github-mcp-server#3109
* fix(http): make server lockdown mode an upper bound over requests by @SamMorrowDrums in github/github-mcp-server#3112
* Centralize sanitization of untrusted GitHub response fields by @SamMorrowDrums in github/github-mcp-server#3114
* Filter invisible Unicode after HTML entity normalization by @SamMorrowDrums in github/github-mcp-server#3110
* fix(raw): reject traversal segments when constructing raw content URLs by @SamMorrowDrums in github/github-mcp-server#3108
* refactor: condense lockdown comments in GetPullRequestCommits by @SamMorrowDrums in github/github-mcp-server#3115
* Limit HTTP request bodies before MCP middleware parsing by @SamMorrowDrums in github/github-mcp-server#3111
* fix(lockdown): isolate repo-access cache per caller identity by @SamMorrowDrums in github/github-mcp-server#3113
* perf(sanitize): make clean text allocation-free on the hot path by @SamMorrowDrums in github/github-mcp-server#3120
* test(sanitize): drop the optimization scaffolding by @SamMorrowDrums in github/github-mcp-server#3121
* fix(copilot): explain review request denials instead of forwarding a bare 404 by @dylanpulver in github/github-mcp-server#3119
* build(deps): bump the npm_and_yarn group across 1 directory with 4 updates by @dependabot[bot] in github/github-mcp-server#3091

## New Contributors
* @Mahmoud772122777 made their first contribution in github/github-mcp-server#3050
* @SyedAnas01 made their first contribution in github/github-mcp-server#3056
* @Hashim1999164 made their first contribution in github/github-mcp-server#3081
* @tgockel made their first contribution in github/github-mcp-server#3064
* @theinfosecguy made their first contribution in github/github-mcp-server#3071
* @dylanpulver made their first contribution in github/github-mcp-server#3119

**Full Changelog**: https://github.com/github/github-mcp-

See merge request: Harmonybrew/homebrew-core!17166
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SamMorrowDrums SamMorrowDrums SamMorrowDrums approved these changes
+1 more reviewer
@abdullah3137867-commits abdullah3137867-commits abdullah3137867-commits approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

request_copilot_review returns 404 for PR author on cross-fork PR while GitHub UI succeeds

3 participants

@dylanpulver @SamMorrowDrums @abdullah3137867-commits