From 7635080e4ffbca2aff9b48c9f594e0bf2815fe9c Mon Sep 17 00:00:00 2001 From: zk <> Date: Fri, 14 Aug 2026 18:20:09 +0800 Subject: [PATCH] fix(agents): launch omp through its local CLI instead of the npx bridge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migration 031 seeded omp as `npx -y @oh-my-pi/pi-coding-agent acp` with `binary_name: "omp"`, copying the shape the Registry-listed npx rows use. That shape does not fit omp, for two independent reasons. There is no Registry entry to conform to. Audited against the public ACP Registry catalogue, 11 of the 13 npx rows match their Registry entry's package and args exactly — for those, npx IS the vendor's declared ACP distribution and bridging is correct. omp and mimo-code are the only non-Registry builtins, so omp's bridge was chosen by analogy rather than from a declared distribution. And the package is not an adapter. `@oh-my-pi/pi-coding-agent` ships bin `omp`; `omp acp` is the vendor's own entrypoint, and the vendor also publishes standalone binaries. Meanwhile `binary_name: "omp"` already made a local `omp` on $PATH mandatory — `probe_resolved_command` fails the row with `PrimaryMissing` without one, 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. 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. 039 is written as an UPDATE of the launch columns rather than a re-seed: `agent_capabilities` and `auth_methods` hold what a live handshake learned on the user's install, and an `ON CONFLICT DO UPDATE` listing them resets that to an integration-time snapshot. An UPDATE cannot reach them at all. Version pinning follows the launch path off the bridge: the release-lock entry is dropped and the two lock-count assertions fall to 12, so omp now tracks the user's installed CLI exactly as qwen and agy do. The omp case leaves the npx migration test for a dedicated one, and the team-capability case moves out of that file's npx grouping. Refs iOfficeAI/AionUi#4009 --- crates/aionui-ai-agent/src/registry.rs | 2 +- .../migrations/039_omp_direct_cli_launch.sql | 40 ++++++++++ .../tests/omp_direct_cli_migration.rs | 79 +++++++++++++++++++ .../tests/registry_npx_agents_migration.rs | 10 +-- .../team_capability_criteria_migration.rs | 3 +- .../resources/acp-registry-npx-lock.json | 4 - .../aionui-runtime/src/registry_npx_lock.rs | 2 +- 7 files changed, 126 insertions(+), 14 deletions(-) create mode 100644 crates/aionui-db/migrations/039_omp_direct_cli_launch.sql create mode 100644 crates/aionui-db/tests/omp_direct_cli_migration.rs 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()); }