fix(git): handle worktrees from forked PRs safely (#2693)
* fix(git): create worktrees from forked PRs via refs/pull/<n>/head fallback A worktree created from a linked GitHub PR whose head branch lives in a fork failed when the fork's head repository was missing (deleted fork) or unfetchable (auth, network): the dialog threw 'PR head repository URL is unavailable' before any git command ran, and the server had no fallback to refs/pull/<n>/head, which GitHub serves on the base repository. - NewWorktreeDialog: when pr.headRepo is absent, send a prRef config (refs/pull/<n>/head from origin) instead of throwing; the fork config now also carries prRef so the server can fall back when the fork fetch fails. - git service: fetchPullRequestHeadRef fetches refs/pull/<n>/head into refs/remotes/<remote>/pr-<n>-head (same refspec shape as fetchRemoteBranchRef) and both validateWorktreeCreate and attachGitWorktreeToCandidate fall back to it when the fork path fails; fallback worktrees get --no-track and no upstream config because a PR ref is not pushable. When both paths fail the original fork error surfaces. - Focused tests cover the prRef-only path and the fork-unreachable fallback. Fixes #2422 * fix(git): harden PR worktree fallback against stale fork refs (#12) After a fork fetch fails, resolve immediately from refs/pull/<n>/head instead of accepting a cached remotes/<fork>/<branch> tracking ref. Match the PR base repository by URL (not a hardcoded origin remote), store fetched PR heads under refs/openchamber/pull/<n>/head, and share one existing-mode resolver between validate and create. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * fix(git): make PR head SHA authoritative and namespace private refs Reuse local/remote branches for linked PRs only when their tip matches pr.headSha; otherwise fall through to fork fetch / refs/pull. Store PR heads under refs/openchamber/github/<owner>/<repo>/pull/<n>/head, prefer HTTPS for direct base-repo fallback, and surface composite fork+fallback errors when both paths fail. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * test(ui): assert validate/create forward deleted-fork PR payload fields Guards the dialog wiring regression where validate omitted prRef while create included it, by asserting worktreeManager forwards prRef, prBaseRepoUrl, and related fields for deleted-fork configs. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * refactor(git): always checkout linked PRs from refs/pull/<n>/head Move PR worktree resolution to the server. The UI now sends only pullRequest identity (number + baseRepoUrl + optional head fields); the server always fetches the authoritative PR head and best-effort configures fork upstream afterward. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * refactor(ui): drop PrWorktreeConfig; send PR identity only Delete the prWorktreeConfig module. NewWorktreeDialog maps linked PRs straight to pullRequest identity, skips upstream defaults for that path, and leaves checkout + optional fork tracking to the server. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * refactor(git): linked PRs are {number, baseRepoUrl} only Drop fork upstream / tracking and head/base owner-repo fields from the linked-PR worktree path. Fetch refs/pull/<n>/head, create --no-track, done. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * fix(git): meet #2422 Must/Should without refs/pull fallback Linked PRs send fork identity only; the server provisions pr-<owner>, fetches the head branch, and fails clearly when the fork is missing or unreachable. Local reuse requires a matching headSha. Prefer HTTPS for headRepoUrl. Do not write upstream tracking when the upstream ref was never fetched. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * fix(git): drop invalid upstream fallback and PR branch collisions Remove setBranchTrackingFallback: if upstream fetch fails, leave tracking unset. When a linked PR's head branch already exists locally with a different tip, create pr-<number> instead of git worktree add -b on the colliding name. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> * fix(git): strip PR worktree create back to fork-remote provision (#15) Keep the original ensureRemoteName/Url path for linked fork PRs, prefer HTTPS clone URLs, fail clearly when the fork is unreachable, and leave upstream tracking unset when the upstream ref was never fetched. Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com> Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
co-authored by
Serhii Dziupin
parent
bf0dfc4e6b
commit
9832c0a4a8
@@ -27,6 +27,7 @@ import {
|
||||
applyHunk,
|
||||
getDiff,
|
||||
getFileDiff,
|
||||
validateWorktreeCreate,
|
||||
} from './service.js';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -806,6 +807,134 @@ describe('createWorktree', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// createWorktree from a forked GitHub PR head (issue #2422)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('createWorktree from a forked GitHub PR', () => {
|
||||
const withDataHome = async (test) => {
|
||||
const previousXdgDataHome = process.env.XDG_DATA_HOME;
|
||||
const dataHome = createTempDir();
|
||||
process.env.XDG_DATA_HOME = dataHome;
|
||||
try {
|
||||
await test(dataHome);
|
||||
} finally {
|
||||
if (previousXdgDataHome === undefined) {
|
||||
delete process.env.XDG_DATA_HOME;
|
||||
} else {
|
||||
process.env.XDG_DATA_HOME = previousXdgDataHome;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const publishForkHead = (repository, forkBare, branchName) => {
|
||||
fs.writeFileSync(path.join(repository, 'FORK.md'), `# ${branchName}\n`);
|
||||
runGit(repository, ['add', 'FORK.md']);
|
||||
runGit(repository, ['commit', '-m', `fork ${branchName}`]);
|
||||
const sha = runGit(repository, ['rev-parse', 'HEAD']).trim();
|
||||
runGit(repository, ['push', forkBare, `HEAD:refs/heads/${branchName}`]);
|
||||
return sha;
|
||||
};
|
||||
|
||||
const getBranchTrackingRemote = (directory, branch) => {
|
||||
try {
|
||||
return runGit(directory, ['config', '--get', `branch.${branch}.remote`]).trim();
|
||||
} catch {
|
||||
return '';
|
||||
}
|
||||
};
|
||||
|
||||
const forkWorktreeInput = ({ fork, worktreeName }) => ({
|
||||
mode: 'existing',
|
||||
branchName: 'feature/login',
|
||||
worktreeName,
|
||||
existingBranch: 'remotes/pr-alice/feature/login',
|
||||
setUpstream: true,
|
||||
upstreamRemote: 'pr-alice',
|
||||
upstreamBranch: 'feature/login',
|
||||
ensureRemoteName: 'pr-alice',
|
||||
ensureRemoteUrl: fork,
|
||||
});
|
||||
|
||||
it('creates a worktree from a reachable fork head remote', async () => {
|
||||
if (!canRunGit()) return;
|
||||
|
||||
await withDataHome(async () => {
|
||||
const { repository } = createRepositoryWithRemote();
|
||||
const fork = createTempDir();
|
||||
runGit(fork, ['init', '--bare']);
|
||||
const sha = publishForkHead(repository, fork, 'feature/login');
|
||||
|
||||
const created = await createWorktree(repository, forkWorktreeInput({
|
||||
fork,
|
||||
worktreeName: 'pr-42',
|
||||
}));
|
||||
|
||||
expect(created.branch).toBe('feature/login');
|
||||
expect(runGit(created.path, ['rev-parse', 'HEAD']).trim()).toBe(sha);
|
||||
await expect.poll(() => fs.existsSync(path.join(created.path, 'FORK.md')), { timeout: 5_000 }).toBe(true);
|
||||
expect(runGit(repository, ['remote', 'get-url', 'pr-alice']).trim()).toBe(fork);
|
||||
await expect.poll(
|
||||
() => getBranchTrackingRemote(created.path, 'feature/login') === 'pr-alice',
|
||||
{ timeout: 5_000 }
|
||||
).toBe(true);
|
||||
});
|
||||
}, 30_000);
|
||||
|
||||
it('rejects an unreachable fork with an actionable error and no worktree', async () => {
|
||||
if (!canRunGit()) return;
|
||||
|
||||
await withDataHome(async () => {
|
||||
const { repository } = createRepositoryWithRemote();
|
||||
const missingFork = path.join(createTempDir(), 'missing-fork.git');
|
||||
const before = runGit(repository, ['worktree', 'list', '--porcelain']);
|
||||
|
||||
await expect(createWorktree(repository, forkWorktreeInput({
|
||||
fork: missingFork,
|
||||
worktreeName: 'pr-42-unreachable',
|
||||
}))).rejects.toThrow(/Unable to (reach|fetch)/i);
|
||||
|
||||
expect(runGit(repository, ['worktree', 'list', '--porcelain'])).toBe(before);
|
||||
|
||||
const validation = await validateWorktreeCreate(repository, forkWorktreeInput({
|
||||
fork: missingFork,
|
||||
worktreeName: 'pr-42-unreachable',
|
||||
}));
|
||||
expect(validation.ok).toBe(false);
|
||||
expect(validation.errors.some((error) => /Unable to (reach|fetch)/i.test(error.message))).toBe(true);
|
||||
});
|
||||
}, 30_000);
|
||||
|
||||
it('does not write upstream tracking when the upstream ref cannot be fetched', async () => {
|
||||
if (!canRunGit()) return;
|
||||
|
||||
await withDataHome(async () => {
|
||||
const { repository } = createRepositoryWithRemote();
|
||||
runGit(repository, ['branch', 'feature/tracking']);
|
||||
const emptyRemote = createTempDir();
|
||||
runGit(emptyRemote, ['init', '--bare']);
|
||||
runGit(repository, ['remote', 'add', 'broken-upstream', emptyRemote]);
|
||||
|
||||
const created = await createWorktree(repository, {
|
||||
mode: 'existing',
|
||||
branchName: 'feature/tracking-wt',
|
||||
worktreeName: 'feature-tracking-wt',
|
||||
existingBranch: 'feature/tracking',
|
||||
setUpstream: true,
|
||||
upstreamRemote: 'broken-upstream',
|
||||
upstreamBranch: 'does-not-exist',
|
||||
});
|
||||
|
||||
await expect.poll(
|
||||
() => getWorktreeBootstrapStatus(created.path).then((status) => status.status === 'ready' || status.status === 'failed'),
|
||||
{ timeout: 5_000 }
|
||||
).toBe(true);
|
||||
|
||||
expect(getBranchTrackingRemote(created.path, 'feature/tracking-wt')).toBe('');
|
||||
});
|
||||
}, 30_000);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// removeWorktree
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user