fix(webfetch): handle bodies without dump

[minpeter]

Summary

• feature-detect BodyReadable.dump() so Bun-compatible response bodies can be discarded safely
• preserve bounded Undici draining and prevent teardown-only stream errors from becoming uncaught exceptions
• add Bun redirect regression coverage and make the explicit-article test runner-compatible

Verification

• bun run check
• bun test (42 passed)
• vitest --run (41 passed)
• live webfetch.execute against the 1Password CLI docs (HTTP 200, 27,395 markdown bytes)
• local HTML redirect scenario under Bun and Node

[minpeter]

CI is currently marked action_required because this fork-origin workflow needs maintainer approval before it can run. @code-yeongyu, please approve the workflow run when convenient: https://github.com/code-yeongyu/pi-webfetch/actions/runs/32966116500. Local validation is clean: Bun 42/42, Node/Vitest 41/41, typecheck and Biome pass.

[code-yeongyu]

Verdict

REQUEST_CHANGES. The body-disposal implementation is directionally correct and passes the local Bun and Node/Vitest suites, but the explicit-article regression test no longer proves the behavior named by the test. That is a blocking test gap because the regression can re-enter production unnoticed.

Blockers

1. test/webfetch-explicit-article.test.ts:3-33 - The changed test is no longer capable of failing when Readability.parse() is called. The mock's parse() throws an Error, but extractReadableArticle() catches Error and returns undefined; htmlToMarkdown() then falls back to Turndown on the original HTML, which still contains both strings asserted by the test. Thus the test passes despite the exact regression it claims to cover. Restore a direct call-count assertion, or make an accidental parse throw a runner-compatible non-Error sentinel that propagates, while retaining the output assertions.

Non-blocking notes

• src/webfetch/fetcher.ts:48-52,212-233 - The optional dump?() contract is feature-detected before calling it, and destroyDiscardedBody() is used both when dump is absent and when dump rejects with an Error. No other call site assumes dump exists. Rethrowing non-Error dump failures preserves the previous behavior and avoids swallowing arbitrary thrown values. In Undici 7.28.0, destroy() and destroy(error) both abort an active, undrained body/socket; the changed call primarily changes the emitted error identity, not reuse semantics for this discard path.

• src/webfetch/fetcher.ts:229-233 - The one-shot error listener is appropriate for the deliberate no-consumer teardown and does not mask a normal consumer mid-read, since it is only installed by discardBody(). If the helper is ever called repeatedly on the same already normally-closed body, however, once("error", noop) can remain attached because no error will be emitted and destroy() is a no-op; a destroyed/closed guard or listener cleanup would avoid that latent leak. This is not currently reachable through the production call graph, which discards each response body once.

• src/webfetch/fetcher.test.ts:43-56 and test/webfetch.test.ts:566-591 - The new tests are real integration tests rather than mocks asserting mock calls. Under Bun, the actual Undici body has no dump(), so the redirect test and the new fetcher test exercise the no-dump branch; under Node, the same redirect test exercises bounded dump() draining. The redirect test therefore does reach discardBody(), but it does not directly assert that the discarded stream was closed or that the connection was reusable.

• The new src/webfetch/fetcher.test.ts is included by bun test but excluded by the Vitest config (test/**/*.test.ts), so the Bun-specific no-dump regression is not covered by the repository's Node CI workflow. This is a coverage/CI note rather than an implementation failure. Local results: Bun 1.4.0 reported 42 passed, 0 failed; npx vitest --run on Node v26.7.0 reported 41 passed, 0 failed; typecheck and Biome were clean.

• The cubic check is NEUTRAL because its review quota was exhausted; it provided no code findings to evaluate. The GitHub CI workflow is also awaiting maintainer approval for this fork-origin run, which is unrelated to the patch.

[code-yeongyu]

Verdict

APPROVE. The round-1 blocker is resolved: the explicit-article regression test now asserts the Readability parse call count, so it fails when the fallback runs. The local mutation check failed under both Bun and Vitest, and the restored implementation passed under both.

Blockers

None.

Non-blocking notes

• The fix commit 313d476 touches only test/webfetch-explicit-article.test.ts; no unrelated or source changes were introduced by that commit.
• Full gates passed: bun test 42 passed / 0 failed; npx vitest --run 41 passed / 0 failed; npm run typecheck passed; npx biome check . passed with no fixes.

[code-yeongyu]

Merging with admin override on the required node-20 contexts. Evidence:

• PR head CI (mirror run 32987862083): node 22 SUCCESS on ubuntu+macos; node 20 FAILURE on both with webidl.util.markAsUncloneable is not a function (undici CacheStorage via @earendil-works/pi-coding-agent).
• Unmodified main baseline (run 32988042922): identical failure pattern — node 20 fails the same way without this PR.
• The node-20 breakage is a pre-existing dependency incompatibility unrelated to this PR's diff and will be addressed separately.
• Note: PR-triggered runs were wedged by a stuck queued run holding the concurrency group (run 32985589782, uncancellable); CI was executed via a head-sha mirror branch instead.