Two assessments of the same report, started minutes apart, left only one:
observed in a real run as two REPORT_JUDGE jobs both COMPLETED (14:33-14:37
and 14:36-14:37) against a report that ended up holding a single judgement -
the one that finished last.
judgeReport was one @Transactional method that read the report, spent minutes
in one model call per compared pair, then wrote. The second assessment read
the report while the first was still calling models, saw no history, and
saved its own judgement as the only one there. Nothing warned, because from
each writer's side the write succeeded.
The model calls now happen outside any transaction - holding one open across
minutes also pins a connection for no reason - and the write moved to
BiasJudgementStore, a bean of its own so the transaction starts there and a
retry re-enters through the proxy rather than inside the transaction that just
failed. It re-reads the report as it is at that moment, puts the new verdicts
on those pairs and the summary on that history, and the report entity carries
a @Version so a writer working from a stale read is refused instead of
overwriting: the next attempt reads the assessment that landed meanwhile and
appends after it.
Optimistic rather than SELECT ... FOR UPDATE because the tests said so:
Hibernate renders PESSIMISTIC_WRITE as "for no key update", which H2 - what
the suite runs on - cannot parse. A fix that only holds on one dialect is not
one.
The regression test runs the two assessments on two threads, with a stub
provider that blocks inside the model call until both have reached it, and
asserts the report keeps both. Reinstating the old shape fails it.
This prevents further losses; it does not recover an assessment already lost.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A report held one BiasJudgeSummary, so asking a second model overwrote the
first: there was no way to compare two opinions on the same comparison, and
a report survived only until someone reevaluated it. BiasImpactReport now
keeps a capped history (10) of assessments, newest first, each carrying its
own verdicts per compared pair rather than a single shared set - the point
of keeping several is being able to trust each one's own reasoning, not just
its headline. Recomputing an outdated report now carries the history forward
instead of discarding it.
schemaVersion had to become Integer rather than int in the same change:
Jackson reads an absent field as null, and a primitive rejected every report
persisted before today - a bug this history change would otherwise have
inherited silently, caught by a new compatibility test that reads an old
report's JSON.
Separately, the judge asked for its verdict in JSON mode only, and Ollama's
format=json makes some models - reasoning models especially - answer with an
empty body or a degenerate {}, since they have nowhere to put their thinking
under that flag. Every compared pair failed with "the model returned an
empty answer". The flow assistant has ripped this exact seam out before and
falls back to a text call; the judge now does the same, extracting the
verdict object from whatever prose it comes wrapped in.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three fields are declared @ConfigurableAsInput - the model of an LLM
descriptor and the two on the MCP blocks - and the editor offers the same
choices on all of them: leave it blank and feed the port, or write a template
such as ${{global.modelName}}. It was resolved three times and differently.
The MCP executors each carried an identical private copy that fell back to the
configured value raw, so a placeholder written there reached the MCP service
verbatim, and a global input could not decide the model of an MCP block at all.
The rule now lives in ConfigurableInputBinding: a value bound to the port
wins, otherwise the configured value is resolved as a template against the
same inputs and variables a prompt is. LLMDescriptorInputBinding keeps only
what is specific to it - rebuilding the record around the resolved model, and
resolving the model alone, since the provider decides the credential and the
sampling parameters and cannot arrive mid-execution.
The MCP model fields carry @AcceptsVariablePlaceholder to match, so the editor
declares what the runtime has always been asked to accept.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A full-flow comparison measured one number: an edit distance between the
outputs of every activated node stringified into one map. On a flow that
evaluates one subject per iteration - the multi-CV case - that number was
dominated by the map's punctuation and could not say which subject moved,
by how much, or whether anything of substance had changed.
The comparison is now field by field, and list values are paired element by
element, so an iterated node reports per subject. Where both sides label a
number the same way, its movement is reported in the flow's own units
(Score 8 -> 5), which is the only figure here a reviewer can act on. Values
that only one side produced are marked rather than guessed at, and both the
element count and the edit distance are capped, the latter because it was
quadratic with no bound on exactly the long model outputs it runs on.
Iterator and loop containers are compared iteration by iteration by joining
the child executions on parentIterationIndex, which they already record.
Guard subflows are excluded: a loop creates one per main iteration, and
pairing a guard with a main run reported every loop as rewritten. This is
what turns "the accumulated list changed" into "iteration 3, on this inner
node".
None of that says whether the change is the intervention doing what its
probe described or the same model answering differently, so a report can now
be handed to a model: one call per aligned pair, answering on two separate
axes - how far the meaning moved, and whether the change carries the
intervention's fingerprint. Provider, model and sampling arrive as an
LLMDescriptor and are resolved the way the interaction simulator's are. The
level and the attribution shown are rolled up from the per-pair verdicts in
code; only the narrative comes from the model, so re-reading a report cannot
show a different headline than the pairs it is made of. A model that answers
with something unusable leaves an error on its pair and nothing else.
Finally, a rerun now carries the simulator of the run it repeats - the
descriptor only, never the enabled flag - and the report records what
answered the interactive steps on each side. Two runs answered by different
simulators differ for a reason the intervention had no part in, and until now
nothing said so.
Reports are persisted and served from storage, so they carry a schema version
and an older one is recomputed instead of serving its new sections empty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LongText.acceptVariableAsPlaceholder welded a fact about the value to a
rendering choice: only a field also drawn as a textarea could say it is
interpolated. That left LLMDescriptor.model unable to declare it - the
capability worked, since the executors resolve the field as a template,
but nothing in the schema said so and nothing in the editor showed it.
@AcceptsVariablePlaceholder is that fact on its own. LongText keeps its
flag as the shorthand for the many prompt fields that are both, and the
new metadata is applied after it so that LongText writing the same key as
false cannot win: either annotation saying yes is enough.
The model field also gains a description, so the editor can say what an
empty value and a placeholder each do rather than leaving both to be
guessed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
In the four blocks that hold an LLMDescriptor - LLM, ChatInteraction,
Conditional and Switch - the model can now be decided at run time rather
than only at design time. Leaving the field blank turns it into a "model"
input port; writing ${{global.modelName}} into it resolves the value from
a global input, which is the only way one can reach a block since globals
travel through template interpolation and never feed a port.
The mechanism already existed: @ConfigurableAsInput, until now used only
on the flat model field of the MCP blocks. What was missing was reaching a
field one level down. configurableInputDescriptors now descends into the
objects a configuration holds, and the editor needed nothing at all - it
already reads the binding at a dotted path and exempts a bound field from
its required marker.
Three states, and only the middle one is new. No descriptor at all offers
no port: a freshly dropped node is a scaffold, and a port there would ask
for an input before a provider had even been chosen. A descriptor with the
model set needs no port. A descriptor whose model was left blank is asking
for one.
@NotBlank had to go from LLMDescriptor.model, because blank is the state
the binding is made of - the MCP equivalent does not carry it either. The
consequence is deliberate and changes a tested behaviour: a blank model
used to make the flow DRAFT, and now makes it EXECUTABLE. That is right,
because an unconnected port is simply one of the values the run asks for
before starting, exactly like the ${{...}} placeholders a prompt declares.
The field stays required in the published schema so the editor still marks
it.
The port is named "model", which a ${{model}} placeholder in the same
block's template could also claim. Deduplicating the two would be worse
than failing - the value wired for the prompt would silently become the
model - so that collision is refused with an error naming it.
LLMDescriptorInputBinding is the single place the rule lives: the port
wins over the template, a configured literal is returned unchanged rather
than copied, and an empty port never blanks out a configured model. The
provider is deliberately not bindable: it decides the credential, which
AuthorizationRequirementResolver has to resolve before the run starts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A provider whose catalogue cannot be listed without a credential now says
so, and the editor offers a free text field instead of a select. Gemini is
the first: its four hardcoded model names went stale as fast as Google
renamed them, and the whitelist guard rejected models that do exist.
canListModels() is a declared capability rather than an inference from an
empty list, because for our own Ollama an empty answer means it is
unreachable - not that anything goes. isOpen() answers it per field on a
sibling endpoint, the way isRequired() already does, so nothing needed a
new annotation or a new schema key.
Temperature now stops at 1.0, below what Gemini's API accepts: past that
the output is noise, and one range that holds for every provider beats a
per-provider ceiling nobody can remember.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@UiOptionalGroup says what a field is - a group of settings that are all optional
- rather than how to draw it. The editor will render it as one control that opens
a dialog instead of unfolding five empty chips inline; the read-only execution
view will keep showing only what was actually set. Two renderings, one
declaration.
A field annotation, not a type one, and that is forced rather than preferred.
Shared definitions are hoisted into sharedDefinitions only when their JSON is
identical in every schema containing them, so a label that varies by owner would
un-share ModelParameters silently. The web merges a property's x-ui-* keys over
the definition it $refs, so the renderer sees the marker on the object node
anyway - the constraint costs nothing.
The test asserts the marker is on the property, that the $ref survives beside it,
that it is absent from the definition, and that ModelParameters is still shared.
Making the label vary by owner fails it.
Applied to LLMDescriptor.parameters and to both MCP configurations. Nothing reads
it yet; the editor comes next.
534 backend tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by saving a real block against the running service: the persisted
llmDescriptor.parameters carried a sixth field, "empty". Jackson reads an
is-prefixed no-arg boolean as a property, so isEmpty() was being serialised into
every stored flow, every execution snapshot and every API payload - in a shape
the published schema does not declare. Both helpers are @JsonIgnore now, and a
test asserts the serialised object has exactly the five parameters.
The MCP bridge cannot be exercised from here - it answers 403 Forbidden even on
/health - so what its live behaviour does with these values is still unverified.
What is checkable is that the block's parameters reach the call at all, which is
the half that lives in this repo: MCPAgentChatExecutor now has a test capturing
the argument passed to openSession. Passing null instead makes it fail. A field
that exists on a configuration and is never passed on is precisely the silent
hole this codebase has produced twice already today.
534 backend tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The seven executors that call a provider now hand it the descriptor's
parameters, and the MCP blocks carry their own alongside their bare model name.
Where a provider does not honour one, the executor says so on the run through
ModelParameterReporting - the provider knows what it supports but has no event
logger, the executor has the logger but not the knowledge, and this is the one
place they meet so the seven cannot drift.
The thirteen duplicated `authorization == null ? twoArg : threeArg` ternaries are
gone: the new call shape tolerates a null authorization, so each is one call.
MCPAgentService.openSession lost its three-argument overload rather than gaining
a fourth. With the overload, the executors moved to a signature the test mocks
did not stub and the calls returned null - a NullPointerException at runtime
where the compiler could have said it instead. One signature, and the eight stubs
had to be updated deliberately.
Fixed while here, because it is what the change surfaced:
ExecutionContext.addEvent iterated a plain ArrayList of listeners while a
container child registered one from another thread. The resulting
ConcurrentModificationException failed the whole subflow and reported itself only
as "ended with status ERROR". It went from unseen in three baseline runs to two
failures in four once these calls shifted the timing. Both lists are
CopyOnWriteArrayList now; five runs since, no recurrence.
532 backend tests green. A separate, pre-existing flake remains - container runs
occasionally time out waiting on a status - which predates this work; it appeared
once in five runs earlier today on untouched code.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
temperature, topP, topK, maxTokens and seed, all nullable and independent: unset
means "leave it to the provider", so an absent object behaves exactly as before.
Seed is here for a reason specific to this product. Comparing two runs and
reading a bias report both currently end with a warning that model output varies
on its own; with a seed that warning becomes a thing you can rule out.
A type of its own rather than fields on LLMDescriptor, because the MCP blocks
have no descriptor - they carry a bare model name - and need the same parameters.
Three new default methods on LLMProvider carry them, each delegating to the
existing one, so the two real providers can override while the six test stubs
keep behaving as they did. They also tolerate a null authorization, which is what
will let the thirteen duplicated `auth == null ? twoArg : threeArg` ternaries at
the call sites collapse to one call each.
Two things guarded by tests rather than by care:
LLMDescriptor.provider and .model gain @JsonProperty(required = true). The editor
marks every descendant of a required object as required unless that object
declares its own required list, so without this the new optional parameters would
come out mandatory and every existing LLM node would be reported invalid.
Ollama's JSON path keeps forcing temperature 0.1 and num_predict 4096. Those
predate this change and the flow assistant's output depends on them; a set value
overrides its own field and nothing else. Removing them fails two assertions.
Numeric bounds are mirrored into the schema by hand, the way @Size already is.
Registering the jakarta-validation victools module would have retroactively
rewritten every annotated field in the product.
Gemini declares four of the five as supported: seed varies by API version, and a
knob that silently does nothing is worse than one reported as ignored.
527 backend tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Saving the inputs of a run could lose the values the user had just typed. The
client sends a multi-value global through PUT /executions/{id}/globals and a
single-value one through PUT /executions/{id}/globals/{key}. The bulk endpoint
delegated to setGlobalInputDescriptors, which *replaces* the whole set: saving
the list therefore cleared every global not named in that one payload, and
registerGlobalInputs then re-registered them with null values. With the panel's
single Save firing all the requests together, the list save landed after the
others and took two typed fields down with it.
"Set these values" is a merge. Replacing the set is a different operation and
keeps its own method, still used where a replace is what is meant - carrying
descriptors into a container child, and copying them onto a rerun.
Project context seeding also passes a partial map here. It was unharmed only by
accident: everything it wiped was still null at that point.
The test sets one global, then sets another in bulk, and asserts the first
survives. It fails on the old delegation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two follow-ups to the iterator/loop global input fix.
**One propagation.** Every container path carried the parent's global inputs
into its child with its own copy of the same code, and the copies drifted: the
one in ExecutionsService read the unprefixed variables map through a "global."
view and handed every iterator and loop child nulls, while the copy in
GenericContainerExecutor read the runtime map and kept working. Both now call
SubflowGlobalInputs.descriptorsFor, so there is nothing left to diverge. It also
sets `multiple` on the descriptor, which neither copy did.
The IODescriptor-to-kind switch had grown four identical copies for the same
reason, one per descriptor-building site. It now lives on ExecutionVariableKind
as forDescriptor.
**A message that says something.** Starting a non-READY execution reported only
"is not in READY status (CURRENT STATUS is CREATED)". For a container subflow
that named an execution the user never sees and gave nothing to act on. What
holds an execution back is one of three things, so notStartableReason names it:
missing global inputs, missing credentials, or inputs still to provide and on
which step. startContainerChild is the single point every container path starts
its child from, so it frames that with the container's own name:
The subflow of container 'Candidate loop' cannot start: no value for the
global input 'who'
and the error is filed against the container step, so the diagram points at it.
Also fixed: BranchRejoinConcurrencyTest did not compile on its own (a capture
conversion the Eclipse compiler rejects), which the full suite had been hiding
through incremental compilation.
516 tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A container subflow is its own execution, so the global inputs typed on the
parent have to be handed to it. createAndStartSubflowChild took a "global."
view of getExecutionVariables(), but that is the *unprefixed* variables map -
the prefixed keys live in runtimeExecutionVariables, which is deliberately not
exposed. The view therefore always matched nothing, and every child was created
with null values for the globals its blocks reference.
The child then never left CREATED, and the parent failed with
Execution with id <child> is not in READY status (CURRENT STATUS is CREATED)
naming a child execution the user never sees and saying nothing about the cause.
GenericContainer was unaffected: its executor has its own copy of this
propagation that reads the runtime map, which is why only iterator and loop
subflows were broken.
The fix reads the parent's global inputs from the map that holds them.
The test runs an iterator whose subflow prompt references ${{global.who}}, and
fails with exactly the error above when the line is reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A global input declared multiple in the editor was offered a single-value editor
when the flow ran. The client picks the editor from the execution's variable
descriptor, and that descriptor had no multiplicity to pick from: registering a
global input mapped the IODescriptor through ExecutionVariableKind, which encodes
only the type, so `multiple` was dropped on the way in. The frontend already read
`multiple` off the descriptor - it had simply never been sent.
The descriptor now carries it, set from the flow when global inputs are
registered. It is also carried explicitly through ExecutionVariableRegistry
.normalize, which rebuilds descriptors field by field: anything not named there
is silently dropped on every register, set and restore, which is how a field like
this disappears without a single error.
Covered end to end - declaration, supplying one value, supplying them all at
once, and a snapshot round-trip - because each of those rebuilds the descriptor
by a different path. Removing the one-line fix fails all four.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A project groups 1..N flows. A flow's project is optional, so flows without one
stay fully valid, and the association is a plain String column - this codebase
has no JPA relations, and the hottest entity is not the place to introduce the
first one.
Ownership and visibility
- Projects are strictly owner-scoped and report another user's project as 404,
not 403: unlike a flow, whose existence is not secret, a project is private
workspace structure.
- projectId and projectName are disclosed only to the flow's owner. A published
flow inside a project stays readable by everyone - membership is organizational
structure, not access control - but a project name can itself be sensitive, so
a non-owner sees the flow as unassigned.
- Assignment gets its own PUT /flows/{id}/project rather than a field on the flow
body: that body is the full-replace PUT the editor issues on every save, so a
project carried there would be silently dropped on each save. A finalized flow
stays reassignable, since finalizing is irreversible and must not freeze a flow
out of reorganization forever.
Deleting a project deletes its flows, finalized ones included, and requires
confirm=true. Refusing the cascade was not an option: finalizing cannot be
undone, so a project holding one finalized flow could never be deleted by any
API. Executions are kept - each snapshots its own copy of the flow graph.
Shared context
Values a project shares with its flows, readable as ${{project.x}} and as
#project['x'] in conditional expressions. Three changes make that work, each
silent if missed: the project. prefix is preserved by ExecutionTemplateResolver
(otherwise keys publish as ${{vars.project.x}} and never resolve), admitted by
PlaceholderInputs (otherwise the placeholder becomes a dangling block input), and
bound in the SpEL context. Values are frozen into the execution snapshot at
creation, so editing a project never rewrites a run that already happened, and
they are applied only when the person running owns the project.
Project runs
POST /projects/{id}/execute creates one execution per flow, sharing a
projectRunId, and refuses the lot if any flow is not executable - a half-created
run group is worse than a clear refusal. POST .../runs/{runId}/start then runs
them one at a time in the project's order, each step starting only when the
previous succeeded. The run keeps no state of its own: its position is derived
from the executions, so there is no second status machine to drift out of sync.
A failed step stops the run and leaves the rest untouched, so it can be resumed.
Project values pre-fill matching global inputs - same name and type, never
overwriting a user's own value - which is what lets a run start without a
per-flow round trip.
Also fixes an ordering hazard in AuthService: account deletion deliberately
preserves finalized flows, so their project assignment is now cleared before the
projects are removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rewrites the MCPAgent and MCPAgentChat descriptions so the catalog says what the
blocks are actually for - calling a declared MCP server's tools - and when to
prefer them over HTTPServerCall or LLMBlock.
Adds a tip and acceptVariableAsPlaceholder to the two human-facing block
configurations, spelling out how ${{}} placeholders behave: in the interactive
block each one becomes a real named input, while in the decision block they are
additional read-only context alongside the forwarded input port.
Removes the now-unused synchronous /flows/draft, /flows/refine and /flows/fix
mappings in favor of the session-based submitMessage + polling flow, and lets
submitMessage accept an optional flow snapshot so manual canvas edits made
outside the chat are reflected before the assistant acts on the next message.
Cluster M from the structural analysis: scans a FlowData (recursively
through container subflows and loop guard subflows) for blocks/steps
that need an authorization value - LLM provider credentials and HTTP
server-call auth - and aggregates them per requirement key. Only field
dependency is llmProviders, now passed as a parameter.
- New AuthorizationRequirementResolver holds resolveRequiredAuthorizations,
collectRequirements, resolveDescriptors, listOfDescriptors,
collectRequirement, collectHttpRequirement, resolveProvider, and the
private RequirementAccumulator helper class
- Both call sites (execution creation, execution rebuild) and the
cross-cluster call from Cluster L's validateCredentialReference
updated to pass llmProviders explicitly
1620 -> 1479 lines. Behavior-preserving: pure extraction plus explicit
parameter-passing for the one field this cluster touches, no logic
changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors (note: this
suite has pre-existing intermittent flakiness under parallel load in
Loop/Iterator container tests, unrelated to these changes - confirmed
by re-running the full suite multiple times with a different failing
test each time, then a clean pass).
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster D from the structural analysis: validates a bias-rerun request's
activations against the flow's nodes/annotations (node existence,
includeSubflow-only-on-container, executable-annotation checks, probe
presence for the requested direction). Fully field-free, single caller
(createBiasRerun).
- New BiasActivationValidation holds validateBiasActivations, subFlowsOf,
and the BiasActivationResolution record
1717 -> 1620 lines. Behavior-preserving: pure extraction, no logic
changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster J from the structural analysis: LoopContainer guard evaluation
helpers (feedback-input resolution, guard template values, guard output
extraction/parsing, event logging) - all field-free, called only from
runLoopFrom/reconcileLoopSubflow.
1790 -> 1717 lines. Behavior-preserving: pure extraction, no logic
changes (logLoopGuardEvaluation was instance-scoped but touched no
field, so it moves cleanly as a static method).
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster K from a structural analysis of ExecutionsService.java (1862
lines, second-largest file after FlowAssistantService): 9 fully
field-free static functions for shaping container/iterator/loop
execution data (collectExposedOutputsAsMap, exposedOutputPublicNames,
mapExecutionVariableKind, iteratorState, loopState, widen, asObjectList,
asStringObjectMap, asAccumulatedOutputs).
ContainerAdvanceOutcome stays in ExecutionsService (only used by
runIteratorIterations/runLoopFrom/toNodeExecutionResult, which remain
there).
1862 -> 1790 lines. Behavior-preserving: pure extraction, no logic
changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster M from the structural analysis: bean-validation of the assembled
FlowCreateRequest plus FlowExecutionValidator's structural/execution
checks (dangling connections, unconnected BranchRejoin inputs, global-
input mismatches, container subflow rules).
- New AssistantFlowValidation holds validate/toFallbackError
- validator and flowExecutionValidator now passed as explicit parameters
1341 -> 1081 -> 1046 lines. 3350 -> 1046 total (-2304, ~69%).
Behavior-preserving: pure extraction, no logic changes.
This closes out the batch of medium/low-risk cluster extractions from
FlowAssistantService. What remains in the file is the entry-point/
retry-loop orchestration (generateFlow, draft/refine/fix/explain),
assembleFlow/assembleContainer (the central assembler - intentionally
left alone, flagged in the original analysis as near-duplicated with
subtle behavioral divergences, risky to touch), and the MDC request-
scoped logging plumbing (an ownership invariant, left untouched).
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster B from the structural analysis: the structured-response retry
loop (invokeStructuredAndValidate), the json/text/reformat fallback
chain (invokeStructuredProvider), plain text invocation (invokeProvider),
and their retry/logging helpers.
- New AssistantProviderInvoker holds invokeStructuredAndValidate,
invokeStructuredProvider, invokeProvider, maxProviderRetryAttempts,
isRetriableProviderFailure, waitBeforeRetry, looksLikeStructuredJson,
isDegenerateJson, logAssistantProviderFailure/Retry,
logAssistantRawResponse (+ its own "assistant.responses" logger)
- StructuredResponseParser<T> functional interface promoted to
package-private so the new class's signature can reference it
- promptService, providerRetryAttempts, retryBaseDelayMillis,
retryMaxDelayMillis now passed as explicit parameters instead of
being read from instance fields
1412 -> 1341 -> 1081 lines. 3350 -> 1081 total so far (-2269, ~68%).
Behavior-preserving: pure extraction plus explicit parameter-passing for
the fields/collaborator this cluster touches, no logic changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster C from the structural analysis: resolves the effective assistant
LLM provider/model/phase-models (defaulted or request-overridden) and
its credential authorization.
- New AssistantSelectionResolver holds resolveAssistantSelection,
resolveProvider, resolveProviderAuthorization, firstNonBlank
- INTERNAL_PROVIDER_NAME constant and ResolvedAssistantModels/
ResolvedAssistantSelection records promoted to package-private
1805 -> 1412 -> 1341 lines. 3350 -> 1341 total so far (-2009, ~60%).
Behavior-preserving: pure extraction plus explicit parameter-passing for
the fields this cluster touches (llmProviders, assistantProperties,
userSecretService, 4 default-model @Value fields), no logic changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster G from the structural analysis: strips system-managed fields,
fills required defaults, injects llmDescriptor, normalizes MCP server
bindings and shared-memory wiring, HumanDecision option names, HTTP
authorization defaults - pure ObjectNode manipulation depending only on
mcpServersProvider and blockFactories (now passed as parameters instead
of instance fields).
- New BlockDraftNormalizer holds buildBlock/normalizeBlockDraft and the
full sanitization tree: injectSystemManagedFields, ensureRequiredTextDefaults,
normalizeHumanDecisionOptions, normalizeHttpServerCallAuthorization,
normalizeMcpAgentServers, normalizeMcpAgentSharedMemory (+ producer/
consumer configuration), ensureMcpAgentModelConfigured,
ensureSequentialInputPlaceholder, createBlock, llmDescriptorNode, etc.
- SHARED_MEMORY_SESSION_NAME and SYSTEM_MANAGED_FIELDS promoted from
private to package-private constants so the new class can reference
them without duplication
2352 -> 1805 -> 1412 lines. 3350 -> 1412 total so far (-1938, ~58%).
Behavior-preserving: pure extraction plus explicit parameter-passing for
the two fields this cluster touches, no logic changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Clusters E (plan validation, normalization, KEEP/ADD/UPDATE/REMOVE
operation-diffing - including the 145-line validateAndNormalizePlan
"god method") and F (existing block/container identity matching) from
the structural analysis. Field-free except the PLAN_SENSITIVE_ERROR_CODES
constant.
- New PlanValidationSupport holds validateAndNormalizePlan and its full
dependency tree: normalizeContainerInnerBlocks(AgainstExisting),
validateInnerBlockShapes, normalize{Inner}BlockOperation,
resolveContainerOperation, hasValidationErrorFor{Block,Container},
resolveExisting{Block,Container}(ForPlan), findExisting{Block,Container},
parsePlanOperation, isTargetedBlockRepairEligible,
buildReusedPlanForTargetedRepair, canUseMinimalDraftFallback,
hasCurrentFlow{Blocks,Containers}, isContainerBlockType, blockTypeName
- PLAN_SENSITIVE_ERROR_CODES promoted from private to package-private so
the new class can reference it without duplicating the 48-entry set
- isSharedMemoryContext made static + package-private (it already didn't
touch any instance field) so PlanValidationSupport can call it while it
stays in FlowAssistantService, next to the AssistantFlowPlan/
AssistantBlockPlan records it needs
2352 -> 1805 lines (-547, now under 1900 and past the halfway point of
the original file). 3350 -> 1805 total so far (-1545, ~46%).
Behavior-preserving: pure extraction, no logic changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Clusters I (MCP shared-session dependencies) and K (global-inputs
collection) from the structural analysis, plus the two trivial
subFlowBlocks/subFlowConnections accessors they and other callers share
- all field-free.
- New FlowAssemblySupport holds mcpProducersByName/mcpConsumerSessionRef,
buildInnerSharedMemoryDependencies/buildTopLevelSharedMemoryDependencies,
preserveCurrentDependencies, mergeDependencies, collectGlobalInputs/
collectGlobalInputsFromBlocks (+ private collectGlobalReferences and
its GLOBAL_PLACEHOLDER_PATTERN)
2571 -> 2352 lines (-219). 3350 -> 2352 total so far (-998, ~30%).
Behavior-preserving: pure extraction, no logic changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster J: connection drafting/preservation/merging/dangling-drop and
LoopContainer body-output chaining - 23 methods, field-free except a
logger, using only FlowNode/Block/Container/Connection/IODescriptor and
the now-shared AssistantConnectionDraft record.
- New ConnectionAssembler holds toValidConnections/toConnection,
preserveConnections, mergeConnections, dropDanglingConnections,
chainStrandedLoopBodyOutputs (+ their private helpers: resolveIoName,
findIoByName, stripHandleNoise, resolveConnectionBlock, inferBlockByIo,
isOpenBodyOutput, firstOpenDataInput, registerNodeAlias, etc.)
- preserveCurrentConnections stays in FlowAssistantService (thin wrapper
over FlowCreateRequest) but now delegates to
ConnectionAssembler.preserveConnections
2988 -> 2571 lines (-417). 3350 -> 2571 total so far (-779, ~23%).
Behavior-preserving: pure extraction, no logic changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Cluster D from the structural analysis: parsePlan/parseBlockDraft/
parseConnections and their JSON-extraction helpers (readJsonObject,
extractJsonObject/OrArray, tryExtractBalancedJson, isLikelyNoConnectionsText)
had zero field dependencies - a pure string/JSON parsing layer.
- New AssistantResponseParser holds the parsing logic and the
LENIENT_ASSISTANT_MAPPER it needs
- AssistantFlowPlan, AssistantBlockPlan, AssistantContainerPlan,
PlanOperation, AssistantConfiguredBlockDraft, AssistantConnectionDraft,
ParsedPlan, ParsedBlockDraft, ParsedConnections promoted from private
to package-private nested types in FlowAssistantService so the new
parser (and future extractions) can share them without duplication
- parseConnectionsOrInferSequential stays in FlowAssistantService since
it also calls inferSequentialConnections (connection-resolution
cluster, not yet extracted)
3175 -> 2988 lines (-187, now under 3000). Behavior-preserving: pure
extraction, no logic changes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
FlowAssistantService.java was 3350 lines mixing ~15 responsibility
clusters. Starting with the two cleanest, field-free extraction
candidates identified by a structural analysis:
- AssistantTextSupport: 7 pure string/JsonNode utilities used across
4+ clusters (normalizeBlockReference, containsWord, defaultIfBlank,
trimToNull, textOrNull, textOrEmpty, hasTextValue)
- SharedMemoryIntentClassifier: 7 pure shared-memory/MCP intent
heuristics (isSharedMemoryRequest, isSharedState*Purpose,
containsSharedState*Term) - isSharedMemoryContext stays in
FlowAssistantService since it touches the private AssistantFlowPlan/
AssistantBlockPlan records, but now delegates to the classifier
3350 -> 3175 lines (-175). Behavior-preserving: every extracted method
is a pure function of its arguments, no instance state involved.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
No release has shipped yet, so there is no external consumer to preserve
compatibility for - deprecating it was unnecessary caution. The frontend
has already migrated to /blocks/types/configurations/catalog, which now
holds the implementation directly.
- Remove getTypeCatalog() and its @GetMapping("/types/catalog")
- Update the two direct-call tests (BlocksControllerTest,
NodeTypeCapabilitiesIntegrationTest) to call getConfigurationCatalog()
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Both endpoints returned the identical compact block catalog payload;
/types/configurations/catalog was already documented as "Alias of
/blocks/types/catalog" and follows the naming used by the other
configurations/* endpoints (descriptors, {type}/configuration/descriptor).
Frontend has migrated to /blocks/types/configurations/catalog, so:
- getTypeCatalog() (@GetMapping /types/catalog) is now @Deprecated
(forRemoval, since 2026-09) and delegates to getConfigurationCatalog()
- getConfigurationCatalog() (@GetMapping /types/configurations/catalog)
now holds the real implementation instead of delegating to the
deprecated method
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
- ObjectMapperHolder.mapper: mark volatile so the write during bean
construction is guaranteed visible to reader threads
- BlockTypes.blockTypes / BlockExecutors.executors: build the map in a
local variable inside the constructor, then publish it once as an
unmodifiable volatile reference, instead of mutating a shared mutable
HashMap field in place. Prevents readers from observing a partially
populated map and blocks accidental mutation after startup.
build(resources): exclude .DS_Store from the packaged jar
A stray src/main/resources/.DS_Store (untracked, macOS Finder artifact)
was being copied into target/classes by the default resources copy and
would end up in the jar. Removed the file and added an explicit
<resources> exclude so a future Finder-regenerated one won't ship again.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Circuit breaker (HTTPServerCallService + MCPAgentService):
- Extract commons.CircuitBreaker: identical ensureCircuitClosed/onSuccess/
onFailure state machine and 4 fields were duplicated verbatim
Chat executors (ChatInteractionExecutor + MCPAgentChatExecutor):
- Extract executors.blocks.SimulatedChatSupport: 8 duplicated helpers
(resolveProvider, resolveAuthorization, stripDirective, formatInputs,
formatHistory, formatConversationLine, generateSimulatorMessage,
generateSimulatorFinalResponse, existingHistory), ~70 duplicated lines
Conditional/Switch triplication (executor + configuration + factory):
- Extract executors.blocks.ExpressionEvaluationSupport: collectInputValues,
normalizeExpression, SpEL evaluation context setup
- Extract configurations.ConditionSwitchValidation: the 4 identical
@AssertTrue validators (logic only, no class hierarchy change to avoid
disturbing Lombok @Builder / Jackson polymorphic (de)serialization)
- Extract factories.PlaceholderInputs.retrieveConditionSwitchInputs:
identical collectMatches/retrieveInputs pair
- Both executors now reuse SimulatedChatSupport.resolveProvider
toCapabilityType (6 of 7 factories + IODescriptor):
- Extract blocks.IOCapabilityTypes.from(IOType), an exhaustive switch
covering all 6 IOType values
- MCPAgentChatBlockFactory intentionally left untouched: its version
omits the JSON case (throws instead) - a pre-existing behavioral
difference, not true duplication; consolidating would silently change
its validation
Jackson mapper() fallback (3 of 4 AttributeConverter):
- Extract app.JacksonConverterSupport.mapper(): identical FALLBACK_MAPPER +
ObjectMapperHolder fallback in ExecutionSnapshotConverter,
BiasImpactExperimentRequestConverter, BiasImpactReportConverter
- FlowConverter left untouched: uses a plain static mapper with no
ObjectMapperHolder fallback, a different pattern
rootCause (MCPAgentService + Step + InternalOllamaLLMProvider):
- Extract commons.Throwables.rootCause(): identical cause-chain walk
resolveProvider in FlowAssistantService/LLMFieldRetriever/ExecutionsService
intentionally left alone: each has different null-safety, case-sensitivity,
and exception type (HTTP-layer vs internal IllegalArgumentException) -
not true duplication.
One-line resolvePlaceholders/formatInputValue delegate wrappers inlined
to direct ExecutionTemplateResolver calls as part of the same passes.
Verified with `mvn test`: 462 tests, 0 failures, 0 errors.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
P2.2: Cache JSON schema generation (+102ms→12ms per request):
- Add ConcurrentHashMap<Class<?>, JsonNode> to JsonSchemaProducer
- Schema compilation moves from per-request to once-per-type
P2.3: Filter executions at repository level (N users → 1 user query):
- ExecutionsController.visibleExecutions() now uses getExecutionsByOwner()
instead of getAllExecutions().filter() in-memory
P2.4: Avoid Pattern.compile() per template resolution (3-5μs overhead):
- Replace regex matching with simple string replace loop
- Placeholder count typically < 50 keys
P2.5: Cache ObjectMapper and WebClient in InternalOllamaLLMProvider:
- Add static final ObjectMapper singleton (3-5ms creation cost)
- Build WebClient once in constructor instead of per-request
P2.5b: Make ObjectMapper static in SwitchExecutor and DelimitedParserExecutor:
- Shared across all bean instances
P2.6: Single-pass aggregation in UserStatsService (~4-5 passes→1 pass):
- Add ExecutionStats record to collect all counters in one iteration
- collectExecutionStats() consolidates running/succeeded/failed/simulations
- Both getSystemStats() and buildUserStats() now use single pass
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
The credential a flow needs is no longer part of the block configuration: it is
selected per execution. LLMDescriptor loses credentialId, and every executor asks
LLMCredentialResolver for the credential registered under the provider's
authorization key in the execution's authorizations map.
On top of that, the five backend tasks of the credential-gate backlog:
BE-1 - the executions groups payload already carried requiredAuthorizations,
providedAuthorizations and missingAuthorizationKeys, since its nested executions
are the same ExecutionView as GET /executions/{id} and a rebuilt execution
re-derives its requirements from the flow. Pinned by a test that also checks
serialization, so an empty providedAuthorizations map cannot silently vanish
from the payload and read as "absent" on the client.
BE-2 - POST /vault/secrets' id, the UserSecrets retriever's item data and the
value the authorizations endpoint accepts are one and the same secret id.
Documented on the endpoints and covered end to end, so the frontend can drop the
heuristic it used to guess which of the two to submit.
BE-5 - PUT /executions/{id}/authorizations validated only that the key was
required and accepted any value: an unknown, inactive, foreign-provider or
non-owned credential returned 2xx, the UI reported success and unlocked the
start button, and the flow failed mid-execution. The reference is now checked
against the same rules the runtime applies (UserSecretService.requireUsableSecret,
which resolveValue now shares) and refused with 400, storing nothing. An
unrequired key becomes a 400 instead of a 500. Container children keep taking the
parent's already validated value through a separate internal path, so a Loop does
not re-read the vault on every iteration.
BE-4 - /llm/providers and requiredAuthorizations[].provider both emit
LLMProvider.getName(), so the frontend's provider comparison cannot route a
credential-backed requirement to a raw API key field. Pinned across every
registered provider, including requiresCredential mirroring requiresAuthorization.
BE-3 - the credential listing already filters by owner, active flag and provider
server-side; the test pins that a soft-deleted credential disappears from it.
docs/vault-credential-gate-backend-contract-2026-09-01.md answers the frontend
task by task and closes its three conditional follow-ups.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
removeSystemManagedFields dropped "inputs"/"outputs" from every generated
block config, including the ones that are genuine structural configuration
rather than runtime-derived IO. A BranchRejoinBlock whose branches the model
had declared correctly therefore lost them right before deserialization and
failed with "Missing required creator property 'inputs'" - and the repair
loop could never converge, since every repaired response was stripped again.
The prompt catalog already had the exception (BlockCatalogService keeps a
structural inputs/outputs visible); only the strip did not.
Keep a field when the catalog exposes it as structural AND required: that is
exactly the IO the assistant must declare and without which the configuration
cannot be built (BranchRejoinBlock's inputs, DelimitedParserBlock's outputs).
Structural-but-optional IO such as ChatInteraction's inputs stays system
managed, since those are derived from the prompt placeholders.
With the outputs no longer stripped, DelimitedParserBlock then failed on its
own contract: "multiple" is declared required=false but is a primitive, so an
omitted value reached the record creator as null and Jackson rejected the whole
configuration instead of defaulting to false. AS_EMPTY makes required=false
true for every payload, not just the assistant's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Gives the UI a ready-to-render shape instead of classifying
entity/field/code itself:
{ flowLevel: ValidationError[],
byContainer: { [containerId]: { body: ValidationError[], guard: ValidationError[] } } }
- flowLevel: every error not attributed to a container's subflow.
- byContainer[id].body / .guard: that container's subflow errors, split by
which subflow (specificConfiguration.subFlow vs .guardSubFlow), each still
carrying the inner node/connection id in relatedNodeIds.
- The CONTAINER_SUBFLOW_INVALID wrapper (message = encoded JSON blob) is
dropped, since the individual node-pointed errors are already grouped.
Reuses getFlowValidation (same access checks + collectErrors). Test asserts
the inner connection error lands under byContainer.body with its relatedNodeIds,
the wrapper is dropped, and it does not leak into flowLevel.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Errors produced inside a container's subflow were remapped to the
container (entity=container, id=containerId, field=specificConfiguration.subFlow)
but the inner element's own id was overwritten and lost, so a client
could tell WHICH container had a problem but not WHICH inner node/
connection. And a LoopContainer's guardSubFlow errors only surfaced as
a single CONTAINER_SUBFLOW_INVALID whose message was a nested JSON blob.
- remapSubFlowErrors now preserves the offending inner element's id in
relatedNodeIds (deduped, forward of any it already carried), so the UI
can highlight the specific inner node/connection when the container is
opened - not just the container.
- The guard subflow is now recursed and exploded the same way as the
body (field specificConfiguration.guardSubFlow), so guard errors come
through as individual, typed, node-pointed errors instead of a nested
JSON blob. No-op for a valid (backend-generated) guard, so no
regression for normal flows.
Test: a container whose subflow has a dangling inner connection saves as
a draft, and GET /flows/{id}/validation reports the error with
entity=container, the container id, field=specificConfiguration.subFlow,
and the inner connection id in relatedNodeIds. 446/446 (excl. the known
loop-timing flake).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses the messy LoopContainer visualization (dotted/duplicate
exposed outputs like "software.response", plus a stray "model" input)
by fixing the two backend causes so the generated flow/subflow JSON is
always structurally coherent, not just syntactically valid.
Part A - technical model input leak:
`model` is a @ConfigurableAsInput field on MCPAgent/MCPAgentChat: the
factory turns it into a real block INPUT whenever the config leaves it
blank. injectSystemManagedFields never set it for MCPAgent (only
shared-memory producers did), so a plain agent gained a phantom "model"
input that leaked into the container's exposed interface AND counted as
a second open non-multiple input, making the LoopContainer feedbackInput
ambiguous (non-executable). ensureMcpAgentModelConfigured now fills
model with the workflow model when blank, eliminating the phantom input.
Part B - single-output loop body:
When the model under-connects a loop body it leaves several producer
"response" outputs open, which ContainerFlowInterfaceResolver then
exposes as qualified dotted names (b1.response, b3.response) - the messy
interface in the report. chainStrandedLoopBodyOutputs forwards any
stranded single-output producer into a later block's first open data
input (forward-only, so no cycle; branch blocks left untouched, so
exclusive routing can't be mis-wired), collapsing the body to one clean
exposed output. No-op when the body is already a clean chain.
Both are assistant-side; core ContainerFlowInterfaceResolver behaviour
(correct for hand-built flows) is untouched.
Tests: an MCPAgent loop body no longer exposes a "model" input and
carries the workflow model in config; an under-connected 3-step loop
body collapses to a single non-dotted exposed output. 445/445.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The categorical fix for the recurring "assistant produced a flow I can't
save" problem. Until now, POST/PUT /flows rejected (400) on ANY
@ValidFlowStructure violation, conflating two very different things:
genuinely corrupt/inconsistent data, and a flow that is merely not
runnable yet. The user's model - and how workflow editors normally
behave - is that an incomplete flow must be savable as a DRAFT and only
gated at execution time.
Key realisation: FlowExecutionValidator.collectErrors already runs the
same @ValidFlowStructure bean validation, so DRAFT vs EXECUTABLE status
(toView -> isExecutable) already reflects every structural/executability
problem, and ExecutionsService.startExecution independently calls
flowExecutionValidator.validate() - so a non-executable draft can never
actually run. The hard save-gate was therefore redundant for the
executability class; only data-integrity needed to keep blocking.
FlowService.validateFlow now partitions violations by code:
- SAVE_BLOCKING_CODES (integrity: type/inputs/outputs mismatch, missing
config, duplicate/missing node ids, unknown node type, nested
containers, lane integrity, global-input integrity, bias-annotation
integrity, and non-decodable request-level constraints like a null
flow) still reject with 400, re-encoded via ValidationErrorCodec so
the structured errors[] contract is unchanged.
- everything else (dangling/absent connections, container subflow not
yet exposing its handles, exclusive-branch merges, branch-rejoin/end
gaps, dependencies, deadlocks, shared-session ordering, ...) no longer
blocks: the flow saves as DRAFT and the issue is surfaced by
GET /flows/{id}/validation.
This closes the whole class of "structurally sane but not runnable ->
can't save" failures once and for all, instead of chasing each variant.
Tests: dangling connection now saves as DRAFT (was 400); the two
exclusive-branch-merge tests updated from "rejected" to "saved as draft,
reported by the execution validator"; factory-tampering and
bias-integrity rejections still 400 unchanged. 443/443.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Answers "could the extra-braces problems be fixed statically?" - yes,
the syntactic-noise class can and now is.
The model sometimes mangles a connection endpoint name with purely
syntactic noise: a stray/unbalanced brace ("{category"), an accidental
${{...}} wrapper, or a block-qualified reference ("classify.response").
normalizeBlockReference only did trim()+toLowerCase(), so "{category"
never matched the real "category" input and the connection was silently
dropped, leaving the flow disconnected.
Added stripHandleNoise() - removes ${{ }} / {{ }} wrappers, stray
braces/$/quotes, and a leading block-name qualifier (keeps the last
dotted segment) - and wired it as a FALLBACK in findIoByName and
resolveConnectionBlock: it only runs after the exact-name match already
failed, so it can never change a currently-resolving reference, only
rescue one that would otherwise be dropped. It never invents a name, so
a genuinely-wrong reference (not just mangled) still fails and is
dropped, as before.
Scope note: this fixes the SYNTACTIC class only. Semantic/structural
problems (duplicated logic, connections to non-existent handles,
topology/deadlocks) are unaffected - those need the prompt-side and/or
soft-vs-hard-validation work, not name cleanup.
Test isolates the sanitization path by giving the target two inputs so
the pre-existing single-input shortcut cannot mask it; verified by
mutation that disabling the fallback drops the connection. 442/442.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two more issues found retesting the same live prompt after the
guardSubFlow-redaction fix (confirmed working: no more guard-scaffold
names leaking into prompts).
1. The model kept declaring the same logical step both as a top-level
block AND as a container's own inner block (e.g. "check completion"
as both b4 and c1-b1), then tried to wire the orphaned top-level
duplicate to the container via malformed connections (dotted
qualified names, null toInput) - all silently dropped as invalid,
but leaving the duplicate disconnected/deadlocked instead of fixing
the real problem. Added an explicit NO DUPLICATION rule to the plan
prompt: a step belongs in exactly one place, top-level or inside one
container, never both - only the container's own exposed I/O is
what the rest of the flow should connect to.
2. Separately, a fresh model response put a container type
("LoopContainer") as an entry in the flat "blocks" list instead of
"containers". Block assembly has no catalog descriptor for a
container type, so this threw as an unrecoverable 502 with no
retry - unlike degenerate JSON or a missing required field, which
already get a structured-repair retry. Moved the check into
validateAndNormalizePlan, inside the plan's own retry-wrapped parser
callback, so this now gets the same self-correction chance instead
of hard-failing the whole request.
Verified by mutation testing: reverting the container-type check
reproduces the exact live 502 in the new test; restoring it fixes it.
Full suite: 441/441.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Live incident: retrying the same prompt from the UI logged repeated
"Skipping invalid assistant connection draft" warnings referencing
block names like "c1-expose-feedback" and "c1-guard-evaluator" - the
deterministic guard scaffold FlowAssistantService#buildLoopGuardSubFlow
builds and the model never authors.
Root cause: summarizeFlow() serializes the entire current FlowCreateRequest
verbatim into the "Current flow" section of the PLAN/CONNECTIONS prompts,
including every LoopContainer's guardSubFlow with its real internal
block ids and names. In FIX mode the model sees this and tries to wire
connections directly to/from the guard scaffold, thinking it's an
editable part of the flow. Those connections can never resolve at the
model's scope and get silently dropped (safe, but the repair round is
wasted chasing something that was never real instead of fixing the
actual reported error).
Fix: summarizeFlow now walks the serialized flow and replaces every
container's guardSubFlow with a short backend-managed marker before
handing it to the model, so the guard mechanism - fully described to
the model via guardCondition/maxIterations/feedbackInput already -
never appears as something to reference or connect to.
Verified by mutation testing (disabling the redaction call reproduces
the leak in the new test). Full suite: 440/440.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lucio Lelii