Fixing statuses not being checked against lifecycle - #6843
Merged
Conversation
… flip the source before creating the target Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The completion hook now flips the source status BEFORE the target is saved, and
the comment block explaining why moved up with it. That comment names
"-transitioned", so indexOf("-transitioned") found the comment rather than the
sendToTopic call and reported the publish as happening before the save - the
assertion failed on a generator that was doing exactly what it should.
Match the call's argument instead. The sibling assertion just below already
guards against the same trap for updateWithoutEvent; this extends it to the
publish, so an explanatory comment can name the topic without standing in for it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous push updated this PR's head but GitHub created no workflow run for it, leaving the PR displaying the previous head's conclusion. Empty commit to fire pull_request.synchronize so the checks run against the current code.
…ing-generate # Conflicts: # components/engine/engine-intent/CLAUDE.md
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.
This PR:
Two status writes were never checked against
lifecycle:, and the worst one runs lastWhen an entity declares
lifecycle:, the parser validatestransitions:entries and workflowsetRelationField/checks:status writes against the edge graph. Two constructs that also writethe lifecycle FK were never added to that check:
generates[].sourceStatus—IntentParser.java:5276only verifies thefromentity has anEntityStatusrelation. No edge check.resolves:outcomesetStatus—validateResolveOutcomesonly verifies a positive seed id andthat the record declares a status relation. No edge check.
Both parse and generate cleanly and fail only at runtime, where
enforceLifecycleMovethrowsValidationException.The ordering half makes
sourceStatusworseIn
Generate.java.templatethe write order was: target saved → items saved → then thesourceStatusflip. No try/catch, no rollback. A lifecycle-rejected flip therefore left a committedtarget document behind whose source never transitioned, so nothing published
-transitionedandevery downstream posting/integration keyed on the new status silently never ran.
Worse, it was unrepairable: the at-most-once guard returns the existing target before reaching
the flip, so a redelivery handed back the orphaned document instead of completing the transition.
Operator symptom: "the invoice exists but the timesheet still shows as approved."
The fix
Parser —
validateStatusWritesAgainstLifecyclenow coversgenerates[].sourceStatusand everyresolves:outcomesetStatusalongside the workflow setters and checks it already covered. Bothvalues are static in the model, so it is the same reachability check applied to two more call sites.
A cross-model source is skipped, exactly as the
generatesblock's own checks skip it.Ordering — the flip moves to just before
save(target), after the target is built in memory (sofield assignments still read the pre-flip snapshot). The
-transitionedpublish stays last: thetransition is complete only once the document it was about exists.
This composes with the redelivery that landed in #6830. A rejected move now throws with nothing
created, and a redelivery after a transient failure re-runs the flip as a no-op
(
enforceLifecycleMoveshort-circuits whenprevious.equals(next)) and goes on to create what ismissing.
Reviewer notes
the source's status (a chained rollup or posting), the two orderings require different edges —
old needed
post-side-effect → Z, new needspre-side-effect → Z. Narrow, but real. An authorrelying on it now gets a parse error instead of silence.
sourceStatustakes a seed id only. It is not one of the sitesStatusSymbolResolverrewrites, so a seeded name never reaches the graph. Documented in the module guide; widening it is
separate work.
findAll(eq(backRefProperty, sourceId))with no further discrimination, so a voided target stillclaims its source and an event-driven create-from can never mint a replacement.
postings:solvedthe same problem with a
stornoProperty/stornoFilterPropertylink;generateshas noequivalent. Button-driven create-from is unaffected — it carries no guard by design.
Verification
LifecycleGraphIntentTest— four new cases (reject + accept for each construct), red-firstverified: 2 failures without the parser change.
IntentEngineIT.generates_completion_hook_flips_the_source_via_targeted_update— extended withthe ordering assertion (flip < save < publish).
formatter:validateclean.sourceRepositoryverified in scope at the later publish.Not run: the IT assertion, for the offline
engine-camelreason above.