fix: include remaining local updates
This commit is contained in:
5
.changeset/fix-merge-build-recovery.md
Normal file
5
.changeset/fix-merge-build-recovery.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@gsxdsm/fusion": patch
|
||||
---
|
||||
|
||||
Recover auto-merge when build verification needs dependency installation, and stop endlessly requeueing exhausted merge failures.
|
||||
@@ -714,6 +714,26 @@ describe("runDashboard — auto-merge pause exclusion", () => {
|
||||
expect(aiMergeTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not auto-merge in-review tasks with exhausted merge retries", async () => {
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: true,
|
||||
pollIntervalMs: 60_000,
|
||||
});
|
||||
mockStore.listTasks.mockResolvedValue([
|
||||
{ id: "FN-EXHAUSTED", column: "in-review", paused: false, mergeRetries: 3 },
|
||||
]);
|
||||
|
||||
const { aiMergeTask } = await import("@fusion/engine");
|
||||
(aiMergeTask as ReturnType<typeof vi.fn>).mockClear();
|
||||
|
||||
await runDashboard(0, { open: false });
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
expect(aiMergeTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not auto-merge in-review tasks with incomplete steps", async () => {
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
@@ -1469,17 +1489,11 @@ describe("runDashboard — merge conflict retry logic", () => {
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// Should log max retries exceeded
|
||||
const maxRetryLog = consoleSpy.mock.calls.find(
|
||||
(call) =>
|
||||
typeof call[0] === "string" && call[0].includes("max retries (3) exceeded"),
|
||||
);
|
||||
expect(maxRetryLog).toBeDefined();
|
||||
|
||||
// Should reset mergeRetries on the task
|
||||
expect(mockStore.updateTask).toHaveBeenCalledWith(
|
||||
// Exhausted tasks are skipped before enqueue, so they should not be merged again.
|
||||
expect(aiMergeTask).not.toHaveBeenCalled();
|
||||
expect(mockStore.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-MAX",
|
||||
expect.objectContaining({ status: null }),
|
||||
expect.objectContaining({ mergeRetries: expect.anything() }),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1554,6 +1568,47 @@ describe("runDashboard — merge conflict retry logic", () => {
|
||||
expect.objectContaining({ mergeRetries: 0 }),
|
||||
);
|
||||
});
|
||||
|
||||
it("marks non-conflict merge failures as exhausted so auto-merge stops retrying", async () => {
|
||||
const { aiMergeTask } = await import("@fusion/engine");
|
||||
|
||||
(aiMergeTask as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||
new Error("Build verification failed for FN-BUILD: Dependency sync failed"),
|
||||
);
|
||||
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: true,
|
||||
autoResolveConflicts: true,
|
||||
pollIntervalMs: 60_000,
|
||||
enginePaused: false,
|
||||
globalPause: false,
|
||||
});
|
||||
|
||||
mockStore.getTask = vi.fn().mockImplementation(async (id: string) => ({
|
||||
id,
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
mergeRetries: 0,
|
||||
}));
|
||||
|
||||
mockStore.listTasks.mockResolvedValue([
|
||||
{ id: "FN-BUILD", column: "in-review", paused: false, mergeRetries: 0 },
|
||||
]);
|
||||
|
||||
await runDashboard(0, { open: false });
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
expect(mockStore.updateTask).toHaveBeenCalledWith(
|
||||
"FN-BUILD",
|
||||
expect.objectContaining({
|
||||
status: null,
|
||||
mergeRetries: 3,
|
||||
error: "Build verification failed for FN-BUILD: Dependency sync failed",
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("runDashboard — PR feedback follow-up wiring", () => {
|
||||
|
||||
@@ -349,6 +349,12 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
const mergeQueue: string[] = [];
|
||||
const mergeActive = new Set<string>(); // IDs queued or currently merging
|
||||
let mergeRunning = false;
|
||||
const maxAutoMergeRetries = 3;
|
||||
|
||||
function canAutoMergeTask(task: { mergeRetries?: number | null; column: string; paused?: boolean; status?: string | null; error?: string | null; steps?: Array<{ status: string }>; workflowStepResults?: Array<{ status: string }> }): boolean {
|
||||
if (getTaskMergeBlocker(task as any)) return false;
|
||||
return (task.mergeRetries ?? 0) < maxAutoMergeRetries;
|
||||
}
|
||||
|
||||
/** Enqueue a task for auto-merge if not already queued/active. */
|
||||
function enqueueMerge(taskId: string): void {
|
||||
@@ -378,7 +384,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
}
|
||||
// Verify the task is still in-review and not paused
|
||||
const task = await store.getTask(taskId);
|
||||
if (getTaskMergeBlocker(task)) {
|
||||
if (!canAutoMergeTask(task as any)) {
|
||||
continue;
|
||||
}
|
||||
const mergeStrategy = getMergeStrategy(settings);
|
||||
@@ -413,7 +419,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
|
||||
if (task && isConflictError) {
|
||||
const currentRetries = task.mergeRetries ?? 0;
|
||||
const maxRetries = 3;
|
||||
const maxRetries = maxAutoMergeRetries;
|
||||
|
||||
if (settings.autoResolveConflicts !== false && currentRetries < maxRetries) {
|
||||
// Increment retry counter and re-enqueue with delay
|
||||
@@ -440,14 +446,24 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
} else {
|
||||
// Non-conflict error - reset task status
|
||||
// Non-conflict error - stop auto-retrying until a user intervenes.
|
||||
// This prevents the periodic sweep from re-enqueueing the same
|
||||
// broken merge on every poll cycle.
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
await store.updateTask(taskId, {
|
||||
status: null,
|
||||
mergeRetries: maxAutoMergeRetries,
|
||||
error: errorMsg,
|
||||
});
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
} else {
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
await store.updateTask(taskId, {
|
||||
status: null,
|
||||
mergeRetries: maxAutoMergeRetries,
|
||||
error: errorMsg,
|
||||
});
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
} finally {
|
||||
@@ -463,7 +479,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
// enqueue it for serialized merge processing.
|
||||
store.on("task:moved", async ({ task, to }) => {
|
||||
if (to !== "in-review") return;
|
||||
if (getTaskMergeBlocker(task)) return;
|
||||
if (!canAutoMergeTask(task as any)) return;
|
||||
try {
|
||||
const settings = await store.getSettings();
|
||||
if (settings.globalPause || settings.enginePaused) return;
|
||||
@@ -678,7 +694,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
// ── Startup sweep: enqueue any tasks already in "in-review" ───────
|
||||
if (settings.autoMerge) {
|
||||
const existing = await store.listTasks();
|
||||
const inReview = existing.filter((t) => !getTaskMergeBlocker(t));
|
||||
const inReview = existing.filter((t) => canAutoMergeTask(t as any));
|
||||
if (inReview.length > 0) {
|
||||
console.log(
|
||||
`[auto-merge] Startup sweep: enqueueing ${inReview.length} in-review task(s)`,
|
||||
@@ -707,7 +723,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
try {
|
||||
const tasks = await store.listTasks();
|
||||
for (const t of tasks) {
|
||||
if (!getTaskMergeBlocker(t)) {
|
||||
if (canAutoMergeTask(t as any)) {
|
||||
enqueueMerge(t.id);
|
||||
}
|
||||
}
|
||||
@@ -766,7 +782,7 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
|
||||
if (!s.globalPause && !s.enginePaused && s.autoMerge) {
|
||||
const tasks = await store.listTasks();
|
||||
for (const t of tasks) {
|
||||
if (!getTaskMergeBlocker(t)) {
|
||||
if (canAutoMergeTask(t as any)) {
|
||||
enqueueMerge(t.id);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -42,6 +42,7 @@ import {
|
||||
parseDiffStat,
|
||||
extractFileScope,
|
||||
validateDiffScope,
|
||||
shouldSyncDependenciesForMerge,
|
||||
type ConflictCategory,
|
||||
} from "./merger.js";
|
||||
import { createKbAgent } from "./pi.js";
|
||||
@@ -1934,6 +1935,81 @@ describe("aiMergeTask — build verification", () => {
|
||||
expect(result.merged).toBe(true);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
|
||||
});
|
||||
|
||||
it("syncs dependencies before build verification when install state is missing", async () => {
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
mockedExistsSync.mockImplementation((path: any) => {
|
||||
const pathStr = String(path);
|
||||
if (pathStr.includes("node_modules") || pathStr.endsWith(".pnp.cjs")) return false;
|
||||
return true;
|
||||
});
|
||||
|
||||
let cachedQuietChecks = 0;
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "2 files changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --name-only --diff-filter=U")) return "" as any;
|
||||
if (cmdStr.includes("git diff --cached --name-only")) {
|
||||
return "package.json\npackages/desktop/package.json" as any;
|
||||
}
|
||||
if (cmdStr.includes("pnpm install --frozen-lockfile")) return "Lockfile is up to date" as any;
|
||||
if (cmdStr.includes("diff --cached --quiet")) {
|
||||
cachedQuietChecks += 1;
|
||||
return cachedQuietChecks === 1 ? "1" as any : "0" as any;
|
||||
}
|
||||
if (cmdStr.includes("show --shortstat")) return "3 files changed, 10 insertions(+), 2 deletions(-)" as any;
|
||||
if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from("");
|
||||
if (cmdStr.includes("worktree remove")) return Buffer.from("");
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
const store = createMockStore(
|
||||
{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
|
||||
[{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
|
||||
);
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...DEFAULT_SETTINGS,
|
||||
buildCommand: "pnpm build",
|
||||
});
|
||||
|
||||
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
|
||||
|
||||
expect(result.merged).toBe(true);
|
||||
const installCall = mockedExecSync.mock.calls.find(
|
||||
(call) => String(call[0]).includes("pnpm install --frozen-lockfile"),
|
||||
);
|
||||
expect(installCall).toBeDefined();
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
"Syncing dependencies before merge build verification: pnpm install --frozen-lockfile",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldSyncDependenciesForMerge", () => {
|
||||
it("returns true when install state is missing", () => {
|
||||
expect(shouldSyncDependenciesForMerge([], false)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns true when staged files change package manifests or lockfiles", () => {
|
||||
expect(shouldSyncDependenciesForMerge(["packages/desktop/package.json"], true)).toBe(true);
|
||||
expect(shouldSyncDependenciesForMerge(["pnpm-lock.yaml"], true)).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for regular source-only changes when install state exists", () => {
|
||||
expect(shouldSyncDependenciesForMerge(["packages/engine/src/merger.ts"], true)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Pre-merge diffstat scope validation tests ────────────────────────
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { execSync } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { getTaskMergeBlocker, type TaskStore, type MergeResult, type MergeDetails, type WorkflowStep, type WorkflowStepResult, type Settings, type AgentPromptsConfig } from "@fusion/core";
|
||||
import { resolveAgentPrompt } from "@fusion/core";
|
||||
import { createKbAgent, describeModel, promptWithFallback } from "./pi.js";
|
||||
@@ -49,6 +50,17 @@ export const GENERATED_PATTERNS = [
|
||||
"generated/*",
|
||||
];
|
||||
|
||||
const DEPENDENCY_SYNC_TRIGGER_PATTERNS = [
|
||||
"package.json",
|
||||
"pnpm-lock.yaml",
|
||||
"pnpm-workspace.yaml",
|
||||
"package-lock.json",
|
||||
"yarn.lock",
|
||||
"bun.lockb",
|
||||
"bun.lock",
|
||||
"packages/*/package.json",
|
||||
];
|
||||
|
||||
/** Check if a path matches a glob pattern (simple glob support: * and **) */
|
||||
function matchGlob(path: string, pattern: string): boolean {
|
||||
// Handle ** which matches across directory boundaries (must do before single *)
|
||||
@@ -94,6 +106,65 @@ function matchGlob(path: string, pattern: string): boolean {
|
||||
return regex.test(fileName) || regex.test(path);
|
||||
}
|
||||
|
||||
export function getStagedFiles(cwd: string): string[] {
|
||||
try {
|
||||
const output = execSync("git diff --cached --name-only", {
|
||||
cwd,
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
}).trim();
|
||||
return output ? output.split("\n").filter(Boolean) : [];
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
export function hasInstallState(rootDir: string): boolean {
|
||||
return existsSync(join(rootDir, "node_modules")) || existsSync(join(rootDir, ".pnp.cjs"));
|
||||
}
|
||||
|
||||
export function shouldSyncDependenciesForMerge(
|
||||
stagedFiles: string[],
|
||||
installStatePresent: boolean,
|
||||
): boolean {
|
||||
if (!installStatePresent) return true;
|
||||
return stagedFiles.some((file) =>
|
||||
DEPENDENCY_SYNC_TRIGGER_PATTERNS.some((pattern) => matchGlob(file, pattern)),
|
||||
);
|
||||
}
|
||||
|
||||
function getDependencySyncCommand(rootDir: string): string | null {
|
||||
if (existsSync(join(rootDir, "pnpm-lock.yaml"))) return "pnpm install --frozen-lockfile";
|
||||
if (existsSync(join(rootDir, "package-lock.json"))) return "npm install";
|
||||
if (existsSync(join(rootDir, "yarn.lock"))) return "yarn install --frozen-lockfile";
|
||||
if (existsSync(join(rootDir, "bun.lock")) || existsSync(join(rootDir, "bun.lockb"))) {
|
||||
return "bun install --frozen-lockfile";
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
async function syncDependenciesForMerge(
|
||||
store: TaskStore,
|
||||
rootDir: string,
|
||||
taskId: string,
|
||||
): Promise<void> {
|
||||
const installCommand = getDependencySyncCommand(rootDir);
|
||||
if (!installCommand) return;
|
||||
|
||||
mergerLog.log(`${taskId}: syncing dependencies before merge build verification`);
|
||||
await store.logEntry(taskId, `Syncing dependencies before merge build verification: ${installCommand}`);
|
||||
try {
|
||||
execSync(installCommand, {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch (error: any) {
|
||||
const details = error?.stderr || error?.stdout || error?.message || String(error);
|
||||
throw new Error(`Dependency sync failed for ${taskId}: ${details}`.trim());
|
||||
}
|
||||
}
|
||||
|
||||
// ── Pre-merge diffstat scope validation ──────────────────────────────
|
||||
|
||||
interface DiffFileEntry {
|
||||
@@ -1152,6 +1223,13 @@ async function executeMergeAttempt(
|
||||
}
|
||||
}
|
||||
|
||||
if (buildCommand) {
|
||||
const stagedFiles = getStagedFiles(rootDir);
|
||||
if (shouldSyncDependenciesForMerge(stagedFiles, hasInstallState(rootDir))) {
|
||||
await syncDependenciesForMerge(store, rootDir, taskId);
|
||||
}
|
||||
}
|
||||
|
||||
// At this point, either:
|
||||
// - No conflicts (attempt 1) - AI writes commit message
|
||||
// - Complex conflicts remain after attempt 2 auto-resolution - AI resolves them
|
||||
|
||||
Reference in New Issue
Block a user