BaryoDev / BaryoDev/barakoCMS

An action cannot produce a value, so a workflow is a list of side effects and not a chain

Open
#575 2 comments 0 reactions 0 assignees View on GitHub
blocker core enhancement
Dominant language
C#
Stars
6
Forks
7
Avg merge
4h 42m
Merged PRs (30d)
307

Description

An action cannot produce a value. `IWorkflowAction.RunAsync` takes `Dictionary` parameters plus the triggering `Content`, and returns `WorkflowActionResult`, which holds `Succeeded`, `Error` and `Retryable` and nothing else (`WorkflowActionResult.cs:12-19`).

So there is no channel from one action to the next. A workflow is a list of independent side effects on the triggering content, not a chain.

## What this blocks

Take an order that has to send a confirmation, total the customer's loyalty points, credit them with a partner, post the journal entry, file a document, adjust stock, and reorder when stock is low.

The confirmation works. Everything after it fails on the same missing thing: step 2 cannot return the total, so step 3 has nothing to send, so step 4 cannot post the right amount, so step 7 cannot compare a count it never received.

Seven steps, one gap.

## What to add

`docs/workflow-engine-rethink.md` section 3 already names this as the one architectural move, and its shape still looks right:

```
RunAsync(WorkflowContext ctx, CancellationToken ct) -> WorkflowActionResult

WorkflowContext { Content, TriggerEvent, TenantId, Variables, Outputs }
WorkflowActionResult { Succeeded, Error, Retryable, Outputs }
```

Persist `Outputs` on the attempt, and let a later action's parameters resolve `{{actions.2.total}}`.

That doc predates #329, so its Tier 1 is already built: idempotency keys, per-attempt records, retries and a dead letter all exist now. This is the piece that did not land.

## The part that needs deciding before any code

**Outputs are third-party response data, stored on a run record that is served over the API.** They will contain personal data and sometimes credentials. `WorkflowActionResult.Error` already carries a warning that it must not hold either, and that warning is much harder to honour when the payload is whatever a provider returned.

Options, roughly in order of how much they cost:

1. An allowlist per action: the definition names which paths it captures, everything else is discarded. Matches how `QueryDefinition.Fields` already works, and is the one consistent with how this codebase treats egress.
2. Capture everything, redact on read. Cheaper to write, and it stores the data anyway, so a database leak still has it.
3. Capture everything, expire quickly. Retention already exists via `WorkflowRunRetentionService`.

I would take option 1. It is the same shape as the rest of the system: name what may move, refuse the rest.

## Done when

- An action returns outputs and a later action reads them.
- A capture allowlist exists, and a path not named is not stored.
- A test proves an unnamed field in a provider response never reaches the run record.

This is a contract change to `IWorkflowAction`, so it is a one-way door.

Contributor guide

Open the contributing guide

Research direction

Read docs/workflow-engine-rethink.md section 3 and WorkflowActionResult.cs:12-19, then trace IWorkflowAction.RunAsync and the existing attempt records. Compare the proposed Outputs shape with QueryDefinition.Fields and WorkflowRunRetentionService before resolving the capture policy. Done means later actions can read outputs, unnamed provider fields are not stored, and a test proves that behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend-api-design, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.