diff --git a/crates/sprout-cli/TESTING.md b/crates/sprout-cli/TESTING.md index 1d0ed0452..ec1c57197 100644 --- a/crates/sprout-cli/TESTING.md +++ b/crates/sprout-cli/TESTING.md @@ -401,6 +401,44 @@ sprout messages vote --event "$FORUM_EVENT_ID" --direction up | jq . sprout messages vote --event "$FORUM_EVENT_ID" --direction down | jq . ``` +### 6.12 Notes (NIP-23 long-form, kind:30023) + +Editable team-knowledge notes keyed by `(kind:30023, you, d=slug)`. `set` is an +idempotent upsert; `rm` is a NIP-09 a-tag deletion. Output is plain text (refs), +not JSON — except `get`/`ls`, which emit JSON. + +```bash +# set (first publish — --title required, body from stdin) +cat <<'EOF' | sprout notes set --name dco-check --title "DCO Check" \ + --summary "How we verify DCO" --tag dco --tag ci --content - +Run `git log --format='%(trailers:key=Signed-off-by)'` ... +EOF +# → prints event_id / naddr / coordinate / slug / title + +# set (edit — omit --title to carry it forward; published_at preserved) +echo "Updated body." | sprout notes set --name dco-check --content - + +# get by name (own author resolves directly; cross-author #d query otherwise) +sprout notes get --name dco-check | jq . +sprout notes get --name dco-check --content-only + +# get by naddr (exact coordinate; paste the naddr from a set/get above) +sprout notes get --naddr "$NADDR" | jq . + +# ls (own by default; --author all across the team; --tag filters) +sprout notes ls | jq . +sprout notes ls --tag dco | jq . +sprout notes ls --author all --limit 10 | jq . + +# rm (NIP-09 a-tag deletion; subsequent get must 404) +sprout notes rm --name dco-check +# → prints deleted / deletion +sprout notes get --name dco-check # exits non-zero: not found + +# rm of a slug you never published → NotFound, no kind:5 emitted +sprout notes rm --name does-not-exist # exits non-zero +``` + --- ## 7. Error Path Testing @@ -532,3 +570,7 @@ sprout channels delete --channel "$FORUM_ID" | jq . | 52 | `upload file` | ☐ | | | 53 | `pack validate` | ☐ | Local, no relay | | 54 | `pack inspect` | ☐ | Local, no relay | +| 55 | `notes set` | ☐ | First publish, edit/carry, --clear-tags, ambiguity, empty-stdin guard | +| 56 | `notes get` | ☐ | By name, by naddr, --content-only, cross-author, ambiguous → exit 1 | +| 57 | `notes ls` | ☐ | Own, --author all, --tag, --limit | +| 58 | `notes rm` | ☐ | Delete→get 404, double-delete idempotent, missing slug → NotFound | diff --git a/crates/sprout-cli/src/commands/mod.rs b/crates/sprout-cli/src/commands/mod.rs index d3b066b6d..a1536cac1 100644 --- a/crates/sprout-cli/src/commands/mod.rs +++ b/crates/sprout-cli/src/commands/mod.rs @@ -3,6 +3,7 @@ pub mod dms; pub mod feed; pub mod mem; pub mod messages; +pub mod notes; pub mod pack; pub mod reactions; pub mod repos; diff --git a/crates/sprout-cli/src/commands/notes.rs b/crates/sprout-cli/src/commands/notes.rs new file mode 100644 index 000000000..9fcd0f317 --- /dev/null +++ b/crates/sprout-cli/src/commands/notes.rs @@ -0,0 +1,1309 @@ +//! `sprout notes` — NIP-23 long-form editable notes (kind:30023). +//! +//! Skill-sharing knowledge base for the team. Notes are parameterized-replaceable +//! events keyed by `(kind=30023, pubkey, d-tag)`; the `d` tag is the human slug. +//! +//! ## Verbs +//! - `notes set --name --title T [--summary S] [--tag t]... --content -` +//! Idempotent upsert. Read-before-write preserves `published_at` and carries +//! forward the existing title when `--title` is omitted on an update. +//! - `notes get (--naddr | --name [--author ]) [--content-only]` +//! `--naddr` / coordinate form is exact. `--name` does a cross-author `#d` +//! query; >1 hit prints candidates and exits 1. +//! - `notes ls [--author ] [--tag t] [--limit N]` — own notes by default. +//! - `notes rm --name ` — NIP-09 deletion (kind:5) targeting the addressable +//! coordinate via an `a` tag only (no `e` tag — see [`build_rm_event`]). +//! +//! ## Implementation state +//! `set`/`get`/`ls`/`rm` are implemented. `set` is the write path: stdin handling, +//! read-before-write via [`fetch_own_note`], pure carry-forward via +//! [`build_set_event`], sign + publish, and the durable-reference output line. +//! `get`/`ls` are the read paths (coordinate / cross-author `#d` resolution and +//! bounded listing). `rm` is the delete path: read-before-write for a clear +//! "nothing to delete" signal, then an a-tag-only kind:5 (the relay's #714 +//! coordinate soft-delete lands in this same change). + +use std::io::Read; +use std::str::FromStr; +use std::time::SystemTime; + +use nostr::{Event, EventBuilder, Kind, PublicKey, Tag, Timestamp, ToBech32}; + +use crate::client::SproutClient; +use crate::error::CliError; +use crate::validate::validate_hex64; + +/// NIP-23 long-form content kind. +pub const KIND_LONG_FORM: u16 = 30023; + +/// Hard cap on slug length. NIP-23 doesn't bound it; we pick a value that's +/// comfortably URL/filename-safe and matches `mem` slug ergonomics. +pub const SLUG_MAX_LEN: usize = 80; + +// --------------------------------------------------------------------------- +// Slug validation +// --------------------------------------------------------------------------- + +/// Validate and normalize a slug for use as a NIP-23 `d` tag. +/// +/// Rules: 1..=80 chars, `[a-z0-9._-]` only. Lowercase ascii keeps memory +/// pointers and shell-safe filenames trivial; the protocol allows more but +/// being strict here costs us nothing and prevents the "is `Dco-Recipe` the +/// same slug as `dco-recipe`?" ambiguity. +pub fn parse_slug(raw: &str) -> Result { + if raw.is_empty() { + return Err(CliError::Usage("slug cannot be empty".into())); + } + if raw.len() > SLUG_MAX_LEN { + return Err(CliError::Usage(format!( + "slug too long ({} > {SLUG_MAX_LEN} chars)", + raw.len() + ))); + } + for c in raw.chars() { + let ok = c.is_ascii_lowercase() || c.is_ascii_digit() || matches!(c, '-' | '_' | '.'); + if !ok { + return Err(CliError::Usage(format!( + "slug contains invalid character {c:?}; allowed: a-z 0-9 - _ ." + ))); + } + } + Ok(raw.to_string()) +} + +// --------------------------------------------------------------------------- +// NoteSnapshot — parsed view of a kind:30023 event, derived once +// --------------------------------------------------------------------------- + +/// Parsed view of a NIP-23 long-form event. Built once via +/// [`NoteSnapshot::from_event`] so the tag-parsing footgun lives in exactly +/// one place; `set` (carry-forward), `get`/`ls` (output shaping), and the +/// ambiguous-candidate path all consume the same derived fields. +#[derive(Debug, Clone)] +pub struct NoteSnapshot { + /// Event id of this specific incarnation (not the addressable coordinate). + pub id: nostr::EventId, + /// Author pubkey. + pub pubkey: PublicKey, + /// `d` tag — the slug. + pub slug: String, + /// NIP-23 `title` tag (required by spec; empty if author omitted it). + pub title: String, + /// NIP-23 `summary` tag. + pub summary: Option, + /// Repeated `t` topic tags. + pub tags: Vec, + /// NIP-23 `published_at` tag, unix seconds. `None` if absent on this event. + pub published_at: Option, + /// `created_at` of this incarnation — the LWW key for the addressable + /// coordinate; reasonable proxy for "last updated". + pub updated_at: u64, + /// Raw markdown body. + pub content: String, +} + +impl NoteSnapshot { + /// Parse a kind:30023 event into a snapshot. Returns `Err` if the event + /// is the wrong kind or missing the mandatory `d` tag. + pub fn from_event(event: &Event) -> Result { + if event.kind != Kind::Custom(KIND_LONG_FORM) { + return Err(CliError::Other(format!( + "expected kind:{KIND_LONG_FORM}, got {}", + event.kind.as_u16() + ))); + } + + let mut slug: Option = None; + let mut title = String::new(); + let mut summary: Option = None; + let mut tags: Vec = Vec::new(); + let mut published_at: Option = None; + + for tag in event.tags.iter() { + let parts = tag.as_slice(); + let Some(name) = parts.first().map(String::as_str) else { + continue; + }; + let val = parts.get(1).map(String::as_str).unwrap_or(""); + match name { + "d" => slug = Some(val.to_string()), + "title" => title = val.to_string(), + "summary" => summary = Some(val.to_string()), + "t" if !val.is_empty() => tags.push(val.to_string()), + "published_at" => { + published_at = val.parse::().ok(); + } + _ => {} + } + } + + let slug = slug.ok_or_else(|| { + CliError::Other("kind:30023 event is missing the required `d` tag".into()) + })?; + + Ok(NoteSnapshot { + id: event.id, + pubkey: event.pubkey, + slug, + title, + summary, + tags, + published_at, + updated_at: event.created_at.as_u64(), + content: event.content.clone(), + }) + } + + /// Canonical addressable coordinate `(kind:30023, pubkey, slug)`. + pub fn coordinate(&self) -> nostr::nips::nip01::Coordinate { + coord_for(&self.pubkey, &self.slug) + } +} + +// --------------------------------------------------------------------------- +// Query helpers +// --------------------------------------------------------------------------- + +fn parse_events(json: &str) -> Result, CliError> { + serde_json::from_str::>(json) + .map_err(|e| CliError::Other(format!("failed to parse relay response: {e}"))) +} + +/// Read-before-write: fetch the caller's current `(kind:30023, me, d=slug)` +/// event. Returns `Ok(None)` when the slug doesn't exist for the caller. +/// +/// Stays protocol-pure (returns a raw `Event`): `set` calls +/// `NoteSnapshot::from_event` on the result for carry-forward, and `rm` only +/// needs its presence to decide first-publish vs. update / deletable-or-not. +/// (Quinn's option (b) — single tag-parser, isolated.) +pub async fn fetch_own_note(client: &SproutClient, slug: &str) -> Result, CliError> { + let me = client.keys().public_key(); + let filter = serde_json::json!({ + "kinds": [KIND_LONG_FORM], + "authors": [me.to_hex()], + "#d": [slug], + "limit": 1, + }); + let raw = client.query(&filter).await?; + let mut events = parse_events(&raw)?; + // Defensive: a parameterized-replaceable coordinate has at most one live + // event, but if the relay returns multiple we take the newest. + events.sort_by_key(|e| std::cmp::Reverse(e.created_at)); + Ok(events.into_iter().next()) +} + +/// Cross-author `#d` lookup for `get --name`. The relay pushes the `#d` +/// filter into SQL for NIP-33 kinds (`req.rs`), so this is a single +/// indexed query, not a fan-out. +pub async fn fetch_by_slug(client: &SproutClient, slug: &str) -> Result, CliError> { + let filter = serde_json::json!({ + "kinds": [KIND_LONG_FORM], + "#d": [slug], + "limit": 50, + }); + let raw = client.query(&filter).await?; + parse_events(&raw) +} + +// --------------------------------------------------------------------------- +// Author resolution +// --------------------------------------------------------------------------- + +/// Resolve an `--author` flag value to a `PublicKey`. +/// +/// Accepts: +/// - `"me"` → the CLI's own keypair. +/// - 64-hex pubkey → parsed directly. +/// - anything else → treated as a petname / display name, searched against +/// kind:0 profiles. Exact-one match required; ambiguity is a hard error. +pub async fn resolve_author( + client: &SproutClient, + author_flag: &str, +) -> Result { + if author_flag == "me" { + return Ok(client.keys().public_key()); + } + if validate_hex64(author_flag).is_ok() { + return PublicKey::from_hex(author_flag) + .map_err(|e| CliError::Usage(format!("invalid pubkey: {e}"))); + } + // Petname lookup: NIP-50 search on kind:0, then exact-name filter. + let filter = serde_json::json!({ + "kinds": [0], + "search": author_flag, + "limit": 100, + }); + let raw = client.query(&filter).await?; + let events = parse_events(&raw)?; + let lower = author_flag.to_ascii_lowercase(); + let matches: Vec<&Event> = events + .iter() + .filter(|e| { + let Ok(meta) = serde_json::from_str::(&e.content) else { + return false; + }; + let name = meta + .get("display_name") + .or_else(|| meta.get("name")) + .and_then(|v| v.as_str()) + .unwrap_or(""); + name.to_ascii_lowercase() == lower + }) + .collect(); + match matches.len() { + 0 => Err(CliError::Usage(format!( + "no user found with display_name {author_flag:?}; pass a 64-hex pubkey or \"me\"" + ))), + 1 => Ok(matches[0].pubkey), + n => Err(CliError::Usage(format!( + "{n} users match display_name {author_flag:?}; disambiguate with --author " + ))), + } +} + +// --------------------------------------------------------------------------- +// Coordinate parsing (naddr / kind:pk:d / NIP-21) +// --------------------------------------------------------------------------- + +/// Parse a `--naddr` flag. Accepts: +/// - bech32 `naddr1…` +/// - `::` (KPI format) +/// - `nostr:naddr1…` (NIP-21 URI) +/// +/// Errors if the parsed kind is not 30023 — `notes` doesn't address any other +/// kind, and silently succeeding on (say) a kind:30024 coordinate would just +/// confuse the user later. +pub fn parse_naddr(raw: &str) -> Result { + let coord = nostr::nips::nip01::Coordinate::from_str(raw) + .map_err(|e| CliError::Usage(format!("invalid naddr/coordinate: {e}")))?; + if coord.kind != Kind::Custom(KIND_LONG_FORM) { + return Err(CliError::Usage(format!( + "coordinate kind is {}, expected {KIND_LONG_FORM}", + coord.kind.as_u16() + ))); + } + if coord.identifier.is_empty() { + return Err(CliError::Usage( + "kind:30023 coordinate is missing its d-tag/slug".into(), + )); + } + Ok(coord) +} + +pub fn coord_for(author: &PublicKey, slug: &str) -> nostr::nips::nip01::Coordinate { + nostr::nips::nip01::Coordinate::new(Kind::Custom(KIND_LONG_FORM), *author) + .identifier(slug.to_string()) +} + +// --------------------------------------------------------------------------- +// Candidate formatting (used when --name resolves to >1 author) +// --------------------------------------------------------------------------- + +/// Format a list of candidate notes for the "ambiguous slug" error path. +/// One line per candidate; sorted newest-first. Designed so the user can +/// paste a pubkey into a follow-up `--author ` invocation. +pub fn format_note_candidates(snapshots: &[NoteSnapshot]) -> String { + let mut rows: Vec<&NoteSnapshot> = snapshots.iter().collect(); + rows.sort_by_key(|s| std::cmp::Reverse(s.updated_at)); + let mut out = String::new(); + for s in rows { + let title = if s.title.is_empty() { + "(untitled)" + } else { + s.title.as_str() + }; + out.push_str(&format!( + " {} {} {}\n", + s.pubkey.to_hex(), + s.updated_at, + title + )); + } + out +} + +// --------------------------------------------------------------------------- +// Output helpers for `get` / `ls` +// --------------------------------------------------------------------------- + +#[derive(Debug, serde::Serialize)] +struct NoteOutput { + id: String, + pubkey: String, + naddr: String, + coordinate: String, + slug: String, + title: String, + summary: Option, + tags: Vec, + published_at: Option, + updated_at: u64, + content: String, +} + +impl TryFrom<&NoteSnapshot> for NoteOutput { + type Error = CliError; + + fn try_from(snapshot: &NoteSnapshot) -> Result { + let coordinate = snapshot.coordinate(); + let naddr = coordinate + .to_bech32() + .map_err(|e| CliError::Other(format!("failed to encode naddr: {e}")))?; + Ok(Self { + id: snapshot.id.to_hex(), + pubkey: snapshot.pubkey.to_hex(), + naddr, + coordinate: coordinate.to_string(), + slug: snapshot.slug.clone(), + title: snapshot.title.clone(), + summary: snapshot.summary.clone(), + tags: snapshot.tags.clone(), + published_at: snapshot.published_at, + updated_at: snapshot.updated_at, + content: snapshot.content.clone(), + }) + } +} + +fn snapshot_from_event(event: &Event) -> Result { + NoteSnapshot::from_event(event) +} + +fn snapshots_from_events(events: Vec) -> Result, CliError> { + events + .iter() + .map(snapshot_from_event) + .collect::, _>>() +} + +async fn fetch_by_coord( + client: &SproutClient, + coord: &nostr::nips::nip01::Coordinate, +) -> Result, CliError> { + let filter = serde_json::json!({ + "kinds": [KIND_LONG_FORM], + "authors": [coord.public_key.to_hex()], + "#d": [coord.identifier], + "limit": 1, + }); + let raw = client.query(&filter).await?; + let mut events = parse_events(&raw)?; + events.sort_by_key(|e| std::cmp::Reverse(e.created_at)); + Ok(events.into_iter().next()) +} + +fn print_snapshot_json(snapshot: &NoteSnapshot) -> Result<(), CliError> { + let output = NoteOutput::try_from(snapshot)?; + println!( + "{}", + serde_json::to_string_pretty(&output) + .map_err(|e| CliError::Other(format!("failed to serialize note: {e}")))? + ); + Ok(()) +} + +fn print_snapshot_list_json(snapshots: &[NoteSnapshot]) -> Result<(), CliError> { + let output = snapshots + .iter() + .map(NoteOutput::try_from) + .collect::, _>>()?; + println!( + "{}", + serde_json::to_string_pretty(&output) + .map_err(|e| CliError::Other(format!("failed to serialize notes: {e}")))? + ); + Ok(()) +} + +fn sort_snapshots_newest_first(snapshots: &mut [NoteSnapshot]) { + snapshots.sort_by_key(|s| std::cmp::Reverse(s.updated_at)); +} + +// --------------------------------------------------------------------------- +// Event builder for `set` — pure, unit-testable carry-forward logic +// --------------------------------------------------------------------------- + +/// Build the unsigned `EventBuilder` for `notes set`. Pure function — no I/O, +/// no clock — so every carry/clear/first-publish case is unit-testable. +/// +/// # Carry-forward semantics (ratified) +/// All three of `title` / `summary` / `tags` use the same omit-vs-clear +/// pattern: `None` means "omit" (carry on update; spec-driven default on +/// create), `Some(empty)` means "explicit clear". +/// +/// - `title: None` → carry from `prior.title`; on first publish (`prior=None`) +/// this is a usage error (NIP-23 requires `title`). +/// - `title: Some("")` → explicit clear; emit empty `title` tag. +/// - `title: Some(s)` → use `s`. +/// - `summary`: same shape as `title`, but `None` on first publish is allowed +/// (summary is optional in NIP-23). +/// - `tags: None` → carry from `prior.tags` (on first publish: emit no +/// `t` tags). +/// - `tags: Some(&[])` → explicit clear; emit no `t` tags. +/// - `tags: Some(slice)` → replace existing `t` tags with `slice` verbatim +/// (not merged). +/// - `published_at`: preserved from `prior` if present; otherwise set to `now` +/// on first publish. +/// +/// `now` is **Unix seconds** (not millis). Used for the event's `created_at` +/// and, on first publish, for the `published_at` tag value. Injectable so +/// tests can assert `published_at` preservation deterministically. +/// +pub fn build_set_event( + prior: Option<&NoteSnapshot>, + slug: &str, + title: Option<&str>, + summary: Option<&str>, + tags: Option<&[String]>, + content: &str, + now: u64, +) -> Result { + // Resolve `title` against the carry-forward / clear / require-on-create matrix. + let title_value: &str = match (title, prior) { + (Some(t), _) => t, // explicit (empty = clear) + (None, Some(p)) => p.title.as_str(), // carry + (None, None) => { + return Err(CliError::Usage( + "--title is required on first publish (NIP-23)".into(), + )); + } + }; + + // `summary` is optional on create, so `None` on first-publish carries no value. + let summary_value: Option<&str> = match (summary, prior) { + (Some(s), _) => Some(s), // explicit (empty = clear) + (None, Some(p)) => p.summary.as_deref(), // carry if prior had one + (None, None) => None, // first publish, no summary + }; + + // `tags`: None = carry-on-edit (or empty-on-create); Some(&[]) = clear; Some(slice) = replace. + let topic_tags: Vec = match (tags, prior) { + (Some(ts), _) => ts.to_vec(), + (None, Some(p)) => p.tags.clone(), + (None, None) => Vec::new(), + }; + + // `published_at`: preserve prior if present, otherwise set to `now` on first publish. + let published_at: u64 = prior.and_then(|p| p.published_at).unwrap_or(now); + + let mut evt_tags: Vec = Vec::with_capacity(4 + topic_tags.len()); + evt_tags.push(Tag::parse(&["d", slug]).map_err(tag_err)?); + evt_tags.push(Tag::parse(&["title", title_value]).map_err(tag_err)?); + if let Some(s) = summary_value { + evt_tags.push(Tag::parse(&["summary", s]).map_err(tag_err)?); + } + for t in &topic_tags { + evt_tags.push(Tag::parse(&["t", t]).map_err(tag_err)?); + } + evt_tags.push(Tag::parse(&["published_at", &published_at.to_string()]).map_err(tag_err)?); + + Ok( + EventBuilder::new(Kind::Custom(KIND_LONG_FORM), content, evt_tags) + .custom_created_at(Timestamp::from(now)), + ) +} + +fn tag_err(e: impl std::fmt::Display) -> CliError { + CliError::Other(format!("failed to build tag: {e}")) +} + +fn now_secs() -> u64 { + SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0) +} + +/// Bound on stdin reads for `--content -`. NIP-23 doesn't cap event size; +/// this is a guardrail against runaway producers OOMing the CLI. 1 MiB is +/// far above any realistic skill-KB note. +pub const SET_STDIN_MAX_BYTES: usize = 1024 * 1024; + +// --------------------------------------------------------------------------- +// Dispatch — stubs for verb implementations (filled by follow-up commits). +// --------------------------------------------------------------------------- + +pub async fn cmd_set( + client: &SproutClient, + slug: &str, + title: Option<&str>, + summary: Option<&str>, + tags: Option<&[String]>, + content: &str, + allow_empty: bool, +) -> Result<(), CliError> { + // Resolve `--content -` from stdin with a hard byte cap. Mirrors the + // `mem set` hygiene: empty stdin without `--allow-empty` is refused + // (almost always an upstream pipeline failure). + let body: String = if content == "-" { + let mut buf = String::new(); + std::io::stdin() + .take((SET_STDIN_MAX_BYTES as u64) + 1) + .read_to_string(&mut buf) + .map_err(|e| CliError::Other(format!("stdin read failed: {e}")))?; + if buf.len() > SET_STDIN_MAX_BYTES { + return Err(CliError::Usage(format!( + "stdin body exceeds {SET_STDIN_MAX_BYTES}-byte limit" + ))); + } + if buf.is_empty() && !allow_empty { + return Err(CliError::Usage( + "refusing to publish an empty body from stdin (an upstream pipeline step \ + likely failed). Pass --allow-empty to confirm." + .into(), + )); + } + buf + } else { + content.to_string() + }; + + // Read-before-write: fetch the current event for (me, slug), if any. + // We use the raw event from `fetch_own_note` and parse once via + // `NoteSnapshot::from_event` so the tag-handling lives in exactly one + // place. `None` here means this is a first-publish. + let prior_event = fetch_own_note(client, slug).await?; + let prior_snapshot = prior_event + .as_ref() + .map(NoteSnapshot::from_event) + .transpose()?; + + let builder = build_set_event( + prior_snapshot.as_ref(), + slug, + title, + summary, + tags, + &body, + now_secs(), + )?; + + let event = client.sign_event(builder)?; + let event_id = event.id; + let me = event.pubkey; + let raw = client.submit_event(event).await?; + let parsed: serde_json::Value = serde_json::from_str(&raw) + .map_err(|e| CliError::Other(format!("relay response is not JSON: {e} ({raw})")))?; + let accepted = parsed + .get("accepted") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + let message = parsed.get("message").and_then(|v| v.as_str()).unwrap_or(""); + if !accepted { + return Err(CliError::Other(format!("relay rejected event: {message}"))); + } + // NIP-33 LWW: the relay accepts but reports `duplicate:` when this write was + // dominated by a newer (or same-second) head and did NOT become the live + // note. Surface a Conflict so we don't print success for a write that + // didn't take. (Mirrors `mem`'s engram submit.) + if message.starts_with("duplicate:") || message == "duplicate" { + return Err(CliError::Conflict( + "relay reported event as duplicate / dominated by a newer head".into(), + )); + } + + // Print the durable references the user can paste into memory. + let coord = coord_for(&me, slug); + let naddr = coord + .to_bech32() + .map_err(|e| CliError::Other(format!("naddr encoding failed: {e}")))?; + println!("event_id {}", event_id.to_hex()); + println!("naddr {naddr}"); + println!("coordinate {KIND_LONG_FORM}:{}:{slug}", me.to_hex()); + println!("slug {slug}"); + // Resolved title actually written: explicit `--title` wins, else carried + // from prior. (Printing prior.title unconditionally was wrong on edits.) + let resolved_title = title + .or_else(|| prior_snapshot.as_ref().map(|p| p.title.as_str())) + .unwrap_or_default(); + println!("title {resolved_title}"); + Ok(()) +} + +pub async fn cmd_get( + client: &SproutClient, + naddr: Option<&str>, + name: Option<&str>, + author: Option<&str>, + content_only: bool, +) -> Result<(), CliError> { + let snapshot = if let Some(raw) = naddr { + let coord = parse_naddr(raw)?; + let event = fetch_by_coord(client, &coord).await?.ok_or_else(|| { + CliError::NotFound(format!( + "note not found: {}:{}:{}", + coord.kind.as_u16(), + coord.public_key.to_hex(), + coord.identifier + )) + })?; + snapshot_from_event(&event)? + } else { + let slug = parse_slug(name.expect("dispatch enforces --name xor --naddr"))?; + if let Some(author_flag) = author { + let author_pk = resolve_author(client, author_flag).await?; + let coord = coord_for(&author_pk, &slug); + let event = fetch_by_coord(client, &coord).await?.ok_or_else(|| { + CliError::NotFound(format!("note not found: {}/{}", author_pk.to_hex(), slug)) + })?; + snapshot_from_event(&event)? + } else { + let mut snapshots = snapshots_from_events(fetch_by_slug(client, &slug).await?)?; + match snapshots.len() { + 0 => return Err(CliError::NotFound(format!("note not found: {slug}"))), + 1 => snapshots.remove(0), + _ => { + sort_snapshots_newest_first(&mut snapshots); + return Err(CliError::Usage(format!( + "note name {slug:?} is ambiguous; pass --author \n{}", + format_note_candidates(&snapshots) + ))); + } + } + } + }; + + if content_only { + print!("{}", snapshot.content); + if !snapshot.content.ends_with('\n') { + println!(); + } + } else { + print_snapshot_json(&snapshot)?; + } + Ok(()) +} + +pub async fn cmd_ls( + client: &SproutClient, + author: Option<&str>, + tag: Option<&str>, + limit: Option, +) -> Result<(), CliError> { + let limit = limit.unwrap_or(50).min(200); + let author = author.unwrap_or("me"); + + let mut filter = serde_json::json!({ + "kinds": [KIND_LONG_FORM], + "limit": limit, + }); + + if author != "all" { + let author_pk = resolve_author(client, author).await?; + filter["authors"] = serde_json::json!([author_pk.to_hex()]); + } + + if let Some(tag) = tag { + if tag.is_empty() { + return Err(CliError::Usage("--tag cannot be empty".into())); + } + filter["#t"] = serde_json::json!([tag]); + } + + let raw = client.query(&filter).await?; + let mut snapshots = snapshots_from_events(parse_events(&raw)?)?; + sort_snapshots_newest_first(&mut snapshots); + print_snapshot_list_json(&snapshots)?; + Ok(()) +} + +/// Build the NIP-09 deletion event (kind:5) for an addressable coordinate. +/// +/// The deletion carries **only** an `a` tag (`30023::`) and no +/// `e` tag. This is load-bearing: the relay routes to its coordinate +/// soft-delete path *only* when the kind:5 has no `e` target ids +/// (`handle_standard_deletion_event` → `handle_a_tag_deletion`). An `e` tag +/// would route to the per-event path and leave the live replaceable row +/// intact — the note would survive the "deletion". Pure and unit-testable. +pub fn build_rm_event(coord: &nostr::nips::nip01::Coordinate) -> Result { + let a_tag = Tag::parse(&["a", &coord.to_string()]).map_err(tag_err)?; + Ok(EventBuilder::new(Kind::EventDeletion, "", vec![a_tag])) +} + +pub async fn cmd_rm(client: &SproutClient, slug: &str) -> Result<(), CliError> { + // Read-before-write: only the author can delete their own note, and we + // want a clear "nothing to delete" signal rather than emitting a kind:5 + // for a coordinate that was never published. + let me = client.keys().public_key(); + if fetch_own_note(client, slug).await?.is_none() { + return Err(CliError::NotFound(format!( + "no note {slug:?} found for you ({}); nothing to delete", + me.to_hex() + ))); + } + + let coord = coord_for(&me, slug); + let builder = build_rm_event(&coord)?; + let event = client.sign_event(builder)?; + let event_id = event.id; + let raw = client.submit_event(event).await?; + let parsed: serde_json::Value = serde_json::from_str(&raw) + .map_err(|e| CliError::Other(format!("relay response is not JSON: {e} ({raw})")))?; + let accepted = parsed + .get("accepted") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + let message = parsed.get("message").and_then(|v| v.as_str()).unwrap_or(""); + if !accepted { + return Err(CliError::Other(format!( + "relay rejected deletion: {message}" + ))); + } + + println!("deleted {KIND_LONG_FORM}:{}:{slug}", me.to_hex()); + println!("deletion {}", event_id.to_hex()); + Ok(()) +} + +pub async fn dispatch(cmd: crate::NotesCmd, client: &SproutClient) -> Result<(), CliError> { + use crate::NotesCmd; + match cmd { + NotesCmd::Set { + name, + title, + summary, + tags, + clear_tags, + content, + allow_empty, + } => { + let slug = parse_slug(&name)?; + // Map (Vec, --clear-tags) → Option<&[String]>: + // --clear-tags → Some(&[]) (explicit clear) + // --tag a --tag b → Some(&["a","b"]) (replace) + // neither → None (carry on update, empty on create) + // `--clear-tags` + any `--tag` is contradictory; reject loudly. + if clear_tags && !tags.is_empty() { + return Err(CliError::Usage( + "--clear-tags is mutually exclusive with --tag; pick one".into(), + )); + } + let tags_arg: Option<&[String]> = if clear_tags { + Some(&[]) + } else if tags.is_empty() { + None + } else { + Some(&tags) + }; + cmd_set( + client, + &slug, + title.as_deref(), + summary.as_deref(), + tags_arg, + &content, + allow_empty, + ) + .await + } + NotesCmd::Get { + naddr, + name, + author, + content_only, + } => { + if naddr.is_some() == name.is_some() { + return Err(CliError::Usage( + "exactly one of --naddr or --name is required".into(), + )); + } + if naddr.is_some() && author.is_some() { + return Err(CliError::Usage( + "--author only applies with --name; --naddr already identifies the author" + .into(), + )); + } + cmd_get( + client, + naddr.as_deref(), + name.as_deref(), + author.as_deref(), + content_only, + ) + .await + } + NotesCmd::Ls { author, tag, limit } => { + cmd_ls(client, author.as_deref(), tag.as_deref(), limit).await + } + NotesCmd::Rm { name } => { + let slug = parse_slug(&name)?; + cmd_rm(client, &slug).await + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_slug_accepts_dco_recipe() { + assert_eq!(parse_slug("dco-recipe").unwrap(), "dco-recipe"); + } + + #[test] + fn parse_slug_accepts_dots_and_underscores() { + assert!(parse_slug("v1.2_notes").is_ok()); + } + + #[test] + fn parse_slug_rejects_empty() { + assert!(matches!(parse_slug(""), Err(CliError::Usage(_)))); + } + + #[test] + fn parse_slug_rejects_uppercase() { + let err = parse_slug("DCO-Recipe").unwrap_err(); + assert!(matches!(err, CliError::Usage(msg) if msg.contains("invalid character"))); + } + + #[test] + fn parse_slug_rejects_spaces() { + assert!(matches!(parse_slug("dco recipe"), Err(CliError::Usage(_)))); + } + + #[test] + fn parse_slug_rejects_overlong() { + let s = "a".repeat(SLUG_MAX_LEN + 1); + assert!(matches!(parse_slug(&s), Err(CliError::Usage(_)))); + } + + #[test] + fn parse_naddr_rejects_wrong_kind() { + // kind:1 with a fake pubkey + slug — well-formed KPI, wrong kind. + let pk = "0000000000000000000000000000000000000000000000000000000000000001"; + let err = parse_naddr(&format!("1:{pk}:hello")).unwrap_err(); + assert!(matches!(err, CliError::Usage(msg) if msg.contains("expected 30023"))); + } + + #[test] + fn parse_naddr_accepts_kpi_form() { + let pk = "0000000000000000000000000000000000000000000000000000000000000001"; + let c = parse_naddr(&format!("30023:{pk}:my-note")).expect("parse"); + assert_eq!(c.identifier, "my-note"); + assert_eq!(c.kind.as_u16(), 30023); + } + + // -- NoteSnapshot -- + + use nostr::{EventBuilder, Keys, Tag, Timestamp}; + + fn build_30023( + keys: &Keys, + ts: u64, + slug: &str, + title: &str, + extra: Vec, + content: &str, + ) -> Event { + let mut tags = vec![ + Tag::parse(&["d", slug]).unwrap(), + Tag::parse(&["title", title]).unwrap(), + ]; + tags.extend(extra); + EventBuilder::new(Kind::Custom(KIND_LONG_FORM), content, tags) + .custom_created_at(Timestamp::from(ts)) + .sign_with_keys(keys) + .unwrap() + } + + #[test] + fn note_snapshot_parses_all_standard_tags() { + let keys = Keys::generate(); + let event = build_30023( + &keys, + 2_000, + "my-slug", + "My Title", + vec![ + Tag::parse(&["summary", "a short summary"]).unwrap(), + Tag::parse(&["t", "rust"]).unwrap(), + Tag::parse(&["t", "cli"]).unwrap(), + Tag::parse(&["published_at", "1700000000"]).unwrap(), + ], + "# body", + ); + let snap = NoteSnapshot::from_event(&event).expect("parse"); + assert_eq!(snap.slug, "my-slug"); + assert_eq!(snap.title, "My Title"); + assert_eq!(snap.summary.as_deref(), Some("a short summary")); + assert_eq!(snap.tags, vec!["rust".to_string(), "cli".to_string()]); + assert_eq!(snap.published_at, Some(1_700_000_000)); + assert_eq!(snap.updated_at, 2_000); + assert_eq!(snap.content, "# body"); + assert_eq!(snap.id, event.id); + assert_eq!(snap.pubkey, event.pubkey); + } + + #[test] + fn note_snapshot_missing_d_tag_is_err() { + let keys = Keys::generate(); + // Synthesize an event without a `d` tag — has to be done with EventBuilder + // directly since `build_30023` always inserts one. + let event = EventBuilder::new( + Kind::Custom(KIND_LONG_FORM), + "body", + vec![Tag::parse(&["title", "no-d"]).unwrap()], + ) + .sign_with_keys(&keys) + .unwrap(); + let err = NoteSnapshot::from_event(&event).unwrap_err(); + assert!(matches!(err, CliError::Other(m) if m.contains("missing the required `d` tag"))); + } + + #[test] + fn note_snapshot_rejects_wrong_kind() { + let keys = Keys::generate(); + let event = EventBuilder::new( + Kind::TextNote, + "hi", + vec![Tag::parse(&["d", "ignored"]).unwrap()], + ) + .sign_with_keys(&keys) + .unwrap(); + let err = NoteSnapshot::from_event(&event).unwrap_err(); + assert!(matches!(err, CliError::Other(m) if m.contains("expected kind:30023"))); + } + + #[test] + fn note_snapshot_garbage_published_at_yields_none() { + // We don't want a malformed `published_at` to fail the entire parse; + // it should fall back to `None` so the carry-forward logic treats it + // as "no prior value" and sets a fresh one. + let keys = Keys::generate(); + let event = build_30023( + &keys, + 1_000, + "x", + "T", + vec![Tag::parse(&["published_at", "not-a-number"]).unwrap()], + "", + ); + let snap = NoteSnapshot::from_event(&event).unwrap(); + assert_eq!(snap.published_at, None); + } + + #[test] + fn format_note_candidates_sorts_newest_first() { + let keys_a = Keys::generate(); + let keys_b = Keys::generate(); + let older = + NoteSnapshot::from_event(&build_30023(&keys_a, 1_000, "shared", "older", vec![], "")) + .unwrap(); + let newer = + NoteSnapshot::from_event(&build_30023(&keys_b, 2_000, "shared", "newer", vec![], "")) + .unwrap(); + let out = format_note_candidates(&[older, newer]); + let lines: Vec<&str> = out.lines().collect(); + assert_eq!(lines.len(), 2); + assert!(lines[0].contains("newer")); + assert!(lines[1].contains("older")); + } + + #[test] + fn format_note_candidates_uses_untitled_for_empty_title() { + let keys = Keys::generate(); + let snap = + NoteSnapshot::from_event(&build_30023(&keys, 1_000, "x", "", vec![], "")).unwrap(); + let out = format_note_candidates(&[snap]); + assert!(out.contains("(untitled)")); + } + + // -- build_set_event -- + // + // We can't introspect an unsigned `EventBuilder` for its tags, so each + // test signs with a throwaway key and inspects the resulting `Event`. + // The signature is irrelevant — we're asserting the tag set and timing. + + /// Sign `build_set_event` output with a fresh key and return the event + /// for tag inspection. `now` and signing key are local to the test; + /// nothing about the result depends on the wall clock. + fn build_and_sign( + prior: Option<&NoteSnapshot>, + slug: &str, + title: Option<&str>, + summary: Option<&str>, + tags: Option<&[String]>, + content: &str, + now: u64, + ) -> Result { + let builder = build_set_event(prior, slug, title, summary, tags, content, now)?; + let keys = Keys::generate(); + Ok(builder.sign_with_keys(&keys).unwrap()) + } + + fn tag_value<'a>(event: &'a Event, name: &str) -> Option<&'a str> { + event + .tags + .iter() + .find(|t| t.as_slice().first().map(String::as_str) == Some(name)) + .and_then(|t| t.as_slice().get(1).map(String::as_str)) + } + + fn t_tags(event: &Event) -> Vec<&str> { + event + .tags + .iter() + .filter(|t| t.as_slice().first().map(String::as_str) == Some("t")) + .filter_map(|t| t.as_slice().get(1).map(String::as_str)) + .collect() + } + + fn prior_snapshot( + ts: u64, + slug: &str, + title: &str, + summary: Option<&str>, + tags: &[&str], + published_at: Option, + content: &str, + ) -> NoteSnapshot { + let keys = Keys::generate(); + let mut extra: Vec = Vec::new(); + if let Some(s) = summary { + extra.push(Tag::parse(&["summary", s]).unwrap()); + } + for t in tags { + extra.push(Tag::parse(&["t", t]).unwrap()); + } + if let Some(p) = published_at { + extra.push(Tag::parse(&["published_at", &p.to_string()]).unwrap()); + } + NoteSnapshot::from_event(&build_30023(&keys, ts, slug, title, extra, content)).unwrap() + } + + #[test] + fn set_first_publish_requires_title() { + // No prior, no --title → usage error (NIP-23 mandates `title`). + let err = build_set_event(None, "x", None, None, None, "body", 1_000).unwrap_err(); + assert!(matches!(err, CliError::Usage(m) if m.contains("title is required"))); + } + + #[test] + fn set_first_publish_sets_published_at_to_now() { + let event = + build_and_sign(None, "x", Some("Hello"), None, None, "body", 1_700_000_000).unwrap(); + assert_eq!(tag_value(&event, "title"), Some("Hello")); + assert_eq!(tag_value(&event, "d"), Some("x")); + assert_eq!(tag_value(&event, "published_at"), Some("1700000000")); + assert_eq!(event.created_at.as_u64(), 1_700_000_000); + // No `summary` tag should be present when none was specified. + assert!(tag_value(&event, "summary").is_none()); + assert!(t_tags(&event).is_empty()); + } + + #[test] + fn set_update_preserves_published_at_and_carries_title() { + let prior = prior_snapshot( + 1_700_000_000, + "x", + "Original Title", + None, + &[], + Some(1_650_000_000), + "old body", + ); + // Omit --title; `now` advances. + let event = build_and_sign( + Some(&prior), + "x", + None, + None, + None, + "new body", + 1_700_001_000, + ) + .unwrap(); + assert_eq!(tag_value(&event, "title"), Some("Original Title")); + assert_eq!(tag_value(&event, "published_at"), Some("1650000000")); + assert_eq!(event.created_at.as_u64(), 1_700_001_000); + assert_eq!(event.content, "new body"); + } + + #[test] + fn set_update_clears_title_when_explicit_empty() { + let prior = prior_snapshot( + 1_700_000_000, + "x", + "Old", + None, + &[], + Some(1_650_000_000), + "", + ); + let event = build_and_sign( + Some(&prior), + "x", + Some(""), + None, + None, + "body", + 1_700_001_000, + ) + .unwrap(); + assert_eq!(tag_value(&event, "title"), Some("")); + } + + #[test] + fn set_update_carries_tags_when_omitted() { + let prior = prior_snapshot( + 1_700_000_000, + "x", + "T", + None, + &["rust", "cli"], + Some(1_650_000_000), + "", + ); + // None = omit; expect prior tags carried through. + let event = + build_and_sign(Some(&prior), "x", None, None, None, "body", 1_700_001_000).unwrap(); + let mut got = t_tags(&event); + got.sort(); + assert_eq!(got, vec!["cli", "rust"]); + } + + #[test] + fn set_update_clears_tags_when_explicit_empty_slice() { + let prior = prior_snapshot( + 1_700_000_000, + "x", + "T", + None, + &["rust", "cli"], + Some(1_650_000_000), + "", + ); + let event = build_and_sign( + Some(&prior), + "x", + None, + None, + Some(&[]), + "body", + 1_700_001_000, + ) + .unwrap(); + assert!(t_tags(&event).is_empty()); + } + + #[test] + fn set_update_replaces_tags_when_provided() { + let prior = prior_snapshot( + 1_700_000_000, + "x", + "T", + None, + &["old1", "old2"], + Some(1_650_000_000), + "", + ); + let new = ["fresh".to_string(), "tags".to_string()]; + let event = build_and_sign( + Some(&prior), + "x", + None, + None, + Some(&new), + "body", + 1_700_001_000, + ) + .unwrap(); + let mut got = t_tags(&event); + got.sort(); + assert_eq!(got, vec!["fresh", "tags"]); + } + + #[test] + fn set_update_carries_summary_when_omitted() { + let prior = prior_snapshot( + 1_700_000_000, + "x", + "T", + Some("old summary"), + &[], + Some(1_650_000_000), + "", + ); + let event = + build_and_sign(Some(&prior), "x", None, None, None, "body", 1_700_001_000).unwrap(); + assert_eq!(tag_value(&event, "summary"), Some("old summary")); + } + + #[test] + fn set_update_clears_summary_when_explicit_empty() { + let prior = prior_snapshot( + 1_700_000_000, + "x", + "T", + Some("old summary"), + &[], + Some(1_650_000_000), + "", + ); + let event = build_and_sign( + Some(&prior), + "x", + None, + Some(""), + None, + "body", + 1_700_001_000, + ) + .unwrap(); + // Explicit clear: an empty `summary` tag is emitted (vs no tag). + assert_eq!(tag_value(&event, "summary"), Some("")); + } + + // -- build_rm_event -- + // + // The relay only honours an addressable (a-tag) deletion when the kind:5 + // carries no `e` target ids. These tests pin both halves of that contract: + // exactly one `a` tag with the right coordinate, and zero `e` tags. + + #[test] + fn rm_event_is_kind5_with_a_tag_only() { + let keys = Keys::generate(); + let coord = coord_for(&keys.public_key(), "dco-recipe"); + let event = build_rm_event(&coord) + .unwrap() + .sign_with_keys(&keys) + .unwrap(); + + assert_eq!(event.kind, Kind::EventDeletion); + + let a_tags: Vec<&str> = event + .tags + .iter() + .filter(|t| t.as_slice().first().map(String::as_str) == Some("a")) + .filter_map(|t| t.as_slice().get(1).map(String::as_str)) + .collect(); + assert_eq!( + a_tags, + vec![format!( + "{KIND_LONG_FORM}:{}:dco-recipe", + keys.public_key().to_hex() + )] + ); + + // Load-bearing: an `e` tag would route the relay to the per-event + // delete path and leave the replaceable coordinate row alive. + assert!( + !event + .tags + .iter() + .any(|t| t.as_slice().first().map(String::as_str) == Some("e")), + "rm deletion must not carry an `e` tag" + ); + } + + #[test] + fn set_first_publish_with_no_prior_published_at_uses_now_even_after_a_garbage_prior() { + // Edge case: an existing event whose `published_at` was malformed + // would round-trip to `published_at: None`. On the next `set`, we + // treat that as "no prior `published_at`" and stamp `now`. + let prior = prior_snapshot(1_000, "x", "T", None, &[], None, ""); + let event = build_and_sign(Some(&prior), "x", None, None, None, "body", 2_000).unwrap(); + assert_eq!(tag_value(&event, "published_at"), Some("2000")); + } +} diff --git a/crates/sprout-cli/src/lib.rs b/crates/sprout-cli/src/lib.rs index fa5ab2f88..b33906413 100644 --- a/crates/sprout-cli/src/lib.rs +++ b/crates/sprout-cli/src/lib.rs @@ -178,6 +178,9 @@ enum Cmd { /// Publish notes and manage the social graph (NIP-01/02) #[command(subcommand)] Social(SocialCmd), + /// Publish and edit long-form NIP-23 notes — team knowledge base + #[command(subcommand)] + Notes(NotesCmd), /// Announce and discover git repositories (NIP-34) #[command(subcommand)] Repos(ReposCmd), @@ -774,6 +777,85 @@ pub enum SocialCmd { }, } +// --------------------------------------------------------------------------- +// Notes subcommands (NIP-23 long-form) +// --------------------------------------------------------------------------- + +#[derive(Subcommand)] +pub enum NotesCmd { + /// Create or update a note. Idempotent upsert keyed by `(me, --name)`. + /// + /// `published_at` is preserved on edits (only set on first create). + /// `--title` is required on first create; on subsequent edits the existing + /// title is carried forward when `--title` is omitted, and `--title ""` + /// explicitly clears it. + #[command( + after_help = "Examples:\n echo '# Hello' | sprout notes set --name hello --title 'Hello' --content -\n sprout notes set --name hello --tag onboarding --content - < draft.md" + )] + Set { + /// Slug — becomes the `d` tag. `[a-z0-9._-]{1,80}`. + #[arg(long)] + name: String, + /// Note title (NIP-23 `title` tag). Required on first create; omit to carry; `""` to clear. + #[arg(long)] + title: Option, + /// Short summary (NIP-23 `summary` tag). Omit to carry; `""` to clear. + #[arg(long)] + summary: Option, + /// Topic tag (NIP-23 `t` tag). May be repeated. Replaces (not merges) existing tags on edit; omit to carry forward. + #[arg(long = "tag")] + tags: Vec, + /// Clear all `t` tags on update. Mutually exclusive with `--tag`. + /// Without this and without `--tag`, existing tags are carried forward. + #[arg(long, default_value_t = false)] + clear_tags: bool, + /// Markdown body. Use `-` to read from stdin. + #[arg(long)] + content: String, + /// Allow committing an empty body (refused by default to catch upstream pipeline failures). + #[arg(long, default_value_t = false)] + allow_empty: bool, + }, + /// Read a note by `--naddr` (exact) or `--name ` (cross-author lookup). + Get { + /// NIP-19 `naddr1…` or `30023::` coordinate. Mutually exclusive with `--name`. + #[arg(long)] + naddr: Option, + /// Slug to look up across authors. Mutually exclusive with `--naddr`. + #[arg(long)] + name: Option, + /// Disambiguate `--name` to a specific author (hex pubkey, display name, or `me`). + #[arg(long)] + author: Option, + /// Print only the markdown body, not the full event JSON. + #[arg(long, default_value_t = false)] + content_only: bool, + }, + /// List notes. Defaults to your own. + Ls { + /// Hex pubkey, display name, `me`, or `all`. + #[arg(long, default_value = "me")] + author: Option, + /// Filter by NIP-23 `t` tag. + #[arg(long)] + tag: Option, + /// Max results (default 50, hard cap 200). + #[arg(long)] + limit: Option, + }, + /// Delete one of your own notes via NIP-09 (kind:5). + /// + /// Emits an a-tag-only deletion targeting the addressable coordinate + /// `30023::` (no `e` tag — an `e` tag would route around the + /// relay's coordinate soft-delete and leave the note alive). Read-before- + /// write gives a clean NotFound when there's nothing to delete. + Rm { + /// Slug of the note to delete. Only your own notes can be removed. + #[arg(long)] + name: String, + }, +} + // --------------------------------------------------------------------------- // Repos subcommands // --------------------------------------------------------------------------- @@ -983,6 +1065,7 @@ async fn run(cli: Cli) -> Result<(), CliError> { Cmd::Workflows(sub) => commands::workflows::dispatch(sub, &client).await, Cmd::Feed(sub) => commands::feed::dispatch(sub, &client).await, Cmd::Social(sub) => commands::social::dispatch(sub, &client).await, + Cmd::Notes(sub) => commands::notes::dispatch(sub, &client).await, Cmd::Repos(sub) => commands::repos::dispatch(sub, &client).await, Cmd::Upload(sub) => commands::upload::dispatch(sub, &client).await, Cmd::Mem(sub) => commands::mem::dispatch(sub, &client).await, @@ -1014,6 +1097,7 @@ mod tests { "feed", "mem", "messages", + "notes", "pack", "reactions", "repos", diff --git a/crates/sprout-db/src/event.rs b/crates/sprout-db/src/event.rs index 043a32093..22b01f880 100644 --- a/crates/sprout-db/src/event.rs +++ b/crates/sprout-db/src/event.rs @@ -550,6 +550,36 @@ pub async fn soft_delete_event(pool: &PgPool, event_id: &[u8]) -> Result { Ok(result.rows_affected() > 0) } +/// Soft-delete the live row for an addressable coordinate +/// `(kind, pubkey, d_tag)` — the NIP-33 replacement key. +/// +/// Used by `handle_a_tag_deletion` to honour NIP-09 a-tag deletions for any +/// parameterized-replaceable kind. The WHERE clause mirrors +/// `replace_parameterized_event` so the coordinate semantics stay consistent: +/// `channel_id` is intentionally NOT in the key (NIP-33 replacement is global +/// per the spec — `channel_id` is stored for query scoping, not identity). +/// +/// Returns `Ok(true)` if a row was deleted, `Ok(false)` if no live row matched +/// (already deleted, or never existed). +pub async fn soft_delete_by_coordinate( + pool: &PgPool, + kind: i32, + pubkey: &[u8], + d_tag: &str, +) -> Result { + let result = sqlx::query( + "UPDATE events SET deleted_at = NOW() \ + WHERE kind = $1 AND pubkey = $2 AND d_tag = $3 AND deleted_at IS NULL", + ) + .bind(kind) + .bind(pubkey) + .bind(d_tag) + .execute(pool) + .await?; + + Ok(result.rows_affected() > 0) +} + /// Atomically soft-delete an event and decrement thread reply counters. /// /// Wraps the delete + counter update in a single transaction so a crash between diff --git a/crates/sprout-db/src/lib.rs b/crates/sprout-db/src/lib.rs index 867875112..6a0ffd385 100644 --- a/crates/sprout-db/src/lib.rs +++ b/crates/sprout-db/src/lib.rs @@ -267,6 +267,17 @@ impl Db { event::soft_delete_event(&self.pool, event_id).await } + /// Soft-delete the live row for an addressable coordinate `(kind, pubkey, d_tag)`. + /// Used by NIP-09 a-tag deletion for parameterized-replaceable kinds. + pub async fn soft_delete_by_coordinate( + &self, + kind: i32, + pubkey: &[u8], + d_tag: &str, + ) -> Result { + event::soft_delete_by_coordinate(&self.pool, kind, pubkey, d_tag).await + } + /// Atomically soft-delete an event and decrement thread reply counters. pub async fn soft_delete_event_and_update_thread( &self, diff --git a/crates/sprout-relay/src/handlers/side_effects.rs b/crates/sprout-relay/src/handlers/side_effects.rs index 6876da2ba..4419d2e50 100644 --- a/crates/sprout-relay/src/handlers/side_effects.rs +++ b/crates/sprout-relay/src/handlers/side_effects.rs @@ -7,9 +7,9 @@ use tracing::{info, warn}; use uuid::Uuid; use sprout_core::kind::{ - event_kind_u32, KIND_GIT_REPO_ANNOUNCEMENT, KIND_MEMBER_ADDED_NOTIFICATION, - KIND_MEMBER_REMOVED_NOTIFICATION, KIND_NIP29_GROUP_ADMINS, KIND_NIP29_GROUP_MEMBERS, - KIND_NIP29_GROUP_METADATA, KIND_NIP43_MEMBERSHIP_LIST, KIND_REACTION, + event_kind_u32, is_parameterized_replaceable, KIND_GIT_REPO_ANNOUNCEMENT, + KIND_MEMBER_ADDED_NOTIFICATION, KIND_MEMBER_REMOVED_NOTIFICATION, KIND_NIP29_GROUP_ADMINS, + KIND_NIP29_GROUP_MEMBERS, KIND_NIP29_GROUP_METADATA, KIND_NIP43_MEMBERSHIP_LIST, KIND_REACTION, }; use sprout_db::channel::MemberRole; @@ -104,7 +104,7 @@ pub async fn validate_standard_deletion_event( let actor_bytes = effective_message_author(event, &state.relay_keypair.public_key()); let target_ids = extract_target_event_ids(event); - if target_ids.is_empty() { + if !has_e_tag(event) { // a-tag deletion: verify author owns the addressable event let a_tag = event .tags @@ -1380,11 +1380,53 @@ async fn handle_a_tag_deletion(event: &Event, state: &Arc) -> anyhow:: } } } + // Generic NIP-33 (parameterized-replaceable) soft-delete by coordinate. + // + // Listed after the workflow branch so workflow's bespoke deletion + // (which doesn't soft-delete the `events` row by design — that's a + // separate concern) takes precedence. For every other addressable + // kind, including kind:30023 (NIP-23 long-form), we soft-delete the + // live row matching `(kind, pubkey, d_tag)` so REQs stop returning it. + // See https://github.com/block/sprout/issues/714. + k if is_parameterized_replaceable(k) => { + let pubkey_bytes = match hex::decode(pubkey_hex) { + Ok(b) => b, + Err(e) => { + return Err(anyhow::anyhow!( + "invalid pubkey hex in a-tag {pubkey_hex}: {e}" + )); + } + }; + // Safe cast: NIP-33 kinds are 30000–39999, well within i32. + let kind_i32 = k as i32; + let deleted = state + .db + .soft_delete_by_coordinate(kind_i32, &pubkey_bytes, d_tag) + .await + .map_err(|e| { + anyhow::anyhow!( + "failed to soft-delete by coordinate {kind_i32}:{pubkey_hex}:{d_tag}: {e}" + ) + })?; + if deleted { + tracing::info!( + kind = k, + d_tag = d_tag, + "NIP-09 a-tag deletion: soft-deleted addressable event by coordinate" + ); + } else { + tracing::debug!( + kind = k, + d_tag = d_tag, + "NIP-09 a-tag deletion: no live row matched coordinate" + ); + } + } _ => { tracing::debug!( kind = kind_num, d_tag = d_tag, - "NIP-09 a-tag deletion for unhandled kind — no side effect" + "NIP-09 a-tag deletion for non-NIP-33 kind — no side effect" ); } } @@ -1397,8 +1439,10 @@ async fn handle_standard_deletion_event( state: &Arc, ) -> anyhow::Result<()> { let target_ids = extract_target_event_ids(event); - if target_ids.is_empty() { - // NIP-09 a-tag deletion path for addressable events + if !has_e_tag(event) { + // NIP-09 a-tag deletion path for addressable events. Keyed on the + // absence of *any* e tag (not just valid e-ids): a malformed e + a must + // not route here and silently soft-delete the coordinate. return handle_a_tag_deletion(event, state).await; } @@ -1553,6 +1597,15 @@ fn effective_message_author(event: &Event, relay_pubkey: &nostr::PublicKey) -> V event.pubkey.serialize().to_vec() } +/// True if the event carries any `e` tag at all, regardless of whether its +/// value decodes to a valid 32-byte id. NIP-09 treats `e`/`a` as target +/// classes: a malformed `e` makes the deletion ambiguous, not addressable-only. +/// Routing keys on this rather than on decoded-target count so a malformed `e` +/// alongside an `a` never silently soft-deletes a coordinate. +fn has_e_tag(event: &Event) -> bool { + event.tags.iter().any(|t| t.kind().to_string() == "e") +} + fn extract_target_event_ids(event: &Event) -> Vec> { event .tags diff --git a/crates/sprout-test-client/tests/e2e_long_form.rs b/crates/sprout-test-client/tests/e2e_long_form.rs index ff2c4934b..7f4cd806f 100644 --- a/crates/sprout-test-client/tests/e2e_long_form.rs +++ b/crates/sprout-test-client/tests/e2e_long_form.rs @@ -317,3 +317,246 @@ async fn test_long_form_stale_write_rejected() { client.disconnect().await.expect("disconnect"); } + +/// NIP-09 a-tag deletion: a kind:5 deletion targeting the addressable +/// coordinate `30023::` causes the live event row for that +/// coordinate to be soft-deleted, so subsequent REQs no longer return it. +/// +/// Regression test for issue #714 — before the fix, +/// `handle_a_tag_deletion` only handled the workflow kind and silently +/// no-op'd for kind:30023. +#[tokio::test] +#[ignore] +async fn test_long_form_a_tag_deletion() { + let url = relay_url(); + let keys = Keys::generate(); + let mut client = SproutTestClient::connect(&url, &keys) + .await + .expect("connect"); + + // Publish a note. + let d_tag = format!("a-del-{}", uuid::Uuid::new_v4().simple()); + let note = build_long_form_event(&keys, &d_tag, "Doomed Article", "Body.", vec![]); + let note_id = note.id; + let ok = client.send_event(note).await.expect("send note"); + assert!(ok.accepted, "note should be accepted: {}", ok.message); + + // Sanity check it's queryable before deletion. + let sid_pre = sub_id("a-del-pre"); + let filter_pre = Filter::new() + .kind(Kind::Custom(KIND_LONG_FORM)) + .author(keys.public_key()) + .custom_tag(SingleLetterTag::lowercase(Alphabet::D), [d_tag.as_str()]); + client + .subscribe(&sid_pre, vec![filter_pre]) + .await + .expect("subscribe pre"); + let pre = client + .collect_until_eose(&sid_pre, Duration::from_secs(5)) + .await + .expect("collect pre"); + assert!( + pre.iter().any(|e| e.id == note_id), + "note should be queryable before deletion" + ); + + // Build the addressable coordinate and emit a kind:5 deletion targeting it. + let a_coord = format!( + "{}:{}:{}", + KIND_LONG_FORM, + keys.public_key().to_hex(), + d_tag + ); + let del = EventBuilder::new( + Kind::EventDeletion, + "", + vec![Tag::parse(&["a", &a_coord]).unwrap()], + ) + .sign_with_keys(&keys) + .unwrap(); + let ok_del = client.send_event(del).await.expect("send deletion"); + assert!( + ok_del.accepted, + "a-tag deletion should be accepted: {}", + ok_del.message + ); + + // Query — should now be empty. + let sid_post = sub_id("a-del-post"); + let filter_post = Filter::new() + .kind(Kind::Custom(KIND_LONG_FORM)) + .author(keys.public_key()) + .custom_tag(SingleLetterTag::lowercase(Alphabet::D), [d_tag.as_str()]); + client + .subscribe(&sid_post, vec![filter_post]) + .await + .expect("subscribe post"); + let post = client + .collect_until_eose(&sid_post, Duration::from_secs(5)) + .await + .expect("collect post"); + assert!( + post.is_empty(), + "a-tag deletion should remove the note from REQ results (got {} events)", + post.len() + ); + + client.disconnect().await.expect("disconnect"); +} + +/// A kind:5 carrying a malformed `e` tag alongside a valid `a` coordinate must +/// NOT be routed as an addressable deletion — a malformed `e` makes the +/// deletion ambiguous, not addressable-only. Regression guard for relay +/// routing keyed on "no e tags present" rather than "no valid e-ids decoded": +/// the note must survive. +#[tokio::test] +#[ignore] +async fn test_long_form_malformed_e_plus_a_does_not_delete() { + let url = relay_url(); + let keys = Keys::generate(); + let mut client = SproutTestClient::connect(&url, &keys) + .await + .expect("connect"); + + let d_tag = format!("mixed-del-{}", uuid::Uuid::new_v4().simple()); + let note = build_long_form_event(&keys, &d_tag, "Survivor", "Body.", vec![]); + let note_id = note.id; + let ok = client.send_event(note).await.expect("send note"); + assert!(ok.accepted, "note should be accepted: {}", ok.message); + + // kind:5 with a *malformed* e tag (not 64 hex chars) plus a valid a coord. + let a_coord = format!( + "{}:{}:{}", + KIND_LONG_FORM, + keys.public_key().to_hex(), + d_tag + ); + let del = EventBuilder::new( + Kind::EventDeletion, + "", + vec![ + Tag::parse(&["e", "not-a-valid-event-id"]).unwrap(), + Tag::parse(&["a", &a_coord]).unwrap(), + ], + ) + .sign_with_keys(&keys) + .unwrap(); + // Relay may accept-and-noop or reject; either is fine. The contract under + // test is that the coordinate is NOT soft-deleted. + let _ = client.send_event(del).await.expect("send mixed deletion"); + + let sid = sub_id("mixed-del-post"); + let filter = Filter::new() + .kind(Kind::Custom(KIND_LONG_FORM)) + .author(keys.public_key()) + .custom_tag(SingleLetterTag::lowercase(Alphabet::D), [d_tag.as_str()]); + client + .subscribe(&sid, vec![filter]) + .await + .expect("subscribe"); + let post = client + .collect_until_eose(&sid, Duration::from_secs(5)) + .await + .expect("collect"); + assert!( + post.iter().any(|e| e.id == note_id), + "malformed-e + a must NOT soft-delete the coordinate; note should survive" + ); + + client.disconnect().await.expect("disconnect"); +} + +/// `notes set` re-publish preserves the original `published_at` while letting +/// `created_at` advance. This is the contract that NIP-23 readers rely on to +/// tell "when the author first wrote this" from "when they last updated it", +/// and the carry-forward logic in `sprout-cli`'s `build_set_event` (unit-tested +/// there) only works if the relay round-trips the tag faithfully. +/// +/// The carry rule is duplicated inline here (rather than reaching into +/// `sprout-cli`) so this e2e crate stays free of CLI deps; the rule's +/// correctness is unit-tested in `commands::notes::tests`. +#[tokio::test] +#[ignore] +async fn test_long_form_set_twice_preserves_published_at() { + let url = relay_url(); + let keys = Keys::generate(); + let mut client = SproutTestClient::connect(&url, &keys) + .await + .expect("connect"); + + let d_tag = format!("preserve-pat-{}", uuid::Uuid::new_v4().simple()); + let original_published_at: u64 = 1_700_000_000; + + // First publish: stamp `published_at` = original_published_at. + let v1 = build_long_form_event( + &keys, + &d_tag, + "First", + "v1 body", + vec![Tag::parse(&["published_at", &original_published_at.to_string()]).unwrap()], + ); + let ok1 = client.send_event(v1).await.expect("send v1"); + assert!(ok1.accepted, "v1 should be accepted: {}", ok1.message); + + // Ensure created_at advances between writes. + tokio::time::sleep(Duration::from_secs(1)).await; + + // Re-publish carrying the original `published_at` forward — what + // `notes set` does on update when `--title` (or nothing) changes. + let v2 = EventBuilder::new( + Kind::Custom(KIND_LONG_FORM), + "v2 body", + vec![ + Tag::parse(&["d", &d_tag]).unwrap(), + Tag::parse(&["title", "First"]).unwrap(), + Tag::parse(&["published_at", &original_published_at.to_string()]).unwrap(), + ], + ) + .custom_created_at(Timestamp::now()) + .sign_with_keys(&keys) + .unwrap(); + let v2_id = v2.id; + let v2_created_at = v2.created_at.as_u64(); + let ok2 = client.send_event(v2).await.expect("send v2"); + assert!(ok2.accepted, "v2 should be accepted: {}", ok2.message); + + // Re-fetch: there should be exactly one live event for (kind, author, d-tag), + // and its `published_at` should still be the original — even though + // `created_at` advanced. + let sid = sub_id("preserve-pat"); + let filter = Filter::new() + .kind(Kind::Custom(KIND_LONG_FORM)) + .author(keys.public_key()) + .custom_tag(SingleLetterTag::lowercase(Alphabet::D), [d_tag.as_str()]); + client + .subscribe(&sid, vec![filter]) + .await + .expect("subscribe"); + + let events = client + .collect_until_eose(&sid, Duration::from_secs(5)) + .await + .expect("collect"); + + assert_eq!(events.len(), 1, "exactly one live event after re-publish"); + let live = &events[0]; + assert_eq!(live.id, v2_id, "surviving event is v2"); + assert_eq!( + live.created_at.as_u64(), + v2_created_at, + "created_at advanced to v2's timestamp" + ); + let pa = live + .tags + .iter() + .find(|t| t.as_slice().first().map(String::as_str) == Some("published_at")) + .and_then(|t| t.as_slice().get(1).cloned()) + .and_then(|v| v.parse::().ok()); + assert_eq!( + pa, + Some(original_published_at), + "published_at must be preserved across re-publish" + ); + + client.disconnect().await.expect("disconnect"); +}