From 1eee4ad5d5dcc805287e7a6e0ebae4bac3b2c2d6 Mon Sep 17 00:00:00 2001 From: Kevin Yin <182213728+yinkev@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:00:00 -0700 Subject: [PATCH] feat(desktop): open repository files from message links --- desktop/src-tauri/src/commands/mod.rs | 1 + desktop/src-tauri/src/commands/project_git.rs | 45 ++-- .../src/commands/project_git_exec.rs | 60 ++++- .../src/commands/project_git_focus.rs | 211 ++++++++++++++++++ .../src/app/navigation/useAppNavigation.ts | 14 +- .../src/app/routes/projects.$projectId.tsx | 25 +-- desktop/src/features/projects/hooks.ts | 34 ++- .../projects/lib/projectDetailRouteState.ts | 24 ++ .../projects/lib/projectDetailSearch.test.mjs | 59 +++++ .../projects/lib/projectDetailSearch.ts | 44 ++++ .../lib/projectRepoSnapshotTarget.test.mjs | 85 +++++++ .../projects/lib/projectRepoSnapshotTarget.ts | 56 +++++ .../lib/repositoryDeepLinkTarget.test.mjs | 113 ++++++++++ .../projects/lib/repositoryDeepLinkTarget.ts | 67 ++++++ .../projects/ui/ProjectDetailScreen.tsx | 89 ++++---- .../projects/ui/ProjectRepositoryPanel.tsx | 130 +++++++++-- .../projects/ui/ProjectWorkspaceTabs.tsx | 18 +- .../projects/ui/RepositoryTargetRef.tsx | 16 ++ desktop/src/shared/api/projectGit.ts | 2 + desktop/src/shared/lib/entityLink.test.mjs | 109 +++++++++ desktop/src/shared/lib/entityLink.ts | 69 +++++- .../src/shared/lib/repositoryTarget.test.mjs | 66 ++++++ desktop/src/shared/lib/repositoryTarget.ts | 67 ++++++ desktop/src/shared/ui/markdown.test.mjs | 41 ++++ .../src/shared/ui/markdown/entityLinks.tsx | 3 + 25 files changed, 1330 insertions(+), 118 deletions(-) create mode 100644 desktop/src-tauri/src/commands/project_git_focus.rs create mode 100644 desktop/src/features/projects/lib/projectDetailRouteState.ts create mode 100644 desktop/src/features/projects/lib/projectDetailSearch.test.mjs create mode 100644 desktop/src/features/projects/lib/projectDetailSearch.ts create mode 100644 desktop/src/features/projects/lib/projectRepoSnapshotTarget.test.mjs create mode 100644 desktop/src/features/projects/lib/projectRepoSnapshotTarget.ts create mode 100644 desktop/src/features/projects/lib/repositoryDeepLinkTarget.test.mjs create mode 100644 desktop/src/features/projects/lib/repositoryDeepLinkTarget.ts create mode 100644 desktop/src/features/projects/ui/RepositoryTargetRef.tsx create mode 100644 desktop/src/shared/lib/repositoryTarget.test.mjs create mode 100644 desktop/src/shared/lib/repositoryTarget.ts diff --git a/desktop/src-tauri/src/commands/mod.rs b/desktop/src-tauri/src/commands/mod.rs index 237bc06e8d..c79dd09837 100644 --- a/desktop/src-tauri/src/commands/mod.rs +++ b/desktop/src-tauri/src/commands/mod.rs @@ -46,6 +46,7 @@ mod project_git; mod project_git_branches; mod project_git_diff; mod project_git_exec; +mod project_git_focus; mod project_git_merge_error; mod project_git_push; mod project_git_workflow; diff --git a/desktop/src-tauri/src/commands/project_git.rs b/desktop/src-tauri/src/commands/project_git.rs index 201f3a0507..3a473e9a4b 100644 --- a/desktop/src-tauri/src/commands/project_git.rs +++ b/desktop/src-tauri/src/commands/project_git.rs @@ -1,6 +1,8 @@ use super::project_git_exec::{ - build_git_auth_config, clean_branch, clean_target_ref, run_git, validate_workspace_clone_url, - GitAuthConfig, + build_git_auth_config, clean_branch, run_git, validate_workspace_clone_url, GitAuthConfig, +}; +use super::project_git_focus::{ + clean_repository_target, select_repository_tree_lines, RepositoryTarget, }; use super::project_git_push::push_project_local_repository_blocking; use super::project_repo_paths::{canonical_repos_roots, find_local_repo_dir}; @@ -313,9 +315,10 @@ fn parse_ls_tree( repo_dir: &std::path::Path, output: &str, latest_commit_by_path: &std::collections::HashMap, + focus_path: Option<&str>, ) -> Vec { - output - .lines() + select_repository_tree_lines(output, focus_path, 250, 250) + .into_iter() .filter_map(|line| { let (meta, path) = line.split_once('\t')?; let mut parts = meta.split_whitespace(); @@ -339,7 +342,6 @@ fn parse_ls_tree( latest_commit: latest_commit_by_path.get(path).cloned(), }) }) - .take(250) .collect() } @@ -348,6 +350,7 @@ fn snapshot_from_repo( auth: &GitAuthConfig, branch_name: Option<&str>, base_branch: Option<&str>, + focus_path: Option<&str>, ) -> ProjectRepoSnapshotInfo { let latest_commit = run_git( &["log", "-1", "--format=%H%x00%h%x00%an%x00%ae%x00%at%x00%s"], @@ -397,10 +400,13 @@ fn snapshot_from_repo( ) .map(|output| parse_latest_commit_by_path(&output)) .unwrap_or_default(); - - run_git(&["ls-tree", "-r", "--long", "HEAD"], Some(repo_dir), auth) - .map(|output| parse_ls_tree(repo_dir, &output, &latest_commit_by_path)) - .unwrap_or_default() + run_git( + &["ls-tree", "-r", "--long", "-z", "HEAD"], + Some(repo_dir), + auth, + ) + .map(|output| parse_ls_tree(repo_dir, &output, &latest_commit_by_path, focus_path)) + .unwrap_or_default() } else { Vec::new() }; @@ -712,18 +718,18 @@ pub async fn get_project_repo_snapshot( base_branch: Option, target_ref: Option, target_commit: Option, + target_path: Option, state: State<'_, AppState>, ) -> Result { validate_workspace_clone_url(&clone_url, &state)?; let auth = build_git_auth_config(&state)?; let branch = clean_branch(default_branch); let base_branch = clean_branch(base_branch); - let target_ref = clean_target_ref(target_ref); - let target_commit = target_commit - .map(|value| value.to_ascii_lowercase()) - .filter(|value| matches!(value.len(), 40 | 64)) - .filter(|value| value.chars().all(|c| c.is_ascii_hexdigit())); - + let RepositoryTarget { + target_ref, + target_commit, + target_path, + } = clean_repository_target(target_ref, target_commit, target_path)?; tauri::async_runtime::spawn_blocking(move || { let temp_dir = tempfile::tempdir().map_err(|error| format!("create temp dir: {error}"))?; let repo_dir = temp_dir.path().join("repo"); @@ -783,8 +789,13 @@ pub async fn get_project_repo_snapshot( } } - let snapshot = - snapshot_from_repo(&repo_dir, &auth, branch.as_deref(), base_branch.as_deref()); + let snapshot = snapshot_from_repo( + &repo_dir, + &auth, + branch.as_deref(), + base_branch.as_deref(), + target_path.as_deref(), + ); Ok(snapshot) }) .await diff --git a/desktop/src-tauri/src/commands/project_git_exec.rs b/desktop/src-tauri/src/commands/project_git_exec.rs index c616d39db1..2c8b6f6884 100644 --- a/desktop/src-tauri/src/commands/project_git_exec.rs +++ b/desktop/src-tauri/src/commands/project_git_exec.rs @@ -267,15 +267,31 @@ pub(crate) fn clean_branch(value: Option) -> Option { pub(crate) fn clean_target_ref(value: Option) -> Option { let value = value?.trim().to_string(); - for prefix in ["refs/tags/", "refs/nostr/"] { + for prefix in ["refs/heads/", "refs/tags/", "refs/nostr/"] { if let Some(name) = value.strip_prefix(prefix) { let clean_name = clean_branch(Some(name.to_string()))?; + if prefix == "refs/heads/" + && (clean_name.ends_with('.') + || clean_name.ends_with(".lock") + || clean_name.contains("//") + || clean_name + .split('/') + .any(|component| component.starts_with('.'))) + { + return None; + } return (clean_name == name).then_some(format!("{prefix}{clean_name}")); } } None } +pub(crate) fn clean_target_commit(value: Option) -> Option { + let value = value?.trim().to_ascii_lowercase(); + (matches!(value.len(), 40 | 64) && value.chars().all(|c| c.is_ascii_hexdigit())) + .then_some(value) +} + pub(crate) fn validate_clone_url(clone_url: &str) -> Result<(), String> { let parsed = Url::parse(clone_url).map_err(|error| format!("invalid clone URL: {error}"))?; if !matches!(parsed.scheme(), "http" | "https") { @@ -393,9 +409,9 @@ fn validate_clone_url_against_relay(clone_url: &str, relay_base: &str) -> Result #[cfg(test)] mod tests { use super::{ - clean_branch, clean_target_ref, credential_helper_config_value, git_needs_credentials, - git_subcommand, validate_clone_url, validate_clone_url_against_relay, - validate_local_clone_url, + clean_branch, clean_target_commit, clean_target_ref, credential_helper_config_value, + git_needs_credentials, git_subcommand, validate_clone_url, + validate_clone_url_against_relay, validate_local_clone_url, }; #[test] @@ -463,7 +479,11 @@ mod tests { } #[test] - fn clean_target_ref_accepts_only_tags_and_pull_request_refs() { + fn clean_target_ref_accepts_heads_tags_and_pull_request_refs() { + assert_eq!( + clean_target_ref(Some("refs/heads/feature/x-1".into())), + Some("refs/heads/feature/x-1".to_string()) + ); assert_eq!( clean_target_ref(Some("refs/tags/v1.0.0".into())), Some("refs/tags/v1.0.0".to_string()) @@ -472,10 +492,38 @@ mod tests { clean_target_ref(Some("refs/nostr/abc123".into())), Some("refs/nostr/abc123".to_string()) ); - assert_eq!(clean_target_ref(Some("refs/heads/main".into())), None); + assert_eq!(clean_target_ref(Some("refs/heads/../main".into())), None); assert_eq!(clean_target_ref(Some("refs/tags/../main".into())), None); } + #[test] + fn clean_target_ref_rejects_noncanonical_head_names() { + for value in [ + "refs/heads/main.", + "refs/heads/main.lock", + "refs/heads/feature//main", + "refs/heads/.hidden/main", + "refs/heads/feature/.hidden", + ] { + assert_eq!(clean_target_ref(Some(value.into())), None, "{value}"); + } + } + + #[test] + fn clean_target_commit_accepts_full_hashes_and_rejects_other_values() { + assert_eq!( + clean_target_commit(Some(" AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA ".into())), + Some("a".repeat(40)) + ); + assert_eq!( + clean_target_commit(Some("B".repeat(64))), + Some("b".repeat(64)) + ); + for value in ["", "abc", &"g".repeat(40), &"a".repeat(39)] { + assert_eq!(clean_target_commit(Some(value.into())), None, "{value}"); + } + } + #[test] fn validate_clone_url_requires_buzz_repo_shape() { let owner = "a".repeat(64); diff --git a/desktop/src-tauri/src/commands/project_git_focus.rs b/desktop/src-tauri/src/commands/project_git_focus.rs new file mode 100644 index 0000000000..137991e2ce --- /dev/null +++ b/desktop/src-tauri/src/commands/project_git_focus.rs @@ -0,0 +1,211 @@ +use super::project_git_exec::{clean_target_commit, clean_target_ref}; + +pub(crate) struct RepositoryTarget { + pub(crate) target_ref: Option, + pub(crate) target_commit: Option, + pub(crate) target_path: Option, +} + +pub(crate) fn clean_repository_target( + target_ref: Option, + target_commit: Option, + target_path: Option, +) -> Result { + Ok(RepositoryTarget { + target_ref: target_ref + .map(|value| { + clean_target_ref(Some(value)) + .ok_or_else(|| "Invalid repository target ref.".to_string()) + }) + .transpose()?, + target_commit: target_commit + .map(|value| { + clean_target_commit(Some(value)) + .ok_or_else(|| "Invalid repository target commit.".to_string()) + }) + .transpose()?, + target_path: target_path + .map(|value| { + clean_repository_focus_path(Some(value)) + .ok_or_else(|| "Invalid repository target path.".to_string()) + }) + .transpose()?, + }) +} + +const MAX_FOCUS_PATH_BYTES: usize = 4096; + +pub(crate) fn clean_repository_focus_path(value: Option) -> Option { + let value = value?; + if value.is_empty() + || value.len() > MAX_FOCUS_PATH_BYTES + || value.starts_with(['/', '\\']) + || value.ends_with('/') + || value.contains('\\') + || value.chars().any(char::is_control) + || value + .split('/') + .any(|component| component.is_empty() || matches!(component, "." | "..")) + { + return None; + } + Some(value) +} + +fn tree_line_path(line: &str) -> Option<&str> { + line.split_once('\t').map(|(_, path)| path) +} + +pub(crate) fn select_repository_tree_lines<'a>( + output: &'a str, + focus_path: Option<&str>, + base_limit: usize, + focus_limit: usize, +) -> Vec<&'a str> { + let records: Vec<_> = output + .split_terminator('\0') + .filter(|record| !record.is_empty()) + .collect(); + let mut selected: Vec<_> = records.iter().take(base_limit).copied().collect(); + let Some(focus_path) = focus_path else { + return selected; + }; + let directory_prefix = format!("{focus_path}/"); + selected.extend( + records + .iter() + .skip(base_limit) + .filter(|line| { + tree_line_path(line) + .is_some_and(|path| path == focus_path || path.starts_with(&directory_prefix)) + }) + .take(focus_limit) + .copied(), + ); + selected +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn repository_target_normalizes_all_explicit_coordinates() { + let target = clean_repository_target( + Some("refs/heads/feature/repo-links".into()), + Some("A".repeat(40)), + Some("GUIDES/setup.md".into()), + ) + .expect("valid target"); + assert_eq!( + target.target_ref.as_deref(), + Some("refs/heads/feature/repo-links") + ); + assert_eq!( + target.target_commit.as_deref(), + Some("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + ); + assert_eq!(target.target_path.as_deref(), Some("GUIDES/setup.md")); + } + + #[test] + fn repository_target_rejects_any_invalid_explicit_coordinate() { + assert!(clean_repository_target(Some("refs/heads/main.lock".into()), None, None).is_err()); + assert!(clean_repository_target(None, Some("short".into()), None).is_err()); + assert!(clean_repository_target(None, None, Some("../README.md".into())).is_err()); + } + + #[test] + fn focus_path_accepts_safe_repository_coordinates() { + assert_eq!( + clean_repository_focus_path(Some("GUIDES/setup/install.md".into())).as_deref(), + Some("GUIDES/setup/install.md") + ); + } + + #[test] + fn focus_path_rejects_absolute_traversal_ambiguous_and_control_paths() { + for value in [ + "", + "/etc/passwd", + "../README.md", + "docs/../README.md", + "docs//README.md", + "docs/./README.md", + "docs\\README.md", + "docs/README.md/", + "docs/\u{0}README.md", + ] { + assert_eq!( + clean_repository_focus_path(Some(value.into())), + None, + "{value:?}" + ); + } + } + + #[test] + fn focused_file_outside_base_cap_is_appended_once() { + let output = [ + "100644 blob a 1\tA.txt", + "100644 blob b 1\tB.txt", + "100644 blob c 1\tGUIDES/RUNBOOK.md", + ] + .join("\0"); + assert_eq!( + select_repository_tree_lines(&output, Some("GUIDES/RUNBOOK.md"), 2, 20), + vec![ + "100644 blob a 1\tA.txt", + "100644 blob b 1\tB.txt", + "100644 blob c 1\tGUIDES/RUNBOOK.md", + ] + ); + } + + #[test] + fn focused_directory_adds_descendants_without_partial_prefix_matches() { + let output = [ + "100644 blob a 1\tA.txt", + "100644 blob b 1\tGUIDES.md", + "100644 blob c 1\tGUIDES/setup/install.md", + "100644 blob d 1\tGUIDES/RUNBOOK.md", + ] + .join("\0"); + assert_eq!( + select_repository_tree_lines(&output, Some("GUIDES"), 1, 20), + vec![ + "100644 blob a 1\tA.txt", + "100644 blob c 1\tGUIDES/setup/install.md", + "100644 blob d 1\tGUIDES/RUNBOOK.md", + ] + ); + } + + #[test] + fn nul_delimited_non_ascii_path_matches_without_git_c_quoting() { + let output = ["100644 blob a 1\tA.txt", "100644 blob b 1\tcafé.txt"].join("\0"); + assert_eq!( + select_repository_tree_lines(&output, Some("café.txt"), 1, 20), + vec!["100644 blob a 1\tA.txt", "100644 blob b 1\tcafé.txt",] + ); + } + + #[test] + fn focused_entries_respect_their_own_cap() { + let output = [ + "100644 blob a 1\tA.txt", + "100644 blob b 1\tdocs/1.md", + "100644 blob c 1\tdocs/2.md", + "100644 blob d 1\tdocs/3.md", + ] + .join("\0"); + assert_eq!( + select_repository_tree_lines(&output, Some("docs"), 1, 2), + vec![ + "100644 blob a 1\tA.txt", + "100644 blob b 1\tdocs/1.md", + "100644 blob c 1\tdocs/2.md", + ] + ); + } +} diff --git a/desktop/src/app/navigation/useAppNavigation.ts b/desktop/src/app/navigation/useAppNavigation.ts index 4c7382a306..8939df6b83 100644 --- a/desktop/src/app/navigation/useAppNavigation.ts +++ b/desktop/src/app/navigation/useAppNavigation.ts @@ -110,9 +110,15 @@ export function useAppNavigation() { pullRequestId?: string; issueId?: string; repositoryId?: string; + repoRef?: string; + repoPath?: string; }, - ) => - commitNavigation( + ) => { + const repositoryTarget = + behavior?.repoRef && behavior.repoPath + ? { repoRef: behavior.repoRef, repoPath: behavior.repoPath } + : {}; + return commitNavigation( { to: "/projects/$projectId", params: { @@ -129,10 +135,12 @@ export function useAppNavigation() { ...(behavior?.repositoryId ? { repositoryId: behavior.repositoryId } : {}), + ...repositoryTarget, }, }, behavior, - ), + ); + }, [commitNavigation], ); diff --git a/desktop/src/app/routes/projects.$projectId.tsx b/desktop/src/app/routes/projects.$projectId.tsx index 4954428748..466a398583 100644 --- a/desktop/src/app/routes/projects.$projectId.tsx +++ b/desktop/src/app/routes/projects.$projectId.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import { createFileRoute } from "@tanstack/react-router"; +import { validateProjectDetailSearch } from "@/features/projects/lib/projectDetailSearch"; import { usePreviewFeatureWarning } from "@/shared/features"; import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback"; @@ -11,24 +12,20 @@ const ProjectDetailScreen = React.lazy(async () => { export const Route = createFileRoute("/projects/$projectId")({ component: ProjectDetailRouteComponent, - validateSearch: (search: Record) => ({ - commitHash: - typeof search.commitHash === "string" ? search.commitHash : undefined, - pullRequestId: - typeof search.pullRequestId === "string" - ? search.pullRequestId - : undefined, - issueId: typeof search.issueId === "string" ? search.issueId : undefined, - repositoryId: - typeof search.repositoryId === "string" ? search.repositoryId : undefined, - }), + validateSearch: validateProjectDetailSearch, }); function ProjectDetailRouteComponent() { usePreviewFeatureWarning("projects"); const { projectId } = Route.useParams(); - const { commitHash, pullRequestId, issueId, repositoryId } = - Route.useSearch(); + const { + commitHash, + pullRequestId, + issueId, + repositoryId, + repoRef, + repoPath, + } = Route.useSearch(); return ( }> @@ -37,6 +34,8 @@ function ProjectDetailRouteComponent() { issueId={issueId} projectId={projectId} pullRequestId={pullRequestId} + repoPath={repoPath} + repoRef={repoRef} repositoryId={repositoryId} /> diff --git a/desktop/src/features/projects/hooks.ts b/desktop/src/features/projects/hooks.ts index f6e5d2f1b0..b7494eb42d 100644 --- a/desktop/src/features/projects/hooks.ts +++ b/desktop/src/features/projects/hooks.ts @@ -38,6 +38,10 @@ import type { RelayEvent, } from "@/shared/api/types"; import { summarizeProjectActivityEvents } from "./projectActivity.mjs"; +import { + projectRepoSnapshotCloneUrl, + projectRepoSnapshotTarget, +} from "./lib/projectRepoSnapshotTarget"; import type { ProjectIssue } from "./projectIssues.mjs"; import { nextProjectIssueCommentCreatedAt, @@ -428,23 +432,27 @@ async function createProjectIssueComment({ async function fetchProjectRepoSnapshot( project: Repository, - branchName?: string | null, + selectedBranch?: string | null, pullRequest?: ProjectPullRequest | null, tag?: { name: string; commit: string } | null, + repositoryTarget?: { ref: string; path: string } | null, ): Promise { - const cloneUrl = pullRequest?.cloneUrls[0] ?? project.cloneUrls[0]; + const cloneUrl = projectRepoSnapshotCloneUrl({ + projectCloneUrls: project.cloneUrls, + pullRequestCloneUrls: pullRequest?.cloneUrls, + repositoryTarget, + }); if (!cloneUrl) return null; - return getProjectRepoSnapshot({ cloneUrl, - defaultBranch: branchName ?? project.defaultBranch, - baseBranch: project.defaultBranch, - targetCommit: tag?.commit ?? pullRequest?.commit ?? null, - targetRef: tag - ? `refs/tags/${tag.name}` - : pullRequest - ? `refs/nostr/${pullRequest.id}` - : null, + targetPath: repositoryTarget?.path ?? null, + ...projectRepoSnapshotTarget({ + selectedBranch, + projectDefaultBranch: project.defaultBranch, + pullRequest, + tag, + repositoryRef: repositoryTarget?.ref, + }), }); } @@ -658,6 +666,7 @@ export function useProjectRepoSnapshotQuery( pullRequest?: ProjectPullRequest | null, tag?: { name: string; commit: string } | null, enabled = true, + repositoryTarget?: { ref: string; path: string } | null, ) { const selectedBranch = branchName ?? project?.defaultBranch ?? null; @@ -672,6 +681,8 @@ export function useProjectRepoSnapshotQuery( pullRequest?.commit ?? "none", tag?.name ?? "no-tag", tag?.commit ?? "no-tag-commit", + repositoryTarget?.ref ?? "no-repository-ref", + repositoryTarget?.path ?? "no-repository-path", ], queryFn: () => { if (!project) throw new Error("No project selected."); @@ -680,6 +691,7 @@ export function useProjectRepoSnapshotQuery( selectedBranch, pullRequest, tag, + repositoryTarget, ); }, staleTime: 30_000, diff --git a/desktop/src/features/projects/lib/projectDetailRouteState.ts b/desktop/src/features/projects/lib/projectDetailRouteState.ts new file mode 100644 index 0000000000..a406b19bf9 --- /dev/null +++ b/desktop/src/features/projects/lib/projectDetailRouteState.ts @@ -0,0 +1,24 @@ +export type ProjectDetailScreenProps = { + commitHash?: string; + projectId: string; + pullRequestId?: string; + issueId?: string; + repoPath?: string; + repoRef?: string; + repositoryId?: string; +}; + +export const PROJECT_DETAIL_PANEL_SEARCH_KEYS = [ + "profile", + "profileTab", + "profileView", +] as const; + +export const PROJECT_REPOSITORY_SEARCH_KEYS = [ + "repositoryId", + "repoRef", + "repoPath", + "issueId", + "pullRequestId", + "commitHash", +] as const; diff --git a/desktop/src/features/projects/lib/projectDetailSearch.test.mjs b/desktop/src/features/projects/lib/projectDetailSearch.test.mjs new file mode 100644 index 0000000000..ea27d66434 --- /dev/null +++ b/desktop/src/features/projects/lib/projectDetailSearch.test.mjs @@ -0,0 +1,59 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { validateProjectDetailSearch } from "./projectDetailSearch.ts"; + +const COMMIT = "a".repeat(40); + +test("validates existing search and repository target atomically", () => { + assert.deepEqual( + validateProjectDetailSearch({ + commitHash: "commit", + pullRequestId: "pr", + issueId: "issue", + repositoryId: "repository", + repoRef: COMMIT.toUpperCase(), + repoPath: "src/main.ts", + }), + { + commitHash: "commit", + pullRequestId: "pr", + issueId: "issue", + repositoryId: "repository", + repoRef: COMMIT, + repoPath: "src/main.ts", + }, + ); +}); + +test("drops incomplete or unsafe repository target search without dropping repository selection", () => { + assert.deepEqual( + validateProjectDetailSearch({ + repositoryId: "repository", + repoRef: "main", + }), + { + commitHash: undefined, + pullRequestId: undefined, + issueId: undefined, + repositoryId: "repository", + repoRef: undefined, + repoPath: undefined, + }, + ); + assert.deepEqual( + validateProjectDetailSearch({ + repositoryId: "repository", + repoRef: "main", + repoPath: "../secret", + }), + { + commitHash: undefined, + pullRequestId: undefined, + issueId: undefined, + repositoryId: "repository", + repoRef: undefined, + repoPath: undefined, + }, + ); +}); diff --git a/desktop/src/features/projects/lib/projectDetailSearch.ts b/desktop/src/features/projects/lib/projectDetailSearch.ts new file mode 100644 index 0000000000..e7a45bbafb --- /dev/null +++ b/desktop/src/features/projects/lib/projectDetailSearch.ts @@ -0,0 +1,44 @@ +import { + normalizeRepositoryPath, + parseRepositoryRef, +} from "../../../shared/lib/repositoryTarget.ts"; + +export type ProjectDetailSearch = { + commitHash: string | undefined; + pullRequestId: string | undefined; + issueId: string | undefined; + repositoryId: string | undefined; + repoRef: string | undefined; + repoPath: string | undefined; +}; + +export function validateProjectDetailSearch( + search: Record, +): ProjectDetailSearch { + const commitHash = + typeof search.commitHash === "string" ? search.commitHash : undefined; + const pullRequestId = + typeof search.pullRequestId === "string" ? search.pullRequestId : undefined; + const issueId = + typeof search.issueId === "string" ? search.issueId : undefined; + const repositoryId = + typeof search.repositoryId === "string" ? search.repositoryId : undefined; + const parsedRef = + typeof search.repoRef === "string" + ? parseRepositoryRef(search.repoRef) + : null; + const repoPath = + typeof search.repoPath === "string" + ? normalizeRepositoryPath(search.repoPath) + : null; + const hasRepositoryTarget = Boolean(parsedRef && repoPath); + + return { + commitHash, + pullRequestId, + issueId, + repositoryId, + repoRef: hasRepositoryTarget ? parsedRef?.value : undefined, + repoPath: hasRepositoryTarget ? (repoPath ?? undefined) : undefined, + }; +} diff --git a/desktop/src/features/projects/lib/projectRepoSnapshotTarget.test.mjs b/desktop/src/features/projects/lib/projectRepoSnapshotTarget.test.mjs new file mode 100644 index 0000000000..ece86de77e --- /dev/null +++ b/desktop/src/features/projects/lib/projectRepoSnapshotTarget.test.mjs @@ -0,0 +1,85 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + projectRepoSnapshotCloneUrl, + projectRepoSnapshotTarget, +} from "./projectRepoSnapshotTarget.ts"; + +const COMMIT = "a".repeat(40); + +test("explicit branch target outranks selected branch, tag, and pull request", () => { + assert.deepEqual( + projectRepoSnapshotTarget({ + selectedBranch: "main", + projectDefaultBranch: "main", + pullRequest: { id: "pr-1", commit: "b".repeat(40) }, + tag: { name: "v1", commit: "c".repeat(40) }, + repositoryRef: "feature/repo-links", + }), + { + defaultBranch: "feature/repo-links", + baseBranch: "main", + targetRef: "refs/heads/feature/repo-links", + targetCommit: null, + }, + ); +}); + +test("explicit commit target outranks branch, tag, and pull request", () => { + assert.deepEqual( + projectRepoSnapshotTarget({ + selectedBranch: "main", + projectDefaultBranch: "main", + pullRequest: { id: "pr-1", commit: "b".repeat(40) }, + tag: { name: "v1", commit: "c".repeat(40) }, + repositoryRef: COMMIT, + }), + { + defaultBranch: "main", + baseBranch: "main", + targetRef: null, + targetCommit: COMMIT, + }, + ); +}); + +test("existing tag and pull-request precedence is preserved without a repository link", () => { + assert.deepEqual( + projectRepoSnapshotTarget({ + selectedBranch: "release", + projectDefaultBranch: "main", + pullRequest: { id: "pr-1", commit: "b".repeat(40) }, + tag: { name: "v1", commit: "c".repeat(40) }, + repositoryRef: null, + }), + { + defaultBranch: "release", + baseBranch: "main", + targetRef: "refs/tags/v1", + targetCommit: "c".repeat(40), + }, + ); +}); + +test("explicit repository targets use the canonical project clone URL", () => { + assert.equal( + projectRepoSnapshotCloneUrl({ + projectCloneUrls: ["https://relay.example/git/owner/project"], + pullRequestCloneUrls: ["https://fork.example/git/owner/fork"], + repositoryTarget: { ref: "main", path: "README.md" }, + }), + "https://relay.example/git/owner/project", + ); +}); + +test("ordinary pull request snapshots retain pull request clone precedence", () => { + assert.equal( + projectRepoSnapshotCloneUrl({ + projectCloneUrls: ["https://relay.example/git/owner/project"], + pullRequestCloneUrls: ["https://fork.example/git/owner/fork"], + repositoryTarget: null, + }), + "https://fork.example/git/owner/fork", + ); +}); diff --git a/desktop/src/features/projects/lib/projectRepoSnapshotTarget.ts b/desktop/src/features/projects/lib/projectRepoSnapshotTarget.ts new file mode 100644 index 0000000000..02b51ca697 --- /dev/null +++ b/desktop/src/features/projects/lib/projectRepoSnapshotTarget.ts @@ -0,0 +1,56 @@ +import { parseRepositoryRef } from "../../../shared/lib/repositoryTarget.ts"; + +type PullRequestTarget = { + cloneUrls: string[]; + id: string; + commit: string | null; +}; +type TagTarget = { name: string; commit: string }; + +export function projectRepoSnapshotCloneUrl(input: { + projectCloneUrls: readonly string[]; + pullRequestCloneUrls?: readonly string[]; + repositoryTarget?: { ref: string; path: string } | null; +}): string | undefined { + return input.repositoryTarget + ? input.projectCloneUrls[0] + : (input.pullRequestCloneUrls?.[0] ?? input.projectCloneUrls[0]); +} + +export function projectRepoSnapshotTarget(input: { + selectedBranch: string | null | undefined; + projectDefaultBranch: string; + pullRequest?: PullRequestTarget | null; + tag?: TagTarget | null; + repositoryRef?: string | null; +}) { + const explicitRef = input.repositoryRef + ? parseRepositoryRef(input.repositoryRef) + : null; + if (explicitRef?.kind === "branch") { + return { + defaultBranch: explicitRef.value, + baseBranch: input.projectDefaultBranch, + targetRef: `refs/heads/${explicitRef.value}`, + targetCommit: null, + }; + } + if (explicitRef?.kind === "commit") { + return { + defaultBranch: input.selectedBranch ?? input.projectDefaultBranch, + baseBranch: input.projectDefaultBranch, + targetRef: null, + targetCommit: explicitRef.value, + }; + } + return { + defaultBranch: input.selectedBranch ?? input.projectDefaultBranch, + baseBranch: input.projectDefaultBranch, + targetRef: input.tag + ? `refs/tags/${input.tag.name}` + : input.pullRequest + ? `refs/nostr/${input.pullRequest.id}` + : null, + targetCommit: input.tag?.commit ?? input.pullRequest?.commit ?? null, + }; +} diff --git a/desktop/src/features/projects/lib/repositoryDeepLinkTarget.test.mjs b/desktop/src/features/projects/lib/repositoryDeepLinkTarget.test.mjs new file mode 100644 index 0000000000..df5674cff7 --- /dev/null +++ b/desktop/src/features/projects/lib/repositoryDeepLinkTarget.test.mjs @@ -0,0 +1,113 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import * as repositoryTarget from "./repositoryDeepLinkTarget.ts"; +import { + effectiveProjectRepoSource, + projectTabForRepositoryTarget, + repositoryTargetResultKey, + resolveRepositoryDeepLinkTarget, + shouldResolveRepositoryTarget, +} from "./repositoryDeepLinkTarget.ts"; + +const files = [ + { path: "README.md", kind: "blob" }, + { path: "GUIDES/RUNBOOK.md", kind: "blob" }, + { path: "GUIDES/setup/install.md", kind: "blob" }, +]; + +test("resolves an exact repository file", () => { + assert.deepEqual( + resolveRepositoryDeepLinkTarget(files, "GUIDES/RUNBOOK.md"), + { + kind: "file", + file: files[1], + parentPath: "GUIDES", + }, + ); +}); + +test("resolves an existing directory from file prefixes", () => { + assert.deepEqual(resolveRepositoryDeepLinkTarget(files, "GUIDES/setup"), { + kind: "directory", + path: "GUIDES/setup", + }); +}); + +test("does not treat a partial filename prefix as a directory", () => { + assert.deepEqual(resolveRepositoryDeepLinkTarget(files, "GUIDES/RUN"), { + kind: "missing", + }); +}); + +test("repository targets force remote snapshots while ordinary routes preserve source", () => { + assert.equal(effectiveProjectRepoSource("local", "README.md"), "remote"); + assert.equal(effectiveProjectRepoSource("remote", "README.md"), "remote"); + assert.equal(effectiveProjectRepoSource("local", undefined), "local"); + assert.equal(effectiveProjectRepoSource("remote", undefined), "remote"); +}); + +test("repository targets open the files tab while ordinary routes open overview", () => { + assert.equal(projectTabForRepositoryTarget("README.md"), "files"); + assert.equal(projectTabForRepositoryTarget(undefined), "overview"); +}); + +test("failed targets retry when the ref becomes available", () => { + const attempt = { key: "target\0tree", outcome: "error" }; + assert.equal( + shouldResolveRepositoryTarget({ + attempt, + hasError: true, + isLoading: false, + resolutionKey: "target\0tree", + }), + false, + ); + assert.equal( + shouldResolveRepositoryTarget({ + attempt, + hasError: false, + isLoading: false, + resolutionKey: "target\0tree", + }), + true, + ); +}); + +test("resolved targets retry only for a changed repository tree", () => { + const attempt = { key: "target\0tree-a", outcome: "resolved" }; + assert.equal( + shouldResolveRepositoryTarget({ + attempt, + hasError: false, + isLoading: false, + resolutionKey: "target\0tree-a", + }), + false, + ); + assert.equal( + shouldResolveRepositoryTarget({ + attempt, + hasError: false, + isLoading: false, + resolutionKey: "target\0tree-b", + }), + true, + ); +}); + +test("repository target resolution is re-armed when snapshot contents change", () => { + const targetKey = "main\0GUIDES/RUNBOOK.md"; + assert.notEqual( + repositoryTargetResultKey(targetKey, "commit-a\0README.md"), + repositoryTargetResultKey(targetKey, "commit-b\0README.md"), + ); +}); + +test("repository target failures offer the repository root", () => { + const onClick = () => {}; + const action = repositoryTarget.repositoryRootToastAction?.(onClick); + assert.ok(action, "repository root action is available"); + assert.equal(action.label, "Open repository root"); + assert.equal(action.onClick, onClick); +}); diff --git a/desktop/src/features/projects/lib/repositoryDeepLinkTarget.ts b/desktop/src/features/projects/lib/repositoryDeepLinkTarget.ts new file mode 100644 index 0000000000..d8e9bf6444 --- /dev/null +++ b/desktop/src/features/projects/lib/repositoryDeepLinkTarget.ts @@ -0,0 +1,67 @@ +export function effectiveProjectRepoSource( + selectedSource: "remote" | "local", + repositoryPath: string | undefined, +): "remote" | "local" { + return repositoryPath ? "remote" : selectedSource; +} + +export type RepositoryDeepLinkTarget = + | { kind: "file"; file: T; parentPath: string } + | { kind: "directory"; path: string } + | { kind: "missing" }; + +export function resolveRepositoryDeepLinkTarget( + files: readonly T[], + targetPath: string, +): RepositoryDeepLinkTarget { + const file = files.find((candidate) => candidate.path === targetPath); + if (file) { + const slash = targetPath.lastIndexOf("/"); + return { + kind: "file", + file, + parentPath: slash >= 0 ? targetPath.slice(0, slash) : "", + }; + } + const directoryPrefix = `${targetPath}/`; + if (files.some((candidate) => candidate.path.startsWith(directoryPrefix))) { + return { kind: "directory", path: targetPath }; + } + return { kind: "missing" }; +} + +export function projectTabForRepositoryTarget( + targetPath: string | undefined, +): "files" | "overview" { + return targetPath ? "files" : "overview"; +} + +export type RepositoryTargetAttempt = { + key: string; + outcome: "error" | "resolved"; +}; + +export function shouldResolveRepositoryTarget(input: { + attempt: RepositoryTargetAttempt | null; + hasError: boolean; + isLoading: boolean; + resolutionKey: string | null; +}): boolean { + if (!input.resolutionKey || input.isLoading) return false; + if (input.attempt?.key !== input.resolutionKey) return true; + return input.attempt.outcome === "error" && !input.hasError; +} + +export function repositoryTargetResultKey( + targetKey: string, + filesKey: string, +): string { + return `${targetKey}\0${filesKey}`; +} + +export function repositoryRootToastAction(onClick: () => void): { + label: string; + onClick: () => void; +} { + return { label: "Open repository root", onClick }; +} diff --git a/desktop/src/features/projects/ui/ProjectDetailScreen.tsx b/desktop/src/features/projects/ui/ProjectDetailScreen.tsx index 1b2adf316a..87cbbec29a 100644 --- a/desktop/src/features/projects/ui/ProjectDetailScreen.tsx +++ b/desktop/src/features/projects/ui/ProjectDetailScreen.tsx @@ -59,6 +59,12 @@ import { projectBranchOptionsFromSync, resolveProjectDefaultBranch, } from "@/features/projects/lib/projectBranches"; +import { + PROJECT_DETAIL_PANEL_SEARCH_KEYS, + PROJECT_REPOSITORY_SEARCH_KEYS, + type ProjectDetailScreenProps, +} from "@/features/projects/lib/projectDetailRouteState"; +import { effectiveProjectRepoSource } from "@/features/projects/lib/repositoryDeepLinkTarget"; import { normalizeRepositoryUrl } from "@/features/projects/lib/projectsViewHelpers"; import { selectProjectRepository } from "@/features/projects/projectModels"; import { KIND_REPO_ANNOUNCEMENT } from "@/shared/constants/kinds"; @@ -82,28 +88,16 @@ import { snapshotHasContent, } from "./projectDetailHelpers"; -type ProjectDetailScreenProps = { - commitHash?: string; - projectId: string; - pullRequestId?: string; - issueId?: string; - repositoryId?: string; -}; - -const PROJECT_DETAIL_PANEL_SEARCH_KEYS = [ - "profile", - "profileTab", - "profileView", -] as const; -const PROJECT_REPOSITORY_SEARCH_KEYS = [ - "repositoryId", - "issueId", - "pullRequestId", - "commitHash", -] as const; - export function ProjectDetailScreen(props: ProjectDetailScreenProps) { - const { commitHash, projectId, pullRequestId, issueId, repositoryId } = props; + const { + commitHash, + projectId, + pullRequestId, + issueId, + repoPath, + repoRef, + repositoryId, + } = props; const { goChannel, goProject, goProjects } = useAppNavigation(); const { activeCommunity } = useCommunities(); const mainInsetRef = useMainInsetRef(); @@ -205,6 +199,16 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { }, [], ); + const handleGoToProjectHome = React.useCallback(() => { + if (repoPath) { + void goProject(projectId, { replace: true }); + return; + } + setSelectedPullRequestId(null); + setSelectedIssueId(null); + setSelectedCommitHash(null); + setTabsResetKey((key) => key + 1); + }, [goProject, projectId, repoPath]); const issuesQuery = useProjectIssuesQuery(repository); const selectedBranchPullRequest = React.useMemo(() => { const projectRepositories = new Set( @@ -231,30 +235,32 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { const [repoSource, setRepoSource] = React.useState<"remote" | "local">( "remote", ); + const effectiveRepoSource = effectiveProjectRepoSource(repoSource, repoPath); const repoSnapshotQuery = useProjectRepoSnapshotQuery( repository, activeBranch, selectedTag ? null : selectedBranchPullRequest, activeTag, repoRemote.host.kind === "buzz", + repoRef && repoPath ? { ref: repoRef, path: repoPath } : null, ); const repoDiffQuery = useProjectRepoDiffQuery( repository, activeBranch, activeRepoPullRequest, - repoSource === "remote", + effectiveRepoSource === "remote", ); const localRepoDiffQuery = useProjectLocalRepoDiffQuery( repository, activeCommunity?.reposDir, activeBranch, activeRepoPullRequest, - repoSource === "local" && Boolean(activeRepoPullRequest), + effectiveRepoSource === "local" && Boolean(activeRepoPullRequest), ); const commitDiffQuery = useProjectCommitDiffQuery( repository, selectedCommitHash, - repoSource, + effectiveRepoSource, activeCommunity?.reposDir, ); const localRepoSnapshotQuery = useProjectLocalRepoSnapshotQuery( @@ -292,11 +298,15 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { ); const hasRemoteSnapshot = snapshotHasContent(repoSnapshotQuery.data); const displayedRepoDiff = - repoSource === "local" ? localRepoDiffQuery.data : repoDiffQuery.data; + effectiveRepoSource === "local" + ? localRepoDiffQuery.data + : repoDiffQuery.data; const displayedRepoDiffError = - repoSource === "local" ? localRepoDiffQuery.error : repoDiffQuery.error; + effectiveRepoSource === "local" + ? localRepoDiffQuery.error + : repoDiffQuery.error; const displayedRepoDiffLoading = - repoSource === "local" + effectiveRepoSource === "local" ? localRepoDiffQuery.isLoading : repoDiffQuery.isLoading; const branchOptionsWithLocal = projectBranchOptionsFromSync( @@ -322,13 +332,13 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { selectBranch(branch); if ( branch && - repoSource === "local" && + effectiveRepoSource === "local" && branch !== repoSyncStatusQuery.data?.localBranch ) { setRepoSource("remote"); } }, - [repoSource, repoSyncStatusQuery.data?.localBranch, selectBranch], + [effectiveRepoSource, repoSyncStatusQuery.data?.localBranch, selectBranch], ); const handleTagChange = React.useCallback( (tag: string) => { @@ -386,7 +396,7 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { deleteBranchDisabled: branchActions.deletePending || Boolean(deleteBranchReason), deleteBranchTitle: deleteBranchReason ?? "Delete this remote branch", - source: selectedTag ? "remote" : repoSource, + source: selectedTag ? "remote" : effectiveRepoSource, onSourceChange: setRepoSource, localDisabled: Boolean(selectedTag) || @@ -775,7 +785,7 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { const selectedIssue = issuesQuery.data?.find((item) => item.id === selectedIssueId) ?? null; const displayedSnapshotCommits = - repoSource === "local" + effectiveRepoSource === "local" ? (localRepoSnapshotQuery.data?.snapshot.commits ?? []) : (repoSnapshotQuery.data?.commits ?? []); const selectedCommit = selectedCommitHash @@ -810,17 +820,11 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { const activeTabCrumb = activeWorkItemCrumb ? null : (PROJECT_TAB_CRUMB_LABELS[activeTab] ?? null); - const handleGoToProjectHome = () => { - setSelectedPullRequestId(null); - setSelectedIssueId(null); - setSelectedCommitHash(null); - // Remount the workspace tabs so the project page opens on Overview - // instead of whatever tab the work item left behind. - setTabsResetKey((key) => key + 1); - }; const handleRepositoryChange = (nextRepositoryId: string) => { applyRepositorySearch({ repositoryId: nextRepositoryId, + repoRef: null, + repoPath: null, issueId: null, pullRequestId: null, commitHash: null, @@ -867,7 +871,7 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) { {repoRemote.webUrl && (repoRemote.host.kind !== "external" || - repoSource === "local") ? ( + effectiveRepoSource === "local") ? (