ai-space/goose
3
0
Fork 0
mirror of https://github.com/aaif-goose/goose.git synced 2026-07-03 14:10:03 +02:00
Code Issues Packages Projects Releases Wiki Activity

Clean room cleanup and resume chat in same window (#5083)

Browse Source
This commit is contained in:
Zane
2025-10-08 16:10:40 -07:00
committed by GitHub
parent 3ebe311f8b
commit 06231fbb71
8 changed files with 124 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/desktop/src/App.tsx
+1 -2
View File
@@ -44,7 +44,6 @@ import {
useAgent,
} from './hooks/useAgent';
import { useNavigation } from './hooks/useNavigation';
import { USE_NEW_CHAT } from './updates';
import Pair2 from './components/Pair2';
// Route Components
@@ -95,7 +94,7 @@ const PairRouteWrapper = ({
const resumeSessionId = searchParams.get('resumeSessionId') ?? undefined;
return USE_NEW_CHAT ? (
return process.env.ALPHA ? (
<Pair2
chat={chat}
setChat={setChat}
ui/desktop/src/components/BaseChat2.tsx
+7
View File
@@ -74,6 +74,13 @@ function BaseChatContent({
const [messages, setMessages] = useState(chat?.messages || []);
// Update messages when chat changes (e.g., when resuming a session)
useEffect(() => {
if (chat?.messages) {
setMessages(chat.messages);
}
}, [chat?.messages, chat?.sessionId]);
const { chatState, handleSubmit, stopStreaming } = useChatStream({
sessionId: chat?.sessionId || '',
messages,
ui/desktop/src/components/Pair2.tsx
-1
View File
@@ -48,7 +48,6 @@ export default function Pair({
return prev;
});
} catch (error) {
console.log(error);
setFatalError(`Agent init failure: ${error instanceof Error ? error.message : '' + error}`);
}
};
ui/desktop/src/components/ToolCallWithResponse.tsx
+10 -3
View File
@@ -42,8 +42,15 @@ export default function ToolCallWithResponse({
isStreamingMessage,
append,
}: ToolCallWithResponseProps) {
const toolCall = toolRequest.toolCall as { name: string; arguments: Record<string, unknown> };
if (!toolCall) {
// Handle both the wrapped ToolResult format and the unwrapped format
// The server serializes ToolResult<T> as { status: "success", value: T } or { status: "error", error: string }
const toolCallData = toolRequest.toolCall as Record<string, unknown>;
const toolCall =
toolCallData?.status === 'success'
? (toolCallData.value as { name: string; arguments: Record<string, unknown> })
: (toolCallData as { name: string; arguments: Record<string, unknown> });
if (!toolCall || !toolCall.name) {
return null;
}
@@ -215,7 +222,7 @@ function ToolCallView({
}
})();
const isToolDetails = Object.entries(toolCall?.arguments).length > 0;
const isToolDetails = toolCall?.arguments && Object.entries(toolCall.arguments).length > 0;
// Check if streaming has finished but no tool response was received
// This is a workaround for cases where the backend doesn't send tool responses
ui/desktop/src/components/context_management/__tests__/ContextManager.test.tsx
+87 -95
View File
@@ -7,7 +7,6 @@ import { ContextManageResponse, Message } from '../../../api';
// Mock the context management functions
vi.mock('../index', () => ({
manageContextFromBackend: vi.fn(),
convertApiMessageToFrontendMessage: vi.fn(),
}));
const mockManageContextFromBackend = vi.mocked(contextManagement.manageContextFromBackend);
@@ -28,13 +27,6 @@ describe('ContextManager', () => {
},
];
const mockSummaryMessage: Message = {
id: 'summary-1',
role: 'assistant',
created: 3000,
content: [{ type: 'text', text: 'This is a summary of the conversation.' }],
};
const mockSetMessages = vi.fn();
const mockAppend = vi.fn();
@@ -109,6 +101,7 @@ describe('ContextManager', () => {
describe('handleAutoCompaction', () => {
it('should successfully perform auto compaction with server-provided messages', async () => {
// Mock the backend response with 3 messages: marker, summary, continuation
// Note: Server messages may not have id/created, which will be added by the code
mockManageContextFromBackend.mockResolvedValue({
messages: [
{
@@ -116,11 +109,11 @@ describe('ContextManager', () => {
content: [
{ type: 'summarizationRequested', msg: 'Conversation compacted and summarized' },
],
},
} as Message,
{
role: 'assistant',
content: [{ type: 'text', text: 'Summary content' }],
},
} as Message,
{
role: 'assistant',
content: [
@@ -129,30 +122,11 @@ describe('ContextManager', () => {
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
},
} as Message,
],
tokenCounts: [8, 100, 50],
});
const mockCompactionMarker: Message = {
id: 'marker-1',
role: 'assistant',
created: 3000,
content: [{ type: 'summarizationRequested', msg: 'Conversation compacted and summarized' }],
};
const mockContinuationMessage: Message = {
id: 'continuation-1',
role: 'assistant',
created: 3000,
content: [
{
type: 'text',
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
};
const { result } = renderContextManager();
await act(async () => {
@@ -170,12 +144,28 @@ describe('ContextManager', () => {
sessionId: 'test-session-id',
});
// Expect setMessages to be called with all 3 converted messages
expect(mockSetMessages).toHaveBeenCalledWith([
mockCompactionMarker,
mockSummaryMessage,
mockContinuationMessage,
]);
// Expect setMessages to be called with all 3 messages from server
// Note: Server doesn't provide id/created fields, so we don't check for them
expect(mockSetMessages).toHaveBeenCalledTimes(1);
const setMessagesCall = mockSetMessages.mock.calls[0][0];
expect(setMessagesCall).toHaveLength(3);
expect(setMessagesCall[0]).toMatchObject({
role: 'assistant',
content: [{ type: 'summarizationRequested', msg: 'Conversation compacted and summarized' }],
});
expect(setMessagesCall[1]).toMatchObject({
role: 'assistant',
content: [{ type: 'text', text: 'Summary content' }],
});
expect(setMessagesCall[2]).toMatchObject({
role: 'assistant',
content: [
{
type: 'text',
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
});
// Fast-forward timers to trigger the append call
act(() => {
@@ -184,7 +174,16 @@ describe('ContextManager', () => {
// Should append the continuation message (index 2) for auto-compaction
expect(mockAppend).toHaveBeenCalledTimes(1);
expect(mockAppend).toHaveBeenCalledWith(mockContinuationMessage);
const appendedMessage = mockAppend.mock.calls[0][0];
expect(appendedMessage).toMatchObject({
role: 'assistant',
content: [
{
type: 'text',
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
});
});
it('should handle compaction errors gracefully', async () => {
@@ -324,25 +323,6 @@ describe('ContextManager', () => {
tokenCounts: [8, 100, 50],
});
const mockCompactionMarker: Message = {
id: 'marker-1',
role: 'assistant',
created: 3000,
content: [{ type: 'summarizationRequested', msg: 'Conversation compacted and summarized' }],
};
const mockContinuationMessage: Message = {
id: 'continuation-1',
role: 'assistant',
created: 3000,
content: [
{
type: 'text',
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
};
const { result } = renderContextManager();
await act(async () => {
@@ -361,11 +341,26 @@ describe('ContextManager', () => {
});
// Verify all three messages are set
expect(mockSetMessages).toHaveBeenCalledWith([
mockCompactionMarker,
mockSummaryMessage,
mockContinuationMessage,
]);
expect(mockSetMessages).toHaveBeenCalledTimes(1);
const setMessagesCall = mockSetMessages.mock.calls[0][0];
expect(setMessagesCall).toHaveLength(3);
expect(setMessagesCall[0]).toMatchObject({
role: 'assistant',
content: [{ type: 'summarizationRequested', msg: 'Conversation compacted and summarized' }],
});
expect(setMessagesCall[1]).toMatchObject({
role: 'assistant',
content: [{ type: 'text', text: 'Manual summary content' }],
});
expect(setMessagesCall[2]).toMatchObject({
role: 'assistant',
content: [
{
type: 'text',
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
});
// Fast-forward timers to check if append would be called
act(() => {
@@ -435,25 +430,6 @@ describe('ContextManager', () => {
tokenCounts: [8, 100, 50],
});
const mockCompactionMarker: Message = {
id: 'marker-1',
role: 'assistant',
created: 3000,
content: [{ type: 'summarizationRequested', msg: 'Conversation compacted and summarized' }],
};
const mockContinuationMessage: Message = {
id: 'continuation-1',
role: 'assistant',
created: 3000,
content: [
{
type: 'text',
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
};
const { result } = renderContextManager();
await act(async () => {
@@ -466,11 +442,26 @@ describe('ContextManager', () => {
});
// Verify all three messages are set
expect(mockSetMessages).toHaveBeenCalledWith([
mockCompactionMarker,
mockSummaryMessage,
mockContinuationMessage,
]);
expect(mockSetMessages).toHaveBeenCalledTimes(1);
const setMessagesCall = mockSetMessages.mock.calls[0][0];
expect(setMessagesCall).toHaveLength(3);
expect(setMessagesCall[0]).toMatchObject({
role: 'assistant',
content: [{ type: 'summarizationRequested', msg: 'Conversation compacted and summarized' }],
});
expect(setMessagesCall[1]).toMatchObject({
role: 'assistant',
content: [{ type: 'text', text: 'Manual summary content' }],
});
expect(setMessagesCall[2]).toMatchObject({
role: 'assistant',
content: [
{
type: 'text',
text: 'The previous message contains a summary that was prepared because a context limit was reached. Do not mention that you read a summary or that conversation summarization occurred Just continue the conversation naturally based on the summarized context',
},
],
});
// Fast-forward timers to check if append would be called
act(() => {
@@ -508,18 +499,11 @@ describe('ContextManager', () => {
content: [
{ type: 'toolResponse', id: 'test', toolResult: { content: 'Not text content' } },
],
},
} as Message,
],
tokenCounts: [100, 50],
});
const mockMessageWithoutText: Message = {
id: 'summary-1',
role: 'assistant',
created: 3000,
content: [{ type: 'toolResponse', id: 'test', toolResult: { status: 'success' } }],
};
const { result } = renderContextManager();
await act(async () => {
@@ -535,8 +519,16 @@ describe('ContextManager', () => {
expect(result.current.isCompacting).toBe(false);
expect(result.current.compactionError).toBe(null);
// Should still set messages with the converted message
expect(mockSetMessages).toHaveBeenCalledWith([mockMessageWithoutText]);
// Should still set messages from server
expect(mockSetMessages).toHaveBeenCalledTimes(1);
const setMessagesCall = mockSetMessages.mock.calls[0][0];
expect(setMessagesCall).toHaveLength(1);
expect(setMessagesCall[0]).toMatchObject({
role: 'assistant',
content: [
{ type: 'toolResponse', id: 'test', toolResult: { content: 'Not text content' } },
],
});
});
});
ui/desktop/src/components/sessions/SessionsInsights.tsx
+3 -1
View File
@@ -86,7 +86,9 @@ export function SessionInsights() {
const handleSessionClick = async (session: Session) => {
try {
resumeSession(session);
resumeSession(session, (sessionId: string) => {
navigate(`/pair?resumeSessionId=${sessionId}`);
});
} catch (error) {
console.error('Failed to start session:', error);
navigate('/sessions', {
ui/desktop/src/sessions.ts
+16 -8
View File
@@ -1,16 +1,24 @@
import { Session } from './api';
export function resumeSession(session: Session) {
console.log('Launching session in new window:', session.description || session.id);
export function resumeSession(
session: Session,
navigateInSameWindow?: (sessionId: string) => void
) {
const workingDir = session.working_dir;
if (!workingDir) {
throw new Error('Cannot resume session: working directory is missing in session');
}
window.electron.createChatWindow(
undefined, // query
workingDir,
undefined, // version
session.id
);
// When ALPHA is true and we have a navigation callback, resume in the same window
// Otherwise, open in a new window (old behavior)
if (process.env.ALPHA && navigateInSameWindow) {
navigateInSameWindow(session.id);
} else {
window.electron.createChatWindow(
undefined, // query
workingDir,
undefined, // version
session.id
);
}
}
ui/desktop/src/updates.ts
-2
View File
@@ -2,5 +2,3 @@ export const UPDATES_ENABLED = true;
export const COST_TRACKING_ENABLED = true;
export const ANNOUNCEMENTS_ENABLED = false;
export const CONFIGURATION_ENABLED = true;
export const USE_NEW_CHAT = true;