diff --git a/TODO.md b/TODO.md index af47eb4..f3b30bb 100644 --- a/TODO.md +++ b/TODO.md @@ -863,5 +863,14 @@ not obvious from any one of them. - [ ] **Report the MegaMek defects in `crates/helm-bv/UPSTREAM.md`.** One unit file so far, found by conforming against the bridge. Worth sending upstream rather than only carrying. -- [ ] **`helm-mcp` has no place to report a stale database.** If the `.sqlite` - was built from a different MegaMek than the one on disk, nothing says so. +- [x] **`helm-mcp` has no place to report a stale database.** It has one now: + pass the install beside the database - `helm-mcp helm.sqlite `, + or `HELM_MEGAMEK` - and a mismatch is warned about on startup and carried + in `list_facets` as `stale`, where an agent reading the library will see + it. Without the install it says nothing rather than guessing. + + A database built from one MegaMek and queried about another answers every + question and answers them about a library that is not there: designs + since added are missing and figures since corrected are the old ones. The + version was already recorded in both places; the comparison just had + nowhere to be made. diff --git a/crates/helm-mcp/src/library.rs b/crates/helm-mcp/src/library.rs index 668dec6..70f06d9 100644 --- a/crates/helm-mcp/src/library.rs +++ b/crates/helm-mcp/src/library.rs @@ -45,9 +45,34 @@ pub struct Library { pub quirk_counts: HashMap, pub megamek_version: String, pub stats_producer: String, + /// Set when the database was built from a different MegaMek than the one + /// on disk beside it. Everything still answers; the answers are just about + /// a library that is no longer there. + pub stale: Option, } impl Library { + /// Say whether this database was built from the MegaMek install it is + /// being served beside. + /// + /// A database built from one MegaMek and queried about another answers + /// every question and answers them about the wrong library: designs that + /// have since been added are missing, battle values that have since been + /// corrected are the old ones, and nothing anywhere says so. The version + /// is in both places, so the check is a string comparison - it just had + /// nowhere to be made. + pub fn check_against(&mut self, install: &std::path::Path) { + let Some(on_disk) = megamek_version(install) else { + return; + }; + if on_disk != self.megamek_version { + self.stale = Some(format!( + "built from MegaMek {} and serving beside {on_disk}: rebuild with `helm build`", + self.megamek_version + )); + } + } + pub fn open(path: &str) -> Result { let db = Connection::open(path).map_err(|e| format!("opening {path}: {e}"))?; @@ -186,6 +211,7 @@ impl Library { quirk_counts, megamek_version, stats_producer, + stale: None, }) } @@ -347,3 +373,15 @@ impl Library { }) } } + +/// The MegaMek version an install declares, from the first release line of its +/// own changelog. The same read `helm build` does, kept here because helm-mcp +/// deliberately does not depend on the unit file readers. +fn megamek_version(install: &std::path::Path) -> Option { + let text = std::fs::read_to_string(install.join("docs/history.txt")).ok()?; + text.lines() + .map(str::trim) + .find(|line| line.starts_with(|c: char| c.is_ascii_digit())) + .and_then(|line| line.split_whitespace().next()) + .map(str::to_string) +} diff --git a/crates/helm-mcp/src/main.rs b/crates/helm-mcp/src/main.rs index 0433def..0e1b0c7 100644 --- a/crates/helm-mcp/src/main.rs +++ b/crates/helm-mcp/src/main.rs @@ -1,6 +1,9 @@ //! An MCP server over helm's unit database. //! -//! helm-mcp (or set HELM_DB) +//! helm-mcp [megamek-install] +//! +//! Or set `HELM_DB` and `HELM_MEGAMEK`. The install is optional and is only +//! used to say whether the database was built from it. //! //! Scoped to BattleMeks. The tools answer from `helm-facet`'s predicate rather //! than from SQL, so what they return is what MegaMek's own filtering would @@ -209,7 +212,15 @@ async fn main() -> Result<(), Box> { .or_else(|| std::env::var("HELM_DB").ok()) .ok_or("usage: helm-mcp (or set HELM_DB)")?; - let lib = Library::open(&path)?; + let mut lib = Library::open(&path)?; + // Where the install is, if the caller says: a database built from another + // MegaMek answers every question about a library that is not there. + if let Some(install) = std::env::args() + .nth(2) + .or_else(|| std::env::var("HELM_MEGAMEK").ok()) + { + lib.check_against(std::path::Path::new(&install)); + } // stdout is the protocol channel, so progress goes to stderr or nowhere. eprintln!( "helm-mcp: {} Meks from MegaMek {} ({})", @@ -218,6 +229,10 @@ async fn main() -> Result<(), Box> { path ); + if let Some(stale) = &lib.stale { + eprintln!("helm-mcp: WARNING - {stale}"); + } + let service = Helm::new(Arc::new(lib)).serve(stdio()).await?; service.waiting().await?; Ok(()) diff --git a/crates/helm-mcp/src/tools.rs b/crates/helm-mcp/src/tools.rs index 32b71f4..888d243 100644 --- a/crates/helm-mcp/src/tools.rs +++ b/crates/helm-mcp/src/tools.rs @@ -374,6 +374,7 @@ pub fn list_facets(lib: &Library) -> Value { "scope": "BattleMeks only, canon and valid", "unit_count": lib.len(), "megamek_version": lib.megamek_version, + "stale": lib.stale, "computed_by": lib.stats_producer, "tech_base": facet_values(lib, |f| f.tech_base.clone()), "role": facet_values(lib, |f| f.role.clone()), diff --git a/crates/helm-mcp/tests/tools.rs b/crates/helm-mcp/tests/tools.rs index 967e3a6..1bf26a4 100644 --- a/crates/helm-mcp/tests/tools.rs +++ b/crates/helm-mcp/tests/tools.rs @@ -672,3 +672,33 @@ fn a_book_selects_the_designs_that_are_in_it() { .collect(); assert!(names.contains(&"TR:SW"), "{names:?}"); } + +// A database built from one MegaMek and served beside another answers every +// question about a library that is not there. The check is a string +// comparison; it just had nowhere to be made. +#[test] +#[ignore = "needs a built database; set HELM_DB"] +fn a_database_says_when_it_was_built_from_another_megamek() { + let mut lib = library(); + assert!(lib.stale.is_none(), "a fresh library is not stale"); + assert_eq!(tools::list_facets(&lib)["stale"], serde_json::Value::Null); + + // An install that is not there says nothing, rather than guessing. + lib.check_against(std::path::Path::new("/nonexistent")); + assert!(lib.stale.is_none()); + + // One that declares a different version says so, and says it where an + // agent reading list_facets will see it. + let older = std::env::temp_dir().join("helm-stale-check/docs"); + std::fs::create_dir_all(&older).unwrap(); + std::fs::write(older.join("history.txt"), "0.49.19 (2019-01-01)\n").unwrap(); + lib.check_against(older.parent().unwrap()); + let stale = lib.stale.clone().expect("a version mismatch is stale"); + assert!(stale.contains("0.49.19"), "{stale}"); + assert!( + tools::list_facets(&lib)["stale"] + .as_str() + .is_some_and(|s| s.contains("rebuild")), + "the warning has to reach the tool output" + ); +}