feat(table): chunked dispatcher + workflow cascade + UX polish#4672
Open
TheodoreSpeaks wants to merge 14 commits into
Open
feat(table): chunked dispatcher + workflow cascade + UX polish#4672TheodoreSpeaks wants to merge 14 commits into
TheodoreSpeaks wants to merge 14 commits into
Conversation
Replaces the all-rows-at-once runWorkflowColumn with a row-window dispatcher
backed by a new table_run_dispatches row. Each user click inserts a dispatch
row and triggers a trigger.dev task that crawls the table 20 rows at a time,
re-enqueueing itself between windows. The HTTP/Mothership entrypoints return
{ dispatchId } immediately instead of holding the request open for minutes
on multi-thousand-row dispatches.
- Per-row cancel stamps cancelledAt; the dispatcher skips cells whose
cancelledAt > dispatch.requestedAt so a mid-cascade cancel sticks even
under isManualRun.
- Table-wide cancel marks active dispatches cancelled atomically so the
dispatcher bails on its next iteration.
- New 'dispatch' SSE event variant plumbed; client ignores for v1.
Run-column with run-mode 'all' wasn't visually flipping rows that already had data — the cell renderer's "value wins" branch kept showing the prior output behind the queued/running state. The dispatcher only cleared one window of rows at a time, so most of the column stayed stale until the cursor walked to it. Now: - Dispatcher's `pending → dispatching` transition runs a single SQL UPDATE that wipes targeted `data` output columns and `executions[gid]` across every targeted row (mode-aware: 'incomplete' skips fully-filled rows). - Per-window clear in `dispatcherStep` is gone — rows are pre-cleared, the loop only filters cancel tombstones / unmet deps and enqueues. - Optimistic patch in `useRunColumn` mirrors the bulk clear by nulling output values in the cached row, so the UI flips queued/running instantly without waiting for the SSE catch-up.
The eager bulk clear for mode: 'incomplete' only skipped rows that were already fully filled, so two overlapping dispatches could race — dispatch B would nuke executions[gid] on a row dispatch A had just stamped 'queued', flickering the cell and potentially confusing the worker. Skip any row whose targeted group is currently queued/running/pending — an 'incomplete' run shouldn't touch what another dispatch is actively working on. The per-walk 'in-flight' eligibility skip already handles rows that flip in-flight between the clear and the cursor reaching them.
Switch the per-window cell fan-out from fire-and-forget tasks.trigger to tasks.batchTriggerAndWait. The dispatcher is now a single long-lived trigger.dev task that loops dispatcherStep until the table is exhausted; trigger.dev CRIU-checkpoints the parent during each wait so we don't pay compute while cells execute. Queue depth is bounded at WINDOW_SIZE per dispatch — no more flooding trigger.dev with a million queued runs. - dispatcher.ts builds payloads via the new shared buildPendingRuns helper and calls tasks.batchTriggerAndWait directly. Pre-stamps each cell to `queued` (jobId=null) so the UI flips instantly. - table-run-dispatcher.ts is now a plain while-true loop. No RUN_BUDGET_MS, no self-re-enqueue, no cold-start tax per window. Cancel: - New cancelCellRunsByTags(tags) paginates runs.list + runs.cancel(id). - cancelWorkflowGroupRuns fires the tag-sweep alongside the per-jobId queue.cancelJob path (preserved for auto-fire cells that have real jobIds from single tasks.trigger calls). - Trigger.dev acks the cancel → batchTriggerAndWait resumes → dispatcher observes the dispatch-row cancel flag → exits. Side fixes: - getAsyncBackendType returns 'trigger-dev' whenever taskContext.isInsideTask is true, regardless of TRIGGER_DEV_ENABLED env. The preview/dev-sim worker silently routing cell jobs to DatabaseJobQueue (no poller) is fixed without any env config change. - runWorkflowColumn skips the dispatcher entirely when trigger.dev is disabled, running cells inline via DatabaseJobQueue.runInline. HTTP response returns dispatchId: null in that mode. - runColumnContract response schema updated to dispatchId.nullable().
isExecInFlight required a jobId for `pending` status, gating it as "real backend pending" vs "optimistic flag only." The row-gutter Stop button keyed on this — so a freshly clicked Play sat as `pending` (no jobId) and the user couldn't cancel it until the server-side `queued` stamp arrived via SSE. With the dispatcher pre-batch stamping cells as `queued` (not `pending`) and no per-cell jobIds under batchTriggerAndWait, the gap was worse. Drop the jobId requirement. `pending` now counts as in-flight everywhere. Cancel writes `cancelled` to the cell exec authoritatively whether or not a real trigger.dev run exists yet — cancelling an optimistic cell means "don't run this," which is correct. Also collapse isOptimisticInFlight into isExecInFlight since the two helpers are now identical.
Two coupled changes: 1. Cell-task runs the row's full cascade in-process. executeWorkflowGroupCellJob acquires a Redis lock per (tableId, rowId) with heartbeat (10s/30s TTL), then loops through eligible workflow groups for the row. One cell-task = one row's full cascade, not N. Resume worker holds the same lock and continues the cascade after a HITL resume. Shared withCascadeLock helper in lib/table/cascade-lock.ts. 2. Every cell-enqueue goes through the dispatcher. The implicit scheduleRunsForRows reactor in service.ts is removed — 8 callsites (insertRow, batchInsertRows, upsertRow, updateRowsByFilter, batchUpdateRows, addWorkflowGroup, updateWorkflowGroup) now fire runWorkflowColumn with mode: 'incomplete', isManualRun: false. HTTP routes that call updateRow directly also fire runWorkflowColumn afterwards. scheduleRunsForTable / scheduleRunsForRowIds deleted; scheduleRunsForRows demoted to private (only the TRIGGER_DEV_ENABLED=false fallback uses it). skipScheduler flag dropped from UpdateRowData / BatchUpdateByIdData — no longer meaningful since there's nothing implicit to suppress. Plumbed isManualRun through the dispatch row (new is_manual_run column, default true) so auto-fire callers honor autoRun: false and don't re-run completed cells. Stamp 'pending' (not 'queued', executionId: null) before batchTriggerAndWait — cell-task writes its own 'queued' on lock acquire. Small UI polish: row gutter Play button spacing, "Delete workflow" → "Delete column" label, optimistic-pending cells now show Stop button (isExecInFlight no longer requires jobId).
…Id cell The dispatcher's pre-batch `pending` stamp leaves executionId unset so any cell-task that wins the cascade lock can claim the cell. The cancellation- guard SQL clause was rejecting these claims because it tested `executions->gid IS NULL` (whole exec missing) but the pre-stamp leaves the exec present with executionId=null. Add a third carve-out: `executions->gid->>'executionId' IS NULL`. Now the guard reads "write allowed if no exec exists, OR no executionId is set yet, OR the executionId matches ours." Symptom: every cell-task's first markWorkflowGroupPickedUp call would log "SQL guard saw cancelled" and skip, leaving cells stuck at the dispatcher's pending stamp.
The dispatcher's row-window SELECT is `position > cursor` for exclusive lower-bound semantics. With cursor initialized to 0, position-0 rows were never picked up — every dispatch silently skipped the table's first row. Start cursor at -1 instead. First window's filter `position > -1` matches position 0; subsequent iterations advance to `lastPosition` which then correctly excludes already-processed rows.
…el via 'new' mode Fix 0: new `DispatchMode = 'new'` for auto-fire callsites. Eligibility skips rows with any prior `executions[gid]` entry — cancelled / errored / completed cells stay sticky until a manual run. Dispatcher's windowed SELECT pushes `NOT jsonb_exists_any(...)` to SQL so CSV imports into mostly-attempted tables don't pay a per-window load+JS-filter. `batchInsertRows` drops its `rowIds` payload (keeps dispatch scope tiny on big imports). Fix A/B/D: client optimistic patches now mirror the backend's actual invariants. `useCreateTableRow.onSuccess` stamps eligible groups via `optimisticallyScheduleNewlyEligibleGroups` so newly-inserted rows show `Queued` instantly. `useCancelTableRuns.onMutate` distinguishes optimistic- only pending (`executionId == null` — strip silently) from real worker claims (stamp cancelled; SSE will reconcile). Drop `onSettled` invalidation on `useUpdateTableRow` / `useBatchUpdateTableRows` to kill the delete-cell flicker. Fix C: active-dispatches overlay. New `listActiveDispatches` helper, contract, and `GET /api/table/[tableId]/dispatches` route. `kind:'dispatch'` SSE events carry scope+cursor+mode on every transition. New `useActiveDispatches` hook + `resolveCellExec` synthesize a virtual `pending` exec for cells in an active dispatch's scope ahead of cursor — queued indicators now survive page refresh during long Run-all dispatches. `cancelWorkflowGroupRuns` emits `kind:'dispatch',status:'cancelled'` events so the overlay clears without a refetch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`runWorkflowColumn` now always inserts a `table_run_dispatches` row and
drives the dispatcher state machine. The trigger.dev / in-process branch
narrows to a single line: trigger.dev fires `tableRunDispatcherTask` (which
calls the new `runDispatcherToCompletion`), the inline path calls the same
helper fire-and-forget. Deletes `scheduleRunsForRows` and
`stampQueuedOrCancel` — the inline-fallback no longer duplicates window
walking, SSE emission, or cancel.
The dispatcher's window-execute call goes through `JobQueueBackend`:
- New `batchEnqueueAndWait` interface method.
- Trigger.dev impl wraps `tasks.batchTriggerAndWait` behind a
`taskContext.isInsideTask` guard (clear error if called from outside a
task).
- Database impl skips `async_jobs` entirely — `Promise.all` over
`options.runner(payload, signal)` per item, with per-cell AbortControllers
tracked by `cancelKey` for cancel.
`cancelInlineRun` moves to the interface as `cancelByKey` so
`cancelWorkflowGroupRuns` no longer reaches into the database backend.
Fix `mode: 'new'` SQL filter:
- `${array}::text[]` interpolated as a tuple-cast which Postgres rejected
("cannot cast type record to text[]") and every inline dispatch silently
failed. Switched to `ARRAY[${sql.join(...)}]::text[]`.
- Predicate was `jsonb_exists_any` ("any one targeted group present"),
which excluded rows that needed at least one group re-run after a
downstream output was deleted. Switched to `jsonb_exists_all` — per-group
JS eligibility handles the rest.
Cascade-loop workflowId bug: `runRowCascadeLoop` was not threading the new
group's `workflowId` when advancing across groups. The cell-task ran the
previous group's workflow against the next group's cell, terminating
`completed` with empty `accumulatedData`. Fixed by tracking
`currentWorkflowId` alongside `currentGroupId` / `currentExecutionId`.
Client optimistic-patch tightening:
- `useRunColumn.onMutate` mirrors server eligibility — skip cells with
unmet deps so unmet rows don't flash Queued and get stuck (no SSE will
arrive for cells the server skipped).
- `resolveCellExec` overlay synthesizes a virtual `pending` only when
`areGroupDepsSatisfied` is true. Rows with unmet deps render Waiting,
matching the dispatcher's actual behavior.
Cleanup from /simplify pass:
- Use `generateShortId(20)` instead of
`generateId().replace(/-/g, '').slice(0, 20)`.
- Inline `batchEnqueueAndWait` no longer allocates synthetic ids
(returned `string[]` is unused).
- Flattened the per-cell `tracked` array — only push entries that
registered controllers, drop the null placeholders.
- Extracted `runDispatcherToCompletion` to share the loop between the
trigger.dev wrapper and the in-process path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lish
Counter (Fix 1): top-right "X running" + per-row badge are now
backend-bootstrapped via a count on `user_table_rows.executions ->> 'status'
= 'running'` returned alongside active dispatches. SSE `kind: 'cell'` events
compute a delta from `prev → next` status to keep the cache live; cell
events for rows outside the loaded page slice trigger a run-state refetch.
On `pruned` we invalidate the cache. Counts only worker-claimed `running`
cells — optimistic queued/pending no longer inflate the badge, and rows
outside the loaded page slice are counted too.
Sidebar (Fix 2 + 3a): `Run after` no longer ticks every column by default
for new groups (empty list). Save is disabled with an inline error when
auto-run is on with zero deps. `edit-group` mode anchors the left-of-current
filter to the group's leftmost column, so a workflow can only depend on
columns to its left.
Reorder scrub (Fix 3b): `updateTableMetadata` walks the schema's workflow
groups when `columnOrder` is in the patch and drops any dep whose new
position lands at or after the group's leftmost column (uses the existing
`stripGroupDeps` helper). Metadata + schema updates land atomically.
Server returns ordered columns (Fix 3b cont'd): `getTableById` /
`listTables` now sort `schema.columns` by `metadata.columnOrder` before
returning, via a new `applyColumnOrderToSchema` helper. Every consumer
(grid, sidebar, copilot, mothership) gets one ordered list — the sidebar's
leftmost-group-column anchor now points at the right index.
Dep-aware retrigger (Fix 4): editing a value that a downstream workflow
depends on now re-runs that workflow.
- `deriveExecClearsForDataPatch` returns
`{ executionsPatch, inFlightDownstreamGroups }`. Walks
`schema.workflowGroups[].dependencies.columns` for every column in the
patch, clears terminal-state downstream entries, and reports in-flight
entries.
- `updateRow` calls `cancelWorkflowGroupRuns` + `runWorkflowColumn`
(`mode: 'incomplete' + isManualRun: true`) for in-flight downstream
groups, then always fires `runWorkflowColumn({ mode: 'new' })` for the
cleared groups. Skips both when `executionsPatch` is provided by the
caller — those are cell-task / cancel writes that would otherwise spawn
a recursive flood of dispatches per partial-write.
- `cancelWorkflowGroupRuns(tableId, rowId, { groupIds? })` accepts a
per-group filter so the cancel only touches the affected groups, not
every in-flight cell on the row.
- `pickNextEligibleGroupForRow` now treats a dispatcher pre-stamp
(`pending` + `executionId: null`) as claimable — the cascade-loop is the
real owner. Without this, the dispatcher's pre-stamp of downstream
groups made the cascade-loop see them as "in-flight" and skip them,
stranding `pending` cells forever.
- `optimisticallyScheduleNewlyEligibleGroups` extends the cache patch to
flip dep-touched groups to `pending` regardless of their current status,
matching the server's cancel-then-rerun behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…der Pending + viewable Three connected issues with workflows that pause mid-cell (e.g. wait blocks): 1. `/api/resume/poll` (the time-pause auto-resumer) called `PauseResumeManager.startResumeExecution` directly, bypassing `executeResumeJob` from `background/resume-execution.ts`. The wrapper is where the cell-context restoration + cascade-loop continuation lives — without it, the resumed workflow ran to completion but never wrote the terminal state back to the table cell. Cell stays `pending` forever even though the underlying execution finished. Fix: dynamically import `executeResumeJob` and use it for the `'starting'` branch. Same primitive the trigger.dev `resumeExecutionTask` wraps — calling it directly handles both trigger.dev-disabled local dev and trigger.dev-enabled prod identically. 2. The cell renderer mapped `status: 'pending'` to `kind: 'queued'` (gray "Queued" badge) regardless of whether the run had started. A HITL-paused run has `status: 'pending'` + `jobId` prefixed `paused-` + a real `executionId` — semantically very different from "queued, hasn't run." Now renders as `pending-upstream` (the existing Pending pill) for paused-jobId rows. 3. Right-click "View execution" was disabled for `pending` cells (gated to `completed | error | running`), so users couldn't open the trace for a paused execution. Paused runs have a viewable trace (the executionId is real and the log row exists). Both the per-row context menu and the action-bar derivation now recognize `pending` + `paused-` jobId as a started run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workflow-output cells now reveal their text character-by-character when an SSE update lands, while page reloads and virtualization remounts still paint the value instantly. A first-render guard inside the new useTypewriter hook distinguishes hydration from live updates with no plumbing through the cell tree. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Adds a run-state surface: Reworks workflow cascade + resume semantics: introduces a Redis Tightens auto-run/optimistic behavior and validation: Includes UI polish: paused HITL cells render as pending (and are viewable), workflow output updates get a typewriter reveal, and some menu/label tweaks (e.g., delete label). Reviewed by Cursor Bugbot for commit b2b1a83. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR rewrites the table workflow execution engine: a chunked dispatcher (
Confidence Score: 3/5Two PATCH routes each fire a second The chunked dispatcher, cascade lock, and backend unification are well-architected and the logic in those layers is sound. The regressions are in the routing layer:
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant PATCHRoute as PATCH /rows/[rowId]
participant ServiceUpdateRow as service.updateRow
participant Dispatcher as dispatcherStep
participant CellTask as executeWorkflowGroupCellJob
participant CascadeLock as withCascadeLock
Client->>PATCHRoute: PATCH row data
PATCHRoute->>ServiceUpdateRow: updateRow(data, no executionsPatch)
ServiceUpdateRow->>ServiceUpdateRow: deriveExecClearsForDataPatch
ServiceUpdateRow-->>ServiceUpdateRow: void runWorkflowColumn mode new fires dispatch 1
ServiceUpdateRow-->>PATCHRoute: updatedRow
PATCHRoute-->>PATCHRoute: void runWorkflowColumn mode incomplete fires dispatch 2
Note over Dispatcher: Dispatch 1 mode new no bulk clear
Dispatcher->>Dispatcher: buildPendingRuns skips rows with existing exec
Dispatcher->>Dispatcher: stampQueuedForBatch
Note over Dispatcher: Dispatch 2 mode incomplete bulk clear runs
Dispatcher->>Dispatcher: bulkClearWorkflowGroupCells wipes completed outputs
Dispatcher->>CellTask: batchEnqueueAndWait
CellTask->>CascadeLock: withCascadeLock(tableId, rowId, executionId)
CascadeLock-->>CellTask: acquired
CellTask->>CellTask: runRowCascadeLoop groups A then B then C
CellTask->>CascadeLock: releaseLock
Reviews (1): Last reviewed commit: "feat(table): typewriter reveal for SSE-d..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+137
to
+147
| // Only `null` when a `cancellationGuard` is supplied and the SQL guard | ||
| // rejects the write — this route doesn't pass one, so reaching null is a bug. | ||
| if (!updatedRow) throw new Error('updateRow returned null without a cancellationGuard') | ||
| // Auto-fire any newly-eligible workflow groups (deps just became met). | ||
| void runWorkflowColumn({ | ||
| tableId, | ||
| workspaceId: validated.workspaceId, | ||
| rowIds: [updatedRow.id], | ||
| mode: 'incomplete', | ||
| isManualRun: false, | ||
| requestId, |
Contributor
There was a problem hiding this comment.
updateRow in service.ts now unconditionally fires runWorkflowColumn({mode:'new', rowIds:[rowId]}) at its own exit for all non-internal writes. Adding a second runWorkflowColumn({mode:'incomplete'}) call here produces two concurrent dispatches for every PATCH.
The dangerous path is bulkClearWorkflowGroupCells for mode:'incomplete'. It clears all targeted groups' output columns on any row where at least one output column is empty — there is no per-group completed check. If a row has group A completed (col_a = 'foo') and group B not yet run (col_b = ''), the NOT allFilled filter fires and col_a is wiped. The same issue exists in the v1 PATCH route.
Suggested change
| // Only `null` when a `cancellationGuard` is supplied and the SQL guard | |
| // rejects the write — this route doesn't pass one, so reaching null is a bug. | |
| if (!updatedRow) throw new Error('updateRow returned null without a cancellationGuard') | |
| // Auto-fire any newly-eligible workflow groups (deps just became met). | |
| void runWorkflowColumn({ | |
| tableId, | |
| workspaceId: validated.workspaceId, | |
| rowIds: [updatedRow.id], | |
| mode: 'incomplete', | |
| isManualRun: false, | |
| requestId, | |
| // Auto-dispatch is handled inside updateRow (mode: 'new') for user edits. |
Comment on lines
+148
to
+155
| void runWorkflowColumn({ | ||
| tableId, | ||
| workspaceId: validated.workspaceId, | ||
| rowIds: [updatedRow.id], | ||
| mode: 'incomplete', | ||
| isManualRun: false, | ||
| requestId, | ||
| }).catch((err) => logger.error(`[${requestId}] auto-dispatch (v1 row update) failed:`, err)) |
Contributor
There was a problem hiding this comment.
updateRow already fires runWorkflowColumn({mode:'new'}) for user edits, so this second mode:'incomplete' call creates a concurrent dispatch that can wipe completed workflow outputs.
Suggested change
| void runWorkflowColumn({ | |
| tableId, | |
| workspaceId: validated.workspaceId, | |
| rowIds: [updatedRow.id], | |
| mode: 'incomplete', | |
| isManualRun: false, | |
| requestId, | |
| }).catch((err) => logger.error(`[${requestId}] auto-dispatch (v1 row update) failed:`, err)) | |
| // Auto-dispatch is handled inside updateRow (mode: 'new') for user edits. |
Comment on lines
+463
to
+465
| const allGroups = table.schema.workflowGroups ?? [] | ||
| const targetGroups = groupIds ? allGroups.filter((g) => groupIds.includes(g.id)) : allGroups | ||
| if (targetGroups.length === 0) return { triggered: 0 } | ||
| if (targetGroups.length === 0) throw new Error('No matching workflow groups for run') |
Contributor
There was a problem hiding this comment.
runWorkflowColumn throws 'No matching workflow groups for run' when targetGroups.length === 0. Every caller wraps it in .catch(logger.error), so every row insert/update on a plain table generates an error-level log. The previous code returned { triggered: 0 } silently. The fix is if (targetGroups.length === 0) return { dispatchId: null }.
Suggested change
| const allGroups = table.schema.workflowGroups ?? [] | |
| const targetGroups = groupIds ? allGroups.filter((g) => groupIds.includes(g.id)) : allGroups | |
| if (targetGroups.length === 0) return { triggered: 0 } | |
| if (targetGroups.length === 0) throw new Error('No matching workflow groups for run') | |
| const allGroups = table.schema.workflowGroups ?? [] | |
| const targetGroups = groupIds ? allGroups.filter((g) => groupIds.includes(g.id)) : allGroups | |
| if (targetGroups.length === 0) return { dispatchId: null } |
Comment on lines
+147
to
+152
| const applyDispatch = (event: Extract<TableEvent, { kind: 'dispatch' }>): void => { | ||
| const { dispatchId, status, scope, cursor, mode } = event | ||
| queryClient.setQueryData<TableRunState>(tableKeys.activeDispatches(tableId), (prev) => { | ||
| if (!prev) return prev | ||
| const list = prev.dispatches | ||
| // Terminal states drop the dispatch from the overlay; client renders |
Contributor
There was a problem hiding this comment.
isManualRun hardcoded false for SSE-inserted dispatches
When a dispatch SSE event arrives for a dispatchId not already in the cache, the entry is inserted with isManualRun: false regardless of the actual run type. The preserved-from-initial-fetch path only applies to entries already present (merged[idx] = { ...next, isManualRun: list[idx].isManualRun }), so a late-arriving manual run renders with the wrong flag.
Two P1 issues + one cleanup from the bot reviewers:
1. **Double-dispatch + completed-output wipe.** Both PATCH row routes
(`app/api/table/[tableId]/rows/[rowId]` and
`app/api/v1/tables/[tableId]/rows/[rowId]`) were firing a second
`runWorkflowColumn({ mode: 'incomplete' })` after `updateRow` returns.
`updateRow` already fires `mode: 'new'` internally for user edits, so
the second call created a concurrent dispatch. Worse, the
`mode: 'incomplete'` path's `bulkClearWorkflowGroupCells` wipes ALL
targeted output columns on any row where any one column is empty —
meaning sibling-group completed outputs could be erased. Removed both
route-level calls; auto-dispatch lives entirely in `updateRow`.
2. **`runWorkflowColumn` log-spamming on plain tables.**
`if (targetGroups.length === 0) throw new Error(...)` fired on every
row insert/update for tables without any workflow groups (the
majority). Every caller wraps with `.catch(logger.error)`, so each
PATCH produced an error-level log. Return `{ dispatchId: null }`
silently — manual `runWorkflowColumn` callers pass `groupIds`
explicitly so they can't reach this branch.
3. **`isManualRun` plumbed through dispatch SSE events.** Late-arriving
`kind: 'dispatch'` events for dispatches not in the initial fetch
were hardcoding `isManualRun: false`. Added the field to the event
shape, emit it from `dispatcherStep` (pending → complete, dispatching
transitions) and `markActiveDispatchesCancelled`, and consume it in
the SSE handler with a sensible fallback for legacy emits.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b2b1a83. Configure here.
| requestId, | ||
| }).catch((err) => | ||
| logger.error(`[${requestId}] auto-dispatch (Mothership update_row) failed:`, err) | ||
| ) |
There was a problem hiding this comment.
Mothership double workflow dispatch
High Severity
After update_row, Mothership still calls runWorkflowColumn with mode: 'incomplete' even though updateRow already auto-dispatches with mode: 'new'. That second run races the first and can run bulkClearWorkflowGroupCells, wiping sibling workflow outputs on the row.
Reviewed by Cursor Bugbot for commit b2b1a83. Configure here.
| const applyDispatch = (event: Extract<TableEvent, { kind: 'dispatch' }>): void => { | ||
| const { dispatchId, status, scope, cursor, mode, isManualRun } = event | ||
| queryClient.setQueryData<TableRunState>(tableKeys.activeDispatches(tableId), (prev) => { | ||
| if (!prev) return prev |
There was a problem hiding this comment.
SSE dispatch dropped before fetch
Medium Severity
applyDispatch returns early when the activeDispatches query cache is still empty, so the first kind: 'dispatch' SSE events are discarded. The queued overlay and cursor may stay wrong until a full refetch, undermining live Run-all UX.
Reviewed by Cursor Bugbot for commit b2b1a83. Configure here.
| // wiped AND downstream dep-changed terminal groups — same rationale as | ||
| // `updateRow`. In-flight downstream groups are dropped here (batch | ||
| // updates don't run the cancel+restart orchestration — those go through | ||
| // single-row `updateRow`). |
There was a problem hiding this comment.
Batch skips in-flight rerun
Medium Severity
batchUpdateRows uses deriveExecClearsForDataPatch but ignores inFlightDownstreamGroups, so dependency edits in a batch update never cancel and restart downstream workflows still running. Single-row updateRow does perform that cancel-and-rerun path.
Reviewed by Cursor Bugbot for commit b2b1a83. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tableRunDispatchesrows + windowed walk viabatchTriggerAndWait(tableId, rowId); reactor removed; every cell-enqueue routes throughrunWorkflowColumnJobQueueBackend.batchEnqueueAndWaitmode: 'new'for auto-fire — skips rows with any priorexecutions[gid]; SQL pushdown viaNOT jsonb_exists_allschema.columnssorted bymetadata.columnOrder/api/resume/pollroutes throughexecuteResumeJobso paused cells get cell-context restoration in local dev; paused cells render Pending pill and surface View ExecutionType of Change
Testing
Tested manually — CSV import, Run all + Stop + refresh, dep edits re-run downstream workflows, cancel scoped per-row, delete-cell flicker gone, reorder scrubs deps, paused (wait-block) cells resume via cron and complete cleanly. tsc + vitest (198/198) +
check:api-validation:strictpass.Checklist