fix(FN-1952): bound recovery and task log growth
This commit is contained in:
@@ -844,6 +844,39 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("fails fast when the configured base ref is missing", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
||||
if (command.includes("git rev-parse --verify")) {
|
||||
const error: any = new Error("fatal: Needed a single revision");
|
||||
error.stderr = Buffer.from("fatal: Needed a single revision");
|
||||
throw error;
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
const onError = vi.fn();
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { onError });
|
||||
await executor.execute({ ...makeTask(), baseBranch: "fusion/missing-base" });
|
||||
|
||||
expect(mockedExecSync).not.toHaveBeenCalledWith(
|
||||
expect.stringContaining("git worktree add"),
|
||||
expect.any(Object),
|
||||
);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
"Worktree base ref is missing",
|
||||
expect.stringContaining("fusion/missing-base"),
|
||||
);
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
);
|
||||
expect(onError).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fails after 3 unsuccessful attempts with detailed error", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
@@ -1173,6 +1206,45 @@ describe("TaskExecutor worktree recovery", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("bounds stale-reference cleanup retries when update-ref succeeds but the ref remains invalid", async () => {
|
||||
const store = createMockStore();
|
||||
let worktreeAddCallCount = 0;
|
||||
|
||||
mockedExecSync.mockImplementation((cmd: string | string[]) => {
|
||||
const command = typeof cmd === "string" ? cmd : cmd[0];
|
||||
if (command.includes("git worktree add")) {
|
||||
worktreeAddCallCount++;
|
||||
const error: any = new Error("fatal: invalid reference: 'fusion/fn-050'");
|
||||
error.stderr = Buffer.from("fatal: invalid reference: 'fusion/fn-050'");
|
||||
throw error;
|
||||
}
|
||||
if (command.includes("git branch -D")) {
|
||||
const error: any = new Error("error: branch 'fusion/fn-050' not found");
|
||||
error.stderr = Buffer.from("error: branch 'fusion/fn-050' not found");
|
||||
throw error;
|
||||
}
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
const onError = vi.fn();
|
||||
const executor = new TaskExecutor(store, "/tmp/test", { onError });
|
||||
const executePromise = executor.execute(makeTask());
|
||||
await vi.advanceTimersByTimeAsync(2000);
|
||||
await executePromise;
|
||||
|
||||
expect(worktreeAddCallCount).toBe(3);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
expect.stringContaining("Worktree creation failed after 3 attempts"),
|
||||
expect.any(String),
|
||||
);
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
"FN-050",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
);
|
||||
expect(onError).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fails task when all stale reference cleanup steps fail", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
|
||||
@@ -2,7 +2,7 @@ import { exec } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
import { join } from "node:path";
|
||||
import { isAbsolute, join } from "node:path";
|
||||
import { existsSync } from "node:fs";
|
||||
import { readFile, writeFile } from "node:fs/promises";
|
||||
import type { TaskStore, Task, TaskDetail, StepStatus, Settings, WorkflowStep, MissionStore, Slice, AgentState, AgentCapability, RunMutationContext } from "@fusion/core";
|
||||
@@ -66,6 +66,8 @@ const STEP_STATUSES: StepStatus[] = ["pending", "in-progress", "done", "skipped"
|
||||
const MAX_WORKFLOW_STEP_RETRIES = 3;
|
||||
const WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS = 4_000;
|
||||
|
||||
class NonRetryableWorktreeError extends Error {}
|
||||
|
||||
function truncateWorkflowScriptOutput(output: string): string {
|
||||
if (output.length <= WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS) return output;
|
||||
return `... output truncated to last ${WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS} characters ...\n${output.slice(-WORKFLOW_SCRIPT_OUTPUT_MAX_CHARS)}`;
|
||||
@@ -3297,15 +3299,19 @@ and show an appropriate message to the user.\`
|
||||
): Promise<{ path: string; branch: string }> {
|
||||
// Track the worktree path we're attempting to use (may change during recovery)
|
||||
const currentPath = path;
|
||||
const resolvedStartPoint = startPoint
|
||||
? await this.resolveWorktreeStartPoint(startPoint, taskId)
|
||||
: undefined;
|
||||
|
||||
for (let attempt = 0; attempt < this.MAX_WORKTREE_RETRIES; attempt++) {
|
||||
try {
|
||||
return await this.tryCreateWorktree(branch, currentPath, taskId, startPoint, attempt);
|
||||
return await this.tryCreateWorktree(branch, currentPath, taskId, resolvedStartPoint, attempt);
|
||||
} catch (error: unknown) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
const isLastAttempt = attempt === this.MAX_WORKTREE_RETRIES - 1;
|
||||
const isTerminalWorktreeError = error instanceof NonRetryableWorktreeError;
|
||||
|
||||
if (isLastAttempt) {
|
||||
if (isLastAttempt || isTerminalWorktreeError) {
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
`Worktree creation failed after ${this.MAX_WORKTREE_RETRIES} attempts`,
|
||||
@@ -3326,6 +3332,27 @@ and show an appropriate message to the user.\`
|
||||
throw new Error("Unexpected exit from worktree creation retry loop");
|
||||
}
|
||||
|
||||
private async resolveWorktreeStartPoint(startPoint: string, taskId: string): Promise<string> {
|
||||
const command = isAbsolute(startPoint) && existsSync(startPoint)
|
||||
? `git -C "${startPoint}" rev-parse --verify HEAD^{commit}`
|
||||
: `git rev-parse --verify "${startPoint}^{commit}"`;
|
||||
|
||||
try {
|
||||
const { stdout } = await execAsync(command, { cwd: this.rootDir });
|
||||
return stdout.trim() || startPoint;
|
||||
} catch (error: unknown) {
|
||||
const errorMessage = error instanceof Error ? error.message : String(error);
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
`Worktree base ref is missing`,
|
||||
`${startPoint}: ${errorMessage}`,
|
||||
);
|
||||
throw new NonRetryableWorktreeError(
|
||||
`Cannot create worktree for ${taskId}: base ref "${startPoint}" does not exist or cannot be resolved`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Single attempt to create a worktree with conflict detection and recovery.
|
||||
* Returns the actual worktree path used (may differ from input if recovery generated new name).
|
||||
@@ -3336,6 +3363,7 @@ and show an appropriate message to the user.\`
|
||||
taskId: string,
|
||||
startPoint?: string,
|
||||
attemptNumber = 0,
|
||||
recoveryDepth = 0,
|
||||
): Promise<{ path: string; branch: string }> {
|
||||
// If directory exists but is not a registered worktree, remove it first
|
||||
if (existsSync(path)) {
|
||||
@@ -3398,10 +3426,15 @@ and show an appropriate message to the user.\`
|
||||
|
||||
// Handle "invalid reference" - stale branch that doesn't exist
|
||||
if (conflictInfo.type === "invalid-reference") {
|
||||
if (recoveryDepth >= this.MAX_WORKTREE_RETRIES - 1) {
|
||||
throw new NonRetryableWorktreeError(
|
||||
`Stale branch reference for ${branch} remained invalid after ${this.MAX_WORKTREE_RETRIES} cleanup attempts`,
|
||||
);
|
||||
}
|
||||
const branchCleaned = await this.cleanupStaleBranch(branch, taskId);
|
||||
if (branchCleaned) {
|
||||
await this.store.logEntry(taskId, `Removed stale branch reference, retrying`);
|
||||
return this.tryCreateWorktree(branch, path, taskId, startPoint, attemptNumber);
|
||||
return this.tryCreateWorktree(branch, path, taskId, startPoint, attemptNumber, recoveryDepth + 1);
|
||||
}
|
||||
throw new Error(
|
||||
`Invalid reference for branch ${branch}: unable to clean up stale reference`,
|
||||
@@ -3444,10 +3477,15 @@ and show an appropriate message to the user.\`
|
||||
|
||||
// Handle stale reference in fallback path too
|
||||
if (fallbackConflictInfo.type === "invalid-reference") {
|
||||
if (recoveryDepth >= this.MAX_WORKTREE_RETRIES - 1) {
|
||||
throw new NonRetryableWorktreeError(
|
||||
`Stale branch reference for ${branch} remained invalid after ${this.MAX_WORKTREE_RETRIES} cleanup attempts`,
|
||||
);
|
||||
}
|
||||
const branchCleaned = await this.cleanupStaleBranch(branch, taskId);
|
||||
if (branchCleaned) {
|
||||
await this.store.logEntry(taskId, `Cleaned up stale reference in fallback, retrying`);
|
||||
return this.tryCreateWorktree(branch, path, taskId, startPoint, attemptNumber);
|
||||
return this.tryCreateWorktree(branch, path, taskId, startPoint, attemptNumber, recoveryDepth + 1);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user