fix(buzz-cli): stop aged projects becoming permanently unmodifiable

crates/buzz-cli/src/commands/projects.rs

@@ -4,8 +4,9 @@
 //!   1. Fetch the caller's own live head via `kinds:[30621] + authors:[self] + #d:[slug]`.
 //!   2. Mutate the tag set (strip `auth`, apply change).
 //!   3. Re-validate the full envelope through Layer A before submitting.
-//!   4. Set `created_at = head.created_at + 1` (never wall-clock) to avoid
-//!      overwriting a concurrently advancing head.
+//!   4. Set `created_at = max(now, head.created_at + 1)` to avoid overwriting
+//!      a concurrently advancing head without stranding the write outside the
+//!      relay's accepted timestamp window (see `next_timestamp`).
 //!
 //! Limitations recorded in this phase:
 //!   - Relay hints are read-preserved but not authored (`--repo` carries
@@ -121,12 +122,40 @@ async fn submit_project(client: &BuzzClient, builder: EventBuilder) -> Result<()
 // ── Build helpers ─────────────────────────────────────────────────────────────
 
 /// Advance the `created_at` counter off an observed head.
+///
+/// Returns `max(now, head.created_at + 1)`.
+///
+/// `head + 1` on its own preserves monotonicity — the write always lands
+/// strictly above the head it read, so it cannot silently erase a
+/// concurrently advancing writer. But it never tracks wall clock, so on a
+/// head older than the relay's accepted timestamp window it signs a stale
+/// `created_at` and the relay rejects the write with "event timestamp too far
+/// from server time". That failure is permanent rather than transient: the
+/// stored head only ages further, so every subsequent mutation — `update`,
+/// `add-repo`, `remove-repo` and `delete` alike — is rejected too, leaving the
+/// project impossible to modify or remove.
+///
+/// Taking the max keeps the monotonicity guarantee (the result is always
+/// greater than the head, including when the head sits in the future) while
+/// following wall clock in the ordinary case where wall clock is ahead.
+///
+/// Trade-off, deliberately accepted: `head + 1` also gave free lost-update
+/// detection. A writer working from a stale read would land at or below an
+/// intervening writer's timestamp and be refused as dominated. Tracking wall
+/// clock removes that backstop — a delayed writer now lands above the
+/// intervening write and silently replaces it. Closing that hole properly
+/// needs a compare-and-swap on the observed head, not a timestamp rule. Until
+/// then this is the better failure mode: the old behaviour made aged projects
+/// permanently unmodifiable and undeletable, which is a certainty, against a
+/// narrow concurrent-writer race, which is not.
 fn next_timestamp(head: &Event) -> Result<Timestamp, CliError> {
-    head.created_at
+    let after_head = head
+        .created_at
         .as_secs()
         .checked_add(1)
-        .map(Timestamp::from)
-        .ok_or_else(|| CliError::Other("project timestamp cannot be advanced".into()))
+        .ok_or_else(|| CliError::Other("project timestamp cannot be advanced".into()))?;
+
+    Ok(Timestamp::from(after_head.max(Timestamp::now().as_secs())))
 }
 
 /// Strip `auth` from a tag list and pass the resulting envelope through
@@ -491,7 +520,7 @@ pub async fn cmd_update(
 ///
 /// Head-based and verified:
 ///   1. Fetch own live head — `NotFound` if absent.
-///   2. Build tombstone at `head.created_at + 1`.
+///   2. Build tombstone at `max(now, head.created_at + 1)`.
 ///   3. Submit.
 ///   4. Re-query the coordinate; if a newer head survived → `Conflict`.
 pub async fn cmd_delete(client: &BuzzClient, slug: &str) -> Result<(), CliError> {
@@ -508,6 +537,9 @@ pub async fn cmd_delete(client: &BuzzClient, slug: &str) -> Result<(), CliError>
         .custom_created_at(next_ts);
 
     let event = client.sign_event(tombstone)?;
+    // Captured before submit: `submit_event` consumes the event, and the
+    // tombstone id is what an operator needs to audit the retraction.
+    let tombstone_id = event.id.to_hex();
     let raw = client.submit_event(event).await?;
     parse_write_response(&raw, "delete event was dominated; a newer head exists")?;
 
@@ -520,7 +552,14 @@ pub async fn cmd_delete(client: &BuzzClient, slug: &str) -> Result<(), CliError>
         )));
     }
 
-    println!("{}", serde_json::json!({ "deleted": slug, "status": "ok" }));
+    println!(
+        "{}",
+        serde_json::json!({
+            "deleted": slug,
+            "event_id": tombstone_id,
+            "status": "ok",
+        })
+    );
     Ok(())
 }
 
@@ -979,22 +1018,27 @@ mod tests {
 
     // ── next_timestamp ordering ───────────────────────────────────────────────
 
-    /// `next_timestamp` must return `head.created_at + 1` regardless of the wall
-    /// clock.  NIP-MP Deletion rule: a tombstone older than the live head does
-    /// NOT remove it, so we must advance strictly off the observed head — never
-    /// use wall-clock time, which could be behind a head that was bumped
-    /// multiple times in the same second.
-    #[test]
-    fn next_timestamp_returns_head_plus_one_when_head_is_ahead_of_wall_clock() {
-        // Build a minimal signed event with a created_at far in the future.
+    /// Build a signed kind:30621 head carrying an exact `created_at`.
+    fn signed_head_at(created_at: Timestamp) -> Event {
         let keys = nostr::Keys::generate();
-        let far_future_ts = Timestamp::from(9_999_999_999u64); // year 2286
         let tags = vec![
             make_test_tag(&["d", "platform"]),
             make_test_tag(&["a", &format!("30617:{OWNER_HEX}:buzz")]),
         ];
-        let builder = rebuild_project("", tags, far_future_ts).expect("valid head envelope");
-        let head = builder.sign_with_keys(&keys).expect("sign");
+        let builder = rebuild_project("", tags, created_at).expect("valid head envelope");
+        builder.sign_with_keys(&keys).expect("sign")
+    }
+
+    /// When the head is ahead of the wall clock, `next_timestamp` must return
+    /// `head.created_at + 1`.  NIP-MP Deletion rule: a tombstone older than the
+    /// live head does NOT remove it, so the result must advance strictly off the
+    /// observed head — wall-clock time alone could land behind a head that was
+    /// bumped multiple times in the same second.
+    #[test]
+    fn next_timestamp_returns_head_plus_one_when_head_is_ahead_of_wall_clock() {
+        // Build a minimal signed event with a created_at far in the future.
+        let far_future_ts = Timestamp::from(9_999_999_999u64); // year 2286
+        let head = signed_head_at(far_future_ts);
         // Verify the event actually has our future timestamp.
         assert_eq!(head.created_at, far_future_ts);
 
@@ -1007,6 +1051,58 @@ mod tests {
         );
     }
 
+    /// An aged head must NOT produce an aged timestamp.  `head + 1` alone would
+    /// sign a `created_at` an hour in the past here, which the relay rejects as
+    /// "event timestamp too far from server time" — permanently, because the
+    /// stored head only ages further.  The result must track the wall clock.
+    #[test]
+    fn next_timestamp_uses_wall_clock_when_head_is_aged() {
+        let now = Timestamp::now().as_secs();
+        let hour_ago = Timestamp::from(now - 3_600);
+        let head = signed_head_at(hour_ago);
+
+        let next = next_timestamp(&head).expect("no overflow").as_secs();
+
+        assert!(
+            next >= now,
+            "aged head must advance to wall clock, got {next} against now {now}"
+        );
+        assert!(
+            next > hour_ago.as_secs(),
+            "result must still be strictly after the observed head"
+        );
+    }
+
+    /// A head written moments ago is already inside the relay's window, so the
+    /// result must stay at wall clock rather than jumping ahead of it.
+    #[test]
+    fn next_timestamp_stays_at_wall_clock_for_a_current_head() {
+        let now = Timestamp::now().as_secs();
+        let head = signed_head_at(Timestamp::from(now - 1));
+
+        let next = next_timestamp(&head).expect("no overflow").as_secs();
+
+        assert!(
+            next >= now && next <= now + 1,
+            "current head must land at wall clock, got {next} against now {now}"
+        );
+    }
+
+    /// `u64::MAX` cannot be advanced — the checked add must surface an error
+    /// rather than wrapping to zero and signing a 1970 timestamp.
+    #[test]
+    fn next_timestamp_rejects_overflow_at_u64_max() {
+        let head = signed_head_at(Timestamp::from(u64::MAX));
+
+        let error = next_timestamp(&head).expect_err("u64::MAX must not advance");
+
+        assert!(
+            matches!(error, CliError::Other(ref message)
+                if message.contains("timestamp cannot be advanced")),
+            "expected a timestamp-advance error, got {error:?}"
+        );
+    }
+
     // ── empty update guard ────────────────────────────────────────────────────
 
     /// `cmd_update` with no setters or clearers must return `CliError::Usage`

crates/buzz-cli/src/commands/repos.rs

@@ -30,6 +30,22 @@ async fn fetch_own_repo_announcement(
     Ok(events.into_iter().next())
 }
 
+/// Advance the `created_at` counter off an observed announcement head.
+///
+/// Returns `max(now, head.created_at + 1)` — strictly after the head, so the
+/// write cannot be dominated by the announcement it read, but never stale
+/// enough for the relay to reject it as "event timestamp too far from server
+/// time" once the head has aged past the accepted window.
+fn next_announcement_timestamp(head: &Event) -> Result<Timestamp, CliError> {
+    let after_head = head
+        .created_at
+        .as_secs()
+        .checked_add(1)
+        .ok_or_else(|| CliError::Other("repository timestamp cannot be advanced".into()))?;
+
+    Ok(Timestamp::from(after_head.max(Timestamp::now().as_secs())))
+}
+
 fn repo_id_from_event(event: &Event) -> Result<&str, CliError> {
     event
         .tags
@@ -270,6 +286,53 @@ pub async fn cmd_create_repo(
     Ok(())
 }
 
+/// `buzz repos delete`
+///
+/// Head-based and verified, mirroring `buzz projects delete`:
+///   1. Fetch the caller's own live announcement head — `NotFound` if absent.
+///   2. Build a NIP-09 kind:5 addressable tombstone for
+///      `30617:<self>:<repo-id>` at `max(now, head.created_at + 1)`.
+///   3. Submit.
+///   4. Re-query the coordinate; if a head survived → `Conflict`.
+///
+/// Targets signer-self only: the filter is scoped to the caller's pubkey, so
+/// another author's announcement at the same `d` tag is never touched.
+pub async fn cmd_delete_repo(client: &BuzzClient, repo_id: &str) -> Result<(), CliError> {
+    let head = current_repo(client, repo_id).await?;
+    let next_ts = next_announcement_timestamp(&head)?;
+
+    let pubkey_hex = client.keys().public_key().to_hex();
+    let tombstone =
+        buzz_sdk::build_delete_addressable(KIND_GIT_REPO_ANNOUNCEMENT, &pubkey_hex, repo_id)
+            .map_err(|error| CliError::Other(format!("failed to build delete event: {error}")))?
+            .custom_created_at(next_ts);
+
+    let event = client.sign_event(tombstone)?;
+    // Captured before submit: `submit_event` consumes the event, and the
+    // tombstone id is what an operator needs to audit the retraction.
+    let tombstone_id = event.id.to_hex();
+    let raw = client.submit_event(event).await?;
+    parse_write_response(&raw, "delete event was dominated; a newer head exists")?;
+
+    // Post-submit verification: re-query to confirm the head is gone.
+    if let Some(survivor) = fetch_own_repo_announcement(client, repo_id).await? {
+        return Err(CliError::Conflict(format!(
+            "repository {repo_id:?} still exists (head at {}); a concurrent write raced the delete",
+            survivor.created_at.as_secs()
+        )));
+    }
+
+    println!(
+        "{}",
+        serde_json::json!({
+            "deleted": repo_id,
+            "event_id": tombstone_id,
+            "status": "ok",
+        })
+    );
+    Ok(())
+}
+
 pub async fn cmd_get_repo(
     client: &BuzzClient,
     repo_id: &str,
@@ -434,6 +497,7 @@ pub async fn dispatch(cmd: crate::ReposCmd, client: &BuzzClient) -> Result<(), C
         ReposCmd::Get { id, owner } => cmd_get_repo(client, &id, owner.as_deref()).await,
         ReposCmd::List { owner, limit } => cmd_list_repos(client, owner.as_deref(), limit).await,
         ReposCmd::Bind { id, channel } => cmd_bind_repo(client, &id, &channel).await,
+        ReposCmd::Delete { id } => cmd_delete_repo(client, &id).await,
         ReposCmd::Protect(command) => match command {
             ReposProtectCmd::List { id } => cmd_protect_list(client, &id).await,
             ReposProtectCmd::Set {
@@ -468,7 +532,7 @@ mod tests {
 
     use super::{
         build_create_announcement, build_protection_tag, build_updated_repo_announcement,
-        protection_rules_json, validate_write_response, RepoChange,
+        next_announcement_timestamp, protection_rules_json, validate_write_response, RepoChange,
     };
 
     fn signed_repo(tags: Vec<Tag>, content: &str, created_at: u64) -> nostr::Event {
@@ -844,4 +908,105 @@ mod tests {
             })
         );
     }
+
+    // ── delete tombstone timestamp ────────────────────────────────────────────
+
+    /// An aged announcement must not produce an aged tombstone. `head + 1`
+    /// alone would sign an hour in the past here, which the relay rejects as
+    /// "event timestamp too far from server time" — and permanently, since the
+    /// stored head only ages further.
+    #[test]
+    fn next_announcement_timestamp_uses_wall_clock_when_head_is_aged() {
+        let now = Timestamp::now().as_secs();
+        let head = signed_repo(vec![tag(&["d", "demo"])], "", now - 3_600);
+
+        let next = next_announcement_timestamp(&head)
+            .expect("no overflow")
+            .as_secs();
+
+        assert!(
+            next >= now,
+            "aged head must advance to wall clock, got {next} against now {now}"
+        );
+    }
+
+    /// A head already inside the relay's window must land at wall clock rather
+    /// than jumping past it.
+    #[test]
+    fn next_announcement_timestamp_stays_at_wall_clock_for_a_current_head() {
+        let now = Timestamp::now().as_secs();
+        let head = signed_repo(vec![tag(&["d", "demo"])], "", now - 1);
+
+        let next = next_announcement_timestamp(&head)
+            .expect("no overflow")
+            .as_secs();
+
+        assert!(
+            next >= now && next <= now + 1,
+            "current head must land at wall clock, got {next} against now {now}"
+        );
+    }
+
+    /// A head in the future must still be superseded strictly, so a tombstone
+    /// can never be dominated by the announcement it is retracting.
+    #[test]
+    fn next_announcement_timestamp_returns_head_plus_one_when_head_is_ahead() {
+        let far_future = 9_999_999_999u64; // year 2286
+        let head = signed_repo(vec![tag(&["d", "demo"])], "", far_future);
+
+        let next = next_announcement_timestamp(&head)
+            .expect("no overflow")
+            .as_secs();
+
+        assert_eq!(
+            next,
+            far_future + 1,
+            "tombstone must be strictly after a future head"
+        );
+    }
+
+    /// `u64::MAX` cannot be advanced — the checked add must surface an error
+    /// rather than wrapping to zero and signing a 1970 timestamp.
+    #[test]
+    fn next_announcement_timestamp_rejects_overflow_at_u64_max() {
+        let head = signed_repo(vec![tag(&["d", "demo"])], "", u64::MAX);
+
+        let error = next_announcement_timestamp(&head).expect_err("u64::MAX must not advance");
+
+        assert!(
+            matches!(error, crate::error::CliError::Other(ref message)
+                if message.contains("timestamp cannot be advanced")),
+            "expected a timestamp-advance error, got {error:?}"
+        );
+    }
+
+    /// The tombstone must be a NIP-09 kind:5 carrying exactly the addressable
+    /// coordinate `30617:<pubkey>:<repo-id>` — that coordinate is what the
+    /// relay matches to retract the announcement.
+    #[test]
+    fn delete_tombstone_targets_the_repo_announcement_coordinate() {
+        let pubkey = "a".repeat(64);
+        let tombstone = buzz_sdk::build_delete_addressable(
+            buzz_core::kind::KIND_GIT_REPO_ANNOUNCEMENT,
+            &pubkey,
+            "demo",
+        )
+        .expect("valid tombstone")
+        .custom_created_at(Timestamp::from(1_700_000_000u64))
+        .sign_with_keys(&Keys::generate())
+        .expect("sign tombstone");
+
+        assert_eq!(tombstone.kind, Kind::Custom(5));
+        let coords: Vec<&str> = tombstone
+            .tags
+            .iter()
+            .filter_map(|t| {
+                let values = t.as_slice();
+                (values.first().map(String::as_str) == Some("a"))
+                    .then(|| values.get(1).map(String::as_str))
+                    .flatten()
+            })
+            .collect();
+        assert_eq!(coords, vec![format!("30617:{pubkey}:demo").as_str()]);
+    }
 }