Skip to content

feat(stepfunctions): implement States.JsonMerge intrinsic - #1661

Closed
abanna wants to merge 1 commit into
floci-io:mainfrom
abanna:feat/stepfunctions-json-merge
Closed

feat(stepfunctions): implement States.JsonMerge intrinsic#1661
abanna wants to merge 1 commit into
floci-io:mainfrom
abanna:feat/stepfunctions-json-merge

Conversation

@abanna

@abanna abanna commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

AslExecutor did not implement the States.JsonMerge intrinsic, so a state machine using it failed
with Unsupported intrinsic function: States.JsonMerge. This implements the shallow merge: the
second object's top-level fields override the first's on a key conflict. Deep-merge (third argument
true) and non-object arguments are rejected, matching AWS. The intrinsic-arg resolver also learns
true/false/null literals so the third argument parses.

Type of change

  • New feature (feat:)

AWS Compatibility

AWS Step Functions' States.JsonMerge(obj1, obj2, false) performs a shallow merge and only
supports the shallow form — passing true (deep merge) is a documented error. Behavior verified
against the intrinsic functions reference:
second object wins on conflict, deep-merge rejected, non-object args rejected.

Checklist

  • ./mvnw test passes locally — AslExecutorJsonMergeTest (run in eclipse-temurin:25-jdk)
  • New test added — AslExecutorJsonMergeTest (shallow merge, deep-merge rejection, non-object rejection, arg-count)
  • Commit messages follow Conventional Commits

AslExecutor did not support the States.JsonMerge intrinsic, failing with
'Unsupported intrinsic function: States.JsonMerge'. Implement the shallow merge
(second object's top-level fields win on a key conflict); deep-merge (third
argument true) and non-object arguments are rejected, matching AWS Step
Functions. Also resolve boolean/null literal intrinsic arguments so the third
argument parses. Covered by AslExecutorJsonMergeTest.
@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown

Greptile Summary

This PR implements the States.JsonMerge intrinsic function in AslExecutor, previously causing a hard failure for any state machine that used it. It also extends resolveIntrinsicArg to handle true, false, and null literals, which are needed to parse the mandatory boolean third argument.

  • AslExecutor.java: Adds the States.JsonMerge case — validates exactly 3 arguments, rejects deep-merge (true), rejects non-object inputs, and performs a shallow merge where the second object's fields win on conflicts.
  • AslExecutorJsonMergeTest.java: New unit test class covering happy-path shallow merge, deep-merge rejection, non-object rejection, and wrong argument count.

Confidence Score: 4/5

Safe to merge — the core shallow-merge logic is correct and well-tested; the main concern is a silent boolean coercion that only activates on malformed inputs.

The happy path (literal false third arg, two plain objects) works correctly and is covered by tests. The only concrete flaw is that .asBoolean() silently returns false for null or non-boolean resolved arguments, so States.JsonMerge($.a, $.b, $.nullRef) would succeed and merge instead of being rejected. This would only arise with invalid input, but the emulator should still reject it rather than silently coerce it.

The third-argument resolution in AslExecutor.java (around the deep boolean extraction) deserves a closer look before merging.

Important Files Changed

Filename Overview
src/main/java/io/github/hectorvent/floci/services/stepfunctions/AslExecutor.java Adds States.JsonMerge intrinsic and true/false/null literal support in resolveIntrinsicArg; the deep-flag extraction uses .asBoolean() without validating the node is actually a boolean, allowing null or other non-boolean references to silently become false.
src/test/java/io/github/hectorvent/floci/services/stepfunctions/AslExecutorJsonMergeTest.java New test class covering the four main JsonMerge scenarios; class name does not follow the project's *ServiceTest.java unit-test convention from AGENTS.md, and a test for the null-as-third-arg edge case is absent.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["States.JsonMerge(a, b, deep)"] --> B[splitIntrinsicArgs]
    B --> C{3 args?}
    C -->|No| ERR1[FailStateException: requires exactly 3 arguments]
    C -->|Yes| D[resolveIntrinsicArg arg1 - JsonNode a]
    D --> E[resolveIntrinsicArg arg2 - JsonNode b]
    E --> F["resolveIntrinsicArg arg3 → .asBoolean() → deep"]
    F --> G{deep is true?}
    G -->|Yes| ERR2[FailStateException: shallow merge only]
    G -->|No| H{a and b are objects?}
    H -->|No| ERR3[FailStateException: requires two JSON objects]
    H -->|Yes| I[createObjectNode merged]
    I --> J[copy all fields from a]
    J --> K[copy all fields from b - overrides on conflict]
    K --> L[yield merged ObjectNode]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["States.JsonMerge(a, b, deep)"] --> B[splitIntrinsicArgs]
    B --> C{3 args?}
    C -->|No| ERR1[FailStateException: requires exactly 3 arguments]
    C -->|Yes| D[resolveIntrinsicArg arg1 - JsonNode a]
    D --> E[resolveIntrinsicArg arg2 - JsonNode b]
    E --> F["resolveIntrinsicArg arg3 → .asBoolean() → deep"]
    F --> G{deep is true?}
    G -->|Yes| ERR2[FailStateException: shallow merge only]
    G -->|No| H{a and b are objects?}
    H -->|No| ERR3[FailStateException: requires two JSON objects]
    H -->|Yes| I[createObjectNode merged]
    I --> J[copy all fields from a]
    J --> K[copy all fields from b - overrides on conflict]
    K --> L[yield merged ObjectNode]
Loading

Reviews (1): Last reviewed commit: "feat(stepfunctions): implement States.Js..." | Re-trigger Greptile

}
JsonNode a = resolveIntrinsicArg(parts.get(0).trim(), root);
JsonNode b = resolveIntrinsicArg(parts.get(1).trim(), root);
boolean deep = resolveIntrinsicArg(parts.get(2).trim(), root).asBoolean();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The third argument is read with .asBoolean() directly, which returns false for any non-boolean node (null, string, number). If the argument is a reference path that resolves to null or a non-boolean value at runtime, the executor silently treats it as false and performs the merge instead of rejecting the call. Adding an explicit isBoolean() guard preserves AWS's rejection of non-boolean third arguments.

Suggested change
boolean deep = resolveIntrinsicArg(parts.get(2).trim(), root).asBoolean();
JsonNode deepArg = resolveIntrinsicArg(parts.get(2).trim(), root);
if (!deepArg.isBoolean()) {
throw new FailStateException("States.Runtime",
"States.JsonMerge third argument must be a boolean");
}
boolean deep = deepArg.booleanValue();

* previously failed with "Unsupported intrinsic function: States.JsonMerge".
*/
class AslExecutorJsonMergeTest {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test naming deviates from AGENTS.md convention

AGENTS.md specifies that unit tests should be named *ServiceTest.java. The class AslExecutorJsonMergeTest doesn't follow that pattern. Renaming it to AslExecutorServiceTest (or appending to any existing AslExecutorServiceTest) would keep the test suite consistent with the project convention.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@abanna

abanna commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Closing as a duplicate of #1662 (identical change — same States.JsonMerge implementation and AslExecutorJsonMergeTest). Two PRs for this fix were opened in parallel by mistake; consolidating on #1662.

@abanna abanna closed this Jun 30, 2026
@abanna
abanna deleted the feat/stepfunctions-json-merge branch June 30, 2026 14:38
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