fix: address review findings (#1770)

src/main/github/client.test.ts

@@ -5,6 +5,7 @@ const {
   ghExecFileAsyncMock,
   getOwnerRepoMock,
   getIssueOwnerRepoMock,
+  getOwnerRepoForRemoteMock,
   gitExecFileAsyncMock,
   acquireMock,
   releaseMock
@@ -13,6 +14,7 @@ const {
   ghExecFileAsyncMock: vi.fn(),
   getOwnerRepoMock: vi.fn(),
   getIssueOwnerRepoMock: vi.fn(),
+  getOwnerRepoForRemoteMock: vi.fn(),
   gitExecFileAsyncMock: vi.fn(),
   acquireMock: vi.fn(),
   releaseMock: vi.fn()
@@ -23,6 +25,12 @@ vi.mock('./gh-utils', () => ({
   ghExecFileAsync: ghExecFileAsyncMock,
   getOwnerRepo: getOwnerRepoMock,
   getIssueOwnerRepo: getIssueOwnerRepoMock,
+  getOwnerRepoForRemote: getOwnerRepoForRemoteMock,
+  gitExecFileAsync: gitExecFileAsyncMock,
+  parseGitHubOwnerRepo: (remoteUrl: string) => {
+    const match = remoteUrl.trim().match(/github\.com[:/]([^/]+)\/([^/]+?)(?:\.git)?$/)
+    return match ? { owner: match[1], repo: match[2] } : null
+  },
   acquire: acquireMock,
   release: releaseMock,
   _resetOwnerRepoCache: vi.fn()
@@ -32,7 +40,7 @@ vi.mock('../git/runner', () => ({
   gitExecFileAsync: gitExecFileAsyncMock
 }))
 
-import { getPRForBranch, _resetOwnerRepoCache } from './client'
+import { getPRForBranch, getPullRequestPushTarget, _resetOwnerRepoCache } from './client'
 
 describe('getPRForBranch', () => {
   beforeEach(() => {
@@ -40,6 +48,7 @@ describe('getPRForBranch', () => {
     ghExecFileAsyncMock.mockReset()
     getOwnerRepoMock.mockReset()
     getIssueOwnerRepoMock.mockReset()
+    getOwnerRepoForRemoteMock.mockReset()
     gitExecFileAsyncMock.mockReset()
     acquireMock.mockReset()
     releaseMock.mockReset()
@@ -260,4 +269,63 @@ describe('getPRForBranch', () => {
 
     expect(pr).toBeNull()
   })
+
+  it('resolves fork PR push target using the origin URL protocol', async () => {
+    getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
+    getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
+    ghExecFileAsyncMock.mockResolvedValueOnce({
+      stdout: JSON.stringify({
+        head: {
+          ref: 'prateek/fix-sidebar-agents-toggle',
+          repo: {
+            full_name: 'prateek/orca',
+            name: 'orca',
+            clone_url: 'https://github.com/prateek/orca.git',
+            ssh_url: 'git@github.com:prateek/orca.git',
+            owner: { login: 'prateek' }
+          }
+        }
+      })
+    })
+    gitExecFileAsyncMock.mockResolvedValueOnce({
+      stdout: 'git@github.com:stablyai/orca.git\n',
+      stderr: ''
+    })
+
+    const target = await getPullRequestPushTarget('/repo-root', 1738)
+
+    expect(ghExecFileAsyncMock).toHaveBeenCalledWith(['api', 'repos/stablyai/orca/pulls/1738'], {
+      cwd: '/repo-root'
+    })
+    expect(target).toEqual({
+      remoteName: 'pr-prateek-orca',
+      branchName: 'prateek/fix-sidebar-agents-toggle',
+      remoteUrl: 'git@github.com:prateek/orca.git'
+    })
+  })
+
+  it('uses origin for same-repository PR push targets', async () => {
+    getOwnerRepoMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
+    getOwnerRepoForRemoteMock.mockResolvedValueOnce({ owner: 'stablyai', repo: 'orca' })
+    ghExecFileAsyncMock.mockResolvedValueOnce({
+      stdout: JSON.stringify({
+        head: {
+          ref: 'fix-sidebar',
+          repo: {
+            full_name: 'stablyai/orca',
+            name: 'orca',
+            clone_url: 'https://github.com/stablyai/orca.git',
+            ssh_url: 'git@github.com:stablyai/orca.git',
+            owner: { login: 'stablyai' }
+          }
+        }
+      })
+    })
+
+    await expect(getPullRequestPushTarget('/repo-root', 1738)).resolves.toEqual({
+      remoteName: 'origin',
+      branchName: 'fix-sidebar'
+    })
+    expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
+  })
 })

src/main/github/client.ts

@@ -2,6 +2,7 @@
 concurrency acquire/release pattern and error handling consistent across operations. */
 import type {
   ClassifiedError,
+  GitPushTarget,
   IssueSourcePreference,
   ListWorkItemsResult,
   PRInfo,
@@ -19,6 +20,7 @@ import { getPRConflictSummary } from './conflict-summary'
 import {
   execFileAsync,
   ghExecFileAsync,
+  gitExecFileAsync,
   acquire,
   release,
   getOwnerRepo,
@@ -73,6 +75,90 @@ export async function checkOrcaStarred(): Promise<boolean | null> {
   }
 }
 
+function pickPushRemoteUrl(args: {
+  originUrl: string | null
+  cloneUrl: string
+  sshUrl: string
+}): string {
+  const { originUrl, cloneUrl, sshUrl } = args
+  if (originUrl && (/^(git@|ssh:)/.test(originUrl) || originUrl.includes('ssh.github.com'))) {
+    return sshUrl
+  }
+  return cloneUrl
+}
+
+function sanitizeRemoteName(owner: string, repo: string): string {
+  const slug = `${owner}-${repo}`
+    .toLowerCase()
+    .replace(/[^a-z0-9._-]+/g, '-')
+    .replace(/-+/g, '-')
+    .replace(/^[.-]+|[.-]+$/g, '')
+  return slug ? `pr-${slug}` : 'pr-head'
+}
+
+export async function getPullRequestPushTarget(
+  repoPath: string,
+  prNumber: number
+): Promise<GitPushTarget | null> {
+  const ownerRepo = await getOwnerRepo(repoPath)
+  if (!ownerRepo) {
+    return null
+  }
+
+  await acquire()
+  try {
+    const [{ stdout: prStdout }, origin] = await Promise.all([
+      ghExecFileAsync(['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${prNumber}`], {
+        cwd: repoPath
+      }),
+      getOwnerRepoForRemote(repoPath, 'origin')
+    ])
+    const pr = JSON.parse(prStdout) as {
+      head?: {
+        ref?: string
+        repo?: {
+          full_name?: string
+          clone_url?: string
+          ssh_url?: string
+          owner?: { login?: string }
+          name?: string
+        } | null
+      }
+    }
+    const headRepo = pr.head?.repo
+    const branchName = pr.head?.ref?.trim()
+    const owner = headRepo?.owner?.login?.trim()
+    const repo = headRepo?.name?.trim() ?? headRepo?.full_name?.split('/')[1]?.trim()
+    const cloneUrl = headRepo?.clone_url?.trim()
+    const sshUrl = headRepo?.ssh_url?.trim()
+    if (!owner || !repo || !branchName || !cloneUrl || !sshUrl) {
+      return null
+    }
+    if (
+      origin &&
+      origin.owner.toLowerCase() === owner.toLowerCase() &&
+      origin.repo.toLowerCase() === repo.toLowerCase()
+    ) {
+      return { remoteName: 'origin', branchName }
+    }
+
+    let originUrl: string | null = null
+    try {
+      const { stdout } = await gitExecFileAsync(['remote', 'get-url', 'origin'], { cwd: repoPath })
+      originUrl = stdout.trim() || null
+    } catch {
+      originUrl = null
+    }
+    return {
+      remoteName: sanitizeRemoteName(owner, repo),
+      branchName,
+      remoteUrl: pickPushRemoteUrl({ originUrl, cloneUrl, sshUrl })
+    }
+  } finally {
+    release()
+  }
+}
+
 /**
  * Star the Orca repo for the authenticated user.
  */