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 intoSep 10, 2026
Conversation
…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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three leftovers from #7165 (#7056, step resilience on a
notify:service task).1. The
SEND_YAMLfixture was a self-loopResilienceBpmnTestbuilt the send fixture with two.replacecalls: the first inserted thenotifyOwnersend step carryingonError: recordFailure, next: done; the second then replaced everyonError: recordFailure, next: donewith... next: notifyOwner- rewriting the just-inserted send too, sonotifyOwnergotnext: notifyOwneranddonewas unreachable. The tests passed only because the parser checks thatnext:names an existing step.PT1Mcycle), so the chain isprovisionApp -> notifyOwner -> done. TwoassertFlowassertions pin the corrected topology so the loop can't come back silently.aSendWithoutResilienceKeysEmitsNoCyclealso carried a dead first.replace("... onError: notifyOwner, ", "")- that text never occurs - now deleted.2.
next: <self>is now refused at parseA step whose
nextnames itself was accepted with no diagnostic (it is an existing step), and a generated.bpmnwith a self-targeting sequence flow spins.IntentParsernow refuses it with a message naming the step;StepResilienceIntentTest.aNextTargetingItselfIsRejectedcovers 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.mdand the javadoc ofResilientClassDelegateFactorydescribed the wiring asconfig.setActivityBehaviorFactory(new DefaultActivityBehaviorFactory(...)); production (BpmFlowableConfig) wiresnew ResilientActivityBehaviorFactory(new ResilientClassDelegateFactory())- the class #7165 added. Both corrected.Verification
mvn formatter:validateafter wiping the formatter cache - BUILD SUCCESS.engine-intentunit suite - green (1188 tests, incl. the fixedResilienceBpmnTestand the newStepResilienceIntentTestcase).IntentEngineIT- green (74/74), confirming valid intents still parse and generate (the new guard refuses only a genuine self-loop).engine-bpm-flowablechange is javadoc-only and compiles clean.A stale-state
core-initializersflake 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