[SLOP(claude-opus-4-8-high)] feat(workflow-engine): add getVersion primitive for workflow versioning

[abcxff]

No description provided.

[abcxff]

Stack for rivet-dev/rivet

• [SLOP(claude-opus-4-8-high)] feat(workflow-engine): add getVersion primitive for workflow versioning #5308 wpnprqor · 2026-06-19 13:46:07 👈
• main

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

[railway-app]

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

Service	Status	Web	Updated (UTC)
website	😴 Sleeping (View Logs)	Web	Jun 19, 2026 at 1:58 pm
frontend-cloud	❌ Build Failed (View Logs)	Web	Jun 19, 2026 at 1:47 pm
ladle	❌ Build Failed (View Logs)	Web	Jun 19, 2026 at 1:47 pm
frontend-inspector	❌ Build Failed (View Logs)	Web	Jun 19, 2026 at 1:47 pm
kitchen-sink	❌ Build Failed (View Logs)	Web	Jun 19, 2026 at 1:46 pm
mcp-hub	✅ Success (View Logs)	Web	Jun 19, 2026 at 1:46 pm

[claude]

Code Review: PR #5308

Summary: This PR adds a getVersion(name, latest) primitive to the workflow engine that allows workflow code to branch by version. Fresh instances resolve to latest; old in-flight instances that already have history entries in their current scope resolve to the implicit floor version 1. The feature touches the persistence schema, BARE codec, inspector transport, frontend visualizer, and test harness.

---

Core Logic

Correctness of the old-in-flight detection heuristic ✓

The central mechanism in hasUnvisitedUnderCurrentScope is correct and elegant: because earlier entries in the same execution are already marked visited by the time getVersion runs, any unvisited entry under the current scope must come from a prior execution, proving the scope already ran under pre-gate code.

Bug: false detection when gate is placed before loop with existing iteration history

The hasUnvisitedUnderCurrentScope method iterates over the entire history and filters by prefix. When the gate appears at the root scope (prefix === ""), isUnderPrefix is always true, meaning all history entries are examined -- including sibling loop iterations not yet visited in the current execution.

Example: v1 has a loop that runs 3 iterations. After iteration 0 completes, iterations 1 and 2 have entries in history. If v2 adds a root-scope getVersion gate before the loop, those unvisited iteration entries cause hasUnvisitedUnderCurrentScope to return true, incorrectly resolving to floor 1 even for a fresh run on a new instance.

Recommendation: Restrict the root-scope scan to entries at exactly depth 1 from the current location rather than all descendants.

Performance: O(N) scan over full history on every getVersion call

hasUnvisitedUnderCurrentScope iterates all history keys on each call. When getVersion is called inside a high-iteration loop, cost becomes O(N*iterations). Since visitedKeys is maintained incrementally, a complementary "unvisited count under current scope" could make this O(1).

Validation gap: no resolved <= latest check on replay

When replaying an existing version_check entry, there is no guard that existing.kind.data.resolved <= latest. If latest is accidentally decreased between deployments (e.g., from 3 to 2 while resolved was 3), the returned value exceeds the caller's latest. Recommend throwing a HistoryDivergedError in this case.

API design: floor version 1 is implicit but undocumented

getVersion("gate", 3) on an old in-flight instance returns 1, not 2. Old-in-flight instances always get the floor, never any intermediate version. This should be explicitly documented in the JSDoc -- user code branching on v === 2 will silently never execute for old in-flight instances when jumping from v1 to v3.

---

Schema & Serialization

BARE persistence schema correctly updated ✓ -- VersionCheckEntry added as tag 8, append-only union ordering preserved.

Transport BARE schema -- missing exhaustive guard: writeWorkflowEntryKind in transport/v1.ts has no x satisfies never / assertUnreachable(x) after its switch. Future variants could be silently dropped.

---

Frontend

getEntrySummary silently skips version_check: Falls through to default: return "". Should have an explicit case "version_check" arm returning the resolved version. Per CLAUDE.md, closed discriminated unions should enumerate all variants.

TypeIcon falls through to generic icon for version_check: Renders faCircleCheck via the default: arm. Should be an explicit case with an appropriate icon (e.g., faCodeBranch or faTag).

transform-workflow-history.ts switch is exhaustive ✓

---

Tests

Solid baseline coverage ✓ -- fresh-instance resolution, old-in-flight detection, correct pinning on replay, removed() retirement, HistoryDivergedError on type mismatch, per-iteration cutover.

Gap: per-iteration cutover test hardcodes yield mode -- not tested in live mode. Should be added to the parametrized loop or have a comment explaining the exclusion.

Gap: no test for root-scope gate with existing loop iteration history -- directly related to the high-severity bug above.

Gap: no test for multi-version skip -- no test verifying that getVersion("gate", 3) on old in-flight always returns 1, not 2.

---

Summary

Category	Severity	Finding
Bug	High	hasUnvisitedUnderCurrentScope false positives when root-scope gate runs after partially-visited loop history
Correctness	Medium	No resolved <= latest guard on replay
API contract	Medium	Floor-to-1 skip semantics undocumented at call site
Frontend	Low	getEntrySummary returns "" for version_check; needs explicit case
Frontend	Low	TypeIcon falls through to generic icon for version_check
Codec	Low	writeWorkflowEntryKind lacks exhaustive guard after switch
Tests	Low	Per-iteration cutover test only in yield mode
Tests	Low	No test for root-scope gate with existing loop iteration history
Tests	Low	No test for multi-version skip
Perf	Low	O(N) history scan per getVersion call inside loops

The high-severity false-detection bug is the main blocker. The resolved <= latest guard and the undocumented floor-version semantics are worth addressing before merging.

🤖 Generated with Claude Code