lelii
/
humainflow-service
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Lucio Lelii 2026-08-03 16:16:31 +02:00
parent e823d8d357
commit 484de60852
2 changed files with 146 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
src/main/java/it/cnr/isti/workflow/manager/flows/FlowService.java
View File

@ -15,11 +15,14 @@ import it.cnr.isti.workflow.manager.flows.repo.FlowEntity;
import it.cnr.isti.workflow.manager.flows.repo.FlowRepository;
import it.cnr.isti.workflow.manager.flows.validation.FlowExecutionValidator;
import it.cnr.isti.workflow.manager.flows.validation.ValidationError;
import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCode;
import it.cnr.isti.workflow.manager.flows.validation.ValidationErrorCodec;
import jakarta.validation.ConstraintViolation;
import jakarta.validation.Validator;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.Set;
import java.util.stream.Collectors;
@Service
public class FlowService {
@ -133,17 +136,85 @@ public class FlowService {
return flowExecutionValidator.collectErrors(entity.getFlow());
}
/**
* Data-integrity errors that genuinely block a save: malformed, internally inconsistent, or
* factory-tampered structure that could not be trusted or safely reloaded. Everything NOT in
* this set is treated as "not yet executable" rather than "corrupt" - see {@link #validateFlow}.
* VALIDATION_ERROR is here because at the save gate it only ever comes from integrity checks
* (null/blank/duplicate global input, null/invalid lane) or a non-decodable request-level
* constraint (e.g. the flow being null); block-config bean violations are NOT cascaded here.
*/
private static final Set<ValidationErrorCode> SAVE_BLOCKING_CODES = EnumSet.of(
ValidationErrorCode.VALIDATION_ERROR,
ValidationErrorCode.FLOW_CONTAINS_NULL_NODE,
ValidationErrorCode.NODE_ID_REQUIRED,
ValidationErrorCode.DUPLICATE_NODE_ID,
ValidationErrorCode.UNSUPPORTED_NODE_TYPE,
ValidationErrorCode.BLOCK_CONFIGURATION_MISSING,
ValidationErrorCode.BLOCK_TYPE_MISMATCH,
ValidationErrorCode.BLOCK_INPUTS_MISMATCH,
ValidationErrorCode.BLOCK_OUTPUTS_MISMATCH,
ValidationErrorCode.CONTAINER_CONFIGURATION_MISSING,
ValidationErrorCode.CONTAINER_TYPE_MISMATCH,
ValidationErrorCode.CONTAINER_INPUTS_MISMATCH,
ValidationErrorCode.CONTAINER_OUTPUTS_MISMATCH,
ValidationErrorCode.NESTED_CONTAINERS_NOT_SUPPORTED,
ValidationErrorCode.DUPLICATE_LANE_ID,
ValidationErrorCode.LANE_NAME_REQUIRED,
ValidationErrorCode.NODE_LANE_NOT_FOUND,
ValidationErrorCode.LANE_ORDER_INVALID,
ValidationErrorCode.BIAS_ANNOTATIONS_NOT_ALLOWED,
ValidationErrorCode.TOO_MANY_BIAS_ANNOTATIONS,
ValidationErrorCode.NULL_BIAS_ANNOTATION,
ValidationErrorCode.DUPLICATE_BIAS_ANNOTATION_ID,
ValidationErrorCode.BIAS_CATEGORY_REQUIRED,
ValidationErrorCode.BIAS_SEVERITY_REQUIRED,
ValidationErrorCode.BIAS_ISSUE_REQUIRED,
ValidationErrorCode.BIAS_FIELD_TOO_LONG,
ValidationErrorCode.BIAS_PROBE_MODE_REQUIRED,
ValidationErrorCode.BIAS_PROBE_INSTRUCTION_REQUIRED,
ValidationErrorCode.BIAS_PROBE_MODE_UNSUPPORTED,
ValidationErrorCode.BIAS_PROBE_TARGET_INPUT_NOT_FOUND,
ValidationErrorCode.BIAS_PROBE_MOCK_OUTPUTS_REQUIRED,
ValidationErrorCode.BIAS_PROBE_MOCK_OUTPUT_NOT_FOUND,
ValidationErrorCode.BIAS_PROBE_MOCK_OUTPUT_TYPE_MISMATCH);
/**
* Permissive save. A flow that is merely NOT YET EXECUTABLE - dangling or absent connections, a
* container subflow that doesn't yet expose the handles it needs, routing/branch/end-node gaps,
* an execution deadlock, ... - must still be storable so it can be iterated in the editor; it is
* simply persisted with status DRAFT (see {@link #toView}). Execution is independently gated by
* {@link FlowExecutionValidator#validate} at run time, so a non-executable draft can never run.
* Only genuine data-integrity problems ({@link #SAVE_BLOCKING_CODES}) reject the save.
*/
private void validateFlow(FlowCreateRequest request) {
Set<ConstraintViolation<FlowCreateRequest>> violations = validator.validate(request);
if (violations.isEmpty()) {
return;
}
String message = violations.stream()
.map(violation -> violation.getPropertyPath() + " " + violation.getMessage())
.collect(Collectors.joining(", "));
logger.warn("Flow validation failed: {}", message);
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, message);
List<ValidationError> blocking = new ArrayList<>();
List<ValidationError> allowedAsDraft = new ArrayList<>();
for (ConstraintViolation<FlowCreateRequest> violation : violations) {
for (ValidationError error : ValidationErrorCodec.decode(violation.getMessage())) {
if (SAVE_BLOCKING_CODES.contains(error.code())) {
blocking.add(error);
} else {
allowedAsDraft.add(error);
}
}
}
if (!blocking.isEmpty()) {
// Keep the reason ValidationErrorCodec-encoded so ApiExceptionHandler surfaces the same
// structured {code, entity, id, field, message} errors[] the clients already consume.
String encoded = ValidationErrorCodec.encode(blocking);
logger.warn("Flow save rejected on data-integrity errors: {}", encoded);
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, encoded);
}
if (!allowedAsDraft.isEmpty()) {
logger.debug("Flow saved as draft despite non-executable validation issues: {}", allowedAsDraft);
}
}
private void ensureMutable(FlowEntity entity) {

83
src/test/java/it/cnr/isti/workflow/manager/controllers/FlowControllerTest.java
View File

@ -818,6 +818,52 @@ public class FlowControllerTest {
assertEquals(FlowViewStatus.DRAFT, createResponse.getBody().status());
}
@Test
public void createFlowWithDanglingConnectionSavesAsDraftInsteadOfRejecting() {
// A connection whose target input name doesn't exist on the target block is a classic
// "not yet executable" state (e.g. a half-rewired flow, or assistant output mid-iteration).
// It must SAVE as a DRAFT so the user can fix it in the editor - not be rejected at save
// time with a 400 - while the execution validator still reports the dangling connection and
// the flow can never actually run until it is fixed.
LLMDescriptor llmDescriptor = LLMDescriptor.builder()
.provider("testProvider")
.model("testModel")
.build();
Block<LLMBlockType> source = blocksController.create(LLMBlockConfiguration.builder()
.name("source")
.llmDescriptor(llmDescriptor)
.prompt("Produce output from ${{seed}}")
.build());
Block<LLMBlockType> target = blocksController.create(LLMBlockConfiguration.builder()
.name("target")
.llmDescriptor(llmDescriptor)
.prompt("Consume ${{expected}}")
.build());
FlowCreateRequest request = new FlowCreateRequest(
"Dangling connection flow",
"The connection points at an input the target block does not have",
FlowData.builder()
.block(source)
.block(target)
.connection(Connection.builder()
.sourceId(source.getId())
.sourceName("response")
.targetId(target.getId())
.targetName("does_not_exist")
.build())
.build());
FlowView created = flowController.createFlow(request, new LoginEntity("testuser", "testpassword")).getBody();
assertNotNull(created);
assertEquals(FlowViewStatus.DRAFT, created.status());
List<ValidationError> validation = flowController
.getFlowValidation(created.id(), new LoginEntity("testuser", "testpassword")).getBody();
assertNotNull(validation);
assertTrue(validation.stream().anyMatch(e -> e.code() == ValidationErrorCode.CONNECTION_TARGET_INPUT_NOT_FOUND),
() -> "expected CONNECTION_TARGET_INPUT_NOT_FOUND among execution errors: " + validation);
}
@Test
public void createEmptyFlowReturnsDraftStatus() {
FlowCreateRequest request = new FlowCreateRequest(
@ -1078,7 +1124,7 @@ public class FlowControllerTest {
}
@Test
public void createFlowRejectsConditionalBranchMerge() {
public void createFlowSavesConditionalBranchMergeAsDraft() {
LLMDescriptor llmDescriptor = LLMDescriptor.builder()
.provider("testProvider")
.model("testModel")
@ -1156,16 +1202,21 @@ public class FlowControllerTest {
.build())
.build());
ResponseStatusException exception = assertThrows(
ResponseStatusException.class,
() -> flowController.createFlow(request, new LoginEntity("testuser", "testpassword")));
assertEquals(HttpStatus.BAD_REQUEST, exception.getStatusCode());
assertTrue(exception.getReason().contains("Exclusive routing branches must not merge"));
// An exclusive-branch merge is a not-yet-executable ROUTING structure, not corrupt data:
// saving is permissive, so it persists as a DRAFT (fixable in the editor) and the merge is
// surfaced by the execution validator - it is no longer rejected outright at save time.
FlowView created = flowController.createFlow(request, new LoginEntity("testuser", "testpassword")).getBody();
assertNotNull(created);
assertEquals(FlowViewStatus.DRAFT, created.status());
List<ValidationError> validation = flowController
.getFlowValidation(created.id(), new LoginEntity("testuser", "testpassword")).getBody();
assertNotNull(validation);
assertTrue(validation.stream().anyMatch(e -> e.code() == ValidationErrorCode.EXCLUSIVE_BRANCH_MERGE),
() -> "expected EXCLUSIVE_BRANCH_MERGE among execution errors: " + validation);
}
@Test
public void createFlowRejectsSwitchBranchMerge() {
public void createFlowSavesSwitchBranchMergeAsDraft() {
LLMDescriptor llmDescriptor = LLMDescriptor.builder()
.provider("testProvider")
.model("testModel")
@ -1244,12 +1295,16 @@ public class FlowControllerTest {
.build())
.build());
ResponseStatusException exception = assertThrows(
ResponseStatusException.class,
() -> flowController.createFlow(request, new LoginEntity("testuser", "testpassword")));
assertEquals(HttpStatus.BAD_REQUEST, exception.getStatusCode());
assertTrue(exception.getReason().contains("Exclusive routing branches must not merge"));
// Same as the conditional case: a switch-branch merge is not-yet-executable routing, saved
// as a DRAFT and reported by the execution validator rather than rejected at save time.
FlowView created = flowController.createFlow(request, new LoginEntity("testuser", "testpassword")).getBody();
assertNotNull(created);
assertEquals(FlowViewStatus.DRAFT, created.status());
List<ValidationError> validation = flowController
.getFlowValidation(created.id(), new LoginEntity("testuser", "testpassword")).getBody();
assertNotNull(validation);
assertTrue(validation.stream().anyMatch(e -> e.code() == ValidationErrorCode.EXCLUSIVE_BRANCH_MERGE),
() -> "expected EXCLUSIVE_BRANCH_MERGE among execution errors: " + validation);
}
@Test