Problem/Motivation

AttachmentManager::startForEntity() binds a content entity to the run it starts. Four things about that binding are wrong, and each one ends the same way: a finished run that looks correctly bound to content nothing ever touched.

The binding was made after the run had already advanced. Orchestra ships execution_mode synchronous, and a synchronous start drains the run inline before start() returns, so a binding made after that call is made after every node up to the first wait has executed, and those are the nodes that read the attachment. An action node targeting the attached entity resolved nothing, was caught into its payload variable as failure, the token advanced and the instance completed.

A binding cannot recognize its own run from the outside. The engine saves the instance it is creating, and seeds its variables, before it announces it, and both of those run every module's entity insert hooks. Another instance can therefore be created and announced inside that window, and its announcement arrives first. Anything binding by watching for the next announcement binds to that rival and leaves the run the caller asked for bound to nothing, already advancing.

An unsaved entity was stored as an empty id. The attachment's entity_id field is required, but a required field is a validation constraint and save() does not validate, so attaching an entity with no id wrote the empty string. Every later lookup resolved nothing and the run completed bound to nothing.

A key past the column width failed differently on each backend. An attachment key is 64 characters wide. Past that MySQL refuses the write and SQLite keeps the whole string, so the same configuration behaved differently depending on the database, and on MySQL the failure surfaced as a raw SQL error rather than as a refusal.

Proposed resolution

Give the engine's start() an optional callable, run inside the start transaction once the instance is saved and its variables are seeded, before the start token is placed and before InstanceStartedEvent is dispatched, and handed the instance the call just created. A caller that binds something of its own to the run it is starting hands the work in and is given its own instance, so there is nothing to recognize and nothing to mistake. The binding is committed with the start and is in place before the first node runs, in either execution mode.

The event stays what it is for: reacting to anyone's start. It now fires after the caller's own work, so a subscriber reads a fully wired instance.

Refuse in attach() what storage cannot hold, since that method is already the single place that decides what a key is: an entity with no id, because a binding has to name something, and a key longer than the column, because a key is a lookup discriminator and truncating it would make two long keys resolve to each other's entity.

Record who started a run, too. Only the webform handler passed an initiator, so a run started from the operator UI, from the API, from an ECA model or with an attached entity recorded nobody: it never appeared in its starter's own list of runs, and under the narrowest read policy, which admits the initiator alone, nobody could read it. Each of those doorways now names the account whose request caused the run, and an unattended one, under cron or a queue, still names nobody. The content's owner is deliberately never used: an initiator is not only a label, since a run admits its initiator to read it, so attributing by ownership would hand whoever the authored-by field names the right to read a run they did not ask for.

Remaining tasks

Review the merge request. Having attach() check that the initiator may see the entity being bound is left for its own issue, since it needs a stated rule for a start that has no initiator.

Release notes snippet

A run now records who started it wherever there is somebody to record: the operator who pressed Start, the account an API call arrived as, whoever's request fired an ECA model. Such a run reaches its starter's own list and is readable by them, and the API reports that id where it previously reported none. A process started with AttachmentManager::startForEntity() now binds its content entity before the run executes its first node, and binds it to the run that call started. Previously, under the default synchronous execution mode, the binding was made after the run had already advanced, so a step that reads the attachment resolved nothing. Attaching an unsaved entity, or a key longer than storage allows, is now refused rather than stored or reported as a database error.

AI-Generated: Yes (Claude Code was used to help draft this issue summary and to write the fix and its tests on the merge request. I reviewed the work, and each new test was confirmed to fail against the code it fixes and to pass with the change.)

Issue fork orchestra-3622714

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Status: Active » Needs review
mably’s picture

Title: Bind the entity before the run advances in startForEntity() » Bind the entity to the run the caller started, and refuse a binding that cannot hold
Issue summary: View changes
mably’s picture

Issue summary: View changes

  • mably committed 601f35ba on 1.x
    fix: #3622714 Bind the entity to the run the caller started, and refuse...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.