fix(FN-2877): restore task card timing and changes fallbacks
This commit is contained in:
@@ -117,6 +117,13 @@ function getDoneCompletionMs(task: Task): number | null {
|
||||
return completionMs;
|
||||
}
|
||||
|
||||
function getInProgressElapsedMs(task: Task, nowMs: number): number | null {
|
||||
const startedMs = parseTimestampToMs(task.columnMovedAt ?? task.updatedAt);
|
||||
if (startedMs == null) return null;
|
||||
|
||||
return Math.max(0, nowMs - startedMs);
|
||||
}
|
||||
|
||||
// Mirrors summarizeWorkflowTiming in TaskTokenStatsPanel: completed steps use
|
||||
// completedAt-startedAt; in-progress steps contribute live elapsed (now-startedAt).
|
||||
function getWorkflowRuntimeMs(task: Task, nowMs: number): number | null {
|
||||
@@ -687,10 +694,9 @@ function TaskCardComponent({
|
||||
}
|
||||
|
||||
if (task.column === "in-progress") {
|
||||
const hasInProgressStep = (task.workflowStepResults ?? []).some(
|
||||
(step) => step.startedAt && !step.completedAt,
|
||||
);
|
||||
if (!hasInProgressStep) {
|
||||
const elapsedMs = getInProgressElapsedMs(task, Date.now());
|
||||
const instrumentedMs = getInstrumentedDurationMs(task, Date.now());
|
||||
if (elapsedMs == null && instrumentedMs == null) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
@@ -708,31 +714,39 @@ function TaskCardComponent({
|
||||
}, LIVE_TIME_INDICATOR_POLL_MS);
|
||||
|
||||
return () => window.clearInterval(interval);
|
||||
}, [task.column, task.workflowStepResults, task.timedExecutionMs]);
|
||||
}, [task.column, task.columnMovedAt, task.updatedAt, task.workflowStepResults, task.timedExecutionMs]);
|
||||
|
||||
const timeIndicator = useMemo(() => {
|
||||
if (!TIME_INDICATOR_COLUMNS.has(task.column)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const instrumentedMs = getInstrumentedDurationMs(task, timeIndicatorNowMs);
|
||||
if (instrumentedMs == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (task.column === "in-progress") {
|
||||
const elapsedLabel = formatElapsedDuration(instrumentedMs);
|
||||
const elapsedMs =
|
||||
getInProgressElapsedMs(task, timeIndicatorNowMs)
|
||||
?? getInstrumentedDurationMs(task, timeIndicatorNowMs);
|
||||
if (elapsedMs == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const elapsedLabel = formatElapsedDuration(elapsedMs);
|
||||
if (!elapsedLabel) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const hasColumnElapsed = getInProgressElapsedMs(task, timeIndicatorNowMs) != null;
|
||||
return {
|
||||
label: elapsedLabel,
|
||||
title: `Execution time ${elapsedLabel}`,
|
||||
ariaLabel: `Execution time ${elapsedLabel}`,
|
||||
title: hasColumnElapsed ? `In progress ${elapsedLabel}` : `Execution time ${elapsedLabel}`,
|
||||
ariaLabel: hasColumnElapsed ? `In progress ${elapsedLabel}` : `Execution time ${elapsedLabel}`,
|
||||
};
|
||||
}
|
||||
|
||||
const instrumentedMs = getInstrumentedDurationMs(task, timeIndicatorNowMs);
|
||||
if (instrumentedMs == null) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const elapsedLabel = formatElapsedDurationDone(instrumentedMs);
|
||||
if (!elapsedLabel) {
|
||||
return null;
|
||||
@@ -1039,9 +1053,17 @@ function TaskCardComponent({
|
||||
const cardClass = `card${dragging ? " dragging" : ""}${queued ? " queued" : ""}${isAgentActive ? " agent-active" : ""}${isFailed ? " failed" : ""}${isPaused ? " paused" : ""}${isStuck ? " stuck" : ""}${isAwaitingApproval ? " awaiting-approval" : ""}${fileDragOver ? " file-drop-target" : ""}${isEditing ? " card-editing" : ""}${isSaving ? " card-saving" : ""}`;
|
||||
|
||||
const filesChangedButton = (() => {
|
||||
if (task.worktree && task.column === "in-progress") {
|
||||
const activeCount = diffStats?.filesChanged;
|
||||
if (activeCount == null || activeCount === 0) {
|
||||
if (task.column === "in-progress") {
|
||||
const activeDiffCount = diffStats?.filesChanged;
|
||||
const fallbackCount =
|
||||
activeDiffCount == null || activeDiffCount === 0
|
||||
? task.modifiedFiles?.length
|
||||
: undefined;
|
||||
const displayCount =
|
||||
activeDiffCount != null && activeDiffCount > 0
|
||||
? activeDiffCount
|
||||
: fallbackCount;
|
||||
if (displayCount == null || displayCount === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -1053,15 +1075,21 @@ function TaskCardComponent({
|
||||
disabled={!onOpenDetailWithTab}
|
||||
>
|
||||
<Folder size={12} />
|
||||
<span>{activeCount} {activeCount === 1 ? "file" : "files"} changed</span>
|
||||
<span>{displayCount} {displayCount === 1 ? "file" : "files"} changed</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
if (task.column === "in-review") {
|
||||
const reviewDiffCount = diffStats?.filesChanged;
|
||||
const fallbackCount = reviewDiffCount == null ? task.modifiedFiles?.length : undefined;
|
||||
const displayCount = reviewDiffCount ?? fallbackCount;
|
||||
const fallbackCount =
|
||||
reviewDiffCount == null || reviewDiffCount === 0
|
||||
? task.modifiedFiles?.length
|
||||
: undefined;
|
||||
const displayCount =
|
||||
reviewDiffCount != null && reviewDiffCount > 0
|
||||
? reviewDiffCount
|
||||
: fallbackCount;
|
||||
if (displayCount == null || displayCount === 0) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -16,11 +16,11 @@ interface TaskChangesTabProps {
|
||||
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.
|
||||
* Used as a last-resort fallback when the live worktree diff is empty or 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.
|
||||
*/
|
||||
modifiedFiles?: string[];
|
||||
}
|
||||
@@ -38,6 +38,58 @@ function getStatusLabel(status: "added" | "modified" | "deleted" | "unknown"): s
|
||||
}
|
||||
}
|
||||
|
||||
function renderModifiedFilesFallback(
|
||||
modifiedFiles: string[],
|
||||
isDone: boolean,
|
||||
mergeDetails?: MergeDetails,
|
||||
) {
|
||||
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">
|
||||
{isDone
|
||||
? "The recorded merge commit has no diff (likely collapsed into a squash on main). Showing file paths only — patches unavailable."
|
||||
: "The live worktree diff is empty. Showing the last file paths captured during execution — 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 changes-file-header--static">
|
||||
<span
|
||||
className="changes-file-status changes-file-status--unknown"
|
||||
title="status unknown"
|
||||
>
|
||||
{getStatusLabel("unknown")}
|
||||
</span>
|
||||
<span className="changes-file-path" title={path}>
|
||||
<bdo dir="ltr">{path}</bdo>
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
/** Normalized file entry used by both worktree-backed and commit-backed paths */
|
||||
interface NormalizedFile {
|
||||
path: string;
|
||||
@@ -164,6 +216,10 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
|
||||
// Non-done task without a worktree → show worktree empty state
|
||||
if (!isDone && !worktree) {
|
||||
if (modifiedFiles && modifiedFiles.length > 0) {
|
||||
return renderModifiedFilesFallback(modifiedFiles, false);
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="detail-section">
|
||||
<div className="task-changes-state task-changes-state--empty">
|
||||
@@ -181,6 +237,10 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
// We must NOT fetch detailed diffs here because the server would fall back
|
||||
// to a repository-wide scan, producing an inflated/unrelated file list.
|
||||
if (isDone && !isDoneWithCommit) {
|
||||
if (modifiedFiles && modifiedFiles.length > 0) {
|
||||
return renderModifiedFilesFallback(modifiedFiles, true, mergeDetails);
|
||||
}
|
||||
|
||||
const summaryFiles = mergeDetails?.filesChanged;
|
||||
const summaryAdditions = mergeDetails?.insertions;
|
||||
const summaryDeletions = mergeDetails?.deletions;
|
||||
@@ -202,55 +262,8 @@ export function TaskChangesTab({ taskId, worktree, projectId, column, mergeDetai
|
||||
}
|
||||
|
||||
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 changes-file-header--static">
|
||||
<span
|
||||
className="changes-file-status changes-file-status--unknown"
|
||||
title="status unknown"
|
||||
>
|
||||
{getStatusLabel("unknown")}
|
||||
</span>
|
||||
<span className="changes-file-path" title={path}>
|
||||
<bdo dir="ltr">{path}</bdo>
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
if (modifiedFiles && modifiedFiles.length > 0) {
|
||||
return renderModifiedFilesFallback(modifiedFiles, isDone, mergeDetails);
|
||||
}
|
||||
|
||||
return (
|
||||
|
||||
@@ -413,6 +413,31 @@ describe("TaskCard", () => {
|
||||
expect(onOpenDetailWithTab).toHaveBeenCalledWith(task, "changes");
|
||||
});
|
||||
|
||||
it("shows in-progress files-changed chip from modifiedFiles fallback when no live diff is available", () => {
|
||||
const onOpenDetailWithTab = vi.fn();
|
||||
const task = makeTask({
|
||||
column: "in-progress",
|
||||
worktree: undefined,
|
||||
modifiedFiles: ["packages/core/src/store.ts", "packages/core/src/types.ts"],
|
||||
});
|
||||
|
||||
render(
|
||||
<TaskCard
|
||||
task={task}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
onOpenDetailWithTab={onOpenDetailWithTab}
|
||||
/>,
|
||||
);
|
||||
|
||||
const filesChangedButton = screen.getByRole("button", { name: "2 files changed" });
|
||||
expect(filesChangedButton).toBeDefined();
|
||||
expect((filesChangedButton as HTMLButtonElement).disabled).toBe(false);
|
||||
|
||||
fireEvent.click(filesChangedButton);
|
||||
expect(onOpenDetailWithTab).toHaveBeenCalledWith(task, "changes");
|
||||
});
|
||||
|
||||
it("shows error toast when upload fails", async () => {
|
||||
const mockUpload = vi.mocked(uploadAttachment);
|
||||
mockUpload.mockRejectedValue(new Error("Upload failed"));
|
||||
@@ -565,13 +590,11 @@ describe("TaskCard", () => {
|
||||
});
|
||||
|
||||
it("updates the in-progress timer when timedExecutionMs changes", () => {
|
||||
const updatedAt = "2026-04-25T12:00:00.000Z";
|
||||
const { container, rerender } = render(
|
||||
<TaskCard
|
||||
task={makeTask({
|
||||
column: "in-progress",
|
||||
timedExecutionMs: 60_000,
|
||||
updatedAt,
|
||||
})}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
@@ -585,7 +608,6 @@ describe("TaskCard", () => {
|
||||
task={makeTask({
|
||||
column: "in-progress",
|
||||
timedExecutionMs: 120_000,
|
||||
updatedAt,
|
||||
})}
|
||||
onOpenDetail={noop}
|
||||
addToast={noop}
|
||||
@@ -760,7 +782,10 @@ describe("TaskCard", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it("does not render timer chip when no instrumentation data is recorded", () => {
|
||||
it("shows wall-clock timer for in-progress cards when columnMovedAt is available", () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(new Date("2026-04-25T12:05:00.000Z"));
|
||||
|
||||
const { container } = render(
|
||||
<TaskCard
|
||||
task={makeTask({
|
||||
@@ -774,7 +799,9 @@ describe("TaskCard", () => {
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(container.querySelector(".card-time-indicator")).toBeNull();
|
||||
const timer = container.querySelector(".card-time-indicator");
|
||||
expect(timer?.textContent).toContain("5m");
|
||||
expect(timer?.getAttribute("title")).toContain("In progress 5m");
|
||||
});
|
||||
|
||||
it("does not render timer chip on done card without instrumentation, even with old timestamps", () => {
|
||||
|
||||
@@ -68,6 +68,25 @@ describe("TaskChangesTab — worktree-backed (non-done tasks)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("shows modifiedFiles fallback when an active task has no worktree diff", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue({ files: [], stats: { filesChanged: 0, additions: 0, deletions: 0 } });
|
||||
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="in-progress"
|
||||
modifiedFiles={["packages/core/src/store.ts", "packages/core/src/types.ts"]}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("2 files modified during execution.")).toBeTruthy();
|
||||
});
|
||||
expect(screen.getByText("packages/core/src/store.ts")).toBeTruthy();
|
||||
expect(screen.getByText("packages/core/src/types.ts")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("loads diff from fetchTaskDiff for in-progress task with worktree", async () => {
|
||||
mockFetchTaskDiff.mockResolvedValue({
|
||||
files: [
|
||||
@@ -439,6 +458,25 @@ describe("TaskChangesTab — regression: non-done tasks still use worktree path"
|
||||
expect(screen.getByText("Merge summary: 1 file changed, +5 additions, -0 deletions.")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("done task without commitSha falls back to modifiedFiles when available", async () => {
|
||||
render(
|
||||
<TaskChangesTab
|
||||
taskId="FN-001"
|
||||
worktree={undefined}
|
||||
column="done"
|
||||
mergeDetails={{ filesChanged: 0, insertions: 0, deletions: 0 }}
|
||||
modifiedFiles={["packages/cli/src/commands/__tests__/settings.test.ts", "packages/cli/src/commands/__tests__/task.test.ts"]}
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("2 files modified during execution.")).toBeTruthy();
|
||||
});
|
||||
expect(screen.getByText("packages/cli/src/commands/__tests__/settings.test.ts")).toBeTruthy();
|
||||
expect(screen.getByText("packages/cli/src/commands/__tests__/task.test.ts")).toBeTruthy();
|
||||
expect(mockFetchTaskDiff).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("done task without commitSha and no mergeDetails shows fallback without summary", async () => {
|
||||
render(
|
||||
<TaskChangesTab
|
||||
|
||||
Reference in New Issue
Block a user