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

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/rivet

• [SLOP(claude-opus-4-8-high)] fix(workflow-engine): mark completed inner-loop iterations visited on resume #5324 wrxknkmx · 2026-06-24 15:26:44 👈
• main

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

[claude]

PR Review: Mark completed inner-loop iterations visited on resume

This PR fixes a real HistoryDivergedError crash in the workflow engine: when a nested loop resumes from a saved iteration, its already-completed prior iterations were never marked visited, causing the enclosing scope's validateComplete() to reject them as history divergences. The fix (markCompletedLoopIterationsVisited) and its test are conceptually correct for the stated scenario.

However, the same root cause exists in two adjacent code paths that the fix does not reach.

---

1. Early-return paths for already-completed loops still have the same gap (confirmed bug)

context.ts lines 1231-1237

When a loop has already completed (status === completed or output !== undefined), it returns before line 1244 without calling markCompletedLoopIterationsVisited:

if (metadata.status === 'completed') {
    return loopData.output as T;   // no markCompletedLoopIterationsVisited
}
if (loopData.output !== undefined) {
    return loopData.output as T;   // no markCompletedLoopIterationsVisited
}

Failure scenario: outer loop at saved iteration K; within outer iteration K, inner loop A completed in a previous run (ran one step, broke) and inner loop B suspended (the case this PR fixes). On resume, outer iteration K replays: inner loop A hits line 1232 or 1237, marks only its own root key visited (line 1197), then returns. Inner loop B is correctly resumed (line 1244 fires). But inner loop A's iteration sub-entries -- e.g. outerK/innerA/~0/step1 -- are still in storage.history.entries and were never added to visitedKeys. branchCtx.validateComplete() at line 1323 then throws HistoryDivergedError.

Pruning does not reliably prevent this: collectLoopPruning only fires when currentIteration > historySize (default 20), so any inner loop that completed in fewer than 20 iterations retains all its iteration entries in storage.

Suggested fix: call markCompletedLoopIterationsVisited before each early return, using loopData.iteration + 1 so all completed iterations 0..iteration are marked. The same applies to the rollback early-return at lines 1223-1224.

---

2. iterationSegment.loop !== loopSegment is always false (cleanup)

context.ts line 538

Both loopSegment (loopLocation[last]) and iterationSegment.loop resolve to registerName(storage, config.name) -- the same idempotent registry call. When isLocationPrefix(loopLocation, entry.location) passes, this condition can never be true. It mirrors the identical unreachable guard in collectLoopPruning at line 1432. Worth removing or annotating if intentionally defensive.

---

3. Multi-paragraph docstring violates CLAUDE.md (conventions)

context.ts lines 499-513

The new method has an 11-line JSDoc block. CLAUDE.md states: 'Never write multi-paragraph docstrings or multi-line comment blocks -- one short line max.' The why is non-obvious and warrants a comment, but it should be condensed to one line.

[abcxff]

LGTM

[railway-app]

🚅 Deployed to the rivet-pr-5324 environment in rivet-frontend

Service	Status	Web	Updated (UTC)
kitchen-sink	😴 Sleeping (View Logs)	Web	Jun 24, 2026 at 3:57 pm
frontend-cloud	😴 Sleeping (View Logs)	Web	Jun 24, 2026 at 3:54 pm
frontend-inspector	😴 Sleeping (View Logs)	Web	Jun 24, 2026 at 3:54 pm
website	😴 Sleeping (View Logs)	Web	Jun 24, 2026 at 3:35 pm
ladle	✅ Success (View Logs)	Web	Jun 24, 2026 at 3:30 pm
mcp-hub	✅ Success (View Logs)	Web	Jun 24, 2026 at 3:27 pm