Skip to content

Commit e1fbda4

Browse files
committed
Extract branch cleanup logic into dedicated module with tests
Extract branch cleanup operations into a separate module to improve code organization and testability. Each cleanup function has a specific purpose based on the branch lifecycle stage. Added functions: - cleanupStaleBranch: Delete both remote and local for branches without PRs - cleanupFailedPrBranches: Clean up after PR creation failure - cleanupSuccessfulPrLocalBranch: Remove only local branch after PR success - cleanupErrorBranches: Handle cleanup after unexpected errors Tests: - 8 unit tests with mocked git functions - 5 integration tests with real git repositories in tmpdir
1 parent 627a0ba commit e1fbda4

File tree

3 files changed

+534
-0
lines changed

3 files changed

+534
-0
lines changed
Lines changed: 282 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,282 @@
1+
import { promises as fs } from 'node:fs'
2+
import { tmpdir } from 'node:os'
3+
import path from 'node:path'
4+
5+
import trash from 'trash'
6+
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
7+
8+
import { spawn } from '@socketsecurity/registry/lib/spawn'
9+
10+
import {
11+
cleanupErrorBranches,
12+
cleanupFailedPrBranches,
13+
cleanupStaleBranch,
14+
cleanupSuccessfulPrLocalBranch,
15+
} from './branch-cleanup.mts'
16+
import {
17+
gitCreateBranch,
18+
gitDeleteBranch,
19+
gitDeleteRemoteBranch,
20+
gitRemoteBranchExists,
21+
} from '../../utils/git.mts'
22+
23+
describe('branch-cleanup integration tests', () => {
24+
let tempDir: string
25+
let repoDir: string
26+
let remoteDir: string
27+
28+
beforeEach(async () => {
29+
// Create a temporary directory with unique name.
30+
tempDir = path.join(
31+
tmpdir(),
32+
`socket-branch-cleanup-test-${Date.now()}-${Math.random().toString(36).slice(2)}`,
33+
)
34+
await fs.mkdir(tempDir, { recursive: true })
35+
36+
// Create separate directories for remote and local repos.
37+
remoteDir = path.join(tempDir, 'remote.git')
38+
repoDir = path.join(tempDir, 'repo')
39+
40+
// Initialize bare remote repository.
41+
await fs.mkdir(remoteDir, { recursive: true })
42+
await spawn('git', ['init', '--bare'], { cwd: remoteDir, stdio: 'ignore' })
43+
44+
// Clone the remote to create local repository.
45+
await spawn('git', ['clone', remoteDir, repoDir], {
46+
cwd: tempDir,
47+
stdio: 'ignore',
48+
})
49+
50+
// Configure git user for commits.
51+
await spawn('git', ['config', 'user.email', 'test@socket-cli.test'], {
52+
cwd: repoDir,
53+
stdio: 'ignore',
54+
})
55+
await spawn('git', ['config', 'user.name', 'Socket CLI Test'], {
56+
cwd: repoDir,
57+
stdio: 'ignore',
58+
})
59+
60+
// Create initial commit on main branch.
61+
await fs.writeFile(path.join(repoDir, 'README.md'), '# Test Repo\n')
62+
await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' })
63+
await spawn('git', ['commit', '-m', 'Initial commit'], {
64+
cwd: repoDir,
65+
stdio: 'ignore',
66+
})
67+
await spawn('git', ['push', 'origin', 'main'], {
68+
cwd: repoDir,
69+
stdio: 'ignore',
70+
})
71+
})
72+
73+
afterEach(async () => {
74+
// Clean up temp directory.
75+
if (tempDir) {
76+
try {
77+
await trash(tempDir)
78+
} catch (e) {
79+
// Ignore cleanup errors.
80+
}
81+
}
82+
})
83+
84+
describe('cleanupStaleBranch', () => {
85+
it('should delete both remote and local stale branches when remote deletion succeeds', async () => {
86+
const branchName = 'socket-fix/GHSA-test-1'
87+
88+
// Create and push a branch.
89+
await gitCreateBranch(branchName, repoDir)
90+
await spawn('git', ['checkout', branchName], {
91+
cwd: repoDir,
92+
stdio: 'ignore',
93+
})
94+
await fs.writeFile(path.join(repoDir, 'test.txt'), 'test')
95+
await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' })
96+
await spawn('git', ['commit', '-m', 'Test commit'], {
97+
cwd: repoDir,
98+
stdio: 'ignore',
99+
})
100+
await spawn('git', ['push', 'origin', branchName], {
101+
cwd: repoDir,
102+
stdio: 'ignore',
103+
})
104+
await spawn('git', ['checkout', 'main'], {
105+
cwd: repoDir,
106+
stdio: 'ignore',
107+
})
108+
109+
// Verify branch exists remotely.
110+
const existsBefore = await gitRemoteBranchExists(branchName, repoDir)
111+
expect(existsBefore).toBe(true)
112+
113+
// Clean up stale branch.
114+
const result = await cleanupStaleBranch(
115+
branchName,
116+
'GHSA-test-1',
117+
repoDir,
118+
)
119+
120+
expect(result).toBe(true)
121+
122+
// Verify remote branch is deleted.
123+
const existsAfter = await gitRemoteBranchExists(branchName, repoDir)
124+
expect(existsAfter).toBe(false)
125+
126+
// Verify local branch is also deleted.
127+
const { stdout } = await spawn('git', ['branch', '--list', branchName], {
128+
cwd: repoDir,
129+
stdio: 'pipe',
130+
})
131+
expect(stdout.trim()).toBe('')
132+
})
133+
})
134+
135+
describe('cleanupFailedPrBranches', () => {
136+
it('should delete both remote and local branches', async () => {
137+
const branchName = 'socket-fix/GHSA-test-2'
138+
139+
// Create and push a branch.
140+
await gitCreateBranch(branchName, repoDir)
141+
await spawn('git', ['checkout', branchName], {
142+
cwd: repoDir,
143+
stdio: 'ignore',
144+
})
145+
await fs.writeFile(path.join(repoDir, 'test.txt'), 'test')
146+
await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' })
147+
await spawn('git', ['commit', '-m', 'Test commit'], {
148+
cwd: repoDir,
149+
stdio: 'ignore',
150+
})
151+
await spawn('git', ['push', 'origin', branchName], {
152+
cwd: repoDir,
153+
stdio: 'ignore',
154+
})
155+
await spawn('git', ['checkout', 'main'], {
156+
cwd: repoDir,
157+
stdio: 'ignore',
158+
})
159+
160+
// Clean up failed PR branches.
161+
await cleanupFailedPrBranches(branchName, repoDir)
162+
163+
// Verify remote branch is deleted.
164+
const existsAfter = await gitRemoteBranchExists(branchName, repoDir)
165+
expect(existsAfter).toBe(false)
166+
167+
// Verify local branch is also deleted.
168+
const { stdout } = await spawn('git', ['branch', '--list', branchName], {
169+
cwd: repoDir,
170+
stdio: 'pipe',
171+
})
172+
expect(stdout.trim()).toBe('')
173+
})
174+
})
175+
176+
describe('cleanupSuccessfulPrLocalBranch', () => {
177+
it('should delete only local branch and keep remote', async () => {
178+
const branchName = 'socket-fix/GHSA-test-3'
179+
180+
// Create and push a branch.
181+
await gitCreateBranch(branchName, repoDir)
182+
await spawn('git', ['checkout', branchName], {
183+
cwd: repoDir,
184+
stdio: 'ignore',
185+
})
186+
await fs.writeFile(path.join(repoDir, 'test.txt'), 'test')
187+
await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' })
188+
await spawn('git', ['commit', '-m', 'Test commit'], {
189+
cwd: repoDir,
190+
stdio: 'ignore',
191+
})
192+
await spawn('git', ['push', 'origin', branchName], {
193+
cwd: repoDir,
194+
stdio: 'ignore',
195+
})
196+
await spawn('git', ['checkout', 'main'], {
197+
cwd: repoDir,
198+
stdio: 'ignore',
199+
})
200+
201+
// Clean up local branch only.
202+
await cleanupSuccessfulPrLocalBranch(branchName, repoDir)
203+
204+
// Verify remote branch still exists.
205+
const remoteExists = await gitRemoteBranchExists(branchName, repoDir)
206+
expect(remoteExists).toBe(true)
207+
208+
// Verify local branch is deleted.
209+
const { stdout } = await spawn('git', ['branch', '--list', branchName], {
210+
cwd: repoDir,
211+
stdio: 'pipe',
212+
})
213+
expect(stdout.trim()).toBe('')
214+
})
215+
})
216+
217+
describe('cleanupErrorBranches', () => {
218+
it('should delete both branches when remote exists', async () => {
219+
const branchName = 'socket-fix/GHSA-test-4'
220+
221+
// Create and push a branch.
222+
await gitCreateBranch(branchName, repoDir)
223+
await spawn('git', ['checkout', branchName], {
224+
cwd: repoDir,
225+
stdio: 'ignore',
226+
})
227+
await fs.writeFile(path.join(repoDir, 'test.txt'), 'test')
228+
await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' })
229+
await spawn('git', ['commit', '-m', 'Test commit'], {
230+
cwd: repoDir,
231+
stdio: 'ignore',
232+
})
233+
await spawn('git', ['push', 'origin', branchName], {
234+
cwd: repoDir,
235+
stdio: 'ignore',
236+
})
237+
await spawn('git', ['checkout', 'main'], {
238+
cwd: repoDir,
239+
stdio: 'ignore',
240+
})
241+
242+
// Clean up error branches (remote exists).
243+
await cleanupErrorBranches(branchName, repoDir, true)
244+
245+
// Verify remote branch is deleted.
246+
const remoteExists = await gitRemoteBranchExists(branchName, repoDir)
247+
expect(remoteExists).toBe(false)
248+
249+
// Verify local branch is deleted.
250+
const { stdout } = await spawn('git', ['branch', '--list', branchName], {
251+
cwd: repoDir,
252+
stdio: 'pipe',
253+
})
254+
expect(stdout.trim()).toBe('')
255+
})
256+
257+
it('should delete only local branch when remote does not exist', async () => {
258+
const branchName = 'socket-fix/GHSA-test-5'
259+
260+
// Create local branch but don't push.
261+
await gitCreateBranch(branchName, repoDir)
262+
await spawn('git', ['checkout', 'main'], {
263+
cwd: repoDir,
264+
stdio: 'ignore',
265+
})
266+
267+
// Clean up error branches (remote does not exist).
268+
await cleanupErrorBranches(branchName, repoDir, false)
269+
270+
// Verify remote branch still doesn't exist.
271+
const remoteExists = await gitRemoteBranchExists(branchName, repoDir)
272+
expect(remoteExists).toBe(false)
273+
274+
// Verify local branch is deleted.
275+
const { stdout } = await spawn('git', ['branch', '--list', branchName], {
276+
cwd: repoDir,
277+
stdio: 'pipe',
278+
})
279+
expect(stdout.trim()).toBe('')
280+
})
281+
})
282+
})
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* Branch cleanup utilities for socket fix command.
3+
* Manages local and remote branch lifecycle during PR creation.
4+
*
5+
* Critical distinction: Remote branches are sacred when a PR exists, disposable when they don't.
6+
*/
7+
8+
import { debugFn } from '@socketsecurity/registry/lib/debug'
9+
import { logger } from '@socketsecurity/registry/lib/logger'
10+
11+
import { gitDeleteBranch, gitDeleteRemoteBranch } from '../../utils/git.mts'
12+
13+
/**
14+
* Clean up a stale branch (both remote and local).
15+
* Safe to delete both since no PR exists for this branch.
16+
*
17+
* Returns true if cleanup succeeded or should continue, false if should skip GHSA.
18+
*/
19+
export async function cleanupStaleBranch(
20+
branch: string,
21+
ghsaId: string,
22+
cwd: string,
23+
): Promise<boolean> {
24+
logger.warn(`Stale branch ${branch} found without open PR, cleaning up...`)
25+
debugFn('notice', `cleanup: deleting stale branch ${branch}`)
26+
27+
const deleted = await gitDeleteRemoteBranch(branch, cwd)
28+
if (!deleted) {
29+
logger.error(
30+
`Failed to delete stale remote branch ${branch}, skipping ${ghsaId}.`,
31+
)
32+
debugFn('error', `cleanup: remote deletion failed for ${branch}`)
33+
return false
34+
}
35+
36+
// Clean up local branch too to avoid conflicts.
37+
await gitDeleteBranch(branch, cwd)
38+
return true
39+
}
40+
41+
/**
42+
* Clean up branches after PR creation failure.
43+
* Safe to delete both remote and local since no PR was created.
44+
*/
45+
export async function cleanupFailedPrBranches(
46+
branch: string,
47+
cwd: string,
48+
): Promise<void> {
49+
// Clean up pushed branch since PR creation failed.
50+
// Safe to delete both remote and local since no PR exists.
51+
await gitDeleteRemoteBranch(branch, cwd)
52+
await gitDeleteBranch(branch, cwd)
53+
}
54+
55+
/**
56+
* Clean up local branch after successful PR creation.
57+
* Keeps remote branch - PR needs it to be mergeable.
58+
*/
59+
export async function cleanupSuccessfulPrLocalBranch(
60+
branch: string,
61+
cwd: string,
62+
): Promise<void> {
63+
// Clean up local branch only - keep remote branch for PR merge.
64+
await gitDeleteBranch(branch, cwd)
65+
}
66+
67+
/**
68+
* Clean up branches in catch block after unexpected error.
69+
* Safe to delete both remote and local since no PR was created.
70+
*/
71+
export async function cleanupErrorBranches(
72+
branch: string,
73+
cwd: string,
74+
remoteBranchExists: boolean,
75+
): Promise<void> {
76+
// Clean up remote branch if it exists (push may have succeeded before error).
77+
// Safe to delete both remote and local since no PR was created.
78+
if (remoteBranchExists) {
79+
await gitDeleteRemoteBranch(branch, cwd)
80+
}
81+
await gitDeleteBranch(branch, cwd)
82+
}

0 commit comments

Comments
 (0)