fix(worktree): retain index warming ownership until group exit

This commit is contained in:
Neil
2026-09-04 23:14:45 -07:00
parent 8c0e7eccd0
commit 17ec0d4f94
6 changed files with 86 additions and 7 deletions
+1 -1
View File
@@ -25,7 +25,7 @@ vi.mock('./worktree-index-warming-ownership', () => ({
WorktreeIndexWarmingOwnership: class {
arm = vi.fn().mockResolvedValue(undefined)
recordPid = vi.fn()
release = vi.fn().mockResolvedValue(undefined)
release = vi.fn().mockResolvedValue(true)
},
canReclaimIndexWarming: vi.fn().mockResolvedValue(true),
removeIndexWarmingOwnership: vi.fn().mockResolvedValue(undefined)
@@ -24,10 +24,39 @@ it('retains the pre-spawn window and clears ownership only after PID persistence
await owner.arm()
expect(await canReclaimIndexWarming(prepared)).toBe(false)
owner.recordPid(12345)
await owner.release()
vi.spyOn(process, 'kill').mockImplementation(() => {
throw Object.assign(new Error('probe'), { code: 'ESRCH' })
})
expect(await owner.release()).toBe(true)
await expect(readFile(`${prepared}.index-warming`)).rejects.toMatchObject({ code: 'ENOENT' })
expect(await canReclaimIndexWarming(prepared)).toBe(true)
})
it('releases a never-spawned owner without probing a process group', async () => {
const owner = new WorktreeIndexWarmingOwnership(prepared)
await owner.arm()
const kill = vi.spyOn(process, 'kill')
expect(await owner.release()).toBe(true)
expect(kill).not.toHaveBeenCalled()
await expect(readFile(`${prepared}.index-warming`)).rejects.toMatchObject({ code: 'ENOENT' })
})
it.each(['EPERM', undefined])(
'retains the marker after root exit while the group is live or unverifiable (%s)',
async (code) => {
const owner = new WorktreeIndexWarmingOwnership(prepared)
await owner.arm()
owner.recordPid(12345)
const kill = vi.spyOn(process, 'kill').mockImplementation(() => {
if (code) {
throw Object.assign(new Error('probe'), { code })
}
return true
})
expect(await owner.release()).toBe(false)
expect(kill).toHaveBeenCalledExactlyOnceWith(-12345, 0)
expect(await readFile(`${prepared}.index-warming`, 'utf8')).toBe('12345\n')
expect(await canReclaimIndexWarming(prepared)).toBe(false)
}
)
it('does not overwrite another ownership record', async () => {
await writeFile(`${prepared}.index-warming`, '12345\n')
await expect(new WorktreeIndexWarmingOwnership(prepared).arm()).rejects.toMatchObject({
+8 -1
View File
@@ -8,6 +8,7 @@ function markerPath(preparedPath: string): string {
/** The pending marker precedes spawn; an interrupted PID write remains unverifiable. */
export class WorktreeIndexWarmingOwnership {
private pidWrite: Promise<void> = Promise.resolve()
private pid: number | undefined
constructor(private readonly preparedPath: string) {}
arm(): Promise<void> {
@@ -15,12 +16,18 @@ export class WorktreeIndexWarmingOwnership {
}
recordPid(pid: number): void {
this.pid = pid
this.pidWrite = writeFile(markerPath(this.preparedPath), `${pid}\n`).catch(() => {})
}
async release(): Promise<void> {
// Git hooks can outlive the root; retain ownership until the whole group has exited.
async release(): Promise<boolean> {
await this.pidWrite
if (this.pid !== undefined && !hasExitedPosixProcessGroup(this.pid)) {
return false
}
await rm(markerPath(this.preparedPath), { force: true })
return true
}
}
@@ -54,7 +54,7 @@ beforeEach(() => {
mocks.git.mockReset().mockResolvedValue({ stdout: '' })
mocks.arm.mockReset().mockResolvedValue(undefined)
mocks.recordPid.mockReset()
mocks.release.mockReset().mockResolvedValue(undefined)
mocks.release.mockReset().mockResolvedValue(true)
})
afterEach(async () => {
await _resetPreparationPoolForTests()
@@ -226,3 +226,35 @@ it('retains the checkout if the ownership marker cannot be removed', async () =>
await vi.advanceTimersByTimeAsync(1_100)
await expect(takePreparation(listPreparations()[0])).resolves.toBe(false)
})
it('successful Git exit with a live group blocks claim, retains ownership and disables warming', async () => {
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
mocks.release.mockResolvedValue(false)
await startPreparation(args)
await vi.advanceTimersByTimeAsync(1_100)
expect(mocks.release).toHaveBeenCalledOnce()
await expect(takePreparation(listPreparations()[0])).resolves.toBe(false)
expect(warn).toHaveBeenCalledWith(expect.stringContaining('process group live or unverifiable'))
await startPreparation(args)
await vi.advanceTimersByTimeAsync(2_000)
expect(mocks.git).toHaveBeenCalledOnce()
expect(mocks.discard).not.toHaveBeenCalled()
})
it('does not discard an expired preparation whose group is live after Git success', async () => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
mocks.release.mockResolvedValue(false)
await startPreparation(args)
await vi.advanceTimersByTimeAsync(WORKTREE_CREATE_PREPARATION_TTL_MS)
expect(mocks.discard).not.toHaveBeenCalled()
})
it('a refresh failure still requires the spawned group to have exited', async () => {
vi.spyOn(console, 'warn').mockImplementation(() => {})
mocks.git.mockImplementation(async (_args, options) => {
options.onChildSpawned(12345)
throw new Error('index refresh failed')
})
mocks.release.mockResolvedValue(false)
await startPreparation(args)
await vi.advanceTimersByTimeAsync(1_100)
expect(mocks.recordPid).toHaveBeenCalledWith(12345)
await expect(takePreparation(listPreparations()[0])).resolves.toBe(false)
})
+9 -2
View File
@@ -81,13 +81,20 @@ export function createPreparedIndexWarming(
if (terminationUnverifiable) {
return false
}
let released: boolean
try {
await ownership.release()
return true
released = await ownership.release()
} catch {
disablePreparedIndexWarming()
return false
}
if (!released) {
disablePreparedIndexWarming()
console.warn(
`[worktree-create] index warming process group live or unverifiable after Git exit; retaining ${preparedPath} and disabling warming`
)
}
return released
})()
}, INDEX_TIMESTAMP_AGE_MS)
timer.unref()
+5 -1
View File
@@ -411,7 +411,11 @@ describe('renderer startup runtime routing', () => {
const shellSource = readSource(WORKSPACE_SHELL_PATH)
const layoutSource = readSource(CHROME_LAYOUT_PATH)
expect(shellSource).toContain("const Terminal = lazy(() => import('../components/Terminal'))")
const loaderSource = readSource('src/renderer/src/lib/terminal-component-loader.ts')
expect(shellSource).toContain("from '@/lib/terminal-component-loader'")
expect(shellSource).toContain('const Terminal = lazy(loadTerminalComponent)')
expect(loaderSource).toContain("() => import('../components/Terminal')")
expect(loaderSource).not.toContain("from '../components/Terminal'")
expect(shellSource).not.toContain("from '../components/Terminal'")
expect(layoutSource).toContain(
'const canMountTerminalWorkbenchNow = activeWorktreeId !== null || backgroundTerminalMountRequested'