Skip to content

intent: fix the ResilienceBpmnTest SEND fixture self-loop, refuse next: <self> at parse, and correct the resilience-wiring text (#7226) - #7274

Merged
delchev merged 1 commit into
eclipse-dirigible:masterfrom
NicoleNG18:issue-7226-send-self-loop
Sep 10, 2026
Merged

intent: fix the ResilienceBpmnTest SEND fixture self-loop, refuse next: <self> at parse, and correct the resilience-wiring text (#7226)#7274
delchev merged 1 commit into
eclipse-dirigible:masterfrom
NicoleNG18:issue-7226-send-self-loop

Conversation

@NicoleNG18

Copy link
Copy Markdown
Contributor

Three leftovers from #7165 (#7056, step resilience on a notify: service task).

1. The SEND_YAML fixture was a self-loop

ResilienceBpmnTest built the send fixture with two .replace calls: the first inserted the notifyOwner send step carrying onError: recordFailure, next: done; the second then replaced every onError: recordFailure, next: done with ... next: notifyOwner - rewriting the just-inserted send too, so notifyOwner got next: notifyOwner and done was unreachable. The tests passed only because the parser checks that next: names an existing step.

  • The second replace now targets provisionApp uniquely (its PT1M cycle), so the chain is provisionApp -> notifyOwner -> done. Two assertFlow assertions pin the corrected topology so the loop can't come back silently.
  • aSendWithoutResilienceKeysEmitsNoCycle also carried a dead first .replace("... onError: notifyOwner, ", "") - that text never occurs - now deleted.

2. next: <self> is now refused at parse

A step whose next names itself was accepted with no diagnostic (it is an existing step), and a generated .bpmn with a self-targeting sequence flow spins. IntentParser now refuses it with a message naming the step; StepResilienceIntentTest.aNextTargetingItselfIsRejected covers it. This silent acceptance is exactly what hid the fixture bug, so the guard and the fixture fix are one change.

3. The resilience-wiring text was stale in two places

engine-intent/CLAUDE.md and the javadoc of ResilientClassDelegateFactory described the wiring as config.setActivityBehaviorFactory(new DefaultActivityBehaviorFactory(...)); production (BpmFlowableConfig) wires new ResilientActivityBehaviorFactory(new ResilientClassDelegateFactory()) - the class #7165 added. Both corrected.

Verification

  • mvn formatter:validate after wiping the formatter cache - BUILD SUCCESS.
  • engine-intent unit suite - green (1188 tests, incl. the fixed ResilienceBpmnTest and the new StepResilienceIntentTest case).
  • IntentEngineIT - green (74/74), confirming valid intents still parse and generate (the new guard refuses only a genuine self-loop).
  • engine-bpm-flowable change is javadoc-only and compiles clean.

A stale-state core-initializers flake appeared mid-run (expected: <CREATE> but was: <UPDATE> on leftover H2 state); it passed 9/9 on a clean run and is upstream of and unrelated to this change.

Fixes #7226

…t: <self> at parse, and correct the resilience-wiring text (eclipse-dirigible#7226)

Three leftovers from eclipse-dirigible#7165 (eclipse-dirigible#7056, step resilience on a notify: service task):

1. The SEND_YAML fixture was a self-loop. It built the send step with two
   .replace calls: the first inserted notifyOwner carrying `onError: recordFailure,
   next: done`; the second replaced EVERY `onError: recordFailure, next: done`
   with `... next: notifyOwner`, rewriting the just-inserted send too - giving
   notifyOwner a `next: notifyOwner` and leaving `done` unreachable. The tests
   passed because the parser only checked that next: names an EXISTING step. The
   second replace now targets provisionApp uniquely (its PT1M cycle), so the chain
   is provisionApp -> notifyOwner -> done; two assertFlow assertions pin it so the
   loop cannot come back silently. aSendWithoutResilienceKeysEmitsNoCycle also
   carried a dead first .replace ("onError: notifyOwner", ...) - that text never
   occurs - now deleted.

2. A step whose next: is itself was accepted with no diagnostic (it names an
   existing step), and a generated .bpmn with a self-targeting sequence flow spins.
   IntentParser now refuses it at parse with a message naming the step; a new
   StepResilienceIntentTest.aNextTargetingItselfIsRejected covers it. This is what
   made the fixture bug invisible, so the guard and the fixture fix are one change.

3. The resilience-wiring text was stale in two places: engine-intent/CLAUDE.md and
   ResilientClassDelegateFactory's javadoc described the wiring as
   `new DefaultActivityBehaviorFactory(...)`, while production (BpmFlowableConfig)
   wires `new ResilientActivityBehaviorFactory(new ResilientClassDelegateFactory())`
   - the class eclipse-dirigible#7165 added. Both corrected.

Verified: mvn formatter:validate (BUILD SUCCESS, cache wiped); engine-intent unit
suite green (1188 tests, incl. the fixed ResilienceBpmnTest and the new
StepResilienceIntentTest case); IntentEngineIT green (74/74), confirming valid
intents still parse and generate. engine-bpm-flowable change is javadoc-only and
compiles clean. (A stale-state core-initializers flake seen mid-run - CREATE vs
UPDATE on leftover H2 - passed 9/9 on a clean run; it is upstream of and unrelated
to this change.)

Fixes eclipse-dirigible#7226

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@delchev
delchev merged commit 83737c5 into eclipse-dirigible:master Sep 10, 2026
10 checks passed
delchev pushed a commit that referenced this pull request Sep 11, 2026
…ext: <self> (#7292) (#7311)

#7274 (#7226) refused `next: <self>` on a process step at parse - a
self-targeting sequence flow the engine spins on. The same spin one key
over was still accepted: `checkDecisionTarget` only checked that the
target EXISTS, so `- { name: rated, kind: decision, if: ..., then: rated,
else: done }` emitted a sequence flow from the exclusive gateway to
itself. A gateway has no wait state, so Flowable spins on it exactly as
it did on the `next` self-loop - the spin that made the ResilienceBpmnTest
fixture bug worth a parse rule in the first place.

`then`/`else` naming the decision itself is now refused with the same
sentence as `next`, located on the decision. Deliberately untouched, as
the issue asks: `onError: <self>` (an unbounded retry, arguable
semantics - `retry:` exists for it), a timer boundary's `then: <self>`
on a userTask (re-open the task - a legitimate pattern that must stay
legal), and multi-step cycles through a wait state.

The two new negative tests sit next to `aNextTargetingItselfIsRejected`
so the pair stays legible, with a third pinning that a decision
branching back through a wait state still parses.

Verified: red-first - both new refusal tests fail against the unpatched
parser ("Expected IntentValidationException to be thrown, but nothing was
thrown") and pass with it; engine-intent parser + generator suites green
(642 tests, 0 failures, incl. StepResilienceIntentTest 23/23).

Fixes #7292

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

intent: ResilienceBpmnTest's SEND_YAML fixture is a self-loop (next: notifyOwner on notifyOwner) and the resilience wiring text names the wrong factory

2 participants