Skip to content

fix(schema): stop Message.Format from modifying the template's media parts - #1374

Open
gu-feng418 wants to merge 2 commits into
cloudwego:mainfrom
gu-feng418:fix/format-mutates-template-media
Open

gu-feng418 wants to merge 2 commits into
cloudwego:mainfrom
gu-feng418:fix/format-mutates-template-media

Conversation

@gu-feng418

Copy link
Copy Markdown

What type of PR is this?

fix

Check the PR title.

  • This PR title match the format: <type>(optional scope): <description>
  • The description of this PR title is user-oriented and clear enough for others to understand.
  • Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. User docs repo

(Optional) Translate the PR title into Chinese.

fix(schema): 修复 Message.Format 会改写模板消息多模态部分的问题

(Optional) More detailed description for this PR(en: English/zh: Chinese).

en:

Message.Format builds copied := *m and then make(...)+copy(...) for MultiContent / UserInputMultiContent, so the slice itself is new but the media fields inside each element (ImageURL / AudioURL / VideoURL / FileURL on ChatMessagePart, and Image / Audio / Video / File on MessageInputPart) are still the same pointers as the template's. The rendered values were then written through those shared pointers:

copiedMC[i].ImageURL.URL = url
copiedUIMC[i].Image.URL = &url

Two consequences, both reported in #1318:

  1. Formatting modifies the template. The first call substitutes the placeholders in the template itself, so every later call renders from the first call's values instead of the original template.
  2. A prompt template is normally shared by all requests (prompt.FromMessages keeps the same *schema.Message and calls Format on it), so concurrent Format calls 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.Format already uses via formatUserInputText / formatUserInputImage / ...:

img := *mc.ImageURL
img.URL = url
copiedMC[i].ImageURL = &img

Extra inside the cloned structs is still a shared map, which is unchanged behaviour and matches the shallow clone that AgenticMessage.Format performs today; Format never 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)仍然指向模板里的同一个对象。渲染结果通过这些共享指针写回去,导致:

  1. 格式化会破坏模板本身。第一次调用就把占位符替换掉了,之后每次调用拿到的都是第一次的结果。
  2. prompt 模板通常被所有请求共用(prompt.FromMessages 保存同一个 *schema.Message 并对其调用 Format),并发调用 Format 会在不同 goroutine 里写同一个结构体。

修复方式是先拷贝指针指向的结构体,写入副本后再把新指针放回切片,与 AgenticMessage.Format 中 formatUserInputText / formatUserInputImage 等的做法一致。

Format 内部各结构体的 Extra map 仍是共享的(属于浅拷贝,和 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=1 passes with the fix, as do the packages that call Format: ./components/prompt/, ./compose/, ./adk/, ./schema/claude|gemini|openai. go vet ./schema/ and gofmt on the changed files are clean.

New test TestFormatDoesNotMutateTemplateMediaParts covers the four MultiContent media types, the four UserInputMultiContent types (both URL and Base64Data), the issue's "format twice with different values" repro, and concurrent Format on one template. It fails on current main:

--- FAIL: TestFormatDoesNotMutateTemplateMediaParts/rendering_the_same_template_twice
    expected: "http://img/2.png"
    actual  : "http://img/1.png"
    expected: "http://img/{id}.png"   // template after Format
    actual  : "http://img/1.png"

The existing TestFormatMultiContent / TestFormatUserInputMultiContent still pass unchanged, including the assert.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 -race is unavailable (-race requires cgo), so the concurrency subtest only checks the observable end state rather than being race-detector-verified. The reporter's DATA RACE traces at schema/message.go:717 and :772 point at exactly the two lines changed here.
  • gofmt -l schema/message_test.go flags a pre-existing trailing-whitespace line in TestConcatToolResults (~line 1859), unrelated to this change; I left it alone to keep the diff minimal.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] schema: Message.Format modifies the template media parts and races under concurrent use

1 participant