fix(git): keep worktree views transactionally consistent
This commit is contained in:
+89
-33
@@ -111,6 +111,9 @@ export function GitView(props: {
|
||||
/** The pending destructive action awaiting confirmation. */
|
||||
const [confirm, setConfirm] = useState<ConfirmState | null>(null)
|
||||
const refreshInFlight = useRef(false)
|
||||
/** Monotonic request id: a manual worktree switch invalidates any older poll
|
||||
* before it can publish state from the previous checkout. */
|
||||
const refreshGeneration = useRef(0)
|
||||
const worktreeChosenByUser = useRef(false)
|
||||
/** selectedWorktree read inside refresh without re-creating the callback:
|
||||
* avoids a spurious full refresh on every auto-select (the very state
|
||||
@@ -119,13 +122,41 @@ export function GitView(props: {
|
||||
const selectedRef = useRef<string | undefined>(undefined)
|
||||
useEffect(() => { selectedRef.current = selectedWorktree }, [selectedWorktree])
|
||||
|
||||
/** Publish a complete checkout-derived view. Status, branch choices and
|
||||
* history are one consistency unit: never mix rows from two worktrees. */
|
||||
const refreshTarget = useCallback(async (
|
||||
target: string | undefined,
|
||||
options: { loading: boolean; generation: number },
|
||||
): Promise<void> => {
|
||||
if (options.loading) setLoading(true)
|
||||
setError(null)
|
||||
try {
|
||||
const [statusResult, branchResult, logResult] = await Promise.all([
|
||||
api.gitStatus(scope, target),
|
||||
api.gitBranch(scope, target).catch(() => ({ current: '', names: [] as string[] })),
|
||||
api.gitLog(scope, LOG_BATCH, 0, target).catch(() => [] as GitLogEntry[]),
|
||||
])
|
||||
if (options.generation !== refreshGeneration.current) return
|
||||
setStatus(statusResult)
|
||||
setBranchNames(branchResult.names)
|
||||
setLogEntries(logResult)
|
||||
setLogEnded(logResult.length < LOG_BATCH)
|
||||
} catch (reason) {
|
||||
if (options.generation === refreshGeneration.current) {
|
||||
setError(reason instanceof Error ? reason.message : String(reason))
|
||||
}
|
||||
} finally {
|
||||
if (options.loading && options.generation === refreshGeneration.current) setLoading(false)
|
||||
}
|
||||
}, [scope.sessionId, scope.cwd])
|
||||
|
||||
const refresh = useCallback(async (silent = false): Promise<void> => {
|
||||
if (refreshInFlight.current) return
|
||||
refreshInFlight.current = true
|
||||
if (!silent) setLoading(true)
|
||||
setError(null)
|
||||
let generation = refreshGeneration.current
|
||||
try {
|
||||
const listed = await api.gitWorktrees(scope)
|
||||
if (generation !== refreshGeneration.current) return
|
||||
setWorktrees(listed)
|
||||
const selectedStillExists = listed.some(entry => entry.path === selectedRef.current)
|
||||
let target = selectedStillExists ? selectedRef.current : listed.find(entry => entry.current)?.path
|
||||
@@ -139,42 +170,63 @@ export function GitView(props: {
|
||||
? dirtyLinked[0]!.path
|
||||
: current?.path
|
||||
}
|
||||
if (target !== selectedRef.current) setSelectedWorktree(target)
|
||||
if (silent) {
|
||||
setStatus(await api.gitStatus(scope, target))
|
||||
const targetChanged = target !== selectedRef.current
|
||||
if (targetChanged) {
|
||||
// Changing the automatically selected checkout invalidates any direct
|
||||
// target refresh that may still be resolving for the previous one.
|
||||
generation = refreshGeneration.current += 1
|
||||
selectedRef.current = target
|
||||
setSelectedWorktree(target)
|
||||
// Remove rows owned by the previous checkout immediately: keeping them
|
||||
// interactive while the target changes could apply a destructive action
|
||||
// to the new checkout with stale history from the old one.
|
||||
setStatus(null)
|
||||
setBranchNames([])
|
||||
setLogEntries([])
|
||||
setLogEnded(false)
|
||||
setLogLoadingMore(false)
|
||||
}
|
||||
// A poll may update status alone only while staying on the same checkout.
|
||||
// Any automatic selection change refreshes the complete derived view.
|
||||
if (silent && !targetChanged) {
|
||||
const statusResult = await api.gitStatus(scope, target)
|
||||
if (generation === refreshGeneration.current) setStatus(statusResult)
|
||||
return
|
||||
}
|
||||
const [statusResult, branchResult, logResult] = await Promise.all([
|
||||
api.gitStatus(scope, target),
|
||||
api.gitBranch(scope, target).catch(() => ({ current: '', names: [] as string[] })),
|
||||
// The first history page only; the rest arrives via "load more".
|
||||
api.gitLog(scope, LOG_BATCH, 0, target).catch(() => [] as GitLogEntry[]),
|
||||
])
|
||||
setStatus(statusResult)
|
||||
setBranchNames(branchResult.names)
|
||||
setLogEntries(logResult)
|
||||
setLogEnded(logResult.length < LOG_BATCH)
|
||||
await refreshTarget(target, { loading: !silent, generation })
|
||||
} catch (reason) {
|
||||
setError(reason instanceof Error ? reason.message : String(reason))
|
||||
if (generation === refreshGeneration.current) {
|
||||
setError(reason instanceof Error ? reason.message : String(reason))
|
||||
if (!silent) setLoading(false)
|
||||
}
|
||||
} finally {
|
||||
refreshInFlight.current = false
|
||||
if (!silent) setLoading(false)
|
||||
}
|
||||
}, [scope.sessionId, scope.cwd])
|
||||
}, [scope.sessionId, scope.cwd, refreshTarget])
|
||||
|
||||
useEffect(() => {
|
||||
refreshGeneration.current += 1
|
||||
refreshInFlight.current = false
|
||||
worktreeChosenByUser.current = false
|
||||
selectedRef.current = undefined
|
||||
setSelectedWorktree(undefined)
|
||||
}, [scope.sessionId, scope.cwd])
|
||||
useEffect(() => { void refresh() }, [refresh])
|
||||
// When the user picks a worktree from the dropdown, fetch its status
|
||||
// immediately instead of waiting for the next 2s poll tick. Gated on
|
||||
// worktreeChosenByUser so the auto-select inside refresh() doesn't
|
||||
// trigger a redundant fetch (that was the N→N+1 loop the ref fix avoids).
|
||||
useEffect(() => {
|
||||
if (!worktreeChosenByUser.current) return
|
||||
void refresh(true)
|
||||
}, [selectedWorktree, refresh])
|
||||
|
||||
/** A user choice invalidates any older poll and atomically refreshes every
|
||||
* checkout-derived surface before destructive history actions can run. */
|
||||
const chooseWorktree = (target: string): void => {
|
||||
worktreeChosenByUser.current = true
|
||||
selectedRef.current = target
|
||||
setSelectedWorktree(target)
|
||||
setStatus(null)
|
||||
setBranchNames([])
|
||||
setLogEntries([])
|
||||
setLogEnded(false)
|
||||
setLogLoadingMore(false)
|
||||
const generation = refreshGeneration.current += 1
|
||||
void refreshTarget(target, { loading: true, generation })
|
||||
}
|
||||
useEffect(() => {
|
||||
if (!visible) return
|
||||
const timer = window.setInterval(() => { void refresh(true) }, 2_000)
|
||||
@@ -184,15 +236,22 @@ export function GitView(props: {
|
||||
/** Append the next history page (lazy: only when the user asks for more). */
|
||||
const loadMoreLog = async (): Promise<void> => {
|
||||
if (logLoadingMore || logEnded) return
|
||||
const generation = refreshGeneration.current
|
||||
const target = selectedRef.current
|
||||
setLogLoadingMore(true)
|
||||
try {
|
||||
const next = await api.gitLog(scope, LOG_BATCH, logEntries.length, selectedWorktree)
|
||||
const next = await api.gitLog(scope, LOG_BATCH, logEntries.length, target)
|
||||
// A worktree switch clears the old history and increments generation.
|
||||
// Never append a late page from that checkout into the new one.
|
||||
if (generation !== refreshGeneration.current || target !== selectedRef.current) return
|
||||
setLogEntries(entries => [...entries, ...next])
|
||||
if (next.length < LOG_BATCH) setLogEnded(true)
|
||||
} catch (reason) {
|
||||
setCommitError(`${t('historyLoadError')}: ${reason instanceof Error ? reason.message : String(reason)}`)
|
||||
if (generation === refreshGeneration.current && target === selectedRef.current) {
|
||||
setCommitError(`${t('historyLoadError')}: ${reason instanceof Error ? reason.message : String(reason)}`)
|
||||
}
|
||||
} finally {
|
||||
setLogLoadingMore(false)
|
||||
if (generation === refreshGeneration.current && target === selectedRef.current) setLogLoadingMore(false)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -341,10 +400,7 @@ export function GitView(props: {
|
||||
value={selectedWorktree ?? ''}
|
||||
title={selectedWorktree}
|
||||
disabled={busy}
|
||||
onChange={(event) => {
|
||||
worktreeChosenByUser.current = true
|
||||
setSelectedWorktree(event.target.value)
|
||||
}}
|
||||
onChange={(event) => { chooseWorktree(event.target.value) }}
|
||||
>
|
||||
{worktrees.map(entry => (
|
||||
<option key={entry.path} value={entry.path}>
|
||||
|
||||
+29
-7
@@ -84,17 +84,31 @@ export function parsePorcelainZ(output: string): GitStatusEntry[] {
|
||||
return entries
|
||||
}
|
||||
|
||||
/** One raw porcelain worktree record. Prunable checkouts are retained by
|
||||
* Git's administrative metadata after their directory disappears and must not
|
||||
* become selectable command targets. Locked checkouts remain usable. */
|
||||
export interface GitWorktreeRecord {
|
||||
path: string
|
||||
branch: string
|
||||
locked: boolean
|
||||
prunable: boolean
|
||||
}
|
||||
|
||||
/** Parse `git worktree list --porcelain` records. Production requests use
|
||||
* `-z` so even newlines and non-ASCII bytes in checkout paths stay lossless;
|
||||
* newline framing remains accepted for small fixtures and older Git output. */
|
||||
export function parseWorktreeList(output: string): Array<{ path: string; branch: string }> {
|
||||
const rows: Array<{ path: string; branch: string }> = []
|
||||
export function parseWorktreeList(output: string): GitWorktreeRecord[] {
|
||||
const rows: GitWorktreeRecord[] = []
|
||||
let path: string | undefined
|
||||
let branch = 'HEAD'
|
||||
let locked = false
|
||||
let prunable = false
|
||||
const flush = (): void => {
|
||||
if (path !== undefined) rows.push({ path, branch })
|
||||
if (path !== undefined) rows.push({ path, branch, locked, prunable })
|
||||
path = undefined
|
||||
branch = 'HEAD'
|
||||
locked = false
|
||||
prunable = false
|
||||
}
|
||||
const sep = output.includes('\0') ? '\0' : '\n'
|
||||
const framed = output.endsWith(sep) ? output : `${output}${sep}`
|
||||
@@ -105,6 +119,10 @@ export function parseWorktreeList(output: string): Array<{ path: string; branch:
|
||||
path = line.slice('worktree '.length)
|
||||
} else if (line.startsWith('branch refs/heads/')) {
|
||||
branch = line.slice('branch refs/heads/'.length)
|
||||
} else if (line === 'locked' || line.startsWith('locked ')) {
|
||||
locked = true
|
||||
} else if (line === 'prunable' || line.startsWith('prunable ')) {
|
||||
prunable = true
|
||||
}
|
||||
}
|
||||
return rows
|
||||
@@ -199,10 +217,12 @@ function pathIdentity(path: string): string {
|
||||
return process.platform === 'win32' ? absolute.toLowerCase() : absolute
|
||||
}
|
||||
|
||||
/** Raw linked checkout records, shared by inventory and target validation. */
|
||||
async function listedWorktrees(cwd: string): Promise<Array<{ path: string; branch: string }>> {
|
||||
/** Raw usable checkout records, shared by inventory and target validation.
|
||||
* Prunable records point at missing paths and are deliberately excluded from
|
||||
* both the selector and the command-target allowlist. */
|
||||
async function listedWorktrees(cwd: string): Promise<GitWorktreeRecord[]> {
|
||||
const raw = await runGit(cwd, ['worktree', 'list', '--porcelain', '-z'])
|
||||
return parseWorktreeList(raw)
|
||||
return parseWorktreeList(raw).filter(entry => !entry.prunable)
|
||||
}
|
||||
|
||||
/** All linked checkouts of the repository containing `cwd`, enriched with a
|
||||
@@ -216,7 +236,9 @@ export async function worktrees(cwd: string): Promise<GitWorktree[]> {
|
||||
path: entry.path,
|
||||
branch: entry.branch,
|
||||
current: pathIdentity(entry.path) === pathIdentity(currentRoot),
|
||||
changes: (await status(entry.path)).entries.length,
|
||||
// One stale/permission-raced linked checkout must not hide the valid
|
||||
// current repository from the panel. Targeted operations still fail loud.
|
||||
changes: await status(entry.path).then(result => result.entries.length, () => 0),
|
||||
})))
|
||||
return rows.sort((left, right) => Number(right.current) - Number(left.current))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
/**
|
||||
* Git worktree selection is an atomic view boundary: status, branch choices
|
||||
* and history must always come from the same checkout. A stale history row is
|
||||
* especially dangerous because its revert/cherry-pick action targets the
|
||||
* currently selected checkout.
|
||||
*/
|
||||
// @vitest-environment jsdom
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createElement } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { act } from 'react-dom/test-utils'
|
||||
import { GitView } from '../src/client/GitView.tsx'
|
||||
import { api, type GitLogEntry, type GitStatusResult, type GitWorktree } from '../src/client/api.ts'
|
||||
|
||||
;(globalThis as Record<string, unknown>).IS_REACT_ACT_ENVIRONMENT = true
|
||||
|
||||
const MAIN = 'C:/repo/main'
|
||||
const AGENT = 'C:/repo/agent'
|
||||
|
||||
const inventories: GitWorktree[] = [
|
||||
{ path: MAIN, branch: 'main', current: true, changes: 0 },
|
||||
{ path: AGENT, branch: 'agent', current: false, changes: 1 },
|
||||
]
|
||||
|
||||
function statusFor(target?: string): GitStatusResult {
|
||||
return target === AGENT
|
||||
? { isRepo: true, branch: 'agent', entries: [{ path: 'agent-change.ts', xy: ' M' }] }
|
||||
: { isRepo: true, branch: 'main', entries: [{ path: 'main-change.ts', xy: ' M' }] }
|
||||
}
|
||||
|
||||
function logFor(target?: string, index = 0): GitLogEntry[] {
|
||||
const agent = target === AGENT
|
||||
const digit = agent ? 'a' : 'b'
|
||||
const suffix = index.toString(16).padStart(8, '0')
|
||||
return [{
|
||||
hash: `${digit.repeat(6)}${suffix.slice(-1)}`,
|
||||
hashFull: `${digit.repeat(32)}${suffix}`,
|
||||
subject: agent ? `Agent checkout commit ${index}` : `Main checkout commit ${index}`,
|
||||
author: 'Test',
|
||||
date: '2026-08-20 00:00:00 +0800',
|
||||
refs: agent ? 'HEAD -> agent' : 'HEAD -> main',
|
||||
}]
|
||||
}
|
||||
|
||||
function deferred<T>(): { promise: Promise<T>; resolve: (value: T) => void } {
|
||||
let resolvePromise!: (value: T) => void
|
||||
const promise = new Promise<T>((resolve) => { resolvePromise = resolve })
|
||||
return { promise, resolve: resolvePromise }
|
||||
}
|
||||
|
||||
async function flushEffects(): Promise<void> {
|
||||
await act(async () => { await Promise.resolve() })
|
||||
await act(async () => { await Promise.resolve() })
|
||||
}
|
||||
|
||||
afterEach(() => { vi.restoreAllMocks() })
|
||||
|
||||
describe('GitView linked-worktree consistency', () => {
|
||||
it('refreshes status, branches and history together on auto and manual selection', async () => {
|
||||
vi.spyOn(api, 'gitWorktrees').mockResolvedValue(inventories)
|
||||
vi.spyOn(api, 'gitStatus').mockImplementation(async (_scope, target) => statusFor(target))
|
||||
const branch = vi.spyOn(api, 'gitBranch').mockImplementation(async (_scope, target) => ({
|
||||
current: target === AGENT ? 'agent' : 'main',
|
||||
names: target === AGENT ? ['agent'] : ['main'],
|
||||
}))
|
||||
const log = vi.spyOn(api, 'gitLog').mockImplementation(async (_scope, _count, _skip, target) => logFor(target))
|
||||
|
||||
const container = document.createElement('div')
|
||||
document.body.append(container)
|
||||
const root: Root = createRoot(container)
|
||||
try {
|
||||
await act(async () => {
|
||||
root.render(createElement(GitView, {
|
||||
scope: { sessionId: 'session', cwd: MAIN },
|
||||
onOpenFile: () => {},
|
||||
onOpenDiff: () => {},
|
||||
visible: false,
|
||||
}))
|
||||
})
|
||||
await flushEffects()
|
||||
|
||||
const selects = container.querySelectorAll<HTMLSelectElement>('select')
|
||||
const worktreeSelect = selects[0]!
|
||||
// A clean primary + exactly one dirty linked checkout auto-selects the
|
||||
// linked checkout and loads every target-derived surface from it.
|
||||
expect(worktreeSelect.value).toBe(AGENT)
|
||||
expect(container.textContent).toContain('agent-change.ts')
|
||||
expect(container.textContent).toContain('Agent checkout commit')
|
||||
expect(container.textContent).not.toContain('Main checkout commit')
|
||||
expect(branch).toHaveBeenCalledWith(expect.anything(), AGENT)
|
||||
expect(log).toHaveBeenCalledWith(expect.anything(), 20, 0, AGENT)
|
||||
|
||||
await act(async () => {
|
||||
Object.getOwnPropertyDescriptor(HTMLSelectElement.prototype, 'value')!.set!.call(worktreeSelect, MAIN)
|
||||
worktreeSelect.dispatchEvent(new Event('change', { bubbles: true }))
|
||||
})
|
||||
await flushEffects()
|
||||
|
||||
expect(worktreeSelect.value).toBe(MAIN)
|
||||
expect(container.textContent).toContain('main-change.ts')
|
||||
expect(container.textContent).toContain('Main checkout commit')
|
||||
expect(container.textContent).not.toContain('Agent checkout commit')
|
||||
expect(branch).toHaveBeenLastCalledWith(expect.anything(), MAIN)
|
||||
expect(log).toHaveBeenLastCalledWith(expect.anything(), 20, 0, MAIN)
|
||||
} finally {
|
||||
act(() => { root.unmount() })
|
||||
container.remove()
|
||||
}
|
||||
})
|
||||
|
||||
it('drops a late history page when the selected worktree changes', async () => {
|
||||
const lateAgentPage = deferred<GitLogEntry[]>()
|
||||
vi.spyOn(api, 'gitWorktrees').mockResolvedValue(inventories)
|
||||
vi.spyOn(api, 'gitStatus').mockImplementation(async (_scope, target) => statusFor(target))
|
||||
vi.spyOn(api, 'gitBranch').mockImplementation(async (_scope, target) => ({
|
||||
current: target === AGENT ? 'agent' : 'main',
|
||||
names: target === AGENT ? ['agent'] : ['main'],
|
||||
}))
|
||||
vi.spyOn(api, 'gitLog').mockImplementation(async (_scope, _count, skip, target) => {
|
||||
if (target === AGENT && skip === 20) return lateAgentPage.promise
|
||||
if (target === AGENT) return Array.from({ length: 20 }, (_value, index) => logFor(AGENT, index)[0]!)
|
||||
return logFor(MAIN)
|
||||
})
|
||||
|
||||
const container = document.createElement('div')
|
||||
document.body.append(container)
|
||||
const root: Root = createRoot(container)
|
||||
try {
|
||||
await act(async () => {
|
||||
root.render(createElement(GitView, {
|
||||
scope: { sessionId: 'session', cwd: MAIN },
|
||||
onOpenFile: () => {},
|
||||
onOpenDiff: () => {},
|
||||
visible: false,
|
||||
}))
|
||||
})
|
||||
await flushEffects()
|
||||
|
||||
const worktreeSelect = container.querySelectorAll<HTMLSelectElement>('select')[0]!
|
||||
const loadMore = [...container.querySelectorAll<HTMLButtonElement>('button')]
|
||||
.find(button => /Load more|加载更多/.test(button.textContent ?? ''))
|
||||
expect(loadMore).not.toBeUndefined()
|
||||
await act(async () => { loadMore!.click() })
|
||||
|
||||
await act(async () => {
|
||||
Object.getOwnPropertyDescriptor(HTMLSelectElement.prototype, 'value')!.set!.call(worktreeSelect, MAIN)
|
||||
worktreeSelect.dispatchEvent(new Event('change', { bubbles: true }))
|
||||
})
|
||||
await flushEffects()
|
||||
expect(container.textContent).toContain('Main checkout commit 0')
|
||||
|
||||
lateAgentPage.resolve(logFor(AGENT, 99))
|
||||
await flushEffects()
|
||||
expect(container.textContent).toContain('Main checkout commit 0')
|
||||
expect(container.textContent).not.toContain('Agent checkout commit 99')
|
||||
} finally {
|
||||
act(() => { root.unmount() })
|
||||
container.remove()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -33,8 +33,25 @@ describe('linked Git worktrees', () => {
|
||||
'detached',
|
||||
'',
|
||||
].join('\n'))).toEqual([
|
||||
{ path: 'C:/repo/main checkout', branch: 'main' },
|
||||
{ path: 'C:/repo/agent checkout', branch: 'HEAD' },
|
||||
{ path: 'C:/repo/main checkout', branch: 'main', locked: false, prunable: false },
|
||||
{ path: 'C:/repo/agent checkout', branch: 'HEAD', locked: false, prunable: false },
|
||||
])
|
||||
})
|
||||
|
||||
it('preserves locked metadata and marks stale records prunable', () => {
|
||||
expect(parseWorktreeList([
|
||||
'worktree C:/repo/locked',
|
||||
'HEAD abc',
|
||||
'branch refs/heads/locked',
|
||||
'locked in use',
|
||||
'',
|
||||
'worktree C:/repo/missing',
|
||||
'HEAD def',
|
||||
'prunable gitdir file points to non-existent location',
|
||||
'',
|
||||
].join('\0'))).toEqual([
|
||||
{ path: 'C:/repo/locked', branch: 'locked', locked: true, prunable: false },
|
||||
{ path: 'C:/repo/missing', branch: 'HEAD', locked: false, prunable: true },
|
||||
])
|
||||
})
|
||||
|
||||
@@ -60,6 +77,16 @@ describe('linked Git worktrees', () => {
|
||||
{ path: 'tracked.txt', xy: ' M' },
|
||||
])
|
||||
await expect(resolveWorktree(main, root)).rejects.toThrow('unknown linked worktree')
|
||||
|
||||
// Git retains administrative metadata after a checkout directory is
|
||||
// deleted, reporting the record as prunable. It must disappear from both
|
||||
// the selector inventory and the accepted command-target allowlist.
|
||||
rmSync(agent, { recursive: true, force: true })
|
||||
const remaining = await worktrees(main)
|
||||
expect(remaining).toHaveLength(1)
|
||||
expect(resolve(remaining[0]!.path)).toBe(resolve(main))
|
||||
expect(remaining[0]!.current).toBe(true)
|
||||
await expect(resolveWorktree(main, agent)).rejects.toThrow('unknown linked worktree')
|
||||
} finally {
|
||||
try { git(main, ['worktree', 'remove', '--force', agent]) } catch { /* fixture may not be fully initialized */ }
|
||||
rmSync(root, { recursive: true, force: true })
|
||||
|
||||
Reference in New Issue
Block a user