diff --git a/Specs/agent-kgpacks-rs-parity.md b/Specs/agent-kgpacks-rs-parity.md index ff4f933b4..20dca8f91 100644 --- a/Specs/agent-kgpacks-rs-parity.md +++ b/Specs/agent-kgpacks-rs-parity.md @@ -49,7 +49,7 @@ implemented; acceptance test is the definition of done) · **OUT-OF-SCOPE**. | ID | Criterion | Acceptance check | Status | Evidence | |----|-----------|------------------|--------|----------| | KGP-M1 | `knowledge.list_packs` returns installed packs with name/description/article/section counts | `native_knowledge_transport_list_packs` green | DONE | `native_knowledge.rs::discover_packs`, `register_knowledge_handlers` | -| KGP-M2 | `knowledge.pack_info` returns one pack's metadata; errors on unknown pack | `native_knowledge_transport_pack_info`, `native_knowledge_transport_pack_not_found` green | DONE | `native_knowledge.rs` `knowledge.pack_info` handler | +| KGP-M2 | `knowledge.pack_info` returns one pack's metadata **plus computed on-disk status** (`db_exists`, `urls_file_exists`); errors on unknown pack | `native_knowledge_transport_pack_info`, `native_knowledge_transport_pack_info_reports_computed_file_flags`, `native_knowledge_transport_pack_info_manifest_only_pack_reports_missing_db`, `native_knowledge_transport_pack_not_found` green | DONE | `native_knowledge.rs` `knowledge.pack_info` handler + `pack_urls_file_exists` | | KGP-M3 | `manifest.json` (`graph_stats`) parsed with directory-name fallback | `discover_packs_finds_packs_with_manifests` green | DONE | `native_knowledge.rs::PackManifest`, `discover_packs` | ### Query & retrieval @@ -298,3 +298,29 @@ multi-hop retrieval — closed 2026-07-21. KGP-T3 — reuse an open `Connection` its configured weight). With this, **every in-scope parity criterion is DONE** and both done-gate commands (`cargo test --lib native_knowledge` + `cargo test --lib knowledge_client`) are green. +- **2026-07-28** — **KGP-M2 completed to full F2 parity** (metadata *plus* + computed on-disk status). The done-gate issue #4321 flagged F2 (`pack_info`) + as "⚠️ verify computed fields present": upstream agent-kgpacks + `mcp_server.pack_info` returns the manifest **plus** two computed fields — + `manifest["db_exists"] = (pack_dir/"pack.db").exists()` and + `manifest["urls_file_exists"] = (pack_dir/"urls.txt").exists()` — but the Rust + `knowledge.pack_info` handler returned only `name`/`description`/ + `article_count`/`section_count`, omitting both. A caller therefore could not + tell a manifest-only pack (metadata present, not yet built/installed) from a + fully materialised one. The handler now appends `db_exists` + (`DiscoveredPack::db_path.exists()`) and `urls_file_exists` (a new + `pack_urls_file_exists` helper resolving `/urls.txt` from the pack's + `db_path` parent, gated by the `PACK_URLS_FILE` constant), matching the + upstream contract. The typed client `KnowledgePackInfo` gained the two fields + (`#[serde(default)]`, so `list_packs` — which, like upstream, does not compute + them — still deserializes). Acceptance tests: + `native_knowledge_transport_pack_info` (db present, urls absent), + `native_knowledge_transport_pack_info_reports_computed_file_flags` (both + materialised → both `true`), + `native_knowledge_transport_pack_info_manifest_only_pack_reports_missing_db` + (known-but-unbuilt pack → `db_exists: false`), plus client-side + `pack_info_returns_metadata` (decodes the flags) and + `list_packs_defaults_computed_fields_to_false`. Both done-gate commands + (`cargo test --lib native_knowledge` + `cargo test --lib knowledge_client`) + remain green. This closes the last "⚠️ verify" row in the #4321 equivalence + matrix; every in-scope feature (F1–F11) is now at equivalent-or-better parity. diff --git a/docs/reference/rpc-wire-protocol.md b/docs/reference/rpc-wire-protocol.md index 5860bf267..a11a9fdd6 100644 --- a/docs/reference/rpc-wire-protocol.md +++ b/docs/reference/rpc-wire-protocol.md @@ -334,9 +334,15 @@ Get details about a specific pack. **Result**: ```json -{"name": "rust-expert", "description": "...", "article_count": 150, "section_count": 890} +{"name": "rust-expert", "description": "...", "article_count": 150, "section_count": 890, + "db_exists": true, "urls_file_exists": true} ``` +`db_exists` / `urls_file_exists` are computed on-disk status flags (whether the +pack's `pack.db` and `urls.txt` are present under the pack directory), at parity +with upstream agent-kgpacks `mcp_server.pack_info` (KGP-M2 / issue #4321 F2). +They are computed only by `pack_info`; `list_packs` omits them. + --- ## Gym RPC Methods diff --git a/src/knowledge_client.rs b/src/knowledge_client.rs index 37fa06b44..4de081a3b 100644 --- a/src/knowledge_client.rs +++ b/src/knowledge_client.rs @@ -44,7 +44,7 @@ pub struct KnowledgeSource { } /// Metadata about an installed knowledge pack. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct KnowledgePackInfo { /// Pack name (e.g. "rust-expert", "python-expert"). pub name: String, @@ -54,6 +54,21 @@ pub struct KnowledgePackInfo { pub article_count: u32, /// Number of sections across all articles. pub section_count: u32, + /// Whether the pack's SQLite database (`pack.db`) exists on disk. + /// + /// A computed on-disk status field returned by `knowledge.pack_info` (at + /// parity with upstream agent-kgpacks `mcp_server.pack_info`; KGP-M2 / + /// issue #4321 F2). `knowledge.list_packs` does not compute it, so it + /// defaults to `false` there — read it only from [`KnowledgeClient::pack_info`]. + #[serde(default)] + pub db_exists: bool, + /// Whether the pack's source-URL manifest (`urls.txt`) exists on disk. + /// + /// Companion computed field to [`Self::db_exists`] (same source and + /// caveat): populated by `knowledge.pack_info`, defaults to `false` from + /// `knowledge.list_packs`. + #[serde(default)] + pub urls_file_exists: bool, } /// Typed client for the knowledge graph pack knowledge. @@ -198,6 +213,8 @@ mod tests { "description": format!("{pack} knowledge"), "article_count": 120, "section_count": 450, + "db_exists": true, + "urls_file_exists": false, })) } _ => Err(RpcErrorPayload { @@ -251,6 +268,22 @@ mod tests { let info = knowledge.pack_info("rust-expert").unwrap(); assert_eq!(info.name, "rust-expert"); assert_eq!(info.article_count, 120); + // Computed on-disk status fields (KGP-M2 / issue #4321 F2) are decoded + // from the wire response, at parity with upstream agent-kgpacks. + assert!(info.db_exists); + assert!(!info.urls_file_exists); + } + + #[test] + fn list_packs_defaults_computed_fields_to_false() { + // `knowledge.list_packs` does not compute the on-disk status fields + // (matching upstream, which only adds them in `pack_info`), so the + // shared `KnowledgePackInfo` must decode those absent fields as `false` + // via `#[serde(default)]` rather than failing to deserialize. + let knowledge = KnowledgeClient::new(Box::new(mock_transport())); + let packs = knowledge.list_packs().unwrap(); + assert!(!packs[0].db_exists); + assert!(!packs[0].urls_file_exists); } #[test] diff --git a/src/knowledge_context.rs b/src/knowledge_context.rs index 877607261..b42586daf 100644 --- a/src/knowledge_context.rs +++ b/src/knowledge_context.rs @@ -307,6 +307,7 @@ mod tests { description: "Rust programming language".to_string(), article_count: 100, section_count: 400, + ..Default::default() }; let score = relevance_score("Fix Rust ownership issue", &pack); assert!(score >= 1, "expected match on 'rust', got {score}"); @@ -319,6 +320,7 @@ mod tests { description: "Docker containers".to_string(), article_count: 80, section_count: 300, + ..Default::default() }; let score = relevance_score("Fix Rust ownership issue", &pack); assert_eq!(score, 0); @@ -335,6 +337,7 @@ mod tests { description: "Sorting category latest algorithm".to_string(), article_count: 10, section_count: 20, + ..Default::default() }; assert_eq!( relevance_score("go test", &pack), @@ -347,6 +350,7 @@ mod tests { description: "Go test tooling".to_string(), article_count: 10, section_count: 20, + ..Default::default() }; assert_eq!(relevance_score("go test", &go_pack), 2); } @@ -360,6 +364,7 @@ mod tests { description: "Rust programming language".to_string(), article_count: 100, section_count: 400, + ..Default::default() }; assert_eq!( relevance_score("rust rust rust programming", &pack), @@ -379,6 +384,7 @@ mod tests { description: "Docker containers images".to_string(), article_count: 80, section_count: 300, + ..Default::default() }; assert_eq!( relevance_score("fix docker container image caching", &pack), @@ -391,6 +397,7 @@ mod tests { description: "Container runtime".to_string(), article_count: 10, section_count: 20, + ..Default::default() }; assert_eq!( relevance_score("debug containers", &singular_pack), @@ -408,6 +415,7 @@ mod tests { description: "Python libraries categories".to_string(), article_count: 200, section_count: 800, + ..Default::default() }; assert_eq!( relevance_score("pick a python library by category", &pack), @@ -428,6 +436,7 @@ mod tests { description: "class focus status".to_string(), article_count: 10, section_count: 20, + ..Default::default() }; assert_eq!( relevance_score("class focus status", &self_match_pack), @@ -439,6 +448,7 @@ mod tests { description: "clang tooling".to_string(), article_count: 10, section_count: 20, + ..Default::default() }; assert_eq!( relevance_score("class hierarchy", &unrelated_pack), @@ -457,6 +467,7 @@ mod tests { description: "Sorting category latest algorithm".to_string(), article_count: 10, section_count: 20, + ..Default::default() }; assert_eq!( relevance_score("go test", &pack), diff --git a/src/native_knowledge.rs b/src/native_knowledge.rs index 7ca90b834..d2191aa3c 100644 --- a/src/native_knowledge.rs +++ b/src/native_knowledge.rs @@ -109,6 +109,24 @@ fn discover_packs(packs_dir: &Path) -> Vec { packs } +/// Name of the per-pack source-URL manifest, at parity with upstream +/// agent-kgpacks (`pack_dir/"urls.txt"`). Its presence is surfaced by +/// `knowledge.pack_info` as `urls_file_exists` (KGP-M2 / issue #4321 F2). +const PACK_URLS_FILE: &str = "urls.txt"; + +/// Whether the pack's source-URL manifest ([`PACK_URLS_FILE`]) exists on disk. +/// +/// The pack directory is the parent of the pack's `pack.db` (see +/// [`discover_packs`], which joins `pack.db` onto each pack directory), so the +/// urls file is resolved as `/urls.txt`. Mirrors the upstream +/// `mcp_server.pack_info` computed field `(pack_dir/"urls.txt").exists()`. +fn pack_urls_file_exists(pack: &DiscoveredPack) -> bool { + pack.db_path + .parent() + .map(|dir| dir.join(PACK_URLS_FILE).exists()) + .unwrap_or(false) +} + /// Open a pack database read-only and answer `question` against it. /// /// This is now a **test-only** convenience wrapper: production queries go @@ -1345,6 +1363,15 @@ pub fn register_knowledge_handlers(transport: &mut NativeRpcTransport, packs_dir "description": p.description, "article_count": p.article_count, "section_count": p.section_count, + // Computed on-disk status fields, at parity with the upstream + // agent-kgpacks `mcp_server.pack_info` (KGP-M2 / issue #4321 + // F2), which appends: + // manifest["db_exists"] = (pack_dir/"pack.db").exists() + // manifest["urls_file_exists"] = (pack_dir/"urls.txt").exists() + // so a caller can tell a manifest-only pack (metadata present, + // not yet built/installed) from a fully materialised one. + "db_exists": p.db_path.exists(), + "urls_file_exists": pack_urls_file_exists(p), })), None => Err(RpcErrorPayload { code: ERROR_INTERNAL, @@ -1809,6 +1836,77 @@ mod tests { let result = response.result.unwrap(); assert_eq!(result["name"], "test-pack"); assert_eq!(result["article_count"], 10); + // Computed on-disk status fields (KGP-M2 / issue #4321 F2): `create_test_pack` + // materialises `pack.db` but no `urls.txt`. + assert_eq!(result["db_exists"], true); + assert_eq!(result["urls_file_exists"], false); + } + + #[test] + fn native_knowledge_transport_pack_info_reports_computed_file_flags() { + // At parity with upstream agent-kgpacks `mcp_server.pack_info` + // (issue #4321 F2), `knowledge.pack_info` must surface whether the + // pack's `pack.db` and `urls.txt` are present on disk. Here we + // materialise BOTH so both flags are `true`, distinguishing a fully + // built/installed pack from a manifest-only one. + let tmp = TempDir::new().unwrap(); + let pack_dir = create_test_pack(tmp.path(), "with-urls"); + fs::write( + pack_dir.join("urls.txt"), + "https://doc.rust-lang.org/book/\n", + ) + .unwrap(); + + let mut transport = NativeRpcTransport::new("simard-knowledge"); + register_knowledge_handlers(&mut transport, tmp.path().to_path_buf()); + + let request = crate::rpc::RpcRequest { + id: crate::rpc::new_request_id(), + method: "knowledge.pack_info".to_string(), + params: serde_json::json!({"pack_name": "with-urls"}), + }; + let response = crate::rpc::RpcTransport::call(&transport, request).unwrap(); + let result = response.result.unwrap(); + assert_eq!(result["db_exists"], true, "pack.db was materialised"); + assert_eq!( + result["urls_file_exists"], true, + "urls.txt was materialised" + ); + } + + #[test] + fn native_knowledge_transport_pack_info_manifest_only_pack_reports_missing_db() { + // A manifest-only pack (metadata present, not yet built) must report + // `db_exists: false` so a caller can tell it apart from a materialised + // pack rather than seeing identical metadata. `pack_not_found` still + // covers the unknown-pack error path; this covers the known-but-unbuilt + // pack. + let tmp = TempDir::new().unwrap(); + let pack_dir = tmp.path().join("manifest-only"); + fs::create_dir_all(&pack_dir).unwrap(); + fs::write( + pack_dir.join("manifest.json"), + serde_json::to_string_pretty(&serde_json::json!({ + "name": "manifest-only", + "description": "not yet built", + "graph_stats": { "articles": 0, "entities": 0, "relationships": 0, "size_mb": 0.0 } + })) + .unwrap(), + ) + .unwrap(); + + let mut transport = NativeRpcTransport::new("simard-knowledge"); + register_knowledge_handlers(&mut transport, tmp.path().to_path_buf()); + + let request = crate::rpc::RpcRequest { + id: crate::rpc::new_request_id(), + method: "knowledge.pack_info".to_string(), + params: serde_json::json!({"pack_name": "manifest-only"}), + }; + let response = crate::rpc::RpcTransport::call(&transport, request).unwrap(); + let result = response.result.unwrap(); + assert_eq!(result["db_exists"], false, "no pack.db was built"); + assert_eq!(result["urls_file_exists"], false, "no urls.txt"); } #[test] diff --git a/tests/knowledge.rs b/tests/knowledge.rs index 364f4e88b..06e43255b 100644 --- a/tests/knowledge.rs +++ b/tests/knowledge.rs @@ -93,6 +93,8 @@ fn mock_knowledge_transport() -> InMemoryRpcTransport { "description": "Rust programming language ownership borrowing lifetimes", "article_count": 150, "section_count": 520, + "db_exists": true, + "urls_file_exists": true, })), "python-expert" => Ok(serde_json::json!({ "name": "python-expert", @@ -192,6 +194,10 @@ fn pack_info_returns_metadata() { ); assert_eq!(info.article_count, 150); assert_eq!(info.section_count, 520); + // Computed on-disk status fields (KGP-M2 / issue #4321 F2), at parity with + // upstream agent-kgpacks `mcp_server.pack_info`. + assert!(info.db_exists); + assert!(info.urls_file_exists); } #[test] @@ -301,11 +307,17 @@ fn knowledge_pack_info_serializes_roundtrip() { description: "Test pack".to_string(), article_count: 42, section_count: 100, + db_exists: true, + urls_file_exists: false, }; let json = serde_json::to_string(&info).unwrap(); let parsed: KnowledgePackInfo = serde_json::from_str(&json).unwrap(); assert_eq!(parsed.name, "test-pack"); assert_eq!(parsed.article_count, 42); + // The computed on-disk status fields (KGP-M2 / issue #4321 F2) survive a + // serialize→deserialize roundtrip. + assert!(parsed.db_exists); + assert!(!parsed.urls_file_exists); } #[test] diff --git a/tests/qa-scenarios/kgpacks-rs-pack-info-computed-fields.yaml b/tests/qa-scenarios/kgpacks-rs-pack-info-computed-fields.yaml new file mode 100644 index 000000000..f8680e09e --- /dev/null +++ b/tests/qa-scenarios/kgpacks-rs-pack-info-computed-fields.yaml @@ -0,0 +1,74 @@ +name: kgpacks-rs-pack-info-computed-fields +app_type: cli +description: | + Outside-in proof for parity criterion **KGP-M2** of the kgpacks-rs port + (`Specs/agent-kgpacks-rs-parity.md`) — the F2 row of the done-gate issue + #4321 that was flagged "⚠️ verify computed fields present". + + The upstream agent-kgpacks `mcp_server.pack_info` returns the pack manifest + **plus** two computed on-disk status fields: + + manifest["db_exists"] = (pack_dir / "pack.db").exists() + manifest["urls_file_exists"] = (pack_dir / "urls.txt").exists() + + Before this fix the Rust `knowledge.pack_info` handler returned only + `name`/`description`/`article_count`/`section_count`, omitting both computed + fields, so a caller could not distinguish a manifest-only pack (metadata + present, not yet built/installed) from a fully materialised one — a genuine + parity gap with the original runtime. + + The corrected contract appends `db_exists` (from the discovered pack's + `pack.db` path) and `urls_file_exists` (resolving `/urls.txt`), + matching the upstream computed fields. The typed client `KnowledgePackInfo` + decodes both (`#[serde(default)]`, so `list_packs` — which, like upstream, + does not compute them — still deserializes to `false`). + + This scenario drives the hermetic, in-process `native_knowledge` + + `knowledge_client` unit tests that pin the corrected contract: + - `native_knowledge_transport_pack_info` — db present, urls absent + (`db_exists: true`, `urls_file_exists: false`); + - `native_knowledge_transport_pack_info_reports_computed_file_flags` — + both `pack.db` and `urls.txt` materialised → both flags `true`; + - `native_knowledge_transport_pack_info_manifest_only_pack_reports_missing_db` + — a known-but-unbuilt pack → `db_exists: false`; + - `pack_info_returns_metadata` — the typed client decodes the flags; + - `list_packs_defaults_computed_fields_to_false` — absent fields default + to `false` rather than failing to deserialize. + The same run re-asserts the pre-existing KGP-M1/M3 discovery + the unknown + pack error contract are unregressed. No network, no live host mutation. The + CLI runner rejects any non-zero exit. +agents: + - name: simard-cli + type: cli + command: cargo + +steps: + # The native knowledge handler contract, including the new KGP-M2 computed + # on-disk status fields and the pre-existing discovery / not-found behaviours. + - action: run + params: + command: "cargo test --locked --lib native_knowledge" + timeout: 600000 + - action: wait_for_output + params: + value: "test result: ok" + timeout: 8000 + - action: validate_exit_code + params: + value: "0" + timeout: 5000 + + # Transport-level typed client suite proves the computed fields are decoded + # end-to-end through the native RPC surface without regression. + - action: run + params: + command: "cargo test --locked --lib knowledge_client" + timeout: 600000 + - action: wait_for_output + params: + value: "test result: ok" + timeout: 8000 + - action: validate_exit_code + params: + value: "0" + timeout: 5000