fix(tables): reliable stop-all, accurate "X running", and rate/usage gating for cell runs#4838
Conversation
…gating for cell runs Stop-all: - Make the cancellation guard status-based (not executionId-scoped) so a `cancelled` tombstone stamped while a cell is still a dispatcher pre-stamp (null executionId) keeps the cell dead — fixes function-execute cells that resurrected after Stop all. Consolidated into shared isExecCancelled / isExecCancelledAfter predicates in deps.ts, reused by the in-memory guard, the SQL guard, the dispatcher tombstone filter, the worker, and resume. - Add an explicit pre-execution cancellation read so a cell that dequeues after Stop all (e.g. from the trigger.dev queue) never runs. - Resume worker aborts a cancelled paused/awaiting cell before resuming; cancelWorkflowGroupRuns marks paused executions cancelling. "X running": - Emit a dispatch SSE at dispatch start so auto-fired/capped runs surface immediately; show the control whenever a dispatch is active. Checkbox dependency: - Treat boolean `false` as an unmet dependency so unchecking never reruns dependents — only checking does. deriveExecClearsForDataPatch no longer re-arms a downstream group whose deps are unmet after the patch. Rate / usage gating: - Route table cell execution through preprocessExecution (billing actor = workspace billed account, usage limit, per-plan timeout), keeping draft. - Rate limit: pace & retry per cell (async counter) so rows aren't skipped. - Usage limit: halt the dispatch without marking cells and emit a usageLimitReached event; the client shows an Upgrade prompt that routes to subscription settings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Run status UI keeps Stop all visible while any dispatch is active ( Checkbox dependencies: Table cell execution now uses Reviewed by Cursor Bugbot for commit aee1374. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@greptile review |
Greptile SummaryThis PR hardens the table cell-run pipeline across four axes: reliable Stop All (status-based cancel guard that can't be resurrected by a later worker), accurate "X running" display via a new dispatch-start SSE event and a client
Confidence Score: 5/5Safe to merge — the cancel guard, billing integration, and dispatch-visibility changes are well-reasoned and each corner case is explicitly addressed with a comment or test. The three concurrent-hazard fixes (status-based cancel guard, completeDispatchIfActive CAS, mid-sleep DB cancel check) are all correctly implemented and mutually consistent. The preprocessExecution integration preserves the existing billing/usage contract. The hasActiveDispatch and dispatch-start SSE changes are narrowly scoped to the client overlay and carry no data-correctness risk. workflow-column-execution.ts and dispatcher.ts warrant the most attention — they carry the new usage-limit and rate-limit paths. The interactions between completeDispatchIfActive, the post-window readDispatch guard, and the absence of a complete dispatch SSE event after a usage-limit halt are correct but subtle. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Dispatcher
participant CellWorker
participant PreprocessExecution
participant RateLimiter
participant DB
Client->>Dispatcher: trigger dispatch
Dispatcher->>DB: pending to dispatching
Dispatcher->>Client: SSE dispatch status dispatching cursor 0
Note over Client: hasActiveDispatch true, Show X running and Stop All
Dispatcher->>DB: stamp cells pending null executionId
Dispatcher->>CellWorker: batchEnqueueAndWait
CellWorker->>DB: getRowById
CellWorker->>CellWorker: cancelledBeforeRun check
alt Stop All fired
DB-->>CellWorker: exec status cancelled
CellWorker-->>Dispatcher: return error skip
else Not cancelled
CellWorker->>PreprocessExecution: checkUsage billing checkRateLimit false
alt Usage limit hit 402
PreprocessExecution-->>CellWorker: success false statusCode 402
CellWorker->>DB: clear pre-stamp executionsPatch null
CellWorker->>DB: completeDispatchIfActive CAS
CellWorker->>Client: SSE usageLimitReached
Note over Client: Remove dispatch overlay toast upgrade prompt
CellWorker-->>Dispatcher: return blocked
else Usage OK
loop Rate-limit pacing up to 6 attempts
CellWorker->>RateLimiter: checkRateLimitWithSubscription
alt Rate limited
CellWorker->>CellWorker: sleep jitter backoff
CellWorker->>DB: re-check cancelled tombstone
else Allowed
CellWorker->>DB: markWorkflowGroupPickedUp
CellWorker->>CellWorker: executeWorkflow with timeout
CellWorker->>DB: writeTerminalState
end
end
end
end
Dispatcher->>DB: readDispatch check if halted
alt Dispatch still active
Dispatcher->>Client: SSE dispatch status dispatching cursor N
Dispatcher->>Dispatcher: next window
else Halted
Dispatcher-->>Client: done no stale event
end
Reviews (7): Last reviewed commit: "fix(tables): don't emit stale dispatchin..." | Re-trigger Greptile |
… cancel Addresses PR review: - Usage limit: only the cell that transitions the dispatch active→complete (via completeDispatchIfActive) emits usageLimitReached, so concurrent cells don't fire up to 20 identical "upgrade" toasts. - Rate-limit retry: re-check the cancelled tombstone after each sleep so a Stop All mid-wait releases the concurrency slot promptly (signal never fires on the trigger.dev backend).
|
@greptile review |
Greptile SummaryThis PR hardens table cell execution across four dimensions: reliable stop-all cancellation (status-based guard instead of executionId-scoped), accurate "X running" display (new dispatch-start SSE event), correct checkbox dependency semantics (
Confidence Score: 3/5Mostly safe to merge with one real UX defect: concurrent workers hitting the usage cap all fire The cancel-guard, checkbox-dep, and resume-worker fixes are clean and well-tested. The dispatch-start SSE and apps/sim/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-event-stream.ts (the Important Files Changed
Sequence DiagramsequenceDiagram
participant D as Dispatcher
participant W as Cell Worker
participant DB as Database
participant SSE as SSE Stream
participant UI as Client UI
D->>DB: stamp cells pending
D->>SSE: dispatch-start event
SSE->>UI: hasActiveDispatch true, show X running
D->>W: enqueue trigger.dev task with dispatchId
W->>DB: getRowById cancelledBeforeRun check
alt cell cancelled before dequeue
W-->>W: return error, skip without overwriting cancel
else cell not cancelled
W->>W: preprocessExecution billing usage timeout
alt usage limit 402
W->>DB: markDispatchComplete
W->>SSE: usageLimitReached event
SSE->>UI: remove dispatch and show upgrade toast once
W-->>W: return blocked
else rate limited 429
loop up to 6 attempts
W->>W: sleep clamped retryAfterMs
W->>W: checkRateLimit
end
end
W->>DB: markWorkflowGroupPickedUp with SQL guard status not cancelled
alt Stop All cancelled cell during sleep
DB-->>W: skipped, guard rejected
else normal execution
W->>W: executeWorkflow with timeout signal
W->>DB: writeWorkflowGroupState
W->>SSE: cell event
SSE->>UI: update cell state
end
end
Reviews (2): Last reviewed commit: "fix(tables): dedupe usage-limit event + ..." | Re-trigger Greptile |
|
@greptile review |
Passing the bucket's shared resetAt as retryAfterMs made backoffWithJitter return a fixed clamped value (no jitter, attempt ignored), so all concurrent cells retried in lockstep. Pass null to get jittered exponential backoff.
|
@greptile review |
…kflow # Conflicts: # apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx
Addresses PR review: - Clear each blocked cell's pre-stamp on a 402 so it reverts to un-run instead of being stuck "Queued" (no error/cancelled badge); covers auto-fire cells with no owning dispatch. - Client re-syncs run-state counts and refetches rows on usageLimitReached so the stale "X running" / Stop-all control clears and queued cells drop. - Make usageLimitReached.dispatchId optional; client only touches the dispatch overlay when present.
|
@greptile review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8bd4694. Configure here.
If a cell halts the dispatch mid-window (usage limit), re-read the dispatch status after the batch and bail instead of emitting a per-window 'dispatching' event that would arrive after the client dropped the dispatch and re-add it (flickering 'X running' back).
|
@greptile review |

Summary
isExecCancelled/isExecCancelledAfterpredicates.falseis treated as an unmet dependency, so only checking a box triggers downstream runs; unchecking still persists the value.preprocessExecution(the same path other triggers use): attributes usage/billing to the workspace billed account, enforces the usage limit, applies the per-plan timeout, keeps running draft. Rate limit → pace & retry on the async counter (rows aren't skipped); usage limit → halt the dispatch without marking cells and surface an upgrade prompt.Type of Change
Testing
preprocessExecutionlogging-suppression option) — passing.bun run lint:checkandbun run check:api-validation:strictpass.Checklist