From e109e78ebff0f624e98796188caaed0b2774463c Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Mon, 20 Jul 2026 17:50:12 -0400 Subject: [PATCH] fix(source-control): keep huge change sets responsive (#9477) * fix(source-control): keep huge change sets responsive * Fix cancellation and retry handling for capped status * Harden capped status for conflict-heavy repositories * Harden capped status recovery and cancellation * fix(source-control): preserve capped status correctness * fix(source-control): translate submodule status at render time --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> --- src/main/git/runner-command-exec.test.ts | 16 ++ src/main/git/runner.ts | 4 + src/main/git/status-porcelain-parser.test.ts | 38 +++- src/main/git/status.test.ts | 72 ++++++++ src/main/git/status.ts | 46 ++--- src/main/ipc/filesystem.test.ts | 24 +++ src/main/providers/ssh-git-provider.test.ts | 7 +- src/main/runtime/rpc/methods/git.test.ts | 6 +- src/relay/agent-exec-handler.test.ts | 56 ++++-- src/relay/agent-exec-handler.ts | 34 +--- src/relay/git-handler-status-ops.test.ts | 163 +++++++++++++++--- src/relay/git-handler-status-ops.ts | 63 ++++--- ...dler-submodule-status-cancellation.test.ts | 42 +++++ src/relay/git-handler.ts | 51 ++++-- src/relay/git-stdout-stream.test.ts | 136 +++++++++++++++ src/relay/git-stdout-stream.ts | 148 ++++++++++++++++ src/relay/subprocess-tree-termination.test.ts | 32 ++++ src/relay/subprocess-tree-termination.ts | 21 +++ .../right-sidebar/SourceControl.tsx | 154 +++++++++++++---- ...source-control-submodule-expansion.test.ts | 50 ++++++ .../source-control-submodule-expansion.ts | 22 ++- ...e-control-too-many-changes-banner.test.tsx | 62 +++++++ .../right-sidebar/useGitStatusPolling.test.ts | 5 +- .../useSourceControlSubmoduleStatus.test.tsx | 8 +- .../useSourceControlSubmoduleStatus.ts | 6 +- src/renderer/src/i18n/locales/en.json | 4 +- src/renderer/src/i18n/locales/es.json | 4 +- src/renderer/src/i18n/locales/ja.json | 4 +- src/renderer/src/i18n/locales/ko.json | 4 +- src/renderer/src/i18n/locales/zh.json | 4 +- src/renderer/src/store/slices/editor.test.ts | 63 +++++++ src/renderer/src/store/slices/editor.ts | 38 +++- src/shared/git-status-limit.test.ts | 38 ++++ src/shared/git-status-limit.ts | 32 +++- .../git-status-porcelain-parser.ts} | 19 +- src/shared/git-uncommitted-line-stats.test.ts | 9 +- tests/e2e/large-file-count-fixtures.ts | 4 + .../source-control-large-file-count.spec.ts | 98 ++++++++--- 38 files changed, 1365 insertions(+), 222 deletions(-) create mode 100644 src/relay/git-handler-submodule-status-cancellation.test.ts create mode 100644 src/relay/git-stdout-stream.test.ts create mode 100644 src/relay/git-stdout-stream.ts create mode 100644 src/relay/subprocess-tree-termination.test.ts create mode 100644 src/relay/subprocess-tree-termination.ts create mode 100644 src/renderer/src/components/right-sidebar/source-control-too-many-changes-banner.test.tsx create mode 100644 src/shared/git-status-limit.test.ts rename src/{main/git/status-porcelain-parser.ts => shared/git-status-porcelain-parser.ts} (90%) diff --git a/src/main/git/runner-command-exec.test.ts b/src/main/git/runner-command-exec.test.ts index 62fbde608..97f8eb825 100644 --- a/src/main/git/runner-command-exec.test.ts +++ b/src/main/git/runner-command-exec.test.ts @@ -661,6 +661,22 @@ describe('gitStreamStdout', () => { await rejection expect(child.kill).toHaveBeenCalled() }) + + it('handles a late spawn error after cancellation', async () => { + const child = createMockChildProcess(0) + spawnMock.mockReturnValue(child) + const controller = new AbortController() + + const promise = gitStreamStdout(['status'], { + cwd: '/repo', + signal: controller.signal, + onStdout: () => {} + }) + controller.abort() + + await expect(promise).rejects.toMatchObject({ name: 'AbortError' }) + expect(() => child.emit('error', new Error('spawn ENOENT'))).not.toThrow() + }) }) describe('translateWslOutputPaths', () => { diff --git a/src/main/git/runner.ts b/src/main/git/runner.ts index fac9f57ec..a4b51ff06 100644 --- a/src/main/git/runner.ts +++ b/src/main/git/runner.ts @@ -1035,6 +1035,10 @@ export async function gitStreamStdout( finish(new Error(`git exited with ${code}: ${stderr}`)) } function onAbort(): void { + if (!child.pid) { + // Why: failed spawn reports ENOENT after abort cleanup; retain a listener so it cannot crash main. + child.once('error', () => {}) + } void killSpawnedCommandTree(child) finish(createAbortError()) } diff --git a/src/main/git/status-porcelain-parser.test.ts b/src/main/git/status-porcelain-parser.test.ts index 4f9347b64..e3cd652e6 100644 --- a/src/main/git/status-porcelain-parser.test.ts +++ b/src/main/git/status-porcelain-parser.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { StatusPorcelainParser } from './status-porcelain-parser' +import { StatusPorcelainParser } from '../../shared/git-status-porcelain-parser' describe('StatusPorcelainParser', () => { it('parses branch headers and changed/untracked/ignored records', () => { @@ -64,6 +64,7 @@ describe('StatusPorcelainParser', () => { parser.finish() expect(parser.entries).toEqual([]) expect(parser.unmergedLines).toHaveLength(1) + expect(parser.statusLength).toBe(1) }) it('carries a partial trailing line across chunk boundaries', () => { @@ -94,6 +95,41 @@ describe('StatusPorcelainParser', () => { expect(parser.statusLength).toBe(4) }) + it('counts unmerged records toward the stop limit', () => { + const parser = new StatusPorcelainParser() + const line = 'u UU N... 100644 100644 100644 100644 aa bb cc conflicted.ts\n' + const stopped = parser.update(line.repeat(4), 3) + + expect(stopped).toBe(true) + expect(parser.unmergedLines).toHaveLength(4) + expect(parser.statusLength).toBe(4) + }) + + it('preserves deferred conflicts in status output order', () => { + const parser = new StatusPorcelainParser() + parser.update( + '? before.ts\n' + + 'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts\n' + + '? after.ts\n', + 0 + ) + + expect(parser.statusRecords).toEqual([ + { + type: 'entry', + entry: { path: 'before.ts', status: 'untracked', area: 'untracked' } + }, + { + type: 'unmerged', + line: 'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts' + }, + { + type: 'entry', + entry: { path: 'after.ts', status: 'untracked', area: 'untracked' } + } + ]) + }) + it('does not signal stop when limit is 0 (disabled)', () => { const parser = new StatusPorcelainParser() const lines = `${Array.from({ length: 50 }, (_, i) => `? f${i}.txt`).join('\n')}\n` diff --git a/src/main/git/status.test.ts b/src/main/git/status.test.ts index fd5e16f91..6084d67f5 100644 --- a/src/main/git/status.test.ts +++ b/src/main/git/status.test.ts @@ -1040,6 +1040,34 @@ describe('getSubmoduleStatus', () => { expect(result.entries).toContainEqual( expect.objectContaining({ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }) ) + expect(gitExecFileAsyncMock.mock.calls.some(([args]) => args.includes('status'))).toBe(false) + }) + + it('caps staged commit-range entries before returning them to the renderer', async () => { + const OLD_OID = 'a'.repeat(40) + const NEW_OID = 'b'.repeat(40) + gitExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockImplementation((args: string[]) => { + if (args.includes('--name-status')) { + return Promise.resolve({ stdout: 'M\tlib/a.dart\nM\tlib/b.dart\n' }) + } + if (args[0] === 'ls-files') { + return Promise.resolve({ stdout: `160000 ${NEW_OID} 0\tflutter_mine\n` }) + } + if (args[0] === 'ls-tree') { + return Promise.resolve({ stdout: `160000 commit ${OLD_OID}\tflutter_mine\n` }) + } + return Promise.resolve({ stdout: '' }) + }) + + const result = await getSubmoduleStatus('/repo', 'flutter_mine', { + staged: true, + limit: 1 + }) + + expect(result.entries).toHaveLength(1) + expect(result.didHitLimit).toBe(true) + expect(result.statusLength).toBe(2) }) }) @@ -1685,6 +1713,50 @@ describe('getStatus', () => { expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) }) + it('caps unmerged conflicts and keeps the visible conflict rows', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(true) + const lines = [ + 'u UU S... 160000 160000 160000 160000 aa bb cc vendor/submodule', + ...Array.from( + { length: 3 }, + (_, i) => `u UU N... 100644 100644 100644 100644 aa bb cc conflict-${i}.ts` + ) + ].join('\n') + gitExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: `${lines}\n` }) + + const result = await getStatus('/repo', { limit: 2 }) + + expect(result.didHitLimit).toBe(true) + expect(result.statusLength).toBe(3) + expect(result.entries).toHaveLength(2) + expect(result.entries.map((entry) => entry.path)).toEqual(['conflict-0.ts', 'conflict-1.ts']) + expect(result.entries.every((entry) => entry.conflictStatus === 'unresolved')).toBe(true) + expect(gitExecFileAsyncMock).toHaveBeenCalledTimes(1) + }) + + it('keeps an early conflict ahead of later ordinary rows at the cap', async () => { + readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') + existsSyncMock.mockReturnValue(true) + const lines = [ + '? before.ts', + 'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts', + '? after.ts' + ].join('\n') + gitExecFileAsyncMock.mockReset() + gitExecFileAsyncMock.mockResolvedValueOnce({ stdout: `${lines}\n` }) + + const result = await getStatus('/repo', { limit: 2 }) + + expect(result.didHitLimit).toBe(true) + expect(result.entries.map((entry) => entry.path)).toEqual(['before.ts', 'conflict.ts']) + expect(result.entries[1]).toMatchObject({ + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + }) + }) + it('does not flag didHitLimit for a normal repo under the limit', async () => { readFileMock.mockResolvedValue('gitdir: /repo/.git/worktrees/feature\n') existsSyncMock.mockReturnValue(false) diff --git a/src/main/git/status.ts b/src/main/git/status.ts index d325003fd..34c5d7504 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -37,8 +37,8 @@ import { gitOptionalLocksDisabledEnv, gitStreamStdout } from './runner' -import { StatusPorcelainParser } from './status-porcelain-parser' -import { DEFAULT_GIT_STATUS_LIMIT } from '../../shared/git-status-limit' +import { StatusPorcelainParser } from '../../shared/git-status-porcelain-parser' +import { capGitStatusEntries, resolveGitStatusLimit } from '../../shared/git-status-limit' import { describeMaxBufferOverflowError, isMaxBufferOverflowError } from './max-buffer-overflow' import { removeSafeUntrackedDiscardTarget, @@ -231,10 +231,7 @@ export async function getStatus( function getStatusReadKey(worktreePath: string, options: GetStatusOptions): string { // Why: each key part can change the output shape or runtime routing. - const limit = - typeof options.limit === 'number' && Number.isInteger(options.limit) && options.limit >= 0 - ? options.limit - : DEFAULT_GIT_STATUS_LIMIT + const limit = resolveGitStatusLimit(options.limit) return [ worktreePath, options.wslDistro ?? '', @@ -254,10 +251,7 @@ async function runGetStatus( let effectiveUpstreamStatus: GitUpstreamStatus | undefined let statusSucceeded = false // Why: a bad limit (negative/fractional/NaN) breaks early-stop; require a valid non-negative int (0 disables the cap). - const limit = - typeof options.limit === 'number' && Number.isInteger(options.limit) && options.limit >= 0 - ? options.limit - : DEFAULT_GIT_STATUS_LIMIT + const limit = resolveGitStatusLimit(options.limit) // Why: detectConflictOperation and git status are independent, so run them concurrently to save I/O latency. const conflictPromise = detectConflictOperation(worktreePath) @@ -301,14 +295,19 @@ async function runGetStatus( // Not a git repo or git not available } - // Why: the parser overshoots by one (checks after pushing), so trim to exactly `limit`. - const entries = didHitLimit ? parser.entries.slice(0, limit) : parser.entries + const entries: GitStatusEntry[] = [] const { head, branch, upstreamName, upstreamAheadBehind } = parser.branch - // Why: unmerged (`u`) records need async per-file lookups; resolve them off the hot path (conflicts are rare). - if (!didHitLimit) { - for (const line of parser.unmergedLines) { - const unmergedEntry = await parseUnmergedEntry(worktreePath, line) + // Why: resolve deferred conflicts in Git's output order so the cap cannot hide + // an early conflict behind ordinary rows that appeared later in the stream. + for (const record of parser.statusRecords) { + if (didHitLimit && entries.length >= limit) { + break + } + if (record.type === 'entry') { + entries.push(record.entry) + } else { + const unmergedEntry = await parseUnmergedEntry(worktreePath, record.line) if (unmergedEntry) { entries.push(unmergedEntry) } @@ -414,10 +413,14 @@ export function resolveSubmoduleWorktreePath(worktreePath: string, submodulePath export async function getSubmoduleStatus( worktreePath: string, submodulePath: string, - options: GitRuntimeOptions & { staged?: boolean } = {} + options: GetStatusOptions & { staged?: boolean } = {} ): Promise { const submoduleWorktreePath = resolveSubmoduleWorktreePath(worktreePath, submodulePath) - const workingResult = await getStatus(submoduleWorktreePath, options) + const limit = resolveGitStatusLimit(options.limit) + // Why: staged expansion only represents HEAD→index; scanning the submodule worktree is wasted work. + const workingResult = options.staged + ? ({ entries: [], conflictOperation: 'unknown' } satisfies GitStatusResult) + : await getStatus(submoduleWorktreePath, options) // Why: a moved gitlink (clean worktree) has no status rows; surface the parent-commit→checkout range as inner rows. const fromOid = options.staged ? await readGitlinkOidFromTree(worktreePath, 'HEAD', submodulePath, options) @@ -434,7 +437,7 @@ export async function getSubmoduleStatus( options ) if (options.staged) { - return { ...workingResult, entries: rangeEntries } + return { ...workingResult, ...capGitStatusEntries(rangeEntries, limit) } } const rangePaths = new Set(rangeEntries.map((entry) => entry.path)) // Range rows win on overlap so the diff matches getDiff's commit-range route. @@ -442,7 +445,10 @@ export async function getSubmoduleStatus( ...rangeEntries, ...workingResult.entries.filter((entry) => !rangePaths.has(entry.path)) ] - return { ...workingResult, entries } + return { + ...workingResult, + ...capGitStatusEntries(entries, limit, workingResult) + } } if (options.staged) { return { ...workingResult, entries: [] } diff --git a/src/main/ipc/filesystem.test.ts b/src/main/ipc/filesystem.test.ts index d7e48baee..61f2dee64 100644 --- a/src/main/ipc/filesystem.test.ts +++ b/src/main/ipc/filesystem.test.ts @@ -1378,6 +1378,30 @@ describe('registerFilesystemHandlers', () => { expect(sshProvider.getStatus).toHaveBeenCalledWith('/remote/repo', { includeIgnored: true }) }) + it('returns capped-state metadata unchanged across local and SSH status IPC', async () => { + const cappedStatus = { + entries: [{ path: 'generated/a.ts', status: 'untracked', area: 'untracked' }], + conflictOperation: 'unknown', + didHitLimit: true, + statusLength: 1_001 + } + registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) + getStatusMock.mockResolvedValue(cappedStatus) + const sshProvider = { getStatus: vi.fn().mockResolvedValue(cappedStatus) } + getSshGitProviderMock.mockReturnValue(sshProvider) + registerFilesystemHandlers(store as never) + + await expect( + handlers.get('git:status')!(null, { worktreePath: WORKTREE_FEATURE_PATH }) + ).resolves.toEqual(cappedStatus) + await expect( + handlers.get('git:status')!(null, { + worktreePath: '/remote/repo', + connectionId: 'ssh-1' + }) + ).resolves.toEqual(cappedStatus) + }) + it('forwards upstream-negative-cache bypass through local and SSH git status IPC', async () => { registerWorktreeRootsForRepo(store as never, 'repo-1', [REPO_PATH, WORKTREE_FEATURE_PATH]) getStatusMock.mockResolvedValue({ entries: [], conflictOperation: 'unknown' }) diff --git a/src/main/providers/ssh-git-provider.test.ts b/src/main/providers/ssh-git-provider.test.ts index d4a275a89..1ee5b0f2b 100644 --- a/src/main/providers/ssh-git-provider.test.ts +++ b/src/main/providers/ssh-git-provider.test.ts @@ -57,7 +57,12 @@ describe('SshGitProvider', () => { }) it('getStatus sends git.status request', async () => { - const statusResult = { entries: [], conflictOperation: 'unknown' } + const statusResult = { + entries: [{ path: 'generated/a.ts', status: 'untracked', area: 'untracked' }], + conflictOperation: 'unknown', + didHitLimit: true, + statusLength: 1_001 + } mux.request.mockResolvedValue(statusResult) const result = await provider.getStatus('/home/user/repo') diff --git a/src/main/runtime/rpc/methods/git.test.ts b/src/main/runtime/rpc/methods/git.test.ts index 1655bb0b3..bf6b0838e 100644 --- a/src/main/runtime/rpc/methods/git.test.ts +++ b/src/main/runtime/rpc/methods/git.test.ts @@ -16,7 +16,9 @@ describe('git RPC methods', () => { entries: [], conflictOperation: 'unknown', branch: 'main', - head: 'abc' + head: 'abc', + didHitLimit: true, + statusLength: 1_001 }) } as unknown as OrcaRuntimeService const dispatcher = new RpcDispatcher({ runtime, methods: GIT_METHODS }) @@ -26,7 +28,7 @@ describe('git RPC methods', () => { expect(runtime.getRuntimeGitStatus).toHaveBeenCalledWith('id:wt-1') expect(response).toMatchObject({ ok: true, - result: { entries: [], branch: 'main' } + result: { entries: [], branch: 'main', didHitLimit: true, statusLength: 1_001 } }) }) diff --git a/src/relay/agent-exec-handler.test.ts b/src/relay/agent-exec-handler.test.ts index 0d6d73bab..c884c499d 100644 --- a/src/relay/agent-exec-handler.test.ts +++ b/src/relay/agent-exec-handler.test.ts @@ -1,4 +1,4 @@ -import { exec, spawn } from 'node:child_process' +import { execFile, spawn } from 'node:child_process' import { beforeEach, describe, expect, it, vi } from 'vitest' import type * as ChildProcess from 'node:child_process' import { createFakeChild, createHandlers, requestContext } from './agent-exec-handler-test-harness' @@ -8,20 +8,20 @@ vi.mock('child_process', async (importOriginal) => { const actual = await importOriginal() return { ...actual, - exec: vi.fn(), + execFile: vi.fn(), spawn: vi.fn() } }) const spawnMock = vi.mocked(spawn) -const execMock = vi.mocked(exec) +const execFileMock = vi.mocked(execFile) type AgentExecResult = { exitCode: number | null; timedOut: boolean } describe('AgentExecHandler', () => { beforeEach(() => { spawnMock.mockReset() - execMock.mockReset() + execFileMock.mockReset() }) it('executes a non-interactive command with captured output and stdin', async () => { @@ -204,7 +204,11 @@ describe('AgentExecHandler', () => { ).resolves.toEqual({ canceled: true }) if (process.platform === 'win32') { - expect(execMock).toHaveBeenCalledWith('taskkill /pid 12345 /T /F', expect.any(Function)) + expect(execFileMock).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12345', '/T', '/F'], + expect.any(Function) + ) } else { expect(child.kill).toHaveBeenCalledWith('SIGKILL') } @@ -257,8 +261,16 @@ describe('AgentExecHandler', () => { ).resolves.toEqual({ canceled: true }) if (process.platform === 'win32') { - expect(execMock).toHaveBeenCalledWith('taskkill /pid 12345 /T /F', expect.any(Function)) - expect(execMock).not.toHaveBeenCalledWith('taskkill /pid 12346 /T /F', expect.any(Function)) + expect(execFileMock).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12345', '/T', '/F'], + expect.any(Function) + ) + expect(execFileMock).not.toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12346', '/T', '/F'], + expect.any(Function) + ) } else { expect(commitChild.kill).toHaveBeenCalledWith('SIGKILL') expect(pullRequestChild.kill).not.toHaveBeenCalled() @@ -303,7 +315,11 @@ describe('AgentExecHandler', () => { controller.abort() if (process.platform === 'win32') { - expect(execMock).toHaveBeenCalledWith('taskkill /pid 12345 /T /F', expect.any(Function)) + expect(execFileMock).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12345', '/T', '/F'], + expect.any(Function) + ) } else { expect(child.kill).toHaveBeenCalledWith('SIGKILL') } @@ -351,8 +367,16 @@ describe('AgentExecHandler', () => { ) if (process.platform === 'win32') { - expect(execMock).toHaveBeenCalledWith('taskkill /pid 12345 /T /F', expect.any(Function)) - expect(execMock).not.toHaveBeenCalledWith('taskkill /pid 12346 /T /F', expect.any(Function)) + expect(execFileMock).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12345', '/T', '/F'], + expect.any(Function) + ) + expect(execFileMock).not.toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12346', '/T', '/F'], + expect.any(Function) + ) } else { expect(firstChild.kill).toHaveBeenCalledWith('SIGKILL') expect(secondChild.kill).not.toHaveBeenCalled() @@ -366,7 +390,11 @@ describe('AgentExecHandler', () => { ).resolves.toEqual({ canceled: true }) if (process.platform === 'win32') { - expect(execMock).toHaveBeenCalledWith('taskkill /pid 12346 /T /F', expect.any(Function)) + expect(execFileMock).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12346', '/T', '/F'], + expect.any(Function) + ) } else { expect(secondChild.kill).toHaveBeenCalledWith('SIGKILL') } @@ -411,7 +439,11 @@ describe('AgentExecHandler', () => { expect(outcome).toBe('timed-out:null') if (process.platform === 'win32') { - expect(execMock).toHaveBeenCalledWith('taskkill /pid 12345 /T /F', expect.any(Function)) + expect(execFileMock).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12345', '/T', '/F'], + expect.any(Function) + ) } else { expect(child.kill).toHaveBeenCalledWith('SIGKILL') } diff --git a/src/relay/agent-exec-handler.ts b/src/relay/agent-exec-handler.ts index 2e3868be3..a82791dcf 100644 --- a/src/relay/agent-exec-handler.ts +++ b/src/relay/agent-exec-handler.ts @@ -1,9 +1,10 @@ -import { exec, spawn, type ChildProcess } from 'node:child_process' +import { spawn, type ChildProcess } from 'node:child_process' import { existsSync } from 'node:fs' import { delimiter, join } from 'node:path' import type { RelayDispatcher, RequestContext } from './dispatcher' import { applyTerminalGitCredentialPromptGuard } from '../shared/terminal-git-credential-guard' import { mergeGitConfigEnvProtocol } from '../shared/git-credential-prompt-env' +import { terminateRelaySubprocessTree } from './subprocess-tree-termination' const DEFAULT_TIMEOUT_MS = 60_000 const MAX_TIMEOUT_MS = 5 * 60 * 1000 @@ -66,29 +67,6 @@ function getWindowsSafeSpawn( return { spawnCmd: getCmdExePath(), spawnArgs: ['/d', '/s', '/c', commandLine] } } -// Why: mirrors src/main/text-generation/commit-message-text-generation.ts. On -// Windows, npm-installed CLIs like `claude`/`codex` are usually `.cmd` shims. -// We route those through cmd.exe so Node can launch them, and taskkill is -// needed to terminate the whole wrapper + node.exe process tree. Kept -// duplicated rather than imported because the relay ships to remote hosts. -function killProcessTree(child: ChildProcess): void { - const pid = child.pid - if (!pid) { - return - } - if (process.platform === 'win32') { - exec(`taskkill /pid ${pid} /T /F`, () => { - // Best-effort; the spawn's `close` listener fires once the tree exits. - }) - return - } - try { - child.kill('SIGKILL') - } catch { - // Child may already have exited between the kill request and now. - } -} - type ExecParams = { binary: unknown args: unknown @@ -232,7 +210,7 @@ export class AgentExecHandler { } const cancelCurrent = (): void => { canceled = true - killProcessTree(child) + terminateRelaySubprocessTree(child) } if (laneKey) { // Why: the relay owns one visible non-interactive job per cwd+operation. @@ -252,14 +230,14 @@ export class AgentExecHandler { // Why: tree-kill because some CLIs trap SIGTERM and continue streaming; // also Windows wraps `.cmd` shims in cmd.exe, so the immediate child // is not the real node.exe process. - killProcessTree(child) + terminateRelaySubprocessTree(child) finish({ stdout, stderr, exitCode: null, timedOut, canceled }) }, timeoutMs) const onStdoutData = (chunk: Buffer): void => { stdoutBytes += chunk.byteLength if (stdoutBytes > MAX_OUTPUT_BYTES) { - killProcessTree(child) + terminateRelaySubprocessTree(child) return } stdout += chunk.toString('utf-8') @@ -267,7 +245,7 @@ export class AgentExecHandler { const onStderrData = (chunk: Buffer): void => { stderrBytes += chunk.byteLength if (stderrBytes > MAX_OUTPUT_BYTES) { - killProcessTree(child) + terminateRelaySubprocessTree(child) return } stderr += chunk.toString('utf-8') diff --git a/src/relay/git-handler-status-ops.test.ts b/src/relay/git-handler-status-ops.test.ts index 4a7494d09..2207a982d 100644 --- a/src/relay/git-handler-status-ops.test.ts +++ b/src/relay/git-handler-status-ops.test.ts @@ -4,12 +4,24 @@ import { tmpdir } from 'node:os' import * as path from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { GitExec } from './git-handler-ops' +import type { RelayGitStreamExec } from './git-stdout-stream' import { getStatusOp } from './git-handler-status-ops' import { clearNoEffectiveUpstreamStatusCache } from './git-status-upstream-negative-cache' import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache' +import { DEFAULT_GIT_STATUS_LIMIT } from '../shared/git-status-limit' const LARGE_STATUS_ENTRY_COUNT = 150_000 +function streamGitFromCapture(git: GitExec): RelayGitStreamExec { + return async (args, cwd, options) => { + const { stdout } = await git(args, cwd, { + disableOptionalLocks: options.disableOptionalLocks, + signal: options.signal + }) + return { stoppedEarly: options.onStdout(stdout) === true } + } +} + function buildLargeStatusOutput(count: number): string { const lines: string[] = [] for (let index = 0; index < count; index += 1) { @@ -38,22 +50,35 @@ describe('getStatusOp', () => { }) it('truncates huge status lists at the limit and flags didHitLimit', async () => { - const statusOutput = buildLargeStatusOutput(LARGE_STATUS_ENTRY_COUNT) + let emittedEntries = 0 const git = vi.fn(async (args) => { - if (args.includes('status')) { - return { stdout: statusOutput, stderr: '' } - } - if (args.includes('diff')) { - return { stdout: '', stderr: '' } - } throw new Error(`Unexpected git command: ${args.join(' ')}`) }) + const streamGit = vi.fn(async (_args, _cwd, options) => { + for (let index = 0; index < LARGE_STATUS_ENTRY_COUNT; index += 1) { + emittedEntries += 1 + if ( + options.onStdout( + `1 A. N... 100644 100644 100644 000000 111111 generated-${index}.txt\n` + ) === true + ) { + return { stoppedEarly: true } + } + } + return { stoppedEarly: false } + }) - const result = await getStatusOp(git, { worktreePath: tmpDir, limit: 10_000 }) + const result = await getStatusOp(git, streamGit, { worktreePath: tmpDir }) expect(result.didHitLimit).toBe(true) - expect(result.statusLength).toBe(LARGE_STATUS_ENTRY_COUNT) - expect(result.entries).toHaveLength(10_000) + expect(result.statusLength).toBe(DEFAULT_GIT_STATUS_LIMIT + 1) + expect(result.entries).toHaveLength(DEFAULT_GIT_STATUS_LIMIT) + expect(emittedEntries).toBe(DEFAULT_GIT_STATUS_LIMIT + 1) + expect(streamGit).toHaveBeenCalledWith( + expect.arrayContaining(['status', '--porcelain=v2']), + tmpDir, + expect.objectContaining({ disableOptionalLocks: true }) + ) expect(result.entries[0]).toEqual({ path: 'generated-0.txt', status: 'added', @@ -75,12 +100,90 @@ describe('getStatusOp', () => { throw new Error(`Unexpected git command: ${args.join(' ')}`) }) - const result = await getStatusOp(git, { worktreePath: tmpDir, limit: 10_000 }) + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + limit: 10_000 + }) expect(result.didHitLimit).toBeUndefined() expect(result.entries).toHaveLength(5) }) + it('returns exactly the cap without a false limit signal', async () => { + const git = vi.fn(async (args) => { + if (args.includes('status')) { + return { stdout: buildLargeStatusOutput(3), stderr: '' } + } + if (args.includes('diff')) { + return { stdout: '', stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + limit: 3 + }) + + expect(result.entries).toHaveLength(3) + expect(result.didHitLimit).toBeUndefined() + expect(result.statusLength).toBeUndefined() + }) + + it('caps unmerged conflicts and keeps the visible conflict rows', async () => { + const lines = [ + 'u UU S... 160000 160000 160000 160000 aa bb cc vendor/submodule', + ...Array.from( + { length: 3 }, + (_, i) => `u UU N... 100644 100644 100644 100644 aa bb cc conflict-${i}.ts` + ) + ].join('\n') + const git = vi.fn(async (args) => { + if (args.includes('status')) { + return { stdout: `${lines}\n`, stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + limit: 2 + }) + + expect(result.didHitLimit).toBe(true) + expect(result.statusLength).toBe(3) + expect(result.entries).toHaveLength(2) + expect(result.entries.map((entry) => entry.path)).toEqual(['conflict-0.ts', 'conflict-1.ts']) + expect(result.entries.every((entry) => entry.conflictStatus === 'unresolved')).toBe(true) + expect(git).toHaveBeenCalledTimes(1) + }) + + it('keeps an early conflict ahead of later ordinary rows at the cap', async () => { + const lines = [ + '? before.ts', + 'u UU N... 100644 100644 100644 100644 aa bb cc conflict.ts', + '? after.ts' + ].join('\n') + const git = vi.fn(async (args) => { + if (args.includes('status')) { + return { stdout: `${lines}\n`, stderr: '' } + } + throw new Error(`Unexpected git command: ${args.join(' ')}`) + }) + + const result = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + limit: 2 + }) + + expect(result.didHitLimit).toBe(true) + expect(result.entries.map((entry) => entry.path)).toEqual(['before.ts', 'conflict.ts']) + expect(result.entries[1]).toMatchObject({ + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + }) + }) + it('reuses unchanged line stats only for hinted safety reads', async () => { const statusOutput = `${buildBranchStatusOutput('head-1', '(detached)')}\n1 .M N... 100644 100644 100644 aaaa aaaa src/a.ts` const git = vi.fn(async (args) => { @@ -93,9 +196,12 @@ describe('getStatusOp', () => { throw new Error(`Unexpected git command: ${args.join(' ')}`) }) - await getStatusOp(git, { worktreePath: tmpDir }) - const reused = await getStatusOp(git, { worktreePath: tmpDir, reuseLineStats: true }) - await getStatusOp(git, { worktreePath: tmpDir }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) + const reused = await getStatusOp(git, streamGitFromCapture(git), { + worktreePath: tmpDir, + reuseLineStats: true + }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) expect(reused.entries).toContainEqual( expect.objectContaining({ path: 'src/a.ts', added: 3, removed: 2 }) @@ -118,7 +224,12 @@ describe('getStatusOp', () => { throw new Error(`Unexpected git command: ${args.join(' ')}`) }) - await getStatusOp(git, { worktreePath: tmpDir }, { signal: controller.signal }) + await getStatusOp( + git, + streamGitFromCapture(git), + { worktreePath: tmpDir }, + { signal: controller.signal } + ) expect(git.mock.calls).not.toHaveLength(0) for (const [, , options] of git.mock.calls) { @@ -140,9 +251,9 @@ describe('getStatusOp', () => { throw new Error(`No upstream fixture for git ${args.join(' ')}`) }) - const first = await getStatusOp(git, { worktreePath: tmpDir }) + const first = await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) const firstCallCount = git.mock.calls.length - const second = await getStatusOp(git, { worktreePath: tmpDir }) + const second = await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) expect(first.upstreamStatus).toEqual({ hasUpstream: false, ahead: 0, behind: 0 }) expect(second.upstreamStatus).toEqual(first.upstreamStatus) @@ -176,9 +287,9 @@ describe('getStatusOp', () => { throw new Error(`No upstream fixture for git ${args.join(' ')}`) }) - await getStatusOp(git, { worktreePath: tmpDir }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) vi.setSystemTime(31_000) - await getStatusOp(git, { worktreePath: tmpDir }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) expect( git.mock.calls.filter(([args]) => args[0] === 'rev-parse' && args.includes('HEAD@{u}')) @@ -205,9 +316,9 @@ describe('getStatusOp', () => { }) await Promise.all([ - getStatusOp(git, { worktreePath: tmpDir }), - getStatusOp(git, { worktreePath: tmpDir }), - getStatusOp(git, { worktreePath: tmpDir }) + getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }), + getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }), + getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) ]) expect( @@ -238,9 +349,9 @@ describe('getStatusOp', () => { throw new Error(`No upstream fixture for git ${args.join(' ')}`) }) - await getStatusOp(git, { worktreePath: tmpDir }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) branch = 'other-feature' - await getStatusOp(git, { worktreePath: tmpDir }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) expect( git.mock.calls @@ -284,8 +395,8 @@ describe('getStatusOp', () => { throw new Error(`No upstream fixture for git ${args.join(' ')}`) }) - await getStatusOp(git, { worktreePath: tmpDir }) - await getStatusOp(git, { worktreePath: tmpDir }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) + await getStatusOp(git, streamGitFromCapture(git), { worktreePath: tmpDir }) expect( git.mock.calls.filter(([args]) => args[0] === 'rev-parse' && args.includes('HEAD@{u}')) diff --git a/src/relay/git-handler-status-ops.ts b/src/relay/git-handler-status-ops.ts index 375013181..1bd05821f 100644 --- a/src/relay/git-handler-status-ops.ts +++ b/src/relay/git-handler-status-ops.ts @@ -6,9 +6,10 @@ import * as path from 'node:path' import { existsSync } from 'node:fs' import { readFile } from 'node:fs/promises' import { parseUnmergedEntry } from './git-handler-utils' -import { parseStatusOutput } from './git-status-output-parser' import type { GitExec } from './git-handler-ops' +import type { RelayGitStreamExec } from './git-stdout-stream' import type { GitUpstreamStatus } from '../shared/types' +import { StatusPorcelainParser } from '../shared/git-status-porcelain-parser' import { splitRemoteBranchName } from '../shared/git-effective-upstream' import { readOrProbeNoEffectiveUpstreamStatus } from './git-status-upstream-negative-cache' import { @@ -17,7 +18,7 @@ import { parseNumstat, type GitLineStats } from '../shared/git-uncommitted-line-stats' -import { DEFAULT_GIT_STATUS_LIMIT } from '../shared/git-status-limit' +import { resolveGitStatusLimit } from '../shared/git-status-limit' import { beginGitStatusLineStatsCacheWrite, clearGitStatusLineStatsCacheKey, @@ -61,6 +62,7 @@ export async function detectConflictOperation(worktreePath: string): Promise, options: { signal?: AbortSignal } = {} ): Promise<{ @@ -78,11 +80,7 @@ export async function getStatusOp( const lineStatsWriteToken = beginGitStatusLineStatsCacheWrite(lineStatsCacheKey) const includeIgnored = params.includeIgnored === true // Why: reject NaN/negative limits — NaN would silently disable capping, negatives would over-truncate. - const rawLimit = params.limit - const limit = - typeof rawLimit === 'number' && Number.isFinite(rawLimit) && rawLimit >= 0 - ? Math.floor(rawLimit) - : DEFAULT_GIT_STATUS_LIMIT + const limit = resolveGitStatusLimit(params.limit) const conflictOperation = await detectConflictOperation(worktreePath) const entries: Record[] = [] let head: string | undefined @@ -105,28 +103,30 @@ export async function getStatusOp( if (includeIgnored) { statusArgs.push('--ignored=matching') } - const { stdout } = await git(statusArgs, worktreePath, { + const parser = new StatusPorcelainParser() + const { stoppedEarly } = await streamGit(statusArgs, worktreePath, { // Why: status polling is read-like; avoid racing terminal Git on .git/worktrees/*/index.lock. disableOptionalLocks: true, - signal: options.signal + signal: options.signal, + onStdout: (chunk) => parser.update(chunk, limit) }) - const parsed = parseStatusOutput(stdout) - head = parsed.head - branch = parsed.branch - upstreamStatus = parsed.upstreamStatus - ignoredPaths = parsed.ignoredPaths - statusLength = parsed.entries.length - // Why: cap entry count so an enormous un-ignored folder can't push tens of thousands of rows through every poll. - if (limit !== 0 && parsed.entries.length > limit) { - didHitLimit = true - for (let i = 0; i < limit; i++) { - entries.push(parsed.entries[i]) - } - } else { - for (const entry of parsed.entries) { - entries.push(entry) - } + if (!stoppedEarly) { + parser.finish() } + head = parser.branch.head + branch = parser.branch.branch + ignoredPaths = parser.ignoredPaths + statusLength = parser.statusLength + didHitLimit = stoppedEarly + const { upstreamName, upstreamAheadBehind } = parser.branch + upstreamStatus = upstreamName + ? { + hasUpstream: true, + upstreamName, + ahead: upstreamAheadBehind?.ahead ?? 0, + behind: upstreamAheadBehind?.behind ?? 0 + } + : { hasUpstream: false, ahead: 0, behind: 0 } if (!didHitLimit) { if (shouldProbeEffectiveUpstreamStatus(branch, upstreamStatus?.upstreamName)) { @@ -146,9 +146,18 @@ export async function getStatusOp( } } } + } - for (const uLine of parsed.unmergedLines) { - const entry = parseUnmergedEntry(worktreePath, uLine) + // Why: resolve deferred conflicts in Git's output order so the cap cannot hide + // an early conflict behind ordinary rows that appeared later in the stream. + for (const record of parser.statusRecords) { + if (didHitLimit && entries.length >= limit) { + break + } + if (record.type === 'entry') { + entries.push(record.entry as Record) + } else { + const entry = parseUnmergedEntry(worktreePath, record.line) if (entry) { entries.push(entry) } diff --git a/src/relay/git-handler-submodule-status-cancellation.test.ts b/src/relay/git-handler-submodule-status-cancellation.test.ts new file mode 100644 index 000000000..316a2d4ef --- /dev/null +++ b/src/relay/git-handler-submodule-status-cancellation.test.ts @@ -0,0 +1,42 @@ +import { mkdtempSync } from 'node:fs' +import { rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { RelayContext } from './context' +import { GitHandler } from './git-handler' +import { + createMockDispatcher, + type MockDispatcher, + type RelayDispatcher +} from './git-handler-test-setup' + +describe('GitHandler submodule status cancellation', () => { + let dispatcher: MockDispatcher + let handler: GitHandler + let worktreePath: string + + beforeEach(() => { + worktreePath = mkdtempSync(join(tmpdir(), 'relay-submodule-status-cancel-')) + dispatcher = createMockDispatcher() + handler = new GitHandler(dispatcher as unknown as RelayDispatcher, new RelayContext()) + }) + + afterEach(async () => { + handler.dispose() + await rm(worktreePath, { recursive: true, force: true }) + }) + + it('rejects a cancelled request before spawning submodule Git work', async () => { + const controller = new AbortController() + controller.abort() + + await expect( + dispatcher.callRequest( + 'git.submoduleStatus', + { worktreePath, submodulePath: 'vendor/library' }, + { isStale: () => false, signal: controller.signal } + ) + ).rejects.toMatchObject({ name: 'AbortError' }) + }) +}) diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index b62759452..54dfa86a6 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -15,7 +15,8 @@ import { computeDiff, branchCompare as branchCompareOp, branchDiffEntries, - validateGitExecArgs + validateGitExecArgs, + type GitExec } from './git-handler-ops' import { buildSubmoduleInnerCommitRangeDiff, @@ -42,6 +43,7 @@ import { forceDeletePreservedRelayBranch } from './git-handler-branch-cleanup' import { refreshLocalBaseRefForWorktreeCreateOp } from './git-handler-local-base-ref-refresh' import { gitExecMutatesRepository } from '../shared/git-exec-mutation' import { detectConflictOperation, getStatusOp } from './git-handler-status-ops' +import { capGitStatusEntries, resolveGitStatusLimit } from '../shared/git-status-limit' import { checkIgnoredPathsOp } from './git-handler-check-ignore' import { resolveRelayPushTarget } from './git-handler-push-target' import { @@ -78,6 +80,7 @@ import { GitResponseStreamRegistry } from './git-response-stream' import { GIT_RESPONSE_STREAM_THRESHOLD } from './protocol' import { endSubprocessStdin } from '../shared/subprocess-stdin-write' import { clearGitStatusLineStatsCache } from '../shared/git-status-line-stats-cache' +import { streamRelayGitStdout } from './git-stdout-stream' const execFileAsync = promisify(execFile) const MAX_GIT_BUFFER = 10 * 1024 * 1024 @@ -189,7 +192,9 @@ export class GitHandler { private registerHandlers(): void { this.dispatcher.onRequest('git.status', (p, context) => this.getStatus(p, context)) - this.dispatcher.onRequest('git.submoduleStatus', (p) => this.getSubmoduleStatus(p)) + this.dispatcher.onRequest('git.submoduleStatus', (p, context) => + this.getSubmoduleStatus(p, context) + ) this.dispatcher.onRequest('git.checkIgnored', (p) => this.checkIgnored(p)) this.dispatcher.onRequest('git.history', (p) => this.history(p)) this.dispatcher.onRequest('git.commit', (p) => this.commit(p)) @@ -330,43 +335,55 @@ export class GitHandler { private async getStatus(params: Record, context: RequestContext) { this.gitDiffReadDedupe.clear() - return getStatusOp(this.git.bind(this), params, { signal: context.signal }) + return getStatusOp(this.git.bind(this), streamRelayGitStdout, params, { + signal: context.signal + }) } // Why: parent status lists one gitlink row per submodule; fetch inner per-file changes by running status inside the submodule's own worktree. - private async getSubmoduleStatus(params: Record) { + private async getSubmoduleStatus(params: Record, context: RequestContext) { const worktreePath = params.worktreePath as string const submodulePath = params.submodulePath as string const area = resolveSubmoduleStatusArea(params) const staged = area === 'staged' const resolved = resolveSubmoduleWorktreePath(worktreePath, submodulePath) - const workingResult = await getStatusOp(this.git.bind(this), { - ...params, - worktreePath: resolved - }) + const limit = resolveGitStatusLimit(params.limit) + // Why: staged expansion only represents HEAD→index; scanning the submodule worktree is wasted work. + const workingResult = staged + ? { entries: [], conflictOperation: 'unknown' } + : await getStatusOp( + this.git.bind(this), + streamRelayGitStdout, + { + ...params, + worktreePath: resolved + }, + { signal: context.signal } + ) + // Why: pointer/range probes are part of the same SSH request and must not outlive its cancellation. + const requestGit: GitExec = (args, cwd, options) => + this.git(args, cwd, { ...options, signal: context.signal }) // Why: a moved gitlink (clean worktree) has no uncommitted rows; surface files changed between recorded and checked-out commits so it isn't empty. const { fromOid, toOid } = await resolveSubmoduleCommitRange( - this.git.bind(this), + requestGit, worktreePath, submodulePath, staged ) if (fromOid && toOid && fromOid !== toOid) { - const rangeEntries = await computeSubmoduleRangeEntries( - this.git.bind(this), - resolved, - fromOid, - toOid - ) + const rangeEntries = await computeSubmoduleRangeEntries(requestGit, resolved, fromOid, toOid) if (staged) { - return { ...workingResult, entries: rangeEntries } + return { ...workingResult, ...capGitStatusEntries(rangeEntries, limit) } } const rangePaths = new Set(rangeEntries.map((entry) => entry.path)) const entries = [ ...rangeEntries, ...workingResult.entries.filter((entry) => !rangePaths.has(entry.path)) ] - return { ...workingResult, entries } + return { + ...workingResult, + ...capGitStatusEntries(entries, limit, workingResult) + } } if (staged) { return { ...workingResult, entries: [] } diff --git a/src/relay/git-stdout-stream.test.ts b/src/relay/git-stdout-stream.test.ts new file mode 100644 index 000000000..bc41c4deb --- /dev/null +++ b/src/relay/git-stdout-stream.test.ts @@ -0,0 +1,136 @@ +import { EventEmitter } from 'node:events' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { spawnMock, terminateMock } = vi.hoisted(() => ({ + spawnMock: vi.fn(), + terminateMock: vi.fn() +})) + +vi.mock('node:child_process', () => ({ spawn: spawnMock })) +vi.mock('./subprocess-tree-termination', () => ({ + terminateRelaySubprocessTree: terminateMock +})) + +import { streamRelayGitStdout } from './git-stdout-stream' + +type MockChild = EventEmitter & { + stdout: EventEmitter + stderr: EventEmitter + pid?: number +} + +function createChild(): MockChild { + const child = new EventEmitter() as MockChild + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + child.pid = 1234 + return child +} + +describe('streamRelayGitStdout', () => { + beforeEach(() => { + spawnMock.mockReset() + terminateMock.mockReset() + }) + + it('decodes split UTF-8 chunks and stops the child at the parser limit', async () => { + const child = createChild() + spawnMock.mockReturnValue(child) + let output = '' + const pending = streamRelayGitStdout(['status', '--porcelain=v2'], '/repo', { + disableOptionalLocks: true, + onStdout: (chunk) => { + output += chunk + return output.includes('\n') + } + }) + const bytes = Buffer.from('? café-😀.txt\n') + const emojiStart = bytes.indexOf(Buffer.from('😀')) + child.stdout.emit('data', bytes.subarray(0, emojiStart + 2)) + child.stdout.emit('data', bytes.subarray(emojiStart + 2)) + + await expect(pending).resolves.toEqual({ stoppedEarly: true }) + expect(output).toBe('? café-😀.txt\n') + expect(terminateMock).toHaveBeenCalledWith(child) + expect(spawnMock).toHaveBeenCalledWith( + 'git', + ['status', '--porcelain=v2'], + expect.objectContaining({ + cwd: '/repo', + env: expect.objectContaining({ GIT_OPTIONAL_LOCKS: '0' }), + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true + }) + ) + expect(child.stdout.listenerCount('data')).toBe(0) + }) + + it('rejects parser failures after terminating and detaching the child', async () => { + const child = createChild() + spawnMock.mockReturnValue(child) + const pending = streamRelayGitStdout(['status'], '/repo', { + onStdout: () => { + throw new Error('parser failed') + } + }) + const rejection = expect(pending).rejects.toThrow('parser failed') + child.stdout.emit('data', Buffer.from('? file.ts\n')) + + await rejection + expect(terminateMock).toHaveBeenCalledWith(child) + expect(child.stdout.listenerCount('data')).toBe(0) + expect(child.stderr.listenerCount('data')).toBe(0) + expect(child.listenerCount('close')).toBe(0) + }) + + it('aborts an in-flight child and rejects instead of returning partial status', async () => { + const child = createChild() + spawnMock.mockReturnValue(child) + const controller = new AbortController() + const pending = streamRelayGitStdout(['status'], '/repo', { + signal: controller.signal, + onStdout: () => {} + }) + const rejection = expect(pending).rejects.toMatchObject({ name: 'AbortError' }) + controller.abort() + + await rejection + expect(terminateMock).toHaveBeenCalledWith(child) + expect(child.listenerCount('error')).toBe(0) + }) + + it('handles a late spawn error after abort cleanup', async () => { + const child = createChild() + child.pid = undefined + spawnMock.mockReturnValue(child) + const controller = new AbortController() + const pending = streamRelayGitStdout(['status'], '/repo', { + signal: controller.signal, + onStdout: () => {} + }) + const rejection = expect(pending).rejects.toMatchObject({ name: 'AbortError' }) + + controller.abort() + await rejection + + expect(() => child.emit('error', new Error('spawn git ENOENT'))).not.toThrow() + expect(child.listenerCount('error')).toBe(0) + }) + + it('bounds stderr and cleans up after command failure', async () => { + const child = createChild() + spawnMock.mockReturnValue(child) + const pending = streamRelayGitStdout(['status'], '/repo', { + maxBuffer: 64, + onStdout: () => {} + }) + const rejection = expect(pending).rejects.toThrow('git exited with 128: fatal: nope') + child.stderr.emit('data', Buffer.from('fatal: nope')) + child.emit('close', 128) + + await rejection + expect(terminateMock).not.toHaveBeenCalled() + expect(child.stdout.listenerCount('data')).toBe(0) + expect(child.stderr.listenerCount('data')).toBe(0) + }) +}) diff --git a/src/relay/git-stdout-stream.ts b/src/relay/git-stdout-stream.ts new file mode 100644 index 000000000..2118af143 --- /dev/null +++ b/src/relay/git-stdout-stream.ts @@ -0,0 +1,148 @@ +import { spawn } from 'node:child_process' +import { StringDecoder } from 'node:string_decoder' +import { expandTilde } from './context' +import { buildRelayGitEnv } from './relay-command-env' +import { terminateRelaySubprocessTree } from './subprocess-tree-termination' + +const DEFAULT_RELAY_GIT_STREAM_MAX_BYTES = 10 * 1024 * 1024 + +export type RelayGitStreamOptions = { + disableOptionalLocks?: boolean + signal?: AbortSignal + maxBuffer?: number + onStdout: (chunk: string) => boolean | void +} + +export type RelayGitStreamExec = ( + args: string[], + cwd: string, + options: RelayGitStreamOptions +) => Promise<{ stoppedEarly: boolean }> + +function createAbortError(): Error { + const error = new Error('The operation was aborted.') + error.name = 'AbortError' + return error +} + +/** Stream Git stdout on the relay host and allow the consumer to stop it early. */ +export const streamRelayGitStdout: RelayGitStreamExec = async (args, cwd, options) => { + const maxBuffer = options.maxBuffer ?? DEFAULT_RELAY_GIT_STREAM_MAX_BYTES + return new Promise((resolve, reject) => { + if (options.signal?.aborted) { + reject(createAbortError()) + return + } + + const env = buildRelayGitEnv() + if (options.disableOptionalLocks) { + env.GIT_OPTIONAL_LOCKS = '0' + } + + let child + try { + child = spawn('git', args, { + cwd: expandTilde(cwd), + env, + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true + }) + } catch (error) { + reject(error instanceof Error ? error : new Error(String(error))) + return + } + + let settled = false + let stoppedEarly = false + let stdoutBytes = 0 + let stderrBytes = 0 + let stderr = '' + // Why: filenames may contain UTF-8 characters split across stream chunks; + // stateful decoding keeps the porcelain record intact. + const stdoutDecoder = new StringDecoder('utf8') + const stderrDecoder = new StringDecoder('utf8') + + const cleanup = (): void => { + child.stdout.off('data', onStdoutData) + child.stderr.off('data', onStderrData) + child.off('error', onError) + child.off('close', onClose) + options.signal?.removeEventListener('abort', onAbort) + stdoutDecoder.end() + stderrDecoder.end() + } + const finish = (error?: Error): void => { + if (settled) { + return + } + settled = true + cleanup() + if (error) { + reject(Object.assign(error, { stderr })) + } else { + resolve({ stoppedEarly }) + } + } + const stopWithError = (error: Error): void => { + terminateRelaySubprocessTree(child) + finish(error) + } + + function onStdoutData(chunk: Buffer): void { + stdoutBytes += chunk.byteLength + if (stdoutBytes > maxBuffer) { + stopWithError(new Error('git stdout exceeded maxBuffer.')) + return + } + const decoded = stdoutDecoder.write(chunk) + if (!decoded) { + return + } + try { + if (options.onStdout(decoded) === true) { + // Why: the status cap is a successful partial result, so detach and + // resolve immediately after stopping Git instead of awaiting close. + stoppedEarly = true + terminateRelaySubprocessTree(child) + finish() + } + } catch (error) { + stopWithError(error instanceof Error ? error : new Error(String(error))) + } + } + function onStderrData(chunk: Buffer): void { + stderrBytes += chunk.byteLength + if (stderrBytes > maxBuffer) { + stopWithError(new Error('git stderr exceeded maxBuffer.')) + return + } + stderr += stderrDecoder.write(chunk) + } + function onError(error: Error): void { + finish(error) + } + function onClose(code: number | null): void { + if (stoppedEarly || code === 0) { + finish() + } else { + finish(new Error(`git exited with ${code}: ${stderr}`)) + } + } + function onAbort(): void { + if (!child.pid) { + // Why: failed spawn reports ENOENT after abort cleanup; handle it so it cannot crash the relay. + child.once('error', () => {}) + } + stopWithError(createAbortError()) + } + + child.stdout.on('data', onStdoutData) + child.stderr.on('data', onStderrData) + child.on('error', onError) + child.on('close', onClose) + options.signal?.addEventListener('abort', onAbort, { once: true }) + if (options.signal?.aborted) { + onAbort() + } + }) +} diff --git a/src/relay/subprocess-tree-termination.test.ts b/src/relay/subprocess-tree-termination.test.ts new file mode 100644 index 000000000..58abe2017 --- /dev/null +++ b/src/relay/subprocess-tree-termination.test.ts @@ -0,0 +1,32 @@ +import * as ChildProcessModule from 'node:child_process' +import { afterEach, describe, expect, it, vi } from 'vitest' + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, execFile: vi.fn() } +}) + +import { terminateRelaySubprocessTree } from './subprocess-tree-termination' + +const originalPlatform = process.platform + +afterEach(() => { + Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform }) + vi.mocked(ChildProcessModule.execFile).mockReset() +}) + +describe('terminateRelaySubprocessTree', () => { + it('invokes Windows taskkill without a shell command', () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + const child = { pid: 12345, kill: vi.fn() } as unknown as ChildProcessModule.ChildProcess + + terminateRelaySubprocessTree(child) + + expect(ChildProcessModule.execFile).toHaveBeenCalledWith( + 'taskkill', + ['/pid', '12345', '/T', '/F'], + expect.any(Function) + ) + expect(child.kill).not.toHaveBeenCalled() + }) +}) diff --git a/src/relay/subprocess-tree-termination.ts b/src/relay/subprocess-tree-termination.ts new file mode 100644 index 000000000..85cbc001b --- /dev/null +++ b/src/relay/subprocess-tree-termination.ts @@ -0,0 +1,21 @@ +import { execFile, type ChildProcess } from 'node:child_process' + +// Why: Windows commands may run through wrappers, so killing only the direct +// child can leave Git or an agent alive after cancellation. +export function terminateRelaySubprocessTree(child: ChildProcess): void { + const pid = child.pid + if (!pid) { + return + } + if (process.platform === 'win32') { + execFile('taskkill', ['/pid', String(pid), '/T', '/F'], () => { + // Best-effort; the child close listener owns completion. + }) + return + } + try { + child.kill('SIGKILL') + } catch { + // Child may already have exited between the kill request and now. + } +} diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 163af51ee..d3701b9fc 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -533,6 +533,7 @@ const SOURCE_CONTROL_ROW_ACTION_OVERLAY_CLASS = const SOURCE_CONTROL_TREE_INDENT_PX = 12 const SOURCE_CONTROL_TREE_DIRECTORY_PADDING_PX = 8 const SOURCE_CONTROL_TREE_FILE_PADDING_PX = 20 +const CAPPED_STATUS_RETRY_TIMEOUT_MS = 15_000 const EMPTY_GIT_HISTORY_STATE: GitHistoryPanelState = { status: 'idle' } const DEFAULT_COLLAPSED_SECTIONS = ['history'] as const const SUBMODULE_WORKTREE_ONLY_LABEL = 'Stage inside submodule' @@ -1246,36 +1247,40 @@ function SourceControlInner(): React.JSX.Element { // Why: the sidebar stays mounted when closed, so gate polling on tab AND open or branchCompare/PR fetch would run with no visible consumer. const isBranchVisible = rightSidebarTab === 'source-control' && rightSidebarOpen - const refreshActiveGitStatus = useCallback(async (): Promise => { - if (!activeWorktreeId || !worktreePath || isFolder) { - return - } - const connectionId = getConnectionId(activeWorktreeId) ?? undefined - await refreshGitStatusForWorktree({ - // Why: route git status by the repo OWNER host, not the focused runtime. - settings: activeRepoSettings, - worktreeId: activeWorktreeId, - worktreePath, - connectionId, - pushTarget: activeWorktree?.pushTarget, - deps: { - setGitStatus, - updateWorktreeGitIdentity, - setUpstreamStatus, - fetchUpstreamStatus + const refreshActiveGitStatus = useCallback( + async (signal?: AbortSignal): Promise => { + if (!activeWorktreeId || !worktreePath || isFolder) { + return } - }) - }, [ - activeRepoSettings, - activeWorktreeId, - activeWorktree?.pushTarget, - fetchUpstreamStatus, - isFolder, - setGitStatus, - setUpstreamStatus, - updateWorktreeGitIdentity, - worktreePath - ]) + const connectionId = getConnectionId(activeWorktreeId) ?? undefined + await refreshGitStatusForWorktree({ + // Why: route git status by the repo OWNER host, not the focused runtime. + settings: activeRepoSettings, + worktreeId: activeWorktreeId, + worktreePath, + connectionId, + pushTarget: activeWorktree?.pushTarget, + deps: { + setGitStatus, + updateWorktreeGitIdentity, + setUpstreamStatus, + fetchUpstreamStatus + }, + ...(signal ? { request: { signal } } : {}) + }) + }, + [ + activeRepoSettings, + activeWorktreeId, + activeWorktree?.pushTarget, + fetchUpstreamStatus, + isFolder, + setGitStatus, + setUpstreamStatus, + updateWorktreeGitIdentity, + worktreePath + ] + ) const refreshActiveGitStatusAfterMutation = useCallback(async (): Promise => { try { @@ -5657,7 +5662,12 @@ function SourceControlInner(): React.JSX.Element { {repositoryHuge && (
- + {/* Why: a slow SSH retry must not keep the next worktree's Retry disabled after navigation. */} +
)} @@ -7472,18 +7482,89 @@ export function OperationBanner({ ) } -export function TooManyChangesBanner({ limit }: { limit: number }): React.JSX.Element { +export function TooManyChangesBanner({ + limit, + onRetry +}: { + limit: number + onRetry: (signal: AbortSignal) => Promise +}): React.JSX.Element { + const [isRetrying, setIsRetrying] = useState(false) + const [showSpinner, setShowSpinner] = useState(false) + const retryControllerRef = useRef(null) + const isMountedRef = useRef(false) + useEffect(() => { + isMountedRef.current = true + return () => { + isMountedRef.current = false + retryControllerRef.current?.abort() + } + }, []) + useEffect(() => { + if (!isRetrying) { + setShowSpinner(false) + return + } + const timer = window.setTimeout(() => setShowSpinner(true), 1_000) + return () => window.clearTimeout(timer) + }, [isRetrying]) + + const handleRetry = async (): Promise => { + if (isRetrying) { + return + } + const controller = new AbortController() + retryControllerRef.current = controller + const timeout = window.setTimeout(() => controller.abort(), CAPPED_STATUS_RETRY_TIMEOUT_MS) + setIsRetrying(true) + try { + await onRetry(controller.signal) + } catch (error) { + if (!isMountedRef.current) { + return + } + // Why: a failed local/SSH retry must leave the capped warning usable + // instead of becoming an unhandled click rejection. + console.warn('[SourceControl] capped status retry failed', error) + toast.error( + translate( + 'auto.components.right.sidebar.SourceControl.97e7124eac', + 'Could not refresh Source Control. Try again.' + ) + ) + } finally { + window.clearTimeout(timeout) + if (retryControllerRef.current === controller) { + retryControllerRef.current = null + } + if (isMountedRef.current) { + setIsRetrying(false) + } + } + } + return (
- + {translate( 'auto.components.right.sidebar.SourceControl.tooManyChanges', 'Too many changes detected. Only the first {{value0}} are shown.', { value0: limit.toLocaleString() } )} +
) @@ -7665,7 +7746,7 @@ function SubmodulePlaceholderRow({ message }: { depth: number - state: 'loading' | 'empty' | 'error' + state: 'loading' | 'empty' | 'error' | 'truncated' message?: string }): React.JSX.Element { const fallback = @@ -7673,7 +7754,12 @@ function SubmodulePlaceholderRow({ ? SUBMODULE_ERROR_LABEL : state === 'empty' ? SUBMODULE_EMPTY_LABEL - : SUBMODULE_LOADING_LABEL + : state === 'truncated' + ? translate( + 'auto.components.right.sidebar.SourceControl.submoduleTruncated', + 'More submodule changes were omitted' + ) + : SUBMODULE_LOADING_LABEL return (
{ expect(child.entry.submoduleRoot).toBe('flutter_mine') }) + it('shows that capped tree results omit additional submodule changes', () => { + const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) + const statuses: Record = { + [FLUTTER_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }], + didHitLimit: true + } + } + + const result = injectExpandedSubmoduleRows( + [node], + new Set([FLUTTER_KEY]), + statuses, + LOADING, + EMPTY + ) + + expect(result.at(-1)).toMatchObject({ + type: 'submodule-placeholder', + state: 'truncated', + submodulePath: 'flutter_mine' + }) + }) + it('keeps inner staged rows staged for tree-view diff routing', () => { const node = fileNode(submoduleEntry({ path: 'flutter_mine' })) const statuses: Record = { @@ -355,6 +380,31 @@ describe('injectExpandedSubmoduleEntries (list view)', () => { message: EMPTY }) }) + + it('shows that capped list results omit additional submodule changes', () => { + const entry = submoduleEntry({ path: 'flutter_mine' }) + const statuses: Record = { + [FLUTTER_KEY]: { + status: 'loaded', + entries: [{ path: 'lib/main.dart', status: 'modified', area: 'unstaged' }], + didHitLimit: true + } + } + + const result = injectExpandedSubmoduleEntries( + [entry], + new Set([FLUTTER_KEY]), + statuses, + LOADING, + EMPTY + ) + + expect(result.at(-1)).toMatchObject({ + type: 'submodule-placeholder', + state: 'truncated', + submodulePath: 'flutter_mine' + }) + }) }) describe('collectListSelectionEntries', () => { diff --git a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts index 0cd0ca92c..aab153605 100644 --- a/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts +++ b/src/renderer/src/components/right-sidebar/source-control-submodule-expansion.ts @@ -19,7 +19,7 @@ export type SubmodulePlaceholderNode = { key: string submodulePath: string depth: number - state: 'loading' | 'empty' | 'error' + state: 'loading' | 'empty' | 'error' | 'truncated' message?: string } @@ -27,7 +27,7 @@ export type RenderableSourceControlNode = SubmoduleSectionTreeNode | SubmodulePl export type SubmoduleStatusState = | { status: 'loading' } - | { status: 'loaded'; entries: GitStatusEntry[] } + | { status: 'loaded'; entries: GitStatusEntry[]; didHitLimit?: boolean } | { status: 'error'; error: string } export function getSubmoduleExpansionKey(entry: Pick): string { @@ -176,6 +176,15 @@ export function injectExpandedSubmoduleEntries( entry: buildSubmoduleChildEntry(submodulePath, innerEntry, entry.area) }) } + if (state.didHitLimit) { + result.push({ + type: 'submodule-placeholder', + key: `submodule-truncated::${entry.area}::${submodulePath}`, + submodulePath, + depth: 1, + state: 'truncated' + }) + } } return result } @@ -262,6 +271,15 @@ export function injectExpandedSubmoduleRows( for (const childNode of buildSubmoduleChildNodes(node, state.entries)) { result.push(childNode) } + if (state.didHitLimit) { + result.push({ + type: 'submodule-placeholder', + key: `submodule-truncated::${node.area}::${submodulePath}`, + submodulePath, + depth: node.depth + 1, + state: 'truncated' + }) + } } return result } diff --git a/src/renderer/src/components/right-sidebar/source-control-too-many-changes-banner.test.tsx b/src/renderer/src/components/right-sidebar/source-control-too-many-changes-banner.test.tsx new file mode 100644 index 000000000..19a25ba50 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-too-many-changes-banner.test.tsx @@ -0,0 +1,62 @@ +// @vitest-environment happy-dom + +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { TooManyChangesBanner } from './SourceControl' + +const { toastErrorMock } = vi.hoisted(() => ({ toastErrorMock: vi.fn() })) + +vi.mock('sonner', () => ({ toast: { error: toastErrorMock } })) + +describe('TooManyChangesBanner', () => { + beforeEach(() => { + toastErrorMock.mockReset() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('aborts stale retry work when the banner unmounts', async () => { + let retrySignal: AbortSignal | undefined + const onRetry = vi.fn( + (signal: AbortSignal) => + new Promise((_resolve, reject) => { + retrySignal = signal + signal.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) + ) + const view = render() + + fireEvent.click(screen.getByRole('button', { name: 'Retry' })) + expect(onRetry).toHaveBeenCalledTimes(1) + expect(retrySignal?.aborted).toBe(false) + + view.unmount() + + await waitFor(() => expect(retrySignal?.aborted).toBe(true)) + expect(toastErrorMock).not.toHaveBeenCalled() + }) + + it('bounds a hung retry and restores the action', async () => { + vi.useFakeTimers() + const onRetry = vi.fn( + (signal: AbortSignal) => + new Promise((_resolve, reject) => { + signal.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) + ) + render() + + const retryButton = screen.getByRole('button', { name: 'Retry' }) + fireEvent.click(retryButton) + expect((retryButton as HTMLButtonElement).disabled).toBe(true) + + await act(async () => { + await vi.advanceTimersByTimeAsync(15_000) + }) + + expect((retryButton as HTMLButtonElement).disabled).toBe(false) + expect(toastErrorMock).toHaveBeenCalledWith('Could not refresh Source Control. Try again.') + }) +}) diff --git a/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts b/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts index 9b226fb01..47d8d5e8f 100644 --- a/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts +++ b/src/renderer/src/components/right-sidebar/useGitStatusPolling.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type * as React from 'react' import type { FsChangedPayload, GitPushTarget, GitStatusResult } from '../../../../shared/types' +import { DEFAULT_GIT_STATUS_LIMIT } from '../../../../shared/git-status-limit' const worktree = { id: 'repo-1::/repo', repoId: 'repo-1', path: '/repo' } const repo = { id: 'repo-1', path: '/repo', kind: 'git', connectionId: null as string | null } @@ -671,7 +672,7 @@ describe('useGitStatusPolling', () => { { expectStatusCall: false, stateOverrides: { - gitStatusHugeByWorktree: { [worktree.id]: { limit: 10_000 } } + gitStatusHugeByWorktree: { [worktree.id]: { limit: DEFAULT_GIT_STATUS_LIMIT } } } } ) @@ -711,7 +712,7 @@ describe('useGitStatusPolling', () => { expectStatusCall: false, documentStub: visibilityDocument, stateOverrides: { - gitStatusHugeByWorktree: { [worktree.id]: { limit: 10_000 } } + gitStatusHugeByWorktree: { [worktree.id]: { limit: DEFAULT_GIT_STATUS_LIMIT } } } } ) diff --git a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx index 6df07a368..5b97bb806 100644 --- a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.test.tsx @@ -231,7 +231,10 @@ describe('useSourceControlSubmoduleStatus', () => { }) it('passes the row area when expanding a staged submodule row', async () => { - mocks.getRuntimeGitSubmoduleStatus.mockResolvedValue({ entries: [innerEntry('from-index.ts')] }) + mocks.getRuntimeGitSubmoduleStatus.mockResolvedValue({ + entries: [innerEntry('from-index.ts')], + didHitLimit: true + }) const container = document.createElement('div') const root = createRoot(container) @@ -253,7 +256,8 @@ describe('useSourceControlSubmoduleStatus', () => { ) expect(latest?.submoduleStatusByKey['staged::sub']).toEqual({ status: 'loaded', - entries: [innerEntry('from-index.ts')] + entries: [innerEntry('from-index.ts')], + didHitLimit: true }) }) }) diff --git a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts index 971fbe760..fc91420b7 100644 --- a/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts +++ b/src/renderer/src/components/right-sidebar/useSourceControlSubmoduleStatus.ts @@ -87,7 +87,11 @@ export function useSourceControlSubmoduleStatus( } setSubmoduleStatusByKey((prev) => ({ ...prev, - [expansionKey]: { status: 'loaded', entries: result.entries } + [expansionKey]: { + status: 'loaded', + entries: result.entries, + ...(result.didHitLimit ? { didHitLimit: true } : {}) + } })) } catch (error) { if (generationRef.current !== generation) { diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 7cc4efd9e..9c34e4df4 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -9690,6 +9690,7 @@ "hugeRepoIgnorePrompt": "This repository has too many active changes. Add \"{{value0}}\" to .gitignore?", "hugeRepoIgnoreAction": "Add to .gitignore", "tooManyChanges": "Too many changes detected. Only the first {{value0}} changes are shown.", + "submoduleTruncated": "More submodule changes were omitted", "bf5082de46": "{{value0}} copied", "c06193ef57": "Failed to copy {{value0}}", "d172a4f068": "Commit hash", @@ -9745,7 +9746,8 @@ "a9bf7c171a": "Push Failed", "834cb3f23d": "Fix with AI", "783a808870": "Close" - } + }, + "97e7124eac": "Could not refresh Source Control. Try again." }, "SourceControlAgentActionDialog": { "8e856842d1": "Could not start the selected agent.", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index b5fe8fc4e..4311a1a9b 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -9667,6 +9667,7 @@ "hugeRepoIgnorePrompt": "Este repositorio tiene demasiados cambios activos. ¿Agregar \"{{value0}}\" a .gitignore?", "hugeRepoIgnoreAction": "Agregar a .gitignore", "tooManyChanges": "Se detectaron demasiados cambios. Solo se muestran los primeros {{value0}}.", + "submoduleTruncated": "Se omitieron más cambios del submódulo", "bf5082de46": "{{value0}} copiado", "c06193ef57": "No se pudo copiar {{value0}}", "d172a4f068": "Hash del commit", @@ -9722,7 +9723,8 @@ "a9bf7c171a": "Push fallido", "834cb3f23d": "Corregir con AI", "783a808870": "Cerrar" - } + }, + "97e7124eac": "No se pudo actualizar Source Control. Vuelve a intentarlo." }, "SourceControlAgentActionDialog": { "8e856842d1": "No se pudo iniciar el agente seleccionado.", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 20cc10366..891f1688b 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -9667,6 +9667,7 @@ "hugeRepoIgnorePrompt": "この repos にはアクティブな変更が多すぎます。「{{value0}}」を .gitignore に追加しますか？", "hugeRepoIgnoreAction": ".gitignore に追加", "tooManyChanges": "検出された変更が多すぎます。最初の {{value0}} 件のみ表示しています。", + "submoduleTruncated": "その他のサブモジュールの変更は省略されました", "bf5082de46": "{{value0}}をコピーしました", "c06193ef57": "{{value0}}をコピーできませんでした", "d172a4f068": "Commit ハッシュ", @@ -9722,7 +9723,8 @@ "a9bf7c171a": "プッシュ失敗", "834cb3f23d": "AIで修正", "783a808870": "閉じる" - } + }, + "97e7124eac": "Source Control を更新できませんでした。もう一度お試しください。" }, "SourceControlAgentActionDialog": { "8e856842d1": "選択した agent を開始できませんでした。", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index c4178fd90..b38075abb 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -9667,6 +9667,7 @@ "hugeRepoIgnorePrompt": "이 repos에 활성 변경 사항이 너무 많습니다. \"{{value0}}\"을(를) .gitignore에 추가하시겠습니까?", "hugeRepoIgnoreAction": ".gitignore에 추가", "tooManyChanges": "변경 사항이 너무 많이 감지되었습니다. 처음 {{value0}}개만 표시됩니다.", + "submoduleTruncated": "추가 서브모듈 변경 사항이 생략되었습니다", "bf5082de46": "{{value0}}이(가) 복사됨", "c06193ef57": "{{value0}}을(를) 복사하지 못했습니다", "d172a4f068": "Commit 해시", @@ -9722,7 +9723,8 @@ "a9bf7c171a": "푸시 실패", "834cb3f23d": "AI로 수정", "783a808870": "닫기" - } + }, + "97e7124eac": "Source Control을 새로 고칠 수 없습니다. 다시 시도하세요." }, "SourceControlAgentActionDialog": { "8e856842d1": "선택한 agent를 시작할 수 없습니다.", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 36fee14d9..29b59a747 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -9667,6 +9667,7 @@ "hugeRepoIgnorePrompt": "此存储库的活动更改过多。是否将 \"{{value0}}\" 添加到 .gitignore？", "hugeRepoIgnoreAction": "添加到 .gitignore", "tooManyChanges": "检测到过多更改。仅显示前 {{value0}} 项。", + "submoduleTruncated": "已省略更多子模块更改", "bf5082de46": "已复制{{value0}}", "c06193ef57": "无法复制{{value0}}", "d172a4f068": "提交哈希", @@ -9722,7 +9723,8 @@ "a9bf7c171a": "推送失败", "834cb3f23d": "使用 AI 修复", "783a808870": "关闭" - } + }, + "97e7124eac": "无法刷新 Source Control。请重试。" }, "SourceControlAgentActionDialog": { "8e856842d1": "无法启动选定的智能体。", diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index 638fc8cb6..42ec67db1 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -2234,6 +2234,69 @@ describe('createEditorSlice conflict status reconciliation', () => { expect(store.getState().gitStatusByWorktree['wt-clean']).toEqual([]) }) + it('keeps the capped state sticky until a complete result recovers', () => { + const store = createEditorStore() + store.getState().setGitStatus('wt-huge', { + conflictOperation: 'unknown', + entries: [{ path: 'generated/a.ts', status: 'untracked', area: 'untracked' }], + didHitLimit: true, + statusLength: 2 + }) + + expect(store.getState().gitStatusHugeByWorktree['wt-huge']).toEqual({ limit: 1 }) + + store.getState().setGitStatus('wt-huge', { + conflictOperation: 'unknown', + entries: [{ path: 'src/index.ts', status: 'modified', area: 'unstaged' }] + }) + + expect(store.getState().gitStatusHugeByWorktree['wt-huge']).toBeUndefined() + }) + + it('preserves omitted conflict state when a capped result is incomplete', () => { + const store = createEditorStore() + const conflict = { + path: 'src/conflict.ts', + status: 'modified' as const, + area: 'unstaged' as const, + conflictKind: 'both_modified' as const, + conflictStatus: 'unresolved' as const, + conflictStatusSource: 'git' as const + } + store.getState().trackConflictPath('wt-huge', conflict.path, conflict.conflictKind) + store.getState().openConflictFile('wt-huge', '/repo', conflict, 'typescript') + store.getState().setGitStatus('wt-huge', { + conflictOperation: 'merge', + entries: [conflict] + }) + + store.getState().setGitStatus('wt-huge', { + conflictOperation: 'unknown', + entries: [{ path: 'generated/a.ts', status: 'untracked', area: 'untracked' }], + didHitLimit: true, + statusLength: 2 + }) + + expect(store.getState().trackedConflictPathsByWorktree['wt-huge']).toEqual({ + 'src/conflict.ts': 'both_modified' + }) + expect(store.getState().gitConflictOperationByWorktree['wt-huge']).toBe('merge') + expect( + store.getState().openFiles.find((file) => file.relativePath === 'src/conflict.ts')?.conflict + ).toMatchObject({ + conflictKind: 'both_modified', + conflictStatus: 'unresolved' + }) + + store.getState().setGitStatus('wt-huge', { + conflictOperation: 'unknown', + entries: [] + }) + + expect(store.getState().trackedConflictPathsByWorktree['wt-huge']).toEqual({}) + expect(store.getState().gitConflictOperationByWorktree['wt-huge']).toBe('unknown') + }) + it('treats a blank git status HEAD as unknown without invalidating branch compare', () => { const store = createEditorStore() const summary = { diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 79f99f81f..fcfffeb64 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -3341,10 +3341,17 @@ export const createEditorSlice: StateCreator = (s (entry) => entry.conflictStatus === 'unresolved' && entry.conflictKind ) const unresolvedByPath = new Map(unresolvedEntries.map((entry) => [entry.path, entry])) + const statusIsComplete = status.didHitLimit !== true + // Why: a capped snapshot cannot prove that an omitted conflict operation ended. + const nextOperation = + !statusIsComplete && status.conflictOperation === 'unknown' + ? prevOperation + : status.conflictOperation // Why: operation → 'unknown' with zero unresolved means an abort (git merge --abort), not resolution; clear tracked paths instead of marking each "Resolved locally". if ( - status.conflictOperation === 'unknown' && + statusIsComplete && + nextOperation === 'unknown' && prevOperation !== 'unknown' && unresolvedByPath.size === 0 ) { @@ -3369,21 +3376,28 @@ export const createEditorSlice: StateCreator = (s } }) - const visiblePaths = new Set(nextEntries.map((entry) => entry.path)) - for (const path of Object.keys(currentTracked)) { - if (!visiblePaths.has(path) && !unresolvedByPath.has(path)) { - delete currentTracked[path] + if (statusIsComplete) { + const visiblePaths = new Set(nextEntries.map((entry) => entry.path)) + for (const path of Object.keys(currentTracked)) { + if (!visiblePaths.has(path) && !unresolvedByPath.has(path)) { + delete currentTracked[path] + } } } - const nextOpenFiles = reconcileOpenFilesForStatus(s.openFiles, worktreeId, nextEntries) + const nextOpenFiles = reconcileOpenFilesForStatus( + s.openFiles, + worktreeId, + nextEntries, + statusIsComplete + ) const statusUnchanged = hadStatusEntry && areGitStatusEntriesEqual(prevEntries, nextEntries) const trackedUnchanged = areTrackedConflictMapsEqual( s.trackedConflictPathsByWorktree[worktreeId] ?? {}, currentTracked ) const openFilesUnchanged = nextOpenFiles === s.openFiles - const operationUnchanged = prevOperation === status.conflictOperation + const operationUnchanged = prevOperation === nextOperation const prevIgnored = s.gitIgnoredPathsByWorktree[worktreeId] const nextIgnored = status.ignoredPaths ?? [] @@ -3461,7 +3475,7 @@ export const createEditorSlice: StateCreator = (s : { ...s.gitIgnoredPathsByWorktree, [worktreeId]: nextIgnored }, gitConflictOperationByWorktree: operationUnchanged ? s.gitConflictOperationByWorktree - : { ...s.gitConflictOperationByWorktree, [worktreeId]: status.conflictOperation }, + : { ...s.gitConflictOperationByWorktree, [worktreeId]: nextOperation }, trackedConflictPathsByWorktree: trackedUnchanged ? s.trackedConflictPathsByWorktree : { ...s.trackedConflictPathsByWorktree, [worktreeId]: currentTracked }, @@ -4411,7 +4425,8 @@ function areUpstreamStatusesEqual( function reconcileOpenFilesForStatus( openFiles: OpenFile[], worktreeId: string, - nextEntries: GitStatusEntry[] + nextEntries: GitStatusEntry[], + statusIsComplete: boolean ): OpenFile[] { const entriesByPath = new Map(nextEntries.map((entry) => [entry.path, entry])) let changed = false @@ -4430,6 +4445,11 @@ function reconcileOpenFilesForStatus( return [file] } + // Why: a capped snapshot cannot prove that an omitted conflict was resolved. + if (!entry && !statusIsComplete) { + return [file] + } + if (!entry || !entry.conflictKind || !entry.conflictStatus || !entry.conflictStatusSource) { changed = true return file.conflict.kind === 'conflict-placeholder' ? [] : [{ ...file, conflict: undefined }] diff --git a/src/shared/git-status-limit.test.ts b/src/shared/git-status-limit.test.ts new file mode 100644 index 000000000..64e333c18 --- /dev/null +++ b/src/shared/git-status-limit.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import { + DEFAULT_GIT_STATUS_LIMIT, + capGitStatusEntries, + resolveGitStatusLimit +} from './git-status-limit' + +describe('resolveGitStatusLimit', () => { + it('accepts non-negative integers and rejects malformed limits', () => { + expect(resolveGitStatusLimit(0)).toBe(0) + expect(resolveGitStatusLimit(25)).toBe(25) + expect(resolveGitStatusLimit(1.5)).toBe(DEFAULT_GIT_STATUS_LIMIT) + expect(resolveGitStatusLimit(Number.NaN)).toBe(DEFAULT_GIT_STATUS_LIMIT) + expect(resolveGitStatusLimit(-1)).toBe(DEFAULT_GIT_STATUS_LIMIT) + }) +}) + +describe('capGitStatusEntries', () => { + it('caps composed entries and reports the observed size', () => { + expect(capGitStatusEntries(['a', 'b', 'c'], 2)).toEqual({ + entries: ['a', 'b'], + didHitLimit: true, + statusLength: 3 + }) + }) + + it('preserves an earlier incomplete-status signal after deduplication', () => { + expect(capGitStatusEntries(['a'], 2, { didHitLimit: true, statusLength: 3 })).toEqual({ + entries: ['a'], + didHitLimit: true, + statusLength: 3 + }) + }) + + it('treats zero as an unlimited result', () => { + expect(capGitStatusEntries(['a', 'b'], 0)).toEqual({ entries: ['a', 'b'] }) + }) +}) diff --git a/src/shared/git-status-limit.ts b/src/shared/git-status-limit.ts index 244443e11..7ed7befa5 100644 --- a/src/shared/git-status-limit.ts +++ b/src/shared/git-status-limit.ts @@ -1,6 +1,28 @@ // Why: git status is capped at this many changed-file entries. A repo with an -// enormous un-ignored folder can otherwise emit a listing large enough to crash -// the process when buffered. When the cap is hit the source-control view shows a -// "too many changes" state instead of the full list. Shared so the local path, -// the relay/SSH path, and the renderer agree on the same threshold. -export const DEFAULT_GIT_STATUS_LIMIT = 10_000 +// enormous un-ignored folder can otherwise freeze the renderer while it builds +// the source-control projections. When the cap is hit the view shows a "too many +// changes" state instead of the full list. Shared so native, WSL, SSH, and the +// renderer agree on the same responsive threshold. +export const DEFAULT_GIT_STATUS_LIMIT = 1_000 + +export function resolveGitStatusLimit(value: unknown): number { + return typeof value === 'number' && Number.isInteger(value) && value >= 0 + ? value + : DEFAULT_GIT_STATUS_LIMIT +} + +export function capGitStatusEntries( + entries: T[], + limit: number, + previous: { didHitLimit?: boolean; statusLength?: number } = {} +): { entries: T[]; didHitLimit?: true; statusLength?: number } { + const exceededLimit = limit > 0 && entries.length > limit + if (!exceededLimit && previous.didHitLimit !== true) { + return { entries } + } + return { + entries: exceededLimit ? entries.slice(0, limit) : entries, + didHitLimit: true, + statusLength: Math.max(previous.statusLength ?? 0, entries.length) + } +} diff --git a/src/main/git/status-porcelain-parser.ts b/src/shared/git-status-porcelain-parser.ts similarity index 90% rename from src/main/git/status-porcelain-parser.ts rename to src/shared/git-status-porcelain-parser.ts index 3353275c0..3d9656015 100644 --- a/src/main/git/status-porcelain-parser.ts +++ b/src/shared/git-status-porcelain-parser.ts @@ -1,5 +1,5 @@ -import type { GitStatusEntry } from '../../shared/git-status-types' -import { decodeGitCQuotedPath } from '../../shared/git-cquoted-path' +import type { GitStatusEntry } from './git-status-types' +import { decodeGitCQuotedPath } from './git-cquoted-path' /** * Incremental parser for `git status --porcelain=v2 --branch` output. @@ -12,9 +12,9 @@ import { decodeGitCQuotedPath } from '../../shared/git-cquoted-path' * carried across chunks. * * Sync record types (1/2/?/!) are parsed into `entries`/`ignoredPaths` here. - * Unmerged (`u`) records need async per-file git lookups, so their raw lines are - * collected and resolved by the caller after the stream ends — they signal - * conflict states and are never the source of huge output. + * Unmerged (`u`) records need per-file worktree lookups, so their raw lines are + * collected and resolved by the caller after the stream ends. They still count + * toward the limit so a conflict-heavy merge cannot bypass the cap. */ export type BranchMetadata = { head?: string @@ -23,6 +23,10 @@ export type BranchMetadata = { upstreamAheadBehind?: { ahead: number; behind: number } } +export type StatusPorcelainRecord = + | { type: 'entry'; entry: GitStatusEntry } + | { type: 'unmerged'; line: string } + export class StatusPorcelainParser { private carry = '' /** Count of changed-file entries seen — the limit is measured against this. */ @@ -32,6 +36,8 @@ export class StatusPorcelainParser { readonly ignoredPaths: string[] = [] /** Raw `u ` lines for the caller to resolve asynchronously. */ readonly unmergedLines: string[] = [] + /** Changed records in Git's output order, including deferred unmerged rows. */ + readonly statusRecords: StatusPorcelainRecord[] = [] readonly branch: BranchMetadata = {} /** Total changed-file entries observed (including any past the limit). */ @@ -123,7 +129,9 @@ export class StatusPorcelainParser { return } if (line.startsWith('u ')) { + this.count += 1 this.unmergedLines.push(line) + this.statusRecords.push({ type: 'unmerged', line }) } } @@ -185,6 +193,7 @@ export class StatusPorcelainParser { private push(entry: GitStatusEntry): void { this.count += 1 this.entries.push(entry) + this.statusRecords.push({ type: 'entry', entry }) } } diff --git a/src/shared/git-uncommitted-line-stats.test.ts b/src/shared/git-uncommitted-line-stats.test.ts index 001e9ef9e..96556be88 100644 --- a/src/shared/git-uncommitted-line-stats.test.ts +++ b/src/shared/git-uncommitted-line-stats.test.ts @@ -13,6 +13,7 @@ import { MAX_UNTRACKED_LINE_COUNT_BYTES, parseNumstat } from './git-uncommitted-line-stats' +import { DEFAULT_GIT_STATUS_LIMIT } from './git-status-limit' function mockFileStat(size: number, mtimeMs = 1) { return { @@ -146,13 +147,15 @@ describe('collectUntrackedAdditions', () => { }) it('keeps the cache effective across polls for a status-limit-sized change set', async () => { - // Why: git status caps at DEFAULT_GIT_STATUS_LIMIT (10,000) entries. A - // cache smaller than one scan FIFO-evicts every entry mid-scan, so the + // Why: a cache smaller than the status cap FIFO-evicts every entry, so the // next poll re-reads every file (#8013). Scan the full limit twice; the // second pass must be stat-only. lstatMock.mockResolvedValue(mockFileStat(5, 7)) readFileMock.mockResolvedValue(Buffer.from('a\nb\nc')) - const paths = Array.from({ length: 10_000 }, (_, i) => `poll-scale/file-${i}.ts`) + const paths = Array.from( + { length: DEFAULT_GIT_STATUS_LIMIT }, + (_, i) => `poll-scale/file-${i}.ts` + ) await collectUntrackedAdditions('/repo', paths) const firstPassReads = readFileMock.mock.calls.length diff --git a/tests/e2e/large-file-count-fixtures.ts b/tests/e2e/large-file-count-fixtures.ts index f3411c02b..f9927ac3e 100644 --- a/tests/e2e/large-file-count-fixtures.ts +++ b/tests/e2e/large-file-count-fixtures.ts @@ -50,6 +50,10 @@ export async function removeLargeFileCountRepo(repoPath: string): Promise } } +export function removeLargeFileCountUntrackedTree(repoPath: string): void { + rmSync(path.join(repoPath, 'generated'), { recursive: true, force: true }) +} + function writeFileTree( repoPath: string, rootDirName: string, diff --git a/tests/e2e/source-control-large-file-count.spec.ts b/tests/e2e/source-control-large-file-count.spec.ts index c54a9651c..fb98eb9a1 100644 --- a/tests/e2e/source-control-large-file-count.spec.ts +++ b/tests/e2e/source-control-large-file-count.spec.ts @@ -20,7 +20,11 @@ import type { ElectronApplication, Page } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { waitForSessionReady } from './helpers/store' -import { createLargeFileCountRepo, removeLargeFileCountRepo } from './large-file-count-fixtures' +import { + createLargeFileCountRepo, + removeLargeFileCountRepo, + removeLargeFileCountUntrackedTree +} from './large-file-count-fixtures' import { DEFAULT_GIT_STATUS_LIMIT } from '../../src/shared/git-status-limit' // Matches the large-diff freeze budget: a blocking stall past 1s is the @@ -32,7 +36,8 @@ const MAX_HEAP_GROWTH_PER_CYCLE_MB = 75 // A virtualized list mounts viewport + overscan rows only; anything past this // bound means the panel is mounting rows proportional to the change set again. -const MAX_MOUNTED_ROWS = 1_000 +const MAX_MOUNTED_ROWS = 200 +const MAX_CAPPED_STATUS_PAYLOAD_BYTES = 200_000 type LoadMeasurement = { entryCount: number @@ -259,13 +264,13 @@ test.describe('Source Control large file count (#8013)', () => { // failing scale must not skip the others — every scenario is a data point. test.use({ seedTestRepo: false }) - test('thousands of untracked files under the status cap stay responsive', async ({ + test('a large untracked set under the status cap stays responsive', async ({ orcaPage, electronApp, registerPostElectronShutdownCleanup }) => { test.setTimeout(600_000) - const untrackedFiles = Number(process.env.ORCA_LARGE_FILE_COUNT ?? '9500') + const untrackedFiles = Number(process.env.ORCA_LARGE_FILE_COUNT ?? '950') // Why: ORCA_LARGE_FILE_BYTES gives untracked files realistic sizes so the // per-poll line-stat reads (cache-capped at 2,048 entries) become visible // in rescanMs instead of hiding behind ~30-byte fixture files. @@ -311,13 +316,13 @@ test.describe('Source Control large file count (#8013)', () => { } }) - test('thousands of modified tracked files under the status cap stay responsive', async ({ + test('a large modified set under the status cap stays responsive', async ({ orcaPage, electronApp, registerPostElectronShutdownCleanup }) => { test.setTimeout(600_000) - const modifiedFiles = Number(process.env.ORCA_LARGE_FILE_COUNT ?? '5000') + const modifiedFiles = Number(process.env.ORCA_LARGE_FILE_COUNT ?? '750') const fixture = createLargeFileCountRepo({ trackedFiles: modifiedFiles, modifiedFiles }) registerPostElectronShutdownCleanup(() => removeLargeFileCountRepo(fixture.repoPath)) try { @@ -352,12 +357,45 @@ test.describe('Source Control large file count (#8013)', () => { registerPostElectronShutdownCleanup }) => { test.setTimeout(600_000) - const untrackedFiles = DEFAULT_GIT_STATUS_LIMIT + 1_000 - const fixture = createLargeFileCountRepo({ trackedFiles: 100, untrackedFiles }) + const untrackedFiles = 12_000 + const fixture = createLargeFileCountRepo({ untrackedFiles }) registerPostElectronShutdownCleanup(() => removeLargeFileCountRepo(fixture.repoPath)) try { await waitForSessionReady(orcaPage) + await orcaPage.evaluate(() => { + const probe = { lastTick: performance.now(), maxLagMs: 0, timer: 0 } + probe.timer = window.setInterval(() => { + const now = performance.now() + probe.maxLagMs = Math.max(probe.maxLagMs, now - probe.lastTick - 50) + probe.lastTick = now + }, 50) + ;( + window as unknown as { + __sourceControlActivationLagProbe?: typeof probe + } + ).__sourceControlActivationLagProbe = probe + }) + const activationStart = performance.now() const worktreeId = await addAndActivateRepo(orcaPage, fixture.repoPath) + const activationMs = performance.now() - activationStart + const activationMaxLagMs = await orcaPage.evaluate(() => { + const target = window as unknown as { + __sourceControlActivationLagProbe?: { + maxLagMs: number + timer: number + } + } + const probe = target.__sourceControlActivationLagProbe + if (!probe) { + return -1 + } + window.clearInterval(probe.timer) + delete target.__sourceControlActivationLagProbe + return probe.maxLagMs + }) + console.log( + `[large-file-count] initial-activation ${JSON.stringify({ activationMs, activationMaxLagMs })}` + ) const workingSetBeforeMb = await readRendererWorkingSetMb(electronApp) const measurement = await measureSourceControlLoad(orcaPage, { worktreeId, @@ -373,10 +411,20 @@ test.describe('Source Control large file count (#8013)', () => { rendererWorkingSetMb: { before: workingSetBeforeMb, after: workingSetAfterMb } }) + const tooManyChangesBanner = orcaPage.getByText('Too many changes detected.', { + exact: false + }) + await expect(tooManyChangesBanner).toBeVisible() + if (process.env.ORCA_LARGE_FILE_SCREENSHOT_PATH) { + await orcaPage.screenshot({ path: process.env.ORCA_LARGE_FILE_SCREENSHOT_PATH }) + } + expect(measurement.didHitLimit).toBe(true) expect(measurement.entryCount).toBeLessThanOrEqual(DEFAULT_GIT_STATUS_LIMIT) + expect(measurement.payloadBytes).toBeLessThan(MAX_CAPPED_STATUS_PAYLOAD_BYTES) expect(measurement.renderedRows).toBeLessThan(MAX_MOUNTED_ROWS) expect(measurement.maxLagMs).toBeLessThan(MAX_EVENT_LOOP_LAG_MS) + expect(activationMaxLagMs).toBeLessThan(MAX_EVENT_LOOP_LAG_MS) // Why: didHitLimit must park the worktree in the huge-status state so // background polling stops re-running tens-of-seconds git scans. @@ -385,27 +433,37 @@ test.describe('Source Control large file count (#8013)', () => { worktreeId ) expect(hugeState).not.toBeNull() + + // Why: watcher refreshes stay parked while huge; the visible Retry is the + // explicit recovery path after the underlying change count drops. + removeLargeFileCountUntrackedTree(fixture.repoPath) + await expect(tooManyChangesBanner).toBeVisible() + await orcaPage.getByRole('button', { name: 'Retry' }).click() + await expect(tooManyChangesBanner).not.toBeVisible() + await expect + .poll(() => + orcaPage.evaluate( + (wId) => window.__store?.getState().gitStatusHugeByWorktree?.[wId] ?? null, + worktreeId + ) + ) + .toBeNull() } finally { await unregisterLargeFileCountRepos(orcaPage, [fixture.repoPath]) } }) - test('untracked line-stat cache stays effective above 2,048 files', async ({ + test('untracked line-stat cache stays effective up to the status cap', async ({ orcaPage, registerPostElectronShutdownCleanup }) => { test.setTimeout(600_000) - // Why: the untracked line-stat cache historically capped at 2,048 entries - // with FIFO eviction, so a sequential scan over more files evicted every - // entry before the next poll revisited it (0% hit rate) and each 3s poll - // re-read every untracked file's contents. The cache is now LRU and sized - // to the status entry limit; this gate keeps it that way by comparing - // per-file warm rescan cost at two scales on the same machine — a healthy - // cache keeps the ratio near 1, thrash makes it several-fold. + // Why: compare warm rescan cost at two sub-cap scales on the same machine; + // a cache sized below one complete status result makes the ratio balloon. const fileBytes = 65_536 const smallRepo = createLargeFileCountRepo({ trackedFiles: 10, - untrackedFiles: 2_000, + untrackedFiles: 400, untrackedFileBytes: fileBytes }) registerPostElectronShutdownCleanup(() => removeLargeFileCountRepo(smallRepo.repoPath)) @@ -413,7 +471,7 @@ test.describe('Source Control large file count (#8013)', () => { try { largeRepo = createLargeFileCountRepo({ trackedFiles: 10, - untrackedFiles: 4_000, + untrackedFiles: 800, untrackedFileBytes: fileBytes }) const largeRepoPath = largeRepo.repoPath @@ -431,8 +489,8 @@ test.describe('Source Control large file count (#8013)', () => { return measurement.rescanMs / files } - const smallPerFileMs = await warmRescanPerFileMs(smallRepo.repoPath, 2_000) - const largePerFileMs = await warmRescanPerFileMs(largeRepo.repoPath, 4_000) + const smallPerFileMs = await warmRescanPerFileMs(smallRepo.repoPath, 400) + const largePerFileMs = await warmRescanPerFileMs(largeRepo.repoPath, 800) console.log( `[large-file-count] line-stat-cache smallPerFileMs=${smallPerFileMs.toFixed(4)} largePerFileMs=${largePerFileMs.toFixed(4)} ratio=${(largePerFileMs / smallPerFileMs).toFixed(2)}` )