diff --git a/src/git.rs b/src/git.rs index f9e072b..deef451 100644 --- a/src/git.rs +++ b/src/git.rs @@ -441,6 +441,28 @@ pub fn read_worktree_file(cwd: impl AsRef, rel_path: &str) -> Result, rel_path: &str, contents: &[u8]) -> Result<()> { + if !is_safe_repo_path(rel_path) { + return Err(GitError::InvalidRepoPath); + } + let root = root(cwd)?; + let canonical_root = root.canonicalize().map_err(|_| GitError::NoWorkdir)?; + let candidate = root.join(rel_path); + let canonical = candidate + .canonicalize() + .map_err(|_| GitError::FileNotFound)?; + if !canonical.starts_with(&canonical_root) { + return Err(GitError::InvalidRepoPath); + } + if !canonical.is_file() { + return Err(GitError::FileNotFound); + } + std::fs::write(&canonical, contents).map_err(|_| GitError::FileNotFound) +} + #[cfg(test)] mod tests { use super::*; @@ -675,6 +697,27 @@ mod tests { )); } + #[test] + fn write_worktree_file_overwrites_existing_files_only() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + git(&root, &["init", "-b", "main"]); + fs::write(root.join("existing.txt"), "before\n").unwrap(); + + write_worktree_file(&root, "existing.txt", b"after\n").unwrap(); + assert_eq!(fs::read(root.join("existing.txt")).unwrap(), b"after\n"); + + assert!(matches!( + write_worktree_file(&root, "created.txt", b"nope"), + Err(GitError::FileNotFound) + )); + assert!(!root.join("created.txt").exists()); + assert!(matches!( + write_worktree_file(&root, "../outside.txt", b"nope"), + Err(GitError::InvalidRepoPath) + )); + } + #[test] fn read_worktree_file_rejects_symlink_escape() { let outside = tempfile::tempdir().unwrap(); diff --git a/src/server.rs b/src/server.rs index 35cebb0..e93038a 100644 --- a/src/server.rs +++ b/src/server.rs @@ -13,7 +13,7 @@ use axum::{ IntoResponse, Response, Sse, sse::{Event, KeepAlive}, }, - routing::{delete, get, post}, + routing::{delete, get, post, put}, }; use notify::{RecursiveMode, Watcher}; use serde::{Deserialize, Serialize}; @@ -119,6 +119,12 @@ struct PullFileQuery { side: Option, } +#[derive(Debug, Deserialize)] +struct WorktreeWriteBody { + path: String, + contents: String, +} + pub struct RunningServer { pub router: Router, _watcher: Option, @@ -183,6 +189,7 @@ pub fn new(cfg: ServerConfig) -> anyhow::Result { ) .route("/api/patch/{org}/{repo}/{number}", get(handle_patch)) .route("/api/blob", get(handle_blob)) + .route("/api/worktree-file", put(handle_write_worktree_file)) .route( "/api/pull/{org}/{repo}/{number}/file", get(handle_pull_file), @@ -557,6 +564,26 @@ async fn handle_blob(State(state): State, Query(query): Query, + Json(body): Json, +) -> Response { + let path = body.path.trim(); + if path.is_empty() { + return error(StatusCode::BAD_REQUEST, "path is required"); + } + if body.contents.len() > MAX_BLOB_BYTES { + return error(StatusCode::PAYLOAD_TOO_LARGE, "contents too large"); + } + match git::write_worktree_file(&state.cwd, path, body.contents.as_bytes()) { + Ok(()) => StatusCode::NO_CONTENT.into_response(), + Err(err) => blob_error(err), + } +} + /// Fetches a single file's raw content from one side of a pull request, for /// `loadDiffFiles` hydration of PR diffs. async fn handle_pull_file( diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index eaa52c0..2b71123 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -20,7 +20,14 @@ import { type FileDiffMetadata, type SelectedLineRange, } from "@pierre/diffs"; -import { CodeView, type CodeViewHandle, useWorkerPool } from "@pierre/diffs/react"; +import { + CodeView, + type CodeViewHandle, + type CodeViewItemEditCompleteHandler, + EditProvider, + useWorkerPool, +} from "@pierre/diffs/react"; +import { Editor, type EditorFactory, type FileDiffEditCompleteEvent } from "@pierre/diffs/edit"; import { applyColorScheme, initialColorScheme, @@ -39,7 +46,7 @@ import { IconExternalLink, IconFileX, } from "@tabler/icons-react"; -import { buttonVariants } from "@/components/ui/button"; +import { Button, buttonVariants } from "@/components/ui/button"; import { Empty, EmptyContent, @@ -379,6 +386,13 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" } | null>(null); const repoContextRequested = useRef(false); const viewerRef = useRef | null>(null); + const [editingItemId, setEditingItemId] = useState(null); + // Mirrors editingItemId for callbacks that must not re-subscribe (SSE load). + const editingItemIdRef = useRef(null); + // Set by Save/Cancel just before toggling edit off; read in onItemEditComplete. + const editDecisionRef = useRef<"accept" | "reject">("reject"); + const pendingDiffReloadRef = useRef(false); + const reloadDiffRef = useRef<(() => void) | null>(null); const codeViewAreaRef = useRef(null); const currentFileRef = useRef(null); const programmaticScrollAtRef = useRef(0); @@ -497,6 +511,12 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" let fallbackInterval: number | undefined; const load = () => { + // A reload replaces the patch and remounts CodeView, which would tear + // down an active edit session; defer until the session completes. + if (editingItemIdRef.current != null) { + pendingDiffReloadRef.current = true; + return; + } const endpoint = isBranch ? `/api/branch-diff?base=${encodeURIComponent(baseRef)}${includeDirty ? "&dirty=1" : ""}` : isLocal @@ -527,6 +547,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" return; } if (!usesLocalStore && (!org || !repo || !number)) return; + reloadDiffRef.current = load; load(); if (usesLocalStore) { eventSource = new EventSource("/api/events"); @@ -901,6 +922,9 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" ]); const onKeyDown = useEffectEvent((e: KeyboardEvent) => { if (e.metaKey || e.ctrlKey || e.altKey) return; + // The inline editor's input lives in shadow DOM, so `target` is the + // shadow host, not an editable element; never steal keys mid-edit. + if (editingItemIdRef.current != null) return; const target = e.target as HTMLElement | null; if (target && (target.isContentEditable || EDITABLE_TAGS.test(target.tagName))) { return; @@ -1153,13 +1177,100 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" } const [oldFile, newFile] = await Promise.all([ loadBlobFileContents(prevObjectId, oldName), - newObjectId && !isZeroOid(newObjectId) - ? loadBlobFileContents(newObjectId, newName) - : loadWorktreeFileContents(newName), + // In local mode the new side is the working tree itself; its `index` + // oid is computed on the fly and generally absent from the object + // database, so read the file instead of the blob. + isLocal && fileDiff.type !== "deleted" + ? loadWorktreeFileContents(newName) + : newObjectId && !isZeroOid(newObjectId) + ? loadBlobFileContents(newObjectId, newName) + : loadWorktreeFileContents(newName), ]); return { oldFile, newFile }; }, - [usesLocalStore, org, repo, number], + [usesLocalStore, isLocal, org, repo, number], + ); + + // Inline editing targets the worktree. The local diff is HEAD → worktree + // (see git::local_diff), so the new side of every non-deleted file is the + // working-tree file itself; PR and branch diffs review committed states and + // stay read-only. + const canEditFile = useCallback( + (fileDiff: FileDiffMetadata): boolean => isLocal && fileDiff.type !== "deleted", + [isLocal], + ); + + const startEditingFile = useCallback((itemId: string) => { + const viewer = viewerRef.current; + const item = viewer?.getItem(itemId); + if (!viewer || !item) return; + editDecisionRef.current = "reject"; + editingItemIdRef.current = itemId; + setEditingItemId(itemId); + // updateItem ignores records whose version is unchanged; bump it. + viewer.updateItem({ ...item, edit: true, version: (item.version ?? 0) + 1 }); + }, []); + + const finishEditingFile = useCallback((itemId: string, decision: "accept" | "reject") => { + editDecisionRef.current = decision; + const viewer = viewerRef.current; + const item = viewer?.getItem(itemId); + if (viewer && item) { + // Toggling edit off ends the session; handleItemEditComplete decides. + viewer.updateItem({ ...item, edit: false, version: (item.version ?? 0) + 1 }); + } else { + editingItemIdRef.current = null; + setEditingItemId(null); + } + }, []); + + const saveWorktreeFile = useCallback(async (path: string, contents: string) => { + try { + await apiFetch("/api/worktree-file", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ path, contents }), + }); + } catch (err) { + console.error(`Failed to save ${path}:`, err); + // Re-sync the view with the worktree, which still has the old content. + reloadDiffRef.current?.(); + } + }, []); + + const handleItemEditComplete = useCallback< + CodeViewItemEditCompleteHandler + >( + (event, item) => { + if (editingItemIdRef.current === item.id) { + editingItemIdRef.current = null; + setEditingItemId(null); + } + const decision = editDecisionRef.current; + editDecisionRef.current = "reject"; + const hadPendingReload = pendingDiffReloadRef.current; + pendingDiffReloadRef.current = false; + // On accept the save itself triggers a watcher reload, so a deferred + // reload only needs to run explicitly on the reject paths. + const reject = () => { + if (hadPendingReload) reloadDiffRef.current?.(); + return "reject" as const; + }; + if (decision !== "accept" || item.type !== "diff") return reject(); + const completed = event as FileDiffEditCompleteEvent; + if (!completed.newFile) return reject(); + // The event is frozen; re-key the accepted diff in place (per the API + // contract) so keyed render caching doesn't serve the replaced value. + completed.fileDiff.cacheKey = `edited:${item.id}:${Date.now()}`; + void saveWorktreeFile(completed.fileDiff.name, completed.newFile.contents); + return "accept"; + }, + [saveWorktreeFile], + ); + + const createEditor = useCallback>( + (editorType, options, editStateKey) => new Editor(editorType, options, editStateKey), + [], ); const codeViewOptions = useMemo( @@ -1276,6 +1387,27 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" const renderHeaderMetadata = useCallback( (item: CodeViewItem) => { if (item.type !== "diff" || !item.fileDiff) return null; + if (editingItemId === item.id) { + return ( +
e.stopPropagation()} + > + + +
+ ); + } const sig = fileSignatures.get(item.fileDiff.name); const isReviewed = sig != null && reviewed.map.get(item.fileDiff.name) === sig; return ( @@ -1309,11 +1441,25 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" startEditingFile(item.id) + : undefined + } /> ); }, - [fileSignatures, reviewed, toggleReviewed, filePatchSections], + [ + fileSignatures, + reviewed, + toggleReviewed, + filePatchSections, + editingItemId, + canEditFile, + startEditingFile, + finishEditingFile, + ], ); if (loading) { @@ -1485,18 +1631,21 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" ) : ( - - key={codeViewKey} - ref={viewerRef} - initialItems={initialItems} - selectedLines={selectedLines} - onSelectedLinesChange={setSelectedLines} - style={codeViewStyle} - options={codeViewOptions} - renderAnnotation={renderAnnotation} - renderHeaderPrefix={renderHeaderPrefix} - renderHeaderMetadata={renderHeaderMetadata} - /> + + + key={codeViewKey} + ref={viewerRef} + initialItems={initialItems} + selectedLines={selectedLines} + onSelectedLinesChange={setSelectedLines} + style={codeViewStyle} + options={codeViewOptions} + renderAnnotation={renderAnnotation} + renderHeaderPrefix={renderHeaderPrefix} + renderHeaderMetadata={renderHeaderMetadata} + onItemEditComplete={handleItemEditComplete} + /> + )} diff --git a/web/src/components/diff-view/FileActionsMenu.tsx b/web/src/components/diff-view/FileActionsMenu.tsx index 31fda36..9f08d61 100644 --- a/web/src/components/diff-view/FileActionsMenu.tsx +++ b/web/src/components/diff-view/FileActionsMenu.tsx @@ -21,9 +21,11 @@ interface CopyAction { export function FileActionsMenu({ path, diffText, + onEdit, }: { path: string; diffText: string | undefined; + onEdit?: () => void; }) { const [copiedKey, setCopiedKey] = useState(null); const resetTimer = useRef(0); @@ -81,6 +83,7 @@ export function FileActionsMenu({ ); })} + {onEdit && Edit file}