fix(workflow-engine): mark completed inner-loop iterations visited on resume

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/actors

• #5605 fix(workflow-engine): mark completed inner-loop iterations v … 👈
• #5603 fix(rivetkit): expose engine ray IDs on errors for debug
• #5601 fix(frontend): submit Vercel drain grace period
• #5604 fix(rivetkit-napi): remove stale HttpRequestBodyStream test …
• main

Get stack: forklift get 5605
Push local edits: forklift submit
Merge when ready: forklift merge 5605

_{change vqowwtqx}

[claude]

Review

Reviewed the fix in rivetkit-typescript/packages/workflow-engine/src/context.ts and the accompanying test.

Correctness

The fix looks correct. When a loop resumes at a persisted iteration > 0, only the resumed iteration and later are replayed — earlier iterations' history entries are never re-visited via the normal markVisited calls that happen inside step/queue/etc. If that loop is nested inside another loop (or any branch), the enclosing branch's validateComplete() walks the entire subtree under its location and would incorrectly throw HistoryDivergedError for those legitimately-skipped entries.

Walking through markCompletedLoopIterationsVisited:

• The resumedIteration <= 0 early-return avoids the scan entirely for the common case (no completed iterations yet), which is a nice optimization.
• Scoping via isLocationPrefix(loopLocation, entry.location) plus checking the segment right after loopLocation.length correctly limits the scan to this loop's own iteration markers, and since it doesn't cap depth, it transitively covers everything nested inside earlier completed iterations (further nested loops/steps), which is exactly what's needed.
• The iterationSegment.loop !== loopSegment check is redundant in practice (only this loop's own appendLoopIteration calls ever place a marker at that exact position), but it's reasonable as defense-in-depth.
• Correctly leaves the resumed iteration (>= resumedIteration) untouched so it's still validated normally as the loop replays forward.
• Runs regardless of rollbackMode, which seems right since validateComplete() is exercised on rollback branches too, though I didn't see any rollback-specific coverage for the nested-loop-resume case in loops.test.ts (rollback isn't tested anywhere in that file, so this isn't a regression, just a gap worth knowing about if rollback + nested suspending loops is a real use case).

Minor nit

!iterationSegment (context.ts:534) treats a NameIndex of 0 as falsy, which is technically imprecise, but the very next check (!isLoopIterationMarker(iterationSegment)) already filters out any non-marker segment including 0, so there's no behavioral bug — just slightly easy to misread. Not blocking.

Performance

markCompletedLoopIterationsVisited does a full scan of storage.history.entries per resumed loop call. This matches an existing pattern already used in collectLoopPruning (context.ts:1423), so it's consistent with the codebase's existing tradeoffs, and it's bounded by loop history pruning (historySize/historyPruneInterval). Worth keeping in mind that in "yield" mode, a deeply nested chain of suspend-heavy loops (e.g. an inner loop that suspends on every iteration inside an outer loop, both with large surviving history) will re-run this scan on every full replay, so it's O(replays × history size) in the worst case. Not a blocker given the precedent and pruning bounds, but worth watching if this pattern shows up in a hot path with very large histories.

Tests

The new test (should resume an inner loop that suspends mid-iteration inside a parent loop) directly reproduces the bug: an inner loop nested in an outer loop, suspending via queue.next mid-iteration, verified across both yield and live modes. It's a solid, targeted regression test that matches the fix's stated scope. No concerns there.

Style / CLAUDE.md conventions

Comments are written as complete sentences without dashes, consistent with the repo's comment style guidance. No terminology, protocol, or trust-boundary concerns apply here — this is pure workflow-engine internal logic.

Summary

No blocking issues found. The fix is well-scoped, the reasoning in the doc comment is clear and matches the implementation, and the test directly targets the regression. The rollback-mode gap and the redundant falsy check are minor, non-blocking observations.