docs: update fetcher spec and add integration tests

claude · claude · commit 08d31537a890 · 2026-01-17T05:40:04.000Z
- Update specs/fetchers.md with complete API documentation
- Add section on how to create new fetchers
- Add integration tests for FetcherRegistry
- Test URL validation, allow/block lists, conversion options
diff --git a/crates/fetchkit/tests/integration.rs b/crates/fetchkit/tests/integration.rs
@@ -1,6 +1,8 @@
 //! Integration tests for FetchKit using wiremock
 
-use fetchkit::{fetch, FetchRequest, HttpMethod, Tool};
+use fetchkit::{
+    fetch, fetch_with_options, FetchOptions, FetchRequest, FetcherRegistry, HttpMethod, Tool,
+};
 use wiremock::matchers::{method, path};
 use wiremock::{Mock, MockServer, ResponseTemplate};
 
@@ -442,3 +444,158 @@ async fn test_excessive_newlines_filtered() {
     // Should have at most 2 consecutive newlines
     assert!(!resp.content.unwrap().contains("\n\n\n"));
 }
+
+// ============================================================================
+// Fetcher System Integration Tests
+// ============================================================================
+
+#[tokio::test]
+async fn test_fetcher_registry_with_defaults() {
+    let mock_server = MockServer::start().await;
+
+    Mock::given(method("GET"))
+        .and(path("/page"))
+        .respond_with(
+            ResponseTemplate::new(200)
+                .set_body_string("<html><body><h1>Test</h1></body></html>")
+                .insert_header("content-type", "text/html"),
+        )
+        .mount(&mock_server)
+        .await;
+
+    let registry = FetcherRegistry::with_defaults();
+    let options = FetchOptions {
+        enable_markdown: true,
+        enable_text: true,
+        ..Default::default()
+    };
+
+    let req = FetchRequest::new(format!("{}/page", mock_server.uri())).as_markdown();
+    let resp = registry.fetch(req, options).await.unwrap();
+
+    assert_eq!(resp.status_code, 200);
+    assert_eq!(resp.format, Some("markdown".to_string()));
+    assert!(resp.content.unwrap().contains("# Test"));
+}
+
+#[tokio::test]
+async fn test_fetcher_registry_url_validation() {
+    let registry = FetcherRegistry::with_defaults();
+    let options = FetchOptions::default();
+
+    // Invalid scheme
+    let req = FetchRequest::new("ftp://example.com");
+    let result = registry.fetch(req, options.clone()).await;
+    assert!(result.is_err());
+    assert!(result.unwrap_err().to_string().contains("http://"));
+
+    // Empty URL handled by fetch_with_options before registry
+    let req = FetchRequest::new("");
+    let result = fetch_with_options(req, options).await;
+    assert!(result.is_err());
+}
+
+#[tokio::test]
+async fn test_fetcher_registry_allow_block_lists() {
+    let mock_server = MockServer::start().await;
+
+    Mock::given(method("GET"))
+        .and(path("/"))
+        .respond_with(ResponseTemplate::new(200).set_body_string("OK"))
+        .mount(&mock_server)
+        .await;
+
+    let registry = FetcherRegistry::with_defaults();
+
+    // Block list
+    let options = FetchOptions {
+        block_prefixes: vec!["http://127.0.0.1".to_string()],
+        ..Default::default()
+    };
+    let req = FetchRequest::new(format!("{}/", mock_server.uri()));
+    let result = registry.fetch(req, options).await;
+    assert!(result.is_err());
+    assert!(result.unwrap_err().to_string().contains("Blocked"));
+
+    // Allow list (not matching)
+    let options = FetchOptions {
+        allow_prefixes: vec!["https://allowed.com".to_string()],
+        ..Default::default()
+    };
+    let req = FetchRequest::new(format!("{}/", mock_server.uri()));
+    let result = registry.fetch(req, options).await;
+    assert!(result.is_err());
+}
+
+#[tokio::test]
+async fn test_github_fetcher_url_matching() {
+    // These URLs should NOT match GitHubRepoFetcher (will use DefaultFetcher)
+    let mock_server = MockServer::start().await;
+
+    // Mock for non-GitHub URLs
+    Mock::given(method("GET"))
+        .and(path("/owner/repo/issues"))
+        .respond_with(
+            ResponseTemplate::new(200)
+                .set_body_string("issues page")
+                .insert_header("content-type", "text/plain"),
+        )
+        .mount(&mock_server)
+        .await;
+
+    let req = FetchRequest::new(format!("{}/owner/repo/issues", mock_server.uri()));
+    let resp = fetch(req).await.unwrap();
+
+    // Should use default fetcher (format is "raw", not "github_repo")
+    assert_eq!(resp.format, Some("raw".to_string()));
+    assert!(resp.content.unwrap().contains("issues page"));
+}
+
+#[tokio::test]
+async fn test_fetch_enables_conversions_by_default() {
+    let mock_server = MockServer::start().await;
+
+    Mock::given(method("GET"))
+        .and(path("/"))
+        .respond_with(
+            ResponseTemplate::new(200)
+                .set_body_string("<html><body><p>Hello</p></body></html>")
+                .insert_header("content-type", "text/html"),
+        )
+        .mount(&mock_server)
+        .await;
+
+    // Using fetch() with as_markdown() should work
+    let req = FetchRequest::new(format!("{}/", mock_server.uri())).as_markdown();
+    let resp = fetch(req).await.unwrap();
+
+    assert_eq!(resp.format, Some("markdown".to_string()));
+}
+
+#[tokio::test]
+async fn test_fetch_with_options_respects_disabled_conversion() {
+    let mock_server = MockServer::start().await;
+
+    Mock::given(method("GET"))
+        .and(path("/"))
+        .respond_with(
+            ResponseTemplate::new(200)
+                .set_body_string("<html><body><p>Hello</p></body></html>")
+                .insert_header("content-type", "text/html"),
+        )
+        .mount(&mock_server)
+        .await;
+
+    // Disable markdown conversion
+    let options = FetchOptions {
+        enable_markdown: false,
+        enable_text: false,
+        ..Default::default()
+    };
+
+    let req = FetchRequest::new(format!("{}/", mock_server.uri())).as_markdown();
+    let resp = fetch_with_options(req, options).await.unwrap();
+
+    // Should be raw because conversion is disabled
+    assert_eq!(resp.format, Some("raw".to_string()));
+}
diff --git a/specs/fetchers.md b/specs/fetchers.md
@@ -22,48 +22,65 @@ Central dispatcher that:
 2. Iterates fetchers, uses first matching one
 3. Falls back to default fetcher if none match
 4. Provides `register()` for adding custom fetchers
+5. Validates URL scheme and allow/block lists before dispatching
 
 ### Built-in Fetchers
 
 #### DefaultFetcher (lowest priority)
 
 - Matches: All HTTP/HTTPS URLs
-- Behavior: Current `client.rs` fetch logic
-- Returns: Standard `FetchResponse` with HTML conversion
+- Behavior: Standard HTTP fetch with HTML conversion support
+- Features:
+  - GET and HEAD methods
+  - HTML to markdown/text conversion (when enabled)
+  - Binary content detection (returns metadata only)
+  - Timeout handling with partial content support
+- Returns: Standard `FetchResponse` with format `"markdown"`, `"text"`, or `"raw"`
 
 #### GitHubRepoFetcher
 
-- Matches: `https://github.com/{owner}/{repo}` (exactly 2 path segments, no file paths)
+- Matches: `https://github.com/{owner}/{repo}` (exactly 2 path segments)
+- Excludes: Reserved paths (settings, explore, trending, etc.)
 - Behavior:
   1. Fetch repo metadata via GitHub API (`/repos/{owner}/{repo}`)
   2. Fetch README content if exists (`/repos/{owner}/{repo}/readme`)
-  3. Combine into structured response
-- Returns: Markdown with repo metadata header + README content
+  3. Decode base64 README content
+  4. Combine into structured markdown response
+- Returns: Markdown with repo metadata + README content
 - Response format field: `"github_repo"`
+- Metadata includes: stars, forks, issues, language, license, topics, dates
 
 ### Response Extensions
 
-`FetchResponse.format` gains new values:
+`FetchResponse.format` values:
+- `"markdown"` - HTML converted to markdown
+- `"text"` - HTML converted to plain text
+- `"raw"` - Original content unchanged
 - `"github_repo"` - GitHub repository metadata + README
 
 ### Configuration
 
 Fetchers receive `FetchOptions` for:
-- User-Agent configuration
-- Allow/block URL lists (applied before fetcher matching)
+- `user_agent` - Custom User-Agent string
+- `allow_prefixes` - URL prefix allow list
+- `block_prefixes` - URL prefix block list
+- `enable_markdown` - Enable markdown conversion
+- `enable_text` - Enable text conversion
 
 ### Extensibility
 
 Design supports hundreds of fetchers by:
 - Each fetcher in separate file under `fetchers/` module
-- Simple registration pattern
+- Simple registration pattern via `registry.register()`
 - No compile-time limit on fetcher count
+- Priority determined by registration order
 
 ### Error Handling
 
 - Fetcher errors bubble up as `FetchError`
 - If specialized fetcher fails, does NOT fall back to default (explicit failure)
-- Add `FetchError::FetcherError(String)` for fetcher-specific errors
+- `FetchError::FetcherError(String)` for fetcher-specific errors
+- GitHub API errors return response with error field set
 
 ## Module Structure
 
@@ -75,10 +92,10 @@ crates/fetchkit/src/
 │   └── github_repo.rs   # GitHubRepoFetcher
 ```
 
-## API Changes
+## API
 
 ```rust
-// New trait
+// Fetcher trait
 #[async_trait]
 pub trait Fetcher: Send + Sync {
     fn name(&self) -> &'static str;
@@ -99,10 +116,37 @@ impl FetcherRegistry {
     pub async fn fetch(&self, request: FetchRequest, options: FetchOptions)
         -> Result<FetchResponse, FetchError>;
 }
+
+// Convenience functions
+pub async fn fetch(req: FetchRequest) -> Result<FetchResponse, FetchError>;
+pub async fn fetch_with_options(req: FetchRequest, options: FetchOptions)
+    -> Result<FetchResponse, FetchError>;
 ```
 
 ## Testing
 
-- Unit tests per fetcher with mocked HTTP
-- Integration tests for registry dispatch
-- GitHub fetcher tests with mocked GitHub API responses
+### Unit Tests
+- Per-fetcher tests with mocked HTTP (wiremock)
+- URL matching logic tests
+- Response parsing tests
+
+### Integration Tests
+- Registry dispatch tests
+- End-to-end fetch tests with mock server
+
+### Example-based Tests
+Run with: `cargo run -p fetchkit --example fetch_urls`
+
+Tests real URLs:
+- Simple HTML pages (example.com)
+- JSON endpoints (httpbin.org)
+- GitHub repositories
+- Raw file content
+
+## Adding a New Fetcher
+
+1. Create `crates/fetchkit/src/fetchers/{name}.rs`
+2. Implement `Fetcher` trait
+3. Add `mod {name};` and `pub use {name}::*;` to `mod.rs`
+4. Register in `FetcherRegistry::with_defaults()` (before DefaultFetcher)
+5. Add test cases to `examples/fetch_urls.rs`
