feat(FN-818): use merge commit diff for done-task Changes tab

- Refactor TaskChangesTab to use merge commit diff for done/archived tasks instead of branch diff
- Add CommitDiffTab component integration for displaying merge commit changes
- Remove legacy API endpoints and tests for branch-based diff on completed tasks
- Add comprehensive test suite for TaskChangesTab covering all task lifecycle states
- Document merge commit diff behavior in dashboard README
This commit is contained in:
gsxdsm
2026-04-03 23:04:13 -07:00
parent d2266466fd
commit 98bcb5df5c
6 changed files with 535 additions and 51 deletions

View File

@@ -9,7 +9,7 @@ interface CommitDiffTabProps {
mergeDetails?: MergeDetails;
}
interface ParsedFile {
export interface ParsedFile {
path: string;
status: "added" | "modified" | "deleted" | "unknown";
additions: number;
@@ -43,7 +43,7 @@ function getStatusLabel(status: ParsedFile["status"]): string {
}
}
function parsePatch(rawPatch: string): ParsedFile[] {
export function parsePatch(rawPatch: string): ParsedFile[] {
const files: ParsedFile[] = [];
// Split on diff boundaries, keeping the delimiter
const parts = rawPatch.split(/(?=^diff --git )/m);

View File

@@ -1,21 +1,16 @@
import { useState, useEffect, useCallback } from "react";
import { FileCode, ChevronDown, ChevronRight, AlertCircle } from "lucide-react";
import { fetchTaskDiff, type TaskDiff } from "../api";
import { FileCode, ChevronDown, ChevronRight, AlertCircle, GitCommit } from "lucide-react";
import type { MergeDetails, Column } from "@fusion/core";
import { fetchTaskDiff, fetchCommitDiff, type TaskDiff } from "../api";
import { parsePatch, type ParsedFile } from "./CommitDiffTab";
import { highlightDiff } from "../utils/highlightDiff";
interface TaskChangesTabProps {
taskId: string;
worktree?: string;
projectId?: string;
}
function getFileStatus(file: string, patch: string): "added" | "modified" | "deleted" | "unknown" {
if (patch.includes("diff --git")) {
if (patch.includes("new file mode")) return "added";
if (patch.includes("deleted file mode")) return "deleted";
return "modified";
}
return "unknown";
column?: Column;
mergeDetails?: MergeDetails;
}
function getStatusColor(status: "added" | "modified" | "deleted" | "unknown"): string {
@@ -31,19 +26,79 @@ function getStatusColor(status: "added" | "modified" | "deleted" | "unknown"): s
}
}
function formatFileSize(bytes: number): string {
if (bytes < 1024) return `${bytes} B`;
if (bytes < 1024 * 1024) return `${(bytes / 1024).toFixed(1)} KB`;
return `${(bytes / (1024 * 1024)).toFixed(1)} MB`;
function getStatusLabel(status: "added" | "modified" | "deleted" | "unknown"): string {
switch (status) {
case "added":
return "A";
case "deleted":
return "D";
case "modified":
return "M";
default:
return "?";
}
}
export function TaskChangesTab({ taskId, worktree, projectId }: TaskChangesTabProps) {
const [diffData, setDiffData] = useState<TaskDiff | null>(null);
/** Normalized file entry used by both worktree-backed and commit-backed paths */
interface NormalizedFile {
path: string;
status: "added" | "modified" | "deleted" | "unknown";
additions: number;
deletions: number;
patch: string;
}
/**
* TaskChangesTab displays file-level diffs for a task.
*
* For in-progress/in-review tasks it loads the diff from the live worktree.
* For done tasks with a recorded merge commit (mergeDetails.commitSha) it loads
* the diff from git history instead, so changes remain visible even after the
* worktree is cleaned up.
*/
export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetails }: TaskChangesTabProps) {
const [files, setFiles] = useState<NormalizedFile[]>([]);
const [stats, setStats] = useState<{ filesChanged: number; additions: number; deletions: number }>({ filesChanged: 0, additions: 0, deletions: 0 });
const [loading, setLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
const [expandedFiles, setExpandedFiles] = useState<Set<string>>(new Set());
const commitSha = column === "done" ? mergeDetails?.commitSha : undefined;
const useCommitDiff = !!commitSha;
const loadDiff = useCallback(async () => {
// Done task with merge commit → use commit-backed diff
if (useCommitDiff) {
try {
setLoading(true);
setError(null);
const data = await fetchCommitDiff(commitSha);
const parsed = parsePatch(data.patch || "");
const normalized: NormalizedFile[] = parsed.map((f) => ({
path: f.path,
status: f.status,
additions: f.additions,
deletions: f.deletions,
patch: f.patch,
}));
setFiles(normalized);
setStats({
filesChanged: mergeDetails?.filesChanged ?? normalized.length,
additions: mergeDetails?.insertions ?? normalized.reduce((s, f) => s + f.additions, 0),
deletions: mergeDetails?.deletions ?? normalized.reduce((s, f) => s + f.deletions, 0),
});
if (normalized.length > 0) {
setExpandedFiles(new Set([normalized[0].path]));
}
} catch (err: any) {
setError(err.message || "Failed to load commit diff");
} finally {
setLoading(false);
}
return;
}
// Non-done task → use worktree-backed diff
if (!worktree) {
setLoading(false);
return;
@@ -52,30 +107,37 @@ export function TaskChangesTab({ taskId, worktree, projectId }: TaskChangesTabPr
try {
setLoading(true);
setError(null);
const data = await fetchTaskDiff(taskId, undefined, projectId);
setDiffData(data);
// Auto-expand first file if there are files
if (data.files.length > 0) {
setExpandedFiles(new Set([data.files[0].path]));
const data: TaskDiff = await fetchTaskDiff(taskId, undefined, projectId);
const normalized: NormalizedFile[] = data.files.map((f) => ({
path: f.path,
status: f.status,
additions: f.additions,
deletions: f.deletions,
patch: f.patch,
}));
setFiles(normalized);
setStats(data.stats);
if (normalized.length > 0) {
setExpandedFiles(new Set([normalized[0].path]));
}
} catch (err: any) {
setError(err.message || "Failed to load diff");
} finally {
setLoading(false);
}
}, [taskId, worktree, projectId]);
}, [taskId, worktree, projectId, useCommitDiff, commitSha, mergeDetails]);
useEffect(() => {
loadDiff();
}, [loadDiff]);
const toggleFile = (file: string) => {
const toggleFile = (filePath: string) => {
setExpandedFiles((prev) => {
const next = new Set(prev);
if (next.has(file)) {
next.delete(file);
if (next.has(filePath)) {
next.delete(filePath);
} else {
next.add(file);
next.add(filePath);
}
return next;
});
@@ -103,7 +165,8 @@ export function TaskChangesTab({ taskId, worktree, projectId }: TaskChangesTabPr
);
}
if (!worktree) {
// Non-done task without a worktree → show worktree empty state
if (!useCommitDiff && !worktree) {
return (
<div className="detail-section">
<div className="task-changes-state task-changes-state--empty">
@@ -117,14 +180,16 @@ export function TaskChangesTab({ taskId, worktree, projectId }: TaskChangesTabPr
);
}
if (!diffData || diffData.files.length === 0) {
if (files.length === 0) {
return (
<div className="detail-section">
<div className="task-changes-state task-changes-state--empty">
<FileCode size={24} />
<p>No files modified.</p>
<span className="task-changes-state-hint">
The agent did not modify any files during execution.
{useCommitDiff
? "No file changes were recorded in the merge commit."
: "The agent did not modify any files during execution."}
</span>
</div>
</div>
@@ -133,10 +198,32 @@ export function TaskChangesTab({ taskId, worktree, projectId }: TaskChangesTabPr
return (
<div className="detail-section task-changes-tab">
{/* Commit metadata for done tasks */}
{useCommitDiff && mergeDetails && (
<div className="commit-diff-meta">
<div className="commit-diff-sha">
<GitCommit size={14} />
<code>{commitSha!.slice(0, 7)}</code>
</div>
{mergeDetails.mergeCommitMessage && (
<div className="commit-diff-message">{mergeDetails.mergeCommitMessage}</div>
)}
{mergeDetails.mergedAt && (
<div className="commit-diff-timestamp">
Merged {new Date(mergeDetails.mergedAt).toLocaleString()}
</div>
)}
</div>
)}
<div className="changes-header">
<h4>
<FileCode size={16} />
Modified Files ({diffData.files.length})
Files Changed ({stats.filesChanged})
<span className="changes-stat-summary">
<span className="diff-add">+{stats.additions}</span>{" "}
<span className="diff-del">-{stats.deletions}</span>
</span>
</h4>
<button
className="btn btn-sm"
@@ -148,46 +235,43 @@ export function TaskChangesTab({ taskId, worktree, projectId }: TaskChangesTabPr
</div>
<div className="changes-file-list">
{diffData.files.map((fileEntry) => {
const { path, status, patch } = fileEntry;
const isExpanded = expandedFiles.has(path);
{files.map((file) => {
const isExpanded = expandedFiles.has(file.path);
return (
<div
key={path}
key={file.path}
className={`changes-file-item ${isExpanded ? "expanded" : ""}`}
>
<button
className="changes-file-header"
onClick={() => toggleFile(path)}
onClick={() => toggleFile(file.path)}
>
<span className="changes-file-toggle">
{isExpanded ? <ChevronDown size={14} /> : <ChevronRight size={14} />}
</span>
<span
className="changes-file-status"
style={{ color: getStatusColor(status) }}
title={status}
style={{ color: getStatusColor(file.status) }}
title={file.status}
>
{status === "added" && "A"}
{status === "modified" && "M"}
{status === "deleted" && "D"}
{getStatusLabel(file.status)}
</span>
<span className="changes-file-path" title={path}>
{path}
<span className="changes-file-path" title={file.path}>
{file.path}
</span>
<span
className="changes-file-stat"
title={`+${fileEntry.additions} -${fileEntry.deletions}`}
title={`+${file.additions} -${file.deletions}`}
>
+{fileEntry.additions} -{fileEntry.deletions}
+{file.additions} -{file.deletions}
</span>
</button>
{isExpanded && patch && (
{isExpanded && file.patch && (
<div className="changes-file-content">
<pre className="changes-diff-patch">
<code>{highlightDiff(patch)}</code>
<code>{highlightDiff(file.patch)}</code>
</pre>
</div>
)}

View File

@@ -789,7 +789,7 @@ export function TaskDetailModal({
/>
</div>
) : activeTab === "changes" ? (
<TaskChangesTab taskId={task.id} worktree={task.worktree} projectId={projectId} />
<TaskChangesTab taskId={task.id} worktree={task.worktree} projectId={projectId} column={task.column} mergeDetails={task.mergeDetails} />
) : activeTab === "commits" ? (
<CommitDiffTab commitSha={task.mergeDetails?.commitSha ?? ""} mergeDetails={task.mergeDetails} />
) : activeTab === "comments" ? (

View File

@@ -0,0 +1,398 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
import { TaskChangesTab } from "../TaskChangesTab";
import type { MergeDetails, Column } from "@fusion/core";
const mockFetchTaskDiff = vi.fn();
const mockFetchCommitDiff = vi.fn();
vi.mock("../../api", () => ({
fetchTaskDiff: (...args: any[]) => mockFetchTaskDiff(...args),
fetchCommitDiff: (...args: any[]) => mockFetchCommitDiff(...args),
}));
vi.mock("lucide-react", () => ({
FileCode: () => null,
ChevronDown: ({ size }: any) => <span data-testid="chevron-down" />,
ChevronRight: ({ size }: any) => <span data-testid="chevron-right" />,
AlertCircle: () => null,
GitCommit: () => null,
}));
vi.mock("../../utils/highlightDiff", () => ({
highlightDiff: (diff: string) => diff,
}));
vi.mock("../CommitDiffTab", () => ({
parsePatch: (rawPatch: string) => {
const files: Array<{
path: string;
status: "added" | "modified" | "deleted" | "unknown";
additions: number;
deletions: number;
patch: string;
}> = [];
const parts = rawPatch.split(/(?=^diff --git )/m);
for (const part of parts) {
const trimmed = part.trim();
if (!trimmed.startsWith("diff --git ")) continue;
const headerMatch = trimmed.match(/^diff --git a\/(.+?) b\/(.+)/m);
const path = headerMatch ? headerMatch[2] : "unknown";
let status: "added" | "modified" | "deleted" | "unknown" = "modified";
if (trimmed.includes("new file mode")) status = "added";
else if (trimmed.includes("deleted file mode")) status = "deleted";
let additions = 0;
let deletions = 0;
for (const line of trimmed.split("\n")) {
if (line.startsWith("+") && !line.startsWith("+++")) additions++;
else if (line.startsWith("-") && !line.startsWith("---")) deletions++;
}
files.push({ path, status, additions, deletions, patch: trimmed });
}
return files;
},
}));
const SAMPLE_PATCH = `diff --git a/src/app.ts b/src/app.ts
index abc1234..def5678 100644
--- a/src/app.ts
+++ b/src/app.ts
@@ -1,3 +1,4 @@
import express from "express";
+import cors from "cors";
const app = express();
app.listen(3000);
diff --git a/src/new-file.ts b/src/new-file.ts
new file mode 100644
index 0000000..abc1234
--- /dev/null
+++ b/src/new-file.ts
@@ -0,0 +1,2 @@
+export function hello() {}
+export function world() {}`;
const MERGE_DETAILS: MergeDetails = {
commitSha: "abc1234567890def",
filesChanged: 2,
insertions: 3,
deletions: 0,
mergeCommitMessage: "Merge branch 'fusion/fn-001' into main",
mergedAt: "2026-01-15T10:30:00Z",
};
beforeEach(() => {
mockFetchTaskDiff.mockReset();
mockFetchCommitDiff.mockReset();
});
describe("TaskChangesTab — worktree-backed (non-done tasks)", () => {
it("shows 'No worktree available' when no worktree and not done", () => {
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="in-progress"
/>,
);
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
});
it("loads diff from fetchTaskDiff for in-progress task with worktree", async () => {
mockFetchTaskDiff.mockResolvedValue({
files: [
{ path: "src/app.ts", status: "modified", additions: 1, deletions: 0, patch: "@@ -1 +1,2 @@" },
],
stats: { filesChanged: 1, additions: 1, deletions: 0 },
});
render(
<TaskChangesTab
taskId="FN-001"
worktree="/path/to/worktree"
column="in-progress"
/>,
);
await waitFor(() => {
expect(screen.getByText("src/app.ts")).toBeTruthy();
});
expect(mockFetchTaskDiff).toHaveBeenCalledWith("FN-001", undefined, undefined);
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
});
it("loads diff from fetchTaskDiff for in-review task with worktree", async () => {
mockFetchTaskDiff.mockResolvedValue({
files: [
{ path: "src/app.ts", status: "modified", additions: 1, deletions: 0, patch: "@@ -1 +1,2 @@" },
],
stats: { filesChanged: 1, additions: 1, deletions: 0 },
});
render(
<TaskChangesTab
taskId="FN-001"
worktree="/path/to/worktree"
column="in-review"
/>,
);
await waitFor(() => {
expect(screen.getByText("src/app.ts")).toBeTruthy();
});
expect(mockFetchTaskDiff).toHaveBeenCalled();
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
});
it("shows 'No files modified' when worktree diff returns empty", async () => {
mockFetchTaskDiff.mockResolvedValue({
files: [],
stats: { filesChanged: 0, additions: 0, deletions: 0 },
});
render(
<TaskChangesTab
taskId="FN-001"
worktree="/path/to/worktree"
column="in-progress"
/>,
);
await waitFor(() => {
expect(screen.getByText("No files modified.")).toBeTruthy();
});
});
it("shows error state when fetchTaskDiff fails", async () => {
mockFetchTaskDiff.mockRejectedValue(new Error("Network error"));
render(
<TaskChangesTab
taskId="FN-001"
worktree="/path/to/worktree"
column="in-progress"
/>,
);
await waitFor(() => {
expect(screen.getByText(/Error loading changes: Network error/)).toBeTruthy();
});
});
it("shows loading state initially", () => {
mockFetchTaskDiff.mockReturnValue(new Promise(() => {})); // never resolves
render(
<TaskChangesTab
taskId="FN-001"
worktree="/path/to/worktree"
column="in-progress"
/>,
);
expect(screen.getByText("Loading changes...")).toBeTruthy();
});
});
describe("TaskChangesTab — commit-backed (done tasks)", () => {
it("loads diff from fetchCommitDiff for done task with commitSha", async () => {
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={MERGE_DETAILS}
/>,
);
await waitFor(() => {
expect(screen.getByText("src/app.ts")).toBeTruthy();
});
expect(mockFetchCommitDiff).toHaveBeenCalledWith("abc1234567890def");
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
});
it("shows commit metadata for done task", async () => {
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={MERGE_DETAILS}
/>,
);
await waitFor(() => {
expect(screen.getByText("abc1234")).toBeTruthy(); // short SHA
});
expect(screen.getByText("Merge branch 'fusion/fn-001' into main")).toBeTruthy();
expect(screen.getByText(/Merged .+/)).toBeTruthy();
});
it("uses mergeDetails stats for summary", async () => {
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={MERGE_DETAILS}
/>,
);
await waitFor(() => {
expect(screen.getByText("Files Changed (2)")).toBeTruthy();
});
expect(screen.getByText("+3")).toBeTruthy();
expect(screen.getByText("-0")).toBeTruthy();
});
it("toggling file expansion shows/hides diff content", async () => {
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
const { container } = render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={MERGE_DETAILS}
/>,
);
await waitFor(() => {
expect(screen.getByText("src/app.ts")).toBeTruthy();
});
// First file should be auto-expanded
let diffBlocks = container.querySelectorAll(".changes-file-content");
expect(diffBlocks.length).toBe(1);
// Click to collapse
fireEvent.click(screen.getByText("src/app.ts"));
diffBlocks = container.querySelectorAll(".changes-file-content");
expect(diffBlocks.length).toBe(0);
// Click on second file to expand
fireEvent.click(screen.getByText("src/new-file.ts"));
diffBlocks = container.querySelectorAll(".changes-file-content");
expect(diffBlocks.length).toBe(1);
});
it("shows 'No files modified' with commit-specific hint when patch is empty", async () => {
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: "" });
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={MERGE_DETAILS}
/>,
);
await waitFor(() => {
expect(screen.getByText("No files modified.")).toBeTruthy();
});
expect(screen.getByText("No file changes were recorded in the merge commit.")).toBeTruthy();
});
it("shows error state when fetchCommitDiff fails", async () => {
mockFetchCommitDiff.mockRejectedValue(new Error("Git error"));
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={MERGE_DETAILS}
/>,
);
await waitFor(() => {
expect(screen.getByText(/Error loading changes: Git error/)).toBeTruthy();
});
});
it("renders commit SHA metadata even when only commitSha is set", async () => {
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
const { container } = render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={{ commitSha: "abc1234567890def" }}
/>,
);
await waitFor(() => {
expect(screen.getByText("src/app.ts")).toBeTruthy();
});
// SHA metadata should render since commitSha is present
expect(container.querySelector(".commit-diff-meta")).toBeTruthy();
expect(screen.getByText("abc1234")).toBeTruthy(); // short SHA
// But message and timestamp should NOT be present since they're not set
expect(container.querySelector(".commit-diff-message")).toBeNull();
expect(container.querySelector(".commit-diff-timestamp")).toBeNull();
});
});
describe("TaskChangesTab — regression: non-done tasks still use worktree path", () => {
it("in-progress without worktree shows worktree empty state, not commit path", () => {
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="in-progress"
mergeDetails={MERGE_DETAILS}
/>,
);
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
});
it("in-review without worktree shows worktree empty state", () => {
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="in-review"
mergeDetails={MERGE_DETAILS}
/>,
);
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
});
it("todo task never loads diff even with mergeDetails", () => {
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="todo"
mergeDetails={MERGE_DETAILS}
/>,
);
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
});
it("done task without commitSha falls through to worktree path", () => {
render(
<TaskChangesTab
taskId="FN-001"
worktree={undefined}
column="done"
mergeDetails={{}} // no commitSha
/>,
);
// Falls through to the !worktree check for non-commit-diff path
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
});
});