Skip to content

Conversation

@pdoerner
Copy link
Contributor

What changed?

Added a new special MSPointer type to the CHASM framework

Why?

To support components invoking methods from the underlying mutable state. Specifically, Nexus callbacks will need to get the Nexus completion from the mutable state.

How did you test it?

updated unit tests

@pdoerner pdoerner marked this pull request as ready for review October 15, 2025 17:53
@pdoerner pdoerner requested review from a team as code owners October 15, 2025 17:53
Copy link
Member

@bergundy bergundy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had some comments but non of them are blocking.

Comment on lines 18 to 19
// State of the workflow is managed by mutable_state_impl, not CHASM engine, so this will always be empty.
State proto.Message
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// State of the workflow is managed by mutable_state_impl, not CHASM engine, so this will always be empty.
State proto.Message
// State of the workflow is managed by mutable_state_impl, not CHASM engine, so this will always be empty.
*emptypb.Empty

State proto.Message

// MSPointer is a special in-memory field for getting mutable state access.
MSPointer chasm.MSPointer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No other component is allowed to have this pointer and this is really just a transitional artifact (even if the transition may take years).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the CHASM framework would panic if any other component tried to use this hack.


if ms.chasmEnabled() {
// Initialize chasm tree once for new workflows.
mutableContext := chasm.NewMutableContext(context.TODO(), ms.chasmTree.(*chasm.Node))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yycptt again us needing to make up these context.Context objects.
@pdoerner I guess you can use context.Background() for this but put a comment explaining why.

s.nodeBackend.EXPECT().GetCurrentVersion().Return(int64(1)).AnyTimes()
s.nodeBackend.EXPECT().UpdateWorkflowStateStatus(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
s.nodeBackend.EXPECT().GetWorkflowKey().Return(tv.Any().WorkflowKey()).AnyTimes()
s.nodeBackend.EXPECT().IsWorkflow().Return(false).AnyTimes()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lina-temporal, @yycptt this is what you get when you use these tight mocks, every little behavior change needs to be propagated to every test in the codebase.

@pdoerner pdoerner enabled auto-merge (squash) October 16, 2025 17:05
@pdoerner pdoerner merged commit c5540c4 into temporalio:main Oct 16, 2025
57 checks passed
@pdoerner pdoerner deleted the chasm-nexus-completions branch October 16, 2025 17:29
@yycptt yycptt mentioned this pull request Oct 27, 2025
5 tasks
yycptt added a commit that referenced this pull request Oct 27, 2025
## What changed?
- Wire up chasm workflow library

## Why?
- #8485 makes workflow start to use CHASM as well and 
we need to register workflow as a chasm library.

## How did you test it?
- [x] built
- [ ] run locally and tested manually
- [ ] covered by existing tests
- [ ] added new unit test(s)
- [ ] added new functional test(s)

## Potential risks
- Logic is only used when chasm feature flag is turned on. The change is
mainly for functional tests.
chaptersix pushed a commit that referenced this pull request Oct 30, 2025
## What changed?
- Wire up chasm workflow library

## Why?
- #8485 makes workflow start to use CHASM as well and 
we need to register workflow as a chasm library.

## How did you test it?
- [x] built
- [ ] run locally and tested manually
- [ ] covered by existing tests
- [ ] added new unit test(s)
- [ ] added new functional test(s)

## Potential risks
- Logic is only used when chasm feature flag is turned on. The change is
mainly for functional tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants