test(desktop): cover the review store (refresh, selection, mutations, ship flow)

apps/desktop/src/store/review.test.ts

@@ -0,0 +1,449 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import type { HermesReviewFile, HermesReviewShipInfo } from '@/global'
+
+import {
+  $reviewCommitDefault,
+  $reviewCommitMsgBusy,
+  $reviewDiff,
+  $reviewDiffLoading,
+  $reviewFiles,
+  $reviewIsRepo,
+  $reviewLoading,
+  $reviewMaxChurn,
+  $reviewOpen,
+  $reviewRevertTarget,
+  $reviewSelectedPath,
+  $reviewShipBusy,
+  $reviewShipInfo,
+  $reviewTreeMode,
+  cancelRevert,
+  clearReviewSelection,
+  closeReview,
+  commitChanges,
+  confirmRevert,
+  createOrOpenPr,
+  generateCommitMessage,
+  openReview,
+  pushChanges,
+  refreshReview,
+  refreshShipInfo,
+  requestRevert,
+  revertReviewFile,
+  selectReviewFile,
+  stageReviewFile,
+  toggleReviewTreeMode,
+  unstageReviewFile
+} from './review'
+import { $currentCwd } from './session'
+
+// requestOneShot is the only cross-module dependency that must be faked (it
+// reaches the gateway); everything else routes through window.hermesDesktop.git,
+// which we stub per-test like the sibling coding-status.test.ts does.
+const requestOneShot = vi.fn(async (_args: unknown) => 'generated message')
+vi.mock('@/lib/oneshot', () => ({ requestOneShot: (args: unknown) => requestOneShot(args) }))
+// refreshRepoStatus is a fire-and-forget side effect of mutations; stub it so it
+// doesn't try to hit the (absent) probe and log.
+vi.mock('./coding-status', () => ({ refreshRepoStatus: vi.fn() }))
+
+function file(path: string, over: Partial<HermesReviewFile> = {}): HermesReviewFile {
+  return { path, status: 'modified', staged: false, added: 1, removed: 0, ...over } as HermesReviewFile
+}
+
+type ReviewStub = Record<string, ReturnType<typeof vi.fn>>
+
+// Install a review bridge on window.hermesDesktop. Any op not supplied defaults
+// to a resolved no-op so a test only declares what it exercises.
+function stubReview(over: ReviewStub = {}) {
+  const review: ReviewStub = {
+    list: vi.fn(async () => ({ files: [] })),
+    diff: vi.fn(async () => ''),
+    stage: vi.fn(async () => undefined),
+    unstage: vi.fn(async () => undefined),
+    revert: vi.fn(async () => undefined),
+    commit: vi.fn(async () => undefined),
+    commitContext: vi.fn(async () => ({ diff: 'd', recent: 'r' })),
+    push: vi.fn(async () => undefined),
+    shipInfo: vi.fn(async () => ({ ghReady: false, pr: null })),
+    createPr: vi.fn(async () => ({ url: 'https://example.com/pr/1' })),
+    ...over
+  }
+
+  ;(window as unknown as { hermesDesktop?: unknown }).hermesDesktop = {
+    git: { review },
+    openExternal: vi.fn()
+  }
+
+  return review
+}
+
+beforeEach(() => {
+  requestOneShot.mockClear()
+  requestOneShot.mockResolvedValue('generated message')
+  // Reset stores touched across tests.
+  $reviewOpen.set(false)
+  $reviewFiles.set([])
+  $reviewLoading.set(false)
+  $reviewIsRepo.set(true)
+  $reviewDiff.set(null)
+  $reviewDiffLoading.set(false)
+  $reviewSelectedPath.set(null)
+  $reviewShipInfo.set({ ghReady: false, pr: null })
+  $reviewShipBusy.set(false)
+  $reviewCommitMsgBusy.set(false)
+  $reviewRevertTarget.set(undefined)
+  $currentCwd.set('/repo')
+})
+
+afterEach(() => {
+  delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
+})
+
+describe('refreshReview', () => {
+  it('is a no-op that clears state when the pane is closed', async () => {
+    const review = stubReview()
+    $reviewOpen.set(false)
+    $reviewFiles.set([file('a.ts')])
+
+    await refreshReview()
+
+    expect(review.list).not.toHaveBeenCalled()
+    expect($reviewFiles.get()).toEqual([])
+    expect($reviewLoading.get()).toBe(false)
+  })
+
+  it('flags not-a-repo (and clears loading) when there is no bridge/cwd', async () => {
+    delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
+    $reviewOpen.set(true)
+    $reviewLoading.set(true)
+
+    await refreshReview()
+
+    expect($reviewIsRepo.get()).toBe(false)
+    expect($reviewLoading.get()).toBe(false)
+  })
+
+  it('populates the changed-file list from the bridge', async () => {
+    stubReview({ list: vi.fn(async () => ({ files: [file('a.ts'), file('b.ts')] })) })
+    $reviewOpen.set(true)
+
+    await refreshReview()
+
+    expect($reviewFiles.get().map(f => f.path)).toEqual(['a.ts', 'b.ts'])
+    expect($reviewIsRepo.get()).toBe(true)
+    expect($reviewLoading.get()).toBe(false)
+  })
+
+  it('filters excluded paths (node_modules et al.) out of the list', async () => {
+    stubReview({ list: vi.fn(async () => ({ files: [file('src/a.ts'), file('node_modules/x/index.js')] })) })
+    $reviewOpen.set(true)
+
+    await refreshReview()
+
+    expect($reviewFiles.get().map(f => f.path)).toEqual(['src/a.ts'])
+  })
+
+  it('drops a selection whose file vanished from the new list', async () => {
+    stubReview({ list: vi.fn(async () => ({ files: [file('kept.ts')] })) })
+    $reviewOpen.set(true)
+    $reviewSelectedPath.set('gone.ts')
+    $reviewDiff.set('old diff')
+
+    await refreshReview()
+
+    expect($reviewSelectedPath.get()).toBeNull()
+    expect($reviewDiff.get()).toBeNull()
+  })
+
+  it('clears the list but keeps isRepo true when the bridge throws', async () => {
+    stubReview({
+      list: vi.fn(async () => {
+        throw new Error('git failed')
+      })
+    })
+    $reviewOpen.set(true)
+    $reviewFiles.set([file('stale.ts')])
+
+    await refreshReview()
+
+    expect($reviewFiles.get()).toEqual([])
+    expect($reviewIsRepo.get()).toBe(true)
+    expect($reviewLoading.get()).toBe(false)
+  })
+})
+
+describe('$reviewMaxChurn', () => {
+  it('is the largest added+removed across files', () => {
+    $reviewFiles.set([file('a', { added: 3, removed: 2 }), file('b', { added: 10, removed: 1 }), file('c')])
+    expect($reviewMaxChurn.get()).toBe(11)
+  })
+
+  it('is 0 for an empty list', () => {
+    $reviewFiles.set([])
+    expect($reviewMaxChurn.get()).toBe(0)
+  })
+})
+
+describe('selectReviewFile / clearReviewSelection', () => {
+  it('sets the selected path and fetches its diff', async () => {
+    const review = stubReview({ diff: vi.fn(async () => 'the diff') })
+
+    await selectReviewFile(file('a.ts'))
+
+    expect($reviewSelectedPath.get()).toBe('a.ts')
+    expect($reviewDiff.get()).toBe('the diff')
+    expect($reviewDiffLoading.get()).toBe(false)
+    expect(review.diff).toHaveBeenCalledWith('/repo', 'a.ts', 'uncommitted', null, false)
+  })
+
+  it('coerces a falsy diff to empty string (not null)', async () => {
+    stubReview({ diff: vi.fn(async () => '') })
+
+    await selectReviewFile(file('a.ts'))
+
+    expect($reviewDiff.get()).toBe('')
+  })
+
+  it('sets diff null when there is no bridge', async () => {
+    delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
+
+    await selectReviewFile(file('a.ts'))
+
+    expect($reviewSelectedPath.get()).toBe('a.ts')
+    expect($reviewDiff.get()).toBeNull()
+  })
+
+  it('clears path, diff and loading', () => {
+    $reviewSelectedPath.set('a.ts')
+    $reviewDiff.set('x')
+    $reviewDiffLoading.set(true)
+
+    clearReviewSelection()
+
+    expect($reviewSelectedPath.get()).toBeNull()
+    expect($reviewDiff.get()).toBeNull()
+    expect($reviewDiffLoading.get()).toBe(false)
+  })
+})
+
+describe('view state', () => {
+  it('toggleReviewTreeMode flips list <-> tree', () => {
+    $reviewTreeMode.set('tree')
+    toggleReviewTreeMode()
+    expect($reviewTreeMode.get()).toBe('list')
+    toggleReviewTreeMode()
+    expect($reviewTreeMode.get()).toBe('tree')
+  })
+
+  it('openReview opens the pane and kicks off a refresh', async () => {
+    const review = stubReview()
+    openReview()
+    expect($reviewOpen.get()).toBe(true)
+    // openReview fires refreshReview + refreshShipInfo without awaiting.
+    await Promise.resolve()
+    await Promise.resolve()
+    expect(review.list).toHaveBeenCalled()
+  })
+
+  it('closeReview closes the pane and clears the selection', () => {
+    stubReview()
+    $reviewOpen.set(true)
+    $reviewSelectedPath.set('a.ts')
+    $reviewDiff.set('x')
+
+    closeReview()
+
+    expect($reviewOpen.get()).toBe(false)
+    expect($reviewSelectedPath.get()).toBeNull()
+    expect($reviewDiff.get()).toBeNull()
+  })
+})
+
+describe('mutations', () => {
+  it('stageReviewFile forwards the path and re-syncs', async () => {
+    const review = stubReview()
+    $reviewOpen.set(true) // afterMutation's refreshReview only lists when the pane is open
+    await stageReviewFile('a.ts')
+    expect(review.stage).toHaveBeenCalledWith('/repo', 'a.ts')
+    expect(review.list).toHaveBeenCalled()
+  })
+
+  it('unstageReviewFile forwards the path', async () => {
+    const review = stubReview()
+    await unstageReviewFile('a.ts')
+    expect(review.unstage).toHaveBeenCalledWith('/repo', 'a.ts')
+  })
+
+  it('revertReviewFile forwards the path', async () => {
+    const review = stubReview()
+    await revertReviewFile('a.ts')
+    expect(review.revert).toHaveBeenCalledWith('/repo', 'a.ts')
+  })
+
+  it('stage with null path means "all"', async () => {
+    const review = stubReview()
+    await stageReviewFile(null)
+    expect(review.stage).toHaveBeenCalledWith('/repo', null)
+  })
+})
+
+describe('revert confirm dialog', () => {
+  it('requestRevert opens a target, cancelRevert closes it', () => {
+    requestRevert('a.ts')
+    expect($reviewRevertTarget.get()).toEqual({ path: 'a.ts' })
+    cancelRevert()
+    expect($reviewRevertTarget.get()).toBeUndefined()
+  })
+
+  it('requestRevert(null) encodes the "revert all" target distinctly from closed', () => {
+    requestRevert(null)
+    expect($reviewRevertTarget.get()).toEqual({ path: null })
+  })
+
+  it('confirmRevert closes the dialog then performs the revert', async () => {
+    const review = stubReview()
+    requestRevert('a.ts')
+
+    await confirmRevert()
+
+    expect($reviewRevertTarget.get()).toBeUndefined()
+    expect(review.revert).toHaveBeenCalledWith('/repo', 'a.ts')
+  })
+
+  it('confirmRevert is a no-op when nothing is pending', async () => {
+    const review = stubReview()
+    $reviewRevertTarget.set(undefined)
+
+    await confirmRevert()
+
+    expect(review.revert).not.toHaveBeenCalled()
+  })
+})
+
+describe('ship flow', () => {
+  it('commitChanges commits the trimmed message and toggles the busy flag', async () => {
+    const review = stubReview()
+    const seen: boolean[] = []
+    const unsub = $reviewShipBusy.subscribe(v => seen.push(v))
+
+    await commitChanges('  a message  ', { push: true })
+
+    expect(review.commit).toHaveBeenCalledWith('/repo', 'a message', true)
+    expect(seen).toContain(true)
+    expect($reviewShipBusy.get()).toBe(false)
+    unsub()
+  })
+
+  it('commitChanges bails on a blank message', async () => {
+    const review = stubReview()
+    await commitChanges('   ')
+    expect(review.commit).not.toHaveBeenCalled()
+  })
+
+  it('pushChanges pushes and refreshes ship info', async () => {
+    const review = stubReview()
+    await pushChanges()
+    expect(review.push).toHaveBeenCalledWith('/repo')
+  })
+
+  it('createOrOpenPr opens the existing PR without creating a new one', async () => {
+    const review = stubReview()
+    $reviewShipInfo.set({ ghReady: true, pr: { url: 'https://example.com/pr/9' } } as HermesReviewShipInfo)
+
+    await createOrOpenPr()
+
+    expect(review.createPr).not.toHaveBeenCalled()
+    expect((window.hermesDesktop as unknown as { openExternal: ReturnType<typeof vi.fn> }).openExternal).toHaveBeenCalledWith(
+      'https://example.com/pr/9'
+    )
+  })
+
+  it('createOrOpenPr creates a PR when none exists, then opens it', async () => {
+    const review = stubReview({ createPr: vi.fn(async () => ({ url: 'https://example.com/pr/new' })) })
+    $reviewShipInfo.set({ ghReady: true, pr: null })
+
+    await createOrOpenPr()
+
+    expect(review.createPr).toHaveBeenCalledWith('/repo')
+    expect((window.hermesDesktop as unknown as { openExternal: ReturnType<typeof vi.fn> }).openExternal).toHaveBeenCalledWith(
+      'https://example.com/pr/new'
+    )
+  })
+})
+
+describe('refreshShipInfo', () => {
+  it('populates ship info from the bridge', async () => {
+    const info: HermesReviewShipInfo = { ghReady: true, pr: { url: 'https://example.com/pr/3' } } as HermesReviewShipInfo
+    stubReview({ shipInfo: vi.fn(async () => info) })
+
+    await refreshShipInfo()
+
+    expect($reviewShipInfo.get()).toEqual(info)
+  })
+
+  it('resets ship info when there is no bridge', async () => {
+    delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
+    $reviewShipInfo.set({ ghReady: true, pr: { url: 'x' } } as HermesReviewShipInfo)
+
+    await refreshShipInfo()
+
+    expect($reviewShipInfo.get()).toEqual({ ghReady: false, pr: null })
+  })
+
+  it('resets ship info when the bridge throws', async () => {
+    stubReview({
+      shipInfo: vi.fn(async () => {
+        throw new Error('gh missing')
+      })
+    })
+    $reviewShipInfo.set({ ghReady: true, pr: { url: 'x' } } as HermesReviewShipInfo)
+
+    await refreshShipInfo()
+
+    expect($reviewShipInfo.get()).toEqual({ ghReady: false, pr: null })
+  })
+})
+
+describe('generateCommitMessage', () => {
+  it('returns a one-shot message from the working-tree diff', async () => {
+    stubReview()
+
+    const msg = await generateCommitMessage('avoid this')
+
+    expect(msg).toBe('generated message')
+    expect(requestOneShot).toHaveBeenCalledWith(
+      expect.objectContaining({
+        template: 'commit_message',
+        variables: expect.objectContaining({ avoid: 'avoid this', diff: 'd', recent_commits: 'r' })
+      })
+    )
+    expect($reviewCommitMsgBusy.get()).toBe(false)
+  })
+
+  it('returns empty (no model call) when the diff is blank', async () => {
+    stubReview({ commitContext: vi.fn(async () => ({ diff: '   ', recent: '' })) })
+
+    const msg = await generateCommitMessage()
+
+    expect(msg).toBe('')
+    expect(requestOneShot).not.toHaveBeenCalled()
+  })
+
+  it('returns empty when the bridge lacks commitContext', async () => {
+    const review = stubReview()
+    delete review.commitContext
+
+    const msg = await generateCommitMessage()
+
+    expect(msg).toBe('')
+  })
+})
+
+describe('$reviewCommitDefault', () => {
+  it('remembers the split-button default action', () => {
+    $reviewCommitDefault.set('commitPush')
+    expect($reviewCommitDefault.get()).toBe('commitPush')
+    $reviewCommitDefault.set('commit')
+    expect($reviewCommitDefault.get()).toBe('commit')
+  })
+})