diff --git a/crates/buzz-db/src/lib.rs b/crates/buzz-db/src/lib.rs index 5ef4122c716..81a241a8319 100644 --- a/crates/buzz-db/src/lib.rs +++ b/crates/buzz-db/src/lib.rs @@ -3164,7 +3164,8 @@ impl Db { } /// Delete a workflow only when it belongs to the provided owner. - /// Returns the deleted workflow's `channel_id`. + /// Returns the deleted workflow's `channel_id`, or `Ok(None)` when no + /// row matched (idempotent no-op). #[datastore_span(name = "delete_workflow_for_owner", system = "postgresql")] pub async fn delete_workflow_for_owner( &self, diff --git a/crates/buzz-db/src/workflow.rs b/crates/buzz-db/src/workflow.rs index e970e978aaf..a408a6f0944 100644 --- a/crates/buzz-db/src/workflow.rs +++ b/crates/buzz-db/src/workflow.rs @@ -768,7 +768,10 @@ pub async fn delete_workflow(pool: &PgPool, community_id: CommunityId, id: Uuid) /// workflow just by learning its UUID. /// /// Returns the deleted workflow's `channel_id` so the caller can invalidate -/// the per-channel trigger cache without a separate lookup. +/// the per-channel trigger cache without a separate lookup. Returns `Ok(None)` +/// when no row matched (already deleted, never existed, or owned by someone +/// else) — NIP-09 deletion is idempotent, so a repeat delete must be a no-op +/// rather than an error (issue #6986). pub async fn delete_workflow_for_owner( pool: &PgPool, community_id: CommunityId, @@ -787,7 +790,7 @@ pub async fn delete_workflow_for_owner( match row { Some(row) => Ok(row.try_get("channel_id")?), - None => Err(DbError::NotFound(format!("workflow {id}"))), + None => Ok(None), } } diff --git a/crates/buzz-relay/src/handlers/side_effects.rs b/crates/buzz-relay/src/handlers/side_effects.rs index d37e7b375ee..ef50b1cd1a3 100644 --- a/crates/buzz-relay/src/handlers/side_effects.rs +++ b/crates/buzz-relay/src/handlers/side_effects.rs @@ -2131,6 +2131,52 @@ async fn handle_leave_request( /// Handle NIP-09 deletion via `a` tag (addressable/parameterized-replaceable events). /// Parses "kind:pubkey:d-tag" and deletes the corresponding DB record. +/// Tombstone the kind:30620 workflow-definition event row for the coordinate +/// `(30620, pubkey, d_tag)` so REQ-based readers (`workflows list`/`get`, Buzz +/// Desktop) stop returning a deleted workflow — the same soft-delete the +/// generic NIP-33 branch performs for every other addressable kind. The +/// execution-side `workflows` row is removed separately; without this the two +/// stores disagree and deleted workflows keep appearing (issue #6986). +async fn soft_delete_workflow_def_event( + tenant: &TenantContext, + state: &Arc, + pubkey_hex: &str, + d_tag: &str, + deletion: &Event, +) -> anyhow::Result<()> { + let pubkey_bytes = hex::decode(pubkey_hex) + .map_err(|e| anyhow::anyhow!("invalid pubkey hex in a-tag {pubkey_hex}: {e}"))?; + // Safe cast: 30620 is well within i32. + let kind_i32 = buzz_core::kind::KIND_WORKFLOW_DEF as i32; + let deleted = state + .db + .soft_delete_by_coordinate( + tenant.community(), + kind_i32, + &pubkey_bytes, + d_tag, + deletion.created_at.as_secs() as i64, + ) + .await + .map_err(|e| { + anyhow::anyhow!( + "failed to soft-delete by coordinate {kind_i32}:{pubkey_hex}:{d_tag}: {e}" + ) + })?; + if deleted { + tracing::info!( + d_tag = d_tag, + "NIP-09 a-tag deletion: soft-deleted workflow definition event" + ); + } else { + tracing::debug!( + d_tag = d_tag, + "NIP-09 a-tag deletion: no live workflow definition event matched coordinate" + ); + } + Ok(()) +} + async fn handle_a_tag_deletion( tenant: &TenantContext, event: &Event, @@ -2160,7 +2206,10 @@ async fn handle_a_tag_deletion( tracing::debug!(d_tag, "NIP-09 deletion ignored for push lease"); } buzz_core::kind::KIND_WORKFLOW_DEF => { + // Bespoke deletion of the execution-side `workflows` row first. // Try UUID first (workflow_id); fall back to name-based lookup. + // A repeat delete finds no row — that's an idempotent no-op, not + // an error (issue #6986). if let Ok(wf_id) = uuid::Uuid::parse_str(d_tag) { let channel_id = state .db @@ -2171,10 +2220,15 @@ async fn handle_a_tag_deletion( state .workflow_engine .invalidate_channel_workflows(tenant.community(), channel_id); + tracing::info!(workflow_id = %wf_id, "Workflow deleted via NIP-09 a-tag (UUID)"); + } else { + tracing::debug!(workflow_id = %wf_id, "NIP-09 a-tag deletion: no workflows row matched (already deleted or not owned)"); } - tracing::info!(workflow_id = %wf_id, "Workflow deleted via NIP-09 a-tag (UUID)"); + soft_delete_workflow_def_event(tenant, state, pubkey_hex, d_tag, event).await?; } else { - // Name-based lookup + // Name-based lookup. The kind:30620 definition event's d-tag + // is the workflow UUID, so tombstone the coordinate resolved + // from the name — the raw name coordinate matches no event row. match state .db .find_workflow_by_owner_and_name(tenant.community(), &actor_bytes, d_tag) @@ -2194,6 +2248,14 @@ async fn handle_a_tag_deletion( .invalidate_channel_workflows(tenant.community(), channel_id); } tracing::info!(workflow_id = %wf.id, name = d_tag, "Workflow deleted via NIP-09 a-tag (name)"); + soft_delete_workflow_def_event( + tenant, + state, + pubkey_hex, + &wf.id.to_string(), + event, + ) + .await?; } Ok(None) => { tracing::warn!( @@ -2208,10 +2270,10 @@ async fn handle_a_tag_deletion( } // 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 + // Listed after the workflow branch, whose bespoke deletion also + // removes the execution-side `workflows` row before tombstoning the + // events row. 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) => { diff --git a/crates/buzz-test-client/tests/e2e_relay.rs b/crates/buzz-test-client/tests/e2e_relay.rs index b119d267740..caecd018449 100644 --- a/crates/buzz-test-client/tests/e2e_relay.rs +++ b/crates/buzz-test-client/tests/e2e_relay.rs @@ -2767,6 +2767,153 @@ async fn test_workflow_reply_in_thread_pushes_live_thread_summary() { ws.disconnect().await.expect("disconnect"); } +/// NIP-09 a-tag deletion of a workflow definition: a kind:5 targeting the +/// addressable coordinate `30620::` must tombstone the +/// kind:30620 event row so subsequent REQs (the path `buzz workflows +/// list`/`get` and Buzz Desktop read) no longer return it — matching what the +/// execution-side workflows table already does. A repeated deletion of the +/// same coordinate must stay accepted (idempotent no-op), and the definition +/// must remain absent. +/// +/// Regression test for issue #6986 — before the fix, the workflow branch of +/// `handle_a_tag_deletion` removed only the execution-side workflows row and +/// skipped the events-row soft-delete the generic NIP-33 branch performs, so +/// deleted workflows kept appearing in list/get and the desktop UI. +#[tokio::test] +#[ignore] +async fn test_workflow_a_tag_deletion_tombstones_definition() { + let url = relay_url(); + let http = relay_http_url(); + let keys = Keys::generate(); + let pubkey_hex = keys.public_key().to_hex(); + let channel = create_test_channel(&keys).await; + + // Publish a workflow definition (kind:30620, d = workflow UUID). + let workflow_id = Uuid::new_v4().to_string(); + let yaml = "name: doomed-workflow\n\ + description: issue 6986 probe\n\ + trigger:\n\ + \x20 on: message_posted\n\ + steps:\n\ + \x20 - id: step1\n\ + \x20 name: Reply\n\ + \x20 action: send_message\n\ + \x20 text: \"never fires\"\n" + .to_string(); + let def = EventBuilder::new(Kind::Custom(30620), yaml) + .tags([ + Tag::parse(["d", &workflow_id]).unwrap(), + Tag::parse(["h", channel.as_str()]).unwrap(), + Tag::parse(["name", "doomed-workflow"]).unwrap(), + ]) + .sign_with_keys(&keys) + .expect("sign workflow def"); + let http_client = reqwest::Client::new(); + let resp = http_client + .post(format!("{http}/events")) + .header("X-Pubkey", &pubkey_hex) + .header("Content-Type", "application/json") + .body(serde_json::to_string(&def).unwrap()) + .send() + .await + .expect("submit workflow def"); + let body: serde_json::Value = resp.json().await.expect("parse def response"); + assert!( + body["accepted"].as_bool().unwrap_or(false), + "workflow def not accepted: {body}" + ); + + // Sanity check: the definition is queryable before deletion, the same + // filter shape `buzz workflows get` uses (kind + d-tag). + let mut ws = BuzzTestClient::connect(&url, &keys).await.expect("connect"); + let sid_pre = sub_id("wf-del-pre"); + let filter_pre = Filter::new().kind(Kind::Custom(30620)).custom_tag( + SingleLetterTag::lowercase(Alphabet::D), + workflow_id.as_str(), + ); + ws.subscribe(&sid_pre, vec![filter_pre]) + .await + .expect("subscribe pre"); + let pre = ws + .collect_until_eose(&sid_pre, Duration::from_secs(5)) + .await + .expect("collect pre"); + assert!( + pre.iter().any(|e| e.id == def.id), + "workflow def should be queryable before deletion" + ); + + // kind:5 deletion targeting the addressable coordinate. + let a_coord = format!("30620:{}:{}", pubkey_hex, workflow_id); + let del = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::parse(["a", &a_coord]).unwrap()]) + .sign_with_keys(&keys) + .unwrap(); + let ok_del = ws.send_event(del).await.expect("send deletion"); + assert!( + ok_del.accepted, + "a-tag deletion should be accepted: {}", + ok_del.message + ); + + // The definition must no longer be returned by REQ. + let sid_post = sub_id("wf-del-post"); + let filter_post = Filter::new().kind(Kind::Custom(30620)).custom_tag( + SingleLetterTag::lowercase(Alphabet::D), + workflow_id.as_str(), + ); + ws.subscribe(&sid_post, vec![filter_post]) + .await + .expect("subscribe post"); + let post = ws + .collect_until_eose(&sid_post, Duration::from_secs(5)) + .await + .expect("collect post"); + assert!( + post.is_empty(), + "a-tag deletion should remove the workflow def from REQ results (got {} events)", + post.len() + ); + + // Repeat the deletion — must stay accepted (idempotent no-op), and the + // definition must remain absent. A distinct `created_at` gives the event + // a distinct id; otherwise the relay dedups it and the idempotent + // side-effect path is never exercised. + let del_again = EventBuilder::new(Kind::EventDeletion, "") + .tags(vec![Tag::parse(["a", &a_coord]).unwrap()]) + .custom_created_at(nostr::Timestamp::now() + 1) + .sign_with_keys(&keys) + .unwrap(); + let ok_again = ws + .send_event(del_again) + .await + .expect("send repeat deletion"); + assert!( + ok_again.accepted, + "repeated a-tag deletion should stay accepted: {}", + ok_again.message + ); + + let sid_final = sub_id("wf-del-final"); + let filter_final = Filter::new().kind(Kind::Custom(30620)).custom_tag( + SingleLetterTag::lowercase(Alphabet::D), + workflow_id.as_str(), + ); + ws.subscribe(&sid_final, vec![filter_final]) + .await + .expect("subscribe final"); + let final_events = ws + .collect_until_eose(&sid_final, Duration::from_secs(5)) + .await + .expect("collect final"); + assert!( + final_events.is_empty(), + "workflow def must remain absent after repeated deletion" + ); + + ws.disconnect().await.expect("disconnect"); +} + /// Read a member's authoritative role from the relay-signed kind:39002 member /// list. The relay's own view of membership, not the client's — a kind:9000 can /// be `accepted` (stored) while its membership side effect fails, so asserting