Set TargetVersionChanged instead of SuggestCaN when TargetVersionChanged#9239
Merged
Set TargetVersionChanged instead of SuggestCaN when TargetVersionChanged#9239
Conversation
carlydf
force-pushed
the
cdf/target-version-changed
branch
from
6d5323c to
979c682
Compare
Shivs11
reviewed
Feb 10, 2026
Shivs11
reviewed
Feb 10, 2026
Shivs11
reviewed
Feb 10, 2026
| currentDeploymentVersion.SeriesName != targetDeploymentVersion.DeploymentName) { | ||
| suggestContinueAsNew = cmp.Or(suggestContinueAsNew, true) | ||
| suggestContinueAsNewReasons = append(suggestContinueAsNewReasons, enumspb.SUGGEST_CONTINUE_AS_NEW_REASON_TARGET_WORKER_DEPLOYMENT_VERSION_CHANGED) | ||
| targetDeploymentVersionChanged = true |
Member
There was a problem hiding this comment.
should we not be checking if the workflow's effective behaviour is pinned or not?
Member
There was a problem hiding this comment.
actually, thought about it a bit more and realized that while there is no immediate harm in having this be set to true for workflows of any versioning behaviour type. However, gonna keep this comment here just to think about if we want this flag to show up and only be used by folks who are authoring pinned workflows.
Shivs11
approved these changes
Feb 10, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
carlydf
added a commit
to temporalio/api
that referenced
this pull request
Feb 12, 2026
…onChanged` with new `TargetVersionChanged` flag (#709) _**READ BEFORE MERGING:** All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted._ <!-- Describe what has changed in this PR --> - Replace SuggestContinueAsNew + SuggestContinueAsNewReasonTargetVersionChanged with new TargetVersionChanged flag - Add new `target_worker_deployment_version_changed` flag to be sent in WorkflowTaskStartedEvent and exposed next to `continue-as-new-suggested` in the workflow info. <!-- Tell your future self why have you made these changes --> **Why?** Setting `SuggestContinueAsNew=true` for Pinned workflows whenever their is a new Target Version available for that workflow causes Pinned workflows to hit that condition much more frequently than they expect. Users who are currently doing: `if workflow_info.suggestContinueAsNew{ do continue-as-new }` in their Pinned workflow code would need to change that code to protect themselves from running into an infinite-CaN-loop, because the default CaN behavior for a Pinned workflow is to stay Pinned. We should not force users to protect themselves from such a situation. Because upgrading on continue-as-new is opt-in, receiving the suggestion to continue-as-new-onto-new-target-version should be opt-in as well. If people are forced to check the new `suggest-continue-as-new-reasons` field to "opt out," that is unsafe, because inevitably some people will forget to do so or misunderstand, and then get hit by this unexpected footgun. Much safer and still ergonomical to let upgrade-on-can be opt-in on both fronts, as proposed here. With this change, the people who are currently doing `if workflow_info.suggestContinueAsNew{ do continue-as-new }` won't see any change in semantics, regardless of their versioning behavior. People who consciously know that they want to do upgrade-on-can / Trampolining will have to change their CaN options anyway, so it's easy enough to teach them to pay attention to this new `TargetWorkerDeploymentVersionChanged` flag. <!-- Are there any breaking changes on binary or code level? --> **I am proposing to completely remove the "new target version" reason from the can-suggested reasons list. Some SDK PRs use it, and they should stop. We could also simply deprecate the reason and just tell people that it's not used anymore, but I'd prefer to flat out delete it. Server is already patched to not send that flag by default.** <!-- If this breaks the Server, please provide the Server PR to merge right after this PR was merged. --> **Server PR** temporalio/temporal#9239 --------- Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io>
temporal-cicd bot
pushed a commit
to temporalio/api-go
that referenced
this pull request
Feb 12, 2026
…onChanged` with new `TargetVersionChanged` flag (#709)
_**READ BEFORE MERGING:** All PRs require approval by both Server AND
SDK teams before merging! This is why the number of required approvals
is "2" and not "1"--two reviewers from the same team is NOT sufficient.
If your PR is not approved by someone in BOTH teams, it may be summarily
reverted._
<!-- Describe what has changed in this PR -->
- Replace SuggestContinueAsNew +
SuggestContinueAsNewReasonTargetVersionChanged with new
TargetVersionChanged flag
- Add new `target_worker_deployment_version_changed` flag to be sent in
WorkflowTaskStartedEvent and exposed next to `continue-as-new-suggested`
in the workflow info.
<!-- Tell your future self why have you made these changes -->
**Why?**
Setting `SuggestContinueAsNew=true` for Pinned workflows whenever their
is a new Target Version available for that workflow causes Pinned
workflows to hit that condition much more frequently than they expect.
Users who are currently doing: `if workflow_info.suggestContinueAsNew{
do continue-as-new }` in their Pinned workflow code would need to change
that code to protect themselves from running into an infinite-CaN-loop,
because the default CaN behavior for a Pinned workflow is to stay
Pinned.
We should not force users to protect themselves from such a situation.
Because upgrading on continue-as-new is opt-in, receiving the suggestion
to continue-as-new-onto-new-target-version should be opt-in as well. If
people are forced to check the new `suggest-continue-as-new-reasons`
field to "opt out," that is unsafe, because inevitably some people will
forget to do so or misunderstand, and then get hit by this unexpected
footgun.
Much safer and still ergonomical to let upgrade-on-can be opt-in on both
fronts, as proposed here. With this change, the people who are currently
doing `if workflow_info.suggestContinueAsNew{ do continue-as-new }`
won't see any change in semantics, regardless of their versioning
behavior.
People who consciously know that they want to do upgrade-on-can /
Trampolining will have to change their CaN options anyway, so it's easy
enough to teach them to pay attention to this new
`TargetWorkerDeploymentVersionChanged` flag.
<!-- Are there any breaking changes on binary or code level? -->
**I am proposing to completely remove the "new target version" reason
from the can-suggested reasons list. Some SDK PRs use it, and they
should stop. We could also simply deprecate the reason and just tell
people that it's not used anymore, but I'd prefer to flat out delete it.
Server is already patched to not send that flag by default.**
<!-- If this breaks the Server, please provide the Server PR to merge
right after this PR was merged. -->
**Server PR**
temporalio/temporal#9239
---------
Co-authored-by: Shivam Saraf <shivam.saraf@temporal.io>
|
|
||
| // Worker-Versioning related settings | ||
| EnableSuggestCaNOnNewTargetVersion dynamicconfig.BoolPropertyFnWithNamespaceFilter | ||
| EnableSendTargetVersionChanged dynamicconfig.BoolPropertyFnWithNamespaceFilter |
There was a problem hiding this comment.
EnableSuggestCaNOnNewTargetVersion config is now dead code
Low Severity
EnableSuggestCaNOnNewTargetVersion is still defined, initialized, and registered as a dynamic config, but the only business logic that referenced it (in workflow_task_state_machine.go) now uses EnableSendTargetVersionChanged instead. The old config field is never read, so setting it has no effect. An operator configuring system.enableSuggestCaNOnNewTargetVersion would expect it to do something, but it silently does nothing.
Additional Locations (2)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
Set TargetVersionChanged instead of SuggestCaN when TargetVersionChanged
temporalio/api#709
Why?
Setting SuggestContinueAsNew=true for Pinned workflows whenever their is a new Target Version available for that workflow causes Pinned workflows to hit that condition much more frequently than they expect. Users who are currently doing: if workflow_info.suggestContinueAsNew{ do continue-as-new } in their Pinned workflow code would need to change that code to protect themselves from running into an infinite-CaN-loop, because the default CaN behavior for a Pinned workflow is to stay Pinned.
We should not force users to protect themselves from such a situation.
Because upgrading on continue-as-new is opt-in, receiving the suggestion to continue-as-new-onto-new-target-version should be opt-in as well. If people are forced to check the new suggest-continue-as-new-reasons field to "opt out," that is unsafe, because inevitably some people will forget to do so or misunderstand, and then get hit by this unexpected footgun.
Much safer and still ergonomical to let upgrade-on-can be opt-in on both fronts, as proposed here. With this change, the people who are currently doing if workflow_info.suggestContinueAsNew{ do continue-as-new } won't see any change in semantics, regardless of their versioning behavior.
People who consciously know that they want to do upgrade-on-can / Trampolining will have to change their CaN options anyway, so it's easy enough to teach them to pay attention to this new TargetWorkerDeploymentVersionChanged flag.
How did you test it?
Potential risks
None
Note
Medium Risk
Touches workflow task started event generation/persistence and versioning-related signaling, which can affect worker behavior and history compatibility; changes are gated by dynamic config and covered by tests.
Overview
Stops using
SuggestContinueAsNew(and its reason tags) to signal pinned workflows that a newer target worker deployment version exists, and instead introduces an explicitTargetWorkerDeploymentVersionChangedboolean onWorkflowTaskStartedevents and persistedWorkflowExecutionInfo.Adds namespace dynamic config
EnableSendTargetVersionChanged(default on) and a new metricworkflow_target_version_changed_countemitted when this flag is set; updates the workflow task state machine, mutable state plumbing/mocks, proto/pb persistence, and functional tests accordingly. Also bumpsgo.temporal.io/apito pick up the new event attribute.Written by Cursor Bugbot for commit 3886491. This will update automatically on new commits. Configure here.