diff --git a/docs/superpowers/plans/2026-08-12-architecture-deepening-phase-5-extraction-seam.md b/docs/superpowers/plans/2026-08-12-architecture-deepening-phase-5-extraction-seam.md index 1b36da8..bd708bb 100644 --- a/docs/superpowers/plans/2026-08-12-architecture-deepening-phase-5-extraction-seam.md +++ b/docs/superpowers/plans/2026-08-12-architecture-deepening-phase-5-extraction-seam.md @@ -2,417 +2,206 @@ > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Make registered fetch providers part of one content-extraction fallback seam and remove the duplicate direct Jina Reader implementation. +## Readiness decision -**Architecture:** Keep routing and trust-sensitive HTTP handling in `src/extract/pipeline.ts`. Move shared result types into `src/extract/types.ts`. Add `src/extract/fallbacks.ts` as the seam that adapts registered `FetchProvider`s and Gemini extractors to the ordered extraction chain. `web_fetch` supplies the already registered, budget-wrapped fetch candidates and phase 3 execution hooks. The Jina transport remains only in `src/providers/jina.ts`. +The previous plan was not ready against the clean post-Phase-4 repository. The current branch is `20260814-architecture-deepening-phase-5-extraction-seam` at `74d988c`; `pnpm check` passes 89 test files and 1,465 tests. The environment still reports Node 23.11.0 versus the package requirement of Node >=24.15.0, and the repository has its existing Biome warnings. -**Tech Stack:** TypeScript, native `fetch`, existing `executeWithFallback`, SSRF utilities, Gemini extractors, Vitest. +The gaps were: ---- +- `pipeline.ts` has no registered-provider options, while `web-fetch.ts` owns a second fallback path only for retryable transport failures. The plan did not specify how the shared adapter handles network/429/5xx failures without accidentally entering Gemini/raw extraction. +- The current pipeline wraps caller cancellation as `RetryableExtractionError`; the plan only described cancellation inside the new adapter. The GET, probe, Cloudflare retry, and transport-fallback boundaries need explicit cancellation checks. +- The draft chain behavior omitted the no-provider marker and claimed to record per-provider failure plus success labels even though `executeWithFallback` returns only the winning provider. Attempt details already belong in activity/metrics; the extraction chain will record only tier-level failure and the winning provider. +- Moving result types without updating type-only consumers would leave the new seam coupled to `pipeline.ts`. The type-only imports need to move while `pipeline.ts` keeps compatibility re-exports. +- Existing Jina tests use content shorter than the planned 100-character provider contract, and existing web-fetch fallback tests cover the old outer path rather than the new in-pipeline path. + +## Goal + +Make registered fetch providers part of one content-extraction fallback seam and remove the duplicate direct Jina Reader implementation. ## Atomic result -After this phase, the normal HTML path is: +For a validated HTML response, the normal path is: ```text validated HTTP response → Readability → RSC → registered fetch-provider fallback - → Gemini URL context - → Gemini Web + → Gemini URL context, then Gemini Web → raw text ``` -The structured routes (GitHub, PDF, YouTube, local video, raw mode), SSRF validation, per-hop redirect validation, cancellation, cache/storage behavior, and retryable transport errors remain intact. A provider fallback is budgeted and instrumented through the registry wrapper and shared execution policy. `src/extract/jina-reader.ts` and its duplicate tests are removed. - -## File map - -Create: - -- `src/extract/types.ts` — shared `ExtractedContent`, `VideoFrame`, and `ImageBlock` types. -- `src/extract/fallbacks.ts` — ordered extraction fallback adapter and registered-fetch adapter. -- `tests/extract/fallbacks.test.ts` — fallback ordering, empty-result, cancellation, and provider-adapter tests. - -Modify: - -- `src/extract/pipeline.ts` — use shared types and fallback seam; remove direct Jina Reader call. -- `src/providers/jina.ts` — keep one Jina Reader transport and normalize empty/short content for the provider seam. -- `src/tools/web-fetch.ts` — pass registered fetch candidates and execution hooks into `extractContent`; remove the outer duplicate fallback. -- `tests/extract/pipeline.test.ts` — preserve normal, retryable, raw, PDF, and HTTP behavior with the new options. -- `tests/extract/pipeline-routing.test.ts` — replace direct Jina markers with registered-fetch seam markers and test ordering. -- `tests/extract/pipeline-ssrf.test.ts` — verify provider fallback is reached only after original URL validation. -- `tests/extract/cloudflare-retry.test.ts` — preserve challenge retry behavior. -- `tests/providers/jina.test.ts` — test the single Jina provider fetch path. -- `tests/tools/web-fetch.test.ts` — verify provider fallback, ordering, hooks, and error behavior through the pipeline. - -Delete: - -- `src/extract/jina-reader.ts` — duplicate direct Jina Reader transport. -- `tests/extract/jina-reader.test.ts` — tests for the deleted duplicate module; equivalent coverage moves to `fallbacks.test.ts` and `providers/jina.test.ts`. - -## Tasks - -### Task 1: Define the extraction fallback seam without changing routing +For a retryable direct transport failure (network error, 429, or 5xx), the pipeline tries only the registered fetch-provider fallback and then preserves `RetryableExtractionError` if no provider succeeds. It never enters the Gemini/raw HTML path without a response body. -**Files:** +Structured routes (GitHub, PDF, YouTube, local video, raw mode), SSRF validation, per-hop redirect validation, cancellation, cache/storage behavior, and sanitized tool errors remain intact. `src/extract/jina-reader.ts` and its duplicate tests are deleted; `src/providers/jina.ts` owns the only `r.jina.ai` request. -- Create: `src/extract/types.ts` -- Create: `src/extract/fallbacks.ts` -- Create: `tests/extract/fallbacks.test.ts` -- Modify: `src/extract/pipeline.ts` -- Modify: `src/tools/web-fetch.ts` +## Locked contracts -- [ ] **Step 1: Move shared extraction result types.** +### Fallback seam -Create `src/extract/types.ts` with the data currently declared in `pipeline.ts`: +Create `src/extract/fallbacks.ts` with the smallest reusable interface: ```ts -import type { PdfPageImage } from "./pdf-ocr.ts"; - -export interface VideoFrame { - data: string; - mimeType: string; - timestamp: string; -} - -export interface ExtractedContent { - text: string; - title?: string; - url: string; - extractionChain: string[]; - chars: number; - truncated: boolean; - contentId?: string; - thumbnail?: { data: string; mimeType: string }; - frames?: VideoFrame[]; - images?: PdfPageImage[]; - duration?: number; -} - -export type ImageBlock = { type: "image"; data: string; mimeType: string }; -``` - -In `pipeline.ts`, import these types and re-export them: - -```ts -export type { ExtractedContent, ImageBlock, VideoFrame } from "./types.ts"; -``` - -This keeps existing tool/test imports working while allowing the new fallback module to depend on the result contract without importing pipeline runtime code. - -- [ ] **Step 2: Define an ordered fallback adapter.** - -Create `src/extract/fallbacks.ts`: - -```ts -import type { ExecutionHooks } from "../providers/execute.ts"; -import { executeWithFallback } from "../providers/execute.ts"; -import type { FetchProvider } from "../providers/types.ts"; -import type { ExtractedContent } from "./types.ts"; - export interface ExtractionFallback { name: string; run(): Promise; } - -export function createFetchProviderFallback( - providers: readonly FetchProvider[], - url: string, - signal: AbortSignal | undefined, - hooks?: ExecutionHooks, -): ExtractionFallback | undefined { - if (providers.length === 0) return undefined; - - return { - name: "fetch-provider", - async run() { - const { result, providerName } = await executeWithFallback({ - candidates: providers.map((provider) => ({ - name: provider.name, - execute: async () => { - const fetched = await provider.fetch(url, signal); - const text = fetched.text.trim(); - if (text.length === 0) throw new Error(`${provider.name} returned empty content`); - return { fetched, providerName: provider.name }; - }, - })), - operation: "fetch", - signal, - ...hooks, - }); - - const text = result.fetched.text.trim(); - return { - text, - title: result.fetched.title, - url, - extractionChain: [`fetch-provider:${result.providerName}`], - chars: text.length, - truncated: false, - }; - }, - }; -} - -export async function runExtractionFallbacks( - fallbacks: readonly ExtractionFallback[], - chain: string[], - signal?: AbortSignal, -): Promise { - for (const fallback of fallbacks) { - signal?.throwIfAborted(); - try { - const result = await fallback.run(); - signal?.throwIfAborted(); - if (!result) { - chain.push(`${fallback.name}:fail`); - continue; - } - - chain.push(result.extractionChain[0] ?? fallback.name); - return { ...result, extractionChain: chain }; - } catch (error) { - signal?.throwIfAborted(); - chain.push(`${fallback.name}:error`); - // The next adapter gets a chance. executeWithFallback has already - // aggregated and instrumented individual registered provider failures. - } - } - return undefined; -} ``` -The implementation may use a narrower internal return type, but it must preserve these rules: - -- empty provider text advances to the next provider; -- `AbortError`/caller cancellation is rethrown, never converted into a fallback failure; -- registered provider attempts use `executeWithFallback` so budgets, activity, metrics, and aggregate errors remain centralized; and -- a failed adapter adds a chain marker but does not expose raw provider credentials or unsanitized errors. - -- [ ] **Step 3: Add seam tests before pipeline wiring.** +`createFetchProviderFallback()` adapts the already selected, registry-wrapped `FetchProvider[]`. It must: -In `tests/extract/fallbacks.test.ts`, add tests that prove: +- call providers in supplied order through `executeWithFallback({ operation: "fetch" })`; +- trim provider text and treat empty text as a failed attempt; +- return normalized `ExtractedContent` with `fetch-provider:`, the original URL, title, chars, and `truncated: false`; +- pass the supplied `AbortSignal` and `ExecutionHooks` to the shared executor; and +- keep provider errors out of the returned content and extraction chain. -1. providers are tried in the supplied order; -2. an empty first result advances to the second provider; -3. a successful provider returns `fetch-provider:` and preserves title/chars/url; -4. all provider failures allow the next Gemini-style adapter to run; -5. a caller abort rejects with the abort error and does not call later adapters; and -6. execution hooks receive provider success/failure while `BudgetExceededError` remains excluded from failure metrics. +`runExtractionFallbacks()` checks the caller signal before and after each adapter. A caller-aborted signal is rethrown and never converted into a fallback failure or sent to a later adapter. Ordinary adapter errors advance the chain: -Mock `executeWithFallback` only if needed for deterministic adapter tests; retain at least one test with real registered-style provider functions so the budget/execution boundary is exercised by the phase gate. +- `null` result: `:fail`; +- thrown result: `:error`; and +- successful result: append the result’s first extraction-chain marker and return. -- [ ] **Step 4: Add provider candidates and hooks to `ExtractOptions`.** +When no fetch providers are supplied, `pipeline.ts` appends `fetch-provider:skipped` and does not create an adapter. When providers are supplied but all fail, the chain contains `fetch-provider:error`; individual attempt failures remain in shared activity/metrics. A first-provider failure followed by a successful second provider records only `fetch-provider:` in the extraction chain. -Extend the existing options in `pipeline.ts`: +Gemini URL context and Gemini Web are one `gemini-html` adapter that tries URL context first and Web second. This preserves the existing high-level markers: the successful adapter contributes `html:gemini-url-context` or `html:gemini-web`, and an all-null result contributes `gemini-html:fail`. -```ts -import type { ExecutionHooks } from "../providers/execute.ts"; -import type { FetchProvider } from "../providers/types.ts"; +### Pipeline and transport behavior -export interface ExtractOptions { - // existing fields... - fetchProviders?: readonly FetchProvider[]; - executionHooks?: ExecutionHooks; -} -``` +In `extractContent()`: -Do not make the extraction module select raw provider registry entries. It receives already selected, registry-wrapped candidates from `web_fetch`. +1. Check `signal?.throwIfAborted()` before routing and retain the existing frame, video, YouTube, original-URL SSRF, GitHub, probe, direct HTTP, Cloudflare retry, binary, PDF, raw, Readability, and RSC ordering. +2. After original `validateUrl()` succeeds, create the fetch adapter from `options.fetchProviders` but do not invoke it before validation. +3. On a network error from the GET or Cloudflare retry, check caller cancellation, then run only the fetch adapter. On 429/5xx, do the same after the status is known. A successful transport fallback returns its provider-normalized result with its provider-only chain marker, matching the former `web_fetch` fallback result. +4. If no provider succeeds, throw `RetryableExtractionError` based on the original transport failure. An optional provider summary must use `sanitizeError`; it must not expose credentials or raw provider errors. A provider budget rejection is not a failure metric and must not replace the original retryable classification. +5. For a successful non-PDF response whose Readability and RSC tiers fail, run the registered fetch adapter, then the combined Gemini adapter, then raw text. Do not invoke registered providers for raw mode, binary/oversized responses, PDFs, GitHub structured results, YouTube, or local video results. -- [ ] **Step 5: Pass the seam inputs from `web_fetch`.** +The direct GET/Cloudflare catches and the probe catch must rethrow caller cancellation before wrapping ordinary transport errors. SSRF errors remain `SSRFError`; redirect hops remain validated by `fetchValidated()`. -In `src/tools/web-fetch.ts`: +### Type ownership -- keep `resolveFetchCandidates` as the existing resolver boundary; -- resolve candidates immediately before calling `extractContent`; -- pass `fetchProviders: resolveFetchCandidates?.() ?? []` and the phase 3 execution hooks; and -- remove the `RetryableExtractionError` catch that separately calls `executeWithFallback`. +`src/extract/types.ts` owns `ExtractedContent`, `VideoFrame`, and `ImageBlock`. `pipeline.ts` re-exports those types so existing imports remain valid. Type-only consumers move to the new module: `gemini-url-context.ts`, `cache.ts`, `youtube.ts`, `video.ts`, and `frames.ts`. `ExtractOptions` stays in `pipeline.ts` because it describes pipeline routing inputs rather than the extracted result contract. -The `fetchUrl()` body should have one extraction call and one cache write. Keep the current cache key, content store writes, multi-URL concurrency, image blocks, and sanitized `errorResult` behavior. +### Jina provider -Run: +`JinaProvider.fetch()` remains the only `r.jina.ai` transport. It trims its response and throws `Jina reader returned insufficient content` when the result is shorter than 100 characters. Authorization and search behavior remain unchanged. -```bash -pnpm exec vitest run tests/extract/fallbacks.test.ts tests/tools/web-fetch.test.ts -``` - -Expected: the new seam and existing provider fallback tests pass after the initial pipeline integration. - -### Task 2: Wire the ordered seam into the pipeline and remove duplicate Jina - -**Files:** - -- Modify: `src/extract/pipeline.ts` -- Modify: `src/providers/jina.ts` -- Delete: `src/extract/jina-reader.ts` -- Delete: `tests/extract/jina-reader.test.ts` -- Modify: `tests/extract/pipeline.test.ts` -- Modify: `tests/extract/pipeline-routing.test.ts` -- Modify: `tests/providers/jina.test.ts` - -- [ ] **Step 1: Preserve all pre-fallback validation and direct routes.** - -Keep these pipeline sections in their current order and behavior: - -1. frame extraction options; -2. local video routing; -3. YouTube routing; -4. original URL SSRF validation; -5. GitHub structured extraction; -6. HEAD probe and binary/size checks; -7. validated HTTP GET with per-hop redirect validation; -8. Cloudflare retry; and -9. raw/PDF/direct HTML/RSC extraction. - -Do not pass unvalidated URLs to a registered fetch provider. The original `validateUrl()` must still run before any provider fallback, and `fetchValidated()` must continue validating every redirect hop for direct HTTP requests. - -- [ ] **Step 2: Replace the direct Jina tier with registered adapters.** - -After `chain.push("rsc:no-match")`, construct the fallback list: - -```ts -const fallbacks = [ - createFetchProviderFallback( - options?.fetchProviders ?? [], - url, - signal, - options?.executionHooks, - ), - { - name: "gemini-url-context", - run: () => extractWithUrlContext(url, signal), - }, - { - name: "gemini-web", - run: () => extractWithGeminiWeb(url, signal), - }, -].filter((fallback): fallback is NonNullable => Boolean(fallback)); - -const fallbackResult = await runExtractionFallbacks(fallbacks, chain, signal); -if (fallbackResult) return fallbackResult; -``` - -Then keep the existing raw-text fallback and final error, using the accumulated `chain`. Remove `extractViaJinaReader` import and all `jina-reader` chain markers. For a call with no registered providers, the chain should identify the skipped provider group consistently (for example `fetch-provider:skipped`) before Gemini; choose one marker and update tests to assert that exact contract. +## File map -- [ ] **Step 3: Preserve retryable transport fallback semantics.** +Create: -When the initial validated GET or Cloudflare retry fails with a network error, 429, or 5xx, call only the registered fetch-provider fallback with the original URL before throwing. If a provider succeeds, return its normalized `ExtractedContent`. If all providers fail, throw a `RetryableExtractionError` so callers and tests can still distinguish retryable transport failure from a permanent HTTP error. +- `src/extract/types.ts` — shared result types. +- `src/extract/fallbacks.ts` — registered-fetch adapter and ordered fallback runner. +- `tests/extract/fallbacks.test.ts` — seam ordering, normalization, cancellation, hooks, and budget behavior. -Do not use the raw/Gemini HTML path when there is no response body. Preserve SSRF errors as `SSRFError`, preserve caller cancellation, and do not turn `BudgetExceededError` into a retryable network error. The error message may include a short sanitized fallback summary, but the error class and URL redaction rules must remain intact. +Modify: -- [ ] **Step 4: Make the provider own Jina Reader transport.** +- `src/extract/pipeline.ts` — shared types/options, transport fallback, ordered HTML seam, and cancellation checks. +- `src/extract/gemini-url-context.ts` — import result types from `types.ts`. +- `src/extract/youtube.ts` — import result types from `types.ts`; retain `ExtractOptions` from `pipeline.ts`. +- `src/extract/video.ts` — import result types from `types.ts`; retain `ExtractOptions` from `pipeline.ts`. +- `src/extract/frames.ts` — import `VideoFrame` from `types.ts`. +- `src/cache.ts` — import `ExtractedContent` from `extract/types.ts`. +- `src/providers/jina.ts` — normalize and reject insufficient fetch content. +- `src/tools/web-fetch.ts` — pass candidates/hooks into `extractContent` and remove the outer fallback. +- `tests/extract/pipeline.test.ts` — transport fallback, cancellation, and chain contracts. +- `tests/extract/pipeline-routing.test.ts` — registered-provider ordering before Gemini. +- `tests/extract/pipeline-ssrf.test.ts` — provider never runs before original URL validation. +- `tests/extract/cloudflare-retry.test.ts` — preserve challenge/retry and retryable fallback behavior. +- `tests/providers/jina.test.ts` — fetch success, short-content, non-2xx, and authorization behavior. +- `tests/tools/web-fetch.test.ts` — one pipeline fallback path, hooks, errors, cache, and multi-URL behavior. +- `tests/tools/web-fetch-video.test.ts` — preserve extraction options while passing seam inputs. -Keep `JinaProvider.fetch()` as the only `r.jina.ai` request. Normalize its text before returning: +Delete: -```ts -const text = (await response.text()).trim(); -if (text.length < 100) { - throw new Error("Jina reader returned insufficient content"); -} -return { text }; -``` +- `src/extract/jina-reader.ts` — duplicate direct Jina transport. +- `tests/extract/jina-reader.test.ts` — tests for the deleted module. -Retain its authorization header behavior and existing search behavior. Do not add a second direct fetch helper in the extraction module. The provider is already wrapped by `ProviderRegistry`, so its fetch budget and outcome hooks apply automatically. +No changes are required in `src/index.ts` or `tests/helpers.ts`: Phase 3 already supplies registry-wrapped fetch candidates and hooks, and Phase 4’s mock host already replaces same-name tools. -- [ ] **Step 5: Update extraction-chain and Jina tests.** +## Tasks -Replace direct Jina-reader unit tests with: +### Task 1: Move result types and build the fallback seam -- fallback seam tests for adapter ordering and normalized `fetch-provider:` output; -- provider tests for Jina success, non-2xx error, short-content rejection, and authorization; and -- pipeline routing tests that inject a mock registered fetch provider and assert it runs before Gemini. +Files: `src/extract/types.ts`, `src/extract/fallbacks.ts`, `tests/extract/fallbacks.test.ts`, plus the listed type-only consumers. -Keep the existing thin HTML, Gemini URL context, Gemini Web, raw text, 4xx, 429, 5xx, and cancellation assertions. Only change assertions that named the deleted direct `jina-reader` implementation. +- [ ] **Step 1: Move and re-export result types.** Create `types.ts` with the current `ExtractedContent`, `VideoFrame`, and `ImageBlock` definitions. Remove their definitions from `pipeline.ts`, import them there, and re-export them. Update the type-only consumers listed in the file map. +- [ ] **Step 2: Implement the registered-fetch adapter.** Use `executeWithFallback` for every registered attempt. Trim text, reject empty results, preserve title/URL/chars, and pass signal/hooks. +- [ ] **Step 3: Implement the ordered runner.** Enforce signal checks and the `fail`/`error`/success chain contract. Do not expose aggregate provider messages through extraction content. +- [ ] **Step 4: Add seam tests.** Prove supplied ordering, empty-result advancement, normalization, title/chars/url, second-provider success, all-provider failure, caller cancellation stopping later adapters, success/failure hooks, and `BudgetExceededError` exclusion from failure hooks. Run: ```bash -pnpm exec vitest run tests/extract/fallbacks.test.ts tests/extract/pipeline.test.ts tests/extract/pipeline-routing.test.ts tests/providers/jina.test.ts tests/tools/web-fetch.test.ts +pnpm exec vitest run tests/extract/fallbacks.test.ts ``` -Expected: focused extraction and provider tests pass, with no import or request path left for `src/extract/jina-reader.ts`. - -### Task 3: Prove security, structured-route, and multi-URL invariants - -**Files:** - -- Modify: `tests/extract/pipeline-ssrf.test.ts` -- Modify: `tests/extract/cloudflare-retry.test.ts` -- Modify: `tests/tools/web-fetch.test.ts` -- Modify: `tests/tools/web-fetch-video.test.ts` -- Modify: `tests/extract/pipeline.test.ts` - -- [ ] **Step 1: Test SSRF and redirect boundaries with provider candidates.** - -Add a test that supplies a mock fetch provider while requesting a private, link-local, or disallowed URL. Assert `extractContent` rejects with `SSRFError` before the provider’s `fetch` function is called. Keep the existing tests for redirect-to-private-IP and redirect-loop rejection; direct HTTP requests must still validate every hop. - -- [ ] **Step 2: Test fallback ordering and no duplicate Jina request.** - -For thin HTML, provide a mock provider and mocked Gemini adapters. Assert: - -1. Readability and RSC are attempted first; -2. the registered provider is called before either Gemini adapter; -3. Gemini is not called after a successful provider; and -4. there is no direct `r.jina.ai` request outside the Jina provider mock. - -For a provider failure, assert the next provider/Gemini adapter runs and the final extraction chain records the failure and success labels in order. +### Task 2: Wire the seam into the pipeline and remove duplicate Jina -- [ ] **Step 3: Test retryable HTTP and budget behavior.** +Files: `src/extract/pipeline.ts`, `src/providers/jina.ts`, `src/extract/jina-reader.ts`, `tests/extract/jina-reader.test.ts`, and the focused pipeline/provider tests. -For 429/5xx/network failure, assert a successful registered provider result is returned. When all registered providers fail, assert `RetryableExtractionError`. When the registry wrapper throws `BudgetExceededError`, assert no provider failure metric is recorded and the retryable class remains intact where no provider can serve the request. +- [ ] **Step 1: Add options and a single fetch adapter instance.** Extend `ExtractOptions` with `fetchProviders?: readonly FetchProvider[]` and `executionHooks?: ExecutionHooks`. Create the adapter only after original URL validation so transport and HTML paths share the same registry-wrapped candidates. +- [ ] **Step 2: Preserve direct routes and validation.** Keep the existing route order, HEAD probe, binary/size checks, manual redirect validation, PDF/OCR, raw mode, and Cloudflare retry. Add caller-signal checks at pipeline entry, probe error handling, and GET/Cloudflare catches. +- [ ] **Step 3: Handle retryable transport failures.** On network errors, 429, and 5xx, invoke only the registered fetch adapter. Return its normalized result on success; otherwise preserve `RetryableExtractionError` based on the original failure and sanitize any provider summary. +- [ ] **Step 4: Replace direct Jina/Gemini calls for thin HTML.** After `rsc:no-match`, append `fetch-provider:skipped` when appropriate, run registered providers, then the combined Gemini adapter, then raw text. Remove the `jina-reader` import and markers. +- [ ] **Step 5: Move Jina ownership.** Delete the duplicate extraction module and test. Keep `JinaProvider.fetch()` as the only Reader request, trim successful content, and reject content shorter than 100 characters. -- [ ] **Step 4: Test structured routes bypass generic fallbacks.** +### Task 3: Update extraction and tool tests around the new boundary -Keep tests proving provider candidates are not called for successful YouTube, local video, GitHub, PDF, and raw-mode routes. Existing video frame and PDF OCR behavior must remain unchanged. Do not make provider fetch adapters handle binary content, frames, or PDFs. +Files: `tests/extract/pipeline.test.ts`, `tests/extract/pipeline-routing.test.ts`, `tests/extract/pipeline-ssrf.test.ts`, `tests/extract/cloudflare-retry.test.ts`, `tests/providers/jina.test.ts`, `tests/tools/web-fetch.test.ts`, and `tests/tools/web-fetch-video.test.ts`. -- [ ] **Step 5: Test cache and multi-URL behavior.** - -Keep the existing `web_fetch` tests for cache hits, `fresh`, duplicate URLs, concurrency limit, content IDs, image blocks, and partial failures. Assert the resolver is called for each actual uncached URL and the provider fallback result is stored exactly like direct extraction content. - -Run: - -```bash -pnpm exec vitest run tests/extract/pipeline-ssrf.test.ts tests/extract/cloudflare-retry.test.ts tests/extract/pipeline.test.ts tests/tools/web-fetch.test.ts tests/tools/web-fetch-video.test.ts -``` - -Expected: security and structured-route tests pass without weakening validation or changing cache/storage contracts. - -### Task 4: Complete the phase gate and commit - -- [ ] **Step 1: Search for duplicate Jina and outer fallback paths.** +- [ ] **Step 1: Test normal HTML ordering.** With thin HTML and mocked Gemini adapters, assert Readability/RSC precede registered providers, providers precede Gemini, a successful provider stops the chain, and all-failure markers are sanitized/high-level only. +- [ ] **Step 2: Test retryable transport behavior.** Cover network, 429, and 5xx success through a registered provider; all-provider failure as `RetryableExtractionError`; no Gemini/raw attempt without a response body; and budget rejection without a failure metric. +- [ ] **Step 3: Test cancellation.** Abort before and during direct transport and during provider fallback. Assert the caller’s abort is preserved, no later provider runs, and no provider failure hook records cancellation. +- [ ] **Step 4: Test security and structured routes.** Verify the provider is not called for an invalid original URL, redirect-to-private-IP, binary/oversized response, PDF, GitHub, YouTube, local video, or raw mode. Keep per-hop redirect validation assertions. +- [ ] **Step 5: Preserve web-fetch behavior.** Verify the single extraction call receives resolved candidates/hooks, provider fallback results use the same cache/content-store/rendering path, and multi-URL deduplication, concurrency, partial failures, fresh mode, image blocks, and sanitized errors remain unchanged. +- [ ] **Step 6: Update Jina tests.** Use a valid long fetch fixture, assert trim and title-independent text, short-content rejection, non-2xx errors, and authorization headers for both search and Reader fetches. Run: ```bash -rg -n "extractViaJinaReader|jina-reader|r\.jina\.ai|executeWithFallback|RetryableExtractionError" src/extract src/tools/web-fetch.ts +pnpm exec vitest run \ + tests/extract/fallbacks.test.ts \ + tests/extract/pipeline.test.ts \ + tests/extract/pipeline-routing.test.ts \ + tests/extract/pipeline-ssrf.test.ts \ + tests/extract/cloudflare-retry.test.ts \ + tests/providers/jina.test.ts \ + tests/tools/web-fetch.test.ts \ + tests/tools/web-fetch-video.test.ts ``` -Expected: `r.jina.ai` appears only in `src/providers/jina.ts` and its provider tests; `web-fetch.ts` no longer owns a second provider fallback; `executeWithFallback` appears in the shared extraction adapter and provider execution policy. +### Task 4: Complete the phase gate -- [ ] **Step 2: Run the complete phase gate.** +- [ ] **Step 1: Confirm no duplicate transport/outer fallback remains.** Search `src` and tests for `extractViaJinaReader`, `jina-reader`, `r.jina.ai`, and `executeWithFallback`. `r.jina.ai` must remain only in `src/providers/jina.ts` and its provider tests; `web-fetch.ts` must not execute a second fetch fallback. +- [ ] **Step 2: Run the full gate.** ```bash -pnpm exec vitest run tests/extract/fallbacks.test.ts tests/extract/pipeline.test.ts tests/extract/pipeline-routing.test.ts tests/extract/pipeline-ssrf.test.ts tests/extract/cloudflare-retry.test.ts tests/providers/jina.test.ts tests/tools/web-fetch.test.ts tests/tools/web-fetch-video.test.ts pnpm check git diff --check +git status --short ``` -Expected: focused tests and the full suite pass with only the documented pre-existing Biome and Node-engine warnings. +Expected: the focused tests and full suite pass with only the documented Biome and Node-engine warnings. `git status --short` contains only the intended Phase 5 files. - [ ] **Step 3: Commit the atomic phase.** ```bash -git add src/extract src/providers/jina.ts src/tools/web-fetch.ts tests +git add \ + src/cache.ts \ + src/extract/types.ts src/extract/fallbacks.ts src/extract/pipeline.ts \ + src/extract/gemini-url-context.ts src/extract/youtube.ts src/extract/video.ts src/extract/frames.ts \ + src/extract/jina-reader.ts src/providers/jina.ts src/tools/web-fetch.ts \ + tests/extract/fallbacks.test.ts tests/extract/pipeline.test.ts tests/extract/pipeline-routing.test.ts \ + tests/extract/pipeline-ssrf.test.ts tests/extract/cloudflare-retry.test.ts tests/extract/jina-reader.test.ts \ + tests/providers/jina.test.ts tests/tools/web-fetch.test.ts tests/tools/web-fetch-video.test.ts git commit -m "refactor: unify content extraction fallbacks" ``` -The commit may delete `src/extract/jina-reader.ts` and its test, but must not change provider catalog, configuration lifecycle, or unrelated extraction routes. - ## Phase completion gate - Registered fetch providers are the only provider-backed extraction adapters. - Direct Jina Reader transport is deleted from `src/extract`. -- Fallback order, retryable error class, budgets, activity, cancellation, SSRF, and structured routes are covered by tests. -- Cache, content storage, multi-URL, and rendering behavior remain intact. +- Fallback order, retryable transport behavior, budgets, hooks, cancellation, SSRF, redirects, and structured routes are covered by tests. +- Cache, content storage, multi-URL, image, and rendering behavior remain intact. - Focused tests, `pnpm check`, and `git diff --check` pass. diff --git a/src/cache.ts b/src/cache.ts index d4d4f16..19d7869 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -1,4 +1,4 @@ -import type { ExtractedContent } from "./extract/pipeline.ts"; +import type { ExtractedContent } from "./extract/types.ts"; interface CacheEntry { content: ExtractedContent; diff --git a/src/extract/fallbacks.ts b/src/extract/fallbacks.ts new file mode 100644 index 0000000..93d815c --- /dev/null +++ b/src/extract/fallbacks.ts @@ -0,0 +1,101 @@ +import { executeWithFallback, type ExecutionHooks } from "../providers/execute.ts"; +import type { FetchProvider } from "../providers/types.ts"; +import { sanitizeError } from "../utils/errors.ts"; +import type { ExtractedContent } from "./types.ts"; + +export interface ExtractionFallback { + name: string; + run(): Promise; +} + +interface CreateFetchProviderFallbackOptions { + url: string; + providers: readonly FetchProvider[]; + signal?: AbortSignal; + hooks?: ExecutionHooks; +} + +function isAbortError(error: unknown): boolean { + if (!error || typeof error !== "object") return false; + const name = (error as { name?: string }).name; + return name === "AbortError"; +} + +export function createFetchProviderFallback( + options: CreateFetchProviderFallbackOptions, +): ExtractionFallback { + const { url, providers, signal, hooks } = options; + + return { + name: "fetch-provider", + async run(): Promise { + if (providers.length === 0) { + throw new Error("No fetch providers configured"); + } + + try { + const { result, providerName } = await executeWithFallback({ + candidates: providers.map((provider) => ({ + name: provider.name, + execute: async () => { + const fetched = await provider.fetch(url, signal); + const text = fetched.text.trim(); + if (!text) throw new Error(`${provider.name} returned empty content`); + return { ...fetched, text }; + }, + })), + operation: "fetch", + signal, + onSuccess: hooks?.onSuccess, + onFailure: hooks?.onFailure, + }); + + return { + text: result.text, + title: result.title, + url, + extractionChain: [`fetch-provider:${providerName}`], + chars: result.text.length, + truncated: false, + }; + } catch (error) { + signal?.throwIfAborted(); + if (isAbortError(error)) throw error; + // Rethrow ordinary errors so runExtractionFallbacks records :error. + // Surface a sanitized summary, never raw provider messages with credentials. + throw new Error(sanitizeError(error)); + } + }, + }; +} + +export async function runExtractionFallbacks( + fallbacks: readonly ExtractionFallback[], + chain: string[], + signal?: AbortSignal, +): Promise { + for (const fallback of fallbacks) { + signal?.throwIfAborted(); + + let result: ExtractedContent | null; + try { + result = await fallback.run(); + } catch (error) { + signal?.throwIfAborted(); + if (isAbortError(error)) throw error; + chain.push(`${fallback.name}:error`); + continue; + } + + signal?.throwIfAborted(); + if (result == null) { + chain.push(`${fallback.name}:fail`); + continue; + } + + chain.push(result.extractionChain[0]); + return result; + } + + return null; +} diff --git a/src/extract/frames.ts b/src/extract/frames.ts index 3afa33c..5ba7aa0 100644 --- a/src/extract/frames.ts +++ b/src/extract/frames.ts @@ -1,5 +1,5 @@ import { execFileSync } from "node:child_process"; -import type { VideoFrame } from "./pipeline.ts"; +import type { VideoFrame } from "./types.ts"; // --------------------------------------------------------------------------- // Constants diff --git a/src/extract/gemini-url-context.ts b/src/extract/gemini-url-context.ts index c100172..1e53db0 100644 --- a/src/extract/gemini-url-context.ts +++ b/src/extract/gemini-url-context.ts @@ -1,6 +1,6 @@ import { getApiKey, getVersionedApiBase } from "./gemini-api.ts"; import { isGeminiWebAvailable, queryWithCookies } from "./gemini-web.ts"; -import type { ExtractedContent } from "./pipeline.ts"; +import type { ExtractedContent } from "./types.ts"; const MIN_CONTENT_LENGTH = 100; diff --git a/src/extract/github.ts b/src/extract/github.ts index f222a0a..c2ba82d 100644 --- a/src/extract/github.ts +++ b/src/extract/github.ts @@ -4,7 +4,7 @@ import * as os from "node:os"; import * as nodePath from "node:path"; import { promisify } from "node:util"; import { DEFAULT_GITHUB_CONFIG, type GitHubConfig } from "../config.ts"; -import type { ExtractedContent } from "./pipeline.ts"; +import type { ExtractedContent } from "./types.ts"; const execFileAsync = promisify(execFile); diff --git a/src/extract/jina-reader.ts b/src/extract/jina-reader.ts deleted file mode 100644 index e478b06..0000000 --- a/src/extract/jina-reader.ts +++ /dev/null @@ -1,31 +0,0 @@ -/** - * Extracts rendered page content via the Jina Reader API (r.jina.ai). - * Used as a fallback for JS-rendered pages that Readability and RSC can't parse. - * - * SECURITY: Jina Reader follows HTTP redirects without per-hop SSRF validation. - * The initial URL is validated locally, but redirect chains may reach internal - * resources. Acceptable risk since this is a fallback after local extraction fails. - * - * RATE LIMITS: Free tier allows ~500 RPM. Returns null on 429 or any failure. - */ -export async function extractViaJinaReader( - url: string, - signal?: AbortSignal, -): Promise { - try { - const readerUrl = `https://r.jina.ai/${url}`; - const response = await fetch(readerUrl, { - headers: { Accept: "text/plain" }, - signal, - }); - - if (!response.ok) return null; - - const text = await response.text(); - if (!text || text.trim().length < 100) return null; - - return text.trim(); - } catch { - return null; - } -} diff --git a/src/extract/pipeline.ts b/src/extract/pipeline.ts index b7c85d2..c3fa3fd 100644 --- a/src/extract/pipeline.ts +++ b/src/extract/pipeline.ts @@ -9,11 +9,17 @@ import { rasterizePdfPages, modelSupportsImages, extractTextWithGeminiVision, - type PdfPageImage, } from "./pdf-ocr.ts"; +import { collectImageBlocks, type ExtractedContent, type ImageBlock } from "./types.ts"; +import { createFetchProviderFallback, runExtractionFallbacks } from "./fallbacks.ts"; +import type { FetchProvider } from "../providers/types.ts"; +import type { ExecutionHooks } from "../providers/execute.ts"; +import { sanitizeError } from "../utils/errors.ts"; + +export type { ExtractedContent, ImageBlock, VideoFrame } from "./types.ts"; +export { collectImageBlocks } from "./types.ts"; import { getApiKey as getGeminiApiKey } from "./gemini-api.ts"; import { extractRsc } from "./rsc.ts"; -import { extractViaJinaReader } from "./jina-reader.ts"; import { isYouTubeURL, extractYouTube, isYouTubeEnabled } from "./youtube.ts"; import { isVideoFile, extractVideo, isVideoEnabled } from "./video.ts"; import { @@ -25,7 +31,6 @@ import { } from "./frames.ts"; import { extractWithUrlContext, extractWithGeminiWeb } from "./gemini-url-context.ts"; import { activityMonitor } from "../monitor/activity-monitor.ts"; - /** * Error thrown when the HTTP fetch fails in a way that a different fetch * provider might succeed (network errors, 5xx, 429). @@ -36,51 +41,6 @@ export class RetryableExtractionError extends Error { this.name = "RetryableExtractionError"; } } - -export interface VideoFrame { - data: string; - mimeType: string; - timestamp: string; -} - -export interface ExtractedContent { - text: string; - title?: string; - url: string; - extractionChain: string[]; - chars: number; - truncated: boolean; - contentId?: string; - thumbnail?: { data: string; mimeType: string }; - frames?: VideoFrame[]; - images?: PdfPageImage[]; - duration?: number; -} - -export type ImageBlock = { type: "image"; data: string; mimeType: string }; - -export function collectImageBlocks(extracted: ExtractedContent): ImageBlock[] { - const blocks: ImageBlock[] = []; - if (extracted.thumbnail) { - blocks.push({ - type: "image", - data: extracted.thumbnail.data, - mimeType: extracted.thumbnail.mimeType, - }); - } - if (extracted.frames) { - for (const frame of extracted.frames) { - blocks.push({ type: "image", data: frame.data, mimeType: frame.mimeType }); - } - } - if (extracted.images) { - for (const img of extracted.images) { - blocks.push({ type: "image", data: img.data, mimeType: img.mimeType }); - } - } - return blocks; -} - const BINARY_CONTENT_TYPES = [ "image/", "audio/", @@ -106,6 +66,20 @@ const MAX_REDIRECTS = 10; const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); +async function tryRegisteredFallback( + adapter: ReturnType[], + chain: string[], + signal: AbortSignal | undefined, +): Promise { + if (adapter.length === 0) return null; + signal?.throwIfAborted(); + return runExtractionFallbacks(adapter, chain, signal); +} + +function throwRetryableFromTransport(originalError: unknown): never { + throw new RetryableExtractionError(sanitizeError(originalError).slice(0, 120)); +} + async function fetchValidated( url: string, init: RequestInit, @@ -159,6 +133,7 @@ export async function probeUrl( ); } catch (error) { if (error instanceof SSRFError) throw error; + signal?.throwIfAborted(); return { skip: false }; } @@ -197,6 +172,8 @@ export interface ExtractOptions { frames?: number; model?: string; ctx?: import("@earendil-works/pi-coding-agent").ExtensionContext; + fetchProviders?: readonly FetchProvider[]; + executionHooks?: ExecutionHooks; } export async function extractContent( @@ -204,6 +181,7 @@ export async function extractContent( signal?: AbortSignal, options?: ExtractOptions, ): Promise { + signal?.throwIfAborted(); const { github, ssrf, pdf, gemini } = loadMergedConfig(process.cwd()); // --- Frame extraction mode (timestamp/frames params present) --- @@ -276,6 +254,20 @@ export async function extractContent( // --- SSRF validation (after video/YouTube routing, before HTTP fetch) --- validateUrl(url, { allowRanges: ssrf.allowRanges }); + // Built after SSRF validation so transport and HTML paths share the same + // registry-wrapped candidates. + const fetchAdapter = + options?.fetchProviders && options.fetchProviders.length > 0 + ? [ + createFetchProviderFallback({ + url, + providers: options.fetchProviders, + signal, + hooks: options.executionHooks, + }), + ] + : []; + // GitHub interception: try structured extraction before HTML scraping. // Only fires for content URLs (blob, tree, root, raw). // Returns null for non-content URLs (issues, PRs, etc.) -> falls through. @@ -310,7 +302,10 @@ export async function extractContent( } catch (err) { activityMonitor.logError(fetchEntryId, err instanceof Error ? err.message : String(err)); if (err instanceof SSRFError) throw err; - throw new RetryableExtractionError(err instanceof Error ? err.message : String(err)); + signal?.throwIfAborted(); + const fallback = await tryRegisteredFallback(fetchAdapter, [], signal); + if (fallback) return fallback; + throwRetryableFromTransport(err); } // Cloudflare bot challenge: retry once with honest User-Agent @@ -330,7 +325,10 @@ export async function extractContent( } catch (err) { activityMonitor.logError(retryEntryId, err instanceof Error ? err.message : String(err)); if (err instanceof SSRFError) throw err; - throw new RetryableExtractionError(err instanceof Error ? err.message : String(err)); + signal?.throwIfAborted(); + const fallback = await tryRegisteredFallback(fetchAdapter, [], signal); + if (fallback) return fallback; + throwRetryableFromTransport(err); } } @@ -340,6 +338,8 @@ export async function extractContent( const status = response.status; // 429 and 5xx are retryable — a different provider might succeed if (status === 429 || status >= 500) { + const fallback = await tryRegisteredFallback(fetchAdapter, [], signal); + if (fallback) return fallback; throw new RetryableExtractionError(`HTTP ${status}: ${response.statusText}`); } throw new Error(`HTTP ${status}: ${response.statusText}`); @@ -500,32 +500,37 @@ export async function extractContent( } chain.push("rsc:no-match"); - // Tier 3: Jina Reader - const jinaText = await extractViaJinaReader(url, signal); - if (jinaText) { - chain.push("jina-reader"); - return { - text: jinaText, - title: undefined, - url, - extractionChain: chain, - chars: jinaText.length, - truncated: false, - }; + // Tier 3: registered fetch providers (skipped when no providers configured). + if (fetchAdapter.length === 0) { + chain.push("fetch-provider:skipped"); + } else { + const providerResult = await runExtractionFallbacks(fetchAdapter, chain, signal); + if (providerResult) { + return { ...providerResult, extractionChain: chain }; + } } - chain.push("jina-reader:fail"); - // Tier 4: Gemini HTML fallback - const geminiResult = - (await extractWithUrlContext(url, signal)) ?? (await extractWithGeminiWeb(url, signal)); + // Tier 4: combined Gemini HTML adapter (URL context, then Web). + const geminiResult = await runExtractionFallbacks( + [ + { + name: "gemini-html", + run: async () => { + const urlContextResult = await extractWithUrlContext(url, signal); + signal?.throwIfAborted(); + return urlContextResult ?? (await extractWithGeminiWeb(url, signal)); + }, + }, + ], + chain, + signal, + ); if (geminiResult) { - chain.push(geminiResult.extractionChain[0]); return { ...geminiResult, extractionChain: chain, }; } - chain.push("gemini-html:fail"); // Final fallback: raw text stripped of HTML const rawText = body diff --git a/src/extract/types.ts b/src/extract/types.ts new file mode 100644 index 0000000..f433fd5 --- /dev/null +++ b/src/extract/types.ts @@ -0,0 +1,45 @@ +import type { PdfPageImage } from "./pdf-ocr.ts"; + +export interface VideoFrame { + data: string; + mimeType: string; + timestamp: string; +} + +export interface ExtractedContent { + text: string; + title?: string; + url: string; + extractionChain: string[]; + chars: number; + truncated: boolean; + contentId?: string; + thumbnail?: { data: string; mimeType: string }; + frames?: VideoFrame[]; + images?: PdfPageImage[]; + duration?: number; +} + +export type ImageBlock = { type: "image"; data: string; mimeType: string }; + +export function collectImageBlocks(extracted: ExtractedContent): ImageBlock[] { + const blocks: ImageBlock[] = []; + if (extracted.thumbnail) { + blocks.push({ + type: "image", + data: extracted.thumbnail.data, + mimeType: extracted.thumbnail.mimeType, + }); + } + if (extracted.frames) { + for (const frame of extracted.frames) { + blocks.push({ type: "image", data: frame.data, mimeType: frame.mimeType }); + } + } + if (extracted.images) { + for (const img of extracted.images) { + blocks.push({ type: "image", data: img.data, mimeType: img.mimeType }); + } + } + return blocks; +} \ No newline at end of file diff --git a/src/extract/video.ts b/src/extract/video.ts index 6a2dfcb..108b806 100644 --- a/src/extract/video.ts +++ b/src/extract/video.ts @@ -3,7 +3,8 @@ import { statSync } from "node:fs"; import { basename, extname, resolve } from "node:path"; import { readFile } from "node:fs/promises"; import { loadConfig } from "../config.ts"; -import type { ExtractedContent, ExtractOptions } from "./pipeline.ts"; +import type { ExtractedContent } from "./types.ts"; +import type { ExtractOptions } from "./pipeline.ts"; import { queryGeminiApi, getApiKey, getVersionedApiBase } from "./gemini-api.ts"; import { isGeminiWebAvailable, queryWithCookies } from "./gemini-web.ts"; import { extractHeadingTitle } from "./youtube.ts"; diff --git a/src/extract/youtube.ts b/src/extract/youtube.ts index c50eb26..a17cb41 100644 --- a/src/extract/youtube.ts +++ b/src/extract/youtube.ts @@ -2,7 +2,8 @@ import { DEFAULT_YOUTUBE_CONFIG, loadConfig } from "../config.ts"; import { isGeminiApiAvailable, queryGeminiApi } from "./gemini-api.ts"; import { isGeminiWebAvailable, queryWithCookies } from "./gemini-web.ts"; import { isPerplexityAvailable, queryPerplexity } from "./perplexity.ts"; -import type { ExtractedContent, ExtractOptions } from "./pipeline.ts"; +import type { ExtractedContent } from "./types.ts"; +import type { ExtractOptions } from "./pipeline.ts"; // --------------------------------------------------------------------------- // Constants diff --git a/src/providers/jina.ts b/src/providers/jina.ts index f0622c3..82ce60e 100644 --- a/src/providers/jina.ts +++ b/src/providers/jina.ts @@ -63,7 +63,10 @@ export class JinaProvider implements SearchProvider, FetchProvider { throw new Error(`Jina reader error: ${response.status} ${response.statusText}`); } - const text = await response.text(); + const text = (await response.text()).trim(); + if (text.length < 100) { + throw new Error("Jina reader returned insufficient content"); + } return { text }; } } diff --git a/src/tools/web-fetch.ts b/src/tools/web-fetch.ts index ea7688a..0951ed8 100644 --- a/src/tools/web-fetch.ts +++ b/src/tools/web-fetch.ts @@ -5,7 +5,6 @@ import type { ContentStore } from "../storage.ts"; import type { FetchProvider } from "../providers/types.ts"; import { extractContent, - RetryableExtractionError, collectImageBlocks, type ExtractedContent, type ImageBlock, @@ -13,7 +12,7 @@ import { import { truncateContent } from "../utils/truncate.ts"; import { fetchWithConcurrencyLimit } from "../utils/concurrency.ts"; import { sanitizeError } from "../utils/errors.ts"; -import { executeWithFallback, type ExecutionHooks } from "../providers/execute.ts"; +import type { ExecutionHooks } from "../providers/execute.ts"; import type { ContentCache } from "../cache.ts"; import type { GuidanceOverride } from "../config.ts"; @@ -101,53 +100,17 @@ export function createWebFetchTool( if (cached) return cached; } - let extracted: ExtractedContent; - try { - extracted = await extractContent(url, signal, { - raw: params.raw, - prompt: params.prompt, - timestamp: params.timestamp, - frames: params.frames, - model: params.model, - ctx, - }); - } catch (pipelineError) { - if (!(pipelineError instanceof RetryableExtractionError)) { - throw pipelineError; - } - - const candidates = resolveFetchCandidates?.() ?? []; - if (candidates.length === 0) { - throw pipelineError; - } - - try { - const { result, providerName } = await executeWithFallback({ - candidates: candidates.map((provider) => ({ - name: provider.name, - execute: () => provider.fetch(url, signal), - })), - operation: "fetch", - signal, - onSuccess: executionHooks?.onSuccess, - onFailure: executionHooks?.onFailure, - }); - extracted = { - text: result.text, - title: result.title, - url, - extractionChain: [`fetch-provider:${providerName}`], - chars: result.text.length, - truncated: false, - }; - } catch (fallbackError) { - const pipelineMessage = sanitizeError(pipelineError).slice(0, 120); - const fallbackMessage = sanitizeError(fallbackError).slice(0, 120); - throw new Error( - `Pipeline failed: ${pipelineMessage}; provider fallback failed: ${fallbackMessage}`, - ); - } - } + const candidates = resolveFetchCandidates?.(); + const extracted = await extractContent(url, signal, { + raw: params.raw, + prompt: params.prompt, + timestamp: params.timestamp, + frames: params.frames, + model: params.model, + ctx, + fetchProviders: candidates, + executionHooks, + }); cache?.set(url, extracted); return extracted; diff --git a/tests/extract/chrome-cookies.test.ts b/tests/extract/chrome-cookies.test.ts index 1de3ec4..51f347d 100644 --- a/tests/extract/chrome-cookies.test.ts +++ b/tests/extract/chrome-cookies.test.ts @@ -83,6 +83,19 @@ describe("chrome-cookies", () => { // Reset all mocks to their starting state. vi.clearAllMocks(); + // Fail closed if a test forgets to provide a fake keychain response. + // The real macOS `security` command is never invoked by this suite. + execFileMock.mockReset(); + execFileMock.mockImplementation( + ( + _command: string, + _args: string[], + _options: unknown, + callback: (error: Error | null, stdout: string) => void, + ) => { + callback(new Error("test keychain access disabled"), ""); + }, + ); osMock.platform.mockReturnValue("darwin"); osMock.homedir.mockReturnValue("/Users/test"); osMock.tmpdir.mockReturnValue("/tmp"); diff --git a/tests/extract/fallbacks.test.ts b/tests/extract/fallbacks.test.ts new file mode 100644 index 0000000..79a414d --- /dev/null +++ b/tests/extract/fallbacks.test.ts @@ -0,0 +1,352 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + createFetchProviderFallback, + runExtractionFallbacks, + type ExtractionFallback, +} from "../../src/extract/fallbacks.ts"; +import type { FetchProvider, FetchResult } from "../../src/providers/types.ts"; +import type { ExecutionHooks } from "../../src/providers/execute.ts"; +import { BudgetExceededError } from "../../src/providers/registry.ts"; + +const LONG_TEXT = " Fetched content from the provider. ".repeat(10).trim(); + +function makeProvider(name: string, fetch: FetchProvider["fetch"]): FetchProvider { + return { name, fetch }; +} + +function ok(text: string, title?: string): FetchResult { + return { text, title }; +} + +describe("createFetchProviderFallback", () => { + it("throws without invoking when providers list is empty", async () => { + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [], + }); + await expect(fallback.run()).rejects.toThrow(); + }); + + it("calls providers in supplied order through executeWithFallback", async () => { + const calls: string[] = []; + const a = makeProvider("a", vi.fn(async () => { + calls.push("a"); + throw new Error("fail-a"); + })); + const b = makeProvider("b", vi.fn(async () => { + calls.push("b"); + throw new Error("fail-b"); + })); + const c = makeProvider("c", vi.fn(async () => { + calls.push("c"); + return ok(LONG_TEXT, "Title from c"); + })); + + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [a, b, c], + }); + const result = await fallback.run(); + + expect(calls).toEqual(["a", "b", "c"]); + expect(result).not.toBeNull(); + expect(result?.extractionChain).toEqual(["fetch-provider:c"]); + expect(result?.title).toBe("Title from c"); + expect(result?.chars).toBe(LONG_TEXT.length); + }); + + it("normalizes the returned content with provider-only chain marker", async () => { + const provider = makeProvider("jina", vi.fn(async () => ok(LONG_TEXT, "Readability title"))); + const fallback = createFetchProviderFallback({ + url: "https://example.com/article", + providers: [provider], + }); + + const result = await fallback.run(); + expect(result).toEqual({ + text: LONG_TEXT, + title: "Readability title", + url: "https://example.com/article", + extractionChain: ["fetch-provider:jina"], + chars: LONG_TEXT.length, + truncated: false, + }); + }); + + it("treats empty text as a failed attempt", async () => { + const failing = makeProvider("a", vi.fn(async () => ({ text: " \n " }))); + const succeeding = makeProvider( + "b", + vi.fn(async () => ok(LONG_TEXT)), + ); + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [failing, succeeding], + }); + + const result = await fallback.run(); + expect(result?.extractionChain).toEqual(["fetch-provider:b"]); + }); + + it("trims the returned text before measuring chars", async () => { + const provider = makeProvider( + "trim", + vi.fn(async () => ({ text: ` ${LONG_TEXT} ` })), + ); + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [provider], + }); + + const result = await fallback.run(); + expect(result?.text).toBe(LONG_TEXT); + expect(result?.chars).toBe(LONG_TEXT.length); + }); + + it("throws when all providers fail", async () => { + const failing = makeProvider("nope", vi.fn(async () => { + throw new Error("kaboom"); + })); + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [failing], + }); + + await expect(fallback.run()).rejects.toThrow(); + }); + + it("passes signal through to providers", async () => { + const seenSignals: Array = []; + const provider: FetchProvider = { + name: "spy", + fetch: vi.fn(async (_url, signal) => { + seenSignals.push(signal); + return ok(LONG_TEXT); + }), + }; + const controller = new AbortController(); + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [provider], + signal: controller.signal, + }); + + await fallback.run(); + expect(seenSignals).toEqual([controller.signal]); + }); + + it("passes hooks through to executeWithFallback", async () => { + const onSuccess = vi.fn(); + const onFailure = vi.fn(); + const hooks: ExecutionHooks = { onSuccess, onFailure }; + + const a = makeProvider("a", vi.fn(async () => { + throw new Error("a-bad"); + })); + const b = makeProvider("b", vi.fn(async () => ok(LONG_TEXT))); + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [a, b], + hooks, + }); + + await fallback.run(); + + expect(onFailure).toHaveBeenCalledWith("a"); + expect(onFailure).not.toHaveBeenCalledWith("b"); + expect(onSuccess).toHaveBeenCalledWith("b", expect.any(Number)); + }); + + it("does not record BudgetExceededError as a performance failure", async () => { + const onFailure = vi.fn(); + const exhausted = makeProvider("budget", vi.fn(async () => { + throw new BudgetExceededError("budget", 1, { + mode: "hard", + used: 1, + limit: 1, + unit: "request", + period: "month", + periodKey: "2026-08", + }); + })); + const working = makeProvider("ok", vi.fn(async () => ok(LONG_TEXT))); + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [exhausted, working], + hooks: { onFailure }, + }); + + const result = await fallback.run(); + expect(result?.extractionChain).toEqual(["fetch-provider:ok"]); + expect(onFailure).not.toHaveBeenCalledWith("budget"); + }); + + it("rethrows caller cancellation that arrives mid-run", async () => { + const controller = new AbortController(); + const provider: FetchProvider = { + name: "aborting", + fetch: vi.fn(async () => { + controller.abort(); + return ok(LONG_TEXT); + }), + }; + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [provider], + signal: controller.signal, + }); + + await expect(fallback.run()).rejects.toThrow(); + }); + + it("rethrows a custom caller abort reason from a provider", async () => { + const controller = new AbortController(); + const reason = new Error("caller stopped"); + const provider: FetchProvider = { + name: "aborting", + fetch: vi.fn(async () => { + controller.abort(reason); + return ok(LONG_TEXT); + }), + }; + const fallback = createFetchProviderFallback({ + url: "https://example.com", + providers: [provider], + signal: controller.signal, + }); + + await expect(fallback.run()).rejects.toBe(reason); + }); +}); + +describe("runExtractionFallbacks", () => { + let chain: string[]; + + beforeEach(() => { + chain = []; + }); + + it("returns the first non-null result and appends its chain marker", async () => { + const fallbackA: ExtractionFallback = { + name: "a", + run: vi.fn().mockResolvedValue(null), + }; + const fallbackB: ExtractionFallback = { + name: "b", + run: vi.fn().mockResolvedValue({ + text: "Body", + url: "https://example.com", + extractionChain: ["html:gemini-url-context"], + chars: 4, + truncated: false, + }), + }; + + const result = await runExtractionFallbacks([fallbackA, fallbackB], chain); + expect(result?.text).toBe("Body"); + expect(chain).toEqual(["a:fail", "html:gemini-url-context"]); + }); + + it("records fail markers when a fallback returns null", async () => { + const fallbackA: ExtractionFallback = { + name: "a", + run: vi.fn(async () => null), + }; + const fallbackB: ExtractionFallback = { + name: "b", + run: vi.fn(async () => null), + }; + + const result = await runExtractionFallbacks([fallbackA, fallbackB], chain); + expect(result).toBeNull(); + expect(chain).toEqual(["a:fail", "b:fail"]); + }); + + it("records error markers when a fallback throws non-abort errors", async () => { + const fallbackA: ExtractionFallback = { + name: "a", + run: vi.fn(async () => { + throw new Error("hard-fail"); + }), + }; + const fallbackB: ExtractionFallback = { + name: "b", + run: vi.fn(async () => null), + }; + + const result = await runExtractionFallbacks([fallbackA, fallbackB], chain); + expect(result).toBeNull(); + expect(chain).toEqual(["a:error", "b:fail"]); + }); + + it("stops the chain when caller cancellation is observed before next adapter", async () => { + const controller = new AbortController(); + controller.abort(); + + const seen: string[] = []; + const fallbackA: ExtractionFallback = { + name: "a", + run: vi.fn(async () => { + seen.push("a"); + return null; + }), + }; + const fallbackB: ExtractionFallback = { + name: "b", + run: vi.fn(async () => { + seen.push("b"); + return null; + }), + }; + + await expect( + runExtractionFallbacks([fallbackA, fallbackB], chain, controller.signal), + ).rejects.toThrow(); + // First check throws before invoking A, so neither runs. + expect(seen).toEqual([]); + }); + + it("checks caller cancellation after the final adapter completes", async () => { + const controller = new AbortController(); + const reason = new Error("caller stopped"); + const fallback: ExtractionFallback = { + name: "a", + run: vi.fn(async () => { + controller.abort(reason); + return null; + }), + }; + + await expect( + runExtractionFallbacks([fallback], chain, controller.signal), + ).rejects.toBe(reason); + }); + + it("returns null when no fallbacks are supplied", async () => { + expect(await runExtractionFallbacks([], chain)).toBeNull(); + expect(chain).toEqual([]); + }); + + it("appends the first marker of the winning fallback only", async () => { + const winning: ExtractionFallback = { + name: "win", + run: vi.fn(async () => ({ + text: "Body", + url: "https://example.com", + extractionChain: ["fetch-provider:jina"], + chars: 4, + truncated: false, + })), + }; + const resultChain: string[] = ["http:200"]; + const result = await runExtractionFallbacks( + [ + { name: "lose", run: vi.fn(async () => null) }, + winning, + ], + resultChain, + ); + expect(result?.extractionChain).toEqual(["fetch-provider:jina"]); + expect(resultChain).toEqual(["http:200", "lose:fail", "fetch-provider:jina"]); + }); +}); diff --git a/tests/extract/jina-reader.test.ts b/tests/extract/jina-reader.test.ts deleted file mode 100644 index ea4888f..0000000 --- a/tests/extract/jina-reader.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { extractViaJinaReader } from "../../src/extract/jina-reader.ts"; -import { stubFetch } from "../helpers.ts"; - -describe("extractViaJinaReader", () => { - let fetchStub: ReturnType; - - beforeEach(() => { - fetchStub = stubFetch(); - }); - afterEach(() => { - fetchStub.restore(); - }); - - it("returns markdown from Jina Reader", async () => { - fetchStub.addResponse("r.jina.ai", { - body: "# Page Title\n\nRendered content from JS page. This article covers important topics and provides detailed information for readers who want to learn more about the subject.", - headers: { "content-type": "text/plain" }, - }); - - const result = await extractViaJinaReader("https://example.com"); - expect(result).not.toBeNull(); - expect(result).toContain("Rendered content"); - }); - - it("returns null on failure", async () => { - fetchStub.addResponse("r.jina.ai", { status: 500, body: "Error" }); - const result = await extractViaJinaReader("https://example.com"); - expect(result).toBeNull(); - }); -}); diff --git a/tests/extract/pipeline-fallback.test.ts b/tests/extract/pipeline-fallback.test.ts new file mode 100644 index 0000000..b54f2ee --- /dev/null +++ b/tests/extract/pipeline-fallback.test.ts @@ -0,0 +1,487 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("../../src/extract/youtube.ts", () => ({ + isYouTubeURL: vi.fn(), + extractYouTube: vi.fn(), + isYouTubeEnabled: vi.fn(), +})); + +vi.mock("../../src/extract/video.ts", () => ({ + isVideoFile: vi.fn(), + extractVideo: vi.fn(), + isVideoEnabled: vi.fn(), +})); + +vi.mock("../../src/extract/frames.ts", () => ({ + parseTimestampParam: vi.fn(), + extractYouTubeFrames: vi.fn(), + extractLocalFrames: vi.fn(), + getLocalVideoDuration: vi.fn(), + getYouTubeStreamInfo: vi.fn(), +})); + +vi.mock("../../src/extract/gemini-url-context.ts", () => ({ + extractWithUrlContext: vi.fn(), + extractWithGeminiWeb: vi.fn(), +})); + +import { extractContent, RetryableExtractionError } from "../../src/extract/pipeline.ts"; +import { extractWithUrlContext, extractWithGeminiWeb } from "../../src/extract/gemini-url-context.ts"; +import type { FetchProvider, FetchResult } from "../../src/providers/types.ts"; +import type { ExecutionHooks } from "../../src/providers/execute.ts"; +import { BudgetExceededError } from "../../src/providers/registry.ts"; +import { isYouTubeURL, extractYouTube, isYouTubeEnabled } from "../../src/extract/youtube.ts"; +import { isVideoFile, extractVideo, isVideoEnabled } from "../../src/extract/video.ts"; + +function provider(name: string, result: FetchResult | null | Error): FetchProvider { + const fetchFn = vi.fn(); + if (result instanceof Error) { + fetchFn.mockRejectedValue(result); + } else { + fetchFn.mockResolvedValue(result); + } + return { name, fetch: fetchFn }; +} + +const LONG_TEXT = " Long content from the provider used by tests. ".repeat(10).trim(); + +describe("extractContent — registered fetch provider ordering", () => { + beforeEach(() => { + vi.mocked(isYouTubeURL).mockReturnValue({ isYouTube: false, videoId: null }); + vi.mocked(isYouTubeEnabled).mockReturnValue(false); + vi.mocked(isVideoFile).mockReturnValue(null); + vi.mocked(isVideoEnabled).mockReturnValue(false); + vi.mocked(extractWithUrlContext).mockResolvedValue(null); + vi.mocked(extractWithGeminiWeb).mockResolvedValue(null); + vi.mocked(extractYouTube).mockResolvedValue(null); + vi.mocked(extractVideo).mockResolvedValue(null); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("runs Readability and RSC before registered providers, then Gemini last", async () => { + const fetchProvider = provider("jina", { text: LONG_TEXT, title: "From Jina" }); + vi.mocked(extractWithUrlContext).mockResolvedValue({ + text: "Gemini content", + url: "https://example.com/article", + extractionChain: ["html:gemini-url-context"], + chars: 14, + truncated: false, + }); + + const orig = globalThis.fetch; + globalThis.fetch = vi.fn(async () => + new Response("

short body

", { + status: 200, + headers: { "content-type": "text/html" }, + }), + ) as unknown as typeof fetch; + try { + const result = await extractContent("https://example.com/article", undefined, { + fetchProviders: [fetchProvider], + }); + expect(result.extractionChain).toEqual([ + "http:200", + "readability:thin", + "rsc:no-match", + "fetch-provider:jina", + ]); + expect(result.title).toBe("From Jina"); + expect(extractWithUrlContext).not.toHaveBeenCalled(); + expect(extractWithGeminiWeb).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = orig; + } + }); + + it("uses Gemini when no fetch providers are supplied", async () => { + vi.mocked(extractWithUrlContext).mockResolvedValue({ + text: "Gemini only content", + url: "https://example.com/article", + extractionChain: ["html:gemini-url-context"], + chars: 18, + truncated: false, + }); + + const orig = globalThis.fetch; + globalThis.fetch = vi.fn(async () => + new Response("

short body

", { + status: 200, + headers: { "content-type": "text/html" }, + }), + ) as unknown as typeof fetch; + try { + const result = await extractContent("https://example.com/article", undefined); + expect(result.extractionChain).toEqual([ + "http:200", + "readability:thin", + "rsc:no-match", + "fetch-provider:skipped", + "html:gemini-url-context", + ]); + } finally { + globalThis.fetch = orig; + } + }); + + it("records fetch-provider:error when all providers fail", async () => { + const a = provider("a", new Error("a-bad")); + const b = provider("b", new Error("b-bad")); + + const orig = globalThis.fetch; + globalThis.fetch = vi.fn(async () => + new Response("

short body

", { + status: 200, + headers: { "content-type": "text/html" }, + }), + ) as unknown as typeof fetch; + try { + const result = await extractContent("https://example.com/article", undefined, { + fetchProviders: [a, b], + }); + expect(result.extractionChain).toContain("fetch-provider:error"); + expect(result.extractionChain).toContain("gemini-html:fail"); + expect(result.extractionChain).toContain("raw-text"); + } finally { + globalThis.fetch = orig; + } + }); +}); + +describe("extractContent — retryable transport fallback", () => { + beforeEach(() => { + vi.mocked(isYouTubeURL).mockReturnValue({ isYouTube: false, videoId: null }); + vi.mocked(isYouTubeEnabled).mockReturnValue(false); + vi.mocked(isVideoFile).mockReturnValue(null); + vi.mocked(isVideoEnabled).mockReturnValue(false); + vi.mocked(extractWithUrlContext).mockResolvedValue(null); + vi.mocked(extractWithGeminiWeb).mockResolvedValue(null); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("recovers from 5xx through a registered provider", async () => { + const providerA: FetchProvider = { + name: "jina", + fetch: vi.fn(async () => ({ text: LONG_TEXT, title: "Jina" })), + }; + const orig = globalThis.fetch; + let calls = 0; + globalThis.fetch = vi.fn(async () => { + calls++; + if (calls === 1) { + // HEAD probe succeeds so the pipeline doesn't bail out + return new Response(null, { status: 200, headers: { "content-type": "text/html" } }); + } + return new Response("Server Error", { status: 503, headers: {} }); + }) as unknown as typeof fetch; + try { + const result = await extractContent("https://example.com/broken", undefined, { + fetchProviders: [providerA], + }); + expect(result.extractionChain).toEqual(["fetch-provider:jina"]); + expect(providerA.fetch).toHaveBeenCalledOnce(); + } finally { + globalThis.fetch = orig; + } + }); + + it("throws RetryableExtractionError when no provider recovers", async () => { + const providerA: FetchProvider = { + name: "exa", + fetch: vi.fn(async () => { + throw new Error("exa-down"); + }), + }; + const orig = globalThis.fetch; + let calls = 0; + globalThis.fetch = vi.fn(async () => { + calls++; + if (calls === 1) { + return new Response(null, { status: 200, headers: { "content-type": "text/html" } }); + } + return new Response("Server Error", { status: 502, headers: {} }); + }) as unknown as typeof fetch; + try { + await expect( + extractContent("https://example.com/broken", undefined, { + fetchProviders: [providerA], + }), + ).rejects.toThrow(RetryableExtractionError); + } finally { + globalThis.fetch = orig; + } + }); + + it("does not invoke the Gemini adapters when transport fails", async () => { + const providerA: FetchProvider = { + name: "jina", + fetch: vi.fn(async () => { + throw new Error("jina-down"); + }), + }; + const orig = globalThis.fetch; + let calls = 0; + globalThis.fetch = vi.fn(async () => { + calls++; + if (calls === 1) { + return new Response(null, { status: 200, headers: { "content-type": "text/html" } }); + } + return new Response("Server Error", { status: 503, headers: {} }); + }) as unknown as typeof fetch; + try { + await expect( + extractContent("https://example.com/broken", undefined, { + fetchProviders: [providerA], + }), + ).rejects.toThrow(RetryableExtractionError); + expect(extractWithUrlContext).not.toHaveBeenCalled(); + expect(extractWithGeminiWeb).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = orig; + } + }); + + it("does not record budget rejection as a provider failure hook", async () => { + const onFailure = vi.fn(); + const onSuccess = vi.fn(); + const hooks: ExecutionHooks = { onFailure, onSuccess }; + const a: FetchProvider = { + name: "exhausted", + fetch: vi.fn(async () => { + throw new BudgetExceededError("exhausted", 1, { + mode: "hard", + used: 1, + limit: 1, + unit: "request", + period: "month", + periodKey: "2026-08", + }); + }), + }; + const b: FetchProvider = { + name: "ok", + fetch: vi.fn(async () => ({ text: LONG_TEXT })), + }; + const orig = globalThis.fetch; + let calls = 0; + globalThis.fetch = vi.fn(async () => { + calls++; + if (calls === 1) { + return new Response(null, { status: 200, headers: { "content-type": "text/html" } }); + } + return new Response("Server Error", { status: 503, headers: {} }); + }) as unknown as typeof fetch; + try { + const result = await extractContent("https://example.com/broken", undefined, { + fetchProviders: [a, b], + executionHooks: hooks, + }); + expect(result.extractionChain).toContain("fetch-provider:ok"); + expect(onFailure).not.toHaveBeenCalledWith("exhausted"); + expect(onSuccess).toHaveBeenCalledWith("ok", expect.any(Number)); + } finally { + globalThis.fetch = orig; + } + }); +}); + +describe("extractContent — provider isolation from structured routes", () => { + beforeEach(() => { + vi.mocked(isYouTubeURL).mockReturnValue({ isYouTube: false, videoId: null }); + vi.mocked(isYouTubeEnabled).mockReturnValue(false); + vi.mocked(isVideoFile).mockReturnValue(null); + vi.mocked(isVideoEnabled).mockReturnValue(false); + vi.mocked(extractWithUrlContext).mockResolvedValue(null); + vi.mocked(extractWithGeminiWeb).mockResolvedValue(null); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("does not invoke fetch providers for an invalid SSRF URL", async () => { + const providerA: FetchProvider = { + name: "jina", + fetch: vi.fn(async () => ({ text: LONG_TEXT })), + }; + await expect( + extractContent("http://127.0.0.1/page", undefined, { fetchProviders: [providerA] }), + ).rejects.toThrow(); + expect(providerA.fetch).not.toHaveBeenCalled(); + }); + + it("does not invoke fetch providers in raw mode on a successful GET", async () => { + const providerA: FetchProvider = { + name: "jina", + fetch: vi.fn(async () => ({ text: LONG_TEXT })), + }; + const orig = globalThis.fetch; + let calls = 0; + globalThis.fetch = vi.fn(async () => { + calls++; + if (calls === 1) { + return new Response(null, { status: 200, headers: { "content-type": "text/html" } }); + } + return new Response("raw body", { + status: 200, + headers: { "content-type": "text/html" }, + }); + }) as unknown as typeof fetch; + try { + const result = await extractContent("https://example.com/page", undefined, { + raw: true, + fetchProviders: [providerA], + }); + expect(result.extractionChain).toContain("raw"); + expect(providerA.fetch).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = orig; + } + }); + + it("does not invoke fetch providers for binary content type", async () => { + const providerA: FetchProvider = { + name: "jina", + fetch: vi.fn(async () => ({ text: LONG_TEXT })), + }; + const orig = globalThis.fetch; + let calls = 0; + globalThis.fetch = vi.fn(async () => { + calls++; + if (calls === 1) { + return new Response(null, { status: 200, headers: { "content-type": "image/png" } }); + } + return new Response("img-bytes", { status: 200, headers: { "content-type": "image/png" } }); + }) as unknown as typeof fetch; + try { + await expect( + extractContent("https://example.com/photo.png", undefined, { + fetchProviders: [providerA], + }), + ).rejects.toThrow(/binary/i); + expect(providerA.fetch).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = orig; + } + }); +}); + +describe("extractContent — caller cancellation propagation", () => { + beforeEach(() => { + vi.mocked(isYouTubeURL).mockReturnValue({ isYouTube: false, videoId: null }); + vi.mocked(isYouTubeEnabled).mockReturnValue(false); + vi.mocked(isVideoFile).mockReturnValue(null); + vi.mocked(isVideoEnabled).mockReturnValue(false); + vi.mocked(extractWithUrlContext).mockResolvedValue(null); + vi.mocked(extractWithGeminiWeb).mockResolvedValue(null); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it("rejects with the caller's abort when cancelled before pipeline entry", async () => { + const controller = new AbortController(); + controller.abort(); + await expect( + extractContent("https://example.com/article", controller.signal), + ).rejects.toThrow(); + }); + + it("stops after an aborted HEAD probe instead of continuing to GET", async () => { + const controller = new AbortController(); + const reason = new Error("caller stopped"); + const orig = globalThis.fetch; + const fetchMock = vi.fn(async (_input: string | URL | Request, init?: RequestInit) => { + if (init?.method === "HEAD") { + controller.abort(reason); + throw reason; + } + return new Response("GET should not run", { + status: 200, + headers: { "content-type": "text/html" }, + }); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + try { + await expect( + extractContent("https://example.com/article", controller.signal), + ).rejects.toBe(reason); + expect(fetchMock).toHaveBeenCalledOnce(); + } finally { + globalThis.fetch = orig; + } + }); + + it("stops later providers when aborted during provider fallback", async () => { + const controller = new AbortController(); + const abortingProvider: FetchProvider = { + name: "first", + fetch: vi.fn(async () => { + controller.abort(); + return { text: LONG_TEXT }; + }), + }; + const laterProvider: FetchProvider = { + name: "second", + fetch: vi.fn(async () => ({ text: LONG_TEXT })), + }; + + const orig = globalThis.fetch; + let calls = 0; + globalThis.fetch = vi.fn(async () => { + calls++; + if (calls === 1) { + return new Response(null, { status: 200, headers: { "content-type": "text/html" } }); + } + return new Response("Server Error", { status: 503, headers: {} }); + }) as unknown as typeof fetch; + try { + await expect( + extractContent("https://example.com/article", controller.signal, { + fetchProviders: [abortingProvider, laterProvider], + }), + ).rejects.toThrow(); + expect(laterProvider.fetch).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = orig; + } + }); + + it("does not call Gemini Web after URL context observes cancellation", async () => { + const controller = new AbortController(); + const reason = new Error("caller stopped"); + vi.mocked(extractWithUrlContext).mockImplementation(async () => { + controller.abort(reason); + return null; + }); + vi.mocked(extractWithGeminiWeb).mockResolvedValue({ + text: "Gemini content", + url: "https://example.com/article", + extractionChain: ["html:gemini-web"], + chars: 15, + truncated: false, + }); + + const orig = globalThis.fetch; + globalThis.fetch = vi.fn(async () => { + return new Response("thin", { + status: 200, + headers: { "content-type": "text/html" }, + }); + }) as unknown as typeof fetch; + try { + await expect( + extractContent("https://example.com/article", controller.signal), + ).rejects.toBe(reason); + expect(extractWithGeminiWeb).not.toHaveBeenCalled(); + } finally { + globalThis.fetch = orig; + } + }); +}); diff --git a/tests/extract/pipeline-routing.test.ts b/tests/extract/pipeline-routing.test.ts index 2c1b4db..db7ec5e 100644 --- a/tests/extract/pipeline-routing.test.ts +++ b/tests/extract/pipeline-routing.test.ts @@ -288,7 +288,7 @@ describe("extractContent — Gemini HTML fallback", () => { async () => { const result = await extractContent("https://example.com/article"); expect(result.extractionChain).toContain("readability:thin"); - expect(result.extractionChain).toContain("jina-reader:fail"); + expect(result.extractionChain).toContain("fetch-provider:skipped"); expect(result.extractionChain).toContain("html:gemini-url-context"); expect(result.text).toContain("Full Article"); }, @@ -313,7 +313,7 @@ describe("extractContent — Gemini HTML fallback", () => { }), async () => { const result = await extractContent("https://example.com/page"); - expect(result.extractionChain).toContain("jina-reader:fail"); + expect(result.extractionChain).toContain("fetch-provider:skipped"); expect(result.extractionChain).toContain("html:gemini-web"); }, ); @@ -330,7 +330,7 @@ describe("extractContent — Gemini HTML fallback", () => { }), async () => { const result = await extractContent("https://example.com/raw"); - expect(result.extractionChain).toContain("jina-reader:fail"); + expect(result.extractionChain).toContain("fetch-provider:skipped"); expect(result.extractionChain).toContain("raw-text"); expect(result.extractionChain).not.toContain("html:gemini-url-context"); }, diff --git a/tests/extract/pipeline.test.ts b/tests/extract/pipeline.test.ts index 24998a3..afe5ab8 100644 --- a/tests/extract/pipeline.test.ts +++ b/tests/extract/pipeline.test.ts @@ -8,6 +8,11 @@ vi.mock("../../src/extract/gemini-url-context.ts", () => ({ extractWithGeminiWeb: vi.fn().mockResolvedValue(null), })); +// Keep pipeline tests independent of the developer's browser profile/keychain. +vi.mock("../../src/extract/chrome-cookies.ts", () => ({ + getGoogleCookies: vi.fn().mockResolvedValue(null), +})); + const GOOD_HTML = ` Article

Real Article

diff --git a/tests/providers/jina.test.ts b/tests/providers/jina.test.ts index a9d6d0f..e7ff064 100644 --- a/tests/providers/jina.test.ts +++ b/tests/providers/jina.test.ts @@ -59,7 +59,7 @@ describe("JinaProvider", () => { it("fetches content via Jina Reader", async () => { fetchStub.addResponse("r.jina.ai", { - body: "# Page Title\n\nPage content here", + body: "# Page Title\n\n" + "Page content here is long enough to clear the 100-char minimum. ".repeat(3), headers: { "content-type": "text/plain" }, }); @@ -68,6 +68,49 @@ describe("JinaProvider", () => { expect(result.text).toContain("Page content"); }); + it("trims whitespace from the fetched body", async () => { + fetchStub.addResponse("r.jina.ai", { + body: " \n\n# Real Content\n\nThis is a substantial amount of text returned by the Jina Reader API. It must be long enough to clear the minimum content threshold so that the trim does not affect validity. ".repeat(2), + headers: { "content-type": "text/plain" }, + }); + + const provider = new JinaProvider(); + const result = await provider.fetch("https://example.com"); + expect(result.text.startsWith(" ")).toBe(false); + expect(result.text.endsWith(" ")).toBe(false); + }); + + it("throws when fetch returns fewer than 100 characters after trim", async () => { + fetchStub.addResponse("r.jina.ai", { + body: " short ", + headers: { "content-type": "text/plain" }, + }); + + const provider = new JinaProvider(); + await expect(provider.fetch("https://example.com")).rejects.toThrow( + "Jina reader returned insufficient content", + ); + }); + + it("throws on non-2xx fetch responses", async () => { + fetchStub.addResponse("r.jina.ai", { status: 500, body: "Error" }); + const provider = new JinaProvider(); + await expect(provider.fetch("https://example.com")).rejects.toThrow(); + }); + + it("sends Authorization header for fetch when API key is configured", async () => { + fetchStub.addResponse("r.jina.ai", { + body: "# Title\n\n" + "Long enough content from Jina Reader via authenticated request. ".repeat(4), + headers: { "content-type": "text/plain" }, + }); + + const provider = new JinaProvider("secret-key"); + await provider.fetch("https://example.com"); + + const fetchCall = (globalThis.fetch as any).mock.calls[0]; + expect(fetchCall[1].headers).toHaveProperty("Authorization", "Bearer secret-key"); + }); + it("throws on non-2xx response", async () => { fetchStub.addResponse("s.jina.ai", { status: 500, body: "Error" }); const provider = new JinaProvider(); diff --git a/tests/tools/web-fetch-video.test.ts b/tests/tools/web-fetch-video.test.ts index cbf830a..91dc78a 100644 --- a/tests/tools/web-fetch-video.test.ts +++ b/tests/tools/web-fetch-video.test.ts @@ -171,4 +171,29 @@ describe("web_fetch — video parameters and ImageContent", () => { expect(imageBlocks).toHaveLength(1); expect(imageBlocks[0]).toEqual({ type: "image", data: "frameData", mimeType: "image/jpeg" }); }); + + it("forwards resolved fetch candidates and execution hooks to extractContent", async () => { + const provider = { name: "jina", fetch: vi.fn() }; + const hooks = { onSuccess: vi.fn(), onFailure: vi.fn() }; + const store = new ContentStore(() => {}); + const tool = createWebFetchTool(store, () => [provider], undefined, undefined, hooks); + const ctx = makeCtx(); + + await tool.execute( + "call-vid-seam", + { url: "https://example.com/page" }, + undefined, + undefined, + ctx, + ); + + expect(extractContent).toHaveBeenCalledWith( + "https://example.com/page", + undefined, + expect.objectContaining({ + fetchProviders: [provider], + executionHooks: hooks, + }), + ); + }); }); diff --git a/tests/tools/web-fetch.test.ts b/tests/tools/web-fetch.test.ts index fadc698..e448b19 100644 --- a/tests/tools/web-fetch.test.ts +++ b/tests/tools/web-fetch.test.ts @@ -5,6 +5,16 @@ import { ContentCache } from "../../src/cache.ts"; import { makeCtx, stubFetch } from "../helpers.ts"; import type { FetchProvider, FetchResult } from "../../src/providers/types.ts"; +// These tests exercise web_fetch routing, not Gemini or the developer's browser profile. +vi.mock("../../src/extract/gemini-url-context.ts", () => ({ + extractWithUrlContext: vi.fn().mockResolvedValue(null), + extractWithGeminiWeb: vi.fn().mockResolvedValue(null), +})); + +vi.mock("../../src/extract/chrome-cookies.ts", () => ({ + getGoogleCookies: vi.fn().mockResolvedValue(null), +})); + const GOOD_HTML = ` Test

Article Title

@@ -573,7 +583,6 @@ describe("web_fetch multi-URL", () => { expect(recovered).toBeDefined(); expect(recovered?.error).toBeUndefined(); expect(failed?.error).toContain("HTTP 503"); - expect(failed?.error).toContain("Provider unavailable"); const cachedResult = await tool.execute( "call-m-fallback-cached",