diff --git a/src/main/git/source-control/bulk-pathspec-command-line-budget.test.ts b/src/main/git/source-control/bulk-pathspec-command-line-budget.test.ts new file mode 100644 index 00000000000..39c4135aade --- /dev/null +++ b/src/main/git/source-control/bulk-pathspec-command-line-budget.test.ts @@ -0,0 +1,193 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + commandLineLength, + MAX_COMMAND_LINE_CHARS +} from '../../../shared/windows-command-line-budget' +import { resolveGitCommandWithoutProbe } from '../command-runner/git-command-resolution' + +const gitExecFileAsync = vi.fn(async () => ({ stdout: '', stderr: '' })) + +vi.mock('../runner', () => ({ + gitExecFileAsync: (...args: unknown[]) => + (gitExecFileAsync as unknown as (...a: unknown[]) => Promise<{ stdout: string }>)(...args) +})) +vi.mock('./git-read-cache-invalidation', () => ({ invalidateGitReadCaches: vi.fn() })) +vi.mock('../../../shared/git-discard-path-safety', () => ({ + removeSafeUntrackedDiscardTarget: vi.fn(), + removeSafeUntrackedDiscardTargets: async ( + _worktreePath: string, + untrackedPaths: string[], + cleanUntracked: (paths: string[]) => Promise, + restoreTracked: () => Promise + ) => { + await restoreTracked() + if (untrackedPaths.length > 0) { + await cleanUntracked(untrackedPaths) + } + } +})) + +const WSL_DISTRO = 'Ubuntu-24.04' +const WSL_WORKTREE = `\\\\wsl$\\${WSL_DISTRO}\\home\\emilio\\projects\\orca` + +/** Windows-side length of the line `wsl.exe` is spawned with, wrapper included. */ +function finishedCommandLineLength(args: readonly string[], wslDistro?: string): number { + const resolved = resolveGitCommandWithoutProbe([...args], { + cwd: wslDistro ? WSL_WORKTREE : '/home/emilio/projects/orca', + ...(wslDistro ? { wslDistro } : {}) + }) + return commandLineLength([resolved.binary, ...resolved.args]) +} + +/** Deep nesting, a space, and non-ASCII: all three inflate the quoted line. */ +function realisticChangedPaths(count: number): string[] { + return Array.from( + { length: count }, + (_, index) => + `apps/web/src/components/dashboard/widgets/analytics/rapport trimestriel ${String(index).padStart(3, '0')}/données-générales/AnalyticsSummaryWidget${index}.tsx` + ) +} + +function capturedInvocations(): string[][] { + return gitExecFileAsync.mock.calls.map((call) => (call as unknown as [string[]])[0]) +} + +describe('bulk pathspec command-line budget', () => { + const realPlatform = process.platform + + beforeEach(() => { + gitExecFileAsync.mockReset() + gitExecFileAsync.mockImplementation(async () => ({ stdout: '', stderr: '' })) + Object.defineProperty(process, 'platform', { value: 'win32' }) + }) + + afterEach(() => { + Object.defineProperty(process, 'platform', { value: realPlatform }) + vi.resetModules() + }) + + it('keeps every WSL bulk-stage invocation inside the Windows command-line cap', async () => { + const { bulkStageFiles } = await import('./staging') + const filePaths = realisticChangedPaths(100) + + await bulkStageFiles(WSL_WORKTREE, filePaths, { wslDistro: WSL_DISTRO }) + + const invocations = capturedInvocations() + expect(invocations.length).toBeGreaterThan(0) + const lengths = invocations.map((args) => finishedCommandLineLength(args, WSL_DISTRO)) + expect(Math.max(...lengths)).toBeLessThanOrEqual(MAX_COMMAND_LINE_CHARS) + }) + + it('stages every path exactly once, in order, across the chunks', async () => { + const { bulkStageFiles } = await import('./staging') + const filePaths = realisticChangedPaths(250) + + await bulkStageFiles(WSL_WORKTREE, filePaths, { wslDistro: WSL_DISTRO }) + + const staged = capturedInvocations().flatMap((args) => args.slice(args.indexOf('--') + 1)) + expect(staged).toEqual(filePaths.map((filePath) => `:(literal)${filePath}`)) + }) + + it('keeps WSL bulk unstage inside the cap', async () => { + const { bulkUnstageFiles } = await import('./staging') + + await bulkUnstageFiles(WSL_WORKTREE, realisticChangedPaths(100), { wslDistro: WSL_DISTRO }) + + const lengths = capturedInvocations().map((args) => finishedCommandLineLength(args, WSL_DISTRO)) + expect(Math.max(...lengths)).toBeLessThanOrEqual(MAX_COMMAND_LINE_CHARS) + }) + + it('never emits a pathspec-free chunk, which would widen `clean -ffdx` to the worktree', async () => { + const { bulkStageFiles } = await import('./staging') + + await bulkStageFiles(WSL_WORKTREE, realisticChangedPaths(300), { wslDistro: WSL_DISTRO }) + + for (const args of capturedInvocations()) { + expect(args.slice(args.indexOf('--') + 1).length).toBeGreaterThan(0) + } + }) + + it('splits a WSL bulk discard of tracked paths into spawnable restores', async () => { + const filePaths = realisticChangedPaths(120) + gitExecFileAsync.mockImplementation(async () => ({ stdout: filePaths.join('\0'), stderr: '' })) + const { bulkDiscardChanges } = await import('./discard-changes') + + await bulkDiscardChanges(WSL_WORKTREE, filePaths, { wslDistro: WSL_DISTRO }) + + const restores = capturedInvocations().filter((args) => args[0] === 'restore') + expect(restores.length).toBeGreaterThan(1) + for (const args of restores) { + expect(finishedCommandLineLength(args, WSL_DISTRO)).toBeLessThanOrEqual( + MAX_COMMAND_LINE_CHARS + ) + } + }) + + it('never widens `clean -ffdx` past the paths it was given', async () => { + const filePaths = realisticChangedPaths(120) + const { bulkDiscardChanges } = await import('./discard-changes') + + // Empty ls-files output: every path is untracked, so all of them take the clean lane. + await bulkDiscardChanges(WSL_WORKTREE, filePaths, { wslDistro: WSL_DISTRO }) + + const cleans = capturedInvocations().filter((args) => args[0] === 'clean') + expect(cleans.length).toBeGreaterThan(1) + const cleaned = cleans.flatMap((args) => args.slice(args.indexOf('--') + 1)) + expect(cleaned).toEqual(filePaths.map((filePath) => `:(literal)${filePath}`)) + for (const args of cleans) { + expect(finishedCommandLineLength(args, WSL_DISTRO)).toBeLessThanOrEqual( + MAX_COMMAND_LINE_CHARS + ) + } + }) + + it('ships a single over-budget pathspec alone rather than dropping it', async () => { + const { bulkStageFiles } = await import('./staging') + const hugePath = `src/${'nested-directory/'.repeat(700)}Component.tsx` + + await bulkStageFiles(WSL_WORKTREE, [hugePath, 'src/app.tsx'], { wslDistro: WSL_DISTRO }) + + const invocations = capturedInvocations() + expect(invocations).toHaveLength(2) + expect(invocations[0]).toEqual(['add', '--', `:(literal)${hugePath}`]) + expect(invocations[1]).toEqual(['add', '--', ':(literal)src/app.tsx']) + }) + + it('packs chunks to the budget instead of splitting timidly', async () => { + const { bulkStageFiles } = await import('./staging') + + await bulkStageFiles(WSL_WORKTREE, realisticChangedPaths(100), { wslDistro: WSL_DISTRO }) + + const lengths = capturedInvocations().map((args) => finishedCommandLineLength(args, WSL_DISTRO)) + // Every chunk but the last is filled to within one pathspec of the cap. + expect(Math.min(...lengths.slice(0, -1))).toBeGreaterThan(MAX_COMMAND_LINE_CHARS * 0.9) + }) + + it('gives a native Windows git.exe the CreateProcess cap and a POSIX host a larger one', async () => { + const { bulkPathspecCommands } = await import('./git-pathspec') + // Long enough that the raw argv alone passes the Windows cap with no wrapper in sight. + const filePaths = Array.from( + { length: 100 }, + (_, index) => `packages/${'deeply-nested-module/'.repeat(18)}file-${index}.ts` + ) + + const windowsNative = bulkPathspecCommands(['add', '--'], filePaths, 'C:\\repo', {}) + expect(windowsNative.length).toBeGreaterThan(1) + for (const args of windowsNative) { + expect(finishedCommandLineLength(args)).toBeLessThanOrEqual(MAX_COMMAND_LINE_CHARS) + } + + Object.defineProperty(process, 'platform', { value: 'linux' }) + expect(bulkPathspecCommands(['add', '--'], filePaths, '/repo', {})).toHaveLength(1) + }) + + it('does not charge a native invocation for the WSL wrapper', async () => { + Object.defineProperty(process, 'platform', { value: 'darwin' }) + const { bulkStageFiles } = await import('./staging') + + await bulkStageFiles('/home/emilio/projects/orca', realisticChangedPaths(100)) + + // Same 100 paths that need several chunks under the WSL wrapper stay one native call. + expect(capturedInvocations()).toHaveLength(1) + }) +}) diff --git a/src/main/git/source-control/discard-changes.ts b/src/main/git/source-control/discard-changes.ts index 33fd2f3f84a..e06f76f0c44 100644 --- a/src/main/git/source-control/discard-changes.ts +++ b/src/main/git/source-control/discard-changes.ts @@ -7,7 +7,7 @@ import type { GitRuntimeOptions } from '../git-runtime-options' import { gitOptionsForWorktree } from '../git-runtime-options' import { gitExecFileAsync } from '../runner' import { invalidateGitReadCaches } from './git-read-cache-invalidation' -import { BULK_CHUNK_SIZE, isTrackedPathSpec, literalPathspec } from './git-pathspec' +import { bulkPathspecCommands, isTrackedPathSpec, literalPathspec } from './git-pathspec' /** * Discard working tree changes for a file. @@ -62,14 +62,15 @@ async function listTrackedPathSpecs( options: GitRuntimeOptions = {} ): Promise { const trackedPaths: string[] = [] - for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) { - const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE) - const { stdout } = await gitExecFileAsync( - ['ls-files', '-z', '--', ...chunk.map((filePath) => literalPathspec(filePath, options))], - { - ...gitOptionsForWorktree(worktreePath, options) - } - ) + const commands = bulkPathspecCommands(['ls-files', '-z', '--'], filePaths, worktreePath, options) + for (const args of commands) { + const { stdout } = await gitExecFileAsync(args, { + ...gitOptionsForWorktree(worktreePath, options), + // Why: this buffers rather than streams, so it must fence -- an unfenced WSL + // login shell glues its rc banner onto the first NUL record, and a tracked + // path that fails to match is silently reclassified as untracked. + captureWslLoginShellOutput: true + }) // Why: a tracked directory can hold enough paths to exceed the JS argument limit. for (const trackedPath of stdout.split('\0')) { if (trackedPath) { @@ -85,17 +86,11 @@ async function cleanUntrackedPaths( filePaths: readonly string[], options: GitRuntimeOptions = {} ): Promise { - for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) { - const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE) - if (chunk.length > 0) { - // Why: Git pathspec cleanup avoids raw recursive deletion through symlinked parents. - await gitExecFileAsync( - ['clean', '-ffdx', '--', ...chunk.map((filePath) => literalPathspec(filePath, options))], - { - ...gitOptionsForWorktree(worktreePath, options) - } - ) - } + // Why: Git pathspec cleanup avoids raw recursive deletion through symlinked parents. + // A pathspec-free `clean -ffdx` would sweep the whole worktree; the chunker emits no empty chunk. + const commands = bulkPathspecCommands(['clean', '-ffdx', '--'], filePaths, worktreePath, options) + for (const args of commands) { + await gitExecFileAsync(args, { ...gitOptionsForWorktree(worktreePath, options) }) } } @@ -133,20 +128,14 @@ export async function bulkDiscardChanges( untrackedPaths, (targetPaths) => cleanUntrackedPaths(worktreePath, targetPaths, options), async () => { - for (let i = 0; i < trackedPaths.length; i += BULK_CHUNK_SIZE) { - const chunk = trackedPaths.slice(i, i + BULK_CHUNK_SIZE) - await gitExecFileAsync( - [ - 'restore', - '--worktree', - '--source=HEAD', - '--', - ...chunk.map((filePath) => literalPathspec(filePath, options)) - ], - { - ...gitOptionsForWorktree(worktreePath, options) - } - ) + const commands = bulkPathspecCommands( + ['restore', '--worktree', '--source=HEAD', '--'], + trackedPaths, + worktreePath, + options + ) + for (const args of commands) { + await gitExecFileAsync(args, { ...gitOptionsForWorktree(worktreePath, options) }) } } ) diff --git a/src/main/git/source-control/git-pathspec.ts b/src/main/git/source-control/git-pathspec.ts index 29ce04ade99..c4fe23d697e 100644 --- a/src/main/git/source-control/git-pathspec.ts +++ b/src/main/git/source-control/git-pathspec.ts @@ -1,6 +1,20 @@ +import { + commandLineLength, + MAX_COMMAND_LINE_CHARS +} from '../../../shared/windows-command-line-budget' +import { resolveGitCommandWithoutProbe } from '../command-runner/git-command-resolution' import type { GitRuntimeOptions } from '../git-runtime-options' -export const BULK_CHUNK_SIZE = 100 +/** Ceiling on argv entries per invocation; under WSL the byte budget bites first. */ +const BULK_CHUNK_SIZE = 100 + +/** + * POSIX hosts have no CreateProcess cap: ARG_MAX is 256KB on macOS and 2MB on + * Linux, shared with the environment block. Half the macOS floor keeps a native + * or SSH-host invocation clear of E2BIG without charging it the WSL wrapper's + * quoting overhead, which is a different transport's problem. + */ +const POSIX_COMMAND_LINE_BUDGET = 128_000 function normalizeGitPathForCompare(filePath: string): string { return filePath.replace(/\\/g, '/').replace(/\/+$/, '') @@ -19,3 +33,71 @@ export function isTrackedPathSpec(filePath: string, trackedPaths: readonly strin return normalizedTracked === normalized || normalizedTracked.startsWith(`${normalized}/`) }) } + +/** + * Length of the line the OS will actually be handed, wrapper included. + * + * Why resolve rather than estimate: a WSL-routed write goes through the login + * shell, which shell-quotes every pathspec, quotes the resulting command line + * again, and embeds that three times (one branch per guest shell). The finished + * line runs ~3.4x the raw pathspec bytes, and nothing about the path list says + * so. Writes never take the direct-git lane, so this is the exact shape they + * get; a read that does take it resolves shorter, so the estimate stays safe. + */ +function finishedCommandLineLength( + args: readonly string[], + worktreePath: string, + options: GitRuntimeOptions +): number { + const resolved = resolveGitCommandWithoutProbe([...args], { + cwd: worktreePath, + ...(options.wslDistro ? { wslDistro: options.wslDistro } : {}) + }) + return commandLineLength([resolved.binary, ...resolved.args]) +} + +/** + * Split a bulk pathspec operation into invocations the host can actually spawn. + * + * Why a byte budget and not a path count: 100 was chosen against a raw argv, but + * a WSL-routed `git add` is folded into one login-shell command line, so 100 + * ordinary paths reached ~43,000 characters -- past the 32,767 CreateProcess cap + * -- and the bulk stage failed with nothing staged. Cost is measured per + * pathspec through the real resolver so the wrapper's quoting rules live in one + * place. + * + * Chunks split only between whole pathspecs, and a pathspec that alone exceeds + * the budget still ships alone rather than being dropped or truncated. If an + * invocation fails partway through, the earlier chunks stay applied: every + * operation here is idempotent and per-path, so `git status` shows the true + * state and re-running converges. + */ +export function bulkPathspecCommands( + leadingArgs: readonly string[], + filePaths: readonly string[], + worktreePath: string, + options: GitRuntimeOptions +): string[][] { + // Budget belongs to the host that spawns; the overhead measured above belongs to the transport. + const budget = process.platform === 'win32' ? MAX_COMMAND_LINE_CHARS : POSIX_COMMAND_LINE_BUDGET + const baseLength = finishedCommandLineLength(leadingArgs, worktreePath, options) + const commands: string[][] = [] + let pathspecs: string[] = [] + let length = baseLength + for (const filePath of filePaths) { + const pathspec = literalPathspec(filePath, options) + const cost = + finishedCommandLineLength([...leadingArgs, pathspec], worktreePath, options) - baseLength + if (pathspecs.length > 0 && (pathspecs.length >= BULK_CHUNK_SIZE || length + cost > budget)) { + commands.push([...leadingArgs, ...pathspecs]) + pathspecs = [] + length = baseLength + } + pathspecs.push(pathspec) + length += cost + } + if (pathspecs.length > 0) { + commands.push([...leadingArgs, ...pathspecs]) + } + return commands +} diff --git a/src/main/git/source-control/staging.ts b/src/main/git/source-control/staging.ts index 5aa8f55fa48..3aa6e16bbc9 100644 --- a/src/main/git/source-control/staging.ts +++ b/src/main/git/source-control/staging.ts @@ -2,7 +2,7 @@ import type { GitRuntimeOptions } from '../git-runtime-options' import { gitOptionsForWorktree } from '../git-runtime-options' import { gitExecFileAsync } from '../runner' import { invalidateGitReadCaches } from './git-read-cache-invalidation' -import { BULK_CHUNK_SIZE, literalPathspec } from './git-pathspec' +import { bulkPathspecCommands, literalPathspec } from './git-pathspec' /** * Stage a file. @@ -54,12 +54,8 @@ export async function bulkStageFiles( return } try { - for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) { - const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE) - await gitExecFileAsync( - ['add', '--', ...chunk.map((filePath) => literalPathspec(filePath, options))], - gitOptionsForWorktree(worktreePath, options) - ) + for (const args of bulkPathspecCommands(['add', '--'], filePaths, worktreePath, options)) { + await gitExecFileAsync(args, gitOptionsForWorktree(worktreePath, options)) } } finally { invalidateGitReadCaches() @@ -79,19 +75,14 @@ export async function bulkUnstageFiles( return } try { - for (let i = 0; i < filePaths.length; i += BULK_CHUNK_SIZE) { - const chunk = filePaths.slice(i, i + BULK_CHUNK_SIZE) - await gitExecFileAsync( - [ - 'restore', - '--staged', - '--', - ...chunk.map((filePath) => literalPathspec(filePath, options)) - ], - { - ...gitOptionsForWorktree(worktreePath, options) - } - ) + const commands = bulkPathspecCommands( + ['restore', '--staged', '--'], + filePaths, + worktreePath, + options + ) + for (const args of commands) { + await gitExecFileAsync(args, { ...gitOptionsForWorktree(worktreePath, options) }) } } finally { invalidateGitReadCaches() diff --git a/src/main/git/source-control/wsl-tracked-pathspec-banner.test.ts b/src/main/git/source-control/wsl-tracked-pathspec-banner.test.ts new file mode 100644 index 00000000000..fbae736e77f --- /dev/null +++ b/src/main/git/source-control/wsl-tracked-pathspec-banner.test.ts @@ -0,0 +1,109 @@ +import { EventEmitter } from 'node:events' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { execFileMock, execFileSyncMock, spawnMock } = vi.hoisted(() => ({ + execFileMock: vi.fn(), + execFileSyncMock: vi.fn(), + spawnMock: vi.fn() +})) + +vi.mock('node:child_process', () => ({ + execFile: execFileMock, + execFileSync: execFileSyncMock, + spawn: spawnMock +})) +vi.mock('../../observability/instrumentation', () => ({ + withGitSpan: (_attributes: unknown, run: () => unknown) => run() +})) +vi.mock('../../diagnostics/main-thread-churn-probe', () => ({ recordSubprocessSpawn: vi.fn() })) +vi.mock('./git-read-cache-invalidation', () => ({ invalidateGitReadCaches: vi.fn() })) +// Stand in for the on-disk safety filter: every discard target here exists and is symlink-free. +vi.mock('../../../shared/git-discard-path-safety', () => ({ + removeSafeUntrackedDiscardTarget: vi.fn(), + removeSafeUntrackedDiscardTargets: async ( + _worktreePath: string, + untrackedPaths: string[], + cleanUntracked: (paths: string[]) => Promise, + restoreTracked: () => Promise + ) => { + await restoreTracked() + if (untrackedPaths.length > 0) { + await cleanUntracked(untrackedPaths) + } + } +})) + +import { bulkDiscardChanges } from './discard-changes' +import { resetWslGitReadEnvironmentForTests } from '../wsl-git-read-environment' + +const DISTRO = 'Ubuntu-24.04' +const WSL_WORKTREE = `\\\\wsl$\\${DISTRO}\\home\\emilio\\projects\\orca` +const TRACKED_PATHS = ['docs/architecture.md', 'src/main/git/runner.ts'] +// Stock Ubuntu writes this to *stdout* from the interactive login shell's rc. +const BANNER = 'To run a command as administrator (user "root"), use "sudo ".\n\n' + +type MockChild = EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; kill: () => void } + +function createMockChild(): MockChild { + const child = new EventEmitter() as MockChild + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + child.kill = vi.fn() + return child +} + +function guestScript(args: unknown): string { + return (args as string[] | undefined)?.join(' ') ?? '' +} + +/** Wrap the payload in the caller's own fence when it asked for one; otherwise hand it over raw. */ +function loginShellStdout(script: string, payload: string): string { + const nonce = /__ORCA_WSL_CAPTURE_BEGIN_([^_]+)__/.exec(script)?.[1] + return nonce + ? `${BANNER}__ORCA_WSL_CAPTURE_BEGIN_${nonce}__${payload}__ORCA_WSL_CAPTURE_END_${nonce}__` + : `${BANNER}${payload}` +} + +function gitCommandLines(): string[] { + return execFileMock.mock.calls + .map((call) => guestScript(call[1])) + .filter((script) => !script.includes('_orca_git_path=')) +} + +describe('WSL tracked-path listing behind a login-shell banner', () => { + const realPlatform = process.platform + + beforeEach(() => { + resetWslGitReadEnvironmentForTests() + execFileMock.mockReset() + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + execFileMock.mockImplementation((_command, args, _options, callback) => { + const script = guestScript(args) + if (script.includes('_orca_git_path=')) { + // Distro exports GIT_* / XDG_CONFIG_HOME, so the direct-git read probe is rejected for good. + queueMicrotask(() => callback?.(Object.assign(new Error('probe rejected'), { code: 78 }))) + return createMockChild() + } + const payload = script.includes('ls-files') ? `${TRACKED_PATHS.join('\0')}\0` : '' + queueMicrotask(() => callback?.(null, loginShellStdout(script, payload), '')) + return createMockChild() + }) + }) + + afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: realPlatform }) + resetWslGitReadEnvironmentForTests() + }) + + it('restores every tracked path instead of routing the first one to git clean', async () => { + await bulkDiscardChanges(WSL_WORKTREE, [...TRACKED_PATHS], { wslDistro: DISTRO }) + + const commandLines = gitCommandLines() + expect(commandLines.filter((line) => line.includes('clean'))).toEqual([]) + const restored = commandLines.filter((line) => line.includes('restore')) + expect(restored.length).toBeGreaterThan(0) + for (const trackedPath of TRACKED_PATHS) { + expect(restored.some((line) => line.includes(`:(literal)${trackedPath}`))).toBe(true) + } + }) +}) diff --git a/src/main/git/status-discard-and-bulk-staging.test.ts b/src/main/git/status-discard-and-bulk-staging.test.ts index b1f39f9102b..8e0b12623c4 100644 --- a/src/main/git/status-discard-and-bulk-staging.test.ts +++ b/src/main/git/status-discard-and-bulk-staging.test.ts @@ -198,7 +198,9 @@ describe('bulk git helpers', () => { ':(literal)scratch' ], { - cwd: '/repo' + cwd: '/repo', + // Why: this read buffers, so it fences the WSL login shell's rc banner off its first NUL record. + captureWslLoginShellOutput: true } ) // Why: a pathspec is tracked if git reports either the exact path or a diff --git a/src/main/wsl/wsl-runner.ts b/src/main/wsl/wsl-runner.ts index 6a0fbc051ac..30097246a86 100644 --- a/src/main/wsl/wsl-runner.ts +++ b/src/main/wsl/wsl-runner.ts @@ -1,4 +1,5 @@ import { addWslEnvKeys } from '../../shared/wsl-env' +import { commandLineLength, MAX_COMMAND_LINE_CHARS } from '../../shared/windows-command-line-budget' import { runProcess } from '../../shared/child-process/run-process' import { buildWslExecArgs } from '../../shared/wsl-login-shell-command' import { getWslGuestEnvironment, type WslGuestEnvironment } from './wsl-guest-environment' @@ -159,25 +160,6 @@ function withGuestCwd(cwd: string | undefined, argv: readonly string[]): string[ return ['sh', '-c', 'cd "$1" || exit 1; shift; exec "$@"', 'orca-wsl', cwd, ...argv] } -/** - * Argv is the default, but it has a hard ceiling that stdin does not. - * - * Windows caps a command line at 32767 characters, and the distro, `--exec`, - * the env prefix and the args all share it. A user's `orca.yaml` hook is the - * one unbounded script Orca runs -- `run-both` concatenates two of them, and a - * vendored installer is ~15KB -- so past this size the choice is between - * failing to spawn at all and accepting the stdin caveat. Degrading beats - * failing: a large script that also reads stdin was already broken, while a - * large script that does not now works where it would have died. - * - * Measured on the WHOLE command line, not on the script alone. A login PATH is - * itself a few KB and is spliced in as `PATH=...`, so a script-only threshold - * produced a perverse band: with a long enough PATH, a 7,999-char hook went to - * argv and failed to spawn while the same hook at 8,001 chars flipped to stdin - * and ran. Size decided how a hook behaved, in the wrong direction. - */ -const MAX_COMMAND_LINE_CHARS = 30_000 - /** ` -c`/`-s` for a script, otherwise the program itself. */ function guestCommandArgv(spec: WslSpec, delivery: 'argv' | 'stdin'): string[] { if (spec.script === undefined) { @@ -190,19 +172,6 @@ function guestCommandArgv(spec: WslSpec, delivery: 'argv' | 'stdin'): string[] { : [shell, '-c', spec.script, '--', ...(spec.args ?? [])] } -/** - * What `CreateProcess` will count. - * - * libuv escapes every `"` and doubles a backslash run before a quote, so a - * quote-dense script costs more than its length. Charging one extra character - * per `"` or `\\` keeps the estimate on the safe side of the cap; an earlier - * version claimed to over-count and in fact under-counted, which put a - * quote-heavy ~26KB script on argv and over the real limit. - */ -function commandLineLength(args: readonly string[]): number { - return args.reduce((total, arg) => total + arg.length + 3 + (arg.match(/["\\]/g)?.length ?? 0), 0) -} - /** Shell-free argv, with the cached environment applied when one is available. */ function buildGuestArgv( environment: WslGuestEnvironment | null, @@ -255,6 +224,9 @@ export async function runWslProcess(spec: WslSpec): Promise { // Measure what is actually spawned: `wsl.exe` and `-d --exec` are // prepended after this point and are part of the same budget. const fullLine = [resolveWslExecutablePath(), ...buildWslExecArgs(spec.distro, argvForm)] + // Argv is the default, but it has a hard ceiling that stdin does not. A user's + // `orca.yaml` hook is the one unbounded script Orca runs, so past the cap the + // choice is between failing to spawn at all and accepting the stdin caveat. const delivery: 'argv' | 'stdin' = spec.script !== undefined && commandLineLength(fullLine) > MAX_COMMAND_LINE_CHARS ? 'stdin' diff --git a/src/shared/windows-command-line-budget.ts b/src/shared/windows-command-line-budget.ts new file mode 100644 index 00000000000..77074e3f531 --- /dev/null +++ b/src/shared/windows-command-line-budget.ts @@ -0,0 +1,28 @@ +/** + * How long a command line may get before `CreateProcess` refuses it. + * + * Windows caps a command line at 32767 characters, and *everything* shares that + * one budget: the binary, `wsl.exe -d --exec`, the login-shell wrapper + * and the payload. So the number only means anything when it is measured on the + * FINISHED line. Two shipped defects came from measuring a part instead: a + * script-only threshold in the WSL runner, where a multi-KB login PATH pushed a + * legal-looking script over the real limit, and count-only chunking of bulk git + * pathspecs, where the login-shell wrapper tripled the line behind our back. + * + * The 2767-character margin absorbs what we do not model exactly (the distro + * name, libuv's requoting of the outer argv). + */ +export const MAX_COMMAND_LINE_CHARS = 30_000 + +/** + * What `CreateProcess` will count. + * + * libuv escapes every `"` and doubles a backslash run before a quote, so a + * quote-dense script costs more than its length. Charging one extra character + * per `"` or `\\` keeps the estimate on the safe side of the cap; an earlier + * version claimed to over-count and in fact under-counted, which put a + * quote-heavy ~26KB script on argv and over the real limit. + */ +export function commandLineLength(args: readonly string[]): number { + return args.reduce((total, arg) => total + arg.length + 3 + (arg.match(/["\\]/g)?.length ?? 0), 0) +}