Preserve delayed PortableValue raw JSON on checkpoint re-serialization to avoid large-int/type-fidelity loss

[PratikDhanave (PratikDhanave)]

What

PortableValue.MarshalJSON called Any() before checking whether the value was still in its delayed json.RawMessage form. For a delayed value whose TypeID is unregistered (or whose content is any-typed/map-shaped), decodeKnownPortableType returns false and Any() falls back to a generic json.Unmarshal into any, yielding map[string]any + float64. That decoded value is never a json.RawMessage, so the existing raw-preserving branch was dead and MarshalJSON re-encoded lossily.

This makes a durable-JSON restore -> re-checkpoint cycle non-idempotent: an untouched delayed value is re-emitted with different bytes, and any int64/uint64 above 2^53 is truncated through float64.

The fix short-circuits on the retained json.RawMessage (when no deserialized cache exists) before calling Any(), re-emitting the original wire bytes verbatim.

Why (parity)

This matches .NET, which retains the incoming JsonElement and re-emits it verbatim on re-serialization rather than round-tripping through a decoded numeric/object representation. The Go delayed-deserialization path is meant to have the same "carry the bytes until someone reads them" semantics; before this change the marshal step broke that.

Tests

Added TestPortableValueMarshal_PreservesUntouchedDelayedRawJSON in workflow/portable_test.go: it builds wire JSON for a delayed value with an unregistered TypeID and a large integer (9007219254740993, > 2^53), unmarshals into a PortableValue (asserting Delayed()), re-marshals it without any typed access, and asserts the re-emitted Value bytes are byte-identical to the original. Pre-fix it truncates to 9007219254740992; post-fix it is byte-identical.

go build ./..., go vet ./workflow/..., and go test ./workflow/... all pass.

[Copilot]

Pull request overview

This PR fixes workflow.PortableValue JSON re-serialization for delayed values by preserving the original json.RawMessage bytes when the value was never read/deserialized, preventing precision/type-fidelity loss (notably for integers > 2^53) during a restore → re-checkpoint cycle.

Changes:

• Short-circuit PortableValue.MarshalJSON to re-emit the retained json.RawMessage verbatim when v.any is raw JSON and v.cache is still unset.
• Add a regression test covering an unregistered TypeID with a large integer to ensure re-marshaling preserves the original numeric bytes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
workflow/portable.go	Preserves untouched delayed raw JSON during MarshalJSON to avoid lossy generic decoding.
workflow/portable_test.go	Adds a regression test ensuring large-integer fidelity is preserved for delayed/unregistered values.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Parity Review: Approved

This PR fixes a bug in PortableValue.MarshalJSON where delayed (never-accessed) values were re-serialized lossily through Any() → json.Unmarshal → map[string]any/float64, instead of re-emitting the original wire bytes verbatim.

No new exported Go API surface is added. The fix adds a short-circuit in MarshalJSON to preserve the retained json.RawMessage when no deserialized cache exists.

Upstream parity mapping:

• .NET: JsonElement is retained and re-emitted verbatim on re-serialization — this fix aligns Go to that same "carry the bytes until someone reads them" contract.
• Python: Python checkpoint serialization uses native dict round-tripping without the same large-integer fidelity concern; no Python parity gap identified.

The restore → re-checkpoint idempotency fix is behavioral parity with .NET. The public-api-change label is not warranted as no exported API signatures changed. The existing parity-approved label is appropriate.

Generated by Go API Consistency Review Agent · sonnet46 · 16.6 AIC · ⌖ 4.07 AIC · ⊞ 6K · ◷