Skip to content

Commit 69289d2

Browse files
authored
fix(execution): stop run-buffer failures from stranding resumed executions (#6187)
* fix(resume): stop a failed run-buffer publish from stranding a resumed execution The run buffer is a replay convenience for stream readers; the durable execution record is authoritative. A failed terminal event publish was deciding the outcome of work that had already run: it threw, the resume was marked failed, and the paused execution was left paused forever. That path is also not retryable — the workflow had already executed — so the next resume attempt re-ran its side effects. Degrade instead. Record the terminal status on the stream meta so readers are not left polling an 'active' stream, log the failure, and let the resume settle on its real result. The same treatment applies when the terminal event is never published at all, which previously synthesized an error for the same stranding effect. Removes the now-unreachable TERMINAL_PUBLISH_ERROR constant and its branch. Pre-execution buffer failures stay fatal and retryable, since no work has happened yet. * fix(execution): give per-user Redis budget keys a fixed window The per-user byte budget refreshed its TTL on every accepted write, so for any user who never went a full TTL without writing, the key never expired. The per-execution data it accounted for kept expiring underneath it, so the counter accrued bytes Redis had already dropped and drifted toward the ceiling. On reaching it, every subsequent write for that user was rejected until they stopped writing for a full TTL — and since a rejected write does not refresh the TTL, it recovered on its own and then refilled. User keys now get a fixed window: the TTL is set when the key is created and never extended. Applied to all five Lua scripts that touch the key so the writers cannot drift apart. Execution-scoped keys keep sliding — they are refreshed on the same schedule as the data they account for, so their counter and bytes stay in step. * fix(execution): heal a user budget key that somehow has no expiry The no-op branch of the flush script dropped the user-key EXPIRE outright rather than guarding it like every other site. No current path can create the key without an expiry, but if one ever did, that branch was the one place that would never give it one — and a user counter with no expiry is the unbounded version of the bug this series fixes. Guard it instead, so the branch heals such a key rather than skipping it, and so all five scripts read the same way. * fix(stream): end a replay cleanly when terminal metadata has no terminal event Terminal metadata is the authoritative end-of-run signal — the happy path writes it atomically with the terminal event, and a run whose terminal event could not be buffered records the status on its own. The reader required both, so the degraded case threw and turned a replay that was merely incomplete into a broken one. Metadata without a matching event means the buffer degraded, not that the run is still going. Log it and close the stream: the reader has already received every event that was buffered, and the durable record is unaffected either way.
1 parent 1377256 commit 69289d2

8 files changed

Lines changed: 200 additions & 24 deletions

File tree

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe('execution stream reconnect route', () => {
9090
expect(mockReadExecutionEventsState).toHaveBeenNthCalledWith(2, 'exec-1', 3)
9191
})
9292

93-
it('errors when terminal metadata has no terminal event to replay', async () => {
93+
it('ends the stream cleanly when terminal metadata has no terminal event to replay', async () => {
9494
mockReadExecutionMetaState
9595
.mockResolvedValueOnce({
9696
status: 'found',
@@ -115,9 +115,7 @@ describe('execution stream reconnect route', () => {
115115
})
116116

117117
expect(response.status).toBe(200)
118-
await expect(response.text()).rejects.toThrow(
119-
'Execution reached terminal metadata without a terminal event'
120-
)
118+
await expect(response.text()).resolves.toContain('data: [DONE]')
121119
})
122120

123121
it('allows replay event id gaps from reserved but unused writer ids', async () => {

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,20 @@ export const GET = withRouteHandler(
142142
if (!closed) controller.close()
143143
}
144144

145+
/**
146+
* Terminal metadata is the authoritative end-of-run signal. The
147+
* happy path writes it atomically with the terminal event, and a run
148+
* whose terminal event could not be buffered records the status on
149+
* its own — so metadata without a matching event means the buffer
150+
* degraded, not that the run is still going. End the stream cleanly:
151+
* the reader has already received every event that was buffered, and
152+
* failing here would turn a degraded replay into a broken one.
153+
*/
145154
const closeAfterTerminalEvent = (events: ExecutionEventEntry[]) => {
146155
if (!enqueueEvents(events)) {
147-
throw new Error('Execution reached terminal metadata without a terminal event')
156+
logger.warn('Execution reached terminal metadata without a terminal event', {
157+
executionId,
158+
})
148159
}
149160
closeWithDone()
150161
}

apps/sim/lib/execution/event-buffer.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@ function isResetScript(script: string): boolean {
6868
return script.includes('retained_bytes') && script.includes('replayStartEventId')
6969
}
7070

71+
function countOccurrences(haystack: string, needle: string): number {
72+
return haystack.split(needle).length - 1
73+
}
74+
7175
describe('execution event buffer', () => {
7276
beforeEach(() => {
7377
vi.clearAllMocks()
@@ -417,6 +421,37 @@ describe('execution event buffer', () => {
417421
)
418422
})
419423

424+
it('never extends an existing user budget window while flushing events', async () => {
425+
let flushScript = ''
426+
mockRedis.eval.mockImplementation(async (script: string) => {
427+
if (isFlushScript(script)) flushScript = script
428+
return [1, false, 0]
429+
})
430+
431+
const writer = createExecutionEventWriter('exec-1', { userId: 'user-1' })
432+
await writer.writeTerminal(makeEvent('terminal'), 'complete')
433+
434+
expect(flushScript).not.toBe('')
435+
const userKeyExpires = countOccurrences(flushScript, "redis.call('EXPIRE', KEYS[5]")
436+
const userKeyTtlGuards = countOccurrences(flushScript, "redis.call('TTL', KEYS[5]) < 0")
437+
expect(userKeyExpires).toBeGreaterThan(0)
438+
expect(userKeyTtlGuards).toBe(userKeyExpires)
439+
})
440+
441+
it('keeps sliding the execution budget window, which expires with its own data', async () => {
442+
let flushScript = ''
443+
mockRedis.eval.mockImplementation(async (script: string) => {
444+
if (isFlushScript(script)) flushScript = script
445+
return [1, false, 0]
446+
})
447+
448+
const writer = createExecutionEventWriter('exec-1', { userId: 'user-1' })
449+
await writer.writeTerminal(makeEvent('terminal'), 'complete')
450+
451+
expect(countOccurrences(flushScript, "redis.call('TTL', KEYS[4]) < 0")).toBe(0)
452+
expect(countOccurrences(flushScript, "redis.call('EXPIRE', KEYS[4]")).toBeGreaterThan(0)
453+
})
454+
420455
it('reports pruned replay buffers before reading incomplete events', async () => {
421456
mockRedis.hgetall.mockResolvedValue({ status: 'active', earliestEventId: '10' })
422457

apps/sim/lib/execution/event-buffer.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,9 @@ if net_bytes > 0 then
8585
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
8686
if #KEYS >= 5 then
8787
redis.call('INCRBY', KEYS[5], net_bytes)
88-
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
88+
if redis.call('TTL', KEYS[5]) < 0 then
89+
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
90+
end
8991
end
9092
elseif net_bytes < 0 then
9193
local release_bytes = -net_bytes
@@ -99,15 +101,15 @@ elseif net_bytes < 0 then
99101
local user_next = redis.call('DECRBY', KEYS[5], release_bytes)
100102
if user_next <= 0 then
101103
redis.call('DEL', KEYS[5])
102-
else
104+
elseif redis.call('TTL', KEYS[5]) < 0 then
103105
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
104106
end
105107
end
106108
else
107109
if redis.call('EXISTS', KEYS[4]) == 1 then
108110
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
109111
end
110-
if #KEYS >= 5 and redis.call('EXISTS', KEYS[5]) == 1 then
112+
if #KEYS >= 5 and redis.call('EXISTS', KEYS[5]) == 1 and redis.call('TTL', KEYS[5]) < 0 then
111113
redis.call('EXPIRE', KEYS[5], budget_ttl_seconds)
112114
end
113115
end
@@ -148,7 +150,7 @@ if retained_bytes > 0 then
148150
local user_next = redis.call('DECRBY', KEYS[4], retained_bytes)
149151
if user_next <= 0 then
150152
redis.call('DEL', KEYS[4])
151-
else
153+
elseif redis.call('TTL', KEYS[4]) < 0 then
152154
redis.call('EXPIRE', KEYS[4], tonumber(ARGV[4]))
153155
end
154156
end
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it, vi } from 'vitest'
5+
import {
6+
getExecutionRedisBudgetKeys,
7+
reserveExecutionRedisBytes,
8+
} from '@/lib/execution/redis-budget.server'
9+
10+
function countOccurrences(haystack: string, needle: string): number {
11+
return haystack.split(needle).length - 1
12+
}
13+
14+
async function captureReserveScript(userId?: string): Promise<string> {
15+
let script = ''
16+
const redis = {
17+
eval: vi.fn(async (source: string) => {
18+
script = source
19+
return [1, 'ok', 0, 0]
20+
}),
21+
}
22+
23+
await reserveExecutionRedisBytes(redis as never, {
24+
executionId: 'exec-1',
25+
userId,
26+
category: 'event_buffer',
27+
operation: 'write_events',
28+
bytes: 128,
29+
})
30+
31+
return script
32+
}
33+
34+
describe('reserveExecutionRedisBytes', () => {
35+
it('scopes the reservation to the execution, and to the user when one is known', () => {
36+
expect(
37+
getExecutionRedisBudgetKeys({
38+
executionId: 'exec-1',
39+
category: 'event_buffer',
40+
operation: 'write_events',
41+
bytes: 1,
42+
})
43+
).toEqual(['execution:redis-budget:execution:exec-1'])
44+
45+
expect(
46+
getExecutionRedisBudgetKeys({
47+
executionId: 'exec-1',
48+
userId: 'user-1',
49+
category: 'event_buffer',
50+
operation: 'write_events',
51+
bytes: 1,
52+
})
53+
).toEqual(['execution:redis-budget:execution:exec-1', 'execution:redis-budget:user:user-1'])
54+
})
55+
56+
/**
57+
* A user key aggregates across every execution that user runs, so extending
58+
* its TTL on each write keeps it alive indefinitely while the per-execution
59+
* data it accounts for expires underneath it. The window must be fixed.
60+
*/
61+
it('never extends an existing user budget window', async () => {
62+
const script = await captureReserveScript('user-1')
63+
64+
const userKeyExpires = countOccurrences(script, "redis.call('EXPIRE', KEYS[2]")
65+
const userKeyTtlGuards = countOccurrences(script, "redis.call('TTL', KEYS[2]) < 0")
66+
expect(userKeyExpires).toBeGreaterThan(0)
67+
expect(userKeyTtlGuards).toBe(userKeyExpires)
68+
})
69+
70+
it('keeps sliding the execution budget window, which expires with its own data', async () => {
71+
const script = await captureReserveScript('user-1')
72+
73+
expect(countOccurrences(script, "redis.call('TTL', KEYS[1]) < 0")).toBe(0)
74+
expect(countOccurrences(script, "redis.call('EXPIRE', KEYS[1]")).toBeGreaterThan(0)
75+
})
76+
})

apps/sim/lib/execution/redis-budget.server.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@ const MAX_EXECUTION_REDIS_BYTES = 64 * 1024 * 1024
1212
const MAX_USER_REDIS_BYTES = 256 * 1024 * 1024
1313
const REDIS_BUDGET_TTL_SECONDS = 60 * 60
1414

15+
/**
16+
* Execution and user budget keys expire differently on purpose.
17+
*
18+
* An execution key accounts for data that is refreshed on the same schedule as
19+
* the key itself, so sliding its TTL on every write keeps the counter and the
20+
* bytes it represents in step.
21+
*
22+
* A user key aggregates across every execution that user runs. Sliding its TTL
23+
* on each write keeps it alive indefinitely for any user who stays active,
24+
* while the per-execution data it accounts for keeps expiring underneath it —
25+
* so the counter accrues bytes Redis has already dropped and eventually pins
26+
* the user at their ceiling until they go a full TTL without writing. User
27+
* keys therefore get a fixed window: the TTL is set when the key is created
28+
* and never extended.
29+
*/
30+
1531
const RESERVE_REDIS_BYTES_SCRIPT = `
1632
local bytes = tonumber(ARGV[1])
1733
local execution_limit = tonumber(ARGV[2])
@@ -32,7 +48,9 @@ redis.call('INCRBY', KEYS[1], bytes)
3248
redis.call('EXPIRE', KEYS[1], ttl_seconds)
3349
if #KEYS >= 2 then
3450
redis.call('INCRBY', KEYS[2], bytes)
35-
redis.call('EXPIRE', KEYS[2], ttl_seconds)
51+
if redis.call('TTL', KEYS[2]) < 0 then
52+
redis.call('EXPIRE', KEYS[2], ttl_seconds)
53+
end
3654
end
3755
return {1, 'ok', execution_current + bytes, user_current + bytes}
3856
`

apps/sim/lib/uploads/utils/user-file-base64.server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ if bytes and bytes > 0 then
5959
local user_next = redis.call('DECRBY', KEYS[4], bytes)
6060
if user_next <= 0 then
6161
redis.call('DEL', KEYS[4])
62-
else
62+
elseif redis.call('TTL', KEYS[4]) < 0 then
6363
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
6464
end
6565
end
@@ -129,7 +129,7 @@ if #KEYS >= 4 then
129129
redis.call('DEL', KEYS[4])
130130
end
131131
end
132-
if redis.call('EXISTS', KEYS[4]) == 1 then
132+
if redis.call('EXISTS', KEYS[4]) == 1 and redis.call('TTL', KEYS[4]) < 0 then
133133
redis.call('EXPIRE', KEYS[4], budget_ttl_seconds)
134134
end
135135
end

apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
flushExecutionStreamReplayBuffer,
1515
initializeExecutionStreamMeta,
1616
resetExecutionStreamBuffer,
17+
setExecutionMeta,
1718
type TerminalExecutionStreamStatus,
1819
} from '@/lib/execution/event-buffer'
1920
import {
@@ -71,7 +72,6 @@ const execDb = dbFor('exec')
7172

7273
const logger = createLogger('HumanInTheLoopManager')
7374
const RUN_BUFFER_UNAVAILABLE_ERROR = 'Run buffer temporarily unavailable'
74-
const TERMINAL_PUBLISH_ERROR = 'Run buffer terminal event publish failed'
7575
const RESUMABLE_PAUSED_STATUSES = ['paused', 'partially_resumed'] as const
7676
const CANCELLABLE_PAUSED_STATUSES = ['paused', 'partially_resumed'] as const
7777
const AUTOMATIC_RESUME_INTERVENTION_PREFIX = 'Automatic resume requires manual intervention: '
@@ -771,7 +771,7 @@ export class PauseResumeManager {
771771
preserveForRetry: true,
772772
retryable: error.retryable,
773773
})
774-
} else if (message === RUN_BUFFER_UNAVAILABLE_ERROR || message === TERMINAL_PUBLISH_ERROR) {
774+
} else if (message === RUN_BUFFER_UNAVAILABLE_ERROR) {
775775
await PauseResumeManager.markResumeAttemptFailed({
776776
resumeEntryId,
777777
pausedExecutionId: pausedExecution.id,
@@ -1280,20 +1280,48 @@ export class PauseResumeManager {
12801280
}
12811281

12821282
let terminalEventPublished = false
1283+
let terminalPublishDegraded = false
1284+
1285+
/**
1286+
* The run buffer is a replay convenience for stream readers; the durable
1287+
* execution record is authoritative. A failed terminal publish must not
1288+
* decide the outcome of work that already ran — the resume is not
1289+
* retryable at this point, so throwing here would strand the execution as
1290+
* paused and re-run its side effects on the next attempt. Degrade instead:
1291+
* record the terminal status on the stream meta so readers are not left
1292+
* polling an 'active' stream forever, and let the resume settle normally.
1293+
*/
1294+
const degradeTerminalPublish = async (
1295+
terminalStatus: TerminalExecutionStreamStatus,
1296+
error: unknown
1297+
) => {
1298+
terminalPublishDegraded = true
1299+
logger.warn('Failed to publish resume terminal event', {
1300+
resumeExecutionId,
1301+
status: terminalStatus,
1302+
error: toError(error).message,
1303+
})
1304+
const metaPersisted = await setExecutionMeta(resumeExecutionId, {
1305+
status: terminalStatus,
1306+
}).catch(() => false)
1307+
if (!metaPersisted) {
1308+
logger.warn('Failed to record degraded terminal status on resume stream meta', {
1309+
resumeExecutionId,
1310+
status: terminalStatus,
1311+
})
1312+
}
1313+
}
1314+
12831315
const writeBufferedEvent = async (
12841316
event: ExecutionEvent,
12851317
terminalStatus?: TerminalExecutionStreamStatus
12861318
) => {
12871319
const isBuffered = !LIVE_ONLY_EXECUTION_EVENT_TYPES.has(event.type)
12881320
if (isBuffered) {
12891321
const entry = terminalStatus
1290-
? await eventWriter.writeTerminal(event, terminalStatus).catch((error) => {
1291-
logger.warn('Failed to publish resume terminal event', {
1292-
resumeExecutionId,
1293-
status: terminalStatus,
1294-
error: toError(error).message,
1295-
})
1296-
throw new Error(TERMINAL_PUBLISH_ERROR)
1322+
? await eventWriter.writeTerminal(event, terminalStatus).catch(async (error) => {
1323+
await degradeTerminalPublish(terminalStatus, error)
1324+
return { eventId: 0, executionId: resumeExecutionId, event }
12971325
})
12981326
: await eventWriter.write(event)
12991327
event.eventId = entry.eventId
@@ -1686,9 +1714,10 @@ export class PauseResumeManager {
16861714
status: finalMetaStatus,
16871715
replayBufferFlushed,
16881716
})
1689-
if (!executionError) {
1690-
executionError = new Error(TERMINAL_PUBLISH_ERROR)
1691-
}
1717+
await degradeTerminalPublish(
1718+
finalMetaStatus,
1719+
new Error('Terminal event was never published')
1720+
)
16921721
} else {
16931722
await eventWriter.close().catch((error) => {
16941723
logger.warn('Failed to close resume event writer after terminal publish', {
@@ -1707,6 +1736,13 @@ export class PauseResumeManager {
17071736
*/
17081737
await loggingSession.waitForPostExecution()
17091738

1739+
if (terminalPublishDegraded) {
1740+
logger.warn('Resume settled with a degraded run buffer', {
1741+
resumeExecutionId,
1742+
status: finalMetaStatus,
1743+
})
1744+
}
1745+
17101746
if (executionError || !result) {
17111747
throw executionError ?? new Error('Resume execution did not produce a result')
17121748
}

0 commit comments

Comments
 (0)