From 1eabbb68acc057eb80d9f36008422310071c9582 Mon Sep 17 00:00:00 2001 From: matevip Date: Thu, 20 Aug 2026 03:26:15 -0400 Subject: [PATCH] fix(team): allow admins to read completed worker runs (#596) --- .../TeamWorkerConversationController.java | 3 +- ...amWorkerConversationGovernanceService.java | 48 ++++++++++++++ .../controller/ConversationController.java | 7 +- ...rkerConversationGovernanceServiceTest.java | 45 ++++++++++++- ...ConversationControllerBatchDeleteTest.java | 4 +- ...ionControllerTeamWorkerTranscriptTest.java | 65 +++++++++++++++++++ mateclaw-ui/src/api/index.ts | 2 +- mateclaw-ui/src/views/ChatConsole.vue | 16 +++-- 8 files changed, 181 insertions(+), 9 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerTeamWorkerTranscriptTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/team/controller/TeamWorkerConversationController.java b/mateclaw-server/src/main/java/vip/mate/team/controller/TeamWorkerConversationController.java index 7a9a2b57..ae167ffd 100644 --- a/mateclaw-server/src/main/java/vip/mate/team/controller/TeamWorkerConversationController.java +++ b/mateclaw-server/src/main/java/vip/mate/team/controller/TeamWorkerConversationController.java @@ -27,7 +27,8 @@ public class TeamWorkerConversationController { @RequestParam(required = false) Long taskId, Authentication authentication) { String username = authentication == null ? "anonymous" : authentication.getName(); - if (!conversationService.isConversationOwner(conversationId, username)) { + if (!conversationService.isConversationOwner(conversationId, username) + && !governanceService.canReadTranscript(conversationId, runId, taskId, username)) { return R.fail(403, "无权访问该会话"); } return governanceService.resolve(conversationId, runId, taskId) diff --git a/mateclaw-server/src/main/java/vip/mate/team/service/TeamWorkerConversationGovernanceService.java b/mateclaw-server/src/main/java/vip/mate/team/service/TeamWorkerConversationGovernanceService.java index 1e0c308a..23ef8c98 100644 --- a/mateclaw-server/src/main/java/vip/mate/team/service/TeamWorkerConversationGovernanceService.java +++ b/mateclaw-server/src/main/java/vip/mate/team/service/TeamWorkerConversationGovernanceService.java @@ -3,10 +3,13 @@ package vip.mate.team.service; import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper; import lombok.RequiredArgsConstructor; import org.springframework.stereotype.Service; +import vip.mate.auth.model.UserEntity; +import vip.mate.auth.service.AuthService; import vip.mate.team.model.TeamRunEntity; import vip.mate.team.model.TeamTaskEntity; import vip.mate.team.repository.TeamRunMapper; import vip.mate.team.repository.TeamTaskMapper; +import vip.mate.workspace.core.service.WorkspaceService; import vip.mate.workspace.conversation.model.ConversationEntity; import vip.mate.workspace.conversation.repository.ConversationMapper; import vip.mate.workspace.conversation.ConversationService; @@ -21,6 +24,8 @@ public class TeamWorkerConversationGovernanceService { private final TeamTaskMapper taskMapper; private final TeamRunMapper runMapper; private final ConversationMapper conversationMapper; + private final AuthService authService; + private final WorkspaceService workspaceService; public Optional resolve( String conversationId, Long requestedRunId, Long requestedTaskId) { @@ -58,4 +63,47 @@ public class TeamWorkerConversationGovernanceService { true, "team_worker", conversationId, run.getId(), task.getId(), run.getTeamId(), run.getLeadConversationId(), task.getAssigneeAgentId())); } + + /** + * Team worker transcripts are read-only evidence for a team run. The worker + * conversation is owned by the executing agent/user, so workspace admins and + * reviewers are not direct conversation owners. Allow them to read only when + * the persisted task/run/conversation linkage is canonical and they belong + * to that run's workspace. + */ + public boolean canReadTranscript(String conversationId, Long requestedRunId, Long requestedTaskId, + String username) { + TeamWorkerAccess access = resolveAccess(conversationId, requestedRunId, requestedTaskId); + if (access == null) { + return false; + } + UserEntity requester = authService.findByUsername(username); + if (requester == null) { + return false; + } + if ("admin".equalsIgnoreCase(requester.getRole())) { + return true; + } + return workspaceService.hasPermissionCached(access.workspaceId(), requester.getId(), "viewer"); + } + + private TeamWorkerAccess resolveAccess(String conversationId, Long requestedRunId, Long requestedTaskId) { + if (resolve(conversationId, requestedRunId, requestedTaskId).isEmpty()) { + return null; + } + TeamTaskEntity task = taskMapper.selectOne(new LambdaQueryWrapper() + .eq(TeamTaskEntity::getConversationId, conversationId) + .last("LIMIT 1")); + if (task == null || task.getRunId() == null) { + return null; + } + TeamRunEntity run = runMapper.selectById(task.getRunId()); + if (run == null || run.getWorkspaceId() == null) { + return null; + } + return new TeamWorkerAccess(run.getWorkspaceId()); + } + + private record TeamWorkerAccess(Long workspaceId) { + } } diff --git a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java index 297e3722..2f2ea7f6 100644 --- a/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java +++ b/mateclaw-server/src/main/java/vip/mate/workspace/conversation/controller/ConversationController.java @@ -9,6 +9,7 @@ import org.springframework.security.core.Authentication; import org.springframework.web.bind.annotation.*; import vip.mate.common.result.R; import vip.mate.channel.web.ChatStreamTracker; +import vip.mate.team.service.TeamWorkerConversationGovernanceService; import vip.mate.workspace.conversation.ConversationService; import vip.mate.workspace.conversation.vo.ConversationVO; import vip.mate.workspace.conversation.vo.MessageVO; @@ -32,6 +33,7 @@ public class ConversationController { private final ConversationService conversationService; private final ChatStreamTracker streamTracker; + private final TeamWorkerConversationGovernanceService teamWorkerGovernanceService; /** * 获取当前用户的会话列表 @@ -94,9 +96,12 @@ public class ConversationController { public R listMessages(@PathVariable String conversationId, @RequestParam(required = false) Long beforeId, @RequestParam(required = false) Integer limit, + @RequestParam(required = false) Long runId, + @RequestParam(required = false) Long taskId, Authentication auth) { String username = auth != null ? auth.getName() : "anonymous"; - if (!conversationService.isConversationOwner(conversationId, username)) { + if (!conversationService.isConversationOwner(conversationId, username) + && !teamWorkerGovernanceService.canReadTranscript(conversationId, runId, taskId, username)) { return R.fail(403, "无权访问该会话"); } diff --git a/mateclaw-server/src/test/java/vip/mate/team/service/TeamWorkerConversationGovernanceServiceTest.java b/mateclaw-server/src/test/java/vip/mate/team/service/TeamWorkerConversationGovernanceServiceTest.java index 7ef5d688..59741d30 100644 --- a/mateclaw-server/src/test/java/vip/mate/team/service/TeamWorkerConversationGovernanceServiceTest.java +++ b/mateclaw-server/src/test/java/vip/mate/team/service/TeamWorkerConversationGovernanceServiceTest.java @@ -4,10 +4,13 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import vip.mate.auth.model.UserEntity; +import vip.mate.auth.service.AuthService; import vip.mate.team.model.TeamRunEntity; import vip.mate.team.model.TeamTaskEntity; import vip.mate.team.repository.TeamRunMapper; import vip.mate.team.repository.TeamTaskMapper; +import vip.mate.workspace.core.service.WorkspaceService; import vip.mate.workspace.conversation.model.ConversationEntity; import vip.mate.workspace.conversation.repository.ConversationMapper; @@ -21,6 +24,8 @@ class TeamWorkerConversationGovernanceServiceTest { @Mock private TeamTaskMapper taskMapper; @Mock private TeamRunMapper runMapper; @Mock private ConversationMapper conversationMapper; + @Mock private AuthService authService; + @Mock private WorkspaceService workspaceService; @Test void returnsVerifiedCanonicalContextOnlyWhenRequestedLinkageMatches() { @@ -102,8 +107,39 @@ class TeamWorkerConversationGovernanceServiceTest { assertThat(service().resolve("team-task-legacy-no-parent", 77L, 501L)).isPresent(); } + @Test + void allowsWorkspaceAdminToReadVerifiedWorkerTranscript() { + TeamTaskEntity task = task(501L, 77L, "worker-conversation"); + when(conversationMapper.selectOne(any())).thenReturn(conversation( + "worker-conversation", 30L, 41L, "lead-conversation", "team_worker")); + when(taskMapper.selectOne(any())).thenReturn(task); + when(runMapper.selectById(77L)).thenReturn(run(77L, 20L, "lead-conversation")); + when(authService.findByUsername("workspace-admin")).thenReturn(user(900L, "user")); + when(workspaceService.hasPermissionCached(30L, 900L, "viewer")).thenReturn(true); + + assertThat(service().canReadTranscript("worker-conversation", 77L, 501L, + "workspace-admin")).isTrue(); + } + + @Test + void rejectsWorkerTranscriptReadWhenRouteLinkageOrWorkspaceMembershipDoesNotMatch() { + TeamTaskEntity task = task(501L, 77L, "worker-conversation"); + when(conversationMapper.selectOne(any())).thenReturn(conversation( + "worker-conversation", 30L, 41L, "lead-conversation", "team_worker")); + when(taskMapper.selectOne(any())).thenReturn(task); + when(runMapper.selectById(77L)).thenReturn(run(77L, 20L, "lead-conversation")); + when(authService.findByUsername("outsider")).thenReturn(user(901L, "user")); + when(workspaceService.hasPermissionCached(30L, 901L, "viewer")).thenReturn(false); + + assertThat(service().canReadTranscript("worker-conversation", 88L, 501L, + "outsider")).isFalse(); + assertThat(service().canReadTranscript("worker-conversation", 77L, 501L, + "outsider")).isFalse(); + } + private TeamWorkerConversationGovernanceService service() { - return new TeamWorkerConversationGovernanceService(taskMapper, runMapper, conversationMapper); + return new TeamWorkerConversationGovernanceService(taskMapper, runMapper, conversationMapper, + authService, workspaceService); } private static TeamTaskEntity task(Long id, Long runId, String conversationId) { @@ -135,4 +171,11 @@ class TeamWorkerConversationGovernanceServiceTest { conversation.setConversationKind(kind); return conversation; } + + private static UserEntity user(long id, String role) { + UserEntity user = new UserEntity(); + user.setId(id); + user.setRole(role); + return user; + } } diff --git a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerBatchDeleteTest.java b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerBatchDeleteTest.java index ad7b65be..7798d987 100644 --- a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerBatchDeleteTest.java +++ b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerBatchDeleteTest.java @@ -8,6 +8,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.security.core.Authentication; import vip.mate.channel.web.ChatStreamTracker; import vip.mate.common.result.R; +import vip.mate.team.service.TeamWorkerConversationGovernanceService; import vip.mate.workspace.conversation.ConversationService; import java.util.ArrayList; @@ -25,13 +26,14 @@ class ConversationControllerBatchDeleteTest { @Mock private ConversationService conversationService; @Mock private ChatStreamTracker streamTracker; + @Mock private TeamWorkerConversationGovernanceService teamWorkerGovernanceService; @Mock private Authentication authentication; private ConversationController controller; @BeforeEach void setUp() { - controller = new ConversationController(conversationService, streamTracker); + controller = new ConversationController(conversationService, streamTracker, teamWorkerGovernanceService); when(authentication.getName()).thenReturn("alice"); } diff --git a/mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerTeamWorkerTranscriptTest.java b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerTeamWorkerTranscriptTest.java new file mode 100644 index 00000000..b8e5d86e --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/workspace/conversation/controller/ConversationControllerTeamWorkerTranscriptTest.java @@ -0,0 +1,65 @@ +package vip.mate.workspace.conversation.controller; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.core.Authentication; +import vip.mate.channel.web.ChatStreamTracker; +import vip.mate.common.result.R; +import vip.mate.team.service.TeamWorkerConversationGovernanceService; +import vip.mate.workspace.conversation.ConversationService; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ConversationControllerTeamWorkerTranscriptTest { + + @Mock private ConversationService conversationService; + @Mock private ChatStreamTracker streamTracker; + @Mock private TeamWorkerConversationGovernanceService teamWorkerGovernanceService; + @Mock private Authentication authentication; + + private ConversationController controller; + + @BeforeEach + void setUp() { + controller = new ConversationController(conversationService, streamTracker, teamWorkerGovernanceService); + when(authentication.getName()).thenReturn("workspace-admin"); + } + + @Test + void listMessagesAllowsVerifiedTeamWorkerTranscriptForNonOwner() { + when(conversationService.isConversationOwner("worker-conversation", "workspace-admin")) + .thenReturn(false); + when(teamWorkerGovernanceService.canReadTranscript("worker-conversation", 77L, 501L, + "workspace-admin")).thenReturn(true); + when(conversationService.listMessageViews("worker-conversation")).thenReturn(List.of()); + + R result = controller.listMessages("worker-conversation", null, null, 77L, 501L, + authentication); + + assertEquals(200, result.getCode()); + assertEquals(List.of(), result.getData()); + } + + @Test + void listMessagesRejectsNonOwnerWhenWorkerTranscriptIsNotVerified() { + when(conversationService.isConversationOwner("ordinary-conversation", "workspace-admin")) + .thenReturn(false); + when(teamWorkerGovernanceService.canReadTranscript("ordinary-conversation", 77L, 501L, + "workspace-admin")).thenReturn(false); + + R result = controller.listMessages("ordinary-conversation", null, null, 77L, 501L, + authentication); + + assertEquals(403, result.getCode()); + verify(conversationService, never()).listMessageViews("ordinary-conversation"); + } +} diff --git a/mateclaw-ui/src/api/index.ts b/mateclaw-ui/src/api/index.ts index 2acad37b..f406a3e4 100644 --- a/mateclaw-ui/src/api/index.ts +++ b/mateclaw-ui/src/api/index.ts @@ -204,7 +204,7 @@ export const conversationApi = { */ page: (params: { page?: number; size?: number; keyword?: string }) => http.get('/conversations/page', { params }), - listMessages: (conversationId: string, params?: { beforeId?: number; limit?: number }) => + listMessages: (conversationId: string, params?: { beforeId?: number; limit?: number; runId?: string; taskId?: string }) => http.get(`/conversations/${encId(conversationId)}/messages`, { params }), getStatus: (conversationId: string) => http.get(`/conversations/${encId(conversationId)}/status`), diff --git a/mateclaw-ui/src/views/ChatConsole.vue b/mateclaw-ui/src/views/ChatConsole.vue index 49a5d4fc..be4357e5 100644 --- a/mateclaw-ui/src/views/ChatConsole.vue +++ b/mateclaw-ui/src/views/ChatConsole.vue @@ -857,6 +857,14 @@ const workerRunContext = computed(() => workerGuard.context.value ?? readLegacyWorkerRouteContext(currentConversationId.value, route.query)) const workerConversationReadOnly = computed(() => workerGuard.readOnly.value) +function workerTranscriptMessageParams() { + const query = teamRunRouteQuery.value + return { + runId: query.teamRunId, + taskId: query.taskId, + } +} + // ============ 连接状态 ============ const connectionStatusClass = computed(() => { if (isGenerating.value) return 'status-streaming' @@ -1678,7 +1686,7 @@ async function refreshCurrentConversationMessages(conversationId: string) { if (isGenerating.value) return if (streamPhase.value === 'awaiting_approval') return try { - const res: any = await conversationApi.listMessages(conversationId) + const res: any = await conversationApi.listMessages(conversationId, workerTranscriptMessageParams()) // Stale guard：await 返回后确认仍是当前会话 if (currentConversationId.value !== conversationId) return // 二次 isGenerating 检查：如果 await 期间用户已发新消息，不覆盖本地状态 @@ -1715,9 +1723,9 @@ async function hydrateStateFromRoute() { currentConversationId.value = conversationId messages.value = [] try { - const res: any = await conversationApi.listMessages(conversationId) + const res: any = await conversationApi.listMessages(conversationId, workerTranscriptMessageParams()) if (currentConversationId.value !== conversationId) return - messages.value = extractMessages(res).messages.map((msg: Message) => normalizeMessage(msg, true)) + messages.value = extractMessages(res).messages.map((msg: Message) => normalizeMessage(msg, true)) } catch { // 消息加载失败，保持空 } @@ -1783,7 +1791,7 @@ async function selectConversation(conv: Conversation, routeAgentId = '') { markConversationViewed(conv.conversationId, conv.lastActiveTime) const requestedConvId = conv.conversationId try { - const res: any = await conversationApi.listMessages(requestedConvId) + const res: any = await conversationApi.listMessages(requestedConvId, workerTranscriptMessageParams()) // Stale guard：await 返回后确认仍是当前会话，否则丢弃 if (currentConversationId.value !== requestedConvId) return // 点同一个会话时，若已有 SSE 在跑就不要覆盖本地消息状态