Repository navigation
fix(schema): stop Message.Format from modifying the template's media parts - #1374
Open
gu-feng418 wants to merge 2 commits into
Open
gu-feng418 wants to merge 2 commits into
gu-feng418 wants to merge 2 commits into
Conversation
…parts formatMultiContent and formatUserInputMultiContent allocate a new slice but copy the elements by value, so the media fields inside them remain pointers shared with the original message. Assigning through those pointers mutated the template and made concurrent Format calls on one template race. Copy each media struct, render into the copy, and store the new pointer instead, matching what AgenticMessage.Format already does.
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
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 type of PR is this?
fix
Check the PR title.
(Optional) Translate the PR title into Chinese.
fix(schema): 修复 Message.Format 会改写模板消息多模态部分的问题
(Optional) More detailed description for this PR(en: English/zh: Chinese).
en:
Message.Formatbuildscopied := *mand thenmake(...)+copy(...)forMultiContent/UserInputMultiContent, so the slice itself is new but the media fields inside each element (ImageURL/AudioURL/VideoURL/FileURLonChatMessagePart, andImage/Audio/Video/FileonMessageInputPart) are still the same pointers as the template's. The rendered values were then written through those shared pointers:Two consequences, both reported in #1318:
prompt.FromMessageskeeps the same*schema.Messageand callsFormaton it), so concurrentFormatcalls write the same structs from different goroutines.The fix clones each media struct before writing, then stores the new pointer, i.e. the same pattern
AgenticMessage.Formatalready uses viaformatUserInputText/formatUserInputImage/ ...:Extrainside the cloned structs is still a shared map, which is unchanged behaviour and matches the shallow clone thatAgenticMessage.Formatperforms today;Formatnever writes into it.Text parts (
Content,ChatMessagePart.Text,MessageInputPart.Text) are value fields on the already-copied structs, so they were never affected and are untouched here.zh(optional):
Message.Format里copied := *m以及MultiContent/UserInputMultiContent的make + copy都只是浅拷贝:切片本身是新的,但每个元素里的多模态字段(ChatMessagePart的ImageURL/AudioURL/VideoURL/FileURL,MessageInputPart的Image/Audio/Video/File)仍然指向模板里的同一个对象。渲染结果通过这些共享指针写回去,导致:prompt.FromMessages保存同一个*schema.Message并对其调用Format),并发调用Format会在不同 goroutine 里写同一个结构体。修复方式是先拷贝指针指向的结构体,写入副本后再把新指针放回切片,与
AgenticMessage.Format中formatUserInputText/formatUserInputImage等的做法一致。Format内部各结构体的Extramap 仍是共享的(属于浅拷贝,和AgenticMessage.Format现状一致),Format不会写入它,因此本次未改动。文本字段是值类型,本来就不受影响。(Optional) Which issue(s) this PR fixes:
Fixes #1318
(optional) The PR that updates user documentation:
Not needed — this restores the documented behaviour of
Format("returns the messages after rendering"), it does not change any API.Testing
go test ./schema/ -count=1passes with the fix, as do the packages that callFormat:./components/prompt/,./compose/,./adk/,./schema/claude|gemini|openai.go vet ./schema/andgofmton the changed files are clean.New test
TestFormatDoesNotMutateTemplateMediaPartscovers the fourMultiContentmedia types, the fourUserInputMultiContenttypes (bothURLandBase64Data), the issue's "format twice with different values" repro, and concurrentFormaton one template. It fails on currentmain:The existing
TestFormatMultiContent/TestFormatUserInputMultiContentstill pass unchanged, including theassert.Equal(t, in, out)case for nil nested pointers.Two notes on what I could not run on this machine (Windows, no gcc):
go test -raceis unavailable (-racerequires cgo), so the concurrency subtest only checks the observable end state rather than being race-detector-verified. The reporter'sDATA RACEtraces atschema/message.go:717and:772point at exactly the two lines changed here.gofmt -l schema/message_test.goflags a pre-existing trailing-whitespace line inTestConcatToolResults(~line 1859), unrelated to this change; I left it alone to keep the diff minimal.