From c5ccce8ebc588955ebc74763cef4113c9a410949 Mon Sep 17 00:00:00 2001 From: matevip Date: Wed, 3 Jun 2026 21:19:26 +0800 Subject: [PATCH] feat(goal): deterministic completion + remaining-criteria followup + auto-followup gates --- .../agent/graph/node/GoalEvaluationNode.java | 4 +- .../vip/mate/goal/config/GoalProperties.java | 25 ++++++-- .../goal/service/GoalFollowupService.java | 63 +++++++++++++++---- .../mate/goal/service/GoalServiceImpl.java | 27 ++++++-- .../mate/tool/builtin/GoalManagementTool.java | 6 +- .../goal/controller/GoalControllerTest.java | 14 ++++- .../goal/service/GoalFollowupServiceTest.java | 24 +++++-- .../tool/builtin/GoalManagementToolTest.java | 9 +-- 8 files changed, 135 insertions(+), 37 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/agent/graph/node/GoalEvaluationNode.java b/mateclaw-server/src/main/java/vip/mate/agent/graph/node/GoalEvaluationNode.java index f3a43207..2f98eb9c 100644 --- a/mateclaw-server/src/main/java/vip/mate/agent/graph/node/GoalEvaluationNode.java +++ b/mateclaw-server/src/main/java/vip/mate/agent/graph/node/GoalEvaluationNode.java @@ -185,7 +185,9 @@ public class GoalEvaluationNode implements NodeAction { // failure on completion) does not propagate into the chat graph // and abort the streamed answer the user already sees. try { - if (result.completed() || result.score() >= 0.95) { + // Completion is the deterministic "all criteria passed" signal the + // evaluator already folded into result.completed() — no score gate. + if (result.completed()) { GoalEntity completed = goalService.markCompleted(refreshed.getId(), result); return MateClawStateAccessor.output() .goalEvaluationResult(result.toMap()) diff --git a/mateclaw-server/src/main/java/vip/mate/goal/config/GoalProperties.java b/mateclaw-server/src/main/java/vip/mate/goal/config/GoalProperties.java index 4e3d15b8..9c42da46 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/config/GoalProperties.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/config/GoalProperties.java @@ -7,10 +7,9 @@ import org.springframework.stereotype.Component; /** * Configuration knobs for the persistent-goal subsystem. * - *

{@link #enabled} is the master gate: when {@code false} (PR1-4 default) - * the StateGraph wiring stays inactive and {@code findActiveByConversation} - * still works for tests, but no graph node touches the table. PR5 flips it - * to {@code true}. + *

{@link #enabled} is the master gate: when {@code false} the StateGraph + * wiring stays inactive (no graph node touches the table), while + * {@code findActiveByConversation} still works for tests. */ @Data @Component @@ -20,12 +19,26 @@ public class GoalProperties { /** * Master switch — when off, the graph never invokes GoalEvaluationNode * (the conditional edge sees no active goal, so the node is unreachable). - * Defaults to true now that the full PR1-5 chain is in place; operators - * who want to disable goal evaluation can override via + * Operators who want to disable goal evaluation entirely can override via * {@code mateclaw.goal.enabled=false} in application.yml. */ private boolean enabled = true; + /** + * Create-time default for a goal's {@code autoFollowupEnabled} when the + * caller leaves it unspecified (null). Explicit true/false in the request + * is never overridden by this. + */ + private boolean defaultAutoFollowup = true; + + /** + * Runtime hard gate for auto-followup. When false, no goal injects a + * follow-up regardless of its per-goal {@code autoFollowupEnabled} flag — + * the operator's kill switch for the self-continuation loop that takes + * effect immediately, even for goals created with the flag on. + */ + private boolean allowAutoFollowup = true; + /** Default turn budget when the user doesn't override. */ private int defaultTurnBudget = 20; diff --git a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalFollowupService.java b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalFollowupService.java index 245fa879..9410403c 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalFollowupService.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalFollowupService.java @@ -1,38 +1,53 @@ package vip.mate.goal.service; +import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; +import vip.mate.goal.config.GoalProperties; +import vip.mate.goal.model.GoalCriteriaCodec; +import vip.mate.goal.model.GoalCriterion; import vip.mate.goal.model.GoalEntity; import vip.mate.goal.model.GoalEvaluationResult; import java.time.Duration; import java.time.LocalDateTime; +import java.util.List; import java.util.Optional; /** - * Decides whether to inject a follow-up user prompt for the next graph - * pass. PR2 wires the plumbing; the actual "yes, continue" path defaults - * to off until PR5 flips {@code mateclaw.goal.enabled=true} and operators - * opt their goals in via {@code auto_followup_enabled}. + * Decides whether to inject a follow-up user prompt for the next graph pass, + * driving the autonomous "continue until the checklist is complete" loop. */ @Slf4j @Service public class GoalFollowupService { + private final GoalProperties properties; + private final ObjectMapper objectMapper; + + public GoalFollowupService(GoalProperties properties, ObjectMapper objectMapper) { + this.properties = properties; + this.objectMapper = objectMapper; + } + /** - * Build the follow-up prompt to inject, or empty when no follow-up - * should fire this turn. Conditions follow RFC 48 §3.10: + * Build the follow-up prompt to inject, or empty when no follow-up should + * fire this turn. Gating order: *

    - *
  1. {@code autoFollowupEnabled} is true.
    2. + *
  2. {@code allow-auto-followup} runtime hard gate (operator kill + * switch; overrides per-goal flag).
    3. + *
  3. Per-goal {@code autoFollowupEnabled}.
    4. *
  4. Evaluator decision is "continue" with score < 0.95.
    5. *
  5. Cooldown since the last follow-up has elapsed.
    6. *
  6. turn_budget has at least one slot left after this turn.
    7. - *
  7. (agent + eval) LLM calls below 90 % of llm_call_budget.
    8. + *
  8. (agent + eval) LLM calls below 90% of llm_call_budget.
    9. *
*/ public Optional maybeBuildFollowup(GoalEntity goal, - GoalEvaluationResult result) { + GoalEvaluationResult result) { if (goal == null || result == null) return Optional.empty(); + // Runtime hard gate first — overrides any per-goal flag. + if (!properties.isAllowAutoFollowup()) return Optional.empty(); if (!Boolean.TRUE.equals(goal.getAutoFollowupEnabled())) return Optional.empty(); if (!GoalEvaluationResult.DECISION_CONTINUE.equals(result.decision())) { return Optional.empty(); @@ -51,17 +66,39 @@ public class GoalFollowupService { int turnsUsed = goal.getTurnsUsed() != null ? goal.getTurnsUsed() : 0; int turnBudget = goal.getTurnBudget() != null ? goal.getTurnBudget() : Integer.MAX_VALUE; - // Leave at least one turn slot for the real user — refuse to burn - // the final slot on an auto-followup that the user can't watch. + // Leave at least one turn slot for the real user — refuse to burn the + // final slot on an auto-followup the user can't watch. if (turnsUsed >= turnBudget - 1) return Optional.empty(); int callBudget = goal.getLlmCallBudget() != null ? goal.getLlmCallBudget() : Integer.MAX_VALUE; if (goal.totalLlmCallsUsed() >= (int) (callBudget * 0.9)) return Optional.empty(); + return Optional.of(buildPrompt(goal, result)); + } + + /** + * Prefer a concrete remaining-criteria list when the goal has a checklist; + * fall back to the free-text gap otherwise. Both end with the same "take + * the next concrete step" instruction. + */ + private String buildPrompt(GoalEntity goal, GoalEvaluationResult result) { + List all = GoalCriteriaCodec.parse(goal.getCriteria(), objectMapper); + List remaining = GoalCriteriaCodec.remaining(all); + if (!remaining.isEmpty()) { + int total = all.size(); + int passed = total - remaining.size(); + StringBuilder sb = new StringBuilder(); + sb.append("Continue working toward the goal. ") + .append(passed).append('/').append(total).append(" criteria passed. Remaining:\n"); + for (GoalCriterion c : remaining) { + sb.append(" - ").append(c.text()).append('\n'); + } + sb.append("Take the next concrete step on the remaining criteria."); + return sb.toString(); + } String gap = result.gap(); if (gap == null || gap.isBlank()) gap = "the goal is not yet complete."; - String prompt = "Continue working on the goal. Still missing: " + gap + return "Continue working on the goal. Still missing: " + gap + "\nTake the next concrete step."; - return Optional.of(prompt); } } diff --git a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalServiceImpl.java b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalServiceImpl.java index 5684c663..3d989d11 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalServiceImpl.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalServiceImpl.java @@ -115,7 +115,11 @@ public class GoalServiceImpl implements GoalService { ? req.getLlmCallBudget() : properties.getDefaultLlmCallBudget()); entity.setAgentLlmCallsUsed(0); entity.setEvalLlmCallsUsed(0); - entity.setAutoFollowupEnabled(Boolean.TRUE.equals(req.getAutoFollowupEnabled())); + // Three-state default: explicit true/false is honored; null falls + // back to the configured create-time default. + entity.setAutoFollowupEnabled(req.getAutoFollowupEnabled() != null + ? req.getAutoFollowupEnabled() + : properties.isDefaultAutoFollowup()); entity.setFollowupCooldownSeconds(req.getFollowupCooldownSeconds() != null ? req.getFollowupCooldownSeconds() : properties.getAutoFollowupCooldownSeconds()); // Normalize any caller-supplied checklist: assign C1..Cn, force @@ -467,19 +471,32 @@ public class GoalServiceImpl implements GoalService { throw new MateClawException("err.goal.criterion_empty", 400, "Criterion must not be empty"); } String trimmed = criterion.trim(); - // Merge against the freshly refetched criteria so a concurrent - // addCriterion never silently overwrites a sibling's append. + // Double-write: append a structured criterion (authoritative) and + // mirror the text into exit_criteria for backward compatibility / + // human readability. New id is the current max ordinal + 1. Merge + // against the freshly refetched row so concurrent appends don't clobber. GoalEntity g = retryOptimistic(id, "appendCriterion", fresh -> { ensureNotTerminal(fresh, "appendCriterion"); + + List list = GoalCriteriaCodec.parse(fresh.getCriteria(), objectMapper); + list.add(new GoalCriterion("C" + (list.size() + 1), trimmed, false, "")); + String criteriaJson = GoalCriteriaCodec.serialize(GoalCriteriaCodec.reindex(list), objectMapper); + String existing = fresh.getExitCriteria() != null ? fresh.getExitCriteria() : ""; - String merged = existing.isEmpty() ? trimmed : existing + "\n+ " + trimmed; + String mergedText = existing.isEmpty() ? trimmed : existing + "\n+ " + trimmed; + LambdaUpdateWrapper w = baseLockedUpdate(fresh) - .set(GoalEntity::getExitCriteria, merged); + .set(GoalEntity::getCriteria, criteriaJson) + .set(GoalEntity::getExitCriteria, mergedText); bumpVersionAndTime(w); return w; }); + List full = GoalCriteriaCodec.parse(g.getCriteria(), objectMapper); + String criterionId = full.isEmpty() ? "" : full.get(full.size() - 1).id(); writeEvent(id, GoalEventType.CRITERION_ADDED, null, Map.of( "criterion", trimmed, + "criterionId", criterionId, + "criteria", full, "by", username)); return g; } diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/GoalManagementTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/GoalManagementTool.java index ea3e88d5..06a796e1 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/GoalManagementTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/GoalManagementTool.java @@ -58,7 +58,8 @@ public class GoalManagementTool { required = false) String exitCriteria, @ToolParam(description = "Max evaluation turns before exhaustion. Default 20.", required = false) Integer turnBudget, - @ToolParam(description = "If true, the agent may auto-followup when progress is incomplete. Default false.", + @ToolParam(description = "If true, the agent may auto-followup when progress is incomplete. " + + "Omit to use the system default.", required = false) Boolean autoFollowup, @ToolParam(description = "Optional initial checklist: a list of short, individually verifiable " + "acceptance criteria. Omit to let the system derive the checklist on first evaluation.", @@ -168,7 +169,8 @@ public class GoalManagementTool { if (streamTracker != null && completed.getConversationId() != null) { streamTracker.broadcastObject(completed.getConversationId(), "goal_completed", Map.of( "goalId", String.valueOf(completed.getId()), - "score", synthetic.score())); + "score", synthetic.score(), + "goal", goalService.toResponse(completed))); } return successJson(Map.of( "goalId", String.valueOf(completed.getId()), diff --git a/mateclaw-server/src/test/java/vip/mate/goal/controller/GoalControllerTest.java b/mateclaw-server/src/test/java/vip/mate/goal/controller/GoalControllerTest.java index f08fcc83..c8df2206 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/controller/GoalControllerTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/controller/GoalControllerTest.java @@ -10,6 +10,7 @@ import vip.mate.common.result.R; import vip.mate.exception.MateClawException; import vip.mate.goal.model.GoalCreateRequest; import vip.mate.goal.model.GoalEntity; +import vip.mate.goal.model.GoalResponse; import vip.mate.goal.model.GoalStatus; import vip.mate.goal.model.GoalUpdateRequest; import vip.mate.goal.service.GoalService; @@ -64,6 +65,13 @@ class GoalControllerTest { return g; } + private GoalResponse resp(Long id, GoalStatus status) { + GoalResponse r = new GoalResponse(); + r.setId(id); + r.setStatus(status); + return r; + } + private GoalCreateRequest req(String convId) { GoalCreateRequest r = new GoalCreateRequest(); r.setConversationId(convId); @@ -90,7 +98,8 @@ class GoalControllerTest { when(conversationService.findByConversationId("conv-1")).thenReturn(conv("conv-1", 10L, 1L)); when(goalService.create(any(), eq("alice"))) .thenReturn(goal(1L, "conv-1", GoalStatus.ACTIVE)); - R result = controller.create(req("conv-1"), auth); + when(goalService.toResponse(any())).thenReturn(resp(1L, GoalStatus.ACTIVE)); + R result = controller.create(req("conv-1"), auth); assertNotNull(result.getData()); assertEquals(1L, result.getData().getId()); } @@ -177,8 +186,9 @@ class GoalControllerTest { when(goalService.getById(1L)).thenReturn(g); when(conversationService.isConversationOwner("conv-1", "alice")).thenReturn(true); when(goalService.pause(1L, "alice")).thenReturn(goal(1L, "conv-1", GoalStatus.PAUSED)); + when(goalService.toResponse(any())).thenReturn(resp(1L, GoalStatus.PAUSED)); - R result = controller.pause(1L, auth); + R result = controller.pause(1L, auth); assertEquals(GoalStatus.PAUSED, result.getData().getStatus()); } diff --git a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalFollowupServiceTest.java b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalFollowupServiceTest.java index 530ddb66..e9c9c507 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalFollowupServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalFollowupServiceTest.java @@ -1,6 +1,8 @@ package vip.mate.goal.service; +import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; +import vip.mate.goal.config.GoalProperties; import vip.mate.goal.model.GoalEntity; import vip.mate.goal.model.GoalEvaluationResult; import vip.mate.goal.model.GoalStatus; @@ -8,16 +10,16 @@ import vip.mate.goal.model.GoalStatus; import java.time.LocalDateTime; import java.util.Optional; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; /** - * Covers the five follow-up gating conditions from RFC 48 §3.10. Every - * negative case must independently block the follow-up. + * Covers the follow-up gating conditions. Every negative case must + * independently block the follow-up. */ class GoalFollowupServiceTest { - private final GoalFollowupService svc = new GoalFollowupService(); + private final GoalProperties properties = new GoalProperties(); + private final GoalFollowupService svc = new GoalFollowupService(properties, new ObjectMapper()); private GoalEntity goal(boolean autoEnabled) { GoalEntity g = new GoalEntity(); @@ -50,6 +52,20 @@ class GoalFollowupServiceTest { assertTrue(out.isEmpty()); } + @Test + void allowAutoFollowupGate_overridesPerGoalFlag() { + properties.setAllowAutoFollowup(false); + try { + // per-goal flag on + budget healthy, yet the runtime hard gate wins. + Optional out = svc.maybeBuildFollowup( + goal(true), + res(0.6, GoalEvaluationResult.DECISION_CONTINUE)); + assertTrue(out.isEmpty()); + } finally { + properties.setAllowAutoFollowup(true); + } + } + @Test void completedDecision_returnsEmpty() { Optional out = svc.maybeBuildFollowup( diff --git a/mateclaw-server/src/test/java/vip/mate/tool/builtin/GoalManagementToolTest.java b/mateclaw-server/src/test/java/vip/mate/tool/builtin/GoalManagementToolTest.java index 800ab397..d98653d6 100644 --- a/mateclaw-server/src/test/java/vip/mate/tool/builtin/GoalManagementToolTest.java +++ b/mateclaw-server/src/test/java/vip/mate/tool/builtin/GoalManagementToolTest.java @@ -75,7 +75,7 @@ class GoalManagementToolTest { @Test void setGoal_disabledFlag_returnsError() { properties.setEnabled(false); - String result = tool.setGoal("title", null, null, null, null, + String result = tool.setGoal("title", null, null, null, null, null, ctxWith("conv-1", 10L, "alice")); assertTrue(result.contains("disabled")); verify(goalService, never()).create(any(), anyString()); @@ -83,7 +83,7 @@ class GoalManagementToolTest { @Test void setGoal_blankTitle_returnsError() { - String result = tool.setGoal(" ", null, null, null, null, + String result = tool.setGoal(" ", null, null, null, null, null, ctxWith("conv-1", 10L, "alice")); assertTrue(result.contains("title is required")); } @@ -95,7 +95,7 @@ class GoalManagementToolTest { String result = tool.setGoal("ship the blog", "deploy to fly.io", "tests pass + deployed", - 15, true, + 15, true, null, ctxWith("conv-1", 10L, "alice")); assertTrue(result.contains("\"goalId\":\"123\"")); assertTrue(result.contains("\"status\":\"active\"")); @@ -103,7 +103,7 @@ class GoalManagementToolTest { @Test void setGoal_missingConversationContext_returnsError() { - String result = tool.setGoal("title", null, null, null, null, null); + String result = tool.setGoal("title", null, null, null, null, null, null); assertTrue(result.contains("requires a bound conversation")); } @@ -151,6 +151,7 @@ class GoalManagementToolTest { GoalEntity completed = goal(GoalStatus.COMPLETED); when(goalService.markCompleted(eq(123L), any(GoalEvaluationResult.class))) .thenReturn(completed); + when(goalService.toResponse(any())).thenReturn(new vip.mate.goal.model.GoalResponse()); String result = tool.completeGoal(ctxWith("conv-1", 10L, "alice")); assertTrue(result.contains("\"status\":\"completed\"")); }