test(cli): add tests for env-helpers and ghsa-tracker

Add comprehensive tests for:
- env-helpers: getFixEnv function covering CI detection, GITHUB_REPOSITORY
  parsing, repo info fallback, PR fetching
- ghsa-tracker: file locking mechanism tests including EEXIST handling,
  stale lock detection, lock cleanup, and error recovery

Author: jdalton

packages/cli/test/unit/commands/fix/env-helpers.test.mts

@@ -48,9 +48,53 @@ vi.mock('../../../../src/env/socket-cli-git-user-email.mts', () => mockGitEmail)
 const mockGitUser = vi.hoisted(() => ({ SOCKET_CLI_GIT_USER_NAME: '' }))
 vi.mock('../../../../src/env/socket-cli-git-user-name.mts', () => mockGitUser)
 
+// Mock GITHUB_REPOSITORY.
+const mockGithubRepo = vi.hoisted(() => ({ GITHUB_REPOSITORY: '' }))
+vi.mock('../../../../src/env/github-repository.mts', () => mockGithubRepo)
+
+// Mock git operations.
+const mockGetBaseBranch = vi.hoisted(() =>
+  vi.fn().mockResolvedValue('main'),
+)
+const mockGetRepoInfo = vi.hoisted(() =>
+  vi.fn().mockResolvedValue({ owner: 'test-owner', repo: 'test-repo' }),
+)
+vi.mock('../../../../src/utils/git/operations.mts', () => ({
+  getBaseBranch: mockGetBaseBranch,
+  getRepoInfo: mockGetRepoInfo,
+}))
+
+// Mock pull-request functions.
+const mockGetSocketFixPrs = vi.hoisted(() => vi.fn().mockResolvedValue([]))
+vi.mock('../../../../src/commands/fix/pull-request.mts', () => ({
+  getSocketFixPrs: mockGetSocketFixPrs,
+}))
+
+// Mock logger.
+const mockLogger = vi.hoisted(() => ({
+  error: vi.fn(),
+  fail: vi.fn(),
+  info: vi.fn(),
+  log: vi.fn(),
+  success: vi.fn(),
+  warn: vi.fn(),
+}))
+vi.mock('@socketsecurity/lib/logger', () => ({
+  getDefaultLogger: () => mockLogger,
+}))
+
+// Mock debug.
+const mockDebug = vi.hoisted(() => vi.fn())
+const mockIsDebug = vi.hoisted(() => vi.fn(() => false))
+vi.mock('@socketsecurity/lib/debug', () => ({
+  debug: mockDebug,
+  isDebug: mockIsDebug,
+}))
+
 import {
   checkCiEnvVars,
   getCiEnvInstructions,
+  getFixEnv,
 } from '../../../../src/commands/fix/env-helpers.mts'
 
 describe('env-helpers', () => {
@@ -60,6 +104,11 @@ describe('env-helpers', () => {
     mockGetSocketCliGithubToken.mockReturnValue(undefined)
     mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = ''
     mockGitUser.SOCKET_CLI_GIT_USER_NAME = ''
+    mockGithubRepo.GITHUB_REPOSITORY = ''
+    mockGetBaseBranch.mockResolvedValue('main')
+    mockGetRepoInfo.mockResolvedValue({ owner: 'test-owner', repo: 'test-repo' })
+    mockGetSocketFixPrs.mockResolvedValue([])
+    mockIsDebug.mockReturnValue(false)
   })
 
   describe('getCiEnvInstructions', () => {
@@ -159,4 +208,156 @@ describe('env-helpers', () => {
       expect(result.present).toHaveLength(4)
     })
   })
+
+  describe('getFixEnv', () => {
+    it('should return basic fix env when not in CI', async () => {
+      mockGetCI.mockReturnValue(false)
+
+      const result = await getFixEnv()
+
+      expect(result.isCi).toBe(false)
+      expect(result.baseBranch).toBe('main')
+      expect(result.prs).toEqual([])
+      expect(result.repoInfo).toEqual({ owner: 'test-owner', repo: 'test-repo' })
+    })
+
+    it('should return isCi true when all CI vars are set', async () => {
+      mockGetCI.mockReturnValue(true)
+      mockGetSocketCliGithubToken.mockReturnValue('ghp_test_token')
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = 'test-user'
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = 'test@example.com'
+      mockGithubRepo.GITHUB_REPOSITORY = 'owner/repo'
+
+      const result = await getFixEnv()
+
+      expect(result.isCi).toBe(true)
+      expect(result.gitUser).toBe('test-user')
+      expect(result.gitEmail).toBe('test@example.com')
+      expect(result.githubToken).toBe('ghp_test_token')
+    })
+
+    it('should use GITHUB_REPOSITORY env var for repoInfo in CI', async () => {
+      mockGetCI.mockReturnValue(true)
+      mockGetSocketCliGithubToken.mockReturnValue('ghp_test_token')
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = 'test-user'
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = 'test@example.com'
+      mockGithubRepo.GITHUB_REPOSITORY = 'my-owner/my-repo'
+
+      const result = await getFixEnv()
+
+      expect(result.repoInfo).toEqual({ owner: 'my-owner', repo: 'my-repo' })
+      // Should not call getRepoInfo when GITHUB_REPOSITORY is valid.
+      expect(mockGetRepoInfo).not.toHaveBeenCalled()
+    })
+
+    it('should fall back to getRepoInfo when GITHUB_REPOSITORY is invalid', async () => {
+      mockGetCI.mockReturnValue(true)
+      mockGetSocketCliGithubToken.mockReturnValue('ghp_test_token')
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = 'test-user'
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = 'test@example.com'
+      // Invalid GITHUB_REPOSITORY (no slash).
+      mockGithubRepo.GITHUB_REPOSITORY = 'invalid-repo'
+
+      const result = await getFixEnv()
+
+      expect(result.repoInfo).toEqual({ owner: 'test-owner', repo: 'test-repo' })
+      expect(mockGetRepoInfo).toHaveBeenCalled()
+    })
+
+    it('should fall back to getRepoInfo when GITHUB_REPOSITORY is empty', async () => {
+      mockGetCI.mockReturnValue(true)
+      mockGetSocketCliGithubToken.mockReturnValue('ghp_test_token')
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = 'test-user'
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = 'test@example.com'
+      mockGithubRepo.GITHUB_REPOSITORY = ''
+
+      const result = await getFixEnv()
+
+      expect(mockGetRepoInfo).toHaveBeenCalled()
+    })
+
+    it('should warn when CI is set but other vars are missing', async () => {
+      mockGetCI.mockReturnValue(true)
+      // Missing: githubToken, gitUser, gitEmail.
+      mockGetSocketCliGithubToken.mockReturnValue(undefined)
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = ''
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = ''
+
+      await getFixEnv()
+
+      expect(mockLogger.warn).toHaveBeenCalledWith(
+        expect.stringContaining('CI mode detected'),
+      )
+      expect(mockLogger.warn).toHaveBeenCalledWith(
+        expect.stringContaining('Missing:'),
+      )
+    })
+
+    it('should not warn when not in CI', async () => {
+      mockGetCI.mockReturnValue(false)
+
+      await getFixEnv()
+
+      expect(mockLogger.warn).not.toHaveBeenCalled()
+    })
+
+    it('should log debug message when not in CI but some vars are set', async () => {
+      mockGetCI.mockReturnValue(false)
+      mockGetSocketCliGithubToken.mockReturnValue('ghp_test_token')
+      mockIsDebug.mockReturnValue(true)
+
+      await getFixEnv()
+
+      expect(mockDebug).toHaveBeenCalledWith(
+        expect.stringContaining('isCi is false'),
+      )
+    })
+
+    it('should fetch PRs when in CI mode', async () => {
+      mockGetCI.mockReturnValue(true)
+      mockGetSocketCliGithubToken.mockReturnValue('ghp_test_token')
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = 'test-user'
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = 'test@example.com'
+      mockGithubRepo.GITHUB_REPOSITORY = 'owner/repo'
+      mockGetSocketFixPrs.mockResolvedValue([
+        { number: 1, title: 'Fix PR' },
+      ])
+
+      const result = await getFixEnv()
+
+      expect(mockGetSocketFixPrs).toHaveBeenCalledWith('owner', 'repo', {
+        author: 'test-user',
+        states: 'all',
+      })
+      expect(result.prs).toEqual([{ number: 1, title: 'Fix PR' }])
+    })
+
+    it('should not fetch PRs when not in CI mode', async () => {
+      mockGetCI.mockReturnValue(false)
+
+      await getFixEnv()
+
+      expect(mockGetSocketFixPrs).not.toHaveBeenCalled()
+    })
+
+    it('should return gitEmail and gitUser from env vars', async () => {
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = 'custom-user'
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = 'custom@example.com'
+
+      const result = await getFixEnv()
+
+      expect(result.gitUser).toBe('custom-user')
+      expect(result.gitEmail).toBe('custom@example.com')
+    })
+
+    it('should return undefined for gitEmail and gitUser when not set', async () => {
+      mockGitUser.SOCKET_CLI_GIT_USER_NAME = ''
+      mockGitEmail.SOCKET_CLI_GIT_USER_EMAIL = ''
+
+      const result = await getFixEnv()
+
+      expect(result.gitUser).toBe('')
+      expect(result.gitEmail).toBe('')
+    })
+  })
 })

packages/cli/test/unit/commands/fix/ghsa-tracker.test.mts

@@ -46,15 +46,21 @@ const mockReadJson = vi.hoisted(() => vi.fn())
 const mockSafeMkdir = vi.hoisted(() => vi.fn())
 const mockWriteJson = vi.hoisted(() => vi.fn())
 
+// Mock fs promises.
+const mockFsWriteFile = vi.hoisted(() => vi.fn())
+const mockFsReadFile = vi.hoisted(() => vi.fn())
+const mockFsUnlink = vi.hoisted(() => vi.fn())
+
 vi.mock('node:fs', async () => {
   const actual = await vi.importActual<typeof import('node:fs')>('node:fs')
   return {
     ...actual,
     promises: {
       ...actual.promises,
       mkdir: vi.fn(),
-      readFile: vi.fn(),
-      writeFile: vi.fn(),
+      readFile: mockFsReadFile,
+      writeFile: mockFsWriteFile,
+      unlink: mockFsUnlink,
     },
   }
 })
@@ -71,6 +77,10 @@ describe('ghsa-tracker', () => {
 
   beforeEach(() => {
     vi.clearAllMocks()
+    // Default: lock file creation succeeds.
+    mockFsWriteFile.mockResolvedValue(undefined)
+    mockFsReadFile.mockResolvedValue('12345')
+    mockFsUnlink.mockResolvedValue(undefined)
   })
 
   describe('loadGhsaTracker', () => {
@@ -364,5 +374,120 @@ describe('ghsa-tracker', () => {
       expect(record).toBeDefined()
       expect(record!.prNumber).toBeUndefined()
     })
+
+    it('handles lock file already exists (EEXIST)', async () => {
+      const existingTracker: GhsaTracker = {
+        version: 1,
+        fixed: [],
+      }
+
+      // First call to writeFile fails with EEXIST, subsequent succeeds.
+      const eexistError = new Error('Lock exists') as NodeJS.ErrnoException
+      eexistError.code = 'EEXIST'
+      mockFsWriteFile.mockRejectedValueOnce(eexistError)
+      mockFsWriteFile.mockResolvedValueOnce(undefined)
+
+      // Mock reading lock file to show stale lock (dead process).
+      mockFsReadFile.mockResolvedValueOnce('99999999')
+
+      mockReadJson.mockResolvedValue(existingTracker)
+
+      await markGhsaFixed(mockCwd, 'GHSA-lock-test', 123)
+
+      // Should still save the tracker.
+      expect(mockWriteJson).toHaveBeenCalled()
+    })
+
+    it('handles lock file read error', async () => {
+      const existingTracker: GhsaTracker = {
+        version: 1,
+        fixed: [],
+      }
+
+      // First call to writeFile fails with EEXIST.
+      const eexistError = new Error('Lock exists') as NodeJS.ErrnoException
+      eexistError.code = 'EEXIST'
+      mockFsWriteFile.mockRejectedValueOnce(eexistError)
+      mockFsWriteFile.mockResolvedValueOnce(undefined)
+
+      // Mock reading lock file fails.
+      mockFsReadFile.mockRejectedValueOnce(new Error('Read error'))
+
+      mockReadJson.mockResolvedValue(existingTracker)
+
+      await markGhsaFixed(mockCwd, 'GHSA-lock-read-error', 123)
+
+      // Should still save the tracker (proceeds without lock).
+      expect(mockWriteJson).toHaveBeenCalled()
+    })
+
+    it('handles lock file with invalid PID', async () => {
+      const existingTracker: GhsaTracker = {
+        version: 1,
+        fixed: [],
+      }
+
+      // First call to writeFile fails with EEXIST.
+      const eexistError = new Error('Lock exists') as NodeJS.ErrnoException
+      eexistError.code = 'EEXIST'
+      mockFsWriteFile.mockRejectedValueOnce(eexistError)
+      mockFsWriteFile.mockResolvedValueOnce(undefined)
+
+      // Mock reading lock file with invalid PID.
+      mockFsReadFile.mockResolvedValueOnce('not-a-number')
+
+      mockReadJson.mockResolvedValue(existingTracker)
+
+      await markGhsaFixed(mockCwd, 'GHSA-invalid-pid', 123)
+
+      // Should proceed anyway.
+      expect(mockWriteJson).toHaveBeenCalled()
+    })
+
+    it('releases lock after successful operation', async () => {
+      const existingTracker: GhsaTracker = {
+        version: 1,
+        fixed: [],
+      }
+
+      mockReadJson.mockResolvedValue(existingTracker)
+
+      await markGhsaFixed(mockCwd, 'GHSA-release-lock', 123)
+
+      // Should attempt to unlink the lock file.
+      expect(mockFsUnlink).toHaveBeenCalled()
+    })
+
+    it('handles lock cleanup error gracefully', async () => {
+      const existingTracker: GhsaTracker = {
+        version: 1,
+        fixed: [],
+      }
+
+      mockReadJson.mockResolvedValue(existingTracker)
+      mockFsUnlink.mockRejectedValueOnce(new Error('Cleanup error'))
+
+      // Should not throw.
+      await expect(
+        markGhsaFixed(mockCwd, 'GHSA-cleanup-error', 123),
+      ).resolves.toBeUndefined()
+    })
+
+    it('proceeds without lock when all attempts fail', async () => {
+      const existingTracker: GhsaTracker = {
+        version: 1,
+        fixed: [],
+      }
+
+      // All lock attempts fail with non-EEXIST error.
+      mockFsWriteFile.mockRejectedValue(new Error('Permission denied'))
+
+      mockReadJson.mockResolvedValue(existingTracker)
+
+      await markGhsaFixed(mockCwd, 'GHSA-no-lock', 123)
+
+      // Should still save the tracker.
+      expect(mockWriteJson).toHaveBeenCalled()
+    })
   })
 })