root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(FN-343): address PR review feedback

Browse Source
This commit is contained in:
Phil Larson
2026-06-11 19:55:58 -07:00
parent 2c8e9c6439
commit 16d71e6773
6 changed files with 141 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
packages/dashboard/app/components/__tests__/workflow-flow-mapping.test.ts
View File

@@ -198,6 +198,35 @@ describe("workflow-flow-mapping v2 round-trip", () => {
expect(byId.j1.config?.onBranchFailure).toBe("fail-fast");
});
it("preserves aliased IR node kinds when round-tripping through editor render kinds", () => {
const ir: WorkflowDefinition["ir"] = {
version: "v2",
name: "merge aliases",
columns: [{ id: "in-progress", name: "In progress", traits: [] }],
nodes: [
{ id: "gate", kind: "merge-gate", column: "in-progress", config: { name: "Gate" } },
{ id: "attempt", kind: "merge-attempt", column: "in-progress" },
{ id: "hold", kind: "manual-merge-hold", column: "in-progress", config: { release: "manual" } },
{ id: "retry", kind: "retry-backoff", column: "in-progress", config: { maxIterations: 2, template: { nodes: [], edges: [] } } },
] as WorkflowDefinition["ir"]["nodes"],
edges: [],
};
const { nodes, edges } = irToFlow(v2Def(ir));
expect(nodes.find((node) => node.id === "gate")?.type).toBe("merge");
expect(nodes.find((node) => node.id === "hold")?.type).toBe("hold");
expect(nodes.find((node) => node.id === "retry")?.type).toBe("loop");
const { ir: out } = flowToIr("merge aliases", nodes, edges, columnsOf(v2Def(ir)));
if (out.version !== "v2") throw new Error("expected v2");
const byId = Object.fromEntries(out.nodes.map((node) => [node.id, node]));
expect(byId.gate.kind).toBe("merge-gate");
expect(byId.attempt.kind).toBe("merge-attempt");
expect(byId.hold.kind).toBe("manual-merge-hold");
expect(byId.retry.kind).toBe("retry-backoff");
});
it("emits swimlane band group nodes that flowToIr strips back out", () => {
const { nodes } = irToFlow(v2Def(ir));
const bands = nodes.filter((n) => isColumnBandNode(n.id));

25
packages/dashboard/app/components/workflow-flow-mapping.ts
View File

@@ -188,6 +188,14 @@ function nodeLabel(node: WorkflowIr["nodes"][number]): string {
return node.id;
}
function dataIrKind(node: WorkflowIrNode, editorNodeKind: WorkflowEditorNodeKind): Partial<WorkflowFlowNodeData> {
return node.kind === editorNodeKind ? {} : { irKind: node.kind };
}
function preservedIrKind(data: WorkflowFlowNodeData): WorkflowIrNode["kind"] | undefined {
return typeof data.irKind === "string" ? (data.irKind as WorkflowIrNode["kind"]) : undefined;
}
/** Build React Flow swimlane band group nodes from the workflow's columns. */
export function columnsToBandNodes(columns: WorkflowIrColumn[]): FlowNode<WorkflowFlowNodeData>[] {
return columns.map((col, index): FlowNode<WorkflowFlowNodeData> => ({
@@ -313,7 +321,7 @@ export function irToFlow(def: WorkflowDefinition): {
position: childPos,
parentId: node.id,
extent: "parent",
data: { kind: innerKind, label: nodeLabel(inner), config: { ...(inner.config ?? {}) } },
data: { kind: innerKind, ...dataIrKind(inner, innerKind), label: nodeLabel(inner), config: { ...(inner.config ?? {}) } },
deletable: true,
zIndex: WF_STEP_NODE_Z_INDEX,
});
@@ -329,6 +337,7 @@ export function irToFlow(def: WorkflowDefinition): {
position: pos ?? { x: 80 + index * 180, y: fallbackY },
data: {
kind,
...dataIrKind(node, kind),
label: nodeLabel(node),
config: { ...restCfg },
column,
@@ -346,6 +355,7 @@ export function irToFlow(def: WorkflowDefinition): {
position: pos ?? { x: 80 + index * 180, y: fallbackY },
data: {
kind,
...dataIrKind(node, kind),
label: nodeLabel(node),
config: { ...(node.config ?? {}) },
column,
@@ -418,7 +428,11 @@ export function flowToIr(
function toIrNode(node: FlowNode<WorkflowFlowNodeData>, localId: string): WorkflowIrNode {
const data = node.data;
const config = nodeConfig(node);
const originalKind = preservedIrKind(data);
if (data.kind === "merge") {
if (originalKind) {
return { id: localId, kind: originalKind, config: config && Object.keys(config).length ? config : undefined };
}
return { id: localId, kind: "prompt", config: { ...(config ?? {}), seam: "merge" } };
}
if (data.kind === "foreach" || data.kind === "loop") {
@@ -436,13 +450,13 @@ export function flowToIr(
const baseCfg = (config ?? {}) as Record<string, unknown>;
return {
id: localId,
kind: data.kind,
kind: originalKind ?? data.kind,
config: { ...baseCfg, template: { nodes: templateNodes, edges: templateEdges } },
};
}
return {
id: localId,
kind: data.kind as WorkflowIrNode["kind"],
kind: originalKind ?? (data.kind as WorkflowIrNode["kind"]),
config: config && Object.keys(config).length ? config : undefined,
};
}
@@ -988,7 +1002,7 @@ function irNodeToFlowNode(
id,
type: kind,
position,
data: { kind, label: nodeLabel(node), config: { ...(node.config ?? {}) } },
data: { kind, ...dataIrKind(node, kind), label: nodeLabel(node), config: { ...(node.config ?? {}) } },
deletable: node.kind !== "start" && node.kind !== "end",
zIndex: WF_STEP_NODE_Z_INDEX,
};
@@ -1066,7 +1080,7 @@ export function insertFragment(
position: childPos,
parentId: id,
extent: "parent",
data: { kind: innerKind, label: nodeLabel(inner), config: { ...(inner.config ?? {}) } },
data: { kind: innerKind, ...dataIrKind(inner, innerKind), label: nodeLabel(inner), config: { ...(inner.config ?? {}) } },
deletable: true,
zIndex: WF_STEP_NODE_Z_INDEX,
});
@@ -1082,6 +1096,7 @@ export function insertFragment(
position: pos,
data: {
kind: groupKind,
...dataIrKind(node, groupKind),
label: nodeLabel(node),
config: { ...restCfg },
templateEmpty: template.nodes.length === 0,

54
packages/engine/src/__tests__/reliability-interactions/worktrunk-worktree-removal.test.ts
View File

@@ -22,6 +22,26 @@ vi.mock("node:fs", async (importOriginal) => {
return { ...actual, existsSync: existsSpy, readdirSync: readdirSpy };
});
function mockWorktreeRemoveFailure(postMergePath: string, porcelainOutput: string): void {
execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => {
if (cmd.includes("git worktree remove")) {
const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`;
cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr);
return;
}
if (cmd === "git worktree prune") {
cb(null, "", "");
return;
}
if (cmd === "git worktree list --porcelain") {
cb(null, porcelainOutput, "");
return;
}
cb(null, "", "");
});
}
function storeForSelfHealing(settings: Partial<Settings>, task: Partial<Task>): TaskStore & EventEmitter {
const emitter = new EventEmitter();
return Object.assign(emitter, {
@@ -59,22 +79,7 @@ describe("reliability interactions: worktrunk worktree removal routing", () => {
it("merger post-merge cleanup logs harmless classified temp residue when porcelain is absent after prune", async () => {
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234";
execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => {
if (cmd.includes("git worktree remove")) {
const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`;
cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr);
return;
}
if (cmd === "git worktree prune") {
cb(null, "", "");
return;
}
if (cmd === "git worktree list --porcelain") {
cb(null, "worktree /repo\nbranch refs/heads/main\n", "");
return;
}
cb(null, "", "");
});
mockWorktreeRemoveFailure(postMergePath, "worktree /repo\nbranch refs/heads/main\n");
await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {});
@@ -91,22 +96,7 @@ describe("reliability interactions: worktrunk worktree removal routing", () => {
it("merger post-merge cleanup keeps still-registered temp worktree failures visible", async () => {
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined);
const postMergePath = "/repo/.worktrees/post-merge-FN-343-abcd1234";
execSpy.mockImplementation((cmd: string, _opts: unknown, cb: (err: any, stdout: string, stderr: string) => void) => {
if (cmd.includes("git worktree remove")) {
const stderr = `fatal: validation failed, cannot remove working tree: '${postMergePath}/.git' is not a .git file, error code 2`;
cb(Object.assign(new Error(stderr), { stderr, status: 2 }), "", stderr);
return;
}
if (cmd === "git worktree prune") {
cb(null, "", "");
return;
}
if (cmd === "git worktree list --porcelain") {
cb(null, `worktree /repo\nbranch refs/heads/main\n\nworktree ${postMergePath}\nbranch refs/heads/fusion/fn-343\n`, "");
return;
}
cb(null, "", "");
});
mockWorktreeRemoveFailure(postMergePath, `worktree /repo\nbranch refs/heads/main\n\nworktree ${postMergePath}\nbranch refs/heads/fusion/fn-343\n`);
await mergerTestHooks.removePostMergeWorktree("/repo", postMergePath, "FN-343", {});

39
packages/engine/src/__tests__/worktree-backend.test.ts
View File

@@ -1032,6 +1032,45 @@ describe("removeWorktree", () => {
);
});
it("preserves the original remove failure when classification probes fail", async () => {
const tempPath = "/var/folders/demo/T/fusion-ai-merge-fn-327-A5uY3j";
const validationError = {
message: `Command failed: git worktree remove --force ${tempPath}`,
stderr: `fatal: validation failed, cannot remove working tree: '${tempPath}/.git' is not a .git file, error code 2`,
status: 2,
};
const probeError = new Error("git worktree prune failed");
execMock
.mockRejectedValueOnce(validationError)
.mockRejectedValueOnce(probeError);
const audit = { git: vi.fn().mockResolvedValue(undefined) } as any;
await expect(
removeWorktree({
rootDir: "/repo",
worktreePath: tempPath,
settings: {},
audit,
taskId: "FN-327",
reason: RemovalReason.MergerCleanup,
}),
).rejects.toBe(validationError);
expect(execMock).toHaveBeenNthCalledWith(2, "git worktree prune", expect.objectContaining({ cwd: "/repo" }));
expect(audit.git).toHaveBeenCalledWith(
expect.objectContaining({
type: "worktree:remove-classification-probe-failed",
target: tempPath,
metadata: expect.objectContaining({
reason: RemovalReason.MergerCleanup,
stderrPreview: expect.stringContaining("is not a .git file"),
probeError: expect.stringContaining("git worktree prune failed"),
}),
}),
);
});
it("uses worktrunk remove and emits worktree:worktrunk-remove", async () => {
execMock.mockResolvedValue({ stdout: "", stderr: "" });
const audit = { git: vi.fn().mockResolvedValue(undefined) } as any;

1
packages/engine/src/run-audit.ts
View File

@@ -93,6 +93,7 @@ export type GitMutationType =
| "worktree:remove"
| "worktree:remove-fallback"
| "worktree:remove-classified-harmless"
| "worktree:remove-classification-probe-failed"
| "worktree:remove-leaked-registered-worktree"
| "worktree:reuse"
| "worktree:incomplete-detected"

43
packages/engine/src/worktree-backend.ts
View File

@@ -95,20 +95,37 @@ async function classifyHarmlessMergeRemoveFailure(input: {
const pathExists = existsSync(input.worktreePath);
const gitFileExists = existsSync(resolve(input.worktreePath, ".git"));
await execAsync("git worktree prune", {
cwd: input.rootDir,
encoding: "utf-8",
timeout: NATIVE_TIMEOUT_MS,
maxBuffer: MAX_BUFFER,
});
let stdout: string;
try {
await execAsync("git worktree prune", {
cwd: input.rootDir,
encoding: "utf-8",
timeout: NATIVE_TIMEOUT_MS,
maxBuffer: MAX_BUFFER,
});
const listResult = await execAsync("git worktree list --porcelain", {
cwd: input.rootDir,
encoding: "utf-8",
timeout: 10_000,
maxBuffer: MAX_BUFFER,
});
const stdout = typeof listResult === "string" ? listResult : String(listResult.stdout ?? "");
const listResult = await execAsync("git worktree list --porcelain", {
cwd: input.rootDir,
encoding: "utf-8",
timeout: 10_000,
maxBuffer: MAX_BUFFER,
});
stdout = typeof listResult === "string" ? listResult : String(listResult.stdout ?? "");
} catch (probeError) {
await input.audit?.git({
type: "worktree:remove-classification-probe-failed",
target: input.worktreePath,
metadata: {
taskId: input.taskId,
reason: input.reason,
stderrPreview,
probeError: previewError(probeError),
pathExists,
gitFileExists,
},
});
return null;
}
const registeredAfterPrune = porcelainContainsWorktree(stdout, input.worktreePath);
if (registeredAfterPrune) {