fix(merger,dashboard): handle empty-squash commitSha to keep card and Changes tab in sync
When a branch contained commits already on main (duplicate cherry-picks), the merger's local squash collapsed to an empty commit. The merger then recorded that empty commit's SHA on mergeDetails.commitSha. The actual content landed later on main as a different SHA via PR merge, but the task kept pointing at the orphaned empty commit. Symptom: TaskCard showed "N files changed" (falling back to task.modifiedFiles), but the Changes tab in the modal showed nothing because the API hit `git diff sha^..sha` on the empty commit and returned no files. Two fixes: 1. merger.ts: detect empty squash commits and skip storing commitSha, logging clearly. recoverInterruptedMergingTasks → findLandedTaskCommit already exists to backfill the right SHA when the real commit lands; a missing commitSha is a known fallback path the UI already handles. 2. TaskChangesTab.tsx: when the API returns no files for a done task, fall back to task.modifiedFiles (paths only, no patches) with a clear note. Mirrors the existing 3-tier fallback in TaskCard.tsx:1090-1124 so card and modal always agree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -13,6 +13,15 @@ interface TaskChangesTabProps {
|
|||||||
projectId?: string;
|
projectId?: string;
|
||||||
column?: Column;
|
column?: Column;
|
||||||
mergeDetails?: MergeDetails;
|
mergeDetails?: MergeDetails;
|
||||||
|
/**
|
||||||
|
* Files modified by the task during execution, captured from the worktree.
|
||||||
|
* Used as a last-resort fallback when the recorded `mergeDetails.commitSha`
|
||||||
|
* resolves to an empty git commit (which can happen when the merger stores
|
||||||
|
* a per-branch SHA that gets collapsed into a different squash on main).
|
||||||
|
* Without this, the tab would show "no changes" while the card shows N —
|
||||||
|
* matches TaskCard.tsx:1090-1124's fallback ladder.
|
||||||
|
*/
|
||||||
|
modifiedFiles?: string[];
|
||||||
}
|
}
|
||||||
|
|
||||||
function getStatusLabel(status: "added" | "modified" | "deleted" | "unknown"): string {
|
function getStatusLabel(status: "added" | "modified" | "deleted" | "unknown"): string {
|
||||||
@@ -51,7 +60,7 @@ interface NormalizedFile {
|
|||||||
* changes. This prevents inflated file counts that don't match the card-level
|
* changes. This prevents inflated file counts that don't match the card-level
|
||||||
* display.
|
* display.
|
||||||
*/
|
*/
|
||||||
export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetails }: TaskChangesTabProps) {
|
export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetails, modifiedFiles }: TaskChangesTabProps) {
|
||||||
const [files, setFiles] = useState<NormalizedFile[]>([]);
|
const [files, setFiles] = useState<NormalizedFile[]>([]);
|
||||||
const [stats, setStats] = useState<{ filesChanged: number; additions: number; deletions: number }>({ filesChanged: 0, additions: 0, deletions: 0 });
|
const [stats, setStats] = useState<{ filesChanged: number; additions: number; deletions: number }>({ filesChanged: 0, additions: 0, deletions: 0 });
|
||||||
const [loading, setLoading] = useState(true);
|
const [loading, setLoading] = useState(true);
|
||||||
@@ -192,6 +201,57 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (files.length === 0) {
|
if (files.length === 0) {
|
||||||
|
// Done task with a commit SHA but the diff came back empty — almost
|
||||||
|
// always means the recorded SHA points to an empty commit (merger
|
||||||
|
// stored a per-branch SHA that became no-op after the squash collapsed
|
||||||
|
// onto main). Fall back to the executor-captured modifiedFiles so the
|
||||||
|
// tab agrees with the card. Patches are unavailable in this path.
|
||||||
|
if (isDone && modifiedFiles && modifiedFiles.length > 0) {
|
||||||
|
return (
|
||||||
|
<div className="detail-section task-changes-tab">
|
||||||
|
{isDone && mergeDetails && (
|
||||||
|
<div className="commit-diff-meta">
|
||||||
|
{mergeDetails.commitSha && (
|
||||||
|
<div className="commit-diff-sha">
|
||||||
|
<GitCommit size={14} />
|
||||||
|
<code>{mergeDetails.commitSha.slice(0, 7)}</code>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
{mergeDetails.mergedAt && (
|
||||||
|
<div className="commit-diff-timestamp">
|
||||||
|
Merged {new Date(mergeDetails.mergedAt).toLocaleString()}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
<div className="task-changes-state task-changes-state--empty">
|
||||||
|
<FileCode size={24} />
|
||||||
|
<p>{modifiedFiles.length} file{modifiedFiles.length === 1 ? "" : "s"} modified during execution.</p>
|
||||||
|
<span className="task-changes-state-hint">
|
||||||
|
The recorded merge commit has no diff (likely collapsed into a squash on main). Showing file paths only — patches unavailable.
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
<div className="changes-file-list task-changes-file-list--compact">
|
||||||
|
{modifiedFiles.map((path) => (
|
||||||
|
<div key={path} className="changes-file-item">
|
||||||
|
<div className="changes-file-header">
|
||||||
|
<span
|
||||||
|
className="changes-file-status changes-file-status--unknown"
|
||||||
|
title="status unknown"
|
||||||
|
>
|
||||||
|
{getStatusLabel("unknown")}
|
||||||
|
</span>
|
||||||
|
<span className="changes-file-path" title={path}>
|
||||||
|
{truncateMiddle(path)}
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="detail-section">
|
<div className="detail-section">
|
||||||
<div className="task-changes-state task-changes-state--empty">
|
<div className="task-changes-state task-changes-state--empty">
|
||||||
|
|||||||
@@ -1355,7 +1355,7 @@ export function TaskDetailModal({
|
|||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
) : activeTab === "changes" ? (
|
) : activeTab === "changes" ? (
|
||||||
<TaskChangesTab taskId={task.id} worktree={task.worktree} projectId={projectId} column={task.column} mergeDetails={task.mergeDetails} />
|
<TaskChangesTab taskId={task.id} worktree={task.worktree} projectId={projectId} column={task.column} mergeDetails={task.mergeDetails} modifiedFiles={task.modifiedFiles} />
|
||||||
) : activeTab === "comments" ? (
|
) : activeTab === "comments" ? (
|
||||||
<TaskComments task={task} addToast={addToast} projectId={projectId} onTaskUpdated={onTaskUpdated} />
|
<TaskComments task={task} addToast={addToast} projectId={projectId} onTaskUpdated={onTaskUpdated} />
|
||||||
) : activeTab === "documents" ? (
|
) : activeTab === "documents" ? (
|
||||||
|
|||||||
@@ -2292,8 +2292,25 @@ export async function aiMergeTask(
|
|||||||
deletions = deletionsMatch ? Number.parseInt(deletionsMatch[1], 10) : 0;
|
deletions = deletionsMatch ? Number.parseInt(deletionsMatch[1], 10) : 0;
|
||||||
} catch { /* non-fatal */ }
|
} catch { /* non-fatal */ }
|
||||||
|
|
||||||
|
// Guard: if the squash collapsed to an empty commit, recording its SHA
|
||||||
|
// misleads every consumer (TaskChangesTab shows "no changes" even though
|
||||||
|
// modifiedFiles is non-empty). Real cause: the branch contained commits
|
||||||
|
// already on main (duplicate cherry-picks), and conflict resolution
|
||||||
|
// dropped them. The actual landing typically happens later via PR merge
|
||||||
|
// on a different SHA — let recoverInterruptedMergingTasks /
|
||||||
|
// findLandedTaskCommit populate the right SHA when that lands. Until
|
||||||
|
// then, store mergeDetails without commitSha so the UI falls back to
|
||||||
|
// task.modifiedFiles instead of a broken diff.
|
||||||
|
const isEmptyCommit = filesChanged === 0;
|
||||||
|
const recordedSha = isEmptyCommit ? undefined : commitSha;
|
||||||
|
if (isEmptyCommit) {
|
||||||
|
mergerLog.warn(
|
||||||
|
`${taskId}: local squash produced an empty commit (${commitSha?.slice(0, 8)}) — branch likely contained dupes of main. Skipping commitSha; recovery will backfill when real commit lands.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
const mergeDetails: MergeDetails = {
|
const mergeDetails: MergeDetails = {
|
||||||
commitSha,
|
commitSha: recordedSha,
|
||||||
filesChanged,
|
filesChanged,
|
||||||
insertions,
|
insertions,
|
||||||
deletions,
|
deletions,
|
||||||
@@ -2307,7 +2324,7 @@ export async function aiMergeTask(
|
|||||||
};
|
};
|
||||||
|
|
||||||
await store.updateTask(taskId, { mergeDetails });
|
await store.updateTask(taskId, { mergeDetails });
|
||||||
mergerLog.log(`${taskId}: merge details stored (commitSha: ${commitSha?.slice(0, 8)})`);
|
mergerLog.log(`${taskId}: merge details stored (commitSha: ${recordedSha?.slice(0, 8) ?? "<deferred>"})`);
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
mergerLog.warn(`${taskId}: failed to collect/store merge details: ${err.message}`);
|
mergerLog.warn(`${taskId}: failed to collect/store merge details: ${err.message}`);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user