fix(execution): stop run-buffer failures from stranding resumed executions - #6187
Conversation
…d 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.
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Reconnect SSE now ends with Per-user Redis byte budget keys use a fixed TTL window: Removes the unreachable Reviewed by Cursor Bugbot for commit 7bb2ca6. Configure here. |
Greptile SummaryThe PR makes terminal run-buffer publication failures nonfatal after resumed workflow execution and consistently applies fixed TTL windows to per-user Redis budgets while retaining sliding execution-scoped budgets.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported user-budget concern was resolved in-thread as an intentional bounded rate-window tradeoff, and the current Lua paths consistently implement that design while deleting nonpositive counters atomically.
|
| Filename | Overview |
|---|---|
| apps/sim/lib/workflows/executor/human-in-the-loop-manager.ts | Degrades terminal event publication failures into metadata updates and logging rather than failing an already-executed resume. |
| apps/sim/lib/execution/redis-budget.server.ts | Changes user budget counters to fixed TTL windows while preserving sliding execution-budget expiration. |
| apps/sim/lib/execution/event-buffer.ts | Applies the fixed user-budget window consistently across event-buffer growth, shrinkage, no-op refresh, and reset paths. |
| apps/sim/lib/uploads/utils/user-file-base64.server.ts | Aligns upload-cache user-budget cleanup and replacement paths with fixed-window expiration. |
| apps/sim/app/api/workflows/[id]/executions/[executionId]/stream/route.ts | Closes degraded terminal streams with the normal completion marker instead of surfacing a replay error. |
Reviews (3): Last reviewed commit: "fix(stream): end a replay cleanly when t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4c7df92. Configure here.
…nal 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.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7bb2ca6. Configure here.
1 participant
Summary
activestream, logs, and lets the resume settle on its real result.TERMINAL_PUBLISH_ERRORconstant and its branch. Pre-execution buffer failures stay fatal and retryable, since no work has happened yet.Type of Change
Testing
apps/sim/lib/execution/redis-budget.server.test.tsplus two cases inevent-buffer.test.ts. Both new guards verified to fail when the fix is reverted.tsc -p apps/sim, biome, andcheck:api-validationpass.lib/execution,lib/workflows/executor,lib/uploads, andapp/api/resume.Checklist