diff --git a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.assistedStats.test.ts b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.assistedStats.test.ts index 095b9e06a3..3852532f95 100644 --- a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.assistedStats.test.ts +++ b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.assistedStats.test.ts @@ -7,6 +7,7 @@ import { countUnreadAssistedHunks, getEffectiveReviewFrontendFilters, getEffectiveReviewIncludeUncommitted, + getReviewSubProjectPathspec, normalizeReviewPanelAssistedHunks, } from "./ReviewPanel"; @@ -117,6 +118,39 @@ describe("buildReviewDiffPathFilter", () => { }); }); +describe("getReviewSubProjectPathspec", () => { + test("derives a git-root pathspec for sub-project workspaces", () => { + expect( + getReviewSubProjectPathspec({ + workspaceMetadata: { + projectPath: "/repo/app", + subProjectPath: "/repo/app/packages/api", + }, + projectPath: "/repo/app", + }) + ).toBe("packages/api"); + }); + + test("ignores missing or unrelated sub-project paths", () => { + expect( + getReviewSubProjectPathspec({ + workspaceMetadata: { + projectPath: "/repo/app", + subProjectPath: "/repo/other/packages/api", + }, + projectPath: "/repo/app", + }) + ).toBeNull(); + + expect( + getReviewSubProjectPathspec({ + workspaceMetadata: { projectPath: "/repo/app" }, + projectPath: "/repo/app", + }) + ).toBeNull(); + }); +}); + describe("buildReviewDiffPathFilterSpecs", () => { const workspaceMetadata = { projects: [ @@ -125,6 +159,48 @@ describe("buildReviewDiffPathFilterSpecs", () => { ], }; + test("non-assisted broad diffs default to the sub-project pathspec", () => { + const specs = buildReviewDiffPathFilterSpecs({ + isImmersive: false, + assistedOnly: false, + assistedHunks: [], + selectedFilePath: null, + selectedDiffPath: "", + workspaceMetadata: null, + projectPath: "/repo/app", + subProjectPathspec: "packages/api", + }); + + expect(specs).toEqual([ + { + repoRootProjectPath: "/repo/app", + pathFilter: " -- 'packages/api'", + selectedFilePath: null, + }, + ]); + }); + + test("broad non-assisted diffs stay unfiltered without a sub-project pathspec", () => { + const specs = buildReviewDiffPathFilterSpecs({ + isImmersive: false, + assistedOnly: false, + assistedHunks: [], + selectedFilePath: null, + selectedDiffPath: "", + workspaceMetadata: null, + projectPath: "/repo/app", + subProjectPathspec: null, + }); + + expect(specs).toEqual([ + { + repoRootProjectPath: "/repo/app", + pathFilter: "", + selectedFilePath: null, + }, + ]); + }); + test("assisted mode roots each multi-project pathspec in the pinned file's repository", () => { const specs = buildReviewDiffPathFilterSpecs({ isImmersive: false, @@ -160,6 +236,7 @@ describe("buildReviewDiffPathFilterSpecs", () => { selectedDiffPath: "", workspaceMetadata: null, projectPath: "/repo/app", + subProjectPathspec: "packages/api", pathContext: { projectPath: "/repo/app", executionRootPath: "/repo/app/packages/api", @@ -175,23 +252,45 @@ describe("buildReviewDiffPathFilterSpecs", () => { ]); }); - test("non-assisted mode keeps selected file rooting for truncation recovery", () => { + test("non-assisted mode keeps in-scope selected file rooting for truncation recovery", () => { const specs = buildReviewDiffPathFilterSpecs({ isImmersive: false, assistedOnly: false, - assistedHunks: [{ path: "project-a/src/main.ts" }], - selectedFilePath: "project-b/src/user-selected.ts", - selectedDiffPath: "src/user-selected.ts", - selectedRepoRootProjectPath: "/repo/project-b", + assistedHunks: [{ path: "project-a/packages/api/src/main.ts" }], + selectedFilePath: "project-a/packages/api/src/user-selected.ts", + selectedDiffPath: "packages/api/src/user-selected.ts", + selectedRepoRootProjectPath: "/repo/project-a", workspaceMetadata, projectPath: "/repo/project-a", + subProjectPathspec: "packages/api", }); expect(specs).toEqual([ { - repoRootProjectPath: "/repo/project-b", - pathFilter: " -- 'src/user-selected.ts'", - selectedFilePath: "project-b/src/user-selected.ts", + repoRootProjectPath: "/repo/project-a", + pathFilter: " -- 'packages/api/src/user-selected.ts'", + selectedFilePath: "project-a/packages/api/src/user-selected.ts", + }, + ]); + }); + + test("non-assisted mode ignores stale selected files outside the sub-project", () => { + const specs = buildReviewDiffPathFilterSpecs({ + isImmersive: false, + assistedOnly: false, + assistedHunks: [], + selectedFilePath: "packages/web/src/user-selected.ts", + selectedDiffPath: "packages/web/src/user-selected.ts", + workspaceMetadata: null, + projectPath: "/repo/app", + subProjectPathspec: "packages/api", + }); + + expect(specs).toEqual([ + { + repoRootProjectPath: "/repo/app", + pathFilter: " -- 'packages/api'", + selectedFilePath: null, }, ]); }); diff --git a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx index 25fba12254..bc84866563 100644 --- a/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ReviewPanel.tsx @@ -34,6 +34,7 @@ import React, { } from "react"; import { buildAssistedReviewPathCandidates, + deriveProjectRelativePath, findAssistedCandidateMatch, normalizeAssistedReviewHunks, resolveAssistedReviewPathCandidatesForHunks, @@ -473,6 +474,37 @@ function toPathFilter(pathspecs: readonly string[]): string { : ""; } +function normalizeReviewPath(pathValue: string | null | undefined): string { + return pathValue?.replaceAll("\\", "/").replace(/^\.\//, "").replace(/\/+$/, "").trim() ?? ""; +} + +function reviewPathMatchesPathspec(pathValue: string, pathspec: string): boolean { + const normalizedPath = normalizeReviewPath(pathValue); + const normalizedPathspec = normalizeReviewPath(pathspec); + return ( + normalizedPath === normalizedPathspec || normalizedPath.startsWith(`${normalizedPathspec}/`) + ); +} + +export function getReviewSubProjectPathspec(params: { + workspaceMetadata: + | Pick + | null + | undefined; + projectPath: string; +}): string | null { + const projectPath = params.workspaceMetadata?.projectPath ?? params.projectPath; + const subProjectPath = params.workspaceMetadata?.subProjectPath; + const relativePath = deriveProjectRelativePath(projectPath, subProjectPath); + return relativePath && normalizeReviewPath(relativePath).length > 0 + ? normalizeReviewPath(relativePath) + : null; +} + +export function getReviewSubProjectPathFilter(subProjectPathspec: string | null): string { + return subProjectPathspec ? toPathFilter([subProjectPathspec]) : ""; +} + export function buildReviewDiffPathFilterSpecs(params: { isImmersive: boolean; assistedOnly: boolean; @@ -482,21 +514,25 @@ export function buildReviewDiffPathFilterSpecs(params: { selectedRepoRootProjectPath?: string | null; workspaceMetadata: Pick | null | undefined; projectPath: string; + subProjectPathspec?: string | null; pathContext?: ProjectRelativePathContext; }): ReviewDiffPathFilterSpec[] { if (!params.assistedOnly) { + const selectedFileActive = + params.selectedFilePath && + !params.isImmersive && + (params.subProjectPathspec == null || + reviewPathMatchesPathspec(params.selectedDiffPath, params.subProjectPathspec)); + const subProjectPathFilter = getReviewSubProjectPathFilter(params.subProjectPathspec ?? null); return [ { - repoRootProjectPath: - params.selectedFilePath && !params.isImmersive - ? (params.selectedRepoRootProjectPath ?? params.projectPath) - : params.projectPath, - pathFilter: - params.selectedFilePath && !params.isImmersive - ? toPathFilter([params.selectedDiffPath]) - : "", - selectedFilePath: - params.selectedFilePath && !params.isImmersive ? params.selectedFilePath : null, + repoRootProjectPath: selectedFileActive + ? (params.selectedRepoRootProjectPath ?? params.projectPath) + : params.projectPath, + pathFilter: selectedFileActive + ? toPathFilter([params.selectedDiffPath]) + : subProjectPathFilter, + selectedFilePath: selectedFileActive ? params.selectedFilePath : null, }, ]; } @@ -549,6 +585,7 @@ export function buildReviewDiffPathFilter(params: { selectedDiffPath: string; workspaceMetadata: Pick | null | undefined; repoRootProjectPath: string | null | undefined; + subProjectPathspec?: string | null; pathContext?: ProjectRelativePathContext; }): string { return ( @@ -783,6 +820,13 @@ export const ReviewPanel: React.FC = ({ const originFetchRef = useRef(null); const { api } = useAPI(); const { workspaceMetadata } = useWorkspaceMetadata(); + const workspaceReviewMetadata = workspaceMetadata.get(workspaceId); + const subProjectPathspec = getReviewSubProjectPathspec({ + workspaceMetadata: workspaceReviewMetadata, + projectPath, + }); + const subProjectPathFilter = getReviewSubProjectPathFilter(subProjectPathspec); + const panelRef = useRef(null); const scrollContainerRef = useRef(null); const searchInputRef = useRef(null); @@ -834,16 +878,28 @@ export const ReviewPanel: React.FC = ({ ); const selectedRepoRootProjectPath = resolveRepoRootProjectPath( - workspaceMetadata.get(workspaceId), + workspaceReviewMetadata, selectedFilePath ); const selectedDiffPath = normalizeRepoRootFilePath( - workspaceMetadata.get(workspaceId), + workspaceReviewMetadata, selectedFilePath ? extractNewPath(selectedFilePath) : null, selectedRepoRootProjectPath ); + useEffect(() => { + if (!subProjectPathspec || selectedFilePath === null) { + return; + } + + if (!reviewPathMatchesPathspec(selectedDiffPath, subProjectPathspec)) { + // Sub-project review should not show a stale parent selection; Assisted + // mode remains the explicit escape hatch for agent-pinned hunks outside this scope. + setSelectedFilePath(null); + } + }, [selectedDiffPath, selectedFilePath, setSelectedFilePath, subProjectPathspec]); + const projectDefaultBaseKey = STORAGE_KEYS.reviewDefaultBase(projectPath); const workspaceDiffBaseKey = STORAGE_KEYS.reviewDiffBase(workspaceId); @@ -1068,7 +1124,6 @@ export const ReviewPanel: React.FC = ({ rawWorkspaceStore.getAssistedReviewHunks(workspaceId) ); - const workspaceReviewMetadata = workspaceMetadata.get(workspaceId); const reviewPathContext = useMemo( () => getReviewPanelPathContext({ workspaceMetadata: workspaceReviewMetadata, projectPath }), [projectPath, workspaceReviewMetadata] @@ -1306,14 +1361,14 @@ export const ReviewPanel: React.FC = ({ const numstatCommand = buildGitDiffCommand( filters.diffBase, filters.includeUncommitted, - "", // No path filter for file tree + subProjectPathFilter, "numstat" ); const nameStatusCommand = buildGitDiffCommand( filters.diffBase, filters.includeUncommitted, - "", // No path filter for file tree + subProjectPathFilter, "name-status" ); @@ -1446,6 +1501,7 @@ export const ReviewPanel: React.FC = ({ workspacePath, projectPath, workspaceMetadata, + subProjectPathFilter, filters.diffBase, filters.includeUncommitted, refreshTrigger, @@ -1474,8 +1530,9 @@ export const ReviewPanel: React.FC = ({ selectedFilePath, selectedDiffPath, selectedRepoRootProjectPath, - workspaceMetadata: workspaceMetadata.get(workspaceId), + workspaceMetadata: workspaceReviewMetadata, projectPath, + subProjectPathspec, pathContext: reviewPathContext, }).map((spec) => { const diffCommand = buildGitDiffCommand( @@ -1626,6 +1683,8 @@ export const ReviewPanel: React.FC = ({ workspacePath, projectPath, workspaceMetadata, + workspaceReviewMetadata, + subProjectPathspec, filters.diffBase, filters.includeUncommitted, filters.assistedOnly, @@ -2349,6 +2408,8 @@ export const ReviewPanel: React.FC = ({ workspaceId={workspaceId} workspacePath={workspacePath} refreshTrigger={refreshTrigger} + repoRootProjectPath={projectPath} + pathFilter={subProjectPathFilter} onRefresh={handleRefresh} /> @@ -2438,9 +2499,19 @@ export const ReviewPanel: React.FC = ({
No changes found
- No changes found for the selected diff base. -
- Try selecting a different base or make some changes. + {subProjectPathspec ? ( + <> + No changes found in this sub-project for the selected diff base. +
+ Assisted pins outside this scope still appear in Assisted mode. + + ) : ( + <> + No changes found for the selected diff base. +
+ Try selecting a different base or make some changes. + + )}
{diagnosticInfo && (
diff --git a/src/browser/features/RightSidebar/CodeReview/UntrackedStatus.tsx b/src/browser/features/RightSidebar/CodeReview/UntrackedStatus.tsx index b692067d8f..9ba8aeb3a8 100644 --- a/src/browser/features/RightSidebar/CodeReview/UntrackedStatus.tsx +++ b/src/browser/features/RightSidebar/CodeReview/UntrackedStatus.tsx @@ -12,6 +12,8 @@ interface UntrackedStatusProps { workspacePath: string; refreshTrigger?: number; onRefresh?: () => void; + repoRootProjectPath?: string | null; + pathFilter?: string; } export const UntrackedStatus: React.FC = ({ @@ -19,6 +21,8 @@ export const UntrackedStatus: React.FC = ({ workspacePath, refreshTrigger, onRefresh, + repoRootProjectPath, + pathFilter = "", }) => { const { api } = useAPI(); const [untrackedFiles, setUntrackedFiles] = useState([]); @@ -45,8 +49,8 @@ export const UntrackedStatus: React.FC = ({ try { const result = await api?.workspace.executeBash({ workspaceId, - script: "git ls-files --others --exclude-standard", - options: repoRootBashOptions(5), + script: `git ls-files --others --exclude-standard${pathFilter}`, + options: repoRootBashOptions(5, repoRootProjectPath), }); if (cancelled || !result) return; @@ -81,7 +85,7 @@ export const UntrackedStatus: React.FC = ({ return () => { cancelled = true; }; - }, [api, workspaceId, workspacePath, refreshTrigger]); + }, [api, workspaceId, workspacePath, refreshTrigger, repoRootProjectPath, pathFilter]); const handleTrackAll = async () => { if (untrackedFiles.length === 0 || isTracking) return; @@ -94,7 +98,7 @@ export const UntrackedStatus: React.FC = ({ const result = await api?.workspace.executeBash({ workspaceId, script: `git add -- ${escapedFiles}`, - options: repoRootBashOptions(10), + options: repoRootBashOptions(10, repoRootProjectPath), }); if (result?.success) {