Skip to content

feat(workflow-engine): add getVersion primitive for workflow versioning#5308

Closed
abcxff wants to merge 1 commit into
mainfrom
stack/slop-claude-opus-4-8-high-feat-workflow-engine-add-getversion-primitive-for-workflow-versioning-wpnprqor
Closed

feat(workflow-engine): add getVersion primitive for workflow versioning#5308
abcxff wants to merge 1 commit into
mainfrom
stack/slop-claude-opus-4-8-high-feat-workflow-engine-add-getversion-primitive-for-workflow-versioning-wpnprqor

Conversation

@abcxff

@abcxff abcxff commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@abcxff

abcxff commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Stack for rivet-dev/rivet

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

@railway-app

railway-app Bot commented Jun 19, 2026

Copy link
Copy Markdown

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

Service Status Web Updated (UTC)
kitchen-sink 😴 Sleeping (View Logs) Web Jul 3, 2026 at 6:46 pm
website 😴 Sleeping (View Logs) Web Jul 3, 2026 at 6:22 pm
ladle ❌ Build Failed (View Logs) Web Jul 3, 2026 at 6:13 pm
frontend-cloud ❌ Build Failed (View Logs) Web Jul 3, 2026 at 6:13 pm
frontend-inspector ❌ Build Failed (View Logs) Web Jul 3, 2026 at 6:13 pm
mcp-hub ✅ Success (View Logs) Web Jul 3, 2026 at 6:13 pm

@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5308 June 19, 2026 13:46 Destroyed
@abcxff abcxff marked this pull request as draft June 19, 2026 13:46
@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Review: getVersion primitive for workflow versioning

Solid feature with thorough engine-level test coverage (247 lines covering fresh/old/loop-cutover/retirement/divergence cases). The core "unvisited-scope" heuristic in executeGetVersion checks out against the traced scenarios, including the per-iteration loop cutover test. A couple of real gaps in the frontend integration and a duplication nit below.

Bugs

  1. TypeIcon is missing a version_check casefrontend/src/components/actors/workflow/xyflow-nodes.tsx. TYPE_COLORS was given a dedicated indigo (#6366f1) entry for version_check, but the TypeIcon switch has no matching case "version_check", so it silently falls through to the default branch and renders the generic faCircleCheck icon instead of a distinct icon. The color was clearly meant to pair with its own icon.

  2. getEntrySummary is missing a version_check casefrontend/src/components/actors/workflow/workflow-to-xyflow.ts (not touched by this PR, so it wasn't updated). Its switch also has a default: return "" fallback, so version_check nodes in the workflow visualizer will render with a blank summary instead of something useful like the resolved version.

    Both of these slipped through specifically because these two switches use a catch-all default instead of exhaustively enumerating the union, which is exactly the pattern CLAUDE.md's "Match statements" section warns against ("Never use a _ => fall-through arm when matching on a ... TypeScript discriminated union ... so adding a new variant later is a compile error instead of a silent behavior change"). Neither the TS compiler nor the test suite caught the gap because of the default. Worth fixing the immediate cases, and possibly worth removing the default arms in both switches (or adding an assertUnreachable) so the compiler catches this the next time an entry kind is added — same pattern already used in workflow/inspector.ts's toWorkflowEntryKind.

Code quality

  1. Duplicated prefix-scan logicrivetkit-typescript/packages/workflow-engine/src/context.ts:2660-2676 (hasUnvisitedUnderCurrentScope) reimplements the exact same string-based "is this key under the current location prefix" check already in validateComplete() (context.ts:571-593), down to the same prefix === "" ? true : key.startsWith(prefix + "/") || key === prefix logic. Consider factoring this into a single private helper (e.g. keysUnderCurrentScope(excludeKey?)) that both validateComplete and hasUnvisitedUnderCurrentScope call, to avoid two copies of a subtle invariant that need to stay in sync.

Minor / worth confirming

  1. New BARE union variant on the only published schema versionVersionCheckEntry was appended to EntryKind in v1.bare directly (CURRENT_VERSION stays 1 in versioned.ts), rather than introducing a v2.bare bridge. Appending to the end of a union is wire-compatible for old data being read by new code, but during a rolling deploy an old binary reading history written by a new binary would hit default: throw new bare.BareError(offset, "invalid tag") on tag 8. If prior EntryKind variants were added the same way historically this is probably an accepted tradeoff for this schema, but flagging since CLAUDE.md calls out "never modify an existing published *.bare version" as a general rule for versioned BARE schemas.

  2. Error message inconsistency — the HistoryDivergedError thrown in executeGetVersion reads Expected version_check at ${key}, found ${existing.kind.type} while every sibling primitive (step/loop/sleep/join/rollback checkpoint) includes the entry's own name: Expected step "${config.name}" at ${key}, found .... Minor, but worth matching for consistent diagnostics.

  3. rivetkit (Rust) parity — per CLAUDE.md, new user-facing capabilities added to rivetkit-typescript should be mirrored in rivetkit (Rust) "where practical, best-effort." This PR doesn't add a get_version/equivalent to the Rust Ctx<A> API. Not blocking, just flagging in case it should be tracked as a follow-up.

Test coverage

Engine-side logic is well covered. There's no test coverage for the frontend workflow visualizer changes (icon/summary rendering for the new entry kind), which is how bugs #1 and #2 above went unnoticed.

@abcxff abcxff force-pushed the stack/slop-claude-opus-4-8-high-feat-workflow-engine-add-getversion-primitive-for-workflow-versioning-wpnprqor branch from 4492a9d to ea8aaf0 Compare June 23, 2026 00:53
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5308 June 23, 2026 00:53 Destroyed
@abcxff abcxff force-pushed the stack/slop-claude-opus-4-8-high-feat-workflow-engine-add-getversion-primitive-for-workflow-versioning-wpnprqor branch from ea8aaf0 to 2635c05 Compare June 23, 2026 04:47
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5308 June 23, 2026 04:47 Destroyed
@NathanFlurry NathanFlurry changed the title [SLOP(claude-opus-4-8-high)] feat(workflow-engine): add getVersion primitive for workflow versioning feat(workflow-engine): add getVersion primitive for workflow versioning Jun 26, 2026
@abcxff abcxff marked this pull request as ready for review July 3, 2026 18:08
@railway-app railway-app Bot temporarily deployed to rivet-frontend / rivet-pr-5308 July 3, 2026 18:08 Destroyed
@abcxff abcxff force-pushed the stack/slop-claude-opus-4-8-high-feat-workflow-engine-add-getversion-primitive-for-workflow-versioning-wpnprqor branch from fb2ba72 to 2635c05 Compare July 3, 2026 18:34
@abcxff abcxff closed this Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant