fix(rivetkit/wf): replay successful result mechanism#4310
Conversation
|
🚅 Deployed to the rivet-pr-4310 environment in rivet-frontend
|
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
PR Review: fix(rivetkit/wf): replay successful result mechanismThe fix correctly addresses a real bug: void steps/loops (those returning What's good
Issues / suggestions1. Missing debug log after refactor (minor) The original code logged when a step was replayed from history: this.log("debug", { msg: "replaying step from history", step: config.name, key });This was silently dropped in the refactor. This log is useful for diagnosing replay issues. Consider restoring it inside the new guard: if (metadata.status === "completed" || stepData.output \!== undefined) {
this.log("debug", { msg: "replaying step from history", step: config.name, key });
return stepData.output as T;
}2. Before the fix, non-void steps had a fast path — they returned immediately without loading metadata. Now 3. Rollback mode behavior change for completed void loops (needs confirmation) In the original code, a completed void loop in rollback mode fell through to re-execute one iteration (because both guards evaluated false): // rollback guard: loopData.output === undefined → no early return
// completion guard: loopData.output === undefined → no early return
// → rollbackSingleIteration = true, loop re-runsAfter the fix, 4. If CI workflow changeRemoval of SummaryThe bug fix is correct and the test coverage is appropriate. The four items above are minor (one log, one perf note, one rollback edge case needing a test, one readability comment). None are blockers, but item 3 (rollback behavior for void loops) is worth a quick sanity check. |
d5f9531 to
2841137
Compare

FIXES RVT-6000
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: