From 47d20b9c9907f45f7bf03840f31bd69744b9842c Mon Sep 17 00:00:00 2001 From: hywznn Date: Thu, 20 Aug 2026 16:36:54 +0900 Subject: [PATCH] fix(task): prevent OCR next-action cycle before approval --- .../action/TaskAvailableActionResolver.java | 31 +++++++--- .../task/RenewalExecutionIntegrationTest.java | 31 +++++++++- .../TaskAvailableActionResolverTest.java | 59 ++++++++++++++++++- 3 files changed, 107 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/fowoco/server/task/application/action/TaskAvailableActionResolver.java b/src/main/java/com/fowoco/server/task/application/action/TaskAvailableActionResolver.java index c4d0e20..91c77b4 100644 --- a/src/main/java/com/fowoco/server/task/application/action/TaskAvailableActionResolver.java +++ b/src/main/java/com/fowoco/server/task/application/action/TaskAvailableActionResolver.java @@ -63,13 +63,6 @@ private TaskActionDecision resolvePreparation(TaskResult result) { TaskAvailableAction.RUN_RENEWAL ); } - if (renewalSupported && renewal.hasMissingSource(DOCUMENT_OCR)) { - return TaskActionDecision.of( - TaskAvailableAction.REVIEW_OCR, - "OCR_REVIEW_REQUIRED", - TaskAvailableAction.REVIEW_OCR - ); - } if (renewalSupported && renewal.hasMissingSource(USER_INPUT)) { return TaskActionDecision.of( TaskAvailableAction.RUN_RENEWAL, @@ -98,6 +91,15 @@ private TaskActionDecision resolvePreparation(TaskResult result) { TaskAvailableAction.REVIEW_WORKER_GUIDE ); } + if (renewalSupported + && renewal.hasMissingSource(DOCUMENT_OCR) + && !renewal.requiresWorkerDocumentCollection()) { + return TaskActionDecision.of( + TaskAvailableAction.REVIEW_OCR, + "OCR_REVIEW_REQUIRED", + TaskAvailableAction.REVIEW_OCR + ); + } List available = new ArrayList<>(); if (renewal.generatedDocumentPresent()) { @@ -124,17 +126,19 @@ private record RenewalProgress( boolean executed, Set missingSlots, Map sourceByField, + String scenario, boolean guideReviewRequired, boolean generatedDocumentPresent ) { private static RenewalProgress from(Map businessData) { Object executionValue = businessData.get("renewal_execution"); if (!(executionValue instanceof Map execution)) { - return new RenewalProgress(false, Set.of(), Map.of(), false, false); + return new RenewalProgress(false, Set.of(), Map.of(), null, false, false); } Set missingSlots = stringSet(execution.get("missing_slots")); Map sources = requestedFieldSources(execution.get("requested_fields")); + String scenario = stringValue(execution.get("scenario")); boolean guideReviewRequired = Boolean.TRUE.equals(execution.get("guide_review_required")); boolean generatedDocumentPresent = execution.get("generated_documents") instanceof List documents && !documents.isEmpty(); @@ -142,6 +146,7 @@ private static RenewalProgress from(Map businessData) { true, missingSlots, sources, + scenario, guideReviewRequired, generatedDocumentPresent ); @@ -151,6 +156,16 @@ private boolean hasMissingSource(String source) { return missingSlots.stream().anyMatch(slot -> source.equals(sourceByField.get(slot))); } + private boolean requiresWorkerDocumentCollection() { + // DOCUMENT_OCR is a future value source in ask_worker. The document can only arrive + // after approval and Worker Link delivery, so it must not block the approval request. + return "ask_worker".equals(scenario); + } + + private static String stringValue(Object value) { + return value instanceof String text && !text.isBlank() ? text : null; + } + private static Set stringSet(Object value) { if (!(value instanceof List values)) { return Set.of(); diff --git a/src/test/java/com/fowoco/server/task/RenewalExecutionIntegrationTest.java b/src/test/java/com/fowoco/server/task/RenewalExecutionIntegrationTest.java index c2c95dd..951160b 100644 --- a/src/test/java/com/fowoco/server/task/RenewalExecutionIntegrationTest.java +++ b/src/test/java/com/fowoco/server/task/RenewalExecutionIntegrationTest.java @@ -213,7 +213,7 @@ void distinguishesAnUnexpectedAgentWorkflow() throws Exception { } @Test - void storesAnAgentWorkerMessageAsAnUnsentDraft() throws Exception { + void storesAnAgentWorkerMessageAndAllowsApprovalBeforeOcrCollection() throws Exception { when(runtimeClient.run(any(), any())).thenAnswer(invocation -> askWorkerResponse(invocation.getArgument(0))); String token = login(HR_A_EMAIL); @@ -236,6 +236,19 @@ void storesAnAgentWorkerMessageAsAnUnsentDraft() throws Exception { "SELECT COUNT(*) FROM audit_event WHERE action = 'DOCUMENT_REQUEST_DRAFT_SAVED'", Integer.class )).isEqualTo(1); + + long taskVersion = ((Number) JsonPath.read(response.body(), "$.task_version")).longValue(); + HttpResponse task = getTask(token); + assertThat(task.statusCode()).isEqualTo(200); + assertThat(JsonPath.read(task.body(), "$.next_action")) + .isEqualTo("REQUEST_APPROVAL"); + assertThat(JsonPath.>read(task.body(), "$.available_actions")) + .containsExactly("REQUEST_APPROVAL"); + + HttpResponse approval = requestApproval(token, taskVersion); + assertThat(approval.statusCode()).isEqualTo(201); + assertThat(JsonPath.read(approval.body(), "$.task_status")) + .isEqualTo("READY_FOR_REVIEW"); } @Test @@ -830,6 +843,10 @@ private HttpResponse postRenewalWithSlots( } private HttpResponse requestApproval(String token) throws Exception { + return requestApproval(token, 0); + } + + private HttpResponse requestApproval(String token, long expectedVersion) throws Exception { HttpRequest request = HttpRequest.newBuilder( uri("/api/v1/tasks/" + TASK_A + "/approval-requests") ) @@ -837,13 +854,21 @@ private HttpResponse requestApproval(String token) throws Exception { .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) .POST(HttpRequest.BodyPublishers.ofString(""" { - "expected_version":0, + "expected_version":%d, "ai_snapshot":{"intent":"EXPIRY_RENEWAL"}, "hr_snapshot":{"worker_id":"%s"}, "changed_fields":[], "source_versions":{"workflow_catalog_version":"0.2.0"} } - """.formatted(WORKER_A))) + """.formatted(expectedVersion, WORKER_A))) + .build(); + return httpClient.send(request, HttpResponse.BodyHandlers.ofString()); + } + + private HttpResponse getTask(String token) throws Exception { + HttpRequest request = HttpRequest.newBuilder(uri("/api/v1/tasks/" + TASK_A)) + .header(HttpHeaders.AUTHORIZATION, "Bearer " + token) + .GET() .build(); return httpClient.send(request, HttpResponse.BodyHandlers.ofString()); } diff --git a/src/test/java/com/fowoco/server/task/application/action/TaskAvailableActionResolverTest.java b/src/test/java/com/fowoco/server/task/application/action/TaskAvailableActionResolverTest.java index cab1ea9..bb9ff1a 100644 --- a/src/test/java/com/fowoco/server/task/application/action/TaskAvailableActionResolverTest.java +++ b/src/test/java/com/fowoco/server/task/application/action/TaskAvailableActionResolverTest.java @@ -41,10 +41,32 @@ void freshSupportedRenewalCanRunAgent() { } @Test - void ocrMissingFieldMustBeReviewedBeforeManualRenewal() { + void workerDocumentCollectionRequiresApprovalBeforeFutureOcrReview() { + TaskActionDecision decision = resolver.resolve(result( + TaskStatus.DRAFT, + renewalExecution( + "ask_worker", + false, + List.of("passport_number"), + List.of(Map.of("key", "passport_number", "source_hint", "DOCUMENT_OCR")), + List.of() + ), + List.of(completedChecklist()), + List.of() + )); + + assertThat(decision.nextAction()).isEqualTo(TaskAvailableAction.REQUEST_APPROVAL); + assertThat(decision.availableActions()).containsExactly(TaskAvailableAction.REQUEST_APPROVAL); + assertThat(decision.blockedReason()).isEqualTo("APPROVAL_REQUIRED_BEFORE_CONTINUATION"); + } + + @Test + void ocrScenarioCanStillRequireReview() { TaskActionDecision decision = resolver.resolve(result( TaskStatus.NEEDS_INFO, renewalExecution( + "ocr", + false, List.of("passport_number"), List.of(Map.of("key", "passport_number", "source_hint", "DOCUMENT_OCR")), List.of() @@ -54,10 +76,30 @@ void ocrMissingFieldMustBeReviewedBeforeManualRenewal() { )); assertThat(decision.nextAction()).isEqualTo(TaskAvailableAction.REVIEW_OCR); - assertThat(decision.availableActions()).doesNotContain(TaskAvailableAction.RUN_RENEWAL); + assertThat(decision.availableActions()).containsExactly(TaskAvailableAction.REVIEW_OCR); assertThat(decision.blockedReason()).isEqualTo("OCR_REVIEW_REQUIRED"); } + @Test + void workerGuideReviewPrecedesApprovalWithFutureOcrFields() { + TaskActionDecision decision = resolver.resolve(result( + TaskStatus.DRAFT, + renewalExecution( + "ask_worker", + true, + List.of("passport_number"), + List.of(Map.of("key", "passport_number", "source_hint", "DOCUMENT_OCR")), + List.of() + ), + List.of(completedChecklist()), + List.of() + )); + + assertThat(decision.nextAction()).isEqualTo(TaskAvailableAction.REVIEW_WORKER_GUIDE); + assertThat(decision.availableActions()).containsExactly(TaskAvailableAction.REVIEW_WORKER_GUIDE); + assertThat(decision.blockedReason()).isEqualTo("WORKER_GUIDE_REVIEW_REQUIRED"); + } + @Test void completedRenewalPreparationRequiresApprovalInsteadOfAnotherRun() { TaskActionDecision decision = resolver.resolve(result( @@ -168,11 +210,22 @@ private Map renewalExecution( List missingSlots, List> requestedFields, List> generatedDocuments + ) { + return renewalExecution("generate", false, missingSlots, requestedFields, generatedDocuments); + } + + private Map renewalExecution( + String scenario, + boolean guideReviewRequired, + List missingSlots, + List> requestedFields, + List> generatedDocuments ) { return Map.of("renewal_execution", Map.of( + "scenario", scenario, "missing_slots", missingSlots, "requested_fields", requestedFields, - "guide_review_required", false, + "guide_review_required", guideReviewRequired, "generated_documents", generatedDocuments )); }