test(git): align admission lifetime contracts

This commit is contained in:
Brennan Benson
2026-08-27 17:45:14 -07:00
parent b990e0a80f
commit bd3cc2f98c
6 changed files with 42 additions and 16 deletions
+8 -4
View File
@@ -67,6 +67,8 @@ export async function withGitAdmission(
return
}
finalized = true
child.off('error', handleError)
child.off('close', handleClose)
grant?.release()
if (error) {
span.fail(error)
@@ -74,21 +76,23 @@ export async function withGitAdmission(
span.end()
}
}
child.on('error', (error) => {
const handleError = (error: Error): void => {
if (!child.pid) {
finalize(error)
} else {
liveError = error
}
})
child.once('close', (code, signal) => {
}
const handleClose = (code: number | null, signal: NodeJS.Signals | null): void => {
const exitError =
liveError ??
(code === 0 && signal === null
? undefined
: new Error(`git exited with ${code ?? signal ?? 'unknown'}.`))
finalize(exitError)
})
}
child.on('error', handleError)
child.once('close', handleClose)
return child
} catch (error) {
grant?.release()
@@ -242,7 +242,7 @@ describe('registerFilesystemHandlers', () => {
filePath: path.join('src', 'file.ts'),
oldPath: path.join('src', 'old-file.ts')
},
{}
{ admissionTier: 'interactive' }
)
})
@@ -96,7 +96,9 @@ describe('registerFilesystemHandlers', () => {
})
).resolves.toEqual({ success: true, message: 'Update README' })
expect(getStagedCommitContextMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {})
expect(getStagedCommitContextMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {
admissionTier: 'interactive'
})
expect(generateCommitMessageFromContextMock).toHaveBeenCalledWith(context, params, {
kind: 'local',
cwd: WORKTREE_FEATURE_PATH
@@ -253,6 +255,7 @@ describe('registerFilesystemHandlers', () => {
})
expect(getStagedCommitContextMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {
admissionTier: 'interactive',
wslDistro: 'Ubuntu'
})
expect(prepareForCodexLaunch).toHaveBeenCalledWith({
@@ -80,7 +80,9 @@ describe('registerFilesystemHandlers', () => {
})
).resolves.toEqual({ success: true })
expect(commitChangesMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, 'feat: ship commit', {})
expect(commitChangesMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, 'feat: ship commit', {
admissionTier: 'interactive'
})
})
it('returns local commit hook failure payload from git:commit', async () => {
@@ -94,11 +94,9 @@ describe('registerFilesystemHandlers', () => {
// Why: validateGitRelativeFilePath uses path.relative() which produces
// platform-specific separators (backslashes on Windows).
expect(stageFileMock).toHaveBeenCalledWith(
WORKTREE_FEATURE_PATH,
path.join('src', 'file.ts'),
{}
)
expect(stageFileMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, path.join('src', 'file.ts'), {
admissionTier: 'interactive'
})
})
it('uses worktree roots seeded by worktrees:list without rebuilding the cache', async () => {
@@ -346,7 +344,9 @@ describe('registerFilesystemHandlers', () => {
connectionId: 'ssh-1'
})
expect(abortMergeMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {})
expect(abortMergeMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {
admissionTier: 'interactive'
})
expect(sshProvider.abortMerge).toHaveBeenCalledWith('/remote/repo')
})
@@ -366,7 +366,9 @@ describe('registerFilesystemHandlers', () => {
connectionId: 'ssh-1'
})
expect(abortRebaseMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {})
expect(abortRebaseMock).toHaveBeenCalledWith(WORKTREE_FEATURE_PATH, {
admissionTier: 'interactive'
})
expect(sshProvider.abortRebase).toHaveBeenCalledWith('/remote/repo')
})
@@ -410,7 +412,7 @@ describe('registerFilesystemHandlers', () => {
expect(bulkStageFilesMock).toHaveBeenCalledWith(
WORKTREE_FEATURE_PATH,
[path.join('src', 'file.ts'), path.join('nested', 'child.ts')],
{}
{ admissionTier: 'interactive' }
)
})
@@ -427,7 +429,7 @@ describe('registerFilesystemHandlers', () => {
expect(bulkDiscardChangesMock).toHaveBeenCalledWith(
WORKTREE_FEATURE_PATH,
[path.join('src', 'file.ts'), path.join('nested', 'child.ts')],
{}
{ admissionTier: 'interactive' }
)
})
@@ -37,6 +37,7 @@ import { EventEmitter } from 'node:events'
import type { Store } from '../persistence'
import type { ChildProcess } from 'node:child_process'
import { FileListingCancelledError } from '../../shared/file-listing-cancellation'
import { _gitAdmissionSnapshotForTests } from '../git/command-runner/git-subprocess-admission'
const SHA1 = '0123456789abcdef0123456789abcdef01234567'
@@ -603,6 +604,7 @@ describe('filesystem-list-files', () => {
await expect(promise).resolves.toEqual(['src/kept.ts'])
expect(primary.kill).toHaveBeenCalled()
primary.emit('close', null, 'SIGTERM')
})
it('git fallback applies hidden dir blocklist', async () => {
@@ -695,6 +697,15 @@ describe('filesystem-list-files', () => {
expect(gitP2.kill).toHaveBeenCalled()
expect((gitP1.stdout as unknown as EventEmitter).listenerCount('data')).toBe(0)
expect((gitP1.stderr as unknown as EventEmitter).listenerCount('data')).toBe(0)
// Admission retains live children after the listing listeners detach.
const admittedBeforeClose = _gitAdmissionSnapshotForTests().budgets.general?.baseUsed
expect(gitP1.listenerCount('error')).toBe(1)
expect(gitP1.listenerCount('close')).toBe(1)
gitP1.emit('close', null, 'SIGKILL')
gitP2.emit('close', null, 'SIGKILL')
expect(admittedBeforeClose).toBe(2)
expect(_gitAdmissionSnapshotForTests().budgets.general?.baseUsed).toBe(0)
expect(gitP1.listenerCount('error')).toBe(0)
expect(gitP1.listenerCount('close')).toBe(0)
} finally {
@@ -748,6 +759,10 @@ describe('filesystem-list-files', () => {
)
expect(gitP2.kill).toHaveBeenCalled()
expect(warnSpy).toHaveBeenCalled()
const admittedBeforeClose = _gitAdmissionSnapshotForTests().budgets.general?.baseUsed
gitP2.emit('close', null, 'SIGKILL')
expect(admittedBeforeClose).toBe(1)
expect(_gitAdmissionSnapshotForTests().budgets.general?.baseUsed).toBe(0)
} finally {
vi.useRealTimers()
warnSpy.mockRestore()