fix(core): preserve prompt attachment order (#35921)

packages/core/src/session/runner/to-llm-message.ts

@@ -21,45 +21,50 @@ const media = (file: FileAttachment): ContentPart => ({
   metadata: file.description === undefined ? undefined : { description: file.description },
 })
 
-const textAttachment = (file: FileAttachment) =>
-  Message.make({
-    role: "user",
-    content: [
-      `Attached file: ${file.name ?? (file.source.type === "uri" ? file.source.uri : "inline attachment")}`,
-      file.description === undefined ? undefined : `Description: ${file.description}`,
-      "",
-      Buffer.from(file.data, "base64").toString("utf8"),
-    ]
-      .filter((line): line is string => line !== undefined)
-      .join("\n"),
-    metadata: {
-      attachment: {
-        source: file.source,
-        name: file.name,
-        description: file.description,
-      },
+const textAttachment = (file: FileAttachment): ContentPart => ({
+  type: "text",
+  text: `\n\n${[
+    `Attached file: ${file.name ?? (file.source.type === "uri" ? file.source.uri : "inline attachment")}`,
+    file.description === undefined ? undefined : `Description: ${file.description}`,
+    "",
+    Buffer.from(file.data, "base64").toString("utf8"),
+  ]
+    .filter((line): line is string => line !== undefined)
+    .join("\n")}`,
+  metadata: {
+    attachment: {
+      source: file.source,
+      name: file.name,
+      description: file.description,
     },
-  })
+  },
+})
 
-const directoryAttachment = (file: FileAttachment) =>
-  Message.make({
-    role: "user",
-    content: [
-      `Attached directory: ${file.name ?? (file.source.type === "uri" ? file.source.uri : "directory")}`,
-      file.description === undefined ? undefined : `Description: ${file.description}`,
-      file.data.length === 0 ? undefined : "",
-      file.data.length === 0 ? undefined : Buffer.from(file.data, "base64").toString("utf8"),
-    ]
-      .filter((line): line is string => line !== undefined)
-      .join("\n"),
-    metadata: {
-      attachment: {
-        source: file.source,
-        name: file.name,
-        description: file.description,
-      },
+const directoryAttachment = (file: FileAttachment): ContentPart => ({
+  type: "text",
+  text: `\n\n${[
+    `Attached directory: ${file.name ?? (file.source.type === "uri" ? file.source.uri : "directory")}`,
+    file.description === undefined ? undefined : `Description: ${file.description}`,
+    file.data.length === 0 ? undefined : "",
+    file.data.length === 0 ? undefined : Buffer.from(file.data, "base64").toString("utf8"),
+  ]
+    .filter((line): line is string => line !== undefined)
+    .join("\n")}`,
+  metadata: {
+    attachment: {
+      source: file.source,
+      name: file.name,
+      description: file.description,
     },
-  })
+  },
+})
+
+const attachmentContent = (file: FileAttachment): ContentPart[] => {
+  if (file.mime === "text/plain") return [textAttachment(file)]
+  if (file.mime === "application/x-directory") return [directoryAttachment(file)]
+  if (imageMimes.has(file.mime)) return [media(file)]
+  return []
+}
 
 const decodeToolInput = Schema.decodeUnknownOption(Schema.UnknownFromJsonString)
 
@@ -174,17 +179,16 @@ function toLLMMessage(message: SessionMessage.Info, model: ModelV2.Ref, provider
     case "model-switched":
       return []
     case "user":
-      const files = message.files ?? []
+      const content = [
+        ...(message.text === "" ? [] : [Message.text(message.text)]),
+        ...(message.files ?? []).flatMap(attachmentContent),
+      ]
+      if (content.length === 0) return []
       return [
-        ...files.filter((file) => file.mime === "text/plain").map(textAttachment),
-        ...files.filter((file) => file.mime === "application/x-directory").map(directoryAttachment),
         Message.make({
           id: message.id,
           role: "user",
-          content: [
-            { type: "text", text: message.text },
-            ...files.filter((file) => imageMimes.has(file.mime)).map(media),
-          ],
+          content,
           metadata: {
             ...message.metadata,
             ...(message.agents?.length ? { agents: message.agents } : {}),

packages/core/test/session-runner-message.test.ts

@@ -144,7 +144,7 @@ Recent work
     ])
   })
 
-  test("lowers text attachments as separate user messages", () => {
+  test("lowers text attachments after the prompt in one user message", () => {
     const file = FileAttachment.make({
       data: Base64.make(Buffer.from("export const value = 1").toString("base64")),
       mime: "text/plain",
@@ -164,21 +164,18 @@ Recent work
       model,
     )
 
-    expect(messages).toHaveLength(2)
+    expect(messages).toHaveLength(1)
     expect(messages[0]).toMatchObject({
-      role: "user",
-      content: [
-        {
-          type: "text",
-          text: "Attached file: main.ts\n\nexport const value = 1",
-        },
-      ],
-      metadata: { attachment: { source: file.source, name: "main.ts" } },
-    })
-    expect(messages[1]).toMatchObject({
       id: id("user-text-file"),
       role: "user",
-      content: [{ type: "text", text: "Review this file" }],
+      content: [
+        { type: "text", text: "Review this file" },
+        {
+          type: "text",
+          text: "\n\nAttached file: main.ts\n\nexport const value = 1",
+          metadata: { attachment: { source: file.source, name: "main.ts" } },
+        },
+      ],
     })
   })
 
@@ -203,10 +200,11 @@ Recent work
       model,
     )
 
-    expect(messages[0]?.content).toEqual([
+    expect(messages[0]?.content).toMatchObject([
+      { type: "text", text: "Review this file" },
       {
         type: "text",
-        text: "Attached file: inline.txt\n\ninline content",
+        text: "\n\nAttached file: inline.txt\n\ninline content",
       },
     ])
   })
@@ -231,13 +229,79 @@ Recent work
       model,
     )
 
-    expect(messages).toHaveLength(2)
+    expect(messages).toHaveLength(1)
     expect(messages[0]).toMatchObject({
+      id: id("user-directory"),
       role: "user",
-      content: [{ type: "text", text: "Attached directory: src/\n\nlib/\nindex.ts" }],
-      metadata: { attachment: { source: directory.source, name: "src/" } },
+      content: [
+        { type: "text", text: "Review this directory" },
+        {
+          type: "text",
+          text: "\n\nAttached directory: src/\n\nlib/\nindex.ts",
+          metadata: { attachment: { source: directory.source, name: "src/" } },
+        },
+      ],
     })
-    expect(messages[1]?.content).toEqual([{ type: "text", text: "Review this directory" }])
+  })
+
+  test("preserves attachment order after the prompt", () => {
+    const messages = toLLMMessages(
+      [
+        SessionMessage.User.make({
+          id: id("user-mixed-files"),
+          type: "user",
+          text: "Review these attachments",
+          files: [
+            FileAttachment.make({
+              data: Base64.make(Buffer.from("index.ts").toString("base64")),
+              mime: "application/x-directory",
+              source: { type: "uri", uri: "file:///project/src" },
+              name: "src/",
+            }),
+            FileAttachment.make({
+              data: Base64.make(Buffer.from("export const value = 1").toString("base64")),
+              mime: "text/plain",
+              source: { type: "uri", uri: "file:///project/main.ts" },
+              name: "main.ts",
+            }),
+          ],
+          time: { created },
+        }),
+      ],
+      model,
+    )
+
+    expect(messages).toHaveLength(1)
+    expect(messages[0]?.content.map((part) => (part.type === "text" ? part.text : part.type))).toEqual([
+      "Review these attachments",
+      "\n\nAttached directory: src/\n\nindex.ts",
+      "\n\nAttached file: main.ts\n\nexport const value = 1",
+    ])
+  })
+
+  test("omits empty prompt text before an attachment", () => {
+    const messages = toLLMMessages(
+      [
+        SessionMessage.User.make({
+          id: id("user-attachment-only"),
+          type: "user",
+          text: "",
+          files: [
+            FileAttachment.make({
+              data: Base64.make(Buffer.from("index.ts").toString("base64")),
+              mime: "application/x-directory",
+              source: { type: "uri", uri: "file:///project/src" },
+              name: "src/",
+            }),
+          ],
+          time: { created },
+        }),
+      ],
+      model,
+    )
+
+    expect(messages).toHaveLength(1)
+    expect(messages[0]?.content).toMatchObject([{ type: "text", text: "\n\nAttached directory: src/\n\nindex.ts" }])
   })
 
   test("uses materialized image data as provider media and drops unsupported attachments", () => {