fix(daemon): seed Claude image attachments (MUL-3907)#2877
Conversation
|
Someone is attempting to deploy a commit to the IndexLabs Team on Vercel. A member of the Team first needs to authorize it. |
37d28c6 to
e8eca27
Compare
be72043 to
89049b6
Compare
89049b6 to
f751f25
Compare
|
Rebased this Claude image attachment seeding fix onto current Conflict resolution kept newer main behavior and layered the image path on top:
Local validation:
CI is running on the new head now. |
|
CI is green on the rebased head |
|
Hey, just came across this while tracking #2400. The fix looks solid — CI is green, rebase is clean, and the author has been very responsive. Is there anything the community can do to help push this forward? Would be great to see it merged. |
Bohan-J
left a comment
There was a problem hiding this comment.
Thanks for tackling #2400 — seeding the image as a native content block up front is the right direction, and the relative-vs-absolute download auth split plus the encoding tests are clean. A few blocking issues before this can go in, though.
Blocking
1. The attachment download can't authenticate → the feature is a silent no-op
Client.DownloadAttachment (server/internal/daemon/client.go:609-611) first fetches metadata from GET /api/attachments/{id}. That route is registered under middleware.Auth + middleware.RequireWorkspaceMember (see server/cmd/server/router.go), which needs a workspace to be resolvable from the request. But:
middleware.Authonly stampsX-Workspace-IDformat_task tokens; for the daemon's PAT / cloud-PAT it setsX-User-IDonly.- The daemon client never sends
X-Workspace-Slug/X-Workspace-IDnor aworkspace_idquery param.
So ResolveWorkspaceIDFromRequest returns empty and the request is rejected with 400 "workspace_id is required" before any bytes are served. claudeInitialImages swallows that as a Warn and continues (server/internal/daemon/daemon.go:3101-3104), so no image is ever embedded — first run or resume. The unit tests pass only because they mock the endpoint with httptest and bypass the real middleware.
Every other workspace-scoped daemon call goes through /api/daemon/... (under DaemonAuth); this is the first daemon call to a member-scoped route, which is why it slips through in tests but fails in a real deployment.
Suggested fix: call the existing GET /api/attachments/{id}/download instead. It's auth-only, self-resolves the workspace from the attachment row, and enforces membership internally (the runtime owner is a workspace member, so it passes). That also drops the separate metadata round-trip — content_type is already in the claim's AttachmentMeta. Alternatively, add a /api/daemon/.../attachments/{id} route under DaemonAuth.
2. Media-type filter is too broad → some images crash the turn
claudeInitialImages (daemon.go:3098 and :3114) accepts anything with an image/ prefix, but the Anthropic API only supports image/jpeg, image/png, image/gif, image/webp. On upload, .svg is rewritten to image/svg+xml (extContentTypes), and http.DetectContentType classifies .bmp / .ico as image/bmp / image/x-icon — all of these pass the filter but get 400'd by the API, which fails the entire initial user message and the task. That's a regression for issues carrying those attachment types. Please restrict to the supported set and skip the rest.
3. Images are re-injected on every resumed turn
runTask sets execOpts.InitialImages unconditionally for Claude (daemon.go:3559) regardless of task.PriorSessionID. On a resumed session the images are already in the transcript, so re-embedding them every turn re-downloads the bytes and piles up duplicate copies in context (N copies after N turns). Gate on cold start, e.g. if provider == "claude" && task.PriorSessionID == "" { ... }.
Nits
- Download-before-size-check:
downloadFilereads up to 100 MB (client.go:652) and the 10 MB cap is only applied afterward (daemon.go:3106), so a 50 MB image is fully downloaded and then discarded.size_bytesis available server-side (and the claim handler already reads the row), so plumbing it intoAttachmentMeta(types.go:119) would let you skip oversized images without downloading them. Related: the loop keeps downloading until it collects 4 images, so it isn't bounded to 4 downloads when early attachments are oversized. buildClaudeInput: the innerif image.MediaType != ""guard is dead — an emptyMediaTypeis alreadycontinued atclaude.go:622.downloadFiletruncates silently at 100 MB+1 rather than erroring on oversize. Fine for images, but a bit rough.- The branch currently conflicts with
main(daemon.go/agent.go/daemon_test.go) — needs a rebase.
Happy to re-review once #1 and #2 are addressed. Thanks again!
f751f25 to
43a0965
Compare
|
Addressed the blocking review items in
Local verification:
CI is green on the updated head. |
43a0965 to
544c175
Compare
|
Rebased onto current |
|
Re-review request: re-verified the three blocking Claude image attachment items on the current head Current behavior:
Verification run today:
|
544c175 to
9011539
Compare
|
Follow-up: rebased this PR onto current Conflict resolution kept the current main chat input-owner / coalesced-comment claim flow and preserved this PR's attachment metadata surface:
Verification after rebase:
|
9011539 to
ac48944
Compare
|
Rebased onto current The earlier blocking review items remain addressed on the rebased head:
Verification after rebase:
GitHub CI is green across changes, backend, frontend, and installers. Ready for re-review. |
What does this PR do?
Fixes Claude-backed daemon tasks not reliably understanding image attachments. The root cause is that Claude was asked to download/read issue images through the CLI/Read tool path, which can degrade into a text-only tool_result flow. This PR puts image attachments into Claude's initial user message as native image content blocks while keeping the existing CLI attachment download path as fallback/context.
Related Issue
Closes #2400
Type of Change
Changes Made
server/internal/handler/daemon.go: include issue attachment metadata in daemon task claims.server/internal/daemon/client.go: add daemon-side attachment metadata/download helper, matching the existing signed-url download behavior.server/internal/daemon/daemon.go: for Claude runtimes, download eligible image attachments and pass up to 4 images (max 10 MB each) into execution options.server/pkg/agent/claude.go: encode those images as Claude JSONL initial user-message image blocks.How to Test
cd server && go test ./pkg/agent ./internal/daemon ./internal/handler -count=1issue_attachmentsand Claude receives native image blocks in the initial message.Checklist
apps/web/features/landing/i18n/), starter-content (packages/views/onboarding/utils/starter-content-content-*.ts), and relevant docs (apps/docs/content/docs/)apps/docs/content/docs/developers/conventions.zh.mdx(terminology, mixed-rule fortask/issue/skill)AI Disclosure
AI tool used: Codex
Prompt / approach: Used a systematic debugging path: traced #2400 from issue/task claims through daemon prompt construction and Claude JSONL input, identified the local-path/Read-tool image path as the failing boundary, then added a minimal native image-block path for Claude with focused Go tests.
Screenshots (optional)
N/A - daemon/backend change only.