Skip to content

Commit 10d7cda

Browse files
committed
address comments
1 parent 94223b9 commit 10d7cda

8 files changed

Lines changed: 464 additions & 38 deletions

File tree

apps/sim/app/api/copilot/chat/stop/route.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,5 +207,43 @@ describe('copilot chat stop route', () => {
207207
role: 'assistant',
208208
content: 'partial',
209209
})
210+
211+
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
212+
workspaceId: 'ws-1',
213+
chatId: 'chat-1',
214+
type: 'completed',
215+
streamId: 'stream-1',
216+
})
217+
})
218+
219+
it('republishes completed status when the assistant was already persisted', async () => {
220+
mockLimit.mockResolvedValueOnce([
221+
{
222+
workspaceId: 'ws-1',
223+
messages: [
224+
{ id: 'stream-1', role: 'user', content: 'hello' },
225+
{ id: 'assistant-1', role: 'assistant', content: 'partial' },
226+
],
227+
conversationId: null,
228+
},
229+
])
230+
231+
const response = await POST(
232+
createRequest({
233+
chatId: 'chat-1',
234+
streamId: 'stream-1',
235+
content: 'partial',
236+
})
237+
)
238+
239+
expect(response.status).toBe(200)
240+
expect(await response.json()).toEqual({ success: true })
241+
expect(mockUpdate).not.toHaveBeenCalled()
242+
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
243+
workspaceId: 'ws-1',
244+
chatId: 'chat-1',
245+
type: 'completed',
246+
streamId: 'stream-1',
247+
})
210248
})
211249
})

apps/sim/app/api/copilot/chat/stop/route.ts

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,16 @@ import { type NextRequest, NextResponse } from 'next/server'
44
import { copilotChatStopContract } from '@/lib/api/contracts/copilot'
55
import { parseRequest } from '@/lib/api/server'
66
import { getSession } from '@/lib/auth'
7-
import { normalizeMessage, type PersistedMessage } from '@/lib/copilot/chat/persisted-message'
7+
import {
8+
normalizeMessage,
9+
type PersistedMessage,
10+
withStoppedContentBlock,
11+
} from '@/lib/copilot/chat/persisted-message'
812
import { finalizeAssistantTurn } from '@/lib/copilot/chat/terminal-state'
9-
import { CopilotStopOutcome } from '@/lib/copilot/generated/trace-attribute-values-v1'
13+
import {
14+
CopilotChatFinalizeOutcome,
15+
CopilotStopOutcome,
16+
} from '@/lib/copilot/generated/trace-attribute-values-v1'
1017
import { TraceAttr } from '@/lib/copilot/generated/trace-attributes-v1'
1118
import { TraceSpan } from '@/lib/copilot/generated/trace-spans-v1'
1219
import { withIncomingGoSpan } from '@/lib/copilot/request/otel'
@@ -49,14 +56,16 @@ export const POST = withRouteHandler((req: NextRequest) =>
4956
: hasContent
5057
? [{ type: 'text', channel: 'assistant', content }, { type: 'stopped' }]
5158
: [{ type: 'stopped' }]
52-
const assistantMessage: PersistedMessage = normalizeMessage({
53-
id: generateId(),
54-
role: 'assistant',
55-
content,
56-
timestamp: new Date().toISOString(),
57-
contentBlocks: synthesizedStoppedBlocks,
58-
...(requestId ? { requestId } : {}),
59-
})
59+
const assistantMessage: PersistedMessage = withStoppedContentBlock(
60+
normalizeMessage({
61+
id: generateId(),
62+
role: 'assistant',
63+
content,
64+
timestamp: new Date().toISOString(),
65+
contentBlocks: synthesizedStoppedBlocks,
66+
...(requestId ? { requestId } : {}),
67+
})
68+
)
6069
const result = await finalizeAssistantTurn({
6170
chatId,
6271
userId: session.user.id,
@@ -65,8 +74,15 @@ export const POST = withRouteHandler((req: NextRequest) =>
6574
streamMarkerPolicy: 'active-or-cleared',
6675
})
6776
span.setAttribute(TraceAttr.CopilotStopAppendedAssistant, result.appendedAssistant)
77+
const stopOutcome = !result.found
78+
? CopilotStopOutcome.ChatNotFound
79+
: result.updated || result.outcome === CopilotChatFinalizeOutcome.AssistantAlreadyPersisted
80+
? CopilotStopOutcome.Persisted
81+
: CopilotStopOutcome.NoMatchingRow
82+
const shouldPublishCompleted =
83+
result.updated || result.outcome === CopilotChatFinalizeOutcome.AssistantAlreadyPersisted
6884

69-
if (result.updated && result.workspaceId) {
85+
if (shouldPublishCompleted && result.workspaceId) {
7086
taskPubSub?.publishStatusChanged({
7187
workspaceId: result.workspaceId,
7288
chatId,
@@ -75,14 +91,7 @@ export const POST = withRouteHandler((req: NextRequest) =>
7591
})
7692
}
7793

78-
span.setAttribute(
79-
TraceAttr.CopilotStopOutcome,
80-
result.found
81-
? result.updated
82-
? CopilotStopOutcome.Persisted
83-
: CopilotStopOutcome.NoMatchingRow
84-
: CopilotStopOutcome.ChatNotFound
85-
)
94+
span.setAttribute(TraceAttr.CopilotStopOutcome, stopOutcome)
8695
return NextResponse.json({ success: true })
8796
} catch (error) {
8897
logger.error('Error stopping chat stream:', error)

apps/sim/lib/copilot/chat/persisted-message.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,45 @@ export function buildPersistedAssistantMessage(
224224
return message
225225
}
226226

227+
export function withStoppedContentBlock(message: PersistedMessage): PersistedMessage {
228+
const contentBlocks = message.contentBlocks ?? []
229+
const hasAssistantText = contentBlocks.some(
230+
(block) =>
231+
block.type === MothershipStreamV1EventType.text &&
232+
block.channel !== MothershipStreamV1TextChannel.thinking &&
233+
block.content?.trim()
234+
)
235+
if (
236+
contentBlocks.some(
237+
(block) =>
238+
block.type === MothershipStreamV1EventType.complete &&
239+
block.status === MothershipStreamV1CompletionStatus.cancelled
240+
)
241+
) {
242+
return message
243+
}
244+
245+
return normalizeMessage({
246+
...message,
247+
contentBlocks: [
248+
...(hasAssistantText || !message.content.trim()
249+
? []
250+
: [
251+
{
252+
type: MothershipStreamV1EventType.text,
253+
channel: MothershipStreamV1TextChannel.assistant,
254+
content: message.content,
255+
},
256+
]),
257+
...contentBlocks,
258+
{
259+
type: MothershipStreamV1EventType.complete,
260+
status: MothershipStreamV1CompletionStatus.cancelled,
261+
},
262+
],
263+
})
264+
}
265+
227266
export interface UserMessageParams {
228267
id: string
229268
content: string

apps/sim/lib/copilot/chat/post.test.ts

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ const {
2727
getPendingChatStreamId,
2828
releasePendingChatStream,
2929
resolveOrCreateChat,
30+
finalizeAssistantTurn,
31+
mockPublishStatusChanged,
3032
} = vi.hoisted(() => ({
3133
getEffectiveDecryptedEnv: vi.fn(),
3234
generateWorkspaceContext: vi.fn(),
@@ -38,6 +40,8 @@ const {
3840
getPendingChatStreamId: vi.fn(),
3941
releasePendingChatStream: vi.fn(),
4042
resolveOrCreateChat: vi.fn(),
43+
finalizeAssistantTurn: vi.fn(),
44+
mockPublishStatusChanged: vi.fn(),
4145
}))
4246

4347
const getSession = authMockFns.mockGetSession
@@ -78,9 +82,13 @@ vi.mock('@/lib/copilot/chat/lifecycle', () => ({
7882
resolveOrCreateChat,
7983
}))
8084

85+
vi.mock('@/lib/copilot/chat/terminal-state', () => ({
86+
finalizeAssistantTurn,
87+
}))
88+
8189
vi.mock('@/lib/copilot/tasks', () => ({
8290
taskPubSub: {
83-
publishStatusChanged: vi.fn(),
91+
publishStatusChanged: mockPublishStatusChanged,
8492
},
8593
}))
8694

@@ -137,6 +145,13 @@ describe('handleUnifiedChatPost', () => {
137145
conversationHistory: [],
138146
isNew: true,
139147
})
148+
finalizeAssistantTurn.mockResolvedValue({
149+
found: true,
150+
updated: true,
151+
appendedAssistant: true,
152+
workspaceId: 'ws-1',
153+
outcome: 'appended_assistant',
154+
})
140155
})
141156

142157
it('routes workflow-attached chat requests through the copilot backend path', async () => {
@@ -176,6 +191,7 @@ describe('handleUnifiedChatPost', () => {
176191
body: JSON.stringify({
177192
message: 'Hello',
178193
workspaceId: 'ws-1',
194+
createNewChat: true,
179195
}),
180196
})
181197
)
@@ -205,6 +221,90 @@ describe('handleUnifiedChatPost', () => {
205221
)
206222
})
207223

224+
it('persists cancelled partial responses from the server lifecycle', async () => {
225+
await handleUnifiedChatPost(
226+
new NextRequest('http://localhost/api/copilot/chat', {
227+
method: 'POST',
228+
body: JSON.stringify({
229+
message: 'Hello',
230+
workspaceId: 'ws-1',
231+
createNewChat: true,
232+
}),
233+
})
234+
)
235+
236+
const streamArgs = createSSEStream.mock.calls[0]?.[0]
237+
const onComplete = streamArgs?.orchestrateOptions?.onComplete
238+
expect(onComplete).toBeTypeOf('function')
239+
240+
await onComplete({
241+
success: false,
242+
cancelled: true,
243+
content: 'partial answer',
244+
contentBlocks: [],
245+
toolCalls: [],
246+
chatId: 'chat-1',
247+
requestId: 'request-1',
248+
})
249+
250+
expect(finalizeAssistantTurn).toHaveBeenCalledWith(
251+
expect.objectContaining({
252+
chatId: 'chat-1',
253+
userMessageId: expect.any(String),
254+
streamMarkerPolicy: 'active-or-cleared',
255+
assistantMessage: expect.objectContaining({
256+
role: 'assistant',
257+
content: 'partial answer',
258+
contentBlocks: expect.arrayContaining([
259+
expect.objectContaining({ type: 'complete', status: 'cancelled' }),
260+
]),
261+
}),
262+
})
263+
)
264+
})
265+
266+
it('republishes completed status when cancelled lifecycle persistence already ran', async () => {
267+
await handleUnifiedChatPost(
268+
new NextRequest('http://localhost/api/copilot/chat', {
269+
method: 'POST',
270+
body: JSON.stringify({
271+
message: 'Hello',
272+
workspaceId: 'ws-1',
273+
createNewChat: true,
274+
}),
275+
})
276+
)
277+
278+
const streamArgs = createSSEStream.mock.calls[0]?.[0]
279+
const onComplete = streamArgs?.orchestrateOptions?.onComplete
280+
expect(onComplete).toBeTypeOf('function')
281+
282+
finalizeAssistantTurn.mockResolvedValueOnce({
283+
found: true,
284+
updated: false,
285+
appendedAssistant: false,
286+
workspaceId: 'ws-1',
287+
outcome: 'assistant_already_persisted',
288+
})
289+
290+
await onComplete({
291+
success: false,
292+
cancelled: true,
293+
content: 'partial answer',
294+
contentBlocks: [],
295+
toolCalls: [],
296+
chatId: 'chat-1',
297+
requestId: 'request-1',
298+
})
299+
300+
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
301+
workspaceId: 'ws-1',
302+
chatId: 'chat-1',
303+
type: 'completed',
304+
streamId: streamArgs?.streamId,
305+
})
306+
})
307+
208308
it('rejects requests that have neither workflow nor workspace attachment', async () => {
209309
const response = await handleUnifiedChatPost(
210310
new NextRequest('http://localhost/api/copilot/chat', {

apps/sim/lib/copilot/chat/post.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { buildCopilotRequestPayload } from '@/lib/copilot/chat/payload'
1414
import {
1515
buildPersistedAssistantMessage,
1616
buildPersistedUserMessage,
17+
withStoppedContentBlock,
1718
} from '@/lib/copilot/chat/persisted-message'
1819
import {
1920
processContextsServer,
@@ -23,6 +24,7 @@ import { finalizeAssistantTurn } from '@/lib/copilot/chat/terminal-state'
2324
import { generateWorkspaceContext } from '@/lib/copilot/chat/workspace-context'
2425
import { COPILOT_REQUEST_MODES } from '@/lib/copilot/constants'
2526
import {
27+
CopilotChatFinalizeOutcome,
2628
CopilotChatPersistOutcome,
2729
CopilotTransport,
2830
} from '@/lib/copilot/generated/trace-attribute-values-v1'
@@ -425,13 +427,31 @@ function buildOnComplete(params: {
425427

426428
if (!chatId) return
427429

428-
// On cancel, /chat/stop is the sole DB writer — it persists
429-
// partial content AND clears conversationId in one UPDATE. If we
430-
// finalize here first the filter misses and content vanishes.
431-
// Real errors still finalize so the stream marker clears.
432-
if (result.cancelled) return
433-
434430
try {
431+
if (result.cancelled) {
432+
const finalization = await finalizeAssistantTurn({
433+
chatId,
434+
userMessageId,
435+
assistantMessage: withStoppedContentBlock(
436+
buildPersistedAssistantMessage(result, requestId)
437+
),
438+
streamMarkerPolicy: 'active-or-cleared',
439+
})
440+
const shouldPublishCompletion =
441+
finalization.updated ||
442+
finalization.outcome === CopilotChatFinalizeOutcome.AssistantAlreadyPersisted
443+
444+
if (notifyWorkspaceStatus && workspaceId && shouldPublishCompletion) {
445+
taskPubSub?.publishStatusChanged({
446+
workspaceId,
447+
chatId,
448+
type: 'completed',
449+
streamId: userMessageId,
450+
})
451+
}
452+
return
453+
}
454+
435455
await finalizeAssistantTurn({
436456
chatId,
437457
userMessageId,

0 commit comments

Comments
 (0)