From 46bc4f5f5723bdb12ccc7e50444981bb330e46b8 Mon Sep 17 00:00:00 2001 From: Xin Date: Wed, 5 Aug 2026 01:25:37 +0100 Subject: [PATCH 1/7] feat(web): add Pierre Soft diff theme --- web/src/components/diff-view/helpers.ts | 6 ++++++ web/src/components/diff-view/types.ts | 1 + 2 files changed, 7 insertions(+) diff --git a/web/src/components/diff-view/helpers.ts b/web/src/components/diff-view/helpers.ts index d9bfe1f..e5e5661 100644 --- a/web/src/components/diff-view/helpers.ts +++ b/web/src/components/diff-view/helpers.ts @@ -18,6 +18,12 @@ export const diffThemeOptions: readonly DiffThemeOption[] = [ theme: { dark: "pierre-dark", light: "pierre-light" }, themeType: "system", }, + { + id: "pierre-soft", + label: "Pierre Soft", + theme: { dark: "pierre-dark-soft", light: "pierre-light-soft" }, + themeType: "system", + }, { id: "github", label: "GitHub", diff --git a/web/src/components/diff-view/types.ts b/web/src/components/diff-view/types.ts index cca7216..66546b7 100644 --- a/web/src/components/diff-view/types.ts +++ b/web/src/components/diff-view/types.ts @@ -12,6 +12,7 @@ export type DiffOrderByOption = { }; export type DiffThemeId = | "pierre" + | "pierre-soft" | "github" | "dark-plus" | "light-plus" From b1a02552814e4351e651eb42aa0625055c5576db Mon Sep 17 00:00:00 2001 From: Xin Date: Wed, 5 Aug 2026 01:26:36 +0100 Subject: [PATCH 2/7] feat(web): add PR summary header and pending-comments footer to CodeView --- web/src/components/DiffView.tsx | 73 +++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 3b7ee1a..3bd6024 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -274,6 +274,22 @@ function createPendingThread(target: CommentTarget, body: string): ReviewThread }; } +function pullRequestHeaderState(info: PullRequestInfo): { label: string; className: string } { + if (info.merged) { + return { label: "Merged", className: "bg-purple-500/10 text-purple-600 dark:text-purple-400" }; + } + if (info.state.toLowerCase() === "closed") { + return { label: "Closed", className: "bg-red-500/10 text-red-600 dark:text-red-400" }; + } + if (info.draft) { + return { + label: "Draft", + className: "bg-neutral-500/10 text-neutral-600 dark:text-neutral-400", + }; + } + return { label: "Open", className: "bg-green-500/10 text-green-600 dark:text-green-400" }; +} + export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" } = {}) { const { org, repo, number } = useParams<{ org: string; @@ -1235,6 +1251,61 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" [fileSignatures, reviewed, toggleReviewed, filePatchSections], ); + const renderCodeViewHeader = useCallback(() => { + if (usesLocalStore || !currentPullRequestInfo) return null; + const info = currentPullRequestInfo; + const state = pullRequestHeaderState(info); + return ( +
+
+ + {info.title} + + + {state.label} + +
+
+ {info.author !== "" && {info.author}} + {info.baseRef !== "" && info.headRef !== "" && ( + + {info.baseRef} + + {info.headRef} + + )} + + {info.changedFiles} files + +{info.additions} + −{info.deletions} + +
+
+ ); + }, [usesLocalStore, currentPullRequestInfo]); + + const renderCodeViewFooter = useCallback(() => { + if (pendingCommentThreads.length === 0) return null; + const count = pendingCommentThreads.length; + return ( +
+ + {count} pending {count === 1 ? "comment" : "comments"} + + +
+ ); + }, [pendingCommentThreads, submitPendingComments, submittingPendingComments]); + if (loading) { return (
Loading diff...
@@ -1414,6 +1485,8 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" renderAnnotation={renderAnnotation} renderHeaderPrefix={renderHeaderPrefix} renderHeaderMetadata={renderHeaderMetadata} + renderCodeViewHeader={renderCodeViewHeader} + renderCodeViewFooter={renderCodeViewFooter} /> )} From 5bdfa30feb70222cebac85eef9d8df718fcce5ee Mon Sep 17 00:00:00 2001 From: Xin Date: Wed, 5 Aug 2026 01:26:50 +0100 Subject: [PATCH 3/7] feat: hydrate partial diffs with full file contents Adds a loadDiffFiles loader to the diff viewer so hunk expansion can reach the entire file instead of dead-ending at the patch context. - GET /api/blob serves repo blobs by (abbreviated) object id, or working-tree files by validated repo-relative path (worktree=1) - GET /api/pull/{org}/{repo}/{number}/file proxies one side of a PR file from GitHub via gh, fork-aware at the base/head sha - both cap contents at 5 MB and reject binary (NUL byte) content - the web loader picks blob oid vs worktree vs PR proxy per source and hydrates both sides in parallel; failures leave the diff partial --- src/gh.rs | 99 ++++++++++++++++ src/git.rs | 200 ++++++++++++++++++++++++++++++++ src/server.rs | 105 +++++++++++++++++ web/src/components/DiffView.tsx | 73 ++++++++++++ 4 files changed, 477 insertions(+) diff --git a/src/gh.rs b/src/gh.rs index 5c1bfd1..33f34c1 100644 --- a/src/gh.rs +++ b/src/gh.rs @@ -270,6 +270,73 @@ async fn fetch_pull( )?) } +/// Fetches a single file's raw content from a pull request's old or new side. +/// `side` is `"old"` (base repo at `base.sha`) or anything else, treated as +/// `"new"` (head repo at `head.sha`, fork-aware, falling back to +/// `{org}/{repo}` when GitHub omits `head.repo`, e.g. a deleted fork). +pub async fn pull_request_file( + github_host: &str, + org: &str, + repo: &str, + number: &str, + path: &str, + side: &str, +) -> anyhow::Result> { + let pull = fetch_pull(github_host, org, repo, number).await?; + let (owner, name, sha) = if side == "old" { + let full_name = pull.base.repo.map(|r| r.full_name); + let (owner, name) = split_full_name(full_name.as_deref(), org, repo); + (owner, name, pull.base.sha) + } else { + let full_name = pull.head.repo.map(|r| r.full_name); + let (owner, name) = split_full_name(full_name.as_deref(), org, repo); + (owner, name, pull.head.sha) + }; + if sha.is_empty() { + bail!("pull request {side} sha is missing"); + } + let args = vec![ + "api".to_string(), + format!( + "repos/{owner}/{name}/contents/{}?ref={sha}", + encode_path_segments(path) + ), + "--hostname".to_string(), + github_host.to_string(), + "-H".to_string(), + "Accept: application/vnd.github.raw+json".to_string(), + ]; + run_bytes("gh api pull request file", &args, GH_PATCH_TIMEOUT).await +} + +/// Splits a GitHub `owner/name` full name into its parts, falling back to +/// `(org, repo)` when absent or malformed (e.g. `head.repo` is `null` for a +/// PR whose fork was deleted). +fn split_full_name(full_name: Option<&str>, org: &str, repo: &str) -> (String, String) { + full_name + .and_then(|full| full.split_once('/')) + .filter(|(owner, name)| !owner.is_empty() && !name.is_empty()) + .map(|(owner, name)| (owner.to_string(), name.to_string())) + .unwrap_or_else(|| (org.to_string(), repo.to_string())) +} + +/// Percent-encodes each `/`-separated segment of `path` for use in a `gh api` +/// REST path (so a segment containing e.g. a space or `#` round-trips, and a +/// literal `/` within a segment can't be smuggled in to escape it). +fn encode_path_segments(path: &str) -> String { + let mut base = Url::parse("https://example.invalid").expect("static URL parses"); + { + let mut segments = base + .path_segments_mut() + .expect("base URL is not cannot-be-a-base"); + segments.clear(); + for segment in path.split('/') { + segments.push(segment); + } + } + base.path().trim_start_matches('/').to_string() +} + async fn pull_request_head_sha( github_host: &str, org: &str, @@ -950,4 +1017,36 @@ mod tests { fn convert_github_thread_skips_empty() { assert!(convert_github_thread(ReviewThread::default()).is_none()); } + + #[test] + fn split_full_name_prefers_repo_full_name_falls_back_to_org_repo() { + assert_eq!( + split_full_name(Some("forker/repo"), "org", "repo"), + ("forker".to_string(), "repo".to_string()) + ); + // Missing (deleted fork) falls back to the PR path's org/repo. + assert_eq!( + split_full_name(None, "org", "repo"), + ("org".to_string(), "repo".to_string()) + ); + // Malformed falls back too. + for bad in ["noslash", "/repo", "owner/"] { + assert_eq!( + split_full_name(Some(bad), "org", "repo"), + ("org".to_string(), "repo".to_string()) + ); + } + } + + #[test] + fn encode_path_segments_percent_encodes_each_segment() { + assert_eq!(encode_path_segments("src/main.rs"), "src/main.rs"); + assert_eq!( + encode_path_segments("a dir/file name.txt"), + "a%20dir/file%20name.txt" + ); + // A literal `/` inside a single logical segment cannot smuggle in an + // extra path component. + assert_eq!(encode_path_segments("weird#name"), "weird%23name"); + } } diff --git a/src/git.rs b/src/git.rs index 7d20140..ff989cb 100644 --- a/src/git.rs +++ b/src/git.rs @@ -17,6 +17,14 @@ pub enum GitError { NoWorkdir, #[error("invalid utf-8 path in repository")] InvalidPath, + #[error("invalid object id")] + InvalidOid, + #[error("invalid repository-relative path")] + InvalidRepoPath, + #[error("blob not found")] + BlobNotFound, + #[error("file not found")] + FileNotFound, } pub type Result = std::result::Result; @@ -354,6 +362,65 @@ fn strip_git_path_prefix(path: &str, workdir: &str) -> Option { .map(str::to_string) } +/// Whether `value` is a well-formed (possibly abbreviated) blob object id: 4-64 +/// lowercase hex characters. Checked before it ever reaches git2, so a +/// malformed id is a 400 rather than whatever git2 makes of it. +pub fn is_hex_oid(value: &str) -> bool { + (4..=64).contains(&value.len()) + && value + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) +} + +/// Reads a blob's raw content by (possibly abbreviated) hex object id. +/// Non-blob objects (trees, commits, tags that don't peel to a blob) and +/// unresolvable ids are reported as `BlobNotFound`. +pub fn read_blob(cwd: impl AsRef, oid: &str) -> Result> { + if !is_hex_oid(oid) { + return Err(GitError::InvalidOid); + } + let repo = discover(cwd)?; + let object = repo + .revparse_single(oid) + .map_err(|_| GitError::BlobNotFound)?; + let blob = object.peel_to_blob().map_err(|_| GitError::BlobNotFound)?; + Ok(blob.content().to_vec()) +} + +/// Whether `path` is a safe repository-relative path for a worktree file +/// read: non-empty, relative (no leading `/`), no backslashes, and no empty, +/// `.`, or `..` segments. Does not check the filesystem; callers must still +/// canonicalize and confirm the result stays under the repository root. +pub fn is_safe_repo_path(path: &str) -> bool { + if path.is_empty() || path.starts_with('/') || path.contains('\\') { + return false; + } + path.split('/') + .all(|segment| !segment.is_empty() && segment != "." && segment != "..") +} + +/// Reads a repository-relative working-tree file's raw content. Rejects +/// unsafe paths (see `is_safe_repo_path`) and any path that, once +/// canonicalized, escapes the repository root (e.g. via a symlink). +pub fn read_worktree_file(cwd: impl AsRef, rel_path: &str) -> 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::read(&canonical).map_err(|_| GitError::FileNotFound) +} + #[cfg(test)] mod tests { use super::*; @@ -449,4 +516,137 @@ mod tests { // Paths outside the working tree must not be treated as ignored. assert!(!is_path_ignored(&repo, "/etc/hosts")); } + + #[test] + fn is_hex_oid_validates_length_and_charset() { + for ok in ["abcd", "0123456789abcdef", &"f".repeat(64)] { + assert!(is_hex_oid(ok), "{ok} should be a valid oid"); + } + for bad in ["", "abc", &"a".repeat(65), "ABCD", "abcz", "abc def"] { + assert!(!is_hex_oid(bad), "{bad:?} should be rejected"); + } + } + + #[test] + fn read_blob_resolves_content_and_rejects_bad_input() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + git(root, &["init", "-b", "main"]); + git(root, &["config", "user.email", "diffs@example.com"]); + git(root, &["config", "user.name", "Diffs Test"]); + fs::write(root.join("file.txt"), "hello blob\n").unwrap(); + git(root, &["add", "."]); + git(root, &["commit", "-m", "initial"]); + + let repo = discover(root).unwrap(); + let blob_oid = repo + .head() + .unwrap() + .peel_to_tree() + .unwrap() + .get_path(Path::new("file.txt")) + .unwrap() + .id() + .to_string(); + let tree_oid = repo + .head() + .unwrap() + .peel_to_tree() + .unwrap() + .id() + .to_string(); + + // Full and abbreviated oids resolve to the blob's content. + let content = read_blob(root, &blob_oid).unwrap(); + assert_eq!(content, b"hello blob\n"); + let content = read_blob(root, &blob_oid[..8]).unwrap(); + assert_eq!(content, b"hello blob\n"); + + // Malformed oid strings are rejected before touching git2. + assert!(matches!( + read_blob(root, "not-hex"), + Err(GitError::InvalidOid) + )); + assert!(matches!(read_blob(root, ""), Err(GitError::InvalidOid))); + + // Well-formed but unresolvable oid, and a non-blob oid, both 404. + assert!(matches!( + read_blob(root, "abcdef0123456789"), + Err(GitError::BlobNotFound) + )); + assert!(matches!( + read_blob(root, &tree_oid), + Err(GitError::BlobNotFound) + )); + } + + #[test] + fn is_safe_repo_path_rejects_traversal_and_absolute_paths() { + for ok in ["file.txt", "src/main.rs", "a/b/c.txt"] { + assert!(is_safe_repo_path(ok), "{ok} should be safe"); + } + for bad in [ + "", + "/etc/passwd", + "..", + "../secret", + "a/../b", + "a/./b", + "a//b", + "a\\b", + ] { + assert!(!is_safe_repo_path(bad), "{bad:?} should be rejected"); + } + } + + #[test] + fn read_worktree_file_reads_tracked_and_untracked_files() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + git(&root, &["init", "-b", "main"]); + fs::write(root.join("tracked.txt"), "tracked content\n").unwrap(); + fs::create_dir_all(root.join("nested")).unwrap(); + fs::write(root.join("nested/untracked.txt"), "untracked content\n").unwrap(); + + assert_eq!( + read_worktree_file(&root, "tracked.txt").unwrap(), + b"tracked content\n" + ); + assert_eq!( + read_worktree_file(&root, "nested/untracked.txt").unwrap(), + b"untracked content\n" + ); + + assert!(matches!( + read_worktree_file(&root, "missing.txt"), + Err(GitError::FileNotFound) + )); + assert!(matches!( + read_worktree_file(&root, "nested"), + Err(GitError::FileNotFound) + )); + assert!(matches!( + read_worktree_file(&root, "../outside.txt"), + Err(GitError::InvalidRepoPath) + )); + } + + #[test] + fn read_worktree_file_rejects_symlink_escape() { + let outside = tempfile::tempdir().unwrap(); + fs::write(outside.path().join("secret.txt"), "top secret\n").unwrap(); + + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + git(&root, &["init", "-b", "main"]); + let outside_root = outside.path().canonicalize().unwrap(); + #[cfg(unix)] + std::os::unix::fs::symlink(outside_root.join("secret.txt"), root.join("escape.txt")) + .unwrap(); + #[cfg(unix)] + assert!(matches!( + read_worktree_file(&root, "escape.txt"), + Err(GitError::InvalidRepoPath) + )); + } } diff --git a/src/server.rs b/src/server.rs index ccfbc6b..355d4db 100644 --- a/src/server.rs +++ b/src/server.rs @@ -100,6 +100,19 @@ struct CommentTargetQuery { number: Option, } +#[derive(Debug, Deserialize)] +struct BlobQuery { + oid: Option, + path: Option, + worktree: Option, +} + +#[derive(Debug, Deserialize)] +struct PullFileQuery { + path: Option, + side: Option, +} + pub struct RunningServer { pub router: Router, _watcher: Option, @@ -162,6 +175,11 @@ pub fn new(cfg: ServerConfig) -> anyhow::Result { get(handle_pull_request_info), ) .route("/api/patch/{org}/{repo}/{number}", get(handle_patch)) + .route("/api/blob", get(handle_blob)) + .route( + "/api/pull/{org}/{repo}/{number}/file", + get(handle_pull_file), + ) .fallback(handle_static) .with_state(state); Ok(RunningServer { @@ -494,6 +512,61 @@ async fn handle_patch( } } +/// Serves either a repository blob by object id (`?oid=`) or a working-tree +/// file by repo-relative path (`?path=&worktree=1`), for `loadDiffFiles` +/// hydration of local/branch diffs. The two forms are mutually exclusive. +async fn handle_blob(State(state): State, Query(query): Query) -> Response { + let oid = query.oid.as_deref().unwrap_or_default().trim(); + let path = query.path.as_deref().unwrap_or_default().trim(); + if oid.is_empty() && path.is_empty() { + return error( + StatusCode::BAD_REQUEST, + "oid or path query parameter is required", + ); + } + if !oid.is_empty() && !path.is_empty() { + return error( + StatusCode::BAD_REQUEST, + "oid and path are mutually exclusive", + ); + } + let result = if !oid.is_empty() { + git::read_blob(&state.cwd, oid) + } else if dirty_enabled(query.worktree.as_deref()) { + git::read_worktree_file(&state.cwd, path) + } else { + return error(StatusCode::BAD_REQUEST, "path requires worktree=1"); + }; + match result { + Ok(bytes) => blob_response(bytes), + 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( + State(state): State, + Path((org, repo, number)): Path<(String, String, String)>, + Query(query): Query, +) -> Response { + if let Err(err) = validate_pr_path(&org, &repo, &number) { + return error(StatusCode::BAD_REQUEST, err); + } + let path = query.path.as_deref().unwrap_or_default().trim(); + if !git::is_safe_repo_path(path) { + return error(StatusCode::BAD_REQUEST, "invalid path query parameter"); + } + let side = query.side.as_deref().unwrap_or_default().trim(); + if side != "old" && side != "new" { + return error(StatusCode::BAD_REQUEST, "side must be old or new"); + } + match gh::pull_request_file(&state.github_host, &org, &repo, &number, path, side).await { + Ok(bytes) => blob_response(bytes), + Err(err) => error(StatusCode::BAD_GATEWAY, err), + } +} + async fn handle_static(uri: axum::http::Uri) -> Response { let path = uri.path().trim_start_matches('/'); let path = if path.is_empty() { "index.html" } else { path }; @@ -549,6 +622,38 @@ fn error(status: StatusCode, err: impl std::fmt::Display) -> Response { (status, Json(json!({ "error": err.to_string() }))).into_response() } +/// Cap on hydrated file contents (blob or PR file): large enough for any +/// source file worth diffing, small enough to bound memory for one request. +const MAX_BLOB_BYTES: usize = 5 * 1024 * 1024; + +/// Renders raw file bytes for `loadDiffFiles` hydration: rejects oversized +/// content (413) and binary content, detected via an embedded NUL byte (415), +/// otherwise decodes as UTF-8 (lossily, for non-UTF-8 text). +fn blob_response(bytes: Vec) -> Response { + if bytes.len() > MAX_BLOB_BYTES { + return error( + StatusCode::PAYLOAD_TOO_LARGE, + "file is too large to hydrate", + ); + } + if bytes.contains(&0) { + return error( + StatusCode::UNSUPPORTED_MEDIA_TYPE, + "binary file cannot be hydrated", + ); + } + text(String::from_utf8_lossy(&bytes).into_owned()) +} + +fn blob_error(err: git::GitError) -> Response { + match err { + git::GitError::InvalidOid | git::GitError::InvalidRepoPath => { + error(StatusCode::BAD_REQUEST, err) + } + _ => error(StatusCode::NOT_FOUND, err), + } +} + fn valid(check: impl Fn(&str) -> bool, value: &str) -> String { if check(value) { value.to_string() diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 3bd6024..51dd062 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -15,6 +15,8 @@ import { type CodeViewItem, type CodeViewScrollBehavior, type DiffLineAnnotation, + type FileContents, + type FileDiffLoadedFiles, type FileDiffMetadata, type SelectedLineRange, } from "@pierre/diffs"; @@ -274,6 +276,42 @@ function createPendingThread(target: CommentTarget, body: string): ReviewThread }; } +// Local/branch diffs use an all-zeros object id for the working-tree side +// (git diffs against the workdir, not a blob), so that side has to be read +// from disk instead of the object database. +function isZeroOid(oid: string): boolean { + return /^0+$/.test(oid); +} + +// Full-file hydration for `loadDiffFiles`: fetches one side of a PR file from +// its GitHub repo/sha via the server-side proxy (fork-aware; see +// gh::pull_request_file). +async function loadPullRequestFileContents( + org: string, + repo: string, + number: string, + path: string, + side: "old" | "new", +): Promise { + const url = `/api/pull/${encodeURIComponent(org)}/${encodeURIComponent(repo)}/${encodeURIComponent(number)}/file?path=${encodeURIComponent(path)}&side=${side}`; + const contents = await apiFetch(url); + return { name: path, contents }; +} + +// Full-file hydration for a committed blob (local/branch diffs' non-worktree +// side), keyed for highlight cache reuse across renders of the same blob. +async function loadBlobFileContents(oid: string, name: string): Promise { + const contents = await apiFetch(`/api/blob?oid=${encodeURIComponent(oid)}`); + return { name, contents, cacheKey: `blob:${oid}` }; +} + +// Full-file hydration for the zero-oid working-tree side of a local/branch +// diff (uncommitted content, so there's no blob to fetch by oid). +async function loadWorktreeFileContents(path: string): Promise { + const contents = await apiFetch(`/api/blob?path=${encodeURIComponent(path)}&worktree=1`); + return { name: path, contents }; +} + function pullRequestHeaderState(info: PullRequestInfo): { label: string; className: string } { if (info.merged) { return { label: "Merged", className: "bg-purple-500/10 text-purple-600 dark:text-purple-400" }; @@ -1099,6 +1137,39 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" } }, [commentThreads, commentTarget, initialItems, filePathToItemId]); + const loadDiffFiles = useCallback( + async (fileDiff: FileDiffMetadata): Promise => { + if (!usesLocalStore) { + if (!org || !repo || !number) throw new Error("missing pull request target"); + const newPath = fileDiff.name; + if (fileDiff.type === "rename-pure") { + const newFile = await loadPullRequestFileContents(org, repo, number, newPath, "new"); + return { oldFile: null, newFile }; + } + const oldPath = fileDiff.prevName ?? fileDiff.name; + const [oldFile, newFile] = await Promise.all([ + loadPullRequestFileContents(org, repo, number, oldPath, "old"), + loadPullRequestFileContents(org, repo, number, newPath, "new"), + ]); + return { oldFile, newFile }; + } + + const prevObjectId = fileDiff.prevObjectId; + if (!prevObjectId) throw new Error(`missing prevObjectId for ${fileDiff.name}`); + const oldName = fileDiff.prevName ?? fileDiff.name; + const newName = fileDiff.name; + const newObjectId = fileDiff.newObjectId; + const [oldFile, newFile] = await Promise.all([ + loadBlobFileContents(prevObjectId, oldName), + newObjectId && !isZeroOid(newObjectId) + ? loadBlobFileContents(newObjectId, newName) + : loadWorktreeFileContents(newName), + ]); + return { oldFile, newFile }; + }, + [usesLocalStore, org, repo, number], + ); + const codeViewOptions = useMemo( () => ({ theme: selectedDiffTheme.theme, @@ -1118,6 +1189,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" unsafeCSS: "[data-utility-button] { margin-right: 0; }", onGutterUtilityClick: openCommentTarget, onLineSelectionEnd: openCommentTarget, + loadDiffFiles, layout: { paddingTop: 0, paddingBottom: 12, gap: 12 }, }), [ @@ -1129,6 +1201,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" showLineNumbers, wordWrap, openCommentTarget, + loadDiffFiles, ], ); From 70f6f554667cc0cd5e74639bcc37c12a45f39352 Mon Sep 17 00:00:00 2001 From: Xin Date: Wed, 5 Aug 2026 16:51:13 +0100 Subject: [PATCH 4/7] refactor(web): drop CodeView PR header and pending-comments footer --- web/src/components/DiffView.tsx | 73 --------------------------------- 1 file changed, 73 deletions(-) diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 51dd062..4b683a0 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -312,22 +312,6 @@ async function loadWorktreeFileContents(path: string): Promise { return { name: path, contents }; } -function pullRequestHeaderState(info: PullRequestInfo): { label: string; className: string } { - if (info.merged) { - return { label: "Merged", className: "bg-purple-500/10 text-purple-600 dark:text-purple-400" }; - } - if (info.state.toLowerCase() === "closed") { - return { label: "Closed", className: "bg-red-500/10 text-red-600 dark:text-red-400" }; - } - if (info.draft) { - return { - label: "Draft", - className: "bg-neutral-500/10 text-neutral-600 dark:text-neutral-400", - }; - } - return { label: "Open", className: "bg-green-500/10 text-green-600 dark:text-green-400" }; -} - export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" } = {}) { const { org, repo, number } = useParams<{ org: string; @@ -1324,61 +1308,6 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" [fileSignatures, reviewed, toggleReviewed, filePatchSections], ); - const renderCodeViewHeader = useCallback(() => { - if (usesLocalStore || !currentPullRequestInfo) return null; - const info = currentPullRequestInfo; - const state = pullRequestHeaderState(info); - return ( -
-
- - {info.title} - - - {state.label} - -
-
- {info.author !== "" && {info.author}} - {info.baseRef !== "" && info.headRef !== "" && ( - - {info.baseRef} - - {info.headRef} - - )} - - {info.changedFiles} files - +{info.additions} - −{info.deletions} - -
-
- ); - }, [usesLocalStore, currentPullRequestInfo]); - - const renderCodeViewFooter = useCallback(() => { - if (pendingCommentThreads.length === 0) return null; - const count = pendingCommentThreads.length; - return ( -
- - {count} pending {count === 1 ? "comment" : "comments"} - - -
- ); - }, [pendingCommentThreads, submitPendingComments, submittingPendingComments]); - if (loading) { return (
Loading diff...
@@ -1558,8 +1487,6 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" renderAnnotation={renderAnnotation} renderHeaderPrefix={renderHeaderPrefix} renderHeaderMetadata={renderHeaderMetadata} - renderCodeViewHeader={renderCodeViewHeader} - renderCodeViewFooter={renderCodeViewFooter} /> )} From 9a4a7b8082ce15e1ef6df074b38597f2ffa48061 Mon Sep 17 00:00:00 2001 From: Xin Date: Wed, 5 Aug 2026 17:09:01 +0100 Subject: [PATCH 5/7] feat: add base branch switcher on branch view --- src/git.rs | 46 ++++++++++- src/server.rs | 14 ++++ web/src/components/DiffView.tsx | 1 + web/src/components/diff-view/DiffToolbar.tsx | 82 +++++++++++++++++++- 4 files changed, 140 insertions(+), 3 deletions(-) diff --git a/src/git.rs b/src/git.rs index ff989cb..f9e072b 100644 --- a/src/git.rs +++ b/src/git.rs @@ -3,7 +3,7 @@ use git2::{ Repository, Status, StatusOptions, }; use serde::Serialize; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::path::{Path, PathBuf}; use thiserror::Error; @@ -140,6 +140,26 @@ pub fn resolve_local_ref(cwd: impl AsRef, name: &str) -> Option { ref_exists(cwd, &candidate).then_some(candidate) } +/// Local and remote-tracking branch shorthands suitable as base refs for +/// `/branch?base=…` (e.g. `main`, `origin/main`). Skips remote HEAD aliases. +pub fn list_branches(cwd: impl AsRef) -> Result> { + let repo = discover(cwd)?; + let mut names = BTreeSet::new(); + for branch_type in [BranchType::Local, BranchType::Remote] { + for entry in repo.branches(Some(branch_type))? { + let (branch, _) = entry?; + let Some(name) = branch.name()?.filter(|name| !name.is_empty()) else { + continue; + }; + if branch_type == BranchType::Remote && name.ends_with("/HEAD") { + continue; + } + names.insert(name.to_string()); + } + } + Ok(names.into_iter().collect()) +} + pub fn remote_url(cwd: impl AsRef, remote: &str) -> Result { let repo = discover(cwd)?; Ok(repo @@ -447,6 +467,30 @@ mod tests { assert_eq!(branch(dir.path()), "trunk"); } + #[test] + fn list_branches_includes_local_and_remote_tracking() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + git(root, &["init", "-b", "main"]); + git(root, &["config", "user.email", "diffs@example.com"]); + git(root, &["config", "user.name", "Diffs Test"]); + fs::write(root.join("a.txt"), "a\n").unwrap(); + git(root, &["add", "."]); + git(root, &["commit", "-m", "initial"]); + git(root, &["branch", "feature"]); + git(root, &["remote", "add", "origin", root.to_str().unwrap()]); + git(root, &["fetch", "origin"]); + + let names = list_branches(root).unwrap(); + assert!(names.contains(&"main".to_string()), "{names:?}"); + assert!(names.contains(&"feature".to_string()), "{names:?}"); + assert!(names.contains(&"origin/main".to_string()), "{names:?}"); + assert!( + !names.iter().any(|name| name.ends_with("/HEAD")), + "{names:?}" + ); + } + #[test] fn changed_files_labels_deletions_and_renames() { let dir = tempfile::tempdir().unwrap(); diff --git a/src/server.rs b/src/server.rs index 355d4db..35cebb0 100644 --- a/src/server.rs +++ b/src/server.rs @@ -87,6 +87,12 @@ struct RepoContextResponse { branch_base: String, } +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +struct BranchesResponse { + branches: Vec, +} + #[derive(Debug, Deserialize)] struct BranchDiffQuery { base: Option, @@ -152,6 +158,7 @@ pub fn new(cfg: ServerConfig) -> anyhow::Result { .route("/api/events", get(handle_events)) .route("/api/local-diff", get(handle_local_diff)) .route("/api/branch-diff", get(handle_branch_diff)) + .route("/api/branches", get(handle_branches)) .route("/api/repo-context", get(handle_repo_context)) .route( "/api/comments", @@ -286,6 +293,13 @@ async fn handle_branch_diff( } } +async fn handle_branches(State(state): State) -> Response { + match git::list_branches(&state.cwd) { + Ok(branches) => (StatusCode::OK, Json(BranchesResponse { branches })).into_response(), + Err(err) => error(StatusCode::BAD_GATEWAY, err), + } +} + async fn handle_repo_context(State(state): State) -> impl IntoResponse { // The two lookups are independent; run them concurrently so the handler's // latency is the slower call, not the sum. diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 4b683a0..93d82ad 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -1419,6 +1419,7 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" config={config} isLocal={usesLocalStore} baseRef={isBranch ? baseRef : undefined} + includeDirty={isBranch ? includeDirty : false} onSettingsOpenChange={setSettingsOpen} onSidebarToggle={openSidebar} onSubmitPendingComments={submitPendingComments} diff --git a/web/src/components/diff-view/DiffToolbar.tsx b/web/src/components/diff-view/DiffToolbar.tsx index 13e0a57..181ccc2 100644 --- a/web/src/components/diff-view/DiffToolbar.tsx +++ b/web/src/components/diff-view/DiffToolbar.tsx @@ -1,7 +1,9 @@ -import { lazy, Suspense, type ReactNode, type SVGProps } from "react"; +import { lazy, Suspense, useState, type ReactNode, type SVGProps } from "react"; import { Link } from "react-router"; import { IconArrowLeft, + IconCheck, + IconChevronDown, IconFileDiff, IconFileExport, IconGitBranch, @@ -28,6 +30,7 @@ import { DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; +import { apiFetch } from "@/lib/api"; import type { AppConfig, DiffSettingsProps, PullRequestInfo } from "./types"; import { displayLocalPath, headerIconButtonClass } from "./helpers"; @@ -59,6 +62,79 @@ function BranchChip({ label, title }: { label: string; title: string }) { ); } +function branchDiffHref(base: string, includeDirty: boolean) { + const params = new URLSearchParams(); + params.set("base", base); + if (includeDirty) params.set("dirty", "1"); + return `/branch?${params.toString()}`; +} + +function BaseBranchSwitcher({ baseRef, includeDirty }: { baseRef: string; includeDirty: boolean }) { + const [branches, setBranches] = useState(null); + const [loading, setLoading] = useState(false); + + const loadBranches = () => { + setLoading(true); + apiFetch<{ branches: string[] }>("/api/branches") + .then((data) => { + const names = [...(data.branches ?? [])]; + if (baseRef !== "" && !names.includes(baseRef)) names.unshift(baseRef); + setBranches(names); + }) + .catch(() => setBranches(baseRef !== "" ? [baseRef] : [])) + .finally(() => setLoading(false)); + }; + + return ( + { + if (open) loadBranches(); + }} + > + + {baseRef} + + + } + /> + + {loading && branches == null ? ( +
Loading branches…
+ ) : (branches?.length ?? 0) === 0 ? ( +
No branches found
+ ) : ( + branches?.map((name) => { + const selected = name === baseRef; + return ( + } + className={selected ? "font-medium" : undefined} + aria-current={selected ? "true" : undefined} + > + + {selected ? : null} + + {name} + + ); + }) + )} + + + ); +} + function PrStat({ value, label, @@ -166,6 +242,7 @@ export function DiffToolbar({ config, isLocal, baseRef, + includeDirty = false, onSettingsOpenChange, onSidebarToggle, onSubmitPendingComments, @@ -190,6 +267,7 @@ export function DiffToolbar({ config: AppConfig; isLocal: boolean; baseRef?: string; + includeDirty?: boolean; onSettingsOpenChange: (open: boolean) => void; onSidebarToggle: () => void; onSubmitPendingComments: () => void; @@ -238,7 +316,7 @@ export function DiffToolbar({ {baseRef && baseRef.trim() !== "" && ( <> - + Date: Wed, 5 Aug 2026 22:04:02 +0100 Subject: [PATCH 6/7] chore: build web assets before cargo run in pnpm dev --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 93367f0..0a90c4e 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "packageManager": "pnpm@10.33.2", "scripts": { "build": "pnpm --dir web build && cargo build --release && scripts/copy-release-binary.sh", - "dev": "cargo run --", + "dev": "pnpm --dir web build && cargo run --", "format": "pnpm format:rust && pnpm format:web", "format:rust": "cargo fmt", "format:web": "pnpm --dir web format", From 32e7d38072c7778ef625f2f250ceac9c472cb407 Mon Sep 17 00:00:00 2001 From: Xin Date: Wed, 5 Aug 2026 22:10:34 +0100 Subject: [PATCH 7/7] fix(web): handle rename-pure and zero-oid old side in loadDiffFiles Pure renames carry no index line, so the local-store hydration path threw 'missing prevObjectId' whenever a rename-pure file was expanded. Read the unchanged content from the worktree instead, and reject an all-zeros prevObjectId up front rather than 404ing on the blob fetch. --- web/src/components/DiffView.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/web/src/components/DiffView.tsx b/web/src/components/DiffView.tsx index 93d82ad..f12fb07 100644 --- a/web/src/components/DiffView.tsx +++ b/web/src/components/DiffView.tsx @@ -1138,11 +1138,19 @@ export function DiffView({ source = "pr" }: { source?: "pr" | "local" | "branch" return { oldFile, newFile }; } - const prevObjectId = fileDiff.prevObjectId; - if (!prevObjectId) throw new Error(`missing prevObjectId for ${fileDiff.name}`); const oldName = fileDiff.prevName ?? fileDiff.name; const newName = fileDiff.name; + const prevObjectId = fileDiff.prevObjectId; const newObjectId = fileDiff.newObjectId; + // Pure renames carry no `index` line (and thus no object ids); their + // content is unchanged, so read the new path from the worktree. + if (fileDiff.type === "rename-pure") { + const newFile = await loadWorktreeFileContents(newName); + return { oldFile: null, newFile }; + } + if (!prevObjectId || isZeroOid(prevObjectId)) { + throw new Error(`missing prevObjectId for ${fileDiff.name}`); + } const [oldFile, newFile] = await Promise.all([ loadBlobFileContents(prevObjectId, oldName), newObjectId && !isZeroOid(newObjectId)