Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 53 additions & 17 deletions apps/pi-extension/server/serverReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ import {
validateCodeNavRequest,
extractChangedFiles,
} from "../generated/code-nav.ts";
import {
REPO_FILE_ERROR_STATUS,
readRepoFile,
} from "../generated/repo-file.ts";
import {
createDefaultSemanticDiffRuntime,
getSemanticDiffAvailability,
Expand Down Expand Up @@ -779,6 +783,28 @@ export async function startReviewServer(options: {
}
return options.agentCwd && existsSync(options.agentCwd) ? options.agentCwd : null;
}
/**
* Local working-tree root for /api/code-nav/file and /api/review-file.
* Mirrors resolveLocalFileRoot in packages/server/review.ts. Synchronous
* here because this server's cwd resolution is synchronous (resolveAgentCwd),
* matching how the Pi code-nav routes already behaved.
*/
function resolveLocalFileRoot(
surface: "Code navigation" | "File viewing",
): { ok: true; root: string } | { ok: false; error: string; status: number } {
if (isGitButlerCommittedView()) {
return {
ok: false,
status: 400,
error: `${surface} is unavailable for committed GitButler views`,
};
}
const hasAccess = !!workspace || !!options.gitContext || !!options.agentCwd || !!options.worktreePool;
if (!hasAccess) {
return { ok: false, status: 400, error: `${surface} requires local access` };
}
return { ok: true, root: resolveAgentCwd() };
}
async function ensurePRCallFlowCwd(): Promise<string | null> {
if (options.worktreePool && prMeta) {
try {
Expand Down Expand Up @@ -2975,31 +3001,41 @@ export async function startReviewServer(options: {
json(res, { error: err instanceof Error ? err.message : "Code navigation failed" }, 500);
}
} else if (url.pathname === "/api/code-nav/file" && req.method === "GET") {
if (isGitButlerCommittedView()) {
json(res, { error: "Code navigation is unavailable for committed GitButler views" }, 400);
// Hardened to go through readRepoFile — see the Bun mirror in
// packages/server/review.ts. This route previously had no size cap.
const rootResult = resolveLocalFileRoot("Code navigation");
if (!rootResult.ok) {
json(res, { error: rootResult.error }, rootResult.status);
return;
}
const hasCodeNavAccess = !!workspace || !!options.gitContext || !!options.agentCwd || !!options.worktreePool;
if (!hasCodeNavAccess) {
json(res, { error: "Code navigation requires local access" }, 400);
const result = readRepoFile(rootResult.root, url.searchParams.get("path"));
if (!result.ok) {
json(res, { error: result.message }, REPO_FILE_ERROR_STATUS[result.reason]);
return;
}
const filePath = url.searchParams.get("path");
if (!filePath) {
json(res, { error: "Missing path" }, 400);
json(res, { content: result.content });
} else if (url.pathname === "/api/review-file" && req.method === "GET") {
// Full file content for the full-file review viewer. Serves the live
// working tree with no snapshot guard, by design — see the Bun mirror.
const rootResult = resolveLocalFileRoot("File viewing");
if (!rootResult.ok) {
json(res, { error: rootResult.error }, rootResult.status);
return;
}
try { validateFilePath(filePath); } catch {
json(res, { error: "Invalid path" }, 400);
const result = readRepoFile(rootResult.root, url.searchParams.get("path"));
if (!result.ok) {
json(
res,
{ error: result.message, reason: result.reason, size: result.size },
REPO_FILE_ERROR_STATUS[result.reason],
);
return;
}
try {
const navCwd = resolveAgentCwd();
const content = readFileSync(`${navCwd}/${filePath}`, "utf-8");
json(res, { content });
} catch {
json(res, { error: "File not found" }, 404);
}
json(res, {
filePath: result.filePath,
content: result.content,
size: result.size,
});
} else if (url.pathname === "/api/config" && req.method === "POST") {
try {
const body = (await parseBody(req)) as { displayName?: string; diffOptions?: Record<string, unknown>; theme?: Record<string, unknown>; favicon?: FaviconStyle; reviewAnalysis?: Record<string, unknown>; conventionalComments?: boolean };
Expand Down
2 changes: 1 addition & 1 deletion apps/pi-extension/vendor.sh
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ for f in config-types storage-types workspace-status-types; do
done

# Everything else in the original flat list stays sourced from packages/shared.
for f in prompts review-core generated-files cli-pagination jj-core gitbutler-core vcs-core review-args draft annotate-history pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common markdown-extensions resolve-file file-browser-watch-core annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args annotate-target at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav data-dir semantic-diff-types semantic-diff call-flow-types call-flow-languages call-flow-pack-locks call-flow-install-lock call-flow call-flow-install single-flight source-save-node review-profiles guide-store guide-instructions-store commit-avatars commit-history port-range annotate-client-lease annotate-decision archive-mode tailscale live-proxy-core live-probe live-proxy-node; do
for f in prompts review-core generated-files cli-pagination jj-core gitbutler-core vcs-core review-args draft annotate-history pr-types pr-context-live pr-artifact-document pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common markdown-extensions resolve-file file-browser-watch-core annotate-reference-roots-node worktree worktree-pool html-to-markdown html-diff html-assets html-assets-node url-to-markdown tour annotate-args annotate-target at-reference review-workspace-node review-workspace pfm-reminder improvement-hooks code-nav repo-file data-dir semantic-diff-types semantic-diff call-flow-types call-flow-languages call-flow-pack-locks call-flow-install-lock call-flow call-flow-install single-flight source-save-node review-profiles guide-store guide-instructions-store commit-avatars commit-history port-range annotate-client-lease annotate-decision archive-mode tailscale live-proxy-core live-probe live-proxy-node; do
src="../../packages/shared/$f.ts"
# Shared modules that import browser-safe siblings from @plannotator/core
# (e.g. guide-store → core/guide-format): generated/ is flat and vendors the
Expand Down
68 changes: 64 additions & 4 deletions packages/review-editor/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ import {
REVIEW_CALL_FLOW_PANEL_ID,
REVIEW_ALL_FILES_PANEL_ID,
REVIEW_CODE_NAV_PANEL_ID,
REVIEW_FULL_FILE_PANEL_ID,
getReviewFullFilePanelFilePath,
} from './dock/reviewPanelTypes';
import type { DiffFile, AnnotationScrollTarget } from './types';
import { annotationMatchesPrScope, proseAnnotationMatchesPr } from './utils/annotationScope';
Expand Down Expand Up @@ -337,6 +339,9 @@ const ReviewApp: React.FC = () => {
const apiModeRef = useRef(false);
const analysisSettingsInitialized = useRef(false);
const [isDiffPanelActive, setIsDiffPanelActive] = useState(false);
const [isFullFileActive, setIsFullFileActive] = useState(false);
/** Path the full-file panel currently holds — feeds diff-panel focus arbitration. */
const [fullFileActivePath, setFullFileActivePath] = useState<string | null>(null);
const [allFilesVisibleFile, setAllFilesVisibleFile] = useState<string | null>(null);
const [pendingSelection, setPendingSelection] = useState<SelectedLineRange | null>(null);
const [lineAnnotationComposeRequest, setLineAnnotationComposeRequest] =
Expand Down Expand Up @@ -713,6 +718,39 @@ const ReviewApp: React.FC = () => {
needsInitialDiffPanel.current = false;
}, [dockApi, files, clearPendingSelection]);

/**
* Open a file whole in the full-file panel (design doc phase 1).
*
* One reused panel retargeted per file, exactly like openDiffFile — the
* recommended model over tab-per-file, which cannot easily be walked back.
* Unlike openDiffFile this does NOT require the path to be in `files`: the
* point of the feature is opening files the patch never mentions (code-nav
* results, and later the repo tree).
*/
const openFullFile = useCallback((filePath: string, line?: number) => {
if (!dockApi) return;
const title = filePath.split('/').pop() || filePath;
const existing = dockApi.getPanel(REVIEW_FULL_FILE_PANEL_ID);
if (existing) {
const existingFilePath = getReviewFullFilePanelFilePath(existing.params);
if (existingFilePath !== filePath || line != null) {
existing.api.updateParameters({ filePath, ...(line != null && { line }) });
existing.api.setTitle(title);
}
setFullFileActivePath(filePath);
existing.api.setActive();
return;
}
clearPendingSelection();
dockApi.addPanel({
id: REVIEW_FULL_FILE_PANEL_ID,
component: REVIEW_PANEL_TYPES.FULL_FILE,
title,
params: { filePath, ...(line != null && { line }) },
});
setFullFileActivePath(filePath);
}, [dockApi, clearPendingSelection]);

const isCallFlowNodeInPatch = useCallback((node: CallFlowNode): boolean => {
if (!node.file || !node.line) return false;
const file = files.find((candidate) => candidate.path === node.file || candidate.oldPath === node.file);
Expand Down Expand Up @@ -1138,6 +1176,7 @@ const ReviewApp: React.FC = () => {
setIsPROverviewActive(false);
setIsPRArtifactsActive(false);
setIsDiffPanelActive(false);
setIsFullFileActive(false);
return;
}
setIsAllFilesActive(panel.id === REVIEW_ALL_FILES_PANEL_ID);
Expand All @@ -1146,6 +1185,10 @@ const ReviewApp: React.FC = () => {
setIsPROverviewActive(panel.id === REVIEW_PR_OVERVIEW_PANEL_ID);
setIsPRArtifactsActive(panel.id === REVIEW_PR_ARTIFACTS_PANEL_ID);
setIsDiffPanelActive(isReviewDiffPanelId(panel.id));
setIsFullFileActive(panel.id === REVIEW_FULL_FILE_PANEL_ID);
if (panel.id === REVIEW_FULL_FILE_PANEL_ID) {
setFullFileActivePath(getReviewFullFilePanelFilePath(panel.params));
}
if (!isReviewDiffPanelId(panel.id)) return;
const filePath = getReviewDiffPanelFilePath(panel.params);
if (!filePath) return;
Expand Down Expand Up @@ -1841,22 +1884,35 @@ const ReviewApp: React.FC = () => {
originalCode?: string,
conventionalLabel?: ConventionalLabel,
decorations?: ConventionalDecoration[],
tokenMeta?: TokenAnnotationMeta
tokenMeta?: TokenAnnotationMeta,
selectionSnippet?: string
) => {
if (!pendingSelection) return;
const lineStart = Math.min(pendingSelection.start, pendingSelection.end);
const lineEnd = Math.max(pendingSelection.start, pendingSelection.end);
const side = pendingSelection.side === 'additions' ? 'new' : 'old';
// Stamp annotations whose lines the agent will NOT find in the patch:
// authored in the full-file viewer (file absent from the diff, or a range
// past every hunk) or on expanded diff context. Computed here rather than
// at export time because only this side of the app holds the patch.
const filePatch = files.find(candidate => candidate.path === filePath)?.patch ?? '';
const outsideDiff = !filePatch || !isLineRangeInPatch(filePatch, lineStart, lineEnd, side);
const newAnnotation: CodeAnnotation = {
id: generateId(),
type,
scope: 'line',
filePath,
lineStart,
lineEnd,
side: pendingSelection.side === 'additions' ? 'new' : 'old',
side,
text,
suggestedCode,
originalCode,
// `originalCode` stays suggestion-only in-diff (it renders as
// "Replaces:"). For an out-of-diff annotation the selected lines are
// attached regardless, because the patch does not contain them and the
// export has to quote them for the agent to see anything at all.
originalCode: originalCode ?? (outsideDiff ? selectionSnippet : undefined),
...(outsideDiff && { outsideDiff: true }),
...(tokenMeta && {
charStart: tokenMeta.charStart,
charEnd: tokenMeta.charEnd,
Expand All @@ -1869,7 +1925,7 @@ const ReviewApp: React.FC = () => {
};
setAnnotations(prev => [...prev, withPRContext(newAnnotation)]);
clearPendingSelection();
}, [pendingSelection, identity, withPRContext, clearPendingSelection]);
}, [pendingSelection, identity, withPRContext, clearPendingSelection, files]);

const handleAddCallFlowAnnotation = useCallback((
targets: readonly CallFlowAnnotationTarget[],
Expand Down Expand Up @@ -2906,6 +2962,8 @@ const ReviewApp: React.FC = () => {
// focus claim at the source instead of threading `guideOpen` through every
// dock panel. Guide-side DiffViewers arbitrate focus among themselves.
focusedFilePath: guideOpen ? null : (files[activeFileIndex]?.path ?? null),
fullFileFocusPath: isFullFileActive ? fullFileActivePath : null,
onOpenFullFile: openFullFile,
diffStyle: effectiveDiffStyle,
onDiffStyleChange: handleDiffStyleChange,
isCompactTouchLayout,
Expand Down Expand Up @@ -4116,6 +4174,7 @@ const ReviewApp: React.FC = () => {
>
<SectionsPanel
files={files}
onOpenFile={openFullFile}
sections={sections!}
width={fileTreeResize.width}
activeFileIndex={isAllFilesActive || isSemanticDiffActive || isCallFlowActive || isPROverviewActive ? -1 : activeFileIndex}
Expand Down Expand Up @@ -4207,6 +4266,7 @@ const ReviewApp: React.FC = () => {
>
<FileTree
files={files}
onOpenFile={openFullFile}
activeFileIndex={activeFileIndex}
onSelectPROverview={() => completeNavigatorSelection(openPROverviewPanel)}
isPROverviewActive={isPROverviewActive}
Expand Down
6 changes: 6 additions & 0 deletions packages/review-editor/components/DiffViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,12 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
<ToolbarHost
ref={toolbarHostRef}
patch={patch}
// Lets an annotation on EXPANDED CONTEXT carry real code. Those lines
// exist in the file but in no hunk, so the patch-only extractor
// returned an empty snippet for them.
fileContent={
fileContents?.forPath === filePath ? fileContents.new ?? undefined : undefined
}
filePath={filePath}
isFocused={isFocused}
onLineSelection={onLineSelection}
Expand Down
4 changes: 4 additions & 0 deletions packages/review-editor/components/FileTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ interface FileTreeProps {
activeFileIndex: number;
onSelectFile: (index: number) => void;
onDoubleClickFile?: (index: number) => void;
/** Open a file whole in the full-file viewer (design doc phase 1). */
onOpenFile?: (filePath: string) => void;
annotations: CodeAnnotation[];
viewedFiles: Set<string>;
onToggleViewed?: (filePath: string) => void;
Expand Down Expand Up @@ -127,6 +129,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
activeFileIndex,
onSelectFile,
onDoubleClickFile,
onOpenFile,
annotations,
viewedFiles,
onToggleViewed,
Expand Down Expand Up @@ -565,6 +568,7 @@ export const FileTree: React.FC<FileTreeProps> = ({
scrollHighlightIndex={isAllFilesActive ? scrollHighlightIndex : undefined}
onSelectFile={onSelectFile}
onDoubleClickFile={onDoubleClickFile}
onOpenFile={onOpenFile}
viewedFiles={viewedFiles}
onToggleViewed={onToggleViewed}
showViewedControls={showViewedControls}
Expand Down
30 changes: 30 additions & 0 deletions packages/review-editor/components/FileTreeNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ interface FileTreeNodeProps {
activeFileIndex: number;
onSelectFile: (index: number) => void;
onDoubleClickFile?: (index: number) => void;
onOpenFile?: (filePath: string) => void;
viewedFiles: Set<string>;
onToggleViewed?: (filePath: string) => void;
showViewedControls?: boolean;
Expand Down Expand Up @@ -55,6 +56,7 @@ export const FileTreeNodeItem: React.FC<FileTreeNodeProps> = ({
activeFileIndex,
onSelectFile,
onDoubleClickFile,
onOpenFile,
viewedFiles,
onToggleViewed,
showViewedControls = true,
Expand Down Expand Up @@ -114,6 +116,7 @@ export const FileTreeNodeItem: React.FC<FileTreeNodeProps> = ({
activeFileIndex={activeFileIndex}
onSelectFile={onSelectFile}
onDoubleClickFile={onDoubleClickFile}
onOpenFile={onOpenFile}
viewedFiles={viewedFiles}
onToggleViewed={onToggleViewed}
showViewedControls={showViewedControls}
Expand Down Expand Up @@ -191,11 +194,38 @@ export const FileTreeNodeItem: React.FC<FileTreeNodeProps> = ({
<span className="truncate">{node.name}</span>
<AnnotationBadge count={annotationCount} />
</div>
{onOpenFile && (
<span
role="button"
tabIndex={-1}
data-testid={`open-full-file:${node.path}`}
title="Open whole file"
aria-label={`Open whole file ${node.path}`}
className="opacity-0 group-hover:opacity-100 focus:opacity-100 flex-shrink-0 px-1 text-[10px] text-muted-foreground hover:text-foreground cursor-pointer"
onClick={(e) => {
// The row button owns the click; this affordance opens the
// whole file instead of the diff.
e.stopPropagation();
e.preventDefault();
onOpenFile(node.path);
}}
>
</span>
)}
<DiffCounts additions={node.file!.additions} deletions={node.file!.deletions} />
</ContextMenu.Trigger>
<ContextMenu.Portal>
<ContextMenu.Positioner className="z-50">
<ContextMenu.Popup className="min-w-[160px] bg-popover text-popover-foreground border border-border rounded shadow-lg overflow-hidden py-1 transition-opacity data-starting-style:opacity-0 data-ending-style:opacity-0">
{onOpenFile && (
<ContextMenu.Item
onClick={() => onOpenFile(node.path)}
className="flex items-center gap-2 mx-1 px-2 py-1.5 text-xs rounded cursor-pointer outline-none text-foreground/80 data-[highlighted]:bg-muted data-[highlighted]:text-foreground"
>
Open whole file
</ContextMenu.Item>
)}
<ContextMenu.Item
onClick={() => { void copyTextToClipboard(node.path); }}
className="flex items-center gap-2 mx-1 px-2 py-1.5 text-xs rounded cursor-pointer outline-none text-foreground/80 data-[highlighted]:bg-muted data-[highlighted]:text-foreground"
Expand Down
Loading
Loading