From 3f2d41264a96abd85b6d0c93e805dc9ec2cc2b6b Mon Sep 17 00:00:00 2001 From: Myles Anderson <135627999+myles332@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:02:28 -0700 Subject: [PATCH] OR-340: Refuse agent spawns on a harness orx up cannot find (#474) * OR-340: Refuse agent spawns on a harness orx up cannot find `orx agent spawn` only queues the helper, so a `--harness` the resident server could not launch reported success and failed afterwards with "codex not found on PATH". Spawn now asks `orx up` (new `GET /api/harnesses/{id}/snapshot`, the server's own PATH) whether a switched-to harness is installed and refuses up front if not. Any failure to ask falls back to queueing as before. Co-Authored-By: Claude Opus 5.5 * OR-340: Accept the callback token on the spawn preflight On a persistent remote `orx up` host, agents authenticate with the callback token, which only covered run submission and cancellation, so the harness snapshot answered 401 and the preflight silently fell open. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- agent-skills/orx-agent-delegation/SKILL.md | 3 +- src/commands/agent.rs | 50 ++++++++++++++++++++-- src/commands/up.rs | 48 ++++++++++++++++++++- src/local/harness/mod.rs | 5 +++ 4 files changed, 101 insertions(+), 5 deletions(-) diff --git a/agent-skills/orx-agent-delegation/SKILL.md b/agent-skills/orx-agent-delegation/SKILL.md index 2cb99385..5fcbc4f0 100644 --- a/agent-skills/orx-agent-delegation/SKILL.md +++ b/agent-skills/orx-agent-delegation/SKILL.md @@ -20,7 +20,8 @@ By default, this chat resumes with the helper's closing reply. Use `--no-wake` only when no follow-up is needed. A spawned session cannot spawn another helper, and the CLI enforces the number of helpers a session may have in flight. If the command refuses a spawn for either reason, do the work here or wait for a helper -to finish. +to finish. It also refuses a `--harness` that OpenResearch cannot find installed; +spawn on this session's harness instead, or tell the user. ## Choose tasks with a clean boundary diff --git a/src/commands/agent.rs b/src/commands/agent.rs index 4f600024..69c0dfc9 100644 --- a/src/commands/agent.rs +++ b/src/commands/agent.rs @@ -35,7 +35,7 @@ pub async fn run(args: crate::AgentArgs) -> Result<()> { harness, model, no_wake, - } => spawn(&store, task, stdin, title, harness, model, !no_wake), + } => spawn(&store, task, stdin, title, harness, model, !no_wake).await, } } @@ -87,7 +87,34 @@ fn spawn_refusal(parent: &StoredChatSession, live: i64) -> Option { }) } -fn spawn( +/// Refuse a helper the resident `orx up` cannot launch: this command only queues +/// it, so otherwise the caller hears "spawned" and the failure lands later. +async fn ensure_installed(harness: &str) -> Result<()> { + // Any failure to ask falls back to queueing. + let Ok(Some(port)) = crate::local::chat::trusted_up_port() else { + return Ok(()); + }; + let Ok(install) = crate::commands::up::harness_install_via_up(port, harness).await else { + return Ok(()); + }; + match install_refusal(harness, &install) { + Some(refusal) => Err(anyhow!(refusal)), + None => Ok(()), + } +} + +fn install_refusal(harness: &str, install: &crate::commands::up::HarnessInstall) -> Option { + (!install.installed).then(|| { + format!( + "The OpenResearch server (the desktop app or `orx up`) cannot find {}, so no agent \ + was spawned. Spawn on this session's own harness by omitting `--harness {harness}`, \ + or ask the user to install it and restart OpenResearch.", + install.name + ) + }) +} + +async fn spawn( store: &Store, task: Option, stdin: bool, @@ -117,6 +144,10 @@ fn spawn( // Settings only carry over when the child runs the same harness; a model or // permission-mode id from one CLI is meaningless to another. let inherits = harness == parent.harness; + // The parent is already running on its own harness, so only a switch needs checking. + if !inherits { + ensure_installed(&harness).await?; + } let changes_model = model .as_deref() .is_some_and(|model| parent.model.as_deref() != Some(model)); @@ -181,7 +212,8 @@ fn spawn( #[cfg(test)] mod tests { - use super::{spawn_refusal, task_text, MAX_LIVE_SPAWNS}; + use super::{install_refusal, spawn_refusal, task_text, MAX_LIVE_SPAWNS}; + use crate::commands::up::HarnessInstall; use crate::store::StoredChatSession; fn parent(parent_session_id: Option<&str>) -> StoredChatSession { @@ -237,4 +269,16 @@ mod tests { // --stdin and a positional together are ambiguous, so neither is used. assert!(task_text(Some("from the args".into()), true).is_err()); } + + #[test] + fn install_refusal_only_for_a_missing_harness() { + let info = |installed| HarnessInstall { + name: "Codex".into(), + installed, + }; + assert_eq!(install_refusal("codex", &info(true)), None); + let missing = install_refusal("codex", &info(false)).unwrap(); + assert!(missing.contains("cannot find Codex"), "{missing}"); + assert!(missing.contains("`--harness codex`"), "{missing}"); + } } diff --git a/src/commands/up.rs b/src/commands/up.rs index 20b21324..46022cf5 100644 --- a/src/commands/up.rs +++ b/src/commands/up.rs @@ -678,6 +678,7 @@ fn router(state: AppState, remote_auth: Option) -> Router { get(lit_sources_settings).post(set_lit_sources_settings), ) .route("/api/harnesses", get(list_harnesses)) + .route("/api/harnesses/{id}/snapshot", get(harness_snapshot)) .route( "/api/harnesses/setup/commands", get(harness_setup::commands), @@ -819,6 +820,13 @@ fn remote_route_forbidden(path: &str) -> bool { } fn is_remote_callback_route(method: &Method, path: &str) -> bool { + if method == Method::GET { + // `orx agent spawn`'s install preflight (read-only). + return path + .strip_prefix("/api/harnesses/") + .and_then(|path| path.strip_suffix("/snapshot")) + .is_some_and(|id| !id.is_empty() && !id.contains('/')); + } if method != Method::POST { return false; } @@ -2117,6 +2125,24 @@ pub(crate) async fn submit_run_via_up( }) } +#[derive(Debug, Deserialize)] +pub(crate) struct HarnessInstall { + pub name: String, + pub installed: bool, +} + +/// `orx up`'s install evidence for `harness`, from its own PATH rather than the caller's. +pub(crate) async fn harness_install_via_up(port: u16, harness: &str) -> Result { + let response = authenticate_up_request(local_client()?.get(format!( + "http://127.0.0.1:{port}/api/harnesses/{harness}/snapshot" + ))) + .timeout(Duration::from_secs(10)) + .send() + .await + .map_err(|error| anyhow!("Could not reach the trusted orx up process: {error}"))?; + decode_local_response(response, "check the harness").await +} + pub(crate) async fn cancel_run_via_up(port: u16, run_id: &str) -> Result<()> { let response = authenticate_up_request( local_client()?.post(format!("http://127.0.0.1:{port}/api/runs/{run_id}/cancel")), @@ -6138,6 +6164,13 @@ fn replace_claude_entry(payload: &mut Value, replacement: Value) { } } +async fn harness_snapshot(Path(id): Path) -> ApiResult { + let info = local::harness::detect_harness_snapshot(&id) + .await + .ok_or_else(|| not_found("harness"))?; + Ok(Json(json!(info))) +} + async fn list_harnesses( State(state): State, Query(q): Query, @@ -7742,7 +7775,7 @@ mod tests { } #[test] - fn remote_callback_token_is_limited_to_run_submission_and_cancellation() { + fn remote_callback_token_is_limited_to_runs_and_the_spawn_preflight() { assert!(is_remote_callback_route(&Method::POST, "/api/runs")); assert!(is_remote_callback_route( &Method::POST, @@ -7757,6 +7790,19 @@ mod tests { &Method::POST, "/api/chat/sessions/s1/message" )); + assert!(is_remote_callback_route( + &Method::GET, + "/api/harnesses/codex/snapshot" + )); + assert!(!is_remote_callback_route( + &Method::POST, + "/api/harnesses/codex/snapshot" + )); + assert!(!is_remote_callback_route(&Method::GET, "/api/harnesses")); + assert!(!is_remote_callback_route( + &Method::GET, + "/api/harnesses/setup/commands" + )); } #[test] diff --git a/src/local/harness/mod.rs b/src/local/harness/mod.rs index da2c7b57..59bde559 100644 --- a/src/local/harness/mod.rs +++ b/src/local/harness/mod.rs @@ -636,6 +636,11 @@ pub async fn detect_harness(id: &str) -> Option { detect_one(harness.as_ref(), false).await } +/// The snapshot pass of [`detect_harness`] (see [`Harness::detect_snapshot`]). +pub async fn detect_harness_snapshot(id: &str) -> Option { + detect_one(chat_harness(id)?.as_ref(), true).await +} + /// Detect every chat-capable harness, in registry order. This is what the /// `orx up` dashboard renders in its harness picker. pub async fn detect_harnesses() -> Vec {