diff --git a/AGENTS.md b/AGENTS.md index e3851bd..5b6a9fe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,7 @@ crates/idx-cli/ binary `idx` └── tests/ cli.rs (behavior), golden.rs + golden/ (output parity) crates/idx-mcp/ binary `idx-mcp`: stdio MCP tools (server.rs), inputs (params.rs) └── tests/mcp.rs JSON-RPC over stdio against the mock provider +tests/agents/ opt-in agent smoke test: real coding agents against idx-mcp scripts/ smoke, installer, and ownership snapshot publisher ``` @@ -52,4 +53,5 @@ Docs: nix flake check -L # canonical gate (fmt, clippy, tests, package) nix develop --command cargo test # focused loop scripts/live-smoke.sh --compare # live drift check, A/B against an older build +tests/agents/agent-smoke.sh # after changing idx-mcp tools: real agents, opt-in ``` diff --git a/Cargo.toml b/Cargo.toml index 5b2edf9..f91aa1b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ members = ["crates/idx-core", "crates/idx-cli", "crates/idx-mcp"] [workspace.package] version = "0.3.0" edition = "2024" -rust-version = "1.85" +rust-version = "1.89" license = "MIT" repository = "https://github.com/0xrsydn/idx-cli" homepage = "https://github.com/0xrsydn/idx-cli" diff --git a/README.md b/README.md index 9b9a5c1..ab10a92 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ helper tool below is available automatically. cargo install idx-cli ``` -Builds from source; requires Rust `1.85+`. +Builds from source; requires Rust `1.89+`. ### Runtime helper tools @@ -172,7 +172,7 @@ client: ```bash claude mcp add idx -- npx -y idx-mcp # Claude Code, via npm -cargo install idx-mcp # or build it (Rust 1.88+) +cargo install idx-mcp # or build it (Rust 1.89+) nix profile install github:0xrsydn/idx-cli#idx-mcp ``` diff --git a/crates/idx-core/src/api/mod.rs b/crates/idx-core/src/api/mod.rs index 258b122..1a7bf04 100644 --- a/crates/idx-core/src/api/mod.rs +++ b/crates/idx-core/src/api/mod.rs @@ -163,6 +163,19 @@ pub fn resolve_symbol(symbol: &str, exchange: &str) -> Result "ticker symbol cannot be empty".into(), )); } + // Tickers name cache files and provider URLs: letters, digits, and the + // separators real symbols use (BBCA.JK, BRK-B, ^JKSE), nothing else. + if trimmed.len() > 32 + || trimmed.contains("..") + || !trimmed + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '.' | '-' | '^' | '=')) + { + return Err(IdxError::InvalidInput(format!( + "invalid ticker symbol '{}'", + symbol.trim() + ))); + } if let Some((base, suffix)) = trimmed.rsplit_once('.') && !base.is_empty() && !suffix.is_empty() @@ -301,6 +314,26 @@ mod tests { assert_eq!(resolve_symbol("BBCA", "JK").unwrap(), "BBCA.JK"); } + #[test] + fn rejects_symbols_that_are_not_tickers() { + for bad in [ + "../../REPORT", + "a/b", + "a\\b", + "BB CA", + "BBCA..JK", + "BBCA\u{0}", + &"A".repeat(40), + ] { + assert!( + matches!(resolve_symbol(bad, "JK"), Err(IdxError::InvalidInput(_))), + "{bad:?} accepted" + ); + } + assert_eq!(resolve_symbol("^jkse", "JK").unwrap(), "^JKSE.JK"); + assert_eq!(resolve_symbol("BRK-B.US", "JK").unwrap(), "BRK-B.US"); + } + #[test] fn selected_provider_exposes_expected_capabilities_for_msn() { let provider = build_selected_provider(ProviderKind::Msn, false, false); diff --git a/crates/idx-core/src/api/yahoo/client.rs b/crates/idx-core/src/api/yahoo/client.rs index 0f96262..6dc3154 100644 --- a/crates/idx-core/src/api/yahoo/client.rs +++ b/crates/idx-core/src/api/yahoo/client.rs @@ -2,6 +2,7 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::process::Stdio; use std::sync::Mutex; +use std::sync::atomic::{AtomicU64, Ordering}; use std::time::Duration; use crate::api::types::{Interval, Period}; @@ -19,6 +20,9 @@ const CRUMB_FETCH_URL: &str = "https://query1.finance.yahoo.com/v1/test/getcrumb pub(super) struct YahooClient { agent: ureq::Agent, crumb: Mutex>, + /// One jar per client, paired with its crumb: clients in the same process + /// (concurrent MCP calls) must not overwrite each other's cookies. + cookie_jar: PathBuf, } impl YahooClient { @@ -29,9 +33,16 @@ impl YahooClient { .build() .into(); + static NEXT_JAR: AtomicU64 = AtomicU64::new(0); + let cookie_jar = std::env::temp_dir().join(format!( + "idx_yf_{}_{}.txt", + std::process::id(), + NEXT_JAR.fetch_add(1, Ordering::Relaxed) + )); Self { agent, crumb: Mutex::new(None), + cookie_jar, } } @@ -49,10 +60,6 @@ impl YahooClient { ) } - fn cookie_jar_path() -> PathBuf { - std::env::temp_dir().join(format!("idx_yf_{}.txt", std::process::id())) - } - pub(super) fn parse_crumb_body(raw: &str) -> Result { let crumb = raw.trim(); if crumb.is_empty() { @@ -123,7 +130,7 @@ impl YahooClient { fn fetch_crumb_via_curl(&self) -> Result { let binary = curl_impersonate::chrome_curl_binary()?; - let cookie_jar = Self::cookie_jar_path(); + let cookie_jar = &self.cookie_jar; let cookie_jar_str = cookie_jar.to_str().ok_or_else(|| { IdxError::Io(format!( "failed to encode Yahoo cookie jar path {}", @@ -235,7 +242,7 @@ impl YahooClient { ) -> Result { for auth_attempt in 0..2 { let crumb = self.get_or_init_crumb()?; - let cookie_header = match Self::cookie_header_from_jar(&Self::cookie_jar_path()) { + let cookie_header = match Self::cookie_header_from_jar(&self.cookie_jar) { Ok(header) => header, Err(err) => { runtime::warn(format!("failed to parse Yahoo cookie jar: {err}")); @@ -284,6 +291,12 @@ fn jitter() -> Duration { Duration::from_millis(fastrand::u64(0..100)) } +impl Drop for YahooClient { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.cookie_jar); + } +} + #[cfg(test)] mod tests { use super::YahooClient; diff --git a/crates/idx-core/src/cache.rs b/crates/idx-core/src/cache.rs index 3740eed..5e1abb5 100644 --- a/crates/idx-core/src/cache.rs +++ b/crates/idx-core/src/cache.rs @@ -136,7 +136,14 @@ impl Cache { }; let raw = serde_json::to_string_pretty(&entry) .map_err(|e| IdxError::ParseError(e.to_string()))?; - fs::write(path, raw).map_err(|e| IdxError::Io(e.to_string())) + // Write then rename, so a concurrent reader never sees a partial + // entry (and deletes it as corrupt). + let temp = path.with_extension(format!("json.{}.tmp", fastrand::u64(..))); + fs::write(&temp, raw).map_err(|e| IdxError::Io(e.to_string()))?; + fs::rename(&temp, &path).map_err(|e| { + let _ = fs::remove_file(&temp); + IdxError::Io(e.to_string()) + }) } pub fn info(&self) -> Result { @@ -226,7 +233,8 @@ impl Cache { runtime::warn(format!( "corrupted cache entry for {data_type}/{symbol}, treating as miss: {e}" )); - let _ = fs::remove_file(&path); + // Not deleted: a concurrent writer may have just replaced it + // with a valid entry. The next put overwrites it atomically. return Ok(None); } }; @@ -237,17 +245,36 @@ impl Cache { entry.schema_version, CURRENT_SCHEMA_VERSION )); - let _ = fs::remove_file(&path); return Ok(None); } Ok(Some(entry)) } fn entry_path(&self, data_type: &str, symbol: &str) -> PathBuf { - self.root.join(data_type).join(format!("{symbol}.json")) + self.root + .join(safe_file_name(data_type)) + .join(format!("{}.json", safe_file_name(symbol))) } } +/// Keys become single path components: anything that could separate or +/// climb directories is percent-encoded. Keys made of letters, digits, and +/// `._-:^=` (every key idx produces) are unchanged. +fn safe_file_name(key: &str) -> String { + let mut out = String::with_capacity(key.len()); + for (i, byte) in key.bytes().enumerate() { + let safe = byte.is_ascii_alphanumeric() + || matches!(byte, b'_' | b'-' | b':' | b'^' | b'=') + || (byte == b'.' && i > 0); + if safe { + out.push(byte as char); + } else { + out.push_str(&format!("%{byte:02X}")); + } + } + out +} + fn is_expired(fetched_at: DateTime, ttl_secs: u64) -> bool { let ttl = chrono::Duration::from_std(Duration::from_secs(ttl_secs)).unwrap_or(chrono::Duration::MAX); @@ -275,7 +302,7 @@ mod tests { use serde::{Deserialize, Serialize}; - use super::{Cache, CacheEntry}; + use super::{Cache, CacheEntry, safe_file_name}; static TMP_COUNTER: AtomicUsize = AtomicUsize::new(0); @@ -325,7 +352,7 @@ mod tests { } #[test] - fn corrupted_cache_entry_returns_none_and_deletes_file() { + fn corrupted_cache_entry_is_a_miss_until_replaced() { let root = tmp(); let cache = Cache::at(root.clone()); @@ -344,12 +371,16 @@ mod tests { .expect("get should not error"); assert_eq!(result, None, "corrupted entry should be treated as miss"); - // The corrupted file should be deleted - assert!(!path.exists(), "corrupted file should be deleted"); + // Left for the next put to replace (deleting could race a writer). + cache + .put("quote", "CORRUPT.JK", &T { v: 7 }, 3600) + .expect("put"); + let result: Option = cache.get("quote", "CORRUPT.JK").expect("get"); + assert_eq!(result, Some(T { v: 7 })); } #[test] - fn corrupted_cache_entry_get_stale_returns_none_and_deletes_file() { + fn corrupted_cache_entry_get_stale_is_a_miss() { let root = tmp(); let cache = Cache::at(root.clone()); @@ -374,7 +405,24 @@ mod tests { "corrupted entry should be treated as miss in get_stale" ); - // The corrupted file should be deleted - assert!(!path.exists(), "corrupted file should be deleted"); + assert!(path.exists(), "left for the next put to replace"); + } + + #[test] + fn cache_keys_cannot_leave_the_cache_root() { + assert_eq!(safe_file_name("BBCA.JK-3mo-1d"), "BBCA.JK-3mo-1d"); + assert_eq!( + safe_file_name("top-performers:id:50"), + "top-performers:id:50" + ); + assert_eq!(safe_file_name("../../REPORT"), "%2E.%2F..%2FREPORT"); + assert_eq!(safe_file_name(".."), "%2E."); + + let cache = Cache::at(std::path::PathBuf::from("/cache/idx")); + let path = cache.entry_path("msn-quote", "../../REPORT"); + assert_eq!( + path.parent().unwrap(), + std::path::Path::new("/cache/idx/msn-quote") + ); } } diff --git a/crates/idx-core/src/investor.rs b/crates/idx-core/src/investor.rs index c8285dc..0b34596 100644 --- a/crates/idx-core/src/investor.rs +++ b/crates/idx-core/src/investor.rs @@ -47,18 +47,49 @@ impl Investor { } } - fn connect(&self) -> Result { + /// Cross-process lock beside the database. Writers (sync install, import, + /// entity edits) hold it exclusively; readers hold it shared, so a + /// snapshot swap never happens under an open connection. Released when + /// the returned file is dropped. + fn lock_file(&self, exclusive: bool) -> Result { + // Every path to one database (symlinks included) shares one lock. + let real = db::resolve_db_path(&self.db_path()?)?; + lock_at(&real, exclusive) + } + + fn write_lock(&self) -> Result { + self.lock_file(true) + } + + /// A read connection holding the shared lock for its lifetime. + fn connect(&self) -> Result { + let real = db::resolve_db_path(&self.db_path()?)?; + let lock = match lock_at(&real, false) { + Ok(lock) => Some(lock), + // No lock file and none can be created (a read-only filesystem): + // no writer can lock it to replace the database either. + Err(_) if unlockable(&real) => None, + Err(err) => return Err(err), + }; + Ok(ReadConnection { + conn: self.open_db()?, + _lock: lock, + }) + } + + /// For writers, which already hold the exclusive lock. + fn open_db(&self) -> Result { db::open_db_at(&self.db_path()?) } pub fn releases(&self) -> Result, IdxError> { - db::query_releases(&self.connect()?) + db::query_releases(&*self.connect()?) } /// Whether the database has data, and which releases it holds. pub fn status(&self) -> Result { let db_path = self.db_path()?; - let releases = db::query_releases(&db::open_db_at(&db_path)?)?; + let releases = db::query_releases(&*self.connect()?)?; Ok(OwnershipStatus { db_path: db_path.display().to_string(), populated: !releases.is_empty(), @@ -116,7 +147,7 @@ impl Investor { } pub fn search(&self, query: &str, limit: usize) -> Result, IdxError> { - search::fts_search(&self.connect()?, query, limit) + search::fts_search(&*self.connect()?, query, limit) } pub fn cross_holders( @@ -124,7 +155,7 @@ impl Investor { min_tickers: usize, top: usize, ) -> Result, IdxError> { - db::query_cross_holders(&self.connect()?, min_tickers, top) + db::query_cross_holders(&*self.connect()?, min_tickers, top) } pub fn concentration( @@ -132,7 +163,7 @@ impl Investor { by: ConcentrationRank, top: usize, ) -> Result, IdxError> { - db::query_concentration(&self.connect()?, by.as_str(), top) + db::query_concentration(&*self.connect()?, by.as_str(), top) } /// Institutional (Bing) flow; `None` when no flow data exists. @@ -151,7 +182,7 @@ impl Investor { to: &str, ticker: Option<&str>, ) -> Result, IdxError> { - let mut rows = db::query_changes(&self.connect()?, from, to)?; + let mut rows = db::query_changes(&*self.connect()?, from, to)?; if let Some(ticker) = ticker { let ticker = ticker.trim().to_uppercase(); rows.retain(|row| row.ticker_code == ticker); @@ -164,20 +195,22 @@ impl Investor { root: &str, depth: usize, ) -> Result<(Vec, Vec), IdxError> { - graph::query_ownership_graph(&self.connect()?, root, depth) + graph::query_ownership_graph(&*self.connect()?, root, depth) } pub fn unresolved(&self, limit: usize) -> Result, IdxError> { - db::list_unresolved(&self.connect()?, limit) + db::list_unresolved(&*self.connect()?, limit) } pub fn map_alias(&self, alias: &str, entity: &str) -> Result<(), IdxError> { - db::manual_map(&self.connect()?, alias, entity) + let _lock = self.write_lock()?; + db::manual_map(&self.open_db()?, alias, entity) } /// Merges entity `merge` into `keep`, reporting how many rows moved. pub fn merge_entities(&self, keep: i64, merge: i64) -> Result { - let conn = self.connect()?; + let _lock = self.write_lock()?; + let conn = self.open_db()?; let count = |table: &str| -> Result { conn.query_row( &format!("SELECT COUNT(*) FROM {table} WHERE entity_id = ?1"), @@ -216,15 +249,17 @@ impl Investor { force: bool, ) -> Result { let manifest_source = snapshot::resolve_manifest_source(manifest)?; - let db_path = self.db_path()?; - snapshot::sync_snapshot(&manifest_source, &db_path, force) + snapshot::sync_snapshot(&manifest_source, &self.db_path()?, force, lock_at) } /// Imports one report. A release already imported (same SHA-256) is /// skipped unless `force`, which replaces it. pub fn import(&self, source: &ImportSource, force: bool) -> Result { + // Validate and download first (unchanged error order), then lock + // only around the database writes. let input = self.resolve_import_input(source)?; - let conn = self.connect()?; + let _lock = self.write_lock()?; + let conn = self.open_db()?; let sha256 = sha256_file(&input.path)?; if !force && db::release_exists(&conn, &sha256)? { @@ -412,6 +447,61 @@ pub struct OwnershipStatus { pub releases: Vec, } +fn lock_path(db_path: &Path) -> PathBuf { + let mut name = db_path.as_os_str().to_os_string(); + name.push(".lock"); + PathBuf::from(name) +} + +/// Locks `.lock` beside the resolved database path. A shared lock does +/// not need write access, so readers fall back to opening an existing lock +/// file read-only (e.g. one created by another user). +fn lock_at(db_path: &Path, exclusive: bool) -> Result { + let path = lock_path(db_path); + let opened = fs::OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(&path); + let file = match opened { + Ok(file) => file, + Err(_) if !exclusive && path.exists() => fs::File::open(&path) + .map_err(|e| IdxError::Io(format!("failed to open ownership lock: {e}")))?, + Err(e) => return Err(IdxError::Io(format!("failed to open ownership lock: {e}"))), + }; + if exclusive { + file.lock() + } else { + file.lock_shared() + } + .map_err(|e| IdxError::Io(format!("failed to lock ownership database: {e}")))?; + Ok(file) +} + +/// True when the lock file is absent and cannot be created. +fn unlockable(db_path: &Path) -> bool { + let path = lock_path(db_path); + !path.exists() + && matches!( + fs::OpenOptions::new().create(true).truncate(false).write(true).open(&path), + Err(e) if matches!(e.kind(), std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::ReadOnlyFilesystem) + ) +} + +struct ReadConnection { + // Dropped (closed) before the lock it is declared ahead of. + conn: Connection, + _lock: Option, +} + +impl std::ops::Deref for ReadConnection { + type Target = Connection; + + fn deref(&self) -> &Connection { + &self.conn + } +} + /// Holder sources for `Investor::ticker`. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum HolderSource { diff --git a/crates/idx-core/src/ownership/archive.rs b/crates/idx-core/src/ownership/archive.rs index 1c7f3a9..f883c47 100644 --- a/crates/idx-core/src/ownership/archive.rs +++ b/crates/idx-core/src/ownership/archive.rs @@ -1,5 +1,5 @@ use std::fs; -use std::io::{Cursor, Read}; +use std::io::Cursor; use std::path::Path; use chrono::NaiveDate; @@ -177,11 +177,7 @@ fn extract_txt_from_zip(path: &Path) -> Result { continue; } - let mut output = String::new(); - file.read_to_string(&mut output).map_err(|e| { - IdxError::ParseError(format!("failed to decode KSEI archive TXT entry: {e}")) - })?; - return Ok(output); + return super::read_zip_entry_bounded(&mut file, "KSEI archive TXT entry"); } Err(IdxError::ParseError( @@ -201,7 +197,8 @@ fn validate_header(header: &str) -> Result<(), IdxError> { fn parse_archive_date(raw: &str) -> Result { let trimmed = raw.trim(); - if trimmed.len() != 11 { + // Fixed byte offsets below; non-ASCII input would split a character. + if trimmed.len() != 11 || !trimmed.is_ascii() { return Err(IdxError::ParseError(format!( "invalid KSEI archive date `{trimmed}`" ))); @@ -357,4 +354,14 @@ mod tests { assert!(pdf_local_cp.total_shares <= archive_local_cp.total_shares); assert!(pdf_local_cp.percentage_bps <= archive_local_cp.percentage_bps); } + + #[test] + fn non_ascii_archive_dates_are_parse_errors_not_panics() { + // 11 bytes, but byte offsets would split the 'é'. + assert!(super::parse_archive_date("1éJan-2026").is_err()); + assert_eq!( + super::parse_archive_date("27-FEB-2026").unwrap(), + chrono::NaiveDate::from_ymd_opt(2026, 2, 27).unwrap() + ); + } } diff --git a/crates/idx-core/src/ownership/db.rs b/crates/idx-core/src/ownership/db.rs index f298383..54ed388 100644 --- a/crates/idx-core/src/ownership/db.rs +++ b/crates/idx-core/src/ownership/db.rs @@ -133,6 +133,43 @@ pub fn open_db() -> Result { } /// Open the ownership database at `db_path`, creating it and migrating the schema. +/// Where the database file really lives: symlinks are followed by reading +/// the link, so this works even while the target does not exist (a fresh +/// install, or mid-replacement during sync). Creates the parent directory. +pub(crate) fn resolve_db_path(path: &Path) -> Result { + let mut current = path.to_path_buf(); + for _ in 0..16 { + let parent = match current.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent.to_path_buf(), + _ => PathBuf::from("."), + }; + fs::create_dir_all(&parent).map_err(|e| IdxError::Io(e.to_string()))?; + let parent = fs::canonicalize(&parent).map_err(|e| IdxError::Io(e.to_string()))?; + let name = current.file_name().ok_or_else(|| { + IdxError::ConfigError(format!( + "ownership database path has no file name: {}", + path.display() + )) + })?; + let candidate = parent.join(name); + match fs::symlink_metadata(&candidate) { + Ok(meta) if meta.file_type().is_symlink() => { + let target = fs::read_link(&candidate).map_err(|e| IdxError::Io(e.to_string()))?; + current = if target.is_absolute() { + target + } else { + parent.join(target) + }; + } + _ => return Ok(candidate), + } + } + Err(IdxError::Io(format!( + "too many symlink levels resolving {}", + path.display() + ))) +} + pub fn open_db_at(db_path: &Path) -> Result { if let Some(parent) = db_path.parent() { fs::create_dir_all(parent).map_err(|e| IdxError::DatabaseError(e.to_string()))?; @@ -1196,7 +1233,10 @@ pub fn compute_concentration(percentages_bps: &[i64]) -> ConcentrationMetrics { let top1_bps = values.first().copied().unwrap_or(0); let top3_bps: i64 = values.iter().take(3).sum(); let total_bps: i64 = values.iter().sum(); - let hhi: i64 = values.iter().map(|p| (p * p) / 10000).sum(); + let hhi: i64 = values + .iter() + .map(|p| p.saturating_mul(*p) / 10000) + .fold(0, i64::saturating_add); ConcentrationMetrics { top1_bps, @@ -1962,4 +2002,37 @@ mod tests { .unwrap(); assert_eq!(buyers, 1); } + + #[cfg(unix)] + #[test] + fn resolve_db_path_follows_symlinks_even_when_the_target_is_missing() { + let root = std::env::temp_dir().join(format!( + "idx-resolve-{}-{}", + std::process::id(), + fastrand::u64(..) + )); + let real_dir = root.join("real"); + std::fs::create_dir_all(&real_dir).unwrap(); + let real_dir = std::fs::canonicalize(&real_dir).unwrap(); + let link = root.join("link.db"); + // Dangling: the target does not exist yet (fresh install, mid-swap). + std::os::unix::fs::symlink(real_dir.join("ownership.db"), &link).unwrap(); + assert_eq!( + super::resolve_db_path(&link).unwrap(), + real_dir.join("ownership.db") + ); + // Relative link, resolved against the link's directory. + let relative = root.join("relative.db"); + std::os::unix::fs::symlink("real/ownership.db", &relative).unwrap(); + assert_eq!( + super::resolve_db_path(&relative).unwrap(), + real_dir.join("ownership.db") + ); + // A plain path is returned as-is (canonical parent). + assert_eq!( + super::resolve_db_path(&real_dir.join("ownership.db")).unwrap(), + real_dir.join("ownership.db") + ); + let _ = std::fs::remove_dir_all(&root); + } } diff --git a/crates/idx-core/src/ownership/entities.rs b/crates/idx-core/src/ownership/entities.rs index 21c94a0..f2a9873 100644 --- a/crates/idx-core/src/ownership/entities.rs +++ b/crates/idx-core/src/ownership/entities.rs @@ -81,7 +81,11 @@ pub fn parse_id_percentage(s: &str) -> Result { IdxError::ParseError(format!("failed to parse Indonesian percentage '{s}': {e}")) })?; - Ok((whole_i * 100) + frac_i) + whole_i + .checked_mul(100) + .and_then(|bps| bps.checked_add(frac_i)) + .filter(|bps| (0..=10_000).contains(bps)) + .ok_or_else(|| IdxError::ParseError(format!("Indonesian percentage '{s}' is out of range"))) } /// Parse KSEI date format to NaiveDate. @@ -453,4 +457,12 @@ mod tests { assert!(normalize_ksei_row(&raw).is_err()); } + + #[test] + fn huge_percentages_are_parse_errors_not_overflow() { + assert!(super::parse_id_percentage("9223372036854775807,99").is_err()); + assert!(super::parse_id_percentage("40000000,00").is_err()); + assert_eq!(super::parse_id_percentage("100,00").unwrap(), 10_000); + assert_eq!(super::parse_id_percentage("54,94").unwrap(), 5494); + } } diff --git a/crates/idx-core/src/ownership/mod.rs b/crates/idx-core/src/ownership/mod.rs index 6635dae..05700eb 100644 --- a/crates/idx-core/src/ownership/mod.rs +++ b/crates/idx-core/src/ownership/mod.rs @@ -8,3 +8,27 @@ pub mod search; pub mod snapshot; pub mod types; pub mod xlsx; + +use std::io::Read; + +use crate::error::IdxError; + +/// Largest decompressed ZIP entry read into memory. Real IDX worksheets and +/// KSEI TXT archives are tens of MB; this bounds a hostile "zip bomb". +const MAX_ZIP_ENTRY_BYTES: u64 = 256 * 1024 * 1024; + +/// Reads one decompressed ZIP entry as UTF-8, failing past the size limit. +pub(crate) fn read_zip_entry_bounded(entry: impl Read, what: &str) -> Result { + let mut bytes = Vec::new(); + entry + .take(MAX_ZIP_ENTRY_BYTES + 1) + .read_to_end(&mut bytes) + .map_err(|e| IdxError::ParseError(format!("failed to read {what}: {e}")))?; + if bytes.len() as u64 > MAX_ZIP_ENTRY_BYTES { + return Err(IdxError::ParseError(format!( + "{what} expands beyond {MAX_ZIP_ENTRY_BYTES} bytes" + ))); + } + String::from_utf8(bytes) + .map_err(|e| IdxError::ParseError(format!("failed to decode {what}: {e}"))) +} diff --git a/crates/idx-core/src/ownership/snapshot.rs b/crates/idx-core/src/ownership/snapshot.rs index 3b26a18..9c4e6fb 100644 --- a/crates/idx-core/src/ownership/snapshot.rs +++ b/crates/idx-core/src/ownership/snapshot.rs @@ -144,20 +144,34 @@ pub fn parse_manifest(raw: &str) -> Result Ok(manifest) } -pub fn sync_snapshot( +/// `lock(resolved_db_path, exclusive)` takes the database's lock: shared +/// while inspecting the local database, exclusive while rechecking and +/// installing, and never held during the downloads. The path is resolved +/// once, so the lock and the replaced file are always the same database. +pub fn sync_snapshot( manifest_source: &str, db_path: &Path, force: bool, + lock: impl Fn(&Path, bool) -> Result, ) -> Result { + // Results report the configured path; files are read and replaced at the + // resolved one, so a symlinked path updates its target instead of being + // overwritten with a regular file. + let configured_db_path = db_path; + let resolved_db_path = db::resolve_db_path(db_path)?; + let db_path = resolved_db_path.as_path(); let manifest = fetch_manifest(manifest_source)?; - let local_state = inspect_local_db(db_path)?; + let local_state = { + let _shared = lock(db_path, false)?; + inspect_local_db(db_path)? + }; let decision = build_sync_decision(local_state.as_ref(), &manifest.snapshot, force); if !decision.should_download { return Ok(OwnershipSyncResult { action: decision.action, manifest: manifest_source.to_string(), - db_path: db_path.display().to_string(), + db_path: configured_db_path.display().to_string(), snapshot_version: manifest.snapshot.version.clone(), latest_as_of_date: manifest.snapshot.latest_as_of_date, release_count: manifest.snapshot.release_count, @@ -202,18 +216,28 @@ pub fn sync_snapshot( let install_result = (|| -> Result<(), IdxError> { validate_snapshot_db(&temp_path, &manifest.snapshot)?; + let _lock = lock(db_path, true)?; + // The decision was made before the download; an import or edit that + // landed since must not be overwritten without --force. + if !force && !same_local_state(local_state.as_ref(), inspect_local_db(db_path)?.as_ref()) { + return Err(IdxError::DatabaseError( + "the local ownership database changed during sync; rerun `idx ownership sync`" + .to_string(), + )); + } install_snapshot_file(&temp_path, db_path) })(); if install_result.is_err() { let _ = fs::remove_file(&temp_path); + let _ = remove_sqlite_sidecars(&temp_path); } install_result?; Ok(OwnershipSyncResult { action: decision.action, manifest: manifest_source.to_string(), - db_path: db_path.display().to_string(), + db_path: configured_db_path.display().to_string(), snapshot_version: manifest.snapshot.version.clone(), latest_as_of_date: manifest.snapshot.latest_as_of_date, release_count: manifest.snapshot.release_count, @@ -340,8 +364,18 @@ fn inspect_local_db(db_path: &Path) -> Result, IdxErr return Ok(None); } + // Schema setup first: it may write to an empty database, and hashing + // afterwards keeps two inspections of an unchanged database equal. A + // published snapshot already has the schema, so its bytes are untouched. + let releases = { + let conn = Connection::open(db_path).map_err(|e| { + IdxError::DatabaseError(format!("failed to open {}: {e}", db_path.display())) + })?; + db::ensure_schema(&conn)?; + db::query_releases(&conn)? + }; + let file_sha256 = if checkpoint_wal(db_path) { - // Hash before opening: schema setup may write to an empty database. let bytes = fs::read(db_path) .map_err(|e| IdxError::Io(format!("failed to read {}: {e}", db_path.display())))?; Some(sha256_hex(&bytes)) @@ -349,12 +383,6 @@ fn inspect_local_db(db_path: &Path) -> Result, IdxErr None }; - let conn = Connection::open(db_path).map_err(|e| { - IdxError::DatabaseError(format!("failed to open {}: {e}", db_path.display())) - })?; - db::ensure_schema(&conn)?; - let releases = db::query_releases(&conn)?; - Ok(Some(LocalSnapshotState { file_sha256, latest_release: releases.first().cloned(), @@ -362,6 +390,21 @@ fn inspect_local_db(db_path: &Path) -> Result, IdxErr })) } +fn same_local_state( + before: Option<&LocalSnapshotState>, + after: Option<&LocalSnapshotState>, +) -> bool { + match (before, after) { + (None, None) => true, + (Some(before), Some(after)) => { + before.file_sha256.is_some() + && before.file_sha256 == after.file_sha256 + && before.release_count == after.release_count + } + _ => false, + } +} + /// Moves committed WAL frames into the database file, so its hash covers local /// edits too. Returns false when changes remain in the WAL. fn checkpoint_wal(db_path: &Path) -> bool { @@ -446,10 +489,12 @@ fn build_sync_decision( }; } - if snapshot - .replaces_sqlite_sha256 - .iter() - .any(|sha256| file_sha256.is_some_and(|local| sha256.eq_ignore_ascii_case(local))) + // A rebuild never silently drops history the local database has. + if snapshot.release_count >= local.release_count + && snapshot + .replaces_sqlite_sha256 + .iter() + .any(|sha256| file_sha256.is_some_and(|local| sha256.eq_ignore_ascii_case(local))) { return SyncDecision { action: OwnershipSyncAction::Updated, @@ -462,7 +507,9 @@ fn build_sync_decision( } if latest.sha256 == snapshot.latest_release_sha256 { - if latest.row_count != snapshot.latest_row_count { + if latest.row_count != snapshot.latest_row_count + && snapshot.release_count >= local.release_count + { return SyncDecision { action: OwnershipSyncAction::Updated, should_download: true, @@ -608,6 +655,8 @@ fn build_temp_path(db_path: &Path) -> PathBuf { } fn install_snapshot_file(temp_path: &Path, db_path: &Path) -> Result<(), IdxError> { + // Validation opened the temp database; its sidecars are named after it. + remove_sqlite_sidecars(temp_path)?; if let Some(parent) = db_path.parent() && !parent.as_os_str().is_empty() { @@ -717,7 +766,8 @@ fn sha256_hex(bytes: &[u8]) -> String { fn read_text(source: &str, context: &str) -> Result { if is_http_source(source) { - let response = ureq::get(source) + let response = http_agent() + .get(source) .header("User-Agent", USER_AGENT) .header("Accept", "application/json,text/plain;q=0.9,*/*;q=0.8") .call() @@ -733,9 +783,20 @@ fn read_text(source: &str, context: &str) -> Result { .map_err(|e| IdxError::Io(format!("failed to read {context} {}: {e}", path.display()))) } +/// Downloads give up rather than hang: a stalled connection must not keep +/// sync (and anything waiting on it) blocked forever. +fn http_agent() -> ureq::Agent { + ureq::Agent::config_builder() + .timeout_connect(Some(std::time::Duration::from_secs(15))) + .timeout_global(Some(std::time::Duration::from_secs(600))) + .build() + .into() +} + fn read_bytes(source: &str, context: &str, limit_bytes: u64) -> Result, IdxError> { if is_http_source(source) { - let response = ureq::get(source) + let response = http_agent() + .get(source) .header("User-Agent", USER_AGENT) .header( "Accept", @@ -909,6 +970,20 @@ mod tests { assert!(!decision.should_download); } + #[test] + fn build_sync_decision_never_drops_history_through_a_rebuild() { + // A forced same-date rebuild with fewer releases lists the old build; + // a client holding it keeps its history instead of downgrading. + let earlier = "e".repeat(64); + let mut snapshot = same_date_snapshot(vec![earlier.clone()]); + snapshot.release_count = 1; + let mut local = same_release_local(&snapshot, &earlier); + local.release_count = 3; + + let decision = build_sync_decision(Some(&local), &snapshot, false); + assert!(!decision.should_download, "{}", decision.reason); + } + #[test] fn build_sync_decision_keeps_a_locally_modified_database() { // Same release metadata but a file matching no published build (local @@ -1060,6 +1135,23 @@ mod tests { assert_eq!(release_count, 1); } + #[test] + fn inspecting_an_empty_database_twice_sees_the_same_state() { + let dir = std::env::temp_dir().join(format!( + "idx-snapshot-empty-{}-{}", + std::process::id(), + fastrand::u64(..) + )); + std::fs::create_dir_all(&dir).unwrap(); + let db_path = dir.join("ownership.db"); + std::fs::write(&db_path, b"").unwrap(); + + let first = super::inspect_local_db(&db_path).unwrap(); + let second = super::inspect_local_db(&db_path).unwrap(); + assert!(super::same_local_state(first.as_ref(), second.as_ref())); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn local_hash_covers_committed_changes_still_in_the_wal() { let dir = std::env::temp_dir().join(format!( diff --git a/crates/idx-core/src/ownership/xlsx.rs b/crates/idx-core/src/ownership/xlsx.rs index 90f7348..c8650d9 100644 --- a/crates/idx-core/src/ownership/xlsx.rs +++ b/crates/idx-core/src/ownership/xlsx.rs @@ -9,7 +9,7 @@ use std::collections::HashMap; use std::fs; -use std::io::{Cursor, Read}; +use std::io::Cursor; use std::path::Path; use chrono::{Duration, NaiveDate}; @@ -83,11 +83,7 @@ fn read_zip_entry( ))); } }; - let mut output = String::new(); - file.read_to_string(&mut output).map_err(|e| { - IdxError::ParseError(format!("failed to decode ownership XLSX entry {name}: {e}")) - })?; - Ok(Some(output)) + super::read_zip_entry_bounded(&mut file, &format!("ownership XLSX entry {name}")).map(Some) } fn first_worksheet_name(zip: &mut zip::ZipArchive>) -> Result { @@ -131,7 +127,10 @@ fn parse_shared_strings(xml: &str) -> Result, IdxError> { b"rPh" => in_phonetic = true, _ => {} }, - Ok(Event::Empty(e)) if e.local_name().as_ref() == b"si" => strings.push(String::new()), + Ok(Event::Empty(e)) if e.local_name().as_ref() == b"si" => { + check_shared_string_count(strings.len())?; + strings.push(String::new()) + } Ok(Event::Text(text)) if in_text && !in_phonetic => { if let Some(value) = current.as_mut() { let unescaped = text @@ -141,7 +140,10 @@ fn parse_shared_strings(xml: &str) -> Result, IdxError> { } } Ok(Event::End(e)) => match e.local_name().as_ref() { - b"si" => strings.push(current.take().unwrap_or_default()), + b"si" => { + check_shared_string_count(strings.len())?; + strings.push(current.take().unwrap_or_default()) + } b"t" => in_text = false, b"rPh" => in_phonetic = false, _ => {} @@ -167,15 +169,49 @@ struct SheetRow { fn row_number(element: &BytesStart<'_>, fallback: usize) -> Result { for attribute in element.attributes() { let attribute = attribute.map_err(|e| xml_error("worksheet", e))?; - if attribute.key.local_name().as_ref() == b"r" { - if let Ok(number) = String::from_utf8_lossy(&attribute.value).trim().parse() { - return Ok(number); - } + if attribute.key.local_name().as_ref() == b"r" + && let Ok(number) = String::from_utf8_lossy(&attribute.value).trim().parse() + { + return Ok(number); } } Ok(fallback) } +/// Bounds on one worksheet. Real above-1% registers hold about 7,000 rows +/// and a few MB of text. +const MAX_CELL_TEXT_BYTES: usize = 256 * 1024 * 1024; +const MAX_SHEET_ROWS: usize = 1_000_000; + +fn text_cap_error() -> IdxError { + xml_error( + "worksheet", + format!("cell text exceeds {MAX_CELL_TEXT_BYTES} bytes"), + ) +} + +const MAX_SHARED_STRINGS: usize = 2_000_000; + +fn check_shared_string_count(count: usize) -> Result<(), IdxError> { + if count >= MAX_SHARED_STRINGS { + return Err(xml_error( + "shared strings", + format!("more than {MAX_SHARED_STRINGS} entries"), + )); + } + Ok(()) +} + +fn check_row_count(rows: usize) -> Result<(), IdxError> { + if rows >= MAX_SHEET_ROWS { + return Err(xml_error( + "worksheet", + format!("more than {MAX_SHEET_ROWS} rows"), + )); + } + Ok(()) +} + fn parse_sheet_rows(xml: &str, shared_strings: &[String]) -> Result, IdxError> { let mut reader = Reader::from_str(xml); let mut rows = Vec::new(); @@ -183,11 +219,13 @@ fn parse_sheet_rows(xml: &str, shared_strings: &[String]) -> Result = None; let mut value = String::new(); let mut in_value = false; + let mut cell_text_bytes = 0usize; loop { match reader.read_event() { Ok(Event::Start(e)) => match e.local_name().as_ref() { b"row" => { + check_row_count(rows.len())?; row = Some(SheetRow { number: row_number(&e, rows.len() + 1)?, cells: HashMap::new(), @@ -200,10 +238,13 @@ fn parse_sheet_rows(xml: &str, shared_strings: &[String]) -> Result in_value = true, _ => {} }, - Ok(Event::Empty(e)) if e.local_name().as_ref() == b"row" => rows.push(SheetRow { - number: row_number(&e, rows.len() + 1)?, - cells: HashMap::new(), - }), + Ok(Event::Empty(e)) if e.local_name().as_ref() == b"row" => { + check_row_count(rows.len())?; + rows.push(SheetRow { + number: row_number(&e, rows.len() + 1)?, + cells: HashMap::new(), + }) + } Ok(Event::Text(text)) if in_value => { let unescaped = text.unescape().map_err(|e| xml_error("worksheet", e))?; value.push_str(&unescaped); @@ -219,12 +260,18 @@ fn parse_sheet_rows(xml: &str, shared_strings: &[String]) -> Result().map_err(|e| { xml_error("worksheet", format!("bad shared string index: {e}")) })?; - shared_strings.get(index).cloned().ok_or_else(|| { + let shared = shared_strings.get(index).ok_or_else(|| { xml_error( "worksheet", format!("shared string {index} out of range"), ) - })? + })?; + // Check before copying: one large string may be + // referenced from many cells. + if cell_text_bytes + shared.len() > MAX_CELL_TEXT_BYTES { + return Err(text_cap_error()); + } + shared.clone() } // Excel stores the share code TRUE (TRINITI DINAMIK) // as a boolean cell holding 1. @@ -235,6 +282,10 @@ fn parse_sheet_rows(xml: &str, shared_strings: &[String]) -> Result value.clone(), }; + cell_text_bytes += text.len(); + if cell_text_bytes > MAX_CELL_TEXT_BYTES { + return Err(text_cap_error()); + } current_row.cells.insert(column, text); } } diff --git a/crates/idx-mcp/Cargo.toml b/crates/idx-mcp/Cargo.toml index c055a91..4ad4c0d 100644 --- a/crates/idx-mcp/Cargo.toml +++ b/crates/idx-mcp/Cargo.toml @@ -3,8 +3,7 @@ name = "idx-mcp" description = "MCP server for Indonesian stock market (IDX) market, financial, and ownership data" version.workspace = true edition.workspace = true -# rmcp requires 1.88. -rust-version = "1.88" +rust-version.workspace = true license.workspace = true repository.workspace = true homepage.workspace = true @@ -24,7 +23,7 @@ rmcp = { version = "3.5", default-features = false, features = ["server", "macro schemars = "1" serde.workspace = true serde_json.workspace = true -tokio = { version = "1", features = ["macros", "rt-multi-thread", "io-std"] } +tokio = { version = "1", features = ["macros", "rt-multi-thread", "io-std", "sync"] } [dev-dependencies] # Tests drive the binary with the mock provider; release builds exclude it. diff --git a/crates/idx-mcp/src/params.rs b/crates/idx-mcp/src/params.rs index 74e3fee..9c4a1b8 100644 --- a/crates/idx-mcp/src/params.rs +++ b/crates/idx-mcp/src/params.rs @@ -8,7 +8,7 @@ use serde::Deserialize; #[derive(Debug, Deserialize, JsonSchema)] pub struct SymbolsParams { - /// IDX tickers, e.g. ["BBCA", "BBRI"]; the `.JK` suffix is optional. + /// IDX tickers, e.g. ["BBCA", "BBRI"]; the `.JK` suffix is optional. At most 20. pub symbols: Vec, } @@ -36,7 +36,7 @@ pub struct ScreenParams { #[serde(default)] #[schemars(schema_with = "region_schema")] pub region: Option, - /// Maximum results (default 50). + /// Maximum results (default 50, at most 200). #[serde(default)] pub limit: Option, } @@ -130,7 +130,7 @@ pub struct EntityParams { pub struct SearchParams { /// Full-text query over investor names. pub query: String, - /// Maximum results (default 20). + /// Maximum results (default 20, at most 100). #[serde(default)] pub limit: Option, } @@ -144,7 +144,7 @@ pub struct ChangesParams { /// Only this ticker's changes. #[serde(default)] pub ticker: Option, - /// Maximum rows returned (default 200); `total` reports the full count. + /// Maximum rows returned (default 200, at most 2000); `total` reports the full count. #[serde(default)] pub limit: Option, } diff --git a/crates/idx-mcp/src/server.rs b/crates/idx-mcp/src/server.rs index ac817b2..eba79fa 100644 --- a/crates/idx-mcp/src/server.rs +++ b/crates/idx-mcp/src/server.rs @@ -15,6 +15,7 @@ use rmcp::model::{CallToolResult, Implementation, ServerCapabilities, ServerConf use rmcp::{ErrorData, ServerHandler, tool, tool_handler, tool_router}; use serde::Serialize; use serde_json::{Map, Value, json}; +use tokio::sync::Semaphore; use crate::params::{ AnalysisKind, AnalyzeParams, ChangesParams, CompanyParams, CompanySection, EntityParams, @@ -26,6 +27,15 @@ const NEWS_LIMIT: usize = 10; const SCREEN_LIMIT: usize = 50; const SEARCH_LIMIT: usize = 20; const CHANGES_LIMIT: usize = 200; +// Hard caps on what one call may request, so a misbehaving or prompt-injected +// agent cannot turn a call into unbounded upstream traffic or output. +const MAX_SYMBOLS: usize = 20; +const MAX_SCREEN_LIMIT: usize = 200; +const MAX_SEARCH_LIMIT: usize = 100; +const MAX_CHANGES_LIMIT: usize = 2000; + +/// Tool calls running at once; the rest wait their turn. +static CALL_SLOTS: Semaphore = Semaphore::const_new(4); const INSTRUCTIONS: &str = "Indonesian stock market (IDX) data. Market and financial \ tools fetch live data (cached on disk) and wrap it as {data, source, fetched_at, stale}; \ @@ -98,10 +108,10 @@ impl IdxServer { for one or more IDX tickers." )] async fn get_quotes(&self, params: Parameters) -> ToolResult { - let SymbolsParams { symbols } = params.0; - if let Some(err) = empty_input("symbols", &symbols) { - return err; - } + let symbols = match checked_symbols(params.0.symbols) { + Ok(symbols) => symbols, + Err(err) => return err, + }; self.live(move |idx| { let quotes = idx.market().quotes(&symbols, CachePolicy::Default)?; Ok(json!({ "quotes": quotes })) @@ -143,7 +153,7 @@ for one or more IDX tickers." .unwrap_or("top-performers") .parse::()?, region: region.as_deref().unwrap_or("id").parse::()?, - limit: limit.unwrap_or(SCREEN_LIMIT), + limit: bounded_limit("limit", limit, SCREEN_LIMIT, MAX_SCREEN_LIMIT)?, }; idx.market().screen(&query, CachePolicy::Default) }) @@ -157,7 +167,8 @@ earnings, analyst sentiment, insights, and news. Sections that fail are listed u )] async fn get_company(&self, params: Parameters) -> ToolResult { let CompanyParams { symbol, sections } = params.0; - let sections = sections.unwrap_or_else(|| CompanySection::ALL.to_vec()); + let mut sections = sections.unwrap_or_else(|| CompanySection::ALL.to_vec()); + dedup_in_order(&mut sections); if let Some(err) = empty_input("sections", §ions) { return err; } @@ -200,7 +211,8 @@ are listed under `errors`; the call fails only if every analysis fails." )] async fn analyze_stock(&self, params: Parameters) -> ToolResult { let AnalyzeParams { symbol, kinds } = params.0; - let kinds = kinds.unwrap_or_else(|| AnalysisKind::ALL.to_vec()); + let mut kinds = kinds.unwrap_or_else(|| AnalysisKind::ALL.to_vec()); + dedup_in_order(&mut kinds); if let Some(err) = empty_input("kinds", &kinds) { return err; } @@ -232,10 +244,10 @@ are listed under `errors`; the call fails only if every analysis fails." tickers. Tickers that fail are listed under `failures`." )] async fn compare_stocks(&self, params: Parameters) -> ToolResult { - let SymbolsParams { symbols } = params.0; - if let Some(err) = empty_input("symbols", &symbols) { - return err; - } + let symbols = match checked_symbols(params.0.symbols) { + Ok(symbols) => symbols, + Err(err) => return err, + }; self.live(move |idx| { let comparison = idx.analysis().compare(&symbols, CachePolicy::Default)?; let failures: Vec = comparison @@ -273,7 +285,8 @@ with concentration metrics (top1, top3, HHI, free float)." async fn search_investors(&self, params: Parameters) -> ToolResult { let SearchParams { query, limit } = params.0; self.local(move |investor| { - let entities = investor.search(&query, limit.unwrap_or(SEARCH_LIMIT))?; + let limit = bounded_limit("limit", limit, SEARCH_LIMIT, MAX_SEARCH_LIMIT)?; + let entities = investor.search(&query, limit)?; Ok(json!({ "entities": entities })) }) .await @@ -291,9 +304,10 @@ release dates, optionally for one ticker." limit, } = params.0; self.local(move |investor| { + let limit = bounded_limit("limit", limit, CHANGES_LIMIT, MAX_CHANGES_LIMIT)?; let mut changes = investor.changes(&from, &to, ticker.as_deref())?; let total = changes.len(); - changes.truncate(limit.unwrap_or(CHANGES_LIMIT)); + changes.truncate(limit); Ok(json!({ "from": from, "to": to, "total": total, "changes": changes })) }) .await @@ -315,6 +329,7 @@ the local database." async fn ownership_sync(&self, params: Parameters) -> ToolResult { let SyncParams { force } = params.0; let investor = Investor::new(&self.config); + // idx-core's database lock keeps the swap away from open readers. blocking(move || investor.sync(None, force)).await } } @@ -380,6 +395,10 @@ where T: Serialize + Send + 'static, F: FnOnce() -> Result + Send + 'static, { + let _slot = CALL_SLOTS + .acquire() + .await + .map_err(|err| ErrorData::internal_error(err.to_string(), None))?; let result = tokio::task::spawn_blocking(move || f().and_then(to_json)) .await .map_err(|err| ErrorData::internal_error(err.to_string(), None))?; @@ -389,15 +408,67 @@ where }) } +fn input_error(message: String) -> ToolResult { + let err = IdxError::InvalidInput(message); + Ok(CallToolResult::structured_error(Value::Object(error_body( + &err, + )))) +} + /// An explicitly empty list is an input error, as in the CLI, rather than an /// empty success an agent could mistake for data. fn empty_input(field: &str, values: &[T]) -> Option { - values.is_empty().then(|| { - let err = IdxError::InvalidInput(format!("{field} must not be empty")); - Ok(CallToolResult::structured_error(Value::Object(error_body( - &err, - )))) - }) + values + .is_empty() + .then(|| input_error(format!("{field} must not be empty"))) +} + +/// Non-empty, deduplicated (case-insensitively, first wins), and capped. +fn checked_symbols(symbols: Vec) -> Result, ToolResult> { + if let Some(err) = empty_input("symbols", &symbols) { + return Err(err); + } + let mut seen = std::collections::HashSet::new(); + let symbols: Vec = symbols + .into_iter() + .filter(|symbol| seen.insert(symbol.trim().to_uppercase())) + .collect(); + if symbols.len() > MAX_SYMBOLS { + return Err(input_error(format!( + "at most {MAX_SYMBOLS} symbols per call (got {})", + symbols.len() + ))); + } + Ok(symbols) +} + +/// Each section or analysis runs once, however often it is requested. +fn dedup_in_order(values: &mut Vec) { + let mut seen = Vec::with_capacity(values.len()); + values.retain(|value| { + let new = !seen.contains(value); + if new { + seen.push(*value); + } + new + }); +} + +fn bounded_limit( + field: &str, + value: Option, + default: usize, + max: usize, +) -> Result { + match value.unwrap_or(default) { + 0 => Err(IdxError::InvalidInput(format!( + "{field} must be at least 1" + ))), + n if n > max => Err(IdxError::InvalidInput(format!( + "{field} must be at most {max} (got {n})" + ))), + n => Ok(n), + } } fn to_json(value: T) -> Result { diff --git a/crates/idx-mcp/tests/golden/tools.json b/crates/idx-mcp/tests/golden/tools.json new file mode 100644 index 0000000..ffd6b54 --- /dev/null +++ b/crates/idx-mcp/tests/golden/tools.json @@ -0,0 +1,333 @@ +[ + { + "description": "Analyze one IDX ticker: technical signals (trend, RSI, MACD, moving averages) and fundamental growth, valuation, and risk ratings. Analyses that fail are listed under `errors`; the call fails only if every analysis fails.", + "inputSchema": { + "$defs": { + "AnalysisKind": { + "enum": [ + "technical", + "growth", + "valuation", + "risk" + ], + "type": "string" + } + }, + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "kinds": { + "description": "Analyses to run (default: all).", + "items": { + "$ref": "#/$defs/AnalysisKind" + }, + "type": [ + "array", + "null" + ] + }, + "symbol": { + "description": "IDX ticker, e.g. \"BBCA\".", + "type": "string" + } + }, + "required": [ + "symbol" + ], + "type": "object" + }, + "name": "analyze_stock" + }, + { + "description": "Compare fundamental reports (growth, valuation, risk) across IDX tickers. Tickers that fail are listed under `failures`.", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "symbols": { + "description": "IDX tickers, e.g. [\"BBCA\", \"BBRI\"]; the `.JK` suffix is optional. At most 20.", + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "symbols" + ], + "type": "object" + }, + "name": "compare_stocks" + }, + { + "description": "Company data for one IDX ticker: profile, financial statements, earnings, analyst sentiment, insights, and news. Sections that fail are listed under `errors`; the call fails only if every section fails.", + "inputSchema": { + "$defs": { + "CompanySection": { + "enum": [ + "profile", + "financials", + "earnings", + "sentiment", + "insights", + "news" + ], + "type": "string" + } + }, + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "sections": { + "description": "Sections to fetch (default: all).", + "items": { + "$ref": "#/$defs/CompanySection" + }, + "type": [ + "array", + "null" + ] + }, + "symbol": { + "description": "IDX ticker, e.g. \"BBCA\".", + "type": "string" + } + }, + "required": [ + "symbol" + ], + "type": "object" + }, + "name": "get_company" + }, + { + "description": "Every IDX holding (above 1%) of one investor, matched by name.", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "entity": { + "description": "Investor name or part of it, e.g. \"DWIMURIA\"; the closest match is used.", + "type": "string" + } + }, + "required": [ + "entity" + ], + "type": "object" + }, + "name": "get_investor_holdings" + }, + { + "description": "Holder changes (new, exit, increase, decrease) between two KSEI release dates, optionally for one ticker.", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "from": { + "description": "Earlier release date, YYYY-MM-DD (see ownership_status).", + "type": "string" + }, + "limit": { + "default": null, + "description": "Maximum rows returned (default 200, at most 2000); `total` reports the full count.", + "format": "uint", + "minimum": 0, + "type": [ + "integer", + "null" + ] + }, + "ticker": { + "default": null, + "description": "Only this ticker's changes.", + "type": [ + "string", + "null" + ] + }, + "to": { + "description": "Later release date, YYYY-MM-DD.", + "type": "string" + } + }, + "required": [ + "from", + "to" + ], + "type": "object" + }, + "name": "get_ownership_changes" + }, + { + "description": "OHLCV price history for one IDX ticker.", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "interval": { + "default": null, + "description": "Candle size (default \"1d\").", + "enum": [ + "1d", + "1wk", + "1mo" + ], + "type": "string" + }, + "period": { + "default": null, + "description": "Lookback window (default \"3mo\").", + "enum": [ + "1d", + "5d", + "1mo", + "3mo", + "6mo", + "1y", + "2y", + "5y" + ], + "type": "string" + }, + "symbol": { + "description": "IDX ticker, e.g. \"BBCA\".", + "type": "string" + } + }, + "required": [ + "symbol" + ], + "type": "object" + }, + "name": "get_price_history" + }, + { + "description": "Latest quotes (price, change, volume, 52-week range, market cap) for one or more IDX tickers.", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "symbols": { + "description": "IDX tickers, e.g. [\"BBCA\", \"BBRI\"]; the `.JK` suffix is optional. At most 20.", + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "symbols" + ], + "type": "object" + }, + "name": "get_quotes" + }, + { + "description": "Shareholders above 1% of an IDX ticker from the latest KSEI release, with concentration metrics (top1, top3, HHI, free float).", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "ticker": { + "description": "IDX ticker, e.g. \"BBCA\".", + "type": "string" + } + }, + "required": [ + "ticker" + ], + "type": "object" + }, + "name": "get_shareholders" + }, + { + "description": "Ownership database status: whether it has data, and the imported KSEI release dates.", + "inputSchema": { + "properties": {}, + "type": "object" + }, + "name": "ownership_status" + }, + { + "description": "Download or refresh the KSEI ownership snapshot (several MB) into the local database.", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "force": { + "default": false, + "description": "Re-download even when the local snapshot is current.", + "type": "boolean" + } + }, + "type": "object" + }, + "name": "ownership_sync" + }, + { + "description": "Run a stock screen (top/worst performers, high dividend, low P/E, 52-week highs/lows, high volume, large cap).", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "filter": { + "default": null, + "description": "Screen to run (default \"top-performers\").", + "enum": [ + "top-performers", + "worst-performers", + "high-dividend", + "low-pe", + "52w-high", + "52w-low", + "high-volume", + "large-cap" + ], + "type": "string" + }, + "limit": { + "default": null, + "description": "Maximum results (default 50, at most 200).", + "format": "uint", + "minimum": 0, + "type": [ + "integer", + "null" + ] + }, + "region": { + "default": null, + "description": "Market region (default \"id\").", + "enum": [ + "id", + "us", + "sg", + "hk", + "jp" + ], + "type": "string" + } + }, + "type": "object" + }, + "name": "screen_stocks" + }, + { + "description": "Search investor names in the KSEI ownership database.", + "inputSchema": { + "$schema": "https://json-schema.org/draft/2020-12/schema", + "properties": { + "limit": { + "default": null, + "description": "Maximum results (default 20, at most 100).", + "format": "uint", + "minimum": 0, + "type": [ + "integer", + "null" + ] + }, + "query": { + "description": "Full-text query over investor names.", + "type": "string" + } + }, + "required": [ + "query" + ], + "type": "object" + }, + "name": "search_investors" + } +] diff --git a/crates/idx-mcp/tests/mcp.rs b/crates/idx-mcp/tests/mcp.rs index 9581448..e7d07b4 100644 --- a/crates/idx-mcp/tests/mcp.rs +++ b/crates/idx-mcp/tests/mcp.rs @@ -181,6 +181,31 @@ fn lists_grouped_tools_and_hides_sync_unless_allowed() { assert_eq!(client.tool_names(), expected); } +/// What agents see (names, descriptions, input schemas) is part of the +/// contract: changes must show up as a reviewed diff. Regenerate with +/// `UPDATE_GOLDEN=1`. +#[test] +fn tool_definitions_match_golden() { + let mut client = Client::start(&["--allow-sync"], false); + let mut tools = client.request("tools/list", json!({}))["tools"] + .as_array() + .unwrap() + .clone(); + tools.sort_by(|a, b| a["name"].as_str().cmp(&b["name"].as_str())); + let rendered = serde_json::to_string_pretty(&tools).unwrap() + "\n"; + + let golden = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/golden/tools.json"); + if std::env::var_os("UPDATE_GOLDEN").is_some() { + std::fs::create_dir_all(golden.parent().unwrap()).unwrap(); + std::fs::write(&golden, &rendered).unwrap(); + } + let expected = std::fs::read_to_string(&golden).expect("tests/golden/tools.json"); + assert!( + rendered == expected, + "tool definitions changed; review and rerun with UPDATE_GOLDEN=1:\n{rendered}" + ); +} + #[test] fn live_tools_wrap_data_with_freshness() { let mut client = Client::start(&[], false); @@ -254,6 +279,22 @@ fn errors_carry_cli_codes() { assert_eq!(body["code"], "INVALIDINPUT", "{tool}"); } + let many: Vec = (0..21).map(|i| format!("S{i}")).collect(); + for (tool, arguments) in [ + ("get_quotes", json!({"symbols": many})), + ("screen_stocks", json!({"limit": 201})), + ("screen_stocks", json!({"limit": 0})), + ("get_quotes", json!({"symbols": ["../../REPORT"]})), + ] { + let (is_error, body) = client.call(tool, arguments.clone()); + assert!(is_error, "{tool} {arguments}: {body}"); + assert_eq!(body["code"], "INVALIDINPUT", "{tool} {arguments}"); + } + // Duplicates collapse instead of counting against the cap. + let repeated = vec!["BBCA"; 50]; + let quotes = client.ok("get_quotes", json!({"symbols": repeated})); + assert_eq!(quotes["quotes"].as_array().unwrap().len(), 1); + // Empty ownership database: investor tools point at the sync path. let status = client.ok("ownership_status", json!({})); assert_eq!(status["populated"], false); diff --git a/docs/SMOKE.md b/docs/SMOKE.md index 6512288..e14234a 100644 --- a/docs/SMOKE.md +++ b/docs/SMOKE.md @@ -8,7 +8,7 @@ offline with pinned tools: | Check | Covers | | --- | --- | | `fmt`, `clippy` | formatting and lints (`--all-targets`) | -| `test` | unit/module tests, `tests/cli.rs` CLI integration tests, and `tests/golden.rs` output parity | +| `test` | unit/module tests, `tests/cli.rs` CLI integration tests, `tests/golden.rs` output parity, and `crates/idx-mcp/tests/mcp.rs` MCP stdio tests | | `default`, `package` | release build and `cargo package` verification | | `packaged-cli` | the wrapped Nix release binary emits JSON, generates completions, and returns a JSON `CACHEMISS` error with exit 3 offline | | `ownership-publisher`, `publisher-test` | packaged publisher entry point and its regression scenarios | @@ -25,6 +25,11 @@ user-visible output changed. Regenerate with `UPDATE_GOLDEN=1 cargo test --test golden` only when the change is intended, and commit the golden diff with the reason. +`crates/idx-mcp/tests/mcp.rs` drives `idx-mcp` over stdio JSON-RPC with the +mock provider. `crates/idx-mcp/tests/golden/tools.json` pins what agents see (tool +names, descriptions, input schemas); regenerate it the same way after an +intended change. + For a focused loop, run one check or filter Cargo tests: ```bash @@ -66,3 +71,36 @@ and keeps per-case `.stdout`, `.stderr`, and `.validation` logs there. `scripts/audit-msn-fundamentals.sh` is a heavier sweep of valuation output across the whole IDX MSN symbol map, for provider-health audits. + +## Agent smoke test + +The MCP tests prove the protocol works; they cannot show whether a real +coding agent picks the right tools. `tests/agents/agent-smoke.sh` runs each +case in `tests/agents/cases.json` through Claude Code, Codex, pi, Hermes Agent, +and opencode in headless mode against a local `idx-mcp` build, then checks +the tools each agent called (and their key arguments) and, with mock data, key +facts in the answer. It is opt-in: it needs agent logins and model credits, +and agent behavior varies between runs, so it is never part of the gate. + +```bash +nix develop .#agents --command tests/agents/agent-smoke.sh # every agent found +nix develop .#agents --command tests/agents/agent-smoke.sh --agents claude,codex --runs 3 +tests/agents/agent-smoke.sh --live --case shareholders --keep # real data +``` + +- Each agent gets an isolated config that registers only the local server; + your own agent settings and MCP servers are not loaded or changed. Logins + are copied from the agent's home (Codex `auth.json`, pi and Hermes auth + files) into a temp dir that is deleted afterwards; `--keep` keeps the run + output but still deletes every copied agent config. +- Tools are auto-approved for the run (Claude `dontAsk` with + `mcp__idx__*`, Codex `default_tools_approval_mode="approve"`); Claude and pi + also get no built-in tools, so they can only answer through MCP. +- `IDX_AGENT_MODEL_` picks a model (for example + `IDX_AGENT_MODEL_CLAUDE=sonnet`). `IDX_AGENT_EXEC_PREFIX` prefixes every + agent command, for hosts that inject API keys through a wrapper. +- The run fails only when an agent fails a case in every run; use `--runs` to + measure pass rates. `--keep` keeps raw agent output and `results.jsonl`. +- `nix develop .#agents` adds Claude Code, Codex, and opencode from nixpkgs + after your PATH, so installed versions win. pi and Hermes Agent must be + installed on the host. diff --git a/flake.nix b/flake.nix index 9c53422..e6e6a4e 100644 --- a/flake.nix +++ b/flake.nix @@ -280,6 +280,7 @@ shellcheck -s sh ${installerSrc}/install.sh shellcheck ${installerSrc}/scripts/install-sh-test.sh shellcheck ${./scripts/live-smoke.sh} + shellcheck ${./tests/agents/agent-smoke.sh} touch "$out" ''; publisher-test = pkgs.runCommand "idx-publisher-test" { @@ -296,6 +297,23 @@ ''; }; + # `nix develop .#agents`: the default shell plus coding-agent CLIs for + # tests/agents/agent-smoke.sh. They are appended to PATH, so agents + # already installed (and logged in) on the host take precedence. + # pi and Hermes Agent are not in the pinned nixpkgs; use host installs. + devShells.agents = + let + agentPkgs = import nixpkgs { + inherit system; + config.allowUnfreePredicate = pkg: lib.getName pkg == "claude-code"; + }; + in + self.devShells.${system}.default.overrideAttrs (old: { + shellHook = (old.shellHook or "") + '' + export PATH="$PATH:${lib.makeBinPath (with agentPkgs; [ claude-code codex opencode ])}" + ''; + }); + devShells.default = pkgs.mkShell { inputsFrom = [ idxPackage ]; packages = with pkgs; [ diff --git a/scripts/publish-ownership-snapshot.sh b/scripts/publish-ownership-snapshot.sh index dd2c9c3..e520ed7 100755 --- a/scripts/publish-ownership-snapshot.sh +++ b/scripts/publish-ownership-snapshot.sh @@ -464,7 +464,9 @@ MANIFEST_PATH="$OUTPUT_DIR/ownership-snapshot-manifest.json" # A rebuild for the published as-of date (e.g. after a parser fix) lists the # builds it replaces, so clients still holding one update without --force. -if [[ "$BUILT_AS_OF" == "$PUBLISHED_AS_OF" && "$PUBLISHED_SHA256" =~ ^[0-9a-fA-F]{64}$ ]]; then +# Only when it keeps at least the published history: clients never downgrade. +if [[ "$BUILT_AS_OF" == "$PUBLISHED_AS_OF" && "$PUBLISHED_SHA256" =~ ^[0-9a-fA-F]{64}$ ]] \ + && [[ "$PUBLISHED_RELEASE_COUNT" =~ ^[0-9]+$ ]] && (( BUILT_RELEASE_COUNT >= PUBLISHED_RELEASE_COUNT )); then replaces_tmp="$PUBLISH_WORKDIR/manifest-replaces.json" jq --arg published "$PUBLISHED_SHA256" \ --argjson previous "$(jq -c '.snapshot.replaces_sqlite_sha256 // []' <<< "$PUBLISHED_MANIFEST" 2>/dev/null || echo '[]')" ' diff --git a/tests/agents/agent-smoke.sh b/tests/agents/agent-smoke.sh new file mode 100755 index 0000000..54372d6 --- /dev/null +++ b/tests/agents/agent-smoke.sh @@ -0,0 +1,316 @@ +#!/usr/bin/env bash +# Agent smoke test: runs real coding agents headless against a local idx-mcp +# build and checks which MCP tools they call. Opt-in: it needs agent logins +# and model credits, and agent behavior is nondeterministic. It is not part +# of `nix flake check`. See docs/SMOKE.md. +set -Eeuo pipefail + +usage() { + cat <<'EOF' +Usage: tests/agents/agent-smoke.sh [options] + +Runs each case in tests/agents/cases.json through each agent, with an +isolated agent config that registers only the local idx-mcp, and checks the +tools called (and, with mock data, key facts in the answer). + +Options: + --agents Comma-separated: claude,codex,pi,hermes,opencode + (default: every one found on PATH) + --case Run only this case (repeatable) + --runs Runs per agent and case (default: 1) + --live Real upstream data and the published ownership snapshot + instead of mock fixtures; answer text is not checked + --keep Keep the work directory (raw agent output, results.jsonl) + --help Show this help + +Environment: + IDX_AGENT_EXEC_PREFIX Command prefix for every agent, e.g. a secrets + wrapper: "with-ai-secrets ai-tools" + IDX_AGENT_MODEL_ Model override, e.g. IDX_AGENT_MODEL_CLAUDE=sonnet, + IDX_AGENT_MODEL_OPENCODE=opencode-go/glm-5.3 + IDX_AGENT_TIMEOUT Seconds per agent run (default: 300) + +Exit status: 0 when every agent passed every case at least once, 1 when some +agent failed a case in all runs, 2 on usage or setup errors. +EOF +} + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +cases_file="$repo_root/tests/agents/cases.json" +all_agents=(claude codex pi hermes opencode) +agents=() +only_cases=() +runs=1 +live=0 +keep=0 +timeout_secs="${IDX_AGENT_TIMEOUT:-300}" +read -r -a exec_prefix <<< "${IDX_AGENT_EXEC_PREFIX:-}" + +while [[ $# -gt 0 ]]; do + case "$1" in + --agents) IFS=, read -r -a agents <<< "${2:?}"; shift 2 ;; + --case) only_cases+=("${2:?}"); shift 2 ;; + --runs) runs="${2:?}"; shift 2 ;; + --live) live=1; shift ;; + --keep) keep=1; shift ;; + --help|-h) usage; exit 0 ;; + *) usage >&2; exit 2 ;; + esac +done + +[[ "$runs" =~ ^[1-9][0-9]*$ ]] || { echo "--runs must be a positive integer" >&2; exit 2; } +if ((BASH_VERSINFO[0] < 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 4))); then + echo "agent-smoke: bash 4.4+ is required (run it inside nix develop)" >&2 + exit 2 +fi +for tool in jq cargo timeout; do + command -v "$tool" >/dev/null || { echo "agent-smoke: $tool is required" >&2; exit 2; } +done + +if [[ ${#agents[@]} -eq 0 ]]; then + for agent in "${all_agents[@]}"; do + command -v "$agent" >/dev/null && agents+=("$agent") + done +fi +[[ ${#agents[@]} -gt 0 ]] || { echo "agent-smoke: no agent CLI found on PATH" >&2; exit 2; } +for agent in "${agents[@]}"; do + [[ " ${all_agents[*]} " == *" $agent "* ]] || { echo "unknown agent: $agent" >&2; exit 2; } + command -v "$agent" >/dev/null || { echo "agent-smoke: $agent not found on PATH" >&2; exit 2; } +done + +work="$(mktemp -d "${TMPDIR:-/tmp}/idx-agent-smoke.XXXXXX")" +cleanup() { + if [[ "$keep" == 1 ]]; then + # Never leave copied agent logins or settings (which may hold API keys) + # behind; each run's raw output is already under runs/. + rm -rf -- "$work/codex" "$work/pi" "$work/hermes" "$work/opencode" + echo "agent-smoke: kept $work (copied agent configs removed)" + elif [[ "$work" == */idx-agent-smoke.* ]]; then + rm -rf -- "$work" + fi +} +trap cleanup EXIT + +# --- idx-mcp under test ---------------------------------------------------- + +echo "agent-smoke: building idx-mcp ($([[ $live == 1 ]] && echo live || echo mock) data)" +features=() +[[ "$live" == 1 ]] || features=(--features idx-core/testing) +(cd "$repo_root" && cargo build -q --locked -p idx-cli -p idx-mcp "${features[@]}") +idx="$repo_root/target/debug/idx" + +data_env=( + "XDG_CONFIG_HOME=$work/idx/config" + "XDG_CACHE_HOME=$work/idx/cache" + "XDG_DATA_HOME=$work/idx/data" +) +if [[ "$live" == 1 ]]; then + env "${data_env[@]}" "$idx" -o json ownership sync >/dev/null +else + data_env+=(IDX_USE_MOCK_PROVIDER=1) + fixtures="$repo_root/crates/idx-core/tests/fixtures" + # The importer derives the release from the IDX file name. + cp "$fixtures/ksei_above1_20260831_excerpt.xlsx" "$work/peng-2026-08-00017-satu-persen.xlsx" + for file in "$work/peng-2026-08-00017-satu-persen.xlsx" \ + "$fixtures/ksei_balancepos_20260130_excerpt.txt" \ + "$fixtures/ksei_balancepos_20260227_excerpt.txt"; do + env "${data_env[@]}" "$idx" -o json ownership import --file "$file" >/dev/null + done +fi + +# One launcher for every agent: isolated data dirs, and the idx-core crate +# as working directory (the mock provider reads tests/fixtures from there). +launcher="$work/idx-mcp" +{ + echo '#!/usr/bin/env bash' + printf 'export %q\n' "${data_env[@]}" + printf 'cd %q\n' "$repo_root/crates/idx-core" + printf 'exec %q "$@"\n' "$repo_root/target/debug/idx-mcp" +} > "$launcher" +chmod +x "$launcher" + +# --- per-agent setup, run, and tool-call extraction --------------------------- +# Each run_ writes raw output to $out.raw, the answer to $out.answer, and +# one {"name","args"} JSON object per idx tool call to $out.calls, with names +# normalized to the bare tool name. + +copy_if_present() { [[ -f "$1" ]] && cp "$1" "$2/"; return 0; } + +setup_claude() { + printf '{"mcpServers":{"idx":{"type":"stdio","command":"%s","args":[]}}}\n' "$launcher" > "$work/claude-mcp.json" +} +run_claude() { + local prompt="$1" out="$2" model=() + [[ -n "${IDX_AGENT_MODEL_CLAUDE:-}" ]] && model=(--model "$IDX_AGENT_MODEL_CLAUDE") + "${exec_prefix[@]}" claude -p "$prompt" "${model[@]}" \ + --mcp-config "$work/claude-mcp.json" --strict-mcp-config \ + --setting-sources project --no-session-persistence --tools "" \ + --permission-mode dontAsk --allowedTools "mcp__idx__*" \ + --output-format stream-json --verbose --max-turns 10 < /dev/null > "$out.raw" + jq -c 'select(.type=="assistant") | .message.content[]? | select(.type=="tool_use") + | select(.name | startswith("mcp__idx__")) + | {name: (.name | ltrimstr("mcp__idx__")), args: .input}' "$out.raw" > "$out.calls" + jq -r 'select(.type=="result") | .result // ""' "$out.raw" > "$out.answer" +} + +setup_codex() { + mkdir -p "$work/codex" + copy_if_present "${CODEX_HOME:-$HOME/.codex}/auth.json" "$work/codex" +} +run_codex() { + local prompt="$1" out="$2" model=() + [[ -n "${IDX_AGENT_MODEL_CODEX:-}" ]] && model=(-m "$IDX_AGENT_MODEL_CODEX") + CODEX_HOME="$work/codex" "${exec_prefix[@]}" codex exec --json --ephemeral \ + --skip-git-repo-check -s read-only "${model[@]}" \ + -c "mcp_servers.idx.command=\"$launcher\"" -c 'mcp_servers.idx.args=[]' \ + -c 'mcp_servers.idx.default_tools_approval_mode="approve"' \ + "$prompt" < /dev/null > "$out.raw" + jq -c 'select(.type=="item.completed" and .item.type=="mcp_tool_call" and .item.server=="idx") + | select(.item.status=="completed") | {name: .item.tool, args: .item.arguments}' "$out.raw" > "$out.calls" + jq -r 'select(.type=="item.completed" and .item.type=="agent_message") | .item.text' "$out.raw" > "$out.answer" +} + +setup_pi() { + local src="${PI_CODING_AGENT_DIR:-$HOME/.pi/agent}" + mkdir -p "$work/pi" + for file in auth.json models.json settings.json; do copy_if_present "$src/$file" "$work/pi"; done + # "direct" exposes each MCP tool by name (the default "codemode" wraps them). + jq -n --arg cmd "$launcher" '{mcpServers: {idx: {command: $cmd, exposure: "direct"}}}' > "$work/pi/mcp.json" +} +run_pi() { + local prompt="$1" out="$2" model=() + [[ -n "${IDX_AGENT_MODEL_PI:-}" ]] && model=(--model "$IDX_AGENT_MODEL_PI") + # No built-in tools, so the agent can only reach the data through MCP. + PI_CODING_AGENT_DIR="$work/pi" "${exec_prefix[@]}" pi -p --mode json --no-session \ + --no-builtin-tools --no-skills --no-context-files "${model[@]}" "$prompt" < /dev/null > "$out.raw" + jq -c 'select(.type=="tool_execution_start") | select(.toolName | startswith("mcp__idx__")) + | {name: (.toolName | ltrimstr("mcp__idx__")), args: .args}' "$out.raw" > "$out.calls" + jq -r 'select(.type=="agent_end") | .messages[-1].content[]? | select(.type=="text") | .text' "$out.raw" > "$out.answer" +} + +setup_hermes() { + local src="${HERMES_HOME:-$HOME/.hermes}" + mkdir -p "$work/hermes" + copy_if_present "$src/auth.json" "$work/hermes" + copy_if_present "$src/.env" "$work/hermes" + { + # The top-level `model:` block, up to the next top-level key. + [[ -f "$src/config.yaml" ]] && awk '/^[^[:space:]#]/ { in_model = /^model:/ } in_model' "$src/config.yaml" + printf 'mcp_servers:\n idx:\n command: %s\n enabled: true\n' "$launcher" + } > "$work/hermes/config.yaml" +} +run_hermes() { + local prompt="$1" out="$2" model=() session + [[ -n "${IDX_AGENT_MODEL_HERMES:-}" ]] && model=(-m "$IDX_AGENT_MODEL_HERMES") + HERMES_HOME="$work/hermes" "${exec_prefix[@]}" hermes -z "$prompt" "${model[@]}" \ + --usage-file "$out.usage.json" < /dev/null > "$out.answer" + # -z prints only the answer; tool calls come from the session export. + session="$(jq -r '.session_id // empty' "$out.usage.json")" + [[ -n "$session" ]] || return 1 + HERMES_HOME="$work/hermes" hermes sessions export --format jsonl --session-id "$session" \ + "$out.raw" --yes < /dev/null >/dev/null + # Hermes may call MCP tools directly or through its tool_call wrapper. + jq -c '.messages[]? | .tool_calls[]? | .function + | (.arguments | (try fromjson catch {})) as $args + | if .name == "tool_call" then ($args.calls[]? | {name, args: .arguments}) else {name, args: $args} end + | select(.name | startswith("mcp__idx__")) + | .name |= ltrimstr("mcp__idx__")' "$out.raw" > "$out.calls" +} + +setup_opencode() { + mkdir -p "$work/opencode/cwd" + jq -n --arg cmd "$launcher" \ + '{"$schema": "https://opencode.ai/config.json", mcp: {idx: {type: "local", command: [$cmd], enabled: true}}}' \ + > "$work/opencode/cwd/opencode.json" +} +run_opencode() { + local prompt="$1" out="$2" model=() home="$work/opencode" + [[ -n "${IDX_AGENT_MODEL_OPENCODE:-}" ]] && model=(-m "$IDX_AGENT_MODEL_OPENCODE") + (cd "$home/cwd" && XDG_CONFIG_HOME="$home/config" XDG_DATA_HOME="$home/data" \ + XDG_CACHE_HOME="$home/cache" XDG_STATE_HOME="$home/state" \ + "${exec_prefix[@]}" opencode run --pure --format json "${model[@]}" "$prompt" < /dev/null > "$out.raw") + jq -c 'select(.type=="tool_use") | .part | select(.tool | startswith("idx_")) + | select(.state.status=="completed") | {name: (.tool | ltrimstr("idx_")), args: .state.input}' "$out.raw" > "$out.calls" + jq -r 'select(.type=="text") | .part.text' "$out.raw" > "$out.answer" +} + +# --- checks ----------------------------------------------------------------- + +# Prints failure reasons for one run; no output means it passed. +check_run() { + local case_json="$1" out="$2" + jq -rn --argjson case "$case_json" --slurpfile calls "$out.calls" ' + $case.expect_tools[] as $want + | if $want.any then + select([$calls[] | .name] | any(. as $n | $want.any | index($n)) | not) + | "no call to any of \($want.any | join(", "))" + else + ([$calls[] | select(.name == $want.name)]) as $matches + | if ($matches | length) == 0 then "no call to \($want.name)" + elif ($want.args // {}) as $args | ($matches | any(.args | contains($args))) | not + then "\($want.name) called without \($want.args | tojson): \($matches | map(.args) | tojson)" + else empty end + end' || echo "could not parse the agent's tool calls (see $out.raw)" + if [[ "$live" == 0 ]]; then + local pattern + while IFS= read -r pattern; do + # jq regexes (Oniguruma) support (?i) on every platform, unlike grep -P. + jq -Rse --arg re "$pattern" 'test($re)' "$out.answer" >/dev/null \ + || echo "answer does not match /$pattern/" + done < <(jq -r '.expect_text[]?' <<< "$case_json") + fi +} + +# --- run -------------------------------------------------------------------- + +mapfile -t case_ids < <(jq -r '.[].id' "$cases_file") +if [[ ${#only_cases[@]} -gt 0 ]]; then + for id in "${only_cases[@]}"; do + [[ " ${case_ids[*]} " == *" $id "* ]] || { echo "unknown case: $id" >&2; exit 2; } + done + case_ids=("${only_cases[@]}") +fi + +results="$work/results.jsonl" +: > "$results" +echo "agent-smoke: agents=${agents[*]} cases=${#case_ids[@]} runs=$runs" +for agent in "${agents[@]}"; do + "setup_$agent" + for id in "${case_ids[@]}"; do + case_json="$(jq -c --arg id "$id" '.[] | select(.id == $id)' "$cases_file")" + prompt="$(jq -r '.prompt' <<< "$case_json") Use the idx MCP tools." + for ((run = 1; run <= runs; run++)); do + out="$work/runs/$agent/$id.$run" + mkdir -p "$(dirname "$out")" + : > "$out.calls"; : > "$out.answer" + started=$SECONDS + if timeout "$timeout_secs" bash -c "set -eo pipefail; $(declare -f "run_$agent"); $(declare -p work launcher exec_prefix); run_$agent \"\$1\" \"\$2\"" _ "$prompt" "$out" 2> "$out.stderr"; then + # Malformed agent output fails this run, not the whole matrix. + failures="$(check_run "$case_json" "$out")" \ + || failures="${failures:+$failures$'\n'}could not check the agent output (see $out.raw)" + else + failures="agent exited with status $? (see $out.stderr)" + fi + status=pass + [[ -z "$failures" ]] || status=fail + jq -nc --arg agent "$agent" --arg case "$id" --argjson run "$run" --arg status "$status" \ + --arg failures "$failures" --argjson secs "$((SECONDS - started))" \ + --slurpfile calls "$out.calls" \ + '{agent: $agent, case: $case, run: $run, status: $status, secs: $secs, + tools: [$calls[] | .name], failures: ($failures | split("\n") | map(select(. != "")))}' >> "$results" + printf ' %-9s %-13s run %d %s %3ss %s\n' "$agent" "$id" "$run" "$status" "$((SECONDS - started))" \ + "$(jq -r '[.[] | .name] | join(",")' <(jq -s . "$out.calls"))" + [[ -z "$failures" ]] || printf ' - %s\n' "${failures//$'\n'/$'\n - '}" + done + done +done + +echo +echo "agent-smoke: pass rate per agent and case" +jq -rs 'group_by(.agent)[] | "\(.[0].agent): " + (group_by(.case) | map("\(.[0].case) \(map(select(.status=="pass")) | length)/\(length)") | join(" "))' "$results" +consistent_failures="$(jq -s '[group_by([.agent, .case])[] | select(all(.status == "fail"))] | length' "$results")" +if [[ "$consistent_failures" -gt 0 ]]; then + echo "agent-smoke: FAIL ($consistent_failures agent/case pairs failed every run)" + exit 1 +fi +echo "agent-smoke: PASS" diff --git a/tests/agents/cases.json b/tests/agents/cases.json new file mode 100644 index 0000000..37c1816 --- /dev/null +++ b/tests/agents/cases.json @@ -0,0 +1,38 @@ +[ + { + "id": "shareholders", + "prompt": "Who are the shareholders above 5% of BBCA?", + "expect_tools": [{"name": "get_shareholders", "args": {"ticker": "BBCA"}}], + "expect_text": ["(?i)dwimuria", "54[.,]9"] + }, + { + "id": "quote", + "prompt": "What is the latest BBCA share price?", + "expect_tools": [{"name": "get_quotes", "args": {"symbols": ["BBCA"]}}], + "expect_text": ["(?i)bbca"] + }, + { + "id": "compare", + "prompt": "Compare the fundamentals of BBCA and BBRI.", + "expect_tools": [{"name": "compare_stocks", "args": {"symbols": ["BBCA", "BBRI"]}}], + "expect_text": ["(?i)bbri"] + }, + { + "id": "technical", + "prompt": "Give me a technical analysis of BBCA: trend, RSI, and MACD.", + "expect_tools": [{"name": "analyze_stock", "args": {"symbol": "BBCA"}}], + "expect_text": ["(?i)rsi"] + }, + { + "id": "investor", + "prompt": "Which IDX stocks does Anthoni Salim hold above 1%?", + "expect_tools": [{"any": ["get_investor_holdings", "search_investors"]}], + "expect_text": ["(?i)bbca"] + }, + { + "id": "changes", + "prompt": "Which shareholders changed between the 2026-01-30 and 2026-02-27 KSEI releases?", + "expect_tools": [{"name": "get_ownership_changes", "args": {"from": "2026-01-30", "to": "2026-02-27"}}], + "expect_text": [] + } +]