feat(FN-1632): merge fusion/fn-1632

This commit is contained in:
gsxdsm
2026-04-12 15:28:10 -07:00
parent 9b20a99bae
commit 59edac4ce2
5 changed files with 499 additions and 20 deletions

View File

@@ -80,6 +80,7 @@ import {
validateDiffScope,
shouldSyncDependenciesForMerge,
summarizeVerificationOutput,
inferDefaultTestCommand,
type ConflictCategory,
} from "./merger.js";
import { createKbAgent } from "./pi.js";
@@ -3634,3 +3635,360 @@ describe("summarizeVerificationOutput", () => {
expect(bulletMatches?.length).toBe(2);
});
});
// ── Default Test Command Inference Tests ──────────────────────────────────
describe("inferDefaultTestCommand", () => {
beforeEach(() => {
vi.clearAllMocks();
// Default: no lock files present
mockedExistsSync.mockReturnValue(false);
});
it("returns null when no package manager lock files exist", () => {
mockedExistsSync.mockReturnValue(false);
const result = inferDefaultTestCommand("/tmp/root");
expect(result).toBeNull();
});
it("returns pnpm test for pnpm-lock.yaml", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
const result = inferDefaultTestCommand("/tmp/root");
expect(result).toEqual({
command: "pnpm test",
testSource: "inferred",
});
});
it("returns npm test for package-lock.json", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("package-lock.json");
});
const result = inferDefaultTestCommand("/tmp/root");
expect(result).toEqual({
command: "npm test",
testSource: "inferred",
});
});
it("returns yarn test for yarn.lock", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("yarn.lock");
});
const result = inferDefaultTestCommand("/tmp/root");
expect(result).toEqual({
command: "yarn test",
testSource: "inferred",
});
});
it("returns bun test for bun.lock", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("bun.lock");
});
const result = inferDefaultTestCommand("/tmp/root");
expect(result).toEqual({
command: "bun test",
testSource: "inferred",
});
});
it("returns bun test for bun.lockb", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("bun.lockb");
});
const result = inferDefaultTestCommand("/tmp/root");
expect(result).toEqual({
command: "bun test",
testSource: "inferred",
});
});
it("prefers pnpm over npm when both exist", () => {
mockedExistsSync.mockImplementation((path: any) => {
const pathStr = String(path);
return pathStr.includes("pnpm-lock.yaml") || pathStr.includes("package-lock.json");
});
const result = inferDefaultTestCommand("/tmp/root");
expect(result?.command).toBe("pnpm test");
});
it("uses explicit testCommand when provided", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
const result = inferDefaultTestCommand("/tmp/root", "vitest run", "pnpm build");
expect(result).toEqual({
command: "vitest run",
testSource: "explicit",
buildSource: "explicit",
});
});
it("ignores empty string explicit testCommand", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
const result = inferDefaultTestCommand("/tmp/root", "", "pnpm build");
expect(result?.command).toBe("pnpm test");
expect(result?.testSource).toBe("inferred");
expect(result?.buildSource).toBe("explicit");
});
it("ignores whitespace-only explicit testCommand", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
const result = inferDefaultTestCommand("/tmp/root", " ", "pnpm build");
expect(result?.command).toBe("pnpm test");
expect(result?.testSource).toBe("inferred");
});
it("returns build source even when test is inferred", () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
const result = inferDefaultTestCommand("/tmp/root", undefined, "pnpm build");
expect(result).toEqual({
command: "pnpm test",
testSource: "inferred",
buildSource: "explicit",
});
});
});
// ── Inferred Test Command Merge Behavior ─────────────────────────────────
describe("aiMergeTask — inferred test command execution", () => {
beforeEach(() => {
vi.clearAllMocks();
mockedExistsSync.mockReturnValue(true);
setupHappyPathExecSync();
mockedCreateHaiAgent.mockResolvedValue({
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
},
} as any);
});
it("runs inferred test command when settings.testCommand is not configured", async () => {
// pnpm-lock.yaml exists, testCommand is not set
mockedExistsSync.mockImplementation((path: any) => {
const pathStr = String(path);
if (pathStr.includes("pnpm-lock.yaml")) return true;
return true; // other files exist for worktree check
});
const verificationCalls: string[] = [];
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("pnpm test")) {
verificationCalls.push("pnpm test");
return Buffer.from("");
}
// Handle all other git commands - matching setupHappyPathExecSync
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 "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
if (cmdStr.includes("diff --cached") && !cmdStr.includes("--quiet")) return "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],
);
// testCommand is not set (undefined in DEFAULT_SETTINGS)
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
...DEFAULT_SETTINGS,
});
await aiMergeTask(store, "/tmp/root", "FN-050");
expect(verificationCalls).toContain("pnpm test");
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
});
it("logs that test command was inferred from project files", async () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
// Setup happy path
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("pnpm test")) return Buffer.from("");
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
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 "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
if (cmdStr.includes("diff --cached") && !cmdStr.includes("--quiet")) return "" 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,
});
await aiMergeTask(store, "/tmp/root", "FN-050");
// Verify log entries include verification with test command mentioned
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
const verificationLogCall = logCalls.find((call: any[]) =>
typeof call[1] === "string" && call[1].includes("pnpm test")
);
expect(verificationLogCall).toBeTruthy();
});
it("failing inferred test command blocks merge and keeps task out of done", async () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("pnpm test")) {
// Simulate test failure
const error = new Error("Test failed") as any;
error.status = 1;
error.stdout = "FAIL: test failed";
error.stderr = "";
throw error;
}
// Handle other commands normally
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
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 "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
if (cmdStr.includes("diff --cached") && !cmdStr.includes("--quiet")) return "" 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,
});
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toThrow(
"Deterministic test verification failed",
);
// Task should NOT be moved to done
expect(store.moveTask).not.toHaveBeenCalledWith("FN-050", "done");
// Log entry should indicate failure
expect(store.logEntry).toHaveBeenCalledWith(
"FN-050",
expect.stringContaining("test verification failed"),
"VerificationError",
);
});
it("explicit settings.testCommand takes precedence over inferred command", async () => {
mockedExistsSync.mockImplementation((path: any) => {
return String(path).includes("pnpm-lock.yaml");
});
const verificationCalls: string[] = [];
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("vitest run")) {
verificationCalls.push("vitest run");
return Buffer.from("");
}
if (cmdStr.includes("pnpm test")) {
verificationCalls.push("pnpm test - SHOULD NOT BE CALLED");
return Buffer.from("");
}
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
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 "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
if (cmdStr.includes("diff --cached") && !cmdStr.includes("--quiet")) return "" 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],
);
// Explicit testCommand is set
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
...DEFAULT_SETTINGS,
testCommand: "vitest run",
});
await aiMergeTask(store, "/tmp/root", "FN-050");
// Explicit command should be used, not inferred
expect(verificationCalls).toContain("vitest run");
expect(verificationCalls).not.toContain("pnpm test - SHOULD NOT BE CALLED");
});
it("skips verification when no lock files exist and no explicit testCommand is set", async () => {
// No lock files exist
mockedExistsSync.mockReturnValue(false);
const verificationCalls: string[] = [];
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("pnpm test") || cmdStr.includes("npm test") || cmdStr.includes("yarn test") || cmdStr.includes("bun test")) {
verificationCalls.push(cmdStr);
}
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
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 "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
if (cmdStr.includes("diff --cached") && !cmdStr.includes("--quiet")) return "" 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,
});
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
// No test verification should have run
expect(verificationCalls).toHaveLength(0);
// Merge should still succeed
expect(result.merged).toBe(true);
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
});
});

View File

@@ -354,6 +354,79 @@ async function syncDependenciesForMerge(
}
}
// ── Default test command inference ────────────────────────────────────
/** Result of inferring a default test command */
interface InferredTestCommand {
command: string;
/** Source indicates whether this was explicitly configured or inferred from project files */
testSource: "explicit" | "inferred";
buildSource?: "explicit" | "inferred";
}
/**
* Infer a default test command based on project files.
* Returns the command and whether it was explicitly configured or inferred.
*
* Inference rules:
* - pnpm-lock.yaml → "pnpm test"
* - yarn.lock → "yarn test"
* - bun.lock/bun.lockb → "bun test"
* - package-lock.json → "npm test"
*
* Returns null if no test command can be inferred.
*/
export function inferDefaultTestCommand(
rootDir: string,
explicitTestCommand?: string,
explicitBuildCommand?: string,
): InferredTestCommand | null {
// If explicit test command is set, use it (no inference needed)
if (explicitTestCommand?.trim()) {
return {
command: explicitTestCommand.trim(),
testSource: "explicit",
buildSource: explicitBuildCommand?.trim() ? "explicit" : undefined,
};
}
// Infer test command from lock files
if (existsSync(join(rootDir, "pnpm-lock.yaml"))) {
return {
command: "pnpm test",
testSource: "inferred",
buildSource: explicitBuildCommand?.trim() ? "explicit" : undefined,
};
}
if (existsSync(join(rootDir, "yarn.lock"))) {
return {
command: "yarn test",
testSource: "inferred",
buildSource: explicitBuildCommand?.trim() ? "explicit" : undefined,
};
}
if (existsSync(join(rootDir, "bun.lock")) || existsSync(join(rootDir, "bun.lockb"))) {
return {
command: "bun test",
testSource: "inferred",
buildSource: explicitBuildCommand?.trim() ? "explicit" : undefined,
};
}
if (existsSync(join(rootDir, "package-lock.json"))) {
return {
command: "npm test",
testSource: "inferred",
buildSource: explicitBuildCommand?.trim() ? "explicit" : undefined,
};
}
// No inference possible — return null, letting the caller decide what to do
return null;
}
// ── Deterministic merge verification ──────────────────────────────────
/** Result of running a single verification command */
@@ -395,6 +468,8 @@ async function runDeterministicVerification(
taskId: string,
testCommand?: string,
buildCommand?: string,
testSource?: "explicit" | "inferred",
buildSource?: "explicit" | "inferred",
): Promise<VerificationResult> {
const result: VerificationResult = { allPassed: true };
@@ -409,16 +484,20 @@ async function runDeterministicVerification(
const hasTestCommand = !!normalizedTestCommand;
const hasBuildCommand = !!normalizedBuildCommand;
// Build source indicator for logging
const testSourceLabel = testSource === "inferred" ? " [inferred]" : "";
const buildSourceLabel = buildSource === "inferred" ? " [inferred]" : "";
mergerLog.log(
`${taskId}: running deterministic verification` +
(hasTestCommand ? ` [test: ${normalizedTestCommand}]` : "") +
(hasBuildCommand ? ` [build: ${normalizedBuildCommand}]` : ""),
(hasTestCommand ? ` [test:${testSourceLabel} ${normalizedTestCommand}]` : "") +
(hasBuildCommand ? ` [build:${buildSourceLabel} ${normalizedBuildCommand}]` : ""),
);
await store.logEntry(
taskId,
"Running deterministic merge verification" +
(hasTestCommand ? ` (testCommand: ${normalizedTestCommand})` : "") +
(hasBuildCommand ? ` (buildCommand: ${normalizedBuildCommand})` : ""),
(hasTestCommand ? ` (test${testSource === "inferred" ? " [inferred]" : ""}: ${normalizedTestCommand})` : "") +
(hasBuildCommand ? ` (build${buildSource === "inferred" ? " [inferred]" : ""}: ${normalizedBuildCommand})` : ""),
);
// Run test command first if configured
@@ -1221,13 +1300,30 @@ export async function aiMergeTask(
// 5. Execute merge with retry logic
await store.updateTask(taskId, { status: "merging" });
// Normalize explicit verification commands from settings
const explicitTestCommand = settings.testCommand?.trim() || undefined;
const explicitBuildCommand = settings.buildCommand?.trim() || undefined;
// Infer default test command if explicit testCommand is not set
// This ensures merge verification runs even when settings.testCommand is not configured
const inferredTest = inferDefaultTestCommand(rootDir, explicitTestCommand, explicitBuildCommand);
const effectiveTestCommand = inferredTest?.command || explicitTestCommand;
const effectiveTestSource = inferredTest?.testSource;
const effectiveBuildCommand = explicitBuildCommand;
const effectiveBuildSource = inferredTest?.buildSource;
// Log what verification commands will be used
if (effectiveTestCommand || effectiveBuildCommand) {
mergerLog.log(
`${taskId}: merge verification commands` +
(effectiveTestCommand ? ` [test: ${effectiveTestCommand} (${effectiveTestSource || "explicit"})]` : "") +
(effectiveBuildCommand ? ` [build: ${effectiveBuildCommand} (${effectiveBuildSource || "explicit"})]` : ""),
);
}
const mergeAttempt = async (attemptNum: 1 | 2 | 3): Promise<boolean> => {
mergerLog.log(`${taskId}: merge attempt ${attemptNum}/3...`);
// Normalize verification commands: treat empty string as undefined
const testCommand = settings.testCommand?.trim() || undefined;
const buildCommand = settings.buildCommand?.trim() || undefined;
try {
// Try the merge with appropriate strategy for this attempt
const success = await executeMergeAttempt({
@@ -1242,8 +1338,10 @@ export async function aiMergeTask(
attemptNum,
options,
result,
testCommand,
buildCommand,
testCommand: effectiveTestCommand,
buildCommand: effectiveBuildCommand,
testSource: effectiveTestSource,
buildSource: effectiveBuildSource,
}, aiTracker);
if (success) {
@@ -1487,6 +1585,10 @@ interface MergeAttemptParams {
result: MergeResult;
testCommand?: string;
buildCommand?: string;
/** Source of the test command: 'explicit' from settings or 'inferred' from project files */
testSource?: "explicit" | "inferred";
/** Source of the build command: 'explicit' from settings or 'inferred' (future use) */
buildSource?: "explicit" | "inferred";
}
/** Mutable flag to track AI agent invocation */
@@ -1517,6 +1619,8 @@ async function executeMergeAttempt(
result,
testCommand,
buildCommand,
testSource,
buildSource,
} = params;
// Attempt 3: Use -X theirs strategy
@@ -1601,7 +1705,7 @@ async function executeMergeAttempt(
}
// Run deterministic verification before completing the merge
if (testCommand || buildCommand) {
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand);
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand, testSource, buildSource);
}
return true;
}
@@ -1619,7 +1723,7 @@ async function executeMergeAttempt(
mergerLog.log(`${taskId}: squash merge staged nothing — already merged`);
// Run deterministic verification (nothing staged but still verify)
if (testCommand || buildCommand) {
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand);
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand, testSource, buildSource);
}
return true;
}
@@ -1641,7 +1745,7 @@ async function executeMergeAttempt(
mergerLog.log(`${taskId}: squash merge staged nothing — already merged`);
// Run deterministic verification (nothing staged but still verify)
if (testCommand || buildCommand) {
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand);
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand, testSource, buildSource);
}
return true;
}
@@ -1709,7 +1813,7 @@ async function executeMergeAttempt(
// Run deterministic verification after AI agent commits
if (testCommand || buildCommand) {
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand);
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand, testSource, buildSource);
}
return true;
@@ -1738,7 +1842,7 @@ async function executeMergeAttempt(
* Attempt 3: Use git merge -X theirs --squash strategy
*/
async function attemptWithTheirsStrategy(params: MergeAttemptParams): Promise<boolean> {
const { rootDir, branch, commitLog, includeTaskId, taskId, store, testCommand, buildCommand } = params;
const { rootDir, branch, commitLog, includeTaskId, taskId, store, testCommand, buildCommand, testSource, buildSource } = params;
mergerLog.log(`${taskId}: attempting merge with -X theirs strategy`);
@@ -1769,7 +1873,7 @@ async function attemptWithTheirsStrategy(params: MergeAttemptParams): Promise<bo
// Nothing staged - already merged
// Run deterministic verification even when nothing is staged
if (testCommand || buildCommand) {
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand);
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand, testSource, buildSource);
}
return true;
}
@@ -1785,7 +1889,7 @@ async function attemptWithTheirsStrategy(params: MergeAttemptParams): Promise<bo
// Run deterministic verification after committing
if (testCommand || buildCommand) {
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand);
await runDeterministicVerification(store, rootDir, taskId, testCommand, buildCommand, testSource, buildSource);
}
return true;