task/if we have a pr open we should 4b0a71

src/components/FinishDialog.client.test.tsx

@@ -4,8 +4,14 @@ import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest';
 import { FinishDialog } from './FinishDialog';
 import { invoke } from '../lib/ipc';
 import { getPrChecks, mergeTask, pushTask } from '../store/store';
+import {
+  getPullRequestDetails,
+  mergePullRequestForTask,
+  stageFailedChecksPrompt,
+} from '../store/github';
 import { IPC } from '../../electron/ipc/channels';
 import type { ChangeTourController } from '../lib/create-change-tour';
+import type { PullRequestDetails } from '../ipc/types';
 import type { Task } from '../store/types';
 
 vi.mock('../store/store', () => ({
@@ -26,7 +32,7 @@ vi.mock('../lib/ipc', () => ({
   invoke: vi.fn(async (channel: string) => {
     if (channel === IPC.GetBranchLog) return '';
     if (channel === IPC.CheckMergeStatus) return { conflicting_files: [], main_ahead_count: 0 };
-    return { current_branch: 'task/parent', has_committed_changes: true };
+    return { current_branch: 'task/parent', has_committed_changes: true, head_sha: 'local' };
   }),
   Channel: class {
     onmessage: ((text: string) => void) | null = null;
@@ -72,6 +78,13 @@ vi.mock('./EvidencePanel', () => ({
   ),
 }));
 vi.mock('../store/tasks', () => ({ sendPrompt: vi.fn() }));
+vi.mock('../store/notification', () => ({ showNotification: vi.fn() }));
+vi.mock('../store/github', () => ({
+  getPullRequestDetails: vi.fn(),
+  mergePullRequestForTask: vi.fn(),
+  stageFailedChecksPrompt: vi.fn(),
+  stageReviewFeedbackPrompt: vi.fn(),
+}));
 vi.mock('./DiffViewerDialog', async () => {
   const { ReviewProvider, useReview } = await import('./ReviewProvider');
   function ReviewEditor() {
@@ -126,7 +139,12 @@ vi.mock('./DiffViewerDialog', async () => {
   };
 });
 vi.mock('./merge-readiness', () => ({ buildMergeReadiness: () => ({ checks: [] }) }));
-vi.mock('../lib/theme', () => ({ theme: {}, bannerStyle: () => ({}) }));
+vi.mock('../lib/theme', () => ({
+  theme: {},
+  bannerStyle: () => ({}),
+  dialogButtonStyle: () => ({}),
+  linkButtonStyle: {},
+}));
 
 let dispose: (() => void) | undefined;
 afterEach(() => {
@@ -167,6 +185,7 @@ function tour(stops = 0): ChangeTourController {
 function mount(options: { task?: Task; tour?: ChangeTourController } = {}) {
   const handlers = {
     onClose: vi.fn(),
+    onCloseTask: vi.fn(),
     onOpenPullRequest: vi.fn(),
     onPushDone: vi.fn(),
     onRegenerateTour: vi.fn(),
@@ -356,7 +375,7 @@ describe('FinishDialog GitHub', () => {
   it('shows an empty state without linking an issue as a PR', () => {
     mount({ task: task({ githubUrl: 'https://github.com/o/r/issues/2' }) });
     expect(document.body.textContent).toContain('No pull request detected');
-    expect(button('PR #2 · Details and actions…')).toBeUndefined();
+    expect(button('PR #2 details…')).toBeUndefined();
   });
 
   it.each(['prUrl', 'githubUrl'] as const)(
@@ -376,11 +395,199 @@ describe('FinishDialog GitHub', () => {
       const url = 'https://github.com/o/r/pull/42';
       const handlers = mount({ task: task({ [field]: url }) });
       expect(document.body.textContent).toContain('Unit tests: fail');
-      expect(document.body.textContent).toContain('Draft pull request');
-      expect(document.body.textContent).toContain('Review: changes requested');
+      expect(document.body.textContent).toContain('Draft · Review: changes requested');
       expect(document.body.textContent).toContain('PR has conflicts');
-      button('PR #42 · Details and actions…')?.click();
+      button('PR #42 details…')?.click();
       expect(handlers.onOpenPullRequest).toHaveBeenCalledWith(url);
     },
   );
 });
+
+const PR_URL = 'https://github.com/o/r/pull/42';
+
+function prDetails(overrides: Partial<PullRequestDetails> = {}): PullRequestDetails {
+  return {
+    number: 42,
+    title: 'Parent',
+    url: PR_URL,
+    state: 'OPEN',
+    isDraft: false,
+    mergeable: 'MERGEABLE',
+    mergeStateStatus: 'CLEAN',
+    baseRefName: 'main',
+    headRefName: 'task/parent',
+    headRefOid: 'local',
+    mergeMethods: ['squash', 'merge'],
+    ...overrides,
+  };
+}
+
+describe('FinishDialog with an open PR', () => {
+  it('merges the PR on GitHub, then offers to close the task', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails());
+    vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails({ state: 'MERGED' }));
+    vi.mocked(mergePullRequestForTask).mockResolvedValueOnce(true);
+    const handlers = mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    expect(confirmButton()?.textContent).toBe('Squash and merge PR #42');
+    // Local merge options would describe a merge that will not happen.
+    expect(document.body.textContent).not.toContain('Squash commits');
+    confirmButton()?.click();
+    await flush();
+    await flush();
+    expect(mergePullRequestForTask).toHaveBeenCalledWith('parent', {
+      prUrl: PR_URL,
+      method: 'squash',
+      headSha: 'local',
+    });
+    expect(mergeTask).not.toHaveBeenCalled();
+    expect(handlers.onClose).not.toHaveBeenCalled();
+    expect(confirmButton()?.textContent).toBe('Close task');
+    confirmButton()?.click();
+    expect(handlers.onClose).toHaveBeenCalled();
+    expect(handlers.onCloseTask).toHaveBeenCalled();
+  });
+
+  it('blocks the PR merge while the PR lacks the latest local commit', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails({ headRefOid: 'older' }));
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    expect(confirmButton()?.disabled).toBe(true);
+    expect(document.querySelector('[data-testid="footer-note"]')?.textContent).toContain(
+      'Push the branch first',
+    );
+  });
+
+  it('can merge locally instead, and switch back', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails());
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    button('Merge locally instead')?.click();
+    expect(confirmButton()?.textContent).toBe('Merge into main');
+    button('Use PR #42 instead')?.click();
+    expect(confirmButton()?.textContent).toBe('Squash and merge PR #42');
+    button('Merge locally instead')?.click();
+    confirmButton()?.click();
+    expect(mergeTask).toHaveBeenCalled();
+    expect(mergePullRequestForTask).not.toHaveBeenCalled();
+  });
+
+  it('falls back to a local merge when the PR cannot be loaded', async () => {
+    vi.mocked(getPullRequestDetails).mockRejectedValue(new Error('gh: not logged in'));
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    await flush();
+    expect(confirmButton()?.textContent).toBe('Merge into main');
+    expect(document.body.textContent).toContain('gh: not logged in');
+  });
+
+  it('stages a Fix CI prompt and closes so it can be reviewed', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails());
+    vi.mocked(stageFailedChecksPrompt).mockResolvedValueOnce(true);
+    vi.mocked(getPrChecks).mockReturnValue({
+      overall: 'failure',
+      passing: 0,
+      pending: 0,
+      failing: 1,
+      checks: [{ name: 'Unit tests', bucket: 'fail' }],
+      checkedAt: '',
+    });
+    const handlers = mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    button('Fix CI')?.click();
+    await flush();
+    expect(stageFailedChecksPrompt).toHaveBeenCalledWith('parent', { number: 42, url: PR_URL });
+    expect(handlers.onClose).toHaveBeenCalled();
+  });
+
+  it('stays on Close task when the refetch after a merge fails', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails());
+    vi.mocked(getPullRequestDetails).mockRejectedValueOnce(new Error('rate limited'));
+    vi.mocked(mergePullRequestForTask).mockResolvedValueOnce(true);
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    confirmButton()?.click();
+    await flush();
+    await flush();
+    expect(confirmButton()?.textContent).toBe('Close task');
+    expect(button('Merge locally instead')).toBeUndefined();
+    expect(document.body.textContent).not.toContain('merges locally');
+  });
+
+  it('closes when GitHub only queued the merge', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails());
+    vi.mocked(mergePullRequestForTask).mockResolvedValueOnce(false);
+    const handlers = mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    confirmButton()?.click();
+    await flush();
+    expect(handlers.onClose).toHaveBeenCalled();
+    expect(handlers.onCloseTask).not.toHaveBeenCalled();
+  });
+
+  it('keeps the dialog open with the error when the PR merge fails', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails());
+    vi.mocked(mergePullRequestForTask).mockRejectedValueOnce(new Error('head moved'));
+    const handlers = mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    confirmButton()?.click();
+    await flush();
+    expect(handlers.onClose).not.toHaveBeenCalled();
+    expect(document.querySelector('[role="alert"]')?.textContent).toContain('head moved');
+  });
+
+  it.each([
+    [{ state: 'CLOSED' as const }, 'PR #42 is closed'],
+    [{ headRefName: 'someone/else' }, 'PR #42 is on someone/else'],
+    [{ headRefName: 'someone/else', state: 'MERGED' as const }, 'PR #42 is on someone/else'],
+  ])('merges locally when the PR cannot carry the work: %j', async (overrides, note) => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails(overrides));
+    mount({ task: task({ githubUrl: PR_URL }) });
+    await flush();
+    expect(confirmButton()?.textContent).toBe('Merge into main');
+    expect(document.body.textContent).toContain(note);
+  });
+
+  it('clears the push-first blocker once a push lands on the PR', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails({ headRefOid: 'older' }));
+    vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails());
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    expect(confirmButton()?.disabled).toBe(true);
+    button('Push branch')?.click();
+    await flush();
+    await flush();
+    expect(confirmButton()?.disabled).toBe(false);
+    expect(confirmButton()?.textContent).toBe('Squash and merge PR #42');
+  });
+
+  it('retries loading the PR after a failure', async () => {
+    vi.mocked(getPullRequestDetails).mockRejectedValueOnce(new Error('offline'));
+    vi.mocked(getPullRequestDetails).mockResolvedValueOnce(prDetails());
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    button('Retry')?.click();
+    await flush();
+    expect(confirmButton()?.textContent).toBe('Squash and merge PR #42');
+  });
+
+  it('offers agent hand-offs only when CI fails or changes are requested', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(prDetails());
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    expect(button('Fix CI')).toBeUndefined();
+    expect(button('Address review')).toBeUndefined();
+  });
+
+  it('offers a local merge for commits made after the PR merged', async () => {
+    vi.mocked(getPullRequestDetails).mockResolvedValue(
+      prDetails({ state: 'MERGED', headRefOid: 'older' }),
+    );
+    mount({ task: task({ prUrl: PR_URL }) });
+    await flush();
+    expect(confirmButton()?.textContent).toBe('Close task');
+    expect(document.body.textContent).toContain('commits that are not in PR #42');
+    button('Merge locally instead')?.click();
+    expect(confirmButton()?.textContent).toBe('Merge into main');
+  });
+});