fix(opencode): surface 2xx-but-not-200 HTTP status in webfetch output - #47959
fix(opencode): surface 2xx-but-not-200 HTTP status in webfetch output#47959alexbuuuuuu wants to merge 2 commits into
Conversation
webfetch only hands the model `output`; `title` and `metadata` are consumed by the UI and never reach it. A 2xx response that is not 200 - a 202 Accepted throttle page, a 204 No Content - therefore arrives looking like the requested content, and the model has no way to tell it should try another source. Prefix `output` with an "[HTTP <status> ...]" notice whenever the status is not 200, and record the status in `metadata`. 200 responses are untouched, so existing output is unchanged. Non-2xx already surfaces its status through the error from filterStatusOk, so this closes the remaining gap. Fixes anomalyco#47897
github-actions
Bot
added
the
needs:compliance
|
The following comment was made by an LLM, it may be inaccurate: Based on my search, I found one potentially related PR: Related PR:
Why it's related: This PR is explicitly mentioned in your PR description. #45937 handles the error path for webfetch (keeping specific failure messages), while your PR #47959 handles the success path for 2xx-but-not-200 responses. Both PRs improve webfetch output visibility but address different scenarios. No duplicate PRs found for the specific issue being fixed (#47897). |
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped, has clear regression tests, and preserves existing 200-response output behavior.
Pull request overview
This PR improves webfetch tool output so 2xx-but-not-200 HTTP responses (e.g., 202/204/206) are distinguishable from a normal 200 in the model-visible output, while also recording the status in tool metadata for UI/consumers.
Changes:
- Prefixes tool
outputwith an[HTTP <status> …]notice whenresponse.status !== 200, and setsmetadata.statusfor all responses. - Adds targeted tests to ensure non-200 success statuses are annotated and 200 responses remain unchanged.
- Documents the new notice behavior in the tool’s usage notes.
File summaries
| File | Description |
|---|---|
| packages/opencode/src/tool/webfetch.ts | Adds non-200 status notice prefix and records HTTP status in tool metadata. |
| packages/opencode/test/tool/webfetch.test.ts | Adds regression tests covering 202-with-body, 202-empty-body, and unchanged 200 behavior. |
| packages/opencode/src/tool/webfetch.txt | Documents the new [HTTP <status> …] output prefix for non-200 responses. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -11,3 +11,4 @@ Usage notes: | |||
| - Format options: "markdown" (default), "text", or "html" | |||
| - This tool is read-only and does not modify any files | |||
| - Results may be summarized if the content is very large | |||
| - If the response status is not 200, the output starts with an "[HTTP <status> ...]" notice; the body may be a throttle or interstitial page rather than the requested content | |||
There was a problem hiding this comment.
Good catch - clarified in 1b11fad. The usage note now says a plain-text "[HTTP ...]" line precedes the body regardless of the requested format, and the opening description mentions it too, so format: "html" callers are not left assuming the output is strictly markup.
…s the body The notice is prefixed regardless of the requested format, so `format: "html"` no longer returns markup alone when the status is not 200. Say so in the tool description instead of leaving callers to assume the output is strictly the requested format.
Issue for this PR
Closes #47897
Type of change
What does this PR do?
webfetchputs the content-type intitleand returnsmetadata: {}. Neitherreaches the model — a completed tool part is turned into a model message from
state.outputalone (session/message-v2.ts), so the model only ever sees the body.That makes a 2xx-but-not-200 response indistinguishable from a successful fetch:
202 Acceptedthrottle or interstitial page (common when a shared egress IP isbeing rate-limited) arrives as if it were the page itself
204 No Contentarrives as an empty pageNon-2xx is already fine —
filterStatusOkfails and the status is in the errormessage. The gap is 202 / 203 / 204 / 206.
This prefixes
outputwith[HTTP <status> ...]when the status is not 200, andrecords the status in
metadatafor the UI. 200 responses are untouched, soexisting output is unchanged.
#45937 covers the error path (keeping the specific failure message); this one covers
the success path. They don't touch the same lines.
How did you verify your code works?
bun test test/tool/webfetch.test.tsinpackages/opencode: 7 pass / 0 fail.webfetch.tsreverted, 4 pass / 3 fail.bun run typecheckinpackages/opencodegives byte-identical output with andwithout my change —
devalready has one pre-existing error insrc/bus/global.ts.prettier --checkclean on both changed files.Screenshots / recordings
Not a UI change.
Checklist