From 5a74bf1f0d93eb7dedc686020fb42e2eff086137 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 26 Sep 2026 15:07:05 +0700 Subject: [PATCH 01/10] update initial sync --- crates/signed_state/src/backend.rs | 34 ++-- crates/signed_state/src/repo.rs | 3 +- crates/signed_state/src/repos.rs | 13 +- docs/backend-audit.md | 217 +++++++++++++++++++++ docs/event-fetching-strategy.md | 296 +++++++++++++++++++++++++++++ 5 files changed, 541 insertions(+), 22 deletions(-) create mode 100644 docs/backend-audit.md create mode 100644 docs/event-fetching-strategy.md diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 1c461b4..156606c 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -23,12 +23,7 @@ pub const USER_KEYRING: &str = "Signed Safe Storage"; pub const NOSTR_CONNECT_TIMEOUT: u64 = 60; /// Relays connected at startup, before any user-specific relay config is known. -pub const BOOTSTRAP_RELAYS: [&str; 4] = [ - "wss://relay.primal.net", - "wss://relay.ditto.pub", - "wss://index.ngit.dev", - "wss://profiles.nostr1.com", -]; +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"]; @@ -43,11 +38,6 @@ pub enum BackendEvent { /// The signer changed on login, logout or account switch. SignerChanged, /// New events were received from a relay and stored in the database. - /// - /// Batched: [`Backend`]'s notification pump coalesces everything a - /// relay delivers within one debounce window into a single event, - /// instead of emitting per-event and making every subscriber debounce - /// the same burst independently. NostrUpdate(Vec), Synced, SyncProgress { @@ -446,9 +436,6 @@ impl Backend { let client = self.client.clone(); // Initialize directly at the user's chosen destination. - // No mirror is pre-populated: `GitCache::ensure_clone` lazily clones - // from the grasp server the first time the repo detail view needs it, - // exactly like every other repository. let destination = { let dir_name = signed_git::sanitize_path_component(&name); let dir_name = if dir_name.is_empty() { @@ -1222,7 +1209,8 @@ impl Backend { .detach(); } - pub fn sync_bootstrap(&mut self, filter: Filter, cx: &mut Context) { + /// 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(); let (tx, mut rx) = SyncProgress::channel(); @@ -1259,8 +1247,19 @@ impl Backend { .detach(); let sync = cx.background_spawn(async move { - let opts = SyncOptions::default().progress(tx); - sync_bootstrap_only(&client, filter, opts).await + let mut first_error = None; + + for filter in filters { + let opts = SyncOptions::default().progress(tx.clone()); + if let Err(error) = sync_bootstrap_only(&client, filter, opts).await { + first_error.get_or_insert(error); + } + } + + match first_error { + Some(error) => Err(error), + None => Ok(()), + } }); cx.spawn(async move |this, cx| { @@ -1419,6 +1418,7 @@ pub(crate) async fn sync_bootstrap_only( .with(BOOTSTRAP_RELAYS) .opts(opts) .await?; + Ok(output.value) } diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 25cdf78..9ba4a81 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -288,9 +288,10 @@ impl RepoStore { self.repo_relays.extend(new.iter().cloned()); let backend = Backend::global(cx); + let filters = Self::repo_filters(&addr); backend.update(cx, |backend, cx| { - backend.connect_repo_relays(new, Self::repo_filters(&addr), cx); + backend.connect_repo_relays(new, filters, cx); }); } diff --git a/crates/signed_state/src/repos.rs b/crates/signed_state/src/repos.rs index f13ee91..c6309c8 100644 --- a/crates/signed_state/src/repos.rs +++ b/crates/signed_state/src/repos.rs @@ -134,10 +134,15 @@ impl RepoListStore { let backend = Backend::global(cx); backend.update(cx, |backend, cx| { - backend.sync_bootstrap(filters::all_announcements(), cx); - backend.sync_bootstrap(filters::all_states(), cx); - // Deletion requests, NIP-09/62, must be known before any announcement is shown. - backend.sync_bootstrap(filters::deletions(), cx); + backend.sync_bootstraps( + vec![ + filters::all_announcements(), + filters::all_states(), + // Deletion requests, NIP-09/62, must be known before any announcement is shown. + filters::deletions(), + ], + cx, + ); }); } diff --git a/docs/backend-audit.md b/docs/backend-audit.md new file mode 100644 index 0000000..0d0969a --- /dev/null +++ b/docs/backend-audit.md @@ -0,0 +1,217 @@ +# Backend audit: relays, sync and NIP-65 + +Status: audit of the current code, done before implementing +`docs/event-fetching-strategy.md`. Where this document and the first draft of +that proposal disagree, this document is authoritative. + +## Scope and method + +Crates read for relay and network behaviour: + +- `signed_state`: `backend`, `repo`, `repos`, `inbox`, `profile`, `checkouts`, + `local_repos`, `refresh`, `git_store`. +- `signed_nostr`: `backend`, `signer`, `update`. +- `signed_core`: `filters`, `deletions`, `model`, `state`, `status`, `inbox`. +- Relay touchpoints of `signed_git`, `settings`, `paths`, `utils`. + +Cross-checked against: + +- rust-nostr at the revision from `Cargo.lock`, + `b230cecf9dbb38e0228e6fff4544ed9d261326fc`. **All** `nostr*` crates + (`nostr`, `nostr-database`, `nostr-gossip`, `nostr-gossip-memory`, + `nostr-lmdb`, `nostr-sdk`) resolve to that single revision. The local + checkout is `~/.cargo/git/checkouts/nostr-619b808bb247a9ed/b230cec`. +- GitWorkshop, cloned with `ngit` from + `nostr://npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/gitworkshop` + at revision `420c0c3`. + +Paths below are relative to those checkouts unless prefixed with +`crates/`, which means this repository. + +## 1. Verified SDK behaviour + +### Target selection + +- `nostr-sdk/src/client/api/req_target.rs`: `ReqTarget::auto` wraps a bare + filter list; `ReqTarget::manual` wraps a relay map. `From>>` + (L91) makes an explicit map a **manual** request. +- `nostr-sdk/src/client/api/util.rs::build_targets`: + - Auto + gossip configured -> `gossip_break_down_filters`. + - Manual -> the map is used as-is, gossip is skipped. +- `nostr-sdk/src/client/api/subscribe.rs` and `api/sync.rs` both go through + these rules. `client.sync(filter)` with no `.with(...)` uses gossip; + `client.sync(filter).with(urls)` does not. +- `pool.sync` errors with `relay not found` when a targeted relay has not been + added to the pool. Targets are not implicitly added. + +### Gossip breakdown + +- `nostr-sdk/src/client/gossip/updater.rs::gossip_break_down_filter` (L412) + extracts pubkeys via `Filter::extract_public_keys` + (`nostr/src/filter/mod.rs` L591): **only `authors` and the lowercase `#p` + tag**. It first calls `ensure_gossip_public_keys_fresh`, then breaks the + filter down, then adds every resolved relay to the pool with + `RelayCapabilities::GOSSIP`. +- Freshness (`updater.rs` L381-L410, L164-L185) negentropy-syncs kind `10002` + for the extracted pubkeys over the pool's `DISCOVERY | READ` relays. This is + the only automatic way the gossip store learns kind `10002`. +- `nostr-sdk/src/client/gossip/resolver.rs::break_down_filter` (L93): + - `authors` only -> each author's **write** relays, plus hint and + most-received relays. + - `#p` only -> each pubkey's **read** relays, plus hint and most-received. + - both -> the union of read and write relays, keeping the filter unchanged. + - neither (`Other`), or no relay found (`Orphan`) -> the pool's **read** + relays (`read_relay_urls`). +- `relay/capabilities.rs`: `GOSSIP` is its own bit. `pool.read_relay_urls` / + `write_relay_urls` / `relay_urls_with_any_cap` use a raw bit test + (`filter_relays_with_any_cap`), so GOSSIP-only relays are excluded from + broadcast and from the `Other`/`Orphan` fallback. Per-relay send checks use + `can_read`/`can_write`, which do include GOSSIP, so gossip-resolved targets + work when named explicitly. +- `GossipConfig::default()` (`builder.rs` L30-L120): limits read 3, write 3, + hints 1, most-received 1, NIP-17 3 per user; `background_refresh` on by + default, disabled by `.no_background_refresh()`. +- `NostrGossipMemory` reads only the first 7 entries of a kind `10002` + (`MAX_NIP65_SIZE`) and stops tracking new hint/most-received relays once a + pubkey is near `MAX_RELAYS_PER_PK` (7). `get_best_relays(pk, selection, allowed)` + (public trait, `gossip/nostr-gossip/src/lib.rs`) reads whatever is in the + store, sorted by received-event count then recency. It does **not** fetch + kind `10002` itself. The store is primed by the freshness step of an Auto + request (or by any kind `10002` that arrives for another reason), and marks + a key outdated 24 h after its last fetch attempt. + +### Publishing + +- `client.send_event(event)` with **no** policy and gossip configured targets + NIP-65: it triggers the same freshness, then sends to the author's outbox + and every `p`-tagged pubkey's inbox (`send_event.rs::gossip_prepare_urls`). +- `client.send_event(event).broadcast()` overrides this to the pool's write + relays only (`send_event.rs` L428-L430). `signed` uses `broadcast()` + everywhere. + +### Notifications + +- Every message from a relay passes `relay/inner.rs::handle_event_msg`, which + saves new events and emits `ClientNotification::Event`. This includes events + received by a negentropy **sync** (the sync's down subscription is + registered as an auto-closing subscription, and the events travel the normal + message path). Events already in the database do not re-notify. +- The comment in `crates/signed_state/src/profile.rs` claiming synced events + produce no `NostrUpdate` is inaccurate for this revision. The re-read after + a sync is harmless, but the comment should not be relied on. + +## 2. What `signed` actually does today + +Every fetch path is manual. There is no Auto request anywhere in the +workspace, so `gossip_break_down_filter` and `ensure_gossip_public_keys_fresh` +never run. The gossip store is configured and passively fed +(`handle_event_msg` calls `gossip.process` for every received event), but it +is never consulted for targeting. + +| Call site | Request | Target relays | Target kind | +| --- | --- | --- | --- | +| `Backend::bootstrap` | `add_relay` + connect | `BOOTSTRAP_RELAYS` (READ\|WRITE), `INDEXER_RELAYS` (`DISCOVERY`) | n/a | +| `Backend::subscribe_bootstrap` -> `subscribe_bootstrap_only` | `client.subscribe(HashMap<&str, Vec>)`, `ExitOnEOSE`, 10 s timeout | `BOOTSTRAP_RELAYS` | manual | +| `Backend::sync_bootstraps` -> `sync_bootstrap_only` | `client.sync(filter).with(BOOTSTRAP_RELAYS)` | `BOOTSTRAP_RELAYS` | manual | +| `Backend::connect_repo_relays` | `add_relay().and_connect()` then `client.sync(filter).with(relays)` | announcement `relays` tag | manual | +| `RepoListStore::subscribe_remote` | `sync_bootstraps(all_announcements, all_states, deletions)` | `BOOTSTRAP_RELAYS` | manual | +| `RepoListStore::sync_own_repo_states` | `connect_repo_relays(state(addr))` | own repos' announced relays | manual | +| `RepoStore::subscribe_remote` | `subscribe_bootstrap(repo_filters)` | `BOOTSTRAP_RELAYS` | manual | +| `RepoStore::connect_announced_relays` | `connect_repo_relays(repo_filters)` | announced relays | manual | +| `RepoStore::run_refresh` root follow-ups | `subscribe_bootstrap(root_filters)` + `connect_repo_relays(root_filters)` | bootstrap + announced relays | manual | +| `Backend::sync_inbox` | `subscribe_bootstrap(notifications, authored_activity)` + `connect_repo_relays` over own announcements | bootstrap + own repos' relays | manual | +| `Backend::bootstrap_user` | `sync_bootstrap_only(grasp_list)` | `BOOTSTRAP_RELAYS` | manual | +| `ProfileStore::handle_requests` | `sync_bootstrap_only(metadata)` | `BOOTSTRAP_RELAYS` | manual | +| All publishes | `client.send_event(event).broadcast()` | pool write relays | no gossip | + +Consequences: + +- `INDEXER_RELAYS` are connected but receive no request: being `DISCOVERY`-only + they are excluded from every manual target, and no Auto request exists to + use them. They become useful only once an Auto request runs, or if a manual + request names them. +- The NIP-34 `p` tags on events do not influence fetching. `extract_public_keys` + reads the **filter**, not the events, and the repo filters carry no pubkeys + outside the announcement/state and author-deletion filters. +- The pool has no `max_relays` (`builder.rs` default `None`) and relays are + never removed. Any future Auto traffic accumulates GOSSIP relays for the + lifetime of the session. + +## 3. The NIP-34 `p` tag, per event kind + +The claim "activity events already carry a `p` tag pointing at the repository +owner" is true for the root kinds and false for kind-1111 comments: + +| Kind | Lowercase `p` | Source | +| --- | --- | --- | +| Issue 1621 | repository owner | `nostr/src/nips/nip34.rs::GitIssue` (L476-L484) | +| Patch 1617 | repository owner | `nip34.rs::GitPatch` (L575-L583); `RepoStore::publish_patch_series` builds patches by hand and adds `Tag::public_key(owner)` explicitly | +| PR 1618 | repository owner | `nip34.rs::GitPullRequest` (L654-L664) | +| PR update 1619 | repository owner | `nip34.rs::GitPullRequestUpdate` (L721-L730) | +| Status 1630-1633 | owner and root author | `RepoStore::set_status` / `publish_applied_status` push both explicitly | +| Comment 1111 | **parent author, not necessarily the owner** | `CommentBuilder` emits the root as uppercase `E`/`K`/`P` and the parent as lowercase `e`/`k`/`p` (`nostr/src/nips/nip22.rs::as_vec`). `RepoStore::comment_builder` passes the root as the parent for top-level comments, so `p` is the issue/PR author there. | + +So a single `Filter` with both `.coordinate(addr)` (`#a`) and +`.pubkey(maintainers)` (`#p`) would drop comments on roots authored by +non-maintainers. Keeping the existing `#a`-only filter and adding a second +`#p`-scoped filter is safe. The `#p`-scoped filter buys read-relay ("inbox") +targeting through gossip, not root coverage. + +## 4. Findings + +1. **Startup ordering hazard (medium).** `Backend::new` defers `bootstrap`, + which adds relays inside a background task; `RepoListStore::new` defers + `subscribe_remote`, which immediately background-spawns + `sync_bootstraps`. If the sync reaches `pool.sync` before the relays are in + the pool, every filter fails with `relay not found`. The global sync runs + only once per session and is not retried, so a lost race means no fresh + announcements, states or deletions are fetched until the next launch (the + persistent LMDB still supplies what previous sessions stored). Fix + options: add the bootstrap relays before spawning, or make the sync helpers + ensure their relays. +2. **Profile coverage vs. the indexers (medium).** Moving + `wss://profiles.nostr1.com` from `BOOTSTRAP_RELAYS` to `INDEXER_RELAYS` + (working tree) means `ProfileStore`, which syncs metadata over + `BOOTSTRAP_RELAYS`, no longer reaches it. `INDEXER_RELAYS` themselves are + currently inert (see above). Decide whether profile metadata should target + a profile indexer explicitly, and whether `profiles.nostr1.com` indexes + kind `10002` at all. +3. **State is owner-only (low, verify against NIP-34).** `filters::state` + and `repo_filters` fetch kind `30618` with `.author(addr.public_key)`, the + announcement author. GitWorkshop reads state from every confirmed + maintainer (`useResolvedRepository.ts`, `stateCandidates`). If co-maintainers + publish state events, `signed` ignores them. +4. **`profile.rs` comment wrong (cosmetic).** Synced events do emit + `NostrUpdate` on this SDK revision; the post-sync re-read in + `handle_requests` is defensive, not load-bearing. +5. **Progress fields are global (cosmetic).** Overlapping `sync_bootstraps` + calls share `sync_progress`; the last writer wins and completion of either + clears it. Progress counters from sequential filters accumulate correctly + because all filters share one watch channel. +6. **Background fetch failures are log-only (policy).** `connect_repo_relays` + and the per-filter sync errors are logged, not surfaced. Given the new + strategy adds more fetch paths, decide whether relay reachability should + feed the existing `last_error` / `last_warning` surfaces. + +## 5. Consequences for the fetching strategy + +- **Auto and Manual can coexist**, as asked. They are independent requests on + the same pool; events deduplicate in the database and only new events + notify. +- **The gossip store must be primed by an Auto request before it can resolve + anything.** `get_best_relays` is a pure store read, and + `ensure_gossip_public_keys_fresh` is private to the client. Sequence + Uncensored as: Auto request (freshness runs as a side effect) -> resolve + maintainers via `get_best_relays(pk, All { .. })` -> manual sync through + `connect_repo_relays`. +- **The manual leg already covers GitWorkshop's "outbox and inbox".** + `BestRelaySelection::All { read, write, hints, most_received }` returns both + directions, so the activity filter does not need `.pubkey(..)` for reach, + and the comment caveat in section 3 becomes optional rather than blocking. + Adding `.pubkey(maintainers)` still helps the Auto leg reach maintainer + inboxes and is worth doing as a second filter. +- **Curated can stay exactly what the code does today** (announced relays, + manual). The only decision is whether the per-repository REQ against + `BOOTSTRAP_RELAYS` remains in Curated or not. +- Publishing is unaffected; it already broadcasts to every pooled write relay + and does not consult the strategy. diff --git a/docs/event-fetching-strategy.md b/docs/event-fetching-strategy.md new file mode 100644 index 0000000..b94edc5 --- /dev/null +++ b/docs/event-fetching-strategy.md @@ -0,0 +1,296 @@ +# Proposal: Event Fetching Strategy (`Curated` / `Uncensored`) + +Status: proposal, not implemented. Verified against `docs/backend-audit.md`; +read that document first. + +References studied: + +- GitWorkshop `main` @ `420c0c3` + (`git clone nostr://npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr//gitworkshop`). +- rust-nostr @ `b230cecf9dbb38e0228e6fff4544ed9d261326fc` from `Cargo.lock`. + Every `nostr*` crate, including `nostr-gossip` and `nostr-gossip-memory`, + resolves to that single revision. + +## Goal + +Add a global setting controlling which relays `RepoStore` queries for a +repository's activity, mirroring GitWorkshop's "Event Fetching Strategy": + +- **Curated** (`repo`): only the relays declared in the repository + announcement. +- **Uncensored** (`outbox`): the repository's declared relays plus every + maintainer's relays, resolved through the NIP-65 outbox model. + +## GitWorkshop reference + +Source paths are relative to the cloned repository. + +- `src/services/settings.ts`: `RelayCurationMode = "repo" | "outbox"`, + persisted to `localStorage`, default `"outbox"` (the adjacent doc comment + wrongly says `repo`; the constant is authoritative). +- `src/pages/Settings.tsx` (`RelayCurationSection`, `CURATION_OPTIONS`): two + selectable cards, "Curated" and "Uncensored". +- `src/hooks/useResolvedRepository.ts` (layers 3-4): + - `repoRelayGroup` (`RepositoryRelayGroup`): the repo announcement's + `relays` tag. The **Curated** frontier. + - `extraRelaysForMaintainerMailboxCoverage`: a delta relay group built from + every maintainer's NIP-65 **outbox + inbox** relays, excluding relays + already in `repoRelayGroup`. The **Uncensored** addition. +- `src/services/nostr.ts`: + - `resolveMailboxes(pubkey)` reads NIP-65 kind `10002` with a 3 s timeout and + caps to `MAX_RESOLVED_RELAYS = 5` per direction. + - `nip34RepoLoader` / `nip34SupplementalRelayLoader` subscribe to the base + **and** extra groups in `outbox` mode, and additionally query each + discovered item's **author** inbox relays (`MAX_AUTHOR_INBOX_RELAYS = 3`). + - Deletions come from base + extra relays in `outbox` mode. + +## How the pinned Nostr SDK handles NIP-65 + +The SDK ships a full NIP-65 outbox model. `signed` configures it but, as of +the audit, no code path exercises it: every fetch uses an explicit relay list. + +- `crates/signed_nostr/src/backend.rs` builds the client with + `.gossip(NostrGossipMemory::unbounded())` and + `.gossip_config(GossipConfig::default().no_background_refresh())`. +- `nostr-sdk/src/client/builder.rs`: + - `GossipConfig { limits, allowed, sync/fetch timeouts, fetch_chunks, background_refresh }`. + `GossipRelayLimits` defaults: read 3, write 3, hint 1, most-used 1, + NIP-17 3, per user. `GossipAllowedRelays` gates onion/local/plain-TLS, not + read/write. + - `no_background_refresh()` disables the periodic refresher that re-fetches + NIP-65 lists for tracked and DB-seen keys. +- `nostr-sdk/src/client/api/req_target.rs`: `ReqTarget::auto` (from a `Filter`, + `Vec`, or `[Filter; N]`) vs `ReqTarget::manual` (from a relay map or + `(relay, filters)` pairs). +- `nostr-sdk/src/client/api/util.rs::build_targets` and + `nostr-sdk/src/client/api/sync.rs`: + - **Auto** target + gossip configured -> filters are broken down by NIP-65. + - **Manual** target (e.g. `client.sync(f).with(urls)`, or an explicit + `HashMap>`), or gossip unset -> no breakdown. +- `nostr-sdk/src/client/gossip/updater.rs::gossip_break_down_filter`: + - Extracts pubkeys via `Filter::extract_public_keys` + (`nostr/src/filter/mod.rs`), which reads **only `authors` and `#p`**. + - Ensures those pubkeys' NIP-65 lists are fresh (`ensure_gossip_public_keys_fresh` + syncs kind `10002` over `DISCOVERY | READ` relays), then breaks the filter + down (`gossip/resolver.rs::break_down_filter`): + - `authors` only -> each author's **write** relays (+ hints, + most-received). + - `#p` only -> each pubkey's **read** relays (+ hints, + most-received). + - both -> union of read + write relays. + - neither (`Other`) or no relay found (`Orphan`) -> the pool's **read** + relays (`pool/mod.rs::read_relay_urls`). + - Resolved gossip relays are added with `RelayCapabilities::GOSSIP` + (`relay/capabilities.rs`), which does **not** include `READ`. +- `gossip/nostr-gossip/src/lib.rs` exposes the public `NostrGossip` trait: + `get_best_relays(pubkey, BestRelaySelection, GossipAllowedRelays)`, plus + `process`. `NostrGossipMemory` (re-exported via + `nostr_gossip_memory::prelude`) implements it, so a retained handle can + resolve a pubkey's relays directly. It is a pure store read: it does not + fetch the kind `10002` itself. The store is primed by the freshness step of + an Auto request (or by any kind `10002` that arrives for another reason), + and marks a key outdated 24 h after its last fetch attempt. +- `client.send_event(event)` without `.broadcast()` is itself NIP-65 aware: + with gossip configured it resolves the author's outbox and the `p` tags' + inboxes. `signed` calls `.broadcast()` everywhere, so publishing goes to the + pool's write relays and is unaffected by this setting. + +### What this means for repository filters + +`RepoStore::repo_filters` mixes filter shapes. It is the **filter** (not the +event) that `extract_public_keys` reads, so filter tags decide what the SDK can +resolve: + +- announcement + state: `.author(owner).identifier(id)` -> has `authors`, so an + Auto request resolves the owner's **write** relays. +- `activity(addr)`: `.coordinate(addr)` only (`#a`). The events do carry the repo + owner in a lowercase `p` tag, per NIP-34 - issues `1621`, patches `1617`, PRs + `1618`, PR updates `1619`, statuses `1630..=1633` (see the SDK's + `nostr/src/nips/nip34.rs` builders and `RepoStore::set_status` / + `publish_patch_series`) - but the filter never asks for `p`, so + `extract_public_keys` is empty and the filter is classified `Other`. +- `deletions_for_repo(addr)`: one `.author(owner)` filter (owner write relays) + and one `.coordinate(addr)` filter (`Other`). + +Adding `.pubkey(owner)` (or all `effective_maintainers()`) to the activity +filter makes an Auto request resolve those pubkeys' **read** relays (the +`#p`-only branch, plus hints and most-received). Two caveats: + +- `Filter::pubkey` ANDs with `.coordinate`. Root kinds and statuses carry the + owner's `p`, per the SDK's `nostr/src/nips/nip34.rs` builders and + `RepoStore::set_status` / `publish_patch_series`, but kind-1111 comments set + `p` to the parent author (`nostr/src/nips/nip22.rs::as_vec` emits the root + as uppercase `E`/`K`/`P` and the parent as lowercase `e`/`k`/`p`), which for + a top-level comment is the issue/PR author, not the repository owner. A + single `#a` + `#p` filter can drop comments. Keep them as two filters (a `#a` + filter plus a `#p` filter, unioned in the database) or accept the loss. +- `#p` yields the **read** (inbox) relays only. GitWorkshop's extra group is + outbox **and** inbox, so covering the write side still needs an explicit + target. + +## Current behaviour in `signed` + +Global discovery is unchanged: `RepoListStore::subscribe_remote` +(`crates/signed_state/src/repos.rs`) syncs `all_announcements`, `all_states`, +and `deletions` from `BOOTSTRAP_RELAYS`. + +Per-repository fetching in `crates/signed_state/src/repo.rs`: + +- `RepoStore::subscribe_remote` -> `Backend::subscribe_bootstrap`, a one-shot + REQ of `repo_filters` on `BOOTSTRAP_RELAYS`. It passes an explicit + `HashMap<&str, Vec>`, i.e. a **Manual** target, so gossip is skipped. +- `RepoStore::connect_announced_relays` -> `Backend::connect_repo_relays`, + which connects the announcement's `relays` tag and negentropy-`sync`s + `repo_filters` with `.with(relays.iter())`, again a **Manual** target. +- `RepoStore::run_refresh` reads results back from the local database. + +Net effect: a repository's activity and per-repo deletions are fetched from the +global bootstrap relays and its announced relays. Although `signed` configures a +gossip store, **every current fetch path uses manual targets and bypasses the +SDK's NIP-65 handling**. + +## Proposed behaviour + +```mermaid +flowchart TD + A[RepoStore opens repo] --> B{Event fetching strategy} + B -->|Curated| C[Repo-declared relays\nmanual targets] + B -->|Uncensored| D[Resolve maintainers' relays\nvia NostrGossip] + D --> E[Repo-declared + maintainer relays] + C --> F[fetch repo_filters] + E --> F + F --> G[Local database] + G --> H[run_refresh] +``` + +- **Curated**: fetch `repo_filters` from the announcement's `relays` tag only. +- **Uncensored**: additionally fetch the same filters from every + `Announcement::effective_maintainers()`'s NIP-65 write + read relays. + +Announcements, state events, and global deletions keep arriving from the global +`RepoListStore` bootstrap sync in both modes. + +### Recommended: an Auto request plus a Manual request + +Both can run on the same `Client`. They are independent - the pool supports +concurrent subscriptions and syncs, and events deduplicate in the database. Use +each for what it is good at: + +1. **Auto**: pass filters straight to `client.subscribe(filters)` / + `client.sync(filter)` (no `.with(...)`). Broadening the author-scoped + filters (announcement, state, author-scoped deletions) to + `effective_maintainers()` resolves each maintainer's NIP-65 **write** relays; + adding `.pubkey(..)` to the activity filter (see above) resolves their + **read** relays. This request also drives `ensure_gossip_public_keys_fresh`, + populating the gossip store. That side effect is what makes step 2 possible + at all, so the Auto request must run before resolution; resolution is a + pure store read. +2. **Manual**: the activity filter cannot reach maintainers' **write** relays + through the Auto path (that branch needs `authors`), so cover the outbox side + explicitly: resolve each maintainer's relays from the retained gossip handle + via `get_best_relays(pk, BestRelaySelection::All { .. }, ...)`, then + negentropy-`sync` through the existing `Backend::connect_repo_relays`. `All` + returns the union of read, write, hint and most-received relays, i.e. + GitWorkshop's outbox **and** inbox. Because this leg targets the whole + filter, it also covers comments that an `#a` + `#p` filter would drop. + +Retain the gossip store handle so step 2 can resolve relays: + +```rust +let gossip = Arc::new(NostrGossipMemory::unbounded()); +let client = ClientBuilder::default().gossip(gossip.clone()) /* ... */ .build(); +``` + +Store `gossip` (as `Arc`) on `Backend`, then call +`gossip.get_best_relays(..)` per maintainer, union the results, drop relays +already in `repo_relays`, and track the rest in a new +`mailbox_relays: HashSet`. + +Prefer splitting by filter shape (author-scoped -> Auto, coordinate-scoped -> +Manual) to avoid duplicate REQs. Sending the full `repo_filters` through both is +also valid; the database dedups events, at the cost of re-querying overlapping +relays. + +### Variant: Auto only + +Drop the manual maintainer request and pass `repo_filters` directly to +`client.subscribe(filters)` / `client.sync(filter)`, adding `.pubkey(..)` to the +activity filter. Simpler, but activity then reaches only maintainers' **read** +relays (plus hints/most-received); their write relays never receive it. + +Recommendation: the Auto + Manual combination for `Uncensored`, matching +GitWorkshop's base + extra relay groups. + +## Setting and UI + +Add to `crates/settings/src/settings.rs`, following the bare-enum pattern of +`AppearanceMode`: + +```rust +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum EventFetchingStrategy { + /// Only the relays declared in the repository announcement. + #[default] + Curated, + /// Repository relays plus every maintainer's NIP-65 relays. + Uncensored, +} +``` + +Add `pub event_fetching: EventFetchingStrategy` to `Settings`, and a section to +`crates/workspace/src/views/sidebar/settings_dialog.rs` rendered from +`settings_view`, reusing the `Select` used by `appearance_section` (or +`setting_block` from `signed_ui` for a two-card layout closer to GitWorkshop). + +### Default + +Recommend **Uncensored**, matching GitWorkshop. Curated remains the +spam-resistant choice. Note the default changes what existing users fetch. + +## Files to change + +- `crates/settings/src/settings.rs` - enum and `Settings` field. +- `crates/workspace/src/views/sidebar/settings_dialog.rs` - new section. +- `crates/signed_nostr/src/backend.rs` - retain the `NostrGossipMemory` handle; + optionally reconsider `no_background_refresh`. +- `crates/signed_state/src/backend.rs` - store the gossip handle; expose a + maintainer-relay resolver using `get_best_relays`; reuse `connect_repo_relays`. +- `crates/signed_state/src/repo.rs` - strategy-aware `subscribe_remote` / + `connect_announced_relays`, new `connect_maintainer_relays`, new + `mailbox_relays` field. +- `crates/signed_core/src/filters.rs` - `relay_list(public_keys)` filter for the + freshness trigger. Note `ensure_gossip_public_keys_fresh` is private to the + client; an Auto request naming the pubkeys is the public way to trigger it, so + this helper is only useful as part of an Auto filter list. + +## Open questions / decisions + +1. **Activity `#p` shaping.** Add a second `.pubkey(maintainers)` activity + filter so the Auto leg reaches maintainer inboxes, or keep the activity + filter coordinate-only? The manual leg already reaches read and write relays + for the whole filter, so comments are covered either way; this only decides + how much the Auto leg contributes. +2. **Bootstrap REQ in Curated mode.** Recommended: drop it for this repository + (global `RepoListStore` still covers announcements, state, deletions). + Alternative: keep it and make the setting purely additive. +3. **Gossip freshness.** Trigger kind `10002` on demand per repo, or re-enable + the SDK background refresher? Background refresh is currently disabled in + `signed_nostr/src/backend.rs`. +4. **Runtime changes.** Re-fetch on `SettingsStore` edit (subscribe + `RepoStore`), or apply on next open? +5. **Per-item author inbox relays.** GitWorkshop also queries each discovered + item author's inbox relays in `outbox` mode. Larger change; propose a phase 2. + +## Testing + +- Unit-test maintainer-relay resolution: given maintainers and a gossip store + seeded with kind `10002`, assert the resolved, deduped relay set reaches + `connect_repo_relays`. +- Assert Curated uses only the announced relays. +- Keep the `settings` round-trip tests updated for the new field. + +## Phasing + +1. Setting, UI, and gossip-handle plumbing. +2. Curated wiring (bootstrap REQ gating) + Uncensored maintainer relay fetch. +3. Runtime re-fetch on setting change. +4. (Optional) per-item author inbox relays. -- 2.54.0 From 80d4ca754b41c316a37607b8cf70270610b004eb Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 26 Sep 2026 15:58:49 +0700 Subject: [PATCH 02/10] add strategy setting --- crates/settings/src/settings.rs | 22 ++ docs/event-fetching-strategy.md | 531 ++++++++++++++++++-------------- 2 files changed, 314 insertions(+), 239 deletions(-) diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index a9a7264..bdef5ec 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -18,6 +18,16 @@ pub enum AppearanceMode { Dark, } +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum EventFetchingStrategy { + /// Only the relays declared in the repository announcement. + Curated, + /// Repository relays plus every maintainer's NIP-65 relays. + #[default] + Uncensored, +} + /// Fields mirror the gpui-component `Theme` surface customized at startup. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[serde(default)] @@ -141,6 +151,7 @@ pub struct CreateRepositorySettings { #[serde(default)] pub struct Settings { pub appearance: AppearanceMode, + pub event_fetching: EventFetchingStrategy, pub theme: ThemeSettings, pub tab_bar: TabBarSettings, pub grasp_servers: GraspServersSettings, @@ -157,6 +168,7 @@ mod tests { fn json_roundtrip_preserves_everything() { let settings = Settings { appearance: AppearanceMode::Dark, + event_fetching: EventFetchingStrategy::Curated, theme: ThemeSettings { radius: 8.0, ..Default::default() @@ -178,9 +190,19 @@ mod tests { serde_json::from_str(r#"{"appearance": "dark", "theme": {"radius": 4.0}}"#).unwrap(); assert_eq!(settings.appearance, AppearanceMode::Dark); assert_eq!(settings.theme.radius, 4.0); + // Unset fields fall back to their defaults, including the fetching strategy. + assert_eq!(settings.event_fetching, EventFetchingStrategy::Uncensored); // The rest of the theme and the other groups keep their defaults. assert_eq!(settings.theme.light_theme, "Signed Light"); assert_eq!(settings.grasp_servers, GraspServersSettings::default()); assert_eq!(settings.create_repository.default_folder, None); } + + #[test] + fn event_fetching_uses_snake_case() { + let json = serde_json::to_string(&EventFetchingStrategy::Curated).unwrap(); + assert_eq!(json, r#""curated""#); + let parsed: EventFetchingStrategy = serde_json::from_str(r#""uncensored""#).unwrap(); + assert_eq!(parsed, EventFetchingStrategy::Uncensored); + } } diff --git a/docs/event-fetching-strategy.md b/docs/event-fetching-strategy.md index b94edc5..d267c9c 100644 --- a/docs/event-fetching-strategy.md +++ b/docs/event-fetching-strategy.md @@ -1,296 +1,349 @@ -# Proposal: Event Fetching Strategy (`Curated` / `Uncensored`) +# Plan: Event Fetching Strategy (`Curated` / `Uncensored`) -Status: proposal, not implemented. Verified against `docs/backend-audit.md`; -read that document first. +Status: implementation plan, not implemented. This document replaces the +earlier proposal of the same name; it keeps the verified background from it +and `docs/backend-audit.md` (authoritative for current behaviour). -References studied: +Cross-checked against: -- GitWorkshop `main` @ `420c0c3` - (`git clone nostr://npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr//gitworkshop`). -- rust-nostr @ `b230cecf9dbb38e0228e6fff4544ed9d261326fc` from `Cargo.lock`. - Every `nostr*` crate, including `nostr-gossip` and `nostr-gossip-memory`, - resolves to that single revision. +- rust-nostr at the revision from `Cargo.lock`, + `b230cecf9dbb38e0228e6fff4544ed9d261326fc` (local checkout + `~/.cargo/git/checkouts/nostr-619b808bb247a9ed/b230cec`). +- GitWorkshop at `420c0c3`. ## Goal -Add a global setting controlling which relays `RepoStore` queries for a -repository's activity, mirroring GitWorkshop's "Event Fetching Strategy": +Mirror GitWorkshop's "Event Fetching Strategy" for the per-repository fetches +in `RepoStore`: -- **Curated** (`repo`): only the relays declared in the repository - announcement. -- **Uncensored** (`outbox`): the repository's declared relays plus every - maintainer's relays, resolved through the NIP-65 outbox model. +- **Curated**: only the relays declared in the repository announcement. +- **Uncensored**: the repository's declared relays plus every maintainer's + NIP-65 relays. + +Global discovery is unchanged in both modes: `RepoListStore` keeps syncing +announcements, states and deletions from `BOOTSTRAP_RELAYS`. ## GitWorkshop reference -Source paths are relative to the cloned repository. - -- `src/services/settings.ts`: `RelayCurationMode = "repo" | "outbox"`, - persisted to `localStorage`, default `"outbox"` (the adjacent doc comment - wrongly says `repo`; the constant is authoritative). -- `src/pages/Settings.tsx` (`RelayCurationSection`, `CURATION_OPTIONS`): two - selectable cards, "Curated" and "Uncensored". -- `src/hooks/useResolvedRepository.ts` (layers 3-4): - - `repoRelayGroup` (`RepositoryRelayGroup`): the repo announcement's - `relays` tag. The **Curated** frontier. - - `extraRelaysForMaintainerMailboxCoverage`: a delta relay group built from - every maintainer's NIP-65 **outbox + inbox** relays, excluding relays - already in `repoRelayGroup`. The **Uncensored** addition. -- `src/services/nostr.ts`: - - `resolveMailboxes(pubkey)` reads NIP-65 kind `10002` with a 3 s timeout and - caps to `MAX_RESOLVED_RELAYS = 5` per direction. - - `nip34RepoLoader` / `nip34SupplementalRelayLoader` subscribe to the base - **and** extra groups in `outbox` mode, and additionally query each - discovered item's **author** inbox relays (`MAX_AUTHOR_INBOX_RELAYS = 3`). - - Deletions come from base + extra relays in `outbox` mode. - -## How the pinned Nostr SDK handles NIP-65 - -The SDK ships a full NIP-65 outbox model. `signed` configures it but, as of -the audit, no code path exercises it: every fetch uses an explicit relay list. - -- `crates/signed_nostr/src/backend.rs` builds the client with - `.gossip(NostrGossipMemory::unbounded())` and - `.gossip_config(GossipConfig::default().no_background_refresh())`. -- `nostr-sdk/src/client/builder.rs`: - - `GossipConfig { limits, allowed, sync/fetch timeouts, fetch_chunks, background_refresh }`. - `GossipRelayLimits` defaults: read 3, write 3, hint 1, most-used 1, - NIP-17 3, per user. `GossipAllowedRelays` gates onion/local/plain-TLS, not - read/write. - - `no_background_refresh()` disables the periodic refresher that re-fetches - NIP-65 lists for tracked and DB-seen keys. -- `nostr-sdk/src/client/api/req_target.rs`: `ReqTarget::auto` (from a `Filter`, - `Vec`, or `[Filter; N]`) vs `ReqTarget::manual` (from a relay map or - `(relay, filters)` pairs). -- `nostr-sdk/src/client/api/util.rs::build_targets` and - `nostr-sdk/src/client/api/sync.rs`: - - **Auto** target + gossip configured -> filters are broken down by NIP-65. - - **Manual** target (e.g. `client.sync(f).with(urls)`, or an explicit - `HashMap>`), or gossip unset -> no breakdown. -- `nostr-sdk/src/client/gossip/updater.rs::gossip_break_down_filter`: - - Extracts pubkeys via `Filter::extract_public_keys` - (`nostr/src/filter/mod.rs`), which reads **only `authors` and `#p`**. - - Ensures those pubkeys' NIP-65 lists are fresh (`ensure_gossip_public_keys_fresh` - syncs kind `10002` over `DISCOVERY | READ` relays), then breaks the filter - down (`gossip/resolver.rs::break_down_filter`): - - `authors` only -> each author's **write** relays (+ hints, + most-received). - - `#p` only -> each pubkey's **read** relays (+ hints, + most-received). - - both -> union of read + write relays. - - neither (`Other`) or no relay found (`Orphan`) -> the pool's **read** - relays (`pool/mod.rs::read_relay_urls`). - - Resolved gossip relays are added with `RelayCapabilities::GOSSIP` - (`relay/capabilities.rs`), which does **not** include `READ`. -- `gossip/nostr-gossip/src/lib.rs` exposes the public `NostrGossip` trait: - `get_best_relays(pubkey, BestRelaySelection, GossipAllowedRelays)`, plus - `process`. `NostrGossipMemory` (re-exported via - `nostr_gossip_memory::prelude`) implements it, so a retained handle can - resolve a pubkey's relays directly. It is a pure store read: it does not - fetch the kind `10002` itself. The store is primed by the freshness step of - an Auto request (or by any kind `10002` that arrives for another reason), - and marks a key outdated 24 h after its last fetch attempt. -- `client.send_event(event)` without `.broadcast()` is itself NIP-65 aware: - with gossip configured it resolves the author's outbox and the `p` tags' - inboxes. `signed` calls `.broadcast()` everywhere, so publishing goes to the - pool's write relays and is unaffected by this setting. - -### What this means for repository filters - -`RepoStore::repo_filters` mixes filter shapes. It is the **filter** (not the -event) that `extract_public_keys` reads, so filter tags decide what the SDK can -resolve: - -- announcement + state: `.author(owner).identifier(id)` -> has `authors`, so an - Auto request resolves the owner's **write** relays. -- `activity(addr)`: `.coordinate(addr)` only (`#a`). The events do carry the repo - owner in a lowercase `p` tag, per NIP-34 - issues `1621`, patches `1617`, PRs - `1618`, PR updates `1619`, statuses `1630..=1633` (see the SDK's - `nostr/src/nips/nip34.rs` builders and `RepoStore::set_status` / - `publish_patch_series`) - but the filter never asks for `p`, so - `extract_public_keys` is empty and the filter is classified `Other`. -- `deletions_for_repo(addr)`: one `.author(owner)` filter (owner write relays) - and one `.coordinate(addr)` filter (`Other`). - -Adding `.pubkey(owner)` (or all `effective_maintainers()`) to the activity -filter makes an Auto request resolve those pubkeys' **read** relays (the -`#p`-only branch, plus hints and most-received). Two caveats: - -- `Filter::pubkey` ANDs with `.coordinate`. Root kinds and statuses carry the - owner's `p`, per the SDK's `nostr/src/nips/nip34.rs` builders and - `RepoStore::set_status` / `publish_patch_series`, but kind-1111 comments set - `p` to the parent author (`nostr/src/nips/nip22.rs::as_vec` emits the root - as uppercase `E`/`K`/`P` and the parent as lowercase `e`/`k`/`p`), which for - a top-level comment is the issue/PR author, not the repository owner. A - single `#a` + `#p` filter can drop comments. Keep them as two filters (a `#a` - filter plus a `#p` filter, unioned in the database) or accept the loss. -- `#p` yields the **read** (inbox) relays only. GitWorkshop's extra group is - outbox **and** inbox, so covering the write side still needs an explicit - target. +- `src/services/settings.ts:197-210`: `RelayCurationMode = "repo" | "outbox"`, + persisted to `localStorage`, default `"outbox"` (Uncensored). +- `src/pages/Settings.tsx:85-100`: two selectable cards, "Curated" and + "Uncensored". +- `src/hooks/useResolvedRepository.ts`: `repoRelayGroup` is the announcement's + `relays` tag. `extraRelaysForMaintainerMailboxCoverage` is a delta group of + every maintainer's NIP-65 outbox + inbox relays, excluding relays already in + the repo group, capped at `MAX_MAILBOX_RELAYS_PER_USER = 3` per direction + (`addMailboxRelaysToGroup`). The pubkey set is the announcement chain's + `discoveryPubkeys` (maintainers, moderators, owner). +- Gating: `src/hooks/useNip34Loaders.ts:447` and + `src/pages/repo/RepoLayout.tsx:319-360` only subscribe the item loaders to + the maintainer group when the mode is `outbox`. Curated never touches + maintainer relays. ## Current behaviour in `signed` -Global discovery is unchanged: `RepoListStore::subscribe_remote` -(`crates/signed_state/src/repos.rs`) syncs `all_announcements`, `all_states`, -and `deletions` from `BOOTSTRAP_RELAYS`. +`crates/signed_state/src/repo.rs`: -Per-repository fetching in `crates/signed_state/src/repo.rs`: +- `subscribe_remote` -> `Backend::subscribe_bootstrap`: one-shot REQ of + `repo_filters` on `BOOTSTRAP_RELAYS` (manual target). +- `connect_announced_relays` -> `Backend::connect_repo_relays`: add and + connect the announcement's `relays` tag, then negentropy-`sync` + `repo_filters` (manual target). Deduped by the `repo_relays` set. Called + from `new`, `announce`, and on every announcement change in `run_refresh`. +- `run_refresh` also fetches comments (`filters::comments_for`) and statuses + (`filters::statuses_for`) for each newly seen root from bootstrap + + `repo_relays`. +- No fetch path uses NIP-65: the gossip store is configured but never + consulted (`docs/backend-audit.md`, section 2). -- `RepoStore::subscribe_remote` -> `Backend::subscribe_bootstrap`, a one-shot - REQ of `repo_filters` on `BOOTSTRAP_RELAYS`. It passes an explicit - `HashMap<&str, Vec>`, i.e. a **Manual** target, so gossip is skipped. -- `RepoStore::connect_announced_relays` -> `Backend::connect_repo_relays`, - which connects the announcement's `relays` tag and negentropy-`sync`s - `repo_filters` with `.with(relays.iter())`, again a **Manual** target. -- `RepoStore::run_refresh` reads results back from the local database. - -Net effect: a repository's activity and per-repo deletions are fetched from the -global bootstrap relays and its announced relays. Although `signed` configures a -gossip store, **every current fetch path uses manual targets and bypasses the -SDK's NIP-65 handling**. - -## Proposed behaviour +## Design ```mermaid flowchart TD - A[RepoStore opens repo] --> B{Event fetching strategy} - B -->|Curated| C[Repo-declared relays\nmanual targets] - B -->|Uncensored| D[Resolve maintainers' relays\nvia NostrGossip] - D --> E[Repo-declared + maintainer relays] - C --> F[fetch repo_filters] + A[RepoStore opens a repo] --> B{Event fetching strategy} + B -->|Curated| C[Manual sync to repo-declared relays] + B -->|Uncensored| D[Auto sync: SDK resolves maintainers' NIP-65 relays from authors and #p] --> E[Manual sync to repo-declared relays] + C --> F[(Local database)] + D --> F E --> F - F --> G[Local database] - G --> H[run_refresh] ``` -- **Curated**: fetch `repo_filters` from the announcement's `relays` tag only. -- **Uncensored**: additionally fetch the same filters from every - `Announcement::effective_maintainers()`'s NIP-65 write + read relays. +1. **Curated stays exactly what the code does today**: bootstrap REQ plus the + manual announced-relay sync. +2. **Uncensored adds one SDK Auto sync.** No relay URLs are resolved, stored, + or tracked, and no kind `10002` events are fetched or parsed by `signed`: + the SDK's NIP-65 gossip targeting resolves the maintainers' relays per + request (`client.sync(filter)` with no `.with(..)`, verified in + `nostr-sdk/src/client/api/sync.rs:151-166` and + `api/util.rs:12-29`). The existing manual announced-relay sync stays for + the repo's own relays in both modes. +3. **The filters must name the maintainers.** Gossip resolution is driven by + `Filter::extract_public_keys` (`nostr/src/filter/mod.rs:591`), which reads + only `authors` and the lowercase `#p` tag: -Announcements, state events, and global deletions keep arriving from the global -`RepoListStore` bootstrap sync in both modes. + | Filter shape | Auto target | + | --- | --- | + | `authors` only | each author's NIP-65 **write** relays | + | `#p` only | each pubkey's NIP-65 **read** relays | + | both | union of read and write relays | + | neither | the pool's read relays | -### Recommended: an Auto request plus a Manual request + Today's `repo_filters` resolve nothing for this purpose: the + announcement/state filters carry only the owner in `authors`, and the + activity filter is `#a`-only, so it is classified `Other` and falls back to + the pool's read relays. Uncensored therefore sends a separate + maintainer-shaped filter set to the Auto sync. -Both can run on the same `Client`. They are independent - the pool supports -concurrent subscriptions and syncs, and events deduplicate in the database. Use -each for what it is good at: +4. **Maintainer filter set** (Uncensored only): -1. **Auto**: pass filters straight to `client.subscribe(filters)` / - `client.sync(filter)` (no `.with(...)`). Broadening the author-scoped - filters (announcement, state, author-scoped deletions) to - `effective_maintainers()` resolves each maintainer's NIP-65 **write** relays; - adding `.pubkey(..)` to the activity filter (see above) resolves their - **read** relays. This request also drives `ensure_gossip_public_keys_fresh`, - populating the gossip store. That side effect is what makes step 2 possible - at all, so the Auto request must run before resolution; resolution is a - pure store read. -2. **Manual**: the activity filter cannot reach maintainers' **write** relays - through the Auto path (that branch needs `authors`), so cover the outbox side - explicitly: resolve each maintainer's relays from the retained gossip handle - via `get_best_relays(pk, BestRelaySelection::All { .. }, ...)`, then - negentropy-`sync` through the existing `Backend::connect_repo_relays`. `All` - returns the union of read, write, hint and most-received relays, i.e. - GitWorkshop's outbox **and** inbox. Because this leg targets the whole - filter, it also covers comments that an `#a` + `#p` filter would drop. + ```rust + /// Filters the SDK resolves through NIP-65 gossip in Uncensored mode. + /// + /// Gossip reads pubkeys from `authors` and the lowercase `#p` tag only, + /// so every filter names the owner and the maintainers. + 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); + } -Retain the gossip store handle so step 2 can resolve relays: + vec![ + // Announcement and state events, including co-maintainer states, + // resolved to write relays. + Filter::new() + .kinds([Kind::GitRepoAnnouncement, Kind::RepoState]) + .authors(pubkeys.clone()) + .identifier(addr.identifier.clone()), + // Activity tagging a maintainer, resolved to their read relays. + Filter::new() + .kinds(filters::ACTIVITY_KINDS) + .coordinate(addr) + .pubkeys(pubkeys.clone()), + // Activity authored by a maintainer, resolved to their write relays. + Filter::new() + .kinds(filters::ACTIVITY_KINDS) + .coordinate(addr) + .authors(pubkeys.clone()), + // Deletions authored by a maintainer, resolved to write relays. + Filter::new() + .kinds([Kind::EventDeletion, Kind::RequestToVanish]) + .authors(pubkeys), + ] + } + ``` -```rust -let gossip = Arc::new(NostrGossipMemory::unbounded()); -let client = ClientBuilder::default().gossip(gossip.clone()) /* ... */ .build(); -``` + `maintainers` is `Announcement::effective_maintainers()`, already computed + by `run_refresh`. Because the announcement/state filter names every + maintainer, co-maintainer state events (kind `30618`) land in the local + database. They are not displayed yet: `run_refresh` reads state through + the owner-only `filters::state` (`docs/backend-audit.md`, finding 3). -Store `gossip` (as `Arc`) on `Backend`, then call -`gossip.get_best_relays(..)` per maintainer, union the results, drop relays -already in `repo_relays`, and track the rest in a new -`mailbox_relays: HashSet`. +5. **The manual leg is unchanged**: `subscribe_bootstrap` plus + `connect_announced_relays` keep covering bootstrap and repo-declared + relays. Per-root follow-ups (`comments_for`, `statuses_for`) also stay + as-is. -Prefer splitting by filter shape (author-scoped -> Auto, coordinate-scoped -> -Manual) to avoid duplicate REQs. Sending the full `repo_filters` through both is -also valid; the database dedups events, at the cost of re-querying overlapping -relays. +6. **Default: Uncensored**, matching GitWorkshop + (`DEFAULT_RELAY_CURATION_MODE = "outbox"`). Flagged under "Decisions" + because it changes what existing users fetch. -### Variant: Auto only +### Coverage -Drop the manual maintainer request and pass `repo_filters` directly to -`client.subscribe(filters)` / `client.sync(filter)`, adding `.pubkey(..)` to the -activity filter. Simpler, but activity then reaches only maintainers' **read** -relays (plus hints/most-received); their write relays never receive it. +- The `#p` activity filter finds roots and statuses that tag a maintainer + (NIP-34 root events and statuses tag the owner) on the maintainers' read + relays. +- The `authors` activity filter finds maintainer-authored events (issues, + PRs, patches, comments, statuses) on their write relays. `signed`'s + comments and statuses carry the `a` tag + (`comment_builder`, `set_status`, `publish_applied_status`), so this filter + does not drop them. +- Comments whose parent author is not a maintainer are not matched by the + `#p` filter, but they are published to the parent author's inbox, not to a + maintainer's relays; maintainer-authored comments are still caught by the + `authors` filter. +- Not covered: status events without an `a` tag published to a maintainer's + outbox. Per-root `#e` filters carry no pubkeys, so they cannot resolve + through gossip; GitWorkshop covers these with per-item supplemental + queries (out of scope below). -Recommendation: the Auto + Manual combination for `Uncensored`, matching -GitWorkshop's base + extra relay groups. +## Changes -## Setting and UI +### 1. Setting, `crates/settings/src/settings.rs` -Add to `crates/settings/src/settings.rs`, following the bare-enum pattern of -`AppearanceMode`: +Follow the bare-enum pattern of `AppearanceMode`: ```rust +/// Which relays `RepoStore` queries for a repository's activity. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum EventFetchingStrategy { /// Only the relays declared in the repository announcement. - #[default] Curated, /// Repository relays plus every maintainer's NIP-65 relays. + #[default] Uncensored, } ``` -Add `pub event_fetching: EventFetchingStrategy` to `Settings`, and a section to -`crates/workspace/src/views/sidebar/settings_dialog.rs` rendered from -`settings_view`, reusing the `Select` used by `appearance_section` (or -`setting_block` from `signed_ui` for a two-card layout closer to GitWorkshop). +Add `pub event_fetching: EventFetchingStrategy` to `Settings`. -### Default +### 2. Settings UI, `crates/workspace/src/views/sidebar/settings_dialog.rs` -Recommend **Uncensored**, matching GitWorkshop. Curated remains the -spam-resistant choice. Note the default changes what existing users fetch. +- Options: `SelectOption::new("curated", "Curated")` and + `SelectOption::new("uncensored", "Uncensored")`. +- Add an `event_fetching: Entity>>` field to + `SettingsControls`, seeded from the persisted value, and a + `SelectEvent::Confirm` subscription that maps the value to the enum and + calls `store.edit(|settings| settings.event_fetching = strategy, cx)`. +- Add an `event_fetching_section` next to `appearance_section`, using + `setting_row` with the title "Event Fetching Strategy" and a description of + both modes, and insert it in `settings_view`. -## Files to change +### 3. Backend, `crates/signed_state/src/backend.rs` -- `crates/settings/src/settings.rs` - enum and `Settings` field. -- `crates/workspace/src/views/sidebar/settings_dialog.rs` - new section. -- `crates/signed_nostr/src/backend.rs` - retain the `NostrGossipMemory` handle; - optionally reconsider `no_background_refresh`. -- `crates/signed_state/src/backend.rs` - store the gossip handle; expose a - maintainer-relay resolver using `get_best_relays`; reuse `connect_repo_relays`. -- `crates/signed_state/src/repo.rs` - strategy-aware `subscribe_remote` / - `connect_announced_relays`, new `connect_maintainer_relays`, new - `mailbox_relays` field. -- `crates/signed_core/src/filters.rs` - `relay_list(public_keys)` filter for the - freshness trigger. Note `ensure_gossip_public_keys_fresh` is private to the - client; an Auto request naming the pubkeys is the public way to trigger it, so - this helper is only useful as part of an Auto filter list. +New method next to `connect_repo_relays`: -## Open questions / decisions +```rust +/// Sync filters through the SDK's NIP-65 gossip targeting. +/// +/// `client.sync(filter)` without `.with(..)` is an Auto request: the SDK +/// resolves each filter's `authors` and lowercase `#p` pubkeys to their +/// NIP-65 relays, connects them, and negentropy-syncs there. +pub fn sync_auto(&mut self, filters: Vec, cx: &mut Context) { + let client = self.client.clone(); -1. **Activity `#p` shaping.** Add a second `.pubkey(maintainers)` activity - filter so the Auto leg reaches maintainer inboxes, or keep the activity - filter coordinate-only? The manual leg already reaches read and write relays - for the whole filter, so comments are covered either way; this only decides - how much the Auto leg contributes. -2. **Bootstrap REQ in Curated mode.** Recommended: drop it for this repository - (global `RepoListStore` still covers announcements, state, deletions). - Alternative: keep it and make the setting purely additive. -3. **Gossip freshness.** Trigger kind `10002` on demand per repo, or re-enable - the SDK background refresher? Background refresh is currently disabled in - `signed_nostr/src/backend.rs`. -4. **Runtime changes.** Re-fetch on `SettingsStore` edit (subscribe - `RepoStore`), or apply on next open? -5. **Per-item author inbox relays.** GitWorkshop also queries each discovered - item author's inbox relays in `outbox` mode. Larger change; propose a phase 2. + cx.spawn(async move |_this, _cx| { + for filter in filters { + if let Err(e) = client.sync(filter).await { + log::warn!("gossip relay fetch failed: {e}"); + } + } + Ok::<(), Error>(()) + }) + .detach(); +} +``` -## Testing +Errors stay log-only, like `connect_repo_relays`; `BackendEvent::Synced` is +deliberately not emitted (it would refresh `RepoListStore` for repo-level +traffic). -- Unit-test maintainer-relay resolution: given maintainers and a gossip store - seeded with kind `10002`, assert the resolved, deduped relay set reaches - `connect_repo_relays`. -- Assert Curated uses only the announced relays. -- Keep the `settings` round-trip tests updated for the new field. +### 4. `RepoStore`, `crates/signed_state/src/repo.rs` + +New field, initialized empty in `new` and `new_local`: + +```rust +/// Maintainers already synced through gossip in Uncensored mode. +synced_maintainers: HashSet, +``` + +New methods after `connect_announced_relays`: `maintainer_filters` (listed +under Design) and + +```rust +/// In Uncensored mode, sync this repository's maintainer-shaped filters +/// through the SDK's NIP-65 gossip targeting. +fn sync_maintainer_relays(&mut self, maintainers: &[PublicKey], cx: &mut Context) { + let strategy = settings::SettingsStore::try_global(cx) + .map(|store| store.read(cx).settings().event_fetching) + .unwrap_or_default(); + if strategy != EventFetchingStrategy::Uncensored { + return; + } + + let Some(addr) = self.addr.clone() else { + return; + }; + + if !maintainers + .iter() + .any(|pk| !self.synced_maintainers.contains(pk)) + { + return; + } + self.synced_maintainers.extend(maintainers.iter().copied()); + + let filters = Self::maintainer_filters(&addr, maintainers); + let backend = Backend::global(cx); + backend.update(cx, |backend, cx| backend.sync_auto(filters, cx)); +} +``` + +Hook into `run_refresh`, in the foreground update after +`connect_announced_relays`: + +```rust +let maintainers = this + .announcement + .as_ref() + .map(Announcement::effective_maintainers) + .unwrap_or_default(); +this.sync_maintainer_relays(&maintainers, cx); +``` + +Notes: + +- `SettingsStore::try_global` keeps wasm safe: the settings store is only + installed by the desktop app (`desktop/src/main.rs:21`). +- No new crate dependencies: `signed_state` already depends on `settings` and + `nostr_sdk`. + +## Tests + +- `crates/settings`: extend `json_roundtrip_preserves_everything` and + `partial_json_merges_with_defaults` for `event_fetching` (snake_case + values, default). +- `crates/signed_state/src/repo.rs`, `mod tests`: unit-test + `maintainer_filters`: every filter names the owner and maintainers via + `authors` or `#p` (the announcement/state filter included), activity + filters carry the `#a` coordinate, and the owner is added for a + subordinate fork. +- `cargo test -p settings -p signed_state` and + `cargo check -p signed_workspace` for the UI. +- Manual smoke: open a repository whose activity exists only on a + maintainer's relays (not on the announced relays or bootstrap) in both + modes. + +## Decisions + +1. **Default.** Uncensored, to match GitWorkshop. Curated preserves today's + relay traffic; flipping the default is a one-line change. +2. **Bootstrap REQ stays in Curated.** It is the index path that keeps + repositories with unreachable announced relays usable. Strict GitWorkshop + parity (repo relays only in Curated) is possible later but is a behaviour + change unrelated to the option itself. +3. **Read and write coverage is approximated by two activity filters** (`#p` + and `authors`) rather than a resolved "all maintainer relays" set. The SDK + resolves the relay sets per filter shape; `signed` stores no relay URLs. +4. **The announcement/state Auto filter names every maintainer plus the + owner.** This also fetches co-maintainer state events into the local + database; showing them is a separate display-side change. +5. **Runtime switching.** `sync_maintainer_relays` reads the setting on every + `run_refresh`, so switching to Uncensored applies at the next refresh + without a restart. Switching back stops new Auto syncs but does not undo + relays already resolved by the gossip pool. +6. **Gossip pool growth.** Auto requests add resolved relays with + `RelayCapabilities::GOSSIP` and never remove them for the session + (`docs/backend-audit.md`); accepted, as the SDK is designed this way. +7. **Errors stay log-only**, consistent with existing background fetches. + +## Out of scope + +- Status events without an `a` tag published to maintainer outboxes + (GitWorkshop's per-item supplemental loader). +- Per-item author inbox relays + (`src/services/nostr.ts:1042,1070`, `MAX_AUTHOR_INBOX_RELAYS = 3`). +- Co-maintainer state display: co-maintainer states are fetched (decision 4) + but `run_refresh` still reads state through the owner-only + `filters::state` (`docs/backend-audit.md`, finding 3). +- Gating or replacing the bootstrap REQ in Curated mode. ## Phasing -1. Setting, UI, and gossip-handle plumbing. -2. Curated wiring (bootstrap REQ gating) + Uncensored maintainer relay fetch. -3. Runtime re-fetch on setting change. -4. (Optional) per-item author inbox relays. +1. Setting enum, field and settings tests. **Done.** +2. Settings UI control. +3. `Backend::sync_auto` and `RepoStore::sync_maintainer_relays` with the + maintainer filter set, plus the filter unit test. +4. `cargo test` / `cargo check`, then a manual smoke test. -- 2.54.0 From 3a70bb7228ec7481aef0b2303be4ccb52c0e1a2b Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sat, 26 Sep 2026 16:18:28 +0700 Subject: [PATCH 03/10] add fetch strategy --- CHANGELOG.md | 1 + crates/signed_state/src/backend.rs | 15 + crates/signed_state/src/repo.rs | 138 ++++++- .../src/views/sidebar/settings_dialog.rs | 46 ++- docs/backend-audit.md | 217 ----------- docs/event-fetching-strategy.md | 349 ------------------ 6 files changed, 197 insertions(+), 569 deletions(-) delete mode 100644 docs/backend-audit.md delete mode 100644 docs/event-fetching-strategy.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e8d330..1981338 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Show an avatar in each panel's tab, using the repository owner's profile picture when set and a pixel avatar otherwise - Add a Tab Bar setting to hide the previous/next tab buttons, hidden by default +- Add an Event Fetching Strategy setting, fetching a repository's activity from its announced relays only (Curated) or from every maintainer's relays as well (Uncensored, the default) ### Changed diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 156606c..0d6998b 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -1192,6 +1192,21 @@ 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(); + + cx.spawn(async move |_this, _cx| { + for filter in filters { + if let Err(e) = client.sync(filter).await { + log::warn!("gossip relay fetch failed: {e}"); + } + } + Ok::<(), Error>(()) + }) + .detach(); + } + pub fn subscribe_bootstrap(&mut self, filters: Vec, cx: &mut Context) { let client = self.client.clone(); diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 9ba4a81..c024078 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -7,6 +7,7 @@ use bitcoin_hashes::sha1::Hash as Sha1Hash; use gpui::{App, AppContext, AsyncApp, Context, SharedString, Subscription, Task, WeakEntity}; use nostr::event::IntoEventBuilder; use nostr_sdk::prelude::*; +use settings::{EventFetchingStrategy, SettingsStore}; use signed_core::{ Announcement, Deletions, RepoAddr, RepoStatus, filters, parse_state, pull_request_patch, pull_request_patches, @@ -84,6 +85,10 @@ pub struct RepoStore { /// /// The per-root fetches cover 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, refresh: RefreshGate, /// Backend subscription of an announced repository. `None` while local-only. _subscription: Option, @@ -132,6 +137,7 @@ impl RepoStore { cloning: false, repo_relays: HashSet::new(), root_fetches: HashSet::new(), + synced_maintainers: HashSet::new(), refresh: RefreshGate::default(), _subscription: Some(subscription), } @@ -160,6 +166,7 @@ impl RepoStore { cloning: false, repo_relays: HashSet::new(), root_fetches: HashSet::new(), + synced_maintainers: HashSet::new(), refresh: RefreshGate::default(), _subscription: None, } @@ -295,6 +302,67 @@ impl RepoStore { }); } + /// Filters the SDK resolves through NIP-65 gossip in Uncensored mode. + 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.clone()), + // Activity tagging a maintainer, resolved to their read relays. + Filter::new() + .kinds(filters::ACTIVITY_KINDS) + .coordinate(addr) + .pubkeys(pubkeys.clone()), + // Activity authored by a maintainer, resolved to their write relays. + Filter::new() + .kinds(filters::ACTIVITY_KINDS) + .coordinate(addr) + .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) + .unwrap_or_default(); + if strategy != EventFetchingStrategy::Uncensored { + return; + } + + let Some(addr) = self.addr.clone() else { + return; + }; + + if !maintainers + .iter() + .any(|public_key| !self.synced_maintainers.contains(public_key)) + { + return; + } + self.synced_maintainers.extend(maintainers.iter().copied()); + + let filters = Self::maintainer_filters(&addr, maintainers); + let backend = Backend::global(cx); + + backend.update(cx, |backend, cx| { + backend.sync_auto(filters, cx); + }); + } + fn subscribe_remote(&mut self, cx: &mut Context) { let Some(addr) = self.addr.clone() else { return; @@ -489,7 +557,6 @@ impl RepoStore { } // The announcement may list relays for this repository's activity. - // Connect to any we have not fetched from yet. let relays = this .announcement .as_ref() @@ -498,6 +565,15 @@ impl RepoStore { this.connect_announced_relays(&relays, cx); + // Uncensored mode also covers the maintainers' NIP-65 relays. + let maintainers = this + .announcement + .as_ref() + .map(Announcement::effective_maintainers) + .unwrap_or_default(); + + this.sync_maintainer_relays(&maintainers, cx); + if let Some((_, head)) = state { this.head = head; } @@ -1734,9 +1810,11 @@ fn comment_builder( #[cfg(test)] mod tests { + use std::collections::HashSet; + use nostr_sdk::prelude::*; - use super::{comment_builder, patch_current_commit}; + use super::{RepoStore, comment_builder, patch_current_commit}; #[test] fn parses_format_patch_header() { @@ -1782,4 +1860,60 @@ mod tests { // Signed's own `references_root` must keep matching the comment. assert!(signed_core::references_root(&event, &root.id)); } + + #[test] + fn maintainer_filters_name_owner_and_maintainers() { + let owner = Keys::generate().public_key(); + let maintainer = Keys::generate().public_key(); + let addr = Coordinate::new(Kind::GitRepoAnnouncement, owner).identifier("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 + .generic_tags + .get(&SingleLetterTag::LOWERCASE_P) + .into_iter() + .flatten() + .filter_map(|value| PublicKey::from_hex(value).ok()); + authors.chain(p_tag).collect() + }; + + // Announcement and state events, scoped to the repository identifier. + let announcement = &filters[0]; + assert_eq!(named(announcement), expected); + assert!( + announcement + .generic_tags + .contains_key(&SingleLetterTag::LOWERCASE_D) + ); + + // Activity filters, scoped to the repository coordinate. + for filter in &filters[1..3] { + assert_eq!(named(filter), expected); + assert!( + filter + .generic_tags + .contains_key(&SingleLetterTag::LOWERCASE_A) + ); + } + + // Deletions, named by author only. + let deletions = &filters[3]; + assert_eq!( + deletions + .authors + .clone() + .unwrap_or_default() + .into_iter() + .collect::>(), + expected + ); + } } diff --git a/crates/workspace/src/views/sidebar/settings_dialog.rs b/crates/workspace/src/views/sidebar/settings_dialog.rs index 52a129b..e986a3c 100644 --- a/crates/workspace/src/views/sidebar/settings_dialog.rs +++ b/crates/workspace/src/views/sidebar/settings_dialog.rs @@ -18,7 +18,7 @@ use gpui_component::{ ActiveTheme, IconName, IndexPath, Sizable, Theme, ThemeMode, ThemeRegistry, WindowExt, h_flex, v_flex, }; -use settings::{AppearanceMode, Settings, SettingsStore}; +use settings::{AppearanceMode, EventFetchingStrategy, Settings, SettingsStore}; use signed_ui::{SelectOption, setting_block, setting_row}; use super::{normalize_server, server_host}; @@ -52,6 +52,7 @@ fn theme_options(cx: &App) -> (Vec, Vec) { /// Created once when the dialog opens, so control state survives re-renders. struct SettingsControls { appearance: Entity>>, + event_fetching: Entity>>, light_theme: Entity>>, dark_theme: Entity>>, font_size: Entity, @@ -89,6 +90,23 @@ impl SettingsControls { ) }); + let event_fetching_options = vec![ + SelectOption::new("curated", "Curated"), + SelectOption::new("uncensored", "Uncensored"), + ]; + let event_fetching_value = match settings.event_fetching { + EventFetchingStrategy::Curated => "curated", + EventFetchingStrategy::Uncensored => "uncensored", + }; + let event_fetching = cx.new(|cx| { + SelectState::new( + event_fetching_options.clone(), + selected_index(&event_fetching_options, event_fetching_value), + window, + cx, + ) + }); + let (light_options, dark_options) = theme_options(cx); let light_theme = cx.new(|cx| { SelectState::new( @@ -147,6 +165,19 @@ impl SettingsControls { } })); + subscriptions.push(cx.subscribe(&event_fetching, |_, event, cx| { + if let SelectEvent::Confirm(Some(value)) = event { + let strategy = match value.as_ref() { + "curated" => EventFetchingStrategy::Curated, + _ => EventFetchingStrategy::Uncensored, + }; + let store = SettingsStore::global(cx); + store.update(cx, |store, cx| { + store.edit(|settings| settings.event_fetching = strategy, cx); + }); + } + })); + subscriptions.push(cx.subscribe(&light_theme, |_, event, cx| { if let SelectEvent::Confirm(Some(value)) = event { let store = SettingsStore::global(cx); @@ -252,6 +283,7 @@ impl SettingsControls { Self { appearance, + event_fetching, light_theme, dark_theme, font_size, @@ -294,6 +326,8 @@ fn settings_view(controls: &SettingsControls, cx: &mut App) -> impl IntoElement .child(Separator::horizontal()) .child(grasp_servers_section(&settings, controls, cx)) .child(Separator::horizontal()) + .child(event_fetching_section(controls, cx)) + .child(Separator::horizontal()) .child(repositories_section(&settings, controls, cx)) } @@ -306,6 +340,16 @@ fn appearance_section(controls: &SettingsControls, cx: &App) -> impl IntoElement )) } +/// Which relays repository activity is fetched from. +fn event_fetching_section(controls: &SettingsControls, cx: &App) -> impl IntoElement { + v_flex().w_full().gap_3().child(setting_row( + cx, + "Event Fetching Strategy", + "Curated fetches events from relays in the repository's announcement. Uncensored also fetches from every maintainer's relays.", + Select::new(&controls.event_fetching).w_full(), + )) +} + fn theme_section(settings: &Settings, controls: &SettingsControls, cx: &App) -> impl IntoElement { v_flex() .gap_3() diff --git a/docs/backend-audit.md b/docs/backend-audit.md deleted file mode 100644 index 0d0969a..0000000 --- a/docs/backend-audit.md +++ /dev/null @@ -1,217 +0,0 @@ -# Backend audit: relays, sync and NIP-65 - -Status: audit of the current code, done before implementing -`docs/event-fetching-strategy.md`. Where this document and the first draft of -that proposal disagree, this document is authoritative. - -## Scope and method - -Crates read for relay and network behaviour: - -- `signed_state`: `backend`, `repo`, `repos`, `inbox`, `profile`, `checkouts`, - `local_repos`, `refresh`, `git_store`. -- `signed_nostr`: `backend`, `signer`, `update`. -- `signed_core`: `filters`, `deletions`, `model`, `state`, `status`, `inbox`. -- Relay touchpoints of `signed_git`, `settings`, `paths`, `utils`. - -Cross-checked against: - -- rust-nostr at the revision from `Cargo.lock`, - `b230cecf9dbb38e0228e6fff4544ed9d261326fc`. **All** `nostr*` crates - (`nostr`, `nostr-database`, `nostr-gossip`, `nostr-gossip-memory`, - `nostr-lmdb`, `nostr-sdk`) resolve to that single revision. The local - checkout is `~/.cargo/git/checkouts/nostr-619b808bb247a9ed/b230cec`. -- GitWorkshop, cloned with `ngit` from - `nostr://npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/gitworkshop` - at revision `420c0c3`. - -Paths below are relative to those checkouts unless prefixed with -`crates/`, which means this repository. - -## 1. Verified SDK behaviour - -### Target selection - -- `nostr-sdk/src/client/api/req_target.rs`: `ReqTarget::auto` wraps a bare - filter list; `ReqTarget::manual` wraps a relay map. `From>>` - (L91) makes an explicit map a **manual** request. -- `nostr-sdk/src/client/api/util.rs::build_targets`: - - Auto + gossip configured -> `gossip_break_down_filters`. - - Manual -> the map is used as-is, gossip is skipped. -- `nostr-sdk/src/client/api/subscribe.rs` and `api/sync.rs` both go through - these rules. `client.sync(filter)` with no `.with(...)` uses gossip; - `client.sync(filter).with(urls)` does not. -- `pool.sync` errors with `relay not found` when a targeted relay has not been - added to the pool. Targets are not implicitly added. - -### Gossip breakdown - -- `nostr-sdk/src/client/gossip/updater.rs::gossip_break_down_filter` (L412) - extracts pubkeys via `Filter::extract_public_keys` - (`nostr/src/filter/mod.rs` L591): **only `authors` and the lowercase `#p` - tag**. It first calls `ensure_gossip_public_keys_fresh`, then breaks the - filter down, then adds every resolved relay to the pool with - `RelayCapabilities::GOSSIP`. -- Freshness (`updater.rs` L381-L410, L164-L185) negentropy-syncs kind `10002` - for the extracted pubkeys over the pool's `DISCOVERY | READ` relays. This is - the only automatic way the gossip store learns kind `10002`. -- `nostr-sdk/src/client/gossip/resolver.rs::break_down_filter` (L93): - - `authors` only -> each author's **write** relays, plus hint and - most-received relays. - - `#p` only -> each pubkey's **read** relays, plus hint and most-received. - - both -> the union of read and write relays, keeping the filter unchanged. - - neither (`Other`), or no relay found (`Orphan`) -> the pool's **read** - relays (`read_relay_urls`). -- `relay/capabilities.rs`: `GOSSIP` is its own bit. `pool.read_relay_urls` / - `write_relay_urls` / `relay_urls_with_any_cap` use a raw bit test - (`filter_relays_with_any_cap`), so GOSSIP-only relays are excluded from - broadcast and from the `Other`/`Orphan` fallback. Per-relay send checks use - `can_read`/`can_write`, which do include GOSSIP, so gossip-resolved targets - work when named explicitly. -- `GossipConfig::default()` (`builder.rs` L30-L120): limits read 3, write 3, - hints 1, most-received 1, NIP-17 3 per user; `background_refresh` on by - default, disabled by `.no_background_refresh()`. -- `NostrGossipMemory` reads only the first 7 entries of a kind `10002` - (`MAX_NIP65_SIZE`) and stops tracking new hint/most-received relays once a - pubkey is near `MAX_RELAYS_PER_PK` (7). `get_best_relays(pk, selection, allowed)` - (public trait, `gossip/nostr-gossip/src/lib.rs`) reads whatever is in the - store, sorted by received-event count then recency. It does **not** fetch - kind `10002` itself. The store is primed by the freshness step of an Auto - request (or by any kind `10002` that arrives for another reason), and marks - a key outdated 24 h after its last fetch attempt. - -### Publishing - -- `client.send_event(event)` with **no** policy and gossip configured targets - NIP-65: it triggers the same freshness, then sends to the author's outbox - and every `p`-tagged pubkey's inbox (`send_event.rs::gossip_prepare_urls`). -- `client.send_event(event).broadcast()` overrides this to the pool's write - relays only (`send_event.rs` L428-L430). `signed` uses `broadcast()` - everywhere. - -### Notifications - -- Every message from a relay passes `relay/inner.rs::handle_event_msg`, which - saves new events and emits `ClientNotification::Event`. This includes events - received by a negentropy **sync** (the sync's down subscription is - registered as an auto-closing subscription, and the events travel the normal - message path). Events already in the database do not re-notify. -- The comment in `crates/signed_state/src/profile.rs` claiming synced events - produce no `NostrUpdate` is inaccurate for this revision. The re-read after - a sync is harmless, but the comment should not be relied on. - -## 2. What `signed` actually does today - -Every fetch path is manual. There is no Auto request anywhere in the -workspace, so `gossip_break_down_filter` and `ensure_gossip_public_keys_fresh` -never run. The gossip store is configured and passively fed -(`handle_event_msg` calls `gossip.process` for every received event), but it -is never consulted for targeting. - -| Call site | Request | Target relays | Target kind | -| --- | --- | --- | --- | -| `Backend::bootstrap` | `add_relay` + connect | `BOOTSTRAP_RELAYS` (READ\|WRITE), `INDEXER_RELAYS` (`DISCOVERY`) | n/a | -| `Backend::subscribe_bootstrap` -> `subscribe_bootstrap_only` | `client.subscribe(HashMap<&str, Vec>)`, `ExitOnEOSE`, 10 s timeout | `BOOTSTRAP_RELAYS` | manual | -| `Backend::sync_bootstraps` -> `sync_bootstrap_only` | `client.sync(filter).with(BOOTSTRAP_RELAYS)` | `BOOTSTRAP_RELAYS` | manual | -| `Backend::connect_repo_relays` | `add_relay().and_connect()` then `client.sync(filter).with(relays)` | announcement `relays` tag | manual | -| `RepoListStore::subscribe_remote` | `sync_bootstraps(all_announcements, all_states, deletions)` | `BOOTSTRAP_RELAYS` | manual | -| `RepoListStore::sync_own_repo_states` | `connect_repo_relays(state(addr))` | own repos' announced relays | manual | -| `RepoStore::subscribe_remote` | `subscribe_bootstrap(repo_filters)` | `BOOTSTRAP_RELAYS` | manual | -| `RepoStore::connect_announced_relays` | `connect_repo_relays(repo_filters)` | announced relays | manual | -| `RepoStore::run_refresh` root follow-ups | `subscribe_bootstrap(root_filters)` + `connect_repo_relays(root_filters)` | bootstrap + announced relays | manual | -| `Backend::sync_inbox` | `subscribe_bootstrap(notifications, authored_activity)` + `connect_repo_relays` over own announcements | bootstrap + own repos' relays | manual | -| `Backend::bootstrap_user` | `sync_bootstrap_only(grasp_list)` | `BOOTSTRAP_RELAYS` | manual | -| `ProfileStore::handle_requests` | `sync_bootstrap_only(metadata)` | `BOOTSTRAP_RELAYS` | manual | -| All publishes | `client.send_event(event).broadcast()` | pool write relays | no gossip | - -Consequences: - -- `INDEXER_RELAYS` are connected but receive no request: being `DISCOVERY`-only - they are excluded from every manual target, and no Auto request exists to - use them. They become useful only once an Auto request runs, or if a manual - request names them. -- The NIP-34 `p` tags on events do not influence fetching. `extract_public_keys` - reads the **filter**, not the events, and the repo filters carry no pubkeys - outside the announcement/state and author-deletion filters. -- The pool has no `max_relays` (`builder.rs` default `None`) and relays are - never removed. Any future Auto traffic accumulates GOSSIP relays for the - lifetime of the session. - -## 3. The NIP-34 `p` tag, per event kind - -The claim "activity events already carry a `p` tag pointing at the repository -owner" is true for the root kinds and false for kind-1111 comments: - -| Kind | Lowercase `p` | Source | -| --- | --- | --- | -| Issue 1621 | repository owner | `nostr/src/nips/nip34.rs::GitIssue` (L476-L484) | -| Patch 1617 | repository owner | `nip34.rs::GitPatch` (L575-L583); `RepoStore::publish_patch_series` builds patches by hand and adds `Tag::public_key(owner)` explicitly | -| PR 1618 | repository owner | `nip34.rs::GitPullRequest` (L654-L664) | -| PR update 1619 | repository owner | `nip34.rs::GitPullRequestUpdate` (L721-L730) | -| Status 1630-1633 | owner and root author | `RepoStore::set_status` / `publish_applied_status` push both explicitly | -| Comment 1111 | **parent author, not necessarily the owner** | `CommentBuilder` emits the root as uppercase `E`/`K`/`P` and the parent as lowercase `e`/`k`/`p` (`nostr/src/nips/nip22.rs::as_vec`). `RepoStore::comment_builder` passes the root as the parent for top-level comments, so `p` is the issue/PR author there. | - -So a single `Filter` with both `.coordinate(addr)` (`#a`) and -`.pubkey(maintainers)` (`#p`) would drop comments on roots authored by -non-maintainers. Keeping the existing `#a`-only filter and adding a second -`#p`-scoped filter is safe. The `#p`-scoped filter buys read-relay ("inbox") -targeting through gossip, not root coverage. - -## 4. Findings - -1. **Startup ordering hazard (medium).** `Backend::new` defers `bootstrap`, - which adds relays inside a background task; `RepoListStore::new` defers - `subscribe_remote`, which immediately background-spawns - `sync_bootstraps`. If the sync reaches `pool.sync` before the relays are in - the pool, every filter fails with `relay not found`. The global sync runs - only once per session and is not retried, so a lost race means no fresh - announcements, states or deletions are fetched until the next launch (the - persistent LMDB still supplies what previous sessions stored). Fix - options: add the bootstrap relays before spawning, or make the sync helpers - ensure their relays. -2. **Profile coverage vs. the indexers (medium).** Moving - `wss://profiles.nostr1.com` from `BOOTSTRAP_RELAYS` to `INDEXER_RELAYS` - (working tree) means `ProfileStore`, which syncs metadata over - `BOOTSTRAP_RELAYS`, no longer reaches it. `INDEXER_RELAYS` themselves are - currently inert (see above). Decide whether profile metadata should target - a profile indexer explicitly, and whether `profiles.nostr1.com` indexes - kind `10002` at all. -3. **State is owner-only (low, verify against NIP-34).** `filters::state` - and `repo_filters` fetch kind `30618` with `.author(addr.public_key)`, the - announcement author. GitWorkshop reads state from every confirmed - maintainer (`useResolvedRepository.ts`, `stateCandidates`). If co-maintainers - publish state events, `signed` ignores them. -4. **`profile.rs` comment wrong (cosmetic).** Synced events do emit - `NostrUpdate` on this SDK revision; the post-sync re-read in - `handle_requests` is defensive, not load-bearing. -5. **Progress fields are global (cosmetic).** Overlapping `sync_bootstraps` - calls share `sync_progress`; the last writer wins and completion of either - clears it. Progress counters from sequential filters accumulate correctly - because all filters share one watch channel. -6. **Background fetch failures are log-only (policy).** `connect_repo_relays` - and the per-filter sync errors are logged, not surfaced. Given the new - strategy adds more fetch paths, decide whether relay reachability should - feed the existing `last_error` / `last_warning` surfaces. - -## 5. Consequences for the fetching strategy - -- **Auto and Manual can coexist**, as asked. They are independent requests on - the same pool; events deduplicate in the database and only new events - notify. -- **The gossip store must be primed by an Auto request before it can resolve - anything.** `get_best_relays` is a pure store read, and - `ensure_gossip_public_keys_fresh` is private to the client. Sequence - Uncensored as: Auto request (freshness runs as a side effect) -> resolve - maintainers via `get_best_relays(pk, All { .. })` -> manual sync through - `connect_repo_relays`. -- **The manual leg already covers GitWorkshop's "outbox and inbox".** - `BestRelaySelection::All { read, write, hints, most_received }` returns both - directions, so the activity filter does not need `.pubkey(..)` for reach, - and the comment caveat in section 3 becomes optional rather than blocking. - Adding `.pubkey(maintainers)` still helps the Auto leg reach maintainer - inboxes and is worth doing as a second filter. -- **Curated can stay exactly what the code does today** (announced relays, - manual). The only decision is whether the per-repository REQ against - `BOOTSTRAP_RELAYS` remains in Curated or not. -- Publishing is unaffected; it already broadcasts to every pooled write relay - and does not consult the strategy. diff --git a/docs/event-fetching-strategy.md b/docs/event-fetching-strategy.md deleted file mode 100644 index d267c9c..0000000 --- a/docs/event-fetching-strategy.md +++ /dev/null @@ -1,349 +0,0 @@ -# Plan: Event Fetching Strategy (`Curated` / `Uncensored`) - -Status: implementation plan, not implemented. This document replaces the -earlier proposal of the same name; it keeps the verified background from it -and `docs/backend-audit.md` (authoritative for current behaviour). - -Cross-checked against: - -- rust-nostr at the revision from `Cargo.lock`, - `b230cecf9dbb38e0228e6fff4544ed9d261326fc` (local checkout - `~/.cargo/git/checkouts/nostr-619b808bb247a9ed/b230cec`). -- GitWorkshop at `420c0c3`. - -## Goal - -Mirror GitWorkshop's "Event Fetching Strategy" for the per-repository fetches -in `RepoStore`: - -- **Curated**: only the relays declared in the repository announcement. -- **Uncensored**: the repository's declared relays plus every maintainer's - NIP-65 relays. - -Global discovery is unchanged in both modes: `RepoListStore` keeps syncing -announcements, states and deletions from `BOOTSTRAP_RELAYS`. - -## GitWorkshop reference - -- `src/services/settings.ts:197-210`: `RelayCurationMode = "repo" | "outbox"`, - persisted to `localStorage`, default `"outbox"` (Uncensored). -- `src/pages/Settings.tsx:85-100`: two selectable cards, "Curated" and - "Uncensored". -- `src/hooks/useResolvedRepository.ts`: `repoRelayGroup` is the announcement's - `relays` tag. `extraRelaysForMaintainerMailboxCoverage` is a delta group of - every maintainer's NIP-65 outbox + inbox relays, excluding relays already in - the repo group, capped at `MAX_MAILBOX_RELAYS_PER_USER = 3` per direction - (`addMailboxRelaysToGroup`). The pubkey set is the announcement chain's - `discoveryPubkeys` (maintainers, moderators, owner). -- Gating: `src/hooks/useNip34Loaders.ts:447` and - `src/pages/repo/RepoLayout.tsx:319-360` only subscribe the item loaders to - the maintainer group when the mode is `outbox`. Curated never touches - maintainer relays. - -## Current behaviour in `signed` - -`crates/signed_state/src/repo.rs`: - -- `subscribe_remote` -> `Backend::subscribe_bootstrap`: one-shot REQ of - `repo_filters` on `BOOTSTRAP_RELAYS` (manual target). -- `connect_announced_relays` -> `Backend::connect_repo_relays`: add and - connect the announcement's `relays` tag, then negentropy-`sync` - `repo_filters` (manual target). Deduped by the `repo_relays` set. Called - from `new`, `announce`, and on every announcement change in `run_refresh`. -- `run_refresh` also fetches comments (`filters::comments_for`) and statuses - (`filters::statuses_for`) for each newly seen root from bootstrap + - `repo_relays`. -- No fetch path uses NIP-65: the gossip store is configured but never - consulted (`docs/backend-audit.md`, section 2). - -## Design - -```mermaid -flowchart TD - A[RepoStore opens a repo] --> B{Event fetching strategy} - B -->|Curated| C[Manual sync to repo-declared relays] - B -->|Uncensored| D[Auto sync: SDK resolves maintainers' NIP-65 relays from authors and #p] --> E[Manual sync to repo-declared relays] - C --> F[(Local database)] - D --> F - E --> F -``` - -1. **Curated stays exactly what the code does today**: bootstrap REQ plus the - manual announced-relay sync. -2. **Uncensored adds one SDK Auto sync.** No relay URLs are resolved, stored, - or tracked, and no kind `10002` events are fetched or parsed by `signed`: - the SDK's NIP-65 gossip targeting resolves the maintainers' relays per - request (`client.sync(filter)` with no `.with(..)`, verified in - `nostr-sdk/src/client/api/sync.rs:151-166` and - `api/util.rs:12-29`). The existing manual announced-relay sync stays for - the repo's own relays in both modes. -3. **The filters must name the maintainers.** Gossip resolution is driven by - `Filter::extract_public_keys` (`nostr/src/filter/mod.rs:591`), which reads - only `authors` and the lowercase `#p` tag: - - | Filter shape | Auto target | - | --- | --- | - | `authors` only | each author's NIP-65 **write** relays | - | `#p` only | each pubkey's NIP-65 **read** relays | - | both | union of read and write relays | - | neither | the pool's read relays | - - Today's `repo_filters` resolve nothing for this purpose: the - announcement/state filters carry only the owner in `authors`, and the - activity filter is `#a`-only, so it is classified `Other` and falls back to - the pool's read relays. Uncensored therefore sends a separate - maintainer-shaped filter set to the Auto sync. - -4. **Maintainer filter set** (Uncensored only): - - ```rust - /// Filters the SDK resolves through NIP-65 gossip in Uncensored mode. - /// - /// Gossip reads pubkeys from `authors` and the lowercase `#p` tag only, - /// so every filter names the owner and the maintainers. - 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, - // resolved to write relays. - Filter::new() - .kinds([Kind::GitRepoAnnouncement, Kind::RepoState]) - .authors(pubkeys.clone()) - .identifier(addr.identifier.clone()), - // Activity tagging a maintainer, resolved to their read relays. - Filter::new() - .kinds(filters::ACTIVITY_KINDS) - .coordinate(addr) - .pubkeys(pubkeys.clone()), - // Activity authored by a maintainer, resolved to their write relays. - Filter::new() - .kinds(filters::ACTIVITY_KINDS) - .coordinate(addr) - .authors(pubkeys.clone()), - // Deletions authored by a maintainer, resolved to write relays. - Filter::new() - .kinds([Kind::EventDeletion, Kind::RequestToVanish]) - .authors(pubkeys), - ] - } - ``` - - `maintainers` is `Announcement::effective_maintainers()`, already computed - by `run_refresh`. Because the announcement/state filter names every - maintainer, co-maintainer state events (kind `30618`) land in the local - database. They are not displayed yet: `run_refresh` reads state through - the owner-only `filters::state` (`docs/backend-audit.md`, finding 3). - -5. **The manual leg is unchanged**: `subscribe_bootstrap` plus - `connect_announced_relays` keep covering bootstrap and repo-declared - relays. Per-root follow-ups (`comments_for`, `statuses_for`) also stay - as-is. - -6. **Default: Uncensored**, matching GitWorkshop - (`DEFAULT_RELAY_CURATION_MODE = "outbox"`). Flagged under "Decisions" - because it changes what existing users fetch. - -### Coverage - -- The `#p` activity filter finds roots and statuses that tag a maintainer - (NIP-34 root events and statuses tag the owner) on the maintainers' read - relays. -- The `authors` activity filter finds maintainer-authored events (issues, - PRs, patches, comments, statuses) on their write relays. `signed`'s - comments and statuses carry the `a` tag - (`comment_builder`, `set_status`, `publish_applied_status`), so this filter - does not drop them. -- Comments whose parent author is not a maintainer are not matched by the - `#p` filter, but they are published to the parent author's inbox, not to a - maintainer's relays; maintainer-authored comments are still caught by the - `authors` filter. -- Not covered: status events without an `a` tag published to a maintainer's - outbox. Per-root `#e` filters carry no pubkeys, so they cannot resolve - through gossip; GitWorkshop covers these with per-item supplemental - queries (out of scope below). - -## Changes - -### 1. Setting, `crates/settings/src/settings.rs` - -Follow the bare-enum pattern of `AppearanceMode`: - -```rust -/// Which relays `RepoStore` queries for a repository's activity. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "snake_case")] -pub enum EventFetchingStrategy { - /// Only the relays declared in the repository announcement. - Curated, - /// Repository relays plus every maintainer's NIP-65 relays. - #[default] - Uncensored, -} -``` - -Add `pub event_fetching: EventFetchingStrategy` to `Settings`. - -### 2. Settings UI, `crates/workspace/src/views/sidebar/settings_dialog.rs` - -- Options: `SelectOption::new("curated", "Curated")` and - `SelectOption::new("uncensored", "Uncensored")`. -- Add an `event_fetching: Entity>>` field to - `SettingsControls`, seeded from the persisted value, and a - `SelectEvent::Confirm` subscription that maps the value to the enum and - calls `store.edit(|settings| settings.event_fetching = strategy, cx)`. -- Add an `event_fetching_section` next to `appearance_section`, using - `setting_row` with the title "Event Fetching Strategy" and a description of - both modes, and insert it in `settings_view`. - -### 3. Backend, `crates/signed_state/src/backend.rs` - -New method next to `connect_repo_relays`: - -```rust -/// Sync filters through the SDK's NIP-65 gossip targeting. -/// -/// `client.sync(filter)` without `.with(..)` is an Auto request: the SDK -/// resolves each filter's `authors` and lowercase `#p` pubkeys to their -/// NIP-65 relays, connects them, and negentropy-syncs there. -pub fn sync_auto(&mut self, filters: Vec, cx: &mut Context) { - let client = self.client.clone(); - - cx.spawn(async move |_this, _cx| { - for filter in filters { - if let Err(e) = client.sync(filter).await { - log::warn!("gossip relay fetch failed: {e}"); - } - } - Ok::<(), Error>(()) - }) - .detach(); -} -``` - -Errors stay log-only, like `connect_repo_relays`; `BackendEvent::Synced` is -deliberately not emitted (it would refresh `RepoListStore` for repo-level -traffic). - -### 4. `RepoStore`, `crates/signed_state/src/repo.rs` - -New field, initialized empty in `new` and `new_local`: - -```rust -/// Maintainers already synced through gossip in Uncensored mode. -synced_maintainers: HashSet, -``` - -New methods after `connect_announced_relays`: `maintainer_filters` (listed -under Design) and - -```rust -/// In Uncensored mode, sync this repository's maintainer-shaped filters -/// through the SDK's NIP-65 gossip targeting. -fn sync_maintainer_relays(&mut self, maintainers: &[PublicKey], cx: &mut Context) { - let strategy = settings::SettingsStore::try_global(cx) - .map(|store| store.read(cx).settings().event_fetching) - .unwrap_or_default(); - if strategy != EventFetchingStrategy::Uncensored { - return; - } - - let Some(addr) = self.addr.clone() else { - return; - }; - - if !maintainers - .iter() - .any(|pk| !self.synced_maintainers.contains(pk)) - { - return; - } - self.synced_maintainers.extend(maintainers.iter().copied()); - - let filters = Self::maintainer_filters(&addr, maintainers); - let backend = Backend::global(cx); - backend.update(cx, |backend, cx| backend.sync_auto(filters, cx)); -} -``` - -Hook into `run_refresh`, in the foreground update after -`connect_announced_relays`: - -```rust -let maintainers = this - .announcement - .as_ref() - .map(Announcement::effective_maintainers) - .unwrap_or_default(); -this.sync_maintainer_relays(&maintainers, cx); -``` - -Notes: - -- `SettingsStore::try_global` keeps wasm safe: the settings store is only - installed by the desktop app (`desktop/src/main.rs:21`). -- No new crate dependencies: `signed_state` already depends on `settings` and - `nostr_sdk`. - -## Tests - -- `crates/settings`: extend `json_roundtrip_preserves_everything` and - `partial_json_merges_with_defaults` for `event_fetching` (snake_case - values, default). -- `crates/signed_state/src/repo.rs`, `mod tests`: unit-test - `maintainer_filters`: every filter names the owner and maintainers via - `authors` or `#p` (the announcement/state filter included), activity - filters carry the `#a` coordinate, and the owner is added for a - subordinate fork. -- `cargo test -p settings -p signed_state` and - `cargo check -p signed_workspace` for the UI. -- Manual smoke: open a repository whose activity exists only on a - maintainer's relays (not on the announced relays or bootstrap) in both - modes. - -## Decisions - -1. **Default.** Uncensored, to match GitWorkshop. Curated preserves today's - relay traffic; flipping the default is a one-line change. -2. **Bootstrap REQ stays in Curated.** It is the index path that keeps - repositories with unreachable announced relays usable. Strict GitWorkshop - parity (repo relays only in Curated) is possible later but is a behaviour - change unrelated to the option itself. -3. **Read and write coverage is approximated by two activity filters** (`#p` - and `authors`) rather than a resolved "all maintainer relays" set. The SDK - resolves the relay sets per filter shape; `signed` stores no relay URLs. -4. **The announcement/state Auto filter names every maintainer plus the - owner.** This also fetches co-maintainer state events into the local - database; showing them is a separate display-side change. -5. **Runtime switching.** `sync_maintainer_relays` reads the setting on every - `run_refresh`, so switching to Uncensored applies at the next refresh - without a restart. Switching back stops new Auto syncs but does not undo - relays already resolved by the gossip pool. -6. **Gossip pool growth.** Auto requests add resolved relays with - `RelayCapabilities::GOSSIP` and never remove them for the session - (`docs/backend-audit.md`); accepted, as the SDK is designed this way. -7. **Errors stay log-only**, consistent with existing background fetches. - -## Out of scope - -- Status events without an `a` tag published to maintainer outboxes - (GitWorkshop's per-item supplemental loader). -- Per-item author inbox relays - (`src/services/nostr.ts:1042,1070`, `MAX_AUTHOR_INBOX_RELAYS = 3`). -- Co-maintainer state display: co-maintainer states are fetched (decision 4) - but `run_refresh` still reads state through the owner-only - `filters::state` (`docs/backend-audit.md`, finding 3). -- Gating or replacing the bootstrap REQ in Curated mode. - -## Phasing - -1. Setting enum, field and settings tests. **Done.** -2. Settings UI control. -3. `Backend::sync_auto` and `RepoStore::sync_maintainer_relays` with the - maintainer filter set, plus the filter unit test. -4. `cargo test` / `cargo check`, then a manual smoke test. -- 2.54.0 From 4d2d0583968de3a3d974e6c17f2fb493c9980553 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 27 Sep 2026 08:03:56 +0700 Subject: [PATCH 04/10] refactor --- crates/signed_state/src/backend.rs | 99 ++++++++++------------- crates/signed_state/src/profile.rs | 11 +-- crates/signed_state/src/repo.rs | 61 +++----------- crates/signed_state/src/repos.rs | 8 -- crates/workspace/src/views/inbox.rs | 2 +- crates/workspace/src/views/sidebar/mod.rs | 12 +-- 6 files changed, 59 insertions(+), 134 deletions(-) diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 0d6998b..f28088c 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -37,15 +37,14 @@ pub enum BackendEvent { PassphraseRequired, /// The signer changed on login, logout or account switch. SignerChanged, - /// New events were received from a relay and stored in the database. + /// Events received from a relay, including this client's own events echoed + /// back by a relay after publishing. NostrUpdate(Vec), Synced, SyncProgress { total: u64, current: u64, }, - /// An event built locally was signed, broadcast and stored. - Published(Box), Error(String), } @@ -91,13 +90,11 @@ impl Backend { let pump: Task> = cx.spawn(async move |this, cx| { let mut notifications = pump_client.notifications(); let mut pending: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); 'outer: loop { - match notifications.next().await { - Some(ClientNotification::Event { event, .. }) => { - pending.push(Update::from_event(&event)); - } - Some(_) => continue, + match next_update(&mut notifications, &mut seen).await { + Some(update) => pending.push(update), None => break, } @@ -113,17 +110,11 @@ impl Backend { let timer = cx.background_executor().timer(deadline - now); futures::pin_mut!(timer); - let next = notifications.next(); + let next = next_update(&mut notifications, &mut seen); futures::pin_mut!(next); match futures::future::select(next, timer).await { - futures::future::Either::Left(( - Some(ClientNotification::Event { event, .. }), - _, - )) => { - pending.push(Update::from_event(&event)); - } - futures::future::Either::Left((Some(_), _)) => continue, + futures::future::Either::Left((Some(update), _)) => pending.push(update), futures::future::Either::Left((None, _)) => break 'outer, futures::future::Either::Right(_) => break, } @@ -500,9 +491,7 @@ impl Backend { let builder = announcement.into_event_builder(); let event = builder.finalize_async(&signer).await?; let output = client.send_event(&event).broadcast().await?; - let event = require_relay_accepted(output, event)?; - this.update(cx, |this, cx| this.announce_published(event.clone(), cx))?; - event + require_relay_accepted(output, event)? }; // The state event is the push authorization. Stage it on each @@ -557,14 +546,10 @@ impl Backend { // 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 { - if let Err(e) = client.send_event(state_event).broadcast().await { - log::warn!("failed to broadcast repository state: {e}"); - } - this.update(cx, |this, cx| { - this.announce_published(state_event.clone(), cx) - }) - .ok(); + if let Some(state_event) = &outcome.state_event + && let Err(e) = client.send_event(state_event).broadcast().await + { + log::warn!("failed to broadcast repository state: {e}"); } let announcement = Announcement::from_event(&event) @@ -653,9 +638,7 @@ impl Backend { let builder = announcement.into_event_builder(); let event = builder.finalize_async(&signer).await?; let output = client.send_event(&event).broadcast().await?; - let event = require_relay_accepted(output, event)?; - this.update(cx, |this, cx| this.announce_published(event.clone(), cx))?; - event + require_relay_accepted(output, event)? }; let refs = state.refs.clone(); @@ -712,12 +695,10 @@ impl Backend { // 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 { - if let Err(e) = client.send_event(state_event).broadcast().await { - log::warn!("failed to broadcast repository state: {e}"); - } - this.update(cx, |this, cx| this.announce_published(state_event.clone(), cx)) - .ok(); + if let Some(state_event) = &outcome.state_event + && let Err(e) = client.send_event(state_event).broadcast().await + { + log::warn!("failed to broadcast repository state: {e}"); } } @@ -894,14 +875,10 @@ 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 { - if let Err(e) = client.send_event(state_event).broadcast().await { - log::warn!("failed to broadcast repository state: {e}"); - } - this.update(cx, |this, cx| { - this.announce_published(state_event.clone(), cx) - }) - .ok(); + if let Some(state_event) = &outcome.state_event + && let Err(e) = client.send_event(state_event).broadcast().await + { + log::warn!("failed to broadcast repository state: {e}"); } Ok(outcome) @@ -1299,14 +1276,6 @@ impl Backend { .detach(); } - /// Emit [`BackendEvent::Published`] for cross-store invalidation. - /// - /// Callers publish with `client.send_event(...)` directly, then call this - /// so stores like `RepoListStore` refresh without re-querying the relays. - pub fn announce_published(&self, event: Event, cx: &mut Context) { - cx.emit(BackendEvent::Published(Box::new(event))); - } - /// Publish a NIP-09 deletion for each of `events`, best-effort. /// /// Each target gets its own deletion event: a relay rejecting or @@ -1329,6 +1298,26 @@ impl Backend { } } +/// Await the next relay event from the notification stream. +async fn next_update( + notifications: &mut (impl futures::Stream + Unpin), + seen: &mut HashSet, +) -> Option { + loop { + match notifications.next().await { + Some(ClientNotification::Message { message, .. }) => { + if let RelayMessage::Event { event, .. } = *message + && seen.insert(event.id) + { + return Some(Update::from_event(&event)); + } + } + Some(_) => continue, + None => return None, + } + } +} + /// Sign and send a single NIP-09 deletion request for `event`. async fn retract_event( client: &Client, @@ -1338,16 +1327,14 @@ async fn retract_event( let builder = EventDeletionRequest::new() .id(event.id) .into_event_builder(); + let deletion = builder.finalize_async(signer).await?; client.send_event(&deletion).broadcast().await?; + Ok(()) } /// The event was accepted by at least one relay, or a descriptive error otherwise. -/// -/// The SDK does not treat "accepted by zero relays" as an error on its own: -/// [`SendEventOutput::success`] may be empty while the call still returns `Ok`. -/// This turns that case into an error the caller can surface. pub(crate) fn require_relay_accepted( output: SendEventOutput, event: Event, diff --git a/crates/signed_state/src/profile.rs b/crates/signed_state/src/profile.rs index 2e25ac5..6474287 100644 --- a/crates/signed_state/src/profile.rs +++ b/crates/signed_state/src/profile.rs @@ -93,8 +93,8 @@ impl ProfileStore { pub(crate) fn new(cx: &mut Context) -> Self { let backend = Backend::global(cx); - let subscription = cx.subscribe(&backend, |this, _backend, event, cx| match event { - BackendEvent::NostrUpdate(updates) => { + let subscription = cx.subscribe(&backend, |this, _backend, event, cx| { + if let BackendEvent::NostrUpdate(updates) = event { for update in updates .iter() .filter(|update| update.kind == Kind::Metadata) @@ -102,13 +102,6 @@ impl ProfileStore { this.apply_author(update.author, cx); } } - BackendEvent::Published(event) if event.kind == Kind::Metadata => { - let metadata = Metadata::from_json(&event.content).unwrap_or_default(); - this.profiles - .insert(event.pubkey, Profile::new(event.pubkey, metadata)); - cx.notify(); - } - _ => {} }); // Fetch requests are queued on a channel, batched into one sync per debounce window. diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index c024078..7d76c7e 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -225,20 +225,6 @@ impl RepoStore { deletion || coordinate || authored || comment || status }), - BackendEvent::Published(event) => { - let announcement = event.kind == Kind::GitRepoAnnouncement; - let author = event.pubkey == addr.public_key; - let coordinate = event.tags.coordinates().into_iter().any(|c| c == *addr); - - let state = event.kind == Kind::RepoState - && author - && event.tags.identifier().as_deref() == Some(addr.identifier.as_str()); - - let deletion = - event.kind == Kind::EventDeletion || event.kind == Kind::RequestToVanish; - - coordinate || (announcement && author) || state || deletion - } _ => false, }; @@ -977,11 +963,6 @@ impl RepoStore { } }; - this.update(cx, |_this, cx| { - Backend::global(cx) - .update(cx, |backend, cx| backend.announce_published(pr_event.clone(), cx)) - })?; - // 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 { @@ -1171,20 +1152,11 @@ impl RepoStore { } .await; - match publish_result { - Ok(event) => { - this.update(cx, |_this, cx| { - Backend::global(cx).update(cx, |backend, cx| { - backend.announce_published(event.clone(), cx) - }) - })?; - } - Err(e) => { - return this.update(cx, |this, cx| { - this.last_error = Some(e.to_string()); - cx.notify(); - }); - } + if let Err(e) = publish_result { + return this.update(cx, |this, cx| { + this.last_error = Some(e.to_string()); + cx.notify(); + }); } Ok(()) @@ -1622,19 +1594,11 @@ impl RepoStore { } .await; - match publish_result { - Ok(event) => { - this.update(cx, |_this, cx| { - Backend::global(cx) - .update(cx, |backend, cx| backend.announce_published(event, cx)) - })?; - } - Err(e) => { - this.update(cx, |this, cx| { - this.last_error = Some(e.to_string()); - cx.notify(); - })?; - } + if let Err(e) = publish_result { + this.update(cx, |this, cx| { + this.last_error = Some(e.to_string()); + cx.notify(); + })?; } Ok(()) @@ -1767,11 +1731,6 @@ async fn publish_patch_series( let event = builder.finalize_async(&signer).await?; let output = client.send_event(&event).broadcast().await?; let event = require_relay_accepted(output, event)?; - this.update(cx, |_this, cx| { - Backend::global(cx).update(cx, |backend, cx| { - backend.announce_published(event.clone(), cx) - }) - })?; if root.is_none() { root = Some(event.clone()); diff --git a/crates/signed_state/src/repos.rs b/crates/signed_state/src/repos.rs index c6309c8..6b122a9 100644 --- a/crates/signed_state/src/repos.rs +++ b/crates/signed_state/src/repos.rs @@ -82,14 +82,6 @@ impl RepoListStore { is_announcement || is_repo_state } }), - BackendEvent::Published(event) => { - let announcement = event.kind == Kind::GitRepoAnnouncement; - let state = event.kind == Kind::RepoState; - let deletion = - event.kind == Kind::EventDeletion || event.kind == Kind::RequestToVanish; - - announcement || state || deletion - } BackendEvent::SignerChanged => { this.state_synced_repos.clear(); true diff --git a/crates/workspace/src/views/inbox.rs b/crates/workspace/src/views/inbox.rs index 38fe63a..e68aafc 100644 --- a/crates/workspace/src/views/inbox.rs +++ b/crates/workspace/src/views/inbox.rs @@ -171,7 +171,7 @@ impl InboxView { self.refresh(cx); } } - BackendEvent::Synced | BackendEvent::Published(_) => self.refresh(cx), + BackendEvent::Synced => self.refresh(cx), _ => {} } } diff --git a/crates/workspace/src/views/sidebar/mod.rs b/crates/workspace/src/views/sidebar/mod.rs index fa4eaf9..fd16d89 100644 --- a/crates/workspace/src/views/sidebar/mod.rs +++ b/crates/workspace/src/views/sidebar/mod.rs @@ -79,16 +79,12 @@ impl SidebarPanel { let signer_changed = matches!(event, BackendEvent::SignerChanged); let signer_required = matches!(event, BackendEvent::SignerRequired); - if !signer_changed && !signer_required { - return; - } - if signer_required { this.banner = pick_banner(); cx.notify(); } - if this.refresh(cx) || signer_required { + if this.refresh(cx) || signer_changed { cx.notify(); } })); @@ -131,16 +127,14 @@ impl SidebarPanel { let user = backend.read(cx).current_user(); let (announcements, local_repos, scanning) = { - let repo_list = RepoListStore::global(cx); - let repo_list = repo_list.read(cx); + let repo_list = RepoListStore::global(cx).read(cx); let announcements = user .as_ref() .map(|user| repo_list.announcements_of(user)) .unwrap_or_default(); - let local = LocalReposStore::global(cx); - let local = local.read(cx); + let local = LocalReposStore::global(cx).read(cx); let local_repos = resolve_local_repos(&local.repos, &repo_list.announcements, &announcements); -- 2.54.0 From c170c4a800c128b60ac04c88bf96bcdede8a8477 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 27 Sep 2026 08:34:13 +0700 Subject: [PATCH 05/10] update per event kind --- crates/signed_core/src/filters.rs | 10 +++++ crates/signed_state/src/backend.rs | 64 +++++++++++++++++++++-------- crates/signed_state/src/profile.rs | 29 ++++++------- crates/signed_state/src/repo.rs | 3 +- crates/signed_state/src/repos.rs | 15 +------ crates/workspace/src/views/inbox.rs | 17 +------- 6 files changed, 75 insertions(+), 63 deletions(-) diff --git a/crates/signed_core/src/filters.rs b/crates/signed_core/src/filters.rs index 678cf50..042a00b 100644 --- a/crates/signed_core/src/filters.rs +++ b/crates/signed_core/src/filters.rs @@ -38,6 +38,16 @@ const GIT_ROOT_KINDS: [Kind; 4] = [ Kind::GitRepoAnnouncement, ]; +/// Kinds that carry repository data: announcements, states, activity and deletions. +pub fn is_repo_kind(kind: Kind) -> bool { + kind == Kind::GitRepoAnnouncement + || kind == Kind::RepoState + || kind == Kind::EventDeletion + || kind == Kind::RequestToVanish + || kind == COVER_NOTE_KIND + || 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 diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index f28088c..31f6ab2 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -37,9 +37,10 @@ pub enum BackendEvent { PassphraseRequired, /// The signer changed on login, logout or account switch. SignerChanged, - /// Events received from a relay, including this client's own events echoed - /// back by a relay after publishing. - NostrUpdate(Vec), + /// 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, SyncProgress { total: u64, @@ -89,12 +90,16 @@ impl Backend { let pump: Task> = cx.spawn(async move |this, cx| { let mut notifications = pump_client.notifications(); - let mut pending: Vec = Vec::new(); + let mut pending_profiles: HashSet = HashSet::new(); + let mut pending_repos: Vec = Vec::new(); let mut seen: HashSet = HashSet::new(); 'outer: loop { match next_update(&mut notifications, &mut seen).await { - Some(update) => pending.push(update), + Some(UpdateEvent::Profile(author)) => { + pending_profiles.insert(author); + } + Some(UpdateEvent::Repo(update)) => pending_repos.push(update), None => break, } @@ -114,18 +119,29 @@ impl Backend { futures::pin_mut!(next); match futures::future::select(next, timer).await { - futures::future::Either::Left((Some(update), _)) => pending.push(update), + futures::future::Either::Left((Some(UpdateEvent::Profile(author)), _)) => { + pending_profiles.insert(author); + } + futures::future::Either::Left((Some(UpdateEvent::Repo(update)), _)) => { + pending_repos.push(update); + } futures::future::Either::Left((None, _)) => break 'outer, futures::future::Either::Right(_) => break, } } - let batch = std::mem::take(&mut pending); + let profiles: Vec = pending_profiles.drain().collect(); + let repos = std::mem::take(&mut pending_repos); - if let Err(e) = - this.update(cx, |_this, cx| cx.emit(BackendEvent::NostrUpdate(batch))) - { - log::warn!("failed to emit nostr update: {e}"); + if let Err(e) = this.update(cx, |_this, cx| { + if !profiles.is_empty() { + cx.emit(BackendEvent::ProfileUpdates(profiles)); + } + if !repos.is_empty() { + cx.emit(BackendEvent::RepoUpdates(repos)); + } + }) { + log::warn!("failed to emit backend update: {e}"); } } @@ -1298,18 +1314,34 @@ impl Backend { } } +/// A relay event the backend routes to a store group. +enum UpdateEvent { + Profile(PublicKey), + Repo(Update), +} + /// Await the next relay event from the notification stream. async fn next_update( notifications: &mut (impl futures::Stream + Unpin), seen: &mut HashSet, -) -> Option { +) -> Option { loop { match notifications.next().await { Some(ClientNotification::Message { message, .. }) => { - if let RelayMessage::Event { event, .. } = *message - && seen.insert(event.id) - { - return Some(Update::from_event(&event)); + let RelayMessage::Event { event, .. } = *message else { + continue; + }; + + let update = match event.kind { + Kind::Metadata => UpdateEvent::Profile(event.pubkey), + kind if filters::is_repo_kind(kind) => { + UpdateEvent::Repo(Update::from_event(&event)) + } + _ => continue, + }; + + if seen.insert(event.id) { + return Some(update); } } Some(_) => continue, diff --git a/crates/signed_state/src/profile.rs b/crates/signed_state/src/profile.rs index 6474287..b05af89 100644 --- a/crates/signed_state/src/profile.rs +++ b/crates/signed_state/src/profile.rs @@ -92,31 +92,28 @@ impl ProfileStore { pub(crate) fn new(cx: &mut Context) -> Self { let backend = Backend::global(cx); + let client = backend.read(cx).client(); let subscription = cx.subscribe(&backend, |this, _backend, event, cx| { - if let BackendEvent::NostrUpdate(updates) = event { - for update in updates - .iter() - .filter(|update| update.kind == Kind::Metadata) - { - this.apply_author(update.author, cx); + if let BackendEvent::ProfileUpdates(authors) = event { + for author in authors { + this.apply_author(*author, cx); } } }); - // Fetch requests are queued on a channel, batched into one sync per debounce window. - let client = backend.read(cx).client(); - let (sender, receiver) = flume::unbounded::(); let entity = cx.entity().downgrade(); + let entity_clone = entity.clone(); + + let (sender, receiver) = flume::unbounded::(); cx.spawn(async move |_this, cx| { Self::handle_requests(entity, &client, &receiver, cx).await }) .detach(); - let weak = cx.entity().downgrade(); cx.defer(move |cx| { - if let Err(error) = weak.update(cx, |this, cx| this.load(cx)) { + if let Err(error) = entity_clone.update(cx, |this, cx| this.load(cx)) { log::warn!("profile store dropped before initial load could run: {error}"); } }); @@ -221,8 +218,6 @@ impl ProfileStore { } /// Re-read the latest metadata of every requested author from the local database. - /// - /// Used after a sync, which produces no NostrUpdate events. fn apply_seen(&mut self, cx: &mut Context) { let authors: Vec = self.seen.borrow().iter().copied().collect(); @@ -298,15 +293,20 @@ impl ProfileStore { // The channel has no async timeout, race the receive against a timer. let deadline = Instant::now() + BATCH_TIMEOUT; + loop { let now = Instant::now(); + if now >= deadline { break; } + let timer = cx.background_executor().timer(deadline - now); futures::pin_mut!(timer); + let recv = receiver.recv_async(); futures::pin_mut!(recv); + match futures::future::select(recv, timer).await { futures::future::Either::Left((Ok(public_key), _)) => { batch.insert(public_key); @@ -320,9 +320,6 @@ impl ProfileStore { .kind(Kind::Metadata) .authors(batch.drain().collect::>()); - // Negentropy-sync with the bootstrap relays. - // Synced events are written to the database directly, no NostrUpdate. - // Re-apply from the database afterwards. match sync_bootstrap_only(client, filter, SyncOptions::default()).await { Ok(_) => { this.update(cx, |this, cx| this.apply_seen(cx)).ok(); diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 7d76c7e..124208b 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -208,8 +208,7 @@ impl RepoStore { }; let relevant = match event { - BackendEvent::NostrUpdate(updates) => updates.iter().any(|update| { - // Deletions may target any event of this repository. + BackendEvent::RepoUpdates(updates) => updates.iter().any(|update| { let deletion = update.kind == Kind::EventDeletion || update.kind == Kind::RequestToVanish; diff --git a/crates/signed_state/src/repos.rs b/crates/signed_state/src/repos.rs index 6b122a9..be79c6d 100644 --- a/crates/signed_state/src/repos.rs +++ b/crates/signed_state/src/repos.rs @@ -68,24 +68,11 @@ impl RepoListStore { let subscription = cx.subscribe(&backend, |this, _backend, event, cx| { let relevant = match event { - BackendEvent::NostrUpdate(updates) => updates.iter().any(|update| { - // Deletions may target anything we list, always refresh. - if update.kind == Kind::EventDeletion || update.kind == Kind::RequestToVanish { - true - } else if filters::ACTIVITY_KINDS.contains(&update.kind) { - // Activity events are addressed to repos via `a` tags. - // Their author is not the repo owner, always refresh. - true - } else { - let is_announcement = update.kind == Kind::GitRepoAnnouncement; - let is_repo_state = update.kind == Kind::RepoState; - is_announcement || is_repo_state - } - }), BackendEvent::SignerChanged => { this.state_synced_repos.clear(); true } + BackendEvent::RepoUpdates(_) => true, BackendEvent::Synced => true, _ => false, }; diff --git a/crates/workspace/src/views/inbox.rs b/crates/workspace/src/views/inbox.rs index e68aafc..7de3cff 100644 --- a/crates/workspace/src/views/inbox.rs +++ b/crates/workspace/src/views/inbox.rs @@ -11,7 +11,7 @@ use gpui::{ use gpui_component::button::{Button, ButtonVariants}; use gpui_component::{ActiveTheme, Icon, IconName, IconNamed, Sizable, StyledExt, h_flex, v_flex}; use nostr::prelude::{Event, EventId, Kind, PublicKey, Timestamp}; -use signed_core::{COVER_NOTE_KIND, InboxItem, InboxReadState, RepoAddr, filters}; +use signed_core::{COVER_NOTE_KIND, InboxItem, InboxReadState, RepoAddr}; use signed_state::{ Backend, BackendEvent, ProfileStore, RefreshGate, RefreshRequest, RepoListStore, query_inbox, }; @@ -157,20 +157,7 @@ impl InboxView { fn handle_backend_event(&mut self, event: &BackendEvent, cx: &mut Context) { match event { - BackendEvent::NostrUpdate(updates) => { - let relevant = updates.iter().any(|update| { - let is_notification = filters::NOTIFICATION_KINDS.contains(&update.kind); - let is_comment = update.kind == Kind::Comment; - let is_event_deletion = update.kind == Kind::EventDeletion; - let is_request_to_vanish = update.kind == Kind::RequestToVanish; - - is_notification || is_comment || is_event_deletion || is_request_to_vanish - }); - - if relevant { - self.refresh(cx); - } - } + BackendEvent::RepoUpdates(_) => self.refresh(cx), BackendEvent::Synced => self.refresh(cx), _ => {} } -- 2.54.0 From e1a6f0a669f56a8c2fffdc650a86dc06bd2db4b6 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 27 Sep 2026 08:37:39 +0700 Subject: [PATCH 06/10] remove cover note --- crates/signed_core/src/annotations.rs | 7 ------ crates/signed_core/src/filters.rs | 32 +++++---------------------- crates/signed_core/src/inbox.rs | 7 ++---- crates/signed_core/src/lib.rs | 2 -- crates/workspace/src/views/inbox.rs | 6 +---- 5 files changed, 9 insertions(+), 45 deletions(-) delete mode 100644 crates/signed_core/src/annotations.rs diff --git a/crates/signed_core/src/annotations.rs b/crates/signed_core/src/annotations.rs deleted file mode 100644 index 0a49935..0000000 --- a/crates/signed_core/src/annotations.rs +++ /dev/null @@ -1,7 +0,0 @@ -use nostr::prelude::*; - -/// GitWorkshop and `ngit` cover-note extension, kind 1624. -/// -/// A markdown note attached to an issue, patch or PR by its author or a maintainer, -/// not part of the NIP-34 draft, read support for interop. -pub const COVER_NOTE_KIND: Kind = Kind::Custom(1624); diff --git a/crates/signed_core/src/filters.rs b/crates/signed_core/src/filters.rs index 042a00b..c8180a0 100644 --- a/crates/signed_core/src/filters.rs +++ b/crates/signed_core/src/filters.rs @@ -2,7 +2,7 @@ use std::time::Duration; use nostr::prelude::*; -use crate::{COVER_NOTE_KIND, RepoAddr}; +use crate::RepoAddr; /// Kinds that make up the activity of a repository. pub const ACTIVITY_KINDS: [Kind; 9] = [ @@ -18,19 +18,18 @@ pub const ACTIVITY_KINDS: [Kind; 9] = [ ]; /// Kinds that notify a user when they tag them via their `p` tag. -pub const NOTIFICATION_KINDS: [Kind; 9] = [ +pub const NOTIFICATION_KINDS: [Kind; 8] = [ Kind::GitIssue, Kind::GitPullRequest, Kind::GitPatch, Kind::GitPullRequestUpdate, - COVER_NOTE_KIND, Kind::GitStatusOpen, Kind::GitStatusApplied, Kind::GitStatusClosed, Kind::GitStatusDraft, ]; -/// Git root kinds that make a comment or cover note count as git activity. +/// Git root kinds that make a comment count as git activity. const GIT_ROOT_KINDS: [Kind; 4] = [ Kind::GitIssue, Kind::GitPatch, @@ -44,7 +43,6 @@ pub fn is_repo_kind(kind: Kind) -> bool { || kind == Kind::RepoState || kind == Kind::EventDeletion || kind == Kind::RequestToVanish - || kind == COVER_NOTE_KIND || ACTIVITY_KINDS.contains(&kind) } @@ -150,13 +148,7 @@ pub fn notifications(me: PublicKey) -> Vec { /// A comment on an unrelated kind is matched too, so results must be filtered /// through [`is_git_activity`] before display. pub fn authored_activity(me: PublicKey) -> Filter { - Filter::new() - .kinds( - ACTIVITY_KINDS - .into_iter() - .chain(std::iter::once(COVER_NOTE_KIND)), - ) - .author(me) + Filter::new().kinds(ACTIVITY_KINDS).author(me) } /// Whether a kind-1111 comment targets a git root, checked via its `K` tag. @@ -165,12 +157,6 @@ fn is_git_comment(event: &Event) -> bool { && tag_kind(event, "K").is_some_and(|kind| GIT_ROOT_KINDS.contains(&kind)) } -/// Whether a kind-1624 cover note targets a git root, checked via its `k` tag. -fn is_git_cover_note(event: &Event) -> bool { - event.kind == COVER_NOTE_KIND - && 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. fn is_git_status(event: &Event) -> bool { tag_kind(event, "k").is_some_and(|kind| GIT_ROOT_KINDS.contains(&kind)) @@ -185,7 +171,7 @@ pub fn is_git_activity(event: &Event) -> bool { | Kind::GitStatusApplied | Kind::GitStatusClosed | Kind::GitStatusDraft => is_git_status(event), - kind => kind == COVER_NOTE_KIND && is_git_cover_note(event), + _ => false, } } @@ -277,17 +263,12 @@ mod tests { } #[test] - fn status_and_cover_note_activity_depend_on_the_lowercase_k_tag() { + fn status_activity_depends_on_the_lowercase_k_tag() { let status = signed( &keys(1), Kind::GitStatusClosed, vec![kind_tag("k", Kind::GitPullRequest)], ); - let cover = signed( - &keys(1), - COVER_NOTE_KIND, - vec![kind_tag("k", Kind::GitPatch)], - ); let unrelated = signed( &keys(1), Kind::GitStatusClosed, @@ -295,7 +276,6 @@ mod tests { ); assert!(is_git_activity(&status)); - assert!(is_git_activity(&cover)); assert!(!is_git_activity(&unrelated)); assert!(!is_git_activity(&signed( &keys(1), diff --git a/crates/signed_core/src/inbox.rs b/crates/signed_core/src/inbox.rs index 582e9b0..c848a29 100644 --- a/crates/signed_core/src/inbox.rs +++ b/crates/signed_core/src/inbox.rs @@ -4,7 +4,7 @@ use std::time::Duration; use nostr::prelude::*; use serde::{Deserialize, Serialize}; -use crate::{COVER_NOTE_KIND, RepoAddr, activity_subject}; +use crate::{RepoAddr, activity_subject}; /// Window before `now` that an advanced cutoff retreats to. const ADVANCE_WINDOW: Duration = Duration::from_secs(3 * 24 * 60 * 60); @@ -121,14 +121,11 @@ impl InboxItem { /// - patch (1617): its `e` parent patch, else itself /// - NIP-22 comment (1111): uppercase `E` root pointer /// - PR update (1619): uppercase `E` -/// - statuses (1630-1633) / cover note (1624): NIP-10 root `e` +/// - statuses (1630-1633): NIP-10 root `e` pub fn notification_root(event: &Event, lookup: &L) -> Option where L: Fn(EventId) -> Option, { - if event.kind == COVER_NOTE_KIND { - return nip10_root_id(event).map(|root| resolve_thread_root(root, lookup)); - } match event.kind { Kind::GitIssue | Kind::GitPullRequest => Some(event.id), Kind::GitPatch => Some(match first_e_id(event) { diff --git a/crates/signed_core/src/lib.rs b/crates/signed_core/src/lib.rs index 2bc3ca6..7232086 100644 --- a/crates/signed_core/src/lib.rs +++ b/crates/signed_core/src/lib.rs @@ -1,5 +1,4 @@ pub mod addr; -pub mod annotations; pub mod deletions; pub mod filters; pub mod inbox; @@ -8,7 +7,6 @@ pub mod state; pub mod status; pub use addr::{RepoAddr, identifier_from_name, repo_addr}; -pub use annotations::COVER_NOTE_KIND; pub use deletions::Deletions; pub use filters::{ NOTIFICATION_KINDS, authored_activity, is_git_activity, notification_comments, notifications, diff --git a/crates/workspace/src/views/inbox.rs b/crates/workspace/src/views/inbox.rs index 7de3cff..81d9301 100644 --- a/crates/workspace/src/views/inbox.rs +++ b/crates/workspace/src/views/inbox.rs @@ -11,7 +11,7 @@ use gpui::{ use gpui_component::button::{Button, ButtonVariants}; use gpui_component::{ActiveTheme, Icon, IconName, IconNamed, Sizable, StyledExt, h_flex, v_flex}; use nostr::prelude::{Event, EventId, Kind, PublicKey, Timestamp}; -use signed_core::{COVER_NOTE_KIND, InboxItem, InboxReadState, RepoAddr}; +use signed_core::{InboxItem, InboxReadState, RepoAddr}; use signed_state::{ Backend, BackendEvent, ProfileStore, RefreshGate, RefreshRequest, RepoListStore, query_inbox, }; @@ -557,10 +557,6 @@ fn sub_activity(event: &Event, me: Option, cx: &App) -> AnyElement { } fn activity_phrase(kind: Kind) -> &'static str { - if kind == COVER_NOTE_KIND { - return "added a note"; - } - match kind { Kind::GitIssue => "opened an issue", Kind::GitPullRequest => "opened a PR", -- 2.54.0 From 0ee67d93b6e1b8d56b1997b782de9c0b7e820149 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 27 Sep 2026 09:14:32 +0700 Subject: [PATCH 07/10] fix task leak in repo store --- AGENTS.md | 2 ++ crates/signed_state/src/checkouts.rs | 50 +++------------------------- crates/signed_state/src/refresh.rs | 4 --- crates/signed_state/src/repo.rs | 32 +++++------------- 4 files changed, 15 insertions(+), 73 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 922e8c2..c28ad0a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -81,6 +81,8 @@ Both `cx.spawn` and `cx.background_spawn` return a `Task`, which is a future A task which doesn't do anything but provide a value can be created with `Task::ready(value)`. +Prefer keeping a task in a field over `.detach()` when the work belongs to a view or store. A detached task outlives the view that started it (for example, a repository panel the user has closed), while a task stored in a field is cancelled when that struct drops. A finished `Task` held in a container is not reaped by GPUI - it stays alive until its handle is dropped - so if the container can accumulate many runs, either drop the finished handles before pushing a new one, or hold a single `Option>` and replace it. + ## Elements The `Render` trait is used to render some state into an element tree that is laid out using flexbox layout. An `Entity` where `T` implements `Render` is sometimes called a "view". diff --git a/crates/signed_state/src/checkouts.rs b/crates/signed_state/src/checkouts.rs index 67e52ce..7822c01 100644 --- a/crates/signed_state/src/checkouts.rs +++ b/crates/signed_state/src/checkouts.rs @@ -17,15 +17,9 @@ use crate::repos::RepoListStore; const REFRESH_DEBOUNCE: Duration = Duration::from_millis(300); /// How often the statuses are recomputed against the local refs. -/// -/// A commit lands in a checkout long before the remote reconciliation cadence, -/// so this fast pass surfaces ready-to-push and ready-to-contribute checkouts -/// within a second or two. It reads the tracking refs only, no network. 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); @@ -46,10 +40,6 @@ pub struct CheckoutStatus { /// Commit the branch points at, for tip-based PR dedupe. pub head: String, /// What the branch is compared against. - /// For ready-to-contribute statuses, the announced HEAD branch. - /// The fallbacks are `main`, then the first local branch. - /// For ready-to-push statuses, the remote-tracking ref. - /// Unpushed commits are counted against it. /// /// It is `refs/remotes/origin/`, else `origin/HEAD` for new branches. pub base: String, @@ -67,11 +57,6 @@ struct Remembered { } /// Global store of local-checkout associations and per-checkout statuses. -/// -/// Readers (the sidebar rows, the repository panels) observe this store and -/// derive what they display from their own snapshots, so publishing needs no -/// fine-grained entities: the store notifies when a slice changed and each -/// reader re-derives only what it shows. pub struct CheckoutsStore { /// Checkout paths per announced repository. by_repo: HashMap>, @@ -144,6 +129,7 @@ impl CheckoutsStore { this.statuses.clear(); this.push_statuses.clear(); cx.notify(); + this.refresh(cx); } })); @@ -282,9 +268,6 @@ impl CheckoutsStore { } /// Re-resolve the associations and the requested statuses. - /// - /// Requests arriving while a pass runs fold into a follow-up, requests - /// arriving while the debounce timer is pending are dropped. pub fn refresh(&mut self, cx: &mut Context) { if self.debounce_pending || self.refresh.request() != RefreshRequest::Schedule { return; @@ -300,12 +283,6 @@ impl CheckoutsStore { } /// One full resolve and apply cycle, the debounced entry point. - /// - /// Re-resolves the associations from the settings, the scan and the - /// announcements, then recomputes the requested statuses against freshly - /// fetched remotes. Full passes run on every input change and on the - /// remote reconciliation cadence ([`Self::local_tick`]); they also restart - /// the fast local pass. fn run_refresh(&mut self, cx: &mut Context) { self.debounce_pending = false; self.refresh.begin(); @@ -431,10 +408,6 @@ impl CheckoutsStore { } /// Schedule the fast local status pass, unless one is already pending. - /// - /// Every [`LOCAL_POLL`] the pass recomputes the requested statuses against - /// the local refs, with no network, so a new commit in a checkout surfaces in - /// a second or two instead of at the next remote reconciliation. fn schedule_local_pass(&mut self, cx: &mut Context) { if self.local_pending { return; @@ -452,10 +425,6 @@ impl CheckoutsStore { } /// The fast local status pass. - /// - /// Recomputes the statuses against the local refs; when the remote - /// reconciliation cadence elapsed, it runs a full pass instead so pushes - /// made elsewhere do not linger as `to push`. fn local_tick(&mut self, cx: &mut Context) { // Nothing watched: the chain idles out until a new request restarts it. if self.status_requested.is_empty() && self.push_requested.is_empty() { @@ -490,10 +459,6 @@ impl CheckoutsStore { } /// Recompute the requested statuses against the tracking refs only. - /// - /// The refs were last refreshed by a full pass. Comparing against them is - /// enough to pick up new local commits, and skipping the network keeps - /// this pass cheap enough to run every [`LOCAL_POLL`]. fn run_local_statuses(&mut self, cx: &mut Context) { let associations = self.by_repo.clone(); @@ -635,10 +600,6 @@ fn checkout_status(path: &Path, announced_head: Option<&str>) -> Option Option { if signed_git::worktree_dirty(path) { return None; @@ -649,8 +610,6 @@ fn checkout_push_status(path: &Path, fetch: bool) -> Option { let origin = signed_git::origin_url(path).ok().flatten()?; if fetch { - // Refresh the remote heads first. - // Commits made elsewhere or pushed from another machine must not linger as `to push`. signed_git::fetch_repo_refs(path, &[origin], "+refs/heads/*:refs/remotes/origin/*").ok(); } @@ -678,10 +637,6 @@ fn checkout_push_status(path: &Path, fetch: bool) -> Option { } /// Compute the requested statuses against the checkout paths of `associations`. -/// -/// Shared by the full and the local pass. `fetch` refreshes the checkouts' -/// remote heads first, so the full pass sees remote moves; the fast local -/// pass reads the tracking refs only, which is enough to detect local commits. fn compute_statuses( associations: &HashMap>, requested: &[(RepoAddr, Option)], @@ -737,12 +692,14 @@ pub fn pr_proposes_checkout( if pr.kind != Kind::GitPullRequest || !open || pr.pubkey != user { return false; } + let branch_matches = pr .tags .iter() .find(|t| t.kind() == "branch-name") .and_then(|t| t.content()) .is_some_and(|name| name == checkout.branch); + // A renamed branch falls back to the proposed tip commit. let tip_matches = pr .tags @@ -750,6 +707,7 @@ pub fn pr_proposes_checkout( .find(|t| t.kind() == "c") .and_then(|t| t.content()) .is_some_and(|tip| tip == checkout.head); + branch_matches || tip_matches } diff --git a/crates/signed_state/src/refresh.rs b/crates/signed_state/src/refresh.rs index e527e0f..695cb7c 100644 --- a/crates/signed_state/src/refresh.rs +++ b/crates/signed_state/src/refresh.rs @@ -18,10 +18,6 @@ impl RefreshGate { self.running } - /// A new refresh request arrived. - /// - /// Folded into a follow-up run while one is in flight, otherwise the - /// caller starts the run itself. pub fn request(&mut self) -> RefreshRequest { if self.running { self.dirty = true; diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 124208b..4be8861 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -20,7 +20,6 @@ use crate::backend::{ }; use crate::checkouts::CheckoutsStore; use crate::git_store::ensure_repo_mirror; -use crate::refresh::{RefreshGate, RefreshRequest}; use crate::repos::RepoListStore; /// Maximum size of one patch event. @@ -31,7 +30,6 @@ const MAX_PATCH_EVENT_BYTES: usize = 60 * 1024; /// Per-repository store. /// /// Holds the announcement, state, issues, patches, PRs, comments and resolved statuses. -/// Always derived from the local database. pub struct RepoStore { /// NIP-34 address. `None` while the repository is local-only. addr: Option, @@ -89,7 +87,8 @@ pub struct RepoStore { /// /// Avoids re-running the maintainer Auto sync on every refresh. synced_maintainers: HashSet, - refresh: RefreshGate, + /// In-flight refresh tasks. + tasks: Vec>>, /// Backend subscription of an announced repository. `None` while local-only. _subscription: Option, } @@ -138,7 +137,7 @@ impl RepoStore { repo_relays: HashSet::new(), root_fetches: HashSet::new(), synced_maintainers: HashSet::new(), - refresh: RefreshGate::default(), + tasks: Vec::new(), _subscription: Some(subscription), } } @@ -167,7 +166,7 @@ impl RepoStore { repo_relays: HashSet::new(), root_fetches: HashSet::new(), synced_maintainers: HashSet::new(), - refresh: RefreshGate::default(), + tasks: Vec::new(), _subscription: None, } } @@ -365,11 +364,6 @@ impl RepoStore { if self.addr.is_none() { return; } - - if self.refresh.request() != RefreshRequest::Schedule { - return; - } - self.run_refresh(cx); } @@ -378,8 +372,6 @@ impl RepoStore { return; }; - self.refresh.begin(); - let backend = Backend::global(cx); let client = backend.read(cx).client(); @@ -498,7 +490,7 @@ impl RepoStore { )) }); - cx.spawn(async move |this, cx| { + let task = cx.spawn(async move |this, cx| { let ( announcement, state, @@ -513,14 +505,13 @@ impl RepoStore { Ok(data) => data, Err(e) => { return this.update(cx, |this, cx| { - this.refresh.abort(); this.last_error = Some(e.to_string()); cx.notify(); }); } }; - let again = this.update(cx, |this, cx| { + this.update(cx, |this, cx| { let keep_hint = announcement.is_none() && !this.loaded; let first_pass = !this.loaded; @@ -604,17 +595,12 @@ impl RepoStore { if changed { cx.notify(); } - - this.refresh.finish() })?; - if again { - this.update(cx, |this, cx| this.refresh(cx))?; - } - Ok(()) - }) - .detach(); + }); + + self.tasks.push(task); } /// Resolve the status of a root event, an issue, patch or PR, per NIP-34. -- 2.54.0 From 8ea320cd6f1cdd846013081aae96da35afb3a47d Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 27 Sep 2026 09:24:41 +0700 Subject: [PATCH 08/10] fix leak tasks --- crates/signed_state/src/repo.rs | 40 ++++++++----------- crates/workspace/src/views/commit_diff/mod.rs | 4 +- .../src/views/pull_requests/detail.rs | 6 +-- .../workspace/src/views/pull_requests/new.rs | 15 ++++--- 4 files changed, 33 insertions(+), 32 deletions(-) diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index 4be8861..e2548ad 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -87,7 +87,7 @@ pub struct RepoStore { /// /// Avoids re-running the maintainer Auto sync on every refresh. synced_maintainers: HashSet, - /// In-flight refresh tasks. + /// In-flight tasks, cancelled when the store drops. tasks: Vec>>, /// Backend subscription of an announced repository. `None` while local-only. _subscription: Option, @@ -770,9 +770,9 @@ impl RepoStore { .collect() }; - cx.spawn(async move |this, cx| { - // The PR references the root patch event. - // Viewers can then find the patch without carrying it inline. + 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( &this, cx, @@ -838,24 +838,17 @@ impl RepoStore { }; let builder = this.update(cx, |this, _cx| { - // NIP-34 PRs carry at least one clone URL. - // The tip commit is downloadable from it. - // The author's `/prs/` URLs come first. - // They are author-controlled and most likely alive. - // The announced mirrors follow. - // The list is fixed before signing. - // The pushed ref name embeds the event id. - // Every candidate URL is listed up front. - // Dead URLs are inert, the linked patch stays the source of truth. let prs_urls: Vec = author_targets .iter() .filter_map(|(url, _)| Url::parse(url).ok()) .collect(); + let base_clone = this .announcement .as_ref() .map(|a| a.clone.clone()) .unwrap_or_default(); + let clone = pr_clone_urls(prs_urls, base_clone); let builder = GitPullRequest { @@ -880,8 +873,6 @@ impl RepoStore { })?; // Sign before publishing. - // The tip is pushed to the grasp servers under `refs/nostr/`. - // Nak's convention, readers fetch that ref for the commit behind the `c` tag. let event = cx .background_spawn({ let signer = signer.clone(); @@ -892,20 +883,22 @@ impl RepoStore { if let Some(path) = push_from.as_ref() { let tip = current_commit.to_string(); let reference = format!("refs/nostr/{}", event.id.to_hex()); + let (pushed, failures) = cx .background_spawn({ let path = path.clone(); let tip = tip.clone(); let reference = reference.clone(); // Author servers first, then the announced base grasp servers. - // The extra targets are best-effort redundancy. let targets: Vec<(String, String)> = author_targets .into_iter() .chain(base_targets) .collect(); + async move { let mut failures = Vec::new(); let mut pushed = 0; + for (url, label) in &targets { match signed_git::push_commit_ref( &path, url, &tip, &reference, @@ -914,6 +907,7 @@ impl RepoStore { Err(e) => failures.push(format!("{label}: {e}")), } } + (pushed, failures) } }) @@ -957,8 +951,8 @@ impl RepoStore { } Ok(()) - }) - .detach(); + }); + self.tasks.push(task); } /// Generate the patch between `merge_base` and `compare_ref` in `repo_path`, @@ -1086,7 +1080,7 @@ impl RepoStore { .map(|a| a.clone.clone()) .unwrap_or_default(); - cx.spawn(async move |this, cx| { + let task: Task> = cx.spawn(async move |this, cx| { if let Err(e) = publish_patch_series( &this, cx, @@ -1145,8 +1139,8 @@ impl RepoStore { } Ok(()) - }) - .detach(); + }); + self.tasks.push(task); } /// Set the status of a root event. @@ -1272,7 +1266,7 @@ impl RepoStore { } Ok(()) }); - task.detach(); + self.tasks.push(task); } /// The latest announcement of this repository, @@ -1588,7 +1582,7 @@ impl RepoStore { Ok(()) }); - task.detach(); + self.tasks.push(task); } } diff --git a/crates/workspace/src/views/commit_diff/mod.rs b/crates/workspace/src/views/commit_diff/mod.rs index b907adb..17799c8 100644 --- a/crates/workspace/src/views/commit_diff/mod.rs +++ b/crates/workspace/src/views/commit_diff/mod.rs @@ -303,6 +303,7 @@ pub struct CommitDiffView { loading: bool, error: Option, pane: Entity, + tasks: Vec>>, } impl CommitDiffView { @@ -336,6 +337,7 @@ impl CommitDiffView { loading: true, error: None, pane, + tasks: Vec::new(), } } @@ -383,7 +385,7 @@ impl CommitDiffView { Ok(()) }); - task.detach(); + self.tasks.push(task); } fn render_header(&self, cx: &mut Context) -> AnyElement { diff --git a/crates/workspace/src/views/pull_requests/detail.rs b/crates/workspace/src/views/pull_requests/detail.rs index 89dd0e4..d1385c3 100644 --- a/crates/workspace/src/views/pull_requests/detail.rs +++ b/crates/workspace/src/views/pull_requests/detail.rs @@ -79,8 +79,7 @@ pub struct PullRequestDetailView { pane: Entity, commit_item_sizes: Rc>>, commit_scroll_handle: VirtualListScrollHandle, - /// The dock caches item panels, so without this observer a panel opened - /// before the store loaded would stay on its placeholder. + tasks: Vec>>, _subscription: Subscription, } @@ -124,6 +123,7 @@ impl PullRequestDetailView { pane, commit_item_sizes: Rc::new(Vec::new()), commit_scroll_handle: VirtualListScrollHandle::new(), + tasks: Vec::new(), _subscription: subscription, } } @@ -327,7 +327,7 @@ impl PullRequestDetailView { Ok(()) }); - task.detach(); + self.tasks.push(task); } /// Open the diff of `commit_id` in the bottom dock of the area. diff --git a/crates/workspace/src/views/pull_requests/new.rs b/crates/workspace/src/views/pull_requests/new.rs index a3e3ef8..4d29ba1 100644 --- a/crates/workspace/src/views/pull_requests/new.rs +++ b/crates/workspace/src/views/pull_requests/new.rs @@ -68,6 +68,7 @@ pub struct NewPullRequestView { pane: Entity, scroll_handle: VirtualListScrollHandle, item_sizes: Rc>>, + tasks: Vec>>, _subscriptions: Vec, } @@ -323,6 +324,7 @@ impl NewPullRequestView { pane, scroll_handle: VirtualListScrollHandle::new(), item_sizes: Rc::new(Vec::new()), + tasks: Vec::new(), _subscriptions: subscriptions, } } @@ -390,7 +392,8 @@ impl NewPullRequestView { Ok(()) }); - task.detach(); + + self.tasks.push(task); } /// Branches and the current branch are read off the UI thread, then applied. @@ -419,7 +422,8 @@ impl NewPullRequestView { Ok(()) }); - task.detach(); + + self.tasks.push(task); } fn apply_checkout( @@ -636,7 +640,8 @@ impl NewPullRequestView { Ok(()) }); - task.detach(); + + self.tasks.push(task); } #[allow(clippy::too_many_arguments)] @@ -830,7 +835,7 @@ impl NewPullRequestView { Ok(()) }); - task.detach(); + self.tasks.push(task); } fn submit(&mut self, window: &mut Window, cx: &mut Context) { @@ -911,7 +916,7 @@ impl NewPullRequestView { Ok(()) }); - task.detach(); + self.tasks.push(task); } fn open_commit_diff(&mut self, commit_id: &str, window: &mut Window, cx: &mut Context) { -- 2.54.0 From 70006f74f0e78231de9fc951efc16fa6f216b8fa Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 27 Sep 2026 09:38:55 +0700 Subject: [PATCH 09/10] optimize --- crates/signed_state/src/backend.rs | 4 +- crates/signed_state/src/inbox.rs | 9 ++- crates/signed_state/src/repo.rs | 68 ++++++++++--------- crates/workspace/src/views/inbox.rs | 8 ++- .../src/views/sidebar/grasp_servers.rs | 10 ++- 5 files changed, 57 insertions(+), 42 deletions(-) diff --git a/crates/signed_state/src/backend.rs b/crates/signed_state/src/backend.rs index 31f6ab2..ccdf0cc 100644 --- a/crates/signed_state/src/backend.rs +++ b/crates/signed_state/src/backend.rs @@ -1066,7 +1066,7 @@ impl Backend { ) .await?; - for url in user_grasp_list_servers(client.clone(), public_key).await? { + for url in user_grasp_list_servers(&client, public_key).await? { client.add_relay(url).and_connect().await.ok(); } @@ -1525,7 +1525,7 @@ fn latest_grasp_list_servers(events: Vec) -> Vec { } pub async fn user_grasp_list_servers( - client: Client, + client: &Client, user: PublicKey, ) -> Result, Error> { let events: Vec = client diff --git a/crates/signed_state/src/inbox.rs b/crates/signed_state/src/inbox.rs index 8d66fec..aed4baa 100644 --- a/crates/signed_state/src/inbox.rs +++ b/crates/signed_state/src/inbox.rs @@ -104,11 +104,16 @@ impl Inbox { /// Sign the state with a random key and store it locally. fn persist(&mut self, cx: &mut Context) { - let Some(me) = Backend::global(cx).read(cx).current_user() else { + let backend = Backend::global(cx); + let (me, client) = { + let backend = backend.read(cx); + (backend.current_user(), backend.client()) + }; + + let Some(me) = me else { return; }; - let client = Backend::global(cx).read(cx).client(); let state = self.state.clone(); let task: Task> = cx.background_spawn(async move { diff --git a/crates/signed_state/src/repo.rs b/crates/signed_state/src/repo.rs index e2548ad..ffd9623 100644 --- a/crates/signed_state/src/repo.rs +++ b/crates/signed_state/src/repo.rs @@ -4,7 +4,7 @@ use std::path::PathBuf; use anyhow::{Error, bail}; use bitcoin_hashes::sha1::Hash as Sha1Hash; -use gpui::{App, AppContext, AsyncApp, Context, SharedString, Subscription, Task, WeakEntity}; +use gpui::{App, AppContext, Context, SharedString, Subscription, Task}; use nostr::event::IntoEventBuilder; use nostr_sdk::prelude::*; use settings::{EventFetchingStrategy, SettingsStore}; @@ -13,6 +13,7 @@ use signed_core::{ pull_request_patches, }; use signed_git::Nip34Binding; +use signed_nostr::UniversalSigner; use crate::backend::{ Backend, BackendEvent, grasp_base_url, grasp06_prs_url, pr_clone_urls, require_relay_accepted, @@ -733,9 +734,12 @@ impl RepoStore { }; let backend = Backend::global(cx); - let signer = backend.read(cx).signer(); + let (signer, client, user) = { + let backend = backend.read(cx); + (backend.signer(), backend.client(), backend.current_user()) + }; - let Some(user) = backend.read(cx).current_user() else { + let Some(user) = user else { self.last_error = Some("Sign in to open a pull request".into()); cx.notify(); return; @@ -774,8 +778,8 @@ impl RepoStore { // The PR references the root patch, // viewers can then find the patch without carrying it inline. let root_patch = match publish_patch_series( - &this, - cx, + &client, + &signer, &addr, owner, euc.as_deref(), @@ -800,11 +804,14 @@ impl RepoStore { // 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. let author_servers = { - let query = this.update(cx, |_this, cx| { - let client = Backend::global(cx).read(cx).client(); - user_grasp_list_servers(client, user) - })?; - match cx.background_spawn(query).await { + let query_client = client.clone(); + let published = cx + .background_spawn( + async move { user_grasp_list_servers(&query_client, user).await }, + ) + .await; + + match published { Ok(published) if !published.is_empty() => published, _ => defaults, } @@ -924,8 +931,6 @@ impl RepoStore { } } - let client = this.update(cx, |_this, cx| Backend::global(cx).read(cx).client())?; - let publish_result: Result = async { let output = client.send_event(&event).broadcast().await?; require_relay_accepted(output, event) @@ -1016,8 +1021,12 @@ impl RepoStore { self.last_warning = None; let backend = Backend::global(cx); + let (user, client, signer) = { + let backend = backend.read(cx); + (backend.current_user(), backend.client(), backend.signer()) + }; - let Some(user) = backend.read(cx).current_user() else { + let Some(user) = user else { self.last_error = Some("Sign in to update the pull request".into()); cx.notify(); return; @@ -1071,9 +1080,11 @@ impl RepoStore { self.not_announced(cx); return; }; + let owner = addr.public_key; let euc = self.announcement.as_ref().and_then(|a| a.euc.clone()); let root = root.clone(); + let clone: Vec = self .announcement .as_ref() @@ -1082,8 +1093,8 @@ impl RepoStore { let task: Task> = cx.spawn(async move |this, cx| { if let Err(e) = publish_patch_series( - &this, - cx, + &client, + &signer, &addr, owner, euc.as_deref(), @@ -1099,7 +1110,7 @@ impl RepoStore { }); } - let builder = this.update(cx, |_this, _cx| { + let builder = { let builder = GitPullRequestUpdate { repository: addr.clone(), pull_request_event: root.id, @@ -1116,13 +1127,7 @@ impl RepoStore { Some(euc) => builder.tag(Tag::parse(["r", euc]).expect("valid r tag")), None => builder, } - })?; - - let (client, signer) = this.update(cx, |_this, cx| { - let backend = Backend::global(cx); - let backend = backend.read(cx); - (backend.client(), backend.signer()) - })?; + }; let publish_result: Result = async { let event = builder.finalize_async(&signer).await?; @@ -1652,8 +1657,8 @@ fn patch_current_commit(patch: &str) -> Option<&str> { /// Returns the root event, the one a PR references. #[allow(clippy::too_many_arguments)] async fn publish_patch_series( - this: &WeakEntity, - cx: &mut AsyncApp, + client: &Client, + signer: &UniversalSigner, addr: &RepoAddr, owner: PublicKey, euc: Option<&str>, @@ -1661,12 +1666,6 @@ async fn publish_patch_series( first_marker: &str, reply_to: Option, ) -> Result { - let (client, signer) = this.update(cx, |_this, cx| { - let backend = Backend::global(cx); - let backend = backend.read(cx); - (backend.client(), backend.signer()) - })?; - let mut root: Option = None; let mut previous = reply_to; @@ -1679,6 +1678,7 @@ async fn publish_patch_series( }; let mut tags = vec![Tag::coordinate(addr.clone(), None), Tag::public_key(owner)]; + if ix == 0 { if let Ok(tag) = Tag::parse(["t", first_marker]) { tags.push(tag); @@ -1693,21 +1693,23 @@ async fn publish_patch_series( { tags.push(tag); } + if let Some(euc) = euc && let Ok(tag) = Tag::parse(["r", euc]) { tags.push(tag); } + if let Ok(tag) = Tag::parse(["commit", commit]) { tags.push(tag); } + if let Ok(tag) = Tag::parse(["r", commit]) { tags.push(tag); } let builder = EventBuilder::new(Kind::GitPatch, part.clone()).tags(tags); - - let event = builder.finalize_async(&signer).await?; + let event = builder.finalize_async(signer).await?; let output = client.send_event(&event).broadcast().await?; let event = require_relay_accepted(output, event)?; diff --git a/crates/workspace/src/views/inbox.rs b/crates/workspace/src/views/inbox.rs index 81d9301..8f04a67 100644 --- a/crates/workspace/src/views/inbox.rs +++ b/crates/workspace/src/views/inbox.rs @@ -187,12 +187,16 @@ impl InboxView { self.refresh.begin(); let backend = Backend::global(cx); - let Some(me) = backend.read(cx).current_user() else { + let (me, client) = { + let backend = backend.read(cx); + (backend.current_user(), backend.client()) + }; + + let Some(me) = me else { self.refresh.abort(); return; }; - let client = backend.read(cx).client(); let state = self.state.clone(); let work = cx.background_spawn(async move { query_inbox(&client, me, &state).await }); diff --git a/crates/workspace/src/views/sidebar/grasp_servers.rs b/crates/workspace/src/views/sidebar/grasp_servers.rs index 2776b4c..1b5d1a9 100644 --- a/crates/workspace/src/views/sidebar/grasp_servers.rs +++ b/crates/workspace/src/views/sidebar/grasp_servers.rs @@ -215,15 +215,19 @@ pub fn load_user_grasp_servers( cx: &mut App, ) { let backend = Backend::global(cx); - let Some(user) = backend.read(cx).current_user() else { + let (user, client) = { + let backend = backend.read(cx); + (backend.current_user(), backend.client()) + }; + + let Some(user) = user else { state.update(cx, |state, _| state.loading_servers = false); return; }; - let client = backend.read(cx).client(); let handle = window.window_handle(); cx.spawn(async move |cx| { - let result = signed_state::user_grasp_list_servers(client, user).await; + let result = signed_state::user_grasp_list_servers(&client, user).await; let _ = cx.update_window(handle, |_, _window, cx| { state.update(cx, |state, _| { -- 2.54.0 From dc3a051a0da15a1902c2dc62e80f63d7b3368a30 Mon Sep 17 00:00:00 2001 From: Ren Amamiya Date: Sun, 27 Sep 2026 09:47:52 +0700 Subject: [PATCH 10/10] . --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1981338..abd775e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,14 +15,18 @@ - Migrate the GPUI foundation to the published `gpui-pre` crates and GPUI Kit 0.6, off the zed and gpui-component git pins - Use the pixel avatar as the single fallback for a missing picture, sized and rounded to match the other avatars - Redesign the dock tab bar, using muted grey active tab, added close buttons, double-click to zoom, and removed panel toolbar +- Connect to fewer relays at startup, keeping only ditto and the git indexer as bootstrap relays ### Fixed - Render every avatar at one consistent size, where a surrounding border had shrunk pictures by two pixels and the pixel avatar ignored an explicit size - Date repositories from their repository state event, so the explore list and open repository views show the latest push instead of the announcement date +- Fix background tasks outliving their view, so a closed repository panel or pull request view stops fetching and publishing on its own ### Removed +- Remove cover note support, the kind-1624 GitWorkshop and `ngit` extension outside the NIP-34 + ### Deprecated ## v0.1.0-alpha - 2026/09/14 -- 2.54.0