Add API for GetWorkflowExecutionResult by long-nt-tran · Pull Request #778 · temporalio/api

long-nt-tran · 2026-04-30T21:14:33Z

Add frontend API and request/response protos for getting workflow execution result.

Since this is intended to be used via Nexus, the API allows caller to attach callbacks for an ongoing workflow execution.

Why?
New API to enable Nexus ops to map more ergonomically in the handler.

Breaking changes
Not breaking, new (unused) API

Server PR
temporalio/temporal#10173

- Split enums to be more granular (NotCompleted/Success/Failure) - Move some common fields to base message - Comment regarding always getting result of latest run for a workflow_id

long-nt-tran · 2026-05-08T15:19:19Z

+        Failure failure = 6;
+    }
+
+    message NotCompleted {}


I think empty message is ok (basically using this as an enum)? I see it used elsewhere, i.e.,

api/temporal/api/workflow/v1/message.proto

Line 460 in 8e0453c

message WorkflowClosed {}

+1 to using an empty message instead of protobuf.Empty{} or similar. This allows us to add more data later if needed. e.g. maybe we'll want to wire in a "time_until_timeout" or "started_at" field later.

stephanos · 2026-05-08T15:51:22Z

+
+    // Completion status of the workflow, which carries relevant information for the caller.
+    oneof completion {
+        NotCompleted not_completed = 4;


What's the purpose of this field if there's already status?

I understand that there might not be a success or failure yet, I just wonder if that should be expressed as an empty/nil completion field. Not sure, maybe more an SDK question as it might impact its user-facing API impl.

I'm using this as an "enum that carries extra info" (i.e., Success carries the result, Failure carries the failure). I was thinking that status is a more granular set of enums in case the user really wants to see the status (I.e., what kind of failure it is), although maybe this is also inspectable via the failure itself.

Leaning towards removing status field since the "enum with extra info" is a bit easier to carry extra info in success/failure cases

- Comments on each proto field - Remove status field -- this can be inferrable via the oneof completion status - Spacing and enumeration

chrsmith

I'll defer to Stephan for approval. But modulo some nits this LGTM!

chrsmith · 2026-05-08T18:13:41Z

+        Failure failure = 6;
+    }
+
+    message NotCompleted {}


+1 to using an empty message instead of protobuf.Empty{} or similar. This allows us to add more data later if needed. e.g. maybe we'll want to wire in a "time_until_timeout" or "started_at" field later.

chrsmith · 2026-05-08T18:18:00Z

    rpc DeleteNexusOperationExecution (DeleteNexusOperationExecutionRequest) returns (DeleteNexusOperationExecutionResponse) {}
+
+    // GetWorkflowExecutionResult returns the result of a workflow execution. If the workflow
+    // is still running, returns a NotCompleted response and optionally registers callbacks


Nit: It doesn't return a "NotCompleted" response. It will always return the same GetWorkflowExecutionResultResponse, which will describe it's status which may be NotComplete.

I'd slightly argue that we can just remove the sentence "If the workflow is still running..." since it doesn't clarify anything. It will always return the result of a workflow execution.

What do you think of the following. Is this strictly better?

// GetWorkflowExecutionResult returns the state of a workflow execution, which may be still running, completed, or failed. It can optionally register new // callbacks to be invoked when Workflow reaches a terminal state. // (Adding a callback to a completed workflow will return an XXX error.)

thank you! I edited to be a variant copied from your comment

chrsmith · 2026-05-08T18:45:18Z

+    string identity = 4;
+    // Callbacks to be called by the server when this activity reaches a terminal state.
+    // Callback addresses must be allowlisted in the server's dynamic configuration.
+    repeated temporal.api.common.v1.Callback completion_callbacks = 5;


I don't know if this is quite right, but reserve the right to be wrong 😅

You would expect an operation like Get* to not mutate the underlying resource. (At least that's the expectation in the HTTP world.) So it seems strange that calling Get* could actually modify the workflow's state, and register new callbacks.

Is there a particular reason we want to allow registering new callbacks? And if not, maybe we can remove this capability?

If we do want to enable registering new callbacks to a Workflow (which seems like it'd be useful), should we instead add a separate UpdateWorkflowExecution operation?

I think from the user's POV, this request is semantically a "get" operation even if the implementation detail requires some mutation to the target's state to deliver that result. I think Nexus-mapped operation would have to attach some sort of callback to deliver result async. An example is:

Alice uses a Nexus op in their namespace that maps to GetWorkflowExecutionResult for a workflow in Bob's namespace

Workflow hasn't completed. As an implementation detail, this request now attaches a "completion callback" which is essentially a URL

Once the workflow in Bob's namespace finishes, the result will be delivered to this callback URL (for example this is its CHASM implementation which is a retryable RPC/HTTP call wrapped in a state machine)

I do agree that it can sound strange to have the "Get" modify the workflow, although it's not really modifying the workflow's state, but rather attaches a listener onto the workflow.

From the user's POV, how the result gets delivered is kinda opaque so I think Get* sounds ok here?

stephanos · 2026-05-08T16:36:39Z

+    string request_id = 3;
+    // The identity of the client who initiated this request.
+    string identity = 4;
+    // Callbacks to be called by the server when this activity reaches a terminal state.


nit: this says "activity"?

bad copypasta from me

stephanos · 2026-05-08T16:36:43Z

+    // provided even if the run_id is set here. We use WorkflowExecution message for forward
+    // compatibility if we want to enable getting the result of a specific run_id in the future.
+    temporal.api.common.v1.WorkflowExecution execution = 2;
+    // A unique identifier for this create request for idempotence. Typically UUIDv4.


nit: this says "create request"?

bad copypasta from me

stephanos · 2026-05-08T16:36:50Z

+    // to be invoked when the workflow completes.
+    rpc GetWorkflowExecutionResult (GetWorkflowExecutionResultRequest) returns (GetWorkflowExecutionResultResponse) {
+        option (google.api.http) = {
+            get: "/namespaces/{namespace}/workflows/{execution.workflow_id}/result"


I was surprised to see this was GET, but I see GetWorkflowExecutionHistoryRequest is also a GET. Worth checking if this should be a POST, though.

yeah... i guess attaching callback is a mutation to the backend (even if an implementation detail)

stephanos · 2026-05-13T18:48:31Z

        },
        "lastAttemptFailure": {
-          "$ref": "#/definitions/v1Failure",
+          "$ref": "#/definitions/failureV1Failure",


hm, this is odd?

from claude, seems legit:

The root cause is that your nested message Failure adds a second message ending in Failure to the generator's scope, which bumps the minimum-unique-depth for temporal.api.failure.v1.Failure and cascades. The fix is to rename the nested message so it doesn't end in Failure: // Before message Failure { temporal.api.failure.v1.Failure failure = 1; } // After message Failed { temporal.api.failure.v1.Failure failure = 1; } And update the oneof: oneof completion_status { NotCompleted not_completed = 3; Success success = 4; Failed failure = 5; // field name stays "failure", type is now "Failed" } This way the generator's scope still has only one message whose FQN ends in .Failure (temporal.api.failure.v1.Failure), so its minimum-unique-depth stays at 1 and it keeps the name v1Failure. Alternatively, if the wrapper is unnecessary, you could use temporal.api.failure.v1.Failure directly in the oneof and drop the nested message entirely — but that depends on whether you need the wrapper for forward compatibility.

It seems harmless(?), but I renamed the enums for now

spkane31 · 2026-05-15T21:04:45Z

+    // If the workflow is completed, it returns the result of the completed workflow.
+    // NOTE: This has to be a POST since we're optionally attaching callbacks to the running workflow, which mutates it.
+    rpc GetWorkflowExecutionResult (GetWorkflowExecutionResultRequest) returns (GetWorkflowExecutionResultResponse) {
+        option (google.api.http) = {


Can you add

option (nexusannotations.v1.operation).tags = "exposed";

at the top of this rpc so we can expose it as a system nexus operation?

same as the SignalWithStartWorkflowExecution rpc

bergundy

Let's wait with this because we are going to define a completely separate API for the system Nexus endpoint for the same purpose and there's still open discussions around #720 that prevents us from having a single API surface.

long-nt-tran mentioned this pull request May 4, 2026

Implement GetWorkflowExecutionResult temporalio/temporal#10173

Open

5 tasks

Add API for GetWorkflowExecutionResult

0cea08d

long-nt-tran force-pushed the get-workflow-result branch from c13b529 to 0cea08d Compare May 5, 2026 22:32

long-nt-tran commented May 7, 2026

View reviewed changes

Comment thread temporal/api/workflowservice/v1/request_response.proto

long-nt-tran requested review from Quinn-With-Two-Ns and stephanos May 7, 2026 22:39

long-nt-tran marked this pull request as ready for review May 7, 2026 22:39

long-nt-tran requested review from a team May 7, 2026 22:39

Quinn-With-Two-Ns reviewed May 8, 2026

View reviewed changes

Comment thread temporal/api/workflowservice/v1/request_response.proto Outdated

Address PR feedback

f56de32

- Split enums to be more granular (NotCompleted/Success/Failure) - Move some common fields to base message - Comment regarding always getting result of latest run for a workflow_id

long-nt-tran commented May 8, 2026

View reviewed changes

long-nt-tran requested a review from Quinn-With-Two-Ns May 8, 2026 15:21

stephanos reviewed May 8, 2026

View reviewed changes

PR feedback

7320d50

- Comments on each proto field - Remove status field -- this can be inferrable via the oneof completion status - Spacing and enumeration

long-nt-tran requested a review from stephanos May 8, 2026 16:28

chrsmith reviewed May 8, 2026

View reviewed changes

stephanos reviewed May 13, 2026

View reviewed changes

long-nt-tran added 4 commits May 14, 2026 17:35

Merge remote-tracking branch 'upstream/master' into get-workflow-result

d9c8a71

Address PR feedback

1eb336a

Rename completion statuses

20acf1d

Revert success enum name

746730e

long-nt-tran requested review from chrsmith and stephanos May 15, 2026 14:30

spkane31 reviewed May 15, 2026

View reviewed changes

bergundy reviewed May 15, 2026

View reviewed changes

Conversation

long-nt-tran commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrsmith 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!

bergundy left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

long-nt-tran commented Apr 30, 2026 •

edited

Loading