Fix discarding identification and audit - #6841
Open
TIVMOF wants to merge 18 commits into
Open
Conversation
…ification Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
openBuilder() returns as soon as navigation does, and the shell's scripts are deferred, so the probe for Alpine and the Builder's stores ran against a page that had not finished booting. On a slow runner that reports a healthy shell as broken - which is what it did on this branch's smoke run, in a test that has nothing to do with the change under review. Poll the same condition to a 30s deadline. A mis-ordered or missing script never registers the stores, so a real breakage still fails; it just fails at the timeout rather than instantly. Polling happens in the calling thread because Selenide binds its WebDriver per-thread. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tches
Three PRs in a row failed their smoke run on assertions that were right about the
generator and wrong about how they looked at it. All three were the same two
hazards, so fix the hazards rather than the sites.
Comments. The events templates carry long explanations that necessarily name the
very calls and topics being asserted on, so a contains() could be satisfied by
prose the code never contained - and an assertFalse() tripped by prose the code
never earned. Read generated Java through codeOf(), which strips comments with a
scanner rather than a regex, because a "//" inside a literal (an endpoint URL, a
JSON template) would otherwise cut real code away. Three assertions turned out to
be about prose on purpose - a scaffold naming its field in the javadoc, a
developer's preserved marker, a guard matched by its trailing comment. The first
two stay on contentOf() and say why; the third now matches the guard itself.
First matches. indexOf() answers with the first hit and says nothing about a
second, so an anchor that becomes ambiguous silently relocates the assertion. That
is how the number stamp's reload assertion came to be about its guard read
instead. Ordering assertions now go through onlyIndexOf(), which fails at the
anchor when it is not unique.
Coverage. ModelGenerationIT exists to catch unresolved ${...} in rendered output,
but its fixture declared no resolves, writers or numbering - three of the four
templates these PRs touched were never rendered by it. Adding them is fixture-only
and verified red-first: dropping setProperty, keyAccessor and series each surfaces
as an unresolved reference.
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.
…arding # 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:
resolves:throws away the identification when the status move is rejectedresolves:batched three semantically independent values — the resolved relation, theoutcome:trace, and the routing status — into ONE
updatePropertiescall. The generated DAO runs thelifecycle:andchecks:gates against the post-write row before persisting anything(
Repository.java.template: values applied in memory →enforceLifecycleMove→enforceChecks→super.updateProperties), so a rejected status move discarded the relation and the trace with it.The system did the work, got the right answer, and threw all of it away — including the audit field
whose entire purpose is recording what happened. And because the listener dispatcher gave no
explanation, a reviewer could not tell this apart from "the lookup never ran".
The fix
stamp()now writes twice, in this order:read, exactly one row covered the date, this is the answer.
A rejected transition can no longer destroy the identification, and the audit trace is the last
thing a failure can take.
The routing write catches
ValidationExceptionspecifically. Retrying cannot help — nothing aboutthe record changes by re-reading the register — so rethrowing would only burn the redelivery budget
(now real, since #6830) and dead-letter. Instead it logs and amends the trace to
<outcome>-notRouted, so a routed-but-rejected record stays distinguishable from a fully processedone. A genuine DB failure is not caught and still propagates and retries.
Both writes stay targeted, so no
-updatedre-fires and no concurrent write to another column isreverted.
Also here
A parser guard that the
outcome:field is long enough for the values written — 9 normally, 19 onceany outcome routes by
setStatus. Truncation happens at the DB where nothing reports it, whichwould have reintroduced the same silent loss through the back door. Only bites an author who
explicitly narrows the field (the default VARCHAR length is 100).
Verification
IntentEngineIT.resolve_writes_the_result_before_the_routing_status— asserts the write order, thatthe status is not in the batched map, the
ValidationExceptioncatch, the amended trace, andthe log line.
ResolveIntentTest.rejectsAnOutcomeFieldTooShortForTheValuesWritten— red-first verified; alsoasserts the non-routing case still accepts a shorter field.
formatter:validateclean.#ifshapes (writesStatus×outcomeProperty).Not run: the new IT.
engine-camel's Maven plugin is unresolvable in my offline environment,which blocks the whole failsafe suite. CI will be its first execution.