feat(FN-1858): merge fusion/fn-1858
This commit is contained in:
@@ -106,6 +106,7 @@ Reference: `useProjectHealth` in `packages/dashboard/app/hooks/useProjectHealth.
|
|||||||
- The null-as-delete pattern for settings: In `TaskStore.updateSettings()`, `null` values in the settings patch are treated as "delete this key from settings" (since `JSON.stringify` drops `undefined` keys). This allows the frontend to explicitly clear a setting by sending `null`. The key is deleted from both `config.settings` and `projectPatch` before merging, so cleared settings fall back to `DEFAULT_SETTINGS`.
|
- The null-as-delete pattern for settings: In `TaskStore.updateSettings()`, `null` values in the settings patch are treated as "delete this key from settings" (since `JSON.stringify` drops `undefined` keys). This allows the frontend to explicitly clear a setting by sending `null`. The key is deleted from both `config.settings` and `projectPatch` before merging, so cleared settings fall back to `DEFAULT_SETTINGS`.
|
||||||
- `TaskStore.logEntry()`, `addComment()`, `addSteeringComment()`, `pauseTask()` accept an optional `RunMutationContext` parameter for audit trail correlation. Always pass it when the caller is an engine module (executor, heartbeat monitor) to maintain the audit trail. The executor constructs a synthetic `runContext` with `runId: "exec-{taskId}-{timestamp}-{random}"` since it doesn't use `AgentHeartbeatRun`.
|
- `TaskStore.logEntry()`, `addComment()`, `addSteeringComment()`, `pauseTask()` accept an optional `RunMutationContext` parameter for audit trail correlation. Always pass it when the caller is an engine module (executor, heartbeat monitor) to maintain the audit trail. The executor constructs a synthetic `runContext` with `runId: "exec-{taskId}-{timestamp}-{random}"` since it doesn't use `AgentHeartbeatRun`.
|
||||||
- **Run-Audit Instrumentation (FN-1404)**: The engine instruments mutation calls with audit events via `createRunAuditor()` from `run-audit.ts`. Each active run (heartbeat, executor, merger) creates an `EngineRunContext` with `runId`, `agentId`, `taskId`, and `phase`. The auditor no-ops cleanly when no run context exists (backward compatible with manual/non-run paths). Use `generateSyntheticRunId()` for executor/merger synthetic IDs. Audit events are emitted for git mutations (worktree/branch/create/remove/reset), database mutations (task:update/move/comment/assign/checkout), and filesystem mutations (file:capture-modified).
|
- **Run-Audit Instrumentation (FN-1404)**: The engine instruments mutation calls with audit events via `createRunAuditor()` from `run-audit.ts`. Each active run (heartbeat, executor, merger) creates an `EngineRunContext` with `runId`, `agentId`, `taskId`, and `phase`. The auditor no-ops cleanly when no run context exists (backward compatible with manual/non-run paths). Use `generateSyntheticRunId()` for executor/merger synthetic IDs. Audit events are emitted for git mutations (worktree/branch/create/remove/reset), database mutations (task:update/move/comment/assign/checkout), and filesystem mutations (file:capture-modified).
|
||||||
|
- **In-Merge Verification Fix (FN-1858)**: When deterministic verification fails during merge, the merger now attempts to fix the issue by spawning an AI agent on the main branch (`cwd: rootDir`). This is different from the executor which runs in worktrees. The fix agent uses `tools: "coding"` for read/write access. Always dispose sessions in `finally` blocks, use `withRateLimitRetry()` for resilience, and cap retry attempts (max 3) to prevent runaway costs.
|
||||||
- **Write-through cache pattern (FN-1336)**: When adding caching to a store, use write-through invalidation (update cache in setter, return cached value in getter). For `GlobalSettingsStore`, the cache survives for the lifetime of the process since it's a singleton per server instance. Add `invalidateCache()` for testing and edge cases where external processes modify the file.
|
- **Write-through cache pattern (FN-1336)**: When adding caching to a store, use write-through invalidation (update cache in setter, return cached value in getter). For `GlobalSettingsStore`, the cache survives for the lifetime of the process since it's a singleton per server instance. Add `invalidateCache()` for testing and edge cases where external processes modify the file.
|
||||||
- **API wrapper tests for validation**: When testing functions that validate parameters synchronously before calling fetch:
|
- **API wrapper tests for validation**: When testing functions that validate parameters synchronously before calling fetch:
|
||||||
- Use `expect(() => fn()).toThrow()` for synchronous throws (not `rejects.toThrow()`)
|
- Use `expect(() => fn()).toThrow()` for synchronous throws (not `rejects.toThrow()`)
|
||||||
|
|||||||
35
AGENTS.md
35
AGENTS.md
@@ -1701,6 +1701,41 @@ The email used in the git `--author` flag for Fusion commits. Only used when `co
|
|||||||
- The committer identity (who physically makes the commit) remains unchanged — only the author metadata is affected
|
- The committer identity (who physically makes the commit) remains unchanged — only the author metadata is affected
|
||||||
- Configure or disable via Settings → Merge → Author attribution in the dashboard
|
- Configure or disable via Settings → Merge → Author attribution in the dashboard
|
||||||
|
|
||||||
|
### `verificationFixRetries` (default: `1`)
|
||||||
|
|
||||||
|
When deterministic verification (test/build commands) fails during a merge, this setting controls how many times the system attempts an in-merge fix before aborting the merge. The fix agent runs on the main branch with the merged code to resolve the failure directly, rather than sending the task back to `in-progress` for re-execution.
|
||||||
|
|
||||||
|
**How it works:**
|
||||||
|
- When verification fails, a fix agent is spawned on the main branch
|
||||||
|
- The fix agent reads the error output, makes targeted fixes, and re-runs the verification command
|
||||||
|
- If verification passes after the fix, the merge commit is amended to include the fixes
|
||||||
|
- If verification still fails, the fix attempt is logged and the next attempt begins
|
||||||
|
- After all fix attempts are exhausted, the system falls back to the existing behavior (moves task to `in-progress`)
|
||||||
|
|
||||||
|
**Valid range:** 0 to 3 (maximum is capped at 3 to prevent runaway AI costs)
|
||||||
|
|
||||||
|
**Configuration:**
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"settings": {
|
||||||
|
"verificationFixRetries": 1
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
**Examples:**
|
||||||
|
- `0` — Disable in-merge fix; use the existing fallback behavior (move to `in-progress`)
|
||||||
|
- `1` — Allow one fix attempt (default)
|
||||||
|
- `2` — Allow two fix attempts
|
||||||
|
- `3` — Allow three fix attempts (maximum)
|
||||||
|
|
||||||
|
**Notes:**
|
||||||
|
- This setting does NOT affect the existing `buildRetryCount` setting for transient build errors
|
||||||
|
- The fix agent uses the same model settings as the merger agent
|
||||||
|
- When no verification commands are configured (`testCommand` or `buildCommand`), this setting has no effect
|
||||||
|
- Fix attempts and outcomes are logged to the task's activity log
|
||||||
|
|
||||||
### `requirePlanApproval` (default: `false`)
|
### `requirePlanApproval` (default: `false`)
|
||||||
|
|
||||||
When enabled, AI-generated task specifications require manual approval before the task can move from "triage" to "todo".
|
When enabled, AI-generated task specifications require manual approval before the task can move from "triage" to "todo".
|
||||||
|
|||||||
@@ -67,6 +67,7 @@ export const DEFAULT_PROJECT_SETTINGS = {
|
|||||||
smartConflictResolution: true,
|
smartConflictResolution: true,
|
||||||
strictScopeEnforcement: false,
|
strictScopeEnforcement: false,
|
||||||
buildRetryCount: 0,
|
buildRetryCount: 0,
|
||||||
|
verificationFixRetries: 1,
|
||||||
buildTimeoutMs: 300_000,
|
buildTimeoutMs: 300_000,
|
||||||
requirePlanApproval: false,
|
requirePlanApproval: false,
|
||||||
specStalenessEnabled: false,
|
specStalenessEnabled: false,
|
||||||
|
|||||||
@@ -1030,6 +1030,10 @@ export interface ProjectSettings {
|
|||||||
/** Maximum number of build retry attempts during merge when a build fails with a
|
/** Maximum number of build retry attempts during merge when a build fails with a
|
||||||
* transient error. Default: 0 (no retry). Set to 1 to allow one retry. */
|
* transient error. Default: 0 (no retry). Set to 1 to allow one retry. */
|
||||||
buildRetryCount?: number;
|
buildRetryCount?: number;
|
||||||
|
/** Maximum number of times to attempt in-merge verification fixes when test/build
|
||||||
|
* commands fail during merge. The fix agent runs on the main branch with the merged
|
||||||
|
* code to resolve failures before aborting the merge. Default: 1. Set to 0 to disable. */
|
||||||
|
verificationFixRetries?: number;
|
||||||
/** Timeout in milliseconds for build commands during merge. Default: 300000 (5 min). */
|
/** Timeout in milliseconds for build commands during merge. Default: 300000 (5 min). */
|
||||||
buildTimeoutMs?: number;
|
buildTimeoutMs?: number;
|
||||||
/** When enabled, AI-generated task specifications require manual approval
|
/** When enabled, AI-generated task specifications require manual approval
|
||||||
|
|||||||
@@ -1989,6 +1989,7 @@ describe("aiMergeTask — build verification", () => {
|
|||||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
...DEFAULT_SETTINGS,
|
...DEFAULT_SETTINGS,
|
||||||
buildCommand: "pnpm build",
|
buildCommand: "pnpm build",
|
||||||
|
verificationFixRetries: 0, // Disable in-merge fix for this test
|
||||||
});
|
});
|
||||||
|
|
||||||
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toThrow(
|
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toThrow(
|
||||||
@@ -4453,3 +4454,289 @@ describe("aiMergeTask — skill selection non-fatal diagnostics (FN-1510/FN-1511
|
|||||||
expect(opts.skillSelection?.requestedSkillNames).toEqual(["custom-skill"]);
|
expect(opts.skillSelection?.requestedSkillNames).toEqual(["custom-skill"]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("aiMergeTask — in-merge verification fix", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
vi.spyOn(console, "error").mockImplementation(() => {});
|
||||||
|
mockedExistsSync.mockReturnValue(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("verification fix is attempted when verification fails", async () => {
|
||||||
|
// Simple mock: always fail verification
|
||||||
|
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("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("vitest run")) {
|
||||||
|
const err = new Error("Test failed") as any;
|
||||||
|
err.status = 1;
|
||||||
|
err.stdout = "";
|
||||||
|
err.stderr = "";
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
|
||||||
|
if (cmdStr.includes("diff --cached")) 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 === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||||
|
return Buffer.from("");
|
||||||
|
});
|
||||||
|
|
||||||
|
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||||
|
const isFixAgent = opts.systemPrompt?.includes("verification fix agent");
|
||||||
|
return {
|
||||||
|
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,
|
||||||
|
testCommand: "vitest run",
|
||||||
|
verificationFixRetries: 1,
|
||||||
|
});
|
||||||
|
|
||||||
|
// With verificationFixRetries: 1, the merge should fail with VerificationError
|
||||||
|
// because the fix agent can't fix the verification (it's mocked to not actually fix anything)
|
||||||
|
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toMatchObject({
|
||||||
|
name: "VerificationError",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Verify that fix agent was spawned (2 calls: merger + fix)
|
||||||
|
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(2);
|
||||||
|
|
||||||
|
// Verify the fix agent was called with correct options
|
||||||
|
const fixAgentCall = mockedCreateHaiAgent.mock.calls[1];
|
||||||
|
expect(fixAgentCall[0].tools).toBe("coding");
|
||||||
|
expect(fixAgentCall[0].cwd).toBe("/tmp/root");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("verification fix is skipped when verificationFixRetries is 0", async () => {
|
||||||
|
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("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("vitest run")) {
|
||||||
|
const err = new Error("Test failed") as any;
|
||||||
|
err.status = 1;
|
||||||
|
err.stdout = "";
|
||||||
|
err.stderr = "";
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
|
||||||
|
if (cmdStr.includes("diff --cached")) 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 === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||||
|
return Buffer.from("");
|
||||||
|
});
|
||||||
|
|
||||||
|
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,
|
||||||
|
testCommand: "vitest run",
|
||||||
|
verificationFixRetries: 0,
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toMatchObject({
|
||||||
|
name: "VerificationError",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Verify fix agent was NOT spawned (only merger)
|
||||||
|
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Verify no fix attempt was logged
|
||||||
|
const logCalls = (store.logEntry as ReturnType<typeof vi.fn>).mock.calls;
|
||||||
|
const fixAttempts = logCalls.filter((call: any[]) =>
|
||||||
|
typeof call[1] === "string" && call[1].includes("in-merge verification fix"),
|
||||||
|
);
|
||||||
|
expect(fixAttempts).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("fix agent uses same model settings as merger", async () => {
|
||||||
|
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("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("vitest run")) {
|
||||||
|
const err = new Error("Test failed") as any;
|
||||||
|
err.status = 1;
|
||||||
|
err.stdout = "";
|
||||||
|
err.stderr = "";
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
|
||||||
|
if (cmdStr.includes("diff --cached")) 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 === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||||
|
return Buffer.from("");
|
||||||
|
});
|
||||||
|
|
||||||
|
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||||
|
return {
|
||||||
|
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,
|
||||||
|
testCommand: "vitest run",
|
||||||
|
verificationFixRetries: 1,
|
||||||
|
defaultProvider: "anthropic",
|
||||||
|
defaultModelId: "claude-sonnet-4-5",
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toMatchObject({
|
||||||
|
name: "VerificationError",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Verify fix agent uses same model settings
|
||||||
|
const fixAgentCall = mockedCreateHaiAgent.mock.calls[1];
|
||||||
|
expect(fixAgentCall[0].defaultProvider).toBe("anthropic");
|
||||||
|
expect(fixAgentCall[0].defaultModelId).toBe("claude-sonnet-4-5");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("fix agent session is disposed", async () => {
|
||||||
|
const disposeMock = vi.fn();
|
||||||
|
|
||||||
|
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("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("vitest run")) {
|
||||||
|
const err = new Error("Test failed") as any;
|
||||||
|
err.status = 1;
|
||||||
|
err.stdout = "";
|
||||||
|
err.stderr = "";
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
|
||||||
|
if (cmdStr.includes("diff --cached")) 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 === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||||
|
return Buffer.from("");
|
||||||
|
});
|
||||||
|
|
||||||
|
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||||
|
const isFixAgent = opts.systemPrompt?.includes("verification fix agent");
|
||||||
|
if (isFixAgent) {
|
||||||
|
return {
|
||||||
|
session: {
|
||||||
|
prompt: vi.fn().mockResolvedValue(undefined),
|
||||||
|
dispose: disposeMock,
|
||||||
|
},
|
||||||
|
} as any;
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
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,
|
||||||
|
testCommand: "vitest run",
|
||||||
|
verificationFixRetries: 1,
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toMatchObject({
|
||||||
|
name: "VerificationError",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(disposeMock).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("max fix retries capped at 3", async () => {
|
||||||
|
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("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("vitest run")) {
|
||||||
|
const err = new Error("Test failed") as any;
|
||||||
|
err.status = 1;
|
||||||
|
err.stdout = "";
|
||||||
|
err.stderr = "";
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
|
||||||
|
if (cmdStr.includes("diff --cached")) 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 === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||||
|
return Buffer.from("");
|
||||||
|
});
|
||||||
|
|
||||||
|
mockedCreateHaiAgent.mockImplementation(async (opts: any) => {
|
||||||
|
return {
|
||||||
|
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,
|
||||||
|
testCommand: "vitest run",
|
||||||
|
verificationFixRetries: 10, // Exceeds max
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toMatchObject({
|
||||||
|
name: "VerificationError",
|
||||||
|
});
|
||||||
|
|
||||||
|
// Should have 3 fix attempts (capped at 3) + 1 merger = 4 calls
|
||||||
|
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(4);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -615,6 +615,165 @@ async function runVerificationCommand(
|
|||||||
return result;
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Attempt an in-merge verification fix by spawning an AI agent on the main branch.
|
||||||
|
* Returns true if verification passes after the fix, false otherwise.
|
||||||
|
* Never throws — errors are caught and logged, and the function returns false.
|
||||||
|
*/
|
||||||
|
async function attemptInMergeVerificationFix(
|
||||||
|
store: TaskStore,
|
||||||
|
rootDir: string,
|
||||||
|
taskId: string,
|
||||||
|
failureContext: {
|
||||||
|
command: string;
|
||||||
|
exitCode: number | null;
|
||||||
|
output: string;
|
||||||
|
type: "test" | "build";
|
||||||
|
},
|
||||||
|
settings: Settings,
|
||||||
|
options: MergerOptions,
|
||||||
|
testCommand?: string,
|
||||||
|
buildCommand?: string,
|
||||||
|
): Promise<boolean> {
|
||||||
|
try {
|
||||||
|
mergerLog.log(`${taskId}: spawning in-merge verification fix agent`);
|
||||||
|
|
||||||
|
// Build skill selection context
|
||||||
|
let skillContext = undefined;
|
||||||
|
if (options.agentStore) {
|
||||||
|
try {
|
||||||
|
const task = await store.getTask(taskId);
|
||||||
|
skillContext = await buildSessionSkillContext({
|
||||||
|
agentStore: options.agentStore,
|
||||||
|
task,
|
||||||
|
sessionPurpose: "merger",
|
||||||
|
projectRootDir: rootDir,
|
||||||
|
});
|
||||||
|
} catch {
|
||||||
|
// Graceful fallback - no skill selection
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create the fix agent session
|
||||||
|
const { session } = await createKbAgent({
|
||||||
|
cwd: rootDir, // Runs on the main branch in the project root
|
||||||
|
systemPrompt: `You are a verification fix agent running during a merge on the main branch.
|
||||||
|
|
||||||
|
A merge has been applied and the verification command failed. Your job is to fix the failing code directly in the working directory.
|
||||||
|
|
||||||
|
## Rules
|
||||||
|
1. Read the error output carefully to understand what's failing
|
||||||
|
2. Make targeted fixes to the failing code
|
||||||
|
3. After fixing, run the verification command to confirm the fix works
|
||||||
|
4. Do NOT make any git commits — just fix the code
|
||||||
|
5. Do NOT modify files unrelated to the failure
|
||||||
|
6. If you cannot fix the issue, explain why`,
|
||||||
|
tools: "coding", // Agent needs read/write file access
|
||||||
|
defaultProvider: settings.defaultProvider,
|
||||||
|
defaultModelId: settings.defaultModelId,
|
||||||
|
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||||
|
// Skill selection: use assigned agent skills if available, otherwise role fallback
|
||||||
|
...(skillContext?.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
// Build the fix prompt
|
||||||
|
const fixPrompt = `Fix the failing ${failureContext.type} verification for task ${taskId}.
|
||||||
|
|
||||||
|
## Failed command
|
||||||
|
Command: \`${failureContext.command}\`
|
||||||
|
Exit code: ${failureContext.exitCode}
|
||||||
|
|
||||||
|
## Error output
|
||||||
|
${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
|
||||||
|
|
||||||
|
## Instructions
|
||||||
|
1. Read the error output and identify the root cause
|
||||||
|
2. Make targeted fixes to resolve the failure
|
||||||
|
3. Run the verification command \`${failureContext.command}\` to confirm your fix works
|
||||||
|
4. If the fix doesn't work, try a different approach
|
||||||
|
5. Do NOT make any git commits`;
|
||||||
|
|
||||||
|
// Run the agent with rate limit retry
|
||||||
|
await withRateLimitRetry(async () => {
|
||||||
|
await promptWithFallback(session, fixPrompt);
|
||||||
|
}, {
|
||||||
|
onRetry: (attempt, delayMs, error) => {
|
||||||
|
const delaySec = Math.round(delayMs / 1000);
|
||||||
|
mergerLog.warn(`⏳ ${taskId} in-merge fix rate limited — retry ${attempt} in ${delaySec}s: ${error.message}`);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
// Re-run the verification command that failed
|
||||||
|
const reRunResult = await runVerificationCommand(
|
||||||
|
store, rootDir, taskId, failureContext.command, failureContext.type,
|
||||||
|
);
|
||||||
|
|
||||||
|
return reRunResult.success;
|
||||||
|
} finally {
|
||||||
|
// Always dispose the session
|
||||||
|
await session.dispose();
|
||||||
|
}
|
||||||
|
} catch (err: unknown) {
|
||||||
|
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||||
|
mergerLog.warn(`${taskId}: in-merge fix agent error: ${errorMessage}`);
|
||||||
|
await store.logEntry(taskId, "In-merge verification fix agent encountered an error", errorMessage);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Stage any changes and amend the merge commit to include verification fixes.
|
||||||
|
* Returns true if changes were amended, false if no changes to amend.
|
||||||
|
* Never throws — errors are logged and the function returns false.
|
||||||
|
*/
|
||||||
|
async function amendMergeCommitWithFixes(
|
||||||
|
rootDir: string,
|
||||||
|
taskId: string,
|
||||||
|
authorArg: string,
|
||||||
|
): Promise<boolean> {
|
||||||
|
try {
|
||||||
|
// Check for staged and unstaged changes
|
||||||
|
const { stdout: stagedFiles } = await execAsync("git diff --cached --name-only", {
|
||||||
|
cwd: rootDir,
|
||||||
|
encoding: "utf-8",
|
||||||
|
});
|
||||||
|
const { stdout: unstagedFiles } = await execAsync("git diff --name-only", {
|
||||||
|
cwd: rootDir,
|
||||||
|
encoding: "utf-8",
|
||||||
|
});
|
||||||
|
|
||||||
|
const hasChanges = stagedFiles.trim().length > 0 || unstagedFiles.trim().length > 0;
|
||||||
|
if (!hasChanges) {
|
||||||
|
mergerLog.log(`${taskId}: no changes to amend after verification fix`);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Stage any unstaged changes
|
||||||
|
if (unstagedFiles.trim().length > 0) {
|
||||||
|
await execAsync("git add -A", { cwd: rootDir });
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if there are staged changes to amend
|
||||||
|
const { stdout: finalStaged } = await execAsync("git diff --cached --name-only", {
|
||||||
|
cwd: rootDir,
|
||||||
|
encoding: "utf-8",
|
||||||
|
});
|
||||||
|
|
||||||
|
if (finalStaged.trim().length > 0) {
|
||||||
|
await execAsync(`git commit --amend --no-edit${authorArg}`, { cwd: rootDir });
|
||||||
|
mergerLog.log(`${taskId}: amended merge commit with verification fixes`);
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
return false;
|
||||||
|
} catch (err: unknown) {
|
||||||
|
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||||
|
mergerLog.warn(`${taskId}: failed to amend merge commit: ${errorMessage}`);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ── Pre-merge diffstat scope validation ──────────────────────────────
|
// ── Pre-merge diffstat scope validation ──────────────────────────────
|
||||||
|
|
||||||
interface DiffFileEntry {
|
interface DiffFileEntry {
|
||||||
@@ -1388,14 +1547,107 @@ export async function aiMergeTask(
|
|||||||
return false;
|
return false;
|
||||||
} catch (error: any) {
|
} catch (error: any) {
|
||||||
// Check if it's a deterministic verification failure (testCommand or buildCommand failed)
|
// Check if it's a deterministic verification failure (testCommand or buildCommand failed)
|
||||||
// VerificationError is fatal - don't retry, propagate immediately
|
// Try in-merge fix attempts before propagating
|
||||||
if (error.name === "VerificationError") {
|
if (error.name === "VerificationError") {
|
||||||
mergerLog.error(`${taskId}: deterministic verification failed — aborting merge`);
|
const verificationErr = error as VerificationError;
|
||||||
throw error; // Fatal - verification failures don't retry
|
const maxFixRetries = Math.min(settings.verificationFixRetries ?? 1, 3);
|
||||||
|
|
||||||
|
if (maxFixRetries > 0 && (verificationErr.verificationResult.testResult || verificationErr.verificationResult.buildResult)) {
|
||||||
|
mergerLog.log(`${taskId}: deterministic verification failed — attempting in-merge fix (up to ${maxFixRetries} attempts)`);
|
||||||
|
await store.logEntry(taskId, `Verification failed during merge — attempting in-merge fix (up to ${maxFixRetries} attempts)`);
|
||||||
|
|
||||||
|
// Extract failure context from the VerificationError
|
||||||
|
const failedResult = verificationErr.verificationResult.testResult?.success === false
|
||||||
|
? verificationErr.verificationResult.testResult
|
||||||
|
: verificationErr.verificationResult.buildResult;
|
||||||
|
const failedType = verificationErr.verificationResult.testResult?.success === false
|
||||||
|
? "test" as const
|
||||||
|
: "build" as const;
|
||||||
|
|
||||||
|
if (failedResult) {
|
||||||
|
let fixSuccess = false;
|
||||||
|
for (let fixAttempt = 1; fixAttempt <= maxFixRetries; fixAttempt++) {
|
||||||
|
mergerLog.log(`${taskId}: in-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`);
|
||||||
|
await store.logEntry(taskId, `In-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`);
|
||||||
|
|
||||||
|
fixSuccess = await attemptInMergeVerificationFix(
|
||||||
|
store, rootDir, taskId,
|
||||||
|
{
|
||||||
|
command: failedResult.command,
|
||||||
|
exitCode: failedResult.exitCode,
|
||||||
|
output: summarizeVerificationOutput(failedResult.stderr || failedResult.stdout, failedType),
|
||||||
|
type: failedType,
|
||||||
|
},
|
||||||
|
settings, options, effectiveTestCommand, effectiveBuildCommand,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (fixSuccess) {
|
||||||
|
mergerLog.log(`${taskId}: in-merge verification fix succeeded on attempt ${fixAttempt}`);
|
||||||
|
await store.logEntry(taskId, `In-merge verification fix succeeded — verification now passes`);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
|
||||||
|
mergerLog.warn(`${taskId}: in-merge verification fix attempt ${fixAttempt} — verification still fails`);
|
||||||
|
await store.logEntry(taskId, `In-merge verification fix attempt ${fixAttempt} — verification still fails`);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (fixSuccess) {
|
||||||
|
// Amend the merge commit to include the fixes
|
||||||
|
const authorArg = getCommitAuthorArg(settings);
|
||||||
|
await amendMergeCommitWithFixes(rootDir, taskId, authorArg);
|
||||||
|
return true; // Merge succeeds
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fix attempts exhausted or disabled — fall back to existing behavior
|
||||||
|
mergerLog.error(`${taskId}: deterministic verification failed — aborting merge (in-merge fix exhausted or disabled)`);
|
||||||
|
throw error;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if it's a build verification failure
|
// Check if it's a build verification failure
|
||||||
if (error.message?.includes("Build verification failed")) {
|
if (error.message?.includes("Build verification failed")) {
|
||||||
|
const maxFixRetries = Math.min(settings.verificationFixRetries ?? 1, 3);
|
||||||
|
|
||||||
|
// Try in-merge fix before falling back to build retry
|
||||||
|
if (maxFixRetries > 0 && (effectiveTestCommand || effectiveBuildCommand)) {
|
||||||
|
mergerLog.log(`${taskId}: build verification failed — attempting in-merge fix`);
|
||||||
|
await store.logEntry(taskId, `Build verification failed during merge — attempting in-merge fix`);
|
||||||
|
|
||||||
|
const fixCommand = effectiveBuildCommand || effectiveTestCommand!;
|
||||||
|
const fixType = effectiveBuildCommand ? "build" as const : "test" as const;
|
||||||
|
|
||||||
|
let fixSuccess = false;
|
||||||
|
for (let fixAttempt = 1; fixAttempt <= maxFixRetries; fixAttempt++) {
|
||||||
|
mergerLog.log(`${taskId}: in-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`);
|
||||||
|
await store.logEntry(taskId, `In-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`);
|
||||||
|
|
||||||
|
fixSuccess = await attemptInMergeVerificationFix(
|
||||||
|
store, rootDir, taskId,
|
||||||
|
{
|
||||||
|
command: fixCommand,
|
||||||
|
exitCode: 1,
|
||||||
|
output: error.message || "Build verification failed",
|
||||||
|
type: fixType,
|
||||||
|
},
|
||||||
|
settings, options, effectiveTestCommand, effectiveBuildCommand,
|
||||||
|
);
|
||||||
|
|
||||||
|
if (fixSuccess) {
|
||||||
|
mergerLog.log(`${taskId}: in-merge verification fix succeeded on attempt ${fixAttempt}`);
|
||||||
|
await store.logEntry(taskId, `In-merge verification fix succeeded`);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (fixSuccess) {
|
||||||
|
const authorArg = getCommitAuthorArg(settings);
|
||||||
|
await amendMergeCommitWithFixes(rootDir, taskId, authorArg);
|
||||||
|
return true; // Merge succeeds
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fall through to existing buildRetryCount logic
|
||||||
const buildRetryCount = settings.buildRetryCount ?? 0;
|
const buildRetryCount = settings.buildRetryCount ?? 0;
|
||||||
if (buildRetryCount > 0 && !result._buildRetried) {
|
if (buildRetryCount > 0 && !result._buildRetried) {
|
||||||
// Allow one build retry — reset merge state and re-attempt same strategy
|
// Allow one build retry — reset merge state and re-attempt same strategy
|
||||||
|
|||||||
Reference in New Issue
Block a user