fix(responses): deep-copy author, recipient, encrypted_content in message copies - #4612
Conversation
168284a to
1dd4107
Compare
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughTwo deep-copy functions in ChangesDeep-copy safety for response message fields
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Address review feedback on maximhq#4612: collapse the verbose local-variable pointer copies (Phase, Signature, EncryptedContent) to the Ptr helper.
…sage copies PR maximhq#4609 added Author/Recipient on ResponsesMessage and EncryptedContent on ResponsesMessageContentBlock to preserve Codex multi_agent_v2 collab_tool_call fields through bifrost's JSON decode/re-encode path. The deep-copy helpers were not updated to mirror them, so any accumulator that deep-copies a message (every streamed output item, and request-side copies) silently drops these fields. As the multi-agent protocol evolves to return collab_tool_call output items, the same stripping reappears on the response side. Mirror the new fields in both deep-copy implementations: - framework/streaming/responses.go: deepCopyResponsesMessage + deepCopyResponsesMessageContentBlock (response/streaming accumulator path) - core/schemas/utils.go: DeepCopyResponsesMessage + deepCopyResponsesMessageContentBlock (request-side copy) Author/Recipient (json.RawMessage = []byte) are copied via append to a fresh slice; EncryptedContent and the sibling reasoning field Signature (*string) are value-copied. Also add the missing Phase (*string) copy in the core helper for parity with the streaming helper.
Address review feedback on maximhq#4612: collapse the verbose local-variable pointer copies (Phase, Signature, EncryptedContent) to the Ptr helper.
Replace the Ptr helper with the new(string) + value-assign form for the Phase, Signature, and EncryptedContent pointer copies in both deep-copy helpers, per review feedback.
19c1022 to
5b35c17
Compare
…sage copies (#4612) * fix(responses): deep-copy author, recipient, encrypted_content in message copies PR #4609 added Author/Recipient on ResponsesMessage and EncryptedContent on ResponsesMessageContentBlock to preserve Codex multi_agent_v2 collab_tool_call fields through bifrost's JSON decode/re-encode path. The deep-copy helpers were not updated to mirror them, so any accumulator that deep-copies a message (every streamed output item, and request-side copies) silently drops these fields. As the multi-agent protocol evolves to return collab_tool_call output items, the same stripping reappears on the response side. Mirror the new fields in both deep-copy implementations: - framework/streaming/responses.go: deepCopyResponsesMessage + deepCopyResponsesMessageContentBlock (response/streaming accumulator path) - core/schemas/utils.go: DeepCopyResponsesMessage + deepCopyResponsesMessageContentBlock (request-side copy) Author/Recipient (json.RawMessage = []byte) are copied via append to a fresh slice; EncryptedContent and the sibling reasoning field Signature (*string) are value-copied. Also add the missing Phase (*string) copy in the core helper for parity with the streaming helper. * refactor(responses): use Ptr helper for pointer field copies Address review feedback on #4612: collapse the verbose local-variable pointer copies (Phase, Signature, EncryptedContent) to the Ptr helper. * refactor(responses): use new(string) for pointer field copies Replace the Ptr helper with the new(string) + value-assign form for the Phase, Signature, and EncryptedContent pointer copies in both deep-copy helpers, per review feedback. --------- Co-authored-by: Akshay Deo <akshay@akshaydeo.com>
Fixes #4608
Follow-up to #4609
#4609 added
Author/RecipienttoResponsesMessageandEncryptedContenttoResponsesMessageContentBlockso Codexmulti_agent_v2collab_tool_callfields survive bifrost's JSON decode/re-encode path. That fix covered the request decode/encode path but not the deep-copy helpers.Problem
Both deep-copy implementations copy
ResponsesMessage/ResponsesMessageContentBlockfield-by-field and were not updated for the new fields, so they get silently stripped whenever a message is deep-copied:framework/streaming/responses.go—deepCopyResponsesMessage+deepCopyResponsesMessageContentBlockrun for every streamed output item. Any plugin that inspects or re-serializes the accumulated streaming response seesAuthor/Recipient/EncryptedContentabsent. As the multi-agent protocol evolves to returncollab_tool_calloutput items, the same stripping bug reappears on the response side.core/schemas/utils.go—DeepCopyResponsesMessage+deepCopyResponsesMessageContentBlockhave the identical gap on the request-side copy.Fix
Mirror the new schema fields in both deep-copy implementations:
Authorjson.RawMessage([]byte)append(json.RawMessage(nil), src...)(nil-guarded, fresh backing array)Recipientjson.RawMessage([]byte)EncryptedContent*stringSignature*stringAlso added the missing
Phase(*string) copy in thecore/schemas/utils.gohelper for parity with the streaming helper.Testing
go build ./schemas/,go vet ./schemas/, andgo test ./schemas/pass.go vet ./streaming/passes.