Nexus chasm cleanup by bergundy · Pull Request #9843 · temporalio/temporal

bergundy · 2026-04-07T18:35:06Z

What changed?

Why?

Cleanup
Improved test coverage
Bug fixes

- Introduce RetryPolicyConfig struct and typed DC setting (replaces separate initial/max interval settings) - Rename MaxConcurrentOperations to MaxConcurrentOperationsPerWorkflow with updated DC key - Fix CallbackURLTemplate converter to use direct type assertion - Add PrincipalType/PrincipalName to disallowed operation headers

Proto schema: - Add started_time and closed_time to OperationState - Remove attempt from timeout task messages (only invocation/backoff tasks track attempt) - Reorder task messages (timeouts first, then invocation) State machine: - Increment attempt on schedule/reschedule (before task emission) rather than after attempt completion - makes attempt number approximate but available in the task - Track StartedTime and ClosedTime in transitions - Support explicit StartTime/CompleteTime in events for async completion race conditions - Propagate Failure through Failed/Canceled transitions - Add resolveUnsuccessfully() for terminal state bookkeeping - Clear NextAttemptScheduleTime in all terminal transitions - Base start-to-close timeout on actual start time, not schedule time - Move RequestedTime assignment to Operation.Cancel() Task handlers: - Privatize all task handler types, options, and constructors - Inject task handlers into Library via constructor - Extract invocation interface with HTTP, system, and timeout implementations - Unify error classification in newInvocationResult() - Simplify Validate to return (bool, nil) for invalid states - Fix OnCancelled -> OnCanceled spelling - Fix fx module name

Workflow registry: - Add Library interface with CommandHandlers() and EventDefinitions() - Replace individual Register* methods with single Register(Library) - Add EventDefinitionByGoType[D] for type-safe event lookup - Add generic AddAndApplyHistoryEvent[D] free function - Rename EventDefinition() to EventDefinitionByEventType() - Privatize workflow Library struct, expose NewLibrary constructor - Use serviceerror.NewInternalf for internal errors Nexus workflow: - Add library type implementing workflow Library interface - Replace registerCommandHandlers/registerEvents with Library methods - Use type-safe AddAndApplyHistoryEvent[D] in command handlers - Consolidate two fx.Invoke calls into one - Set initial Attempt to 0 (TransitionScheduled now increments) - Rename chasmCtx to ctx - Fix OnNexusOperationCancelled -> OnNexusOperationCanceled spelling

bergundy · 2026-04-07T18:44:48Z

chasm/lib/nexusoperation/library.go


-func newLibrary() *Library {
-	return &Library{}
+func newLibrary(


@stephanos I left it up to you to break out the components only library.

bergundy · 2026-04-07T18:54:18Z

chasm/lib/nexusoperation/operation_statemachine.go

@stephanos there are some key behavior changes here: attempt is now increments on scheduled, start and close time can be provided externally (needed for async completion).

stephanos · 2026-04-07T21:23:19Z

chasm/lib/nexusoperation/config.go


-var MaxConcurrentOperations = dynamicconfig.NewNamespaceIntSetting(
-	"nexusoperation.limit.operation.concurrency",
+var MaxConcurrentOperationsPerWorkflow = dynamicconfig.NewNamespaceIntSetting(


Do we have/need a limit for standalone?

No, this is just pending nexus operations in a single workflow.

stephanos · 2026-04-07T21:33:39Z

chasm/lib/nexusoperation/config.go

 		RequestTimeout:                     RequestTimeout.Get(dc),
 		MinRequestTimeout:                  MinRequestTimeout.Get(dc),
-		MaxConcurrentOperations:            MaxConcurrentOperations.Get(dc),
+		MaxConcurrentOperations:            MaxConcurrentOperationsPerWorkflow.Get(dc),


Should we rename the field, too?

stephanos · 2026-04-07T22:38:38Z

chasm/lib/workflow/workflow.go

-func (w *Workflow) AddAndApplyHistoryEvent(
+// AddAndApplyHistoryEvent adds a history event to the workflow and applies the corresponding event definition,
+// looked up by Go type. This is the preferred way to add and apply events as it provides go-to-definition navigation.
+func AddAndApplyHistoryEvent[D EventDefinition](


I like that a lot.

Yeah, but we should consider moving all of the workflow events into chasm/lib/workflow so we can actually refer to them, otherwise we run into circular dependency issues that are annoying to break.

stephanos · 2026-04-07T23:06:33Z

chasm/lib/nexusoperation/operation_statemachine.go

+		// Clear the next attempt schedule time when leaving BACKING_OFF state. This field is only valid in
+		// BACKING_OFF state.
+		o.NextAttemptScheduleTime = nil
+		o.ClosedTime = timestamppb.New(ctx.Now(o))


Should this use CompleteTime if non-nil?

stephanos · 2026-04-07T23:12:42Z

chasm/lib/nexusoperation/proto/v1/operation.proto

  // The number of attempts made to deliver the start operation request.
-  // This number represents a minimum bound since the attempt is incremented after the request completes.
-  int32 attempt = 12;
+  // This number is approximate, it is incremeted when a task is added to the history queue.


Suggested change

// This number is approximate, it is incremeted when a task is added to the history queue.

// This number is approximate, it is incremented when a task is added to the history queue.

stephanos · 2026-04-07T23:34:03Z

chasm/lib/nexusoperation/task_handler_helpers.go

+	// Special marker for Temporal->Temporal calls to indicate that the original failure should be unwrapped.
+	// Temporal uses a wrapper operation error with no additional information to transmit the OperationError over the network.
+	// The meaningful information is in the operation error's cause.
+	unwrapError := opErr.OriginalFailure.Metadata["unwrap-error"] == "true"


is OriginalFailure always set? In the branch before this is called I see if handlerErr.OriginalFailure != nil

Hmm.. it would only be nil if it is constructed in process, and we only do that for HandlerError instances. But good callout, I'll add that here.

stephanos · 2026-04-07T23:37:55Z

chasm/lib/nexusoperation/task_handler_helpers.go

+		}
+		return invocationResultFail{failure: failure}, nil
+	}
+	if opTimeoutBelowMinErr, ok := errors.AsType[*operationTimeoutBelowMinError](callErr); ok {


nit^2: is the missing newline above this if block intentional? It signals to me that these two belong together more than the other ones but I don't quite see why they do.

I think it was because both of these are cases for errors from the nexus SDK but it's fine either way.

stephanos · 2026-04-07T23:38:29Z

chasm/lib/nexusoperation/task_handler_helpers.go

+	}
+
+	if errors.Is(callErr, context.DeadlineExceeded) || errors.Is(callErr, context.Canceled) {
+		// If timed out, don't leak internal info to the user


Suggested change

// If timed out, don't leak internal info to the user

// If timed out, don't leak internal info to the user.

stephanos · 2026-04-07T23:44:46Z

chasm/lib/nexusoperation/task_handler_helpers.go

-	if errors.Is(callErr, ErrResponseBodyTooLarge) || errors.Is(callErr, ErrInvalidOperationToken) {
-		return nonRetryableFailResult(callErr)
-	}


If I'm not mistaken these two became invocationResultRetry now instead of being non-retryable?

stephanos · 2026-04-07T23:47:13Z

chasm/lib/nexusoperation/operation.go

-// saveResult is an UpdateComponent callback that saves the invocation outcome.
-func (o *Operation) saveResult(
+// saveResultInput is the input to the Operation.saveResult method used in UpdateComponent.
+type saveResultInput struct {


nit: should this be saveInvocationResultInput? Granted, a bit long but would match the other types and methods.

## What changed? - [[Nexus-Chasm] Improve nexus operation dynamic config](59a7068) - [[Nexus-Chasm] Improve operation state machine and task handlers](c6f1fa8) - [[Nexus-Chasm] Refactor workflow registry and nexus workflow integration](45d0cfc) ## Why? - Cleanup - Improved test coverage - Bug fixes

bergundy added 3 commits April 7, 2026 11:23

bergundy requested review from a team as code owners April 7, 2026 18:35

bergundy commented Apr 7, 2026

View reviewed changes

stephanos self-requested a review April 7, 2026 20:30

stephanos reviewed Apr 7, 2026

View reviewed changes

bergundy added 2 commits April 7, 2026 17:25

Fix CI

6cab7fb

Address review comments

151c5c8

bergundy merged commit 6421ece into temporalio:nexus/hsm-to-chasm-migration Apr 8, 2026
45 checks passed

bergundy deleted the nexus-chasm-cleanup branch April 8, 2026 04:16

	// This number is approximate, it is incremeted when a task is added to the history queue.
	// This number is approximate, it is incremented when a task is added to the history queue.

	// If timed out, don't leak internal info to the user
	// If timed out, don't leak internal info to the user.

Conversation

bergundy commented Apr 7, 2026

What changed?

Why?

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants