Fix PR creation logic to check PR existence instead of branch

Previously, the socket fix command would only create a PR once and then
never again because it checked for branch existence instead of PR
existence. Since branches persist after PR creation, subsequent runs
would skip PR creation entirely.

Changes:
- Check for open PR existence instead of branch existence
- Add stale branch cleanup before creating new branches
- Move GitHub token check before git operations for early validation
- Integrate cleanup functions for proper branch lifecycle management
- Add comprehensive error handling for all PR creation failure types
- Remove unused gitDeleteRemoteBranch import

This ensures PRs can be recreated after being closed/merged and prevents
branch accumulation from failed PR attempts.

Author: jdalton

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,18 +465,59 @@ 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) {
       logger.warn(
         `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

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.

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.

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 }
     }