feat(HAI-033): add dependency-chain worktree reuse and conditional cleanup
- Add executor logic to resolve and reuse dependency worktrees for warm build caches - Add reuseWorktree method that creates a new branch in an existing worktree - Add findWorktreeUser helper to check if a worktree is shared across tasks - Update executor and merger cleanup to skip worktree removal when still in use - Add comprehensive tests for worktree reuse and conditional cleanup paths
This commit is contained in:
Dustin Byrne
@@ -8,6 +8,9 @@ vi.mock("./pi.js", () => ({
|
||||
vi.mock("./reviewer.js", () => ({
|
||||
reviewStep: vi.fn(),
|
||||
}));
|
||||
vi.mock("./merger.js", () => ({
|
||||
findWorktreeUser: vi.fn().mockResolvedValue(null),
|
||||
}));
|
||||
|
||||
// Mock node modules used by executor
|
||||
vi.mock("node:child_process", () => ({
|
||||
@@ -20,6 +23,8 @@ vi.mock("node:fs", () => ({
|
||||
import { TaskExecutor } from "./executor.js";
|
||||
import { createHaiAgent } from "./pi.js";
|
||||
import { execSync } from "node:child_process";
|
||||
import { findWorktreeUser } from "./merger.js";
|
||||
import type { Column, Task } from "@hai/core";
|
||||
|
||||
const mockedCreateHaiAgent = vi.mocked(createHaiAgent);
|
||||
|
||||
@@ -310,3 +315,248 @@ describe("TaskExecutor worktreeInitCommand", () => {
|
||||
expect(store.getSettings).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
const mockedFindWorktreeUser = vi.mocked(findWorktreeUser);
|
||||
|
||||
describe("TaskExecutor worktree reuse", () => {
|
||||
const makeTask = (overrides: Partial<Task> = {}): Task => ({
|
||||
id: "HAI-020",
|
||||
title: "Dependent task",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
mockedFindWorktreeUser.mockResolvedValue(null);
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
});
|
||||
|
||||
it("reuses dependency worktree when dep has existing worktree on disk", async () => {
|
||||
const store = createMockStore();
|
||||
const depWorktreePath = "/tmp/test/.worktrees/HAI-019";
|
||||
|
||||
// Dep task is in-review with an existing worktree
|
||||
store.listTasks.mockResolvedValue([
|
||||
makeTask({
|
||||
id: "HAI-019",
|
||||
column: "in-review",
|
||||
worktree: depWorktreePath,
|
||||
dependencies: [],
|
||||
}),
|
||||
makeTask({
|
||||
id: "HAI-020",
|
||||
column: "in-progress",
|
||||
dependencies: ["HAI-019"],
|
||||
}),
|
||||
]);
|
||||
|
||||
// existsSync: dep worktree exists on disk
|
||||
mockedExistsSync.mockImplementation((p: any) => {
|
||||
return p === depWorktreePath;
|
||||
});
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(makeTask({ id: "HAI-020", dependencies: ["HAI-019"] }));
|
||||
|
||||
// Should call `git checkout -b` (reuse), NOT `git worktree add`
|
||||
const checkoutCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git checkout -b"),
|
||||
);
|
||||
expect(checkoutCall).toBeDefined();
|
||||
expect(checkoutCall![0]).toContain("hai/hai-020");
|
||||
|
||||
const worktreeAddCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git worktree add"),
|
||||
);
|
||||
expect(worktreeAddCall).toBeUndefined();
|
||||
|
||||
// Task's worktree should be set to the reused path
|
||||
expect(store.updateTask).toHaveBeenCalledWith("HAI-020", { worktree: depWorktreePath });
|
||||
});
|
||||
|
||||
it("creates fresh worktree when dependency worktree does NOT exist on disk", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
// Dep is done but worktree was removed (cleared on done)
|
||||
store.listTasks.mockResolvedValue([
|
||||
makeTask({ id: "HAI-019", column: "done", dependencies: [] }),
|
||||
makeTask({ id: "HAI-020", column: "in-progress", dependencies: ["HAI-019"] }),
|
||||
]);
|
||||
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(makeTask({ id: "HAI-020", dependencies: ["HAI-019"] }));
|
||||
|
||||
// Should call `git worktree add`, NOT `git checkout -b`
|
||||
const worktreeAddCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git worktree add"),
|
||||
);
|
||||
expect(worktreeAddCall).toBeDefined();
|
||||
|
||||
const checkoutCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git checkout -b"),
|
||||
);
|
||||
expect(checkoutCall).toBeUndefined();
|
||||
});
|
||||
|
||||
it("creates fresh worktree when task has NO dependencies", async () => {
|
||||
const store = createMockStore();
|
||||
store.listTasks.mockResolvedValue([]);
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(makeTask({ id: "HAI-020", dependencies: [] }));
|
||||
|
||||
const worktreeAddCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git worktree add"),
|
||||
);
|
||||
expect(worktreeAddCall).toBeDefined();
|
||||
});
|
||||
|
||||
it("does NOT run worktreeInitCommand when reusing a worktree", async () => {
|
||||
const store = createMockStore();
|
||||
const depWorktreePath = "/tmp/test/.worktrees/HAI-019";
|
||||
|
||||
store.listTasks.mockResolvedValue([
|
||||
makeTask({
|
||||
id: "HAI-019",
|
||||
column: "in-review",
|
||||
worktree: depWorktreePath,
|
||||
dependencies: [],
|
||||
}),
|
||||
]);
|
||||
|
||||
store.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: false,
|
||||
worktreeInitCommand: "pnpm install",
|
||||
});
|
||||
|
||||
mockedExistsSync.mockImplementation((p: any) => p === depWorktreePath);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(makeTask({ id: "HAI-020", dependencies: ["HAI-019"] }));
|
||||
|
||||
// Init command should NOT have been run
|
||||
const initCall = mockedExecSync.mock.calls.find(
|
||||
(call) => call[0] === "pnpm install",
|
||||
);
|
||||
expect(initCall).toBeUndefined();
|
||||
});
|
||||
|
||||
it("resolveDependencyWorktree picks first dependency with existing worktree", async () => {
|
||||
const store = createMockStore();
|
||||
const depAPath = "/tmp/test/.worktrees/HAI-018";
|
||||
const depBPath = "/tmp/test/.worktrees/HAI-019";
|
||||
|
||||
// Two deps: A has no worktree on disk, B does
|
||||
store.listTasks.mockResolvedValue([
|
||||
makeTask({ id: "HAI-018", column: "in-review" as Column, worktree: depAPath, dependencies: [] }),
|
||||
makeTask({ id: "HAI-019", column: "in-review" as Column, worktree: depBPath, dependencies: [] }),
|
||||
]);
|
||||
|
||||
mockedExistsSync.mockImplementation((p: any) => p === depBPath);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
await executor.execute(makeTask({ id: "HAI-020", dependencies: ["HAI-018", "HAI-019"] }));
|
||||
|
||||
// Should reuse HAI-019's worktree (the one that exists)
|
||||
expect(store.updateTask).toHaveBeenCalledWith("HAI-020", { worktree: depBPath });
|
||||
|
||||
const checkoutCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git checkout -b"),
|
||||
);
|
||||
expect(checkoutCall).toBeDefined();
|
||||
expect(checkoutCall![1]).toMatchObject({ cwd: depBPath });
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskExecutor cleanup — chain-aware", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
mockedFindWorktreeUser.mockResolvedValue(null);
|
||||
mockedCreateHaiAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
});
|
||||
|
||||
it("does NOT remove worktree if another task still uses it", async () => {
|
||||
const store = createMockStore();
|
||||
mockedFindWorktreeUser.mockResolvedValue("HAI-021");
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
|
||||
// Execute a task to register a worktree
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
await executor.execute({
|
||||
id: "HAI-020",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress" as const,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
// Now cleanup — should skip removal because findWorktreeUser returns "HAI-021"
|
||||
await executor.cleanup("HAI-020");
|
||||
|
||||
const removeCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git worktree remove"),
|
||||
);
|
||||
expect(removeCall).toBeUndefined();
|
||||
});
|
||||
|
||||
it("removes worktree when no other task uses it", async () => {
|
||||
const store = createMockStore();
|
||||
mockedFindWorktreeUser.mockResolvedValue(null);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
await executor.execute({
|
||||
id: "HAI-020",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress" as const,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
await executor.cleanup("HAI-020");
|
||||
|
||||
const removeCall = mockedExecSync.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("git worktree remove"),
|
||||
);
|
||||
expect(removeCall).toBeDefined();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user