Add standalone activity dispatch hook - #11798
Merged
Merged
Conversation
Contributor
|
Claude finished @fretz12's task in 3m 53s —— View job Reviewed 5 files, 6 findings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
dandavison
reviewed
Sep 1, 2026
| ctx context.Context, | ||
| request *matchingservice.AddActivityTaskRequest, | ||
| ) error { | ||
| _, err := h.opts.MatchingClient.AddActivityTask(ctx, request) |
Contributor
There was a problem hiding this comment.
I don't think this 1-line helper function improves anything, right?
|
|
||
| return &activitypb.ActivityDispatchTask{ | ||
| Stamp: a.LastAttempt.Get(ctx).GetStamp(), | ||
| Stamp: attempt.GetStamp(), |
Contributor
There was a problem hiding this comment.
My understanding is that these changes are fixing a bug wherein we were classifying Resets as Retries. This bit LGTM.
| if err != nil { | ||
| return err | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Can you add a comment here explaining why this task execute function needs this hook machinery instead of just doing its job directly?
prathyushpv
approved these changes
Sep 3, 2026
fretz12
marked this pull request as draft
September 3, 2026 16:39
fretz12
marked this pull request as ready for review
September 3, 2026 16:39
Contributor
|
Claude finished @fretz12's task in 4m 53s —— View job Reviewed 5 files, 4 findings. |
yiminc
approved these changes
Sep 3, 2026
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
4 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?
Why?
Consumers need a reliable way to classify and intercept standalone activity dispatches. Reset attempt 1 was previously classified as a retry, and processing before CHASM validation could include stale tasks replaced by reset, update, pause, or unpause operations.
These changes ensure dispatch reasons reflect the actual attempt and allow integrations to act only on valid dispatches.
How did you test it?
Potential risks
An incorrectly implemented dispatch hook could block, drop, or duplicate an activity dispatch.
Hook errors now propagate through task processing and may cause the dispatch task to retry.
Consumers relying on the previous reset dispatch classification may observe reset attempt 1 changing from RETRY to IMMEDIATE.
When no hook is configured, dispatch behavior remains unchanged.