diff --git a/crates/aionui-ai-agent/src/registry.rs b/crates/aionui-ai-agent/src/registry.rs index e1f8a54a2..d2831d124 100644 --- a/crates/aionui-ai-agent/src/registry.rs +++ b/crates/aionui-ai-agent/src/registry.rs @@ -1681,7 +1681,7 @@ mod tests { .unwrap_or_else(|error| panic!("missing release lock for {backend}: {error}")); locked += 1; } - assert_eq!(locked, 13); + assert_eq!(locked, 12); } /// On a host that has *none* of the seeded CLIs installed, the diff --git a/crates/aionui-db/migrations/039_omp_direct_cli_launch.sql b/crates/aionui-db/migrations/039_omp_direct_cli_launch.sql new file mode 100644 index 000000000..9319b52b3 --- /dev/null +++ b/crates/aionui-db/migrations/039_omp_direct_cli_launch.sql @@ -0,0 +1,40 @@ +-- omp launches its local CLI directly instead of bridging through npx. +-- +-- 031 seeded omp as `npx -y @oh-my-pi/pi-coding-agent acp` with +-- `binary_name: "omp"`, following the shape used by the Registry-listed npx +-- rows. That shape does not fit omp: +-- +-- * The Registry rows bridge because the ACP Registry declares an npx +-- distribution for them (verified against the public catalogue: 11 of the +-- 13 npx rows match their Registry entry's package and args exactly). omp +-- is NOT listed on the Registry, so there is no declared distribution to +-- conform to — the bridge was chosen by analogy, not by evidence. +-- * `@oh-my-pi/pi-coding-agent` is not an ACP adapter wrapping some other +-- CLI; it ships bin `omp` (package.json `bin: {omp: dist/cli.js}`) and +-- `omp acp` is the vendor's own entrypoint. The package IS the product. +-- * `binary_name: "omp"` already made a local `omp` on $PATH mandatory: +-- `probe_resolved_command` fails the row with `PrimaryMissing` without it, +-- and `cli_probe::validate_with_budget` already runs the local +-- `omp --version`. So the bridge re-downloaded a CLI the user had to have +-- installed before the row was even offered. +-- +-- The cost of that was measured, spawn to `initialize` response: 81.0s on a +-- cold npx cache and 10.0s warm, against 0.7s for the local binary. The cold +-- figure is from a fast link and exceeded the 30s handshake budget outright, +-- which is the failure reported in iOfficeAI/AionUi#4009. +-- +-- Written as an UPDATE rather than a re-seed on purpose: `agent_capabilities` +-- and `auth_methods` hold what a live handshake learned on THIS install, and +-- an `ON CONFLICT DO UPDATE` that lists them resets that to an integration-time +-- snapshot. An UPDATE of the launch columns cannot reach them at all. +-- +-- Version pinning moves with the launch path: the `acp-registry-npx-lock.json` +-- entry is dropped in the same change, so omp now tracks whatever the user has +-- installed, exactly as the other direct-CLI builtins (qwen, agy) do. +UPDATE agent_metadata +SET command = 'omp', + args = '["acp"]', + agent_source_info = '{"binary_name":"omp"}', + updated_at = unixepoch('now','subsec')*1000 +WHERE agent_source = 'builtin' + AND backend = 'omp'; diff --git a/crates/aionui-db/tests/omp_direct_cli_migration.rs b/crates/aionui-db/tests/omp_direct_cli_migration.rs new file mode 100644 index 000000000..ba07b7212 --- /dev/null +++ b/crates/aionui-db/tests/omp_direct_cli_migration.rs @@ -0,0 +1,79 @@ +//! omp launches its local CLI directly rather than through the npx bridge. +//! +//! omp is a non-Registry builtin, so there is no Registry-declared npx +//! distribution to conform to: `@oh-my-pi/pi-coding-agent` ships bin `omp`, +//! and `omp acp` is the vendor's own ACP entrypoint. The row already gated +//! availability on a local `omp` through `binary_name`, so bridging the spawn +//! through npx re-downloaded a CLI the user was required to have installed +//! before the row was even offered. + +use aionui_db::{IAgentMetadataRepository, SqliteAgentMetadataRepository, init_database_memory}; + +#[tokio::test] +async fn omp_spawns_its_local_cli_instead_of_bridging_through_npx() { + let db = init_database_memory().await.unwrap(); + let repo = SqliteAgentMetadataRepository::new(db.pool().clone()); + + let row = repo + .find_builtin_by_backend("omp") + .await + .unwrap() + .expect("omp is seeded"); + + assert_eq!(row.command.as_deref(), Some("omp"), "omp command"); + assert_eq!(row.args.as_deref(), Some(r#"["acp"]"#), "omp args"); + + let source: serde_json::Value = + serde_json::from_str(row.agent_source_info.as_deref().expect("omp agent_source_info")).unwrap(); + assert_eq!(source["binary_name"], "omp", "omp binary_name"); + assert!( + source.get("bridge_binary").is_none(), + "a direct-CLI row must not declare a bridge: {source}" + ); +} + +/// The re-seed must not reset what a live handshake taught this install. A +/// migration that lists `agent_capabilities` / `auth_methods` in its +/// `ON CONFLICT DO UPDATE` set is dead code on a fresh row and silent data +/// loss on an existing one, so the columns are asserted here rather than +/// trusted to review. +#[tokio::test] +async fn omp_keeps_its_probed_handshake_columns_and_skills_dirs() { + let db = init_database_memory().await.unwrap(); + let repo = SqliteAgentMetadataRepository::new(db.pool().clone()); + + let row = repo + .find_builtin_by_backend("omp") + .await + .unwrap() + .expect("omp is seeded"); + + assert_eq!( + row.native_skills_dirs.as_deref(), + Some(r#"[".omp/skills",".claude/skills"]"#), + "omp skills dirs" + ); + assert!( + row.agent_capabilities.is_some(), + "omp keeps the agent_capabilities its probe seeded" + ); + assert!( + row.auth_methods.is_some(), + "omp keeps the auth_methods its probe seeded" + ); + assert_eq!(row.yolo_id.as_deref(), None, "omp advertises no yolo mode"); +} + +/// The lock manifest pins npx packages. A direct-CLI row has no package to +/// pin, so leaving the entry behind would keep asserting a version nothing +/// launches. +#[test] +fn omp_is_no_longer_pinned_in_the_npx_release_lock() { + let lock = include_str!("../../aionui-runtime/resources/acp-registry-npx-lock.json"); + let parsed: serde_json::Value = serde_json::from_str(lock).unwrap(); + + assert!( + parsed["agents"].get("omp").is_none(), + "omp must not remain in the npx release lock once it launches directly" + ); +} diff --git a/crates/aionui-db/tests/registry_npx_agents_migration.rs b/crates/aionui-db/tests/registry_npx_agents_migration.rs index 994e99344..84f42b7dd 100644 --- a/crates/aionui-db/tests/registry_npx_agents_migration.rs +++ b/crates/aionui-db/tests/registry_npx_agents_migration.rs @@ -57,13 +57,9 @@ async fn verified_registry_npx_agents_use_stable_packages_and_conservative_team_ Some(r#"[".compass/skills"]"#), None, ), - ( - "omp", - "omp", - r#"["-y","@oh-my-pi/pi-coding-agent","acp"]"#, - Some(r#"[".omp/skills",".claude/skills"]"#), - None, - ), + // omp is deliberately absent: 039 moved it off the npx bridge to a + // direct `omp acp` launch. Its shape is asserted in + // `omp_direct_cli_migration.rs`. ("sigit", "sigit", r#"["-y","@smbcloud/sigit"]"#, None, None), ]; diff --git a/crates/aionui-db/tests/team_capability_criteria_migration.rs b/crates/aionui-db/tests/team_capability_criteria_migration.rs index f05a9fabc..30fa30d0f 100644 --- a/crates/aionui-db/tests/team_capability_criteria_migration.rs +++ b/crates/aionui-db/tests/team_capability_criteria_migration.rs @@ -148,8 +148,9 @@ async fn probed_registry_agents_carry_seeded_mcp_capabilities() { ("kilo", Some((true, true))), ("mimo-code", Some((true, true))), ("nova", Some((true, true))), - ("omp", Some((true, true))), ("sigit", Some((false, false))), + // direct CLI launch (031 seeded it on npx; 039 moved it off the bridge) + ("omp", Some((true, true))), // binary distributions (025) ("amp-acp", Some((true, true))), ("cortex-code", None), diff --git a/crates/aionui-runtime/resources/acp-registry-npx-lock.json b/crates/aionui-runtime/resources/acp-registry-npx-lock.json index 5cfc600a1..a19f96fa4 100644 --- a/crates/aionui-runtime/resources/acp-registry-npx-lock.json +++ b/crates/aionui-runtime/resources/acp-registry-npx-lock.json @@ -50,10 +50,6 @@ "package": "@compass-ai/nova", "version": "1.1.34" }, - "omp": { - "package": "@oh-my-pi/pi-coding-agent", - "version": "17.1.8" - }, "pi": { "registry_json_id": "pi-acp", "package": "pi-acp", diff --git a/crates/aionui-runtime/src/registry_npx_lock.rs b/crates/aionui-runtime/src/registry_npx_lock.rs index 20e91b1ce..e9c9adec3 100644 --- a/crates/aionui-runtime/src/registry_npx_lock.rs +++ b/crates/aionui-runtime/src/registry_npx_lock.rs @@ -130,7 +130,7 @@ mod tests { #[test] fn every_lock_entry_has_an_exact_version() { let lock = registry_npx_lock().unwrap(); - assert_eq!(lock.agents.len(), 13); + assert_eq!(lock.agents.len(), 12); for package in lock.agents.values() { assert!(semver::Version::parse(&package.version).is_ok()); }