PollComponent and PollActivityExecution by dandavison · Pull Request #8563 · temporalio/temporal

dandavison · 2025-10-29T01:13:42Z

What changed?

Add history.ChasmNotifier for subscribing to CHASM execution state transitions
Implement chasm.PollComponent
Add PollActivityExecution API handler

Why?

Needed for standalone activity
Needed for other situations where we long-poll a CHASM execution/entity

How did you test it?

built
run locally and tested manually (TODO: we are set up to do this with the Python prototype)
covered by existing tests
added new unit test(s)
added new functional test(s)

Note

Implements long‑polling for CHASM components and standalone activities via new PollComponent API, PollActivityExecution endpoint, and an execution notifier, with validation, ref handling, and tests.

CHASM Core:
- Add Engine.PollComponent (with NotifyExecution) and helper ExecutionStateChanged; extend Context with structuredRef.
- Introduce ChasmNotifier for execution-level subscriptions; wire into history engine; emit notifications on CHASM mutations.
- Tighten ref deserialization with ErrMalformedComponentRef/ErrInvalidComponentRef and validation; expose structuredRef in tree.
Activity:
- Implement PollActivityExecution frontend and history handler using PollComponent; build ActivityExecutionInfo and response assembly.
- Add dynamic config for long-poll (LongPollTimeout, LongPollBuffer); provide via FX modules.
APIs/Protos:
- Define PollActivityExecution{Request,Response} messages and service RPC; generate client/grpc helpers.
Tests:
- Unit: notifier subscribe/notify, engine PollComponent (no-wait, wait, stale).
- Functional: standalone activity polling (no-wait, wait any state change, deadline behavior, invalid args, not found); start-to-close timeout path.
Mocks/Interfaces:
- Update engine mocks for new PollComponent signature and NotifyExecution; history engine interface adds NotifyChasmExecution.

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

service/history/chasm_engine.go

dandavison · 2025-12-04T02:35:54Z

service/history/chasm_engine.go

+		ref, err := e.predicateSatisfied(ctx, monotonicPredicate, requestRef, executionLease)
+		if err != nil {
+			if errors.Is(err, consts.ErrStaleState) {
+				err = serviceerror.NewUnavailable("please retry")


I need a better error message here, but I haven't decided what.

"request data ahead of persisted data"

?

Also, should it be PollActivityExecution that converts this to serviceerror?

bergundy · 2025-12-05T00:14:45Z

chasm/lib/activity/config.go

+
+var (
+	LongPollTimeout = dynamicconfig.NewNamespaceDurationSetting(
+		"chasm.activity.longPollTimeout",


Not blocking this PR but I don't think putting chasm. as a prefix for user configs is good practice. We will likely want to change this across all libraries though.

bergundy

I didn't review the tests very closely.
There are still come open comments, please address before merging but I do not feel like I need another pass here.

chasm/lib/activity/config.go

bergundy · 2025-12-05T00:17:39Z

chasm/lib/activity/handler.go

+			// TODO(dan): include execution key in error message; we may do this at the CHASM
+			// framework level.


TBH I don't think this is needed. Not blocking the PR.

I've removed the comment

bergundy · 2025-12-05T00:19:52Z

chasm/lib/activity/handler.go

+		if len(token) == 0 {
+			return chasm.ReadComponent(ctx, ref, (*Activity).buildPollActivityExecutionResponse, req, nil)
+		}


This seems more like an invalid argument to me.

OK, we will address when we split the API

bergundy · 2025-12-05T00:36:09Z

service/history/chasm_notifier.go

+type (
+	// ChasmNotifier allows subscribers to receive notifications relating to a CHASM execution.
+	ChasmNotifier struct {
+		executions map[chasm.EntityKey]*subscriptionTracker


Not blocking, but maybe put a TODO here to use the sharded map which will guarantee less lock contention?

Done, and internally tracking need for an audit of TODOs in code

bergundy · 2025-12-05T00:37:46Z

service/history/history_engine.go

 	e.eventNotifier.NotifyNewHistoryEvent(notification)
 }

+func (e *historyEngineImpl) ChasmEngine() chasm.Engine {


Is this called anywhere? Maybe I missed it?

Thanks, removed

bergundy · 2025-12-05T00:38:40Z

tests/standalone_activity_test.go

 )

+var (
+	defaultInput = &commonpb.Payloads{


I mentioned this before I believe, use payloads.EncodeString() from common

Done (not specifically related to this PR so it may conflict, but good to do now in case we forget)

bergundy · 2025-12-05T00:39:24Z

tests/standalone_activity_test.go

-
+func (s *standaloneActivityTestSuite) SetupSuite() {
+	s.FunctionalTestBase.SetupSuite()
+	s.tv = testvars.New(s.T())


I think this needs to be done in SetupTest

bergundy · 2025-12-05T00:39:54Z

tests/standalone_activity_test.go

 	}
 	input := createDefaultInput()
-	taskQueue := uuid.New().String()
+	taskQueue := testcore.RandomizeStr(t.Name())


If you're already using testvars, might as well fully use it. IMHO that utility doesn't give us much but leaving it up to you.

I've switched them over

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

yycptt

Also need to change all EntityKey -> ExecutionKey

yycptt · 2025-12-05T00:47:43Z

chasm/ref.go

+	}
 	var pRef persistencespb.ChasmComponentRef
 	if err := pRef.Unmarshal(data); err != nil {
 		return ComponentRef{}, err


ErrMalformedComponentRef?

Thanks, done

yycptt · 2025-12-05T00:48:30Z

chasm/ref.go

+// ErrMalformedComponentRef is returned when component ref bytes cannot be deserialized.
+var ErrMalformedComponentRef = errors.New("malformed component ref")
+
+// ErrInvalidComponentRef is returned when component ref bytes deserialize to an invalid component ref.
+var ErrInvalidComponentRef = errors.New("invalid component ref")


I'd return invalidRequest here unless we are sure all api handlers have proper error conversion logic.

Right, done.

yycptt · 2025-12-05T00:49:09Z

service/history/interfaces/engine.go


 		NotifyNewHistoryEvent(event *events.Notification)
 		NotifyNewTasks(tasks map[tasks.Category][]tasks.Task)
+		ChasmEngine() chasm.Engine


nit: is this one required/used?

No, thanks!

yycptt · 2025-12-05T00:56:26Z

service/history/workflow/transaction_impl.go

+		// Notify for current workflow if it has CHASM updates
+		if len(currentWorkflowMutation.UpsertChasmNodes) > 0 ||
+			len(currentWorkflowMutation.DeleteChasmNodes) > 0 {
+			engine.NotifyChasmExecution(chasm.EntityKey{


let's do it in ConflictResolveExecution as well. Create execution is probably fine, I guess there won't be any poller before execution is created.

hmm I haven't think through if it needs to be in OperationPossiblySucceeded. Can you elaborate your thoughts a bit here.

Thanks, let's address this in one more PR to target standalone-activity. It will all arrive in main at the same time.

yycptt · 2025-12-05T00:59:58Z

service/history/chasm_engine.go

+			if ref != nil {
+				return ref, nil
+			}
+		case <-ctx.Done():


do we have some tail room here to return an empty response and avoid a timeout error?

In the design that @bergundy and I have settled on, the caller sets the tail room. See PollActivityExecution in chasm/lib/activity/handler.go.

yycptt · 2025-12-05T01:03:08Z

service/history/chasm_engine.go

+	// behind the requested reference. However, getExecutionLease does not currently guarantee that
+	// execution VT >= ref VT, therefore we call IsStale() again here and return any error (which at
+	// this point must be ErrStaleState; ErrStaleReference has already been eliminated).
+	err := chasmTree.IsStale(ref)


this is already checked in getExecutionLease? or it's for fixing the bug we discussed before that getExecutionLease needs to do another stale check after reload?

Yes that's right. We can remove this when that bug is fixed.

dandavison · 2025-12-05T01:37:11Z

Also need to change all EntityKey -> ExecutionKey

Right, we will do that shortly when we merge/rebase. It will be nice to have that done.

cursor · 2025-12-05T01:56:11Z

chasm/lib/activity/handler.go

+			req *activitypb.PollActivityExecutionRequest,
+		) (*activitypb.PollActivityExecutionResponse, bool, error) {
+			// TODO(dan): check for terminal activity states
+			panic("pollActivityExecutionWaitCompletion is not implemented")


Bug: Panic in WaitCompletion handler crashes server

The PollActivityExecution handler contains a panic("pollActivityExecutionWaitCompletion is not implemented") statement in the WaitCompletion case branch. If a user sends a PollActivityExecutionRequest with a WaitCompletion wait policy, this will crash the server. This panic should be replaced with returning a proper error like serviceerror.NewUnimplemented("WaitCompletion is not yet implemented") to avoid server crashes.

This reverts commit 1f413ce.

cursor · 2025-12-05T03:22:10Z

chasm/lib/activity/handler.go

+	waitPolicy := req.GetFrontendRequest().GetWaitPolicy()
+
+	if waitPolicy == nil {
+		return chasm.ReadComponent(ctx, ref, (*Activity).buildPollActivityExecutionResponse, req, nil)


Bug: Deferred error transformation bypassed on early returns

The deferred function at lines 80-85 transforms NotFound errors into a user-friendly message by modifying the named return variable err. However, the direct return chasm.ReadComponent(...) statements on lines 90 and 109 bypass the named return variable entirely. In Go, when using return expr1, expr2 with named returns, the expressions go directly to the caller without updating the named variables. This means NotFound errors from the waitPolicy == nil and len(token) == 0 code paths won't be transformed to "activity execution not found".

Additional Locations (1)

chasm/lib/activity/handler.go#L108-L109

cursor · 2025-12-05T03:57:36Z

chasm/transition_history.go

+	ref, err := DeserializeComponentRef(refBytes)
+	if err != nil {
+		return false, ErrMalformedComponentRef
+	}


Bug: ExecutionStateChanged always returns ErrMalformedComponentRef discarding original error type

The function ExecutionStateChanged unconditionally returns ErrMalformedComponentRef when DeserializeComponentRef fails, but DeserializeComponentRef can return either ErrMalformedComponentRef or ErrInvalidComponentRef (for empty data or missing fields). The function's doc comment claims it "may return ErrInvalidComponentRef or ErrMalformedComponentRef" but the implementation always substitutes ErrMalformedComponentRef on deserialization error. The original error should be returned directly (return false, err) instead of always returning ErrMalformedComponentRef.

dandavison force-pushed the update-protos branch from cb5f326 to 21cdb37 Compare October 30, 2025 20:00

Base automatically changed from update-protos to standalone-activity October 30, 2025 20:25

dandavison force-pushed the standalone-activity branch from ded2e1b to efc4722 Compare October 30, 2025 20:49

dandavison force-pushed the poll-component branch 10 times, most recently from 3281080 to 9c65e47 Compare November 11, 2025 03:18

dandavison force-pushed the poll-component branch 11 times, most recently from ce95cbe to 29a0bd9 Compare November 12, 2025 18:32

dandavison marked this pull request as ready for review November 12, 2025 18:51

dandavison requested review from a team as code owners November 12, 2025 18:51

dandavison force-pushed the poll-component branch from 29a0bd9 to 33e981e Compare November 12, 2025 19:01

cursor bot reviewed Nov 12, 2025

View reviewed changes

service/history/chasm_engine.go Show resolved Hide resolved

dandavison force-pushed the poll-component branch from 40e0b6b to 28665bb Compare November 12, 2025 20:06

dandavison added 3 commits December 3, 2025 20:51

Rename: ChasmEngine()

a6835e0

Properly wire Notify method through ChasmEngine

1b821f3

Error message fixup

8aa7189

dandavison commented Dec 4, 2025

View reviewed changes

bergundy reviewed Dec 5, 2025

View reviewed changes

bergundy approved these changes Dec 5, 2025

View reviewed changes

dandavison and others added 4 commits December 4, 2025 19:45

Improve docstring

f12d710

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

Improve docstring

1018228

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

Improve docstring

4a3e8c3

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

Cleaner return of err and ref

34b3f8a

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

yycptt reviewed Dec 5, 2025

View reviewed changes

dandavison added 2 commits December 4, 2025 20:31

Cleanup

f28fe2a

Cleanup

26c6058

dandavison added 2 commits December 4, 2025 20:46

Remove unnecessary method from interface

605beb3

Use serviceerror InvalidArgument

ba38880

cursor bot reviewed Dec 5, 2025

View reviewed changes

dandavison added 9 commits December 4, 2025 20:57

Use testvars

eae7532

Use payloads.EncodeString

f8f4992

Add TODO

6820744

Code review improvements to chasm_notifier

ab0e5cc

Remove unnecessary lc

ead88c0

Add TODO

ab1a8ab

Simplify test

1f413ce

Revert "Simplify test"

9902b0d

This reverts commit 1f413ce.

Cleanup

2c24dfc

cursor bot reviewed Dec 5, 2025

View reviewed changes

Lint

1d59e66

cursor bot reviewed Dec 5, 2025

View reviewed changes

dandavison mentioned this pull request Jan 14, 2026

Notify CHASM components on additional persistence write events #9030

Merged

5 tasks

		// TODO(dan): include execution key in error message; we may do this at the CHASM
		// framework level.

Conversation

dandavison commented Oct 29, 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?

Uh oh!

Uh oh!

dandavison Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

Uh oh!

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!

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

dandavison commented Dec 5, 2025

Uh oh!

cursor bot Dec 5, 2025

dandavison commented Oct 29, 2025 •

edited

Loading

dandavison Dec 4, 2025 •

edited

Loading

Bug: `ExecutionStateChanged` always returns `ErrMalformedComponentRef` discarding original error type