Task: Finish the workflow fixture

on_start_workflow calls stamp_command_published after it publishes, and on_step_completed calls set_step_state when the answer arrives. Both are a read-modify-write of the same row under optimistic locking, and the two subscriptions are not serialised against each other, whatever the engine's own header claims. The fixture answers in about two milliseconds, so the two overlap; the three-millisecond window is the whole bug.

Reproduce it against a live fleet before changing anything. What it looks like when it lands, from ores.workflow.service's log on the rebased #2184:

Publishing step command:  step_index=0 step_name=one step_id=c3f1d1ce-...
Step-completed event received: step=c3f1d1ce-... outcome=completed
Workflow STARTED: step_count=2 first_step=one
ERROR Executing INSERT failed: Version conflict: expected version 1, but current version is 2
ERROR Unhandled exception in NATS message handler for subject '...events.step-completed'

and workflow wait then reports [1/1] one: in_progress until it times out, with nothing the shell can report as an error.

The fix is not a mutex bolted on. If a publish can re-enter a handler on the same thread, a non-recursive std::mutex deadlocks and a recursive one hides the re-entrancy rather than removing it. Establish which of the two it is first: the timestamps above say the completion is processed inside the start, which is what a synchronous delivery on one thread would also look like, and the two want opposite fixes.

workflow start prints "Dispatched", an instance id and a follow-up command whatever happens. It did so for an unregistered type, for a request with no steps, and while every workflow in the product was failing to insert. Three of the four defects found on #2184 were invisible at the shell for exactly that reason, and were read out of a log instead.

The reply must report acceptance and not completion. The start is a durable JetStream publish and callers still poll for the terminal state, so this must not quietly turn a crash-safe publish into a request/response transaction. Record that where the reply is defined.

Named, one script each, asserting terminal state rather than printing it. The set designed on the identity workflow task: happy_path, fan_out, with_warnings, fail_fast, saga, long_running, recovery, empty and unknown_type.

--instance-id is what makes this one invocation per configuration rather than two, so a script can start a run under a name it chose and then wait on it.

Each script must fail when the path it covers does nothing. That is the point of the task, and it is why the race comes first: a configuration written on top of it would pass whenever it happened to win.

Found while measuring the suite. test_timeout_listener, registered by twenty test mains, slept in one-second ticks and testCaseEnded joined it, so every case that finished inside a tick waited out the rest of it. ores.shell.workflow.tests reported 34 cases at almost exactly 1.000s each – 34.17s for tests whose work is milliseconds – and ores.shell.application.tests took 22.23s the same way.

It reached this branch by rebase: the shell workflow tests used to register database_lifecycle_listener, and another environment's commit swapped it for this one, which is why the same target went from 1.3s to 34s at the rebase. The watchdog now waits on a condition variable, so teardown is immediate: 34.17s to 0.11s and 22.23s to 0.69s.

Worth knowing: changing that listener's members is an ABI break for every test binary that instantiates it. Building only the target under test leaves the others linking the new libores.testing.so with the old layout, which aborts with "terminate called without an active exception". All twenty were rebuilt.

The fixture needs no other component's data, so every configuration runs as super_admin in the system tenant. Drive it from ores.shell:

compass shell -l <script>.ores

--load runs a script non-interactively and exits. A script cannot read a command's output back, which is why a run that has to be followed is named by its caller:

workflow start identity_workflow '{"steps":[{"name":"one"}]}' --instance-id <uuid>
workflow wait <uuid> --expect-steps 1

Measured first, because the two candidate causes want opposite fixes. Both engine handlers were given a log line carrying std::this_thread::get_id(), the service was restarted and a two-step identity run was started. They are on different threads:

on_start_workflow   thread=140192874710720
on_step_completed   thread=140192883103424

and the interleaving reproduced on the same run:

23:13:26.802333 Publishing step command: step_index=0 step_id=7f7da171-...
23:13:26.804214 Step-completed event received: step=7f7da171-... outcome=completed
23:13:26.816318 Workflow STARTED: step_count=2 first_step=one
23:13:26.820405 ERROR Executing INSERT failed: Version conflict: expected version 1, but current version is 2

So it is true concurrency, not re-entrancy, and the engine's own header claim that the callbacks are "protected by the NATS library's serialisation guarantees" is simply false. That is what made the fix safe to pick: a plain std::mutex cannot deadlock, because a publish is asynchronous and the handler it provokes never re-enters this one on the same thread.

The engine now holds a std::mutex for the whole of on_start_workflow, on_step_completed and recover_in_progress, and the header says what is actually true, with the measurement in it. Held for the whole handler rather than around the write, because the read-modify-write is the thing that has to be atomic.

Verified on the same probe, a fresh id, with the fix in:

Start-workflow request received: instance_id=eeeeeeee-1111-2222-3333-444444444444
Publishing step command: step_index=0 step_name=one
Workflow STARTED: step_count=2 first_step=one
Step-completed event received: step=0 outcome=completed
Publishing step command: step_index=1 step_name=two
Dispatched step 1/1 (two)
Step-completed event received: step=1 outcome=completed
Workflow COMPLETED: steps=2

Zero version conflicts in the whole log, and the shell agrees:

workflow wait eeeeeeee-... --timeout 60 --expect-steps 2
  [1/2] one: completed
  [2/2] two: completed
✓ All 2 step(s) completed.

All four workflow test targets pass.

workflow start now matches the type against the registered definitions before it dispatches anything, and then waits for the instance row the engine creates. What it reports is acceptance, not completion: the publish stays a durable JetStream publish and callers still follow the run with workflow wait.

Verified against the live fleet:

workflow start no_such_workflow '{"steps":[{"name":"one"}]}'
✗ No workflow type named 'no_such_workflow' is registered.
✗ Registered types: bundle_publish_workflow identity_workflow ore_import_workflow
  provision_parties_workflow report_execution_workflow
✗ Script aborted at line 1

workflow start identity_workflow '{"steps":[{"name":"one"}]}' --instance-id ffffffff-...
Started identity_workflow.
workflow_instance_id: ffffffff-1111-2222-3333-444444444444

Six recipes under doc/recipes/shell/workflow/, each tangling into a script that asserts its own terminal state: happy_path, fan_out, with_warnings, fail_fast, saga and long_running. All six verified in one pass against the live fleet, twelve commands and no aborts.

What they needed was workflow wait --expect-state, because a wait that only knows "all steps completed" cannot assert a run whose correct end is failed or compensated, and an aborted script cannot be told from a broken one.

Three paths have no script, and section 4 covers them instead. empty and unknown_type start nothing, so there is no terminal state for a script to assert; recovery needs a service restart mid-run, which a script cannot orchestrate; and a duplicate completion cannot be produced from outside the engine at all.

The instance ids are fixed, so a second run of any script reports the existing run rather than re-exercising the engine: each path is proven on first run only.

Four cases in ores.workflow.core.tests, in workflow_engine_tests.cpp. A --load script can only start a run and watch it, so it cannot ask the engine to refuse a start, cannot deliver a completion twice, and cannot run a recovery pass. The handlers are called directly here, with the real store and the real bus either side of them.

• starts nothing for a type it does not know and starts nothing for a definition with no steps assert no instance row and no step command. Each then makes the same call with a type that does resolve, and asserts that it does create one: that control is the only thing separating "the engine refused" from "the harness could not have worked either way".
• advances once when a step completes twice. Two steps are declared, so a second advance would end the run rather than repeat a step. The instance is still in_progress at step 1, with two step rows and two dispatched commands.
• recovery re-dispatches the step that was in progress. The re-dispatch carries the same step id, which is the idempotency key a service deduplicates on.

The harness runs the fixture in the test tenant, so the rows a case writes are its own, and reads the state ids from the system tenant, where dq seeds them. A state id carries no foreign key to a machine, so the two halves need not agree on a tenant.

A suite is only a regression test if it fails when the engine stops doing what it claims. Disabling the duplicate guard in on_step_completed fails exactly one case, on the instance state it asserts, and no other; the guard was then restored. The two refusal cases carry their control inside the case, so they cannot pass against a harness that starts nothing at all.

recover_in_progress is tenant-wide: it re-dispatches every instance the tenant has in progress, including the ones the earlier cases leave behind. The recovery case first counted the commands arriving on its subject, so the count depended on how many recoverable instances the other cases had left and on the order they ran in – green in one run, four commands and a foreign step id in the next. Every wait is now scoped to the case's own instance id, read from the X-Workflow-Instance-Id header. Five consecutive runs are green.