From 41f15805b42cbf5fc5efeec879f53a9dc4b7c37e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 20 Oct 2026 02:36:11 -0700 Subject: [PATCH] fix(runtime): PR/MR worktree base refuses SSH projects named only by executionHostId (#31054) * fix(runtime): PR/MR worktree base refuses SSH projects named only by executionHostId Codex review 6 #4: the guard read the legacy connectionId, so a row stamped executionHostId 'ssh:' without it ran git fetch on this machine. * fix(runtime): one gate for local Git on a repo row; base resolution agrees with create Review follow-up: a local row with a stale connectionId and runtime: rows now refuse too (create already refused runtime: rows), through requireRepoLocalGitRoute in repo-execution-host.ts. * fix(runtime): keep base resolution for runtime-stamped rows this server owns Codex follow-up: older servers keep a client-minted runtime: stamp on rows whose files are here, and base resolution worked for them before. requireRepoLocalGitRoute reads the stored host (getStoredRepoExecutionHostId), so they run locally again; SSH rows and any row with a connectionId still refuse. --- src/main/repo-execution-host.ts | 19 ++++ .../runtime/runtime-github-worktree-base.ts | 7 +- .../runtime/runtime-gitlab-worktree-base.ts | 7 +- ...eview-worktree-base-execution-host.test.ts | 86 +++++++++++++++++++ 4 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 src/main/runtime/runtime-review-worktree-base-execution-host.test.ts diff --git a/src/main/repo-execution-host.ts b/src/main/repo-execution-host.ts index dae5b0e48d4..4665013af0c 100644 --- a/src/main/repo-execution-host.ts +++ b/src/main/repo-execution-host.ts @@ -2,9 +2,12 @@ import { getRepoExecutionHostId, getSshTargetIdForExecutionHost, LOCAL_EXECUTION_HOST_ID, + normalizeHostPart, type ExecutionHostId } from '../shared/execution-host' import type { Repo } from '../shared/repo-types' +import { requireReachableGitRoute } from './providers/execution-host-provider-dispatch' +import { SshHostNotServedError } from './providers/ssh-host-not-served-error' /** Only for rows from this process's Store: runtime stamps address files registered here. */ export function getStoredRepoExecutionHostId( @@ -19,3 +22,19 @@ export function getStoredRepoSshConnectionId( ): string | null { return getSshTargetIdForExecutionHost(getStoredRepoExecutionHostId(repo)) } + +/** + * The gate for Git this process runs on a row from its own Store. `ssh:` rows throw, as does any row + * still carrying a `connectionId` (a nested SSH target, or a stale one beside `local`). A `runtime:` + * stamp alone addresses files registered here (`getStoredRepoExecutionHostId`), so it runs. + */ +export function requireRepoLocalGitRoute( + repo: Pick +): ReturnType { + const route = requireReachableGitRoute(getStoredRepoExecutionHostId(repo)) + const staleConnectionId = normalizeHostPart(repo.connectionId) + if (staleConnectionId) { + throw new SshHostNotServedError(staleConnectionId) + } + return route +} diff --git a/src/main/runtime/runtime-github-worktree-base.ts b/src/main/runtime/runtime-github-worktree-base.ts index c3e760a67e7..43660d64c2f 100644 --- a/src/main/runtime/runtime-github-worktree-base.ts +++ b/src/main/runtime/runtime-github-worktree-base.ts @@ -11,8 +11,8 @@ import { getLocalProjectGitExecOptions, getLocalProjectGhExecOptions } from '../project-runtime-git-options' -import { SshHostNotServedError } from '../providers/ssh-host-not-served-error' import { isFolderRepo } from '../../shared/repo-kind' +import { requireRepoLocalGitRoute } from '../repo-execution-host' import type { ProjectRuntimeResolutionStore } from '../local-project-runtime-resolution' export type RuntimeGitHubWorktreeBaseArgs = { @@ -42,9 +42,8 @@ export async function resolveRuntimeGitHubWorktreeBase( if (isFolderRepo(repo)) { return { error: 'Folder mode does not support creating worktrees.' } } - if (repo.connectionId) { - throw new SshHostNotServedError(repo.connectionId) - } + // Why the shared gate: a row may name its SSH host only as `executionHostId`. + requireRepoLocalGitRoute(repo) const localGitExecOptions = getLocalProjectGitExecOptions(deps.store, repo) const localGhOptions = getLocalProjectGhExecOptions(deps.store, repo) const gitExec = (gitArgs: string[]) => gitExecFileAsync(gitArgs, localGitExecOptions) diff --git a/src/main/runtime/runtime-gitlab-worktree-base.ts b/src/main/runtime/runtime-gitlab-worktree-base.ts index 7c0874e47a4..239b9a623be 100644 --- a/src/main/runtime/runtime-gitlab-worktree-base.ts +++ b/src/main/runtime/runtime-gitlab-worktree-base.ts @@ -1,6 +1,7 @@ import type { GitPushTarget } from '../../shared/worktree/types' import type { Repo } from '../../shared/repo-types' import { isFolderRepo } from '../../shared/repo-kind' +import { requireRepoLocalGitRoute } from '../repo-execution-host' import { fetchCompareBaseRefWithLocalFallback } from '../git/compare-base-ref-fetch' import { gitExecFileAsync } from '../git/runner' import { getProjectRefForRemote, getWorkItemByProjectRef } from '../gitlab/client' @@ -9,7 +10,6 @@ import { getLocalProjectGitExecOptions, getLocalProjectWorktreeGitOptions } from '../project-runtime-git-options' -import { SshHostNotServedError } from '../providers/ssh-host-not-served-error' import type { ProjectRuntimeResolutionStore } from '../local-project-runtime-resolution' import { resolveRuntimeGitLabForkMrBase } from './runtime-gitlab-fork-mr-base' import { resolveRuntimeGitLabIssueSourceRemote } from './runtime-gitlab-issue-source-remote' @@ -45,9 +45,8 @@ export async function resolveRuntimeGitLabWorktreeBase( if (isFolderRepo(repo)) { return { error: 'Folder mode does not support creating worktrees.' } } - if (repo.connectionId) { - throw new SshHostNotServedError(repo.connectionId) - } + // Why the shared gate: a row may name its SSH host only as `executionHostId`. + requireRepoLocalGitRoute(repo) const localGitExecOptions = getLocalProjectGitExecOptions(deps.store, repo) const localWorktreeGitOptions = getLocalProjectWorktreeGitOptions(deps.store, repo) const gitExec = (gitArgs: string[]) => gitExecFileAsync(gitArgs, localGitExecOptions) diff --git a/src/main/runtime/runtime-review-worktree-base-execution-host.test.ts b/src/main/runtime/runtime-review-worktree-base-execution-host.test.ts new file mode 100644 index 00000000000..990cce67116 --- /dev/null +++ b/src/main/runtime/runtime-review-worktree-base-execution-host.test.ts @@ -0,0 +1,86 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../shared/repo-types' +import { SshHostNotServedError } from '../providers/ssh-host-not-served-error' + +const { gitExecFileAsyncMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn() })) + +vi.mock('../git/runner', () => ({ + gitExecFileAsync: gitExecFileAsyncMock, + gitExecFileSync: vi.fn() +})) + +import { resolveRuntimeGitHubWorktreeBase } from './runtime-github-worktree-base' +import { resolveRuntimeGitLabWorktreeBase } from './runtime-gitlab-worktree-base' + +const store = { getProjects: () => [], getSettings: () => ({}) } + +function repoOn(fields: Pick): Repo { + return { + id: 'repo-1', + path: '/srv/project', + displayName: 'project', + badgeColor: '#000', + addedAt: 0, + kind: 'git', + ...fields + } +} + +const PROVIDERS = [ + [ + 'GitHub', + (resolveRepo: () => Promise) => + resolveRuntimeGitHubWorktreeBase( + { repoSelector: 'id:repo-1', prNumber: 7, headRefName: 'feature' }, + { store, resolveRepo } + ) + ], + [ + 'GitLab', + (resolveRepo: () => Promise) => + resolveRuntimeGitLabWorktreeBase( + { repoSelector: 'id:repo-1', mrIid: 7, sourceBranch: 'feature', targetBranch: 'main' }, + { store, resolveRepo } + ) + ] +] as const + +// Why each row: rows written by unified-host code carry only `executionHostId`; a `connectionId` +// beside `local` contradicts itself, and beside `runtime:` names a nested SSH target. +const REFUSED_ROWS = [ + ['executionHostId only', { executionHostId: 'ssh:gpu' }, new SshHostNotServedError('gpu')], + ['legacy connectionId', { connectionId: 'gpu' }, new SshHostNotServedError('gpu')], + [ + 'local with a stale connectionId', + { executionHostId: 'local', connectionId: 'gpu' }, + new SshHostNotServedError('gpu') + ], + [ + 'runtime host with a nested SSH target', + { executionHostId: 'runtime:env', connectionId: 'gpu' }, + new SshHostNotServedError('gpu') + ] +] as const + +describe('runtime PR/MR worktree base runs Git only for rows on this machine', () => { + beforeEach(() => { + gitExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockResolvedValue({ stdout: 'origin\n', stderr: '' }) + }) + + for (const [provider, resolveBase] of PROVIDERS) { + it.each(REFUSED_ROWS)(`${provider} refuses a row: %s`, async (_label, fields, expected) => { + await expect(resolveBase(async () => repoOn(fields))).rejects.toEqual(expected) + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + + // Why runtime: older servers kept the client-minted `runtime:` stamp on rows they own. + it.each([ + ['local', { executionHostId: 'local' }], + ['runtime-stamped on this server', { executionHostId: 'runtime:env' }] + ] as const)(`${provider} runs local Git for a row here: %s`, async (_label, fields) => { + await resolveBase(async () => repoOn(fields)).catch(() => {}) + expect(gitExecFileAsyncMock).toHaveBeenCalled() + }) + } +})