diff --git a/Cargo.lock b/Cargo.lock index ef1d040..479945c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -238,6 +238,27 @@ dependencies = [ "crypto-common 0.2.2", ] +[[package]] +name = "dirs" +version = "5.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" +dependencies = [ + "dirs-sys", +] + +[[package]] +name = "dirs-sys" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" +dependencies = [ + "libc", + "option-ext", + "redox_users", + "windows-sys 0.48.0", +] + [[package]] name = "displaydoc" version = "0.2.6" @@ -511,6 +532,12 @@ version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" +[[package]] +name = "hex" +version = "0.4.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" + [[package]] name = "http" version = "1.4.2" @@ -820,6 +847,15 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "libredox" +version = "0.1.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2026a5056764a10b2bf5d56488cba40da507f5493a6a429340e2004d9ed085fa" +dependencies = [ + "libc", +] + [[package]] name = "libsqlite3-sys" version = "0.38.1" @@ -918,6 +954,12 @@ version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" +[[package]] +name = "option-ext" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" + [[package]] name = "parking_lot" version = "0.12.5" @@ -991,7 +1033,7 @@ dependencies = [ "rustc-hash", "rustls", "socket2", - "thiserror", + "thiserror 2.0.18", "tokio", "tracing", "web-time", @@ -1013,7 +1055,7 @@ dependencies = [ "rustls", "rustls-pki-types", "slab", - "thiserror", + "thiserror 2.0.18", "tinyvec", "tracing", "web-time", @@ -1083,6 +1125,17 @@ dependencies = [ "bitflags", ] +[[package]] +name = "redox_users" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ba009ff324d1fc1b900bd1fdb31564febe58a8ccc8a6fdbb93b543d33b13ca43" +dependencies = [ + "getrandom 0.2.17", + "libredox", + "thiserror 1.0.69", +] + [[package]] name = "ref-cast" version = "1.0.25" @@ -1194,7 +1247,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c51c9ae4df8a7fba42103df5c621fa3c37eccf3a3c650879e90fc48b11cc192c" dependencies = [ "hashbrown 0.16.1", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1421,6 +1474,16 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8fadd59c855ef2080decdef8ff161eb6661b86933c9d82e5ba29dc602a55aba" +[[package]] +name = "signal-hook-registry" +version = "1.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c4db69cba1110affc0e9f7bcd48bbf87b3f4fc7c61fc9155afd4c469eb3d6c1b" +dependencies = [ + "errno", + "libc", +] + [[package]] name = "slab" version = "0.4.12" @@ -1522,13 +1585,33 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "thiserror" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" +dependencies = [ + "thiserror-impl 1.0.69", +] + [[package]] name = "thiserror" version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl", + "thiserror-impl 2.0.18", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.118", ] [[package]] @@ -1554,7 +1637,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.11.0", - "thiserror", + "thiserror 2.0.18", "tokio", "tracing", ] @@ -1566,9 +1649,11 @@ dependencies = [ "anyhow", "async-trait", "chrono", + "dirs", "dotenvy", "futures", "git2", + "hex", "log", "parking_lot", "rand", @@ -1580,7 +1665,7 @@ dependencies = [ "serde_json", "sha2 0.10.9", "tempfile", - "thiserror", + "thiserror 2.0.18", "tinyagents", "tinycortex-api", "tokio", @@ -1601,7 +1686,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.10.9", - "thiserror", + "thiserror 2.0.18", "uuid", ] @@ -1640,6 +1725,7 @@ dependencies = [ "libc", "mio", "pin-project-lite", + "signal-hook-registry", "socket2", "tokio-macros", "windows-sys 0.61.2", @@ -2052,13 +2138,22 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets 0.48.5", +] + [[package]] name = "windows-sys" version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "282be5f36a8ce781fad8c8ae18fa3f9beff57ec1b52cb3de0789201425d9a33d" dependencies = [ - "windows-targets", + "windows-targets 0.52.6", ] [[package]] @@ -2070,34 +2165,67 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-targets" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a2fa6e2155d7247be68c096456083145c183cbbbc2764150dda45a87197940c" +dependencies = [ + "windows_aarch64_gnullvm 0.48.5", + "windows_aarch64_msvc 0.48.5", + "windows_i686_gnu 0.48.5", + "windows_i686_msvc 0.48.5", + "windows_x86_64_gnu 0.48.5", + "windows_x86_64_gnullvm 0.48.5", + "windows_x86_64_msvc 0.48.5", +] + [[package]] name = "windows-targets" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b724f72796e036ab90c1021d4780d4d3d648aca59e491e6b98e725b84e99973" dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", + "windows_aarch64_gnullvm 0.52.6", + "windows_aarch64_msvc 0.52.6", + "windows_i686_gnu 0.52.6", "windows_i686_gnullvm", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", + "windows_i686_msvc 0.52.6", + "windows_x86_64_gnu 0.52.6", + "windows_x86_64_gnullvm 0.52.6", + "windows_x86_64_msvc 0.52.6", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b38e32f0abccf9987a4e3079dfb67dcd799fb61361e53e2882c3cbaf0d905d8" + [[package]] name = "windows_aarch64_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" +[[package]] +name = "windows_aarch64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc35310971f3b2dbbf3f0690a219f40e2d9afcf64f9ab7cc1be722937c26b4bc" + [[package]] name = "windows_aarch64_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" +[[package]] +name = "windows_i686_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a75915e7def60c94dcef72200b9a8e58e5091744960da64ec734a6c6e9b3743e" + [[package]] name = "windows_i686_gnu" version = "0.52.6" @@ -2110,24 +2238,48 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" +[[package]] +name = "windows_i686_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f55c233f70c4b27f66c523580f78f1004e8b5a8b659e05a4eb49d4166cca406" + [[package]] name = "windows_i686_msvc" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" +[[package]] +name = "windows_x86_64_gnu" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53d40abd2583d23e4718fddf1ebec84dbff8381c07cae67ff7768bbf19c6718e" + [[package]] name = "windows_x86_64_gnu" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0b7b52767868a23d5bab768e390dc5f5c55825b6d30b86c844ff2dc7414044cc" + [[package]] name = "windows_x86_64_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" +[[package]] +name = "windows_x86_64_msvc" +version = "0.48.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed94fce61571a4006852b7389a063ab983c02eb1bb37b47f8272ce92d06d9538" + [[package]] name = "windows_x86_64_msvc" version = "0.52.6" diff --git a/Cargo.toml b/Cargo.toml index 216d105..0c046ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,6 +63,22 @@ sync = ["dep:reqwest", "dep:tracing", "tokio"] # the OpenRouter reference provider, but the pipeline depends only on the # `ChatProvider` / `Summariser` / `EmbeddingBackend` traits. persona = [] + +# Git-backed wiki mirror of derived summary nodes +# (`memory::store::content::wiki_git`): initialises `/wiki/.git`, +# commits `summaries/**`, and stores read high-water marks as lightweight +# `refs/tags/read/*` pointers. Enables `git2` directly rather than implying +# `git-diff`, which would additionally compile in the unrelated `memory::diff` +# module — the same posture as `sync` and `providers-http` both enabling +# `dep:reqwest` independently. +wiki-git = ["dep:git2", "dep:hex"] + +# Obsidian vault interop (`memory::store::content::{obsidian,obsidian_registry}`): +# stages the bundled `.obsidian/` defaults into the content root, and +# best-effort detection of whether that root is a vault Obsidian already knows +# about (its `obsidian.json` registry). +obsidian = ["dep:dirs"] + [dependencies] anyhow = "1" log = "0.4" @@ -103,8 +119,15 @@ git2 = { version = "0.21", default-features = false, features = [ reqwest = { version = "0.12", default-features = false, features = [ "json", "rustls-tls", + # `Response::bytes_stream()` — lets the network readers cap body size while + # streaming instead of after buffering the whole response. + "stream", ], optional = true } tracing = { version = "0.1", optional = true } +# Per-OS config/home dir probing for the Obsidian vault registry (`obsidian`). +dirs = { version = "5", optional = true } +# Hex-encodes summary read-pointer ids into git tag names (`wiki-git`). +hex = { version = "0.4", optional = true } # tokio powers the optional background worker loops (`tokio` feature). It is # also listed under dev-dependencies so the async test suite always compiles. tokio = { version = "1", features = [ @@ -113,6 +136,12 @@ tokio = { version = "1", features = [ "macros", "time", "sync", + # `process` powers the GitHub source reader's `gh` / `git` subprocess calls + # (`memory::sources::readers::github`, behind the `sync` feature). + "process", + # `net` powers the web-page reader's DNS-resolution SSRF guard + # (`tokio::net::lookup_host`, behind the `sync` feature). + "net", ], optional = true } [dev-dependencies] diff --git a/src/memory/health.rs b/src/memory/health.rs new file mode 100644 index 0000000..3bf1fa8 --- /dev/null +++ b/src/memory/health.rs @@ -0,0 +1,350 @@ +//! Typed failure + degradation model for the memory pipeline. +//! +//! The chunk→wiki pipeline and the time-tree summarizer fail in several +//! distinct ways (budget exhausted, missing/invalid key, missing local +//! model, dimension mismatch, extraction timeout, transient network). +//! Historically these all collapsed into an opaque error string and were +//! retried identically — so a hard "Insufficient budget" 4xx burned the +//! retry budget and the user saw a generic `error: N failed jobs`. +//! +//! This module is the single source of truth that fixes that: +//! +//! - [`FailureCode`] enumerates every distinguishable cause. +//! - Each code maps to a [`FailureClass`] (`Transient` ⇒ retry with +//! backoff, `Unrecoverable` ⇒ fail fast) and a stable i18n +//! `remediation_key` so the status surface / doctor / job row all show +//! consistent, actionable text. Embeddings remediation leads with the +//! local-Ollama path (the steered primary fix), with BYO key secondary. +//! - [`PipelineFailure`] is a `std::error::Error`, so it can be wrapped in +//! `anyhow` and propagated up through the job processor, then downcast in +//! the queue worker to decide retry-vs-fail. +//! - [`DegradedState`] captures "the pipeline ran but recall/structure is +//! reduced" — surfaced so degraded output is never presented as success. +//! +//! ## Scope: taxonomy only +//! +//! This is the engine's own failure vocabulary and nothing else. The +//! *process-global degradation flags* that the embed/extract stages set and the +//! `pipeline_status` RPC reads stay in the embedding host, because they are +//! coupled to a host socket broadcast and to host plumbing. So does the +//! `doctor` report (it reads the host's scheduler-gate config) and the +//! user-error publisher (its `kind` string is a pinned frontend contract). +//! +//! Note the name: `tinycortex_api::health` is a *different* thing — driver +//! liveness (`MemoryHealth`). This module is pipeline failure classification. +//! +//! The `remediation_key` values are i18n keys resolved by the host's frontend. +//! They travel with [`FailureCode`] because `PipelineFailure::remediation_key` +//! is a serialized wire field populated by `PipelineFailure::new`; splitting the +//! table out would change the type's shape. The emitted strings are unchanged. + +use std::fmt; + +mod types; + +pub use types::{DegradedState, FailureClass, FailureCode, PipelineFailure}; + +impl FailureClass { + pub fn as_str(self) -> &'static str { + match self { + Self::Transient => "transient", + Self::Unrecoverable => "unrecoverable", + } + } +} + +impl FailureCode { + /// Stable wire string. + pub fn as_str(self) -> &'static str { + match self { + Self::BudgetExhausted => "budget_exhausted", + Self::AuthMissing => "auth_missing", + Self::AuthInvalid => "auth_invalid", + Self::EmbeddingsUnconfigured => "embeddings_unconfigured", + Self::EmbeddingDimMismatch => "embedding_dim_mismatch", + Self::LocalModelUnavailable => "local_model_unavailable", + Self::ExtractionTimeout => "extraction_timeout", + Self::SummarizerUnavailable => "summarizer_unavailable", + Self::EmptyInputRefused => "empty_input_refused", + Self::StorageUnavailable => "storage_unavailable", + Self::Transient => "transient", + } + } + + /// Parses the stable wire string produced by [`Self::as_str`]. + /// + /// Deliberately an inherent method returning `Option`, not a + /// [`std::str::FromStr`] impl: the trait must return `Result`, and this + /// arrived here as a verbatim relocation from the OpenHuman host. Changing + /// the signature would be an API change smuggled inside a move, which is + /// exactly what the port was structured to avoid. Revisit as its own + /// change if a `FromStr` impl is ever wanted. + #[allow(clippy::should_implement_trait)] + pub fn from_str(s: &str) -> Option { + Some(match s { + "budget_exhausted" => Self::BudgetExhausted, + "auth_missing" => Self::AuthMissing, + "auth_invalid" => Self::AuthInvalid, + "embeddings_unconfigured" => Self::EmbeddingsUnconfigured, + "embedding_dim_mismatch" => Self::EmbeddingDimMismatch, + "local_model_unavailable" => Self::LocalModelUnavailable, + "extraction_timeout" => Self::ExtractionTimeout, + "summarizer_unavailable" => Self::SummarizerUnavailable, + "empty_input_refused" => Self::EmptyInputRefused, + "storage_unavailable" => Self::StorageUnavailable, + "transient" => Self::Transient, + _ => return None, + }) + } + + /// Retry policy for this cause. + /// + /// [`LocalModelUnavailable`](Self::LocalModelUnavailable) is deliberately + /// **transient** even though the user has to act: the condition (Ollama + /// daemon stopped, model not pulled) clears from outside the app, and only + /// transient rows are picked up by `requeue_transient_failed` — the + /// automatic self-healing requeue. Classifying it unrecoverable would park + /// every affected job until someone clicks "Retry failed" by hand, so a + /// user who simply restarts Ollama would never see ingestion resume. + pub fn class(self) -> FailureClass { + match self { + Self::Transient | Self::ExtractionTimeout | Self::LocalModelUnavailable => { + FailureClass::Transient + } + _ => FailureClass::Unrecoverable, + } + } + + /// i18n key for the user-facing remediation. Embeddings causes lead + /// with the local-Ollama path (the steered primary fix per spec FR-015). + pub fn remediation_key(self) -> &'static str { + match self { + Self::BudgetExhausted => "memory.health.remediation.budget_exhausted", + Self::AuthMissing => "memory.health.remediation.auth_missing", + Self::AuthInvalid => "memory.health.remediation.auth_invalid", + Self::EmbeddingsUnconfigured => "memory.health.remediation.embeddings_unconfigured", + Self::EmbeddingDimMismatch => "memory.health.remediation.embedding_dim_mismatch", + Self::LocalModelUnavailable => "memory.health.remediation.local_model_unavailable", + Self::ExtractionTimeout => "memory.health.remediation.extraction_timeout", + Self::SummarizerUnavailable => "memory.health.remediation.summarizer_unavailable", + Self::EmptyInputRefused => "memory.health.remediation.empty_input_refused", + Self::StorageUnavailable => "memory.health.remediation.storage_unavailable", + Self::Transient => "memory.health.remediation.transient", + } + } +} + +impl PipelineFailure { + /// Build a failure from a code, deriving class + remediation key. + pub fn new(code: FailureCode) -> Self { + Self { + code, + class: code.class(), + remediation_key: code.remediation_key().to_string(), + detail: None, + } + } + + /// Attach a non-localized detail string (bounded by `truncate_detail`; + /// never log secrets). + pub fn with_detail(mut self, detail: impl Into) -> Self { + let detail = detail.into(); + self.detail = Some(truncate_detail(&detail)); + self + } + + /// True when this failure should fail fast (no retry budget). + pub fn is_unrecoverable(&self) -> bool { + self.class == FailureClass::Unrecoverable + } +} + +/// Classify an embedding-stage error into a typed [`PipelineFailure`]. +/// +/// The embed path bottoms out in `embeddings::openai::OpenAiEmbedding::embed`, +/// which on a non-2xx response bails with the message +/// `"Embedding API error (): "` (status is reqwest's +/// `StatusCode` Display, e.g. `402 Payment Required`). Dimension mismatches +/// surface from the memory-tree `CloudEmbedder`/trait validator as +/// `"... returned N dims, expected M"` or `"... dims, expected ..."`. We +/// parse those shapes to decide retry-vs-fail: +/// +/// - `401` / `403` → `auth_invalid` (a bearer was sent but rejected). +/// - `402` / `429` / a body mentioning budget/quota/insufficient → +/// `budget_exhausted` (the managed Voyage route is out of budget; the +/// user must bring their own key or top up — retrying won't help). +/// - dimension-mismatch text → `embedding_dim_mismatch`. +/// - Ollama daemon-unreachable / model-not-pulled text → +/// `local_model_unavailable`, so the panel names the local-runtime fix. +/// - everything else (5xx, timeouts, transport, unparseable) → `transient`, +/// so the worker's existing retry-with-backoff still applies. +/// +/// Operates on the flattened `anyhow` chain (`{err:#}`) so it still matches +/// when the embed error has been `.context()`-wrapped on the way up. +pub fn classify_embed_error(err: &anyhow::Error) -> PipelineFailure { + let msg = format!("{err:#}"); + classify_embed_error_str(&msg) +} + +/// String-level core of [`classify_embed_error`], split out so unit tests can +/// exercise the mapping without constructing reqwest errors. +pub fn classify_embed_error_str(msg: &str) -> PipelineFailure { + let lower = msg.to_ascii_lowercase(); + + // #13021: client-side refusal from the provider pre-flight guard fires + // *before* any HTTP round-trip, so it carries no `Embedding API error + // ()` shape. Without an explicit match it would fall through to + // `Transient` and the `reembed_backfill` worker would retry the same + // un-embeddable row forever (and eventually fail the whole job). + // Classify as unrecoverable per-row so the worker tombstones the chunk / + // summary instead. Both `OpenAiEmbedding::embed` and + // `OpenHumanCloudEmbedding::embed` use the literal phrase + // "refusing empty/whitespace". + if lower.contains("refusing empty/whitespace") { + return PipelineFailure::new(FailureCode::EmptyInputRefused) + .with_detail(truncate_detail(msg)); + } + + // Sibling of the #13021 case above: `OpenHumanCloudEmbedding::resolve_bearer` + // bails *before any HTTP round-trip* when the desktop/backend session + // bearer is absent (user signed out), with the literal phrase + // "No backend session for cloud embeddings ..." (see + // `src/openhuman/inference/embeddings/cloud.rs`). Being a client-side bail it carries + // no `Embedding API error ()` shape, so without this match it falls + // through to `Transient` — the Memory Tree then shows "temporary error… + // will retry automatically" and the worker retries an auth failure that a + // retry can never fix. Classify as `AuthMissing` so the health banner + // surfaces the "log in to OpenHuman" remediation and the job fails fast. + if lower.contains("no backend session") { + return PipelineFailure::new(FailureCode::AuthMissing).with_detail(truncate_detail(msg)); + } + + // #5354 — the local Ollama runtime is not usable: the daemon is not + // listening, or the configured embedding model was never pulled. Both are + // emitted by `tinyagents::harness::embeddings::ollama` with the fix already + // in the text: + // + // "ollama embed request failed (is Ollama running at ?): …" + // "Ollama embedding model `` is not installed at . Run `ollama pull ` …" + // + // Neither carries an `Embedding API error ()` shape — the first is a + // transport bail, the second a rewritten 404 — so both used to fall through + // to `Transient` and surface as "a temporary error … will retry + // automatically". That is the wrong remediation: retrying cannot start a + // daemon or pull a model, and the user was never told what to do. Match the + // two shapes explicitly so the status panel renders the + // `local_model_unavailable` remediation instead. The class stays transient + // (see `FailureCode::class`) so jobs auto-resume once Ollama is back. + // + // Anchored on Ollama-specific wording so a generic cloud-embedder transport + // failure ("error sending request for url …") keeps its `Transient` code. + if lower.contains("is ollama running at") + || (lower.contains("ollama embedding model") && lower.contains("is not installed at")) + { + return PipelineFailure::new(FailureCode::LocalModelUnavailable) + .with_detail(truncate_detail(msg)); + } + + // Dimension mismatch — the trait validator / CloudEmbedder rejects a + // vector whose length isn't EMBEDDING_DIM. Check before status parsing: + // it's a 2xx-but-wrong-shape case with no HTTP status to match. + if lower.contains("dims, expected") || lower.contains("dimensions, expected") { + return PipelineFailure::new(FailureCode::EmbeddingDimMismatch) + .with_detail(truncate_detail(msg)); + } + + // Budget/quota wording wins regardless of the numeric status — the + // managed backend may surface budget exhaustion as 4xx with an explicit + // body, and we always want the BYO-key remediation here. + if lower.contains("insufficient budget") + || lower.contains("budget") + || lower.contains("quota") + || lower.contains("payment required") + { + return PipelineFailure::new(FailureCode::BudgetExhausted) + .with_detail(truncate_detail(msg)); + } + + // Parse the HTTP status out of the `Embedding API error (): ...` + // shape. reqwest renders e.g. `402 Payment Required`, so the first + // 3-digit run after the opening paren is the code. + if let Some(code) = parse_http_status(msg) { + return match code { + 401 | 403 => { + PipelineFailure::new(FailureCode::AuthInvalid).with_detail(truncate_detail(msg)) + } + 402 => { + PipelineFailure::new(FailureCode::BudgetExhausted).with_detail(truncate_detail(msg)) + } + 429 => PipelineFailure::new(FailureCode::Transient).with_detail(truncate_detail(msg)), + // 4xx other than the above is a hard client error retrying won't + // fix (malformed request, model not found); fail fast but tag it + // generically as auth_invalid's sibling — use Transient only for + // 5xx/unknown. We treat unknown 4xx as unrecoverable via + // budget? No — be conservative: only the known codes above are + // unrecoverable; other 4xx fall through to transient so we don't + // wedge on a transient 408/425. + 500..=599 => { + PipelineFailure::new(FailureCode::Transient).with_detail(truncate_detail(msg)) + } + _ => PipelineFailure::new(FailureCode::Transient).with_detail(truncate_detail(msg)), + }; + } + + // No recognizable status — transport error, timeout, connection reset, + // or an unparseable message. Treat as transient so retry/backoff applies. + PipelineFailure::new(FailureCode::Transient).with_detail(truncate_detail(msg)) +} + +/// Extract the HTTP status code from an `Embedding API error ()` +/// message. Anchors on the `Embedding API error (` marker rather than the +/// first `(` in the flattened anyhow chain, so a wrapper context that happens +/// to contain parentheses before the real error cannot break the parse. +fn parse_http_status(msg: &str) -> Option { + let (_, rest) = msg.split_once("Embedding API error (")?; + let digits: String = rest + .trim_start() + .chars() + .take_while(|c| c.is_ascii_digit()) + .collect(); + if digits.len() == 3 { + digits.parse().ok() + } else { + None + } +} + +/// Cap a detail string so we never balloon logs / wire payloads with a full +/// provider response body. Never contains a secret (it's an error body), but +/// keep it short anyway. +fn truncate_detail(s: &str) -> String { + const MAX: usize = 200; + if s.chars().count() <= MAX { + return s.to_string(); + } + let truncated: String = s.chars().take(MAX).collect(); + format!("{truncated}…") +} + +impl fmt::Display for PipelineFailure { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} ({})", self.code.as_str(), self.class.as_str())?; + if let Some(detail) = &self.detail { + write!(f, ": {detail}")?; + } + Ok(()) + } +} + +impl std::error::Error for PipelineFailure {} + +impl DegradedState { + /// True when any degradation is present. + pub fn is_degraded(&self) -> bool { + self.semantic_recall || self.structure || self.storage + } +} + +#[cfg(test)] +#[path = "health_tests.rs"] +mod tests; diff --git a/src/memory/health/types.rs b/src/memory/health/types.rs new file mode 100644 index 0000000..bacf72f --- /dev/null +++ b/src/memory/health/types.rs @@ -0,0 +1,107 @@ +//! Type definitions for the pipeline failure + degradation taxonomy. +//! +//! The taxonomy itself — `FailureClass`, `FailureCode`, `PipelineFailure`, +//! `DegradedState` — is data: serde types carried on the wire and embedded in +//! `PipelineFailure`. The classification logic (`classify_embed_error`), the +//! `std::error::Error` plumbing, and the remediation-key/class derivation live +//! in the parent `health` module, so this file stays pure type definitions. + +use serde::{Deserialize, Serialize}; + +/// Whether a failure should be retried (`Transient`) or fail fast +/// (`Unrecoverable`). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FailureClass { + /// Retry with backoff up to `max_attempts` (network 5xx, timeouts, + /// truncated streams). + Transient, + /// Stop immediately — retrying the same input cannot succeed (budget + /// exhausted, bad/missing key, missing local model, dim mismatch). + Unrecoverable, +} + +/// A distinguishable pipeline failure cause. Each variant carries a fixed +/// [`FailureClass`] and i18n remediation key. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum FailureCode { + /// Managed embeddings route returned an out-of-budget error (4xx). + BudgetExhausted, + /// No auth/session available for the embeddings provider. + AuthMissing, + /// Auth present but rejected (expired/invalid key or JWT). + AuthInvalid, + /// No embeddings provider is configured at all. + EmbeddingsUnconfigured, + /// Provider returned vectors of an unexpected dimensionality. + EmbeddingDimMismatch, + /// A required local model (Ollama) is not available. + LocalModelUnavailable, + /// The extraction model timed out / exhausted retries. + ExtractionTimeout, + /// No summarization provider could be resolved for "Build Summary Trees" + /// — neither local AI nor a configured cloud chat provider. Distinct from + /// [`LocalModelUnavailable`](Self::LocalModelUnavailable), which implies the + /// local path was selected; this covers the cloud-only setup whose provider + /// failed to resolve, so the remediation names both paths. + SummarizerUnavailable, + /// The embedding provider refused an empty/whitespace input at the + /// pre-flight guard (#13021). Unrecoverable per-row: the offending row + /// will never become embeddable, so the worker must tombstone it instead + /// of retrying. Bail wording for both `OpenAiEmbedding::embed` and + /// `OpenHumanCloudEmbedding::embed` starts with + /// `" embed: refusing empty/whitespace input ..."`. + EmptyInputRefused, + /// The host filesystem cannot service the memory_tree path — `create_dir` + /// / DB open returned a persistent OS-level I/O error (EIO `5`, ENOSPC + /// `28`, EROFS `30`), e.g. a failing/disconnected SD card or a volume the + /// kernel remounted read-only. Unrecoverable from inside the app: only the + /// user can reseat/replace/free the storage. Distinct from the embeddings + /// provider faults above and from the SQLite-level `SQLITE_FULL` / + /// `SQLITE_CORRUPT` handled in the queue worker — this is the + /// directory/DB-init layer below them. + StorageUnavailable, + /// Catch-all transient failure (network 5xx, timeout, truncated JSON). + Transient, +} + +/// A typed pipeline failure: a [`FailureCode`] plus the derived class + +/// remediation key (carried on the wire so the frontend stays +/// presentational) and an optional human-readable detail for logs/diagnosis. +/// +/// Implements [`std::error::Error`] so it can be `anyhow`-wrapped at the +/// embed/extract/summarize boundary, propagated through the job processor, +/// and downcast in the queue worker to drive retry-vs-fail. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct PipelineFailure { + pub code: FailureCode, + pub class: FailureClass, + /// i18n key — the frontend resolves this to localized remediation text. + pub remediation_key: String, + /// Optional non-localized detail for logs/diagnosis (never a secret). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub detail: Option, +} + +/// "The pipeline ran, but output quality is reduced." Surfaced so degraded +/// results are never presented as success. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct DegradedState { + /// True when embeddings were skipped (no usable provider) so semantic + /// recall falls back to recency-only. + pub semantic_recall: bool, + /// True when extraction yielded empty across the board so the wiki has + /// no entity/topic structure. + pub structure: bool, + /// True when the memory_tree's own storage path is unusable — the host + /// filesystem returned a persistent I/O error on dir-create / DB open + /// (EIO/ENOSPC/EROFS). This is the most severe degradation: the pipeline + /// can't even open its DB, so nothing else runs. `#[serde(default)]` keeps + /// the wire format backward-compatible (older clients omit it → `false`). + #[serde(default)] + pub storage: bool, + /// The cause of the most significant degradation, when known. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub cause: Option, +} diff --git a/src/memory/health_tests.rs b/src/memory/health_tests.rs new file mode 100644 index 0000000..18c4392 --- /dev/null +++ b/src/memory/health_tests.rs @@ -0,0 +1,372 @@ +//! Tests for the pipeline failure taxonomy and the embed-error classifier. + +use super::*; + +const ALL_CODES: [FailureCode; 11] = [ + FailureCode::BudgetExhausted, + FailureCode::AuthMissing, + FailureCode::AuthInvalid, + FailureCode::EmbeddingsUnconfigured, + FailureCode::EmbeddingDimMismatch, + FailureCode::LocalModelUnavailable, + FailureCode::ExtractionTimeout, + FailureCode::SummarizerUnavailable, + FailureCode::EmptyInputRefused, + FailureCode::StorageUnavailable, + FailureCode::Transient, +]; + +#[test] +fn every_code_has_class_and_nonempty_remediation_key() { + for code in ALL_CODES { + let key = code.remediation_key(); + assert!( + !key.is_empty(), + "{} has empty remediation key", + code.as_str() + ); + assert!( + key.starts_with("memory.health.remediation."), + "{} remediation key has unexpected prefix: {key}", + code.as_str() + ); + // class() must be total (no panic); Transient, ExtractionTimeout + // and LocalModelUnavailable are retryable, everything else is + // unrecoverable. + let class = code.class(); + match code { + FailureCode::Transient + | FailureCode::ExtractionTimeout + | FailureCode::LocalModelUnavailable => { + assert_eq!( + class, + FailureClass::Transient, + "{} should be transient", + code.as_str() + ); + } + _ => { + assert_eq!( + class, + FailureClass::Unrecoverable, + "{} should be unrecoverable", + code.as_str() + ); + } + } + } +} + +#[test] +fn code_str_roundtrips() { + for code in ALL_CODES { + assert_eq!(FailureCode::from_str(code.as_str()), Some(code)); + } + assert_eq!(FailureCode::from_str("nonsense"), None); +} + +#[test] +fn new_fills_class_and_remediation_from_code() { + let f = PipelineFailure::new(FailureCode::BudgetExhausted); + assert_eq!(f.code, FailureCode::BudgetExhausted); + assert_eq!(f.class, FailureClass::Unrecoverable); + assert_eq!( + f.remediation_key, + "memory.health.remediation.budget_exhausted" + ); + assert!(f.detail.is_none()); + assert!(f.is_unrecoverable()); +} + +#[test] +fn with_detail_and_display() { + let f = PipelineFailure::new(FailureCode::Transient).with_detail("HTTP 503"); + assert_eq!(f.detail.as_deref(), Some("HTTP 503")); + assert!(!f.is_unrecoverable()); + assert_eq!(f.to_string(), "transient (transient): HTTP 503"); +} + +#[test] +fn with_detail_truncates_long_input() { + // `with_detail` must bound the stored detail itself, not rely on every + // caller remembering to pre-truncate — an unbounded detail would balloon + // logs / wire payloads with a full provider response body. + let f = PipelineFailure::new(FailureCode::Transient).with_detail("x".repeat(500)); + let detail = f.detail.expect("detail must be set"); + assert!( + detail.chars().count() <= 201, + "detail not bounded, got {} chars", + detail.chars().count() + ); + assert!(detail.ends_with('…')); +} + +#[test] +fn pipeline_failure_serde_roundtrips() { + let f = PipelineFailure::new(FailureCode::EmbeddingDimMismatch).with_detail("got 3072"); + let json = serde_json::to_string(&f).unwrap(); + let back: PipelineFailure = serde_json::from_str(&json).unwrap(); + assert_eq!(f, back); + // detail omitted when None. + let none = PipelineFailure::new(FailureCode::AuthMissing); + assert!(!serde_json::to_string(&none).unwrap().contains("detail")); +} + +#[test] +fn degraded_state_default_is_healthy() { + let d = DegradedState::default(); + assert!(!d.is_degraded()); + let d2 = DegradedState { + structure: true, + ..Default::default() + }; + assert!(d2.is_degraded()); +} + +#[test] +fn pipeline_failure_is_error_and_downcasts_from_anyhow() { + let err: anyhow::Error = anyhow::Error::new(PipelineFailure::new(FailureCode::BudgetExhausted)); + let downcast = err.downcast_ref::(); + assert!(downcast.is_some()); + assert!(downcast.unwrap().is_unrecoverable()); +} + +// ── classify_embed_error (T008) ────────────────────────────────────── + +#[test] +fn classify_budget_from_body_wording() { + // The managed Voyage route surfaces budget exhaustion in the body. + let f = classify_embed_error_str( + "Embedding API error (400 Bad Request): {\"error\":\"Insufficient budget\"}", + ); + assert_eq!(f.code, FailureCode::BudgetExhausted); + assert!(f.is_unrecoverable()); +} + +#[test] +fn classify_budget_from_402() { + let f = classify_embed_error_str("Embedding API error (402 Payment Required): nope"); + assert_eq!(f.code, FailureCode::BudgetExhausted); + assert!(f.is_unrecoverable()); +} + +#[test] +fn classify_429_rate_limit_as_transient() { + let f = classify_embed_error_str("Embedding API error (429 Too Many Requests): nope"); + assert_eq!(f.code, FailureCode::Transient); + assert!(!f.is_unrecoverable()); +} + +#[test] +fn classify_auth_from_401_403() { + for status in ["401 Unauthorized", "403 Forbidden"] { + let f = classify_embed_error_str(&format!("Embedding API error ({status}): denied")); + assert_eq!(f.code, FailureCode::AuthInvalid, "status {status}"); + assert!(f.is_unrecoverable()); + } +} + +#[test] +fn classify_dim_mismatch() { + let f = classify_embed_error_str("cloud embedder returned 3072 dims, expected 1024"); + assert_eq!(f.code, FailureCode::EmbeddingDimMismatch); + assert!(f.is_unrecoverable()); +} + +/// #13021: the provider pre-flight bail wording from both OpenAI and the +/// cloud wrapper must classify as `EmptyInputRefused` (unrecoverable) so +/// `reembed_backfill` tombstones the offending row instead of retrying +/// the same blank input forever and eventually failing the job. +#[test] +fn classify_empty_input_refusal_as_unrecoverable() { + for msg in [ + "openai embed: refusing empty/whitespace input at index 0 of 1 (model=text-embedding-3-small)", + "cloud embed: refusing empty/whitespace input at index 2 of 5 (model=embedding-v1)", + ] { + let f = classify_embed_error_str(msg); + assert_eq!( + f.code, + FailureCode::EmptyInputRefused, + "expected EmptyInputRefused for {msg:?}" + ); + assert!( + f.is_unrecoverable(), + "EmptyInputRefused must be unrecoverable for {msg:?}" + ); + } +} + +/// The refusal must out-rank the dim-mismatch and budget rules even when +/// the wrapped error happens to contain those tokens — the refusal phrase +/// is the most specific signal and the only one that means "this row is +/// permanently un-embeddable", not "the provider is misbehaving". +#[test] +fn classify_empty_input_refusal_through_anyhow_context_chain() { + let base = anyhow::anyhow!( + "openai embed: refusing empty/whitespace input at index 0 of 1 (model=embedding-v1)" + ); + let wrapped = base + .context("embed summary during seal tree_id=t level=0") + .context("reembed_backfill chunk_id=c"); + let f = classify_embed_error(&wrapped); + assert_eq!(f.code, FailureCode::EmptyInputRefused); + assert!(f.is_unrecoverable()); +} + +/// #4359: `OpenHumanCloudEmbedding::resolve_bearer` bails with "No backend +/// session for cloud embeddings ..." *before any HTTP call* when the user +/// is signed out. This must classify as `AuthMissing` (unrecoverable, "log +/// in to OpenHuman" remediation) rather than falling through to `Transient` +/// ("will retry automatically" — a loop that an auth failure can never win). +#[test] +fn classify_no_backend_session_as_auth_missing() { + let msg = "No backend session for cloud embeddings: log in to OpenHuman, or set \ + memory.embedding_provider to \"ollama\" / \"none\" in config.toml"; + let f = classify_embed_error_str(msg); + assert_eq!( + f.code, + FailureCode::AuthMissing, + "expected AuthMissing for {msg:?}" + ); + assert_eq!(f.class, FailureClass::Unrecoverable); + assert_eq!(f.remediation_key, "memory.health.remediation.auth_missing"); + assert!(f.is_unrecoverable()); +} + +/// The match must be case-insensitive and survive `anyhow` context wrapping: +/// the bail is `.context()`-wrapped on its way up through the embed pipeline +/// (e.g. `embed_each_via_provider` adds "cloud embeddings failed"), and +/// `classify_embed_error` flattens the chain via `{err:#}`. +#[test] +fn classify_no_backend_session_through_anyhow_context_chain() { + let base = anyhow::anyhow!( + "No backend session for cloud embeddings: log in to OpenHuman, or set \ + memory.embedding_provider to \"ollama\" / \"none\" in config.toml" + ); + let wrapped = base + .context("cloud embeddings failed") + .context("reembed_backfill chunk_id=c"); + let f = classify_embed_error(&wrapped); + assert_eq!(f.code, FailureCode::AuthMissing); + assert!(f.is_unrecoverable()); +} + +#[test] +fn classify_5xx_is_transient() { + let f = classify_embed_error_str("Embedding API error (503 Service Unavailable): retry"); + assert_eq!(f.code, FailureCode::Transient); + assert!(!f.is_unrecoverable()); +} + +#[test] +fn classify_transport_error_is_transient() { + let f = classify_embed_error_str("error sending request for url (...): connection reset"); + assert_eq!(f.code, FailureCode::Transient); + assert!(!f.is_unrecoverable()); +} + +/// #5354 — the Ollama daemon is not listening. Verbatim wording from +/// `tinyagents::harness::embeddings::ollama::OllamaEmbeddingModel::request`. +/// Note the parenthesised hint: `parse_http_status` reads the first `(`, so +/// without an explicit match this fell through to `Transient` and the panel +/// told the user to wait for a retry that can never start their daemon. +#[test] +fn classify_ollama_daemon_down_as_local_model_unavailable() { + let f = classify_embed_error_str( + "ollama embed request failed (is Ollama running at http://localhost:11434?): \ + error sending request for url (http://localhost:11434/api/embed)", + ); + assert_eq!(f.code, FailureCode::LocalModelUnavailable); + assert_eq!( + f.remediation_key, + "memory.health.remediation.local_model_unavailable" + ); + // Transient so `requeue_transient_failed` resumes ingestion by itself + // once the user starts Ollama again. + assert!(!f.is_unrecoverable()); +} + +/// #5354 — the model was never pulled. `ollama_http_error` rewrites the +/// 404 into remediation prose, so the `Embedding API error ()` +/// shape the status parser looks for is gone. +#[test] +fn classify_ollama_model_not_pulled_as_local_model_unavailable() { + let f = classify_embed_error_str( + "Ollama embedding model `bge-m3` is not installed at http://localhost:11434. \ + Run `ollama pull bge-m3` or choose an installed embedding model", + ); + assert_eq!(f.code, FailureCode::LocalModelUnavailable); + assert!(!f.is_unrecoverable()); +} + +/// The real call path wraps the provider error twice (`ProviderEmbedder` +/// adds "ollama embeddings failed", then the seal/reembed site adds its +/// own context), so the matcher must survive the flattened chain. +#[test] +fn classify_ollama_daemon_down_through_anyhow_context_chain() { + let base = anyhow::anyhow!( + "ollama embed request failed (is Ollama running at http://127.0.0.1:11434?): \ + tcp connect error: Connection refused (os error 61)" + ); + let wrapped = base + .context("ollama embeddings failed") + .context("seal embedding failed"); + let f = classify_embed_error(&wrapped); + assert_eq!(f.code, FailureCode::LocalModelUnavailable); +} + +/// Regression guard for the matcher's blast radius: a cloud-embedder +/// transport failure carries no Ollama wording and must keep its generic +/// `Transient` code, or every network blip would start telling users to +/// install Ollama. +#[test] +fn classify_non_ollama_transport_error_stays_transient() { + let f = classify_embed_error_str( + "cloud embeddings failed: error sending request for url \ + (https://api.tinyhumans.ai/openai/v1/embeddings): connection reset", + ); + assert_eq!(f.code, FailureCode::Transient); +} + +#[test] +fn classify_through_anyhow_context_chain() { + // The embed error is commonly `.context()`-wrapped on the way up; + // the flattened `{err:#}` must still classify. + let base = anyhow::anyhow!("Embedding API error (402 Payment Required): out of budget"); + let wrapped = base + .context("cloud embeddings failed") + .context("seal embed"); + let f = classify_embed_error(&wrapped); + assert_eq!(f.code, FailureCode::BudgetExhausted); +} + +#[test] +fn parse_http_status_extracts_leading_code() { + assert_eq!( + parse_http_status("Embedding API error (402 Payment Required): x"), + Some(402) + ); + assert_eq!(parse_http_status("no parens here"), None); + assert_eq!(parse_http_status("(not a status): x"), None); +} + +#[test] +fn classify_embed_error_survives_context_with_parens() { + // `parse_http_status` must anchor on the `Embedding API error (` marker, + // not the first `(` in the flattened anyhow chain. A wrapper context + // containing parentheses before the real error used to make the parse + // return `None`, demoting a hard auth failure to `Transient`. + let base = anyhow::anyhow!("Embedding API error (401 Unauthorized): bad bearer"); + let wrapped = base.context("provider rejected the request (see logs for details)"); + let f = classify_embed_error(&wrapped); + assert_eq!(f.code, FailureCode::AuthInvalid); + assert!(f.is_unrecoverable()); +} + +#[test] +fn truncate_detail_caps_length() { + let long = "x".repeat(500); + let out = truncate_detail(&long); + assert!(out.chars().count() <= 201, "got {}", out.chars().count()); + assert!(out.ends_with('…')); +} diff --git a/src/memory/mod.rs b/src/memory/mod.rs index 531c56e..6614394 100644 --- a/src/memory/mod.rs +++ b/src/memory/mod.rs @@ -73,6 +73,7 @@ pub mod entities; pub mod fsutil; pub mod goals; pub mod graph; +pub mod health; pub mod ingest; pub mod queue; pub mod retrieval; diff --git a/src/memory/queue/worker.rs b/src/memory/queue/worker.rs index 6c1291a..6db7d44 100644 --- a/src/memory/queue/worker.rs +++ b/src/memory/queue/worker.rs @@ -189,11 +189,24 @@ fn settle_planned_job( } Err(err) => { // Preserve the full anyhow cause chain in `last_error` so a reader - // can see the root cause. If the chain carries a typed `JobFailure`, + // can see the root cause. If the chain carries a typed failure, // pass it through so an unrecoverable cause fails fast instead of // burning the retry budget. let message = format!("{err:#}"); - let typed = err.downcast_ref::(); + // The pipeline's own failure taxonomy (`health::PipelineFailure`) is + // the type `classify_embed_error` returns; handlers wrap it in the + // anyhow chain with `.context(...)`. Map it onto the queue's + // settlement type so unrecoverable codes (`budget_exhausted`, + // `auth_invalid`, …) still fail fast. + let converted_pipeline_failure = err + .downcast_ref::() + .map(|f| JobFailure { + code: f.code.as_str(), + class: f.class.as_str(), + }); + let typed = err + .downcast_ref::() + .or(converted_pipeline_failure.as_ref()); mark_failed_typed(config, job, &message, typed)?; // The handler clears this flag on every successful terminal diff --git a/src/memory/queue/worker_tests.rs b/src/memory/queue/worker_tests.rs index ea7d328..73675ce 100644 --- a/src/memory/queue/worker_tests.rs +++ b/src/memory/queue/worker_tests.rs @@ -73,6 +73,45 @@ async fn run_once_parks_unparseable_payload_as_unrecoverable() { ); } +/// A handler that propagates the pipeline taxonomy +/// (`health::PipelineFailure`) must still fail fast: the worker downcasts it +/// and maps `code`/`class` onto the queue settlement type. +#[test] +fn pipeline_failure_downcast_fails_fast_as_unrecoverable() { + use crate::memory::health::{FailureCode, PipelineFailure}; + use crate::memory::queue::types::JobKind; + + let (_tmp, cfg) = test_config(); + let poison = NewJob { + kind: JobKind::FlushStale, + payload_json: "{}".into(), + dedupe_key: None, + available_at_ms: None, + max_attempts: Some(5), + }; + let id = enqueue(&cfg, &poison).unwrap().expect("enqueued"); + let claimed = claim_next(&cfg, DEFAULT_LOCK_DURATION_MS).unwrap().unwrap(); + + // The production shape: a handler returns a `PipelineFailure` and the + // processor wraps it in anyhow context on the way up. The worker's + // downcast must still find it through the chain. + let err = anyhow::Error::new( + PipelineFailure::new(FailureCode::BudgetExhausted) + .with_detail("managed voyage route exhausted"), + ) + .context("flush stale handler failed"); + settle_job(&cfg, &claimed, Err(err)).unwrap(); + + let job = get_job(&cfg, &id).unwrap().unwrap(); + assert_eq!( + job.status, + JobStatus::Failed, + "budget_exhausted PipelineFailure must fail fast, not retry" + ); + assert_eq!(job.failure_class.as_deref(), Some("unrecoverable")); + assert_eq!(job.failure_reason.as_deref(), Some("budget_exhausted")); +} + #[tokio::test] async fn run_once_claims_and_completes_a_flush_stale_job() { let (_tmp, cfg) = test_config(); diff --git a/src/memory/sources/mod.rs b/src/memory/sources/mod.rs index f013a1d..85965e9 100644 --- a/src/memory/sources/mod.rs +++ b/src/memory/sources/mod.rs @@ -17,11 +17,12 @@ //! //! ## Ownership boundary //! -//! Per the engine spec, TinyCortex does **not** own live sync, polling, or -//! OAuth. Network-backed kinds keep their type contracts and validation here, -//! but their live fetchers are host-owned. Only the local kinds — `folder` and -//! `conversation` — ship real readers (see [`readers::reader_for`]). The host's -//! sync runner consumes this registry to decide what to sync and when. +//! TinyCortex owns fetching and parsing — `github_repo`, `rss_feed`, and +//! `web_page` ship readers here behind the `sync` feature — but it does **not** +//! own live sync scheduling, polling cadence, OAuth, or credentials. The host's +//! sync runner consumes this registry to decide what to sync and when, and +//! [`readers::reader_for`] hands out only the two kinds (`folder`, +//! `conversation`) that are safe to read on a timer with no network egress. pub mod readers; pub mod registry; diff --git a/src/memory/sources/readers/github.rs b/src/memory/sources/readers/github.rs new file mode 100644 index 0000000..7f02810 --- /dev/null +++ b/src/memory/sources/readers/github.rs @@ -0,0 +1,344 @@ +//! 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::memory::config::MemoryConfig; +use crate::memory::error::MemoryEngineResult; +use crate::memory::sources::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; +use crate::memory::store::content::raw::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 +} + +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, + config: &MemoryConfig, + ) -> MemoryEngineResult> { + self.list_items_inner(source, config) + .await + .map_err(into_engine_error) + } + + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + config: &MemoryConfig, + ) -> MemoryEngineResult { + self.read_item_inner(source, item_id, config) + .await + .map_err(into_engine_error) + } +} + +impl GithubReader { + async fn list_items_inner( + &self, + source: &MemorySourceEntry, + config: &MemoryConfig, + ) -> 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(&config.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, + config: &MemoryConfig, + ) -> 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(&config.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/src/memory/sources/readers/github/api.rs b/src/memory/sources/readers/github/api.rs new file mode 100644 index 0000000..0f9a281 --- /dev/null +++ b/src/memory/sources/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::memory::sources::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/src/memory/sources/readers/github/git.rs b/src/memory/sources/readers/github/git.rs new file mode 100644 index 0000000..c3a60d2 --- /dev/null +++ b/src/memory/sources/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::memory::sources::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/src/memory/sources/readers/github/git_tests.rs b/src/memory/sources/readers/github/git_tests.rs new file mode 100644 index 0000000..9c1011a --- /dev/null +++ b/src/memory/sources/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/src/memory/sources/readers/github/issues.rs b/src/memory/sources/readers/github/issues.rs new file mode 100644 index 0000000..e70d752 --- /dev/null +++ b/src/memory/sources/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::memory::sources::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/src/memory/sources/readers/github/types.rs b/src/memory/sources/readers/github/types.rs new file mode 100644 index 0000000..11327e9 --- /dev/null +++ b/src/memory/sources/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/src/memory/sources/readers/github_tests.rs b/src/memory/sources/readers/github_tests.rs new file mode 100644 index 0000000..1d90ffc --- /dev/null +++ b/src/memory/sources/readers/github_tests.rs @@ -0,0 +1,293 @@ +use super::*; +use crate::memory::store::content::raw::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::memory::sources::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/src/memory/sources/readers/mod.rs b/src/memory/sources/readers/mod.rs index ab3ac03..3ec5a5e 100644 --- a/src/memory/sources/readers/mod.rs +++ b/src/memory/sources/readers/mod.rs @@ -6,24 +6,45 @@ //! //! ## Ownership boundary //! -//! Per the engine spec, TinyCortex does **not** own live sync, polling, or -//! OAuth. The network-backed kinds (`composio`, `github_repo`, `rss_feed`, -//! `web_page`, `twitter_query`) keep their type contracts and validation in -//! [`crate::memory::sources::types`] / [`crate::memory::sources::validation`], -//! but their live fetchers are host-owned and are deliberately not implemented -//! here. Only the local kinds — [`folder::FolderReader`] and -//! [`conversation::ConversationReader`] — ship real readers. +//! 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. //! -//! [`reader_for`] returns `Some` only for the locally-readable kinds; callers -//! handling a `None` should defer to the host's sync runner. +//! 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 +//! [`crate::memory::sync::workspace`] 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 = "sync")] +pub mod github; +#[cfg(feature = "sync")] +pub mod rss; +#[cfg(feature = "sync")] +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 = "sync")] +mod ssrf; use async_trait::async_trait; use crate::memory::config::MemoryConfig; use crate::memory::error::MemoryEngineResult; +#[cfg(feature = "sync")] +use crate::memory::error::MemoryError; use super::types::{MemorySourceEntry, SourceContent, SourceItem, SourceKind}; @@ -52,19 +73,21 @@ pub trait SourceReader: Send + Sync { ) -> MemoryEngineResult; } -/// Whether a kind has a local reader implemented in TinyCortex. +/// Whether a kind can be read from local state alone, with no network egress. /// -/// Network-backed kinds return `false`: their live fetchers are host-owned. +/// 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 local reader for a source kind, if one exists in TinyCortex. +/// Get the reader for a source kind that is safe to drive on a timer. /// /// Returns `Some` for [`SourceKind::Folder`] and [`SourceKind::Conversation`]. -/// For network-backed kinds (`composio`, `github_repo`, `rss_feed`, -/// `web_page`, `twitter_query`) this returns `None` — those are read by the -/// host's sync runner, not TinyCortex. +/// 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)), @@ -76,3 +99,14 @@ pub fn reader_for(kind: &SourceKind) -> Option> { | 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 = "sync")] +pub(crate) fn into_engine_error(message: String) -> MemoryError { + MemoryError::Other(anyhow::anyhow!(message)) +} diff --git a/src/memory/sources/readers/rss.rs b/src/memory/sources/readers/rss.rs new file mode 100644 index 0000000..8b7528b --- /dev/null +++ b/src/memory/sources/readers/rss.rs @@ -0,0 +1,397 @@ +//! 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::memory::config::MemoryConfig; +use crate::memory::error::MemoryEngineResult; +use crate::memory::sources::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); + +pub struct RssReader { + cache: Mutex>, +} + +impl RssReader { + 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, + config: &MemoryConfig, + ) -> MemoryEngineResult> { + self.list_items_inner(source, config) + .await + .map_err(into_engine_error) + } + + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + config: &MemoryConfig, + ) -> MemoryEngineResult { + self.read_item_inner(source, item_id, config) + .await + .map_err(into_engine_error) + } +} + +impl RssReader { + async fn list_items_inner( + &self, + source: &MemorySourceEntry, + _config: &MemoryConfig, + ) -> 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, + _config: &MemoryConfig, + ) -> 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/src/memory/sources/readers/rss/types.rs b/src/memory/sources/readers/rss/types.rs new file mode 100644 index 0000000..cf915c8 --- /dev/null +++ b/src/memory/sources/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 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 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/src/memory/sources/readers/rss_tests.rs b/src/memory/sources/readers/rss_tests.rs new file mode 100644 index 0000000..8f9e20b --- /dev/null +++ b/src/memory/sources/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/src/memory/sources/readers/ssrf.rs b/src/memory/sources/readers/ssrf.rs new file mode 100644 index 0000000..85a8e96 --- /dev/null +++ b/src/memory/sources/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/src/memory/sources/readers/ssrf_tests.rs b/src/memory/sources/readers/ssrf_tests.rs new file mode 100644 index 0000000..4f8189a --- /dev/null +++ b/src/memory/sources/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/src/memory/sources/readers/web_page.rs b/src/memory/sources/readers/web_page.rs new file mode 100644 index 0000000..12721c1 --- /dev/null +++ b/src/memory/sources/readers/web_page.rs @@ -0,0 +1,483 @@ +//! 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::memory::config::MemoryConfig; +use crate::memory::error::MemoryEngineResult; +use crate::memory::sources::types::{ + ContentType, MemorySourceEntry, SourceContent, SourceItem, SourceKind, +}; + +use super::{into_engine_error, SourceReader}; + +pub struct WebPageReader; + +#[async_trait] +impl SourceReader for WebPageReader { + fn kind(&self) -> SourceKind { + SourceKind::WebPage + } + + async fn list_items( + &self, + source: &MemorySourceEntry, + config: &MemoryConfig, + ) -> MemoryEngineResult> { + self.list_items_inner(source, config) + .await + .map_err(into_engine_error) + } + + async fn read_item( + &self, + source: &MemorySourceEntry, + item_id: &str, + config: &MemoryConfig, + ) -> MemoryEngineResult { + self.read_item_inner(source, item_id, config) + .await + .map_err(into_engine_error) + } +} + +impl WebPageReader { + async fn list_items_inner( + &self, + source: &MemorySourceEntry, + _config: &MemoryConfig, + ) -> 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, + _config: &MemoryConfig, + ) -> 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/src/memory/store/content/atomic.rs b/src/memory/store/content/atomic.rs index db22409..d154ce3 100644 --- a/src/memory/store/content/atomic.rs +++ b/src/memory/store/content/atomic.rs @@ -125,6 +125,8 @@ pub fn stage_summary_with_layout( scope_slug: &str, layout: SummaryDiskLayout<'_>, ) -> anyhow::Result { + super::ensure_obsidian_defaults_if_enabled(content_root); + let rel_path = summary_rel_path_with_layout( input.tree_kind, scope_slug, diff --git a/src/memory/store/content/content_tests.rs b/src/memory/store/content/content_tests.rs index 58eed69..1930a0c 100644 --- a/src/memory/store/content/content_tests.rs +++ b/src/memory/store/content/content_tests.rs @@ -82,6 +82,30 @@ fn stage_chunks_replaces_stale_on_disk_body() { ); } +#[cfg(feature = "obsidian")] +#[test] +fn stage_chunks_stages_obsidian_defaults_when_feature_enabled() { + // Every content-write route must drop the bundled `.obsidian/` defaults + // into a fresh content root so a user opening the vault gets the + // intended graph colour mapping without manual configuration. + let dir = TempDir::new().unwrap(); + let chunks = vec![sample_chunk(0)]; + stage_chunks(dir.path(), &chunks).unwrap(); + + let graph = dir.path().join(".obsidian").join("graph.json"); + let types = dir.path().join(".obsidian").join("types.json"); + assert!( + graph.exists(), + "graph.json should be staged into content root" + ); + assert!( + types.exists(), + "types.json should be staged into content root" + ); + let g = std::fs::read_to_string(&graph).unwrap(); + assert!(g.contains("colorGroups"), "graph.json missing colorGroups"); +} + #[test] fn stage_chunks_email_skips_disk_write() { let dir = TempDir::new().unwrap(); diff --git a/src/memory/store/content/mod.rs b/src/memory/store/content/mod.rs index 17f3e60..aabc380 100644 --- a/src/memory/store/content/mod.rs +++ b/src/memory/store/content/mod.rs @@ -13,26 +13,55 @@ //! - `read` — reads, SHA-256 verification, and front-matter splitting //! - `tags` — chunk-tag updates and Obsidian tag slugifiers //! - `raw` — verbatim per-item raw archive (`raw///…`) +//! - `obsidian` / `obsidian_registry` — Obsidian vault interop (`obsidian` +//! feature): stage bundled `.obsidian/` defaults, detect vault registration +//! - `wiki_git` — git-backed mirror of summary nodes (`wiki-git` feature) //! //! ## Deferred //! -//! The Obsidian-vault registry (`content::obsidian*`) and the git-backed wiki -//! mirror (`content::wiki_git`) pull host config and git surfaces beyond this -//! storage-primitive port; they are intentionally **not** ported here. The -//! Config/SQLite-aware high-level readers (`read_chunk_body`, summary tag +//! The Config/SQLite-aware high-level readers (`read_chunk_body`, summary tag //! rewrite, `stage_chunks` SQLite upsert) live with the chunk store. pub mod atomic; pub mod compose; +#[cfg(feature = "obsidian")] +pub mod obsidian; +#[cfg(feature = "obsidian")] +pub mod obsidian_registry; pub mod paths; pub mod raw; pub mod read; pub mod tags; +#[cfg(feature = "wiki-git")] +pub mod wiki_git; use std::path::Path; use crate::memory::chunks::{Chunk, SourceKind, StagedChunk}; +/// Best-effort stage of the bundled `.obsidian/` vault defaults into +/// `content_root`, no-op when the `obsidian` feature is disabled. +/// +/// Every content-write route (`stage_chunks`, `stage_summary*`, raw writes) +/// calls this before creating files, so a fresh content root gets its +/// `.obsidian/` directory on first write. The underlying helper is idempotent +/// (never overwrites an existing file) and never returns a hard error — a +/// failed stage logs a warning and is ignored so persistence is never aborted +/// over a cosmetic vault default. +#[cfg(feature = "obsidian")] +fn ensure_obsidian_defaults_if_enabled(content_root: &Path) { + if let Err(err) = obsidian::ensure_obsidian_defaults(content_root) { + log::warn!( + "[content_store] stage obsidian defaults failed at {:?}: {err:#}", + content_root + ); + } +} + +/// Feature-off twin: no-op so callers don't need `#[cfg]` per call site. +#[cfg(not(feature = "obsidian"))] +fn ensure_obsidian_defaults_if_enabled(_content_root: &Path) {} + pub use atomic::{stage_summary, stage_summary_with_layout, StagedSummary}; pub use compose::{ compose_chunk_file, compose_summary_md, rewrite_summary_tags, rewrite_tags, split_front_matter, @@ -64,6 +93,8 @@ pub use tags::{entity_tag, slugify_tag_kind, slugify_tag_value, update_chunk_tag /// per-message raw archive, so a `StagedChunk` row with an empty `content_path` /// is emitted and read paths fall back to the raw archive. pub fn stage_chunks(content_root: &Path, chunks: &[Chunk]) -> anyhow::Result> { + ensure_obsidian_defaults_if_enabled(content_root); + let mut staged = Vec::with_capacity(chunks.len()); for chunk in chunks { diff --git a/src/memory/store/content/obsidian.rs b/src/memory/store/content/obsidian.rs new file mode 100644 index 0000000..2bb3ac6 --- /dev/null +++ b/src/memory/store/content/obsidian.rs @@ -0,0 +1,100 @@ +//! Obsidian vault defaults. +//! +//! When the memory_tree content root is first populated we drop a small +//! `.obsidian/` directory into it so a user opening the vault gets the +//! intended graph-view colour mapping (one colour per summary level) and +//! the front-matter type hints (`time_range_*` as `date`, `sealed_at` as +//! `datetime`) without any manual configuration. +//! +//! The bundled defaults live as static files under `obsidian_defaults/` +//! and are baked into the binary via `include_str!`. We only stage them +//! when the corresponding `.obsidian/` doesn't already exist — +//! never overwrite a file the user has tweaked. +//! +//! Callers should invoke [`ensure_obsidian_defaults`] from any code path +//! that creates files under `content_root` (summary stage, raw write, +//! etc.). The function is idempotent and cheap on the steady-state path +//! (one `Path::exists()` per file). +//! +//! Failure mode: best-effort. A failed stage logs a warn and returns +//! `Ok(())` so seal/raw-write callers don't abort persistence over a +//! cosmetic vault default. + +use std::path::Path; + +use anyhow::Result; + +const GRAPH_JSON: &str = include_str!("obsidian_defaults/graph.json"); +const TYPES_JSON: &str = include_str!("obsidian_defaults/types.json"); + +/// Write the bundled `.obsidian/` defaults into `content_root` if they +/// aren't already there. Idempotent — never overwrites existing files. +pub fn ensure_obsidian_defaults(content_root: &Path) -> Result<()> { + let obsidian_dir = content_root.join(".obsidian"); + if let Err(err) = std::fs::create_dir_all(&obsidian_dir) { + log::warn!( + "[content_store::obsidian] create .obsidian dir failed at {:?}: {err:#} — skipping defaults", + obsidian_dir + ); + return Ok(()); + } + + write_default_if_missing(&obsidian_dir, "graph.json", GRAPH_JSON); + write_default_if_missing(&obsidian_dir, "types.json", TYPES_JSON); + Ok(()) +} + +fn write_default_if_missing(obsidian_dir: &Path, name: &str, body: &str) { + use std::io::{ErrorKind, Write}; + let target = obsidian_dir.join(name); + // `create_new(true)` makes existence-check + create atomic at the + // OS level, so a concurrent staging from another process can't + // race past `target.exists()` and clobber the winner. The + // AlreadyExists branch is the steady-state idempotent no-op. + let mut file = match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&target) + { + Ok(f) => f, + Err(err) if err.kind() == ErrorKind::AlreadyExists => return, + Err(err) => { + log::warn!( + "[content_store::obsidian] create default {} failed at {:?}: {err:#}", + name, + target + ); + return; + } + }; + match file.write_all(body.as_bytes()) { + Ok(()) => log::info!( + "[content_store::obsidian] staged default {} at {}", + name, + target.display() + ), + Err(err) => { + // `create_new` already produced an empty file at `target`; + // a write_all failure (disk full, transient I/O) leaves a + // truncated remnant. Without cleanup, the next call hits + // the AlreadyExists fast-path and never repairs the bad + // file. Remove it so the next call retries cleanly. + if let Err(cleanup_err) = std::fs::remove_file(&target) { + log::warn!( + "[content_store::obsidian] cleanup partial default {} failed at {:?}: {cleanup_err:#}", + name, + target + ); + } + log::warn!( + "[content_store::obsidian] write default {} failed at {:?}: {err:#}", + name, + target + ); + } + } +} + +#[cfg(test)] +#[path = "obsidian_tests.rs"] +mod tests; diff --git a/src/memory/store/content/obsidian_defaults/graph.json b/src/memory/store/content/obsidian_defaults/graph.json new file mode 100644 index 0000000..a582d66 --- /dev/null +++ b/src/memory/store/content/obsidian_defaults/graph.json @@ -0,0 +1,65 @@ +{ + "collapse-filter": false, + "search": "", + "showTags": false, + "showAttachments": false, + "hideUnresolved": true, + "showOrphans": true, + "collapse-color-groups": false, + "colorGroups": [ + { + "query": "path:L1", + "color": { + "a": 1, + "rgb": 14701138 + } + }, + { + "query": "path:L2", + "color": { + "a": 1, + "rgb": 14725458 + } + }, + { + "query": "path:L3", + "color": { + "a": 1, + "rgb": 11657298 + } + }, + { + "query": "path:L4", + "color": { + "a": 1, + "rgb": 5420768 + } + }, + { + "query": "path:L5", + "color": { + "a": 1, + "rgb": 5431504 + } + }, + { + "query": "path:L6", + "color": { + "a": 1, + "rgb": 14701261 + } + } + ], + "collapse-display": false, + "showArrow": false, + "textFadeMultiplier": 0.9, + "nodeSizeMultiplier": 1.34371527777778, + "lineSizeMultiplier": 1.44048177083333, + "collapse-forces": false, + "centerStrength": 0.493880208333333, + "repelStrength": 10, + "linkStrength": 1, + "linkDistance": 250, + "scale": 0.5443310539518227, + "close": false +} \ No newline at end of file diff --git a/src/memory/store/content/obsidian_defaults/types.json b/src/memory/store/content/obsidian_defaults/types.json new file mode 100644 index 0000000..34f5676 --- /dev/null +++ b/src/memory/store/content/obsidian_defaults/types.json @@ -0,0 +1,10 @@ +{ + "types": { + "aliases": "aliases", + "cssclasses": "multitext", + "tags": "tags", + "time_range_end": "date", + "time_range_start": "date", + "sealed_at": "datetime" + } +} \ No newline at end of file diff --git a/src/memory/store/content/obsidian_registry.rs b/src/memory/store/content/obsidian_registry.rs new file mode 100644 index 0000000..ff9f49a --- /dev/null +++ b/src/memory/store/content/obsidian_registry.rs @@ -0,0 +1,167 @@ +//! Obsidian vault-*registration* detection. +//! +//! Sibling to [`super::obsidian`] (which writes the `.obsidian/` *defaults* +//! into the content root). This module answers a different question: is the +//! content root actually a vault Obsidian knows about? +//! +//! `obsidian://open?path=` only resolves against vaults already recorded +//! in Obsidian's `obsidian.json` registry — it can **not** register a new +//! vault, and a `.obsidian/` folder on disk is not enough. So before the +//! Memory tab fires that deep link we check whether the content root (or an +//! ancestor) is a registered vault. If it isn't, the UI guides the user to add +//! it once ("Open folder as vault") instead of firing a link Obsidian rejects +//! with *"Unable to find a vault for the URL"*. +//! +//! Detection is **best-effort**: Obsidian can live in non-standard locations +//! (Flatpak, Snap, custom `$XDG_CONFIG_HOME`, portable). A negative result must +//! never block the user — the caller still offers "open anyway" + "reveal +//! folder" + a config-dir override that feeds back in here as `extra`. + +mod types; + +use std::path::{Path, PathBuf}; + +use types::ObsidianConfig; +pub use types::VaultRegistration; + +/// Candidate `obsidian.json` locations, in priority order. `extra` (a +/// user-supplied override pointing at Obsidian's *config dir*) is checked +/// first so a power user can correct a non-standard install. +fn candidate_config_files(extra: Option<&Path>) -> Vec { + let mut out = Vec::new(); + + if let Some(dir) = extra { + // Accept either the config dir itself or its parent (users often + // can't tell whether the path should end in `obsidian/`). + out.push(dir.join("obsidian.json")); + out.push(dir.join("obsidian").join("obsidian.json")); + } + + // Standard per-OS config dir: `~/.config` (Linux), `~/Library/Application + // Support` (macOS), `%APPDATA%` (Windows). + if let Some(cfg) = dirs::config_dir() { + out.push(cfg.join("obsidian").join("obsidian.json")); + } + + // Linux sandbox installs keep their own config tree. Harmless to probe on + // other OSes — the paths simply won't exist. + if let Some(home) = dirs::home_dir() { + out.push(home.join(".var/app/md.obsidian.Obsidian/config/obsidian/obsidian.json")); // Flatpak + out.push(home.join("snap/obsidian/current/.config/obsidian/obsidian.json")); + // Snap + } + + out +} + +/// Best-effort: is `content_root` (or an ancestor) a registered Obsidian +/// vault? `extra_config_dir` optionally points at Obsidian's config dir for +/// non-standard installs. Never errors — probe failures report +/// `registered = false`. +pub fn vault_registration_status( + content_root: &Path, + extra_config_dir: Option<&Path>, +) -> VaultRegistration { + registration_in_files(content_root, &candidate_config_files(extra_config_dir)) +} + +/// Core of [`vault_registration_status`], split out so tests can supply an +/// explicit, isolated set of `obsidian.json` paths instead of depending on +/// whatever Obsidian config happens to exist on the host. +fn registration_in_files(content_root: &Path, files: &[PathBuf]) -> VaultRegistration { + // Absolutize the content root against the current directory (lexically, + // without touching the filesystem) so a relative workspace/content_root + // — supported by `MemoryConfig::from_toml_file` — compares equal to the + // absolute vault paths Obsidian records in `obsidian.json`. Without this, + // a relative root can never be a prefix of an absolute vault path and an + // already-registered vault is always reported unregistered. + let target = std::path::absolute(content_root) + .map(|abs| lexically_normalize(&abs)) + .unwrap_or_else(|_| lexically_normalize(content_root)); + let mut config_found = false; + + for path in files { + let body = match std::fs::read_to_string(path) { + Ok(b) => b, + Err(_) => continue, // missing/unreadable candidate — try the next. + }; + config_found = true; + + let parsed: ObsidianConfig = match serde_json::from_str(&body) { + Ok(p) => p, + Err(err) => { + // Redact the path — it embeds the user's home/username. + log::warn!( + "[content_store::obsidian_registry] parse {} failed: {err} — skipping", + crate::memory::chunks::redact(&path.display().to_string()) + ); + continue; + } + }; + + for entry in parsed.vaults.values() { + let vault = lexically_normalize(Path::new(&entry.path)); + // A malformed/empty vault path normalizes to "" and would otherwise + // match every content root (empty ancestor ⊂ anything) — skip it. + if vault.as_os_str().is_empty() { + continue; + } + if is_ancestor_or_equal(&vault, &target) { + log::debug!( + "[content_store::obsidian_registry] content root is a registered vault \ + (matched in {})", + crate::memory::chunks::redact(&path.display().to_string()) + ); + return VaultRegistration { + registered: true, + config_found: true, + }; + } + } + } + + log::debug!( + "[content_store::obsidian_registry] content root NOT registered (config_found={})", + config_found + ); + VaultRegistration { + registered: false, + config_found, + } +} + +/// Strip trailing separators so `/a/b` and `/a/b/` compare equal. Lexical +/// only — we deliberately do not canonicalize: the vault path may be on an +/// unmounted volume or use a symlink, and canonicalize would error or rewrite +/// it. Both inputs come from trusted local sources, so a textual compare is +/// the safe, dependency-free choice. +fn lexically_normalize(p: &Path) -> PathBuf { + let s = p.to_string_lossy(); + let trimmed = s.trim_end_matches(['/', '\\']); + if trimmed.is_empty() { + // Was a pure root like "/" — keep it. + PathBuf::from(s.as_ref()) + } else { + PathBuf::from(trimmed) + } +} + +/// `true` when `ancestor == descendant`, or `ancestor` is a path-prefix of +/// `descendant` on component boundaries (so `/a/b` contains `/a/b/c` but not +/// `/a/bc`). Case-sensitive — adequate for the Linux target; a false negative +/// on case-insensitive volumes only makes detection conservative (the caller +/// still offers "open anyway"). +fn is_ancestor_or_equal(ancestor: &Path, descendant: &Path) -> bool { + let a: Vec<_> = ancestor.components().collect(); + let d: Vec<_> = descendant.components().collect(); + // An empty ancestor must not match (it would otherwise be a prefix of + // everything); also bail when the ancestor is longer than the descendant. + if a.is_empty() || a.len() > d.len() { + return false; + } + a.iter().zip(d.iter()).all(|(x, y)| x == y) +} + +#[cfg(test)] +#[path = "obsidian_registry_tests.rs"] +mod tests; diff --git a/src/memory/store/content/obsidian_registry/types.rs b/src/memory/store/content/obsidian_registry/types.rs new file mode 100644 index 0000000..36482dc --- /dev/null +++ b/src/memory/store/content/obsidian_registry/types.rs @@ -0,0 +1,33 @@ +//! Types for Obsidian vault-*registration* detection: the probe result and +//! the minimal `obsidian.json` shape. + +use std::collections::HashMap; + +use serde::Deserialize; + +/// Outcome of a registration probe. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VaultRegistration { + /// `true` when some registered Obsidian vault's path equals or is an + /// ancestor of the content root. + pub registered: bool, + /// `true` when at least one candidate `obsidian.json` was found/read (even + /// if parsing it later fails — see the parse-error branch, which still + /// counts the file as found). Lets the UI distinguish "Obsidian is set up, + /// vault just not added yet" from "couldn't find Obsidian at all" (offer + /// install vs. offer add-as-vault). + pub config_found: bool, +} + +/// Minimal shape of Obsidian's `obsidian.json`. We only need each vault's +/// `path`; `ts`/`open` and any future keys are ignored by `serde`. +#[derive(Debug, Deserialize)] +pub struct ObsidianConfig { + #[serde(default)] + pub vaults: HashMap, +} + +#[derive(Debug, Deserialize)] +pub struct VaultEntry { + pub path: String, +} diff --git a/src/memory/store/content/obsidian_registry_tests.rs b/src/memory/store/content/obsidian_registry_tests.rs new file mode 100644 index 0000000..c302d3a --- /dev/null +++ b/src/memory/store/content/obsidian_registry_tests.rs @@ -0,0 +1,155 @@ +use super::*; +use std::io::Write; + +/// Write an `obsidian.json` containing `vault_paths` and return its path. +fn write_config(dir: &Path, vault_paths: &[&str]) -> PathBuf { + let entries: Vec = vault_paths + .iter() + .enumerate() + .map(|(i, p)| { + format!( + "\"id{i}\": {{ \"path\": {}, \"ts\": 1700000000000, \"open\": true }}", + serde_json::to_string(p).unwrap() + ) + }) + .collect(); + let body = format!("{{ \"vaults\": {{ {} }} }}", entries.join(", ")); + let path = dir.join("obsidian.json"); + let mut f = std::fs::File::create(&path).unwrap(); + f.write_all(body.as_bytes()).unwrap(); + path +} + +#[test] +fn exact_match_is_registered() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("memory_tree/content"); + let cfg = write_config(tmp.path(), &[root.to_str().unwrap()]); + let got = registration_in_files(&root, &[cfg]); + assert_eq!( + got, + VaultRegistration { + registered: true, + config_found: true + } + ); +} + +#[test] +fn ancestor_vault_is_registered() { + // A vault rooted at the parent still "contains" the content root. + let tmp = tempfile::tempdir().unwrap(); + let parent = tmp.path().join("workspace"); + let root = parent.join("memory_tree/content"); + let cfg = write_config(tmp.path(), &[parent.to_str().unwrap()]); + assert!(registration_in_files(&root, &[cfg]).registered); +} + +#[test] +fn trailing_slash_does_not_matter() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("memory_tree/content"); + let with_slash = format!("{}/", root.to_str().unwrap()); + let cfg = write_config(tmp.path(), &[&with_slash]); + assert!(registration_in_files(&root, &[cfg]).registered); +} + +#[test] +fn relative_content_root_matches_absolute_vault() { + // `MemoryConfig::from_toml_file` accepts a relative workspace/content_root. + // The registry check must absolutize that root against the CWD before the + // prefix comparison, or an already-registered vault is always reported + // unregistered. + let tmp = tempfile::tempdir().unwrap(); + let cwd = std::env::current_dir().unwrap(); + let abs_root = cwd.join("memory_tree/content"); + let cfg = write_config(tmp.path(), &[abs_root.to_str().unwrap()]); + let got = registration_in_files(Path::new("memory_tree/content"), &[cfg]); + assert!( + got.registered, + "relative content root must be absolutized against the CWD before matching" + ); +} + +#[test] +fn unrelated_vault_is_not_registered_but_config_found() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("memory_tree/content"); + let cfg = write_config(tmp.path(), &["/some/other/vault"]); + let got = registration_in_files(&root, &[cfg]); + assert_eq!( + got, + VaultRegistration { + registered: false, + config_found: true + } + ); +} + +#[test] +fn empty_vault_path_does_not_match_every_root() { + // Regression: a malformed entry with an empty `path` must not + // normalize to "" and match every content root as an ancestor. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("memory_tree/content"); + let cfg = write_config(tmp.path(), &[""]); + let got = registration_in_files(&root, &[cfg]); + assert_eq!( + got, + VaultRegistration { + registered: false, + config_found: true + } + ); +} + +#[test] +fn sibling_prefix_is_not_a_false_match() { + // `/a/b/content` must NOT match a vault at `/a/b/content-archive`. + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("content"); + let decoy = format!("{}-archive", root.to_str().unwrap()); + let cfg = write_config(tmp.path(), &[&decoy]); + assert!(!registration_in_files(&root, &[cfg]).registered); +} + +#[test] +fn missing_config_reports_not_found() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("memory_tree/content"); + let missing = tmp.path().join("does-not-exist.json"); + let got = registration_in_files(&root, &[missing]); + assert_eq!( + got, + VaultRegistration { + registered: false, + config_found: false + } + ); +} + +#[test] +fn malformed_config_is_skipped_not_fatal() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("memory_tree/content"); + let bad = tmp.path().join("obsidian.json"); + std::fs::write(&bad, b"{ this is not json ").unwrap(); + // config_found is true (we read it) but parse fails → not registered. + let got = registration_in_files(&root, &[bad]); + assert_eq!( + got, + VaultRegistration { + registered: false, + config_found: true + } + ); +} + +#[test] +fn second_candidate_wins_when_first_missing() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join("memory_tree/content"); + let missing = tmp.path().join("nope.json"); + let real = write_config(tmp.path(), &[root.to_str().unwrap()]); + assert!(registration_in_files(&root, &[missing, real]).registered); +} diff --git a/src/memory/store/content/obsidian_tests.rs b/src/memory/store/content/obsidian_tests.rs new file mode 100644 index 0000000..2b4f376 --- /dev/null +++ b/src/memory/store/content/obsidian_tests.rs @@ -0,0 +1,43 @@ +use super::*; +use tempfile::TempDir; + +#[test] +fn stages_defaults_into_fresh_root() { + let tmp = TempDir::new().unwrap(); + ensure_obsidian_defaults(tmp.path()).unwrap(); + let graph = tmp.path().join(".obsidian").join("graph.json"); + let types = tmp.path().join(".obsidian").join("types.json"); + assert!(graph.exists(), "graph.json should be staged"); + assert!(types.exists(), "types.json should be staged"); + // Body must be the bundled content, not empty. + let g = std::fs::read_to_string(&graph).unwrap(); + assert!(g.contains("colorGroups"), "graph.json missing colorGroups"); +} + +#[test] +fn does_not_overwrite_existing_file() { + let tmp = TempDir::new().unwrap(); + let obs = tmp.path().join(".obsidian"); + std::fs::create_dir_all(&obs).unwrap(); + let graph = obs.join("graph.json"); + std::fs::write(&graph, r#"{"user":"custom"}"#).unwrap(); + + ensure_obsidian_defaults(tmp.path()).unwrap(); + + let body = std::fs::read_to_string(&graph).unwrap(); + assert_eq!( + body, r#"{"user":"custom"}"#, + "user-customised graph.json must not be clobbered" + ); +} + +#[test] +fn idempotent_second_call_is_no_op() { + let tmp = TempDir::new().unwrap(); + ensure_obsidian_defaults(tmp.path()).unwrap(); + ensure_obsidian_defaults(tmp.path()).unwrap(); + // Second call must succeed without panicking and must not have + // duplicated or grown the file. + let g = std::fs::read_to_string(tmp.path().join(".obsidian/graph.json")).unwrap(); + assert!(g.contains("colorGroups")); +} diff --git a/src/memory/store/content/raw.rs b/src/memory/store/content/raw.rs index d418cff..6dc5951 100644 --- a/src/memory/store/content/raw.rs +++ b/src/memory/store/content/raw.rs @@ -78,6 +78,8 @@ pub fn write_raw_items( if items.is_empty() { return Ok(0); } + super::ensure_obsidian_defaults_if_enabled(content_root); + let mut written = 0usize; for item in items { let dir = raw_kind_dir(content_root, source_id, item.kind); diff --git a/src/memory/store/content/wiki_git/mod.rs b/src/memory/store/content/wiki_git/mod.rs new file mode 100644 index 0000000..b380aba --- /dev/null +++ b/src/memory/store/content/wiki_git/mod.rs @@ -0,0 +1,379 @@ +//! Git history for derived wiki summary nodes. +//! +//! The repository lives at `/wiki/.git` and intentionally tracks +//! only summary-node markdown (`summaries/**`) plus its own restrictive +//! `.gitignore`. Raw source mirrors, chunk intermediates, Obsidian defaults, +//! and future non-summary wiki artifacts are left out of history. + +mod types; + +use std::path::{Path, PathBuf}; +use std::sync::Mutex; + +use anyhow::{Context, Result}; +use chrono::{DateTime, Utc}; +use git2::{ErrorCode, Oid, Repository, RepositoryOpenFlags, Signature}; + +use super::paths::WIKI_PREFIX; +pub use types::{SummaryCommitBatch, SummaryCommitEntry}; + +static WIKI_GIT_LOCK: Mutex<()> = Mutex::new(()); + +/// Acquire the process-wide wiki-git mutex, recovering from a poisoned lock. +/// +/// The mutex guards no data — it only serialises git operations. If a prior +/// call panicked while holding the guard, a plain `.lock().expect(...)` would +/// poison every later wiki-git operation for the process lifetime. Recovering +/// the guard via [`PoisonError::into_inner`] keeps later operations working. +fn lock_wiki_git() -> std::sync::MutexGuard<'static, ()> { + WIKI_GIT_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +const SIG_NAME: &str = "OpenHuman Memory"; +const SIG_EMAIL: &str = "memory-wiki@openhuman.local"; +const GITIGNORE_BODY: &str = "*\n!/.gitignore\n!/summaries/\n!/summaries/**\n"; + +/// Ensure the wiki repository exists and has a commit containing the supplied +/// summary files. Existing non-summary tracked entries are removed from the +/// index so history stays scoped to summary nodes only. +pub fn commit_summaries(content_root: &Path, batch: &SummaryCommitBatch) -> Result<()> { + if batch.entries.is_empty() { + return Ok(()); + } + let summary_repo_paths: Vec = batch + .entries + .iter() + .map(|entry| summary_repo_path(&entry.content_path)) + .collect::>>()?; + let _guard = lock_wiki_git(); + + let repo = open_prepared_repo(content_root)?; + let wiki_root = content_root.join(WIKI_PREFIX); + + let mut index = repo.index().context("open wiki git index")?; + prune_stale_or_non_summary_entries(&mut index, repo.workdir().unwrap_or(&wiki_root))?; + index + .add_path(Path::new(".gitignore")) + .context("stage wiki .gitignore")?; + for path in &summary_repo_paths { + index + .add_path(Path::new(path)) + .with_context(|| format!("stage wiki summary: {path}"))?; + } + stage_existing_summary_paths(&mut index, &wiki_root)?; + index + .write() + .context("write wiki git index after staging summary")?; + + commit_index_if_changed(&repo, batch) +} + +/// Add a timestamped lightweight git tag that represents a reader's high-water +/// mark, and move a stable `latest` alias for quick lookup. +/// +/// This writes `refs/tags/read//` +/// to `target_commit`, or to wiki `HEAD` when `target_commit` is `None`, and +/// also updates `refs/tags/read//latest`. Tags update read +/// state without creating another history commit. +pub fn set_read_pointer_tag( + content_root: &Path, + pointer_id: &str, + target_commit: Option<&str>, +) -> Result { + let _guard = lock_wiki_git(); + let repo = open_prepared_repo(content_root)?; + let oid = match target_commit { + Some(commit) => { + // Parse then resolve against this repo so an abbreviated, invalid, + // or foreign commit id fails here — before `repo.reference` writes + // a tag pointing at an object that doesn't exist in this repo. + let parsed = + Oid::from_str(commit).with_context(|| format!("bad commit id: {commit}"))?; + repo.find_commit(parsed) + .with_context(|| format!("commit not in wiki repo: {commit}"))? + .id() + } + None => repo.head()?.peel_to_commit()?.id(), + }; + let tag_ref = read_pointer_timestamp_ref(pointer_id, Utc::now()); + repo.reference(&tag_ref, oid, true, "advance memory wiki read pointer") + .with_context(|| format!("set wiki read pointer tag: {tag_ref}"))?; + let latest_ref = read_pointer_latest_ref(pointer_id); + repo.reference( + &latest_ref, + oid, + true, + "advance latest memory wiki read pointer", + ) + .with_context(|| format!("set latest wiki read pointer tag: {latest_ref}"))?; + log::debug!( + "[content_store::wiki_git] advanced read pointer tags {} latest={} -> {}", + tag_ref, + latest_ref, + oid + ); + Ok(oid.to_string()) +} + +/// Return the commit id a read-pointer tag currently references. +pub fn get_read_pointer_tag(content_root: &Path, pointer_id: &str) -> Result> { + let _guard = lock_wiki_git(); + let wiki_root = content_root.join(WIKI_PREFIX); + let repo = match open_existing_repo(&wiki_root) { + Ok(repo) => repo, + Err(err) if err.code() == ErrorCode::NotFound => return Ok(None), + Err(err) => return Err(err).context("open wiki git repo for read pointer"), + }; + let tag_ref = read_pointer_latest_ref(pointer_id); + let target = match repo.find_reference(&tag_ref) { + Ok(reference) => Ok(reference.target().map(|oid| oid.to_string())), + Err(err) if err.code() == ErrorCode::NotFound => Ok(None), + Err(err) => Err(err).with_context(|| format!("find wiki read pointer tag: {tag_ref}")), + }; + target +} + +fn open_prepared_repo(content_root: &Path) -> Result { + let wiki_root = content_root.join(WIKI_PREFIX); + std::fs::create_dir_all(&wiki_root) + .with_context(|| format!("create wiki git root: {}", wiki_root.display()))?; + + let repo = open_or_init_repo(&wiki_root)?; + ensure_gitignore(&wiki_root)?; + Ok(repo) +} + +fn open_or_init_repo(wiki_root: &Path) -> Result { + match open_existing_repo(wiki_root) { + Ok(repo) => Ok(repo), + Err(err) if err.code() == ErrorCode::NotFound => { + log::debug!( + "[content_store::wiki_git] initialising summary wiki git repo at {}", + wiki_root.display() + ); + Repository::init(wiki_root) + .with_context(|| format!("init wiki git repo: {}", wiki_root.display())) + } + Err(err) => { + Err(err).with_context(|| format!("open wiki git repo: {}", wiki_root.display())) + } + } +} + +fn open_existing_repo(wiki_root: &Path) -> Result { + Repository::open_ext( + wiki_root, + RepositoryOpenFlags::NO_SEARCH, + &[] as &[&std::ffi::OsStr], + ) +} + +fn ensure_gitignore(wiki_root: &Path) -> Result<()> { + let path = wiki_root.join(".gitignore"); + match std::fs::read_to_string(&path) { + Ok(existing) if existing == GITIGNORE_BODY => Ok(()), + _ => { + std::fs::write(&path, GITIGNORE_BODY) + .with_context(|| format!("write wiki gitignore: {}", path.display()))?; + log::debug!( + "[content_store::wiki_git] wrote summary-only .gitignore at {}", + path.display() + ); + Ok(()) + } + } +} + +fn prune_stale_or_non_summary_entries(index: &mut git2::Index, wiki_root: &Path) -> Result<()> { + let to_remove: Vec = index + .iter() + .filter_map(|entry| { + let path = std::str::from_utf8(&entry.path).ok()?; + if should_keep_index_entry(wiki_root, path) { + None + } else { + Some(PathBuf::from(path)) + } + }) + .collect(); + + for path in to_remove { + index + .remove_path(&path) + .with_context(|| format!("remove non-summary wiki git entry: {}", path.display()))?; + } + Ok(()) +} + +fn should_keep_index_entry(wiki_root: &Path, path: &str) -> bool { + if !is_tracked_wiki_path(path) { + return false; + } + path == ".gitignore" || wiki_root.join(path).exists() +} + +fn is_tracked_wiki_path(path: &str) -> bool { + path == ".gitignore" || path.starts_with("summaries/") +} + +fn stage_existing_summary_paths(index: &mut git2::Index, wiki_root: &Path) -> Result<()> { + let summaries_root = wiki_root.join("summaries"); + if !summaries_root.exists() { + return Ok(()); + } + stage_summary_dir(index, wiki_root, &summaries_root) +} + +fn stage_summary_dir(index: &mut git2::Index, wiki_root: &Path, dir: &Path) -> Result<()> { + for entry in + std::fs::read_dir(dir).with_context(|| format!("read summary dir: {}", dir.display()))? + { + let entry = entry.with_context(|| format!("read summary dir entry: {}", dir.display()))?; + // `DirEntry::file_type` does not follow symlinks. `Path::is_dir` / + // `is_file` would, so a symlink inside `summaries/` pointing at an + // ancestor directory would recurse without bound (stack overflow), and + // one pointing at a file outside the wiki root would be staged as + // content. Skipping symlinks entirely keeps the walk bounded. + let file_type = entry + .file_type() + .with_context(|| format!("stat summary dir entry: {}", entry.path().display()))?; + let path = entry.path(); + if file_type.is_dir() { + stage_summary_dir(index, wiki_root, &path)?; + } else if file_type.is_file() { + let repo_path = path + .strip_prefix(wiki_root) + .with_context(|| format!("summary path outside wiki root: {}", path.display()))?; + index + .add_path(repo_path) + .with_context(|| format!("stage existing wiki summary: {}", repo_path.display()))?; + } + } + Ok(()) +} + +fn commit_index_if_changed(repo: &Repository, batch: &SummaryCommitBatch) -> Result<()> { + let tree_oid = repo.index()?.write_tree()?; + let tree = repo.find_tree(tree_oid)?; + + let parent_commit = match repo.head() { + Ok(head) => Some(head.peel_to_commit()?), + // A freshly-initialised repo has an unborn HEAD (no commits yet) — the + // first commit has no parent. Any other `head()` failure is a real + // error (corrupt HEAD, missing ref) and must not be silently treated + // as "no parent". + Err(err) if err.code() == ErrorCode::UnbornBranch => None, + Err(err) => return Err(err).context("resolve wiki git HEAD"), + }; + + if let Some(parent) = &parent_commit { + if parent.tree_id() == tree_oid { + log::debug!( + "[content_store::wiki_git] summary wiki git clean after staging tree_id={} entries={}", + batch.tree_id, + batch.entries.len() + ); + return Ok(()); + } + } + + let sig = Signature::now(SIG_NAME, SIG_EMAIL).context("build wiki git signature")?; + let message = build_commit_message(batch); + let parents: Vec<&git2::Commit> = parent_commit.iter().collect(); + let commit_oid = repo + .commit(Some("HEAD"), &sig, &sig, &message, &tree, &parents) + .context("commit wiki summary update")?; + + log::debug!( + "[content_store::wiki_git] committed summary wiki update commit={} tree_id={} entries={}", + commit_oid, + batch.tree_id, + batch.entries.len() + ); + Ok(()) +} + +fn build_commit_message(batch: &SummaryCommitBatch) -> String { + let mut min_level = u32::MAX; + let mut max_level = 0; + let mut child_count = 0usize; + let mut token_count = 0u32; + let mut start: Option> = None; + let mut end: Option> = None; + + for entry in &batch.entries { + min_level = min_level.min(entry.level); + max_level = max_level.max(entry.level); + child_count = child_count.saturating_add(entry.child_count); + token_count = token_count.saturating_add(entry.token_count); + start = Some(start.map_or(entry.time_range_start, |s| s.min(entry.time_range_start))); + end = Some(end.map_or(entry.time_range_end, |e| e.max(entry.time_range_end))); + } + + let level_label = if min_level == max_level { + format!("L{min_level}") + } else { + format!("L{min_level}-L{max_level}") + }; + let title = format!( + "Seal memory tree {} {} summaries", + batch.tree_scope, level_label + ); + + let mut msg = String::new(); + msg.push_str(&title); + msg.push_str("\n\n"); + msg.push_str(&format!("Reason: {}\n", batch.reason)); + msg.push_str(&format!("Tree-Id: {}\n", batch.tree_id)); + msg.push_str(&format!("Tree-Scope: {}\n", batch.tree_scope)); + msg.push_str(&format!("Summary-Count: {}\n", batch.entries.len())); + msg.push_str(&format!("Level-Range: {level_label}\n")); + msg.push_str(&format!("Child-Count: {child_count}\n")); + msg.push_str(&format!("Token-Count: {token_count}\n")); + if let (Some(start), Some(end)) = (start, end) { + msg.push_str(&format!("Time-Range-Start: {}\n", start.to_rfc3339())); + msg.push_str(&format!("Time-Range-End: {}\n", end.to_rfc3339())); + } + msg.push_str("\nSummaries:\n"); + for entry in &batch.entries { + msg.push_str(&format!( + "- {} L{} children={} tokens={} path={}\n", + entry.summary_id, entry.level, entry.child_count, entry.token_count, entry.content_path + )); + } + msg +} + +fn summary_repo_path(summary_content_path: &str) -> Result { + let prefix = format!("{WIKI_PREFIX}/"); + let Some(repo_path) = summary_content_path.strip_prefix(&prefix) else { + anyhow::bail!( + "summary content path must live under {WIKI_PREFIX}/: {summary_content_path}" + ); + }; + if !repo_path.starts_with("summaries/") { + anyhow::bail!("wiki git only tracks summary nodes: {summary_content_path}"); + } + Ok(repo_path.to_string()) +} + +fn read_pointer_latest_ref(pointer_id: &str) -> String { + format!( + "refs/tags/read/{}/latest", + hex::encode(pointer_id.as_bytes()) + ) +} + +fn read_pointer_timestamp_ref(pointer_id: &str, timestamp: DateTime) -> String { + format!( + "refs/tags/read/{}/{}", + hex::encode(pointer_id.as_bytes()), + timestamp.format("%Y%m%dT%H%M%S%.9fZ") + ) +} + +#[cfg(test)] +#[path = "wiki_git_tests.rs"] +mod tests; diff --git a/src/memory/store/content/wiki_git/types.rs b/src/memory/store/content/wiki_git/types.rs new file mode 100644 index 0000000..c035c25 --- /dev/null +++ b/src/memory/store/content/wiki_git/types.rs @@ -0,0 +1,25 @@ +//! Data types for the wiki git mirror: the per-summary metadata carried in a +//! commit and the batch that ties a tree seal to its entries. + +use chrono::{DateTime, Utc}; + +/// Metadata for one summary node included in a wiki git commit. +#[derive(Clone, Debug)] +pub struct SummaryCommitEntry { + pub summary_id: String, + pub content_path: String, + pub level: u32, + pub child_count: usize, + pub token_count: u32, + pub time_range_start: DateTime, + pub time_range_end: DateTime, +} + +/// Metadata for one tree seal represented as a wiki git commit. +#[derive(Clone, Debug)] +pub struct SummaryCommitBatch { + pub reason: String, + pub tree_id: String, + pub tree_scope: String, + pub entries: Vec, +} diff --git a/src/memory/store/content/wiki_git/wiki_git_tests.rs b/src/memory/store/content/wiki_git/wiki_git_tests.rs new file mode 100644 index 0000000..e2f5d5b --- /dev/null +++ b/src/memory/store/content/wiki_git/wiki_git_tests.rs @@ -0,0 +1,403 @@ +use super::*; +use git2::IndexAddOption; +use tempfile::TempDir; + +#[test] +fn commit_summary_initializes_repo_and_tracks_only_summaries() { + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let summary = wiki.join("summaries/source-slack/L1/summary-1.md"); + let raw = wiki.join("raw/should-not-track.md"); + let note = wiki.join("notes/also-ignored.md"); + std::fs::create_dir_all(summary.parent().unwrap()).unwrap(); + std::fs::create_dir_all(raw.parent().unwrap()).unwrap(); + std::fs::create_dir_all(note.parent().unwrap()).unwrap(); + std::fs::write(&summary, "---\nkind: summary\n---\nbody").unwrap(); + std::fs::write(&raw, "raw").unwrap(); + std::fs::write(¬e, "note").unwrap(); + + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry( + "summary-1", + "wiki/summaries/source-slack/L1/summary-1.md", + )], + ), + ) + .unwrap(); + + let repo = Repository::open(&wiki).unwrap(); + let head = repo.head().unwrap().peel_to_commit().unwrap(); + let tree = head.tree().unwrap(); + assert!(tree.get_path(Path::new(".gitignore")).is_ok()); + assert!(tree + .get_path(Path::new("summaries/source-slack/L1/summary-1.md")) + .is_ok()); + assert!(tree.get_path(Path::new("raw/should-not-track.md")).is_err()); + assert!(tree.get_path(Path::new("notes/also-ignored.md")).is_err()); +} + +#[test] +fn commit_summary_prunes_existing_non_summary_tracked_entries() { + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + std::fs::create_dir_all(wiki.join("raw")).unwrap(); + std::fs::create_dir_all(wiki.join("summaries/source/L1")).unwrap(); + std::fs::write(wiki.join("raw/old.md"), "old raw").unwrap(); + std::fs::write(wiki.join("summaries/source/L1/new.md"), "new summary").unwrap(); + + let repo = Repository::init(&wiki).unwrap(); + let mut index = repo.index().unwrap(); + index + .add_all(["*"].iter(), IndexAddOption::DEFAULT, None) + .unwrap(); + index.write().unwrap(); + let tree_oid = index.write_tree().unwrap(); + let tree = repo.find_tree(tree_oid).unwrap(); + let sig = Signature::now(SIG_NAME, SIG_EMAIL).unwrap(); + repo.commit(Some("HEAD"), &sig, &sig, "old mixed commit", &tree, &[]) + .unwrap(); + + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("new", "wiki/summaries/source/L1/new.md")], + ), + ) + .unwrap(); + + let head = repo.head().unwrap().peel_to_commit().unwrap(); + let tree = head.tree().unwrap(); + assert!(tree + .get_path(Path::new("summaries/source/L1/new.md")) + .is_ok()); + assert!(tree.get_path(Path::new("raw/old.md")).is_err()); +} + +#[test] +fn commit_summary_opens_only_the_nested_wiki_repo() { + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let summary = wiki.join("summaries/source/L1/summary-1.md"); + std::fs::create_dir_all(summary.parent().unwrap()).unwrap(); + std::fs::write(&summary, "summary").unwrap(); + + let parent_repo = Repository::init(dir.path()).unwrap(); + + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("summary-1", "wiki/summaries/source/L1/summary-1.md")], + ), + ) + .unwrap(); + + let repo = Repository::open(&wiki).unwrap(); + let tree = repo + .head() + .unwrap() + .peel_to_commit() + .unwrap() + .tree() + .unwrap(); + assert!(tree + .get_path(Path::new("summaries/source/L1/summary-1.md")) + .is_ok()); + assert!( + parent_repo.head().is_err(), + "summary history should not mutate the parent repo" + ); +} + +#[test] +fn commit_summary_drops_deleted_summary_entries_from_the_index() { + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let old_summary = wiki.join("summaries/source/L1/old.md"); + let new_summary = wiki.join("summaries/source/L1/new.md"); + std::fs::create_dir_all(old_summary.parent().unwrap()).unwrap(); + std::fs::write(&old_summary, "old summary").unwrap(); + + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("old", "wiki/summaries/source/L1/old.md")], + ), + ) + .unwrap(); + + std::fs::remove_file(&old_summary).unwrap(); + std::fs::write(&new_summary, "new summary").unwrap(); + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("new", "wiki/summaries/source/L1/new.md")], + ), + ) + .unwrap(); + + let repo = Repository::open(&wiki).unwrap(); + let tree = repo + .head() + .unwrap() + .peel_to_commit() + .unwrap() + .tree() + .unwrap(); + assert!(tree + .get_path(Path::new("summaries/source/L1/new.md")) + .is_ok()); + assert!(tree + .get_path(Path::new("summaries/source/L1/old.md")) + .is_err()); +} + +#[test] +fn commit_summary_recovers_existing_uncommitted_summary_files() { + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let missed_summary = wiki.join("summaries/source/L1/missed.md"); + let new_summary = wiki.join("summaries/source/L1/new.md"); + std::fs::create_dir_all(missed_summary.parent().unwrap()).unwrap(); + std::fs::write(&missed_summary, "missed summary").unwrap(); + std::fs::write(&new_summary, "new summary").unwrap(); + + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("new", "wiki/summaries/source/L1/new.md")], + ), + ) + .unwrap(); + + let repo = Repository::open(&wiki).unwrap(); + let tree = repo + .head() + .unwrap() + .peel_to_commit() + .unwrap() + .tree() + .unwrap(); + assert!(tree + .get_path(Path::new("summaries/source/L1/new.md")) + .is_ok()); + assert!(tree + .get_path(Path::new("summaries/source/L1/missed.md")) + .is_ok()); +} + +#[test] +fn commit_summary_rejects_non_summary_paths() { + let dir = TempDir::new().unwrap(); + let err = commit_summaries( + dir.path(), + &batch("bad", vec![entry("bad", "wiki/notes/one.md")]), + ) + .unwrap_err(); + assert!(err.to_string().contains("only tracks summary nodes")); +} + +#[test] +fn commit_message_describes_seal_metadata() { + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let summary = wiki.join("summaries/source/L2/summary-2.md"); + std::fs::create_dir_all(summary.parent().unwrap()).unwrap(); + std::fs::write(&summary, "---\nkind: summary\n---\nbody").unwrap(); + + commit_summaries( + dir.path(), + &batch( + "sync_cascade", + vec![SummaryCommitEntry { + summary_id: "summary-2".to_string(), + content_path: "wiki/summaries/source/L2/summary-2.md".to_string(), + level: 2, + child_count: 7, + token_count: 123, + time_range_start: ts(1_700_000_000_000), + time_range_end: ts(1_700_003_600_000), + }], + ), + ) + .unwrap(); + + let repo = Repository::open(&wiki).unwrap(); + let head = repo.head().unwrap().peel_to_commit().unwrap(); + let msg = head.message().unwrap(); + assert!(msg.contains("Seal memory tree slack:#eng L2 summaries")); + assert!(msg.contains("Reason: sync_cascade")); + assert!(msg.contains("Summary-Count: 1")); + assert!(msg.contains("Child-Count: 7")); + assert!(msg.contains("Token-Count: 123")); + assert!(msg.contains("summary-2 L2 children=7 tokens=123")); +} + +#[test] +fn read_pointer_tags_are_timestamped_and_move_latest_without_new_commit() { + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let summary = wiki.join("summaries/source/L1/summary-1.md"); + std::fs::create_dir_all(summary.parent().unwrap()).unwrap(); + std::fs::write(&summary, "---\nkind: summary\n---\nbody").unwrap(); + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("summary-1", "wiki/summaries/source/L1/summary-1.md")], + ), + ) + .unwrap(); + + let repo = Repository::open(&wiki).unwrap(); + let head = repo.head().unwrap().peel_to_commit().unwrap(); + let head_id = head.id().to_string(); + + let tagged = set_read_pointer_tag(dir.path(), "agent:default", None).unwrap(); + assert_eq!(tagged, head_id); + assert_eq!( + get_read_pointer_tag(dir.path(), "agent:default") + .unwrap() + .as_deref(), + Some(head_id.as_str()) + ); + let tag_prefix = format!( + "refs/tags/read/{}/", + hex::encode("agent:default".as_bytes()) + ); + let tags = repo.references().unwrap().fold(Vec::new(), |mut acc, r| { + let r = r.unwrap(); + let name = r.name().unwrap(); + if name.starts_with(&tag_prefix) { + acc.push(name.to_string()); + } + acc + }); + assert!( + tags.iter().any(|name| name.ends_with("/latest")), + "latest read pointer tag should be present: {tags:?}" + ); + assert!( + tags.iter().any(|name| { + let suffix = name.strip_prefix(&tag_prefix).unwrap_or_default(); + suffix.len() == "20260626T045537.123456789Z".len() + && suffix.ends_with('Z') + && suffix.contains('T') + }), + "timestamped read pointer tag should be present: {tags:?}" + ); + let mut walk = repo.revwalk().unwrap(); + walk.push_head().unwrap(); + assert_eq!( + walk.count(), + 1, + "moving the read pointer must not create commits" + ); +} + +#[test] +fn lock_wiki_git_recovers_after_poison() { + // A panic while holding the guard poisons the mutex; a naive + // `.lock().expect(...)` would then permanently disable wiki history for + // the process lifetime. `lock_wiki_git` must recover the guard instead. + let handle = std::thread::spawn(|| { + let _guard = WIKI_GIT_LOCK.lock().unwrap(); + panic!("intentional poison"); + }); + assert!(handle.join().is_err(), "lock should have been poisoned"); + let _guard = lock_wiki_git(); +} + +#[test] +fn set_read_pointer_tag_rejects_commit_not_in_wiki_repo() { + // A well-formed Oid that doesn't exist in this repo must fail before + // `repo.reference` writes a tag pointing at nothing. + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let summary = wiki.join("summaries/source/L1/summary-1.md"); + std::fs::create_dir_all(summary.parent().unwrap()).unwrap(); + std::fs::write(&summary, "---\nkind: summary\n---\nbody").unwrap(); + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("summary-1", "wiki/summaries/source/L1/summary-1.md")], + ), + ) + .unwrap(); + + let foreign = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; + let err = set_read_pointer_tag(dir.path(), "agent:default", Some(foreign)).unwrap_err(); + assert!( + err.to_string().contains("commit not in wiki repo"), + "expected validation error, got: {err:#}" + ); +} + +#[cfg(unix)] +#[test] +fn stage_summary_skips_symlink_loops() { + // A symlink inside `summaries/` pointing back at an ancestor directory + // must be skipped (via DirEntry::file_type) rather than recursed into + // without bound, which would abort on stack overflow. + use std::os::unix::fs::symlink; + + let dir = TempDir::new().unwrap(); + let wiki = dir.path().join("wiki"); + let summary = wiki.join("summaries/source/L1/summary-1.md"); + std::fs::create_dir_all(summary.parent().unwrap()).unwrap(); + std::fs::write(&summary, "---\nkind: summary\n---\nbody").unwrap(); + symlink("summaries", wiki.join("summaries/loop")).unwrap(); + + commit_summaries( + dir.path(), + &batch( + "queued_seal", + vec![entry("summary-1", "wiki/summaries/source/L1/summary-1.md")], + ), + ) + .unwrap(); + + let repo = Repository::open(&wiki).unwrap(); + let head = repo.head().unwrap().peel_to_commit().unwrap(); + let tree = head.tree().unwrap(); + assert!(tree + .get_path(Path::new("summaries/source/L1/summary-1.md")) + .is_ok()); + assert!( + tree.get_path(Path::new("summaries/loop")).is_err(), + "symlink must not be staged as content" + ); +} + +fn batch(reason: &str, entries: Vec) -> SummaryCommitBatch { + SummaryCommitBatch { + reason: reason.to_string(), + tree_id: "tree-1".to_string(), + tree_scope: "slack:#eng".to_string(), + entries, + } +} + +fn entry(summary_id: &str, content_path: &str) -> SummaryCommitEntry { + SummaryCommitEntry { + summary_id: summary_id.to_string(), + content_path: content_path.to_string(), + level: 1, + child_count: 2, + token_count: 10, + time_range_start: ts(1_700_000_000_000), + time_range_end: ts(1_700_000_001_000), + } +} + +fn ts(ms: i64) -> DateTime { + DateTime::::from_timestamp_millis(ms).unwrap() +} diff --git a/src/memory/sync/composio/providers/common.rs b/src/memory/sync/composio/providers/common.rs index 78a7059..a0e5481 100644 --- a/src/memory/sync/composio/providers/common.rs +++ b/src/memory/sync/composio/providers/common.rs @@ -2,6 +2,19 @@ use serde_json::Value; use crate::memory::sync::traits::SkillDocument; +/// Walk a JSON document by dotted path and return the first non-empty scalar. +/// +/// # Not interchangeable with [`normalize::helpers::pick_str`] +/// +/// A second `pick_str` lives in [`normalize::helpers`], and the two differ. +/// This one resolves paths with [`Value::pointer`] (so a numeric segment +/// indexes into an array) and **coerces `Number` to its string form**; that +/// one walks with [`Value::get`] (objects only) and returns `None` for any +/// non-string leaf. Swapping one for the other changes what normalisers emit +/// for numeric fields. Keep them separate. +/// +/// [`normalize::helpers`]: super::normalize::helpers +/// [`normalize::helpers::pick_str`]: super::normalize::helpers::pick_str pub fn pick_str(value: &Value, paths: &[&str]) -> Option { paths.iter().find_map(|path| { let pointer = format!("/{}", path.replace('.', "/")); diff --git a/src/memory/sync/composio/providers/mod.rs b/src/memory/sync/composio/providers/mod.rs index 7ca905b..2aa7fe7 100644 --- a/src/memory/sync/composio/providers/mod.rs +++ b/src/memory/sync/composio/providers/mod.rs @@ -8,6 +8,7 @@ mod google_docs; mod google_drive; mod google_sheets; mod linear; +pub mod normalize; mod notion; mod outlook; mod slack; diff --git a/src/memory/sync/composio/providers/normalize/clickup.rs b/src/memory/sync/composio/providers/normalize/clickup.rs new file mode 100644 index 0000000..ae340b6 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/clickup.rs @@ -0,0 +1,133 @@ +//! ClickUp host normalization helpers — result extraction, task-title extraction, +//! and time utilities. +//! +//! ClickUp's REST API (and therefore Composio's wrapping of it) returns +//! task lists in a small handful of shapes depending on which endpoint +//! is called. The functions here walk the union of common shapes so the +//! provider doesn't have to branch per Composio envelope variant. + +use serde_json::Value; + +use super::helpers::pick_str; + +/// Walk the Composio response envelope for ClickUp task list results. +/// +/// ClickUp's "filtered team tasks" endpoint returns `{ "tasks": [...] }` +/// at the top level; Composio re-wraps the upstream payload under +/// `data` or `data.data` depending on the action. We probe each shape +/// in order and return the first array we find. +pub fn extract_tasks(data: &Value) -> Vec { + let candidates = [ + data.pointer("/data/tasks"), + data.pointer("/tasks"), + data.pointer("/data/data/tasks"), + data.pointer("/data/results"), + data.pointer("/results"), + data.pointer("/data/items"), + data.pointer("/items"), + ]; + for cand in candidates.into_iter().flatten() { + if let Some(arr) = cand.as_array() { + return arr.clone(); + } + } + Vec::new() +} + +/// Extract a human-readable title from a ClickUp task object. +/// +/// ClickUp tasks store the name at `name` (or `data.name` after Composio +/// envelope wrapping). When the name is missing we fall back to the +/// task ID so chunks remain identifiable. +pub fn extract_task_name(task: &Value) -> Option { + pick_str(task, &["name", "data.name", "title", "data.title"]) +} + +/// Extract a stable cursor timestamp (milliseconds since epoch as a +/// string) from a ClickUp task object. +/// +/// The ClickUp API returns `date_updated` as a stringified epoch ms +/// (e.g. `"1733412345678"`); we keep it as a string so lexicographic +/// comparison against the stored cursor remains valid as long as the +/// length doesn't change (it won't until year 33658). +pub fn extract_task_updated(task: &Value) -> Option { + pick_str( + task, + &[ + "date_updated", + "data.date_updated", + "updated_at", + "data.updated_at", + "dateUpdated", + "data.dateUpdated", + ], + ) +} + +/// Current wall-clock time in milliseconds since the UNIX epoch. +pub fn now_ms() -> u64 { + use std::time::{SystemTime, UNIX_EPOCH}; + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) +} + +/// Extract the authorized user's numeric ID from the +/// `CLICKUP_GET_AUTHORIZED_USER` response. +/// +/// Composio wraps the upstream `{"user": {"id": …}}` shape; this walker +/// is defensive against both raw and wrapped payloads. Returns the ID +/// as a string because `CLICKUP_GET_FILTERED_TEAM_TASKS` accepts the +/// `assignees` filter as a string array. +pub fn extract_user_id(data: &Value) -> Option { + let candidates = [ + data.pointer("/user/id"), + data.pointer("/data/user/id"), + data.pointer("/id"), + data.pointer("/data/id"), + ]; + for cand in candidates.into_iter().flatten() { + if let Some(n) = cand.as_u64() { + return Some(n.to_string()); + } + if let Some(n) = cand.as_i64() { + return Some(n.to_string()); + } + if let Some(s) = cand.as_str() { + let trimmed = s.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_string()); + } + } + } + None +} + +/// Extract a list of workspace (team) IDs from the +/// `CLICKUP_GET_AUTHORIZED_TEAMS_WORKSPACES` response. +/// +/// ClickUp returns `{"teams": [{"id": "...", "name": "..."}, …]}`. We +/// keep the IDs as strings — `CLICKUP_GET_FILTERED_TEAM_TASKS` requires +/// a `team_id` (string) argument. +pub fn extract_workspace_ids(data: &Value) -> Vec { + let candidates = [ + data.pointer("/teams"), + data.pointer("/data/teams"), + data.pointer("/workspaces"), + data.pointer("/data/workspaces"), + ]; + for cand in candidates.into_iter().flatten() { + if let Some(arr) = cand.as_array() { + return arr + .iter() + .filter_map(|t| pick_str(t, &["id", "team_id", "workspace_id"])) + .collect(); + } + } + Vec::new() +} + +#[cfg(test)] +#[path = "clickup_tests.rs"] +mod tests; diff --git a/src/memory/sync/composio/providers/normalize/clickup_tests.rs b/src/memory/sync/composio/providers/normalize/clickup_tests.rs new file mode 100644 index 0000000..14ec117 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/clickup_tests.rs @@ -0,0 +1,96 @@ +use super::*; +use serde_json::json; + +#[test] +fn extract_tasks_from_data_tasks() { + let data = json!({ "data": { "tasks": [{"id": "t1"}] } }); + assert_eq!(extract_tasks(&data).len(), 1); +} + +#[test] +fn extract_tasks_from_top_level_tasks() { + let data = json!({ "tasks": [{"id": "a"}, {"id": "b"}] }); + assert_eq!(extract_tasks(&data).len(), 2); +} + +#[test] +fn extract_tasks_empty_when_missing() { + let data = json!({ "foo": "bar" }); + assert!(extract_tasks(&data).is_empty()); +} + +#[test] +fn extract_task_name_from_top_level() { + let task = json!({ "id": "t1", "name": "Build feature X" }); + assert_eq!(extract_task_name(&task), Some("Build feature X".into())); +} + +#[test] +fn extract_task_name_falls_back_to_data_name() { + let task = json!({ "data": { "name": "Wrapped" } }); + assert_eq!(extract_task_name(&task), Some("Wrapped".into())); +} + +#[test] +fn extract_task_name_none_when_missing() { + let task = json!({ "id": "t1" }); + assert!(extract_task_name(&task).is_none()); +} + +#[test] +fn extract_task_updated_handles_string_form() { + let task = json!({ "date_updated": "1733412345678" }); + assert_eq!( + extract_task_updated(&task), + Some("1733412345678".to_string()) + ); +} + +#[test] +fn extract_task_updated_handles_nested_data() { + let task = json!({ "data": { "dateUpdated": "1700000000000" } }); + assert_eq!( + extract_task_updated(&task), + Some("1700000000000".to_string()) + ); +} + +#[test] +fn extract_user_id_handles_numeric_id() { + let data = json!({ "user": { "id": 12345 } }); + assert_eq!(extract_user_id(&data), Some("12345".to_string())); +} + +#[test] +fn extract_user_id_handles_wrapped_payload() { + let data = json!({ "data": { "user": { "id": "777" } } }); + assert_eq!(extract_user_id(&data), Some("777".to_string())); +} + +#[test] +fn extract_user_id_none_when_missing() { + let data = json!({ "foo": "bar" }); + assert!(extract_user_id(&data).is_none()); +} + +#[test] +fn extract_workspace_ids_from_teams_array() { + let data = json!({ + "teams": [ + { "id": "ws1", "name": "Personal" }, + { "id": "ws2", "name": "Acme" }, + ] + }); + assert_eq!(extract_workspace_ids(&data), vec!["ws1", "ws2"]); +} + +#[test] +fn extract_workspace_ids_empty_when_no_teams() { + let data = json!({ "foo": "bar" }); + assert!(extract_workspace_ids(&data).is_empty()); +} + +#[test] +fn now_ms_returns_nonzero() { + assert!(now_ms() > 0); +} diff --git a/src/memory/sync/composio/providers/normalize/github.rs b/src/memory/sync/composio/providers/normalize/github.rs new file mode 100644 index 0000000..393e93e --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/github.rs @@ -0,0 +1,130 @@ +//! GitHub host normalization helpers — result extraction, identity helpers, and time utilities. +//! +//! GitHub's REST API (proxied through Composio) returns search results and +//! authenticated-user payloads in a small number of shapes. The functions here +//! walk the union of common Composio envelope variants so the provider stays +//! clean and branch-free. + +use serde_json::Value; + +use super::helpers::pick_str; + +/// Walk the Composio response envelope for GitHub search issue results. +/// +/// `GITHUB_SEARCH_ISSUES_AND_PULL_REQUESTS` wraps GitHub's `GET /search/issues` response, which +/// returns `{"total_count": N, "items": [...]}`. Composio may re-wrap this under +/// `data` or `data.data`; we probe each shape in order. +pub fn extract_issues(data: &Value) -> Vec { + let candidates = [ + data.pointer("/data/items"), + data.pointer("/items"), + data.pointer("/data/data/items"), + data.pointer("/data/results"), + data.pointer("/results"), + ]; + for cand in candidates.into_iter().flatten() { + if let Some(arr) = cand.as_array() { + return arr.clone(); + } + } + Vec::new() +} + +/// Extract a stable, globally unique identifier for a GitHub issue or PR. +/// +/// GitHub's internal `id` field is a large integer unique across all issues +/// and PRs on github.com. We convert it to a string for use as a sync key. +/// Falls back to composing from `html_url` path if `id` is absent. +pub fn extract_issue_id(issue: &Value) -> Option { + // Primary: numeric internal GitHub ID. + if let Some(id) = issue.get("id").or_else(|| issue.pointer("/data/id")) { + if let Some(n) = id.as_u64() { + return Some(n.to_string()); + } + if let Some(s) = id.as_str() { + let trimmed = s.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_string()); + } + } + } + // Fallback: parse owner/repo/number from html_url path segments. + // URL shape: https://github.com/{owner}/{repo}/issues/{number} + if let Some(url) = pick_str(issue, &["html_url", "data.html_url", "url", "data.url"]) { + if let Some(slug) = github_url_to_slug(&url) { + return Some(slug); + } + } + None +} + +/// Build a human-readable document title for a GitHub issue/PR. +/// +/// Format: `GitHub: {owner}/{repo}#{number}: {title}`. +/// Falls back to just the title or a placeholder when fields are missing. +pub fn extract_issue_title(issue: &Value) -> Option { + let title = pick_str(issue, &["title", "data.title"])?; + + // Best-effort: extract owner/repo#N from html_url for the prefix. + let prefix = pick_str(issue, &["html_url", "data.html_url"]) + .and_then(|url| github_url_to_slug(&url)) + .unwrap_or_default(); + + if prefix.is_empty() { + Some(title) + } else { + Some(format!("GitHub: {prefix}: {title}")) + } +} + +/// Parse `https://github.com/{owner}/{repo}/issues/{number}` (or `/pull/`) +/// into `"{owner}/{repo}#{number}"`. Returns `None` for unrecognised shapes. +fn github_url_to_slug(url: &str) -> Option { + let segs: Vec<&str> = url.trim_end_matches('/').split('/').collect(); + // Minimum: ["https:", "", "github.com", owner, repo, "issues", number] + if segs.len() >= 7 { + let number = segs[segs.len() - 1]; + let _kind = segs[segs.len() - 2]; // "issues" or "pull" — ignored + let repo = segs[segs.len() - 3]; + let owner = segs[segs.len() - 4]; + if !owner.is_empty() && !repo.is_empty() && !number.is_empty() { + return Some(format!("{owner}/{repo}#{number}")); + } + } + None +} + +/// Extract the `updated_at` ISO 8601 timestamp from a GitHub issue. +/// +/// GitHub returns `updated_at` as `"2024-05-21T15:30:00Z"`. ISO 8601 strings +/// sort lexicographically, so we use them directly as the sync cursor. +pub fn extract_issue_updated_at(issue: &Value) -> Option { + pick_str( + issue, + &[ + "updated_at", + "data.updated_at", + "updatedAt", + "data.updatedAt", + ], + ) +} + +/// Extract the authenticated user's login handle from a +/// `GITHUB_GET_THE_AUTHENTICATED_USER` response. +pub fn extract_user_login(data: &Value) -> Option { + pick_str(data, &["login", "data.login"]) +} + +/// Current wall-clock time in milliseconds since the UNIX epoch. +pub fn now_ms() -> u64 { + use std::time::{SystemTime, UNIX_EPOCH}; + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) +} + +#[cfg(test)] +#[path = "github_tests.rs"] +mod tests; diff --git a/src/memory/sync/composio/providers/normalize/github_tests.rs b/src/memory/sync/composio/providers/normalize/github_tests.rs new file mode 100644 index 0000000..67cd3c5 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/github_tests.rs @@ -0,0 +1,118 @@ +use super::*; +use serde_json::json; + +#[test] +fn extract_issues_from_data_items() { + let data = json!({ "data": { "items": [{"id": 1}] } }); + assert_eq!(extract_issues(&data).len(), 1); +} + +#[test] +fn extract_issues_from_top_level_items() { + let data = json!({ "items": [{"id": 1}, {"id": 2}] }); + assert_eq!(extract_issues(&data).len(), 2); +} + +#[test] +fn extract_issues_empty_when_missing() { + let data = json!({ "foo": "bar" }); + assert!(extract_issues(&data).is_empty()); +} + +#[test] +fn extract_issue_id_from_numeric_field() { + let issue = json!({ "id": 123456789u64, "title": "Fix bug" }); + assert_eq!(extract_issue_id(&issue), Some("123456789".to_string())); +} + +#[test] +fn extract_issue_id_from_wrapped_data() { + let issue = json!({ "data": { "id": 99u64 } }); + assert_eq!(extract_issue_id(&issue), Some("99".to_string())); +} + +#[test] +fn extract_issue_id_falls_back_to_html_url() { + let issue = json!({ + "html_url": "https://github.com/owner/repo/issues/42" + }); + assert_eq!(extract_issue_id(&issue), Some("owner/repo#42".to_string())); +} + +#[test] +fn extract_issue_id_none_when_missing() { + let issue = json!({ "title": "No ID here" }); + assert!(extract_issue_id(&issue).is_none()); +} + +#[test] +fn extract_issue_title_builds_prefixed_title() { + let issue = json!({ + "id": 1u64, + "title": "Fix race condition", + "html_url": "https://github.com/acme/core/issues/99" + }); + assert_eq!( + extract_issue_title(&issue), + Some("GitHub: acme/core#99: Fix race condition".to_string()) + ); +} + +#[test] +fn extract_issue_title_returns_raw_title_when_no_url() { + let issue = json!({ "title": "Bare title" }); + assert_eq!(extract_issue_title(&issue), Some("Bare title".to_string())); +} + +#[test] +fn extract_issue_title_none_when_missing() { + let issue = json!({ "id": 1u64 }); + assert!(extract_issue_title(&issue).is_none()); +} + +#[test] +fn extract_issue_updated_at_from_top_level() { + let issue = json!({ "updated_at": "2024-05-21T15:30:00Z" }); + assert_eq!( + extract_issue_updated_at(&issue), + Some("2024-05-21T15:30:00Z".to_string()) + ); +} + +#[test] +fn extract_issue_updated_at_from_data_wrapper() { + let issue = json!({ "data": { "updated_at": "2023-01-01T00:00:00Z" } }); + assert_eq!( + extract_issue_updated_at(&issue), + Some("2023-01-01T00:00:00Z".to_string()) + ); +} + +#[test] +fn extract_issue_updated_at_none_when_missing() { + let issue = json!({ "id": 1u64 }); + assert!(extract_issue_updated_at(&issue).is_none()); +} + +#[test] +fn extract_user_login_from_top_level() { + let data = json!({ "login": "octocat" }); + assert_eq!(extract_user_login(&data), Some("octocat".to_string())); +} + +#[test] +fn extract_user_login_from_data_wrapper() { + let data = json!({ "data": { "login": "monalisa" } }); + assert_eq!(extract_user_login(&data), Some("monalisa".to_string())); +} + +#[test] +fn extract_user_login_none_when_missing() { + let data = json!({ "id": 1u64 }); + assert!(extract_user_login(&data).is_none()); +} + +#[test] +fn now_ms_returns_nonzero() { + assert!(now_ms() > 0); +} diff --git a/src/memory/sync/composio/providers/normalize/gmail_post_process.rs b/src/memory/sync/composio/providers/normalize/gmail_post_process.rs new file mode 100644 index 0000000..9404141 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/gmail_post_process.rs @@ -0,0 +1,489 @@ +//! Gmail-specific post-processing of Composio action responses. +//! +//! The upstream `GMAIL_FETCH_EMAILS` payload is extremely verbose +//! (full MIME tree under `payload.parts[]`, 50+ `Received:` headers, +//! display-layer noise the model never uses). This module rewrites +//! it into a slim envelope per message: +//! +//! ```json +//! { +//! "messages": [ +//! { +//! "id": "…", +//! "threadId": "…", +//! "subject": "…", +//! "from": "…", +//! "to": "…", +//! "date": "…", +//! "labels": ["INBOX", "UNREAD"], +//! "markdown": "…body…", +//! "attachments": [ { "filename": "...", "mimeType": "..." } ] +//! } +//! ], +//! "nextPageToken": "…", +//! "resultSizeEstimate": 201 +//! } +//! ``` +//! +//! ## Body source +//! +//! Composio's backend ships a +//! `markdownFormatted` field on the response envelope — one string +//! per tool call, pre-rendered with HTML stripped, URLs shortened, +//! footers removed, whitespace normalised. We split it per message +//! along `\n---\n` boundaries (with `## ` heading fallbacks) and +//! pin each slice to the corresponding entry in `messages[]` via +//! [`apply_response_level_markdown`]. The reshape's +//! `extract_markdown_body` then prefers that pinned field over +//! falling back to the upstream `messageText`. +//! +//! No in-house HTML→markdown conversion lives here anymore — the +//! backend does the cleaning. If `markdownFormatted` is absent for +//! a given response we fall through to whatever plain text the +//! upstream provided in `messageText`. +//! +//! Callers that need the raw Composio shape can pass `raw_html: +//! true` (or `rawHtml: true`) in the action arguments — this +//! short-circuits the reshape entirely. +//! +//! Only `GMAIL_FETCH_EMAILS` is reshaped today; other Gmail action +//! responses are passed through unchanged. When we add envelopes for +//! more slugs they should live in this file, branched from +//! [`post_process`]. + +use serde_json::{json, Map, Value}; + +/// Entry point called from `GmailProvider::post_process_action_result`. +/// +/// Dispatches on the Composio action slug. Unknown Gmail slugs fall +/// through to a no-op. +pub fn post_process(slug: &str, arguments: Option<&Value>, data: &mut Value) { + if is_raw_html_flag_set(arguments) { + tracing::debug!( + slug, + "[composio:gmail][post-process] raw_html flag set, passing through" + ); + return; + } + if slug == "GMAIL_FETCH_EMAILS" { + reshape_fetch_emails(data) + } +} + +/// Stash per-message slices of the response-level `markdownFormatted` +/// onto the corresponding entries inside `data.messages[]`. +/// +/// The Composio backend (tinyhumansai/backend#683) ships ONE +/// `markdownFormatted` string per tool call covering all messages — +/// already URL-shortened, footer-stripped, and whitespace-normalised. +/// To get per-email files in the raw archive we split that string +/// along section boundaries (`## ` headings or `---` rules) and pin +/// each slice to the message at the same index. `extract_markdown_body` +/// then prefers `msg.markdownFormatted` over re-decoding the MIME +/// tree. +/// +/// **Must be called BEFORE [`post_process`]** because `post_process` +/// reshapes `data` into the slim envelope; once `messages[]` carries +/// our slim shape the upstream message ordering is already locked in +/// but we may have lost original ordering signals if any. +/// +/// No-op when the slice count doesn't match `messages.len()` — we +/// can't safely align segments to messages without an exact match, +/// so we let `extract_markdown_body` fall through to its MIME path. +pub fn apply_response_level_markdown(data: &mut Value, top_md: &str) { + let trimmed = top_md.trim(); + if trimmed.is_empty() { + return; + } + let container = match data.get_mut("messages") { + Some(_) => data, + None => match data.get_mut("data").and_then(|v| v.as_object_mut()) { + Some(_) => data.get_mut("data").unwrap(), + None => { + tracing::debug!( + "[composio:gmail][post-process] apply_response_level_markdown: \ + no messages container in response — skipping" + ); + return; + } + }, + }; + let Some(messages) = container.get_mut("messages").and_then(|v| v.as_array_mut()) else { + return; + }; + let count = messages.len(); + if count == 0 { + return; + } + // Clone hints out of the messages array so the slice borrows + // don't conflict with the upcoming `messages.iter_mut()` mutation. + let hints: Vec = messages.clone(); + let Some(slices) = split_response_markdown_per_message_with_hint(trimmed, count, Some(&hints)) + else { + tracing::debug!( + messages = count, + md_len = trimmed.len(), + "[composio:gmail][post-process] could not split response-level markdownFormatted \ + into {count} slices — falling back to per-message MIME decode" + ); + return; + }; + for (msg, slice) in messages.iter_mut().zip(slices) { + if let Some(obj) = msg.as_object_mut() { + obj.insert("markdownFormatted".to_string(), Value::String(slice)); + } + } + tracing::debug!( + messages = count, + "[composio:gmail][post-process] stashed per-message markdownFormatted slices" + ); +} + +/// Split a top-level `markdownFormatted` string into per-message +/// segments. Returns `Some(slices)` only when the split yields +/// exactly `expected_count` entries — otherwise the format isn't one +/// of the patterns we know about and we let the caller fall back. +/// +/// Primary boundary is the `\n---\n` horizontal rule the backend +/// emits between messages (confirmed against real +/// `GMAIL_FETCH_EMAILS` output). H2/H3 headings are kept as +/// fallbacks for older renderings. The preamble (`# Inbox (N +/// messages)`-style intro, if present) is dropped — we accept +/// either `expected` parts (no preamble) or `expected + 1` +/// (preamble + N messages). +/// +/// `messages_hint` is the slim message array from the same response +/// — when present we use the per-message `subject` field to verify +/// each segment really does belong to the message at the same index. +/// Mismatches force a fallback so we never write a wrong-message body +/// to the raw archive. +pub fn split_response_markdown_per_message(md: &str, expected_count: usize) -> Option> { + split_response_markdown_per_message_with_hint(md, expected_count, None) +} + +pub fn split_response_markdown_per_message_with_hint( + md: &str, + expected_count: usize, + messages_hint: Option<&[Value]>, +) -> Option> { + if expected_count == 0 { + return None; + } + if expected_count == 1 { + return Some(vec![md.to_string()]); + } + + // Boundary patterns to try, in priority order. `\n---\n` is the + // confirmed marker; the heading variants stay as belt-and-braces + // for older / variant backend renderings. + let candidates: &[(&str, &str)] = &[ + ("\n---\n", "---\n"), + ("\n\n## ", "## "), + ("\n\n### ", "### "), + ("\n\n# ", "# "), + ("\n***\n", "***\n"), + ]; + + for (sep, prefix) in candidates { + let parts: Vec<&str> = md.split(sep).collect(); + let (drop_preamble, prepend_first) = if parts.len() == expected_count { + (false, false) // no preamble; first segment had no prefix + } else if parts.len() == expected_count + 1 { + (true, true) // preamble dropped; every kept segment had a prefix + } else { + continue; + }; + let segments: Vec = parts + .into_iter() + .skip(if drop_preamble { 1 } else { 0 }) + .enumerate() + .map(|(i, s)| { + if i == 0 && !prepend_first { + s.to_string() + } else { + format!("{prefix}{s}") + } + }) + .collect(); + + // Validate alignment against the JSON message array: every + // segment whose corresponding message has a non-empty subject + // must mention that subject somewhere in its body. If a single + // pair fails, we treat the split as unreliable and try the + // next pattern. Empty / null subjects skip validation (e.g. + // notification mails where the subject is ""). + if let Some(hints) = messages_hint { + if !validate_segments_against_hints(&segments, hints) { + tracing::debug!( + expected = expected_count, + sep = sep, + "[composio:gmail][post-process] split candidate failed subject check" + ); + continue; + } + } + return Some(segments); + } + None +} + +/// True if every (segment, message) pair where the message has a +/// non-empty subject contains that subject somewhere in the segment +/// (case-insensitive substring match — a defensive heuristic, not a +/// strict equality check, since the backend may format subjects +/// inside markdown links or with surrounding decoration). +fn validate_segments_against_hints(segments: &[String], hints: &[Value]) -> bool { + if segments.len() != hints.len() { + return false; + } + for (seg, hint) in segments.iter().zip(hints.iter()) { + let subject = hint + .get("subject") + .and_then(|v| v.as_str()) + .unwrap_or("") + .trim(); + if subject.is_empty() { + continue; + } + if !seg + .to_ascii_lowercase() + .contains(&subject.to_ascii_lowercase()) + { + return false; + } + } + true +} + +/// Returns true when the caller explicitly set `raw_html: true` (or the +/// camelCase `rawHtml: true`) in the `arguments` object. +fn is_raw_html_flag_set(arguments: Option<&Value>) -> bool { + let Some(obj) = arguments.and_then(|v| v.as_object()) else { + return false; + }; + obj.get("raw_html") + .or_else(|| obj.get("rawHtml")) + .and_then(|v| v.as_bool()) + .unwrap_or(false) +} + +/// Rewrite a `GMAIL_FETCH_EMAILS` `data` object in place into the slim +/// envelope documented at the module level. +/// +/// The Composio response can be shaped either as `{ messages, nextPageToken, ... }` +/// directly, or wrapped one level deeper under `{ data: { messages: … } }` +/// depending on backend version; we handle both. +fn reshape_fetch_emails(data: &mut Value) { + // Unwrap an optional `data:` envelope so downstream logic only has + // to deal with one shape. + let container = match data.get_mut("messages") { + Some(_) => data, + None => match data.get_mut("data").and_then(|v| v.as_object_mut()) { + Some(_) => data.get_mut("data").unwrap(), + None => return, + }, + }; + + let Some(obj) = container.as_object_mut() else { + return; + }; + + let raw_messages = obj + .remove("messages") + .and_then(|v| match v { + Value::Array(arr) => Some(arr), + _ => None, + }) + .unwrap_or_default(); + let next_page_token = obj.remove("nextPageToken").unwrap_or(Value::Null); + let result_size_estimate = obj.remove("resultSizeEstimate").unwrap_or(Value::Null); + + let messages: Vec = raw_messages.into_iter().map(reshape_message).collect(); + + let mut envelope = Map::new(); + envelope.insert("messages".into(), Value::Array(messages)); + if !next_page_token.is_null() { + envelope.insert("nextPageToken".into(), next_page_token); + } + if !result_size_estimate.is_null() { + envelope.insert("resultSizeEstimate".into(), result_size_estimate); + } + + *container = Value::Object(envelope); +} + +/// Parse an RFC 3339 or RFC 2822 date string into a UTC `DateTime`. +pub fn parse_email_date(date_str: &str) -> Option> { + date_str + .parse::>() + .or_else(|_| { + chrono::DateTime::parse_from_rfc2822(date_str).map(|d| d.with_timezone(&chrono::Utc)) + }) + .ok() +} + +const EMAIL_LOCAL_TIME_FMT: &str = "%Y-%m-%d %I:%M %p %:z"; + +/// Format a UTC `DateTime` in the given timezone. Returns `None` when the +/// formatted result is identical to the UTC rendering (no-op for UTC hosts). +pub fn format_at_tz( + utc: chrono::DateTime, + tz: &Tz, +) -> Option +where + Tz::Offset: std::fmt::Display, +{ + let local_dt = utc.with_timezone(tz); + let formatted = local_dt.format(EMAIL_LOCAL_TIME_FMT).to_string(); + + let utc_formatted = utc.format(EMAIL_LOCAL_TIME_FMT).to_string(); + if formatted == utc_formatted { + return None; + } + Some(formatted) +} + +/// Convert a UTC email timestamp string to a human-readable local-time string. +/// +/// Accepts RFC 3339 (`"2026-05-31T10:33:00Z"`) or RFC 2822 +/// (`"Sat, 31 May 2026 10:33:00 +0000"`) input. Returns a formatted string +/// in the host's local timezone, e.g. `"2026-05-31 05:33 AM -05:00"`, +/// so the agent can present local times without UTC arithmetic. +/// +/// The raw `date` field is always preserved alongside this field so +/// internal sorting, deduplication, and debugging remain UTC-based. +/// +/// Returns `None` when the input cannot be parsed or the output format +/// would be identical to the UTC input (no-op for UTC hosts). +pub fn format_email_local_time(date_str: &str) -> Option { + let utc = parse_email_date(date_str)?; + format_at_tz(utc, &chrono::Local) +} + +/// Map one raw Composio message object to its slim counterpart. +/// +/// Body source picked by [`extract_markdown_body`]: +/// 1. The per-message `markdownFormatted` slice pinned by +/// [`apply_response_level_markdown`] (preferred — backend-rendered). +/// 2. The upstream `messageText` plaintext (fallback). +/// 3. Empty string. +fn reshape_message(raw: Value) -> Value { + let Value::Object(obj) = raw else { + return raw; + }; + + let id = obj.get("messageId").cloned().unwrap_or(Value::Null); + let thread_id = obj.get("threadId").cloned().unwrap_or(Value::Null); + let subject = obj.get("subject").cloned().unwrap_or(Value::Null); + let sender = obj.get("sender").cloned().unwrap_or(Value::Null); + let to = obj.get("to").cloned().unwrap_or(Value::Null); + let date = obj + .get("messageTimestamp") + .cloned() + .or_else(|| pick_header(&obj, "Date")) + .unwrap_or(Value::Null); + let labels = obj + .get("labelIds") + .cloned() + .unwrap_or_else(|| Value::Array(Vec::new())); + let list_unsubscribe = pick_header(&obj, "List-Unsubscribe").unwrap_or(Value::Null); + + let markdown = extract_markdown_body(&obj); + let attachments = extract_attachments(&obj); + + // Compute a local-time representation of the UTC `date` so the agent + // presents times in the user's timezone rather than quoting raw UTC. + let date_local = date.as_str().and_then(format_email_local_time); + + let mut out = Map::new(); + out.insert("id".into(), id); + out.insert("threadId".into(), thread_id); + out.insert("subject".into(), subject); + out.insert("from".into(), sender); + out.insert("to".into(), to); + out.insert("date".into(), date); + if let Some(local) = date_local { + out.insert("date_local".into(), Value::String(local)); + } + out.insert("labels".into(), labels); + if !list_unsubscribe.is_null() { + out.insert("list_unsubscribe".into(), list_unsubscribe); + } + out.insert("markdown".into(), Value::String(markdown)); + if !attachments.is_empty() { + out.insert("attachments".into(), Value::Array(attachments)); + } + Value::Object(out) +} + +/// Find a header value by (case-insensitive) name in the Composio +/// `payload.headers[]` array. Returns `Some(Value::String)` on hit. +fn pick_header(msg: &Map, name: &str) -> Option { + let headers = msg.get("payload")?.get("headers")?.as_array()?; + for h in headers { + let hn = h.get("name").and_then(|v| v.as_str()).unwrap_or(""); + if hn.eq_ignore_ascii_case(name) { + if let Some(v) = h.get("value").and_then(|v| v.as_str()) { + return Some(Value::String(v.to_string())); + } + } + } + None +} + +/// Pick a body for the slim envelope. +/// +/// We trust the Composio backend's pre-rendered `markdownFormatted` +/// (set per-message by [`apply_response_level_markdown`] from the +/// response-level field). When that's absent we fall back to the +/// upstream's plain-text `messageText` verbatim — no in-house +/// HTML→markdown decoding lives here anymore. The backend already +/// strips HTML, shortens URLs, and normalises whitespace; running +/// our own pipeline on top duplicated work and corrupted some +/// renderings. +fn extract_markdown_body(msg: &Map) -> String { + if let Some(formatted) = msg + .get("markdownFormatted") + .or_else(|| msg.get("markdown_formatted")) + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + { + return formatted.to_string(); + } + if let Some(text) = msg + .get("messageText") + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + { + return text.to_string(); + } + String::new() +} + +/// Pull a minimal attachments descriptor from the Composio +/// `attachmentList` array. +fn extract_attachments(msg: &Map) -> Vec { + if let Some(list) = msg.get("attachmentList").and_then(|v| v.as_array()) { + return list + .iter() + .filter_map(|a| { + let filename = a.get("filename").and_then(|v| v.as_str())?; + if filename.is_empty() { + return None; + } + let mime = a + .get("mimeType") + .and_then(|v| v.as_str()) + .unwrap_or_default(); + Some(json!({ "filename": filename, "mimeType": mime })) + }) + .collect(); + } + Vec::new() +} + +#[cfg(test)] +#[path = "gmail_post_process_tests.rs"] +mod tests; diff --git a/src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs b/src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs new file mode 100644 index 0000000..a143e95 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/gmail_post_process_tests.rs @@ -0,0 +1,354 @@ +use super::*; +use serde_json::json; + +fn fixture_with_backend_markdown() -> Value { + json!({ + "messages": [ + { + "messageId": "m1", + "threadId": "t1", + "subject": "Hello", + "sender": "a@x.com", + "to": "b@y.com", + "messageTimestamp": "2026-04-17T12:00:00Z", + "labelIds": ["INBOX", "UNREAD"], + // Pre-rendered slice (set by `apply_response_level_markdown` + // in production; inline here for the reshape test). + "markdownFormatted": "# Hello\n\nbody copy", + "messageText": "fallback should not be used", + "display_url": "ignore-me", + "preview": { "body": "Hi plain", "subject": "Hello" }, + "attachmentList": [ + { "filename": "report.pdf", "mimeType": "application/pdf", "size": 12345 }, + { "filename": "", "mimeType": "text/html" } + ], + "payload": {} + } + ], + "nextPageToken": "tok-1", + "resultSizeEstimate": 42 + }) +} + +#[test] +fn reshape_emits_slim_envelope() { + let mut v = fixture_with_backend_markdown(); + post_process("GMAIL_FETCH_EMAILS", None, &mut v); + + assert_eq!(v["nextPageToken"], "tok-1"); + assert_eq!(v["resultSizeEstimate"], 42); + + let msgs = v["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + let m = &msgs[0]; + + assert_eq!(m["id"], "m1"); + assert_eq!(m["threadId"], "t1"); + assert_eq!(m["subject"], "Hello"); + assert_eq!(m["from"], "a@x.com"); + assert_eq!(m["to"], "b@y.com"); + assert_eq!(m["date"], "2026-04-17T12:00:00Z"); + assert_eq!(m["labels"], json!(["INBOX", "UNREAD"])); + + let md = m["markdown"].as_str().unwrap(); + assert_eq!(md, "# Hello\n\nbody copy"); + + // Noise fields removed. + assert!(m.get("display_url").is_none()); + assert!(m.get("preview").is_none()); + assert!(m.get("payload").is_none()); + assert!(m.get("messageText").is_none()); + + // Attachments: empty filename entry is filtered. + let atts = m["attachments"].as_array().unwrap(); + assert_eq!(atts.len(), 1); + assert_eq!(atts[0]["filename"], "report.pdf"); + assert_eq!(atts[0]["mimeType"], "application/pdf"); +} + +#[test] +fn raw_html_flag_passes_through_unchanged() { + let mut v = fixture_with_backend_markdown(); + let original = v.clone(); + let args = json!({ "raw_html": true }); + post_process("GMAIL_FETCH_EMAILS", Some(&args), &mut v); + assert_eq!( + v, original, + "raw_html=true must preserve the Composio shape" + ); +} + +#[test] +fn camel_case_raw_html_also_recognized() { + let mut v = fixture_with_backend_markdown(); + let original = v.clone(); + let args = json!({ "rawHtml": true }); + post_process("GMAIL_FETCH_EMAILS", Some(&args), &mut v); + assert_eq!(v, original); +} + +#[test] +fn falls_back_to_message_text_when_no_backend_markdown() { + let mut v = json!({ + "messages": [{ + "messageId": "m1", + "threadId": "t1", + "subject": "s", + "sender": "a@x.com", + "to": "b@y.com", + "messageTimestamp": "2026-04-17", + "labelIds": [], + "messageText": " plain body text ", + "payload": {} + }], + "nextPageToken": null + }); + post_process("GMAIL_FETCH_EMAILS", None, &mut v); + let md = v["messages"][0]["markdown"].as_str().unwrap(); + assert_eq!(md, "plain body text"); + assert!(v.get("nextPageToken").is_none(), "null tokens dropped"); +} + +#[test] +fn unwraps_data_envelope() { + let mut v = json!({ + "data": { + "messages": [{ + "messageId": "m1", + "threadId": "t1", + "subject": "s", + "sender": "a@x.com", + "to": "b@y.com", + "messageTimestamp": "2026-04-17", + "labelIds": [], + "messageText": "body", + "payload": {} + }] + } + }); + post_process("GMAIL_FETCH_EMAILS", None, &mut v); + // Reshape writes into `data` in place. + let msgs = v["data"]["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["markdown"], "body"); +} + +#[test] +fn non_fetch_slug_is_noop() { + let mut v = json!({ "messages": [{ "messageId": "m1", "messageText": "x" }] }); + let original = v.clone(); + post_process("GMAIL_SEND_EMAIL", None, &mut v); + assert_eq!(v, original); +} + +#[test] +fn prefers_backend_markdown_formatted_when_present() { + // Composio backend (tinyhumansai/backend#683 +) ships + // `markdownFormatted` already URL-shortened + footer-stripped + // per message (after `apply_response_level_markdown` slices the + // response-level field). When present, our post-processor must + // use it verbatim instead of falling back to `messageText`. + let mut v = json!({ + "messages": [{ + "messageId": "m1", + "threadId": "t1", + "subject": "s", + "sender": "a@x.com", + "to": "b@y.com", + "messageTimestamp": "2026-04-17", + "labelIds": [], + "markdownFormatted": "# Already nice\n\nShort URL: https://gh.io/abc", + "messageText": "fallback should not be used", + "payload": {} + }] + }); + post_process("GMAIL_FETCH_EMAILS", None, &mut v); + let md = v["messages"][0]["markdown"].as_str().unwrap(); + assert_eq!(md, "# Already nice\n\nShort URL: https://gh.io/abc"); +} + +#[test] +fn empty_markdown_formatted_falls_through_to_message_text() { + let mut v = json!({ + "messages": [{ + "messageId": "m1", + "threadId": "t1", + "subject": "s", + "sender": "a@x.com", + "to": "b@y.com", + "messageTimestamp": "2026-04-17", + "labelIds": [], + "markdownFormatted": " \n \n", + "messageText": "real body", + "payload": {} + }] + }); + post_process("GMAIL_FETCH_EMAILS", None, &mut v); + let md = v["messages"][0]["markdown"].as_str().unwrap(); + assert!(md.contains("real body")); +} + +// ── split_response_markdown_per_message ───────────────────────────────── + +#[test] +fn split_response_markdown_uses_horizontal_rule_marker() { + // The confirmed backend marker is `\n---\n`. Three messages → + // expect three slices when there's no preamble. + let md = "## Alice's update\n\nbody A with https://gh.io/abc\n---\n## Bob's reply\n\nbody B\n---\n## Carol\n\nbody C"; + let slices = super::split_response_markdown_per_message(md, 3).unwrap(); + assert_eq!(slices.len(), 3); + assert!(slices[0].contains("Alice's update")); + assert!(slices[1].contains("Bob's reply")); + assert!(slices[2].contains("Carol")); + // The `---\n` prefix is preserved on every-but-the-first segment + // so the section break survives the round-trip. + assert!(slices[1].starts_with("---\n")); + assert!(slices[2].starts_with("---\n")); +} + +#[test] +fn split_response_markdown_drops_preamble() { + // When a preamble like `# Inbox` precedes the first marker, we + // see N+1 parts after split — the preamble must be dropped. + let md = "# Inbox (2 messages)\n---\n## A\n\nbody A\n---\n## B\n\nbody B"; + let slices = super::split_response_markdown_per_message(md, 2).unwrap(); + assert_eq!(slices.len(), 2); + assert!(slices[0].contains("body A")); + assert!(slices[1].contains("body B")); + // Both segments should carry the prefix when preamble was dropped. + assert!(slices[0].starts_with("---\n")); + assert!(slices[1].starts_with("---\n")); +} + +#[test] +fn split_response_markdown_falls_back_to_h2_marker() { + // No `---` rules — backend used h2 headings as boundaries. + let md = "## Alice\n\nbody A\n\n## Bob\n\nbody B"; + let slices = super::split_response_markdown_per_message(md, 2).unwrap(); + assert_eq!(slices.len(), 2); + assert!(slices[0].contains("body A")); + assert!(slices[1].contains("body B")); +} + +#[test] +fn split_response_markdown_returns_none_on_count_mismatch() { + let md = "## only one section here"; + assert!(super::split_response_markdown_per_message(md, 3).is_none()); +} + +#[test] +fn split_response_markdown_single_message_returns_whole_input() { + let md = "## solo\n\nthe whole body"; + let slices = super::split_response_markdown_per_message(md, 1).unwrap(); + assert_eq!(slices, vec![md.to_string()]); +} + +#[test] +fn split_with_hint_rejects_when_subjects_dont_match() { + let md = "## Foo\nbody1\n---\n## Bar\nbody2"; + let hints = vec![ + json!({"subject": "Completely different subject A"}), + json!({"subject": "Completely different subject B"}), + ]; + let out = super::split_response_markdown_per_message_with_hint(md, 2, Some(&hints)); + assert!(out.is_none(), "subject mismatch must force fallback"); +} + +#[test] +fn split_with_hint_accepts_when_subjects_match() { + let md = "## Welcome to Gmail\nbody1\n---\n## Your invoice\nbody2"; + let hints = vec![ + json!({"subject": "Welcome to Gmail"}), + json!({"subject": "Your invoice"}), + ]; + let slices = super::split_response_markdown_per_message_with_hint(md, 2, Some(&hints)).unwrap(); + assert_eq!(slices.len(), 2); + assert!(slices[0].contains("Welcome to Gmail")); + assert!(slices[1].contains("Your invoice")); +} + +#[test] +fn split_with_hint_skips_messages_with_blank_subject() { + let md = "## A\nbody1\n---\n## B\nbody2"; + let hints = vec![json!({"subject": "A"}), json!({"subject": ""})]; + let slices = super::split_response_markdown_per_message_with_hint(md, 2, Some(&hints)).unwrap(); + assert_eq!(slices.len(), 2); +} + +// ── format_email_local_time ────────────────────────────────────────────────── + +#[test] +fn format_email_local_time_returns_none_for_unparseable_date() { + assert!(super::format_email_local_time("not-a-date").is_none()); + assert!(super::format_email_local_time("").is_none()); +} + +#[test] +fn format_email_local_time_preserves_utc_raw_date_in_reshape() { + let mut v = json!({ + "messages": [{ + "messageId": "m1", + "threadId": "t1", + "subject": "Test", + "sender": "a@example.com", + "to": "b@example.com", + "messageTimestamp": "2026-05-31T10:33:00Z", + "labelIds": [], + "messageText": "body", + "payload": {} + }] + }); + post_process("GMAIL_FETCH_EMAILS", None, &mut v); + let msg = &v["messages"][0]; + assert_eq!(msg["date"], "2026-05-31T10:33:00Z"); +} + +#[test] +fn parse_email_date_accepts_rfc3339_and_rfc2822() { + assert!(super::parse_email_date("2026-05-31T10:33:00Z").is_some()); + assert!(super::parse_email_date("Sun, 31 May 2026 10:33:00 +0000").is_some()); + assert!(super::parse_email_date("not-a-date").is_none()); +} + +#[test] +fn format_at_tz_deterministic_with_fixed_offset() { + use chrono::FixedOffset; + + let utc = super::parse_email_date("2026-05-31T10:33:00Z").unwrap(); + + let est = FixedOffset::west_opt(5 * 3600).unwrap(); + let result = super::format_at_tz(utc, &est).unwrap(); + assert_eq!(result, "2026-05-31 05:33 AM -05:00"); + + let ist = FixedOffset::east_opt(5 * 3600 + 1800).unwrap(); + let result = super::format_at_tz(utc, &ist).unwrap(); + assert_eq!(result, "2026-05-31 04:03 PM +05:30"); +} + +#[test] +fn format_at_tz_returns_none_for_utc() { + let utc = super::parse_email_date("2026-05-31T10:33:00Z").unwrap(); + let utc_tz = chrono::FixedOffset::east_opt(0).unwrap(); + assert!(super::format_at_tz(utc, &utc_tz).is_none()); +} + +#[test] +fn apply_response_level_markdown_stashes_per_message_field() { + let mut data = json!({ + "messages": [ + {"messageId": "m1", "subject": "Hello"}, + {"messageId": "m2", "subject": "World"}, + ] + }); + let top_md = "## Hello\nbody A — link https://gh.io/abc\n---\n## World\nbody B"; + super::apply_response_level_markdown(&mut data, top_md); + let m1 = data["messages"][0]["markdownFormatted"].as_str().unwrap(); + let m2 = data["messages"][1]["markdownFormatted"].as_str().unwrap(); + assert!(m1.contains("Hello")); + assert!( + m1.contains("https://gh.io/abc"), + "shortened URL must survive" + ); + assert!(m2.contains("World")); + assert!(!m1.contains("World"), "no cross-message bleed"); +} diff --git a/src/memory/sync/composio/providers/normalize/helpers.rs b/src/memory/sync/composio/providers/normalize/helpers.rs new file mode 100644 index 0000000..101239e --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/helpers.rs @@ -0,0 +1,50 @@ +//! Shared helpers for the provider normalisers in this module. + +/// Walk a JSON object using a list of dotted-path candidates and return the +/// first non-empty **string** match. +/// +/// # This is deliberately NOT `super::super::common::pick_str` +/// +/// The crate carries two `pick_str` functions with the same name and +/// genuinely different behaviour. Do not "deduplicate" them: +/// +/// | | this one (`normalize::helpers`) | `common::pick_str` | +/// |---|---|---| +/// | traversal | `Value::get` per `.`-separated segment — objects only | `Value::pointer` — also indexes into arrays | +/// | non-string leaf | rejected, returns `None` | `Number` is coerced via `to_string()` | +/// +/// The number case is the one that bites. A payload whose `id` is `42` +/// rather than `"42"` yields `None` here and `Some("42")` there, which +/// silently changes what a normaliser emits as a document id. The callers of +/// this function were written against the reject-non-strings behaviour and +/// have a test pinning it (`pick_str_rejects_non_string_values` below, and +/// the host-side mirror of it). +pub fn pick_str(value: &serde_json::Value, paths: &[&str]) -> Option { + for path in paths { + let mut cur = value; + let mut ok = true; + for segment in path.split('.') { + match cur.get(segment) { + Some(next) => cur = next, + None => { + ok = false; + break; + } + } + } + if !ok { + continue; + } + if let Some(s) = cur.as_str() { + let trimmed = s.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_string()); + } + } + } + None +} + +#[cfg(test)] +#[path = "helpers_tests.rs"] +mod tests; diff --git a/src/memory/sync/composio/providers/normalize/helpers_tests.rs b/src/memory/sync/composio/providers/normalize/helpers_tests.rs new file mode 100644 index 0000000..e481f19 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/helpers_tests.rs @@ -0,0 +1,35 @@ +use super::*; +use serde_json::json; + +#[test] +fn pick_str_finds_first_non_empty_match() { + let v = json!({"data": {"user": {"name": "Ada", "email": "ada@example.com"}}}); + assert_eq!( + pick_str(&v, &["data.user.name", "data.user.email"]), + Some("Ada".into()) + ); + assert_eq!( + pick_str(&v, &["data.missing", "data.user.email"]), + Some("ada@example.com".into()) + ); + assert_eq!(pick_str(&v, &["nope.nope"]), None); +} + +#[test] +fn pick_str_respects_path_order() { + let v = json!({"a": "first", "b": "second"}); + assert_eq!(pick_str(&v, &["a", "b"]), Some("first".into())); + assert_eq!(pick_str(&v, &["b", "a"]), Some("second".into())); +} + +/// The drift guard for the divergence documented on [`pick_str`]. If this +/// ever starts returning `Some("42")`, someone has re-pointed the +/// normalisers at `common::pick_str` and changed their output. +#[test] +fn pick_str_rejects_non_string_values() { + let v = json!({"count": 42, "flag": true, "empty": "", "whitespace": " "}); + assert_eq!(pick_str(&v, &["count"]), None); + assert_eq!(pick_str(&v, &["flag"]), None); + assert_eq!(pick_str(&v, &["empty"]), None); + assert_eq!(pick_str(&v, &["whitespace"]), None); +} diff --git a/src/memory/sync/composio/providers/normalize/linear.rs b/src/memory/sync/composio/providers/normalize/linear.rs new file mode 100644 index 0000000..843f98a --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/linear.rs @@ -0,0 +1,157 @@ +//! Linear host normalization helpers — result extraction, issue-title extraction, +//! viewer identity, cursor extraction, and time utilities. +//! +//! Linear's GraphQL API (and therefore Composio's wrapping of it) returns +//! connection-style lists (`{ nodes: [...], pageInfo: {...} }`) at the top +//! level or nested under `data`. The functions here walk the union of +//! common shapes so the provider does not have to branch per Composio +//! envelope variant. + +use serde_json::Value; + +use super::helpers::pick_str; + +/// Walk the Composio response envelope for Linear issue list results. +/// +/// Linear's list endpoints return `{ nodes: [...] }` or +/// `{ issues: { nodes: [...] } }` shapes; Composio may re-wrap the +/// upstream payload under `data` or `data.data`. We probe each shape +/// in order and return the first array we find. +pub fn extract_issues(data: &Value) -> Vec { + let candidates = [ + data.pointer("/data/nodes"), + data.pointer("/nodes"), + data.pointer("/data/issues/nodes"), + data.pointer("/issues/nodes"), + data.pointer("/data/data/nodes"), + data.pointer("/data/data/issues/nodes"), + data.pointer("/data/results"), + data.pointer("/results"), + data.pointer("/data/items"), + data.pointer("/items"), + ]; + for cand in candidates.into_iter().flatten() { + if let Some(arr) = cand.as_array() { + return arr.clone(); + } + } + Vec::new() +} + +/// Extract a human-readable title from a Linear issue object. +/// +/// Linear issues store the name at `title` (or `data.title` after +/// Composio envelope wrapping). Falls back to `name` / `identifier` +/// so the chunk remains identifiable even for unusual response shapes. +pub fn extract_issue_title(issue: &Value) -> Option { + pick_str( + issue, + &[ + "title", + "data.title", + "name", + "data.name", + "identifier", + "data.identifier", + ], + ) +} + +/// Extract a stable cursor timestamp from a Linear issue object. +/// +/// Linear uses ISO-8601 strings for timestamps (`updatedAt`). We keep +/// the value as a string so lexicographic comparison against the stored +/// cursor is valid. +pub fn extract_issue_updated(issue: &Value) -> Option { + pick_str( + issue, + &[ + "updatedAt", + "data.updatedAt", + "updated_at", + "data.updated_at", + ], + ) +} + +/// Extract the viewer (authenticated user) object from a +/// `LINEAR_LIST_LINEAR_USERS { isMe: true }` response. +/// +/// Linear's GraphQL viewer endpoint returns `{ nodes: [{ id, email, … }] }`. +/// Composio may wrap this under `data` or `data.data`. We probe each +/// shape and return the first element of the nodes array, falling back +/// to the payload itself if it looks like a direct user object (has +/// `id` or `email`). +pub fn extract_viewer(data: &Value) -> Option { + let array_candidates = [ + data.pointer("/data/nodes"), + data.pointer("/nodes"), + data.pointer("/data/data/nodes"), + data.pointer("/data/users/nodes"), + ]; + for cand in array_candidates.into_iter().flatten() { + if let Some(arr) = cand.as_array() { + if let Some(first) = arr.first() { + return Some(first.clone()); + } + } + } + // Fallback: if the payload itself looks like a user object, return it. + if data.get("id").is_some() || data.get("email").is_some() { + return Some(data.clone()); + } + None +} + +/// Extract the viewer's ID string from a `LINEAR_LIST_LINEAR_USERS` +/// response. Returns `None` if the payload does not contain a +/// recognizable user ID. +pub fn extract_viewer_id(data: &Value) -> Option { + let viewer = extract_viewer(data)?; + pick_str(&viewer, &["id", "data.id"]) +} + +/// Extract a pagination cursor from a Linear connection `pageInfo` block. +/// +/// Returns `Some(endCursor)` only when `hasNextPage` is `true`; +/// `None` when the last page has been reached or when the envelope does +/// not carry `pageInfo` at all. +pub fn extract_pagination_cursor(data: &Value) -> Option { + // Mirrors the `extract_issues` envelope shapes, so every shape that can + // carry a node list can also carry its `pageInfo` cursor. + let page_info_candidates = [ + data.pointer("/data/pageInfo"), + data.pointer("/pageInfo"), + data.pointer("/data/data/pageInfo"), + data.pointer("/data/issues/pageInfo"), + data.pointer("/data/data/issues/pageInfo"), + ]; + for cand in page_info_candidates.into_iter().flatten() { + let has_next = cand + .get("hasNextPage") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + if has_next { + if let Some(cursor) = cand.get("endCursor").and_then(|v| v.as_str()) { + let trimmed = cursor.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_string()); + } + } + } + } + None +} + +/// Current wall-clock time in milliseconds since the UNIX epoch. +pub fn now_ms() -> u64 { + use std::time::{SystemTime, UNIX_EPOCH}; + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) +} + +#[cfg(test)] +#[path = "linear_tests.rs"] +mod tests; diff --git a/src/memory/sync/composio/providers/normalize/linear_tests.rs b/src/memory/sync/composio/providers/normalize/linear_tests.rs new file mode 100644 index 0000000..d796f67 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/linear_tests.rs @@ -0,0 +1,184 @@ +use super::*; +use serde_json::json; + +// ── extract_issues ─────────────────────────────────────────────── + +#[test] +fn extract_issues_from_data_nodes() { + let data = json!({ "data": { "nodes": [{"id": "i1"}, {"id": "i2"}] } }); + assert_eq!(extract_issues(&data).len(), 2); +} + +#[test] +fn extract_issues_from_top_level_nodes() { + let data = json!({ "nodes": [{"id": "i3"}] }); + assert_eq!(extract_issues(&data).len(), 1); +} + +#[test] +fn extract_issues_from_data_issues_nodes() { + let data = + json!({ "data": { "issues": { "nodes": [{"id": "i4"}, {"id": "i5"}, {"id": "i6"}] } } }); + assert_eq!(extract_issues(&data).len(), 3); +} + +#[test] +fn extract_issues_from_top_level_issues_nodes() { + let data = json!({ "issues": { "nodes": [{"id": "i7"}] } }); + assert_eq!(extract_issues(&data).len(), 1); +} + +#[test] +fn extract_issues_from_doubly_nested_issues_nodes() { + let data = + json!({ "data": { "data": { "issues": { "nodes": [{"id": "i8"}, {"id": "i9"}] } } } }); + assert_eq!(extract_issues(&data).len(), 2); +} + +#[test] +fn extract_issues_from_results() { + let data = json!({ "results": [{"id": "i7"}] }); + assert_eq!(extract_issues(&data).len(), 1); +} + +#[test] +fn extract_issues_empty_when_missing() { + let data = json!({ "foo": "bar" }); + assert!(extract_issues(&data).is_empty()); +} + +// ── extract_issue_title ────────────────────────────────────────── + +#[test] +fn extract_issue_title_from_title_field() { + let issue = json!({ "id": "i1", "title": "Fix the login bug" }); + assert_eq!( + extract_issue_title(&issue), + Some("Fix the login bug".into()) + ); +} + +#[test] +fn extract_issue_title_falls_back_to_wrapped_data() { + let issue = json!({ "data": { "title": "Wrapped issue" } }); + assert_eq!(extract_issue_title(&issue), Some("Wrapped issue".into())); +} + +#[test] +fn extract_issue_title_falls_back_to_identifier() { + let issue = json!({ "identifier": "ENG-42" }); + assert_eq!(extract_issue_title(&issue), Some("ENG-42".into())); +} + +// ── extract_issue_updated ──────────────────────────────────────── + +#[test] +fn extract_issue_updated_from_updated_at() { + let issue = json!({ "updatedAt": "2026-03-01T12:00:00.000Z" }); + assert_eq!( + extract_issue_updated(&issue), + Some("2026-03-01T12:00:00.000Z".to_string()) + ); +} + +#[test] +fn extract_issue_updated_falls_back_to_snake_case() { + let issue = json!({ "data": { "updated_at": "2026-01-15T08:30:00.000Z" } }); + assert_eq!( + extract_issue_updated(&issue), + Some("2026-01-15T08:30:00.000Z".to_string()) + ); +} + +// ── extract_viewer ─────────────────────────────────────────────── + +#[test] +fn extract_viewer_from_data_nodes() { + let data = json!({ "data": { "nodes": [{ "id": "usr_1", "email": "a@b.com" }] } }); + let v = extract_viewer(&data).expect("should find viewer"); + assert_eq!(v["id"], "usr_1"); +} + +#[test] +fn extract_viewer_from_top_level_nodes() { + let data = json!({ "nodes": [{ "id": "usr_2" }] }); + let v = extract_viewer(&data).expect("should find viewer"); + assert_eq!(v["id"], "usr_2"); +} + +#[test] +fn extract_viewer_fallback_direct_object() { + let data = json!({ "id": "usr_direct", "name": "Direct User" }); + let v = extract_viewer(&data).expect("should return direct object"); + assert_eq!(v["id"], "usr_direct"); +} + +#[test] +fn extract_viewer_returns_none_when_absent() { + let data = json!({ "foo": "bar" }); + assert!(extract_viewer(&data).is_none()); +} + +// ── extract_pagination_cursor ──────────────────────────────────── + +#[test] +fn extract_pagination_cursor_returns_cursor_when_has_next_page() { + let data = json!({ + "data": { + "pageInfo": { + "hasNextPage": true, + "endCursor": "cursor_abc" + } + } + }); + assert_eq!( + extract_pagination_cursor(&data), + Some("cursor_abc".to_string()) + ); +} + +#[test] +fn extract_pagination_cursor_returns_none_when_last_page() { + let data = json!({ + "pageInfo": { + "hasNextPage": false, + "endCursor": "cursor_xyz" + } + }); + assert!(extract_pagination_cursor(&data).is_none()); +} + +#[test] +fn extract_pagination_cursor_from_doubly_nested_issues() { + // The same `data.data.issues` shape `extract_issues` reads must also + // expose its pageInfo cursor, or a doubly-nested payload never pages. + let data = json!({ + "data": { + "data": { + "issues": { + "pageInfo": { + "hasNextPage": true, + "endCursor": "cursor_issue_2" + } + } + } + } + }); + assert_eq!( + extract_pagination_cursor(&data), + Some("cursor_issue_2".to_string()) + ); +} + +#[test] +fn extract_pagination_cursor_returns_none_when_absent() { + let data = json!({ "nodes": [{"id": "i1"}] }); + assert!(extract_pagination_cursor(&data).is_none()); +} + +// ── now_ms ─────────────────────────────────────────────────────── + +#[test] +fn now_ms_returns_nonzero() { + assert!(now_ms() > 0); +} diff --git a/src/memory/sync/composio/providers/normalize/mod.rs b/src/memory/sync/composio/providers/normalize/mod.rs new file mode 100644 index 0000000..5f90652 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/mod.rs @@ -0,0 +1,23 @@ +//! Provider payload normalisers for hosts that drive Composio through their +//! own provider abstraction, rather than through the [`SyncPipeline`] +//! implementations in this directory's siblings. +//! +//! These are pure `serde_json::Value` → `Value` transforms: given a raw +//! Composio action response, pull out the fields that make up a task, an +//! issue, a page or a message. They hold no credentials, touch no network, +//! and make no scheduling decisions — provider-specific normalisation is +//! driver-side by definition (see the host's `docs/specs/kernel.md` §4). +//! +//! [`SyncPipeline`]: crate::memory::sync::traits::SyncPipeline + +pub mod clickup; +pub mod github; +pub mod helpers; +pub mod linear; +pub mod notion; + +// Named `_post_process` rather than ``: `slack.rs` and +// `github.rs` (the SyncPipeline implementations) already occupy those names one +// directory up, and `gmail.rs` one directory above that. +pub mod gmail_post_process; +pub mod slack_post_process; diff --git a/src/memory/sync/composio/providers/normalize/notion.rs b/src/memory/sync/composio/providers/normalize/notion.rs new file mode 100644 index 0000000..0f73670 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/notion.rs @@ -0,0 +1,115 @@ +//! Notion host normalization helpers — result extraction, pagination cursor, +//! page title extraction, and time utilities. + +use serde_json::Value; + +use super::helpers::pick_str; + +/// Walk the Composio response envelope for Notion page results. +pub fn extract_results(data: &Value) -> Vec { + let candidates = [ + data.pointer("/data/results"), + data.pointer("/results"), + data.pointer("/data/data/results"), + data.pointer("/data/items"), + data.pointer("/items"), + ]; + for cand in candidates.into_iter().flatten() { + if let Some(arr) = cand.as_array() { + return arr.clone(); + } + } + Vec::new() +} + +/// Extract the rendered page body markdown from a `NOTION_GET_PAGE_MARKDOWN` +/// response. Composio wraps action output in varying envelope shapes, so we +/// try the common locations tolerantly and return the first non-empty string. +/// Returns `None` if no markdown field is found (caller falls back to the +/// metadata-only body and logs the raw shape for diagnosis). +pub fn extract_page_markdown(data: &Value) -> Option { + const PATHS: &[&str] = &[ + "/markdown", + "/data/markdown", + "/data/response_data/markdown", + "/response_data/markdown", + "/data/content", + "/content", + "/data/markdown_content", + "/markdown_content", + "/text", + "/data/text", + ]; + for p in PATHS { + if let Some(s) = data.pointer(p).and_then(Value::as_str) { + if !s.trim().is_empty() { + return Some(s.to_string()); + } + } + } + None +} + +/// Extract the Notion pagination cursor (for `start_cursor` on the +/// next request). +pub fn extract_notion_cursor(data: &Value) -> Option { + let candidates = [ + data.pointer("/data/next_cursor"), + data.pointer("/next_cursor"), + data.pointer("/data/data/next_cursor"), + ]; + for cand in candidates.into_iter().flatten() { + if let Some(s) = cand.as_str() { + let trimmed = s.trim(); + if !trimmed.is_empty() { + return Some(trimmed.to_string()); + } + } + } + None +} + +/// Try to extract a human-readable title from a Notion page object. +/// +/// Notion pages store the title in `properties.title` or +/// `properties.Name.title[0].plain_text`. We try several shapes. +pub fn extract_page_title(page: &Value) -> Option { + // Try the common `properties.title.title[0].plain_text` shape. + let props = page + .get("properties") + .or_else(|| page.get("data")?.get("properties")); + if let Some(props) = props { + // Walk all properties looking for a "title" type field. + if let Some(obj) = props.as_object() { + for (_key, val) in obj { + if val.get("type").and_then(Value::as_str) == Some("title") { + if let Some(arr) = val.get("title").and_then(Value::as_array) { + let text: String = arr + .iter() + .filter_map(|t| t.get("plain_text").and_then(Value::as_str)) + .collect::>() + .join(""); + if !text.is_empty() { + return Some(text); + } + } + } + } + } + } + + // Fallback: top-level "title" field (some Composio shapes). + pick_str(page, &["title", "data.title", "name", "data.name"]) +} + +pub fn now_ms() -> u64 { + use std::time::{SystemTime, UNIX_EPOCH}; + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) +} + +#[cfg(test)] +#[path = "notion_tests.rs"] +mod tests; diff --git a/src/memory/sync/composio/providers/normalize/notion_tests.rs b/src/memory/sync/composio/providers/normalize/notion_tests.rs new file mode 100644 index 0000000..f235b3a --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/notion_tests.rs @@ -0,0 +1,137 @@ +use super::*; +use serde_json::json; + +#[test] +fn extract_results_from_data_results() { + let data = json!({"data": {"results": [{"id": "page1"}]}}); + let results = extract_results(&data); + assert_eq!(results.len(), 1); +} + +#[test] +fn extract_page_markdown_reads_top_level_field() { + // Matches the live GET_PAGE_MARKDOWN envelope observed empirically: + // {id, markdown, object, request_id, truncated, unknown_block_ids}. + let data = json!({ + "id": "p1", + "markdown": "# Heading\n\nbody text", + "object": "page", + "truncated": false, + }); + assert_eq!( + extract_page_markdown(&data).as_deref(), + Some("# Heading\n\nbody text") + ); +} + +#[test] +fn extract_page_markdown_reads_nested_envelope() { + let data = json!({ "data": { "markdown": "nested body" } }); + assert_eq!(extract_page_markdown(&data).as_deref(), Some("nested body")); +} + +#[test] +fn extract_page_markdown_none_for_empty_or_missing() { + // Empty markdown (a DB row with no body blocks) → None → metadata-only. + assert_eq!(extract_page_markdown(&json!({ "markdown": "" })), None); + assert_eq!(extract_page_markdown(&json!({ "markdown": " " })), None); + // No markdown field at all → None. + assert_eq!(extract_page_markdown(&json!({ "id": "p1" })), None); +} + +#[test] +fn extract_results_from_top_level() { + let data = json!({"results": [{"id": "a"}, {"id": "b"}]}); + let results = extract_results(&data); + assert_eq!(results.len(), 2); +} + +#[test] +fn extract_results_from_data_items() { + let data = json!({"data": {"items": [{"id": "x"}]}}); + let results = extract_results(&data); + assert_eq!(results.len(), 1); +} + +#[test] +fn extract_results_empty_when_no_match() { + let data = json!({"foo": "bar"}); + assert!(extract_results(&data).is_empty()); +} + +#[test] +fn extract_notion_cursor_from_data() { + let data = json!({"data": {"next_cursor": "cur123"}}); + assert_eq!(extract_notion_cursor(&data), Some("cur123".into())); +} + +#[test] +fn extract_notion_cursor_from_top_level() { + let data = json!({"next_cursor": "abc"}); + assert_eq!(extract_notion_cursor(&data), Some("abc".into())); +} + +#[test] +fn extract_notion_cursor_none_when_empty() { + let data = json!({"data": {"next_cursor": " "}}); + assert_eq!(extract_notion_cursor(&data), None); +} + +#[test] +fn extract_notion_cursor_none_when_missing() { + assert_eq!(extract_notion_cursor(&json!({})), None); +} + +#[test] +fn extract_page_title_from_properties_title_type() { + let page = json!({ + "properties": { + "Name": { + "type": "title", + "title": [{"plain_text": "Hello"}, {"plain_text": " World"}] + } + } + }); + assert_eq!(extract_page_title(&page), Some("Hello World".into())); +} + +#[test] +fn extract_page_title_from_nested_data_properties() { + let page = json!({ + "data": { + "properties": { + "Title": { + "type": "title", + "title": [{"plain_text": "My Page"}] + } + } + } + }); + assert_eq!(extract_page_title(&page), Some("My Page".into())); +} + +#[test] +fn extract_page_title_fallback_to_top_level_title() { + let page = json!({"title": "Fallback Title"}); + assert_eq!(extract_page_title(&page), Some("Fallback Title".into())); +} + +#[test] +fn extract_page_title_none_when_empty() { + let page = json!({"properties": {"Name": {"type": "title", "title": []}}}); + // Empty title array means no text + assert!( + extract_page_title(&page).is_none() || extract_page_title(&page) == Some(String::new()) + ); +} + +#[test] +fn extract_page_title_none_when_no_title_field() { + let page = json!({"id": "123"}); + assert!(extract_page_title(&page).is_none()); +} + +#[test] +fn now_ms_returns_nonzero() { + assert!(now_ms() > 0); +} diff --git a/src/memory/sync/composio/providers/normalize/slack_post_process.rs b/src/memory/sync/composio/providers/normalize/slack_post_process.rs new file mode 100644 index 0000000..6ada3e1 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/slack_post_process.rs @@ -0,0 +1,320 @@ +//! Slack-specific post-processing of Composio action responses. +//! +//! Composio's Slack responses are verbose API envelopes. This module +//! rewrites each supported action's response into a slim, stable shape +//! that the ingest pipeline and enrichers can consume without walking +//! Composio's unstable nested envelopes. +//! +//! ## Supported slugs +//! +//! - `SLACK_FETCH_CONVERSATION_HISTORY` — reshapes into top-level +//! `messages[]` with `{ ts, user, text, thread_ts, channel_id }`. +//! Empty-text messages are dropped. `channel_id` is absent here (it's +//! in the request, not the response); the caller injects it via the +//! enricher in +//! [`crate::memory::sync::composio::providers::SlackSyncPipeline`]. +//! +//! - `SLACK_LIST_CONVERSATIONS` — reshapes into top-level `channels[]` +//! with `{ id, name, is_private }` per channel. Entries with an empty +//! id are dropped. +//! +//! - `SLACK_SEARCH_MESSAGES` — reshapes `messages.matches[]` (possibly +//! nested) into top-level `messages[]` with `{ ts, user, text, +//! thread_ts, channel_id }`. `channel_id` is pulled from each match's +//! `channel.id` field. `paging.pages` is preserved at top-level for +//! caller pagination. +//! +//! ## Design note: user-id resolution is NOT here +//! +//! `SlackUsers` is a per-sync cache built from a separate API call — +//! not a function of any individual response. Resolving user ids +//! happens in +//! [`crate::memory::sync::composio::providers::SlackSyncPipeline`] +//! (the enricher layer), keeping this module purely data-shape–oriented. +//! This matches Gmail's pattern of "post_process is data-only". +//! +//! Unknown slugs are silently no-ops so new Composio actions don't +//! break the provider. + +use serde_json::{Map, Value}; + +/// Entry point called from `SlackProvider::post_process_action_result`. +/// +/// Dispatches on the Composio action slug and rewrites `data` in place. +/// Unknown slugs are silently ignored. +pub fn post_process(slug: &str, _arguments: Option<&Value>, data: &mut Value) { + log::debug!("[composio:slack][post-process] slug={slug}"); + match slug { + "SLACK_FETCH_CONVERSATION_HISTORY" => reshape_fetch_history(data), + "SLACK_LIST_CONVERSATIONS" => reshape_list_conversations(data), + "SLACK_SEARCH_MESSAGES" => reshape_search_messages(data), + _ => { + log::debug!("[composio:slack][post-process] unknown slug={slug}, passing through"); + } + } +} + +// ─── SLACK_FETCH_CONVERSATION_HISTORY ────────────────────────────────────── + +/// Rewrite a `SLACK_FETCH_CONVERSATION_HISTORY` response in place. +/// +/// Walks possible nested envelopes (`/data/messages`, `/messages`, +/// `/data/data/messages`) to find the raw messages array, drops messages +/// with empty `text`, and emits a slim `{ ts, user, text, thread_ts }` +/// shape under a top-level `messages[]` key. The consumed nested array is +/// removed from the payload so the raw verbose rows don't linger alongside +/// the slim copy. The caller injects `channel_id` via +/// [`super::sync::extract_messages`]. +fn reshape_fetch_history(data: &mut Value) { + let arr = take_array( + data, + &["/data/messages", "/messages", "/data/data/messages"], + 0, + ); + let slim: Vec = arr.into_iter().filter_map(slim_history_message).collect(); + let obj = ensure_object(data); + obj.insert("messages".to_string(), Value::Array(slim)); + log::debug!("[composio:slack][post-process] SLACK_FETCH_CONVERSATION_HISTORY reshaped"); +} + +fn slim_history_message(raw: Value) -> Option { + let text = raw + .get("text") + .and_then(|v| v.as_str()) + .unwrap_or("") + .trim(); + if text.is_empty() { + return None; + } + let mut out = Map::new(); + if let Some(ts) = raw.get("ts") { + out.insert("ts".into(), ts.clone()); + } else { + return None; // ts is required — no ts means we can't cursor or archive + } + if let Some(user) = raw.get("user").or_else(|| raw.get("bot_id")) { + out.insert("user".into(), user.clone()); + } + out.insert("text".into(), Value::String(text.to_string())); + if let Some(thread_ts) = raw.get("thread_ts") { + out.insert("thread_ts".into(), thread_ts.clone()); + } + if let Some(permalink) = raw.get("permalink") { + out.insert("permalink".into(), permalink.clone()); + } + Some(Value::Object(out)) +} + +/// Find the first array at any of `candidates`, remove that field (plus +/// `envelope_depth` ancestor object envelopes) from `data`, and return the +/// array. Removing the consumed nested payload keeps the reshaped output from +/// carrying duplicate raw rows. +fn take_array(data: &mut Value, candidates: &[&str], envelope_depth: usize) -> Vec { + for path in candidates { + let arr = match data.pointer(path).and_then(|v| v.as_array().cloned()) { + Some(a) => a, + None => continue, + }; + let mut remove_path = path.to_string(); + for _ in 0..envelope_depth { + remove_path = match remove_path.rsplit_once('/') { + Some((parent, _)) => parent.to_string(), + None => break, + }; + } + remove_nested(data, &remove_path); + return arr; + } + Vec::new() +} + +/// Remove the field at `path` from `data`, pruning any ancestor object that +/// the removal left empty so a consumed `data` envelope disappears entirely +/// instead of lingering as `{}`. +fn remove_nested(data: &mut Value, path: &str) { + let segments: Vec<&str> = path + .trim_start_matches('/') + .split('/') + .filter(|s| !s.is_empty()) + .collect(); + if segments.is_empty() { + return; + } + + // Remove the leaf field. + let mut current = &mut *data; + for seg in &segments[..segments.len() - 1] { + current = match current.get_mut(*seg) { + Some(next) => next, + None => return, + }; + } + if let Value::Object(map) = current { + map.remove(segments[segments.len() - 1]); + } + + // Prune empty object ancestors, deepest first. + for depth in (0..segments.len().saturating_sub(1)).rev() { + // Re-walk to the object at `segments[..=depth]`. + let mut ancestor = &mut *data; + for seg in &segments[..=depth] { + ancestor = match ancestor.get_mut(*seg) { + Some(next) => next, + None => return, + }; + } + if !matches!(ancestor, Value::Object(m) if m.is_empty()) { + break; + } + // Remove it from its parent (`segments[..depth]`). For `depth == 0` + // the parent is the top-level object, so an emptied `data` envelope + // key disappears entirely. + let mut parent = &mut *data; + for seg in &segments[..depth] { + parent = match parent.get_mut(*seg) { + Some(next) => next, + None => return, + }; + } + if let Value::Object(map) = parent { + map.remove(segments[depth]); + } + } +} + +// ─── SLACK_LIST_CONVERSATIONS ─────────────────────────────────────────────── + +/// Rewrite a `SLACK_LIST_CONVERSATIONS` response in place. +/// +/// Reshapes into a top-level `channels[]` with `{ id, name, is_private }` +/// per channel; entries with an empty id are dropped. +fn reshape_list_conversations(data: &mut Value) { + let arr = take_array( + data, + &[ + "/data/channels", + "/channels", + "/data/data/channels", + "/data/conversations", + "/conversations", + ], + 0, + ); + + let slim: Vec = arr.into_iter().filter_map(slim_channel).collect(); + let obj = ensure_object(data); + obj.insert("channels".to_string(), Value::Array(slim)); + log::debug!("[composio:slack][post-process] SLACK_LIST_CONVERSATIONS reshaped"); +} + +fn slim_channel(raw: Value) -> Option { + let id = raw.get("id").and_then(|v| v.as_str()).unwrap_or("").trim(); + if id.is_empty() { + return None; + } + let name = raw + .get("name") + .and_then(|v| v.as_str()) + .unwrap_or(id) + .trim(); + let is_private = raw + .get("is_private") + .and_then(|v| v.as_bool()) + .unwrap_or(false); + Some(Value::Object({ + let mut m = Map::new(); + m.insert("id".into(), Value::String(id.to_string())); + m.insert("name".into(), Value::String(name.to_string())); + m.insert("is_private".into(), Value::Bool(is_private)); + m + })) +} + +// ─── SLACK_SEARCH_MESSAGES ────────────────────────────────────────────────── + +/// Rewrite a `SLACK_SEARCH_MESSAGES` response in place. +/// +/// Reshapes `messages.matches[]` (possibly nested under one or two +/// `data` envelopes) into top-level `messages[]`. `channel_id` is pulled +/// from each match's `channel.id` field. `paging.pages` is preserved at +/// top-level under `pages` for the caller to drive pagination. +fn reshape_search_messages(data: &mut Value) { + // Preserve paging info before mutating data (take_array below removes the + // envelope that carries it). + let pages = [ + data.pointer("/data/messages/paging/pages"), + data.pointer("/messages/paging/pages"), + data.pointer("/data/data/messages/paging/pages"), + ] + .into_iter() + .flatten() + .find_map(|v| v.as_u64()) + .unwrap_or(1); + + // Envelope depth 1 removes the `messages` object (matches + paging) that + // held the consumed rows, not just the `matches` array. + let arr = take_array( + data, + &[ + "/data/messages/matches", + "/messages/matches", + "/data/data/messages/matches", + ], + 1, + ); + + let slim: Vec = arr.into_iter().filter_map(slim_search_match).collect(); + let obj = ensure_object(data); + obj.insert("messages".to_string(), Value::Array(slim)); + obj.insert("pages".to_string(), Value::Number(pages.into())); + log::debug!("[composio:slack][post-process] SLACK_SEARCH_MESSAGES reshaped"); +} + +fn slim_search_match(raw: Value) -> Option { + let text = raw + .get("text") + .and_then(|v| v.as_str()) + .unwrap_or("") + .trim(); + if text.is_empty() { + return None; + } + let ts = raw.get("ts")?; + let channel_id = raw + .pointer("/channel/id") + .and_then(|v| v.as_str()) + .unwrap_or("") + .trim(); + + let mut out = Map::new(); + out.insert("ts".into(), ts.clone()); + if let Some(user) = raw.get("user").or_else(|| raw.get("bot_id")) { + out.insert("user".into(), user.clone()); + } + out.insert("text".into(), Value::String(text.to_string())); + if let Some(thread_ts) = raw.get("thread_ts") { + out.insert("thread_ts".into(), thread_ts.clone()); + } + if !channel_id.is_empty() { + out.insert("channel_id".into(), Value::String(channel_id.to_string())); + } + if let Some(permalink) = raw.get("permalink") { + out.insert("permalink".into(), permalink.clone()); + } + Some(Value::Object(out)) +} + +// ─── Helpers ──────────────────────────────────────────────────────────────── + +/// Ensure `data` is a JSON object, replacing it with an empty object if +/// not. Returns a mutable ref to the inner map. +fn ensure_object(data: &mut Value) -> &mut Map { + if !data.is_object() { + *data = Value::Object(Map::new()); + } + data.as_object_mut().unwrap() +} + +#[cfg(test)] +#[path = "slack_post_process_tests.rs"] +mod tests; diff --git a/src/memory/sync/composio/providers/normalize/slack_post_process_tests.rs b/src/memory/sync/composio/providers/normalize/slack_post_process_tests.rs new file mode 100644 index 0000000..7a9ff19 --- /dev/null +++ b/src/memory/sync/composio/providers/normalize/slack_post_process_tests.rs @@ -0,0 +1,257 @@ +use super::*; +use serde_json::json; + +// ─── SLACK_FETCH_CONVERSATION_HISTORY ───────────────────────────────────── + +#[test] +fn history_reshapes_top_level_messages() { + let mut data = json!({ + "messages": [ + { "ts": "1714003200.000100", "user": "U1", "text": "hello" }, + { "ts": "1714003300.000200", "user": "U2", "text": "world", "thread_ts": "1714003200.0" }, + { "ts": "1714003400.000300", "user": "U3", "text": " " }, // dropped: empty text + ], + "response_metadata": { "next_cursor": "abc" } + }); + post_process("SLACK_FETCH_CONVERSATION_HISTORY", None, &mut data); + + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 2, "empty-text message must be dropped"); + assert_eq!(msgs[0]["ts"], "1714003200.000100"); + assert_eq!(msgs[0]["user"], "U1"); + assert_eq!(msgs[0]["text"], "hello"); + assert!(msgs[0].get("thread_ts").is_none()); + assert_eq!(msgs[1]["thread_ts"], "1714003200.0"); +} + +#[test] +fn history_reshapes_nested_data_envelope() { + let mut data = json!({ + "data": { + "messages": [ + { "ts": "1714003200.0", "user": "U1", "text": "hi" } + ] + } + }); + post_process("SLACK_FETCH_CONVERSATION_HISTORY", None, &mut data); + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["text"], "hi"); +} + +#[test] +fn history_reshapes_doubly_nested_envelope() { + let mut data = json!({ + "data": { + "data": { + "messages": [ + { "ts": "1714003200.0", "user": "U1", "text": "deep" } + ] + } + } + }); + post_process("SLACK_FETCH_CONVERSATION_HISTORY", None, &mut data); + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["text"], "deep"); +} + +#[test] +fn history_drops_message_without_ts() { + let mut data = json!({ + "messages": [ + { "user": "U1", "text": "no timestamp" }, + { "ts": "1714003200.0", "user": "U2", "text": "has ts" }, + ] + }); + post_process("SLACK_FETCH_CONVERSATION_HISTORY", None, &mut data); + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["text"], "has ts"); +} + +#[test] +fn history_removes_nested_envelope_after_reshape() { + let mut data = json!({ + "data": { + "messages": [ + { "ts": "1714003200.0", "user": "U1", "text": "hi" } + ] + } + }); + post_process("SLACK_FETCH_CONVERSATION_HISTORY", None, &mut data); + + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["text"], "hi"); + assert!( + data.pointer("/data").is_none(), + "consumed `data.messages` envelope must be removed, got: {data}" + ); +} + +// ─── SLACK_LIST_CONVERSATIONS ───────────────────────────────────────────── + +#[test] +fn list_conversations_reshapes_channels() { + let mut data = json!({ + "data": { + "channels": [ + { "id": "C1", "name": "eng", "is_private": false, "extra": "noise" }, + { "id": "G1", "name": "ops", "is_private": true }, + { "id": "", "name": "empty-id" }, // dropped + ] + } + }); + post_process("SLACK_LIST_CONVERSATIONS", None, &mut data); + let channels = data["channels"].as_array().unwrap(); + assert_eq!(channels.len(), 2, "empty-id entry must be dropped"); + assert_eq!(channels[0]["id"], "C1"); + assert_eq!(channels[0]["name"], "eng"); + assert_eq!(channels[0]["is_private"], false); + assert!( + channels[0].get("extra").is_none(), + "noise fields must be removed" + ); + assert_eq!(channels[1]["id"], "G1"); + assert_eq!(channels[1]["is_private"], true); +} + +#[test] +fn list_conversations_falls_back_to_conversations_key() { + let mut data = json!({ + "conversations": [ + { "id": "C2", "name": "dev", "is_private": false } + ] + }); + post_process("SLACK_LIST_CONVERSATIONS", None, &mut data); + let channels = data["channels"].as_array().unwrap(); + assert_eq!(channels.len(), 1); + assert_eq!(channels[0]["id"], "C2"); + assert!( + data.pointer("/conversations").is_none(), + "consumed `conversations` field must be removed" + ); +} + +// ─── SLACK_SEARCH_MESSAGES ──────────────────────────────────────────────── + +#[test] +fn search_messages_reshapes_matches() { + let mut data = json!({ + "messages": { + "matches": [ + { + "ts": "1714003200.0", + "user": "U1", + "text": "hello from search", + "channel": { "id": "C1" } + }, + { + "ts": "1714003300.0", + "user": "U2", + "text": " ", // dropped: whitespace only + "channel": { "id": "C1" } + }, + ], + "paging": { "pages": 3 } + } + }); + post_process("SLACK_SEARCH_MESSAGES", None, &mut data); + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1, "empty-text match must be dropped"); + assert_eq!(msgs[0]["ts"], "1714003200.0"); + assert_eq!(msgs[0]["text"], "hello from search"); + assert_eq!(msgs[0]["channel_id"], "C1"); + assert_eq!(data["pages"], 3, "paging.pages must be preserved"); +} + +#[test] +fn search_messages_nested_data_envelope() { + let mut data = json!({ + "data": { + "messages": { + "matches": [ + { "ts": "1714003200.0", "user": "U1", "text": "nested", "channel": { "id": "C2" } } + ], + "paging": { "pages": 1 } + } + } + }); + post_process("SLACK_SEARCH_MESSAGES", None, &mut data); + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["channel_id"], "C2"); + assert_eq!(data["pages"], 1_u64); +} + +#[test] +fn search_messages_no_matches_emits_empty_array() { + let mut data = json!({ "messages": { "matches": [] } }); + post_process("SLACK_SEARCH_MESSAGES", None, &mut data); + let msgs = data["messages"].as_array().unwrap(); + assert!(msgs.is_empty()); +} + +#[test] +fn search_messages_removes_nested_envelope_after_reshape() { + let mut data = json!({ + "data": { + "messages": { + "matches": [ + { "ts": "1714003200.0", "user": "U1", "text": "nested", "channel": { "id": "C2" } } + ], + "paging": { "pages": 1 } + } + } + }); + post_process("SLACK_SEARCH_MESSAGES", None, &mut data); + + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["channel_id"], "C2"); + assert_eq!(data["pages"], 1_u64); + assert!( + data.pointer("/data").is_none(), + "consumed `data.messages` envelope must be removed, got: {data}" + ); +} + +#[test] +fn search_messages_doubly_nested_paging_preserved() { + let mut data = json!({ + "data": { + "data": { + "messages": { + "matches": [ + { "ts": "1714003200.0", "user": "U1", "text": "deep", "channel": { "id": "C3" } } + ], + "paging": { "pages": 4 } + } + } + } + }); + post_process("SLACK_SEARCH_MESSAGES", None, &mut data); + + let msgs = data["messages"].as_array().unwrap(); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0]["text"], "deep"); + assert_eq!( + data["pages"], 4_u64, + "doubly-nested paging must be preserved" + ); + assert!( + data.pointer("/data").is_none(), + "consumed `data.data.messages` envelope must be removed, got: {data}" + ); +} + +// ─── Unknown slug ───────────────────────────────────────────────────────── + +#[test] +fn unknown_slug_is_noop() { + let mut data = json!({ "foo": "bar" }); + let original = data.clone(); + post_process("SLACK_SEND_MESSAGE", None, &mut data); + assert_eq!(data, original, "unknown slug must not mutate data"); +}