diff --git a/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowExecutionValidator.java b/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowExecutionValidator.java index a938217..b435e3e 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowExecutionValidator.java +++ b/src/main/java/it/cnr/isti/workflow/manager/flows/validation/FlowExecutionValidator.java @@ -157,15 +157,21 @@ public class FlowExecutionValidator { Set subFlowExternalSessions = externalSessionsAvailableTo(container, flowData, externalSessions); - errors.addAll(collectErrors(containerConfiguration.getSubFlow(), subFlowExternalSessions).stream() - .map(error -> new ValidationError( - error.code(), - "container", - container.getId(), - "specificConfiguration.subFlow", - error.message(), - error.relatedNodeIds())) - .toList()); + errors.addAll(remapSubFlowErrors( + collectErrors(containerConfiguration.getSubFlow(), subFlowExternalSessions), + container.getId(), + "specificConfiguration.subFlow")); + if (containerConfiguration instanceof LoopContainerConfiguration loopConfiguration + && loopConfiguration.getGuardSubFlow() != null + && !loopConfiguration.getGuardSubFlow().getNodes().isEmpty()) { + // Explode the guard subflow's own errors too (attributed to this container, + // field guardSubFlow) so the UI can show them per-node like the body's, + // instead of only as a nested JSON blob inside a CONTAINER_SUBFLOW_INVALID. + errors.addAll(remapSubFlowErrors( + collectErrors(loopConfiguration.getGuardSubFlow(), subFlowExternalSessions), + container.getId(), + "specificConfiguration.guardSubFlow")); + } } } } @@ -179,6 +185,39 @@ public class FlowExecutionValidator { return errors; } + /** + * Re-tags every error produced inside a container's subflow so a client can attribute it to the + * enclosing container: entity {@code container}, the container's id, and {@code field} set to the + * subflow it came from ({@code specificConfiguration.subFlow} or {@code ...guardSubFlow}). The + * inner element's own id (the offending block/connection) is preserved into {@code relatedNodeIds} + * - it would otherwise be lost when the id is overwritten by the container id - so the UI can + * still highlight the specific inner node/connection, not just the container. + */ + private List remapSubFlowErrors(List innerErrors, String containerId, + String field) { + return innerErrors.stream() + .map(error -> new ValidationError( + error.code(), + "container", + containerId, + field, + error.message(), + withInnerElementId(error.id(), error.relatedNodeIds()))) + .toList(); + } + + private List withInnerElementId(String innerId, List existing) { + if (innerId == null || innerId.isBlank()) { + return existing; + } + List merged = new ArrayList<>(); + merged.add(innerId); + if (existing != null) { + existing.stream().filter(id -> !merged.contains(id)).forEach(merged::add); + } + return List.copyOf(merged); + } + /** * Shared-MCP-session names available to a container's subflow: those inherited from the * enclosing scope, plus those produced by a top-level block in {@code flowData} that runs diff --git a/src/test/java/it/cnr/isti/workflow/manager/controllers/FlowControllerTest.java b/src/test/java/it/cnr/isti/workflow/manager/controllers/FlowControllerTest.java index 20bd379..5b48f40 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/controllers/FlowControllerTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/controllers/FlowControllerTest.java @@ -864,6 +864,54 @@ public class FlowControllerTest { () -> "expected CONNECTION_TARGET_INPUT_NOT_FOUND among execution errors: " + validation); } + @Test + public void containerSubFlowValidationErrorIsAttributedToContainerAndInnerElement() { + // An error inside a container's subflow must be attributable by a client to (a) the specific + // container and (b) the offending inner element - so the editor can surface it when that + // container is opened and still highlight the inner node/connection, not just "the flow is + // invalid". The inner element's id is preserved in relatedNodeIds even though the error's own + // id is re-tagged to the container. + LLMDescriptor llmDescriptor = LLMDescriptor.builder().provider("testProvider").model("testModel").build(); + Block inner1 = blocksController.create(LLMBlockConfiguration.builder() + .name("Analyze").llmDescriptor(llmDescriptor).prompt("Analyze ${{candidate}}").build()); + Block inner2 = blocksController.create(LLMBlockConfiguration.builder() + .name("Score").llmDescriptor(llmDescriptor).prompt("Score ${{profile}}").build()); + // inner2's real input is "profile"; this connection targets a non-existent one. + Connection danglingInner = Connection.builder() + .sourceId(inner1.getId()).sourceName("response") + .targetId(inner2.getId()).targetName("does_not_exist") + .build(); + Container container = containersController.create( + IteratorContainerConfiguration.builder() + .name("Review") + .subFlow(FlowData.builder().block(inner1).block(inner2).connection(danglingInner).build()) + .build()); + + FlowCreateRequest request = new FlowCreateRequest( + "Container with bad subflow", + "The subflow has a dangling inner connection", + FlowData.builder().container(container).build()); + + FlowView created = flowController.createFlow(request, new LoginEntity("testuser", "testpassword")).getBody(); + assertNotNull(created); + assertEquals(FlowViewStatus.DRAFT, created.status()); + + List validation = flowController + .getFlowValidation(created.id(), new LoginEntity("testuser", "testpassword")).getBody(); + assertNotNull(validation); + ValidationError subFlowError = validation.stream() + .filter(e -> e.code() == ValidationErrorCode.CONNECTION_TARGET_INPUT_NOT_FOUND) + .findFirst() + .orElseThrow(() -> new AssertionError("expected an exploded inner connection error: " + validation)); + // (a) attributed to the specific container and its body subflow... + assertEquals("container", subFlowError.entity()); + assertEquals(container.getId(), subFlowError.id()); + assertEquals("specificConfiguration.subFlow", subFlowError.field()); + // (b) ...while still pinpointing the offending inner connection. + assertTrue(subFlowError.relatedNodeIds().contains(danglingInner.getId()), + () -> "expected inner connection id " + danglingInner.getId() + " in " + subFlowError.relatedNodeIds()); + } + @Test public void createEmptyFlowReturnsDraftStatus() { FlowCreateRequest request = new FlowCreateRequest(