From f0bf9546d5301a8b58e27d58a1de4ff27f775e83 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 15 Oct 2026 19:14:00 -0700 Subject: [PATCH 1/2] test(git): move worktree lifecycle compatibility cases into their own file #24093 added a checkout-progress case to the real-Git compatibility suite, which took src/shared/git-binary-compatibility.test.ts to 820 counted lines, over the 800-line limit for test files. Every PR's static analysis now fails on that file. Move the four worktree lifecycle cases into a registered cases file, called at the same point so the shared repository sees the same order. --- src/shared/git-binary-compatibility.test.ts | 151 +---------------- ...fecycle-binary-compatibility.test-cases.ts | 158 ++++++++++++++++++ 2 files changed, 160 insertions(+), 149 deletions(-) create mode 100644 src/shared/git-worktree-lifecycle-binary-compatibility.test-cases.ts diff --git a/src/shared/git-binary-compatibility.test.ts b/src/shared/git-binary-compatibility.test.ts index 238d15397818..589c6f53dda6 100644 --- a/src/shared/git-binary-compatibility.test.ts +++ b/src/shared/git-binary-compatibility.test.ts @@ -5,7 +5,6 @@ import { readFile, realpath, rm, - unlink, utimes, writeFile } from 'node:fs/promises' @@ -16,7 +15,6 @@ import { isUnsupportedMergeTreeMergeBaseError, isUnsupportedMergeTreeWriteTreeError } from './git-merge-tree-capability' -import { isBranchCheckedOutInWorktreeError } from './git-branch-delete-refusal' import { branchHasNoUnmergedChangesOnAnyTarget, type GitBranchCleanupExec @@ -30,14 +28,12 @@ import { isForEachRefExcludeUnsupportedError } from './git-ref-command-capabilit import { isNoWriteFetchHeadUnsupportedError } from './git-fetch-head-capability' import { hasUnsupportedRevParsePathFormatEcho, - isUnsupportedWorktreeAddLockReasonError, isUnsupportedWorktreeListZError } from './git-worktree-command-capabilities' import { gitCredentialPromptGuardEnv } from './git-credential-prompt-env' import { buildGitGrepArgs } from './text-search' import { parseGitRemoteFetchUrls } from './git-remote-url-index' import { GIT_HISTORY_COMMIT_FORMAT, parseGitHistoryLog } from './git-history-log-parser' -import { createGitProgressRecordReader, type GitProgressRecord } from './git-progress-records' import { githubPullRequestHeadLocalRef, gitlabMergeRequestHeadLocalRef, @@ -50,6 +46,7 @@ import { encodeGitPathspecs } from './git-pathspec-stdin' import { GitBinaryTestFixture } from './git-binary.test-fixture' import { registerGitRepositoryRegistrationBinaryCompatibilityCase } from './git-repository-registration-binary-compatibility.test-cases' import { registerGitResolutionBinaryCompatibilityCases } from './git-resolution-binary-compatibility.test-cases' +import { registerGitWorktreeLifecycleBinaryCompatibilityCases } from './git-worktree-lifecycle-binary-compatibility.test-cases' const image = process.env.ORCA_GIT_COMPAT_IMAGE const binary = process.env.ORCA_GIT_COMPAT_BINARY @@ -329,151 +326,7 @@ describeBinaryCompatibility('real Git binary compatibility', () => { ).resolves.toBeDefined() }) - // Why pin this: worktree removal decides whether to prune and retry `branch -d` by - // matching Git's refusal text, and the wording moved inside the supported range - // (<=2.40 "Cannot delete branch 'x' checked out at", >=2.43 "cannot delete branch 'x' - // used by worktree at"). It is also the only evidence that the refusal is a stderr - // message on every supported Git rather than something a caller could read off stdout. - it('refuses to delete a branch another worktree holds, on stderr, in a recognized wording', async () => { - await runGit(['worktree', 'add', '-b', 'compat-held', 'held-wt']) - try { - const refusal = await runGit(['branch', '-d', '--', 'compat-held']).then( - () => null, - (error: unknown) => error - ) - expect(refusal).not.toBeNull() - expect(isBranchCheckedOutInWorktreeError(refusal)).toBe(true) - const streams = refusal as { stdout?: string; stderr?: string } - expect(streams.stderr ?? '').toMatch(/delete branch .*compat-held/i) - expect(streams.stdout ?? '').toBe('') - } finally { - await runGit(['worktree', 'remove', '--force', 'held-wt']) - await runGit(['branch', '-D', 'compat-held']) - } - }) - - // Why pin this: the create card reads the checkout meter `worktree add` writes into a pipe - // unasked (it has no --progress flag), so the record format is a contract with every Git. - it('writes checkout progress records into a pipe during worktree add', async () => { - const fileCount = 120 - const source = join(repoPath, 'progress-source') - await mkdir(source) - try { - await Promise.all( - Array.from({ length: fileCount }, (_, index) => - writeFile(join(source, `file-${index}.txt`), `${index}\n`) - ) - ) - await runGit(['-C', 'progress-source', 'init', '-q']) - await runGit(['-C', 'progress-source', 'add', '.']) - await runGit([ - '-C', - 'progress-source', - '-c', - 'user.name=Compatibility Test', - '-c', - 'user.email=compatibility@example.invalid', - 'commit', - '-qm', - 'files' - ]) - // Why the env: git otherwise waits ~1 s before its first record, longer than this checkout. - const { stderr } = await runGit( - ['-C', 'progress-source', 'worktree', 'add', '-b', 'compat-progress', '../progress-wt'], - { GIT_PROGRESS_DELAY: '0' } - ) - const records: GitProgressRecord[] = [] - createGitProgressRecordReader('Updating files', (record) => records.push(record))(stderr) - - expect(records.length).toBeGreaterThan(1) - expect(records.at(-1)).toEqual({ - percent: 100, - completed: fileCount, - total: fileCount, - done: true - }) - const completed = records.map((record) => record.completed) - expect(completed).toEqual([...completed].sort((left, right) => left - right)) - } finally { - await rm(source, { recursive: true, force: true }) - await rm(join(repoPath, 'progress-wt'), { recursive: true, force: true }) - } - }) - - it('removes locked prepared worktrees without a separate unlock', async () => { - await runGit(['worktree', 'add', '--detach', '--no-checkout', 'compat-discard', 'HEAD']) - await runGit(['-C', 'compat-discard', 'reset', '--hard', 'HEAD']) - await runGit(['worktree', 'lock', '--reason', 'owned preparation', 'compat-discard']) - await runGit(['worktree', 'remove', '--force', '--force', 'compat-discard']) - expect((await runGit(['worktree', 'list', '--porcelain'])).stdout).not.toContain( - 'compat-discard' - ) - }) - - it('supports prepared worktree creation and finalization', async () => { - const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() - const lockPath = join(repoPath, '.git', 'worktrees', 'compat-prepared', 'locked') - const lockReason = 'orca-create-preparation:v1:compat\n' - try { - await runGit([ - 'worktree', - 'add', - '--detach', - '--no-checkout', - '--lock', - '--reason', - lockReason.slice(0, -1), - 'compat-prepared', - 'HEAD' - ]) - expect(supports(2, 33)).toBe(true) - } catch (error) { - expect(supports(2, 33)).toBe(false) - expect(isUnsupportedWorktreeAddLockReasonError(error)).toBe(true) - await runGit(['worktree', 'add', '--detach', '--no-checkout', 'compat-prepared', 'HEAD']) - await writeFile(lockPath, lockReason, { flag: 'wx' }) - } - await expect(readFile(lockPath, 'utf8')).resolves.toBe(lockReason) - await expect(readFile(join(repoPath, 'compat-prepared', 'tracked.txt'))).rejects.toThrow() - await runGit(['-C', 'compat-prepared', 'reset', '--hard', 'HEAD']) - const lockPointers = await runGit([ - '-C', - 'compat-prepared', - 'rev-parse', - '--git-path', - 'locked', - '--git-common-dir' - ]) - const pointerLines = lockPointers.stdout.split('\n') - expect(pointerLines).toHaveLength(3) - expect(pointerLines[0]?.replace(/\r$/, '').replaceAll('\\', '/')).toMatch( - /\.git\/worktrees\/compat-prepared\/locked$/ - ) - expect(pointerLines[1]?.replace(/\r$/, '').replaceAll('\\', '/')).toMatch(/(?:^|\/)\.git$/) - expect(pointerLines[2]).toBe('') - // Why: `-f -f` moves a locked preparation while preserving its lock reason (Git >=2.25). - await runGit(['worktree', 'move', '-f', '-f', 'compat-prepared', 'compat-final']) - await runGit([ - '-C', - 'compat-final', - 'checkout', - '--no-track', - '-b', - 'compat-prepared-final', - head - ]) - - await expect(runGit(['-C', 'compat-final', 'branch', '--show-current'])).resolves.toMatchObject( - { stdout: 'compat-prepared-final\n' } - ) - await expect(runGit(['-C', 'compat-final', 'rev-parse', 'HEAD'])).resolves.toMatchObject({ - stdout: `${head}\n` - }) - await expect(readFile(lockPath, 'utf8')).resolves.toBe(lockReason) - await unlink(lockPath) - await runGit(['worktree', 'remove', '--force', 'compat-final']) - await runGit(['branch', '-D', 'compat-prepared-final']) - }) + registerGitWorktreeLifecycleBinaryCompatibilityCases(runGit, () => repoPath, supports) // Why pin this: the prepared-checkout retarget bound reads these as data, and it fails closed, // so a version that printed a different shape would silently stop every retarget rather than diff --git a/src/shared/git-worktree-lifecycle-binary-compatibility.test-cases.ts b/src/shared/git-worktree-lifecycle-binary-compatibility.test-cases.ts new file mode 100644 index 000000000000..c76717f9de43 --- /dev/null +++ b/src/shared/git-worktree-lifecycle-binary-compatibility.test-cases.ts @@ -0,0 +1,158 @@ +import { mkdir, readFile, rm, unlink, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { expect, it } from 'vitest' +import { isBranchCheckedOutInWorktreeError } from './git-branch-delete-refusal' +import { createGitProgressRecordReader, type GitProgressRecord } from './git-progress-records' +import { isUnsupportedWorktreeAddLockReasonError } from './git-worktree-command-capabilities' + +export function registerGitWorktreeLifecycleBinaryCompatibilityCases( + runGit: (args: string[], env?: NodeJS.ProcessEnv) => Promise<{ stdout: string; stderr: string }>, + repoPath: () => string, + supports: (major: number, minor: number) => boolean +): void { + // Why pin this: worktree removal decides whether to prune and retry `branch -d` by + // matching Git's refusal text, and the wording moved inside the supported range + // (<=2.40 "Cannot delete branch 'x' checked out at", >=2.43 "cannot delete branch 'x' + // used by worktree at"). It is also the only evidence that the refusal is a stderr + // message on every supported Git rather than something a caller could read off stdout. + it('refuses to delete a branch another worktree holds, on stderr, in a recognized wording', async () => { + await runGit(['worktree', 'add', '-b', 'compat-held', 'held-wt']) + try { + const refusal = await runGit(['branch', '-d', '--', 'compat-held']).then( + () => null, + (error: unknown) => error + ) + expect(refusal).not.toBeNull() + expect(isBranchCheckedOutInWorktreeError(refusal)).toBe(true) + const streams = refusal as { stdout?: string; stderr?: string } + expect(streams.stderr ?? '').toMatch(/delete branch .*compat-held/i) + expect(streams.stdout ?? '').toBe('') + } finally { + await runGit(['worktree', 'remove', '--force', 'held-wt']) + await runGit(['branch', '-D', 'compat-held']) + } + }) + + // Why pin this: the create card reads the checkout meter `worktree add` writes into a pipe + // unasked (it has no --progress flag), so the record format is a contract with every Git. + it('writes checkout progress records into a pipe during worktree add', async () => { + const fileCount = 120 + const source = join(repoPath(), 'progress-source') + await mkdir(source) + try { + await Promise.all( + Array.from({ length: fileCount }, (_, index) => + writeFile(join(source, `file-${index}.txt`), `${index}\n`) + ) + ) + await runGit(['-C', 'progress-source', 'init', '-q']) + await runGit(['-C', 'progress-source', 'add', '.']) + await runGit([ + '-C', + 'progress-source', + '-c', + 'user.name=Compatibility Test', + '-c', + 'user.email=compatibility@example.invalid', + 'commit', + '-qm', + 'files' + ]) + // Why the env: git otherwise waits ~1 s before its first record, longer than this checkout. + const { stderr } = await runGit( + ['-C', 'progress-source', 'worktree', 'add', '-b', 'compat-progress', '../progress-wt'], + { GIT_PROGRESS_DELAY: '0' } + ) + const records: GitProgressRecord[] = [] + createGitProgressRecordReader('Updating files', (record) => records.push(record))(stderr) + + expect(records.length).toBeGreaterThan(1) + expect(records.at(-1)).toEqual({ + percent: 100, + completed: fileCount, + total: fileCount, + done: true + }) + const completed = records.map((record) => record.completed) + expect(completed).toEqual([...completed].sort((left, right) => left - right)) + } finally { + await rm(source, { recursive: true, force: true }) + await rm(join(repoPath(), 'progress-wt'), { recursive: true, force: true }) + } + }) + + it('removes locked prepared worktrees without a separate unlock', async () => { + await runGit(['worktree', 'add', '--detach', '--no-checkout', 'compat-discard', 'HEAD']) + await runGit(['-C', 'compat-discard', 'reset', '--hard', 'HEAD']) + await runGit(['worktree', 'lock', '--reason', 'owned preparation', 'compat-discard']) + await runGit(['worktree', 'remove', '--force', '--force', 'compat-discard']) + expect((await runGit(['worktree', 'list', '--porcelain'])).stdout).not.toContain( + 'compat-discard' + ) + }) + + it('supports prepared worktree creation and finalization', async () => { + const head = (await runGit(['rev-parse', 'HEAD'])).stdout.trim() + const lockPath = join(repoPath(), '.git', 'worktrees', 'compat-prepared', 'locked') + const lockReason = 'orca-create-preparation:v1:compat\n' + try { + await runGit([ + 'worktree', + 'add', + '--detach', + '--no-checkout', + '--lock', + '--reason', + lockReason.slice(0, -1), + 'compat-prepared', + 'HEAD' + ]) + expect(supports(2, 33)).toBe(true) + } catch (error) { + expect(supports(2, 33)).toBe(false) + expect(isUnsupportedWorktreeAddLockReasonError(error)).toBe(true) + await runGit(['worktree', 'add', '--detach', '--no-checkout', 'compat-prepared', 'HEAD']) + await writeFile(lockPath, lockReason, { flag: 'wx' }) + } + await expect(readFile(lockPath, 'utf8')).resolves.toBe(lockReason) + await expect(readFile(join(repoPath(), 'compat-prepared', 'tracked.txt'))).rejects.toThrow() + await runGit(['-C', 'compat-prepared', 'reset', '--hard', 'HEAD']) + const lockPointers = await runGit([ + '-C', + 'compat-prepared', + 'rev-parse', + '--git-path', + 'locked', + '--git-common-dir' + ]) + const pointerLines = lockPointers.stdout.split('\n') + expect(pointerLines).toHaveLength(3) + expect(pointerLines[0]?.replace(/\r$/, '').replaceAll('\\', '/')).toMatch( + /\.git\/worktrees\/compat-prepared\/locked$/ + ) + expect(pointerLines[1]?.replace(/\r$/, '').replaceAll('\\', '/')).toMatch(/(?:^|\/)\.git$/) + expect(pointerLines[2]).toBe('') + // Why: `-f -f` moves a locked preparation while preserving its lock reason (Git >=2.25). + await runGit(['worktree', 'move', '-f', '-f', 'compat-prepared', 'compat-final']) + await runGit([ + '-C', + 'compat-final', + 'checkout', + '--no-track', + '-b', + 'compat-prepared-final', + head + ]) + + await expect(runGit(['-C', 'compat-final', 'branch', '--show-current'])).resolves.toMatchObject( + { stdout: 'compat-prepared-final\n' } + ) + await expect(runGit(['-C', 'compat-final', 'rev-parse', 'HEAD'])).resolves.toMatchObject({ + stdout: `${head}\n` + }) + await expect(readFile(lockPath, 'utf8')).resolves.toBe(lockReason) + await unlink(lockPath) + await runGit(['worktree', 'remove', '--force', 'compat-final']) + await runGit(['branch', '-D', 'compat-prepared-final']) + }) +} From 155c82262f78c387da552488e4c7ebcb869fac2a Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 15 Oct 2026 19:37:54 -0700 Subject: [PATCH 2/2] test(mobile): move the chat view's Stop cases into the turn-status tests #29099 took mobile/src/session/MobileNativeChatView.test.ts to 806 counted lines, over the 800-line test cap, so the phone app's lint fails on main. Move the four Stop and Stopping cases verbatim into MobileNativeChatView.turn-status.test.ts, which has the same setup, and put the listIds doc comment back above the function it describes. --- .../src/session/MobileNativeChatView.test.ts | 61 ------------------- .../MobileNativeChatView.turn-status.test.ts | 61 +++++++++++++++++++ 2 files changed, 61 insertions(+), 61 deletions(-) diff --git a/mobile/src/session/MobileNativeChatView.test.ts b/mobile/src/session/MobileNativeChatView.test.ts index 2c235337bb8a..fe01a4014e1e 100644 --- a/mobile/src/session/MobileNativeChatView.test.ts +++ b/mobile/src/session/MobileNativeChatView.test.ts @@ -157,68 +157,7 @@ describe('MobileNativeChatView', () => { }) } - it("holds Stop and says Stopping while this phone's own Stop request is in flight", async () => { - await render({ - structuredActivityUi: true, - agentWorking: true, - canStop: true, - turnIndicator: { - thinking: false, - activityText: null, - stopping: true, - stopRequestInFlight: true - } - }) - const stop = renderer!.root.find( - (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Stopping…' - ) - expect(stop.props.disabled).toBe(true) - }) - - it.each([ - ['queue', 'Queue a message to run after the stop'], - ['send', 'Send a message to run after the stop'] - ] as const)( - 'tells the composer a message sent now runs after the stop (%s)', - async (afterStop, placeholder) => { - await render({ - structuredActivityUi: true, - agentWorking: true, - canStop: true, - turnIndicator: { thinking: false, activityText: null, stopping: true, afterStop } - }) - const composer = renderer!.root.find((node) => node.type === 'Composer') - expect(composer.props.placeholder).toBe(placeholder) - } - ) - - // A Stop the provider took and never answered ends only at a repeat Stop. - it('keeps Stop for the repeat that escalates while the host alone says Stopping', async () => { - await render({ - structuredActivityUi: true, - agentWorking: true, - canStop: true, - turnIndicator: { thinking: false, activityText: null, stopping: true } - }) - const stop = renderer!.root.find( - (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Stop the agent' - ) - expect(stop.props.disabled).toBe(false) - }) - /** Ids of the rows the list is currently rendering. */ - it('keeps Stop hidden during a structured dispatch until a provider turn can be cancelled', async () => { - const props = { structuredActivityUi: true, agentWorking: true, canStop: false } - await render(props) - const stops = () => - renderer!.root.findAll((node) => node.props.accessibilityLabel === 'Stop the agent') - expect(stops()).toHaveLength(0) - await update({ ...props, canStop: true }) - expect(stops()).toHaveLength(1) - await update({ agentWorking: true }) - expect(stops()).toHaveLength(1) - }) - function listIds(): string[] { return (list().props.data as { id: string }[]).map((row) => row.id) } diff --git a/mobile/src/session/MobileNativeChatView.turn-status.test.ts b/mobile/src/session/MobileNativeChatView.turn-status.test.ts index c49e3d89d5f3..cec5253b1e88 100644 --- a/mobile/src/session/MobileNativeChatView.turn-status.test.ts +++ b/mobile/src/session/MobileNativeChatView.turn-status.test.ts @@ -166,6 +166,67 @@ describe('MobileNativeChatView', () => { return listNode.props.renderItem({ item: data[index], index }) } + it("holds Stop and says Stopping while this phone's own Stop request is in flight", async () => { + await render({ + structuredActivityUi: true, + agentWorking: true, + canStop: true, + turnIndicator: { + thinking: false, + activityText: null, + stopping: true, + stopRequestInFlight: true + } + }) + const stop = renderer!.root.find( + (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Stopping…' + ) + expect(stop.props.disabled).toBe(true) + }) + + it.each([ + ['queue', 'Queue a message to run after the stop'], + ['send', 'Send a message to run after the stop'] + ] as const)( + 'tells the composer a message sent now runs after the stop (%s)', + async (afterStop, placeholder) => { + await render({ + structuredActivityUi: true, + agentWorking: true, + canStop: true, + turnIndicator: { thinking: false, activityText: null, stopping: true, afterStop } + }) + const composer = renderer!.root.find((node) => node.type === 'Composer') + expect(composer.props.placeholder).toBe(placeholder) + } + ) + + // A Stop the provider took and never answered ends only at a repeat Stop. + it('keeps Stop for the repeat that escalates while the host alone says Stopping', async () => { + await render({ + structuredActivityUi: true, + agentWorking: true, + canStop: true, + turnIndicator: { thinking: false, activityText: null, stopping: true } + }) + const stop = renderer!.root.find( + (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Stop the agent' + ) + expect(stop.props.disabled).toBe(false) + }) + + it('keeps Stop hidden during a structured dispatch until a provider turn can be cancelled', async () => { + const props = { structuredActivityUi: true, agentWorking: true, canStop: false } + await render(props) + const stops = () => + renderer!.root.findAll((node) => node.props.accessibilityLabel === 'Stop the agent') + expect(stops()).toHaveLength(0) + await update({ ...props, canStop: true }) + expect(stops()).toHaveLength(1) + await update({ agentWorking: true }) + expect(stops()).toHaveLength(1) + }) + describe('structured turn status wiring', () => { const userTurn = (id: string, text: string): NativeChatMessage => ({ id,