diff --git a/src/commands/fix/branch-cleanup.integration.test.mts b/src/commands/fix/branch-cleanup.integration.test.mts new file mode 100644 index 000000000..c9c83f46d --- /dev/null +++ b/src/commands/fix/branch-cleanup.integration.test.mts @@ -0,0 +1,282 @@ +import { promises as fs } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' + +import trash from 'trash' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { spawn } from '@socketsecurity/registry/lib/spawn' + +import { + cleanupErrorBranches, + cleanupFailedPrBranches, + cleanupStaleBranch, + cleanupSuccessfulPrLocalBranch, +} from './branch-cleanup.mts' +import { + gitCreateBranch, + gitDeleteBranch, + gitDeleteRemoteBranch, + gitRemoteBranchExists, +} from '../../utils/git.mts' + +describe('branch-cleanup integration tests', () => { + let tempDir: string + let repoDir: string + let remoteDir: string + + beforeEach(async () => { + // Create a temporary directory with unique name. + tempDir = path.join( + tmpdir(), + `socket-branch-cleanup-test-${Date.now()}-${Math.random().toString(36).slice(2)}`, + ) + await fs.mkdir(tempDir, { recursive: true }) + + // Create separate directories for remote and local repos. + remoteDir = path.join(tempDir, 'remote.git') + repoDir = path.join(tempDir, 'repo') + + // Initialize bare remote repository. + await fs.mkdir(remoteDir, { recursive: true }) + await spawn('git', ['init', '--bare'], { cwd: remoteDir, stdio: 'ignore' }) + + // Clone the remote to create local repository. + await spawn('git', ['clone', remoteDir, repoDir], { + cwd: tempDir, + stdio: 'ignore', + }) + + // Configure git user for commits. + await spawn('git', ['config', 'user.email', 'test@socket-cli.test'], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['config', 'user.name', 'Socket CLI Test'], { + cwd: repoDir, + stdio: 'ignore', + }) + + // Create initial commit on main branch. + await fs.writeFile(path.join(repoDir, 'README.md'), '# Test Repo\n') + await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' }) + await spawn('git', ['commit', '-m', 'Initial commit'], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['push', 'origin', 'main'], { + cwd: repoDir, + stdio: 'ignore', + }) + }) + + afterEach(async () => { + // Clean up temp directory. + if (tempDir) { + try { + await trash(tempDir) + } catch (e) { + // Ignore cleanup errors. + } + } + }) + + describe('cleanupStaleBranch', () => { + it('should delete both remote and local stale branches when remote deletion succeeds', async () => { + const branchName = 'socket-fix/GHSA-test-1' + + // Create and push a branch. + await gitCreateBranch(branchName, repoDir) + await spawn('git', ['checkout', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await fs.writeFile(path.join(repoDir, 'test.txt'), 'test') + await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' }) + await spawn('git', ['commit', '-m', 'Test commit'], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['push', 'origin', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['checkout', 'main'], { + cwd: repoDir, + stdio: 'ignore', + }) + + // Verify branch exists remotely. + const existsBefore = await gitRemoteBranchExists(branchName, repoDir) + expect(existsBefore).toBe(true) + + // Clean up stale branch. + const result = await cleanupStaleBranch( + branchName, + 'GHSA-test-1', + repoDir, + ) + + expect(result).toBe(true) + + // Verify remote branch is deleted. + const existsAfter = await gitRemoteBranchExists(branchName, repoDir) + expect(existsAfter).toBe(false) + + // Verify local branch is also deleted. + const { stdout } = await spawn('git', ['branch', '--list', branchName], { + cwd: repoDir, + stdio: 'pipe', + }) + expect(stdout.trim()).toBe('') + }) + }) + + describe('cleanupFailedPrBranches', () => { + it('should delete both remote and local branches', async () => { + const branchName = 'socket-fix/GHSA-test-2' + + // Create and push a branch. + await gitCreateBranch(branchName, repoDir) + await spawn('git', ['checkout', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await fs.writeFile(path.join(repoDir, 'test.txt'), 'test') + await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' }) + await spawn('git', ['commit', '-m', 'Test commit'], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['push', 'origin', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['checkout', 'main'], { + cwd: repoDir, + stdio: 'ignore', + }) + + // Clean up failed PR branches. + await cleanupFailedPrBranches(branchName, repoDir) + + // Verify remote branch is deleted. + const existsAfter = await gitRemoteBranchExists(branchName, repoDir) + expect(existsAfter).toBe(false) + + // Verify local branch is also deleted. + const { stdout } = await spawn('git', ['branch', '--list', branchName], { + cwd: repoDir, + stdio: 'pipe', + }) + expect(stdout.trim()).toBe('') + }) + }) + + describe('cleanupSuccessfulPrLocalBranch', () => { + it('should delete only local branch and keep remote', async () => { + const branchName = 'socket-fix/GHSA-test-3' + + // Create and push a branch. + await gitCreateBranch(branchName, repoDir) + await spawn('git', ['checkout', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await fs.writeFile(path.join(repoDir, 'test.txt'), 'test') + await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' }) + await spawn('git', ['commit', '-m', 'Test commit'], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['push', 'origin', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['checkout', 'main'], { + cwd: repoDir, + stdio: 'ignore', + }) + + // Clean up local branch only. + await cleanupSuccessfulPrLocalBranch(branchName, repoDir) + + // Verify remote branch still exists. + const remoteExists = await gitRemoteBranchExists(branchName, repoDir) + expect(remoteExists).toBe(true) + + // Verify local branch is deleted. + const { stdout } = await spawn('git', ['branch', '--list', branchName], { + cwd: repoDir, + stdio: 'pipe', + }) + expect(stdout.trim()).toBe('') + }) + }) + + describe('cleanupErrorBranches', () => { + it('should delete both branches when remote exists', async () => { + const branchName = 'socket-fix/GHSA-test-4' + + // Create and push a branch. + await gitCreateBranch(branchName, repoDir) + await spawn('git', ['checkout', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await fs.writeFile(path.join(repoDir, 'test.txt'), 'test') + await spawn('git', ['add', '.'], { cwd: repoDir, stdio: 'ignore' }) + await spawn('git', ['commit', '-m', 'Test commit'], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['push', 'origin', branchName], { + cwd: repoDir, + stdio: 'ignore', + }) + await spawn('git', ['checkout', 'main'], { + cwd: repoDir, + stdio: 'ignore', + }) + + // Clean up error branches (remote exists). + await cleanupErrorBranches(branchName, repoDir, true) + + // Verify remote branch is deleted. + const remoteExists = await gitRemoteBranchExists(branchName, repoDir) + expect(remoteExists).toBe(false) + + // Verify local branch is deleted. + const { stdout } = await spawn('git', ['branch', '--list', branchName], { + cwd: repoDir, + stdio: 'pipe', + }) + expect(stdout.trim()).toBe('') + }) + + it('should delete only local branch when remote does not exist', async () => { + const branchName = 'socket-fix/GHSA-test-5' + + // Create local branch but don't push. + await gitCreateBranch(branchName, repoDir) + await spawn('git', ['checkout', 'main'], { + cwd: repoDir, + stdio: 'ignore', + }) + + // Clean up error branches (remote does not exist). + await cleanupErrorBranches(branchName, repoDir, false) + + // Verify remote branch still doesn't exist. + const remoteExists = await gitRemoteBranchExists(branchName, repoDir) + expect(remoteExists).toBe(false) + + // Verify local branch is deleted. + const { stdout } = await spawn('git', ['branch', '--list', branchName], { + cwd: repoDir, + stdio: 'pipe', + }) + expect(stdout.trim()).toBe('') + }) + }) +}) diff --git a/src/commands/fix/branch-cleanup.mts b/src/commands/fix/branch-cleanup.mts new file mode 100644 index 000000000..7493fdb03 --- /dev/null +++ b/src/commands/fix/branch-cleanup.mts @@ -0,0 +1,82 @@ +/** + * Branch cleanup utilities for socket fix command. + * Manages local and remote branch lifecycle during PR creation. + * + * Critical distinction: Remote branches are sacred when a PR exists, disposable when they don't. + */ + +import { debugFn } from '@socketsecurity/registry/lib/debug' +import { logger } from '@socketsecurity/registry/lib/logger' + +import { gitDeleteBranch, gitDeleteRemoteBranch } from '../../utils/git.mts' + +/** + * Clean up a stale branch (both remote and local). + * Safe to delete both since no PR exists for this branch. + * + * Returns true if cleanup succeeded or should continue, false if should skip GHSA. + */ +export async function cleanupStaleBranch( + branch: string, + ghsaId: string, + cwd: string, +): Promise { + logger.warn(`Stale branch ${branch} found without open PR, cleaning up...`) + debugFn('notice', `cleanup: deleting stale branch ${branch}`) + + const deleted = await gitDeleteRemoteBranch(branch, cwd) + if (!deleted) { + logger.error( + `Failed to delete stale remote branch ${branch}, skipping ${ghsaId}.`, + ) + debugFn('error', `cleanup: remote deletion failed for ${branch}`) + return false + } + + // Clean up local branch too to avoid conflicts. + await gitDeleteBranch(branch, cwd) + return true +} + +/** + * Clean up branches after PR creation failure. + * Safe to delete both remote and local since no PR was created. + */ +export async function cleanupFailedPrBranches( + branch: string, + cwd: string, +): Promise { + // Clean up pushed branch since PR creation failed. + // Safe to delete both remote and local since no PR exists. + await gitDeleteRemoteBranch(branch, cwd) + await gitDeleteBranch(branch, cwd) +} + +/** + * Clean up local branch after successful PR creation. + * Keeps remote branch - PR needs it to be mergeable. + */ +export async function cleanupSuccessfulPrLocalBranch( + branch: string, + cwd: string, +): Promise { + // Clean up local branch only - keep remote branch for PR merge. + await gitDeleteBranch(branch, cwd) +} + +/** + * Clean up branches in catch block after unexpected error. + * Safe to delete both remote and local since no PR was created. + */ +export async function cleanupErrorBranches( + branch: string, + cwd: string, + remoteBranchExists: boolean, +): Promise { + // Clean up remote branch if it exists (push may have succeeded before error). + // Safe to delete both remote and local since no PR was created. + if (remoteBranchExists) { + await gitDeleteRemoteBranch(branch, cwd) + } + await gitDeleteBranch(branch, cwd) +} diff --git a/src/commands/fix/branch-cleanup.test.mts b/src/commands/fix/branch-cleanup.test.mts new file mode 100644 index 000000000..61e4f6c3f --- /dev/null +++ b/src/commands/fix/branch-cleanup.test.mts @@ -0,0 +1,170 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { + cleanupErrorBranches, + cleanupFailedPrBranches, + cleanupStaleBranch, + cleanupSuccessfulPrLocalBranch, +} from './branch-cleanup.mts' + +const mockLogger = vi.hoisted(() => ({ + error: vi.fn(), + warn: vi.fn(), +})) + +const mockDebugFn = vi.hoisted(() => vi.fn()) + +const mockGitDeleteBranch = vi.hoisted(() => vi.fn()) +const mockGitDeleteRemoteBranch = vi.hoisted(() => vi.fn()) + +vi.mock('@socketsecurity/registry/lib/logger', () => ({ + logger: mockLogger, +})) + +vi.mock('@socketsecurity/registry/lib/debug', () => ({ + debugFn: mockDebugFn, +})) + +vi.mock('../../utils/git.mts', () => ({ + gitDeleteBranch: mockGitDeleteBranch, + gitDeleteRemoteBranch: mockGitDeleteRemoteBranch, +})) + +describe('branch-cleanup', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGitDeleteBranch.mockResolvedValue(true) + mockGitDeleteRemoteBranch.mockResolvedValue(true) + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe('cleanupStaleBranch', () => { + it('should return true and delete both branches when remote deletion succeeds', async () => { + const result = await cleanupStaleBranch( + 'socket-fix/GHSA-test', + 'GHSA-test', + '/test/repo', + ) + + expect(result).toBe(true) + expect(mockGitDeleteRemoteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + expect(mockGitDeleteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('Stale branch'), + ) + }) + + it('should return false and skip local deletion when remote deletion fails', async () => { + mockGitDeleteRemoteBranch.mockResolvedValue(false) + + const result = await cleanupStaleBranch( + 'socket-fix/GHSA-test', + 'GHSA-test', + '/test/repo', + ) + + expect(result).toBe(false) + expect(mockGitDeleteRemoteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + expect(mockGitDeleteBranch).not.toHaveBeenCalled() + expect(mockLogger.error).toHaveBeenCalledWith( + expect.stringContaining('Failed to delete stale remote branch'), + ) + }) + }) + + describe('cleanupFailedPrBranches', () => { + it('should delete both remote and local branches', async () => { + await cleanupFailedPrBranches('socket-fix/GHSA-test', '/test/repo') + + expect(mockGitDeleteRemoteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + expect(mockGitDeleteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + }) + + it('should call functions in correct order (remote first, then local)', async () => { + const calls: string[] = [] + mockGitDeleteRemoteBranch.mockImplementation(async () => { + calls.push('remote') + return true + }) + mockGitDeleteBranch.mockImplementation(async () => { + calls.push('local') + return true + }) + + await cleanupFailedPrBranches('socket-fix/GHSA-test', '/test/repo') + + expect(calls).toEqual(['remote', 'local']) + }) + }) + + describe('cleanupSuccessfulPrLocalBranch', () => { + it('should only delete local branch', async () => { + await cleanupSuccessfulPrLocalBranch('socket-fix/GHSA-test', '/test/repo') + + expect(mockGitDeleteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + expect(mockGitDeleteRemoteBranch).not.toHaveBeenCalled() + }) + }) + + describe('cleanupErrorBranches', () => { + it('should delete both remote and local when remote exists', async () => { + await cleanupErrorBranches('socket-fix/GHSA-test', '/test/repo', true) + + expect(mockGitDeleteRemoteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + expect(mockGitDeleteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + }) + + it('should only delete local when remote does not exist', async () => { + await cleanupErrorBranches('socket-fix/GHSA-test', '/test/repo', false) + + expect(mockGitDeleteRemoteBranch).not.toHaveBeenCalled() + expect(mockGitDeleteBranch).toHaveBeenCalledWith( + 'socket-fix/GHSA-test', + '/test/repo', + ) + }) + + it('should call functions in correct order when remote exists', async () => { + const calls: string[] = [] + mockGitDeleteRemoteBranch.mockImplementation(async () => { + calls.push('remote') + return true + }) + mockGitDeleteBranch.mockImplementation(async () => { + calls.push('local') + return true + }) + + await cleanupErrorBranches('socket-fix/GHSA-test', '/test/repo', true) + + expect(calls).toEqual(['remote', 'local']) + }) + }) +}) diff --git a/src/commands/fix/coana-fix.mts b/src/commands/fix/coana-fix.mts index 732dfa65c..975869898 100644 --- a/src/commands/fix/coana-fix.mts +++ b/src/commands/fix/coana-fix.mts @@ -8,6 +8,12 @@ import { readJsonSync } from '@socketsecurity/registry/lib/fs' import { logger } from '@socketsecurity/registry/lib/logger' import { pluralize } from '@socketsecurity/registry/lib/words' +import { + cleanupErrorBranches, + cleanupFailedPrBranches, + cleanupStaleBranch, + cleanupSuccessfulPrLocalBranch, +} from './branch-cleanup.mts' import { checkCiEnvVars, getCiEnvInstructions, @@ -341,10 +347,39 @@ export async function coanaFix( const branch = getSocketFixBranchName(ghsaId) try { - // Check if branch already exists. + // Check if an open PR already exists for this GHSA. + // eslint-disable-next-line no-await-in-loop + const existingOpenPrs = await getSocketFixPrs( + fixEnv.repoInfo.owner, + fixEnv.repoInfo.repo, + { ghsaId, states: GQL_PR_STATE_OPEN }, + ) + + if (existingOpenPrs.length > 0) { + const prNum = existingOpenPrs[0]!.number + logger.info(`PR #${prNum} already exists for ${ghsaId}, skipping.`) + debugFn('notice', `skip: open PR #${prNum} exists for ${ghsaId}`) + continue ghsaLoop + } + + // If branch exists but no open PR, delete the stale branch. + // This handles cases where PR creation failed but branch was pushed. // eslint-disable-next-line no-await-in-loop if (await gitRemoteBranchExists(branch, cwd)) { - debugFn('notice', `skip: remote branch "${branch}" exists`) + // eslint-disable-next-line no-await-in-loop + const shouldContinue = await cleanupStaleBranch(branch, ghsaId, cwd) + if (!shouldContinue) { + continue ghsaLoop + } + } + + // Check for GitHub token before doing any git operations. + if (!fixEnv.githubToken) { + logger.error( + 'Cannot create pull request: SOCKET_CLI_GITHUB_TOKEN environment variable is not set.\n' + + 'Set SOCKET_CLI_GITHUB_TOKEN or GITHUB_TOKEN to enable PR creation.', + ) + debugFn('error', `skip: missing GitHub token for ${ghsaId}`) continue ghsaLoop } @@ -386,19 +421,6 @@ export async function coanaFix( } // Set up git remote. - if (!fixEnv.githubToken) { - logger.error( - 'Cannot create pull request: SOCKET_CLI_GITHUB_TOKEN environment variable is not set.\n' + - 'Set SOCKET_CLI_GITHUB_TOKEN or GITHUB_TOKEN to enable PR creation.', - ) - // eslint-disable-next-line no-await-in-loop - await gitResetAndClean(fixEnv.baseBranch, cwd) - // eslint-disable-next-line no-await-in-loop - await gitCheckoutBranch(fixEnv.baseBranch, cwd) - // eslint-disable-next-line no-await-in-loop - await gitDeleteBranch(branch, cwd) - continue ghsaLoop - } // eslint-disable-next-line no-await-in-loop await setGitRemoteGithubRepoUrl( fixEnv.repoInfo.owner, @@ -408,7 +430,7 @@ export async function coanaFix( ) // eslint-disable-next-line no-await-in-loop - const prResponse = await openSocketFixPr( + const prResult = await openSocketFixPr( fixEnv.repoInfo.owner, fixEnv.repoInfo.repo, branch, @@ -421,8 +443,8 @@ export async function coanaFix( }, ) - if (prResponse) { - const { data } = prResponse + if (prResult.ok) { + const { data } = prResult.pr const prRef = `PR #${data.number}` logger.success(`Opened ${prRef} for ${ghsaId}.`) @@ -443,11 +465,47 @@ export async function coanaFix( logger.dedent() spinner?.dedent() } + + // Clean up local branch only - keep remote branch for PR merge. + // eslint-disable-next-line no-await-in-loop + await cleanupSuccessfulPrLocalBranch(branch, cwd) + } else { + // Handle PR creation failures. + if (prResult.reason === 'already_exists') { + logger.info( + `PR already exists for ${ghsaId} (this should not happen due to earlier check).`, + ) + // Don't delete branch - PR exists and needs it. + } else if (prResult.reason === 'validation_error') { + logger.error( + `Failed to create PR for ${ghsaId}:\n${prResult.details}`, + ) + // eslint-disable-next-line no-await-in-loop + await cleanupFailedPrBranches(branch, cwd) + } else if (prResult.reason === 'permission_denied') { + logger.error( + `Failed to create PR for ${ghsaId}: Permission denied. Check SOCKET_CLI_GITHUB_TOKEN permissions.`, + ) + // eslint-disable-next-line no-await-in-loop + await cleanupFailedPrBranches(branch, cwd) + } else if (prResult.reason === 'network_error') { + logger.error( + `Failed to create PR for ${ghsaId}: Network error. Please try again.`, + ) + // eslint-disable-next-line no-await-in-loop + await cleanupFailedPrBranches(branch, cwd) + } else { + logger.error( + `Failed to create PR for ${ghsaId}: ${prResult.error.message}`, + ) + // eslint-disable-next-line no-await-in-loop + await cleanupFailedPrBranches(branch, cwd) + } } // Reset back to base branch for next iteration. // eslint-disable-next-line no-await-in-loop - await gitResetAndClean(branch, cwd) + await gitResetAndClean(fixEnv.baseBranch, cwd) // eslint-disable-next-line no-await-in-loop await gitCheckoutBranch(fixEnv.baseBranch, cwd) } catch (e) { @@ -455,6 +513,11 @@ export async function coanaFix( `Unexpected condition: Push failed for ${ghsaId}, skipping PR creation.`, ) debugDir('error', e) + // Clean up branches (push may have succeeded before error). + // eslint-disable-next-line no-await-in-loop + const remoteBranchExists = await gitRemoteBranchExists(branch, cwd) + // eslint-disable-next-line no-await-in-loop + await cleanupErrorBranches(branch, cwd, remoteBranchExists) // eslint-disable-next-line no-await-in-loop await gitResetAndClean(fixEnv.baseBranch, cwd) // eslint-disable-next-line no-await-in-loop diff --git a/src/commands/fix/pull-request.mts b/src/commands/fix/pull-request.mts index 294870b30..0af517125 100644 --- a/src/commands/fix/pull-request.mts +++ b/src/commands/fix/pull-request.mts @@ -35,13 +35,26 @@ export type OpenSocketFixPrOptions = { ghsaDetails?: Map | undefined } +export type OpenPrResult = + | { ok: true; pr: OctokitResponse } + | { ok: false; reason: 'already_exists'; error: RequestError } + | { + ok: false + reason: 'validation_error' + error: RequestError + details: string + } + | { ok: false; reason: 'permission_denied'; error: RequestError } + | { ok: false; reason: 'network_error'; error: RequestError } + | { ok: false; reason: 'unknown'; error: Error } + export async function openSocketFixPr( owner: string, repo: string, branch: string, ghsaIds: string[], options?: OpenSocketFixPrOptions | undefined, -): Promise | undefined> { +): Promise { const { baseBranch = 'main', ghsaDetails } = { __proto__: null, ...options, @@ -59,25 +72,51 @@ export async function openSocketFixPr( body: getSocketFixPullRequestBody(ghsaIds, ghsaDetails), } debugDir('inspect', { octokitPullsCreateParams }) - return await octokit.pulls.create(octokitPullsCreateParams) + const pr = await octokit.pulls.create(octokitPullsCreateParams) + return { ok: true, pr } } catch (e) { - let message = `Failed to open pull request` - const errors = - e instanceof RequestError - ? (e.response?.data as any)?.['errors'] - : undefined - if (Array.isArray(errors) && errors.length) { - const details = errors - .map( - d => - `- ${d.message?.trim() ?? `${d.resource}.${d.field} (${d.code})`}`, + // Handle RequestError from Octokit. + if (e instanceof RequestError) { + const errors = (e.response?.data as any)?.['errors'] + const errorMessages = Array.isArray(errors) + ? errors.map( + d => d.message?.trim() ?? `${d.resource}.${d.field} (${d.code})`, + ) + : [] + + // Check for "PR already exists" error. + if ( + errorMessages.some(msg => + msg.toLowerCase().includes('pull request already exists'), ) - .join('\n') - message += `:\n${details}` + ) { + debugFn('error', 'Failed to open pull request: already exists') + return { ok: false, reason: 'already_exists', error: e } + } + + // Check for validation errors (e.g., no commits between branches). + if (errors && errors.length > 0) { + const details = errorMessages.map(d => `- ${d}`).join('\n') + debugFn('error', `Failed to open pull request:\n${details}`) + return { ok: false, reason: 'validation_error', error: e, details } + } + + // Check HTTP status codes. + if (e.status === 403 || e.status === 401) { + debugFn('error', 'Failed to open pull request: permission denied') + return { ok: false, reason: 'permission_denied', error: e } + } + + if (e.status && e.status >= 500) { + debugFn('error', 'Failed to open pull request: network error') + return { ok: false, reason: 'network_error', error: e } + } } - debugFn('error', message) + + // Unknown error. + debugFn('error', `Failed to open pull request: ${e}`) + return { ok: false, reason: 'unknown', error: e as Error } } - return undefined } export type GQL_MERGE_STATE_STATUS = diff --git a/src/commands/scan/cmd-scan-create.test.mts b/src/commands/scan/cmd-scan-create.test.mts index e8d53aaf4..cb2742def 100644 --- a/src/commands/scan/cmd-scan-create.test.mts +++ b/src/commands/scan/cmd-scan-create.test.mts @@ -56,6 +56,8 @@ describe('socket scan create', async () => { Reachability Options (when --reach is used) --reach-analysis-memory-limit The maximum memory in MB to use for the reachability analysis. The default is 8192MB. --reach-analysis-timeout Set timeout for the reachability analysis. Split analysis runs may cause the total scan time to exceed this timeout significantly. + --reach-concurrency Set the maximum number of concurrent reachability analysis runs. It is recommended to choose a concurrency level that ensures each analysis run has at least the --reach-analysis-memory-limit amount of memory available. NPM reachability analysis does not support concurrent execution, so the concurrency level is ignored for NPM. + --reach-disable-analysis-splitting Limits Coana to at most 1 reachability analysis run per workspace. --reach-disable-analytics Disable reachability analytics sharing with Socket. Also disables caching-based optimizations. --reach-ecosystems List of ecosystems to conduct reachability analysis on, as either a comma separated value or as multiple flags. Defaults to all ecosystems. --reach-exclude-paths List of paths to exclude from reachability analysis, as either a comma separated value or as multiple flags. diff --git a/src/commands/scan/cmd-scan-reach.test.mts b/src/commands/scan/cmd-scan-reach.test.mts index c543d739f..4639d0e41 100644 --- a/src/commands/scan/cmd-scan-reach.test.mts +++ b/src/commands/scan/cmd-scan-reach.test.mts @@ -39,8 +39,8 @@ describe('socket scan reach', async () => { --reach-analysis-memory-limit The maximum memory in MB to use for the reachability analysis. The default is 8192MB. --reach-analysis-timeout Set timeout for the reachability analysis. Split analysis runs may cause the total scan time to exceed this timeout significantly. --reach-concurrency Set the maximum number of concurrent reachability analysis runs. It is recommended to choose a concurrency level that ensures each analysis run has at least the --reach-analysis-memory-limit amount of memory available. NPM reachability analysis does not support concurrent execution, so the concurrency level is ignored for NPM. - --reach-disable-analytics Disable reachability analytics sharing with Socket. Also disables caching-based optimizations. --reach-disable-analysis-splitting Limits Coana to at most 1 reachability analysis run per workspace. + --reach-disable-analytics Disable reachability analytics sharing with Socket. Also disables caching-based optimizations. --reach-ecosystems List of ecosystems to conduct reachability analysis on, as either a comma separated value or as multiple flags. Defaults to all ecosystems. --reach-exclude-paths List of paths to exclude from reachability analysis, as either a comma separated value or as multiple flags. --reach-skip-cache Skip caching-based optimizations. By default, the reachability analysis will use cached configurations from previous runs to speed up the analysis. diff --git a/src/utils/dlx.mts b/src/utils/dlx.mts index cdf863d30..3064d7e46 100644 --- a/src/utils/dlx.mts +++ b/src/utils/dlx.mts @@ -216,7 +216,8 @@ export async function spawnCoanaDlx( const localCoanaPath = process.env['SOCKET_CLI_COANA_LOCAL_PATH'] // Use local Coana CLI if path is provided. if (localCoanaPath) { - const isBinary = !localCoanaPath.endsWith('.js') && !localCoanaPath.endsWith('.mjs'); + const isBinary = + !localCoanaPath.endsWith('.js') && !localCoanaPath.endsWith('.mjs') const finalEnv = { ...process.env, @@ -225,12 +226,16 @@ export async function spawnCoanaDlx( ...spawnEnv, } - const spawnArgs = isBinary ? args : [localCoanaPath, ...args]; - const spawnResult = await spawn(isBinary ? localCoanaPath : 'node', spawnArgs, { - cwd: dlxOptions.cwd, - env: finalEnv, - stdio: spawnExtra?.['stdio'] || 'inherit', - }) + const spawnArgs = isBinary ? args : [localCoanaPath, ...args] + const spawnResult = await spawn( + isBinary ? localCoanaPath : 'node', + spawnArgs, + { + cwd: dlxOptions.cwd, + env: finalEnv, + stdio: spawnExtra?.['stdio'] || 'inherit', + }, + ) return { ok: true, data: spawnResult.stdout } }