diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index d19d34e545d..845d0936780 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -424,6 +424,10 @@ jobs: if: needs.code_paths.outputs.static_analysis == 'true' run: node config/scripts/check-id-only-store-write-ratchet.mjs + - name: Enforce repo host-read ratchet + if: needs.code_paths.outputs.static_analysis == 'true' + run: node config/scripts/check-repo-host-read-ratchet.mjs + - name: Check Node runtime pin if: needs.code_paths.outputs.static_analysis == 'true' run: node config/scripts/check-node-runtime-pin.mjs diff --git a/config/repo-host-read-baseline.txt b/config/repo-host-read-baseline.txt new file mode 100644 index 00000000000..572a007576e --- /dev/null +++ b/config/repo-host-read-baseline.txt @@ -0,0 +1,68 @@ +# Raw reads of a repo row's legacy `connectionId` (Codex review 6 #4), as ` `. +# This is a RATCHET: counts may only go DOWN. Resolve the host instead (see fix text). +# A lower bound: destructured and differently named rows are not counted. +# Prune after a fix: pnpm check:repo-host-read-ratchet --prune +1 src/cli/automation-destination.ts +1 src/main/folder-repo-git-conversion.ts +1 src/main/github/account-binding-required-host.ts +3 src/main/ipc/ephemeral-vm-recipe-context.ts +2 src/main/ipc/filesystem/filesystem-source-control-ai-targets.ts +1 src/main/ipc/folder-repo-git-upgrade.ts +2 src/main/ipc/github-repo-routing.ts +1 src/main/ipc/github-work-item-args.ts +1 src/main/ipc/gitlab-repo-access.ts +1 src/main/ipc/local-worktree-runtime-options.ts +1 src/main/ipc/registered-worktree-root-owner.ts +4 src/main/ipc/repos/local-repo-registration.ts +1 src/main/ipc/repos/nested-repo-import-handler.ts +1 src/main/ipc/repos/repo-git-username-handler.ts +2 src/main/ipc/workspace-cleanup-candidate.ts +1 src/main/ipc/workspace-cleanup-disconnected-ssh.ts +2 src/main/ipc/worktree-base-directory-watch-targets.ts +1 src/main/ipc/worktrees-test-metadata-pruning-store.ts +1 src/main/ipc/worktrees/listing/authoritative-local-worktree-metadata-pruning.ts +1 src/main/ipc/worktrees/listing/register-host-catalog-handlers.ts +2 src/main/ipc/worktrees/removal/execute-worktree-removal.ts +2 src/main/ipc/worktrees/removal/nested-worktree-removal.ts +2 src/main/ipc/worktrees/removal/register-worktree-forget-handlers.ts +1 src/main/ipc/worktrees/removal/register-worktree-removal-handlers.ts +1 src/main/ipc/worktrees/repo-host-ownership.ts +2 src/main/orca-profiles/profile-project-transfer-payload.ts +1 src/main/orca-profiles/profile-project-worktree-identity.ts +1 src/main/persistence/leasing-ssh-ptys/ssh-target-reassignment.ts +1 src/main/persistence/migrating-orcad-catalog/orcad-source-ownership.ts +2 src/main/persistence/restoring-sessions/folder-scope-migration.ts +1 src/main/persistence/restoring-sessions/tab-terminal-pane-binding-repair.ts +2 src/main/persistence/scheduling-automations/automation-definition-operations.ts +1 src/main/persistence/scheduling-automations/automation-owner-projection.ts +1 src/main/persistence/tracking-repos/local-worktree-metadata-scan-expectation.ts +1 src/main/persistence/tracking-repos/worktree-metadata-normalization.ts +1 src/main/ports/ssh-advertised-url-enrichment.ts +2 src/main/ports/workspace-port-ownership.ts +5 src/main/project-groups/folder-workspace-path-status.ts +1 src/main/provisioned-root-ssh-adoption.ts +2 src/main/repo-git-username-enrichment.ts +4 src/main/runtime/missing-worktree-terminal-reconciliation.ts +1 src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts +1 src/main/runtime/runtime-agent-launch-resolution.ts +1 src/main/runtime/runtime-managed-worktree-queries.ts +3 src/main/runtime/runtime-reference-agents.ts +1 src/main/runtime/runtime-terminal-spawn-push-target-materialization.ts +1 src/main/runtime/runtime-worktree-drift-probe.ts +1 src/main/runtime/runtime-worktree-selection.ts +1 src/main/runtime/terminal/wait/terminal-interactive-wait.ts +1 src/main/runtime/workspaces/workspace-host-notifications.ts +1 src/main/runtime/workspaces/workspace-selectors.ts +1 src/main/ssh/targets/ssh-target-orcad-claims.ts +1 src/main/workspace-space-worktree-row.ts +1 src/main/worktree-create-base-prefetch.ts +2 src/main/worktree-name-retirement.ts +1 src/shared/automation-execution-target.ts +1 src/shared/endpoint-catalog-adoption.ts +1 src/shared/folder-workspace-execution-host.ts +1 src/shared/github/github-cache-key.ts +1 src/shared/new-workspace-dialog-repo.ts +1 src/shared/sidebar-list/host-section-rows.ts +1 src/shared/sidebar-list/workspace-review-facts.ts +1 src/shared/workspace-session-terminal-buffers.ts +1 src/shared/worktree/ownership.ts diff --git a/config/scripts/check-repo-host-read-ratchet.mjs b/config/scripts/check-repo-host-read-ratchet.mjs new file mode 100644 index 00000000000..c8aeb13b0b6 --- /dev/null +++ b/config/scripts/check-repo-host-read-ratchet.mjs @@ -0,0 +1,85 @@ +import process from 'node:process' +import { pathToFileURL } from 'node:url' +import { stripSlashComments } from './check-path-literal-ratchet.mjs' +import { readTrackedSources, runCountRatchet } from './ratchet-baseline.mjs' + +// Ratchet on host decisions read off a repo row's legacy `connectionId` (Codex review 6 #4). A row +// may name its SSH host only as `executionHostId: 'ssh:'`, so a raw `repo.connectionId` +// check answers "local" for it and runs remote work on this machine. Counts may only shrink. +// A lower bound, not a proof: destructuring, indexed access and rows not named `*repo*`/`*Repo*` +// are not seen, and the renderer is out of scope. + +const BASELINE_PATH = 'config/repo-host-read-baseline.txt' +const SCAN_ROOTS = ['src/main/', 'src/shared/', 'src/cli/'] +// A property read on a repo-named receiver; an assignment target is a write, not a decision. +// `repo`, `sourceRepo`, `repoRow`; not `report` or `repos`. +const RAW_READ = /\b(?:repo|\w*Repo)(?:[A-Z]\w*)?!?\??\.connectionId\b(?!\s*=(?!=))/g + +/** Files that define what the two spellings mean, with the reason. Keep each reason true. */ +export const RESOLVER_FILES = new Map([ + ['src/shared/execution-host.ts', 'the canonical resolvers read both spellings by definition'], + ['src/main/repo-execution-host.ts', 'the main-process gates built on those resolvers'], + [ + 'src/main/ipc/worktrees/listing/worktree-host-ownership.ts', + 'ownership evidence must see a contradictory row as contradictory' + ] +]) + +export function isScannedFile(rel) { + return ( + rel.endsWith('.ts') && + !rel.endsWith('.d.ts') && + !/\.(?:test|spec)\.ts$|fixture/.test(rel) && + !RESOLVER_FILES.has(rel) + ) +} + +// Copying the field verbatim into a projected row decides nothing; that row's reader does. +const VERBATIM_COPY = + /\.\.\.\(\s*(\w+)\.connectionId !== undefined \? \{ connectionId: \1\.connectionId \} : \{\}\s*\)/g + +/** Raw `.connectionId` reads in one file's code; comments are prose and do not count. */ +export function countRawRepoHostReads(text) { + const code = stripSlashComments(text).replace(VERBATIM_COPY, '') + return [...code.matchAll(RAW_READ)].length +} + +export function collectRepoHostReads(root = process.cwd()) { + const counts = new Map() + for (const [rel, text] of readTrackedSources(root, SCAN_ROOTS, isScannedFile)) { + const count = countRawRepoHostReads(text) + if (count > 0) { + counts.set(rel, count) + } + } + return counts +} + +export function main(root = process.cwd(), arg = undefined) { + return runCountRatchet( + { + name: 'repo-host-read', + baselinePath: BASELINE_PATH, + unit: 'raw repo connectionId read(s)', + header: [ + "# Raw reads of a repo row's legacy `connectionId` (Codex review 6 #4), as ` `.", + '# This is a RATCHET: counts may only go DOWN. Resolve the host instead (see fix text).', + '# A lower bound: destructured and differently named rows are not counted.', + '# Prune after a fix: pnpm check:repo-host-read-ratchet --prune' + ], + collect: collectRepoHostReads, + fix: [ + "Resolve the row's host instead of reading `connectionId` (src/shared/execution-host.ts):", + ' getRepoRoutingConnectionId(repo) drop-in for `repo.connectionId` in a routing/SSH check', + ' getRepoExecutionHostId(repo) the host id, for comparisons and host-scoped lookups', + ' requireRepoLocalGitRoute(repo) (src/main/repo-execution-host.ts) before local Git' + ] + }, + root, + arg + ) +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + process.exit(main(process.cwd(), process.argv[2])) +} diff --git a/config/scripts/check-repo-host-read-ratchet.test.mjs b/config/scripts/check-repo-host-read-ratchet.test.mjs new file mode 100644 index 00000000000..6e043c23996 --- /dev/null +++ b/config/scripts/check-repo-host-read-ratchet.test.mjs @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest' + +import { + countRawRepoHostReads, + isScannedFile, + RESOLVER_FILES +} from './check-repo-host-read-ratchet.mjs' + +describe('countRawRepoHostReads', () => { + it('counts host decisions read off a repo row', () => { + const text = [ + 'if (repo.connectionId) {', + 'const id = sourceRepo?.connectionId ?? null', + 'repos.filter((repo) => !repo.connectionId)', + 'if (existingRepo.connectionId === targetId) {', + 'const id = repoRow!.connectionId' + ].join('\n') + expect(countRawRepoHostReads(text)).toBe(5) + }) + + it('ignores writes, comments, type positions and non-repo receivers', () => { + const text = [ + 'repo.connectionId = null', + '// `!repo.connectionId` used to answer local', + "type T = Repo['connectionId']", + 'if (pty.connectionId) {', + 'const repoConnectionId = args.connectionId', + 'if (getRepoRoutingConnectionId(repo)) {', + 'if (report.connectionId) {', + 'for (const r of repos) r.connectionId' + ].join('\n') + expect(countRawRepoHostReads(text)).toBe(0) + }) + + it('ignores a verbatim copy into a projected row, but not a decision beside it', () => { + const copy = '...(repo.connectionId !== undefined ? { connectionId: repo.connectionId } : {}),' + expect(countRawRepoHostReads(copy)).toBe(0) + expect(countRawRepoHostReads(`${copy}\nif (repo.connectionId) {}`)).toBe(1) + const mismatched = + '...(repo.connectionId !== undefined ? { connectionId: sourceRepo.connectionId } : {})' + expect(countRawRepoHostReads(mismatched)).toBe(2) + }) + + it('still counts an equality comparison, which is not an assignment', () => { + expect(countRawRepoHostReads('if (repo.connectionId == null) {}')).toBe(1) + }) +}) + +describe('isScannedFile', () => { + it('scans source, not tests, declarations or the resolvers', () => { + expect(isScannedFile('src/main/worktree-trash.ts')).toBe(true) + expect(isScannedFile('src/main/worktree-trash.test.ts')).toBe(false) + expect(isScannedFile('src/shared/types.d.ts')).toBe(false) + for (const rel of RESOLVER_FILES.keys()) { + expect(isScannedFile(rel)).toBe(false) + } + }) +}) diff --git a/config/scripts/pr-preflight-gates.test.mjs b/config/scripts/pr-preflight-gates.test.mjs index 83d4dd3b3de..b35f4e23e07 100644 --- a/config/scripts/pr-preflight-gates.test.mjs +++ b/config/scripts/pr-preflight-gates.test.mjs @@ -258,6 +258,7 @@ it('pins every foreground and background step to its selected phase', () => { ['Enforce legacy relay fence ratchet', staticPhase], ['Enforce unproven PTY retirement ratchet', staticPhase], ['Enforce id-only store write ratchet', staticPhase], + ['Enforce repo host-read ratchet', staticPhase], ['Check Node runtime pin', staticPhase], ['Decide whether the orcad terminal smoke runs', staticPhase], ['Verify the generated RPC params catalog', staticPhase], diff --git a/package.json b/package.json index b0cb89432cd..1094504d95b 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "audit:perf": "oxlint --config config/oxlint-performance-audit.json --format json src", "test:perf:contracts": "vitest run --config config/vitest.performance.config.ts", "format": "oxfmt --write .", - "lint": "oxlint && pnpm run audit:anti-slop && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:dead-classes && pnpm run check:max-lines-ratchet && pnpm run check:ts-nocheck-ratchet && pnpm run check:runtime-electron-ratchet && pnpm run check:focus-read-ratchet && pnpm run check:layout-writer-ratchets && pnpm run check:path-literal-ratchet && pnpm run check:legacy-relay-fence-ratchet && pnpm run check:unproven-pty-retirement-ratchet && pnpm run check:id-only-store-write-ratchet && pnpm run check:readme-local-links && pnpm run check:node-runtime-pin && pnpm run verify:rpc-params-catalog && pnpm run verify:acp-protocol && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalogs && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage && pnpm run check:process-host-imports && pnpm run check:main-sync-spawn-ratchet && pnpm run check:compat-boundary-imports", + "lint": "oxlint && pnpm run audit:anti-slop && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:dead-classes && pnpm run check:max-lines-ratchet && pnpm run check:ts-nocheck-ratchet && pnpm run check:runtime-electron-ratchet && pnpm run check:focus-read-ratchet && pnpm run check:layout-writer-ratchets && pnpm run check:path-literal-ratchet && pnpm run check:legacy-relay-fence-ratchet && pnpm run check:unproven-pty-retirement-ratchet && pnpm run check:id-only-store-write-ratchet && pnpm run check:repo-host-read-ratchet && pnpm run check:readme-local-links && pnpm run check:node-runtime-pin && pnpm run verify:rpc-params-catalog && pnpm run verify:acp-protocol && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalogs && pnpm run verify:localization-extraction && pnpm run verify:localization-coverage && pnpm run check:process-host-imports && pnpm run check:main-sync-spawn-ratchet && pnpm run check:compat-boundary-imports", "audit:code-quality": "pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run audit:react-doctor", "audit:code-quality:native": "oxlint --config config/oxlint-code-quality-native-plugins.json src config tests mobile --deny-warnings", "audit:code-quality:type-aware": "oxlint --type-aware --config config/oxlint-code-quality-type-aware.json src config tests --deny-warnings", @@ -42,6 +42,7 @@ "check:legacy-relay-fence-ratchet": "node config/scripts/check-legacy-relay-fence-ratchet.mjs", "check:unproven-pty-retirement-ratchet": "node config/scripts/check-unproven-pty-retirement-ratchet.mjs", "check:id-only-store-write-ratchet": "node config/scripts/check-id-only-store-write-ratchet.mjs", + "check:repo-host-read-ratchet": "node config/scripts/check-repo-host-read-ratchet.mjs", "check:worktree-id-composer-ratchet": "node config/scripts/check-worktree-id-composer-ratchet.mjs", "check:e2e-typecheck-ratchet": "node config/scripts/check-e2e-typecheck-ratchet.mjs", "check:runtime-electron-ratchet": "node config/scripts/check-runtime-electron-ratchet.mjs", diff --git a/src/main/github/pr-head-tracking-ref.test.ts b/src/main/github/pr-head-tracking-ref.test.ts index df70d6ba060..f01c487ea70 100644 --- a/src/main/github/pr-head-tracking-ref.test.ts +++ b/src/main/github/pr-head-tracking-ref.test.ts @@ -40,6 +40,13 @@ describe('fetchPrHeadTrackingRef', () => { expect(gitExecFileAsyncMock).not.toHaveBeenCalled() }) + it('throws for an SSH repo named only by executionHostId', async () => { + await expect( + fetchPrHeadTrackingRef({ path: '/repo', executionHostId: 'ssh:gpu' }, 'origin', 'feature/x') + ).rejects.toThrow('SSH Git provider is not available') + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + }) + it('fetches a GitHub pull head into its remote-scoped Orca ref for local repos', async () => { const localRef = await fetchGitHubPullRequestHeadRef( { path: '/repo', connectionId: null }, diff --git a/src/main/github/pr-head-tracking-ref.ts b/src/main/github/pr-head-tracking-ref.ts index f74057aa3e0..b6147de4989 100644 --- a/src/main/github/pr-head-tracking-ref.ts +++ b/src/main/github/pr-head-tracking-ref.ts @@ -10,6 +10,8 @@ import { SSH_GIT_RECONNECT_MESSAGE, SshHostNotServedError } from '../providers/ssh-host-not-served-error' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' +import type { Repo } from '../../shared/repo-types' type LocalGitExecOptions = { cwd: string @@ -17,13 +19,14 @@ type LocalGitExecOptions = { } export async function fetchPrHeadTrackingRef( - repo: { path: string; connectionId?: string | null }, + repo: { path: string } & Pick, remote: string, branch: string, options: { localGitExecOptions?: LocalGitExecOptions } = {} ): Promise { - if (repo.connectionId) { - throw new SshHostNotServedError(repo.connectionId, SSH_GIT_RECONNECT_MESSAGE) + const sshTargetId = getRepoRoutingConnectionId(repo) + if (sshTargetId) { + throw new SshHostNotServedError(sshTargetId, SSH_GIT_RECONNECT_MESSAGE) } await gitExecFileAsync( ['fetch', remote, `+refs/heads/${branch}:refs/remotes/${remote}/${branch}`], @@ -32,7 +35,7 @@ export async function fetchPrHeadTrackingRef( } export async function fetchGitHubPullRequestHeadRef( - repo: { path: string; connectionId?: string | null }, + repo: { path: string } & Pick, remote: string, prNumber: number, options: { localGitExecOptions?: LocalGitExecOptions } = {} @@ -43,8 +46,9 @@ export async function fetchGitHubPullRequestHeadRef( if (!isSafeReviewHeadFetchRemote(remote)) { throw new Error('Pull request fetch remote must not start with "-".') } - if (repo.connectionId) { - throw new SshHostNotServedError(repo.connectionId, SSH_GIT_RECONNECT_MESSAGE) + const sshTargetId = getRepoRoutingConnectionId(repo) + if (sshTargetId) { + throw new SshHostNotServedError(sshTargetId, SSH_GIT_RECONNECT_MESSAGE) } const localGitExecOptions = options.localGitExecOptions ?? { cwd: repo.path } const remoteComponent = await getReviewHeadRemoteComponent(remote, localGitExecOptions) diff --git a/src/main/gitlab/mr-head-tracking-ref.ts b/src/main/gitlab/mr-head-tracking-ref.ts index 9f5f6170f77..7b9dc97bdb1 100644 --- a/src/main/gitlab/mr-head-tracking-ref.ts +++ b/src/main/gitlab/mr-head-tracking-ref.ts @@ -10,6 +10,8 @@ import { SSH_GIT_RECONNECT_MESSAGE, SshHostNotServedError } from '../providers/ssh-host-not-served-error' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' +import type { Repo } from '../../shared/repo-types' type LocalGitExecOptions = { cwd: string @@ -18,7 +20,7 @@ type LocalGitExecOptions = { // Why: mirrors fetchGitHubPullRequestHeadRef so both forges pin the durable head ref the same way. export async function fetchGitLabMergeRequestHeadRef( - repo: { path: string; connectionId?: string | null }, + repo: { path: string } & Pick, remote: string, mrIid: number, options: { localGitExecOptions?: LocalGitExecOptions } = {} @@ -29,8 +31,9 @@ export async function fetchGitLabMergeRequestHeadRef( if (!isSafeReviewHeadFetchRemote(remote)) { throw new Error('Merge request fetch remote must not start with "-".') } - if (repo.connectionId) { - throw new SshHostNotServedError(repo.connectionId, SSH_GIT_RECONNECT_MESSAGE) + const sshTargetId = getRepoRoutingConnectionId(repo) + if (sshTargetId) { + throw new SshHostNotServedError(sshTargetId, SSH_GIT_RECONNECT_MESSAGE) } const localGitExecOptions = options.localGitExecOptions ?? { cwd: repo.path } const remoteComponent = await getReviewHeadRemoteComponent(remote, localGitExecOptions) diff --git a/src/main/ipc/repos/base-ref-query-handlers.ts b/src/main/ipc/repos/base-ref-query-handlers.ts index 6cff029445c..df4b1fcd274 100644 --- a/src/main/ipc/repos/base-ref-query-handlers.ts +++ b/src/main/ipc/repos/base-ref-query-handlers.ts @@ -7,7 +7,11 @@ import { isRepoSearchRefsRequestLimit } from '../../../shared/repo-search-limits' import { isFolderRepo } from '../../../shared/repo-kind' -import { getRepoExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' +import { + getRepoExecutionHostId, + getRepoRoutingConnectionId, + type ExecutionHostId +} from '../../../shared/execution-host' import { getBaseRefDefault, getRemoteCount, searchBaseRefDetails } from '../../git/repo' export function registerBaseRefQueryHandlers(store: Store): void { @@ -23,7 +27,7 @@ export function registerBaseRefQueryHandlers(store: Store): void { return { defaultBaseRef: null, remoteCount: 0 } } // Why: an SSH repo's git data lives on a host this process does not serve. - if (repo.connectionId) { + if (getRepoRoutingConnectionId(repo)) { return { defaultBaseRef: null, remoteCount: 0 } } // Why: run in parallel; a remote-count failure must not break default detection. @@ -71,7 +75,7 @@ async function searchBaseRefDetailsForRepo( // Keep the public IPC shape forgiving while bounding Git and retained results // for callers that request an unusually large page. const limit = clampRepoSearchRefsLimit(requestedLimit) - if (repo.connectionId) { + if (getRepoRoutingConnectionId(repo)) { return [] } return searchBaseRefDetails(repo.path, args.query, limit) diff --git a/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts b/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts index c56955fa5da..d5423331cf1 100644 --- a/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts +++ b/src/main/ipc/worktrees-ssh-pr-head-fetch.test.ts @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { REVIEW_HEAD_FETCH_TIMEOUT_MS } from '../../shared/review-head-tracking-ref' import { getPullRequestPushTargetMock, gitExecFileAsyncMock } from './worktrees-test-module-mocks' -import { handlers, setupWorktreeHandlers } from './worktrees-test-harness' +import { handlers, harnessRepo, setupWorktreeHandlers, store } from './worktrees-test-harness' import { ORIGIN_HEAD_COMPONENT, ORIGIN_REMOTE_URL } from './worktrees-test-fixtures' import type { WorktreeRuntimeStub } from './worktrees-test-runtime-stub' @@ -127,6 +127,23 @@ describe('registerWorktreeHandlers', () => { }) }) + // Why: a row may name its SSH host only as `executionHostId`; local git must not run for it. + it('refuses local git for a PR base on an executionHostId-only SSH repo', async () => { + store.getRepo.mockReturnValue({ ...harnessRepo, executionHostId: 'ssh:gpu' }) + gitExecFileAsyncMock.mockResolvedValue({ stdout: `${ORIGIN_REMOTE_URL}\n`, stderr: '' }) + + const result = await Promise.resolve( + handlers['worktrees:resolvePrBase'](null, { + repoId: 'repo-1', + prNumber: 1849, + headRefName: 'feat/x' + }) + ).catch((error: unknown) => error) + + expect(gitExecFileAsyncMock).not.toHaveBeenCalled() + expect(JSON.stringify(result)).toMatch(/SSH/) + }) + it('falls back to refs/pull//head when branch fetch fails for a PR', async () => { gitExecFileAsyncMock.mockImplementation(async (args: string[]) => { if ( diff --git a/src/main/ipc/worktrees/create/register-review-base-handlers.ts b/src/main/ipc/worktrees/create/register-review-base-handlers.ts index bb2842e5756..dcf7e97d1bf 100644 --- a/src/main/ipc/worktrees/create/register-review-base-handlers.ts +++ b/src/main/ipc/worktrees/create/register-review-base-handlers.ts @@ -1,6 +1,7 @@ import { ipcMain } from 'electron' import type { GitHubPrStartPoint, GitPushTarget } from '../../../../shared/worktree/types' import { isFolderRepo } from '../../../../shared/repo-kind' +import { getRepoRoutingConnectionId } from '../../../../shared/execution-host' import { gitExecFileAsync } from '../../../git/runner' import { getLocalProjectGitExecOptions, @@ -40,13 +41,15 @@ export function registerReviewBaseHandlers(context: WorktreeIpcContext): void { if (isFolderRepo(repo)) { return { error: 'Folder mode does not support creating worktrees.' } } + // Why the resolver: a row may name its SSH host only as `executionHostId: 'ssh:'`. + const connectionId = getRepoRoutingConnectionId(repo) const gitExec = async (args: string[]): Promise<{ stdout: string; stderr: string }> => { - if (!repo.connectionId) { + if (!connectionId) { return gitExecFileAsync(args, getLocalProjectGitExecOptions(store, repo)) } - throw new SshHostNotServedError(repo.connectionId, SSH_GIT_RECONNECT_MESSAGE) + throw new SshHostNotServedError(connectionId, SSH_GIT_RECONNECT_MESSAGE) } - const localGhOptions = repo.connectionId + const localGhOptions = connectionId ? repo.ghAccount ? { ghAccount: repo.ghAccount } : undefined @@ -67,7 +70,7 @@ export function registerReviewBaseHandlers(context: WorktreeIpcContext): void { baseRefName: args.baseRefName, isCrossRepository: args.isCrossRepository, issueSourcePreference: repo.issueSourcePreference, - connectionId: repo.connectionId ?? null, + connectionId, localGitOptions: localGhOptions, gitExec, fetchRemoteTrackingRef, @@ -78,7 +81,7 @@ export function registerReviewBaseHandlers(context: WorktreeIpcContext): void { resolveGitHubReviewHeadRemote({ repoPath: repo.path, issueSourcePreference: repo.issueSourcePreference, - connectionId: repo.connectionId ?? null, + connectionId, localGitOptions: localGhOptions, gitExec }) diff --git a/src/main/runtime/runtime-github-issue-comment-commands.ts b/src/main/runtime/runtime-github-issue-comment-commands.ts index e3f19c36d76..5990f71b485 100644 --- a/src/main/runtime/runtime-github-issue-comment-commands.ts +++ b/src/main/runtime/runtime-github-issue-comment-commands.ts @@ -10,6 +10,7 @@ import { createIssue, updateIssue } from '../github/client' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' type LocalGitArgs = [] | [LocalProjectGhExecOptions] @@ -33,7 +34,7 @@ export class RuntimeGitHubIssueCommentCommands { title, body, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), fields, ...this.deps.getLocalGitArgs(repo) ) @@ -49,7 +50,7 @@ export class RuntimeGitHubIssueCommentCommands { repo.path, number, updates, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), this.deps.getLocalGitArgs(repo)[0], repo.issueSourcePreference ) @@ -67,7 +68,7 @@ export class RuntimeGitHubIssueCommentCommands { repo.path, number, body, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, this.deps.getLocalGitArgs(repo)[0], // Why: the issue source selector only scopes issues; PR comments keep their resolution. @@ -82,7 +83,7 @@ export class RuntimeGitHubIssueCommentCommands { const repo = await this.deps.resolveRepo(repoSelector) return addPRReviewComment({ repoPath: repo.path, - connectionId: repo.connectionId ?? null, + connectionId: getRepoRoutingConnectionId(repo), localGitOptions: this.deps.getLocalGitArgs(repo)[0], ...args }) @@ -109,7 +110,7 @@ export class RuntimeGitHubIssueCommentCommands { args.threadId, args.path, args.line, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), args.prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) diff --git a/src/main/runtime/runtime-github-repository-query-commands.ts b/src/main/runtime/runtime-github-repository-query-commands.ts index fd1e950d15c..bef1026e851 100644 --- a/src/main/runtime/runtime-github-repository-query-commands.ts +++ b/src/main/runtime/runtime-github-repository-query-commands.ts @@ -18,6 +18,7 @@ import { import { getRateLimit } from '../github/rate-limit' import { resolveGhAccountRequiredHost } from '../github/account-binding-required-host' import { getWorkItemDetails } from '../github/work-item-details' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' type LocalGitArgs = [] | [LocalProjectGhExecOptions] @@ -43,7 +44,7 @@ export class RuntimeGitHubRepositoryQueryCommands { query, page, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), noCache, ...this.deps.getLocalGitArgs(repo) ) @@ -58,7 +59,7 @@ export class RuntimeGitHubRepositoryQueryCommands { repo.path, limit, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) return result.items @@ -74,7 +75,7 @@ export class RuntimeGitHubRepositoryQueryCommands { repo.path, number, type, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), this.deps.getLocalGitArgs(repo)[0] ?? {}, repo.issueSourcePreference ) @@ -92,7 +93,7 @@ export class RuntimeGitHubRepositoryQueryCommands { ownerRepo, number, type, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -107,7 +108,7 @@ export class RuntimeGitHubRepositoryQueryCommands { repo.path, number, type, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), this.deps.getLocalGitArgs(repo)[0] ?? {}, repo.issueSourcePreference ) @@ -119,7 +120,7 @@ export class RuntimeGitHubRepositoryQueryCommands { repo.path, query, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -129,7 +130,7 @@ export class RuntimeGitHubRepositoryQueryCommands { return listLabels( repo.path, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -141,7 +142,7 @@ export class RuntimeGitHubRepositoryQueryCommands { return listAssignableUsers( repo.path, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } diff --git a/src/main/runtime/runtime-github-review-mutation-commands.ts b/src/main/runtime/runtime-github-review-mutation-commands.ts index 82182286674..61390fd39d1 100644 --- a/src/main/runtime/runtime-github-review-mutation-commands.ts +++ b/src/main/runtime/runtime-github-review-mutation-commands.ts @@ -17,6 +17,7 @@ import { updatePRState, updatePRTitle } from '../github/client' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' type LocalGitArgs = [] | [LocalProjectGhExecOptions] @@ -38,7 +39,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, prNumber, options, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -56,7 +57,7 @@ export class RuntimeGitHubReviewMutationCommands { reactionSubjectId, content, reacted, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -73,7 +74,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, threadId, resolve, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -91,7 +92,7 @@ export class RuntimeGitHubReviewMutationCommands { const repo = await this.deps.resolveRepo(repoSelector) return setPRFileViewed({ repoPath: repo.path, - connectionId: repo.connectionId ?? null, + connectionId: getRepoRoutingConnectionId(repo), localGitOptions: this.deps.getLocalGitArgs(repo)[0], ...args }) @@ -108,7 +109,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, prNumber, title, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -125,7 +126,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, prNumber, updates, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -142,7 +143,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, prNumber, method, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -161,7 +162,7 @@ export class RuntimeGitHubReviewMutationCommands { prNumber, enabled, method, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -176,7 +177,7 @@ export class RuntimeGitHubReviewMutationCommands { return markPRReadyForReview( repo.path, prNumber, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -193,7 +194,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, prNumber, updates, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -210,7 +211,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, prNumber, reviewers, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) @@ -227,7 +228,7 @@ export class RuntimeGitHubReviewMutationCommands { repo.path, prNumber, reviewers, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), prRepo ?? null, ...this.deps.getLocalGitArgs(repo) ) diff --git a/src/main/runtime/runtime-github-review-query-commands.ts b/src/main/runtime/runtime-github-review-query-commands.ts index e9d24864828..355500f08cc 100644 --- a/src/main/runtime/runtime-github-review-query-commands.ts +++ b/src/main/runtime/runtime-github-review-query-commands.ts @@ -3,6 +3,7 @@ import type { Repo } from '../../shared/repo-types' import type { LocalProjectGhExecOptions } from '../project-runtime-git-options' import { getIssue, getPRCheckDetails, getPRChecks, getPRComments } from '../github/client' import { getPRFileContents } from '../github/work-item-details' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' type LocalGitArgs = [] | [LocalProjectGhExecOptions] @@ -22,7 +23,7 @@ export class RuntimeGitHubReviewQueryCommands { return getIssue( repo.path, number, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -41,7 +42,7 @@ export class RuntimeGitHubReviewQueryCommands { headSha, prRepo ?? null, options, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -62,7 +63,7 @@ export class RuntimeGitHubReviewQueryCommands { return getPRCheckDetails( repo.path, { ...args, prRepo: args.prRepo ?? null }, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), localGitOptions, signal ) @@ -79,7 +80,7 @@ export class RuntimeGitHubReviewQueryCommands { repo.path, prNumber, { ...options, prRepo: prRepo ?? null }, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -99,7 +100,7 @@ export class RuntimeGitHubReviewQueryCommands { const repo = await this.deps.resolveRepo(repoSelector) return getPRFileContents({ repoPath: repo.path, - connectionId: repo.connectionId ?? null, + connectionId: getRepoRoutingConnectionId(repo), localGitOptions: this.deps.getLocalGitArgs(repo)[0], ...args }) diff --git a/src/main/runtime/runtime-gitlab-mutation-commands.ts b/src/main/runtime/runtime-gitlab-mutation-commands.ts index 171afca6fab..c5f80cef22d 100644 --- a/src/main/runtime/runtime-gitlab-mutation-commands.ts +++ b/src/main/runtime/runtime-gitlab-mutation-commands.ts @@ -20,6 +20,7 @@ import { updateMR, updateMRReviewers } from '../gitlab/client' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' type LocalGitArgs = [] | [{ wslDistro?: string }] @@ -38,7 +39,7 @@ export class RuntimeGitLabMutationCommands { title, body, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -55,7 +56,7 @@ export class RuntimeGitLabMutationCommands { number, updates, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -73,7 +74,7 @@ export class RuntimeGitLabMutationCommands { number, body, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -91,7 +92,7 @@ export class RuntimeGitLabMutationCommands { iid, body, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -109,7 +110,7 @@ export class RuntimeGitLabMutationCommands { iid, input, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -129,7 +130,7 @@ export class RuntimeGitLabMutationCommands { discussionId, resolved, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -145,7 +146,7 @@ export class RuntimeGitLabMutationCommands { repo.path, jobId, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -161,7 +162,7 @@ export class RuntimeGitLabMutationCommands { repo.path, jobId, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -179,7 +180,7 @@ export class RuntimeGitLabMutationCommands { iid, method ?? 'merge', repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -197,7 +198,7 @@ export class RuntimeGitLabMutationCommands { repo.path, iid, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -215,7 +216,7 @@ export class RuntimeGitLabMutationCommands { iid, updates, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) @@ -233,7 +234,7 @@ export class RuntimeGitLabMutationCommands { iid, reviewerIds, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, ...this.deps.getLocalGitArgs(repo) ) diff --git a/src/main/runtime/runtime-gitlab-query-commands.ts b/src/main/runtime/runtime-gitlab-query-commands.ts index 5a994cb9188..8ff8c69ef75 100644 --- a/src/main/runtime/runtime-gitlab-query-commands.ts +++ b/src/main/runtime/runtime-gitlab-query-commands.ts @@ -18,6 +18,7 @@ import { type GitLabIssueListState } from '../gitlab/gitlab-preload-args' import { getWorkItemDetails } from '../gitlab/work-item-details' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' type LocalGitArgs = [] | [{ wslDistro?: string }] @@ -45,7 +46,7 @@ export class RuntimeGitLabQueryCommands { perPage ?? 20, repo.issueSourcePreference, query, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -65,7 +66,7 @@ export class RuntimeGitLabQueryCommands { normalizeGitLabPositiveInteger(perPage, 20, 100), repo.issueSourcePreference, query, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -90,7 +91,7 @@ export class RuntimeGitLabQueryCommands { repo.issueSourcePreference, normalized.state, normalized.assignee, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), this.deps.getLocalGitArgs(repo)[0] ?? {}, normalized.page ) @@ -116,7 +117,11 @@ export class RuntimeGitLabQueryCommands { async listGitLabRepoTodos(repoSelector: string): Promise>> { const repo = await this.deps.resolveRepo(repoSelector) - return listTodos(repo.path, repo.connectionId ?? null, ...this.deps.getLocalGitArgs(repo)) + return listTodos( + repo.path, + getRepoRoutingConnectionId(repo), + ...this.deps.getLocalGitArgs(repo) + ) } diagnoseGitLabAuth(): Promise>> { @@ -137,7 +142,7 @@ export class RuntimeGitLabQueryCommands { return listLabels( repo.path, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -149,7 +154,7 @@ export class RuntimeGitLabQueryCommands { return listAssignableUsers( repo.path, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) } @@ -167,7 +172,7 @@ export class RuntimeGitLabQueryCommands { iid, type, repo.issueSourcePreference, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), projectRef, this.deps.getLocalGitArgs(repo)[0] ?? {}, { includeReviewDetails } @@ -186,7 +191,7 @@ export class RuntimeGitLabQueryCommands { projectRef, iid, type, - repo.connectionId ?? null, + getRepoRoutingConnectionId(repo), ...this.deps.getLocalGitArgs(repo) ) // Why: successful remote pasted-URL lookups update the same recents as desktop IPC. diff --git a/src/main/runtime/runtime-preserved-branch-cleanup.ts b/src/main/runtime/runtime-preserved-branch-cleanup.ts index 6bb2f7d70fc..1b51ef498eb 100644 --- a/src/main/runtime/runtime-preserved-branch-cleanup.ts +++ b/src/main/runtime/runtime-preserved-branch-cleanup.ts @@ -4,7 +4,11 @@ import type { } from '../../shared/worktree/create-types' import type { GitPushTarget } from '../../shared/worktree/types' import { preservedBranchCleanupScopeKey } from '../../shared/preserved-branch-cleanup' -import { parseExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' +import { + getRepoRoutingConnectionId, + parseExecutionHostId, + type ExecutionHostId +} from '../../shared/execution-host' import { isFolderRepo } from '../../shared/repo-kind' import { forceDeleteLocalBranch } from '../git/worktree' import { gitExecFileAsync } from '../git/runner' @@ -125,8 +129,9 @@ export class RuntimePreservedBranchCleanup { throw new Error('Folder workspaces do not have local Git branches.') } - if (repo.connectionId) { - throw new SshHostNotServedError(repo.connectionId) + const sshTargetId = getRepoRoutingConnectionId(repo) + if (sshTargetId) { + throw new SshHostNotServedError(sshTargetId) } const options = getLocalProjectWorktreeGitOptions(store, repo) await (Object.keys(options).length > 0 diff --git a/src/main/runtime/runtime-provider-command-ssh-routing.test.ts b/src/main/runtime/runtime-provider-command-ssh-routing.test.ts new file mode 100644 index 00000000000..28faf576cf9 --- /dev/null +++ b/src/main/runtime/runtime-provider-command-ssh-routing.test.ts @@ -0,0 +1,71 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../shared/repo-types' + +const { getIssueMock, listTodosMock } = vi.hoisted(() => ({ + getIssueMock: vi.fn(), + listTodosMock: vi.fn() +})) +vi.mock('../github/client', () => ({ + getIssue: getIssueMock, + getPRCheckDetails: vi.fn(), + getPRChecks: vi.fn(), + getPRComments: vi.fn() +})) +vi.mock('../github/work-item-details', () => ({ getPRFileContents: vi.fn() })) +vi.mock('../gitlab/client', async (importOriginal) => ({ + ...(await importOriginal>()), + listTodos: listTodosMock +})) + +import { RuntimeGitHubReviewQueryCommands } from './runtime-github-review-query-commands' +import { RuntimeGitLabQueryCommands } from './runtime-gitlab-query-commands' + +function repoOn(fields: Pick): Repo { + return { + id: 'repo-1', + path: '/srv/project', + displayName: 'project', + badgeColor: '#000', + addedAt: 0, + kind: 'git', + ...fields + } +} + +// Why: these commands used to pass the raw `connectionId`, so an `ssh:`-only row ran gh/glab here. +describe('runtime provider commands route an SSH project to its host', () => { + beforeEach(() => { + getIssueMock.mockReset().mockResolvedValue(null) + listTodosMock.mockReset().mockResolvedValue([]) + }) + + it.each([ + ['executionHostId only', { executionHostId: 'ssh:gpu' as const }, 'gpu'], + ['legacy connectionId', { connectionId: 'gpu' }, 'gpu'], + ['local', { executionHostId: 'local' as const }, null] + ])('GitHub issue lookup (%s)', async (_label, fields, expected) => { + const commands = new RuntimeGitHubReviewQueryCommands({ + resolveRepo: async () => repoOn(fields), + getLocalGitArgs: () => [] + }) + + await commands.getRepoIssue('id:repo-1', 7) + + expect(getIssueMock).toHaveBeenCalledWith('/srv/project', 7, expected) + }) + + it.each([ + ['executionHostId only', { executionHostId: 'ssh:gpu' as const }, 'gpu'], + ['local', { executionHostId: 'local' as const }, null] + ])('GitLab todo list (%s)', async (_label, fields, expected) => { + const commands = new RuntimeGitLabQueryCommands({ + resolveRepo: async () => repoOn(fields), + getLocalGitArgs: () => [], + recordProjectRecent: () => {} + }) + + await commands.listGitLabRepoTodos('id:repo-1') + + expect(listTodosMock).toHaveBeenCalledWith('/srv/project', expected) + }) +}) diff --git a/src/main/runtime/runtime-repository-ref-queries.ts b/src/main/runtime/runtime-repository-ref-queries.ts index 1931da9bf52..222422c34dc 100644 --- a/src/main/runtime/runtime-repository-ref-queries.ts +++ b/src/main/runtime/runtime-repository-ref-queries.ts @@ -7,6 +7,7 @@ import { isRepoSearchRefsRequestLimit } from '../../shared/repo-search-limits' import { getBaseRefDefault, getRemoteCount, searchBaseRefDetails } from '../git/repo' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' type RuntimeRepositoryRefQueryDependencies = { resolveRepo: (selector: string) => Promise @@ -31,7 +32,7 @@ export class RuntimeRepositoryRefQueries { return { refs: [], truncated: false } } // Why: an `ssh:` repo is not served here; it reports no refs and never searches this machine. - const refDetails = repo.connectionId + const refDetails = getRepoRoutingConnectionId(repo) ? [] : await searchBaseRefDetails(repo.path, query, probeLimit, includeQualifiedRefs) return { @@ -51,7 +52,7 @@ export class RuntimeRepositoryRefQueries { if (isFolderRepo(repo)) { return { defaultBaseRef: null, remoteCount: 0 } } - if (repo.connectionId) { + if (getRepoRoutingConnectionId(repo)) { return { defaultBaseRef: null, remoteCount: 0 } } const [defaultBaseRef, remoteCount] = await Promise.all([ diff --git a/src/main/runtime/runtime-worktree-scan-cache.ts b/src/main/runtime/runtime-worktree-scan-cache.ts index 5c12a260c98..21280376948 100644 --- a/src/main/runtime/runtime-worktree-scan-cache.ts +++ b/src/main/runtime/runtime-worktree-scan-cache.ts @@ -1,11 +1,14 @@ import type { Repo } from '../../shared/repo-types' import { isAgentScratchRepoRootPath } from '../../shared/agent-scratch-worktrees' +import { getRepoRoutingConnectionId } from '../../shared/execution-host' const WORKTREE_SCAN_CACHE_TTL_MS = 30_000 const WORKTREE_SCAN_AGENT_SCRATCH_TTL_MS = 5 * 60_000 -export function resolveWorktreeScanCacheTtlMs(repo: Pick): number { - return !repo.connectionId && isAgentScratchRepoRootPath(repo.path) +export function resolveWorktreeScanCacheTtlMs( + repo: Pick +): number { + return !getRepoRoutingConnectionId(repo) && isAgentScratchRepoRootPath(repo.path) ? WORKTREE_SCAN_AGENT_SCRATCH_TTL_MS : WORKTREE_SCAN_CACHE_TTL_MS } diff --git a/src/main/usage-worktree-metadata.ts b/src/main/usage-worktree-metadata.ts index 2060f22a4f7..a0d74f4a12b 100644 --- a/src/main/usage-worktree-metadata.ts +++ b/src/main/usage-worktree-metadata.ts @@ -7,6 +7,7 @@ import { } from '../shared/worktree/id' import { isFolderRepo } from '../shared/repo-kind' import type { Store } from './persistence' +import { getRepoRoutingConnectionId } from '../shared/execution-host' export type UsageWorktreeRef = { worktreeId: string @@ -22,7 +23,7 @@ export function loadKnownUsageWorktreesByRepo( store: Pick, repos: Repo[] ): Map { - const localRepos = repos.filter((repo) => !repo.connectionId) + const localRepos = repos.filter((repo) => !getRepoRoutingConnectionId(repo)) // Why: all three usage scanners revisit persisted worktree metadata; index // repos once instead of linearly searching the full list for every row. const localReposById = new Map() diff --git a/src/main/worktree-create-preparation.ts b/src/main/worktree-create-preparation.ts index 79313504795..869bac8c260 100644 --- a/src/main/worktree-create-preparation.ts +++ b/src/main/worktree-create-preparation.ts @@ -47,6 +47,7 @@ export { WORKTREE_CREATE_PREPARATION_LIMIT, WORKTREE_CREATE_PREPARATION_TTL_MS } from './worktree-create-preparation-pool' +import { getRepoRoutingConnectionId } from '../shared/execution-host' /** A prepared checkout is a create that is either in flight or imminent. */ export function hasPendingWorktreeCreatePreparations(): boolean { @@ -118,7 +119,7 @@ async function prepareWorktreeCreateInBackground( baseBranch: string, beforeMaterialization?: Promise ): Promise { - if (repo.connectionId || isFolderRepo(repo)) { + if (getRepoRoutingConnectionId(repo) || isFolderRepo(repo)) { return } const options = getLocalProjectWorktreeGitOptions(store, repo) diff --git a/src/main/worktree-trash.ts b/src/main/worktree-trash.ts index 088c51bfbc9..6b2dc3d903e 100644 --- a/src/main/worktree-trash.ts +++ b/src/main/worktree-trash.ts @@ -10,6 +10,7 @@ import { computeWorkspaceRoot, getWorktreePathSettings } from './ipc/worktree-lo import type { GlobalSettings } from '../shared/global-settings-types' import type { Repo } from '../shared/repo-types' import { parseWslPath } from './wsl' +import { getRepoRoutingConnectionId } from '../shared/execution-host' export const WORKTREE_TRASH_DIR_NAME = '.orca-worktree-trash' @@ -86,7 +87,7 @@ export function collectWorktreeTrashSweepRoots( ): string[] { const roots = new Set() for (const repo of repos) { - if (repo.connectionId || isFolderRepo(repo) || parseWslPath(repo.path)) { + if (getRepoRoutingConnectionId(repo) || isFolderRepo(repo) || parseWslPath(repo.path)) { continue } try { diff --git a/src/shared/execution-host.ts b/src/shared/execution-host.ts index 47e9d867720..0a900bea92f 100644 --- a/src/shared/execution-host.ts +++ b/src/shared/execution-host.ts @@ -279,6 +279,17 @@ export function getRepoSshConnectionId( return host?.kind === 'runtime' ? normalizeHostPart(repo.connectionId) : null } +/** + * Replaces a raw `repo.connectionId` in a routing or "is this SSH?" check. Never answers local for + * a row whose raw field is set, and also finds a row that names SSH only as `ssh:`. On a + * contradictory `ssh:B` row with `connectionId: 'A'`, B wins, as in `getRepoExecutionHostId`. + */ +export function getRepoRoutingConnectionId( + repo: Pick +): string | null { + return getRepoSshConnectionId(repo) ?? normalizeHostPart(repo.connectionId) +} + export function getWorktreeExecutionHostId( worktree: Pick, repo: Pick | undefined, diff --git a/src/shared/repo-routing-connection-id.test.ts b/src/shared/repo-routing-connection-id.test.ts new file mode 100644 index 00000000000..aa1f88d7583 --- /dev/null +++ b/src/shared/repo-routing-connection-id.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest' +import { getRepoRoutingConnectionId } from './execution-host' + +describe('getRepoRoutingConnectionId', () => { + it.each([ + ['legacy connectionId', { connectionId: 'gpu' }, 'gpu'], + ['ssh executionHostId only', { executionHostId: 'ssh:gpu' as const }, 'gpu'], + [ + 'runtime host with nested SSH', + { executionHostId: 'runtime:env' as const, connectionId: 'gpu' }, + 'gpu' + ], + [ + 'local with a stale connectionId', + { executionHostId: 'local' as const, connectionId: 'gpu' }, + 'gpu' + ], + ['local', { executionHostId: 'local' as const }, null], + ['runtime host', { executionHostId: 'runtime:env' as const }, null], + ['unstamped', {}, null] + ])('%s', (_label, repo, expected) => { + expect(getRepoRoutingConnectionId(repo)).toBe(expected) + }) + + it('never answers local for a row whose raw connectionId is set', () => { + for (const executionHostId of [ + undefined, + 'local', + 'runtime:env', + 'ssh:gpu', + 'ssh:other' + ] as const) { + expect(getRepoRoutingConnectionId({ executionHostId, connectionId: 'gpu' })).not.toBeNull() + } + }) + + it('lets executionHostId win on a contradictory SSH row, as getRepoExecutionHostId does', () => { + expect(getRepoRoutingConnectionId({ executionHostId: 'ssh:b', connectionId: 'a' })).toBe('b') + }) +})