fix(goal): sync completion memory after transaction commit

This commit is contained in:
mateaix 2026-09-13 23:39:45 +08:00
parent 542f075759
commit 52a2843bdf
2 changed files with 107 additions and 13 deletions

View File

@ -11,6 +11,11 @@ import org.springframework.dao.DuplicateKeyException;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.transaction.TransactionDefinition;
import org.springframework.transaction.support.TransactionSynchronization;
import org.springframework.transaction.support.TransactionSynchronizationManager;
import org.springframework.transaction.support.TransactionTemplate;
import vip.mate.audit.service.AuditEventService;
import vip.mate.exception.MateClawException;
import vip.mate.goal.config.GoalProperties;
@ -57,6 +62,7 @@ public class GoalServiceImpl implements GoalService {
private final AuditEventService auditEventService;
private final ObjectMapper objectMapper;
private ApplicationEventPublisher applicationEventPublisher;
private PlatformTransactionManager transactionManager;
/**
* Optional only set when the memory subsystem is wired. On goal
@ -89,6 +95,11 @@ public class GoalServiceImpl implements GoalService {
this.applicationEventPublisher = publisher;
}
@Autowired(required = false)
public void setTransactionManager(PlatformTransactionManager manager) {
this.transactionManager = manager;
}
// ==================== CRUD ====================
@Override
@ -431,24 +442,45 @@ public class GoalServiceImpl implements GoalService {
writeEvent(id, GoalEventType.COMPLETED, null, detail);
recordAudit("goal.completed", g, detail);
// Forward to long-term memory on completion. Best-effort: a failing
// memory pipeline must not roll back the DB transition.
if (memoryManager != null) {
syncCompletionMemoryAfterCommit(g, result);
return g;
}
private void syncCompletionMemoryAfterCommit(GoalEntity goal, GoalEvaluationResult result) {
var target = memoryManager;
if (target == null) return;
// Snapshot values before returning the mutable entity to the caller.
Long agentId = goal.getAgentId();
String conversationId = goal.getConversationId();
String subject = "[goal completed] " + goal.getTitle();
String summary = goal.getProgressSummary() != null && !goal.getProgressSummary().isBlank()
? goal.getProgressSummary() : "Final score: " + (result != null ? result.score() : "");
Runnable sync = () -> {
try {
String summary = g.getProgressSummary() != null && !g.getProgressSummary().isBlank()
? g.getProgressSummary()
: "Final score: " + (result != null ? result.score() : "");
memoryManager.syncAll(
g.getAgentId(),
g.getConversationId(),
"[goal completed] " + g.getTitle(),
summary);
if (transactionManager != null) {
// afterCommit still has the old transaction's resources bound.
// Adapter DB writes need their own transaction to commit reliably.
TransactionTemplate independent = new TransactionTemplate(transactionManager);
independent.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
independent.executeWithoutResult(status -> target.syncAll(agentId, conversationId, subject, summary));
} else {
target.syncAll(agentId, conversationId, subject, summary);
}
} catch (Exception e) {
log.debug("[GoalService] memory syncAll on goal completion failed: {}", e.getMessage());
}
};
if (TransactionSynchronizationManager.isActualTransactionActive()) {
if (TransactionSynchronizationManager.isSynchronizationActive()) {
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
@Override public void afterCommit() { sync.run(); }
});
} else {
log.debug("[GoalService] skipped completion memory: no commit synchronization available");
}
} else {
sync.run(); // Direct/non-transactional callers retain best-effort behavior.
}
return g;
}
@Override

View File

@ -229,6 +229,68 @@ class GoalPersistenceIntegrationTest {
org.mockito.Mockito.verifyNoInteractions(memory);
}
private GoalEntity readyForCompletion(String conversation, String title) {
GoalEntity goal = goalService.create(req(conversation, title), "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);
return goalService.getById(goal.getId());
}
@Test
void rolledBackCompletionDoesNotSyncMemory() {
GoalEntity goal = readyForCompletion("completion-memory-rollback", "report");
new TransactionTemplate(transactionManager).executeWithoutResult(status -> {
goalService.markCompleted(goal.getId(), null);
status.setRollbackOnly();
});
assertEquals(GoalStatus.ACTIVE, goalService.getById(goal.getId()).getStatus());
assertEquals(0L, goalService.listEvents(goal.getId(), 30).stream()
.filter(event -> "completed".equals(event.getEventType())).count());
org.mockito.Mockito.verifyNoInteractions(memory);
}
@Test
void completionMemoryRunsAfterCommitAndItsDatabaseWritesCommitIndependently() {
GoalEntity goal = readyForCompletion("completion-memory-after-commit", "original title");
jdbc.execute("CREATE TABLE IF NOT EXISTS goal_memory_callback_probe(goal_id BIGINT PRIMARY KEY)");
org.mockito.Mockito.doAnswer(call -> {
inIndependentTransaction(() -> assertEquals(GoalStatus.COMPLETED,
goalService.getById(goal.getId()).getStatus()));
assertEquals("[goal completed] original title", call.getArgument(2));
jdbc.update("INSERT INTO goal_memory_callback_probe(goal_id) VALUES(?)", goal.getId());
return null;
}).when(memory).syncAll(org.mockito.ArgumentMatchers.anyLong(), org.mockito.ArgumentMatchers.anyString(),
org.mockito.ArgumentMatchers.anyString(), org.mockito.ArgumentMatchers.anyString());
new TransactionTemplate(transactionManager).executeWithoutResult(status -> {
GoalEntity returned = goalService.markCompleted(goal.getId(), null);
returned.setTitle("mutated after return");
org.mockito.Mockito.verifyNoInteractions(memory);
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
@Override public void afterCommit() {
inIndependentTransaction(() -> assertEquals(1, jdbc.queryForObject(
"SELECT COUNT(*) FROM goal_memory_callback_probe WHERE goal_id=?", Integer.class, goal.getId())));
}
});
});
org.mockito.Mockito.verify(memory, org.mockito.Mockito.times(1)).syncAll(
org.mockito.ArgumentMatchers.anyLong(), org.mockito.ArgumentMatchers.anyString(),
org.mockito.ArgumentMatchers.anyString(), org.mockito.ArgumentMatchers.anyString());
}
@Test
void memoryFailureCannotUndoCommittedCompletion() {
GoalEntity goal = readyForCompletion("completion-memory-failure", "report");
org.mockito.Mockito.doThrow(new IllegalStateException("fixture failure")).when(memory).syncAll(
org.mockito.ArgumentMatchers.anyLong(), org.mockito.ArgumentMatchers.anyString(),
org.mockito.ArgumentMatchers.anyString(), org.mockito.ArgumentMatchers.anyString());
goalService.markCompleted(goal.getId(), null);
assertEquals(GoalStatus.COMPLETED, goalService.getById(goal.getId()).getStatus());
assertEquals(1L, goalService.listEvents(goal.getId(), 30).stream()
.filter(event -> "completed".equals(event.getEventType())).count());
}
@Test
@DisplayName("GoalStatus values persist as lowercase literals — load-bearing for uk_agent_goal_active_conv")
void status_persistsAsLowercaseString() {