From e4f086f28f94d94affaf980f99eaabf9a61fc77d Mon Sep 17 00:00:00 2001 From: Daniel Wise Date: Mon, 9 Mar 2026 21:11:16 -0700 Subject: [PATCH 1/2] fix(doctor): surface remote GitHub base conflicts Refresh tracked trunk refs during post-merge maintenance, surface remote-base mismatch issues in doctor, and make merge-check fail when GitHub still reports a PR as conflicting. --- QUICKSTART.md | 3 +- README.md | 13 +++- apps/docs/content/docs/commands/doctor.mdx | 14 +++- packages/cli/src/commands/doctor.test.ts | 76 +++++++++++++++++++ packages/cli/src/commands/doctor.ts | 28 +++++++ packages/cli/src/commands/merge-check.test.ts | 32 ++++++++ packages/cli/src/commands/merge-check.ts | 11 +++ packages/cli/src/commands/post-merge.test.ts | 35 ++++++++- packages/cli/src/commands/post-merge.ts | 24 +++++- packages/cli/src/lib/github.test.ts | 26 +++++++ packages/cli/src/lib/github.ts | 63 +++++++++++++++ 11 files changed, 319 insertions(+), 6 deletions(-) diff --git a/QUICKSTART.md b/QUICKSTART.md index 9934db0..b4c93e7 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -220,6 +220,7 @@ dub doctor dub ready dub prune # preview stale tracked branches dub prune --apply # apply stale metadata cleanup +dub merge-check # verify stack order and GitHub mergeability ``` ## 8) Handle Restack Conflicts @@ -319,7 +320,7 @@ dub undo | `dub doctor` | Run stack health checks | | `dub ready` | Run pre-submit checklist | | `dub prune` | Preview/remove stale tracked metadata | -| `dub merge-check` | Validate stack merge order | +| `dub merge-check` | Validate stack merge order and GitHub mergeability | | `dub merge-next` / `dub land` | Merge next safe PR + maintenance | | `dub post-merge` | Repair state/retarget after manual merges | | `dub restack` | Rebase stack onto updated parents | diff --git a/README.md b/README.md index d94f989..02f9eca 100644 --- a/README.md +++ b/README.md @@ -517,6 +517,15 @@ Checks include: - submit branching blockers - local/remote SHA drift - structural parent/child ancestry drift that can leave GitHub conflicted while local refs look clean +- remote GitHub base drift, where the remote PR head is no longer descended from the base branch GitHub is actually evaluating + +If `dub doctor` reports a GitHub base mismatch, refresh that base first, then replay and resubmit: + +```bash +git checkout main && git pull --ff-only origin main +dub restack +dub submit --path current +``` ### `dub ready` @@ -543,7 +552,7 @@ dub prune --all --apply ### `dub merge-check` -Validate merge order for a stack PR. +Validate merge order and GitHub mergeability for a stack PR. ```bash # check current branch PR @@ -861,7 +870,7 @@ dub restack --continue | Need stack-aware branch deletion | Use `dub delete` with `--upstack` / `--downstack` | | Sync skipped branch | Re-run with `--interactive` or `--force` as appropriate | | Wrong operation during create/restack | Use `dub undo` (single-level) | -| PR merge blocked by order | Run `dub merge-check --pr ` and merge previous PR first | +| PR merge blocked by order or GitHub conflict | Run `dub merge-check --pr ` to verify stack order and remote mergeability | | Manual merge left stack inconsistent | Run `dub post-merge` | ### Stale Branch Recovery diff --git a/apps/docs/content/docs/commands/doctor.mdx b/apps/docs/content/docs/commands/doctor.mdx index f9c2e7e..88ad3c0 100644 --- a/apps/docs/content/docs/commands/doctor.mdx +++ b/apps/docs/content/docs/commands/doctor.mdx @@ -23,6 +23,7 @@ Checks include: - submit blockers - local/remote SHA drift - structural parent/child ancestry drift, where a child branch is no longer based on its tracked parent even if local and remote refs match +- remote GitHub base drift, where the remote PR head is no longer descended from the base branch GitHub is actually evaluating If `dub doctor` reports that a branch is no longer based on its parent, start with: @@ -37,6 +38,14 @@ If the stack looks healthy after restacking and you still need to refresh the PR dub submit --path current ``` +If `dub doctor` reports that a branch is not based on its GitHub base, refresh that base first, then replay and resubmit the branch: + +```bash +git checkout main && git pull --ff-only origin main +dub restack +dub submit --path current +``` + ## dub ready Run pre-submit checklist (doctor + submit preflight): @@ -62,7 +71,7 @@ dub prune --all --apply ## dub merge-check -Validate merge order for a stack PR: +Validate merge order and GitHub mergeability for a stack PR: ```bash # Check current branch PR @@ -136,4 +145,7 @@ dub doctor # 4) Refresh the current PR path if needed dub submit --path current + +# 5) Confirm GitHub now sees the PR as mergeable +dub merge-check ``` diff --git a/packages/cli/src/commands/doctor.test.ts b/packages/cli/src/commands/doctor.test.ts index 000f8ad..68d50cc 100644 --- a/packages/cli/src/commands/doctor.test.ts +++ b/packages/cli/src/commands/doctor.test.ts @@ -9,6 +9,10 @@ vi.mock('../lib/git.js', () => ({ remoteBranchExists: vi.fn(), })); +vi.mock('../lib/github.js', () => ({ + getBranchPrSyncInfo: vi.fn(), +})); + vi.mock('../lib/operation-state.js', () => ({ detectActiveOperation: vi.fn(), })); @@ -29,6 +33,7 @@ import { isAncestor, remoteBranchExists, } from '../lib/git'; +import { getBranchPrSyncInfo } from '../lib/github'; import { detectActiveOperation } from '../lib/operation-state'; import type { DubState } from '../lib/state'; import { readState } from '../lib/state'; @@ -40,6 +45,7 @@ const mockGetCurrentBranch = getCurrentBranch as ReturnType; const mockGetRefSha = getRefSha as ReturnType; const mockIsAncestor = isAncestor as ReturnType; const mockRemoteBranchExists = remoteBranchExists as ReturnType; +const mockGetBranchPrSyncInfo = getBranchPrSyncInfo as ReturnType; const mockDetectActiveOperation = detectActiveOperation as ReturnType< typeof vi.fn >; @@ -69,6 +75,10 @@ beforeEach(() => { mockFetchBranches.mockResolvedValue(undefined); mockBranchExists.mockResolvedValue(true); mockRemoteBranchExists.mockResolvedValue(true); + mockGetBranchPrSyncInfo.mockResolvedValue({ + state: 'NONE', + baseRefName: null, + }); mockGetRefSha.mockImplementation(async (ref: string) => `${ref}-sha`); mockIsAncestor.mockResolvedValue(true); }); @@ -172,4 +182,70 @@ describe('doctor', () => { expect(issue?.fixes).toContain('dub doctor'); expect(result.healthy).toBe(false); }); + + it('reports a branch that is still based on its local parent but no longer based on the remote PR base', async () => { + mockGetCurrentBranch.mockResolvedValue('feat/hub-performance-streaming'); + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { name: 'docs/parity-pulse', parent: 'main' }, + { + name: 'feat/hub-performance-streaming', + parent: 'docs/parity-pulse', + }, + ]), + ); + mockGetBranchPrSyncInfo.mockImplementation(async (branch: string) => { + if (branch === 'feat/hub-performance-streaming') { + return { + state: 'OPEN', + baseRefName: 'main', + }; + } + return { + state: 'NONE', + baseRefName: null, + }; + }); + mockGetRefSha.mockImplementation(async (ref: string) => { + switch (ref) { + case 'docs/parity-pulse': + return 'docs-remote-sha'; + case 'feat/hub-performance-streaming': + case 'origin/feat/hub-performance-streaming': + return 'tip-sha'; + case 'main': + return 'main-local-sha'; + case 'origin/main': + return 'main-remote-sha'; + default: + return `${ref}-sha`; + } + }); + mockIsAncestor.mockImplementation(async (left: string, right: string) => { + if (left === 'docs-remote-sha' && right === 'tip-sha') { + return true; + } + if (left === 'main-remote-sha' && right === 'tip-sha') { + return false; + } + return true; + }); + + const result = await doctor('/repo'); + const issue = result.issues.find( + (entry) => entry.code === 'remote-base-mismatch', + ); + + expect(issue?.summary).toContain( + "Branch 'feat/hub-performance-streaming' is not based on GitHub base 'main'", + ); + expect(issue?.details).toContain('GitHub is evaluating this PR against'); + expect(issue?.fixes[0]).toBe( + 'git checkout main && git pull --ff-only origin main', + ); + expect(issue?.fixes).toContain('dub restack'); + expect(issue?.fixes).toContain('dub submit --path current'); + expect(result.healthy).toBe(false); + }); }); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index df8abfb..fc2ed08 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -7,6 +7,7 @@ import { isAncestor, remoteBranchExists, } from '../lib/git'; +import { getBranchPrSyncInfo } from '../lib/github'; import { detectActiveOperation } from '../lib/operation-state'; import { type Branch, @@ -21,6 +22,7 @@ export type DoctorIssueCode = | 'untracked-current-branch' | 'submit-branching-blocker' | 'parent-mismatch' + | 'remote-base-mismatch' | 'missing-local' | 'missing-remote' | 'remote-drift' @@ -211,6 +213,32 @@ async function appendBranchHealthIssues( }); } } + + let githubBaseRef: string | null = null; + try { + const prInfo = await getBranchPrSyncInfo(branch.name, cwd); + githubBaseRef = prInfo.state === 'OPEN' ? prInfo.baseRefName : null; + } catch { + githubBaseRef = null; + } + + if (githubBaseRef && (await remoteBranchExists(githubBaseRef, cwd))) { + const remoteBaseSha = await getRefSha(`origin/${githubBaseRef}`, cwd); + const basedOnRemoteBase = await isAncestor(remoteBaseSha, remoteSha, cwd); + if (!basedOnRemoteBase) { + issues.push({ + code: 'remote-base-mismatch', + summary: `Branch '${branch.name}' is not based on GitHub base '${githubBaseRef}'.`, + details: `GitHub is evaluating this PR against origin/${githubBaseRef}, but the remote branch tip is not descended from that base. GitHub may still report merge conflicts even when local parent checks pass.`, + fixes: [ + `git checkout ${githubBaseRef} && git pull --ff-only origin ${githubBaseRef}`, + 'dub restack', + 'dub submit --path current', + 'dub merge-check', + ], + }); + } + } } catch (error) { if (error instanceof DubError) { issues.push({ diff --git a/packages/cli/src/commands/merge-check.test.ts b/packages/cli/src/commands/merge-check.test.ts index a53069c..6a0bf8a 100644 --- a/packages/cli/src/commands/merge-check.test.ts +++ b/packages/cli/src/commands/merge-check.test.ts @@ -5,6 +5,7 @@ vi.mock('../lib/github.js', () => ({ ensureGhInstalled: vi.fn(), getPr: vi.fn(), getPrByNumber: vi.fn(), + getPrMergeStatusByNumber: vi.fn(), getPrStateByNumber: vi.fn(), })); @@ -18,6 +19,7 @@ import { ensureGhInstalled, getPr, getPrByNumber, + getPrMergeStatusByNumber, getPrStateByNumber, } from '../lib/github'; import { mergeCheck } from './merge-check'; @@ -26,6 +28,9 @@ const mockEnsureGhInstalled = ensureGhInstalled as ReturnType; const mockCheckGhAuth = checkGhAuth as ReturnType; const mockGetPr = getPr as ReturnType; const mockGetPrByNumber = getPrByNumber as ReturnType; +const mockGetPrMergeStatusByNumber = getPrMergeStatusByNumber as ReturnType< + typeof vi.fn +>; const mockGetPrStateByNumber = getPrStateByNumber as ReturnType; const mockGetCurrentBranch = getCurrentBranch as ReturnType; @@ -46,6 +51,10 @@ beforeEach(() => { title: 'feat: a', body: 'plain body', }); + mockGetPrMergeStatusByNumber.mockResolvedValue({ + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }); mockGetPrStateByNumber.mockResolvedValue('MERGED'); }); @@ -93,4 +102,27 @@ describe('mergeCheck', () => { 'cannot be merged yet', ); }); + + it('fails when GitHub reports the PR is conflicting', async () => { + mockGetPrByNumber.mockResolvedValue({ + number: 12, + url: 'https://github.com/o/r/pull/12', + title: 'feat: b', + body: [ + 'body', + '', + ].join('\n'), + }); + mockGetPrStateByNumber.mockResolvedValue('MERGED'); + mockGetPrMergeStatusByNumber.mockResolvedValue({ + mergeable: 'CONFLICTING', + mergeStateStatus: 'DIRTY', + }); + + await expect(mergeCheck('/repo', { pr: 12 })).rejects.toThrow( + 'PR #12 is not mergeable on GitHub', + ); + }); }); diff --git a/packages/cli/src/commands/merge-check.ts b/packages/cli/src/commands/merge-check.ts index bc0d400..b723206 100644 --- a/packages/cli/src/commands/merge-check.ts +++ b/packages/cli/src/commands/merge-check.ts @@ -5,6 +5,7 @@ import { ensureGhInstalled, getPr, getPrByNumber, + getPrMergeStatusByNumber, getPrStateByNumber, } from '../lib/github'; import { parseDubstackMetadata } from '../lib/pr-body'; @@ -58,6 +59,16 @@ export async function mergeCheck( ); } + const mergeStatus = await getPrMergeStatusByNumber(pr.number, cwd); + if ( + mergeStatus.mergeable === 'CONFLICTING' || + mergeStatus.mergeStateStatus === 'DIRTY' + ) { + throw new DubError( + `PR #${pr.number} is not mergeable on GitHub. GitHub reports mergeable='${mergeStatus.mergeable ?? 'unknown'}' and mergeStateStatus='${mergeStatus.mergeStateStatus ?? 'unknown'}'. Resolve or resubmit the branch before merging.`, + ); + } + return { ok: true, prNumber: pr.number, diff --git a/packages/cli/src/commands/post-merge.test.ts b/packages/cli/src/commands/post-merge.test.ts index b12c215..7d056ea 100644 --- a/packages/cli/src/commands/post-merge.test.ts +++ b/packages/cli/src/commands/post-merge.test.ts @@ -2,7 +2,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('../lib/git.js', () => ({ checkoutBranch: vi.fn(), + fastForwardBranchToRef: vi.fn(), + fetchBranches: vi.fn(), getCurrentBranch: vi.fn(), + remoteBranchExists: vi.fn(), })); vi.mock('../lib/state.js', async (importOriginal) => { @@ -30,7 +33,13 @@ vi.mock('./submit.js', () => ({ submit: vi.fn(), })); -import { checkoutBranch, getCurrentBranch } from '../lib/git'; +import { + checkoutBranch, + fastForwardBranchToRef, + fetchBranches, + getCurrentBranch, + remoteBranchExists, +} from '../lib/git'; import { checkGhAuth, ensureGhInstalled, @@ -45,7 +54,12 @@ import { restack } from './restack'; import { submit } from './submit'; const mockCheckoutBranch = checkoutBranch as ReturnType; +const mockFastForwardBranchToRef = fastForwardBranchToRef as ReturnType< + typeof vi.fn +>; +const mockFetchBranches = fetchBranches as ReturnType; const mockGetCurrentBranch = getCurrentBranch as ReturnType; +const mockRemoteBranchExists = remoteBranchExists as ReturnType; const mockEnsureGhInstalled = ensureGhInstalled as ReturnType; const mockCheckGhAuth = checkGhAuth as ReturnType; const mockGetBranchPrLifecycleState = getBranchPrLifecycleState as ReturnType< @@ -92,8 +106,11 @@ function makeState(): DubState { beforeEach(() => { vi.clearAllMocks(); mockGetCurrentBranch.mockResolvedValue('feat/b'); + mockFetchBranches.mockResolvedValue(undefined); + mockFastForwardBranchToRef.mockResolvedValue(true); mockEnsureGhInstalled.mockResolvedValue(undefined); mockCheckGhAuth.mockResolvedValue(undefined); + mockRemoteBranchExists.mockResolvedValue(true); mockReadState.mockResolvedValue(makeState()); mockWriteState.mockResolvedValue(undefined); mockGetBranchPrLifecycleState.mockImplementation(async (branch: string) => { @@ -193,6 +210,12 @@ describe('postMerge', () => { it('runs restack and submit maintenance by default', async () => { await postMerge('/repo'); + expect(mockFetchBranches).toHaveBeenCalledWith(['main'], '/repo'); + expect(mockFastForwardBranchToRef).toHaveBeenCalledWith( + 'main', + 'origin/main', + '/repo', + ); expect(mockRestack).toHaveBeenCalled(); expect(mockSubmit).toHaveBeenCalledWith('/repo', false, { path: 'stack', @@ -277,6 +300,16 @@ describe('postMerge', () => { expect(mockCheckoutBranch).toHaveBeenCalledWith('feat/b', '/repo'); }); + it('fails before restack when the local root cannot be fast-forwarded to remote', async () => { + mockFastForwardBranchToRef.mockResolvedValue(false); + + await expect(postMerge('/repo')).rejects.toThrow( + "Post-merge could not fast-forward trunk 'main' to 'origin/main'", + ); + expect(mockRestack).not.toHaveBeenCalled(); + expect(mockSubmit).not.toHaveBeenCalled(); + }); + it('submits each stack when --all is enabled', async () => { const allStacksState: DubState = { stacks: [ diff --git a/packages/cli/src/commands/post-merge.ts b/packages/cli/src/commands/post-merge.ts index b780691..1a6c87f 100644 --- a/packages/cli/src/commands/post-merge.ts +++ b/packages/cli/src/commands/post-merge.ts @@ -1,5 +1,11 @@ import { DubError } from '../lib/errors'; -import { checkoutBranch, getCurrentBranch } from '../lib/git'; +import { + checkoutBranch, + fastForwardBranchToRef, + fetchBranches, + getCurrentBranch, + remoteBranchExists, +} from '../lib/git'; import { checkGhAuth, ensureGhInstalled, @@ -99,6 +105,22 @@ export async function postMerge( (branch) => branch.type === 'root', )?.name; if (!root) continue; + await fetchBranches([root], cwd); + if (await remoteBranchExists(root, cwd)) { + const remoteRef = `origin/${root}`; + const fastForwarded = await fastForwardBranchToRef( + root, + remoteRef, + cwd, + ); + if (!fastForwarded) { + throw new DubError( + `Post-merge could not fast-forward trunk '${root}' to '${remoteRef}'.\n` + + 'Refresh your local trunk first, then rerun the maintenance flow.\n' + + ` git checkout ${root} && git pull --ff-only origin ${root}`, + ); + } + } await checkoutBranch(root, cwd); const restackResult = await restack(cwd); if (restackResult.status === 'conflict') { diff --git a/packages/cli/src/lib/github.test.ts b/packages/cli/src/lib/github.test.ts index fa99688..c6ee327 100644 --- a/packages/cli/src/lib/github.test.ts +++ b/packages/cli/src/lib/github.test.ts @@ -20,6 +20,7 @@ import { getBranchPrSyncInfo, getPr, getPrByNumber, + getPrMergeStatusByNumber, getPrStateByNumber, getRepositoryWebUrl, mergePr, @@ -183,6 +184,31 @@ describe('getPrStateByNumber', () => { }); }); +describe('getPrMergeStatusByNumber', () => { + it('returns mergeability fields when present', async () => { + mockExeca.mockResolvedValueOnce({ + stdout: JSON.stringify({ + mergeable: 'CONFLICTING', + mergeStateStatus: 'DIRTY', + }), + }); + await expect(getPrMergeStatusByNumber(5, '/repo')).resolves.toEqual({ + mergeable: 'CONFLICTING', + mergeStateStatus: 'DIRTY', + }); + }); + + it('returns null fields when the PR is missing', async () => { + mockExeca.mockRejectedValueOnce( + new Error('GraphQL: Could not resolve to a PullRequest with the number'), + ); + await expect(getPrMergeStatusByNumber(999999, '/repo')).resolves.toEqual({ + mergeable: null, + mergeStateStatus: null, + }); + }); +}); + describe('getBranchPrSyncInfo', () => { it('returns baseRefName when present', async () => { mockExeca.mockResolvedValueOnce({ diff --git a/packages/cli/src/lib/github.ts b/packages/cli/src/lib/github.ts index fbe5a55..713fcc2 100644 --- a/packages/cli/src/lib/github.ts +++ b/packages/cli/src/lib/github.ts @@ -17,6 +17,11 @@ export interface BranchPrSyncInfo { baseRefName: string | null; } +export interface PrMergeStatus { + mergeable: string | null; + mergeStateStatus: string | null; +} + /** * Ensures the `gh` CLI is installed and available in PATH. * @throws {DubError} If `gh` is not found. @@ -222,6 +227,64 @@ export async function getPrStateByNumber( } } +/** + * Returns GitHub's mergeability status for a PR by number. + */ +export async function getPrMergeStatusByNumber( + prNumber: number, + cwd: string, +): Promise { + let stdout: string; + try { + const result = await execa( + 'gh', + [ + 'pr', + 'view', + String(prNumber), + '--json', + 'mergeable,mergeStateStatus', + '--jq', + '.', + ], + { cwd }, + ); + stdout = result.stdout; + } catch (error) { + if (isPrNotFoundError(error)) { + return { + mergeable: null, + mergeStateStatus: null, + }; + } + const message = error instanceof Error ? error.message : String(error); + throw new DubError( + `Failed to fetch mergeability for PR #${prNumber}: ${message}`, + ); + } + + const trimmed = stdout.trim(); + if (!trimmed || trimmed === 'null') { + return { + mergeable: null, + mergeStateStatus: null, + }; + } + + try { + const parsed = JSON.parse(trimmed) as { + mergeable?: string | null; + mergeStateStatus?: string | null; + }; + return { + mergeable: parsed.mergeable ?? null, + mergeStateStatus: parsed.mergeStateStatus ?? null, + }; + } catch { + throw new DubError(`Failed to parse mergeability for PR #${prNumber}.`); + } +} + function isPrNotFoundError(error: unknown): boolean { const message = error instanceof Error ? error.message : String(error); const normalized = message.toLowerCase(); From bcf8091783dad443b62f533b0b6059c933e2f4d3 Mon Sep 17 00:00:00 2001 From: Daniel Wise Date: Mon, 9 Mar 2026 21:24:45 -0700 Subject: [PATCH 2/2] fix(review): harden GitHub mergeability checks Fetch GitHub base refs before remote-base ancestry checks, surface GitHub query failures in doctor, and require explicitly safe GitHub mergeability states in merge-check. --- packages/cli/src/commands/doctor.test.ts | 89 +++++++++++++++++++ packages/cli/src/commands/doctor.ts | 49 ++++++++-- packages/cli/src/commands/merge-check.test.ts | 23 +++++ packages/cli/src/commands/merge-check.ts | 14 +-- 4 files changed, 163 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/commands/doctor.test.ts b/packages/cli/src/commands/doctor.test.ts index 68d50cc..d50f2a8 100644 --- a/packages/cli/src/commands/doctor.test.ts +++ b/packages/cli/src/commands/doctor.test.ts @@ -248,4 +248,93 @@ describe('doctor', () => { expect(issue?.fixes).toContain('dub submit --path current'); expect(result.healthy).toBe(false); }); + + it('fetches the GitHub base ref before checking remote-base mismatch', async () => { + mockGetCurrentBranch.mockResolvedValue('feat/hub-performance-streaming'); + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { + name: 'feat/hub-performance-streaming', + parent: 'main', + }, + ]), + ); + const fetchedRefs = new Set(); + mockGetBranchPrSyncInfo.mockResolvedValue({ + state: 'OPEN', + baseRefName: 'release/1.95', + }); + mockFetchBranches.mockImplementation(async (refs: string[]) => { + for (const ref of refs) fetchedRefs.add(ref); + }); + mockRemoteBranchExists.mockImplementation(async (branch: string) => { + if (branch === 'release/1.95') { + return fetchedRefs.has('release/1.95'); + } + return true; + }); + mockGetRefSha.mockImplementation(async (ref: string) => { + switch (ref) { + case 'feat/hub-performance-streaming': + case 'origin/feat/hub-performance-streaming': + return 'tip-sha'; + case 'main': + return 'main-local-sha'; + case 'origin/release/1.95': + return 'release-remote-sha'; + default: + return `${ref}-sha`; + } + }); + mockIsAncestor.mockImplementation(async (left: string, right: string) => { + if (left === 'main-local-sha' && right === 'tip-sha') { + return true; + } + if (left === 'release-remote-sha' && right === 'tip-sha') { + return false; + } + return true; + }); + + const result = await doctor('/repo'); + const issue = result.issues.find( + (entry) => entry.code === 'remote-base-mismatch', + ); + + expect(mockFetchBranches).toHaveBeenCalledWith( + expect.arrayContaining(['release/1.95']), + '/repo', + ); + expect(issue?.summary).toContain( + "Branch 'feat/hub-performance-streaming' is not based on GitHub base 'release/1.95'", + ); + }); + + it('surfaces a remote-check-failed issue when the GitHub base query fails', async () => { + mockGetCurrentBranch.mockResolvedValue('feat/hub-performance-streaming'); + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { + name: 'feat/hub-performance-streaming', + parent: 'main', + }, + ]), + ); + mockGetBranchPrSyncInfo.mockRejectedValue(new Error('gh auth failed')); + + const result = await doctor('/repo'); + const issue = result.issues.find( + (entry) => entry.code === 'remote-check-failed', + ); + + expect(issue?.summary).toContain( + "Could not query GitHub PR info for 'feat/hub-performance-streaming'.", + ); + expect(issue?.details).toContain('gh auth failed'); + expect(issue?.fixes).toContain('gh auth status'); + expect(issue?.fixes).toContain('gh auth login'); + expect(result.healthy).toBe(false); + }); }); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index fc2ed08..4a5d445 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -218,10 +218,20 @@ async function appendBranchHealthIssues( try { const prInfo = await getBranchPrSyncInfo(branch.name, cwd); githubBaseRef = prInfo.state === 'OPEN' ? prInfo.baseRefName : null; - } catch { + } catch (error) { + pushGithubCheckFailure( + issues, + branch.name, + error, + `Could not query GitHub PR info for '${branch.name}'.`, + ); githubBaseRef = null; } + if (githubBaseRef) { + await fetchBranches([githubBaseRef], cwd); + } + if (githubBaseRef && (await remoteBranchExists(githubBaseRef, cwd))) { const remoteBaseSha = await getRefSha(`origin/${githubBaseRef}`, cwd); const basedOnRemoteBase = await isAncestor(remoteBaseSha, remoteSha, cwd); @@ -241,12 +251,37 @@ async function appendBranchHealthIssues( } } catch (error) { if (error instanceof DubError) { - issues.push({ - code: 'remote-check-failed', - summary: `Could not compare local/remote SHAs for '${branch.name}'.`, - details: error.message, - fixes: ['git fetch --all --prune', 'dub sync --no-restack'], - }); + pushRemoteCheckFailed( + issues, + `Could not compare local/remote SHAs for '${branch.name}'.`, + error.message, + ); } } } + +function pushGithubCheckFailure( + issues: DoctorIssue[], + branchName: string, + error: unknown, + summary: string, +): void { + const details = + error instanceof Error && error.message + ? error.message + : `The GitHub PR base-drift check could not be performed for '${branchName}'. Ensure the GitHub CLI ('gh') is installed, authenticated, and that the repository has network access.`; + pushRemoteCheckFailed(issues, summary, details); +} + +function pushRemoteCheckFailed( + issues: DoctorIssue[], + summary: string, + details: string, +): void { + issues.push({ + code: 'remote-check-failed', + summary, + details, + fixes: ['gh auth status', 'gh auth login', 'git fetch --all --prune'], + }); +} diff --git a/packages/cli/src/commands/merge-check.test.ts b/packages/cli/src/commands/merge-check.test.ts index 6a0bf8a..d7ca0da 100644 --- a/packages/cli/src/commands/merge-check.test.ts +++ b/packages/cli/src/commands/merge-check.test.ts @@ -125,4 +125,27 @@ describe('mergeCheck', () => { 'PR #12 is not mergeable on GitHub', ); }); + + it('fails when GitHub mergeability is not explicitly safe', async () => { + mockGetPrByNumber.mockResolvedValue({ + number: 12, + url: 'https://github.com/o/r/pull/12', + title: 'feat: b', + body: [ + 'body', + '', + ].join('\n'), + }); + mockGetPrStateByNumber.mockResolvedValue('MERGED'); + mockGetPrMergeStatusByNumber.mockResolvedValue({ + mergeable: 'UNKNOWN', + mergeStateStatus: 'BLOCKED', + }); + + await expect(mergeCheck('/repo', { pr: 12 })).rejects.toThrow( + "GitHub reports mergeable='UNKNOWN' and mergeStateStatus='BLOCKED'", + ); + }); }); diff --git a/packages/cli/src/commands/merge-check.ts b/packages/cli/src/commands/merge-check.ts index b723206..b3405ea 100644 --- a/packages/cli/src/commands/merge-check.ts +++ b/packages/cli/src/commands/merge-check.ts @@ -16,6 +16,8 @@ export interface MergeCheckResult { reason: string; } +const SAFE_MERGE_STATE_STATUSES = new Set(['CLEAN', 'HAS_HOOKS']); + export async function mergeCheck( cwd: string, options: { pr?: number; branch?: string } = {}, @@ -60,12 +62,14 @@ export async function mergeCheck( } const mergeStatus = await getPrMergeStatusByNumber(pr.number, cwd); - if ( - mergeStatus.mergeable === 'CONFLICTING' || - mergeStatus.mergeStateStatus === 'DIRTY' - ) { + const mergeable = mergeStatus.mergeable ?? 'unknown'; + const mergeStateStatus = mergeStatus.mergeStateStatus ?? 'unknown'; + const safelyMergeable = + mergeable === 'MERGEABLE' && + SAFE_MERGE_STATE_STATUSES.has(mergeStateStatus); + if (!safelyMergeable) { throw new DubError( - `PR #${pr.number} is not mergeable on GitHub. GitHub reports mergeable='${mergeStatus.mergeable ?? 'unknown'}' and mergeStateStatus='${mergeStatus.mergeStateStatus ?? 'unknown'}'. Resolve or resubmit the branch before merging.`, + `PR #${pr.number} is not mergeable on GitHub. GitHub reports mergeable='${mergeable}' and mergeStateStatus='${mergeStateStatus}'. Resolve or resubmit the branch before merging.`, ); }