Skip to content

Commit 7068722

Browse files
committed
fix(executor): honor cancellation after backoff, persist retry policy, read statusCode
1 parent 693340f commit 7068722

9 files changed

Lines changed: 48 additions & 70 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/text/text.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export function Text({ blockId, subBlockId, content, className }: TextProps) {
3333
className={`rounded-md border bg-[var(--surface-2)] p-4 shadow-sm ${className || ''}`}
3434
>
3535
<div
36-
className='max-w-none break-words text-[var(--text-secondary)] text-sm [&_a]:text-[var(--brand-secondary)] [&_a]:underline [&_a]:underline-offset-2 [&_a]:hover-hover:brightness-110 [&_code]:rounded [&_code]:bg-[var(--surface-5)] [&_code]:px-1 [&_code]:py-0.5 [&_code]:text-[var(--text-tertiary)] [&_code]:text-xs [&_ul]:ml-5 [&_ul]:list-disc [&_ul]:marker:text-[var(--text-muted)] [&_strong]:font-medium [&_strong]:text-[var(--text-primary)]'
36+
className='max-w-none break-words text-[var(--text-secondary)] text-sm [&_a]:text-[var(--brand-secondary)] [&_a]:underline [&_a]:underline-offset-2 [&_a]:hover-hover:brightness-110 [&_code]:rounded [&_code]:bg-[var(--surface-5)] [&_code]:px-1 [&_code]:py-0.5 [&_code]:text-[var(--text-tertiary)] [&_code]:text-xs [&_strong]:font-medium [&_strong]:text-[var(--text-primary)] [&_ul]:ml-5 [&_ul]:list-disc [&_ul]:marker:text-[var(--text-muted)]'
3737
dangerouslySetInnerHTML={{ __html: content }}
3838
/>
3939
</div>

apps/sim/executor/execution/block-executor.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,12 @@ export class BlockExecutor {
428428
error: normalizeError(error),
429429
})
430430
await sleep(delayMs)
431+
/**
432+
* Re-checked after the wait, not only before it: `sleep` is not abort-aware,
433+
* so a run cancelled during backoff would otherwise start another attempt
434+
* against a workflow that has already stopped.
435+
*/
436+
if (ctx.abortSignal?.aborted === true) throw error
431437
}
432438
}
433439
}

apps/sim/executor/execution/block-retry.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,13 @@ export function isRetryableBlockError(error: unknown): boolean {
107107
*/
108108
if (ChildWorkflowError.isChildWorkflowError(current)) return false
109109

110-
const candidate = current as { name?: string; code?: string; status?: number; message?: string }
110+
const candidate = current as {
111+
name?: string
112+
code?: string
113+
status?: number
114+
statusCode?: number
115+
message?: string
116+
}
111117

112118
/**
113119
* An abort is a deliberate stop — a user pressing Stop, or a block timeout
@@ -118,9 +124,13 @@ export function isRetryableBlockError(error: unknown): boolean {
118124

119125
if (candidate.name === 'TimeoutError') return true
120126
if (candidate.code && RETRYABLE_ERROR_CODES.has(candidate.code)) return true
121-
if (typeof candidate.status === 'number' && RETRYABLE_HTTP_STATUSES.has(candidate.status)) {
122-
return true
123-
}
127+
/**
128+
* Both spellings are read: `HttpError` and the generic tool layer expose
129+
* `statusCode`, while provider and SDK errors use `status`. Reading only one
130+
* silently excludes most integration blocks from status-based retry.
131+
*/
132+
const httpStatus = candidate.status ?? candidate.statusCode
133+
if (typeof httpStatus === 'number' && RETRYABLE_HTTP_STATUSES.has(httpStatus)) return true
124134
if (candidate.message?.includes(BUN_SOCKET_CLOSED_MESSAGE)) return true
125135

126136
current = (current as { cause?: unknown }).cause

apps/sim/executor/handlers/pi/cloud-review-tools.test.ts

Lines changed: 1 addition & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { execFile } from 'node:child_process'
5-
import { mkdir, mkdtemp, rm, symlink, writeFile as writeLocalFile } from 'node:fs/promises'
5+
import { mkdir, mkdtemp, rm, writeFile as writeLocalFile } from 'node:fs/promises'
66
import { tmpdir } from 'node:os'
77
import { join } from 'node:path'
88
import { promisify } from 'node:util'
@@ -61,70 +61,6 @@ describe('cloud review tools', () => {
6161
expect(source).not.toContain('--unified=20')
6262
})
6363

64-
it('enforces read-size and canonical path bounds in the actual helper', async () => {
65-
await installCloudReviewTools(runner)
66-
const source = writeFile.mock.calls[0][1] as string
67-
const testDir = await mkdtemp(join(tmpdir(), 'sim-review-tools-'))
68-
const repoDir = join(testDir, 'repo')
69-
const scriptPath = join(testDir, 'review-tools.py')
70-
const outsidePath = join(testDir, 'outside.txt')
71-
72-
try {
73-
await mkdir(repoDir)
74-
await writeLocalFile(
75-
scriptPath,
76-
source.replace(
77-
"pathlib.Path('/workspace/repo')",
78-
`pathlib.Path(${JSON.stringify(repoDir)})`
79-
)
80-
)
81-
await writeLocalFile(join(repoDir, 'safe.txt'), 'one\ntwo\n')
82-
await writeLocalFile(outsidePath, 'secret')
83-
await symlink(outsidePath, join(repoDir, 'escape.txt'))
84-
await mkdir(join(repoDir, '.git'))
85-
await writeLocalFile(join(repoDir, '.git', 'secret.txt'), 'DO_NOT_EXPOSE')
86-
87-
const execute = (operation: string, args: Record<string, unknown>) =>
88-
execFileAsync('python3', [scriptPath], {
89-
env: {
90-
...process.env,
91-
REVIEW_TOOL_OPERATION: operation,
92-
REVIEW_TOOL_ARGS: JSON.stringify(args),
93-
},
94-
})
95-
96-
await expect(
97-
execute('read', { path: 'safe.txt', offset: 1, limit: 2 })
98-
).resolves.toMatchObject({
99-
stdout: '1: one\n2: two',
100-
})
101-
await expect(execute('read', { path: '../outside.txt' })).rejects.toMatchObject({
102-
stderr: expect.stringContaining('path must stay within the repository'),
103-
})
104-
await expect(execute('read', { path: 'escape.txt' })).rejects.toMatchObject({
105-
stderr: expect.stringContaining('path resolves outside the repository'),
106-
})
107-
108-
const found = await execute('find', { path: '.', pattern: '**/*', limit: 20 })
109-
expect(found.stdout).toContain('safe.txt')
110-
expect(found.stdout).not.toContain('.git')
111-
const searched = await execute('search', {
112-
path: '.',
113-
pattern: 'DO_NOT_EXPOSE',
114-
glob: '**/*',
115-
literal: true,
116-
})
117-
expect(searched.stdout).toBe('No matches found')
118-
119-
await writeLocalFile(join(repoDir, 'large.bin'), Buffer.alloc(5_000_001))
120-
await expect(execute('read', { path: 'large.bin' })).rejects.toMatchObject({
121-
stderr: expect.stringContaining('exceeds the 5 MB read limit'),
122-
})
123-
} finally {
124-
await rm(testDir, { recursive: true, force: true })
125-
}
126-
})
127-
12864
it('validates inline coordinates against an exact local diff', async () => {
12965
await installCloudReviewTools(runner)
13066
const source = writeFile.mock.calls[0][1] as string

apps/sim/lib/api/contracts/workflows.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
import {
2+
BLOCK_RETRY_MAX_ATTEMPTS,
3+
BLOCK_RETRY_MAX_WAIT_MS,
4+
BLOCK_RETRY_MIN_ATTEMPTS,
5+
BLOCK_RETRY_MIN_WAIT_MS,
6+
} from '@sim/workflow-types/workflow'
17
import { z } from 'zod'
28
import {
39
requiredFieldSchema,
@@ -51,6 +57,20 @@ const workflowEdgeHandleSchema = z
5157
.nullish()
5258
.transform((value) => value ?? undefined)
5359

60+
const workflowBlockRetrySchema = z.object({
61+
maxAttempts: z
62+
.number()
63+
.int()
64+
.min(BLOCK_RETRY_MIN_ATTEMPTS, `maxAttempts must be at least ${BLOCK_RETRY_MIN_ATTEMPTS}`)
65+
.max(BLOCK_RETRY_MAX_ATTEMPTS, `maxAttempts cannot exceed ${BLOCK_RETRY_MAX_ATTEMPTS}`),
66+
waitMs: z
67+
.number()
68+
.int()
69+
.min(BLOCK_RETRY_MIN_WAIT_MS, 'waitMs cannot be negative')
70+
.max(BLOCK_RETRY_MAX_WAIT_MS, `waitMs cannot exceed ${BLOCK_RETRY_MAX_WAIT_MS}ms`)
71+
.optional(),
72+
})
73+
5474
const workflowBlockStateSchema = z.object({
5575
id: z.string(),
5676
type: z.string(),
@@ -65,6 +85,7 @@ const workflowBlockStateSchema = z.object({
6585
triggerMode: z.boolean().optional(),
6686
data: workflowBlockDataSchema.optional(),
6787
locked: z.boolean().optional(),
88+
retry: workflowBlockRetrySchema.optional(),
6889
})
6990

7091
const workflowEdgeSchema = z.object({

apps/sim/serializer/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ export class Serializer {
442442
serializedBlock.config?.params?.triggerMode === true ||
443443
serializedBlock.metadata?.category === 'triggers',
444444
advancedMode: serializedBlock.config?.params?.advancedMode === true,
445+
...(serializedBlock.retry ? { retry: serializedBlock.retry } : {}),
445446
}
446447
}
447448
}

packages/db/schema.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,8 @@ export const workflowBlocks = pgTable(
295295
advancedMode: boolean('advanced_mode').notNull().default(false),
296296
triggerMode: boolean('trigger_mode').notNull().default(false),
297297
locked: boolean('locked').notNull().default(false),
298+
/** Opt-in retry policy; NULL means the block never retries. */
299+
retry: jsonb('retry'),
298300
height: decimal('height').notNull().default('0'),
299301

300302
subBlocks: jsonb('sub_blocks').notNull().default('{}'),

packages/workflow-persistence/src/load.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ export async function loadWorkflowFromNormalizedTablesRaw(
8383
horizontalHandles: block.horizontalHandles,
8484
advancedMode: block.advancedMode,
8585
triggerMode: block.triggerMode,
86+
retry: (block.retry as BlockState['retry']) ?? undefined,
8687
height: Number(block.height),
8788
subBlocks: (block.subBlocks as BlockState['subBlocks']) || {},
8889
outputs: (block.outputs as BlockState['outputs']) || {},

packages/workflow-persistence/src/save.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export async function saveWorkflowToNormalizedTables(
4040
horizontalHandles: block.horizontalHandles ?? true,
4141
advancedMode: block.advancedMode ?? false,
4242
triggerMode: block.triggerMode ?? false,
43+
retry: block.retry ?? null,
4344
height: String(block.height || 0),
4445
subBlocks: block.subBlocks || {},
4546
outputs: block.outputs || {},

0 commit comments

Comments
 (0)