fix(stepfunctions): fix execution overwrite, IsPresent null, catcher short-circuit#274
Merged
vieiralucas merged 4 commits intomainfrom Apr 12, 2026
Merged
Conversation
…short-circuit, wait warnings - Guard succeed_execution/fail_execution against overwriting terminal states (ABORTED) - Fix IsPresent to treat present-but-null fields as present (check parent object) - Fix find_catcher to skip malformed catchers instead of aborting via ? operator - Add warning log when Wait state has no valid wait parameters
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-stepfunctions/src/choice.rs">
<violation number="1" location="crates/fakecloud-stepfunctions/src/choice.rs:240">
P2: The new `IsPresent` existence check regresses JsonPath support by not handling array-index segments (e.g. `$.items[0]`), so existing fields can be treated as missing.</violation>
</file>
<file name="crates/fakecloud-stepfunctions/src/interpreter.rs">
<violation number="1" location="crates/fakecloud-stepfunctions/src/interpreter.rs:1539">
P1: The terminal-status guard runs after writing the terminal history event, so aborted/stopped executions can still record `ExecutionSucceeded`/`ExecutionFailed` events. Move the guard before `add_event` to avoid inconsistent history.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…ndle array indices in IsPresent - Move terminal-status guard before add_event to prevent recording spurious history events - Handle array-index segments (e.g. $.items[0]) in field_exists_in_input for IsPresent
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-stepfunctions/src/choice.rs">
<violation number="1" location="crates/fakecloud-stepfunctions/src/choice.rs:255">
P1: Array index parsing assumes a trailing `]` and can panic or mis-parse malformed JsonPath segments.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="crates/fakecloud-stepfunctions/src/choice.rs">
<violation number="1" location="crates/fakecloud-stepfunctions/src/choice.rs:260">
P2: Array-index path parsing now accepts malformed segments with trailing characters after `]`, which can make invalid JsonPath expressions evaluate as existing fields.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
succeed_execution/fail_executionagainst overwriting terminal states (e.g., ABORTED byStopExecution) — background interpreter no longer clobbers stopped executionsIsPresentto correctly returntruefor fields explicitly set tonull— checks parent object instead of relying onresolve_pathwhich returnsValue::Nullfor both missing and nullfind_catcherto skip malformed catchers (missingNext) instead of aborting the entire search via?operatorAddresses unresolved Cubic findings from PRs #241, #244, #242.
Test plan
cargo clippy -p fakecloud-stepfunctions -- -D warningspassestest_is_present_with_null_value,test_find_catcher_skips_malformed_and_finds_next)Summary by cubic
Fixes in
fakecloud-stepfunctions: stop overwriting terminal executions by guarding beforeadd_event, treat explicit nulls as present forIsPresent(including array indices; reject malformed segments likefoo[0]extra), skip catchers withoutNext, and warn when Wait lacks valid Seconds/SecondsPath/Timestamp/TimestampPath. Prevents clobbered ABORTED runs and addresses Cubic findings from #241, #242, and #244.Written for commit 5aad2bf. Summary will update on new commits.