Add additional standalone activity request validations#9285
Add additional standalone activity request validations#9285fretz12 merged 11 commits intotemporalio:mainfrom
Conversation
…ments' into fredtzeng/saa-validation-improvements
There was a problem hiding this comment.
Pull request overview
Adds additional request validation coverage for standalone activity APIs, aligning frontend behavior with stricter ID/size checks and reorganizing functional tests into grouped subtests.
Changes:
- Added/expanded frontend request validations for Start/Terminate/RequestCancel standalone activity APIs (ID length, identity, run ID format, blob/reason/input size).
- Refactored/centralized validation helpers in
validator.go(including renaming/refactoring blob-size validation). - Restructured standalone activity functional tests into grouped validation subtests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/standalone_activity_test.go | Adds new validation-focused subtests for Start/RequestCancel/Terminate and refactors existing validation tests into grouped subtests. |
| chasm/lib/activity/validator.go | Introduces new request validators for cancel/terminate, refactors blob-size validation helper, and makes poll validation unexported. |
| chasm/lib/activity/frontend.go | Wires new validators into frontend handlers and adjusts error messages to be more specific/consistent. |
| chasm/lib/activity/handler.go | Updates poll handler to match updated response builder signature (no error return). |
| chasm/lib/activity/activity.go | Simplifies response/info builders by removing unused error returns and updating call sites accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
tests/standalone_activity_test.go:3349
- This subtest discards the DescribeActivityExecution response by assigning it to
_and then asserts on the outerdescribeRespvariable (from a previous call). Capture the returned response into a local variable and assert against that, otherwise this test can pass even if the "NoCallerDeadline" behavior regresses.
_, err = s.FrontendClient().DescribeActivityExecution(ctx, &workflowservice.DescribeActivityExecutionRequest{
Namespace: s.Namespace().String(),
ActivityId: activityID,
RunId: startResp.RunId,
LongPollToken: describeResp.LongPollToken,
})
require.NoError(t, err)
require.Empty(t, describeResp.GetInfo())
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dandavison
left a comment
dandavison
left a comment
There was a problem hiding this comment.
LGTM, made a few suggestions
2 participants
What changed?
Add additional standalone activity request validations. Renamed blob error tags to be in line with existing workflow tags. Code cleanup. Refactor tests into grouped subtests.
Why?
After an audit of the standalone activity frontend validations, this PR provides additional coverage of missing corners.
How did you test it?