fix(workflow): stop deleted workflows from returning after a stale edit

[BradGroux]

Fixes #2390.

What users saw

A deleted workflow could return after someone edited a stale copy. A later delete removed the workflow row again, but the replacement definition could remain queryable on the relay.

What changed

Canonical workflow deletion now uses one database transaction that:

• acquires the coordinate-scoped PostgreSQL advisory lock used by kind:30620 replacement writes;
• keeps the workflow row when a live definition is newer than the signed deletion event;
• tombstones only definition versions at or before the deletion event's created_at, as required by NIP-09;
• commits the workflow-row decision and event tombstones together;
• returns every affected definition channel so relay query caches are invalidated even when the workflow row was already gone.

The timestamp cutoff prevents a replayed tombstone from deleting a newer replacement. The complete affected-channel result also closes an orphan-repair gap where the database could tombstone a definition while leaving a stale query result cached.

Safety and scope

The existing validator still proves signer authority. The database mutation independently scopes the row and definitions by community and owner.

This PR does not add a permanent resurrection ban, sweep legacy name aliases, or change Desktop refetch behavior. It complements rather than replaces the separate work on legacy/name no-op handling and effective agent-author attribution.

Verification

• cargo fmt --all -- --check
• cargo clippy -p buzz-db -p buzz-relay -p buzz-test-client --all-targets --all-features -- -D warnings
• cargo test -p buzz-db tests::coordinate_delete_spares_head_newer_than_the_deletion -- --ignored --nocapture: passed against PostgreSQL.
• The regression creates definitions before and after the deletion timestamp, proves the older definition is retired while the newer head and workflow survive, then proves a current tombstone removes both.

[BradGroux]

I rebased this branch onto current main and traced the orphan-repair path through the relay cache rather than assuming a successful database delete completed the job.

The core transaction remains relevant. The audit found one remaining defect: when the workflow row was already missing but live definitions still existed, the database tombstoned those definitions yet returned no row channel. The relay therefore had no cache key to invalidate, so a stale query result could survive a successful orphan cleanup.

Head ea5729ef8e8c69b01de80108c3cf4dc25c233aa8 returns the complete set of affected definition channels plus the workflow-row channel, deduplicates them, and invalidates each cache. The lifecycle regression now covers the orphan-definition-only case.

The affected package suites, strict Clippy, and Rust formatting were refreshed. The non-database tests passed. Nine unrelated relay media/admin tests could not obtain this host's local PostgreSQL pool, and a serial rerun hit the same boundary. The new ignored lifecycle regression compiled, but I am not presenting the unavailable database run as passing evidence. This remains intentionally separate from legacy alias policy, Desktop refetch behavior, and the other deletion/attribution PRs noted in the updated body.

[BradGroux]

Rebased this branch onto current block/buzz main at c55e421a0.

Head moved from 0c36797ad to 7345b8420. git range-diff shows the workflow tombstone patch is unchanged, and the branch now has a direct merge base with current main.

Verification:

• The full buzz-db suite passed: 94 tests passed; 153 Postgres-backed tests remained explicitly ignored.
• All 5 branch-specific relay side-effect tests passed.
• The workflow-deletion E2E target compiled and remained explicitly ignored because no relay service was running.
• A broad relay-library run was not fully green in this local environment: 9 Postgres-backed failures reproduced on an exact main baseline with no Postgres service available. The one additional global tracing-order failure from the first branch run passed in an isolated rerun with RUST_LOG unset.
• git diff --check, DCO trailer checks, and the direct merge-base audit passed.

GitHub checks are rerunning on the new head.

[BradGroux]

Rebased onto current block/buzz main at b1b283cd4 and rechecked the competing deletion work. #3392 explicitly identifies #2489 as the broader transaction and cache-safe fix and offers to close in its favor; #2669 addresses a separate client and owner deletion gap.

The current-main audit exposed a NIP-09 safety gap in the old branch: its workflow transaction tombstoned every live definition at the coordinate, even if the signed deletion event was older than a replacement. The refreshed transaction now applies the deletion event's created_at cutoff atomically, preserves the workflow row when a newer live definition exists, and has a regression for a tombstone timestamped between two versions.

Published head: 181d7b5f8ea6c9318ee070a1f09fcf784bef694d.

Verification passed for formatting, strict Clippy across the affected database, relay, and test-client packages, and the PostgreSQL-backed between-version tombstone regression.

[BradGroux]

Rebased onto block/buzz main at 65f7a100353b9a5302da2614f2d85edee1c136a2 and rechecked the deletion path against current relay behavior, issue #2390, and overlapping work.

The PR remains valid. Current main still deletes the workflow control-plane row and returns before tombstoning the live kind-30620 addressable definition, which allows the deleted workflow to reappear. #3392 explicitly identifies this PR as the broader transaction- and cache-safe fix, while #2669 addresses a separate client-side owner-coordinate gap. Neither supersedes this change.

The rebase was clean. The existing correction remains necessary: workflow-row deletion and definition tombstoning occur in one transaction under the same coordinate advisory lock used by replacement writes; the NIP-09 timestamp cutoff preserves newer replacements; wrong-owner deletion remains a no-op; orphaned definitions are still retired; and every affected channel cache is invalidated. No further production-path change was needed.

Exact-head verification on e4dc30f7f9916ed6ae33fe4592105627a1c21320:

• Full buzz-db coverage completed with 94 passing tests, 0 failures, and 155 database-gated tests explicitly ignored.
• buzz-test-client unit and non-ignored integration coverage passed, and its database/relay-gated targets compiled.
• All 5 focused relay side-effect tests passed.
• The broad relay library run completed with 829 passing tests, 37 ignored, and 10 failures. Nine are the existing unavailable-database failures: 7 direct Sqlx(PoolTimedOut) media failures and 2 admin 500-versus-404 assertions under the same database path. The tenth was an unrelated tracing-global-state assertion; it passed immediately when rerun in isolation.
• Rust formatting passed, and strict Clippy passed across buzz-db, buzz-relay, and buzz-test-client with all targets, all features, and warnings denied.
• git diff --check, conflict-marker scan, merge-base audit, and required DCO trailers passed.

The PostgreSQL-backed timestamp-order, replacement-serialization, cache-invalidation, and end-to-end deletion regressions compiled but could not be executed on this host because no Docker daemon, PostgreSQL service, or Redis service is available. I am not presenting those unavailable integration gates as new passing evidence; the current branch remains limited to the verified code and package results above.

[BradGroux]

Review and rebase summary

Reviewed the PR for accuracy against current block/buzz main at d42d60d64 and rebased the branch onto that commit.

What this PR does

Makes workflow deletion atomic: deleting a workflow now tombstones its live kind:30620 definition event in the same transaction as the workflow row delete. The delete_workflow_for_owner function is replaced by delete_workflow_and_definition_for_owner, which takes a PostgreSQL advisory lock on the NIP-33 coordinate (kind, owner_pubkey, d_tag) before either mutation. The deletion is timestamp-bounded: a stale client referencing an older event ID cannot leave a newer definition queryable. The operation is idempotent — an already-missing workflow or definition is a no-op.

Accuracy review

• The advisory lock (pg_advisory_xact_lock) on the replacement coordinate prevents an in-flight replacement event from committing after the workflow row is deleted. The lock key is the same one used by workflow definition ingest, so the two paths serialize correctly.
• The NOT EXISTS subquery in the workflow DELETE prevents deleting the workflow row when a newer live definition exists — this handles the case where a stale tombstone arrives after a recreation.
• The definition tombstone UPDATE scopes to created_at <= deletion_created_at, so a newer definition survives a stale tombstone. The test workflow_delete_tombstones_each_recreated_definition verifies this across first-delete, wrong-owner-denial, stale-tombstone, and current-tombstone scenarios.
• The WorkflowLifecycleDeleteResult return type carries workflow_deleted, affected_channel_ids, and definition_events_deleted so the caller can invalidate the per-channel trigger cache without a separate lookup.
• The event_replacement_lock_key function is now pub so the workflow deletion path can use it.
• No unwrap() or expect() in production paths. No new dependencies.

Rebase result

Head moved from e4dc30f7f to e62dcdbab. No conflicts. git range-diff shows the patch is unchanged.

CI

DCO passes. Semgrep OSS and zizmor were pending at the time of this comment.

[BradGroux]

Rebase and accuracy review (2026-08-07)

Rebased onto current main (2b873cf20). Clean fast-forward through the intermediate d42d60d64 base — 52 commits applied, no conflicts.

Branch state: ea5729ef8 (was e62dcdbab at the intermediate base) Mergeable: ✅ MERGEABLE · BLOCKED (branch protection — awaiting maintainer review)

Accuracy review

The PR makes workflow deletion atomic by tombstoning the workflow definition in the database instead of leaving a window where a stale edit can resurrect a deleted workflow. The buzz-db/src/workflow.rs module adds a deleted_at tombstone column and the command_executor.rs and side_effects.rs handlers check the tombstone before processing edits or triggers.

The e2e test (e2e_workflow_deletion.rs) covers the race condition: delete a workflow, then attempt a stale edit, and verify the workflow does not return. The test is properly marked as #[ignore] since it requires a running relay.

No unwrap()/expect() in production paths. No new dependencies. The 460-line addition to workflow.rs is well-structured with clear SQL migration and atomic delete/update operations.

[BradGroux]

This is the third recurring review + rebase pass for the open BradGroux PRs on block/buzz.

What this PR does

Fixes a bug where deleted workflows could return after a stale edit. The old delete_workflow_for_owner deleted only the workflow control-plane row but left the live kind:30620 definition event untouched. A stale client could then re-ingest the orphaned definition event and re-create the workflow.

The fix replaces delete_workflow_for_owner with delete_workflow_and_definition_for_owner, which atomically (in a single transaction):

1. Takes the same coordinate advisory lock (pg_advisory_xact_lock) as workflow definition ingest, preventing an in-flight replacement event from committing after the workflow row is deleted.
2. Deletes the workflow row only if no newer live definition event exists (prevents deleting a workflow that was updated after the deletion was issued).
3. Tombstones (soft-deletes via deleted_at = NOW()) all live kind:30620 definition events at or before the deletion timestamp.
4. Returns a WorkflowLifecycleDeleteResult with the workflow deletion status, affected channel IDs (for cache invalidation), and count of tombstoned definition events.

The event_replacement_lock_key function is made pub so the workflow deletion path can use the same lock as definition ingest.

Correctness

• The transaction-scoped advisory lock prevents the race between definition ingest and workflow deletion.
• The NOT EXISTS subquery checking for newer definition events is correct — it prevents deleting a workflow that was updated after the deletion's created_at.
• The tombstone query only affects events with deleted_at IS NULL AND created_at <= deletion_created_at, preserving newer definitions.
• The operation is idempotent: an already-missing workflow or definition is a no-op.
• The BTreeSet for affected channel IDs ensures deduplication across workflow and definition channel scopes.
• No new dependencies. No unwrap/expect in production paths.

Rebase result

Rebased from 2b873cf20 to current main 02f640bc4 (7 commits). Clean rebase, no conflicts.

• Old head SHA: 579efe61793f8790fe4cfc7f6685a9492ab639b2
• New head SHA: d1d09bc182dc5d25b930f22ab9394f014c10852c
• Mergeable: ✅ MERGEABLE
• CI: DCO ✅

No code changes were made — this was a review + rebase pass only.