fix(FN-4698): harden worktrunk path resolution and coverage
Fusion-Task-Id: FN-4698 Fusion-Task-Lineage: 267131ce-b02a-4e54-8d71-c949e3b55e56
This commit is contained in:
committed by
gsxdsm
parent
b98afd4ea3
commit
6e74f6366c
@@ -1,12 +1,13 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
|
||||
const { execMock } = vi.hoisted(() => {
|
||||
const { execMock, existsSyncMock } = vi.hoisted(() => {
|
||||
const mock = vi.fn();
|
||||
(mock as any)[Symbol.for("nodejs.util.promisify.custom")] = mock;
|
||||
return { execMock: mock };
|
||||
return { execMock: mock, existsSyncMock: vi.fn() };
|
||||
});
|
||||
|
||||
vi.mock("node:child_process", () => ({ exec: execMock }));
|
||||
vi.mock("node:fs", () => ({ existsSync: existsSyncMock }));
|
||||
vi.mock("../worktree-pool.js", async () => {
|
||||
const actual = await vi.importActual<any>("../worktree-pool.js");
|
||||
return { ...actual, isUsableTaskWorktree: vi.fn().mockResolvedValue(true) };
|
||||
@@ -43,6 +44,8 @@ const makeAudit = () => {
|
||||
|
||||
beforeEach(() => {
|
||||
execMock.mockReset();
|
||||
existsSyncMock.mockReset();
|
||||
existsSyncMock.mockReturnValue(true);
|
||||
});
|
||||
|
||||
describe("acquireTaskWorktree worktrunk wiring", () => {
|
||||
@@ -79,7 +82,17 @@ describe("acquireTaskWorktree worktrunk wiring", () => {
|
||||
});
|
||||
|
||||
it("emits worktrunk + native create audits when worktrunk succeeds", async () => {
|
||||
execMock.mockResolvedValue({ stdout: "", stderr: "" });
|
||||
execMock.mockImplementation((command: string) => {
|
||||
if (command.includes('"config" "show"')) return Promise.resolve({ stdout: "", stderr: "" });
|
||||
if (command.includes('"switch" "--create"')) return Promise.resolve({ stdout: "", stderr: "" });
|
||||
if (command === "git worktree list --porcelain") {
|
||||
return Promise.resolve({
|
||||
stdout: "worktree /repo/.worktrees/fusion/fn-1\nbranch refs/heads/fusion/fn-1\n",
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
return Promise.resolve({ stdout: "", stderr: "" });
|
||||
});
|
||||
const { acquireTaskWorktree } = await import("../worktree-acquisition.js");
|
||||
const { audit, events } = makeAudit();
|
||||
|
||||
@@ -96,6 +109,36 @@ describe("acquireTaskWorktree worktrunk wiring", () => {
|
||||
expect(events.filter((event) => event.type === "worktree:create")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("propagates resolved worktrunk path into result and task store", async () => {
|
||||
const store = makeStore();
|
||||
execMock.mockImplementation((command: string) => {
|
||||
if (command.includes('"config" "show"')) return Promise.resolve({ stdout: "", stderr: "" });
|
||||
if (command.includes('"switch" "--create"')) return Promise.resolve({ stdout: "", stderr: "" });
|
||||
if (command === "git worktree list --porcelain") {
|
||||
return Promise.resolve({
|
||||
stdout: "worktree /repo/.worktrees/custom/fusion-fn-1\nbranch refs/heads/fusion/fn-1\n",
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
return Promise.resolve({ stdout: "", stderr: "" });
|
||||
});
|
||||
existsSyncMock.mockImplementation((path: string) => path === "/repo/.worktrees/custom/fusion-fn-1");
|
||||
const { acquireTaskWorktree } = await import("../worktree-acquisition.js");
|
||||
|
||||
const result = await acquireTaskWorktree({
|
||||
task,
|
||||
rootDir: "/repo",
|
||||
store: store as any,
|
||||
settings: { worktrunk: { enabled: true, binaryPath: "worktrunk", onFailure: "fail" } } as any,
|
||||
});
|
||||
|
||||
expect(result.worktreePath).toBe("/repo/.worktrees/custom/fusion-fn-1");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-1", {
|
||||
worktree: "/repo/.worktrees/custom/fusion-fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
});
|
||||
});
|
||||
|
||||
it("fails hard without fallback when onFailure=fail", async () => {
|
||||
execMock.mockRejectedValue({ stderr: "nope", status: 9 });
|
||||
const { acquireTaskWorktree } = await import("../worktree-acquisition.js");
|
||||
|
||||
@@ -7,13 +7,14 @@ import {
|
||||
resolveWorktreeBackend,
|
||||
} from "../worktree-backend.js";
|
||||
|
||||
const { execMock, accessMock } = vi.hoisted(() => {
|
||||
const { execMock, accessMock, existsSyncMock } = vi.hoisted(() => {
|
||||
const mock = vi.fn();
|
||||
(mock as any)[Symbol.for("nodejs.util.promisify.custom")] = mock;
|
||||
return { execMock: mock, accessMock: vi.fn() };
|
||||
return { execMock: mock, accessMock: vi.fn(), existsSyncMock: vi.fn() };
|
||||
});
|
||||
|
||||
vi.mock("node:child_process", () => ({ exec: execMock }));
|
||||
vi.mock("node:fs", () => ({ existsSync: existsSyncMock }));
|
||||
vi.mock("node:fs/promises", () => ({ access: accessMock }));
|
||||
vi.mock("../branch-conflicts.js", () => ({
|
||||
inspectBranchConflict: vi.fn().mockResolvedValue({ kind: "stale" }),
|
||||
@@ -22,7 +23,9 @@ vi.mock("../branch-conflicts.js", () => ({
|
||||
beforeEach(() => {
|
||||
execMock.mockReset();
|
||||
accessMock.mockReset();
|
||||
existsSyncMock.mockReset();
|
||||
accessMock.mockResolvedValue(undefined);
|
||||
existsSyncMock.mockReturnValue(true);
|
||||
});
|
||||
|
||||
describe("NativeWorktreeBackend", () => {
|
||||
@@ -156,7 +159,10 @@ describe("WorktrunkWorktreeBackend", () => {
|
||||
it("invokes create mapping with timeout/maxBuffer and cwd", async () => {
|
||||
execMock
|
||||
.mockResolvedValueOnce({ stdout: "", stderr: "" })
|
||||
.mockResolvedValueOnce({ stdout: "worktree /repo/.worktrees/fusion/fn-1\n", stderr: "" });
|
||||
.mockResolvedValueOnce({
|
||||
stdout: "worktree /repo/.worktrees/fusion/fn-1\nbranch refs/heads/fusion/fn-1\n",
|
||||
stderr: "",
|
||||
});
|
||||
const backend = new WorktrunkWorktreeBackend({ binaryPath: "worktrunk" });
|
||||
|
||||
await backend.create({
|
||||
@@ -174,6 +180,118 @@ describe("WorktrunkWorktreeBackend", () => {
|
||||
);
|
||||
});
|
||||
|
||||
describe("create() — path resolution", () => {
|
||||
it("returns porcelain-resolved path and warns on drift", async () => {
|
||||
const logger = { log: vi.fn(), warn: vi.fn() };
|
||||
execMock
|
||||
.mockResolvedValueOnce({ stdout: "", stderr: "" })
|
||||
.mockResolvedValueOnce({
|
||||
stdout:
|
||||
"worktree /repo/.worktrees/custom/fusion-fn-1\nbranch refs/heads/fusion/fn-1\n\nworktree /repo\nbranch refs/heads/main\n",
|
||||
stderr: "",
|
||||
});
|
||||
existsSyncMock.mockImplementation((path: string) => path === "/repo/.worktrees/custom/fusion-fn-1");
|
||||
|
||||
const result = await new WorktrunkWorktreeBackend({ binaryPath: "worktrunk", logger }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
});
|
||||
|
||||
expect(result).toEqual({ path: "/repo/.worktrees/custom/fusion-fn-1", branch: "fusion/fn-1" });
|
||||
expect(logger.warn).toHaveBeenCalledTimes(1);
|
||||
expect(logger.warn).toHaveBeenCalledWith(
|
||||
"[worktree-backend] worktrunk created branch fusion/fn-1 at /repo/.worktrees/custom/fusion-fn-1 (fusion assumed /repo/.worktrees/fn-1); using worktrunk-assigned path",
|
||||
);
|
||||
});
|
||||
|
||||
it("fails when no branch match exists", async () => {
|
||||
execMock
|
||||
.mockResolvedValueOnce({ stdout: "", stderr: "" })
|
||||
.mockResolvedValueOnce({ stdout: "worktree /repo/.worktrees/other\nbranch refs/heads/other\n", stderr: "" });
|
||||
|
||||
await expect(
|
||||
new WorktrunkWorktreeBackend({ binaryPath: "worktrunk" }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
name: "WorktrunkOperationError",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: expect.stringContaining("fusion/fn-1"),
|
||||
});
|
||||
});
|
||||
|
||||
it("fails when multiple branch matches exist", async () => {
|
||||
execMock
|
||||
.mockResolvedValueOnce({ stdout: "", stderr: "" })
|
||||
.mockResolvedValueOnce({
|
||||
stdout:
|
||||
"worktree /repo/.worktrees/a\nbranch refs/heads/fusion/fn-1\n\nworktree /repo/.worktrees/b\nbranch refs/heads/fusion/fn-1\n",
|
||||
stderr: "",
|
||||
});
|
||||
|
||||
await expect(
|
||||
new WorktrunkWorktreeBackend({ binaryPath: "worktrunk" }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
name: "WorktrunkOperationError",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: expect.stringContaining("/repo/.worktrees/a, /repo/.worktrees/b"),
|
||||
});
|
||||
});
|
||||
|
||||
it("fails when resolved path does not exist on disk", async () => {
|
||||
existsSyncMock.mockReturnValue(false);
|
||||
execMock
|
||||
.mockResolvedValueOnce({ stdout: "", stderr: "" })
|
||||
.mockResolvedValueOnce({
|
||||
stdout: "worktree /repo/.worktrees/missing\nbranch refs/heads/fusion/fn-1\n",
|
||||
stderr: "",
|
||||
});
|
||||
|
||||
await expect(
|
||||
new WorktrunkWorktreeBackend({ binaryPath: "worktrunk" }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
name: "WorktrunkOperationError",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: "worktrunk reported worktree at /repo/.worktrees/missing but the path does not exist",
|
||||
});
|
||||
});
|
||||
|
||||
it("wraps porcelain command failures as worktrunk operation errors", async () => {
|
||||
execMock
|
||||
.mockResolvedValueOnce({ stdout: "", stderr: "" })
|
||||
.mockRejectedValueOnce({ stderr: "porcelain failed", status: 2 });
|
||||
|
||||
await expect(
|
||||
new WorktrunkWorktreeBackend({ binaryPath: "worktrunk" }).create({
|
||||
rootDir: "/repo",
|
||||
worktreePath: "/repo/.worktrees/fn-1",
|
||||
branch: "fusion/fn-1",
|
||||
taskId: "FN-1",
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
name: "WorktrunkOperationError",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: "porcelain failed",
|
||||
exitCode: 2,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it("invokes remove mapping", async () => {
|
||||
execMock.mockResolvedValue({ stdout: "", stderr: "" });
|
||||
const backend = new WorktrunkWorktreeBackend({ binaryPath: "worktrunk" });
|
||||
|
||||
@@ -355,56 +355,56 @@ export class WorktrunkWorktreeBackend implements WorktreeBackend {
|
||||
|
||||
return { path: resolvedPath, branch: input.branch };
|
||||
}
|
||||
+
|
||||
+ private async resolveCreatedWorktreePath(input: { rootDir: string; branch: string }): Promise<string> {
|
||||
+ let rows: Array<{ path: string; branch?: string }>;
|
||||
+ try {
|
||||
+ const { stdout } = await execAsync("git worktree list --porcelain", {
|
||||
+ cwd: input.rootDir,
|
||||
+ encoding: "utf-8",
|
||||
+ timeout: 30_000,
|
||||
+ maxBuffer: MAX_BUFFER,
|
||||
+ });
|
||||
+ rows = parseWorktreesFromPorcelain(stdout);
|
||||
+ } catch (error) {
|
||||
+ throw new WorktrunkOperationError({
|
||||
+ operation: "create",
|
||||
+ code: "worktrunk_operation_failed",
|
||||
+ stderr: getErrorStderr(error) ?? String(error),
|
||||
+ exitCode: getErrorExitCode(error),
|
||||
+ });
|
||||
+ }
|
||||
+
|
||||
+ const matches = rows.filter((row) => row.branch === input.branch);
|
||||
+ if (matches.length === 0) {
|
||||
+ throw new WorktrunkOperationError({
|
||||
+ operation: "create",
|
||||
+ code: "worktrunk_operation_failed",
|
||||
+ stderr: `worktrunk created branch ${input.branch} but no registered worktree was found`,
|
||||
+ exitCode: null,
|
||||
+ });
|
||||
+ }
|
||||
+ if (matches.length > 1) {
|
||||
+ throw new WorktrunkOperationError({
|
||||
+ operation: "create",
|
||||
+ code: "worktrunk_operation_failed",
|
||||
+ stderr: `worktrunk created branch ${input.branch} but multiple registered worktrees claim it: ${matches.map((match) => match.path).join(", ")}`,
|
||||
+ exitCode: null,
|
||||
+ });
|
||||
+ }
|
||||
+
|
||||
+ const resolvedPath = matches[0]?.path;
|
||||
+ if (!resolvedPath || !existsSync(resolvedPath)) {
|
||||
+ throw new WorktrunkOperationError({
|
||||
+ operation: "create",
|
||||
+ code: "worktrunk_operation_failed",
|
||||
+ stderr: `worktrunk reported worktree at ${resolvedPath ?? "<unknown>"} but the path does not exist`,
|
||||
+ exitCode: null,
|
||||
+ });
|
||||
+ }
|
||||
+
|
||||
+ return resolvedPath;
|
||||
+ }
|
||||
|
||||
private async resolveCreatedWorktreePath(input: { rootDir: string; branch: string }): Promise<string> {
|
||||
let rows: Array<{ path: string; branch?: string }>;
|
||||
try {
|
||||
const { stdout } = await execAsync("git worktree list --porcelain", {
|
||||
cwd: input.rootDir,
|
||||
encoding: "utf-8",
|
||||
timeout: 30_000,
|
||||
maxBuffer: MAX_BUFFER,
|
||||
});
|
||||
rows = parseWorktreesFromPorcelain(stdout);
|
||||
} catch (error) {
|
||||
throw new WorktrunkOperationError({
|
||||
operation: "create",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: getErrorStderr(error) ?? String(error),
|
||||
exitCode: getErrorExitCode(error),
|
||||
});
|
||||
}
|
||||
|
||||
const matches = rows.filter((row) => row.branch === input.branch);
|
||||
if (matches.length === 0) {
|
||||
throw new WorktrunkOperationError({
|
||||
operation: "create",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: `worktrunk created branch ${input.branch} but no registered worktree was found`,
|
||||
exitCode: null,
|
||||
});
|
||||
}
|
||||
if (matches.length > 1) {
|
||||
throw new WorktrunkOperationError({
|
||||
operation: "create",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: `worktrunk created branch ${input.branch} but multiple registered worktrees claim it: ${matches.map((match) => match.path).join(", ")}`,
|
||||
exitCode: null,
|
||||
});
|
||||
}
|
||||
|
||||
const resolvedPath = matches[0]?.path;
|
||||
if (!resolvedPath || !existsSync(resolvedPath)) {
|
||||
throw new WorktrunkOperationError({
|
||||
operation: "create",
|
||||
code: "worktrunk_operation_failed",
|
||||
stderr: `worktrunk reported worktree at ${resolvedPath ?? "<unknown>"} but the path does not exist`,
|
||||
exitCode: null,
|
||||
});
|
||||
}
|
||||
|
||||
return resolvedPath;
|
||||
}
|
||||
|
||||
async remove(input: WorktreeRemoveInput): Promise<void> {
|
||||
const target = input.branch ?? input.worktreePath;
|
||||
|
||||
Reference in New Issue
Block a user