Files
kandev/apps/web/components/task/task-changes-panel.tsx
T
José Almeida 6cbf050fa1 fix: prevent agentctl OOM from unbounded diff generation in workspace tracker (#598)
* fix: prevent agentctl OOM from unbounded diff generation in workspace tracker

Add per-file size cap (10MB), binary detection (null-byte in 8KB),
diff output cap (256KB), and total budget (2MB) to workspace tracker
diff enrichment. Strip diff content from event bus path. Surface skip
reasons in frontend. Add pprof endpoints to agentctl.

* fix: address PR review feedback

Close stdout pipe on cmd.Start failure to prevent fd leak.
Limit untracked file read to maxDiffOutputSize (256KB) instead of
maxDiffFileSize (10MB) to reduce peak allocation from ~20MB to ~512KB.

* fix: add filepath.Clean to os.Stat call to satisfy CodeQL path-injection check

* fix: address second round of PR review feedback

Extract all DiffSkipReason strings as named constants. Add diff_skip_reason
to UncommittedFile type removing unsafe cast. Add unit tests for
isBinaryContent, totalDiffBytes, capDiffOutput truncation, and all
enrichUntrackedFileDiffs skip-reason paths.

* fix: use bytes.IndexByte for null-byte check clarity

* fix: show truncation banner when diff has partial content

* fix: revert diff stripping from event bus — frontend needs diffs from this path

E2E tests confirmed the frontend receives git status diffs through the
event bus (agentctl → backend WS → event bus → gateway → frontend).
Stripping diffs broke the changes panel. The per-file size caps in
workspace_git_diff.go already bound the data.

* fix: extract renderDiffContent to keep FileDiffSection under 100-line lint limit

* fix: correct onCommentRun type in renderDiffContent helper

* fix: format long destructuring line for prettier

* fix: move diffSkipReasonLabel to types.ts to keep review-diff-list under 600 lines

Co-authored-by: José Almeida <undefined@users.noreply.github.com>

* fix: coerce optional forceLoad to boolean for renderDiffContent

* fix: shorten className and comment to stay within prettier printWidth

* fix: apply prettier formatting to review-diff-list

* fix: correct off-by-one in untracked file line count

---------

Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: José Almeida <undefined@users.noreply.github.com>
2026-04-17 15:16:18 +01:00

500 lines
16 KiB
TypeScript

"use client";
import { memo, useMemo, useCallback, createRef, useState, useEffect, useRef } from "react";
import { PanelRoot, PanelBody } from "./panel-primitives";
import { useToast } from "@/components/toast-provider";
import { useAppStore } from "@/components/state-provider";
import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status";
import { useCumulativeDiff } from "@/hooks/domains/session/use-cumulative-diff";
import { useActiveTaskPR } from "@/hooks/domains/github/use-task-pr";
import { usePRDiff } from "@/hooks/domains/github/use-pr-diff";
import { useGitOperations } from "@/hooks/use-git-operations";
import { useSessionFileReviews } from "@/hooks/use-session-file-reviews";
import { useCommentsStore, isDiffComment } from "@/lib/state/slices/comments";
import { getWebSocketClient } from "@/lib/ws/connection";
import { updateUserSettings } from "@/lib/api";
import { formatReviewCommentsAsMarkdown } from "@/lib/state/slices/comments/format";
import { ReviewDiffList } from "@/components/review/review-diff-list";
import type { ReviewFile } from "@/components/review/types";
import { hashDiff, normalizeDiffContent } from "@/components/review/types";
import type { PRDiffFile } from "@/lib/types/github";
import { usePanelActions } from "@/hooks/use-panel-actions";
import { ChangesTopBar } from "./changes-top-bar";
import type { SelectedDiff } from "./task-layout";
import { useIsTaskArchived, ArchivedPanelPlaceholder } from "./task-archived-context";
type TaskChangesPanelProps = {
mode?: "all" | "file";
filePath?: string;
selectedDiff: SelectedDiff | null;
onClearSelected: () => void;
onBecameEmpty?: () => void;
/** Callback to open file in editor */
onOpenFile?: (filePath: string) => void;
};
type UncommittedFile = {
diff?: string;
diff_skip_reason?: ReviewFile["diff_skip_reason"];
status?: string;
additions?: number;
deletions?: number;
staged?: boolean;
};
type CumulativeFile = { diff?: string; status?: string; additions?: number; deletions?: number };
function addUncommittedFiles(
fileMap: Map<string, ReviewFile>,
files: Record<string, UncommittedFile>,
) {
for (const [path, file] of Object.entries(files)) {
const diff = file.diff ? normalizeDiffContent(file.diff) : "";
const skipReason = file.diff_skip_reason;
if (diff || skipReason) {
fileMap.set(path, {
path,
diff,
status: file.status ?? "modified",
additions: file.additions ?? 0,
deletions: file.deletions ?? 0,
staged: file.staged ?? false,
source: "uncommitted",
diff_skip_reason: skipReason,
});
}
}
}
function addCumulativeFiles(
fileMap: Map<string, ReviewFile>,
files: Record<string, CumulativeFile>,
) {
for (const [path, file] of Object.entries(files)) {
if (fileMap.has(path)) continue;
const diff = file.diff ? normalizeDiffContent(file.diff) : "";
if (diff) {
fileMap.set(path, {
path,
diff,
status: file.status || "modified",
additions: file.additions ?? 0,
deletions: file.deletions ?? 0,
staged: false,
source: "committed",
});
}
}
}
function prFileStatus(status: string): "added" | "deleted" | "modified" {
if (status === "added") return "added";
if (status === "removed") return "deleted";
return "modified";
}
function addPRFiles(fileMap: Map<string, ReviewFile>, files: PRDiffFile[]) {
for (const file of files) {
if (fileMap.has(file.filename)) continue;
const diff = file.patch ? normalizeDiffContent(file.patch) : "";
if (diff)
fileMap.set(file.filename, {
path: file.filename,
diff,
status: prFileStatus(file.status),
additions: file.additions ?? 0,
deletions: file.deletions ?? 0,
staged: false,
source: "pr",
});
}
}
/** Merge PR + uncommitted + committed files into a single sorted list */
function mergeReviewFiles(
gitStatus: ReturnType<typeof useSessionGitStatus>,
cumulativeDiff: { files?: Record<string, CumulativeFile> } | null,
prDiffFiles?: PRDiffFile[],
): ReviewFile[] {
const fileMap = new Map<string, ReviewFile>();
if (prDiffFiles) addPRFiles(fileMap, prDiffFiles);
if (gitStatus?.files)
addUncommittedFiles(fileMap, gitStatus.files as Record<string, UncommittedFile>);
if (cumulativeDiff?.files) addCumulativeFiles(fileMap, cumulativeDiff.files);
return Array.from(fileMap.values()).sort((a, b) => a.path.localeCompare(b.path));
}
function useChangesData(selectedDiff: SelectedDiff | null, onClearSelected: () => void) {
const activeSessionId = useAppStore((state) => state.tasks.activeSessionId);
const gitStatus = useSessionGitStatus(activeSessionId);
const { diff: cumulativeDiff, loading: cumulativeLoading } = useCumulativeDiff(activeSessionId);
const pr = useActiveTaskPR();
const { files: prDiffFiles, loading: prDiffLoading } = usePRDiff(
pr?.owner ?? null,
pr?.repo ?? null,
pr?.pr_number ?? null,
);
const { reviews } = useSessionFileReviews(activeSessionId);
const byId = useCommentsStore((s) => s.byId);
const commentSessionIds = useCommentsStore((s) =>
activeSessionId ? s.bySession[activeSessionId] : undefined,
);
const allFiles = useMemo<ReviewFile[]>(
() =>
mergeReviewFiles(gitStatus, cumulativeDiff, prDiffFiles.length > 0 ? prDiffFiles : undefined),
[gitStatus, cumulativeDiff, prDiffFiles],
);
const { reviewedFiles, staleFiles } = useMemo(() => {
const reviewed = new Set<string>();
const stale = new Set<string>();
// When a PR exists but its diff files are still loading, the file list
// temporarily uses cumulative diff content which has a different hash.
// Skip review computation until PR diffs arrive to avoid a 1/1 -> 0/1 flash.
if (pr && prDiffLoading) {
return { reviewedFiles: reviewed, staleFiles: stale };
}
for (const file of allFiles) {
const reviewState = reviews.get(file.path);
if (!reviewState?.reviewed) continue;
const currentHash = hashDiff(file.diff);
if (reviewState.diffHash && reviewState.diffHash !== currentHash) {
stale.add(file.path);
} else {
reviewed.add(file.path);
}
}
return { reviewedFiles: reviewed, staleFiles: stale };
}, [allFiles, reviews, pr, prDiffLoading]);
const totalCommentCount = useMemo(() => {
if (!commentSessionIds || commentSessionIds.length === 0) return 0;
let count = 0;
for (const id of commentSessionIds) {
const comment = byId[id];
if (comment && isDiffComment(comment)) count++;
}
return count;
}, [byId, commentSessionIds]);
// Derive a stable key from file paths so refs are only recreated when
// the file list itself changes, not when diff content updates.
const filePathsKey = useMemo(() => allFiles.map((f) => f.path).join("\0"), [allFiles]);
const fileRefs = useMemo(() => {
const refs = new Map<string, React.RefObject<HTMLDivElement | null>>();
for (const file of allFiles) refs.set(file.path, createRef<HTMLDivElement>());
return refs;
// eslint-disable-next-line react-hooks/exhaustive-deps -- keyed on stable path list, not allFiles reference
}, [filePathsKey]);
const scrolledRef = useRef<string | null>(null);
useEffect(() => {
if (!selectedDiff?.path || scrolledRef.current === selectedDiff.path) return;
scrolledRef.current = selectedDiff.path;
const ref = fileRefs.get(selectedDiff.path);
if (ref?.current)
requestAnimationFrame(() => {
ref.current?.scrollIntoView({ behavior: "smooth", block: "start" });
});
onClearSelected();
}, [selectedDiff, fileRefs, onClearSelected]);
useEffect(() => {
if (!selectedDiff) scrolledRef.current = null;
}, [selectedDiff]);
return {
activeSessionId,
allFiles,
reviewedFiles,
staleFiles,
totalCommentCount,
fileRefs,
cumulativeLoading,
};
}
function persistAutoMarkSetting(checked: boolean) {
const client = getWebSocketClient();
const payload = { review_auto_mark_on_scroll: checked };
if (client) {
client.request("user.settings.update", payload).catch(() => {
updateUserSettings(payload, { cache: "no-store" }).catch(() => {});
});
return;
}
updateUserSettings(payload, { cache: "no-store" }).catch(() => {});
}
function useChangesActions(activeSessionId: string | null | undefined, allFiles: ReviewFile[]) {
const activeTaskId = useAppStore((state) => state.tasks.activeTaskId);
const autoMarkOnScroll = useAppStore((s) => s.userSettings.reviewAutoMarkOnScroll);
const setUserSettings = useAppStore((state) => state.setUserSettings);
const userSettings = useAppStore((state) => state.userSettings);
const { discard } = useGitOperations(activeSessionId ?? null);
const { markReviewed, markUnreviewed } = useSessionFileReviews(activeSessionId ?? null);
const getPendingComments = useCommentsStore((s) => s.getPendingComments);
const markCommentsSent = useCommentsStore((s) => s.markCommentsSent);
const { toast } = useToast();
const [splitView, setSplitView] = useState(
() => typeof window !== "undefined" && localStorage.getItem("diff-view-mode") === "split",
);
const [wordWrap, setWordWrap] = useState(false);
const handleToggleSplitView = useCallback((split: boolean) => {
setSplitView(split);
const mode = split ? "split" : "unified";
localStorage.setItem("diff-view-mode", mode);
window.dispatchEvent(new CustomEvent("diff-view-mode-change", { detail: mode }));
}, []);
const handleToggleReviewed = useCallback(
(path: string, reviewed: boolean) => {
if (reviewed) {
const file = allFiles.find((f) => f.path === path);
markReviewed(path, file ? hashDiff(file.diff) : "");
} else {
markUnreviewed(path);
}
},
[allFiles, markReviewed, markUnreviewed],
);
const handleDiscard = useCallback(
async (path: string) => {
try {
const result = await discard([path]);
if (result.success) {
toast({ title: "Changes discarded", description: path, variant: "success" });
} else {
toast({
title: "Discard failed",
description: result.error || "An error occurred",
variant: "error",
});
}
} catch (e) {
toast({
title: "Discard failed",
description: e instanceof Error ? e.message : "An error occurred",
variant: "error",
});
}
},
[discard, toast],
);
const handleToggleAutoMark = useCallback(
(checked: boolean) => {
const next = { ...userSettings, reviewAutoMarkOnScroll: checked };
setUserSettings(next);
persistAutoMarkSetting(checked);
},
[setUserSettings, userSettings],
);
const handleFixComments = useCallback(() => {
if (!activeSessionId || !activeTaskId) return;
const allPending = getPendingComments();
const comments = allPending.filter(isDiffComment);
if (comments.length === 0) return;
const markdown = formatReviewCommentsAsMarkdown(comments);
if (!markdown) return;
const client = getWebSocketClient();
if (client) {
client
.request("message.add", {
task_id: activeTaskId,
session_id: activeSessionId,
content: markdown,
})
.catch(() => {
toast({ title: "Failed to send comments", variant: "error" });
});
}
markCommentsSent(comments.map((c) => c.id));
}, [activeSessionId, activeTaskId, getPendingComments, markCommentsSent, toast]);
return {
splitView,
wordWrap,
setWordWrap,
autoMarkOnScroll,
handleToggleSplitView,
handleToggleReviewed,
handleDiscard,
handleToggleAutoMark,
handleFixComments,
};
}
const TaskChangesPanel = memo(function TaskChangesPanel({
mode = "all",
filePath,
selectedDiff,
onClearSelected,
onBecameEmpty,
onOpenFile: onOpenFileProp,
}: TaskChangesPanelProps) {
const isArchived = useIsTaskArchived();
const { openFile: panelOpenFile, openFileInMarkdownPreview } = usePanelActions();
const handleOpenFile = onOpenFileProp ?? panelOpenFile;
const {
activeSessionId,
allFiles,
reviewedFiles,
staleFiles,
totalCommentCount,
fileRefs,
cumulativeLoading,
} = useChangesData(selectedDiff, onClearSelected);
const visibleFiles = useMemo(() => {
if (mode === "file" && filePath) {
return allFiles.filter((file) => file.path === filePath);
}
return allFiles;
}, [allFiles, mode, filePath]);
const visibleFileRefs = useMemo(() => {
if (mode !== "file" || !filePath) return fileRefs;
const refs = new Map<string, React.RefObject<HTMLDivElement | null>>();
const fileRef = fileRefs.get(filePath);
if (fileRef) refs.set(filePath, fileRef);
return refs;
}, [mode, filePath, fileRefs]);
const {
splitView,
wordWrap,
setWordWrap,
autoMarkOnScroll,
handleToggleSplitView,
handleToggleReviewed,
handleDiscard,
handleToggleAutoMark,
handleFixComments,
} = useChangesActions(activeSessionId, allFiles);
const reviewedCount = useMemo(
() =>
visibleFiles.reduce((count, file) => {
if (!staleFiles.has(file.path) && reviewedFiles.has(file.path)) return count + 1;
return count;
}, 0),
[visibleFiles, reviewedFiles, staleFiles],
);
const totalCount = visibleFiles.length;
const progressPercent = totalCount > 0 ? (reviewedCount / totalCount) * 100 : 0;
const prevVisibleCountRef = useRef<number | null>(null);
useEffect(() => {
if (!onBecameEmpty) return;
const prevCount = prevVisibleCountRef.current;
if (prevCount !== null && prevCount > 0 && visibleFiles.length === 0) {
onBecameEmpty();
}
prevVisibleCountRef.current = visibleFiles.length;
}, [onBecameEmpty, visibleFiles.length]);
if (isArchived) return <ArchivedPanelPlaceholder />;
return (
<PanelRoot>
<ChangesTopBar
autoMarkOnScroll={autoMarkOnScroll}
splitView={splitView}
wordWrap={wordWrap}
totalCommentCount={totalCommentCount}
reviewedCount={reviewedCount}
totalCount={totalCount}
progressPercent={progressPercent}
setWordWrap={setWordWrap}
handleToggleSplitView={handleToggleSplitView}
handleToggleAutoMark={handleToggleAutoMark}
handleFixComments={handleFixComments}
/>
<PanelBody padding={false} scroll={false} className="overflow-hidden">
<ChangesPanelContent
isLoading={cumulativeLoading}
files={visibleFiles}
activeSessionId={activeSessionId}
reviewedFiles={reviewedFiles}
staleFiles={staleFiles}
autoMarkOnScroll={autoMarkOnScroll}
wordWrap={wordWrap}
selectedFile={mode === "file" ? filePath : undefined}
onToggleReviewed={handleToggleReviewed}
onDiscard={handleDiscard}
onOpenFile={handleOpenFile}
onPreviewMarkdown={openFileInMarkdownPreview}
fileRefs={visibleFileRefs}
/>
</PanelBody>
</PanelRoot>
);
});
function ChangesPanelContent({
isLoading,
files,
activeSessionId,
reviewedFiles,
staleFiles,
autoMarkOnScroll,
wordWrap,
selectedFile,
onToggleReviewed,
onDiscard,
onOpenFile,
onPreviewMarkdown,
fileRefs,
}: {
isLoading: boolean;
files: ReviewFile[];
activeSessionId: string | null | undefined;
reviewedFiles: Set<string>;
staleFiles: Set<string>;
autoMarkOnScroll: boolean;
wordWrap: boolean;
selectedFile?: string | null;
onToggleReviewed: (path: string, reviewed: boolean) => void;
onDiscard: (path: string) => Promise<void>;
onOpenFile: (path: string) => void;
onPreviewMarkdown?: (path: string) => void;
fileRefs: Map<string, React.RefObject<HTMLDivElement | null>>;
}) {
if (isLoading && files.length === 0) {
return (
<div className="flex items-center justify-center h-full text-muted-foreground text-sm">
Loading changes...
</div>
);
}
if (files.length === 0) {
return (
<div className="flex items-center justify-center h-full text-muted-foreground text-sm">
No changes
</div>
);
}
if (!activeSessionId) return null;
return (
<ReviewDiffList
files={files}
reviewedFiles={reviewedFiles}
staleFiles={staleFiles}
sessionId={activeSessionId}
autoMarkOnScroll={autoMarkOnScroll}
wordWrap={wordWrap}
selectedFile={selectedFile}
onToggleReviewed={onToggleReviewed}
onDiscard={onDiscard}
onOpenFile={onOpenFile}
onPreviewMarkdown={onPreviewMarkdown}
fileRefs={fileRefs}
/>
);
}
export { TaskChangesPanel };