feat(workflows): mutating actions on transition triggers + per-tenant cycle cap (PMS-467) #345

Merged

YousifShkara merged 1 commit from feat/PMS-467-workflow-mutating-transitions into main

2026-06-24 08:14:52 +02:00

YousifShkara commented

2026-06-24 07:09:19 +02:00

Owner

PMS-448 phase 1 + phase 2 left a half-built workflow surface: ticket.created ran mutating actions (assign / set_priority / add_tag / add_internal_note) in-transaction, while ticket.status_changed / ticket.priority_changed only wrote workflow_rule_runs audit rows and refused to mutate. The deferral was intentional because a status-mutating status_changed rule can re-trigger itself indefinitely; phase 3 needed a story for the loop.

This change promotes the transition triggers to mutating. An action that itself moves status_id or priority_id re-fires the matching trigger at depth + 1; the per-tenant workflows/rule_max_depth cap (default 3, clamped 1..=10) refuses to fire any rule whose call depth has reached the ceiling and instead writes a workflow_rule_runs row whose error quotes the cap so the operator's audit trail captures why the cascade stopped.

What landed:

Migration 072_workflow_rule_max_depth.sql documents the well-known settings key (category='workflows', key='rule_max_depth') and seeds the default for every existing tenant so the row is browseable from the SPA immediately after upgrade. The reader falls back to 3 when the row is absent for new tenants created after this migration.
validate_setting_value("workflows", "rule_max_depth") accepts integers in 1..=10. The lower bound (1) is "no cascade beyond the originating transition"; the upper bound (10) prevents a typo from letting a tenant DOS itself with a status-flipping rule.
settings::read_workflow_rule_max_depth(tx, tenant_id) reads the value off a raw PgConnection so the workflow executor can call it from inside its own transaction without nesting a begin_with_tenant (which would deadlock on the app.current_tenant GUC SET).
executor.rs splits the previous apply_actions into two flavours. apply_create_actions runs for ticket.created and does NOT cascade (the create is the originating event; the next mutation is the operator's). apply_transition_actions runs for ticket.status_changed / ticket.priority_changed, applies the non-cascading actions first so they are visible to nested rules, then handles set_priority_id / set_status_id last, UPDATEs the column, and re-fires the matching transition trigger via Box::pin(run_*_at_depth(.., depth + 1, max_depth)). The Box::pin is the standard escape hatch for direct async recursion (the future would otherwise be infinitely sized).
run_ticket_status_changed / run_ticket_priority_changed keep their public signature; both now read the per-tenant cap at the top of the call and delegate to the internal *_at_depth helpers. Depth 0 is the operator-originated transition; depth max_depth is the level at which matching rules are refused.
New set_status_id action mirrors the existing set_priority_id. On the create path it's applied without cascade (no status_changed trigger fires from a create); on the transition path it cascades into the status_changed trigger.

Integration tests at tests/workflow_rules_phase3.rs:

status_changed_mutating_rule_fires_note: a to_status_id rule with add_internal_note fires when the matching transition lands and the note row appears in ticket_notes.
status_changed_self_cascade_hits_depth_cap: two cross-firing rules (A->B and B->A) trigger the cascade, which walks depth 0, 1, 2 and refuses at depth 3 with error = 'cycle cap reached at depth 3'.
priority_changed_non_mutating_rule_fires_once: a tag-only rule on priority_changed fires exactly once with no depth-cap rows, confirming the non-cascading path is unaffected.

Phase 3 follow-ups that did NOT land here, deferred to their own tickets:

SPA rule-builder UI for the new set_status_id action and the workflows/rule_max_depth knob (no client work in scope on PMS-467).
Additional triggers (time_entry.created, invoice.paid, richer LIKE / range / NOT IN condition operators) tracked under their own backlog items.

#PMS-467

PMS-448 phase 1 + phase 2 left a half-built workflow surface: `ticket.created` ran mutating actions (assign / set_priority / add_tag / add_internal_note) in-transaction, while `ticket.status_changed` / `ticket.priority_changed` only wrote `workflow_rule_runs` audit rows and refused to mutate. The deferral was intentional because a status-mutating status_changed rule can re-trigger itself indefinitely; phase 3 needed a story for the loop. This change promotes the transition triggers to mutating. An action that itself moves `status_id` or `priority_id` re-fires the matching trigger at depth + 1; the per-tenant `workflows/rule_max_depth` cap (default 3, clamped 1..=10) refuses to fire any rule whose call depth has reached the ceiling and instead writes a `workflow_rule_runs` row whose `error` quotes the cap so the operator's audit trail captures why the cascade stopped. What landed: * Migration `072_workflow_rule_max_depth.sql` documents the well-known settings key (`category='workflows', key='rule_max_depth'`) and seeds the default for every existing tenant so the row is browseable from the SPA immediately after upgrade. The reader falls back to 3 when the row is absent for new tenants created after this migration. * `validate_setting_value("workflows", "rule_max_depth")` accepts integers in 1..=10. The lower bound (1) is "no cascade beyond the originating transition"; the upper bound (10) prevents a typo from letting a tenant DOS itself with a status-flipping rule. * `settings::read_workflow_rule_max_depth(tx, tenant_id)` reads the value off a raw `PgConnection` so the workflow executor can call it from inside its own transaction without nesting a `begin_with_tenant` (which would deadlock on the `app.current_tenant` GUC SET). * `executor.rs` splits the previous `apply_actions` into two flavours. `apply_create_actions` runs for `ticket.created` and does NOT cascade (the create is the originating event; the next mutation is the operator's). `apply_transition_actions` runs for `ticket.status_changed` / `ticket.priority_changed`, applies the non-cascading actions first so they are visible to nested rules, then handles `set_priority_id` / `set_status_id` last, UPDATEs the column, and re-fires the matching transition trigger via `Box::pin(run_*_at_depth(.., depth + 1, max_depth))`. The `Box::pin` is the standard escape hatch for direct async recursion (the future would otherwise be infinitely sized). * `run_ticket_status_changed` / `run_ticket_priority_changed` keep their public signature; both now read the per-tenant cap at the top of the call and delegate to the internal `*_at_depth` helpers. Depth 0 is the operator-originated transition; depth `max_depth` is the level at which matching rules are refused. * New `set_status_id` action mirrors the existing `set_priority_id`. On the create path it's applied without cascade (no status_changed trigger fires from a create); on the transition path it cascades into the status_changed trigger. Integration tests at `tests/workflow_rules_phase3.rs`: * `status_changed_mutating_rule_fires_note`: a `to_status_id` rule with `add_internal_note` fires when the matching transition lands and the note row appears in `ticket_notes`. * `status_changed_self_cascade_hits_depth_cap`: two cross-firing rules (A->B and B->A) trigger the cascade, which walks depth 0, 1, 2 and refuses at depth 3 with `error = 'cycle cap reached at depth 3'`. * `priority_changed_non_mutating_rule_fires_once`: a tag-only rule on `priority_changed` fires exactly once with no depth-cap rows, confirming the non-cascading path is unaffected. Phase 3 follow-ups that did NOT land here, deferred to their own tickets: * SPA rule-builder UI for the new `set_status_id` action and the `workflows/rule_max_depth` knob (no client work in scope on PMS-467). * Additional triggers (`time_entry.created`, `invoice.paid`, richer LIKE / range / NOT IN condition operators) tracked under their own backlog items. #PMS-467