← 输入框|github.com|刷新
Skip to content

Offer to fork when "Push Commits" targets a read-only remote in Create PR - #9136

Merged
Alex Ross (alexr00) merged 8 commits into
mainfrom
copilot/auto-fork-when-no-permission
Nov 4, 2026
Merged

Alex Ross (alexr00) merged 8 commits into
mainfrom
copilot/auto-fork-when-no-permission

Conversation

Copilot AI commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Creating a PR on a repo you can't push to (for example, a quick fix in Codespaces) fails when the branch already tracks the read-only remote and has unpushed commits. The Publish Branch path already offers to fork; the Push Commits path pushed straight to the tracked remote. That push either failed, or VS Code's built-in GitHub push-error handling forked behind our back, so the PR head pointed at the original repo instead of the fork.

Changes (CreatePullRequestViewProvider)

  • Push through publishBranch: a new pushCommits helper sends Push Commits through FolderRepositoryManager.publishBranch. That gives this path the same permission check, fork prompt, and push-error modals as Publish Branch. If the tracked remote isn't a known GitHub repository, it falls back to a plain push as before.
  • Fork becomes the head: when the commits land in a fork, compareOwner/compareRepositoryName are reassigned to the fork. headRepo then resolves to the fork, the head becomes <fork-owner>:<branch>, and the PR still targets the original repo.
  • Clean cancel: declining the fork, hitting a rate limit, or a rejected push cancels creation without creating a PR.
const pushResult = await this.pushCommits(compareBranch.upstream.remote, compareBranchName);
if (!pushResult) { /* cancelled */ return; }
if (pushResult.fork) {
	compareOwner = pushResult.fork.owner;
	compareRepositoryName = pushResult.fork.repositoryName;
}

Review notes

  • Behaviour change for users with write access: Push Commits now pushes with -u and shows the modal warnings from Publish Branch on failure.
  • Model not updated: CreatePullRequestDataModel.compareOwner isn't changed after a fork, which matches the existing publish path. So if creation fails after a fork, a retry from the same view would use the original repo as the head.
  • Not covered: creating a patch-n branch automatically when compare == base, as GitHub's web editor does. The view still disables Create in that case.

Tests

New Create pull request with unpushed commits suite in createPRViewProvider.test.ts covers three cases:

  • Write access: pushes to origin with -u; head is github:D4.
  • Read access, fork accepted: forks, pushes to the fork, and creates the PR with head contributor:D4 against github/test.
  • Read access, fork declined: no push and no PR.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

…ing a PR

Co-authored-by: alexr00 <38270282+alexr00@users.noreply.github.com>
Copilot AI changed the title [WIP] Add auto-fork feature for branch creation Offer to fork when "Push Commits" targets a read-only remote in Create PR Oct 29, 2026

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.

🟡 Changes recommended

Fork publishing mishandles configured push destinations and retry state, which can block valid pushes, make retries fail, or create pull requests from the wrong repository.

3 open issues
What changed in this PR

Extends the Create PR flow to offer a fork when unpushed commits target a read-only remote, addressing the Codespaces contribution workflow described in #2998.

Changes:

  • Routes Push Commits through the existing publish flow for permission checks, fork prompts, and push-error handling.
  • Uses the fork as the PR head and updates the compare-owner model after a successful fork push.
  • Adds tests for writable remotes, accepted forks, and declined forks.
File Description
src/​test/​github/​createPRViewProvider.test.ts Adds coverage for pushing unpushed commits and fork decisions.
src/​github/​createPRViewProvider.ts Adds fork-aware pushing, cancellation handling, and fork head selection.

Review effort: Balanced

Comment thread src/github/createPRViewProvider.ts Resolved
Comment thread src/github/createPRViewProvider.ts Resolved
Comment thread src/github/createPRViewProvider.ts Resolved
@alexr00
Alex Ross (alexr00) marked this pull request as ready for review November 2, 2026 16:46
Copilot AI balanced review requested due to automatic review settings November 2, 2026 16:46

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.

🟡 Changes recommended

Automatic fork reuse does not verify server identity, risking Enterprise commits being pushed to a public repository without a fork prompt.

4 open issues
3 resolved since last review

Review effort: Balanced

Comment thread src/github/folderRepositoryManager.ts Resolved
Comment thread src/github/createPRViewProvider.ts Resolved
Comment thread src/github/folderRepositoryManager.ts Resolved
Comment thread src/github/folderRepositoryManager.ts Resolved
@alexr00
Alex Ross (alexr00) marked this pull request as draft November 2, 2026 16:54
@alexr00
Alex Ross (alexr00) marked this pull request as ready for review November 3, 2026 12:29
Copilot AI balanced review requested due to automatic review settings November 3, 2026 12:30

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.

🟡 Changes recommended

Push-destination validation can send commits to an unrelated repository, while URL-alias handling and stale compare state can leave the PR pointing at the wrong head.

1 open issue
2 previously missed issues

In code that hasn't changed since last review

Medium severity Persist the head repository for differently named forks

src/​github/​createPRViewProvider.ts:1691

For a writable fork with a different name, such as contributor/test-fork, this guard skips updating both the model and the webview. If pushing succeeds but PR creation fails, a retry with no unpushed commits skips pushCommits() and uses the stale original repository as the head, potentially omitting the pushed changes. Persist both the actual compare owner and repository independently of the base repository name.

Medium severity Resolve push URL aliases before validating the destination

src/​github/​folderRepositoryManager.ts:3335

With a GitHub fetch URL and an aliased push URL such as gh:contributor/test, this parses gh as the host instead of applying Git's url.*.insteadOf substitution. The server check fails, so pushCommits() falls back to a plain push and leaves the PR head on the original repository even though the commits land in the fork. Resolve the alias before parsing and checking the push destination, as parseRepositoryRemotesAsync() does.

4 resolved since last review

Review effort: Balanced

Comment thread src/github/folderRepositoryManager.ts Resolved
Copilot AI balanced review requested due to automatic review settings November 3, 2026 14:07

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.

🟡 Changes recommended

Remote validation can accept an unrelated push target or reject a valid fork with an outdated local URL, risking misdirected pushes or failed fork operations.

1 open issue
1 resolved since last review

Review effort: Balanced

private async fetchesFrom(remote: GitHubRemote): Promise<boolean> {
const fetchUrl = this.repository.state.remotes.find(candidate => candidate.name === remote.remoteName)?.fetchUrl;
const fetchRemote = fetchUrl ? parseRemote(remote.remoteName, await resolveGitUrl(fetchUrl, this.repository)) : null;
return !!fetchRemote && isOnServerOf(fetchRemote, remote) && isSameRepository(fetchRemote, remote);
@alexr00
Alex Ross (alexr00) merged commit 1c32013 into main Nov 4, 2026
7 checks passed
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.

Auto-fork when when you don't have permissions to commit

5 participants