diff --git a/apps/rocm/src/main.rs b/apps/rocm/src/main.rs index 2de7389a2..c33241bc3 100644 --- a/apps/rocm/src/main.rs +++ b/apps/rocm/src/main.rs @@ -7193,7 +7193,7 @@ fn maybe_auto_install_sdk_preferred_engine( let mut config = RocmCliConfig::load(paths)?; let env_root = env_root_for_engine_install(paths, &config, engine, &finalized.runtime_key)?; - let response = engine_request_with_env_root::<_, InstallResponse>( + let response = match engine_request_with_env_root::<_, InstallResponse>( Some(paths), engine, EngineMethod::Install, @@ -7204,7 +7204,23 @@ fn maybe_auto_install_sdk_preferred_engine( env_root: env_root.clone(), }, env_root.as_deref(), - )?; + ) { + Ok(response) => response, + Err(error) => { + // A failed engine install is the case most likely to leave the SDK's torch + // sitting over the engine's pin, because the install is what would have + // restored it. Reporting the check only on success would print nothing here + // — and `install sdk` still exits 0 — which is exactly the silent broken + // runtime this block exists to prevent. + report_engine_dependency_check( + paths, + engine, + runtime_python_for_key(paths, &finalized.runtime_key).as_deref(), + &finalized.runtime_key, + ); + return Err(error); + } + }; println!(" reinstall: false"); println!(" env_id: {}", response.env_id); println!(" env_path: {}", response.env_path); @@ -7223,6 +7239,17 @@ fn maybe_auto_install_sdk_preferred_engine( engine_config.preferred_env_id = Some(response.env_id.clone()); } config.save(paths)?; + + // The SDK and the engine share this environment, and the SDK's torch stack was + // just written into it. Say plainly whether the engine's own requirements + // survived that, so a runtime the engine cannot use is never reported only as a + // successful install. + report_engine_dependency_check( + paths, + engine, + Some(Path::new(&response.python_executable)), + &finalized.runtime_key, + ); } record_cli_audit_event( @@ -7239,6 +7266,125 @@ fn maybe_auto_install_sdk_preferred_engine( Ok(()) } +/// Whether an installed engine's declared requirements are met in the environment it +/// shares with the ROCm SDK. +#[derive(Debug, Clone, PartialEq, Eq)] +enum EngineDependencyCheck { + /// Every requirement the engine declares is satisfied. + Satisfied, + /// The engine declares requirements the environment does not meet, one line each, + /// as the resolver reported them. + Violated(Vec), + /// The check itself could not run (no usable `uv`, unreadable environment). + NotVerified(String), +} + +/// Print the dependency check for `engine` and record it in the CLI audit log. +/// +/// A surviving violation does not fail the install: the SDK itself installed correctly, +/// and abandoning a multi-gigabyte install would cost the user more than the warning and +/// the one-line remedy it names. +fn report_engine_dependency_check( + paths: &AppPaths, + engine: &str, + python: Option<&Path>, + runtime_key: &str, +) { + let outcome = engine_dependency_check(paths, engine, python); + print!("{}", render_engine_dependency_check(engine, &outcome)); + let (level, message) = match &outcome { + EngineDependencyCheck::Satisfied => ( + "info", + format!("engine={engine} runtime_id={runtime_key} dependency_check=satisfied"), + ), + EngineDependencyCheck::Violated(details) => ( + "error", + format!( + "engine={engine} runtime_id={runtime_key} dependency_check=violated: {}", + details.join("; ") + ), + ), + EngineDependencyCheck::NotVerified(reason) => ( + "info", + format!( + "engine={engine} runtime_id={runtime_key} dependency_check=not_verified: {reason}" + ), + ), + }; + record_cli_audit_event( + paths, + "engine", + "engine_dependency_check", + level, + message, + None, + ); +} + +/// The Python interpreter of the managed runtime `runtime_key`, when its manifest names +/// one. Used to check an environment whose engine install did not get far enough to +/// report its own interpreter. +fn runtime_python_for_key(paths: &AppPaths, runtime_key: &str) -> Option { + let manifests = therock::load_runtime_manifests(paths).ok()?; + let manifest = runtime_manifest_for_selector(&manifests, runtime_key)?; + manifest.python_executable.as_deref().map(PathBuf::from) +} + +fn engine_dependency_check( + paths: &AppPaths, + engine: &str, + python: Option<&Path>, +) -> EngineDependencyCheck { + let Some(python) = python else { + return EngineDependencyCheck::NotVerified( + "the runtime's Python environment could not be located".to_owned(), + ); + }; + match rocm_core::check_dependencies(paths, python) { + Ok(violations) => { + let owned = rocm_core::violations_requiring(&violations, engine); + if owned.is_empty() { + EngineDependencyCheck::Satisfied + } else { + EngineDependencyCheck::Violated( + owned + .iter() + .map(|violation| violation.detail.clone()) + .collect(), + ) + } + } + Err(error) => EngineDependencyCheck::NotVerified(error.to_string()), + } +} + +fn render_engine_dependency_check(engine: &str, outcome: &EngineDependencyCheck) -> String { + let mut output = String::new(); + match outcome { + EngineDependencyCheck::Satisfied => { + let _ = writeln!(output, " dependency_check: satisfied"); + } + EngineDependencyCheck::NotVerified(reason) => { + let _ = writeln!( + output, + " dependency_check: not_verified ({})", + sanitize_log_value(reason) + ); + } + EngineDependencyCheck::Violated(details) => { + let _ = writeln!(output, " dependency_check: violated"); + for detail in details { + let _ = writeln!(output, " violation: {}", sanitize_log_value(detail)); + } + let _ = writeln!( + output, + " action: rocm engines install {engine} --reinstall" + ); + } + } + output +} + fn render_sdk_install_success(finalized: &SdkInstallFinalization) -> String { format!( "ROCm SDK installed successfully.\n install folder: {}\n active runtime: {}\n next step: run `rocm help` to see how to use rocm-cli.\n", @@ -25432,6 +25578,65 @@ ID_LIKE="suse opensuse" Ok(()) } + #[test] + fn a_satisfied_dependency_check_is_stated_plainly() { + let rendered = render_engine_dependency_check("vllm", &EngineDependencyCheck::Satisfied); + + assert_eq!(rendered, " dependency_check: satisfied\n"); + } + + #[test] + fn a_violated_dependency_check_names_the_pin_and_the_remedy() { + // The SDK torch stack replaced the build vLLM pins. The install + // succeeded, so the only signal the user gets is this block. + let rendered = render_engine_dependency_check( + "vllm", + &EngineDependencyCheck::Violated(vec![ + "The package `vllm` requires `torch==2.10.0+git8514f05`, but `2.9.1+rocm7.14.0a20260611` is installed".to_owned(), + ]), + ); + + assert!(rendered.contains(" dependency_check: violated\n")); + assert!(rendered.contains("torch==2.10.0+git8514f05")); + assert!(rendered.contains("2.9.1+rocm7.14.0a20260611")); + assert!(rendered.contains(" action: rocm engines install vllm --reinstall\n")); + } + + #[test] + fn an_unlocatable_environment_is_reported_not_assumed_healthy() { + // The engine install can fail before it reports its own interpreter. That path + // still prints a block, and with no interpreter to check it must say so rather + // than fall through to `satisfied`. + let (root, paths) = test_paths("engine-dependency-no-python"); + + let outcome = engine_dependency_check(&paths, "vllm", None); + let _ = fs::remove_dir_all(root); + + assert_eq!( + outcome, + EngineDependencyCheck::NotVerified( + "the runtime's Python environment could not be located".to_owned() + ) + ); + assert!( + render_engine_dependency_check("vllm", &outcome).contains("not_verified"), + "the failure path still prints a dependency_check block" + ); + } + + #[test] + fn a_dependency_check_that_could_not_run_says_so_instead_of_claiming_health() { + let rendered = render_engine_dependency_check( + "vllm", + &EngineDependencyCheck::NotVerified("uv binary is unavailable\nofflinehost".to_owned()), + ); + + assert!(rendered.contains("not_verified")); + assert!(!rendered.contains("satisfied")); + // A multi-line failure must stay on one reported line. + assert_eq!(rendered.lines().count(), 1, "{rendered}"); + } + #[test] fn render_engine_inventory_text_surfaces_external_plugin_policy() { let (root, paths) = test_paths("engine-plugin-policy"); diff --git a/crates/rocm-core/src/lib.rs b/crates/rocm-core/src/lib.rs index 1846e0131..060926eb1 100644 --- a/crates/rocm-core/src/lib.rs +++ b/crates/rocm-core/src/lib.rs @@ -70,9 +70,10 @@ pub use runtime::{ runtime_rocm_library_filename, shell_command_for_host, user_runtime_dir, }; pub use uv::{ - DEFAULT_UV_TIMEOUT_SECS, UV_CACHE_DIR_ENV, UV_CACHE_DIR_OVERRIDE_ENV, UvCacheSource, - ensure_uv_binary, uv_binary_name, uv_cache_source, uv_command_env, uv_http_timeout_secs, - uv_pip_freeze_args, uv_pip_install_base, uv_venv_args, + DEFAULT_UV_TIMEOUT_SECS, DependencyViolation, UV_CACHE_DIR_ENV, UV_CACHE_DIR_OVERRIDE_ENV, + UvCacheSource, check_dependencies, ensure_uv_binary, uv_binary_name, uv_cache_source, + uv_command_env, uv_http_timeout_secs, uv_pip_check_args, uv_pip_freeze_args, + uv_pip_install_base, uv_venv_args, violations_requiring, }; pub const DEFAULT_LOCAL_PORT: u16 = 11_435; diff --git a/crates/rocm-core/src/uv.rs b/crates/rocm-core/src/uv.rs index b9ab379e8..ef99cf503 100644 --- a/crates/rocm-core/src/uv.rs +++ b/crates/rocm-core/src/uv.rs @@ -201,6 +201,124 @@ pub fn uv_pip_freeze_args(venv_python: &Path) -> Vec { ] } +/// Arguments for `uv pip check` targeting the interpreter `venv_python`. +/// +/// `--color never` is not cosmetic: this output is parsed, and `uv` colorizes its +/// findings whenever `FORCE_COLOR` / `CLICOLOR_FORCE` is exported — even when stderr is +/// a pipe rather than a terminal. The escape prefix would push every finding past the +/// parser, which reads as "the check could not run" and silently restores the very +/// behavior the check exists to catch. +pub fn uv_pip_check_args(venv_python: &Path) -> Vec { + vec![ + "pip".to_owned(), + "check".to_owned(), + "--color".to_owned(), + "never".to_owned(), + "--python".to_owned(), + venv_python.to_string_lossy().into_owned(), + ] +} + +/// One unsatisfied `Requires-Dist` in an environment, as reported by `uv pip check`. +/// +/// The managed runtime venv has more than one owner: `rocm install sdk` writes the +/// TheRock torch stack into it, and engines installed against that runtime write their +/// own pinned dependencies over the top. Either side can leave the other's requirements +/// unmet without removing any package, so "the distribution is importable" is not +/// evidence that it can run. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DependencyViolation { + /// Distribution whose `Requires-Dist` is unsatisfied (the requirer, not the + /// package at the wrong version). + pub requiring: String, + /// The line `uv` reported, kept verbatim so the user sees the requirement and the + /// installed version exactly as the resolver saw them. + pub detail: String, +} + +/// Opening frame of every line `uv pip check` emits about a distribution. +const UV_CHECK_VIOLATION_PREFIX: &str = "The package `"; + +/// What follows the requirer's name on an unsatisfied-requirement line specifically. +/// +/// The trailing backtick is the whole point: `uv` reports several other conditions under +/// the same opening frame — a missing `WHEEL`/`METADATA` file, an unsatisfied +/// `Requires-Python`, a package with multiple installed distributions — and none of them +/// is a `Requires-Dist` a reinstall of the requirer can resolve. Only a requirement +/// `uv` quotes is one. +const UV_CHECK_REQUIREMENT_INFIX: &str = " requires `"; + +/// Report the unsatisfied `Requires-Dist` entries in the environment at `venv_python`. +/// +/// An empty vector means every installed distribution's requirements are met. `uv pip +/// check` writes its findings to stderr and exits non-zero when it finds any, so a +/// non-zero exit with parsed findings is a successful check — but a non-zero exit with +/// nothing parsed is a failed invocation (missing interpreter, unusable `uv`) and is +/// surfaced as an error rather than mistaken for a clean environment. +pub fn check_dependencies( + paths: &AppPaths, + venv_python: &Path, +) -> Result> { + let uv = ensure_uv_binary(paths).context("failed to acquire uv binary for dependency check")?; + let output = Command::new(&uv) + .args(uv_pip_check_args(venv_python)) + .envs(uv_command_env(paths)) + .output() + .with_context(|| format!("failed to run {} pip check", uv.display()))?; + let stderr = String::from_utf8_lossy(&output.stderr); + let violations = parse_dependency_violations(&stderr); + if output.status.success() { + return Ok(Vec::new()); + } + if violations.is_empty() { + bail!( + "`uv pip check` failed for {}: {}", + venv_python.display(), + stderr.trim() + ); + } + Ok(violations) +} + +/// The violations whose requirer is `distribution`, matched case-insensitively. +/// +/// Environments that host an inference engine routinely carry unrelated upstream +/// conflicts between third-party packages. Callers filter to the distribution they own +/// so those do not bury the one finding they can act on. +pub fn violations_requiring<'a>( + violations: &'a [DependencyViolation], + distribution: &str, +) -> Vec<&'a DependencyViolation> { + violations + .iter() + .filter(|violation| violation.requiring.eq_ignore_ascii_case(distribution)) + .collect() +} + +/// Pull the unsatisfied-requirement lines out of a `uv pip check` stderr body. +/// +/// `uv` frames one as ``The package `` requires ``, but `` is +/// installed`` (or `` but it's not installed``), among progress lines (`Using Python +/// ...`, `Checked N packages ...`, `Found N incompatibilities`) and reports of other +/// conditions that share the opening frame. Both halves of the frame are required, so +/// neither a change in the surrounding chatter nor a differently-shaped `uv` diagnostic +/// can invent a violation. +fn parse_dependency_violations(stderr: &str) -> Vec { + stderr + .lines() + .map(str::trim) + .filter_map(|line| { + let requirer = line.strip_prefix(UV_CHECK_VIOLATION_PREFIX)?; + let (requirer, rest) = requirer.split_once('`')?; + rest.starts_with(UV_CHECK_REQUIREMENT_INFIX) + .then(|| DependencyViolation { + requiring: requirer.to_owned(), + detail: line.to_owned(), + }) + }) + .collect() +} + /// Ensure a usable `uv` binary is available, downloading and caching one if needed. /// Returns the path to the executable. pub fn ensure_uv_binary(paths: &AppPaths) -> Result { @@ -461,6 +579,139 @@ mod tests { ); } + #[test] + fn pip_check_args_target_venv_python_and_disable_color() { + let args = uv_pip_check_args(Path::new("/envs/run/bin/python")); + assert_eq!( + args, + vec![ + "pip", + "check", + "--color", + "never", + "--python", + "/envs/run/bin/python" + ], + "the output is parsed, so color must be off even under FORCE_COLOR" + ); + } + + #[test] + fn a_colorized_finding_is_never_silently_dropped() { + // `--color never` is what keeps this out of the real output, but the parser must + // not be the only thing standing between a colorized line and a false all-clear: + // an escape-prefixed finding must not parse as a clean environment. + let stderr = "\u{1b}[1mThe package `vllm` requires `torch==2.10.0+git8514f05`, but `2.9.1` is installed\u{1b}[0m\n"; + + assert!( + parse_dependency_violations(stderr).is_empty(), + "a colorized line does not parse — check_dependencies reports the non-zero \ + exit as a failed invocation rather than a clean environment" + ); + } + + #[test] + fn an_absent_requirement_is_a_violation() { + // Verbatim `uv pip check` output for a dependency that is missing outright + // rather than present at the wrong version — also an unsatisfied Requires-Dist. + let stderr = + "The package `vllm` requires `torch==2.10.0+git8514f05`, but it's not installed\n"; + + let violations = parse_dependency_violations(stderr); + + assert_eq!(violations.len(), 1, "{violations:?}"); + assert_eq!(violations[0].requiring, "vllm"); + } + + #[test] + fn other_uv_diagnostics_sharing_the_frame_are_not_violations() { + // Verbatim shapes `uv pip check` emits under the same `The package `x`` opening. + // None is a Requires-Dist a reinstall of the requirer would resolve, so treating + // them as violations would force a futile multi-gigabyte reinstall and print a + // remedy that cannot clear the condition. + let stderr = "\ +The package `vllm` is broken or incomplete (unable to read `WHEEL` file). Consider recreating the virtualenv, or removing the package directory at: /rocm/site-packages/vllm-0.26.0.dist-info. +The package `vllm` is broken or incomplete (unable to read `METADATA`). Consider recreating the virtualenv, or removing the package directory at: /rocm/site-packages/vllm-0.26.0.dist-info. +The package `vllm` requires Python >=3.99, but `3.12.9` is installed +The package `vllm` has multiple installed distributions: /rocm/site-packages/vllm-0.26.0.dist-info +"; + + assert!( + parse_dependency_violations(stderr).is_empty(), + "{:?}", + parse_dependency_violations(stderr) + ); + } + + #[test] + fn dependency_violations_come_only_from_framed_findings() { + // Verbatim `uv pip check` stderr for an environment where the SDK torch stack + // was written over the engine's pinned torch, plus an unrelated + // upstream conflict of the kind these environments routinely carry. + let stderr = "\ +Using Python 3.12.9 environment at: /rocm/runtimes/wheel/nightly-gfx94x +Checked 251 packages in 12ms +Found 2 incompatibilities +The package `vllm` requires `torch==2.10.0+git8514f05`, but `2.9.1+rocm7.14.0a20260611` is installed +The package `tilelang` requires `cloudpickle>=3.0`, but `2.2.1` is installed +"; + + let violations = parse_dependency_violations(stderr); + + assert_eq!(violations.len(), 2, "{violations:?}"); + assert_eq!(violations[0].requiring, "vllm"); + assert!( + violations[0].detail.contains("torch==2.10.0+git8514f05"), + "the requirement and the installed version are kept verbatim: {}", + violations[0].detail + ); + assert_eq!(violations[1].requiring, "tilelang"); + } + + #[test] + fn a_compatible_environment_reports_no_violations() { + let stderr = "\ +Using Python 3.12.9 environment at: /rocm/runtimes/wheel/nightly-gfx94x +Checked 251 packages in 12ms +All installed packages are compatible +"; + + assert!(parse_dependency_violations(stderr).is_empty()); + assert!(parse_dependency_violations("").is_empty()); + } + + #[test] + fn unframed_output_never_invents_a_violation() { + // An invocation failure (bad interpreter) must not read as a finding; the + // caller distinguishes it from a clean environment by the empty result. + let stderr = "error: Failed to inspect Python interpreter from /nope/bin/python\n"; + + assert!(parse_dependency_violations(stderr).is_empty()); + } + + #[test] + fn violations_are_filtered_to_the_distribution_that_owns_them() { + let violations = vec![ + DependencyViolation { + requiring: "vLLM".to_owned(), + detail: "The package `vLLM` requires `torch==2.10.0`, but `2.9.1` is installed" + .to_owned(), + }, + DependencyViolation { + requiring: "tilelang".to_owned(), + detail: + "The package `tilelang` requires `cloudpickle>=3.0`, but `2.2.1` is installed" + .to_owned(), + }, + ]; + + let owned = violations_requiring(&violations, "vllm"); + + assert_eq!(owned.len(), 1, "match is case-insensitive: {owned:?}"); + assert_eq!(owned[0].requiring, "vLLM"); + assert!(violations_requiring(&violations, "torch").is_empty()); + } + #[test] fn pip_install_base_targets_venv_python() { let args = uv_pip_install_base(Path::new("/envs/run/bin/python")); diff --git a/engines/vllm/src/lib.rs b/engines/vllm/src/lib.rs index 25d61c32c..7971d2b4f 100644 --- a/engines/vllm/src/lib.rs +++ b/engines/vllm/src/lib.rs @@ -5,8 +5,9 @@ use anyhow::{Context, Result, anyhow, bail}; use clap::{Parser, Subcommand}; use rocm_core::{ - AppPaths, DEFAULT_LOCAL_PORT, ensure_uv_binary, format_http_base_url, - openai_models_endpoint_has_model, require_nonempty, uv_command_env, uv_pip_install_base, + AppPaths, DEFAULT_LOCAL_PORT, DependencyViolation, check_dependencies, ensure_uv_binary, + format_http_base_url, openai_models_endpoint_has_model, require_nonempty, uv_command_env, + uv_pip_install_base, violations_requiring, }; use rocm_engine_protocol::{ DEFAULT_LOG_TAIL_LINES, DetectRequest, DetectResponse, DevicePolicy, @@ -406,22 +407,49 @@ fn capabilities() -> EngineCapabilities { } fn install_response(request: InstallRequest) -> Result { - let already_installed = if request.reinstall { - None - } else { - resolve_vllm_runtime(Some(&request.runtime_id)).ok() + // Resolve regardless of `reinstall`. Resolution answers *which* interpreter holds + // vLLM, and a forced reinstall needs that answer just as much as a repair does — + // gating it on `reinstall` left `--reinstall` with no assessed environment, so it + // fell back to `resolve_managed_runtime_python` (first prefix-matching candidate) + // and could reinstall a healthy environment while leaving the broken one broken. + // Only the short-circuit below is gated on `reinstall`. + let resolved = resolve_vllm_runtime(Some(&request.runtime_id)).ok(); + // A resolvable vLLM is not necessarily a usable one. `rocm install sdk` writes the + // TheRock torch stack into the same environment vLLM lives in, so a second run + // replaces the torch build vLLM pins without touching vLLM itself. + // Short-circuiting on "vllm resolves" left that environment unrepaired; short-circuit + // on "vllm's own requirements are met" instead, so the install below restores them. + let (already_installed, repair, assessed) = match resolved { + Some(runtime) => { + let repair = assess_runtime_repair(&runtime); + if request.reinstall || repair.needed { + (None, repair, assessed_python_for_repair(&runtime)) + } else { + (Some(runtime), repair, None) + } + } + None => (None, RepairAssessment::default(), None), }; let runtime = if let Some(runtime) = already_installed { runtime } else { - let managed = resolve_managed_runtime_python(Some(&request.runtime_id))?.with_context( - || { - format!( - "runtime `{}` did not resolve to a managed TheRock Python environment for automatic vLLM install", - request.runtime_id - ) - }, - )?; + // A repair installs into the environment that was *assessed*. The two resolvers + // do not agree: `resolve_vllm_runtime` walks the candidates until one actually + // has vLLM beside it, while `resolve_managed_runtime_python` takes the first + // candidate unconditionally — and `runtime_id` matches by prefix, so several + // candidates routinely qualify. Installing into a different interpreter than the + // one found broken would leave the broken one broken and report it fixed. + let managed = match assessed { + Some(assessed) => assessed, + None => resolve_managed_runtime_python(Some(&request.runtime_id))?.with_context( + || { + format!( + "runtime `{}` did not resolve to a managed TheRock Python environment for automatic vLLM install", + request.runtime_id + ) + }, + )?, + }; install_vllm_with_uv(&managed.python_executable, request.reinstall)?; resolve_vllm_runtime(Some(&managed.runtime_id)).with_context(|| { format!( @@ -457,10 +485,95 @@ fn install_response(request: InstallRequest) -> Result { )], capabilities: capabilities(), lock_hash: runtime_lock_hash(&runtime), - warnings: vllm_runtime_warnings(&runtime), + warnings: repair + .notes + .into_iter() + .chain(vllm_runtime_warnings(&runtime)) + .collect(), + }) +} + +/// Whether a resolvable vLLM environment still needs an install pass, and what to tell +/// the user about why. +#[derive(Debug, Default, Clone, PartialEq, Eq)] +struct RepairAssessment { + /// The environment holds vLLM but not the dependencies vLLM declares, so the + /// install must run even though `reinstall` was not requested. + needed: bool, + /// Findings to surface with the install response. + notes: Vec, +} + +/// The environment a repair must target: the one that was assessed, named by its own +/// runtime id rather than by the (possibly prefix-matching) id the caller requested. +fn assessed_python_for_repair(runtime: &VllmRuntime) -> Option { + Some(ManagedRuntimePython { + runtime_id: runtime.runtime_id.clone(), + python_executable: runtime.python_executable.clone()?, }) } +/// Decide whether an already-resolvable vLLM environment must be reinstalled. +/// +/// Only managed environments are assessed. An external environment belongs to the user: +/// rocm-cli reports on it but does not rewrite its packages, which is the very failure +/// mode this check exists to catch. +fn assess_runtime_repair(runtime: &VllmRuntime) -> RepairAssessment { + if !runtime_is_managed(runtime) { + return RepairAssessment::default(); + } + let Some(python) = runtime.python_executable.as_ref() else { + return RepairAssessment::default(); + }; + let paths = match AppPaths::discover() { + Ok(paths) => paths, + Err(error) => return unverified_repair(&error.to_string()), + }; + match check_dependencies(&paths, python) { + Ok(violations) => repair_from_violations(&violations), + // An unusable `uv` or an offline host must not block an install that would + // otherwise succeed; report that the check did not run and carry on as before. + Err(error) => unverified_repair(&error.to_string()), + } +} + +/// The repair decision for a set of violations found in the environment. +fn repair_from_violations(violations: &[DependencyViolation]) -> RepairAssessment { + let owned = violations_requiring(violations, ENGINE_NAME); + if owned.is_empty() { + return RepairAssessment::default(); + } + let mut notes = vec![ + "the runtime environment did not satisfy vLLM's pinned dependencies; vLLM was reinstalled to restore them".to_owned(), + ]; + // One note per violation rather than one joined line. The real failure is the whole + // torch stack — torch, torchvision and torchaudio move together when the SDK writes + // over the engine's pins — so joining them produced a single ~380-character line that + // is unreadable in a terminal. Mirrors the per-finding `violation:` lines the + // CLI-side renderer already emits. + notes.extend( + owned + .iter() + .map(|violation| format!("violation: {}", violation.detail)), + ); + notes.push( + "if this recurs after `rocm install sdk`, the SDK torch stack is being written over vLLM's pinned torch".to_owned(), + ); + RepairAssessment { + needed: true, + notes, + } +} + +fn unverified_repair(reason: &str) -> RepairAssessment { + RepairAssessment { + needed: false, + notes: vec![format!( + "vLLM's pinned dependencies could not be verified in this environment: {reason}" + )], + } +} + fn runtime_is_managed(runtime: &VllmRuntime) -> bool { runtime.source.starts_with("managed_runtime_manifest") } @@ -2403,6 +2516,149 @@ mod tests { resolve_vllm_install_target(index.map(ToOwned::to_owned)) } + fn violation(requiring: &str, detail: &str) -> DependencyViolation { + DependencyViolation { + requiring: requiring.to_owned(), + detail: detail.to_owned(), + } + } + + #[test] + fn a_repair_targets_the_environment_that_was_assessed() { + // `resolve_managed_runtime_python` returns the first candidate unconditionally, + // while the assessment walked on to the candidate that actually has vLLM. Both + // match when `runtime_id` matching is by prefix, so the repair must follow the + // assessed environment or it fixes an interpreter nobody found broken. + let assessed = VllmRuntime { + runtime_id: "nightly-wheel-gfx94x-dcgpu-7-14-0a20260611".to_owned(), + env_id: "external-vllm-therock".to_owned(), + command: PathBuf::from("/rocm/runtimes/wheel/nightly-gfx94x/bin/vllm"), + python_executable: Some(PathBuf::from( + "/rocm/runtimes/wheel/nightly-gfx94x/bin/python", + )), + version: Some("0.26.0".to_owned()), + source: "managed_runtime_manifest:nightly-wheel-gfx94x-dcgpu".to_owned(), + sdk_root: None, + sdk_bin: None, + sdk_bin_paths: Vec::new(), + sdk_library_paths: Vec::new(), + }; + + let target = assessed_python_for_repair(&assessed).expect("a managed runtime has a python"); + + assert_eq!( + target.python_executable, + PathBuf::from("/rocm/runtimes/wheel/nightly-gfx94x/bin/python") + ); + assert_eq!( + target.runtime_id, "nightly-wheel-gfx94x-dcgpu-7-14-0a20260611", + "the assessed runtime's own id, not the prefix the caller asked for" + ); + } + + #[test] + fn a_consistent_environment_is_not_reinstalled() { + assert_eq!(repair_from_violations(&[]), RepairAssessment::default()); + } + + #[test] + fn a_replaced_pinned_torch_forces_a_reinstall() { + // A second `rocm install sdk` writes the SDK's torch over the build + // vLLM pins. vLLM still imports, so resolution alone cannot see the breakage. + let assessment = repair_from_violations(&[violation( + "vllm", + "The package `vllm` requires `torch==2.10.0+git8514f05`, but `2.9.1+rocm7.14.0a20260611` is installed", + )]); + + assert!(assessment.needed); + assert!( + assessment + .notes + .iter() + .any(|note| note.contains("2.10.0+git8514f05")), + "the reported note names the pin that was violated: {:?}", + assessment.notes + ); + } + + #[test] + fn the_whole_replaced_torch_stack_is_reported_one_finding_per_line() { + // What the failure actually looks like on hardware: the SDK moves torch, + // torchvision and torchaudio together, so all three pins are violated at + // once. Joining them into a single note produced one ~380-character line; + // each finding gets its own so a terminal can show them. + let assessment = repair_from_violations(&[ + violation( + "vllm", + "The package `vllm` requires `torch==2.11.0+gitd0c8b1f`, but `2.11.0+rocm7.13.0` is installed", + ), + violation( + "vllm", + "The package `vllm` requires `torchvision==0.24.1+d801a34`, but `0.26.0+rocm7.13.0` is installed", + ), + violation( + "vllm", + "The package `vllm` requires `torchaudio==2.9.0+eaa9e4e`, but `2.11.0+rocm7.13.0` is installed", + ), + ]); + + assert!(assessment.needed); + let violation_notes: Vec<&String> = assessment + .notes + .iter() + .filter(|note| note.starts_with("violation: ")) + .collect(); + assert_eq!( + violation_notes.len(), + 3, + "every violated pin gets its own note: {:?}", + assessment.notes + ); + for package in ["torch==", "torchvision==", "torchaudio=="] { + assert!( + violation_notes.iter().any(|note| note.contains(package)), + "{package} is missing from the reported notes: {:?}", + assessment.notes + ); + } + assert!( + assessment.notes.iter().all(|note| note.len() < 200), + "no note should be a wall of joined findings: {:?}", + assessment.notes + ); + } + + #[test] + fn unrelated_upstream_conflicts_do_not_force_a_reinstall() { + // These environments routinely carry conflicts between third-party packages. + // Reinstalling vLLM would not resolve them, so they must not trigger one. + let assessment = repair_from_violations(&[ + violation( + "tilelang", + "The package `tilelang` requires `cloudpickle>=3.0`, but `2.2.1` is installed", + ), + violation( + "torch", + "The package `torch` requires `sympy>=1.13`, but `1.12` is installed", + ), + ]); + + assert_eq!(assessment, RepairAssessment::default()); + } + + #[test] + fn an_unrunnable_check_reports_itself_without_forcing_a_reinstall() { + let assessment = unverified_repair("uv binary is unavailable"); + + assert!(!assessment.needed); + assert_eq!(assessment.notes.len(), 1); + assert!( + assessment.notes[0].contains("could not be verified"), + "{:?}", + assessment.notes + ); + } + #[test] fn vllm_extra_index_url_defaults_to_const() { assert_eq!( diff --git a/tests/e2e-cucumber/features/runtime_setup.feature b/tests/e2e-cucumber/features/runtime_setup.feature index e81eb8fac..61d837de7 100644 --- a/tests/e2e-cucumber/features/runtime_setup.feature +++ b/tests/e2e-cucumber/features/runtime_setup.feature @@ -19,6 +19,19 @@ Feature: Runtime configuration When the user inspects the system Then the managed runtime folder path is not recursively nested + # The SDK and the engine share one Python environment, so a second + # `install sdk` wrote the SDK's torch stack over the build the engine pins. The + # engine still resolved, so the install reported success and every health surface + # kept saying `ready` — the first signal was a serve failure naming neither. Needs + # a real SDK install, a real engine install, and a second SDK install, so it runs + # on the nightly GPU lane. `@requires-engine:vllm` because only vLLM shares the + # runtime environment; Lemonade manages its own. + @id:runtime-sdk-reinstall-keeps-engine-consistent @requires-gpu @requires-engine:vllm @nightly + Scenario: 4 - Reinstalling the SDK leaves the installed engine's requirements satisfied + Given a managed runtime with an inference engine already installed + When the user installs the SDK again + Then the install reports the engine's requirements as satisfied + # Linux-only: the step adopts a standard `/opt/rocm` install with a Unix python # path. On Windows those paths don't exist (the CLI resolves `/usr/bin/python3` # to a bogus `C:/usr/bin/python3` and errors on the missing path before it can diff --git a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs index cf678b565..6f22a8c66 100644 --- a/tests/e2e-cucumber/tests/e2e/runtime_steps.rs +++ b/tests/e2e-cucumber/tests/e2e/runtime_steps.rs @@ -47,12 +47,61 @@ async fn setup_active_runtime(world: &mut E2eWorld) { ); } +#[given("a managed runtime with an inference engine already installed")] +async fn setup_runtime_with_engine(world: &mut E2eWorld) { + // Share the runtimes tree for the same reason `a managed runtime is active` does: + // the first scenario to find it empty pays for the multi-GiB SDK pull, the rest + // reuse it. `install sdk` auto-installs the family's preferred engine, so one + // install satisfies both halves of this precondition. + world.use_shared_runtimes(); + let (stdout, _, _) = crate::run_rocm(world, &["runtimes", "list"]); + if stdout.contains("installed: none") { + crate::run_rocm_ok(world, &["install", "sdk"]); + } + assert_engine_ready(world); +} + #[when("the user installs the SDK")] async fn user_installs_sdk(world: &mut E2eWorld) { let stdout = crate::run_rocm_ok(world, &["install", "sdk"]); world.cli_output = Some(stdout); } +#[when("the user installs the SDK again")] +async fn user_reinstalls_sdk(world: &mut E2eWorld) { + user_installs_sdk(world).await; +} + +#[then("the install reports the engine's requirements as satisfied")] +async fn assert_engine_requirements_satisfied(world: &mut E2eWorld) { + let output = world.cli_output.as_deref().expect("no install output"); + assert!( + !output.contains("dependency_check: violated"), + "the reinstall left the engine's declared requirements unmet:\n{output}" + ); + assert!( + output.contains("dependency_check: satisfied"), + "the install did not report on the engine's requirements at all:\n{output}" + ); +} + +/// The engine inventory reports a usable engine runtime. +/// +/// A precondition only. It deliberately has no Then counterpart: `engines list` +/// reports `runtime: ready` even while the engine's pinned dependencies are +/// violated — that false green is the very thing this feature's scenario exists +/// to catch — so asserting it afterwards would pass whether or not the fix +/// works. Teaching that surface to notice a violated pin is tracked separately; +/// until it does, the `dependency_check: satisfied` assertion is the only +/// falsifiable signal available. +fn assert_engine_ready(world: &mut E2eWorld) { + let (stdout, _, _) = crate::run_rocm(world, &["engines", "list"]); + assert!( + stdout.contains("runtime: ready"), + "no engine runtime is ready:\n{stdout}" + ); +} + #[when("the user tries to adopt the existing install")] async fn user_tries_adopt(world: &mut E2eWorld) { let (stdout, stderr, rc) = crate::run_rocm(