feat(FN-670): add mandatory build verification before merge

- Add report_build_failure tool to merger agent for explicit build failure signaling
- Run buildCommand before finalizing merge, abort if build fails with git reset --merge
- Treat build failures as fatal errors (no retry) and keep task in in-review
- Add build verification instruction to executor system prompt
- Update executor to use steeringComments field for mid-execution guidance
This commit is contained in:
gsxdsm
2026-04-01 07:28:28 -07:00
parent 31fbe031e7
commit c1ca8a4316
7 changed files with 370 additions and 18 deletions

View File

@@ -24,7 +24,8 @@
"dependencies": {
"@fusion/core": "workspace:*",
"@mariozechner/pi-ai": "^0.62.0",
"@mariozechner/pi-coding-agent": "^0.62.0"
"@mariozechner/pi-coding-agent": "^0.62.0",
"@sinclair/typebox": "^0.34.48"
},
"devDependencies": {
"@modelcontextprotocol/sdk": "^1.0.0",

View File

@@ -1617,7 +1617,7 @@ describe("buildExecutionPrompt", () => {
it("includes Comments section when comments has entries", () => {
const task = createMockTaskDetail({
comments: [
steeringComments: [
{
id: "1",
text: "Please handle the edge case",
@@ -1637,7 +1637,7 @@ describe("buildExecutionPrompt", () => {
it("formats multiple comments correctly", () => {
const now = new Date();
const task = createMockTaskDetail({
comments: [
steeringComments: [
{
id: "1",
text: "First comment",
@@ -1660,8 +1660,8 @@ describe("buildExecutionPrompt", () => {
expect(result).toContain("> Second comment");
});
it("omits Comments section when comments is empty", () => {
const task = createMockTaskDetail({ comments: [] });
it("omits Comments section when steeringComments is empty", () => {
const task = createMockTaskDetail({ steeringComments: [] });
const result = buildExecutionPrompt(task);
expect(result).not.toContain("## Steering Comments");

View File

@@ -1870,7 +1870,9 @@ Use \`task_update\` to report progress on every step transition.
Use \`task_log\` for important actions and decisions.
Use \`task_create\` if you find out-of-scope work that needs doing.
Commit at step boundaries: \`git commit -m "feat(${task.id}): complete Step N — description"\`
When all steps are complete: call \`task_done()\``;
When all steps are complete: call \`task_done()\`
Verify build passes using the configured build command before calling \`task_done()\`.`;
}
/**

View File

@@ -1665,3 +1665,232 @@ describe("isTrivialWhitespaceConflict", () => {
expect(cmdStr).toContain(':3:"src/utils.ts"');
});
});
// ── Build Verification Tests ─────────────────────────────────────────
describe("aiMergeTask — build verification", () => {
beforeEach(() => {
vi.clearAllMocks();
mockedExistsSync.mockReturnValue(true);
// Default happy path exec mock
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
if (cmdStr.includes("git log")) return "- feat: something" as any;
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("");
if (cmdStr.includes("reset --merge")) return Buffer.from("");
return Buffer.from("");
});
});
it("system prompt contains build verification section", async () => {
let capturedSystemPrompt: string | undefined;
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
capturedSystemPrompt = opts.systemPrompt;
return {
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
},
} as any;
});
const store = createMockStore();
await aiMergeTask(store, "/tmp/root", "FN-050");
expect(capturedSystemPrompt).toContain("## Build verification");
expect(capturedSystemPrompt).toContain("If a build command is configured for this project, you MUST run it");
expect(capturedSystemPrompt).toContain("BUILD FAILED:");
});
it("includes build command in merge prompt when configured", async () => {
let capturedArgs: any;
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
capturedArgs = opts;
// Simulate agent committing by returning session that results in clean state
return {
session: {
prompt: vi.fn().mockImplementation(async () => {
// Simulate commit happening by making staged check return "0" (clean)
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
if (cmdStr.includes("git log")) return "- feat: something" as any;
if (cmdStr.includes("--stat")) return "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
// After commit, diff shows clean
if (cmdStr.includes("diff --cached --quiet")) return "0" as any;
if (cmdStr.includes("branch -d")) return Buffer.from("");
if (cmdStr.includes("worktree remove")) return Buffer.from("");
return Buffer.from("");
});
}),
dispose: vi.fn(),
},
} as any;
});
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",
});
await aiMergeTask(store, "/tmp/root", "FN-050");
// Verify custom tool was passed
expect(capturedArgs.customTools).toBeDefined();
expect(capturedArgs.customTools.some((t: any) => t.name === "report_build_failure")).toBe(true);
});
it("merge succeeds when build passes (agent reports success)", async () => {
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
return {
session: {
prompt: vi.fn().mockImplementation(async () => {
// Simulate commit happening by making staged check return "0" (clean)
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
if (cmdStr.includes("git log")) return "- feat: something" as any;
if (cmdStr.includes("--stat")) return "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
// After commit, diff shows clean
if (cmdStr.includes("diff --cached --quiet")) return "0" as any;
if (cmdStr.includes("branch -d")) return Buffer.from("");
if (cmdStr.includes("worktree remove")) return Buffer.from("");
return Buffer.from("");
});
}),
dispose: vi.fn(),
},
} as any;
});
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);
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
});
it("merge aborts when build fails via report_build_failure tool", async () => {
// Mock agent that calls the report_build_failure tool execute method
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
const reportTool = opts.customTools?.find((t: any) => t.name === "report_build_failure");
return {
session: {
prompt: vi.fn().mockImplementation(async () => {
// Simulate the agent calling the tool when session.prompt() is called
if (reportTool) {
await reportTool.execute("tool-call-123", { message: "Type error in src/utils.ts" });
}
}),
dispose: vi.fn(),
},
} as any;
});
const resetCalls: string[] = [];
mockedExecSync.mockImplementation((cmd: any) => {
const cmdStr = String(cmd);
if (cmdStr.includes("reset --merge")) {
resetCalls.push(cmdStr);
return Buffer.from("");
}
// Default happy path for other commands
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
if (cmdStr.includes("git log")) return "- feat: something" as any;
if (cmdStr.includes("--stat")) return "1 file changed" as any;
if (cmdStr.includes("merge --squash")) return Buffer.from("");
// Staged changes present (agent didn't commit due to build failure)
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
if (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",
});
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toThrow(
"Build verification failed for FN-050: Type error in src/utils.ts",
);
// Verify git reset --merge was called
expect(resetCalls.length).toBeGreaterThan(0);
// Verify task was NOT moved to done
expect(store.moveTask).not.toHaveBeenCalled();
// Verify log entry was made
expect(store.logEntry).toHaveBeenCalledWith(
"FN-050",
"Build verification failed during merge",
"Type error in src/utils.ts",
);
});
it("merge proceeds normally when no build command is configured", async () => {
mockedCreateHaiAgent.mockResolvedValue({
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
},
} as any);
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],
);
// buildCommand is undefined by default in DEFAULT_SETTINGS
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
expect(result.merged).toBe(true);
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
});
it("merge proceeds when buildCommand is empty string (treated as undefined)", async () => {
mockedCreateHaiAgent.mockResolvedValue({
session: {
prompt: vi.fn().mockResolvedValue(undefined),
dispose: vi.fn(),
},
} as any);
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: " ", // whitespace-only, should be treated as undefined
});
const result = await aiMergeTask(store, "/tmp/root", "FN-050");
expect(result.merged).toBe(true);
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
});
});

View File

@@ -6,6 +6,8 @@ import type { WorktreePool } from "./worktree-pool.js";
import { AgentLogger } from "./agent-logger.js";
import { mergerLog } from "./logger.js";
import { isUsageLimitError, checkSessionError, type UsageLimitPauser } from "./usage-limit-detector.js";
import type { ToolDefinition } from "@mariozechner/pi-coding-agent";
import { Type } from "@sinclair/typebox";
/** Conflict type classification for merge conflict resolution */
export type ConflictType =
@@ -471,7 +473,19 @@ Look at the branch commits and diff to understand what was done, then run:
${commitFormat}
Do NOT use generic messages like "merge branch" or "resolve conflicts".
Base the message on the ACTUAL work done in the branch commits.`;
Base the message on the ACTUAL work done in the branch commits.
## Build verification
If a build command is configured for this project, you MUST run it before committing.
1. Run the build command (shown in the prompt context below)
2. If the build succeeds (exit code 0), proceed with the commit
3. If the build fails (non-zero exit code), DO NOT commit. Instead:
- Respond with "BUILD FAILED: <error details>"
- Stop and do not proceed further
The merge will only be completed if the build passes or no build command is configured.`;
}
/**
@@ -596,6 +610,9 @@ export async function aiMergeTask(
const mergeAttempt = async (attemptNum: 1 | 2 | 3): Promise<boolean> => {
mergerLog.log(`${taskId}: merge attempt ${attemptNum}/3...`);
// Normalize buildCommand: treat empty string as undefined
const buildCommand = settings.buildCommand?.trim() || undefined;
try {
// Try the merge with appropriate strategy for this attempt
const success = await executeMergeAttempt({
@@ -610,6 +627,7 @@ export async function aiMergeTask(
attemptNum,
options,
result,
buildCommand,
}, aiTracker);
if (success) {
@@ -630,6 +648,11 @@ export async function aiMergeTask(
return false;
} catch (error: any) {
// Check if it's a build verification failure - don't retry, propagate immediately
if (error.message?.includes("Build verification failed")) {
throw error; // Fatal - don't retry build failures
}
// Clean up on error before potentially rethrowing or retrying
if (attemptNum < 3 && smartConflictResolution) {
mergerLog.log(`${taskId}: attempt ${attemptNum} error, cleaning up for retry...`);
@@ -751,6 +774,7 @@ interface MergeAttemptParams {
attemptNum: 1 | 2 | 3;
options: MergerOptions;
result: MergeResult;
buildCommand?: string;
}
/** Mutable flag to track AI agent invocation */
@@ -779,6 +803,7 @@ async function executeMergeAttempt(
attemptNum,
options,
result,
buildCommand,
} = params;
// Attempt 3: Use -X theirs strategy
@@ -920,7 +945,7 @@ async function executeMergeAttempt(
// - Complex conflicts remain after attempt 2 auto-resolution - AI resolves them
// Spawn AI agent
aiTracker.aiWasInvoked = true; // Track that AI was invoked
return await runAiAgentForCommit({
const agentResult = await runAiAgentForCommit({
store,
rootDir,
taskId,
@@ -931,8 +956,32 @@ async function executeMergeAttempt(
hasConflicts,
simplifiedContext: attemptNum === 2,
options,
buildCommand,
});
// Handle build failure
if (!agentResult.success) {
// Build verification failed - log, reset staged changes, and throw
const errorMessage = agentResult.error || "Build verification failed";
await store.logEntry(taskId, "Build verification failed during merge", errorMessage);
// Reset staged changes to abort the merge
try {
execSync("git reset --merge", { cwd: rootDir, stdio: "pipe" });
} catch {
// Ignore reset errors
}
throw new Error(`Build verification failed for ${taskId}: ${errorMessage}`);
}
return true;
} catch (error: any) {
// Check if it's a build verification failure - don't retry, propagate immediately
if (error.message?.includes("Build verification failed")) {
throw error; // Fatal - don't retry build failures
}
// Check if it's a non-conflict merge failure
if (error.message?.includes("Merge failed")) {
throw error; // Fatal
@@ -1011,12 +1060,15 @@ interface AiAgentParams {
hasConflicts: boolean;
simplifiedContext: boolean;
options: MergerOptions;
buildCommand?: string;
}
/**
* Run the AI agent to resolve conflicts and/or write commit message.
* Returns { success: true } on success, { success: false, error: string } on build failure.
* Throws on agent errors or unrecoverable failures.
*/
async function runAiAgentForCommit(params: AiAgentParams): Promise<boolean> {
async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: boolean; error?: string }> {
const {
store,
rootDir,
@@ -1028,10 +1080,34 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<boolean> {
hasConflicts,
simplifiedContext,
options,
buildCommand,
} = params;
const settings = await store.getSettings();
// Track build failure state
let buildFailed = false;
let buildErrorMessage = "";
// Create custom tool for reporting build failures
const reportBuildFailureTool: ToolDefinition = {
name: "report_build_failure",
label: "Report Build Failure",
description: "Report that the build verification failed. Use this when the build command returns a non-zero exit code. Provide the error details in the message parameter.",
parameters: Type.Object({
message: Type.String({ description: "Error message describing why the build failed" }),
}),
execute: async (_toolCallId: string, params: unknown) => {
const { message } = params as { message: string };
buildFailed = true;
buildErrorMessage = message;
return {
content: [{ type: "text", text: `Build failure reported: ${message}` }],
details: undefined
};
},
};
mergerLog.log(`${taskId}: ${hasConflicts ? "resolving conflicts + " : ""}writing commit message`);
const agentLogger = new AgentLogger({
@@ -1050,6 +1126,7 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<boolean> {
cwd: rootDir,
systemPrompt: buildMergeSystemPrompt(includeTaskId),
tools: "coding",
customTools: [reportBuildFailureTool],
onText: agentLogger.onText,
onThinking: agentLogger.onThinking,
onToolStart: agentLogger.onToolStart,
@@ -1070,11 +1147,18 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<boolean> {
diffStat,
hasConflicts,
simplifiedContext,
buildCommand,
});
await session.prompt(prompt);
checkSessionError(session);
// Check if build failed
if (buildFailed) {
mergerLog.error(`Build verification failed for ${taskId}: ${buildErrorMessage}`);
return { success: false, error: buildErrorMessage };
}
// Verify commit happened
const staged = execSync("git diff --cached --quiet 2>&1; echo $?", {
cwd: rootDir,
@@ -1082,16 +1166,24 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<boolean> {
}).trim();
if (staged !== "0") {
mergerLog.log("Agent didn't commit — committing with fallback message");
const escapedLog = commitLog.replace(/"/g, '\\"');
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
execSync(
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"`,
{ cwd: rootDir, stdio: "pipe" },
);
// Only use fallback commit if no build command was configured
// If build command was configured, agent should have committed or reported failure
if (!buildCommand) {
mergerLog.log("Agent didn't commit — committing with fallback message");
const escapedLog = commitLog.replace(/"/g, '\\"');
const fallbackPrefix = includeTaskId ? `feat(${taskId})` : "feat";
execSync(
`git commit -m "${fallbackPrefix}: merge ${branch}" -m "${escapedLog}"`,
{ cwd: rootDir, stdio: "pipe" },
);
} else {
// Build command was configured but agent didn't commit and didn't report failure
// This is an error condition - agent didn't follow instructions
throw new Error(`Agent did not commit and did not report build failure for ${taskId}`);
}
}
return true;
return { success: true };
} catch (err: any) {
mergerLog.error(`Agent failed: ${err.message}`);
@@ -1113,10 +1205,11 @@ interface MergePromptParams {
diffStat: string;
hasConflicts: boolean;
simplifiedContext?: boolean;
buildCommand?: string;
}
function buildMergePrompt(params: MergePromptParams): string {
const { taskId, branch, commitLog, diffStat, hasConflicts, simplifiedContext } = params;
const { taskId, branch, commitLog, diffStat, hasConflicts, simplifiedContext, buildCommand } = params;
const parts = [
`Finalize the merge of branch \`${branch}\` for task ${taskId}.`,
@@ -1154,6 +1247,17 @@ function buildMergePrompt(params: MergePromptParams): string {
);
}
// Add build command section if provided
if (buildCommand) {
parts.push(
"",
"## Build command",
`Build command: \`${buildCommand}\``,
"",
"Run this command via bash tool before committing to verify the build passes.",
);
}
return parts.join("\n");
}