Fix ambiguities related to `chasmcallbackpb.CallbackState::request_id`
The file chasm/lib/callback/proto/v1/message.proto adds field to CallbackState:
// Request ID that added the callback.
string request_id = 9;That field is a foot gun that is used incorrectly today, and leads to bugs.
When Callback deliveries are made, we MUST send a stable ID with each deliver to be used as an idempotency key. So the callback's handler can disambiguate a new delivery from a redelivery.
However, today we set the request_id field to the user-generated request ID that was part of "whatever added the callback". (e.g. the call to StartWorkflowExecution or StartNexusOperationExecution.) However, because those request types accept multiple completion callbacks, it's possible to have multiple callbacks get routed to the same handler. And so they would have the same request ID, leading to ambiguities.
The Fix
(1) Confirm how CallbackState::request_id is used today. If it is ONLY used as the idempotency key for callback deliveries, then all is well. And we just need to fix the callers of callback.NewCallback(...) and have them create a new, server-generated request ID for each callback.
However, the "callback request ID" may be used as part of the Workflow codepath, for buffered events. (Something something, the Link is not a WorkflowEventID, but a RequestID reference, so another lookup is required.)
(2) If request_id IS in actually used for the Workflow codepath, then we will want to rename request_id to source_request_id for clarity. And introduce a new request_id or idempotency_key field, and use that for outgoing request IDs.
Source: temporalio/temporal