feat(workflow): implement WF-08 approval gates

[afkehaya]

Summary

Implements WF-08: the missing bridge between the workflow executor's StepResult::Suspended and the existing approval grant/deny/resume infrastructure.

This PR ports proven approval-gate patterns from Interchange into Buzz's Nostr-native workflow engine as an open-source contribution. Interchange's workflow state-machine and inference gate layers already solved durable approval gating with battle-tested patterns — rather than reinventing from scratch, this adapts those concepts for relay-signed Nostr events and NIP-33 parameterized-replaceable semantics.

Relationship to existing work

• Issue WF-08: wire approval-gate persist/suspend/resume (design + implementation) #2376 — the WF-08 design/implementation issue. This PR is an independent implementation of that spec.
• PR feat(workflow): persist and resume approval gates (WF-08) #2377 — an existing open PR by @brocoppler implementing the same WF-08 gap. We overlap significantly on the core wiring (persist in finalize_run, ActionSink::emit_approval_requested, hash_approval_token promoted to pub). Our PR differs in: the race condition fix (status-before-record write order with rollback), definition-time approver spec validation in schema.rs, the resolve_channel helper extraction, and the Interchange-informed design decisions documented below. Happy to consolidate or defer to whichever approach maintainers prefer.
• Issue Workflow request_approval: the documented from: syntax is rejected at approval time, and the only syntax that works accepts any key in the community #2878 — bug report that the documented from: syntax is rejected at approval time. Our PR fixes this: schema.rs validate() now rejects unsupported specs at definition time, and all test fixtures use the accepted "any" format.
• Issue Approval gates: the grant binds a run and a step index, not the step #2830 — grant binds run + step index, not the step. Not addressed in this PR.
• PR feat(workflow): complete all 7 actions + depth-cap loop prevention + monitoring + query actions #3301 — broader workflow engine completion PR (all 7 actions). May overlap on the request_approval action.

No prior issue was opened for this specific contribution — none found that covers the Interchange pattern-porting angle specifically.

What it does

• Suspended trace entry — when the executor hits a request_approval step, it generates a CSPRNG token and returns Suspended with a redacted trace entry (the raw token never appears in traces)
• emit_approval_requested on ActionSink — new trait method + RelayActionSink impl that builds a kind:46010 Nostr event with d/h/p/buzz:workflow tags, persists it, and fans out to channel subscribers
• persist_approval_gate in finalize_run — replaces the WF-08 stub with the real pipeline: fetch workflow metadata, transition run to WaitingApproval, create approval record, emit kind:46010 (best-effort)
• Approver spec validation — schema.rs validate() now rejects unsupported from: formats at definition time (only "", "any", and 64-char hex pubkeys are accepted)
• Race condition fix — flipped write order in persist_approval_gate so status transitions before record creation, with rollback on failure
• resolve_channel helper — extracts ~45 lines of shared validation between send_message and emit_approval_requested
• 7 E2E tests — grant resumes, deny cancels, kind:46010 emission, no-approval regression, expired token rejection, wrong approver rejection, double-grant idempotency

Key design decisions from Interchange

• Durable-before-transmit ordering (from Interchange's packages/workflow/src/state-machine/) — persist the approval record and run status to Postgres before emitting the notification event. If event emission fails, the approval is still actionable via the token hash.
• Absolute-timeout design (from Interchange's packages/inference/src/gates.ts) — store a computed expires_at timestamp rather than running countdown timers. The grant handler checks expires_at < now with no background jobs needed.
• Status-before-record write order — transition the run to WaitingApproval before creating the approval record, so a fast grant sees the correct status. If record creation fails, a rollback marks the run as Failed.

Quality process

The implementation went through two critique loops (12 findings, all resolved), a three-reviewer simplification pass (reuse, quality, efficiency — 6 improvements applied), and a final fix round for the race condition, definition-time validation, and missing E2E coverage.

Test plan

• cargo test -p buzz-workflow — 156 tests pass
• cargo clippy -p buzz-workflow -p buzz-relay -p buzz-test-client -- -D warnings — clean
• cargo fmt --check — clean
• cargo check -p buzz-test-client --tests — 7 E2E tests compile
• E2E tests against live relay (cargo test --test e2e_workflow_approval -- --ignored)

[mfanafuthimhlanga]

Read through this against #3525's caveat list. The shape looks right to me, and two decisions stand out as better than the obvious alternative.

Re-loading run → workflow → definition inside persist_approval_gate, rather than widening ExecutionResult, keeps the executor ignorant of persistence at the cost of two queries on a step that is about to block on a human. That seems like the right trade. And the _ => "step {step_id} at index {step_index} is not a RequestApproval action" arm is what actually defuses #3525's step_index caveat — an off-by-one becomes a loud definition error instead of a silent second token. Worth keeping even if the surrounding code moves.

Disclosure before the substance: I am looking at building a governance layer on top of approval gates, so I have a stake in point 2 below. Flagging that up front — the finding stands on its own, but you should know where I am coming from.

1. I do not think the tests in this PR run in CI.

just test-unit, which is what the Unit Tests job runs, covers buzz-core, buzz-auth, buzz-voice, buzz-cli, buzz-db --lib, buzz-conformance, and buzz-push-gateway. I could not find buzz-workflow in any cargo test or nextest invocation under .github/workflows/ or scripts/run-tests.sh, and buzz-relay's lib tests appear to run only when named by one of the four -E filters in the Backend Integration job.

I checked rather than assumed: on a fork I added a test, watched CI go green, then inverted the assertion so it had to fail — still green. They compile, because cargo clippy --workspace --all-targets builds test targets, but nothing executes them.

If that is right, the schema.rs changes here would pass or fail silently. Adding cargo nextest run -p buzz-workflow --lib to just test-unit made them run (154 tests, all passing). -p buzz-relay --lib is not safe to add wholesale — four tests in api::admin and api::media need Postgres but are not #[ignore]d, so they hang ~30s each and then fail. Is the gap known, or worth a separate PR?

2. The from validation is a behavioral change worth calling out explicitly.

ActionDef::RequestApproval documents the field as "User mention or role (e.g. @release-manager)" (schema.rs:134), and two existing tests encode that: parse_workflow_with_all_actions uses @manager, parse_approval_gate_example uses @engineering-lead. This PR rewrites both fixtures to 'any' and adds validate_rejects_at_anyone_approver_spec, while the doc comment still promises mentions.

The new validator faithfully mirrors check_approver_spec, which already fails closed on role specs — so today the documented syntax parses, saves, and then can never be approved. That is #2878, and moving the failure from grant time to definition time is a real improvement.

I pinned both halves in CI to be sure of it rather than arguing from a read:

PASS buzz-workflow schema::tests::accepts_documented_mention_syntax_at_definition_time_see_issue_2878
PASS buzz-relay    handlers::command_executor::approver_spec_tests::rejects_documented_mention_syntax_see_issue_2878

My concern is that a plumbing PR ends up deciding, via a fixture edit, that mentions and roles are not coming to from:. Practically that leaves two options for anyone wanting a specific human to approve: hardcode a pubkey into every workflow definition, or use "any" and let anyone in the community approve.

So: is the empty / "any" / 64-hex allowlist intended as the final set, or is #2878 expected to widen it? If it is final, could the schema.rs:134 doc comment change alongside it? If it is meant to widen, keeping those two fixtures #[ignore]d with a pointer to #2878 would preserve the record better than rewriting them — and I would be glad to take the resolution work (from: "@handle" and from: "role:admin" against channel membership) as its own PR. workflow_sink.rs already has resolve_mention_pubkeys, so the primitive may largely exist.

3. #3525's double-approve caveat looks unaddressed.

This does not touch command_executor.rs, so the run.status != WaitingApproval guards at :1240 and :1296 stay check-then-act rather than an atomic WaitingApproval → Running. I think it is safe in practice — update_approval_by_stored_hash's if !updated serializes on the approval row, and a correct step_index means one run never has two live tokens — but that is three assumptions holding it up and none are written down. Worth a single-flight guard on run_id, or a comment recording why the row-level check suffices?

Two smaller things: parse_duration_secs(timeout_str).unwrap_or(86400) silently swallows a malformed timeout, which the new definition-time validation seems well placed to catch instead; and a workflow with no channel_id persists the gate but returns before emitting kind:46010, leaving the run suspended with nobody notified.

Happy to be wrong on any of these.