[Nexus] Support internal callback completion routing to CHASM by lina-temporal · Pull Request #8372 · temporalio/temporal

lina-temporal · 2025-09-23T22:23:44Z

What changed?

The Callbacks component now detects Nexus URLs of the form temporal://internal/chasm, which are routed to CHASM components through a new ChasmInvocation.
- CHASM routing information is serialized into a header, the URL is left opaque.
A new CHASM interface has been added, chasm.NexusCompletionHandler, which components can implement to receive Nexus callbacks.
A CHASM helper method, GetNexusCallback, has been added that returns a Callback protobuf routing to the given component.
The frontend's callback argument validator has been patched to allow "temporal" schemes unconditionally. Scheduler starts workflows through the frontend, in order to be counted by metering, and so frontend must allow requests with internal-facing callbacks.
- I believe @chaptersix 's changes also touch this area, we may not need my interim patch there.

Why?

CHASM Scheduler needs to find out about workflow completions, and we want to do that with Nexus completion callbacks.

How did you test it?

Potential risks

The funkiness is in the frontend handler - see the TODO. I don't think it presents a security risk, since a customer would only be able to forge completions within their own namespace (since this validation takes place within StartWorkflowExecution's frontend flow). But it would be nice to validate that it was only coming from internal pods.

…erated from History

bergundy · 2025-09-25T00:22:50Z

+	NexusComponentRefHeader = "X-CHASM-Component-Ref"
+
+	// Base URL for Nexus->CHASM callbacks.
+	NexusCompletionHandlerURL = "temporal://internal/chasm"


Suggested change

NexusCompletionHandlerURL = "temporal://internal/chasm"

NexusCompletionHandlerURL = "temporal://internal"

bergundy · 2025-09-25T00:23:19Z

+
+const (
+	// Header name for the CHASM ComponentRef.
+	NexusComponentRefHeader = "X-CHASM-Component-Ref"


No need, just put this in the callback token header.

When I synced up with @chaptersix , we decided to use SystemCallbackURL so it goes through the same handler as other internal callbacks, which I think makes sense. However, if we don't have a unique URL nor a unique CHASM header, how would we distinguish this as a CHASM request in the callback executor?

We are thinking of deprecating the callback headers for the most part, which is why I would ask that you use a single token field.

Moved to the commonnexus.CallbackTokenHeader field. Relies on temporal://internal being unique to CHASM.

bergundy · 2025-09-25T00:27:30Z

-			if err != nil {
-				return err
-			}
 		case *persistencespb.Callback_Hsm:


Can just remove this branch and the rest of the code it references.

Sure, I'll open a separate PR to remove the Callback_Hsm invocation stuff.

bergundy · 2025-09-25T00:31:17Z

+				chasmInvokable := chasmInvocation{}
+				chasmInvokable.nexus = variant.Nexus
+				chasmInvokable.attempt = callback.Attempt
+				invokable = chasmInvokable


You're going to want to set the completion on here too...

Would you also please just set all of the fields here inline with the struct definition?
Could just do:

invokable = chasmInvocation{ nexus: variant.Nexus, ... }

Oops, yeah, cleaned these up.

bergundy · 2025-09-25T00:31:45Z

+			}
+
 			// variant struct is immutable and ok to reference without copying
 			nexusInvokable := nexusInvocation{}


Same here, would be nice to just inline the whole field assignment (unrelated to your change).

bergundy · 2025-09-25T00:34:29Z

+	// this similarly as we would a pure task (holding an exclusive lock), as the
+	// assumption is that the accessed component will be recording (or generating a
+	// task) based on this result.
+	_, err = e.ChasmEngine.UpdateComponent(ctx, ref, func(ctx chasm.MutableContext, component chasm.Component) error {


This doesn't do an RPC, you're going to have to add an RPC to the shard that owns the execution of the original caller.
We can then optimize it so if the ref is something that we can resolve locally we would bypass the RPC call.

The RPC would be historyservice.CompleteNexusOperation, you'll want to update that method to invoke ChasmEngine.UpdateComponent and take a different form of completion.

Feel free to rename the completion field to hsm_completion to disambiguate between HSM and CHASM refs.

Let's also not change the Nexus RPC stuff for now since we are not using that SDK for delivering the completions.

We can change it later when the SDK supports pluggable transports if we want to but that's probably not critical.

bergundy · 2025-09-25T00:37:33Z

 	}
+	// Internally routed.
+	// TODO - validate requests involving this URL come from a history pod.
+	if u.Scheme == "temporal" {


@chaptersix will take care of this, he's modifying this code right now.

We need to validate that the caller is from within the cluster if it tries to attach a callback with an internal URL.

You need to merge your code with main (or rebase if you prefer :))

I'll do that now that the stacked PRs are merged.

lina-temporal · 2025-10-01T02:15:28Z

+	// Base URL for Nexus->CHASM callbacks.
+	NexusCompletionHandlerURL = "temporal://system/nexus/callback/chasm"


I'm still planning to get rid of this and use the constant SystemCallbackURL, but this branch is stacked on top of others, so I'll do that after others have merged in.

I would consider using temporal://internal to avoid having to unpack the callback tokens.

A good reason to use temporal://internal callbacks is because they will trip a separate circuit breaker in the outbound queue.
We should also consider running these callbacks on the immediate "transfer" queue instead and avoid using circuit breakers altogether but let's add an issue for this, we are going to refactor callbacks to use CHASM soon and can do this work as part of that.

ghost · 2025-10-01T17:05:29Z

+
+const (
+	// Header name for the CHASM ComponentRef.
+	NexusComponentRefHeader = "X-CHASM-Component-Ref"


We are thinking of deprecating the callback headers for the most part, which is why I would ask that you use a single token field.

ghost · 2025-10-01T17:06:24Z

+	// Base URL for Nexus->CHASM callbacks.
+	NexusCompletionHandlerURL = "temporal://system/nexus/callback/chasm"


I would consider using temporal://internal to avoid having to unpack the callback tokens.

ghost · 2025-10-01T17:15:43Z

+func (c chasmInvocation) Invoke(ctx context.Context, ns *namespace.Namespace, e taskExecutor, task InvocationTask) invocationResult {
+	// Get back the base64-encoded ComponentRef from the header.
+	encodedRef, ok := c.nexus.GetHeader()[chasm.NexusComponentRefHeader]
+	if !ok {
+		return invocationResultFail{errors.New("callback missing CHASM header")}
+	}
+
+	decodedRef, err := base64.RawURLEncoding.DecodeString(encodedRef)
+	if err != nil {
+		return invocationResultFail{fmt.Errorf("failed to decode CHASM ComponentRef: %v", err)}
+	}
+
+	ref := &persistencespb.ChasmComponentRef{}
+	err = proto.Unmarshal(decodedRef, ref)
+	if err != nil {
+		return invocationResultFail{fmt.Errorf("failed to unmarshal CHASM ComponentRef: %v", err)}
+	}
+
+	request, err := c.getHistoryRequest(ref)
+	if err != nil {
+		return invocationResultFail{fmt.Errorf("failed to build history request: %v", err)}
+	}
+
+	// RPC to History for cross-shard completion delivery.
+	_, err = e.HistoryClient.CompleteNexusOperationChasm(ctx, request)
+	if err != nil {
+		return invocationResultRetry{fmt.Errorf("failed to complete Nexus operation: %v", err)}
+	}
+
+	return invocationResultOK{}
+}


Are these internal errors ever exposed to users or is this all internal?

This is internal-facing, for operators who are log-diving.

ghost · 2025-10-01T17:19:24Z

+
+func (c chasmInvocation) WrapError(result invocationResult, err error) error {
+	if failure, ok := result.(invocationResultFail); ok {
+		return queues.NewUnprocessableTaskError(failure.err.Error())


This will prevent the task from being retried, is that what you are trying to do here? Doesn't seem like it...

Yes, that's what I'm trying to do; the errors I've marked as 'fail' explicitly are non-retryable (e.g., fails validation checks).

I see. That's fine. I think you just don't want to wrap in a destination down error though to avoid triggering the circuit breaker.

Updated to not wrap in DestinationDown.

ghost · 2025-10-01T17:25:45Z

+			return nil, fmt.Errorf("failed to convert Nexus links: %v", err)
+		}
+
+		payload, err := io.ReadAll(op.Reader)


Use proto.Unmarshal into common.Payload message here. And consider that the payload may be nil.

ghost · 2025-10-01T21:55:43Z

+// ChasmNexusCompletion includes details about a completed Nexus operation.
+message ChasmNexusCompletionInfo {
+    // Operation state - may only be successful / failed / canceled.
+    string state = 1;


As mentioned the oneof is enough.

ghost · 2025-10-01T21:56:01Z

 }
+
+// ChasmNexusCompletion includes details about a completed Nexus operation.
+message ChasmNexusCompletionInfo {


Info is redundant IMHO.

It's to disambiguate against the token, ChasmNexusCompletion; I could suffix that instead, maybe?

Suffix how?

Renamed to ChasmNexusCompletion.

ghost · 2025-10-01T23:09:21Z

 	}
+	// Internally routed.
+	// TODO - validate requests involving this URL come from a history pod.
+	if u.Scheme == "temporal" {


You need to merge your code with main (or rebase if you prefer :))

ghost · 2025-10-01T23:11:20Z

+	_, err := h.chasmEngine.UpdateComponent(ctx, ref, func(ctx chasm.MutableContext, component chasm.Component) error {
+		handler, ok := component.(chasm.NexusCompletionHandler)
+		if !ok {
+			return serviceerror.NewUnimplementedf("component '%T' does not implement NexusCompletionHandler", component)


Will the caller consider this a retryable error? We should verify that it doesn't because the callback executor will just spin indefinitely.

Good point, yeah the ChasmInvocation shouldn't be retrying on this.

ghost · 2025-10-01T23:13:43Z

+	// Base URL for Nexus->CHASM callbacks.
+	NexusCompletionHandlerURL = "temporal://system/nexus/callback/chasm"


A good reason to use temporal://internal callbacks is because they will trip a separate circuit breaker in the outbound queue.
We should also consider running these callbacks on the immediate "transfer" queue instead and avoid using circuit breakers altogether but let's add an issue for this, we are going to refactor callbacks to use CHASM soon and can do this work as part of that.

ghost

Didn't have a lot left to add. This is very close to ready.

ghost · 2025-10-06T23:19:32Z

+) (*historyservice.CompleteNexusOperationChasmRequest, error) {
+	var req *historyservice.CompleteNexusOperationChasmRequest
+
+	token := &tokenspb.NexusOperationChasmCompletion{


nit: call this variable completion?

ghost · 2025-10-06T23:20:57Z

+		req = &historyservice.CompleteNexusOperationChasmRequest{
+			Completion: token,
+			Outcome: &historyservice.CompleteNexusOperationChasmRequest_Failure{
+				Failure: apiFailure,
+			},
+			CloseTime: timestamppb.New(op.CloseTime),


nit: you could just set the outcome here and initialize the request outside of the switch statement.

I thought so too, but CloseTime is independent on both CompletionSuccess/CompletionFailure variants, so you have to type assert for that as well anyways.

ghost · 2025-10-06T23:24:27Z

    // Complete an async Nexus Operation using a completion token. The completion state could be successful, failed, or
    // canceled.
+    //
+    // Deprecated. Will be renamed to CompleteNexusOperationHsm in a future release.


Not to start using it, but have it defined already. You need two versions to safely point the client to use the new API.

ghost · 2025-10-06T23:25:38Z

 }
+
+// ChasmNexusCompletion includes details about a completed Nexus operation.
+message ChasmNexusCompletionInfo {


Suffix how?

ghost · 2025-10-06T23:27:41Z

 }
+
+// A completion token for a Nexus operation started from a CHASM component.
+message NexusOperationChasmCompletion {


Hmm... can we merge this into the existing NexusOperationCompletion and deprecate the original fields?

You can just add a component_ref field and it would work well.

Not critical but it would simplify the decoding later.

Sure, moved to that.

ghost · 2025-10-06T23:31:18Z

+	// this similarly as we would a pure task (holding an exclusive lock), as the
+	// assumption is that the accessed component will be recording (or generating a
+	// task) based on this result.
+	_, err := h.chasmEngine.UpdateComponent(ctx, ref, func(ctx chasm.MutableContext, component chasm.Component) error {


Use chasm.UpdateComponent and avoid casting. If you merge the current main changes, you'll see that the chasm engine should already be on the request context.

Use chasm.UpdateComponent and avoid casting

I don't think I can do that here in a way that keeps this generic. I'm using the NexusCompletionHandler interface which doesn't bundle in Component, therefore, if I try to accept a NexusCompletionHandler here, it won't meet the generic requirements of implementing Component's LifecycleState.

That sounds like a bug in CHASM. You shouldn't need to inspect the type. There's no reason that chasmEngine.UpdateComponent would work and chasm.UpdateComponent(ctx) would not work.

What component type would you expect I use here? NexusCompletionHandler won't work since it isn't a Component.

That's what I expect, that you can use the interface instead of a concrete type.

ghost

This is pretty close to done AFAIC.

ghost · 2025-10-08T22:30:12Z

+	}
+
+	if retry, ok := result.(invocationResultRetry); ok {
+		return queues.NewDestinationDownError(retry.err.Error(), err)


Suggested change

return queues.NewDestinationDownError(retry.err.Error(), err)

return err

ghost · 2025-10-08T22:56:45Z

+	return err
+}
+
+func (c chasmInvocation) Invoke(ctx context.Context, ns *namespace.Namespace, e taskExecutor, task InvocationTask) invocationResult {


Thinking about this some more, we shouldn't expose these errors. They are user visible. I suggest that instead we follow this pattern.

referenceID := uuid.NewString() e.Logger.Error("failed to decode CHASM ComponentRef", tag.Error(err), tag.NewStringTag("reference-id", referendID)) return invocationResultFail{fmt.Errorf("internal error, reference-id: %v", referenceID)}

Same for invocationResultRetry

ghost · 2025-10-08T23:03:01Z

+		{
+			name: "success-with-successful-operation",
+			setupHistoryClient: func(t *testing.T, ctrl *gomock.Controller) *historyservicemock.MockHistoryServiceClient {
+				client := historyservicemock.NewMockHistoryServiceClient(ctrl)


Unrelated to this PR but I couldn't help myself. Really the whole mock and codegen here is redundant, this is so much simpler:

type MockHistoryClient struct { historyservice.HistoryServiceClient HandleCompleteNexusOperationChasm func(ctx context.Context, req *historyservice.CompleteNexusOperationChasmRequest, opts ...grpc.CallOption) (*historyservice.CompleteNexusOperationChasmResponse, error) }

ghost · 2025-10-08T23:15:14Z

+	requestID  string
+}
+
+var ErrUnimplementedHandler = serviceerror.NewUnimplemented("component does not implement NexusCompletionHandler")


Is this used?

Nope, got rid of it.

ghost · 2025-10-08T23:17:01Z

+// ChasmNexusCompletion includes details about a completed Nexus operation.
+message ChasmNexusCompletion {


Suggested change

// ChasmNexusCompletion includes details about a completed Nexus operation.

message ChasmNexusCompletion {

// NexusCompletion includes details about a completed Nexus operation.

message NexusCompletion {

Every other type in this protobuf is prefixed with Chasm, I'd rather keep that consistent or change all at once.

It's not CHASM specific, you can move this somewhere else but not blocking the PR.

ghost · 2025-10-08T23:20:28Z

+	// this similarly as we would a pure task (holding an exclusive lock), as the
+	// assumption is that the accessed component will be recording (or generating a
+	// task) based on this result.
+	_, err := h.chasmEngine.UpdateComponent(ctx, ref, func(ctx chasm.MutableContext, component chasm.Component) error {


That sounds like a bug in CHASM. You shouldn't need to inspect the type. There's no reason that chasmEngine.UpdateComponent would work and chasm.UpdateComponent(ctx) would not work.

…esponse.proto Co-authored-by: Roey Berman <roey@temporal.io>

ghost · 2025-10-10T18:59:02Z

+// ChasmNexusCompletion includes details about a completed Nexus operation.
+message ChasmNexusCompletion {


It's not CHASM specific, you can move this somewhere else but not blocking the PR.

ghost · 2025-10-10T18:59:37Z

+	// this similarly as we would a pure task (holding an exclusive lock), as the
+	// assumption is that the accessed component will be recording (or generating a
+	// task) based on this result.
+	_, err := h.chasmEngine.UpdateComponent(ctx, ref, func(ctx chasm.MutableContext, component chasm.Component) error {


That's what I expect, that you can use the interface instead of a concrete type.

lina-temporal added 2 commits September 18, 2025 17:16

[Nexus] Add CloseTime header field to Nexus operation completions gen…

1ef0b12

…erated from History

Merge branch 'main' into nexus-close-time

dca87bb

lina-temporal requested review from bergundy, chaptersix and yycptt September 23, 2025 22:23

lina-temporal requested a review from a team as a code owner September 23, 2025 22:23

bergundy requested changes Sep 25, 2025

View reviewed changes

lina-temporal commented Oct 1, 2025

View reviewed changes

lina-temporal requested a review from bergundy October 1, 2025 02:15

bergundy reviewed Oct 1, 2025

View reviewed changes

lina-temporal requested a review from bergundy October 2, 2025 22:46

bergundy reviewed Oct 6, 2025

View reviewed changes

Lina Jodoin added 10 commits October 8, 2025 12:51

Merge branch 'main' into nexus-close-time

a8bb19e

[Nexus] Support internal callback completion routing to CHASM

5f74bf8

clean up chasm interface

0c8f92b

assert iface upfront

34f0f2d

superfluous comment

d632295

feedback: complete via CompleteNexusOperationChasm

d4fb2c3

Fix tests

a92de13

Use NexusCompletionInfo to pass around completion details

fb6d2fa

PR Feedback

889b75c

Use main Nexus header for CHASM ref

9aee894

lina-temporal force-pushed the nexus-chasm branch from 0b752c9 to 9aee894 Compare October 8, 2025 19:59

lina-temporal requested a review from a team as a code owner October 8, 2025 19:59

Lina Jodoin added 2 commits October 8, 2025 13:21

renames and PR feedback

8fab2f2

Remove superfluous suffix

3f31165

lina-temporal requested a review from bergundy October 8, 2025 20:29

bergundy reviewed Oct 8, 2025

View reviewed changes

Lina Jodoin and others added 2 commits October 9, 2025 13:04

Update proto/internal/temporal/server/api/historyservice/v1/request_r…

b4abb4e

…esponse.proto Co-authored-by: Roey Berman <roey@temporal.io>

PR feedback re logging

e4773f4

lina-temporal requested a review from bergundy October 9, 2025 20:05

bergundy approved these changes Oct 10, 2025

View reviewed changes

Base automatically changed from nexus-close-time to main October 16, 2025 21:23

Lina Jodoin added 2 commits October 16, 2025 14:32

Merge branch 'main' into nexus-chasm

8ebde1c

Merge branch 'main' into nexus-chasm

a52a5e3

lina-temporal enabled auto-merge (squash) October 16, 2025 21:33

lina-temporal merged commit 2234ce8 into main Oct 16, 2025

lina-temporal deleted the nexus-chasm branch October 16, 2025 21:58

tekkaya mentioned this pull request Jul 27, 2026

Surface canceled CHASM Nexus operations as CanceledError #11312

Merged

4 of 5 tasks

	NexusCompletionHandlerURL = "temporal://internal/chasm"
	NexusCompletionHandlerURL = "temporal://internal"

		// Base URL for Nexus->CHASM callbacks.
		NexusCompletionHandlerURL = "temporal://system/nexus/callback/chasm"

	return queues.NewDestinationDownError(retry.err.Error(), err)
	return err

		// ChasmNexusCompletion includes details about a completed Nexus operation.
		message ChasmNexusCompletion {

Conversation

lina-temporal commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed?

Why?

How did you test it?

Potential risks

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!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

ghost Oct 1, 2025

Choose a reason for hiding this comment

lina-temporal commented Sep 23, 2025 •

edited

Loading