From 2fb7c7523c2c4350c0e496d8ee9fb2eceb86aca0 Mon Sep 17 00:00:00 2001 From: Evgeniy Podivilov Date: Wed, 16 Sep 2026 12:57:37 +0100 Subject: [PATCH 1/2] fix(update): reconcile feature tracking refs --- .changeset/calm-branches-reconcile.md | 5 + README.md | 12 +- .../use-cases/update-worktrees.test.ts | 91 ++++++++ src/application/use-cases/update-worktrees.ts | 198 +++++++++++++++++- src/cli/commands/update.test.ts | 45 +++- src/cli/commands/update.ts | 58 ++++- src/domain/ports/git-port.ts | 8 + src/domain/schemas/command-args-schema.ts | 1 + .../adapters/bun-git-adapter.test.ts | 22 ++ .../adapters/bun-git-adapter.ts | 53 +++++ src/test-utils/fake-git.ts | 23 ++ 11 files changed, 504 insertions(+), 12 deletions(-) create mode 100644 .changeset/calm-branches-reconcile.md diff --git a/.changeset/calm-branches-reconcile.md b/.changeset/calm-branches-reconcile.md new file mode 100644 index 0000000..d9e4ff0 --- /dev/null +++ b/.changeset/calm-branches-reconcile.md @@ -0,0 +1,5 @@ +--- +"worktree-kit": minor +--- + +Reconcile active feature worktrees with their configured tracking refs before updating their parent stacks. diff --git a/README.md b/README.md index da699bd..dadd464 100644 --- a/README.md +++ b/README.md @@ -236,6 +236,7 @@ wt update [branch] [options] |------|-------------| | `--dry-run` | Show what would be done without making changes | | `--cleanup` | Automatically clean up branches with gone remotes after update | +| `--reconcile rebase\|abort\|reset` | Resolve genuine feature/upstream divergence (`reset` requires a named branch) | **Examples:** @@ -256,9 +257,14 @@ wt update --cleanup **How it works:** 1. Fetches all remotes -2. Fast-forwards the default branch (or updates its ref if no worktree exists for it) -3. Detects parent branches via merge-base -4. Rebases feature branches in correct order — parents before children +2. Reconciles each targeted feature worktree with that branch's configured Git upstream +3. Fast-forwards the default branch (or updates its ref if no worktree exists for it) +4. Re-detects parent branches from the reconciled tips +5. Rebases feature branches in correct order — parents before children + +Feature reconciliation keeps equal and local-only tips unchanged, fast-forwards remote-only advances, and automatically accepts a patch-equivalent remote rewrite after saving the old tip under `refs/worktree-kit/recovery/`. Genuine local/remote divergence prompts in interactive mode, defaulting to rebase. Non-interactive runs must choose `--reconcile rebase` or leave the branch unresolved with `--reconcile abort`; destructive `--reconcile reset` is accepted only for an explicitly named branch and also creates a recovery ref. A dirty worktree that would have to move is left untouched together with its dependent subtree. + +Use `--dry-run` to inspect classifications, policies, recovery actions, root synchronization, and the final rebase plan without changing refs, running hooks, or opening a reconciliation prompt. **Fork workflow** — when `upstream` is set in config (see `wt init --upstream`), the default branch is synced from `/` instead of `origin/`, whether it is fast-forwarded in its own worktree or updated by ref because no worktree has it checked out. After a successful upstream sync, `post-update` hooks also run for the default branch (so you can, for example, push the synced default branch back to your fork); with no worktree for the default branch they run in the repository root. diff --git a/src/application/use-cases/update-worktrees.test.ts b/src/application/use-cases/update-worktrees.test.ts index 4f3a4ca..d19899b 100644 --- a/src/application/use-cases/update-worktrees.test.ts +++ b/src/application/use-cases/update-worktrees.test.ts @@ -271,6 +271,97 @@ describe("updateWorktrees", () => { }); }); +describe("updateWorktrees — feature tracking reconciliation (WTK-70)", () => { + function reconciliationGit(options: Parameters[0] = {}) { + const worktrees = [mainWt, featureA]; + return createFakeGit({ + worktrees, + ...flatBranchesConfig(worktrees), + branchUpstreams: new Map([["feature-a", "fork/topic"]]), + ...options, + }); + } + + test("R2: remote-only advance fast-forwards the feature before its parent rebase", async () => { + const fastForwardToRefCalls: { worktreePath: string; ref: string }[] = []; + const git = reconciliationGit({ + commitCountMap: new Map([ + ...flatBranchesConfig([mainWt, featureA]).commitCountMap, + ["feature-a..fork/topic", 2], + ["fork/topic..feature-a", 0], + ]), + fastForwardToRefCalls, + }); + const output = expectOk(await updateWorktrees({ dryRun: false }, { git })); + + expect(fastForwardToRefCalls).toEqual([{ worktreePath: "/repo-a", ref: "fork/topic" }]); + expect(output.reconciliations[0]).toMatchObject({ state: "remote-only", action: "fast-forwarded" }); + expect(output.unresolved).toBe(false); + }); + + test("R3: patch-equivalent rewrite saves a recovery ref and realigns", async () => { + const createRecoveryRefCalls: string[] = []; + const resetHardToRefCalls: { worktreePath: string; ref: string }[] = []; + const git = reconciliationGit({ + commitCountMap: new Map([ + ...flatBranchesConfig([mainWt, featureA]).commitCountMap, + ["feature-a..fork/topic", 2], + ["fork/topic..feature-a", 2], + ]), + revListCherryPickMap: new Map([["fork/topic...feature-a", []]]), + createRecoveryRefCalls, + resetHardToRefCalls, + }); + const output = expectOk(await updateWorktrees({ dryRun: false }, { git })); + + expect(createRecoveryRefCalls).toEqual(["feature-a"]); + expect(resetHardToRefCalls).toEqual([{ worktreePath: "/repo-a", ref: "fork/topic" }]); + expect(output.reconciliations[0]).toMatchObject({ state: "remote-rewrite", action: "realigned" }); + }); + + test("R4: genuine divergence rebases only under the selected policy", async () => { + const recoveryCalls: string[] = []; + const rebaseCalls: FakeRebaseCall[] = []; + const git = reconciliationGit({ + commitCountMap: new Map([ + ...flatBranchesConfig([mainWt, featureA]).commitCountMap, + ["feature-a..fork/topic", 1], + ["fork/topic..feature-a", 1], + ]), + revListCherryPickMap: new Map([["fork/topic...feature-a", ["local"]]]), + createRecoveryRefCalls: recoveryCalls, + rebaseCalls, + }); + const output = expectOk(await updateWorktrees({ dryRun: false, reconcile: "rebase" }, { git })); + + expect(recoveryCalls).toEqual(["feature-a"]); + expect(rebaseCalls[0]).toMatchObject({ worktreePath: "/repo-a", onto: "fork/topic" }); + expect(output.reconciliations[0]).toMatchObject({ state: "diverged", action: "rebased" }); + }); + + test("R5/R6: dirty dry-run reports the block and performs no mutation", async () => { + const fastForwardToRefCalls: { worktreePath: string; ref: string }[] = []; + const git = reconciliationGit({ + commitCountMap: new Map([ + ...flatBranchesConfig([mainWt, featureA]).commitCountMap, + ["feature-a..fork/topic", 1], + ["fork/topic..feature-a", 0], + ]), + dirtyWorktrees: new Set(["/repo-a"]), + fastForwardToRefCalls, + }); + const output = expectOk(await updateWorktrees({ dryRun: true }, { git })); + + expect(fastForwardToRefCalls).toEqual([]); + expect(output.reconciliations[0]).toMatchObject({ action: "skipped-dirty" }); + expect(output.reports.find((report) => report.branch === "feature-a")?.result).toMatchObject({ + status: "skipped", + reason: "remote reconciliation unresolved", + }); + expect(output.unresolved).toBe(true); + }); +}); + describe("updateWorktrees — parent detection", () => { // main: A — B — C // feat-a: C — D — E diff --git a/src/application/use-cases/update-worktrees.ts b/src/application/use-cases/update-worktrees.ts index 9b35d4c..8892715 100644 --- a/src/application/use-cases/update-worktrees.ts +++ b/src/application/use-cases/update-worktrees.ts @@ -32,6 +32,28 @@ export interface UpdateWorktreesInput { upstream?: string; /** Max worktrees to rebase concurrently. Defaults to {@link DEFAULT_JOBS}. */ jobs?: number; + /** Policy for genuine feature/upstream divergence. */ + reconcile?: "rebase" | "abort" | "reset"; +} + +export type ReconcileChoice = "rebase" | "reset" | "abort"; + +export interface ReconciliationReport { + branch: string; + upstream?: string; + state: "missing" | "gone" | "equal" | "local-only" | "remote-only" | "remote-rewrite" | "diverged"; + action: + | "unchanged" + | "fast-forwarded" + | "realigned" + | "rebased" + | "aborted" + | "skipped-dirty" + | "would-fast-forward" + | "would-realign" + | "would-rebase" + | "would-reset"; + recoveryRef?: string; } /** @@ -129,13 +151,16 @@ export interface UpdateWorktreesOutput { * root (no upstream, or upstream has no other same-named local branch). */ rootSyncs: RootSyncReport[]; + reconciliations: ReconciliationReport[]; reports: WorktreeReport[]; + unresolved: boolean; } export interface UpdateWorktreesDeps { git: GitPort; shell?: ShellPort; progress?: UpdateProgressReporter; + chooseReconciliation?: (branch: string, upstream: string) => Promise; } /** @@ -495,6 +520,163 @@ export async function updateWorktrees( const localRootNames = new Set(roots.filter((r) => r.local).map((r) => r.name)); const rootBases = new Set(roots.map((r) => r.base)); + // Freeze the mutation scope from the pre-reconciliation graph. Reconciliation can + // change ancestry, so this graph is used only for target selection; the normal + // parent-discovery pass below runs again from the reconciled tips (WTK-70 R1). + const initialParentMap: Record = {}; + const initialProbeSem = new Semaphore(DEFAULT_PROBE_CONCURRENCY); + const initialFeatures = worktrees.filter((wt) => wt.branch && !localRootNames.has(wt.branch)); + await Promise.all( + initialFeatures.map(async (wt) => { + initialParentMap[wt.branch] = ( + await findParentBranch( + wt.branch, + worktrees, + defaultBranch, + roots, + localRootNames, + goneSet, + git, + initialProbeSem, + ) + ).parent; + }), + ); + const initialOrder = buildRebaseOrder(worktrees, initialParentMap, defaultBranch, rootBases, localRootNames); + if (input.branch && input.branch !== defaultBranch && !worktrees.some((w) => w.branch === input.branch)) { + return R.err(new Error(`Branch "${input.branch}" not found in worktrees`)); + } + const reconciliationTargets = + input.branch && input.branch !== defaultBranch + ? filterDescendants(input.branch, initialOrder, initialParentMap) + : initialOrder; + const reconciliationFailed = new Set(); + const reconciliations: ReconciliationReport[] = []; + + for (const wt of reconciliationTargets) { + const parent = initialParentMap[wt.branch]; + if (parent && reconciliationFailed.has(parent)) { + reconciliationFailed.add(wt.branch); + reconciliations.push({ branch: wt.branch, state: "diverged", action: "aborted" }); + continue; + } + if (goneSet.has(wt.branch)) { + reconciliations.push({ branch: wt.branch, state: "gone", action: "unchanged" }); + continue; + } + const upstreamResult = await git.getBranchUpstream(wt.branch); + const upstream = upstreamResult.success ? upstreamResult.data : null; + if (!upstream) { + reconciliations.push({ branch: wt.branch, state: "missing", action: "unchanged" }); + continue; + } + const [behindResult, aheadResult] = await Promise.all([ + git.getCommitCount(wt.branch, upstream), + git.getCommitCount(upstream, wt.branch), + ]); + if (!behindResult.success || !aheadResult.success) { + reconciliationFailed.add(wt.branch); + reconciliations.push({ branch: wt.branch, upstream, state: "diverged", action: "aborted" }); + continue; + } + const behind = behindResult.data; + const ahead = aheadResult.data; + if (behind === 0) { + reconciliations.push({ + branch: wt.branch, + upstream, + state: ahead === 0 ? "equal" : "local-only", + action: "unchanged", + }); + continue; + } + + if (ahead === 0) { + const dirty = await git.isDirty(wt.path); + if (!dirty.success || dirty.data) { + reconciliationFailed.add(wt.branch); + reconciliations.push({ + branch: wt.branch, + upstream, + state: "remote-only", + action: "skipped-dirty", + }); + continue; + } + if (input.dryRun) { + reconciliations.push({ branch: wt.branch, upstream, state: "remote-only", action: "would-fast-forward" }); + continue; + } + const moved = await git.fastForwardToRef(wt.path, upstream); + if (!moved.success) reconciliationFailed.add(wt.branch); + reconciliations.push({ + branch: wt.branch, + upstream, + state: "remote-only", + action: moved.success ? "fast-forwarded" : "aborted", + }); + continue; + } + + const uniqueLocal = await git.revListCherryPick({ base: upstream, feature: wt.branch }); + const remoteRewrite = uniqueLocal.success && uniqueLocal.data.length === 0; + const dirty = await git.isDirty(wt.path); + if (!dirty.success || dirty.data) { + reconciliationFailed.add(wt.branch); + reconciliations.push({ + branch: wt.branch, + upstream, + state: remoteRewrite ? "remote-rewrite" : "diverged", + action: "skipped-dirty", + }); + continue; + } + let choice: ReconcileChoice = remoteRewrite ? "reset" : (input.reconcile ?? "abort"); + if (!remoteRewrite && input.reconcile === "reset" && wt.branch !== input.branch) choice = "abort"; + if (!remoteRewrite && !input.dryRun && input.reconcile === undefined && deps.chooseReconciliation) { + choice = await deps.chooseReconciliation(wt.branch, upstream); + } + if (choice === "abort") { + reconciliationFailed.add(wt.branch); + reconciliations.push({ branch: wt.branch, upstream, state: "diverged", action: "aborted" }); + continue; + } + if (input.dryRun) { + reconciliations.push({ + branch: wt.branch, + upstream, + state: remoteRewrite ? "remote-rewrite" : "diverged", + action: remoteRewrite ? "would-realign" : choice === "reset" ? "would-reset" : "would-rebase", + recoveryRef: "refs/worktree-kit/recovery//", + }); + continue; + } + const recovery = await git.createRecoveryRef(wt.branch); + if (!recovery.success) { + reconciliationFailed.add(wt.branch); + reconciliations.push({ + branch: wt.branch, + upstream, + state: remoteRewrite ? "remote-rewrite" : "diverged", + action: "aborted", + }); + continue; + } + const moved = + choice === "reset" ? await git.resetHardToRef(wt.path, upstream) : await git.rebase(wt.path, upstream); + if (!moved.success) { + if (choice === "rebase") await git.rebaseAbort(wt.path); + reconciliationFailed.add(wt.branch); + } + reconciliations.push({ + branch: wt.branch, + upstream, + state: remoteRewrite ? "remote-rewrite" : "diverged", + action: moved.success ? (remoteRewrite ? "realigned" : choice === "reset" ? "realigned" : "rebased") : "aborted", + recoveryRef: recovery.data, + }); + } + // Sync every LOCAL root from its own upstream (R1), each with the WTK-61 // divergence handling (R5). Absent roots are rebase targets only and cannot // diverge, so they are skipped here. @@ -581,17 +763,13 @@ export async function updateWorktrees( const orderedWorktrees = buildRebaseOrder(worktrees, parentMap, defaultBranch, rootBases, localRootNames); - if (input.branch && input.branch !== defaultBranch && !worktrees.some((w) => w.branch === input.branch)) { - return R.err(new Error(`Branch "${input.branch}" not found in worktrees`)); - } - const targetWorktrees = input.branch && input.branch !== defaultBranch ? filterDescendants(input.branch, orderedWorktrees, parentMap) : orderedWorktrees; const reports: WorktreeReport[] = []; - const failedBranches = new Set(); + const failedBranches = new Set(reconciliationFailed); // The default branch is reported when it has a worktree; without one there is // nothing to rebase, but hook results still need somewhere to surface. @@ -614,6 +792,14 @@ export async function updateWorktrees( const retargetedFrom = retargetMap[wt.branch]; const base = { branch: wt.branch, path: wt.path, parent, retargetedFrom }; + if (reconciliationFailed.has(wt.branch)) { + return { + ...base, + result: { status: "skipped", reason: "remote reconciliation unresolved" }, + hookNotifications: [], + }; + } + if (failedBranches.has(parent)) { failedBranches.add(wt.branch); return { ...base, result: { status: "skipped", reason: `parent ${parent} failed` }, hookNotifications: [] }; @@ -820,6 +1006,8 @@ export async function updateWorktrees( defaultBranchRemoteRef, syncedFromUpstream, rootSyncs, + reconciliations, reports, + unresolved: failedBranches.size > 0, }); } diff --git a/src/cli/commands/update.test.ts b/src/cli/commands/update.test.ts index 12f033f..bd7efa1 100644 --- a/src/cli/commands/update.test.ts +++ b/src/cli/commands/update.test.ts @@ -357,6 +357,48 @@ describe("update upstream auto-detection", () => { }); }); +describe("update --reconcile (WTK-70)", () => { + const fs = () => + createFakeFilesystem({ + files: { [`${ROOT}/${CONFIG_FILENAME}`]: JSON.stringify({ rootDir: ".worktrees", upstream: false }) }, + directories: [ROOT, `${ROOT}/.worktrees`, featureWt.path], + }); + + test("reset is rejected without a named branch before git mutation", async () => { + const mergeFFOnlyCalls: { worktreePath: string; branch: string; remote: string }[] = []; + const git = createFakeGit({ worktrees: [mainWt, featureWt], mergeFFOnlyCalls }); + const { ui } = createFakeUi({ nonInteractive: true }); + + const code = await runUpdate(buildContainer(ui, git, fs()), { + "dry-run": false, + reconcile: "reset", + }); + + expect(code).toBe(3); + expect(mergeFFOnlyCalls).toEqual([]); + }); + + test("interactive genuine divergence offers rebase first and renders the recovery ref", async () => { + const commitCountMap = new Map([ + ["feature..origin/feature", 1], + ["origin/feature..feature", 1], + ]); + const git = createFakeGit({ + worktrees: [mainWt, featureWt], + branchUpstreams: new Map([["feature", "origin/feature"]]), + commitCountMap, + revListCherryPickMap: new Map([["origin/feature...feature", ["local"]]]), + }); + const { ui, log, selectCalls } = createFakeUi({ select: "rebase" }); + + const code = await runUpdate(buildContainer(ui, git, fs()), { "dry-run": false }); + + expect(code).toBe(0); + expect(selectCalls[0]?.values).toEqual(["rebase", "reset", "abort"]); + expect(log.info.some((line) => line.includes("refs/worktree-kit/recovery/feature/"))).toBe(true); + }); +}); + describe("update --cleanup — dirty worktree", () => { test("dirty gone branch is hidden from cleanup and reported as kept", async () => { const { fs, git } = dirtyGoneScenario(); @@ -587,7 +629,8 @@ describe("update — per-worktree progress (WTK-58)", () => { const code = await runUpdate(container, { "dry-run": false }); - expect(code).toBe(0); + // An unresolved parent conflict now makes the overall update fail (WTK-70 R7). + expect(code).toBe(3); // The spinner is seeded with every targeted worktree, including the child // that will be skipped because its parent conflicted. expect([...multiSpinner.keys].sort()).toEqual(["a", "b", "c"]); diff --git a/src/cli/commands/update.ts b/src/cli/commands/update.ts index b5c519f..802eeb6 100644 --- a/src/cli/commands/update.ts +++ b/src/cli/commands/update.ts @@ -128,6 +128,11 @@ export function updateCommand(container: Container) { description: "Max worktrees to rebase concurrently (default 4)", required: false, }, + reconcile: { + type: "string", + description: "Divergence policy for feature tracking refs: rebase, abort, or targeted reset", + required: false, + }, }, async run({ args }) { const { ui, git, fs, shell } = container; @@ -136,8 +141,11 @@ export function updateCommand(container: Container) { await runCommand(async () => { const parsed = v.parse(UpdateArgsSchema, args); - const { branch, cleanup: autoCleanup, jobs } = parsed; + const { branch, cleanup: autoCleanup, jobs, reconcile } = parsed; const dryRun = parsed["dry-run"]; + if (reconcile === "reset" && !branch) { + throw new CommandError("--reconcile reset requires a branch", EXIT_FAILURE); + } const configResult = await loadConfig({ git, fs }); const postUpdateHooks = configResult.success ? configResult.data.config.hooks["post-update"] : []; const onConflictHooks = configResult.success ? configResult.data.config.hooks["on-conflict"] : []; @@ -243,8 +251,38 @@ export function updateCommand(container: Container) { phaseSpinner = ui.createSpinner(); phaseSpinner.start("Fetching and analyzing worktrees..."); const result = await updateWorktrees( - { dryRun, branch, postUpdateHooks, onConflictHooks, repoRoot, upstream, jobs }, - { git, shell: needsShell ? shell : undefined, progress }, + { + dryRun, + branch, + postUpdateHooks, + onConflictHooks, + repoRoot, + upstream, + jobs, + reconcile: reconcile ?? (dryRun && !ui.nonInteractive ? "rebase" : undefined), + }, + { + git, + shell: needsShell ? shell : undefined, + progress, + chooseReconciliation: ui.nonInteractive + ? undefined + : async (branchName, trackingRef) => { + phaseSpinner?.stop(); + phaseSpinner = undefined; + const choice = await ui.select({ + message: `${branchName} diverged from ${trackingRef}`, + options: [ + { value: "rebase" as const, label: "Rebase local changes", hint: "recommended" }, + { value: "reset" as const, label: "Accept remote", hint: "saved under a recovery ref" }, + { value: "abort" as const, label: "Leave unchanged" }, + ], + }); + phaseSpinner = ui.createSpinner(); + phaseSpinner.start("Fetching and analyzing worktrees..."); + return ui.isCancel(choice) ? "abort" : choice; + }, + }, ); cleanup.clear(); @@ -268,9 +306,19 @@ export function updateCommand(container: Container) { defaultBranchRemoteRef, syncedFromUpstream, rootSyncs, + reconciliations, reports, + unresolved, } = result.data; + for (const report of reconciliations) { + const tracking = report.upstream ? ` (${report.upstream})` : ""; + const recovery = report.recoveryRef ? `; recover with: git reset --hard ${report.recoveryRef}` : ""; + const message = `${report.branch}: ${report.state}${tracking} — ${report.action}${recovery}`; + if (report.action === "aborted" || report.action === "skipped-dirty") ui.warn(message); + else ui.info(message); + } + // One summary line for the default branch, then one per extra fork root // (WTK-64). The default branch keeps its dedicated output fields; the extra // roots carry their own outcome in `rootSyncs`. @@ -320,6 +368,10 @@ export function updateCommand(container: Container) { } } + if (unresolved) { + throw new CommandError("Some worktree subtrees remain unresolved", EXIT_FAILURE); + } + const outroMessage = dryRun ? "Dry run — no changes made" : "Done!"; const goneResult = await git.listGoneBranches(); diff --git a/src/domain/ports/git-port.ts b/src/domain/ports/git-port.ts index d560b30..f19c5ea 100644 --- a/src/domain/ports/git-port.ts +++ b/src/domain/ports/git-port.ts @@ -54,6 +54,14 @@ export interface GitPort { */ getPrimaryRemote(): string; listGoneBranches(): Promise>; + /** Configured upstream ref for an arbitrary local branch, or null when none is configured. */ + getBranchUpstream(branch: string): Promise>; + /** Save the current branch tip below the internal recovery namespace and return that ref. */ + createRecoveryRef(branch: string): Promise>; + /** Fast-forward a checked-out branch to an arbitrary ref. */ + fastForwardToRef(worktreePath: string, ref: string): Promise>; + /** Hard-reset a checked-out branch to an arbitrary ref. */ + resetHardToRef(worktreePath: string, ref: string): Promise>; mergeFFOnly(worktreePath: string, branch: string, remote?: string): Promise>; /** Fast-forward a local branch ref that is not checked out anywhere, from `remote` (default: primary remote). */ updateBranchRef(branch: string, remote?: string): Promise>; diff --git a/src/domain/schemas/command-args-schema.ts b/src/domain/schemas/command-args-schema.ts index 564b769..dd687fc 100644 --- a/src/domain/schemas/command-args-schema.ts +++ b/src/domain/schemas/command-args-schema.ts @@ -25,6 +25,7 @@ export const UpdateArgsSchema = v.object({ branch: v.optional(v.string()), "dry-run": v.optional(v.boolean(), false), cleanup: v.optional(v.boolean(), false), + reconcile: v.optional(v.picklist(["rebase", "abort", "reset"])), // Max worktrees to rebase concurrently. citty hands the flag over as a string // (or a number if already numeric); coerce, then reject anything that is not a // positive integer as an argument validation error. diff --git a/src/infrastructure/adapters/bun-git-adapter.test.ts b/src/infrastructure/adapters/bun-git-adapter.test.ts index ff07864..f4898db 100644 --- a/src/infrastructure/adapters/bun-git-adapter.test.ts +++ b/src/infrastructure/adapters/bun-git-adapter.test.ts @@ -976,6 +976,28 @@ describe("BunGitAdapter", () => { expect(error.code).toBe("MERGE_FAILED"); }); + test("feature reconciliation primitives use the configured tracking ref and preserve recovery", async () => { + await using tmp = await createTempDir(); + const fixture = await createRemoteFixture(tmp.path); + await fixture.addTrackedBranch("feat"); + const wtPath = join(tmp.path, "feat-wt"); + await Bun.$`git -C ${fixture.repoPath} worktree add -q ${wtPath} feat`.quiet(); + + await withCwd(fixture.repoPath, async () => { + expect(expectOk(await git.getBranchUpstream("feat"))).toBe("origin/feat"); + expect(expectOk(await git.getBranchUpstream("main"))).toBe("origin/main"); + + const before = (await Bun.$`git -C ${fixture.repoPath} rev-parse feat`.quiet().text()).trim(); + const recovery = expectOk(await git.createRecoveryRef("feat")); + expect(recovery).toStartWith("refs/worktree-kit/recovery/feat/"); + expect((await Bun.$`git -C ${fixture.repoPath} rev-parse ${recovery}`.quiet().text()).trim()).toBe(before); + + await Bun.$`git -C ${wtPath} commit --allow-empty -m local`.quiet(); + expectOk(await git.resetHardToRef(wtPath, "origin/feat")); + expect((await Bun.$`git -C ${wtPath} rev-parse HEAD`.quiet().text()).trim()).toBe(before); + }); + }); + test("createWorktreeFromRemote checks out a remote-only branch with tracking", async () => { await using tmp = await createTempDir(); const fixture = await createRemoteFixture(tmp.path); diff --git a/src/infrastructure/adapters/bun-git-adapter.ts b/src/infrastructure/adapters/bun-git-adapter.ts index 36aa41e..2267041 100644 --- a/src/infrastructure/adapters/bun-git-adapter.ts +++ b/src/infrastructure/adapters/bun-git-adapter.ts @@ -561,6 +561,59 @@ export function createBunGitAdapter(logger: LoggerPort, primaryRemote: string): } }, + async getBranchUpstream(branch: string): Promise> { + try { + const { exitCode, stdout, stderr } = await runGit([ + "for-each-ref", + "--format=%(upstream:short)", + `refs/heads/${branch}`, + ]); + if (exitCode !== 0) { + return Result.err({ code: "UNKNOWN", message: stderr || `Failed to resolve upstream for ${branch}` }); + } + return Result.ok(stdout.trim() || null); + } catch { + return Result.err({ code: "UNKNOWN", message: `Failed to resolve upstream for ${branch}` }); + } + }, + + async createRecoveryRef(branch: string): Promise> { + const stamp = new Date().toISOString().replace(/[:.]/g, "-"); + const ref = `refs/worktree-kit/recovery/${branch}/${stamp}-${process.pid}`; + try { + const zeroOid = "0000000000000000000000000000000000000000"; + const { exitCode, stderr } = await runGit(["update-ref", ref, branch, zeroOid]); + if (exitCode !== 0) { + return Result.err({ code: "UNKNOWN", message: stderr || `Failed to create recovery ref for ${branch}` }); + } + return Result.ok(ref); + } catch { + return Result.err({ code: "UNKNOWN", message: `Failed to create recovery ref for ${branch}` }); + } + }, + + async fastForwardToRef(worktreePath: string, ref: string): Promise> { + try { + const { exitCode, stderr } = await runGit(["-C", worktreePath, "merge", "--ff-only", ref]); + return exitCode === 0 + ? Result.ok(undefined) + : Result.err({ code: "MERGE_FAILED", message: stderr || `Failed to fast-forward to ${ref}` }); + } catch { + return Result.err({ code: "UNKNOWN", message: `Failed to fast-forward to ${ref}` }); + } + }, + + async resetHardToRef(worktreePath: string, ref: string): Promise> { + try { + const { exitCode, stderr } = await runGit(["-C", worktreePath, "reset", "--hard", ref]); + return exitCode === 0 + ? Result.ok(undefined) + : Result.err({ code: "MERGE_FAILED", message: stderr || `Failed to reset to ${ref}` }); + } catch { + return Result.err({ code: "UNKNOWN", message: `Failed to reset to ${ref}` }); + } + }, + async mergeFFOnly(worktreePath: string, branch: string, remote?: string): Promise> { try { const remoteName = remote ?? primaryRemote; diff --git a/src/test-utils/fake-git.ts b/src/test-utils/fake-git.ts index d9ed416..f80c2ba 100644 --- a/src/test-utils/fake-git.ts +++ b/src/test-utils/fake-git.ts @@ -39,6 +39,10 @@ export interface FakeGitOptions { updateBranchRefCalls?: { branch: string; remote: string }[]; resetHardToRemoteCalls?: { worktreePath: string; branch: string; remote: string }[]; forceUpdateBranchRefCalls?: { branch: string; remote: string }[]; + branchUpstreams?: Map; + createRecoveryRefCalls?: string[]; + fastForwardToRefCalls?: { worktreePath: string; ref: string }[]; + resetHardToRefCalls?: { worktreePath: string; ref: string }[]; mergeBaseMap?: Map; commitCountMap?: Map; trackedPaths?: Set; @@ -284,6 +288,25 @@ export function createFakeGit(options: FakeGitOptions = {}): GitPort { return Result.ok([...goneBranches]); }, + async getBranchUpstream(branch: string): Promise> { + return Result.ok(options.branchUpstreams?.get(branch) ?? null); + }, + + async createRecoveryRef(branch: string): Promise> { + options.createRecoveryRefCalls?.push(branch); + return Result.ok(`refs/worktree-kit/recovery/${branch}/fake-timestamp`); + }, + + async fastForwardToRef(worktreePath: string, ref: string): Promise> { + options.fastForwardToRefCalls?.push({ worktreePath, ref }); + return Result.ok(undefined); + }, + + async resetHardToRef(worktreePath: string, ref: string): Promise> { + options.resetHardToRefCalls?.push({ worktreePath, ref }); + return Result.ok(undefined); + }, + async mergeFFOnly(worktreePath: string, branch: string, remote = primaryRemote): Promise> { options.mergeFFOnlyCalls?.push({ worktreePath, branch, remote }); if (mergeFFOnlyFails || options.mergeFFOnlyFailBranches?.has(branch)) { From 567a7db584129c2b40f67f5ff5dbb572c73a2b13 Mon Sep 17 00:00:00 2001 From: Evgeniy Podivilov Date: Wed, 16 Sep 2026 13:48:46 +0100 Subject: [PATCH 2/2] fix(update): address reconciliation review findings --- .../use-cases/update-worktrees.test.ts | 39 ++++++++++++ src/application/use-cases/update-worktrees.ts | 62 ++++++++++++++++--- src/cli/commands/update.test.ts | 32 +++++++++- src/cli/commands/update.ts | 31 ++++++---- src/test-utils/fake-git.ts | 4 ++ 5 files changed, 147 insertions(+), 21 deletions(-) diff --git a/src/application/use-cases/update-worktrees.test.ts b/src/application/use-cases/update-worktrees.test.ts index d19899b..51d6e39 100644 --- a/src/application/use-cases/update-worktrees.test.ts +++ b/src/application/use-cases/update-worktrees.test.ts @@ -360,6 +360,45 @@ describe("updateWorktrees — feature tracking reconciliation (WTK-70)", () => { }); expect(output.unresolved).toBe(true); }); + + test("R7: upstream lookup failures leave the branch unresolved", async () => { + const git = reconciliationGit({ + getBranchUpstreamFail: { code: "UNKNOWN", message: "cannot read config" }, + }); + const output = expectOk(await updateWorktrees({ dryRun: false }, { git })); + + expect(output.reconciliations[0]).toMatchObject({ + branch: "feature-a", + state: "missing", + action: "aborted", + warning: "Failed to resolve tracking ref: cannot read config", + }); + expect(output.reports.find((report) => report.branch === "feature-a")?.result).toMatchObject({ + status: "skipped", + reason: "remote reconciliation unresolved", + }); + expect(output.unresolved).toBe(true); + }); + + test("R7: a failed reconciliation rebase reports a failed abort", async () => { + const git = reconciliationGit({ + commitCountMap: new Map([ + ...flatBranchesConfig([mainWt, featureA]).commitCountMap, + ["feature-a..fork/topic", 1], + ["fork/topic..feature-a", 1], + ]), + revListCherryPickMap: new Map([["fork/topic...feature-a", ["local"]]]), + rebaseConflicts: new Set(["/repo-a"]), + rebaseAbortFail: { code: "UNKNOWN", message: "abort failed" }, + }); + const output = expectOk(await updateWorktrees({ dryRun: false, reconcile: "rebase" }, { git })); + + expect(output.reconciliations[0]).toMatchObject({ + action: "aborted", + warning: "Rebase abort failed: abort failed", + }); + expect(output.unresolved).toBe(true); + }); }); describe("updateWorktrees — parent detection", () => { diff --git a/src/application/use-cases/update-worktrees.ts b/src/application/use-cases/update-worktrees.ts index 8892715..1c189cc 100644 --- a/src/application/use-cases/update-worktrees.ts +++ b/src/application/use-cases/update-worktrees.ts @@ -40,6 +40,7 @@ export type ReconcileChoice = "rebase" | "reset" | "abort"; export interface ReconciliationReport { branch: string; + worktreePath: string; upstream?: string; state: "missing" | "gone" | "equal" | "local-only" | "remote-only" | "remote-rewrite" | "diverged"; action: @@ -54,6 +55,7 @@ export interface ReconciliationReport { | "would-rebase" | "would-reset"; recoveryRef?: string; + warning?: string; } /** @@ -550,6 +552,7 @@ export async function updateWorktrees( input.branch && input.branch !== defaultBranch ? filterDescendants(input.branch, initialOrder, initialParentMap) : initialOrder; + const reconciliationTargetBranches = new Set(reconciliationTargets.map((wt) => wt.branch)); const reconciliationFailed = new Set(); const reconciliations: ReconciliationReport[] = []; @@ -557,17 +560,28 @@ export async function updateWorktrees( const parent = initialParentMap[wt.branch]; if (parent && reconciliationFailed.has(parent)) { reconciliationFailed.add(wt.branch); - reconciliations.push({ branch: wt.branch, state: "diverged", action: "aborted" }); + reconciliations.push({ branch: wt.branch, worktreePath: wt.path, state: "diverged", action: "aborted" }); continue; } if (goneSet.has(wt.branch)) { - reconciliations.push({ branch: wt.branch, state: "gone", action: "unchanged" }); + reconciliations.push({ branch: wt.branch, worktreePath: wt.path, state: "gone", action: "unchanged" }); continue; } const upstreamResult = await git.getBranchUpstream(wt.branch); - const upstream = upstreamResult.success ? upstreamResult.data : null; + if (!upstreamResult.success) { + reconciliationFailed.add(wt.branch); + reconciliations.push({ + branch: wt.branch, + worktreePath: wt.path, + state: "missing", + action: "aborted", + warning: `Failed to resolve tracking ref: ${upstreamResult.error.message}`, + }); + continue; + } + const upstream = upstreamResult.data; if (!upstream) { - reconciliations.push({ branch: wt.branch, state: "missing", action: "unchanged" }); + reconciliations.push({ branch: wt.branch, worktreePath: wt.path, state: "missing", action: "unchanged" }); continue; } const [behindResult, aheadResult] = await Promise.all([ @@ -576,7 +590,13 @@ export async function updateWorktrees( ]); if (!behindResult.success || !aheadResult.success) { reconciliationFailed.add(wt.branch); - reconciliations.push({ branch: wt.branch, upstream, state: "diverged", action: "aborted" }); + reconciliations.push({ + branch: wt.branch, + worktreePath: wt.path, + upstream, + state: "diverged", + action: "aborted", + }); continue; } const behind = behindResult.data; @@ -584,6 +604,7 @@ export async function updateWorktrees( if (behind === 0) { reconciliations.push({ branch: wt.branch, + worktreePath: wt.path, upstream, state: ahead === 0 ? "equal" : "local-only", action: "unchanged", @@ -597,6 +618,7 @@ export async function updateWorktrees( reconciliationFailed.add(wt.branch); reconciliations.push({ branch: wt.branch, + worktreePath: wt.path, upstream, state: "remote-only", action: "skipped-dirty", @@ -604,13 +626,20 @@ export async function updateWorktrees( continue; } if (input.dryRun) { - reconciliations.push({ branch: wt.branch, upstream, state: "remote-only", action: "would-fast-forward" }); + reconciliations.push({ + branch: wt.branch, + worktreePath: wt.path, + upstream, + state: "remote-only", + action: "would-fast-forward", + }); continue; } const moved = await git.fastForwardToRef(wt.path, upstream); if (!moved.success) reconciliationFailed.add(wt.branch); reconciliations.push({ branch: wt.branch, + worktreePath: wt.path, upstream, state: "remote-only", action: moved.success ? "fast-forwarded" : "aborted", @@ -625,6 +654,7 @@ export async function updateWorktrees( reconciliationFailed.add(wt.branch); reconciliations.push({ branch: wt.branch, + worktreePath: wt.path, upstream, state: remoteRewrite ? "remote-rewrite" : "diverged", action: "skipped-dirty", @@ -638,12 +668,19 @@ export async function updateWorktrees( } if (choice === "abort") { reconciliationFailed.add(wt.branch); - reconciliations.push({ branch: wt.branch, upstream, state: "diverged", action: "aborted" }); + reconciliations.push({ + branch: wt.branch, + worktreePath: wt.path, + upstream, + state: "diverged", + action: "aborted", + }); continue; } if (input.dryRun) { reconciliations.push({ branch: wt.branch, + worktreePath: wt.path, upstream, state: remoteRewrite ? "remote-rewrite" : "diverged", action: remoteRewrite ? "would-realign" : choice === "reset" ? "would-reset" : "would-rebase", @@ -656,6 +693,7 @@ export async function updateWorktrees( reconciliationFailed.add(wt.branch); reconciliations.push({ branch: wt.branch, + worktreePath: wt.path, upstream, state: remoteRewrite ? "remote-rewrite" : "diverged", action: "aborted", @@ -664,16 +702,22 @@ export async function updateWorktrees( } const moved = choice === "reset" ? await git.resetHardToRef(wt.path, upstream) : await git.rebase(wt.path, upstream); + let warning: string | undefined; if (!moved.success) { - if (choice === "rebase") await git.rebaseAbort(wt.path); + if (choice === "rebase") { + const abortResult = await git.rebaseAbort(wt.path); + if (!abortResult.success) warning = `Rebase abort failed: ${abortResult.error.message}`; + } reconciliationFailed.add(wt.branch); } reconciliations.push({ branch: wt.branch, + worktreePath: wt.path, upstream, state: remoteRewrite ? "remote-rewrite" : "diverged", action: moved.success ? (remoteRewrite ? "realigned" : choice === "reset" ? "realigned" : "rebased") : "aborted", recoveryRef: recovery.data, + warning, }); } @@ -765,7 +809,7 @@ export async function updateWorktrees( const targetWorktrees = input.branch && input.branch !== defaultBranch - ? filterDescendants(input.branch, orderedWorktrees, parentMap) + ? orderedWorktrees.filter((wt) => reconciliationTargetBranches.has(wt.branch)) : orderedWorktrees; const reports: WorktreeReport[] = []; diff --git a/src/cli/commands/update.test.ts b/src/cli/commands/update.test.ts index bd7efa1..578b77d 100644 --- a/src/cli/commands/update.test.ts +++ b/src/cli/commands/update.test.ts @@ -395,7 +395,37 @@ describe("update --reconcile (WTK-70)", () => { expect(code).toBe(0); expect(selectCalls[0]?.values).toEqual(["rebase", "reset", "abort"]); - expect(log.info.some((line) => line.includes("refs/worktree-kit/recovery/feature/"))).toBe(true); + expect( + log.info.some((line) => + line.includes(`recover with: git -C '${featureWt.path}' reset --hard 'refs/worktree-kit/recovery/feature/`), + ), + ).toBe(true); + }); + + test("unresolved reconciliation still runs requested cleanup before failing", async () => { + const git = createFakeGit({ + worktrees: [mainWt, featureWt], + branches: ["main", "feature", "gone"], + goneBranches: ["gone"], + mergedBranches: ["gone"], + branchUpstreams: new Map([["feature", "origin/feature"]]), + commitCountMap: new Map([ + ["feature..origin/feature", 1], + ["origin/feature..feature", 1], + ["main..gone", 1], + ]), + revListMap: new Map([["main..gone", ["gone-sha"]]]), + revListCherryPickMap: new Map([ + ["origin/feature...feature", ["local"]], + ["main...gone", []], + ]), + }); + const { ui, log } = createFakeUi({ nonInteractive: true }); + + const code = await runUpdate(buildContainer(ui, git, fs()), { "dry-run": false, cleanup: true }); + + expect(code).toBe(3); + expect(log.success).toContain("gone — branch removed (no matching worktree found)"); }); }); diff --git a/src/cli/commands/update.ts b/src/cli/commands/update.ts index 802eeb6..991cfd8 100644 --- a/src/cli/commands/update.ts +++ b/src/cli/commands/update.ts @@ -23,6 +23,10 @@ import { type LockedWorktree, warnLockedWorktreesGroup } from "../locked-worktre import { resolveUpstream } from "../resolve-upstream.ts"; import { CommandError, runCommand } from "../run-command.ts"; +function shellQuote(value: string): string { + return `'${value.replaceAll("'", `'"'"'`)}'`; +} + /** * Maps a per-worktree report to a single terminal spinner line. `complete` (✓) is * used for benign outcomes (rebased, would-be-rebased, already-merged skip); `fail` @@ -313,8 +317,11 @@ export function updateCommand(container: Container) { for (const report of reconciliations) { const tracking = report.upstream ? ` (${report.upstream})` : ""; - const recovery = report.recoveryRef ? `; recover with: git reset --hard ${report.recoveryRef}` : ""; - const message = `${report.branch}: ${report.state}${tracking} — ${report.action}${recovery}`; + const recovery = report.recoveryRef + ? `; recover with: git -C ${shellQuote(report.worktreePath)} reset --hard ${shellQuote(report.recoveryRef)}` + : ""; + const warning = report.warning ? `; ${report.warning}` : ""; + const message = `${report.branch}: ${report.state}${tracking} — ${report.action}${recovery}${warning}`; if (report.action === "aborted" || report.action === "skipped-dirty") ui.warn(message); else ui.info(message); } @@ -368,11 +375,13 @@ export function updateCommand(container: Container) { } } - if (unresolved) { - throw new CommandError("Some worktree subtrees remain unresolved", EXIT_FAILURE); - } - const outroMessage = dryRun ? "Dry run — no changes made" : "Done!"; + const finish = () => { + if (unresolved) { + throw new CommandError("Some worktree subtrees remain unresolved", EXIT_FAILURE); + } + ui.outro(outroMessage); + }; const goneResult = await git.listGoneBranches(); const staleBranches = Result.isOk(goneResult) ? goneResult.data.filter((b) => b !== defaultBranch) : []; @@ -388,7 +397,7 @@ export function updateCommand(container: Container) { .map((r) => r.branch); if (staleBranches.length === 0 && rebaseMerged.length === 0) { - ui.outro(outroMessage); + finish(); return; } @@ -444,13 +453,13 @@ export function updateCommand(container: Container) { if (kept.length > 0) { ui.info(keptMessage); } - ui.outro(outroMessage); + finish(); return; } if (ui.nonInteractive && !autoCleanup) { ui.warn(`${merged.length} branch(es) have gone remotes, run 'wt cleanup'`); - ui.outro(outroMessage); + finish(); return; } @@ -478,7 +487,7 @@ export function updateCommand(container: Container) { } if (!shouldCleanup) { - ui.outro(outroMessage); + finish(); return; } @@ -565,7 +574,7 @@ export function updateCommand(container: Container) { warnLockedWorktreesGroup(ui, lockedWorktrees, "wt update"); - ui.outro(outroMessage); + finish(); }, ui); }, }); diff --git a/src/test-utils/fake-git.ts b/src/test-utils/fake-git.ts index f80c2ba..f2b7068 100644 --- a/src/test-utils/fake-git.ts +++ b/src/test-utils/fake-git.ts @@ -40,6 +40,7 @@ export interface FakeGitOptions { resetHardToRemoteCalls?: { worktreePath: string; branch: string; remote: string }[]; forceUpdateBranchRefCalls?: { branch: string; remote: string }[]; branchUpstreams?: Map; + getBranchUpstreamFail?: { code: GitError["code"]; message: string }; createRecoveryRefCalls?: string[]; fastForwardToRefCalls?: { worktreePath: string; ref: string }[]; resetHardToRefCalls?: { worktreePath: string; ref: string }[]; @@ -289,6 +290,9 @@ export function createFakeGit(options: FakeGitOptions = {}): GitPort { }, async getBranchUpstream(branch: string): Promise> { + if (options.getBranchUpstreamFail !== undefined) { + return Result.err(options.getBranchUpstreamFail); + } return Result.ok(options.branchUpstreams?.get(branch) ?? null); },