refactor(goose2): remove attachment preamble (#9052)

This commit is contained in:
Alex Hancock
2026-05-06 13:33:25 -04:00
committed by GitHub
parent 2ee9687741
commit 0ed8c66755
4 changed files with 34 additions and 39 deletions
@@ -50,7 +50,7 @@ describe("useChat attachments", () => {
mockAcpSetModel.mockResolvedValue(undefined); 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 { result } = renderHook(() => useChat("session-1"));
const attachments = [ const attachments = [
{ {
@@ -93,7 +93,7 @@ describe("useChat attachments", () => {
]); ]);
expect(mockAcpSendMessage).toHaveBeenCalledWith( expect(mockAcpSendMessage).toHaveBeenCalledWith(
"session-1", "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, systemPrompt: undefined,
personaId: undefined, personaId: undefined,
@@ -101,6 +101,12 @@ describe("useChat attachments", () => {
images: undefined, 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 () => { it("keeps image attachments in ACP images while preserving path metadata", async () => {
@@ -142,19 +148,15 @@ describe("useChat attachments", () => {
}, },
}, },
]); ]);
expect(mockAcpSendMessage).toHaveBeenCalledWith( expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", " ", {
"session-1", systemPrompt: undefined,
"Attached items:\n- [image] diagram.png (image attached)\n ", personaId: undefined,
{ personaName: undefined,
systemPrompt: undefined, images: [["abc123", "image/png"]],
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 { result } = renderHook(() => useChat("session-1"));
const attachments = [ const attachments = [
{ {
@@ -191,7 +193,7 @@ describe("useChat attachments", () => {
expect(mockAcpSendMessage).toHaveBeenCalledWith( expect(mockAcpSendMessage).toHaveBeenCalledWith(
"session-1", "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, systemPrompt: undefined,
personaId: undefined, personaId: undefined,
@@ -231,7 +233,7 @@ describe("useChat attachments", () => {
]); ]);
expect(mockAcpSendMessage).toHaveBeenCalledWith( expect(mockAcpSendMessage).toHaveBeenCalledWith(
"session-1", "session-1",
"Attached items:\n- [file] report.pdf\nPlease review this", "Please review this",
{ {
systemPrompt: undefined, systemPrompt: undefined,
personaId: undefined, personaId: undefined,
+4 -7
View File
@@ -21,8 +21,8 @@ import {
import { findLastIndex } from "@/shared/lib/arrays"; import { findLastIndex } from "@/shared/lib/arrays";
import { perfLog } from "@/shared/lib/perfLog"; import { perfLog } from "@/shared/lib/perfLog";
import { import {
appendAttachmentPaths,
buildAcpImages, buildAcpImages,
buildAttachmentPromptPreamble,
buildMessageAttachments, buildMessageAttachments,
} from "../lib/attachments"; } from "../lib/attachments";
import { sanitizeReplayMessages } from "../lib/replaySanitizer"; import { sanitizeReplayMessages } from "../lib/replaySanitizer";
@@ -229,12 +229,9 @@ export function useChat(
await options?.ensurePrepared?.(effectivePersonaInfo?.id); await options?.ensurePrepared?.(effectivePersonaInfo?.id);
store.setChatState(sessionId, "streaming"); store.setChatState(sessionId, "streaming");
// When images are present with no text, pass a single space so the ACP const promptWithPaths = appendAttachmentPaths(text.trim(), attachments);
// driver doesn't send an empty text content block that goose rejects. const acpPrompt =
const attachmentPromptPreamble = promptWithPaths || (images?.length ? " " : promptWithPaths);
buildAttachmentPromptPreamble(attachments);
const promptBody = text.trim() || (images?.length ? " " : text);
const acpPrompt = `${attachmentPromptPreamble}${promptBody}`;
const tAcp = performance.now(); const tAcp = performance.now();
perfLog( perfLog(
`[perf:send] ${sid} → acpSendMessage (setup took ${(tAcp - tSendStart).toFixed(1)}ms)`, `[perf:send] ${sid} → acpSendMessage (setup took ${(tAcp - tSendStart).toFixed(1)}ms)`,
+9 -17
View File
@@ -3,28 +3,20 @@ import type {
MessageAttachment, MessageAttachment,
} from "@/shared/types/messages"; } from "@/shared/types/messages";
function formatAttachmentReference(attachment: ChatAttachmentDraft): string { export function appendAttachmentPaths(
const location = text: string,
attachment.kind === "image"
? `${attachment.name} (image attached)`
: (attachment.path ?? attachment.name);
return `- [${attachment.kind}] ${location}`;
}
export function buildAttachmentPromptPreamble(
attachments: ChatAttachmentDraft[] | undefined, attachments: ChatAttachmentDraft[] | undefined,
): string { ): string {
const referencedAttachments = attachments ?? []; const paths = (attachments ?? [])
.filter((attachment) => attachment.kind !== "image" && attachment.path)
.map((attachment) => attachment.path as string);
if (referencedAttachments.length === 0) { if (paths.length === 0) {
return ""; return text;
} }
return [ const joined = paths.join(" ");
"Attached items:", return text ? `${text} ${joined}` : joined;
...referencedAttachments.map(formatAttachmentReference),
"",
].join("\n");
} }
export function buildMessageAttachments( export function buildMessageAttachments(
@@ -47,6 +47,7 @@ export function buildInitScript(options?: {
defaultModel: "claude-sonnet-4-20250514", defaultModel: "claude-sonnet-4-20250514",
configured: true, configured: true,
providerType: "Preferred", providerType: "Preferred",
category: "model",
configKeys: [], configKeys: [],
setupSteps: [], setupSteps: [],
supportsRefresh: true, supportsRefresh: true,
@@ -72,6 +73,7 @@ export function buildInitScript(options?: {
defaultModel: "gpt-4.1", defaultModel: "gpt-4.1",
configured: true, configured: true,
providerType: "Preferred", providerType: "Preferred",
category: "model",
configKeys: [], configKeys: [],
setupSteps: [], setupSteps: [],
supportsRefresh: true, supportsRefresh: true,
@@ -203,6 +205,8 @@ export function buildInitScript(options?: {
} }
case "_goose/providers/list": case "_goose/providers/list":
return jsonRpcResult(message.id, { entries: PROVIDER_INVENTORY }); return jsonRpcResult(message.id, { entries: PROVIDER_INVENTORY });
case "_goose/providers/setup/catalog/list":
return jsonRpcResult(message.id, { providers: [] });
case "_goose/providers/inventory/refresh": case "_goose/providers/inventory/refresh":
return jsonRpcResult(message.id, { started: [], skipped: [] }); return jsonRpcResult(message.id, { started: [], skipped: [] });
case "_goose/defaults/read": case "_goose/defaults/read":