Skip to content

Add system Nexus signal-with-start workflow API#744

Open
tconley1428 wants to merge 12 commits into
mainfrom
nexus-api-gen-signal-with-start
Open

Add system Nexus signal-with-start workflow API#744
tconley1428 wants to merge 12 commits into
mainfrom
nexus-api-gen-signal-with-start

Conversation

@tconley1428

Copy link
Copy Markdown
Contributor

Summary

  • generate system Nexus workflow APIs from temporary WIT during the SDK build
  • expose generated operations from the Workflow partial class without checking generated C# into source
  • handle system Nexus envelopes with binary protobuf outer payloads and generated nested payload traversal for codecs
  • add a focused workflow test for signal-with-start from workflow with a payload codec

Testing

  • dotnet build src/Temporalio/Temporalio.csproj -f netstandard2.0 -clp:ErrorsOnly -v:q
  • dotnet test tests/Temporalio.Tests/Temporalio.Tests.csproj --filter FullyQualifiedName~ExecuteWorkflowAsync_SignalWithStartFromWorkflow_Succeeds -clp:ErrorsOnly -v:n

Comment thread src/Temporalio.SystemNexus.Generator/wit/deps/nexus-temporal-types/model.wit Outdated
Comment thread src/Temporalio/SystemNexus/Generated/Service.cs Outdated
Comment thread src/Temporalio/Workflows/Generated/Models.cs
Comment thread src/Temporalio/SystemNexus/Generated/Operations.cs Outdated
Comment thread src/Temporalio.SystemNexus.Generator/Temporalio.SystemNexus.Generator.csproj Outdated
Comment thread src/Temporalio.SystemNexus.Generator/Temporalio.SystemNexus.Generator.csproj Outdated
Comment thread src/Temporalio.SystemNexus.Generator/Program.cs
Comment thread src/Temporalio.SystemNexus.Generator/Program.cs Outdated
});
}

static void GeneratePayloadVisitorRegistry(string operationsPath, string descriptorPath)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm slightly concerned about the fragility of this. It uses regex on the C# output from the nex-gen call, the message structure from the proto descriptors, and does some kind of fuzzy-ish join on the two. A couple of things:

  • Can we have nex-gen emit the operation to message mapping as some kind of structured data and read that instead?
  • Can this project be refactored a bit to be testable? It would be good to have this generator be testable and to add tests for the generated payload visitor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We talked about this. I think we're going to replace it with a more generic payload visitor soon as needed for external storage and that should replace this brittle generator.

@jmaeagle99 jmaeagle99 self-assigned this Jun 22, 2026
@tconley1428 tconley1428 marked this pull request as ready for review June 23, 2026 22:33
@tconley1428 tconley1428 requested a review from a team as a code owner June 23, 2026 22:33

@jmaeagle99 jmaeagle99 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would ask that the ExpressionUtil forward and marking public methods / types as experimental are done before merging. Adding GeneratedCode attribute would be nice, but can be deferred.

Comment thread src/Temporalio/Workflows/Generated/Operations.cs
Comment thread src/Temporalio/Workflows/Generated/Operations.cs
Comment thread src/Temporalio/Workflows/Generated/Models.cs
Comment thread src/Temporalio/Workflows/Generated/Operations.cs
Comment thread src/Temporalio/Worker/Generated/SystemNexusPayloadVisitor.cs
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.

2 participants