Repository navigation
Conversation
dplyukhin
marked this pull request as ready for review
September 29, 2026 17:31
Generated Go code converted operation models to protos eagerly, calling toProto(ctx) at the call site before handing the result to ExecuteOperation. That made Go the odd one out: Python, .NET, and TypeScript all perform model<->proto conversion inside the payload converter, so the SDK's chosen converter also encodes the model's inner user payloads (Args, SignalArgs, Memo, UserMetadata). Emit workflow.TransferTypeConverter for top-level operation inputs and outputs instead, and pass the model itself to ExecuteOperation. The SDK now drives conversion, so inner payloads are encoded by the converter the SDK selected for the operation. This is a prerequisite for supporting @nexus.serialization-context in Go. @nexus.source fields move onto the model as unexported fields, populated in the generated operation function where a workflow.Context is in scope, and read back in FromProto so sourced fields round-trip. Behaviour changes: - A proto-backed operation model can no longer be a workflow's top-level return value or an activity argument. Converting one outside a workflow fails with "can only be converted inside a workflow", matching Python's equivalent RuntimeError. - Nexus conversion failures now surface on the returned future rather than synchronously. - An @nexus.omit'ed field can no longer feed a resource-return constructor argument; this is now a clear UnsupportedGoProtoConversion error rather than silently broken code. Converters are emitted only for top-level operation inputs and outputs, not for every proto-backed model. Override-converter types and resource-return/output-transform outputs keep the eager proto path, because nexgen does not own those types or the planner emits no usable model for them. Depends on temporalio/sdk-go#2703; advanced/samples/go/go.mod pins a pseudo-version of that branch.
dplyukhin
force-pushed
the
go-transfer-types
branch
from
September 29, 2026 17:31
377394e to
7e8f10a
Compare
dplyukhin
added this pull request to stack #207
September 30, 2026 14:42
This branch has not been deployed
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
1 participant
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.
Warning
Do not merge before temporalio/sdk-go#2703.
Before this PR, Go converted operation models to protos by calling
toProto(ctx)before handing the result toExecuteOperation. That isn't consistent with Python, .NET, and Typescript.The PR attaches a transfer type converter to the model type, so that inner user payloads (Args, SignalArgs, Memo, UserMetadata) are encoded during the SDK’s payload-conversion call. We need this for
@nexus.serialization-context.Behaviour changes
activity argument. Converting one outside a workflow fails with
can only be converted inside a workflow, matching Python's equivalentRuntimeError.@nexus.omited field can no longer feed a resource-return constructor argument; this isnow a
UnsupportedGoProtoConversionerror.Latest Go SDK dependency
Updated to temporalio/sdk-go#2703 head
1e5a27aa2a2df781a2b0b119e03749ff66443161(v1.49.1-0.20261005152832-1e5a27aa2a2d). This is a dependency update; the nexgen PR still targetsmain.workflow.NewTransferTypeConverterwith four callbacks instead of the removed six-callbackNewContextAwareTransferTypeConverter.TransferTypeConvertermethod.Capability audit
No required capability was removed. Workflow-context callbacks and serialization-context propagation remain available. The integration tests confirm that nested user payloads are converted before the proto envelope, the envelope gets the Nexus serialization context, and nested payloads retain the calling workflow's serialization context. Redirecting nested payloads to the target workflow remains future
@nexus.serialization-contextwork, not a feature added by this PR.The SDK now explicitly limits transfer conversion to top-level values, non-pointer model/transfer type arguments, and decoding into
*Trather than**T. Generated operation models already use the supported value-type contract. The removed context-free callbacks do not reduce our functionality: they previously returned the same workflow-context-required error as the ordinarycontext.Contextcallbacks.Validation
cargo validatepassed for Rust, Python, TypeScript, Go, Java, and .NET (using the installed .NET SDK onPATH).cargo test --all-features --test generate_go: 59 passed.go test ./...passed against the pinned SDK head.