fix: unify diff endpoints and show changed files on done task cards
- Server /tasks/:id/diff and /tasks/:id/file-diffs now both use
resolveDiffBase() for consistent file lists across card and modal
- Both endpoints handle done tasks via mergeDetails.commitSha,
returning structured { files, stats } from the merge commit
- TaskChangesTab uses fetchTaskDiff for all columns (no more
fetchCommitDiff/parsePatch client-side path for done tasks)
- useChangedFiles hook simplified — server handles done tasks
- Done task cards show "N files changed" button using mergeDetails
- Updated tests for new unified data flow
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -83,7 +83,7 @@ function AppInner() {
|
||||
const [terminalOpen, setTerminalOpen] = useState(false);
|
||||
const [filesOpen, setFilesOpen] = useState(false);
|
||||
const [fileBrowserWorkspace, setFileBrowserWorkspace] = useState("project");
|
||||
const [changedFilesState, setChangedFilesState] = useState<{ taskId: string; worktree: string | undefined; column: string; commitSha?: string } | null>(null);
|
||||
const [changedFilesState, setChangedFilesState] = useState<{ taskId: string; worktree: string | undefined; column: string } | null>(null);
|
||||
const [activityLogOpen, setActivityLogOpen] = useState(false);
|
||||
const [gitManagerOpen, setGitManagerOpen] = useState(false);
|
||||
const [workflowStepsOpen, setWorkflowStepsOpen] = useState(false);
|
||||
@@ -469,8 +469,8 @@ function AppInner() {
|
||||
setFilesOpen(true);
|
||||
}, []);
|
||||
|
||||
const handleOpenChangedFiles = useCallback((taskId: string, worktree: string | undefined, column: string, commitSha?: string) => {
|
||||
setChangedFilesState({ taskId, worktree, column, commitSha });
|
||||
const handleOpenChangedFiles = useCallback((taskId: string, worktree: string | undefined, column: string) => {
|
||||
setChangedFilesState({ taskId, worktree, column });
|
||||
}, []);
|
||||
|
||||
const handleCloseChangedFiles = useCallback(() => {
|
||||
@@ -705,7 +705,6 @@ function AppInner() {
|
||||
worktree={changedFilesState.worktree}
|
||||
column={changedFilesState.column}
|
||||
projectId={currentProject?.id}
|
||||
commitSha={changedFilesState.commitSha}
|
||||
isOpen={true}
|
||||
onClose={handleCloseChangedFiles}
|
||||
/>
|
||||
|
||||
@@ -35,7 +35,7 @@ interface BoardProps {
|
||||
* Called when the user clicks the "Subtask" button in the inline create card.
|
||||
*/
|
||||
onSubtaskBreakdown?: (description: string) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: string) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
|
||||
favoriteProviders?: string[];
|
||||
favoriteModels?: string[];
|
||||
onToggleFavorite?: (provider: string) => void;
|
||||
|
||||
@@ -19,7 +19,6 @@ interface ChangedFilesModalProps {
|
||||
worktree: string | undefined;
|
||||
column: string;
|
||||
projectId?: string;
|
||||
commitSha?: string;
|
||||
isOpen: boolean;
|
||||
onClose: () => void;
|
||||
}
|
||||
@@ -67,7 +66,6 @@ export function ChangedFilesModal({
|
||||
worktree,
|
||||
column,
|
||||
projectId,
|
||||
commitSha,
|
||||
isOpen,
|
||||
onClose,
|
||||
}: ChangedFilesModalProps) {
|
||||
@@ -76,7 +74,6 @@ export function ChangedFilesModal({
|
||||
worktree,
|
||||
column,
|
||||
projectId,
|
||||
commitSha,
|
||||
);
|
||||
|
||||
const [isMobile, setIsMobile] = useState(false);
|
||||
|
||||
@@ -46,7 +46,7 @@ interface ColumnProps {
|
||||
* Called when the user clicks the "Subtask" button in the inline create card.
|
||||
*/
|
||||
onSubtaskBreakdown?: (description: string) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: string) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
|
||||
favoriteProviders?: string[];
|
||||
favoriteModels?: string[];
|
||||
onToggleFavorite?: (provider: string) => void;
|
||||
|
||||
@@ -44,7 +44,7 @@ interface TaskCardProps {
|
||||
) => Promise<Task>;
|
||||
onArchiveTask?: (id: string) => Promise<Task>;
|
||||
onUnarchiveTask?: (id: string) => Promise<Task>;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: string) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
|
||||
}
|
||||
|
||||
function areTaskBadgeInfosEqual(
|
||||
@@ -694,7 +694,7 @@ function TaskCardComponent({
|
||||
className="card-session-files"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onOpenFilesForTask?.(task.id, task.worktree, task.column, task.mergeDetails?.commitSha);
|
||||
onOpenFilesForTask?.(task.id, task.worktree, task.column);
|
||||
}}
|
||||
disabled={!onOpenFilesForTask}
|
||||
>
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
import { useState, useEffect, useCallback } from "react";
|
||||
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 { fetchTaskDiff, type TaskDiff } from "../api";
|
||||
import { highlightDiff } from "../utils/highlightDiff";
|
||||
|
||||
interface TaskChangesTabProps {
|
||||
@@ -50,43 +49,10 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
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 canLoad = column === "in-progress" || column === "in-review" || column === "done";
|
||||
|
||||
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) {
|
||||
if (!canLoad) {
|
||||
setLoading(false);
|
||||
return;
|
||||
}
|
||||
@@ -112,7 +78,7 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
} finally {
|
||||
setLoading(false);
|
||||
}
|
||||
}, [taskId, worktree, projectId, useCommitDiff, commitSha, mergeDetails]);
|
||||
}, [taskId, projectId, canLoad]);
|
||||
|
||||
useEffect(() => {
|
||||
loadDiff();
|
||||
@@ -152,8 +118,10 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
);
|
||||
}
|
||||
|
||||
const isDone = column === "done";
|
||||
|
||||
// Non-done task without a worktree → show worktree empty state
|
||||
if (!useCommitDiff && !worktree) {
|
||||
if (!isDone && !worktree) {
|
||||
return (
|
||||
<div className="detail-section">
|
||||
<div className="task-changes-state task-changes-state--empty">
|
||||
@@ -174,7 +142,7 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
<FileCode size={24} />
|
||||
<p>No files modified.</p>
|
||||
<span className="task-changes-state-hint">
|
||||
{useCommitDiff
|
||||
{isDone
|
||||
? "No file changes were recorded in the merge commit."
|
||||
: "The agent did not modify any files during execution."}
|
||||
</span>
|
||||
@@ -186,12 +154,14 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
return (
|
||||
<div className="detail-section task-changes-tab">
|
||||
{/* Commit metadata for done tasks */}
|
||||
{useCommitDiff && mergeDetails && (
|
||||
{isDone && mergeDetails && (
|
||||
<div className="commit-diff-meta">
|
||||
<div className="commit-diff-sha">
|
||||
<GitCommit size={14} />
|
||||
<code>{commitSha!.slice(0, 7)}</code>
|
||||
</div>
|
||||
{mergeDetails.commitSha && (
|
||||
<div className="commit-diff-sha">
|
||||
<GitCommit size={14} />
|
||||
<code>{mergeDetails.commitSha.slice(0, 7)}</code>
|
||||
</div>
|
||||
)}
|
||||
{mergeDetails.mergeCommitMessage && (
|
||||
<div className="commit-diff-message">{mergeDetails.mergeCommitMessage}</div>
|
||||
)}
|
||||
|
||||
@@ -16,7 +16,7 @@ interface WorktreeGroupProps {
|
||||
id: string,
|
||||
updates: { title?: string; description?: string; dependencies?: string[] }
|
||||
) => Promise<Task>;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: string) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string) => void;
|
||||
}
|
||||
|
||||
function WorktreeGroupComponent({
|
||||
|
||||
@@ -4,11 +4,9 @@ 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", () => ({
|
||||
@@ -23,53 +21,13 @@ 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 DONE_TASK_DIFF = {
|
||||
files: [
|
||||
{ path: "src/app.ts", status: "modified" as const, additions: 1, deletions: 0, patch: "diff --git a/src/app.ts b/src/app.ts\n@@ -1 +1,2 @@\n import express from \"express\";\n+import cors from \"cors\";" },
|
||||
{ path: "src/new-file.ts", status: "added" as const, additions: 2, deletions: 0, patch: "diff --git a/src/new-file.ts b/src/new-file.ts\nnew file mode 100644\n@@ -0,0 +1,2 @@\n+export function hello() {}\n+export function world() {}" },
|
||||
],
|
||||
stats: { filesChanged: 2, additions: 3, deletions: 0 },
|
||||
};
|
||||
|
||||
const MERGE_DETAILS: MergeDetails = {
|
||||
commitSha: "abc1234567890def",
|
||||
@@ -82,11 +40,12 @@ const MERGE_DETAILS: MergeDetails = {
|
||||
|
||||
beforeEach(() => {
|
||||
mockFetchTaskDiff.mockReset();
|
||||
mockFetchCommitDiff.mockReset();
|
||||
});
|
||||
|
||||
describe("TaskChangesTab — worktree-backed (non-done tasks)", () => {
|
||||
it("shows 'No worktree available' when no worktree and not done", () => {
|
||||
it("shows 'No worktree available' when no worktree and not done", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
@@ -94,9 +53,10 @@ describe("TaskChangesTab — worktree-backed (non-done tasks)", () => {
|
||||
column="in-progress"
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("loads diff from fetchTaskDiff for in-progress task with worktree", async () => {
|
||||
@@ -119,7 +79,6 @@ describe("TaskChangesTab — worktree-backed (non-done tasks)", () => {
|
||||
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 () => {
|
||||
@@ -142,7 +101,6 @@ describe("TaskChangesTab — worktree-backed (non-done tasks)", () => {
|
||||
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 () => {
|
||||
@@ -194,8 +152,8 @@ describe("TaskChangesTab — worktree-backed (non-done tasks)", () => {
|
||||
});
|
||||
|
||||
describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
it("loads diff from fetchCommitDiff for done task with commitSha", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
it("loads diff from fetchTaskDiff for done task with commitSha", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
@@ -209,12 +167,11 @@ describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("src/app.ts")).toBeTruthy();
|
||||
});
|
||||
expect(mockFetchCommitDiff).toHaveBeenCalledWith("abc1234567890def");
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalledWith("FN-001", undefined, undefined);
|
||||
});
|
||||
|
||||
it("shows commit metadata for done task", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
@@ -233,7 +190,7 @@ describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
});
|
||||
|
||||
it("uses mergeDetails stats for summary", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
@@ -252,7 +209,7 @@ describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
});
|
||||
|
||||
it("toggling file expansion shows/hides diff content", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
@@ -282,7 +239,7 @@ describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
});
|
||||
|
||||
it("shows 'No files modified' with commit-specific hint when patch is empty", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: "" });
|
||||
mockFetchTaskDiff.mockResolvedValue({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
@@ -299,8 +256,8 @@ describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
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"));
|
||||
it("shows error state when fetchTaskDiff fails", async () => {
|
||||
mockFetchTaskDiff.mockRejectedValue(new Error("Git error"));
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
@@ -317,7 +274,7 @@ describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
});
|
||||
|
||||
it("renders commit SHA metadata even when only commitSha is set", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
@@ -342,7 +299,9 @@ describe("TaskChangesTab — commit-backed (done tasks)", () => {
|
||||
});
|
||||
|
||||
describe("TaskChangesTab — regression: non-done tasks still use worktree path", () => {
|
||||
it("in-progress without worktree shows worktree empty state, not commit path", () => {
|
||||
it("in-progress without worktree shows worktree empty state, not commit path", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
@@ -351,11 +310,15 @@ describe("TaskChangesTab — regression: non-done tasks still use worktree path"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
|
||||
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("in-review without worktree shows worktree empty state", () => {
|
||||
it("in-review without worktree shows worktree empty state", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
@@ -364,8 +327,10 @@ describe("TaskChangesTab — regression: non-done tasks still use worktree path"
|
||||
mergeDetails={MERGE_DETAILS}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
|
||||
expect(mockFetchCommitDiff).not.toHaveBeenCalled();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("No worktree available for this task.")).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("todo task never loads diff even with mergeDetails", () => {
|
||||
@@ -379,10 +344,11 @@ describe("TaskChangesTab — regression: non-done tasks still use worktree path"
|
||||
);
|
||||
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", () => {
|
||||
it("done task without commitSha calls fetchTaskDiff (server handles it)", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
@@ -391,15 +357,16 @@ describe("TaskChangesTab — regression: non-done tasks still use worktree path"
|
||||
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();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchTaskDiff).toHaveBeenCalledWith("FN-001", undefined, undefined);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskChangesTab — status-to-class mapping", () => {
|
||||
it("applies semantic status class for each file status", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
@@ -414,7 +381,7 @@ describe("TaskChangesTab — status-to-class mapping", () => {
|
||||
expect(screen.getByText("src/app.ts")).toBeTruthy();
|
||||
});
|
||||
|
||||
// src/app.ts is modified (no new file mode / deleted file mode markers)
|
||||
// src/app.ts is modified, src/new-file.ts is added
|
||||
const statusBadges = container.querySelectorAll(".changes-file-status");
|
||||
expect(statusBadges.length).toBeGreaterThanOrEqual(2);
|
||||
|
||||
@@ -429,7 +396,7 @@ describe("TaskChangesTab — status-to-class mapping", () => {
|
||||
});
|
||||
|
||||
it("uses CSS classes instead of inline styles for status colors", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
@@ -452,7 +419,7 @@ describe("TaskChangesTab — status-to-class mapping", () => {
|
||||
});
|
||||
|
||||
it("renders stat summary with diff-add and diff-del classes", async () => {
|
||||
mockFetchCommitDiff.mockResolvedValue({ stat: "", patch: SAMPLE_PATCH });
|
||||
mockFetchTaskDiff.mockResolvedValue(DONE_TASK_DIFF);
|
||||
|
||||
const { container } = render(
|
||||
<TaskChangesTab
|
||||
|
||||
@@ -35,17 +35,15 @@ describe("useChangedFiles", () => {
|
||||
expect(mockFetchTaskFileDiffs).toHaveBeenCalledWith("KB-651", undefined);
|
||||
});
|
||||
|
||||
it("does not fetch for tasks without worktrees or inactive columns", async () => {
|
||||
const { result: noWorktree } = renderHook(() => useChangedFiles("KB-651", undefined, "in-progress"));
|
||||
const { result: inactive } = renderHook(() => useChangedFiles("KB-651", "/repo/.worktrees/kb-651", "todo"));
|
||||
it("does not fetch for tasks in inactive columns", async () => {
|
||||
const { result: triage } = renderHook(() => useChangedFiles("KB-651", "/repo/.worktrees/kb-651", "triage"));
|
||||
const { result: todo } = renderHook(() => useChangedFiles("KB-651", "/repo/.worktrees/kb-651", "todo"));
|
||||
|
||||
await waitFor(() => expect(noWorktree.current.loading).toBe(false));
|
||||
await waitFor(() => expect(inactive.current.loading).toBe(false));
|
||||
await waitFor(() => expect(triage.current.loading).toBe(false));
|
||||
await waitFor(() => expect(todo.current.loading).toBe(false));
|
||||
|
||||
expect(noWorktree.current.files).toEqual([]);
|
||||
expect(noWorktree.current.selectedFile).toBeNull();
|
||||
expect(inactive.current.files).toEqual([]);
|
||||
expect(inactive.current.selectedFile).toBeNull();
|
||||
expect(triage.current.files).toEqual([]);
|
||||
expect(todo.current.files).toEqual([]);
|
||||
expect(mockFetchTaskFileDiffs).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
@@ -120,12 +118,16 @@ describe("useChangedFiles", () => {
|
||||
expect(result.current.selectedFile).toBeNull();
|
||||
});
|
||||
|
||||
it("returns empty files and null selection when column is inactive", () => {
|
||||
const { result } = renderHook(() => useChangedFiles("KB-651", "/repo/.worktrees/kb-651", "done"));
|
||||
it("fetches changed files for done tasks", async () => {
|
||||
mockFetchTaskFileDiffs.mockResolvedValueOnce([
|
||||
{ path: "src/a.ts", status: "modified", diff: "diff" },
|
||||
]);
|
||||
|
||||
expect(result.current.files).toEqual([]);
|
||||
expect(result.current.loading).toBe(false);
|
||||
expect(result.current.selectedFile).toBeNull();
|
||||
expect(result.current.error).toBeNull();
|
||||
const { result } = renderHook(() => useChangedFiles("KB-651", undefined, "done"));
|
||||
|
||||
await waitFor(() => expect(result.current.loading).toBe(false));
|
||||
|
||||
expect(result.current.files).toHaveLength(1);
|
||||
expect(mockFetchTaskFileDiffs).toHaveBeenCalledWith("KB-651", undefined);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,5 @@
|
||||
import { useEffect, useState, useCallback } from "react";
|
||||
import { fetchTaskFileDiffs, fetchCommitDiff, type TaskFileDiff } from "../api";
|
||||
import { parsePatch } from "../components/CommitDiffTab";
|
||||
|
||||
const ACTIVE_COLUMNS = new Set(["in-progress", "in-review"]);
|
||||
import { fetchTaskFileDiffs, type TaskFileDiff } from "../api";
|
||||
|
||||
interface UseChangedFilesResult {
|
||||
files: TaskFileDiff[];
|
||||
@@ -15,22 +12,19 @@ interface UseChangedFilesResult {
|
||||
|
||||
export function useChangedFiles(
|
||||
taskId: string,
|
||||
worktree: string | undefined,
|
||||
_worktree: string | undefined,
|
||||
column: string,
|
||||
projectId?: string,
|
||||
commitSha?: string,
|
||||
): UseChangedFilesResult {
|
||||
const [files, setFiles] = useState<TaskFileDiff[]>([]);
|
||||
const [loading, setLoading] = useState(false);
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [selectedFile, setSelectedFile] = useState<TaskFileDiff | null>(null);
|
||||
|
||||
const isDone = column === "done";
|
||||
const canLoad = column === "in-progress" || column === "in-review" || column === "done";
|
||||
|
||||
useEffect(() => {
|
||||
// For active tasks: need worktree
|
||||
// For done tasks: need commitSha
|
||||
if (!taskId || (!isDone && (!worktree || !ACTIVE_COLUMNS.has(column))) || (isDone && !commitSha)) {
|
||||
if (!taskId || !canLoad) {
|
||||
setFiles([]);
|
||||
setLoading(false);
|
||||
setError(null);
|
||||
@@ -44,21 +38,7 @@ export function useChangedFiles(
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
try {
|
||||
let result: TaskFileDiff[];
|
||||
if (isDone && commitSha) {
|
||||
// Done task: fetch from commit history
|
||||
const data = await fetchCommitDiff(commitSha);
|
||||
const parsed = parsePatch(data.patch || "");
|
||||
result = parsed.map((f) => ({
|
||||
path: f.path,
|
||||
status: f.status === "unknown" ? "modified" as const : f.status,
|
||||
diff: f.patch,
|
||||
oldPath: undefined,
|
||||
}));
|
||||
} else {
|
||||
// Active task: fetch from worktree
|
||||
result = await fetchTaskFileDiffs(taskId, projectId);
|
||||
}
|
||||
const result = await fetchTaskFileDiffs(taskId, projectId);
|
||||
if (cancelled) return;
|
||||
setFiles(result);
|
||||
setSelectedFile((current) => {
|
||||
@@ -88,7 +68,7 @@ export function useChangedFiles(
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [taskId, worktree, column, projectId, commitSha, isDone]);
|
||||
}, [taskId, column, projectId, canLoad]);
|
||||
|
||||
const resetSelection = useCallback(() => {
|
||||
setSelectedFile(null);
|
||||
|
||||
@@ -7024,32 +7024,91 @@ Output ONLY the prompt text (no markdown, no explanations).`;
|
||||
return;
|
||||
}
|
||||
|
||||
// Done tasks: use commit-backed diff from mergeDetails.commitSha
|
||||
if (task.column === "done" && task.mergeDetails?.commitSha) {
|
||||
const rootDir = scopedStore.getRootDir();
|
||||
const sha = task.mergeDetails.commitSha;
|
||||
const nameStatus = nodeChildProcess.execSync(
|
||||
`git show --name-status --format="" ${sha}`,
|
||||
{ cwd: rootDir, encoding: "utf-8", timeout: 10000 },
|
||||
).trim();
|
||||
|
||||
const doneFiles: Array<{
|
||||
path: string;
|
||||
status: "added" | "modified" | "deleted";
|
||||
additions: number;
|
||||
deletions: number;
|
||||
patch: string;
|
||||
}> = [];
|
||||
|
||||
for (const line of nameStatus.split("\n").filter(Boolean)) {
|
||||
const parts = line.split("\t");
|
||||
const statusCode = parts[0] ?? "M";
|
||||
const filePath = parts[1] ?? "";
|
||||
if (!filePath) continue;
|
||||
|
||||
let status: "added" | "modified" | "deleted" = "modified";
|
||||
if (statusCode.startsWith("A")) status = "added";
|
||||
else if (statusCode.startsWith("D")) status = "deleted";
|
||||
|
||||
let patch = "";
|
||||
try {
|
||||
patch = nodeChildProcess.execSync(
|
||||
`git show ${sha} -- "${filePath}"`,
|
||||
{ cwd: rootDir, encoding: "utf-8", timeout: 10000 },
|
||||
);
|
||||
} catch { /* ignore */ }
|
||||
|
||||
const additions = (patch.match(/^\+[^+]/gm) || []).length;
|
||||
const deletions = (patch.match(/^-[^-]/gm) || []).length;
|
||||
doneFiles.push({ path: filePath, status, additions, deletions, patch });
|
||||
}
|
||||
|
||||
const doneStats = {
|
||||
filesChanged: doneFiles.length,
|
||||
additions: doneFiles.reduce((s, f) => s + f.additions, 0),
|
||||
deletions: doneFiles.reduce((s, f) => s + f.deletions, 0),
|
||||
};
|
||||
|
||||
res.json({ files: doneFiles, stats: doneStats });
|
||||
return;
|
||||
}
|
||||
|
||||
const worktree = typeof req.query.worktree === "string" ? req.query.worktree : undefined;
|
||||
const cwd = worktree || scopedStore.getRootDir();
|
||||
const cwd = worktree || task.worktree || scopedStore.getRootDir();
|
||||
|
||||
// Get the base branch for merge-base comparison
|
||||
const baseBranch = task.baseBranch ?? "main";
|
||||
let diffBase = `${baseBranch}...HEAD`;
|
||||
// Use resolveDiffBase for consistent diff base across all endpoints
|
||||
const diffBase = resolveDiffBase(task, cwd);
|
||||
const diffRange = diffBase ? `${diffBase}..HEAD` : "HEAD";
|
||||
|
||||
// Get list of changed files — include both committed and working-tree changes
|
||||
const fileMap = new Map<string, string>();
|
||||
|
||||
if (diffBase) {
|
||||
try {
|
||||
const committedOutput = nodeChildProcess.execSync(
|
||||
`git diff --name-status ${diffBase}..HEAD`,
|
||||
{ encoding: "utf-8", cwd, timeout: 10000 },
|
||||
).trim();
|
||||
for (const line of committedOutput.split("\n").filter(Boolean)) {
|
||||
const parts = line.split("\t");
|
||||
fileMap.set(parts[1] ?? "", parts[0] ?? "M");
|
||||
}
|
||||
} catch {
|
||||
// committed diff failed
|
||||
}
|
||||
}
|
||||
|
||||
// Get the diff
|
||||
const { execSync } = await import("node:child_process");
|
||||
|
||||
// Get list of changed files using merge-base comparison
|
||||
let filesOutput = "";
|
||||
try {
|
||||
filesOutput = execSync(`git diff --name-status ${diffBase}`, {
|
||||
encoding: "utf-8",
|
||||
cwd,
|
||||
timeout: 10000,
|
||||
const workingTreeOutput = nodeChildProcess.execSync("git diff --name-status", {
|
||||
encoding: "utf-8", cwd, timeout: 10000,
|
||||
}).trim();
|
||||
for (const line of workingTreeOutput.split("\n").filter(Boolean)) {
|
||||
const parts = line.split("\t");
|
||||
fileMap.set(parts[1] ?? "", parts[0] ?? "M");
|
||||
}
|
||||
} catch {
|
||||
// Fallback to current HEAD if merge-base fails
|
||||
diffBase = "HEAD";
|
||||
filesOutput = execSync("git diff --name-status HEAD", {
|
||||
encoding: "utf-8",
|
||||
cwd,
|
||||
timeout: 10000,
|
||||
}).trim();
|
||||
// working tree diff failed
|
||||
}
|
||||
|
||||
const files: Array<{
|
||||
@@ -7060,13 +7119,9 @@ Output ONLY the prompt text (no markdown, no explanations).`;
|
||||
patch: string;
|
||||
}> = [];
|
||||
|
||||
for (const line of filesOutput.trim().split("\n")) {
|
||||
if (!line.trim()) continue;
|
||||
|
||||
const parts = line.split("\t");
|
||||
const statusCode = parts[0];
|
||||
const filePath = parts[1];
|
||||
|
||||
for (const [filePath, statusCode] of fileMap) {
|
||||
if (!filePath) continue;
|
||||
|
||||
let status: "added" | "modified" | "deleted";
|
||||
if (statusCode.startsWith("A")) status = "added";
|
||||
else if (statusCode.startsWith("D")) status = "deleted";
|
||||
@@ -7075,7 +7130,7 @@ Output ONLY the prompt text (no markdown, no explanations).`;
|
||||
// Get patch for this file
|
||||
let patch = "";
|
||||
try {
|
||||
patch = execSync(`git diff ${diffBase} -- "${filePath}"`, {
|
||||
patch = nodeChildProcess.execSync(`git diff ${diffRange} -- "${filePath}"`, {
|
||||
encoding: "utf-8",
|
||||
cwd,
|
||||
timeout: 10000,
|
||||
@@ -7115,6 +7170,40 @@ Output ONLY the prompt text (no markdown, no explanations).`;
|
||||
try {
|
||||
const scopedStore = await getScopedStore(req);
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
|
||||
// Done tasks: derive file diffs from the merge commit
|
||||
if (task.column === "done" && task.mergeDetails?.commitSha) {
|
||||
const rootDir = scopedStore.getRootDir();
|
||||
const sha = task.mergeDetails.commitSha;
|
||||
try {
|
||||
const nameStatus = nodeChildProcess.execSync(
|
||||
`git show --name-status --format="" ${sha}`,
|
||||
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
|
||||
).trim();
|
||||
const doneFiles = nameStatus.split("\n").filter(Boolean).map((line) => {
|
||||
const parts = line.split("\t");
|
||||
const statusCode = parts[0] ?? "M";
|
||||
const filePath = parts[1] ?? "";
|
||||
let status: "added" | "modified" | "deleted" | "renamed" = "modified";
|
||||
if (statusCode.startsWith("A")) status = "added";
|
||||
else if (statusCode.startsWith("D")) status = "deleted";
|
||||
else if (statusCode.startsWith("R")) status = "renamed";
|
||||
let diff = "";
|
||||
try {
|
||||
diff = nodeChildProcess.execSync(
|
||||
`git show ${sha} -- "${filePath}"`,
|
||||
{ cwd: rootDir, encoding: "utf-8", timeout: 5000 },
|
||||
);
|
||||
} catch { /* ignore */ }
|
||||
return { path: filePath, status, diff };
|
||||
});
|
||||
res.json(doneFiles);
|
||||
} catch {
|
||||
res.json([]);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (!task.worktree || !nodeFs.existsSync(task.worktree)) {
|
||||
res.json([]);
|
||||
return;
|
||||
|
||||
Reference in New Issue
Block a user