mirror of
https://github.com/alphaXiv/OpenResearch.git
synced 2026-10-02 01:34:34 +08:00
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 <noreply@anthropic.com>
* 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 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
4c3e1b84a1
commit
3f2d41264a
@@ -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
|
||||
|
||||
|
||||
+47
-3
@@ -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<String> {
|
||||
})
|
||||
}
|
||||
|
||||
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<String> {
|
||||
(!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<String>,
|
||||
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}");
|
||||
}
|
||||
}
|
||||
|
||||
+47
-1
@@ -678,6 +678,7 @@ fn router(state: AppState, remote_auth: Option<RemoteAuth>) -> 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<HarnessInstall> {
|
||||
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<String>) -> 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<AppState>,
|
||||
Query(q): Query<HarnessQuery>,
|
||||
@@ -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]
|
||||
|
||||
@@ -636,6 +636,11 @@ pub async fn detect_harness(id: &str) -> Option<HarnessInfo> {
|
||||
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<HarnessInfo> {
|
||||
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<HarnessInfo> {
|
||||
|
||||
Reference in New Issue
Block a user