fix(FN-756): reset recycled worktree baselines

This commit is contained in:
gsxdsm
2026-04-03 07:09:27 -07:00
parent bed9fcb378
commit a7e2ca5adb
6 changed files with 138 additions and 9 deletions

View File

@@ -135,6 +135,9 @@ describe("GET /api/tasks/:id/session-files", () => {
const store = new MockStore();
store.addTask(createTask({ id: "FN-675-base", baseCommitSha: "abc123" }));
mockExecSync.mockImplementation((command) => {
if (String(command) === "git merge-base --is-ancestor abc123 HEAD") {
return "" as any;
}
if (String(command) === "git diff --name-only abc123..HEAD") {
return "src/a.ts\nsrc/b.ts\n" as any;
}
@@ -145,10 +148,56 @@ describe("GET /api/tasks/:id/session-files", () => {
expect(response.status).toBe(200);
expect(response.body).toEqual(["src/a.ts", "src/b.ts"]);
expect(mockExecSync).toHaveBeenCalledWith("git diff --name-only abc123..HEAD", expect.objectContaining({ cwd: "/tmp/fn-675" }));
expect(mockExecSync).toHaveBeenNthCalledWith(
1,
"git merge-base --is-ancestor abc123 HEAD",
expect.objectContaining({ cwd: "/tmp/fn-675" }),
);
expect(mockExecSync).toHaveBeenNthCalledWith(
2,
"git diff --name-only abc123..HEAD",
expect.objectContaining({ cwd: "/tmp/fn-675" }),
);
expect(mockExecSync).not.toHaveBeenCalledWith(expect.stringContaining("...HEAD"), expect.anything());
});
it("ignores stale baseCommitSha values that are not ancestors of HEAD", async () => {
const store = new MockStore();
store.addTask(createTask({ id: "FN-675-stale-base", baseCommitSha: "stale123" }));
mockExecSync.mockImplementation((command) => {
if (String(command) === "git merge-base --is-ancestor stale123 HEAD") {
throw new Error("not ancestor");
}
if (String(command) === "git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main") {
return "mergebase123\n" as any;
}
if (String(command) === "git diff --name-only mergebase123..HEAD") {
return "packages/engine/src/executor.ts\n" as any;
}
throw new Error(`Unexpected command: ${String(command)}`);
});
const response = await requestSessionFiles(store, "FN-675-stale-base");
expect(response.status).toBe(200);
expect(response.body).toEqual(["packages/engine/src/executor.ts"]);
expect(mockExecSync).toHaveBeenNthCalledWith(
1,
"git merge-base --is-ancestor stale123 HEAD",
expect.objectContaining({ cwd: "/tmp/fn-675" }),
);
expect(mockExecSync).toHaveBeenNthCalledWith(
2,
"git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main",
expect.objectContaining({ cwd: "/tmp/fn-675" }),
);
expect(mockExecSync).toHaveBeenNthCalledWith(
3,
"git diff --name-only mergebase123..HEAD",
expect.objectContaining({ cwd: "/tmp/fn-675" }),
);
});
it("computes fallback base ref with merge-base and returns matching file list", async () => {
const store = new MockStore();
store.addTask(createTask({ id: "FN-675-merge-base", baseCommitSha: undefined }));
@@ -217,7 +266,15 @@ describe("GET /api/tasks/:id/session-files", () => {
it("uses the 10-second cache before recomputing", async () => {
const store = new MockStore();
store.addTask(createTask({ id: "FN-675-cache", baseCommitSha: "cachebase" }));
mockExecSync.mockReturnValue("cached/file.ts\n" as any);
mockExecSync.mockImplementation((command) => {
if (String(command) === "git merge-base --is-ancestor cachebase HEAD") {
return "" as any;
}
if (String(command) === "git diff --name-only cachebase..HEAD") {
return "cached/file.ts\n" as any;
}
throw new Error(`Unexpected command: ${String(command)}`);
});
const handler = await getSessionFilesHandler(store);
const first = await requestSessionFilesWithHandler(handler, "FN-675-cache");
@@ -225,12 +282,12 @@ describe("GET /api/tasks/:id/session-files", () => {
expect(first.body).toEqual(["cached/file.ts"]);
expect(second.body).toEqual(["cached/file.ts"]);
expect(mockExecSync).toHaveBeenCalledTimes(1);
expect(mockExecSync).toHaveBeenCalledTimes(2);
vi.advanceTimersByTime(10001);
const third = await requestSessionFilesWithHandler(handler, "FN-675-cache");
expect(third.body).toEqual(["cached/file.ts"]);
expect(mockExecSync).toHaveBeenCalledTimes(2);
expect(mockExecSync).toHaveBeenCalledTimes(4);
});
});

View File

@@ -1927,6 +1927,18 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
try {
let baseRef = task.baseCommitSha;
if (baseRef) {
try {
nodeChildProcess.execSync(`git merge-base --is-ancestor ${baseRef} HEAD`, {
cwd: task.worktree,
stdio: "pipe",
timeout: 5000,
});
} catch {
baseRef = undefined;
}
}
if (!baseRef) {
try {
baseRef = nodeChildProcess.execSync("git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main", {

View File

@@ -1318,6 +1318,48 @@ describe("TaskExecutor worktree pool integration", () => {
expect(pool.size).toBe(0);
});
it("overwrites baseCommitSha when starting from a pooled worktree", async () => {
const pool = new WorktreePool();
pool.release("/tmp/test/.worktrees/idle-wt");
mockedExistsSync.mockImplementation((p) => p === "/tmp/test/.worktrees/idle-wt");
mockedExecSync.mockImplementation((cmd: any) => {
if (String(cmd) === "git rev-parse HEAD") {
return "newbase123\n" as any;
}
return "" as any;
});
const store = createMockStore();
store.getTask.mockResolvedValue({
id: "FN-020",
title: "Test",
description: "Test",
column: "in-progress",
dependencies: [],
steps: [],
currentStep: 0,
log: [],
prompt: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check",
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
baseCommitSha: "stale-base",
});
store.getSettings.mockResolvedValue({
maxConcurrent: 2,
maxWorktrees: 4,
pollIntervalMs: 15000,
groupOverlappingFiles: false,
autoMerge: false,
recycleWorktrees: true,
});
const executor = new TaskExecutor(store, "/tmp/test", { pool });
await executor.execute(makeTask());
expect(store.updateTask).toHaveBeenCalledWith("FN-020", { baseCommitSha: "newbase123" });
});
it("creates fresh worktree when pool is empty", async () => {
const pool = new WorktreePool();
// Pool is empty

View File

@@ -503,9 +503,10 @@ export class TaskExecutor {
worktreePath = await this.createWorktree(branchName, worktreePath, task.id);
}
// Capture the base commit SHA for diff computation
// This is done after worktree creation when we're on the new branch
if (!task.baseCommitSha) {
// Capture the base commit SHA for diff computation whenever a task
// starts with a newly assigned worktree. Recycled worktrees must
// overwrite any prior task baseline instead of inheriting it.
if (!isResume) {
try {
const baseCommitSha = execSync("git rev-parse HEAD", {
cwd: worktreePath,

View File

@@ -138,6 +138,11 @@ describe("WorktreePool", () => {
it("creates branch from main with force-reset", () => {
pool.prepareForTask("/tmp/wt", "fusion/fn-042");
expect(mockedExecSync).toHaveBeenCalledWith(
"git checkout --detach main",
expect.objectContaining({ cwd: "/tmp/wt" }),
);
const checkoutCall = mockedExecSync.mock.calls.find(
(c) => typeof c[0] === "string" && (c[0] as string).includes("checkout -B"),
);
@@ -157,6 +162,11 @@ describe("WorktreePool", () => {
it("creates branch from custom startPoint when provided", () => {
pool.prepareForTask("/tmp/wt", "fusion/fn-042", "fusion/fn-041");
expect(mockedExecSync).toHaveBeenCalledWith(
"git checkout --detach fusion/fn-041",
expect.objectContaining({ cwd: "/tmp/wt" }),
);
const checkoutCall = mockedExecSync.mock.calls.find(
(c) => typeof c[0] === "string" && (c[0] as string).includes("checkout -B"),
);
@@ -176,6 +186,7 @@ describe("WorktreePool", () => {
// Should still run clean and branch creation
const calls = mockedExecSync.mock.calls.map((c) => c[0]);
expect(calls).toContain("git clean -fd");
expect(calls).toContain("git checkout --detach main");
expect(calls).toContain('git checkout -B "fusion/fn-001" main');
});

View File

@@ -108,7 +108,8 @@ export class WorktreePool {
* Steps performed:
* 1. `git checkout -- .` — discard tracked file modifications
* 2. `git clean -fd` — remove untracked files (but not .gitignore'd caches)
* 3. `git checkout -B <branchName> <startPoint>` — create/reset branch from start point
* 3. `git checkout --detach <startPoint>` — move HEAD to the latest base commit
* 4. `git checkout -B <branchName> <startPoint>` — create/reset branch from start point
*
* Returns the actual branch name used. This may differ from `branchName`
* when conflict recovery generates a suffixed name (e.g., `kb/fn-042-2`).
@@ -129,8 +130,13 @@ export class WorktreePool {
// Remove untracked files (but not .gitignore'd build caches)
execSync("git clean -fd", { cwd: worktreePath, stdio: "pipe" });
// Create or force-reset the branch from the start point (or main)
const base = startPoint || "main";
execSync(`git checkout --detach ${base}`, {
cwd: worktreePath,
stdio: "pipe",
});
// Create or force-reset the branch from the start point (or main)
const checkoutCmd = `git checkout -B "${branchName}" ${base}`;
try {
execSync(checkoutCmd, {