From 43611a3344d26b69831d3538dede9c2ea658ec8f Mon Sep 17 00:00:00 2001 From: Daniel Kim Date: Fri, 17 Jul 2026 18:07:30 -0700 Subject: [PATCH 1/2] Centralize local-vs-server control-plane dispatch in a resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commands used to hand-roll the "is this id a local project/run or a server one?" decision as a copy-pasted get_local_project / local_run fork across 14 files. Add local::resolve — ProjectRef/resolve_project and RunRef/resolve_run (which reuses local::local_run, since store membership alone is not enough: server HF runs also live in the runs table) — and route every dispatch site through it. Reject-guard sites use the is_local() guard form; dual-mode sites carry the fetched project through the Local arm, so no second store lookup. up.rs and supervise.rs keep their direct store calls deliberately: those are lookups and the cloud-mirror bridge, not ownership dispatch. Store::open_at(dir) lets the resolver's tests run against a throwaway temp-dir store without mutating the process-global ORX_DATA_DIR, which the localbox lifecycle test owns under the parallel test runner. Phase 1 of the control-plane separation plan; a later phase abstracts the per-arm bodies behind the resolver. --- src/commands/artifact.rs | 3 +- src/commands/artifacts.rs | 3 +- src/commands/chart.rs | 3 +- src/commands/create_experiment.rs | 3 +- src/commands/env.rs | 3 +- src/commands/exp.rs | 3 +- src/commands/experiments.rs | 3 +- src/commands/logs.rs | 3 +- src/commands/project.rs | 5 +- src/commands/query.rs | 3 +- src/commands/report.rs | 3 +- src/commands/runs.rs | 3 +- src/commands/search_logs.rs | 3 +- src/commands/wandb.rs | 3 +- src/local/mod.rs | 4 +- src/local/resolve.rs | 182 ++++++++++++++++++++++++++++++ src/store.rs | 9 +- 17 files changed, 222 insertions(+), 17 deletions(-) create mode 100644 src/local/resolve.rs diff --git a/src/commands/artifact.rs b/src/commands/artifact.rs index 74446d7..f78a478 100644 --- a/src/commands/artifact.rs +++ b/src/commands/artifact.rs @@ -9,10 +9,11 @@ use std::io::Write; use crate::client::read_artifact; use crate::error::require_credentials; use crate::error::{anyhow, Result}; +use crate::local::resolve::resolve_run; pub async fn run(args: crate::ArtifactArgs) -> Result<()> { let store = crate::store::Store::open()?; - if crate::local::local_run(&store, &args.run_id)?.is_some() { + if resolve_run(&store, &args.run_id)?.is_local() { return Err(crate::local::unsupported("artifact")); } let creds = require_credentials().await; diff --git a/src/commands/artifacts.rs b/src/commands/artifacts.rs index 97f6d15..84799d6 100644 --- a/src/commands/artifacts.rs +++ b/src/commands/artifacts.rs @@ -7,11 +7,12 @@ use crate::client::list_artifacts; use crate::error::require_credentials; use crate::error::Result; +use crate::local::resolve::resolve_run; use crate::output::print_table; pub async fn run(args: crate::ArtifactsArgs) -> Result<()> { let store = crate::store::Store::open()?; - if crate::local::local_run(&store, &args.run_id)?.is_some() { + if resolve_run(&store, &args.run_id)?.is_local() { return Err(crate::local::unsupported("artifacts")); } let creds = require_credentials().await; diff --git a/src/commands/chart.rs b/src/commands/chart.rs index c00fbe0..09037da 100644 --- a/src/commands/chart.rs +++ b/src/commands/chart.rs @@ -8,6 +8,7 @@ use std::path::PathBuf; use crate::client::{render_wandb_chart, WandbChartBody, WandbChartResult, WandbRunSpec}; use crate::error::{require_credentials, Result}; +use crate::local::resolve::resolve_project; const USAGE: &str = "Usage: orx chart wandb --metric \"\" --run [:label] [--run ...] [--smoothing <0-0.99>] [--out ]"; @@ -171,7 +172,7 @@ pub async fn run(args: crate::ChartArgs) -> Result<()> { } let store = crate::store::Store::open()?; - if store.get_local_project(&args.project_id)?.is_some() { + if resolve_project(&store, &args.project_id)?.is_local() { return Err(crate::local::unsupported("chart")); } let creds = require_credentials().await; diff --git a/src/commands/create_experiment.rs b/src/commands/create_experiment.rs index 7d3e667..f867a47 100644 --- a/src/commands/create_experiment.rs +++ b/src/commands/create_experiment.rs @@ -19,6 +19,7 @@ use crate::client::{ CreateChildBody, Experiment, }; use crate::error::{anyhow, require_credentials, Result}; +use crate::local::resolve::{resolve_project, ProjectRef}; use crate::store::Store; const USAGE: &str = "Usage: orx create-experiment --title \"\" [--parent <experimentId>] [--description \"<text>\"] [--run-command \"<cmd>\"]"; @@ -34,7 +35,7 @@ pub async fn run(args: crate::CreateExperimentArgs) -> Result<()> { // Local project (orx up): create the row + branch locally, no api. let store = Store::open()?; - if let Some(project) = store.get_local_project(&args.project_id)? { + if let ProjectRef::Local(project) = resolve_project(&store, &args.project_id)? { run_local( &store, &project, diff --git a/src/commands/env.rs b/src/commands/env.rs index a2b370e..0609b19 100644 --- a/src/commands/env.rs +++ b/src/commands/env.rs @@ -5,11 +5,12 @@ use crate::client::list_env_var_names; use crate::error::{require_credentials, Result}; +use crate::local::resolve::resolve_project; /// Lists a project's effective env var names, grouped by source. pub async fn run(args: crate::EnvArgs) -> Result<()> { let store = crate::store::Store::open()?; - if store.get_local_project(&args.project_id)?.is_some() { + if resolve_project(&store, &args.project_id)?.is_local() { return Err(crate::local::unsupported("env")); } let creds = require_credentials().await; diff --git a/src/commands/exp.rs b/src/commands/exp.rs index ce75eba..6e4e9c6 100644 --- a/src/commands/exp.rs +++ b/src/commands/exp.rs @@ -20,6 +20,7 @@ use crate::client::{ use crate::error::{anyhow, require_credentials, Result}; use crate::jobs::{huggingface as hf, BackendDescriptor}; use crate::local::model::LocalExperiment; +use crate::local::resolve::resolve_project; use crate::output::{format_duration, run_failure_detail}; use crate::store::{now_ms, Store, StoredRun}; use crate::{ExpCommand, ExpRunArgs}; @@ -112,7 +113,7 @@ async fn wait( wait_experiment(&creds, &exp_id, interval, deadline).await } (None, Some(project_id)) => { - if store.get_local_project(&project_id)?.is_some() { + if resolve_project(store, &project_id)?.is_local() { return local_wait_project(store, &project_id, interval, deadline).await; } let creds = require_credentials().await; diff --git a/src/commands/experiments.rs b/src/commands/experiments.rs index cb02498..9c3fed6 100644 --- a/src/commands/experiments.rs +++ b/src/commands/experiments.rs @@ -5,11 +5,12 @@ use std::collections::{HashMap, HashSet}; use crate::client::{list_experiments, Experiment}; use crate::error::{require_credentials, Result}; +use crate::local::resolve::resolve_project; /// Lists a project's experiments as an indented tree (by parentExperimentId). pub async fn run(args: crate::ExperimentsArgs) -> Result<()> { let store = crate::store::Store::open()?; - if store.get_local_project(&args.project_id)?.is_some() { + if resolve_project(&store, &args.project_id)?.is_local() { return Err(crate::local::unsupported("experiments")); } let creds = require_credentials().await; diff --git a/src/commands/logs.rs b/src/commands/logs.rs index 1eaf8f6..346ec3a 100644 --- a/src/commands/logs.rs +++ b/src/commands/logs.rs @@ -3,6 +3,7 @@ use std::io::{Read as _, Seek as _, Write}; use crate::client::read_run_log; use crate::error::require_credentials; use crate::error::Result; +use crate::local::resolve::resolve_run; use crate::store::{log_path, Store}; /// Parses a string the way JS `Number(s)` does for our purposes and returns it @@ -61,7 +62,7 @@ pub async fn run(args: crate::LogsArgs) -> Result<()> { // Local run (orx up): the log is a plain file beside the store — read it // directly, no api / login needed. let store = Store::open()?; - if crate::local::local_run(&store, &args.run_id)?.is_some() { + if resolve_run(&store, &args.run_id)?.is_local() { return run_local(&args.run_id, mode, max_bytes, start_byte, end_byte); } diff --git a/src/commands/project.rs b/src/commands/project.rs index b8a061f..c45ff5a 100644 --- a/src/commands/project.rs +++ b/src/commands/project.rs @@ -13,6 +13,7 @@ use crate::client::{ }; use crate::commands::experiments::print_tree; use crate::error::{anyhow, require_credentials, Result}; +use crate::local::resolve::{resolve_project, ProjectRef}; use crate::ProjectCommand; pub async fn run(args: crate::ProjectArgs) -> Result<()> { @@ -20,7 +21,7 @@ pub async fn run(args: crate::ProjectArgs) -> Result<()> { ProjectCommand::View { project_id } | ProjectCommand::Edit { project_id, .. } => project_id, }; let store = crate::store::Store::open()?; - if let Some(project) = store.get_local_project(project_id)? { + if let ProjectRef::Local(project) = resolve_project(&store, project_id)? { return match args.command { ProjectCommand::View { .. } => view_local(&store, &project), ProjectCommand::Edit { @@ -37,7 +38,7 @@ pub async fn run(args: crate::ProjectArgs) -> Result<()> { "Local projects support --name and --run-command only." )); } - edit_local(&store, project, name, run_command) + edit_local(&store, *project, name, run_command) } }; } diff --git a/src/commands/query.rs b/src/commands/query.rs index e2a392d..7ebb5bf 100644 --- a/src/commands/query.rs +++ b/src/commands/query.rs @@ -2,6 +2,7 @@ use crate::client::{query_project, SyncStatus}; use crate::error::{require_credentials, Result}; +use crate::local::resolve::resolve_project; use crate::output::{cell, print_table}; /// Renders the wire form of a sync status (matches the TS `"ready" | ...` union). @@ -17,7 +18,7 @@ pub async fn run(args: crate::QueryArgs) -> Result<()> { // clap enforces the required positionals, so the TS `if (!projectId || !sql)` // usage guard is unnecessary here. let store = crate::store::Store::open()?; - if store.get_local_project(&args.project_id)?.is_some() { + if resolve_project(&store, &args.project_id)?.is_local() { return Err(crate::local::unsupported("query")); } let creds = require_credentials().await; diff --git a/src/commands/report.rs b/src/commands/report.rs index e76053d..2a66191 100644 --- a/src/commands/report.rs +++ b/src/commands/report.rs @@ -14,6 +14,7 @@ use crate::client::{ }; use crate::config::Credentials; use crate::error::{anyhow, require_credentials, Result}; +use crate::local::resolve::{resolve_project, ProjectRef}; pub async fn run(args: crate::ReportArgs) -> Result<()> { let project_id = match &args.command { @@ -23,7 +24,7 @@ pub async fn run(args: crate::ReportArgs) -> Result<()> { | crate::ReportCommand::Download { project_id, .. } => project_id, }; let store = crate::store::Store::open()?; - if let Some(project) = store.get_local_project(project_id)? { + if let ProjectRef::Local(project) = resolve_project(&store, project_id)? { return local_guidance(&project); } match args.command { diff --git a/src/commands/runs.rs b/src/commands/runs.rs index e3d758e..76e677e 100644 --- a/src/commands/runs.rs +++ b/src/commands/runs.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; use crate::client::{list_experiments, list_runs}; use crate::error::{require_credentials, Result}; +use crate::local::resolve::resolve_project; use crate::output::{format_duration, print_table, run_failure_detail}; use crate::store::Store; @@ -11,7 +12,7 @@ use crate::store::Store; pub async fn run(args: crate::RunsArgs) -> Result<()> { // Local project (orx up): the store is the truth, no api / login needed. let store = Store::open()?; - if store.get_local_project(&args.project_id)?.is_some() { + if resolve_project(&store, &args.project_id)?.is_local() { return run_local(&store, &args); } diff --git a/src/commands/search_logs.rs b/src/commands/search_logs.rs index d5bf314..e23a3d9 100644 --- a/src/commands/search_logs.rs +++ b/src/commands/search_logs.rs @@ -4,6 +4,7 @@ use crate::client::{search_logs, SearchLogsBody}; use crate::error::{require_credentials, Result}; +use crate::local::resolve::resolve_project; pub async fn run(args: crate::SearchLogsArgs) -> Result<()> { if args.run.is_none() && args.experiment.is_none() { @@ -12,7 +13,7 @@ pub async fn run(args: crate::SearchLogsArgs) -> Result<()> { } let store = crate::store::Store::open()?; - if store.get_local_project(&args.project_id)?.is_some() { + if resolve_project(&store, &args.project_id)?.is_local() { return Err(crate::local::unsupported("search-logs")); } let creds = require_credentials().await; diff --git a/src/commands/wandb.rs b/src/commands/wandb.rs index 9690460..e9eb568 100644 --- a/src/commands/wandb.rs +++ b/src/commands/wandb.rs @@ -7,10 +7,11 @@ use crate::client::list_wandb_runs; use crate::error::require_credentials; use crate::error::Result; +use crate::local::resolve::resolve_run; pub async fn run(args: crate::WandbArgs) -> Result<()> { let store = crate::store::Store::open()?; - if crate::local::local_run(&store, &args.run_id)?.is_some() { + if resolve_run(&store, &args.run_id)?.is_local() { return Err(crate::local::unsupported("wandb")); } let creds = require_credentials().await; diff --git a/src/local/mod.rs b/src/local/mod.rs index e42091d..c1d3b3c 100644 --- a/src/local/mod.rs +++ b/src/local/mod.rs @@ -6,7 +6,8 @@ //! //! Detection rule: an experiment/run is "local" iff its experiment id exists //! in `local_experiments`. CLI commands check the local store FIRST and only -//! require credentials on the server path. +//! require credentials on the server path — dispatch on it via +//! `resolve::{resolve_project, resolve_run}`, never by hand. pub mod agent_skills; pub mod chat; @@ -25,6 +26,7 @@ pub mod model; pub mod opencode; pub mod openresearch; pub mod projects; +pub mod resolve; pub mod skills; pub mod slurm; pub mod ssh; diff --git a/src/local/resolve.rs b/src/local/resolve.rs new file mode 100644 index 0000000..ebd1b90 --- /dev/null +++ b/src/local/resolve.rs @@ -0,0 +1,182 @@ +//! Control-plane resolver — the single place that answers "does this id belong +//! to the local store or the server (cloud) api?". +//! +//! The detection rule is documented in `local/mod.rs`: an experiment/run is +//! "local" iff its experiment id exists in `local_experiments`. Every dual-mode +//! and reject-guard command dispatches through here so the rule lives in one +//! place. Note that store membership alone is NOT enough for runs — server-mode +//! HF runs also live in the `runs` table — so `resolve_run` reuses the existing +//! `local::local_run` correctness test rather than re-deriving it. + +use crate::error::Result; +use crate::local::model::LocalProject; +use crate::store::{Store, StoredRun}; + +/// Which control plane owns a given project id. +pub enum ProjectRef { + /// The id resolves to a row in `local_projects`; the fetched project is + /// carried through so the local arm needs no second store lookup. Boxed so + /// the enum isn't dominated by `LocalProject`'s size (the common arm is + /// `Server`). + Local(Box<LocalProject>), + /// The id is not local; treat it as a server (api) project. + Server(String), +} + +/// Which control plane owns a given run id. +pub enum RunRef { + /// The run belongs to a local experiment (per `local::local_run`); the + /// fetched run is carried through for call sites that need it. Boxed so the + /// enum isn't dominated by `StoredRun`'s size (the common arm is `Server`). + Local(Box<StoredRun>), + /// The run is not local (or does not exist locally); treat it as a server run. + Server(String), +} + +impl ProjectRef { + /// Guard form for reject-only call sites that don't consume the project. + pub fn is_local(&self) -> bool { + matches!(self, ProjectRef::Local(_)) + } +} + +impl RunRef { + /// Guard form for reject-only call sites that don't consume the run. + pub fn is_local(&self) -> bool { + matches!(self, RunRef::Local(_)) + } +} + +/// Decide once: a project id is local iff it names a known local project. +pub fn resolve_project(store: &Store, project_id: &str) -> Result<ProjectRef> { + match store.get_local_project(project_id)? { + Some(p) => Ok(ProjectRef::Local(Box::new(p))), + None => Ok(ProjectRef::Server(project_id.to_string())), + } +} + +/// Decide once for a run id. Reuses `local::local_run`, which encodes the +/// correct test (the run's experiment must be in `local_experiments`) — store +/// membership alone is not enough, since server HF runs also live in `runs`. +pub fn resolve_run(store: &Store, run_id: &str) -> Result<RunRef> { + match crate::local::local_run(store, run_id)? { + Some(run) => Ok(RunRef::Local(Box::new(run))), + None => Ok(RunRef::Server(run_id.to_string())), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::local::model::{LocalExperiment, LocalProject}; + use crate::store::{now_ms, StoredRun}; + + /// A fresh, throwaway store rooted at a unique temp dir. Opened via + /// `Store::open_at` — never by mutating `$ORX_DATA_DIR`, which is + /// process-global and owned by the localbox lifecycle test. + fn temp_store() -> Store { + let dir = std::env::temp_dir().join(format!("orx-resolve-{}", uuid::Uuid::new_v4())); + Store::open_at(dir).expect("open temp store") + } + + fn project(id: &str) -> LocalProject { + let now = now_ms(); + LocalProject { + id: id.to_string(), + name: "P".to_string(), + slug: format!("slug-{id}"), + github_owner: "o".to_string(), + github_repo: "r".to_string(), + baseline_branch: "main".to_string(), + repo_path: "/tmp/repo".to_string(), + run_command: None, + paper_id: None, + created_at: now, + updated_at: now, + } + } + + fn experiment(id: &str, project_id: &str) -> LocalExperiment { + let now = now_ms(); + LocalExperiment { + id: id.to_string(), + project_id: project_id.to_string(), + parent_experiment_id: None, + slug: format!("exp-{id}"), + branch_name: format!("orx/{id}"), + title: None, + description: None, + run_command: "echo hi".to_string(), + agent_status: "idle".to_string(), + created_at: now, + updated_at: now, + } + } + + fn run(id: &str, experiment_id: &str, project_id: &str) -> StoredRun { + let now = now_ms(); + StoredRun { + id: id.to_string(), + experiment_id: experiment_id.to_string(), + project_id: project_id.to_string(), + status: "running".to_string(), + backend_json: "{}".to_string(), + command: "echo hi".to_string(), + created_at: now, + updated_at: now, + ended_at: None, + exit_code: None, + commit_sha: None, + result_markdown: None, + cancel_requested: false, + } + } + + #[test] + fn local_project_id_resolves_local() { + let store = temp_store(); + store.create_local_project(&project("p1")).unwrap(); + match resolve_project(&store, "p1").unwrap() { + ProjectRef::Local(p) => assert_eq!(p.id, "p1"), + ProjectRef::Server(_) => panic!("known local project must resolve Local"), + } + } + + #[test] + fn unknown_project_id_resolves_server() { + let store = temp_store(); + match resolve_project(&store, "nope").unwrap() { + ProjectRef::Server(id) => assert_eq!(id, "nope"), + ProjectRef::Local(_) => panic!("unknown id must resolve Server"), + } + } + + #[test] + fn server_hf_run_in_runs_table_resolves_server() { + // Regression guard: a server-mode HF run lands in the `runs` table but + // its experiment is NOT in `local_experiments`. Store membership alone + // must not make it "local". + let store = temp_store(); + store + .upsert_run(&run("r1", "exp-not-local", "srv-project")) + .unwrap(); + match resolve_run(&store, "r1").unwrap() { + RunRef::Server(id) => assert_eq!(id, "r1"), + RunRef::Local(_) => panic!("run without a local experiment must resolve Server"), + } + } + + #[test] + fn run_of_local_experiment_resolves_local() { + let store = temp_store(); + store.create_local_project(&project("p1")).unwrap(); + store + .create_local_experiment(&experiment("e1", "p1")) + .unwrap(); + store.upsert_run(&run("r1", "e1", "p1")).unwrap(); + match resolve_run(&store, "r1").unwrap() { + RunRef::Local(r) => assert_eq!(r.id, "r1"), + RunRef::Server(_) => panic!("run of a local experiment must resolve Local"), + } + } +} diff --git a/src/store.rs b/src/store.rs index 39b4d31..c22aa04 100644 --- a/src/store.rs +++ b/src/store.rs @@ -154,7 +154,14 @@ impl Store { /// Open (creating dirs/schema as needed). WAL so the supervise writers and /// the serve readers never block each other. pub fn open() -> Result<Self> { - let dir = data_dir(); + Self::open_at(data_dir()) + } + + /// Open a store rooted at an explicit directory, bypassing `data_dir()` + /// resolution. For tests: a throwaway temp dir here avoids mutating the + /// process-global `$ORX_DATA_DIR`, which the localbox lifecycle test owns + /// (tests in different modules share env under the parallel runner). + pub fn open_at(dir: PathBuf) -> Result<Self> { std::fs::create_dir_all(dir.join("run-logs")) .map_err(|e| anyhow!("Could not create {}: {}", dir.display(), e))?; let conn = Connection::open(dir.join("orx.db"))?; From 833d80449310b7d3b6d121a41ab7fb76472bfae5 Mon Sep 17 00:00:00 2001 From: Daniel Kim <sox8502@gmail.com> Date: Fri, 17 Jul 2026 19:07:04 -0700 Subject: [PATCH 2/2] Phase 2: total-match dispatch + ExperimentRef for dual-mode commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert the six dual-mode commands (exp, runs, logs, project, report, create-experiment) from local-early-return guards to a uniform total match over the resolver, with server bodies extracted into named run_server-style fns — the mechanical hoist target for the later ControlPlane trait. Adds ExperimentRef/resolve_experiment (keyed on local_experiments) and routes exp.rs's five experiment-scoped dispatch sites plus both wait branches through it. Bodies are unchanged; credentials stay inside the server arms so local mode keeps working logged-out. Verified end-to-end against an isolated data dir + config (local arms serve from the store; server arms stop at the login check without touching the api). --- src/commands/create_experiment.rs | 36 +++++++------ src/commands/exp.rs | 88 +++++++++++++++++-------------- src/commands/logs.rs | 26 ++++----- src/commands/project.rs | 11 ++-- src/commands/report.rs | 9 +++- src/commands/runs.rs | 10 ++-- src/local/mod.rs | 2 +- src/local/resolve.rs | 50 ++++++++++++++++-- 8 files changed, 151 insertions(+), 81 deletions(-) diff --git a/src/commands/create_experiment.rs b/src/commands/create_experiment.rs index f867a47..3307ccd 100644 --- a/src/commands/create_experiment.rs +++ b/src/commands/create_experiment.rs @@ -24,8 +24,8 @@ use crate::store::Store; const USAGE: &str = "Usage: orx create-experiment <projectId> --title \"<title>\" [--parent <experimentId>] [--description \"<text>\"] [--run-command \"<cmd>\"]"; -pub async fn run(args: crate::CreateExperimentArgs) -> Result<()> { - let title = match args.title { +pub async fn run(mut args: crate::CreateExperimentArgs) -> Result<()> { + let title = match args.title.take() { Some(t) => t, None => { eprintln!("{}", USAGE); @@ -35,21 +35,27 @@ pub async fn run(args: crate::CreateExperimentArgs) -> Result<()> { // Local project (orx up): create the row + branch locally, no api. let store = Store::open()?; - if let ProjectRef::Local(project) = resolve_project(&store, &args.project_id)? { - run_local( - &store, - &project, - title, - args.parent, - args.baseline, - args.description, - args.run_command, - )?; - // Key event, fired only on success. Coarse props only — no ids/names. - crate::telemetry::capture_experiment_started("create", true, None); - return Ok(()); + match resolve_project(&store, &args.project_id)? { + ProjectRef::Local(project) => { + run_local( + &store, + &project, + title, + args.parent, + args.baseline, + args.description, + args.run_command, + )?; + // Key event, fired only on success. Coarse props only — no ids/names. + crate::telemetry::capture_experiment_started("create", true, None); + Ok(()) + } + ProjectRef::Server(_) => run_server(args, title).await, } +} +/// Server-mode create via the api. +async fn run_server(args: crate::CreateExperimentArgs, title: String) -> Result<()> { // The server child-create API carries no run command field — refuse rather // than silently drop it. (The baseline create below does accept one.) if args.run_command.is_some() && args.parent.is_some() { diff --git a/src/commands/exp.rs b/src/commands/exp.rs index 6e4e9c6..db4a7d3 100644 --- a/src/commands/exp.rs +++ b/src/commands/exp.rs @@ -20,7 +20,7 @@ use crate::client::{ use crate::error::{anyhow, require_credentials, Result}; use crate::jobs::{huggingface as hf, BackendDescriptor}; use crate::local::model::LocalExperiment; -use crate::local::resolve::resolve_project; +use crate::local::resolve::{resolve_experiment, resolve_project, ExperimentRef, ProjectRef}; use crate::output::{format_duration, run_failure_detail}; use crate::store::{now_ms, Store, StoredRun}; use crate::{ExpCommand, ExpRunArgs}; @@ -31,42 +31,44 @@ pub async fn run(args: crate::ExpArgs) -> Result<()> { // user may never have logged in). let store = Store::open()?; match args.command { - ExpCommand::Status { exp_id } => { - if let Some(exp) = store.get_local_experiment(&exp_id)? { - return local_status(&store, &exp); + ExpCommand::Status { exp_id } => match resolve_experiment(&store, &exp_id)? { + ExperimentRef::Local(exp) => local_status(&store, &exp), + ExperimentRef::Server(exp_id) => { + let creds = require_credentials().await; + status(&creds, &exp_id).await } - let creds = require_credentials().await; - status(&creds, &exp_id).await - } - ExpCommand::Cmd { exp_id, set } => { - if store.get_local_experiment(&exp_id)?.is_some() { - return Err(crate::local::unsupported("exp cmd")); + }, + ExpCommand::Cmd { exp_id, set } => match resolve_experiment(&store, &exp_id)? { + ExperimentRef::Local(_) => Err(crate::local::unsupported("exp cmd")), + ExperimentRef::Server(exp_id) => { + let creds = require_credentials().await; + cmd(&creds, &exp_id, set).await } - let creds = require_credentials().await; - cmd(&creds, &exp_id, set).await - } - ExpCommand::Desc { exp_id, set, stdin } => { - if let Some(exp) = store.get_local_experiment(&exp_id)? { - return local_desc(&store, exp, set, stdin).await; + }, + ExpCommand::Desc { exp_id, set, stdin } => match resolve_experiment(&store, &exp_id)? { + ExperimentRef::Local(exp) => local_desc(&store, *exp, set, stdin).await, + ExperimentRef::Server(exp_id) => { + let creds = require_credentials().await; + desc(&creds, &exp_id, set, stdin).await } - let creds = require_credentials().await; - desc(&creds, &exp_id, set, stdin).await - } + }, ExpCommand::Run(run_args) => { let run_args = *run_args; - if store.get_local_experiment(&run_args.exp_id)?.is_some() { - return local_launch(run_args).await; + match resolve_experiment(&store, &run_args.exp_id)? { + ExperimentRef::Local(_) => local_launch(run_args).await, + ExperimentRef::Server(_) => { + let creds = require_credentials().await; + launch(&creds, run_args).await + } } - let creds = require_credentials().await; - launch(&creds, run_args).await } - ExpCommand::Cancel { exp_id } => { - if let Some(exp) = store.get_local_experiment(&exp_id)? { - return local_cancel(&store, &exp); + ExpCommand::Cancel { exp_id } => match resolve_experiment(&store, &exp_id)? { + ExperimentRef::Local(exp) => local_cancel(&store, &exp), + ExperimentRef::Server(exp_id) => { + let creds = require_credentials().await; + cancel(&creds, &exp_id).await } - let creds = require_credentials().await; - cancel(&creds, &exp_id).await - } + }, ExpCommand::Wait { exp_id, project, @@ -105,20 +107,24 @@ async fn wait( "Specify what to wait on: `orx exp wait <expId>` (one run) or \ `orx exp wait --project <projectId>` (any run in a project)." )), - (Some(exp_id), None) => { - if store.get_local_experiment(&exp_id)?.is_some() { - return local_wait_experiment(store, &exp_id, interval, deadline).await; + (Some(exp_id), None) => match resolve_experiment(store, &exp_id)? { + ExperimentRef::Local(_) => { + local_wait_experiment(store, &exp_id, interval, deadline).await } - let creds = require_credentials().await; - wait_experiment(&creds, &exp_id, interval, deadline).await - } - (None, Some(project_id)) => { - if resolve_project(store, &project_id)?.is_local() { - return local_wait_project(store, &project_id, interval, deadline).await; + ExperimentRef::Server(exp_id) => { + let creds = require_credentials().await; + wait_experiment(&creds, &exp_id, interval, deadline).await } - let creds = require_credentials().await; - wait_project(&creds, &project_id, interval, deadline).await - } + }, + (None, Some(project_id)) => match resolve_project(store, &project_id)? { + ProjectRef::Local(_) => { + local_wait_project(store, &project_id, interval, deadline).await + } + ProjectRef::Server(project_id) => { + let creds = require_credentials().await; + wait_project(&creds, &project_id, interval, deadline).await + } + }, } } diff --git a/src/commands/logs.rs b/src/commands/logs.rs index 346ec3a..76b13ed 100644 --- a/src/commands/logs.rs +++ b/src/commands/logs.rs @@ -3,7 +3,7 @@ use std::io::{Read as _, Seek as _, Write}; use crate::client::read_run_log; use crate::error::require_credentials; use crate::error::Result; -use crate::local::resolve::resolve_run; +use crate::local::resolve::{resolve_run, RunRef}; use crate::store::{log_path, Store}; /// Parses a string the way JS `Number(s)` does for our purposes and returns it @@ -62,21 +62,23 @@ pub async fn run(args: crate::LogsArgs) -> Result<()> { // Local run (orx up): the log is a plain file beside the store — read it // directly, no api / login needed. let store = Store::open()?; - if resolve_run(&store, &args.run_id)?.is_local() { - return run_local(&args.run_id, mode, max_bytes, start_byte, end_byte); + match resolve_run(&store, &args.run_id)? { + RunRef::Local(_) => run_local(&args.run_id, mode, max_bytes, start_byte, end_byte), + RunRef::Server(_) => run_server(&args.run_id, mode, max_bytes, start_byte, end_byte).await, } +} +/// Server-mode log read via the api. +async fn run_server( + run_id: &str, + mode: &str, + max_bytes: Option<i64>, + start_byte: Option<i64>, + end_byte: Option<i64>, +) -> Result<()> { let creds = require_credentials().await; - let log = read_run_log( - &creds, - &args.run_id, - Some(mode), - max_bytes, - start_byte, - end_byte, - ) - .await?; + let log = read_run_log(&creds, run_id, Some(mode), max_bytes, start_byte, end_byte).await?; // The log itself goes to stdout (pipe-friendly); metadata to stderr so it // doesn't pollute a `| grep` or a redirect. diff --git a/src/commands/project.rs b/src/commands/project.rs index c45ff5a..14049cd 100644 --- a/src/commands/project.rs +++ b/src/commands/project.rs @@ -21,8 +21,8 @@ pub async fn run(args: crate::ProjectArgs) -> Result<()> { ProjectCommand::View { project_id } | ProjectCommand::Edit { project_id, .. } => project_id, }; let store = crate::store::Store::open()?; - if let ProjectRef::Local(project) = resolve_project(&store, project_id)? { - return match args.command { + match resolve_project(&store, project_id)? { + ProjectRef::Local(project) => match args.command { ProjectCommand::View { .. } => view_local(&store, &project), ProjectCommand::Edit { name, @@ -40,8 +40,13 @@ pub async fn run(args: crate::ProjectArgs) -> Result<()> { } edit_local(&store, *project, name, run_command) } - }; + }, + ProjectRef::Server(_) => run_server(args).await, } +} + +/// Server-mode `orx project` via the api. +async fn run_server(args: crate::ProjectArgs) -> Result<()> { // The server project PATCH carries no run command field — refuse before // even asking for credentials. if let ProjectCommand::Edit { diff --git a/src/commands/report.rs b/src/commands/report.rs index 2a66191..58767e5 100644 --- a/src/commands/report.rs +++ b/src/commands/report.rs @@ -24,9 +24,14 @@ pub async fn run(args: crate::ReportArgs) -> Result<()> { | crate::ReportCommand::Download { project_id, .. } => project_id, }; let store = crate::store::Store::open()?; - if let ProjectRef::Local(project) = resolve_project(&store, project_id)? { - return local_guidance(&project); + match resolve_project(&store, project_id)? { + ProjectRef::Local(project) => local_guidance(&project), + ProjectRef::Server(_) => run_server(args).await, } +} + +/// Server-mode `orx report` via the api. +async fn run_server(args: crate::ReportArgs) -> Result<()> { match args.command { crate::ReportCommand::Upload { project_id, diff --git a/src/commands/runs.rs b/src/commands/runs.rs index 76e677e..f2fd2dc 100644 --- a/src/commands/runs.rs +++ b/src/commands/runs.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use crate::client::{list_experiments, list_runs}; use crate::error::{require_credentials, Result}; -use crate::local::resolve::resolve_project; +use crate::local::resolve::{resolve_project, ProjectRef}; use crate::output::{format_duration, print_table, run_failure_detail}; use crate::store::Store; @@ -12,10 +12,14 @@ use crate::store::Store; pub async fn run(args: crate::RunsArgs) -> Result<()> { // Local project (orx up): the store is the truth, no api / login needed. let store = Store::open()?; - if resolve_project(&store, &args.project_id)?.is_local() { - return run_local(&store, &args); + match resolve_project(&store, &args.project_id)? { + ProjectRef::Local(_) => run_local(&store, &args), + ProjectRef::Server(_) => run_server(&args).await, } +} +/// Server-mode listing from the api. +async fn run_server(args: &crate::RunsArgs) -> Result<()> { let creds = require_credentials().await; // Fetch experiments too, so we can label each run with its experiment title diff --git a/src/local/mod.rs b/src/local/mod.rs index c1d3b3c..a144fa0 100644 --- a/src/local/mod.rs +++ b/src/local/mod.rs @@ -7,7 +7,7 @@ //! Detection rule: an experiment/run is "local" iff its experiment id exists //! in `local_experiments`. CLI commands check the local store FIRST and only //! require credentials on the server path — dispatch on it via -//! `resolve::{resolve_project, resolve_run}`, never by hand. +//! `resolve::{resolve_project, resolve_experiment, resolve_run}`, never by hand. pub mod agent_skills; pub mod chat; diff --git a/src/local/resolve.rs b/src/local/resolve.rs index ebd1b90..170955f 100644 --- a/src/local/resolve.rs +++ b/src/local/resolve.rs @@ -4,12 +4,13 @@ //! The detection rule is documented in `local/mod.rs`: an experiment/run is //! "local" iff its experiment id exists in `local_experiments`. Every dual-mode //! and reject-guard command dispatches through here so the rule lives in one -//! place. Note that store membership alone is NOT enough for runs — server-mode -//! HF runs also live in the `runs` table — so `resolve_run` reuses the existing -//! `local::local_run` correctness test rather than re-deriving it. +//! place. `resolve_project` keys on `local_projects`, `resolve_experiment` on +//! `local_experiments`. Note that store membership alone is NOT enough for runs +//! — server-mode HF runs also live in the `runs` table — so `resolve_run` reuses +//! the existing `local::local_run` correctness test rather than re-deriving it. use crate::error::Result; -use crate::local::model::LocalProject; +use crate::local::model::{LocalExperiment, LocalProject}; use crate::store::{Store, StoredRun}; /// Which control plane owns a given project id. @@ -23,6 +24,17 @@ pub enum ProjectRef { Server(String), } +/// Which control plane owns a given experiment id. +pub enum ExperimentRef { + /// The id resolves to a row in `local_experiments`; the fetched experiment + /// is carried through so the local arm needs no second store lookup. Boxed + /// so the enum isn't dominated by `LocalExperiment`'s size (the common arm + /// is `Server`). + Local(Box<LocalExperiment>), + /// The id is not local; treat it as a server (api) experiment. + Server(String), +} + /// Which control plane owns a given run id. pub enum RunRef { /// The run belongs to a local experiment (per `local::local_run`); the @@ -55,6 +67,14 @@ pub fn resolve_project(store: &Store, project_id: &str) -> Result<ProjectRef> { } } +/// Decide once: an experiment id is local iff it names a known local experiment. +pub fn resolve_experiment(store: &Store, exp_id: &str) -> Result<ExperimentRef> { + match store.get_local_experiment(exp_id)? { + Some(e) => Ok(ExperimentRef::Local(Box::new(e))), + None => Ok(ExperimentRef::Server(exp_id.to_string())), + } +} + /// Decide once for a run id. Reuses `local::local_run`, which encodes the /// correct test (the run's experiment must be in `local_experiments`) — store /// membership alone is not enough, since server HF runs also live in `runs`. @@ -151,6 +171,28 @@ mod tests { } } + #[test] + fn local_experiment_id_resolves_local() { + let store = temp_store(); + store.create_local_project(&project("p1")).unwrap(); + store + .create_local_experiment(&experiment("e1", "p1")) + .unwrap(); + match resolve_experiment(&store, "e1").unwrap() { + ExperimentRef::Local(e) => assert_eq!(e.id, "e1"), + ExperimentRef::Server(_) => panic!("known local experiment must resolve Local"), + } + } + + #[test] + fn unknown_experiment_id_resolves_server() { + let store = temp_store(); + match resolve_experiment(&store, "nope").unwrap() { + ExperimentRef::Server(id) => assert_eq!(id, "nope"), + ExperimentRef::Local(_) => panic!("unknown id must resolve Server"), + } + } + #[test] fn server_hf_run_in_runs_table_resolves_server() { // Regression guard: a server-mode HF run lands in the `runs` table but