feat(tools): web_fetch builtin — fetch a URL as clean, LLM-readable content (#266)

[initializ-mk]

Closes #266.

Gap

Agents had web_search (returns results, not page contents) and http_request (returns the raw response), but no "read this URL" tool. Reading a linked spec / changelog / API doc meant fetching raw HTML via http_request and hand-parsing it — expensive in tokens and unreliable. Claude Code exposes WebFetch for exactly this.

Change: web_fetch builtin

• Read-only GET over the same egress path as http_request — the context egress transport + SafeRedirectPolicy + optional R9 JIT credential injector. No second network path; honors the egress allowlist / SSRF protections. Anything mutating stays on http_request.
• HTML → clean text/markdown via golang.org/x/net/html: drops script/style/nav/footer/aside/head chrome; keeps the <title> (as #), headings (#..######), list items (- ), and links (text (href)). Non-HTML textual payloads (text/*, JSON, XML) pass through as-is.
• Content-type guard — refuses binary (image/*, application/pdf, octet-stream, …) rather than streaming a blob into context.
• Caps — redirect cap (5), byte cap (2 MiB, 8 MiB under compression), and a max_chars cap (default 50 000) with a truncation marker.
• Returns {url (final, post-redirect), content_type, status, content, truncated?}.

Registered default-on in builtins.All(), wired to the same HTTPCredentialInjector as http_request, and added to the Skill Builder prompt (enforced by the ListsEveryDefaultBuiltin drift-pin).

Acceptance

• web_fetch registered as a builtin (default-on like http_request).
• Honors egress allowlist; redirect + size caps; content-type guard.
• Returns cleaned text/markdown, not raw HTML.
• Unit tests for extraction + truncation + egress-denied path.

On reusing forge-plugins markdown

The issue suggested reusing the forge-plugins markdown converter, but that package converts markdown → platform HTML (Telegram/Slack) — the wrong direction — and forge-core importing forge-plugins would invert the dependency. golang.org/x/net/html was already in the module graph, so extraction adds no new external dependency (x/net just moves indirect → direct).

Tests

• TestWebFetch_ExtractsReadableText — chrome (script/style/nav/footer) stripped; title/heading/paragraph/list/link text kept.
• TestWebFetch_TruncatesAtMaxChars — cap + marker.
• TestWebFetch_PlainTextPassthrough — non-HTML textual returns as-is.
• TestWebFetch_RejectsBinaryContentType — image/png refused.
• TestWebFetch_RoutesThroughEgressTransport — a denying egress transport fails the fetch (proves no bypass).
• TestExtractReadableText_MalformedHTML — lenient parse still yields text.

Bonus: fixes doc-sync debt

While wiring the Skill Builder prompt, I found .claude/skills/forge-skill-builder.md had drifted several prompt rewrites behind skillBuilderPromptBase — #252/#270/#297 were never re-ported, so it was missing the entire "Built-in Tools" section. Re-ported the current prompt verbatim and added TestForgeSkillBuilderMD_MirrorsPrompt, which pins the md body to the Go constant so it can never silently rot again.

[initializ-mk]

Review: web_fetch builtin

The architecture is right: it reuses http_request's exact egress plumbing — EgressTransportFromContext + SafeRedirectPolicy + the R9 JIT credential injector — rather than opening a second network path, so the allowlist/SSRF protections and per-domain credentials apply identically (verified against http_request on main — same Transport: EgressTransportFromContext(ctx) pattern). HTML→text via x/net/html adds no new external dependency, and the forge-plugins-is-the-wrong-direction reasoning is sound. Tests cover extraction, truncation, passthrough, binary rejection, egress routing, malformed HTML. CI fully green.

Two Medium findings + two minors inline, plus a standout bonus:

The doc-drift fix is the highlight. Finding that .claude/skills/forge-skill-builder.md had drifted several prompt rewrites behind skillBuilderPromptBase (missing the entire "Built-in Tools" section from #252/#270/#297), re-porting it, and adding TestForgeSkillBuilderMD_MirrorsPrompt to pin the md to the Go constant — same anti-rot pin class as #297's builtin-list drift test, now applied to the skill-builder mirror. Real latent debt paid down. And the #297 ListsEveryDefaultBuiltin pin automatically enforces web_fetch is advertised in the prompt — the earlier test paying off as intended.

Nits (no inline anchor)

• Non-2xx responses return the error page's body as content with status set — defensible and consistent with http_request, but a one-line doc note ("a 404 yields the 404 page's text") sets expectations.
• Only http/https hrefs are emitted as text (href); relative/anchor links are dropped. Resolving relatives against finalURL would preserve them — out of scope, minor content loss.

Test gaps (finding 4)

Untested branches: the 2 MiB LimitReader byte cap, the empty-Content-Type→HTML path (a documented behavior worth pinning), the redirect cap, and that WithCredentialInjector actually stamps the materialized headers. Each is a few lines against httptest.

Verdict

Merge-ready pending finding 1 (code-block whitespace mangling — degrades the tool's primary doc-reading use case; fix is a preformatted-depth flag). Finding 2 (fail-open to DefaultTransport) is a defense-in-depth hardening I'd want tested-and-documented if not fixed — it mirrors http_request, so a shared-seam follow-up is acceptable. Findings 3–4 are follow-ups. Core design is exactly right.

[initializ-mk]

Medium (content quality): <pre>/<code> whitespace is collapsed, mangling code blocks.

collapseInlineWS runs on every text node, including inside <pre>/<code>. <pre> gets a newline before/after (it's a blockTag), but its INTERNAL newlines and indentation collapse to single spaces — so a fetched code sample or config snippet returns as one unstructured run of tokens. This matters because the tool's headline use case is "read this spec / API doc / changelog," and those pages are exactly where code blocks live.

Fix: track a preformatted-depth during the walk; inside <pre>/<code>, emit the raw text (preserve \n and space runs) instead of calling collapseInlineWS. Worth a test with a <pre> block asserting the indentation survives.

[initializ-mk]

Medium (security, defense-in-depth): fails OPEN to http.DefaultTransport when the context has no egress client.

EgressTransportFromContext(ctx) returns nil when no egress client is installed, and http.Client{Transport: nil} silently falls back to http.DefaultTransport — bypassing the allowlist and SSRF/DNS-rebinding protections entirely. The egress-routing test only proves the deny path WHEN a transport is present.

This mirrors http_request's existing behavior (not a regression, and the runtime installs the egress client via WithEgressClient before tools run), but web_fetch is a new, default-on, LLM-URL-driven surface — the exact shape where fail-open is worst.

Recommend: when EgressTransportFromContext(ctx) == nil, refuse the fetch (or fall back to SafeTransport, never DefaultTransport), and add a test pinning the no-egress-client path. If you'd rather keep strict parity with http_request, at minimum file a follow-up to harden both at the shared seam so this is on record.

[initializ-mk]

Minor: no charset handling — string(raw) assumes UTF-8, so a page served as ISO-8859-1 or with a charset= Content-Type parameter comes back as mojibake. golang.org/x/net/html/charset (already reachable via x/net) detects and transcodes the reader. Minor since most modern pages are UTF-8, but older spec/RFC pages — squarely in this tool's wheelhouse — are often Latin-1.

[initializ-mk]

Thanks — both Mediums fixed, the charset minor fixed, and the test gaps filled (4351bde).

1 (Medium — <pre>/<code> mangling) — fixed. The walker now tracks a preformatted depth; inside <pre>/<code> it emits text verbatim (newlines + indentation preserved) instead of collapseInlineWS. TestExtractReadableText_PreservesPreformatted asserts a code block's newlines and indentation survive while a normal paragraph's incidental whitespace still collapses. This matters exactly where you said — the spec/API-doc/changelog pages this tool targets are where code blocks live.

2 (Medium — fail-open to DefaultTransport) — fixed (fail-closed). When EgressTransportFromContext(ctx) == nil, web_fetch now refuses rather than silently using http.DefaultTransport (which would bypass the allowlist + SSRF/DNS-rebinding protections). TestWebFetch_RefusesWithoutEgressClient pins it. http_request has the same seam, so per your suggestion I filed #308 to harden both at a shared seam (and added a #308 reference in the code comment) rather than diverging silently.

3 (Minor — charset) — fixed. Non-UTF-8 bodies are transcoded via golang.org/x/net/html/charset before extraction (reads the charset= param / <meta charset> / BOM), so an ISO-8859-1 RFC page no longer returns mojibake. TestWebFetch_TranscodesCharset pins Latin-1 café. Still no new external dependency — it's under x/net.

4 (test gaps) — filled. TestClassifyContentType (all branches incl. the documented empty-CT → HTML and the binary rejections), TestWebFetch_StampsInjectedHeaders (the R9 injector's materialized header reaches the request, via the static provider), and TestWebFetch_ByteCapBoundsRawRead (the 2 MiB pre-extraction cap).

Nits: added a docs note that a non-2xx returns the error page's content with its status. The relative-link resolution I left as noted (out of scope) — happy to fold it in if you'd like relative/anchor hrefs resolved against finalURL.

Full tools/... + forge-ui suites green (12 web_fetch tests), lint clean.

[initializ-mk]

Re-review of 4351bde — all findings resolved, #308 filed, CI green

Finding 1 (pre/code mangling) — fixed correctly. A preDepth counter increments on <pre>/<code> entry, decrements on exit, and text nodes inside emit verbatim instead of collapsed. Nesting is handled by the depth count, and — nice detail — since only <pre> is in blockTags (not <code>), inline <code> stays inline while preserving its content, and block <pre> keeps its newlines. TestExtractReadableText_PreservesPreformatted pins that indentation + newlines survive AND that non-pre whitespace still collapses — both directions.

Finding 2 (fail-open) — fixed fail-closed, shared seam tracked. An explicit transport == nil check refuses the fetch with a clear error instead of falling to DefaultTransport. The comment documents that http_request shares the seam and that hardening both is tracked in #308 (verified open, accurately titled). TestWebFetch_RefusesWithoutEgressClient pins the refusal. Exactly the fix-the-new-surface-now, follow-up-the-shared-debt split recommended.

Finding 3 (charset) — fixed with fail-safe fallback. charset.NewReader transcodes from the Content-Type charset / <meta charset> / BOM, applied to both HTML and text kinds, falling back to raw bytes on any error rather than failing the fetch. TestWebFetch_TranscodesCharset pins Latin-1. No new dependency.

Finding 4 (test gaps) — all four filled: TestClassifyContentType (incl. empty-CT→HTML + binary rejection), TestWebFetch_StampsInjectedHeaders, and TestWebFetch_ByteCapBoundsRawRead. Docs note the pre/charset/refuse behavior and the non-2xx-returns-error-page-content contract.

Verdict

All findings resolved — merge-ready (all 10 checks green). Thorough cycle: both Mediums fixed with correct implementations and pinning tests, the shared-seam hardening honestly split into #308 rather than papered over, and the minors + test gaps all closed in one pass.