From 3c396f5f9ae0b5aba2013a46addca8f85343dd85 Mon Sep 17 00:00:00 2001
From: MK <mk@initializ.io>
Date: Tue, 14 Jul 2026 01:44:09 -0400
Subject: [PATCH 1/2] =?UTF-8?q?feat(tools):=20web=5Ffetch=20builtin=20?=
 =?UTF-8?q?=E2=80=94=20read=20a=20URL=20as=20clean=20text/markdown=20(clos?=
 =?UTF-8?q?es=20#266)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Agents had web_search (results, not contents) and http_request (raw
response), but no 'read this page' tool — so reading a linked spec /
changelog / API doc meant fetching raw HTML via http_request and
hand-parsing it (expensive, unreliable).

Add web_fetch:
- Read-only GET over the SAME egress-controlled path as http_request
  (context egress transport + SafeRedirectPolicy + optional R9 JIT
  credential injector) — no new network path, honors the allowlist.
- HTML -> clean text/markdown via golang.org/x/net/html: drops
  script/style/nav/footer/aside chrome, keeps title/headings (# ),
  list items (- ), and links (text (href)). Non-HTML textual payloads
  (text/*, json, xml) pass through as-is.
- Content-type guard: refuses binary (image/pdf/octet-stream/...) rather
  than streaming a blob into context. Redirect cap (5) + byte cap (2/8MB
  relaxed) + max_chars cap (default 50k) 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). Added to the Skill Builder
prompt (required by the ListsEveryDefaultBuiltin drift-pin).

Tests: HTML extraction (chrome stripped, structure kept), truncation +
marker, plain-text passthrough, binary content-type rejection, egress
transport routing (denying transport fails the fetch), malformed-HTML
resilience.

Also fixes doc-sync debt: .claude/skills/forge-skill-builder.md had
drifted several prompt rewrites behind skillBuilderPromptBase (#252/#270/
#297 never re-ported — it was missing the whole Built-in Tools section).
Re-ported the current prompt verbatim and added
TestForgeSkillBuilderMD_MirrorsPrompt so the port can never silently rot
again.

golang.org/x/net promoted from indirect to direct (already in the module
graph; no new external dependency).
---
 .claude/skills/forge-skill-builder.md       | 140 ++++++--
 docs/core-concepts/tools-and-builtins.md    |   1 +
 forge-core/go.mod                           |   2 +-
 forge-core/tools/builtins/register.go       |   1 +
 forge-core/tools/builtins/web_fetch.go      | 368 ++++++++++++++++++++
 forge-core/tools/builtins/web_fetch_test.go | 162 +++++++++
 forge-ui/skill_builder_context.go           |   1 +
 forge-ui/skill_builder_md_sync_test.go      |  70 ++++
 8 files changed, 722 insertions(+), 23 deletions(-)
 create mode 100644 forge-core/tools/builtins/web_fetch.go
 create mode 100644 forge-core/tools/builtins/web_fetch_test.go
 create mode 100644 forge-ui/skill_builder_md_sync_test.go

diff --git a/.claude/skills/forge-skill-builder.md b/.claude/skills/forge-skill-builder.md
index efc0957d..37228067 100644
--- a/.claude/skills/forge-skill-builder.md
+++ b/.claude/skills/forge-skill-builder.md
@@ -37,6 +37,38 @@ You help users design skills by:
 3. Generating a complete, valid SKILL.md file
 4. Optionally generating helper scripts if the skill requires them
 
+## Conversation Style — Converge Quickly
+
+The goal is to PRODUCE a skill; questions are only a means to that end. A stuck or looping interview is a failure. Follow these rules every turn:
+
+- Read the ENTIRE conversation before replying. NEVER re-ask something the user has already answered, even if it was phrased differently — re-asking an answered question is the worst thing you can do.
+- Ask AT MOST ONE clarifying question per turn, and only when a genuinely blocking unknown remains.
+- To draft a skill you need FOUR things: (1) the task and the tool(s) it exposes, (2) the credentials / env vars it needs, (3) the command-line tools its scripts invoke, and (4) an install recipe for every binary the base image lacks (see requires.bins below). The moment you know all four, STOP asking and return the complete SKILL.md. NEVER draft with an invented package name or download URL — if a needed binary isn't standard and you weren't told how to install it, that fourth thing is still unknown: ask for it.
+- Prefer a sensible default (and note it in the description or ## Important Notes) over asking. E.g. "review a GitHub PR and comment, authenticating with a GitHub PAT" is already enough to draft: tool = the gh CLI (or curl to api.github.com), credential = GITHUB_TOKEN (secret), egress = api.github.com.
+- Egress domains can usually be inferred from the task — don't ask for them.
+- On later turns, apply the user's change and return the FULL updated skill (honoring the edit-mode rules when editing an existing skill).
+- **You AUTHOR a SKILL.md — you do NOT perform the behavior.** Never answer the user's request in the chat, and NEVER fabricate tool output (inventing a specific time, a web result, an API response). If the skill needs live data, the SKILL.md tells the AGENT to call a tool; your job is to write that instruction, not to run it. Emit the skill in the `skill` field, not a role-played reply in `message`.
+- **Prefer a built-in tool over a custom tool or prose.** Before scaffolding a `## Tool:` / script or proposing a custom tool, check the **Built-in Tools** list below — if a built-in already provides the capability, the skill just instructs the agent to call it. There is NO valid "conversational only" skill for anything needing live data (current time, live web, an API call, a calculation): the agent cannot know it without a tool call, so a tool-less skill would only make it hallucinate. Do not offer a tool-less option for such requests.
+- **Scheduling (gated):** ONLY when the skill's behavior is time- or event-oriented — recurrence, reminders, monitoring/polling, digests, "every/daily/hourly/weekly", or reacting to a time/external trigger — proactively ask ONCE whether it should run on a schedule; if yes, wire `schedule_set` with the parsed cadence. For skills with no temporal dimension (formatting, parsing, one-shot lookups, tone/style), do NOT ask. If the user explicitly asks for a schedule, always wire `schedule_set` regardless.
+
+## Built-in Tools (always available — prefer these)
+
+Every Forge agent has these built-in tools registered automatically. A skill USES a built-in by instructing the agent to call it **by name** in the skill body — a built-in needs NO `## Tool:` section, NO script, and NO `requires.bins` entry (those are only for CUSTOM tools the skill itself provides). Match the request to a built-in BEFORE inventing anything:
+
+- `datetime_now` — current date/time in any timezone. Args: `timezone` (IANA name, e.g. `Australia/Brisbane`; default UTC), `format` (`rfc3339` | `unix` | `date` | `time` | `datetime`). Use for ANY "what's the time/date" need — never state a time from your own knowledge.
+- `web_search` — live web search (requires a web-search provider key configured). Use for current/live information from the web.
+- `web_fetch` — fetch a specific URL and return its main content as clean, readable text/markdown (strips nav/scripts/styling). Use to READ a known page/doc/spec/changelog. (`web_search` finds pages; `web_fetch` reads one; `http_request` is for raw bytes or non-GET.)
+- `http_request` — HTTP call to an allowed egress domain. Use to hit a REST API directly (no script needed for a simple call).
+- `json_parse` / `csv_parse` — parse JSON / CSV payloads.
+- `math_calculate` — evaluate an arithmetic expression.
+- `uuid_generate` — generate a UUID.
+- `file_create` — create a file (e.g. a generated report or export); the runtime attaches it to the channel response. Use for "generate a file / report and send it" needs instead of scaffolding a script.
+- `schedule_set` / `schedule_list` / `schedule_delete` / `schedule_history` — register / list / remove / inspect scheduled jobs. `schedule_set` takes a `cron` expression (5-field, `@daily`/`@hourly`/…, or `@every <duration>`) and a `task`. Use for anything recurring or time-triggered — writing "runs every day" in prose schedules NOTHING; the agent must call `schedule_set`. (Note: on Kubernetes deployments, dynamic `schedule_set` calls require `scheduler.kubernetes.allow_dynamic: true` — off by default; note this in ## Important Notes when the skill relies on scheduling.)
+
+Rules:
+- If a built-in covers the need, the skill instructs the agent to call it — do NOT scaffold a `## Tool:` / script or a custom tool that duplicates a built-in (e.g. never invent a `brisbane_time` tool when `datetime_now` exists).
+- A built-in-only skill (no custom tools) is complete with a title, description, the instruction to call the built-in(s), and Safety/Important-Notes as relevant — it has NO `## Tool:` sections.
+
 ## SKILL.md Format
 
 A SKILL.md file has two parts:
@@ -50,7 +82,7 @@ category: ops                          # Optional: sre, research, ops, developer
 tags:                                  # Optional: discovery keywords (lowercase kebab-case)
   - example
   - automation
-description: One-line description      # Required: what this skill does
+description: One-line description      # Required: what it does AND when it fires (triggers) — see "Trigger-rich description" below
 metadata:
   forge:
     requires:
@@ -69,6 +101,25 @@ metadata:
 ---
 ```
 
+### Declaring binaries and their install recipe
+
+Each entry in `requires.bins` is EITHER a bare name (already present in the base image — most standard tools: curl, jq, git, kubectl, aws, gh) OR a mapping that also tells the build how to install a binary the base image lacks:
+
+- Distro package: `- {name: ripgrep, apt: ripgrep}` (use `apk:` for the Alpine base).
+- Direct download: `- {name: initializ-cli, url: "https://.../initializ-cli", dest: /usr/local/bin/initializ-cli, chmod: "0755"}`.
+- Custom steps: `- {name: foo, run: ["curl -L https://… | tar xz -C /usr/local/bin"]}`.
+
+Only add an install recipe for a binary that is NOT already available. NEVER invent a download URL or package name — if the skill needs a custom tool and you don't have its install details, ask the user for the apt/apk package name OR the download URL (+ destination path).
+
+### Trigger-rich description (this is how the skill gets ACTIVATED)
+
+At runtime the agent sees only each skill's `description` in a catalog and routes a user request to a skill by MATCHING the request against that description — it hasn't loaded the skill body yet. So the description must state **when the skill fires**, not just what it does: include the trigger phrases, keywords, and intents a user would actually say. A vague description means the agent never routes to the skill and falls back to its own default answer (issue #271).
+
+- Weak: `description: German time skill`
+- Strong: `description: When the user asks the time ("what time is it", "current time", "clock"), reply in German with the current time in Brisbane.`
+
+Lead with the trigger ("When the user asks …"/"Use when …") and name the concrete phrases. Put discovery keywords in `tags` too, but the `description` is what the agent routes on.
+
 ### 2. Markdown Body
 
 After the frontmatter, write the skill body in markdown:
@@ -82,11 +133,11 @@ After the frontmatter, write the skill body in markdown:
 
 ## Required Body Sections
 
-Every generated SKILL.md body MUST include ALL of the following:
+Every generated SKILL.md body MUST include:
 
 1. **# Title** — A clear, descriptive heading for the skill
 2. **Description paragraph** — 2-3 sentences explaining what this skill does, who it is for, and the key value it provides
-3. **## Tool: tool_name** sections (one per tool) — each MUST contain:
+3. **## Tool: tool_name** sections — **required only for CUSTOM tools the skill provides** (a script under `scripts/`, or a binary via `requires.bins`). A skill that only orchestrates **built-in** tools (see Built-in Tools above) has NO `## Tool:` sections — it just instructs the agent to call the built-in by name. When you DO define a custom tool, its `## Tool:` section MUST contain:
    - **`**Input:**`** parameter table** with columns: Parameter | Type | Required | Description
    - **`**Output:**`** JSON schema** showing the structure of what the tool returns
    - **`**Examples:**`** table** with columns: User Request | Tool Input — at least 5 rows mapping natural-language requests to concrete tool invocations
@@ -132,6 +183,24 @@ Example: k8s-incident-triage uses `kubectl` — it only needs `bins: [kubectl]`
 For custom logic, provide executable scripts in a `scripts/` directory.
 Tool name maps to script: underscores → hyphens (e.g. `my_search` → `scripts/my-search.sh`).
 
+### Skill-relative references (files & scripts the instructions point to)
+Everything a skill ships lives in its OWN directory and can be referenced by a
+path RELATIVE TO THE SKILL in the instructions — no absolute paths, no `..`:
+- To have the agent READ a bundled file, write e.g. "read `reference/runbook.md`";
+  the agent loads it with `read_skill` (its `file` argument), resolved against the
+  skill directory.
+- To have the agent RUN a bundled helper script, write e.g. "run
+  `scripts/check.py`"; the agent runs it with `run_skill_script`, which resolves the
+  path against the skill directory, picks the interpreter by extension
+  (`.sh`→bash, `.py`→python3, `.js`→node), and executes it WITH THE SKILL
+  DIRECTORY AS THE WORKING DIRECTORY (so the script's own relative reads
+  resolve). JSON args are passed to the script as its first positional
+  argument (`$1`).
+
+Runnable helper-script languages: shell (`.sh`), Python (`.py`), JavaScript
+(`.js`). Add the interpreter to `requires.bins` (`python3` / `node`) when the
+skill ships that kind of script. TypeScript must be shipped as compiled `.js`.
+
 ## Script Decision Logic
 
 Prefer this order:
@@ -153,31 +222,58 @@ Always justify why a script is needed if you create one.
 - **denied_tools**: List tools the skill must NOT use (e.g. http_request if using cli_execute)
 - **No `sh -c`**: Never use shell command strings; use proper scripts instead
 
-## Output Format
+## Output Format — STRUCTURED JSON
+
+Every reply you send is a SINGLE JSON object and NOTHING else. No prose before or after, no markdown code fences around it. The object has exactly two fields:
+
+```json
+{
+  "message": "<your chat reply to the user: a clarifying question, a note about a default you chose, or a short summary of the skill you just drafted>",
+  "skill": null
+}
+```
+
+- `message` (string, required) — what the user reads in the chat. While you are still interviewing, this is your ONE clarifying question. When you draft or update a skill, this is a brief summary (and, in edit mode, the **Changed:** bullet list).
+- `skill` (object or null) — set to `null` on any turn where you are still gathering requirements. The moment the skill is draftable (all four things known), set it to:
+
+```json
+{
+  "skill_md": "<the COMPLETE SKILL.md content: frontmatter + full markdown body>",
+  "scripts": { "my-search.sh": "<complete script content>" }
+}
+```
+
+- `skill_md` is the entire SKILL.md as a single string (embedded newlines as `\n`, embedded quotes escaped — it is a JSON string value). It MUST contain the full frontmatter AND the full markdown body with every required `## Tool:` section (Input table, Output JSON schema, Examples table, detection heuristics), Safety Constraints, and Important Notes — exactly as the examples below show.
+- `scripts` maps script filename → complete script content. Omit it or use `{}` for binary-backed skills with no scripts.
+- Return the FULL skill_md every time you draft or revise — never a diff or a fragment. The editor swaps the whole file atomically.
+
+The worked examples below show SKILL.md CONTENT. When you respond, that content goes INSIDE the `skill_md` string of the JSON envelope — do not wrap your actual reply in backtick fences.
 
-When you generate skill content, use QUADRUPLE-backtick labeled fences (```` not ```).
-This is critical — inner triple-backtick code blocks (JSON schemas, etc.) must nest safely.
+## Complete Example: Built-in-only Skill (german-brisbane-time)
+
+The whole skill is instructions to call a built-in — no custom tool, no `## Tool:` section, no script, no `requires.bins`. This is the correct shape for "when I ask the time, answer in German with Brisbane time":
 
-For the SKILL.md content:
-`````
 ````skill.md
 ---
-name: example-skill
-...
+name: german-brisbane-time
+category: ops
+description: When the user asks the time ("what time is it", "current time", "clock"), reply in German with the current time in Brisbane, Australia.
 ---
-# Example Skill
-...
-````
-`````
-
-For optional scripts (only if needed):
-`````
-````script:my-search.sh
-#!/bin/bash
-set -euo pipefail
-...
+
+# German Brisbane Time
+
+When the user asks what the time is, call the `datetime_now` built-in tool with
+`timezone` set to `"Australia/Brisbane"`, then respond in German stating that time.
+Use the tool's returned value — never guess or state a time without calling
+`datetime_now` first.
+
+Example: the tool returns `14:05` → reply: „Es ist 14:05 Uhr in Brisbane, Australien.“
+
+## Important Notes
+
+- `Australia/Brisbane` is UTC+10 with no daylight saving, so `datetime_now` returns the correct wall time year-round.
+- No credentials, egress, scripts, or `requires.bins` — `datetime_now` is a built-in.
 ````
-`````
 
 ## Complete Example: Binary-backed Skill (k8s-incident-triage)
 
diff --git a/docs/core-concepts/tools-and-builtins.md b/docs/core-concepts/tools-and-builtins.md
index 690ef7ed..dd6c4e46 100644
--- a/docs/core-concepts/tools-and-builtins.md
+++ b/docs/core-concepts/tools-and-builtins.md
@@ -26,6 +26,7 @@ Tools are capabilities that an LLM agent can invoke during execution. Forge prov
 | `uuid_generate` | Generate UUID v4 identifiers |
 | `math_calculate` | Evaluate mathematical expressions |
 | `web_search` | Search the web for quick lookups and recent information |
+| `web_fetch` | Fetch a URL and return its main content as clean, readable text/markdown (strips nav/scripts/styling). Read-only GET, egress-controlled, with redirect + size caps and a content-type guard. Use to *read* a page; `web_search` finds pages, `http_request` returns raw bytes |
 | `file_create` | Create a downloadable file, written to the agent's `.forge/files/` directory |
 | `read_skill` | Load full instructions for an available skill on demand |
 | `memory_search` | Search long-term memory (when enabled) |
diff --git a/forge-core/go.mod b/forge-core/go.mod
index 5746b5e2..aa2cdb2a 100644
--- a/forge-core/go.mod
+++ b/forge-core/go.mod
@@ -16,6 +16,7 @@ require (
 	go.opentelemetry.io/otel/sdk v1.44.0
 	go.opentelemetry.io/otel/trace v1.44.0
 	golang.org/x/crypto v0.52.0
+	golang.org/x/net v0.55.0
 	gopkg.in/yaml.v3 v3.0.1
 )
 
@@ -33,7 +34,6 @@ require (
 	go.opentelemetry.io/auto/sdk v1.2.1 // indirect
 	go.opentelemetry.io/otel/metric v1.44.0 // indirect
 	go.opentelemetry.io/proto/otlp v1.10.0 // indirect
-	golang.org/x/net v0.55.0 // indirect
 	golang.org/x/sys v0.45.0 // indirect
 	golang.org/x/text v0.37.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20260526163538-3dc84a4a5aaa // indirect
diff --git a/forge-core/tools/builtins/register.go b/forge-core/tools/builtins/register.go
index f0f6b374..aa18cb54 100644
--- a/forge-core/tools/builtins/register.go
+++ b/forge-core/tools/builtins/register.go
@@ -28,6 +28,7 @@ func All(opts ...Options) []tools.Tool {
 	}
 	return []tools.Tool{
 		(&httpRequestTool{}).WithCredentialInjector(o.HTTPCredentialInjector),
+		(&webFetchTool{}).WithCredentialInjector(o.HTTPCredentialInjector),
 		&jsonParseTool{},
 		&csvParseTool{},
 		&datetimeNowTool{},
diff --git a/forge-core/tools/builtins/web_fetch.go b/forge-core/tools/builtins/web_fetch.go
new file mode 100644
index 00000000..bdac10a5
--- /dev/null
+++ b/forge-core/tools/builtins/web_fetch.go
@@ -0,0 +1,368 @@
+package builtins
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"strings"
+	"time"
+
+	"golang.org/x/net/html"
+
+	"github.com/initializ/forge/forge-core/credentials"
+	"github.com/initializ/forge/forge-core/security"
+	"github.com/initializ/forge/forge-core/tools"
+)
+
+// webFetchTool fetches a single URL and returns its main content as clean,
+// LLM-readable text/markdown — the "read this page" tool that web_search
+// (results, not contents) and http_request (raw response) don't provide (#266).
+//
+// It reuses http_request's egress plumbing (the context egress transport +
+// safe-redirect policy + optional JIT credential injector) rather than opening
+// a second network path, so it honors the same allowlist / SSRF protections.
+// Read-only GET; anything mutating stays on http_request.
+type webFetchTool struct {
+	credInjector *credentials.Injector
+}
+
+// WithCredentialInjector attaches an R9 JIT-credential injector, mirroring
+// http_request so a domain's operator-declared credentials also apply when the
+// agent reads a page there. nil-safe.
+func (t *webFetchTool) WithCredentialInjector(inj *credentials.Injector) *webFetchTool {
+	t.credInjector = inj
+	return t
+}
+
+const (
+	// webFetchByteLimit caps the raw body read BEFORE extraction, so a huge
+	// page can't blow up memory. Scales with compression (RelaxedLimits) like
+	// http_request.
+	webFetchByteLimit        = 2 << 20 // 2 MiB
+	webFetchByteLimitRelaxed = 8 << 20 // 8 MiB
+
+	// webFetchDefaultMaxChars caps the CLEANED content returned to the model
+	// when the caller doesn't specify max_chars. ~12-15k tokens of readable
+	// text — enough for a doc page, bounded against context blowup.
+	webFetchDefaultMaxChars = 50000
+
+	webFetchTimeout      = 30 * time.Second
+	webFetchMaxRedirects = 5
+)
+
+type webFetchInput struct {
+	URL      string `json:"url"`
+	MaxChars int    `json:"max_chars,omitempty"`
+}
+
+func (t *webFetchTool) Name() string { return "web_fetch" }
+func (t *webFetchTool) Description() string {
+	return "Fetch a URL and return its main content as clean, readable text/markdown " +
+		"(strips navigation, scripts, and styling). Read-only GET — use it to read a " +
+		"specific page, doc, spec, or changelog. For raw response bytes/headers or a " +
+		"non-GET method, use http_request instead; for finding pages, use web_search."
+}
+func (t *webFetchTool) Category() tools.Category { return tools.CategoryBuiltin }
+
+func (t *webFetchTool) InputSchema() json.RawMessage {
+	return json.RawMessage(`{
+		"type": "object",
+		"properties": {
+			"url": {"type": "string", "description": "The URL to fetch (http/https, must be on the egress allowlist)"},
+			"max_chars": {"type": "integer", "description": "Maximum characters of cleaned content to return (default 50000). Content over the cap is truncated with a marker."}
+		},
+		"required": ["url"]
+	}`)
+}
+
+func (t *webFetchTool) Execute(ctx context.Context, args json.RawMessage) (string, error) {
+	var input webFetchInput
+	if err := json.Unmarshal(args, &input); err != nil {
+		return "", fmt.Errorf("parsing input: %w", err)
+	}
+	if strings.TrimSpace(input.URL) == "" {
+		return "", fmt.Errorf("url is required")
+	}
+	maxChars := input.MaxChars
+	if maxChars <= 0 {
+		maxChars = webFetchDefaultMaxChars
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, input.URL, nil)
+	if err != nil {
+		return "", fmt.Errorf("creating request: %w", err)
+	}
+	// A UA + Accept nudge servers toward returning HTML/text rather than a
+	// bot-block or an API/JSON variant.
+	req.Header.Set("Accept", "text/html,application/xhtml+xml,text/plain,text/markdown;q=0.9,*/*;q=0.5")
+	if req.Header.Get("User-Agent") == "" {
+		req.Header.Set("User-Agent", "forge-web-fetch/1.0")
+	}
+
+	// R9 JIT credentials, same contract as http_request: operator-declared
+	// creds override any LLM-supplied header. nil injector → no-op.
+	if t.credInjector != nil {
+		handle, herr := t.credInjector.Materialize(ctx, "web_fetch", "", args)
+		if herr != nil {
+			return "", fmt.Errorf("web_fetch: minting JIT credentials: %w", herr)
+		}
+		if handle != nil {
+			defer func() { _ = handle.Close(ctx) }()
+			for k, v := range handle.Headers() {
+				req.Header.Set(k, v)
+			}
+		}
+	}
+
+	client := &http.Client{
+		Transport:     security.EgressTransportFromContext(ctx),
+		Timeout:       webFetchTimeout,
+		CheckRedirect: security.SafeRedirectPolicy(webFetchMaxRedirects),
+	}
+	resp, err := client.Do(req)
+	if err != nil {
+		return "", fmt.Errorf("fetching %s: %w", input.URL, err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	contentType := resp.Header.Get("Content-Type")
+	kind, ok := classifyContentType(contentType)
+	if !ok {
+		// Don't stream a binary blob (image / pdf / octet-stream / video) back
+		// into the context — return a clear, small message instead.
+		return "", fmt.Errorf("web_fetch does not read non-textual content (Content-Type %q); use http_request for binary responses", firstToken(contentType))
+	}
+
+	limit := int64(webFetchByteLimit)
+	if tools.RelaxedLimits(ctx) {
+		limit = webFetchByteLimitRelaxed
+	}
+	raw, err := io.ReadAll(io.LimitReader(resp.Body, limit))
+	if err != nil {
+		return "", fmt.Errorf("reading response: %w", err)
+	}
+
+	var content string
+	if kind == contentHTML {
+		content = extractReadableText(string(raw))
+	} else {
+		content = strings.TrimSpace(string(raw))
+	}
+
+	truncated := false
+	if len([]rune(content)) > maxChars {
+		content = string([]rune(content)[:maxChars]) + "\n\n[content truncated at max_chars]"
+		truncated = true
+	}
+
+	finalURL := input.URL
+	if resp.Request != nil && resp.Request.URL != nil {
+		finalURL = resp.Request.URL.String()
+	}
+
+	result := map[string]any{
+		"url":          finalURL,
+		"content_type": firstToken(contentType),
+		"status":       resp.StatusCode,
+		"content":      content,
+	}
+	if truncated {
+		result["truncated"] = true
+	}
+	data, _ := json.Marshal(result)
+	return string(data), nil
+}
+
+// contentKind distinguishes HTML (needs extraction) from already-textual
+// payloads (returned as-is).
+type contentKind int
+
+const (
+	contentHTML contentKind = iota
+	contentText
+)
+
+// classifyContentType returns whether the Content-Type is textual (and if so,
+// whether it needs HTML extraction). An EMPTY content type is treated as HTML
+// — servers commonly omit it for pages, and the extractor is safe on plain
+// text too. Binary types (image/pdf/octet-stream/video/audio/font) return ok=false.
+func classifyContentType(ct string) (contentKind, bool) {
+	mt := strings.ToLower(firstToken(ct))
+	switch {
+	case mt == "":
+		return contentHTML, true
+	case mt == "text/html", mt == "application/xhtml+xml":
+		return contentHTML, true
+	case strings.HasPrefix(mt, "text/"):
+		return contentText, true
+	case mt == "application/json",
+		mt == "application/xml",
+		strings.HasSuffix(mt, "+xml"),
+		strings.HasSuffix(mt, "+json"):
+		return contentText, true
+	default:
+		return contentText, false
+	}
+}
+
+// firstToken returns the media type without parameters ("text/html; charset=..."
+// → "text/html"), trimmed.
+func firstToken(ct string) string {
+	if i := strings.IndexByte(ct, ';'); i >= 0 {
+		ct = ct[:i]
+	}
+	return strings.TrimSpace(ct)
+}
+
+// skipTags are element subtrees dropped entirely during extraction — script /
+// style / chrome. Their text never reaches the output.
+var skipTags = map[string]bool{
+	"script": true, "style": true, "noscript": true, "head": true,
+	"svg": true, "template": true, "iframe": true, "nav": true,
+	"footer": true, "aside": true, "form": true, "button": true,
+	"select": true, "option": true,
+}
+
+// blockTags emit a newline around their content so structure survives.
+var blockTags = map[string]bool{
+	"p": true, "div": true, "section": true, "article": true, "main": true,
+	"header": true, "ul": true, "ol": true, "table": true, "tr": true,
+	"blockquote": true, "pre": true, "hr": true, "dl": true, "dd": true,
+	"dt": true, "figure": true, "figcaption": true,
+}
+
+// extractReadableText parses HTML and returns clean, structured text: headings
+// as markdown `#`, list items as `- `, links as `text (href)`, block elements
+// separated by blank lines. script/style/nav chrome is dropped. Malformed HTML
+// still parses (net/html is lenient); a hard parse failure falls back to the
+// raw string so the caller always gets *something*.
+func extractReadableText(htmlContent string) string {
+	doc, err := html.Parse(strings.NewReader(htmlContent))
+	if err != nil {
+		return strings.TrimSpace(htmlContent)
+	}
+
+	var b strings.Builder
+	if title := findTitle(doc); title != "" {
+		b.WriteString("# " + title + "\n\n")
+	}
+
+	var walk func(*html.Node)
+	walk = func(n *html.Node) {
+		switch n.Type {
+		case html.TextNode:
+			b.WriteString(collapseInlineWS(n.Data))
+			return
+		case html.ElementNode:
+			if skipTags[n.Data] {
+				return
+			}
+			switch {
+			case isHeading(n.Data):
+				b.WriteString("\n\n" + strings.Repeat("#", int(n.Data[1]-'0')) + " ")
+			case n.Data == "li":
+				b.WriteString("\n- ")
+			case n.Data == "br":
+				b.WriteString("\n")
+			case blockTags[n.Data]:
+				b.WriteString("\n")
+			}
+		}
+		for c := n.FirstChild; c != nil; c = c.NextSibling {
+			walk(c)
+		}
+		if n.Type == html.ElementNode {
+			if n.Data == "a" {
+				if href := attr(n, "href"); isHTTPURL(href) {
+					b.WriteString(" (" + href + ")")
+				}
+			}
+			if isHeading(n.Data) || n.Data == "li" || blockTags[n.Data] {
+				b.WriteString("\n")
+			}
+		}
+	}
+	walk(doc)
+	return tidyText(b.String())
+}
+
+func isHeading(tag string) bool {
+	return len(tag) == 2 && tag[0] == 'h' && tag[1] >= '1' && tag[1] <= '6'
+}
+
+func attr(n *html.Node, name string) string {
+	for _, a := range n.Attr {
+		if a.Key == name {
+			return a.Val
+		}
+	}
+	return ""
+}
+
+func isHTTPURL(s string) bool {
+	return strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://")
+}
+
+// findTitle returns the first <title> text, or "".
+func findTitle(doc *html.Node) string {
+	var title string
+	var walk func(*html.Node) bool
+	walk = func(n *html.Node) bool {
+		if n.Type == html.ElementNode && n.Data == "title" && n.FirstChild != nil {
+			title = strings.TrimSpace(n.FirstChild.Data)
+			return true
+		}
+		for c := n.FirstChild; c != nil; c = c.NextSibling {
+			if walk(c) {
+				return true
+			}
+		}
+		return false
+	}
+	walk(doc)
+	return title
+}
+
+// collapseInlineWS collapses runs of any whitespace (incl. incidental HTML
+// source newlines/indentation) within a text node to a single space.
+func collapseInlineWS(s string) string {
+	var b strings.Builder
+	prevSpace := false
+	for _, r := range s {
+		if r == ' ' || r == '\t' || r == '\n' || r == '\r' || r == '\f' {
+			if !prevSpace {
+				b.WriteByte(' ')
+				prevSpace = true
+			}
+			continue
+		}
+		b.WriteRune(r)
+		prevSpace = false
+	}
+	return b.String()
+}
+
+// tidyText normalizes the assembled output: trim trailing spaces per line,
+// collapse 3+ blank lines to one, and trim the whole.
+func tidyText(s string) string {
+	lines := strings.Split(s, "\n")
+	for i := range lines {
+		lines[i] = strings.TrimRight(lines[i], " \t")
+	}
+	var out []string
+	blanks := 0
+	for _, ln := range lines {
+		if strings.TrimSpace(ln) == "" {
+			blanks++
+			if blanks > 1 {
+				continue
+			}
+		} else {
+			blanks = 0
+		}
+		out = append(out, ln)
+	}
+	return strings.TrimSpace(strings.Join(out, "\n"))
+}
diff --git a/forge-core/tools/builtins/web_fetch_test.go b/forge-core/tools/builtins/web_fetch_test.go
new file mode 100644
index 00000000..bdd3b9b5
--- /dev/null
+++ b/forge-core/tools/builtins/web_fetch_test.go
@@ -0,0 +1,162 @@
+package builtins
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/initializ/forge/forge-core/security"
+)
+
+func runWebFetch(t *testing.T, ctx context.Context, args map[string]any) (map[string]any, error) {
+	t.Helper()
+	raw, _ := json.Marshal(args)
+	out, err := (&webFetchTool{}).Execute(ctx, raw)
+	if err != nil {
+		return nil, err
+	}
+	var m map[string]any
+	if uerr := json.Unmarshal([]byte(out), &m); uerr != nil {
+		t.Fatalf("result is not JSON: %v (%q)", uerr, out)
+	}
+	return m, nil
+}
+
+// TestWebFetch_ExtractsReadableText is the core AC: HTML in, clean text out —
+// script/style/nav chrome dropped, headings/paragraph/link text kept.
+func TestWebFetch_ExtractsReadableText(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "text/html; charset=utf-8")
+		_, _ = w.Write([]byte(`<html><head><title>Doc Title</title>
+			<style>.x{color:red}</style><script>alert('xss')</script></head>
+			<body>
+			<nav>Home About Contact</nav>
+			<h1>Main Heading</h1>
+			<p>The quick brown fox jumps over the lazy dog.</p>
+			<ul><li>first item</li><li>second item</li></ul>
+			<a href="https://example.com/spec">the spec</a>
+			<footer>copyright 2026</footer>
+			</body></html>`))
+	}))
+	defer srv.Close()
+
+	m, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL})
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	content, _ := m["content"].(string)
+
+	mustContain := []string{"Doc Title", "Main Heading", "The quick brown fox", "first item", "second item", "the spec", "https://example.com/spec"}
+	for _, s := range mustContain {
+		if !strings.Contains(content, s) {
+			t.Errorf("cleaned content missing %q:\n%s", s, content)
+		}
+	}
+	mustNotContain := []string{"alert(", "color:red", "Home About Contact", "copyright 2026"}
+	for _, s := range mustNotContain {
+		if strings.Contains(content, s) {
+			t.Errorf("cleaned content should have stripped %q:\n%s", s, content)
+		}
+	}
+	if m["content_type"] != "text/html" {
+		t.Errorf("content_type = %v, want text/html", m["content_type"])
+	}
+}
+
+// TestWebFetch_TruncatesAtMaxChars pins the size cap + marker.
+func TestWebFetch_TruncatesAtMaxChars(t *testing.T) {
+	body := "<html><body><p>" + strings.Repeat("word ", 5000) + "</p></body></html>"
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "text/html")
+		_, _ = w.Write([]byte(body))
+	}))
+	defer srv.Close()
+
+	m, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL, "max_chars": 100})
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	if m["truncated"] != true {
+		t.Errorf("expected truncated=true, got %v", m["truncated"])
+	}
+	content, _ := m["content"].(string)
+	if !strings.Contains(content, "[content truncated at max_chars]") {
+		t.Errorf("missing truncation marker:\n%s", content)
+	}
+	// 100 chars + the marker line — well under the untruncated length.
+	if len([]rune(content)) > 100+len([]rune("\n\n[content truncated at max_chars]")) {
+		t.Errorf("content longer than max_chars + marker: %d runes", len([]rune(content)))
+	}
+}
+
+// TestWebFetch_PlainTextPassthrough: non-HTML textual content returns as-is.
+func TestWebFetch_PlainTextPassthrough(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		_, _ = w.Write([]byte("plain body, not HTML"))
+	}))
+	defer srv.Close()
+
+	m, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL})
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	if got, _ := m["content"].(string); got != "plain body, not HTML" {
+		t.Errorf("plain text content = %q", got)
+	}
+}
+
+// TestWebFetch_RejectsBinaryContentType is the content-type guard: never stream
+// a binary blob into the context.
+func TestWebFetch_RejectsBinaryContentType(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "image/png")
+		_, _ = w.Write([]byte{0x89, 0x50, 0x4e, 0x47})
+	}))
+	defer srv.Close()
+
+	_, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL})
+	if err == nil {
+		t.Fatal("expected web_fetch to reject image/png")
+	}
+	if !strings.Contains(err.Error(), "non-textual") {
+		t.Errorf("expected a non-textual content-type error, got %v", err)
+	}
+}
+
+// denyTransport is an egress transport that refuses every request — stands in
+// for a blocked (not-on-allowlist) domain.
+type denyTransport struct{}
+
+func (denyTransport) RoundTrip(*http.Request) (*http.Response, error) {
+	return nil, errors.New("egress: domain not on allowlist")
+}
+
+// TestWebFetch_RoutesThroughEgressTransport proves web_fetch uses the context
+// egress client (no second network path / bypass): a denying transport makes
+// the fetch fail. Guards the "must respect the egress allowlist" AC.
+func TestWebFetch_RoutesThroughEgressTransport(t *testing.T) {
+	ctx := security.WithEgressClient(context.Background(), &http.Client{Transport: denyTransport{}})
+	_, err := runWebFetch(t, ctx, map[string]any{"url": "https://blocked.example/page"})
+	if err == nil {
+		t.Fatal("expected the denying egress transport to fail the fetch")
+	}
+	if !strings.Contains(err.Error(), "allowlist") {
+		t.Errorf("expected the egress denial to surface, got %v", err)
+	}
+}
+
+// TestExtractReadableText_MalformedHTML: net/html is lenient, but a totally
+// broken document must still yield text rather than an error.
+func TestExtractReadableText_MalformedHTML(t *testing.T) {
+	got := extractReadableText(`<p>hello <b>world</p></b> <div>tail`)
+	for _, s := range []string{"hello", "world", "tail"} {
+		if !strings.Contains(got, s) {
+			t.Errorf("malformed HTML lost %q: %q", s, got)
+		}
+	}
+}
diff --git a/forge-ui/skill_builder_context.go b/forge-ui/skill_builder_context.go
index 28d4a926..3f6b8f68 100644
--- a/forge-ui/skill_builder_context.go
+++ b/forge-ui/skill_builder_context.go
@@ -102,6 +102,7 @@ Every Forge agent has these built-in tools registered automatically. A skill USE
 
 - ` + "`" + `datetime_now` + "`" + ` — current date/time in any timezone. Args: ` + "`" + `timezone` + "`" + ` (IANA name, e.g. ` + "`" + `Australia/Brisbane` + "`" + `; default UTC), ` + "`" + `format` + "`" + ` (` + "`" + `rfc3339` + "`" + ` | ` + "`" + `unix` + "`" + ` | ` + "`" + `date` + "`" + ` | ` + "`" + `time` + "`" + ` | ` + "`" + `datetime` + "`" + `). Use for ANY "what's the time/date" need — never state a time from your own knowledge.
 - ` + "`" + `web_search` + "`" + ` — live web search (requires a web-search provider key configured). Use for current/live information from the web.
+- ` + "`" + `web_fetch` + "`" + ` — fetch a specific URL and return its main content as clean, readable text/markdown (strips nav/scripts/styling). Use to READ a known page/doc/spec/changelog. (` + "`" + `web_search` + "`" + ` finds pages; ` + "`" + `web_fetch` + "`" + ` reads one; ` + "`" + `http_request` + "`" + ` is for raw bytes or non-GET.)
 - ` + "`" + `http_request` + "`" + ` — HTTP call to an allowed egress domain. Use to hit a REST API directly (no script needed for a simple call).
 - ` + "`" + `json_parse` + "`" + ` / ` + "`" + `csv_parse` + "`" + ` — parse JSON / CSV payloads.
 - ` + "`" + `math_calculate` + "`" + ` — evaluate an arithmetic expression.
diff --git a/forge-ui/skill_builder_md_sync_test.go b/forge-ui/skill_builder_md_sync_test.go
new file mode 100644
index 00000000..d2dcc754
--- /dev/null
+++ b/forge-ui/skill_builder_md_sync_test.go
@@ -0,0 +1,70 @@
+package forgeui
+
+import (
+	"os"
+	"strings"
+	"testing"
+)
+
+// skillBuilderMDPath is the knowledge-skill port of the Skill Builder prompt,
+// relative to this package. The sync-docs rule requires its body to stay
+// byte-identical to skillBuilderPromptBase (apart from the frontmatter + intro
+// wrapper the md adds).
+const skillBuilderMDPath = "../.claude/skills/forge-skill-builder.md"
+
+// TestForgeSkillBuilderMD_MirrorsPrompt pins the doc-sync invariant: the body
+// of .claude/skills/forge-skill-builder.md (everything after its intro) must
+// equal skillBuilderPromptBase verbatim. Before this, the md had silently
+// drifted several prompt rewrites behind the Go constant (#252/#270/#297 never
+// re-ported), so a new default builtin like web_fetch (#266) was advertised in
+// the UI prompt but not in the packaged skill. Fails loudly on the next drift.
+func TestForgeSkillBuilderMD_MirrorsPrompt(t *testing.T) {
+	raw, err := os.ReadFile(skillBuilderMDPath)
+	if err != nil {
+		t.Fatalf("read %s: %v", skillBuilderMDPath, err)
+	}
+	md := string(raw)
+
+	// The prompt body begins at its first line; the lines above it are the
+	// md-only frontmatter + "How to use this skill" intro.
+	const promptStart = "You are the Forge Skill Designer,"
+	idx := strings.Index(md, promptStart)
+	if idx < 0 {
+		t.Fatalf("%s does not contain the prompt body (looked for %q)", skillBuilderMDPath, promptStart)
+	}
+	gotBody := strings.TrimSpace(md[idx:])
+	wantBody := strings.TrimSpace(skillBuilderPromptBase)
+
+	if gotBody != wantBody {
+		// Point at the first divergence so the fix is obvious.
+		min := len(gotBody)
+		if len(wantBody) < min {
+			min = len(wantBody)
+		}
+		div := min
+		for i := 0; i < min; i++ {
+			if gotBody[i] != wantBody[i] {
+				div = i
+				break
+			}
+		}
+		lo := div - 60
+		if lo < 0 {
+			lo = 0
+		}
+		t.Fatalf("%s body has drifted from skillBuilderPromptBase near offset %d.\n"+
+			"Re-port the constant into the md (keep the frontmatter + intro).\n"+
+			"  md:   ...%q...\n  want: ...%q...",
+			skillBuilderMDPath, div, snip(gotBody, lo, div+40), snip(wantBody, lo, div+40))
+	}
+}
+
+func snip(s string, lo, hi int) string {
+	if lo < 0 {
+		lo = 0
+	}
+	if hi > len(s) {
+		hi = len(s)
+	}
+	return s[lo:hi]
+}

From 4351bde0c4b8dcf1f551fba57003ec6ab4322ef4 Mon Sep 17 00:00:00 2001
From: MK <mk@initializ.io>
Date: Tue, 14 Jul 2026 02:14:10 -0400
Subject: [PATCH 2/2] =?UTF-8?q?fix(tools):=20web=5Ffetch=20=E2=80=94=20pre?=
 =?UTF-8?q?serve=20pre/code,=20refuse=20without=20egress,=20transcode=20ch?=
 =?UTF-8?q?arset=20(#307=20review)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Medium (content): <pre>/<code> whitespace was collapsed, mangling code
  samples on the doc pages this tool is meant to read. Track a
  preformatted depth in the walker and emit verbatim text (newlines +
  indentation) inside pre/code. Test asserts indentation + newlines
  survive while non-pre whitespace still collapses.

- Medium (security, defense-in-depth): web_fetch fell open to
  http.DefaultTransport when the context had no egress client, bypassing
  the allowlist / SSRF protections. Refuse instead (fail-closed) — it's a
  new default-on, LLM-URL-driven surface. http_request has the same seam;
  hardening both is tracked in #308. Test pins the no-egress-client refusal.

- Minor (charset): non-UTF-8 pages (older spec/RFC pages are often
  ISO-8859-1 or declare charset=) came back as mojibake. Transcode via
  golang.org/x/net/html/charset before extraction. Test pins Latin-1.

- Test gaps (finding 4): classifyContentType unit test (incl. empty-CT →
  HTML and binary rejections), JIT-credential-injector header stamping
  (static provider), and the 2 MiB byte-cap bound.

- Docs: note the pre/charset/refuse behavior and that a non-2xx response
  returns the error page's content with its status.

No new external dependency — charset is under golang.org/x/net (already direct).
---
 docs/core-concepts/tools-and-builtins.md    |   2 +-
 forge-core/tools/builtins/web_fetch.go      |  48 ++++++-
 forge-core/tools/builtins/web_fetch_test.go | 151 +++++++++++++++++++-
 3 files changed, 191 insertions(+), 10 deletions(-)

diff --git a/docs/core-concepts/tools-and-builtins.md b/docs/core-concepts/tools-and-builtins.md
index dd6c4e46..de97661e 100644
--- a/docs/core-concepts/tools-and-builtins.md
+++ b/docs/core-concepts/tools-and-builtins.md
@@ -26,7 +26,7 @@ Tools are capabilities that an LLM agent can invoke during execution. Forge prov
 | `uuid_generate` | Generate UUID v4 identifiers |
 | `math_calculate` | Evaluate mathematical expressions |
 | `web_search` | Search the web for quick lookups and recent information |
-| `web_fetch` | Fetch a URL and return its main content as clean, readable text/markdown (strips nav/scripts/styling). Read-only GET, egress-controlled, with redirect + size caps and a content-type guard. Use to *read* a page; `web_search` finds pages, `http_request` returns raw bytes |
+| `web_fetch` | Fetch a URL and return its main content as clean, readable text/markdown (strips nav/scripts/styling; preserves `<pre>`/`<code>` and transcodes non-UTF-8 charsets). Read-only GET, egress-controlled (refuses if no egress client is present — no `DefaultTransport` fallback), with redirect + size caps and a content-type guard. A non-2xx response still returns the error page's content with its `status`. Use to *read* a page; `web_search` finds pages, `http_request` returns raw bytes |
 | `file_create` | Create a downloadable file, written to the agent's `.forge/files/` directory |
 | `read_skill` | Load full instructions for an available skill on demand |
 | `memory_search` | Search long-term memory (when enabled) |
diff --git a/forge-core/tools/builtins/web_fetch.go b/forge-core/tools/builtins/web_fetch.go
index bdac10a5..407ab32a 100644
--- a/forge-core/tools/builtins/web_fetch.go
+++ b/forge-core/tools/builtins/web_fetch.go
@@ -1,6 +1,7 @@
 package builtins
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -10,6 +11,7 @@ import (
 	"time"
 
 	"golang.org/x/net/html"
+	"golang.org/x/net/html/charset"
 
 	"github.com/initializ/forge/forge-core/credentials"
 	"github.com/initializ/forge/forge-core/security"
@@ -116,8 +118,20 @@ func (t *webFetchTool) Execute(ctx context.Context, args json.RawMessage) (strin
 		}
 	}
 
+	// Defense-in-depth: never fall back to http.DefaultTransport. When no
+	// egress client is installed, EgressTransportFromContext returns nil and a
+	// bare http.Client would silently bypass the allowlist / SSRF protections
+	// entirely. web_fetch is a default-on, LLM-URL-driven surface — the exact
+	// shape where a fail-open is worst — so refuse instead. The runtime always
+	// installs the egress client via WithEgressClient before tools run (#266
+	// review). (http_request still fails open at this seam; hardening both at
+	// a shared seam is tracked in #308.)
+	transport := security.EgressTransportFromContext(ctx)
+	if transport == nil {
+		return "", fmt.Errorf("web_fetch: refusing to fetch without egress enforcement (no egress client in context)")
+	}
 	client := &http.Client{
-		Transport:     security.EgressTransportFromContext(ctx),
+		Transport:     transport,
 		Timeout:       webFetchTimeout,
 		CheckRedirect: security.SafeRedirectPolicy(webFetchMaxRedirects),
 	}
@@ -144,11 +158,23 @@ func (t *webFetchTool) Execute(ctx context.Context, args json.RawMessage) (strin
 		return "", fmt.Errorf("reading response: %w", err)
 	}
 
+	// Transcode to UTF-8. Older spec/RFC pages — squarely this tool's
+	// wheelhouse — are often ISO-8859-1 or declare a charset= parameter;
+	// string(raw) would return mojibake. charset.NewReader reads the
+	// Content-Type charset, a <meta charset>, or a BOM (#266 review). On any
+	// detection/transcode error we keep the raw bytes rather than fail.
+	text := string(raw)
+	if r, cerr := charset.NewReader(bytes.NewReader(raw), contentType); cerr == nil {
+		if decoded, rerr := io.ReadAll(r); rerr == nil {
+			text = string(decoded)
+		}
+	}
+
 	var content string
 	if kind == contentHTML {
-		content = extractReadableText(string(raw))
+		content = extractReadableText(text)
 	} else {
-		content = strings.TrimSpace(string(raw))
+		content = strings.TrimSpace(text)
 	}
 
 	truncated := false
@@ -249,16 +275,27 @@ func extractReadableText(htmlContent string) string {
 		b.WriteString("# " + title + "\n\n")
 	}
 
+	// preDepth > 0 inside <pre>/<code>: emit text VERBATIM (keep newlines +
+	// indentation) so fetched code samples / config snippets survive intact,
+	// instead of collapsing to one run of tokens (#266 review).
+	preDepth := 0
 	var walk func(*html.Node)
 	walk = func(n *html.Node) {
 		switch n.Type {
 		case html.TextNode:
-			b.WriteString(collapseInlineWS(n.Data))
+			if preDepth > 0 {
+				b.WriteString(n.Data)
+			} else {
+				b.WriteString(collapseInlineWS(n.Data))
+			}
 			return
 		case html.ElementNode:
 			if skipTags[n.Data] {
 				return
 			}
+			if n.Data == "pre" || n.Data == "code" {
+				preDepth++
+			}
 			switch {
 			case isHeading(n.Data):
 				b.WriteString("\n\n" + strings.Repeat("#", int(n.Data[1]-'0')) + " ")
@@ -274,6 +311,9 @@ func extractReadableText(htmlContent string) string {
 			walk(c)
 		}
 		if n.Type == html.ElementNode {
+			if n.Data == "pre" || n.Data == "code" {
+				preDepth--
+			}
 			if n.Data == "a" {
 				if href := attr(n, "href"); isHTTPURL(href) {
 					b.WriteString(" (" + href + ")")
diff --git a/forge-core/tools/builtins/web_fetch_test.go b/forge-core/tools/builtins/web_fetch_test.go
index bdd3b9b5..cc38adfa 100644
--- a/forge-core/tools/builtins/web_fetch_test.go
+++ b/forge-core/tools/builtins/web_fetch_test.go
@@ -9,13 +9,27 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/initializ/forge/forge-core/credentials"
+	_ "github.com/initializ/forge/forge-core/credentials/static" // register the "static" provider
 	"github.com/initializ/forge/forge-core/security"
 )
 
+// egressCtx installs a permissive egress client (DefaultTransport) so tests can
+// reach the localhost httptest server. web_fetch REFUSES when no egress client
+// is present (fail-closed), so every real-fetch test must install one.
+func egressCtx() context.Context {
+	return security.WithEgressClient(context.Background(), &http.Client{Transport: http.DefaultTransport})
+}
+
 func runWebFetch(t *testing.T, ctx context.Context, args map[string]any) (map[string]any, error) {
+	t.Helper()
+	return runWebFetchTool(t, &webFetchTool{}, ctx, args)
+}
+
+func runWebFetchTool(t *testing.T, tool *webFetchTool, ctx context.Context, args map[string]any) (map[string]any, error) {
 	t.Helper()
 	raw, _ := json.Marshal(args)
-	out, err := (&webFetchTool{}).Execute(ctx, raw)
+	out, err := tool.Execute(ctx, raw)
 	if err != nil {
 		return nil, err
 	}
@@ -44,7 +58,7 @@ func TestWebFetch_ExtractsReadableText(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	m, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL})
+	m, err := runWebFetch(t, egressCtx(), map[string]any{"url": srv.URL})
 	if err != nil {
 		t.Fatalf("Execute: %v", err)
 	}
@@ -76,7 +90,7 @@ func TestWebFetch_TruncatesAtMaxChars(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	m, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL, "max_chars": 100})
+	m, err := runWebFetch(t, egressCtx(), map[string]any{"url": srv.URL, "max_chars": 100})
 	if err != nil {
 		t.Fatalf("Execute: %v", err)
 	}
@@ -101,7 +115,7 @@ func TestWebFetch_PlainTextPassthrough(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	m, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL})
+	m, err := runWebFetch(t, egressCtx(), map[string]any{"url": srv.URL})
 	if err != nil {
 		t.Fatalf("Execute: %v", err)
 	}
@@ -119,7 +133,7 @@ func TestWebFetch_RejectsBinaryContentType(t *testing.T) {
 	}))
 	defer srv.Close()
 
-	_, err := runWebFetch(t, context.Background(), map[string]any{"url": srv.URL})
+	_, err := runWebFetch(t, egressCtx(), map[string]any{"url": srv.URL})
 	if err == nil {
 		t.Fatal("expected web_fetch to reject image/png")
 	}
@@ -160,3 +174,130 @@ func TestExtractReadableText_MalformedHTML(t *testing.T) {
 		}
 	}
 }
+
+// TestExtractReadableText_PreservesPreformatted is the #266-review fix: <pre>/
+// <code> content keeps its newlines + indentation instead of collapsing to one
+// run of tokens — the tool's headline use case is reading docs, where code
+// blocks live.
+func TestExtractReadableText_PreservesPreformatted(t *testing.T) {
+	got := extractReadableText("<html><body><pre>func main() {\n    fmt.Println(\"hi\")\n}</pre></body></html>")
+	if !strings.Contains(got, "func main() {\n") {
+		t.Errorf("pre newlines collapsed:\n%q", got)
+	}
+	if !strings.Contains(got, "\n    fmt.Println") {
+		t.Errorf("pre indentation collapsed:\n%q", got)
+	}
+	// A normal paragraph outside <pre> still collapses incidental whitespace.
+	para := extractReadableText("<p>one   two\n\tthree</p>")
+	if strings.Contains(para, "   ") {
+		t.Errorf("non-pre whitespace should collapse: %q", para)
+	}
+}
+
+// TestWebFetch_RefusesWithoutEgressClient pins the fail-CLOSED behavior: with no
+// egress client in context, web_fetch refuses rather than falling back to
+// http.DefaultTransport (which would bypass the allowlist / SSRF protections).
+func TestWebFetch_RefusesWithoutEgressClient(t *testing.T) {
+	_, err := (&webFetchTool{}).Execute(context.Background(), []byte(`{"url":"https://example.com"}`))
+	if err == nil || !strings.Contains(err.Error(), "egress enforcement") {
+		t.Fatalf("expected a refusal without an egress client, got %v", err)
+	}
+}
+
+// TestClassifyContentType covers the guard's branches, including the documented
+// empty-Content-Type → HTML behavior and the binary rejections.
+func TestClassifyContentType(t *testing.T) {
+	cases := []struct {
+		ct       string
+		wantKind contentKind
+		wantOK   bool
+	}{
+		{"", contentHTML, true}, // documented: empty CT treated as HTML
+		{"text/html; charset=utf-8", contentHTML, true},
+		{"application/xhtml+xml", contentHTML, true},
+		{"text/plain", contentText, true},
+		{"text/markdown", contentText, true},
+		{"application/json", contentText, true},
+		{"application/rss+xml", contentText, true},
+		{"image/png", contentText, false},
+		{"application/pdf", contentText, false},
+		{"application/octet-stream", contentText, false},
+	}
+	for _, c := range cases {
+		k, ok := classifyContentType(c.ct)
+		if ok != c.wantOK || (ok && k != c.wantKind) {
+			t.Errorf("classifyContentType(%q) = (%d,%v), want (%d,%v)", c.ct, k, ok, c.wantKind, c.wantOK)
+		}
+	}
+}
+
+// TestWebFetch_TranscodesCharset pins the charset fix: an ISO-8859-1 page
+// (common for older spec/RFC pages) comes back as valid UTF-8, not mojibake.
+func TestWebFetch_TranscodesCharset(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "text/html; charset=iso-8859-1")
+		// "café latte" with é as the Latin-1 byte 0xE9.
+		_, _ = w.Write([]byte("<html><body><p>caf\xe9 latte</p></body></html>"))
+	}))
+	defer srv.Close()
+
+	m, err := runWebFetch(t, egressCtx(), map[string]any{"url": srv.URL})
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	if content, _ := m["content"].(string); !strings.Contains(content, "café latte") {
+		t.Errorf("ISO-8859-1 not transcoded to UTF-8: %q", content)
+	}
+}
+
+// TestWebFetch_StampsInjectedHeaders pins that the R9 JIT credential injector's
+// materialized headers reach the outbound request (same plumbing as
+// http_request).
+func TestWebFetch_StampsInjectedHeaders(t *testing.T) {
+	got := make(chan string, 1)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		got <- r.Header.Get("X-Api-Token")
+		w.Header().Set("Content-Type", "text/plain")
+		_, _ = w.Write([]byte("ok"))
+	}))
+	defer srv.Close()
+
+	inj, err := credentials.NewInjector(context.Background(), credentials.DefaultRegistry,
+		[]credentials.CredentialSpec{{
+			Tool:     "web_fetch",
+			Provider: "static",
+			Spec:     json.RawMessage(`{"headers":{"X-Api-Token":"secret123"}}`),
+		}}, nil)
+	if err != nil {
+		t.Fatalf("NewInjector: %v", err)
+	}
+
+	tool := (&webFetchTool{}).WithCredentialInjector(inj)
+	if _, ferr := runWebFetchTool(t, tool, egressCtx(), map[string]any{"url": srv.URL}); ferr != nil {
+		t.Fatalf("Execute: %v", ferr)
+	}
+	if h := <-got; h != "secret123" {
+		t.Errorf("injected header not stamped on the request: got %q", h)
+	}
+}
+
+// TestWebFetch_ByteCapBoundsRawRead pins the 2 MiB pre-extraction byte cap: a
+// response far larger than the cap yields bounded content even when max_chars
+// is effectively unlimited.
+func TestWebFetch_ByteCapBoundsRawRead(t *testing.T) {
+	big := strings.Repeat("A", 3<<20) // 3 MiB > webFetchByteLimit (2 MiB)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "text/plain")
+		_, _ = w.Write([]byte(big))
+	}))
+	defer srv.Close()
+
+	m, err := runWebFetch(t, egressCtx(), map[string]any{"url": srv.URL, "max_chars": 100_000_000})
+	if err != nil {
+		t.Fatalf("Execute: %v", err)
+	}
+	content, _ := m["content"].(string)
+	if len(content) > webFetchByteLimit {
+		t.Errorf("content %d bytes exceeds the byte cap %d", len(content), webFetchByteLimit)
+	}
+}