From d66ac00dab92df9d08b912a52e15fa4db9dab429 Mon Sep 17 00:00:00 2001 From: mateaix <7333791@qq.com> Date: Tue, 15 Sep 2026 04:04:34 +0800 Subject: [PATCH] fix(goal): recheck approver account during managed replay --- .../vip/mate/channel/web/ChatController.java | 44 ++++++++++++++++--- .../docs/en/managed-json-acceptance.md | 2 + .../docs/zh/managed-json-acceptance.md | 2 + .../GoalJsonHttpRuntimeIntegrationTest.java | 34 +++++++++++++- 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java index fbbc6218..9268d59c 100644 --- a/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java +++ b/mateclaw-server/src/main/java/vip/mate/channel/web/ChatController.java @@ -74,6 +74,12 @@ public class ChatController { @org.springframework.beans.factory.annotation.Autowired private ConversationTurnGate turnGate = new ConversationTurnGate(); + @org.springframework.beans.factory.annotation.Autowired + private vip.mate.goal.service.GoalJsonAcceptanceService jsonAcceptance; + + @org.springframework.beans.factory.annotation.Autowired + private vip.mate.goal.service.GoalApprovalRunService goalApprovalRuns; + // Virtual thread per SSE task: matches the app-wide virtual-thread model // (spring.threads.virtual.enabled=true) and, unlike a cached platform-thread // pool, never reuses a thread across tasks, so no ThreadLocal state can leak @@ -241,7 +247,13 @@ public class ChatController { // deny: workflow.resolve handles DB + metadata + memory atomically. if (isDenyCommand) { - ResolveOutcome denyOutcome = approvalService.resolve(pending.getPendingId(), username, "denied"); + ResolveOutcome denyOutcome; + try { + denyOutcome = resolveWithCurrentApprover(pending, auth, username, false); + } catch (vip.mate.exception.MateClawException revoked) { + sendErrorDoneAndComplete(emitter, revoked.getMessage()); + return emitter; + } conversationService.removeApprovalPlaceholders(conversationId); log.info("[Approval-Stream] User {} denied pending {} for conversation {} (dbSynced={}, msgRewritten={})", username, pending.getPendingId(), conversationId, @@ -251,7 +263,13 @@ public class ChatController { // approve: atomic resolveAndConsume; workflow handles DB + metadata + memory. PendingApproval consumed = null; if (isApprovalCommand) { - ResolveOutcome consumeOutcome = approvalService.resolveAndConsume(pending.getPendingId(), username); + ResolveOutcome consumeOutcome; + try { + consumeOutcome = resolveWithCurrentApprover(pending, auth, username, true); + } catch (vip.mate.exception.MateClawException revoked) { + sendErrorDoneAndComplete(emitter, revoked.getMessage()); + return emitter; + } if (consumeOutcome.isAlreadyResolved()) { try { sendEvent(emitter, "error", Map.of("message", "审批记录已过期或已被处理")); @@ -1344,11 +1362,23 @@ public class ChatController { return vip.mate.agent.context.ChatOrigin.web(conversationId, username, workspaceId, null, baseUrl, requesterUserId); } - /** - * Extract the authenticated user's immutable numeric id from the - * {@link Authentication} details (stamped by {@code JwtAuthFilter} for both - * the JWT and PAT paths). Null when not authenticated or details absent. - */ + /** Hold the selected Goal's approver identity through the approval write. */ + private ResolveOutcome resolveWithCurrentApprover(PendingApproval pending, Authentication auth, + String username, boolean approve) { + if (goalApprovalRuns != null && jsonAcceptance != null) { + var origin = approvalService.restoreChatOrigin(pending.getChatOrigin()); + if (origin != null && goalApprovalRuns.requiresHandoff(origin.withApprovalId(pending.getPendingId()))) { + return jsonAcceptance.withAuthenticatedUser(requesterUserIdOf(auth), username, + current -> approve + ? approvalService.resolveAndConsume(pending.getPendingId(), current) + : approvalService.resolve(pending.getPendingId(), current, "denied")); + } + } + return approve ? approvalService.resolveAndConsume(pending.getPendingId(), username) + : approvalService.resolve(pending.getPendingId(), username, "denied"); + } + + /** Extract the immutable account id stamped in Authentication details. */ private Long requesterUserIdOf(org.springframework.security.core.Authentication auth) { if (auth == null) return null; Object details = auth.getDetails(); diff --git a/mateclaw-server/src/main/resources/docs/en/managed-json-acceptance.md b/mateclaw-server/src/main/resources/docs/en/managed-json-acceptance.md index d4f0b663..f78af830 100644 --- a/mateclaw-server/src/main/resources/docs/en/managed-json-acceptance.md +++ b/mateclaw-server/src/main/resources/docs/en/managed-json-acceptance.md @@ -72,3 +72,5 @@ When a background Goal settles into awaiting approval, its original attempt leas V200 records the exact attempt that settled into approval waiting and makes the approval-to-new-attempt association unique. Older waiting rows remain unbound and cannot be inferred into new execution authority. Replay renews its lease every 20 seconds and settles on normal completion. Cancellation, failure, or lease loss preserves an uncertain checkpoint; expiry recovery pauses for review instead of blindly replaying side effects. Because graph tool events can arrive in batches, an intermediate completion event does not make the whole replay safe to retry. Approval snapshots use the identity bound to tool execution in graph state, rather than missing or unrelated ambient thread identity. Background replay restores the previous thread context immediately after constructing graph state; persisted account, Goal, and attempt associations are still rechecked during execution. + +For a Goal with managed JSON requirements, Web approval consumption also locks and rechecks the requesting account ID through the approval transaction. If an account is retired and replaced under the same username after JWT validation, the old in-flight request neither consumes the pending approval nor starts replay; a newly signed-in account can still approve. Denial uses the same current-account check. diff --git a/mateclaw-server/src/main/resources/docs/zh/managed-json-acceptance.md b/mateclaw-server/src/main/resources/docs/zh/managed-json-acceptance.md index 824b720e..7c092d00 100644 --- a/mateclaw-server/src/main/resources/docs/zh/managed-json-acceptance.md +++ b/mateclaw-server/src/main/resources/docs/zh/managed-json-acceptance.md @@ -74,3 +74,5 @@ JWT请求同时核对签名令牌的userId与当前启用账户ID。同名账户 V200 保存待审批结算对应的准确 attempt，并对审批生成的新 attempt 设置唯一关联。升级前的待审批行保持未绑定，不能推断为任何新的执行权限。重放每20秒续租，正常结束后结算；取消、异常或失租保留不确定检查点，到期恢复会暂停并要求核实，不盲目重放工具副作用。图的工具事件可能延后汇总，因此中途完成事件不等于整段执行可安全重试。 审批快照从工具执行所绑定的图身份创建，不借用环境线程中缺失或属于其他请求的身份。后台重放构建图状态后立即恢复原线程上下文；持久账户、Goal与attempt关联仍在执行时重新校验。 + +选定JSON要求的Goal在Web审批消费时，也按当前请求的认证账户ID持锁复查，直到审批写入事务结束。若请求通过JWT校验后旧账户被停用并由同名新账户取代，旧请求不会消费待审批记录或触发重放；新账户重新登录后仍可批准。拒绝审批同样按当前账户复查。 diff --git a/mateclaw-server/src/test/java/vip/mate/goal/GoalJsonHttpRuntimeIntegrationTest.java b/mateclaw-server/src/test/java/vip/mate/goal/GoalJsonHttpRuntimeIntegrationTest.java index 47ee9273..0fc1ea6a 100644 --- a/mateclaw-server/src/test/java/vip/mate/goal/GoalJsonHttpRuntimeIntegrationTest.java +++ b/mateclaw-server/src/test/java/vip/mate/goal/GoalJsonHttpRuntimeIntegrationTest.java @@ -40,6 +40,7 @@ class GoalJsonHttpRuntimeIntegrationTest { @MockBean private GoalEvaluationService evaluator; @Autowired private GoalContinuationSupervisor supervisor; @MockBean private ProviderChatModelFactory modelFactory; + @org.springframework.boot.test.mock.mockito.SpyBean private vip.mate.workspace.conversation.ConversationService conversationService; @Autowired private JdbcTemplate jdbc; @Autowired private vip.mate.config.LoginRateLimitFilter loginLimiter; @Autowired private vip.mate.llm.failover.AvailableProviderPool providerPool; @@ -84,10 +85,12 @@ class GoalJsonHttpRuntimeIntegrationTest { "false,detached-approval,true", "true,detached-approval,true", "false,scheduled-detached-approval,true", "true,scheduled-detached-approval,true", "false,foreign-approval,true", "true,foreign-approval,true", - "false,scheduled-foreign-approval,true", "true,scheduled-foreign-approval,true"}) + "false,scheduled-foreign-approval,true", "true,scheduled-foreign-approval,true", + "false,scheduled-reassigned-approval,true", "true,scheduled-reassigned-approval,true"}) void authenticatedGoalCompletesThroughHttpOrScheduledProductionRuntime(boolean plan, String entry, boolean accepted) throws Exception { boolean approval = entry.endsWith("approval"); boolean doubleApproval = entry.equals("scheduled-double-approval"); + boolean reassigned = entry.contains("reassigned"); boolean detached = entry.contains("detached"); boolean foreign = entry.contains("foreign"); boolean supervised = entry.startsWith("supervised"); @@ -331,6 +334,32 @@ class GoalJsonHttpRuntimeIntegrationTest { } else assertEquals(userId, approvals.restoreChatOrigin(persistedOrigin).requesterUserId()); Long approvedPlan = plan ? jdbc.queryForObject("SELECT id FROM mate_plan WHERE conversation_id=?", Long.class, conversation) : null; + if (reassigned) { + var replaceOnce = new java.util.concurrent.atomic.AtomicBoolean(true); + doAnswer(invocation -> { + if (replaceOnce.compareAndSet(true, false)) { + long replacementId = IdWorker.getId(); + jdbc.update("UPDATE mate_user SET username=?,deleted=1,enabled=FALSE WHERE id=?", "retired-" + userId, userId); + jdbc.update("INSERT INTO mate_user(id,username,password,enabled,role,create_time,update_time,deleted) VALUES (?,?,?,TRUE,'user',CURRENT_TIMESTAMP,CURRENT_TIMESTAMP,0)", replacementId, username, + new org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder().encode(password)); + jdbc.update("INSERT INTO mate_workspace_member(id,workspace_id,user_id,role,create_time,update_time,deleted) VALUES (?,1,?,'member',CURRENT_TIMESTAMP,CURRENT_TIMESTAMP,0)", IdWorker.getId(), replacementId); + } + return invocation.callRealMethod(); + }).when(conversationService).isConversationOwner(conversation, username); + planApprovalReplay.set(plan); + var oldRequest = HttpRequest.newBuilder(URI.create("http://127.0.0.1:" + port + "/api/v1/chat/stream")) + .timeout(Duration.ofSeconds(45)).header("Content-Type", "application/json") + .header("X-Workspace-Id", "1").header("Authorization", "Bearer " + token) + .POST(HttpRequest.BodyPublishers.ofString(json.writeValueAsString(Map.of("agentId", String.valueOf(agentId), + "conversationId", conversation, "message", "/approve", "pendingApprovalId", pendingId)))).build(); + var rejected = HttpClient.newHttpClient().send(oldRequest, HttpResponse.BodyHandlers.ofString()); + assertFalse(replaceOnce.get(), "Replacement must happen after JWT authentication"); + assertEquals(GoalStatus.ACTIVE, goals.getById(goal.getId()).getStatus(), rejected.body()); + assertEquals("PENDING", jdbc.queryForObject("SELECT status FROM mate_tool_approval WHERE pending_id=?", String.class, pendingId)); + assertEquals(0, jdbc.queryForObject("SELECT COUNT(*) FROM mate_goal_json_artifact WHERE goal_id=?", Integer.class, goal.getId())); + token = request("POST", "/api/v1/auth/login", null, Map.of("username", username, "password", password)).path("data").path("token").asText(); + assertFalse(token.isBlank()); + } planApprovalReplay.set(plan); String replay = requestBody("POST", "/api/v1/chat/stream", token, Map.of("agentId", String.valueOf(agentId), "conversationId", conversation, "message", "/approve", "pendingApprovalId", pendingId)); @@ -374,9 +403,10 @@ class GoalJsonHttpRuntimeIntegrationTest { guardRegistry.reload(); } } else if (queued) { + String queuedToken = token; var response = java.util.concurrent.CompletableFuture.supplyAsync(() -> { try { - return requestBody("POST", "/api/v1/chat/stream", token, + return requestBody("POST", "/api/v1/chat/stream", queuedToken, Map.of("agentId", String.valueOf(agentId), "conversationId", conversation, "message", "Wait for a follow-up fixture.")); } catch (Exception error) { throw new java.util.concurrent.CompletionException(error); } });