From 6ae4b26a589f869f591dfb5b5307cd2f05ab2ef1 Mon Sep 17 00:00:00 2001 From: Sven Wagner-Boysen <3901085+svenwb@users.noreply.github.com> Date: Sun, 20 Sep 2026 11:13:55 +0000 Subject: [PATCH] feat(omnidev): support pnpm through Corepack Signed-off-by: Sven Wagner-Boysen <3901085+svenwb@users.noreply.github.com> --- dev/omnidev/README.md | 6 +- dev/omnidev/src/main.rs | 12 +- dev/omnidev/src/process.rs | 162 +++++++++++++++++++++++++ dev/omnidev/src/supervisor.rs | 53 ++++---- dev/omnidev/tests/web_prerequisites.rs | 38 ++++++ 5 files changed, 235 insertions(+), 36 deletions(-) create mode 100644 dev/omnidev/tests/web_prerequisites.rs diff --git a/dev/omnidev/README.md b/dev/omnidev/README.md index a8ce4028700..2fe7818283c 100644 --- a/dev/omnidev/README.md +++ b/dev/omnidev/README.md @@ -34,14 +34,16 @@ replaces the three-terminal local dev flow (`omnigent server`, `omnigent host`, ## Build & run -Requires the repo's usual dev prerequisites (`uv` for Python, `pnpm` for the -web UI) plus a Rust toolchain. +Requires the repo's usual dev prerequisites (`uv` for Python, Node 22+ and +either `pnpm` or Corepack for the web UI) plus a Rust toolchain. ```bash cd dev/omnidev cargo run # launches the TUI for the surrounding checkout ``` +If `pnpm` is missing from `PATH`, omnidev automatically uses `corepack pnpm`. + Run it from anywhere inside the checkout — it walks up to the repo root (the `.jj`/`.git` marker) and requires `omnigent/` and `web/` to be present. Build a release binary with `cargo build --release` diff --git a/dev/omnidev/src/main.rs b/dev/omnidev/src/main.rs index abb90d911e0..16b1e542298 100644 --- a/dev/omnidev/src/main.rs +++ b/dev/omnidev/src/main.rs @@ -258,6 +258,11 @@ async fn run_supervisor(args: RunArgs) -> Result<()> { profile, )?); + let web = if args.no_vite { + None + } else { + Some(process::WebCommands::resolve(&pod)?) + }; let shared = Shared::new(&pod); let (cmd_tx, cmd_rx) = mpsc::unbounded_channel::(); @@ -272,12 +277,7 @@ async fn run_supervisor(args: RunArgs) -> Result<()> { )?; // Supervisor runs on the tokio runtime; the TUI drives it via cmd_tx. - let supervisor = Supervisor::new( - pod.clone(), - shared.clone(), - !args.no_vite, - args.trust_lan_origins, - ); + let supervisor = Supervisor::new(pod.clone(), shared.clone(), web, args.trust_lan_origins); let sup_handle = tokio::spawn(supervisor.run(cmd_rx)); // Run the TUI (owns the terminal) until the user quits. diff --git a/dev/omnidev/src/process.rs b/dev/omnidev/src/process.rs index cdc43949943..287ae4a0b88 100644 --- a/dev/omnidev/src/process.rs +++ b/dev/omnidev/src/process.rs @@ -1,7 +1,11 @@ //! Concrete command specs for the three supervised processes. +use std::ffi::OsStr; +use std::os::unix::fs::PermissionsExt; use std::path::PathBuf; +use anyhow::{bail, Result}; + use crate::install::PYTHON_VERSION; use crate::pod::Pod; @@ -14,6 +18,57 @@ pub struct ProcSpec { pub extra_env: Vec<(String, String)>, } +/// Web commands resolved before the supervisor starts any children. +pub struct WebCommands { + pub vite: ProcSpec, + pub prepare: Option, +} + +impl WebCommands { + pub fn resolve(pod: &Pod) -> Result { + Self::resolve_on_path(pod, std::env::var_os("PATH").as_deref()) + } + + fn resolve_on_path(pod: &Pod, path: Option<&OsStr>) -> Result { + let mut commands = Self { + vite: ProcSpec::vite(pod), + prepare: if pod.profile.as_ref().is_some_and(|p| p.prepare.is_none()) { + None + } else { + Some(ProcSpec::web_prepare(pod)) + }, + }; + if pod.profile.is_some() { + return Ok(commands); + } + + let available = |program: &str| { + path.is_some_and(|path| { + std::env::split_paths(path).any(|dir| { + std::fs::metadata(pod.web_dir().join(dir).join(program)) + .is_ok_and(|meta| meta.is_file() && meta.permissions().mode() & 0o111 != 0) + }) + }) + }; + if available("pnpm") { + return Ok(commands); + } + if !available("corepack") { + bail!( + "Neither `pnpm` nor `corepack` is on PATH. The dev UI requires Node 22+ \ + and pnpm. Install pnpm (`npm install -g pnpm`), or install Corepack \ + (`npm install -g corepack`), then retry. Use --no-vite for backend-only development." + ); + } + // Corepack can run the repository's pinned pnpm without installing global shims. + for spec in std::iter::once(&mut commands.vite).chain(commands.prepare.iter_mut()) { + spec.program = "corepack".into(); + spec.args.insert(0, "pnpm".into()); + } + Ok(commands) + } +} + impl ProcSpec { fn from_profile(pod: &Pod, profile: &crate::profile::ProcessProfile) -> ProcSpec { let expand = |value: &str| { @@ -150,6 +205,98 @@ mod tests { use crate::ports::Ports; use crate::profile::{ProcessProfile, Profile}; + fn web_pod(repo: PathBuf) -> Pod { + std::fs::create_dir_all(repo.join("web")).unwrap(); + Pod { + dir: repo.join("pod"), + repo_root: repo, + ports: Ports { + server: 19191, + vite: 19292, + }, + vite_host: "127.0.0.1".into(), + trusted_origins: Vec::new(), + profile: None, + } + } + + fn fake_executable(dir: &std::path::Path, name: &str) { + let path = dir.join(name); + std::fs::write(&path, "#!/bin/sh\nprintf '%s\\n' \"$@\"\n").unwrap(); + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + + #[test] + fn corepack_fallback_runs_install_and_vite_with_original_arguments() { + let repo = tempdir(); + let bin = repo.join("bin"); + std::fs::create_dir(&bin).unwrap(); + fake_executable(&bin, "corepack"); + let pod = web_pod(repo.clone()); + + let commands = WebCommands::resolve_on_path(&pod, Some(bin.as_os_str())).unwrap(); + for (spec, expected) in [ + (commands.prepare.unwrap(), "pnpm\ninstall\n"), + ( + commands.vite, + "pnpm\nrun\ndev\n--host\n127.0.0.1\n--port\n19292\n--strictPort\n", + ), + ] { + assert_eq!(spec.program, "corepack"); + let output = std::process::Command::new(bin.join(&spec.program)) + .args(&spec.args) + .current_dir(&spec.cwd) + .output() + .unwrap(); + assert!(output.status.success()); + assert_eq!(String::from_utf8(output.stdout).unwrap(), expected); + assert_eq!(spec.cwd, pod.web_dir()); + } + std::fs::remove_dir_all(repo).unwrap(); + } + + #[test] + fn standalone_pnpm_is_preferred_over_corepack() { + let repo = tempdir(); + fake_executable(&repo, "pnpm"); + fake_executable(&repo, "corepack"); + let pod = web_pod(repo.clone()); + let commands = WebCommands::resolve_on_path(&pod, Some(repo.as_os_str())).unwrap(); + + assert_eq!(commands.vite.program, "pnpm"); + assert_eq!(commands.vite.args[0], "run"); + let prepare = commands.prepare.unwrap(); + assert_eq!(prepare.program, "pnpm"); + assert_eq!(prepare.args, ["install"]); + std::fs::remove_dir_all(repo).unwrap(); + } + + #[test] + fn missing_web_tools_report_setup_instructions() { + let repo = tempdir(); + let pod = web_pod(repo.clone()); + for path in [None, Some(repo.as_os_str())] { + let error = WebCommands::resolve_on_path(&pod, path).err().unwrap(); + let message = error.to_string(); + assert!(message.contains("Neither `pnpm` nor `corepack` is on PATH")); + assert!(message.contains("npm install -g pnpm")); + assert!(message.contains("--no-vite")); + } + std::fs::remove_dir_all(repo).unwrap(); + } + + #[test] + fn nonexecutable_pnpm_does_not_hide_corepack() { + let repo = tempdir(); + let pod = web_pod(repo.clone()); + std::fs::write(repo.join("pnpm"), "not executable").unwrap(); + fake_executable(&repo, "corepack"); + + let commands = WebCommands::resolve_on_path(&pod, Some(repo.as_os_str())).unwrap(); + assert_eq!(commands.vite.program, "corepack"); + std::fs::remove_dir_all(repo).unwrap(); + } + #[test] fn vite_forwards_configured_host_and_port_but_backend_url_stays_loopback() { let repo = tempdir(); @@ -278,6 +425,21 @@ mod tests { ); assert_eq!(spec.cwd, repo.join("service")); assert!(!pod.host_enabled()); + + let web = WebCommands::resolve_on_path(&pod, None).unwrap(); + assert_eq!(web.vite.program, "server"); + assert_eq!(web.vite.args, spec.args); + assert!(web.prepare.is_none()); + + pod.profile.as_mut().unwrap().prepare = Some(ProcessProfile { + command: vec!["custom-install".into(), "--offline".into()], + cwd: "ui".into(), + }); + let web = WebCommands::resolve_on_path(&pod, None).unwrap(); + let prepare = web.prepare.unwrap(); + assert_eq!(prepare.program, "custom-install"); + assert_eq!(prepare.args, ["--offline"]); + assert_eq!(prepare.cwd, repo.join("ui")); } fn tempdir() -> std::path::PathBuf { diff --git a/dev/omnidev/src/supervisor.rs b/dev/omnidev/src/supervisor.rs index cb244b0e77d..76d111cee23 100644 --- a/dev/omnidev/src/supervisor.rs +++ b/dev/omnidev/src/supervisor.rs @@ -15,7 +15,7 @@ use tokio::time::{sleep, timeout}; use crate::browser; use crate::omnigent_cmd; use crate::pod::Pod; -use crate::process::ProcSpec; +use crate::process::{ProcSpec, WebCommands}; use crate::state::{ProcId, ProcStatus, Shared}; const CONVERSATION_PREFILL_VERSION: &str = "v1"; @@ -77,7 +77,7 @@ pub struct Supervisor { pod: Arc, shared: Arc>, env: Vec<(String, String)>, - vite_enabled: bool, + web: Option, /// Whether `--trust-lan-origins` was requested, so we can warn if it was /// asked for but no LAN interface turned up any origins to trust. trust_lan_origins: bool, @@ -93,7 +93,7 @@ impl Supervisor { pub fn new( pod: Arc, shared: Arc>, - vite_enabled: bool, + web: Option, trust_lan_origins: bool, ) -> Supervisor { let env = pod.env(); @@ -102,7 +102,7 @@ impl Supervisor { pod, shared, env, - vite_enabled, + web, trust_lan_origins, slots: Default::default(), expected_stops: HashSet::new(), @@ -139,10 +139,14 @@ impl Supervisor { } self.start_backend().await; - if self.vite_enabled { + if let Some(web) = &self.web { + if web.vite.program == "corepack" && self.pod.profile.is_none() { + self.event("pnpm is not on PATH; using corepack pnpm for web commands"); + } self.prepare_vite().await; - self.spawn(ProcId::Vite); - self.open_ui_when_ready(); + if self.spawn(ProcId::Vite) { + self.open_ui_when_ready(); + } } loop { @@ -336,7 +340,7 @@ impl Supervisor { // backend, so treat it as a backend restart. ProcId::Server | ProcId::Host => self.start_backend_restart().await, ProcId::Vite => { - if self.vite_enabled { + if self.web.is_some() { self.event("restarting vite"); self.stop(ProcId::Vite).await; self.prepare_vite().await; @@ -346,17 +350,13 @@ impl Supervisor { } } - fn spec(&self, id: ProcId) -> ProcSpec { - match id { - ProcId::Server => ProcSpec::server(&self.pod), - ProcId::Host => ProcSpec::host(&self.pod), - ProcId::Vite => ProcSpec::vite(&self.pod), - } - } - /// Spawn a child in its own process group and wire up output + exit monitor. - fn spawn(&mut self, id: ProcId) { - let spec = self.spec(id); + fn spawn(&mut self, id: ProcId) -> bool { + let spec = match id { + ProcId::Server => &ProcSpec::server(&self.pod), + ProcId::Host => &ProcSpec::host(&self.pod), + ProcId::Vite => &self.web.as_ref().expect("web is enabled").vite, + }; self.set_status(id, ProcStatus::Starting); let mut cmd = Command::new(&spec.program); @@ -385,7 +385,7 @@ impl Supervisor { .unwrap() .log_proc(id, format!("failed to spawn {}: {e}", spec.program)); self.set_status(id, ProcStatus::Crashed); - return; + return false; } }; @@ -422,6 +422,7 @@ impl Supervisor { status, }); }); + true } /// Prepare web dependencies before Vite starts, but only when they are @@ -430,13 +431,10 @@ impl Supervisor { /// non-fatal: we still let Vite try, so a transient pnpm hiccup doesn't block /// the whole session. async fn prepare_vite(&self) { - if self - .pod - .profile - .as_ref() - .is_some_and(|profile| profile.prepare.is_none()) - || !self.pod.needs_web_prepare() - { + let Some(spec) = self.web.as_ref().and_then(|web| web.prepare.as_ref()) else { + return; + }; + if !self.pod.needs_web_prepare() { return; } self.set_status(ProcId::Vite, ProcStatus::Starting); @@ -445,7 +443,6 @@ impl Supervisor { "web deps missing or stale — preparing dependencies".into(), ); - let spec = ProcSpec::web_prepare(&self.pod); let mut cmd = Command::new(&spec.program); cmd.args(&spec.args) .current_dir(&spec.cwd) @@ -596,7 +593,7 @@ impl Supervisor { } } ProcId::Vite => { - if self.vite_enabled { + if self.web.is_some() { self.spawn(ProcId::Vite); } } diff --git a/dev/omnidev/tests/web_prerequisites.rs b/dev/omnidev/tests/web_prerequisites.rs new file mode 100644 index 00000000000..36483296294 --- /dev/null +++ b/dev/omnidev/tests/web_prerequisites.rs @@ -0,0 +1,38 @@ +//! Missing web tooling should fail before starting children or entering the TUI. + +use std::fs; +use std::process::Command; + +#[test] +fn missing_pnpm_and_corepack_fail_before_startup() { + let repo = std::env::temp_dir().join(format!( + "omnidev-web-prerequisites-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + for dir in [".git", "omnigent", "web", "bin"] { + fs::create_dir_all(repo.join(dir)).unwrap(); + } + let output = Command::new(env!("CARGO_BIN_EXE_omnidev")) + .current_dir(&repo) + .env("PATH", repo.join("bin")) + .env("OMNIGENT_CONFIG_HOME", repo.join("config")) + .args(["--pod-dir", repo.join("pod").to_str().unwrap()]) + .output() + .unwrap(); + + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("Neither `pnpm` nor `corepack` is on PATH"), + "{stderr}" + ); + assert!(stderr.contains("npm install -g pnpm")); + assert!(stderr.contains("--no-vite")); + assert!(output.stdout.is_empty(), "the TUI must not start"); + assert!(!repo.join("pod/logs/server.log").exists()); + fs::remove_dir_all(repo).unwrap(); +}