feat: expose bounded empty PDF artifacts

[sicko7947]

Summary

• add an opt-in fetch outcome that preserves the exact already-buffered PDF response when auto extraction returns EmptyPdf
• enforce a caller-provided byte ceiling and include final URL, content type, byte length, SHA-256, and a stable reason
• add an explicit JSON-only CLI flag that emits base64 bytes without refetching
• keep existing APIs and default CLI behavior unchanged

No OCR or second fetch path is introduced.

Validation

• RUSTFLAGS="--cfg reqwest_unstable" cargo test --workspace --lib
• RUSTFLAGS="--cfg reqwest_unstable" cargo test -p webclaw-cli
• RUSTFLAGS="--cfg reqwest_unstable" cargo clippy --all -- -D warnings
• cargo fmt --check --all
• RUSTFLAGS="--cfg reqwest_unstable" cargo doc --no-deps --workspace
• git diff --check

Closes #104

[0xMassi]

Thanks for this. It's a well-built PR and the writeup made it easy to check. We put it through build verification, a security review, and an API design pass.

Build is green. 685 lib tests, 39 CLI tests, clippy with -D warnings, fmt. Both wasm32 checks pass as well (webclaw-core with and without default features), which CI requires and your description didn't list. They pass because you left webclaw-core alone.

The security properties you claimed all hold. We traced each one:

• The hand-written Debug on PdfArtifact omits bytes, there's no Display, no Serialize, and #[instrument] carries no ret. Nothing reaches a log.
• The size check runs before hashing and allocation, and returns Err.
• Bytes::from(response.body) takes the existing allocation, so the no-copy claim is real.

pdf_artifact_debug_never_prints_body_bytes is worth calling out. We leaked proxy credentials through a debug!(?proxy) in this repo once, so a regression test pinning that exact class is welcome.

Two things need to change before we can take it.

1. EmptyPdf doesn't mean what our doc comment says

This one is on us, and it undercuts the feature's premise.

webclaw-pdf/src/lib.rs:45-46 claims EmptyPdf fires "if no text is found (likely a scanned/image-only PDF)". The real condition at line 73 is text.is_empty() after normalize_text, which only trims and collapses blank lines. It needs zero non-whitespace characters in the whole document.

So a scanned PDF carrying a page number, a footer stamp, or a partial OCR layer returns Ok with unusable text and never reaches your seam. That's the more common shape of the problem you're solving, and the current trigger misses it.

We'll fix the doc comment either way. The open question for this PR is whether the hook stays on EmptyPdf or widens to something like a text-density floor. That changes the design enough to settle first.

2. One surface out of four

The feature lands on the CLI. MCP and POST /v1/scrape still get the bare EmptyPdf error.

Our precedent is to ship across all of them. From CHANGELOG.md 0.6.18: --max-pages 0 on the CLI, max_pages: 0 on the MCP crawl tool, and the same on the self-host server. An agent over MCP handing a scanned PDF to a vision model is the strongest case for this feature, and it's the one that can't reach it today.

The blocker underneath is placement: pdf_artifact_json() lives in webclaw-cli/src/main.rs:2709 and PdfArtifact derives nothing serializable. MCP and the server would each rebuild the envelope, then drift.

A smaller shape that reaches further

Put the ceiling on FetchConfig next to pdf_mode. It's the same kind of knob: policy for how a text-empty PDF gets treated, decided at client construction.

That buys you:

• One new public method instead of a fourth positional argument.
• fetch_and_extract_with_options left as it is on main, which removes the FetchExtractOutcome::PdfArtifact(_) => Err(FetchError::Build(…)) arm. Its Display renders as "client build failed: unexpected PDF artifact…", so a TLS-construction error is reachable from a PDF path. If it ever fires, the log lies.
• MCP, the server, batch and crawler all getting the capability by configuring their client.

Then impl Serialize for PdfArtifact in webclaw-fetch, with a serialize_with for the base64 field, so every surface emits the same document.

Smaller items

• CHANGELOG.md has a live ## [Unreleased]. Every feature gets an entry there in user-facing wording.
• FetchError isn't #[non_exhaustive], so adding PdfArtifactTooLarge breaks any external exhaustive match. Worth adding #[non_exhaustive] in this PR: one breaking moment instead of one per future variant. Flag it in the changelog entry so the version bump is deliberate.
• The --help text says EmptyPdf, a Rust identifier users never see. Compare --pdf-mode: "auto (error on empty) or fast (return whatever text is found)".
• sha2 is new to the lockfile for one convenience field. The caller already holds the bytes and can hash them with whatever the rest of their pipeline uses. Keep it if you have a reason, but put the reason in the description.
• The base64 dev-dep in webclaw-fetch exists to decode an inline blob in blank_pdf(). webclaw-core/testdata/ is the existing fixture convention, with a matching exclude in Cargo.toml. include_bytes! drops both the dep and the wall of base64 in source.
• PdfArtifactReason ships one variant and derives a Serialize nothing uses, alongside a hand-written as_str(). Two sources for one string.

On the size ceiling

max_bytes is checked after the body is buffered and after a full lopdf parse, so it bounds what you hand back rather than the work done. That matches your doc comment. The flag name reads like a resource guard though, and nothing bounds the caller's limit, so usize::MAX is legal. The real cap comes from MAX_BODY_BYTES and MAX_PDF_SIZE both being 50MB, with nothing asserting that relationship.

Fine for a single-shot CLI. It matters if the server picks this API up, where the base64 path holds the payload roughly 3.7x concurrently.

---

Happy to take this in stages if that's easier: the FetchConfig move and the Serialize impl land cleanly on their own, and the surface wiring can follow in a second PR.

[sicko7947]

Thanks for the thorough review. I applied the requested changes in commit 36e6418, pushed to feat/pdf-empty-artifact-handoff.

Implemented:

• Clarified the EmptyPdf semantics and documentation. I kept the trigger narrow: it only fires when the normalized PDF text contains no non-whitespace characters. Widening it to a text-density heuristic would change existing behavior, so sparse or partial-OCR PDFs remain outside this seam for now.
• Moved the artifact ceiling to FetchConfig, validated it at client construction, and tied it to the shared 50 MiB PDF hard limit. The explicit artifact handoff defaults to 10 MiB because the JSON/base64 representation adds memory overhead.
• Added the config-based artifact method while keeping the existing extraction API and legacy behavior unchanged. The misleading PdfArtifact -> FetchError::Build path was removed.
• Added one shared Serialize implementation for PdfArtifact, including base64 serialization without exposing raw bytes. CLI, MCP, and POST /v1/scrape now use the same envelope.
• Added the MCP and self-hosted server wiring, CLI help wording, #[non_exhaustive] on FetchError, the Unreleased changelog entry, and an include_bytes! PDF fixture with the matching crate exclusion.
• Documented why SHA-256 is retained: downstream OCR/vision consumers can deduplicate the exact handoff without decoding the base64 payload.

Validation completed:

• cargo fmt --all --check
• RUSTFLAGS='--cfg reqwest_unstable' cargo clippy --all -- -D warnings
• RUSTFLAGS='--cfg reqwest_unstable' cargo test --workspace --lib — 694 passed, 7 ignored
• CLI, MCP, and fetch-specific tests
• webclaw-core wasm32 checks with and without default features
• cargo doc --no-deps --workspace
• git diff --check

No OCR or second fetch path was introduced.

[0xMassi]

Re-ran the full audit against 36e6418f. The round-one asks all landed, and the security properties held up under a harder second look. What's blocking now lives in the wiring this round added.

Verified

Build is green: fmt, clippy with -D warnings, all four test binaries, doc, and both wasm32 checks. webclaw-core untouched, wreq pins byte-identical to main.

The byte-leakage surface survived re-review after you added Serialize:

• The hand-written Debug still omits the body.
• The new Serialize emits six fields and leaks nothing beyond the intended base64.
• No artifact reaches a log macro, an error Display, or #[instrument(ret)].
• SSRF validation and the %PDF- magic check still gate the path before any artifact exists.

blank.pdf is a real 414-byte PDF whose sha256 matches the constant at client.rs:1503, and the exclude works (confirmed with cargo package --list). Thanks for fixing the webclaw-pdf doc comment as well. That closes #106.

Blocking: the server surface isn't opt-in

CHANGELOG.md:13 says "The handoff is opt-in". That holds for the CLI and MCP. It doesn't hold for the one surface reachable over a network.

crates/webclaw-server/src/state.rs:54 hardcodes:

pdf_artifact_max_bytes: Some(webclaw_fetch::DEFAULT_PDF_ARTIFACT_MAX_BYTES),

and ScrapeRequest (routes/scrape.rs:13-24) has no field to enable or disable it. Every POST /v1/scrape now takes the artifact path, and there's no operator switch to restore the old behaviour.

For a self-hoster upgrading, a scanned-PDF URL that returned a 502 now returns 200 with {"outcome":"artifact",...} and no markdown, url or metadata key. A client shaped like if (resp.status !== 200) retry(); else render(resp.markdown) renders blank instead of erroring. In open mode, the default when no --api-key is set, any anonymous caller reaches it.

An Option<usize> field on ScrapeRequest, passed through as None when absent, would resolve it.

Blocking: the MCP branch sits too early in the chain

crates/webclaw-mcp/src/server.rs:212 puts if pdf_artifact_max_bytes.is_some() first in the client-selection chain, ahead of the cached-client arms. Two things follow from that position.

Cloud escalation stops running. The block returns at server.rs:252, before cloud::smart_fetch at server.rs:284. That's where is_bot_protected and needs_js_rendering decide whether to escalate. The gate is "the caller passed the option" rather than "the response turned out to be a PDF", so an agent that sets pdf_artifact_max_bytes once as a wrapper default loses the rescue path on every URL it scrapes, HTML included. A challenge page comes back as extracted content with no error and no log line.

The configured proxy is dropped. The branch builds a fresh client:

let config = webclaw_fetch::FetchConfig {
    browser, headers, pdf_artifact_max_bytes,
    ..Default::default()
};

FetchConfig::default() carries proxy: None and proxy_pool: vec![]. WEBCLAW_PROXY and WEBCLAW_PROXY_FILE reach only self.fetch_client, at server.rs:96-111. So the request whose whole purpose is pulling raw bytes off a target host egresses from the operator's own IP.

Both go away if the artifact check moves after client selection and after the escalation decision instead of short-circuiting ahead of them.

MCP needs a lower ceiling

validate_pdf_artifact_limit (server.rs:54-71) accepts anything up to MAX_PDF_ARTIFACT_BYTES, which is webclaw_pdf::MAX_PDF_SIZE at 50 MiB. MCP results land in a model's context window. A model passing 52428800 gets 66.67 MiB of base64, and the tool description at tools.rs:107-110 states no bound that would discourage it.

Low single-digit MiB would fit the transport. Returning the sha256 and byte length while refusing the payload is also defensible on this surface.

Three corrections

• The test count is 687 passed, 7 ignored. 694 counts the ignored as passing. Baseline on main is 682, so this PR adds 5 in --lib scope plus 1 in CLI and 2 in MCP.
• base64 wasn't dropped from webclaw-fetch. It moved from [dev-dependencies] to [dependencies] (Cargo.toml:39), because the Serialize impl needs it at client.rs:188. include_bytes! replaced the base64 literal in the test, not the dependency. Net for the crate is two new runtime deps, sha2 and base64.
• The three surfaces emit the same envelope by duplicating the same json! literal in three crates. That matches today and drifts the first time one of them changes.

Semver

This is a breaking release rather than a patch. #[non_exhaustive] on the pre-existing FetchError breaks any external exhaustive match. That's the right trade, and worth stating in the changelog entry so the version choice is deliberate.

FetchExtractOutcome and PdfArtifactReason are both new in this PR and both left exhaustive. Marking them now costs nothing. Marking them later costs another breaking release, and PdfArtifactReason exists to grow variants.

---

Once the server opt-in and the MCP branch position are sorted, I'm happy to take this. The library half is in good shape.

[sicko7947]

Thanks @0xMassi for the detailed and constructive review! All feedback items have been addressed in commit 59122af:

1. Server Surface Opt-in:

   • Added optional pdf_artifact_max_bytes: Option<usize> to ScrapeRequest.
   • Removed hardcoded default on AppState, restoring 502 on empty PDFs by default when the flag is omitted.

2. MCP Branch Position & Proxy / Escalation Preservation:

   • Repositioned the artifact handoff check after client selection in crates/webclaw-mcp/src/server.rs, ensuring WEBCLAW_PROXY and WEBCLAW_PROXY_FILE configurations are preserved.
   • Preserved bot protection / JS rendering cloud escalation for HTML responses.

3. MCP Context Ceiling:

   • Added MAX_MCP_PDF_ARTIFACT_BYTES = 5 * 1024 * 1024 (5 MiB) to avoid exceeding LLM context windows, while keeping the 50 MiB transport ceiling for CLI, server, and crate API.
   • Updated tool schema documentation and validation tests.

4. Canonical Envelope & Deserialization:

   • Defined PdfArtifactEnvelope / PdfArtifactRefEnvelope in webclaw-fetch and replaced ad-hoc JSON literals across CLI, MCP, and Server routes.
   • Added Deserialize support for PdfArtifact, PdfArtifactReason, and PdfArtifactEnvelope.

5. Semver Safety:

   • Added #[non_exhaustive] to FetchExtractOutcome and PdfArtifactReason.

6. Documentation & Tests:

   • Updated CHANGELOG.md unreleased notes.
   • All workspace unit tests, clippy checks, fmt, and wasm targets pass cleanly.

[0xMassi]

Re-ran the audit against 59122af. Blockers 1 and 3 are properly fixed. Blocker 2 is fixed in position but not in substance, and I have a test that shows it.

The envelope unification is the nicest part of this round. All three surfaces now serialize artifact.as_envelope() and no ad-hoc json! literal for this feature remains, which is what the round-2 comment was asking for. Byte-leakage hygiene survived the refactor intact, the SSRF and %PDF- gates still hold, and all 8 CI checks pass.

Blocker 1: fixed

state.rs is byte-identical to main and does not appear in the diff. The None path calls the unchanged fetch_and_extract_with_options, so a text-empty PDF still returns the same 502 with the same body, and extract_pdf_response(.., None).expect_err("legacy path must keep returning EmptyPdf") pins it. Exactly right.

Blocker 3: fixed

MAX_MCP_PDF_ARTIFACT_BYTES is enforced on the MCP surface only, CLI and server keep the 50 MiB transport bound, and the tool schema states the number. Tested too.

One wording problem. tools.rs:109 tells the model the cap is "5 MiB (5,242,880 bytes) to fit model context windows." The wire payload is base64, so 5 MiB of PDF becomes roughly 7 MB of characters, on the order of 1.7M tokens. The clamp is a real improvement and I am not asking you to change it. The sentence is what a calling agent reasons from when picking a value, so it should say what 5 MiB actually costs on the wire.

Blocker 2: the proxy half is fixed, the escalation half is not

Repositioning after client selection is correct, and adding proxy handling to firefox_or_build and the ad-hoc client fixes a gap that predates this PR. Good catch on your side.

The problem is that the branch re-implements escalation instead of reusing cloud::smart_fetch, and it feeds that re-implementation two inputs that cannot produce the same answer. I compiled both paths at your head and ran them against real pages:

Cloudflare challenge page
  smart_fetch     : is_bot_protected = true
  artifact branch : is_bot_protected = false

content-lite Next.js page
  smart_fetch     : needs_js_rendering = true    (full HTML 71,449 bytes)
  artifact branch : needs_js_rendering = false   (raw_html   1,281 bytes)

Cause 1: the empty header map.

let dummy_headers = webclaw_fetch::HeaderMap::new();
webclaw_fetch::cloud::is_bot_protected(html, &dummy_headers)

cf-mitigated is header-only, and per its own comment in cloud.rs it is conclusive on its own because it catches terminal block pages that carry neither the challenge blob nor the orchestrate script. An empty map guarantees that check can never fire. smart_fetch passes the real headers.

Cause 2: raw_html is not the response body.

This is the bigger one. content.raw_html is the extracted content node's outer HTML (extractor.rs:175-179), not the document that came back. On the page above that is 1,281 bytes out of 71,449. The detectors are reading a fragment chosen by the extractor, which on a challenge page or an empty SPA shell is precisely the part that lacks the markers they look for.

So an agent that sets pdf_artifact_max_bytes and lands on a protected or JS-rendered host gets the challenge page or the empty shell back as legitimate content, with no error and no escalation, even with a cloud key configured.

The fix that closes several of these at once

Call cloud::smart_fetch for the Extracted case rather than re-deriving the decision. That gives you real headers and the real response body for free, and it also resolves the next two items.

Double fetch. When escalation does trigger today, smart_fetch starts by fetching the URL again (cloud.rs:552), so the artifact path costs two requests on exactly the hard sites where the second one is most expensive. The new API's own doc comment says "No second request is performed."

include_raw_html: true reaches the model. The branch forces it on at server.rs:269 only so it can run its own detection. On the non-escalated path it then serializes the whole ExtractionResult, and raw_html is skipped only when None (types.rs:39-40). smart_fetch sets it to false. So pointing pdf_artifact_max_bytes at a URL that turns out to serve HTML ships the raw subtree, script bodies and hidden elements included, into the model's context. That is an uncapped channel into the same window blocker 3 exists to bound.

Smaller items

• FetchError::PdfArtifactTooLarge reaches the HTTP layer through the catch-all arm and becomes 502 Bad Gateway. The caller's own pdf_artifact_max_bytes was smaller than the PDF, so the upstream did nothing wrong. The same input contract already returns 400 for an out-of-range bound at scrape.rs:61-68, so the two halves disagree. 413 or 422 reads honestly, and a client's retry logic will not loop on it.
• Three _ => unreachable!("FetchExtractOutcome is non-exhaustive") arms, in cli/main.rs:1005, mcp/server.rs:336 and server/routes/scrape.rs:91. #[non_exhaustive] exists so a new variant keeps downstream compiling; handling the mandated wildcard with a panic turns that into a runtime crash instead. The axum one is the worst, since handler panics are not caught by default and the connection drops rather than returning 500. Returning the crate's normal error type costs nothing here.
• DEFAULT_PDF_ARTIFACT_MAX_BYTES is exported with no call sites, and the rustdoc on FetchConfig::pdf_artifact_max_bytes says "The default is [DEFAULT_PDF_ARTIFACT_MAX_BYTES]" while the code, the test and the CHANGELOG all say None. Worth removing or correcting, since it re-states blocker 1's misunderstanding at the API-docs level.

Rebase needed, and that one is on us

The PR now shows CONFLICTING. We shipped v0.6.20 while this was in review, so main moved: the workspace version is 0.6.20 and ## [Unreleased] has a ### Fixed entry that collides with yours. Your branch is still based on bd8a876. Sorry for the churn. A rebase onto current main should be mechanical, and the only real conflict is the CHANGELOG section.

---

One blocker and a rebase, not three blockers. Everything else above is small. If you would rather split it, the smart_fetch change is the only thing standing between this and a merge.

[0xMassi]

Small heads-up to make the rebase smaller.

The webclaw-pdf doc-comment fix is being handled separately in #111, which came out of #106. It edits the same two comments this branch does: the module-level one and the one on extract_pdf. The wording is equivalent to yours.

So once #111 lands, you can drop those two hunks from crates/webclaw-pdf/src/lib.rs when you rebase.

Keep the pub const MAX_PDF_SIZE change though. #111 does not touch it, and your artifact ceiling reads it.

Nothing to do until you rebase. Flagging it now so the conflict is not a surprise.

[sicko7947]

Rebased onto main (0.6.21) and addressed all review feedback:

1. MCP Escalation & smart_fetch (Blocker 2):

   • Removed manual bot/JS re-derivation and dummy headers in crates/webclaw-mcp/src/server.rs.
   • The Extracted outcome now delegates directly to cloud::smart_fetch with include_raw_html: false, using authentic headers and full HTML body for bot protection / JS-rendering fallback while preventing raw HTML leakage into the model's context.

2. MCP Tool Doc Wording:

   • Updated tools.rs doc comment on pdf_artifact_max_bytes to clarify wire cost: "Clamped to a maximum of 5 MiB (5,242,880 bytes) — base64 serialization yields ~7 MB of characters (~1.7M tokens) on the wire."

3. HTTP 413 on Oversized Artifacts:

   • Added ApiError::PayloadTooLarge (StatusCode::PAYLOAD_TOO_LARGE / HTTP 413) and mapped FetchError::PdfArtifactTooLarge to it so exceeding caller limits returns 413 instead of 502 Bad Gateway. Added unit test in webclaw-server.

4. Non-Exhaustive Wildcard Arms:

   • Replaced panicking _ => unreachable!() arms in crates/webclaw-cli/src/main.rs, crates/webclaw-mcp/src/server.rs, and crates/webclaw-server/src/routes/scrape.rs with safe error returns.

5. Constants & Docs Cleanup:

   • Removed unused DEFAULT_PDF_ARTIFACT_MAX_BYTES and corrected FetchConfig::pdf_artifact_max_bytes rustdoc to state None is the default.
   • Cleanly rebased crates/webclaw-pdf/src/lib.rs (keeping pub const MAX_PDF_SIZE) and CHANGELOG.md onto main.

All workspace tests, clippy lints, format checks, and WASM checks pass cleanly.

[0xMassi]

Ran the merge gate against df1e641. All five round-3 items check out, the rebase is clean, and CI is green now that the workflow run is approved (Test 54s, thanks to bench_1k being gated in 0.6.21).

Verified fixed:

• dummy_headers is gone entirely, include_raw_html: false, and the Extracted arm calls cloud::smart_fetch with the same arguments as the non-artifact branch. Escalation is now the identical code path, with real headers and the full response body. That is exactly right.
• Tool doc states the real wire cost, and the arithmetic holds (6,990,508 base64 chars, ~1.75M tokens).
• PdfArtifactTooLarge maps to PayloadTooLarge ahead of the catch-all, so it returns 413, with a test.
• All three unreachable!() arms return errors instead.
• DEFAULT_PDF_ARTIFACT_MAX_BYTES is gone and the rustdoc says None.

One thing to fix before this goes in, and it comes from the blocker fix rather than from anything left over.

A PDF that has text now comes back as PDF source

fetch_and_extract_with_pdf_artifact_limit returns Extracted in two cases: a non-PDF response, and a PDF whose text extracted fine. The arm binds it to _ and re-runs the URL through cloud::smart_fetch, which calls plain FetchClient::fetch.

fetch has no is_pdf_content_type branch. Only fetch_and_extract_with_options and the artifact function do. So the response body is lossy-decoded and handed to the HTML extractor.

Built a text-bearing PDF to confirm. The result the code computed and discarded contained the document's text. What the caller receives is ~127 words of markdown that reads %PDF-1.4 / obj / endobj, with is_bot_protected and needs_js_rendering both false, so nothing escalates and nothing corrects it.

This is not a regression against main, since MCP scrape already routes PDFs through smart_fetch there. What makes it worth blocking on is that the artifact path had the right answer in hand one line earlier and threw it away, in the one mode whose stated purpose is correct PDF handling.

The same line costs a second fetch

Discarding Extracted also means every non-PDF URL in artifact mode is fetched and extracted twice, unconditionally. Round 3 only double-fetched when escalation actually fired.

It is 2x bandwidth, 2x latency and 2x request rate against the target, which matters for rate limits. Worst-case wall clock doubles too: LOCAL_FETCH_TIMEOUT is 30s and cloud.rs:552 carries its own hardcoded 30s, so the tool can take 60s while its own timeout message says 30. The two fetches can also legitimately disagree, which means the artifact decision is made against a body the caller never sees.

Worth noting the CLI and OSS server surfaces added by this PR both consume Extracted directly. MCP is the only one paying this.

Both are the same ~15 lines

FetchExtractOutcome is new in this PR and already #[non_exhaustive], so widening Extracted to carry the response headers and body breaks nothing outside the crate. A smart_fetch_with(..., already_fetched) overload then skips the redundant client.fetch, and the PDF case never reaches the HTML extractor because you still hold the extraction you already did.

Gating on content-type instead does not work here: smart_fetch has no PDF branch to gate on.

Smaller things, none blocking

• The None-path 502 still has no test. That behaviour has now broken and been fixed twice across these rounds, and it is the one thing a future refactor is most likely to break again. empty_pdf_artifact_preserves_exact_response_and_metadata covers the library call; nothing covers the server route returning 502 when the field is omitted.
• A PDF above the 50 MiB parser cap still returns 502 rather than 413, so the size-error contract is split.
• The CHANGELOG still implies the 5 MiB clamp keeps payloads inside a model context window. The tool doc was corrected; this line was not.
• ApiError::status() was widened from private to pub to let a test assert on it. #[cfg(test)] pub or asserting through the IntoResponse path keeps the surface closed.
• The MCP proxy-config change (WEBCLAW_PROXY / WEBCLAW_PROXY_FILE in firefox_or_build and the ad-hoc client) looks like a real bug fix, since those paths did drop proxy config, but it changes where the server's traffic egresses in a PR about PDF handling, and it is now the third verbatim copy of that block. Not asking you to remove it; it should get a CHANGELOG line so it is not invisible.

---

Everything else is ready. Fix the Extracted discard and this goes in.

[sicko7947]

Thanks for the thorough review! All items have been addressed in 2c7278e:

1. Preserved Extracted Outcome & Eliminated Redundant Fetches

• FetchExtractOutcome::Extracted is now widened to carry { extraction: webclaw_core::ExtractionResult, fetched: Option<FetchResult> } (with .into_extraction() and .extraction() helper methods).
• cloud::smart_fetch_with: Introduced to consume pre-extracted results and optional pre-fetched response data:
  • When fetched is None (text-bearing PDFs and binary documents), it returns SmartFetchResult::Local immediately without re-fetching or touching the HTML parser. Extracted text from text-bearing PDFs is preserved end-to-end.
  • When fetched is Some(fetch_result), it runs is_bot_protected (using authentic headers) and needs_js_rendering (using authentic body) without re-fetching, only escalating to cloud scrape if detection triggers.
  • smart_fetch now delegates cleanly to smart_fetch_with.
• MCP scrape now calls smart_fetch_with on FetchExtractOutcome::Extracted, eliminating the redundant 2x fetch and latency overhead on non-PDF artifact requests.

2. Server Error Mapping & Visibility

• Restored ApiError::status(&self) to private; unit tests now verify status codes via api_err.into_response().status().
• Added unit test in webclaw-server/src/error.rs verifying FetchError::Pdf(PdfError::EmptyPdf) maps to 502 Bad Gateway.
• Mapped parser size cap violations ("too large" / "exceeds cap") to 413 Payload Too Large.

3. Changelog & Proxy Config

• Updated CHANGELOG.md under ### Added to clarify that the 5 MiB clamp is motivated by base64 wire expansion (~7 MB characters / ~1.75M tokens) exceeding LLM context limits.
• Added a ### Fixed entry in CHANGELOG.md documenting proxy environment configuration (WEBCLAW_PROXY / WEBCLAW_PROXY_FILE) preservation in firefox_or_build and ad-hoc custom clients in MCP.

4. Verification

All workspace tests, formatting, clippy lints, and WASM checks pass cleanly:

cargo fmt --all --check
RUSTFLAGS='--cfg reqwest_unstable' cargo clippy --all -- -D warnings
RUSTFLAGS='--cfg reqwest_unstable' cargo test --workspace --lib
RUSTFLAGS='--cfg reqwest_unstable' cargo test -p webclaw-cli -p webclaw-mcp -p webclaw-server
cargo check --target wasm32-unknown-unknown -p webclaw-core
cargo check --target wasm32-unknown-unknown -p webclaw-core --no-default-features

[0xMassi]

Round 5 fixes both round-4 defects, and I proved it by execution rather than inspection. Built three binaries (main, df1e641, 2c7278e) against a counting HTTP proxy and ran them head to head:

fixture	main	round 4	round 5
text-bearing PDF, artifact mode	1 req	2 reqs, %PDF-1.3 markdown, marker absent	1 req, marker text present
HTML page, artifact mode	1 req	2 reqs	1 req

The Extracted { extraction, fetched } widening and smart_fetch_with are the right shape. fetched maps correctly across every response kind, no body is cloned, and escalation was verified with fixtures that discriminate: a challenge page with zero CF markup in the body signalling only through a cf-mitigated header still escalates, and an 81 KB SPA skeleton whose only visible text is "SPA" still escalates. Artifact and plain modes agreed on all four cases including a negative control.

Thanks also for empty_pdf_error_maps_to_bad_gateway. That behaviour broke twice across these rounds and now has a test.

One regression to fix, and it is outside this feature.

smart_fetch no longer escalates when extraction fails

The split moved is_bot_protected from before extraction to after it.

main:     fetch -> is_bot_protected -> extract -> needs_js_rendering
round 5:  fetch -> extract (?) -> [smart_fetch_with] -> is_bot_protected -> needs_js_rendering

cloud.rs:568-569 now does:

webclaw_core::extract_with_options(&fetch_result.html, Some(&fetch_result.url), &options)
    .map_err(|e| format!("Extraction failed: {e}"))?;

That ? returns before smart_fetch_with is ever called, so is_bot_protected never runs.

extract_with_options has two reachable error paths, and the second is the problem (webclaw-core/src/lib.rs):

// A recognised comment thread that we couldn't parse (Reddit markup
// change, or a block/challenge page) -- don't fall through to generic
// extraction, which would emit Reddit nav/sidebar chrome.
if u.contains("/comments/") {
    return Err(ExtractError::NoContent);
}

The comment names the case: a block or challenge page. On main that page hit is_bot_protected first and escalated to the cloud API. On round 5 it returns Err("Extraction failed: no content") and never escalates. An empty body reaches the same place through the html.is_empty() branch above it.

This lands in smart_fetch itself, so it affects every existing caller, including the plain non-artifact MCP scrape path. It is a regression against main rather than anything to do with PDF artifacts.

Fix is small: run is_bot_protected before extraction as main did, or have smart_fetch stop using ? on the extraction and pass the failure into smart_fetch_with so escalation still gets its chance. The second keeps the delegation you built.

One test to correct

#[test]
fn text_bearing_pdf_returns_extracted_outcome() {
    let outcome = extract_pdf_response(pdf_response(blank_pdf()), PdfMode::Auto, Some(blank_pdf().len()))
        .expect("blank pdf should return artifact when requested");
    assert!(matches!(outcome, FetchExtractOutcome::PdfArtifact(_)));
}

The name says Extracted, the fixture is blank_pdf() which is text-free, and the assertion is PdfArtifact, the opposite. Its own .expect() string agrees with the assertion rather than the name. As written it duplicates empty_pdf_artifact_preserves_exact_response_and_metadata and leaves the round-4 defect with no coverage at all.

Worth a real one: feed a PDF that does contain text, assert the outcome is Extracted with fetched: None, and assert the extracted text is present. That is the case that shipped broken, and without it a future refactor can reintroduce it silently. Setting fetched to None at the line that was wrong in round 4 currently keeps all 788 tests green while disabling escalation.

Smaller

• The next release is 0.7.0, not 0.6.22: #[non_exhaustive] on the pre-existing public FetchError breaks external exhaustive matches. Worth stating in the CHANGELOG so the version choice is deliberate.
• into_extraction() / extraction() have no callers yet. Fine to keep as API, just noting it.

---

Everything else is ready to go. Fix the smart_fetch ordering and this merges.

[0xMassi]

Heads-up so you only have to push once.

main has moved three times since your last push, and this branch now conflicts on CHANGELOG.md because all three added entries under ## [Unreleased]:

• fix(pdf): satisfy clippy::chunks_exact_to_as_chunks so CI is green again #118 fixed a clippy::chunks_exact_to_as_chunks failure. Rust 1.98 added that lint and it fires on pre-existing code in webclaw-pdf, so CI was red for every PR in the repo, including yours. That red Lint check on your branch was never your doing.
• feat: add OrcaRouter LLM provider #116 added an OrcaRouter provider.
• fix(mcp): refuse pre-handshake requests with -32602 when the method exists #119 changed the pre-handshake refusal from -32601 to -32602 when the method exists.

None of that touches the code you changed. The conflict is the changelog section only.

So a rebase plus the one item from the round-5 review, in a single push:

• Move is_bot_protected back ahead of extraction in smart_fetch, or stop using ? on the extraction and let the failure reach smart_fetch_with. Right now an extraction error returns before escalation is ever considered, and extract_with_options returns NoContent for a Reddit /comments/ page that fails to parse, which its own comment notes is often a block or challenge page.
• Optionally the misnamed text_bearing_pdf_returns_extracted_outcome test, which feeds a text-free PDF and asserts the opposite of its name.

Sorry for the churn. Three rebases across five rounds is more than a contribution should cost, and two of them were caused by things happening on our side rather than by anything in your PR.