fix: avoid completion side effects on goal no-op transitions

This commit is contained in:
mateaix 2026-09-13 23:10:45 +08:00
parent 8cc7eaaae1
commit 542f075759
3 changed files with 55 additions and 1 deletions

View File

@ -378,7 +378,10 @@ public class GoalServiceImpl implements GoalService {
}
private GoalEntity completeGoal(Long id, GoalEvaluationResult result, boolean evaluated) {
boolean[] transitioned = {false};
GoalEntity g = retryOptimistic(id, "markCompleted", fresh -> {
// A failed CAS may retry against another worker's completed row.
transitioned[0] = false;
if (fresh.getStatus().isTerminal()) {
if (evaluated && fresh.getStatus() != GoalStatus.COMPLETED) {
throw new MateClawException("err.goal.completion_not_verified", 409,
@ -416,8 +419,10 @@ public class GoalServiceImpl implements GoalService {
w.set(GoalEntity::getCriteria, GoalCriteriaCodec.serialize(allPassed, objectMapper));
}
bumpVersionAndTime(w);
transitioned[0] = true;
return w;
});
if (!transitioned[0]) return g;
Map<String, Object> detail = new LinkedHashMap<>();
detail.put("finalScore", result != null ? result.score() : null);
detail.put("agentLlmCallsUsed", g.getAgentLlmCallsUsed());

View File

@ -4,6 +4,8 @@ import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import vip.mate.memory.spi.MemoryManager;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.dao.DuplicateKeyException;
import org.springframework.jdbc.core.JdbcTemplate;
@ -57,10 +59,12 @@ import static org.junit.jupiter.api.Assertions.fail;
"spring.datasource.url=jdbc:h2:mem:goal_persistence_${random.uuid};MODE=MySQL;DATABASE_TO_LOWER=TRUE;CASE_INSENSITIVE_IDENTIFIERS=TRUE;DB_CLOSE_DELAY=-1",
"spring.ai.dashscope.api-key=test-key",
"spring.main.web-application-type=none",
"mateclaw.goal.enabled=false"
"mateclaw.goal.enabled=false", "mateclaw.plugin.enabled=false", "mateclaw.skill.workspace.auto-init=false",
"mateclaw.skill.workspace.root=${java.io.tmpdir}/mateclaw-goal-persistence-skills-${random.uuid}"
})
class GoalPersistenceIntegrationTest {
@MockBean private MemoryManager memory;
@Autowired private GoalService goalService;
@Autowired private JdbcTemplate jdbc;
@Autowired private GoalContinuationStore continuations;
@ -198,6 +202,33 @@ class GoalPersistenceIntegrationTest {
assertEquals(1L, saved.getEvaluationRevision());
}
@Test
void repeatedCompletionWritesOneEventAndSyncsMemoryOnce() {
GoalEntity goal = goalService.create(req("completion-event-idempotence", "report"), "alice");
goalService.appendCriterion(goal.getId(), "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, "report evidence")), null);
goalService.recordEvaluation(goal.getId(), passed, 0, 1);
goalService.markEvaluatedCompleted(goal.getId(), passed);
goalService.markEvaluatedCompleted(goal.getId(), passed);
goalService.markCompleted(goal.getId(), null);
assertEquals(1L, goalService.listEvents(goal.getId(), 30).stream()
.filter(event -> "completed".equals(event.getEventType())).count());
org.mockito.Mockito.verify(memory, org.mockito.Mockito.times(1)).syncAll(
org.mockito.ArgumentMatchers.eq(1L), org.mockito.ArgumentMatchers.eq(goal.getConversationId()),
org.mockito.ArgumentMatchers.anyString(), org.mockito.ArgumentMatchers.anyString());
}
@Test
void abandonedGoalCannotBeRecordedAsCompletedByAnIdempotentCall() {
GoalEntity goal = goalService.create(req("abandoned-no-completion-event", "report"), "alice");
goalService.abandon(goal.getId(), "alice");
assertEquals(GoalStatus.ABANDONED, goalService.markCompleted(goal.getId(), null).getStatus());
assertEquals(0L, goalService.listEvents(goal.getId(), 30).stream()
.filter(event -> "completed".equals(event.getEventType())).count());
org.mockito.Mockito.verifyNoInteractions(memory);
}
@Test
@DisplayName("GoalStatus values persist as lowercase literals — load-bearing for uk_agent_goal_active_conv")
void status_persistsAsLowercaseString() {

View File

@ -37,6 +37,8 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@ -514,11 +516,27 @@ class GoalServiceTest {
@Test
void markCompleted_isIdempotent_onTerminal() {
var memory = mock(vip.mate.memory.spi.MemoryManager.class);
service.setMemoryManager(memory);
GoalEntity g = persisted(1L, GoalStatus.COMPLETED);
when(goalMapper.selectById(1L)).thenReturn(g);
GoalEntity result = service.markCompleted(1L, null);
assertEquals(GoalStatus.COMPLETED, result.getStatus());
verify(goalMapper, never()).update(any(), any(LambdaUpdateWrapper.class));
verifyNoInteractions(eventMapper, auditEventService, memory);
}
@Test
void completionCasLoserDoesNotRepeatWinnerSideEffects() {
var memory = mock(vip.mate.memory.spi.MemoryManager.class);
service.setMemoryManager(memory);
GoalEntity active = persisted(1L, GoalStatus.ACTIVE);
GoalEntity completed = statusFlipped(active, GoalStatus.COMPLETED);
when(goalMapper.selectById(1L)).thenReturn(active, completed);
when(goalMapper.update(any(), any(LambdaUpdateWrapper.class))).thenReturn(0);
assertEquals(GoalStatus.COMPLETED, service.markCompleted(1L, null).getStatus());
verify(goalMapper, times(1)).update(any(), any(LambdaUpdateWrapper.class));
verifyNoInteractions(eventMapper, auditEventService, memory);
}
@Test