Commit Graph

227 Commits

Author SHA1 Message Date
Lucio Lelii 4fa87ae3a2 Carry the parent's global inputs into an iterator or loop subflow
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>
2026-09-03 14:53:09 +02:00
Lucio Lelii 7e69e21197 Keep a global input's multiplicity at execution time
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>
2026-09-03 14:04:57 +02:00
Lucio Lelii f9b94d823e Add projects: grouping, shared context, and sequential project runs
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>
2026-09-03 07:53:25 +02:00
Lucio Lelii 61bbbe50d4 Sharpen block descriptions and add placeholder tips
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.
2026-09-03 07:52:52 +02:00
Lucio Lelii ee695b2f5f Finalize async-only assistant flow endpoints, sync client flow edits into session
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.
2026-09-02 13:32:06 +02:00
Lucio Lelii 7f57b8683a refactor(executions): extract authorization requirement resolution into AuthorizationRequirementResolver
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>
2026-09-02 10:57:56 +02:00
Lucio Lelii f5b88d66b4 refactor(executions): extract bias activation validation into BiasActivationValidation
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>
2026-09-02 10:53:10 +02:00
Lucio Lelii 4c627c6c76 refactor(executions): extract loop guard helpers into LoopGuardSupport
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>
2026-09-02 10:51:16 +02:00
Lucio Lelii 4cabbe90d3 refactor(executions): extract container data-shape helpers into ContainerExecutionSupport
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>
2026-09-02 10:49:04 +02:00
Lucio Lelii 9c7e0233a2 refactor(assistant): extract post-assembly validation into AssistantFlowValidation
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>
2026-09-02 10:39:03 +02:00
Lucio Lelii 756c07fe70 refactor(assistant): extract LLM invocation/retry into AssistantProviderInvoker
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>
2026-09-02 10:37:24 +02:00
Lucio Lelii 9c8b0612d4 refactor(assistant): extract provider selection/auth into AssistantSelectionResolver
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>
2026-09-02 10:33:10 +02:00
Lucio Lelii 55a13af364 refactor(assistant): extract block-config sanitization into BlockDraftNormalizer
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>
2026-09-02 10:30:06 +02:00
Lucio Lelii 46a1fb2d3e refactor(assistant): extract plan validation/normalization into PlanValidationSupport
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>
2026-09-02 10:26:25 +02:00
Lucio Lelii 848c703619 refactor(assistant): extract dependency/global-input building into FlowAssemblySupport
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>
2026-09-02 10:19:47 +02:00
Lucio Lelii cf068ffe7f refactor(assistant): extract connection resolution into ConnectionAssembler
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>
2026-09-02 10:16:47 +02:00
Lucio Lelii 76aa3fa71d refactor(assistant): extract JSON response parsing into AssistantResponseParser
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>
2026-09-02 10:12:50 +02:00
Lucio Lelii 4544f0c52d refactor(assistant): extract text/shared-memory helpers from FlowAssistantService
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>
2026-09-02 10:09:16 +02:00
Lucio Lelii 48ddcb70de refactor(api): remove legacy /blocks/types/catalog endpoint
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>
2026-09-02 09:57:50 +02:00
Lucio Lelii 370125eb94 deprecate(api): mark legacy /blocks/types/catalog in favor of /blocks/types/configurations/catalog
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>
2026-09-02 09:56:26 +02:00
Lucio Lelii a18d517b24 fix(concurrency): harden static holders populated by Spring constructors
- 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>
2026-09-02 09:53:13 +02:00
Lucio Lelii 4b3684bf0f refactor(dedup): consolidate P3 code duplication across executors and services
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>
2026-09-02 09:47:50 +02:00
Lucio Lelii eaa5b04821 perf(optimization): implement P2 optimizations with measured impact
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>
2026-09-02 09:24:14 +02:00
Lucio Lelii 9c3cb070b9 optimize(classloader): cache ClassGraph scan in Dynamic*TypeResolver
- Move full-classpath scan from constructor (per ObjectMapper) to static
  initializer (once per class load)
- Add .acceptPackages("it.cnr.isti.workflow.manager") filter to reduce
  scope: 540-880ms → 30-37ms per scan (~95% reduction)
- With 2 independent ObjectMappers (Spring + FlowConverter), recovers ~2s
  of startup latency on first serialization/deserialization

refactor(cleanup): remove dead code and unused imports

Remove 5 unused types (120 LOC):
- ExecutorDescriptor, OutputProducer, InputConsumer, ModelDescriptor,
  app/Validator, SourceBlockType

Remove 14 dead methods (~80 LOC):
- AuthRequest.isValid() (broken + unused)
- AuthRepository (2 methods), LoginEntity.isActiveUser()
- ContainerFlowInterfaceResolver (2), IteratorContainerInterfaceResolver (1)
- ExecutionContext (3), Step (3)
- ExecutionVariableRegistry.valuesView(), MCPSharedSessionRegistry.sharedKeys()
- FlowSharedVariableCatalogService.removeDraftBlock()
- BiasPreparedExecution.getAnnotations()
- 2× iterable() duplicate (JsonSchemaCatalogBundler, JsonSchemaProducer)

Remove 17 unused imports (14 main, 3 test)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
2026-09-02 09:18:47 +02:00
Lucio Lelii 000f6c1133 feat(executions): close the backend half of the vault credential gate
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>
2026-09-01 22:53:01 +02:00
Lucio Lelii 86dfe1bb98 feat: resolve block LLM credentials from user vault 2026-09-01 17:02:49 +02:00
Lucio Lelii d7783e0d23 fix(assistant): stop stripping the IO the assistant is asked to declare
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>
2026-09-01 16:43:31 +02:00
Lucio Lelii 5b5f42a5f9 feat: add user credential vault for assistant providers 2026-09-01 16:32:00 +02:00
Lucio Lelii b27cfc8888 feat: support provider selection for flow assistant 2026-09-01 15:34:41 +02:00
Lucio Lelii 1728ab3c9b feat(flows): add GET /flows/{id}/validation/grouped for pre-grouped errors
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>
2026-08-03 21:44:49 +02:00
Lucio Lelii f297a057de feat(validation): attribute subflow errors to the container and the inner element
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>
2026-08-03 17:34:38 +02:00
Lucio Lelii d5d4118cba fix(assistant): keep container interfaces clean - no technical model input, single-output loop bodies
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>
2026-08-03 17:02:31 +02:00
Lucio Lelii 484de60852 feat(flows): make saving permissive - not-yet-executable flows persist as DRAFT
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>
2026-08-03 16:16:31 +02:00
Lucio Lelii e823d8d357 fix(assistant): statically sanitize brace/wrapper noise in connection endpoint names
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>
2026-08-03 16:01:59 +02:00
Lucio Lelii fad1eaf4b4 fix(assistant): stop duplicating loop-body logic at the top level; retry a misplaced container type instead of 502ing
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>
2026-08-03 15:36:10 +02:00
Lucio Lelii f94fb883d9 fix(assistant): redact LoopContainer guardSubFlow from the model's view
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>
2026-08-03 15:10:27 +02:00
Lucio Lelii 3d16a3b388 fix(assistant): stop resurrecting stale connections across forced-regen repair rounds
Answers "perché il fix non riesce a risolvere il problema?" - traced via
a live repro (session log + direct save attempt) why a LoopContainer
body left in a fully closed 2-block cycle (no input left open for
guard feedback) never got fixed even after both repair rounds ran.

Root cause: when a container-level validation error isn't attributable
to any specific inner block, normalizeContainerInnerBlocksAgainstExisting
force-ADDs every inner block each round (the existing "a FIX round that
changes nothing would never fix the error" fallback) - even though the
model marks them KEEP. But resolveExistingBlock's position-based
fallback still matches the old block regardless of the forced operation,
so oldInnerIdToAssembled/oldNodeIdToAssembledNode got populated anyway,
and preserveConnections carried the OLD (still-broken) connections
forward every round on top of whatever the fresh connections-for-
container call produced. The model has no way to ask for a connection's
removal, so the stale, well-formed (non-dangling) connection survived
indefinitely - the previous dangling-reference fixes didn't catch this
because nothing here is dangling, it's a stale-but-valid connection.

Fix: only populate the old-to-new node id mapping used for connection
preservation when the block/container's operation is genuinely KEEP or
UPDATE, never ADD - regardless of why it became ADD (explicit model
choice or the force-regen fallback). Applied consistently at all three
sites (top-level blocks, top-level containers, container-inner blocks).

Verified by mutation testing: reverting the container-inner guard
reproduces the exact live failure (2 connections instead of 1) in the
new regression test; restoring it fixes it. Full suite: 439/439.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-08-03 13:13:20 +02:00
Lucio Lelii 66112541a5 fix(assistant): add a final defense-in-depth filter against dangling connections
The previous fix made preserveConnections drop a stale carried-forward
connection. That closes the one reproduced cause, but the same failure
mode (a connection referencing a handle name that no longer exists,
which FlowDataValidator treats as a hard, save-blocking structural
error rather than a soft "not executable" one) could in principle recur
through a different code path we haven't hit yet.

Add dropDanglingConnections as a last-resort check applied to the fully
merged connection list (preserved + freshly generated), at both the
top-level flow and every container subflow, right before it's handed
to FlowData.builder(). It drops any connection whose source/target
node or handle name doesn't resolve against the current node graph,
and logs a warning so a future occurrence is visible instead of only
surfacing as a save failure. Dropping a connection whose handle name
was never real does not change what a node exposes, so this is safe
regardless of where the staleness originates.

This is a generic backstop, not a fix for a newly found bug - existing
tests already cover the reproduced scenario and continue to pass
unchanged (438/438), now exercising two redundant layers instead of one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-08-03 12:46:50 +02:00
Lucio Lelii 7319443dab fix(assistant): drop stale connections and mismatched feedbackInput on container FIX
Root-caused a live "Software Development Loop" flow that could not be
saved (400, CONNECTION_TARGET_INPUT_NOT_FOUND on the LoopContainer's
subFlow). Reproduced end-to-end via the running service + assistant
response log:

1. The validation error was reported at the container level (no inner
   block id attached). normalizeContainerInnerBlocksAgainstExisting
   resolves every inner block to KEEP in that case (none match the
   error), which trips the "a FIX round that changes nothing would
   never fix the error" fallback and forces a full regen - every inner
   block gets reconfigured fresh even though the model explicitly
   marked them KEEP.
2. One reconfigured block got a different prompt placeholder, so its
   actual input name changed. preserveConnections still carried the
   OLD connection forward (remapped only by node id, not by whether
   the endpoint's I/O still existed), and mergeConnections unions old
   and new by exact (source,name,target,name) key, so the stale
   connection survived alongside the correct new one - a dangling
   reference to a since-removed input, rejected at save time.
3. Separately, an assistant-guessed feedbackInput ("previous_response")
   that didn't match any actual body input was kept verbatim instead
   of falling back to inference, another guaranteed validation failure
   on the same flow.

Fix: preserveConnections (shared by top-level and container-subflow
reassembly) now drops a carried-forward connection whose source/target
no longer expose that output/input name. LoopContainerConfiguration's
feedbackInput is normalized against the assembled body's actual open
inputs, discarding a non-matching guess so the factory's own inference
(or a clear ambiguity error) takes over instead of a guaranteed-wrong
value.

Tests reproduce the exact scenario: an all-KEEP FIX round that forces
full regen must not leave a stale connection behind, and a mismatched
feedbackInput must not block an otherwise-valid single-input loop body.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-08-03 12:31:42 +02:00
Lucio Lelii effb506572 feat(assistant): make the MCP server catalog central to block selection
The planning and block-config models never saw which MCP servers were
declared, so they could not reach for tool-running steps (writing to a
workspace, compiling, running shell) - the coding-agent-mcp server was
effectively invisible and the assistant fell back to HTTPServerCall.

- Inject the declared MCP catalog (id/name/description) into both the
  PLAN and BLOCK_CONFIG prompts, and steer MCPAgent/MCPAgentChat toward
  running declared-server tools instead of emulating them via HTTP.
- Let the model bind a concrete catalog server via mcpServers; validate
  chosen serverName against the catalog and drop hallucinated ids so a
  bad choice degrades to a clean validation/fallback path instead of a
  runtime "Unknown MCP server" failure. The model's explicit choice is
  preserved and no longer overwritten by the rag default.

Tests: catalog reaches both prompts; a chosen catalog server is kept;
an unknown server id is dropped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-08-03 11:47:15 +02:00
Lucio Lelii 97ad1776a6 fix: null-safe IODescriptor equals/hashCode + normalize HumanDecision option names
Live-testing the assistant (now that the empty-plan fix lets qwen3 produce a
real plan) surfaced a 500 on a plan containing a HumanDecisionBlock. Two
causes, both fixed:

1. IODescriptor.equals/hashCode NPE'd when name was null (name.equals /
   name.hashCode). FlowDataValidator.validateBlock compares block outputs via
   IODescriptor.equals, so a null-named output made the @ValidFlowStructure
   ConstraintValidator throw -> Hibernate HV000028 -> HTTP 500 instead of a
   clean validation error. Made both null-safe with java.util.Objects. Now a
   malformed config is reported as an invalid flow (and repaired), never a 500 -
   a general robustness fix for any flow, not just assistant-generated ones.

2. The null-named outputs came from the model emitting HumanDecision options as
   {label, value} instead of {name, label} (name is the routing key / branch
   output). buildBlock now normalizes options (normalizeHumanDecisionOptions):
   a missing option name is filled from its "value" (the intended routing key)
   or a slug of its "label", so every branch gets a real, non-null output name
   and the flow is valid.

Tests: draftNormalizesHumanDecisionOptionNamesFromValueOrLabel (options given
as {label,value} and {label} -> names "yes"/"reject", no null outputs, valid).
Full suite green (434).
2026-08-03 11:26:35 +02:00
Lucio Lelii 653184eeae fix: recover from empty-plan JSON and default missing required config fields
Two robustness fixes found by testing the assistant live with the default
planning model (qwen3:14b), which produced a trivial single-block flow.

1. Empty-plan recovery. A reasoning model under Ollama's format=json can
   return a degenerate "{}" (it cannot emit its <think> block under the JSON
   grammar). invokeStructuredProvider accepted "{}" as a valid structured
   response, so the plan came back empty and validateAndNormalizePlan applied
   its single-LLMBlock fallback - masking the failure as a trivial flow. A
   degenerate JSON body ("{}", "[]", "") is now treated like a blank response
   and falls through to the non-json generate call, where such models emit the
   real content (parsed by the balanced-brace extractor). The single-block
   fallback stays as a genuine last resort. Fixes the trivial-flow symptom for
   reasoning planning models without a config change.

2. Missing required text field. When the model omits a required free-text
   config field (observed: MCPAgentChatBlockConfiguration.goalDescription),
   deserialization failed with a hard 502. buildBlock now fills any required,
   non-enum, non-structural string field the model left blank with a default
   derived from the block's purpose (ensureRequiredTextDefaults), so an
   omission degrades to a sensible default instead of a 502.

Tests: draftRecoversRealPlanWhenJsonModeReturnsEmptyObject (generateJson
returns "{}", generate returns the real 2-block plan -> flow has 2 blocks, not
the fallback); draftDefaultsMissingRequiredTextFieldInsteadOf502 (MCPAgentChat
with no goalDescription -> valid, goalDescription defaulted to the purpose).
Full assistant suite green; only the known Loop-container timing flake fails
under full-suite load (passes in isolation).
2026-08-03 11:05:58 +02:00
Lucio Lelii 9cc110464d refactor: order MCP shared sessions with a dependency, drop the state_ready connection hack
MCP shared-session ordering was enforced with a fake data connection: the
consumer's prompt carried a magic ${{state_ready}} placeholder to synthesize
an input, and the producer's output was wired into it purely to force
execution order - the consumer never actually used that data (the real
shared state is the MCP session, accessed by name). This replaces that hack
with a Dependency, the mechanism meant exactly for ordering-without-data.
No engine change: the executor already gates readiness on dependencies and
the validator already counts them for reachability; MCPAgent uses the
default activity() capabilities so it can be a dependency source/target.

- FlowAssistantService: after assembly, generate ordering dependencies
  deterministically for all three cases:
    - top-level chain: Dependency(producer, consumer)
    - within-container chain: Dependency(producer, consumer) in the subflow
    - cross-boundary (top-level producer -> consumer inside a container):
      Dependency(producer, container) - the container runs after the producer
      and inherits its session.
  Existing dependencies are preserved on REFINE (preserveCurrentDependencies).
  Removed completeRequiredSequentialConnections, validateSharedMemorySemantics
  and their now-unused helpers (isReachable, summarizeMcpSessionBlocks); the
  ordering no longer flows through connections.
- Prompt: removed the ${{state_ready}} instructions; the model is told the
  backend orders the consumer after the producer via a dependency and must
  not add a placeholder or connection for it.
- Tests: updated all MCP mocks to stop emitting state_ready (prompt and
  connections) and assert the dependency instead (top-level, within-container,
  cross-boundary). Zero state_ready references remain.

Full suite green (431).
2026-07-26 11:37:42 +02:00
Lucio Lelii 68a86564fa feat: teach the assistant that EndBlock is a terminal label, not the flow's output
The assistant tended to close every path with an EndBlock (e.g. the
Jensen flows), which hides the actual result: an EndBlock consumes its
input and produces no output - the value is kept only as an outcome
payload, not as a normal flow output. The flow's readable result is
whatever block outputs are left open (unconnected), per
ExecutionContext.completeStep (which only adds an output to the result
map when !output.isConnected()).

- BLOCK-CHOICE GUIDE: EndBlock is a labelled terminal marker used only to
  record which terminal state a branched path reached (HIRED vs REJECTED)
  or to close a dead branch - never for the main result, and not needed
  to "finish" a linear flow (EndBlock is optional; nothing requires it).
- CONNECTIONS rules: leaving the result-producing block's output open is
  intentional and correct - that open output is the flow's readable
  result; don't wire it into an EndBlock just to terminate it.

Prompt-only change; full assistant suite still green (39).
2026-07-26 00:45:08 +02:00
Lucio Lelii 5d5e94f315 feat: FIX-path container diffing, cross-boundary MCP sessions, multi-input loop bodies
The three optional assistant refinements.

FIX-path incremental container diffing:
- validateAndNormalizePlan now diffs a container's inner blocks against
  the existing subflow in FIX mode too, not only REFINE - so a container-
  internal fix reconfigures only the inner block(s) the model marks,
  reusing the rest. Container-internal validation errors are remapped to
  the container id (the inner block id is lost), so if a FIX round would
  keep every inner block unchanged (the model gave no guidance) it falls
  back to full regeneration, guaranteeing the fix actually happens.

Cross-boundary MCP shared sessions (top-level producer -> inner consumer):
- Verified the runtime supports it: createAndStartSubflowChild (Iterator/
  Loop) and GenericContainerExecutor thread the parent's execution-variable
  descriptors (the MCP session registry) into a subflow and propagate them
  back, so a top-level producer's session is available to a consumer inside
  a container that runs after it.
- FlowExecutionValidator.collectErrors is now external-session aware:
  collectErrors(subFlow, externalSessions) threads the set of sessions
  produced by a top-level block reachable before the container, and a
  consumer is satisfied by such an external producer. This replaces the
  separate per-scope subflow pass for main subflows (which validated them
  in isolation and would reject a valid cross-boundary chain); loop guard
  subflows keep a dedicated external-aware pass since the recursion doesn't
  cover them. Bounded to top-level producers - inner-producer cross-boundary
  chains stay in-scope-only.
- Prompt: relaxed the "never split across a boundary" rule to permit a
  top-level producer wired to run before the container holding the consumer.

Multi-open-input LoopContainer bodies:
- AssistantContainerPlan gains feedbackInput; the LoopContainer config now
  passes it through, so a body exposing several open inputs resolves the
  otherwise-ambiguous feedback-target inference. Prompt updated.

Tests: fixReusesUnchangedInnerBlocksWhenTheModelMarksThem,
draftWiresMcpChainFromTopLevelProducerToConsumerInsideAContainer,
draftAuthorsLoopContainerWithMultiInputBodyAndExplicitFeedbackInput. Full
suite green (431). Roadmap: optional refinements marked done - only bias
annotation authoring (a separate product track) remains.
2026-07-25 20:11:51 +02:00
Lucio Lelii b0c9ef06f9 feat: teach the flow assistant to author LoopContainer via deterministic guard scaffolding
The assistant could author Generic and Iterator containers but not
LoopContainer ("repeat the subflow until a condition is met"). The
guard subflow has a rigid structural contract - it must expose a
non-multiple boolean output named "guard" and a non-multiple text
output named "feedback", produced through SwitchBlocks, reading the
body's latest result via the special ${{outputs.<name>}} reference.
Having the LLM author that freely would be fragile, so the assistant
supplies only the semantic intent and the backend builds the structure.

- AssistantContainerPlan gains guardCondition (natural-language stop/
  continue rule, required for a LoopContainer) and maxIterations;
  validateAndNormalizePlan accepts "LoopContainer" and requires a
  guardCondition when the body is (re)built.
- assembleContainer builds the body subflow like any container, then
  buildLoopGuardSubFlow deterministically assembles the guard subflow:
  a Guard Evaluator LLM (reads ${{outputs.<bodyOutput>}}, answers
  true=continue / false=stop per the guardCondition) -> an expose-guard
  SwitchBlock (boolean guard); a Feedback Builder LLM -> an
  expose-feedback SwitchBlock (text feedback) - mirroring the proven
  guard structure in ExecutionTest. feedbackInput is left for the config
  to infer (the body must expose one open input); the Feedback Builder
  deliberately doesn't reference ${{outputs.x}} to avoid a two-block
  exposed-input collision.
- Prompt: when to prefer LoopContainer, guardCondition/maxIterations,
  and that the body must expose exactly one open input (receives
  feedback each iteration). LoopContainer added to the plan schema.

Regression test draftAuthorsLoopContainerWithDeterministicGuardScaffold:
a one-body-block LoopContainer round-trips to a valid flow whose guard
subflow has the four scaffold blocks; passing validation proves the
scaffold satisfies the LOOP_GUARD/LOOP_BODY contracts. Full suite green
(428). Roadmap: item 2 marked done - all three container types
(Generic/Iterator/Loop) are now authorable.
2026-07-25 19:52:06 +02:00
Lucio Lelii 6142f3fc24 feat: array-valued globals + container-inner globals + within-container MCP sessions
Two related container-scope assistant-correctness gaps (roadmap items 5 and 6).

Item 5 - global input multiplicity and container-inner globals:
- The [] array marker now works symmetrically on globals end to end.
  ExecutionTemplateResolver emits a ${{global.name[]}} substitution key
  alongside ${{global.name}} (as it already did for normal inputs), so an
  array-marked global renders (a list value is newline-joined).
- FlowExecutionValidator strips a trailing [] from an extracted global
  reference name so ${{global.cvs[]}} matches the declared global "cvs".
- collectGlobalInputs scans top-level blocks AND every container's inner
  subflow blocks; a []-marked reference declares the global multiple=true.
  A global referenced only inside a container is now declared both at the
  top level (so the flow collects its value) and in the container subflow's
  own globalInputs (collectGlobalInputsFromBlocks) - the subflow is
  validated as its own scope and, at runtime, the container executor feeds
  it the parent's globals by name. This also fixes a pre-existing gap where
  a global referenced only inside a container failed GLOBAL_INPUT_NOT_DECLARED.
- Prompt: "never write ${{global.name[]}}" replaced with "append [] when the
  global holds a list".

Item 6 - within-container MCP shared sessions:
- Verified via GenericContainerExecutor that a container subflow inherits the
  parent's execution-variable descriptors (which hold the MCP session
  registry) and propagates them back, so an MCP chain inside one container
  genuinely shares state at runtime.
- assembleContainer computes isSharedMemoryContext for the container's inner
  plan and threads it into inner buildBlock / completeRequiredSequential-
  Connections / validateSharedMemorySemantics - the same normalization the
  top level uses, scoped to the subflow (was hardcoded false).
- FlowExecutionValidator.validateSubFlowSharedExecutionVariableOrdering
  validates each container subflow's shared sessions as its own scope, so a
  broken inner chain is caught at validation time.
- Prompt: a shared-session MCP chain must stay within one scope (all
  top-level or all inside the same container), never split across a boundary.

Still out of scope (documented): MCP chains split across a container boundary.

Tests: resolvesGlobalVariableWithArrayMarker (resolver);
draftDeclaresArrayGlobalInputFromArrayMarker,
draftDeclaresGlobalReferencedOnlyInsideAContainerInnerBlock,
draftWiresMcpSharedSessionChainInsideAContainer (assistant). Full suite green
(427). Roadmap doc: items 5 and 6 marked done.
2026-07-25 19:23:53 +02:00
Lucio Lelii 065256fb6c feat: incremental container diffing on REFINE (reuse unchanged inner blocks)
A container was previously either KEEP (reused whole) or ADD/UPDATE
(its entire inner block set regenerated from scratch) - the same waste
and churn risk targeted repair already removed for top-level blocks,
one level down. A REFINE like "add a step inside the review container"
re-authored every existing inner block.

- Extracted reusable cores from the top-level machinery so inner
  assembly shares identical rules: resolveExistingBlock(blockPlan,
  existingBlocks, ...) and preserveConnections(sourceConnections, ...).
- validateAndNormalizePlan resolves the existing container once, then in
  REFINE + UPDATE diffs inner blocks against the existing container's
  subflow (normalizeContainerInnerBlocksAgainstExisting /
  normalizeInnerBlockOperation): unchanged -> KEEP, changed -> UPDATE,
  new -> ADD, dropped -> REMOVE.
- assembleContainer takes the existing container: KEEP inner blocks are
  reused verbatim (no BLOCK_CONFIG call) and existing inner connections
  between surviving blocks are preserved and merged with generated ones.
- Prompt: the inner "operation" field is now documented
  (KEEP/ADD/UPDATE/REMOVE when a container is UPDATEd; omit for
  ADD/DRAFT), replacing the old "do not set operation on inner blocks".

Deliberately scoped to REFINE: FIX and ADD still fully regenerate a
container's inner blocks. FIX on purpose - a container-internal error is
most reliably fixed by rebuilding, and inner-block errors don't reliably
carry the inner block's id for targeted attribution, so incremental FIX
risked "KEEP everything and fix nothing". Left as a possible future
refinement (item 7 in the roadmap).

Along the way normalizeContainerOperation was replaced by
resolveContainerOperation (takes the already-resolved existing
container) so the same match is reused for inner diffing.

Regression test refineReusesUnchangedInnerBlocksWhenUpdatingAContainer:
REFINE a 2-block container to add a third; the mock throws if a KEEP
inner block is ever reconfigured, and the two kept blocks come back by
id with unchanged config and the pre-existing inner connection
preserved. Full suite green apart from the known Loop-container timing
flake (passes in isolation). Roadmap doc: item 3 marked done.
2026-07-25 18:56:30 +02:00
Lucio Lelii 85dcc11039 feat: teach the flow assistant to author IteratorContainer groupings
The assistant could group blocks into a GenericContainer but could not
express "run this subflow once per element of a list" - the per-item
iteration pattern (score each CV, process each document) that had to be
built by hand earlier this session.

- AssistantContainerPlan gains an `iterationInput` field, and
  validateAndNormalizePlan now accepts containerType "IteratorContainer"
  alongside "GenericContainer" (iterationInput is kept only for the
  former, dropped for the latter).
- assembleGenericContainer is generalized into assembleContainer: it
  builds the inner subflow exactly as before, then branches on
  containerType to construct either a GenericContainerConfiguration
  (GenericContainerFactory) or an IteratorContainerConfiguration with
  the iterationInput (IteratorContainerFactory). The factory infers
  iterationInput when the subflow has exactly one open input, and
  IteratorContainerInterfaceResolver auto-promotes the iterated input
  and every exposed output to multiple on the external interface - the
  assistant never declares the array-ness itself.
- The connection-wiring machinery needed no changes: a container is
  already a FlowNode endpoint, so an IteratorContainer slots into the
  same path.
- Prompt updates: the PLAN schema shows an IteratorContainer example
  with iterationInput; rules teach when to prefer it over N duplicated
  blocks, and that the inner block must use a single-value placeholder
  ${{cv}} (never ${{cv[]}}) named by iterationInput. CONNECTIONS rules
  note the iteration input takes the whole array and outputs are arrays.
- A wrong iterationInput is caught by IteratorContainerConfiguration's
  bean-validation on the FlowExecutionValidator pass (wired in earlier
  this session), triggering a repair round rather than shipping broken.

Bug found and fixed along the way: validateAndNormalizePlan treated
"no top-level blocks" as an empty plan and fell back to a single
synthetic LLMBlock, silently discarding every container when a plan put
all its work inside containers. The empty-check now requires both blocks
and containers to be empty before falling back, and the block loop is
null-safe for container-only plans.

Regression test draftAuthorsIteratorContainerForPerItemWork: a
container-only plan round-trips to a valid flow whose IteratorContainer
exposes cv/response as multiple while the inner cv input stays
non-multiple. Full suite green (422). Roadmap doc updated: items 1 and 4
marked done, item 3 (incremental container diffing) is now the top open
gap.
2026-07-25 17:33:34 +02:00
Lucio Lelii ab1def92f1 feat: allow targeted repair on flows that contain containers
isTargetedBlockRepairEligible previously bailed out to a full replan
whenever currentFlow had any containers at all, because
buildReusedPlanForTargetedRepair only reconstructed the top-level block
list - running it on a flow with containers would have silently
dropped them from the rebuilt plan.

buildReusedPlanForTargetedRepair now also reconstructs an
AssistantContainerPlan (operation KEEP, empty inner blocks) for every
container in currentFlow.flow().getContainers(), so containers survive
the targeted-repair path unchanged instead of disappearing. The
eligibility check still requires every error to be scoped to an
existing top-level block - an error scoped to a container (or anything
else) still falls back to a full replan, since a KEEP-only container
plan can't fix a container-internal problem.

Added a regression test: a flow with a container present has a
top-level block's overlong EndBlock.outcomeLabel fixed via a single
targeted BLOCK_CONFIG call (PLAN/CONNECTIONS both skipped), and the
container comes back byte-for-byte identical (same id, same inner
subflow).

Updated docs/assistant-completion-roadmap-2026-07-25.md to mark this
item done and note what's still open (container-internal errors are
still not targeted-fixable - that's the separate, larger "incremental
container diffing" item).
2026-07-25 16:59:15 +02:00