From ba0dc1f06f5f98ccf694b58b9f1f4e1d50964ba6 Mon Sep 17 00:00:00 2001 From: Daniel Wise Date: Sun, 8 Mar 2026 16:49:09 -0700 Subject: [PATCH 1/2] feat: automate stack repair and enable restack by default in sync --- .../skills/dubstack/references/workflows.md | 14 ++- QUICKSTART.md | 10 +- README.md | 24 +++- packages/cli/src/commands/post-merge.test.ts | 78 ++++++++++++- packages/cli/src/commands/post-merge.ts | 100 ++++------------ packages/cli/src/commands/restack.ts | 14 +++ .../cli/src/commands/stack-maintenance.ts | 103 +++++++++++++++++ packages/cli/src/commands/submit.test.ts | 6 + packages/cli/src/commands/submit.ts | 6 + packages/cli/src/commands/sync.test.ts | 107 +++++++++++++++++- packages/cli/src/commands/sync.ts | 95 ++++++++++++---- packages/cli/src/index.ts | 12 +- packages/cli/src/lib/git.test.ts | 29 +++++ packages/cli/src/lib/git.ts | 26 +++++ packages/cli/src/lib/state.test.ts | 40 +++++++ packages/cli/src/lib/state.ts | 10 ++ 16 files changed, 562 insertions(+), 112 deletions(-) create mode 100644 packages/cli/src/commands/stack-maintenance.ts diff --git a/.agents/skills/dubstack/references/workflows.md b/.agents/skills/dubstack/references/workflows.md index f7cb24d..759d015 100644 --- a/.agents/skills/dubstack/references/workflows.md +++ b/.agents/skills/dubstack/references/workflows.md @@ -50,7 +50,13 @@ dub sync --no-interactive If you want automatic restack after sync: ```bash -dub sync --restack +dub sync +``` + +If you need to skip rebases for one run: + +```bash +dub sync --no-restack ``` If you explicitly want destructive reconciliation: @@ -82,12 +88,14 @@ dub merge-next dub merge-next ``` -If manual merges happened: +If manual merges happened in GitHub or another UI: ```bash -dub post-merge +dub sync ``` +`dub post-merge` remains available when you want the explicit repair command. + ## 6) Conflict Recovery During Restack ```bash diff --git a/QUICKSTART.md b/QUICKSTART.md index db834d7..928acb9 100644 --- a/QUICKSTART.md +++ b/QUICKSTART.md @@ -187,7 +187,7 @@ Common sync variants: dub sync --all dub sync --no-interactive dub sync --force -dub sync --restack +dub sync --no-restack ``` ## 7) Preflight And Cleanup @@ -232,7 +232,13 @@ History note: - `main` is configured for linear, squash-style merges. - For stacks that target intermediate base branches, merge the top stack branch into `main` after lower layers merge so the full stack lands on `main`. -If merges happened manually: +If merges happened manually in GitHub or another UI, the happy path is: + +```bash +dub sync +``` + +`dub post-merge` is still available as an explicit repair command: ```bash dub post-merge diff --git a/README.md b/README.md index 5f591fb..c285ebe 100644 --- a/README.md +++ b/README.md @@ -435,7 +435,8 @@ dub pr 123 ### `dub sync` -Synchronize tracked branches with remote refs. +Synchronize tracked branches with remote refs and repair stack state after +manual merges. ```bash # sync current stack @@ -450,16 +451,31 @@ dub sync --no-interactive # force destructive sync decisions dub sync --force -# include post-sync restack -dub sync --restack +# keep sync conservative if you need to skip rebases +dub sync --no-restack ``` Current sync behavior includes: - fetch tracked refs from `origin` - attempt trunk fast-forward (or overwrite with `--force`) - auto-clean local branches for merged PRs (and closed PRs confirmed in trunk) +- retarget surviving child PRs after merged-parent cleanup +- refresh affected branch PRs after post-merge maintenance - reconcile local/remote divergence states per branch -- optional restack when `--restack` is set +- restack by default unless `--no-restack` is set + +Recommended post-merge flow: + +```bash +# merged in GitHub or another UI +dub sync +``` + +If sync hits a real conflict, prefer: + +```bash +dub continue --ai +``` ### `dub doctor` diff --git a/packages/cli/src/commands/post-merge.test.ts b/packages/cli/src/commands/post-merge.test.ts index 9f382f7..b12c215 100644 --- a/packages/cli/src/commands/post-merge.test.ts +++ b/packages/cli/src/commands/post-merge.test.ts @@ -195,12 +195,88 @@ describe('postMerge', () => { expect(mockRestack).toHaveBeenCalled(); expect(mockSubmit).toHaveBeenCalledWith('/repo', false, { - path: 'current', + path: 'stack', fix: true, }); expect(mockCheckoutBranch).toHaveBeenCalledWith('main', '/repo'); }); + it('refreshes from the surviving child branch when the original branch is trunk', async () => { + mockGetCurrentBranch.mockResolvedValue('main'); + mockReadState.mockResolvedValue({ + stacks: [ + { + id: 'stack-1', + branches: [ + { + name: 'main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/a', + parent: 'main', + pr_number: 1, + pr_link: 'https://x/1', + }, + { + name: 'feat/b', + parent: 'feat/a', + pr_number: 2, + pr_link: 'https://x/2', + }, + { + name: 'feat/c', + parent: 'feat/b', + pr_number: 3, + pr_link: 'https://x/3', + }, + ], + }, + ], + }); + mockGetBranchPrLifecycleState.mockImplementation(async (branch: string) => { + if (branch === 'feat/a') return 'MERGED'; + if (branch === 'feat/b' || branch === 'feat/c') return 'OPEN'; + return 'NONE'; + }); + mockGetBranchPrSyncInfo.mockImplementation(async (branch: string) => { + if (branch === 'feat/b') { + return { state: 'OPEN', baseRefName: 'feat/a' }; + } + if (branch === 'feat/c') { + return { state: 'OPEN', baseRefName: 'feat/b' }; + } + return { state: 'NONE', baseRefName: null }; + }); + mockSubmit.mockImplementation(async (_cwd, _dryRun, options) => { + const lastCheckout = mockCheckoutBranch.mock.calls.at(-1)?.[0]; + if (lastCheckout !== 'feat/b') { + throw new Error( + `expected surviving child checkout, got ${lastCheckout}`, + ); + } + if (options?.path !== 'stack') { + throw new Error(`expected full-stack refresh, got ${options?.path}`); + } + return { + pushed: ['feat/b', 'feat/c'], + created: [], + updated: ['feat/b', 'feat/c'], + path: 'stack', + dryRun: false, + fallbackApplied: false, + }; + }); + + const result = await postMerge('/repo'); + + expect(result.submittedBranches).toEqual(['feat/b', 'feat/c']); + expect(mockCheckoutBranch).toHaveBeenCalledWith('feat/b', '/repo'); + }); + 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 6f90d35..b780691 100644 --- a/packages/cli/src/commands/post-merge.ts +++ b/packages/cli/src/commands/post-merge.ts @@ -4,8 +4,6 @@ import { checkGhAuth, ensureGhInstalled, getBranchPrLifecycleState, - getBranchPrSyncInfo, - retargetPrBase, } from '../lib/github'; import { findStackForBranch, @@ -14,7 +12,12 @@ import { writeState, } from '../lib/state'; import { restack } from './restack'; -import { submit } from './submit'; +import { + hasNonRootBranches, + resolvePreferredBranch, + retargetOpenPrBranches, + submitRefreshedStacks, +} from './stack-maintenance'; export interface PostMergeResult { cleaned: string[]; @@ -67,6 +70,7 @@ export async function postMerge( dryRun, }; let preferredBranch: string | null = null; + const reparentedBranchNames = new Set(); for (const stack of workingStacks) { const mergedBottom = await getMergedBottomBranches(stack, cwd); @@ -74,21 +78,15 @@ export async function postMerge( result.cleaned.push(branchName); const reparented = removeBranchFromStack(stack, branchName); result.reparented.push(...reparented); - } - } - - for (const stack of workingStacks) { - for (const branch of stack.branches) { - if (branch.type === 'root' || !branch.parent) continue; - const prInfo = await getBranchPrSyncInfo(branch.name, cwd); - if (prInfo.state !== 'OPEN') continue; - if (prInfo.baseRefName === branch.parent) continue; - result.retargeted.push(branch.name); - if (!dryRun) { - await retargetPrBase(branch.name, branch.parent, cwd); + for (const entry of reparented) { + reparentedBranchNames.add(entry.branch); } } } + result.retargeted = await retargetOpenPrBranches(workingStacks, cwd, { + dryRun, + branches: [...reparentedBranchNames], + }); if (!dryRun) { await writeState(state, cwd); @@ -106,7 +104,7 @@ export async function postMerge( if (restackResult.status === 'conflict') { throw new DubError( `Post-merge restack hit conflicts on '${restackResult.conflictBranch ?? 'unknown'}'.\n` + - "Resolve conflicts, then run 'dub continue'. Run 'dub abort' to cancel.", + "Resolve conflicts, then run 'dub continue --ai' to let DubStack try the small conflict for you. Run 'dub continue' to resume manually or 'dub abort' to cancel.", ); } } @@ -125,14 +123,17 @@ export async function postMerge( } if (!dryRun && shouldSubmit) { - const submitResult = options.all - ? await submitAllStacks(cwd, workingStacks) - : await submit(cwd, false, { - path: 'current', - fix: true, - }); - result.submitted = true; - result.submittedBranches = submitResult.pushed; + const hasRefreshableBranches = workingStacks.some(hasNonRootBranches); + if (hasRefreshableBranches) { + result.submittedBranches = await submitRefreshedStacks( + cwd, + workingStacks, + { + all: options.all ?? false, + }, + ); + result.submitted = true; + } } if (!dryRun && preferredBranch) { await checkoutBranch(preferredBranch, cwd); @@ -143,29 +144,6 @@ export async function postMerge( return result; } -async function submitAllStacks( - cwd: string, - stacks: Stack[], -): Promise<{ pushed: string[] }> { - const submitTargets = stacks - .map( - (stack) => stack.branches.find((branch) => branch.type !== 'root')?.name, - ) - .filter((branchName): branchName is string => Boolean(branchName)); - const pushed = new Set(); - for (const branchName of submitTargets) { - await checkoutBranch(branchName, cwd); - const submitResult = await submit(cwd, false, { - path: 'current', - fix: true, - }); - for (const branch of submitResult.pushed) { - pushed.add(branch); - } - } - return { pushed: [...pushed].sort() }; -} - async function getMergedBottomBranches( stack: Stack, cwd: string, @@ -217,31 +195,3 @@ function removeBranchFromStack( ); return reparented; } - -function hasNonRootBranches(stack: Stack): boolean { - return stack.branches.some((branch) => branch.type !== 'root'); -} - -function resolvePreferredBranch( - workingStacks: Stack[], - originalBranch: string, - scopeStacks: Stack[], -): string | null { - const inWorking = findStackForBranch( - { stacks: workingStacks }, - originalBranch, - ); - if (inWorking) { - return originalBranch; - } - - const scopedIds = new Set(scopeStacks.map((stack) => stack.id)); - const preferredStack = - workingStacks.find((stack) => scopedIds.has(stack.id)) ?? workingStacks[0]; - if (!preferredStack) return null; - return ( - preferredStack.branches.find((branch) => branch.type !== 'root')?.name ?? - preferredStack.branches.find((branch) => branch.type === 'root')?.name ?? - null - ); -} diff --git a/packages/cli/src/commands/restack.ts b/packages/cli/src/commands/restack.ts index 6a51d91..dab7c0c 100644 --- a/packages/cli/src/commands/restack.ts +++ b/packages/cli/src/commands/restack.ts @@ -8,6 +8,7 @@ import { getCurrentBranch, getMergeBase, rebaseContinue as gitRebaseContinue, + hasUniquePatchCommits, isWorkingTreeClean, rebaseOnto, } from '../lib/git'; @@ -160,6 +161,19 @@ async function executeRestackSteps( const parentNewTip = await getBranchTip(step.parent, cwd); if (parentNewTip === step.parentOldTip) { step.status = 'skipped'; + updateParentRevision(state, step.branch, parentNewTip); + await writeProgress(progress, cwd); + continue; + } + + const hasUniquePatches = await hasUniquePatchCommits( + parentNewTip, + step.branch, + cwd, + ); + if (!hasUniquePatches) { + step.status = 'skipped'; + updateParentRevision(state, step.branch, parentNewTip); await writeProgress(progress, cwd); continue; } diff --git a/packages/cli/src/commands/stack-maintenance.ts b/packages/cli/src/commands/stack-maintenance.ts new file mode 100644 index 0000000..ea6dfe5 --- /dev/null +++ b/packages/cli/src/commands/stack-maintenance.ts @@ -0,0 +1,103 @@ +import { checkoutBranch } from '../lib/git'; +import { getBranchPrSyncInfo, retargetPrBase } from '../lib/github'; +import { findStackForBranch, type Stack, topologicalOrder } from '../lib/state'; +import { submit } from './submit'; + +export async function retargetOpenPrBranches( + stacks: Stack[], + cwd: string, + options: { dryRun?: boolean; branches?: string[] } = {}, +): Promise { + const dryRun = options.dryRun ?? false; + const targetBranches = options.branches ? new Set(options.branches) : null; + const retargeted: string[] = []; + + for (const stack of stacks) { + for (const branch of stack.branches) { + if (branch.type === 'root' || !branch.parent) continue; + if (targetBranches && !targetBranches.has(branch.name)) continue; + const prInfo = await getBranchPrSyncInfo(branch.name, cwd); + if (prInfo.state !== 'OPEN') continue; + if (prInfo.baseRefName === branch.parent) continue; + retargeted.push(branch.name); + if (!dryRun) { + await retargetPrBase(branch.name, branch.parent, cwd); + } + } + } + + return retargeted.sort(); +} + +export function hasNonRootBranches(stack: Stack): boolean { + return stack.branches.some((branch) => branch.type !== 'root'); +} + +export function resolvePreferredBranch( + workingStacks: Stack[], + originalBranch: string, + scopeStacks: Stack[], +): string | null { + const originalEntry = findStackForBranch( + { stacks: workingStacks }, + originalBranch, + )?.branches.find((branch) => branch.name === originalBranch); + if (originalEntry && originalEntry.type !== 'root') { + return originalBranch; + } + + const scopedIds = new Set(scopeStacks.map((stack) => stack.id)); + const preferredStack = + workingStacks.find((stack) => scopedIds.has(stack.id)) ?? workingStacks[0]; + if (!preferredStack) return null; + + if (originalEntry?.type === 'root') { + return ( + topologicalOrder(preferredStack).find((branch) => branch.type !== 'root') + ?.name ?? + preferredStack.branches.find((branch) => branch.type === 'root')?.name ?? + null + ); + } + + return ( + topologicalOrder(preferredStack).find((branch) => branch.type !== 'root') + ?.name ?? + preferredStack.branches.find((branch) => branch.type === 'root')?.name ?? + null + ); +} + +export async function submitRefreshedStacks( + cwd: string, + stacks: Stack[], + options: { all: boolean }, +): Promise { + if (options.all) { + const pushed = new Set(); + const submitTargets = stacks + .map( + (stack) => + topologicalOrder(stack).find((branch) => branch.type !== 'root') + ?.name, + ) + .filter((branchName): branchName is string => Boolean(branchName)); + for (const branchName of submitTargets) { + await checkoutBranch(branchName, cwd); + const result = await submit(cwd, false, { + path: 'stack', + fix: true, + }); + for (const branch of result.pushed) { + pushed.add(branch); + } + } + return [...pushed].sort(); + } + + const result = await submit(cwd, false, { + path: 'stack', + fix: true, + }); + return result.pushed; +} diff --git a/packages/cli/src/commands/submit.test.ts b/packages/cli/src/commands/submit.test.ts index 49ee961..aa96203 100644 --- a/packages/cli/src/commands/submit.test.ts +++ b/packages/cli/src/commands/submit.test.ts @@ -603,6 +603,12 @@ describe('submit', () => { base_branch: 'main', source: 'submit', }); + expect(featBranch?.last_reconciled_version).toEqual({ + head_sha: 'feat/a-sha', + base_sha: 'main-sha', + base_branch: 'main', + source: 'submit', + }); expect(featBranch?.sync_source).toBe('submit'); expect(featBranch?.last_synced_at).toBeTruthy(); }); diff --git a/packages/cli/src/commands/submit.ts b/packages/cli/src/commands/submit.ts index 06aafd7..1434e2a 100644 --- a/packages/cli/src/commands/submit.ts +++ b/packages/cli/src/commands/submit.ts @@ -199,6 +199,12 @@ export async function submit( version_number: null, source: 'submit', }; + stateBranch.last_reconciled_version = { + head_sha: headSha, + base_sha: baseSha, + base_branch: branch.parent as string, + source: 'submit', + }; if (stateBranch.parent_revision == null) { stateBranch.parent_revision = baseSha; } diff --git a/packages/cli/src/commands/sync.test.ts b/packages/cli/src/commands/sync.test.ts index a2efbcd..8c3c785 100644 --- a/packages/cli/src/commands/sync.test.ts +++ b/packages/cli/src/commands/sync.test.ts @@ -36,6 +36,7 @@ vi.mock('../lib/github.js', () => ({ ensureGhInstalled: vi.fn(), getBranchPrLifecycleState: vi.fn(), getBranchPrSyncInfo: vi.fn(), + retargetPrBase: vi.fn(), })); import { @@ -56,13 +57,19 @@ import { ensureGhInstalled, getBranchPrLifecycleState, getBranchPrSyncInfo, + retargetPrBase, } from '../lib/github'; import { detectActiveOperation } from '../lib/operation-state'; import type { DubState } from '../lib/state'; import { readState, writeState } from '../lib/state'; import { restack } from './restack'; +import { submit } from './submit'; import { sync } from './sync'; +vi.mock('./submit.js', () => ({ + submit: vi.fn(), +})); + const mockBranchExists = branchExists as ReturnType; const mockCheckoutBranch = checkoutBranch as ReturnType; const mockCheckoutRemoteBranch = checkoutRemoteBranch as ReturnType< @@ -90,8 +97,10 @@ const mockGetBranchPrLifecycleState = getBranchPrLifecycleState as ReturnType< typeof vi.fn >; const mockGetBranchPrSyncInfo = getBranchPrSyncInfo as ReturnType; +const mockRetargetPrBase = retargetPrBase as ReturnType; const mockEnsureGhInstalled = ensureGhInstalled as ReturnType; const mockCheckGhAuth = checkGhAuth as ReturnType; +const mockSubmit = submit as ReturnType; function makeState( branches: { name: string; parent: string | null; type?: 'root' }[], @@ -141,10 +150,19 @@ beforeEach(() => { state: 'OPEN', baseRefName: 'main', }); + mockRetargetPrBase.mockResolvedValue(undefined); mockDetectActiveOperation.mockResolvedValue('none'); mockEnsureGhInstalled.mockResolvedValue(undefined); mockCheckGhAuth.mockResolvedValue(undefined); mockWriteState.mockResolvedValue(undefined); + mockSubmit.mockResolvedValue({ + pushed: ['feat/a'], + created: [], + updated: ['feat/a'], + path: 'current', + dryRun: false, + fallbackApplied: false, + }); }); describe('sync', () => { @@ -174,7 +192,7 @@ describe('sync', () => { expect(mockRestack).not.toHaveBeenCalled(); }); - it('does not restack unless --restack is explicitly requested', async () => { + it('restacks by default', async () => { mockReadState.mockResolvedValue( makeState([ { name: 'main', parent: null, type: 'root' }, @@ -184,8 +202,8 @@ describe('sync', () => { mockGetRefSha.mockResolvedValue('same-sha'); const result = await sync('/repo', { interactive: false }); - expect(result.restacked).toBe(false); - expect(mockRestack).not.toHaveBeenCalled(); + expect(result.restacked).toBe(true); + expect(mockRestack).toHaveBeenCalled(); }); it('restores missing local branch from remote', async () => { @@ -376,6 +394,58 @@ describe('sync', () => { } }); + it('refreshes the surviving child branch after cleaning a merged parent while on trunk', async () => { + mockGetCurrentBranch.mockResolvedValue('main'); + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { name: 'feat/a', parent: 'main' }, + { name: 'feat/b', parent: 'feat/a' }, + { name: 'feat/c', parent: 'feat/b' }, + ]), + ); + mockGetBranchPrLifecycleState.mockImplementation(async (branch: string) => + branch === 'feat/a' ? 'MERGED' : 'OPEN', + ); + mockGetBranchPrSyncInfo.mockImplementation(async (branch: string) => { + if (branch === 'feat/b') { + return { state: 'OPEN', baseRefName: 'feat/a' }; + } + if (branch === 'feat/c') { + return { state: 'OPEN', baseRefName: 'feat/b' }; + } + return { state: 'NONE', baseRefName: null }; + }); + mockSubmit.mockImplementation(async (_cwd, _dryRun, options) => { + const lastCheckout = mockCheckoutBranch.mock.calls.at(-1)?.[0]; + if (lastCheckout !== 'feat/b') { + throw new Error( + `expected surviving child checkout, got ${lastCheckout}`, + ); + } + if (options?.path !== 'stack') { + throw new Error(`expected full-stack refresh, got ${options?.path}`); + } + return { + pushed: ['feat/b', 'feat/c'], + created: [], + updated: ['feat/b', 'feat/c'], + path: 'stack', + dryRun: false, + fallbackApplied: false, + }; + }); + + await sync('/repo', { interactive: false, restack: false }); + + expect(mockRetargetPrBase).toHaveBeenCalledWith('feat/b', 'main', '/repo'); + expect(mockSubmit).toHaveBeenCalledWith('/repo', false, { + path: 'stack', + fix: true, + }); + expect(mockCheckoutBranch).toHaveBeenCalledWith('feat/b', '/repo'); + }); + it('preserves parent_revision when auto-cleaning reparents children', async () => { mockReadState.mockResolvedValue({ stacks: [ @@ -462,6 +532,12 @@ describe('sync', () => { (b) => b.name === 'feat/a', ); expect(featA?.parent_revision).toBe('same-sha'); + expect(featA?.last_reconciled_version).toEqual({ + head_sha: 'same-sha', + base_sha: 'same-sha', + base_branch: 'main', + source: 'sync-noop', + }); }); it('handles parent-mismatch status in non-interactive mode by skipping', async () => { @@ -490,6 +566,31 @@ describe('sync', () => { expect(result.branches[0].action).toBe('skipped'); }); + it('does not retarget PR bases when parent authority is unresolved', async () => { + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { name: 'feat/a', parent: 'local-parent' }, + ]), + ); + mockGetRefSha + .mockResolvedValueOnce('local-sha') + .mockResolvedValueOnce('remote-sha'); + mockIsAncestor.mockResolvedValue(false); + mockGetBranchPrSyncInfo.mockResolvedValue({ + state: 'OPEN', + baseRefName: 'remote-parent', + }); + + await sync('/repo', { + interactive: false, + force: false, + restack: false, + }); + + expect(mockRetargetPrBase).not.toHaveBeenCalled(); + }); + it('throws actionable recovery guidance when restack phase conflicts', async () => { mockReadState.mockResolvedValue( makeState([ diff --git a/packages/cli/src/commands/sync.ts b/packages/cli/src/commands/sync.ts index a12d241..d30e914 100644 --- a/packages/cli/src/commands/sync.ts +++ b/packages/cli/src/commands/sync.ts @@ -38,6 +38,12 @@ import type { SyncResult, } from '../lib/sync/types'; import { restack } from './restack'; +import { + hasNonRootBranches, + resolvePreferredBranch, + retargetOpenPrBranches, + submitRefreshedStacks, +} from './stack-maintenance'; function isInteractiveShell(): boolean { return Boolean(process.stdout.isTTY && process.stdin.isTTY); @@ -88,7 +94,7 @@ export async function sync( await checkGhAuth(); const options: SyncOptions = { - restack: rawOptions.restack ?? false, + restack: rawOptions.restack ?? true, force: rawOptions.force ?? false, all: rawOptions.all ?? false, interactive: rawOptions.interactive ?? isInteractiveShell(), @@ -138,6 +144,10 @@ export async function sync( }; const rootHasRemote = new Map(); let pendingError: Error | null = null; + let restoreTarget = originalBranch; + let needsSubmitRefresh = false; + let restackChanged = false; + const reparentedBranchNames = new Set(); try { console.log('🌲 Fetching branches from remote...'); @@ -218,7 +228,9 @@ export async function sync( } await checkoutBranch(roots[0] ?? originalBranch, cwd); await deleteBranch(branch, cwd); - removeBranchFromState(scopeStacks, branch); + for (const entry of removeBranchFromState(scopeStacks, branch)) { + reparentedBranchNames.add(entry.branch); + } result.cleaned.push(branch); } for (const skipped of cleanupPlan.skipped) { @@ -301,7 +313,7 @@ export async function sync( printBranchOutcome(outcome); const restoredSha = await getRefSha(branch, cwd); await markBranchSynced(stateBranchMap, branch, restoredSha, cwd, { - source: 'sync', + source: 'sync-adopt-remote', baseBranch: stateBranchMap.get(branch)?.parent ?? null, }); continue; @@ -322,7 +334,7 @@ export async function sync( localSha ?? remoteSha ?? null, cwd, { - source: 'sync', + source: 'sync-noop', baseBranch: stateBranchMap.get(branch)?.parent ?? null, }, ); @@ -344,7 +356,7 @@ export async function sync( localSha ?? remoteSha ?? null, cwd, { - source: 'imported', + source: 'sync-noop', baseBranch: stateBranchMap.get(branch)?.parent ?? null, }, ); @@ -362,7 +374,7 @@ export async function sync( result.branches.push(outcome); printBranchOutcome(outcome); await markBranchSynced(stateBranchMap, branch, remoteSha, cwd, { - source: 'sync', + source: 'sync-adopt-remote', baseBranch: stateBranchMap.get(branch)?.parent ?? null, }); continue; @@ -390,7 +402,7 @@ export async function sync( message: `✔ Synced unsubmitted branch '${branch}' to remote with --force.`, }; await markBranchSynced(stateBranchMap, branch, remoteSha, cwd, { - source: 'sync', + source: 'sync-adopt-remote', baseBranch: localParent, }); } else if (!options.interactive) { @@ -413,7 +425,7 @@ export async function sync( message: `✔ Synced unsubmitted branch '${branch}' to remote.`, }; await markBranchSynced(stateBranchMap, branch, remoteSha, cwd, { - source: 'sync', + source: 'sync-adopt-remote', baseBranch: localParent, }); } else { @@ -447,7 +459,7 @@ export async function sync( message: `✔ Synced '${branch}' to remote and adopted remote parent '${prSyncInfo.baseRefName ?? 'unknown'}'.`, }; await markBranchSynced(stateBranchMap, branch, remoteSha, cwd, { - source: 'sync', + source: 'sync-adopt-remote', baseBranch: prSyncInfo.baseRefName ?? localParent, }); } else if (!options.interactive) { @@ -482,7 +494,7 @@ export async function sync( message: `✔ Synced '${branch}' to remote and adopted remote parent.`, }; await markBranchSynced(stateBranchMap, branch, remoteSha, cwd, { - source: 'sync', + source: 'sync-adopt-remote', baseBranch: prSyncInfo.baseRefName ?? localParent, }); } else if (parentDecision === 'local') { @@ -537,7 +549,7 @@ export async function sync( message: `✔ Synced '${branch}' to remote version.`, }; await markBranchSynced(stateBranchMap, branch, remoteSha, cwd, { - source: 'sync', + source: 'sync-adopt-remote', baseBranch: stateBranchMap.get(branch)?.parent ?? null, }); } else if (decision === 'keep-local') { @@ -560,7 +572,7 @@ export async function sync( if (reconciled) { const newSha = await getRefSha(branch, cwd); await markBranchSynced(stateBranchMap, branch, newSha, cwd, { - source: 'sync', + source: 'sync-restack', baseBranch: stateBranchMap.get(branch)?.parent ?? null, }); } @@ -580,6 +592,13 @@ export async function sync( await writeState(state, cwd); + const retargeted = await retargetOpenPrBranches(scopeStacks, cwd, { + branches: [...reparentedBranchNames], + }); + if (retargeted.length > 0) { + needsSubmitRefresh = true; + } + if (options.restack) { console.log('🥞 Restacking branches...'); const rootsToRestack = options.all ? roots : [roots[0]].filter(Boolean); @@ -591,13 +610,34 @@ export async function sync( `Sync paused: conflict while restacking '${restackResult.conflictBranch ?? 'unknown'}'.\n` + 'Recovery:\n' + ' 1. Resolve conflicts and stage files.\n' + - " 2. Run 'dub continue' to resume.\n" + - " 3. Run 'dub abort' to cancel recovery and roll back progress.", + " 2. Run 'dub continue --ai' to let DubStack try the small conflict for you.\n" + + " 3. Run 'dub continue' to resume manually.\n" + + " 4. Run 'dub abort' to cancel recovery and roll back progress.", ); } + if (restackResult.status === 'success') { + restackChanged = true; + } } result.restacked = true; } + + if (result.cleaned.length > 0 || needsSubmitRefresh || restackChanged) { + const preferredBranch = resolvePreferredBranch( + scopeStacks, + originalBranch, + scopeStacks, + ); + restoreTarget = preferredBranch ?? originalBranch; + if (needsSubmitRefresh && scopeStacks.some(hasNonRootBranches)) { + if (preferredBranch) { + await checkoutBranch(preferredBranch, cwd); + } + await submitRefreshedStacks(cwd, scopeStacks, { + all: options.all, + }); + } + } } catch (error) { pendingError = await wrapSyncError(error, cwd); } @@ -605,12 +645,12 @@ export async function sync( const activeOperation = await detectActiveOperation(cwd).catch(() => 'none'); if (activeOperation === 'none') { try { - await checkoutBranch(originalBranch, cwd); + await checkoutBranch(restoreTarget, cwd); } catch { if (!pendingError) { pendingError = new DubError( - `Sync completed but could not restore original branch '${originalBranch}'.\n` + - `Run 'git checkout ${originalBranch}' to return to your original context.`, + `Sync completed but could not restore branch '${restoreTarget}'.\n` + + `Run 'git checkout ${restoreTarget}' to return to your working context.`, ); } } @@ -636,8 +676,9 @@ async function wrapSyncError(error: unknown, cwd: string): Promise { return new DubError( `${baseError.message}\n` + 'Recovery:\n' + - " 1. Run 'dub continue' after resolving conflicts.\n" + - " 2. Run 'dub abort' to exit the in-progress operation safely.", + " 1. Run 'dub continue --ai' to let DubStack try the small conflict for you.\n" + + " 2. Run 'dub continue' after resolving conflicts manually.\n" + + " 3. Run 'dub abort' to exit the in-progress operation safely.", ); } @@ -646,7 +687,10 @@ async function markBranchSynced( branchName: string, headSha: string | null, cwd: string, - options: { source: 'sync' | 'imported'; baseBranch: string | null }, + options: { + source: 'sync-adopt-remote' | 'sync-noop' | 'sync-restack'; + baseBranch: string | null; + }, ): Promise { if (!headSha) return; const entry = branchMap.get(branchName); @@ -668,6 +712,12 @@ async function markBranchSynced( base_sha: resolvedBaseSha, base_branch: resolvedBaseBranch, version_number: priorBaseline?.version_number ?? null, + source: options.source === 'sync-noop' ? 'imported' : 'sync', + }; + entry.last_reconciled_version = { + head_sha: headSha, + base_sha: resolvedBaseSha, + base_branch: resolvedBaseBranch, source: options.source, }; try { @@ -678,7 +728,7 @@ async function markBranchSynced( // If ancestry check fails, keep existing parent_revision. } entry.last_synced_at = new Date().toISOString(); - entry.sync_source = options.source; + entry.sync_source = options.source === 'sync-noop' ? 'imported' : 'sync'; } function getDescendants(stacks: Array<{ branches: Branch[] }>, branch: string) { @@ -706,6 +756,7 @@ function removeBranchFromState( stacks: Array<{ branches: Branch[] }>, branch: string, ) { + const reparented: Array<{ branch: string; parent: string | null }> = []; for (const stack of stacks) { const deleted = stack.branches.find((b) => b.name === branch); if (!deleted) continue; @@ -713,8 +764,10 @@ function removeBranchFromState( for (const child of stack.branches) { if (child.parent === branch) { child.parent = newParent; + reparented.push({ branch: child.name, parent: child.parent }); } } stack.branches = stack.branches.filter((b) => b.name !== branch); } + return reparented; } diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 0dda8bc..f999a91 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -496,7 +496,11 @@ program program .command('sync') .description('Sync tracked branches with remote and reconcile divergence') - .option('--restack', 'Restack branches after sync') + .option( + '--restack', + 'Restack branches after sync (disable with --no-restack)', + true, + ) .option( '-f, --force', 'Skip prompts for branch reset/reconcile sync decisions', @@ -538,7 +542,7 @@ Examples: ); console.log( chalk.dim( - ' Resolve conflicts, stage changes, then run: dub restack --continue', + ' Resolve conflicts, stage changes, then run: dub continue --ai (or dub restack --continue)', ), ); } else { @@ -571,7 +575,9 @@ program ), ); console.log( - chalk.dim(' Resolve conflicts, stage changes, then run: dub continue'), + chalk.dim( + ' Resolve conflicts, stage changes, then run: dub continue --ai (or dub continue)', + ), ); return; } diff --git a/packages/cli/src/lib/git.test.ts b/packages/cli/src/lib/git.test.ts index cfa17b9..f718460 100644 --- a/packages/cli/src/lib/git.test.ts +++ b/packages/cli/src/lib/git.test.ts @@ -15,6 +15,7 @@ import { getDiffBetween, getMergeBase, hasStagedChanges, + hasUniquePatchCommits, isGitRepo, isValidBranchName, isWorkingTreeClean, @@ -194,6 +195,34 @@ describe('getDiffBetween', () => { }); }); +describe('hasUniquePatchCommits', () => { + it('returns true when a branch has unique work not present upstream', async () => { + await gitInRepo(dir, ['checkout', '-b', 'feat']); + fs.writeFileSync(path.join(dir, 'feat.txt'), 'feature'); + await gitInRepo(dir, ['add', '.']); + await gitInRepo(dir, ['commit', '-m', 'feat-commit']); + + await expect(hasUniquePatchCommits('main', 'feat', dir)).resolves.toBe( + true, + ); + }); + + it('returns false when the branch changes were squash-merged upstream', async () => { + await gitInRepo(dir, ['checkout', '-b', 'feat']); + fs.writeFileSync(path.join(dir, 'feat.txt'), 'feature'); + await gitInRepo(dir, ['add', '.']); + await gitInRepo(dir, ['commit', '-m', 'feat-commit']); + + await gitInRepo(dir, ['checkout', 'main']); + await gitInRepo(dir, ['merge', '--squash', 'feat']); + await gitInRepo(dir, ['commit', '-m', 'squash feat']); + + await expect(hasUniquePatchCommits('main', 'feat', dir)).resolves.toBe( + false, + ); + }); +}); + describe('getBranchTip', () => { it('returns the commit SHA of a branch', async () => { const expected = ( diff --git a/packages/cli/src/lib/git.ts b/packages/cli/src/lib/git.ts index 340ba85..8c521fc 100644 --- a/packages/cli/src/lib/git.ts +++ b/packages/cli/src/lib/git.ts @@ -563,6 +563,32 @@ export async function getDiffBetween( } } +/** + * Returns whether a branch has unique patch content not already present upstream. + * + * Uses `git cherry`, which marks patch-equivalent commits with `-` and unique + * commits with `+`. + */ +export async function hasUniquePatchCommits( + baseRef: string, + headRef: string, + cwd: string, +): Promise { + try { + const { stdout } = await execa('git', ['cherry', baseRef, headRef], { + cwd, + }); + return stdout + .split('\n') + .map((line) => line.trim()) + .some((line) => line.startsWith('+')); + } catch { + throw new DubError( + `Failed to compare patch-equivalent commits for '${headRef}' against '${baseRef}'.`, + ); + } +} + function parseDiffCount(raw: string): number { if (raw === '-') return 0; const value = Number(raw); diff --git a/packages/cli/src/lib/state.test.ts b/packages/cli/src/lib/state.test.ts index b1d39d6..74e9599 100644 --- a/packages/cli/src/lib/state.test.ts +++ b/packages/cli/src/lib/state.test.ts @@ -143,6 +143,46 @@ describe('writeState and readState roundtrip', () => { expect(loaded.stacks[0].branches[1].parent_revision).toBe('deadbeef'); }); + it('normalizes and roundtrips reconciliation metadata', async () => { + await initState(dir); + const state: DubState = { + stacks: [ + { + id: 'test-id', + branches: [ + { + name: 'main', + type: 'root', + parent: null, + pr_number: null, + pr_link: null, + }, + { + name: 'feat/a', + parent: 'main', + pr_number: null, + pr_link: null, + last_reconciled_version: { + head_sha: 'feat-head', + base_sha: 'main-head', + base_branch: 'main', + source: 'sync-adopt-remote', + }, + }, + ], + }, + ], + }; + await writeState(state, dir); + const loaded = await readState(dir); + expect(loaded.stacks[0].branches[1].last_reconciled_version).toEqual({ + head_sha: 'feat-head', + base_sha: 'main-head', + base_branch: 'main', + source: 'sync-adopt-remote', + }); + }); + it('creates parent directory if missing', async () => { const state: DubState = { stacks: [] }; await writeState(state, dir); diff --git a/packages/cli/src/lib/state.ts b/packages/cli/src/lib/state.ts index c9b3e14..4aa87c7 100644 --- a/packages/cli/src/lib/state.ts +++ b/packages/cli/src/lib/state.ts @@ -26,6 +26,13 @@ export interface Branch { version_number: number | null; source: 'submit' | 'sync' | 'imported'; } | null; + /** Last known effective branch/base relationship after sync maintenance. */ + last_reconciled_version?: { + head_sha: string; + base_sha: string; + base_branch: string; + source: 'submit' | 'sync-adopt-remote' | 'sync-noop' | 'sync-restack'; + } | null; /** ISO timestamp of the most recent successful sync for this branch. */ last_synced_at?: string | null; /** Source of the branch's current sync baseline metadata. */ @@ -196,6 +203,7 @@ export function addBranchToStack( pr_number: null, pr_link: null, last_submitted_version: null, + last_reconciled_version: null, last_synced_at: null, sync_source: null, }; @@ -211,6 +219,7 @@ export function addBranchToStack( pr_number: null, pr_link: null, last_submitted_version: null, + last_reconciled_version: null, last_synced_at: null, sync_source: null, }; @@ -234,6 +243,7 @@ function normalizeBranch(branch: Branch): Branch { return { ...branch, last_submitted_version: branch.last_submitted_version ?? null, + last_reconciled_version: branch.last_reconciled_version ?? null, last_synced_at: branch.last_synced_at ?? null, sync_source: branch.sync_source ?? null, }; From fa3f155f41225af3bef9c9de58e8a1edee6a5506 Mon Sep 17 00:00:00 2001 From: Daniel Wise Date: Sun, 8 Mar 2026 17:15:27 -0700 Subject: [PATCH 2/2] fix(sync): preserve maintenance provenance --- packages/cli/src/commands/restack.test.ts | 31 +++++++++++++++++++++++ packages/cli/src/commands/restack.ts | 1 - packages/cli/src/commands/sync.test.ts | 26 +++++++++++++++++++ packages/cli/src/commands/sync.ts | 6 +++-- 4 files changed, 61 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/restack.test.ts b/packages/cli/src/commands/restack.test.ts index 446aa2a..f76d753 100644 --- a/packages/cli/src/commands/restack.test.ts +++ b/packages/cli/src/commands/restack.test.ts @@ -302,6 +302,37 @@ describe('restack', () => { }); describe('squash-merge-then-restack', () => { + it('preserves parent_revision when patch-equivalent work is skipped', async () => { + await create('feat/a', dir); + fs.writeFileSync(path.join(dir, 'file-a.txt'), 'feature a content'); + await gitInRepo(dir, ['add', '.']); + await gitInRepo(dir, ['commit', '-m', 'add file-a']); + + const stateBeforeMerge = await readState(dir); + const featARevisionBefore = stateBeforeMerge.stacks[0].branches.find( + (b) => b.name === 'feat/a', + )?.parent_revision; + expect(featARevisionBefore).toBeTruthy(); + + await gitInRepo(dir, ['checkout', 'main']); + await gitInRepo(dir, ['merge', '--squash', 'feat/a']); + await gitInRepo(dir, ['commit', '-m', 'squash A']); + const mainTipAfterSquash = await getBranchTip('main', dir); + + await gitInRepo(dir, ['checkout', 'feat/a']); + const result = await restack(dir); + + expect(result.status).toBe('up-to-date'); + expect(result.rebased).toHaveLength(0); + + const stateAfterRestack = await readState(dir); + const featAAfter = stateAfterRestack.stacks[0].branches.find( + (b) => b.name === 'feat/a', + ); + expect(featAAfter?.parent_revision).toBe(featARevisionBefore); + expect(featAAfter?.parent_revision).not.toBe(mainTipAfterSquash); + }); + it('produces no false conflicts after squash-merge', async () => { // Create main → feat/a with a commit on file-a.txt await create('feat/a', dir); diff --git a/packages/cli/src/commands/restack.ts b/packages/cli/src/commands/restack.ts index dab7c0c..b720283 100644 --- a/packages/cli/src/commands/restack.ts +++ b/packages/cli/src/commands/restack.ts @@ -173,7 +173,6 @@ async function executeRestackSteps( ); if (!hasUniquePatches) { step.status = 'skipped'; - updateParentRevision(state, step.branch, parentNewTip); await writeProgress(progress, cwd); continue; } diff --git a/packages/cli/src/commands/sync.test.ts b/packages/cli/src/commands/sync.test.ts index 8c3c785..53e9937 100644 --- a/packages/cli/src/commands/sync.test.ts +++ b/packages/cli/src/commands/sync.test.ts @@ -333,6 +333,32 @@ describe('sync', () => { expect(result.branches[0].status).toBe( 'updated-outside-dubstack-but-up-to-date', ); + const writtenState = mockWriteState.mock.calls.at(-1)?.[0] as DubState; + const featA = writtenState.stacks[0].branches.find( + (b) => b.name === 'feat/a', + ); + expect(featA?.last_submitted_version?.source).toBe('imported'); + expect(featA?.sync_source).toBe('imported'); + }); + + it('preserves existing provenance on no-op sync runs', async () => { + mockReadState.mockResolvedValue( + makeState([ + { name: 'main', parent: null, type: 'root' }, + { name: 'feat/a', parent: 'main' }, + ]), + ); + mockGetRefSha.mockResolvedValue('same-sha'); + + await sync('/repo', { interactive: false, restack: false }); + + const writtenState = mockWriteState.mock.calls.at(-1)?.[0] as DubState; + const featA = writtenState.stacks[0].branches.find( + (b) => b.name === 'feat/a', + ); + expect(featA?.last_submitted_version?.source).toBe('submit'); + expect(featA?.sync_source).toBe('submit'); + expect(featA?.last_reconciled_version?.source).toBe('sync-noop'); }); it('cleans merged branch automatically without force', async () => { diff --git a/packages/cli/src/commands/sync.ts b/packages/cli/src/commands/sync.ts index d30e914..eb50aa0 100644 --- a/packages/cli/src/commands/sync.ts +++ b/packages/cli/src/commands/sync.ts @@ -707,12 +707,14 @@ async function markBranchSynced( } } if (!resolvedBaseBranch || !resolvedBaseSha) return; + const preservedSource = + priorBaseline?.source ?? entry.sync_source ?? 'imported'; entry.last_submitted_version = { head_sha: headSha, base_sha: resolvedBaseSha, base_branch: resolvedBaseBranch, version_number: priorBaseline?.version_number ?? null, - source: options.source === 'sync-noop' ? 'imported' : 'sync', + source: options.source === 'sync-noop' ? preservedSource : 'sync', }; entry.last_reconciled_version = { head_sha: headSha, @@ -728,7 +730,7 @@ async function markBranchSynced( // If ancestry check fails, keep existing parent_revision. } entry.last_synced_at = new Date().toISOString(); - entry.sync_source = options.source === 'sync-noop' ? 'imported' : 'sync'; + entry.sync_source = options.source === 'sync-noop' ? preservedSource : 'sync'; } function getDescendants(stacks: Array<{ branches: Branch[] }>, branch: string) {