From 0ed8c66755c5f47d6c1681f0d4ed1e7b6cabce99 Mon Sep 17 00:00:00 2001 From: Alex Hancock Date: Wed, 6 May 2026 13:33:25 -0400 Subject: [PATCH] refactor(goose2): remove attachment preamble (#9052) --- .../__tests__/useChat.attachments.test.ts | 32 ++++++++++--------- ui/goose2/src/features/chat/hooks/useChat.ts | 11 +++---- .../src/features/chat/lib/attachments.ts | 26 ++++++--------- ui/goose2/tests/e2e/fixtures/tauri-mock.ts | 4 +++ 4 files changed, 34 insertions(+), 39 deletions(-) diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useChat.attachments.test.ts b/ui/goose2/src/features/chat/hooks/__tests__/useChat.attachments.test.ts index a4f99b3d..573bf4ae 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/useChat.attachments.test.ts +++ b/ui/goose2/src/features/chat/hooks/__tests__/useChat.attachments.test.ts @@ -50,7 +50,7 @@ describe("useChat attachments", () => { mockAcpSetModel.mockResolvedValue(undefined); }); - it("stores non-image attachments in metadata and prepends path references to the prompt", async () => { + it("stores non-image attachments in metadata and appends absolute paths to the prompt", async () => { const { result } = renderHook(() => useChat("session-1")); const attachments = [ { @@ -93,7 +93,7 @@ describe("useChat attachments", () => { ]); expect(mockAcpSendMessage).toHaveBeenCalledWith( "session-1", - "Attached items:\n- [file] /tmp/report.pdf\n- [directory] /tmp/screenshots\nPlease review these", + "Please review these /tmp/report.pdf /tmp/screenshots", { systemPrompt: undefined, personaId: undefined, @@ -101,6 +101,12 @@ describe("useChat attachments", () => { images: undefined, }, ); + + // The bubble's displayed text must remain the raw user input — appended + // paths are wire-only so they don't clutter the rendered message. + expect(message.content).toEqual([ + { type: "text", text: "Please review these" }, + ]); }); it("keeps image attachments in ACP images while preserving path metadata", async () => { @@ -142,19 +148,15 @@ describe("useChat attachments", () => { }, }, ]); - expect(mockAcpSendMessage).toHaveBeenCalledWith( - "session-1", - "Attached items:\n- [image] diagram.png (image attached)\n ", - { - systemPrompt: undefined, - personaId: undefined, - personaName: undefined, - images: [["abc123", "image/png"]], - }, - ); + expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", " ", { + systemPrompt: undefined, + personaId: undefined, + personaName: undefined, + images: [["abc123", "image/png"]], + }); }); - it("includes image attachments in the prompt summary for mixed sends", async () => { + it("includes file/directory paths in the prompt for mixed sends; images flow through ACP image content blocks only", async () => { const { result } = renderHook(() => useChat("session-1")); const attachments = [ { @@ -191,7 +193,7 @@ describe("useChat attachments", () => { expect(mockAcpSendMessage).toHaveBeenCalledWith( "session-1", - "Attached items:\n- [file] /tmp/mobile-confirmation.html\n- [directory] /tmp/neighborhood block\n- [image] Screenshot 2026-04-09 at 1.25.32 PM.png (image attached)\ncan you see the attachments i attached?", + "can you see the attachments i attached? /tmp/mobile-confirmation.html /tmp/neighborhood block", { systemPrompt: undefined, personaId: undefined, @@ -231,7 +233,7 @@ describe("useChat attachments", () => { ]); expect(mockAcpSendMessage).toHaveBeenCalledWith( "session-1", - "Attached items:\n- [file] report.pdf\nPlease review this", + "Please review this", { systemPrompt: undefined, personaId: undefined, diff --git a/ui/goose2/src/features/chat/hooks/useChat.ts b/ui/goose2/src/features/chat/hooks/useChat.ts index 409c2c52..1d7829cd 100644 --- a/ui/goose2/src/features/chat/hooks/useChat.ts +++ b/ui/goose2/src/features/chat/hooks/useChat.ts @@ -21,8 +21,8 @@ import { import { findLastIndex } from "@/shared/lib/arrays"; import { perfLog } from "@/shared/lib/perfLog"; import { + appendAttachmentPaths, buildAcpImages, - buildAttachmentPromptPreamble, buildMessageAttachments, } from "../lib/attachments"; import { sanitizeReplayMessages } from "../lib/replaySanitizer"; @@ -229,12 +229,9 @@ export function useChat( await options?.ensurePrepared?.(effectivePersonaInfo?.id); store.setChatState(sessionId, "streaming"); - // When images are present with no text, pass a single space so the ACP - // driver doesn't send an empty text content block that goose rejects. - const attachmentPromptPreamble = - buildAttachmentPromptPreamble(attachments); - const promptBody = text.trim() || (images?.length ? " " : text); - const acpPrompt = `${attachmentPromptPreamble}${promptBody}`; + const promptWithPaths = appendAttachmentPaths(text.trim(), attachments); + const acpPrompt = + promptWithPaths || (images?.length ? " " : promptWithPaths); const tAcp = performance.now(); perfLog( `[perf:send] ${sid} → acpSendMessage (setup took ${(tAcp - tSendStart).toFixed(1)}ms)`, diff --git a/ui/goose2/src/features/chat/lib/attachments.ts b/ui/goose2/src/features/chat/lib/attachments.ts index a69a3836..0ee629ad 100644 --- a/ui/goose2/src/features/chat/lib/attachments.ts +++ b/ui/goose2/src/features/chat/lib/attachments.ts @@ -3,28 +3,20 @@ import type { MessageAttachment, } from "@/shared/types/messages"; -function formatAttachmentReference(attachment: ChatAttachmentDraft): string { - const location = - attachment.kind === "image" - ? `${attachment.name} (image attached)` - : (attachment.path ?? attachment.name); - return `- [${attachment.kind}] ${location}`; -} - -export function buildAttachmentPromptPreamble( +export function appendAttachmentPaths( + text: string, attachments: ChatAttachmentDraft[] | undefined, ): string { - const referencedAttachments = attachments ?? []; + const paths = (attachments ?? []) + .filter((attachment) => attachment.kind !== "image" && attachment.path) + .map((attachment) => attachment.path as string); - if (referencedAttachments.length === 0) { - return ""; + if (paths.length === 0) { + return text; } - return [ - "Attached items:", - ...referencedAttachments.map(formatAttachmentReference), - "", - ].join("\n"); + const joined = paths.join(" "); + return text ? `${text} ${joined}` : joined; } export function buildMessageAttachments( diff --git a/ui/goose2/tests/e2e/fixtures/tauri-mock.ts b/ui/goose2/tests/e2e/fixtures/tauri-mock.ts index d5aa353b..5d629a35 100644 --- a/ui/goose2/tests/e2e/fixtures/tauri-mock.ts +++ b/ui/goose2/tests/e2e/fixtures/tauri-mock.ts @@ -47,6 +47,7 @@ export function buildInitScript(options?: { defaultModel: "claude-sonnet-4-20250514", configured: true, providerType: "Preferred", + category: "model", configKeys: [], setupSteps: [], supportsRefresh: true, @@ -72,6 +73,7 @@ export function buildInitScript(options?: { defaultModel: "gpt-4.1", configured: true, providerType: "Preferred", + category: "model", configKeys: [], setupSteps: [], supportsRefresh: true, @@ -203,6 +205,8 @@ export function buildInitScript(options?: { } case "_goose/providers/list": return jsonRpcResult(message.id, { entries: PROVIDER_INVENTORY }); + case "_goose/providers/setup/catalog/list": + return jsonRpcResult(message.id, { providers: [] }); case "_goose/providers/inventory/refresh": return jsonRpcResult(message.id, { started: [], skipped: [] }); case "_goose/defaults/read":