From a56b0b88812534cfcd587061b838737b5a320753 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 3 Oct 2026 16:31:37 +0700 Subject: [PATCH] strip redundant doc comments from signed_core and signed_git --- crates/signed_core/src/addr.rs | 16 +-- crates/signed_core/src/deletions.rs | 23 +--- crates/signed_core/src/filters.rs | 39 +----- crates/signed_core/src/inbox.rs | 59 ++------ crates/signed_core/src/model.rs | 95 ++----------- crates/signed_core/src/state.rs | 9 +- crates/signed_core/src/status.rs | 10 +- crates/signed_git/src/cache.rs | 6 +- crates/signed_git/src/diff.rs | 27 +--- crates/signed_git/src/history.rs | 42 +----- crates/signed_git/src/nip34.rs | 24 ++-- crates/signed_git/src/patch.rs | 66 +++------ crates/signed_git/src/remote.rs | 52 ++----- crates/signed_git/src/repo.rs | 101 ++++---------- crates/signed_git/src/scan.rs | 5 +- crates/signed_git/src/tests.rs | 50 ------- crates/signed_git/src/worktree.rs | 50 ++----- crates/signed_nostr/src/signer.rs | 3 - crates/signed_nostr/src/update.rs | 2 - crates/signed_state/src/backend.rs | 80 +++-------- crates/signed_state/src/bootstrap.rs | 5 - crates/signed_state/src/checkouts.rs | 115 +++------------- crates/signed_state/src/git_store.rs | 6 - crates/signed_state/src/inbox.rs | 10 +- crates/signed_state/src/lib.rs | 11 -- crates/signed_state/src/local_repos.rs | 11 -- crates/signed_state/src/profile.rs | 18 +-- crates/signed_state/src/push.rs | 135 ++++++------------ crates/signed_state/src/refresh.rs | 6 +- crates/signed_state/src/repo.rs | 182 +++++-------------------- crates/signed_state/src/repos.rs | 38 ++---- 31 files changed, 265 insertions(+), 1031 deletions(-) diff --git a/crates/signed_core/src/addr.rs b/crates/signed_core/src/addr.rs index 0945259..a3d0194 100644 --- a/crates/signed_core/src/addr.rs +++ b/crates/signed_core/src/addr.rs @@ -4,9 +4,6 @@ use std::str::FromStr; use nostr::prelude::*; use serde::{Deserialize, Serialize}; -/// Address of a NIP-34 repository announcement, `30617::`. -/// -/// Wraps the Rust Nostr SDK's [`Coordinate`], which parses, formats and hashes this. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(transparent)] pub struct RepoAddr(Coordinate); @@ -16,7 +13,6 @@ impl RepoAddr { Self(Coordinate::new(Kind::GitRepoAnnouncement, owner).identifier(identifier)) } - /// NIP-34 `d` tag identifier for a user-visible repository name. pub fn identifier_from_name(name: &str) -> String { name.chars() .map(|c| { @@ -45,7 +41,6 @@ impl RepoAddr { &self.0 } - /// Latest announcement event for this repository. pub fn announcement_filter(&self) -> Filter { Filter::new() .kind(Kind::GitRepoAnnouncement) @@ -53,7 +48,6 @@ impl RepoAddr { .identifier(self.identifier()) } - /// Latest state event for this repository, carrying refs and HEAD. pub fn state_filter(&self) -> Filter { Filter::new() .kind(Kind::RepoState) @@ -61,21 +55,13 @@ impl RepoAddr { .identifier(self.identifier()) } - /// All NIP-34 activity addressed to this repository via its `#a` tag. - /// Covers issues, PRs, patches, statuses and kind-1111 comments. - /// The `a` tag is optional on status events per NIP-34. - /// Statuses published without it are not matched here. + // Statuses may omit the `a` tag per NIP-34; those are not matched here. pub fn activity_filter(&self) -> Filter { Filter::new() .kinds(crate::filters::ACTIVITY_KINDS) .coordinate(&self.0) } - /// Deletion events relevant to this repository. - /// - /// Requests authored by the repository owner. - /// - /// Requests addressed to the repository coordinate via its `#a` tag. pub fn deletion_filters(&self) -> Vec { vec![ Filter::new() diff --git a/crates/signed_core/src/deletions.rs b/crates/signed_core/src/deletions.rs index 1fbcd66..7fca784 100644 --- a/crates/signed_core/src/deletions.rs +++ b/crates/signed_core/src/deletions.rs @@ -2,25 +2,14 @@ use std::collections::HashSet; use nostr::prelude::*; -/// NIP-09 deletion requests and NIP-62 vanish requests, -/// built from the kind-5 and kind-62 events in the local database. -/// -/// Deleted events are hidden before they reach the UI. -/// -/// Pass any event through [`Deletions::is_deleted`] before showing it. pub struct Deletions { - /// `(deleted event id, expected author)` from `e` tags of kind-5 events. ids: HashSet<(EventId, PublicKey)>, - /// `(coordinate, expected author, cutoff)` from `a` tags of kind-5 events. - /// - /// All versions of the addressable event up to `cutoff` are deleted. + // All versions of the addressable event up to `cutoff` are deleted. coords: Vec<(Coordinate, PublicKey, Timestamp)>, - /// `(author, cutoff)` from kind-62 vanish requests. vanished: Vec<(PublicKey, Timestamp)>, } impl Deletions { - /// Build the deletion index from raw kind-5 and kind-62 events. pub fn from_events(events: impl IntoIterator) -> Self { let mut ids = HashSet::new(); let mut coords = Vec::new(); @@ -36,8 +25,8 @@ impl Deletions { .map(|c| (c, event.pubkey, event.created_at)), ); } else if event.kind == Kind::RequestToVanish { - // Client-side we can't verify which relay the request targeted. - // Any vanish request is then honored for the author's events. + // We can't verify which relay the request targeted, so any + // vanish request is honored for the author's events. vanished.push((event.pubkey, event.created_at)); } } @@ -49,10 +38,8 @@ impl Deletions { } } - /// Whether the event is covered by a valid deletion or vanish request. - /// A request is valid when its author matches the deleted event's author, per NIP-09. - /// - /// Addressable events are deleted up to the request's `created_at`. + // A request is valid when its author matches the deleted event's author, + // per NIP-09. Addressable events are deleted up to the request's `created_at`. pub fn is_deleted(&self, event: &Event) -> bool { if self .vanished diff --git a/crates/signed_core/src/filters.rs b/crates/signed_core/src/filters.rs index 01710e2..9b09a6a 100644 --- a/crates/signed_core/src/filters.rs +++ b/crates/signed_core/src/filters.rs @@ -2,7 +2,6 @@ use std::time::Duration; use nostr::prelude::*; -/// Kinds that make up the activity of a repository. pub const ACTIVITY_KINDS: [Kind; 9] = [ Kind::Comment, Kind::GitPatch, @@ -15,7 +14,6 @@ pub const ACTIVITY_KINDS: [Kind; 9] = [ Kind::GitStatusDraft, ]; -/// Kinds that notify a user when they tag them via their `p` tag. const NOTIFICATION_KINDS: [Kind; 8] = [ Kind::GitIssue, Kind::GitPullRequest, @@ -27,7 +25,6 @@ const NOTIFICATION_KINDS: [Kind; 8] = [ Kind::GitStatusDraft, ]; -/// Git root kinds that make a comment count as git activity. const GIT_ROOT_KINDS: [Kind; 4] = [ Kind::GitIssue, Kind::GitPatch, @@ -35,7 +32,6 @@ const GIT_ROOT_KINDS: [Kind; 4] = [ Kind::GitRepoAnnouncement, ]; -/// Whether `kind` carries repository data: announcements, states, activity and deletions. pub fn is_repo_kind(kind: Kind) -> bool { kind == Kind::GitRepoAnnouncement || kind == Kind::RepoState @@ -44,7 +40,6 @@ pub fn is_repo_kind(kind: Kind) -> bool { || ACTIVITY_KINDS.contains(&kind) } -/// Value of the first tag named `name` on `event`. fn tag_value<'a>(event: &'a Event, name: &str) -> Option<&'a str> { event .tags @@ -53,19 +48,15 @@ fn tag_value<'a>(event: &'a Event, name: &str) -> Option<&'a str> { .and_then(|tag| tag.content()) } -/// Kind named by the first tag `name` on `event`. fn tag_kind(event: &Event, name: &str) -> Option { tag_value(event, name)?.parse::().ok() } -/// Namespace for the repository-agnostic filter constructors. pub struct Filters; impl Filters { - /// How far back deletion requests are fetched and stored. const DELETIONS_LOOKBACK: Duration = Duration::from_secs(3 * 365 * 86_400); - /// Status events, kinds `1630..=1633`, referencing any of the given root events. pub fn statuses_for(roots: impl IntoIterator) -> Filter { Filter::new() .kinds([ @@ -77,17 +68,13 @@ impl Filters { .events(roots) } - /// A user's grasp list, kind `10317`. pub fn grasp_list(public_key: PublicKey) -> Filter { Filter::new() .kind(Kind::GitUserGraspList) .author(public_key) } - /// NIP-22 comments, kind `1111`, referencing any of the given root events. - /// The roots are issues, patches and PRs. - /// - /// Returns two filters, since combining `#E` and `#e` would AND the conditions. + // Two filters: combining `#E` and `#e` would AND the conditions. pub fn comments_for(roots: impl IntoIterator) -> Vec { let roots: Vec = roots.into_iter().map(|id| id.to_hex()).collect(); if roots.is_empty() { @@ -103,7 +90,6 @@ impl Filters { ] } - /// NIP-22 comments on our issues, patches and pull requests. fn notification_comments(me: PublicKey) -> Filter { Filter::new() .kind(Kind::Comment) @@ -111,8 +97,7 @@ impl Filters { .custom_tags(SingleLetterTag::UPPERCASE_K, ["1621", "1617", "1618"]) } - /// Activity directed at us: comments on our roots, and git events tagging us - /// via their lowercase `p` tag. `Filter::pubkey` sets that `p` tag. + // `Filter::pubkey` matches the git events' lowercase `p` tag. pub fn notifications(me: PublicKey) -> Vec { vec![ Self::notification_comments(me), @@ -120,37 +105,27 @@ impl Filters { ] } - /// Git activity authored by `me`, for "Continue where you left off". - /// - /// A comment on an unrelated kind matches too, so results must be filtered - /// through `GitEvent::is_git_activity` before display. + // A comment on an unrelated kind matches too, so results must be filtered + // through `GitEvent::is_git_activity` before display. pub fn authored_activity(me: PublicKey) -> Filter { Filter::new().kinds(ACTIVITY_KINDS).author(me) } - /// All repository announcements, for global discovery. pub fn all_announcements() -> Filter { Filter::new().kind(Kind::GitRepoAnnouncement) } - /// All repository state events, carrying each repository's refs and last push time. pub fn all_states() -> Filter { Filter::new().kind(Kind::RepoState) } - /// `now` minus [`Self::DELETIONS_LOOKBACK`]. - /// Quantized to whole days so identical filters hash the same. - /// - /// This lets the backend's sync dedup match identical filters. + // Quantized to whole days so identical filters hash the same. fn deletions_since() -> Timestamp { let now = Timestamp::now().as_secs(); Timestamp::from_secs(now - now % 86_400) - Self::DELETIONS_LOOKBACK } - /// All deletion-related events within [`Self::DELETIONS_LOOKBACK`]. - /// These are NIP-09 kind `5` and NIP-62 kind `62`. - /// - /// Deletion requests must be known before any other event is shown. + // Deletion requests must be known before any other event is shown. pub fn deletions() -> Filter { Filter::new() .kinds([Kind::EventDeletion, Kind::RequestToVanish]) @@ -158,13 +133,11 @@ impl Filters { } } -/// Whether a kind-1111 comment targets a git root, checked via its `K` tag. pub(crate) fn is_git_comment(event: &Event) -> bool { event.kind == Kind::Comment && tag_kind(event, "K").is_some_and(|kind| GIT_ROOT_KINDS.contains(&kind)) } -/// Whether a status event references a git root, checked via its `k` tag. pub(crate) fn is_git_status(event: &Event) -> bool { tag_kind(event, "k").is_some_and(|kind| GIT_ROOT_KINDS.contains(&kind)) } diff --git a/crates/signed_core/src/inbox.rs b/crates/signed_core/src/inbox.rs index 6f9cbaa..d3365d7 100644 --- a/crates/signed_core/src/inbox.rs +++ b/crates/signed_core/src/inbox.rs @@ -6,33 +6,21 @@ use serde::{Deserialize, Serialize}; use crate::{GitEvent, RepoAddr}; -/// Window before `now` that an advanced cutoff retreats to. const ADVANCE_WINDOW: Duration = Duration::from_secs(3 * 24 * 60 * 60); - -/// Window before `now` that a mark-all cutoff retreats to. const MARK_ALL_WINDOW: Duration = Duration::from_secs(10 * 24 * 60 * 60); -/// A thread of notification and own-activity events sharing one root. #[derive(Debug, Clone)] pub struct InboxItem { - /// The root issue, patch or pull request the events belong to. pub root: EventId, - /// The root event itself, when it is known locally. pub root_event: Option, - /// Repository the root belongs to, from the root's `a` tag. pub address: Option, - /// Notification events directed at the user, newest first. pub events: Vec, - /// The user's own events in the thread, newest first. pub own_events: Vec, - /// Unread event ids, oldest first. pub unread_ids: Vec, - /// Whether every notification event in the thread is archived. pub archived: bool, } impl InboxItem { - /// Title of the thread, read from its root issue/patch/PR when known. pub fn title(&self) -> String { self.root_event .as_ref() @@ -50,7 +38,6 @@ impl InboxItem { .map(|event| event.kind) } - /// Timestamp of the newest event in the thread. pub fn latest_activity(&self) -> Timestamp { self.root_event .as_ref() @@ -62,7 +49,6 @@ impl InboxItem { .unwrap_or_default() } - /// Up to `limit` events of the thread, oldest first. pub fn timeline(&self, limit: usize) -> Vec { let mut seen: HashSet = HashSet::new(); let mut events: Vec = Vec::new(); @@ -92,7 +78,6 @@ impl InboxItem { events } - /// Whether the thread has an unread event still visible in the inbox. pub fn is_unread(&self) -> bool { !self.archived && !self.unread_ids.is_empty() } @@ -112,7 +97,6 @@ impl InboxItem { } } -/// Resolves thread roots by following parent pointers through known events. pub struct ThreadResolver<'a, L: ?Sized> { lookup: &'a L, } @@ -125,16 +109,14 @@ where Self { lookup } } - /// Root issue, patch or pull request of a notification event. - /// - /// Returns `None` when the event is not git-related, or when its root is a - /// coordinate rather than an event. - /// - /// - issue (1621) / PR (1618): itself - /// - patch (1617): its `e` parent patch, else itself - /// - NIP-22 comment (1111): uppercase `E` root pointer - /// - PR update (1619): uppercase `E` - /// - statuses (1630-1633): NIP-10 root `e` + // Kind → root mapping: + // - issue (1621) / PR (1618): itself + // - patch (1617): its `e` parent patch, else itself + // - NIP-22 comment (1111): uppercase `E` root pointer + // - PR update (1619): uppercase `E` + // - statuses (1630-1633): NIP-10 root `e` + // Returns `None` when the event is not git-related, or when its root is a + // coordinate rather than an event. pub fn notification_root(&self, event: &Event) -> Option { match event.kind { Kind::GitIssue | Kind::GitPullRequest => Some(event.id), @@ -159,7 +141,7 @@ where } } - /// Follow NIP-10/NIP-22 parent pointers until a root item is reached. + // Follow NIP-10/NIP-22 parent pointers until a root item is reached. pub fn resolve_thread_root(&self, id: EventId) -> EventId { let mut seen = HashSet::new(); let mut root = id; @@ -184,7 +166,7 @@ where } } - /// Parent of a thread event, mirroring gitworkshop's `getParentId`. + // Mirrors gitworkshop's `getParentId`. fn parent_id(&self, event: &Event) -> Option { for marker in ["reply", "root"] { if let Some(id) = event @@ -217,7 +199,6 @@ where self.first_uppercase_e_id(event) } - /// NIP-10 root of an event: the `e` tag marked `root`, else the first `e` tag. fn nip10_root_id(&self, event: &Event) -> Option { event .tags @@ -226,12 +207,10 @@ where .or_else(|| self.first_e_id(event)) } - /// First `e` tag id, in document order. fn first_e_id(&self, event: &Event) -> Option { self.first_tag_id(event, "e") } - /// First uppercase `E` tag id, in document order. fn first_uppercase_e_id(&self, event: &Event) -> Option { self.first_tag_id(event, "E") } @@ -246,7 +225,6 @@ where }) } - /// Event id from a four-element `e` tag carrying `marker`. fn e_tag_with_marker(&self, tag: &Tag, marker: &str) -> Option { let slice = tag.as_slice(); if tag.kind() != "e" || slice.len() != 4 || slice[3] != marker { @@ -257,7 +235,6 @@ where } } -/// Group notification events and the user's own events into one item per thread. pub fn group( events: E, own: O, @@ -328,7 +305,8 @@ where items } -/// Read and archive state of the inbox, a high-water-mark model. +// High-water-mark model: events at or before the cutoff +// are covered without an entry in the id set. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct InboxReadState { #[serde(default)] @@ -342,31 +320,26 @@ pub struct InboxReadState { } impl InboxReadState { - /// Whether `event` is at or before the read cutoff, or marked read. pub fn is_read(&self, event: &Event) -> bool { event.created_at <= self.read_before || self.read_ids.contains(&event.id) } - /// Whether `event` is at or before the archived cutoff, or marked archived. pub fn is_archived(&self, event: &Event) -> bool { event.created_at <= self.archived_before || self.archived_ids.contains(&event.id) } - /// Mark one event read. Events at or before the cutoff are already read. pub fn mark_read(&mut self, event: &Event) { if event.created_at > self.read_before { self.read_ids.insert(event.id); } } - /// Mark one event archived. Events at or before the cutoff are already archived. pub fn mark_archived(&mut self, event: &Event) { if event.created_at > self.archived_before { self.archived_ids.insert(event.id); } } - /// Mark every non-self event read, anchoring the cutoff ten days back. pub fn mark_all_read(&mut self, all: &[Event], me: PublicKey, now: Timestamp) { let cutoff = now - MARK_ALL_WINDOW; self.read_before = cutoff; @@ -377,15 +350,14 @@ impl InboxReadState { .collect(); } - /// Advance the read cutoff to the newest point that keeps unread events - /// unread, then prune the id set. + // Advance the cutoff to the newest point that keeps unread events unread, + // then prune the id set. pub fn advance_read(&mut self, all: &[Event], me: PublicKey, now: Timestamp) { let cutoff = advance_cutoff(all, me, now, self.read_before, |event| self.is_read(event)); self.read_before = cutoff; prune_ids(&mut self.read_ids, all, cutoff); } - /// Advance the archived cutoff, mirroring [`Self::advance_read`]. pub fn advance_archived(&mut self, all: &[Event], me: PublicKey, now: Timestamp) { let cutoff = advance_cutoff(all, me, now, self.archived_before, |event| { self.is_archived(event) @@ -395,7 +367,6 @@ impl InboxReadState { } } -/// Newest cutoff that keeps unread events unread, never earlier than `current`. fn advance_cutoff( all: &[Event], me: PublicKey, @@ -422,7 +393,6 @@ where candidate.max(current) } -/// Drop ids whose event is unknown or now covered by the cutoff. fn prune_ids(ids: &mut HashSet, all: &[Event], cutoff: Timestamp) { let created_at: HashMap = all .iter() @@ -578,7 +548,6 @@ mod tests { assert_eq!(items[0].kind(), Some(Kind::GitIssue)); assert_eq!(items[0].title(), "Add retry logic"); assert_eq!(items[0].events, vec![reply.clone()]); - // The own events are kept apart from the notifications, newest first. assert_eq!(items[0].own_events, vec![mine.clone(), issue.clone()]); assert_eq!( items[0] diff --git a/crates/signed_core/src/model.rs b/crates/signed_core/src/model.rs index 479e6a2..8baafb4 100644 --- a/crates/signed_core/src/model.rs +++ b/crates/signed_core/src/model.rs @@ -4,42 +4,28 @@ use nostr::prelude::*; use crate::RepoAddr; -/// Parsed NIP-34 repository announcement, plain data ready for the UI. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Announcement { - /// ID of the announcement event itself. pub event_id: EventId, - /// Repository ID, the `d` tag. pub id: String, - /// Author of the announcement event. pub owner: PublicKey, - /// When the announcement was published, used for latest-wins resolution. pub created_at: Timestamp, pub name: Option, pub description: Option, - /// Webpage URLs for browsing. pub web: Vec, - /// URLs for `git clone`. pub clone: Vec, - /// Relays the repository monitors for patches and issues. pub relays: Vec, - /// Earliest unique commit ID, the `r` tag with `euc` marker. pub euc: Option, - /// Other recognized maintainers. pub maintainers: Vec, - /// Marks the repository as a subordinate fork of the upstream, per NIP-34. pub upstream: Option, - /// Hashtags labelling the repository, the `t` tags. pub hashtags: Vec, } -/// The `u` tag of a fork announcement, per NIP-34. +// The `u` tag of a fork announcement, per NIP-34. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Upstream { - /// Raw first value of the `u` tag, a coordinate or git URL. pub raw: String, - /// Upstream repository coordinate when the `u` tag names a NIP-34 repository. - /// `None` for the git-URL form. + // `None` for the git-URL form. pub addr: Option, } @@ -65,27 +51,20 @@ impl Upstream { } } -/// Tag accessors for NIP-34 git collaboration events. pub trait GitEvent { - /// Subject of an issue or pull request event. fn activity_subject(&self) -> String; - /// The `c` tag, the tip of the proposed branch, as hex. fn current_commit(&self) -> Option; - /// The `merge-base` tag, the base commit a pull request diffs against. fn merge_base(&self) -> Option; - /// The `clone` tag, URLs the tip commit can be fetched from. fn clone_urls(&self) -> Option>; - /// The `branch-name` tag, the proposed branch's name. fn branch_name(&self) -> Option; - /// Whether the event is git activity worth showing in the activity list. fn is_git_activity(&self) -> bool; - /// Whether the event carries an `e`/`E` tag pointing at `root`. + // Matches both NIP-10 lowercase `e` and NIP-22 uppercase `E` root pointers. fn references_root(&self, root: &EventId) -> bool; } @@ -196,7 +175,6 @@ impl GitEvent for &T { } } -/// A NIP-34 pull request root event, with its patch set and update history. pub struct PullRequest<'a>(pub &'a Event); impl<'a> PullRequest<'a> { @@ -204,22 +182,17 @@ impl<'a> PullRequest<'a> { Self(event) } - /// The patch set of the pull request. - /// - /// Returns an empty list when no patch event can be linked to the PR. + // The PR references its root patch via an `e` tag. pub fn patches(&self, patches: impl IntoIterator) -> Vec<&'a Event> { let pr = self.0; let patches: Vec<&'a Event> = patches.into_iter().collect(); - // The PR references its root patch via an `e` tag. if let Some(root_id) = pr.tags.event_ids().next() && let Some(root) = patches.iter().find(|patch| patch.id == root_id) { return Self::forward_series(root, &patches); } - // The PR has no `e` tag. - // The last patch of the set carries the tip commit in its `commit` or `r` tag. let Some(tip) = pr.current_commit() else { return Vec::new(); }; @@ -250,9 +223,7 @@ impl<'a> PullRequest<'a> { series } - /// The patch set of the pull request joined into one diff text. - /// - /// Falls back to the root event's content when no patch set is found. + // Falls back to the root event's content when no patch set is found. pub fn patch(&self, patches: impl IntoIterator) -> String { let pr = self.0; let patches: Vec<&'a Event> = patches.into_iter().collect(); @@ -267,10 +238,8 @@ impl<'a> PullRequest<'a> { .join("\n") } - /// The newest `GitPullRequestUpdate` revising `root`, from the root's own author. - /// - /// A pull request's tip is only mutable by its author per NIP-34, updates - /// from anyone else are ignored even if they are newer. + // A pull request's tip is only mutable by its author per NIP-34, + // updates from anyone else are ignored even if they are newer. pub fn latest_update( events: impl Iterator, root: &Event, @@ -287,7 +256,6 @@ impl<'a> PullRequest<'a> { .max_by_key(|e| e.created_at) } - /// The chain of patches replying to `root` via NIP-10 `e` tags, oldest first. fn forward_series(root: &'a Event, patches: &[&'a Event]) -> Vec<&'a Event> { let mut series = vec![root]; loop { @@ -309,9 +277,7 @@ impl<'a> PullRequest<'a> { series } - /// Whether `patch` produces `commit`, found via its `commit` or `r` tag. - /// - /// It lets clients find existing patches for a specific commit. + // Lets clients find existing patches for a specific commit. fn patch_produces_commit(patch: &Event, commit: &str) -> bool { patch .tags @@ -324,9 +290,7 @@ impl<'a> PullRequest<'a> { } impl Announcement { - /// The announced forks of `base` a new pull request compare can be built from. - /// - /// The user's own forks are listed first. + // The user's own forks are listed first. pub fn forks_in<'a>( announcements: &'a [Announcement], base: &RepoAddr, @@ -413,15 +377,11 @@ impl Announcement { RepoAddr::new(self.owner, self.id.clone()) } - /// The name of the repository, or a default if none is provided. pub fn name(&self) -> String { self.name.clone().unwrap_or("Untitled".into()) } - /// Whether this announcement is a fork of the repository at `base`. - /// Its `u` tag points at `base`, which also covers permanent forks whose EUC diverged. - /// - /// Or it shares `base`'s earliest unique commit and is not the base itself. + // The `u` tag pointing at `base` also covers permanent forks whose EUC diverged. pub fn is_fork_of(&self, base: &RepoAddr, base_euc: Option<&str>) -> bool { if self.addr() == *base { return false; @@ -432,17 +392,14 @@ impl Announcement { base_euc.is_some_and(|euc| self.euc.as_deref() == Some(euc)) } - /// The description of the repository, or a default if none is provided. pub fn description(&self) -> String { self.description .clone() .unwrap_or("No description".to_string()) } - /// The effective maintainers of this repository, - /// the announced `maintainers` plus the announcement author. - /// - /// A `u` tag that marks the repository as a subordinate fork excludes them, per NIP-34. + // A `u` tag marking the repository as a subordinate fork excludes the + // announcement author from the maintainers, per NIP-34. pub fn effective_maintainers(&self) -> Vec { let mut maintainers = self.maintainers.clone(); if self.upstream.is_none() && !maintainers.contains(&self.owner) { @@ -451,7 +408,6 @@ impl Announcement { maintainers } - /// The `git clone` URLs for this repository, deduplicated. pub fn clone_urls(&self) -> Vec { let mut seen = HashSet::new(); self.clone @@ -545,7 +501,6 @@ mod tests { let announcement = Announcement::from_event(&event).expect("parses"); - // An invalid URL keeps the whole clone tag from being parsed. assert!(announcement.clone.is_empty()); assert_eq!( announcement.relays, @@ -568,8 +523,6 @@ mod tests { let announcement = Announcement::from_event(&event).expect("parses"); let upstream = announcement.upstream.expect("parses the u tag"); - // The coordinate part resolves to a repository address. - // The raw value keeps the `|git-url` suffix. assert_eq!( upstream.addr, Some(RepoAddr::new( @@ -589,7 +542,6 @@ mod tests { #[test] fn is_fork_of_matches_the_u_tag_coordinate() { - // The base repository, announced by the `u` tag's owner. let base = RepoAddr::new( PublicKey::from_hex(MAINTAINER_HEX).expect("valid pubkey"), "upstream", @@ -597,26 +549,20 @@ mod tests { let event = announcement_event(&[&["d", "my-fork"], &["u", &base.to_string()]]); let fork = Announcement::from_event(&event).expect("parses"); - // A `u` tag pointing at the base address marks a fork. - // This holds even when neither side announces an EUC. assert!(fork.is_fork_of(&base, None)); } #[test] fn is_fork_of_matches_a_shared_euc() { let euc = "aa231c4c6a5777dc89b42207b499891a344add5c"; - // The base repo has no `u` tag. It announces the family EUC. let base_event = announcement_event(&[&["d", "upstream"], &["r", euc, "euc"]]); let base = Announcement::from_event(&base_event).expect("parses"); let base_addr = base.addr(); - // A fork with no `u` tag, a pure mirror or cross-hosted clone, shares the EUC. - // Clients of the family can then find it. let fork_event = announcement_event(&[&["d", "mirror"], &["r", euc, "euc"]]); let fork = Announcement::from_event(&fork_event).expect("parses"); assert!(fork.is_fork_of(&base_addr, base.euc.as_deref())); - // An unrelated repository with a different EUC is not a fork. let other_event = announcement_event(&[ &["d", "other"], &["r", "bb231c4c6a5777dc89b42207b499891a344add5c", "euc"], @@ -624,14 +570,11 @@ mod tests { let other = Announcement::from_event(&other_event).expect("parses"); assert!(!other.is_fork_of(&base_addr, base.euc.as_deref())); - // Without a base EUC there is nothing to compare against. assert!(!fork.is_fork_of(&base_addr, None)); } #[test] fn is_fork_of_matches_permanent_forks_with_a_diverged_euc() { - // A permanent fork re-announces its EUC, the first commit after the fork. - // Only the `u` tag still relates it to the base. let base = RepoAddr::new( PublicKey::from_hex(MAINTAINER_HEX).expect("valid pubkey"), "upstream", @@ -654,8 +597,6 @@ mod tests { let announcement = Announcement::from_event(&event).expect("parses"); let maintainers = announcement.effective_maintainers(); - // The owner asserts themselves as a maintainer of the primary project, per NIP-34. - // Announced co-maintainers are included too. assert_eq!(maintainers.len(), 2); assert!(maintainers.contains(&announcement.owner)); assert!(maintainers.contains(&PublicKey::from_hex(MAINTAINER_HEX).expect("valid pubkey"))); @@ -672,8 +613,6 @@ mod tests { let announcement = Announcement::from_event(&event).expect("parses"); let maintainers = announcement.effective_maintainers(); - // A `u` tag marks the repository as a subordinate fork. - // The author is then not a maintainer of the primary project, per NIP-34. assert!(!maintainers.contains(&announcement.owner)); assert_eq!( maintainers, @@ -698,8 +637,6 @@ mod tests { #[test] fn pull_request_patch_joins_the_whole_patch_set() { - // A PR references the root patch, per NIP-34. - // Later patches of the set reply to the previous one via NIP-10 `e` tags. let root = patch_event("patch-one", vec![], 100); let second = patch_event("patch-two", vec![Tag::event(root.id)], 200); let pr = pr_event("description", vec![Tag::event(root.id)]); @@ -733,8 +670,6 @@ mod tests { #[test] fn pull_request_patches_finds_the_set_via_the_tip_commit() { - // PRs without an `e` tag fall back to the patch producing the tip commit. - // Walk the reply chain backward to the root. let root = patch_event("patch-one", vec![], 100); let tip = "1111111111111111111111111111111111111111"; let last = patch_event( @@ -794,7 +729,6 @@ mod tests { created_at, ) }; - // An update revising a different PR must be ignored even though it is newer. let unrelated = signed_at( Kind::GitPullRequestUpdate, vec![Tag::parse(["E", OTHER_ROOT_HEX]).expect("valid tag")], @@ -822,8 +756,6 @@ mod tests { .finalize(&other) .expect("signed event"); - // The tip of a PR is only mutable by its author. - // A newer update from anyone else must not win. assert!(PullRequest::latest_update([&stranger, &root].into_iter(), &root).is_none()); } @@ -861,8 +793,6 @@ mod tests { PublicKey::from_hex(OWNER_KEYS[0]).expect("pubkey"), "upstream", ); - // Newest first, as RepoListStore keeps them. - // Unrelated repo, the user's fork with the shared EUC, another fork with a `u` tag. let all = vec![ owned_announcements( 2, @@ -936,7 +866,6 @@ mod tests { assert_eq!(forks.len(), 1); assert_eq!(forks[0].id, "mirror"); - // Without a base EUC only `u`-tag forks match. all.push( owned_announcements( 2, diff --git a/crates/signed_core/src/state.rs b/crates/signed_core/src/state.rs index fcf488f..16f444b 100644 --- a/crates/signed_core/src/state.rs +++ b/crates/signed_core/src/state.rs @@ -1,19 +1,13 @@ use nostr::prelude::*; -/// Refs and HEAD parsed from a kind `30618` repository state event. #[derive(Debug, Clone, PartialEq, Eq)] pub struct RepoState { - /// `(refname, commit-id)` pairs. pub refs: Vec<(String, String)>, - /// Branch pointed to by the `HEAD` tag, if any. pub head: Option, } impl RepoState { - /// Build a kind `30618` repository state event from refs and HEAD, - /// it is published as `ref: refs/heads/`. - /// - /// The `d` tag matches the repository id. + // The `d` tag matches the repository id. pub fn build(id: &str, refs: &[(String, String)], head: Option<&str>) -> EventBuilder { let mut tags: Vec = vec![Tag::identifier(id.to_owned())]; for (name, commit) in refs { @@ -27,7 +21,6 @@ impl RepoState { EventBuilder::new(Kind::RepoState, "").tags(tags) } - /// Parse a kind `30618` repository state event into refs and HEAD. pub fn parse(event: &Event) -> Self { let mut refs = Vec::new(); let mut head = None; diff --git a/crates/signed_core/src/status.rs b/crates/signed_core/src/status.rs index 1537465..091ed21 100644 --- a/crates/signed_core/src/status.rs +++ b/crates/signed_core/src/status.rs @@ -1,6 +1,5 @@ use nostr::prelude::*; -/// Status of a root patch, pull request or issue, kinds `1630..=1633`. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum RepoStatus { Open, @@ -29,9 +28,7 @@ impl RepoStatus { } } - /// Resolve the status of a root event per NIP-34. - /// - /// Defaults to [`RepoStatus::Open`]. + // Defaults to Open when no authorized status event exists. pub fn resolve<'a, I>( status_events: I, root_author: &PublicKey, @@ -50,9 +47,8 @@ impl RepoStatus { } } -/// NIP-10 and NIP-34 use the lowercase `e` tag. -/// -/// NIP-22 comments, kind `1111`, use the uppercase `E` tag for the thread root. +// NIP-10 and NIP-34 use the lowercase `e` tag; NIP-22 comments, kind `1111`, +// use the uppercase `E` tag for the thread root. pub fn references_root(event: &Event, root: &EventId) -> bool { let root = root.to_hex(); event diff --git a/crates/signed_git/src/cache.rs b/crates/signed_git/src/cache.rs index 106dd01..5cc04d2 100644 --- a/crates/signed_git/src/cache.rs +++ b/crates/signed_git/src/cache.rs @@ -5,7 +5,6 @@ use signed_core::{Announcement, RepoAddr}; use crate::repo::Repo; -/// On-disk cache of cloned repositories, keyed by owner pubkey / repo id. #[derive(Debug, Clone)] pub struct GitCache { root: PathBuf, @@ -36,7 +35,6 @@ impl GitCache { } } - /// Open the existing clone, fetching it first. pub fn ensure_clone>(&self, addr: &RepoAddr, clone_urls: &[U]) -> Result { let path = self.repo_path(addr); @@ -53,7 +51,8 @@ impl GitCache { Repo::clone(clone_urls, &path) } - /// Map an untrusted repository id or display name to a safe single path component. + // `id` is untrusted relay content: it must never escape the cache root as + // a single path component. pub fn sanitize_path_component(id: &str) -> String { let sanitized: String = id .chars() @@ -73,7 +72,6 @@ impl GitCache { sanitized } - /// The refs namespace of a fork's import in the target mirror. pub fn fork_namespace(announcement: &Announcement) -> String { format!( "{}/{}", diff --git a/crates/signed_git/src/diff.rs b/crates/signed_git/src/diff.rs index ed8a394..81e799a 100644 --- a/crates/signed_git/src/diff.rs +++ b/crates/signed_git/src/diff.rs @@ -5,7 +5,6 @@ use crate::repo::Repo; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum DiffLineKind { - /// An unchanged context line, present on both sides. Context, Addition, Deletion, @@ -14,21 +13,15 @@ pub enum DiffLineKind { #[derive(Debug, Clone)] pub struct DiffLine { pub kind: DiffLineKind, - /// 1-based line number in the old version, if the line exists there. pub old: Option, - /// 1-based line number in the new version, if the line exists there. pub new: Option, - /// Line content without the trailing newline. pub text: String, } -/// A hunk of a file diff, like `@@ -a,b +c,d @@`. #[derive(Debug, Clone)] pub struct DiffHunk { - /// 1-based start line in the old version. pub old_start: u32, pub old_lines: u32, - /// 1-based start line in the new version. pub new_start: u32, pub new_lines: u32, pub lines: Vec, @@ -45,18 +38,13 @@ pub enum DiffStatus { #[derive(Debug, Clone)] pub struct FileDiff { - /// Path of the file relative to the repo root. - /// - /// For renames and copies, this is the destination path. + // For renames and copies, the destination path. pub path: String, - /// Previous path, for renames and copies. pub old_path: Option, pub status: DiffStatus, - /// Number of added lines, 0 for binary files. pub insertions: usize, - /// Number of removed lines, 0 for binary files. pub deletions: usize, - /// True if either version is binary, then `hunks` is empty. + // Binary files have empty `hunks`. pub binary: bool, pub hunks: Vec, } @@ -67,9 +55,7 @@ pub struct CommitDiff { } impl Repo { - /// The changes of the commit `id`, short or full. - /// - /// Compared against its first parent, the empty tree for the root commit. + // Compared against the first parent; the empty tree for the root commit. pub fn commit_diff(&self, id: &str) -> Result { let commit_id = self.inner.rev_parse_single(id.as_bytes())?; let commit = commit_id.object()?.into_commit(); @@ -81,9 +67,7 @@ impl Repo { Self::tree_diff(self, old_tree.as_ref(), &new_tree) } - /// The changes between two commits, `base`..`tip`, like `git diff base tip`. - /// - /// Directories and submodules are skipped, files are sorted by path. + // Directories and submodules are skipped, files are sorted by path. pub fn range_diff(&self, base: &str, tip: &str) -> Result { let base_tree = self .inner @@ -119,7 +103,6 @@ impl Repo { for change in changes { let attached = Change::from_change_ref(change.to_ref(), &repo.inner, &repo.inner); - // Skip directory trees and submodule gitlinks, only files are listed. let (path, old_path, status) = match attached { Change::Addition { location, @@ -175,7 +158,6 @@ impl Repo { _ => continue, }; - // Always diff with the built-in algorithm. // External diff drivers would shell out, out of scope for a read-only viewer. let platform = attached.diff(&mut cache)?; platform @@ -224,7 +206,6 @@ impl Repo { } } -/// Collects the hunks of one blob diff while tracking per-line numbers. struct HunkCollector<'a> { hunks: &'a mut Vec, insertions: &'a mut usize, diff --git a/crates/signed_git/src/history.rs b/crates/signed_git/src/history.rs index 2669c4f..d26c916 100644 --- a/crates/signed_git/src/history.rs +++ b/crates/signed_git/src/history.rs @@ -5,33 +5,22 @@ use anyhow::Result; use crate::repo::Repo; -/// Metadata of a commit, as shown in the repository browser's file header. #[derive(Debug, Clone)] pub struct FileCommit { - /// Shortened commit id, 7+ hex chars, disambiguated if needed. pub id: String, - /// First line of the commit message. pub summary: String, - /// Rest of the commit message after the title. - /// - /// `None` for single-line commit messages. pub description: Option, pub author: String, - /// Author time, seconds since the Unix epoch. pub time: i64, } impl FileCommit { - /// A [`FileCommit`] with author, message title, body and shortened id. - /// - /// The diff panel fetches the full commit on demand. fn from_commit(commit: &gix::Commit<'_>) -> Result { Self::from_commit_with_description(commit, true) } - /// A [`FileCommit`] without the message body, for history lists that never display it. - /// - /// Skipping the body saves an allocation per listed commit. + // History lists never display the body, + // skipping it saves an allocation per listed commit. fn from_commit_summary(commit: &gix::Commit<'_>) -> Result { Self::from_commit_with_description(commit, false) } @@ -61,10 +50,7 @@ impl FileCommit { } impl Repo { - /// Newest commit touching each of `rels`, like `git log -1 -- ` per path. - /// `rels` are paths relative to the worktree. - /// - /// Paths without any commit, like untracked files, are absent from the result. + // Paths without any commit, like untracked files, are absent from the result. pub fn last_commits(&self, rels: &[PathBuf]) -> Result> { use gix::traverse::commit::simple::CommitTimeOrder; @@ -125,9 +111,6 @@ impl Repo { Ok(found) } - /// All commits reachable from `HEAD`, newest first, with author and summary. - /// - /// Returns an empty list for a repository without any commits yet. pub fn all_commits(&self) -> Result { use gix::traverse::commit::simple::CommitTimeOrder; @@ -159,7 +142,6 @@ impl Repo { Ok(CommitList { total, commits }) } - /// Commits in the range `base`..`tip`, newest first, like `git log base..tip`. pub fn commit_range(&self, base: &str, tip: &str) -> Result> { use gix::traverse::commit::simple::CommitTimeOrder; @@ -183,9 +165,7 @@ impl Repo { Ok(commits) } - /// The commit HEAD points to, like `git log -1`. - /// - /// `Ok(None)` for a repository without commits yet, an unborn HEAD. + // `Ok(None)` for an unborn HEAD. pub fn head_commit(&self) -> Result> { let Some(head) = self.inner.head_id().ok() else { return Ok(None); @@ -194,10 +174,7 @@ impl Repo { Ok(Some(FileCommit::from_commit(&commit)?)) } - /// Full metadata of the commit `id`, short or full. - /// Like [`Repo::head_commit`] for an arbitrary commit. - /// - /// `Ok(None)` when the id cannot be resolved. + // `Ok(None)` when the id cannot be resolved. pub fn commit(&self, id: &str) -> Result> { match self.inner.rev_parse_single(id.as_bytes()) { Ok(commit_id) => { @@ -209,16 +186,11 @@ impl Repo { } } -/// Cap on [`CommitList::commits`]. The virtual list renders a window at a time, -/// the tab badge shows the real count. -/// -/// A huge history is never fully materialized in memory. +// The virtual list renders a window at a time, the tab badge shows the real +// count: a huge history is never fully materialized in memory. pub const MAX_LISTED_COMMITS: usize = 20_000; -/// Commits reachable from `HEAD`, newest first, possibly capped. pub struct CommitList { - /// Number of commits reachable from HEAD. pub total: usize, - /// Newest commits, capped at [`MAX_LISTED_COMMITS`]. pub commits: Vec, } diff --git a/crates/signed_git/src/nip34.rs b/crates/signed_git/src/nip34.rs index 36aa032..d81caa2 100644 --- a/crates/signed_git/src/nip34.rs +++ b/crates/signed_git/src/nip34.rs @@ -7,11 +7,11 @@ use crate::repo::Repo; /// The kind of NIP-34 relationship a local repository has on disk. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Nip34Kind { - /// Bound to a NIP-34 coordinate, by `nak`'s `nip34.json` or `ngit`'s `nostr.repo`. + // By `nak`'s `nip34.json` or `ngit`'s `nostr.repo`. Initialized, - /// Cloned from a `nostr://` remote but never initialized locally. + // Cloned from a `nostr://` remote but never initialized locally. Cloned, - /// Nostr tooling touched the repository but no binding is recoverable. + // Nostr tooling touched the repository but no binding is recoverable. ToolingOnly, } @@ -22,7 +22,7 @@ pub struct GraspSignals { pub nostr_repo_config: bool, pub nostr_remote: bool, pub grasp_remote: bool, - /// `nip34_grasp_remote` is the `nak`-specific `nip34/grasp/` remote name. + // `nak`-specific `nip34/grasp/` remote name. pub nip34_grasp_remote: bool, pub nip34_state_refs: bool, pub nostr_cache: bool, @@ -41,7 +41,7 @@ impl GraspSignals { pub struct Nip34Binding { pub kind: Nip34Kind, pub signals: GraspSignals, - /// Coordinate owner and identifier, from `nip34.json` or `nostr.repo`. + // From `nip34.json` or `nostr.repo`. pub owner: Option, pub identifier: Option, pub grasp_urls: Vec, @@ -54,9 +54,7 @@ struct Nip34Json { } impl Repo { - /// What the repository's on-disk state says about its NIP-34 binding. - /// - /// `None` when no Nostr tooling left a marker. + // `None` when no Nostr tooling left a marker. pub fn nip34_binding(&self) -> Option { let common_dir = self.inner.common_dir().to_path_buf(); let workdir = self.inner.workdir().map(std::path::Path::to_path_buf); @@ -186,7 +184,6 @@ impl Repo { }) } - /// Record the repository's NIP-34 coordinate in its local `nostr.repo` config. pub fn set_nostr_repo(&self, naddr: &str) -> Result<()> { self.edit_local_config(|config| { config.set_raw_value("nostr.repo", naddr)?; @@ -194,8 +191,8 @@ impl Repo { }) } - /// Mirrors `nak`'s `IsGraspURL`: two path segments, a path of at least 65 bytes, - /// and a first segment that decodes as an `npub`. + // Mirrors `nak`'s `IsGraspURL`: two path segments, + // a path of at least 65 bytes, and a first segment that decodes as an `npub`. fn is_grasp_url(url: &str) -> bool { let Ok(parsed) = Url::parse(url) else { return false; @@ -233,8 +230,8 @@ impl Repo { Some((coordinate.public_key, identifier)) } - /// Handles a bare `naddr`, an `npub`, and the optional `[ssh-key-file@]`, - /// `[protocol/]` and `[relay/]` components. An `nip05` owner yields no binding. + // Handles a bare `naddr`, an `npub`, and the optional `[ssh-key-file@]`, + // `[protocol/]` and `[relay/]` components. An `nip05` owner yields no binding. fn parse_nostr_url(url: &str) -> Option<(PublicKey, String)> { let rest = url.strip_prefix("nostr://")?; @@ -252,7 +249,6 @@ impl Repo { parts.remove(0); } - // `[owner, (relay), identifier]`. if parts.len() < 2 { return None; } diff --git a/crates/signed_git/src/patch.rs b/crates/signed_git/src/patch.rs index 05a1216..2331924 100644 --- a/crates/signed_git/src/patch.rs +++ b/crates/signed_git/src/patch.rs @@ -9,14 +9,10 @@ use crate::diff::{CommitDiff, DiffHunk, DiffLine, DiffLineKind, DiffStatus, File use crate::history::FileCommit; use crate::repo::Repo; -/// Pure parsing of `git format-patch` output, no repository involved. pub struct PatchParser; impl PatchParser { - /// Split a `git format-patch` series into its individual patches, mbox messages. - /// - /// A single patch yields one element. - /// A malformed input yields one element covering it. + // A malformed input yields one element covering it. pub fn split_patch_series(patch: &str) -> Vec<&str> { Self::envelopes(patch) .into_iter() @@ -24,11 +20,8 @@ impl PatchParser { .collect() } - /// Parse `git format-patch` output, a single patch or a series. - /// - /// Backed by [`diffy::patch_set`], which implements git's extended diff format: - /// `diff --git` headers, rename and copy detection, binary detection, and - /// C-style quoted or octal-escaped paths. + // Backed by `diffy::patch_set`, which implements git's extended diff format: + // rename and copy detection, binary detection, quoted/escaped paths. pub fn patch_diffs(patch: &str) -> Result { if !patch.lines().any(|line| line.starts_with("diff --git ")) { return Ok(CommitDiff { files: Vec::new() }); @@ -43,9 +36,8 @@ impl PatchParser { Ok(CommitDiff { files }) } - /// Commits of a `git format-patch` output, a single patch or a series. - /// - /// Entries appear in patch order, oldest first as `git format-patch` produces them. + // Entries appear in patch order, oldest first as `git format-patch` + // produces them. pub fn patch_commits(patch: &str) -> Vec { Self::envelopes(patch) .into_iter() @@ -70,15 +62,11 @@ impl PatchParser { .collect() } - /// Splits a `git format-patch` mbox into its messages by their envelopes, - /// the one parser [`Self::split_patch_series`] and [`Self::patch_commits`] share. - /// - /// A malformed input yields one message covering the whole input. + // The one parser `split_patch_series` and `patch_commits` share. + // A malformed input yields one message covering the whole input. fn envelopes(patch: &str) -> Vec> { let mut messages: Vec> = Vec::new(); - // Byte offset of the current envelope line, its commit id, and its headers. let mut current: Option<(usize, &str, Vec<&str>)> = None; - // Headers run up to the blank line before the commit message. let mut headers_closed = false; let mut offset = 0usize; @@ -86,7 +74,6 @@ impl PatchParser { let line_start = offset; offset += line.len() + 1; - // A message starts at its `From ` envelope line. let is_envelope = line .strip_prefix("From ") .and_then(|rest| rest.split_whitespace().next()) @@ -132,7 +119,6 @@ impl PatchParser { } if messages.is_empty() { - // Not an mbox at all: one message covering the whole input. messages.push(Envelope { text: patch, id: "", @@ -150,9 +136,7 @@ impl PatchParser { } } - /// Strip the patch prefix from a `Subject:` header. - /// - /// Examples are `[PATCH]`, `[PATCH 1/2]` and `[RFC PATCH]`. + // Matches `[PATCH]`, `[PATCH 1/2]`, `[RFC PATCH]`, etc. fn strip_patch_prefix(subject: &str) -> String { let trimmed = subject.trim(); let Some(rest) = trimmed.strip_prefix('[') else { @@ -169,9 +153,9 @@ impl PatchParser { } fn file_diff(file: FilePatch<'_, str>) -> Result { - // The `---`/`+++` paths carry the `a/`/`b/` prefix, so the first path - // component is dropped, the same way `git apply -p1` does. - // Rename and copy paths come from their own headers, unprefixed. + // The `---`/`+++` paths carry the `a/`/`b/` prefix, dropped the same way + // `git apply -p1` does; rename and copy paths come from their own + // headers, unprefixed. let stripped; let operation = match file.operation() { operation @ (FileOperation::Rename { .. } | FileOperation::Copy { .. }) => operation, @@ -229,11 +213,8 @@ impl PatchParser { }) } - /// The [`DiffHunk`] of one parsed hunk, including the line number of every line. - /// - /// `diffy` reports only the hunk header ranges. The per-line numbers are - /// counted from them the way the header encodes them: context lines advance - /// both sides, deletions only the old, insertions only the new. + // `diffy` reports only the hunk header ranges; the per-line numbers are + // counted from them the way the header encodes them. fn hunk_diff(hunk: &Hunk<'_, str>) -> DiffHunk { let old_range = hunk.old_range(); let new_range = hunk.new_range(); @@ -285,22 +266,16 @@ impl PatchParser { } } - /// The content of a parsed line without its line ending. - /// - /// `diffy` keeps the trailing `\n`, the way `str::lines` splits it off. fn line_text(text: &str) -> String { let text = text.strip_suffix('\n').unwrap_or(text); text.strip_suffix('\r').unwrap_or(text).to_owned() } } -/// A `git format-patch` mbox message, split on its `From <40-hex> ` envelope. +// A `git format-patch` mbox message, split on its `From <40-hex> ` envelope. struct Envelope<'a> { - /// The whole message, envelope and diff. text: &'a str, - /// Commit id from the `From ` line. id: &'a str, - /// Header lines between the envelope and the commit message. headers: Vec<&'a str>, } @@ -315,11 +290,8 @@ impl Envelope<'_> { } impl Repo { - /// Apply a `git format-patch` patch or series with `git am`. - /// - /// Uses the git CLI because it handles the mbox format natively. - /// - /// TODO: replace with a pure-Rust implementation later without changing callers. + // The git CLI handles the mbox format natively. + // TODO: replace with a pure-Rust implementation later without changing callers. pub fn apply_patch(&self, patch: &str) -> Result<()> { let workdir = self .inner @@ -349,10 +321,8 @@ impl Repo { Ok(()) } - /// The `git format-patch` mbox series of `base..tip`, like `git format-patch --stdout`. - /// Fails when the range has no commits. - /// - /// The mbox is returned untrimmed. Trailing newlines are part of the format. + // Fails when the range has no commits. The mbox is returned untrimmed; + // trailing newlines are part of the format. pub fn format_patch_between(&self, base: &str, tip: &str) -> Result { let output = Repo::run_git( self.workdir_or_dot(), diff --git a/crates/signed_git/src/remote.rs b/crates/signed_git/src/remote.rs index 3c78ffe..7170444 100644 --- a/crates/signed_git/src/remote.rs +++ b/crates/signed_git/src/remote.rs @@ -9,7 +9,6 @@ use gix::progress::Discard; use crate::repo::Repo; impl Repo { - /// Fetch all configured refspecs from `origin`, plus the `refs/nostr/*` namespace. pub fn fetch(&self) -> Result<()> { let options = gix::remote::ref_map::Options { extra_refspecs: vec![ @@ -29,7 +28,6 @@ impl Repo { Ok(()) } - /// Push `commit` to `reference` at `url`. pub fn push_ref(&self, url: &str, commit: &str, reference: &str) -> Result<()> { let output = Self::run_git( self.workdir_or_dot(), @@ -46,7 +44,6 @@ impl Repo { Ok(()) } - /// Push the local `main` branch to a grasp server. pub fn push_main(&self, base_url: &str, owner: &str, repo_id: &str) -> Result<()> { self.push_refspecs( base_url, @@ -56,9 +53,6 @@ impl Repo { ) } - /// Push every local branch and tag to a grasp server. - /// - /// This mirrors an initialized repository's whole history. pub fn push_all(&self, base_url: &str, owner: &str, repo_id: &str) -> Result<()> { self.push_refspecs( base_url, @@ -93,12 +87,10 @@ impl Repo { Ok(()) } - /// Whether `url` advertises every ref in `expected` at the given commit. - /// - /// Extra advertised refs are ignored: the question is whether the data this - /// push wanted to land is already there, not whether the remote is an exact mirror. - /// This is the convergence probe for a push that lost the compare-and-swap race - /// to the grasp server's own background ref alignment. + // The convergence probe for a push that lost the compare-and-swap race to + // the grasp server's own background ref alignment. Extra advertised refs + // are ignored: the question is whether the pushed data is already there, + // not whether the remote is an exact mirror. pub fn remote_has_refs(&self, url: &str, expected: &[(String, String)]) -> Result { if expected.is_empty() { return Ok(true); @@ -106,9 +98,8 @@ impl Repo { let url = Self::transport_url(url); - // A URL-created remote has no configured fetch refspecs, and `ref_map` only - // keeps refs that match one. Match each expected ref by its exact name, - // like `git ls-remote ` would; ref maps never write to the repository. + // A URL-created remote has no configured fetch refspecs, and `ref_map` + // only keeps refs matching one; match each expected ref by exact name. let refspecs = expected .iter() .map(|(name, _)| { @@ -135,9 +126,8 @@ impl Repo { .ref_map(Discard, options) .with_context(|| format!("listing refs of {url} failed"))?; - // Peeled tag entries carry the tag object in their direct oid, so mapping - // each advertised ref to its direct oid matches `git ls-remote` while - // skipping the duplicated `^{}` lines. + // Peeled tag entries carry the tag object in their direct oid, matching + // `git ls-remote` while skipping the duplicated `^{}` lines. let advertised: HashMap = refs .remote_refs .iter() @@ -152,9 +142,6 @@ impl Repo { .all(|(name, oid)| advertised.get(name.as_str()) == Some(oid))) } - /// Add `origin` pointing at `url` when the repository has no remote yet. - /// - /// No-op if `origin` already exists. pub fn ensure_origin(&self, url: &str) -> Result<()> { if self.inner.find_remote("origin").is_ok() { return Ok(()); @@ -168,16 +155,12 @@ impl Repo { }) } - /// Point `origin` at `url`, replacing an existing remote, - /// used after a clone whose `origin` points at the cloned-from path. - /// - /// A working copy cloned from a local mirror is re-targeted at the grasp server. + // A working copy cloned from a local mirror is re-targeted at the + // grasp server; a pre-existing fetch refspec is left untouched. pub fn set_origin(&self, url: &str) -> Result<()> { let had_origin = self.inner.find_remote("origin").is_ok(); self.edit_local_config(|config| { - // Replaces the existing url, like `git remote set-url origin `. - // A pre-existing fetch refspec is left untouched. config.set_raw_value("remote.origin.url", url)?; if !had_origin { @@ -189,9 +172,6 @@ impl Repo { }) } - /// The URL of the `origin` remote. - /// - /// `None` when the repository has no `origin` yet. pub fn origin_url(&self) -> Result> { let Ok(remote) = self.inner.find_remote("origin") else { return Ok(None); @@ -202,10 +182,8 @@ impl Repo { .map(|url| url.to_string())) } - /// Fetch `refspec` from the first working URL in `urls`. - /// When no URL works, the last error is returned. - /// - /// Never touches the checked-out refs or the worktree. + // Never touches the checked-out refs or the worktree. The last error is + // returned when no URL works. pub fn fetch_refs>(&self, urls: &[U], refspec: &str) -> Result<()> { let refspec = gix::refspec::parse( gix::bstr::BStr::new(refspec), @@ -247,7 +225,6 @@ impl Repo { } } - /// Apply `edit` to the repository-local configuration and persist it. pub(crate) fn edit_local_config( &self, edit: impl FnOnce(&mut gix::config::File) -> Result<()>, @@ -265,7 +242,6 @@ impl Repo { match gix::config::File::from_path_no_includes(config_path, gix::config::Source::Local) { Ok(config) => config, - // A repository without a config file yet starts from scratch. Err(gix::config::file::init::from_paths::Error::Io { source, .. }) if source.kind() == std::io::ErrorKind::NotFound => { @@ -289,9 +265,6 @@ impl Repo { self.inner.workdir().unwrap_or_else(|| Path::new(".")) } - /// Run `git -C dir args`, disabling the terminal prompt and capturing stderr. - /// - /// `what` names the command in the spawn error. pub(crate) fn run_git(dir: &Path, args: &[&str], what: &str) -> Result { Self::git_command(dir) .args(args) @@ -300,7 +273,6 @@ impl Repo { .with_context(|| format!("failed to spawn `{what}`")) } - /// A `git -C dir` command with the terminal prompt disabled. pub(crate) fn git_command(dir: &Path) -> Command { let mut command = Command::new("git"); command.arg("-C").arg(dir).env("GIT_TERMINAL_PROMPT", "0"); diff --git a/crates/signed_git/src/repo.rs b/crates/signed_git/src/repo.rs index 29cbedc..fe6184f 100644 --- a/crates/signed_git/src/repo.rs +++ b/crates/signed_git/src/repo.rs @@ -2,46 +2,34 @@ use std::path::Path; use anyhow::{Context, Result}; -/// In-memory object cache for history walks, see [`Repo::open_cached`]. -/// -/// Without one, a walk re-decodes the same commit objects from the object database. -/// Sized generously: a walk can cover a large portion of the repository's history. +// History walks re-decode the same commit objects without one; sized +// generously, a walk can cover a large portion of the history. const OBJECT_CACHE_BYTES: usize = 64 * 1024 * 1024; -/// An opened git repository. -/// -/// One `open` per operation instead of every helper re-opening by path. +// One `open` per operation instead of every helper re-opening by path. pub struct Repo { pub(crate) inner: gix::Repository, } impl Repo { - /// Open the repository at `workdir`. pub fn open(workdir: &Path) -> Result { Ok(Self { inner: gix::open(workdir)?, }) } - /// Open the repository at `workdir`, `None` when the path is not one. pub fn try_open(workdir: &Path) -> Option { Self::open(workdir).ok() } - /// Open the repository at `workdir` with an in-memory object cache. - /// - /// Only history walks benefit from it, they re-decode the same commit - /// objects repeatedly. Single-object reads open the repository plain. + // Only history walks benefit from the object cache, they re-decode the + // same commit objects repeatedly; single-object reads open plain. pub fn open_cached(workdir: &Path) -> Result { let mut repo = gix::open(workdir)?; repo.object_cache_size_if_unset(OBJECT_CACHE_BYTES); Ok(Self { inner: repo }) } - /// Create a repository at `path` with an initial `main` branch. - /// Write a `README.md` from `name` and `description`, then create the initial commit. - /// - /// Returns the initial commit id. pub fn init(path: &Path, name: &str, description: &str) -> Result { use gix::refs::transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog}; @@ -53,8 +41,8 @@ impl Repo { let (signature, mut time_buf) = Self::repository_signature(); let signature = signature.to_ref(&mut time_buf); - // The initial branch is `main`, regardless of `init.defaultBranch` in - // the user's git configuration: point the unborn HEAD there. + // The initial branch is `main` regardless of `init.defaultBranch`: + // point the unborn HEAD there. let head = gix::refs::FullName::try_from("HEAD") .map_err(|e| anyhow::anyhow!("invalid ref name: {e}"))?; @@ -107,17 +95,15 @@ impl Repo { Vec::::new(), )?; - // Populate the index so the fresh repository is clean, - // as `git add` and `git commit` would leave it. + // Populate the index so the fresh repository is clean, as `git add` + // and `git commit` would leave it. let mut index = repo.index_from_tree(&tree)?; index.write(gix::index::write::Options::default())?; Ok(commit.to_string()) } - /// Clone into `path` from the first working URL in `clone_urls`. - /// - /// Unlike [`crate::GitCache::ensure_clone`], the clone is not kept in any cache. + // Not kept in any cache, unlike `GitCache::ensure_clone`. pub fn clone>(clone_urls: &[U], path: &Path) -> Result { if path.exists() { anyhow::bail!("destination {} already exists", path.display()); @@ -128,7 +114,7 @@ impl Repo { for url in clone_urls { match Self::clone_from(url.as_ref(), path) { Ok(repo) => { - // The initial clone uses the default refspecs. Also fetch the `refs/nostr/*` PR refs. + // The default-refspec clone misses the `refs/nostr/*` PR refs. repo.fetch().ok(); return Ok(repo); } @@ -159,39 +145,28 @@ impl Repo { &self.inner } - /// The worktree directory, `None` for a bare repository. pub fn workdir(&self) -> Option<&Path> { self.inner.workdir() } - /// The commit id HEAD points to. - /// - /// `None` when the repository has no commits yet, an unborn HEAD. + // `None` for an unborn HEAD. pub fn head(&self) -> Option { self.inner.head_id().ok().map(|id| id.to_string()) } - /// The merge base of two revisions, - /// revisions may be branch names, remote-tracking refs or commit ids. - /// - /// `Ok(None)` when the revisions share no common ancestor. - /// - /// Unresolvable revisions are errors. + // `Ok(None)` when the revisions share no common ancestor — a valid + // outcome for a proposal; unresolvable revisions are errors. pub fn merge_base(&self, a: &str, b: &str) -> Result> { let a = self.inner.rev_parse_single(a.as_bytes())?; let b = self.inner.rev_parse_single(b.as_bytes())?; match self.inner.merge_base(a, b) { Ok(id) => Ok(Some(id.to_string())), - // No common ancestor, a valid outcome for a proposal. Err(gix::repository::merge_base::Error::NotFound { .. }) => Ok(None), Err(e) => Err(e.into()), } } - /// The commits in `base..HEAD`, oldest first. - /// This is the order `git am` creates them. - /// - /// `HEAD` alone when `base` is `None`. + // Oldest first: the order `git am` creates them. pub fn commits_since(&self, base: Option<&str>) -> Result> { let head = match self.inner.head_id() { Ok(head) => head, @@ -218,16 +193,12 @@ impl Repo { commits.push(info?.id().to_string()); } - // Oldest first, like `git rev-list --reverse`, the order `git am` creates them. commits.reverse(); Ok(commits) } - /// The earliest unique commit of the repository. - /// Used as the NIP-34 announcement's `euc` marker. - /// - /// `None` for a repository without commits. + // The NIP-34 announcement's `euc` marker; `None` without commits. pub fn root_commit(&self) -> Result> { let Ok(head) = self.inner.head_id() else { return Ok(None); @@ -250,10 +221,6 @@ impl Repo { Ok(None) } - /// Full ref names under `prefix`, sorted lexicographically, like `git for-each-ref`. - /// `prefix` is a ref namespace like `refs/fork//`. - /// - /// Returns an empty list when nothing matches. pub fn refs_with_prefix(&self, prefix: &str) -> Result> { let pattern = prefix.trim_end_matches('/'); let mut names = Vec::new(); @@ -262,7 +229,7 @@ impl Repo { let reference = reference.map_err(|error| anyhow::anyhow!("{error}"))?; let name = String::from_utf8_lossy(reference.name().as_bstr()).into_owned(); - // Match the pattern itself and everything beneath it, like `git for-each-ref`. + // Match the pattern itself and everything beneath it. let under_pattern = name .strip_prefix(pattern) .is_some_and(|rest| rest.is_empty() || rest.starts_with('/')); @@ -277,8 +244,6 @@ impl Repo { Ok(names) } - /// Delete every ref under `prefix`. - /// `prefix` is a ref namespace like `refs/fork//`. pub fn delete_refs_with_prefix(&self, prefix: &str) -> Result<()> { use gix::refs::transaction::{Change, PreviousValue, RefEdit, RefLog}; @@ -308,8 +273,6 @@ impl Repo { Ok(()) } - /// Short name of the branch HEAD points to, - /// `None` when detached or unreadable, like `git branch --show-current`. pub fn current_branch(&self) -> Option { let head = self.inner.head().ok()?; let name = head.referent_name()?; @@ -320,7 +283,6 @@ impl Repo { self.inner.find_reference(name).is_ok() } - /// Short names of local branches, `refs/heads/*`, sorted alphabetically. pub fn branches(&self) -> Result> { let mut names = Vec::new(); for reference in self.inner.references()?.local_branches()? { @@ -331,7 +293,6 @@ impl Repo { Ok(names) } - /// Short names of tags, `refs/tags/*`, sorted alphabetically. pub fn tags(&self) -> Result> { let mut names = Vec::new(); for reference in self.inner.references()?.tags()? { @@ -342,9 +303,6 @@ impl Repo { Ok(names) } - /// Branch, tag and HEAD refs of the repository. - /// - /// Ready for a NIP-34 kind-30618 repository state announcement. pub fn ref_state(&self) -> Result { let mut refs = Vec::new(); @@ -376,9 +334,8 @@ impl Repo { Ok(RepoRefState { refs, head }) } - /// Fast-forward local branches that trail their remote-tracking counterpart. - /// - /// Returns whether any branch moved. + // Fast-forward only: local-only commits or diverged history must never + // be rewritten by a refresh. Returns whether any branch moved. pub fn fast_forward_branches(&self) -> Result { use gix::refs::transaction::{Change, LogChange, PreviousValue, RefEdit, RefLog}; @@ -400,7 +357,6 @@ impl Repo { }; let remote = format!("refs/remotes/origin/{branch}"); - // No remote-tracking counterpart means the remote lacks this branch. let Ok(mut remote_reference) = self.inner.find_reference(&remote) else { continue; }; @@ -424,8 +380,7 @@ impl Repo { continue; } - // Only fast-forward. - // Local-only commits or diverged history must never be rewritten by a refresh. + // Only fast-forward: the base must be the local tip. let Ok(base) = self.inner.merge_base(local_oid, remote_oid) else { continue; }; @@ -454,7 +409,6 @@ impl Repo { }; if current.as_deref() == Some(branch) { - // Merge so the checked-out worktree follows the branch. // Only proceed on a clean worktree, like `git merge --ff-only`. if self.is_dirty() { continue; @@ -483,10 +437,8 @@ impl Repo { Ok(moved) } - /// The identity written to reflogs and commits created by this crate itself. - /// - /// Like `git -c user.name=… -c user.email=…` per invocation: the repository works - /// without a global git identity, and `gix` runs no hooks and never signs. + // Like `git -c user.name=… -c user.email=…` per invocation: the repository + // works without a global git identity, and `gix` runs no hooks and never signs. pub(crate) fn repository_signature() -> (gix::actor::Signature, gix::date::parse::TimeBuf) { let seconds = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -502,10 +454,8 @@ impl Repo { (signature, gix::date::parse::TimeBuf::default()) } - /// Rewrite a grasp server URL to the https URL the git transport actually uses. - /// - /// GRASP servers announce `grasp:////` clone URLs. - /// The transport is git smart HTTP, so the scheme is rewritten for gix. + // GRASP servers announce `grasp://` clone URLs but the transport is git + // smart HTTP, so the scheme is rewritten for gix. pub(crate) fn transport_url(url: &str) -> String { url.strip_prefix("grasp://") .map(|rest| format!("https://{rest}")) @@ -513,11 +463,8 @@ impl Repo { } } -/// Branch, tag and HEAD refs of a repository. #[derive(Debug, Clone, PartialEq, Eq)] pub struct RepoRefState { - /// `(full refname, commit id)` pairs for heads and tags, sorted. pub refs: Vec<(String, String)>, - /// Short branch name HEAD points to, or `None` when detached. pub head: Option, } diff --git a/crates/signed_git/src/scan.rs b/crates/signed_git/src/scan.rs index 8bb3830..7ef09a2 100644 --- a/crates/signed_git/src/scan.rs +++ b/crates/signed_git/src/scan.rs @@ -5,18 +5,15 @@ use ignore::WalkBuilder; use crate::nip34::Nip34Binding; use crate::repo::Repo; -/// Caps nesting so pathological trees can't stall the scan. +// Caps nesting so pathological trees can't stall the scan. const SCAN_MAX_DEPTH: usize = 12; -/// A git repository discovered under a scan root. #[derive(Debug, Clone)] pub struct LocalRepo { pub path: PathBuf, - /// `None` for a plain repository. pub nip34: Option, } -/// Walk `root` recursively and collect the git repositories below it. pub fn find_git_repos(root: &Path) -> Vec { if !root.is_dir() { return Vec::new(); diff --git a/crates/signed_git/src/tests.rs b/crates/signed_git/src/tests.rs index e470ff6..fcb5f18 100644 --- a/crates/signed_git/src/tests.rs +++ b/crates/signed_git/src/tests.rs @@ -7,7 +7,6 @@ use super::*; fn blocks_parent_components() { assert_eq!(GitCache::sanitize_path_component(".."), "_"); assert_eq!(GitCache::sanitize_path_component("."), "_"); - // Separators are neutralized before the check, so these stay safe. assert_eq!(GitCache::sanitize_path_component("../.."), ".._.."); assert_eq!(GitCache::sanitize_path_component("a/../b"), "a_.._b"); } @@ -21,7 +20,6 @@ fn root_commit_reports_the_first_ancestor() { let root = repo.root_commit().expect("root").expect("commit"); assert_eq!(root.len(), 40); - // The root commit does not change when history grows. std::fs::write(dir.join("b.txt"), b"two").expect("write"); commit_all(&repo, "second"); assert_eq!( @@ -32,8 +30,6 @@ fn root_commit_reports_the_first_ancestor() { #[test] fn push_all_mirrors_branches_and_tags() { - // A bare server repository reachable via a `file://` URL. - // Mirrors a grasp server's `{base}/{owner}/{repo-id}.git` layout. let server = tempfile::tempdir().unwrap(); let server_repo = bare_server(server.path(), "npub1test", "my-repo"); @@ -82,12 +78,9 @@ fn remote_has_refs_reports_whether_pushed_refs_landed() { assert!(repo.remote_has_refs(&url, &expected).expect("probe")); - // A stale expectation - the exact race a retry resolves - is false. let stale = vec![("refs/heads/main".to_owned(), "0".repeat(40))]; assert!(!repo.remote_has_refs(&url, &stale).expect("probe")); - // Extra remote refs (e.g. a tag pushed later) do not invalidate the - // refs this push wanted to land. git_run(dir, &["tag", "v1.0"]); repo.push_all( &format!("file://{}", server.path().display()), @@ -137,7 +130,6 @@ fn repo_ref_state_lists_branches_tags_and_head() { assert_eq!(state.refs.len(), 3); } -/// Build a throwaway non-bare repository from `(rel, bytes)` file pairs. fn fixture(files: &[(&str, &[u8])]) -> (tempfile::TempDir, Repo) { let dir = tempfile::tempdir().expect("tempdir"); gix::init(&dir).expect("init"); @@ -152,8 +144,6 @@ fn fixture(files: &[(&str, &[u8])]) -> (tempfile::TempDir, Repo) { (dir, repo) } -/// Stage everything and create a commit with the git CLI. -/// Like [`Repo::apply_patch`], the crate already shells out to the CLI. fn commit_all(repo: &Repo, message: &str) { git_run(repo.workdir().expect("workdir"), &["add", "-A"]); git_run(repo.workdir().expect("workdir"), &["commit", "-m", message]); @@ -166,8 +156,6 @@ fn merge_base_finds_the_fork_point_and_reports_unrelated_history() { let initial = Repo::init(&path, "My Repo", "desc").expect("init"); let repo = Repo::open(&path).expect("open"); - // A feature branch and a mainline commit diverge from the initial commit. - // The initial commit is their merge base. git_run(&path, &["checkout", "-b", "feature"]); std::fs::write(path.join("feature.txt"), "feature\n").expect("write"); commit_all(&repo, "feature commit"); @@ -182,13 +170,11 @@ fn merge_base_finds_the_fork_point_and_reports_unrelated_history() { Some(initial.as_str()) ); - // An orphan branch shares no history with main, so `Ok(None)`. git_run(&path, &["checkout", "--orphan", "orphan"]); std::fs::write(path.join("orphan.txt"), "orphan\n").expect("write"); commit_all(&repo, "orphan commit"); assert_eq!(repo.merge_base("orphan", "main").expect("ok"), None); - // An unresolvable revision is an error, not a missing ancestor. assert!(repo.merge_base("orphan", "no-such-ref").is_err()); } @@ -213,7 +199,6 @@ fn split_patch_series_splits_real_multi_commit_mboxes() { 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); @@ -228,7 +213,6 @@ fn head_commit_and_commits_since_track_applied_commits() { let repo = Repo::open(&path).expect("open"); assert_eq!(repo.head().as_deref(), Some(initial.as_str())); - // No base given, `HEAD` alone. assert_eq!( repo.commits_since(None).expect("commits"), vec![initial.clone()] @@ -242,7 +226,6 @@ fn head_commit_and_commits_since_track_applied_commits() { commit_all(&repo, "second commit"); let second = repo.head().expect("on a branch"); - // Oldest first, like the order `git am` creates them. assert_eq!( repo.commits_since(Some(&initial)).expect("commits"), vec![first.clone(), second.clone()] @@ -255,10 +238,6 @@ fn head_commit_and_commits_since_track_applied_commits() { #[test] fn working_copy_cloned_from_the_mirror_matches_head_and_origin() { - // The mirror is a freshly initialized repository, standing in for - // the grasp server. Its `origin` points at the (fake) grasp server. - // `GitCache::ensure_clone` lazily clones from a URL shaped like this - // the first time a repository is opened. let dir = tempfile::tempdir().expect("tempdir"); let mirror = dir.path().join("mirror"); let commit = Repo::init(&mirror, "My Repo", "Does things.").expect("init"); @@ -267,9 +246,6 @@ fn working_copy_cloned_from_the_mirror_matches_head_and_origin() { .ensure_origin("https://gitnostr.com/npub1test/my-repo.git") .expect("origin"); - // The working copy is cloned from the mirror. - // It then shares the announced history exactly. - // `origin` is re-pointed at the grasp server instead of the mirror path. let destination = dir.path().join("folder").join("My_Repo"); std::fs::create_dir_all(destination.parent().unwrap()).expect("parent"); Repo::clone(&[format!("file://{}", mirror.display())], &destination).expect("clone"); @@ -289,7 +265,6 @@ fn working_copy_cloned_from_the_mirror_matches_head_and_origin() { #[test] fn fast_forward_branches_moves_the_mirror_and_keeps_local_work() { - // A bare server, like a grasp server's `{base}/{owner}/{repo}.git` layout. let dir = tempfile::tempdir().expect("tempdir"); bare_server(dir.path(), "npub1test", "repo"); @@ -301,7 +276,6 @@ fn fast_forward_branches_moves_the_mirror_and_keeps_local_work() { .push_all(&base_url, "npub1test", "repo") .expect("push"); - // A mirror clone, like the app's GitCache clones. let mirror = dir.path().join("mirror"); git_run( dir.path(), @@ -315,8 +289,6 @@ fn fast_forward_branches_moves_the_mirror_and_keeps_local_work() { let initial = git_in(&mirror, &["rev-parse", "HEAD"]).expect("initial"); let repo = Repo::open(&mirror).expect("open"); - // The owner pushes a new commit. - // The mirror fetches it, but its local `main` and worktree stay behind. std::fs::write(work.join("new.txt"), b"new\n").expect("write"); commit_all(&work_repo, "new commit"); work_repo @@ -330,8 +302,6 @@ fn fast_forward_branches_moves_the_mirror_and_keeps_local_work() { ); assert_ne!(remote, initial); - // Fast-forwarding catches the branch and its worktree up. - // The second call has nothing left to move. assert!(repo.fast_forward_branches().expect("ff")); assert_eq!( git_in(&mirror, &["rev-parse", "HEAD"]).expect("local"), @@ -340,7 +310,6 @@ fn fast_forward_branches_moves_the_mirror_and_keeps_local_work() { assert!(mirror.join("new.txt").is_file()); assert!(!repo.fast_forward_branches().expect("idle")); - // A branch with local commits of its own is never touched. git_run(&mirror, &["checkout", "-b", "wip"]); std::fs::write(mirror.join("wip.txt"), b"wip\n").expect("write"); commit_all(&repo, "local wip"); @@ -356,8 +325,6 @@ fn fast_forward_branches_moves_the_mirror_and_keeps_local_work() { fn fetch_repo_refs_imports_heads_under_a_prefix() { let dir = tempfile::tempdir().expect("tempdir"); - // A bare base server holding the initial commit. - // Like a grasp server's `{base}/{owner}/{repo-id}.git` layout. let base_server = bare_server(dir.path(), "npub1base", "base"); let (upstream_dir, upstream_repo) = fixture(&[("a.txt", b"one")]); @@ -380,8 +347,6 @@ fn fetch_repo_refs_imports_heads_under_a_prefix() { ); let mirror_repo = Repo::open(&mirror).expect("open"); - // The fork server has the same initial commit. - // It also carries a feature commit on its own `feature` branch. let fork_work = dir.path().join("fork-work"); git_run( dir.path(), @@ -402,8 +367,6 @@ fn fetch_repo_refs_imports_heads_under_a_prefix() { ) .expect("push"); - // Import the fork's heads into the mirror under a private prefix. - // The first dead URL is skipped, the second works. let dead = format!("file://{}/missing.git", dir.path().display()); mirror_repo .fetch_refs( @@ -418,7 +381,6 @@ fn fetch_repo_refs_imports_heads_under_a_prefix() { .expect("refs"), vec!["refs/fork/npub1fork/fork/feature"] ); - // Nothing leaked into the normal ref namespaces. assert_eq!( mirror_repo .refs_with_prefix("refs/heads/fork") @@ -426,8 +388,6 @@ fn fetch_repo_refs_imports_heads_under_a_prefix() { Vec::::new() ); - // The mirror can now range across both histories. - // The fork point is the shared initial commit, the proposal covers the fork commit. assert_eq!( mirror_repo .merge_base( @@ -455,7 +415,6 @@ fn fetch_repo_refs_imports_heads_under_a_prefix() { ); } -/// Run a git command in `dir` with a fixed test identity, asserting success. fn git_run(dir: &Path, args: &[&str]) -> std::process::Output { let output = Command::new("git") .current_dir(dir) @@ -471,7 +430,6 @@ fn git_run(dir: &Path, args: &[&str]) -> std::process::Output { output } -/// Create a bare `{base}/{owner}/{name}.git` repository, like a grasp server. fn bare_server(base: &Path, owner: &str, name: &str) -> PathBuf { let repo = base.join(owner).join(format!("{name}.git")); std::fs::create_dir_all(repo.parent().expect("parent")).expect("mkdir"); @@ -531,10 +489,6 @@ diff --git a/b.txt b/b.txt #[test] fn parses_real_format_patch_output() { - // Build a commit touching a mix of file kinds. - // Feed genuine `git format-patch` output through the parser. - // It covers quoted and octal-escaped paths. - // There are also a rename-free modification, an addition and a binary deletion. let (dir, repo) = fixture(&[ ("src/main.rs", b"fn main() {\n println!(\"one\");\n}\n"), ("my file.txt", b"hello\n"), @@ -570,12 +524,10 @@ fn parses_real_format_patch_output() { .unwrap_or_else(|| panic!("missing file {path:?}")) }; - // Space in the name makes git quote the path in the header. let file = by_path("my file.txt"); assert_eq!(file.status, DiffStatus::Modified); assert_eq!(file.insertions, 1); - // UTF-8 names are emitted as octal escapes. let file = by_path("\u{8bf4}\u{660e}.md"); assert_eq!(file.status, DiffStatus::Modified); assert_eq!(file.insertions, 1); @@ -590,8 +542,6 @@ fn parses_real_format_patch_output() { assert_eq!(file.status, DiffStatus::Added); assert_eq!(file.insertions, 1); - // A binary deletion emits no `---` or `+++` lines. - // Only the mode line and the `Binary files` marker remain. let file = by_path("img.png"); assert_eq!(file.status, DiffStatus::Deleted); assert!(file.binary); diff --git a/crates/signed_git/src/worktree.rs b/crates/signed_git/src/worktree.rs index bb566c4..3e3a350 100644 --- a/crates/signed_git/src/worktree.rs +++ b/crates/signed_git/src/worktree.rs @@ -8,19 +8,17 @@ use crate::history::FileCommit; use crate::repo::Repo; impl Repo { - /// Whether the worktree has uncommitted changes. - /// - /// Best-effort: any read failure is reported as clean. + // Best-effort: any read failure is reported as clean. pub fn is_dirty(&self) -> bool { - // Changes to tracked files, staged or not; untracked files are excluded. + // Tracked files, staged or not; untracked files are handled below. match self.inner.is_dirty() { Ok(true) => return true, Ok(false) => {} Err(_) => return false, } - // Untracked files surface as `DirectoryContents` items of the index-vs-worktree walk, - // tracked files only appear there when modified. + // Untracked files surface as `DirectoryContents` items of the + // index-vs-worktree walk; tracked files only appear there when modified. let Ok(platform) = self.inner.status(Discard) else { return false; }; @@ -41,9 +39,7 @@ impl Repo { false } - /// Commits in `base..branch`. - /// - /// Best-effort: 0 when the range cannot be computed. + // Best-effort: 0 when the range cannot be computed. pub fn commits_ahead(&self, base: &str, branch: &str) -> u32 { let (Some(base), Some(branch)) = (self.resolve_commit(base), self.resolve_commit(branch)) else { @@ -57,15 +53,13 @@ impl Repo { walk.filter_map(Result::ok).count().min(u32::MAX as usize) as u32 } - /// Resolve `rev` to a commit id, accepting full refs or the bare branch names - /// callers pass. `gix`'s revision parser already applies git's ref DWIM. + // Accepts full refs or the bare branch names callers pass; `gix`'s + // revision parser already applies git's ref DWIM. fn resolve_commit<'a>(&'a self, rev: &str) -> Option> { self.inner.rev_parse_single(rev.as_bytes()).ok() } - /// Relative paths of all entries in the worktree, files and directories. - /// - /// The `.git` directory is skipped. + // The `.git` directory is skipped. pub fn entries(&self) -> Result> { let workdir = self.inner.workdir().context("repository has no worktree")?; @@ -80,9 +74,7 @@ impl Repo { Ok(entries.into_iter().map(|(path, _)| path).collect()) } - /// Read a file from the worktree. - /// - /// Returns `Ok(None)` if the path is missing or not a regular file. + // `Ok(None)` when the path is missing or not a regular file. pub fn read(&self, rel: &Path) -> Result>> { let workdir = self.inner.workdir().context("repository has no worktree")?; let path = workdir.join(rel); @@ -95,9 +87,7 @@ impl Repo { } } - /// Find the README file in the repository root. - /// - /// Falls back to any other file whose name starts with `readme`. + // Falls back to any other file whose name starts with `readme`. pub fn find_readme(&self) -> Result> { let Some(workdir) = self.inner.workdir() else { return Ok(None); @@ -133,7 +123,6 @@ impl Repo { .and_then(|path| path.strip_prefix(workdir).ok().map(Path::to_path_buf))) } - /// Everything the browser needs to refresh after a branch or tag switch. pub fn snapshot(&self) -> Result { let readme_path = self.find_readme()?; let readme = match &readme_path { @@ -151,7 +140,7 @@ impl Repo { }) } - /// Check out the local branch `name`, HEAD stays attached to it. + // HEAD stays attached to the branch. pub fn checkout_branch(&self, name: &str) -> Result<()> { let full = format!("refs/heads/{name}"); @@ -175,7 +164,7 @@ impl Repo { Ok(()) } - /// Check out the tag `name`, HEAD becomes detached at the tagged commit. + // HEAD becomes detached at the tagged commit. pub fn checkout_tag(&self, name: &str) -> Result<()> { let full = format!("refs/tags/{name}"); @@ -207,8 +196,8 @@ impl Repo { let mut index = self.inner.index_from_tree(tree)?; - // Files the previous index tracked but `tree` no longer contains are removed, - // like git deleting files that vanish between branches. + // Files the previous index tracked but `tree` no longer contains are + // removed, like git deleting files that vanish between branches. if let Ok(previous) = self.inner.index_or_empty() { let keep: HashSet = index .entries() @@ -262,7 +251,6 @@ impl Repo { Ok(()) } - /// Point `HEAD` at `target` and record the switch in the reflog. fn move_head( &self, signature: gix::actor::SignatureRef<'_>, @@ -294,7 +282,6 @@ impl Repo { Ok(()) } - /// Relative paths of all entries below `dir`, relative to `root`. fn collect_entries(root: &Path, dir: &Path, out: &mut Vec<(PathBuf, bool)>) -> Result<()> { for entry in std::fs::read_dir(dir)? { let entry = entry?; @@ -315,20 +302,13 @@ impl Repo { } } -/// Everything the browser needs to refresh after a branch or tag switch. pub struct WorktreeSnapshot { - /// Relative paths of all worktree entries, directories first. pub entries: Vec, - /// README path relative to the worktree, if any. pub readme_path: Option, - /// Contents of the README, if any. pub readme: Option>, - /// Branch HEAD points to, `None` when detached, for example on a tag. + // `None` when detached, for example on a tag. pub current_branch: Option, - /// Commit HEAD points to, if any, see [`Repo::head_commit`]. pub head_commit: Option, - /// Short names of local branches, sorted alphabetically. pub branches: Vec, - /// Short names of tags, sorted alphabetically. pub tags: Vec, } diff --git a/crates/signed_nostr/src/signer.rs b/crates/signed_nostr/src/signer.rs index 8ebf43e..601a286 100644 --- a/crates/signed_nostr/src/signer.rs +++ b/crates/signed_nostr/src/signer.rs @@ -8,7 +8,6 @@ use nostr_connect::client::AuthUrlHandler; use nostr_sdk::error::Error as SignerError; use nostr_sdk::prelude::*; -/// A type-erased signer whose inner signer can be swapped in-place. #[derive(Clone, Debug)] pub struct UniversalSigner { inner: Arc>>, @@ -27,7 +26,6 @@ impl UniversalSigner { } } - /// Swap the inner signer in-place. All clones see the new signer. pub fn swap_inner(&self, new_signer: T) where T: AsyncGetPublicKey + AsyncSignEvent + AsyncNip44 + 'static, @@ -160,7 +158,6 @@ impl AsyncNip44 for UniversalSigner { } } -/// Opens the NIP-46 auth URL in the default browser. #[derive(Debug, Clone)] pub struct SignedAuthUrlHandler; diff --git a/crates/signed_nostr/src/update.rs b/crates/signed_nostr/src/update.rs index a7d1998..277f6c3 100644 --- a/crates/signed_nostr/src/update.rs +++ b/crates/signed_nostr/src/update.rs @@ -1,10 +1,8 @@ use nostr_sdk::prelude::*; -/// A lightweight change notification for the UI. #[derive(Debug, Clone)] pub struct Update { pub kind: Kind, - /// First `a` tag value of the event, if any, for example the repository coordinate. pub coordinate: Option, pub author: PublicKey, } diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 140203b..938a268 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -21,7 +21,6 @@ use crate::push::{GraspPush, PushOutcome, grasp_base_url, grasp_clone_url}; use crate::repos::RepoListStore; pub const USER_KEYRING: &str = "Signed Safe Storage"; -/// Timeout for NIP-46 signer responses. pub const NOSTR_CONNECT_TIMEOUT: u64 = 60; const PUMP_DEBOUNCE: Duration = Duration::from_millis(200); @@ -29,13 +28,9 @@ const PUMP_DEBOUNCE: Duration = Duration::from_millis(200); #[derive(Debug, Clone)] pub enum BackendEvent { SignerRequired, - /// The stored identity is NIP-49 encrypted key. PassphraseRequired, - /// The signer changed on login, logout or account switch. SignerChanged, - /// Kind-0 metadata arrived for these authors; re-read them from the store. ProfileUpdates(Vec), - /// Repository events arrived: announcements, states, activity and deletions. RepoUpdates(Vec), Synced, Error(String), @@ -55,7 +50,6 @@ pub struct Backend { signer: UniversalSigner, current_user: Option, inbox: Entity, - /// True when the stored credential is NIP-49 encrypted. passphrase_required: bool, pushing_repos: Entity>, } @@ -158,10 +152,6 @@ impl Backend { } } - /// Restore the saved session from the Keyring. - /// - /// - Emits [`BackendEvent::SignerRequired`] when no credential is stored. - /// - Emits [`BackendEvent::PassphraseRequired`] for a NIP-49 encrypted identity. fn restore_session(&mut self, cx: &mut Context) { if cfg!(target_arch = "wasm32") { cx.emit(BackendEvent::SignerRequired); @@ -226,7 +216,6 @@ impl Backend { task.detach(); } - /// Decrypt the NIP-49 keyring credential with the given passphrase. pub fn restore_with_passphrase( &mut self, password: &str, @@ -295,7 +284,6 @@ impl Backend { write.await?; this.update(cx, |this, cx| { - // Become the new identity so later publishes are signed with the new keys. this.signer.swap_inner(keys); this.current_user = Some(public_key); this.bootstrap_user(public_key, cx); @@ -352,9 +340,6 @@ impl Backend { }) } - /// Initialize a local clone with a `main` branch and a `README.md`. - /// - /// The task yields the announcement and the path of the working copy. pub fn create_repository( &mut self, name: &str, @@ -400,7 +385,6 @@ impl Backend { let servers = grasp_servers.clone(); let client = self.client.clone(); - // Initialize directly at the user's chosen destination. let destination = { let dir_name = GitCache::sanitize_path_component(&name); let dir_name = if dir_name.is_empty() { @@ -473,7 +457,6 @@ impl Backend { }) } - /// Publish an existing local repository to NIP-34. pub fn publish_local_repo( &mut self, path: PathBuf, @@ -533,7 +516,6 @@ impl Backend { let announcement = repository_announcement(&repo_id, &name, &description, &owner, &servers, euc); - // The state event is the push authorization. It must be accepted before the push below. let event = announce_repository_and_push( &this, &client, @@ -550,7 +532,6 @@ impl Backend { ) .await?; - // Point `origin` at the first grasp server so later pushes have a target. if let Some(base) = servers.first().and_then(grasp_base_url) { let url = format!("{base}/{owner}/{repo_id}.git"); let path = path.clone(); @@ -562,8 +543,8 @@ impl Backend { .await; } - // Record the ngit-compatible `nostr.repo` marker, - // so the next scan detects the repository instead of offering to publish it again. + // The ngit-compatible `nostr.repo` marker makes the next scan + // detect the repository instead of offering to publish it again. let coordinate = RepoAddr::new(event.pubkey, repo_id.clone()); match Nip19Coordinate::new(coordinate.into(), servers.clone()).to_bech32() { Ok(naddr) => { @@ -587,10 +568,8 @@ impl Backend { }) } - /// Re-push the repository's current refs to the grasp servers in its `relays` tag. - /// - /// Errors when no grasp server accepted the push, the outcome reports - /// which servers did when only some accepted it. + // Errors when no grasp server accepted the push; the outcome reports + // which servers did when only some accepted it. pub fn push_repository( &mut self, announcement: Announcement, @@ -600,10 +579,6 @@ impl Backend { self.push_repo_from(announcement, path, None, cx) } - /// Push the refs of a local checkout to the grasp servers in its `relays` tag. - /// The checkout is the working copy of the user's own repository. - /// - /// Publish a fresh state event, then push every branch and tag of the checkout. pub fn push_checkout( &mut self, announcement: Announcement, @@ -614,7 +589,6 @@ impl Backend { self.push_repo_from(announcement, checkout, announced_head, cx) } - /// Shared body of the mirror-based and checkout-based pushes. fn push_repo_from( &mut self, announcement: Announcement, @@ -640,8 +614,8 @@ impl Backend { let relays = announcement.relays.clone(); cx.spawn(async move |this, cx| { - // Held for the whole task. Runs on completion, on error and on - // cancellation alike, since dropping the task drops this guard. + // Runs on completion, on error and on cancellation alike; the + // guard would be dropped with the task if not held. let _guard = cx.on_drop(&this, { let addr = addr.clone(); move |backend, cx| { @@ -660,10 +634,8 @@ impl Backend { work.await? }; - // The state event announces the pushed refs. - // Keep the announced default branch in `HEAD` when it is among the pushed refs. - // - // Otherwise `HEAD` stays the checkout's current branch. + // Keep the announced default branch in `HEAD` when it is among + // the pushed refs; otherwise `HEAD` stays the checkout's branch. let heads: Vec<&str> = state .refs .iter() @@ -676,9 +648,8 @@ impl Backend { state.head = Some(head); } - // Grasp servers authorize a push by the state event they hold in purgatory. - // Stage the state event on each server's own relay, then push the git data, - // retrying transient purgatory denials. + // Grasp servers authorize a push by the state event they hold in + // purgatory: stage it on each server's relay before the git push. let refs = state.refs.clone(); let head = state.head.clone(); @@ -725,8 +696,6 @@ impl Backend { } // Fan the state out to the relays once a git server holds the objects. - // Staging already stored the event locally, publishing notifies - // the repository views and other relays and clients. if let Some(state_event) = &outcome.state_event && let Err(e) = client.send_event(state_event).broadcast().await { @@ -737,9 +706,6 @@ impl Backend { }) } - /// Delete the repository from nostr. - /// - /// Only the repository owner may delete it. pub fn delete_repository( &mut self, addr: RepoAddr, @@ -888,7 +854,6 @@ impl Backend { .detach(); } - /// Connect to a repository's announced relays, its NIP-34 `relays` tag. pub fn connect_repo_relays( &mut self, relays: Vec, @@ -926,7 +891,6 @@ impl Backend { .detach(); } - /// Sync filters through the SDK's NIP-65 gossip targeting. pub fn sync_auto(&mut self, filters: Vec, cx: &mut Context) { let client = self.client.clone(); @@ -958,7 +922,6 @@ impl Backend { .detach(); } - /// Sync several bootstrap filters in order, within a single task. pub fn sync_bootstraps(&mut self, filters: Vec, cx: &mut Context) { let client = self.client.clone(); @@ -997,10 +960,8 @@ impl Backend { .detach(); } - /// Publish a NIP-09 deletion for each of `events`, best-effort. - /// - /// Each target gets its own deletion event: a relay rejecting or - /// dropping one does not affect the others. + // Each target gets its own deletion event: a relay rejecting or dropping + // one does not affect the others. fn retract_events(&mut self, events: &[Event], cx: &mut Context) { let pusher = GraspPush::new(self.client.clone(), self.signer.clone()); @@ -1017,7 +978,6 @@ impl Backend { } } -/// The announcement of one repository, as published to the relays. fn repository_announcement( repo_id: &str, name: &str, @@ -1041,7 +1001,10 @@ fn repository_announcement( } } -/// Announce a repository, stage the announcement on each grasp server's relay +// Announce the repository, then stage the state event and push: the state +// event is the push authorization, so it must be accepted before the push. +// Connect to all servers first — the nostr client queues events until each +// relay is connected. #[allow(clippy::too_many_arguments)] async fn announce_repository_and_push( backend: &WeakEntity, @@ -1057,12 +1020,10 @@ async fn announce_repository_and_push( cx: &mut AsyncApp, push: impl Fn(&Path, &str, &str, &str) -> Result<(), Error> + Send + 'static, ) -> Result { - // The nostr client queues events until each relay is connected. for url in servers { client.add_relay(url).and_connect().await.ok(); } - // The state event is the push authorization. It must be accepted before the push below. let event = { let builder = announcement.into_event_builder(); let event = builder.finalize_async(signer).await?; @@ -1102,8 +1063,8 @@ async fn announce_repository_and_push( }; if outcome.accepted() == 0 { - // The announcement is already published. Retract it so the - // repository is not left announced without content. + // Retract the announcement so the repository is not left announced + // without content. backend .update(cx, |backend, cx| { backend.retract_events(std::slice::from_ref(&event), cx); @@ -1118,8 +1079,6 @@ async fn announce_repository_and_push( } // Fan the state out to the relays once a git server holds the objects. - // Staging already stored the event locally, publishing makes it - // visible to the other relays and clients. if let Some(state_event) = &outcome.state_event && let Err(e) = client.send_event(state_event).broadcast().await { @@ -1129,14 +1088,12 @@ async fn announce_repository_and_push( Ok(event) } -/// A relay event the backend routes to a store group. enum UpdateEvent { Profile(PublicKey), Repo(Update), } impl UpdateEvent { - /// Await the next relay event from the notification stream. async fn next( notifications: &mut (impl futures::Stream + Unpin), seen: &mut HashSet, @@ -1167,7 +1124,6 @@ impl UpdateEvent { } } -/// Split a stored bunker credential into the plain URI and the session key. fn extract_master_key(credential: &str) -> (&str, Keys) { match credential.split_once("master=") { Some((base, nsec)) => { diff --git a/crates/signed_state/src/bootstrap.rs b/crates/signed_state/src/bootstrap.rs index 906ac84..eb6c360 100644 --- a/crates/signed_state/src/bootstrap.rs +++ b/crates/signed_state/src/bootstrap.rs @@ -7,12 +7,9 @@ use nostr_sdk::client::SyncSummary; use nostr_sdk::prelude::*; use signed_core::Filters; -/// Relays connected at startup, before any user-specific relay config is known. pub const BOOTSTRAP_RELAYS: [&str; 2] = ["wss://relay.ditto.pub", "wss://index.ngit.dev"]; -/// Relays used to index the user's NIP-65 relay list. pub const INDEXER_RELAYS: [&str; 2] = ["wss://indexer.coracle.social", "wss://user.kindpag.es"]; -/// Add and connect the startup relays. async fn ensure_bootstrap_relays(client: &Client) -> Result<(), Error> { for url in BOOTSTRAP_RELAYS { client.add_relay(url).and_connect().await?; @@ -65,7 +62,6 @@ pub(crate) async fn sync_bootstrap_only( Ok(output.value) } -/// The `g` tag servers of one kind-10317 grasp list event, in tag order. fn grasp_list_servers(event: &Event) -> Vec { event .tags @@ -76,7 +72,6 @@ fn grasp_list_servers(event: &Event) -> Vec { .collect() } -/// Grasp servers of the newest kind-10317 grasp list among `events`. fn latest_grasp_list_servers(events: Vec) -> Vec { events .into_iter() diff --git a/crates/signed_state/src/checkouts.rs b/crates/signed_state/src/checkouts.rs index d102197..1d2b827 100644 --- a/crates/signed_state/src/checkouts.rs +++ b/crates/signed_state/src/checkouts.rs @@ -18,11 +18,8 @@ use crate::repos::RepoListStore; const REFRESH_DEBOUNCE: Duration = Duration::from_millis(300); -/// How often the statuses are recomputed against the local refs. const LOCAL_POLL: Duration = Duration::from_secs(2); -/// How often a full pass refreshes the remotes while any repository panel is open. const STATUS_POLL: Duration = Duration::from_secs(15); -/// Remote refresh interval for the `ready to push` badges of the user's own repositories. const PUSH_POLL: Duration = Duration::from_secs(60); const MAX_STATUS_CHECKOUTS: usize = 8; @@ -31,63 +28,37 @@ struct GlobalCheckoutsStore(Entity); impl Global for GlobalCheckoutsStore {} -/// One associated local checkout of a repository. -/// -/// Carries the git facts needed to suggest a pull request. #[derive(Debug, Clone, PartialEq, Eq)] pub struct CheckoutStatus { pub path: PathBuf, - /// The branch checked out. A detached checkout is idle and yields no status. + // A detached checkout is idle and yields no status. pub branch: String, - /// Commit the branch points at, for tip-based PR dedupe. + // For tip-based PR dedupe. pub head: String, - /// What the branch is compared against. - /// - /// It is `refs/remotes/origin/`, else `origin/HEAD` for new branches. + // `refs/remotes/origin/`, else `origin/HEAD` for new branches. pub base: String, - /// Commits in `base..branch`. - /// - /// Zero-ahead checkouts are dropped, so this is always above zero. + // Zero-ahead checkouts are dropped, so always above zero. pub ahead: u32, } -/// A remembered record, with the address already parsed. struct Remembered { path: PathBuf, addr: RepoAddr, last_used: u64, } -/// Global store of local-checkout associations and per-checkout statuses. pub struct CheckoutsStore { - /// Checkout paths per announced repository. by_repo: HashMap>, - /// Ready-to-contribute statuses of the requested repositories. - /// - /// Those are the repository detail panels currently open. statuses: HashMap>, - /// Repositories whose statuses are recomputed on every input change. - /// - /// Those are the repository detail panels currently open. status_requested: HashSet, - /// Repositories whose `ready to push` statuses are recomputed on the same cycle. - /// - /// The sidebar rows of the user's own repositories and their detail panels. push_requested: HashSet, - /// The ready-to-push statuses of the requested own repositories. push_statuses: HashMap>, - /// Last announced head branch per requested repository. - /// - /// A recompute defaults the base the same way. requested_head: HashMap>, refresh: RefreshGate, - /// True while the timer between a scheduled refresh and its run is pending. debounce_pending: bool, local_pending: bool, - /// When the last full pass (with a remote refresh) completed. - /// - /// The local pass runs a full pass again once this is older than the - /// reconciliation cadence, so remote moves still land. + // The local pass runs a full pass again once this is older than the + // reconciliation cadence, so remote moves still land. last_full_sync: Option, _subscriptions: Vec, } @@ -189,17 +160,10 @@ impl CheckoutsStore { }); } - /// The associated checkouts of `addr`, freshest first. - /// - /// Empty when none are known or the resolution has not run yet. pub fn associations_of(&self, addr: &RepoAddr) -> Vec { self.by_repo.get(addr).cloned().unwrap_or_default() } - /// Ask for the `ready to contribute` statuses of `addr` to stay current. - /// Called while the repository's detail panel is open. - /// - /// `announced_head` is the announced HEAD branch, used to default the base. pub fn request_statuses( &mut self, addr: &RepoAddr, @@ -213,26 +177,18 @@ impl CheckoutsStore { self.refresh(cx); } - /// The ready-to-contribute statuses of `addr`. - /// - /// Empty while none are known or nothing is ahead. pub fn ready_statuses_of(&self, addr: &RepoAddr) -> Vec { self.statuses.get(addr).cloned().unwrap_or_default() } - /// Ask for the `ready to push` statuses of `addr` to stay current. pub fn request_push_statuses(&mut self, addr: &RepoAddr, cx: &mut Context) { self.push_requested.insert(addr.clone()); self.refresh(cx); } - /// The checkout at `path` was just pushed to the remote. - /// - /// Its ready-to-push status is obsolete. Drop it from the cached statuses - /// and notify observers right away, so the sidebar badge and the push - /// banner update immediately instead of waiting for the next background - /// pass, which re-scans and re-fetches the remote. The debounced refresh - /// reconciles the remaining checkouts of the repository afterwards. + // Drop the stale ready-to-push status and notify observers right away, so + // the sidebar badge updates immediately instead of waiting for the next + // background pass. The debounced refresh reconciles the remaining checkouts. pub fn checkout_pushed(&mut self, addr: &RepoAddr, path: &Path, cx: &mut Context) { let mut removed = false; @@ -255,9 +211,6 @@ impl CheckoutsStore { self.request_push_statuses(addr, cx); } - /// The ready-to-push statuses of `addr`. - /// - /// Empty while none are known or nothing is unpushed. pub fn push_statuses_of(&self, addr: &RepoAddr) -> Vec { self.push_statuses.get(addr).cloned().unwrap_or_default() } @@ -269,7 +222,6 @@ impl CheckoutsStore { .unwrap_or(0) } - /// Re-resolve the associations and the requested statuses. pub fn refresh(&mut self, cx: &mut Context) { if self.debounce_pending || self.refresh.request() != RefreshRequest::Schedule { return; @@ -284,7 +236,6 @@ impl CheckoutsStore { .detach(); } - /// One full resolve and apply cycle, the debounced entry point. fn run_refresh(&mut self, cx: &mut Context) { self.debounce_pending = false; self.refresh.begin(); @@ -325,14 +276,11 @@ impl CheckoutsStore { let poll = !self.status_requested.is_empty() || !self.push_requested.is_empty(); let work = cx.background_spawn(async move { - // Read the git facts of every scanned repository off the main thread. - // - // The facts are the origin URL and the root commit, both CLI reads. let mut facts: Vec<(PathBuf, Option, Option)> = Vec::new(); for scanned in scanned.iter() { let path = &scanned.path; - // The browser's mirror clones share the announce URLs and EUCs. They are not user checkouts. + // The browser's mirror clones are not user checkouts. if cache_root .as_ref() .is_some_and(|root| path.starts_with(root)) @@ -353,7 +301,6 @@ impl CheckoutsStore { let associations = CheckoutsStore::resolve_associations(&remembered, &facts, announcements.iter()); - // Missing directories are stale records, drop them. let associations: HashMap> = associations .into_iter() .map(|(addr, paths)| (addr, paths.into_iter().filter(|p| p.is_dir()).collect())) @@ -369,7 +316,6 @@ impl CheckoutsStore { let (associations, statuses, push_statuses) = match work.await { Ok(results) => results, Err(_) => { - // Git reads are best-effort, keep the last results. return this.update(cx, |this, cx| { this.refresh.abort(); if poll { @@ -388,8 +334,7 @@ impl CheckoutsStore { this.statuses = statuses; this.push_statuses = push_statuses; - // Notify only when something actually changed, so observers - // skip the no-op heartbeats. + // Notify only when something actually changed. if associations_changed || statuses_changed || push_statuses_changed { cx.notify(); } @@ -402,9 +347,6 @@ impl CheckoutsStore { this.update(cx, |this, cx| this.refresh(cx))?; } - // Restart the fast local pass so the freshly resolved - // associations drive it. The pass itself decides when the next - // full pass runs. this.update(cx, |this, cx| { if poll { this.schedule_local_pass(cx); @@ -416,7 +358,6 @@ impl CheckoutsStore { .detach(); } - /// Schedule the fast local status pass, unless one is already pending. fn schedule_local_pass(&mut self, cx: &mut Context) { if self.local_pending { return; @@ -433,20 +374,17 @@ impl CheckoutsStore { .detach(); } - /// The fast local status pass. fn local_tick(&mut self, cx: &mut Context) { - // Nothing watched: the chain idles out until a new request restarts it. + // Nothing watched: the pass idles until a new request restarts it. if self.status_requested.is_empty() && self.push_requested.is_empty() { return; } - // A full pass or a fresh request covers this tick, skip it. if self.refresh.running() || self.debounce_pending { self.schedule_local_pass(cx); return; } - // Open panels get the faster remote cadence. let cadence = if self.status_requested.is_empty() { PUSH_POLL } else { @@ -467,7 +405,6 @@ impl CheckoutsStore { self.schedule_local_pass(cx); } - /// Recompute the requested statuses against the tracking refs only. fn run_local_statuses(&mut self, cx: &mut Context) { let associations = self.by_repo.clone(); @@ -492,13 +429,12 @@ impl CheckoutsStore { let task: gpui::Task> = cx.spawn(async move |this, cx| { let Ok((statuses, push_statuses)) = work.await else { - // Git reads are best-effort, keep the last results. return Ok(()); }; this.update(cx, |this, cx| { - // A full pass or a fresh request will apply fresher data - // (the tracking refs move only when a full pass fetches). + // The tracking refs move only when a full pass fetches; a full + // pass or a fresh request will apply fresher data. if this.refresh.running() || this.debounce_pending { return; } @@ -593,7 +529,7 @@ impl CheckoutsStore { }) } - /// The `ready to push` status of one checkout of the user's own repository. + // Never fetches the checked-out refs; reads the tracking refs as-is. fn checkout_push_status(path: &Path, fetch: bool) -> Option { let repo = Repo::try_open(path)?; if repo.is_dirty() { @@ -611,8 +547,8 @@ impl CheckoutsStore { let remote = format!("refs/remotes/origin/{branch}"); - // A branch never fetched or pushed yet compares against the remote HEAD. - // The remote HEAD is the fork point in practice. + // A branch never fetched or pushed yet compares against the remote + // HEAD, the fork point in practice. let base = if repo.ref_exists(&remote) { remote } else if repo.ref_exists("refs/remotes/origin/HEAD") { @@ -632,7 +568,6 @@ impl CheckoutsStore { }) } - /// Compute the requested statuses against the checkout paths of `associations`. fn compute_statuses( associations: &HashMap>, requested: &[(RepoAddr, Option)], @@ -716,7 +651,6 @@ mod tests { #[test] fn same_repo_url_ignores_the_transport_scheme() { - // grasp announce vs https origin, with and without `.git`. assert!(same_repo_url( "grasp://relay.ngit.dev/npub1test/repo", "https://relay.ngit.dev/npub1test/repo.git" @@ -725,7 +659,6 @@ mod tests { "ws://localhost:8080/npub1test/repo", "http://localhost:8080/npub1test/repo" )); - // The port and the path matter. assert!(!same_repo_url( "wss://localhost:8081/npub1test/repo", "wss://localhost:8080/npub1test/repo" @@ -734,7 +667,6 @@ mod tests { "wss://host/npub1test/repo", "wss://host/npub1other/repo" )); - // Unparseable URLs compare literally. assert!(same_repo_url("/local/path", "/local/path")); assert!(!same_repo_url("/local/path", "/local/other")); } @@ -762,7 +694,6 @@ mod tests { run(&["commit", "-m", message]); }; - // A feature branch ahead of main, ready to contribute. run(&["checkout", "-b", "feature"]); std::fs::write(path.join("feature.txt"), "x\n").expect("write"); commit("feature work"); @@ -772,22 +703,16 @@ mod tests { assert_eq!(status.ahead, 1); assert_eq!(status.head.len(), 40); - // Dirty worktrees are never suggested. std::fs::write(path.join("uncommitted.txt"), "y\n").expect("write"); assert!(CheckoutsStore::checkout_status(&path, Some("main")).is_none()); run(&["checkout", "--", "."]); - // Even on main, nothing to propose. run(&["checkout", "main"]); assert_eq!(CheckoutsStore::checkout_status(&path, Some("main")), None); } #[test] fn checkout_push_status_counts_unpushed_commits_only() { - // The `grasp remote` is a plain repository the checkout clones from. - // Its origin URL is a local path, so the whole cycle runs offline. - // Git refuses pushes to a checked-out branch by default. - // Act like a grasp server and allow them. let dir = tempfile::tempdir().expect("tempdir"); let remote = dir.path().join("remote"); Repo::init(&remote, "My Repo", "").expect("init"); @@ -827,7 +752,6 @@ mod tests { // A fresh clone has nothing to push. assert_eq!(CheckoutsStore::checkout_push_status(&checkout, true), None); - // One local commit, ready to push, counted against the remote. std::fs::write(checkout.join("work.txt"), "x\n").expect("write"); run(&["add", "-A"]); run(&["commit", "-m", "local work"]); @@ -837,17 +761,12 @@ mod tests { assert_eq!(status.ahead, 1); assert_eq!(status.head.len(), 40); - // The local-only pass reads the tracking refs, no fetch needed: - // a commit lands locally long before the remote is reconciled. let local = CheckoutsStore::checkout_push_status(&checkout, false).expect("local status"); assert_eq!(local.ahead, 1); - // After the push the same commit is on the remote, idle again. run(&["push", "origin", "main"]); assert_eq!(CheckoutsStore::checkout_push_status(&checkout, true), None); - // A commit made by someone else on the remote must not count as local work. - // It is behind, not ahead. let remote_run = |args: &[&str]| { let status = Command::new("git") .current_dir(&remote) diff --git a/crates/signed_state/src/git_store.rs b/crates/signed_state/src/git_store.rs index c4072a2..8048b90 100644 --- a/crates/signed_state/src/git_store.rs +++ b/crates/signed_state/src/git_store.rs @@ -8,11 +8,9 @@ use signed_git::{GitCache, Repo}; static GIT_CACHE: OnceLock = OnceLock::new(); -/// The global git repository mirror cache. pub struct Mirrors; impl Mirrors { - /// Install the global mirror cache root, once. pub fn install(root: impl Into) { if GIT_CACHE.set(GitCache::new(root.into())).is_err() { log::warn!("git cache root is already set, keeping the first one"); @@ -25,22 +23,18 @@ impl Mirrors { .expect("git cache is initialized by signed_state::init") } - /// The root directory of the repository mirrors. pub(crate) fn root() -> PathBuf { Self::cache().root().to_path_buf() } - /// The on-disk path of the mirror of `addr`. pub fn path(addr: &RepoAddr) -> PathBuf { Self::cache().repo_path(addr) } - /// Open the mirror of `addr`, if it has been cloned. pub fn open(addr: &RepoAddr) -> Result> { Self::cache().open(addr) } - /// Open the mirror of `addr`, cloning it first when it does not exist yet. pub fn ensure(addr: &RepoAddr, clone_urls: &[Url]) -> Result { Self::cache().ensure_clone(addr, clone_urls) } diff --git a/crates/signed_state/src/inbox.rs b/crates/signed_state/src/inbox.rs index 96e56b7..69bab27 100644 --- a/crates/signed_state/src/inbox.rs +++ b/crates/signed_state/src/inbox.rs @@ -7,7 +7,6 @@ use signed_core::{Deletions, Filters, GitEvent, InboxItem, InboxReadState, inbox use crate::backend::Backend; -/// The user's persisted inbox read state. #[derive(Default)] pub struct Inbox { state: InboxReadState, @@ -15,7 +14,6 @@ pub struct Inbox { } impl Inbox { - /// The current read/archive cutoffs. pub fn state(&self) -> &InboxReadState { &self.state } @@ -67,7 +65,6 @@ impl Inbox { cx.notify(); } - /// Sign the state with a random key and store it locally. fn persist(&mut self, cx: &mut Context) { let backend = Backend::global(cx); let (me, client) = { @@ -91,7 +88,6 @@ impl Inbox { task.detach(); } - /// The notifications and authored activity of `me`, grouped into inbox items. pub async fn query( client: &Client, me: PublicKey, @@ -127,7 +123,6 @@ impl Inbox { } impl Inbox { - /// `d` tag identifying the inbox state event of `me`. fn inbox_state_d_tag(me: PublicKey) -> String { format!("signed-inbox-state:{}", me.to_hex()) } @@ -152,7 +147,8 @@ impl Inbox { } } - /// Sign with a random key and store locally. + // Signed with a random key: the state is local-only, authorship does not + // matter and no key material needs to be kept. async fn save_state( client: &Client, me: PublicKey, @@ -167,7 +163,6 @@ impl Inbox { Ok(()) } - /// Notification events and a lookup of every ancestor they reference. async fn fetch_notifications( client: &Client, me: PublicKey, @@ -223,7 +218,6 @@ impl Inbox { Ok((notifications, by_id)) } - /// Event ids referenced by `event` through its `e` and `E` tags. fn event_references(event: &Event) -> impl Iterator + '_ { event.tags.iter().filter_map(|tag| { if tag.kind() != "e" && tag.kind() != "E" { diff --git a/crates/signed_state/src/lib.rs b/crates/signed_state/src/lib.rs index 163d5be..9d858ff 100644 --- a/crates/signed_state/src/lib.rs +++ b/crates/signed_state/src/lib.rs @@ -37,28 +37,17 @@ pub fn init( // rustls uses the `aws_lc_rs` provider by default. let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); - // Initialize the nostr client and signer let backend = cx .foreground_executor() .block_on(async move { NostrBackend::open(db_path.as_ref()).await }); let backend = backend.expect("failed to initialize nostr backend"); let (client, signer) = (backend.client, backend.signer); - // Set Git cache for the repos root Mirrors::install(repos_root); - // Set global stores for the backend Backend::set_global(cx.new(|cx| Backend::new(client, signer, cx)), cx); - - // Set global stores for the profile ProfileStore::set_global(cx.new(ProfileStore::new), cx); - - // Set global stores for the repo list and local repos RepoListStore::set_global(cx.new(RepoListStore::new), cx); - - // Set global stores for the local repos LocalReposStore::set_global(cx.new(|cx| LocalReposStore::new(scan_paths, cx)), cx); - - // Set global stores for the checkouts CheckoutsStore::set_global(cx.new(CheckoutsStore::new), cx); } diff --git a/crates/signed_state/src/local_repos.rs b/crates/signed_state/src/local_repos.rs index 12b84d0..5fd5488 100644 --- a/crates/signed_state/src/local_repos.rs +++ b/crates/signed_state/src/local_repos.rs @@ -11,10 +11,8 @@ struct GlobalLocalReposStore(Entity); impl Global for GlobalLocalReposStore {} -/// Store of the git repositories discovered under a set of scan paths. pub struct LocalReposStore { pub roots: Arc>, - /// Git repositories discovered under [`Self::roots`], sorted by path. pub repos: Arc>, pub scanning: bool, scan_dirty: bool, @@ -45,7 +43,6 @@ impl LocalReposStore { } } - /// Forget a repository that has just been published to NIP-34. pub fn remove(&mut self, path: &Path, cx: &mut Context) { self.repos = Arc::new( self.repos @@ -94,7 +91,6 @@ impl LocalReposStore { dirty })?; - // Scans requested while this one ran are coalesced into one follow-up scan. if again { this.update(cx, |this, cx| this.rescan(cx))?; } @@ -106,8 +102,6 @@ impl LocalReposStore { } } -/// The NIP-34 coordinate a repository's detection resolved, when both the owner -/// and the identifier were recovered. pub fn local_repo_addr(repo: &LocalRepo) -> Option { let binding = repo.nip34.as_ref()?; let owner = binding.owner?; @@ -116,18 +110,14 @@ pub fn local_repo_addr(repo: &LocalRepo) -> Option { Some(RepoAddr::new(owner, identifier)) } -/// A scanned repository resolved against the known announcements. #[derive(Debug, Clone, PartialEq)] pub struct ResolvedLocalRepo { pub path: PathBuf, - /// `None` for a plain repository. pub nip34: Option, - /// The known announcement this repository is bound to, when one matched. pub announcement: Option, } impl ResolvedLocalRepo { - /// The repository's directory name, or `Untitled` when the path has none. pub fn name(&self) -> SharedString { self.path .file_name() @@ -136,7 +126,6 @@ impl ResolvedLocalRepo { } } -/// Resolve the scanned repositories against the known announcements. impl LocalReposStore { pub fn resolve( repos: &[LocalRepo], diff --git a/crates/signed_state/src/profile.rs b/crates/signed_state/src/profile.rs index daeffc3..0f59430 100644 --- a/crates/signed_state/src/profile.rs +++ b/crates/signed_state/src/profile.rs @@ -13,14 +13,10 @@ use utils::shorten_pubkey; use crate::backend::{Backend, BackendEvent}; use crate::bootstrap::sync_bootstrap_only; -/// How long to wait for more requests before firing a batched fetch. const BATCH_TIMEOUT: Duration = Duration::from_millis(500); -/// Max authors per profile request, keeping each filter within relay limits. const REQUEST_CHUNK: usize = 100; -/// Recent profiles prefetched at startup and read back from the cache. const WARM_LIMIT: usize = 500; -/// A user profile as plain data for the UI, from the kind-0 metadata. #[derive(Debug, Clone)] pub struct Profile { public_key: PublicKey, @@ -43,7 +39,7 @@ impl Profile { &self.metadata } - /// Display name, falling back to `name`, then a shortened npub. + // Falls back to `name`, then a shortened npub. pub fn name(&self) -> SharedString { if let Some(display_name) = self.metadata.display_name.as_ref() && !display_name.is_empty() @@ -69,14 +65,9 @@ impl Profile { } } -/// Global profile cache. -/// -/// Profiles are fetched in batches and kept as plain data. pub struct ProfileStore { profiles: HashMap, - /// Public keys requested this session, main thread only. seen: RefCell>, - /// Sender for queuing fetch requests, batched by a background task. sender: Sender, _subscription: Subscription, } @@ -126,9 +117,7 @@ impl ProfileStore { } } - /// Get a profile. - /// - /// Returns a placeholder with default metadata. Queues a fetch when the profile is not cached yet. + // Returns a placeholder until fetched; queues a fetch when uncached. pub fn get(&self, public_key: &PublicKey) -> Profile { if let Some(profile) = self.profiles.get(public_key) { return profile.clone(); @@ -179,7 +168,6 @@ impl ProfileStore { .detach(); } - /// Re-read the latest metadata of `authors` from the local database in one query. fn apply_authors(&mut self, authors: Vec, cx: &mut Context) { if authors.is_empty() { return; @@ -232,13 +220,11 @@ impl ProfileStore { .detach(); } - /// Re-read the latest metadata of every requested author from the local database. fn apply_seen(&mut self, cx: &mut Context) { let authors: Vec = self.seen.borrow().iter().copied().collect(); self.apply_authors(authors, cx); } - /// Fetch metadata for requested authors in batches, debounced to collect requests. async fn handle_requests( this: WeakEntity, client: &Client, diff --git a/crates/signed_state/src/push.rs b/crates/signed_state/src/push.rs index 9bd06af..6858e0a 100644 --- a/crates/signed_state/src/push.rs +++ b/crates/signed_state/src/push.rs @@ -12,14 +12,11 @@ use signed_nostr::UniversalSigner; pub(crate) const GRASP_PUSH_ATTEMPTS: usize = 3; -/// Pause before re-staging a state event after a transient denial. const GRASP_RETRY_DELAY: Duration = Duration::from_secs(1); -/// Base URL of a grasp server, `https://`. -/// -/// `ws://` grasp servers use `http://`, like ngit. +// `ws://` grasp servers, like ngit, use `http://`; secure relays map to +// `https://`. pub(crate) fn grasp_base_url(relay: &RelayUrl) -> Option { - // `domain()` drops the port. let parsed = Url::parse(relay.as_str()).ok()?; let host = parsed.host_str()?; let port = parsed.port().map(|p| format!(":{p}")).unwrap_or_default(); @@ -37,14 +34,12 @@ pub(crate) fn grasp_clone_url(relay: &RelayUrl, owner: &str, repo_id: &str) -> O Url::parse(&format!("{base}/{owner}/{repo_id}.git")).ok() } -/// GRASP-06 contributor namespace URL of a pull request tip. +// GRASP-06 contributor namespace URL of a pull request tip. pub(crate) fn grasp06_prs_url(base_url: &str, npub: &str, repo_id: &str) -> String { format!("{base_url}/prs/{npub}/{repo_id}.git") } -/// Assemble the `clone` URLs of a pull request. -/// -/// The author's GRASP-06 `/prs/` URLs come first. +// The author's GRASP-06 `/prs/` URLs come first. pub(crate) fn pr_clone_urls(prs_urls: Vec, base_clone_urls: Vec) -> Vec { let mut seen = std::collections::HashSet::new(); let mut urls = Vec::new(); @@ -59,7 +54,6 @@ pub(crate) fn pr_clone_urls(prs_urls: Vec, base_clone_urls: Vec) -> Ve #[derive(Debug, Clone)] pub struct GraspServer { relay: RelayUrl, - /// `None` when the server accepted the data, the reason otherwise. reason: Option, } @@ -82,7 +76,6 @@ impl GraspServer { &self.relay } - /// `None` when the server accepted the data, the reason otherwise. pub fn reason(&self) -> Option<&str> { self.reason.as_deref() } @@ -90,11 +83,9 @@ impl GraspServer { #[derive(Debug, Clone, Default)] pub struct PushOutcome { - /// Per-server results, in the order the servers were listed. pub servers: Vec, - /// The newest state event a grasp relay accepted for this push, if any. - /// - /// Broadcast to the other relays once a git server holds the data. + // The newest state event a grasp relay accepted for this push; broadcast + // to the other relays once a git server holds the data. pub state_event: Option, } @@ -121,9 +112,7 @@ impl PushOutcome { .join("; ") } - /// A warning for a push only some grasp servers accepted. - /// - /// `None` when every server accepted the push or nothing was pushed. + // `None` when every server accepted the push or nothing was pushed. pub fn partial_warning(&self) -> Option { let accepted = self.accepted(); if self.servers.is_empty() || accepted == self.servers.len() { @@ -137,25 +126,23 @@ impl PushOutcome { } } -/// Reasons a push attempt should be retried with a freshly staged state -/// event and a fresh git advertisement. -/// -/// Two families are retried: -/// -/// - **Purgatory denials**: the grasp server sends these when the state -/// event for the push has not reached its purgatory yet. Re-staging a -/// fresh event resolves them. -/// - **Stale advertisement races**: `git receive-pack` compares each ref -/// update against the value it advertised when the push started. The grasp -/// server's own background sync can move a ref in between - typically by -/// aligning the repository to a parked state event once the objects of an -/// earlier attempt land - so the compare-and-swap fails with `cannot lock -/// ref` / `incorrect old value provided`. A retry against the fresh -/// advertisement converges, and when the race is lost the pushed data is -/// usually already on the server (see `is_stale_advertisement_race` and -/// the convergence probe in `push_staged_to_grasps`). -/// -/// Other rejections are not retried. +// Reasons a push attempt should be retried with a freshly staged state event +// and a fresh git advertisement. Two families are retried: +// +// - **Purgatory denials**: the grasp server sends these when the state event +// for the push has not reached its purgatory yet. Re-staging a fresh event +// resolves them. +// - **Stale advertisement races**: `git receive-pack` compares each ref +// update against the value it advertised when the push started. The grasp +// server's own background sync can move a ref in between - typically by +// aligning the repository to a parked state event once the objects of an +// earlier attempt land - so the compare-and-swap fails with `cannot lock +// ref` / `incorrect old value provided`. A retry against the fresh +// advertisement converges, and when the race is lost the pushed data is +// usually already on the server (see `is_stale_advertisement_race` and +// the convergence probe in `push_staged_to_grasps`). +// +// Other rejections are not retried. fn is_transient_grasp_denial(stderr: &str) -> bool { let error = stderr.to_lowercase(); [ @@ -171,19 +158,17 @@ fn is_transient_grasp_denial(stderr: &str) -> bool { .any(|marker| error.contains(marker)) } -/// A push rejected because `git receive-pack`'s compare-and-swap lost to the -/// grasp server's own background ref alignment: the ref moved between this -/// push's advertisement and its ref transaction (`cannot lock ref ... is at -/// ... but expected ...` / `incorrect old value provided`). The pushed data -/// is usually already on the server by then. +// A push rejected because `git receive-pack`'s compare-and-swap lost to the +// grasp server's own background ref alignment: the ref moved between this +// push's advertisement and its ref transaction (`cannot lock ref ... is at +// ... but expected ...` / `incorrect old value provided`). The pushed data +// is usually already on the server by then. fn is_stale_advertisement_race(stderr: &str) -> bool { let error = stderr.to_lowercase(); error.contains("cannot lock ref") || error.contains("incorrect old value provided") } -/// Keep `event` as the push's fan-out state event when it is newer than the -/// current one. All staged events carry the same refs; the newest timestamp -/// wins on the relays. +// All staged events carry the same refs; the newest timestamp wins on the relays. fn keep_newest(state_event: &mut Option, event: Event) { if state_event .as_ref() @@ -193,8 +178,8 @@ fn keep_newest(state_event: &mut Option, event: Event) { } } -/// The grasp push pipeline: stage a signed state event on each server's -/// relay, then push the git data, retrying transient denials. +// The grasp push pipeline: stage a signed state event on each server's +// relay, then push the git data, retrying transient denials. #[derive(Clone)] pub(crate) struct GraspPush { client: Client, @@ -206,7 +191,6 @@ impl GraspPush { Self { client, signer } } - /// The event was accepted by at least one relay, or a descriptive error otherwise. pub(crate) fn require_relay_accepted( output: SendEventOutput, event: Event, @@ -224,10 +208,7 @@ impl GraspPush { Ok(event) } - /// Sign and broadcast `builder`, logging rather than surfacing failures. - /// - /// Used for best-effort identity bootstrap events, where a relay hiccup - /// should not block sign-up. + // A relay hiccup should not block sign-up, so failures are only logged. pub(crate) async fn publish_best_effort(&self, builder: EventBuilder) { let result: Result<(), Error> = async { let event = builder.finalize_async(&self.signer).await?; @@ -242,21 +223,16 @@ impl GraspPush { } } - /// Sign `builder`, broadcast the event and require a relay to accept it. - /// Returns the signed event. pub(crate) async fn publish_one(&self, builder: EventBuilder) -> Result { let event = builder.finalize_async(&self.signer).await?; self.send_accepted(event).await } - /// Broadcast an already signed event and require a relay to accept it. - /// Returns the event. pub(crate) async fn send_accepted(&self, event: Event) -> Result { let output = self.client.send_event(&event).broadcast().await?; Self::require_relay_accepted(output, event) } - /// Sign and send a single NIP-09 deletion request for `event`. pub(crate) async fn retract_event(&self, event: &Event) -> Result<(), Error> { let builder = EventDeletionRequest::new() .id(event.id) @@ -268,12 +244,9 @@ impl GraspPush { Ok(()) } - /// Sign a fresh kind `30618` state event for the push. - /// - /// `last_created_at` is the timestamp of the previous event signed for this push. - /// Retries within the same second get the next second: a grasp relay - /// treats a same-id resend as a duplicate and does not re-run its ingest, - /// so an identical resend cannot re-park a state event lost from its purgatory. + // Retries within the same second get the next second: a grasp relay + // treats a same-id resend as a duplicate and does not re-run its ingest, + // so an identical resend cannot re-park a state event lost from its purgatory. async fn sign_state_event( &self, repo_id: &str, @@ -297,11 +270,8 @@ impl GraspPush { Ok((event, created_at)) } - /// Ensure the relay is known and connected, then publish `event` to it. - /// - /// `Ok` only when the relay confirmed the event. - /// On a grasp relay the accept parks the event in purgatory, - /// which authorizes the paired git push. + // `Ok` only when the relay confirmed the event. On a grasp relay the + // accept parks the event in purgatory, which authorizes the paired git push. async fn stage_event_on_relay(&self, relay: &RelayUrl, event: &Event) -> Result<(), String> { self.client .add_relay(relay) @@ -353,13 +323,12 @@ impl GraspPush { let mut reason = None; let mut last_created_at = 0; - // The last state event staged on this server, for the convergence - // probe below when every push attempt lost the stale-ref race. + // For the convergence probe when every push attempt lost the + // stale-ref race. let mut staged_event = None; 'server: for attempt in 1..=GRASP_PUSH_ATTEMPTS { if attempt > 1 { - // Give the server's ingest a moment before re-staging. executor.timer(GRASP_RETRY_DELAY).await; } @@ -376,14 +345,12 @@ impl GraspPush { last_created_at = created_at; - // Stage the state event on this server's own relay. - // A failed stage means the grasp never parked the state, - // so the git push would be denied anyway: skip it (the eligibility gate). + // A failed stage means the grasp never parked the state, so + // the git push would be denied anyway: skip it. if let Err(e) = self.stage_event_on_relay(relay, &event).await { // One retry absorbs a relay connect blip, on the first // attempt only. if attempt == 1 && self.stage_event_on_relay(relay, &event).await.is_ok() { - // staged on the retry } else { reason = Some(e); break 'server; @@ -410,10 +377,9 @@ impl GraspPush { // The grasp's own background sync aligns refs to staged state // events as soon as the objects land, which can beat every push - // attempt's compare-and-swap (`cannot lock ref ... but expected`). - // When the last denial was that race the sync has usually finished - // by now: verify the advertised refs and accept the server when the - // pushed data is already there. + // attempt's compare-and-swap. When the last denial was that race + // the sync has usually finished by now: verify the advertised refs + // and accept the server when the pushed data is already there. if let Some(last_reason) = &reason && is_stale_advertisement_race(last_reason) && Repo::open(path) @@ -476,7 +442,6 @@ mod tests { grasp06_prs_url("https://relay.ngit.dev", "npub1author", "my-repo"), "https://relay.ngit.dev/prs/npub1author/my-repo.git" ); - // `ws://` grasp servers, local dev, keep their plain-HTTP base. assert_eq!( grasp06_prs_url("http://localhost:8080", "npub1author", "my-repo"), "http://localhost:8080/prs/npub1author/my-repo.git" @@ -508,14 +473,11 @@ mod tests { #[test] fn transient_grasp_denials_are_classified() { - // The exact server rejection that started this work: the state event - // had not reached the grasp's purgatory before the git push. let reported = "remote: ERR authorisation failed: No state events in purgatory\n\ fatal: the remote end hung up unexpectedly\n\ error: failed to push some refs to 'https://relay.ngit.dev/...git'"; assert!(is_transient_grasp_denial(reported)); - // The other purgatory states a fresh event resolves. assert!(is_transient_grasp_denial( "remote: ERR authorisation failed: No matching state event found in purgatory" )); @@ -531,7 +493,6 @@ mod tests { "remote: ERR authorisation failed: No repository announcement found" )); - // Rejections a fresh state event cannot fix are not retried. assert!(!is_transient_grasp_denial( "remote: ERR authorisation failed: not a maintainer of this repository" )); @@ -546,9 +507,6 @@ mod tests { #[test] fn stale_ref_races_are_retried() { - // The grasp's background sync aligned the ref to a parked state event - // between this push's advertisement and its ref transaction. The ref - // is usually already where the push wants it, so a retry converges. let reported = "remote: error: cannot lock ref 'refs/heads/main': is at \ cac2ac91b6f5fb8dfcb6962785babc6e65350cb3 but expected \ bc5e892aa84dc6240a5fbcd59367a4857d26f49b\n\ @@ -558,7 +516,6 @@ mod tests { assert!(is_transient_grasp_denial(reported)); assert!(is_stale_advertisement_race(reported)); - // Markers match independently of the surrounding git output. assert!(is_stale_advertisement_race( "cannot lock ref 'refs/heads/main'" )); @@ -566,10 +523,8 @@ mod tests { "! [remote rejected] main -> main (incorrect old value provided)" )); - // A purgatory denial is not a stale-advertisement race. assert!(!is_stale_advertisement_race("No state events in purgatory")); - // A real divergence is a different error and stays permanent. assert!(!is_transient_grasp_denial( " ! [rejected] main -> main (non-fast-forward)" )); diff --git a/crates/signed_state/src/refresh.rs b/crates/signed_state/src/refresh.rs index e296d72..0f63e56 100644 --- a/crates/signed_state/src/refresh.rs +++ b/crates/signed_state/src/refresh.rs @@ -1,4 +1,3 @@ -/// Refresh coalescing shared by the event stores. #[derive(Debug, Default)] pub struct RefreshGate { running: bool, @@ -7,9 +6,7 @@ pub struct RefreshGate { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RefreshRequest { - /// No run covers the request, start one now. Schedule, - /// A run is in flight and covers the request, fold it into a follow-up. Fold, } @@ -31,13 +28,12 @@ impl RefreshGate { self.running = true; } - /// The run ended. Whether a request arrived while it ran. pub fn finish(&mut self) -> bool { self.running = false; std::mem::take(&mut self.dirty) } - /// The run was abandoned, e.g. on error. Pending follow-up requests survive. + // Pending follow-up requests survive an abandoned run. pub fn abort(&mut self) { self.running = false; } diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 532ac6e..fd4f907 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -21,74 +21,41 @@ use crate::checkouts::CheckoutsStore; use crate::push::{GraspPush, PushOutcome, grasp_base_url, grasp06_prs_url, pr_clone_urls}; use crate::repos::RepoListStore; -/// Maximum size of one patch event. -/// -/// NIP-34 suggests patches when each event is under 60kb. +// NIP-34 suggests patches when each event is under 60kb. const MAX_PATCH_EVENT_BYTES: usize = 60 * 1024; -/// Per-repository store. -/// -/// Holds the announcement, state, issues, patches, PRs, comments and resolved statuses. pub struct RepoStore { - /// NIP-34 address. `None` while the repository is local-only. addr: Option, - /// Latest announcement. Seeded from the open-time hint, replaced by the - /// database's latest on the first pass. `None` while local-only. + // Seeded from the open-time hint, replaced by the database's latest on the + // first pass. `None` while local-only. pub announcement: Option, - /// Local working copy. The scan path for a local repository, kept when it is - /// later announced so the panel keeps its worktree. + // The scan path for a local repository, kept when it is later announced so + // the panel keeps its worktree. pub path: Option, - /// NIP-34 state detected on disk for a local repository, if any. pub nip34: Option, - /// The first local pass has been applied. - /// - /// Views distinguish "no data yet" from a genuinely empty repository with it. + // Views distinguish "no data yet" from a genuinely empty repository with it. pub loaded: bool, - /// Branch pointed to by `HEAD` in the latest state announcement. pub head: Option, pub issues: Vec, pub patches: Vec, pub pull_requests: Vec, - /// Comments on issues / PRs, oldest first. pub comments: Vec, - /// Resolved status per root event, issue, patch or PR. status_by_root: HashMap, - /// Open issue and root PR counts. - /// Computed with [`Self::status_by_root`] on every refresh. open_issue_count: usize, open_pr_count: usize, pub last_error: Option, - /// Non-fatal warning of the last action, if any. - /// - /// Example, a PR published without its commit reaching a grasp server. pub last_warning: Option, - /// Warning of the last push that only some grasp servers accepted. - /// - /// The repository is out of sync on the rejected servers until it is republished. + // The repository is out of sync on the rejected servers until republished. pub last_push_warning: Option, - /// A republish or a checkout push is in flight. - /// - /// Views show a spinner and disable their push triggers while it is set. pub pushing: bool, - /// A clone-into-a-folder operation is in flight. - /// - /// Views show a spinner and disable the clone trigger while it is set. pub cloning: bool, - /// Relays already asked to connect to, from this repository's NIP-34 `relays` tag. - /// - /// Avoids re-subscribing and re-fetching on every refresh. + // Avoids re-subscribing and re-fetching on every refresh. repo_relays: HashSet, - /// Root events, issues, patches and PRs, already fetched per root. - /// - /// The per-root fetches cover NIP-22 comments and statuses without an `a` tag. + // Covers NIP-22 comments and statuses without an `a` tag. root_fetches: HashSet, - /// Maintainers already synced through gossip in Uncensored mode. - /// - /// Avoids re-running the maintainer Auto sync on every refresh. synced_maintainers: HashSet, - /// In-flight tasks, cancelled when the store drops. + // In-flight tasks, cancelled when the store drops. tasks: Vec>>, - /// Backend subscription of an announced repository. `None` while local-only. _subscription: Option, } @@ -141,7 +108,6 @@ impl RepoStore { } } - /// Local repository discovered by the scan, not announced to NIP-34 yet. pub fn new_local(path: PathBuf, nip34: Option) -> Self { Self { addr: None, @@ -170,7 +136,6 @@ impl RepoStore { } } - /// An announced repository whose working copy is already on disk. pub fn from_worktree( addr: RepoAddr, announcement: Announcement, @@ -182,7 +147,6 @@ impl RepoStore { store } - /// Switch a local repository to its NIP-34 mode, keeping its path. pub fn announce(&mut self, announcement: Announcement, cx: &mut Context) { self.addr = Some(announcement.addr()); self.announcement = Some(announcement.clone()); @@ -235,7 +199,6 @@ impl RepoStore { self.addr.as_ref() } - /// Returns the repository's name, or `Unknown` when not known. pub fn name(&self) -> SharedString { self.announcement .as_ref() @@ -244,9 +207,6 @@ impl RepoStore { }) } - /// Filters that make up a repository. - /// - /// Announcement, state, activity and deletions targeting it. fn repo_filters(addr: &RepoAddr) -> Vec { let mut filters = vec![ Filter::new() @@ -255,12 +215,11 @@ impl RepoStore { .identifier(addr.identifier()), addr.activity_filter(), ]; - // Deletion requests, NIP-09/62, must be known before any event is shown. + // Deletion requests must be known before any event is shown. filters.extend(addr.deletion_filters()); filters } - /// Fetch this repository's events from the relays in its NIP-34 `relays` tag. fn connect_announced_relays(&mut self, relays: &[RelayUrl], cx: &mut Context) { let Some(addr) = self.addr.clone() else { return; @@ -285,39 +244,33 @@ impl RepoStore { }); } - /// Filters the SDK resolves through NIP-65 gossip in Uncensored mode. + // NIP-34 events tag the announcement author, which may not be a + // maintainer for subordinate forks. fn maintainer_filters(addr: &RepoAddr, maintainers: &[PublicKey]) -> Vec { let mut pubkeys = maintainers.to_vec(); - // NIP-34 events tag the announcement author, - // which may not be a maintainer for subordinate forks. if !pubkeys.contains(&addr.public_key()) { pubkeys.push(addr.public_key()); } vec![ - // Announcement and state events, including co-maintainer states. Filter::new() .kinds([Kind::GitRepoAnnouncement, Kind::RepoState]) .authors(pubkeys.clone()) .identifier(addr.identifier()), - // Activity tagging a maintainer, resolved to their read relays. Filter::new() .kinds(filters::ACTIVITY_KINDS) .coordinate(addr.coordinate()) .pubkeys(pubkeys.clone()), - // Activity authored by a maintainer, resolved to their write relays. Filter::new() .kinds(filters::ACTIVITY_KINDS) .coordinate(addr.coordinate()) .authors(pubkeys.clone()), - // Deletions authored by a maintainer. Filter::new() .kinds([Kind::EventDeletion, Kind::RequestToVanish]) .authors(pubkeys), ] } - /// In Uncensored mode, sync the maintainer-shaped filters through the SDK's NIP-65 gossip targeting fn sync_maintainer_relays(&mut self, maintainers: &[PublicKey], cx: &mut Context) { let strategy = SettingsStore::try_global(cx) .map(|store| store.read(cx).settings().event_fetching) @@ -358,7 +311,6 @@ impl RepoStore { }); } - /// Re-query the local database and update all fields. pub fn refresh(&mut self, cx: &mut Context) { if self.addr.is_none() { return; @@ -531,7 +483,6 @@ impl RepoStore { this.announcement = announcement; } - // The announcement may list relays for this repository's activity. let relays = this .announcement .as_ref() @@ -540,7 +491,6 @@ impl RepoStore { this.connect_announced_relays(&relays, cx); - // Uncensored mode also covers the maintainers' NIP-65 relays. let maintainers = this .announcement .as_ref() @@ -602,22 +552,18 @@ impl RepoStore { self.tasks.push(task); } - /// Resolve the status of a root event, an issue, patch or PR, per NIP-34. pub fn status_of(&self, root: &Event) -> RepoStatus { status_of(&self.status_by_root, root) } - /// Number of open issues. pub fn issue_count(&self) -> usize { self.open_issue_count } - /// Number of open pull requests. pub fn pull_request_count(&self) -> usize { self.open_pr_count } - /// Whether `user` is the author or owner of this repository. pub fn is_author(&self, user: &PublicKey) -> bool { self.addr .as_ref() @@ -647,14 +593,10 @@ impl RepoStore { .filter(move |e| e.references_root(root)) } - /// Comment on a root event, an issue or PR, per NIP-34, kind 1111. pub fn comment(&mut self, root: &Event, content: String, cx: &mut Context) { self.reply(root, None, content, cx); } - /// Reply to `parent`, a comment on `root`, with a NIP-22 threaded comment. - /// - /// `None` publishes a top-level comment on the root itself. fn reply( &mut self, root: &Event, @@ -710,8 +652,8 @@ impl RepoStore { return; } - // The tip of the series is its last commit. - // `git format-patch` orders patches oldest first. + // The tip of the series is its last commit; `git format-patch` orders + // patches oldest first. let Some(current_commit) = series.tip_commit() else { self.last_error = Some( "Patch must be `git format-patch` output with a `From ` header".into(), @@ -731,7 +673,6 @@ impl RepoStore { cx.notify(); return; }; - // The author's npub names their GRASP-06 namespace, `/prs/...`. let author_npub = user.to_bech32().unwrap(); let owner = addr.public_key(); @@ -762,8 +703,6 @@ impl RepoStore { }; let task: Task> = cx.spawn(async move |this, cx| { - // The PR references the root patch, - // viewers can then find the patch without carrying it inline. let root_patch = match publish_patch_series( &client, &signer, @@ -785,11 +724,10 @@ impl RepoStore { } }; - // GRASP-06 pushes the tip to the author's own grasp servers. - // The path is `/prs//.git`. - // Contributing to another project never depends on that project's servers. - // Resolve the servers from the author's latest kind-10317 grasp list. - // The settings defaults stand in when no list is published or the query fails. + // GRASP-06 pushes the tip to the author's own grasp servers at + // `/prs//.git`; contributing to another + // project never depends on that project's servers. Resolve the + // servers from the author's latest kind-10317 grasp list. let author_servers = { let query_client = client.clone(); let published = cx @@ -866,7 +804,6 @@ impl RepoStore { } })?; - // Sign before publishing. let event = cx .background_spawn({ let signer = signer.clone(); @@ -883,7 +820,6 @@ impl RepoStore { let path = path.clone(); let tip = tip.clone(); let reference = reference.clone(); - // Author servers first, then the announced base grasp servers. let targets: Vec<(String, String)> = author_targets .into_iter() .chain(base_targets) @@ -933,8 +869,6 @@ impl RepoStore { } }; - // A draft PR carries a kind-1633 status event, NIP-34. - // Publish it right after the PR event so viewers never show it open. if draft { this.update(cx, |this, cx| { this.set_status(&pr_event, RepoStatus::Draft, cx); @@ -946,12 +880,6 @@ impl RepoStore { self.tasks.push(task); } - /// Generate the patch between `merge_base` and `compare_ref` in `repo_path`, - /// then open a pull request from it. - /// - /// Fails descriptively when there are no commits to propose or the patch - /// could not be generated; otherwise publishes exactly like - /// [`Self::open_pull_request`]. #[allow(clippy::too_many_arguments)] pub fn open_pull_request_from_refs( &mut self, @@ -965,8 +893,8 @@ impl RepoStore { cx: &mut Context, ) -> Task> { cx.spawn(async move |this, cx| { - // Regenerate the series at submit time. - // The published patch covers the current tip of the compare branch. + // Regenerate at submit time so the published patch covers the + // current tip of the compare branch. let patch = cx .background_spawn({ let repo_path = repo_path.clone(); @@ -999,9 +927,6 @@ impl RepoStore { }) } - /// Update a pull request. - /// - /// 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; @@ -1034,7 +959,7 @@ impl RepoStore { return; } - // The new tip of the PR is the last commit of the series. + // The tip of the updated PR is the last commit of the series. let Some(current_commit) = series.tip_commit() else { self.last_error = Some( "Patch must be `git format-patch` output with a `From ` header".into(), @@ -1043,8 +968,8 @@ impl RepoStore { return; }; - // The first revision patch replies to the original root patch, NIP-34. - // Use the PR's `e` tag, or the oldest patch of the linked set if the PR has none. + // The first revision patch replies to the original root patch, NIP-34: + // use the PR's `e` tag, or the oldest patch of the linked set. let root_patch_id = root.tags.event_ids().next().or_else(|| { PullRequest::new(root) .patches(self.patches.iter()) @@ -1097,8 +1022,8 @@ impl RepoStore { } .into_event_builder(); - // The `r` EUC tag lets clients subscribe to all PR updates. - // The SDK builder omits it. + // The `r` EUC tag lets clients subscribe to all PR updates; the + // SDK builder omits it. match euc.as_deref() { Some(euc) => builder.tag(Tag::parse(["r", euc]).expect("valid r tag")), None => builder, @@ -1122,9 +1047,6 @@ impl RepoStore { self.tasks.push(task); } - /// Set the status of a root event. - /// - /// Only the root author or a maintainer may set it, per NIP-34. fn set_status(&mut self, root: &Event, status: RepoStatus, cx: &mut Context) { self.last_error = None; @@ -1166,8 +1088,6 @@ impl RepoStore { self.publish(builder, cx); } - /// The latest announcement of this repository, - /// for operations that need its clone URLs and relays. fn action_announcement(&self, cx: &App) -> Option { let addr = self.addr.as_ref()?; self.announcement.clone().or_else(|| { @@ -1227,11 +1147,6 @@ impl RepoStore { self.run_push(push, Some((addr, path)), cx) } - /// Run a backend push task, tracking progress in [`Self::pushing`] and - /// the outcome in [`Self::last_error`] and [`Self::last_push_warning`]. - /// - /// `pushed_checkout` names the checkout whose ready-to-push statuses - /// should be recomputed after the remote moved. fn run_push( &mut self, push: Task>, @@ -1252,11 +1167,8 @@ impl RepoStore { match &result { Ok(outcome) => { this.last_error = None; - // A push only some grasp servers accepted is a warning: - // the repo is out of sync on the rest until it is republished. this.last_push_warning = outcome.partial_warning(); if let Some((addr, path)) = &pushed_checkout { - // The remote moved, so recompute the ready-to-push statuses. CheckoutsStore::global(cx).update(cx, |store, cx| { store.checkout_pushed(addr, path, cx); }); @@ -1275,10 +1187,6 @@ impl RepoStore { }) } - /// Delete the repository from nostr, announcement, state and activity. - /// - /// Only the repository owner may delete it. The lists update when the - /// deletion events arrive. pub fn delete_repository(&mut self, cx: &mut Context) -> Task> { let Some(addr) = self.addr.clone() else { return self.action_error("This repository is not published to Nostr yet", cx); @@ -1300,8 +1208,7 @@ impl RepoStore { }) } - /// Clone the repository into `destination`, a user-chosen folder outside - /// the cache, and remember the clone as a checkout of this repository. + // A user-chosen folder outside the cache; remembered as a checkout. pub fn clone_to_folder( &mut self, destination: PathBuf, @@ -1329,8 +1236,7 @@ impl RepoStore { let clone = { let destination = destination.clone(); - // The clone's repository handle is dropped in the task: gix handles - // are not `Send`, they must not cross the spawn boundary. + // gix handles are not `Send` and must not cross the spawn boundary. cx.background_spawn(async move { Repo::clone(&clone_urls, &destination).map(|_| ()) }) }; @@ -1376,12 +1282,8 @@ impl RepoStore { Task::ready(Err(anyhow::anyhow!("{message}"))) } - /// Sign `builder`, broadcast it and track the outcome in [`Self::last_error`]. - /// - /// Every one-shot repository event (issue, comment, status) goes through - /// this. Multi-step flows (opening or updating a pull request, a patch - /// series) call the SDK directly instead, since their error handling and - /// post-conditions differ per step. + // Every one-shot repository event (issue, comment, status) goes through + // this; multi-step flows call the SDK directly instead. fn publish(&mut self, builder: EventBuilder, cx: &mut Context) { self.last_error = None; @@ -1446,15 +1348,13 @@ fn resolve_statuses( .collect() } -/// The proposed commit of a `git format-patch` output. -/// It is the `From ` header on the first line. +// The `From ` header on the first line. fn patch_current_commit(patch: &str) -> Option<&str> { let line = patch.lines().next()?; let hex = line.strip_prefix("From ")?; hex.split_whitespace().next().filter(|hex| hex.len() == 40) } -/// A `git format-patch` series with the facts derived from its parts. struct PatchSeries { parts: Vec, } @@ -1469,7 +1369,7 @@ impl PatchSeries { } } - /// The byte length of the first part over the NIP-34 size suggestion. + // The byte length of the first part over the NIP-34 size suggestion. fn oversized_length(&self) -> Option { self.parts .iter() @@ -1477,8 +1377,8 @@ impl PatchSeries { .find(|length| *length > MAX_PATCH_EVENT_BYTES) } - /// The tip of the series is its last commit; - /// `git format-patch` orders patches oldest first. + // The tip of the series is its last commit; `git format-patch` orders + // patches oldest first. fn tip_commit(&self) -> Option { self.parts .last() @@ -1494,9 +1394,7 @@ impl PatchSeries { } } -/// Publish a `git format-patch` series as chained kind-1617 events. -/// -/// Returns the root event, the one a PR references. +// Returns the root event, the one a PR references. #[allow(clippy::too_many_arguments)] async fn publish_patch_series( client: &Client, @@ -1566,7 +1464,6 @@ async fn publish_patch_series( root.ok_or_else(|| anyhow::anyhow!("patch series is empty")) } -/// Build a NIP-22 kind-1111 comment. fn comment_builder( root: &Event, parent: Option<&Event>, @@ -1630,19 +1527,15 @@ mod tests { assert!(kinds.contains(&expected), "missing {expected} tag"); } - // The uppercase `E` tag scopes the root, with its id, relay hint and author. let e = event.tags.iter().find(|t| t.kind() == "E").expect("E tag"); let slice = e.as_slice(); assert_eq!(slice[1], root.id.to_hex()); assert_eq!(slice[2], relay.as_str()); assert_eq!(slice[3], root.pubkey.to_hex()); - // The lowercase `e` tag references the parent. - // For a top-level comment the parent is the root itself. let e = event.tags.iter().find(|t| t.kind() == "e").expect("e tag"); assert_eq!(e.as_slice()[1], root.id.to_hex()); - // Signed's own `references_root` must keep matching the comment. assert!(event.references_root(&root.id)); } @@ -1652,13 +1545,11 @@ mod tests { let maintainer = Keys::generate().public_key(); let addr = signed_core::RepoAddr::new(owner, "my-repo"); - // The owner is not among the maintainers, as on a subordinate fork. let filters = RepoStore::maintainer_filters(&addr, &[maintainer]); assert_eq!(filters.len(), 4); let expected = HashSet::from([owner, maintainer]); - // Gossip only resolves pubkeys from `authors` and the lowercase `#p` tag. let named = |filter: &Filter| -> HashSet { let authors = filter.authors.iter().flatten().copied(); let p_tag = filter @@ -1670,7 +1561,6 @@ mod tests { authors.chain(p_tag).collect() }; - // Announcement and state events, scoped to the repository identifier. let announcement = &filters[0]; assert_eq!(named(announcement), expected); assert!( @@ -1679,7 +1569,6 @@ mod tests { .contains_key(&SingleLetterTag::LOWERCASE_D) ); - // Activity filters, scoped to the repository coordinate. for filter in &filters[1..3] { assert_eq!(named(filter), expected); assert!( @@ -1689,7 +1578,6 @@ mod tests { ); } - // Deletions, named by author only. let deletions = &filters[3]; assert_eq!( deletions diff --git a/crates/signed_state/src/repos.rs b/crates/signed_state/src/repos.rs index 469974b..b3faba6 100644 --- a/crates/signed_state/src/repos.rs +++ b/crates/signed_state/src/repos.rs @@ -17,37 +17,27 @@ struct GlobalRepoListStore(Entity); impl Global for GlobalRepoListStore {} -/// NIP-34 activity event counts per repository, ranking the explore list by popularity. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] pub struct RepoActivityCounts { - /// Root `30611` issue events addressed to the repository. pub issues: u32, - /// Root `3063` pull request events addressed to the repository. - /// - /// PR updates are not new PRs and do not count. + // PR updates are not new PRs and do not count. pub pull_requests: u32, - /// `1617` patch events addressed to the repository. pub commits: u32, } impl RepoActivityCounts { - /// Total issues, pull requests and commits, the popularity ranking key. + // The popularity ranking key. pub fn score(self) -> u32 { self.issues + self.pull_requests + self.commits } } -/// Store listing the discovered repository announcements, newest first. pub struct RepoListStore { - /// Shared so views can clone the list per frame without a deep copy. + // Shared so views can clone the list per frame without a deep copy. pub announcements: Arc>, - /// Latest known activity timestamp per repository. pub last_activity: Arc>, - /// Issues, pull requests and commits per repository. - /// - /// Used for the Popular ranking of the explore list. + // For the Popular ranking of the explore list. pub counts: Arc>, - /// Own repositories whose state events were fetched from their announced relays. state_synced_repos: HashSet, refresh: RefreshGate, _subscription: Subscription, @@ -100,7 +90,6 @@ impl RepoListStore { } } - /// The announcements of `user`, newest first. pub fn announcements_of(&self, user: &PublicKey) -> Vec { self.announcements .iter() @@ -125,7 +114,6 @@ impl RepoListStore { }); } - /// Fetch the state events of the user's own repositories. fn sync_own_repo_states(&mut self, cx: &mut Context) { let backend = Backend::global(cx); let Some(me) = backend.read(cx).current_user() else { @@ -149,10 +137,8 @@ impl RepoListStore { } } - /// Re-query the local database. - /// - /// Runs immediately. The backend pump already batches the relay events that - /// trigger a refresh, so no per-store debounce is needed. + // Runs immediately: the backend pump already batches the relay events + // that trigger a refresh, so no per-store debounce is needed. pub fn refresh(&mut self, cx: &mut Context) { if self.refresh.request() != RefreshRequest::Schedule { return; @@ -174,8 +160,8 @@ impl RepoListStore { let deletion_events = client.database().query(Filters::deletions()).await?; let deletions = Deletions::from_events(deletion_events); - // Dedup and sort off the main thread. - // Only the final list crosses back into the entity. + // Dedup and sort off the main thread; only the final list crosses + // back into the entity. let mut by_repo: HashMap = HashMap::new(); for event in events { @@ -220,7 +206,6 @@ impl RepoListStore { *entry = (*entry).max(event.created_at); } - // Bound the activity query to a recent window. // Older repos fall back to their announcement or state timestamps. let activity_filter = Filter::new() .kinds(filters::ACTIVITY_KINDS) @@ -242,8 +227,8 @@ impl RepoListStore { } } - // Popularity counts per repository, issues, pull requests and patches. - // Unbounded, unlike the windowed activity query above, so totals are exact. + // Unbounded, unlike the windowed activity query above, so totals + // are exact. let mut counts: HashMap = HashMap::new(); let count_filter = Filter::new().kinds([Kind::GitIssue, Kind::GitPullRequest, Kind::GitPatch]); @@ -274,7 +259,6 @@ impl RepoListStore { cx.spawn(async move |this, cx| { let (announcements, last_activity, counts) = match work.await { Ok(results) => results, - // Database errors are transient, keep the last list. Err(_) => { return this.update(cx, |this, _cx| { this.refresh.abort(); @@ -292,8 +276,6 @@ impl RepoListStore { this.refresh.finish() })?; - // Requests that arrived while the refresh was running. - // They are coalesced into one follow-up refresh. if again { this.update(cx, |this, cx| this.refresh(cx))?; }