From 7d0994383ae443502e34da5a94eb81daadb6b3fc Mon Sep 17 00:00:00 2001 From: Shanu Date: Wed, 19 Aug 2026 00:03:49 +0530 Subject: [PATCH 1/2] =?UTF-8?q?Move=20the=20memory-source=20contracts=20an?= =?UTF-8?q?d=20readers=20off=20the=20engine=20(#18=20=C2=A7B4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `core/src/sources/` described a source in the *engine's* vocabulary: its types, registry and readers were all re-exports of, or thin delegations to, `crate::engine::backend::sources`. A host binding a different driver could not so much as describe a source. They live in a new `tinymemory-sources` crate now, which names no engine. What moved, and the four decisions inside the move -------------------------------------------------- 5,952 lines: the source types, field validation, the `sources.toml` registry, and the folder/conversation/GitHub/RSS/web-page readers with their tests. Four engine couplings needed a decision rather than a rewrite: 1. `MemoryConfig` -- the readers used exactly one field of it, `config.workspace`. They take `&Path` now. A narrower signature instead of an imported config type. 2. `MemoryEngineResult`/`MemoryError` -- the contract's `MemoryError` already models `PathEscape`, the one variant `ensure_within_base` needs, so `SourceResult = Result` is an exact retype, not a widening. A reader now fails in the same vocabulary as the driver that stores what it read. 3. `RawKind` -- defined locally, deliberately a second copy of the engine's. Safe because it never crosses a boundary: `raw_archive_coords` is its only consumer, nothing outside the crate calls that, and core never touches the type. Documented as such at the definition. 4. The engine's `sync` feature gate -- renamed `network`, because `tinymemory-sync` is a sibling crate and one name for two things is how this workspace's `SourceKind` confusion started. The gate earns its keep: default links 63 crates and no HTTP stack; `network` (63 -> 150) adds the GitHub/RSS/web-page readers and is what core asks for. Deliberately NOT done: renaming `SourceItem`/`SourceKind`, which collide in name (not in scope) with different contract-crate concepts. The pairs never meet, and the churn would be ~150 call sites here plus 24 in OpenHuman. The crate doc records the distinction instead. Core keeps its own `SourceReader` trait (over the host `Config`) and its two host-only readers (composio, twitter); only the delegation target changed. OpenHuman imports three functions from `core::sources` -- `apply_kind_defaults`, `get_source`, `list_sources` -- all still re-exported, so downstream is untouched. The engine's copy of `sources/` stays until the tinycortex companion PR deletes it, the same two-step §B3 used (tinymemory#41 + tinycortex#153). The moved code lands under this crate's stricter lints (`unwrap_used`, `expect_used`, `missing_docs` warn): two guarded `unwrap`s rewritten to carry their proof (`is_some_and`, index in the `Option`), the registry's lock-poison `expect` replaced with `PoisonError::into_inner` recovery, reader structs documented. Tests keep `unwrap` by a scoped `cfg_attr(test, allow)` -- a panic in a test is the failure report. --- Cargo.lock | 23 + Cargo.toml | 4 +- core/Cargo.toml | 4 + core/src/sources/readers/conversation.rs | 14 +- core/src/sources/readers/folder.rs | 14 +- core/src/sources/readers/github.rs | 16 +- core/src/sources/readers/rss.rs | 14 +- core/src/sources/readers/web_page.rs | 17 +- core/src/sources/registry.rs | 13 +- core/src/sources/sync.rs | 4 +- core/src/sources/types.rs | 16 +- sources/Cargo.toml | 72 ++++ sources/src/lib.rs | 63 +++ sources/src/raw_kind.rs | 49 +++ sources/src/readers/conversation.rs | 162 ++++++++ sources/src/readers/conversation_tests.rs | 218 ++++++++++ sources/src/readers/folder.rs | 233 +++++++++++ sources/src/readers/folder_tests.rs | 151 +++++++ sources/src/readers/github.rs | 346 +++++++++++++++ sources/src/readers/github/api.rs | 360 ++++++++++++++++ sources/src/readers/github/git.rs | 320 ++++++++++++++ sources/src/readers/github/git_tests.rs | 122 ++++++ sources/src/readers/github/issues.rs | 300 +++++++++++++ sources/src/readers/github/types.rs | 122 ++++++ sources/src/readers/github_tests.rs | 293 +++++++++++++ sources/src/readers/mod.rs | 112 +++++ sources/src/readers/rss.rs | 402 ++++++++++++++++++ sources/src/readers/rss/types.rs | 24 ++ sources/src/readers/rss_tests.rs | 243 +++++++++++ sources/src/readers/ssrf.rs | 226 ++++++++++ sources/src/readers/ssrf_tests.rs | 165 ++++++++ sources/src/readers/web_page.rs | 485 ++++++++++++++++++++++ sources/src/readers/web_page/types.rs | 12 + sources/src/readers/web_page_tests.rs | 246 +++++++++++ sources/src/registry.rs | 355 ++++++++++++++++ sources/src/registry_tests.rs | 311 ++++++++++++++ sources/src/types.rs | 403 ++++++++++++++++++ sources/src/types_tests.rs | 261 ++++++++++++ sources/src/validation.rs | 82 ++++ sources/src/validation_tests.rs | 77 ++++ 40 files changed, 6305 insertions(+), 49 deletions(-) create mode 100644 sources/Cargo.toml create mode 100644 sources/src/lib.rs create mode 100644 sources/src/raw_kind.rs create mode 100644 sources/src/readers/conversation.rs create mode 100644 sources/src/readers/conversation_tests.rs create mode 100644 sources/src/readers/folder.rs create mode 100644 sources/src/readers/folder_tests.rs create mode 100644 sources/src/readers/github.rs create mode 100644 sources/src/readers/github/api.rs create mode 100644 sources/src/readers/github/git.rs create mode 100644 sources/src/readers/github/git_tests.rs create mode 100644 sources/src/readers/github/issues.rs create mode 100644 sources/src/readers/github/types.rs create mode 100644 sources/src/readers/github_tests.rs create mode 100644 sources/src/readers/mod.rs create mode 100644 sources/src/readers/rss.rs create mode 100644 sources/src/readers/rss/types.rs create mode 100644 sources/src/readers/rss_tests.rs create mode 100644 sources/src/readers/ssrf.rs create mode 100644 sources/src/readers/ssrf_tests.rs create mode 100644 sources/src/readers/web_page.rs create mode 100644 sources/src/readers/web_page/types.rs create mode 100644 sources/src/readers/web_page_tests.rs create mode 100644 sources/src/registry.rs create mode 100644 sources/src/registry_tests.rs create mode 100644 sources/src/types.rs create mode 100644 sources/src/types_tests.rs create mode 100644 sources/src/validation.rs create mode 100644 sources/src/validation_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 56cccda..68faf84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1863,6 +1863,7 @@ dependencies = [ "tinymemory", "tinymemory-api", "tinymemory-conformance", + "tinymemory-sources", "tinymemory-sync", "tokio", "tracing", @@ -1887,6 +1888,28 @@ dependencies = [ "tokio", ] +[[package]] +name = "tinymemory-sources" +version = "0.1.0" +dependencies = [ + "anyhow", + "async-trait", + "chrono", + "futures", + "regex", + "reqwest", + "schemars", + "serde", + "serde_json", + "tempfile", + "tinymemory-api", + "tokio", + "toml 0.9.12+spec-1.1.0", + "tracing", + "uuid", + "walkdir", +] + [[package]] name = "tinymemory-sync" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 132779b..5709509 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [workspace] # `sync` is the engine-neutral Composio normalisers (issue #18 §B3). -members = [".", "api", "core", "sync", "adapters/tinycortex", "adapters/remote", "conformance"] -default-members = [".", "api", "core", "sync", "adapters/tinycortex", "adapters/remote", "conformance"] +members = [".", "api", "core", "sync", "sources", "adapters/tinycortex", "adapters/remote", "conformance"] +default-members = [".", "api", "core", "sync", "sources", "adapters/tinycortex", "adapters/remote", "conformance"] # `vendor/` holds engine submodules (tinycortex, tinybus, tinyagents), each of # which is its own workspace with its own lockfile. Same exclusion # `vendor/tinycortex` uses for its own nested vendor directory. diff --git a/core/Cargo.toml b/core/Cargo.toml index 25d9b17..87f2e81 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -21,6 +21,10 @@ tinymemory-api = { path = "../api" } # different engine could not have Composio sync despite none of this code # caring which engine is bound. tinymemory-sync = { path = "../sync" } +# The memory-source contracts and readers (#18 §B4). `network` because this +# crate's sync path drives the GitHub/RSS/web-page readers; a host that only +# reads local folders can take the crate without them. +tinymemory-sources = { path = "../sources", features = ["network"] } # The default embedded engine. `store/`, `tree/` and `sync/` drive it directly; # `tinycortex-api` is a direct dependency because `tinycortex::memory` aliases diff --git a/core/src/sources/readers/conversation.rs b/core/src/sources/readers/conversation.rs index 2380821..fc9bdeb 100644 --- a/core/src/sources/readers/conversation.rs +++ b/core/src/sources/readers/conversation.rs @@ -1,4 +1,4 @@ -//! Product `Config` adapter for the tinycortex conversation reader. +//! Product `Config` adapter for the engine-neutral conversation reader. use async_trait::async_trait; @@ -19,10 +19,10 @@ impl SourceReader for ConversationReader { source: &MemorySourceEntry, config: &Config, ) -> Result, String> { - crate::engine::backend::sources::SourceReader::list_items( - &crate::engine::backend::sources::readers::conversation::ConversationReader, + tinymemory_sources::readers::SourceReader::list_items( + &tinymemory_sources::readers::conversation::ConversationReader, source, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) @@ -34,11 +34,11 @@ impl SourceReader for ConversationReader { item_id: &str, config: &Config, ) -> Result { - crate::engine::backend::sources::SourceReader::read_item( - &crate::engine::backend::sources::readers::conversation::ConversationReader, + tinymemory_sources::readers::SourceReader::read_item( + &tinymemory_sources::readers::conversation::ConversationReader, source, item_id, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) diff --git a/core/src/sources/readers/folder.rs b/core/src/sources/readers/folder.rs index 56bbbfb..53d7a5d 100644 --- a/core/src/sources/readers/folder.rs +++ b/core/src/sources/readers/folder.rs @@ -1,4 +1,4 @@ -//! Product `Config` adapter for the tinycortex folder reader. +//! Product `Config` adapter for the engine-neutral folder reader. use async_trait::async_trait; @@ -19,10 +19,10 @@ impl SourceReader for FolderReader { source: &MemorySourceEntry, config: &Config, ) -> Result, String> { - crate::engine::backend::sources::SourceReader::list_items( - &crate::engine::backend::sources::readers::folder::FolderReader, + tinymemory_sources::readers::SourceReader::list_items( + &tinymemory_sources::readers::folder::FolderReader, source, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) @@ -34,11 +34,11 @@ impl SourceReader for FolderReader { item_id: &str, config: &Config, ) -> Result { - crate::engine::backend::sources::SourceReader::read_item( - &crate::engine::backend::sources::readers::folder::FolderReader, + tinymemory_sources::readers::SourceReader::read_item( + &tinymemory_sources::readers::folder::FolderReader, source, item_id, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) diff --git a/core/src/sources/readers/github.rs b/core/src/sources/readers/github.rs index b35af29..678d35f 100644 --- a/core/src/sources/readers/github.rs +++ b/core/src/sources/readers/github.rs @@ -1,4 +1,4 @@ -//! Product `Config` adapter for the tinycortex GitHub repo reader. +//! Product `Config` adapter for the engine-neutral GitHub repo reader. //! //! The reader itself — commit/issue/PR fetching over `gh`, `git`, and the //! public REST API — lives in the engine. This module keeps the host-side @@ -12,7 +12,7 @@ use crate::sources::readers::SourceReader; use crate::sources::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; use crate::Config; -pub use crate::engine::backend::sources::readers::github::{ +pub use tinymemory_sources::readers::github::{ repo_archive_source_id, repo_chunk_scope, }; @@ -29,10 +29,10 @@ impl SourceReader for GithubReader { source: &MemorySourceEntry, config: &Config, ) -> Result, String> { - crate::engine::backend::sources::SourceReader::list_items( - &crate::engine::backend::sources::readers::github::GithubReader, + tinymemory_sources::readers::SourceReader::list_items( + &tinymemory_sources::readers::github::GithubReader, source, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) @@ -44,11 +44,11 @@ impl SourceReader for GithubReader { item_id: &str, config: &Config, ) -> Result { - crate::engine::backend::sources::SourceReader::read_item( - &crate::engine::backend::sources::readers::github::GithubReader, + tinymemory_sources::readers::SourceReader::read_item( + &tinymemory_sources::readers::github::GithubReader, source, item_id, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) diff --git a/core/src/sources/readers/rss.rs b/core/src/sources/readers/rss.rs index d6213a2..a7ac469 100644 --- a/core/src/sources/readers/rss.rs +++ b/core/src/sources/readers/rss.rs @@ -1,4 +1,4 @@ -//! Product `Config` adapter for the tinycortex RSS/Atom feed reader. +//! Product `Config` adapter for the engine-neutral RSS/Atom feed reader. use async_trait::async_trait; @@ -12,13 +12,13 @@ use crate::Config; /// `read_item`, so constructing it per trait call would turn one sync into /// N+1 downloads. pub struct RssReader { - inner: crate::engine::backend::sources::readers::rss::RssReader, + inner: tinymemory_sources::readers::rss::RssReader, } impl RssReader { pub fn new() -> Self { Self { - inner: crate::engine::backend::sources::readers::rss::RssReader::new(), + inner: tinymemory_sources::readers::rss::RssReader::new(), } } } @@ -40,10 +40,10 @@ impl SourceReader for RssReader { source: &MemorySourceEntry, config: &Config, ) -> Result, String> { - crate::engine::backend::sources::SourceReader::list_items( + tinymemory_sources::readers::SourceReader::list_items( &self.inner, source, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) @@ -55,11 +55,11 @@ impl SourceReader for RssReader { item_id: &str, config: &Config, ) -> Result { - crate::engine::backend::sources::SourceReader::read_item( + tinymemory_sources::readers::SourceReader::read_item( &self.inner, source, item_id, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) diff --git a/core/src/sources/readers/web_page.rs b/core/src/sources/readers/web_page.rs index 69723c8..8603d29 100644 --- a/core/src/sources/readers/web_page.rs +++ b/core/src/sources/readers/web_page.rs @@ -1,4 +1,7 @@ -//! Product `Config` adapter for the tinycortex single-page web reader. +//! Product `Config` adapter for the engine-neutral single-page web reader. +//! +//! The reader itself lives in `tinymemory-sources` (#18 §B4); this adapts the +//! host's `Config` to the workspace path it takes. use async_trait::async_trait; @@ -19,10 +22,10 @@ impl SourceReader for WebPageReader { source: &MemorySourceEntry, config: &Config, ) -> Result, String> { - crate::engine::backend::sources::SourceReader::list_items( - &crate::engine::backend::sources::readers::web_page::WebPageReader, + tinymemory_sources::readers::SourceReader::list_items( + &tinymemory_sources::readers::web_page::WebPageReader, source, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) @@ -34,11 +37,11 @@ impl SourceReader for WebPageReader { item_id: &str, config: &Config, ) -> Result { - crate::engine::backend::sources::SourceReader::read_item( - &crate::engine::backend::sources::readers::web_page::WebPageReader, + tinymemory_sources::readers::SourceReader::read_item( + &tinymemory_sources::readers::web_page::WebPageReader, source, item_id, - &crate::engine::memory_config_from(config, config.workspace_dir().clone()), + config.workspace_dir(), ) .await .map_err(|error| error.to_string()) diff --git a/core/src/sources/registry.rs b/core/src/sources/registry.rs index f5c90b9..66bad33 100644 --- a/core/src/sources/registry.rs +++ b/core/src/sources/registry.rs @@ -1,11 +1,14 @@ -//! Product config discovery and locking around tinycortex source registry CRUD. +//! Product config discovery and locking around the source registry's CRUD. +//! +//! The registry itself moved to `tinymemory-sources` (#18 §B4); this layer adds +//! the host's config path and the lock that serialises writes to it. use std::sync::OnceLock; use crate::config_loader as config_rpc; use crate::sources::types::{MemorySourceEntry, SourceKind}; -pub use crate::engine::backend::sources::{ +pub use tinymemory_sources::{ memory_sync_defaults_for_toolkit, ComposioUpsertTarget, MemorySourcePatch, }; @@ -18,9 +21,9 @@ pub(crate) async fn memory_sources_write_guard() -> tokio::sync::MutexGuard<'sta .await } -async fn registry() -> Result { +async fn registry() -> Result { let config = config_rpc::load_config_with_timeout().await?; - Ok(crate::engine::backend::sources::SourceRegistry::new( + Ok(tinymemory_sources::registry::SourceRegistry::new( config.config_path(), )) } @@ -59,7 +62,7 @@ pub fn get_source_in( config: &crate::Config, id: &str, ) -> Result, String> { - crate::engine::backend::sources::SourceRegistry::new(config.config_path().clone()) + tinymemory_sources::registry::SourceRegistry::new(config.config_path().clone()) .get(id) .map_err(|error| error.to_string()) } diff --git a/core/src/sources/sync.rs b/core/src/sources/sync.rs index 134a6e8..4ae194e 100644 --- a/core/src/sources/sync.rs +++ b/core/src/sources/sync.rs @@ -1,6 +1,6 @@ //! Per-source sync dispatcher. //! -//! Thin routing layer: dispatches supported sources through tinycortex and +//! Thin routing layer: dispatches supported sources through the engine and //! retains the product-owned background lock, events, and reconcile shell. //! - Twitter → placeholder //! @@ -382,7 +382,7 @@ pub fn derive_scopes(source: &MemorySourceEntry, config: &Config) -> Vec = Result; + +/// Largest file a folder source will read. +/// +/// Moved with the readers: it is a reader policy, and the engine's config was +/// only its previous address. +pub const FOLDER_FILE_SIZE_CAP_BYTES: u64 = 10 * 1024 * 1024; + +pub use registry::{memory_sync_defaults_for_toolkit, ComposioUpsertTarget, SourceRegistry}; +pub use types::{ + ContentType, MemorySourceEntry, MemorySourcePatch, SourceContent, SourceItem, SourceKind, +}; diff --git a/sources/src/raw_kind.rs b/sources/src/raw_kind.rs new file mode 100644 index 0000000..d56c6a1 --- /dev/null +++ b/sources/src/raw_kind.rs @@ -0,0 +1,49 @@ +//! Which sub-directory of the raw archive an item belongs in. +//! +//! Moved with the readers (#18 §B4) rather than imported, because importing it +//! would mean depending on the engine for one dependency-free enum — the exact +//! coupling this move removes. +//! +//! This is a deliberate second definition, and it is safe because it never +//! crosses a boundary: `raw_archive_coords` is the only consumer, nothing +//! outside this crate calls it, and the engine keeps its own copy for its +//! storage layer. If a caller ever needs to exchange one, that is the moment to +//! lift it into the contract instead. + +/// Category of a raw item, selecting the per-kind subdirectory under +/// `raw///`. +#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)] +pub enum RawKind { + /// Email messages (Gmail, Outlook, …). + Email, + /// Chat / DM messages (Slack, Telegram, WhatsApp, Discord, …). + Chat, + /// Standalone documents — Notion pages, Drive files, attachments. + Document, + /// One file per person reachable via this source. + Contact, + /// Long-form posts — LinkedIn posts, tweets, blog entries. + Post, + /// Git commits (one file per commit) — GitHub repo sources. + Commit, + /// Issues with their conversation + metadata — GitHub repo sources. + Issue, + /// Pull requests with their body + metadata — GitHub repo sources. + PullRequest, +} + +impl RawKind { + /// Directory name used on disk for this kind (plural). + pub const fn as_dir(&self) -> &'static str { + match self { + Self::Email => "emails", + Self::Chat => "chats", + Self::Document => "documents", + Self::Contact => "contacts", + Self::Post => "posts", + Self::Commit => "commits", + Self::Issue => "issues", + Self::PullRequest => "prs", + } + } +} diff --git a/sources/src/readers/conversation.rs b/sources/src/readers/conversation.rs new file mode 100644 index 0000000..6602738 --- /dev/null +++ b/sources/src/readers/conversation.rs @@ -0,0 +1,162 @@ +//! Conversation source reader. +//! +//! Treats every agent conversation thread as a memory source item. Threads are +//! JSON files under `/threads/`; when synced, each thread's messages +//! are rendered to markdown and stored as durable memory alongside other +//! sources. +//! +//! Safety: `item_id` is rejected if it contains path separators or `..`, and the +//! resolved file is re-checked for containment within the threads directory. + +use async_trait::async_trait; + + +use tinymemory_api::error::MemoryError; + +use crate::SourceResult; +use crate::types::{ + ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, +}; +use crate::validation::ensure_within_base; + +use super::SourceReader; + +/// A reader over local agent conversation threads. +pub struct ConversationReader; + +#[async_trait] +impl SourceReader for ConversationReader { + fn kind(&self) -> SourceKind { + SourceKind::Conversation + } + + async fn list_items( + &self, + _source: &MemorySourceEntry, + workspace: &std::path::Path, + ) -> SourceResult> { + let threads_dir = workspace.join("threads"); + if !threads_dir.exists() { + return Ok(Vec::new()); + } + + let mut items = Vec::new(); + for entry in std::fs::read_dir(&threads_dir)? { + let entry = match entry { + Ok(e) => e, + Err(_) => continue, + }; + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) != Some("json") { + continue; + } + let id = path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or_default() + .to_string(); + + let modified_ms = entry + .metadata() + .ok() + .and_then(|m| m.modified().ok()) + .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) + .map(|d| d.as_millis() as i64); + + items.push(SourceItem { + title: id.clone(), + id, + updated_at_ms: modified_ms, + }); + } + + Ok(items) + } + + /// Read one thread's content by its `item_id` (the thread's file stem, as + /// produced by [`list_items`](Self::list_items)). + /// + async fn read_item( + &self, + _source: &MemorySourceEntry, + item_id: &str, + workspace: &std::path::Path, + ) -> SourceResult { + // Validate item_id to prevent path traversal before touching the FS. + if matches!(item_id, "." | "..") || item_id.contains('/') || item_id.contains('\\') { + return Err(MemoryError::Invalid( + "invalid item_id: path traversal denied".to_string(), + )); + } + + let threads_dir = workspace.join("threads"); + let thread_path = threads_dir.join(format!("{item_id}.json")); + + if !thread_path.exists() { + return Err(MemoryError::NotFound(format!( + "thread '{item_id}' not found" + ))); + } + + // Re-check containment after resolving symlinks. + ensure_within_base(&threads_dir, &thread_path)?; + + let raw = std::fs::read_to_string(&thread_path)?; + let parsed: serde_json::Value = serde_json::from_str(&raw)?; + + let title = parsed + .get("title") + .and_then(|v| v.as_str()) + .unwrap_or(item_id) + .to_string(); + + let body = format_thread_as_markdown(&parsed); + + Ok(SourceContent { + id: item_id.to_string(), + title, + body, + content_type: ContentType::Markdown, + metadata: serde_json::json!({ + "source_type": "conversation", + "thread_id": item_id, + }), + }) + } +} + +/// Render a thread JSON value (`{ title, messages: [{ role, content }] }`) to +/// markdown. Messages with empty content are skipped. +fn format_thread_as_markdown(thread: &serde_json::Value) -> String { + let mut out = String::new(); + + if let Some(title) = thread.get("title").and_then(|v| v.as_str()) { + out.push_str(&format!("# {title}\n\n")); + } + + let messages = thread + .get("messages") + .and_then(|v| v.as_array()) + .cloned() + .unwrap_or_default(); + + for msg in &messages { + let role = msg + .get("role") + .and_then(|v| v.as_str()) + .unwrap_or("unknown"); + let content = msg.get("content").and_then(|v| v.as_str()).unwrap_or(""); + + if content.is_empty() { + continue; + } + + out.push_str(&format!("**{role}**: {content}\n\n")); + } + + out +} + +#[cfg(test)] +#[path = "conversation_tests.rs"] +mod tests; diff --git a/sources/src/readers/conversation_tests.rs b/sources/src/readers/conversation_tests.rs new file mode 100644 index 0000000..36e15c1 --- /dev/null +++ b/sources/src/readers/conversation_tests.rs @@ -0,0 +1,218 @@ +//! Tests for the conversation reader. + +use super::*; + +use std::fs; +use tempfile::tempdir; + +fn conversation_source() -> MemorySourceEntry { + MemorySourceEntry { + id: "src_conv".into(), + kind: SourceKind::Conversation, + label: "Conversations".into(), + enabled: true, + toolkit: None, + connection_id: None, + path: None, + glob: None, + url: None, + branch: None, + paths: Vec::new(), + max_commits: None, + max_issues: None, + max_prs: None, + query: None, + since_days: None, + max_items: None, + selector: None, + max_tokens_per_sync: None, + max_cost_per_sync_usd: None, + sync_depth_days: None, + } +} + +#[test] +fn format_thread_produces_markdown() { + let thread = serde_json::json!({ + "title": "Test chat", + "messages": [ + {"role": "user", "content": "Hello"}, + {"role": "assistant", "content": "Hi there!"}, + ] + }); + let md = format_thread_as_markdown(&thread); + assert!(md.contains("# Test chat")); + assert!(md.contains("**user**: Hello")); + assert!(md.contains("**assistant**: Hi there!")); +} + +#[test] +fn format_thread_skips_empty_content() { + let thread = serde_json::json!({ + "title": "Sparse", + "messages": [ + {"role": "user", "content": ""}, + {"role": "assistant", "content": "Reply"}, + {"role": "user", "content": ""}, + ] + }); + let md = format_thread_as_markdown(&thread); + assert!(!md.contains("**user**:")); + assert!(md.contains("**assistant**: Reply")); +} + +#[test] +fn format_thread_handles_missing_title() { + let thread = serde_json::json!({ + "messages": [{"role": "user", "content": "Hi"}] + }); + let md = format_thread_as_markdown(&thread); + assert!(!md.starts_with('#')); + assert!(md.contains("**user**: Hi")); +} + +#[test] +fn format_thread_handles_no_messages() { + let thread = serde_json::json!({"title": "Empty"}); + let md = format_thread_as_markdown(&thread); + assert!(md.contains("# Empty")); + assert_eq!(md.trim(), "# Empty"); +} + +#[tokio::test] +async fn list_items_returns_empty_when_no_threads_dir() { + let tmp = tempdir().unwrap(); + let config = tmp.path(); + + let source = conversation_source(); + let reader = ConversationReader; + let items = reader.list_items(&source, config).await.unwrap(); + assert!(items.is_empty()); +} + +#[tokio::test] +async fn list_items_finds_json_thread_files() { + let tmp = tempdir().unwrap(); + let threads_dir = tmp.path().join("threads"); + fs::create_dir_all(&threads_dir).unwrap(); + + fs::write( + threads_dir.join("thread_abc.json"), + r#"{"title":"Chat 1","messages":[]}"#, + ) + .unwrap(); + fs::write( + threads_dir.join("thread_def.json"), + r#"{"title":"Chat 2","messages":[]}"#, + ) + .unwrap(); + // Non-json file should be ignored. + fs::write(threads_dir.join("notes.txt"), "ignored").unwrap(); + + let config = tmp.path(); + let source = conversation_source(); + let reader = ConversationReader; + let items = reader.list_items(&source, config).await.unwrap(); + assert_eq!(items.len(), 2); + + let ids: Vec<&str> = items.iter().map(|i| i.id.as_str()).collect(); + assert!(ids.contains(&"thread_abc")); + assert!(ids.contains(&"thread_def")); +} + +#[tokio::test] +async fn read_item_returns_formatted_content() { + let tmp = tempdir().unwrap(); + let threads_dir = tmp.path().join("threads"); + fs::create_dir_all(&threads_dir).unwrap(); + + let thread_json = serde_json::json!({ + "title": "Test Conversation", + "messages": [ + {"role": "user", "content": "What is 2+2?"}, + {"role": "assistant", "content": "4"}, + ] + }); + fs::write( + threads_dir.join("conv_123.json"), + serde_json::to_string(&thread_json).unwrap(), + ) + .unwrap(); + + let config = tmp.path(); + let source = conversation_source(); + let reader = ConversationReader; + let content = reader + .read_item(&source, "conv_123", config) + .await + .unwrap(); + + assert_eq!(content.id, "conv_123"); + assert_eq!(content.title, "Test Conversation"); + assert_eq!(content.content_type, ContentType::Markdown); + assert!(content.body.contains("**user**: What is 2+2?")); + assert!(content.body.contains("**assistant**: 4")); +} + +#[tokio::test] +async fn read_item_accepts_legitimate_double_dot_in_stem() { + let tmp = tempdir().unwrap(); + let threads_dir = tmp.path().join("threads"); + fs::create_dir_all(&threads_dir).unwrap(); + fs::write( + threads_dir.join("standup..2026.json"), + r#"{"title":"Standup","messages":[]}"#, + ) + .unwrap(); + let reader = ConversationReader; + let content = reader + .read_item( + &conversation_source(), + "standup..2026", + tmp.path(), + ) + .await + .unwrap(); + assert_eq!(content.id, "standup..2026"); +} + +#[tokio::test] +async fn read_item_returns_error_for_missing_thread() { + let tmp = tempdir().unwrap(); + let threads_dir = tmp.path().join("threads"); + fs::create_dir_all(&threads_dir).unwrap(); + + let config = tmp.path(); + let source = conversation_source(); + let reader = ConversationReader; + let result = reader.read_item(&source, "nonexistent", config).await; + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("not found")); +} + +#[tokio::test] +async fn read_item_rejects_path_traversal() { + let tmp = tempdir().unwrap(); + let threads_dir = tmp.path().join("threads"); + fs::create_dir_all(&threads_dir).unwrap(); + + let config = tmp.path(); + let source = conversation_source(); + let reader = ConversationReader; + + let result = reader.read_item(&source, "../config", config).await; + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("path traversal denied")); + + let result = reader + .read_item(&source, "foo/../../etc/passwd", config) + .await; + assert!(result.is_err()); + assert!(result + .unwrap_err() + .to_string() + .contains("path traversal denied")); +} diff --git a/sources/src/readers/folder.rs b/sources/src/readers/folder.rs new file mode 100644 index 0000000..e1fbfbf --- /dev/null +++ b/sources/src/readers/folder.rs @@ -0,0 +1,233 @@ +//! Local folder source reader. +//! +//! Walks files under a local directory, matching an optional glob (default +//! `**/*.md`), and reads their content as markdown, HTML, or plaintext. +//! +//! Safety: file sizes are capped at +//! [`FOLDER_FILE_SIZE_CAP_BYTES`] +//! (10 MB) on both list and read, and `read_item` is guarded against path +//! traversal via [`ensure_within_base`]. +//! +//! The directory walk uses `walkdir`; glob patterns are compiled to a `regex` +//! (matched against the slash-normalised path relative to the folder root). + +use async_trait::async_trait; +use std::path::{Path, PathBuf}; + +use regex::Regex; +use walkdir::WalkDir; + +use crate::FOLDER_FILE_SIZE_CAP_BYTES; +use tinymemory_api::error::MemoryError; + +use crate::SourceResult; +use crate::types::{ + ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, +}; +use crate::validation::ensure_within_base; + +use super::SourceReader; + +/// Default glob applied when a folder source does not specify one. +const DEFAULT_GLOB: &str = "**/*.md"; + +/// A reader over a local folder of files. +pub struct FolderReader; + +#[async_trait] +impl SourceReader for FolderReader { + fn kind(&self) -> SourceKind { + SourceKind::Folder + } + + async fn list_items( + &self, + source: &MemorySourceEntry, + _workspace: &std::path::Path, + ) -> SourceResult> { + let base_path = source + .path + .as_deref() + .ok_or_else(|| MemoryError::Invalid("folder source requires a path".to_string()))?; + let pattern = source.glob.as_deref().unwrap_or(DEFAULT_GLOB); + + let base = PathBuf::from(base_path); + if !base.exists() { + return Err(MemoryError::NotFound(format!( + "folder does not exist: {base_path}" + ))); + } + + let matcher = glob_to_regex(pattern)?; + + let mut items = Vec::new(); + for entry in WalkDir::new(&base).follow_links(false) { + let entry = match entry { + Ok(e) => e, + Err(_) => continue, + }; + if !entry.file_type().is_file() { + continue; + } + let path = entry.path(); + let rel = match path.strip_prefix(&base) { + Ok(r) => r, + Err(_) => continue, + }; + let rel_str = normalize_rel(rel); + if !matcher.is_match(&rel_str) { + continue; + } + let metadata = match std::fs::metadata(path) { + Ok(m) => m, + Err(_) => continue, + }; + if metadata.len() > FOLDER_FILE_SIZE_CAP_BYTES { + continue; + } + let modified_ms = metadata + .modified() + .ok() + .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) + .map(|d| d.as_millis() as i64); + + items.push(SourceItem { + id: rel_str.clone(), + title: rel_str, + updated_at_ms: modified_ms, + }); + } + + Ok(items) + } + + /// Read one file's content by its `item_id` (the slash-normalised path + /// relative to the source's `path`, as produced by + /// [`list_items`](Self::list_items)). + /// + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + _workspace: &std::path::Path, + ) -> SourceResult { + let base_path = source + .path + .as_deref() + .ok_or_else(|| MemoryError::Invalid("folder source requires a path".to_string()))?; + + let pattern = source.glob.as_deref().unwrap_or(DEFAULT_GLOB); + let matcher = glob_to_regex(pattern)?; + let normalized_id = normalize_rel(Path::new(item_id)); + if !matcher.is_match(&normalized_id) { + return Err(MemoryError::Invalid(format!( + "item '{item_id}' is outside source glob '{pattern}'" + ))); + } + + let file_path = Path::new(base_path).join(item_id); + if !file_path.exists() { + return Err(MemoryError::NotFound(format!( + "file not found: {}", + file_path.display() + ))); + } + + // Canonicalize and verify the resolved file stays within the folder + // root — defends against `..` traversal and symlink escapes. + let canonical_file = ensure_within_base(Path::new(base_path), &file_path)?; + + // Apply the same size cap as list_items so a huge file can't blow up + // the renderer or the chunker. + let metadata = std::fs::metadata(&canonical_file)?; + if metadata.len() > FOLDER_FILE_SIZE_CAP_BYTES { + return Err(MemoryError::Invalid(format!( + "file exceeds {FOLDER_FILE_SIZE_CAP_BYTES}-byte limit: {}", + canonical_file.display() + ))); + } + + let body = std::fs::read_to_string(&canonical_file)?; + + let content_type = if item_id.ends_with(".md") { + ContentType::Markdown + } else if item_id.ends_with(".html") || item_id.ends_with(".htm") { + ContentType::Html + } else { + ContentType::Plaintext + }; + + Ok(SourceContent { + id: item_id.to_string(), + title: item_id.to_string(), + body, + content_type, + metadata: serde_json::json!({}), + }) + } +} + +/// Normalise a relative path to forward slashes for glob matching. +fn normalize_rel(rel: &Path) -> String { + rel.components() + .map(|c| c.as_os_str().to_string_lossy()) + .collect::>() + .join("/") +} + +/// Compile a shell-style glob into an anchored [`Regex`] matched against a +/// slash-normalised relative path. +/// +/// Supported syntax: `*` (any run of non-separator chars), `?` (one +/// non-separator char), `**` (any run including separators), and `**/` (zero or +/// more leading directories). All other regex metacharacters are escaped. +fn glob_to_regex(pattern: &str) -> SourceResult { + let chars: Vec = pattern.chars().collect(); + let mut re = String::from("^"); + let mut i = 0; + while i < chars.len() { + let c = chars[i]; + match c { + '*' => { + if i + 1 < chars.len() && chars[i + 1] == '*' { + if i + 2 < chars.len() && chars[i + 2] == '/' { + // `**/` — zero or more leading directories. + re.push_str("(?:.*/)?"); + i += 3; + } else { + // `**` — any run including separators. + re.push_str(".*"); + i += 2; + } + } else { + // `*` — any run excluding separators. + re.push_str("[^/]*"); + i += 1; + } + } + '?' => { + re.push_str("[^/]"); + i += 1; + } + '/' => { + re.push('/'); + i += 1; + } + '.' | '+' | '(' | ')' | '|' | '^' | '$' | '{' | '}' | '[' | ']' | '\\' => { + re.push('\\'); + re.push(c); + i += 1; + } + other => { + re.push(other); + i += 1; + } + } + } + re.push('$'); + Regex::new(&re).map_err(|e| MemoryError::Invalid(format!("invalid glob pattern: {e}"))) +} + +#[cfg(test)] +#[path = "folder_tests.rs"] +mod tests; diff --git a/sources/src/readers/folder_tests.rs b/sources/src/readers/folder_tests.rs new file mode 100644 index 0000000..7ebda63 --- /dev/null +++ b/sources/src/readers/folder_tests.rs @@ -0,0 +1,151 @@ +//! Tests for the local folder reader. + +use super::*; + +use std::fs; +use tempfile::TempDir; + +fn folder_source(path: &str) -> MemorySourceEntry { + MemorySourceEntry { + id: "src_folder".into(), + kind: SourceKind::Folder, + label: "Test folder".into(), + enabled: true, + toolkit: None, + connection_id: None, + path: Some(path.into()), + glob: None, + url: None, + branch: None, + paths: Vec::new(), + max_commits: None, + max_issues: None, + max_prs: None, + query: None, + since_days: None, + max_items: None, + selector: None, + max_tokens_per_sync: None, + max_cost_per_sync_usd: None, + sync_depth_days: None, + } +} + +fn config() -> &'static std::path::Path { + std::path::Path::new("/unused") +} + +#[test] +fn glob_to_regex_matches_default_pattern() { + let re = glob_to_regex("**/*.md").unwrap(); + assert!(re.is_match("note.md")); + assert!(re.is_match("sub/dir/note.md")); + assert!(!re.is_match("note.txt")); +} + +#[test] +fn glob_to_regex_single_star_excludes_separators() { + let re = glob_to_regex("*.md").unwrap(); + assert!(re.is_match("note.md")); + assert!(!re.is_match("sub/note.md")); +} + +#[tokio::test] +async fn list_items_finds_md_files() { + let tmp = TempDir::new().unwrap(); + fs::write(tmp.path().join("note.md"), "# Hello").unwrap(); + fs::write(tmp.path().join("data.txt"), "ignored").unwrap(); + + let source = folder_source(&tmp.path().to_string_lossy()); + let reader = FolderReader; + let items = reader.list_items(&source, config()).await.unwrap(); + + assert_eq!(items.len(), 1); + assert_eq!(items[0].id, "note.md"); +} + +#[tokio::test] +async fn list_items_recurses_into_subdirectories() { + let tmp = TempDir::new().unwrap(); + fs::create_dir_all(tmp.path().join("sub")).unwrap(); + fs::write(tmp.path().join("top.md"), "a").unwrap(); + fs::write(tmp.path().join("sub/nested.md"), "b").unwrap(); + + let source = folder_source(&tmp.path().to_string_lossy()); + let reader = FolderReader; + let items = reader.list_items(&source, config()).await.unwrap(); + + let ids: Vec<&str> = items.iter().map(|i| i.id.as_str()).collect(); + assert_eq!(items.len(), 2); + assert!(ids.contains(&"top.md")); + assert!(ids.contains(&"sub/nested.md")); +} + +#[tokio::test] +async fn read_item_returns_file_content() { + let tmp = TempDir::new().unwrap(); + fs::write(tmp.path().join("test.md"), "# Test\nBody").unwrap(); + + let source = folder_source(&tmp.path().to_string_lossy()); + let reader = FolderReader; + let content = reader + .read_item(&source, "test.md", config()) + .await + .unwrap(); + + assert_eq!(content.body, "# Test\nBody"); + assert_eq!(content.content_type, ContentType::Markdown); +} + +#[tokio::test] +async fn read_item_enforces_configured_glob() { + let tmp = TempDir::new().unwrap(); + fs::create_dir_all(tmp.path().join("docs")).unwrap(); + fs::write(tmp.path().join("docs/allowed.md"), "allowed").unwrap(); + fs::write(tmp.path().join("docs/secret.env"), "secret").unwrap(); + let mut source = folder_source(&tmp.path().to_string_lossy()); + source.glob = Some("docs/**/*.md".into()); + let reader = FolderReader; + + assert!(reader + .read_item(&source, "docs/allowed.md", config()) + .await + .is_ok()); + let err = reader + .read_item(&source, "docs/secret.env", config()) + .await + .unwrap_err(); + assert!(err.to_string().contains("outside source glob")); +} + +#[tokio::test] +async fn read_item_prevents_path_traversal() { + let tmp = TempDir::new().unwrap(); + fs::write(tmp.path().join("safe.md"), "ok").unwrap(); + + let source = folder_source(&tmp.path().to_string_lossy()); + let reader = FolderReader; + let result = reader + .read_item(&source, "../../../etc/passwd", config()) + .await; + + assert!(result.is_err()); +} + +#[tokio::test] +async fn list_items_nonexistent_folder_errors() { + let source = folder_source("/nonexistent/path/xyz"); + let reader = FolderReader; + let result = reader.list_items(&source, config()).await; + assert!(result.is_err()); +} + +#[tokio::test] +async fn read_item_missing_file_errors() { + let tmp = TempDir::new().unwrap(); + let source = folder_source(&tmp.path().to_string_lossy()); + let reader = FolderReader; + let result = reader.read_item(&source, "missing.md", config()).await; + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("not found")); +} diff --git a/sources/src/readers/github.rs b/sources/src/readers/github.rs new file mode 100644 index 0000000..9bf2a52 --- /dev/null +++ b/sources/src/readers/github.rs @@ -0,0 +1,346 @@ +//! GitHub repo source reader. +//! +//! Pulls **project activity** (commits, issues, PRs) from a GitHub +//! repository — not source code. Uses the `gh` CLI when available for +//! authenticated, higher-rate-limit access; falls back to the public +//! GitHub REST API for unauthenticated reads. +//! +//! ## Module layout +//! +//! - [`self`] — [`GithubReader`] orchestration: item listing/reading, URL +//! parsing, raw-archive coordinates, shared utilities, and the cached +//! `gh`-availability probe. +//! - `types` — API response models and the `gh`-fallback list cache. +//! - `git` — local bare-clone + `git log` / `git show` helpers. +//! - `api` — `gh api` / REST transport plus commit list/read helpers. +//! - `issues` — issue and pull-request list/read helpers. + +mod api; +mod git; +mod issues; +mod types; + +#[cfg(test)] +#[path = "github_tests.rs"] +mod tests; + +use std::time::Duration; + +use async_trait::async_trait; + + +use crate::SourceResult; +use crate::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; +use crate::raw_kind::RawKind; + +use super::{into_engine_error, SourceReader}; + +// Re-export for the sibling submodules and the test module. +pub(crate) use types::{ItemKind, LIST_CACHE}; + +/// Default number of items of **each** type (commits, issues, PRs) to pull +/// when the source entry doesn't override it. Tunable per-source via +/// `max_commits` / `max_issues` / `max_prs` on [`MemorySourceEntry`]. +pub(crate) const DEFAULT_GITHUB_ITEM_LIMIT: u32 = 1000; + +/// Timeout for a single `gh` CLI invocation (including the availability +/// probe). +const GH_CLI_TIMEOUT: Duration = Duration::from_secs(30); + +/// Whether the `gh` CLI is on PATH and runs. Probed once per process and +/// cached: `gh api` is the preferred transport for authenticated, +/// higher-rate-limit access, and re-probing on every item read is wasteful. +static GH_AVAILABLE: tokio::sync::OnceCell = tokio::sync::OnceCell::const_new(); + +/// Probe `gh --version` (async, so a stuck `gh` cannot block a worker +/// thread) and cache the result for the process lifetime. +async fn gh_available() -> bool { + *GH_AVAILABLE + .get_or_init(|| async { + let status = tokio::time::timeout( + GH_CLI_TIMEOUT, + tokio::process::Command::new("gh") + .arg("--version") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status(), + ) + .await; + status + .map(|s| s.map(|st| st.success()).unwrap_or(false)) + .unwrap_or(false) + }) + .await +} + +/// Reader for a GitHub repository source: lists and fetches commits, issues +/// and pull requests via the REST API, and file content via a shallow clone. +pub struct GithubReader; + +/// Parse `owner` and `repo` from a GitHub URL. +/// +/// Accepts only the canonical `https://github.com//[.git][/]` +/// shape — extra segments like `/tree/main` or `/blob/...` are rejected +/// so callers can't accidentally derive the wrong owner/repo from a +/// deep link. +pub(crate) fn parse_github_url(url: &str) -> Result<(String, String), String> { + let trimmed = url.trim(); + let rest = trimmed + .strip_prefix("https://github.com/") + .or_else(|| trimmed.strip_prefix("http://github.com/")) + .or_else(|| trimmed.strip_prefix("git@github.com:")) + .ok_or_else(|| format!("not a GitHub URL: {url}"))?; + let cleaned = rest.trim_end_matches('/').trim_end_matches(".git"); + let parts: Vec<&str> = cleaned.split('/').collect(); + if parts.len() != 2 || parts[0].is_empty() || parts[1].is_empty() { + return Err(format!( + "expected https://github.com//, got: {url}" + )); + } + Ok((parts[0].to_string(), parts[1].to_string())) +} + +// ── Raw-archive coordinates ───────────────────────────────────────── + +/// Slugifiable raw-archive source id for a repo URL. +/// +/// Returns `github.com//`, which slugifies (via +/// `slugify_source_id`) to `github-com--` so a source's +/// commits/issues/PRs land under +/// `raw/github-com--/{commits,issues,prs}/`. +pub fn repo_archive_source_id(url: &str) -> Option { + let (owner, repo) = parse_github_url(url).ok()?; + Some(format!("github.com/{owner}/{repo}")) +} + +/// Chunk-store source id for a single repo item (dedup key). +/// +/// `github:/:` keeps per-item uniqueness for the +/// `mem_tree_ingested_sources` dedup table while the separate +/// [`repo_chunk_scope`] drives a shared directory. +pub fn chunk_source_id(url: &str, item_id: &str) -> Option { + let (owner, repo) = parse_github_url(url).ok()?; + Some(format!("github:{owner}/{repo}:{item_id}")) +} + +/// Repo-scoped chunk path scope so all items from one repo share a +/// single directory in the content store (e.g. `document/github-org-repo/`). +pub fn repo_chunk_scope(url: &str) -> Option { + let (owner, repo) = parse_github_url(url).ok()?; + Some(format!("github:{owner}/{repo}")) +} + +/// Map a [`SourceItem`] id (`commit:`, `issue:`, `pr:`) to its +/// raw-archive [`RawKind`] and the clean uid used as the filename suffix. +pub fn raw_archive_coords(item_id: &str) -> Option<(RawKind, String)> { + let (kind, rest) = ItemKind::from_id(item_id)?; + let raw_kind = match kind { + ItemKind::Commit => RawKind::Commit, + ItemKind::Issue => RawKind::Issue, + ItemKind::PullRequest => RawKind::PullRequest, + }; + Some((raw_kind, rest.to_string())) +} + +// ── Reader implementation ─────────────────────────────────────────── + +#[async_trait] +impl SourceReader for GithubReader { + fn kind(&self) -> SourceKind { + SourceKind::GithubRepo + } + + async fn list_items( + &self, + source: &MemorySourceEntry, + workspace: &std::path::Path, + ) -> SourceResult> { + self.list_items_inner(source, workspace) + .await + .map_err(into_engine_error) + } + + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + workspace: &std::path::Path, + ) -> SourceResult { + self.read_item_inner(source, item_id, workspace) + .await + .map_err(into_engine_error) + } +} + +impl GithubReader { + async fn list_items_inner( + &self, + source: &MemorySourceEntry, + workspace: &std::path::Path, + ) -> Result, String> { + let url = source + .url + .as_deref() + .ok_or("github source requires a url")?; + let (owner, repo) = parse_github_url(url)?; + let use_gh = gh_available().await; + + let max_commits = source.max_commits.unwrap_or(DEFAULT_GITHUB_ITEM_LIMIT); + let max_issues = source.max_issues.unwrap_or(DEFAULT_GITHUB_ITEM_LIMIT); + let max_prs = source.max_prs.unwrap_or(DEFAULT_GITHUB_ITEM_LIMIT); + // A configured branch narrows commits to that ref; configured paths + // narrow them to the touched files. Both fall through to the API + // fallback so the two transports agree on scope. + let branch = source.branch.as_deref(); + let paths = source.paths.as_slice(); + + let cache_dir = git::git_cache_dir(workspace, &owner, &repo); + + tracing::debug!( + owner = %owner, + repo = %repo, + use_gh = use_gh, + branch = %branch.unwrap_or("(all)"), + max_commits, + max_issues, + max_prs, + cache = %cache_dir.display(), + "[memory_sources:github] listing items" + ); + + // Clear the list cache so stale data from a prior sync doesn't + // leak into this run. + if let Ok(mut cache) = LIST_CACHE.lock() { + cache.clear(); + } + + let mut items = Vec::new(); + let mut errors = Vec::new(); + + // Commits via local git (clone/fetch bare repo, then git log) + match git::list_commits_git(&owner, &repo, max_commits, &cache_dir, branch, paths).await { + Ok(commits) => items.extend(commits), + Err(e) => { + tracing::warn!(error = %e, "[memory_sources:github] git commit list failed, falling back to API"); + match api::list_commits_api(&owner, &repo, max_commits, use_gh, branch, paths).await + { + Ok(commits) => items.extend(commits), + Err(e2) => { + tracing::warn!(error = %e2, "[memory_sources:github] API commit list also failed"); + errors.push(e2); + } + } + } + } + + // Issues and PRs via gh CLI / API (no local equivalent) + match issues::list_issues(&owner, &repo, max_issues, use_gh).await { + Ok(issues) => items.extend(issues), + Err(e) => { + tracing::warn!(error = %e, "[memory_sources:github] failed to list issues"); + errors.push(e); + } + } + + match issues::list_prs(&owner, &repo, max_prs, use_gh).await { + Ok(prs) => items.extend(prs), + Err(e) => { + tracing::warn!(error = %e, "[memory_sources:github] failed to list PRs"); + errors.push(e); + } + } + + if items.is_empty() && !errors.is_empty() { + return Err(format!( + "all GitHub API calls failed: {}", + errors.join("; ") + )); + } + + tracing::debug!(count = items.len(), "[memory_sources:github] found items"); + Ok(items) + } + + async fn read_item_inner( + &self, + source: &MemorySourceEntry, + item_id: &str, + workspace: &std::path::Path, + ) -> Result { + let url = source + .url + .as_deref() + .ok_or("github source requires a url")?; + let (owner, repo) = parse_github_url(url)?; + let use_gh = gh_available().await; + + let (kind, ref_id) = + ItemKind::from_id(item_id).ok_or_else(|| format!("invalid item id: {item_id}"))?; + + tracing::debug!( + item_id = %item_id, + kind = ?kind, + "[memory_sources:github] reading item" + ); + + match kind { + ItemKind::Commit => { + let cache_dir = git::git_cache_dir(workspace, &owner, &repo); + match git::read_commit_git(&owner, &repo, ref_id, &cache_dir).await { + Ok(content) => Ok(content), + Err(e) => { + tracing::debug!( + sha = %ref_id, + error = %e, + "[memory_sources:github] git read_commit failed, falling back to API" + ); + api::read_commit_api(&owner, &repo, ref_id, use_gh).await + } + } + } + ItemKind::Issue => { + let num: u64 = ref_id + .parse() + .map_err(|_| format!("invalid issue number: {ref_id}"))?; + issues::read_issue(&owner, &repo, num, use_gh).await + } + ItemKind::PullRequest => { + let num: u64 = ref_id + .parse() + .map_err(|_| format!("invalid PR number: {ref_id}"))?; + issues::read_pr(&owner, &repo, num, use_gh).await + } + } + } +} + +// ── Utilities ─────────────────────────────────────────────────────── + +fn parse_iso_ts(s: &str) -> Option { + chrono::DateTime::parse_from_rfc3339(s) + .ok() + .map(|dt| dt.timestamp_millis()) +} + +/// Render GitHub logins as a deduped, order-preserving, space-separated +/// list of `@handle`s. Empty / `unknown` logins are skipped; an empty +/// result renders as `none`. Used so unique committers/commenters surface +/// as `handle:` entities in the memory tree. +fn unique_handles<'a>(logins: impl Iterator) -> String { + let mut seen = std::collections::HashSet::new(); + let mut out: Vec = Vec::new(); + for login in logins { + let l = login.trim(); + if l.is_empty() || l == "unknown" { + continue; + } + if seen.insert(l.to_string()) { + out.push(format!("@{l}")); + } + } + if out.is_empty() { + "none".to_string() + } else { + out.join(" ") + } +} diff --git a/sources/src/readers/github/api.rs b/sources/src/readers/github/api.rs new file mode 100644 index 0000000..112063b --- /dev/null +++ b/sources/src/readers/github/api.rs @@ -0,0 +1,360 @@ +//! `gh` CLI + REST API helpers for the GitHub reader. +//! +//! [`fetch_github`] prefers the authenticated `gh api` path and falls back to +//! the unauthenticated REST API. Commit list/read helpers live here; issue and +//! pull-request list/read helpers live in the sibling `super::issues` module, +//! and commit reads additionally have a local `git` path in the sibling +//! `super::git` module. +//! +//! Branch/path filters are honored on the commits list: `sha=` and +//! `path=` query params narrow what the API returns to the configured +//! scope. + +use std::collections::HashSet; + +use crate::types::{ContentType, SourceContent, SourceItem}; + +use super::types::GhCommit; +use super::{parse_iso_ts, GH_CLI_TIMEOUT}; + +/// GitHub REST API maximum page size (`per_page`). +pub(super) const GH_PAGE_SIZE: u32 = 100; + +/// Hard ceiling on pagination loops so a misbehaving API (always returning a +/// full page) can never spin forever even if `max` is enormous. +pub(super) const GH_MAX_PAGES: u32 = 1000; + +/// Run `gh ` and return stdout as UTF-8. +pub(super) async fn gh_json(args: &[&str]) -> Result { + let output = tokio::time::timeout( + GH_CLI_TIMEOUT, + tokio::process::Command::new("gh").args(args).output(), + ) + .await + .map_err(|_| format!("gh command timed out after {}s", GH_CLI_TIMEOUT.as_secs()))? + .map_err(|e| format!("gh command failed: {e}"))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!("gh exited {}: {stderr}", output.status)); + } + + String::from_utf8(output.stdout).map_err(|e| format!("gh output not utf8: {e}")) +} + +/// Unauthenticated GET against the GitHub REST API. +pub(super) async fn api_get(path: &str) -> Result { + let url = format!("https://api.github.com{path}"); + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(20)) + .build() + .map_err(|e| format!("failed to build GitHub client: {e}"))?; + let resp = client + .get(&url) + .header("User-Agent", "openhuman") + .header("Accept", "application/vnd.github.v3+json") + .send() + .await + .map_err(|e| format!("GitHub API request failed: {e}"))?; + + if !resp.status().is_success() { + let status = resp.status(); + let body = resp.text().await.unwrap_or_default(); + return Err(format!("GitHub API returned {status}: {body}")); + } + + resp.text() + .await + .map_err(|e| format!("failed to read response: {e}")) +} + +/// Try `gh api` first, fall back to unauthenticated REST API. +pub(super) async fn fetch_github(api_path: &str, use_gh: bool) -> Result { + if use_gh { + match gh_json(&["api", api_path]).await { + Ok(s) => return Ok(s), + Err(e) => { + tracing::debug!( + error = %e, + path = %api_path, + "[memory_sources:github] gh failed, falling back to API" + ); + } + } + } + api_get(&format!("/{api_path}")).await +} + +/// Fetch up to `max` rows from a paginated GitHub list endpoint. +/// +/// Walks `?per_page=100&page=N` with a constant page size — GitHub's +/// offset-based pagination is per_page-relative, so shrinking the page size +/// mid-walk would re-window the offsets and silently skip rows (e.g. `max=150` +/// would fetch items 51-100 a second time instead of 101-150). Iteration stops +/// once `max` rows are collected or the API returns a short page (the last +/// page); `extra_query` is appended verbatim (e.g. `"state=all"`). The result +/// is truncated to exactly `max`. +pub(super) async fn fetch_all_pages( + owner: &str, + repo: &str, + resource: &str, + extra_query: &str, + max: u32, + use_gh: bool, +) -> Result, String> { + let fetch = |page: u32| async_fetch_page(page, owner, repo, resource, extra_query, use_gh); + collect_pages(resource, max, fetch).await +} + +/// Fetch one page's raw JSON at a constant [`GH_PAGE_SIZE`]. +async fn async_fetch_page( + page: u32, + owner: &str, + repo: &str, + resource: &str, + extra_query: &str, + use_gh: bool, +) -> Result { + let mut path = format!("repos/{owner}/{repo}/{resource}?per_page={GH_PAGE_SIZE}&page={page}"); + if !extra_query.is_empty() { + path.push('&'); + path.push_str(extra_query); + } + fetch_github(&path, use_gh).await +} + +/// Core pagination walk, split out from [`fetch_all_pages`] so the loop is +/// unit-testable with a fake fetch instead of a live GitHub API. +/// +/// `fetch` maps a 1-based page number to the raw JSON for that page. The page +/// size the fetch encodes must stay constant across pages — see +/// [`fetch_all_pages`] for why shrinking it mid-walk skips rows. +pub(super) async fn collect_pages( + label: &str, + max: u32, + mut fetch: F, +) -> Result, String> +where + T: serde::de::DeserializeOwned, + F: FnMut(u32) -> Fut, + Fut: std::future::Future>, +{ + let mut out: Vec = Vec::new(); + let mut page = 1u32; + + while (out.len() as u32) < max && page <= GH_MAX_PAGES { + let json_str = fetch(page).await?; + let batch: Vec = serde_json::from_str(&json_str) + .map_err(|e| format!("parse {label} page {page}: {e}"))?; + let got = batch.len(); + out.extend(batch); + + // Short page ⇒ no more rows upstream. + if got < GH_PAGE_SIZE as usize { + break; + } + page += 1; + } + + out.truncate(max as usize); + Ok(out) +} + +/// Percent-encode a branch or path value for use as a URL query parameter. +/// +/// RFC 3986 unreserved characters and `/` are kept as-is; everything else +/// (`&`, `=`, `#`, `?`, `%`, spaces, …) is percent-encoded so a value cannot +/// be misparsed as query syntax and corrupt the filter. `/` is left intact +/// because it is legal in a query component and GitHub's commits `sha`/`path` +/// filters expect the common `path=src/lib.rs` shape unencoded. +fn percent_encode_query(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for b in s.bytes() { + match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'.' | b'_' | b'~' | b'/' => { + out.push(b as char); + } + _ => out.push_str(&format!("%{b:02X}")), + } + } + out +} + +/// Build the `extra_query` strings for the commits endpoint — one per +/// configured path (the endpoint accepts a single `path` filter), each +/// carrying the branch's `sha` when set. An empty path list means "no path +/// filter" (a single query carrying only the branch filter, if any). +/// Branch/path values are percent-encoded so `&`, `#`, `=` inside them cannot +/// corrupt the query. Extracted as a pure helper so the filter wiring is +/// unit-testable. +pub(super) fn commit_list_queries(branch: Option<&str>, paths: &[String]) -> Vec { + let sha_q = branch + .filter(|b| !b.is_empty()) + .map(|b| format!("sha={}", percent_encode_query(b))); + let path_qs: Vec = if paths.is_empty() { + vec![String::new()] + } else { + paths + .iter() + .map(|p| format!("path={}", percent_encode_query(p))) + .collect() + }; + path_qs + .into_iter() + .map(|path_q| { + let mut extra = String::new(); + if let Some(q) = &sha_q { + extra.push_str(q); + } + if !path_q.is_empty() { + if !extra.is_empty() { + extra.push('&'); + } + extra.push_str(&path_q); + } + extra + }) + .collect() +} + +/// List commits via the REST `commits` endpoint (fallback when local git is +/// unavailable). +/// +/// A configured `branch` is sent as `sha=`. The GitHub commits +/// endpoint accepts a single `path` filter, so multiple configured paths are +/// fetched one query each (each bounded at `max` so the walk stays finite), +/// merged and deduped by sha, ordered by commit time, and truncated to `max`. +pub(super) async fn list_commits_api( + owner: &str, + repo: &str, + max: u32, + use_gh: bool, + branch: Option<&str>, + paths: &[String], +) -> Result, String> { + let mut batches: Vec> = Vec::new(); + for extra in commit_list_queries(branch, paths) { + let commits: Vec = + fetch_all_pages(owner, repo, "commits", &extra, max, use_gh).await?; + batches.push(commits); + } + Ok(merge_commit_batches(batches, max)) +} + +/// Merge per-path commit batches into the final item list. +/// +/// The GitHub commits endpoint accepts a single `path` filter, so multiple +/// configured paths are fetched one query each; every path must be walked +/// (not just until the first fills `max`) or later paths are silently starved. +/// Batches are deduped by sha, ordered newest-first by commit time, and +/// truncated to `max` — the same union semantics the local `git log` path gives +/// a multi-pathspec walk. Extracted as a pure helper so the merge is +/// unit-testable without a live API. +pub(super) fn merge_commit_batches(batches: Vec>, max: u32) -> Vec { + let mut out: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); + for commits in batches { + for c in commits { + if seen.insert(c.sha.clone()) { + let title = c.commit.message.lines().next().unwrap_or("").to_string(); + let ts = c + .commit + .committer + .as_ref() + .and_then(|a| a.date.as_deref()) + .and_then(parse_iso_ts); + out.push(SourceItem { + id: format!("commit:{}", c.sha), + title, + updated_at_ms: ts, + }); + } + } + } + // Each path's query returns its commits newest-first, but the merged set + // is path-ordered. Re-sort by commit time (newest first) so the global + // truncation keeps the most recent commits across all configured paths. + out.sort_by_key(|b| std::cmp::Reverse(b.updated_at_ms)); + out.truncate(max as usize); + out +} + +/// Read one commit via the REST API (fallback when local git is unavailable). +pub(super) async fn read_commit_api( + owner: &str, + repo: &str, + sha: &str, + use_gh: bool, +) -> Result { + let json_str = fetch_github(&format!("repos/{owner}/{repo}/commits/{sha}"), use_gh).await?; + + let commit: GhCommit = + serde_json::from_str(&json_str).map_err(|e| format!("parse commit: {e}"))?; + + let author = commit + .commit + .author + .as_ref() + .map(|a| { + format!( + "{} <{}>", + a.name.as_deref().unwrap_or("unknown"), + a.email.as_deref().unwrap_or("") + ) + }) + .unwrap_or_default(); + + // GitHub login of the committer, rendered as an `@handle` so the + // entity extractor registers it as a `handle:` entity in the memory + // tree (unique committers become first-class entities). + let handle = commit + .author + .as_ref() + .map(|u| format!("@{}", u.login)) + .unwrap_or_default(); + + let date = commit + .commit + .committer + .as_ref() + .and_then(|a| a.date.as_deref()) + .unwrap_or("unknown"); + + let title = commit + .commit + .message + .lines() + .next() + .unwrap_or("") + .to_string(); + + let author_line = if handle.is_empty() { + author.clone() + } else { + format!("{author} ({handle})") + }; + + let body = format!( + "# Commit: {title}\n\n\ + **SHA:** {sha}\n\ + **Author:** {author_line}\n\ + **Date:** {date}\n\n\ + ## Message\n\n\ + {}", + commit.commit.message, + ); + + Ok(SourceContent { + id: format!("commit:{sha}"), + title, + body, + content_type: ContentType::Markdown, + metadata: serde_json::json!({ + "owner": owner, + "repo": repo, + "sha": sha, + "author": author, + "author_handle": commit.author.as_ref().map(|u| u.login.clone()), + }), + }) +} diff --git a/sources/src/readers/github/git.rs b/sources/src/readers/github/git.rs new file mode 100644 index 0000000..0a330bc --- /dev/null +++ b/sources/src/readers/github/git.rs @@ -0,0 +1,320 @@ +//! Local bare-clone helpers for the GitHub reader. +//! +//! Commits are listed via a per-repo bare clone (`git log`) rather than the +//! REST API whenever the repo is reachable over git: the clone's refs are a +//! superset of what the API exposes and reads are fully offline after the +//! initial clone/fetch. The clone lives under +//! `workspace/git_cache//.git`. +//! +//! Branch/path filters are honored here: a configured `branch` narrows `git +//! log` to that ref (instead of the bare clone's `HEAD`), and configured +//! `paths` become git pathspecs so commits touching unrelated paths are not +//! ingested. + +use std::path::{Path, PathBuf}; +use std::time::Duration; + +use crate::types::{ContentType, SourceContent, SourceItem}; + +use super::parse_iso_ts; + +/// Timeout for a single `git clone` / `git fetch` (slow on a cold cache). +const GIT_CLONE_TIMEOUT: Duration = Duration::from_secs(120); +/// Timeout for a single `git log` / `git show` (fast, local). +const GIT_LOG_TIMEOUT: Duration = Duration::from_secs(30); + +/// Path to the bare clone for a repo, created lazily under +/// `workspace/git_cache//.git`. +pub(super) fn git_cache_dir(workspace: &Path, owner: &str, repo: &str) -> PathBuf { + workspace + .join("git_cache") + .join(owner) + .join(format!("{repo}.git")) +} + +/// Ensure a bare clone of `owner/repo` exists at `cache_dir` — fetching into +/// an existing clone, cloning fresh when absent. A missing remote (private or +/// renamed repo) surfaces as an error handled by the caller's fallback. +pub(super) async fn ensure_bare_clone( + owner: &str, + repo: &str, + cache_dir: &Path, +) -> Result<(), String> { + if cache_dir.join("HEAD").exists() { + return fetch_existing_bare(cache_dir).await; + } + + let clone_url = format!("https://github.com/{owner}/{repo}.git"); + clone_bare(&clone_url, cache_dir).await +} + +/// `git fetch` into an existing bare clone. +/// +/// The refspec is explicit (`+refs/heads/*:refs/heads/*`): a bare +/// `git clone` records no `remote.origin.fetch` mapping, so a bare `git fetch` +/// without one would only update `FETCH_HEAD` and leave `refs/heads/*` at the +/// initial clone — every later sync would silently miss new GitHub activity. +/// `--prune` also drops local heads the remote has since deleted. +/// +/// After the fetch, `HEAD` is refreshed to the remote's current default branch +/// so an unconfigured sync keeps following the repo's default even when that +/// default changes between clones (see [`refresh_default_branch_head`]). +async fn fetch_existing_bare(cache_dir: &Path) -> Result<(), String> { + tracing::debug!( + cache = %cache_dir.display(), + "[memory_sources:github:git] fetching into existing bare clone" + ); + let output = tokio::time::timeout( + GIT_CLONE_TIMEOUT, + tokio::process::Command::new("git") + .args([ + "fetch", + "--prune", + "--quiet", + "origin", + "+refs/heads/*:refs/heads/*", + ]) + .current_dir(cache_dir) + .output(), + ) + .await + .map_err(|_| "git fetch timed out".to_string())? + .map_err(|e| format!("git fetch failed: {e}"))?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!("git fetch exited {}: {stderr}", output.status)); + } + refresh_default_branch_head(cache_dir).await; + Ok(()) +} + +/// Repoint the bare clone's `HEAD` to the remote's current default branch. +/// +/// `git clone --bare` pins `HEAD` to the default branch selected at clone +/// time, and the fetch refspec above updates `refs/heads/*` but never `HEAD`. +/// If the remote later changes its default branch (while keeping the old +/// branch alive), an unconfigured `git log HEAD` would keep walking the old +/// branch forever, diverging from the REST fallback which follows the new +/// default. Reading the remote `HEAD` symref (`ref: refs/heads/`) and +/// writing it back keeps the clone's default in sync. +/// +/// Best-effort: `git ls-remote` can fail transiently (network), and there is +/// nothing to refresh on an unborn default branch; neither should fail the +/// fetch that already succeeded. +async fn refresh_default_branch_head(cache_dir: &Path) { + let Ok(output) = tokio::time::timeout( + GIT_CLONE_TIMEOUT, + tokio::process::Command::new("git") + .args(["ls-remote", "--symref", "origin", "HEAD"]) + .current_dir(cache_dir) + .output(), + ) + .await + else { + return; + }; + let Ok(output) = output else { return }; + if !output.status.success() { + return; + } + // `--symref` prints `ref: refs/heads/\tHEAD` on the first line. + let stdout = String::from_utf8_lossy(&output.stdout); + let Some(first) = stdout.lines().next() else { + return; + }; + let Some(remote_ref) = first + .strip_prefix("ref: ") + .and_then(|r| r.split_whitespace().next()) + else { + return; + }; + if !remote_ref.starts_with("refs/heads/") { + return; + } + let _ = tokio::time::timeout( + GIT_CLONE_TIMEOUT, + tokio::process::Command::new("git") + .args(["symbolic-ref", "HEAD", remote_ref]) + .current_dir(cache_dir) + .output(), + ) + .await; +} + +/// Fresh bare clone of `clone_url` into `cache_dir`. +async fn clone_bare(clone_url: &str, cache_dir: &Path) -> Result<(), String> { + if let Some(parent) = cache_dir.parent() { + std::fs::create_dir_all(parent).map_err(|e| format!("create cache dir: {e}"))?; + } + + tracing::info!( + url = %clone_url, + cache = %cache_dir.display(), + "[memory_sources:github:git] cloning bare repo" + ); + + let output = tokio::time::timeout( + GIT_CLONE_TIMEOUT, + tokio::process::Command::new("git") + .args(["clone", "--bare", "--quiet", clone_url]) + .arg(cache_dir) + .output(), + ) + .await + .map_err(|_| "git clone timed out".to_string())? + .map_err(|e| format!("git clone failed: {e}"))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!("git clone exited {}: {stderr}", output.status)); + } + + Ok(()) +} + +/// List commits in the bare clone, newest first, up to `max`. +/// +/// `branch` restricts the walk to a single ref (default `HEAD` — the bare +/// clone's default branch, matching the REST fallback's default-branch +/// scope), and `paths` narrows it to commits touching any of the given +/// pathspecs. +pub(super) async fn list_commits_git( + owner: &str, + repo: &str, + max: u32, + cache_dir: &Path, + branch: Option<&str>, + paths: &[String], +) -> Result, String> { + ensure_bare_clone(owner, repo, cache_dir).await?; + + let args = log_args(max, branch, paths); + + let output = tokio::time::timeout( + GIT_LOG_TIMEOUT, + tokio::process::Command::new("git") + .args(&args) + .current_dir(cache_dir) + .output(), + ) + .await + .map_err(|_| "git log timed out".to_string())? + .map_err(|e| format!("git log failed: {e}"))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!("git log exited {}: {stderr}", output.status)); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let items: Vec = stdout + .lines() + .filter(|line| !line.is_empty()) + .map(|line| { + let parts: Vec<&str> = line.splitn(3, '\t').collect(); + let sha = parts.first().unwrap_or(&""); + let subject = parts.get(1).unwrap_or(&""); + let date = parts.get(2).unwrap_or(&""); + SourceItem { + id: format!("commit:{sha}"), + title: subject.to_string(), + updated_at_ms: parse_iso_ts(date), + } + }) + .collect(); + + tracing::debug!( + count = items.len(), + "[memory_sources:github:git] listed commits via local git" + ); + Ok(items) +} + +/// Build the `git log` argument list for the commit walk. +/// +/// `branch` restricts the walk to a single ref (default `HEAD` — the bare +/// clone's default branch, matching the REST fallback's default-branch +/// scope), and `paths` narrows it to commits touching any of the given +/// pathspecs (trailing `-- path1 path2`). Extracted as a pure helper so the +/// filter wiring is unit-testable without a real clone. +pub(super) fn log_args(max: u32, branch: Option<&str>, paths: &[String]) -> Vec { + let mut args: Vec = vec!["log".to_string()]; + match branch { + Some(b) if !b.is_empty() => args.push(b.to_string()), + _ => args.push("HEAD".to_string()), + } + args.push(format!("--max-count={max}")); + args.push("--format=%H\t%s\t%aI".to_string()); + if !paths.is_empty() { + args.push("--".to_string()); + args.extend(paths.iter().cloned()); + } + args +} + +/// Read one commit's full message and metadata from the bare clone. +pub(super) async fn read_commit_git( + owner: &str, + repo: &str, + sha: &str, + cache_dir: &Path, +) -> Result { + if !cache_dir.join("HEAD").exists() { + return Err("bare clone not present".to_string()); + } + + // git show with a custom format for author, date, and full message. + let output = tokio::time::timeout( + GIT_LOG_TIMEOUT, + tokio::process::Command::new("git") + .args(["show", "--no-patch", "--format=%H%n%aN%n%aE%n%aI%n%B", sha]) + .current_dir(cache_dir) + .output(), + ) + .await + .map_err(|_| "git show timed out".to_string())? + .map_err(|e| format!("git show failed: {e}"))?; + + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + return Err(format!("git show exited {}: {stderr}", output.status)); + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let mut lines = stdout.lines(); + let full_sha = lines.next().unwrap_or(sha); + let author_name = lines.next().unwrap_or("unknown"); + let author_email = lines.next().unwrap_or(""); + let date = lines.next().unwrap_or("unknown"); + let message: String = lines.collect::>().join("\n"); + let message = message.trim(); + + let title = message.lines().next().unwrap_or("").to_string(); + let author = format!("{author_name} <{author_email}>"); + + let body = format!( + "# Commit: {title}\n\n\ + **SHA:** {full_sha}\n\ + **Author:** {author}\n\ + **Date:** {date}\n\n\ + ## Message\n\n\ + {message}", + ); + + Ok(SourceContent { + id: format!("commit:{sha}"), + title, + body, + content_type: ContentType::Markdown, + metadata: serde_json::json!({ + "owner": owner, + "repo": repo, + "sha": full_sha, + "author": author, + }), + }) +} + +#[cfg(test)] +#[path = "git_tests.rs"] +mod tests; diff --git a/sources/src/readers/github/git_tests.rs b/sources/src/readers/github/git_tests.rs new file mode 100644 index 0000000..9c1011a --- /dev/null +++ b/sources/src/readers/github/git_tests.rs @@ -0,0 +1,122 @@ +use super::*; + +use std::process::Command; + +/// Run `git` with the given args in `cwd`, asserting success and returning +/// stdout as a string. +fn git_ok(cwd: &Path, args: &[&str]) -> String { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("spawn git"); + assert!( + out.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).into_owned() +} + +/// Create a source repo with one commit at `dir`. +fn init_repo(dir: &Path) { + std::fs::create_dir_all(dir).expect("create repo dir"); + git_ok(dir, &["init", "-q"]); + git_ok(dir, &["config", "user.email", "test@example.com"]); + git_ok(dir, &["config", "user.name", "Test"]); + std::fs::write(dir.join("a.txt"), "one").expect("write file"); + git_ok(dir, &["add", "."]); + git_ok(dir, &["commit", "-qm", "first"]); +} + +#[tokio::test] +async fn fetch_existing_bare_refreshes_default_branch_head() { + // Regression: the clone's default branch can change upstream. The bare + // clone's HEAD is pinned at clone time, and the fetch refspec updates + // refs/heads/* but not HEAD, so an unconfigured `git log HEAD` would keep + // walking the old default while the REST fallback follows the new one. + // After fetching, HEAD must be repointed to the remote's current default. + let tmp = tempfile::tempdir().expect("tempdir"); + let src = tmp.path().join("src"); + std::fs::create_dir_all(&src).expect("create repo dir"); + git_ok(&src, &["init", "-q", "-b", "master"]); + git_ok(&src, &["config", "user.email", "test@example.com"]); + git_ok(&src, &["config", "user.name", "Test"]); + std::fs::write(src.join("a.txt"), "one").expect("write file"); + git_ok(&src, &["add", "."]); + git_ok(&src, &["commit", "-qm", "first"]); + + let cache = tmp.path().join("cache.git"); + git_ok( + tmp.path(), + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + cache.to_str().unwrap(), + ], + ); + let head_ref = git_ok(&cache, &["symbolic-ref", "HEAD"]); + assert_eq!( + head_ref.trim(), + "refs/heads/master", + "clone pins default HEAD" + ); + + // Upstream renames its default branch: create `main` and switch HEAD to it + // while keeping `master` alive (a repo that changes its default branch). + git_ok(&src, &["checkout", "-q", "-b", "main"]); + std::fs::write(src.join("b.txt"), "two").expect("write file"); + git_ok(&src, &["add", "."]); + git_ok(&src, &["commit", "-qm", "second"]); + git_ok(&src, &["symbolic-ref", "HEAD", "refs/heads/main"]); + + // A plain fetch (without the refresh) would leave HEAD on `master`. + fetch_existing_bare(&cache).await.expect("fetch succeeds"); + let refreshed = git_ok(&cache, &["symbolic-ref", "HEAD"]); + assert_eq!( + refreshed.trim(), + "refs/heads/main", + "fetch must repoint HEAD to the remote's new default branch" + ); +} + +#[tokio::test] +async fn fetch_existing_bare_advances_local_heads() { + // A bare clone records no remote.origin.fetch refspec, so a bare `git + // fetch` (no refspec) would only touch FETCH_HEAD. The explicit + // `+refs/heads/*:refs/heads/*` must advance refs/heads/* to the remote's + // new commits, otherwise every later sync silently misses them. + let tmp = tempfile::tempdir().expect("tempdir"); + let src = tmp.path().join("src"); + init_repo(&src); + + let cache = tmp.path().join("cache.git"); + git_ok( + tmp.path(), + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + cache.to_str().unwrap(), + ], + ); + let first_head = git_ok(&cache, &["rev-parse", "HEAD"]); + + // A second commit lands upstream. + std::fs::write(src.join("b.txt"), "two").expect("write file"); + git_ok(&src, &["add", "."]); + git_ok(&src, &["commit", "-qm", "second"]); + let upstream_head = git_ok(&src, &["rev-parse", "HEAD"]); + assert_ne!(first_head, upstream_head, "test setup: new commit expected"); + + // Fetch into the existing bare clone and confirm the local head advances. + fetch_existing_bare(&cache).await.expect("fetch succeeds"); + let cached_head = git_ok(&cache, &["rev-parse", "HEAD"]); + assert_eq!( + cached_head, upstream_head, + "fetch must advance refs/heads/* so git log --all sees new commits" + ); +} diff --git a/sources/src/readers/github/issues.rs b/sources/src/readers/github/issues.rs new file mode 100644 index 0000000..b71b50c --- /dev/null +++ b/sources/src/readers/github/issues.rs @@ -0,0 +1,300 @@ +//! Issue and pull-request list/read helpers for the GitHub reader. +//! +//! Both endpoints share the [`fetch_github`](super::api::fetch_github) +//! transport and the list-pass cache in `super::types::LIST_CACHE`: the issues +//! endpoint returns pull requests mixed in with issues, and the PR endpoint is +//! the only one that returns merge state, so the list pass stashes the full +//! row and the read pass reuses it instead of re-fetching. + +use serde::Deserialize; + +use crate::types::{ContentType, SourceContent, SourceItem}; + +use super::api::{fetch_all_pages, fetch_github, GH_MAX_PAGES, GH_PAGE_SIZE}; +use super::types::{CachedItem, GhIssue, GhPr, GhUser, IssueComment}; +use super::{parse_iso_ts, unique_handles}; + +/// List issues (excluding pull requests, which the issues endpoint also +/// returns) with the full row cached for later reads. +pub(super) async fn list_issues( + owner: &str, + repo: &str, + max: u32, + use_gh: bool, +) -> Result, String> { + let mut out: Vec = Vec::new(); + let mut page = 1u32; + + while (out.len() as u32) < max && page <= GH_MAX_PAGES { + let path = + format!("repos/{owner}/{repo}/issues?per_page={GH_PAGE_SIZE}&page={page}&state=all"); + let json_str = fetch_github(&path, use_gh).await?; + let batch: Vec = serde_json::from_str(&json_str) + .map_err(|e| format!("parse issues page {page}: {e}"))?; + let got = batch.len(); + + for i in batch { + if i.pull_request.is_some() { + continue; + } + let ts = i.updated_at.as_deref().and_then(parse_iso_ts); + let item_id = format!("issue:{}", i.number); + let cache_key = format!("{owner}/{repo}:{item_id}"); + out.push(SourceItem { + id: item_id, + title: format!("#{} {}", i.number, i.title), + updated_at_ms: ts, + }); + if let Ok(mut cache) = super::types::LIST_CACHE.lock() { + cache.insert(cache_key, CachedItem::Issue(i)); + } + if out.len() as u32 >= max { + break; + } + } + + if got < GH_PAGE_SIZE as usize { + break; + } + page += 1; + } + + Ok(out) +} + +/// List pull requests with the full row cached for later reads. +pub(super) async fn list_prs( + owner: &str, + repo: &str, + max: u32, + use_gh: bool, +) -> Result, String> { + let prs: Vec = fetch_all_pages(owner, repo, "pulls", "state=all", max, use_gh).await?; + + let items: Vec = prs + .into_iter() + .map(|p| { + let ts = p.updated_at.as_deref().and_then(parse_iso_ts); + let item_id = format!("pr:{}", p.number); + let cache_key = format!("{owner}/{repo}:{item_id}"); + let item = SourceItem { + id: item_id, + title: format!("PR #{} {}", p.number, p.title), + updated_at_ms: ts, + }; + if let Ok(mut cache) = super::types::LIST_CACHE.lock() { + cache.insert(cache_key, CachedItem::Pr(p)); + } + item + }) + .collect(); + + Ok(items) +} + +/// Read one issue, preferring the row cached by the list pass. +pub(super) async fn read_issue( + owner: &str, + repo: &str, + number: u64, + use_gh: bool, +) -> Result { + let cache_key = format!("{owner}/{repo}:issue:{number}"); + let from_cache = super::types::LIST_CACHE + .lock() + .ok() + .and_then(|mut c| c.remove(&cache_key)); + let issue: GhIssue = match from_cache { + Some(CachedItem::Issue(i)) => i, + _ => { + let json_str = + fetch_github(&format!("repos/{owner}/{repo}/issues/{number}"), use_gh).await?; + serde_json::from_str(&json_str).map_err(|e| format!("parse issue: {e}"))? + } + }; + + let author = issue + .user + .as_ref() + .map(|u| u.login.as_str()) + .unwrap_or("unknown"); + let labels: Vec<&str> = issue.labels.iter().map(|l| l.name.as_str()).collect(); + let issue_body = issue.body.as_deref().unwrap_or(""); + + let comments = fetch_issue_comments(owner, repo, number, use_gh).await; + let participants = + unique_handles(std::iter::once(author).chain(comments.iter().map(|c| c.user.as_str()))); + + let mut body = format!( + "# Issue #{number}: {title}\n\n\ + **State:** {state}\n\ + **Author:** @{author}\n\ + **Participants:** {participants}\n\ + **Labels:** {label_str}\n\ + **Created:** {created}\n\ + **Updated:** {updated}\n\n\ + ## Description\n\n\ + {issue_body}", + title = issue.title, + state = issue.state, + label_str = if labels.is_empty() { + "none".to_string() + } else { + labels.join(", ") + }, + created = issue.created_at.as_deref().unwrap_or("unknown"), + updated = issue.updated_at.as_deref().unwrap_or("unknown"), + ); + + if !comments.is_empty() { + body.push_str("\n\n## Comments\n"); + for comment in &comments { + body.push_str(&format!( + "\n### @{} ({})\n\n{}\n", + comment.user, comment.created_at, comment.body + )); + } + } + + Ok(SourceContent { + id: format!("issue:{number}"), + title: format!("#{number} {}", issue.title), + body, + content_type: ContentType::Markdown, + metadata: serde_json::json!({ + "owner": owner, + "repo": repo, + "number": number, + "state": issue.state, + "labels": labels, + }), + }) +} + +/// Read one pull request, preferring the row cached by the list pass. +pub(super) async fn read_pr( + owner: &str, + repo: &str, + number: u64, + use_gh: bool, +) -> Result { + let cache_key = format!("{owner}/{repo}:pr:{number}"); + let from_cache = super::types::LIST_CACHE + .lock() + .ok() + .and_then(|mut c| c.remove(&cache_key)); + let pr: GhPr = match from_cache { + Some(CachedItem::Pr(p)) => p, + _ => { + let json_str = + fetch_github(&format!("repos/{owner}/{repo}/pulls/{number}"), use_gh).await?; + serde_json::from_str(&json_str).map_err(|e| format!("parse PR: {e}"))? + } + }; + + let author = pr + .user + .as_ref() + .map(|u| u.login.as_str()) + .unwrap_or("unknown"); + let labels: Vec<&str> = pr.labels.iter().map(|l| l.name.as_str()).collect(); + let pr_body = pr.body.as_deref().unwrap_or(""); + + let merged_str = match pr.merged_at.as_deref() { + Some(ts) => format!("merged at {ts}"), + None => "not merged".to_string(), + }; + + let comments = fetch_issue_comments(owner, repo, number, use_gh).await; + let participants = + unique_handles(std::iter::once(author).chain(comments.iter().map(|c| c.user.as_str()))); + + let mut body = format!( + "# PR #{number}: {title}\n\n\ + **State:** {state} ({merged})\n\ + **Author:** @{author}\n\ + **Participants:** {participants}\n\ + **Labels:** {label_str}\n\ + **Created:** {created}\n\ + **Updated:** {updated}\n\n\ + ## Description\n\n\ + {pr_body}", + title = pr.title, + state = pr.state, + merged = merged_str, + label_str = if labels.is_empty() { + "none".to_string() + } else { + labels.join(", ") + }, + created = pr.created_at.as_deref().unwrap_or("unknown"), + updated = pr.updated_at.as_deref().unwrap_or("unknown"), + ); + + if !comments.is_empty() { + body.push_str("\n\n## Comments\n"); + for comment in &comments { + body.push_str(&format!( + "\n### @{} ({})\n\n{}\n", + comment.user, comment.created_at, comment.body + )); + } + } + + Ok(SourceContent { + id: format!("pr:{number}"), + title: format!("PR #{number} {}", pr.title), + body, + content_type: ContentType::Markdown, + metadata: serde_json::json!({ + "owner": owner, + "repo": repo, + "number": number, + "state": pr.state, + "merged": pr.merged_at.is_some(), + "labels": labels, + }), + }) +} + +/// Fetch up to 50 comments on an issue/PR. Best-effort: any failure (or +/// parse error) yields an empty list — comment text is enrichment, not the +/// item's substance, so a missing comments API must not fail the read. +async fn fetch_issue_comments( + owner: &str, + repo: &str, + number: u64, + use_gh: bool, +) -> Vec { + #[derive(Deserialize)] + struct RawComment { + user: Option, + body: Option, + created_at: Option, + } + + let json_str = fetch_github( + &format!("repos/{owner}/{repo}/issues/{number}/comments?per_page=50"), + use_gh, + ) + .await; + + let Ok(json_str) = json_str else { + return Vec::new(); + }; + + let comments: Vec = serde_json::from_str(&json_str).unwrap_or_default(); + + comments + .into_iter() + .map(|c| IssueComment { + user: c + .user + .as_ref() + .map(|u| u.login.clone()) + .unwrap_or_else(|| "unknown".into()), + body: c.body.unwrap_or_default(), + created_at: c.created_at.unwrap_or_else(|| "unknown".into()), + }) + .collect() +} diff --git a/sources/src/readers/github/types.rs b/sources/src/readers/github/types.rs new file mode 100644 index 0000000..11327e9 --- /dev/null +++ b/sources/src/readers/github/types.rs @@ -0,0 +1,122 @@ +//! API response models and the `gh`-fallback list cache for the GitHub +//! reader. Pure data — no I/O lives here. Models are deliberately kept loose +//! (only the fields the reader consumes are declared) so new GitHub response +//! fields don't force a struct change. + +use std::collections::HashMap; +use std::sync::{LazyLock, Mutex}; + +use serde::Deserialize; + +/// A commit object from the REST commits endpoint. +#[derive(Debug, Deserialize)] +pub(crate) struct GhCommit { + pub(crate) sha: String, + pub(crate) commit: GhCommitInner, + /// Top-level GitHub user that authored the commit (distinct from the + /// embedded git author identity). Present when the commit author maps + /// to a GitHub account; absent for unlinked email-only authors. + #[serde(default)] + pub(crate) author: Option, +} + +#[derive(Debug, Deserialize)] +pub(crate) struct GhCommitInner { + pub(crate) message: String, + pub(crate) author: Option, + pub(crate) committer: Option, +} + +#[derive(Debug, Deserialize)] +pub(crate) struct GhAuthor { + pub(crate) name: Option, + pub(crate) email: Option, + pub(crate) date: Option, +} + +/// An issue list entry (`GET /repos/{owner}/{repo}/issues`). +#[derive(Debug, Clone, Deserialize)] +pub(crate) struct GhIssue { + pub(crate) number: u64, + pub(crate) title: String, + pub(crate) body: Option, + pub(crate) state: String, + pub(crate) user: Option, + pub(crate) labels: Vec, + pub(crate) created_at: Option, + pub(crate) updated_at: Option, + /// Present when the row is actually a pull request (the issues endpoint + /// returns PRs with a `pull_request` envelope). + pub(crate) pull_request: Option, +} + +#[derive(Debug, Clone, Deserialize)] +pub(crate) struct GhUser { + pub(crate) login: String, +} + +#[derive(Debug, Clone, Deserialize)] +pub(crate) struct GhLabel { + pub(crate) name: String, +} + +/// A pull request list entry. +#[derive(Debug, Clone, Deserialize)] +pub(crate) struct GhPr { + pub(crate) number: u64, + pub(crate) title: String, + pub(crate) body: Option, + pub(crate) state: String, + pub(crate) user: Option, + pub(crate) labels: Vec, + pub(crate) created_at: Option, + pub(crate) updated_at: Option, + pub(crate) merged_at: Option, +} + +/// A comment on an issue or PR, slimmed to the fields the reader renders. +#[derive(Debug, Clone)] +pub(crate) struct IssueComment { + pub(crate) user: String, + pub(crate) body: String, + pub(crate) created_at: String, +} + +/// What kind of GitHub item a list row refers to. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ItemKind { + Commit, + Issue, + PullRequest, +} + +impl ItemKind { + /// Parse a `SourceItem` id (`commit:`, `issue:`, `pr:`) into + /// its kind and the ref (sha / number) that follows the prefix. + pub(crate) fn from_id(id: &str) -> Option<(Self, &str)> { + if let Some(rest) = id.strip_prefix("commit:") { + Some((ItemKind::Commit, rest)) + } else if let Some(rest) = id.strip_prefix("issue:") { + Some((ItemKind::Issue, rest)) + } else if let Some(rest) = id.strip_prefix("pr:") { + Some((ItemKind::PullRequest, rest)) + } else { + None + } + } +} + +/// A cached issue/PR row, keyed by its list id (`"/:"`). +/// The issues endpoint returns pull requests mixed in with issues, and the PR +/// endpoint is the only one that returns merge state, so the list pass stashes +/// the full row here and the read pass reuses it instead of re-fetching. +#[derive(Debug, Clone)] +pub(crate) enum CachedItem { + Issue(GhIssue), + Pr(GhPr), +} + +/// Process-wide cache of issue/PR list rows, cleared at the start of each +/// `list_items` run so stale data from a prior sync can't leak in. +pub(crate) static LIST_CACHE: LazyLock>> = + LazyLock::new(|| Mutex::new(HashMap::new())); diff --git a/sources/src/readers/github_tests.rs b/sources/src/readers/github_tests.rs new file mode 100644 index 0000000..04f13f3 --- /dev/null +++ b/sources/src/readers/github_tests.rs @@ -0,0 +1,293 @@ +use super::*; +use crate::raw_kind::RawKind; + +#[test] +fn git_log_args_default_to_head_without_branch() { + // With no branch configured the walk must stay on the bare clone's HEAD + // (the default branch), matching the REST fallback's default-branch scope + // rather than walking every ref. + let args = git::log_args(50, None, &[]); + assert_eq!( + args, + vec![ + "log".to_string(), + "HEAD".to_string(), + "--max-count=50".to_string(), + "--format=%H\t%s\t%aI".to_string(), + ] + ); +} + +#[test] +fn git_log_args_restrict_to_branch_and_paths() { + let args = git::log_args( + 50, + Some("main"), + &["src/lib.rs".to_string(), "docs/".to_string()], + ); + assert_eq!( + args, + vec![ + "log".to_string(), + "main".to_string(), + "--max-count=50".to_string(), + "--format=%H\t%s\t%aI".to_string(), + "--".to_string(), + "src/lib.rs".to_string(), + "docs/".to_string(), + ] + ); + // Empty/whitespace branch falls back to HEAD, never an empty ref. + let args = git::log_args(1, Some(""), &[]); + assert_eq!(args[1], "HEAD"); +} + +#[test] +fn commit_list_queries_carry_branch_and_path_filters() { + // No filters → a single empty query (plain pagination). + assert_eq!(api::commit_list_queries(None, &[]), vec![String::new()]); + // Branch only → `sha=`. + assert_eq!( + api::commit_list_queries(Some("main"), &[]), + vec![String::from("sha=main")] + ); + // One path → `path=

`. + assert_eq!( + api::commit_list_queries(None, &["src/".to_string()]), + vec![String::from("path=src/")] + ); + // Branch + one path → `sha=&path=

`. + assert_eq!( + api::commit_list_queries(Some("main"), &["src/lib.rs".to_string()]), + vec![String::from("sha=main&path=src/lib.rs")] + ); + // Multiple paths → one query per path, dedup happens in the caller. + assert_eq!( + api::commit_list_queries(Some("main"), &["a/".to_string(), "b/".to_string()]), + vec![ + String::from("sha=main&path=a/"), + String::from("sha=main&path=b/") + ] + ); + // Empty branch is treated as unset. + assert_eq!( + api::commit_list_queries(Some(""), &["a/".to_string()]), + vec![String::from("path=a/")] + ); +} + +#[test] +fn commit_list_queries_percent_encode_special_chars() { + // `&`, `#`, `=` and spaces inside a branch or path value would be parsed + // as query syntax and corrupt the filter; they must be percent-encoded. + // `/` is left intact (legal in a query component, and GitHub's commits + // `path` filter expects the common `path=src/` shape unencoded). + assert_eq!( + api::commit_list_queries(Some("feature/one&two"), &["src/#1.rs".to_string()]), + vec![String::from("sha=feature/one%26two&path=src/%231.rs")] + ); + // Unreserved values are unchanged. + assert_eq!( + api::commit_list_queries(Some("main"), &["docs/".to_string()]), + vec![String::from("sha=main&path=docs/")] + ); +} + +#[tokio::test] +async fn fetch_all_pages_keeps_page_size_constant_and_truncates() { + // Regression: the page size must not shrink mid-walk. With `max = 150` + // (not a multiple of 100), a shrinking `per_page` would re-window the + // offsets — page 2 at per_page=50 returns items 51-100 again, skipping + // 101-150. A constant page size walks page 1 and page 2 both at + // per_page=100 and truncates the 200 collected rows to 150. + let mut requested: Vec = Vec::new(); + let pages = api::collect_pages::("commits", 150, |page| { + let url = format!("per_page=100&page={page}"); + requested.push(url); + // 100 rows per page, all full (never a short page before the cap). + let rows: Vec = (1..=100) + .map(|i| format!("{}", (page - 1) * 100 + i)) + .collect(); + async move { Ok(format!("[{}]", rows.join(","))) } + }) + .await + .unwrap(); + + assert_eq!( + requested, + vec![ + "per_page=100&page=1".to_string(), + "per_page=100&page=2".to_string(), + ] + ); + assert_eq!(pages.len(), 150); + // No overlap: the second page is the next window (101..), not 51..100. + assert_eq!(pages[0], 1); + assert_eq!(pages[100], 101); + assert_eq!(pages[149], 150); +} + +#[tokio::test] +async fn fetch_all_pages_stops_at_a_short_page() { + // A short page (fewer than GH_PAGE_SIZE rows) is the last page; the walk + // must not request page 2 after it. + let mut requested: Vec = Vec::new(); + let pages = crate::readers::github::api::collect_pages::( + "commits", + 1000, + |page| { + requested.push(page); + async move { + // Page 1 is short (3 rows) — stop after it even though max is large. + Ok("[1,2,3]".to_string()) + } + }, + ) + .await + .unwrap(); + + assert_eq!(requested, vec![1]); + assert_eq!(pages, vec![1, 2, 3]); +} + +/// Build a synthetic `GhCommit` for merge tests. +fn gh_commit(sha: &str, subject: &str, ts: &str) -> types::GhCommit { + types::GhCommit { + sha: sha.into(), + commit: types::GhCommitInner { + message: subject.into(), + author: None, + committer: Some(types::GhAuthor { + name: None, + email: None, + date: Some(ts.into()), + }), + }, + author: None, + } +} + +#[test] +fn merge_commit_batches_walks_every_path_before_truncating() { + // Two configured paths: the first returns two commits, the second one. + // The pre-fix code stopped after the first path once `out` reached `max`, + // silently dropping the `src` commit even though it is newer than the + // second `docs` commit. + let docs = vec![gh_commit("a", "docs first", "2024-01-01T00:00:00Z")]; + let src = vec![gh_commit("b", "src newer", "2024-02-01T00:00:00Z")]; + + let merged = api::merge_commit_batches(vec![docs, src], 3); + let ids: Vec<&str> = merged.iter().map(|i| i.id.as_str()).collect(); + assert_eq!( + ids, + vec!["commit:b", "commit:a"], + "newest-first, both paths kept" + ); +} + +#[test] +fn merge_commit_batches_dedups_by_sha_and_truncates_globally() { + // A commit touching both paths appears in both batches but only once. + let docs = vec![ + gh_commit("a", "docs first", "2024-01-01T00:00:00Z"), + gh_commit("shared", "touches both", "2024-02-01T00:00:00Z"), + ]; + let src = vec![gh_commit("shared", "touches both", "2024-02-01T00:00:00Z")]; + + let merged = api::merge_commit_batches(vec![docs, src], 1); + let ids: Vec<&str> = merged.iter().map(|i| i.id.as_str()).collect(); + assert_eq!(ids, vec!["commit:shared"], "deduped and truncated to max"); +} + +#[test] +fn parse_github_url_extracts_owner_and_repo() { + let (owner, repo) = parse_github_url("https://github.com/openai/tiktoken").unwrap(); + assert_eq!(owner, "openai"); + assert_eq!(repo, "tiktoken"); +} + +#[test] +fn parse_github_url_handles_trailing_slash_and_git() { + let (owner, repo) = parse_github_url("https://github.com/org/repo.git/").unwrap(); + assert_eq!(owner, "org"); + assert_eq!(repo, "repo"); +} + +#[test] +fn parse_github_url_rejects_non_repo_paths() { + // Deep links like /tree/main must not silently extract the wrong + // owner/repo. Bare host or non-github URLs also rejected. + assert!(parse_github_url("https://github.com/org/repo/tree/main").is_err()); + assert!(parse_github_url("https://gitlab.com/org/repo").is_err()); + assert!(parse_github_url("https://github.com/org").is_err()); + assert!(parse_github_url("not-a-url").is_err()); +} + +#[test] +fn item_kind_round_trips() { + let cases = [ + ("commit:abc123", ItemKind::Commit, "abc123"), + ("issue:42", ItemKind::Issue, "42"), + ("pr:99", ItemKind::PullRequest, "99"), + ]; + for (id, expected_kind, expected_ref) in cases { + let (kind, ref_id) = ItemKind::from_id(id).unwrap(); + assert_eq!(kind, expected_kind); + assert_eq!(ref_id, expected_ref); + } +} + +#[test] +fn item_kind_rejects_invalid() { + assert!(ItemKind::from_id("unknown:123").is_none()); + assert!(ItemKind::from_id("noprefix").is_none()); +} + +#[test] +fn repo_archive_source_id_slugs_to_repo_folder() { + // `github.com//` → slugify → `github-com--`. + assert_eq!( + repo_archive_source_id("https://github.com/tinyhumansai/openhuman").as_deref(), + Some("github.com/tinyhumansai/openhuman") + ); + assert!(repo_archive_source_id("not-a-url").is_none()); +} + +#[test] +fn chunk_source_id_is_clean_and_per_item() { + assert_eq!( + chunk_source_id("https://github.com/org/repo", "commit:abc123").as_deref(), + Some("github:org/repo:commit:abc123") + ); + assert_eq!( + chunk_source_id("https://github.com/org/repo", "pr:42").as_deref(), + Some("github:org/repo:pr:42") + ); +} + +#[test] +fn unique_handles_dedups_and_skips_unknown() { + assert_eq!( + unique_handles(["alice", "bob", "alice", "unknown", ""].into_iter()), + "@alice @bob" + ); + assert_eq!(unique_handles(["unknown", ""].into_iter()), "none"); + assert_eq!(unique_handles(std::iter::empty()), "none"); +} + +#[test] +fn raw_archive_coords_maps_kind_and_uid() { + assert_eq!( + raw_archive_coords("commit:deadbeef"), + Some((RawKind::Commit, "deadbeef".to_string())) + ); + assert_eq!( + raw_archive_coords("issue:7"), + Some((RawKind::Issue, "7".to_string())) + ); + assert_eq!( + raw_archive_coords("pr:99"), + Some((RawKind::PullRequest, "99".to_string())) + ); + assert!(raw_archive_coords("bogus:1").is_none()); +} diff --git a/sources/src/readers/mod.rs b/sources/src/readers/mod.rs new file mode 100644 index 0000000..5ce0d65 --- /dev/null +++ b/sources/src/readers/mod.rs @@ -0,0 +1,112 @@ +//! Source readers: the [`SourceReader`] trait plus local implementations. +//! +//! A reader knows how to *list* the items available in a source and *read* the +//! content of one item. The trait is intentionally narrow so the host can drive +//! ingestion uniformly across every source kind. +//! +//! ## Ownership boundary +//! +//! Fetching and parsing a source is engine work, so the `github_repo`, +//! `rss_feed`, and `web_page` readers live here behind the `sync` feature +//! alongside the always-compiled local kinds ([`folder::FolderReader`], +//! [`conversation::ConversationReader`]). What TinyCortex still does **not** +//! own is *when* a network read happens: scheduling, polling cadence, OAuth, +//! credentials, and egress/cost budgeting stay with the host. +//! +//! That is why [`reader_for`] and [`is_locally_readable`] draw their line at +//! **local vs. network**, not at implemented vs. absent. A network reader is +//! constructed explicitly (`github::GithubReader`, `rss::RssReader`, +//! `web_page::WebPageReader`) by a caller that has already decided the fetch is +//! allowed; it is never handed out by the kind-dispatch that +//! the workspace sync loop drives on a timer. A `None` from +//! [`reader_for`] therefore still means "route this through the host's sync +//! runner", which is what keeps the host in charge of hitting the network. +//! +//! `composio` and `twitter_query` have no reader here at all — the former is a +//! credentialed OAuth pipeline, the latter is unimplemented. + +pub mod conversation; +pub mod folder; +#[cfg(feature = "network")] +pub mod github; +#[cfg(feature = "network")] +pub mod rss; +#[cfg(feature = "network")] +pub mod web_page; + +/// SSRF guard + fetch hygiene shared by the sync-gated network readers +/// (`web_page`, `rss`). See the `ssrf` module docs. +#[cfg(feature = "network")] +mod ssrf; + +use async_trait::async_trait; + + +use crate::SourceResult; +#[cfg(feature = "network")] +use tinymemory_api::error::MemoryError; + +use super::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; + +/// A reader that can list items and read content from a memory source. +/// +/// Implementations are synchronous internally but expose an async surface so a +/// network-backed reader (host-owned) can satisfy the same contract. +#[async_trait] +pub trait SourceReader: Send + Sync { + /// The [`SourceKind`] this reader serves. + fn kind(&self) -> SourceKind; + + /// List the items currently available in `source`. + async fn list_items( + &self, + source: &MemorySourceEntry, + workspace: &std::path::Path, + ) -> SourceResult>; + + /// Read the content of a single item by its reader-scoped `item_id`. + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + workspace: &std::path::Path, + ) -> SourceResult; +} + +/// Whether a kind can be read from local state alone, with no network egress. +/// +/// Network-backed kinds return `false` even when this build ships their reader +/// (see the module docs): the host decides when a fetch is allowed. +pub fn is_locally_readable(kind: &SourceKind) -> bool { + matches!(kind, SourceKind::Folder | SourceKind::Conversation) +} + +/// Get the reader for a source kind that is safe to drive on a timer. +/// +/// Returns `Some` for [`SourceKind::Folder`] and [`SourceKind::Conversation`]. +/// Network-backed kinds (`composio`, `github_repo`, `rss_feed`, `web_page`, +/// `twitter_query`) return `None` so the caller defers to the host's sync +/// runner — including the three whose readers this crate now implements, which +/// callers construct by name once the host has authorized the fetch. +pub fn reader_for(kind: &SourceKind) -> Option> { + match kind { + SourceKind::Folder => Some(Box::new(folder::FolderReader)), + SourceKind::Conversation => Some(Box::new(conversation::ConversationReader)), + SourceKind::Composio + | SourceKind::GithubRepo + | SourceKind::TwitterQuery + | SourceKind::RssFeed + | SourceKind::WebPage => None, + } +} + +/// Wrap a reader's plain-string failure as a [`MemoryError`]. +/// +/// The network readers below carry their diagnostics as `String` internally. +/// [`MemoryError::Other`] is `#[error(transparent)]`, so `to_string()` on the +/// result reproduces the original message byte-for-byte — callers that match on +/// reader error text keep working unchanged. +#[cfg(feature = "network")] +pub(crate) fn into_engine_error(message: String) -> MemoryError { + MemoryError::Other(anyhow::anyhow!(message)) +} diff --git a/sources/src/readers/rss.rs b/sources/src/readers/rss.rs new file mode 100644 index 0000000..7312522 --- /dev/null +++ b/sources/src/readers/rss.rs @@ -0,0 +1,402 @@ +//! RSS/Atom feed source reader. +//! +//! Fetches and parses an RSS or Atom feed, returning entries as +//! source items. Uses a lightweight XML parser (`quick-xml` via +//! manual parsing) to avoid pulling in heavy feed crates. +//! +//! Fetches go through the shared `ssrf` guard (scheme/host policy, a DNS +//! resolver that pins connections to globally routable addresses, and +//! per-hop redirect re-checks), and the parsed feed is cached briefly so a +//! list-then-read sync pass downloads it once rather than once per entry. + +mod types; + +use std::sync::Mutex; +use std::time::{Duration, Instant}; + +use async_trait::async_trait; + + +use crate::SourceResult; +use crate::types::{ + ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, +}; + +use super::ssrf::{build_client, is_url_allowed, read_body_capped}; +use super::{into_engine_error, SourceReader}; +use types::{FeedCache, FeedEntry}; + +const DEFAULT_MAX_ITEMS: u32 = 50; +const MAX_FEED_BYTES: u64 = 5 * 1024 * 1024; // 5 MiB — guards against pathological feeds + +/// How long a fetched feed is reused before the next read re-downloads it. +/// +/// Kept short so a feed that updates mid-sync is picked up on the next sync; +/// long enough to cover a list-then-read pass over a 50-entry feed. +const FEED_CACHE_TTL: Duration = Duration::from_secs(60); + +/// Reader for an RSS/Atom feed source. +/// +/// Holds a short-lived cache of the last fetched feed so that a `list_items` +/// immediately followed by per-item `read_item` calls fetches the feed once. +pub struct RssReader { + cache: Mutex>, +} + +impl RssReader { + /// A reader with an empty feed cache. + pub fn new() -> Self { + Self::default() + } + + /// Fetch (or reuse a very fresh copy of) the feed at `url`. + /// + /// The workspace sync pipeline holds one reader across a tick and calls + /// `list_items` once, then `read_item` once per entry. Without a cache + /// that is N+1 downloads of the same feed per sync (and a rate-limit + /// risk against the feed host); the cache turns it into one fetch whose + /// results are reused for the read phase. + async fn fetch_entries(&self, url: &str) -> Result, String> { + // Read the cache in a nested scope so the mutex guard is dropped before + // the await below — the guard is not `Send`, and holding it across an + // await would make the reader's async methods non-`Send`. + { + let cache = self.cache.lock().unwrap_or_else(|e| e.into_inner()); + if let Some(cached) = cache.as_ref() { + if cached.url == url && cached.fetched_at.elapsed() < FEED_CACHE_TTL { + return Ok(cached.entries.clone()); + } + } + } + + let body = fetch_url(url).await?; + let entries = parse_feed_full(&body)?; + *self.cache.lock().unwrap_or_else(|e| e.into_inner()) = Some(FeedCache { + url: url.to_string(), + fetched_at: Instant::now(), + entries: entries.clone(), + }); + Ok(entries) + } +} + +impl Default for RssReader { + fn default() -> Self { + Self { + cache: Mutex::new(None), + } + } +} + +#[async_trait] +impl SourceReader for RssReader { + fn kind(&self) -> SourceKind { + SourceKind::RssFeed + } + + async fn list_items( + &self, + source: &MemorySourceEntry, + workspace: &std::path::Path, + ) -> SourceResult> { + self.list_items_inner(source, workspace) + .await + .map_err(into_engine_error) + } + + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + workspace: &std::path::Path, + ) -> SourceResult { + self.read_item_inner(source, item_id, workspace) + .await + .map_err(into_engine_error) + } +} + +impl RssReader { + async fn list_items_inner( + &self, + source: &MemorySourceEntry, + _workspace: &std::path::Path, + ) -> Result, String> { + let url = source.url.as_deref().ok_or("rss source requires a url")?; + let max_items = source.max_items.unwrap_or(DEFAULT_MAX_ITEMS) as usize; + + tracing::debug!( + host = %url_host(url), + max_items = max_items, + "[memory_sources:rss] listing items" + ); + + let entries = self.fetch_entries(url).await?; + + tracing::debug!(count = entries.len(), "[memory_sources:rss] parsed entries"); + + Ok(entries + .into_iter() + .take(max_items) + .map(|e| SourceItem { + id: e.id, + title: e.title, + updated_at_ms: e.updated_at_ms, + }) + .collect()) + } + + async fn read_item_inner( + &self, + source: &MemorySourceEntry, + item_id: &str, + _workspace: &std::path::Path, + ) -> Result { + let url = source.url.as_deref().ok_or("rss source requires a url")?; + + tracing::debug!( + host = %url_host(url), + item_id = %item_id, + "[memory_sources:rss] reading item" + ); + + let entries = self.fetch_entries(url).await?; + let entry = entries + .into_iter() + .find(|e| e.id == item_id) + .ok_or_else(|| format!("item '{item_id}' not found in feed"))?; + + let content_type = if entry.body.contains('<') { + ContentType::Html + } else { + ContentType::Plaintext + }; + + Ok(SourceContent { + id: entry.id, + title: entry.title, + body: entry.body, + content_type, + metadata: serde_json::json!({ + "link": entry.link, + "published": entry.published, + }), + }) + } +} + +/// Extract just the host portion of a URL for debug-log redaction so we +/// don't leak query params, paths, or embedded credentials (userinfo). +fn url_host(url: &str) -> String { + // A real parse drops any `user:pass@` prefix via `host_str()`. When the + // value is not a parseable URL (it will be rejected by the SSRF guard + // later anyway), fall back to a textual host extraction that still strips + // userinfo and the path/query/fragment. + reqwest::Url::parse(url) + .ok() + .and_then(|u| u.host_str().map(str::to_string)) + .unwrap_or_else(|| { + let authority = url + .trim_start_matches("https://") + .trim_start_matches("http://") + .split(['/', '?', '#']) + .next() + .unwrap_or(url); + // Only the last `@`-separated segment can be the host; anything + // before it is credentials and must not reach the log. + authority + .rsplit('@') + .next() + .unwrap_or(authority) + .to_string() + }) +} + +async fn fetch_url(url: &str) -> Result { + // SSRF guard: validate scheme and host, reject private/internal targets, + // and refuse redirects that would escape that policy. + let parsed = reqwest::Url::parse(url).map_err(|e| format!("invalid URL: {e}"))?; + if !is_url_allowed(&parsed) { + return Err(format!( + "rss source requires an http(s) URL to a public host, got: {}", + url.chars().take(64).collect::() + )); + } + + let client = build_client()?; + let resp = client + .get(parsed) + .header("User-Agent", "openhuman") + .send() + .await + .map_err(|e| format!("failed to fetch feed: {e}"))?; + + if !resp.status().is_success() { + return Err(format!("feed returned {}", resp.status())); + } + + // Stream the body with a cap so a pathological feed can't OOM us before + // the size check runs (`Content-Length` can be omitted or understated). + let bytes = read_body_capped(resp, MAX_FEED_BYTES).await?; + String::from_utf8(bytes).map_err(|e| format!("feed body is not valid UTF-8: {e}")) +} + +fn parse_feed_full(xml: &str) -> Result, String> { + // Detect RSS vs Atom by looking for Result, String> { + let mut entries = Vec::new(); + let mut offset = 0; + + while let Some(item_start) = xml[offset..].find("") + .map(|i| abs_start + i + 7) + .unwrap_or(xml.len()); + + let item_xml = &xml[abs_start..item_end]; + let title = extract_tag(item_xml, "title").unwrap_or_default(); + let link = extract_tag(item_xml, "link"); + let guid = extract_tag(item_xml, "guid"); + let description = extract_tag(item_xml, "description") + // An empty `` is a present-but-empty + // tag: `extract_tag` returns `Some("")`, which would short-circuit + // the `content:encoded` fallback below and ingest an empty body. + // Filter it out so a populated `content:encoded` still wins. + .filter(|s| !s.is_empty()) + .or_else(|| extract_cdata(item_xml, "content:encoded")) + .unwrap_or_default(); + let pub_date = extract_tag(item_xml, "pubDate"); + + let id = guid + .or_else(|| link.clone()) + .unwrap_or_else(|| format!("rss-{}", entries.len())); + + entries.push(FeedEntry { + updated_at_ms: pub_date.as_deref().and_then(rss_timestamp_ms), + id, + title, + body: description, + link, + published: pub_date, + }); + + offset = item_end; + } + + Ok(entries) +} + +fn parse_atom(xml: &str) -> Result, String> { + let mut entries = Vec::new(); + let mut offset = 0; + + while let Some(entry_start) = xml[offset..].find("") + .map(|i| abs_start + i + 8) + .unwrap_or(xml.len()); + + let entry_xml = &xml[abs_start..entry_end]; + let title = extract_tag(entry_xml, "title").unwrap_or_default(); + let id = extract_tag(entry_xml, "id").unwrap_or_else(|| format!("atom-{}", entries.len())); + let content = extract_tag(entry_xml, "content") + // Same shape as the RSS `description`/`content:encoded` pair: an + // empty `` must not block the `summary` + // fallback. + .filter(|s| !s.is_empty()) + .or_else(|| extract_tag(entry_xml, "summary")) + .unwrap_or_default(); + let link = extract_attr(entry_xml, "link", "href"); + let updated = + extract_tag(entry_xml, "updated").or_else(|| extract_tag(entry_xml, "published")); + + entries.push(FeedEntry { + updated_at_ms: updated.as_deref().and_then(rss_timestamp_ms), + id, + title, + body: content, + link, + published: updated, + }); + + offset = entry_end; + } + + Ok(entries) +} + +/// Parse an RSS `pubDate` (RFC 2822) or Atom `updated`/`published` (RFC 3339) +/// timestamp into epoch milliseconds, so workspace sync can skip unchanged +/// entries instead of re-reading every item on every pass. +fn rss_timestamp_ms(value: &str) -> Option { + chrono::DateTime::parse_from_rfc2822(value) + .or_else(|_| chrono::DateTime::parse_from_rfc3339(value)) + .map(|dt| dt.timestamp_millis()) + .ok() +} + +/// Remove a surrounding `` wrapper, if present. +fn unwrap_cdata(s: &str) -> &str { + s.strip_prefix("")) + .unwrap_or(s) +} + +fn extract_tag(xml: &str, tag: &str) -> Option { + let open = format!("<{tag}"); + let close = format!(""); + let start = xml.find(&open)?; + let content_start = xml[start..].find('>')? + start + 1; + let end = xml[content_start..].find(&close)? + content_start; + let content = &xml[content_start..end]; + let trimmed = content.trim(); + let unwrapped = unwrap_cdata(trimmed).trim(); + // CDATA content is literal text, so entity decoding applies only outside + // a CDATA wrapper; decoding `<` inside one would corrupt the content. + if trimmed.starts_with(" Option { + // `extract_tag` already unwraps ``, so it serves both the + // plain-text and CDATA-wrapped shapes. + extract_tag(xml, tag) +} + +fn extract_attr(xml: &str, tag: &str, attr: &str) -> Option { + let open = format!("<{tag} "); + let start = xml.find(&open)?; + let tag_end = xml[start..].find('>')? + start; + let tag_str = &xml[start..tag_end]; + let attr_start = tag_str.find(&format!("{attr}=\""))? + attr.len() + 2; + let attr_end = tag_str[attr_start..].find('"')? + attr_start; + Some(tag_str[attr_start..attr_end].to_string()) +} + +fn decode_xml_entities(s: &str) -> String { + // `&` is decoded last so escaped entity text (`&lt;` → `<`) + // survives as literal text instead of being decoded a second time. + s.replace("<", "<") + .replace(">", ">") + .replace(""", "\"") + .replace("'", "'") + .replace("&", "&") +} + +#[cfg(test)] +#[path = "rss_tests.rs"] +mod tests; diff --git a/sources/src/readers/rss/types.rs b/sources/src/readers/rss/types.rs new file mode 100644 index 0000000..1804f1c --- /dev/null +++ b/sources/src/readers/rss/types.rs @@ -0,0 +1,24 @@ +//! Private feed-shape types for the RSS reader: the parsed entry model and +//! the short-lived cache snapshot reused across a list-then-read sync pass. + +use std::time::Instant; + +/// A fetched feed snapshot cached across a list-then-read sync pass. +pub(super) struct FeedCache { + pub url: String, + pub fetched_at: Instant, + pub entries: Vec, +} + +/// One parsed RSS/Atom entry: the dedupe id, the fields surfaced as a source +/// item / content, and the raw link + publication timestamp carried in the +/// content metadata. +#[derive(Debug, Clone)] +pub(super) struct FeedEntry { + pub id: String, + pub title: String, + pub body: String, + pub link: Option, + pub published: Option, + pub updated_at_ms: Option, +} diff --git a/sources/src/readers/rss_tests.rs b/sources/src/readers/rss_tests.rs new file mode 100644 index 0000000..8f9e20b --- /dev/null +++ b/sources/src/readers/rss_tests.rs @@ -0,0 +1,243 @@ +use super::*; + +#[test] +fn parse_rss_extracts_items() { + let xml = r#" + + + Test Feed + + First post + https://example.com/1 + Body of first post + + + Second post + guid-2 + Body of second + + + "#; + + let entries = parse_rss(xml).unwrap(); + assert_eq!(entries.len(), 2); + assert_eq!(entries[0].title, "First post"); + assert_eq!(entries[0].id, "https://example.com/1"); + assert_eq!(entries[1].id, "guid-2"); +} + +#[test] +fn parse_atom_extracts_entries() { + let xml = r#" + + + Atom entry + urn:entry:1 + Content here + + + "#; + + let entries = parse_atom(xml).unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].title, "Atom entry"); + assert_eq!(entries[0].id, "urn:entry:1"); + assert_eq!( + entries[0].link.as_deref(), + Some("https://example.com/atom/1") + ); +} + +#[test] +fn parse_feed_detects_format() { + let rss = "T"; + assert!(parse_feed_full(rss).is_ok()); + + let atom = "T1"; + assert!(parse_feed_full(atom).is_ok()); + + assert!(parse_feed_full("").is_err()); +} + +// ── Entry timestamps ─────────────────────────────────────────────── + +#[test] +fn parse_rss_emits_pubdate_timestamp() { + let xml = r#" + + Post + 1 + Wed, 02 Oct 2002 13:00:00 GMT + + "#; + + let entries = parse_rss(xml).unwrap(); + // 2002-10-02T13:00:00Z in epoch milliseconds. + assert_eq!(entries[0].updated_at_ms, Some(1_033_563_600_000)); +} + +#[test] +fn parse_atom_emits_updated_timestamp() { + let xml = r#" + + Atom entry + urn:entry:1 + 2026-03-01T12:00:00.000Z + + "#; + + let entries = parse_atom(xml).unwrap(); + // 2026-03-01T12:00:00Z in epoch milliseconds. + assert_eq!(entries[0].updated_at_ms, Some(1_772_366_400_000)); +} + +#[test] +fn parse_item_without_timestamp_is_unknown() { + let xml = "T"; + let entries = parse_rss(xml).unwrap(); + assert_eq!(entries[0].updated_at_ms, None); +} + +#[test] +fn rss_timestamp_ms_rejects_garbage() { + assert_eq!(rss_timestamp_ms("not a date"), None); + assert_eq!(rss_timestamp_ms(""), None); +} + +// ── CDATA unwrapping ──────────────────────────────────────────────── + +#[test] +fn extract_tag_unwraps_cdata() { + // The common RSS shape `body

]]>` + // must yield clean HTML, not the literal CDATA markers. + let xml = "body

]]>"; + assert_eq!( + extract_tag(xml, "description").as_deref(), + Some("

body

") + ); +} + +#[test] +fn extract_tag_does_not_entity_decode_inside_cdata() { + // CDATA content is literal: `<` must survive intact, not become `<`. + let xml = ""; + assert_eq!( + extract_tag(xml, "description").as_deref(), + Some("Say <tag> literally") + ); +} + +#[test] +fn extract_tag_entity_decodes_outside_cdata() { + let xml = "A & B <b> bold"; + assert_eq!(extract_tag(xml, "title").as_deref(), Some("A & B bold")); +} + +#[test] +fn extract_cdata_reuses_tag_extraction() { + // `content:encoded` is typically CDATA-wrapped; extract_cdata must match + // extract_tag on the same input. + let xml = "full

]]>"; + assert_eq!( + extract_cdata(xml, "content:encoded").as_deref(), + Some("

full

") + ); +} + +#[test] +fn parse_rss_description_with_cdata_is_clean() { + let xml = r#" + + Post + 1 + Hello world

]]> + + "#; + + let entries = parse_rss(xml).unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].body, "

Hello world

"); + assert!(!entries[0].body.contains("CDATA")); +} + +#[test] +fn parse_rss_empty_description_falls_back_to_encoded_content() { + // A present-but-empty `` must not block the + // `content:encoded` fallback — the item carries its body there instead. + let xml = r#" + + Post + 1 + + Full body from content:encoded

]]> + + "#; + + let entries = parse_rss(xml).unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].body, "

Full body from content:encoded

"); +} + +#[test] +fn parse_atom_empty_content_falls_back_to_summary() { + // Mirrors the RSS `description`/`content:encoded` pair: an empty + // `` must fall through to a populated ``. + let xml = r#" + + Atom entry + urn:entry:1 + + Summary body + + "#; + + let entries = parse_atom(xml).unwrap(); + assert_eq!(entries.len(), 1); + assert_eq!(entries[0].body, "Summary body"); +} + +// ── URL host redaction ────────────────────────────────────────────── + +#[test] +fn url_host_redacts_userinfo() { + // Credentials embedded in a source URL must never reach debug traces — + // only the host is logged. + assert_eq!( + url_host("https://alice:secret@example.com/feed.xml"), + "example.com" + ); +} + +#[test] +fn url_host_drops_path_query_and_fragment() { + assert_eq!( + url_host("https://example.com/feed?token=abc#top"), + "example.com" + ); +} + +#[test] +fn url_host_fallback_strips_userinfo_without_scheme() { + // Scheme-less values are unparseable by reqwest; the textual fallback + // must still drop the `user:pass@` prefix and the path. + assert_eq!(url_host("alice:secret@example.com/feed"), "example.com"); + assert_eq!(url_host("example.com/feed"), "example.com"); +} + +// ── Entity decoding ───────────────────────────────────────────────── + +#[test] +fn decode_xml_entities_decodes_amp_last() { + // `&lt;` is the escaped form of `<`; it must decode once to `<`, + // not twice to `<`. + assert_eq!(decode_xml_entities("&lt;"), "<"); + assert_eq!(decode_xml_entities("&amp;"), "&"); +} + +#[test] +fn decode_xml_entities_handles_all_named() { + assert_eq!( + decode_xml_entities("<b> "q" 'a' & more"), + " \"q\" 'a' & more" + ); +} diff --git a/sources/src/readers/ssrf.rs b/sources/src/readers/ssrf.rs new file mode 100644 index 0000000..85a8e96 --- /dev/null +++ b/sources/src/readers/ssrf.rs @@ -0,0 +1,226 @@ +//! Shared SSRF guard and fetch hygiene for the network source readers. +//! +//! The web-page and RSS readers both fetch user-configured URLs, so they share +//! the policy in this module. +//! +//! The hostname *text* check (`is_blocked_host`) rejects private IP literals +//! (including their IPv4-mapped IPv6 forms, e.g. `::ffff:127.0.0.1`), +//! `localhost`, `.local` / `.internal` names, and single-label hostnames, but +//! a public-looking name can resolve to a loopback / private / link-local +//! address (including the cloud-metadata `169.254.169.254`) at lookup time. +//! `PublicOnlyResolver` therefore vets the resolved addresses and only lets the +//! connection proceed to a globally routable IP, so the request is pinned to an +//! address we have already allowed (no re-resolution between the check and the +//! connect). Redirects are re-checked through `is_url_allowed` so a public URL +//! cannot bounce the fetch onto an internal host. +//! +//! `read_body_capped` streams a response body and stops at a byte cap, so a +//! hostile or gigantic page/feed cannot OOM the process before the size check +//! runs. + +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use std::sync::Arc; + +use futures::stream::StreamExt; +use reqwest::dns::{Addrs, Name, Resolve, Resolving}; + +/// Build an HTTP client with a redirect policy that re-applies the SSRF +/// host/scheme check to every redirect hop, and a DNS resolver that only +/// yields globally routable addresses. +pub(super) fn build_client() -> Result { + reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(20)) + .redirect(reqwest::redirect::Policy::custom(|attempt| { + if is_url_allowed(attempt.url()) { + attempt.follow() + } else { + // `stop` returns the redirect response to the caller instead + // of following it; the read then fails on the non-2xx status. + attempt.stop() + } + })) + .dns_resolver(Arc::new(PublicOnlyResolver)) + .build() + .map_err(|e| format!("failed to build http client: {e}")) +} + +/// Stream a response body, failing once it exceeds `max` bytes. +/// +/// `Response::bytes()` buffers the entire body before any size check, so a +/// server that omits or understates `Content-Length` (for example a chunked +/// response) could OOM the process despite the cap. Reading incrementally +/// enforces the limit while the bytes arrive. +pub(super) async fn read_body_capped(resp: reqwest::Response, max: u64) -> Result, String> { + // Trust a truthful Content-Length up front so a known-huge body is + // rejected before the first byte is read. + if let Some(len) = resp.content_length() { + if len > max { + return Err(format!( + "response body exceeds {max}-byte limit (Content-Length={len})" + )); + } + } + + let mut body = Vec::new(); + let mut stream = resp.bytes_stream(); + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|e| format!("failed to read response body: {e}"))?; + body.extend_from_slice(&chunk); + if body.len() as u64 > max { + return Err(format!( + "response body exceeds {max}-byte limit (read {} bytes)", + body.len() + )); + } + } + Ok(body) +} + +/// A DNS resolver that only yields globally routable addresses. +/// +/// The text-based `is_blocked_host` check rejects private IP *literals* and +/// local hostnames, but a public-looking hostname can resolve to a loopback, +/// private, link-local, or cloud-metadata address (`169.254.169.254`) at +/// lookup time. Installing this resolver means reqwest connects to addresses +/// we have already vetted: a hostname whose current resolution is non-public +/// fails the request instead of silently reaching an internal service, and the +/// validated address is the one the connection is pinned to (no re-resolution +/// between the check and the connect). +#[derive(Debug, Default)] +struct PublicOnlyResolver; + +impl Resolve for PublicOnlyResolver { + fn resolve(&self, name: Name) -> Resolving { + let host = name.as_str().to_string(); + Box::pin(async move { + let addrs: Vec = tokio::net::lookup_host((host.as_str(), 0)) + .await + .map_err(box_err)? + .filter(|addr| is_public_ip(addr.ip())) + .collect(); + if addrs.is_empty() { + return Err(box_err(std::io::Error::new( + std::io::ErrorKind::AddrNotAvailable, + format!("host {host} resolved to no public addresses"), + ))); + } + Ok(Box::new(addrs.into_iter()) as Addrs) + }) + } +} + +fn box_err( + e: impl std::error::Error + Send + Sync + 'static, +) -> Box { + Box::new(e) +} + +/// Whether `ip` is a globally routable address — the resolved-address half of +/// the SSRF guard. Mirrors the literal/name policy in `is_blocked_host`: +/// loopback, private, link-local, unique-local, multicast, broadcast, +/// unspecified, and documentation/reserved ranges are not fetchable. +fn is_public_ip(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(v4) => is_public_ipv4(v4), + IpAddr::V6(v6) => is_public_ipv6(v6), + } +} + +fn is_public_ipv4(ip: Ipv4Addr) -> bool { + if is_private_ipv4(ip) || ip.is_multicast() || ip.is_broadcast() { + return false; + } + let o = ip.octets(); + // Documentation (192.0.2.0/24, 198.51.100.0/24, 203.0.113.0/24), + // benchmarking (198.18.0.0/15), and reserved (240.0.0.0/4) ranges are not + // globally routable. + !((o[0] == 192 && o[1] == 0 && o[2] == 2) + || (o[0] == 198 && o[1] == 51 && o[2] == 100) + || (o[0] == 203 && o[1] == 0 && o[2] == 113) + || (o[0] == 198 && o[1] == 18) + || o[0] >= 240) +} + +fn is_public_ipv6(ip: Ipv6Addr) -> bool { + if is_private_ipv6(ip) || ip.is_multicast() { + return false; + } + let o = ip.octets(); + // Documentation prefix 2001:db8::/32. + if o[0] == 0x20 && o[1] == 0x01 && o[2] == 0x0d && o[3] == 0xb8 { + return false; + } + // IPv4-mapped (`::ffff:a.b.c.d`) delegate to the embedded IPv4, so a + // mapped loopback/private address stays blocked. + if let Some(v4) = ip.to_ipv4_mapped() { + return is_public_ipv4(v4); + } + true +} + +/// Whether a URL may be fetched: `http(s)` scheme against a public host. +pub(super) fn is_url_allowed(url: &reqwest::Url) -> bool { + match url.scheme() { + "http" | "https" => {} + _ => return false, + } + let Some(host) = url.host_str() else { + return false; + }; + !is_blocked_host(host) +} + +/// Reject hosts that could target non-public resources: IP literals in +/// loopback / private / link-local / unique-local / unspecified ranges (and +/// their IPv4-mapped IPv6 forms), plus `localhost`, `.local` / `.internal` +/// names, and single-label hostnames (internal service names such as `mongo` +/// or `redis`). +fn is_blocked_host(host: &str) -> bool { + let host = host.trim().trim_end_matches('.').to_ascii_lowercase(); + if host.is_empty() { + return true; + } + if let Ok(ip) = host.parse::() { + // Use the same public-address classification as the resolved-address + // guard (and the IPv6 literal branch) so reserved/multicast/broadcast/ + // documentation/benchmarking literals are rejected too. A literal never + // goes through DNS resolution, so the `PublicOnlyResolver` never sees + // it — this text check is the only line of defense for it. + return !is_public_ipv4(ip); + } + if let Ok(ip) = host.parse::() { + // Use the same public-address classification as the resolved-address + // guard so an IPv4-mapped literal (`::ffff:127.0.0.1`, + // `::ffff:10.0.0.1`) is rejected like its bare IPv4 counterpart. A + // literal never goes through DNS resolution, so the `PublicOnlyResolver` + // never sees it — this text check is the only line of defense for it. + return !is_public_ipv6(ip); + } + if host == "localhost" || host.ends_with(".local") || host.ends_with(".internal") { + return true; + } + // A single-label name is an internal-service name, not a public domain. + !host.contains('.') +} + +fn is_private_ipv4(ip: std::net::Ipv4Addr) -> bool { + if ip.is_loopback() || ip.is_private() || ip.is_link_local() || ip.is_unspecified() { + return true; + } + let o = ip.octets(); + // 100.64.0.0/10 CGNAT and 192.0.0.0/24 (IETF protocol assignments). + (o[0] == 100 && o[1] & 0xc0 == 0x40) || (o[0] == 192 && o[1] == 0) +} + +fn is_private_ipv6(ip: std::net::Ipv6Addr) -> bool { + if ip.is_loopback() || ip.is_unspecified() { + return true; + } + let o = ip.octets(); + // Unique-local fc00::/7 and link-local fe80::/10. + (o[0] == 0xfc || o[0] == 0xfd) || (o[0] == 0xfe && o[1] & 0xc0 == 0x80) +} + +#[cfg(test)] +#[path = "ssrf_tests.rs"] +mod tests; diff --git a/sources/src/readers/ssrf_tests.rs b/sources/src/readers/ssrf_tests.rs new file mode 100644 index 0000000..4f8189a --- /dev/null +++ b/sources/src/readers/ssrf_tests.rs @@ -0,0 +1,165 @@ +use super::*; + +// ── SSRF guard ────────────────────────────────────────────────────── + +#[test] +fn is_url_allowed_accepts_public_http_urls() { + assert!(is_url_allowed( + &reqwest::Url::parse("https://example.com").unwrap() + )); + assert!(is_url_allowed( + &reqwest::Url::parse("http://example.com/x").unwrap() + )); + assert!(is_url_allowed( + &reqwest::Url::parse("https://sub.example.com").unwrap() + )); + assert!(is_url_allowed( + &reqwest::Url::parse("https://8.8.8.8").unwrap() + )); +} + +#[test] +fn is_url_allowed_rejects_private_and_internal_targets() { + // Private / loopback / link-local IP literals. + assert!(!is_url_allowed( + &reqwest::Url::parse("http://127.0.0.1").unwrap() + )); + assert!(!is_url_allowed( + &reqwest::Url::parse("http://10.0.0.1").unwrap() + )); + assert!(!is_url_allowed( + &reqwest::Url::parse("http://192.168.1.1").unwrap() + )); + assert!(!is_url_allowed( + &reqwest::Url::parse("http://169.254.169.254").unwrap() + )); + assert!(!is_url_allowed( + &reqwest::Url::parse("http://[::1]").unwrap() + )); + // Internal service names and local-only names. + assert!(!is_url_allowed( + &reqwest::Url::parse("http://localhost").unwrap() + )); + assert!(!is_url_allowed( + &reqwest::Url::parse("http://mongo").unwrap() + )); + assert!(!is_url_allowed( + &reqwest::Url::parse("http://service.internal").unwrap() + )); + // Non-http scheme. + assert!(!is_url_allowed( + &reqwest::Url::parse("ftp://example.com").unwrap() + )); + assert!(!is_url_allowed( + &reqwest::Url::parse("file:///etc/passwd").unwrap() + )); +} + +#[test] +fn is_blocked_host_rejects_ip_ranges_and_local_names() { + let blocked = [ + "127.0.0.1", + "0.0.0.0", + "10.0.0.1", + "172.16.0.1", + "192.168.0.1", + "169.254.169.254", + "100.64.0.1", // CGNAT + "192.0.0.1", // IETF protocol assignments + "localhost", + "foo.local", + "bar.internal", + "mongo", + "::1", + "fc00::1", // unique-local + "fe80::1", // link-local + "::ffff:127.0.0.1", // IPv4-mapped loopback literal + "::ffff:10.0.0.1", // IPv4-mapped private literal + "::ffff:169.254.169.254", // IPv4-mapped link-local / cloud metadata + // Special IPv4 literals that are not globally routable: multicast, + // broadcast, documentation, benchmarking, and reserved ranges. A + // literal never goes through DNS resolution, so the text check is the + // only line of defense for these. + "224.0.0.1", // multicast + "255.255.255.255", // broadcast + "192.0.2.1", // documentation + "198.51.100.1", // documentation + "203.0.113.1", // documentation + "198.18.0.1", // benchmarking + "240.0.0.1", // reserved + ]; + for host in blocked { + assert!(is_blocked_host(host), "expected {host:?} to be blocked"); + } +} + +#[test] +fn is_blocked_host_accepts_public_hosts() { + let allowed = [ + "8.8.8.8", + "1.1.1.1", + "example.com", + "sub.example.com", + "example.co.uk", + "8.8.8.8.", // trailing dot is normalized away + "EXAMPLE.com", // case-insensitive + "2001:4860:4860::8888", + "::ffff:8.8.8.8", // IPv4-mapped public literal + ]; + for host in allowed { + assert!(!is_blocked_host(host), "expected {host:?} to be allowed"); + } +} + +// ── resolved-address (DNS) SSRF classification ────────────────────── + +fn public_ip(s: &str) -> IpAddr { + s.parse().expect("valid ip literal") +} + +#[test] +fn is_public_ip_rejects_internal_and_special_ranges() { + let blocked = [ + "127.0.0.1", // loopback + "0.0.0.0", // unspecified + "10.0.0.1", // private + "172.16.0.1", // private + "192.168.1.1", // private + "169.254.169.254", // link-local / cloud metadata + "100.64.0.1", // CGNAT + "192.0.0.1", // IETF protocol assignments + "224.0.0.1", // multicast + "255.255.255.255", // broadcast + "192.0.2.1", // documentation + "198.51.100.1", // documentation + "203.0.113.1", // documentation + "198.18.0.1", // benchmarking + "240.0.0.1", // reserved + "::1", // loopback + "::", // unspecified + "fc00::1", // unique-local + "fe80::1", // link-local + "ff00::1", // multicast + "2001:db8::1", // documentation + "::ffff:127.0.0.1", // IPv4-mapped loopback + "::ffff:169.254.169.254", // IPv4-mapped link-local + ]; + for s in blocked { + assert!(!is_public_ip(public_ip(s)), "expected {s:?} to be rejected"); + } +} + +#[test] +fn is_public_ip_accepts_global_addresses() { + let allowed = [ + "8.8.8.8", + "1.1.1.1", + "93.184.216.34", + "2001:4860:4860::8888", + "2606:4700:4700::1111", + "::ffff:8.8.8.8", // IPv4-mapped public + ]; + for s in allowed { + assert!(is_public_ip(public_ip(s)), "expected {s:?} to be allowed"); + } +} diff --git a/sources/src/readers/web_page.rs b/sources/src/readers/web_page.rs new file mode 100644 index 0000000..4f6ff22 --- /dev/null +++ b/sources/src/readers/web_page.rs @@ -0,0 +1,485 @@ +//! Web page source reader. +//! +//! Fetches a single URL and extracts its text content. When a CSS +//! `selector` is configured, only matching elements are included; +//! otherwise the full page body is returned. +//! +//! The fetch-side SSRF guard (scheme/host policy plus a DNS resolver that +//! pins connections to globally routable addresses) lives in the shared +//! `ssrf` module, which the RSS reader uses too. + +mod types; + +use async_trait::async_trait; + +use super::ssrf::{build_client, is_url_allowed, read_body_capped}; +use types::SelectorSpec; + + +use crate::SourceResult; +use crate::types::{ + ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, +}; + +use super::{into_engine_error, SourceReader}; + +/// Reader for a single-page web source: fetches one URL and extracts its +/// readable text. +pub struct WebPageReader; + +#[async_trait] +impl SourceReader for WebPageReader { + fn kind(&self) -> SourceKind { + SourceKind::WebPage + } + + async fn list_items( + &self, + source: &MemorySourceEntry, + workspace: &std::path::Path, + ) -> SourceResult> { + self.list_items_inner(source, workspace) + .await + .map_err(into_engine_error) + } + + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + workspace: &std::path::Path, + ) -> SourceResult { + self.read_item_inner(source, item_id, workspace) + .await + .map_err(into_engine_error) + } +} + +impl WebPageReader { + async fn list_items_inner( + &self, + source: &MemorySourceEntry, + _workspace: &std::path::Path, + ) -> Result, String> { + let url = source + .url + .as_deref() + .ok_or("web_page source requires a url")?; + + Ok(vec![SourceItem { + id: url.to_string(), + title: source.label.clone(), + updated_at_ms: None, + }]) + } + + async fn read_item_inner( + &self, + source: &MemorySourceEntry, + item_id: &str, + _workspace: &std::path::Path, + ) -> Result { + let url = if item_id.starts_with("http") { + item_id.to_string() + } else { + source.url.clone().ok_or("web_page source requires a url")? + }; + + // SSRF guard: validate scheme and host, reject private/internal + // targets, and refuse redirects that would escape that policy. + let parsed = reqwest::Url::parse(&url).map_err(|e| format!("invalid URL: {e}"))?; + if !is_url_allowed(&parsed) { + return Err(format!( + "web_page source requires an http(s) URL to a public host, got: {}", + url.chars().take(64).collect::() + )); + } + + tracing::debug!( + host = %parsed.host_str().unwrap_or(""), + selector = ?source.selector, + "[memory_sources:web_page] reading item" + ); + + let client = build_client()?; + let resp = client + .get(parsed) + .header("User-Agent", "openhuman") + .send() + .await + .map_err(|e| format!("failed to fetch page: {e}"))?; + + if !resp.status().is_success() { + return Err(format!("page returned {}", resp.status())); + } + + // Cap response body to 10 MiB so a hostile/giant page can't OOM us. + // The read is streamed so the cap is enforced while downloading, not + // after the whole body has been buffered into memory. + const MAX_BODY_BYTES: u64 = 10 * 1024 * 1024; + let bytes = read_body_capped(resp, MAX_BODY_BYTES).await?; + let body = String::from_utf8_lossy(&bytes).into_owned(); + + let extracted = if let Some(selector) = source.selector.as_deref() { + extract_by_selector(&body, selector) + } else { + strip_html_tags(&body) + }; + + Ok(SourceContent { + id: url.clone(), + title: extract_title(&body).unwrap_or_else(|| url.clone()), + body: extracted, + content_type: ContentType::Plaintext, + metadata: serde_json::json!({ "url": url }), + }) + } +} + +// ── Text extraction ───────────────────────────────────────────────── + +fn extract_title(html: &str) -> Option { + let start = html.find("')? + start + 1; + let end = html[content_start..].find("")? + content_start; + Some(html[content_start..end].trim().to_string()) +} + +fn parse_selector(selector: &str) -> Option { + let last = selector + .trim() + .rsplit(char::is_whitespace) + .next() + .unwrap_or("") + .trim(); + if last.is_empty() { + return None; + } + + let mut spec = SelectorSpec { + tag: None, + id: None, + classes: Vec::new(), + }; + let mut part = String::new(); + let mut sep = ' '; // leading bare token is the tag + for ch in last.chars() { + match ch { + '.' | '#' => { + push_selector_part(&mut spec, &mut part, sep); + sep = ch; + } + _ => part.push(ch), + } + } + push_selector_part(&mut spec, &mut part, sep); + + if spec.tag.is_none() && spec.id.is_none() && spec.classes.is_empty() { + None + } else { + Some(spec) + } +} + +fn push_selector_part(spec: &mut SelectorSpec, part: &mut String, sep: char) { + let part = std::mem::take(part); + if part.is_empty() { + return; + } + match sep { + '#' => spec.id = Some(part), + '.' => spec.classes.push(part), + _ => { + if spec.tag.is_none() { + spec.tag = Some(part); + } else { + spec.classes.push(part); + } + } + } +} + +/// Extract text from elements matching a simple CSS selector. +/// +/// Falls back to the whole stripped page when the selector never matches +/// (rather than erroring), mirroring the reader's lenient posture for pages +/// whose structure changes between list and read time. +fn extract_by_selector(html: &str, selector: &str) -> String { + let Some(spec) = parse_selector(selector) else { + return strip_html_tags(html); + }; + // Match against a script/style-stripped copy so JS strings and CSS rules + // cannot be mistaken for nested elements, and so a selector that lands on + // a `

World

"; + let result = strip_html_tags(html); + assert_eq!(result, "Hello World"); +} + +#[test] +fn strip_script_and_style_handles_unclosed() { + // Unclosed `"; + let result = extract_by_selector(html, ".content"); + assert!(result.contains("Real")); + assert!(!result.contains("Fake")); +} diff --git a/sources/src/registry.rs b/sources/src/registry.rs new file mode 100644 index 0000000..3646f97 --- /dev/null +++ b/sources/src/registry.rs @@ -0,0 +1,355 @@ +//! The configured-source registry. +//! +//! Sources are persisted as `[[memory_sources]]` entries in a TOML config file +//! (typically `config.toml`). In OpenHuman this lived on a large shared `Config` +//! struct loaded through an async RPC; TinyCortex does not own that global +//! config, so the registry here is a small self-contained reader/writer over a +//! single TOML file. Other top-level keys in the file are preserved across +//! writes — only the `memory_sources` array is rewritten. +//! +//! Every mutation follows the spec's atomic load-modify-validate-save cycle: +//! load the current file, apply the change in memory, validate, and persist. +//! Each on-disk write (`SourceRegistry::atomic_write`) is atomic (temp file + +//! rename), so a crash mid-write cannot leave a truncated `config.toml`. +//! +//! The complete load-modify-save cycle is guarded by a process-wide mutation +//! lock, so separate [`SourceRegistry`] handles cannot overwrite one another's +//! in-process updates. Atomic rename protects each individual disk write. + +use std::path::{Path, PathBuf}; +use std::sync::{LazyLock, Mutex}; + +use anyhow::{anyhow, bail, Context, Result}; + +use super::types::{MemorySourceEntry, MemorySourcePatch, SourceKind}; + +/// Serializes each registry load-modify-save transaction in this process. +/// +/// A single lock deliberately covers every path: registry mutation is rare, +/// and correctness is more important than allowing unrelated config files to +/// race through their atomic renames. The on-disk rename remains the crash- +/// safety boundary; this mutex closes the in-process lost-update window. +static REGISTRY_MUTATION_LOCK: LazyLock> = LazyLock::new(|| Mutex::new(())); + +fn mutation_guard() -> std::sync::MutexGuard<'static, ()> { + // A poisoned lock means a previous mutation panicked while holding it. The + // guard's data is `()`, so there is nothing torn to inherit -- recover and + // continue rather than cascading the panic into every later mutation. + REGISTRY_MUTATION_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) +} + +/// Conservative default sync caps for a Composio toolkit, keyed by toolkit slug. +/// +/// Single source of truth for the cheap out-of-the-box sync volume. Applied to a +/// source entry when it is first registered. Never overwrites a user-customised +/// cap. Returns `(max_items, sync_depth_days)`. +pub fn memory_sync_defaults_for_toolkit(toolkit: &str) -> (Option, Option) { + match toolkit { + "gmail" => (Some(100), Some(30)), + "slack" => (Some(50), Some(14)), + "notion" => (Some(30), Some(30)), + "linear" => (Some(50), Some(30)), + "clickup" => (Some(50), Some(30)), + "github" => (Some(50), Some(30)), + // Generic fallback for any toolkit not listed above. + _ => (Some(30), Some(14)), + } +} + +/// A registry of [`MemorySourceEntry`] values backed by a TOML config file. +/// +/// Construct one with [`SourceRegistry::new`], pointing at the `config.toml` +/// path. The file need not exist yet — reads return an empty list and the first +/// write creates it (and any missing parent directories). +#[derive(Debug, Clone)] +pub struct SourceRegistry { + path: PathBuf, +} + +impl SourceRegistry { + /// Create a registry persisted at `config_path`. + pub fn new(config_path: impl Into) -> Self { + Self { + path: config_path.into(), + } + } + + /// The config file path this registry reads and writes. + pub fn path(&self) -> &Path { + &self.path + } + + /// Read the whole config file into a TOML table (empty if it doesn't exist). + fn read_table(&self) -> Result { + if !self.path.exists() { + return Ok(toml::Table::new()); + } + let text = std::fs::read_to_string(&self.path) + .with_context(|| format!("failed to read {}", self.path.display()))?; + let table: toml::Table = toml::from_str(&text) + .with_context(|| format!("failed to parse {}", self.path.display()))?; + Ok(table) + } + + /// List all configured sources. + pub fn list(&self) -> Result> { + let table = self.read_table()?; + match table.get("memory_sources") { + Some(value) => value + .clone() + .try_into() + .context("failed to decode [[memory_sources]]"), + None => Ok(Vec::new()), + } + } + + /// List enabled sources of a given [`SourceKind`]. + pub fn list_enabled_by_kind(&self, kind: SourceKind) -> Result> { + Ok(self + .list()? + .into_iter() + .filter(|s| s.kind == kind && s.enabled) + .collect()) + } + + /// Get a single source by id, if present. + pub fn get(&self, id: &str) -> Result> { + Ok(self.list()?.into_iter().find(|s| s.id == id)) + } + + /// Persist the full source list, preserving any other top-level config + /// keys. + /// + /// Writes are atomic: the new TOML is written to a same-directory temp file + /// and then renamed over the config. This keeps a failed/crashed write from + /// leaving a truncated `config.toml`, matching the OpenHuman source + /// registry contract. + /// + /// Mutation callers hold [`REGISTRY_MUTATION_LOCK`] across their initial + /// read and this preserving re-read, keeping the two snapshots ordered with + /// respect to every other in-process writer. + fn write_all(&self, entries: &[MemorySourceEntry]) -> Result<()> { + let mut table = self.read_table()?; + let value = toml::Value::try_from(entries).context("failed to encode memory_sources")?; + table.insert("memory_sources".to_string(), value); + let text = toml::to_string_pretty(&table).context("failed to serialize config")?; + if let Some(parent) = self.path.parent() { + if !parent.as_os_str().is_empty() { + std::fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + } + self.atomic_write(text.as_bytes())?; + Ok(()) + } + + fn atomic_write(&self, bytes: &[u8]) -> Result<()> { + let parent = self + .path + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let filename = self + .path + .file_name() + .and_then(|n| n.to_str()) + .ok_or_else(|| anyhow!("config path has no file name: {}", self.path.display()))?; + let tmp_path = parent.join(format!( + ".{filename}.tmp-{}", + uuid::Uuid::new_v4().as_simple() + )); + + let write_result = (|| -> Result<()> { + { + let mut file = std::fs::File::create(&tmp_path) + .with_context(|| format!("failed to create {}", tmp_path.display()))?; + use std::io::Write; + file.write_all(bytes) + .with_context(|| format!("failed to write {}", tmp_path.display()))?; + file.sync_all() + .with_context(|| format!("failed to sync {}", tmp_path.display()))?; + } + std::fs::rename(&tmp_path, &self.path).with_context(|| { + format!( + "failed to atomically replace {} with {}", + self.path.display(), + tmp_path.display() + ) + })?; + Ok(()) + })(); + + if write_result.is_err() { + let _ = std::fs::remove_file(&tmp_path); + } + write_result + } + + /// Validate and add a new source. Fails if the id already exists. + pub fn add(&self, entry: MemorySourceEntry) -> Result { + let _guard = mutation_guard(); + entry.validate().map_err(|e| anyhow!(e))?; + let mut sources = self.list()?; + if sources.iter().any(|s| s.id == entry.id) { + bail!("source with id '{}' already exists", entry.id); + } + sources.push(entry.clone()); + self.write_all(&sources)?; + Ok(entry) + } + + /// Apply a [`MemorySourcePatch`] to an existing source, then re-validate and + /// save. Fails if no source has the given id. + pub fn update(&self, id: &str, patch: MemorySourcePatch) -> Result { + let _guard = mutation_guard(); + let mut sources = self.list()?; + let entry = sources + .iter_mut() + .find(|s| s.id == id) + .ok_or_else(|| anyhow!("source '{id}' not found"))?; + + patch.validate_for_kind(entry.kind.clone())?; + patch.apply_to(entry); + entry.validate().map_err(|e| anyhow!(e))?; + let updated = entry.clone(); + self.write_all(&sources)?; + Ok(updated) + } + + /// Remove a source by id. Returns `true` if an entry was removed. + pub fn remove(&self, id: &str) -> Result { + let _guard = mutation_guard(); + let mut sources = self.list()?; + let before = sources.len(); + sources.retain(|s| s.id != id); + let removed = sources.len() < before; + if removed { + self.write_all(&sources)?; + } + Ok(removed) + } + + /// Remove every composio source bound to `connection_id`. Returns the count + /// removed. Mirrors [`SourceRegistry::upsert_composio_source`], which keys + /// composio sources on `connection_id` rather than the `src_*` id. + pub fn remove_composio_source_by_connection_id(&self, connection_id: &str) -> Result { + let _guard = mutation_guard(); + let mut sources = self.list()?; + let before = sources.len(); + sources.retain(|s| { + !(s.kind == SourceKind::Composio && s.connection_id.as_deref() == Some(connection_id)) + }); + let removed = before - sources.len(); + if removed > 0 { + self.write_all(&sources)?; + } + Ok(removed) + } + + /// Upsert a composio source keyed on `connection_id`. + /// + /// If a source with the same `connection_id` exists, its label is updated; + /// otherwise a new entry is inserted with conservative per-toolkit caps. The + /// update path never clobbers user-customised caps. + pub fn upsert_composio_source( + &self, + toolkit: &str, + connection_id: &str, + label: &str, + ) -> Result { + let _guard = mutation_guard(); + let mut sources = self.list()?; + let (entry, _was_insert) = + upsert_composio_entry_in_place(&mut sources, toolkit, connection_id, label); + self.write_all(&sources)?; + Ok(entry) + } + + /// Batch-upsert Composio sources with one load and one atomic save. + pub fn upsert_composio_sources_batch(&self, targets: &[ComposioUpsertTarget]) -> Result { + if targets.is_empty() { + return Ok(0); + } + let _guard = mutation_guard(); + let mut sources = self.list()?; + for (toolkit, connection_id, label) in targets { + upsert_composio_entry_in_place(&mut sources, toolkit, connection_id, label); + } + self.write_all(&sources)?; + Ok(targets.len().min(u32::MAX as usize) as u32) + } + + /// Enable every source and clear all per-source caps ("All In" mode). + pub fn apply_all_in(&self) -> Result> { + let _guard = mutation_guard(); + let mut sources = self.list()?; + for source in &mut sources { + source.enabled = true; + source.max_items = None; + source.since_days = None; + source.sync_depth_days = None; + source.max_commits = None; + source.max_issues = None; + source.max_prs = None; + source.max_tokens_per_sync = None; + source.max_cost_per_sync_usd = None; + } + self.write_all(&sources)?; + Ok(sources) + } +} + +/// `(toolkit, account_id, label)` — the three fields that identify which +/// Composio account a source upserts into. +pub type ComposioUpsertTarget = (String, String, String); + +/// Apply a single composio upsert to an in-memory source list. +/// +/// Pure (no I/O) so the registry path and unit tests share one find-or-push +/// predicate. Returns the resulting entry and whether it was a fresh insert. +pub(crate) fn upsert_composio_entry_in_place( + sources: &mut Vec, + toolkit: &str, + connection_id: &str, + label: &str, +) -> (MemorySourceEntry, bool) { + if let Some(existing) = sources.iter_mut().find(|s| { + s.kind == SourceKind::Composio && s.connection_id.as_deref() == Some(connection_id) + }) { + existing.label = label.to_string(); + return (existing.clone(), false); + } + + let (default_max_items, default_sync_depth_days) = memory_sync_defaults_for_toolkit(toolkit); + let entry = MemorySourceEntry { + id: format!("src_{}", uuid::Uuid::new_v4().as_simple()), + kind: SourceKind::Composio, + label: label.to_string(), + enabled: true, + toolkit: Some(toolkit.to_string()), + connection_id: Some(connection_id.to_string()), + path: None, + glob: None, + url: None, + branch: None, + paths: Vec::new(), + max_commits: None, + max_issues: None, + max_prs: None, + query: None, + since_days: None, + max_items: default_max_items, + selector: None, + max_tokens_per_sync: None, + max_cost_per_sync_usd: None, + sync_depth_days: default_sync_depth_days, + }; + sources.push(entry.clone()); + (entry, true) +} + +#[cfg(test)] +#[path = "registry_tests.rs"] +mod tests; diff --git a/sources/src/registry_tests.rs b/sources/src/registry_tests.rs new file mode 100644 index 0000000..22b40a4 --- /dev/null +++ b/sources/src/registry_tests.rs @@ -0,0 +1,311 @@ +//! Tests for the TOML-backed source registry. + +use super::*; +use crate::types::SourceKind; +use tempfile::TempDir; + +fn registry() -> (TempDir, SourceRegistry) { + let tmp = TempDir::new().unwrap(); + let reg = SourceRegistry::new(tmp.path().join("config.toml")); + (tmp, reg) +} + +fn folder_entry(id: &str) -> MemorySourceEntry { + let mut e = MemorySourceEntry { + id: id.into(), + kind: SourceKind::Folder, + label: "Notes".into(), + enabled: true, + toolkit: None, + connection_id: None, + path: Some("/tmp/notes".into()), + glob: None, + url: None, + branch: None, + paths: Vec::new(), + max_commits: None, + max_issues: None, + max_prs: None, + query: None, + since_days: None, + max_items: None, + selector: None, + max_tokens_per_sync: None, + max_cost_per_sync_usd: None, + sync_depth_days: None, + }; + e.glob = Some("**/*.md".into()); + e +} + +#[test] +fn list_is_empty_for_missing_file() { + let (_tmp, reg) = registry(); + assert!(reg.list().unwrap().is_empty()); + assert!(reg.get("anything").unwrap().is_none()); +} + +#[test] +fn add_get_list_round_trip() { + let (_tmp, reg) = registry(); + let added = reg.add(folder_entry("src_1")).unwrap(); + assert_eq!(added.id, "src_1"); + + let got = reg.get("src_1").unwrap().unwrap(); + assert_eq!(got.kind, SourceKind::Folder); + assert_eq!(got.path.as_deref(), Some("/tmp/notes")); + + let all = reg.list().unwrap(); + assert_eq!(all.len(), 1); +} + +#[test] +fn add_rejects_duplicate_id() { + let (_tmp, reg) = registry(); + reg.add(folder_entry("src_dup")).unwrap(); + assert!(reg.add(folder_entry("src_dup")).is_err()); +} + +#[test] +fn add_rejects_invalid_entry() { + let (_tmp, reg) = registry(); + let mut bad = folder_entry("src_bad"); + bad.path = None; // folder requires a path + assert!(reg.add(bad).is_err()); + assert!(reg.list().unwrap().is_empty()); +} + +#[test] +fn concurrent_registry_adds_preserve_every_source() { + let (_tmp, reg) = registry(); + let writers = 24; + let barrier = std::sync::Arc::new(std::sync::Barrier::new(writers)); + let mut threads = Vec::new(); + for index in 0..writers { + let reg = reg.clone(); + let barrier = barrier.clone(); + threads.push(std::thread::spawn(move || { + barrier.wait(); + reg.add(folder_entry(&format!("src_concurrent_{index}"))) + .unwrap(); + })); + } + for thread in threads { + thread.join().unwrap(); + } + + let mut ids: Vec<_> = reg + .list() + .unwrap() + .into_iter() + .map(|entry| entry.id) + .collect(); + ids.sort(); + assert_eq!(ids.len(), writers); + for index in 0..writers { + assert!(ids.contains(&format!("src_concurrent_{index}"))); + } +} + +#[test] +fn update_applies_patch_and_persists() { + let (_tmp, reg) = registry(); + reg.add(folder_entry("src_u")).unwrap(); + + let patch = MemorySourcePatch { + label: Some("Renamed".into()), + enabled: Some(false), + ..Default::default() + }; + let updated = reg.update("src_u", patch).unwrap(); + assert_eq!(updated.label, "Renamed"); + assert!(!updated.enabled); + + // Re-read from disk to confirm persistence. + let got = reg.get("src_u").unwrap().unwrap(); + assert_eq!(got.label, "Renamed"); + assert!(!got.enabled); +} + +#[test] +fn update_missing_id_errors() { + let (_tmp, reg) = registry(); + assert!(reg.update("nope", MemorySourcePatch::default()).is_err()); +} + +#[test] +fn remove_returns_whether_anything_was_removed() { + let (_tmp, reg) = registry(); + reg.add(folder_entry("src_r")).unwrap(); + assert!(reg.remove("src_r").unwrap()); + assert!(!reg.remove("src_r").unwrap()); + assert!(reg.list().unwrap().is_empty()); +} + +#[test] +fn list_enabled_by_kind_filters() { + let (_tmp, reg) = registry(); + reg.add(folder_entry("src_a")).unwrap(); + let mut disabled = folder_entry("src_b"); + disabled.enabled = false; + reg.add(disabled).unwrap(); + + let enabled = reg.list_enabled_by_kind(SourceKind::Folder).unwrap(); + assert_eq!(enabled.len(), 1); + assert_eq!(enabled[0].id, "src_a"); + assert!(reg + .list_enabled_by_kind(SourceKind::Conversation) + .unwrap() + .is_empty()); +} + +#[test] +fn write_preserves_other_top_level_config_keys() { + let (tmp, reg) = registry(); + let path = tmp.path().join("config.toml"); + std::fs::write(&path, "workspace = \"/data\"\n").unwrap(); + + reg.add(folder_entry("src_keep")).unwrap(); + + let text = std::fs::read_to_string(&path).unwrap(); + assert!(text.contains("workspace = \"/data\"")); + assert!(text.contains("[[memory_sources]]")); +} + +#[test] +fn write_uses_atomic_temp_file_without_leaving_stale_temp() { + let (tmp, reg) = registry(); + reg.add(folder_entry("src_atomic")).unwrap(); + + let text = std::fs::read_to_string(reg.path()).unwrap(); + assert!(text.contains("src_atomic")); + + let stale_temp_files: Vec<_> = std::fs::read_dir(tmp.path()) + .unwrap() + .filter_map(Result::ok) + .filter(|entry| { + entry + .file_name() + .to_string_lossy() + .starts_with(".config.toml.tmp-") + }) + .collect(); + assert!(stale_temp_files.is_empty()); +} + +// ── Composio upsert ── + +#[test] +fn composio_defaults_for_known_and_unknown_toolkits() { + assert_eq!( + memory_sync_defaults_for_toolkit("gmail"), + (Some(100), Some(30)) + ); + assert_eq!( + memory_sync_defaults_for_toolkit("slack"), + (Some(50), Some(14)) + ); + assert_eq!( + memory_sync_defaults_for_toolkit("unknown_xyz"), + (Some(30), Some(14)) + ); +} + +#[test] +fn in_place_upsert_inserts_then_updates_label_only() { + let mut sources: Vec = vec![]; + let (entry, was_insert) = + upsert_composio_entry_in_place(&mut sources, "gmail", "conn_a", "Gmail · conn_a"); + assert!(was_insert); + assert_eq!(entry.toolkit.as_deref(), Some("gmail")); + assert_eq!(entry.max_items, Some(100)); + assert_eq!(entry.sync_depth_days, Some(30)); + + // User customises a cap, then a second upsert updates label only. + sources[0].max_items = Some(7); + let (entry, was_insert) = + upsert_composio_entry_in_place(&mut sources, "gmail", "conn_a", "new label"); + assert!(!was_insert); + assert_eq!(sources.len(), 1); + assert_eq!(entry.label, "new label"); + assert_eq!(entry.max_items, Some(7)); +} + +#[test] +fn upsert_composio_source_persists_and_disconnect_removes() { + let (_tmp, reg) = registry(); + reg.upsert_composio_source("gmail", "conn_a", "Gmail") + .unwrap(); + reg.upsert_composio_source("slack", "conn_b", "Slack") + .unwrap(); + assert_eq!(reg.list().unwrap().len(), 2); + + let removed = reg + .remove_composio_source_by_connection_id("conn_a") + .unwrap(); + assert_eq!(removed, 1); + assert_eq!(reg.list().unwrap().len(), 1); +} + +#[test] +fn apply_all_in_enables_and_clears_caps() { + let (_tmp, reg) = registry(); + let mut capped = folder_entry("src_capped"); + capped.enabled = false; + capped.max_items = Some(5); + capped.sync_depth_days = Some(3); + reg.add(capped).unwrap(); + + let updated = reg.apply_all_in().unwrap(); + assert_eq!(updated.len(), 1); + assert!(updated[0].enabled); + assert!(updated[0].max_items.is_none()); + assert!(updated[0].sync_depth_days.is_none()); +} + +#[test] +fn memory_source_patch_deserializes_partial_and_github_fields() { + let json = serde_json::json!({ + "label": "New label", + "enabled": false, + "max_commits": 100, + "max_issues": 50, + "max_prs": 25 + }); + let patch: MemorySourcePatch = serde_json::from_value(json).unwrap(); + assert_eq!(patch.label.as_deref(), Some("New label")); + assert_eq!(patch.enabled, Some(false)); + assert_eq!(patch.max_commits, Some(Some(100))); + assert_eq!(patch.max_issues, Some(Some(50))); + assert_eq!(patch.max_prs, Some(Some(25))); + assert!(patch.toolkit.is_none()); +} + +#[test] +fn memory_source_patch_can_clear_optional_fields_with_null() { + let (_tmp, reg) = registry(); + let mut entry = folder_entry("src_clear"); + entry.glob = Some("**/*.md".into()); + entry.max_items = Some(10); + reg.add(entry).unwrap(); + + let patch: MemorySourcePatch = serde_json::from_value(serde_json::json!({ + "glob": null, + "max_items": null + })) + .unwrap(); + let updated = reg.update("src_clear", patch).unwrap(); + assert!(updated.glob.is_none()); + assert!(updated.max_items.is_none()); +} + +#[test] +fn update_rejects_fields_that_do_not_apply_to_source_kind() { + let (_tmp, reg) = registry(); + reg.add(folder_entry("src_kind")).unwrap(); + let patch: MemorySourcePatch = serde_json::from_value(serde_json::json!({ + "url": "https://example.com/repo" + })) + .unwrap(); + assert!(reg.update("src_kind", patch).is_err()); +} diff --git a/sources/src/types.rs b/sources/src/types.rs new file mode 100644 index 0000000..52abba9 --- /dev/null +++ b/sources/src/types.rs @@ -0,0 +1,403 @@ +//! Core types for memory sources. +//! +//! A *memory source* answers the question "what feeds my memory?". Each +//! configured source is a [`MemorySourceEntry`] persisted in `config.toml` +//! under `[[memory_sources]]`. The [`SourceKind`] discriminator selects which +//! kind-specific fields are required; required-field checks live in +//! [`crate::validation`] and are surfaced via +//! [`MemorySourceEntry::validate`]. +//! +//! Reader output contracts ([`SourceItem`], [`SourceContent`], [`ContentType`]) +//! are shared across every reader implementation so the host can ingest source +//! payloads uniformly regardless of where they came from. +//! +//! Wire strings are snake_case and are part of the persisted contract — do not +//! rename them when porting from OpenHuman. + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +pub(crate) fn default_true() -> bool { + true +} + +/// The kind of a configured memory source. +/// +/// The wire representation is snake_case (`github_repo`, `rss_feed`, …) and is +/// persisted in `config.toml`; it must stay stable across versions. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum SourceKind { + /// A Composio OAuth connector (Gmail, Slack, Notion, …). Network-backed; + /// the live fetch is owned by the host, not TinyCortex. + Composio, + /// Local agent conversation transcripts stored in the workspace. + Conversation, + /// A local folder of files matched by an optional glob. + Folder, + /// A GitHub repository's project activity (commits, issues, PRs). + GithubRepo, + /// A Twitter/X search query. + TwitterQuery, + /// An RSS/Atom feed. + RssFeed, + /// A single web page, optionally narrowed by a CSS selector. + WebPage, +} + +impl SourceKind { + /// The stable snake_case wire string for this kind. + pub fn as_str(&self) -> &'static str { + match self { + SourceKind::Composio => "composio", + SourceKind::Conversation => "conversation", + SourceKind::Folder => "folder", + SourceKind::GithubRepo => "github_repo", + SourceKind::TwitterQuery => "twitter_query", + SourceKind::RssFeed => "rss_feed", + SourceKind::WebPage => "web_page", + } + } +} + +/// A configured memory source entry persisted in `config.toml`. +/// +/// All kind-specific fields are flattened onto the struct as `Option`s. The +/// [`kind`](MemorySourceEntry::kind) discriminator determines which fields are +/// required; validation is enforced at add/update time via +/// [`MemorySourceEntry::validate`]. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +pub struct MemorySourceEntry { + /// Stable unique id (e.g. `src_`). + pub id: String, + /// Discriminator selecting the kind-specific fields below. + pub kind: SourceKind, + /// Human-readable label shown in UIs. + pub label: String, + /// Whether this source participates in sync. Defaults to `true`. + #[serde(default = "default_true")] + pub enabled: bool, + + // ── Composio ── + /// Composio toolkit slug (e.g. `gmail`). Required for `composio`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub toolkit: Option, + /// Composio connection id. Required for `composio`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub connection_id: Option, + + // ── Folder ── + /// Filesystem path of the folder to read. Required for `folder`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub path: Option, + /// Optional glob applied under `path` (defaults to `**/*.md`). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub glob: Option, + + // ── GithubRepo / RssFeed / WebPage (shared) ── + /// Source URL. Required for `github_repo`, `rss_feed`, and `web_page`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub url: Option, + + // ── GithubRepo ── + /// Branch to read (defaults to the repo default when absent). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub branch: Option, + /// Optional path filters within the repo. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub paths: Vec, + /// Max commits to pull per sync (default 1000 when absent). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_commits: Option, + /// Max issues to pull per sync (default 1000 when absent). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_issues: Option, + /// Max pull requests to pull per sync (default 1000 when absent). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_prs: Option, + + // ── TwitterQuery ── + /// Search query. Required for `twitter_query`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub query: Option, + /// Optional look-back window in days for the query. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub since_days: Option, + + // ── RssFeed ── + /// Max feed items to pull per sync. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_items: Option, + + // ── WebPage ── + /// Optional CSS selector to narrow extracted content. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub selector: Option, + + // ── Sync Budget (all source kinds) ── + /// Maximum tokens to consume per sync run. Sync stops once this budget is hit. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_tokens_per_sync: Option, + /// Maximum cost in USD per sync run. Refuses LLM calls once reached. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub max_cost_per_sync_usd: Option, + /// Sync depth in days — only fetch items from the last N days. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sync_depth_days: Option, +} + +impl MemorySourceEntry { + /// Validate required fields for this entry's [`SourceKind`]. + /// + /// Delegates to [`crate::validation::validate_entry`]. + /// Returns a human-readable error message on the first failing rule. + pub fn validate(&self) -> Result<(), String> { + crate::validation::validate_entry(self) + } +} + +fn deserialize_double_option<'de, D, T>(deserializer: D) -> Result>, D::Error> +where + D: serde::Deserializer<'de>, + T: serde::Deserialize<'de>, +{ + as serde::Deserialize>::deserialize(deserializer).map(Some) +} + +/// Partial update payload for a source entry. +/// +/// An absent field leaves the current value unchanged. For optional source +/// properties, an explicit JSON `null` clears the value while a concrete value +/// replaces it. +#[derive(Debug, Default, Deserialize)] +pub struct MemorySourcePatch { + /// New human-readable label for the source. + #[serde(default)] + pub label: Option, + /// Toggle whether the source participates in sync. + #[serde(default)] + pub enabled: Option, + /// Composio toolkit slug (e.g. `gmail`, `slack`). + #[serde(default, deserialize_with = "deserialize_double_option")] + pub toolkit: Option>, + /// Composio connection id this source binds to. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub connection_id: Option>, + /// Filesystem root for a local-files source. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub path: Option>, + /// Glob filter applied under [`MemorySourcePatch::path`]. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub glob: Option>, + /// Remote URL for a git/web source. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub url: Option>, + /// Git branch to track. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub branch: Option>, + /// Explicit path allowlist within a repo source. + #[serde(default)] + pub paths: Option>, + /// Search/filter query string for query-driven sources. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub query: Option>, + /// Lookback window in days for items to ingest. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub since_days: Option>, + /// Cap on the number of items pulled per sync. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub max_items: Option>, + /// Source-specific selector (e.g. a CSS selector for a web page). + #[serde(default, deserialize_with = "deserialize_double_option")] + pub selector: Option>, + /// Token budget per sync run. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub max_tokens_per_sync: Option>, + /// Cost budget per sync run, in USD. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub max_cost_per_sync_usd: Option>, + /// History depth in days for tree/summary backfill. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub sync_depth_days: Option>, + /// Cap on commits ingested from a git source. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub max_commits: Option>, + /// Cap on issues ingested from a repo source. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub max_issues: Option>, + /// Cap on pull requests ingested from a repo source. + #[serde(default, deserialize_with = "deserialize_double_option")] + pub max_prs: Option>, +} + +impl MemorySourcePatch { + /// Reject fields that do not apply to `kind`. + /// + /// A patch is a partial update, so a caller can set a field the source's + /// kind has no use for — a git branch on an RSS feed. Catching that here + /// keeps a nonsensical value out of the registry rather than letting the + /// reader discover it later. + /// + /// # Errors + /// + /// Returns the first inapplicable field, named. + pub fn validate_for_kind(&self, kind: SourceKind) -> anyhow::Result<()> { + let reject = |field: &str| { + Err(anyhow::anyhow!( + "field '{field}' is not applicable to source kind '{}'", + kind.as_str() + )) + }; + if (self.toolkit.is_some() || self.connection_id.is_some()) && kind != SourceKind::Composio + { + return reject("toolkit/connection_id"); + } + if (self.path.is_some() || self.glob.is_some()) && kind != SourceKind::Folder { + return reject("path/glob"); + } + if (self.branch.is_some() + || self.paths.is_some() + || self.max_commits.is_some() + || self.max_issues.is_some() + || self.max_prs.is_some()) + && kind != SourceKind::GithubRepo + { + return reject("github repository fields"); + } + if self.query.is_some() && kind != SourceKind::TwitterQuery { + return reject("query"); + } + if matches!(self.since_days, Some(Some(_))) && kind != SourceKind::TwitterQuery { + return reject("since_days"); + } + if self.selector.is_some() && kind != SourceKind::WebPage { + return reject("selector"); + } + // `max_items` is the per-run ingest cap. It applies to RSS feeds and to + // Composio connections — the host UI (`SourceSettingsPanel`) exposes it + // for both, and a Composio source is created with a toolkit default, so + // rejecting it on edit desynced the UI from the store. Other kinds have + // no per-run item cap. + if matches!(self.max_items, Some(Some(_))) + && !matches!(kind, SourceKind::RssFeed | SourceKind::Composio) + { + return reject("max_items"); + } + if self.url.is_some() + && kind != SourceKind::GithubRepo + && kind != SourceKind::RssFeed + && kind != SourceKind::WebPage + { + return reject("url"); + } + Ok(()) + } + + /// Apply each present field of this patch onto `entry` in place. + pub fn apply_to(self, entry: &mut MemorySourceEntry) { + if let Some(value) = self.label { + entry.label = value; + } + if let Some(value) = self.enabled { + entry.enabled = value; + } + if let Some(value) = self.toolkit { + entry.toolkit = value; + } + if let Some(value) = self.connection_id { + entry.connection_id = value; + } + if let Some(value) = self.path { + entry.path = value; + } + if let Some(value) = self.glob { + entry.glob = value; + } + if let Some(value) = self.url { + entry.url = value; + } + if let Some(value) = self.branch { + entry.branch = value; + } + if let Some(value) = self.paths { + entry.paths = value; + } + if let Some(value) = self.query { + entry.query = value; + } + if let Some(value) = self.since_days { + entry.since_days = value; + } + if let Some(value) = self.max_items { + entry.max_items = value; + } + if let Some(value) = self.selector { + entry.selector = value; + } + if let Some(value) = self.max_tokens_per_sync { + entry.max_tokens_per_sync = value; + } + if let Some(value) = self.max_cost_per_sync_usd { + entry.max_cost_per_sync_usd = value; + } + if let Some(value) = self.sync_depth_days { + entry.sync_depth_days = value; + } + if let Some(value) = self.max_commits { + entry.max_commits = value; + } + if let Some(value) = self.max_issues { + entry.max_issues = value; + } + if let Some(value) = self.max_prs { + entry.max_prs = value; + } + } +} + +/// One item listed from a source reader. +/// +/// `id` is reader-scoped (e.g. a folder-relative path or a thread id) and is +/// stable enough to pass back into `SourceReader::read_item`. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +pub struct SourceItem { + /// Reader-scoped item id. + pub id: String, + /// Human-readable title. + pub title: String, + /// Last-modified time in epoch milliseconds, when known. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub updated_at_ms: Option, +} + +/// The rendered content type of a [`SourceContent`] body. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum ContentType { + /// Markdown body. + Markdown, + /// Raw HTML body. + Html, + /// Plain text body. + Plaintext, +} + +/// Content read from a single source item. +#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +pub struct SourceContent { + /// Reader-scoped item id (matches the [`SourceItem::id`] it was read from). + pub id: String, + /// Human-readable title. + pub title: String, + /// The item body, rendered as [`content_type`](SourceContent::content_type). + pub body: String, + /// How [`body`](SourceContent::body) should be interpreted. + pub content_type: ContentType, + /// Reader-specific metadata (JSON object). + #[serde(default)] + pub metadata: serde_json::Value, +} + +#[cfg(test)] +#[path = "types_tests.rs"] +mod tests; diff --git a/sources/src/types_tests.rs b/sources/src/types_tests.rs new file mode 100644 index 0000000..ed9f5ec --- /dev/null +++ b/sources/src/types_tests.rs @@ -0,0 +1,261 @@ +//! Tests for source type contracts, serde wire strings, and validation. + +use super::*; + +#[test] +fn source_kind_round_trips_via_serde() { + for kind in [ + SourceKind::Composio, + SourceKind::Conversation, + SourceKind::Folder, + SourceKind::GithubRepo, + SourceKind::TwitterQuery, + SourceKind::RssFeed, + SourceKind::WebPage, + ] { + let json = serde_json::to_string(&kind).unwrap(); + let decoded: SourceKind = serde_json::from_str(&json).unwrap(); + assert_eq!(decoded, kind); + } +} + +#[test] +fn source_kind_as_str_matches_wire_strings() { + assert_eq!(SourceKind::Composio.as_str(), "composio"); + assert_eq!(SourceKind::Conversation.as_str(), "conversation"); + assert_eq!(SourceKind::Folder.as_str(), "folder"); + assert_eq!(SourceKind::GithubRepo.as_str(), "github_repo"); + assert_eq!(SourceKind::TwitterQuery.as_str(), "twitter_query"); + assert_eq!(SourceKind::RssFeed.as_str(), "rss_feed"); + assert_eq!(SourceKind::WebPage.as_str(), "web_page"); +} + +#[test] +fn validate_composio_requires_toolkit_and_connection_id() { + let entry = MemorySourceEntry { + id: "src_1".into(), + kind: SourceKind::Composio, + label: "Gmail".into(), + enabled: true, + toolkit: Some("gmail".into()), + connection_id: None, + ..default_entry() + }; + assert!(entry.validate().is_err()); + + let valid = MemorySourceEntry { + connection_id: Some("cmp_123".into()), + ..entry + }; + assert!(valid.validate().is_ok()); +} + +#[test] +fn validate_folder_requires_path() { + let entry = MemorySourceEntry { + id: "src_2".into(), + kind: SourceKind::Folder, + label: "Notes".into(), + enabled: true, + path: None, + ..default_entry() + }; + assert!(entry.validate().is_err()); +} + +#[test] +fn validate_github_requires_url() { + let entry = MemorySourceEntry { + id: "src_3".into(), + kind: SourceKind::GithubRepo, + label: "Repo".into(), + enabled: true, + url: Some("https://github.com/org/repo".into()), + ..default_entry() + }; + assert!(entry.validate().is_ok()); +} + +#[test] +fn validate_twitter_requires_query() { + let entry = MemorySourceEntry { + id: "src_tw".into(), + kind: SourceKind::TwitterQuery, + label: "Tweets".into(), + enabled: true, + query: None, + ..default_entry() + }; + assert!(entry.validate().is_err()); +} + +#[test] +fn validate_rss_and_web_page_require_url() { + let rss = MemorySourceEntry { + id: "src_rss".into(), + kind: SourceKind::RssFeed, + label: "Feed".into(), + enabled: true, + url: None, + ..default_entry() + }; + assert!(rss.validate().is_err()); + + let web = MemorySourceEntry { + id: "src_web".into(), + kind: SourceKind::WebPage, + label: "Page".into(), + enabled: true, + url: Some("https://example.com".into()), + ..default_entry() + }; + assert!(web.validate().is_ok()); +} + +#[test] +fn validate_conversation_needs_only_id_and_label() { + let entry = MemorySourceEntry { + id: "src_conv".into(), + kind: SourceKind::Conversation, + label: "Agent Conversations".into(), + enabled: true, + ..default_entry() + }; + assert!(entry.validate().is_ok()); +} + +#[test] +fn validate_conversation_fails_with_empty_id() { + let entry = MemorySourceEntry { + id: "".into(), + kind: SourceKind::Conversation, + label: "Convos".into(), + enabled: true, + ..default_entry() + }; + assert!(entry.validate().is_err()); +} + +#[test] +fn validate_conversation_fails_with_empty_label() { + let entry = MemorySourceEntry { + id: "src_conv".into(), + kind: SourceKind::Conversation, + label: "".into(), + enabled: true, + ..default_entry() + }; + assert!(entry.validate().is_err()); +} + +#[test] +fn conversation_kind_serializes_to_snake_case() { + let json = serde_json::to_string(&SourceKind::Conversation).unwrap(); + assert_eq!(json, "\"conversation\""); +} + +#[test] +fn content_type_serializes_to_snake_case() { + assert_eq!( + serde_json::to_string(&ContentType::Markdown).unwrap(), + "\"markdown\"" + ); + assert_eq!( + serde_json::to_string(&ContentType::Html).unwrap(), + "\"html\"" + ); + assert_eq!( + serde_json::to_string(&ContentType::Plaintext).unwrap(), + "\"plaintext\"" + ); +} + +#[test] +fn toml_round_trip() { + let entry = MemorySourceEntry { + id: "src_1".into(), + kind: SourceKind::Folder, + label: "My notes".into(), + enabled: true, + path: Some("/tmp/notes".into()), + glob: Some("**/*.md".into()), + ..default_entry() + }; + let toml_str = toml::to_string_pretty(&entry).unwrap(); + let decoded: MemorySourceEntry = toml::from_str(&toml_str).unwrap(); + assert_eq!(decoded.id, "src_1"); + assert_eq!(decoded.kind, SourceKind::Folder); + assert_eq!(decoded.path.as_deref(), Some("/tmp/notes")); +} + +#[test] +fn conversation_toml_round_trip() { + let entry = MemorySourceEntry { + id: "src_conv".into(), + kind: SourceKind::Conversation, + label: "Conversations".into(), + enabled: true, + ..default_entry() + }; + let toml_str = toml::to_string_pretty(&entry).unwrap(); + let decoded: MemorySourceEntry = toml::from_str(&toml_str).unwrap(); + assert_eq!(decoded.id, "src_conv"); + assert_eq!(decoded.kind, SourceKind::Conversation); + assert_eq!(decoded.label, "Conversations"); + assert!(decoded.enabled); +} + +#[test] +fn enabled_defaults_to_true_when_absent() { + let toml_str = r#" +id = "src_x" +kind = "conversation" +label = "Convos" +"#; + let decoded: MemorySourceEntry = toml::from_str(toml_str).unwrap(); + assert!(decoded.enabled); +} + +/// A fully-`None` entry used as a `..default_entry()` base in the tests above. +pub(super) fn default_entry() -> MemorySourceEntry { + MemorySourceEntry { + id: String::new(), + kind: SourceKind::Folder, + label: String::new(), + enabled: true, + toolkit: None, + connection_id: None, + path: None, + glob: None, + url: None, + branch: None, + paths: Vec::new(), + max_commits: None, + max_issues: None, + max_prs: None, + query: None, + since_days: None, + max_items: None, + selector: None, + max_tokens_per_sync: None, + max_cost_per_sync_usd: None, + sync_depth_days: None, + } +} + +#[test] +fn max_items_is_applicable_to_composio_and_rss_but_not_other_kinds() { + // The host UI exposes `max_items` for Composio sources and creates them with + // a toolkit default, so editing one must not be rejected — the regression + // this guards ("field 'max_items' is not applicable to source kind + // 'composio'"). RSS keeps it; kinds with no per-run item cap still reject. + let patch = || MemorySourcePatch { + max_items: Some(Some(100)), + ..Default::default() + }; + assert!(patch().validate_for_kind(SourceKind::Composio).is_ok()); + assert!(patch().validate_for_kind(SourceKind::RssFeed).is_ok()); + assert!(patch().validate_for_kind(SourceKind::Folder).is_err()); + assert!(patch().validate_for_kind(SourceKind::GithubRepo).is_err()); + assert!(patch().validate_for_kind(SourceKind::WebPage).is_err()); +} diff --git a/sources/src/validation.rs b/sources/src/validation.rs new file mode 100644 index 0000000..6bf4b24 --- /dev/null +++ b/sources/src/validation.rs @@ -0,0 +1,82 @@ +//! Field rules for a configured source (#18 §B4). +//! +//! Moved from the engine with the types they validate. `ensure_within_base` +//! came with them: it returned the engine's `SourceResult`, and the +//! contract's `MemoryError` already carries the `PathEscape` variant it needs, +//! so the retype is exact rather than a widening. + +use std::path::{Path, PathBuf}; + +use tinymemory_api::error::MemoryError; + +use super::types::{MemorySourceEntry, SourceKind}; + +/// Validate required fields for `entry` based on its [`SourceKind`]. +/// +/// Returns a human-readable error message describing the first failing rule. +/// `id` and `label` are required for every kind; kind-specific fields follow. +/// +pub fn validate_entry(entry: &MemorySourceEntry) -> Result<(), String> { + if entry.id.trim().is_empty() { + return Err("id is required".to_string()); + } + if entry.id.contains(':') || entry.id.chars().any(char::is_control) { + return Err("id must not contain ':' or control characters".to_string()); + } + if entry.label.is_empty() { + return Err("label is required".to_string()); + } + match entry.kind { + SourceKind::Composio => { + require_field(&entry.toolkit, "toolkit")?; + require_field(&entry.connection_id, "connection_id")?; + } + SourceKind::Conversation => { + // No kind-specific required fields — just enabled/disabled. + } + SourceKind::Folder => { + require_field(&entry.path, "path")?; + } + SourceKind::GithubRepo => { + require_field(&entry.url, "url")?; + } + SourceKind::TwitterQuery => { + require_field(&entry.query, "query")?; + } + SourceKind::RssFeed => { + require_field(&entry.url, "url")?; + } + SourceKind::WebPage => { + require_field(&entry.url, "url")?; + } + } + Ok(()) +} + +/// Require that `value` is present and non-empty, naming it `name` in errors. +fn require_field(value: &Option, name: &str) -> Result<(), String> { + match value { + Some(v) if !v.is_empty() => Ok(()), + _ => Err(format!("{name} is required for this source kind")), + } +} + +/// Canonicalize `target` and ensure it stays within canonicalized `base`. +/// +/// This is the shared path-traversal guard for local readers. Both paths must +/// exist (they are passed through [`std::fs::canonicalize`], which resolves +/// symlinks and `..` segments). If the resolved target escapes the base +/// directory, a [`MemoryError::PathEscape`] carrying `"path traversal denied"` +/// is returned. +pub fn ensure_within_base(base: &Path, target: &Path) -> Result { + let canonical_base = std::fs::canonicalize(base)?; + let canonical_target = std::fs::canonicalize(target)?; + if !canonical_target.starts_with(&canonical_base) { + return Err(MemoryError::PathEscape("path traversal denied".to_string())); + } + Ok(canonical_target) +} + +#[cfg(test)] +#[path = "validation_tests.rs"] +mod tests; diff --git a/sources/src/validation_tests.rs b/sources/src/validation_tests.rs new file mode 100644 index 0000000..5dafd58 --- /dev/null +++ b/sources/src/validation_tests.rs @@ -0,0 +1,77 @@ +//! Tests for required-field validation and the path-traversal guard. + +use super::*; +use crate::types::SourceKind; +use std::fs; +use tempfile::TempDir; + +fn entry(kind: SourceKind) -> MemorySourceEntry { + MemorySourceEntry { + id: "src_x".into(), + kind, + label: "Label".into(), + enabled: true, + toolkit: None, + connection_id: None, + path: None, + glob: None, + url: None, + branch: None, + paths: Vec::new(), + max_commits: None, + max_issues: None, + max_prs: None, + query: None, + since_days: None, + max_items: None, + selector: None, + max_tokens_per_sync: None, + max_cost_per_sync_usd: None, + sync_depth_days: None, + } +} + +#[test] +fn empty_id_or_label_is_rejected_for_every_kind() { + let mut e = entry(SourceKind::Conversation); + e.id = String::new(); + assert!(validate_entry(&e).is_err()); + + let mut e = entry(SourceKind::Conversation); + e.label = String::new(); + assert!(validate_entry(&e).is_err()); +} + +#[test] +fn empty_string_field_counts_as_missing() { + let mut e = entry(SourceKind::Folder); + e.path = Some(String::new()); + assert!(validate_entry(&e).is_err()); +} + +#[test] +fn composio_requires_both_toolkit_and_connection() { + let mut e = entry(SourceKind::Composio); + e.toolkit = Some("gmail".into()); + assert!(validate_entry(&e).is_err()); + e.connection_id = Some("conn".into()); + assert!(validate_entry(&e).is_ok()); +} + +#[test] +fn ensure_within_base_accepts_contained_file() { + let tmp = TempDir::new().unwrap(); + fs::write(tmp.path().join("ok.md"), "hi").unwrap(); + let resolved = ensure_within_base(tmp.path(), &tmp.path().join("ok.md")).unwrap(); + assert!(resolved.ends_with("ok.md")); +} + +#[test] +fn ensure_within_base_rejects_escape() { + let tmp = TempDir::new().unwrap(); + fs::write(tmp.path().join("ok.md"), "hi").unwrap(); + // Build a target that escapes the base via `..`. + let escaping = tmp.path().join("../../etc/hosts"); + let result = ensure_within_base(tmp.path(), &escaping); + assert!(result.is_err()); +} From 5fa80e56916aa952e328af6bd218a8452aefdcd1 Mon Sep 17 00:00:00 2001 From: Shanu Date: Wed, 19 Aug 2026 00:13:24 +0530 Subject: [PATCH 2/2] fmt: format the lint fixes The clippy fixes landed after the local fmt check ran, so their edits were never formatted. cargo fmt --all; no semantic change. --- core/src/sources/readers/github.rs | 4 +--- sources/src/readers/conversation.rs | 7 ++----- sources/src/readers/conversation_tests.rs | 11 ++--------- sources/src/readers/folder.rs | 6 ++---- sources/src/readers/github.rs | 5 ++--- sources/src/readers/github_tests.rs | 18 +++++++----------- sources/src/readers/mod.rs | 1 - sources/src/readers/rss.rs | 5 +---- sources/src/readers/web_page.rs | 5 +---- 9 files changed, 18 insertions(+), 44 deletions(-) diff --git a/core/src/sources/readers/github.rs b/core/src/sources/readers/github.rs index 678d35f..b85b029 100644 --- a/core/src/sources/readers/github.rs +++ b/core/src/sources/readers/github.rs @@ -12,9 +12,7 @@ use crate::sources::readers::SourceReader; use crate::sources::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; use crate::Config; -pub use tinymemory_sources::readers::github::{ - repo_archive_source_id, repo_chunk_scope, -}; +pub use tinymemory_sources::readers::github::{repo_archive_source_id, repo_chunk_scope}; pub struct GithubReader; diff --git a/sources/src/readers/conversation.rs b/sources/src/readers/conversation.rs index 6602738..e5d9886 100644 --- a/sources/src/readers/conversation.rs +++ b/sources/src/readers/conversation.rs @@ -10,14 +10,11 @@ use async_trait::async_trait; - use tinymemory_api::error::MemoryError; -use crate::SourceResult; -use crate::types::{ - ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, -}; +use crate::types::{ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind}; use crate::validation::ensure_within_base; +use crate::SourceResult; use super::SourceReader; diff --git a/sources/src/readers/conversation_tests.rs b/sources/src/readers/conversation_tests.rs index 36e15c1..3a2bd73 100644 --- a/sources/src/readers/conversation_tests.rs +++ b/sources/src/readers/conversation_tests.rs @@ -142,10 +142,7 @@ async fn read_item_returns_formatted_content() { let config = tmp.path(); let source = conversation_source(); let reader = ConversationReader; - let content = reader - .read_item(&source, "conv_123", config) - .await - .unwrap(); + let content = reader.read_item(&source, "conv_123", config).await.unwrap(); assert_eq!(content.id, "conv_123"); assert_eq!(content.title, "Test Conversation"); @@ -166,11 +163,7 @@ async fn read_item_accepts_legitimate_double_dot_in_stem() { .unwrap(); let reader = ConversationReader; let content = reader - .read_item( - &conversation_source(), - "standup..2026", - tmp.path(), - ) + .read_item(&conversation_source(), "standup..2026", tmp.path()) .await .unwrap(); assert_eq!(content.id, "standup..2026"); diff --git a/sources/src/readers/folder.rs b/sources/src/readers/folder.rs index e1fbfbf..a057f59 100644 --- a/sources/src/readers/folder.rs +++ b/sources/src/readers/folder.rs @@ -20,11 +20,9 @@ use walkdir::WalkDir; use crate::FOLDER_FILE_SIZE_CAP_BYTES; use tinymemory_api::error::MemoryError; -use crate::SourceResult; -use crate::types::{ - ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, -}; +use crate::types::{ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind}; use crate::validation::ensure_within_base; +use crate::SourceResult; use super::SourceReader; diff --git a/sources/src/readers/github.rs b/sources/src/readers/github.rs index 9bf2a52..0f58a7d 100644 --- a/sources/src/readers/github.rs +++ b/sources/src/readers/github.rs @@ -28,10 +28,9 @@ use std::time::Duration; use async_trait::async_trait; - -use crate::SourceResult; -use crate::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; use crate::raw_kind::RawKind; +use crate::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; +use crate::SourceResult; use super::{into_engine_error, SourceReader}; diff --git a/sources/src/readers/github_tests.rs b/sources/src/readers/github_tests.rs index 04f13f3..e19779c 100644 --- a/sources/src/readers/github_tests.rs +++ b/sources/src/readers/github_tests.rs @@ -132,17 +132,13 @@ async fn fetch_all_pages_stops_at_a_short_page() { // A short page (fewer than GH_PAGE_SIZE rows) is the last page; the walk // must not request page 2 after it. let mut requested: Vec = Vec::new(); - let pages = crate::readers::github::api::collect_pages::( - "commits", - 1000, - |page| { - requested.push(page); - async move { - // Page 1 is short (3 rows) — stop after it even though max is large. - Ok("[1,2,3]".to_string()) - } - }, - ) + let pages = crate::readers::github::api::collect_pages::("commits", 1000, |page| { + requested.push(page); + async move { + // Page 1 is short (3 rows) — stop after it even though max is large. + Ok("[1,2,3]".to_string()) + } + }) .await .unwrap(); diff --git a/sources/src/readers/mod.rs b/sources/src/readers/mod.rs index 5ce0d65..3e93f15 100644 --- a/sources/src/readers/mod.rs +++ b/sources/src/readers/mod.rs @@ -41,7 +41,6 @@ mod ssrf; use async_trait::async_trait; - use crate::SourceResult; #[cfg(feature = "network")] use tinymemory_api::error::MemoryError; diff --git a/sources/src/readers/rss.rs b/sources/src/readers/rss.rs index 7312522..af38c4c 100644 --- a/sources/src/readers/rss.rs +++ b/sources/src/readers/rss.rs @@ -16,11 +16,8 @@ use std::time::{Duration, Instant}; use async_trait::async_trait; - +use crate::types::{ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind}; use crate::SourceResult; -use crate::types::{ - ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, -}; use super::ssrf::{build_client, is_url_allowed, read_body_capped}; use super::{into_engine_error, SourceReader}; diff --git a/sources/src/readers/web_page.rs b/sources/src/readers/web_page.rs index 4f6ff22..396f060 100644 --- a/sources/src/readers/web_page.rs +++ b/sources/src/readers/web_page.rs @@ -15,11 +15,8 @@ use async_trait::async_trait; use super::ssrf::{build_client, is_url_allowed, read_body_capped}; use types::SelectorSpec; - +use crate::types::{ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind}; use crate::SourceResult; -use crate::types::{ - ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, -}; use super::{into_engine_error, SourceReader};