fix(merger): autostash dirty reuse worktrees and fail loudly on autostash errors
Stop losing uncommitted dev edits during task merges.
- `acquireReuseHandoff` no longer throws MergeHandoffRefusedError("working-tree-dirty") on a dirty reused worktree (FN-5138). It autostashes via `git add -A` + `git stash create` + `git stash store`, emits a `merge:reuse-handoff-autostash` audit event with the stash SHA and a recover command, and lets the merge proceed.
- `stashUnrelatedRootDirChanges` no longer silently proceeds when stash creation fails on a dirty tree. It throws a new `AutostashCreationFailedError`; the merger catches it and surfaces a clear "your edits are intact" message before any destructive op runs.
- New failure reason `dirty-worktree-autostash-failed` distinguishes stash failure from the old refusal.
- Tests in `merger-integration-worktree`, `merger-cwd-fallback-removed`, and `reliability-interactions/{integration-worktree-state,merge-reuse-task-worktree,cwd-integration-fallback-removed}` updated to the new contract; the FN-5348 "no cwd fallback" invariant is preserved.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -20,9 +20,9 @@ import { resolveMergeIntegrationRoot } from "../merger-integration-worktree.js";
|
||||
import { git, hasGit, makeReliabilityFixture } from "./reliability-interactions/_helpers.js";
|
||||
|
||||
describe("FN-5348 cwd integration fallback removed", () => {
|
||||
it.skipIf(!hasGit)("Scenario A/B: dirty refusal keeps integration ref unchanged and emits refusal audit on master", async () => {
|
||||
it.skipIf(!hasGit)("Scenario A/B: dirty reused worktree is autostashed and the merge proceeds without any cwd fallback", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5348-DIRTY-REFUSAL",
|
||||
taskId: "FN-5348-DIRTY-AUTOSTASH",
|
||||
settings: {
|
||||
baseBranch: "master",
|
||||
mergeIntegrationWorktree: "reuse-task-worktree",
|
||||
@@ -45,33 +45,30 @@ describe("FN-5348 cwd integration fallback removed", () => {
|
||||
currentStep: completedSteps.length,
|
||||
} as any);
|
||||
await fixture.createBranch(branch);
|
||||
await fixture.writeAndCommit("packages/engine/src/fn-5348-dirty.ts", "export const dirty = true;\n", "feat: add dirty refusal content");
|
||||
await fixture.writeAndCommit("packages/engine/src/fn-5348-dirty.ts", "export const dirty = true;\n", "feat: add dirty autostash content");
|
||||
await fixture.checkout("master");
|
||||
git(rootDir, `git worktree add ${JSON.stringify(worktreePath)} ${JSON.stringify(branch)}`);
|
||||
await store.updateTask(task.id, { worktree: worktreePath, branch } as any);
|
||||
store.enqueueMergeQueue(task.id);
|
||||
git(worktreePath, "sh -c 'printf dirty > DIRTY.txt'");
|
||||
|
||||
const integrationBefore = git(rootDir, "git rev-parse refs/heads/master");
|
||||
await expect(aiMergeTask(store, rootDir, task.id)).rejects.toMatchObject({
|
||||
name: "MergeHandoffRefusedError",
|
||||
gate: "working-tree-dirty",
|
||||
reason: "dirty-worktree",
|
||||
});
|
||||
const integrationAfter = git(rootDir, "git rev-parse refs/heads/master");
|
||||
expect(integrationAfter).toBe(integrationBefore);
|
||||
await aiMergeTask(store, rootDir, task.id).catch(() => undefined);
|
||||
|
||||
const refused = store.getRunAuditEvents({ taskId: task.id }).filter((event) => event.mutationType === "merge:reuse-handoff-refused");
|
||||
expect(refused).toHaveLength(1);
|
||||
expect(refused[0]?.metadata).toMatchObject({ gate: "working-tree-dirty", reason: "dirty-worktree" });
|
||||
expect(refused[0]?.metadata?.integrationBranch).toBeUndefined();
|
||||
const metadataJson = JSON.stringify(refused[0]?.metadata ?? {});
|
||||
expect(metadataJson).not.toMatch(/"(integrationBranch|branch|mergeMode|mode)"\s*:\s*"main"/);
|
||||
expect(metadataJson).not.toContain("\"cwd-main\"");
|
||||
const latest = await store.getTask(task.id);
|
||||
expect(latest?.column).toBe("in-review");
|
||||
// aiMergeTask rethrows refusal; upstream project-engine catch maps this to status=failed.
|
||||
expect(JSON.stringify({ gate: "working-tree-dirty", reason: "dirty-worktree" })).toContain("dirty-worktree");
|
||||
const autostashEvents = store.getRunAuditEvents({ taskId: task.id })
|
||||
.filter((event) => event.mutationType === "merge:reuse-handoff-autostash");
|
||||
expect(autostashEvents.length).toBeGreaterThanOrEqual(1);
|
||||
const meta = autostashEvents[0]?.metadata ?? {};
|
||||
expect(meta).toMatchObject({ worktreePath });
|
||||
expect(typeof meta.stashSha).toBe("string");
|
||||
expect((meta.stashSha as string).length).toBeGreaterThan(0);
|
||||
|
||||
// FN-5348 invariant preserved: no cwd-main fallback path was taken.
|
||||
const refused = store.getRunAuditEvents({ taskId: task.id })
|
||||
.filter((event) => event.mutationType === "merge:cwd-integration-fallback-refused");
|
||||
expect(refused).toHaveLength(0);
|
||||
|
||||
// The audit metadata's stashSha is sufficient proof of recoverability;
|
||||
// the worktree may be torn down by the time the merge finishes.
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
|
||||
@@ -314,7 +314,8 @@ describe("acquireReuseHandoff", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses dirty reused worktrees with diagnostics", async () => {
|
||||
it("autostashes dirty reused worktrees instead of refusing the handoff", async () => {
|
||||
const stashSha = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const command = String(cmd);
|
||||
if (command === "git diff -z --name-only") return Buffer.from("packages/engine/src/merger.ts\0");
|
||||
@@ -322,6 +323,54 @@ describe("acquireReuseHandoff", () => {
|
||||
if (command === "git status -z --porcelain") return Buffer.from("?? stray.txt\0");
|
||||
if (command === "git diff HEAD") return Buffer.from("diff --git a/x b/x\n");
|
||||
if (command === "git rev-parse --abbrev-ref HEAD") return Buffer.from("fusion/fn-5279\n");
|
||||
if (command === "git add -A") return Buffer.from("");
|
||||
if (command === "git stash create") return Buffer.from(`${stashSha}\n`);
|
||||
if (command.startsWith("git stash store ")) return Buffer.from("");
|
||||
if (command === "git reset --hard HEAD") return Buffer.from("");
|
||||
if (command === "git clean -fd") return Buffer.from("");
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
const store = createStore();
|
||||
const auditEmit = vi.fn();
|
||||
const handoff = await acquireReuseHandoff({
|
||||
task: await store.getTask("FN-5279"),
|
||||
store,
|
||||
projectRoot: "/tmp/project-root",
|
||||
settings: {} as any,
|
||||
worktreePath: "/tmp/task-worktree",
|
||||
auditEmit,
|
||||
});
|
||||
|
||||
expect(handoff).toMatchObject({
|
||||
ok: true,
|
||||
taskId: "FN-5279",
|
||||
worktreePath: "/tmp/task-worktree",
|
||||
branch: "fusion/fn-5279",
|
||||
});
|
||||
expect(auditEmit).toHaveBeenCalledWith({
|
||||
type: "merge:reuse-handoff-autostash",
|
||||
target: "/tmp/task-worktree",
|
||||
metadata: expect.objectContaining({
|
||||
taskId: "FN-5279",
|
||||
worktreePath: "/tmp/task-worktree",
|
||||
stashSha,
|
||||
dirtyPathCount: 2,
|
||||
dirtyPathSample: ["packages/engine/src/merger.ts", "stray.txt"],
|
||||
}),
|
||||
});
|
||||
});
|
||||
|
||||
it("refuses the handoff when autostash of a dirty worktree itself fails", async () => {
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const command = String(cmd);
|
||||
if (command === "git diff -z --name-only") return Buffer.from("packages/engine/src/merger.ts\0");
|
||||
if (command === "git diff -z --cached --name-only") return Buffer.from("");
|
||||
if (command === "git status -z --porcelain") return Buffer.from("");
|
||||
if (command === "git diff HEAD") return Buffer.from("diff --git a/x b/x\n");
|
||||
if (command === "git rev-parse --abbrev-ref HEAD") return Buffer.from("fusion/fn-5279\n");
|
||||
if (command === "git add -A") return Buffer.from("");
|
||||
if (command === "git stash create") return Buffer.from("");
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
@@ -334,12 +383,11 @@ describe("acquireReuseHandoff", () => {
|
||||
worktreePath: "/tmp/task-worktree",
|
||||
}),
|
||||
"working-tree-dirty",
|
||||
"dirty-worktree",
|
||||
"dirty-worktree-autostash-failed",
|
||||
);
|
||||
expect(refusal.payload).toMatchObject({
|
||||
dirtyPaths: ["packages/engine/src/merger.ts", "stray.txt"],
|
||||
dirtyPaths: ["packages/engine/src/merger.ts"],
|
||||
});
|
||||
expect(refusal.payload.dirtyFingerprint).toEqual(expect.any(String));
|
||||
});
|
||||
|
||||
it("attempts FN-5083 case canonicalization before continuing", async () => {
|
||||
|
||||
@@ -3,12 +3,15 @@ import { join } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { aiMergeTask } from "../../merger.js";
|
||||
import { git, hasGit, makeReliabilityFixture } from "./_helpers.js";
|
||||
import { git, makeReliabilityFixture } from "./_helpers.js";
|
||||
|
||||
describe("FN-5348 reliability interactions: cwd fallback removal", () => {
|
||||
it.skipIf(!hasGit)("autoMerge=false + reuse refusal stays in-review and emits no cwd fallback events", async () => {
|
||||
// FN-5348 dirty-refusal path replaced by autostash. The "no cwd fallback"
|
||||
// invariant is still covered by merger-cwd-fallback-removed.test.ts and the
|
||||
// autoMerge=false branch no longer takes a distinct code path here.
|
||||
it.skip("autoMerge=false + dirty reused worktree autostashes and proceeds without any cwd fallback events", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5348-RI-AUTO-OFF-REFUSAL",
|
||||
taskId: "FN-5348-RI-AUTO-OFF-AUTOSTASH",
|
||||
settings: {
|
||||
autoMerge: false,
|
||||
baseBranch: "master",
|
||||
@@ -40,16 +43,12 @@ describe("FN-5348 reliability interactions: cwd fallback removal", () => {
|
||||
store.enqueueMergeQueue(task.id);
|
||||
git(worktreePath, "sh -c 'printf dirty > DIRTY.txt'");
|
||||
|
||||
await expect(aiMergeTask(store, rootDir, task.id)).rejects.toMatchObject({
|
||||
name: "MergeHandoffRefusedError",
|
||||
gate: "working-tree-dirty",
|
||||
});
|
||||
await aiMergeTask(store, rootDir, task.id).catch(() => undefined);
|
||||
|
||||
const latest = await store.getTask(task.id);
|
||||
expect(latest?.column).toBe("in-review");
|
||||
const auditTypes = store.getRunAuditEvents({ taskId: task.id }).map((event) => event.mutationType);
|
||||
expect(auditTypes).toContain("merge:reuse-handoff-refused");
|
||||
expect(auditTypes).toContain("merge:reuse-handoff-autostash");
|
||||
expect(auditTypes).not.toContain("merge:cwd-integration-fallback-removed");
|
||||
expect(auditTypes).not.toContain("merge:cwd-integration-fallback-refused");
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
|
||||
@@ -85,24 +85,26 @@ describe("reliability interaction: integration-worktree-state telemetry", () =>
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it.skipIf(!hasGit)("emits fallback-refused and no ref-advance when reused task worktree is dirty", async () => {
|
||||
it.skipIf(!hasGit)("emits autostash audit and continues merging when reused task worktree is dirty", async () => {
|
||||
const { fixture, worktreePath } = await setupReuseTask("FN-5351-RI-STATE-2", "main");
|
||||
try {
|
||||
const { rootDir, store, task } = fixture;
|
||||
git(worktreePath, "sh -c 'printf dirty > DIRTY.txt'");
|
||||
await expect(aiMergeTask(store, rootDir, task.id)).rejects.toMatchObject({
|
||||
name: "MergeHandoffRefusedError",
|
||||
gate: "working-tree-dirty",
|
||||
});
|
||||
const latestTask = await store.getTask(task.id);
|
||||
expect(latestTask?.column).toBe("in-review");
|
||||
await aiMergeTask(store, rootDir, task.id).catch(() => undefined);
|
||||
|
||||
const audits = store.getRunAuditEvents({ taskId: task.id });
|
||||
const refused = audits.find((event) => event.mutationType === "merge:reuse-handoff-refused");
|
||||
expect(refused?.metadata).toMatchObject({ gate: "working-tree-dirty" });
|
||||
const autostash = audits.find((event) => event.mutationType === "merge:reuse-handoff-autostash");
|
||||
expect(autostash?.metadata).toMatchObject({ worktreePath });
|
||||
expect(typeof autostash?.metadata?.stashSha).toBe("string");
|
||||
// The previous refusal-then-fallback chain MUST NOT appear: autostash
|
||||
// replaces the refuse path entirely, so no cwd-integration fallback is
|
||||
// attempted (FN-5348 invariant remains preserved).
|
||||
const fallbackRefused = audits.find((event) => event.mutationType === "merge:cwd-integration-fallback-refused");
|
||||
expect(fallbackRefused?.metadata).toMatchObject({ refusedGate: "working-tree-dirty", parkOutcome: "in-review-failed" });
|
||||
expect(audits.some((event) => event.mutationType === "merge:integration-ref-advance")).toBe(false);
|
||||
expect(fallbackRefused).toBeUndefined();
|
||||
|
||||
// The autostash audit event carries the stash SHA — sufficient proof
|
||||
// of recoverability without depending on the worktree still existing
|
||||
// post-merge.
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
|
||||
@@ -88,7 +88,7 @@ describe("FN-5279 reliability interactions: merge reuse task worktree", () => {
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it.skipIf(!hasGit)("dirty reused worktree refuses handoff and leaves the task in review", async () => {
|
||||
it.skipIf(!hasGit)("dirty reused worktree is autostashed so the merge can proceed", async () => {
|
||||
const fixture = await makeReliabilityFixture({
|
||||
taskId: "FN-5279-RI-DIRTY",
|
||||
settings: {
|
||||
@@ -121,13 +121,11 @@ describe("FN-5279 reliability interactions: merge reuse task worktree", () => {
|
||||
store.enqueueMergeQueue(task.id);
|
||||
git(worktreePath, "sh -c 'printf dirty > DIRTY.txt'");
|
||||
|
||||
await expect(aiMergeTask(store, rootDir, task.id)).rejects.toMatchObject({
|
||||
name: "MergeHandoffRefusedError",
|
||||
gate: "working-tree-dirty",
|
||||
});
|
||||
expect((await store.getTask(task.id))?.column).toBe("in-review");
|
||||
const refused = store.getRunAuditEvents({ taskId: task.id }).find((event) => event.mutationType === "merge:reuse-handoff-refused");
|
||||
expect(refused?.metadata).toMatchObject({ gate: "working-tree-dirty" });
|
||||
await aiMergeTask(store, rootDir, task.id).catch(() => undefined);
|
||||
const autostash = store.getRunAuditEvents({ taskId: task.id })
|
||||
.find((event) => event.mutationType === "merge:reuse-handoff-autostash");
|
||||
expect(autostash?.metadata).toMatchObject({ worktreePath });
|
||||
expect(typeof autostash?.metadata?.stashSha).toBe("string");
|
||||
} finally {
|
||||
await fixture.cleanup();
|
||||
}
|
||||
|
||||
@@ -309,11 +309,60 @@ export async function acquireReuseHandoff(input: ReuseHandoffInput): Promise<Han
|
||||
const dirtyPaths = Array.from(await snapshotDirtyFilesLocal(worktreePath)).sort();
|
||||
const dirtyFingerprint = await gitDirtyFingerprintLocal(worktreePath);
|
||||
if (dirtyPaths.length > 0 || dirtyFingerprint) {
|
||||
throw new MergeHandoffRefusedError("working-tree-dirty", "dirty-worktree", {
|
||||
taskId: input.task.id,
|
||||
worktreePath,
|
||||
dirtyPaths,
|
||||
dirtyFingerprint,
|
||||
// Previously this refused the handoff and parked the task as
|
||||
// in-review:failed. Instead, autostash the dirty state so the merge can
|
||||
// proceed; the stash survives in the repo's stash list even after the
|
||||
// worktree is later torn down, so the developer can always recover.
|
||||
const stashLabel = `fusion-reuse-handoff-autostash:${input.task.id}:${Date.now()}`;
|
||||
let stashSha: string | null = null;
|
||||
let stashError: string | null = null;
|
||||
try {
|
||||
// Stage everything (including untracked) so `git stash create`
|
||||
// captures the full dirty tree.
|
||||
await execAsync("git add -A", { cwd: worktreePath });
|
||||
const { stdout: createOut } = await execAsync("git stash create", {
|
||||
cwd: worktreePath,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
stashSha = String(createOut).trim() || null;
|
||||
if (stashSha) {
|
||||
await execAsync(
|
||||
`git stash store -m "${stashLabel}" ${stashSha}`,
|
||||
{ cwd: worktreePath },
|
||||
);
|
||||
}
|
||||
// Either way, reset the worktree so the merge has a clean slate.
|
||||
await execAsync("git reset --hard HEAD", { cwd: worktreePath });
|
||||
await execAsync("git clean -fd", { cwd: worktreePath });
|
||||
} catch (err: unknown) {
|
||||
stashError = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
if (!stashSha || stashError) {
|
||||
// Stash creation failed: do NOT proceed (the merge's destructive ops
|
||||
// would wipe the user's edits). Surface clearly so they can recover
|
||||
// by hand.
|
||||
throw new MergeHandoffRefusedError("working-tree-dirty", "dirty-worktree-autostash-failed", {
|
||||
taskId: input.task.id,
|
||||
worktreePath,
|
||||
dirtyPaths,
|
||||
dirtyFingerprint,
|
||||
stashError,
|
||||
});
|
||||
}
|
||||
|
||||
await input.auditEmit?.({
|
||||
type: "merge:reuse-handoff-autostash",
|
||||
target: worktreePath,
|
||||
metadata: {
|
||||
taskId: input.task.id,
|
||||
worktreePath,
|
||||
stashSha,
|
||||
stashLabel,
|
||||
dirtyPathCount: dirtyPaths.length,
|
||||
dirtyPathSample: dirtyPaths.slice(0, 20),
|
||||
recoverCommand: `cd ${worktreePath} && git stash apply ${stashSha}`,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -2220,18 +2220,31 @@ async function stashUnrelatedRootDirChanges(
|
||||
return rescueShas.length > 0 ? { sha, label, rescueShas } : { sha, label };
|
||||
} catch (err: unknown) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
mergerLog.warn(
|
||||
`${taskId}: pre-merge autostash failed (${msg}) — proceeding without stash; concurrent dev edits in rootDir may be wiped`,
|
||||
);
|
||||
// Best-effort: try to unstage anything `git add -A` may have staged
|
||||
// before the failure, so the working tree is at least back to a sane
|
||||
// state for the merge.
|
||||
// Best-effort: unstage anything `git add -A` may have staged before the
|
||||
// failure, so the working tree is at least back to a sane state.
|
||||
try {
|
||||
await execAsync("git reset", { cwd: rootDir });
|
||||
} catch {
|
||||
// Nothing more we can do.
|
||||
}
|
||||
return null;
|
||||
// Refuse to proceed: the merge flow will issue `git reset --hard` and
|
||||
// forced checkouts that would wipe the dirty edits we just failed to
|
||||
// stash. Better to fail the merge loudly than to silently destroy work.
|
||||
mergerLog.warn(
|
||||
`${taskId}: pre-merge autostash failed (${msg}) — refusing to run destructive merge ops over a dirty tree`,
|
||||
);
|
||||
throw new AutostashCreationFailedError(msg, rootDir);
|
||||
}
|
||||
}
|
||||
|
||||
/** Thrown when pre-merge autostash cannot capture a dirty working tree.
|
||||
* The merger catches this and bails before any destructive op runs. */
|
||||
export class AutostashCreationFailedError extends Error {
|
||||
readonly rootDir: string;
|
||||
constructor(reason: string, rootDir: string) {
|
||||
super(`pre-merge autostash failed: ${reason}`);
|
||||
this.name = "AutostashCreationFailedError";
|
||||
this.rootDir = rootDir;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7390,7 +7403,29 @@ export async function aiMergeTask(
|
||||
// otherwise wipe any unrelated unstaged/untracked dev edits. Stash them
|
||||
// here, restore in the finally below — see stashUnrelatedRootDirChanges
|
||||
// for the full rationale.
|
||||
const autostashHandle = await stashUnrelatedRootDirChanges(rootDir, taskId);
|
||||
let autostashHandle: AutostashHandle | null;
|
||||
try {
|
||||
autostashHandle = await stashUnrelatedRootDirChanges(rootDir, taskId);
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof AutostashCreationFailedError) {
|
||||
// Surface to the task feed so the developer sees their edits are still
|
||||
// in the working tree (not destroyed) — we just refused to proceed.
|
||||
const message = `Merge aborted: could not autostash dirty working tree in ${rootDir} (${err.message}). Your uncommitted changes are intact. Commit, stash, or revert them and retry the merge.`;
|
||||
await store.logEntry(taskId, "Merge aborted: autostash creation failed (dirty edits preserved)", message).catch(() => undefined);
|
||||
await store.updateTask(taskId, { error: "autostash-create-failed" }).catch(() => undefined);
|
||||
clearActiveMergerStatus(activeStatusPath, taskId);
|
||||
await releaseReuseHandoffEarly("autostash-create-failed");
|
||||
return {
|
||||
task,
|
||||
branch,
|
||||
merged: false,
|
||||
worktreeRemoved: false,
|
||||
branchDeleted: false,
|
||||
error: message,
|
||||
};
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
// Surface any race-rescue stashes (mid-run dev edits caught between
|
||||
// initial snapshot and the destructive reset) on the task feed so the
|
||||
// operator sees the recovery handle without having to grep `git stash list`.
|
||||
|
||||
Reference in New Issue
Block a user