diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 2f91c946bc7..0a4169c6b7d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -17,7 +17,7 @@ jobs: uses: actions/checkout@v6 - name: Install native build tools - run: sudo apt-get update && sudo apt-get install -y build-essential python3 zsh + run: sudo apt-get update && sudo apt-get install -y build-essential python3 zlib1g-dev zsh - name: Setup Node.js uses: actions/setup-node@v6 @@ -103,6 +103,33 @@ jobs: - name: Typecheck run: pnpm typecheck + # Why: real old Git diagnostics differ from mocked errors. Keep the + # fallback predicates executable across the baseline, transition, and + # current command shapes so a newly added flag cannot silently regress. + - name: Verify Git binary compatibility matrix + run: | + archive="$RUNNER_TEMP/git-2.25.5.tar.gz" + source="$RUNNER_TEMP/git-2.25.5" + curl -fsSL https://www.kernel.org/pub/software/scm/git/git-2.25.5.tar.gz -o "$archive" + echo "41662c52fc16fec4963bfc41075e71f8ead6b5e386797eb6f9a1111ff95a8ddf $archive" \ + | sha256sum --check + mkdir -p "$source" + tar -xzf "$archive" -C "$source" --strip-components=1 + make -C "$source" -j2 NO_GETTEXT=YesPlease NO_TCLTK=YesPlease NO_PYTHON=YesPlease git + ORCA_GIT_COMPAT_BINARY="$source/git" ORCA_GIT_COMPAT_VERSION="2.25.5" \ + pnpm exec vitest run --config config/vitest.config.ts \ + src/shared/git-binary-compatibility.test.ts + + for spec in \ + "alpine/git:edge-2.38.1|2.38.1" \ + "alpine/git:v2.49.1|2.49.1"; do + image="${spec%%|*}" + version="${spec#*|}" + ORCA_GIT_COMPAT_IMAGE="$image" ORCA_GIT_COMPAT_VERSION="$version" \ + pnpm exec vitest run --config config/vitest.config.ts \ + src/shared/git-binary-compatibility.test.ts + done + # Why: postinstall rebuilds better-sqlite3 for Electron's ABI via # @electron/rebuild, but vitest runs under system Node.js. Rebuild # it for Node so orchestration tests can load the native module. diff --git a/AGENTS.md b/AGENTS.md index 91df55e6c65..989fa611226 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -45,6 +45,7 @@ When adding or changing a Git command: - Check when every subcommand and option was introduced. For newer behavior, keep a baseline-compatible fallback or degrade safely. - Use `GitCapabilityCache` with a narrow unsupported-error predicate so recurring operations do not retry a known-invalid command. Do not rely only on `git --version`; wrappers such as `simple-git` do not remove host-version differences. - Scope capability state to the host that executes Git: native, WSL distro, SSH provider, or relay connection. Cover the first fallback, later cached calls, concurrent probes, and relevant host isolation in tests. +- Keep the real-binary compatibility contract in PR CI current. When adopting a newer Git feature, add its version boundary so the preferred command and fallback both run against representative Git releases. - Preserve commands that begin with global Git options such as `-c` before the subcommand, including auto-maintenance suppression used by worktree-create fetches. ## Git Provider Compatibility diff --git a/config/scripts/git-binary-compatibility-workflow.test.mjs b/config/scripts/git-binary-compatibility-workflow.test.mjs new file mode 100644 index 00000000000..67509f7f6cc --- /dev/null +++ b/config/scripts/git-binary-compatibility-workflow.test.mjs @@ -0,0 +1,20 @@ +import { readFileSync } from 'node:fs' +import { parse } from 'yaml' +import { describe, expect, it } from 'vitest' + +describe('Git binary compatibility PR gate', () => { + it('runs the real-binary contract at each compatibility boundary', () => { + const workflow = parse(readFileSync('.github/workflows/pr.yml', 'utf8')) + const step = workflow.jobs.verify.steps.find( + (candidate) => candidate.name === 'Verify Git binary compatibility matrix' + ) + + expect(step?.run).toContain('git-2.25.5.tar.gz') + expect(step?.run).toContain('41662c52fc16fec4963bfc41075e71f8ead6b5e386797eb6f9a1111ff95a8ddf') + expect(step?.run).toContain('ORCA_GIT_COMPAT_BINARY="$source/git"') + expect(step?.run).toContain('alpine/git:edge-2.38.1|2.38.1') + expect(step?.run).toContain('alpine/git:v2.49.1|2.49.1') + expect(step?.run).toContain('ORCA_GIT_COMPAT_IMAGE="$image"') + expect(step?.run).toContain('src/shared/git-binary-compatibility.test.ts') + }) +}) diff --git a/docs/reference/git-compatibility.md b/docs/reference/git-compatibility.md index 6ca8e8796b7..c943bc9ce5f 100644 --- a/docs/reference/git-compatibility.md +++ b/docs/reference/git-compatibility.md @@ -51,9 +51,12 @@ its own WSL/SSH routing, cancellation, tracing, redaction, process cleanup, and bounded output handling. Replacing the runner would move—not remove—the capability problem. -## Reliability Follow-up +## CI Contract -The strongest remaining gate is a small old-Git contract matrix that runs the -capability tests against Git 2.25, a pre-2.36 release, and current Git. Keep the -unit tests regardless: they cover localized error text, WSL/SSH isolation, and -concurrent probes that a single container run does not exercise. +PR checks run the capability contract against real Git 2.25.5, 2.38.1, and +2.49.1 binaries. This spans the core-workflow baseline, the transitional +`merge-tree --write-tree` behavior before `--merge-base`, and current Git. + +Keep the unit tests alongside that matrix. They cover concurrent probes, +native/WSL/SSH/relay isolation, and error-stream shapes that a single real +binary invocation cannot exercise deterministically. diff --git a/src/main/git/git-capability-state.test.ts b/src/main/git/git-capability-state.test.ts new file mode 100644 index 00000000000..0d499aea163 --- /dev/null +++ b/src/main/git/git-capability-state.test.ts @@ -0,0 +1,37 @@ +import { beforeEach, describe, expect, it } from 'vitest' +import { + clearGitCapabilityStateForTests, + getLocalGitCapabilityCache, + getSshGitCapabilityCache +} from './git-capability-state' + +describe('Git capability execution-host state', () => { + beforeEach(() => { + clearGitCapabilityStateForTests() + }) + + it('shares native state while isolating each WSL distro', () => { + expect(getLocalGitCapabilityCache({ cwd: '/repo-a' })).toBe( + getLocalGitCapabilityCache({ cwd: '/repo-b' }) + ) + expect(getLocalGitCapabilityCache({ wslDistro: 'Ubuntu' })).toBe( + getLocalGitCapabilityCache({ cwd: '\\\\wsl.localhost\\Ubuntu\\home\\repo' }) + ) + expect(getLocalGitCapabilityCache({ wslDistro: 'Ubuntu' })).not.toBe( + getLocalGitCapabilityCache({ wslDistro: 'Debian' }) + ) + expect(getLocalGitCapabilityCache()).not.toBe( + getLocalGitCapabilityCache({ wslDistro: 'Ubuntu' }) + ) + }) + + it('shares one SSH provider lifetime without leaking into a replacement provider', () => { + const provider = {} + const replacementProvider = {} + + expect(getSshGitCapabilityCache(provider)).toBe(getSshGitCapabilityCache(provider)) + expect(getSshGitCapabilityCache(provider)).not.toBe( + getSshGitCapabilityCache(replacementProvider) + ) + }) +}) diff --git a/src/main/git/repo.ts b/src/main/git/repo.ts index b69dc226c7a..f649017cdeb 100644 --- a/src/main/git/repo.ts +++ b/src/main/git/repo.ts @@ -5,6 +5,7 @@ import { gitExecFileSync, gitExecFileAsync } from './runner' import type { BaseRefSearchResult } from '../../shared/types' import { parseGitRevListAheadBehindCounts } from '../../shared/git-rev-list-output' import { normalizeRuntimePathSeparators } from '../../shared/cross-platform-path' +import { isForEachRefExcludeUnsupportedError } from '../../shared/git-ref-command-capabilities' import { parseWslUncPath } from '../../shared/wsl-paths' import { toWindowsWslPath } from '../wsl' import { buildHostedRemoteCommitUrl, buildHostedRemoteFileUrl } from './hosted-remote-url' @@ -835,17 +836,7 @@ export function mergeBaseRefSearchResultGroups( return merged } -export function isForEachRefExcludeUnsupportedError(error: unknown): boolean { - if (!error || typeof error !== 'object') { - return false - } - const maybe = error as { message?: unknown; stderr?: unknown; stdout?: unknown } - const text = [maybe.message, maybe.stderr, maybe.stdout] - .filter((value): value is string => typeof value === 'string') - .join('\n') - .toLowerCase() - return text.includes('unknown option') && text.includes('exclude') -} +export { isForEachRefExcludeUnsupportedError } from '../../shared/git-ref-command-capabilities' /** * Resolve the default push remote for a repo. diff --git a/src/main/git/worktree.ts b/src/main/git/worktree.ts index a3df6e37aca..a2dc7405831 100644 --- a/src/main/git/worktree.ts +++ b/src/main/git/worktree.ts @@ -15,6 +15,11 @@ import type { } from '../../shared/types' import { parseGitRevListAheadBehindCounts } from '../../shared/git-rev-list-output' import { parseWslUncPath } from '../../shared/wsl-paths' +import { + hasUnsupportedRevParsePathFormatEcho, + isUnsupportedRevParsePathFormatError, + isUnsupportedWorktreeListZError +} from '../../shared/git-worktree-command-capabilities' import { getLocalGitCapabilityCache } from './git-capability-state' import { gitExecFileAsync, translateWslOutputPaths } from './runner' import { resolveGitDir } from './status' @@ -110,22 +115,6 @@ function isNotGitRepositoryError(error: unknown): boolean { return /not a git repository/i.test(getErrorText(error)) } -function isUnsupportedWorktreeListZError(error: unknown): boolean { - // `-z` is this command's only flag older Git (<2.36) lacks, so its usage - // exit 129 signals the rejection in any locale; stderr match is a fallback. - if (getErrorCode(error) === '129') { - return true - } - - return /(?:unknown|invalid|unrecognized) (?:switch|option).*`?-?z'?/i.test(getErrorText(error)) -} - -function isUnsupportedRevParsePathFormatError(error: unknown): boolean { - return /(?:unknown|invalid|unrecognized).*(?:--path-format|path-format)/i.test( - getErrorText(error) - ) -} - function isBranchCheckedOutInWorktreeError(error: unknown): boolean { return /cannot delete branch .*(?:used by worktree|checked out)|branch .*is checked out/i.test( getErrorText(error) @@ -404,10 +393,6 @@ function parseRepoLocation(repoPath: string, output: string): RepoLocation | und } } -function echoedUnsupportedPathFormatOption(output: string): boolean { - return output.split(/\r?\n/).some((line) => line.startsWith('--path-format')) -} - async function readRepoLocation( repoPath: string, resolveBasePath: string, @@ -425,7 +410,7 @@ async function readRepoLocation( ['rev-parse', '--path-format=absolute', '--show-toplevel', '--git-common-dir'], gitExecOptions(repoPath, options) ) - if (echoedUnsupportedPathFormatOption(stdout)) { + if (hasUnsupportedRevParsePathFormatEcho(stdout)) { // Why: some old Git versions echo the unknown option and exit zero; // remember that compatibility signal even though parsing can recover. capabilities.rememberUnsupported('rev-parse-path-format') diff --git a/src/main/github/conflict-summary.ts b/src/main/github/conflict-summary.ts index b092b68c596..73ec657977c 100644 --- a/src/main/github/conflict-summary.ts +++ b/src/main/github/conflict-summary.ts @@ -1,5 +1,8 @@ import type { PRConflictSummary } from '../../shared/types' -import { isUnsupportedMergeTreeWriteTreeError } from '../../shared/git-merge-tree-capability' +import { + isUnsupportedMergeTreeMergeBaseError, + isUnsupportedMergeTreeWriteTreeError +} from '../../shared/git-merge-tree-capability' import { gitExecFileAsync } from '../git/runner' import { clearGitCapabilityStateForTests, @@ -267,7 +270,7 @@ async function loadConflictingFiles( } }, () => loadConflictingFilesWithLegacyMergeTree(repoPath, legacyArgs, localGitOptions), - isUnsupportedMergeBaseOption + isUnsupportedMergeTreeMergeBaseError ), async () => { // Why: Git before 2.38 cannot derive a reliable real-merge conflict list; @@ -315,12 +318,3 @@ function getGitErrorOutput(error: unknown, key: 'stdout' | 'stderr'): string { const output = (error as Partial>)[key] return typeof output === 'string' ? output : '' } - -function isUnsupportedMergeBaseOption(error: unknown): boolean { - const output = `${getGitErrorOutput(error, 'stderr')}\n${ - error instanceof Error ? error.message : '' - }` - return /(?:unknown|unrecognized) option(?::|\s+)[`']?(?:--?)?merge-base[`']?(?:\s|$)/i.test( - output - ) -} diff --git a/src/relay/git-handler-branch-cleanup.test.ts b/src/relay/git-handler-branch-cleanup.test.ts index cf84b54d591..8058d55ae29 100644 --- a/src/relay/git-handler-branch-cleanup.test.ts +++ b/src/relay/git-handler-branch-cleanup.test.ts @@ -1,9 +1,17 @@ import { describe, expect, it, vi } from 'vitest' import * as path from 'node:path' +import { GitCapabilityCache } from '../shared/git-capability-cache' import type { GitExec } from './git-handler-ops' import { removeWorktreeOp } from './git-handler-worktree-ops' import { forceDeletePreservedRelayBranch } from './git-handler-branch-cleanup' +function removeWorktreeWithCapabilityCache( + git: GitExec, + params: Parameters[1] +) { + return removeWorktreeOp(git, params, new GitCapabilityCache()) +} + function worktreeList(...entries: { path: string; branch?: string }[]): string { return entries .map((entry, index) => @@ -192,7 +200,9 @@ describe('removeWorktreeOp branch cleanup', () => { return { stdout: '', stderr: '' } }) - await expect(removeWorktreeOp(git, { worktreePath: '/repo-feature' })).resolves.toEqual({}) + await expect( + removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) + ).resolves.toEqual({}) expect(git).toHaveBeenCalledWith(['branch', '-d', '--', 'feature/test'], expect.any(String)) expect(git).toHaveBeenCalledWith( @@ -279,7 +289,9 @@ describe('removeWorktreeOp branch cleanup', () => { return { stdout: '', stderr: '' } }) - await expect(removeWorktreeOp(git, { worktreePath: '/repo-feature' })).resolves.toEqual({}) + await expect( + removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) + ).resolves.toEqual({}) expect(git).toHaveBeenCalledWith( ['update-ref', '-d', 'refs/heads/feature/test', '1'], @@ -341,7 +353,9 @@ describe('removeWorktreeOp branch cleanup', () => { return { stdout: '', stderr: '' } }) - await expect(removeWorktreeOp(git, { worktreePath: '/repo-feature' })).resolves.toEqual({}) + await expect( + removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) + ).resolves.toEqual({}) const commandIndex = (expectedArgs: string[]) => calls.findIndex(({ args }) => JSON.stringify(args) === JSON.stringify(expectedArgs)) @@ -404,7 +418,9 @@ describe('removeWorktreeOp branch cleanup', () => { return { stdout: '', stderr: '' } }) - await expect(removeWorktreeOp(git, { worktreePath: '/repo-feature' })).resolves.toEqual({ + await expect( + removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) + ).resolves.toEqual({ preservedBranch: { branchName: 'feature/test', head: '1' } }) diff --git a/src/relay/git-handler-local-base-ref-refresh.ts b/src/relay/git-handler-local-base-ref-refresh.ts index ae4cfaa6ef3..299780799f0 100644 --- a/src/relay/git-handler-local-base-ref-refresh.ts +++ b/src/relay/git-handler-local-base-ref-refresh.ts @@ -1,11 +1,11 @@ import type { GitExec } from './git-handler-ops' import { areRelayWorktreePathsEqual, readRelayWorktreeList } from './git-handler-worktree-ops' -import { GitCapabilityCache } from '../shared/git-capability-cache' +import type { GitCapabilityCache } from '../shared/git-capability-cache' export async function refreshLocalBaseRefForWorktreeCreateOp( git: GitExec, params: Record, - capabilities = new GitCapabilityCache() + capabilities: GitCapabilityCache ): Promise { const repoPath = params.repoPath as string const fullRef = params.fullRef as string diff --git a/src/relay/git-handler-utils.ts b/src/relay/git-handler-utils.ts index f0a395da27b..6df064ced61 100644 --- a/src/relay/git-handler-utils.ts +++ b/src/relay/git-handler-utils.ts @@ -9,6 +9,7 @@ import { existsSync } from 'node:fs' import * as path from 'node:path' import { isBinaryBuffer } from '../shared/binary-buffer' import type { GitLineStats } from '../shared/git-uncommitted-line-stats' +export { isUnsupportedWorktreeListZError } from '../shared/git-worktree-command-capabilities' export function parseBranchStatusChar(char: string): string { switch (char) { @@ -133,36 +134,6 @@ export function parseBranchDiff( // ─── Worktree parsing ──────────────────────────────────────────────── -function getErrorText(error: unknown): string { - if (typeof error === 'object' && error !== null) { - const parts: string[] = [] - if ('message' in error && typeof error.message === 'string') { - parts.push(error.message) - } - if ('stderr' in error && typeof error.stderr === 'string') { - parts.push(error.stderr) - } - return parts.join('\n') - } - return String(error) -} - -function getErrorCode(error: unknown): string | undefined { - return typeof error === 'object' && error !== null && 'code' in error - ? String((error as { code?: unknown }).code) - : undefined -} - -export function isUnsupportedWorktreeListZError(error: unknown): boolean { - // `-z` is this command's only flag older Git (<2.36) lacks, so its usage exit - // 129 signals the rejection in any locale; key for SSH remotes on old Git. - if (getErrorCode(error) === '129') { - return true - } - - return /(?:unknown|invalid|unrecognized) (?:switch|option).*`?-?z'?/i.test(getErrorText(error)) -} - export function parseWorktreeList( output: string, options: { nulDelimited?: boolean } = {} diff --git a/src/relay/git-handler-worktree-git-capabilities.test.ts b/src/relay/git-handler-worktree-git-capabilities.test.ts index 3c5e569cf8e..6d58a999393 100644 --- a/src/relay/git-handler-worktree-git-capabilities.test.ts +++ b/src/relay/git-handler-worktree-git-capabilities.test.ts @@ -50,6 +50,35 @@ describe('relay worktree Git capabilities', () => { ]) }) + it('re-probes after a relay handler is replaced', async () => { + const mockOldGit = (target: GitHandler) => + vi.spyOn(target as unknown as GitSpyTarget, 'git').mockImplementation((args: string[]) => { + if (args.includes('-z')) { + return Promise.reject( + Object.assign(new Error('git usage error'), { + code: 129, + stderr: 'usage: git worktree list []\n' + }) + ) + } + return Promise.resolve({ stdout: WORKTREE_LIST_OUTPUT, stderr: '' }) + }) + const firstGit = mockOldGit(handler) + const replacementDispatcher = createMockDispatcher() + const replacementHandler = new GitHandler( + replacementDispatcher as unknown as RelayDispatcher, + new RelayContext() + ) + const replacementGit = mockOldGit(replacementHandler) + + await dispatcher.callRequest('git.listWorktrees', { repoPath: '/repo' }) + await dispatcher.callRequest('git.listWorktrees', { repoPath: '/repo' }) + await replacementDispatcher.callRequest('git.listWorktrees', { repoPath: '/repo' }) + + expect(firstGit.mock.calls.filter(([args]) => args.includes('-z'))).toHaveLength(1) + expect(replacementGit.mock.calls.filter(([args]) => args.includes('-z'))).toHaveLength(1) + }) + it('does not repeat a known-unsupported rev-parse --path-format probe', async () => { const gitSpy = vi .spyOn(handler as unknown as GitSpyTarget, 'git') diff --git a/src/relay/git-handler-worktree-list.ts b/src/relay/git-handler-worktree-list.ts new file mode 100644 index 00000000000..bc58386ad82 --- /dev/null +++ b/src/relay/git-handler-worktree-list.ts @@ -0,0 +1,39 @@ +import type { GitCapabilityCache } from '../shared/git-capability-cache' +import type { GitExec } from './git-handler-ops' +import { isUnsupportedWorktreeListZError, parseWorktreeList } from './git-handler-utils' + +export type RelayWorktreeInfo = { + path: string + branch?: string + head?: string +} + +export async function readRelayWorktreeList( + git: GitExec, + repoPath: string, + capabilities: GitCapabilityCache +): Promise { + return capabilities.runWithFallback( + 'worktree-list-z', + async () => { + const { stdout } = await git(['worktree', 'list', '--porcelain', '-z'], repoPath) + return normalizeRelayWorktrees(parseWorktreeList(stdout, { nulDelimited: true })) + }, + async () => { + // Why: `-z` preserves newlines; fallback keeps Git <2.36 compatible. + const { stdout } = await git(['worktree', 'list', '--porcelain'], repoPath) + return normalizeRelayWorktrees(parseWorktreeList(stdout)) + }, + isUnsupportedWorktreeListZError + ) +} + +function normalizeRelayWorktrees(worktrees: Record[]): RelayWorktreeInfo[] { + return worktrees + .map((worktree) => ({ + path: typeof worktree.path === 'string' ? worktree.path : '', + head: typeof worktree.head === 'string' ? worktree.head : undefined, + branch: typeof worktree.branch === 'string' ? worktree.branch : undefined + })) + .filter((worktree) => worktree.path.length > 0) +} diff --git a/src/relay/git-handler-worktree-ops.test.ts b/src/relay/git-handler-worktree-ops.test.ts index 52cc1236054..9180f6cbed0 100644 --- a/src/relay/git-handler-worktree-ops.test.ts +++ b/src/relay/git-handler-worktree-ops.test.ts @@ -1,8 +1,16 @@ import { describe, expect, it, vi } from 'vitest' import * as path from 'node:path' +import { GitCapabilityCache } from '../shared/git-capability-cache' import type { GitExec } from './git-handler-ops' import { addWorktreeOp, removeWorktreeOp } from './git-handler-worktree-ops' +function removeWorktreeWithCapabilityCache( + git: GitExec, + params: Parameters[1] +) { + return removeWorktreeOp(git, params, new GitCapabilityCache()) +} + function worktreeList(...entries: { path: string; branch?: string }[]): string { return entries .map((entry, index) => @@ -152,7 +160,7 @@ describe('removeWorktreeOp', () => { return { stdout: '', stderr: '' } }) - await removeWorktreeOp(git, { worktreePath: '/repo-feature' }) + await removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) expect(calls).toEqual([ '/repo-feature$ rev-parse --git-common-dir', @@ -188,7 +196,9 @@ describe('removeWorktreeOp', () => { }) // The unmerged-branch refusal must be surfaced without failing workspace removal. - await expect(removeWorktreeOp(git, { worktreePath: '/repo-feature' })).resolves.toEqual({ + await expect( + removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) + ).resolves.toEqual({ preservedBranch: { branchName: 'feature/test', head: '1' } }) @@ -218,7 +228,7 @@ describe('removeWorktreeOp', () => { return { stdout: '', stderr: '' } }) - await removeWorktreeOp(git, { + await removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature', force: true, forceBranchDelete: true @@ -247,7 +257,10 @@ describe('removeWorktreeOp', () => { return { stdout: '', stderr: '' } }) - await removeWorktreeOp(git, { worktreePath: '/repo-feature', deleteBranch: false }) + await removeWorktreeWithCapabilityCache(git, { + worktreePath: '/repo-feature', + deleteBranch: false + }) expect(calls).toEqual([ '/repo-feature$ rev-parse --git-common-dir', @@ -286,7 +299,7 @@ describe('removeWorktreeOp', () => { return { stdout: '', stderr: '' } }) - await removeWorktreeOp(git, { worktreePath: '/repo-feature' }) + await removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) expect(git).toHaveBeenCalledWith(['branch', '-d', '--', 'feature/test'], expect.any(String)) expect(git).toHaveBeenCalledWith(['worktree', 'prune'], expect.any(String)) diff --git a/src/relay/git-handler-worktree-ops.ts b/src/relay/git-handler-worktree-ops.ts index 34ee8f34afd..487fabd7688 100644 --- a/src/relay/git-handler-worktree-ops.ts +++ b/src/relay/git-handler-worktree-ops.ts @@ -1,9 +1,8 @@ import * as path from 'node:path' import { resolveWorktreeAddBaseRef } from '../shared/worktree-base-ref' import type { GitExec } from './git-handler-ops' -import { isUnsupportedWorktreeListZError, parseWorktreeList } from './git-handler-utils' -import { GitCapabilityCache } from '../shared/git-capability-cache' export { removeWorktreeOp } from './git-handler-worktree-remove' +export { readRelayWorktreeList } from './git-handler-worktree-list' async function persistRelayWorktreeCreationBase( git: GitExec, @@ -112,42 +111,6 @@ export async function addWorktreeOp(git: GitExec, params: Record { - return capabilities.runWithFallback( - 'worktree-list-z', - async () => { - const { stdout } = await git(['worktree', 'list', '--porcelain', '-z'], repoPath) - return normalizeRelayWorktrees(parseWorktreeList(stdout, { nulDelimited: true })) - }, - async () => { - // Why: `-z` preserves newlines; fallback keeps Git <2.36 compatible. - const { stdout } = await git(['worktree', 'list', '--porcelain'], repoPath) - return normalizeRelayWorktrees(parseWorktreeList(stdout)) - }, - isUnsupportedWorktreeListZError - ) -} - -function normalizeRelayWorktrees(worktrees: Record[]): RelayWorktreeInfo[] { - return worktrees - .map((worktree) => ({ - path: typeof worktree.path === 'string' ? worktree.path : '', - head: typeof worktree.head === 'string' ? worktree.head : undefined, - branch: typeof worktree.branch === 'string' ? worktree.branch : undefined - })) - .filter((worktree) => worktree.path.length > 0) -} - function isPosixAbsolutePath(value: string): boolean { return value.startsWith('/') } diff --git a/src/relay/git-handler-worktree-paths.test.ts b/src/relay/git-handler-worktree-paths.test.ts index 7bdb0826561..d5f7a7755cb 100644 --- a/src/relay/git-handler-worktree-paths.test.ts +++ b/src/relay/git-handler-worktree-paths.test.ts @@ -1,8 +1,16 @@ import { describe, expect, it, vi } from 'vitest' import * as path from 'node:path' +import { GitCapabilityCache } from '../shared/git-capability-cache' import type { GitExec } from './git-handler-ops' import { removeWorktreeOp } from './git-handler-worktree-ops' +function removeWorktreeWithCapabilityCache( + git: GitExec, + params: Parameters[1] +) { + return removeWorktreeOp(git, params, new GitCapabilityCache()) +} + function lineWorktreeList(...entries: { path: string; branch?: string }[]): string { return entries .map((entry, index) => @@ -56,7 +64,7 @@ describe('relay worktree path parsing', () => { return { stdout: '', stderr: '' } }) - await removeWorktreeOp(git, { worktreePath }) + await removeWorktreeWithCapabilityCache(git, { worktreePath }) expect(git).toHaveBeenCalledWith(['branch', '-d', '--', 'feature/newline'], resolvedRepoPath()) }) @@ -90,7 +98,7 @@ describe('relay worktree path parsing', () => { return { stdout: '', stderr: '' } }) - await removeWorktreeOp(git, { worktreePath: '/repo-feature' }) + await removeWorktreeWithCapabilityCache(git, { worktreePath: '/repo-feature' }) expect(calls).toEqual([ '/repo-feature$ rev-parse --git-common-dir', diff --git a/src/relay/git-handler-worktree-remove.ts b/src/relay/git-handler-worktree-remove.ts index fbac5b13918..540566e1679 100644 --- a/src/relay/git-handler-worktree-remove.ts +++ b/src/relay/git-handler-worktree-remove.ts @@ -2,14 +2,8 @@ import * as path from 'node:path' import type { RemoveWorktreeResult } from '../shared/types' import { deleteAlreadyMergedRelayBranchAfterSafeDeleteFailure } from './git-handler-branch-cleanup' import type { GitExec } from './git-handler-ops' -import { isUnsupportedWorktreeListZError, parseWorktreeList } from './git-handler-utils' -import { GitCapabilityCache } from '../shared/git-capability-cache' - -type RelayWorktreeInfo = { - path: string - branch?: string - head?: string -} +import type { GitCapabilityCache } from '../shared/git-capability-cache' +import { readRelayWorktreeList } from './git-handler-worktree-list' function getErrorText(error: unknown): string { if (typeof error === 'object' && error !== null) { @@ -75,36 +69,6 @@ function areRelayWorktreePathsEqual(leftPath: string, rightPath: string): boolea return compareCaseInsensitive ? left.toLowerCase() === right.toLowerCase() : left === right } -function normalizeRelayWorktrees(worktrees: Record[]): RelayWorktreeInfo[] { - return worktrees - .map((worktree) => ({ - path: typeof worktree.path === 'string' ? worktree.path : '', - head: typeof worktree.head === 'string' ? worktree.head : undefined, - branch: typeof worktree.branch === 'string' ? worktree.branch : undefined - })) - .filter((worktree) => worktree.path.length > 0) -} - -async function readRelayWorktreeList( - git: GitExec, - repoPath: string, - capabilities: GitCapabilityCache -): Promise { - return capabilities.runWithFallback( - 'worktree-list-z', - async () => { - const { stdout } = await git(['worktree', 'list', '--porcelain', '-z'], repoPath) - return normalizeRelayWorktrees(parseWorktreeList(stdout, { nulDelimited: true })) - }, - async () => { - // Why: `-z` preserves newlines; fallback keeps Git <2.36 compatible. - const { stdout } = await git(['worktree', 'list', '--porcelain'], repoPath) - return normalizeRelayWorktrees(parseWorktreeList(stdout)) - }, - isUnsupportedWorktreeListZError - ) -} - async function listRelayWorktreesForRemoval( git: GitExec, repoPath: string, @@ -159,7 +123,7 @@ async function deleteRelayBranchAfterWorktreeRemoval( export async function removeWorktreeOp( git: GitExec, params: Record, - capabilities = new GitCapabilityCache() + capabilities: GitCapabilityCache ): Promise { const worktreePath = params.worktreePath as string const force = params.force as boolean | undefined diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index f4d468e69a3..533c06f27b7 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -62,6 +62,10 @@ import { syncForkDefaultBranch, validateGitForkSyncExpectedUpstream } from '../s import { InFlightPromiseDedupe, stableInFlightKey } from '../shared/in-flight-promise-dedupe' import { GIT_FETCH_SKIP_AUTO_MAINTENANCE_CONFIG_ARGS } from '../shared/git-fetch-auto-maintenance' import { GitCapabilityCache } from '../shared/git-capability-cache' +import { + hasUnsupportedRevParsePathFormatEcho, + isUnsupportedRevParsePathFormatError +} from '../shared/git-worktree-command-capabilities' import { GitResponseStreamRegistry } from './git-response-stream' import { GIT_RESPONSE_STREAM_THRESHOLD } from './protocol' @@ -78,26 +82,6 @@ function resolveSubmoduleStatusArea( return 'unstaged' } -function getErrorText(error: unknown): string { - if (typeof error === 'object' && error !== null) { - const parts: string[] = [] - if ('message' in error && typeof error.message === 'string') { - parts.push(error.message) - } - if ('stderr' in error && typeof error.stderr === 'string') { - parts.push(error.stderr) - } - return parts.join('\n') - } - return String(error) -} - -function isUnsupportedRevParsePathFormatError(error: unknown): boolean { - return /(?:unknown|invalid|unrecognized).*(?:--path-format|path-format)/i.test( - getErrorText(error) - ) -} - function isWindowsAbsolutePath(value: string): boolean { return /^[A-Za-z]:[\\/]/.test(value) || value.startsWith('\\\\') } @@ -1286,7 +1270,7 @@ export class GitHandler { ['rev-parse', '--path-format=absolute', '--show-toplevel', '--git-common-dir'], repoPath ) - if (stdout.split(/\r?\n/).some((line) => line.startsWith('--path-format'))) { + if (hasUnsupportedRevParsePathFormatEcho(stdout)) { // Why: some old Git versions echo the unknown option and exit zero; // remember that signal even though the trailing paths remain usable. this.gitCapabilities.rememberUnsupported('rev-parse-path-format') diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts new file mode 100644 index 00000000000..3f737cf9989 --- /dev/null +++ b/src/shared/git-binary-compatibility.test.ts @@ -0,0 +1,140 @@ +import { execFile } from 'node:child_process' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { promisify } from 'node:util' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { + isUnsupportedMergeTreeMergeBaseError, + isUnsupportedMergeTreeWriteTreeError +} from './git-merge-tree-capability' +import { isForEachRefExcludeUnsupportedError } from './git-ref-command-capabilities' +import { + hasUnsupportedRevParsePathFormatEcho, + isUnsupportedWorktreeListZError +} from './git-worktree-command-capabilities' + +const execFileAsync = promisify(execFile) +const image = process.env.ORCA_GIT_COMPAT_IMAGE +const binary = process.env.ORCA_GIT_COMPAT_BINARY +const expectedVersion = process.env.ORCA_GIT_COMPAT_VERSION +const describeBinaryCompatibility = image || binary ? describe : describe.skip + +type GitResult = { stdout: string; stderr: string } + +describeBinaryCompatibility('real Git binary compatibility', () => { + let repoPath = '' + let version = { major: 0, minor: 0 } + + async function runGit(args: string[]): Promise { + if (image) { + return execFileAsync( + 'docker', + [ + 'run', + '--rm', + '--network=none', + '-v', + `${repoPath}:/repo`, + '-w', + '/repo', + image, + '-c', + 'safe.directory=/repo', + ...args + ], + { maxBuffer: 2 * 1024 * 1024 } + ) + } + return execFileAsync(binary!, args, { cwd: repoPath, maxBuffer: 2 * 1024 * 1024 }) + } + + function supports(major: number, minor: number): boolean { + return version.major > major || (version.major === major && version.minor >= minor) + } + + async function expectPreferredOrRecognizedFallback( + args: string[], + expectedSupport: boolean, + recognizesUnsupported: (error: unknown) => boolean + ): Promise { + try { + await runGit(args) + expect(expectedSupport).toBe(true) + } catch (error) { + expect(expectedSupport).toBe(false) + expect(recognizesUnsupported(error)).toBe(true) + } + } + + beforeAll(async () => { + repoPath = await mkdtemp(join(tmpdir(), 'orca-git-binary-compat-')) + const versionOutput = await runGit(['--version']) + expect(versionOutput.stdout).toContain(`git version ${expectedVersion}`) + const match = versionOutput.stdout.match(/git version (\d+)\.(\d+)/) + expect(match).not.toBeNull() + version = { major: Number(match![1]), minor: Number(match![2]) } + + await runGit(['init', '-q']) + await runGit(['config', 'user.email', 'compatibility@example.invalid']) + await runGit(['config', 'user.name', 'Compatibility Test']) + await writeFile(join(repoPath, 'tracked.txt'), 'compatibility\n') + await runGit(['add', 'tracked.txt']) + await runGit(['commit', '-qm', 'initial']) + }) + + afterAll(async () => { + if (repoPath) { + await rm(repoPath, { recursive: true, force: true }) + } + }) + + it('recognizes worktree-list and rev-parse compatibility boundaries', async () => { + await expectPreferredOrRecognizedFallback( + ['worktree', 'list', '--porcelain', '-z'], + supports(2, 36), + isUnsupportedWorktreeListZError + ) + await expect(runGit(['worktree', 'list', '--porcelain'])).resolves.toMatchObject({ + stdout: expect.stringContaining('worktree ') + }) + + const preferred = await runGit([ + 'rev-parse', + '--path-format=absolute', + '--show-toplevel', + '--git-common-dir' + ]) + expect(hasUnsupportedRevParsePathFormatEcho(preferred.stdout)).toBe(!supports(2, 31)) + await expect( + runGit(['rev-parse', '--show-toplevel', '--git-common-dir']) + ).resolves.toBeDefined() + }) + + it('recognizes ref and merge-tree compatibility boundaries', async () => { + await expectPreferredOrRecognizedFallback( + ['for-each-ref', '--format=%(refname)', '--exclude=refs/remotes/**/HEAD', '--count=10'], + supports(2, 42), + isForEachRefExcludeUnsupportedError + ) + await expect( + runGit(['for-each-ref', '--format=%(refname)', '--count=10']) + ).resolves.toBeDefined() + + await expectPreferredOrRecognizedFallback( + ['merge-tree', '--write-tree', 'HEAD', 'HEAD'], + supports(2, 38), + isUnsupportedMergeTreeWriteTreeError + ) + if (supports(2, 38)) { + const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() + const legacyArgs = ['merge-tree', '--write-tree', '--name-only', '-z', '--no-messages'] + await expectPreferredOrRecognizedFallback( + [...legacyArgs, '--merge-base', head, head, head], + supports(2, 40), + isUnsupportedMergeTreeMergeBaseError + ) + await expect(runGit([...legacyArgs, head, head])).resolves.toBeDefined() + } + }) +}) diff --git a/src/shared/git-merge-tree-capability.test.ts b/src/shared/git-merge-tree-capability.test.ts index de44b78bdcb..b83cf40ae55 100644 --- a/src/shared/git-merge-tree-capability.test.ts +++ b/src/shared/git-merge-tree-capability.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { isUnsupportedMergeTreeWriteTreeError } from './git-merge-tree-capability' +import { + isUnsupportedMergeTreeMergeBaseError, + isUnsupportedMergeTreeWriteTreeError +} from './git-merge-tree-capability' describe('isUnsupportedMergeTreeWriteTreeError', () => { it.each([ @@ -17,4 +20,13 @@ describe('isUnsupportedMergeTreeWriteTreeError', () => { }) ).toBe(false) }) + + it('recognizes only an unsupported merge-base option', () => { + expect( + isUnsupportedMergeTreeMergeBaseError({ + stderr: "error: unknown option `merge-base'" + }) + ).toBe(true) + expect(isUnsupportedMergeTreeMergeBaseError({ stderr: 'fatal: merge-base failed' })).toBe(false) + }) }) diff --git a/src/shared/git-merge-tree-capability.ts b/src/shared/git-merge-tree-capability.ts index 5bea669f59a..0ec56cd02cb 100644 --- a/src/shared/git-merge-tree-capability.ts +++ b/src/shared/git-merge-tree-capability.ts @@ -18,3 +18,10 @@ export function isUnsupportedMergeTreeWriteTreeError(error: unknown): boolean { /usage:\s*git merge-tree\s+\s+\s+/i.test(output) ) } + +export function isUnsupportedMergeTreeMergeBaseError(error: unknown): boolean { + const output = getGitErrorText(error) + return /(?:unknown|invalid|unrecognized) option(?::|\s+)[`']?(?:--?)?merge-base[`']?(?:\s|$)/i.test( + output + ) +} diff --git a/src/shared/git-ref-command-capabilities.ts b/src/shared/git-ref-command-capabilities.ts new file mode 100644 index 00000000000..be3f5d9e06f --- /dev/null +++ b/src/shared/git-ref-command-capabilities.ts @@ -0,0 +1,14 @@ +function getGitErrorText(error: unknown): string { + if (typeof error !== 'object' || error === null) { + return error instanceof Error ? error.message : String(error) + } + const values = ['message', 'stderr', 'stdout'] + .map((key) => (error as Record)[key]) + .filter((value): value is string => typeof value === 'string') + return values.join('\n') +} + +export function isForEachRefExcludeUnsupportedError(error: unknown): boolean { + const output = getGitErrorText(error).toLowerCase() + return output.includes('unknown option') && output.includes('exclude') +} diff --git a/src/shared/git-worktree-command-capabilities.ts b/src/shared/git-worktree-command-capabilities.ts new file mode 100644 index 00000000000..76bd705432c --- /dev/null +++ b/src/shared/git-worktree-command-capabilities.ts @@ -0,0 +1,34 @@ +function getGitErrorText(error: unknown): string { + if (typeof error !== 'object' || error === null) { + return error instanceof Error ? error.message : String(error) + } + const values = ['message', 'stderr', 'stdout'] + .map((key) => (error as Record)[key]) + .filter((value): value is string => typeof value === 'string') + return values.join('\n') +} + +function getGitErrorCode(error: unknown): string | undefined { + return typeof error === 'object' && error !== null && 'code' in error + ? String((error as { code?: unknown }).code) + : undefined +} + +export function isUnsupportedWorktreeListZError(error: unknown): boolean { + // `-z` is this fixed command's only post-baseline flag, so exit 129 is the + // locale-independent rejection signal on old native, WSL, and SSH Git. + if (getGitErrorCode(error) === '129') { + return true + } + return /(?:unknown|invalid|unrecognized) (?:switch|option).*`?-?z'?/i.test(getGitErrorText(error)) +} + +export function isUnsupportedRevParsePathFormatError(error: unknown): boolean { + return /(?:unknown|invalid|unrecognized).*(?:--path-format|path-format)/i.test( + getGitErrorText(error) + ) +} + +export function hasUnsupportedRevParsePathFormatEcho(output: string): boolean { + return output.split(/\r?\n/).some((line) => line.startsWith('--path-format')) +}