diff --git a/README.md b/README.md index 516ff5d..d94f989 100644 --- a/README.md +++ b/README.md @@ -516,6 +516,7 @@ Checks include: - missing tracked local/remote branches - submit branching blockers - local/remote SHA drift +- structural parent/child ancestry drift that can leave GitHub conflicted while local refs look clean ### `dub ready` diff --git a/apps/docs/content/docs/commands/doctor.mdx b/apps/docs/content/docs/commands/doctor.mdx index 9606723..f9c2e7e 100644 --- a/apps/docs/content/docs/commands/doctor.mdx +++ b/apps/docs/content/docs/commands/doctor.mdx @@ -17,7 +17,25 @@ dub doctor --all dub doctor --no-fetch ``` -Checks include: in-progress operation detection, missing tracked branches, submit blockers, and local/remote SHA drift. +Checks include: +- in-progress operation detection +- missing tracked branches +- 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 + +If `dub doctor` reports that a branch is no longer based on its parent, start with: + +```bash +dub restack +dub doctor +``` + +If the stack looks healthy after restacking and you still need to refresh the PR from your current branch, follow with: + +```bash +dub submit --path current +``` ## dub ready @@ -101,3 +119,21 @@ dub ready # 5) Submit current linear path dub submit --path current ``` + +## Hidden Conflict Recovery + +If GitHub shows a stacked child PR as conflicting but your local branch looks clean: + +```bash +# 1) Surface structural drift locally +dub doctor + +# 2) Replay children onto their tracked parents +dub restack + +# 3) Re-check health before pushing +dub doctor + +# 4) Refresh the current PR path if needed +dub submit --path current +``` diff --git a/packages/cli/src/commands/doctor.test.ts b/packages/cli/src/commands/doctor.test.ts index 38713e7..000f8ad 100644 --- a/packages/cli/src/commands/doctor.test.ts +++ b/packages/cli/src/commands/doctor.test.ts @@ -5,6 +5,7 @@ vi.mock('../lib/git.js', () => ({ fetchBranches: vi.fn(), getCurrentBranch: vi.fn(), getRefSha: vi.fn(), + isAncestor: vi.fn(), remoteBranchExists: vi.fn(), })); @@ -25,6 +26,7 @@ import { fetchBranches, getCurrentBranch, getRefSha, + isAncestor, remoteBranchExists, } from '../lib/git'; import { detectActiveOperation } from '../lib/operation-state'; @@ -36,6 +38,7 @@ const mockBranchExists = branchExists as ReturnType; const mockFetchBranches = fetchBranches as ReturnType; const mockGetCurrentBranch = getCurrentBranch as ReturnType; const mockGetRefSha = getRefSha as ReturnType; +const mockIsAncestor = isAncestor as ReturnType; const mockRemoteBranchExists = remoteBranchExists as ReturnType; const mockDetectActiveOperation = detectActiveOperation as ReturnType< typeof vi.fn @@ -67,6 +70,7 @@ beforeEach(() => { mockBranchExists.mockResolvedValue(true); mockRemoteBranchExists.mockResolvedValue(true); mockGetRefSha.mockImplementation(async (ref: string) => `${ref}-sha`); + mockIsAncestor.mockResolvedValue(true); }); describe('doctor', () => { @@ -127,4 +131,45 @@ describe('doctor', () => { ); expect(branching?.details).toContain('main -> feat/a, feat/b'); }); + + it('reports a child branch that matches remote but is no longer based on its parent', async () => { + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { name: 'feat/a', parent: 'main' }, + { name: 'feat/b', parent: 'feat/a' }, + ]), + ); + mockGetRefSha.mockImplementation(async (ref: string) => { + switch (ref) { + case 'feat/a': + case 'origin/feat/a': + return 'parent-new-sha'; + case 'feat/b': + case 'origin/feat/b': + return 'child-remote-sha'; + default: + return `${ref}-sha`; + } + }); + mockIsAncestor.mockImplementation(async (left: string, right: string) => { + if (left === 'parent-new-sha' && right === 'child-remote-sha') { + return false; + } + return true; + }); + + const result = await doctor('/repo'); + const issue = result.issues.find( + (entry) => entry.code === 'parent-mismatch', + ); + + expect(issue?.summary).toContain( + "Branch 'feat/b' is no longer based on 'feat/a'", + ); + expect(issue?.details).toContain('structural stack drift'); + expect(issue?.fixes[0]).toBe('dub restack'); + expect(issue?.fixes).toContain('dub doctor'); + expect(result.healthy).toBe(false); + }); }); diff --git a/packages/cli/src/commands/doctor.ts b/packages/cli/src/commands/doctor.ts index 617a333..df8abfb 100644 --- a/packages/cli/src/commands/doctor.ts +++ b/packages/cli/src/commands/doctor.ts @@ -4,6 +4,7 @@ import { fetchBranches, getCurrentBranch, getRefSha, + isAncestor, remoteBranchExists, } from '../lib/git'; import { detectActiveOperation } from '../lib/operation-state'; @@ -19,6 +20,7 @@ export type DoctorIssueCode = | 'operation-in-progress' | 'untracked-current-branch' | 'submit-branching-blocker' + | 'parent-mismatch' | 'missing-local' | 'missing-remote' | 'remote-drift' @@ -195,6 +197,20 @@ async function appendBranchHealthIssues( fixes: ['dub sync --no-restack', 'dub sync --force --no-restack'], }); } + + if (branch.parent) { + const parentSha = await getRefSha(branch.parent, cwd); + const basedOnParent = await isAncestor(parentSha, localSha, cwd); + if (!basedOnParent) { + issues.push({ + code: 'parent-mismatch', + summary: `Branch '${branch.name}' is no longer based on '${branch.parent}'.`, + details: + 'The tracked child branch is not descended from the current tip of its tracked parent, so structural stack drift is present and local submit/readiness checks would be misleading.', + fixes: ['dub restack', 'dub doctor', 'dub submit --path current'], + }); + } + } } catch (error) { if (error instanceof DubError) { issues.push({ diff --git a/packages/cli/src/commands/sync.test.ts b/packages/cli/src/commands/sync.test.ts index 53e9937..97f87c4 100644 --- a/packages/cli/src/commands/sync.test.ts +++ b/packages/cli/src/commands/sync.test.ts @@ -62,6 +62,7 @@ import { import { detectActiveOperation } from '../lib/operation-state'; import type { DubState } from '../lib/state'; import { readState, writeState } from '../lib/state'; +import { doctor } from './doctor'; import { restack } from './restack'; import { submit } from './submit'; import { sync } from './sync'; @@ -566,6 +567,358 @@ describe('sync', () => { }); }); + it('preserves the last submitted base sha when forced sync adopts an older remote child tip', async () => { + mockReadState.mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + parent: null, + type: 'root', + pr_number: null, + pr_link: null, + last_submitted_version: null, + last_synced_at: null, + sync_source: null, + }, + { + name: 'feat/a', + parent: 'main', + parent_revision: 'main-sha', + pr_number: null, + pr_link: null, + last_submitted_version: { + head_sha: 'parent-new-sha', + base_sha: 'main-sha', + base_branch: 'main', + version_number: null, + source: 'submit', + }, + last_reconciled_version: { + head_sha: 'parent-new-sha', + base_sha: 'main-sha', + base_branch: 'main', + source: 'submit', + }, + last_synced_at: null, + sync_source: 'submit', + }, + { + name: 'feat/b', + parent: 'feat/a', + parent_revision: 'parent-new-sha', + pr_number: null, + pr_link: null, + last_submitted_version: { + head_sha: 'child-local-sha', + base_sha: 'parent-old-sha', + base_branch: 'feat/a', + version_number: null, + source: 'submit', + }, + last_reconciled_version: { + head_sha: 'child-local-sha', + base_sha: 'parent-new-sha', + base_branch: 'feat/a', + source: 'sync-restack', + }, + last_synced_at: null, + sync_source: 'submit', + }, + ], + }, + ], + }); + mockGetCurrentBranch.mockResolvedValue('feat/a'); + mockGetRefSha.mockImplementation(async (ref: string) => { + switch (ref) { + case 'main': + case 'origin/main': + return 'main-sha'; + case 'feat/a': + case 'origin/feat/a': + return 'parent-new-sha'; + case 'feat/b': + return 'child-local-sha'; + case 'origin/feat/b': + return 'child-remote-sha'; + default: + return `${ref}-sha`; + } + }); + mockIsAncestor.mockImplementation(async (left: string, right: string) => { + if (left === 'main-sha' && right === 'parent-new-sha') return true; + if (left === 'feat/b' && right === 'origin/feat/b') return true; + if (left === 'origin/feat/b' && right === 'feat/b') return false; + if (left === 'feat/a' && right === 'origin/feat/a') return true; + if (left === 'origin/feat/a' && right === 'feat/a') return true; + if (left === 'parent-old-sha' && right === 'child-remote-sha') + return true; + if (left === 'parent-new-sha' && right === 'child-remote-sha') + return false; + return false; + }); + mockGetBranchPrSyncInfo.mockImplementation(async (branch: string) => { + if (branch === 'feat/b') { + return { state: 'OPEN', baseRefName: 'feat/a' }; + } + return { state: 'OPEN', baseRefName: 'main' }; + }); + + const result = await sync('/repo', { + interactive: false, + force: true, + restack: false, + }); + + expect( + result.branches.find((branch) => branch.branch === 'feat/b')?.action, + ).toBe('synced'); + const writtenState = mockWriteState.mock.calls.at(-1)?.[0] as DubState; + const featB = writtenState.stacks[0].branches.find( + (branch) => branch.name === 'feat/b', + ); + expect(featB?.last_submitted_version?.base_sha).toBe('parent-old-sha'); + expect(featB?.last_reconciled_version?.base_sha).toBe('parent-old-sha'); + expect(featB?.parent_revision).toBe('parent-old-sha'); + }); + + it('clears parent_revision when forced sync cannot trust the prior parent base', async () => { + mockReadState.mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + parent: null, + type: 'root', + pr_number: null, + pr_link: null, + last_submitted_version: null, + last_synced_at: null, + sync_source: null, + }, + { + name: 'feat/a', + parent: 'local-parent', + parent_revision: 'stale-parent-sha', + pr_number: null, + pr_link: null, + last_submitted_version: { + head_sha: 'child-local-sha', + base_sha: 'local-parent-sha', + base_branch: 'local-parent', + version_number: null, + source: 'submit', + }, + last_reconciled_version: { + head_sha: 'child-local-sha', + base_sha: 'stale-parent-sha', + base_branch: 'local-parent', + source: 'sync-restack', + }, + last_synced_at: null, + sync_source: 'submit', + }, + ], + }, + ], + }); + mockGetCurrentBranch.mockResolvedValue('feat/a'); + mockGetRefSha.mockImplementation(async (ref: string) => { + switch (ref) { + case 'feat/a': + return 'child-local-sha'; + case 'origin/feat/a': + return 'child-remote-sha'; + case 'remote-parent': + return 'remote-parent-sha'; + default: + return `${ref}-sha`; + } + }); + mockIsAncestor.mockImplementation(async (left: string, right: string) => { + if (left === 'feat/a' && right === 'origin/feat/a') return false; + if (left === 'origin/feat/a' && right === 'feat/a') return false; + if (left === 'remote-parent-sha' && right === 'child-remote-sha') + return false; + return false; + }); + mockGetBranchPrSyncInfo.mockResolvedValue({ + state: 'OPEN', + baseRefName: 'remote-parent', + }); + + const result = await sync('/repo', { + interactive: false, + force: true, + restack: false, + }); + + expect(result.branches[0]?.action).toBe('synced'); + const writtenState = mockWriteState.mock.calls.at(-1)?.[0] as DubState; + const featA = writtenState.stacks[0].branches.find( + (branch) => branch.name === 'feat/a', + ); + expect(featA?.parent).toBe('remote-parent'); + expect(featA?.last_submitted_version?.base_sha).toBe('remote-parent-sha'); + expect(featA?.parent_revision).toBeNull(); + }); + + it('reports parent-mismatch after forced sync adopts an older remote child tip', async () => { + let currentState: DubState = { + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + parent: null, + type: 'root', + pr_number: null, + pr_link: null, + last_submitted_version: null, + last_synced_at: null, + sync_source: null, + }, + { + name: 'feat/a', + parent: 'main', + parent_revision: 'main-sha', + pr_number: null, + pr_link: null, + last_submitted_version: { + head_sha: 'parent-new-sha', + base_sha: 'main-sha', + base_branch: 'main', + version_number: null, + source: 'submit', + }, + last_reconciled_version: { + head_sha: 'parent-new-sha', + base_sha: 'main-sha', + base_branch: 'main', + source: 'submit', + }, + last_synced_at: null, + sync_source: 'submit', + }, + { + name: 'feat/b', + parent: 'feat/a', + parent_revision: 'parent-new-sha', + pr_number: null, + pr_link: null, + last_submitted_version: { + head_sha: 'child-local-sha', + base_sha: 'parent-old-sha', + base_branch: 'feat/a', + version_number: null, + source: 'submit', + }, + last_reconciled_version: { + head_sha: 'child-local-sha', + base_sha: 'parent-new-sha', + base_branch: 'feat/a', + source: 'sync-restack', + }, + last_synced_at: null, + sync_source: 'submit', + }, + ], + }, + ], + }; + const refShas: Record = { + main: 'main-sha', + 'origin/main': 'main-sha', + 'feat/a': 'parent-new-sha', + 'origin/feat/a': 'parent-new-sha', + 'feat/b': 'child-local-sha', + 'origin/feat/b': 'child-remote-sha', + }; + mockReadState.mockImplementation(async () => structuredClone(currentState)); + mockWriteState.mockImplementation(async (nextState: DubState) => { + currentState = structuredClone(nextState); + }); + mockHardResetBranchToRef.mockImplementation(async (branch: string) => { + refShas[branch] = refShas[`origin/${branch}`]; + }); + mockGetRefSha.mockImplementation(async (ref: string) => { + return refShas[ref] ?? `${ref}-sha`; + }); + mockIsAncestor.mockImplementation(async (left: string, right: string) => { + if (left === 'feat/b' && right === 'origin/feat/b') return true; + if (left === 'origin/feat/b' && right === 'feat/b') return false; + if (left === 'feat/a' && right === 'origin/feat/a') return true; + if (left === 'origin/feat/a' && right === 'feat/a') return true; + if (left === 'parent-old-sha' && right === 'child-remote-sha') + return true; + if (left === 'parent-new-sha' && right === 'child-remote-sha') + return false; + return false; + }); + mockGetBranchPrSyncInfo.mockImplementation(async (branch: string) => { + if (branch === 'feat/b') { + return { state: 'OPEN', baseRefName: 'feat/a' }; + } + return { state: 'OPEN', baseRefName: 'main' }; + }); + + await sync('/repo', { + interactive: false, + force: true, + restack: false, + }); + + const result = await doctor('/repo'); + const issue = result.issues.find( + (entry) => + entry.code === 'parent-mismatch' && + entry.summary.includes("Branch 'feat/b'"), + ); + + expect(issue?.summary).toContain( + "Branch 'feat/b' is no longer based on 'feat/a'", + ); + expect(issue?.fixes[0]).toBe('dub restack'); + expect(result.healthy).toBe(false); + }); + + it('keeps local-ahead branches when PR base mismatches the tracked parent', async () => { + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { name: 'feat/a', parent: 'local-parent' }, + ]), + ); + mockGetRefSha + .mockResolvedValueOnce('local-sha') + .mockResolvedValueOnce('remote-sha'); + mockIsAncestor.mockImplementation(async (left: string, right: string) => { + if (left === 'feat/a' && right === 'origin/feat/a') return false; + if (left === 'origin/feat/a' && right === 'feat/a') return true; + return false; + }); + mockGetBranchPrSyncInfo.mockResolvedValue({ + state: 'OPEN', + baseRefName: 'remote-parent', + }); + + const result = await sync('/repo', { + interactive: false, + force: true, + restack: false, + }); + + expect(result.branches[0].status).toBe('local-ahead'); + expect(result.branches[0].action).toBe('kept-local'); + expect(mockHardResetBranchToRef).not.toHaveBeenCalled(); + }); + it('handles parent-mismatch status in non-interactive mode by skipping', async () => { mockReadState.mockResolvedValue( makeState([ diff --git a/packages/cli/src/commands/sync.ts b/packages/cli/src/commands/sync.ts index eb50aa0..40d4395 100644 --- a/packages/cli/src/commands/sync.ts +++ b/packages/cli/src/commands/sync.ts @@ -282,6 +282,7 @@ export async function sync( hasRemote && hasLocal && localSha !== remoteSha && + status !== 'local-ahead' && prSyncInfo.baseRefName && localParent && prSyncInfo.baseRefName !== localParent @@ -696,17 +697,32 @@ async function markBranchSynced( const entry = branchMap.get(branchName); if (!entry) return; const priorBaseline = entry.last_submitted_version; + const isAdoptingRemote = options.source === 'sync-adopt-remote'; const resolvedBaseBranch = options.baseBranch ?? priorBaseline?.base_branch ?? null; - let resolvedBaseSha = priorBaseline?.base_sha ?? null; + const canPreservePriorBaseSha = + isAdoptingRemote && + resolvedBaseBranch != null && + priorBaseline?.base_branch === resolvedBaseBranch && + priorBaseline.base_sha != null; + let resolvedBaseSha = canPreservePriorBaseSha + ? (priorBaseline?.base_sha ?? null) + : null; if (resolvedBaseBranch) { - try { - resolvedBaseSha = await getRefSha(resolvedBaseBranch, cwd); - } catch { - // Keep existing baseline SHA if base ref isn't currently resolvable. + if (!resolvedBaseSha) { + try { + resolvedBaseSha = await getRefSha(resolvedBaseBranch, cwd); + } catch { + // Keep existing baseline SHA if base ref isn't currently resolvable. + } } } - if (!resolvedBaseBranch || !resolvedBaseSha) return; + if (!resolvedBaseBranch || !resolvedBaseSha) { + if (isAdoptingRemote) { + entry.parent_revision = null; + } + return; + } const preservedSource = priorBaseline?.source ?? entry.sync_source ?? 'imported'; entry.last_submitted_version = { @@ -722,12 +738,30 @@ async function markBranchSynced( base_branch: resolvedBaseBranch, source: options.source, }; - try { - if (await isAncestor(resolvedBaseSha, headSha, cwd)) { + if (isAdoptingRemote) { + if (canPreservePriorBaseSha) { entry.parent_revision = resolvedBaseSha; + } else { + try { + entry.parent_revision = (await isAncestor( + resolvedBaseSha, + headSha, + cwd, + )) + ? resolvedBaseSha + : null; + } catch { + entry.parent_revision = null; + } + } + } else { + try { + if (await isAncestor(resolvedBaseSha, headSha, cwd)) { + entry.parent_revision = resolvedBaseSha; + } + } catch { + // If ancestry check fails, keep existing parent_revision. } - } catch { - // If ancestry check fails, keep existing parent_revision. } entry.last_synced_at = new Date().toISOString(); entry.sync_source = options.source === 'sync-noop' ? preservedSource : 'sync';