Skip to content

Commit 9c1b0bc

Browse files
waleedlatif1waleedlatif1
authored
improvement(ui): remove anti-patterns, fix follow-up auto-scroll, move CopyCodeButton to emcn (#4148)
* improvement(ui): restore smooth streaming animation, fix follow-up auto-scroll, move CopyCodeButton to emcn * fix(ui): restore delayed animation, handle tilde fences, fix follow-up scroll root cause * fix(ui): extract useStreamingReveal to followup, keep cleanup changes * fix(ui): restore hydratedStreamingRef for reconnect path order-of-ops * fix(ui): restore full hydratedStreamingRef effect for reconnect path * fix(ui): use hover-hover prefix on CopyCodeButton callers to correctly override ghost variant * fix(logs): remove destructive color from cancel execution menu item * feat(logs): optimistic cancelling status on cancel execution * feat(logs): allow cancellation of pending (paused) executions * fix(hitl): cancel paused executions directly in DB Paused HITL executions are idle in the DB — they don't poll Redis or run in-process, so the existing cancel signals had no effect. The DB status stayed 'pending', causing the optimistic 'cancelling' update to revert on refetch. - Add PauseResumeManager.cancelPausedExecution: atomically sets paused_executions.status and workflow_execution_logs.status to 'cancelled' inside a FOR UPDATE transaction - Guard enqueueOrStartResume against resuming a cancelled execution - Include pausedCancelled in the cancel route success check * upgrade turbo * test(hitl): update cancel route tests for paused execution cancellation - Mock PauseResumeManager.cancelPausedExecution to prevent DB calls - Add pausedCancelled to all expected response objects - Add test for HITL paused execution cancellation path - Add missing auth/authz tests - Switch to vi.hoisted pattern for all mocks * fix(hitl): set endedAt when cancelling paused execution Without endedAt, the logs API running filter (isNull(endedAt)) would keep cancelled paused executions in the running view indefinitely. * fix(hitl): emit execution:cancelled event to canvas when cancelling paused execution Paused HITL executions have no active SSE stream, so the canvas never received the cancellation event. Now writes execution:cancelled to the event buffer and updates the stream meta so the canvas reconnect path picks it up and shows 'Execution Cancelled'. * fix(hitl): isolate cancelPausedExecution failure from successful cancellation Wrap cancelPausedExecution in try/catch so a DB error does not mask a prior successful Redis or in-process cancellation. Also move the resource-collapse side effect in home.tsx to a useEffect to avoid the stale closure on the resources array. * fix(hitl): add .catch() to fire-and-forget event buffer calls in cancel route
1 parent 0e6ada4 commit 9c1b0bc

File tree

17 files changed

+245
-158
lines changed

17 files changed

+245
-158
lines changed

‎apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.test.ts‎

Lines changed: 97 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,24 @@
55
import { NextRequest } from 'next/server'
66
import { beforeEach, describe, expect, it, vi } from 'vitest'
77

8-
const mockCheckHybridAuth = vi.fn()
9-
const mockAuthorizeWorkflowByWorkspacePermission = vi.fn()
10-
const mockMarkExecutionCancelled = vi.fn()
11-
const mockAbortManualExecution = vi.fn()
12-
13-
vi.mock('@sim/logger', () => ({
14-
createLogger: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn() }),
8+
const {
9+
mockCheckHybridAuth,
10+
mockAuthorizeWorkflowByWorkspacePermission,
11+
mockMarkExecutionCancelled,
12+
mockAbortManualExecution,
13+
mockCancelPausedExecution,
14+
mockSetExecutionMeta,
15+
mockWriteEvent,
16+
mockCloseWriter,
17+
} = vi.hoisted(() => ({
18+
mockCheckHybridAuth: vi.fn(),
19+
mockAuthorizeWorkflowByWorkspacePermission: vi.fn(),
20+
mockMarkExecutionCancelled: vi.fn(),
21+
mockAbortManualExecution: vi.fn(),
22+
mockCancelPausedExecution: vi.fn(),
23+
mockSetExecutionMeta: vi.fn(),
24+
mockWriteEvent: vi.fn(),
25+
mockCloseWriter: vi.fn(),
1526
}))
1627

1728
vi.mock('@/lib/auth/hybrid', () => ({
@@ -26,19 +37,48 @@ vi.mock('@/lib/execution/manual-cancellation', () => ({
2637
abortManualExecution: (...args: unknown[]) => mockAbortManualExecution(...args),
2738
}))
2839

40+
vi.mock('@/lib/workflows/executor/human-in-the-loop-manager', () => ({
41+
PauseResumeManager: {
42+
cancelPausedExecution: (...args: unknown[]) => mockCancelPausedExecution(...args),
43+
},
44+
}))
45+
2946
vi.mock('@/lib/workflows/utils', () => ({
3047
authorizeWorkflowByWorkspacePermission: (params: unknown) =>
3148
mockAuthorizeWorkflowByWorkspacePermission(params),
3249
}))
3350

51+
vi.mock('@/lib/posthog/server', () => ({
52+
captureServerEvent: vi.fn(),
53+
}))
54+
55+
vi.mock('@/lib/execution/event-buffer', () => ({
56+
setExecutionMeta: (...args: unknown[]) => mockSetExecutionMeta(...args),
57+
createExecutionEventWriter: () => ({
58+
write: (...args: unknown[]) => mockWriteEvent(...args),
59+
close: () => mockCloseWriter(),
60+
}),
61+
}))
62+
3463
import { POST } from './route'
3564

65+
const makeRequest = () =>
66+
new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
67+
method: 'POST',
68+
})
69+
70+
const makeParams = () => ({ params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }) })
71+
3672
describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
3773
beforeEach(() => {
3874
vi.clearAllMocks()
3975
mockCheckHybridAuth.mockResolvedValue({ success: true, userId: 'user-1' })
4076
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({ allowed: true })
4177
mockAbortManualExecution.mockReturnValue(false)
78+
mockCancelPausedExecution.mockResolvedValue(false)
79+
mockSetExecutionMeta.mockResolvedValue(undefined)
80+
mockWriteEvent.mockResolvedValue({ eventId: 1 })
81+
mockCloseWriter.mockResolvedValue(undefined)
4282
})
4383

4484
it('returns success when cancellation was durably recorded', async () => {
@@ -47,14 +87,7 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
4787
reason: 'recorded',
4888
})
4989

50-
const response = await POST(
51-
new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
52-
method: 'POST',
53-
}),
54-
{
55-
params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }),
56-
}
57-
)
90+
const response = await POST(makeRequest(), makeParams())
5891

5992
expect(response.status).toBe(200)
6093
await expect(response.json()).resolves.toEqual({
@@ -63,6 +96,7 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
6396
redisAvailable: true,
6497
durablyRecorded: true,
6598
locallyAborted: false,
99+
pausedCancelled: false,
66100
reason: 'recorded',
67101
})
68102
})
@@ -73,14 +107,7 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
73107
reason: 'redis_unavailable',
74108
})
75109

76-
const response = await POST(
77-
new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
78-
method: 'POST',
79-
}),
80-
{
81-
params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }),
82-
}
83-
)
110+
const response = await POST(makeRequest(), makeParams())
84111

85112
expect(response.status).toBe(200)
86113
await expect(response.json()).resolves.toEqual({
@@ -89,6 +116,7 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
89116
redisAvailable: false,
90117
durablyRecorded: false,
91118
locallyAborted: false,
119+
pausedCancelled: false,
92120
reason: 'redis_unavailable',
93121
})
94122
})
@@ -99,14 +127,7 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
99127
reason: 'redis_write_failed',
100128
})
101129

102-
const response = await POST(
103-
new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
104-
method: 'POST',
105-
}),
106-
{
107-
params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }),
108-
}
109-
)
130+
const response = await POST(makeRequest(), makeParams())
110131

111132
expect(response.status).toBe(200)
112133
await expect(response.json()).resolves.toEqual({
@@ -115,6 +136,7 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
115136
redisAvailable: true,
116137
durablyRecorded: false,
117138
locallyAborted: false,
139+
pausedCancelled: false,
118140
reason: 'redis_write_failed',
119141
})
120142
})
@@ -126,14 +148,7 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
126148
})
127149
mockAbortManualExecution.mockReturnValue(true)
128150

129-
const response = await POST(
130-
new NextRequest('http://localhost/api/workflows/wf-1/executions/ex-1/cancel', {
131-
method: 'POST',
132-
}),
133-
{
134-
params: Promise.resolve({ id: 'wf-1', executionId: 'ex-1' }),
135-
}
136-
)
151+
const response = await POST(makeRequest(), makeParams())
137152

138153
expect(response.status).toBe(200)
139154
await expect(response.json()).resolves.toEqual({
@@ -142,7 +157,50 @@ describe('POST /api/workflows/[id]/executions/[executionId]/cancel', () => {
142157
redisAvailable: false,
143158
durablyRecorded: false,
144159
locallyAborted: true,
160+
pausedCancelled: false,
161+
reason: 'redis_unavailable',
162+
})
163+
})
164+
165+
it('returns success when a paused HITL execution is cancelled directly in the database', async () => {
166+
mockMarkExecutionCancelled.mockResolvedValue({
167+
durablyRecorded: false,
168+
reason: 'redis_unavailable',
169+
})
170+
mockCancelPausedExecution.mockResolvedValue(true)
171+
172+
const response = await POST(makeRequest(), makeParams())
173+
174+
expect(response.status).toBe(200)
175+
await expect(response.json()).resolves.toEqual({
176+
success: true,
177+
executionId: 'ex-1',
178+
redisAvailable: false,
179+
durablyRecorded: false,
180+
locallyAborted: false,
181+
pausedCancelled: true,
145182
reason: 'redis_unavailable',
146183
})
147184
})
185+
186+
it('returns 401 when auth fails', async () => {
187+
mockCheckHybridAuth.mockResolvedValue({ success: false, error: 'Unauthorized' })
188+
189+
const response = await POST(makeRequest(), makeParams())
190+
191+
expect(response.status).toBe(401)
192+
})
193+
194+
it('returns 403 when workflow access is denied', async () => {
195+
mockMarkExecutionCancelled.mockResolvedValue({ durablyRecorded: true, reason: 'recorded' })
196+
mockAuthorizeWorkflowByWorkspacePermission.mockResolvedValue({
197+
allowed: false,
198+
message: 'Access denied',
199+
status: 403,
200+
})
201+
202+
const response = await POST(makeRequest(), makeParams())
203+
204+
expect(response.status).toBe(403)
205+
})
148206
})

‎apps/sim/app/api/workflows/[id]/executions/[executionId]/cancel/route.ts‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import { createLogger } from '@sim/logger'
22
import { type NextRequest, NextResponse } from 'next/server'
33
import { checkHybridAuth } from '@/lib/auth/hybrid'
44
import { markExecutionCancelled } from '@/lib/execution/cancellation'
5+
import { createExecutionEventWriter, setExecutionMeta } from '@/lib/execution/event-buffer'
56
import { abortManualExecution } from '@/lib/execution/manual-cancellation'
67
import { captureServerEvent } from '@/lib/posthog/server'
8+
import { PauseResumeManager } from '@/lib/workflows/executor/human-in-the-loop-manager'
79
import { authorizeWorkflowByWorkspacePermission } from '@/lib/workflows/utils'
810

911
const logger = createLogger('CancelExecutionAPI')
@@ -49,19 +51,41 @@ export async function POST(
4951

5052
const cancellation = await markExecutionCancelled(executionId)
5153
const locallyAborted = abortManualExecution(executionId)
54+
let pausedCancelled = false
55+
try {
56+
pausedCancelled = await PauseResumeManager.cancelPausedExecution(executionId)
57+
} catch (error) {
58+
logger.warn('Failed to cancel paused execution in database', { executionId, error })
59+
}
5260

5361
if (cancellation.durablyRecorded) {
5462
logger.info('Execution marked as cancelled in Redis', { executionId })
5563
} else if (locallyAborted) {
5664
logger.info('Execution cancelled via local in-process fallback', { executionId })
65+
} else if (pausedCancelled) {
66+
logger.info('Paused execution cancelled directly in database', { executionId })
67+
void setExecutionMeta(executionId, { status: 'cancelled', workflowId }).catch(() => {})
68+
const writer = createExecutionEventWriter(executionId)
69+
void writer
70+
.write({
71+
type: 'execution:cancelled',
72+
timestamp: new Date().toISOString(),
73+
executionId,
74+
workflowId,
75+
data: { duration: 0 },
76+
})
77+
.then(() => writer.close())
78+
.catch(() => {})
5779
} else {
5880
logger.warn('Execution cancellation was not durably recorded', {
5981
executionId,
6082
reason: cancellation.reason,
6183
})
6284
}
6385

64-
if (cancellation.durablyRecorded || locallyAborted) {
86+
const success = cancellation.durablyRecorded || locallyAborted || pausedCancelled
87+
88+
if (success) {
6589
const workspaceId = workflowAuthorization.workflow?.workspaceId
6690
captureServerEvent(
6791
auth.userId,
@@ -72,11 +96,12 @@ export async function POST(
7296
}
7397

7498
return NextResponse.json({
75-
success: cancellation.durablyRecorded || locallyAborted,
99+
success,
76100
executionId,
77101
redisAvailable: cancellation.reason !== 'redis_unavailable',
78102
durablyRecorded: cancellation.durablyRecorded,
79103
locallyAborted,
104+
pausedCancelled,
80105
reason: cancellation.reason,
81106
})
82107
} catch (error: any) {

‎apps/sim/app/chat/components/message/components/markdown-renderer.tsx‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
import React, { type HTMLAttributes, memo, type ReactNode, useMemo } from 'react'
22
import { Streamdown } from 'streamdown'
33
import 'streamdown/styles.css'
4-
import { Tooltip } from '@/components/emcn'
5-
import { CopyCodeButton } from '@/components/ui/copy-code-button'
4+
import { CopyCodeButton, Tooltip } from '@/components/emcn'
65
import { extractTextContent } from '@/lib/core/utils/react-node-text'
76

87
export function LinkWithPreview({ href, children }: { href: string; children: React.ReactNode }) {
@@ -100,7 +99,7 @@ function createCustomComponents(LinkComponent: typeof LinkWithPreview) {
10099
</span>
101100
<CopyCodeButton
102101
code={extractTextContent(codeContent)}
103-
className='text-gray-400 hover:bg-gray-700 hover:text-gray-200'
102+
className='text-gray-400 hover-hover:bg-gray-700 hover-hover:text-gray-200'
104103
/>
105104
</div>
106105
<pre className='overflow-x-auto p-4 font-mono text-gray-200 dark:text-gray-100'>

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ import 'prismjs/components/prism-bash'
88
import 'prismjs/components/prism-css'
99
import 'prismjs/components/prism-markup'
1010
import '@/components/emcn/components/code/code.css'
11-
import { Checkbox, highlight, languages } from '@/components/emcn'
12-
import { CopyCodeButton } from '@/components/ui/copy-code-button'
11+
import { Checkbox, CopyCodeButton, highlight, languages } from '@/components/emcn'
1312
import { cn } from '@/lib/core/utils/cn'
1413
import { extractTextContent } from '@/lib/core/utils/react-node-text'
1514
import {
@@ -148,7 +147,7 @@ const MARKDOWN_COMPONENTS = {
148147
<span className='text-[var(--text-tertiary)] text-xs'>{language || 'code'}</span>
149148
<CopyCodeButton
150149
code={codeString}
151-
className='text-[var(--text-tertiary)] hover:bg-[var(--surface-5)] hover:text-[var(--text-secondary)]'
150+
className='-mr-2 text-[var(--text-tertiary)] hover-hover:bg-[var(--surface-5)] hover-hover:text-[var(--text-secondary)]'
152151
/>
153152
</div>
154153
<div className='code-editor-theme bg-[var(--surface-5)] dark:bg-[var(--code-bg)]'>
@@ -265,12 +264,7 @@ export function ChatContent({
265264
useEffect(() => {
266265
const handler = (e: Event) => {
267266
const { type, id, title } = (e as CustomEvent).detail
268-
const RESOURCE_TYPE_MAP: Record<string, string> = {}
269-
onWorkspaceResourceSelectRef.current?.({
270-
type: RESOURCE_TYPE_MAP[type] || type,
271-
id,
272-
title: title || id,
273-
})
267+
onWorkspaceResourceSelectRef.current?.({ type, id, title: title || id })
274268
}
275269
window.addEventListener('wsres-click', handler)
276270
return () => window.removeEventListener('wsres-click', handler)

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-chat/mothership-chat.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ export function MothershipChat({
176176
blocks={msg.contentBlocks || []}
177177
fallbackContent={msg.content}
178178
isStreaming={isThisStreaming}
179-
onOptionSelect={isLastMessage && !isStreamActive ? onSubmit : undefined}
179+
onOptionSelect={isLastMessage ? onSubmit : undefined}
180180
onWorkspaceResourceSelect={onWorkspaceResourceSelect}
181181
/>
182182
{!isThisStreaming && (msg.content || msg.contentBlocks?.length) && (

‎apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.tsx‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ export function UserInput({
232232
const canSubmit = (value.trim().length > 0 || hasFiles) && !isSending
233233

234234
const valueRef = useRef(value)
235+
valueRef.current = value
235236
const sttPrefixRef = useRef('')
236237

237238
const handleTranscript = useCallback((text: string) => {
@@ -271,10 +272,6 @@ export function UserInput({
271272
const isSendingRef = useRef(isSending)
272273
isSendingRef.current = isSending
273274

274-
useEffect(() => {
275-
valueRef.current = value
276-
}, [value])
277-
278275
const textareaRef = mentionMenu.textareaRef
279276
const wasSendingRef = useRef(false)
280277
const atInsertPosRef = useRef<number | null>(null)
@@ -358,9 +355,7 @@ export function UserInput({
358355
}
359356
// Reset after batch so the next non-drop insert uses the cursor position
360357
atInsertPosRef.current = null
361-
} catch {
362-
// Invalid JSON — ignore
363-
}
358+
} catch {}
364359
textareaRef.current?.focus()
365360
return
366361
}
@@ -372,9 +367,7 @@ export function UserInput({
372367
const resource = JSON.parse(resourceJson) as MothershipResource
373368
handleResourceSelect(resource)
374369
atInsertPosRef.current = null
375-
} catch {
376-
// Invalid JSON — ignore
377-
}
370+
} catch {}
378371
textareaRef.current?.focus()
379372
return
380373
}

0 commit comments

Comments
 (0)