[Nexus-Chasm] Migrate OperationInvocationTaskExecutor by gow · Pull Request #9680 · temporalio/temporal

gow · 2026-03-26T06:39:17Z

What changed?

This PR migrates the Nexus operation invocation task handler from HSM version to Chasm.

Why?

Migrating from HSM to Chasm

How did you test it?

Note

Medium Risk
Introduces a new CHASM-based Nexus StartOperation execution path with endpoint lookup, callback URL/token generation, and error classification; mistakes could cause failed invocations, incorrect retries/timeouts, or misrouted callbacks. Risk is mitigated somewhat by strict task validation and added unit coverage, but the change touches critical workflow/history integration and outbound request handling.

Overview
Migrates Nexus operation invocation execution to CHASM by implementing OperationInvocationTaskHandler.Validate/Execute end-to-end, including endpoint resolution (ID with name fallback), callback URL selection (system vs templated), callback token generation, timeout budgeting, outbound StartOperation calls (HTTP or internal history service), metrics/logging, and classification of results into operation state transitions.

Adds supporting plumbing: OperationStore.NexusOperationInvocationData and workflow implementation that loads invocation input/headers from the scheduled history event, plus a new MSPointer.LoadHistoryEvent/NodeBackend.LoadHistoryEvent API. Configuration is extended to parse CallbackURLTemplate into a *template.Template, add UseSystemCallbackURL, and pass NumHistoryShards for internal routing; new helper utilities centralize callback building, error/failure conversion, and internal/HTTP start logic.

^{Written by Cursor Bugbot for commit 4b13977. This will update automatically on new commits. Configure here.}

chasm/lib/nexusoperation/task_handler_helpers.go

chasm/lib/nexusoperation/operation_tasks.go

bergundy

Submitting what I have so far since there's going to be another pass when you add the tests.

bergundy · 2026-03-30T21:15:36Z

chasm/lib/nexusoperation/config.go

 query complexity. Consider the cardinality impact when enabling these tags.`,
 )

+var UseSystemCallbackURL = dynamicconfig.NewGlobalBoolSetting(


We'll deprecate this soon enough. I'm kinda on the fence if we should even include it in the CHASM implementation since we only guarantee compatibility with the previous minor version and 1.30 supports system callback URLs already.

I'm inclined towards removing this since by the time chasm version makes it to production, we might not need it anymore.

bergundy · 2026-03-30T21:35:35Z

chasm/lib/nexusoperation/invocation_helpers.go

+type ClientProvider func(ctx context.Context, namespaceID string, entry *persistencespb.NexusEndpointEntry, service string) (*nexusrpc.HTTPClient, error)
+
+// OperationInvocationTaskHandlerOptions is the fx parameter object for the invocation task executor.
+type OperationInvocationTaskHandlerOptions struct {


This should go next to the handler definition IMHO, it "belongs" to that struct.

bergundy · 2026-03-30T21:37:08Z

chasm/lib/nexusoperation/invocation_helpers.go

+}
+
+// loadStartArgs is a ReadComponent callback that loads the start arguments from the operation.
+func (o *Operation) loadStartArgs(


Put struct methods in the file the struct is defined in please to keep the code organized.

Done. Moved these methods to operation.go next to the the operation struct.

bergundy · 2026-03-30T21:38:27Z

chasm/lib/nexusoperation/invocation_helpers.go

+	ctx chasm.Context,
+	_ chasm.NoValue,
+) (startArgs, error) {
+	invocationData, err := o.GetInvocationData(ctx)


This is unnecessary. The operation can just call its parent here or get whatever is embedded in the component for standalone. You may need to add a TODO for the standalone path to implement what's needed here. CC @stephanos.

I had the TODO in that method. Removed the method and kept everything inline.

bergundy · 2026-03-30T21:38:53Z

chasm/lib/nexusoperation/invocation_helpers.go

+}
+
+// saveResult is an UpdateComponent callback that saves the invocation outcome.
+func (o *Operation) saveResult(


Same here, it should be in operation.go.

bergundy · 2026-03-30T22:10:17Z

chasm/lib/nexusoperation/operation.go

 }

+// GetInvocationData loads invocation data from the store or returns an error if no store is present.
+func (o *Operation) GetInvocationData(ctx chasm.Context) (InvocationData, error) {


I don't think you need this function, and please don't use the Get prefix for getters in Go.

I added this method as a placeholder so that @stephanos can implement loading the data from operation state.
Renamed the method to InvocationData()

bergundy · 2026-03-30T22:11:00Z

chasm/lib/nexusoperation/operation.go

+}
+
 // OnStarted applies the started transition or delegates to the store if one is present.
 func (o *Operation) OnStarted(ctx chasm.MutableContext, _ *Operation, operationToken string, links []*commonpb.Link) error {


As I mentioned here and in another review where my comment was lost. There's no need to take Operation as an argument on methods of Operation.

bergundy · 2026-03-30T22:14:23Z

chasm/lib/nexusoperation/operation_tasks.go

+		config:                 opts.Config,
+		namespaceRegistry:      opts.NamespaceRegistry,
+		metricsHandler:         opts.MetricsHandler,
+		logger:                 opts.Logger,
+		callbackTokenGenerator: opts.CallbackTokenGenerator,
+		clientProvider:         opts.ClientProvider,
+		endpointRegistry:       opts.EndpointRegistry,
+		httpTraceProvider:      opts.HTTPTraceProvider,
+		historyClient:          opts.HistoryClient,
+		chasmRegistry:          opts.ChasmRegistry,


Alternatively you can just store opts on the struct. You can't embed options in the handler because it has an fx.In embedded in it already and AFAIR that causes issues.

I think it's better to copy just the necessary dependencies than to save the whole opts object. Will leave this as is for now.

bergundy · 2026-03-30T22:24:19Z

chasm/lib/nexusoperation/operation_tasks.go

+
+	// Skip endpoint lookup for system-internal operations.
+	if args.endpointName != commonnexus.SystemEndpoint {
+		if args.endpointID == "" {


I see a bunch of comments that were not transferred to this function's implementation too, specifically the one we had here.

Copied over the comments (from HSM version) that looked important.

bergundy · 2026-03-30T22:25:59Z

chasm/lib/nexusoperation/operation_tasks.go

+
+	callbackURL, err := buildCallbackURL(h.config.UseSystemCallbackURL(), h.config.CallbackURLTemplate(), ns, endpoint)
+	if err != nil {
+		return serviceerror.NewFailedPreconditionf("failed to build callback URL: %v", err)


This doesn't need to be a failed precondition but it doesn't hurt apart from losing the wrapping of the original error with %w.

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

cursor · 2026-04-01T05:00:01Z

chasm/lib/nexusoperation/operation_tasks.go

+	}
+	if op.GetAttempt() != task.GetAttempt() {
+		return false, serviceerror.NewFailedPreconditionf("task attempt %d does not match operation attempt %d", task.GetAttempt(), op.GetAttempt())
+	}


Validate returns error for stale tasks instead of dropping

Medium Severity

Validate returns (false, error) when the operation is no longer in the scheduled state or the attempt doesn't match. Per the TaskValidator contract, (false, nil) means silently drop an obsolete task, while (anything, error) means validation failed. Stale invocation tasks (e.g., operation already started/completed, or attempt advanced) are expected and should be silently dropped, not treated as validation failures. The other task handlers (backoff, timeout) correctly return (bool, nil).

This PR migrates the Nexus operation invocation task handler from HSM version to Chasm. Migrating from HSM to Chasm - [x] built - [ ] run locally and tested manually - [ ] covered by existing tests - [x] added new unit test(s) - [ ] added new functional test(s)  --- > [!NOTE] > **Medium Risk** > Introduces a new CHASM-based Nexus `StartOperation` execution path with endpoint lookup, callback URL/token generation, and error classification; mistakes could cause failed invocations, incorrect retries/timeouts, or misrouted callbacks. Risk is mitigated somewhat by strict task validation and added unit coverage, but the change touches critical workflow/history integration and outbound request handling. > > **Overview** > Migrates Nexus operation invocation execution to CHASM by implementing `OperationInvocationTaskHandler.Validate/Execute` end-to-end, including endpoint resolution (ID with name fallback), callback URL selection (system vs templated), callback token generation, timeout budgeting, outbound StartOperation calls (HTTP or internal history service), metrics/logging, and classification of results into operation state transitions. > > Adds supporting plumbing: `OperationStore.NexusOperationInvocationData` and workflow implementation that loads invocation input/headers from the scheduled history event, plus a new `MSPointer.LoadHistoryEvent`/`NodeBackend.LoadHistoryEvent` API. Configuration is extended to parse `CallbackURLTemplate` into a `*template.Template`, add `UseSystemCallbackURL`, and pass `NumHistoryShards` for internal routing; new helper utilities centralize callback building, error/failure conversion, and internal/HTTP start logic. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 4b13977. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>

gow requested review from a team as code owners March 26, 2026 06:39

gow marked this pull request as draft March 26, 2026 06:39

cursor bot reviewed Mar 26, 2026

View reviewed changes

chasm/lib/nexusoperation/task_handler_helpers.go Show resolved Hide resolved

gow force-pushed the cg/nexus/task_executors_2 branch from 78f3c98 to 761370c Compare March 27, 2026 16:49

gow force-pushed the cg/nexus/task_executors_1 branch 2 times, most recently from 350d41a to b29957d Compare March 30, 2026 04:43

gow force-pushed the cg/nexus/task_executors_2 branch from 761370c to c0a5d28 Compare March 30, 2026 04:44

Base automatically changed from cg/nexus/task_executors_1 to nexus/hsm-to-chasm-migration March 30, 2026 05:26

gow force-pushed the cg/nexus/task_executors_2 branch 3 times, most recently from 7492591 to c480b70 Compare March 30, 2026 06:47

gow requested review from bergundy and stephanos March 30, 2026 07:12

gow marked this pull request as ready for review March 30, 2026 07:12

cursor bot reviewed Mar 30, 2026

View reviewed changes

chasm/lib/nexusoperation/operation_tasks.go Show resolved Hide resolved

chasm/lib/nexusoperation/operation_tasks.go Show resolved Hide resolved

gow requested a review from Quinn-With-Two-Ns March 30, 2026 17:31

bergundy reviewed Mar 31, 2026

View reviewed changes

cursor bot reviewed Apr 1, 2026

View reviewed changes

gow force-pushed the nexus/hsm-to-chasm-migration branch from d07f7e9 to 48c21b2 Compare April 1, 2026 06:18

gow force-pushed the cg/nexus/task_executors_2 branch from 35b4a1f to 312e79a Compare April 1, 2026 06:30

gow requested review from S15 and bergundy April 1, 2026 16:53

gow force-pushed the nexus/hsm-to-chasm-migration branch from 48c21b2 to 5c6bbd9 Compare April 3, 2026 16:58

gow added 3 commits April 3, 2026 09:58

[Nexus-Chasm] Migrate OperationInvocationTaskExecutor

df9b8ab

Fix lint errors

a58132b

Get rid of time.Now() calls

2c841f8

gow added 6 commits April 3, 2026 09:58

Addressing feedback

e1c7f9b

Use dynamicconfig.TypedPropertyFn

48519ac

Address review feedback-2

3df249f

Add unit tests for validation

f6e2c59

Fix lint errors

e3ac28f

NexusOperationInvocationData()

4b13977

gow force-pushed the cg/nexus/task_executors_2 branch from 312e79a to 4b13977 Compare April 3, 2026 16:59

bergundy merged commit 291cb26 into nexus/hsm-to-chasm-migration Apr 6, 2026
47 checks passed

bergundy deleted the cg/nexus/task_executors_2 branch April 6, 2026 17:09

Conversation

gow commented Mar 26, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed?

Why?

How did you test it?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bergundy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Apr 1, 2026

Choose a reason for hiding this comment

Validate returns error for stale tasks instead of dropping

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gow commented Mar 26, 2026 •

edited by cursor bot

Loading