From 0c045eaff8f4aaf6072e590303af1d0c3de3a30c Mon Sep 17 00:00:00 2001 From: mateaix <7333791@qq.com> Date: Sun, 13 Sep 2026 22:40:59 +0800 Subject: [PATCH] fix: fence goal evaluations against definition revisions --- .../java/vip/mate/goal/model/GoalEntity.java | 3 + .../mate/goal/model/GoalEvaluationResult.java | 17 +++- .../goal/service/GoalEvaluationService.java | 11 ++- .../mate/goal/service/GoalServiceImpl.java | 36 ++++++++- .../h2/V193__goal_evaluation_revision.sql | 2 + .../V193__goal_evaluation_revision.sql | 2 + .../mysql/V193__goal_evaluation_revision.sql | 2 + .../goal/GoalPersistenceIntegrationTest.java | 79 +++++++++++++++++++ .../service/GoalEvaluationServiceTest.java | 13 +++ .../mate/goal/service/GoalServiceTest.java | 19 +++++ 10 files changed, 176 insertions(+), 8 deletions(-) create mode 100644 mateclaw-server/src/main/resources/db/migration/h2/V193__goal_evaluation_revision.sql create mode 100644 mateclaw-server/src/main/resources/db/migration/kingbase/V193__goal_evaluation_revision.sql create mode 100644 mateclaw-server/src/main/resources/db/migration/mysql/V193__goal_evaluation_revision.sql diff --git a/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEntity.java b/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEntity.java index a1c89e46..07bec384 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEntity.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEntity.java @@ -53,6 +53,9 @@ public class GoalEntity { /** Long-form objective. Always non-null but may be short. */ private String description; + /** Advances on evaluation-definition edits, independently of optimistic-lock/usage version. */ + private long evaluationRevision; + /** LLM-readable exit criteria; evaluator scores against this. Nullable. */ @TableField(value = "exit_criteria", updateStrategy = FieldStrategy.ALWAYS) private String exitCriteria; diff --git a/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEvaluationResult.java b/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEvaluationResult.java index 415e10b7..a6da2b29 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEvaluationResult.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/model/GoalEvaluationResult.java @@ -37,7 +37,22 @@ public record GoalEvaluationResult( int llmCallsConsumed, long latencyMs, List criterionVerdicts, - List bootstrapCriteria) { + List bootstrapCriteria, + long evaluationRevision) { + + /** Compatibility for pre-revision callers: valid only for an unedited definition (revision zero). */ + public GoalEvaluationResult(double score, String gap, String decision, boolean completed, + String evaluatorModel, int llmCallsConsumed, long latencyMs, + List criterionVerdicts, List bootstrapCriteria) { + this(score, gap, decision, completed, evaluatorModel, llmCallsConsumed, latencyMs, + criterionVerdicts, bootstrapCriteria, 0L); + } + + /** Stamp the server-captured revision; the model does not choose this value. */ + public GoalEvaluationResult withEvaluationRevision(long revision) { + return new GoalEvaluationResult(score, gap, decision, completed, evaluatorModel, llmCallsConsumed, + latencyMs, criterionVerdicts, bootstrapCriteria, revision); + } public static final String DECISION_COMPLETED = "completed"; public static final String DECISION_CONTINUE = "continue"; diff --git a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java index 7b3a926f..5c8061e2 100644 --- a/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java +++ b/mateclaw-server/src/main/java/vip/mate/goal/service/GoalEvaluationService.java @@ -122,6 +122,7 @@ public class GoalEvaluationService implements Evaluator { List existing = GoalCriteriaCodec.parse(goal.getCriteria(), objectMapper); boolean bootstrap = existing.isEmpty(); + long evaluationRevision = goal.getEvaluationRevision(); long start = System.currentTimeMillis(); try { @@ -148,16 +149,18 @@ public class GoalEvaluationService implements Evaluator { if (body == null || body.isBlank()) { log.warn("[GoalEvaluation] empty response from evaluator model={}", model.getModelName()); // The call was really spent — bill it. - return GoalEvaluationResult.fallbackAfterCall("empty_response", model.getModelName(), elapsed); + return GoalEvaluationResult.fallbackAfterCall("empty_response", model.getModelName(), elapsed) + .withEvaluationRevision(evaluationRevision); } - return bootstrap + return (bootstrap ? parseBootstrap(body, model.getModelName(), elapsed) - : parseVerdict(body, existing, model.getModelName(), elapsed); + : parseVerdict(body, existing, model.getModelName(), elapsed)) + .withEvaluationRevision(evaluationRevision); } catch (Throwable t) { long elapsed = System.currentTimeMillis() - start; log.warn("[GoalEvaluation] evaluator call failed after {}ms: {}", elapsed, t.toString()); - return GoalEvaluationResult.fallback("call_failed"); + return GoalEvaluationResult.fallback("call_failed").withEvaluationRevision(evaluationRevision); } } 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 b83ecbbe..20758c3c 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 @@ -32,6 +32,7 @@ import java.time.LocalDateTime; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; /** * Default implementation. Concurrency safety relies on: @@ -259,10 +260,32 @@ public class GoalServiceImpl implements GoalService { if (!changed) { return null; // idempotent no-op } + boolean exitsChanged = req.getExitCriteria() != null + && !Objects.equals(req.getExitCriteria(), fresh.getExitCriteria()); + boolean definitionChanged = exitsChanged + || req.getPersistentExecution() != null + && !Objects.equals(req.getPersistentExecution(), Boolean.TRUE.equals(fresh.getPersistentExecution())) + || req.getTitle() != null && !req.getTitle().isBlank() + && !Objects.equals(req.getTitle().trim(), fresh.getTitle()) + || req.getDescription() != null && !Objects.equals(req.getDescription(), fresh.getDescription()) + || req.getSuccessCheckPrompt() != null + && !Objects.equals(req.getSuccessCheckPrompt(), fresh.getSuccessCheckPrompt()); + if (definitionChanged) { + // Replacing the free-text exit definition requires a new draft. + // Other context edits preserve user criterion text but revoke its old verdicts. + String criteria = exitsChanged ? null : GoalCriteriaCodec.serialize( + GoalCriteriaCodec.parse(fresh.getCriteria(), objectMapper).stream() + .map(c -> new GoalCriterion(c.id(), c.text(), false, "")).toList(), objectMapper); + w.set(GoalEntity::getEvaluationRevision, Math.addExact(fresh.getEvaluationRevision(), 1L)) + .set(GoalEntity::getCriteria, criteria) + .set(GoalEntity::getCompletionScore, 0.0) + .set(GoalEntity::getProgressSummary, "Goal definition changed; reevaluation required"); + } bumpVersionAndTime(w); return w; }); - recordAudit("goal.updated", updated, Map.of("by", username)); + recordAudit("goal.updated", updated, Map.of("by", username, + "evaluationRevision", updated.getEvaluationRevision())); return updated; } @@ -363,6 +386,10 @@ public class GoalServiceImpl implements GoalService { } return null; // idempotent } + if (evaluated && result.evaluationRevision() != fresh.getEvaluationRevision()) { + throw new MateClawException("err.goal.completion_not_verified", 409, + "Automatic completion requires the current evaluation definition revision"); + } boolean persistent = Boolean.TRUE.equals(fresh.getPersistentExecution()); List existing = GoalCriteriaCodec.parse(fresh.getCriteria(), objectMapper); // Rechecked against the fresh row on every CAS retry. A stale @@ -471,8 +498,8 @@ public class GoalServiceImpl implements GoalService { .set(GoalEntity::getLastEvaluationAt, LocalDateTime.now()); // Late model results still consume usage, but cannot overwrite a // persistent pause/input boundary established while the call ran. - if (result != null && (!Boolean.TRUE.equals(fresh.getPersistentExecution()) - || fresh.getStatus() == GoalStatus.ACTIVE)) { + if (result != null && result.evaluationRevision() == fresh.getEvaluationRevision() + && (!Boolean.TRUE.equals(fresh.getPersistentExecution()) || fresh.getStatus() == GoalStatus.ACTIVE)) { // Persist the checklist by carrier: bootstrap writes the fresh // draft; verdict merges the per-criterion delta into the // current list (re-read on the locked `fresh` to avoid races). @@ -507,6 +534,9 @@ public class GoalServiceImpl implements GoalService { detail.put("decision", result.decision()); detail.put("evaluatorScore", result.score()); detail.put("evaluatorGap", result.gap()); + detail.put("evaluatedRevision", result.evaluationRevision()); + detail.put("currentEvaluationRevision", g.getEvaluationRevision()); + detail.put("staleEvaluation", result.evaluationRevision() != g.getEvaluationRevision()); detail.put("evaluatorModel", result.evaluatorModel()); detail.put("latencyMs", result.latencyMs()); } diff --git a/mateclaw-server/src/main/resources/db/migration/h2/V193__goal_evaluation_revision.sql b/mateclaw-server/src/main/resources/db/migration/h2/V193__goal_evaluation_revision.sql new file mode 100644 index 00000000..7fb44461 --- /dev/null +++ b/mateclaw-server/src/main/resources/db/migration/h2/V193__goal_evaluation_revision.sql @@ -0,0 +1,2 @@ +-- Independent revision of the goal evaluation definition; usage/version updates do not advance it. +ALTER TABLE mate_agent_goal ADD COLUMN evaluation_revision BIGINT NOT NULL DEFAULT 0; diff --git a/mateclaw-server/src/main/resources/db/migration/kingbase/V193__goal_evaluation_revision.sql b/mateclaw-server/src/main/resources/db/migration/kingbase/V193__goal_evaluation_revision.sql new file mode 100644 index 00000000..7fb44461 --- /dev/null +++ b/mateclaw-server/src/main/resources/db/migration/kingbase/V193__goal_evaluation_revision.sql @@ -0,0 +1,2 @@ +-- Independent revision of the goal evaluation definition; usage/version updates do not advance it. +ALTER TABLE mate_agent_goal ADD COLUMN evaluation_revision BIGINT NOT NULL DEFAULT 0; diff --git a/mateclaw-server/src/main/resources/db/migration/mysql/V193__goal_evaluation_revision.sql b/mateclaw-server/src/main/resources/db/migration/mysql/V193__goal_evaluation_revision.sql new file mode 100644 index 00000000..7fb44461 --- /dev/null +++ b/mateclaw-server/src/main/resources/db/migration/mysql/V193__goal_evaluation_revision.sql @@ -0,0 +1,2 @@ +-- Independent revision of the goal evaluation definition; usage/version updates do not advance it. +ALTER TABLE mate_agent_goal ADD COLUMN evaluation_revision BIGINT NOT NULL DEFAULT 0; diff --git a/mateclaw-server/src/test/java/vip/mate/goal/GoalPersistenceIntegrationTest.java b/mateclaw-server/src/test/java/vip/mate/goal/GoalPersistenceIntegrationTest.java index 75360adb..1f85e420 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/GoalPersistenceIntegrationTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/GoalPersistenceIntegrationTest.java @@ -119,6 +119,85 @@ class GoalPersistenceIntegrationTest { assertThrows(MateClawException.class, () -> goalService.markEvaluatedCompleted(created.getId(), delayed)); } + @Test + void replacingExitCriteriaRevokesOldCompletion() { + GoalEntity created = goalService.create(req("edited-definition-completion", "report"), "alice"); + goalService.appendCriterion(created.getId(), "old report", "alice"); + var passed = new vip.mate.goal.model.GoalEvaluationResult(1.0, "", "completed", true, "fixture", 1, 0, + java.util.List.of(new vip.mate.goal.model.GoalChecklistVerdict.CriterionVerdict( + "C1", true, "old report written")), null); + goalService.recordEvaluation(created.getId(), passed, 1, 1); + var edit = new vip.mate.goal.model.GoalUpdateRequest(); + edit.setExitCriteria("require a different report with an appendix"); + goalService.update(created.getId(), edit, "alice"); + assertThrows(MateClawException.class, () -> goalService.markEvaluatedCompleted(created.getId(), passed)); + assertEquals(0.0, goalService.getById(created.getId()).getCompletionScore()); + } + + @Test + void staleDraftAndVerdictCannotCrossDefinitionRevisionButCurrentOnesCan() { + GoalEntity created = goalService.create(req("definition-revision-carriers", "report"), "alice"); + var oldDraft = new vip.mate.goal.model.GoalEvaluationResult(0.0, "draft", "continue", false, + "fixture", 1, 0, java.util.List.of(), java.util.List.of( + new vip.mate.goal.model.GoalCriterion("C1", "old report", false, ""))); + var edit = new vip.mate.goal.model.GoalUpdateRequest(); edit.setExitCriteria("new report"); + goalService.update(created.getId(), edit, "alice"); + assertEquals(1L, goalService.getById(created.getId()).getEvaluationRevision()); + goalService.recordEvaluation(created.getId(), oldDraft, 1, 1); + assertEquals(null, goalService.getById(created.getId()).getCriteria()); + var newDraft = new vip.mate.goal.model.GoalEvaluationResult(0.0, "draft", "continue", false, + "fixture", 1, 0, java.util.List.of(), java.util.List.of( + new vip.mate.goal.model.GoalCriterion("C1", "new report", false, ""))).withEvaluationRevision(1); + goalService.recordEvaluation(created.getId(), newDraft, 1, 1); + var oldPass = new vip.mate.goal.model.GoalEvaluationResult(1.0, "", "completed", true, "fixture", 1, 0, + java.util.List.of(new vip.mate.goal.model.GoalChecklistVerdict.CriterionVerdict( + "C1", true, "old report evidence")), null); + goalService.recordEvaluation(created.getId(), oldPass, 1, 1); + assertEquals(0.0, goalService.getById(created.getId()).getCompletionScore()); + var currentPass = new vip.mate.goal.model.GoalEvaluationResult(1.0, "", "completed", true, "fixture", 1, 0, + java.util.List.of(new vip.mate.goal.model.GoalChecklistVerdict.CriterionVerdict( + "C1", true, "new report evidence")), null).withEvaluationRevision(1); + goalService.recordEvaluation(created.getId(), currentPass, 1, 1); + // Even after current criteria pass, an old result cannot perform the final transition. + assertThrows(MateClawException.class, () -> goalService.markEvaluatedCompleted(created.getId(), oldPass)); + assertEquals(4, goalService.getById(created.getId()).getEvalLlmCallsUsed()); + assertEquals(GoalStatus.COMPLETED, goalService.markEvaluatedCompleted(created.getId(), currentPass).getStatus()); + } + + @Test + void definitionRevisionSurvivesAbaAndIgnoresIdenticalAndBudgetEdits() { + var request = req("definition-revision-aba", "report"); request.setExitCriteria("A"); + GoalEntity created = goalService.create(request, "alice"); + var edit = new vip.mate.goal.model.GoalUpdateRequest(); edit.setExitCriteria("A"); edit.setTurnBudget(12); + goalService.update(created.getId(), edit, "alice"); + assertEquals(0L, goalService.getById(created.getId()).getEvaluationRevision()); + edit.setExitCriteria("B"); goalService.update(created.getId(), edit, "alice"); + edit.setExitCriteria("A"); goalService.update(created.getId(), edit, "alice"); + assertEquals(2L, goalService.getById(created.getId()).getEvaluationRevision()); + var stale = new vip.mate.goal.model.GoalEvaluationResult(0.0, "draft", "continue", false, + "fixture", 1, 0, java.util.List.of(), java.util.List.of( + new vip.mate.goal.model.GoalCriterion("C1", "A", false, ""))); + goalService.recordEvaluation(created.getId(), stale, 0, 1); + assertEquals(null, goalService.getById(created.getId()).getCriteria()); + } + + @Test + void contextEditPreservesCriterionTextButRevokesPriorPass() { + GoalEntity created = goalService.create(req("definition-context-edit", "report"), "alice"); + goalService.appendCriterion(created.getId(), "user criterion", "alice"); + var passed = new vip.mate.goal.model.GoalEvaluationResult(1.0, "", "completed", true, "fixture", 1, 0, + java.util.List.of(new vip.mate.goal.model.GoalChecklistVerdict.CriterionVerdict( + "C1", true, "old evidence")), null); + goalService.recordEvaluation(created.getId(), passed, 0, 1); + var edit = new vip.mate.goal.model.GoalUpdateRequest(); edit.setDescription("changed context"); + var saved = goalService.update(created.getId(), edit, "alice"); + var criteria = vip.mate.goal.model.GoalCriteriaCodec.parse(saved.getCriteria(), new com.fasterxml.jackson.databind.ObjectMapper()); + assertEquals("user criterion", criteria.getFirst().text()); + org.junit.jupiter.api.Assertions.assertFalse(criteria.getFirst().passed()); + assertEquals("", criteria.getFirst().evidence()); + assertEquals(1L, saved.getEvaluationRevision()); + } + @Test @DisplayName("GoalStatus values persist as lowercase literals — load-bearing for uk_agent_goal_active_conv") void status_persistsAsLowercaseString() { diff --git a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java index 5b6c266b..81297355 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalEvaluationServiceTest.java @@ -108,6 +108,19 @@ class GoalEvaluationServiceTest { assertTrue(result.gap().contains("TLS enabled")); } + @Test + void stampsRevisionCapturedBeforeTheModelCall() { + GoalEntity goal = goalWithCriteria(); goal.setEvaluationRevision(7L); + when(modelConfigService.getDefaultModel()).thenReturn(model("fixture")); + when(chatModelFactory.buildFor(any(ModelConfigEntity.class), any(RetryTemplate.class))).thenReturn(chatModel); + when(chatModel.call(any(Prompt.class))).thenAnswer(call -> { + goal.setEvaluationRevision(8L); + return new ChatResponse(List.of(new Generation(new AssistantMessage( + "{\"criterionVerdicts\":[],\"summary\":\"unchanged\"}")))); + }); + assertEquals(7L, svc.evaluate(goal, List.of(), "answer").evaluationRevision()); + } + // ==================== Pre-flight guards ==================== @Test diff --git a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalServiceTest.java b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalServiceTest.java index 7c550559..a0ff786c 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/service/GoalServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/service/GoalServiceTest.java @@ -690,6 +690,25 @@ class GoalServiceTest { assertTrue(retry.getParamNameValuePairs().containsValue("Still missing: appendix")); } + @Test + void staleEvaluationStopsProjectingAfterCasRevisionChange() { + GoalEntity original = persisted(1L, GoalStatus.ACTIVE); + original.setCriteria("[{\"id\":\"C1\",\"text\":\"report\",\"passed\":false,\"evidence\":\"\"}]"); + GoalEntity fresh = persisted(1L, GoalStatus.ACTIVE); + fresh.setVersion(1); fresh.setEvaluationRevision(1L); fresh.setCriteria(original.getCriteria()); + when(goalMapper.selectById(1L)).thenReturn(original, original, fresh, fresh); + when(goalMapper.update(any(), any(LambdaUpdateWrapper.class))).thenReturn(0, 1); + var result = new GoalEvaluationResult(1.0, "", "completed", true, "fixture", 1, 0, + java.util.List.of(new vip.mate.goal.model.GoalChecklistVerdict.CriterionVerdict("C1", true, "evidence")), null); + service.recordEvaluation(1L, result, 2, 1); + ArgumentCaptor writes = ArgumentCaptor.forClass(LambdaUpdateWrapper.class); + verify(goalMapper, times(2)).update(any(), writes.capture()); + assertTrue(setsProperty(writes.getAllValues().getFirst(), "criteria")); + assertFalse(setsProperty(writes.getAllValues().getLast(), "criteria")); + assertFalse(setsProperty(writes.getAllValues().getLast(), "completionScore")); + assertTrue(writes.getAllValues().getLast().getSqlSet().contains("eval_llm_calls_used = eval_llm_calls_used + 1")); + } + // ==================== criteria checklist ==================== @Test