fix: use merge-base for diffStat in merger and show files changed on done task cards
The merger was using `git diff HEAD..branch --stat` which includes artifacts from other tasks when branches fork from older main commits. Switch to `git diff $(merge-base)..branch --stat` so commit messages only describe the branch's own changes. Also surface the "files changed" button on done task cards using mergeDetails, opening the same ChangedFilesModal with commit-backed diffs (matching the Changes tab in the task modal). 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 } | null>(null);
|
||||
const [changedFilesState, setChangedFilesState] = useState<{ taskId: string; worktree: string | undefined; column: string; commitSha?: 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) => {
|
||||
setChangedFilesState({ taskId, worktree, column });
|
||||
const handleOpenChangedFiles = useCallback((taskId: string, worktree: string | undefined, column: string, commitSha?: string) => {
|
||||
setChangedFilesState({ taskId, worktree, column, commitSha });
|
||||
}, []);
|
||||
|
||||
const handleCloseChangedFiles = useCallback(() => {
|
||||
@@ -705,6 +705,7 @@ 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) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: string) => void;
|
||||
favoriteProviders?: string[];
|
||||
favoriteModels?: string[];
|
||||
onToggleFavorite?: (provider: string) => void;
|
||||
|
||||
@@ -19,6 +19,7 @@ interface ChangedFilesModalProps {
|
||||
worktree: string | undefined;
|
||||
column: string;
|
||||
projectId?: string;
|
||||
commitSha?: string;
|
||||
isOpen: boolean;
|
||||
onClose: () => void;
|
||||
}
|
||||
@@ -66,6 +67,7 @@ export function ChangedFilesModal({
|
||||
worktree,
|
||||
column,
|
||||
projectId,
|
||||
commitSha,
|
||||
isOpen,
|
||||
onClose,
|
||||
}: ChangedFilesModalProps) {
|
||||
@@ -74,6 +76,7 @@ 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) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: 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) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: string) => void;
|
||||
}
|
||||
|
||||
function areTaskBadgeInfosEqual(
|
||||
@@ -688,6 +688,20 @@ function TaskCardComponent({
|
||||
</span>
|
||||
</button>
|
||||
)}
|
||||
{task.column === "done" && task.mergeDetails?.filesChanged != null && task.mergeDetails.filesChanged > 0 && (
|
||||
<button
|
||||
type="button"
|
||||
className="card-session-files"
|
||||
onClick={(e) => {
|
||||
e.stopPropagation();
|
||||
onOpenFilesForTask?.(task.id, task.worktree, task.column, task.mergeDetails?.commitSha);
|
||||
}}
|
||||
disabled={!onOpenFilesForTask}
|
||||
>
|
||||
<Folder size={12} />
|
||||
<span>{task.mergeDetails.filesChanged} files changed</span>
|
||||
</button>
|
||||
)}
|
||||
{((task.dependencies && task.dependencies.length > 0) || queued || task.status === "queued" || task.blockedBy) && (
|
||||
<div className="card-meta">
|
||||
{task.dependencies && task.dependencies.length > 0 && (
|
||||
|
||||
@@ -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) => void;
|
||||
onOpenFilesForTask?: (taskId: string, worktree: string | undefined, column: string, commitSha?: string) => void;
|
||||
}
|
||||
|
||||
function WorktreeGroupComponent({
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useEffect, useState, useCallback } from "react";
|
||||
import { fetchTaskFileDiffs, type TaskFileDiff } from "../api";
|
||||
import { fetchTaskFileDiffs, fetchCommitDiff, type TaskFileDiff } from "../api";
|
||||
import { parsePatch } from "../components/CommitDiffTab";
|
||||
|
||||
const ACTIVE_COLUMNS = new Set(["in-progress", "in-review"]);
|
||||
|
||||
@@ -17,14 +18,19 @@ export function useChangedFiles(
|
||||
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";
|
||||
|
||||
useEffect(() => {
|
||||
if (!taskId || !worktree || !ACTIVE_COLUMNS.has(column)) {
|
||||
// For active tasks: need worktree
|
||||
// For done tasks: need commitSha
|
||||
if (!taskId || (!isDone && (!worktree || !ACTIVE_COLUMNS.has(column))) || (isDone && !commitSha)) {
|
||||
setFiles([]);
|
||||
setLoading(false);
|
||||
setError(null);
|
||||
@@ -38,7 +44,21 @@ export function useChangedFiles(
|
||||
setLoading(true);
|
||||
setError(null);
|
||||
try {
|
||||
const result = await fetchTaskFileDiffs(taskId, projectId);
|
||||
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);
|
||||
}
|
||||
if (cancelled) return;
|
||||
setFiles(result);
|
||||
setSelectedFile((current) => {
|
||||
@@ -68,7 +88,7 @@ export function useChangedFiles(
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [taskId, worktree, column, projectId]);
|
||||
}, [taskId, worktree, column, projectId, commitSha, isDone]);
|
||||
|
||||
const resetSelection = useCallback(() => {
|
||||
setSelectedFile(null);
|
||||
|
||||
@@ -94,6 +94,7 @@ function setupHappyPathExecSync() {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
// Post-squash check: --quiet means "did squash stage anything?" → "1" = yes
|
||||
@@ -280,6 +281,7 @@ describe("aiMergeTask — empty squash merge (branch already merged via dep)", (
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
// Squash staged nothing → "0"
|
||||
@@ -304,6 +306,7 @@ describe("aiMergeTask — empty squash merge (branch already merged via dep)", (
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --cached --quiet")) return "0" as any;
|
||||
@@ -381,6 +384,7 @@ describe("aiMergeTask — includeTaskIdInCommit setting", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --cached")) return "1" as any;
|
||||
@@ -409,6 +413,7 @@ describe("aiMergeTask — includeTaskIdInCommit setting", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --cached")) return "1" as any;
|
||||
@@ -1102,6 +1107,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash") || cmdStr.includes("merge -X")) return Buffer.from("");
|
||||
// Post-squash check: "1" = has staged changes
|
||||
@@ -1133,6 +1139,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something";
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed";
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
// No conflicts
|
||||
@@ -1169,6 +1176,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something";
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed";
|
||||
|
||||
if (cmdStr.includes("merge --squash")) {
|
||||
@@ -1218,6 +1226,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something";
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed";
|
||||
|
||||
if (cmdStr.includes("merge --squash")) {
|
||||
@@ -1268,6 +1277,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something";
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed";
|
||||
|
||||
// First two regular squash merges fail with conflicts
|
||||
@@ -1339,6 +1349,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something";
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed";
|
||||
|
||||
if (cmdStr.includes("merge --squash") || cmdStr.includes("merge -X theirs")) {
|
||||
@@ -1385,6 +1396,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something";
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed";
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --name-only --diff-filter=U")) return ""; // No conflicts
|
||||
@@ -1693,6 +1705,7 @@ describe("aiMergeTask — build verification", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
|
||||
@@ -1740,6 +1753,7 @@ describe("aiMergeTask — build verification", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
// After commit, diff shows clean
|
||||
@@ -1784,6 +1798,7 @@ describe("aiMergeTask — build verification", () => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
// After commit, diff shows clean
|
||||
@@ -1840,6 +1855,7 @@ describe("aiMergeTask — build verification", () => {
|
||||
// Default happy path for other commands
|
||||
if (cmdStr.includes("rev-parse")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
// Staged changes present (agent didn't commit due to build failure)
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { execSync } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import type { TaskStore, MergeResult } from "@fusion/core";
|
||||
import { getTaskMergeBlocker, type TaskStore, type MergeResult } from "@fusion/core";
|
||||
import { createKbAgent, promptWithFallback } from "./pi.js";
|
||||
import type { WorktreePool } from "./worktree-pool.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
@@ -585,10 +585,9 @@ export async function aiMergeTask(
|
||||
): Promise<MergeResult> {
|
||||
// 1. Validate task state
|
||||
const task = await store.getTask(taskId);
|
||||
if (task.column !== "in-review") {
|
||||
throw new Error(
|
||||
`Cannot merge ${taskId}: task is in '${task.column}', must be in 'in-review'`,
|
||||
);
|
||||
const mergeBlocker = getTaskMergeBlocker(task);
|
||||
if (mergeBlocker) {
|
||||
throw new Error(`Cannot merge ${taskId}: ${mergeBlocker}`);
|
||||
}
|
||||
|
||||
const branch = task.branch || `kb/${taskId.toLowerCase()}`;
|
||||
@@ -635,7 +634,11 @@ export async function aiMergeTask(
|
||||
commitLog = "(unable to read commit log)";
|
||||
}
|
||||
try {
|
||||
diffStat = execSync(`git diff HEAD..${branch} --stat`, {
|
||||
const mergeBase = execSync(`git merge-base HEAD ${branch}`, {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
}).trim();
|
||||
diffStat = execSync(`git diff ${mergeBase}..${branch} --stat`, {
|
||||
cwd: rootDir,
|
||||
encoding: "utf-8",
|
||||
}).trim();
|
||||
|
||||
Reference in New Issue
Block a user