From ee0c2df33ab195582d29297cb6a0bbbdb371d5b0 Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Wed, 20 May 2026 13:43:06 +0800 Subject: [PATCH 1/2] persist cancelled tool call status --- .../ConversationPersistenceManagerTests.java | 38 +++++++++++ .../ConversationPersistenceManager.java | 65 +++++++++++++++++++ .../copilot/eclipse/ui/chat/ChatView.java | 2 +- 3 files changed, 104 insertions(+), 1 deletion(-) diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java index 977fb9d2..551da3f3 100644 --- a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java @@ -43,6 +43,7 @@ import com.microsoft.copilot.eclipse.core.lsp.protocol.Turn; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.EditAgentRoundData; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ReplyData; +import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ToolCallData; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ThinkingBlockData; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ThinkingBlockState; import com.microsoft.copilot.eclipse.core.persistence.UserTurnData.MessageData; @@ -311,6 +312,28 @@ void testPersistConversationProgress_Success() throws Exception { verify(mockPersistenceService).saveConversation(any(ConversationData.class)); } + @Test + void testMarkRunningToolCallsCancelledAndPersist_UpdatesOnlyRunningToolCalls() throws Exception { + String conversationId = "00000000-0000-0000-0000-000000000000"; + ConversationData conversationData = createTestConversationData(conversationId); + CopilotTurnData copilotTurnData = (CopilotTurnData) conversationData.getTurns().get(1); + ToolCallData runningToolCall = createTestToolCallData("tool-1", "running"); + ToolCallData completedToolCall = createTestToolCallData("tool-2", "completed"); + EditAgentRoundData roundData = new EditAgentRoundData(); + roundData.setRoundId(1); + roundData.setToolCalls(List.of(runningToolCall, completedToolCall)); + copilotTurnData.getReply().setEditAgentRounds(List.of(roundData)); + + Map cache = getConversationCache(); + cache.put(conversationId, conversationData); + + persistenceManager.markRunningToolCallsCancelledAndPersist(conversationId).get(); + + assertEquals("cancelled", runningToolCall.getStatus()); + assertEquals("completed", completedToolCall.getStatus()); + verify(mockPersistenceService).saveConversation(conversationData); + } + @Test void testUpdateConversationProgress_NewConversation() throws Exception { String conversationId = "00000000-0000-0000-0000-000000000001"; @@ -438,4 +461,19 @@ private void setPrivateField(Object target, String fieldName, Object value) thro field.setAccessible(true); field.set(target, value); } + + private ToolCallData createTestToolCallData(String id, String status) { + ToolCallData toolCallData = new ToolCallData(); + toolCallData.setId(id); + toolCallData.setName("run_in_terminal"); + toolCallData.setProgressMessage("Running command"); + toolCallData.setStatus(status); + return toolCallData; + } + + private Map getConversationCache() throws Exception { + var cacheField = ConversationPersistenceManager.class.getDeclaredField("conversationCache"); + cacheField.setAccessible(true); + return (Map) cacheField.get(persistenceManager); + } } \ No newline at end of file diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java index 5e4d14c2..237122f8 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java @@ -28,6 +28,7 @@ import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ReplyData; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ThinkingBlockData; import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ThinkingBlockState; +import com.microsoft.copilot.eclipse.core.persistence.CopilotTurnData.ToolCallData; import com.microsoft.copilot.eclipse.core.persistence.UserTurnData.MessageData; /** @@ -42,6 +43,8 @@ public class ConversationPersistenceManager { // Maximum number of conversations to keep persisted private static final int MAX_PERSISTED_CONVERSATIONS = 256; + private static final String TOOL_STATUS_RUNNING = "running"; + private static final String TOOL_STATUS_CANCELLED = "cancelled"; /** * Constructor for ConversationPersistenceManager. @@ -270,6 +273,43 @@ public CompletableFuture persistCachedConversation(String conversationId) }); } + /** + * Marks cached running tool calls as cancelled, then persists the cached conversation to disk. + * + * @param conversationId the ID of the cached conversation to persist + * @return a future that completes when the cached conversation has been persisted + */ + public CompletableFuture markRunningToolCallsCancelledAndPersist(String conversationId) { + if (StringUtils.isBlank(conversationId)) { + return CompletableFuture.completedFuture(null); + } + + ConversationData conversationData; + lock.writeLock().lock(); + try { + conversationData = conversationCache.get(conversationId); + if (conversationData == null) { + return CompletableFuture.completedFuture(null); + } + if (!markRunningToolCallsCancelled(conversationData)) { + return CompletableFuture.completedFuture(null); + } + } finally { + lock.writeLock().unlock(); + } + + return CompletableFuture.runAsync(() -> { + lock.writeLock().lock(); + try { + persistAndCacheConversation(conversationData); + } catch (IOException e) { + CopilotCore.LOGGER.error("Failed to persist cancelled tool calls for conversation: " + conversationId, e); + } finally { + lock.writeLock().unlock(); + } + }); + } + /** * Updates a conversation with progress data. This method is synchronous and handles all IO operations internally. */ @@ -498,6 +538,31 @@ private void persistAndCacheConversation(ConversationData conversation) throws I conversationCache.put(conversation.getConversationId(), conversation); } + private boolean markRunningToolCallsCancelled(ConversationData conversationData) { + boolean updated = false; + if (conversationData.getTurns() == null) { + return false; + } + for (AbstractTurnData turn : conversationData.getTurns()) { + if (!(turn instanceof CopilotTurnData copilotTurnData) || copilotTurnData.getReply() == null + || copilotTurnData.getReply().getEditAgentRounds() == null) { + continue; + } + for (EditAgentRoundData round : copilotTurnData.getReply().getEditAgentRounds()) { + if (round.getToolCalls() == null) { + continue; + } + for (ToolCallData toolCall : round.getToolCalls()) { + if (toolCall != null && TOOL_STATUS_RUNNING.equalsIgnoreCase(toolCall.getStatus())) { + toolCall.setStatus(TOOL_STATUS_CANCELLED); + updated = true; + } + } + } + } + return updated; + } + /** * Removes a conversation by ID from both disk and in-memory cache. * diff --git a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java index 49f5bb31..65cf0a74 100644 --- a/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java +++ b/com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/ChatView.java @@ -1281,7 +1281,7 @@ public void onCancel() { this.lastRunSubagentToolCallId = null; if (persistenceManager != null && StringUtils.isNotBlank(this.conversationId)) { - persistenceManager.persistCachedConversation(this.conversationId); + persistenceManager.markRunningToolCallsCancelledAndPersist(this.conversationId); } conversationFutures.forEach(future -> { future.cancel(false); From 20c1625c8e10392bff8537656157844e4fb9eb3a Mon Sep 17 00:00:00 2001 From: xinyi-gong Date: Wed, 20 May 2026 14:11:56 +0800 Subject: [PATCH 2/2] resolve comments --- .../ConversationPersistenceManagerTests.java | 29 ++++++++++++++++--- .../ConversationPersistenceManager.java | 18 ++++-------- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java index 551da3f3..ff213bcc 100644 --- a/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java +++ b/com.microsoft.copilot.eclipse.core.test/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManagerTests.java @@ -38,6 +38,7 @@ import com.microsoft.copilot.eclipse.core.logger.CopilotForEclipseLogger; import com.microsoft.copilot.eclipse.core.lsp.protocol.AgentRound; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatProgressValue; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatStepStatus; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.Thinking; import com.microsoft.copilot.eclipse.core.lsp.protocol.Turn; @@ -317,8 +318,8 @@ void testMarkRunningToolCallsCancelledAndPersist_UpdatesOnlyRunningToolCalls() t String conversationId = "00000000-0000-0000-0000-000000000000"; ConversationData conversationData = createTestConversationData(conversationId); CopilotTurnData copilotTurnData = (CopilotTurnData) conversationData.getTurns().get(1); - ToolCallData runningToolCall = createTestToolCallData("tool-1", "running"); - ToolCallData completedToolCall = createTestToolCallData("tool-2", "completed"); + ToolCallData runningToolCall = createTestToolCallData("tool-1", ChatStepStatus.RUNNING); + ToolCallData completedToolCall = createTestToolCallData("tool-2", ChatStepStatus.COMPLETED); EditAgentRoundData roundData = new EditAgentRoundData(); roundData.setRoundId(1); roundData.setToolCalls(List.of(runningToolCall, completedToolCall)); @@ -329,8 +330,28 @@ void testMarkRunningToolCallsCancelledAndPersist_UpdatesOnlyRunningToolCalls() t persistenceManager.markRunningToolCallsCancelledAndPersist(conversationId).get(); - assertEquals("cancelled", runningToolCall.getStatus()); - assertEquals("completed", completedToolCall.getStatus()); + assertEquals(ChatStepStatus.CANCELLED, runningToolCall.getStatus()); + assertEquals(ChatStepStatus.COMPLETED, completedToolCall.getStatus()); + verify(mockPersistenceService).saveConversation(conversationData); + } + + @Test + void testMarkRunningToolCallsCancelledAndPersist_PersistsWhenNoRunningToolCalls() throws Exception { + String conversationId = "00000000-0000-0000-0000-000000000000"; + ConversationData conversationData = createTestConversationData(conversationId); + CopilotTurnData copilotTurnData = (CopilotTurnData) conversationData.getTurns().get(1); + ToolCallData completedToolCall = createTestToolCallData("tool-1", ChatStepStatus.COMPLETED); + EditAgentRoundData roundData = new EditAgentRoundData(); + roundData.setRoundId(1); + roundData.setToolCalls(List.of(completedToolCall)); + copilotTurnData.getReply().setEditAgentRounds(List.of(roundData)); + + Map cache = getConversationCache(); + cache.put(conversationId, conversationData); + + persistenceManager.markRunningToolCallsCancelledAndPersist(conversationId).get(); + + assertEquals(ChatStepStatus.COMPLETED, completedToolCall.getStatus()); verify(mockPersistenceService).saveConversation(conversationData); } diff --git a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java index 237122f8..6916e15c 100644 --- a/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java +++ b/com.microsoft.copilot.eclipse.core/src/com/microsoft/copilot/eclipse/core/persistence/ConversationPersistenceManager.java @@ -20,6 +20,7 @@ import com.microsoft.copilot.eclipse.core.AuthStatusManager; import com.microsoft.copilot.eclipse.core.CopilotCore; import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatProgressValue; +import com.microsoft.copilot.eclipse.core.lsp.protocol.ChatStepStatus; import com.microsoft.copilot.eclipse.core.lsp.protocol.CopilotModel; import com.microsoft.copilot.eclipse.core.lsp.protocol.TodoItem; import com.microsoft.copilot.eclipse.core.lsp.protocol.Turn; @@ -43,8 +44,6 @@ public class ConversationPersistenceManager { // Maximum number of conversations to keep persisted private static final int MAX_PERSISTED_CONVERSATIONS = 256; - private static final String TOOL_STATUS_RUNNING = "running"; - private static final String TOOL_STATUS_CANCELLED = "cancelled"; /** * Constructor for ConversationPersistenceManager. @@ -291,9 +290,7 @@ public CompletableFuture markRunningToolCallsCancelledAndPersist(String co if (conversationData == null) { return CompletableFuture.completedFuture(null); } - if (!markRunningToolCallsCancelled(conversationData)) { - return CompletableFuture.completedFuture(null); - } + markRunningToolCallsCancelled(conversationData); } finally { lock.writeLock().unlock(); } @@ -538,10 +535,9 @@ private void persistAndCacheConversation(ConversationData conversation) throws I conversationCache.put(conversation.getConversationId(), conversation); } - private boolean markRunningToolCallsCancelled(ConversationData conversationData) { - boolean updated = false; + private void markRunningToolCallsCancelled(ConversationData conversationData) { if (conversationData.getTurns() == null) { - return false; + return; } for (AbstractTurnData turn : conversationData.getTurns()) { if (!(turn instanceof CopilotTurnData copilotTurnData) || copilotTurnData.getReply() == null @@ -553,14 +549,12 @@ private boolean markRunningToolCallsCancelled(ConversationData conversationData) continue; } for (ToolCallData toolCall : round.getToolCalls()) { - if (toolCall != null && TOOL_STATUS_RUNNING.equalsIgnoreCase(toolCall.getStatus())) { - toolCall.setStatus(TOOL_STATUS_CANCELLED); - updated = true; + if (toolCall != null && ChatStepStatus.RUNNING.equalsIgnoreCase(toolCall.getStatus())) { + toolCall.setStatus(ChatStepStatus.CANCELLED); } } } } - return updated; } /**