mirror of
https://gitee.com/mateos/mateclaw.git
synced 2026-09-15 03:55:09 +08:00
feat(approval): record human-approval and timeout resolutions, retire grants on conversation delete
This commit is contained in:
matevip
parent
b7e923fac4
commit
2653356613
@ -20,6 +20,7 @@ import org.springframework.transaction.support.TransactionSynchronization;
|
|||||||
import org.springframework.transaction.support.TransactionSynchronizationManager;
|
import org.springframework.transaction.support.TransactionSynchronizationManager;
|
||||||
import vip.mate.agent.context.ChatOrigin;
|
import vip.mate.agent.context.ChatOrigin;
|
||||||
import vip.mate.agent.context.ChatOriginHolder;
|
import vip.mate.agent.context.ChatOriginHolder;
|
||||||
|
import vip.mate.approval.event.ApprovalResolutionEvent;
|
||||||
import vip.mate.approval.event.WorkflowApprovalResolvedEvent;
|
import vip.mate.approval.event.WorkflowApprovalResolvedEvent;
|
||||||
import vip.mate.approval.model.ToolApprovalEntity;
|
import vip.mate.approval.model.ToolApprovalEntity;
|
||||||
import vip.mate.approval.repository.ToolApprovalMapper;
|
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);
|
boolean consumed = "consumed".equals(snapshotStatus);
|
||||||
ResolveOutcome outcome = consumed
|
ResolveOutcome outcome = consumed
|
||||||
? ResolveOutcome.consumed(snapshot, true, rewritten)
|
? ResolveOutcome.consumed(snapshot, true, rewritten)
|
||||||
|
|||||||
@ -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.
|
||||||
|
* <p>
|
||||||
|
* 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.
|
||||||
|
*
|
||||||
|
* <p><b>Decision source mapping</b> (see {@code ApprovalResolutionLog.DecisionSource}):
|
||||||
|
* <ul>
|
||||||
|
* <li>{@code APPROVED} / {@code DENIED} / {@code CONSUMED} → {@code USER_MANUAL}</li>
|
||||||
|
* <li>{@code TIMEOUT} → {@code TIMEOUT}</li>
|
||||||
|
* <li>{@code SUPERSEDED} → no event (not a final user decision; the replacement
|
||||||
|
* approval will fire its own event when it resolves)</li>
|
||||||
|
* </ul>
|
||||||
|
*
|
||||||
|
* <p>{@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}.
|
||||||
|
*
|
||||||
|
* <p>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
|
||||||
|
) {}
|
||||||
@ -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.
|
||||||
|
* <p>
|
||||||
|
* 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.
|
||||||
|
*
|
||||||
|
* <p>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<Map<String, Object>>}),
|
||||||
|
* 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<Map<String, Object>> 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);
|
||||||
|
}
|
||||||
|
}
|
||||||
@ -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.
|
||||||
|
* <p>
|
||||||
|
* Two actions, run in order:
|
||||||
|
* <ol>
|
||||||
|
* <li>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.</li>
|
||||||
|
* <li>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.</li>
|
||||||
|
* </ol>
|
||||||
|
*
|
||||||
|
* <p>{@link ConversationDeletedEvent} is published <i>after</i> 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);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@ -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).
|
||||||
|
* <p>
|
||||||
|
* 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<ApprovalResolutionLog> rows = resolutionMapper.selectList(
|
||||||
|
Wrappers.<ApprovalResolutionLog>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.<ApprovalResolutionLog>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;
|
||||||
|
}
|
||||||
|
}
|
||||||
@ -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}.
|
||||||
|
* <p>
|
||||||
|
* Coverage targets:
|
||||||
|
* <ul>
|
||||||
|
* <li>USER_MANUAL approval event → resolution_log row with correct fields.</li>
|
||||||
|
* <li>TIMEOUT event → row with decision_source = TIMEOUT.</li>
|
||||||
|
* <li>findingsJson with multiple ruleIds → comma-joined, deduplicated rule_ids.</li>
|
||||||
|
* <li>workspace_id resolution goes through the cache (so deleted conversations
|
||||||
|
* produce a null workspace, allowed by V128 schema).</li>
|
||||||
|
* <li>Mapper failure does not propagate (the listener swallows so the resolved
|
||||||
|
* approval commit isn't endangered).</li>
|
||||||
|
* </ul>
|
||||||
|
*/
|
||||||
|
@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<ApprovalResolutionLog> 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<ApprovalResolutionLog> 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<ApprovalResolutionLog> 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<ApprovalResolutionLog> 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<ApprovalResolutionLog> 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<ApprovalResolutionLog> cap = ArgumentCaptor.forClass(ApprovalResolutionLog.class);
|
||||||
|
verify(resolutionMapper).insert(cap.capture());
|
||||||
|
assertThat(cap.getValue().getArgsPreview()).hasSize(500);
|
||||||
|
}
|
||||||
|
}
|
||||||
@ -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(""));
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Reference in New Issue
Block a user