Problem
CmfPluginInvoker::invoke is a read-modify-write across an await: it snapshots the shared payload under the lock, releases it, awaits invoke_entries, then writes the plugin's returned payload back wholesale.
A route with parallel: ["plugin(redact-ssn)", "plugin(scrub-token)"] spawns both branches onto a JoinSet sharing one CmfPluginInvoker. Both snapshot the same original payload, each plugin redacts its own field, and the second write overwrites the first. One redaction is lost.
The read-modify-write predates #157. What #157 changes is the consequence: the sticky payload_modified flag now makes the route handler forward the surviving payload and assert it was modified, so a lost redaction is forwarded to the host as a successfully-mutated payload rather than being uniformly dropped.
Separately, apl_core::rules::Effect::Plugin returns false from contains_mutation, so validate_parallel_purity permits plugin(...) inside parallel: in the first place. apl_cpex::parallel_safety::validate_parallel_plugin_modes exists and is unit-tested, but is never called during route registration — so nothing rejects a payload-modifying plugin under parallel: at config load.
Two candidate fixes (choosing between them is the point of this issue)
- Reject at load. Wire
validate_parallel_plugin_modes into the route-registration path in visitor.rs alongside the other post-IR validations, so a Sequential/Transform plugin inside parallel: fails config load. Smaller, and matches how FieldOp and Delegate are already refused inside parallel:. Cost: narrows what operators can express.
- Close the window. Hold a single
MutexGuard on self.payload across invoke_entries, or re-read under the lock and merge the returned payload the way apply_changed_paths merges pipeline edits, rather than assigning wholesale. Cost: serializing branches through the payload mutex partly defeats the purpose of parallel:.
Reviewers split between these, which is why this is filed rather than fixed in #157.
Evidence
crates/apl-cpex/src/cmf_invoker.rs:335 — let current_payload = self.payload.lock().await.clone(); (snapshot, lock released at end of statement)
crates/apl-cpex/src/cmf_invoker.rs:381 — *self.payload.lock().await = modified.clone(); (wholesale write-back of whatever a sibling branch wrote in between)
crates/apl-core/src/rules.rs:282-291 — Effect::Plugin returns false from contains_mutation, so validate_parallel_purity allows it inside parallel:
crates/apl-cpex/src/parallel_safety.rs — validate_parallel_plugin_modes exists and is tested, but has no caller in the registration path
crates/apl-core/src/evaluator.rs:1006-1028 — dispatch_parallel spawns branches with Arc::clone(plugins), so they share one invoker
Suggested test
A parallel: block with two mutating plugins either fails to load (fix 1) or preserves both mutations (fix 2). Neither direction is covered today.
Severity: P2
Confidence: 100 (merged from three independent reviewers at 75/50/50)
Reviewer(s): adversarial, correctness, security
Finding ID: crates/apl-cpex/src/cmf_invoker.rs:381:parallel-branches-lose-payload-mutations
Surfaced by: code review of #157
Run artifact: /tmp/compound-engineering/ce-code-review/20260806-022016-2517f78a/
Problem
CmfPluginInvoker::invokeis a read-modify-write across an await: it snapshots the shared payload under the lock, releases it, awaitsinvoke_entries, then writes the plugin's returned payload back wholesale.A route with
parallel: ["plugin(redact-ssn)", "plugin(scrub-token)"]spawns both branches onto a JoinSet sharing oneCmfPluginInvoker. Both snapshot the same original payload, each plugin redacts its own field, and the second write overwrites the first. One redaction is lost.The read-modify-write predates #157. What #157 changes is the consequence: the sticky
payload_modifiedflag now makes the route handler forward the surviving payload and assert it was modified, so a lost redaction is forwarded to the host as a successfully-mutated payload rather than being uniformly dropped.Separately,
apl_core::rules::Effect::Pluginreturnsfalsefromcontains_mutation, sovalidate_parallel_puritypermitsplugin(...)insideparallel:in the first place.apl_cpex::parallel_safety::validate_parallel_plugin_modesexists and is unit-tested, but is never called during route registration — so nothing rejects a payload-modifying plugin underparallel:at config load.Two candidate fixes (choosing between them is the point of this issue)
validate_parallel_plugin_modesinto the route-registration path invisitor.rsalongside the other post-IR validations, so a Sequential/Transform plugin insideparallel:fails config load. Smaller, and matches howFieldOpandDelegateare already refused insideparallel:. Cost: narrows what operators can express.MutexGuardonself.payloadacrossinvoke_entries, or re-read under the lock and merge the returned payload the wayapply_changed_pathsmerges pipeline edits, rather than assigning wholesale. Cost: serializing branches through the payload mutex partly defeats the purpose ofparallel:.Reviewers split between these, which is why this is filed rather than fixed in #157.
Evidence
crates/apl-cpex/src/cmf_invoker.rs:335—let current_payload = self.payload.lock().await.clone();(snapshot, lock released at end of statement)crates/apl-cpex/src/cmf_invoker.rs:381—*self.payload.lock().await = modified.clone();(wholesale write-back of whatever a sibling branch wrote in between)crates/apl-core/src/rules.rs:282-291—Effect::Pluginreturns false fromcontains_mutation, sovalidate_parallel_purityallows it insideparallel:crates/apl-cpex/src/parallel_safety.rs—validate_parallel_plugin_modesexists and is tested, but has no caller in the registration pathcrates/apl-core/src/evaluator.rs:1006-1028—dispatch_parallelspawns branches withArc::clone(plugins), so they share one invokerSuggested test
A
parallel:block with two mutating plugins either fails to load (fix 1) or preserves both mutations (fix 2). Neither direction is covered today.Severity: P2
Confidence: 100 (merged from three independent reviewers at 75/50/50)
Reviewer(s): adversarial, correctness, security
Finding ID:
crates/apl-cpex/src/cmf_invoker.rs:381:parallel-branches-lose-payload-mutationsSurfaced by: code review of #157
Run artifact:
/tmp/compound-engineering/ce-code-review/20260806-022016-2517f78a/