From f5da3b745ef4ffdf26cd10a588c01f81fc4d88f1 Mon Sep 17 00:00:00 2001 From: hermes Date: Fri, 16 Oct 2026 10:46:00 +0000 Subject: [PATCH 01/27] fix(deps): move zip off the yanked 2.4.2, to 9.0.4 Every zip 2.x release from 2.4.2 on is yanked; crates.io warned on each 0.3.2 publish. zip 9 returns entry names as a Result (it validates their encoding): skip undecodable names, which can be neither the archive's TXT payload nor an XLSX worksheet. zip 9 needs Rust 1.88, within our 1.89. Closes #51. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- Cargo.lock | 71 ++++++++++++------------ Cargo.toml | 2 +- crates/idx-core/src/ownership/archive.rs | 5 +- crates/idx-core/src/ownership/xlsx.rs | 4 +- 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c3be9fc..8de2fe0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -107,15 +107,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "arbitrary" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3d036a3c4ab069c7b410a2ce876bd74808d2d0888a82667669f8e783a898bf1" -dependencies = [ - "derive_arbitrary", -] - [[package]] name = "arrayvec" version = "0.7.8" @@ -203,6 +194,12 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f079e83a288787bcd14a6aea84cee5c87a67c5a3e660c30f557a3d24761b3527" + [[package]] name = "chrono" version = "0.4.44" @@ -350,12 +347,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "crossbeam-utils" -version = "0.8.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" - [[package]] name = "crossterm" version = "0.29.0" @@ -453,17 +444,6 @@ dependencies = [ "powerfmt", ] -[[package]] -name = "derive_arbitrary" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e567bd82dcff979e4b03460c307b3cdc9e96fde3d73bed1496d2bc75d9dd62a" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.117", -] - [[package]] name = "difflib" version = "0.4.0" @@ -581,6 +561,7 @@ checksum = "843fba2746e448b37e26a819579957415c8cef339bf08564fe8b7ddbd959573c" dependencies = [ "crc32fast", "miniz_oxide", + "zlib-rs", ] [[package]] @@ -987,9 +968,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.182" +version = "0.2.190" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6800badb6cb2082ffd7b6a67e6125bb39f18782f793520caee8cb8846be06112" +checksum = "ce5d3ddc6d3fa000eb1536d85e147bfe31aacaba692ed6a876f95cb7c855be78" [[package]] name = "libredox" @@ -1066,6 +1047,18 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "nix" +version = "0.31.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf20d2fde8ff38632c426f1165ed7436270b44f199fc55284c38276f9db47c3d" +dependencies = [ + "bitflags", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "nom" version = "7.1.3" @@ -1820,6 +1813,12 @@ dependencies = [ "once_cell", ] +[[package]] +name = "typed-path" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e28f89b80c87b8fb0cf04ab448d5dd0dd0ade2f8891bae878de66a75a28600e" + [[package]] name = "typenum" version = "1.19.0" @@ -2364,21 +2363,25 @@ dependencies = [ [[package]] name = "zip" -version = "2.4.2" +version = "9.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fabe6324e908f85a1c52063ce7aa26b68dcb7eb6dbc83a2d148403c9bc3eba50" +checksum = "a954114b47a54df49f83d2722814e86b23d4bb1b54a1b90fb7d92b41619ddc22" dependencies = [ - "arbitrary", "crc32fast", - "crossbeam-utils", - "displaydoc", "flate2", "indexmap", "memchr", - "thiserror 2.0.18", + "nix", + "typed-path", "zopfli", ] +[[package]] +name = "zlib-rs" +version = "0.6.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b268e58e7c693d7c271f93ffc4ba3b380412554231c85bf61ca7af91042a4112" + [[package]] name = "zmij" version = "1.0.21" diff --git a/Cargo.toml b/Cargo.toml index 59b3c94..ff3f403 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,4 +20,4 @@ chrono = { version = "0.4", features = ["serde"] } ureq = { version = "3", features = ["json", "cookies"] } rusqlite = { version = "0.32", features = ["bundled-full"] } sha2 = "0.10" -zip = { version = "2", default-features = false, features = ["deflate"] } +zip = { version = "9", default-features = false, features = ["deflate"] } diff --git a/crates/idx-core/src/ownership/archive.rs b/crates/idx-core/src/ownership/archive.rs index f883c47..d042308 100644 --- a/crates/idx-core/src/ownership/archive.rs +++ b/crates/idx-core/src/ownership/archive.rs @@ -172,7 +172,10 @@ fn extract_txt_from_zip(path: &Path) -> Result { continue; } - let name = file.name().to_ascii_lowercase(); + // An undecodable entry name cannot be the TXT payload. + let Ok(name) = file.name().map(|name| name.to_ascii_lowercase()) else { + continue; + }; if !name.ends_with(".txt") { continue; } diff --git a/crates/idx-core/src/ownership/xlsx.rs b/crates/idx-core/src/ownership/xlsx.rs index c8650d9..58751f8 100644 --- a/crates/idx-core/src/ownership/xlsx.rs +++ b/crates/idx-core/src/ownership/xlsx.rs @@ -87,11 +87,13 @@ fn read_zip_entry( } fn first_worksheet_name(zip: &mut zip::ZipArchive>) -> Result { + // Undecodable entry names cannot be worksheets; skip them. let mut sheets: Vec = zip .file_names() + .filter_map(Result::ok) .filter(|name| name.starts_with("xl/worksheets/") && name.ends_with(".xml")) .filter(|name| !name["xl/worksheets/".len()..].contains('/')) - .map(str::to_string) + .map(|name| name.into_owned()) .collect(); // sheet1.xml, sheet2.xml, ... sort by their numeric suffix. sheets.sort_by_key(|name| { From 1d3fe5ca88fb676032342c8924ae290b3788e7f3 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 04:09:51 +0000 Subject: [PATCH 02/27] fix(ownership): correct changes, alias search, graph, and name lookups From a full-repo Codex audit, each reproduced on the synced ownership DB: - `changes` paired an investor's accounts across releases instead of comparing totals: KREN's KRESNA PRIMA INVEST went 9.44% -> 13.45% (a second 4.01% account) but was reported as decreased 944 -> 401 bps. Sum a holder's accounts per ticker, and read one release per date (the latest imported, as other queries do), so a corrected file supersedes the original instead of mixing with it. - `changes` / get_ownership_changes accepted any dates: `from: ""` reported all 7,154 holders as new, unknown dates returned no changes. Dates must be YYYY-MM-DD imported releases with `from` before `to`. - get_investor_holdings / `entity` with an empty name matched every investor and returned an arbitrary one ("AMY"); `%` and `_` acted as LIKE wildcards. Reject empty names; match wildcards literally. - Alias search never worked: the FTS table declared an `aliases` column its content table lacks (`no such column: T.aliases`), so searches fell back to canonical names and a raw KSEI name such as "PT ASTRA INTERNATIONAL TBK" found nothing. Search names and aliases directly, ranked exact name, exact alias, prefix, then shorter names. - `graph` enumerated every path (exponential in depth) and its cycle check matched substrings, dropping entity:1 when entity:10 was on the path. Traverse breadth-first with an exact visited set. - Name normalization removed leading PT tokens from the front of a Vec, quadratic in hostile names; narrow a slice instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/investor.rs | 37 +++- crates/idx-core/src/ownership/db.rs | 211 +++++++++++++++++++--- crates/idx-core/src/ownership/entities.rs | 51 +++--- crates/idx-core/src/ownership/graph.rs | 144 ++++++++------- crates/idx-core/src/ownership/search.rs | 180 ++++++++++-------- crates/idx-mcp/tests/mcp.rs | 16 ++ 6 files changed, 440 insertions(+), 199 deletions(-) diff --git a/crates/idx-core/src/investor.rs b/crates/idx-core/src/investor.rs index 0b34596..15c03a8 100644 --- a/crates/idx-core/src/investor.rs +++ b/crates/idx-core/src/investor.rs @@ -134,11 +134,23 @@ impl Investor { /// Holdings of the entity whose name best matches `name` (shortest /// canonical name containing it). pub fn entity(&self, name: &str) -> Result { + let name = name.trim(); + // An empty pattern would match every investor and return an arbitrary one. + if name.is_empty() { + return Err(IdxError::InvalidInput( + "investor name must not be empty".into(), + )); + } let conn = self.connect()?; - let pattern = format!("%{}%", name.trim()); + // `%` and `_` in the name are literal characters, not LIKE wildcards. + let escaped = name + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_"); + let pattern = format!("%{escaped}%"); let entity_id = conn .query_row( - "SELECT id FROM entities WHERE canonical_name LIKE ?1 COLLATE NOCASE ORDER BY LENGTH(canonical_name) ASC LIMIT 1", + "SELECT id FROM entities WHERE canonical_name LIKE ?1 ESCAPE '\\' COLLATE NOCASE ORDER BY LENGTH(canonical_name) ASC LIMIT 1", rusqlite::params![pattern], |row| row.get::<_, i64>(0), ) @@ -147,7 +159,7 @@ impl Investor { } pub fn search(&self, query: &str, limit: usize) -> Result, IdxError> { - search::fts_search(&*self.connect()?, query, limit) + search::search_entities(&*self.connect()?, query, limit) } pub fn cross_holders( @@ -175,14 +187,29 @@ impl Investor { db::query_bing_flow(&conn, ticker_id) } - /// Holder changes between two release dates, optionally for one ticker. + /// Holder changes between two release dates (`from` earlier than `to`, + /// both imported releases), optionally for one ticker. pub fn changes( &self, from: &str, to: &str, ticker: Option<&str>, ) -> Result, IdxError> { - let mut rows = db::query_changes(&*self.connect()?, from, to)?; + let date = |field: &str, value: &str| { + NaiveDate::parse_from_str(value.trim(), "%Y-%m-%d").map_err(|_| { + IdxError::InvalidInput(format!( + "{field} must be a release date as YYYY-MM-DD (got {value:?})" + )) + }) + }; + let (from_date, to_date) = (date("from", from)?, date("to", to)?); + if from_date >= to_date { + return Err(IdxError::InvalidInput(format!( + "from ({from_date}) must be earlier than to ({to_date})" + ))); + } + let (from, to) = (from_date.to_string(), to_date.to_string()); + 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); diff --git a/crates/idx-core/src/ownership/db.rs b/crates/idx-core/src/ownership/db.rs index 54ed388..bafa263 100644 --- a/crates/idx-core/src/ownership/db.rs +++ b/crates/idx-core/src/ownership/db.rs @@ -7,7 +7,6 @@ use rusqlite::{Connection, params}; use crate::config::get_config_value; use crate::error::IdxError; -use crate::ownership::search; use crate::ownership::types::{ BingHolding, ChangeRow, ChangeType, ConcentrationMetrics, CrossHolderRow, Entity, EntityHoldings, EntityTickerRow, FlowSignal, HolderRow, InstitutionalFlow, KseiHolding, @@ -36,14 +35,6 @@ CREATE TABLE IF NOT EXISTS entities ( updated_at INTEGER NOT NULL ); -CREATE VIRTUAL TABLE IF NOT EXISTS entity_fts USING fts5( - canonical_name, - aliases, - content='entities', - content_rowid='id', - tokenize='trigram' -); - CREATE TABLE IF NOT EXISTS entity_aliases ( id INTEGER PRIMARY KEY, entity_id INTEGER NOT NULL REFERENCES entities(id), @@ -916,43 +907,73 @@ pub fn query_bing_flow( })) } -/// Compare two KSEI snapshots by date and return ownership changes. +/// The release used for a report date: the latest imported one, as +/// `latest_ksei_release` picks, so a corrected file supersedes the original. +fn release_for_date(conn: &Connection, date: &str) -> Result, IdxError> { + conn.query_row( + "SELECT sha256 FROM ownership_releases + WHERE as_of_date = ?1 + ORDER BY imported_at DESC, id DESC + LIMIT 1", + params![date], + |row| row.get::<_, String>(0), + ) + .map(Some) + .or_else(|e| match e { + rusqlite::Error::QueryReturnedNoRows => Ok(None), + other => Err(IdxError::DatabaseError(other.to_string())), + }) +} + +/// Compare two KSEI releases by date and return ownership changes. /// Finds: new holders, exited holders, percentage increases/decreases. +/// Each date reads one release, and a holder's accounts in a ticker are +/// summed, so an investor with two accounts compares by its total stake. pub fn query_changes( conn: &Connection, from_date: &str, to_date: &str, ) -> Result, IdxError> { + let release = |date: &str| { + release_for_date(conn, date)?.ok_or_else(|| { + IdxError::InvalidInput(format!( + "no KSEI release imported for {date}; list the imported dates with `idx ownership releases` or the ownership_status tool" + )) + }) + }; + let (from_release, to_release) = (release(from_date)?, release(to_date)?); let mut stmt = conn .prepare( "WITH from_snapshot AS ( SELECT t.code AS ticker_code, - COALESCE(e.canonical_name, k.raw_investor_name) AS entity_name, + MIN(COALESCE(e.canonical_name, k.raw_investor_name)) AS entity_name, CASE WHEN k.entity_id IS NOT NULL THEN 'id:' || k.entity_id ELSE 'raw:' || UPPER(TRIM(k.raw_investor_name)) END AS holder_key, - k.percentage_bps AS old_bps + SUM(k.percentage_bps) AS old_bps FROM ksei_holdings k JOIN tickers t ON t.id = k.ticker_id LEFT JOIN entities e ON e.id = k.entity_id - WHERE k.report_date = ?1 + WHERE k.release_sha256 = ?1 + GROUP BY t.code, holder_key ), to_snapshot AS ( SELECT t.code AS ticker_code, - COALESCE(e.canonical_name, k.raw_investor_name) AS entity_name, + MIN(COALESCE(e.canonical_name, k.raw_investor_name)) AS entity_name, CASE WHEN k.entity_id IS NOT NULL THEN 'id:' || k.entity_id ELSE 'raw:' || UPPER(TRIM(k.raw_investor_name)) END AS holder_key, - k.percentage_bps AS new_bps + SUM(k.percentage_bps) AS new_bps FROM ksei_holdings k JOIN tickers t ON t.id = k.ticker_id LEFT JOIN entities e ON e.id = k.entity_id - WHERE k.report_date = ?2 + WHERE k.release_sha256 = ?2 + GROUP BY t.code, holder_key ), keys AS ( SELECT ticker_code, holder_key FROM from_snapshot @@ -978,7 +999,7 @@ pub fn query_changes( .map_err(|e| IdxError::DatabaseError(e.to_string()))?; let rows = stmt - .query_map(params![from_date, to_date], |row| { + .query_map(params![from_release, to_release], |row| { Ok(( row.get::<_, String>(0)?, row.get::<_, String>(1)?, @@ -1123,7 +1144,6 @@ pub fn manual_map(conn: &Connection, raw_name: &str, canonical_name: &str) -> Re ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - let _ = search::rebuild_fts(conn); Ok(()) })(); @@ -1194,7 +1214,6 @@ pub fn merge_entities(conn: &Connection, keep_id: i64, merge_id: i64) -> Result< conn.execute("DELETE FROM entities WHERE id = ?1", params![merge_id]) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - let _ = search::rebuild_fts(conn); Ok(()) })(); @@ -1383,13 +1402,15 @@ mod tests { use chrono::NaiveDate; use rusqlite::{Connection, params}; + use crate::error::IdxError; use crate::ownership::db::{ compute_concentration, ensure_schema, get_ticker_id, insert_bing_holdings, - insert_ksei_holdings, insert_release, query_concentration, query_cross_holders, - query_ticker_holdings, release_exists, upsert_ticker, write_ksei_release, + insert_ksei_holdings, insert_release, query_changes, query_concentration, + query_cross_holders, query_ticker_holdings, release_exists, upsert_ticker, + write_ksei_release, }; use crate::ownership::types::{ - BingHolding, FlowSignal, KseiHolding, Locality, OwnershipRelease, + BingHolding, ChangeType, FlowSignal, KseiHolding, Locality, OwnershipRelease, }; fn setup() -> Connection { @@ -1398,6 +1419,152 @@ mod tests { conn } + /// Writes one release of `(entity_id, raw_name, bps)` rows for ticker_id. + fn write_release( + conn: &Connection, + sha256: &str, + date: &str, + imported_at: i64, + ticker_id: i64, + rows: &[(Option, &str, i64)], + ) { + let as_of_date = NaiveDate::parse_from_str(date, "%Y-%m-%d").unwrap(); + let holdings: Vec = rows + .iter() + .map(|&(entity_id, raw, bps)| KseiHolding { + id: 0, + ticker_id, + entity_id, + raw_investor_name: raw.to_string(), + investor_type: None, + locality: None, + nationality: None, + domicile: None, + holdings_scripless: 1, + holdings_scrip: 0, + total_shares: 1, + percentage_bps: bps, + report_date: as_of_date, + release_sha256: sha256.to_string(), + }) + .collect(); + let release = OwnershipRelease { + id: 0, + source_url: None, + sha256: sha256.to_string(), + as_of_date, + row_count: holdings.len(), + imported_at, + }; + write_ksei_release(conn, &release, &holdings, false).unwrap(); + } + + fn entity(conn: &Connection, name: &str) -> i64 { + conn.execute( + "INSERT INTO entities (canonical_name, entity_type, country, created_at, updated_at) + VALUES (?1, NULL, NULL, 0, 0)", + params![name], + ) + .unwrap(); + conn.last_insert_rowid() + } + + #[test] + fn changes_sum_an_investors_accounts_in_a_ticker() { + // KREN, 2026-09: a second account took KRESNA from 9.44% to 13.45%. + let conn = setup(); + let kren = upsert_ticker(&conn, "KREN", None).unwrap(); + let kresna = entity(&conn, "KRESNA PRIMA INVEST"); + write_release( + &conn, + "aug", + "2026-08-31", + 1, + kren, + &[(Some(kresna), "PT KRESNA PRIMA INVEST", 944)], + ); + write_release( + &conn, + "sep", + "2026-09-30", + 2, + kren, + &[ + (Some(kresna), "PT KRESNA PRIMA INVEST", 944), + (Some(kresna), "PT. KRESNA PRIMA INVEST", 401), + ], + ); + + let rows = query_changes(&conn, "2026-08-31", "2026-09-30").unwrap(); + + assert_eq!(rows.len(), 1, "{rows:?}"); + assert_eq!(rows[0].change_type, ChangeType::Increased); + assert_eq!( + (rows[0].old_bps, rows[0].new_bps, rows[0].delta_bps), + (Some(944), Some(1345), 401) + ); + } + + #[test] + fn changes_read_the_latest_import_for_a_date() { + // A corrected January file supersedes the original, so Jan -> Feb at + // the corrected 12% shows no change, not a spurious increase. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + let alpha = entity(&conn, "ALPHA"); + write_release( + &conn, + "jan", + "2026-01-30", + 1, + bbca, + &[(Some(alpha), "ALPHA", 1000)], + ); + write_release( + &conn, + "jan-fix", + "2026-01-30", + 2, + bbca, + &[(Some(alpha), "ALPHA", 1200)], + ); + write_release( + &conn, + "feb", + "2026-02-27", + 3, + bbca, + &[(Some(alpha), "ALPHA", 1200)], + ); + + assert!( + query_changes(&conn, "2026-01-30", "2026-02-27") + .unwrap() + .is_empty() + ); + } + + #[test] + fn changes_reject_a_date_without_a_release() { + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + write_release( + &conn, + "feb", + "2026-02-27", + 1, + bbca, + &[(None, "ALPHA", 1200)], + ); + + let err = query_changes(&conn, "2026-01-30", "2026-02-27").unwrap_err(); + + assert!( + matches!(err, IdxError::InvalidInput(ref m) if m.contains("2026-01-30")), + "{err:?}" + ); + } + #[test] fn test_compute_concentration_pure_function() { let metrics = compute_concentration(&[4000, 2500, 1000, 500]); diff --git a/crates/idx-core/src/ownership/entities.rs b/crates/idx-core/src/ownership/entities.rs index f2a9873..bea4ede 100644 --- a/crates/idx-core/src/ownership/entities.rs +++ b/crates/idx-core/src/ownership/entities.rs @@ -109,34 +109,25 @@ pub fn normalize_name(raw: &str) -> String { return upper; } - let mut tokens: Vec<&str> = upper.split_whitespace().collect(); - - loop { - let mut changed = false; - - if let Some(first) = tokens.first().copied() - && matches!(first, "PT" | "PT.") - { - tokens.remove(0); - changed = true; - } - - if let Some(last) = tokens.last().copied() - && matches!( - last, - "TBK" | "Tbk" | "(PERSERO)" | "LIMITED" | "PTE" | "LTD" - ) - { - let _ = tokens.pop(); - changed = true; - } - - if !changed { - break; - } + let tokens: Vec<&str> = upper.split_whitespace().collect(); + // Trim prefix and suffix tokens by narrowing a slice: linear, where + // removing from the front of the Vec was quadratic in hostile names. + let mut kept = tokens.as_slice(); + while let [first, rest @ ..] = kept + && matches!(*first, "PT" | "PT.") + { + kept = rest; + } + while let [rest @ .., last] = kept + && matches!( + *last, + "TBK" | "Tbk" | "(PERSERO)" | "LIMITED" | "PTE" | "LTD" + ) + { + kept = rest; } - collapse_whitespace(&tokens.join(" ")) + collapse_whitespace(&kept.join(" ")) } /// Convert a KseiRawRow into a normalized KseiHoldingDraft. @@ -357,6 +348,14 @@ mod tests { ); } + #[test] + fn normalize_name_strips_many_prefixes_in_linear_time() { + // Removing from the front of a Vec made this quadratic (~5e9 shifts). + let hostile = format!("{}ACME TBK", "PT ".repeat(100_000)); + assert_eq!(normalize_name(&hostile), "ACME"); + assert_eq!(normalize_name("PT TBK"), ""); + } + #[test] fn test_normalize_name() { assert_eq!( diff --git a/crates/idx-core/src/ownership/graph.rs b/crates/idx-core/src/ownership/graph.rs index 981e5cc..c295376 100644 --- a/crates/idx-core/src/ownership/graph.rs +++ b/crates/idx-core/src/ownership/graph.rs @@ -21,75 +21,31 @@ pub fn query_ownership_graph( let root_node_id = detect_root_node(conn, root)?; - let mut visited: BTreeSet = BTreeSet::new(); - { - let mut stmt = conn - .prepare( - "WITH RECURSIVE - latest_ksei AS ( - SELECT sha256 - FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC - LIMIT 1 - ), - latest_bing AS ( - SELECT ticker_id, MAX(report_date) AS report_date - FROM bing_holdings - GROUP BY ticker_id - ), - all_edges AS ( - SELECT - 'entity:' || k.entity_id AS from_id, - 'ticker:' || t.code AS to_id - FROM ksei_holdings k - JOIN tickers t ON t.id = k.ticker_id - JOIN latest_ksei lk ON lk.sha256 = k.release_sha256 - WHERE k.entity_id IS NOT NULL - - UNION - - SELECT - 'entity:' || b.entity_id AS from_id, - 'ticker:' || t.code AS to_id - FROM bing_holdings b - JOIN latest_bing lb - ON lb.ticker_id = b.ticker_id - AND lb.report_date = b.report_date - JOIN tickers t ON t.id = b.ticker_id - WHERE b.entity_id IS NOT NULL - AND b.signal = 'holder' - ), - neighbors AS ( - SELECT from_id AS a, to_id AS b FROM all_edges - UNION - SELECT to_id AS a, from_id AS b FROM all_edges - ), - walk(node_id, depth, path) AS ( - SELECT ?1 AS node_id, 0 AS depth, ?1 AS path - UNION ALL - SELECT n.b, w.depth + 1, w.path || '>' || n.b - FROM walk w - JOIN neighbors n ON n.a = w.node_id - WHERE w.depth < ?2 - AND instr(w.path, n.b) = 0 - ) - SELECT DISTINCT node_id - FROM walk", - ) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - let rows = stmt - .query_map(params![root_node_id, depth as i64], |row| { - row.get::<_, String>(0) - }) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - for row in rows { - visited.insert(row.map_err(|e| IdxError::DatabaseError(e.to_string()))?); - } - } - + // Breadth-first over the latest edges with an exact visited set: linear + // in the graph size, unlike enumerating every path, and a node is never + // mistaken for one whose id merely contains it (`entity:1` in `entity:10`). let mut edges = query_all_edges(conn)?; + let mut neighbors: HashMap<&str, Vec<&str>> = HashMap::new(); + for edge in &edges { + neighbors.entry(&edge.from).or_default().push(&edge.to); + neighbors.entry(&edge.to).or_default().push(&edge.from); + } + let mut visited: BTreeSet = BTreeSet::from([root_node_id.clone()]); + let mut frontier = vec![root_node_id.as_str()]; + for _ in 0..depth { + let mut next = Vec::new(); + for node in frontier { + for &neighbor in neighbors.get(node).into_iter().flatten() { + if visited.insert(neighbor.to_string()) { + next.push(neighbor); + } + } + } + if next.is_empty() { + break; + } + frontier = next; + } edges.retain(|edge| visited.contains(&edge.from) && visited.contains(&edge.to)); let nodes = build_nodes(conn, &visited)?; @@ -380,6 +336,58 @@ mod tests { conn } + #[test] + fn graph_reaches_holders_whose_id_is_a_prefix_of_a_visited_one() { + // From entity:10 via ticker:BB to entity:1; "entity:1" is a substring + // of "entity:10", which the old path check treated as visited. + let conn = setup(); + let bb = upsert_ticker(&conn, "BB", None).unwrap(); + conn.execute( + "INSERT INTO entities (id, canonical_name, created_at, updated_at) + VALUES (10, 'HOLDER TEN', 0, 0), (1, 'HOLDER ONE', 0, 0)", + [], + ) + .unwrap(); + let release = OwnershipRelease { + id: 0, + source_url: None, + sha256: "prefix-graph".to_string(), + as_of_date: NaiveDate::from_ymd_opt(2026, 2, 27).unwrap(), + row_count: 2, + imported_at: 1, + }; + let row = |entity_id: i64, name: &str| KseiHolding { + id: 0, + ticker_id: bb, + entity_id: Some(entity_id), + raw_investor_name: name.to_string(), + investor_type: None, + locality: None, + nationality: None, + domicile: None, + holdings_scripless: 1, + holdings_scrip: 0, + total_shares: 1, + percentage_bps: 600, + report_date: release.as_of_date, + release_sha256: release.sha256.clone(), + }; + write_ksei_release( + &conn, + &release, + &[row(10, "HOLDER TEN"), row(1, "HOLDER ONE")], + false, + ) + .unwrap(); + + let (nodes, edges) = query_ownership_graph(&conn, "HOLDER TEN", 2).unwrap(); + + let mut labels: Vec<_> = nodes.iter().map(|node| node.label.as_str()).collect(); + labels.sort(); + assert_eq!(labels, ["BB", "HOLDER ONE", "HOLDER TEN"]); + assert_eq!(edges.len(), 2); + } + #[test] fn graph_uses_latest_ksei_release_only() { let conn = setup(); diff --git a/crates/idx-core/src/ownership/search.rs b/crates/idx-core/src/ownership/search.rs index 6e63532..40fd1f1 100644 --- a/crates/idx-core/src/ownership/search.rs +++ b/crates/idx-core/src/ownership/search.rs @@ -3,47 +3,50 @@ use rusqlite::{Connection, params}; use crate::error::IdxError; use crate::ownership::types::Entity; -pub fn fts_search(conn: &Connection, query: &str, limit: usize) -> Result, IdxError> { +/// Investors whose canonical name or any recorded alias (the raw names KSEI +/// prints, such as `PT ASTRA INTERNATIONAL TBK`) contains `query`, ignoring +/// ASCII case. Ranked by an exact name, then an exact alias, then a name +/// prefix, then shorter names. +pub fn search_entities( + conn: &Connection, + query: &str, + limit: usize, +) -> Result, IdxError> { let q = query.trim(); if q.is_empty() { return Ok(Vec::new()); } - - let limit_i64 = + let limit = i64::try_from(limit).map_err(|e| IdxError::DatabaseError(format!("invalid limit: {e}")))?; + // `%` and `_` in the query are literal characters, not LIKE wildcards. + let escaped = q + .replace('\\', "\\\\") + .replace('%', "\\%") + .replace('_', "\\_"); + let contains = format!("%{escaped}%"); + let prefix = format!("{escaped}%"); - // Ensure entity_fts has content. For external-content FTS table, populate manually. - let fts_count: i64 = conn - .query_row("SELECT COUNT(*) FROM entity_fts", [], |row| row.get(0)) - .unwrap_or(0); - - if fts_count == 0 { - let _ = rebuild_fts(conn); - } - - if let Ok(rows) = fts_query(conn, q, limit_i64) - && !rows.is_empty() - { - return Ok(rows); - } - - like_query(conn, q, limit_i64) -} - -fn fts_query(conn: &Connection, query: &str, limit: i64) -> Result, IdxError> { let mut stmt = conn .prepare( - "SELECT e.id, e.canonical_name, e.entity_type, e.country - FROM entity_fts f - JOIN entities e ON e.id = f.rowid - WHERE entity_fts MATCH ?1 - ORDER BY rank - LIMIT ?2", + "SELECT e.id, e.canonical_name, e.entity_type, e.country, + MIN(CASE + WHEN UPPER(e.canonical_name) = UPPER(?1) THEN 0 + WHEN UPPER(a.raw_name) = UPPER(?1) THEN 1 + WHEN e.canonical_name LIKE ?3 ESCAPE '\\' THEN 2 + ELSE 3 + END) AS match_rank + FROM entities e + LEFT JOIN entity_aliases a ON a.entity_id = e.id + WHERE e.canonical_name LIKE ?2 ESCAPE '\\' + OR a.raw_name LIKE ?2 ESCAPE '\\' + GROUP BY e.id + ORDER BY match_rank, LENGTH(e.canonical_name), e.canonical_name + LIMIT ?4", ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; let rows = stmt - .query_map(params![query, limit], |row| { + .query_map(params![q, contains, prefix, limit], |row| { Ok(Entity { id: row.get(0)?, canonical_name: row.get(1)?, @@ -53,59 +56,80 @@ fn fts_query(conn: &Connection, query: &str, limit: i64) -> Result, }) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - let mut out = Vec::new(); - for row in rows { - out.push(row.map_err(|e| IdxError::DatabaseError(e.to_string()))?); + rows.map(|row| row.map_err(|e| IdxError::DatabaseError(e.to_string()))) + .collect() +} + +#[cfg(test)] +mod tests { + use rusqlite::{Connection, params}; + + use super::search_entities; + use crate::ownership::db::ensure_schema; + + fn conn_with(entities: &[(&str, &[&str])]) -> Connection { + let conn = Connection::open_in_memory().unwrap(); + ensure_schema(&conn).unwrap(); + for (name, aliases) in entities { + conn.execute( + "INSERT INTO entities (canonical_name, created_at, updated_at) VALUES (?1, 0, 0)", + params![name], + ) + .unwrap(); + let id = conn.last_insert_rowid(); + for alias in *aliases { + conn.execute( + "INSERT INTO entity_aliases (entity_id, raw_name, source, method) + VALUES (?1, ?2, 'ksei', 'exact')", + params![id, alias], + ) + .unwrap(); + } + } + conn } - Ok(out) -} -fn like_query(conn: &Connection, query: &str, limit: i64) -> Result, IdxError> { - let mut stmt = conn - .prepare( - "SELECT id, canonical_name, entity_type, country - FROM entities - WHERE canonical_name LIKE ?1 COLLATE NOCASE - ORDER BY canonical_name ASC - LIMIT ?2", - ) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - let like = format!("%{query}%"); - let rows = stmt - .query_map(params![like, limit], |row| { - Ok(Entity { - id: row.get(0)?, - canonical_name: row.get(1)?, - entity_type: row.get(2)?, - country: row.get(3)?, - }) - }) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; - - let mut out = Vec::new(); - for row in rows { - out.push(row.map_err(|e| IdxError::DatabaseError(e.to_string()))?); + fn names(conn: &Connection, query: &str) -> Vec { + search_entities(conn, query, 10) + .unwrap() + .into_iter() + .map(|e| e.canonical_name) + .collect() } - Ok(out) -} -pub fn rebuild_fts(conn: &Connection) -> Result<(), IdxError> { - conn.execute("DELETE FROM entity_fts", []) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; + #[test] + fn finds_investors_by_alias() { + let conn = conn_with(&[ + ("ASTRA INTERNATIONAL", &["PT ASTRA INTERNATIONAL TBK"]), + ("ASTRA OTOPARTS", &[]), + ]); - conn.execute( - "INSERT INTO entity_fts(rowid, canonical_name, aliases) - SELECT e.id, e.canonical_name, COALESCE(a.aliases, '') - FROM entities e - LEFT JOIN ( - SELECT entity_id, GROUP_CONCAT(raw_name, ' ') AS aliases - FROM entity_aliases - GROUP BY entity_id - ) a ON a.entity_id = e.id", - [], - ) - .map_err(|e| IdxError::DatabaseError(e.to_string()))?; + assert_eq!( + names(&conn, "pt astra international tbk"), + ["ASTRA INTERNATIONAL"] + ); + } - Ok(()) + #[test] + fn ranks_exact_then_prefix_then_shorter_and_lists_each_once() { + let conn = conn_with(&[ + ("SALIM GROUP HOLDINGS", &["PT SALIM", "SALIM"]), + ("ANTHONI SALIM", &[]), + ("SALIM", &[]), + ]); + + assert_eq!( + names(&conn, "salim"), + ["SALIM", "SALIM GROUP HOLDINGS", "ANTHONI SALIM"] + ); + } + + #[test] + fn treats_like_wildcards_literally_and_empty_queries_as_no_match() { + let conn = conn_with(&[("ALPHA_FUND", &[]), ("ALPHAXFUND", &[]), ("BETA 100%", &[])]); + + assert_eq!(names(&conn, "alpha_fund"), ["ALPHA_FUND"]); + assert_eq!(names(&conn, "100%"), ["BETA 100%"]); + assert!(names(&conn, " ").is_empty()); + } } diff --git a/crates/idx-mcp/tests/mcp.rs b/crates/idx-mcp/tests/mcp.rs index 906c87f..8438bfc 100644 --- a/crates/idx-mcp/tests/mcp.rs +++ b/crates/idx-mcp/tests/mcp.rs @@ -394,6 +394,22 @@ fn ownership_tools_read_the_local_database() { assert!(page["next_offset"].is_null(), "offset {past}: {page}"); } + // Bad, unknown, or reversed dates are errors, not "everyone is new". + for (from, to) in [ + ("", "2026-02-27"), + ("1999-01-01", "2026-02-27"), + ("2026-02-27", "2026-01-30"), + ("2026-01-30", "2026-01-30"), + ] { + let (is_error, body) = + client.call("get_ownership_changes", json!({"from": from, "to": to})); + assert!(is_error, "{from}..{to}: {body}"); + assert_eq!(body["code"], "INVALIDINPUT", "{from}..{to}: {body}"); + } + let (is_error, body) = client.call("get_investor_holdings", json!({"entity": " "})); + assert!(is_error, "{body}"); + assert_eq!(body["code"], "INVALIDINPUT", "{body}"); + let (is_error, body) = client.call( "get_ownership_changes", json!({"from": "2026-01-30", "to": "2026-02-27", "limit": 301}), From 6abc4a071208508c52bcf71aa9414adcdec6cc4f Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:07:41 +0000 Subject: [PATCH 03/27] fix(ownership): one release order everywhere; keep the old search API From the Codex re-gate of the 0.3.3 fixes: - `changes` broke ties between same-date releases by row id, but the ticker, entity, and graph queries and the snapshot builder did not, so two files imported in the same second could resolve differently per query. Order by as_of_date, imported_at, then id everywhere. - search::fts_search and search::rebuild_fts were public in a published crate; keep them as deprecated wrappers. - Invalid --source and --by are INVALIDINPUT, like other argument errors. - The prefix-ranking test could not tell prefix from exact-alias ranking; add one where only prefix vs substring differs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/investor.rs | 4 +-- crates/idx-core/src/ownership/db.rs | 37 +++++++++++++++++++++++-- crates/idx-core/src/ownership/graph.rs | 2 +- crates/idx-core/src/ownership/search.rs | 22 +++++++++++++++ scripts/build-ownership-snapshot.sh | 6 ++-- 5 files changed, 63 insertions(+), 8 deletions(-) diff --git a/crates/idx-core/src/investor.rs b/crates/idx-core/src/investor.rs index 15c03a8..001fdaa 100644 --- a/crates/idx-core/src/investor.rs +++ b/crates/idx-core/src/investor.rs @@ -555,7 +555,7 @@ impl FromStr for HolderSource { "all" => Ok(Self::All), "ksei" => Ok(Self::Ksei), "bing" => Ok(Self::Bing), - _ => Err(IdxError::ParseError( + _ => Err(IdxError::InvalidInput( "invalid --source, expected: all|ksei|bing".to_string(), )), } @@ -588,7 +588,7 @@ impl FromStr for ConcentrationRank { "top1" => Ok(Self::Top1), "top3" => Ok(Self::Top3), "hhi" => Ok(Self::Hhi), - _ => Err(IdxError::ParseError( + _ => Err(IdxError::InvalidInput( "invalid --by, expected: top1|top3|hhi".to_string(), )), } diff --git a/crates/idx-core/src/ownership/db.rs b/crates/idx-core/src/ownership/db.rs index bafa263..7668c6b 100644 --- a/crates/idx-core/src/ownership/db.rs +++ b/crates/idx-core/src/ownership/db.rs @@ -797,7 +797,7 @@ pub fn query_releases(conn: &Connection) -> Result, IdxErr .prepare( "SELECT id, source_url, sha256, as_of_date, row_count, imported_at FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC", + ORDER BY as_of_date DESC, imported_at DESC, id DESC", ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; @@ -1298,7 +1298,7 @@ fn latest_ksei_release(conn: &Connection) -> Result, I conn.query_row( "SELECT sha256, as_of_date FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC + ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1", [], |row| { @@ -1544,6 +1544,39 @@ mod tests { ); } + #[test] + fn same_second_imports_resolve_to_the_later_file_everywhere() { + // Original and corrected file for one date imported in the same + // second: the ticker view and `changes` must read the same one. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + let alpha = entity(&conn, "ALPHA"); + write_release( + &conn, + "jan", + "2026-01-30", + 7, + bbca, + &[(Some(alpha), "ALPHA", 1000)], + ); + write_release( + &conn, + "jan-fix", + "2026-01-30", + 7, + bbca, + &[(Some(alpha), "ALPHA", 1200)], + ); + + let ticker = query_ticker_holdings(&conn, "BBCA").unwrap(); + + assert_eq!( + ticker.holders[0].percentage_bps, 1200, + "{:?}", + ticker.holders + ); + } + #[test] fn changes_reject_a_date_without_a_release() { let conn = setup(); diff --git a/crates/idx-core/src/ownership/graph.rs b/crates/idx-core/src/ownership/graph.rs index c295376..d51fd12 100644 --- a/crates/idx-core/src/ownership/graph.rs +++ b/crates/idx-core/src/ownership/graph.rs @@ -182,7 +182,7 @@ fn query_all_edges(conn: &Connection) -> Result, IdxError> { if let Ok(release_sha) = conn.query_row( "SELECT sha256 FROM ownership_releases - ORDER BY as_of_date DESC, imported_at DESC + ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1", [], |row| row.get::<_, String>(0), diff --git a/crates/idx-core/src/ownership/search.rs b/crates/idx-core/src/ownership/search.rs index 40fd1f1..9b0af1e 100644 --- a/crates/idx-core/src/ownership/search.rs +++ b/crates/idx-core/src/ownership/search.rs @@ -60,6 +60,19 @@ pub fn search_entities( .collect() } +/// Former name of [`search_entities`], kept for library users. +#[deprecated(since = "0.3.3", note = "use search_entities")] +pub fn fts_search(conn: &Connection, query: &str, limit: usize) -> Result, IdxError> { + search_entities(conn, query, limit) +} + +/// Search no longer uses an FTS index, so there is nothing to rebuild; kept +/// for library users. +#[deprecated(since = "0.3.3", note = "search reads entities and aliases directly")] +pub fn rebuild_fts(_conn: &Connection) -> Result<(), IdxError> { + Ok(()) +} + #[cfg(test)] mod tests { use rusqlite::{Connection, params}; @@ -124,6 +137,15 @@ mod tests { ); } + #[test] + fn a_name_prefix_ranks_above_a_mid_name_match() { + // Neither is an exact name or alias: only prefix vs substring differs, + // and the substring match is shorter, so length alone would flip them. + let conn = conn_with(&[("BANK MANDIRI TASPEN", &[]), ("PT BANK", &[])]); + + assert_eq!(names(&conn, "bank"), ["BANK MANDIRI TASPEN", "PT BANK"]); + } + #[test] fn treats_like_wildcards_literally_and_empty_queries_as_no_match() { let conn = conn_with(&[("ALPHA_FUND", &[]), ("ALPHAXFUND", &[]), ("BETA 100%", &[])]); diff --git a/scripts/build-ownership-snapshot.sh b/scripts/build-ownership-snapshot.sh index 997b70b..b423c7f 100755 --- a/scripts/build-ownership-snapshot.sh +++ b/scripts/build-ownership-snapshot.sh @@ -79,9 +79,9 @@ fi mkdir -p "$OUTPUT_DIR" -LATEST_AS_OF="$(sqlite3 "$DB_PATH" "SELECT as_of_date FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC LIMIT 1;")" -LATEST_RELEASE_SHA="$(sqlite3 "$DB_PATH" "SELECT sha256 FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC LIMIT 1;")" -LATEST_ROW_COUNT="$(sqlite3 "$DB_PATH" "SELECT row_count FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC LIMIT 1;")" +LATEST_AS_OF="$(sqlite3 "$DB_PATH" "SELECT as_of_date FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1;")" +LATEST_RELEASE_SHA="$(sqlite3 "$DB_PATH" "SELECT sha256 FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1;")" +LATEST_ROW_COUNT="$(sqlite3 "$DB_PATH" "SELECT row_count FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1;")" RELEASE_COUNT="$(sqlite3 "$DB_PATH" "SELECT COUNT(*) FROM ownership_releases;")" TICKER_COUNT="$(sqlite3 "$DB_PATH" "SELECT COUNT(*) FROM tickers;")" From 346eb47cd459ff588c2b6512d52f729b3c481fb9 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:19:39 +0000 Subject: [PATCH 04/27] fix(publisher): order releases by rowid, which every table has id is the rowid alias in the real schema; rowid also works on the minimal fixture DBs the publisher tests build. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- scripts/build-ownership-snapshot.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/build-ownership-snapshot.sh b/scripts/build-ownership-snapshot.sh index b423c7f..b485c62 100755 --- a/scripts/build-ownership-snapshot.sh +++ b/scripts/build-ownership-snapshot.sh @@ -79,9 +79,9 @@ fi mkdir -p "$OUTPUT_DIR" -LATEST_AS_OF="$(sqlite3 "$DB_PATH" "SELECT as_of_date FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1;")" -LATEST_RELEASE_SHA="$(sqlite3 "$DB_PATH" "SELECT sha256 FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1;")" -LATEST_ROW_COUNT="$(sqlite3 "$DB_PATH" "SELECT row_count FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, id DESC LIMIT 1;")" +LATEST_AS_OF="$(sqlite3 "$DB_PATH" "SELECT as_of_date FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, rowid DESC LIMIT 1;")" +LATEST_RELEASE_SHA="$(sqlite3 "$DB_PATH" "SELECT sha256 FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, rowid DESC LIMIT 1;")" +LATEST_ROW_COUNT="$(sqlite3 "$DB_PATH" "SELECT row_count FROM ownership_releases ORDER BY as_of_date DESC, imported_at DESC, rowid DESC LIMIT 1;")" RELEASE_COUNT="$(sqlite3 "$DB_PATH" "SELECT COUNT(*) FROM ownership_releases;")" TICKER_COUNT="$(sqlite3 "$DB_PATH" "SELECT COUNT(*) FROM tickers;")" From 4cf86ce1ff40c8f2729638d1f9b94d1525dacf16 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 12:18:20 +0000 Subject: [PATCH 05/27] fix(ownership): compare each date's own rows, even in a mixed-date file A KSEI file can carry rows from several report dates; its release is dated by the latest. Filtering on the release alone pulled January rows into a February comparison (Pullfrog review on #53). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/ownership/db.rs | 67 +++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/crates/idx-core/src/ownership/db.rs b/crates/idx-core/src/ownership/db.rs index 7668c6b..0691565 100644 --- a/crates/idx-core/src/ownership/db.rs +++ b/crates/idx-core/src/ownership/db.rs @@ -909,11 +909,17 @@ pub fn query_bing_flow( /// The release used for a report date: the latest imported one, as /// `latest_ksei_release` picks, so a corrected file supersedes the original. +/// The latest import holding rows reported on `date`. A file can carry rows +/// from several report dates, so a release is matched by its rows rather +/// than by its `as_of_date` (the latest date in it). fn release_for_date(conn: &Connection, date: &str) -> Result, IdxError> { conn.query_row( - "SELECT sha256 FROM ownership_releases - WHERE as_of_date = ?1 - ORDER BY imported_at DESC, id DESC + "SELECT r.sha256 FROM ownership_releases r + WHERE EXISTS ( + SELECT 1 FROM ksei_holdings k + WHERE k.release_sha256 = r.sha256 AND k.report_date = ?1 + ) + ORDER BY r.imported_at DESC, r.id DESC LIMIT 1", params![date], |row| row.get::<_, String>(0), @@ -927,7 +933,7 @@ fn release_for_date(conn: &Connection, date: &str) -> Result, Idx /// Compare two KSEI releases by date and return ownership changes. /// Finds: new holders, exited holders, percentage increases/decreases. -/// Each date reads one release, and a holder's accounts in a ticker are +/// Each date reads its rows from one release, and a holder's accounts in a ticker are /// summed, so an investor with two accounts compares by its total stake. pub fn query_changes( conn: &Connection, @@ -957,7 +963,7 @@ pub fn query_changes( FROM ksei_holdings k JOIN tickers t ON t.id = k.ticker_id LEFT JOIN entities e ON e.id = k.entity_id - WHERE k.release_sha256 = ?1 + WHERE k.release_sha256 = ?1 AND k.report_date = ?3 GROUP BY t.code, holder_key ), to_snapshot AS ( @@ -972,7 +978,7 @@ pub fn query_changes( FROM ksei_holdings k JOIN tickers t ON t.id = k.ticker_id LEFT JOIN entities e ON e.id = k.entity_id - WHERE k.release_sha256 = ?2 + WHERE k.release_sha256 = ?2 AND k.report_date = ?4 GROUP BY t.code, holder_key ), keys AS ( @@ -999,14 +1005,17 @@ pub fn query_changes( .map_err(|e| IdxError::DatabaseError(e.to_string()))?; let rows = stmt - .query_map(params![from_release, to_release], |row| { - Ok(( - row.get::<_, String>(0)?, - row.get::<_, String>(1)?, - row.get::<_, Option>(2)?, - row.get::<_, Option>(3)?, - )) - }) + .query_map( + params![from_release, to_release, from_date, to_date], + |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, Option>(2)?, + row.get::<_, Option>(3)?, + )) + }, + ) .map_err(|e| IdxError::DatabaseError(e.to_string()))?; let mut out = Vec::new(); @@ -1544,6 +1553,36 @@ mod tests { ); } + #[test] + fn changes_keep_each_date_to_its_own_rows() { + // A file with a January ALPHA row and February BETA row is the + // February release; ALPHA was never held in February, so Feb -> Mar + // must not report it as exited. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + write_release( + &conn, + "mixed", + "2026-02-27", + 1, + bbca, + &[(None, "ALPHA", 1000), (None, "BETA", 200)], + ); + conn.execute( + "UPDATE ksei_holdings SET report_date = '2026-01-30' WHERE raw_investor_name = 'ALPHA'", + [], + ) + .unwrap(); + write_release(&conn, "mar", "2026-03-31", 2, bbca, &[(None, "BETA", 200)]); + + let rows = query_changes(&conn, "2026-02-27", "2026-03-31").unwrap(); + assert!(rows.is_empty(), "{rows:?}"); + + let rows = query_changes(&conn, "2026-01-30", "2026-03-31").unwrap(); + let alpha = rows.iter().find(|r| r.entity_name == "ALPHA").unwrap(); + assert_eq!(alpha.change_type, ChangeType::Exited); + } + #[test] fn same_second_imports_resolve_to_the_later_file_everywhere() { // Original and corrected file for one date imported in the same From 523e1409d3e0f8a97fa8503448c62a3c33e03be1 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 04:18:57 +0000 Subject: [PATCH 06/27] fix(sync): atomic install, stored-row validation, no local artifacts from remote manifests From the full-repo Codex audit: - Install moved the database to a backup, then renamed the snapshot in; a crash between the two left no database, and the next sync deleted the backup first. Replace it with a single rename (old file or new, never neither), and restore a backup an older version left behind. - Validation trusted the release's row_count metadata, so a snapshot with an empty or missing holdings table passed and replaced good data. Also count the rows stored for the latest release. - A manifest fetched over HTTP(S) could name a local download_url such as /dev/zero, read without a size limit. Remote manifests must give an HTTP(S) artifact; local reads take regular files only, within the size limit, and manifests are capped at 1 MiB. - A same-date build with a different row count replaced the local file even when it did not list it in replaces_sqlite_sha256, so a replayed older build could undo a corrected one. Same-date rebuilds now go only through replaces_sqlite_sha256, which the publisher fills. Verified against the published snapshot: fresh install, no-op re-sync, and forced reinstall all succeed with no leftover files. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/ownership/snapshot.rs | 234 +++++++++++++++++----- docs/OWNERSHIP_SYNC.md | 9 +- 2 files changed, 190 insertions(+), 53 deletions(-) diff --git a/crates/idx-core/src/ownership/snapshot.rs b/crates/idx-core/src/ownership/snapshot.rs index 9c4e6fb..fe4f2e1 100644 --- a/crates/idx-core/src/ownership/snapshot.rs +++ b/crates/idx-core/src/ownership/snapshot.rs @@ -189,6 +189,7 @@ pub fn sync_snapshot( // A body larger than the manifest says is rejected while downloading // (ureq fails once a body reaches its limit, hence the +1); the exact size // and SHA-256 are verified afterwards. + check_download_source(manifest_source, &manifest.snapshot.download_url)?; let bytes = read_bytes( &manifest.snapshot.download_url, "ownership snapshot SQLite", @@ -506,17 +507,12 @@ fn build_sync_decision( }; } - if latest.sha256 == snapshot.latest_release_sha256 { - if latest.row_count != snapshot.latest_row_count - && snapshot.release_count >= local.release_count - { - return SyncDecision { - action: OwnershipSyncAction::Updated, - should_download: true, - reason: "local ownership database has the same latest release date but different row_count metadata".to_string(), - }; - } - + // A same-date rebuild replaces the local file only through + // `replaces_sqlite_sha256` above; differing row counts alone would let a + // replayed older build undo a corrected one. + if latest.sha256 == snapshot.latest_release_sha256 + && latest.row_count == snapshot.latest_row_count + { if local.release_count < snapshot.release_count { return SyncDecision { action: OwnershipSyncAction::Updated, @@ -551,6 +547,18 @@ fn build_sync_decision( } } +/// A manifest fetched over HTTP(S) must not name a local file: it could make +/// sync read a device such as /dev/zero, or any local file. A local manifest +/// (the user's own) may. +fn check_download_source(manifest_source: &str, download_url: &str) -> Result<(), IdxError> { + if is_http_source(manifest_source) && !is_http_source(download_url) { + return Err(IdxError::InvalidInput(format!( + "ownership snapshot manifest from {manifest_source} names a non-HTTP(S) download_url: {download_url}" + ))); + } + Ok(()) +} + fn validate_downloaded_bytes( bytes: &[u8], snapshot: &OwnershipSnapshotArtifact, @@ -626,6 +634,21 @@ fn validate_snapshot_db(path: &Path, snapshot: &OwnershipSnapshotArtifact) -> Re snapshot.latest_row_count, latest.row_count ))); } + // Count the stored rows too: release metadata alone would accept a + // snapshot whose holdings table is empty or missing. + let stored_rows: i64 = conn + .query_row( + "SELECT COUNT(*) FROM ksei_holdings WHERE release_sha256 = ?1", + rusqlite::params![latest.sha256], + |row| row.get(0), + ) + .map_err(|e| IdxError::DatabaseError(format!("failed counting snapshot holdings: {e}")))?; + if usize::try_from(stored_rows).ok() != Some(snapshot.latest_row_count) { + return Err(IdxError::ParseError(format!( + "downloaded snapshot holds {stored_rows} rows for its latest release; manifest expected {}", + snapshot.latest_row_count + ))); + } let ticker_count: i64 = conn .query_row("SELECT COUNT(*) FROM tickers", [], |row| row.get(0)) @@ -668,47 +691,34 @@ fn install_snapshot_file(temp_path: &Path, db_path: &Path) -> Result<(), IdxErro })?; } + // Versions before 0.3.3 moved the database to a backup before installing; + // a crash between the two renames left only the backup. Restore it, or + // drop it when the database is there. let backup_path = build_backup_path(db_path); if backup_path.exists() { - fs::remove_file(&backup_path).map_err(|e| { + let result = if db_path.exists() { + fs::remove_file(&backup_path) + } else { + fs::rename(&backup_path, db_path) + }; + result.map_err(|e| { IdxError::Io(format!( - "failed to remove stale backup {}: {e}", + "failed to resolve leftover backup {}: {e}", backup_path.display() )) })?; } - let had_existing = db_path.exists(); + // One rename replaces the old database atomically: a crash leaves either + // the old file or the new one, never neither. Queries do not write to a + // synced file, so its sidecars hold no data and go first. remove_sqlite_sidecars(db_path)?; - - if had_existing { - fs::rename(db_path, &backup_path).map_err(|e| { - IdxError::Io(format!( - "failed to move existing ownership database {} to backup {}: {e}", - db_path.display(), - backup_path.display() - )) - })?; - } - - if let Err(err) = fs::rename(temp_path, db_path) { - if had_existing { - let _ = fs::rename(&backup_path, db_path); - } - return Err(IdxError::Io(format!( - "failed to install ownership snapshot into {}: {err}", + fs::rename(temp_path, db_path).map_err(|e| { + IdxError::Io(format!( + "failed to install ownership snapshot into {}: {e}", db_path.display() - ))); - } - - if had_existing && backup_path.exists() { - fs::remove_file(&backup_path).map_err(|e| { - IdxError::Io(format!( - "failed to remove snapshot backup {}: {e}", - backup_path.display() - )) - })?; - } + )) + })?; remove_sqlite_sidecars(db_path)?; Ok(()) @@ -772,17 +782,24 @@ fn read_text(source: &str, context: &str) -> Result { .header("Accept", "application/json,text/plain;q=0.9,*/*;q=0.8") .call() .map_err(|e| IdxError::Http(format!("failed to fetch {context}: {e}")))?; - let mut body = response.into_body(); - return body + return response + .into_body() + .with_config() + .limit(MANIFEST_LIMIT_BYTES) .read_to_string() .map_err(|e| IdxError::Http(format!("failed reading {context} body: {e}"))); } let path = local_source_path(source); - fs::read_to_string(&path) - .map_err(|e| IdxError::Io(format!("failed to read {context} {}: {e}", path.display()))) + let bytes = read_local_bounded(&path, context, MANIFEST_LIMIT_BYTES)?; + String::from_utf8(bytes).map_err(|e| { + IdxError::ParseError(format!("{context} {} is not UTF-8: {e}", path.display())) + }) } +/// A manifest is a few KB; anything near this is not one. +const MANIFEST_LIMIT_BYTES: u64 = 1 << 20; + /// 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 { @@ -816,9 +833,26 @@ fn read_bytes(source: &str, context: &str, limit_bytes: u64) -> Result, }); } - let path = local_source_path(source); - fs::read(&path) - .map_err(|e| IdxError::Io(format!("failed to read {context} {}: {e}", path.display()))) + read_local_bounded(&local_source_path(source), context, limit_bytes) +} + +/// Reads at most `limit_bytes` of a regular file (not a device or FIFO); a +/// larger file comes back truncated and fails the caller's size check. +fn read_local_bounded(path: &Path, context: &str, limit_bytes: u64) -> Result, IdxError> { + let read_error = |e: std::io::Error| { + IdxError::Io(format!("failed to read {context} {}: {e}", path.display())) + }; + let file = fs::File::open(path).map_err(read_error)?; + if !file.metadata().map_err(read_error)?.is_file() { + return Err(IdxError::InvalidInput(format!( + "{context} {} is not a regular file", + path.display() + ))); + } + let mut bytes = Vec::new(); + std::io::Read::read_to_end(&mut std::io::Read::take(file, limit_bytes), &mut bytes) + .map_err(read_error)?; + Ok(bytes) } fn is_http_source(source: &str) -> bool { @@ -841,7 +875,8 @@ mod tests { use super::{ DEFAULT_SNAPSHOT_MANIFEST_URL, OwnershipSnapshotArtifact, OwnershipSnapshotManifest, OwnershipSnapshotSource, OwnershipSyncAction, SNAPSHOT_MANIFEST_SCHEMA_VERSION, - build_sync_decision, parse_manifest, resolve_manifest_source_with, + build_backup_path, build_sync_decision, check_download_source, install_snapshot_file, + parse_manifest, read_local_bounded, resolve_manifest_source_with, validate_snapshot_db, }; use crate::ownership::db::{ensure_schema, insert_release}; use crate::ownership::types::OwnershipRelease; @@ -984,6 +1019,105 @@ mod tests { assert!(!decision.should_download, "{}", decision.reason); } + #[test] + fn build_sync_decision_ignores_a_replayed_older_build_for_the_same_date() { + // Local holds corrected build B (7200 rows); an older build A for the + // same release (7154 rows) that does not list B must not undo it. + let snapshot = same_date_snapshot(Vec::new()); + let mut local = same_release_local(&snapshot, &"c".repeat(64)); + if let Some(release) = local.latest_release.as_mut() { + release.row_count = 7200; + } + + let decision = build_sync_decision(Some(&local), &snapshot, false); + assert!(!decision.should_download, "{}", decision.reason); + } + + #[test] + fn validation_rejects_a_snapshot_whose_holdings_are_missing() { + // Release metadata and manifest agree (7154 rows), but no holdings + // are stored: installing it would replace good data with nothing. + let snapshot = same_date_snapshot(Vec::new()); + let path = std::env::temp_dir().join(format!("idx-empty-{}.sqlite", fastrand::u64(..))); + let conn = Connection::open(&path).unwrap(); + ensure_schema(&conn).unwrap(); + for (index, sha256) in ["1", "2"].into_iter().map(|c| c.repeat(64)).enumerate() { + let as_of_date = NaiveDate::from_ymd_opt(2026, 7 + index as u32, 28).unwrap(); + insert_release( + &conn, + &OwnershipRelease { + id: 0, + source_url: None, + sha256, + as_of_date, + row_count: 1, + imported_at: 0, + }, + ) + .unwrap(); + } + insert_release( + &conn, + &OwnershipRelease { + id: 0, + source_url: None, + sha256: snapshot.latest_release_sha256.clone(), + as_of_date: snapshot.latest_as_of_date, + row_count: snapshot.latest_row_count, + imported_at: 0, + }, + ) + .unwrap(); + for code in 0..snapshot.ticker_count { + conn.execute( + "INSERT INTO tickers (code) VALUES (?1)", + [format!("T{code}")], + ) + .unwrap(); + } + drop(conn); + + let err = validate_snapshot_db(&path, &snapshot).expect_err("no holdings stored"); + assert!(err.to_string().contains("holds 0 rows"), "{err}"); + std::fs::remove_file(&path).unwrap(); + } + + #[test] + fn remote_manifests_may_not_name_local_artifacts() { + let remote = "https://example.com/ownership-snapshot-manifest.json"; + assert!(check_download_source(remote, "/dev/zero").is_err()); + assert!(check_download_source(remote, "file:///etc/passwd").is_err()); + assert!(check_download_source(remote, "https://example.com/a.sqlite").is_ok()); + // The user's own local manifest may point at a local file. + assert!(check_download_source("/tmp/manifest.json", "/tmp/a.sqlite").is_ok()); + } + + #[cfg(unix)] + #[test] + fn local_reads_refuse_devices() { + let err = read_local_bounded(std::path::Path::new("/dev/zero"), "snapshot", 16) + .expect_err("a device is not a snapshot file"); + assert!(err.to_string().contains("not a regular file"), "{err}"); + } + + #[test] + fn install_restores_a_backup_left_by_an_interrupted_older_install() { + let dir = std::env::temp_dir().join(format!("idx-install-{}", fastrand::u64(..))); + std::fs::create_dir_all(&dir).unwrap(); + let db = dir.join("ownership.db"); + // An interrupted pre-0.3.3 install: only the backup is left. + std::fs::write(build_backup_path(&db), b"previous").unwrap(); + let temp = dir.join("incoming.tmp"); + std::fs::write(&temp, b"snapshot").unwrap(); + + install_snapshot_file(&temp, &db).unwrap(); + + assert_eq!(std::fs::read(&db).unwrap(), b"snapshot"); + assert!(!build_backup_path(&db).exists()); + assert!(!temp.exists()); + std::fs::remove_dir_all(&dir).unwrap(); + } + #[test] fn build_sync_decision_keeps_a_locally_modified_database() { // Same release metadata but a file matching no published build (local diff --git a/docs/OWNERSHIP_SYNC.md b/docs/OWNERSHIP_SYNC.md index 92215e8..8de6b84 100644 --- a/docs/OWNERSHIP_SYNC.md +++ b/docs/OWNERSHIP_SYNC.md @@ -62,7 +62,8 @@ Semantics: (`ownership-snapshot-YYYY-MM-DD-.sqlite`), so consumers must always follow `download_url` and validate it with `sqlite_sha256` and `size_bytes`. - `sqlite_sha256` and `size_bytes` are validated before install. -- `latest_*` and `release_count` are validated against the downloaded SQLite contents before replacement. +- `latest_*` and `release_count` are validated against the downloaded SQLite contents before replacement, including the number of holdings rows actually stored for the latest release. +- A manifest fetched over `http(s)://` must give an `http(s)://` `download_url`; only a local manifest may point at a local file. Local files must be regular files, and manifests are limited to 1 MiB. - `replaces_sqlite_sha256` (optional, omitted when empty) lists earlier builds published for the same `latest_as_of_date` that this snapshot supersedes, for example after a parser fix. Clients up to v0.2.x ignore it. @@ -78,9 +79,11 @@ Without `--force`, sync behaves conservatively: - local DB file is byte-identical to a build in `replaces_sqlite_sha256`: replace (queries never write to a synced file, so this means it was not modified; committed changes still in the `-wal` file are checkpointed before hashing) -- local latest release/date/count matches snapshot: no-op +- local latest release/date/count matches snapshot: no-op (a same-date build + with a different row count replaces the local file only when it lists that + file in `replaces_sqlite_sha256`) - local latest release matches but has fewer releases than snapshot: replace to fill missing history - local DB newer than snapshot: no-op and keep local data - local DB diverges at the same latest date: no-op and require `--force` -Replacement is staged through a temp file and validated before the local DB is swapped in. +Replacement is staged through a temp file and validated before the local DB is swapped in with a single rename, so an interrupted sync leaves either the old database or the new one. From b472d76e277ecde86dcc009834b981990a9a1340 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:10:18 +0000 Subject: [PATCH 07/27] fix(sync): releases record stored rows; oversized manifests are errors From the Codex re-gate of the 0.3.3 fixes: - Imports recorded row_count as the rows parsed, but INSERT OR IGNORE drops a file's repeated ticker/investor rows, so the count could exceed what is stored, and the new stored-row validation would then reject a snapshot the publisher built from such a file. Record the rows stored. - A local manifest over the 1 MiB limit was read truncated rather than rejected; read one byte more and fail when it does not fit. - The backup-recovery test also passed on the old code; it now fails an install after the leftover backup was found and checks the old data is back. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/ownership/db.rs | 40 ++++++++++++++++++++++- crates/idx-core/src/ownership/snapshot.rs | 38 +++++++++++++++++---- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/crates/idx-core/src/ownership/db.rs b/crates/idx-core/src/ownership/db.rs index 0691565..20cab5d 100644 --- a/crates/idx-core/src/ownership/db.rs +++ b/crates/idx-core/src/ownership/db.rs @@ -355,7 +355,13 @@ pub fn write_ksei_release( } let inserted = insert_ksei_holdings_rows(conn, holdings)?; - let _ = insert_release(conn, release)?; + // Record the rows stored, not parsed: INSERT OR IGNORE drops repeated + // ticker/investor rows, and snapshot validation counts what is stored. + let stored = OwnershipRelease { + row_count: inserted, + ..release.clone() + }; + let _ = insert_release(conn, &stored)?; Ok(inserted) })(); @@ -1616,6 +1622,38 @@ mod tests { ); } + #[test] + fn a_release_records_the_rows_it_stored() { + // A file listing the same ticker/investor twice stores one row; the + // release metadata must say 1, or snapshot validation rejects it. + let conn = setup(); + let bbca = upsert_ticker(&conn, "BBCA", None).unwrap(); + write_release( + &conn, + "dup", + "2026-02-27", + 1, + bbca, + &[(None, "ALPHA", 1000), (None, "ALPHA", 1000)], + ); + + let row_count: i64 = conn + .query_row( + "SELECT row_count FROM ownership_releases WHERE sha256 = 'dup'", + [], + |row| row.get(0), + ) + .unwrap(); + let stored: i64 = conn + .query_row( + "SELECT COUNT(*) FROM ksei_holdings WHERE release_sha256 = 'dup'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!((row_count, stored), (1, 1)); + } + #[test] fn changes_reject_a_date_without_a_release() { let conn = setup(); diff --git a/crates/idx-core/src/ownership/snapshot.rs b/crates/idx-core/src/ownership/snapshot.rs index fe4f2e1..abb16fb 100644 --- a/crates/idx-core/src/ownership/snapshot.rs +++ b/crates/idx-core/src/ownership/snapshot.rs @@ -791,7 +791,14 @@ fn read_text(source: &str, context: &str) -> Result { } let path = local_source_path(source); - let bytes = read_local_bounded(&path, context, MANIFEST_LIMIT_BYTES)?; + // One byte over the limit tells an oversized manifest from one that fits. + let bytes = read_local_bounded(&path, context, MANIFEST_LIMIT_BYTES + 1)?; + if bytes.len() as u64 > MANIFEST_LIMIT_BYTES { + return Err(IdxError::InvalidInput(format!( + "{context} {} is larger than {MANIFEST_LIMIT_BYTES} bytes", + path.display() + ))); + } String::from_utf8(bytes).map_err(|e| { IdxError::ParseError(format!("{context} {} is not UTF-8: {e}", path.display())) }) @@ -1105,19 +1112,38 @@ mod tests { let dir = std::env::temp_dir().join(format!("idx-install-{}", fastrand::u64(..))); std::fs::create_dir_all(&dir).unwrap(); let db = dir.join("ownership.db"); - // An interrupted pre-0.3.3 install: only the backup is left. + // An interrupted pre-0.3.3 install left only the backup. Even if this + // install then fails (its temp file is gone), the old data comes back + // instead of being deleted, as the old code did first. std::fs::write(build_backup_path(&db), b"previous").unwrap(); - let temp = dir.join("incoming.tmp"); + let missing_temp = dir.join("incoming.tmp"); + + assert!(install_snapshot_file(&missing_temp, &db).is_err()); + assert_eq!(std::fs::read(&db).unwrap(), b"previous"); + + let temp = dir.join("incoming-2.tmp"); std::fs::write(&temp, b"snapshot").unwrap(); - install_snapshot_file(&temp, &db).unwrap(); - assert_eq!(std::fs::read(&db).unwrap(), b"snapshot"); assert!(!build_backup_path(&db).exists()); - assert!(!temp.exists()); std::fs::remove_dir_all(&dir).unwrap(); } + #[test] + fn local_manifests_over_the_limit_are_rejected_not_truncated() { + let path = std::env::temp_dir().join(format!("idx-manifest-{}.json", fastrand::u64(..))); + std::fs::write( + &path, + vec![b' '; (super::MANIFEST_LIMIT_BYTES + 1) as usize], + ) + .unwrap(); + + let err = super::read_text(path.to_str().unwrap(), "manifest").unwrap_err(); + + assert!(err.to_string().contains("larger than"), "{err}"); + std::fs::remove_file(&path).unwrap(); + } + #[test] fn build_sync_decision_keeps_a_locally_modified_database() { // Same release metadata but a file matching no published build (local From 94aec74b09fb5b794a0b856beb15078328901b9f Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 12:20:20 +0000 Subject: [PATCH 08/27] fix(sync): restore an old install's backup before deciding to replace A pre-0.3.3 install interrupted between its renames left only a backup. Restoring it inside the install step came after the decision, so an older snapshot replaced a newer database without --force, and a failed download left no database (Pullfrog review on #54). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/tests/cli.rs | 12 ++++ crates/idx-core/src/ownership/snapshot.rs | 71 +++++++++-------------- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index f99edd5..76cef35 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -1924,12 +1924,24 @@ fn ownership_sync_installs_snapshot_and_preserves_query_behavior() { } let bytes = fs::read(&target_db).unwrap(); + // A pre-0.3.3 install interrupted between its two renames left only a + // backup. Sync restores it before deciding, so it is kept like any local + // database (no_change, not a reinstall), and survives a failed download. + let backup = sync_root.join(".ownership.db.sync-backup"); + fs::rename(&target_db, &backup).unwrap(); + let result = + run_success_json(bin_with_root(&sync_root).args(["-o", "json", "ownership", "sync"])); + assert_eq!(result["action"], "no_change"); + assert_eq!(fs::read(&target_db).unwrap(), bytes); + assert!(!backup.exists()); + // Both failures happen after a usable local snapshot has been installed. write_snapshot_manifest( &manifest_path, &source_db, Some("cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc"), ); + fs::rename(&target_db, &backup).unwrap(); run_error_json( bin_with_root(&sync_root).args(["-o", "json", "ownership", "sync", "--force"]), "PARSEERROR", diff --git a/crates/idx-core/src/ownership/snapshot.rs b/crates/idx-core/src/ownership/snapshot.rs index abb16fb..91e59a5 100644 --- a/crates/idx-core/src/ownership/snapshot.rs +++ b/crates/idx-core/src/ownership/snapshot.rs @@ -161,6 +161,10 @@ pub fn sync_snapshot( 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 _lock = lock(db_path, true)?; + recover_interrupted_install(db_path)?; + } let local_state = { let _shared = lock(db_path, false)?; inspect_local_db(db_path)? @@ -691,24 +695,6 @@ fn install_snapshot_file(temp_path: &Path, db_path: &Path) -> Result<(), IdxErro })?; } - // Versions before 0.3.3 moved the database to a backup before installing; - // a crash between the two renames left only the backup. Restore it, or - // drop it when the database is there. - let backup_path = build_backup_path(db_path); - if backup_path.exists() { - let result = if db_path.exists() { - fs::remove_file(&backup_path) - } else { - fs::rename(&backup_path, db_path) - }; - result.map_err(|e| { - IdxError::Io(format!( - "failed to resolve leftover backup {}: {e}", - backup_path.display() - )) - })?; - } - // One rename replaces the old database atomically: a crash leaves either // the old file or the new one, never neither. Queries do not write to a // synced file, so its sidecars hold no data and go first. @@ -724,6 +710,29 @@ fn install_snapshot_file(temp_path: &Path, db_path: &Path) -> Result<(), IdxErro Ok(()) } +/// Versions before 0.3.3 moved the database to a backup before installing; +/// a crash between the two renames left only the backup. Restore it, or drop +/// it when the database is there. Runs before the local database is +/// inspected, so the restored one gets the usual newer/modified safeguards +/// and stays in place if the download then fails. +fn recover_interrupted_install(db_path: &Path) -> Result<(), IdxError> { + let backup_path = build_backup_path(db_path); + if !backup_path.exists() { + return Ok(()); + } + let result = if db_path.exists() { + fs::remove_file(&backup_path) + } else { + fs::rename(&backup_path, db_path) + }; + result.map_err(|e| { + IdxError::Io(format!( + "failed to resolve leftover backup {}: {e}", + backup_path.display() + )) + }) +} + fn build_backup_path(db_path: &Path) -> PathBuf { let file_name = db_path .file_name() @@ -882,8 +891,8 @@ mod tests { use super::{ DEFAULT_SNAPSHOT_MANIFEST_URL, OwnershipSnapshotArtifact, OwnershipSnapshotManifest, OwnershipSnapshotSource, OwnershipSyncAction, SNAPSHOT_MANIFEST_SCHEMA_VERSION, - build_backup_path, build_sync_decision, check_download_source, install_snapshot_file, - parse_manifest, read_local_bounded, resolve_manifest_source_with, validate_snapshot_db, + build_sync_decision, check_download_source, parse_manifest, read_local_bounded, + resolve_manifest_source_with, validate_snapshot_db, }; use crate::ownership::db::{ensure_schema, insert_release}; use crate::ownership::types::OwnershipRelease; @@ -1107,28 +1116,6 @@ mod tests { assert!(err.to_string().contains("not a regular file"), "{err}"); } - #[test] - fn install_restores_a_backup_left_by_an_interrupted_older_install() { - let dir = std::env::temp_dir().join(format!("idx-install-{}", fastrand::u64(..))); - std::fs::create_dir_all(&dir).unwrap(); - let db = dir.join("ownership.db"); - // An interrupted pre-0.3.3 install left only the backup. Even if this - // install then fails (its temp file is gone), the old data comes back - // instead of being deleted, as the old code did first. - std::fs::write(build_backup_path(&db), b"previous").unwrap(); - let missing_temp = dir.join("incoming.tmp"); - - assert!(install_snapshot_file(&missing_temp, &db).is_err()); - assert_eq!(std::fs::read(&db).unwrap(), b"previous"); - - let temp = dir.join("incoming-2.tmp"); - std::fs::write(&temp, b"snapshot").unwrap(); - install_snapshot_file(&temp, &db).unwrap(); - assert_eq!(std::fs::read(&db).unwrap(), b"snapshot"); - assert!(!build_backup_path(&db).exists()); - std::fs::remove_dir_all(&dir).unwrap(); - } - #[test] fn local_manifests_over_the_limit_are_rejected_not_truncated() { let path = std::env::temp_dir().join(format!("idx-manifest-{}.json", fastrand::u64(..))); From 66207d24b652f784a291ba4039d42c7b8e4a8cc1 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 04:28:30 +0000 Subject: [PATCH 09/27] fix(net): bounded request time, curl size and time caps, lock waits, sane prices From the full-repo Codex audit: - MSN and Yahoo clients set connect and body timeouts but none for the response headers, so a server that stalled before answering hung the call; four such calls take every MCP call slot. Add a 30 s overall deadline. Every curl-impersonate run gets --max-time 600 and a 256 MiB --max-filesize, and output above that is rejected (curl cannot cap a response without a length); the Yahoo cookie fetch gets --max-time. - Ownership commands blocked on the database lock without limit, so a long sync or import, or a stuck process, held MCP call slots indefinitely. Wait up to 30 s, then fail with a "busy, try again" error. - A price of 1e20 saturated `as i64` and the change subtraction then overflowed. Prices that are not finite, negative, or above 1e12 IDR are treated as missing, and a quote's own price as an error. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/api/msn/client.rs | 3 ++ crates/idx-core/src/api/msn/map.rs | 55 +++++++++++++++------- crates/idx-core/src/api/yahoo/client.rs | 12 ++++- crates/idx-core/src/api/yahoo/map.rs | 28 ++++++++---- crates/idx-core/src/curl_impersonate.rs | 19 +++++++- crates/idx-core/src/investor.rs | 61 ++++++++++++++++++++++--- 6 files changed, 144 insertions(+), 34 deletions(-) diff --git a/crates/idx-core/src/api/msn/client.rs b/crates/idx-core/src/api/msn/client.rs index f61ef71..5c1fbaf 100644 --- a/crates/idx-core/src/api/msn/client.rs +++ b/crates/idx-core/src/api/msn/client.rs @@ -30,6 +30,9 @@ impl MsnClient { let agent: ureq::Agent = ureq::Agent::config_builder() .timeout_connect(Some(Duration::from_secs(5))) .timeout_recv_body(Some(Duration::from_secs(10))) + // A server that stalls before sending headers must not hold a + // call (and an MCP call slot) forever. + .timeout_global(Some(Duration::from_secs(30))) .build() .into(); diff --git a/crates/idx-core/src/api/msn/map.rs b/crates/idx-core/src/api/msn/map.rs index b47f1a9..d2a51f9 100644 --- a/crates/idx-core/src/api/msn/map.rs +++ b/crates/idx-core/src/api/msn/map.rs @@ -25,11 +25,15 @@ pub(super) fn parse_quote(symbol: &str, quotes: &[MsnQuote]) -> Result Result) -> Option { value.filter(|number| number.is_finite() && *number >= 0.01) } -fn round_price(value: f64) -> i64 { - value.round() as i64 +/// Prices are IDR; anything non-finite, negative, or above this is not a +/// real quote, and treating it as missing keeps arithmetic from overflowing. +const MAX_PRICE: f64 = 1e12; + +fn round_price(value: f64) -> Option { + (value.is_finite() && (0.0..=MAX_PRICE).contains(&value)).then(|| value.round() as i64) } fn round_u64(value: Option) -> Option { @@ -667,11 +675,11 @@ pub(super) fn parse_screener_results(raw: &RawScreenerResponse) -> Result 0.0)?; - let price = round_price(raw_price); - let prev_close = q.price_previous_close.map(round_price); + let price = round_price(raw_price)?; + let prev_close = q.price_previous_close.and_then(round_price); let change = prev_close .map(|pc| price - pc) - .or_else(|| q.price_change.map(round_price)); + .or_else(|| q.price_change.and_then(round_price)); let ticker = q .symbol .as_deref() @@ -698,8 +706,8 @@ pub(super) fn parse_screener_results(raw: &RawScreenerResponse) -> Result], idx: usize) -> Option { .copied() .flatten() .filter(|value| value.is_finite()) - .map(round_price) + .and_then(round_price) } fn series_volume_at(series: &RawChartSeries, idx: usize) -> u64 { @@ -889,6 +896,22 @@ mod tests { assert_eq!(sentiment.statistics[0].neutral, 3); } + #[test] + fn parse_quote_rejects_implausible_prices_instead_of_overflowing() { + let raw: Vec = + serde_json::from_str(r#"[{"symbol":"BBCA","price":1e20,"pricePreviousClose":-1e20}]"#) + .unwrap(); + assert!(parse_quote("BBCA.JK", &raw).is_err()); + + let raw: Vec = serde_json::from_str( + r#"[{"symbol":"BBCA","price":6075.0,"pricePreviousClose":-1e20}]"#, + ) + .unwrap(); + let quote = parse_quote("BBCA.JK", &raw).unwrap(); + assert_eq!(quote.price, 6075); + assert_eq!(quote.prev_close, None); + } + #[test] fn parses_news_fixture_items() { let raw: RawNewsFeed = diff --git a/crates/idx-core/src/api/yahoo/client.rs b/crates/idx-core/src/api/yahoo/client.rs index 6dc3154..2029a84 100644 --- a/crates/idx-core/src/api/yahoo/client.rs +++ b/crates/idx-core/src/api/yahoo/client.rs @@ -30,6 +30,9 @@ impl YahooClient { let agent: ureq::Agent = ureq::Agent::config_builder() .timeout_connect(Some(Duration::from_secs(5))) .timeout_recv_body(Some(Duration::from_secs(10))) + // A server that stalls before sending headers must not hold a + // call (and an MCP call slot) forever. + .timeout_global(Some(Duration::from_secs(30))) .build() .into(); @@ -141,7 +144,14 @@ impl YahooClient { // Step 1: fetch fc.yahoo.com to set A3 cookie (returns 404 but writes cookie jar). // We allow non-zero exit here since 404 still writes the cookie. let _ = Command::new(binary) - .args(["--silent", "--cookie-jar", cookie_jar_str, COOKIE_FETCH_URL]) + .args([ + "--silent", + "--max-time", + "30", + "--cookie-jar", + cookie_jar_str, + COOKIE_FETCH_URL, + ]) .stdout(Stdio::null()) .stderr(Stdio::null()) .output(); diff --git a/crates/idx-core/src/api/yahoo/map.rs b/crates/idx-core/src/api/yahoo/map.rs index 20d8212..75c415f 100644 --- a/crates/idx-core/src/api/yahoo/map.rs +++ b/crates/idx-core/src/api/yahoo/map.rs @@ -20,8 +20,12 @@ pub(super) fn parse_quote(symbol: &str, chart: &ChartResponse) -> Result Result i64 { - value.round() as i64 +/// Prices are IDR; anything non-finite, negative, or above this is not a +/// real quote, and treating it as missing keeps arithmetic from overflowing. +const MAX_PRICE: f64 = 1e12; + +fn round_price(value: f64) -> Option { + (value.is_finite() && (0.0..=MAX_PRICE).contains(&value)).then(|| value.round() as i64) } pub(super) fn map_yahoo_error(symbol: &str, endpoint: &str, err: &ChartError) -> IdxError { diff --git a/crates/idx-core/src/curl_impersonate.rs b/crates/idx-core/src/curl_impersonate.rs index dbabb8c..0b1d69b 100644 --- a/crates/idx-core/src/curl_impersonate.rs +++ b/crates/idx-core/src/curl_impersonate.rs @@ -97,8 +97,19 @@ pub fn run_owned_with_fallback( Err(last_error.expect("at least one profile was tried")) } +/// Every curl run gives up after this long, so a stalled server cannot hang +/// a command or an MCP call. +const MAX_TIME_SECS: &str = "600"; +/// Largest response kept in memory (IDX report files are a few MB). +const MAX_RESPONSE_BYTES: usize = 256 * 1024 * 1024; + fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result { - let output = Command::new(binary).args(args).output().map_err(|e| { + let max_filesize = MAX_RESPONSE_BYTES.to_string(); + let output = Command::new(binary) + .args(["--max-time", MAX_TIME_SECS, "--max-filesize", max_filesize.as_str()]) + .args(args) + .output() + .map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { return IdxError::Http(format!( "curl-impersonate binary '{binary}' not found; set {OVERRIDE_ENV} or install nixpkgs#curl-impersonate-chrome" @@ -107,6 +118,12 @@ fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result MAX_RESPONSE_BYTES { + return Err(IdxError::Http(format!( + "{stage} response exceeds {MAX_RESPONSE_BYTES} bytes" + ))); + } if output.status.success() { return Ok(output); } diff --git a/crates/idx-core/src/investor.rs b/crates/idx-core/src/investor.rs index 001fdaa..b5012a2 100644 --- a/crates/idx-core/src/investor.rs +++ b/crates/idx-core/src/investor.rs @@ -484,6 +484,14 @@ fn lock_path(db_path: &Path) -> PathBuf { /// 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 { + lock_at_within(db_path, exclusive, LOCK_WAIT) +} + +fn lock_at_within( + db_path: &Path, + exclusive: bool, + wait: std::time::Duration, +) -> Result { let path = lock_path(db_path); let opened = fs::OpenOptions::new() .create(true) @@ -496,15 +504,38 @@ fn lock_at(db_path: &Path, exclusive: bool) -> Result { .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() + // Wait a bounded time: a blocking lock behind a long sync or import (or + // a stuck process) would hold every MCP call slot indefinitely. + let deadline = std::time::Instant::now() + wait; + loop { + let attempt = if exclusive { + file.try_lock() + } else { + file.try_lock_shared() + }; + match attempt { + Ok(()) => return Ok(file), + Err(fs::TryLockError::WouldBlock) if std::time::Instant::now() < deadline => { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + Err(fs::TryLockError::WouldBlock) => { + return Err(IdxError::Io(format!( + "ownership database is busy (another idx is syncing or importing); try again in a moment ({})", + path.display() + ))); + } + Err(fs::TryLockError::Error(e)) => { + return Err(IdxError::Io(format!( + "failed to lock ownership database: {e}" + ))); + } + } } - .map_err(|e| IdxError::Io(format!("failed to lock ownership database: {e}")))?; - Ok(file) } +/// How long ownership commands wait for another process's lock. +const LOCK_WAIT: std::time::Duration = std::time::Duration::from_secs(30); + /// True when the lock file is absent and cannot be created. fn unlockable(db_path: &Path) -> bool { let path = lock_path(db_path); @@ -780,3 +811,21 @@ mod tests { assert!(parse_report_family("above10").is_err()); } } + +#[cfg(test)] +mod lock_tests { + use std::time::Duration; + + use super::lock_at_within; + + #[test] + fn a_held_lock_times_out_with_a_busy_error() { + let db = std::env::temp_dir().join(format!("idx-lock-{}.db", fastrand::u64(..))); + let _held = lock_at_within(&db, true, Duration::ZERO).unwrap(); + + let err = lock_at_within(&db, false, Duration::from_millis(200)).unwrap_err(); + + assert!(err.to_string().contains("busy"), "{err}"); + let _ = std::fs::remove_file(super::lock_path(&db)); + } +} From 46cf69db6e778282c63d1644029959fe1d74216f Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:12:11 +0000 Subject: [PATCH 10/27] fix(quotes): keep negative price changes From the Codex re-gate of the 0.3.3 fixes: the price sanity check also applied to MSN's priceChange, so a falling stock without a previous close got change: null. Bound a change by its size instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/api/msn/map.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/crates/idx-core/src/api/msn/map.rs b/crates/idx-core/src/api/msn/map.rs index d2a51f9..b0f445f 100644 --- a/crates/idx-core/src/api/msn/map.rs +++ b/crates/idx-core/src/api/msn/map.rs @@ -33,7 +33,7 @@ pub(super) fn parse_quote(symbol: &str, quotes: &[MsnQuote]) -> Result Option { (value.is_finite() && (0.0..=MAX_PRICE).contains(&value)).then(|| value.round() as i64) } +/// A price change can be negative; same bound on its size. +fn round_change(value: f64) -> Option { + (value.is_finite() && value.abs() <= MAX_PRICE).then(|| value.round() as i64) +} + fn round_u64(value: Option) -> Option { value.and_then(|number| { if !number.is_finite() || number.is_sign_negative() { @@ -679,7 +684,7 @@ pub(super) fn parse_screener_results(raw: &RawScreenerResponse) -> Result = + serde_json::from_str(r#"[{"symbol":"BBCA","price":100.0,"priceChange":-5.0}]"#) + .unwrap(); + assert_eq!(parse_quote("BBCA.JK", &raw).unwrap().change, Some(-5)); + } + #[test] fn parses_news_fixture_items() { let raw: RawNewsFeed = From b2b511e9c2021476994798d74e4de06b2e71e8e1 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 12:21:37 +0000 Subject: [PATCH 11/27] fix(net): cap curl output while reading it, not after Command::output() buffered the whole response before the size check, and curl before 8.20 does not apply --max-filesize to decoded bodies, so a small compressed response could exhaust memory. Read stdout up to the limit, then kill and reap curl (Pullfrog review on #55). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/curl_impersonate.rs | 85 ++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/crates/idx-core/src/curl_impersonate.rs b/crates/idx-core/src/curl_impersonate.rs index 0b1d69b..c8cdaa2 100644 --- a/crates/idx-core/src/curl_impersonate.rs +++ b/crates/idx-core/src/curl_impersonate.rs @@ -1,4 +1,5 @@ -use std::process::{Command, Output}; +use std::io::Read; +use std::process::{Command, Output, Stdio}; use crate::error::IdxError; @@ -104,11 +105,26 @@ const MAX_TIME_SECS: &str = "600"; const MAX_RESPONSE_BYTES: usize = 256 * 1024 * 1024; fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result { - let max_filesize = MAX_RESPONSE_BYTES.to_string(); - let output = Command::new(binary) + run_binary_limited(stage, binary, args, MAX_RESPONSE_BYTES) +} + +/// `--max-filesize` cannot stop a response without a length, nor (before +/// curl 8.20) a small compressed one that decodes to more, so stdout is +/// also capped while it is read: past `limit`, curl is killed and reaped. +fn run_binary_limited( + stage: &str, + binary: &str, + args: &[String], + limit: usize, +) -> Result { + let max_filesize = limit.to_string(); + let mut child = Command::new(binary) .args(["--max-time", MAX_TIME_SECS, "--max-filesize", max_filesize.as_str()]) .args(args) - .output() + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() .map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { return IdxError::Http(format!( @@ -118,12 +134,41 @@ fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result MAX_RESPONSE_BYTES { - return Err(IdxError::Http(format!( - "{stage} response exceeds {MAX_RESPONSE_BYTES} bytes" - ))); + // stderr drains on its own thread so a chatty curl cannot block on it. + let mut stderr_pipe = child.stderr.take().expect("stderr is piped"); + let stderr_reader = std::thread::spawn(move || { + let mut stderr = Vec::new(); + let _ = stderr_pipe.read_to_end(&mut stderr); + stderr + }); + let mut stdout = Vec::new(); + let read = child + .stdout + .take() + .expect("stdout is piped") + .take(limit as u64 + 1) + .read_to_end(&mut stdout); + if read.is_err() || stdout.len() > limit { + let _ = child.kill(); + let _ = child.wait(); + let _ = stderr_reader.join(); + return Err(match read { + Err(e) => IdxError::Http(format!( + "failed to read {stage} response from {binary}: {e}" + )), + Ok(_) => IdxError::Http(format!("{stage} response exceeds {limit} bytes")), + }); } + let status = child + .wait() + .map_err(|e| IdxError::Http(format!("failed to wait for {binary} for {stage}: {e}")))?; + let stderr = stderr_reader.join().unwrap_or_default(); + let output = Output { + status, + stdout, + stderr, + }; + if output.status.success() { return Ok(output); } @@ -140,3 +185,25 @@ fn run_binary(stage: &str, binary: &str, args: &[String]) -> Result Date: Sat, 17 Oct 2026 12:46:01 +0000 Subject: [PATCH 12/27] fix(net): return the size error without waiting on a wrapped curl The curl_* profiles run curl without exec, so killing the wrapper left curl holding stderr, and joining stderr waited up to --max-time. Close stdout and return; curl ends on its next write (Pullfrog review on #55). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/curl_impersonate.rs | 26 +++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/idx-core/src/curl_impersonate.rs b/crates/idx-core/src/curl_impersonate.rs index c8cdaa2..a157b09 100644 --- a/crates/idx-core/src/curl_impersonate.rs +++ b/crates/idx-core/src/curl_impersonate.rs @@ -142,16 +142,19 @@ fn run_binary_limited( stderr }); let mut stdout = Vec::new(); - let read = child - .stdout - .take() - .expect("stdout is piped") + let mut stdout_pipe = child.stdout.take().expect("stdout is piped"); + let read = (&mut stdout_pipe) .take(limit as u64 + 1) .read_to_end(&mut stdout); if read.is_err() || stdout.len() > limit { + // The curl_* profiles are scripts that run curl without exec, so the + // kill reaches only the wrapper; curl itself still holds stderr. Do + // not wait for it: closing stdout ends it on its next write (or + // --max-time ends a stalled one), and the stderr thread exits with it. + drop(stdout_pipe); let _ = child.kill(); let _ = child.wait(); - let _ = stderr_reader.join(); + drop(stderr_reader); return Err(match read { Err(e) => IdxError::Http(format!( "failed to read {stage} response from {binary}: {e}" @@ -191,19 +194,26 @@ mod tests { use super::*; #[test] - fn output_past_the_limit_is_cut_off_and_curl_is_reaped() { - // A stand-in curl that never stops writing, like a decompression bomb. + fn output_past_the_limit_fails_at_once_behind_a_profile_wrapper() { + // Like the curl_* profile scripts, the wrapper runs "curl" without + // exec; this one writes too much, then stalls holding stderr open. let dir = std::env::temp_dir().join(format!("idx-curl-{}", fastrand::u64(..))); std::fs::create_dir_all(&dir).unwrap(); let fake = dir.join("curl"); - std::fs::write(&fake, "#!/bin/sh\nexec yes\n").unwrap(); + std::fs::write( + &fake, + "#!/bin/sh\n( head -c 8192 /dev/zero; sleep 30 )\ntrue\n", + ) + .unwrap(); let mut perms = std::fs::metadata(&fake).unwrap().permissions(); std::os::unix::fs::PermissionsExt::set_mode(&mut perms, 0o755); std::fs::set_permissions(&fake, perms).unwrap(); + let started = std::time::Instant::now(); let err = run_binary_limited("test", fake.to_str().unwrap(), &[], 4096).unwrap_err(); assert!(err.to_string().contains("exceeds 4096 bytes"), "{err}"); + assert!(started.elapsed() < std::time::Duration::from_secs(10)); std::fs::remove_dir_all(&dir).unwrap(); } } From a360d0b429be8f3dc21fb5b330d9614bf48e23de Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 13:04:39 +0000 Subject: [PATCH 13/27] fix(net): kill curl itself, not only its profile script, on overflow Run curl in its own process group and kill the group, so the curl behind a curl_* wrapper script ends at once instead of at its next write or --max-time (Pullfrog review on #55). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- Cargo.lock | 1 + crates/idx-core/Cargo.toml | 3 ++ crates/idx-core/src/curl_impersonate.rs | 60 ++++++++++++++++++++----- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8de2fe0..07b53a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -908,6 +908,7 @@ dependencies = [ "clap", "directories", "fastrand", + "libc", "quick-xml", "rusqlite", "serde", diff --git a/crates/idx-core/Cargo.toml b/crates/idx-core/Cargo.toml index 1ac22af..1965fd8 100644 --- a/crates/idx-core/Cargo.toml +++ b/crates/idx-core/Cargo.toml @@ -34,3 +34,6 @@ rusqlite = { workspace = true, optional = true } quick-xml = { version = "0.37", optional = true } sha2 = { workspace = true, optional = true } zip = { workspace = true, optional = true } + +[target.'cfg(unix)'.dependencies] +libc = "0.2" diff --git a/crates/idx-core/src/curl_impersonate.rs b/crates/idx-core/src/curl_impersonate.rs index a157b09..ac7f512 100644 --- a/crates/idx-core/src/curl_impersonate.rs +++ b/crates/idx-core/src/curl_impersonate.rs @@ -118,12 +118,25 @@ fn run_binary_limited( limit: usize, ) -> Result { let max_filesize = limit.to_string(); - let mut child = Command::new(binary) - .args(["--max-time", MAX_TIME_SECS, "--max-filesize", max_filesize.as_str()]) + let mut command = Command::new(binary); + command + .args([ + "--max-time", + MAX_TIME_SECS, + "--max-filesize", + max_filesize.as_str(), + ]) .args(args) .stdin(Stdio::null()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) + .stderr(Stdio::piped()); + // The curl_* profiles are scripts that run curl without exec; a process + // group of its own lets an overflow kill curl too, not just the script. + // Ctrl+C then reaches only idx: curl ends on its next write to the closed + // pipe, or at --max-time. + #[cfg(unix)] + std::os::unix::process::CommandExt::process_group(&mut command, 0); + let mut child = command .spawn() .map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { @@ -147,14 +160,10 @@ fn run_binary_limited( .take(limit as u64 + 1) .read_to_end(&mut stdout); if read.is_err() || stdout.len() > limit { - // The curl_* profiles are scripts that run curl without exec, so the - // kill reaches only the wrapper; curl itself still holds stderr. Do - // not wait for it: closing stdout ends it on its next write (or - // --max-time ends a stalled one), and the stderr thread exits with it. drop(stdout_pipe); - let _ = child.kill(); + kill_process_group(&mut child); let _ = child.wait(); - drop(stderr_reader); + let _ = stderr_reader.join(); return Err(match read { Err(e) => IdxError::Http(format!( "failed to read {stage} response from {binary}: {e}" @@ -189,6 +198,18 @@ fn run_binary_limited( ))) } +/// Kills curl and the profile script around it (their group, on Unix). +fn kill_process_group(child: &mut std::process::Child) { + #[cfg(unix)] + if let Ok(pgid) = libc::pid_t::try_from(child.id()) { + // SAFETY: kill(2) takes plain integers; the group is the one this + // child leads (process_group(0) at spawn), not yet reaped. + unsafe { libc::kill(-pgid, libc::SIGKILL) }; + return; + } + let _ = child.kill(); +} + #[cfg(all(test, unix))] mod tests { use super::*; @@ -200,9 +221,13 @@ mod tests { let dir = std::env::temp_dir().join(format!("idx-curl-{}", fastrand::u64(..))); std::fs::create_dir_all(&dir).unwrap(); let fake = dir.join("curl"); + let pid_file = dir.join("curl.pid"); std::fs::write( &fake, - "#!/bin/sh\n( head -c 8192 /dev/zero; sleep 30 )\ntrue\n", + format!( + "#!/bin/sh\nsh -c 'echo $$ > {}; head -c 8192 /dev/zero; exec sleep 30'\ntrue\n", + pid_file.display() + ), ) .unwrap(); let mut perms = std::fs::metadata(&fake).unwrap().permissions(); @@ -214,6 +239,21 @@ mod tests { assert!(err.to_string().contains("exceeds 4096 bytes"), "{err}"); assert!(started.elapsed() < std::time::Duration::from_secs(10)); + // The stand-in curl behind the wrapper is gone too, not left to its + // timeout (allow a moment for init to reap it). + let pid: libc::pid_t = std::fs::read_to_string(&pid_file) + .unwrap() + .trim() + .parse() + .unwrap(); + let alive = || unsafe { libc::kill(pid, 0) } == 0; + for _ in 0..50 { + if !alive() { + break; + } + std::thread::sleep(std::time::Duration::from_millis(100)); + } + assert!(!alive(), "curl {pid} outlived the overflow"); std::fs::remove_dir_all(&dir).unwrap(); } } From 57169c75f2ebd095defd7392f42b3e916fd2ad2f Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 13:24:20 +0000 Subject: [PATCH 14/27] revert: keep curl in idx's process group so Ctrl+C reaches it Reverts a360d0b. A separate process group let an overflow kill the curl behind a profile script, but Ctrl+C then reached only idx and left a stalled transfer running (Pullfrog review on #55). Cancellation is the common path; on overflow, curl still ends on its next write to the closed pipe, and the error returns at once either way. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- Cargo.lock | 1 - crates/idx-core/Cargo.toml | 3 -- crates/idx-core/src/curl_impersonate.rs | 60 +++++-------------------- 3 files changed, 10 insertions(+), 54 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 07b53a3..8de2fe0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -908,7 +908,6 @@ dependencies = [ "clap", "directories", "fastrand", - "libc", "quick-xml", "rusqlite", "serde", diff --git a/crates/idx-core/Cargo.toml b/crates/idx-core/Cargo.toml index 1965fd8..1ac22af 100644 --- a/crates/idx-core/Cargo.toml +++ b/crates/idx-core/Cargo.toml @@ -34,6 +34,3 @@ rusqlite = { workspace = true, optional = true } quick-xml = { version = "0.37", optional = true } sha2 = { workspace = true, optional = true } zip = { workspace = true, optional = true } - -[target.'cfg(unix)'.dependencies] -libc = "0.2" diff --git a/crates/idx-core/src/curl_impersonate.rs b/crates/idx-core/src/curl_impersonate.rs index ac7f512..a157b09 100644 --- a/crates/idx-core/src/curl_impersonate.rs +++ b/crates/idx-core/src/curl_impersonate.rs @@ -118,25 +118,12 @@ fn run_binary_limited( limit: usize, ) -> Result { let max_filesize = limit.to_string(); - let mut command = Command::new(binary); - command - .args([ - "--max-time", - MAX_TIME_SECS, - "--max-filesize", - max_filesize.as_str(), - ]) + let mut child = Command::new(binary) + .args(["--max-time", MAX_TIME_SECS, "--max-filesize", max_filesize.as_str()]) .args(args) .stdin(Stdio::null()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()); - // The curl_* profiles are scripts that run curl without exec; a process - // group of its own lets an overflow kill curl too, not just the script. - // Ctrl+C then reaches only idx: curl ends on its next write to the closed - // pipe, or at --max-time. - #[cfg(unix)] - std::os::unix::process::CommandExt::process_group(&mut command, 0); - let mut child = command + .stderr(Stdio::piped()) .spawn() .map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { @@ -160,10 +147,14 @@ fn run_binary_limited( .take(limit as u64 + 1) .read_to_end(&mut stdout); if read.is_err() || stdout.len() > limit { + // The curl_* profiles are scripts that run curl without exec, so the + // kill reaches only the wrapper; curl itself still holds stderr. Do + // not wait for it: closing stdout ends it on its next write (or + // --max-time ends a stalled one), and the stderr thread exits with it. drop(stdout_pipe); - kill_process_group(&mut child); + let _ = child.kill(); let _ = child.wait(); - let _ = stderr_reader.join(); + drop(stderr_reader); return Err(match read { Err(e) => IdxError::Http(format!( "failed to read {stage} response from {binary}: {e}" @@ -198,18 +189,6 @@ fn run_binary_limited( ))) } -/// Kills curl and the profile script around it (their group, on Unix). -fn kill_process_group(child: &mut std::process::Child) { - #[cfg(unix)] - if let Ok(pgid) = libc::pid_t::try_from(child.id()) { - // SAFETY: kill(2) takes plain integers; the group is the one this - // child leads (process_group(0) at spawn), not yet reaped. - unsafe { libc::kill(-pgid, libc::SIGKILL) }; - return; - } - let _ = child.kill(); -} - #[cfg(all(test, unix))] mod tests { use super::*; @@ -221,13 +200,9 @@ mod tests { let dir = std::env::temp_dir().join(format!("idx-curl-{}", fastrand::u64(..))); std::fs::create_dir_all(&dir).unwrap(); let fake = dir.join("curl"); - let pid_file = dir.join("curl.pid"); std::fs::write( &fake, - format!( - "#!/bin/sh\nsh -c 'echo $$ > {}; head -c 8192 /dev/zero; exec sleep 30'\ntrue\n", - pid_file.display() - ), + "#!/bin/sh\n( head -c 8192 /dev/zero; sleep 30 )\ntrue\n", ) .unwrap(); let mut perms = std::fs::metadata(&fake).unwrap().permissions(); @@ -239,21 +214,6 @@ mod tests { assert!(err.to_string().contains("exceeds 4096 bytes"), "{err}"); assert!(started.elapsed() < std::time::Duration::from_secs(10)); - // The stand-in curl behind the wrapper is gone too, not left to its - // timeout (allow a moment for init to reap it). - let pid: libc::pid_t = std::fs::read_to_string(&pid_file) - .unwrap() - .trim() - .parse() - .unwrap(); - let alive = || unsafe { libc::kill(pid, 0) } == 0; - for _ in 0..50 { - if !alive() { - break; - } - std::thread::sleep(std::time::Duration::from_millis(100)); - } - assert!(!alive(), "curl {pid} outlived the overflow"); std::fs::remove_dir_all(&dir).unwrap(); } } From ec2e870c6ab16241aa32f17fdff777181197fb7b Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 04:43:09 +0000 Subject: [PATCH 15/27] fix(cli): output contract, terminal-safe text, char-safe truncation, cache symlinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the full-repo Codex audit: - Provider and KSEI text reached the terminal raw, so an escape sequence in a news title or insight could clear or rewrite the screen. Every table cell, insight line, and graph label now has control characters and bidirectional overrides replaced. - Table rendering sliced strings by byte (a description at 200, a URL at 72, an earnings period), panicking when a multi-byte character such as "’" or "é" fell on the cut. Truncate by character. - In JSON mode, argument errors were clap's plain text and warnings were plain lines ahead of the JSON error. Argument errors are now the usual JSON error (INVALIDINPUT, exit 2) when JSON output is selected, and diagnostics are JSON lines; --help and --version are unchanged. - `config` commands ignored the output format; they follow it like every other command (JSON when piped or with -o json, text on a terminal). - Ownership argument mistakes (--limit 0, --format, missing or conflicting import sources, --family, an empty graph root) returned PARSEERROR / exit 1; they are INVALIDINPUT / exit 2 as documented. - `cache clear` followed symlinked directories and deleted files outside the cache; it now removes links without following them. Output change, intentional: goldens regenerated for the JSON argument error and the five config commands, which run with stdout piped. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/cli/config.rs | 30 +- crates/idx-cli/src/cli/ownership.rs | 126 ++++----- crates/idx-cli/src/main.rs | 59 +++- crates/idx-cli/src/output/mod.rs | 1 + crates/idx-cli/src/output/table.rs | 257 +++++++++--------- crates/idx-cli/src/output/text.rs | 70 +++++ crates/idx-cli/tests/golden/config-get.golden | 2 +- .../idx-cli/tests/golden/config-init.golden | 2 +- .../idx-cli/tests/golden/config-path.golden | 2 +- .../golden/config-set-output-jsonl.golden | 2 +- crates/idx-cli/tests/golden/config-set.golden | 2 +- .../error-usage-missing-symbol.json.golden | 7 +- crates/idx-core/src/cache.rs | 41 ++- crates/idx-core/src/investor.rs | 2 +- crates/idx-core/src/lib.rs | 27 +- crates/idx-core/src/ownership/graph.rs | 2 +- docs/ARCHITECTURE.md | 9 +- 17 files changed, 410 insertions(+), 231 deletions(-) create mode 100644 crates/idx-cli/src/output/text.rs diff --git a/crates/idx-cli/src/cli/config.rs b/crates/idx-cli/src/cli/config.rs index cd77da2..e4d7dac 100644 --- a/crates/idx-cli/src/cli/config.rs +++ b/crates/idx-cli/src/cli/config.rs @@ -1,6 +1,7 @@ use clap::{Args, Subcommand}; use crate::config::{config_path, ensure_default_config, get_config_value, set_config_value}; +use crate::output::{OutputFormat, json}; use idx_core::error::IdxError; #[derive(Debug, Args)] @@ -22,24 +23,39 @@ pub enum ConfigSubcommand { Path, } -pub fn handle(cmd: &ConfigCmd) -> Result<(), IdxError> { +pub fn handle(cmd: &ConfigCmd, format: &OutputFormat) -> Result<(), IdxError> { + // Each result as plain text, or one JSON document with -o json. + let print = |text: String, document: serde_json::Value| { + if format.is_json() { + json::print_json(&document) + } else { + println!("{text}"); + Ok(()) + } + }; match &cmd.command { ConfigSubcommand::Init => { - let path = ensure_default_config()?; - println!("{}", path.display()); + let path = ensure_default_config()?.display().to_string(); + print(path.clone(), serde_json::json!({ "path": path })) } ConfigSubcommand::Get { key } => { let value = get_config_value(key)? .ok_or_else(|| IdxError::ConfigError(format!("key not found: {key}")))?; - println!("{value}"); + print( + value.to_string(), + serde_json::json!({ "key": key, "value": value.to_string() }), + ) } ConfigSubcommand::Set { key, value } => { set_config_value(key, value)?; - println!("ok"); + print( + "ok".to_string(), + serde_json::json!({ "key": key, "value": value, "ok": true }), + ) } ConfigSubcommand::Path => { - println!("{}", config_path()?.display()); + let path = config_path()?.display().to_string(); + print(path.clone(), serde_json::json!({ "path": path })) } } - Ok(()) } diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index d9b4261..0d19885 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -1,13 +1,14 @@ use std::path::PathBuf; use clap::{Args, Subcommand}; -use comfy_table::{Cell, ContentArrangement, Table, presets::UTF8_FULL}; +use comfy_table::{ContentArrangement, Table, presets::UTF8_FULL}; use serde::Serialize; use crate::config::IdxConfig; use crate::output::json; use crate::output::style::Paint; use crate::output::table::format_idr; +use crate::output::text::{cell, terminal_safe}; use idx_core::error::IdxError; use idx_core::investor::{ ConcentrationRank, HolderSource, ImportSource, Investor, parse_report_family, @@ -262,7 +263,7 @@ fn handle_discover( config: &IdxConfig, ) -> Result<(), IdxError> { if args.limit == 0 { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "--limit must be greater than 0".to_string(), )); } @@ -284,17 +285,17 @@ fn handle_discover( for report in reports { table.add_row(vec![ - Cell::new(report.publish_date.split('T').next().unwrap_or("-")), - Cell::new(report.family.label()), - Cell::new(report.status.label()), - Cell::new(if report.is_attachment { + cell(report.publish_date.split('T').next().unwrap_or("-")), + cell(report.family.label()), + cell(report.status.label()), + cell(if report.is_attachment { "attachment" } else { "main" }), - Cell::new(report.original_filename.unwrap_or_else(|| "-".to_string())), - Cell::new(report.title), - Cell::new(report.pdf_url), + cell(report.original_filename.unwrap_or_else(|| "-".to_string())), + cell(report.title), + cell(report.pdf_url), ]); } @@ -334,21 +335,21 @@ fn handle_ticker( for h in &data.holders { table.add_row(vec![ - Cell::new(h.rank), - Cell::new(match h.source { + cell(h.rank), + cell(match h.source { OwnershipSource::Ksei => "KSEI", OwnershipSource::Bing => "BING", }), - Cell::new(&h.name), - Cell::new(h.investor_type.clone().unwrap_or_else(|| "-".to_string())), - Cell::new(match h.locality { + cell(&h.name), + cell(h.investor_type.clone().unwrap_or_else(|| "-".to_string())), + cell(match h.locality { Some(idx_core::ownership::types::Locality::Local) => "L", Some(idx_core::ownership::types::Locality::Foreign) => "F", None => "-", }), - Cell::new(format_idr(h.shares)), - Cell::new(format_bps(h.percentage_bps)), - Cell::new(format_signal(h.signal)), + cell(format_idr(h.shares)), + cell(format_bps(h.percentage_bps)), + cell(format_signal(h.signal)), ]); } println!("{table}"); @@ -390,14 +391,14 @@ fn handle_entity( for row in &data.holdings { table.add_row(vec![ - Cell::new(&row.ticker.code), - Cell::new(match row.source { + cell(&row.ticker.code), + cell(match row.source { OwnershipSource::Ksei => "KSEI", OwnershipSource::Bing => "BING", }), - Cell::new(format_idr(row.shares)), - Cell::new(format_bps(row.percentage_bps)), - Cell::new(row.report_date.format("%Y-%m-%d").to_string()), + cell(format_idr(row.shares)), + cell(format_bps(row.percentage_bps)), + cell(row.report_date.format("%Y-%m-%d").to_string()), ]); } println!("{table}"); @@ -430,14 +431,14 @@ fn handle_search( for entity in &entities { table.add_row(vec![ - Cell::new(&entity.canonical_name), - Cell::new( + cell(&entity.canonical_name), + cell( entity .entity_type .clone() .unwrap_or_else(|| "-".to_string()), ), - Cell::new(entity.country.clone().unwrap_or_else(|| "-".to_string())), + cell(entity.country.clone().unwrap_or_else(|| "-".to_string())), ]); } println!("{table}"); @@ -463,10 +464,10 @@ fn handle_cross_holders( for (i, row) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(i + 1), - Cell::new(&row.entity.canonical_name), - Cell::new(row.ticker_count), - Cell::new(format_bps(row.total_bps)), + cell(i + 1), + cell(&row.entity.canonical_name), + cell(row.ticker_count), + cell(format_bps(row.total_bps)), ]); } @@ -502,13 +503,13 @@ fn handle_concentration( for (idx, (ticker, m)) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(idx + 1), - Cell::new(ticker), - Cell::new(format_bps(m.top1_bps)), - Cell::new(format_bps(m.top3_bps)), - Cell::new(m.hhi), - Cell::new(format_bps(m.free_float_bps)), - Cell::new(m.holder_count), + cell(idx + 1), + cell(ticker), + cell(format_bps(m.top1_bps)), + cell(format_bps(m.top3_bps)), + cell(m.hhi), + cell(format_bps(m.free_float_bps)), + cell(m.holder_count), ]); } @@ -556,11 +557,11 @@ fn print_flow_section(title: &str, rows: &[HolderRow]) { for (i, row) in rows.iter().enumerate() { table.add_row(vec![ - Cell::new(i + 1), - Cell::new(&row.name), - Cell::new(format_idr(row.shares)), - Cell::new(format_bps(row.percentage_bps)), - Cell::new(format_signal(row.signal)), + cell(i + 1), + cell(&row.name), + cell(format_idr(row.shares)), + cell(format_bps(row.percentage_bps)), + cell(format_signal(row.signal)), ]); } println!("{table}"); @@ -583,14 +584,17 @@ fn handle_graph(investor: &Investor, args: &GraphArgs, config: &IdxConfig) -> Re match args.format.trim().to_lowercase().as_str() { "table" => { - print!("{}", graph::format_graph_text(&nodes, &edges)); + print!( + "{}", + terminal_safe(&graph::format_graph_text(&nodes, &edges)) + ); Ok(()) } "dot" => { print!("{}", graph::format_graph_dot(&nodes, &edges)); Ok(()) } - other => Err(IdxError::ParseError(format!( + other => Err(IdxError::InvalidInput(format!( "invalid --format '{other}', expected table|dot" ))), } @@ -624,20 +628,20 @@ fn handle_changes( for row in changes { table.add_row(vec![ - Cell::new(row.ticker_code), - Cell::new(row.entity_name), - Cell::new(format_change_type(row.change_type)), - Cell::new( + cell(row.ticker_code), + cell(row.entity_name), + cell(format_change_type(row.change_type)), + cell( row.old_bps .map(format_bps) .unwrap_or_else(|| "-".to_string()), ), - Cell::new( + cell( row.new_bps .map(format_bps) .unwrap_or_else(|| "-".to_string()), ), - Cell::new(format_signed_bps(row.delta_bps)), + cell(format_signed_bps(row.delta_bps)), ]); } @@ -677,11 +681,11 @@ fn handle_resolve( for row in rows { table.add_row(vec![ - Cell::new(row.raw_name), - Cell::new(row.source), - Cell::new(row.ticker_code), - Cell::new(row.current_entity.unwrap_or_else(|| "-".to_string())), - Cell::new( + cell(row.raw_name), + cell(row.source), + cell(row.ticker_code), + cell(row.current_entity.unwrap_or_else(|| "-".to_string())), + cell( row.confidence .map(|v| format!("{v:.2}")) .unwrap_or_else(|| "-".to_string()), @@ -734,13 +738,13 @@ fn handle_import( config: &IdxConfig, ) -> Result<(), IdxError> { if args.file.is_none() && args.url.is_none() && args.fetch_bing.is_none() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "provide one of: --file, --url, or --fetch-bing".to_string(), )); } if args.file.is_some() && args.url.is_some() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "--file and --url are mutually exclusive".to_string(), )); } @@ -802,11 +806,11 @@ fn handle_releases(investor: &Investor, config: &IdxConfig) -> Result<(), IdxErr for r in releases { table.add_row(vec![ - Cell::new(r.as_of_date.format("%Y-%m-%d").to_string()), - Cell::new(r.row_count), - Cell::new(r.sha256), - Cell::new(r.source_url.unwrap_or_else(|| "-".to_string())), - Cell::new(r.imported_at), + cell(r.as_of_date.format("%Y-%m-%d").to_string()), + cell(r.row_count), + cell(r.sha256), + cell(r.source_url.unwrap_or_else(|| "-".to_string())), + cell(r.imported_at), ]); } diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index 91f75a1..c984112 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -20,7 +20,10 @@ fn main() { } fn run() -> Result<(), IdxError> { - let cli = Cli::parse(); + let cli = match Cli::try_parse() { + Ok(cli) => cli, + Err(err) => return Err(argument_error(err)), + }; runtime::set_quiet(cli.quiet); let config = match IdxConfig::load_with_cli(&cli) { Ok(config) => config, @@ -30,6 +33,7 @@ fn run() -> Result<(), IdxError> { } }; output::json::configure(config.output, config::stdout_is_terminal()); + runtime::set_json(config.output.is_json()); output::style::set_color(!config.no_color); match &cli.command { @@ -58,7 +62,7 @@ fn run() -> Result<(), IdxError> { } } Commands::Config(cfg) => { - if let Err(err) = cli::config::handle(cfg) { + if let Err(err) = cli::config::handle(cfg, &config.output) { emit_error(&err, &config.output); return Err(err); } @@ -82,6 +86,57 @@ fn run() -> Result<(), IdxError> { } /// Output format for errors raised before the configuration has loaded. +/// Argument errors honor the output contract: in JSON mode they are one JSON +/// error on stderr like any other failure (clap prints plain text). Help and +/// `--version` print as usual. +fn argument_error(err: clap::Error) -> IdxError { + use clap::error::ErrorKind; + if matches!( + err.kind(), + ErrorKind::DisplayHelp + | ErrorKind::DisplayVersion + | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ) || !requested_output_from_args().is_json() + { + err.exit(); + } + let rendered = err.render().to_string(); + let message = rendered + .trim() + .strip_prefix("error: ") + .unwrap_or(rendered.trim()) + .to_string(); + let error = IdxError::InvalidInput(message); + emit_error(&error, &requested_output_from_args()); + error +} + +/// The output format the raw arguments ask for, when they did not parse: +/// `-o`/`--output` (any spelling), then `IDX_OUTPUT`, then auto. +fn requested_output_from_args() -> OutputFormat { + let args: Vec = std::env::args().skip(1).collect(); + let flag = args.iter().enumerate().find_map(|(i, arg)| { + if arg == "-o" || arg == "--output" { + args.get(i + 1).cloned() + } else if let Some(value) = arg.strip_prefix("--output=") { + Some(value.to_string()) + } else if arg.starts_with("-o") && arg.len() > 2 { + Some(arg[2..].trim_start_matches('=').to_string()) + } else { + None + } + }); + let setting = flag + .and_then(|value| OutputSetting::parse(&value)) + .or_else(|| { + std::env::var("IDX_OUTPUT") + .ok() + .and_then(|value| OutputSetting::parse(&value)) + }) + .unwrap_or_default(); + setting.resolve(config::stdout_is_terminal()) +} + fn bootstrap_output_format(cli: &Cli) -> OutputFormat { let setting = cli .output diff --git a/crates/idx-cli/src/output/mod.rs b/crates/idx-cli/src/output/mod.rs index 2f1b8e4..055d08f 100644 --- a/crates/idx-cli/src/output/mod.rs +++ b/crates/idx-cli/src/output/mod.rs @@ -1,6 +1,7 @@ pub mod json; pub mod style; pub mod table; +pub mod text; use std::io::IsTerminal; diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index fb5df0c..961a7a9 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -1,4 +1,5 @@ use crate::output::style::Paint; +use crate::output::text::{cell, terminal_safe, truncate_chars}; use comfy_table::{Cell, Color, ContentArrangement, Table, presets::UTF8_FULL}; use idx_core::analysis::fundamental::{ @@ -56,27 +57,27 @@ pub fn print_quotes(quotes: &[Quote], no_color: bool) -> Result<(), IdxError> { for q in quotes { let pct_cell = match q.change_pct { - None => Cell::new("-"), - Some(pct) if no_color => Cell::new(format!("{pct:+.2}%")), - Some(pct) if pct >= 0.0 => Cell::new(format!("{pct:+.2}%")).fg(Color::Green), - Some(pct) => Cell::new(format!("{pct:+.2}%")).fg(Color::Red), + None => cell("-"), + Some(pct) if no_color => cell(format!("{pct:+.2}%")), + Some(pct) if pct >= 0.0 => cell(format!("{pct:+.2}%")).fg(Color::Green), + Some(pct) => cell(format!("{pct:+.2}%")).fg(Color::Red), }; table.add_row(vec![ - Cell::new(&q.symbol), - Cell::new(format_idr(q.price)), - Cell::new( + cell(&q.symbol), + cell(format_idr(q.price)), + cell( q.change .map(|c| format!("{c:+}")) .unwrap_or_else(|| "-".to_string()), ), pct_cell, - Cell::new(q.volume.map(format_u64).unwrap_or_else(|| "-".to_string())), - Cell::new( + cell(q.volume.map(format_u64).unwrap_or_else(|| "-".to_string())), + cell( q.market_cap .map(format_u64) .unwrap_or_else(|| "-".to_string()), ), - Cell::new(format_52w_range_bar(q.week52_position)), + cell(format_52w_range_bar(q.week52_position)), ]); } @@ -114,12 +115,12 @@ pub fn print_history(symbol: &str, history: &[Ohlc]) -> Result<(), IdxError> { .set_header(vec!["DATE", "OPEN", "HIGH", "LOW", "CLOSE", "VOLUME"]); for item in history { table.add_row(vec![ - Cell::new(item.date), - Cell::new(format_idr(item.open)), - Cell::new(format_idr(item.high)), - Cell::new(format_idr(item.low)), - Cell::new(format_idr(item.close)), - Cell::new(format_u64(item.volume)), + cell(item.date), + cell(format_idr(item.open)), + cell(format_idr(item.high)), + cell(format_idr(item.low)), + cell(format_idr(item.close)), + cell(format_u64(item.volume)), ]); } println!("{table}"); @@ -143,54 +144,54 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Current Price"), - Cell::new(format_idr(report.current_price)), - Cell::new("-"), + cell("Current Price"), + cell(format_idr(report.current_price)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 20"), - Cell::new(format_idr_option(report.sma20)), - Cell::new("-"), + cell("SMA 20"), + cell(format_idr_option(report.sma20)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 50"), - Cell::new(format_idr_option(report.sma50)), - Cell::new("-"), + cell("SMA 50"), + cell(format_idr_option(report.sma50)), + cell("-"), ]); table.add_row(vec![ - Cell::new("SMA 200"), - Cell::new(format_idr_option(report.sma200)), - Cell::new("-"), + cell("SMA 200"), + cell(format_idr_option(report.sma200)), + cell("-"), ]); table.add_row(vec![ - Cell::new("RSI (14)"), - Cell::new(format_float(report.rsi14, 2)), - Cell::new(format_signal(report.signals.rsi, no_color, false)), + cell("RSI (14)"), + cell(format_float(report.rsi14, 2)), + cell(format_signal(report.signals.rsi, no_color, false)), ]); table.add_row(vec![ - Cell::new("MACD (12,26,9)"), - Cell::new(format!( + cell("MACD (12,26,9)"), + cell(format!( "{}/{}/{}", format_float(report.macd.line, 2), format_float(report.macd.signal, 2), format_float(report.macd.histogram, 2) )), - Cell::new(format_signal(report.signals.macd, no_color, false)), + cell(format_signal(report.signals.macd, no_color, false)), ]); table.add_row(vec![ - Cell::new("Trend"), - Cell::new(trend_context(report)), - Cell::new(format_signal(report.signals.trend, no_color, false)), + cell("Trend"), + cell(trend_context(report)), + cell(format_signal(report.signals.trend, no_color, false)), ]); table.add_row(vec![ - Cell::new("Volume Ratio (20)"), - Cell::new(format_volume_ratio(report)), - Cell::new("-"), + cell("Volume Ratio (20)"), + cell(format_volume_ratio(report)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Overall Signal"), - Cell::new("-"), - Cell::new(format_signal(report.signals.overall, no_color, true)), + cell("Overall Signal"), + cell("-"), + cell(format_signal(report.signals.overall, no_color, true)), ]); println!("{table}"); @@ -207,19 +208,19 @@ pub fn print_growth(symbol: &str, report: &GrowthReport, no_color: bool) -> Resu .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Revenue Growth"), - Cell::new(format_pct(report.revenue_growth_pct)), - Cell::new(format_growth_signal(&report.revenue_signal, no_color)), + cell("Revenue Growth"), + cell(format_pct(report.revenue_growth_pct)), + cell(format_growth_signal(&report.revenue_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Earnings Growth"), - Cell::new(format_pct(report.earnings_growth_pct)), - Cell::new(format_growth_signal(&report.earnings_signal, no_color)), + cell("Earnings Growth"), + cell(format_pct(report.earnings_growth_pct)), + cell(format_growth_signal(&report.earnings_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_growth_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + cell(format_growth_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -240,39 +241,39 @@ pub fn print_valuation( .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("P/E (Trailing)"), - Cell::new(format_opt_f64(report.pe_trailing, 2)), - Cell::new(format_valuation_signal(&report.pe_signal, no_color)), + cell("P/E (Trailing)"), + cell(format_opt_f64(report.pe_trailing, 2)), + cell(format_valuation_signal(&report.pe_signal, no_color)), ]); table.add_row(vec![ - Cell::new("P/E (Forward)"), - Cell::new(format_opt_f64(report.pe_forward, 2)), - Cell::new("-"), + cell("P/E (Forward)"), + cell(format_opt_f64(report.pe_forward, 2)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Price/Book"), - Cell::new(format_opt_f64(report.pb, 2)), - Cell::new(format_valuation_signal(&report.pb_signal, no_color)), + cell("Price/Book"), + cell(format_opt_f64(report.pb, 2)), + cell(format_valuation_signal(&report.pb_signal, no_color)), ]); table.add_row(vec![ - Cell::new("ROE"), - Cell::new(format_pct(report.roe_pct)), - Cell::new(format_valuation_signal(&report.roe_signal, no_color)), + cell("ROE"), + cell(format_pct(report.roe_pct)), + cell(format_valuation_signal(&report.roe_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Net Margin"), - Cell::new(format_pct(report.net_margin_pct)), - Cell::new(format_valuation_signal(&report.margin_signal, no_color)), + cell("Net Margin"), + cell(format_pct(report.net_margin_pct)), + cell(format_valuation_signal(&report.margin_signal, no_color)), ]); table.add_row(vec![ - Cell::new("EV/EBITDA"), - Cell::new(format_opt_f64(report.ev_ebitda, 2)), - Cell::new(format_valuation_signal(&report.ev_ebitda_signal, no_color)), + cell("EV/EBITDA"), + cell(format_opt_f64(report.ev_ebitda, 2)), + cell(format_valuation_signal(&report.ev_ebitda_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_valuation_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + cell(format_valuation_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -289,24 +290,24 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( .set_header(vec!["METRIC", "VALUE", "SIGNAL"]); table.add_row(vec![ - Cell::new("Debt/Equity"), - Cell::new(format_opt_f64(report.debt_to_equity, 2)), - Cell::new(format_risk_signal(&report.de_signal, no_color)), + cell("Debt/Equity"), + cell(format_opt_f64(report.debt_to_equity, 2)), + cell(format_risk_signal(&report.de_signal, no_color)), ]); table.add_row(vec![ - Cell::new("Current Ratio"), - Cell::new(format_opt_f64(report.current_ratio, 2)), - Cell::new(format_risk_signal(&report.current_ratio_signal, no_color)), + cell("Current Ratio"), + cell(format_opt_f64(report.current_ratio, 2)), + cell(format_risk_signal(&report.current_ratio_signal, no_color)), ]); table.add_row(vec![ - Cell::new("ROA"), - Cell::new(format_pct(report.roa_pct)), - Cell::new("-"), + cell("ROA"), + cell(format_pct(report.roa_pct)), + cell("-"), ]); table.add_row(vec![ - Cell::new("Overall"), - Cell::new("-"), - Cell::new(format_risk_signal(&report.overall_signal, no_color)), + cell("Overall"), + cell("-"), + cell(format_risk_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -341,8 +342,8 @@ pub fn print_compare(reports: &[FundamentalReport], no_color: bool) -> Result<() .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic); - let mut header = vec![Cell::new("METRIC")]; - header.extend(reports.iter().map(|report| Cell::new(&report.symbol))); + let mut header = vec![cell("METRIC")]; + header.extend(reports.iter().map(|report| cell(&report.symbol))); table.set_header(header); add_compare_row( @@ -519,10 +520,11 @@ fn format_earnings_period(period: &str) -> String { return format!("FY{trimmed}"); } + // ASCII only, so the byte slices below fall on character boundaries. if trimmed.len() == 6 + && trimmed.is_ascii() && trimmed.starts_with('Q') - && trimmed[1..2].chars().all(|ch| ch.is_ascii_digit()) - && trimmed[2..].chars().all(|ch| ch.is_ascii_digit()) + && trimmed[1..].bytes().all(|byte| byte.is_ascii_digit()) { return format!("{} {}", &trimmed[..2], &trimmed[2..]); } @@ -644,7 +646,7 @@ fn format_text_signal( } fn add_compare_row(table: &mut Table, label: &str, values: Vec) { - let mut row = vec![Cell::new(label)]; + let mut row = vec![cell(label)]; row.extend(values.into_iter().map(Cell::new)); table.add_row(row); } @@ -664,7 +666,7 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { let add_if_present = |t: &mut Table, label: &str, value: &str| { if !value.is_empty() { - t.add_row(vec![Cell::new(label), Cell::new(value)]); + t.add_row(vec![cell(label), cell(value)]); } }; @@ -677,24 +679,17 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { add_if_present(&mut table, "City", &profile.city); add_if_present(&mut table, "Phone", &profile.phone); if profile.employees > 0 { - table.add_row(vec![ - Cell::new("Employees"), - Cell::new(profile.employees.to_string()), - ]); + table.add_row(vec![cell("Employees"), cell(profile.employees.to_string())]); } if !profile.description.is_empty() { // Truncate long descriptions for table display - let desc = if profile.description.len() > 200 { - format!("{}...", &profile.description[..200]) - } else { - profile.description.clone() - }; - table.add_row(vec![Cell::new("Description"), Cell::new(desc)]); + let desc = truncate_chars(&profile.description, 200); + table.add_row(vec![cell("Description"), cell(desc)]); } if !profile.officers.is_empty() { table.add_row(vec![ - Cell::new("Executives"), - Cell::new( + cell("Executives"), + cell( profile .officers .iter() @@ -726,7 +721,7 @@ pub fn print_financials(fin: &FinancialStatements) -> Result<(), IdxError> { entries.sort_by(|(left, _), (right, _)| left.cmp(right)); for (label, value) in entries { - t.add_row(vec![Cell::new(label), Cell::new(format_idr(*value as i64))]); + t.add_row(vec![cell(label), cell(format_idr(*value as i64))]); } println!("{t}"); }; @@ -803,10 +798,10 @@ pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { .set_header(vec!["RANGE", "BULLISH", "BEARISH", "NEUTRAL"]); for row in &data.statistics { table.add_row(vec![ - Cell::new(&row.time_range), - Cell::new(row.bullish), - Cell::new(row.bearish), - Cell::new(row.neutral), + cell(&row.time_range), + cell(row.bullish), + cell(row.bearish), + cell(row.neutral), ]); } println!("{table}"); @@ -814,20 +809,20 @@ pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { } pub fn print_insights(data: &InsightData) -> Result<(), IdxError> { - println!("{}", data.summary); + println!("{}", terminal_safe(&data.summary)); if !data.last_updated.is_empty() { - println!("Last updated: {}", data.last_updated); + println!("Last updated: {}", terminal_safe(&data.last_updated)); } if !data.highlights.is_empty() { println!("Highlights:"); for h in &data.highlights { - println!("- {h}"); + println!("- {}", terminal_safe(h)); } } if !data.risks.is_empty() { println!("Risks:"); for r in &data.risks { - println!("- {r}"); + println!("- {}", terminal_safe(r)); } } Ok(()) @@ -840,11 +835,11 @@ pub fn print_news(items: &[NewsItem]) -> Result<(), IdxError> { .set_header(vec!["TITLE", "ABOUT", "PROVIDER", "DATE", "URL"]); for item in items { table.add_row(vec![ - Cell::new(&item.title), - Cell::new(item.relevance.as_str()), - Cell::new(&item.provider), - Cell::new(&item.published_at), - Cell::new(truncate_url(&item.url)), + cell(&item.title), + cell(item.relevance.as_str()), + cell(&item.provider), + cell(&item.published_at), + cell(truncate_url(&item.url)), ]); } println!("{table}"); @@ -853,13 +848,13 @@ pub fn print_news(items: &[NewsItem]) -> Result<(), IdxError> { fn add_earnings_history_row(table: &mut Table, row: &EarningsData) { table.add_row(vec![ - Cell::new(format_earnings_period(&row.period_type)), - Cell::new(format_float(row.eps_actual, 2)), - Cell::new(format_float(row.eps_forecast, 2)), - Cell::new(format_float(row.eps_surprise, 2)), - Cell::new(format_float(row.eps_surprise_pct, 2)), - Cell::new(format_idr_option_from_f64(row.revenue_actual)), - Cell::new(format_table_date_option( + cell(format_earnings_period(&row.period_type)), + cell(format_float(row.eps_actual, 2)), + cell(format_float(row.eps_forecast, 2)), + cell(format_float(row.eps_surprise, 2)), + cell(format_float(row.eps_surprise_pct, 2)), + cell(format_idr_option_from_f64(row.revenue_actual)), + cell(format_table_date_option( row.earning_release_date.as_deref(), )), ]); @@ -867,21 +862,17 @@ fn add_earnings_history_row(table: &mut Table, row: &EarningsData) { fn add_earnings_forecast_row(table: &mut Table, row: &EarningsData) { table.add_row(vec![ - Cell::new(format_earnings_period(&row.period_type)), - Cell::new(format_float(row.eps_forecast, 2)), - Cell::new(format_idr_option_from_f64(row.revenue_forecast)), - Cell::new(format_table_date_option( + cell(format_earnings_period(&row.period_type)), + cell(format_float(row.eps_forecast, 2)), + cell(format_idr_option_from_f64(row.revenue_forecast)), + cell(format_table_date_option( row.earning_release_date.as_deref(), )), ]); } fn truncate_url(url: &str) -> String { - if url.len() > 72 { - format!("{}...", &url[..72]) - } else { - url.to_string() - } + truncate_chars(url, 72) } #[cfg(test)] diff --git a/crates/idx-cli/src/output/text.rs b/crates/idx-cli/src/output/text.rs new file mode 100644 index 0000000..c3c92d1 --- /dev/null +++ b/crates/idx-cli/src/output/text.rs @@ -0,0 +1,70 @@ +//! Text from providers and KSEI files is untrusted: before it reaches a +//! terminal, control characters (which can start escape sequences that clear +//! the screen, rewrite output, or set the window title) and bidirectional +//! overrides (which can reorder what is displayed) are replaced. + +use comfy_table::Cell; + +/// `text` with control characters other than newline, and bidirectional +/// override characters, replaced by U+FFFD. +pub fn terminal_safe(text: &str) -> String { + text.chars() + .map(|ch| { + let bidi = matches!(ch, '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}'); + if (ch.is_control() && ch != '\n') || bidi { + '\u{FFFD}' + } else { + ch + } + }) + .collect() +} + +/// A table cell whose content is made [`terminal_safe`]. +pub fn cell(value: impl ToString) -> Cell { + Cell::new(terminal_safe(&value.to_string())) +} + +/// At most `max` characters of `text` (never splitting one), with `...` +/// appended when it was cut. +pub fn truncate_chars(text: &str, max: usize) -> String { + match text.char_indices().nth(max) { + Some((end, _)) => format!("{}...", &text[..end]), + None => text.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::{terminal_safe, truncate_chars}; + + #[test] + fn escape_sequences_and_bidi_overrides_are_neutralized() { + assert_eq!( + terminal_safe("\u{1b}[2J\u{1b}[Hfake"), + "\u{FFFD}[2J\u{FFFD}[Hfake" + ); + assert_eq!( + terminal_safe("\u{1b}]0;title\u{7}x"), + "\u{FFFD}]0;title\u{FFFD}x" + ); + assert_eq!(terminal_safe("a\u{9b}31mb"), "a\u{FFFD}31mb"); + assert_eq!(terminal_safe("ab\u{202E}cd"), "ab\u{FFFD}cd"); + assert_eq!(terminal_safe("line one\nline two"), "line one\nline two"); + assert_eq!( + terminal_safe("Saham Grup Djarum’s 14,81%"), + "Saham Grup Djarum’s 14,81%" + ); + } + + #[test] + fn truncation_never_splits_a_character() { + let text = format!("{}é tail", "a".repeat(199)); + assert_eq!( + truncate_chars(&text, 200), + format!("{}é...", "a".repeat(199)) + ); + assert_eq!(truncate_chars("short", 200), "short"); + assert_eq!(truncate_chars("ééé", 2), "éé..."); + } +} diff --git a/crates/idx-cli/tests/golden/config-get.golden b/crates/idx-cli/tests/golden/config-get.golden index c9ca9de..a90ab07 100644 --- a/crates/idx-cli/tests/golden/config-get.golden +++ b/crates/idx-cli/tests/golden/config-get.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -msn +{"key":"general.provider","value":"msn"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-init.golden b/crates/idx-cli/tests/golden/config-init.golden index ab029c5..28a0e2f 100644 --- a/crates/idx-cli/tests/golden/config-init.golden +++ b/crates/idx-cli/tests/golden/config-init.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -/config/idx/config.toml +{"path":"/config/idx/config.toml"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-path.golden b/crates/idx-cli/tests/golden/config-path.golden index ab029c5..28a0e2f 100644 --- a/crates/idx-cli/tests/golden/config-path.golden +++ b/crates/idx-cli/tests/golden/config-path.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -/config/idx/config.toml +{"path":"/config/idx/config.toml"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-set-output-jsonl.golden b/crates/idx-cli/tests/golden/config-set-output-jsonl.golden index aecf1eb..02653fc 100644 --- a/crates/idx-cli/tests/golden/config-set-output-jsonl.golden +++ b/crates/idx-cli/tests/golden/config-set-output-jsonl.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -ok +{"key":"general.output","ok":true,"value":"jsonl"} --- stderr diff --git a/crates/idx-cli/tests/golden/config-set.golden b/crates/idx-cli/tests/golden/config-set.golden index aecf1eb..0529834 100644 --- a/crates/idx-cli/tests/golden/config-set.golden +++ b/crates/idx-cli/tests/golden/config-set.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -ok +{"key":"general.provider","ok":true,"value":"yahoo"} --- stderr diff --git a/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden b/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden index e653302..60dd4a1 100644 --- a/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden +++ b/crates/idx-cli/tests/golden/error-usage-missing-symbol.json.golden @@ -1,9 +1,4 @@ exit: 2 --- stdout --- stderr -error: the following required arguments were not provided: - ... - -Usage: idx stocks quote ... - -For more information, try '--help'. +{"code":"INVALIDINPUT","error":true,"message":"invalid input: the following required arguments were not provided:\n ...\n\nUsage: idx stocks quote ...\n\nFor more information, try '--help'."} diff --git a/crates/idx-core/src/cache.rs b/crates/idx-core/src/cache.rs index 5e1abb5..85669f8 100644 --- a/crates/idx-core/src/cache.rs +++ b/crates/idx-core/src/cache.rs @@ -185,13 +185,11 @@ impl Cache { } let mut removed = 0usize; let mut failed = Vec::new(); - Self::walk(&self.root, &mut |p| { - if p.is_file() { - match fs::remove_file(p) { - Ok(_) => removed += 1, - Err(_) => failed.push(p.to_path_buf()), - } - } + // `walk` yields files and symlinks, never entering a linked + // directory; remove_file on a symlink removes the link, not its target. + Self::walk(&self.root, &mut |p| match fs::remove_file(p) { + Ok(_) => removed += 1, + Err(_) => failed.push(p.to_path_buf()), })?; Ok((removed, failed)) } @@ -200,7 +198,10 @@ impl Cache { for entry in fs::read_dir(dir).map_err(|e| IdxError::Io(e.to_string()))? { let entry = entry.map_err(|e| IdxError::Io(e.to_string()))?; let path = entry.path(); - if path.is_dir() { + // file_type() does not follow symlinks, so a link to a directory + // outside the cache is never walked (or emptied by `clear`). + let file_type = entry.file_type().map_err(|e| IdxError::Io(e.to_string()))?; + if file_type.is_dir() { Self::walk(&path, f)?; } else { f(&path); @@ -297,6 +298,30 @@ pub fn cache_dir() -> Result { #[cfg(test)] mod tests { + + #[cfg(unix)] + #[test] + fn clear_removes_symlinks_without_touching_their_targets() { + let base = std::env::temp_dir().join(format!("idx-cache-link-{}", fastrand::u64(..))); + let outside = base.join("documents"); + let root = base.join("cache"); + std::fs::create_dir_all(&outside).unwrap(); + std::fs::create_dir_all(root.join("quote")).unwrap(); + std::fs::write(outside.join("keep.txt"), b"mine").unwrap(); + std::fs::write(root.join("quote").join("BBCA.json"), b"{}").unwrap(); + std::os::unix::fs::symlink(&outside, root.join("link")).unwrap(); + + let (removed, failed) = super::Cache::at(root.clone()).clear().unwrap(); + + assert!(failed.is_empty(), "{failed:?}"); + assert_eq!(removed, 2, "the cache file and the link itself"); + assert!( + outside.join("keep.txt").exists(), + "the link's target is left alone" + ); + assert!(!root.join("link").exists()); + std::fs::remove_dir_all(&base).unwrap(); + } use std::fs; use std::sync::atomic::{AtomicUsize, Ordering}; diff --git a/crates/idx-core/src/investor.rs b/crates/idx-core/src/investor.rs index b5012a2..6a13ca8 100644 --- a/crates/idx-core/src/investor.rs +++ b/crates/idx-core/src/investor.rs @@ -635,7 +635,7 @@ pub fn parse_report_family(raw: &str) -> Result, I "investor-type" | "investor_type" | "investortype" => { Ok(Some(OwnershipReportFamily::InvestorTypeBreakdown)) } - _ => Err(IdxError::ParseError( + _ => Err(IdxError::InvalidInput( "invalid --family, expected: all|above1|above5|investor-type".to_string(), )), } diff --git a/crates/idx-core/src/lib.rs b/crates/idx-core/src/lib.rs index bcc2e52..a89bc2f 100644 --- a/crates/idx-core/src/lib.rs +++ b/crates/idx-core/src/lib.rs @@ -23,6 +23,7 @@ pub mod runtime { use std::sync::atomic::{AtomicBool, Ordering}; static QUIET: AtomicBool = AtomicBool::new(false); + static JSON: AtomicBool = AtomicBool::new(false); pub fn set_quiet(quiet: bool) { QUIET.store(quiet, Ordering::Relaxed); @@ -32,15 +33,29 @@ pub mod runtime { QUIET.load(Ordering::Relaxed) } - pub fn info(message: impl Display) { - if !is_quiet() { - eprintln!("info: {message}"); + /// In JSON mode diagnostics are JSON lines, so stderr stays + /// machine-readable alongside a JSON error. + pub fn set_json(json: bool) { + JSON.store(json, Ordering::Relaxed); + } + + fn emit(level: &str, message: impl Display) { + if is_quiet() { + return; + } + if JSON.load(Ordering::Relaxed) { + let line = serde_json::json!({ "level": level, "message": message.to_string() }); + eprintln!("{line}"); + } else { + eprintln!("{level}: {message}"); } } + pub fn info(message: impl Display) { + emit("info", message); + } + pub fn warn(message: impl Display) { - if !is_quiet() { - eprintln!("warning: {message}"); - } + emit("warning", message); } } diff --git a/crates/idx-core/src/ownership/graph.rs b/crates/idx-core/src/ownership/graph.rs index d51fd12..274ec81 100644 --- a/crates/idx-core/src/ownership/graph.rs +++ b/crates/idx-core/src/ownership/graph.rs @@ -14,7 +14,7 @@ pub fn query_ownership_graph( ) -> Result<(Vec, Vec), IdxError> { let root = root.trim(); if root.is_empty() { - return Err(IdxError::ParseError( + return Err(IdxError::InvalidInput( "graph root cannot be empty".to_string(), )); } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d5c10ae..bb7d1dd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -162,7 +162,12 @@ CLI flags > environment variables > config file > defaults ## Error Strategy - `IdxError` enum (thiserror) with structured error codes - Table mode: human-readable error on stderr -- JSON and JSONL modes: `{"error": true, "code": "...", "message": "..."}` on stderr, stdout empty +- JSON and JSONL modes: `{"error": true, "code": "...", "message": "..."}` on stderr, stdout empty. + This includes argument errors (`INVALIDINPUT`, exit 2) when `-o`/`--output`, + `IDX_OUTPUT`, or auto mode with stdout not a terminal selects JSON; `--help` + and `--version` print normally. +- Untrusted text (provider strings, KSEI names) has control characters and + bidirectional overrides replaced before it reaches a terminal table. - Exit codes by class: `0` success, `2` `INVALIDINPUT`/`CONFIGERROR` and clap usage errors, `3` `SYMBOLNOTFOUND`/`NOMARKETDATA`/`CACHEMISS`, `4` `HTTP`/`RATELIMITED`/ `PROVIDERUNAVAILABLE`/`OFFLINE`/`AUTHERROR`, `5` `UNSUPPORTED`, `1` everything else @@ -182,4 +187,6 @@ CLI flags > environment variables > config file > defaults - `stocks quote` and `stocks compare` require at least one symbol. - Offline reads warn on stderr when serving cache entries past their TTL, including the cache fetch timestamp. `--quiet` suppresses these warnings. + In JSON mode warnings and info are JSON lines, + `{"level": "warning", "message": "..."}`, so stderr stays parseable. Staleness is not yet represented by a versioned JSON result envelope. From e3df327f31d13019eb7d124087e023470a4423b5 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:17:18 +0000 Subject: [PATCH 16/27] fix(cli): keep idx's own colors; sanitize remaining provider text; no UTF-8 panic From the Codex re-gate of the 0.3.3 fixes: - Colored signal cells (Bullish/Bearish, ratings, ownership change and flow labels) were painted before the new cell sanitizer, which turned their escape codes into U+FFFD on every color terminal; the goldens run without color and missed it. Labels idx produces itself go into cells untouched (`painted`), with the label made safe before styling. - Statement dates and currency, and the entity name in `ownership entity`, still reached the terminal unsanitized. - The JSON-mode argument handler read std::env::args(), which panics on a non-UTF-8 argument (exit 101 instead of 2); use args_os. - A missing subcommand printed help as plain text in JSON mode; it is an error (exit 2), so it is a JSON error there too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/cli/ownership.rs | 10 +++--- crates/idx-cli/src/main.rs | 13 +++++--- crates/idx-cli/src/output/table.rs | 51 ++++++++++++++++------------- crates/idx-cli/src/output/text.rs | 15 ++++++++- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index 0d19885..6abe2a6 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -8,7 +8,7 @@ use crate::config::IdxConfig; use crate::output::json; use crate::output::style::Paint; use crate::output::table::format_idr; -use crate::output::text::{cell, terminal_safe}; +use crate::output::text::{cell, painted, terminal_safe}; use idx_core::error::IdxError; use idx_core::investor::{ ConcentrationRank, HolderSource, ImportSource, Investor, parse_report_family, @@ -349,7 +349,7 @@ fn handle_ticker( }), cell(format_idr(h.shares)), cell(format_bps(h.percentage_bps)), - cell(format_signal(h.signal)), + painted(format_signal(h.signal)), ]); } println!("{table}"); @@ -381,7 +381,7 @@ fn handle_entity( println!( "{} {}", "Entity:".bold(), - data.entity.canonical_name.as_str().bold() + terminal_safe(&data.entity.canonical_name).bold() ); let mut table = Table::new(); table @@ -561,7 +561,7 @@ fn print_flow_section(title: &str, rows: &[HolderRow]) { cell(&row.name), cell(format_idr(row.shares)), cell(format_bps(row.percentage_bps)), - cell(format_signal(row.signal)), + painted(format_signal(row.signal)), ]); } println!("{table}"); @@ -630,7 +630,7 @@ fn handle_changes( table.add_row(vec![ cell(row.ticker_code), cell(row.entity_name), - cell(format_change_type(row.change_type)), + painted(format_change_type(row.change_type)), cell( row.old_bps .map(format_bps) diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index c984112..f94fb62 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -91,11 +91,11 @@ fn run() -> Result<(), IdxError> { /// `--version` print as usual. fn argument_error(err: clap::Error) -> IdxError { use clap::error::ErrorKind; + // A missing subcommand prints help but is an error (exit 2), so in JSON + // mode it is a JSON error too. if matches!( err.kind(), - ErrorKind::DisplayHelp - | ErrorKind::DisplayVersion - | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ErrorKind::DisplayHelp | ErrorKind::DisplayVersion ) || !requested_output_from_args().is_json() { err.exit(); @@ -114,7 +114,12 @@ fn argument_error(err: clap::Error) -> IdxError { /// The output format the raw arguments ask for, when they did not parse: /// `-o`/`--output` (any spelling), then `IDX_OUTPUT`, then auto. fn requested_output_from_args() -> OutputFormat { - let args: Vec = std::env::args().skip(1).collect(); + // args_os: an argument that is not UTF-8 (itself the error being + // reported) must not panic here. + let args: Vec = std::env::args_os() + .skip(1) + .map(|arg| arg.to_string_lossy().into_owned()) + .collect(); let flag = args.iter().enumerate().find_map(|(i, arg)| { if arg == "-o" || arg == "--output" { args.get(i + 1).cloned() diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index 961a7a9..8dd5be6 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -1,5 +1,5 @@ use crate::output::style::Paint; -use crate::output::text::{cell, terminal_safe, truncate_chars}; +use crate::output::text::{cell, painted, terminal_safe, truncate_chars}; use comfy_table::{Cell, Color, ContentArrangement, Table, presets::UTF8_FULL}; use idx_core::analysis::fundamental::{ @@ -166,7 +166,7 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I table.add_row(vec![ cell("RSI (14)"), cell(format_float(report.rsi14, 2)), - cell(format_signal(report.signals.rsi, no_color, false)), + painted(format_signal(report.signals.rsi, no_color, false)), ]); table.add_row(vec![ cell("MACD (12,26,9)"), @@ -176,12 +176,12 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I format_float(report.macd.signal, 2), format_float(report.macd.histogram, 2) )), - cell(format_signal(report.signals.macd, no_color, false)), + painted(format_signal(report.signals.macd, no_color, false)), ]); table.add_row(vec![ cell("Trend"), cell(trend_context(report)), - cell(format_signal(report.signals.trend, no_color, false)), + painted(format_signal(report.signals.trend, no_color, false)), ]); table.add_row(vec![ cell("Volume Ratio (20)"), @@ -191,7 +191,7 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I table.add_row(vec![ cell("Overall Signal"), cell("-"), - cell(format_signal(report.signals.overall, no_color, true)), + painted(format_signal(report.signals.overall, no_color, true)), ]); println!("{table}"); @@ -210,17 +210,17 @@ pub fn print_growth(symbol: &str, report: &GrowthReport, no_color: bool) -> Resu table.add_row(vec![ cell("Revenue Growth"), cell(format_pct(report.revenue_growth_pct)), - cell(format_growth_signal(&report.revenue_signal, no_color)), + painted(format_growth_signal(&report.revenue_signal, no_color)), ]); table.add_row(vec![ cell("Earnings Growth"), cell(format_pct(report.earnings_growth_pct)), - cell(format_growth_signal(&report.earnings_signal, no_color)), + painted(format_growth_signal(&report.earnings_signal, no_color)), ]); table.add_row(vec![ cell("Overall"), cell("-"), - cell(format_growth_signal(&report.overall_signal, no_color)), + painted(format_growth_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -243,7 +243,7 @@ pub fn print_valuation( table.add_row(vec![ cell("P/E (Trailing)"), cell(format_opt_f64(report.pe_trailing, 2)), - cell(format_valuation_signal(&report.pe_signal, no_color)), + painted(format_valuation_signal(&report.pe_signal, no_color)), ]); table.add_row(vec![ cell("P/E (Forward)"), @@ -253,27 +253,27 @@ pub fn print_valuation( table.add_row(vec![ cell("Price/Book"), cell(format_opt_f64(report.pb, 2)), - cell(format_valuation_signal(&report.pb_signal, no_color)), + painted(format_valuation_signal(&report.pb_signal, no_color)), ]); table.add_row(vec![ cell("ROE"), cell(format_pct(report.roe_pct)), - cell(format_valuation_signal(&report.roe_signal, no_color)), + painted(format_valuation_signal(&report.roe_signal, no_color)), ]); table.add_row(vec![ cell("Net Margin"), cell(format_pct(report.net_margin_pct)), - cell(format_valuation_signal(&report.margin_signal, no_color)), + painted(format_valuation_signal(&report.margin_signal, no_color)), ]); table.add_row(vec![ cell("EV/EBITDA"), cell(format_opt_f64(report.ev_ebitda, 2)), - cell(format_valuation_signal(&report.ev_ebitda_signal, no_color)), + painted(format_valuation_signal(&report.ev_ebitda_signal, no_color)), ]); table.add_row(vec![ cell("Overall"), cell("-"), - cell(format_valuation_signal(&report.overall_signal, no_color)), + painted(format_valuation_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -292,12 +292,12 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( table.add_row(vec![ cell("Debt/Equity"), cell(format_opt_f64(report.debt_to_equity, 2)), - cell(format_risk_signal(&report.de_signal, no_color)), + painted(format_risk_signal(&report.de_signal, no_color)), ]); table.add_row(vec![ cell("Current Ratio"), cell(format_opt_f64(report.current_ratio, 2)), - cell(format_risk_signal(&report.current_ratio_signal, no_color)), + painted(format_risk_signal(&report.current_ratio_signal, no_color)), ]); table.add_row(vec![ cell("ROA"), @@ -307,7 +307,7 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( table.add_row(vec![ cell("Overall"), cell("-"), - cell(format_risk_signal(&report.overall_signal, no_color)), + painted(format_risk_signal(&report.overall_signal, no_color)), ]); println!("{table}"); @@ -632,16 +632,18 @@ fn format_text_signal( positive: &[&str], negative: &[&str], ) -> String { + // The label comes from the analysis report; make it safe before styling. + let label = terminal_safe(signal); if no_color { - return signal.to_string(); + return label; } if positive.contains(&signal) { - signal.green().to_string() + label.green().to_string() } else if negative.contains(&signal) { - signal.red().to_string() + label.red().to_string() } else { - signal.yellow().to_string() + label.yellow().to_string() } } @@ -706,9 +708,12 @@ pub fn print_profile(profile: &CompanyProfile) -> Result<(), IdxError> { pub fn print_financials(fin: &FinancialStatements) -> Result<(), IdxError> { let print_section = |label: &str, section: &idx_core::api::types::StatementSection| { - println!("\n── {label} ({}) ──", format_table_date(§ion.end_date)); + println!( + "\n── {label} ({}) ──", + terminal_safe(&format_table_date(§ion.end_date)) + ); let mut t = Table::new(); - let value_header = format!("VALUE ({})", section.currency); + let value_header = format!("VALUE ({})", terminal_safe(§ion.currency)); t.load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) .set_header(vec!["LINE ITEM", value_header.as_str()]); diff --git a/crates/idx-cli/src/output/text.rs b/crates/idx-cli/src/output/text.rs index c3c92d1..d9c64cc 100644 --- a/crates/idx-cli/src/output/text.rs +++ b/crates/idx-cli/src/output/text.rs @@ -25,6 +25,12 @@ pub fn cell(value: impl ToString) -> Cell { Cell::new(terminal_safe(&value.to_string())) } +/// A cell for text idx produced itself (labels, numbers) that may carry its +/// own color codes, which [`cell`] would strip. Never for provider text. +pub fn painted(styled: String) -> Cell { + Cell::new(styled) +} + /// At most `max` characters of `text` (never splitting one), with `...` /// appended when it was cut. pub fn truncate_chars(text: &str, max: usize) -> String { @@ -36,7 +42,7 @@ pub fn truncate_chars(text: &str, max: usize) -> String { #[cfg(test)] mod tests { - use super::{terminal_safe, truncate_chars}; + use super::{painted, terminal_safe, truncate_chars}; #[test] fn escape_sequences_and_bidi_overrides_are_neutralized() { @@ -57,6 +63,13 @@ mod tests { ); } + #[test] + fn painted_cells_keep_idx_color_codes() { + // comfy-table appends a reset; the color codes themselves survive. + let green = "\u{1b}[32mBullish\u{1b}[39m".to_string(); + assert!(painted(green.clone()).content().starts_with(&green)); + } + #[test] fn truncation_never_splits_a_character() { let text = format!("{}é tail", "a".repeat(199)); From 75b8819d9516cd569744d46818d9b65bc7a557d4 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:30:58 +0000 Subject: [PATCH 17/27] fix(cli): sanitize every provider or user value printed outside tables From the second Codex re-gate: the empty-sentiment line printed MSN's symbol raw, so a hostile response could clear the screen. Sweep every print in the table and ownership renderers that interpolates data: the sentiment and analysis headings, stale-quote notes, sync reasons, flow period, search query, alias mapping, and DOT graph output. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/cli/ownership.rs | 30 +++++++++++++++++++++++------ crates/idx-cli/src/output/table.rs | 26 ++++++++++++++++++------- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index 6abe2a6..dca938f 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -250,7 +250,7 @@ fn handle_sync(investor: &Investor, args: &SyncArgs, config: &IdxConfig) -> Resu snapshot::OwnershipSyncAction::NoChange | snapshot::OwnershipSyncAction::SkippedNewer | snapshot::OwnershipSyncAction::SkippedDiverged => { - println!("{}", result.reason); + println!("{}", terminal_safe(&result.reason)); } } @@ -419,7 +419,10 @@ fn handle_search( } if entities.is_empty() { - println!("No entities found for query: {}", args.query); + println!( + "No entities found for query: {}", + terminal_safe(&args.query) + ); return Ok(()); } @@ -526,7 +529,10 @@ fn handle_flow(investor: &Investor, args: &FlowArgs, config: &IdxConfig) -> Resu &Option::::None, ); } - println!("No institutional flow data available for {symbol}."); + println!( + "No institutional flow data available for {}.", + terminal_safe(&symbol) + ); return Ok(()); }; @@ -534,7 +540,12 @@ fn handle_flow(investor: &Investor, args: &FlowArgs, config: &IdxConfig) -> Resu return json::print_json(&flow); } - println!("{} {} ({})", "Flow:".bold(), symbol.bold(), flow.period); + println!( + "{} {} ({})", + "Flow:".bold(), + terminal_safe(&symbol).bold(), + terminal_safe(&flow.period) + ); print_flow_section("TOP BUYERS", &flow.top_buyers); print_flow_section("TOP SELLERS", &flow.top_sellers); print_flow_section("NEW POSITIONS", &flow.new_positions); @@ -591,7 +602,10 @@ fn handle_graph(investor: &Investor, args: &GraphArgs, config: &IdxConfig) -> Re Ok(()) } "dot" => { - print!("{}", graph::format_graph_dot(&nodes, &edges)); + print!( + "{}", + terminal_safe(&graph::format_graph_dot(&nodes, &edges)) + ); Ok(()) } other => Err(IdxError::InvalidInput(format!( @@ -704,7 +718,11 @@ fn handle_resolve( "entity": entity, })); } - println!("Mapped alias '{alias}' -> '{entity}'."); + println!( + "Mapped alias '{}' -> '{}'.", + terminal_safe(alias), + terminal_safe(entity) + ); Ok(()) } ResolveCommand::Merge { keep, merge } => { diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index 8dd5be6..21264c7 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -83,7 +83,7 @@ pub fn print_quotes(quotes: &[Quote], no_color: bool) -> Result<(), IdxError> { println!("{table}"); for note in stale_quote_notes(quotes, chrono::Utc::now()) { - println!("{note}"); + println!("{}", terminal_safe(¬e)); } Ok(()) } @@ -107,7 +107,10 @@ fn stale_quote_notes(quotes: &[Quote], now: chrono::DateTime) -> Ve } pub fn print_history(symbol: &str, history: &[Ohlc]) -> Result<(), IdxError> { - println!("{}", format!("History for {symbol}").bold()); + println!( + "{}", + format!("History for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table .load_preset(UTF8_FULL) @@ -199,7 +202,10 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I } pub fn print_growth(symbol: &str, report: &GrowthReport, no_color: bool) -> Result<(), IdxError> { - println!("{}", format!("Growth Analysis for {symbol}").bold()); + println!( + "{}", + format!("Growth Analysis for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table @@ -232,7 +238,10 @@ pub fn print_valuation( report: &ValuationReport, no_color: bool, ) -> Result<(), IdxError> { - println!("{}", format!("Valuation Analysis for {symbol}").bold()); + println!( + "{}", + format!("Valuation Analysis for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table @@ -281,7 +290,10 @@ pub fn print_valuation( } pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<(), IdxError> { - println!("{}", format!("Risk Analysis for {symbol}").bold()); + println!( + "{}", + format!("Risk Analysis for {}", terminal_safe(symbol)).bold() + ); let mut table = Table::new(); table @@ -317,7 +329,7 @@ pub fn print_risk(symbol: &str, report: &RiskReport, no_color: bool) -> Result<( pub fn print_fundamental(report: &FundamentalReport, no_color: bool) -> Result<(), IdxError> { println!( "{}", - format!("Fundamental Analysis for {}", report.symbol).bold() + format!("Fundamental Analysis for {}", terminal_safe(&report.symbol)).bold() ); println!(); print_growth(&report.symbol, &report.growth, no_color)?; @@ -794,7 +806,7 @@ pub fn print_earnings(report: &EarningsReport) -> Result<(), IdxError> { pub fn print_sentiment(data: &SentimentData) -> Result<(), IdxError> { if data.statistics.is_empty() { - println!("No sentiment votes for {}.", data.symbol); + println!("No sentiment votes for {}.", terminal_safe(&data.symbol)); return Ok(()); } let mut table = Table::new(); From 927d4224e9bb42e7f395dbff241ee67da3c607b9 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:44:37 +0000 Subject: [PATCH 18/27] fix(cli): sanitize errors and diagnostics at their choke points From the narrow Codex confirmation: provider, KSEI, and manifest text still reached the terminal through error messages (a Yahoo error description, a manifest's `kind`), idx-core diagnostics, the ownership heading's ticker code and Bing date, the technical heading and comparison symbol, and printed paths. Sanitize where every error (emit_error) and every diagnostic (runtime) is printed instead of at each caller, plus the remaining direct prints. terminal_safe moves to idx_core::text so the runtime can use it; the CLI re-exports it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/cli/cache.rs | 5 +++- crates/idx-cli/src/cli/config.rs | 2 +- crates/idx-cli/src/cli/ownership.rs | 10 +++---- crates/idx-cli/src/output/mod.rs | 3 +- crates/idx-cli/src/output/table.rs | 5 ++-- crates/idx-cli/src/output/text.rs | 41 ++------------------------- crates/idx-core/src/lib.rs | 6 +++- crates/idx-core/src/text.rs | 44 +++++++++++++++++++++++++++++ 8 files changed, 67 insertions(+), 49 deletions(-) create mode 100644 crates/idx-core/src/text.rs diff --git a/crates/idx-cli/src/cli/cache.rs b/crates/idx-cli/src/cli/cache.rs index f542d10..6e13895 100644 --- a/crates/idx-cli/src/cli/cache.rs +++ b/crates/idx-cli/src/cli/cache.rs @@ -34,7 +34,10 @@ pub fn handle(cmd: &CacheCmd, format: &OutputFormat) -> Result<(), IdxError> { "newest": info.newest, })); } - println!("path: {}", info.path.display()); + println!( + "path: {}", + crate::output::text::terminal_safe(&info.path.display().to_string()) + ); println!("files: {}", info.files); println!("size_bytes: {}", info.total_size); println!( diff --git a/crates/idx-cli/src/cli/config.rs b/crates/idx-cli/src/cli/config.rs index e4d7dac..f6a3696 100644 --- a/crates/idx-cli/src/cli/config.rs +++ b/crates/idx-cli/src/cli/config.rs @@ -29,7 +29,7 @@ pub fn handle(cmd: &ConfigCmd, format: &OutputFormat) -> Result<(), IdxError> { if format.is_json() { json::print_json(&document) } else { - println!("{text}"); + println!("{}", crate::output::text::terminal_safe(&text)); Ok(()) } }; diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index dca938f..9ef70cd 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -228,7 +228,7 @@ fn handle_sync(investor: &Investor, args: &SyncArgs, config: &IdxConfig) -> Resu result.latest_as_of_date.format("%Y-%m-%d"), result.release_count, result.ticker_count, - result.db_path + terminal_safe(&result.db_path) ); } snapshot::OwnershipSyncAction::Updated => { @@ -237,14 +237,14 @@ fn handle_sync(investor: &Investor, args: &SyncArgs, config: &IdxConfig) -> Resu result.latest_as_of_date.format("%Y-%m-%d"), result.release_count, result.ticker_count, - result.db_path + terminal_safe(&result.db_path) ); } snapshot::OwnershipSyncAction::Refreshed => { println!( "Refreshed ownership snapshot {} in {}.", result.latest_as_of_date.format("%Y-%m-%d"), - result.db_path + terminal_safe(&result.db_path) ); } snapshot::OwnershipSyncAction::NoChange @@ -318,11 +318,11 @@ fn handle_ticker( println!( "{} {} KSEI={} Bing={}", "Ownership:".bold(), - data.ticker.code.bold(), + terminal_safe(&data.ticker.code).bold(), data.ksei_as_of .map(|d| d.format("%Y-%m-%d").to_string()) .unwrap_or_else(|| "-".to_string()), - data.bing_as_of.unwrap_or_else(|| "-".to_string()) + terminal_safe(&data.bing_as_of.unwrap_or_else(|| "-".to_string())) ); let mut table = Table::new(); diff --git a/crates/idx-cli/src/output/mod.rs b/crates/idx-cli/src/output/mod.rs index 055d08f..f9057b2 100644 --- a/crates/idx-cli/src/output/mod.rs +++ b/crates/idx-cli/src/output/mod.rs @@ -211,7 +211,8 @@ pub fn render_screener( pub fn emit_error(err: &IdxError, format: &OutputFormat) { if !format.is_json() { - eprintln!("Error: {err}"); + // Errors can carry provider, KSEI, or manifest text. + eprintln!("Error: {}", text::terminal_safe(&err.to_string())); return; } let payload = serde_json::json!({ diff --git a/crates/idx-cli/src/output/table.rs b/crates/idx-cli/src/output/table.rs index 21264c7..05f0907 100644 --- a/crates/idx-cli/src/output/table.rs +++ b/crates/idx-cli/src/output/table.rs @@ -135,7 +135,8 @@ pub fn print_technical(report: &TechnicalReport, no_color: bool) -> Result<(), I "{}", format!( "Technical Analysis for {} ({})", - report.symbol, report.as_of + terminal_safe(&report.symbol), + report.as_of ) .bold() ); @@ -363,7 +364,7 @@ pub fn print_compare(reports: &[FundamentalReport], no_color: bool) -> Result<() "Symbol", reports .iter() - .map(|report| report.symbol.clone()) + .map(|report| terminal_safe(&report.symbol)) .collect::>(), ); add_compare_row( diff --git a/crates/idx-cli/src/output/text.rs b/crates/idx-cli/src/output/text.rs index d9c64cc..91ffad4 100644 --- a/crates/idx-cli/src/output/text.rs +++ b/crates/idx-cli/src/output/text.rs @@ -1,24 +1,8 @@ -//! Text from providers and KSEI files is untrusted: before it reaches a -//! terminal, control characters (which can start escape sequences that clear -//! the screen, rewrite output, or set the window title) and bidirectional -//! overrides (which can reorder what is displayed) are replaced. +//! Terminal-safe table cells; see [`idx_core::text`]. use comfy_table::Cell; -/// `text` with control characters other than newline, and bidirectional -/// override characters, replaced by U+FFFD. -pub fn terminal_safe(text: &str) -> String { - text.chars() - .map(|ch| { - let bidi = matches!(ch, '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}'); - if (ch.is_control() && ch != '\n') || bidi { - '\u{FFFD}' - } else { - ch - } - }) - .collect() -} +pub use idx_core::text::terminal_safe; /// A table cell whose content is made [`terminal_safe`]. pub fn cell(value: impl ToString) -> Cell { @@ -42,26 +26,7 @@ pub fn truncate_chars(text: &str, max: usize) -> String { #[cfg(test)] mod tests { - use super::{painted, terminal_safe, truncate_chars}; - - #[test] - fn escape_sequences_and_bidi_overrides_are_neutralized() { - assert_eq!( - terminal_safe("\u{1b}[2J\u{1b}[Hfake"), - "\u{FFFD}[2J\u{FFFD}[Hfake" - ); - assert_eq!( - terminal_safe("\u{1b}]0;title\u{7}x"), - "\u{FFFD}]0;title\u{FFFD}x" - ); - assert_eq!(terminal_safe("a\u{9b}31mb"), "a\u{FFFD}31mb"); - assert_eq!(terminal_safe("ab\u{202E}cd"), "ab\u{FFFD}cd"); - assert_eq!(terminal_safe("line one\nline two"), "line one\nline two"); - assert_eq!( - terminal_safe("Saham Grup Djarum’s 14,81%"), - "Saham Grup Djarum’s 14,81%" - ); - } + use super::{painted, truncate_chars}; #[test] fn painted_cells_keep_idx_color_codes() { diff --git a/crates/idx-core/src/lib.rs b/crates/idx-core/src/lib.rs index a89bc2f..0e67aca 100644 --- a/crates/idx-core/src/lib.rs +++ b/crates/idx-core/src/lib.rs @@ -15,6 +15,7 @@ pub mod investor; pub mod market; #[cfg(feature = "ownership")] pub mod ownership; +pub mod text; pub use client::{CachePolicy, Fetched, Idx}; @@ -47,7 +48,10 @@ pub mod runtime { let line = serde_json::json!({ "level": level, "message": message.to_string() }); eprintln!("{line}"); } else { - eprintln!("{level}: {message}"); + eprintln!( + "{level}: {}", + crate::text::terminal_safe(&message.to_string()) + ); } } diff --git a/crates/idx-core/src/text.rs b/crates/idx-core/src/text.rs new file mode 100644 index 0000000..4421815 --- /dev/null +++ b/crates/idx-core/src/text.rs @@ -0,0 +1,44 @@ +//! Text from providers, KSEI files, and snapshot manifests is untrusted. +//! Before it reaches a terminal, control characters (which can start escape +//! sequences that clear the screen, rewrite output, or set the window title) +//! and bidirectional overrides (which can reorder what is displayed) are +//! replaced. + +/// `text` with control characters other than newline, and bidirectional +/// override characters, replaced by U+FFFD. +pub fn terminal_safe(text: &str) -> String { + text.chars() + .map(|ch| { + let bidi = matches!(ch, '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}'); + if (ch.is_control() && ch != '\n') || bidi { + '\u{FFFD}' + } else { + ch + } + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::terminal_safe; + + #[test] + fn escape_sequences_and_bidi_overrides_are_neutralized() { + assert_eq!( + terminal_safe("\u{1b}[2J\u{1b}[Hfake"), + "\u{FFFD}[2J\u{FFFD}[Hfake" + ); + assert_eq!( + terminal_safe("\u{1b}]0;title\u{7}x"), + "\u{FFFD}]0;title\u{FFFD}x" + ); + assert_eq!(terminal_safe("a\u{9b}31mb"), "a\u{FFFD}31mb"); + assert_eq!(terminal_safe("ab\u{202E}cd"), "ab\u{FFFD}cd"); + assert_eq!(terminal_safe("line one\nline two"), "line one\nline two"); + assert_eq!( + terminal_safe("Saham Grup Djarum’s 14,81%"), + "Saham Grup Djarum’s 14,81%" + ); + } +} From 0f2806f9f5fc4fdfcebd97a868d01c991fef4428 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 12:23:47 +0000 Subject: [PATCH 19/27] fix(cli): pick the argument-error format the way clap picks -o The raw scan took the first -o and missed clustered short flags, so `idx -o table config -o json get` and `idx -qojson config get` printed plain-text errors. Resolve -o with clap itself, ignoring the error being reported (Pullfrog review on #56). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/main.rs | 30 +++++++++++------------------- crates/idx-cli/tests/cli.rs | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index f94fb62..7214d17 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -112,27 +112,19 @@ fn argument_error(err: clap::Error) -> IdxError { } /// The output format the raw arguments ask for, when they did not parse: -/// `-o`/`--output` (any spelling), then `IDX_OUTPUT`, then auto. +/// `-o`/`--output` as clap reads it (clustered short flags, the last one +/// at any subcommand level), then `IDX_OUTPUT`, then auto. fn requested_output_from_args() -> OutputFormat { - // args_os: an argument that is not UTF-8 (itself the error being - // reported) must not panic here. - let args: Vec = std::env::args_os() - .skip(1) - .map(|arg| arg.to_string_lossy().into_owned()) - .collect(); - let flag = args.iter().enumerate().find_map(|(i, arg)| { - if arg == "-o" || arg == "--output" { - args.get(i + 1).cloned() - } else if let Some(value) = arg.strip_prefix("--output=") { - Some(value.to_string()) - } else if arg.starts_with("-o") && arg.len() > 2 { - Some(arg[2..].trim_start_matches('=').to_string()) - } else { - None - } - }); + use clap::CommandFactory; + // ignore_errors keeps every argument clap could read; args_os, because an + // argument that is not UTF-8 (itself the error being reported) must not + // panic here. + let flag = Cli::command() + .ignore_errors(true) + .try_get_matches_from(std::env::args_os()) + .ok() + .and_then(|matches| matches.get_one::("output").copied()); let setting = flag - .and_then(|value| OutputSetting::parse(&value)) .or_else(|| { std::env::var("IDX_OUTPUT") .ok() diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index 76cef35..c3c4ff5 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -2117,6 +2117,29 @@ fn offline_and_no_cache_flags_are_rejected() { ); } +#[test] +fn argument_errors_follow_the_output_format_clap_resolves() { + // The last -o wins at any subcommand level, and -o may sit in a cluster: + // argument errors pick the same format a successful command would. + for args in [ + &["-o", "table", "config", "-o", "json", "get"][..], + &["-qojson", "config", "get"][..], + ] { + run_error_json(test_bin("argument-error-json").args(args), "INVALIDINPUT"); + } + for args in [ + &["-o", "json", "config", "-o", "table", "get"][..], + &["-qotable", "config", "get"][..], + ] { + test_bin("argument-error-table") + .args(args) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); + } +} + #[test] fn msn_screen_rejects_invalid_filter() { test_bin("msn-screen-invalid-filter") From 70e923334db7b4537261b5e3e85898d008974adc Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 12:47:36 +0000 Subject: [PATCH 20/27] fix(cli): read an -o that follows an unknown argument clap stops parsing at an unknown argument, so `idx config get --bogus -o json` lost the -o. Drop each unknown argument and parse again before reading the output flag (Pullfrog review on #56). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/main.rs | 44 ++++++++++++++++++++++++++++--------- crates/idx-cli/tests/cli.rs | 13 +++++++++++ 2 files changed, 47 insertions(+), 10 deletions(-) diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index 7214d17..6c1f349 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -115,16 +115,9 @@ fn argument_error(err: clap::Error) -> IdxError { /// `-o`/`--output` as clap reads it (clustered short flags, the last one /// at any subcommand level), then `IDX_OUTPUT`, then auto. fn requested_output_from_args() -> OutputFormat { - use clap::CommandFactory; - // ignore_errors keeps every argument clap could read; args_os, because an - // argument that is not UTF-8 (itself the error being reported) must not - // panic here. - let flag = Cli::command() - .ignore_errors(true) - .try_get_matches_from(std::env::args_os()) - .ok() - .and_then(|matches| matches.get_one::("output").copied()); - let setting = flag + // args_os: an argument that is not UTF-8 (itself the error being + // reported) must not panic here. + let setting = output_flag(std::env::args_os().collect()) .or_else(|| { std::env::var("IDX_OUTPUT") .ok() @@ -134,6 +127,37 @@ fn requested_output_from_args() -> OutputFormat { setting.resolve(config::stdout_is_terminal()) } +/// clap stops at an unknown argument, so an `-o` after it would be missed; +/// drop each unknown argument and parse again. What is left is read with +/// ignore_errors, which keeps the matches made before any other error. +fn output_flag(mut args: Vec) -> Option { + use clap::CommandFactory; + use clap::error::{ContextKind, ContextValue, ErrorKind}; + for _ in 0..args.len() { + let Err(err) = Cli::command().try_get_matches_from(&args) else { + break; + }; + if err.kind() != ErrorKind::UnknownArgument { + break; + } + let Some(ContextValue::String(bad)) = err.get(ContextKind::InvalidArg) else { + break; + }; + let Some(index) = args.iter().skip(1).position(|arg| { + let arg = arg.to_string_lossy(); + arg == bad.as_str() || arg.starts_with(&format!("{bad}=")) + }) else { + break; + }; + args.remove(index + 1); + } + Cli::command() + .ignore_errors(true) + .try_get_matches_from(args) + .ok() + .and_then(|matches| matches.get_one::("output").copied()) +} + fn bootstrap_output_format(cli: &Cli) -> OutputFormat { let setting = cli .output diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index c3c4ff5..9ab558e 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -2127,6 +2127,19 @@ fn argument_errors_follow_the_output_format_clap_resolves() { ] { run_error_json(test_bin("argument-error-json").args(args), "INVALIDINPUT"); } + // An -o after an unknown argument still counts, over IDX_OUTPUT too. + run_error_json( + test_bin("argument-error-json") + .env("IDX_OUTPUT", "table") + .args(["config", "get", "--bogus", "-o", "json"]), + "INVALIDINPUT", + ); + test_bin("argument-error-table") + .args(["config", "get", "--bogus", "-o", "table"]) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); for args in [ &["-o", "json", "config", "-o", "table", "get"][..], &["-qotable", "config", "get"][..], From c8e79b8f7b83205bb34d7f7bca80bb9ac4933cf0 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 13:06:16 +0000 Subject: [PATCH 21/27] fix(cli): keep a recovered -o when a retry reaches --help or --version Dropping an unknown argument can expose a later --help, and clap then returns no matches, losing an -o already read. Keep the last pass that read one (Pullfrog review on #56). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/main.rs | 21 ++++++++++++++------- crates/idx-cli/tests/cli.rs | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index 6c1f349..df02338 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -128,11 +128,21 @@ fn requested_output_from_args() -> OutputFormat { } /// clap stops at an unknown argument, so an `-o` after it would be missed; -/// drop each unknown argument and parse again. What is left is read with -/// ignore_errors, which keeps the matches made before any other error. +/// drop each unknown argument and parse again. Each pass is read with +/// ignore_errors, which keeps the matches made before any other error; the +/// last pass that read an `-o` wins (a later pass can reach `--help`, which +/// returns no matches at all). fn output_flag(mut args: Vec) -> Option { use clap::CommandFactory; use clap::error::{ContextKind, ContextValue, ErrorKind}; + let read = |args: &[std::ffi::OsString]| { + Cli::command() + .ignore_errors(true) + .try_get_matches_from(args) + .ok() + .and_then(|matches| matches.get_one::("output").copied()) + }; + let mut found = read(&args); for _ in 0..args.len() { let Err(err) = Cli::command().try_get_matches_from(&args) else { break; @@ -150,12 +160,9 @@ fn output_flag(mut args: Vec) -> Option { break; }; args.remove(index + 1); + found = read(&args).or(found); } - Cli::command() - .ignore_errors(true) - .try_get_matches_from(args) - .ok() - .and_then(|matches| matches.get_one::("output").copied()) + found } fn bootstrap_output_format(cli: &Cli) -> OutputFormat { diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index 9ab558e..5cdd8ee 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -2134,6 +2134,25 @@ fn argument_errors_follow_the_output_format_clap_resolves() { .args(["config", "get", "--bogus", "-o", "json"]), "INVALIDINPUT", ); + // ...and so does one before it when --help or --version follows. + for args in [ + &["-o", "json", "config", "get", "--bogus", "--help"][..], + &["-o", "json", "--bogus", "--version"][..], + ] { + run_error_json( + test_bin("argument-error-json") + .env("IDX_OUTPUT", "table") + .args(args), + "INVALIDINPUT", + ); + } + test_bin("argument-error-table") + .env("IDX_OUTPUT", "json") + .args(["-o", "table", "config", "get", "--bogus", "--help"]) + .assert() + .failure() + .stdout("") + .stderr(predicate::str::starts_with("error: ")); test_bin("argument-error-table") .args(["config", "get", "--bogus", "-o", "table"]) .assert() From 57eccc58433e8429f2daa6f925bd692f9a325f55 Mon Sep 17 00:00:00 2001 From: hermes Date: Sun, 18 Oct 2026 02:11:31 +0000 Subject: [PATCH 22/27] fix(cli): idx config prints JSON only when JSON is asked for Auto output stays plain text when piped, as in 0.3.2, so scripts that read $(idx config path) or $(idx config get KEY) keep working; -o json, IDX_OUTPUT=json, or general.output = "json" print JSON. The config goldens return to their 0.3.2 content. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/cli/config.rs | 9 +++-- crates/idx-cli/src/config.rs | 18 +++++---- crates/idx-cli/src/main.rs | 2 +- crates/idx-cli/tests/cli.rs | 37 +++++++++++++++++++ crates/idx-cli/tests/golden/config-get.golden | 2 +- .../idx-cli/tests/golden/config-init.golden | 2 +- .../idx-cli/tests/golden/config-path.golden | 2 +- .../golden/config-set-output-jsonl.golden | 2 +- crates/idx-cli/tests/golden/config-set.golden | 2 +- web/src/content/docs/start/config.md | 4 ++ 10 files changed, 62 insertions(+), 18 deletions(-) diff --git a/crates/idx-cli/src/cli/config.rs b/crates/idx-cli/src/cli/config.rs index f6a3696..976c54a 100644 --- a/crates/idx-cli/src/cli/config.rs +++ b/crates/idx-cli/src/cli/config.rs @@ -1,7 +1,7 @@ use clap::{Args, Subcommand}; use crate::config::{config_path, ensure_default_config, get_config_value, set_config_value}; -use crate::output::{OutputFormat, json}; +use crate::output::{OutputSetting, json}; use idx_core::error::IdxError; #[derive(Debug, Args)] @@ -23,10 +23,11 @@ pub enum ConfigSubcommand { Path, } -pub fn handle(cmd: &ConfigCmd, format: &OutputFormat) -> Result<(), IdxError> { - // Each result as plain text, or one JSON document with -o json. +pub fn handle(cmd: &ConfigCmd, setting: OutputSetting) -> Result<(), IdxError> { + // Plain text, as scripts read `$(idx config path)`, unless JSON is asked + // for (-o json, IDX_OUTPUT, or general.output); auto stays text when piped. let print = |text: String, document: serde_json::Value| { - if format.is_json() { + if matches!(setting, OutputSetting::Json | OutputSetting::Jsonl) { json::print_json(&document) } else { println!("{}", crate::output::text::terminal_safe(&text)); diff --git a/crates/idx-cli/src/config.rs b/crates/idx-cli/src/config.rs index ba85b4c..a0ad8f3 100644 --- a/crates/idx-cli/src/config.rs +++ b/crates/idx-cli/src/config.rs @@ -14,6 +14,8 @@ use idx_core::error::IdxError; pub struct IdxConfig { pub core: CoreConfig, pub output: OutputFormat, + /// The setting `output` came from, before auto was resolved. + pub output_setting: OutputSetting, pub no_color: bool, } @@ -32,7 +34,7 @@ impl IdxConfig { pub fn load_with_cli(cli: &Cli) -> Result { let mut core = CoreConfig::load()?; core.verbose = cli.verbose > 0; - let mut output = OutputSetting::Auto.resolve(stdout_is_terminal()); + let mut setting = OutputSetting::Auto; let mut no_color = false; let path = config_path()?; @@ -41,8 +43,8 @@ impl IdxConfig { let parsed: FileConfig = toml::from_str(&raw).map_err(|e| IdxError::ConfigError(e.to_string()))?; if let Some(general) = parsed.general { - if let Some(setting) = general.output { - output = setting.resolve(stdout_is_terminal()); + if let Some(file_setting) = general.output { + setting = file_setting; } if let Some(color) = general.color { no_color = !color; @@ -51,25 +53,25 @@ impl IdxConfig { } if let Ok(value) = std::env::var("IDX_OUTPUT") { - let setting = OutputSetting::parse(&value).ok_or_else(|| { + setting = OutputSetting::parse(&value).ok_or_else(|| { IdxError::ConfigError(format!( "invalid IDX_OUTPUT value: '{value}', expected 'auto', 'table', 'json', or 'jsonl'" )) })?; - output = setting.resolve(stdout_is_terminal()); } if let Ok(value) = std::env::var("IDX_NO_COLOR") { no_color = value == "1" || value.eq_ignore_ascii_case("true"); } - if let Some(setting) = cli.output { - output = setting.resolve(stdout_is_terminal()); + if let Some(cli_setting) = cli.output { + setting = cli_setting; } no_color = no_color || cli.no_color || !stdout_is_terminal(); Ok(Self { core, - output, + output: setting.resolve(stdout_is_terminal()), + output_setting: setting, no_color, }) } diff --git a/crates/idx-cli/src/main.rs b/crates/idx-cli/src/main.rs index df02338..f5a7dec 100644 --- a/crates/idx-cli/src/main.rs +++ b/crates/idx-cli/src/main.rs @@ -62,7 +62,7 @@ fn run() -> Result<(), IdxError> { } } Commands::Config(cfg) => { - if let Err(err) = cli::config::handle(cfg, &config.output) { + if let Err(err) = cli::config::handle(cfg, config.output_setting) { emit_error(&err, &config.output); return Err(err); } diff --git a/crates/idx-cli/tests/cli.rs b/crates/idx-cli/tests/cli.rs index 5cdd8ee..0fb9812 100644 --- a/crates/idx-cli/tests/cli.rs +++ b/crates/idx-cli/tests/cli.rs @@ -980,6 +980,43 @@ fn config_set_and_get_provider_round_trip() { .stdout(predicate::str::contains("msn")); } +#[test] +fn config_prints_json_only_when_json_is_asked_for() { + // Piped with auto output, scripts read plain text: `$(idx config path)`. + let root = test_env_dir("config-output-format"); + bin_with_root(&root) + .args(["config", "set", "general.provider", "msn"]) + .assert() + .success() + .stdout("ok\n"); + bin_with_root(&root) + .args(["config", "get", "general.provider"]) + .assert() + .success() + .stdout("msn\n"); + + let flag = run_success_json(bin_with_root(&root).args([ + "-o", + "json", + "config", + "get", + "general.provider", + ])); + assert_eq!( + flag, + serde_json::json!({"key": "general.provider", "value": "msn"}) + ); + let env = run_success_json( + bin_with_root(&root) + .env("IDX_OUTPUT", "json") + .args(["config", "path"]), + ); + assert!( + env["path"].as_str().unwrap().ends_with("config.toml"), + "{env}" + ); +} + #[test] fn config_set_and_get_ownership_db_path_round_trip() { let root = test_env_dir("config-ownership-db-path"); diff --git a/crates/idx-cli/tests/golden/config-get.golden b/crates/idx-cli/tests/golden/config-get.golden index a90ab07..c9ca9de 100644 --- a/crates/idx-cli/tests/golden/config-get.golden +++ b/crates/idx-cli/tests/golden/config-get.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -{"key":"general.provider","value":"msn"} +msn --- stderr diff --git a/crates/idx-cli/tests/golden/config-init.golden b/crates/idx-cli/tests/golden/config-init.golden index 28a0e2f..ab029c5 100644 --- a/crates/idx-cli/tests/golden/config-init.golden +++ b/crates/idx-cli/tests/golden/config-init.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -{"path":"/config/idx/config.toml"} +/config/idx/config.toml --- stderr diff --git a/crates/idx-cli/tests/golden/config-path.golden b/crates/idx-cli/tests/golden/config-path.golden index 28a0e2f..ab029c5 100644 --- a/crates/idx-cli/tests/golden/config-path.golden +++ b/crates/idx-cli/tests/golden/config-path.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -{"path":"/config/idx/config.toml"} +/config/idx/config.toml --- stderr diff --git a/crates/idx-cli/tests/golden/config-set-output-jsonl.golden b/crates/idx-cli/tests/golden/config-set-output-jsonl.golden index 02653fc..aecf1eb 100644 --- a/crates/idx-cli/tests/golden/config-set-output-jsonl.golden +++ b/crates/idx-cli/tests/golden/config-set-output-jsonl.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -{"key":"general.output","ok":true,"value":"jsonl"} +ok --- stderr diff --git a/crates/idx-cli/tests/golden/config-set.golden b/crates/idx-cli/tests/golden/config-set.golden index 0529834..aecf1eb 100644 --- a/crates/idx-cli/tests/golden/config-set.golden +++ b/crates/idx-cli/tests/golden/config-set.golden @@ -1,4 +1,4 @@ exit: 0 --- stdout -{"key":"general.provider","ok":true,"value":"yahoo"} +ok --- stderr diff --git a/web/src/content/docs/start/config.md b/web/src/content/docs/start/config.md index 2595074..4304b4d 100644 --- a/web/src/content/docs/start/config.md +++ b/web/src/content/docs/start/config.md @@ -22,6 +22,10 @@ idx config get cache.quote_ttl idx config path ``` +`idx config` prints plain text, even when piped, so `$(idx config path)` works +in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or +`output = "json"`. + ```toml [general] provider = "msn" # msn | yahoo From 5cff14bf909ed7754472e1024407e6536b7a0820 Mon Sep 17 00:00:00 2001 From: hermes Date: Sun, 18 Oct 2026 02:13:50 +0000 Subject: [PATCH 23/27] docs(cli): help for config keys and output, changes dates, alias search `idx config --help` lists the keys and says when it prints JSON; `ownership changes` says both dates must be imported releases and that accounts are summed; `ownership search` and `entity` describe alias and substring matching. Regenerated the CLI reference page. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-cli/src/cli/config.rs | 17 ++- crates/idx-cli/src/cli/ownership.rs | 17 ++- .../idx-cli/tests/golden/help-config.golden | 44 +++++- .../tests/golden/help-ownership.golden | 2 +- web/src/content/docs/cli/reference.md | 139 ++++++++++++++---- 5 files changed, 177 insertions(+), 42 deletions(-) diff --git a/crates/idx-cli/src/cli/config.rs b/crates/idx-cli/src/cli/config.rs index 976c54a..95c6c79 100644 --- a/crates/idx-cli/src/cli/config.rs +++ b/crates/idx-cli/src/cli/config.rs @@ -5,7 +5,11 @@ use crate::output::{OutputSetting, json}; use idx_core::error::IdxError; #[derive(Debug, Args)] -#[command(about = "Manage configuration")] +#[command( + about = "Manage configuration", + long_about = "Manage configuration in ~/.config/idx/config.toml (or $XDG_CONFIG_HOME/idx/config.toml).\n\nResults print as plain text, even when piped, so `$(idx config path)` works in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or `general.output = \"json\"`.", + after_help = "Keys:\n general.provider, general.history_provider, general.exchange, general.output,\n general.color, cache.quote_ttl, cache.fundamental_ttl, ownership.db_path,\n ownership.snapshot_manifest\n\nExamples:\n idx config init\n idx config set general.output json\n idx config get cache.quote_ttl\n idx config path" +)] pub struct ConfigCmd { #[command(subcommand)] pub command: ConfigSubcommand, @@ -16,9 +20,16 @@ pub enum ConfigSubcommand { #[command(about = "Create default configuration file")] Init, #[command(about = "Get a configuration value by key")] - Get { key: String }, + Get { + /// Key such as general.provider (see `idx config --help`). + key: String, + }, #[command(about = "Set a configuration value by key")] - Set { key: String, value: String }, + Set { + /// Key such as general.provider (see `idx config --help`). + key: String, + value: String, + }, #[command(about = "Print configuration file path")] Path, } diff --git a/crates/idx-cli/src/cli/ownership.rs b/crates/idx-cli/src/cli/ownership.rs index 9ef70cd..125952f 100644 --- a/crates/idx-cli/src/cli/ownership.rs +++ b/crates/idx-cli/src/cli/ownership.rs @@ -50,8 +50,15 @@ pub enum OwnershipCommand { /// Show all holders for a ticker (KSEI + Bing combined). Ticker(TickerArgs), /// Show all holdings for an entity across tickers. + #[command( + after_help = "Examples:\n idx ownership entity \"ASTRA INTERNATIONAL\"\n idx ownership search astra # list candidates first" + )] Entity(EntityArgs), - /// Search entities by name. + /// Search entities by name or KSEI alias. + #[command( + long_about = "Search entities by name or KSEI alias.\n\nMatches canonical names and the raw names KSEI prints (e.g. `PT ASTRA INTERNATIONAL TBK`), case-insensitively. Ranked: exact name, exact alias, prefix, then substring.", + after_help = "Examples:\n idx ownership search astra\n idx ownership search \"PT ASTRA INTERNATIONAL TBK\" --limit 5" + )] Search(SearchArgs), /// Rank entities by cross-ownership breadth. CrossHolders(CrossHolderArgs), @@ -60,6 +67,10 @@ pub enum OwnershipCommand { /// Show Bing institutional flow for a ticker. Flow(FlowArgs), /// Diff two KSEI releases. + #[command( + long_about = "Diff two KSEI releases: new, exited, increased, and decreased holders.\n\nBoth dates must be imported releases (`idx ownership releases` lists them), with --from before --to. A holder's accounts in a ticker are summed, so an investor with several accounts compares by its total stake.", + after_help = "Examples:\n idx ownership releases\n idx ownership changes --from 2026-08-31 --to 2026-09-30" + )] Changes(ChangesArgs), /// Ownership network graph. Graph(GraphArgs), @@ -114,11 +125,13 @@ pub struct TickerArgs { #[derive(Debug, Args)] pub struct EntityArgs { + /// Entity name or part of one, case-insensitive; the shortest match is shown. pub name: String, } #[derive(Debug, Args)] pub struct SearchArgs { + /// Name, KSEI alias, or part of one. pub query: String, #[arg(long, default_value_t = 20)] pub limit: usize, @@ -147,8 +160,10 @@ pub struct FlowArgs { #[derive(Debug, Args)] pub struct ChangesArgs { + /// Earlier release date, YYYY-MM-DD. #[arg(long)] pub from: String, + /// Later release date, YYYY-MM-DD. #[arg(long)] pub to: String, } diff --git a/crates/idx-cli/tests/golden/help-config.golden b/crates/idx-cli/tests/golden/help-config.golden index e594fbc..845a21d 100644 --- a/crates/idx-cli/tests/golden/help-config.golden +++ b/crates/idx-cli/tests/golden/help-config.golden @@ -1,6 +1,8 @@ exit: 0 --- stdout -Manage configuration +Manage configuration in ~/.config/idx/config.toml (or $XDG_CONFIG_HOME/idx/config.toml). + +Results print as plain text, even when piped, so `$(idx config path)` works in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or `general.output = "json"`. Usage: idx config [OPTIONS] @@ -12,11 +14,37 @@ Commands: help Print this message or the help of the given subcommand(s) Options: - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Keys: + general.provider, general.history_provider, general.exchange, general.output, + general.color, cache.quote_ttl, cache.fundamental_ttl, ownership.db_path, + ownership.snapshot_manifest + +Examples: + idx config init + idx config set general.output json + idx config get cache.quote_ttl + idx config path --- stderr diff --git a/crates/idx-cli/tests/golden/help-ownership.golden b/crates/idx-cli/tests/golden/help-ownership.golden index 54501d7..a7e4741 100644 --- a/crates/idx-cli/tests/golden/help-ownership.golden +++ b/crates/idx-cli/tests/golden/help-ownership.golden @@ -15,7 +15,7 @@ Commands: sync Install or refresh a maintained ownership SQLite snapshot ticker Show all holders for a ticker (KSEI + Bing combined) entity Show all holdings for an entity across tickers - search Search entities by name + search Search entities by name or KSEI alias cross-holders Rank entities by cross-ownership breadth concentration Rank tickers by ownership concentration flow Show Bing institutional flow for a ticker diff --git a/web/src/content/docs/cli/reference.md b/web/src/content/docs/cli/reference.md index 2236c01..095347b 100644 --- a/web/src/content/docs/cli/reference.md +++ b/web/src/content/docs/cli/reference.md @@ -463,7 +463,9 @@ Examples: ## `idx config` ```text -Manage configuration +Manage configuration in ~/.config/idx/config.toml (or $XDG_CONFIG_HOME/idx/config.toml). + +Results print as plain text, even when piped, so `$(idx config path)` works in scripts. Ask for JSON with `-o json`, `IDX_OUTPUT=json`, or `general.output = "json"`. Usage: idx config [OPTIONS] @@ -475,13 +477,39 @@ Commands: help Print this message or the help of the given subcommand(s) Options: - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Keys: + general.provider, general.history_provider, general.exchange, general.output, + general.color, cache.quote_ttl, cache.fundamental_ttl, ownership.db_path, + ownership.snapshot_manifest + +Examples: + idx config init + idx config set general.output json + idx config get cache.quote_ttl + idx config path ``` ### `idx config init` @@ -509,7 +537,7 @@ Get a configuration value by key Usage: idx config get [OPTIONS] Arguments: - + Key such as general.provider (see `idx config --help`) Options: -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] @@ -529,7 +557,7 @@ Set a configuration value by key Usage: idx config set [OPTIONS] Arguments: - + Key such as general.provider (see `idx config --help`) Options: @@ -633,7 +661,7 @@ Commands: sync Install or refresh a maintained ownership SQLite snapshot ticker Show all holders for a ticker (KSEI + Bing combined) entity Show all holdings for an entity across tickers - search Search entities by name + search Search entities by name or KSEI alias cross-holders Rank entities by cross-ownership breadth concentration Rank tickers by ownership concentration flow Show Bing institutional flow for a ticker @@ -833,7 +861,7 @@ Show all holdings for an entity across tickers Usage: idx ownership entity [OPTIONS] Arguments: - + Entity name or part of one, case-insensitive; the shortest match is shown Options: -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] @@ -843,27 +871,55 @@ Options: --offline Run without network requests --no-cache Bypass local cache -h, --help Print help + +Examples: + idx ownership entity "ASTRA INTERNATIONAL" + idx ownership search astra # list candidates first ``` ### `idx ownership search` ```text -Search entities by name +Search entities by name or KSEI alias. + +Matches canonical names and the raw names KSEI prints (e.g. `PT ASTRA INTERNATIONAL TBK`), case-insensitively. Ranked: exact name, exact alias, prefix, then substring. Usage: idx ownership search [OPTIONS] Arguments: + Name, KSEI alias, or part of one Options: - --limit [default: 20] - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + --limit + [default: 20] + + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Examples: + idx ownership search astra + idx ownership search "PT ASTRA INTERNATIONAL TBK" --limit 5 ``` ### `idx ownership cross-holders` @@ -927,20 +983,45 @@ Options: ### `idx ownership changes` ```text -Diff two KSEI releases +Diff two KSEI releases: new, exited, increased, and decreased holders. + +Both dates must be imported releases (`idx ownership releases` lists them), with --from before --to. A holder's accounts in a ticker are summed, so an investor with several accounts compares by its total stake. Usage: idx ownership changes [OPTIONS] --from --to Options: --from - -o, --output Output format [default: auto = table on a terminal, json when piped] [possible values: auto, table, json, jsonl] - --no-color Disable colored output + Earlier release date, YYYY-MM-DD + + -o, --output + Output format [default: auto = table on a terminal, json when piped] + + [possible values: auto, table, json, jsonl] + + --no-color + Disable colored output + --to - -q, --quiet Suppress non-essential output - -v, --verbose... Increase verbosity (-v, -vv) - --offline Run without network requests - --no-cache Bypass local cache - -h, --help Print help + Later release date, YYYY-MM-DD + + -q, --quiet + Suppress non-essential output + + -v, --verbose... + Increase verbosity (-v, -vv) + + --offline + Run without network requests + + --no-cache + Bypass local cache + + -h, --help + Print help (see a summary with '-h') + +Examples: + idx ownership releases + idx ownership changes --from 2026-08-31 --to 2026-09-30 ``` ### `idx ownership graph` From 49b2d3aababd46b314a77e111688c14f206773e6 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 04:48:10 +0000 Subject: [PATCH 24/27] fix(analysis): ratings need evidence; negative equity is high risk From the full-repo Codex audit: - The overall fundamental rating was "healthy" whenever growth was growing and valuation and risk were merely not "expensive" / not "high risk", so with no valuation or risk data at all (partial MSN fundamentals) it still read "healthy". It now needs valuation undervalued or fairly valued and risk low or moderate. - Risk recognized only heavy leverage or a weak current ratio as high risk; negative equity fell through to "moderate risk", and a known adverse signal with the other metric missing became "incomplete data". Any known adverse signal now makes it high risk. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- crates/idx-core/src/analysis/fundamental.rs | 48 ++++++++++++++++++--- crates/idx-core/src/api/types.rs | 2 +- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/crates/idx-core/src/analysis/fundamental.rs b/crates/idx-core/src/analysis/fundamental.rs index aec0e80..557a4cc 100644 --- a/crates/idx-core/src/analysis/fundamental.rs +++ b/crates/idx-core/src/analysis/fundamental.rs @@ -148,12 +148,16 @@ pub fn analyze_risk(fundamentals: &Fundamentals) -> RiskReport { let de_signal = de_signal(fundamentals.debt_to_equity); let current_ratio_signal = cr_signal(fundamentals.current_ratio); - let overall_signal = if de_signal == "no data" && current_ratio_signal == "no data" { + // A known adverse signal decides on its own: negative equity or heavy + // leverage is high risk even when the current ratio is missing. + let overall_signal = if matches!(de_signal, "highly leveraged" | "negative equity") + || current_ratio_signal == "weak" + { + "high risk" + } else if de_signal == "no data" && current_ratio_signal == "no data" { "no data" } else if de_signal == "no data" || current_ratio_signal == "no data" { "incomplete data" - } else if de_signal == "highly leveraged" || current_ratio_signal == "weak" { - "high risk" } else if de_signal == "conservative" && matches!(current_ratio_signal, "strong" | "adequate") { "low risk" } else { @@ -176,9 +180,14 @@ pub fn analyze_fundamental(symbol: &str, fundamentals: &Fundamentals) -> Fundame let valuation = analyze_valuation(fundamentals); let risk = analyze_risk(fundamentals); + // "healthy" needs evidence on all three: missing valuation or risk data + // must not pass for "not expensive" or "not risky". let overall_signal = if growth.overall_signal == "growing" - && valuation.overall_signal != "expensive" - && risk.overall_signal != "high risk" + && matches!( + valuation.overall_signal.as_str(), + "undervalued" | "fairly valued" + ) + && matches!(risk.overall_signal.as_str(), "low risk" | "moderate risk") { "healthy" } else if growth.overall_signal == "shrinking" && risk.overall_signal == "high risk" { @@ -375,6 +384,35 @@ mod tests { } } + #[test] + fn missing_valuation_and_risk_data_is_not_healthy() { + // Growth alone (12% revenue, 22% earnings) used to rate "healthy". + let fundamentals = Fundamentals { + revenue_growth: Some(0.12), + earnings_growth: Some(0.22), + ..Fundamentals::default() + }; + let report = analyze_fundamental("X.JK", &fundamentals); + assert_eq!(report.growth.overall_signal, "growing"); + assert_eq!(report.overall_signal, "mixed"); + } + + #[test] + fn negative_equity_is_high_risk_and_not_healthy() { + let mut fundamentals = sample_fundamentals(); + fundamentals.debt_to_equity = Some(-10.0); + assert_eq!(analyze_risk(&fundamentals).overall_signal, "high risk"); + assert_eq!( + analyze_fundamental("X.JK", &fundamentals).overall_signal, + "mixed" + ); + + // Known heavy leverage decides even with the current ratio missing. + fundamentals.debt_to_equity = Some(250.0); + fundamentals.current_ratio = None; + assert_eq!(analyze_risk(&fundamentals).overall_signal, "high risk"); + } + #[test] fn growth_signal_thresholds_match_python() { assert_eq!(growth_signal(None), "no data"); diff --git a/crates/idx-core/src/api/types.rs b/crates/idx-core/src/api/types.rs index 8949e8f..920a238 100644 --- a/crates/idx-core/src/api/types.rs +++ b/crates/idx-core/src/api/types.rs @@ -74,7 +74,7 @@ pub struct Ohlc { } /// Fundamental metrics normalized from Yahoo Finance `/v10/finance/quoteSummary`. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct Fundamentals { pub trailing_pe: Option, pub forward_pe: Option, From 304709f3ac041724658dbc38f45fc843fcbe3fcb Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 04:53:08 +0000 Subject: [PATCH 25/27] fix(release): robust publisher fields, WAL-safe copy, installer tag order, pinned actions From the full-repo Codex audit: - The publisher read discovery fields one per line, so a report title with newlines shifted every later field and published a manifest without source.publish_date, which every client rejects (blocking all syncs). Read them NUL-separated, and refuse to write a manifest missing a field clients require. - The snapshot builder copied the SQLite file with cp; committed data still in the -wal file of an open database would be missing. Checkpoint first (a no-op for the publisher's closed database, so the bytes and hash are unchanged) and refuse to copy when that cannot complete. - install.sh took the tag from a greedy match, which on a compact single-line releases response picks the last tag, not the newest (it installed v0.2.3 over v0.2.4 in the new test). Split fields first. - scripts/audit-msn-fundamentals.sh read the symbol map from a path that no longer exists and silently checked no stocks. - release.yml granted contents: write to every job; only publish gets it now. Actions in the release and snapshot workflows are pinned to commit SHAs. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- .../workflows/publish-ownership-snapshot.yml | 6 +++--- .github/workflows/release.yml | 17 +++++++++------- install.sh | 5 ++++- scripts/audit-msn-fundamentals.sh | 4 +++- scripts/build-latest-ownership-snapshot.sh | 20 ++++++++++++++----- scripts/build-ownership-snapshot.sh | 9 +++++++++ scripts/install-sh-test.sh | 11 ++++++++++ 7 files changed, 55 insertions(+), 17 deletions(-) diff --git a/.github/workflows/publish-ownership-snapshot.yml b/.github/workflows/publish-ownership-snapshot.yml index 3204670..179c76e 100644 --- a/.github/workflows/publish-ownership-snapshot.yml +++ b/.github/workflows/publish-ownership-snapshot.yml @@ -26,10 +26,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Install Nix - uses: cachix/install-nix-action@v30 + uses: cachix/install-nix-action@08dcb3a5e62fa31e2da3d490afc4176ef55ecd72 # v30 - name: Build ownership publisher run: nix build .#ownership-publisher @@ -54,7 +54,7 @@ jobs: - name: Upload workflow artifact copy # A legitimate up-to-date run produces no new local artifacts. - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: ownership-snapshot-${{ github.run_id }} path: dist/ownership-snapshot/* diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 135a50f..7dc0bdd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,8 +20,9 @@ on: - "scripts/install.js" - "install.sh" +# Read-only by default; only the publish job writes the release. permissions: - contents: write + contents: read env: RELEASE_TAG: ${{ github.event_name == 'workflow_dispatch' && inputs.tag || github.ref_name }} @@ -49,12 +50,12 @@ jobs: steps: - name: Check out source - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: ref: ${{ github.event_name == 'workflow_dispatch' && inputs.tag || github.ref }} - name: Install Rust toolchain and target - uses: dtolnay/rust-toolchain@stable + uses: dtolnay/rust-toolchain@b725eda525362e7cf34af9a7243cac1e3f01a7ec # stable with: targets: ${{ matrix.target }} @@ -83,7 +84,7 @@ jobs: ./idx-mcp-${{ matrix.asset }} --help >/dev/null - name: Upload target artifacts - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: idx-${{ matrix.asset }} path: | @@ -96,9 +97,11 @@ jobs: needs: build if: github.event_name != 'pull_request' runs-on: ubuntu-latest + permissions: + contents: write steps: - name: Check out install script - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: ref: ${{ github.event_name == 'workflow_dispatch' && inputs.tag || github.ref }} sparse-checkout: install.sh @@ -106,7 +109,7 @@ jobs: path: src - name: Download target artifacts - uses: actions/download-artifact@v4 + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 with: pattern: idx-* merge-multiple: true @@ -119,7 +122,7 @@ jobs: > SHA256SUMS - name: Attach binaries to the version tag - uses: softprops/action-gh-release@v2 + uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2 with: tag_name: ${{ env.RELEASE_TAG }} files: | diff --git a/install.sh b/install.sh index af4f8b6..91b90de 100755 --- a/install.sh +++ b/install.sh @@ -97,8 +97,11 @@ resolve_latest_version() { fetch "$api_url/repos/$REPO/releases?per_page=30" "$releases" || err "could not list releases; pass --version to skip the lookup" + # One field per line first: on a single-line (compact) response the greedy + # match would otherwise pick the last tag, not the newest. tag="$( - sed -n 's/.*"tag_name"[[:space:]]*:[[:space:]]*"\(v[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)".*/\1/p' "$releases" | + tr ',' '\n' < "$releases" | + sed -n 's/.*"tag_name"[[:space:]]*:[[:space:]]*"\(v[0-9][0-9]*\.[0-9][0-9]*\.[0-9][0-9]*\)".*/\1/p' | head -n 1 )" [ -n "$tag" ] || err "no vX.Y.Z release found for $REPO" diff --git a/scripts/audit-msn-fundamentals.sh b/scripts/audit-msn-fundamentals.sh index 593c8f1..b36825e 100755 --- a/scripts/audit-msn-fundamentals.sh +++ b/scripts/audit-msn-fundamentals.sh @@ -130,7 +130,9 @@ resolve_tickers() { done IFS="$old_ifs" else - mapfile -t TICKER_LIST < <(cut -f1 "$ROOT_DIR/src/api/msn/symbol_ids.tsv") + symbol_map="$ROOT_DIR/crates/idx-core/src/api/msn/symbol_ids.tsv" + [[ -f "$symbol_map" ]] || { echo "symbol map not found: $symbol_map" >&2; exit 1; } + mapfile -t TICKER_LIST < <(cut -f1 "$symbol_map") fi if [[ ${#TICKER_LIST[@]} -eq 0 ]]; then diff --git a/scripts/build-latest-ownership-snapshot.sh b/scripts/build-latest-ownership-snapshot.sh index 0c5b434..0641f25 100755 --- a/scripts/build-latest-ownership-snapshot.sh +++ b/scripts/build-latest-ownership-snapshot.sh @@ -163,8 +163,11 @@ DB_PATH="$XDG_DATA_HOME/idx/ownership.db" printf 'Discovering latest supported IDX/KSEI above-1%% report...\n' "$IDX_BIN" -o json ownership discover --family above1 --limit 50 > "$DISCOVERY_JSON" -discovery_payload="$( - jq -r ' +# NUL-separated, so a field containing newlines (a report title) cannot +# shift the fields after it. Written to a file: command substitution drops +# NULs, and a jq failure must still stop the script under set -e. +DISCOVERY_FIELDS="$WORKDIR/discovery-fields" +jq --raw-output0 ' if type != "array" or length == 0 then error("ownership discover returned no reports") else @@ -184,9 +187,8 @@ discovery_payload="$( ] | .[] end - ' "$DISCOVERY_JSON" -)" -mapfile -t discovery_fields <<< "$discovery_payload" + ' "$DISCOVERY_JSON" > "$DISCOVERY_FIELDS" +mapfile -d '' -t discovery_fields < "$DISCOVERY_FIELDS" DISCOVERED_FAMILY="${discovery_fields[0]:-}" DISCOVERED_LISTING_PAGE_URL="${discovery_fields[1]:-}" @@ -308,6 +310,14 @@ jq \ original_filename: ($original_filename | if . == "" then null else . end) } ' "$MANIFEST_PATH" > "$TMP_MANIFEST_PATH" +# Clients reject a manifest missing any of these, so a bad one would block +# every `idx ownership sync`; refuse to produce it. +if ! jq -e '[.source.pdf_url, .source.title, .source.publish_date, .snapshot.version] + | all(type == "string" and (gsub("^\\s+|\\s+$"; "") | length > 0))' \ + "$TMP_MANIFEST_PATH" > /dev/null; then + echo "built manifest is missing source.pdf_url, source.title, source.publish_date, or snapshot.version" >&2 + exit 1 +fi mv "$TMP_MANIFEST_PATH" "$MANIFEST_PATH" printf 'Prepared release-ready snapshot from %s (%s)\n' \ diff --git a/scripts/build-ownership-snapshot.sh b/scripts/build-ownership-snapshot.sh index b485c62..d626209 100755 --- a/scripts/build-ownership-snapshot.sh +++ b/scripts/build-ownership-snapshot.sh @@ -94,6 +94,15 @@ MANIFEST_PATH="$OUTPUT_DIR/ownership-snapshot-manifest.json" TMP_ARTIFACT_PATH="$OUTPUT_DIR/.ownership-snapshot-${LATEST_AS_OF}.sqlite.tmp.$$" # Remove a partial copy if the script fails before the atomic rename below. trap 'rm -f "$TMP_ARTIFACT_PATH"' EXIT +# Committed data can still sit in the -wal file while another connection has +# the database open; checkpoint it into the main file first (a no-op for a +# closed database, so its bytes and hash are unchanged), and refuse to copy +# when that cannot complete. +checkpoint="$(sqlite3 "$DB_PATH" "PRAGMA wal_checkpoint(TRUNCATE);")" +if [[ "${checkpoint%%|*}" != "0" ]]; then + echo "ownership database is busy; could not checkpoint its WAL before copying: $DB_PATH" >&2 + exit 1 +fi cp "$DB_PATH" "$TMP_ARTIFACT_PATH" # Immutable, content-addressed name. The publisher relies on this to upload and diff --git a/scripts/install-sh-test.sh b/scripts/install-sh-test.sh index 4bcff0e..769e91c 100755 --- a/scripts/install-sh-test.sh +++ b/scripts/install-sh-test.sh @@ -191,6 +191,17 @@ else fail "expands quoted ~ and tolerates trailing slashes"; cat "${work}/out9" >&2 fi +# 10. a compact single-line releases response still resolves the newest tag. +printf '%s' '[{"tag_name":"ownership-snapshot-current"},{"tag_name":"v0.2.4","name":"v0.2.4"},{"tag_name":"v0.2.3"}]' \ + >"${api_dir}/releases" +dir="${work}/compact" +if run_installer "$test_shell" "$installer" --dir "$dir" >"${work}/out10" 2>&1 && + [[ "$("${dir}/idx")" == "idx fake v0.2.4" ]]; then + pass "resolves the newest tag from a compact releases response" +else + fail "resolves the newest tag from a compact releases response"; cat "${work}/out10" >&2 +fi + if ((failures > 0)); then echo "install.sh tests: ${failures} failure(s) with ${test_shell}" >&2 exit 1 From 9f714dfed2edc928c7e2d710af82a1c2bbb40b35 Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:20:46 +0000 Subject: [PATCH 26/27] fix(publisher): require the jq and bash it uses; pin CI actions From the Codex re-gate of the 0.3.3 fixes: NUL-separated discovery fields need jq 1.7 (--raw-output0) and bash 4.4 (mapfile -d); check both up front with a clear message instead of failing midway on older tools. ci.yml actions are pinned to commit SHAs like the release workflows. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- .github/workflows/ci.yml | 8 ++++---- scripts/build-latest-ownership-snapshot.sh | 10 ++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 91a4a52..e77d4d9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,13 +19,13 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Install Nix - uses: cachix/install-nix-action@v31 + uses: cachix/install-nix-action@448cde739abec1eaf0646249e3d25bb8d9bc7702 # v31 - name: Restore and save Nix store - uses: nix-community/cache-nix-action@v7 + uses: nix-community/cache-nix-action@7df957e333c1e5da7721f60227dbba6d06080569 # v7 with: primary-key: nix-v2-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('flake.nix', 'flake.lock', 'Cargo.lock', 'Cargo.toml') }}-${{ hashFiles('crates/**', 'scripts/**', 'install.sh', 'README.md', 'LICENSE') }} restore-prefixes-first-match: | @@ -47,7 +47,7 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - name: Test install.sh with sh run: scripts/install-sh-test.sh diff --git a/scripts/build-latest-ownership-snapshot.sh b/scripts/build-latest-ownership-snapshot.sh index 0641f25..c09fd75 100755 --- a/scripts/build-latest-ownership-snapshot.sh +++ b/scripts/build-latest-ownership-snapshot.sh @@ -126,6 +126,16 @@ if ! command -v jq >/dev/null 2>&1; then echo "jq is required for manifest/source metadata processing" >&2 exit 1 fi +# NUL-separated discovery fields need jq 1.7 (--raw-output0) and bash 4.4 +# (mapfile -d); the packaged publisher provides both. +if ! jq --raw-output0 -n '"x"' >/dev/null 2>&1; then + echo "jq 1.7 or newer is required (for --raw-output0); found $(jq --version 2>&1)" >&2 + exit 1 +fi +if ((BASH_VERSINFO[0] < 4 || (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 4))); then + echo "bash 4.4 or newer is required (for mapfile -d); found ${BASH_VERSION}" >&2 + exit 1 +fi if ! "$IDX_BIN" version >/dev/null 2>&1; then echo "failed to run idx binary: $IDX_BIN" >&2 From ebdba13a418793ca62222c7d8fb4eb8ecd57817f Mon Sep 17 00:00:00 2001 From: hermes Date: Sat, 17 Oct 2026 05:57:14 +0000 Subject: [PATCH 27/27] chore: release 0.3.3 Patch release for the full-repo audit of 0.3.2: ownership query correctness (changes, alias search, graph, investor lookup), snapshot sync safety, request and lock time limits, the CLI output contract and terminal-safe output, fundamental rating logic, publisher and installer robustness, and zip off a yanked release (#51). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01V3syn2fN2LNo5pvTSqc6eh --- Cargo.lock | 6 +++--- Cargo.toml | 4 ++-- README.md | 2 +- docs/INSTALL.md | 4 ++-- docs/NPM_DISTRIBUTION.md | 4 ++-- npm/idx-mcp/package.json | 2 +- package.json | 2 +- web/src/content/docs/start/install.mdx | 2 +- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8de2fe0..306781f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -882,7 +882,7 @@ dependencies = [ [[package]] name = "idx-cli" -version = "0.3.2" +version = "0.3.3" dependencies = [ "assert_cmd", "chrono", @@ -902,7 +902,7 @@ dependencies = [ [[package]] name = "idx-core" -version = "0.3.2" +version = "0.3.3" dependencies = [ "chrono", "clap", @@ -921,7 +921,7 @@ dependencies = [ [[package]] name = "idx-mcp" -version = "0.3.2" +version = "0.3.3" dependencies = [ "clap", "idx-core", diff --git a/Cargo.toml b/Cargo.toml index ff3f403..32dab2d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ resolver = "3" members = ["crates/idx-core", "crates/idx-cli", "crates/idx-mcp"] [workspace.package] -version = "0.3.2" +version = "0.3.3" edition = "2024" rust-version = "1.89" license = "MIT" @@ -11,7 +11,7 @@ repository = "https://github.com/0xrsydn/idx-cli" homepage = "https://idx.rasyidanaf.com" [workspace.dependencies] -idx-core = { path = "crates/idx-core", version = "=0.3.2" } +idx-core = { path = "crates/idx-core", version = "=0.3.3" } clap = { version = "4", features = ["derive"] } serde = { version = "1", features = ["derive"] } serde_json = { version = "1", features = ["raw_value"] } diff --git a/README.md b/README.md index 27089da..d00b12e 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | s Downloads the prebuilt `idx` binary for your platform from GitHub Releases, verifies it against the release `SHA256SUMS`, and installs it into `~/.local/bin` without sudo. Pin a version or change the directory with -`sh -s -- --version v0.3.2 --dir ~/bin`. See [docs/INSTALL.md](docs/INSTALL.md). +`sh -s -- --version v0.3.3 --dir ~/bin`. See [docs/INSTALL.md](docs/INSTALL.md). ### npm / npx diff --git a/docs/INSTALL.md b/docs/INSTALL.md index fee5306..a093c3c 100644 --- a/docs/INSTALL.md +++ b/docs/INSTALL.md @@ -11,7 +11,7 @@ Each release also carries a copy of the script as a release asset, so a versioned URL stays stable: ```bash -curl -fsSL https://github.com/0xrsydn/idx-cli/releases/download/v0.3.2/install.sh | sh +curl -fsSL https://github.com/0xrsydn/idx-cli/releases/download/v0.3.3/install.sh | sh ``` ## What it does @@ -45,7 +45,7 @@ them yourself. Pass flags through the pipe with `sh -s --`: ```bash -curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.2 --dir ~/bin +curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.3 --dir ~/bin ``` ## Supported platforms diff --git a/docs/NPM_DISTRIBUTION.md b/docs/NPM_DISTRIBUTION.md index e04345b..700e020 100644 --- a/docs/NPM_DISTRIBUTION.md +++ b/docs/NPM_DISTRIBUTION.md @@ -38,7 +38,7 @@ npx idx-cli --help ``` The package version and the release tag must match. For example, package -version `0.3.2` downloads assets from the `v0.3.2` GitHub Release. +version `0.3.3` downloads assets from the `v0.3.3` GitHub Release. The postinstall script downloads `SHA256SUMS` from the same release and refuses to install a binary whose checksum does not match. @@ -105,7 +105,7 @@ URLs must stay aligned with `scripts/install.js`. To attach binaries to a tag that already exists, run the workflow manually: ```bash -gh workflow run release.yml -f tag=v0.3.2 +gh workflow run release.yml -f tag=v0.3.3 ``` Pull requests that touch the workflow or the installer run the build jobs diff --git a/npm/idx-mcp/package.json b/npm/idx-mcp/package.json index 0617695..718d686 100644 --- a/npm/idx-mcp/package.json +++ b/npm/idx-mcp/package.json @@ -1,6 +1,6 @@ { "name": "idx-mcp", - "version": "0.3.2", + "version": "0.3.3", "description": "MCP server for Indonesian stock market (IDX) market, financial, and ownership data", "license": "MIT", "repository": { diff --git a/package.json b/package.json index 45e2df7..bc7967c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "idx-cli", - "version": "0.3.2", + "version": "0.3.3", "description": "CLI tool for Indonesian stock market (IDX) analysis", "license": "MIT", "repository": { diff --git a/web/src/content/docs/start/install.mdx b/web/src/content/docs/start/install.mdx index 6ea07a7..e8da2b4 100644 --- a/web/src/content/docs/start/install.mdx +++ b/web/src/content/docs/start/install.mdx @@ -40,7 +40,7 @@ against the release `SHA256SUMS`, and installs it into `~/.local/bin` without sudo. Pin a version or change the directory: ```bash -curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.2 --dir ~/bin +curl -fsSL https://raw.githubusercontent.com/0xrsydn/idx-cli/main/install.sh | sh -s -- --version v0.3.3 --dir ~/bin ```