fix(relay): retire the kind:30620 event on workflow delete (#2879)

[bonpiedlaroute]

Fixes #2879.

Summary

buzz workflows delete removed the workflow row and invalidated the engine
cache, but left the kind:30620 definition event live. workflows list and
workflows get both read that event, so every inspection surface kept
reporting a workflow that workflows trigger rejected with
invalid: workflow not found. The only way to discover the deletion had
landed was to trigger it.

Cause

handle_a_tag_deletion's KIND_WORKFLOW_DEF arm returns after
delete_workflow_for_owner + invalidate_channel_workflows, so the generic
NIP-33 arm below it — the only caller of soft_delete_by_coordinate — is never
reached for kind:30620. Every other addressable kind, kind:30023 included, is
soft-deleted so REQs stop returning it.

Change

Extract the soft-delete block into soft_delete_addressable_coordinate and
call it from both arms. No new logic: the behaviour already existed, it was
just unreachable for kind:30620.

The comment above the generic arm stated the current behaviour was deliberate
("doesn't soft-delete the events row by design — that's a separate concern").
It is rewritten, since it would otherwise assert the opposite of what the code
does. My reading is that the concern it deferred is workflow row deletion
semantics, not the visibility of the definition event. Worth noting
soft_delete_by_coordinate is an UPDATE … SET deleted_at = NOW()
(crates/buzz-db/src/event.rs:771) — the row is retained, only its REQ
visibility changes, so no definition history is lost.

If the intent really was for the kind:30620 event to outlive the workflow, the
fix belongs in cmd_list_workflows / cmd_get_workflow instead, filtering out
definitions with no live workflow. Happy to switch to that shape.

Authorization

No new surface. Ownership of the a-tag coordinate is already enforced by
validate_standard_deletion_event
(crates/buzz-relay/src/handlers/side_effects.rs:234-253), which runs from
ingest.rs:1967 before the event is stored and before any side effect —
it requires target_pubkey == actor || is_agent_owner(...), otherwise
must be event author. The check is identical for both arms.

Testing

New e2e test test_workflow_delete_retires_definition_event, mirroring
test_long_form_a_tag_deletion which covers this path for kind:30023.

Verified it is a real regression guard:

• without the change: FAILED — workflow deletion should retire the definition event (got 1 events)
• with the change: ok

[bonpiedlaroute]

Flagging an overlap I should have caught before opening this: #2489 (open since
Jul 23, rebased today) fixes #2390, which is the same root cause reported four
days before #2879. It also adds a crates/buzz-test-client/tests/e2e_workflow_deletion.rs,
so the two branches would conflict on that path.

#2489 is the broader fix: one transaction under the kind:30620 coordinate
advisory lock, tombstoning every live definition for the coordinate rather than
the current one, and returning affected channels so query caches are
invalidated even when the workflow row was already gone.

This PR is the minimal version: reuse the existing soft_delete_by_coordinate
call from the generic NIP-33 arm by extracting it into a helper, so the
KIND_WORKFLOW_DEF arm stops short-circuiting past it. 2 files, +221/-32, no
schema or transaction changes.

Maintainers should take whichever fits the review budget — I'm happy to close
this in favour of #2489, and equally happy to keep it as the small-surface
option if #2489 needs more review time. Not looking to duplicate anyone's work.

[bonpiedlaroute]

Closing in favour of #4234.

Agreeing with @aweiker's review: the created_at-bounded guard on the projection
delete is the piece this branch is missing, and it's a real defect rather than a
nitpick. #3392 predates the deletion_created_at_secs parameter that
soft_delete_by_coordinate has since gained on main, so a replayed or
late-arriving deletion could erase a newer replacement head. #4234 gets that
right, and it puts both writes in one transaction.

Also worth recording that this root cause now has #2489, #3650, #4318 and #4882
proposing overlapping fixes, plus four open issues (#2390, #2879, #3087, #4580).
Seven overlapping PRs is a signal about review budget, not about the fix — I'd
rather withdraw mine than add to the pile.

One artefact #4234 doesn't carry, in case it's useful: a relay-level e2e test.
Its regressions are Postgres-level in buzz-db. This branch has
crates/buzz-test-client/tests/e2e_workflow_deletion.rs — publish a kind:30620
under a real channel, assert it is queryable, delete via the a-tag coordinate the
CLI actually emits, assert the REQ comes back empty. Verified as a genuine
regression guard rather than a tautology: it fails on main without the fix
(workflow deletion should retire the definition event (got 1 events)) and passes
with it. That exercises @aweiker's suggestion (2) from the client side rather than
the DB side.

Happy to port it onto #4234 if @Wirenut33 wants it — #2489 and #4882 also carry
e2e tests, so take whichever fits best.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

This feature is valid, but this PR is not safe to resurrect or merge for release.

Blockers

1. Workflow deletion remains a non-atomic, non-retryable post-storage side effect. The kind-5 request is committed first; projection deletion and the new kind-30620 soft-delete then run as separate writes. Any transient failure is only logged while ingest still returns success. Replaying the same signed request takes the duplicate fast path before side effects, so the missing lifecycle write is never retried. This can still strand query state and executable state on opposite sides of the deletion. The event tombstone and workflow projection must be committed as one revision-fenced transaction, or the command needs durable retry/reconciliation.

2. The path is incomplete for the already-authorized managed-agent owner case. validate_standard_deletion_event permits a human owner to delete their agent’s kind-30620 coordinate, but handle_a_tag_deletion passes the effective human actor to delete_workflow_for_owner rather than the coordinate owner from the a tag. The projection delete therefore fails after the request has already been accepted/stored. Use the validated coordinate owner and cover this case end to end.

3. The branch predates timestamp-fenced coordinate deletion. Current main requires deletion_created_at_secs in soft_delete_by_coordinate; this helper calls the old signature. Resolving the merge conflict mechanically would either fail to compile or omit the NIP-09 guarantee that a stale/replayed tombstone cannot erase a newer replacement. Pass the deletion event timestamp through and test old-delete/new-replacement ordering.

The new relay E2E is a useful regression test for the basic UUID happy path, but it does not cover atomic failure/replay recovery, stale deletion ordering, or human-owner deletion of an agent-authored workflow. The safer route is to supersede this closed branch with the atomic, revision-fenced implementation being developed in the newer workflow-deletion work, then retain this E2E scenario there.

[wesbillman]

Correction: this review was submitted before one delegated review lane returned. It should be treated as procedurally withdrawn pending the consolidated review. GitHub did not reflect the dismissal attempt on this closed PR.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

This feature is valid, but this PR is not safe to resurrect or merge for release.

Blockers

1. Workflow deletion remains a non-atomic, non-retryable post-storage side effect. The kind-5 request is committed first; projection deletion and the new kind-30620 soft-delete then run as separate writes. Any transient failure is only logged while ingest still returns success. Replaying the same signed request takes the duplicate fast path before side effects, so the missing lifecycle write is never retried. This can still strand query state and executable state on opposite sides of the deletion. The event tombstone and workflow projection must be committed as one revision-fenced transaction, or the command needs durable retry/reconciliation.

2. The path is incomplete for the already-authorized managed-agent owner case. validate_standard_deletion_event permits a human owner to delete their agent’s kind-30620 coordinate, but handle_a_tag_deletion passes the effective human actor to delete_workflow_for_owner rather than the coordinate owner from the a tag. The projection delete therefore fails after the request has already been accepted/stored. Use the validated coordinate owner and cover this case end to end.

3. The branch predates timestamp-fenced coordinate deletion. Current main requires deletion_created_at_secs in soft_delete_by_coordinate; this helper calls the old signature. Resolving the merge conflict mechanically would either fail to compile or omit the NIP-09 guarantee that a stale/replayed tombstone cannot erase a newer replacement. Pass the deletion event timestamp through and test old-delete/new-replacement ordering.

The new relay E2E is a useful regression test for the basic UUID happy path, but it does not cover atomic failure/replay recovery, stale deletion ordering, or human-owner deletion of an agent-authored workflow. The safer route is to supersede this closed branch with the atomic, revision-fenced implementation being developed in the newer workflow-deletion work, then retain this E2E scenario there.