fix: stabilize diff review apply state and missing-file UX

- Invalidate main-process file content cache after applying review decisions
- Clear stale per-file review state after instant-apply to prevent re-apply conflicts
- Disable accept/reject for preview-only files and surface deleted/renamed/moved badges
- Persist per-file collapse state across reopen

Made-with: Cursor
This commit is contained in:
iliya 2026-03-04 14:36:05 +02:00
parent 887c7406d1
commit dfc2a43a91
7 changed files with 358 additions and 53 deletions

View file

@ -257,7 +257,19 @@ async function handleApplyDecisions(
fileContents.set(d.filePath, resolved);
}
return getApplier().applyReviewDecisions(request, fileContents);
const result = await getApplier().applyReviewDecisions(request, fileContents);
// Invalidate resolved file content cache after applying decisions so subsequent
// diff operations read the latest disk state (avoids "stuck" decisions in instant-apply flows).
try {
for (const d of request.decisions) {
getContentResolver().invalidateFile(d.filePath);
}
} catch (error) {
logger.debug('applyDecisions cache invalidation failed:', error);
}
return result;
});
}

View file

@ -83,6 +83,7 @@ export const ChangeReviewDialog = ({
applySingleFileDecision,
removeReviewFile,
addReviewFile,
clearReviewStateForFile,
editedContents,
updateEditedContent,
discardFileEdits,
@ -98,11 +99,33 @@ export const ChangeReviewDialog = ({
hunkContextHashesByFile,
} = useStore();
// Build scope keys (pure values — safe to compute before hooks that depend on them)
const scopeKey = mode === 'task' ? `task:${taskId ?? ''}` : `agent:${memberName ?? ''}`;
// Filesystem-safe: use `-` instead of `:` for decision persistence key
const decisionScopeKey = mode === 'task' ? `task-${taskId ?? ''}` : `agent-${memberName ?? ''}`;
// Active file from scroll-spy (replaces selectedReviewFilePath for continuous scroll)
const [activeFilePath, setActiveFilePath] = useState<string | null>(null);
const [autoViewed, setAutoViewed] = useState(true);
const [timelineOpen, setTimelineOpen] = useState(false);
const [discardCounters, setDiscardCounters] = useState<Record<string, number>>({});
const collapseStorageKey = useMemo(
() => `review:collapsed:${teamName}:${decisionScopeKey}`,
[teamName, decisionScopeKey]
);
const [collapsedFiles, setCollapsedFiles] = useState<Set<string>>(() => {
if (typeof window === 'undefined') return new Set<string>();
try {
const raw = window.localStorage.getItem(collapseStorageKey);
const parsed = raw ? (JSON.parse(raw) as unknown) : null;
if (Array.isArray(parsed)) {
return new Set(parsed.filter((v): v is string => typeof v === 'string'));
}
} catch {
// ignore
}
return new Set<string>();
});
// Selection menu state
const [selectionInfo, setSelectionInfo] = useState<EditorSelectionInfo | null>(null);
@ -148,18 +171,92 @@ export const ChangeReviewDialog = ({
scrollContainerRef,
});
// Build scope key for viewed storage
const scopeKey = mode === 'task' ? `task:${taskId ?? ''}` : `agent:${memberName ?? ''}`;
// Build scope key for decision persistence (filesystem-safe: use `-` instead of `:`)
const decisionScopeKey = mode === 'task' ? `task-${taskId ?? ''}` : `agent-${memberName ?? ''}`;
// File paths for viewed tracking
const allFilePaths = useMemo(
() => (activeChangeSet?.files ?? []).map((f) => f.filePath),
[activeChangeSet]
);
const pathChangeLabels = useMemo(() => {
if (!activeChangeSet)
return {} as Record<
string,
| { kind: 'deleted' }
| { kind: 'moved' | 'renamed'; direction: 'from' | 'to'; otherPath: string }
>;
const normalize = (s: string): string =>
s.replace(/\r\n/g, '\n').replace(/\r/g, '\n').trimEnd();
const hashFull = (s: string): string => {
// DJB2 (full string) — good enough for heuristic rename/move pairing
let h = 5381;
for (let i = 0; i < s.length; i++) h = ((h << 5) + h + s.charCodeAt(i)) | 0;
return (h >>> 0).toString(36);
};
const baseName = (p: string): string => p.split(/[\\/]/).filter(Boolean).pop() ?? p;
type Label =
| { kind: 'deleted' }
| { kind: 'moved' | 'renamed'; direction: 'from' | 'to'; otherPath: string };
const out: Record<string, Label> = {};
const deletedCandidates: { file: FileChangeSummary; hash: string }[] = [];
const newCandidates: { file: FileChangeSummary; hash: string }[] = [];
for (const f of activeChangeSet.files) {
const content = fileContents[f.filePath];
if (!content) continue;
const modified = content.modifiedFullContent;
const original = content.originalFullContent;
if (!f.isNewFile && modified == null) {
if (original != null) {
deletedCandidates.push({ file: f, hash: hashFull(normalize(original)) });
} else {
out[f.filePath] = { kind: 'deleted' };
}
}
if (f.isNewFile && modified != null) {
newCandidates.push({ file: f, hash: hashFull(normalize(modified)) });
}
}
const deletedByHash = new Map<string, { file: FileChangeSummary; count: number }>();
for (const d of deletedCandidates) {
const prev = deletedByHash.get(d.hash);
deletedByHash.set(d.hash, { file: d.file, count: (prev?.count ?? 0) + 1 });
}
const usedDeleted = new Set<string>();
for (const n of newCandidates) {
const entry = deletedByHash.get(n.hash);
if (!entry) continue;
if (entry.count !== 1) continue; // ambiguous
const oldFile = entry.file;
if (usedDeleted.has(oldFile.filePath)) continue;
usedDeleted.add(oldFile.filePath);
const oldName = baseName(oldFile.relativePath);
const newName = baseName(n.file.relativePath);
const kind: 'moved' | 'renamed' =
oldName === newName && oldFile.relativePath !== n.file.relativePath ? 'moved' : 'renamed';
out[n.file.filePath] = { kind, direction: 'from', otherPath: oldFile.relativePath };
out[oldFile.filePath] = { kind, direction: 'to', otherPath: n.file.relativePath };
}
for (const d of deletedCandidates) {
if (!usedDeleted.has(d.file.filePath) && !(d.file.filePath in out)) {
out[d.file.filePath] = { kind: 'deleted' };
}
}
return out;
}, [activeChangeSet, fileContents]);
const {
viewedSet,
isViewed,
@ -296,6 +393,15 @@ export const ChangeReviewDialog = ({
lastNewFileRemoveAtRef.current = Date.now();
removeReviewFile(filePath);
}
} else {
const hasErrorForFile = !!result?.errors.some((e) => e.filePath === filePath);
if (result && !hasErrorForFile) {
// Disk state is now authoritative. Clear stale decisions/cache so reopening
// doesn't try to re-apply and the diff can re-resolve from disk.
clearReviewStateForFile(filePath);
setDiscardCounters((prev) => ({ ...prev, [filePath]: (prev[filePath] ?? 0) + 1 }));
void fetchFileContent(teamName, memberName, filePath);
}
}
}
} finally {
@ -311,6 +417,8 @@ export const ChangeReviewDialog = ({
memberName,
removeReviewFile,
fileContents,
clearReviewStateForFile,
fetchFileContent,
]
);
@ -336,10 +444,25 @@ export const ChangeReviewDialog = ({
}
hunkDecisionUndoStackRef.current[filePath].push(originalIndex);
if (REVIEW_INSTANT_APPLY) {
void applySingleFileDecision(teamName, filePath, taskId, memberName);
void applySingleFileDecision(teamName, filePath, taskId, memberName).then((result) => {
const hasErrorForFile = !!result?.errors.some((e) => e.filePath === filePath);
if (result && !hasErrorForFile) {
clearReviewStateForFile(filePath);
setDiscardCounters((prev) => ({ ...prev, [filePath]: (prev[filePath] ?? 0) + 1 }));
void fetchFileContent(teamName, memberName, filePath);
}
});
}
},
[setHunkDecision, applySingleFileDecision, teamName, taskId, memberName]
[
setHunkDecision,
applySingleFileDecision,
teamName,
taskId,
memberName,
clearReviewStateForFile,
fetchFileContent,
]
);
const handleContentChanged = useCallback(
@ -490,6 +613,42 @@ export const ChangeReviewDialog = ({
getFileHunkCount(filePath, fallbackSnippetsLength, fileChunkCounts)
);
const toggleCollapsedFile = useCallback((filePath: string) => {
setCollapsedFiles((prev) => {
const next = new Set(prev);
if (next.has(filePath)) next.delete(filePath);
else next.add(filePath);
return next;
});
}, []);
// Persist collapsed state (best-effort)
useEffect(() => {
if (!open) return;
if (typeof window === 'undefined') return;
const id = window.setTimeout(() => {
try {
window.localStorage.setItem(collapseStorageKey, JSON.stringify([...collapsedFiles]));
} catch {
// ignore
}
}, 200);
return () => window.clearTimeout(id);
}, [open, collapseStorageKey, collapsedFiles]);
// Prune collapsed entries to only current files to avoid stale growth
useEffect(() => {
if (!activeChangeSet) return;
const allowed = new Set(activeChangeSet.files.map((f) => f.filePath));
setCollapsedFiles((prev) => {
const next = new Set<string>();
for (const fp of prev) {
if (allowed.has(fp)) next.add(fp);
}
return next.size === prev.size ? prev : next;
});
}, [activeChangeSet]);
// Load data on open
useEffect(() => {
if (!open) return;
@ -902,6 +1061,8 @@ export const ChangeReviewDialog = ({
<div className="w-64 shrink-0 overflow-y-auto border-r border-border bg-surface-sidebar">
<ReviewFileTree
files={activeChangeSet.files}
fileContents={fileContents}
pathChangeLabels={pathChangeLabels}
selectedFilePath={null}
onSelectFile={handleTreeFileClick}
viewedSet={viewedSet}
@ -964,6 +1125,9 @@ export const ChangeReviewDialog = ({
onAcceptFile={handleAcceptFile}
onRejectFile={handleRejectFile}
onRestoreMissingFile={handleRestoreMissingFile}
pathChangeLabels={pathChangeLabels}
collapsedFiles={collapsedFiles}
onToggleCollapse={toggleCollapsedFile}
onVisibleFileChange={handleVisibleFileChange}
scrollContainerRef={scrollContainerRef}
editorViewMapRef={editorViewMapRef}

View file

@ -41,6 +41,13 @@ interface ContinuousScrollViewProps {
onAcceptFile: (filePath: string) => void;
onRejectFile: (filePath: string) => void;
onRestoreMissingFile?: (filePath: string, content: string) => void;
pathChangeLabels?: Record<
string,
{ kind: 'deleted' } | { kind: 'moved' | 'renamed'; direction: 'from' | 'to'; otherPath: string }
>;
/** Controlled collapsed state (persisted by parent). If omitted, component manages it locally. */
collapsedFiles?: Set<string>;
onToggleCollapse?: (filePath: string) => void;
onVisibleFileChange: (filePath: string) => void;
scrollContainerRef: React.RefObject<HTMLDivElement>;
editorViewMapRef: React.MutableRefObject<Map<string, EditorView>>;
@ -77,6 +84,9 @@ export const ContinuousScrollView = ({
onAcceptFile,
onRejectFile,
onRestoreMissingFile,
pathChangeLabels,
collapsedFiles: collapsedFilesProp,
onToggleCollapse: onToggleCollapseProp,
onVisibleFileChange,
scrollContainerRef,
editorViewMapRef,
@ -87,19 +97,27 @@ export const ContinuousScrollView = ({
onSelectionChange,
}: ContinuousScrollViewProps): React.ReactElement => {
const setFileChunkCount = useStore((s) => s.setFileChunkCount);
const [collapsedFiles, setCollapsedFiles] = useState<Set<string>>(new Set());
const [localCollapsedFiles, setLocalCollapsedFiles] = useState<Set<string>>(() => new Set());
const collapsedFiles = collapsedFilesProp ?? localCollapsedFiles;
const handleToggleCollapse = useCallback((filePath: string) => {
setCollapsedFiles((prev) => {
const next = new Set(prev);
if (next.has(filePath)) {
next.delete(filePath);
} else {
next.add(filePath);
const handleToggleCollapse = useCallback(
(filePath: string) => {
if (onToggleCollapseProp) {
onToggleCollapseProp(filePath);
return;
}
return next;
});
}, []);
setLocalCollapsedFiles((prev) => {
const next = new Set(prev);
if (next.has(filePath)) {
next.delete(filePath);
} else {
next.add(filePath);
}
return next;
});
},
[onToggleCollapseProp]
);
const filePaths = useMemo(() => files.map((f) => f.filePath), [files]);
@ -222,6 +240,7 @@ export const ContinuousScrollView = ({
file={file}
fileContent={content}
fileDecision={decision}
pathChangeLabel={pathChangeLabels?.[filePath]}
hasEdits={hasEdits}
applying={applying}
isCollapsed={isCollapsed}

View file

@ -93,7 +93,7 @@ export const FileSectionDiff = ({
})();
const resolvedOriginal = fileContent?.originalFullContent ?? null;
const isMissingOnDisk = fileContent?.contentSource === 'unavailable';
const isMissingOnDisk = fileContent ? fileContent.modifiedFullContent == null : false;
// Show CodeMirror only when we have a trustworthy original baseline:
// - new files: original is legitimately empty

View file

@ -19,6 +19,9 @@ interface FileSectionHeaderProps {
file: FileChangeSummary;
fileContent: FileChangeWithContent | null;
fileDecision: HunkDecision | undefined;
pathChangeLabel?:
| { kind: 'deleted' }
| { kind: 'moved' | 'renamed'; direction: 'from' | 'to'; otherPath: string };
hasEdits: boolean;
applying: boolean;
isCollapsed: boolean;
@ -34,6 +37,7 @@ export const FileSectionHeader = ({
file,
fileContent,
fileDecision,
pathChangeLabel,
hasEdits,
applying,
isCollapsed,
@ -44,7 +48,8 @@ export const FileSectionHeader = ({
onAcceptFile,
onRejectFile,
}: FileSectionHeaderProps): React.ReactElement => {
const isMissingOnDisk = fileContent?.contentSource === 'unavailable';
const isMissingOnDisk = fileContent ? fileContent.modifiedFullContent == null : false;
const isPreviewOnly = isMissingOnDisk || fileContent?.contentSource === 'unavailable';
const restoreContent =
fileContent?.modifiedFullContent ??
(() => {
@ -54,8 +59,7 @@ export const FileSectionHeader = ({
if (writeSnippets.length === 0) return null;
return writeSnippets[writeSnippets.length - 1].newString;
})();
const canRestore =
!!onRestoreMissingFile && isMissingOnDisk && !hasEdits && restoreContent != null;
const canRestore = !!onRestoreMissingFile && isPreviewOnly && !hasEdits && restoreContent != null;
const handleHeaderClick = (e: React.MouseEvent): void => {
// Don't collapse when clicking action buttons
@ -89,22 +93,41 @@ export const FileSectionHeader = ({
</span>
)}
{pathChangeLabel?.kind === 'deleted' && (
<span className="rounded bg-red-500/20 px-1.5 py-0.5 text-[10px] text-red-300">
DELETED
</span>
)}
{pathChangeLabel && pathChangeLabel.kind !== 'deleted' && (
<Tooltip>
<TooltipTrigger asChild>
<span className="rounded bg-purple-500/20 px-1.5 py-0.5 text-[10px] text-purple-300">
{pathChangeLabel.kind.toUpperCase()}
</span>
</TooltipTrigger>
<TooltipContent side="bottom" className="max-w-xs">
{pathChangeLabel.direction === 'from' ? 'From' : 'To'} {pathChangeLabel.otherPath}
</TooltipContent>
</Tooltip>
)}
{fileContent?.contentSource && (
<Tooltip>
<TooltipTrigger asChild>
<span
className={[
'rounded px-1.5 py-0.5 text-[10px]',
fileContent.contentSource === 'unavailable'
? 'bg-red-500/20 text-red-300'
: 'bg-surface-raised text-text-muted',
isPreviewOnly ? 'bg-red-500/20 text-red-300' : 'bg-surface-raised text-text-muted',
].join(' ')}
>
{CONTENT_SOURCE_LABELS[fileContent.contentSource] ?? fileContent.contentSource}
{isPreviewOnly
? 'Missing on disk'
: (CONTENT_SOURCE_LABELS[fileContent.contentSource] ?? fileContent.contentSource)}
</span>
</TooltipTrigger>
<TooltipContent side="bottom" className="max-w-xs">
{fileContent.contentSource === 'unavailable' ? (
{isPreviewOnly ? (
<div className="space-y-1">
<div className="font-medium text-text">File is missing on disk</div>
<div className="text-text-muted">
@ -148,32 +171,54 @@ export const FileSectionHeader = ({
{(onAcceptFile || onRejectFile) && (
<div className="mr-1 flex items-center gap-1.5">
{onAcceptFile && (
<button
onClick={() => onAcceptFile(file.filePath)}
disabled={applying}
className={[
'rounded px-2 py-1 text-xs font-medium transition-colors disabled:opacity-50',
fileDecision === 'accepted'
? 'bg-green-500/25 text-green-300'
: 'bg-green-500/15 text-green-400 hover:bg-green-500/25',
].join(' ')}
>
Accept
</button>
<Tooltip>
<TooltipTrigger asChild>
<span>
<button
onClick={() => onAcceptFile(file.filePath)}
disabled={applying || isPreviewOnly}
className={[
'rounded px-2 py-1 text-xs font-medium transition-colors disabled:opacity-50',
fileDecision === 'accepted'
? 'bg-green-500/25 text-green-300'
: 'bg-green-500/15 text-green-400 hover:bg-green-500/25',
].join(' ')}
>
Accept
</button>
</span>
</TooltipTrigger>
{isPreviewOnly && (
<TooltipContent side="bottom">
Accept/Reject is disabled while the file is missing on disk.
</TooltipContent>
)}
</Tooltip>
)}
{onRejectFile && (
<button
onClick={() => onRejectFile(file.filePath)}
disabled={applying}
className={[
'rounded px-2 py-1 text-xs font-medium transition-colors disabled:opacity-50',
fileDecision === 'rejected'
? 'bg-red-500/25 text-red-300'
: 'bg-red-500/15 text-red-400 hover:bg-red-500/25',
].join(' ')}
>
Reject
</button>
<Tooltip>
<TooltipTrigger asChild>
<span>
<button
onClick={() => onRejectFile(file.filePath)}
disabled={applying || isPreviewOnly}
className={[
'rounded px-2 py-1 text-xs font-medium transition-colors disabled:opacity-50',
fileDecision === 'rejected'
? 'bg-red-500/25 text-red-300'
: 'bg-red-500/15 text-red-400 hover:bg-red-500/25',
].join(' ')}
>
Reject
</button>
</span>
</TooltipTrigger>
{isPreviewOnly && (
<TooltipContent side="bottom">
Accept/Reject is disabled while the file is missing on disk.
</TooltipContent>
)}
</Tooltip>
)}
</div>
)}

View file

@ -20,10 +20,16 @@ import {
import type { TreeNode } from '@renderer/utils/fileTreeBuilder';
import type { HunkDecision } from '@shared/types';
import type { FileChangeWithContent } from '@shared/types';
import type { FileChangeSummary } from '@shared/types/review';
interface ReviewFileTreeProps {
files: FileChangeSummary[];
fileContents?: Record<string, FileChangeWithContent>;
pathChangeLabels?: Record<
string,
{ kind: 'deleted' } | { kind: 'moved' | 'renamed'; direction: 'from' | 'to'; otherPath: string }
>;
selectedFilePath: string | null;
onSelectFile: (filePath: string) => void;
viewedSet?: Set<string>;
@ -108,6 +114,7 @@ const TreeItem = ({
viewedSet,
collapsedFolders,
onToggleFolder,
pathChangeLabels,
}: {
node: TreeNode<FileChangeSummary>;
selectedFilePath: string | null;
@ -120,11 +127,13 @@ const TreeItem = ({
viewedSet?: Set<string>;
collapsedFolders: Set<string>;
onToggleFolder: (fullPath: string) => void;
pathChangeLabels?: ReviewFileTreeProps['pathChangeLabels'];
}): JSX.Element => {
if (node.isFile && node.data) {
const isSelected = node.data.filePath === selectedFilePath;
const isActive = node.data.filePath === activeFilePath && !isSelected;
const status = getFileStatus(node.data, hunkDecisions, fileDecisions, fileChunkCounts);
const label = pathChangeLabels?.[node.data.filePath];
return (
<button
data-tree-file={node.data.filePath}
@ -164,6 +173,16 @@ const TreeItem = ({
new
</span>
)}
{label?.kind === 'deleted' && (
<span className="shrink-0 rounded bg-red-500/20 px-1.5 py-0.5 text-[10px] font-medium text-red-300">
deleted
</span>
)}
{label && label.kind !== 'deleted' && (
<span className="shrink-0 rounded bg-purple-500/20 px-1.5 py-0.5 text-[10px] font-medium text-purple-300">
{label.kind}
</span>
)}
<span className="ml-1 flex shrink-0 items-center gap-1">
{node.data.linesAdded > 0 && (
<span className="text-green-400">+{node.data.linesAdded}</span>
@ -210,6 +229,7 @@ const TreeItem = ({
viewedSet={viewedSet}
collapsedFolders={collapsedFolders}
onToggleFolder={onToggleFolder}
pathChangeLabels={pathChangeLabels}
/>
))}
</div>
@ -248,6 +268,7 @@ function getAncestorFolderPaths(tree: TreeNode<FileChangeSummary>[], filePath: s
export const ReviewFileTree = ({
files,
pathChangeLabels,
selectedFilePath,
onSelectFile,
viewedSet,
@ -441,6 +462,7 @@ export const ReviewFileTree = ({
viewedSet={viewedSet}
collapsedFolders={collapsedFolders}
onToggleFolder={toggleFolder}
pathChangeLabels={pathChangeLabels}
/>
))}
</div>

View file

@ -131,6 +131,11 @@ export interface ChangeReviewSlice {
file: FileChangeSummary,
options?: { index?: number; content?: FileChangeWithContent }
) => void;
/**
* Clear in-memory review state for a single file after applying changes to disk.
* Prevents stale decisions from being re-applied later and forces fresh content resolve.
*/
clearReviewStateForFile: (filePath: string) => void;
invalidateChangeStats: (teamName: string) => void;
// Editable diff actions
@ -889,6 +894,44 @@ export const createChangeReviewSlice: StateCreator<AppState, [], [], ChangeRevie
});
},
clearReviewStateForFile: (filePath: string) => {
set((s) => {
const nextHunkDecisions = { ...s.hunkDecisions };
const prefix = `${filePath}:`;
for (const key of Object.keys(nextHunkDecisions)) {
if (key.startsWith(prefix) && nextHunkDecisions[key] === 'rejected') {
delete nextHunkDecisions[key];
}
}
const nextFileDecisions = { ...s.fileDecisions };
if (nextFileDecisions[filePath] === 'rejected') {
delete nextFileDecisions[filePath];
}
const nextFileChunkCounts = { ...s.fileChunkCounts };
delete nextFileChunkCounts[filePath];
const nextFileContents = { ...s.fileContents };
delete nextFileContents[filePath];
const nextFileContentsLoading = { ...s.fileContentsLoading };
delete nextFileContentsLoading[filePath];
const nextEditedContents = { ...s.editedContents };
delete nextEditedContents[filePath];
return {
hunkDecisions: nextHunkDecisions,
fileDecisions: nextFileDecisions,
fileChunkCounts: nextFileChunkCounts,
fileContents: nextFileContents,
fileContentsLoading: nextFileContentsLoading,
editedContents: nextEditedContents,
};
});
},
// ── Editable diff actions ──
updateEditedContent: (filePath: string, content: string) => {