From 2653356613bbbbdd11da82ca9c3c7a1a0835811d Mon Sep 17 00:00:00 2001 From: matevip Date: Wed, 27 May 2026 14:07:48 +0800 Subject: [PATCH] feat(approval): record human-approval and timeout resolutions, retire grants on conversation delete --- .../approval/ApprovalWorkflowService.java | 36 ++++ .../event/ApprovalResolutionEvent.java | 41 +++++ .../ApprovalResolutionLogListener.java | 104 ++++++++++++ .../ConversationLifecycleListener.java | 62 +++++++ .../approval/grant/ApprovalGrantPr2IT.java | 127 ++++++++++++++ .../ApprovalResolutionLogListenerTest.java | 159 ++++++++++++++++++ .../ConversationLifecycleListenerTest.java | 69 ++++++++ 7 files changed, 598 insertions(+) create mode 100644 mateclaw-server/src/main/java/vip/mate/approval/event/ApprovalResolutionEvent.java create mode 100644 mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListener.java create mode 100644 mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ConversationLifecycleListener.java create mode 100644 mateclaw-server/src/test/java/vip/mate/approval/grant/ApprovalGrantPr2IT.java create mode 100644 mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListenerTest.java create mode 100644 mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ConversationLifecycleListenerTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java b/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java index 839143ca..fe40501d 100644 --- a/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java +++ b/mateclaw-server/src/main/java/vip/mate/approval/ApprovalWorkflowService.java @@ -20,6 +20,7 @@ import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; import vip.mate.agent.context.ChatOrigin; import vip.mate.agent.context.ChatOriginHolder; +import vip.mate.approval.event.ApprovalResolutionEvent; import vip.mate.approval.event.WorkflowApprovalResolvedEvent; import vip.mate.approval.model.ToolApprovalEntity; import vip.mate.approval.repository.ToolApprovalMapper; @@ -707,6 +708,41 @@ public class ApprovalWorkflowService implements ApplicationRunner { } } + // Phase 5 — generic resolution event so the auto-grant resolution log + // can record this final decision. Distinct from the workflow-bridge + // event above: this fires for EVERY resolved approval (not just wf-*), + // and its consumer writes one mate_approval_resolution_log row. + // SUPERSEDED is not a user decision — the replacement pending will fire + // its own event when it resolves, so we skip the event here. + if (events != null && !"SUPERSEDED".equals(dbStatus)) { + String decisionSource = "TIMEOUT".equals(dbStatus) + ? "TIMEOUT" + : "USER_MANUAL"; + String note = "USER_MANUAL".equals(decisionSource) && "DENIED".equals(dbStatus) + ? "denied" + : null; + ApprovalResolutionEvent resolutionEvent = new ApprovalResolutionEvent( + snapshot.getPendingId(), + snapshot.getConversationId(), + snapshot.getAgentId(), + /* userId resolves to actor or original requester */ + userId != null ? userId : snapshot.getUserId(), + snapshot.getToolName(), + snapshot.getToolArguments(), + snapshot.getMaxSeverity(), + snapshot.getFindingsJson(), + decisionSource, + note); + afterCommit(() -> { + try { + events.publishEvent(resolutionEvent); + } catch (Exception e) { + log.warn("[ApprovalWorkflow] failed to publish ApprovalResolutionEvent for {}: {}", + snapshot.getPendingId(), e.getMessage()); + } + }); + } + boolean consumed = "consumed".equals(snapshotStatus); ResolveOutcome outcome = consumed ? ResolveOutcome.consumed(snapshot, true, rewritten) diff --git a/mateclaw-server/src/main/java/vip/mate/approval/event/ApprovalResolutionEvent.java b/mateclaw-server/src/main/java/vip/mate/approval/event/ApprovalResolutionEvent.java new file mode 100644 index 00000000..68e958fb --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/approval/event/ApprovalResolutionEvent.java @@ -0,0 +1,41 @@ +package vip.mate.approval.event; + +/** + * Generic application event fired AFTER an approval row reaches a final + * decision through the human-approval path (approved / denied / consumed) or + * through the timeout sweep. + *

+ * Distinct from {@code WorkflowApprovalResolvedEvent}, which is workflow-bridge + * specific (only published for {@code pendingId} starting with {@code "wf-"}). + * This event is published for every tool-call approval row so the auto-grant + * resolution-log subsystem can record exactly one row per final decision. + * + *

Decision source mapping (see {@code ApprovalResolutionLog.DecisionSource}): + *

+ * + *

{@code findingsJson} is the original JSON serialization of the + * {@code GuardEvaluation.findings} captured at {@code createPending} time. + * The listener extracts {@code ruleId}s from it for {@code resolution_log.rule_ids}. + * + *

All fields are nullable: a row created via the legacy command-injection + * path may not carry every snapshot field. The listener treats missing fields + * as empty rather than skipping the row, so the resolution-log audit stays + * complete even when upstream context is partial. + */ +public record ApprovalResolutionEvent( + String pendingId, + String conversationId, + String agentId, + String userId, + String toolName, + String toolArguments, + String maxSeverity, + String findingsJson, + String decisionSource, + String resolutionNote +) {} diff --git a/mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListener.java b/mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListener.java new file mode 100644 index 00000000..3ed639f1 --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListener.java @@ -0,0 +1,104 @@ +package vip.mate.approval.grant.listener; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Component; +import vip.mate.approval.event.ApprovalResolutionEvent; +import vip.mate.approval.grant.WorkspaceLookupCache; +import vip.mate.approval.grant.entity.ApprovalResolutionLog; +import vip.mate.approval.grant.repository.ApprovalResolutionLogMapper; + +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; + +/** + * Records one row in {@code mate_approval_resolution_log} per final + * human-approval decision (USER_MANUAL / TIMEOUT), complementing the rows + * written directly by {@code ApprovalGrantResolver} for HARD_BLOCK and + * AUTO_GRANT. + *

+ * The listener runs out-of-tx (the publisher fires events from an + * {@code afterCommit} hook), so a DB write failure here cannot roll back the + * already-committed approval state. We log the failure and continue — losing + * one resolution-log row is far less harmful than re-opening the approval row + * for double-resolve. + * + *

Workspace resolution goes through {@link WorkspaceLookupCache} so we get + * the same conversation→workspace mapping the resolver uses on the hot path, + * with the same null-fallback behavior: a deleted conversation produces a row + * with {@code workspace_id = null}, which is allowed by the V128 schema. + */ +@Slf4j +@Component +@RequiredArgsConstructor +public class ApprovalResolutionLogListener { + + private static final int ARGS_PREVIEW_MAX = 500; + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + + private final ApprovalResolutionLogMapper resolutionMapper; + private final WorkspaceLookupCache workspaceLookupCache; + + @EventListener + public void onApprovalResolved(ApprovalResolutionEvent event) { + try { + ApprovalResolutionLog row = new ApprovalResolutionLog(); + row.setWorkspaceId(workspaceLookupCache.resolveByConversation(event.conversationId())); + row.setConversationId(event.conversationId()); + row.setAgentId(event.agentId()); + row.setUserId(event.userId()); + row.setToolName(event.toolName()); + row.setMaxSeverity(event.maxSeverity()); + row.setRuleIds(extractRuleIds(event.findingsJson())); + row.setDecisionSource(event.decisionSource()); + row.setGrantId(null); + row.setPendingId(event.pendingId()); + row.setArgsPreview(previewArgs(event.toolArguments())); + row.setNote(event.resolutionNote()); + + resolutionMapper.insert(row); + } catch (Exception e) { + log.warn("[APPROVAL] ApprovalResolutionLogListener failed to record {} for pending {}: {}", + event.decisionSource(), event.pendingId(), e.getMessage()); + } + } + + /** + * Pulls {@code ruleId}s out of the serialized findings JSON captured at + * {@code createPending} time. The JSON is the standard + * {@code GuardFinding.toMap()} array form (a {@code List>}), + * so we read the list and pick the {@code "ruleId"} key out of each map. + * Returns {@code null} on missing or unparseable input — the row still gets + * written, just without rule-id provenance. + */ + private static String extractRuleIds(String findingsJson) { + if (findingsJson == null || findingsJson.isBlank()) { + return null; + } + try { + List> findings = OBJECT_MAPPER.readValue( + findingsJson, new TypeReference<>() {}); + String joined = findings.stream() + .map(m -> m.get("ruleId")) + .filter(Objects::nonNull) + .map(Object::toString) + .filter(s -> !s.isBlank()) + .distinct() + .collect(Collectors.joining(",")); + return joined.isEmpty() ? null : joined; + } catch (Exception e) { + log.debug("[APPROVAL] Failed to parse findingsJson for rule_ids extraction: {}", e.getMessage()); + return null; + } + } + + private static String previewArgs(String raw) { + if (raw == null) return null; + return raw.length() <= ARGS_PREVIEW_MAX ? raw : raw.substring(0, ARGS_PREVIEW_MAX); + } +} diff --git a/mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ConversationLifecycleListener.java b/mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ConversationLifecycleListener.java new file mode 100644 index 00000000..c2dbac27 --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/approval/grant/listener/ConversationLifecycleListener.java @@ -0,0 +1,62 @@ +package vip.mate.approval.grant.listener; + +import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import org.springframework.context.event.EventListener; +import org.springframework.stereotype.Component; +import vip.mate.approval.grant.WorkspaceLookupCache; +import vip.mate.approval.grant.service.ApprovalGrantService; +import vip.mate.workspace.conversation.event.ConversationDeletedEvent; + +/** + * Tears down conversation-scoped state in the auto-grant subsystem when a + * conversation is deleted. + *

+ * Two actions, run in order: + *

    + *
  1. Soft-revoke every {@code UNTIL_CONVERSATION_END} grant whose + * {@code scope_type = CONVERSATION} and {@code scope_id = conversationId}. + * Without this, the grant would linger as an apparently-active row that + * can never match again (its scope no longer exists), but still shows up + * in the management page and the chip {@code (N)} counter.
    2. + *
  2. Drop the {@code conversationId → workspaceId} entry from + * {@link WorkspaceLookupCache}. A re-created conversation with the same + * id (rare but possible across a backup restore) would otherwise inherit + * the stale mapping for up to five minutes.
    3. + *
+ * + *

{@link ConversationDeletedEvent} is published after the delete tx + * commits, so this listener runs in a clean tx and the soft-revoke either + * succeeds or fails in isolation — it cannot poison the delete itself. + */ +@Slf4j +@Component +@RequiredArgsConstructor +public class ConversationLifecycleListener { + + private final ApprovalGrantService grantService; + private final WorkspaceLookupCache workspaceLookupCache; + + @EventListener + public void onConversationDeleted(ConversationDeletedEvent event) { + String conversationId = event.conversationId(); + if (conversationId == null || conversationId.isEmpty()) { + return; + } + try { + int revoked = grantService.revokeConversationScopedGrants(conversationId); + if (revoked > 0) { + log.info("[APPROVAL] ConversationLifecycleListener: revoked {} UNTIL_CONVERSATION_END grant(s) for {}", + revoked, conversationId); + } + } catch (Exception e) { + log.warn("[APPROVAL] ConversationLifecycleListener: failed to revoke grants for {}: {}", + conversationId, e.getMessage()); + } finally { + // Always invalidate the cache, even if grant revocation threw: a stale + // workspace mapping is more dangerous than a missed revoke (the grant + // can no longer match its conversation anyway). + workspaceLookupCache.invalidate(conversationId); + } + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/approval/grant/ApprovalGrantPr2IT.java b/mateclaw-server/src/test/java/vip/mate/approval/grant/ApprovalGrantPr2IT.java new file mode 100644 index 00000000..84da957c --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/approval/grant/ApprovalGrantPr2IT.java @@ -0,0 +1,127 @@ +package vip.mate.approval.grant; + +import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper; +import com.baomidou.mybatisplus.core.toolkit.Wrappers; +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.context.ApplicationEventPublisher; +import org.springframework.context.annotation.Import; +import org.springframework.test.context.TestPropertySource; +import vip.mate.MateClawApplication; +import vip.mate.approval.event.ApprovalResolutionEvent; +import vip.mate.approval.grant.entity.ApprovalGrant; +import vip.mate.approval.grant.entity.ApprovalResolutionLog; +import vip.mate.approval.grant.repository.ApprovalGrantMapper; +import vip.mate.approval.grant.repository.ApprovalResolutionLogMapper; +import vip.mate.workflow.runtime.StubAgentInvokerConfig; +import vip.mate.workspace.conversation.event.ConversationDeletedEvent; + +import java.time.LocalDateTime; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * End-to-end wiring test for PR-2: publishing a generic approval resolution event + * lands one row in {@code mate_approval_resolution_log}, and publishing a + * {@code ConversationDeletedEvent} soft-revokes UNTIL_CONVERSATION_END grants + * (leaving other-scope grants alone). + *

+ * Reuses the same test profile shape as {@code AgentLifecycleTriggerTest}: + * isolated in-memory H2 per test (no dev DB file), {@code webEnvironment=NONE} + * (no WebSocket container), and the workflow stub configs so the workflow + * trigger bridge doesn't pull in real graph dependencies. + */ +@SpringBootTest( + classes = MateClawApplication.class, + webEnvironment = SpringBootTest.WebEnvironment.NONE +) +@TestPropertySource(properties = { + "spring.datasource.url=jdbc:h2:mem:approval_pr2_${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.workflow.trigger.async-dispatch=false" +}) +@Import(StubAgentInvokerConfig.class) +class ApprovalGrantPr2IT { + + @Autowired private ApplicationEventPublisher publisher; + @Autowired private ApprovalResolutionLogMapper resolutionMapper; + @Autowired private ApprovalGrantMapper grantMapper; + + @Test + @DisplayName("USER_MANUAL ApprovalResolutionEvent → one resolution_log row written by the listener.") + void userManualEventLandsRow() { + long before = resolutionMapper.selectCount(null); + + publisher.publishEvent(new ApprovalResolutionEvent( + "pid-it-1", "conv-it-1", "agent-it-1", "user-it-1", + "read_file", "{\"path\":\"a.txt\"}", "LOW", + "[{\"ruleId\":\"shell.read\",\"severity\":\"LOW\"}]", + "USER_MANUAL", null)); + + List rows = resolutionMapper.selectList( + Wrappers.lambdaQuery() + .eq(ApprovalResolutionLog::getPendingId, "pid-it-1")); + assertThat(rows).hasSize(1); + assertThat(rows.get(0).getDecisionSource()).isEqualTo("USER_MANUAL"); + assertThat(rows.get(0).getRuleIds()).isEqualTo("shell.read"); + assertThat(resolutionMapper.selectCount(null)).isEqualTo(before + 1); + } + + @Test + @DisplayName("TIMEOUT ApprovalResolutionEvent → resolution_log row carries decision_source=TIMEOUT.") + void timeoutEventLandsRow() { + publisher.publishEvent(new ApprovalResolutionEvent( + "pid-it-timeout", "conv-it-timeout", "agent-it-timeout", null, + "execute_shell_command", "ls /tmp", "MEDIUM", null, + "TIMEOUT", null)); + + ApprovalResolutionLog row = resolutionMapper.selectOne( + Wrappers.lambdaQuery() + .eq(ApprovalResolutionLog::getPendingId, "pid-it-timeout")); + assertThat(row).isNotNull(); + assertThat(row.getDecisionSource()).isEqualTo("TIMEOUT"); + } + + @Test + @DisplayName("ConversationDeletedEvent → UNTIL_CONVERSATION_END grant revoked, ALWAYS grant untouched.") + void conversationDeleteRevokesScopedGrantOnly() { + String conversationId = "conv-it-delete"; + long workspaceId = 555L; + + ApprovalGrant conversationGrant = newGrant(workspaceId, "CONVERSATION", + conversationId, "read_file", "ALWAYS", "UNTIL_CONVERSATION_END"); + ApprovalGrant agentGrant = newGrant(workspaceId, "AGENT", + "agent-it-delete", "read_file", "ALWAYS", "ALWAYS"); + grantMapper.insert(conversationGrant); + grantMapper.insert(agentGrant); + + publisher.publishEvent(new ConversationDeletedEvent(conversationId)); + + ApprovalGrant convAfter = grantMapper.selectById(conversationGrant.getId()); + ApprovalGrant agentAfter = grantMapper.selectById(agentGrant.getId()); + assertThat(convAfter.getRevoked()).isEqualTo(1); + assertThat(agentAfter.getRevoked()).isEqualTo(0); + } + + /** Builds a minimal grant row; create/update timestamps default in the DB. */ + private ApprovalGrant newGrant(long workspaceId, String scopeType, String scopeId, + String toolName, String maxSeverity, String grantKind) { + ApprovalGrant g = new ApprovalGrant(); + g.setWorkspaceId(workspaceId); + g.setScopeType(scopeType); + g.setScopeId(scopeId); + g.setToolName(toolName); + g.setRuleId(null); + g.setMaxSeverity(maxSeverity); + g.setGrantKind(grantKind); + g.setGrantedBy(1L); + g.setGrantedAt(LocalDateTime.now()); + g.setRevoked(0); + g.setDeleted(0); + return g; + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListenerTest.java b/mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListenerTest.java new file mode 100644 index 00000000..7979f694 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ApprovalResolutionLogListenerTest.java @@ -0,0 +1,159 @@ +package vip.mate.approval.grant.listener; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import vip.mate.approval.event.ApprovalResolutionEvent; +import vip.mate.approval.grant.WorkspaceLookupCache; +import vip.mate.approval.grant.entity.ApprovalResolutionLog; +import vip.mate.approval.grant.repository.ApprovalResolutionLogMapper; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link ApprovalResolutionLogListener}. + *

+ * Coverage targets: + *

+ */ +@ExtendWith(MockitoExtension.class) +class ApprovalResolutionLogListenerTest { + + @Mock ApprovalResolutionLogMapper resolutionMapper; + @Mock WorkspaceLookupCache workspaceLookupCache; + + @InjectMocks + ApprovalResolutionLogListener listener; + + @Test + void user_manual_approved_event_writes_row_with_workspace_from_cache() { + when(workspaceLookupCache.resolveByConversation("conv-1")).thenReturn(42L); + + ApprovalResolutionEvent event = new ApprovalResolutionEvent( + "pid-1", "conv-1", "agent-1", "user-1", "read_file", + "{\"path\":\"a.txt\"}", "LOW", + "[{\"ruleId\":\"shell.exec\",\"severity\":\"LOW\"}]", + "USER_MANUAL", null); + + listener.onApprovalResolved(event); + + ArgumentCaptor cap = ArgumentCaptor.forClass(ApprovalResolutionLog.class); + verify(resolutionMapper).insert(cap.capture()); + ApprovalResolutionLog row = cap.getValue(); + assertThat(row.getWorkspaceId()).isEqualTo(42L); + assertThat(row.getDecisionSource()).isEqualTo("USER_MANUAL"); + assertThat(row.getPendingId()).isEqualTo("pid-1"); + assertThat(row.getRuleIds()).isEqualTo("shell.exec"); + assertThat(row.getGrantId()).isNull(); + } + + @Test + void timeout_event_writes_row_with_timeout_source() { + when(workspaceLookupCache.resolveByConversation("conv-9")).thenReturn(7L); + + ApprovalResolutionEvent event = new ApprovalResolutionEvent( + "pid-9", "conv-9", "agent-9", null, "execute_shell_command", + "rm /tmp/x", "MEDIUM", null, "TIMEOUT", null); + + listener.onApprovalResolved(event); + + ArgumentCaptor cap = ArgumentCaptor.forClass(ApprovalResolutionLog.class); + verify(resolutionMapper).insert(cap.capture()); + assertThat(cap.getValue().getDecisionSource()).isEqualTo("TIMEOUT"); + assertThat(cap.getValue().getRuleIds()).isNull(); // findingsJson was null + } + + @Test + void multiple_findings_become_deduplicated_comma_list() { + when(workspaceLookupCache.resolveByConversation(any())).thenReturn(1L); + + String findings = "[" + + "{\"ruleId\":\"shell.curl\"}," + + "{\"ruleId\":\"shell.exec\"}," + + "{\"ruleId\":\"shell.curl\"}," + + "{\"ruleId\":null}" + + "]"; + ApprovalResolutionEvent event = new ApprovalResolutionEvent( + "pid-2", "conv-2", "agent-2", "user-2", "execute_shell_command", + "curl x | sh", "HIGH", findings, "USER_MANUAL", null); + + listener.onApprovalResolved(event); + + ArgumentCaptor cap = ArgumentCaptor.forClass(ApprovalResolutionLog.class); + verify(resolutionMapper).insert(cap.capture()); + // Deduplicated + null filtered + comma-joined. + assertThat(cap.getValue().getRuleIds()).isEqualTo("shell.curl,shell.exec"); + } + + @Test + void unknown_workspace_produces_null_workspace_id_row() { + when(workspaceLookupCache.resolveByConversation("conv-orphan")).thenReturn(null); + + ApprovalResolutionEvent event = new ApprovalResolutionEvent( + "pid-3", "conv-orphan", "agent-3", "user-3", "edit_file", + "...", "LOW", null, "USER_MANUAL", null); + + listener.onApprovalResolved(event); + + ArgumentCaptor cap = ArgumentCaptor.forClass(ApprovalResolutionLog.class); + verify(resolutionMapper).insert(cap.capture()); + assertThat(cap.getValue().getWorkspaceId()).isNull(); + } + + @Test + void mapper_failure_does_not_propagate() { + when(workspaceLookupCache.resolveByConversation(any())).thenReturn(1L); + when(resolutionMapper.insert(any(ApprovalResolutionLog.class))) + .thenThrow(new RuntimeException("DB down")); + + // No exception escapes — the resolved approval has already committed. + listener.onApprovalResolved(new ApprovalResolutionEvent( + "pid-x", "conv-x", "agent-x", "user-x", "tool", + "args", "LOW", null, "USER_MANUAL", null)); + } + + @Test + void malformed_findings_json_still_writes_row_without_rule_ids() { + when(workspaceLookupCache.resolveByConversation(any())).thenReturn(1L); + + ApprovalResolutionEvent event = new ApprovalResolutionEvent( + "pid-bad", "conv-bad", "agent-bad", "user-bad", "tool", + "args", "LOW", "{not-an-array}", "USER_MANUAL", null); + + listener.onApprovalResolved(event); + + ArgumentCaptor cap = ArgumentCaptor.forClass(ApprovalResolutionLog.class); + verify(resolutionMapper).insert(cap.capture()); + assertThat(cap.getValue().getRuleIds()).isNull(); + assertThat(cap.getValue().getDecisionSource()).isEqualTo("USER_MANUAL"); + } + + @Test + void long_args_are_truncated_to_500_chars() { + when(workspaceLookupCache.resolveByConversation(any())).thenReturn(1L); + + String longArgs = "x".repeat(800); + listener.onApprovalResolved(new ApprovalResolutionEvent( + "pid-long", "conv-long", "agent-long", "user-long", "tool", + longArgs, "LOW", null, "USER_MANUAL", null)); + + ArgumentCaptor cap = ArgumentCaptor.forClass(ApprovalResolutionLog.class); + verify(resolutionMapper).insert(cap.capture()); + assertThat(cap.getValue().getArgsPreview()).hasSize(500); + } +} diff --git a/mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ConversationLifecycleListenerTest.java b/mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ConversationLifecycleListenerTest.java new file mode 100644 index 00000000..84136a1d --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/approval/grant/listener/ConversationLifecycleListenerTest.java @@ -0,0 +1,69 @@ +package vip.mate.approval.grant.listener; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import vip.mate.approval.grant.WorkspaceLookupCache; +import vip.mate.approval.grant.service.ApprovalGrantService; +import vip.mate.workspace.conversation.event.ConversationDeletedEvent; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Unit tests for {@link ConversationLifecycleListener}. + */ +@ExtendWith(MockitoExtension.class) +class ConversationLifecycleListenerTest { + + @Mock ApprovalGrantService grantService; + @Mock WorkspaceLookupCache workspaceLookupCache; + + @InjectMocks + ConversationLifecycleListener listener; + + @Test + void delete_revokes_grants_and_invalidates_cache() { + when(grantService.revokeConversationScopedGrants("conv-1")).thenReturn(2); + + listener.onConversationDeleted(new ConversationDeletedEvent("conv-1")); + + verify(grantService).revokeConversationScopedGrants("conv-1"); + verify(workspaceLookupCache).invalidate("conv-1"); + } + + @Test + void delete_with_no_active_grants_still_invalidates_cache() { + when(grantService.revokeConversationScopedGrants("conv-2")).thenReturn(0); + + listener.onConversationDeleted(new ConversationDeletedEvent("conv-2")); + + verify(workspaceLookupCache).invalidate("conv-2"); + } + + @Test + void grant_service_failure_still_invalidates_cache() { + // A stale workspace mapping is more dangerous than a missed revoke + // (the grant can no longer match its conversation anyway), so the + // finally-block invalidation runs even if revocation throws. + when(grantService.revokeConversationScopedGrants("conv-3")) + .thenThrow(new RuntimeException("DB down")); + + listener.onConversationDeleted(new ConversationDeletedEvent("conv-3")); + + verify(workspaceLookupCache).invalidate("conv-3"); + } + + @Test + void blank_or_null_conversation_id_is_no_op() { + listener.onConversationDeleted(new ConversationDeletedEvent("")); + listener.onConversationDeleted(new ConversationDeletedEvent(null)); + + verify(grantService, never()).revokeConversationScopedGrants(eq("")); + verify(workspaceLookupCache, never()).invalidate(eq("")); + } +}