fix(engine): block dirty integration merge landings
Refuse AI merge landing on a checked-out dirty integration worktree by default. This prevents Fusion from stashing/restoring unrelated project-root edits into main and then marking tasks done against contaminated state. Fusion-Task-Id: FN-5780
This commit is contained in:
5
.changeset/dirty-integration-merge-block.md
Normal file
5
.changeset/dirty-integration-merge-block.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Block AI merge finalization when the checked-out integration worktree is dirty instead of stashing local changes into the merge landing path by default, with an explicit Merge settings UI escape hatch for the legacy dirty-checkout sync behavior.
|
||||
@@ -268,7 +268,7 @@ export const DEFAULT_PROJECT_SETTINGS = {
|
||||
],
|
||||
prerebaseDivergenceThreshold: 50,
|
||||
mergeConflictStrategy: "smart-prefer-main",
|
||||
merger: { mode: "ai", maxReviewPasses: 3 },
|
||||
merger: { mode: "ai", maxReviewPasses: 3, allowDirtyLocalCheckoutSync: false },
|
||||
mergeDiffVolumeMinLines: undefined,
|
||||
mergeDiffVolumeThreshold: undefined,
|
||||
mergeDiffVolumeAllowlist: undefined,
|
||||
|
||||
@@ -367,6 +367,12 @@ export interface MergerSettings {
|
||||
* hard-failing (blocking). Default: 3. The reviewer uses the project's
|
||||
* validator/reviewer model lane — there is no merge-specific model setting. */
|
||||
maxReviewPasses?: number;
|
||||
/** Dangerous compatibility escape hatch for the AI merge landing path.
|
||||
* When false (default), Fusion refuses to land an AI merge if the checked-out
|
||||
* integration worktree is dirty. When true, restores the legacy stash →
|
||||
* fast-forward → restore behavior for operators who explicitly accept that
|
||||
* unrelated local edits can be reintroduced after landing. */
|
||||
allowDirtyLocalCheckoutSync?: boolean;
|
||||
}
|
||||
|
||||
export const AUTO_RECOVERY_MODES = ["off", "deterministic-only", "programmatic", "ai-assisted"] as const;
|
||||
|
||||
@@ -444,6 +444,7 @@ export function SettingsModal({
|
||||
mergeStrategy: "direct",
|
||||
mergeIntegrationWorktree: "reuse-task-worktree",
|
||||
mergeAdvanceAutoSync: "stash-and-ff",
|
||||
merger: { mode: "ai", maxReviewPasses: 3, allowDirtyLocalCheckoutSync: false },
|
||||
recycleWorktrees: false,
|
||||
executorAllowSiblingBranchRename: false,
|
||||
worktreeNaming: "random",
|
||||
@@ -4629,6 +4630,31 @@ export function SettingsModal({
|
||||
/>
|
||||
<small>AI corrective rounds before landing the best result (advisory concern) or hard-failing (unfixable correctness concern). Default 3. The reviewer uses your project's reviewer/validator model.</small>
|
||||
</div>
|
||||
<div className="form-group">
|
||||
<label htmlFor="mergerAllowDirtyLocalCheckoutSync" className="checkbox-label">
|
||||
<input
|
||||
id="mergerAllowDirtyLocalCheckoutSync"
|
||||
type="checkbox"
|
||||
checked={form.merger?.allowDirtyLocalCheckoutSync === true}
|
||||
onChange={(e) =>
|
||||
setForm((f) => ({
|
||||
...f,
|
||||
merger: { ...(f.merger ?? {}), allowDirtyLocalCheckoutSync: e.target.checked },
|
||||
}))
|
||||
}
|
||||
/>
|
||||
Allow AI merge to sync a dirty checked-out integration branch
|
||||
</label>
|
||||
<details className="settings-option-details">
|
||||
<summary>More details</summary>
|
||||
<small>
|
||||
Dangerous compatibility escape hatch. Leave off unless you explicitly want the legacy
|
||||
stash → fast-forward → restore behavior when your checked-out integration branch has
|
||||
unrelated local edits. When off, AI merge blocks before advancing the branch so dirty
|
||||
project-root edits cannot contaminate a completed merge.
|
||||
</small>
|
||||
</details>
|
||||
</div>
|
||||
</>
|
||||
)}
|
||||
<div className="form-group">
|
||||
|
||||
@@ -409,18 +409,34 @@ describe("landSquash (advance + local-checkout sync)", () => {
|
||||
expect(git(dir, "rev-parse --abbrev-ref HEAD")).toBe("somewhere-else");
|
||||
});
|
||||
|
||||
it("stashes dirty edits, fast-forwards, and restores them", async () => {
|
||||
it("refuses to land onto a dirty checked-out integration branch by default", async () => {
|
||||
const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" });
|
||||
const { tipSha, squashSha } = makeDescendantSquash(dir, () => writeFileSync(join(dir, "landed.txt"), "landed\n"));
|
||||
writeFileSync(join(dir, "mydraft.txt"), "local draft\n");
|
||||
|
||||
const audit = auditStub();
|
||||
await expect(landSquash({ projectRootDir: dir, mergeRoot: dir, integrationBranch: "main", tipSha, squashSha, taskId: "FN-1", audit })).rejects.toThrow(/dirty integration checkout/i);
|
||||
expect(audit.git).toHaveBeenCalledWith(expect.objectContaining({
|
||||
type: "merge:ai-local-sync",
|
||||
metadata: expect.objectContaining({ outcome: "blocked-dirty-checkout", reason: "dirty-integration-checkout" }),
|
||||
}));
|
||||
expect(git(dir, "rev-parse main")).toBe(tipSha);
|
||||
expect(existsSync(join(dir, "landed.txt"))).toBe(false);
|
||||
expect(readFileSync(join(dir, "mydraft.txt"), "utf-8")).toContain("local draft");
|
||||
});
|
||||
|
||||
it("stashes dirty edits, fast-forwards, and restores them when explicitly allowed", async () => {
|
||||
const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" });
|
||||
const { tipSha, squashSha } = makeDescendantSquash(dir, () => writeFileSync(join(dir, "landed.txt"), "landed\n"));
|
||||
writeFileSync(join(dir, "mydraft.txt"), "local draft\n"); // dirty, non-conflicting
|
||||
|
||||
const res = await landSquash({ projectRootDir: dir, mergeRoot: dir, integrationBranch: "main", tipSha, squashSha, taskId: "FN-1", audit: auditStub() });
|
||||
const res = await landSquash({ projectRootDir: dir, mergeRoot: dir, integrationBranch: "main", tipSha, squashSha, taskId: "FN-1", audit: auditStub(), allowDirtyLocalCheckoutSync: true });
|
||||
expect(res.localSync).toBe("stash-ff-restore");
|
||||
expect(existsSync(join(dir, "landed.txt"))).toBe(true);
|
||||
expect(readFileSync(join(dir, "mydraft.txt"), "utf-8")).toContain("local draft");
|
||||
});
|
||||
|
||||
it("invokes the AI resolver when restoring the stash conflicts, then lands resolved", async () => {
|
||||
it("invokes the AI resolver when restoring the stash conflicts, then lands resolved when explicitly allowed", async () => {
|
||||
const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" });
|
||||
const { tipSha, squashSha } = makeDescendantSquash(dir, () => writeFileSync(join(dir, "base.txt"), "base\nlanded-upstream\n"));
|
||||
writeFileSync(join(dir, "base.txt"), "base\nmy-local-edit\n"); // dirty edit on the same line → restore conflict
|
||||
@@ -430,7 +446,7 @@ describe("landSquash (advance + local-checkout sync)", () => {
|
||||
execSync("git add -A", { cwd, stdio: "pipe" });
|
||||
});
|
||||
|
||||
const res = await landSquash({ projectRootDir: dir, mergeRoot: dir, integrationBranch: "main", tipSha, squashSha, taskId: "FN-1", audit: auditStub(), resolveConflicts: resolver });
|
||||
const res = await landSquash({ projectRootDir: dir, mergeRoot: dir, integrationBranch: "main", tipSha, squashSha, taskId: "FN-1", audit: auditStub(), resolveConflicts: resolver, allowDirtyLocalCheckoutSync: true });
|
||||
expect(resolver).toHaveBeenCalled();
|
||||
expect(res.localSync).toBe("stash-ff-airesolved");
|
||||
expect(git(dir, "rev-parse main")).toBe(squashSha);
|
||||
|
||||
@@ -21,9 +21,11 @@
|
||||
* common path.
|
||||
* 4. CAS fast-forward of `refs/heads/<integration>` to the squash (retry on a
|
||||
* concurrent advance by rebuilding on the new tip).
|
||||
* 5. Sync the user's local checkout to the new tip — fast-forward if clean,
|
||||
* stash → ff → restore if dirty (best-effort, never destroys uncommitted
|
||||
* work) — then finalize (delete branch, task → done, remove temp worktree).
|
||||
* 5. Sync the user's local checkout to the new tip only when it is clean by
|
||||
* default. Dirty checked-out integration worktrees fail closed before the
|
||||
* branch ref advances, preventing unrelated local changes from poisoning
|
||||
* subsequent merge runs. An explicit escape hatch can opt into the legacy
|
||||
* stash → ff → restore path.
|
||||
*
|
||||
* Pure helpers (prompt builders, verdict parser) are exported for unit testing;
|
||||
* the orchestrator accepts injectable agent functions for the same reason.
|
||||
@@ -527,6 +529,7 @@ export type LocalSyncOutcome =
|
||||
| "stash-ff-restore"
|
||||
| "stash-ff-airesolved"
|
||||
| "stash-ff-conflict"
|
||||
| "blocked-dirty-checkout"
|
||||
| "skipped-dirty-unstashable"
|
||||
| "skipped-other-branch";
|
||||
|
||||
@@ -551,10 +554,10 @@ async function hasUnresolvedConflicts(cwd: string): Promise<boolean> {
|
||||
* checkout — `git merge --ff-only <squash>` (it moves both the branch ref
|
||||
* and the working tree). The user's real dirty state is read accurately
|
||||
* BEFORE the fast-forward (while HEAD === tipSha, so `git status` isn't
|
||||
* polluted by the ref move). Dirty edits are stashed, fast-forwarded, then
|
||||
* restored — and if the restore conflicts, the AI merger reconciles them.
|
||||
* If the checkout HEAD has already moved off tipSha, that's a concurrent
|
||||
* advance → rebuild.
|
||||
* polluted by the ref move). By default a dirty checked-out integration
|
||||
* worktree is a hard blocker; callers must explicitly opt into stash/pop
|
||||
* reconciliation. If the checkout HEAD has already moved off tipSha,
|
||||
* that's a concurrent advance → rebuild.
|
||||
*
|
||||
* B. The checkout is on a different branch (or the target isn't checked out
|
||||
* here). We advance the ref atomically via `update-ref` (CAS) and leave the
|
||||
@@ -572,8 +575,15 @@ export async function landSquash(input: {
|
||||
taskId: string;
|
||||
audit: RunAuditor;
|
||||
resolveConflicts?: (cwd: string, prompt: string) => Promise<void>;
|
||||
/**
|
||||
* Explicit escape hatch for callers that truly want Fusion to stash/pop real
|
||||
* local edits in the checked-out integration worktree. The default is false:
|
||||
* automation must not land a task while also manufacturing uncommitted local
|
||||
* state in the project root, because that poisons subsequent merge runs.
|
||||
*/
|
||||
allowDirtyLocalCheckoutSync?: boolean;
|
||||
}): Promise<LandResult> {
|
||||
const { projectRootDir, mergeRoot, integrationBranch, tipSha, squashSha, taskId, audit, resolveConflicts } = input;
|
||||
const { projectRootDir, mergeRoot, integrationBranch, tipSha, squashSha, taskId, audit, resolveConflicts, allowDirtyLocalCheckoutSync = false } = input;
|
||||
const emit = (outcome: LocalSyncOutcome, extra: Record<string, unknown> = {}) =>
|
||||
audit.git({ type: "merge:ai-local-sync", target: integrationBranch, metadata: { taskId, outcome, squashSha, ...extra } }).catch(() => undefined);
|
||||
|
||||
@@ -603,6 +613,13 @@ export async function landSquash(input: {
|
||||
return { outcome: "concurrent", localSync: "skipped-other-branch" };
|
||||
}
|
||||
const dirty = (await git(["status", "--porcelain"], projectRootDir)).length > 0;
|
||||
if (dirty && !allowDirtyLocalCheckoutSync) {
|
||||
await emit("blocked-dirty-checkout", { reason: "dirty-integration-checkout" });
|
||||
throw new Error(
|
||||
`AI merge for ${taskId}: dirty integration checkout on ${integrationBranch}; refusing to land onto a dirty project root. `
|
||||
+ `Commit, stash, or clean local changes before retrying.`,
|
||||
);
|
||||
}
|
||||
const stashed = dirty
|
||||
? await gitOk(["stash", "push", "--include-untracked", "-m", `fusion-ai-merge-sync-${taskId}`], projectRootDir)
|
||||
: false;
|
||||
@@ -804,6 +821,7 @@ export async function runAiMerge(
|
||||
const landed = await landSquash({
|
||||
projectRootDir, mergeRoot, integrationBranch, tipSha, squashSha, taskId, audit,
|
||||
resolveConflicts: stashResolveAgent,
|
||||
allowDirtyLocalCheckoutSync: options.allowDirtyLocalCheckoutSync === true,
|
||||
});
|
||||
if (landed.outcome === "concurrent") {
|
||||
if (advanceRetries < MAX_CONCURRENT_ADVANCE_RETRIES) {
|
||||
|
||||
@@ -5925,6 +5925,12 @@ export interface MergerOptions {
|
||||
agentStore?: import("@fusion/core").AgentStore;
|
||||
/** Plugin runner for runtime selection. When provided, enables plugin runtime lookup. */
|
||||
pluginRunner?: import("./plugin-runner.js").PluginRunner;
|
||||
/**
|
||||
* Escape hatch for trusted callers that want the AI merge path to stash/pop
|
||||
* dirty edits in the checked-out integration worktree instead of failing
|
||||
* closed. Defaults false because project-root dirt contaminates later merges.
|
||||
*/
|
||||
allowDirtyLocalCheckoutSync?: boolean;
|
||||
}
|
||||
|
||||
function quoteArg(value: string): string {
|
||||
|
||||
@@ -1848,9 +1848,14 @@ export class ProjectEngine {
|
||||
// FN-5633: "ai" mode (default) uses the standalone AI merge path
|
||||
// (clean-room worktree + AI merge + AI reviewer); "deterministic"
|
||||
// keeps the legacy aiMergeTask pipeline.
|
||||
const mergerMode = normalizeMergerMode((await store.getSettings().catch(() => ({}) as Settings)).merger?.mode);
|
||||
const settings = await store.getSettings().catch(() => ({}) as Settings);
|
||||
const mergerMode = normalizeMergerMode(settings.merger?.mode);
|
||||
const mergeOptionsWithSettings = {
|
||||
...mergerOptions,
|
||||
allowDirtyLocalCheckoutSync: settings.merger?.allowDirtyLocalCheckoutSync === true,
|
||||
};
|
||||
return mergerMode === "ai"
|
||||
? runAiMerge(store, cwd, taskId, mergerOptions)
|
||||
? runAiMerge(store, cwd, taskId, mergeOptionsWithSettings)
|
||||
: aiMergeTask(store, cwd, taskId, mergerOptions);
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user