diff --git a/crates/signed_git/src/lib.rs b/crates/signed_git/src/lib.rs index 4413542..835ec77 100644 --- a/crates/signed_git/src/lib.rs +++ b/crates/signed_git/src/lib.rs @@ -195,6 +195,185 @@ pub fn apply_patch(repo_path: &Path, patch: &str) -> Result<()> { Ok(()) } +/// The merge base of two revisions (branch names, remote-tracking refs or +/// commit ids) in the repository at `repo_path`. `Ok(None)` when the +/// revisions share no common ancestor; unresolvable revisions are errors. +pub fn merge_base(repo_path: &Path, a: &str, b: &str) -> Result> { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["merge-base", a, b]) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git merge-base`")?; + + match output.status.code() { + // Exit 1: no common ancestor (a valid outcome for a proposal). + Some(1) => Ok(None), + Some(0) => Ok(Some( + String::from_utf8_lossy(&output.stdout).trim().to_owned(), + )), + _ => bail!( + "git merge-base failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ), + } +} + +/// The `git format-patch` series of `base..tip` (mbox), like +/// `git format-patch --stdout`. Fails when the range has no commits. The +/// mbox is returned untrimmed; trailing newlines are part of the format. +pub fn format_patch_between(repo_path: &Path, base: &str, tip: &str) -> Result { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["format-patch", "--stdout", &format!("{base}..{tip}")]) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git format-patch`")?; + + if !output.status.success() { + bail!( + "git format-patch failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + } + let patch = String::from_utf8_lossy(&output.stdout).into_owned(); + if patch.trim().is_empty() { + bail!("no commits between {base} and {tip}"); + } + Ok(patch) +} + +/// Whether `patch` (a `git format-patch` series) applies to the working +/// tree of `repo_path`, without modifying anything +/// (`git apply --check --3way`). Best-effort: useful to surface conflicts +/// before a patch is published or applied. +pub fn patch_applies(repo_path: &Path, patch: &str) -> Result<()> { + let mut child = Command::new("git") + .arg("apply") + .args(["--check", "--3way", "--whitespace=nowarn", "-"]) + .current_dir(repo_path) + .stdin(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .context("failed to spawn `git apply --check`")?; + + child + .stdin + .as_mut() + .expect("stdin piped") + .write_all(patch.as_bytes())?; + + let output = child.wait_with_output()?; + if !output.status.success() { + bail!( + "patch does not apply: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + } + Ok(()) +} + +/// Push `commit` to `reference` (e.g. `refs/nostr/`) on the git +/// server at `url`, from the repository at `repo_path`. GRASP servers host +/// the `refs/nostr` namespace so anyone can contribute a commit; nak pushes +/// pull request tips there before publishing the PR event, and readers +/// fetch the ref to get the commit behind a PR's `c` tag. +pub fn push_commit_ref(repo_path: &Path, url: &str, commit: &str, reference: &str) -> Result<()> { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["push"]) + .arg(url) + .arg(format!("{commit}:{reference}")) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git push`")?; + + if !output.status.success() { + bail!( + "git push failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ); + } + Ok(()) +} + +/// Split a `git format-patch` series into its individual patches (mbox +/// messages). Each message begins with a `From <40-hex> ` boundary line; +/// `>From` quoting inside bodies means no false positives. A single patch +/// yields one element; a malformed input yields one element covering it. +pub fn split_patch_series(patch: &str) -> Vec<&str> { + let mut starts = vec![0usize]; + let mut search_from = 1; + while let Some(rel) = patch[search_from..].find("\nFrom ") { + let ix = search_from + rel + 1; + let hex = patch[ix + 5..] + .split(|c: char| !c.is_ascii_hexdigit()) + .next() + .unwrap_or(""); + if hex.len() == 40 { + starts.push(ix); + } + search_from = ix + 1; + } + + starts + .iter() + .enumerate() + .map(|(i, &start)| { + let end = starts.get(i + 1).copied().unwrap_or(patch.len()); + &patch[start..end] + }) + .collect() +} + +/// The commit HEAD points to in the repository at `repo_path`, or `None` +/// when the repository has no commits yet (unborn HEAD). +pub fn head_commit_id(repo_path: &Path) -> Result> { + let output = Command::new("git") + .arg("-C") + .arg(repo_path) + .args(["rev-parse", "HEAD"]) + .env("GIT_TERMINAL_PROMPT", "0") + .stderr(Stdio::piped()) + .output() + .context("failed to spawn `git rev-parse`")?; + + if !output.status.success() { + return Ok(None); + } + Ok(Some( + String::from_utf8_lossy(&output.stdout).trim().to_owned(), + )) +} + +/// The commits in `base..HEAD` of the repository at `repo_path`, oldest +/// first (the order `git am` creates them); `HEAD` alone when `base` is +/// `None`. An empty range yields an empty list. +pub fn commits_since(repo_path: &Path, base: Option<&str>) -> Result> { + let output = match base { + Some(base) => git_in( + repo_path, + &["rev-list", "--reverse", &format!("{base}..HEAD")], + )?, + // No `base` (unborn HEAD): there is nothing to walk yet. + None => match git_in(repo_path, &["rev-parse", "HEAD"]) { + Ok(head) => head, + Err(_) => return Ok(Vec::new()), + }, + }; + Ok(output + .lines() + .map(str::to_owned) + .filter(|line| !line.is_empty()) + .collect()) +} + fn clone(url: &str, path: &Path) -> Result { // GRASP servers announce `grasp:////` clone URLs; // the transport is git smart HTTP, so rewrite the scheme for gix. @@ -1884,6 +2063,200 @@ mod tests { git_run(repo.workdir().expect("workdir"), &["commit", "-m", message]); } + #[test] + fn merge_base_finds_the_fork_point_and_reports_unrelated_history() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + // A feature branch and a mainline commit diverge from the initial + // commit; it is their merge base. + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("feature.txt"), "feature\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "feature commit"); + git_run(&path, &["checkout", "main"]); + std::fs::write(path.join("main.txt"), "main\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "mainline commit"); + + assert_eq!( + merge_base(&path, "feature", "main") + .expect("merge base") + .as_deref(), + Some(initial.as_str()) + ); + + // An orphan branch shares no history with main: `Ok(None)`. + git_run(&path, &["checkout", "--orphan", "orphan"]); + std::fs::write(path.join("orphan.txt"), "orphan\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "orphan commit"); + assert_eq!(merge_base(&path, "orphan", "main").expect("ok"), None); + + // An unresolvable revision is an error, not a missing ancestor. + assert!(merge_base(&path, "orphan", "no-such-ref").is_err()); + } + + #[test] + fn format_patch_between_produces_the_series_and_rejects_empty_ranges() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("feature.txt"), "feature\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "feature commit"); + + let patch = format_patch_between(&path, &initial, "feature").expect("patch"); + assert!(patch.contains("Subject: [PATCH] feature commit")); + assert!(patch.contains("feature.txt")); + + // An empty range has no commits to send. + assert!(format_patch_between(&path, "feature", "feature").is_err()); + } + + #[test] + fn patch_applies_checks_without_modifying_the_tree() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("feature.txt"), "feature\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "feature commit"); + let patch = format_patch_between(&path, &initial, "feature").expect("patch"); + + // A clone of the initial state accepts the series... + let clone = dir.path().join("clone"); + git_run( + dir.path(), + &[ + "clone", + "-q", + path.to_str().unwrap(), + clone.to_str().unwrap(), + ], + ); + git_run(&clone, &["checkout", "-q", &initial]); + assert!(patch_applies(&clone, &patch).is_ok()); + // ...and the check must not have modified the working tree. + assert!(!clone.join("feature.txt").exists()); + + // A conflicting file makes the same series fail the check. + std::fs::write(clone.join("feature.txt"), "conflicting\n").expect("write"); + assert!(patch_applies(&clone, &patch).is_err()); + } + + #[test] + fn push_commit_ref_pushes_to_the_event_namespace() { + // A bare "server" repository reachable via a `file://` URL, like a + // grasp server's `{base}/{owner}/{repo-id}.git` layout. + let server = tempfile::tempdir().unwrap(); + let server_repo = server.path().join("npub1test").join("my-repo.git"); + std::fs::create_dir_all(server_repo.parent().unwrap()).unwrap(); + let init_status = Command::new("git") + .args(["init", "--bare", "-q"]) + .arg(&server_repo) + .status() + .expect("spawn git init --bare"); + assert!(init_status.success()); + + let (dir, repo) = fixture(&[("a.txt", b"one")]); + commit_all(&repo, "initial"); + let dir = dir.path(); + let tip = git_in(dir, &["rev-parse", "HEAD"]).expect("tip"); + + let url = format!("file://{}/npub1test/my-repo.git", server.path().display()); + push_commit_ref(dir, &url, &tip, "refs/nostr/abcd1234").expect("push"); + + let refs = git_in(&server_repo, &["show-ref"]).expect("server refs"); + assert!(refs.contains("refs/nostr/abcd1234")); + } + + #[test] + fn split_patch_series_splits_real_multi_commit_mboxes() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + git_run(&path, &["checkout", "-b", "feature"]); + std::fs::write(path.join("one.txt"), "one\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "first commit"); + std::fs::write(path.join("two.txt"), "two\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "second commit"); + + let series = format_patch_between(&path, &initial, "feature").expect("series"); + let parts = split_patch_series(&series); + + assert_eq!(parts.len(), 2); + assert!(parts[0].contains("Subject: [PATCH 1/2] first commit")); + assert!(parts[1].contains("Subject: [PATCH 2/2] second commit")); + // Each part starts its own mbox message with its own commit id. + let first = parts[0].lines().next().expect("first header"); + let second = parts[1].lines().next().expect("second header"); + assert!(first.starts_with("From ") && first.len() >= 45); + assert_ne!(first, second); + } + + #[test] + fn split_patch_series_keeps_single_patches_whole() { + let patch = "From abcdefabcdefabcdefabcdefabcdefabcdefab Mon Sep 17 00:00:00 2001\nFrom: A \nSubject: [PATCH] fix\n\n---\n"; + let parts = split_patch_series(patch); + assert_eq!(parts.len(), 1); + assert_eq!(parts[0], patch); + } + + #[test] + fn head_commit_and_commits_since_track_applied_commits() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let initial = init_repository(&path, "My Repo", "desc").expect("init"); + + assert_eq!( + head_commit_id(&path).expect("head").as_deref(), + Some(initial.as_str()) + ); + // No commits yet: `HEAD` alone. + assert_eq!( + commits_since(&path, None).expect("commits"), + vec![initial.clone()] + ); + + std::fs::write(path.join("one.txt"), "one\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "first commit"); + let first = head_commit_id(&path).expect("head").expect("on a branch"); + + std::fs::write(path.join("two.txt"), "two\n").expect("write"); + commit_all(&gix::open(&path).expect("open"), "second commit"); + let second = head_commit_id(&path).expect("head").expect("on a branch"); + + // Oldest first, like the order `git am` creates them. + assert_eq!( + commits_since(&path, Some(&initial)).expect("commits"), + vec![first.clone(), second.clone()] + ); + assert_eq!( + commits_since(&path, Some(&first)).expect("commits"), + vec![second] + ); + } + + #[test] + fn head_commit_reports_unborn_repositories() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("repo"); + let status = Command::new("git") + .args(["init", "-q"]) + .arg(&path) + .status() + .expect("spawn git init"); + assert!(status.success()); + + assert_eq!(head_commit_id(&path).expect("head"), None); + assert_eq!( + commits_since(&path, None).expect("commits"), + Vec::::new() + ); + } + #[test] fn init_repository_creates_main_branch_and_readme() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 5236925..190130c 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -1302,6 +1302,55 @@ impl Backend { }) } + /// Broadcast and locally store an already-signed event, like + /// [`Self::send`] without the signing step. Callers that signed early + /// (e.g. to learn the event id before pushing a commit to the grasp + /// servers) publish through this. + pub fn publish_event( + &mut self, + event: Event, + cx: &mut Context, + ) -> Task> { + let client = self.client.clone(); + + cx.spawn(async move |this, cx| { + let work = cx.background_spawn(async move { + let output = client.send_event(&event).await?; + + if output.success.is_empty() && !output.failed.is_empty() { + let reasons = output + .failed + .values() + .cloned() + .collect::>() + .join(", "); + return Err(anyhow!("event not accepted by any relay: {reasons}")); + } + + Ok(event.clone()) + }); + + let result = work.await; + + match &result { + Ok(event) => { + this.update(cx, |_this, cx| { + cx.emit(BackendEvent::Published(Box::new(event.clone()))); + }) + .ok(); + } + Err(e) => { + this.update(cx, |_this, cx| { + cx.emit(BackendEvent::error(e.to_string())); + }) + .ok(); + } + } + + result + }) + } + /// Publish a NIP-34 repository announcement (kind 30617) with the /// current signer. The returned task yields the published event, so /// callers can show inline progress/errors. @@ -1414,7 +1463,12 @@ async fn connect_repo_relays_only( let relays = &relays; let sync_opts = sync_opts.clone(); async move { - if let Err(e) = client.sync(filter).with(relays.iter()).opts(sync_opts).await { + if let Err(e) = client + .sync(filter) + .with(relays.iter()) + .opts(sync_opts) + .await + { log::warn!("repo relay negentropy sync failed: {e}"); } } @@ -1469,7 +1523,7 @@ fn with_master_key(uri: &str, keys: &Keys) -> String { /// A `https://` (or `http://` for `ws://` grasp servers, like /// ngit) base URL for a grasp server. The repository then lives at /// `{base}/{npub}/{repo-id}.git`. -fn grasp_base_url(relay: &RelayUrl) -> Option { +pub(crate) fn grasp_base_url(relay: &RelayUrl) -> Option { // `domain()` drops the port; parse the full URL to keep it (local dev // grasp servers commonly run on a custom port). let parsed = Url::parse(relay.as_str()).ok()?; diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index a5a154c..ed66c7c 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -1,10 +1,11 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::path::PathBuf; use std::time::Duration; use anyhow::Error; use bitcoin_hashes::sha1::Hash as Sha1Hash; -use gpui::{AppContext, Context, Subscription, Task}; +use gpui::{AppContext, AsyncApp, Context, Subscription, Task, WeakEntity}; use nostr::event::IntoEventBuilder; use nostr_sdk::prelude::*; use signed_core::{ @@ -13,13 +14,17 @@ use signed_core::{ subject_override, }; -use crate::backend::{Backend, BackendEvent}; +use crate::backend::{Backend, BackendEvent, grasp_base_url}; use crate::git_store::GitStore; /// Delay between a refresh request and the actual re-query, so bursts of /// events (e.g. per-event `NostrUpdate`s) collapse into one query. const REFRESH_DEBOUNCE: Duration = Duration::from_millis(300); +/// Maximum size of one patch event, following NIP-34's guidance that +/// patches should be used when each event is under 60kb. +const MAX_PATCH_EVENT_BYTES: usize = 60 * 1024; + /// Per-repository store: announcement, state, issues, patches, PRs, /// comments and their resolved statuses. Always derived from the local /// database. @@ -52,6 +57,9 @@ pub struct RepoStore { version: u64, /// Error of the last action initiated from this store, if any. pub last_error: Option, + /// Non-fatal warning of the last action (e.g. a PR published without + /// its commit reaching a grasp server), if any. + pub last_warning: Option, /// Relays announced by this repository (NIP-34 `relays` tag) that we /// have already been asked to connect to and fetch from, to avoid /// re-subscribing on every refresh. @@ -130,6 +138,7 @@ impl RepoStore { labels: Vec::new(), version: 0, last_error: None, + last_warning: None, repo_relays: HashSet::new(), root_fetches: HashSet::new(), refreshing: false, @@ -620,14 +629,23 @@ impl RepoStore { /// (kind 1617) carrying the `git format-patch` output, which the PR /// references via an `e` tag (NIP-34). /// - /// The patch is published first so the PR can reference its id. The - /// proposed commit is parsed from the patch's `From ` header; - /// without one publishing is refused, because the PR's `c` tag must - /// carry a real commit id for other NIP-34 clients to verify and apply - /// the proposal. The `clone` tag carries the announced mirror URLs; the - /// linked patch is the source of truth until the commit is pushed there. + /// The patch series is published first (one kind-1617 event per commit, + /// chained with NIP-10 `e` replies, each under [`MAX_PATCH_EVENT_BYTES`]) + /// so the PR can reference the root patch's id. The proposed commit is + /// parsed from the series' last `From ` header (the tip); without + /// one publishing is refused, because the PR's `c` tag must carry a real + /// commit id for other NIP-34 clients to verify and apply the proposal. + /// The `clone` tag carries the announced mirror URLs, and when + /// `push_from` is set the tip is pushed to those servers under + /// `refs/nostr/` (best-effort) before the PR is published, so + /// the commit is actually downloadable there; the linked patch stays the + /// source of truth either way. + /// /// `branch_name` lands in the PR's `branch-name` tag (NIP-34); `draft` - /// publishes a kind-1633 status right after the PR event. + /// publishes a kind-1633 status right after the PR event. `merge_base` + /// is the hex commit the proposed branch forked from, computed from a + /// local checkout when the patch was generated there. + #[allow(clippy::too_many_arguments)] pub fn open_pull_request( &mut self, subject: Option, @@ -635,12 +653,36 @@ impl RepoStore { branch_name: Option, patch: String, draft: bool, + merge_base: Option, + push_from: Option, cx: &mut Context, ) { self.last_error = None; + self.last_warning = None; - let Some(current_commit) = - patch_current_commit(&patch).and_then(|hex| hex.parse::().ok()) + let series: Vec = signed_git::split_patch_series(&patch) + .into_iter() + .map(str::to_owned) + .collect(); + if let Some(oversized) = series + .iter() + .find(|part| part.len() > MAX_PATCH_EVENT_BYTES) + { + self.last_error = Some(format!( + "patch too large ({} bytes; NIP-34 suggests keeping each patch under {} bytes)", + oversized.len(), + MAX_PATCH_EVENT_BYTES + )); + cx.notify(); + return; + } + + // The tip of the series is its last commit; `git format-patch` + // orders patches oldest first. + let Some(current_commit) = series + .last() + .and_then(|part| patch_current_commit(part)) + .and_then(|hex| hex.parse::().ok()) else { self.last_error = Some( "Patch must be `git format-patch` output with a `From ` header".into(), @@ -649,35 +691,41 @@ impl RepoStore { return; }; - let Ok(root_marker) = Tag::parse(["t", "root"]) else { - return; - }; - let commit_hex = current_commit.to_string(); - let mut patch_tags = vec![ - Tag::coordinate(self.addr.clone(), None), - Tag::public_key(self.addr.public_key), - root_marker, - ]; - // NIP-34: the `r` EUC tag lets clients subscribe to all patches of - // this repository; `commit`/`r` tags reference the proposed commit. - if let Some(euc) = self.announcement.as_ref().and_then(|a| a.euc.clone()) - && let Ok(tag) = Tag::parse(["r", &euc]) - { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["commit", &commit_hex]) { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["r", &commit_hex]) { - patch_tags.push(tag); - } - let patch_builder = EventBuilder::new(Kind::GitPatch, patch).tags(patch_tags); - let backend = Backend::global(cx); - let patch_task = backend.update(cx, |backend, cx| backend.send(patch_builder, cx)); + if backend.read(cx).current_user().is_none() { + self.last_error = Some("Sign in to open a pull request".into()); + cx.notify(); + return; + } + let signer = backend.read(cx).signer(); + + let addr = self.addr.clone(); + let owner = self.addr.public_key; + let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); + let (push_owner, push_repo_id, push_relays) = self + .announcement + .as_ref() + .map(|a| { + let owner = a.owner.to_bech32().unwrap_or_else(|_| a.owner.to_hex()); + (owner, a.id.clone(), a.relays.clone()) + }) + .unwrap_or_default(); self.tasks.push(cx.spawn(async move |this, cx| { - let patch_event = match patch_task.await { + // The PR references the root patch event so viewers can find + // the patch without carrying it inline. + let root_patch = match publish_patch_series( + &this, + cx, + &addr, + owner, + euc.as_deref(), + &series, + "root", + None, + ) + .await + { Ok(event) => event, Err(e) => { return this.update(cx, |this, cx| { @@ -687,9 +735,7 @@ impl RepoStore { } }; - // The PR references the patch event so viewers can find the - // patch without carrying it inline. - let pr_task = this.update(cx, |this, cx| { + let builder = this.update(cx, |this, _cx| { let builder = GitPullRequest { repository: this.addr.clone(), content: description, @@ -697,30 +743,82 @@ impl RepoStore { labels: Vec::new(), branch_name, // NIP-34: PRs carry at least one clone URL where the - // tip commit can be downloaded; use the repository's - // announced mirrors until a push backend exists. + // tip commit can be downloaded; the announced mirrors + // are also the servers the tip is pushed to below. clone: this .announcement .as_ref() .map(|a| a.clone.clone()) .unwrap_or_default(), current_commit, - root_patch_event: Some(patch_event.id), - merge_base: None, + root_patch_event: Some(root_patch.id), + merge_base: merge_base + .and_then(|hex| hex.parse::().ok()), } .into_event_builder(); - // NIP-34: the `r` EUC tag lets clients subscribe to all PRs of this repository. - let builder = match this.announcement.as_ref().and_then(|a| a.euc.clone()) { + // NIP-34: the `r` EUC tag lets clients subscribe to all + // PRs of this repository; the SDK builder omits it. + match this.announcement.as_ref().and_then(|a| a.euc.clone()) { Some(euc) => builder.tag(Tag::parse(["r", &euc]).expect("valid r tag")), None => builder, - }; - - let backend = Backend::global(cx); - backend.update(cx, |backend, cx| backend.send(builder, cx)) + } })?; - let pr_event = match pr_task.await { + // Sign before publishing so the tip can be pushed to the grasp + // servers under `refs/nostr/` (nak's convention): + // readers fetch that ref to get the commit behind the `c` tag. + let event = cx + .background_spawn({ + let signer = signer.clone(); + async move { builder.finalize_async(&signer).await } + }) + .await?; + + if let Some(path) = push_from.as_ref() { + let tip = current_commit.to_string(); + let reference = format!("refs/nostr/{}", event.id.to_hex()); + let (pushed, failures) = cx + .background_spawn({ + let path = path.clone(); + let tip = tip.clone(); + let reference = reference.clone(); + let owner = push_owner.clone(); + let repo_id = push_repo_id.clone(); + let relays = push_relays.clone(); + async move { + let mut failures = Vec::new(); + let mut pushed = 0; + for relay in &relays { + let Some(base) = grasp_base_url(relay) else { + continue; + }; + let url = format!("{base}/{owner}/{repo_id}.git"); + match signed_git::push_commit_ref(&path, &url, &tip, &reference) { + Ok(()) => pushed += 1, + Err(e) => failures.push(format!("{relay}: {e}")), + } + } + (pushed, failures) + } + }) + .await; + if pushed == 0 { + this.update(cx, |this, cx| { + this.last_warning = Some(format!( + "Pull request published, but the commit could not be pushed to any grasp server ({}); the patch is still the source of truth", + failures.join("; ") + )); + cx.notify(); + })?; + } + } + + let publish_task = this.update(cx, |_this, cx| { + let backend = Backend::global(cx); + backend.update(cx, |backend, cx| backend.publish_event(event, cx)) + })?; + let pr_event = match publish_task.await { Ok(event) => event, Err(e) => { return this.update(cx, |this, cx| { @@ -742,13 +840,15 @@ impl RepoStore { })); } - /// Update a pull request: publish a revision patch event chained to the - /// original root patch (`t root-revision` and a NIP-10 `e` reply, per - /// NIP-34), then a kind-1619 PR update event carrying the new tip. + /// Update a pull request: publish revision patch events chained to the + /// original root patch (`t root-revision` and a NIP-10 `e` reply on the + /// first, per NIP-34), then a kind-1619 PR update event carrying the + /// new tip. /// /// Only the PR author may update it; other authors must open a new PR. pub fn update_pull_request(&mut self, root: &Event, patch: String, cx: &mut Context) { self.last_error = None; + self.last_warning = None; let backend = Backend::global(cx); @@ -764,8 +864,28 @@ impl RepoStore { return; } - let Some(current_commit) = - patch_current_commit(&patch).and_then(|hex| hex.parse::().ok()) + let series: Vec = signed_git::split_patch_series(&patch) + .into_iter() + .map(str::to_owned) + .collect(); + if let Some(oversized) = series + .iter() + .find(|part| part.len() > MAX_PATCH_EVENT_BYTES) + { + self.last_error = Some(format!( + "patch too large ({} bytes; NIP-34 suggests keeping each patch under {} bytes)", + oversized.len(), + MAX_PATCH_EVENT_BYTES + )); + cx.notify(); + return; + } + + // The new tip of the PR is the last commit of the series. + let Some(current_commit) = series + .last() + .and_then(|part| patch_current_commit(part)) + .and_then(|hex| hex.parse::().ok()) else { self.last_error = Some( "Patch must be `git format-patch` output with a `From ` header".into(), @@ -783,45 +903,29 @@ impl RepoStore { .map(|p| p.id) }); - let commit_hex = current_commit.to_string(); - let mut patch_tags = vec![ - Tag::coordinate(self.addr.clone(), None), - Tag::public_key(self.addr.public_key), - Tag::parse(["t", "root-revision"]).expect("valid root-revision tag"), - ]; - if let Some(root_patch_id) = root_patch_id - && let Ok(tag) = Tag::parse(["e", &root_patch_id.to_hex(), "", "reply"]) - { - patch_tags.push(tag); - } - // NIP-34: the `r` EUC tag lets clients subscribe to all patches of - // this repository; `commit`/`r` tags reference the new tip. - if let Some(euc) = self.announcement.as_ref().and_then(|a| a.euc.clone()) - && let Ok(tag) = Tag::parse(["r", &euc]) - { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["commit", &commit_hex]) { - patch_tags.push(tag); - } - if let Ok(tag) = Tag::parse(["r", &commit_hex]) { - patch_tags.push(tag); - } - let patch_builder = EventBuilder::new(Kind::GitPatch, patch).tags(patch_tags); - - let backend = Backend::global(cx); - let patch_task = backend.update(cx, |backend, cx| backend.send(patch_builder, cx)); - + let addr = self.addr.clone(); + let owner = self.addr.public_key; + let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); let root = root.clone(); let clone: Vec = self .announcement .as_ref() .map(|a| a.clone.clone()) .unwrap_or_default(); - let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); self.tasks.push(cx.spawn(async move |this, cx| { - if let Err(e) = patch_task.await { + if let Err(e) = publish_patch_series( + &this, + cx, + &addr, + owner, + euc.as_deref(), + &series, + "root-revision", + root_patch_id, + ) + .await + { return this.update(cx, |this, cx| { this.last_error = Some(e.to_string()); cx.notify(); @@ -846,6 +950,7 @@ impl RepoStore { None => builder, }; + let backend = Backend::global(cx); backend.update(cx, |backend, cx| backend.send(builder, cx)) })?; @@ -953,7 +1058,10 @@ impl RepoStore { /// Merge a pull request: apply its patch (the content of the linked /// root patch event) to the local clone of this repository, then publish - /// the merged status. + /// a kind-1631 (Applied) status event with merge provenance: the commits + /// `git am` created (`applied-as-commits` + `r` tags) and the applied + /// patch events (`q` tags, plus `e` reply tags for every patch beyond + /// the root, per NIP-34). /// /// Only the repository author may merge. The clone is created on demand /// from the announcement's clone URLs when needed. Patch application @@ -961,6 +1069,7 @@ impl RepoStore { /// no longer applies) surface in [`Self::last_error`]. pub fn merge_pull_request(&mut self, root: &Event, cx: &mut Context) { self.last_error = None; + self.last_warning = None; let is_author = Backend::global(cx) .read(cx) @@ -979,21 +1088,46 @@ impl RepoStore { .map(|a| a.clone.iter().map(ToString::to_string).collect()) .unwrap_or_default(); let patch = pull_request_patch(root, self.patches.iter()); + // The applied patch events, for the status tags below. + let patches: Vec = pull_request_patches(root, self.patches.iter()) + .into_iter() + .cloned() + .collect(); + let relay_hint = self + .announcement + .as_ref() + .and_then(|a| a.relays.first()) + .map(ToString::to_string) + .unwrap_or_default(); + let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); let root = root.clone(); let apply = cx.background_spawn(async move { let repo = cache.ensure_clone(&addr, &clone_urls)?; let workdir = repo .workdir() - .ok_or_else(|| anyhow::anyhow!("repository has no worktree"))?; - signed_git::apply_patch(workdir, &patch) + .ok_or_else(|| anyhow::anyhow!("repository has no worktree"))? + .to_path_buf(); + // The commits created by the apply: everything between the + // previous HEAD and the new one, oldest first. + let previous = signed_git::head_commit_id(&workdir)?; + signed_git::apply_patch(&workdir, &patch)?; + let applied = signed_git::commits_since(&workdir, previous.as_deref())?; + Ok::<_, Error>(applied) }); self.tasks.push(cx.spawn(async move |this, cx| { match apply.await { - Ok(()) => { + Ok(applied) => { this.update(cx, |this, cx| { - this.set_status(&root, RepoStatus::Applied, cx); + this.publish_applied_status( + &root, + &patches, + &applied, + &relay_hint, + euc.as_deref(), + cx, + ); })?; } Err(e) => { @@ -1007,6 +1141,62 @@ impl RepoStore { })); } + /// Publish a kind-1631 (Applied) status event for `root` after a merge: + /// `applied-as-commits` + `r` tags for the commits `git am` created, + /// `q` tags for the applied patch events, and `e` reply tags for every + /// patch of the series beyond the root (NIP-34). + fn publish_applied_status( + &mut self, + root: &Event, + patches: &[Event], + applied: &[String], + relay_hint: &str, + euc: Option<&str>, + cx: &mut Context, + ) { + let mut tags = vec![ + Tag::parse(["e", &root.id.to_hex(), "", "root"]).expect("valid root tag"), + Tag::public_key(self.addr.public_key), + Tag::public_key(root.pubkey), + Tag::coordinate(self.addr.clone(), None), + ]; + if let Some(euc) = euc + && let Ok(tag) = Tag::parse(["r", euc]) + { + tags.push(tag); + } + // The applied patch events: a `q` tag per event, plus an `e` reply + // for every event beyond the root (chain parts and revisions), so + // their statuses resolve to Applied too. + for (ix, patch) in patches.iter().enumerate() { + if let Ok(tag) = + Tag::parse(["q", &patch.id.to_hex(), relay_hint, &patch.pubkey.to_hex()]) + { + tags.push(tag); + } + if ix > 0 + && let Ok(tag) = Tag::parse(["e", &patch.id.to_hex(), "", "reply"]) + { + tags.push(tag); + } + } + // The commits `git am` created on top of the previous HEAD. + if !applied.is_empty() { + let mut applied_tag = vec!["applied-as-commits".to_string()]; + applied_tag.extend(applied.iter().cloned()); + if let Ok(tag) = Tag::parse(applied_tag) { + tags.push(tag); + } + for commit in applied { + if let Ok(tag) = Tag::parse(["r", commit]) { + tags.push(tag); + } + } + } + + self.send(EventBuilder::new(Kind::GitStatusApplied, "").tags(tags), cx); + } + fn send(&mut self, builder: EventBuilder, cx: &mut Context) { self.last_error = None; @@ -1092,6 +1282,78 @@ fn patch_current_commit(patch: &str) -> Option<&str> { hex.split_whitespace().next().filter(|hex| hex.len() == 40) } +/// Publish a `git format-patch` series as chained kind-1617 events and +/// return the root event (the one a PR references). The first part carries +/// `first_marker` (`t root`, or `t root-revision` with an `e` reply to +/// `reply_to` for revisions); every later part replies to the previous one +/// (NIP-34). Every part gets the repository coordinate, the owner, its own +/// `commit`/`r` tags, and the repository EUC when known. +#[allow(clippy::too_many_arguments)] +async fn publish_patch_series( + this: &WeakEntity, + cx: &mut AsyncApp, + addr: &RepoAddr, + owner: PublicKey, + euc: Option<&str>, + series: &[String], + first_marker: &str, + reply_to: Option, +) -> Result { + let mut root: Option = None; + let mut previous = reply_to; + + for (ix, part) in series.iter().enumerate() { + let Some(commit) = patch_current_commit(part).filter(|hex| hex.len() == 40) else { + return Err(anyhow::anyhow!( + "patch {} of the series has no `From ` header", + ix + 1 + )); + }; + + let mut tags = vec![Tag::coordinate(addr.clone(), None), Tag::public_key(owner)]; + if ix == 0 { + if let Ok(tag) = Tag::parse(["t", first_marker]) { + tags.push(tag); + } + if let Some(root_id) = reply_to + && let Ok(tag) = Tag::parse(["e", &root_id.to_hex(), "", "reply"]) + { + tags.push(tag); + } + } else if let Some(previous) = previous + && let Ok(tag) = Tag::parse(["e", &previous.to_hex(), "", "reply"]) + { + tags.push(tag); + } + if let Some(euc) = euc + && let Ok(tag) = Tag::parse(["r", euc]) + { + tags.push(tag); + } + if let Ok(tag) = Tag::parse(["commit", commit]) { + tags.push(tag); + } + if let Ok(tag) = Tag::parse(["r", commit]) { + tags.push(tag); + } + + let builder = EventBuilder::new(Kind::GitPatch, part.clone()).tags(tags); + + let task = this.update(cx, |_this, cx| { + let backend = Backend::global(cx); + backend.update(cx, |backend, cx| backend.send(builder, cx)) + })?; + let event = task.await?; + + if root.is_none() { + root = Some(event.clone()); + } + previous = Some(event.id); + } + + root.ok_or_else(|| anyhow::anyhow!("patch series is empty")) +} + /// Build a NIP-22 kind-1111 comment: uppercase `E`/`K`/`P` tags scope the /// thread root, lowercase `e`/`k`/`p` the direct parent (or the root for a /// top-level comment). An `a` tag with the repository coordinate (not part diff --git a/crates/workspace/src/views/repo_detail/pull_requests.rs b/crates/workspace/src/views/repo_detail/pull_requests.rs index a7554df..916edbf 100644 --- a/crates/workspace/src/views/repo_detail/pull_requests.rs +++ b/crates/workspace/src/views/repo_detail/pull_requests.rs @@ -1,12 +1,14 @@ +use std::path::{Path, PathBuf}; use std::rc::Rc; use assets::CustomIconName; use dock::{BasePanel, DockArea, DockPlacement, Panel, PanelEvent, panel_handle}; use gpui::prelude::*; use gpui::{ - AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, Pixels, Render, - SharedString, Size, WeakEntity, Window, div, px, size, + AnyElement, App, Context, Entity, EventEmitter, FocusHandle, Focusable, PathPromptOptions, + Pixels, Render, SharedString, Size, WeakEntity, Window, div, px, size, }; +use gpui_component::alert::Alert; use gpui_component::button::{Button, ButtonVariants}; use gpui_component::checkbox::Checkbox; use gpui_component::dialog::{DialogDescription, DialogFooter, DialogHeader, DialogTitle}; @@ -14,11 +16,13 @@ use gpui_component::form::{field, v_form}; use gpui_component::input::{Input, InputState, Textarea, TextareaState}; use gpui_component::scroll::Scrollbar; use gpui_component::{ - ActiveTheme, Icon, VirtualListScrollHandle, WindowExt, h_flex, v_flex, v_virtual_list, + ActiveTheme, Disableable, Icon, IconName, VirtualListScrollHandle, WindowExt, h_flex, v_flex, + v_virtual_list, }; use nostr::prelude::{EventId, Kind}; use signed_core::{RepoStatus, activity_subject}; -use signed_state::{ProfileStore, RepoStore}; +use signed_git::{format_patch_between, merge_base, patch_applies}; +use signed_state::{GitStore, ProfileStore, RepoStore}; use signed_ui::image_cache::{MAX_IMAGES, image_cache}; use signed_ui::{SegmentButton, UserAvatar, placeholder, status_badge}; use utils::relative_time; @@ -283,15 +287,55 @@ impl PullRequestsView { } } -/// State of the new pull request dialog, so the draft checkbox re-renders. +/// A patch series generated from a local repository, with the metadata +/// derived from it. +struct GeneratedPatch { + /// The `git format-patch` series (fills the patch textarea). + patch: String, + /// The merge base with the target branch, as hex. + merge_base: Option, +} + +/// State of the new pull request dialog, so the async generation, the +/// apply check and the draft checkbox re-render. #[derive(Default)] struct NewPullRequestDialogState { draft: bool, + /// The last generated patch series; its merge base is reused at submit + /// only while the patch textarea is unchanged. + generated: Option, + /// Result of the pre-publish applicability check against the app's + /// mirror clone of the target repository. + apply_check: Option>, + /// A patch generation is in flight. + generating: bool, + /// Error of the last generation attempt. + error: Option, +} + +impl NewPullRequestDialogState { + /// Text and whether it is good news, for the line under the patch field. + fn apply_check_message(&self) -> Option<(SharedString, bool)> { + match &self.apply_check { + Some(Ok(())) => Some(( + "Applies cleanly to the repository's default branch".into(), + true, + )), + Some(Err(error)) => Some(( + format!("May not apply cleanly to the repository's default branch: {error}").into(), + false, + )), + None => None, + } + } } /// Open the "new pull request" dialog: a title, an optional description, /// an optional branch name and a patch input that submit through -/// [`RepoStore::open_pull_request`] when confirmed. +/// [`RepoStore::open_pull_request`] when confirmed. The patch can either be +/// pasted, or generated from a local checkout: pick a repository, a source +/// and a target branch, and the app runs `git format-patch` itself and +/// checks the series against the app's mirror clone of the target. pub(super) fn open_new_pull_request_dialog( store: Entity, window: &mut Window, @@ -301,6 +345,9 @@ pub(super) fn open_new_pull_request_dialog( let description = cx.new(|cx| TextareaState::new(window, cx).placeholder("Describe the change...")); let branch = cx.new(|cx| InputState::new(window, cx).placeholder("Branch name (optional)")); + let repo_path = cx.new(|cx| InputState::new(window, cx).placeholder("Pick a local checkout…")); + let source = cx.new(|cx| InputState::new(window, cx).placeholder("Source branch")); + let target = cx.new(|cx| InputState::new(window, cx).placeholder("Target branch")); let patch = cx .new(|cx| TextareaState::new(window, cx).placeholder("Paste `git format-patch` output...")); let state = cx.new(|_| NewPullRequestDialogState::default()); @@ -309,21 +356,28 @@ pub(super) fn open_new_pull_request_dialog( let subject = subject.clone(); let description = description.clone(); let branch = branch.clone(); + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); let patch = patch.clone(); let store = store.clone(); let state = state.clone(); dialog - .width(px(520.)) + .width(px(560.)) .margin_top(px(50.)) .content(move |body, _window, cx| { + let generating = state.read(cx).generating; let draft = state.read(cx).draft; + let error = state.read(cx).error.clone(); + let apply_check = state.read(cx).apply_check_message(); body.child( DialogHeader::new() .child(DialogTitle::new().child("New pull request")) .child( - DialogDescription::new() - .child("Propose a change with the output of `git format-patch`."), + DialogDescription::new().child( + "Propose a change with the output of `git format-patch`.", + ), ), ) .child( @@ -339,6 +393,104 @@ pub(super) fn open_new_pull_request_dialog( .label("Description") .child(Textarea::new(&description).h(px(96.))), ) + .child( + field() + .label("Local repository") + .description( + "Generate the patch from a local checkout; leave empty to paste it", + ) + .child( + h_flex() + .gap_1() + .items_center() + .child( + div() + .flex_1() + .child(Input::new(&repo_path).disabled(true)), + ) + .child( + Button::new("choose-checkout") + .icon(IconName::FolderOpen) + .ghost() + .tooltip("Choose local checkout") + .on_click({ + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); + let patch = patch.clone(); + let branch = branch.clone(); + let state = state.clone(); + let store = store.clone(); + move |_ev, window, cx| { + choose_local_repo( + &repo_path, + &source, + &target, + &patch, + &branch, + &state, + &store, + window, + cx, + ); + } + }), + ) + .child( + Button::new("generate-patch") + .ghost() + .label("Generate") + .tooltip( + "Generate the patch from the local checkout", + ) + .loading(generating) + .disabled(generating) + .on_click({ + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); + let patch = patch.clone(); + let branch = branch.clone(); + let state = state.clone(); + let store = store.clone(); + move |_ev, window, cx| { + let path = + repo_path.read(cx).value().to_string(); + let source = + source.read(cx).value().to_string(); + let target = + target.read(cx).value().to_string(); + if !path.is_empty() + && !source.is_empty() + && !target.is_empty() + { + generate_patch( + &state, + &patch, + &branch, + path, + source, + target, + &store, + window, + cx, + ); + } + } + }), + ), + ), + ) + .child( + field() + .label("Source branch") + .child(Input::new(&source)), + ) + .child( + field() + .label("Target branch") + .child(Input::new(&target)), + ) .child( field() .label("Branch") @@ -346,9 +498,31 @@ pub(super) fn open_new_pull_request_dialog( .child(Input::new(&branch)), ) .child( - field() - .label("Patch") - .child(Textarea::new(&patch).h(px(160.))), + field().label("Patch").child( + v_flex() + .gap_1() + .child(Textarea::new(&patch).h(px(140.))) + .when_some(apply_check, |this, (message, ok)| { + this.child( + div() + .text_xs() + .text_color(if ok { + cx.theme().success + } else { + cx.theme().warning + }) + .child(message), + ) + }) + .when_some(error, |this, message| { + this.child( + div() + .text_xs() + .text_color(cx.theme().danger) + .child(message), + ) + }), + ), ) .child( field().child( @@ -370,15 +544,21 @@ pub(super) fn open_new_pull_request_dialog( .primary() .label("Create pull request") .tooltip("Create pull request") + .loading(generating) + .disabled(generating) .on_click({ let subject = subject.clone(); let description = description.clone(); let branch = branch.clone(); let patch = patch.clone(); + let repo_path = repo_path.clone(); let store = store.clone(); let state = state.clone(); move |_event, window, cx| { + if state.read(cx).generating { + return; + } let subject = subject.read(cx).value().to_string(); let description = description.read(cx).value().to_string(); let branch = branch.read(cx).value().to_string(); @@ -386,6 +566,21 @@ pub(super) fn open_new_pull_request_dialog( let subject = (!subject.is_empty()).then_some(subject); let branch = (!branch.is_empty()).then_some(branch); let draft = state.read(cx).draft; + // The generated merge base stays valid + // only while the patch is unchanged; an + // edited patch falls back to none. + let merge_base = state + .read(cx) + .generated + .as_ref() + .filter(|generated| generated.patch == patch) + .and_then(|generated| generated.merge_base.clone()); + // The checkout (when set) is where the + // tip commit is pushed from, so other + // clients can fetch it. + let repo_path = repo_path.read(cx).value().to_string(); + let push_from = (!repo_path.is_empty()) + .then(|| PathBuf::from(repo_path)); store.update(cx, |store, cx| { store.open_pull_request( @@ -394,6 +589,8 @@ pub(super) fn open_new_pull_request_dialog( branch, patch, draft, + merge_base, + push_from, cx, ); }); @@ -407,6 +604,188 @@ pub(super) fn open_new_pull_request_dialog( }); } +/// Prompt for a local checkout, fill the source/target defaults (the +/// checkout's current branch and the repository's announced HEAD) and +/// generate the patch series right away. +#[allow(clippy::too_many_arguments)] +fn choose_local_repo( + repo_path: &Entity, + source: &Entity, + target: &Entity, + patch: &Entity, + branch: &Entity, + state: &Entity, + store: &Entity, + window: &mut Window, + cx: &mut App, +) { + let handle = window.window_handle(); + let repo_path = repo_path.clone(); + let source = source.clone(); + let target = target.clone(); + let patch = patch.clone(); + let branch = branch.clone(); + let state = state.clone(); + let store = store.clone(); + // The announced HEAD branch is the natural target default. + let target_default = store.read(cx).head.clone().unwrap_or_default(); + + let prompt = cx.prompt_for_paths(PathPromptOptions { + files: false, + directories: true, + multiple: false, + prompt: Some("Choose local checkout".into()), + }); + + cx.spawn(async move |cx| { + if let Ok(Ok(Some(mut paths))) = prompt.await + && let Some(path) = paths.pop() + { + let path = path.to_string_lossy().to_string(); + + // The checkout's current branch is the source default; resolve + // it off the UI thread. + let current = cx + .background_executor() + .spawn({ + let path = path.clone(); + async move { + gix::open(Path::new(&path)) + .ok() + .and_then(|repo| signed_git::current_branch(&repo).ok().flatten()) + } + }) + .await; + + let _ = handle.update(cx, |_, window, cx| { + repo_path.update(cx, |input, cx| { + input.set_value(path.clone(), window, cx); + }); + source.update(cx, |input, cx| { + input.set_value(current.clone().unwrap_or_default(), window, cx); + }); + target.update(cx, |input, cx| { + input.set_value(target_default.clone(), window, cx); + }); + + if let Some(current) = current + && !current.is_empty() + && !target_default.is_empty() + { + generate_patch( + &state, + &patch, + &branch, + path, + current, + target_default, + &store, + window, + cx, + ); + } + }); + } + }) + .detach(); +} + +/// Generate the patch series `source..target` of the local checkout at +/// `repo_path`, fill the patch textarea and record the merge base and the +/// pre-publish applicability check in `state`. +#[allow(clippy::too_many_arguments)] +fn generate_patch( + state: &Entity, + patch_input: &Entity, + branch_input: &Entity, + repo_path: String, + source: String, + target: String, + store: &Entity, + window: &mut Window, + cx: &mut App, +) { + state.update(cx, |state, cx| { + state.generating = true; + state.error = None; + state.apply_check = None; + cx.notify(); + }); + + let cache = GitStore::global(cx).cache().clone(); + let (addr, clone_urls) = { + let store = store.read(cx); + ( + store.addr().clone(), + store + .announcement + .as_ref() + .map(|a| { + a.clone + .iter() + .map(ToString::to_string) + .collect::>() + }) + .unwrap_or_default(), + ) + }; + + let handle = window.window_handle(); + let state = state.clone(); + let patch_input = patch_input.clone(); + let branch_input = branch_input.clone(); + + let task = cx.spawn(async move |cx| { + // The branch-name tag defaults to the source branch; keep a copy + // for the UI update after the background generation moves it. + let source_label = source.clone(); + let generated = cx + .background_executor() + .spawn(async move { + let base = + merge_base(Path::new(&repo_path), &source, &target)?.ok_or_else(|| { + anyhow::anyhow!("{source} and {target} share no common ancestor") + })?; + let patch = format_patch_between(Path::new(&repo_path), &base, &source)?; + // Best-effort: does the series apply to the current default + // branch of the app's mirror clone of the target repository? + let check = cache + .ensure_clone(&addr, &clone_urls) + .ok() + .and_then(|repo| repo.workdir().map(|workdir| workdir.to_path_buf())) + .map(|workdir| patch_applies(&workdir, &patch).map_err(|e| e.to_string())); + Ok::<_, anyhow::Error>((patch, Some(base), check)) + }) + .await; + + let _ = handle.update(cx, |_, window, cx| match generated { + Ok((patch, merge_base, check)) => { + patch_input.update(cx, |input, cx| { + input.set_value(patch.clone(), window, cx); + }); + // The branch-name tag defaults to the source branch. + if branch_input.read(cx).value().is_empty() { + branch_input.update(cx, |input, cx| { + input.set_value(source_label.clone(), window, cx); + }); + } + state.update(cx, |state, cx| { + state.generating = false; + state.generated = Some(GeneratedPatch { patch, merge_base }); + state.apply_check = check; + cx.notify(); + }); + } + Err(error) => state.update(cx, |state, cx| { + state.generating = false; + state.error = Some(error.to_string().into()); + cx.notify(); + }), + }); + }); + task.detach(); +} + impl BasePanel for PullRequestsView { fn panel_name(&self) -> &'static str { "pull-requests" @@ -480,10 +859,33 @@ impl Render for PullRequestsView { let scroll_handle = self.scroll_handle.clone(); let view = cx.entity().clone(); + // Non-fatal warnings and errors of the last action (e.g. creating + // or updating a PR), shown as dismissible banners above the list. + let (last_error, last_warning) = { + let store = self.store.read(cx); + (store.last_error.clone(), store.last_warning.clone()) + }; + v_flex() .size_full() .image_cache(image_cache("pull-requests", MAX_IMAGES)) .child(self.render_header(cx)) + .when_some(last_warning, |this, warning| { + this.child(Alert::warning("pr-warning", warning).banner().on_close({ + let store = self.store.clone(); + move |_event, _window, cx| { + store.update(cx, |store, _| store.last_warning = None); + } + })) + }) + .when_some(last_error, |this, error| { + this.child(Alert::error("pr-error", error).banner().on_close({ + let store = self.store.clone(); + move |_event, _window, cx| { + store.update(cx, |store, _| store.last_error = None); + } + })) + }) .child( v_flex() .relative() diff --git a/docs/PLAN.md b/docs/PLAN.md deleted file mode 100644 index a57534b..0000000 --- a/docs/PLAN.md +++ /dev/null @@ -1,165 +0,0 @@ -# Plan - -Two work streams: - -1. **Fork support (display + navigation UI)** — show when a repository is a fork and let the user jump to the upstream repository. -2. **Pull request improvement** — bring PR creation/updating in line with the other NIP-34 clients (nak, ngit). - ---- - -## 1. Fork support - -### Background: what NIP-34 says about forks - -NIP-34 has no fork event kind — a fork is an ordinary kind-30617 announcement by another author (or the same author under a different `d`). Fork-ness is expressed by two tags: - -- **`u` tag** on the fork's announcement: - `["u", "30617::|", "", ""]`. - Including `u` means the author does **not** assert maintainership of the primary project (the fork is a *subordinate* of the upstream). -- **EUC** (`r` tag with `euc` marker): shared between the fork and its upstream (and other mirrors), so clients can group them. For a permanent fork, the EUC is the first commit after the fork point. - -### Current state - -- `Announcement::from_event` parses the `u` tag into an opaque string (`crates/signed_core/src/model.rs:212`; only the first value is kept, manually, because the SDK's `Nip34Tag` has no `Upstream` variant). -- `effective_maintainers` excludes the fork author (`model.rs:250`) — already correct per NIP-34. -- The About dialog shows the raw upstream string as a plain row (`crates/workspace/src/views/repo_detail/about.rs:68`). -- Nothing shows fork-ness in the repo list or the repo detail header, and there is no way to navigate to the upstream. - -### Goal - -- **Repo list card** (`crates/workspace/src/views/repo_list.rs::render_card`): show a "Forked from " badge instead of/in addition to the description, with a fork icon. -- **Repo detail header** (`crates/workspace/src/views/repo_detail/mod.rs::render_header`): show a "Forked from " text button near the repo name. -- **Clicking the upstream** opens the upstream repository as a center panel (same as clicking any repo card). - -### Design - -#### 1.1 Structured `Upstream` model (`signed_core`) - -Add a structured type and keep the manual parse: - -```rust -pub struct Upstream { - /// `30617::` (navigable) or a git URL (not navigable). - pub target: UpstreamTarget, - pub relay_hint: Option, - pub author: Option, -} - -pub enum UpstreamTarget { - /// Parseable via the SDK `Coordinate` (`30617::`). - Repo(RepoAddr), - /// Git https URL form: no NIP-34 announcement, not navigable. - GitUrl(Url), -} -``` - -- Change `Announcement.upstream: Option` to `Option`; parse all three `u` values (the SDK's `Nip34Tag::parse` is not usable here — keep the manual `tag.kind() == "u"` branch and extend it). -- `RepoAddr` is the SDK `Coordinate` (`crates/signed_core/src/addr.rs`), so `Coordinate::from_str` gives the upstream address directly; validate it is kind `30617`. -- Update `about.rs` (renders `upstream`), `effective_maintainers`, and the `model.rs` tests (`parses_upstream_tag`, `effective_maintainers_exclude_owner_for_subordinate_forks`). - -#### 1.2 Resolving the upstream announcement - -Opening a panel needs an `Announcement` (`RepoDetailView::new`), so resolve the upstream announcement before (or while) opening: - -1. **Lookup, no fetch**: the global `RepoListStore` holds every announcement in the local database (`crates/signed_state/src/repo_list.rs:51`). Look up the upstream `RepoAddr` there — covers the common case (upstream already browsed/known) with zero network. -2. **Miss → fetch, then open**: add a `Backend` method (e.g. `fetch_announcement(addr) -> Task>`) doing a one-shot query with `filters::announcement(addr)` (`crates/signed_core/src/filters.rs:21`), mirroring the bootstrap fetch in `RepoStore::subscribe_remote` (`crates/signed_state/src/repo.rs:196`). Show the upstream as a disabled/loading row until it resolves; on failure fall back to showing the raw address. -3. **Git-URL upstreams**: not navigable — render as plain text with a copy action (like `copy_row` in `signed_ui`), no panel. - -#### 1.3 Shared "open repo panel" helper - -The open-panel sequence is currently duplicated three times: - -- `crates/workspace/src/views/repo_list.rs:170` (`RepoListView::open_repo`) -- `crates/workspace/src/views/sidebar/mod.rs:155` (`SidebarPanel::open_repo`) -- `crates/workspace/src/views/sidebar/create_repo_dialog.rs:259` (`open_repo`) - -Extract one helper (e.g. `open_repo_panel(dock_area, announcement, window, cx)` in the `workspace` views layer) and reuse it from all three plus the new fork button, so the fork navigation behaves exactly like clicking a repo card. - -#### 1.4 Repo list badge - -In `render_card`, when `announcement.upstream` is set: - -- Resolve the upstream's display name via the `RepoListStore` lookup (1.2); fall back to the raw address string. -- Render a small "Forked from " line (fork icon + `text_xs` muted), replacing or joining the description line. Add a `git-fork.svg` asset to `crates/assets/assets/icons/` + a `CustomIconName::GitFork` variant (lucide's `git-fork`), or reuse `git-branch.svg` if an asset addition is undesirable. - -#### 1.5 Repo detail header - -In `render_header` (`crates/workspace/src/views/repo_detail/mod.rs:1231`), next to the repo name: - -- "Forked from " as a **text button** (`gpui_base::Button` or styled `div`), which calls the shared open-panel helper with the resolved upstream announcement. -- Keep the About dialog row in sync: make it the same clickable control (or at least the same resolved display name). -- Handle "upstream not in store yet": spawn the `fetch_announcement` task; button shows a subtle loading state; on success open the panel (needs `window`/`cx` — the task is spawned on the view, `apply_announcement`-style flow). - -#### 1.6 (Stretch) Fork grouping by EUC - -`RepoListStore` already has `euc` per announcement; add a "N forks" count on the detail header by scanning announcements sharing the same EUC, with a filter or navigation into the explore list. Not required for the first iteration. - -### Checklist - -- [ ] `signed_core`: `Upstream`/`UpstreamTarget` types + full `u`-tag parse; `Announcement.upstream` type change; tests. -- [ ] `Backend::fetch_announcement(addr)` one-shot fetch. -- [ ] Shared `open_repo_panel` helper; switch the three existing call sites. -- [ ] Repo list card fork badge (+ `git-fork.svg` asset if used). -- [ ] Repo detail header "Forked from" button + About row sync. -- [ ] Manual test: fork with coordinate upstream (navigates), fork with git-URL upstream (copy only), upstream announcement absent (fetch-then-open). - ---- - -## 2. Pull request improvement - -### Why - -Current PR creation (`RepoStore::open_pull_request`, `crates/signed_state/src/repo.rs:626`; dialog `crates/workspace/src/views/repo_detail/pull_requests.rs:288`) requires pasting `git format-patch` output, publishes no `merge-base`/`branch-name`, cannot update an existing PR, and advertises clone URLs the author usually cannot push to. Compared with nak (`pr send`/`pr update`/`pr merge`) and ngit (push-based PRs with merge-base inference), the gaps are: - -| Area | Today | Fix (phase) | -| --- | --- | --- | -| Patch generation | manual paste | generate from a local checkout (P2) | -| `merge-base` tag | always `None` | compute vs state HEAD (P1) | -| `branch-name` tag | never | send local branch name (P1) | -| PR updates (kind 1619) | not producible | author-only update flow (P1) | -| Update reader trusts any author | `latest_update` has no author filter | filter by PR author (P1) | -| `clone` URL truthfulness | repo mirrors (author can't push) | push tip to grasp first (P3) | -| Multi-commit series | one oversized event | NIP-10 chain / size-aware (P3) | -| Pre-publish validation | none | `git am --check` dry run (P2) | -| Draft on create | no | optional 1633 status (P1) | -| Merge provenance | plain 1631 | `merge-commit`/`applied-as-commits` (P4) | - -### Phase 1 — Correctness & interop (small, surgical) ✅ implemented - -1. **Compute and publish `merge-base`, `branch-name`, `r` EUC** in `open_pull_request`: - - Target tip = `RepoStore.head` ref from the state announcement (`refs`/`head`, `crates/signed_state/src/repo.rs:28-30`); add `signed_git::merge_base(repo, a, b)` (shell out like `apply_patch`). - - Fill the `GitPullRequest` builder's existing `merge_base`/`branch_name` fields (currently hardcoded `None`, `repo.rs:691-702`); pass the branch name and tip through from the dialog. - - Add the `r` EUC tag manually to the PR event (the SDK builder omits it; NIP-34 recommends it for subscription efficiency). -2. **Add `RepoStore::update_pull_request(root, new_tip, …)`** producing a kind-1619 event via the SDK `GitPullRequestUpdate` builder (`E`/`P`/`K` NIP-22 tags) plus a chained root-revision patch (`t root-revision`, `e` reply to the original root patch). Author-only, mirroring nak's `pr update`. Wire a button into `pull_request_detail.rs`. -3. **Fix `latest_update`** (`crates/workspace/src/views/repo_detail/pull_request_detail.rs:1125`): filter by the root PR's author (nak and ngit both restrict tip updates to the PR author). -4. **Draft toggle** in the new-PR dialog: publish a 1633 status right after the PR event (reuse `set_status`). - -**Status:** items 1 (partial — `branch-name` + `r` EUC done; `merge-base` remains `None` because the paste-based flow has no access to the author's git objects to compute a merge base; it becomes computable in Phase 2 when the patch is generated from a local checkout), 2, 3, 4 are implemented. - -### Phase 2 — UX: replace the paste - -5. **Local-repo picker** replaces the paste textarea (keep it as an advanced fallback): user picks a git checkout (or the app's `GitCache` mirror), source branch and target branch. The app then: - - resolves the tip (`git rev-parse`), - - computes `merge-base` vs the target tip, - - runs `format-patch base..tip --stdout` itself (add `signed_git::format_patch_between`, like nak), - - **dry-runs `git am --3way --check`** against the cached clone before publishing (`signed_git::apply_patch` infra, `crates/signed_git/src/lib.rs:176`), surfacing "does not apply" before anything hits the relays. - -### Phase 3 — Truthful clone URLs (interop) - -6. **Push before publishing**: add `signed_git::push_commit_ref(path, url, commit, ref)` and reuse the grasp-push infrastructure (`grasp_base_url`, `push_to_grasp_servers`, `crates/signed_state/src/backend.rs:1472,1500`) to push the tip to `refs/nostr/` on the announced grasp servers (nak's `gitPushCommitToGraspRefs`). On success the `clone` tag carries the real URL; on failure fall back to the current patch-event model with a warning. -7. **Size-aware publishing**: split multi-commit mboxes into a NIP-10-chained 1617 series (each < 60 KB per NIP-34) or go PR-only above that size; adopt ngit's patch→PR upgrade (new PR + close-status for the original patch). -8. Optional: GRASP-06 `/prs//.git` + kind-10317 user grasp-list fallback (ngit's server-selection cascade). Fork support (section 1) makes the fork's own grasp server a natural push target here. - -### Phase 4 — Merge provenance - -9. In `merge_pull_request` (`crates/signed_state/src/repo.rs:817`), publish the 1631 status with `merge-commit` (or `applied-as-commits`) and `q` tags so nak/ngit/GitWorkshop show merge provenance correctly. - -### Checklist - -- [x] P1: merge-base + branch-name + `r` EUC on creation (merge-base deferred to P2 — not computable from a pasted patch). -- [x] P1: `update_pull_request` (1619) + UI button; author check. -- [x] P1: `latest_update` author filter. -- [x] P1: draft toggle on create. -- [ ] P2: local checkout picker + generated patch + pre-publish apply check. -- [ ] P3: push tip to grasp, truthful `clone` tags, size-aware series. -- [ ] P4: merge status tags. diff --git a/docs/TODO.md b/docs/TODO.md index c5cbf8e..47df0cd 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -2,19 +2,24 @@ ## Fork support -- [x] Add UI for fork (see `PLAN.md` section 1): - - [x] Fork badge on repo list cards (`repo_list.rs::render_card`). - - [x] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog. - - [x] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper). +- [x] Fork badge on repo list cards (`repo_list.rs::render_card`). +- [x] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog. +- [x] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper). -## Pull request improvement (see `PLAN.md` section 2) +## Pull request improvement - [x] P1: `branch-name` tag + `r` EUC tag on PR creation; draft checkbox in the new-PR dialog. - [x] P1: `RepoStore::update_pull_request` (kind 1619 + root-revision patch) with an author-only "Update" button on the PR detail header. - [x] P1: `latest_update` filters by PR author. -- [ ] P2: local checkout picker + generated patch + pre-publish apply check (also enables `merge-base`). -- [ ] P3: push tip to grasp, truthful `clone` tags, size-aware patch series. -- [ ] P4: `merge-commit`/`applied-as-commits` tags on merge status. +- [x] P2: local checkout picker in the new-PR dialog (folder picker + source/target branches + Generate): `signed_git::{merge_base, format_patch_between, patch_applies}`; `merge-base` tag now published; best-effort apply check shown under the patch field. +- [x] P3: push tip to grasp servers under `refs/nostr/` before publishing (from the local checkout); multi-commit series published as NIP-10-chained 1617 events with a 60 KB per-patch cap; PR list shows dismissible error/warning banners (incl. push failures). +- [x] P4: merge status tags — `merge_pull_request` publishes 1631 with `applied-as-commits` + `r` per applied commit and `q`/`e`-reply tags per applied patch event. + +### Pull request follow-ups + +- [ ] GRASP-06 `/prs//.git` contributor endpoints + kind-10317 user grasp-list fallback. +- [ ] Merge button in the PR detail view (`merge_pull_request` is store-only today), then fetch-and-merge (`merge-commit`) when the push backend is guaranteed. +- [ ] Local-checkout generation for the update-PR dialog (currently paste-only). ## Performance: render path