fix: preserve staged and unstaged changes per file (#3104)

This commit is contained in:
José Almeida
2026-08-30 13:26:18 +00:00
committed by GitHub
parent c946140812
commit cc2ee0b43d
48 changed files with 2075 additions and 112 deletions
@@ -90,8 +90,8 @@ export const ReviewDiffList = memo(function ReviewDiffList({
// All in-memory state (selectedFile, reviewedFiles, staleFiles, fileRefs)
// is keyed by `reviewFileKey(file)` so two files at the same path in
// different repos (e.g. `kandev/README.md` + `lvc/README.md`) get
// distinct rows. Without this every multi-repo review with same-named
// files would mark them all reviewed when one is checked.
// distinct rows. Layer-qualified mixed changes use the same key helper so
// reviewing one facet does not mark the other facet reviewed.
const selectedIndex = selectedFile
? files.findIndex((f) => reviewFileKey(f) === selectedFile)
: -1;
@@ -163,6 +163,14 @@ describe("reviewFileKey", () => {
const b = reviewFileKey({ path: "README.md", repository_name: "backend" });
expect(a).not.toBe(b);
});
it("keeps mixed layers distinct without colliding with repository keys", () => {
const staged = reviewFileKey({ path: "mixed.ts", change_layer: "staged" });
const repositoryFile = reviewFileKey({ path: "staged", repository_name: "mixed.ts" });
expect(staged).toBe(`mixed.ts${SEP}${SEP}staged`);
expect(staged).not.toBe(repositoryFile);
});
});
describe("resolvePRReviewRepositoryName", () => {
@@ -240,6 +248,20 @@ describe("splitReviewFileKey", () => {
});
});
it("round-trips a layer-qualified repository key", () => {
const key = reviewFileKey({
path: APP_PATH,
repository_name: "frontend",
change_layer: "unstaged",
});
expect(splitReviewFileKey(key)).toEqual({
repositoryName: "frontend",
path: APP_PATH,
changeLayer: "unstaged",
});
});
it("keeps a legacy bare-path key without a repository scope", () => {
expect(splitReviewFileKey(APP_PATH)).toEqual({ repositoryName: "", path: APP_PATH });
});
+42 -8
View File
@@ -1,6 +1,16 @@
import { djb2Hash } from "@/lib/utils/hash";
import { t } from "@/lib/i18n";
import type { FileChangeStatus } from "@/lib/utils/file-change-status";
import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types";
export type ReviewChangeFacet = {
diff: string;
status: FileChangeStatus;
additions: number;
deletions: number;
old_path?: string;
diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded";
};
export type ReviewFile = {
path: string;
@@ -12,6 +22,10 @@ export type ReviewFile = {
source: "uncommitted" | "committed" | "pr";
old_path?: string;
diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded";
staged_change?: ReviewChangeFacet;
unstaged_change?: ReviewChangeFacet;
/** Frontend-only layer selected from a mixed uncommitted file. */
change_layer?: GitChangeLayer;
/**
* Repository this file belongs to. Set on multi-repo task changes so the
* file tree groups files under per-repo top-level nodes. Optional for
@@ -35,13 +49,22 @@ export type ReviewFile = {
* keep the legacy bare-path key for backwards compatibility with existing
* `session_file_reviews` rows. An explicit empty name represents the real
* workspace root in a multi-repo payload and therefore keeps the separator.
* Layer-qualified mixed changes append a double-NUL marker plus the layer;
* the doubled marker cannot collide with a legacy repository/path key.
*/
const FILE_KEY_SEP = "\u0000";
const FILE_LAYER_KEY_SEP = `${FILE_KEY_SEP}${FILE_KEY_SEP}`;
export function reviewFileKey(file: { path: string; repository_name?: string }): string {
return file.repository_name === undefined
? file.path
: `${file.repository_name}${FILE_KEY_SEP}${file.path}`;
export function reviewFileKey(file: {
path: string;
repository_name?: string;
change_layer?: GitChangeLayer;
}): string {
const baseKey =
file.repository_name === undefined
? file.path
: `${file.repository_name}${FILE_KEY_SEP}${file.path}`;
return file.change_layer ? `${baseKey}${FILE_LAYER_KEY_SEP}${file.change_layer}` : baseKey;
}
/** Mirrors backend `worktree.SanitizeRepoDirName`, which defines the
@@ -124,10 +147,21 @@ export function getCumulativeReviewRepositoryNames(
return Array.from(names);
}
export function splitReviewFileKey(key: string): { repositoryName: string; path: string } {
const sep = key.indexOf(FILE_KEY_SEP);
if (sep < 0) return { repositoryName: "", path: key };
return { repositoryName: key.slice(0, sep), path: key.slice(sep + 1) };
export function splitReviewFileKey(key: string): {
repositoryName: string;
path: string;
changeLayer?: GitChangeLayer;
} {
const layerSep = key.lastIndexOf(FILE_LAYER_KEY_SEP);
const layer = layerSep < 0 ? undefined : key.slice(layerSep + FILE_LAYER_KEY_SEP.length);
const changeLayer = layer === "staged" || layer === "unstaged" ? layer : undefined;
const baseKey = changeLayer ? key.slice(0, layerSep) : key;
const sep = baseKey.indexOf(FILE_KEY_SEP);
const identity =
sep < 0
? { repositoryName: "", path: baseKey }
: { repositoryName: baseKey.slice(0, sep), path: baseKey.slice(sep + 1) };
return changeLayer ? { ...identity, changeLayer } : identity;
}
export function diffSkipReasonLabel(reason?: string): string {
@@ -1,4 +1,7 @@
import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types";
export type DiffSource = "uncommitted" | "committed" | "pr";
export type ChangeLayer = GitChangeLayer;
export type LocalCommitDetailTarget = {
source: "local";
@@ -23,6 +26,7 @@ export type OpenDiffOptions = {
source?: DiffSource;
repositoryName?: string;
prKey?: string;
changeLayer?: ChangeLayer;
};
export type DiffSheetMode =
@@ -33,5 +37,6 @@ export type DiffSheetMode =
sourceFilter?: "all" | DiffSource;
repositoryName?: string;
prKey?: string;
changeLayer?: ChangeLayer;
}
| { kind: "commit"; target: CommitDetailTarget };
@@ -1,6 +1,7 @@
import { useMemo } from "react";
import { IconAlertTriangle } from "@tabler/icons-react";
import { reviewFileKey } from "@/components/review/types";
import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types";
import { useTranslation } from "react-i18next";
/** Composite key for the file currently selected in single-file mode. */
@@ -8,13 +9,18 @@ export function useSelectedFileKey(
mode: "all" | "file",
filePath: string | undefined,
fileRepositoryName: string | undefined,
changeLayer?: GitChangeLayer,
): string | undefined {
return useMemo(
() =>
mode === "file" && filePath
? reviewFileKey({ path: filePath, repository_name: fileRepositoryName })
? reviewFileKey({
path: filePath,
repository_name: fileRepositoryName,
change_layer: changeLayer,
})
: undefined,
[mode, filePath, fileRepositoryName],
[mode, filePath, fileRepositoryName, changeLayer],
);
}
@@ -96,7 +96,12 @@ describe("FileRow truncation (regression: path overlaps diff stats in narrow pan
<TooltipProvider>
<ul>
<FileRow
file={{ ...baseFile, path: "README.md", repositoryName: "frontend" }}
file={{
...baseFile,
path: "README.md",
repositoryName: "frontend",
changeLayer: "staged",
}}
isPending={false}
onSelect={noopSelect}
onOpenDiff={onOpenDiff}
@@ -115,6 +120,7 @@ describe("FileRow truncation (regression: path overlaps diff stats in narrow pan
expect(onOpenDiff).toHaveBeenCalledWith("README.md", {
source: "uncommitted",
repositoryName: "frontend",
changeLayer: "staged",
});
});
@@ -306,6 +312,36 @@ describe("FileRow tree-mode hover stage action", () => {
expect(rightActions).not.toBeNull();
expect(rightActions!.querySelector("button[title='Unstage file']")).toBeNull();
});
it("keeps the tree-mode stage action visible and touch-sized for coarse pointers", () => {
responsive.isFinePointer = false;
const { container } = render(
<TooltipProvider>
<ul>
<FileRow
file={{ ...baseFile, path: "src/mobile.go", staged: true }}
isPending={false}
treeMode
onSelect={noopSelect}
onOpenDiff={noop}
onStage={noop}
onUnstage={noop}
onDiscard={noop}
onEditFile={noop}
/>
</ul>
</TooltipProvider>,
);
const iconSlot = container.querySelector("[data-testid='file-row-icon-action-slot']");
const action = iconSlot?.querySelector("button[title='Unstage file']");
expect(iconSlot?.className).toContain("size-11");
expect(action?.className).toContain("min-h-11");
expect(action?.className).toContain("min-w-11");
expect(action?.parentElement?.className).not.toContain("opacity-0");
});
});
describe("FileRow active-tab highlight", () => {
@@ -21,6 +21,8 @@ import type { OpenDiffOptions } from "./changes-diff-target";
import { useTranslation } from "react-i18next";
import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint";
const COARSE_POINTER_TARGET_CLASS = "min-h-11 min-w-11";
const splitPath = (path: string) => {
const lastSlash = path.lastIndexOf("/");
if (lastSlash === -1) return { folder: "", file: path };
@@ -85,6 +87,7 @@ export function FileRow({
onOpenDiff(file.path, {
source: "uncommitted",
repositoryName: file.repositoryName,
...(file.changeLayer ? { changeLayer: file.changeLayer } : {}),
});
}
};
@@ -112,6 +115,7 @@ export function FileRow({
<TreeModeFileActionSlot
name={name}
isPending={isPending}
isFinePointer={isFinePointer}
staged={file.staged}
path={file.path}
repo={file.repositoryName}
@@ -122,6 +126,7 @@ export function FileRow({
<StageButton
isPending={isPending}
staged={file.staged}
touchSized={!isFinePointer}
path={file.path}
repo={file.repositoryName}
onStage={onStage}
@@ -170,6 +175,7 @@ function FileRowStats({ file, isFinePointer }: { file: ChangedFile; isFinePointe
function TreeModeFileActionSlot({
name,
isPending,
isFinePointer,
staged,
path,
repo,
@@ -178,6 +184,7 @@ function TreeModeFileActionSlot({
}: {
name: string;
isPending: boolean;
isFinePointer: boolean;
staged: boolean;
path: string;
repo?: string;
@@ -187,16 +194,27 @@ function TreeModeFileActionSlot({
return (
<div
data-testid="file-row-icon-action-slot"
className="grid size-4 shrink-0 items-center justify-center [&>*]:col-start-1 [&>*]:row-start-1"
className={cn(
"grid shrink-0 items-center justify-center [&>*]:col-start-1 [&>*]:row-start-1",
isFinePointer ? "size-4" : "size-11",
)}
>
<FileIcon
fileName={name}
className="size-4 transition-opacity group-hover:opacity-0 pointer-events-none"
/>
<div className="opacity-0 transition-opacity pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto">
{isFinePointer && (
<FileIcon
fileName={name}
className="size-4 transition-opacity group-hover:opacity-0 pointer-events-none"
/>
)}
<div
className={cn(
isFinePointer &&
"opacity-0 transition-opacity pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto",
)}
>
<StageButton
isPending={isPending}
staged={staged}
touchSized={!isFinePointer}
path={path}
repo={repo}
onStage={onStage}
@@ -210,6 +228,7 @@ function TreeModeFileActionSlot({
function StageButton({
isPending,
staged,
touchSized,
path,
repo,
onStage,
@@ -217,6 +236,7 @@ function StageButton({
}: {
isPending: boolean;
staged: boolean;
touchSized?: boolean;
path: string;
repo?: string;
onStage: (path: string, repo?: string) => void;
@@ -225,7 +245,12 @@ function StageButton({
const { t } = useTranslation();
if (isPending) {
return (
<div className="flex-shrink-0 flex items-center justify-center size-4">
<div
className={cn(
"flex-shrink-0 flex items-center justify-center size-4",
touchSized && COARSE_POINTER_TARGET_CLASS,
)}
>
<IconLoader2 className="h-3 w-3 animate-spin text-muted-foreground" />
</div>
);
@@ -235,7 +260,10 @@ function StageButton({
<button
type="button"
title={t("task:unstageFile")}
className="group/unstage flex-shrink-0 flex items-center justify-center size-4 rounded bg-emerald-500/20 text-emerald-600 hover:bg-rose-500/20 hover:text-rose-600 cursor-pointer"
className={cn(
"group/unstage flex-shrink-0 flex items-center justify-center size-4 rounded bg-emerald-500/20 text-emerald-600 hover:bg-rose-500/20 hover:text-rose-600 cursor-pointer",
touchSized && COARSE_POINTER_TARGET_CLASS,
)}
onClick={(e) => {
e.stopPropagation();
onUnstage(path, repo);
@@ -250,7 +278,10 @@ function StageButton({
<button
type="button"
title={t("task:stageFile")}
className="flex-shrink-0 flex items-center justify-center size-4 rounded border border-dashed border-muted-foreground/50 text-muted-foreground hover:border-emerald-500 hover:text-emerald-500 hover:bg-emerald-500/10 cursor-pointer"
className={cn(
"flex-shrink-0 flex items-center justify-center size-4 rounded border border-dashed border-muted-foreground/50 text-muted-foreground hover:border-emerald-500 hover:text-emerald-500 hover:bg-emerald-500/10 cursor-pointer",
touchSized && COARSE_POINTER_TARGET_CLASS,
)}
onClick={(e) => {
e.stopPropagation();
onStage(path, repo);
@@ -292,7 +323,7 @@ function FileRowActions({
aria-label={t("task:discardChanges2")}
className={cn(
"text-muted-foreground hover:text-foreground cursor-pointer",
!isFinePointer && "min-h-11 min-w-11",
!isFinePointer && COARSE_POINTER_TARGET_CLASS,
)}
onClick={(e) => {
e.stopPropagation();
@@ -311,7 +342,7 @@ function FileRowActions({
aria-label={t("common:edit")}
className={cn(
"text-muted-foreground hover:text-foreground cursor-pointer",
!isFinePointer && "min-h-11 min-w-11",
!isFinePointer && COARSE_POINTER_TARGET_CLASS,
)}
onClick={(e) => {
e.stopPropagation();
@@ -6,7 +6,11 @@ import { useShallow } from "zustand/react/shallow";
import { useAppStore } from "@/components/state-provider";
import { useDockviewStore } from "@/lib/state/dockview-store";
import type { AppState } from "@/lib/state/store";
import type { FileInfo, GitStatusEntry } from "@/lib/state/slices/session-runtime/types";
import type {
FileChangeFacet,
FileInfo,
GitStatusEntry,
} from "@/lib/state/slices/session-runtime/types";
import { djb2Hash } from "@/lib/utils/hash";
type DockviewPanel = NonNullable<ReturnType<DockviewApi["getPanel"]>>;
@@ -54,6 +58,18 @@ export function autoActivateChangesPanel(): ActivateChangesPanelResult {
const fileFingerprintCache = new WeakMap<FileInfo, string>();
const gitStatusFingerprintCache = new WeakMap<GitStatusEntry, GitStatusFingerprintCacheEntry>();
function changeFacetFingerprint(facet: FileChangeFacet | undefined): string {
if (!facet) return "";
return [
facet.status,
facet.additions ?? 0,
facet.deletions ?? 0,
facet.old_path ?? "",
djb2Hash(facet.diff ?? ""),
facet.diff_skip_reason ?? "",
].join(":");
}
function fileFingerprint(file: FileInfo): string {
const cached = fileFingerprintCache.get(file);
if (cached !== undefined) return cached;
@@ -67,6 +83,8 @@ function fileFingerprint(file: FileInfo): string {
file.old_path ?? "",
djb2Hash(file.diff ?? ""),
file.diff_skip_reason ?? "",
changeFacetFingerprint(file.staged_change),
changeFacetFingerprint(file.unstaged_change),
file.repository_name ?? "",
].join(":");
fileFingerprintCache.set(file, fingerprint);
@@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest";
import {
buildPrByRepoMap,
computeReviewProgress,
mapToChangedFiles,
mapPRFilesToChangedFiles,
selectPRFilesForReviewProgress,
type ReviewProgressPRFile,
@@ -12,6 +13,27 @@ import type { PRDiffFile } from "@/lib/types/github";
const PRIMARY_PR_ID = "primary-pr";
const RENAMED_PATH = "src/renamed.ts";
describe("mapToChangedFiles", () => {
it("preserves a projected mixed-change layer", () => {
expect(
mapToChangedFiles([
{
path: "src/mixed.ts",
status: "modified",
staged: true,
change_layer: "staged",
},
]),
).toEqual([
expect.objectContaining({
path: "src/mixed.ts",
staged: true,
changeLayer: "staged",
}),
]);
});
});
function diffFile(overrides: Partial<PRDiffFile>): PRDiffFile {
return {
filename: "src/app.ts",
@@ -10,7 +10,7 @@ import type { FileInfo } from "@/lib/state/store";
import type { PRDiffFile } from "@/lib/types/github";
import { normalizeFileChangeStatus, type FileChangeStatus } from "@/lib/utils/file-change-status";
import type { PRChangedFile } from "./changes-panel-timeline";
import type { CommitDetailTarget } from "./changes-diff-target";
import type { ChangeLayer, CommitDetailTarget } from "./changes-diff-target";
import type { CommitPresentation } from "./commit-row";
export type ChangedFile = {
@@ -22,6 +22,7 @@ export type ChangedFile = {
oldPath: string | undefined;
/** Repository this file belongs to in multi-repo workspaces; empty for single-repo. */
repositoryName?: string;
changeLayer?: ChangeLayer;
};
/**
@@ -62,6 +63,7 @@ export function mapToChangedFiles(files: FileInfo[]): ChangedFile[] {
minus: file.deletions,
oldPath: file.old_path,
repositoryName: file.repository_name,
changeLayer: file.change_layer,
}));
}
@@ -118,6 +118,8 @@ function DiffViewerContent({
const selectedRepositoryName =
panelKind === "file" ? (params?.repositoryName as string | undefined) : undefined;
const selectedPRKey = panelKind === "file" ? (params?.prKey as string | undefined) : undefined;
const selectedChangeLayer =
panelKind === "file" ? (params?.changeLayer as OpenDiffOptions["changeLayer"]) : undefined;
const sourceFilter = ((params?.source as string) || "all") as "all" | ReviewSource;
const panelSelectedDiff = panelKind === "all" ? selectedDiff : null;
const handleClosePanel = useCallback(() => {
@@ -132,6 +134,7 @@ function DiffViewerContent({
filePath={selectedPath}
fileRepositoryName={selectedRepositoryName}
prKey={selectedPRKey}
changeLayer={selectedChangeLayer}
sourceFilter={sourceFilter}
selectedDiff={panelSelectedDiff}
onClearSelected={() => setSelectedDiff(null)}
@@ -171,6 +174,7 @@ function ChangesContent({ panelId }: { panelId: string }) {
source: options?.source,
repositoryName: options?.repositoryName,
prKey: options?.prKey,
changeLayer: options?.changeLayer,
}),
[addFileDiffPanel],
);
@@ -294,6 +294,8 @@ function DiffViewerContent({
const selectedRepositoryName =
panelKind === "file" ? (params?.repositoryName as string | undefined) : undefined;
const selectedPRKey = panelKind === "file" ? (params?.prKey as string | undefined) : undefined;
const selectedChangeLayer =
panelKind === "file" ? (params?.changeLayer as OpenDiffOptions["changeLayer"]) : undefined;
const sourceFilter = ((params?.source as string) || "all") as "all" | ReviewSource;
const panelSelectedDiff = panelKind === "all" ? selectedDiff : null;
useResyncGitStatusOnTabActivate(panelId, activeSessionId);
@@ -309,6 +311,7 @@ function DiffViewerContent({
filePath={selectedPath}
fileRepositoryName={selectedRepositoryName}
prKey={selectedPRKey}
changeLayer={selectedChangeLayer}
sourceFilter={sourceFilter}
selectedDiff={panelSelectedDiff}
onClearSelected={() => setSelectedDiff(null)}
@@ -352,6 +355,7 @@ function ChangesContent({ panelId }: { panelId: string }) {
source: options?.source,
repositoryName: options?.repositoryName,
prKey: options?.prKey,
changeLayer: options?.changeLayer,
}),
[addFileDiffPanel],
);
@@ -86,6 +86,7 @@ export const MobileChangesPanel = memo(function MobileChangesPanel({
sourceFilter: options?.source ?? "all",
repositoryName: options?.repositoryName || undefined,
prKey: options?.prKey,
changeLayer: options?.changeLayer,
});
}, []);
@@ -178,6 +178,7 @@ function renderPanel(
const filePath = mode.kind === "file" ? mode.path : undefined;
const fileRepositoryName = mode.kind === "file" ? mode.repositoryName : undefined;
const prKey = mode.kind === "file" ? mode.prKey : undefined;
const changeLayer = mode.kind === "file" ? mode.changeLayer : undefined;
const effectiveSourceFilter = mode.kind === "all" ? activeSource : (mode.sourceFilter ?? "all");
return (
<TaskChangesPanel
@@ -185,6 +186,7 @@ function renderPanel(
filePath={filePath}
fileRepositoryName={fileRepositoryName}
prKey={prKey}
changeLayer={changeLayer}
selectedDiff={selectedDiff}
onClearSelected={onClearSelected}
onOpenFile={onOpenFile}
@@ -0,0 +1,126 @@
import { describe, expect, it } from "vitest";
import { renderHook } from "@testing-library/react";
import { hashDiff, reviewFileKey, type ReviewFile } from "@/components/review/types";
import { useSelectedFileKey } from "./changes-panel-banner";
import {
computeChangesReviewSets,
filterVisibleFiles,
reviewDiffHashForKey,
useVisibleDiffState,
} from "./task-changes-panel-state";
const MIXED_PATH = "mixed.ts";
const STAGED_DIFF = "staged diff";
const UNSTAGED_DIFF = "unstaged diff";
function mixedFile(): ReviewFile {
return {
path: MIXED_PATH,
source: "uncommitted",
status: "modified",
staged: false,
additions: 2,
deletions: 0,
diff: "combined",
staged_change: {
status: "modified",
additions: 1,
deletions: 0,
diff: STAGED_DIFF,
},
unstaged_change: {
status: "modified",
additions: 1,
deletions: 0,
diff: UNSTAGED_DIFF,
},
};
}
function projectLayer(file: ReviewFile, changeLayer: "staged" | "unstaged"): ReviewFile {
const [projected] = filterVisibleFiles([file], {
mode: "file",
filePath: MIXED_PATH,
fileRepositoryName: undefined,
sourceFilter: "uncommitted",
changeLayer,
});
return projected;
}
describe("filterVisibleFiles mixed layers", () => {
it("projects the requested layer for a mixed uncommitted file", () => {
expect(projectLayer(mixedFile(), "staged")).toEqual(
expect.objectContaining({
diff: STAGED_DIFF,
staged: true,
change_layer: "staged",
additions: 1,
}),
);
});
it("keeps staged and unstaged review identities independent", () => {
const mixed = mixedFile();
const staged = projectLayer(mixed, "staged");
const unstaged = projectLayer(mixed, "unstaged");
expect(reviewFileKey(staged)).not.toBe(reviewFileKey(unstaged));
});
it("selects the layer-qualified review row", () => {
const { result } = renderHook(() =>
useSelectedFileKey("file", MIXED_PATH, undefined, "staged"),
);
expect(result.current).toBe(reviewFileKey({ path: MIXED_PATH, change_layer: "staged" }));
});
});
describe("mixed layer review state", () => {
it("restores review state against the selected layer diff", () => {
const mixed = mixedFile();
const staged = projectLayer(mixed, "staged");
const stagedKey = reviewFileKey(staged);
const state = computeChangesReviewSets(
[mixed],
new Map([[stagedKey, { reviewed: true, diffHash: hashDiff(staged.diff) }]]),
);
expect(state.reviewedFiles).toEqual(new Set([stagedKey]));
expect(state.staleFiles).toEqual(new Set());
});
it("persists the selected layer diff hash", () => {
const mixed = mixedFile();
const staged = projectLayer(mixed, "staged");
expect(reviewDiffHashForKey([mixed], reviewFileKey(staged))).toBe(hashDiff(staged.diff));
});
it("keys the selected layer ref by its review identity", () => {
const mixed = mixedFile();
const ref = { current: null };
const fileRefs = new Map([[reviewFileKey(mixed), ref]]);
const { result } = renderHook(() =>
useVisibleDiffState({
allFiles: [mixed],
rawPRFiles: [],
mode: "file",
filePath: MIXED_PATH,
fileRepositoryName: undefined,
sourceFilter: "uncommitted",
prKey: undefined,
changeLayer: "staged",
fileRefs,
reviewedFiles: new Set(),
staleFiles: new Set(),
}),
);
const [visible] = result.current.visibleFiles;
expect(result.current.visibleFileRefs.get(reviewFileKey(visible))).toBe(ref);
});
});
@@ -1,8 +1,9 @@
"use client";
import { useEffect, useMemo, useRef, type MutableRefObject, type RefObject } from "react";
import { reviewFileKey, splitReviewFileKey, type ReviewFile } from "@/components/review/types";
import { hashDiff, reviewFileKey, type ReviewFile } from "@/components/review/types";
import type { ReviewSource } from "@/hooks/domains/session/use-review-sources";
import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types";
export function shouldDeferReviewStateForPR(
hasSelectedPR: boolean,
@@ -104,13 +105,84 @@ type FilterVisibleFilesOpts = {
sourceFilter: "all" | ReviewSource;
rawPRFiles?: ReviewFile[];
prKey?: string;
changeLayer?: GitChangeLayer;
};
function projectReviewFileLayer(file: ReviewFile, layer: GitChangeLayer): ReviewFile | null {
if (file.source !== "uncommitted") return file;
const facet = layer === "staged" ? file.staged_change : file.unstaged_change;
if (!facet) {
if (file.staged !== (layer === "staged")) return null;
return { ...file, change_layer: layer };
}
return {
...file,
...facet,
staged: layer === "staged",
change_layer: layer,
};
}
type PersistedReviewState = { reviewed: boolean; diffHash: string };
function reviewStateFiles(files: ReviewFile[]): ReviewFile[] {
const stateFiles: ReviewFile[] = [];
for (const file of files) {
stateFiles.push(file);
if (
file.source !== "uncommitted" ||
file.change_layer ||
!file.staged_change ||
!file.unstaged_change
) {
continue;
}
for (const layer of ["staged", "unstaged"] as const) {
const projected = projectReviewFileLayer(file, layer);
if (projected) stateFiles.push(projected);
}
}
return stateFiles;
}
export function computeChangesReviewSets(
files: ReviewFile[],
reviews: ReadonlyMap<string, PersistedReviewState>,
): { reviewedFiles: Set<string>; staleFiles: Set<string> } {
const reviewedFiles = new Set<string>();
const staleFiles = new Set<string>();
for (const file of reviewStateFiles(files)) {
const key = reviewFileKey(file);
const reviewState = reviews.get(key);
if (!reviewState?.reviewed) continue;
const currentHash = hashDiff(file.diff);
if (reviewState.diffHash && reviewState.diffHash !== currentHash) staleFiles.add(key);
else reviewedFiles.add(key);
}
return { reviewedFiles, staleFiles };
}
export function reviewDiffHashForKey(files: ReviewFile[], key: string): string {
const file = reviewStateFiles(files).find((candidate) => reviewFileKey(candidate) === key);
return file ? hashDiff(file.diff) : "";
}
function projectRequestedLayer(
files: ReviewFile[],
mode: FilterVisibleFilesOpts["mode"],
changeLayer: GitChangeLayer | undefined,
): ReviewFile[] {
if (mode !== "file" || !changeLayer) return files;
return files
.map((file) => projectReviewFileLayer(file, changeLayer))
.filter((file): file is ReviewFile => file !== null);
}
export function filterVisibleFiles(
allFiles: ReviewFile[],
opts: FilterVisibleFilesOpts,
): ReviewFile[] {
const { mode, filePath, fileRepositoryName, sourceFilter, rawPRFiles, prKey } = opts;
const { mode, filePath, fileRepositoryName, sourceFilter, rawPRFiles, prKey, changeLayer } = opts;
const repositoryFilter = prKey && sourceFilter === "pr" ? undefined : fileRepositoryName;
if (mode === "file" && filePath && sourceFilter === "pr" && rawPRFiles?.length) {
let prFiles = rawPRFiles.filter((file) => file.path === filePath && file.source === "pr");
@@ -128,7 +200,7 @@ export function filterVisibleFiles(
}
}
if (sourceFilter !== "all") files = files.filter((file) => file.source === sourceFilter);
return files;
return projectRequestedLayer(files, mode, changeLayer);
}
export function useVisibleDiffState(opts: {
@@ -139,6 +211,7 @@ export function useVisibleDiffState(opts: {
fileRepositoryName: string | undefined;
sourceFilter: "all" | ReviewSource;
prKey: string | undefined;
changeLayer: GitChangeLayer | undefined;
fileRefs: Map<string, RefObject<HTMLDivElement | null>>;
reviewedFiles: Set<string>;
staleFiles: Set<string>;
@@ -151,6 +224,7 @@ export function useVisibleDiffState(opts: {
fileRepositoryName,
sourceFilter,
prKey,
changeLayer,
fileRefs,
reviewedFiles,
staleFiles,
@@ -164,23 +238,21 @@ export function useVisibleDiffState(opts: {
sourceFilter,
rawPRFiles,
prKey,
changeLayer,
}),
[allFiles, mode, filePath, fileRepositoryName, sourceFilter, rawPRFiles, prKey],
[allFiles, mode, filePath, fileRepositoryName, sourceFilter, rawPRFiles, prKey, changeLayer],
);
const repositoryFilter = prKey && sourceFilter === "pr" ? undefined : fileRepositoryName;
const visibleFileRefs = useMemo(() => {
if (mode !== "file" || !filePath) return fileRefs;
const refs = new Map<string, RefObject<HTMLDivElement | null>>();
for (const [key, ref] of fileRefs.entries()) {
const split = splitReviewFileKey(key);
if (split.path !== filePath) continue;
if (repositoryFilter !== undefined && (split.repositoryName || "") !== repositoryFilter) {
continue;
}
refs.set(key, ref);
for (const file of visibleFiles) {
const visibleKey = reviewFileKey(file);
const rawKey = reviewFileKey({ path: file.path, repository_name: file.repository_name });
const ref = fileRefs.get(visibleKey) ?? fileRefs.get(rawKey);
if (ref) refs.set(visibleKey, ref);
}
return refs;
}, [mode, filePath, fileRefs, repositoryFilter]);
}, [mode, filePath, fileRefs, visibleFiles]);
const reviewedCount = useMemo(
() =>
visibleFiles.reduce((count, file) => {
+13 -18
View File
@@ -17,12 +17,14 @@ import { ReviewDiffList } from "@/components/review/review-diff-list";
import { ReviewPRDiffBoundary } from "@/components/review/review-dialog-pr-state";
import { DEFAULT_DIFF_WORD_WRAP } from "@/components/diff/diff-defaults";
import type { ReviewFile } from "@/components/review/types";
import { hashDiff, reviewFileKey, splitReviewFileKey } from "@/components/review/types";
import { reviewFileKey, splitReviewFileKey } from "@/components/review/types";
import { usePanelActions } from "@/hooks/use-panel-actions";
import { useRequestChangesWalkthrough } from "@/hooks/domains/session/use-request-changes-walkthrough";
import { ChangesTopBar } from "./changes-top-bar";
import {
computeChangesReviewSets,
resolveSelectedFileRepositoryName,
reviewDiffHashForKey,
shouldBlockChangesForPR,
shouldDeferReviewStateForPR,
useAutoCloseWhenEmpty,
@@ -31,11 +33,13 @@ import {
import type { SelectedDiff } from "./task-layout";
import { useIsTaskArchived, ArchivedPanelPlaceholder } from "./task-archived-context";
import { useTranslation } from "react-i18next";
import type { GitChangeLayer } from "@/lib/state/slices/session-runtime/types";
type TaskChangesPanelProps = {
mode?: "all" | "file";
filePath?: string;
fileRepositoryName?: string;
changeLayer?: GitChangeLayer;
prKey?: string;
selectedDiff: SelectedDiff | null;
onClearSelected: () => void;
@@ -118,18 +122,7 @@ function useChangesView(
if (shouldDeferReviewStateForPR(Boolean(pr), prDiffLoading, sourceFilter)) {
return { reviewedFiles: reviewed, staleFiles: stale };
}
for (const file of allFiles) {
const key = reviewFileKey(file);
const reviewState = reviews.get(key);
if (!reviewState?.reviewed) continue;
const currentHash = hashDiff(file.diff);
if (reviewState.diffHash && reviewState.diffHash !== currentHash) {
stale.add(key);
} else {
reviewed.add(key);
}
}
return { reviewedFiles: reviewed, staleFiles: stale };
return computeChangesReviewSets(allFiles, reviews);
}, [allFiles, reviews, pr, prDiffLoading, sourceFilter]);
const totalCommentCount = useMemo(() => {
@@ -210,6 +203,7 @@ function useChangesPRPresentation(opts: {
mode: "all" | "file";
filePath: string | undefined;
fileRepositoryName: string | undefined;
changeLayer: GitChangeLayer | undefined;
visibleFiles: ReviewFile[];
}) {
const visibleSelectedFile = opts.visibleFiles.find((file) => file.path === opts.filePath);
@@ -222,6 +216,7 @@ function useChangesPRPresentation(opts: {
opts.fileRepositoryName,
visibleSelectedFile?.repository_name,
),
opts.changeLayer,
);
const blockChangesForPR = shouldBlockChangesForPR(opts.sourceFilter, opts.visibleFiles);
return { selectedFileKey, blockChangesForPR };
@@ -258,8 +253,7 @@ function useChangesActions(
const handleToggleReviewed = useCallback(
(key: string, reviewed: boolean) => {
if (reviewed) {
const file = allFiles.find((f) => reviewFileKey(f) === key);
markReviewed(key, file ? hashDiff(file.diff) : "");
markReviewed(key, reviewDiffHashForKey(allFiles, key));
} else {
markUnreviewed(key);
}
@@ -338,6 +332,7 @@ const TaskChangesPanel = memo(function TaskChangesPanel({
mode = "all",
filePath,
fileRepositoryName,
changeLayer,
prKey,
selectedDiff,
onClearSelected,
@@ -355,14 +350,13 @@ const TaskChangesPanel = memo(function TaskChangesPanel({
const relevantPRLoading = usesPRDiff && view.prDiffLoading;
const actions = useChangesActions(view.activeSessionId, view.allFiles, wordWrapProp);
const handleRequestWalkthrough = useWalkthroughRequest(view.activeSessionId, view.allFiles);
const fileTarget = { filePath, fileRepositoryName, prKey, changeLayer };
const visible = useVisibleDiffState({
allFiles: view.allFiles,
rawPRFiles: view.rawPRFiles,
mode,
filePath,
fileRepositoryName,
...fileTarget,
sourceFilter,
prKey,
fileRefs: view.fileRefs,
reviewedFiles: view.reviewedFiles,
staleFiles: view.staleFiles,
@@ -373,6 +367,7 @@ const TaskChangesPanel = memo(function TaskChangesPanel({
mode,
filePath,
fileRepositoryName,
changeLayer,
visibleFiles: visible.visibleFiles,
});
useAutoCloseWhenEmpty({
@@ -8,6 +8,7 @@ import type { Page } from "@playwright/test";
import fs from "node:fs";
import path from "node:path";
import { execSync } from "node:child_process";
import { waitForDiffText, waitForDiffTextAbsent } from "./diff-update-helpers";
// ---------------------------------------------------------------------------
// Git helper for E2E tests - runs git commands in the test repository
@@ -206,6 +207,71 @@ test.describe("Git Changes Panel", () => {
await expect(session.changes.getByText("test-file.txt")).toBeVisible({ timeout: 15_000 });
});
// @covers AC-PLATFORM-WORKSPACE-GIT-STATUS-001.10
test("shows same path in staged and unstaged sections", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
const profile = await createStandardProfile(apiClient, "Git Mixed Change Profile");
await apiClient.createTaskWithAgent(seedData.workspaceId, "Git Mixed Change Test", profile.id, {
description: "Testing mixed staged and unstaged changes",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
});
const session = await openTaskSession(testPage, "Git Mixed Change Test");
await session.waitForChatIdle();
const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo");
const git = new GitHelper(repoDir, {
...process.env,
HOME: backend.tmpDir,
GIT_AUTHOR_NAME: "E2E Test",
GIT_AUTHOR_EMAIL: "e2e@test.local",
GIT_COMMITTER_NAME: "E2E Test",
GIT_COMMITTER_EMAIL: "e2e@test.local",
});
git.createFile("mixed-layer.txt", "base\n");
git.stageFile("mixed-layer.txt");
git.commit("Add mixed layer fixture");
git.modifyFile("mixed-layer.txt", "base\nSTAGED_LAYER_MARKER\n");
git.stageFile("mixed-layer.txt");
git.modifyFile("mixed-layer.txt", "base\nSTAGED_LAYER_MARKER\nUNSTAGED_LAYER_MARKER\n");
await session.clickTab("Changes");
await expect(session.changes).toBeVisible({ timeout: 10_000 });
await session.expandChangesSection("unstaged-files-section");
await session.expandChangesSection("staged-files-section");
const unstagedRow = testPage
.getByTestId("unstaged-files-section")
.getByTestId("file-row-mixed-layer.txt");
const stagedRow = testPage
.getByTestId("staged-files-section")
.getByTestId("file-row-mixed-layer.txt");
await expect(unstagedRow).toBeVisible();
await expect(stagedRow).toBeVisible();
await expect(unstagedRow.getByText("+1", { exact: true })).toBeVisible();
await expect(stagedRow.getByText("+1", { exact: true })).toBeVisible();
await unstagedRow.click();
await waitForDiffText(testPage, "UNSTAGED_LAYER_MARKER");
await stagedRow.click();
await waitForDiffText(testPage, "STAGED_LAYER_MARKER");
await waitForDiffTextAbsent(testPage, "UNSTAGED_LAYER_MARKER");
await stagedRow.getByTitle("Unstage file").click();
await expect(stagedRow).toHaveCount(0);
await expect(unstagedRow).toBeVisible();
await expect(unstagedRow.getByText("+2", { exact: true })).toBeVisible();
});
/**
* Verifies that new untracked files appear in the unstaged section.
*/
@@ -42,6 +42,32 @@ async function expectDiffText(testPage: Page, text: string, timeout = 45_000) {
);
}
async function expectDiffTextAbsent(testPage: Page, text: string, timeout = 10_000) {
await expect
.poll(
() =>
testPage.evaluate((searchText: string) => {
const visibleContainers = Array.from(document.querySelectorAll("diffs-container")).filter(
(container) => {
const bounds = container.getBoundingClientRect();
const style = window.getComputedStyle(container);
return (
bounds.width > 0 &&
bounds.height > 0 &&
style.display !== "none" &&
style.visibility !== "hidden"
);
},
);
return visibleContainers.every(
(container) => !container.shadowRoot?.textContent?.includes(searchText),
);
}, text),
{ timeout },
)
.toBe(true);
}
test.describe("Mobile changes panel", () => {
test.describe.configure({ retries: 1, timeout: 120_000 });
@@ -181,6 +207,74 @@ test.describe("Mobile changes panel", () => {
await testPage.getByTestId("mobile-diff-sheet-close").tap();
});
// @covers AC-PLATFORM-WORKSPACE-GIT-STATUS-001.12
test("shows same path in staged and unstaged sections", async ({
testPage,
apiClient,
seedData,
backend,
}) => {
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Mobile Mixed Change",
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
},
);
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await session.waitForChatIdle();
const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo");
const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir));
git.createFile("mobile-mixed-layer.txt", "base\n");
git.stageFile("mobile-mixed-layer.txt");
git.commit("Add mobile mixed layer fixture");
git.modifyFile("mobile-mixed-layer.txt", "base\nMOBILE_STAGED_LAYER_MARKER\n");
git.stageFile("mobile-mixed-layer.txt");
git.modifyFile(
"mobile-mixed-layer.txt",
"base\nMOBILE_STAGED_LAYER_MARKER\nMOBILE_UNSTAGED_LAYER_MARKER\n",
);
await openMobileChangesPanel(testPage);
await expandSection(testPage, "unstaged-files-section");
await expandSection(testPage, "staged-files-section");
const unstagedRow = testPage
.getByTestId("unstaged-files-section")
.getByTestId("file-row-mobile-mixed-layer.txt");
const stagedRow = testPage
.getByTestId("staged-files-section")
.getByTestId("file-row-mobile-mixed-layer.txt");
await expect(unstagedRow).toBeVisible();
await expect(stagedRow).toBeVisible();
await expect(unstagedRow.getByText("+1", { exact: true })).toBeVisible();
await expect(stagedRow.getByText("+1", { exact: true })).toBeVisible();
await unstagedRow.tap();
await expect(testPage.getByText("File Changes")).toBeVisible({ timeout: 10_000 });
await expectDiffText(testPage, "MOBILE_UNSTAGED_LAYER_MARKER");
await testPage.getByTestId("mobile-diff-sheet-close").tap();
await stagedRow.tap();
await expect(testPage.getByText("File Changes")).toBeVisible({ timeout: 10_000 });
await expectDiffText(testPage, "MOBILE_STAGED_LAYER_MARKER");
await expectDiffTextAbsent(testPage, "MOBILE_UNSTAGED_LAYER_MARKER");
await testPage.getByTestId("mobile-diff-sheet-close").tap();
await stagedRow.getByTitle("Unstage file").tap();
await expect(stagedRow).toHaveCount(0);
await expect(unstagedRow).toBeVisible();
await expect(unstagedRow.getByText("+2", { exact: true })).toBeVisible();
});
test("tapping a PR file row opens file diff sheet with diff content", async ({
testPage,
apiClient,
@@ -0,0 +1,67 @@
import { describe, expect, it } from "vitest";
import type { FileInfo } from "@/lib/state/slices/session-runtime/types";
import { splitFilesByChangeLayer } from "./git-change-facets";
describe("splitFilesByChangeLayer", () => {
it("projects one mixed raw file into staged and unstaged views", () => {
const mixed = {
path: "src/mixed.ts",
status: "modified",
staged: false,
additions: 2,
diff: "combined",
staged_change: {
status: "modified",
additions: 1,
deletions: 0,
diff: "staged diff",
},
unstaged_change: {
status: "modified",
additions: 1,
deletions: 0,
diff: "unstaged diff",
},
} as FileInfo;
const result = splitFilesByChangeLayer([mixed]);
expect(result.stagedFiles).toEqual([
expect.objectContaining({
path: "src/mixed.ts",
staged: true,
change_layer: "staged",
additions: 1,
diff: "staged diff",
}),
]);
expect(result.unstagedFiles).toEqual([
expect.objectContaining({
path: "src/mixed.ts",
staged: false,
change_layer: "unstaged",
additions: 1,
diff: "unstaged diff",
}),
]);
expect(mixed.diff).toBe("combined");
});
it("keeps legacy files in exactly one section", () => {
const staged = {
path: "staged.ts",
status: "added",
staged: true,
} as FileInfo;
const unstaged = {
path: "unstaged.ts",
status: "modified",
staged: false,
} as FileInfo;
expect(splitFilesByChangeLayer([staged, unstaged])).toEqual({
stagedFiles: [staged],
unstagedFiles: [unstaged],
});
});
});
@@ -0,0 +1,51 @@
import type {
FileChangeFacet,
FileInfo,
GitChangeLayer,
} from "@/lib/state/slices/session-runtime/types";
function projectFileChange(
file: FileInfo,
layer: GitChangeLayer,
facet: FileChangeFacet,
): FileInfo {
return {
...file,
status: facet.status,
staged: layer === "staged",
additions: facet.additions,
deletions: facet.deletions,
old_path: facet.old_path,
diff: facet.diff,
diff_skip_reason: facet.diff_skip_reason,
change_layer: layer,
};
}
/**
* Builds section-specific file views while preserving one canonical raw file
* per repository/path for counts and mutations.
*/
export function splitFilesByChangeLayer(allFiles: FileInfo[]): {
stagedFiles: FileInfo[];
unstagedFiles: FileInfo[];
} {
const stagedFiles: FileInfo[] = [];
const unstagedFiles: FileInfo[] = [];
for (const file of allFiles) {
const hasFacets = Boolean(file.staged_change || file.unstaged_change);
if (!hasFacets) {
(file.staged ? stagedFiles : unstagedFiles).push(file);
continue;
}
if (file.staged_change) {
stagedFiles.push(projectFileChange(file, "staged", file.staged_change));
}
if (file.unstaged_change) {
unstagedFiles.push(projectFileChange(file, "unstaged", file.unstaged_change));
}
}
return { stagedFiles, unstagedFiles };
}
@@ -0,0 +1,38 @@
import { describe, expect, it } from "vitest";
import { buildReviewSources } from "./use-review-sources";
describe("buildReviewSources mixed facets", () => {
it("preserves staged and unstaged facets on an uncommitted source", () => {
const result = buildReviewSources({
gitStatus: {
files: {
"src/mixed.ts": {
diff: "combined",
status: "modified",
staged: false,
staged_change: {
status: "modified",
additions: 1,
deletions: 0,
diff: "staged diff",
},
unstaged_change: {
status: "modified",
additions: 1,
deletions: 0,
diff: "unstaged diff",
},
} as never,
},
},
statusByRepo: undefined,
cumulativeDiff: null,
prDiffFiles: undefined,
});
expect(result.allFiles[0]).toMatchObject({
staged_change: { additions: 1, diff: "staged diff" },
unstaged_change: { additions: 1, diff: "unstaged diff" },
});
});
});
@@ -18,7 +18,7 @@ import {
suppressAvailableGitlinkFiles,
} from "@/components/review/types";
import { createDebugLogger } from "@/lib/debug/log";
import type { ReviewFile } from "@/components/review/types";
import type { ReviewChangeFacet, ReviewFile } from "@/components/review/types";
import type { PRDiffFile, TaskPR } from "@/lib/types/github";
import { normalizeFileChangeStatus } from "@/lib/utils/file-change-status";
import { prTaskKey } from "@/components/github/pr-utils";
@@ -38,6 +38,17 @@ type UncommittedFile = {
deletions?: number;
staged?: boolean;
is_submodule?: boolean;
staged_change?: UncommittedChangeFacet;
unstaged_change?: UncommittedChangeFacet;
};
type UncommittedChangeFacet = {
diff?: string;
diff_skip_reason?: ReviewFile["diff_skip_reason"];
status?: string;
old_path?: string;
additions?: number;
deletions?: number;
};
type CumulativeFile = {
@@ -78,12 +89,28 @@ function addUncommittedFiles(
source: "uncommitted",
old_path: file.old_path,
diff_skip_reason: skipReason,
staged_change: normalizeUncommittedFacet(file.staged_change),
unstaged_change: normalizeUncommittedFacet(file.unstaged_change),
repository_name: repositoryName,
is_submodule: file.is_submodule ?? isSubmodule,
});
}
}
function normalizeUncommittedFacet(
facet: UncommittedChangeFacet | undefined,
): ReviewChangeFacet | undefined {
if (!facet) return undefined;
return {
diff: facet.diff ? normalizeDiffContent(facet.diff) : "",
status: normalizeFileChangeStatus(facet.status),
additions: facet.additions ?? 0,
deletions: facet.deletions ?? 0,
old_path: facet.old_path,
diff_skip_reason: facet.diff_skip_reason,
};
}
function addCumulativeFiles(
fileMap: Map<string, ReviewFile>,
files: Record<string, CumulativeFile>,
@@ -79,6 +79,23 @@ describe("deriveMultiRepoSummary", () => {
]);
});
it("reports both layers for one mixed file", () => {
const mixed = {
...file("frontend"),
staged_change: { status: "modified" as const },
unstaged_change: { status: "modified" as const },
};
const result = deriveMultiRepoSummary([repoStatus("frontend")], [mixed], ["frontend"]);
expect(result.perRepoStatus).toEqual([
expect.objectContaining({
repository_name: "frontend",
hasStaged: true,
hasUnstaged: true,
}),
]);
});
it("keeps base divergence separate from upstream push and pull counts", () => {
const result = deriveMultiRepoSummary(
[
@@ -1,6 +1,7 @@
import { useMemo } from "react";
import type { FileInfo } from "@/lib/state/slices/session-runtime/types";
import { useSessionGitStatusByRepo } from "./use-session-git-status";
import { splitFilesByChangeLayer } from "./git-change-facets";
type RepositoryStatus = ReturnType<typeof useSessionGitStatusByRepo>[number];
@@ -69,10 +70,14 @@ export function deriveMultiRepoSummary(
const stagedByRepo = new Map<string, boolean>();
const unstagedByRepo = new Map<string, boolean>();
for (const file of allFiles) {
const { stagedFiles, unstagedFiles } = splitFilesByChangeLayer(allFiles);
for (const file of stagedFiles) {
const repositoryName = file.repository_name ?? "";
if (file.staged) stagedByRepo.set(repositoryName, true);
else unstagedByRepo.set(repositoryName, true);
stagedByRepo.set(repositoryName, true);
}
for (const file of unstagedFiles) {
const repositoryName = file.repository_name ?? "";
unstagedByRepo.set(repositoryName, true);
}
const hasNamed = statusByRepo.some((status) => status.repository_name !== "");
const filtered =
@@ -25,6 +25,7 @@ import { useMultiRepoSummary } from "./use-session-git-summary";
import { deriveComparisonValues, deriveSessionGitValues } from "./use-session-git-derived";
import { useScopedStageOperations } from "./use-scoped-stage-operations";
import { normalizeGitStatusFiles } from "@/lib/state/slices/session-runtime/git-status-normalizer";
import { splitFilesByChangeLayer } from "./git-change-facets";
/**
* Per-repo result emitted by frontend-side fan-outs (commit, push, pull,
@@ -626,8 +627,10 @@ function useFileDerivations(
allFilesCount: allFiles.length,
});
}, [statusByRepo, gitStatus, allFiles]);
const unstagedFiles = useMemo(() => allFiles.filter((f) => !f.staged), [allFiles]);
const stagedFiles = useMemo(() => allFiles.filter((f) => f.staged), [allFiles]);
const { stagedFiles, unstagedFiles } = useMemo(
() => splitFilesByChangeLayer(allFiles),
[allFiles],
);
const repoForPath = useMemo(() => {
const m = new Map<string, string>();
for (const f of allFiles) {
+18
View File
@@ -89,6 +89,24 @@ describe("buildGitFileSignature", () => {
} as FileInfo);
expect(before).not.toBe(after);
});
it("changes when only a mixed-change facet changes", () => {
const base = {
path: PATH,
status: "modified" as const,
staged: false,
diff: "combined",
staged_change: { status: "modified" as const, diff: "staged one" },
unstaged_change: { status: "modified" as const, diff: "unstaged" },
};
expect(buildGitFileSignature(base)).not.toBe(
buildGitFileSignature({
...base,
staged_change: { status: "modified", diff: "staged two" },
}),
);
});
});
describe("syncOpenFileFromWorkspace", () => {
+14
View File
@@ -10,6 +10,18 @@ import { updatePanelAfterSave } from "./use-file-save-delete";
import type { FileInfo } from "@/lib/state/store";
import type { GitStatusEntry } from "@/lib/state/slices/session-runtime/types";
function buildChangeFacetSignature(facet: FileInfo["staged_change"]): string {
if (!facet) return "";
return [
facet.status,
facet.additions ?? 0,
facet.deletions ?? 0,
facet.old_path ?? "",
facet.diff ?? "",
facet.diff_skip_reason ?? "",
].join("\0");
}
/**
* Builds a stable signature string from a file's git status entry. The hook
* compares signatures across renders to decide whether the editor's content
@@ -24,6 +36,8 @@ export function buildGitFileSignature(file: FileInfo | undefined): string {
String(file.deletions ?? 0),
file.old_path ?? "",
file.diff ?? "",
buildChangeFacetSignature(file.staged_change),
buildChangeFacetSignature(file.unstaged_change),
].join("|");
}
@@ -403,6 +403,25 @@ describe("addFileDiffPanel — preview behavior", () => {
});
});
describe("addFileDiffPanel — mixed-layer identity", () => {
it("does not reuse a pinned diff for another layer of the same file", () => {
const { api, actions } = build(makeApi());
actions.addFileDiffPanel(SHARED_PATH, {
pin: true,
repositoryName: "frontend",
changeLayer: "staged",
});
actions.addFileDiffPanel(SHARED_PATH, {
repositoryName: "frontend",
changeLayer: "unstaged",
});
const preview = api.getPanel(PREVIEW_DIFF_ID) as unknown as MockPanel;
expect(preview).toBeDefined();
expect(preview.params.changeLayer).toBe("unstaged");
});
});
describe("addCommitDetailPanel — preview behavior", () => {
let api: DockviewApi;
let actions: ReturnType<typeof buildPanelActions>;
+13 -2
View File
@@ -1,5 +1,5 @@
import type { DockviewApi, DockviewGroupPanel } from "dockview-react";
import type { CommitDetailTarget } from "@/components/task/changes-diff-target";
import type { ChangeLayer, CommitDetailTarget } from "@/components/task/changes-diff-target";
import { t } from "@/lib/i18n";
import { focusOrAddPanel } from "./dockview-layout-builders";
import { reviewPanelId, type ReviewPanelTarget } from "./dockview-review-panel-id";
@@ -374,6 +374,11 @@ function buildFileEditorAction(get: StoreGet) {
};
}
function buildFileDiffItemId(path: string, scope: string | undefined, layer?: ChangeLayer): string {
const scopedItemId = buildRepoScopedItemId(path, scope);
return layer ? `${layer}:${scopedItemId}` : scopedItemId;
}
/**
* Build the `addFileDiffPanel` action: opens the file-diff preview (or pinned
* panel) for a path, passing content/source/repository/pr params through and
@@ -388,11 +393,16 @@ function buildFileDiffAction(get: StoreGet) {
source?: string;
repositoryName?: string;
prKey?: string;
changeLayer?: ChangeLayer;
},
) => {
const { api, centerGroupId } = get();
if (!api) return;
const itemId = buildRepoScopedItemId(path, opts?.prKey ?? opts?.repositoryName);
const itemId = buildFileDiffItemId(
path,
opts?.prKey ?? opts?.repositoryName,
opts?.changeLayer,
);
openOrReplacePreview({
api,
type: "file-diff",
@@ -405,6 +415,7 @@ function buildFileDiffAction(get: StoreGet) {
source: opts?.source,
repositoryName: opts?.repositoryName,
prKey: opts?.prKey,
changeLayer: opts?.changeLayer,
},
groupId: opts?.groupId ?? centerGroupId,
quiet: opts?.quiet,
+1
View File
@@ -149,6 +149,7 @@ type DockviewStore = {
source?: string;
repositoryName?: string;
prKey?: string;
changeLayer?: import("@/components/task/changes-diff-target").ChangeLayer;
},
) => void;
addCommitDetailPanel: (
@@ -5,6 +5,7 @@ import type {
SessionPollMode,
GitStatusEntry,
FileInfo,
FileChangeFacet,
} from "./types";
import { createDebugLogger, isDebug } from "@/lib/debug/log";
import { normalizeGitStatusEntry } from "./git-status-normalizer";
@@ -50,10 +51,24 @@ const COMPARABLE_FILE_FIELDS = [
"old_path",
"diff",
"diff_skip_reason",
"staged_change",
"unstaged_change",
"repository_name",
"is_submodule",
] as const;
function comparableChangeFacet(facet: FileChangeFacet | undefined): string {
if (!facet) return "";
return [
facet.status,
facet.additions ?? 0,
facet.deletions ?? 0,
facet.old_path ?? "",
facet.diff ?? "",
facet.diff_skip_reason ?? "",
].join("\0");
}
function comparableFileInfo(file: FileInfo) {
return {
path: file.path,
@@ -64,6 +79,8 @@ function comparableFileInfo(file: FileInfo) {
old_path: file.old_path ?? "",
diff: file.diff ?? "",
diff_skip_reason: file.diff_skip_reason ?? "",
staged_change: comparableChangeFacet(file.staged_change),
unstaged_change: comparableChangeFacet(file.unstaged_change),
repository_name: file.repository_name ?? "",
is_submodule: file.is_submodule ?? false,
};
@@ -9,6 +9,8 @@ function makeStore() {
}
const SESSION = "sess-1";
const MODIFIED_STATUS = "modified" as const;
const NEWER_TIMESTAMP = "2026-05-28T00:01:00Z";
function status(overrides: Partial<GitStatusEntry> = {}): GitStatusEntry {
return {
@@ -22,7 +24,7 @@ function status(overrides: Partial<GitStatusEntry> = {}): GitStatusEntry {
ahead: 0,
behind: 0,
files: {
"a.ts": { path: "a.ts", status: "modified", staged: false, diff: "-old\n+new" },
"a.ts": { path: "a.ts", status: MODIFIED_STATUS, staged: false, diff: "-old\n+new" },
},
timestamp: "2026-05-28T00:00:00Z",
...overrides,
@@ -39,15 +41,15 @@ describe("setGitStatus change reporting (single deep compare)", () => {
it("returns true on the first status and false for an identical follow-up", () => {
expect(store.getState().setGitStatus(SESSION, status())).toBe(true);
// Same content, newer timestamp only — must not count as a change.
expect(
store.getState().setGitStatus(SESSION, status({ timestamp: "2026-05-28T00:01:00Z" })),
).toBe(false);
expect(store.getState().setGitStatus(SESSION, status({ timestamp: NEWER_TIMESTAMP }))).toBe(
false,
);
});
it("does not rewrite byEnvironmentId for a duplicate snapshot", () => {
store.getState().setGitStatus(SESSION, status());
const ref = store.getState().gitStatus.byEnvironmentId[SESSION];
store.getState().setGitStatus(SESSION, status({ timestamp: "2026-05-28T00:01:00Z" }));
store.getState().setGitStatus(SESSION, status({ timestamp: NEWER_TIMESTAMP }));
expect(store.getState().gitStatus.byEnvironmentId[SESSION]).toBe(ref);
});
@@ -57,13 +59,42 @@ describe("setGitStatus change reporting (single deep compare)", () => {
SESSION,
status({
files: {
"a.ts": { path: "a.ts", status: "modified", staged: false, diff: "-old\n+newer" },
"a.ts": { path: "a.ts", status: MODIFIED_STATUS, staged: false, diff: "-old\n+newer" },
},
}),
);
expect(changed).toBe(true);
});
it("returns true when only a mixed-change facet changes", () => {
const first = status({
files: {
"a.ts": {
path: "a.ts",
status: MODIFIED_STATUS,
staged: false,
diff: "combined",
staged_change: { status: MODIFIED_STATUS, diff: "staged one" },
unstaged_change: { status: MODIFIED_STATUS, diff: "unstaged" },
} as never,
},
});
store.getState().setGitStatus(SESSION, first);
const changed = store.getState().setGitStatus(SESSION, {
...first,
timestamp: NEWER_TIMESTAMP,
files: {
"a.ts": {
...first.files["a.ts"],
staged_change: { status: MODIFIED_STATUS, diff: "staged two" },
} as never,
},
});
expect(changed).toBe(true);
});
it("returns true when only the submodule marker changes", () => {
store.getState().setGitStatus(SESSION, status({ is_submodule: false }));
@@ -75,7 +106,7 @@ describe("setGitStatus change reporting (single deep compare)", () => {
SESSION,
status({
files: {
"frontend\u0000src/app.ts": { status: "modified", staged: false } as never,
"frontend\u0000src/app.ts": { status: MODIFIED_STATUS, staged: false } as never,
},
}),
);
@@ -96,7 +127,7 @@ describe("setGitStatus change reporting (single deep compare)", () => {
expect(
store
.getState()
.setGitStatus(SESSION, status({ is_submodule: true, timestamp: "2026-05-28T00:01:00Z" })),
.setGitStatus(SESSION, status({ is_submodule: true, timestamp: NEWER_TIMESTAMP })),
).toBe(false);
});
});
@@ -40,6 +40,17 @@ export type ProcessState = {
devProcessBySessionId: Record<string, string>;
};
export type GitChangeLayer = "staged" | "unstaged";
export type FileChangeFacet = {
status: "modified" | "added" | "deleted" | "untracked" | "renamed";
additions?: number;
deletions?: number;
old_path?: string;
diff?: string;
diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded";
};
export type FileInfo = {
path: string;
status: "modified" | "added" | "deleted" | "untracked" | "renamed";
@@ -49,6 +60,10 @@ export type FileInfo = {
old_path?: string;
diff?: string;
diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded";
staged_change?: FileChangeFacet;
unstaged_change?: FileChangeFacet;
/** Frontend-only projection used when one raw path appears in both change sections. */
change_layer?: GitChangeLayer;
/** Exact old-side ref for cumulative committed diffs. */
base_ref?: string;
/**
+11
View File
@@ -298,6 +298,15 @@ export type OfficeInboxItemNotificationPayload = {
body: string;
};
export type FileChangeFacet = {
status: "modified" | "added" | "deleted" | "untracked" | "renamed";
additions?: number;
deletions?: number;
old_path?: string;
diff?: string;
diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded";
};
export type FileInfo = {
path: string;
status: "modified" | "added" | "deleted" | "untracked" | "renamed";
@@ -307,6 +316,8 @@ export type FileInfo = {
old_path?: string;
diff?: string;
diff_skip_reason?: "too_large" | "binary" | "truncated" | "budget_exceeded";
staged_change?: FileChangeFacet;
unstaged_change?: FileChangeFacet;
};
// Executor and environment payload types (extracted to reduce file size)