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:<target>' 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:<id> 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.
This commit is contained in:
Neil
2026-10-20 02:36:11 -07:00
committed by GitHub
parent e3ad4acaa3
commit 41f15805b4
4 changed files with 111 additions and 8 deletions
+19
View File
@@ -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<Repo, 'connectionId' | 'executionHostId'>
): ReturnType<typeof requireReachableGitRoute> {
const route = requireReachableGitRoute(getStoredRepoExecutionHostId(repo))
const staleConnectionId = normalizeHostPart(repo.connectionId)
if (staleConnectionId) {
throw new SshHostNotServedError(staleConnectionId)
}
return route
}
@@ -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)
@@ -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)
@@ -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, 'connectionId' | 'executionHostId'>): Repo {
return {
id: 'repo-1',
path: '/srv/project',
displayName: 'project',
badgeColor: '#000',
addedAt: 0,
kind: 'git',
...fields
}
}
const PROVIDERS = [
[
'GitHub',
(resolveRepo: () => Promise<Repo>) =>
resolveRuntimeGitHubWorktreeBase(
{ repoSelector: 'id:repo-1', prNumber: 7, headRefName: 'feature' },
{ store, resolveRepo }
)
],
[
'GitLab',
(resolveRepo: () => Promise<Repo>) =>
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:<id>` 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()
})
}
})