Skip to content

Commit 91bbe3e

Browse files
committed
Merge remote-tracking branch 'new-repo/main' into tori/persist-subagent
# Conflicts: # com.microsoft.copilot.eclipse.ui/src/com/microsoft/copilot/eclipse/ui/chat/BaseTurnWidget.java
2 parents b0265d9 + 49f0123 commit 91bbe3e

4 files changed

Lines changed: 345 additions & 9 deletions

File tree

Lines changed: 284 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,284 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
package com.microsoft.copilot.eclipse.ui.chat;
5+
6+
import static org.junit.jupiter.api.Assertions.assertFalse;
7+
import static org.junit.jupiter.api.Assertions.assertNotNull;
8+
import static org.junit.jupiter.api.Assertions.assertNull;
9+
import static org.junit.jupiter.api.Assertions.assertTrue;
10+
import static org.mockito.Mockito.lenient;
11+
import static org.mockito.Mockito.mock;
12+
import static org.mockito.Mockito.mockStatic;
13+
14+
import java.lang.reflect.Field;
15+
import java.util.HashMap;
16+
import java.util.Map;
17+
18+
import org.eclipse.swt.SWT;
19+
import org.eclipse.swt.custom.StyledText;
20+
import org.eclipse.swt.widgets.Display;
21+
import org.eclipse.swt.widgets.Shell;
22+
import org.junit.jupiter.api.AfterEach;
23+
import org.junit.jupiter.api.BeforeEach;
24+
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.api.extension.ExtendWith;
26+
import org.mockito.Mock;
27+
import org.mockito.MockedStatic;
28+
import org.mockito.junit.jupiter.MockitoExtension;
29+
30+
import com.google.gson.Gson;
31+
import com.microsoft.copilot.eclipse.core.lsp.protocol.AgentToolCall;
32+
import com.microsoft.copilot.eclipse.ui.CopilotUi;
33+
import com.microsoft.copilot.eclipse.ui.chat.services.AvatarService;
34+
import com.microsoft.copilot.eclipse.ui.chat.services.ChatFontService;
35+
import com.microsoft.copilot.eclipse.ui.chat.services.ChatServiceManager;
36+
import com.microsoft.copilot.eclipse.ui.utils.SwtUtils;
37+
38+
/**
39+
* Verifies that {@link BaseTurnWidget#appendToolCallStatus} renders text for
40+
* tool calls that finish with status {@code error}.
41+
*/
42+
@ExtendWith(MockitoExtension.class)
43+
class BaseTurnWidgetToolCallStatusTest {
44+
45+
private static final String TURN_ID = "turn-1";
46+
private static final String TOOL_CALL_ID = "tool-call-1";
47+
private static final String TOOL_NAME = "edit_file";
48+
private static final String SUBAGENT_TOOL_NAME = "run_subagent";
49+
private static final String ERROR_MESSAGE = "file not found";
50+
private static final String PROGRESS_MESSAGE = "Editing file.txt";
51+
52+
private Shell shell;
53+
private MockedStatic<CopilotUi> copilotUiMock;
54+
private CopilotUi mockPlugin;
55+
56+
@Mock
57+
private ChatServiceManager mockChatServiceManager;
58+
@Mock
59+
private AvatarService mockAvatarService;
60+
@Mock
61+
private ChatFontService mockChatFontService;
62+
63+
@BeforeEach
64+
void setUp() {
65+
lenient().when(mockChatServiceManager.getAvatarService()).thenReturn(mockAvatarService);
66+
lenient().when(mockChatServiceManager.getChatFontService()).thenReturn(mockChatFontService);
67+
lenient().when(mockAvatarService.getAvatarForCopilot()).thenReturn(null);
68+
69+
// Mockito's static mocks are scoped to the registering thread. The widgets under
70+
// test execute on the SWT Display thread (via SwtUtils.invokeOnDisplayThread), so
71+
// the mock must be registered there as well; otherwise CopilotUi.getPlugin() calls
72+
// from the display thread would not see it.
73+
SwtUtils.invokeOnDisplayThread(() -> {
74+
shell = new Shell(Display.getDefault());
75+
copilotUiMock = mockStatic(CopilotUi.class);
76+
mockPlugin = mock(CopilotUi.class);
77+
copilotUiMock.when(CopilotUi::getPlugin).thenReturn(mockPlugin);
78+
lenient().when(mockPlugin.getChatServiceManager()).thenReturn(mockChatServiceManager);
79+
});
80+
}
81+
82+
@AfterEach
83+
void tearDown() {
84+
SwtUtils.invokeOnDisplayThread(() -> {
85+
if (copilotUiMock != null) {
86+
copilotUiMock.close();
87+
copilotUiMock = null;
88+
}
89+
if (shell != null && !shell.isDisposed()) {
90+
shell.dispose();
91+
}
92+
});
93+
}
94+
95+
@Test
96+
void appendToolCallStatus_errorWithMessage_rendersErrorText() {
97+
SwtUtils.invokeOnDisplayThread(() -> {
98+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
99+
100+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "error", PROGRESS_MESSAGE, ERROR_MESSAGE));
101+
102+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
103+
assertNotNull(text, "Status label text must be created when an error event is delivered");
104+
assertTrue(text.getText().contains(ERROR_MESSAGE),
105+
"Expected error message to be rendered next to the error icon, got: '" + text.getText() + "'");
106+
assertTrue(text.getText().contains(TOOL_NAME),
107+
"Expected error text to be prefixed with the tool name, got: '" + text.getText() + "'");
108+
});
109+
}
110+
111+
@Test
112+
void appendToolCallStatus_errorWithoutProgressMessage_isStillProcessed() {
113+
SwtUtils.invokeOnDisplayThread(() -> {
114+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
115+
116+
// No progressMessage (e.g., tool failed before the running event was dispatched server-side).
117+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "error", null, ERROR_MESSAGE));
118+
119+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
120+
assertNotNull(text, "Error event without progressMessage must still be processed");
121+
assertTrue(text.getText().contains(ERROR_MESSAGE),
122+
"Expected error message to be rendered, got: '" + text.getText() + "'");
123+
});
124+
}
125+
126+
@Test
127+
void appendToolCallStatus_runningThenError_replacesProgressTextWithErrorText() {
128+
SwtUtils.invokeOnDisplayThread(() -> {
129+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
130+
131+
// First the running event sets the progress text.
132+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "running", PROGRESS_MESSAGE, null));
133+
StyledText runningText = getStatusLabelText(widget, TOOL_CALL_ID);
134+
assertNotNull(runningText, "Running event should have populated text");
135+
assertTrue(runningText.getText().contains(PROGRESS_MESSAGE),
136+
"Running text should be visible before failure, got: '" + runningText.getText() + "'");
137+
138+
// Then the same tool call fails: the error text must overwrite the stale running text.
139+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "error", PROGRESS_MESSAGE, ERROR_MESSAGE));
140+
141+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
142+
assertNotNull(text);
143+
assertTrue(text.getText().contains(ERROR_MESSAGE),
144+
"Error text should replace stale running text, got: '" + text.getText() + "'");
145+
});
146+
}
147+
148+
@Test
149+
void appendToolCallStatus_errorFallsBackToProgressMessage_whenErrorIsBlank() {
150+
SwtUtils.invokeOnDisplayThread(() -> {
151+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
152+
153+
// Whitespace-only error must fall back to progressMessage rather than rendering empty text.
154+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "error", PROGRESS_MESSAGE, " "));
155+
156+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
157+
assertNotNull(text, "Status label text must be created on error even when only progressMessage is present");
158+
assertTrue(text.getText().contains(PROGRESS_MESSAGE),
159+
"Expected progressMessage to be used as fallback, got: '" + text.getText() + "'");
160+
});
161+
}
162+
163+
@Test
164+
void appendToolCallStatus_errorFallsBackToProgressMessage_whenErrorIsNull() {
165+
SwtUtils.invokeOnDisplayThread(() -> {
166+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
167+
168+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "error", PROGRESS_MESSAGE, null));
169+
170+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
171+
assertNotNull(text);
172+
assertTrue(text.getText().contains(PROGRESS_MESSAGE),
173+
"Expected progressMessage to be used as fallback, got: '" + text.getText() + "'");
174+
});
175+
}
176+
177+
@Test
178+
void appendToolCallStatus_errorWithBothFieldsBlank_rendersGenericFallbackWithToolName() {
179+
SwtUtils.invokeOnDisplayThread(() -> {
180+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
181+
182+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "error", null, null));
183+
184+
StyledText text = getStatusLabelText(widget, TOOL_CALL_ID);
185+
assertNotNull(text, "Error event with no fields must still surface a generic message");
186+
assertTrue(text.getText().contains(TOOL_NAME),
187+
"Generic fallback should still tell the user which tool failed, got: '" + text.getText() + "'");
188+
});
189+
}
190+
191+
@Test
192+
void appendToolCallStatus_nonErrorStatusWithOnlyError_isIgnored() {
193+
SwtUtils.invokeOnDisplayThread(() -> {
194+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
195+
196+
// Non-error events with only `error` populated must be dropped; otherwise
197+
// setRunningStatus/setCompletedStatus/setText would call setMarkup(null).
198+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "running", null, ERROR_MESSAGE));
199+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "completed", " ", ERROR_MESSAGE));
200+
widget.appendToolCallStatus(buildToolCall(TOOL_NAME, "cancelled", null, ERROR_MESSAGE));
201+
202+
@SuppressWarnings("unchecked")
203+
Map<String, AgentStatusLabel> labels = (Map<String, AgentStatusLabel>) getField(widget, "statusLabels");
204+
assertTrue(labels.isEmpty(),
205+
"Non-error events without progressMessage should be ignored, got: " + labels.keySet());
206+
});
207+
}
208+
209+
/**
210+
* Regression: a {@code run_subagent} terminal event with no progressMessage must still
211+
* clear the subagent block state. Otherwise subsequent messages get routed to a ghost
212+
* {@code currentSubagentBlock} (see review comment on PR #145).
213+
*/
214+
@Test
215+
void appendToolCallStatus_subagentTerminalEventWithBlankMessage_clearsSubagentState() {
216+
SwtUtils.invokeOnDisplayThread(() -> {
217+
CopilotTurnWidget widget = new CopilotTurnWidget(shell, SWT.NONE, mockChatServiceManager, TURN_ID);
218+
219+
// Open a subagent block.
220+
widget.appendToolCallStatus(buildToolCall(SUBAGENT_TOOL_NAME, "running", null, null));
221+
assertTrue((boolean) getField(widget, "inSubagentBlock"),
222+
"Subagent block should be active after a 'running' event");
223+
assertNotNull(getField(widget, "currentSubagentBlock"),
224+
"currentSubagentBlock should be populated after a 'running' event");
225+
226+
// Terminal event with no display message — the early-return guard for blank
227+
// progressMessage must NOT short-circuit subagent state cleanup.
228+
widget.appendToolCallStatus(buildToolCall(SUBAGENT_TOOL_NAME, "completed", null, null));
229+
230+
assertFalse((boolean) getField(widget, "inSubagentBlock"),
231+
"inSubagentBlock should be cleared after a terminal subagent event");
232+
assertNull(getField(widget, "currentSubagentBlock"),
233+
"currentSubagentBlock should be cleared after a terminal subagent event");
234+
});
235+
}
236+
237+
private static AgentToolCall buildToolCall(String name, String status, String progressMessage, String error) {
238+
Gson gson = new Gson();
239+
Map<String, Object> fields = new HashMap<>();
240+
fields.put("id", TOOL_CALL_ID);
241+
fields.put("name", name);
242+
fields.put("status", status);
243+
if (progressMessage != null) {
244+
fields.put("progressMessage", progressMessage);
245+
}
246+
if (error != null) {
247+
fields.put("error", error);
248+
}
249+
return gson.fromJson(gson.toJson(fields), AgentToolCall.class);
250+
}
251+
252+
private static StyledText getStatusLabelText(BaseTurnWidget widget, String toolCallId) {
253+
@SuppressWarnings("unchecked")
254+
Map<String, AgentStatusLabel> labels = (Map<String, AgentStatusLabel>) getField(widget, "statusLabels");
255+
AgentStatusLabel label = labels.get(toolCallId);
256+
assertNotNull(label, "AgentStatusLabel for tool call '" + toolCallId + "' should have been created");
257+
Object markupViewer = getField(label, "textLabel");
258+
if (markupViewer == null) {
259+
return null;
260+
}
261+
try {
262+
return (StyledText) markupViewer.getClass().getMethod("getTextWidget").invoke(markupViewer);
263+
} catch (ReflectiveOperationException e) {
264+
throw new RuntimeException("Failed to obtain StyledText from textLabel", e);
265+
}
266+
}
267+
268+
private static Object getField(Object target, String name) {
269+
Class<?> cls = target.getClass();
270+
while (cls != null) {
271+
try {
272+
Field f = cls.getDeclaredField(name);
273+
f.setAccessible(true);
274+
return f.get(target);
275+
} catch (NoSuchFieldException e) {
276+
cls = cls.getSuperclass();
277+
} catch (IllegalAccessException e) {
278+
throw new RuntimeException(e);
279+
}
280+
}
281+
throw new RuntimeException("Field '" + name + "' not found on " + target.getClass());
282+
}
283+
}
284+

0 commit comments

Comments
 (0)