From f94fb883d963a493a4d6595a4a1b5ba8bc111113 Mon Sep 17 00:00:00 2001 From: Lucio Lelii Date: Mon, 3 Aug 2026 15:10:27 +0200 Subject: [PATCH] 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) --- .../assistant/FlowAssistantPromptService.java | 38 +++++++- .../controllers/AssistantControllerTest.java | 90 +++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java index d5ba8a7..79bf72d 100644 --- a/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java +++ b/src/main/java/it/cnr/isti/workflow/manager/assistant/FlowAssistantPromptService.java @@ -8,6 +8,9 @@ import org.springframework.stereotype.Service; import it.cnr.isti.workflow.manager.app.ObjectMapperHolder; import it.cnr.isti.workflow.manager.flows.model.FlowCreateRequest; import it.cnr.isti.workflow.manager.flows.validation.ValidationError; +import tools.jackson.databind.JsonNode; +import tools.jackson.databind.node.ArrayNode; +import tools.jackson.databind.node.ObjectNode; @Service public class FlowAssistantPromptService { @@ -470,7 +473,40 @@ public class FlowAssistantPromptService { if (flow == null || flow.flow() == null) { return "(none)"; } - return toJson(flow); + JsonNode tree = ObjectMapperHolder.mapper.valueToTree(flow); + redactLoopGuardSubFlows(tree); + return tree.toPrettyString(); + } + + /** + * A LoopContainer's guardSubFlow is a rigid scaffold the backend builds deterministically from + * guardCondition (see FlowAssistantService#buildLoopGuardSubFlow) - the assistant never authors + * or edits it. Showing its real block ids/names (e.g. "c1-expose-feedback") in the "Current + * flow" context invites the model to wire connections directly to/from them, which can never + * resolve at the model's scope and is silently dropped - but wastes a repair round doing so and + * can crowd out the legitimate fix the round actually needed. Redact it to a short marker so the + * model never sees it as something to reference or edit. + */ + private void redactLoopGuardSubFlows(JsonNode node) { + if (node == null) { + return; + } + if (node.isObject()) { + ObjectNode object = (ObjectNode) node; + JsonNode specificConfiguration = object.get("specificConfiguration"); + if (specificConfiguration != null && specificConfiguration.isObject() + && specificConfiguration.get("guardSubFlow") != null) { + ((ObjectNode) specificConfiguration).put("guardSubFlow", + "(backend-managed guard mechanism - not shown, do not reference or connect to it)"); + } + for (java.util.Map.Entry entry : object.properties()) { + redactLoopGuardSubFlows(entry.getValue()); + } + } else if (node.isArray()) { + for (JsonNode element : (ArrayNode) node) { + redactLoopGuardSubFlows(element); + } + } } private String summarizeErrors(List errors) { diff --git a/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java b/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java index b089089..49703e3 100644 --- a/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java +++ b/src/test/java/it/cnr/isti/workflow/manager/controllers/AssistantControllerTest.java @@ -2888,6 +2888,96 @@ public class AssistantControllerTest { assertNotEquals("previous_response", configuration.getFeedbackInput()); } + @Test + public void fixPromptRedactsLoopGuardSubFlowInsteadOfExposingItForEditing() { + // Live incident: in FIX mode the model was shown the full "Current flow" JSON, including a + // LoopContainer's guardSubFlow - the deterministic, backend-built scaffold it never authors + // (see buildLoopGuardSubFlow). Seeing real internal block names like "c1-expose-feedback" in + // that context, the model tried to wire connections directly to/from them. Those connections + // can never resolve at the model's scope (they get silently dropped), but the round is wasted + // instead of fixing the actual reported error. The guardSubFlow must be redacted from what + // the model sees. + Answer draftAnswer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of( + "rationale", "Revise the draft repeatedly until it passes review - a loop.", + "plan", java.util.Map.of( + "name", "Iterative revision", + "description", "Revise a draft until it has no issues.", + "blocks", java.util.List.of(), + "containers", java.util.List.of( + java.util.Map.of( + "containerId", "c1", + "containerType", "LoopContainer", + "purpose", "Revise until clean", + "operation", "ADD", + "guardCondition", "stop when the draft has no remaining issues", + "maxIterations", 4, + "blocks", java.util.List.of( + java.util.Map.of("blockId", "c1-b1", "blockType", "LLMBlock", + "purpose", "Revise the draft using feedback"))))))); + } + if (prompt.contains("TASK: BLOCK_CONFIG")) { + return TestAssistantResponses.wrap(java.util.Map.of( + "rationale", "The loop body revises the draft; its single open input receives feedback.", + "block", java.util.Map.of("blockId", "c1-b1", "name", "revise-draft", + "config", java.util.Map.of( + "prompt", "Revise this draft applying the latest feedback: ${{draft}}")))); + } + throw new IllegalStateException("Unexpected assistant prompt:\n" + prompt); + }; + Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(draftAnswer); + Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(draftAnswer); + + AssistantFlowResponse draftResponse = assistantController.draft( + new AssistantGenerationRequest("keep revising the draft until it passes review", MODEL, 1)); + assertTrue(draftResponse.valid(), () -> "Unexpected validation errors: " + draftResponse.validationErrors()); + + java.util.List capturedPrompts = new java.util.concurrent.CopyOnWriteArrayList<>(); + Answer fixAnswer = invocation -> { + String prompt = invocation.getArgument(1, String.class); + capturedPrompts.add(prompt); + if (prompt.contains("TASK: PLAN")) { + return TestAssistantResponses.wrap(java.util.Map.of( + "rationale", "Keep the loop as-is.", + "plan", java.util.Map.of( + "name", "Iterative revision", + "description", "Revise a draft until it has no issues.", + "blocks", java.util.List.of(), + "containers", java.util.List.of( + java.util.Map.of( + "containerId", "c1", + "containerType", "LoopContainer", + "purpose", "Revise until clean", + "operation", "KEEP", + "guardCondition", "stop when the draft has no remaining issues", + "maxIterations", 4, + "blocks", java.util.List.of( + java.util.Map.of("blockId", "c1-b1", "blockType", "LLMBlock", + "purpose", "Revise the draft using feedback", + "operation", "KEEP"))))))); + } + throw new IllegalStateException("Unexpected assistant prompt:\n" + prompt); + }; + Mockito.when(internalOllamaLLMProvider.generate(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(fixAnswer); + Mockito.when(internalOllamaLLMProvider.generateJson(Mockito.eq(MODEL), Mockito.anyString())).thenAnswer(fixAnswer); + + java.util.List errors = java.util.List.of(new ValidationError( + ValidationErrorCode.VALIDATION_ERROR, "flow", null, "connections", "some unrelated issue")); + assistantController.fix(new AssistantFixRequest("no-op fix", draftResponse.flow(), errors, MODEL, 0)); + + assertFalse(capturedPrompts.isEmpty(), "expected at least one prompt to have been sent during fix()"); + for (String prompt : capturedPrompts) { + assertFalse(prompt.contains("c1-expose-feedback"), + "guard subflow internals must not be shown to the model:\n" + prompt); + assertFalse(prompt.contains("c1-guard-evaluator"), + "guard subflow internals must not be shown to the model:\n" + prompt); + assertTrue(prompt.contains("backend-managed guard mechanism"), + "expected the redaction marker in place of the guard subflow:\n" + prompt); + } + } + @Test public void draftWiresMcpChainFromTopLevelProducerToConsumerInsideAContainer() { Answer answer = invocation -> {