From caf17de00692ff1a19e3823a2599c6347bbff2eb Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Mon, 7 Sep 2026 23:01:11 -0700 Subject: [PATCH] Bind review pushes to execution-host endpoint authority --- .../fetch-refspec-abbreviation-real.test.ts | 69 ++++++ .../git/git-upstream-status-read-owner.ts | 12 +- src/main/git/remote.test.ts | 21 +- src/main/git/remote.ts | 5 + src/main/git/upstream-identity-real.test.ts | 6 +- src/main/github/client-pr-push-target.test.ts | 32 ++- .../client/lookup/pull-request-push-target.ts | 19 +- .../github/github-remote-identity-parsing.ts | 124 +--------- .../pr-start-point-compare-base.test.ts | 6 +- src/main/github/pr-start-point.test.ts | 3 + src/main/github/pr-start-point.ts | 6 +- .../github/review-push-endpoint-real.test.ts | 228 ++++++++++++++++++ src/main/gitlab/mappers.ts | 5 + src/main/gitlab/project-ref-parser.ts | 129 +--------- .../git-remote/branch-mutation-handlers.ts | 2 + src/main/ipc/worktree-push-target-setup.ts | 13 + .../ipc/worktree-review-push-target.test.ts | 38 +++ src/main/ipc/worktree-review-push-target.ts | 27 +++ .../ipc/worktrees-wsl-runtime-routing.test.ts | 3 +- .../ssh-git-provider-remote-sync.test.ts | 17 +- .../providers/ssh-git-remote-sync-provider.ts | 11 + .../gitlab-and-pr-bases-part-03.spec.ts | 9 +- src/main/runtime/rpc/methods/git-params.ts | 8 +- .../rpc/methods/git-push-target-schema.ts | 16 ++ .../methods/git-review-target-schema.test.ts | 14 ++ .../rpc/methods/worktree-create-schemas.ts | 9 +- .../runtime/rpc/methods/worktree-schemas.ts | 10 +- src/main/runtime/runtime-git-sync-commands.ts | 2 + .../runtime-gitlab-push-authority.test.ts | 69 ++++++ .../runtime/runtime-gitlab-worktree-base.ts | 29 ++- src/relay/git-exec-validator.test.ts | 2 +- src/relay/git-handler-push-target.test.ts | 13 +- src/relay/git-handler-push-target.ts | 2 + .../checks-panel-git-status-snapshot.ts | 2 + .../push-target-upstream-refresh-cache.ts | 2 + ...-control-hosted-review-push-target.test.ts | 15 +- .../src/runtime/runtime-git-client.test.ts | 6 +- .../src/runtime/runtime-git-sync-client.ts | 12 + .../runtime-review-push-authority.test.ts | 43 ++++ .../editor-git-status-reconciliation.test.ts | 7 +- ...tor-linked-review-operation-target.test.ts | 14 +- .../actions/linked-review-operation-target.ts | 30 +-- .../editor/git/git-status-reconciliation.ts | 4 + .../slices/worktree-listing-branch-switch.ts | 2 + ...orktrees-linked-review-push-target.test.ts | 34 ++- ...orktrees-queued-review-push-target.test.ts | 3 +- .../hosted-review-push-target-ensure.ts | 4 +- .../metadata/hosted-review-push-target.ts | 4 +- .../metadata/update-worktree-meta.ts | 2 +- src/shared/__fixtures__/git-review-target.ts | 18 ++ src/shared/git-binary-compatibility.test.ts | 18 ++ src/shared/git-publish-target-status.ts | 4 + src/shared/git-push-target-validation.ts | 45 +++- src/shared/git-remote-tracking-ref.test.ts | 23 ++ src/shared/git-remote-tracking-ref.ts | 42 +++- src/shared/git-review-push-authority.test.ts | 81 +++++++ src/shared/git-review-push-authority.ts | 90 +++++++ src/shared/git-status-types.ts | 3 + src/shared/github/remote-identity-parsing.ts | 123 ++++++++++ src/shared/gitlab-project-ref-parser.ts | 128 ++++++++++ src/shared/gitlab-types.ts | 2 + .../hosted-review-push-target-admission.ts | 12 +- src/shared/linked-review-operation-target.ts | 31 +++ src/shared/worktree/types.ts | 9 + 64 files changed, 1367 insertions(+), 405 deletions(-) create mode 100644 src/main/git/fetch-refspec-abbreviation-real.test.ts create mode 100644 src/main/github/review-push-endpoint-real.test.ts create mode 100644 src/main/ipc/worktree-review-push-target.test.ts create mode 100644 src/main/ipc/worktree-review-push-target.ts create mode 100644 src/main/runtime/rpc/methods/git-push-target-schema.ts create mode 100644 src/main/runtime/rpc/methods/git-review-target-schema.test.ts create mode 100644 src/main/runtime/runtime-gitlab-push-authority.test.ts create mode 100644 src/renderer/src/runtime/runtime-review-push-authority.test.ts create mode 100644 src/shared/__fixtures__/git-review-target.ts create mode 100644 src/shared/git-review-push-authority.test.ts create mode 100644 src/shared/git-review-push-authority.ts create mode 100644 src/shared/github/remote-identity-parsing.ts create mode 100644 src/shared/gitlab-project-ref-parser.ts create mode 100644 src/shared/linked-review-operation-target.ts diff --git a/src/main/git/fetch-refspec-abbreviation-real.test.ts b/src/main/git/fetch-refspec-abbreviation-real.test.ts new file mode 100644 index 00000000000..868039af6be --- /dev/null +++ b/src/main/git/fetch-refspec-abbreviation-real.test.ts @@ -0,0 +1,69 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it } from 'vitest' +import { gitExecFileAsync } from './command-runner/git-exec-file' +import { readGitRemoteTrackingRef } from '../../shared/git-remote-tracking-ref' +import { getPublishTargetStatus } from '../../shared/git-publish-target-status' + +let root: string +let env: NodeJS.ProcessEnv +const run = (args: string[]) => gitExecFileAsync(args, { cwd: root, env, timeout: 10_000 }) +const git = async (...args: string[]) => (await run(args)).stdout.trim() +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-refspec-abbreviation-')) + env = { + ...process.env, + HOME: root, + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: join(root, 'empty-config') + } + await git('init', '-q') + await git( + '-c', + 'user.name=Fixture', + '-c', + 'user.email=fixture@example.invalid', + '-c', + 'commit.gpgSign=false', + 'commit', + '--allow-empty', + '-qm', + 'base' + ) + await git('branch', '-M', 'feature') + await git('clone', '--bare', '-q', root, join(root, 'remote.git')) + await git('remote', 'add', 'origin', join(root, 'remote.git')) +}) +afterEach(async () => { + await rm(root, { recursive: true, force: true }) +}) + +it.each([ + ['feature:refs/custom/exact-feature', 'refs/custom/exact-feature'], + ['heads/feature:refs/custom/exact-feature', 'refs/custom/exact-feature'], + ['feature:custom', 'refs/heads/custom'], + ['feature:heads/tracked', 'refs/heads/tracked'], + ['feature:tags/tracked', 'refs/tags/tracked'], + ['feature:remotes/origin/tracked', 'refs/remotes/origin/tracked'] +])('corroborates source and destination like real Git for %s', async (mapping, ref) => { + await git('config', 'remote.origin.fetch', mapping) + await git('fetch', '-q', 'origin') + expect(await readGitRemoteTrackingRef(run, 'origin', 'feature')).toBe(ref) + expect( + await getPublishTargetStatus(run, { remoteName: 'origin', branchName: 'feature' }) + ).toMatchObject({ hasUpstream: true, upstreamIdentity: { trackingRef: ref } }) + const before = await git('for-each-ref', '--format=%(refname) %(objectname)') + await git('fetch', '--dry-run', 'origin') + expect(await git('for-each-ref', '--format=%(refname) %(objectname)')).toBe(before) +}) + +it('does not relabel a higher-ranked tag or a missing source as the review branch', async () => { + await git('--git-dir', join(root, 'remote.git'), 'tag', 'feature') + await git('config', 'remote.origin.fetch', 'feature:refs/custom/from-tag') + await git('fetch', '-q', 'origin') + expect(await git('rev-parse', '--verify', 'refs/custom/from-tag')).toBeTruthy() + expect(await readGitRemoteTrackingRef(run, 'origin', 'feature')).toBeNull() + await git('config', 'remote.origin.fetch', 'missing:refs/custom/from-tag') + expect(await readGitRemoteTrackingRef(run, 'origin', 'missing')).toBeNull() +}) diff --git a/src/main/git/git-upstream-status-read-owner.ts b/src/main/git/git-upstream-status-read-owner.ts index c122674d9d2..00fc0307bb7 100644 --- a/src/main/git/git-upstream-status-read-owner.ts +++ b/src/main/git/git-upstream-status-read-owner.ts @@ -1,3 +1,4 @@ +import { reviewHeadKey } from '../../shared/git-review-push-authority' import type { GitUpstreamStatus } from '../../shared/git-status-types' import type { GitPushTarget } from '../../shared/worktree/types' import { InFlightPromiseDedupe, stableInFlightKey } from '../../shared/in-flight-promise-dedupe' @@ -9,10 +10,17 @@ import { InFlightPromiseDedupe, stableInFlightKey } from '../../shared/in-flight * compile error instead of a silently shared lease between two different targets. */ function pushTargetKeyParts(pushTarget: GitPushTarget): readonly unknown[] { - const { remoteName, branchName, remoteUrl, remoteCreated, ...rest } = pushTarget + const { remoteName, branchName, remoteUrl, remoteCreated, reviewHead, ...rest } = pushTarget const exhaustive: Record = rest void exhaustive - return ['explicit-target', remoteName, branchName, remoteUrl ?? null, remoteCreated ?? null] + return [ + 'explicit-target', + remoteName, + branchName, + remoteUrl ?? null, + remoteCreated ?? null, + reviewHeadKey(reviewHead) ?? null + ] } export type GitUpstreamStatusExecutionIdentity = diff --git a/src/main/git/remote.test.ts b/src/main/git/remote.test.ts index ab52194d22d..60beb4a7e34 100644 --- a/src/main/git/remote.test.ts +++ b/src/main/git/remote.test.ts @@ -284,11 +284,18 @@ describe('git remote operations', () => { }) it('uses an explicit push target even when it differs from the local branch name', async () => { - gitExecFileAsyncMock - .mockResolvedValueOnce({ stdout: '', stderr: '' }) - .mockResolvedValueOnce({ stdout: '', stderr: '' }) + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: '', stderr: '' }).mockResolvedValueOnce({ + stdout: 'origin\thttps://github.com/team/repo.git (push)', + stderr: '' + }) await gitPush('/repo', false, { + reviewHead: { + provider: 'github', + host: 'github.com', + repository: 'team/repo', + branchName: 'contributor/fix-sidebar' + }, remoteName: 'origin', branchName: 'contributor/fix-sidebar' }) @@ -297,13 +304,7 @@ describe('git remote operations', () => { ['push', '--set-upstream', 'origin', 'HEAD:refs/heads/contributor/fix-sidebar'], { cwd: '/repo' } ) - expect(gitExecFileAsyncMock.mock.calls).toEqual([ - [['check-ref-format', '--branch', 'contributor/fix-sidebar'], { cwd: '/repo' }], - [ - ['push', '--set-upstream', 'origin', 'HEAD:refs/heads/contributor/fix-sidebar'], - { cwd: '/repo' } - ] - ]) + expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', '-v'], { cwd: '/repo' }) }) it('passes --force-with-lease when requested', async () => { diff --git a/src/main/git/remote.ts b/src/main/git/remote.ts index d025895319d..74f24d79b33 100644 --- a/src/main/git/remote.ts +++ b/src/main/git/remote.ts @@ -1,3 +1,4 @@ +import { assertGitReviewPushAuthority } from '../../shared/git-review-push-authority' import { normalizeGitErrorMessage, runPullWithDivergenceFallback @@ -38,6 +39,10 @@ export async function gitPush( try { if (pushTarget) { await validateGitPushTarget(worktreePath, pushTarget, options) + await assertGitReviewPushAuthority( + (args) => gitExecFileAsync(args, gitOptionsForWorktree(worktreePath, options)), + pushTarget + ) } // Why: push to the branch's configured upstream when one exists. PR-created // worktrees can track a contributor fork remote; hardcoding origin here diff --git a/src/main/git/upstream-identity-real.test.ts b/src/main/git/upstream-identity-real.test.ts index 8512d32cb61..853f95188e8 100644 --- a/src/main/git/upstream-identity-real.test.ts +++ b/src/main/git/upstream-identity-real.test.ts @@ -73,7 +73,9 @@ it.each([ trackingRef: upstreamBefore!.upstreamRef }) const pushTarget = { remoteName: remote, branchName: expectedBranch } - expect(hasUsableHostedReviewPushTarget({ pushTarget, upstreamStatus: statusBefore })).toBe(true) + expect(hasUsableHostedReviewPushTarget({ pushTarget, upstreamStatus: statusBefore })).toBe( + false + ) const { upstreamIdentity: _identity, ...oldPeerStatus } = statusBefore expect(hasUsableHostedReviewPushTarget({ pushTarget, upstreamStatus: oldPeerStatus })).toBe( false @@ -86,7 +88,7 @@ it.each([ upstreamName: 'unrelated/display/label' } }) - ).toBe(true) + ).toBe(false) const watch = (trackingRef?: string) => resolveGitStatusUpstreamRef( (args) => run(args), diff --git a/src/main/github/client-pr-push-target.test.ts b/src/main/github/client-pr-push-target.test.ts index daaa05155ae..35a58730990 100644 --- a/src/main/github/client-pr-push-target.test.ts +++ b/src/main/github/client-pr-push-target.test.ts @@ -67,7 +67,7 @@ describe('getPRForBranch', () => { expect(ghExecFileAsyncMock).toHaveBeenCalledWith(['api', 'repos/stablyai/orca/pulls/1738'], { cwd: '/repo-root' }) - expect(target).toEqual({ + expect(target).toMatchObject({ pushTarget: { remoteName: 'pr-prateek-orca', branchName: 'prateek/fix-sidebar-agents-toggle', @@ -134,7 +134,7 @@ describe('getPRForBranch', () => { }) getRemoteUrlForRepoMock.mockResolvedValueOnce('git@github.com:stablyai/orca.git') - await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toEqual({ + await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toMatchObject({ pushTarget: { remoteName: 'pr-prateek-orca', branchName: 'prateek/fix-sidebar-agents-toggle', @@ -145,6 +145,10 @@ describe('getPRForBranch', () => { }) it('omits maintainerCanModify when the API does not report the flag', async () => { + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'origin\thttps://github.com/stablyai/orca.git (push)', + stderr: '' + }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) ghExecFileAsyncMock.mockResolvedValueOnce({ @@ -162,7 +166,7 @@ describe('getPRForBranch', () => { }) }) - await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toEqual({ + await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toMatchObject({ pushTarget: { remoteName: 'origin', branchName: 'fix-sidebar' @@ -171,6 +175,10 @@ describe('getPRForBranch', () => { }) it('uses origin for same-repository PR push targets', async () => { + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'origin\thttps://github.com/stablyai/orca.git (push)', + stderr: '' + }) getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' }) ghExecFileAsyncMock.mockResolvedValueOnce({ @@ -188,13 +196,13 @@ describe('getPRForBranch', () => { }) }) - await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toEqual({ + await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toMatchObject({ pushTarget: { remoteName: 'origin', branchName: 'fix-sidebar' } }) - expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + expect(gitExecFileAsyncMock).toHaveBeenCalledWith(['remote', '-v'], { cwd: '/repo-root' }) }) it('keeps getRepoSlug origin-based on a fork checkout (#7331)', async () => { @@ -286,6 +294,10 @@ describe('getPRForBranch', () => { }) it('routes GHES push-target probes through the Enterprise host', async () => { + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'origin\thttps://github.acme-corp.com/team/orca.git (push)', + stderr: '' + }) const ghes = { owner: 'team', repo: 'orca', host: 'github.acme-corp.com' } resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ candidates: [ghes], @@ -307,7 +319,7 @@ describe('getPRForBranch', () => { }) }) - await expect(getPullRequestPushTarget('/repo-root', 7)).resolves.toEqual({ + await expect(getPullRequestPushTarget('/repo-root', 7)).resolves.toMatchObject({ pushTarget: { remoteName: 'origin', branchName: 'feature' } }) // Why: the candidate probe must pin options.host so the runner targets the @@ -342,7 +354,7 @@ describe('getPRForBranch', () => { }) }) - await expect(getPullRequestPushTarget('/repo-root', 7)).resolves.toEqual({ + await expect(getPullRequestPushTarget('/repo-root', 7)).resolves.toMatchObject({ pushTarget: { remoteName: 'pr-team-orca', branchName: 'feature', @@ -352,6 +364,10 @@ describe('getPRForBranch', () => { }) it('probes additional PR repo candidates when the first lookup is not found', async () => { + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'origin\thttps://github.com/fork/orca.git (push)', + stderr: '' + }) resolvePRRepositoryCandidatesMock.mockResolvedValueOnce({ candidates: [ { owner: 'fork', repo: 'orca' }, @@ -377,7 +393,7 @@ describe('getPRForBranch', () => { }) }) - await expect(getPullRequestPushTarget('/repo-root', 1849)).resolves.toEqual({ + await expect(getPullRequestPushTarget('/repo-root', 1849)).resolves.toMatchObject({ pushTarget: { remoteName: 'origin', branchName: 'feature/test' diff --git a/src/main/github/client/lookup/pull-request-push-target.ts b/src/main/github/client/lookup/pull-request-push-target.ts index 3dc19733c6c..162a40d384f 100644 --- a/src/main/github/client/lookup/pull-request-push-target.ts +++ b/src/main/github/client/lookup/pull-request-push-target.ts @@ -1,7 +1,10 @@ +import { readGitReviewPushAuthority } from '../../../../shared/git-review-push-authority' +import { requireSshGitProvider } from '../../../providers/ssh-git-dispatch' import type { IssueSourcePreference } from '../../../../shared/repo-types' import type { GitPushTarget } from '../../../../shared/worktree/types' import { ghExecFileAsync, + gitExecFileAsync, acquire, release, ghRepoExecOptions, @@ -122,13 +125,26 @@ export async function getPullRequestPushTarget( if (!owner || !repo || !branchName || !cloneUrl || !sshUrl) { return null } + const reviewHead = { + provider: 'github' as const, + host: matchedRepository.host ?? 'github.com', + repository: `${owner}/${repo}`, + branchName + } if ( origin && githubRepoIdentityKey(origin) === githubRepoIdentityKey({ owner, repo, host: matchedRepository.host }) ) { + const pushTarget = { remoteName: 'origin', branchName, reviewHead } + const runGit = connectionId + ? (args: string[]) => requireSshGitProvider(connectionId).exec(args, repoPath) + : (args: string[]) => gitExecFileAsync(args, { cwd: repoPath, ...localGitOptions }) + if ((await readGitReviewPushAuthority(runGit, pushTarget)).kind !== 'verified') { + return null + } return { - pushTarget: { remoteName: 'origin', branchName }, + pushTarget, ...(maintainerCanModify !== undefined ? { maintainerCanModify } : {}) } } @@ -142,6 +158,7 @@ export async function getPullRequestPushTarget( } return { pushTarget: { + reviewHead, remoteName: sanitizeRemoteName(owner, repo), branchName, remoteUrl: pickPushRemoteUrl({ originUrl, cloneUrl, sshUrl }) diff --git a/src/main/github/github-remote-identity-parsing.ts b/src/main/github/github-remote-identity-parsing.ts index 32c640b8796..02c3cbbab14 100644 --- a/src/main/github/github-remote-identity-parsing.ts +++ b/src/main/github/github-remote-identity-parsing.ts @@ -1,123 +1 @@ -import { normalizeGitHubRemoteHost } from '../../shared/git-remote-host-alias' -import type { GitHubOwnerRepo } from '../../shared/github/pull-request-types' - -export type GitHubRemoteIdentity = GitHubOwnerRepo & { host: string } - -// Why: HTTP ports identify the GHES web/API endpoint; SSH and git ports are -// transport-only and must not leak into gh's host identity. -function hostFromRemoteUrl(url: URL): string { - const protocol = url.protocol.toLowerCase() - return protocol === 'http:' || protocol === 'https:' ? url.host : url.hostname -} - -function parseGitHubRemotePath(path: string): Pick | null { - const parts = path.replace(/^\/+/, '').replace(/\/+$/, '').split('/') - if (parts.length !== 2) { - return null - } - const [owner, repoWithSuffix] = parts - const repo = repoWithSuffix.replace(/\.git$/i, '') - if (!owner || !repo) { - return null - } - return { owner, repo } -} - -/** SCP-style / ssh:// / git+ssh:// remotes may use an OpenSSH Host alias. */ -export function remoteUrlUsesSshTransport(remoteUrl: string): boolean { - const trimmed = remoteUrl.trim().toLowerCase() - return ( - trimmed.startsWith('git@') || trimmed.startsWith('ssh://') || trimmed.startsWith('git+ssh://') - ) -} - -/** SSH transport host as written, preserving SCP alias case. */ -export function rawSshTransportHost(remoteUrl: string): string | null { - const trimmed = remoteUrl.trim() - const scpMatch = trimmed.match(/^git@([^:]+):/i) - if (scpMatch) { - return scpMatch[1] - } - try { - const url = new URL(trimmed) - if (!['ssh:', 'git+ssh:'].includes(url.protocol.toLowerCase())) { - return null - } - return url.hostname || null - } catch { - return null - } -} - -/** Non-GitHub SSH host that may need OpenSSH alias expansion. */ -export function gitHubSshConfigHostAlias(remoteUrl: string): string | null { - if (!remoteUrlUsesSshTransport(remoteUrl)) { - return null - } - const identity = parseGitHubRemoteIdentity(remoteUrl) - if (!identity || identity.host === 'github.com') { - return null - } - return rawSshTransportHost(remoteUrl) ?? identity.host -} - -export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdentity | null { - const trimmed = remoteUrl.trim() - const sshMatch = trimmed.match(/^git@([^:]+):([^/]+)\/([^/]+?)(?:\.git)?$/i) - if (sshMatch) { - return { host: normalizeGitHubRemoteHost(sshMatch[1]), owner: sshMatch[2], repo: sshMatch[3] } - } - - try { - const url = new URL(trimmed) - if (!['git:', 'git+ssh:', 'http:', 'https:', 'ssh:'].includes(url.protocol.toLowerCase())) { - return null - } - const path = parseGitHubRemotePath(url.pathname) - return path ? { host: normalizeGitHubRemoteHost(hostFromRemoteUrl(url)), ...path } : null - } catch { - return null - } -} - -export function parseGitHubOwnerRepo(remoteUrl: string): GitHubOwnerRepo | null { - const identity = parseGitHubRemoteIdentity(remoteUrl) - if (!identity || identity.host.toLowerCase() !== 'github.com') { - return null - } - return { owner: identity.owner, repo: identity.repo } -} - -/** Parse github.com identity using an expanded SSH HostName. */ -export function parseGitHubOwnerRepoWithResolvedSshHostname( - remoteUrl: string, - resolvedSshHostname: string | null | undefined -): GitHubOwnerRepo | null { - const direct = parseGitHubOwnerRepo(remoteUrl) - if (direct) { - return direct - } - if (!remoteUrlUsesSshTransport(remoteUrl)) { - return null - } - if (!resolvedSshHostname?.trim()) { - return null - } - const identity = parseGitHubRemoteIdentity(remoteUrl) - if (!identity) { - return null - } - if (normalizeGitHubRemoteHost(resolvedSshHostname.trim()) !== 'github.com') { - return null - } - return { owner: identity.owner, repo: identity.repo } -} - -/** Effective forge host after optional SSH config HostName expansion. */ -export function effectiveGitHubRemoteHost( - parsedHost: string, - resolvedSshHostname?: string | null -): string { - const candidate = resolvedSshHostname?.trim() || parsedHost - return normalizeGitHubRemoteHost(candidate) -} +export * from '../../shared/github/remote-identity-parsing' diff --git a/src/main/github/pr-start-point-compare-base.test.ts b/src/main/github/pr-start-point-compare-base.test.ts index 7a6b4e458a7..42f2824d292 100644 --- a/src/main/github/pr-start-point-compare-base.test.ts +++ b/src/main/github/pr-start-point-compare-base.test.ts @@ -132,8 +132,7 @@ describe('resolveGitHubPrStartPoint compare base', () => { baseBranch: 'same-repo-head-sha', compareBaseRef: 'refs/remotes/origin/main', headSha: 'same-repo-head-sha', - branchNameOverride: 'feature/fix', - pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } + branchNameOverride: 'feature/fix' }) }) @@ -164,8 +163,7 @@ describe('resolveGitHubPrStartPoint compare base', () => { expect(result).toEqual({ baseBranch: 'same-repo-head-sha', headSha: 'same-repo-head-sha', - branchNameOverride: 'feature/fix', - pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } + branchNameOverride: 'feature/fix' }) }) }) diff --git a/src/main/github/pr-start-point.test.ts b/src/main/github/pr-start-point.test.ts index 9669ef29a93..84d020759d1 100644 --- a/src/main/github/pr-start-point.test.ts +++ b/src/main/github/pr-start-point.test.ts @@ -469,6 +469,9 @@ describe('resolveGitHubPrStartPoint', () => { }) it('returns the verified head SHA, branch override, and push target when same-repo branch fetch succeeds', async () => { + getPullRequestPushTargetMock.mockResolvedValue({ + pushTarget: { remoteName: 'origin', branchName: 'feature/add-feature' } + }) const fetchRemoteTrackingRef = vi.fn(async () => {}) const gitExec = vi.fn(async (args: string[]) => { const url = remoteGetUrl(args) diff --git a/src/main/github/pr-start-point.ts b/src/main/github/pr-start-point.ts index 118cbc474ef..f052db4225a 100644 --- a/src/main/github/pr-start-point.ts +++ b/src/main/github/pr-start-point.ts @@ -84,9 +84,7 @@ export async function resolveGitHubPrStartPoint( } } - if (isCrossRepository) { - await resolvePushTarget() - } + await resolvePushTarget() let remote: string try { @@ -243,6 +241,6 @@ export async function resolveGitHubPrStartPoint( ...(compareBaseFetched && compareBaseRef ? { compareBaseRef } : {}), headSha, branchNameOverride: headRefName, - pushTarget: { remoteName: remote, branchName: headRefName } + ...(pushTarget ? { pushTarget } : {}) } } diff --git a/src/main/github/review-push-endpoint-real.test.ts b/src/main/github/review-push-endpoint-real.test.ts new file mode 100644 index 00000000000..70de21374d0 --- /dev/null +++ b/src/main/github/review-push-endpoint-real.test.ts @@ -0,0 +1,228 @@ +import { mkdtemp, readFile, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { createServer } from 'node:net' +import { once } from 'node:events' +import { afterAll, beforeAll, expect, it, vi } from 'vitest' +import { spawnProcess } from '../../shared/child-process/run-process' +import { gitExecFileAsync as realGit } from '../git/command-runner/git-exec-file' +import { getPublishTargetStatus } from '../../shared/git-publish-target-status' +import { hasUsableHostedReviewPushTarget } from '../../shared/hosted-review-push-target-admission' +import { resolveRelayPushTarget } from '../../relay/git-handler-push-target' + +const state = vi.hoisted(() => ({ + root: '', + endpoint: '', + env: {} as NodeJS.ProcessEnv, + pushes: [] as string[] +})) +vi.mock('./gh-utils', () => ({ + acquire: async () => {}, + release: () => {}, + githubRepoContext: () => ({}), + ghRepoExecOptions: () => ({}), + getRemoteUrlForRepo: async () => state.endpoint, + gitExecFileAsync: (args: string[]) => + realGit(args, { cwd: state.root, env: state.env, timeout: 10_000 }), + ghExecFileAsync: async () => ({ + stdout: JSON.stringify({ + head: { + ref: 'feature', + repo: { + name: 'repo', + owner: { login: 'team' }, + clone_url: state.endpoint, + ssh_url: state.endpoint + } + } + }) + }) +})) +vi.mock('./github-api-repository', () => ({ + getGitHubApiRepositoryForRemote: async () => ({ host: '127.0.0.1', owner: 'team', repo: 'repo' }), + githubHostExecOptions: () => ({}) +})) +vi.mock('./client/pull-request-lookup-candidates', () => ({ + resolvePullRequestLookupCandidates: async () => [ + { host: '127.0.0.1', owner: 'team', repo: 'repo' } + ] +})) +vi.mock('./client', async () => ({ + getPullRequestPushTarget: (await import('./client/lookup/pull-request-push-target')) + .getPullRequestPushTarget, + getWorkItem: async () => ({ type: 'pr', branchName: 'feature' }) +})) +vi.mock('../git/runner', () => ({ + gitExecFileAsync: async (args: string[]) => { + const result = await realGit( + args[0] === 'push' ? ['push', '--dry-run', '--porcelain', ...args.slice(1)] : args, + { cwd: state.root, env: state.env, timeout: 10_000 } + ) + if (args[0] === 'push') { + state.pushes.push(result.stdout) + } + return result + } +})) +import { getPullRequestPushTarget } from './client/lookup/pull-request-push-target' +import { resolveGitHubPrStartPoint } from './pr-start-point' +import { gitPush } from '../git/remote' + +let root: string +let daemon: ReturnType +let other: string +const run = (args: string[]) => realGit(args, { cwd: root, env: state.env, timeout: 10_000 }) +const git = async (...args: string[]) => (await run(args)).stdout.trim() +const refs = () => + Promise.all( + [root, join(root, 'team/repo.git'), join(root, 'other/repo.git')].map( + async (cwd) => + ( + await realGit(['for-each-ref', '--format=%(refname) %(objectname)'], { + cwd, + env: state.env + }) + ).stdout + ) + ) + +beforeAll(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-push-authority-')) + state.root = root + state.env = { + ...process.env, + HOME: root, + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_GLOBAL: join(root, 'empty-config'), + ORCA_BACKGROUND_LAUNCH: '1' + } + await git('init', '-q') + await git( + '-c', + 'user.name=Fixture', + '-c', + 'user.email=fixture@example.invalid', + '-c', + 'commit.gpgSign=false', + 'commit', + '--allow-empty', + '-qm', + 'base' + ) + await git('branch', '-M', 'feature') + for (const path of ['team/repo.git', 'other/repo.git']) { + await git('clone', '--bare', '-q', root, join(root, path)) + } + const server = createServer() + server.listen(0, '127.0.0.1') + await once(server, 'listening') + const port = (server.address() as { port: number }).port + await new Promise((resolve) => server.close(() => resolve())) + daemon = spawnProcess({ + program: 'git', + args: [ + 'daemon', + '--verbose', + '--export-all', + '--enable=receive-pack', + '--listen=127.0.0.1', + `--port=${port}`, + `--base-path=${root}`, + root + ], + cwd: root, + env: state.env + }) + await new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new Error('Git daemon startup timed out')), 10_000) + daemon.once('error', reject) + daemon.stderr.on('data', (chunk) => { + if (String(chunk).includes('Ready to rumble')) { + clearTimeout(timer) + resolve() + } + }) + }) + state.endpoint = `git://127.0.0.1:${port}/team/repo.git` + other = `git://127.0.0.1:${port}/other/repo.git` + await git('remote', 'add', 'origin', state.endpoint) + await git('fetch', '-q', 'origin') +}) +afterAll(async () => { + if (daemon) { + expect(daemon.spawnargs).toContain(root) + const closed = once(daemon, 'close') + daemon.kill('SIGTERM') + await closed + } + await rm(root, { recursive: true, force: true }) +}) + +it('carries shipping hydrated identity through status, admission, local and relay execution', async () => { + const resolved = await resolveGitHubPrStartPoint({ + repoPath: root, + prNumber: 42, + gitExec: run, + resolveRemote: async () => 'origin', + fetchRemoteTrackingRef: async () => { + await git('fetch', '-q', 'origin') + }, + fetchPullRequestHeadRef: async () => { + throw new Error('unexpected fork fetch') + } + }) + expect(resolved).not.toHaveProperty('error') + if ('error' in resolved || !resolved.pushTarget) { + throw new Error('Missing hydrated target') + } + const target = resolved.pushTarget + const status = await getPublishTargetStatus(run, target) + expect( + hasUsableHostedReviewPushTarget({ + pushTarget: target, + upstreamStatus: status, + hasResolvableHostedReviewPushTargetLink: true + }) + ).toBe(true) + const before = await refs() + const config = await readFile(join(root, '.git/config')) + await gitPush(root, false, target) + expect(state.pushes.at(-1)).toContain(state.endpoint) + const relay = await resolveRelayPushTarget((args) => run(args), root, target) + await git('push', '--dry-run', '--porcelain', relay!.remote, relay!.refspec) + expect(await refs()).toEqual(before) + expect(await readFile(join(root, '.git/config'))).toEqual(config) + + for (const urls of [[other], [state.endpoint, other]]) { + await git('config', '--replace-all', 'remote.origin.pushurl', urls[0]!) + if (urls[1]) { + await git('config', '--add', 'remote.origin.pushurl', urls[1]) + } + const configBefore = await readFile(join(root, '.git/config')) + expect(await getPullRequestPushTarget(root, 42)).toBeNull() + const denied = await getPublishTargetStatus(run, target) + expect( + hasUsableHostedReviewPushTarget({ + pushTarget: target, + upstreamStatus: denied, + hasResolvableHostedReviewPushTargetLink: true + }) + ).toBe(false) + await expect(gitPush(root, false, target)).rejects.toThrow('authority') + await expect(resolveRelayPushTarget((args) => run(args), root, target)).rejects.toThrow( + 'authority' + ) + const control = await git( + 'push', + '--dry-run', + '--porcelain', + 'origin', + 'HEAD:refs/heads/feature' + ) + for (const url of urls) { + expect(control).toContain(url) + } + expect(await refs()).toEqual(before) + expect(await readFile(join(root, '.git/config'))).toEqual(configBefore) + } +}) diff --git a/src/main/gitlab/mappers.ts b/src/main/gitlab/mappers.ts index 645392ef2cb..5e96437bad2 100644 --- a/src/main/gitlab/mappers.ts +++ b/src/main/gitlab/mappers.ts @@ -245,6 +245,11 @@ export function mapMRToWorkItem( labels, updatedAt: data.updated_at ?? '', author: data.author?.username ?? null, + ...(projectRef && + data.source_project_id !== undefined && + data.source_project_id === data.target_project_id + ? { headProjectRef: projectRef } + : {}), branchName: data.source_branch, baseRefName: data.target_branch, isCrossRepository: diff --git a/src/main/gitlab/project-ref-parser.ts b/src/main/gitlab/project-ref-parser.ts index 575e6093179..84d7459a01d 100644 --- a/src/main/gitlab/project-ref-parser.ts +++ b/src/main/gitlab/project-ref-parser.ts @@ -1,128 +1 @@ -import type { GitLabProjectRef } from '../../shared/gitlab-types' - -export type ProjectRef = GitLabProjectRef - -/** - * Hosts always treated as GitLab. Self-hosted instances are added at - * runtime via `getGlabKnownHosts()`, which inspects `glab auth status`. - */ -export const DEFAULT_GITLAB_HOSTS = ['gitlab.com'] as const - -export function normalizeGitLabHost(value: string): string { - return value.trim().toLowerCase() -} - -// Why: host recognition is port-aware so two services on the same hostname -// but different ports (e.g. a GitLab on :8080 and a Gitea on :3030) are not -// conflated. The hostname (port-less) part is kept for legacy known-host -// entries that were recorded without a port. -function hostnameOf(host: string): string { - // `host` may be `name` or `name:port`. Strip a trailing `:digits` port. - return host.replace(/:\d+$/, '') -} - -function stripGitSuffix(path: string): string { - return path.replace(/\/+$/, '').replace(/\.git$/i, '') -} - -// Why: the GitLab host identity is the web/API endpoint, which is what `glab -// --hostname` and the known-hosts list speak in terms of. For http(s) -// remotes the URL port IS that endpoint port (e.g. self-hosted on :8080), -// so it must be kept. For ssh/git remotes the port is a transport port -// (e.g. ssh on :2222) that does not identify the GitLab instance, so it is -// dropped and only the hostname is used. -function hostIdentityFromUrl(url: URL): string { - const protocol = url.protocol.toLowerCase() - if (protocol === 'http:' || protocol === 'https:') { - return url.host - } - return url.hostname -} - -function makeProjectRefForTrustedHost(host: string, path: string): ProjectRef | null { - const normalizedHost = normalizeGitLabHost(host) - const normalizedPath = stripGitSuffix(path.replace(/^\/+/, '')).trim() - // Reject paths without at least one group segment — `gitlab.com:foo` - // alone is not a project reference. - if (!normalizedPath.includes('/')) { - return null - } - return { host: normalizedHost, path: normalizedPath } -} - -/** - * Does `urlHost` (which may include a `:port`) match a known-host entry? - * - An exact match (including any port) always counts. - * - A known entry WITHOUT a port also matches a URL host on the same - * hostname regardless of the URL's port — this preserves recognition for - * legacy `gitlab.com` / bare-hostname known entries. - * - A known entry WITH a port only matches a URL host with the exact same - * port, so `gitlab.example.com:8443` does not accept a - * `gitea.example.com:3000` (or same-host different-port) remote. - */ -function knownHostMatches(urlHost: string, knownHost: string): boolean { - if (urlHost === knownHost) { - return true - } - if (hostnameOf(knownHost) === knownHost) { - // Known entry has no port — match on hostname alone. - return hostnameOf(urlHost) === knownHost - } - return false -} - -function makeProjectRef( - host: string, - path: string, - knownHosts: readonly string[] -): ProjectRef | null { - const normalizedHost = normalizeGitLabHost(host) - const normalizedKnownHosts = knownHosts.map(normalizeGitLabHost) - if (!normalizedKnownHosts.some((knownHost) => knownHostMatches(normalizedHost, knownHost))) { - return null - } - return makeProjectRefForTrustedHost(normalizedHost, path) -} - -export function parseRemoteProjectRefCandidate(remoteUrl: string): ProjectRef | null { - const trimmed = remoteUrl.trim() - if (!/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed)) { - const scpLike = trimmed.match(/^(?:[^@/:]+@)?([^:\s/]+):([^\s]+?)(?:\.git)?$/) - if (scpLike) { - return makeProjectRefForTrustedHost(scpLike[1], scpLike[2]) - } - } - - try { - const url = new URL(trimmed) - if (!['http:', 'https:', 'ssh:', 'git:', 'git+ssh:'].includes(url.protocol.toLowerCase())) { - return null - } - return makeProjectRefForTrustedHost(hostIdentityFromUrl(url), url.pathname) - } catch { - return null - } -} - -export function parseGitLabProjectRef( - remoteUrl: string, - knownHosts: readonly string[] = DEFAULT_GITLAB_HOSTS -): ProjectRef | null { - const trimmed = remoteUrl.trim() - if (!/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed)) { - const scpLike = trimmed.match(/^(?:[^@/:]+@)?([^:\s/]+):([^\s]+?)(?:\.git)?$/) - if (scpLike) { - return makeProjectRef(scpLike[1], scpLike[2], knownHosts) - } - } - - try { - const url = new URL(trimmed) - if (!['http:', 'https:', 'ssh:', 'git:', 'git+ssh:'].includes(url.protocol.toLowerCase())) { - return null - } - return makeProjectRef(hostIdentityFromUrl(url), url.pathname, knownHosts) - } catch { - return null - } -} +export * from '../../shared/gitlab-project-ref-parser' diff --git a/src/main/ipc/filesystem/git-remote/branch-mutation-handlers.ts b/src/main/ipc/filesystem/git-remote/branch-mutation-handlers.ts index 2c3273b8c00..a4b83dd18e8 100644 --- a/src/main/ipc/filesystem/git-remote/branch-mutation-handlers.ts +++ b/src/main/ipc/filesystem/git-remote/branch-mutation-handlers.ts @@ -1,3 +1,4 @@ +import { resolveStoredReviewPushTarget } from '../../worktree-review-push-target' import { ipcMain } from 'electron' import type { GitPushTarget } from '../../../../shared/worktree/types' import { gitFastForward, gitPull, gitPullRebaseFromBase, gitPush } from '../../../git/remote' @@ -33,6 +34,7 @@ export function registerGitRemoteBranchMutationHandlers(context: FilesystemHandl ): Promise => { // Why: coerce to strict boolean so a malformed payload (e.g. string 'false') can't enable --set-upstream; mirror in src/relay/git-handler.ts. const publish = args.publish === true + args = { ...args, pushTarget: resolveStoredReviewPushTarget(store, args) } if (args.connectionId) { if (args.pushTarget) { assertGitPushTargetShape(args.pushTarget) diff --git a/src/main/ipc/worktree-push-target-setup.ts b/src/main/ipc/worktree-push-target-setup.ts index c4a82d94933..d6731c6275d 100644 --- a/src/main/ipc/worktree-push-target-setup.ts +++ b/src/main/ipc/worktree-push-target-setup.ts @@ -1,3 +1,4 @@ +import { assertGitReviewPushAuthority } from '../../shared/git-review-push-authority' // Why: preparing a fork-PR push target means adding (or reusing) the contributor's // fork as a git remote, fetching the head, and wiring the new branch's upstream. // The git-driven core lives here behind an injectable `execGit` seam so the @@ -109,6 +110,12 @@ export async function prepareWorktreePushTargetWithExec( const existingRemote = await findRemoteForUrl(execGit, repoPath, target.remoteUrl) if (existingRemote) { remoteName = existingRemote + if (target.reviewHead) { + await assertGitReviewPushAuthority((args) => execGit(args, repoPath), { + ...target, + remoteName + }) + } // Why: if a later PR worktree reuses an Orca-created fork remote, it // must inherit ownership so deleting the final user can remove it. remoteCreated = isRemoteCreatedByKnownWorktree(existingRemote) @@ -144,6 +151,12 @@ export async function prepareWorktreePushTargetWithExec( } try { + if (target.reviewHead) { + await assertGitReviewPushAuthority((args) => execGit(args, repoPath), { + ...target, + remoteName + }) + } await execGit( ['fetch', remoteName, buildNarrowForkFetchRefspec(remoteName, target.branchName)], repoPath diff --git a/src/main/ipc/worktree-review-push-target.test.ts b/src/main/ipc/worktree-review-push-target.test.ts new file mode 100644 index 00000000000..fe91fe02b44 --- /dev/null +++ b/src/main/ipc/worktree-review-push-target.test.ts @@ -0,0 +1,38 @@ +import { expect, it, vi } from 'vitest' +import { resolveStoredReviewPushTarget } from './worktree-review-push-target' +import { reviewTarget } from '../../shared/__fixtures__/git-review-target' +import type { Store } from '../persistence' + +it('rejects missing and superseded targets at the local IPC metadata boundary', () => { + const meta = { + linkedPR: 42, + pushTarget: undefined as ReturnType | undefined + } + const store = { + getAllWorktreeMetaForHost: vi.fn(() => ({ 'repo::/repo/wt': meta })) + } as unknown as Store + expect(() => resolveStoredReviewPushTarget(store, { worktreePath: '/repo/wt' })).toThrow( + 'unresolved' + ) + meta.pushTarget = reviewTarget('origin', 'feature') + expect(resolveStoredReviewPushTarget(store, { worktreePath: '/repo/wt' })).toEqual( + meta.pushTarget + ) + expect(() => + resolveStoredReviewPushTarget(store, { + worktreePath: '/repo/wt', + pushTarget: reviewTarget('fork', 'feature') + }) + ).toThrow('changed') + expect(store.getAllWorktreeMetaForHost).toHaveBeenCalledWith('local') +}) + +it('reads SSH-owned metadata without substituting the local row', () => { + const store = { + getAllWorktreeMetaForHost: vi.fn(() => ({ 'repo::/repo/wt': { linkedGitLabMR: 42 } })) + } as unknown as Store + expect(() => + resolveStoredReviewPushTarget(store, { worktreePath: '/repo/wt', connectionId: 'host' }) + ).toThrow('unresolved') + expect(store.getAllWorktreeMetaForHost).toHaveBeenCalledWith('ssh:host') +}) diff --git a/src/main/ipc/worktree-review-push-target.ts b/src/main/ipc/worktree-review-push-target.ts new file mode 100644 index 00000000000..79a9d1b9710 --- /dev/null +++ b/src/main/ipc/worktree-review-push-target.ts @@ -0,0 +1,27 @@ +import { resolve } from 'node:path' +import { LOCAL_EXECUTION_HOST_ID, toSshExecutionHostId } from '../../shared/execution-host' +import { splitWorktreeId } from '../../shared/worktree/id' +import { linkedReviewOperationTarget } from '../../shared/linked-review-operation-target' +import type { GitPushTarget } from '../../shared/worktree/types' +import { readAllWorktreeMetaForHost } from '../persistence/host-qualified-worktree-meta' +import type { Store } from '../persistence' + +export function resolveStoredReviewPushTarget( + store: Store, + args: { + worktreePath: string + connectionId?: string + pushTarget?: GitPushTarget + } +): GitPushTarget | undefined { + const host = args.connectionId ? toSshExecutionHostId(args.connectionId) : LOCAL_EXECUTION_HOST_ID + const comparable = (path: string): string => (args.connectionId ? path : resolve(path)) + const entries = Object.entries(readAllWorktreeMetaForHost(store, host)).filter(([id]) => { + const parsed = splitWorktreeId(id) + return parsed && comparable(parsed.worktreePath) === comparable(args.worktreePath) + }) + if (entries.length > 1) { + throw new Error('Review push workspace ownership is ambiguous.') + } + return linkedReviewOperationTarget(entries[0]?.[1], args.pushTarget) +} diff --git a/src/main/ipc/worktrees-wsl-runtime-routing.test.ts b/src/main/ipc/worktrees-wsl-runtime-routing.test.ts index 8c936764291..20f1ff20fd9 100644 --- a/src/main/ipc/worktrees-wsl-runtime-routing.test.ts +++ b/src/main/ipc/worktrees-wsl-runtime-routing.test.ts @@ -463,8 +463,7 @@ describe('registerWorktreeHandlers', () => { expect(result).toMatchObject({ baseBranch: 'def456', headSha: 'def456', - branchNameOverride: 'feature/add-feature', - pushTarget: { remoteName: 'origin', branchName: 'feature/add-feature' } + branchNameOverride: 'feature/add-feature' }) }) diff --git a/src/main/providers/ssh-git-provider-remote-sync.test.ts b/src/main/providers/ssh-git-provider-remote-sync.test.ts index 4a25d54b8a6..743f2ab1806 100644 --- a/src/main/providers/ssh-git-provider-remote-sync.test.ts +++ b/src/main/providers/ssh-git-provider-remote-sync.test.ts @@ -1,3 +1,4 @@ +import { reviewTarget } from '../../shared/__fixtures__/git-review-target' import { describe, expect, it, beforeEach } from 'vitest' import { SshGitProvider } from './ssh-git-provider' import { createMockMux, type MockMultiplexer } from './ssh-git-provider-test-harness' @@ -38,17 +39,19 @@ describe('SshGitProvider', () => { }) it('pushBranch sends git.push request and forwards publish mode and target', async () => { - await provider.pushBranch('/home/user/repo', true, { - remoteName: 'pr-fork-orca', - branchName: 'contributor/fix' + mux.request.mockResolvedValue({ + stdout: 'pr-fork-orca\thttps://github.com/team/repo.git (push)', + stderr: '' }) + await provider.pushBranch( + '/home/user/repo', + true, + reviewTarget('pr-fork-orca', 'contributor/fix') + ) expect(mux.request).toHaveBeenCalledWith('git.push', { worktreePath: '/home/user/repo', publish: true, - pushTarget: { - remoteName: 'pr-fork-orca', - branchName: 'contributor/fix' - } + pushTarget: reviewTarget('pr-fork-orca', 'contributor/fix') }) }) diff --git a/src/main/providers/ssh-git-remote-sync-provider.ts b/src/main/providers/ssh-git-remote-sync-provider.ts index bead9414f3b..83725f30b83 100644 --- a/src/main/providers/ssh-git-remote-sync-provider.ts +++ b/src/main/providers/ssh-git-remote-sync-provider.ts @@ -1,3 +1,5 @@ +import { requestGitStreamable } from '../ssh/ssh-git-response-stream-reader' +import { assertGitReviewPushAuthority } from '../../shared/git-review-push-authority' import type { GitForkSyncExpectedUpstream, GitForkSyncResult } from '../../shared/git-fork-sync' import type { GitPushTarget } from '../../shared/worktree/types' import { REBASE_FROM_BASE_RPC_TIMEOUT_MS } from '../../shared/git-rebase-source' @@ -11,6 +13,15 @@ export class SshGitRemoteSyncProvider extends SshGitWorkingTreeProvider { options: { forceWithLease?: boolean } = {} ): Promise { await this.runWithGitReadInvalidation(async () => { + if (pushTarget) { + await assertGitReviewPushAuthority( + async (args) => + (await requestGitStreamable(this.mux, 'git.exec', { args, cwd: worktreePath })) as { + stdout: string + }, + pushTarget + ) + } await this.mux.request('git.push', { worktreePath, publish, diff --git a/src/main/runtime/orca-runtime-tests/gitlab-and-pr-bases-part-03.spec.ts b/src/main/runtime/orca-runtime-tests/gitlab-and-pr-bases-part-03.spec.ts index 056d951e8df..6214c6697cf 100644 --- a/src/main/runtime/orca-runtime-tests/gitlab-and-pr-bases-part-03.spec.ts +++ b/src/main/runtime/orca-runtime-tests/gitlab-and-pr-bases-part-03.spec.ts @@ -122,8 +122,7 @@ describe('OrcaRuntimeService', () => { expect(result).toEqual({ baseBranch: 'origin/feature/fix', - compareBaseRef: 'refs/remotes/origin/main', - pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } + compareBaseRef: 'refs/remotes/origin/main' }) expect(provider.fetchRemoteTrackingRef).toHaveBeenCalledWith( '/remote/repo', @@ -191,8 +190,7 @@ describe('OrcaRuntimeService', () => { }) expect(result).toEqual({ - baseBranch: 'origin/feature/fix', - pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } + baseBranch: 'origin/feature/fix' }) expect(result).not.toHaveProperty('compareBaseRef') expect(result).not.toHaveProperty('error') @@ -252,8 +250,7 @@ describe('OrcaRuntimeService', () => { expect(result).toEqual({ baseBranch: 'origin/feature/fix', - compareBaseRef: 'refs/remotes/origin/main', - pushTarget: { remoteName: 'origin', branchName: 'feature/fix' } + compareBaseRef: 'refs/remotes/origin/main' }) } finally { warnSpy.mockRestore() diff --git a/src/main/runtime/rpc/methods/git-params.ts b/src/main/runtime/rpc/methods/git-params.ts index f69b01cd053..4822a31e238 100644 --- a/src/main/runtime/rpc/methods/git-params.ts +++ b/src/main/runtime/rpc/methods/git-params.ts @@ -1,3 +1,4 @@ +import { GitPushTargetParam } from './git-push-target-schema' import { z } from 'zod' import { OptionalGitAdmissionTier } from './git-admission-tier-schema' @@ -206,13 +207,6 @@ export const GitBulkPaths = WorktreeSelector.extend({ filePaths: z.array(z.string().min(1, 'Missing file path')) }) -const GitPushTargetParam = z.object({ - remoteName: z.string(), - branchName: z.string(), - remoteUrl: z.string().optional(), - remoteCreated: z.boolean().optional() -}) - export const GitPush = WorktreeSelector.extend({ publish: z.boolean().optional(), forceWithLease: z.boolean().optional(), diff --git a/src/main/runtime/rpc/methods/git-push-target-schema.ts b/src/main/runtime/rpc/methods/git-push-target-schema.ts new file mode 100644 index 00000000000..9695995d820 --- /dev/null +++ b/src/main/runtime/rpc/methods/git-push-target-schema.ts @@ -0,0 +1,16 @@ +import { z } from 'zod' + +export const GitPushTargetParam = z.object({ + remoteName: z.string(), + branchName: z.string(), + remoteUrl: z.string().optional(), + remoteCreated: z.boolean().optional(), + reviewHead: z + .object({ + provider: z.enum(['github', 'gitlab']), + host: z.string().min(1), + repository: z.string().min(1), + branchName: z.string().min(1) + }) + .optional() +}) diff --git a/src/main/runtime/rpc/methods/git-review-target-schema.test.ts b/src/main/runtime/rpc/methods/git-review-target-schema.test.ts new file mode 100644 index 00000000000..b8aafeb49ad --- /dev/null +++ b/src/main/runtime/rpc/methods/git-review-target-schema.test.ts @@ -0,0 +1,14 @@ +import { expect, it } from 'vitest' +import { GitPush } from './git-params' +import { WorktreeSet } from './worktree-schemas' +import { WorktreeCreate } from './worktree-create-schemas' +import { reviewTarget } from '../../../../shared/__fixtures__/git-review-target' + +it('preserves provider head identity through push, metadata and creation RPC schemas', () => { + const pushTarget = reviewTarget('origin', 'feature') + expect(GitPush.parse({ worktree: 'id:wt', pushTarget }).pushTarget).toEqual(pushTarget) + expect(WorktreeSet.parse({ worktree: 'id:wt', pushTarget }).pushTarget).toEqual(pushTarget) + expect( + WorktreeCreate.parse({ repo: 'id:repo', branch: 'feature', pushTarget }).pushTarget + ).toEqual(pushTarget) +}) diff --git a/src/main/runtime/rpc/methods/worktree-create-schemas.ts b/src/main/runtime/rpc/methods/worktree-create-schemas.ts index 61f6eb65e35..71e1961ca8a 100644 --- a/src/main/runtime/rpc/methods/worktree-create-schemas.ts +++ b/src/main/runtime/rpc/methods/worktree-create-schemas.ts @@ -1,3 +1,4 @@ +import { GitPushTargetParam } from './git-push-target-schema' import { z } from 'zod' import { isTuiAgent } from '../../../../shared/tui-agent-config' import { workspaceSourceSchema } from '../../../../shared/telemetry-events' @@ -61,13 +62,7 @@ export const WorktreeCreate = z presetId: OptionalString }) .optional(), - pushTarget: z - .object({ - remoteName: z.string(), - branchName: z.string(), - remoteUrl: OptionalString - }) - .optional(), + pushTarget: GitPushTargetParam.optional(), runHooks: OptionalBoolean, activate: OptionalBoolean, // Why: activation on create is view intent, so it is addressed like worktree.activate. diff --git a/src/main/runtime/rpc/methods/worktree-schemas.ts b/src/main/runtime/rpc/methods/worktree-schemas.ts index b7c3b9493a9..2dfe79bf530 100644 --- a/src/main/runtime/rpc/methods/worktree-schemas.ts +++ b/src/main/runtime/rpc/methods/worktree-schemas.ts @@ -1,3 +1,4 @@ +import { GitPushTargetParam } from './git-push-target-schema' import { z } from 'zod' import { isTuiAgent } from '../../../../shared/tui-agent-config' import type { TuiAgent } from '../../../../shared/tui-agent' @@ -142,14 +143,7 @@ export const WorktreeSet = WorktreeSelector.extend({ sparsePresetId: OptionalString, baseRef: OptionalString, workspaceStatus: OptionalString, - pushTarget: z - .object({ - remoteName: z.string(), - branchName: z.string(), - remoteUrl: OptionalString - }) - .nullable() - .optional(), + pushTarget: GitPushTargetParam.nullable().optional(), diffComments: z.array(z.unknown()).optional(), mobileDiffReview: z.unknown().optional(), parentWorktree: OptionalString, diff --git a/src/main/runtime/runtime-git-sync-commands.ts b/src/main/runtime/runtime-git-sync-commands.ts index 542d459f743..af6efa5cec5 100644 --- a/src/main/runtime/runtime-git-sync-commands.ts +++ b/src/main/runtime/runtime-git-sync-commands.ts @@ -1,3 +1,4 @@ +import { linkedReviewOperationTarget } from '../../shared/linked-review-operation-target' import type { GitForkSyncExpectedUpstream, GitForkSyncResult } from '../../shared/git-fork-sync' import type { GitUpstreamStatus } from '../../shared/git-status-types' import type { GitPushTarget } from '../../shared/worktree/types' @@ -201,6 +202,7 @@ export class RuntimeGitSyncCommands { forceWithLease?: boolean ): Promise<{ ok: true }> { const target = await this.host.resolveRuntimeGitTarget(worktreeSelector) + pushTarget = linkedReviewOperationTarget(target.worktree, pushTarget) const provider = requireRuntimeGitProvider(target) if (provider) { const materializedPushTarget = pushTarget diff --git a/src/main/runtime/runtime-gitlab-push-authority.test.ts b/src/main/runtime/runtime-gitlab-push-authority.test.ts new file mode 100644 index 00000000000..43366a86f3b --- /dev/null +++ b/src/main/runtime/runtime-gitlab-push-authority.test.ts @@ -0,0 +1,69 @@ +import { expect, it, vi } from 'vitest' +import type { Store } from '../persistence' +import type { Repo } from '../../shared/repo-types' +import { mapMRToWorkItem } from '../gitlab/mappers' +const mocks = vi.hoisted(() => ({ git: vi.fn(), item: vi.fn() })) +vi.mock('../git/runner', () => ({ gitExecFileAsync: mocks.git })) +vi.mock('../gitlab/client', () => ({ + getProjectRefForRemote: async () => ({ host: 'gitlab.com', path: 'team/repo' }), + getWorkItemByProjectRef: mocks.item +})) +vi.mock('../gitlab/gl-utils', () => ({ getGlabKnownHosts: async () => ['gitlab.com'] })) +vi.mock('../project-runtime-git-options', () => ({ + getLocalProjectGitExecOptions: () => ({}), + getLocalProjectWorktreeGitOptions: () => ({}) +})) +vi.mock('./runtime-gitlab-issue-source-remote', () => ({ + resolveRuntimeGitLabIssueSourceRemote: async () => 'origin' +})) +import { resolveRuntimeGitLabWorktreeBase } from './runtime-gitlab-worktree-base' + +it.each([ + ['git@gitlab.com:team/repo.git', true], + ['git@gitlab.com:other/repo.git', false], + ['git@gitlab.com:team/repo.git\norigin\tgit@gitlab.com:other/repo.git', false] +])('binds same-project GitLab hydration to all execution endpoints %s', async (urls, accepted) => { + mocks.item.mockResolvedValue( + mapMRToWorkItem( + { + title: 'Review', + source_branch: 'feature', + target_branch: 'main', + source_project_id: 7, + target_project_id: 7 + }, + 'repo', + { host: 'gitlab.com', path: 'team/repo' } + ) + ) + mocks.git.mockImplementation(async (args: string[]) => ({ + stdout: + args[0] === 'remote' + ? urls + .split('\n') + .map((url, index) => `${index === 0 ? 'origin\t' : ''}${url} (push)`) + .join('\n') + : 'oid', + stderr: '' + })) + const result = await resolveRuntimeGitLabWorktreeBase( + { repoSelector: 'repo', mrIid: 42 }, + { + store: {} as Store, + resolveRepo: async () => ({ id: 'repo', path: '/repo' }) as Repo + } + ) + expect(result).not.toHaveProperty('error') + if ('error' in result) { + throw new Error(result.error) + } + expect(!!result.pushTarget).toBe(accepted) + if (accepted) { + expect(result.pushTarget?.reviewHead).toEqual({ + provider: 'gitlab', + host: 'gitlab.com', + repository: 'team/repo', + branchName: 'feature' + }) + } +}) diff --git a/src/main/runtime/runtime-gitlab-worktree-base.ts b/src/main/runtime/runtime-gitlab-worktree-base.ts index 37649ec4085..b7d513c22d4 100644 --- a/src/main/runtime/runtime-gitlab-worktree-base.ts +++ b/src/main/runtime/runtime-gitlab-worktree-base.ts @@ -1,3 +1,4 @@ +import { readGitReviewPushAuthority } from '../../shared/git-review-push-authority' import type { GitPushTarget } from '../../shared/worktree/types' import type { Repo } from '../../shared/repo-types' import { isFolderRepo } from '../../shared/repo-kind' @@ -55,7 +56,8 @@ export async function resolveRuntimeGitLabWorktreeBase( let sourceBranch = args.sourceBranch?.trim() ?? '' let targetBranch = args.targetBranch?.trim() ?? '' let isCrossRepository = args.isCrossRepository === true - if (!sourceBranch) { + let pushTarget: GitPushTarget | undefined + if (!sourceBranch || !isCrossRepository) { let discoveryRemote: string try { discoveryRemote = await resolveRuntimeGitLabIssueSourceRemote( @@ -86,15 +88,30 @@ export async function resolveRuntimeGitLabWorktreeBase( repo.connectionId ?? null, localWorktreeGitOptions ) - if (!item || item.type !== 'mr') { + if ((!item || item.type !== 'mr') && !sourceBranch) { return { error: `MR !${args.mrIid} not found.` } } - sourceBranch = (item.branchName ?? '').trim() - targetBranch = (item.baseRefName ?? '').trim() + if (item?.headProjectRef && item.branchName) { + const candidate: GitPushTarget = { + remoteName: discoveryRemote, + branchName: item.branchName, + reviewHead: { + provider: 'gitlab', + host: item.headProjectRef.host, + repository: item.headProjectRef.path, + branchName: item.branchName + } + } + if ((await readGitReviewPushAuthority(gitExec, candidate)).kind === 'verified') { + pushTarget = candidate + } + } + sourceBranch = item?.branchName?.trim() || sourceBranch + targetBranch = item?.baseRefName?.trim() || targetBranch if (!sourceBranch) { return { error: `MR !${args.mrIid} has no source branch.` } } - if (item.isCrossRepository === true) { + if (item?.isCrossRepository === true) { isCrossRepository = true } } @@ -151,6 +168,6 @@ export async function resolveRuntimeGitLabWorktreeBase( return { baseBranch: remoteRef, ...(compareBaseFetched ? { compareBaseRef } : {}), - pushTarget: { remoteName: remote, branchName: sourceBranch } + ...(pushTarget ? { pushTarget } : {}) } } diff --git a/src/relay/git-exec-validator.test.ts b/src/relay/git-exec-validator.test.ts index 3f1dd0f2413..5ea5e6f906f 100644 --- a/src/relay/git-exec-validator.test.ts +++ b/src/relay/git-exec-validator.test.ts @@ -184,7 +184,7 @@ describe('validateGitExecArgs', () => { [['remote', 'add', '../escape', 'https://github.com/contributor/orca.git']], [['remote', 'remove', '-f']], [['remote', 'add', 'fork', 'ext::sh -c payload']], - [['remote', 'add', 'fork', 'https://evil.test/contributor/orca.git']], + [['remote', 'add', 'fork', 'https://gitlab.example/contributor/orca.git?unsupported=1']], [['remote', 'add', 'fork', '/etc/passwd']] ])('rejects unsafe remote write args %j', (args) => { expectBlocked(args, 'Destructive git remote operations') diff --git a/src/relay/git-handler-push-target.test.ts b/src/relay/git-handler-push-target.test.ts index e3f46b52133..4762051b841 100644 --- a/src/relay/git-handler-push-target.test.ts +++ b/src/relay/git-handler-push-target.test.ts @@ -163,12 +163,21 @@ describe('resolveRelayPushTarget', () => { }) it('uses an explicit push target without reading branch config', async () => { - const git = vi.fn(async () => ({ stdout: '', stderr: '' })) + const git = vi.fn(async () => ({ + stdout: 'fork\thttps://github.com/team/repo.git (push)', + stderr: '' + })) await expect( resolveRelayPushTarget(git, '/repo', { remoteName: 'fork', - branchName: 'feature/head' + branchName: 'feature/head', + reviewHead: { + provider: 'github', + host: 'github.com', + repository: 'team/repo', + branchName: 'feature/head' + } }) ).resolves.toEqual({ remote: 'fork', diff --git a/src/relay/git-handler-push-target.ts b/src/relay/git-handler-push-target.ts index 629e23d8bde..ecb122b5da5 100644 --- a/src/relay/git-handler-push-target.ts +++ b/src/relay/git-handler-push-target.ts @@ -1,3 +1,4 @@ +import { assertGitReviewPushAuthority } from '../shared/git-review-push-authority' import { assertGitPushTargetShape } from '../shared/git-push-target-validation' import { resolveConfiguredGitPushTarget, @@ -20,6 +21,7 @@ export async function resolveRelayPushTarget( // Why here and not in the shared resolver: an explicit target arrives over the wire, // so the host re-validates its shape and asks Git to vet the branch name itself. await git(['check-ref-format', '--branch', explicitTarget.branchName], worktreePath) + await assertGitReviewPushAuthority((args) => git(args, worktreePath), explicitTarget) return { remote: explicitTarget.remoteName, refspec: `HEAD:refs/heads/${explicitTarget.branchName}` diff --git a/src/renderer/src/components/right-sidebar/checks-panel-git-status-snapshot.ts b/src/renderer/src/components/right-sidebar/checks-panel-git-status-snapshot.ts index e41b0cfc109..e10a4538d8b 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-git-status-snapshot.ts +++ b/src/renderer/src/components/right-sidebar/checks-panel-git-status-snapshot.ts @@ -1,3 +1,4 @@ +import { reviewHeadKey } from '../../../../shared/git-review-push-authority' import type { GitStatusEntry, GitUpstreamStatus } from '../../../../shared/git-status-types' import type { GitPushTarget } from '../../../../shared/worktree/types' @@ -92,6 +93,7 @@ export function buildChecksPanelGitStatusContextKey( localExecutionScope: input.localExecutionScope ?? null, pushTarget: input.pushTarget ? { + reviewHead: reviewHeadKey(input.pushTarget.reviewHead) ?? null, remoteName: input.pushTarget.remoteName, branchName: input.pushTarget.branchName, remoteUrl: input.pushTarget.remoteUrl ?? null, diff --git a/src/renderer/src/components/right-sidebar/push-target-upstream-refresh-cache.ts b/src/renderer/src/components/right-sidebar/push-target-upstream-refresh-cache.ts index e8f0a8c39fb..9616638fa4e 100644 --- a/src/renderer/src/components/right-sidebar/push-target-upstream-refresh-cache.ts +++ b/src/renderer/src/components/right-sidebar/push-target-upstream-refresh-cache.ts @@ -1,3 +1,4 @@ +import { reviewHeadKey } from '../../../../shared/git-review-push-authority' import type { GitStatusResult, GitUpstreamStatus } from '../../../../shared/git-status-types' import type { GlobalSettings } from '../../../../shared/global-settings-types' import type { GitPushTarget } from '../../../../shared/worktree/types' @@ -24,6 +25,7 @@ function getRuntimeEnvironmentKey( function getPushTargetKey(pushTarget: GitPushTarget): readonly unknown[] { return [ + reviewHeadKey(pushTarget.reviewHead) ?? null, pushTarget.remoteName, pushTarget.branchName, pushTarget.remoteUrl ?? null, diff --git a/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts b/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts index f14e9abc29e..775e24bdeeb 100644 --- a/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts +++ b/src/renderer/src/components/right-sidebar/source-control-hosted-review-push-target.test.ts @@ -1,3 +1,4 @@ +import { reviewTarget } from '../../../../shared/__fixtures__/git-review-target' import { describe, expect, it } from 'vitest' import { hasPositiveHostedReviewNumberLink, @@ -239,6 +240,10 @@ describe('hasUsableHostedReviewPushTarget', () => { it('distinguishes equal labels with different named remote and merge identities', () => { const upstreamStatus = { + reviewPushAuthority: { + kind: 'verified' as const, + reviewHead: reviewTarget('origin/team', 'feature').reviewHead + }, hasUpstream: true, upstreamName: 'origin/team/feature', ahead: 1, @@ -252,7 +257,7 @@ describe('hasUsableHostedReviewPushTarget', () => { expect( hasUsableHostedReviewPushTarget({ upstreamStatus, - pushTarget: { remoteName: 'origin/team', branchName: 'feature' } + pushTarget: reviewTarget('origin/team', 'feature') }) ).toBe(true) expect( @@ -270,7 +275,7 @@ describe('hasUsableHostedReviewPushTarget', () => { selector: { kind: 'literal-url' } } }, - pushTarget: { remoteName: 'origin/team', branchName: 'feature' } + pushTarget: reviewTarget('origin/team', 'feature') }) ).toBe(false) }) @@ -303,12 +308,12 @@ describe('hasUsableHostedReviewPushTarget', () => { ).toBe(false) }) - it('accepts either persisted target metadata or branch-configured push metadata', () => { + it('rejects legacy target metadata and retains ordinary configured push policy', () => { expect( hasUsableHostedReviewPushTarget({ pushTarget: { remoteName: 'fork', branchName: 'feature' } }) - ).toBe(true) + ).toBe(false) expect( hasUsableHostedReviewPushTarget({ pushTarget: { remoteName: 'fork', branchName: 'feature' }, @@ -324,7 +329,7 @@ describe('hasUsableHostedReviewPushTarget', () => { behind: 0 } }) - ).toBe(true) + ).toBe(false) expect( hasUsableHostedReviewPushTarget({ upstreamStatus: { diff --git a/src/renderer/src/runtime/runtime-git-client.test.ts b/src/renderer/src/runtime/runtime-git-client.test.ts index 9b426035b60..733455ee7f8 100644 --- a/src/renderer/src/runtime/runtime-git-client.test.ts +++ b/src/renderer/src/runtime/runtime-git-client.test.ts @@ -628,8 +628,7 @@ describe('runtime git client', () => { await generateRuntimeCommitMessage(context) await cancelRuntimeGenerateCommitMessage(context) await pushRuntimeGit(context, { - publish: true, - pushTarget: { remoteName: 'origin', branchName: 'feature' } + publish: true }) await fetchRuntimeGit(context, { remoteName: 'fork', branchName: 'feature' }) await fastForwardRuntimeGit(context, { remoteName: 'fork', branchName: 'feature' }) @@ -670,8 +669,7 @@ describe('runtime git client', () => { method: 'git.push', params: { worktree: 'id:wt-1', - publish: true, - pushTarget: { remoteName: 'origin', branchName: 'feature' } + publish: true }, timeoutMs: 30_000 }) diff --git a/src/renderer/src/runtime/runtime-git-sync-client.ts b/src/renderer/src/runtime/runtime-git-sync-client.ts index 5f1652a3893..2143ad61ff4 100644 --- a/src/renderer/src/runtime/runtime-git-sync-client.ts +++ b/src/renderer/src/runtime/runtime-git-sync-client.ts @@ -1,3 +1,4 @@ +import { hasUsableHostedReviewPushTarget } from '../../../shared/hosted-review-push-target-admission' import type { GitForkSyncExpectedUpstream, GitForkSyncResult } from '../../../shared/git-fork-sync' import type { GitUpstreamStatus } from '../../../shared/git-status-types' import { REBASE_FROM_BASE_RPC_TIMEOUT_MS } from '../../../shared/git-rebase-source' @@ -195,6 +196,17 @@ export async function pushRuntimeGit( }) return } + if ( + args.pushTarget && + !hasUsableHostedReviewPushTarget({ + pushTarget: args.pushTarget, + upstreamStatus: await getRuntimeGitUpstreamStatus(context, args.pushTarget) + }) + ) { + throw new Error( + 'The execution host has not verified the review push endpoints. Update or reconnect the host and retry.' + ) + } await callRuntimeRpc( target, 'git.push', diff --git a/src/renderer/src/runtime/runtime-review-push-authority.test.ts b/src/renderer/src/runtime/runtime-review-push-authority.test.ts new file mode 100644 index 00000000000..d942596bced --- /dev/null +++ b/src/renderer/src/runtime/runtime-review-push-authority.test.ts @@ -0,0 +1,43 @@ +import { beforeEach, expect, it, vi } from 'vitest' +import { reviewTarget } from '../../../shared/__fixtures__/git-review-target' +const rpc = vi.hoisted(() => vi.fn()) +vi.mock('./runtime-rpc-client', () => ({ + getActiveRuntimeTarget: () => ({ kind: 'remote', environmentId: 'env' }), + callRuntimeRpc: rpc +})) +import { pushRuntimeGit } from './runtime-git-sync-client' +const context = { + settings: { activeRuntimeEnvironmentId: 'env' }, + worktreeId: 'wt', + worktreePath: '/repo' +} +const target = reviewTarget('origin', 'feature') +beforeEach(() => rpc.mockReset()) +it.each([ + undefined, + { kind: 'mismatch', reviewHead: target.reviewHead }, + { kind: 'ambiguous', reviewHead: target.reviewHead } +])('rejects old or unverified host evidence %j before push', async (authority) => { + rpc.mockResolvedValue({ hasUpstream: true, ahead: 1, behind: 0, reviewPushAuthority: authority }) + await expect(pushRuntimeGit(context, { pushTarget: target })).rejects.toThrow('not verified') + expect(rpc).toHaveBeenCalledOnce() + expect(rpc.mock.calls[0]?.[1]).toBe('git.upstreamStatus') +}) +it('dispatches only after host status corroborates the same target identity', async () => { + rpc + .mockResolvedValueOnce({ + hasUpstream: true, + ahead: 1, + behind: 0, + upstreamIdentity: { + selector: { kind: 'named-remote', value: 'origin' }, + mergeRef: 'refs/heads/feature', + trackingRef: null + }, + reviewPushAuthority: { kind: 'verified', reviewHead: target.reviewHead } + }) + .mockResolvedValueOnce({ ok: true }) + await pushRuntimeGit(context, { pushTarget: target }) + expect(rpc.mock.calls[1]?.[1]).toBe('git.push') + expect(rpc.mock.calls[1]?.[2]).toMatchObject({ pushTarget: target }) +}) diff --git a/src/renderer/src/store/slices/editor-git-status-reconciliation.test.ts b/src/renderer/src/store/slices/editor-git-status-reconciliation.test.ts index 8d42f8bb77e..e7cabbf7313 100644 --- a/src/renderer/src/store/slices/editor-git-status-reconciliation.test.ts +++ b/src/renderer/src/store/slices/editor-git-status-reconciliation.test.ts @@ -1,3 +1,4 @@ +import { reviewTarget } from '../../../../shared/__fixtures__/git-review-target' import type { GitUpstreamStatus } from '../../../../shared/git-status-types' import { hasUsableHostedReviewPushTarget } from '../../../../shared/hosted-review-push-target-admission' import { describe, expect, it, vi } from 'vitest' @@ -571,12 +572,16 @@ it('preserves identity-only upstream delivery and old-peer uncertainty', () => { store.getState().setUpstreamStatus('wt-identity', upstreamStatus) const usable = () => hasUsableHostedReviewPushTarget({ - pushTarget: { remoteName: 'origin/team', branchName: 'feature' }, + pushTarget: reviewTarget('origin/team', 'feature'), upstreamStatus: store.getState().remoteStatusesByWorktree['wt-identity'] }) const oldPeer = { hasUpstream: true, upstreamName: 'origin/team/feature', ahead: 1, behind: 0 } const canonical: GitUpstreamStatus = { ...oldPeer, + reviewPushAuthority: { + kind: 'verified', + reviewHead: reviewTarget('origin/team', 'feature').reviewHead + }, upstreamIdentity: { selector: { kind: 'named-remote', value: 'origin/team' }, mergeRef: 'refs/heads/feature', diff --git a/src/renderer/src/store/slices/editor-linked-review-operation-target.test.ts b/src/renderer/src/store/slices/editor-linked-review-operation-target.test.ts index d58c9a72e8c..aa698d9f979 100644 --- a/src/renderer/src/store/slices/editor-linked-review-operation-target.test.ts +++ b/src/renderer/src/store/slices/editor-linked-review-operation-target.test.ts @@ -1,3 +1,4 @@ +import { reviewTarget } from '../../../../shared/__fixtures__/git-review-target' import { beforeEach, expect, it, vi } from 'vitest' import { createEditorStore } from './editor-slice-test-harness' import { makeWorktree } from './worktrees-slice-test-fixtures' @@ -53,16 +54,19 @@ it.each(['linkedPR', 'linkedGitLabMR'] as const)( // A failed lookup retains the same unresolved state; no configured push fallback is admitted. await expect(store.getState().pushBranch('wt', '/repo')).rejects.toThrow('unresolved') for (const remoteName of ['contributor', 'other-repository']) { - worktree.pushTarget = { remoteName, branchName: 'feature' } + worktree.pushTarget = reviewTarget( + remoteName, + 'feature', + link === 'linkedPR' ? 'github' : 'gitlab' + ) await store.getState().pushBranch('wt', '/repo') expect(push).toHaveBeenLastCalledWith( expect.objectContaining({ pushTarget: worktree.pushTarget }) ) await expect( - store.getState().pushBranch('wt', '/repo', false, undefined, { - remoteName: 'origin', - branchName: 'feature' - }) + store + .getState() + .pushBranch('wt', '/repo', false, undefined, reviewTarget('origin', 'feature')) ).rejects.toThrow('changed') } } diff --git a/src/renderer/src/store/slices/editor/actions/linked-review-operation-target.ts b/src/renderer/src/store/slices/editor/actions/linked-review-operation-target.ts index 62f1861f29c..1b20f1c88b4 100644 --- a/src/renderer/src/store/slices/editor/actions/linked-review-operation-target.ts +++ b/src/renderer/src/store/slices/editor/actions/linked-review-operation-target.ts @@ -1,29 +1 @@ -import { isPositiveHostedReviewNumber } from '../../../../../../shared/hosted-review' -import type { GitPushTarget, Worktree } from '../../../../../../shared/worktree/types' - -export function linkedReviewOperationTarget( - worktree: Worktree | undefined, - requested: GitPushTarget | undefined, - fallbackGitHubPR?: number -): GitPushTarget | undefined { - if ( - !isPositiveHostedReviewNumber(worktree?.linkedPR) && - !isPositiveHostedReviewNumber(worktree?.linkedGitLabMR) && - !isPositiveHostedReviewNumber(fallbackGitHubPR) - ) { - return requested - } - const resolved = worktree?.pushTarget - if (!resolved) { - throw new Error('The linked review push target is still unresolved. Retry after it loads.') - } - if ( - requested && - (requested.remoteName !== resolved.remoteName || - requested.branchName !== resolved.branchName || - requested.remoteUrl !== resolved.remoteUrl) - ) { - throw new Error('The linked review push target changed. Retry with the current target.') - } - return resolved -} +export { linkedReviewOperationTarget } from '../../../../../../shared/linked-review-operation-target' diff --git a/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts b/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts index f564627f9f3..de4dd859610 100644 --- a/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts +++ b/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts @@ -1,3 +1,4 @@ +import { reviewHeadKey } from '../../../../../../shared/git-review-push-authority' import { areGitUpstreamIdentitiesEqual } from '../../../../../../shared/git-upstream-identity' import { translate } from '@/i18n/i18n' import type { @@ -120,6 +121,9 @@ export function areUpstreamStatusesEqual( prev !== undefined && prev.hasUpstream === next.hasUpstream && prev.upstreamName === next.upstreamName && + prev.reviewPushAuthority?.kind === next.reviewPushAuthority?.kind && + reviewHeadKey(prev.reviewPushAuthority?.reviewHead) === + reviewHeadKey(next.reviewPushAuthority?.reviewHead) && areGitUpstreamIdentitiesEqual(prev.upstreamIdentity, next.upstreamIdentity) && prev.ahead === next.ahead && prev.behind === next.behind && diff --git a/src/renderer/src/store/slices/worktree-listing-branch-switch.ts b/src/renderer/src/store/slices/worktree-listing-branch-switch.ts index a72c50b7ce0..5ba9d798cc3 100644 --- a/src/renderer/src/store/slices/worktree-listing-branch-switch.ts +++ b/src/renderer/src/store/slices/worktree-listing-branch-switch.ts @@ -1,3 +1,4 @@ +import { reviewHeadKey } from '../../../../shared/git-review-push-authority' import type { Worktree } from '../../../../shared/worktree/types' function indexUnambiguousWorktrees( @@ -21,6 +22,7 @@ function branchScopedReviewContextMatches(left: Worktree, right: Worktree): bool left.linkedBitbucketPR === right.linkedBitbucketPR && left.linkedAzureDevOpsPR === right.linkedAzureDevOpsPR && left.linkedGiteaPR === right.linkedGiteaPR && + reviewHeadKey(left.pushTarget?.reviewHead) === reviewHeadKey(right.pushTarget?.reviewHead) && left.pushTarget?.remoteName === right.pushTarget?.remoteName && left.pushTarget?.branchName === right.pushTarget?.branchName ) diff --git a/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts b/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts index e086b246135..6fb044f7080 100644 --- a/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts +++ b/src/renderer/src/store/slices/worktrees-linked-review-push-target.test.ts @@ -1,3 +1,4 @@ +import { reviewTarget } from '../../../../shared/__fixtures__/git-review-target' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { AppState } from '../types' import type { RuntimeEnvironmentCallRequest } from '../../runtime/runtime-compatibility-test-fixture' @@ -37,7 +38,7 @@ describe('worktree remote runtime mutations', () => { it('resolves and persists a push target when manually linking a GitHub PR', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'origin', branchName: 'bot/pr-bug-scan-2504' } + const pushTarget = reviewTarget('origin', 'bot/pr-bug-scan-2504') const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', @@ -79,7 +80,7 @@ describe('worktree remote runtime mutations', () => { it('clears a stale push target when unlinking the GitHub PR that supplied it', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'fork', branchName: 'owner/old-pr' } + const pushTarget = reviewTarget('fork', 'owner/old-pr') const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', @@ -112,7 +113,7 @@ describe('worktree remote runtime mutations', () => { path: '/path/wt1', branch: 'refs/heads/review-branch', linkedPR: 2548, - pushTarget: { remoteName: 'fork', branchName: 'owner/old-pr' } + pushTarget: reviewTarget('fork', 'owner/old-pr') }) store.setState({ repos: [ @@ -139,8 +140,8 @@ describe('worktree remote runtime mutations', () => { it('clears an older GitHub link and target when replacing it with a GitLab MR', async () => { const store = createTestStore() - const oldPushTarget = { remoteName: 'fork', branchName: 'owner/old-pr' } - const newPushTarget = { remoteName: 'upstream', branchName: 'owner/new-mr' } + const oldPushTarget = reviewTarget('fork', 'owner/old-pr') + const newPushTarget = reviewTarget('upstream', 'owner/new-mr') const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', @@ -202,7 +203,7 @@ describe('worktree remote runtime mutations', () => { it('resolves a manually linked GitHub PR through the worktree owner runtime', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'fork', branchName: 'owner-runtime/manual-pr' } + const pushTarget = reviewTarget('fork', 'owner-runtime/manual-pr') const wt = makeWorktree({ id: 'repo1::/remote/wt1', repoId: 'repo1', @@ -262,7 +263,7 @@ describe('worktree remote runtime mutations', () => { it('sends a runtime clear when unlinking a review-owned push target', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'fork', branchName: 'owner-runtime/old-pr' } + const pushTarget = reviewTarget('fork', 'owner-runtime/old-pr') const wt = makeWorktree({ id: 'repo1::/remote/wt1', repoId: 'repo1', @@ -323,7 +324,7 @@ describe('worktree remote runtime mutations', () => { it('does not resolve a push target when re-saving the same linked GitHub PR', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'origin', branchName: 'bot/pr-bug-scan-2504' } + const pushTarget = reviewTarget('origin', 'bot/pr-bug-scan-2504') const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', @@ -349,7 +350,7 @@ describe('worktree remote runtime mutations', () => { it('recovers a missing push target when re-saving the same linked GitHub PR', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'fork', branchName: 'contributor/fix' } + const pushTarget = reviewTarget('fork', 'contributor/fix') const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', @@ -383,10 +384,7 @@ describe('worktree remote runtime mutations', () => { it('hydrates a missing push target for an existing linked GitHub PR', async () => { const store = createTestStore() - const pushTarget = { - remoteName: 'pr-tmchow-orca', - branchName: 'tmchow/worktree-delete-button' - } + const pushTarget = reviewTarget('pr-tmchow-orca', 'tmchow/worktree-delete-button') const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', @@ -420,7 +418,7 @@ describe('worktree remote runtime mutations', () => { it('hydrates a missing linked GitHub PR push target through the active remote runtime', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'fork', branchName: 'feature/runtime-pr' } + const pushTarget = reviewTarget('fork', 'feature/runtime-pr') const wt = makeWorktree({ id: 'repo1::/path/runtime-wt', repoId: 'repo1', @@ -475,7 +473,7 @@ describe('worktree remote runtime mutations', () => { it('hydrates a host-stamped linked GitHub PR push target through the worktree owner runtime', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'fork', branchName: 'feature/owner-runtime-pr' } + const pushTarget = reviewTarget('fork', 'feature/owner-runtime-pr') const wt = makeWorktree({ id: 'repo1::/path/owner-runtime-wt', repoId: 'repo1', @@ -531,7 +529,7 @@ describe('worktree remote runtime mutations', () => { it('hydrates an SSH-owned linked GitHub PR push target through local IPC when a runtime is focused', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'fork', branchName: 'feature/ssh-pr' } + const pushTarget = reviewTarget('fork', 'feature/ssh-pr') const wt = makeWorktree({ id: 'repo-ssh::/home/orca/runtime-wt', repoId: 'repo-ssh', @@ -674,7 +672,7 @@ describe('worktree remote runtime mutations', () => { it('cleans up the in-flight lookup when restore is skipped for a genuinely ambiguous owner', async () => { const store = createTestStore() const worktreeId = 'repo-shared::/same/path' - const pushTarget = { remoteName: 'fork', branchName: 'feature/disambiguated' } + const pushTarget = reviewTarget('fork', 'feature/disambiguated') const ownedWorktree = makeWorktree({ id: worktreeId, repoId: 'repo-shared', @@ -740,7 +738,7 @@ describe('worktree remote runtime mutations', () => { it('hydrates a missing push target for an existing linked GitLab MR when supported', async () => { const store = createTestStore() - const pushTarget = { remoteName: 'upstream', branchName: 'feature/mr' } + const pushTarget = reviewTarget('upstream', 'feature/mr') const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', diff --git a/src/renderer/src/store/slices/worktrees-queued-review-push-target.test.ts b/src/renderer/src/store/slices/worktrees-queued-review-push-target.test.ts index a1d4e268c24..07a24305413 100644 --- a/src/renderer/src/store/slices/worktrees-queued-review-push-target.test.ts +++ b/src/renderer/src/store/slices/worktrees-queued-review-push-target.test.ts @@ -1,3 +1,4 @@ +import { reviewTarget } from '../../../../shared/__fixtures__/git-review-target' import { beforeEach, expect, it, vi } from 'vitest' import type { AppState } from '../types' import { makeWorktree } from './worktrees-slice-test-fixtures' @@ -48,7 +49,7 @@ it('hydrates queued review repository identity and rejects a superseded queue lo worktreesByRepo: { repo1: [wt] }, prCache: queued(42) } as Partial) - const target = { remoteName: 'contributor', branchName: 'feature' } + const target = reviewTarget('contributor', 'feature') let resolve!: (value: unknown) => void mockApi.worktrees.resolvePrBase.mockImplementationOnce( () => diff --git a/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target-ensure.ts b/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target-ensure.ts index dcfab4d859b..5f885de91e0 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target-ensure.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target-ensure.ts @@ -13,7 +13,7 @@ export function createEnsureHostedReviewPushTarget( ): WorktreeSlice['ensureHostedReviewPushTarget'] { return async (worktreeId) => { const worktree = get().getKnownWorktreeById(worktreeId) - if (!worktree || worktree.pushTarget) { + if (!worktree || worktree.pushTarget?.reviewHead) { return } const lookup = getHostedReviewPushTargetLookup( @@ -35,7 +35,7 @@ export function createEnsureHostedReviewPushTarget( return } const current = get().getKnownWorktreeById(worktreeId) - if (!current || current.pushTarget) { + if (!current || current.pushTarget?.reviewHead) { return } const currentLookup = getHostedReviewPushTargetLookup( diff --git a/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target.ts b/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target.ts index 2f85b206040..5a411c56476 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/hosted-review-push-target.ts @@ -29,7 +29,7 @@ export async function resolveGitHubReviewPushTarget( console.warn(`Failed to resolve push target for PR #${prNumber}: ${result.error}`) return undefined } - return result.pushTarget + return result.pushTarget?.reviewHead ? result.pushTarget : undefined } catch (error) { console.warn( `Failed to resolve push target for PR #${prNumber}:`, @@ -59,7 +59,7 @@ export async function resolveGitLabReviewPushTarget( console.warn(`Failed to resolve push target for MR !${mrIid}: ${result.error}`) return undefined } - return result.pushTarget + return result.pushTarget?.reviewHead ? result.pushTarget : undefined } catch (error) { console.warn( `Failed to resolve push target for MR !${mrIid}:`, diff --git a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts index 5e5c38a71e2..202f1cfc393 100644 --- a/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts +++ b/src/renderer/src/store/slices/worktrees/metadata/update-worktree-meta.ts @@ -83,7 +83,7 @@ export function createUpdateWorktreeMeta( linkedPrForPushTarget !== null && normalizedUpdates.pushTarget === undefined && existingWorktree && - !existingWorktree.pushTarget + !existingWorktree.pushTarget?.reviewHead ? trySettingsForWorktreeOwner(get(), worktreeId, executionHostId) : null const resolvedPushTarget = diff --git a/src/shared/__fixtures__/git-review-target.ts b/src/shared/__fixtures__/git-review-target.ts new file mode 100644 index 00000000000..d00885521f8 --- /dev/null +++ b/src/shared/__fixtures__/git-review-target.ts @@ -0,0 +1,18 @@ +import type { GitPushTarget, GitReviewHead } from '../worktree/types' + +export function reviewTarget( + remoteName: string, + branchName: string, + provider: GitReviewHead['provider'] = 'github' +): GitPushTarget { + return { + remoteName, + branchName, + reviewHead: { + provider, + host: provider === 'github' ? 'github.com' : 'gitlab.com', + repository: 'team/repo', + branchName + } + } +} diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index ddcc19584b0..24b1f76222d 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -1,3 +1,4 @@ +import { readGitRemoteTrackingRef } from './git-remote-tracking-ref' import { execFile } from 'node:child_process' import { mkdtemp, readFile, rename, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -71,6 +72,23 @@ describeBinaryCompatibility('real Git binary compatibility', () => { }) } + it('resolves abbreviated fetch mappings using Git-ranked source evidence', async () => { + await runGit(['update-ref', 'refs/heads/abbreviated-source', 'HEAD']) + await runGit(['clone', '--bare', '.', 'abbreviated-remote.git']) + await runGit(['remote', 'add', 'abbreviated', './abbreviated-remote.git']) + await runGit([ + 'config', + 'remote.abbreviated.fetch', + 'abbreviated-source:refs/custom/abbreviated' + ]) + await runGit(['fetch', 'abbreviated']) + expect(await readGitRemoteTrackingRef(runGit, 'abbreviated', 'abbreviated-source')).toBe( + 'refs/custom/abbreviated' + ) + await runGit(['--git-dir=abbreviated-remote.git', 'tag', 'abbreviated-source', 'HEAD']) + expect(await readGitRemoteTrackingRef(runGit, 'abbreviated', 'abbreviated-source')).toBeNull() + }) + function supports(major: number, minor: number): boolean { return version.major > major || (version.major === major && version.minor >= minor) } diff --git a/src/shared/git-publish-target-status.ts b/src/shared/git-publish-target-status.ts index fa84738a5b6..c9437d8aff3 100644 --- a/src/shared/git-publish-target-status.ts +++ b/src/shared/git-publish-target-status.ts @@ -1,3 +1,4 @@ +import { readGitReviewPushAuthority } from './git-review-push-authority' import { readGitRemoteTrackingRef } from './git-remote-tracking-ref' import type { GitUpstreamStatusIdentity } from './git-upstream-identity' import type { GitUpstreamStatus } from './git-status-types' @@ -19,6 +20,7 @@ export async function getPublishTargetStatus( target: GitPushTarget, getBehindCommitsArePatchEquivalent?: (upstreamName: string) => Promise ): Promise { + const reviewPushAuthority = await readGitReviewPushAuthority(runGit, target) const upstreamName = getPublishTargetDisplayName(target) const remoteRef = await readGitRemoteTrackingRef(runGit, target.remoteName, target.branchName) const upstreamIdentity: GitUpstreamStatusIdentity = { @@ -29,6 +31,7 @@ export async function getPublishTargetStatus( if (!remoteRef) { return { + ...(target.reviewHead ? { reviewPushAuthority } : {}), hasUpstream: false, upstreamName, upstreamIdentity: { ...upstreamIdentity, trackingRef: null }, @@ -53,6 +56,7 @@ export async function getPublishTargetStatus( : undefined return { + ...(target.reviewHead ? { reviewPushAuthority } : {}), hasUpstream: true, upstreamName, upstreamIdentity, diff --git a/src/shared/git-push-target-validation.ts b/src/shared/git-push-target-validation.ts index 4f907d568e7..8b0daea65dc 100644 --- a/src/shared/git-push-target-validation.ts +++ b/src/shared/git-push-target-validation.ts @@ -1,8 +1,6 @@ import type { GitPushTarget } from './worktree/types' const SAFE_REMOTE_NAME_SEGMENT = /^[A-Za-z0-9][A-Za-z0-9._-]*$/ -const GITHUB_CLONE_URL = /^https:\/\/github\.com\/[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+\.git$/ -const GITHUB_SSH_URL = /^git@github\.com:[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+\.git$/ function assertString(value: unknown, name: string): asserts value is string { if (typeof value !== 'string') { @@ -29,7 +27,33 @@ export function isSafeGitRemoteName(remoteName: string): boolean { // Why: the relay allows a fork remote to be added via git.exec, so the exec // validator needs the same URL rule the pushTarget-carrying RPCs already apply. export function isSafePushTargetRemoteUrl(remoteUrl: string): boolean { - return GITHUB_CLONE_URL.test(remoteUrl) || GITHUB_SSH_URL.test(remoteUrl) + const scp = /^git@([A-Za-z0-9.-]+):(.+)$/.exec(remoteUrl) + let path: string + if (scp) { + path = scp[2]! + } else { + try { + const url = new URL(remoteUrl) + if ( + !['https:', 'ssh:'].includes(url.protocol) || + url.password || + url.search || + url.hash || + (url.username && (url.protocol !== 'ssh:' || url.username !== 'git')) + ) { + return false + } + path = url.pathname.slice(1) + } catch { + return false + } + } + const parts = path.split('/') + return ( + parts.length >= 2 && + path.endsWith('.git') && + parts.every((part) => part !== '.' && part !== '..' && /^[A-Za-z0-9_.-]+$/.test(part)) + ) } export function assertGitPushTargetShape(target: unknown): asserts target is GitPushTarget { @@ -45,6 +69,21 @@ export function assertGitPushTargetShape(target: unknown): asserts target is Git if (!candidate.branchName || candidate.branchName.startsWith('-')) { throw new Error(`Invalid git branch name: ${candidate.branchName}`) } + if (candidate.reviewHead !== undefined) { + const head = candidate.reviewHead as Record | null + if ( + !head || + (head.provider !== 'github' && head.provider !== 'gitlab') || + typeof head.host !== 'string' || + !head.host || + typeof head.repository !== 'string' || + !head.repository.includes('/') || + typeof head.branchName !== 'string' || + head.branchName !== candidate.branchName + ) { + throw new Error('Invalid provider review head identity.') + } + } if (candidate.remoteUrl !== undefined) { assertString(candidate.remoteUrl, 'remote URL') if (!isSafePushTargetRemoteUrl(candidate.remoteUrl)) { diff --git a/src/shared/git-remote-tracking-ref.test.ts b/src/shared/git-remote-tracking-ref.test.ts index 485466359d3..2c8c6f5e83f 100644 --- a/src/shared/git-remote-tracking-ref.test.ts +++ b/src/shared/git-remote-tracking-ref.test.ts @@ -50,3 +50,26 @@ it('does not promote a stale second destination when the first configured mappin 'refs/remotes/origin/feature' ]) }) + +it.each([ + ['feature:refs/custom/feature', 'refs/custom/feature'], + ['heads/feature:refs/custom/feature', 'refs/custom/feature'], + ['feature:custom', 'refs/heads/custom'], + ['feature:heads/tracked', 'refs/heads/tracked'], + ['feature:remotes/origin/tracked', 'refs/remotes/origin/tracked'], + ['refs/tags/feature:refs/custom/tag', null], + ['feature*:refs/custom/wild*', null] +])( + 'resolves Git exact abbreviations separately from wildcard patterns %s', + async (mapping, expected) => { + const run = vi.fn(async (args: string[]) => ({ + stdout: + args[0] === 'config' + ? mapping! + : args[0] === 'ls-remote' + ? 'oid\trefs/heads/feature' + : 'oid' + })) + expect(await readGitRemoteTrackingRef(run, 'origin', 'feature')).toBe(expected) + } +) diff --git a/src/shared/git-remote-tracking-ref.ts b/src/shared/git-remote-tracking-ref.ts index 115b688e0fe..22857778698 100644 --- a/src/shared/git-remote-tracking-ref.ts +++ b/src/shared/git-remote-tracking-ref.ts @@ -26,6 +26,33 @@ function isMissingTrackingRef(error: unknown): boolean { return candidate.code === 1 || /(?:exited with|exit code) 1\b/i.test(candidate.message) } +async function resolveAbbreviatedFetchSource( + runGit: GitCommandRunner, + remote: string, + abbreviation: string, + source: string +): Promise { + // Git remote.c chooses the highest-ranked refname_match, including tags before heads. + const candidates = [ + abbreviation, + `refs/${abbreviation}`, + `refs/tags/${abbreviation}`, + `refs/heads/${abbreviation}`, + `refs/remotes/${abbreviation}`, + `refs/remotes/${abbreviation}/HEAD` + ] + if (!candidates.includes(source)) { + return false + } + try { + const { stdout } = await runGit(['ls-remote', '--refs', '--', remote, ...candidates]) + const refs = new Set(stdout.split(/\r?\n/).map((line) => line.split('\t')[1])) + return candidates.find((candidate) => refs.has(candidate)) === source + } catch { + return false + } +} + // Only configured fetch destinations establish tracking authority, regardless of namespace. export async function readGitRemoteTrackingRef( runGit: GitCommandRunner, @@ -51,11 +78,22 @@ export async function readGitRemoteTrackingRef( if (!from || !to || extra !== undefined) { continue } - const match = refspecMatch(from, source) + const match = + refspecMatch(from, source) ?? + (!from.includes('*') && + !from.startsWith('refs/') && + (await resolveAbbreviatedFetchSource(runGit, remote, from, source)) + ? '' + : null) if (match === null || from.includes('*') !== to.includes('*') || to.split('*').length > 2) { continue } - const ref = to.replace('*', () => match) + const destination = to.replace('*', () => match) + const ref = destination.startsWith('refs/') + ? destination + : /^(heads|tags|remotes)\//.test(destination) + ? `refs/${destination}` + : `refs/heads/${destination}` if (!isSafeGitRefName(ref)) { continue } diff --git a/src/shared/git-review-push-authority.test.ts b/src/shared/git-review-push-authority.test.ts new file mode 100644 index 00000000000..b2d5fa76caf --- /dev/null +++ b/src/shared/git-review-push-authority.test.ts @@ -0,0 +1,81 @@ +import { expect, it, vi } from 'vitest' +import { + assertGitReviewPushAuthority, + readGitReviewPushAuthority +} from './git-review-push-authority' +import type { GitPushTarget } from './worktree/types' + +const target: GitPushTarget = { + remoteName: 'origin', + branchName: 'feature', + reviewHead: { + provider: 'github', + host: 'github.com', + repository: 'team/repo', + branchName: 'feature' + } +} + +it.each([ + [['https://github.com/team/repo.git'], 'verified'], + [['git@github.com:team/repo.git'], 'verified'], + [['ssh://git@ssh.github.com:443/team/repo.git'], 'verified'], + [['https://github.com/other/repo.git'], 'mismatch'], + [['https://github.com/team/other.git'], 'mismatch'], + [['https://github.other.com/team/repo.git'], 'mismatch'], + [['https://github.com/team/repo.git', 'https://github.com/other/repo.git'], 'ambiguous'], + [['https://github.com/team/repo.git', 'git@github.com:team/repo.git'], 'ambiguous'], + [[], 'unverifiable'] +])('binds the entire push destination set %j', async (urls, expected) => { + const run = vi.fn(async () => ({ + stdout: [ + 'origin\thttps://github.com/team/repo.git (fetch)', + ...urls.map((url) => `origin\t${url} (push)`) + ].join('\n') + })) + expect((await readGitReviewPushAuthority(run, target)).kind).toBe(expected) + expect(run).toHaveBeenCalledWith(['remote', '-v']) + if (expected !== 'verified') { + await expect(assertGitReviewPushAuthority(run, target)).rejects.toThrow(String(expected)) + } +}) + +it('does not promote legacy metadata, matching fetch identity, or remoteCreated', async () => { + const run = vi.fn() + expect( + await readGitReviewPushAuthority(run, { + remoteName: 'origin', + branchName: 'feature', + remoteCreated: true + }) + ).toEqual({ kind: 'unresolved' }) + expect(run).not.toHaveBeenCalled() +}) + +it('corroborates GitLab nested project identity and host aliases', async () => { + const gitlab: GitPushTarget = { + ...target, + reviewHead: { + provider: 'gitlab', + host: 'gitlab.com', + repository: 'team/group/repo', + branchName: 'feature' + } + } + expect( + ( + await readGitReviewPushAuthority( + async () => ({ + stdout: 'origin\tssh://git@altssh.gitlab.com:443/team/group/repo.git (push)' + }), + gitlab + ) + ).kind + ).toBe('verified') +}) + +it('reports failed host reads as unverifiable and never substitutes execution', async () => { + const run = vi.fn().mockRejectedValue(new Error('disconnected')) + expect((await readGitReviewPushAuthority(run, target)).kind).toBe('unverifiable') + expect(run).toHaveBeenCalledOnce() +}) diff --git a/src/shared/git-review-push-authority.ts b/src/shared/git-review-push-authority.ts new file mode 100644 index 00000000000..daa28440119 --- /dev/null +++ b/src/shared/git-review-push-authority.ts @@ -0,0 +1,90 @@ +import { parseGitHubRemoteIdentity } from './github/remote-identity-parsing' +import { parseRemoteProjectRefCandidate } from './gitlab-project-ref-parser' +import { normalizeGitLabRemoteHost } from './git-remote-host-alias' +import { parseGitRemoteVerboseLine } from './git-remote-url-index' +import type { GitPushTarget, GitReviewHead } from './worktree/types' + +type GitRunner = (args: string[]) => Promise<{ stdout: string }> +export type GitReviewPushAuthority = { + kind: 'verified' | 'unresolved' | 'ambiguous' | 'unverifiable' | 'mismatch' + reviewHead?: GitReviewHead +} + +export function reviewHeadKey(head: GitReviewHead | undefined): string | undefined { + return head + ? JSON.stringify([ + head.provider, + head.host.toLowerCase(), + head.repository.toLowerCase(), + head.branchName + ]) + : undefined +} + +function endpointMatchesHead(endpoint: string, head: GitReviewHead): boolean { + if (head.provider === 'github') { + const identity = parseGitHubRemoteIdentity(endpoint) + return ( + !!identity && + identity.host.toLowerCase() === head.host.toLowerCase() && + `${identity.owner}/${identity.repo}`.toLowerCase() === head.repository.toLowerCase() + ) + } + const identity = parseRemoteProjectRefCandidate(endpoint) + return ( + !!identity && + normalizeGitLabRemoteHost(identity.host) === head.host.toLowerCase() && + identity.path.toLowerCase() === head.repository.toLowerCase() + ) +} + +// Always read through the execution host; persisted hydration and fetch URLs are not push evidence. +export async function readGitReviewPushAuthority( + runGit: GitRunner, + target: GitPushTarget +): Promise { + const reviewHead = target.reviewHead + if (!reviewHead) { + return { kind: 'unresolved' } + } + if (reviewHead.branchName !== target.branchName) { + return { kind: 'mismatch', reviewHead } + } + try { + const { stdout } = await runGit(['remote', '-v']) + const entries = stdout + .split(/\r?\n/) + .map(parseGitRemoteVerboseLine) + .filter((entry) => entry !== null) + if (entries.length > 512 || new Set(entries.map((entry) => entry.name)).size > 128) { + return { kind: 'unverifiable', reviewHead } + } + const endpoints = entries.filter( + (entry) => entry.name === target.remoteName && entry.direction === 'push' + ) + if (endpoints.length > 1) { + return { kind: 'ambiguous', reviewHead } + } + if (!endpoints[0]) { + return { kind: 'unverifiable', reviewHead } + } + return { + kind: endpointMatchesHead(endpoints[0].url, reviewHead) ? 'verified' : 'mismatch', + reviewHead + } + } catch { + return { kind: 'unverifiable', reviewHead } + } +} + +export async function assertGitReviewPushAuthority( + runGit: GitRunner, + target: GitPushTarget +): Promise { + const result = await readGitReviewPushAuthority(runGit, target) + if (result.kind !== 'verified') { + throw new Error( + `Review push endpoint authority is ${result.kind}. Resolve the review target and remote configuration before retrying.` + ) + } +} diff --git a/src/shared/git-status-types.ts b/src/shared/git-status-types.ts index 2f573f11bab..4063a1782b9 100644 --- a/src/shared/git-status-types.ts +++ b/src/shared/git-status-types.ts @@ -1,3 +1,4 @@ +import type { GitReviewPushAuthority } from './git-review-push-authority' import type { GitUpstreamStatusIdentity } from './git-upstream-identity' export type GitFileStatus = 'modified' | 'added' | 'deleted' | 'renamed' | 'untracked' | 'copied' export type GitStagingArea = 'staged' | 'unstaged' | 'untracked' @@ -93,6 +94,8 @@ export type GitStatusResult = { // Kept as a named type because explicit upstream refreshes can still fail for // reasons unrelated to working-tree status (e.g., no upstream is expected). export type GitUpstreamStatus = { + /** Absent on old peers: never authorizes a review push. */ + reviewPushAuthority?: GitReviewPushAuthority hasUpstream: boolean upstreamName?: string /** Optional on older execution hosts; absence does not establish operation identity. */ diff --git a/src/shared/github/remote-identity-parsing.ts b/src/shared/github/remote-identity-parsing.ts new file mode 100644 index 00000000000..e873b0735ee --- /dev/null +++ b/src/shared/github/remote-identity-parsing.ts @@ -0,0 +1,123 @@ +import { normalizeGitHubRemoteHost } from '../git-remote-host-alias' +import type { GitHubOwnerRepo } from './pull-request-types' + +export type GitHubRemoteIdentity = GitHubOwnerRepo & { host: string } + +// Why: HTTP ports identify the GHES web/API endpoint; SSH and git ports are +// transport-only and must not leak into gh's host identity. +function hostFromRemoteUrl(url: URL): string { + const protocol = url.protocol.toLowerCase() + return protocol === 'http:' || protocol === 'https:' ? url.host : url.hostname +} + +function parseGitHubRemotePath(path: string): Pick | null { + const parts = path.replace(/^\/+/, '').replace(/\/+$/, '').split('/') + if (parts.length !== 2) { + return null + } + const [owner, repoWithSuffix] = parts + const repo = repoWithSuffix.replace(/\.git$/i, '') + if (!owner || !repo) { + return null + } + return { owner, repo } +} + +/** SCP-style / ssh:// / git+ssh:// remotes may use an OpenSSH Host alias. */ +export function remoteUrlUsesSshTransport(remoteUrl: string): boolean { + const trimmed = remoteUrl.trim().toLowerCase() + return ( + trimmed.startsWith('git@') || trimmed.startsWith('ssh://') || trimmed.startsWith('git+ssh://') + ) +} + +/** SSH transport host as written, preserving SCP alias case. */ +export function rawSshTransportHost(remoteUrl: string): string | null { + const trimmed = remoteUrl.trim() + const scpMatch = trimmed.match(/^git@([^:]+):/i) + if (scpMatch) { + return scpMatch[1] + } + try { + const url = new URL(trimmed) + if (!['ssh:', 'git+ssh:'].includes(url.protocol.toLowerCase())) { + return null + } + return url.hostname || null + } catch { + return null + } +} + +/** Non-GitHub SSH host that may need OpenSSH alias expansion. */ +export function gitHubSshConfigHostAlias(remoteUrl: string): string | null { + if (!remoteUrlUsesSshTransport(remoteUrl)) { + return null + } + const identity = parseGitHubRemoteIdentity(remoteUrl) + if (!identity || identity.host === 'github.com') { + return null + } + return rawSshTransportHost(remoteUrl) ?? identity.host +} + +export function parseGitHubRemoteIdentity(remoteUrl: string): GitHubRemoteIdentity | null { + const trimmed = remoteUrl.trim() + const sshMatch = trimmed.match(/^git@([^:]+):([^/]+)\/([^/]+?)(?:\.git)?$/i) + if (sshMatch) { + return { host: normalizeGitHubRemoteHost(sshMatch[1]), owner: sshMatch[2], repo: sshMatch[3] } + } + + try { + const url = new URL(trimmed) + if (!['git:', 'git+ssh:', 'http:', 'https:', 'ssh:'].includes(url.protocol.toLowerCase())) { + return null + } + const path = parseGitHubRemotePath(url.pathname) + return path ? { host: normalizeGitHubRemoteHost(hostFromRemoteUrl(url)), ...path } : null + } catch { + return null + } +} + +export function parseGitHubOwnerRepo(remoteUrl: string): GitHubOwnerRepo | null { + const identity = parseGitHubRemoteIdentity(remoteUrl) + if (!identity || identity.host.toLowerCase() !== 'github.com') { + return null + } + return { owner: identity.owner, repo: identity.repo } +} + +/** Parse github.com identity using an expanded SSH HostName. */ +export function parseGitHubOwnerRepoWithResolvedSshHostname( + remoteUrl: string, + resolvedSshHostname: string | null | undefined +): GitHubOwnerRepo | null { + const direct = parseGitHubOwnerRepo(remoteUrl) + if (direct) { + return direct + } + if (!remoteUrlUsesSshTransport(remoteUrl)) { + return null + } + if (!resolvedSshHostname?.trim()) { + return null + } + const identity = parseGitHubRemoteIdentity(remoteUrl) + if (!identity) { + return null + } + if (normalizeGitHubRemoteHost(resolvedSshHostname.trim()) !== 'github.com') { + return null + } + return { owner: identity.owner, repo: identity.repo } +} + +/** Effective forge host after optional SSH config HostName expansion. */ +export function effectiveGitHubRemoteHost( + parsedHost: string, + resolvedSshHostname?: string | null +): string { + const candidate = resolvedSshHostname?.trim() || parsedHost + return normalizeGitHubRemoteHost(candidate) +} diff --git a/src/shared/gitlab-project-ref-parser.ts b/src/shared/gitlab-project-ref-parser.ts new file mode 100644 index 00000000000..bd7cd9160a9 --- /dev/null +++ b/src/shared/gitlab-project-ref-parser.ts @@ -0,0 +1,128 @@ +import type { GitLabProjectRef } from './gitlab-types' + +export type ProjectRef = GitLabProjectRef + +/** + * Hosts always treated as GitLab. Self-hosted instances are added at + * runtime via `getGlabKnownHosts()`, which inspects `glab auth status`. + */ +export const DEFAULT_GITLAB_HOSTS = ['gitlab.com'] as const + +export function normalizeGitLabHost(value: string): string { + return value.trim().toLowerCase() +} + +// Why: host recognition is port-aware so two services on the same hostname +// but different ports (e.g. a GitLab on :8080 and a Gitea on :3030) are not +// conflated. The hostname (port-less) part is kept for legacy known-host +// entries that were recorded without a port. +function hostnameOf(host: string): string { + // `host` may be `name` or `name:port`. Strip a trailing `:digits` port. + return host.replace(/:\d+$/, '') +} + +function stripGitSuffix(path: string): string { + return path.replace(/\/+$/, '').replace(/\.git$/i, '') +} + +// Why: the GitLab host identity is the web/API endpoint, which is what `glab +// --hostname` and the known-hosts list speak in terms of. For http(s) +// remotes the URL port IS that endpoint port (e.g. self-hosted on :8080), +// so it must be kept. For ssh/git remotes the port is a transport port +// (e.g. ssh on :2222) that does not identify the GitLab instance, so it is +// dropped and only the hostname is used. +function hostIdentityFromUrl(url: URL): string { + const protocol = url.protocol.toLowerCase() + if (protocol === 'http:' || protocol === 'https:') { + return url.host + } + return url.hostname +} + +function makeProjectRefForTrustedHost(host: string, path: string): ProjectRef | null { + const normalizedHost = normalizeGitLabHost(host) + const normalizedPath = stripGitSuffix(path.replace(/^\/+/, '')).trim() + // Reject paths without at least one group segment — `gitlab.com:foo` + // alone is not a project reference. + if (!normalizedPath.includes('/')) { + return null + } + return { host: normalizedHost, path: normalizedPath } +} + +/** + * Does `urlHost` (which may include a `:port`) match a known-host entry? + * - An exact match (including any port) always counts. + * - A known entry WITHOUT a port also matches a URL host on the same + * hostname regardless of the URL's port — this preserves recognition for + * legacy `gitlab.com` / bare-hostname known entries. + * - A known entry WITH a port only matches a URL host with the exact same + * port, so `gitlab.example.com:8443` does not accept a + * `gitea.example.com:3000` (or same-host different-port) remote. + */ +function knownHostMatches(urlHost: string, knownHost: string): boolean { + if (urlHost === knownHost) { + return true + } + if (hostnameOf(knownHost) === knownHost) { + // Known entry has no port — match on hostname alone. + return hostnameOf(urlHost) === knownHost + } + return false +} + +function makeProjectRef( + host: string, + path: string, + knownHosts: readonly string[] +): ProjectRef | null { + const normalizedHost = normalizeGitLabHost(host) + const normalizedKnownHosts = knownHosts.map(normalizeGitLabHost) + if (!normalizedKnownHosts.some((knownHost) => knownHostMatches(normalizedHost, knownHost))) { + return null + } + return makeProjectRefForTrustedHost(normalizedHost, path) +} + +export function parseRemoteProjectRefCandidate(remoteUrl: string): ProjectRef | null { + const trimmed = remoteUrl.trim() + if (!/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed)) { + const scpLike = trimmed.match(/^(?:[^@/:]+@)?([^:\s/]+):([^\s]+?)(?:\.git)?$/) + if (scpLike) { + return makeProjectRefForTrustedHost(scpLike[1], scpLike[2]) + } + } + + try { + const url = new URL(trimmed) + if (!['http:', 'https:', 'ssh:', 'git:', 'git+ssh:'].includes(url.protocol.toLowerCase())) { + return null + } + return makeProjectRefForTrustedHost(hostIdentityFromUrl(url), url.pathname) + } catch { + return null + } +} + +export function parseGitLabProjectRef( + remoteUrl: string, + knownHosts: readonly string[] = DEFAULT_GITLAB_HOSTS +): ProjectRef | null { + const trimmed = remoteUrl.trim() + if (!/^[a-z][a-z0-9+.-]*:\/\//i.test(trimmed)) { + const scpLike = trimmed.match(/^(?:[^@/:]+@)?([^:\s/]+):([^\s]+?)(?:\.git)?$/) + if (scpLike) { + return makeProjectRef(scpLike[1], scpLike[2], knownHosts) + } + } + + try { + const url = new URL(trimmed) + if (!['http:', 'https:', 'ssh:', 'git:', 'git+ssh:'].includes(url.protocol.toLowerCase())) { + return null + } + return makeProjectRef(hostIdentityFromUrl(url), url.pathname, knownHosts) + } catch { + return null + } +} diff --git a/src/shared/gitlab-types.ts b/src/shared/gitlab-types.ts index 1b709d35ee9..ae4d6229673 100644 --- a/src/shared/gitlab-types.ts +++ b/src/shared/gitlab-types.ts @@ -163,6 +163,8 @@ export type GitLabMRReviewersUpdateResult = | { ok: false; error: string } export type GitLabWorkItem = { + /** Present only when provider project IDs establish the source repository. */ + headProjectRef?: GitLabProjectRef id: string type: 'issue' | 'mr' number: number diff --git a/src/shared/hosted-review-push-target-admission.ts b/src/shared/hosted-review-push-target-admission.ts index fab79c5597a..d1064a77dc0 100644 --- a/src/shared/hosted-review-push-target-admission.ts +++ b/src/shared/hosted-review-push-target-admission.ts @@ -1,3 +1,4 @@ +import { reviewHeadKey } from './git-review-push-authority' import type { GitUpstreamStatus } from './git-status-types' import type { GitPushTarget } from './worktree/types' @@ -10,10 +11,13 @@ export function hasUsableHostedReviewPushTarget(args: { const identity = args.upstreamStatus?.upstreamIdentity if (args.pushTarget) { return ( - args.upstreamStatus === undefined || - (identity?.selector.kind === 'named-remote' && - identity.selector.value === args.pushTarget.remoteName && - identity.mergeRef === `refs/heads/${args.pushTarget.branchName}`) + !!args.pushTarget.reviewHead && + args.upstreamStatus?.reviewPushAuthority?.kind === 'verified' && + reviewHeadKey(args.upstreamStatus.reviewPushAuthority.reviewHead) === + reviewHeadKey(args.pushTarget.reviewHead) && + identity?.selector.kind === 'named-remote' && + identity.selector.value === args.pushTarget.remoteName && + identity.mergeRef === `refs/heads/${args.pushTarget.branchName}` ) } if (args.hasResolvableHostedReviewPushTargetLink) { diff --git a/src/shared/linked-review-operation-target.ts b/src/shared/linked-review-operation-target.ts new file mode 100644 index 00000000000..f1caa5fa0e7 --- /dev/null +++ b/src/shared/linked-review-operation-target.ts @@ -0,0 +1,31 @@ +import { reviewHeadKey } from './git-review-push-authority' +import { isPositiveHostedReviewNumber } from './hosted-review' +import type { GitPushTarget, Worktree } from './worktree/types' + +export function linkedReviewOperationTarget( + worktree: Pick | undefined, + requested: GitPushTarget | undefined, + fallbackGitHubPR?: number +): GitPushTarget | undefined { + if ( + !isPositiveHostedReviewNumber(worktree?.linkedPR) && + !isPositiveHostedReviewNumber(worktree?.linkedGitLabMR) && + !isPositiveHostedReviewNumber(fallbackGitHubPR) + ) { + return requested + } + const resolved = worktree?.pushTarget + if (!resolved?.reviewHead) { + throw new Error('The linked review push target is still unresolved. Retry after it loads.') + } + if ( + requested && + (requested.remoteName !== resolved.remoteName || + requested.branchName !== resolved.branchName || + requested.remoteUrl !== resolved.remoteUrl || + reviewHeadKey(requested.reviewHead) !== reviewHeadKey(resolved.reviewHead)) + ) { + throw new Error('The linked review push target changed. Retry with the current target.') + } + return resolved +} diff --git a/src/shared/worktree/types.ts b/src/shared/worktree/types.ts index e368716dc01..0c6897899c7 100644 --- a/src/shared/worktree/types.ts +++ b/src/shared/worktree/types.ts @@ -181,7 +181,16 @@ export type AutomationWorkspaceProvenanceRequest = { createRequestId: string } +export type GitReviewHead = { + provider: 'github' | 'gitlab' + host: string + repository: string + branchName: string +} + export type GitPushTarget = { + /** Provider-reported identity; execution must corroborate current operation endpoints. */ + reviewHead?: GitReviewHead remoteName: string branchName: string remoteUrl?: string