Skip to content

Conversation

@liorshk
Copy link
Contributor

@liorshk liorshk commented Dec 29, 2025

Problem

When a user attaches a file (e.g., a JSON config file) via an ACP client, the file displays correctly in the initial session. However, when the session is reloaded (via loadSession), the file attachment appears as plain text instead of as a proper attachment with filename and metadata.

Before this fix:

  • User attaches firebase.json → shows as attachment ✓
  • Session reload → shows as raw JSON text in the message body ✗

Root Cause

The ACP prompt function was storing text resources as plain text parts, which preserved the content for the LLM but lost the file metadata (filename, mimeType). The processMessage function (used for session replay) had no handler for file parts at all.

Solution

This PR fixes the issue with a two-part approach:

1. Store resources as file parts (agent.ts)

Text and binary resources are now stored as file parts with base64 data URLs:

case "resource": {
  const base64 = Buffer.from(resource.text, "utf-8").toString("base64")
  parts.push({
    type: "file",
    url: `data:${mime};base64,${base64}`,
    filename,
    mime,
  })
}

This preserves the filename and mimeType for proper replay.

2. Handle file parts during session replay (agent.ts)

Added handling in processMessage to replay file parts as ACP blocks:

  • Images → ACP image blocks
  • Text files → ACP resource blocks (decoded from base64)

3. Decode text-based files for LLM compatibility (message-v2.ts)

Since file parts now contain base64-encoded content, toModelMessage decodes text-based files before sending to the LLM:

if (isTextBased) {
  const text = Buffer.from(match[1], "base64").toString("utf-8")
  userMessage.parts.push({
    type: "text",
    text: `[File: ${part.filename || "file"}]\n${text}`,
  })
}

Binary files (images) are sent as file parts for multimodal models.

Alternatives Considered

  1. Store text resources as text parts (original behavior): Simpler, but loses file metadata for replay.

  2. Save attachments to disk and let LLM use read tool: Would require saving files to disk first, and the read tool doesn't work with data URLs.

  3. Store both text part and metadata separately: More complex data model changes.

The chosen approach is minimal and preserves both LLM compatibility and proper ACP replay.

Additional Fixes

  • Image filename preservation: Extracts filename from URI instead of hardcoding "image"
  • Style guide compliance: Removed else statements, replaced deprecated atob/btoa with Buffer

Testing

  • Verified file attachments display correctly on initial prompt
  • Verified file attachments replay correctly after session reload
  • Verified LLM receives decoded text content for text-based files
  • Verified images are sent as file parts for multimodal processing

noamzbr and others added 7 commits December 16, 2025 15:53
Avoid per-session SSE subscriptions that caused cross-session event pollution and duplicate events on repeated loadSession().
ACP now maintains one global event stream and forwards updates to the correct ACP session using event payload sessionID.
Adds ACP regression tests for cross-session isolation and subscription duplication.
- Preserve original filenames from URI in image blocks
- Store resource blocks as file parts instead of flattening to text
- Replay file parts as proper ACP image/resource blocks during session load
- Convert text-based files (JSON, XML, etc.) to text parts for LLM compatibility
- Keep binary files (images) as file parts for proper handling

This fixes the issue where file attachments lost their metadata (filename,
MIME type) when sessions were reloaded, causing them to appear as plain
text instead of structured attachments.
- Store ACP resources as file parts to preserve filename and mimeType
- Add file part handling in processMessage for proper session replay
- Decode text-based files for LLM compatibility in toModelMessage
- Preserve original filename from image URI instead of hardcoding 'image'
- Follow style guide: avoid else statements, use Buffer instead of atob/btoa
ACP resource blocks only support text content. Binary files like PDFs
would be decoded as UTF-8 and produce garbage. Skip them during replay
while still sending them to the LLM as file parts.
@liorshk liorshk marked this pull request as draft December 29, 2025 08:27
@liorshk
Copy link
Contributor Author

liorshk commented Dec 29, 2025

Closing in favor of #6342 which contains only the file attachment metadata fix without unrelated changes.

@liorshk liorshk closed this Dec 29, 2025
@liorshk liorshk deleted the fix/acp-file-attachments-preserve-metadata branch December 29, 2025 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants