Avoid duplicate codec calls for System Nexus history - #1206
chaptersix wants to merge 7 commits into
Conversation
|
not merging until there's a tagged api version |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0645eae636
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| messageDescriptor, err := protoregistry.GlobalTypes.FindMessageByName(messageType) | ||
| if err != nil { | ||
| return fmt.Errorf("system nexus payload references unknown message type %q: %w", messageType, err) |
There was a problem hiding this comment.
When a newer server records a marked system operation whose protobuf type is absent from this CLI's linked API version, FindMessageByName fails and aborts the entire workflow show --detailed command. The previous registry-based implementation treated unknown operations as a no-op, allowing the remaining history to render; an unavailable type should likewise skip the optional unwrapped view rather than make history unusable.
Useful? React with 👍 / 👎.
1 participant
Related issues
Related to #1193.
Context:
What changed?
CLI #1017 explicitly called the remote codec while rendering detailed System Nexus history. With api-go #297, the gRPC interceptor now finds and decodes the nested payloads itself. Keeping both paths can send the same payloads to the codec server twice.
This change:
v1.63.6release, which includes the marked System Nexus traversal;v1.33.0-164.0, which includes the server marker from temporal#10948, and updates the root module to its required Go 1.27;v1.49.0, including the SDK field rename toStaticDetailswhile preserving the CLI flag;messageTypemetadata instead of maintaining a hard-coded operation/type registry; andworkflow showboth renders the decoded nested value and makes exactly one/decoderequest.Raw JSON output remains unchanged. Because this feature has not been enabled, this change intentionally does not preserve compatibility with the earlier unmarked envelope implementation.
This does not solve the whole serialization issue in #1193. Payload groups elsewhere in history are still visited serially: SDK Go's
PayloadCodecGRPCClientInterceptorOptionsdoes not currently expose and pass api-go'sVisitPayloadsOptions.ConcurrencyLimit. A follow-up SDK Go change is still required to make those codec-server requests concurrent.Checklist
The test server uses the latest non-RC Cloud development tag,
v1.33.0-164.0, because it includes the System Nexus marker absent from stable OSSv1.32.0.How did you test this
The shared-server end-to-end test uses an HTTP codec endpoint and checks that detailed
workflow showrenders the decoded nested System Nexus value while making exactly one/decoderequest. Unit coverage checks visitor decoding and local unwrapping, resolution throughmessageType, completed events linked to prior scheduled events, and no-op handling for nil payloads, unknown endpoints, and completed events without a prior scheduled event.