Skip to content

refactor(goose2): remove attachment preamble #9052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexhancock merged 2 commits into from
May 6, 2026
Merged

refactor(goose2): remove attachment preamble #9052

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9849150
refactor(goose2): remove attachment preamble
alexhancock May 6, 2026
eec6c0c
fix(goose2): align e2e tauri mock with provider inventory schema
alexhancock May 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions ui/goose2/src/features/chat/hooks/__tests__/useChat.attachments.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -93,14 +93,20 @@ 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,
personaName: 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 () => {
Expand Down Expand Up @@ -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 = [
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 4 additions & 7 deletions ui/goose2/src/features/chat/hooks/useChat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Comment on lines +233 to +234

@chatgpt-codex-connector chatgpt-codex-connector Bot May 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve non-empty prompt when attachments lack absolute paths

If a user sends only non-image attachments that have no path (the pathless browser-file case covered by tests) and no message text, appendAttachmentPaths(text.trim(), attachments) returns "" and acpPrompt stays empty. The earlier guard still allows this send because hasAttachments is true, so acpSendMessage receives an empty text block, which is the same class of payload this function already special-cases for image-only sends via " ". In practice this makes attachment-only submits fail for pathless files/directories instead of reaching the model.

Useful? React with 👍 / 👎.

const tAcp = performance.now();
perfLog(
`[perf:send] ${sid} → acpSendMessage (setup took ${(tAcp - tSendStart).toFixed(1)}ms)`,
Expand Down
26 changes: 9 additions & 17 deletions ui/goose2/src/features/chat/lib/attachments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions ui/goose2/tests/e2e/fixtures/tauri-mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export function buildInitScript(options?: {
defaultModel: "claude-sonnet-4-20250514",
configured: true,
providerType: "Preferred",
category: "model",
configKeys: [],
setupSteps: [],
supportsRefresh: true,
Expand All @@ -72,6 +73,7 @@ export function buildInitScript(options?: {
defaultModel: "gpt-4.1",
configured: true,
providerType: "Preferred",
category: "model",
configKeys: [],
setupSteps: [],
supportsRefresh: true,
Expand Down Expand Up @@ -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":
Expand Down
Loading