Skip to content

Conversation

@nikki-dag
Copy link
Contributor

@nikki-dag nikki-dag commented Feb 6, 2026

What changed?

  1. Exposed metadata functionality:
    • Made SetContextMetadata() public in MutableState interface
    • Added constants for metadata keys (MetadataKeyWorkflowType, MetadataKeyWorkflowTaskQueue)
    • Added ContextHasMetadata() helper function to check if context has metadata support
  2. Fixed readonly operations:
    - QueryWorkflow: Now explicitly calls SetContextMetadata() since queries never close transactions (readonly operation)
    - RespondWorkflowTaskCompleted: Calls SetContextMetadata() when there are only messages (e.g., update rejections) and no commands, since these also don't close transactions
  3. Added tests for the new ContextHasMetadata() helper and SetContextMetadata() functionality

Why?

Context metadata (workflow type, task queue) is automatically populated when mutable state transactions close. However, readonly operations never close transactions, so the metadata was missing from their contexts.
This change ensures that successful readonly operations (queries, update rejections) also have workflow metadata populated in their contexts

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Note

Medium Risk
Touches core history API request paths and mutable state interfaces; while behavior is additive (context tagging), incorrect invocation or key usage could affect observability/metrics and tests across multiple workflows.

Overview
Ensures workflow context metadata (workflow type/task queue) is consistently populated even for readonly paths that don’t close mutable-state transactions.

This exposes MutableState.SetContextMetadata(ctx) (renaming the internal helper), standardizes metadata keys via contextutil.MetadataKeyWorkflowType/MetadataKeyWorkflowTaskQueue, adds ContextHasMetadata() for debugging, and explicitly calls SetContextMetadata in QueryWorkflow and in RespondWorkflowTaskCompleted when Commands is empty (e.g., update rejections / heartbeats). Tests are expanded to assert metadata presence across success and several error/edge cases.

Written by Cursor Bugbot for commit 69c3140. This will update automatically on new commits. Configure here.

@nikki-dag nikki-dag requested review from a team as code owners February 6, 2026 06:21
@nikki-dag nikki-dag force-pushed the nikki/readonly-context-metadata branch from 7497389 to 4221700 Compare February 6, 2026 06:44
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

}
}

if len(request.Commands) == 0 && len(request.Messages) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

hmm... do we have any action metric to emit if both are empty? Is workflow task heartbeat an action?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this!

Copy link
Member

@yycptt yycptt left a comment

Choose a reason for hiding this comment

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

We need some tests ensuring those two apis handler have the context populated.

@nikki-dag nikki-dag force-pushed the nikki/readonly-context-metadata branch 3 times, most recently from f2a2de6 to c3b8c49 Compare February 10, 2026 23:24
@nikki-dag nikki-dag force-pushed the nikki/readonly-context-metadata branch from c3b8c49 to a522a1e Compare February 11, 2026 00:43
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to change the existing tests in history_engine_tests.go?

// Context metadata is automatically set during mutable state transaction close. For RespondWorkflowTaskCompleted
// with no commands (e.g., workflow task heartbeat or only readonly messages like `update.Rejection`), the transaction
// is never closed. We explicitly call SetContextMetadata here to ensure workflow metadata is populated in the context.
workflowLease.GetMutableState().SetContextMetadata(ctx)
Copy link
Member

@yycptt yycptt Feb 11, 2026

Choose a reason for hiding this comment

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

fwiw, I don't think the mutable state you get from workflowLease.GetMutableState() will always be the latest mutable state because it's a separate copy in this workflowLease object, while the real one is in workflowLease.Context. Doesn't really matter for your case the can be confusing?

Why not use the ms variable instead and do it before the defer() method? similar to what you have in the Query api

@nikki-dag nikki-dag merged commit 2e3c285 into temporalio:main Feb 11, 2026
71 checks passed
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