fix(acp): accept https:// URIs in image content blocks#24772
Closed
truenorth-lj wants to merge 1 commit intoanomalyco:devfrom
Closed
fix(acp): accept https:// URIs in image content blocks#24772truenorth-lj wants to merge 1 commit intoanomalyco:devfrom
truenorth-lj wants to merge 1 commit intoanomalyco:devfrom
Conversation
The previous prefix check `startsWith("http:")` only matches `http://...`
URLs and silently drops `https://...` URLs. Every other URL check in this
codebase (instruction.ts, import.ts, webfetch.ts, config.ts) checks both
prefixes — this appears to be a typo.
The bug surfaces when an ACP client sends an image content block with a
two-stage upload URL (e.g. a GCS signed URL), which is always https. The
case falls through and the image is silently dropped from the prompt.
Repro:
Send ACP session/prompt with content:
{ type: "image", uri: "https://example.com/x.png", mimeType: "image/png" }
Expected: image part forwarded to LLM provider
Actual: image silently dropped (no error, no parts.push)
Fix: align with the other 5 URL checks in this codebase by accepting both
http:// and https:// prefixes explicitly.
Contributor
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
Contributor
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Contributor
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
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.
Problem
packages/opencode/src/acp/agent.ts:1394reads:This prefix check matches
http://...URLs but nothttps://...URLs ("http:"is a literal 5-character prefix;"https://..."does not start with that exact substring because position 4 issnot:).Every other URL check in the same codebase uses both prefixes:
packages/opencode/src/session/instruction.ts:146,168startsWith("https://") || startsWith("http://")packages/opencode/src/cli/cmd/import.ts:98startsWith("http://") || startsWith("https://")packages/opencode/src/tool/webfetch.ts:23!startsWith("http://") && !startsWith("https://")packages/opencode/src/config/config.ts:1270startsWith("http://") || startsWith("https://")Only
acp/agent.ts:1394is missinghttps://. It looks like a typo (probably meant to write"http://"but the second/was dropped).Reproducer
Send an ACP
session/promptrequest with an image content block whoseuriishttps://:{ "type": "image", "uri": "https://storage.googleapis.com/bucket/img.png", "mimeType": "image/png" }Expected: image part is forwarded to the LLM provider as a
filepart.Actual: the case falls through (no
parts.push), and the image is silently dropped from the prompt — no error, no warning.This is the most likely path for any ACP client doing two-stage uploads (e.g. signing a GCS or S3 URL and sending the URL rather than inlining base64), because cloud-storage signed URLs are always
https://.Fix
One-line: align with the other 5 URL checks in this codebase by accepting both
http://andhttps://prefixes explicitly.Tests
I haven't added a unit test in this PR because I couldn't find an existing test for the surrounding
case "image"block that I could extend. Happy to add one if you'd like — point me at the right test file pattern.Out of scope
This is a typo fix — no behavior change for
http://URIs or for inline-datablocks. No new dependencies.