diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 69bce82..da72c8b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,13 @@ env: CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: fmt: name: Format @@ -39,10 +46,18 @@ jobs: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable - uses: Swatinem/rust-cache@42dc69e1aa15d09112580998cf2ef0119e2e91ae # v2 - - run: cargo test --lib --all-features - - run: cargo test --test '*_types' - - run: cargo test --test wire_format_peers - - run: cargo test --test compile_assertions + - run: cargo test --all-features + + msrv: + name: MSRV + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 + - uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable + with: + toolchain: 1.88.0 + - uses: Swatinem/rust-cache@42dc69e1aa15d09112580998cf2ef0119e2e91ae # v2 + - run: cargo check --all-targets --all-features doc: name: Docs diff --git a/AGENTS.md b/AGENTS.md index 014e171..d165a4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,7 +3,7 @@ # GitNexus — Code Intelligence -This project is indexed by GitNexus as **honcho-rust-sdk** (2147 symbols, 4981 relationships, 186 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. +This project is indexed by GitNexus as **honcho-rust-sdk** (2184 symbols, 5073 relationships, 190 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. > If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first. diff --git a/CHANGELOG.md b/CHANGELOG.md index c20cca3..1d3fe96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,32 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [0.1.2] - 2026-05-25 + +### Added + +- `HonchoError::is_retryable()` to expose the SDK retry policy for callers. +- Client-side validation for dialectic queries, including the 10,000 character maximum. +- Client-side validation for pagination parameters (`page >= 1`, `size` between 1 and 100). +- Client-side validation for workspace IDs (`1..=512`, ASCII alphanumeric, `_`, or `-`). +- CI MSRV check for Rust 1.88.0. + +### Changed + +- Workspace metadata and configuration reads now use the OpenAPI `POST /v3/workspaces` get-or-create endpoint. +- `Honcho::base_url()` now reports the same normalized base URL used by the HTTP client. +- CI now runs the full all-features test suite and avoids duplicate doctest execution. +- Upload and peer-configuration docs now describe supported MIME types and peer existence requirements. + +### Fixed + +- Invalid base URLs such as `localhost:8000`, unsupported schemes, or URLs without hosts are rejected during client construction. +- Base URLs with non-root trailing slashes are normalized consistently between `Honcho` and `HttpClient`. +- `Honcho::schedule_dream` rejects an empty `observer` before making network requests. +- Pagination rejects invalid `page`/`size` values before making network requests. +- Route and docs tests cover new validation boundaries and workspace get-or-create behavior. +- README upload and peer-management examples corrected. + ## [0.1.1] - 2025-05-13 ### Breaking Changes diff --git a/CLAUDE.md b/CLAUDE.md index 7600b8d..be79895 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,7 +3,7 @@ # GitNexus — Code Intelligence -This project is indexed by GitNexus as **honcho-rust-sdk** (2147 symbols, 4981 relationships, 186 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. +This project is indexed by GitNexus as **honcho-rust-sdk** (2184 symbols, 5073 relationships, 190 execution flows). Use the GitNexus MCP tools to understand code, assess impact, and navigate safely. > If any GitNexus tool warns the index is stale, run `npx gitnexus analyze` in terminal first. diff --git a/Cargo.lock b/Cargo.lock index cdefbce..f6e2c0e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -656,7 +656,7 @@ checksum = "fc0fef456e4baa96da950455cd02c081ca953b141298e41db3fc7e36b1da849c" [[package]] name = "honcho-ai" -version = "0.1.1" +version = "0.1.2" dependencies = [ "async-stream", "bon", diff --git a/Cargo.toml b/Cargo.toml index 8b25954..2a44381 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "honcho-ai" -version = "0.1.1" +version = "0.1.2" edition = "2024" rust-version = "1.88" license = "MIT" diff --git a/README.md b/README.md index c2be01a..c38b46b 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ let msgs = session.upload_file(FileSource::bytes("doc.pdf", data, "application/p .send().await?; // Streaming upload -let msgs = session.upload_file_streamed("large.bin", reader, "application/octet-stream") +let msgs = session.upload_file_streamed("large.txt", reader, "text/plain") .peer("alice") .send().await?; @@ -141,6 +141,7 @@ session.set_peers(["alice", "bob"]).await?; session.remove_peers(["bob"]).await?; let peers = session.peers().await?; let cfg = session.get_peer_configuration("alice").await?; +// alice must already be present; add_peer/add_peers/set_peers above satisfy this. session.set_peer_configuration("alice", &cfg).await?; // Context & Search @@ -165,6 +166,8 @@ session.set_configuration(config).await?; ### Pagination +Page numbers are 1-based. Page size must be between 1 and 100 inclusive. + ```rust let page = client.peers().await?; for peer in page.items() { @@ -239,7 +242,7 @@ These APIs have no equivalent in the Python/TypeScript SDKs: | Context (OpenAI/Anthropic) | ✓ | ✓ | | Blocking API | ✗ | ✓ | | Webhooks | ✓ | ✗ | -| API keys | ✓ | ✗ | +| API keys | ✓ | ✓ | ## Links diff --git a/src/blocking/client.rs b/src/blocking/client.rs index 56ef957..9bf4b19 100644 --- a/src/blocking/client.rs +++ b/src/blocking/client.rs @@ -169,6 +169,8 @@ impl Honcho { } /// List peers with filters, collecting across pages. + /// + /// `page` is 1-based. `size` must be in `1..=100`. pub fn peers_with_filters( &self, filters: HashMap, @@ -194,6 +196,8 @@ impl Honcho { } /// List sessions with filters, collecting across pages. + /// + /// `page` is 1-based. `size` must be in `1..=100`. pub fn sessions_with_filters( &self, filters: HashMap, diff --git a/src/blocking/conclusion.rs b/src/blocking/conclusion.rs index 6cf16b7..cae773c 100644 --- a/src/blocking/conclusion.rs +++ b/src/blocking/conclusion.rs @@ -214,7 +214,7 @@ impl BlockingListConclusionsBuilder { } } - /// Page size. + /// Page size. Must be in `1..=100`. #[must_use] pub fn size(self, size: u32) -> Self { Self { diff --git a/src/blocking/session.rs b/src/blocking/session.rs index 86a7fe2..269905e 100644 --- a/src/blocking/session.rs +++ b/src/blocking/session.rs @@ -152,6 +152,10 @@ impl Session { } /// Set per-peer configuration. + /// + /// The peer must already be present in the session. This method does not + /// create or add peers; use [`Session::add_peer`] or [`Session::add_peers`] + /// first. If the peer is absent, the server may return 404/`NotFound`. pub fn set_peer_configuration(&self, peer_id: &str, config: &SessionPeerConfig) -> Result<()> { block_on(self.inner.set_peer_configuration(peer_id, config)) } @@ -248,6 +252,9 @@ impl Session { /// Begin a file upload to this session. /// + /// The API currently accepts `text/plain`, `application/pdf`, and + /// `application/json`; other MIME types may be rejected by the server. + /// /// Returns a [`BlockingUploadFileBuilder`]. You **must** call `.peer(id)` /// and then `.send()` to complete the upload. /// @@ -272,6 +279,9 @@ impl Session { /// Begin a file upload from a synchronous reader. /// + /// The API currently accepts `text/plain`, `application/pdf`, and + /// `application/json`; other MIME types may be rejected by the server. + /// /// The reader is consumed in a background thread and piped to the async /// multipart stream **without buffering the entire file in memory**. /// diff --git a/src/client.rs b/src/client.rs index c251d84..a54c8c9 100644 --- a/src/client.rs +++ b/src/client.rs @@ -10,7 +10,7 @@ use tokio::sync::OnceCell; use url::Url; use crate::error::{HonchoError, Result}; -use crate::http::client::HttpClient; +use crate::http::client::{HttpClient, normalize_base_url}; use crate::http::routes; use crate::peer::Peer; use crate::session::{PeerSpec, Session}; @@ -92,15 +92,12 @@ impl Honcho { /// # Ok::<(), honcho_ai::error::HonchoError>(()) /// ``` pub fn new(base_url: &str, workspace_id: &str) -> Result { - if workspace_id.is_empty() { - return Err(HonchoError::Configuration( - "workspace_id must not be empty".into(), - )); - } - let http = - HttpClient::from_params(HttpClient::builder().base_url(base_url.to_string()).build())?; - let url = Url::parse(base_url) - .map_err(|e| HonchoError::Configuration(format!("invalid base_url: {e}")))?; + validate_workspace_id(workspace_id)?; + let url = normalize_base_url(base_url)?; + let http = HttpClient::from_params_with_base_url( + HttpClient::builder().base_url(base_url.to_owned()).build(), + url.clone(), + )?; Ok(Self { inner: Arc::new(Inner { http, @@ -154,16 +151,10 @@ impl Honcho { .or_else(|| std::env::var("HONCHO_WORKSPACE_ID").ok()) .unwrap_or_else(|| "default".to_owned()); - if resolved_workspace_id.is_empty() { - return Err(HonchoError::Configuration( - "workspace_id must not be empty".into(), - )); - } - - let base_url = Url::parse(&resolved_base_url) - .map_err(|e| HonchoError::Configuration(format!("invalid base_url: {e}")))?; + validate_workspace_id(&resolved_workspace_id)?; + let base_url = normalize_base_url(&resolved_base_url)?; - let http = HttpClient::from_params( + let http = HttpClient::from_params_with_base_url( HttpClient::builder() .base_url(resolved_base_url) .maybe_api_key(resolved_api_key) @@ -173,6 +164,7 @@ impl Honcho { .default_headers(params.default_headers.unwrap_or_default()) .default_query(params.default_query.unwrap_or_default()) .build(), + base_url.clone(), )?; Ok(Self { @@ -252,10 +244,15 @@ impl Honcho { /// Fetch workspace metadata from the server. pub async fn get_metadata(&self) -> Result> { + let body = crate::types::workspace::WorkspaceCreate { + id: self.inner.workspace_id.clone(), + metadata: None, + configuration: None, + }; let ws: Workspace = self .inner .http - .get(&routes::workspace(self.workspace_id())?, &[]) + .post(&routes::workspaces(), Some(&body), &[]) .await?; Ok(ws.metadata) } @@ -282,10 +279,15 @@ impl Honcho { /// } /// ``` pub async fn get_configuration(&self) -> Result { + let body = crate::types::workspace::WorkspaceCreate { + id: self.inner.workspace_id.clone(), + metadata: None, + configuration: None, + }; let ws: Workspace = self .inner .http - .get(&routes::workspace(self.workspace_id())?, &[]) + .post(&routes::workspaces(), Some(&body), &[]) .await?; Ok(ws.configuration) } @@ -320,10 +322,15 @@ impl Honcho { /// Use this when the server returns fields not yet represented in /// [`WorkspaceConfiguration`]. pub async fn get_configuration_raw(&self) -> Result> { + let body = crate::types::workspace::WorkspaceCreate { + id: self.inner.workspace_id.clone(), + metadata: None, + configuration: None, + }; let raw: serde_json::Value = self .inner .http - .get(&routes::workspace(self.workspace_id())?, &[]) + .post(&routes::workspaces(), Some(&body), &[]) .await?; match raw.get("configuration") { Some(serde_json::Value::Object(map)) => { @@ -557,6 +564,11 @@ impl Honcho { session_id: Option<&str>, observed_peer: Option<&str>, ) -> Result<()> { + if observer.is_empty() { + return Err(HonchoError::Validation( + "observer must not be empty".to_string(), + )); + } self.ensure_workspace().await?; let observed_peer = observed_peer.unwrap_or(observer); let body = crate::types::dream::ScheduleDreamRequest { @@ -623,6 +635,8 @@ impl Honcho { /// List peers with filters. /// + /// `page` is 1-based. `size` must be in `1..=100`. + /// /// # Examples /// /// ```no_run @@ -691,6 +705,8 @@ impl Honcho { /// List sessions with filters. /// + /// `page` is 1-based. `size` must be in `1..=100`. + /// /// # Examples /// /// ```no_run @@ -756,3 +772,28 @@ impl Honcho { Ok(page.map(|ws| ws.id)) } } + +fn validate_workspace_id(workspace_id: &str) -> Result<()> { + if workspace_id.is_empty() { + return Err(HonchoError::Configuration( + "workspace_id must not be empty".into(), + )); + } + + if workspace_id.len() > 512 { + return Err(HonchoError::Configuration( + "workspace_id must be at most 512 characters".into(), + )); + } + + if !workspace_id + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'_' | b'-')) + { + return Err(HonchoError::Configuration( + "workspace_id must match [a-zA-Z0-9_-]+".into(), + )); + } + + Ok(()) +} diff --git a/src/conclusion.rs b/src/conclusion.rs index 61a6b41..5aad357 100644 --- a/src/conclusion.rs +++ b/src/conclusion.rs @@ -636,7 +636,7 @@ impl ListConclusionsBuilder { self } - /// Set the page size (default 50). + /// Set the page size (default 50, must be in `1..=100`). /// /// # Examples /// @@ -690,11 +690,6 @@ impl ListConclusionsBuilder { /// # } /// ``` pub async fn send(self) -> Result { - if self.size == 0 { - return Err(HonchoError::Validation( - "page size must be greater than 0".to_string(), - )); - } let mut filters = serde_json::json!({ "observer_id": self.scope.inner.observer, "observed_id": self.scope.inner.observed, diff --git a/src/error.rs b/src/error.rs index ec05f8e..f1efa33 100644 --- a/src/error.rs +++ b/src/error.rs @@ -160,6 +160,13 @@ impl HonchoError { } } + /// Returns whether the error matches the SDK retry policy. + #[must_use] + pub fn is_retryable(&self) -> bool { + matches!(self, Self::Timeout { .. } | Self::Connection { .. }) + || matches!(self.status_code(), Some(429 | 500 | 502 | 503 | 504)) + } + /// Returns the suggested wait time for rate-limited requests. #[must_use] pub fn retry_after(&self) -> Option { diff --git a/src/http/client.rs b/src/http/client.rs index 0f42bc1..d5bddc4 100644 --- a/src/http/client.rs +++ b/src/http/client.rs @@ -15,6 +15,34 @@ const DEFAULT_MAX_RETRIES: u32 = 2; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(60); const INITIAL_RETRY_DELAY: Duration = Duration::from_millis(500); +pub(crate) fn normalize_base_url(base_url: &str) -> Result { + let mut base_url = Url::parse(base_url) + .map_err(|e| HonchoError::Configuration(format!("invalid base_url: {e}")))?; + + match base_url.scheme() { + "http" | "https" => {} + scheme => { + return Err(HonchoError::Configuration(format!( + "invalid base_url scheme: {scheme}" + ))); + } + } + + if base_url.host().is_none() { + return Err(HonchoError::Configuration( + "base_url must include a host".into(), + )); + } + + let path = base_url.path().to_owned(); + if path.ends_with('/') && path.len() > 1 { + let trimmed = path.trim_end_matches('/'); + base_url.set_path(trimmed); + } + + Ok(base_url) +} + struct Inner { client: reqwest::Client, base_url: Url, @@ -59,15 +87,14 @@ impl HttpClient { } pub fn from_params(params: HttpClientParams) -> Result { - let mut base_url = Url::parse(¶ms.base_url) - .map_err(|e| HonchoError::Configuration(format!("invalid base_url: {e}")))?; - - let path = base_url.path().to_owned(); - if path.ends_with('/') && path.len() > 1 { - let trimmed = path.trim_end_matches('/'); - base_url.set_path(trimmed); - } + let base_url = normalize_base_url(¶ms.base_url)?; + Self::from_params_with_base_url(params, base_url) + } + pub(crate) fn from_params_with_base_url( + params: HttpClientParams, + base_url: Url, + ) -> Result { let version = env!("CARGO_PKG_VERSION"); let mut client_headers = HeaderMap::new(); client_headers.insert( @@ -564,6 +591,26 @@ mod tests { assert!(result.is_ok()); } + #[tokio::test] + async fn builder_accepts_pre_normalized_base_url() { + let server = MockServer::start().await; + let base_url = normalize_base_url(&format!("{}/", server.uri())).unwrap(); + let client = HttpClient::from_params_with_base_url( + HttpClient::builder().base_url("unused value").build(), + base_url, + ) + .unwrap(); + + Mock::given(method("GET")) + .and(path("/v3/test")) + .respond_with(ResponseTemplate::new(200).set_body_json(peer_json())) + .mount(&server) + .await; + + let result: Peer = client.get("/v3/test", &[]).await.unwrap(); + assert_eq!(result.id, "p1"); + } + #[tokio::test] async fn builder_rejects_invalid_url() { let result = HttpClient::from_params( diff --git a/src/peer.rs b/src/peer.rs index 548960c..37aea99 100644 --- a/src/peer.rs +++ b/src/peer.rs @@ -16,7 +16,7 @@ use crate::http::client::HttpClient; use crate::http::routes; use crate::http::sse::parse_sse_stream; use crate::types::dialectic::RepresentationResponse; -use crate::types::dialectic::{DialecticOptions, ReasoningLevel}; +use crate::types::dialectic::{DialecticOptions, ReasoningLevel, validate_dialectic_query}; use crate::types::message::{MessageCreate, MessageResponse, MessageSearchOptions}; use crate::types::pagination::{self, Page}; use crate::types::peer::Peer as PeerResponse; @@ -380,7 +380,7 @@ impl Peer { /// Send a simple non-streaming dialectic chat query. /// /// Returns `Ok(None)` when the server response has no content. - /// Returns `Err(HonchoError::Validation)` when `query` is empty. + /// Returns `Err(HonchoError::Validation)` when `query` is empty or too long. /// /// # Examples /// @@ -394,11 +394,7 @@ impl Peer { /// ``` #[cfg_attr(feature = "tracing", tracing::instrument(skip(self), fields(peer_id = self.inner.id.as_str())))] pub async fn chat(&self, query: &str) -> Result> { - if query.is_empty() { - return Err(HonchoError::Validation( - "query must not be empty".to_owned(), - )); - } + validate_dialectic_query(query)?; let body = crate::types::dialectic::DialecticOptions { query: query.to_owned(), session_id: None, @@ -424,7 +420,7 @@ impl Peer { /// Send a dialectic chat query with full options (session, target, reasoning level). /// /// Returns `Ok(None)` when the server response has no content. - /// Returns `Err(HonchoError::Validation)` when the query in options is empty. + /// Returns `Err(HonchoError::Validation)` when the query in options is empty or too long. /// /// # Examples /// @@ -441,11 +437,7 @@ impl Peer { /// ``` #[cfg_attr(feature = "tracing", tracing::instrument(skip(self, options), fields(peer_id = self.inner.id.as_str())))] pub async fn chat_with_options(&self, options: &DialecticOptions) -> Result> { - if options.query.is_empty() { - return Err(HonchoError::Validation( - "query must not be empty".to_owned(), - )); - } + options.validate()?; let resp: ChatResponse = self .inner .http @@ -665,6 +657,8 @@ impl Peer { /// List sessions for this peer with filters and pagination options. /// + /// `page` is 1-based. `size` must be in `1..=100`. + /// /// # Examples /// /// ```no_run @@ -1024,17 +1018,13 @@ impl ChatStreamBuilder { /// /// # Errors /// - /// Returns `HonchoError::Validation` if the query is empty. + /// Returns `HonchoError::Validation` if the query is empty or too long. /// Returns transport/API errors if the request fails. #[allow(clippy::type_complexity)] pub async fn send( self, ) -> Result> + Send>>>> { - if self.query.is_empty() { - return Err(HonchoError::Validation( - "query must not be empty".to_owned(), - )); - } + validate_dialectic_query(&self.query)?; let opts = DialecticOptions::builder() .query(self.query) diff --git a/src/session.rs b/src/session.rs index 2d040df..ae46eaa 100644 --- a/src/session.rs +++ b/src/session.rs @@ -770,6 +770,10 @@ impl Session { /// Set per-peer configuration for a specific peer in this session. /// + /// The peer must already be present in the session. This method does not + /// create or add peers; use [`Session::add_peer`] or [`Session::add_peers`] + /// first. If the peer is absent, the server may return 404/`NotFound`. + /// /// # Examples /// /// ```ignore @@ -859,6 +863,8 @@ impl Session { /// List messages in this session with optional filters, page, size, and reverse. /// + /// `page` is 1-based. `size` must be in `1..=100`. + /// /// # Examples /// /// ```no_run @@ -902,6 +908,9 @@ impl Session { /// Begin a file upload to this session. /// + /// The API currently accepts `text/plain`, `application/pdf`, and + /// `application/json`; other MIME types may be rejected by the server. + /// /// Returns an [`UploadFileBuilder`]. You **must** call `.peer(id)` and /// then `.send()` to complete the upload. /// @@ -927,6 +936,9 @@ impl Session { /// Begin a file upload to this session from a streaming reader. /// + /// The API currently accepts `text/plain`, `application/pdf`, and + /// `application/json`; other MIME types may be rejected by the server. + /// /// The reader is fully buffered into memory before uploading. This is /// **not** true streaming — use [`Session::upload_file`] with a /// [`FileSource::path`] for filesystem streaming that avoids buffering. diff --git a/src/types/dialectic.rs b/src/types/dialectic.rs index b85ebe4..c9bfd70 100644 --- a/src/types/dialectic.rs +++ b/src/types/dialectic.rs @@ -2,6 +2,10 @@ use serde::{Deserialize, Serialize}; +use crate::error::{HonchoError, Result}; + +const MAX_DIALECTIC_QUERY_CHARS: usize = 10_000; + /// Reasoning effort level for dialectic queries. #[non_exhaustive] #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] @@ -56,6 +60,30 @@ pub struct DialecticOptions { pub reasoning_level: ReasoningLevel, } +/// Validate a dialectic query before sending it to the API. +pub fn validate_dialectic_query(query: &str) -> Result<()> { + if query.is_empty() { + return Err(HonchoError::Validation( + "query must not be empty".to_owned(), + )); + } + + if query.chars().count() > MAX_DIALECTIC_QUERY_CHARS { + return Err(HonchoError::Validation(format!( + "query must be at most {MAX_DIALECTIC_QUERY_CHARS} characters" + ))); + } + + Ok(()) +} + +impl DialecticOptions { + /// Validate options before sending them to the API. + pub fn validate(&self) -> Result<()> { + validate_dialectic_query(&self.query) + } +} + /// Response from the representation endpoint. /// /// Maps `RepresentationResponse` from the `OpenAPI` spec. diff --git a/src/types/pagination.rs b/src/types/pagination.rs index c0f90e0..95752ef 100644 --- a/src/types/pagination.rs +++ b/src/types/pagination.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use futures_util::Stream; use serde::de::DeserializeOwned; -use crate::error::Result; +use crate::error::{HonchoError, Result}; use crate::http::client::HttpClient; type PageFetcher = Arc< @@ -389,6 +389,8 @@ pub async fn paginate_post( where T: DeserializeOwned + Clone + Send + 'static, { + validate_pagination(page, size)?; + let page_str = page.to_string(); let size_str = size.to_string(); let rev_str; @@ -423,3 +425,17 @@ where }) })) } + +pub(crate) fn validate_pagination(page: u64, size: u64) -> Result<()> { + if page == 0 { + return Err(HonchoError::Validation( + "page must be greater than or equal to 1".to_string(), + )); + } + if !(1..=100).contains(&size) { + return Err(HonchoError::Validation( + "size must be between 1 and 100".to_string(), + )); + } + Ok(()) +} diff --git a/src/types/session.rs b/src/types/session.rs index 45e0168..01b8cd7 100644 --- a/src/types/session.rs +++ b/src/types/session.rs @@ -292,7 +292,7 @@ pub struct SessionListOptions { #[serde(default = "default_page")] #[builder(default = default_page())] pub page: u64, - /// Page size. + /// Page size. Must be in `1..=100`. #[serde(default = "default_size")] #[builder(default = default_size())] pub size: u64, diff --git a/src/types/workspace.rs b/src/types/workspace.rs index ed9b65e..70bdc8d 100644 --- a/src/types/workspace.rs +++ b/src/types/workspace.rs @@ -52,7 +52,7 @@ pub struct WorkspaceConfiguration { #[builder(on(String, into))] #[builder(finish_fn = build)] pub struct WorkspaceCreate { - /// Unique identifier for the new workspace (1-100 chars, `[a-zA-Z0-9_-]+`). + /// Unique identifier for the new workspace (1-512 chars, `[a-zA-Z0-9_-]+`). pub id: String, /// Arbitrary metadata. Defaults to `{}`. #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/src/upload.rs b/src/upload.rs index 287e2f9..724b4f7 100644 --- a/src/upload.rs +++ b/src/upload.rs @@ -10,6 +10,9 @@ use tokio::io::AsyncRead; /// Construct with [`FileSource::bytes`], [`FileSource::path`], or /// [`FileSource::stream`], or convert from [`PathBuf`]/[`std::path::Path`] via the /// `From` impls. +/// +/// The API currently accepts `text/plain`, `application/pdf`, and +/// `application/json`; other MIME types may be rejected by the server. pub enum FileSource { /// Raw bytes with explicit filename and content type. Bytes { @@ -62,6 +65,9 @@ impl std::fmt::Debug for FileSource { impl FileSource { /// Create a `Bytes` variant from explicit parts. + /// + /// The API currently accepts `text/plain`, `application/pdf`, and + /// `application/json`; other MIME types may be rejected by the server. pub fn bytes( filename: impl Into, data: impl Into>, @@ -88,6 +94,9 @@ impl FileSource { /// For files on disk, prefer [`FileSource::path`] which streams from the /// filesystem without buffering. /// + /// The API currently accepts `text/plain`, `application/pdf`, and + /// `application/json`; other MIME types may be rejected by the server. + /// /// # Examples /// /// ``` diff --git a/tests/blocking_smoke.rs b/tests/blocking_smoke.rs index b84a9f9..3323d87 100644 --- a/tests/blocking_smoke.rs +++ b/tests/blocking_smoke.rs @@ -613,12 +613,6 @@ async fn blocking_client_get_configuration() { .mount(&server) .await; - Mock::given(method("GET")) - .and(path("/v3/workspaces/ws1")) - .respond_with(ResponseTemplate::new(200).set_body_json(ws_json_with_config())) - .mount(&server) - .await; - let uri = server.uri(); let config = blocking(move || { let client = Honcho::new(&uri, "ws1").unwrap(); @@ -898,17 +892,6 @@ async fn blocking_client_get_metadata() { .mount(&server) .await; - Mock::given(method("GET")) - .and(path("/v3/workspaces/ws1")) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "id": "ws1", - "metadata": {"env": "test"}, - "configuration": {}, - "created_at": "2025-01-15T10:30:00Z" - }))) - .mount(&server) - .await; - let uri = server.uri(); let meta = blocking(move || { let client = Honcho::new(&uri, "ws1").unwrap(); @@ -955,18 +938,6 @@ async fn blocking_client_refresh() { .mount(&server) .await; - Mock::given(method("GET")) - .and(path("/v3/workspaces/ws1")) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "id": "ws1", - "metadata": {"env": "test"}, - "configuration": {"reasoning": {"enabled": true}}, - "created_at": "2025-01-15T10:30:00Z" - }))) - .up_to_n_times(2) - .mount(&server) - .await; - let uri = server.uri(); blocking(move || { let client = Honcho::new(&uri, "ws1").unwrap(); diff --git a/tests/client_builder.rs b/tests/client_builder.rs index c70e7a1..9e1693e 100644 --- a/tests/client_builder.rs +++ b/tests/client_builder.rs @@ -49,6 +49,59 @@ fn builder_workspace_id_explicit() { assert_eq!(honcho.workspace_id(), "my-workspace"); } +#[test] +fn builder_accepts_valid_workspace_id() { + let honcho = build_honcho( + Honcho::builder() + .base_url("http://localhost:8000") + .workspace_id("abc-XYZ_123") + .build(), + ); + assert_eq!(honcho.workspace_id(), "abc-XYZ_123"); +} + +#[test] +fn builder_rejects_invalid_workspace_ids() { + for workspace_id in ["", "has space", "slash/id", "nonascii-é"] { + let result = Honcho::from_params( + Honcho::builder() + .base_url("http://localhost:8000") + .workspace_id(workspace_id) + .build(), + ); + assert!(result.is_err(), "accepted workspace_id: {workspace_id}"); + } +} + +#[test] +fn builder_rejects_too_long_workspace_id() { + let result = Honcho::from_params( + Honcho::builder() + .base_url("http://localhost:8000") + .workspace_id("a".repeat(513)) + .build(), + ); + assert!(result.is_err()); +} + +#[test] +fn builder_rejects_invalid_base_urls() { + for base_url in ["localhost:8000", "ftp://example.com", "http://"] { + let result = Honcho::from_params(Honcho::builder().base_url(base_url).build()); + assert!(result.is_err(), "accepted base_url: {base_url}"); + } +} + +#[test] +fn builder_normalizes_subpath_trailing_slash() { + let honcho = build_honcho( + Honcho::builder() + .base_url("http://localhost:8000/api/") + .build(), + ); + assert_eq!(honcho.base_url().as_str(), "http://localhost:8000/api"); +} + #[test] fn builder_workspace_id_from_env_then_arg_overrides() { temp_env::with_var("HONCHO_WORKSPACE_ID", Some("env-workspace"), || { diff --git a/tests/client_metadata.rs b/tests/client_metadata.rs index 355b03d..a60e390 100644 --- a/tests/client_metadata.rs +++ b/tests/client_metadata.rs @@ -28,14 +28,15 @@ fn workspace_response( } #[tokio::test] -async fn gets_workspace_metadata_by_id() { +async fn gets_workspace_metadata_by_post_get_or_create() { let server = MockServer::start().await; let metadata = json!({"env": "production", "team": "core"}); let response = workspace_response(metadata, json!({})); - Mock::given(method("GET")) - .and(path("/v3/workspaces/test-ws")) + Mock::given(method("POST")) + .and(path("/v3/workspaces")) + .and(body_json(json!({"id": "test-ws"}))) .respond_with(ResponseTemplate::new(200).set_body_json(response)) .mount(&server) .await; @@ -53,8 +54,9 @@ async fn get_metadata_empty_when_no_metadata() { let response = workspace_response(json!({}), json!({})); - Mock::given(method("GET")) - .and(path("/v3/workspaces/test-ws")) + Mock::given(method("POST")) + .and(path("/v3/workspaces")) + .and(body_json(json!({"id": "test-ws"}))) .respond_with(ResponseTemplate::new(200).set_body_json(response)) .mount(&server) .await; @@ -110,14 +112,15 @@ async fn set_metadata_server_error_returns_error() { } #[tokio::test] -async fn gets_workspace_configuration_by_id() { +async fn gets_workspace_configuration_by_post_get_or_create() { let server = MockServer::start().await; let config = json!({"reasoning": {"enabled": true}}); let response = workspace_response(json!({}), config); - Mock::given(method("GET")) - .and(path("/v3/workspaces/test-ws")) + Mock::given(method("POST")) + .and(path("/v3/workspaces")) + .and(body_json(json!({"id": "test-ws"}))) .respond_with(ResponseTemplate::new(200).set_body_json(response)) .mount(&server) .await; @@ -134,8 +137,9 @@ async fn get_configuration_empty_when_no_configuration() { let response = workspace_response(json!({}), json!({})); - Mock::given(method("GET")) - .and(path("/v3/workspaces/test-ws")) + Mock::given(method("POST")) + .and(path("/v3/workspaces")) + .and(body_json(json!({"id": "test-ws"}))) .respond_with(ResponseTemplate::new(200).set_body_json(response)) .mount(&server) .await; @@ -149,6 +153,29 @@ async fn get_configuration_empty_when_no_configuration() { assert!(result.dream.is_none()); } +#[tokio::test] +async fn gets_workspace_configuration_raw_by_post_get_or_create() { + let server = MockServer::start().await; + + let config = json!({"unknown_future_field": {"enabled": true}}); + let response = workspace_response(json!({}), config); + + Mock::given(method("POST")) + .and(path("/v3/workspaces")) + .and(body_json(json!({"id": "test-ws"}))) + .respond_with(ResponseTemplate::new(200).set_body_json(response)) + .mount(&server) + .await; + + let honcho = Honcho::new(&server.uri(), "test-ws").unwrap(); + let result = honcho.get_configuration_raw().await.unwrap(); + + assert_eq!( + result.get("unknown_future_field").unwrap(), + &json!({"enabled": true}) + ); +} + #[tokio::test] async fn set_configuration_puts_to_workspace_id() { let server = MockServer::start().await; @@ -178,11 +205,12 @@ async fn workspace_id_accessor() { } #[tokio::test] -async fn get_metadata_returns_error_on_404() { +async fn get_metadata_returns_error_when_get_or_create_fails() { let server = MockServer::start().await; - Mock::given(method("GET")) - .and(path("/v3/workspaces/nonexistent")) + Mock::given(method("POST")) + .and(path("/v3/workspaces")) + .and(body_json(json!({"id": "nonexistent"}))) .respond_with(ResponseTemplate::new(404).set_body_json(json!({"error": "not found"}))) .mount(&server) .await; @@ -197,11 +225,12 @@ async fn get_metadata_returns_error_on_404() { } #[tokio::test] -async fn get_configuration_returns_error_on_404() { +async fn get_configuration_returns_error_when_get_or_create_fails() { let server = MockServer::start().await; - Mock::given(method("GET")) - .and(path("/v3/workspaces/nonexistent")) + Mock::given(method("POST")) + .and(path("/v3/workspaces")) + .and(body_json(json!({"id": "nonexistent"}))) .respond_with(ResponseTemplate::new(404).set_body_json(json!({"error": "not found"}))) .mount(&server) .await; @@ -220,3 +249,38 @@ async fn honcho_constructor_rejects_invalid_url() { let result = Honcho::new("not a url", "test-ws"); assert!(result.is_err()); } + +#[tokio::test] +async fn honcho_constructor_rejects_invalid_base_urls() { + for base_url in ["localhost:8000", "ftp://example.com", "http://"] { + let result = Honcho::new(base_url, "test-ws"); + assert!(result.is_err(), "accepted base_url: {base_url}"); + } +} + +#[tokio::test] +async fn honcho_constructor_normalizes_subpath_trailing_slash() { + let honcho = Honcho::new("http://localhost:8000/api/", "test-ws").unwrap(); + assert_eq!(honcho.base_url().as_str(), "http://localhost:8000/api"); +} + +#[tokio::test] +async fn honcho_constructor_rejects_invalid_workspace_ids() { + for workspace_id in ["", "has space", "slash/id", "nonascii-é"] { + let result = Honcho::new("http://localhost:8000", workspace_id); + assert!(result.is_err(), "accepted workspace_id: {workspace_id}"); + } +} + +#[tokio::test] +async fn honcho_constructor_rejects_too_long_workspace_id() { + let workspace_id = "a".repeat(513); + let result = Honcho::new("http://localhost:8000", &workspace_id); + assert!(result.is_err()); +} + +#[tokio::test] +async fn honcho_constructor_accepts_valid_workspace_id() { + let honcho = Honcho::new("http://localhost:8000", "abc-XYZ_123").unwrap(); + assert_eq!(honcho.workspace_id(), "abc-XYZ_123"); +} diff --git a/tests/client_operations.rs b/tests/client_operations.rs index 1d19ce5..32b0b1f 100644 --- a/tests/client_operations.rs +++ b/tests/client_operations.rs @@ -8,6 +8,7 @@ )] use honcho_ai::client::Honcho; +use honcho_ai::error::HonchoError; use honcho_ai::types::dream::QueueStatus; use wiremock::matchers::{body_json, method, path}; use wiremock::{Mock, MockServer, ResponseTemplate}; @@ -141,6 +142,21 @@ async fn schedule_dream_posts_correct_body() { honcho.schedule_dream("alice", None, None).await.unwrap(); } +#[tokio::test] +async fn schedule_dream_rejects_empty_observer_before_request() { + let server = MockServer::start().await; + let honcho = make_honcho(&server, "ws1"); + + let err = honcho.schedule_dream("", None, None).await.unwrap_err(); + assert!(matches!( + err, + HonchoError::Validation(ref message) if message == "observer must not be empty" + )); + + let requests = server.received_requests().await.unwrap(); + assert!(requests.is_empty(), "no request should be sent"); +} + #[tokio::test] async fn delete_workspace_calls_delete() { let server = MockServer::start().await; diff --git a/tests/dialectic_dream_types.rs b/tests/dialectic_dream_types.rs index 3a8034f..94070f1 100644 --- a/tests/dialectic_dream_types.rs +++ b/tests/dialectic_dream_types.rs @@ -83,6 +83,35 @@ fn reasoning_level_default_is_low() { assert_eq!(ReasoningLevel::default(), ReasoningLevel::Low); } +#[test] +fn dialectic_options_validate_accepts_10000_chars() { + let options = DialecticOptions::builder() + .query("a".repeat(10_000)) + .build(); + + options.validate().unwrap(); +} + +#[test] +fn dialectic_options_validate_rejects_10001_chars() { + let options = DialecticOptions::builder() + .query("a".repeat(10_001)) + .build(); + + let err = options.validate().unwrap_err(); + assert_eq!(err.code(), "validation_error"); + assert_eq!(err.message(), "query must be at most 10000 characters"); +} + +#[test] +fn dialectic_options_validate_counts_unicode_chars_not_bytes() { + let options = DialecticOptions::builder() + .query("🦀".repeat(10_000)) + .build(); + + options.validate().unwrap(); +} + // ── Dream ─────────────────────────────────────────────────────────── #[test] diff --git a/tests/error_mapping.rs b/tests/error_mapping.rs index f83ac5f..e15f383 100644 --- a/tests/error_mapping.rs +++ b/tests/error_mapping.rs @@ -279,6 +279,72 @@ fn error_code_is_stable_string() { } } +#[rstest] +#[case( + HonchoError::RateLimit { + message: String::new(), + retry_after: None, + }, + true +)] +#[case( + HonchoError::Server { + status: 500, + message: String::new(), + }, + true +)] +#[case( + HonchoError::Server { + status: 502, + message: String::new(), + }, + true +)] +#[case( + HonchoError::Server { + status: 503, + message: String::new(), + }, + true +)] +#[case( + HonchoError::Server { + status: 504, + message: String::new(), + }, + true +)] +#[case( + HonchoError::Server { + status: 501, + message: String::new(), + }, + false +)] +#[case( + HonchoError::BadRequest { + message: String::new(), + body: None, + }, + false +)] +#[case( + HonchoError::Timeout { + message: String::new(), + }, + true +)] +#[case( + HonchoError::Connection { + message: String::new(), + }, + true +)] +fn retryable_policy_matches_http_client(#[case] err: HonchoError, #[case] expected: bool) { + assert_eq!(err.is_retryable(), expected); +} + use std::error::Error; #[tokio::test] @@ -290,6 +356,7 @@ async fn source_chain_for_transport_and_io_and_decode() { .unwrap_err() .into(); assert!(transport_err.source().is_some()); + assert!(!transport_err.is_retryable()); let json_err = serde_json::from_str::>("{}").unwrap_err(); let decode_err = HonchoError::Decode { diff --git a/tests/pagination.rs b/tests/pagination.rs index 94c5a0f..0e6abcc 100644 --- a/tests/pagination.rs +++ b/tests/pagination.rs @@ -668,6 +668,122 @@ async fn paginate_post_with_reverse_param() { assert!(!page.has_next()); } +#[tokio::test] +async fn paginate_post_sends_page_one_size_one_query() { + use wiremock::matchers::{method, path, query_param}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + use honcho_ai::http::client::HttpClient; + use honcho_ai::types::pagination::paginate_post; + + let server = MockServer::start().await; + let http = HttpClient::from_params( + HttpClient::builder() + .base_url(server.uri()) + .max_retries(0) + .build(), + ) + .unwrap(); + + let page_body = page_json(&["alice"], 1, 1, 1, 1); + + Mock::given(method("POST")) + .and(path("/v3/workspaces/ws1/peers/list")) + .and(query_param("page", "1")) + .and(query_param("size", "1")) + .respond_with(ResponseTemplate::new(200).set_body_json(page_body)) + .expect(1) + .mount(&server) + .await; + + let page: Page = paginate_post(&http, "/v3/workspaces/ws1/peers/list", None, 1, 1, false) + .await + .unwrap(); + + assert_eq!(page.items()[0].id, "alice"); + assert_eq!(page.page(), 1); + assert_eq!(page.size(), 1); +} + +#[tokio::test] +async fn paginate_post_rejects_invalid_page_and_size_before_request() { + use honcho_ai::http::client::HttpClient; + use honcho_ai::types::pagination::paginate_post; + use wiremock::MockServer; + + let server = MockServer::start().await; + let http = HttpClient::from_params( + HttpClient::builder() + .base_url(server.uri()) + .max_retries(0) + .build(), + ) + .unwrap(); + + for (page, size) in [(0, 50), (1, 0), (1, 101)] { + let err = paginate_post::( + &http, + "/v3/workspaces/ws1/peers/list", + None, + page, + size, + false, + ) + .await + .unwrap_err(); + assert!(matches!(err, HonchoError::Validation(_))); + } + + let requests = server.received_requests().await.unwrap(); + assert!( + requests.is_empty(), + "invalid pagination should not send requests" + ); +} + +#[tokio::test] +async fn paginate_post_allows_large_page_and_size_100() { + use wiremock::matchers::{method, path, query_param}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + + use honcho_ai::http::client::HttpClient; + use honcho_ai::types::pagination::paginate_post; + + let server = MockServer::start().await; + let http = HttpClient::from_params( + HttpClient::builder() + .base_url(server.uri()) + .max_retries(0) + .build(), + ) + .unwrap(); + + let page_body = page_json(&[], 0, 9999, 100, 0); + + Mock::given(method("POST")) + .and(path("/v3/workspaces/ws1/peers/list")) + .and(query_param("page", "9999")) + .and(query_param("size", "100")) + .respond_with(ResponseTemplate::new(200).set_body_json(page_body)) + .expect(1) + .mount(&server) + .await; + + let page: Page = paginate_post( + &http, + "/v3/workspaces/ws1/peers/list", + None, + 9999, + 100, + false, + ) + .await + .unwrap(); + + assert_eq!(page.page(), 9999); + assert_eq!(page.size(), 100); +} + // ═══════════════════════════════════════════════════════════════════════ // P1.8 — Error propagation when page 2 returns HTTP 500 // ═══════════════════════════════════════════════════════════════════════ diff --git a/tests/peer_core.rs b/tests/peer_core.rs index 3587df4..3aff5ec 100644 --- a/tests/peer_core.rs +++ b/tests/peer_core.rs @@ -322,6 +322,25 @@ async fn peer_chat_with_session_and_target() { assert_eq!(result, Some("Bob likes Rust".to_owned())); } +#[tokio::test] +async fn peer_chat_with_options_rejects_long_query_without_request() { + let server = MockServer::start().await; + let peer = make_peer(&server).await; + + use honcho_ai::types::dialectic::DialecticOptions; + let options = DialecticOptions::builder() + .query("a".repeat(10_001)) + .stream(false) + .build(); + + let err = peer.chat_with_options(&options).await.unwrap_err(); + assert_eq!(err.code(), "validation_error"); + assert_eq!(err.message(), "query must be at most 10000 characters"); + + let requests = server.received_requests().await.unwrap(); + assert_eq!(requests.len(), 2); +} + // ── F5.4: Card ──────────────────────────────────────────────────────── #[tokio::test]