Support org SSH users and gh-resolved remotes for pull request review

electron/__tests__/pull-request-review.test.ts

@@ -155,3 +155,97 @@ process.stdin.on('end', () => {
     await removeGitTestDirectory(directory);
   }
 });
+
+test('matches pull requests to remotes with custom SSH users or through gh', async () => {
+  const directory = await mkdtemp(join(tmpdir(), 'codiff-pull-request-review-'));
+  const sshUserRepo = join(directory, 'ssh-user-repo');
+  const aliasedRepo = join(directory, 'aliased-repo');
+  const fakeBin = join(directory, 'bin');
+  const fakeGh = join(fakeBin, 'gh');
+  const callsPath = join(directory, 'calls.jsonl');
+  const previousPath = process.env.PATH;
+  const previousCallsPath = process.env.CODIFF_GITHUB_REVIEW_TEST_CALLS;
+
+  try {
+    await Promise.all([mkdir(sshUserRepo), mkdir(aliasedRepo), mkdir(fakeBin)]);
+    await execFileAsync('git', ['-C', sshUserRepo, 'init']);
+    await execFileAsync('git', [
+      '-C',
+      sshUserRepo,
+      'remote',
+      'add',
+      'origin',
+      'org-12345@github.com:acme/widgets.git',
+    ]);
+    await execFileAsync('git', ['-C', aliasedRepo, 'init']);
+    await execFileAsync('git', [
+      '-C',
+      aliasedRepo,
+      'remote',
+      'add',
+      'origin',
+      'git@github-work:acme/widgets.git',
+    ]);
+    await writeFile(
+      fakeGh,
+      `#!/usr/bin/env node
+const { appendFileSync } = require('node:fs');
+const args = process.argv.slice(2);
+let input = '';
+process.stdin.setEncoding('utf8');
+process.stdin.on('data', (chunk) => { input += chunk; });
+process.stdin.on('end', () => {
+  appendFileSync(
+    process.env.CODIFF_GITHUB_REVIEW_TEST_CALLS,
+    JSON.stringify({ args, input }) + '\\n',
+  );
+  process.stdout.write(
+    args[0] === 'repo' && args[1] === 'view'
+      ? '{"owner":{"login":"acme"},"name":"widgets"}'
+      : '{}',
+  );
+});
+`,
+    );
+    await chmod(fakeGh, 0o755);
+    process.env.PATH = `${fakeBin}:${previousPath ?? ''}`;
+    process.env.CODIFF_GITHUB_REVIEW_TEST_CALLS = callsPath;
+
+    const review = {
+      body: 'Looks good.',
+      comments: [],
+      event: 'APPROVE' as const,
+      source: {
+        provider: 'github' as const,
+        type: 'pull-request' as const,
+        url: 'https://github.com/acme/widgets/pull/42',
+      },
+    };
+
+    await submitPullRequestReview(sshUserRepo, review);
+    await submitPullRequestReview(aliasedRepo, review);
+
+    const calls = (await readFile(callsPath, 'utf8'))
+      .trim()
+      .split('\n')
+      .map((line) => JSON.parse(line) as { args: ReadonlyArray<string>; input: string });
+    expect(calls).toHaveLength(3);
+    // The custom SSH user matches through remote parsing alone — no gh lookup.
+    expect(calls[0].args).toContain('repos/acme/widgets/pulls/42/reviews');
+    // The aliased host hides github.com, so the repository resolves through gh.
+    expect(calls[1].args.join(' ')).toBe('repo view --json owner,name');
+    expect(calls[2].args).toContain('repos/acme/widgets/pulls/42/reviews');
+  } finally {
+    if (previousPath == null) {
+      delete process.env.PATH;
+    } else {
+      process.env.PATH = previousPath;
+    }
+    if (previousCallsPath == null) {
+      delete process.env.CODIFF_GITHUB_REVIEW_TEST_CALLS;
+    } else {
+      process.env.CODIFF_GITHUB_REVIEW_TEST_CALLS = previousCallsPath;
+    }
+    await removeGitTestDirectory(directory);
+  }
+});

electron/git-state/pull-request.cjs

@@ -64,7 +64,9 @@ const parseGitHubPullRequestUrl = (value) => {
 /** @param {string} value @returns {GitHubRemote | null} */
 const parseGitHubRemoteUrl = (value) => {
   const trimmed = value.trim();
-  const sshMatch = trimmed.match(/^git@github\.com:([^/]+)\/(.+?)(?:\.git)?$/i);
+  // GitHub organizations with SSO issue SSH remotes as `org-<id>@github.com`
+  // instead of `git@github.com`, so both users are recognized.
+  const sshMatch = trimmed.match(/^(?:git|org-\d+)@github\.com:([^/]+)\/(.+?)(?:\.git)?$/i);
   if (sshMatch) {
     return {
       owner: sshMatch[1],
@@ -118,6 +120,62 @@ const getRemotePriority = (remote) =>
       ? 2
       : 3;
 
+/** @param {string} repoRoot @returns {Promise<GitHubRepositoryReference>} */
+const readGitHubRepositoryReference = (repoRoot) =>
+  new Promise((resolve, reject) => {
+    const child = spawn('gh', ['repo', 'view', '--json', 'owner,name'], {
+      cwd: repoRoot,
+      stdio: ['ignore', 'pipe', 'pipe'],
+    });
+    /** @type {Array<Buffer>} */
+    const stdout = [];
+    /** @type {Array<Buffer>} */
+    const stderr = [];
+
+    child.stdout.on('data', (chunk) => stdout.push(chunk));
+    child.stderr.on('data', (chunk) => stderr.push(chunk));
+    child.on('error', reject);
+    child.on('close', (code) => {
+      if (code !== 0) {
+        const errorOutput = Buffer.concat(stderr).toString('utf8').trim();
+        reject(new Error(errorOutput || `gh repo view exited with code ${code}.`));
+        return;
+      }
+
+      try {
+        const data = JSON.parse(Buffer.concat(stdout).toString('utf8'));
+        const owner = data?.owner?.login;
+        const repo = data?.name;
+        if (owner && repo) {
+          resolve({ owner, repo });
+          return;
+        }
+        reject(new Error('gh repo view did not return the repository owner and name.'));
+      } catch (error) {
+        reject(error instanceof Error ? error : new Error('gh repo view returned invalid JSON.'));
+      }
+    });
+  });
+
+/** @type {Map<string, Promise<GitHubRepositoryReference | null>>} */
+const repositoryReferenceCache = new Map();
+
+/** @param {string} repoRoot @param {PullRequestReference} pullRequest @returns {Promise<boolean>} */
+const gitHubRepositoryMatchesPullRequest = async (repoRoot, pullRequest) => {
+  let reference = repositoryReferenceCache.get(repoRoot);
+  if (!reference) {
+    reference = readGitHubRepositoryReference(repoRoot).catch(() => null);
+    repositoryReferenceCache.set(repoRoot, reference);
+  }
+
+  const repository = await reference;
+  return (
+    repository != null &&
+    repository.owner.toLowerCase() === pullRequest.owner.toLowerCase() &&
+    repository.repo.toLowerCase() === pullRequest.repo.toLowerCase()
+  );
+};
+
 /** @param {string} repoRoot @param {PullRequestReference} pullRequest @returns {Promise<GitHubRemote>} */
 const selectPullRequestRemote = async (repoRoot, pullRequest) => {
   const remotes = await readLocalGitHubRemotes(repoRoot);
@@ -129,13 +187,25 @@ const selectPullRequestRemote = async (repoRoot, pullRequest) => {
     )
     .sort((left, right) => getRemotePriority(left) - getRemotePriority(right))[0];
 
-  if (!remote) {
-    throw new Error(
-      `Pull request ${pullRequest.owner}/${pullRequest.repo} does not match a GitHub remote in this repository.`,
-    );
+  if (remote) {
+    return remote;
   }
 
-  return remote;
+  // Remotes using SSH host aliases or `insteadOf` rewrites hide the github.com
+  // host from URL parsing. Only when such a remote exists, ask gh to resolve
+  // the repository and fall back to that remote's name so git can fetch.
+  const names = (await gitOrEmpty(repoRoot, ['remote']))
+    .split('\n')
+    .map((name) => name.trim())
+    .filter((name) => name && !remotes.some((remote) => remote.name === name));
+  const name = names.includes('origin') ? 'origin' : names[0];
+  if (name && (await gitHubRepositoryMatchesPullRequest(repoRoot, pullRequest))) {
+    return { direction: 'fetch', name, owner: pullRequest.owner, repo: pullRequest.repo };
+  }
+
+  throw new Error(
+    `Pull request ${pullRequest.owner}/${pullRequest.repo} does not match a GitHub remote in this repository.`,
+  );
 };
 
 /** @param {string} repoRoot @param {PullRequestReference} pullRequest */