fix(gas): fix history location assignment for optional execs #4102

MasterPtato · 2026-02-03T21:19:31Z

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

MasterPtato · 2026-02-03T21:19:43Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add 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.

pkg-pr-new · 2026-02-03T21:22:06Z

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4102

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@4102

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4102

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4102

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4102

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4102

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4102

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4102

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4102

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4102

commit: cc9ebec

railway-app · 2026-02-04T18:49:41Z

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

Service	Status	Web	Updated (UTC)
mcp-hub	✅ Success (View Logs)	Web	Feb 4, 2026 at 6:50 pm
frontend-inspector	🕒 Building (View Logs)	Web	Feb 4, 2026 at 6:49 pm
website	🕒 Building (View Logs)	Web	Feb 4, 2026 at 6:49 pm
frontend-cloud	🕒 Building (View Logs)	Web	Feb 4, 2026 at 6:49 pm

graphite-app · 2026-02-04T18:50:12Z

Merge activity

Feb 4, 6:50 PM UTC: MasterPtato added this pull request to the Graphite merge queue.
Feb 4, 6:51 PM UTC: CI is running for this pull request on a draft pull request (#4113) due to your merge queue CI optimization settings.
Feb 4, 6:51 PM UTC: Merged by the Graphite merge queue via draft PR: #4113.

# Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. ## Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes

claude · 2026-02-04T18:54:12Z

Code Review: PR #4102 - Fix history location assignment for optional execs

Summary

This PR fixes a bug in the Gasoline workflow engine where Option<T: Executable> was incorrectly shifting the cursor even when the option was None. This caused history location misalignment and incorrect indentation when displaying workflow history.

Changes Reviewed

1. engine/packages/gasoline/src/executable.rs (Primary Fix)

Change: Modified shift_cursor() for Option<T: Executable> to only increment the cursor when Some.

Analysis:

✅ Correct Fix: Properly aligns with the execute() implementation which only executes when Some
✅ Consistency: Matches patterns used by other Executable implementations
✅ Reasoning: The cursor tracks workflow history position. None should not consume a history position
⚠️ Potential Concern: Behavioral change may affect existing workflows with Option::None in their history

2. engine/packages/engine/src/util/wf/mod.rs (Display Fix)

Change: Added indentation before printing version check information (line 497).

Analysis:

✅ Correct: Matches the indentation pattern used by all other event types
✅ Consistency: Follows existing visual structure patterns

Code Quality Assessment

Strengths:

Minimal, focused changes that address the root cause
Follows existing code patterns and conventions
Proper use of conditional logic

Areas for Improvement:

Missing Tests: No tests were added to verify the fix
Missing Documentation: The PR description is empty
Migration Path: No consideration for backward compatibility

Security Considerations

✅ No security concerns identified

Performance Considerations

✅ No performance impact

Breaking Changes

⚠️ Potentially Breaking: Workflows with existing history containing Option::None executables may experience history divergence errors during replay

Recommendations

✅ Approve the logic fix - The change is correct and necessary
⚠️ Request tests - Add unit tests for Option<T: Executable> cursor behavior
⚠️ Clarify migration - Document whether this breaks existing workflows
📝 Update PR description - Replace template with actual description

Verdict

Conditional Approval: The code changes are correct and fix a real bug, but the PR needs:

Unit tests to prevent regression
Clarity on backward compatibility impact
Proper PR description

The fix itself is sound and follows best practices.

MasterPtato requested a review from NathanFlurry February 3, 2026 21:19

This was referenced Feb 3, 2026

feat: rewrite load test suite #4099

Closed

fix(gateway): prevent timeout after hibernation wake, improve logging #4090

Closed

fix(api): use matched path to reduce metric cardinality #4094

Closed

This was referenced Feb 3, 2026

chore(guard): centralize cors config #4097

Closed

fix(pb): fix metrics wf inconsistent calculation #4103

Closed

fix(pb): handle going away signals gracefully #4104

Closed

fix(pb): add backoff to error retries of metadata poller #4105

Closed

MasterPtato force-pushed the 02-02-feat_rewrite_load_test_suite branch from 07288c0 to 1728003 Compare February 3, 2026 21:30

MasterPtato force-pushed the 02-03-fix_gas_fix_history_location_assignment_for_optional_execs branch from cc9ebec to 813aa32 Compare February 3, 2026 21:30

This was referenced Feb 3, 2026

fix(pb): set actor sleep key when allocated after pending #4107

Closed

chore: add udb and gas metrics #4112

Closed

fix(gas): fix history location assignment for optional execs

2818a7d

MasterPtato force-pushed the 02-03-fix_gas_fix_history_location_assignment_for_optional_execs branch from 813aa32 to 2818a7d Compare February 4, 2026 18:49

MasterPtato force-pushed the 02-02-feat_rewrite_load_test_suite branch from 1728003 to 349575f Compare February 4, 2026 18:49

railway-app bot temporarily deployed to rivet-frontend / rivet-pr-4102 February 4, 2026 18:49 Destroyed

graphite-app bot closed this Feb 4, 2026

graphite-app bot deleted the 02-03-fix_gas_fix_history_location_assignment_for_optional_execs branch February 4, 2026 18:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(gas): fix history location assignment for optional execs #4102

fix(gas): fix history location assignment for optional execs #4102

MasterPtato commented Feb 3, 2026

Uh oh!

MasterPtato commented Feb 3, 2026 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Feb 3, 2026

Uh oh!

railway-app bot commented Feb 4, 2026 •

edited

Loading

Uh oh!

graphite-app bot commented Feb 4, 2026 •

edited

Loading

Uh oh!

claude bot commented Feb 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix(gas): fix history location assignment for optional execs #4102

fix(gas): fix history location assignment for optional execs #4102

Conversation

MasterPtato commented Feb 3, 2026

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

MasterPtato commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

pkg-pr-new bot commented Feb 3, 2026

Uh oh!

railway-app bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graphite-app bot commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

claude bot commented Feb 4, 2026

Code Review: PR #4102 - Fix history location assignment for optional execs

Summary

Changes Reviewed

1. engine/packages/gasoline/src/executable.rs (Primary Fix)

2. engine/packages/engine/src/util/wf/mod.rs (Display Fix)

Code Quality Assessment

Security Considerations

Performance Considerations

Breaking Changes

Recommendations

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

MasterPtato commented Feb 3, 2026 •

edited

Loading

railway-app bot commented Feb 4, 2026 •

edited

Loading

graphite-app bot commented Feb 4, 2026 •

edited

Loading