fix(workflow): atomically delete workflow definitions

[Wirenut33]

Summary

NIP-09 deletion of a workflow removed the executable workflows row but left
the live kind-30620 definition queryable. The workflow therefore still appeared
in workflows list/get, and a retry could observe inconsistent executable and
event state.

This change:

• tombstones the kind-30620 definition and deletes the owned workflow row in one
  PostgreSQL transaction
• respects the deletion event timestamp, preserving a newer replacement when a
  stale deletion arrives
• retains the existing pre-storage authorization rule that permits the event
  author or the owning human of an agent-authored workflow
• returns the deleted workflow's optional channel so the relay can invalidate
  the correct trigger cache, including global workflows with no channel

Related issue

N/A. Searches for workflow deletion definition found no related issue or PR.

The separate send_dm implementation is deliberately excluded because it is
already proposed in #2614 and discussed in #2628.

Testing

• cargo fmt --all -- --check
• cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings
• cargo test -p buzz-db --lib: 94 passed, 156 ignored
• Explicit PostgreSQL regressions, all passed:
  • deletion atomically tombstones the definition and removes the workflow row
  • a stale deletion preserves both the newer definition and workflow row
  • a global workflow is deleted when its definition is already absent

Manual verification after deployment:

1. Create a workflow and confirm its kind-30620 definition is returned by
   workflows list/get.
2. Publish its authorized kind-5 a-tag deletion.
3. Confirm the executable workflow and query-visible definition are both gone.
4. Publish a deletion older than a replacement definition and confirm the newer
   definition and executable workflow remain.

The repository-wide just ci run reached mobile tests after Rust lint/unit,
desktop, Tauri, web, and mobile analyze passed. It then hit an unrelated,
reproducible failure in
mobile/test/features/channels/channel_detail_page_test.dart (keeps follow mode off while a tall newest message stays visible); this PR changes no mobile
files. The relay library suite separately passed 835 tests with 38 ignored and
hit one unrelated, reproducible mesh-demo HTTP 504 in
api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo.

No deployment or live-relay change was performed.

Originating Buzz channel: ace93da2-be69-44d7-84b6-061038ca55ee (Teal Tools Main Improvements).

[Wirenut33]

The focused deletion change is ready for maintainer review. GitHub currently
reports the CI, Docker image, and Desktop Release Candidate workflow runs
as action_required, so they need approval from a repository maintainer before
the hosted checks can execute.

Local validation for the cumulative three-file diff passed:

• cargo fmt --all -- --check
• strict Clippy for buzz-db and buzz-relay
• the three PostgreSQL workflow-deletion regressions covering atomic deletion,
  stale replacement preservation, and a global workflow whose definition is
  already absent

The overlapping send_dm work was removed from this PR and its self-wake
evidence was contributed to #2628, where the existing proposal is tracked.

Could a maintainer approve the forked workflow runs and review this focused
deletion fix?

[aweiker]

Reviewed this against the other open PRs for #2879/#717 and against main. This is the most complete of the bunch — it's the only one that puts both writes in a single transaction and handles the stale-deletion ordering correctly. Notes below, grouped by severity, each with source evidence.

What's right (verified)

• Atomicity + reverse-ghost safety. Tombstone-then-conditional-projection-delete in one tx, with the Stale rollback when a newer live definition exists (newer_definition_exists check), correctly prevents the reverse inconsistency (definition tombstoned but executable row spared, or vice-versa). Several sibling PRs tombstone the event without guarding the projection delete with the same created_at bound — this one gets it right. The stale_workflow_delete_preserves_definition_and_record test pins it.
• The actor_bytes → target_owner_bytes (a-tag pubkey) switch is correct, not a weakening. I want to call this out explicitly because it looks like an auth change and reviewers may flinch at it. It's sound: validate_standard_deletion_event (side_effects.rs:225–255) already runs at ingest before the side effect (ingest.rs:2306–2310, rejects pre-storage) and requires the signer to be either the a-tag pubkey itself or its is_agent_owner. So scoping the projection DELETE to the a-tag owner is what lets an owning human actually delete their agent's workflow — the old actor_bytes predicate silently no-op'd that case (the root of fix(desktop): address workflow deletions at the workflow's actual owner #2669/[Bug] Workflow deletes from Desktop are addressed at the caller, not the workflow's author — agent-created workflows are undeletable #2671), because the row's owner_pubkey is the agent, not the signing human. Worth a one-line comment in the code and PR body so this doesn't get misread as loosening authorization.

Suggestions (non-blocking)

1. Add a 32-byte length guard on the decoded a-tag pubkey. hex::decode(pubkey_hex) (side_effects.rs, new line ~2094) accepts any even-length hex, so a malformed a-tag yields a short/long pubkey that's then used as a query bind. It'll simply match nothing here, so it's not a security issue, but an explicit if bytes.len() != 32 { return Err(...) } fails fast with a clear message instead of a silent no-op, and documents the coordinate contract. (The pre-existing generic branch below doesn't check either — but the new code is a good place to set the better precedent.)

2. Add a test for the projection-already-gone ghost-recovery case with a live event. workflow_delete_commits_global_row_when_definition_is_already_absent covers row-present / event-absent. The field-reported failure mode in buzz workflows delete removes the workflow but leaves its kind:30620 event live, so workflows list and workflows get still return it #2879 is the mirror: the projection row already deleted (by a prior half-successful delete) but the kind:30620 event still live — the literal ghost operators hit. Worth asserting that this path still tombstones the event (rather than erroring out) so the fix demonstrably recovers pre-existing ghosts, not just prevents new ones.

3. Minor: consider surfacing the outcome to the delete ack. Right now Stale and Deleted{definition_deleted:false} only differ in logs; the client still gets a bare accepted:true. Out of scope for this PR, but a follow-up could thread the outcome into the ack message so buzz workflows delete on a no-op/typo'd UUID doesn't look identical to a real delete. Flagging as a cross-reference, not a request.

For context, I independently reached a nearly identical design (transactional two-store delete with a NOT EXISTS(newer live head) guard on the projection DELETE instead of the explicit Stale-rollback — same correctness outcome), plus the 32-byte guard and the ghost-recovery test in (1)/(2). Happy to hand those over here rather than open a competing PR; this is the right base.