fix(signal): inline image and PDF attachments as base64#2529
Open
brentkearney wants to merge 1 commit into
Open
fix(signal): inline image and PDF attachments as base64#2529brentkearney wants to merge 1 commit into
brentkearney wants to merge 1 commit into
Conversation
Bug: signal.ts was emitting `{path: <host-path>}` for image attachments
in the structured `attachments` array, plus a `[Image: <host-path>]`
line in the text body. The host never mounts signal-cli's attachments
directory (`~/.local/share/signal-cli/attachments`) into agent
containers, so the agent received a path that didn't resolve from
inside the container. Vision-capable models couldn't see images at
all; PDFs were dropped entirely (no handler for `application/pdf`).
Fix: read each attachment file off disk and inline it as base64 in
the structured `attachments` array (`{data, name, type, contentType,
size}`). The host's existing
session-manager.extractAttachmentFiles saves base64 attachments to
`<sessionDir>/inbox/<msgId>/<filename>` (= `/workspace/inbox/...`
inside the container) and rewrites the entry as `localPath`. The
container formatter renders `[image|document: <name> — saved to
/workspace/inbox/.../...]`. The agent can then Read the file.
Coverage:
- Images (existing flow): now uses base64 + correct container path.
Drops the now-redundant `[Image: <host-path>]` text line.
- PDFs (new): same handler via a shared `inlineAttachment` helper.
Preserves the original filename when signal-cli supplies one
(typical for documents) — falls back to `<id>.<ext>` otherwise.
- Voice attachments: unaffected. They're transcribed on the host
before being embedded in the text content, no file is passed to
the container.
Tests: existing image test updated to assert the new structured
attachment shape and an empty text body. New PDF test added covering
filename preservation and `type: 'document'`. Both pass.
Verified live on a downstream install: signal images and PDFs now
both render correctly to the agent.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2528.
Type of Change
Problem
src/channels/signal.tswas emitting{path: <host-path>}for image attachments in the structuredattachmentsarray, plus a[Image: <host-path>]line in the text body. The host never mounts signal-cli's attachments directory (~/.local/share/signal-cli/attachments) into agent containers, so the agent received a path that didn't resolve inside the container. Vision-capable models couldn't see images; PDFs were dropped entirely (no handler forapplication/pdf).Full root cause and reproduction in the linked issue.
Solution
The host already has a complete pipeline:
session-manager.extractAttachmentFilessaves anyattachments[].database64 to<sessionDir>/inbox/<msgId>/<filename>(=/workspace/inbox/...in container) and rewrites the entry aslocalPath. The container formatter renders[image|document: <name> — saved to /workspace/inbox/.../...].signal.ts just needs to read each attachment off disk and inline it as base64 in the
attachmentsarray. No host changes, no new mount, no new dependency.Changes
src/channels/signal.ts: extract a sharedinlineAttachmenthelper, call it once forimage/*attachments and once forapplication/pdfattachments. Helper reads file, base64-encodes, pushes{data, name, type, contentType, size}. Preserves the originalfilenamewhen signal-cli supplies one (typical for documents); synthesizes<id>.<ext>otherwise. Drop the now-redundant[Image: <host-path>]line — the formatter renders its own ref line from the structured attachment.src/channels/signal.test.ts: existing image test updated to assert the new structured shape (data,name: '<id>.jpeg',type: 'image') and an empty text body. New PDF test covers filename preservation andtype: 'document'. Both tests stage a real file in tmpdir and round-trip its base64 through the adapter. 38/38 tests pass.Voice attachments
Unaffected. They're transcribed on the host (via WHISPER_BIN / OPENAI_API_KEY) before being embedded in the text content, so no file is passed to the container.
Verified
Running live on a downstream install since 2026-05-17. Confirmed end-to-end:
🤖 Generated with Claude Code