Skip to content

Commit 4c7bc83

Browse files
fix(wait): address bot review — setNextResumeAt + suspend unit default
- setNextResumeAt now matches paused OR partially_resumed; otherwise the cron poller can't null nextResumeAt after dispatching a chained-wait row, so it keeps reappearing in every poll batch until execution ends (flagged by both greptile and bugbot) - Suspend mode now defaults missing timeUnitLong to 'minutes' instead of falling back to 'seconds' and immediately erroring (flagged by bugbot)
1 parent 4a180c0 commit 4c7bc83

3 files changed

Lines changed: 22 additions & 4 deletions

File tree

apps/sim/executor/handlers/wait/wait-handler.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,19 @@ describe('WaitBlockHandler', () => {
136136
).rejects.toThrow('Wait time exceeds maximum of 5 minutes')
137137
})
138138

139+
it('should default the suspend unit to minutes when timeUnitLong is missing', async () => {
140+
vi.setSystemTime(new Date('2026-04-28T00:00:00.000Z'))
141+
142+
const result = (await handler.execute(mockContext, mockBlock, {
143+
suspend: true,
144+
timeValue: '3',
145+
})) as Record<string, any>
146+
147+
const waitMs = 3 * 60 * 1000
148+
expect(result.waitDuration).toBe(waitMs)
149+
expect(result.status).toBe('waiting')
150+
})
151+
139152
it('should reject seconds as a unit when Suspend Workflow is enabled', async () => {
140153
await expect(
141154
handler.execute(mockContext, mockBlock, {

apps/sim/executor/handlers/wait/wait-handler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class WaitBlockHandler implements BlockHandler {
126126
): Promise<BlockOutput> {
127127
const suspend = inputs.suspend === true || inputs.suspend === 'true'
128128
const timeValue = Number.parseFloat(inputs.timeValue || '10')
129-
const timeUnit = (suspend ? inputs.timeUnitLong : inputs.timeUnit) || 'seconds'
129+
const timeUnit = suspend ? inputs.timeUnitLong || 'minutes' : inputs.timeUnit || 'seconds'
130130

131131
if (!Number.isFinite(timeValue) || timeValue <= 0) {
132132
throw new Error('Wait amount must be a positive number')

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1692,9 +1692,11 @@ export class PauseResumeManager {
16921692
}
16931693

16941694
/**
1695-
* Updates `next_resume_at` only when the row is still `status='paused'`.
1695+
* Updates `next_resume_at` only when the row is still in a poll-eligible state.
16961696
* Guard prevents the cron poller from clobbering a freshly-written value when a
1697-
* concurrent manual resume has already advanced the row's state.
1697+
* concurrent manual resume has already advanced the row's state. `partially_resumed`
1698+
* rows must also be writable so the cron poller can null out their `nextResumeAt`
1699+
* after dispatch; otherwise the row keeps reappearing in every poll batch.
16981700
*/
16991701
static async setNextResumeAt(args: {
17001702
pausedExecutionId: string
@@ -1704,7 +1706,10 @@ export class PauseResumeManager {
17041706
.update(pausedExecutions)
17051707
.set({ nextResumeAt: args.nextResumeAt })
17061708
.where(
1707-
and(eq(pausedExecutions.id, args.pausedExecutionId), eq(pausedExecutions.status, 'paused'))
1709+
and(
1710+
eq(pausedExecutions.id, args.pausedExecutionId),
1711+
inArray(pausedExecutions.status, ['paused', 'partially_resumed'])
1712+
)
17081713
)
17091714
}
17101715

0 commit comments

Comments
 (0)