chore: simplify and clarify server, CLI, and integration tests

apps/skit-cli/src/client.rs

@@ -216,7 +216,6 @@ pub async fn process_oneshot_with_client(
         "Starting oneshot pipeline processing"
     );
 
-    // Validate input files exist
     if !Path::new(pipeline_path).exists() {
         return Err(format!("Pipeline file not found: {pipeline_path}").into());
     }
@@ -230,7 +229,6 @@ pub async fn process_oneshot_with_client(
     debug!("Reading pipeline configuration from {pipeline_path}");
     let pipeline_content = fs::read_to_string(pipeline_path).await?;
 
-    // Create multipart form
     let mut form = multipart::Form::new().text("config", pipeline_content);
     for input in inputs {
         debug!("Reading input media file from {}", input.path);
@@ -256,20 +254,17 @@ pub async fn process_oneshot_with_client(
         form = form.part(input.field.clone(), part);
     }
 
-    // Send request to server
     let url = http_base_url(server_url)?.join("/api/v1/process")?;
 
     info!("Sending request to {url}");
     let response = client.post(url).multipart(form).send().await?;
 
-    // Check response status
     if !response.status().is_success() {
         let status = response.status();
         let error_text = response.text().await.unwrap_or_default();
         return Err(format!("Server returned error {status}: {error_text}").into());
     }
 
-    // Get content type for logging
     let content_type =
         response.headers().get("content-type").and_then(|ct| ct.to_str().ok()).unwrap_or("unknown");
 
@@ -341,22 +336,19 @@ pub async fn create_session(
     // Prepare HTTP request body
     let request_body = CreateSessionRequest { name: name.clone(), yaml: pipeline_content };
 
-    // Send HTTP POST request
     let client = reqwest::Client::new();
     let url = http_base_url(server_url)?.join("/api/v1/sessions")?;
 
     info!("Sending HTTP POST request to {url}");
     let response = client.post(url).json(&request_body).send().await?;
 
-    // Check response status
     if !response.status().is_success() {
         let status = response.status();
         let error_text = response.text().await.unwrap_or_default();
         error!("Failed to create session: {status} - {error_text}");
         return Err(format!("Server returned error {status}: {error_text}").into());
     }
 
-    // Parse response
     let result: CreateSessionResponse = response.json().await?;
     let session_id = result.session_id;
     let session_name = result.name;
@@ -455,10 +447,8 @@ pub async fn tune_node(
         "Tuning node parameter"
     );
 
-    // Parse the parameter value as YAML
     let param_value: serde_json::Value = serde_saphyr::from_str(value)?;
 
-    // Create a JSON object with the single parameter
     let mut params = serde_json::Map::new();
     params.insert(param.to_string(), param_value);
     let update_params = serde_json::Value::Object(params);

apps/skit-cli/src/load_test/mod.rs

@@ -33,7 +33,6 @@ pub async fn run_load_test(
     duration_override: Option<u64>,
     cleanup: bool,
 ) -> Result<()> {
-    // Load and parse config
     let mut config = LoadTestConfig::from_file(config_path)
         .with_context(|| format!("Failed to load config from {config_path}"))?;
 
@@ -68,7 +67,6 @@ pub async fn run_load_test(
         info!("Dynamic sessions: {}", config.dynamic.session_count);
     }
 
-    // Set up graceful shutdown handler
     let shutdown_token = tokio_util::sync::CancellationToken::new();
     let shutdown_handle = shutdown_token.clone();
     let ctrl_c_handle = tokio::spawn(async move {
@@ -87,7 +85,6 @@ pub async fn run_load_test(
         populate_environment(&config).await?;
     }
 
-    // Initialize metrics collector
     let metrics = MetricsCollector::new();
 
     // Run the appropriate scenario

apps/skit-cli/src/load_test/populate.rs

@@ -13,14 +13,12 @@ use crate::load_test::config::LoadTestConfig;
 pub async fn populate_environment(config: &LoadTestConfig) -> Result<()> {
     let client = reqwest::Client::new();
 
-    // Load native plugins
     for plugin_path in &config.populate.plugins_native {
         if let Err(e) = load_native_plugin(&client, &config.server.url, plugin_path).await {
             warn!("Failed to load native plugin {}: {}", plugin_path, e);
         }
     }
 
-    // Load WASM plugins
     for plugin_path in &config.populate.plugins_wasm {
         if let Err(e) = load_wasm_plugin(&client, &config.server.url, plugin_path).await {
             warn!("Failed to load WASM plugin {}: {}", plugin_path, e);

apps/skit-cli/src/load_test/workers.rs

@@ -31,7 +31,6 @@ pub async fn oneshot_worker(
     let pipeline_path = &config.oneshot.pipeline;
     let input_path = &config.oneshot.input_file;
 
-    // Create a temp output path that we won't actually write
     let output_path = if cfg!(windows) { "NUL" } else { "/dev/null" };
 
     loop {
@@ -297,7 +296,6 @@ async fn create_session_with_pipeline(
         session_id: String,
     }
 
-    // Generate a unique suffix for this session to avoid conflicts
     let unique_id = rand::rng()
         .sample_iter(&Alphanumeric)
         .take(8)
@@ -318,7 +316,6 @@ async fn create_session_with_pipeline(
     // Prepare the request
     let request = CreateSessionRequest { yaml, name: Some(session_name.to_string()) };
 
-    // Send HTTP POST to /api/v1/sessions
     let client = reqwest::Client::new();
     let url = format!("{server_url}/api/v1/sessions");
     let response = client.post(&url).json(&request).send().await?;
@@ -563,7 +560,6 @@ pub async fn session_tuner_worker(
                                     rand::rng().random_range(0..session.tunable_node_ids.len());
                                 let node_id = &session.tunable_node_ids[node_idx];
 
-                                // Generate random gain value between 0.5 and 2.0
                                 let gain_value = rand::rng().random_range(0.5..2.0);
 
                                 ws.tune_node(

apps/skit-cli/src/main.rs

@@ -343,7 +343,6 @@ enum ControlCommands {
 
 #[tokio::main]
 async fn main() {
-    // Initialize basic logging for client
     tracing_subscriber::fmt::init();
 
     let cli = Cli::parse();

apps/skit-cli/src/shell.rs

@@ -235,7 +235,6 @@ impl Shell {
     ///
     /// Panics if URL scheme conversion fails (unreachable in practice as "ws" and "wss" are always valid)
     pub fn new(server_url: &str) -> Result<Self, Box<dyn std::error::Error + Send + Sync>> {
-        // Parse server URL to convert to ws:// or wss://
         let mut ws_url = Url::parse(server_url)?;
         match ws_url.scheme() {
             // set_scheme only fails for invalid schemes, but "ws" and "wss" are always valid
@@ -270,7 +269,6 @@ impl Shell {
         editor.bind_sequence(KeyEvent::alt('n'), Cmd::HistorySearchForward);
         editor.bind_sequence(KeyEvent::alt('p'), Cmd::HistorySearchBackward);
 
-        // Load history
         if editor.load_history(".skit_history").is_err() {
             debug!("No previous history found");
         }
@@ -332,7 +330,6 @@ impl Shell {
             }
         }
 
-        // Save history
         if let Err(e) = self.editor.save_history(".skit_history") {
             warn!("Failed to save history: {e}");
         }
@@ -397,12 +394,10 @@ impl Shell {
     async fn refresh_sessions(&mut self) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
         let sessions = self.fetch_sessions().await?;
 
-        // Update completions
         if let Some(helper) = self.editor.helper_mut() {
             helper.completer.update_sessions(sessions.clone());
         }
 
-        // Use clone_from for efficient assignment
         self.current_sessions.clone_from(&sessions);
 
         Ok(())
@@ -424,12 +419,10 @@ impl Shell {
             }
         }
 
-        // Update completions
         if let Some(helper) = self.editor.helper_mut() {
             helper.completer.update_sessions(sessions.clone());
         }
 
-        // Use clone_from for efficient assignment
         self.current_sessions.clone_from(&sessions);
 
         Ok(())
@@ -500,7 +493,6 @@ impl Shell {
         let pipeline_path = args[0];
         let mut name = None;
 
-        // Parse optional --name argument
         let mut i = 1;
         while i < args.len() {
             match args[i] {
@@ -520,7 +512,6 @@ impl Shell {
             }
         }
 
-        // Convert WebSocket URL to HTTP URL for the create_session function
         let http_url = self
             .ws_url
             .replace("ws://", "http://")
@@ -612,17 +603,12 @@ impl Shell {
 
         println!("🚀 Processing oneshot pipeline: {input_path} → {pipeline_path} → {output_path}");
 
-        // Convert WebSocket URL back to HTTP URL for the oneshot HTTP API
-        // ws://host:port/api/v1/control -> http://host:port
-        // wss://host:port/api/v1/control -> https://host:port
         let http_url = self
             .ws_url
             .replace("ws://", "http://")
             .replace("wss://", "https://")
             .replace("/api/v1/control", "");
 
-        // Use the existing process_oneshot function from client.rs
-        // This makes a multipart HTTP POST to /api/v1/process
         let inputs = vec![crate::client::InputFile {
             field: "media".to_string(),
             path: input_path.to_string(),

apps/skit/src/assets.rs

@@ -31,22 +31,18 @@ const ALLOWED_AUDIO_FORMATS: &[&str] = &["opus", "ogg", "flac", "mp3", "wav"];
 
 /// Validates a filename for security
 fn validate_audio_filename(filename: &str) -> Result<String, AssetsError> {
-    // Check length
     if filename.len() > MAX_FILENAME_LENGTH {
         return Err(AssetsError::InvalidFilename("Filename too long".to_string()));
     }
 
-    // Check if empty
     if filename.is_empty() {
         return Err(AssetsError::InvalidFilename("Filename cannot be empty".to_string()));
     }
 
-    // Check for path traversal attempts
     if filename.contains("..") || filename.contains('/') || filename.contains('\\') {
         return Err(AssetsError::InvalidFilename("Invalid characters in filename".to_string()));
     }
 
-    // Extract extension and validate it's an audio format
     let extension = filename
         .rsplit('.')
         .next()
@@ -79,7 +75,6 @@ async fn read_license_file(license_path: &PathBuf) -> Option<String> {
     use std::fmt::Write as _;
 
     fs::read_to_string(license_path).await.map_or(None, |contents| {
-        // Extract relevant info from SPDX license file
         let mut license_info = String::new();
         // REUSE-IgnoreStart
         for line in contents.lines() {
@@ -117,25 +112,20 @@ async fn process_audio_entry(
 
     let filename = path.file_name().and_then(|s| s.to_str())?.to_string();
 
-    // Validate extension
     let extension = path.extension().and_then(|s| s.to_str()).map(str::to_lowercase)?;
 
     if !ALLOWED_AUDIO_FORMATS.contains(&extension.as_str()) {
         return None;
     }
 
-    // Get file metadata
     let metadata = fs::metadata(&path).await.ok()?;
     let size_bytes = metadata.len();
 
-    // Generate ID from full filename (including extension) to ensure uniqueness
     let id = filename.clone();
 
-    // Generate display name from filename without extension
     let name_without_ext = filename.trim_end_matches(&format!(".{extension}"));
     let display_name = name_without_ext.replace(['_', '-'], " ");
 
-    // Check permissions
     let asset_path_str = if is_system {
         format!("samples/audio/system/{filename}")
     } else {
@@ -170,7 +160,6 @@ async fn scan_audio_directory(
 ) -> Result<Vec<AudioAsset>, AssetsError> {
     let mut assets = Vec::new();
 
-    // Check if directory exists
     if !dir_path.exists() {
         warn!("Audio directory does not exist: {:?}", dir_path);
         return Ok(assets);
@@ -442,7 +431,6 @@ async fn delete_audio_files(
         .await
         .map_err(|e| AssetsError::IoError(format!("Failed to delete file: {e}")))?;
 
-    // Delete license file if it exists
     let license_path = file_path.with_extension(format!("{extension}.license"));
     if license_path.exists() {
         if let Err(e) = fs::remove_file(&license_path).await {
@@ -469,7 +457,6 @@ pub async fn delete_asset_handler(
     let user_dir = base_path.join("user");
     let file_path = user_dir.join(&id);
 
-    // Extract extension from filename
     let extension = match id.rsplit('.').next() {
         Some(ext) => ext.to_string(),
         None => return AssetsError::NotFound(id).into_response(),
@@ -945,7 +932,6 @@ async fn serve_image_asset_handler(
             .into_response();
     }
 
-    // Validate scope
     if scope != "user" && scope != "system" {
         return AssetsError::InvalidFilename(
             "Invalid scope: must be 'user' or 'system'".to_string(),
@@ -1208,7 +1194,6 @@ async fn process_font_upload(
     let written_bytes = stream_field_to_file(field, &file_path, MAX_FONT_FILE_SIZE).await?;
     create_license_sidecar(&file_path, &extension).await;
 
-    // Validate that the uploaded file is actually a font by checking magic bytes.
     let header = match fs::read(&file_path).await {
         Ok(data) if data.len() >= 4 => data[..4].to_vec(),
         Ok(_) => {

apps/skit/src/auth/extractor.rs

@@ -86,7 +86,6 @@ impl MaybeAuth {
 /// Checks the Authorization header first (Bearer token format),
 /// then falls back to the configured cookie name.
 pub fn extract_token(headers: &HeaderMap, config: &Config) -> Option<String> {
-    // Try Authorization header first: "Bearer <token>"
     if let Some(auth_header) = headers.get(AUTHORIZATION) {
         if let Ok(auth_str) = auth_header.to_str() {
             if let Some(token) = auth_str.strip_prefix("Bearer ") {
@@ -95,7 +94,6 @@ pub fn extract_token(headers: &HeaderMap, config: &Config) -> Option<String> {
         }
     }
 
-    // Fall back to cookie
     if let Some(cookie_header) = headers.get(COOKIE) {
         if let Ok(cookie_str) = cookie_header.to_str() {
             let cookie_name = &config.auth.cookie_name;
@@ -129,7 +127,6 @@ pub async fn validate_token_from_headers(
     config: &Config,
     permissions_config: &crate::permissions::PermissionsConfig,
 ) -> Result<AuthContext, (StatusCode, String)> {
-    // Extract token
     let token = extract_token(headers, config).ok_or_else(|| {
         (StatusCode::UNAUTHORIZED, "No authentication token provided".to_string())
     })?;
@@ -147,21 +144,18 @@ pub async fn validate_token(
     auth_state: &AuthState,
     permissions_config: &crate::permissions::PermissionsConfig,
 ) -> Result<AuthContext, (StatusCode, String)> {
-    // Validate JWT
     let claims = auth_state
         .validate_api_token(token)
         .map_err(|e| (StatusCode::UNAUTHORIZED, format!("Invalid token: {e}")))?;
 
     let token_hash = super::hash_token(token);
 
-    // Check revocation
     if let Some(revocation_store) = auth_state.revocation_store() {
         if revocation_store.is_revoked(&token_hash) {
             return Err((StatusCode::UNAUTHORIZED, "Token has been revoked".to_string()));
         }
     }
 
-    // Check "tokens we mint" enforcement
     if let Some(metadata_store) = auth_state.token_metadata_store() {
         if !metadata_store.exists(&claims.jti).await {
             return Err((
@@ -175,7 +169,6 @@ pub async fn validate_token(
         return Err((StatusCode::UNAUTHORIZED, "Token has unknown role".to_string()));
     }
 
-    // Get permissions for the role
     let permissions = permissions_config.get_role(&claims.role);
 
     Ok(AuthContext { role: claims.role.clone(), claims, permissions })