fixup! cache issues when dl files

Signed-off-by: Max Olender <molender@nvidia.com>

Author: maxo-nv

crates/rvs/src/artifact/io.rs

@@ -128,6 +128,10 @@ async fn download_one(
 ) -> Result<(), RvsError> {
     let path = std::path::Path::new(&artifact.output_path);
 
+    // Cache hit: trust that a non-tmp file at `path` is complete, because we
+    // only rename into place after a fully streamed body (and checksum, when
+    // advertised) succeeds. We do NOT re-verify on hit -- a stable URL is
+    // assumed to map to stable bytes for the lifetime of the cache.
     if path.exists() {
         tracing::debug!(path = artifact.output_path, "artifact: cache hit, skipping");
         return Ok(());
@@ -159,7 +163,12 @@ async fn download_one(
         .and_then(|v| v.to_str().ok())
         .map(str::to_lowercase);
 
-    let mut file = tokio::fs::File::create(path).await?;
+    // Stream to a sibling `.partial` file and rename on success, so an
+    // interrupted download never poisons the cache with a truncated file.
+    // Append (not `with_extension`) so `foo.bin` and `foo.json` get distinct
+    // tmp paths instead of colliding on `foo.partial`.
+    let tmp_path = std::path::PathBuf::from(format!("{}.partial", artifact.output_path));
+    let mut file = tokio::fs::File::create(&tmp_path).await?;
     let mut hasher = Sha256::new();
     let mut stream = response.bytes_stream();
     while let Some(chunk) = stream.next().await {
@@ -168,19 +177,22 @@ async fn download_one(
         hasher.update(&chunk);
         file.write_all(&chunk).await?;
     }
+    file.flush().await?;
 
     if let Some(expected) = expected_sha256 {
         let actual = hex::encode(hasher.finalize());
         if actual != expected {
+            let _ = tokio::fs::remove_file(&tmp_path).await;
             return Err(RvsError::ChecksumMismatch {
                 path: artifact.output_path.clone(),
                 expected,
                 actual,
             });
         }
-        tracing::debug!(path = artifact.output_path, "artifact: checksum OK");
+        tracing::info!(path = artifact.output_path, "artifact: checksum OK");
     }
 
+    tokio::fs::rename(&tmp_path, path).await?;
     Ok(())
 }
 

crates/rvs/src/bin/carbide-rvs.rs

@@ -109,13 +109,13 @@ async fn main() -> Result<(), RvsError> {
     let validation_cancel_token = cancel_token.clone();
 
     tokio::spawn(async move {
+        let Ok(mut sigint) = signal(SignalKind::interrupt()) else {
+            return;
+        };
+        let Ok(mut sigterm) = signal(SignalKind::terminate()) else {
+            return;
+        };
         loop {
-            let Ok(mut sigint) = signal(SignalKind::interrupt()) else {
-                break;
-            };
-            let Ok(mut sigterm) = signal(SignalKind::terminate()) else {
-                break;
-            };
             // Wait for SIGINT or SIGTERM
             let received_signal = tokio::select! {
                 _ = sigint.recv() => "SIGINT",

crates/rvs/src/scenario/mod.rs

@@ -27,9 +27,9 @@ pub struct Artifact {
     pub name: String,
     pub output: String,
     /// Direct download URL (mutually exclusive with `sotpath`).
-    // TODO[#416]: enforce exactly one of `uri`/`sotpath` is set - add a
-    // post-deserialization validation step in Scenario::load or a custom
-    // Deserialize impl. Currently both can be set (or neither) without error.
+    ///
+    /// Exactly one of `uri`/`sotpath` must be set; enforced in
+    /// `Scenario::load` after deserialization.
     pub uri: Option<String>,
     /// JSONPath into SOT JSON to resolve download URL.
     pub sotpath: Option<String>,
@@ -74,7 +74,33 @@ impl Scenario {
     pub fn load(path: &Path) -> Result<Self, String> {
         let content =
             std::fs::read_to_string(path).map_err(|e| format!("read {}: {e}", path.display()))?;
-        toml::from_str(&content).map_err(|e| format!("parse {}: {e}", path.display()))
+        let scenario: Scenario =
+            toml::from_str(&content).map_err(|e| format!("parse {}: {e}", path.display()))?;
+        scenario
+            .validate()
+            .map_err(|e| format!("validate {}: {e}", path.display()))?;
+        Ok(scenario)
+    }
+
+    fn validate(&self) -> Result<(), String> {
+        for artifact in &self.artifacts {
+            match (&artifact.uri, &artifact.sotpath) {
+                (Some(_), Some(_)) => {
+                    return Err(format!(
+                        "artifact '{}': both 'uri' and 'sotpath' set; exactly one required",
+                        artifact.name
+                    ));
+                }
+                (None, None) => {
+                    return Err(format!(
+                        "artifact '{}': neither 'uri' nor 'sotpath' set; exactly one required",
+                        artifact.name
+                    ));
+                }
+                _ => {}
+            }
+        }
+        Ok(())
     }
 }
 
@@ -96,4 +122,55 @@ mod tests {
         assert_eq!(scenario.teardown.len(), 1);
         assert_eq!(scenario.test[0].name, "nv_basic");
     }
+
+    fn scenario_with_artifacts(artifacts: Vec<Artifact>) -> Scenario {
+        Scenario {
+            rack: RackTarget {
+                model: "gb200nvl".to_string(),
+                sot_release: "1.2.5".to_string(),
+            },
+            os: OsImage { uri: "https://example.com/os.img".to_string() },
+            artifacts,
+            setup: vec![],
+            test: vec![],
+            teardown: vec![],
+        }
+    }
+
+    fn artifact(name: &str, uri: Option<&str>, sotpath: Option<&str>) -> Artifact {
+        Artifact {
+            name: name.to_string(),
+            output: format!("{name}.bin"),
+            uri: uri.map(str::to_string),
+            sotpath: sotpath.map(str::to_string),
+        }
+    }
+
+    #[test]
+    fn validate_accepts_uri_only() {
+        let s = scenario_with_artifacts(vec![artifact("a", Some("https://x/y"), None)]);
+        assert!(s.validate().is_ok());
+    }
+
+    #[test]
+    fn validate_accepts_sotpath_only() {
+        let s = scenario_with_artifacts(vec![artifact("a", None, Some("$.foo"))]);
+        assert!(s.validate().is_ok());
+    }
+
+    #[test]
+    fn validate_rejects_both_uri_and_sotpath() {
+        let s = scenario_with_artifacts(vec![artifact("a", Some("https://x/y"), Some("$.foo"))]);
+        let err = s.validate().unwrap_err();
+        assert!(err.contains("both"), "got: {err}");
+        assert!(err.contains("'a'"), "got: {err}");
+    }
+
+    #[test]
+    fn validate_rejects_neither_uri_nor_sotpath() {
+        let s = scenario_with_artifacts(vec![artifact("a", None, None)]);
+        let err = s.validate().unwrap_err();
+        assert!(err.contains("neither"), "got: {err}");
+        assert!(err.contains("'a'"), "got: {err}");
+    }
 }