cli: Fix non-idiomatic Rust patterns across services

Replace `let _ = ` patterns that silently ignore Results with proper
error handling using `if let Err(e)` and warning logs. Convert
`.to_string()` calls on string literals to `String::from()` for
clarity. Add explanatory comments for `.clone()` calls that are
required by borrow checker constraints.

Co-authored-by: SCE <sce@crocoder.dev>

Author: davidabram

cli/build.rs

@@ -105,21 +105,23 @@ fn generate_embedded_asset_manifest() -> io::Result<()> {
         collect_files(&source_root, &source_root, &mut files)?;
         files.sort_unstable_by(|a, b| a.relative_path.cmp(&b.relative_path));
 
-        let _ = writeln!(
+        writeln!(
             output,
             "pub static {}: &[EmbeddedAsset] = &[",
             target.const_name
-        );
+        )
+        .expect("writing to String buffer should never fail");
 
         for file in &files {
             println!("cargo:rerun-if-changed={}", file.absolute_path.display());
-            let _ = writeln!(
+            writeln!(
                 output,
                 "    EmbeddedAsset {{ relative_path: \"{}\", bytes: include_bytes!(concat!(env!(\"CARGO_MANIFEST_DIR\"), \"{}{}\")) }},",
                 escape_for_rust_string(&file.relative_path),
                 target.include_prefix,
                 escape_for_rust_string(&file.relative_path),
-            );
+            )
+            .expect("writing to String buffer should never fail");
         }
 
         output.push_str("];\n\n");

cli/src/app.rs

@@ -122,7 +122,8 @@ where
     let styled_message =
         services::style::error_text(&services::security::redact_sensitive_text(&rendered));
 
-    let _ = writeln!(writer, "{styled_heading} [{styled_code}]: {styled_message}");
+    writeln!(writer, "{styled_heading} [{styled_code}]: {styled_message}")
+        .expect("writing error diagnostic to writer should not fail");
 }
 
 #[allow(clippy::too_many_lines)]
@@ -345,7 +346,7 @@ fn classify_clap_error(error: &clap::Error) -> ClassifiedError {
 }
 
 fn render_subcommand_help_from_args(args: &[String]) -> Option<(String, String)> {
-    let command_name = args.get(1)?.clone();
+    let command_name = args.get(1)?.to_owned();
     let command_path = args[1..]
         .iter()
         .take_while(|arg| !arg.starts_with('-'))
@@ -651,10 +652,11 @@ fn dispatch(
 ) -> Result<String, ClassifiedError> {
     match command {
         Command::Help => Ok(command_surface::help_text()),
-        Command::HelpText { text, .. } => Ok(text.clone()),
+        Command::HelpText { text, .. } => Ok(text.to_owned()),
         Command::Auth(request) => services::auth_command::run_auth_subcommand(*request)
             .map_err(|error| ClassifiedError::runtime(error.to_string())),
         Command::Completion(request) => Ok(services::completion::render_completion(*request)),
+        // Clone required: run_config_subcommand takes ownership of ConfigSubcommand
         Command::Config(subcommand) => services::config::run_config_subcommand(subcommand.clone())
             .map_err(|error| ClassifiedError::runtime(error.to_string())),
         Command::Setup(request) => {
@@ -710,8 +712,10 @@ fn dispatch(
         }
         Command::Doctor(request) => services::doctor::run_doctor(*request)
             .map_err(|error| ClassifiedError::runtime(error.to_string())),
+        // Clone required: run_hooks_subcommand takes ownership of HookSubcommand
         Command::Hooks(subcommand) => services::hooks::run_hooks_subcommand(subcommand.clone())
             .map_err(|error| ClassifiedError::runtime(error.to_string())),
+        // Clone required: run_trace_subcommand takes ownership of TraceRequest
         Command::Trace(request) => services::trace::run_trace_subcommand(request.clone())
             .map_err(|error| ClassifiedError::runtime(error.to_string())),
         Command::Sync(request) => services::sync::run_placeholder_sync(*request)

cli/src/cli_schema.rs

@@ -24,6 +24,8 @@ pub fn render_help_for_path(path: &[&str]) -> Option<String> {
     let mut command = Cli::command();
 
     for segment in path {
+        // Clone required: find_subcommand_mut returns a mutable reference that cannot
+        // be kept alive across loop iterations, so we must clone to get an owned value
         command = command.find_subcommand_mut(segment)?.clone();
     }
 

cli/src/command_surface.rs

@@ -97,7 +97,7 @@ pub fn help_text() -> String {
             command_name(command.name),
             command.purpose
         )
-        .unwrap();
+        .expect("writing to String should never fail");
     }
 
     format!(

cli/src/services/auth.rs

@@ -66,7 +66,7 @@ pub struct TokenResponse {
 }
 
 fn default_token_type() -> String {
-    "Bearer".to_string()
+    String::from("Bearer")
 }
 
 fn default_access_token_expires_in() -> u64 {
@@ -193,7 +193,7 @@ pub async fn ensure_valid_token(
 
     let Some(stored) = load_tokens()? else {
         return Err(AuthError::Unauthorized(
-            "No stored WorkOS credentials were found. Try: run 'sce login' before running authenticated commands.".to_string(),
+            String::from("No stored WorkOS credentials were found. Try: run 'sce login' before running authenticated commands."),
         ));
     };
 
@@ -220,7 +220,7 @@ pub async fn renew_stored_token(
 
     let Some(stored) = load_tokens()? else {
         return Err(AuthError::Unauthorized(
-            "No stored WorkOS credentials were found. Try: run 'sce auth login' before running authenticated commands.".to_string(),
+            String::from("No stored WorkOS credentials were found. Try: run 'sce auth login' before running authenticated commands."),
         ));
     };
 
@@ -257,9 +257,9 @@ pub async fn request_device_authorization(
             || parsed.user_code.trim().is_empty()
             || parsed.verification_uri.trim().is_empty()
         {
-            return Err(AuthError::InvalidResponse(
-                "device authorization response is missing required fields".to_string(),
-            ));
+            return Err(AuthError::InvalidResponse(String::from(
+                "device authorization response is missing required fields",
+            )));
         }
         return Ok(parsed);
     }
@@ -299,7 +299,7 @@ async fn poll_for_device_token(
         attempts = attempts.saturating_add(1);
         if attempts > max_polls {
             return Err(AuthError::Unauthorized(
-                "WorkOS device authorization expired before approval completed. Try: run 'sce login' again and complete verification before the code expires.".to_string(),
+                String::from("WorkOS device authorization expired before approval completed. Try: run 'sce login' again and complete verification before the code expires."),
             ));
         }
 

cli/src/services/config.rs

@@ -1677,12 +1677,12 @@ fn format_bash_policies_text(value: &ResolvedOptionalValue<BashPolicyConfig>) ->
     match (value.value.as_ref(), value.source) {
         (Some(config), Some(source)) => {
             let presets = if config.presets.is_empty() {
-                "(none)".to_string()
+                String::from("(none)")
             } else {
                 config.presets.join(", ")
             };
             let custom = if config.custom.is_empty() {
-                "(none)".to_string()
+                String::from("(none)")
             } else {
                 config
                     .custom
@@ -1918,7 +1918,7 @@ fn format_optional_auth_resolved_value_json(
 
 fn format_text_display_value(key: &str, value: &str) -> String {
     if should_fully_redact_text_value(key) {
-        return "[REDACTED]".to_string();
+        return String::from("[REDACTED]");
     }
 
     if looks_credential_like(value) {

cli/src/services/doctor.rs

@@ -289,8 +289,8 @@ fn build_report_with_dependencies(
             category: ProblemCategory::RepositoryTargeting,
             severity: ProblemSeverity::Error,
             fixability: ProblemFixability::ManualOnly,
-            summary: "Git is not available on this machine.".to_string(),
-            remediation: "Install an accessible 'git' binary and ensure it is on PATH before rerunning 'sce doctor'.".to_string(),
+            summary: String::from("Git is not available on this machine."),
+            remediation: String::from("Install an accessible 'git' binary and ensure it is on PATH before rerunning 'sce doctor'."),
             next_action: "manual_steps",
         });
         Vec::new()
@@ -299,8 +299,8 @@ fn build_report_with_dependencies(
             category: ProblemCategory::RepositoryTargeting,
             severity: ProblemSeverity::Error,
             fixability: ProblemFixability::ManualOnly,
-            summary: "The current repository is bare and does not support local SCE hook rollout.".to_string(),
-            remediation: "Run 'sce doctor' from a non-bare working tree clone to inspect repo-scoped SCE hook health.".to_string(),
+            summary: String::from("The current repository is bare and does not support local SCE hook rollout."),
+            remediation: String::from("Run 'sce doctor' from a non-bare working tree clone to inspect repo-scoped SCE hook health."),
             next_action: "manual_steps",
         });
         Vec::new()
@@ -309,8 +309,8 @@ fn build_report_with_dependencies(
             category: ProblemCategory::RepositoryTargeting,
             severity: ProblemSeverity::Error,
             fixability: ProblemFixability::ManualOnly,
-            summary: "The current directory is not inside a git repository.".to_string(),
-            remediation: "Run 'sce doctor' from inside the target repository working tree to inspect repo-scoped SCE hook health.".to_string(),
+            summary: String::from("The current directory is not inside a git repository."),
+            remediation: String::from("Run 'sce doctor' from inside the target repository working tree to inspect repo-scoped SCE hook health."),
             next_action: "manual_steps",
         });
         Vec::new()
@@ -321,8 +321,8 @@ fn build_report_with_dependencies(
             category: ProblemCategory::RepositoryTargeting,
             severity: ProblemSeverity::Error,
             fixability: ProblemFixability::ManualOnly,
-            summary: "Unable to resolve git hooks directory.".to_string(),
-            remediation: "Verify that git repository inspection succeeds and rerun 'sce doctor' inside a non-bare git repository.".to_string(),
+            summary: String::from("Unable to resolve git hooks directory."),
+            remediation: String::from("Verify that git repository inspection succeeds and rerun 'sce doctor' inside a non-bare git repository."),
             next_action: "manual_steps",
         });
         Vec::new()
@@ -379,7 +379,7 @@ fn collect_global_state_health(
             severity: ProblemSeverity::Error,
             fixability: ProblemFixability::ManualOnly,
             summary: format!("Unable to resolve expected state root: {error}"),
-            remediation: "Verify that the current platform exposes a writable SCE state directory before rerunning 'sce doctor'.".to_string(),
+            remediation: String::from("Verify that the current platform exposes a writable SCE state directory before rerunning 'sce doctor'."),
             next_action: "manual_steps",
         }),
     }
@@ -415,7 +415,7 @@ fn collect_global_state_health(
             severity: ProblemSeverity::Error,
             fixability: ProblemFixability::ManualOnly,
             summary: format!("Unable to resolve expected global config path: {error}"),
-            remediation: "Verify that the current platform exposes a writable SCE config directory before rerunning 'sce doctor'.".to_string(),
+            remediation: String::from("Verify that the current platform exposes a writable SCE config directory before rerunning 'sce doctor'."),
             next_action: "manual_steps",
         }),
     }
@@ -465,7 +465,7 @@ fn collect_global_state_health(
                 severity: ProblemSeverity::Error,
                 fixability: ProblemFixability::ManualOnly,
                 summary: format!("Unable to resolve expected Agent Trace local DB path: {error}"),
-                remediation: "Verify that the SCE state root can be resolved on this machine before rerunning 'sce doctor'.".to_string(),
+                remediation: String::from("Verify that the SCE state root can be resolved on this machine before rerunning 'sce doctor'."),
                 next_action: "manual_steps",
             });
             None
@@ -493,7 +493,7 @@ fn inspect_agent_trace_db_health(
                 "Agent Trace local DB path '{}' has no parent directory.",
                 db_health.path.display()
             ),
-            remediation: "Verify that the SCE state root resolves to a normal filesystem path before rerunning 'sce doctor'.".to_string(),
+            remediation: String::from("Verify that the SCE state root resolves to a normal filesystem path before rerunning 'sce doctor'."),
             next_action: "manual_steps",
         });
         return;
@@ -1241,7 +1241,7 @@ fn run_auto_fixes(
             fix_results.push(DoctorFixResultRecord {
                 category: ProblemCategory::HookRollout,
                 outcome: FixResult::Failed,
-                detail: "Automatic hook repair could not start because the repository root was not resolved during diagnosis.".to_string(),
+                detail: String::from("Automatic hook repair could not start because the repository root was not resolved during diagnosis."),
             });
             return fix_results;
         };
@@ -1273,7 +1273,7 @@ fn run_filesystem_auto_fixes(
         return vec![DoctorFixResultRecord {
             category: ProblemCategory::FilesystemPermissions,
             outcome: FixResult::Failed,
-            detail: "Automatic Agent Trace directory repair could not start because the expected local DB path was not resolved during diagnosis.".to_string(),
+            detail: String::from("Automatic Agent Trace directory repair could not start because the expected local DB path was not resolved during diagnosis."),
         }];
     };
 
@@ -1464,7 +1464,10 @@ mod tests {
 
     impl Drop for TestDir {
         fn drop(&mut self) {
-            let _ = fs::remove_dir_all(&self.path);
+            // Best-effort cleanup in test harness; ignore errors since we can't propagate them
+            if let Err(e) = fs::remove_dir_all(&self.path) {
+                eprintln!("Warning: Failed to clean up test directory: {e}");
+            }
         }
     }
 
@@ -1489,7 +1492,7 @@ mod tests {
             severity: ProblemSeverity::Error,
             fixability: ProblemFixability::AutoFixable,
             summary: summary.to_string(),
-            remediation: "Run 'sce doctor --fix'.".to_string(),
+            remediation: String::from("Run 'sce doctor --fix'."),
             next_action: "doctor_fix",
         }
     }

cli/src/services/hooks.rs

@@ -183,7 +183,7 @@ fn run_git_command(repository_root: &Path, args: &[&str], context_message: &str)
     if !output.status.success() {
         let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
         let diagnostic = if stderr.is_empty() {
-            "git command exited with a non-zero status".to_string()
+            String::from("git command exited with a non-zero status")
         } else {
             stderr
         };
@@ -215,7 +215,7 @@ fn run_git_command_allow_empty(
     if !output.status.success() {
         let stderr = String::from_utf8_lossy(&output.stderr).trim().to_string();
         let diagnostic = if stderr.is_empty() {
-            "git command exited with a non-zero status".to_string()
+            String::from("git command exited with a non-zero status")
         } else {
             stderr
         };
@@ -262,9 +262,11 @@ fn collect_pending_checkpoint(repository_root: &Path) -> Result<PendingCheckpoin
 
     let mut all_paths = HashSet::new();
     for path in staged_ranges.keys() {
+        // Clone required: HashSet::insert requires ownership of the key
         all_paths.insert(path.clone());
     }
     for path in unstaged_ranges.keys() {
+        // Clone required: HashSet::insert requires ownership of the key
         all_paths.insert(path.clone());
     }
 
@@ -959,7 +961,7 @@ fn load_post_commit_checkpoint_files(repository_root: &Path) -> Result<Vec<FileA
         files.push(FileAttributionInput {
             path: path.to_string(),
             conversations: vec![ConversationInput {
-                url: "https://crocoder.dev/sce/local-hooks/pre-commit-checkpoint".to_string(),
+                url: String::from("https://crocoder.dev/sce/local-hooks/pre-commit-checkpoint"),
                 related: Vec::new(),
                 ranges,
             }],
@@ -2104,7 +2106,7 @@ async fn write_trace_record_to_local_db(
         .metadata
         .get(METADATA_QUALITY_STATUS)
         .cloned()
-        .unwrap_or_else(|| "final".to_string());
+        .unwrap_or_else(|| String::from("final"));
 
     conn.execute(
         "INSERT INTO trace_records (repository_id, commit_id, trace_id, version, content_type, notes_ref, payload_json, quality_status, idempotency_key, recorded_at) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)",
@@ -2326,7 +2328,10 @@ impl TraceEmissionLedger for FileTraceEmissionLedger {
             .append(true)
             .open(&self.path)
         {
-            let _ = writeln!(file, "{commit_sha}");
+            // Best-effort append to notes; errors are logged but don't fail the commit
+            if let Err(e) = writeln!(file, "{commit_sha}") {
+                eprintln!("Warning: Failed to append commit SHA to notes: {e}");
+            }
         }
     }
 }

cli/src/services/observability.rs

@@ -107,6 +107,7 @@ impl TelemetryRuntime {
     ) -> Result<Self> {
         Self::from_config(&TelemetryConfig {
             enabled: config.otel_enabled,
+            // Clone required: TelemetryConfig owns the endpoint String
             endpoint: config.otel_endpoint.clone(),
             protocol: match config.otel_protocol {
                 config::OtlpProtocol::Grpc => OtlpProtocol::Grpc,
@@ -132,11 +133,13 @@ impl TelemetryRuntime {
         let exporter = match config.protocol {
             OtlpProtocol::Grpc => opentelemetry_otlp::SpanExporter::builder()
                 .with_tonic()
+                // Clone required: with_endpoint takes ownership of the endpoint String
                 .with_endpoint(config.endpoint.clone())
                 .build()
                 .map_err(|error| anyhow!("Failed to initialize OTLP gRPC exporter: {error}"))?,
             OtlpProtocol::HttpProtobuf => opentelemetry_otlp::SpanExporter::builder()
                 .with_http()
+                // Clone required: with_endpoint takes ownership of the endpoint String
                 .with_endpoint(config.endpoint.clone())
                 .build()
                 .map_err(|error| anyhow!("Failed to initialize OTLP HTTP exporter: {error}"))?,
@@ -169,7 +172,10 @@ impl TelemetryRuntime {
 impl Drop for TelemetryRuntime {
     fn drop(&mut self) {
         if let Some(provider) = self.provider.take() {
-            let _ = provider.shutdown();
+            // Best-effort shutdown during drop; errors are logged but not propagated
+            if let Err(e) = provider.shutdown() {
+                eprintln!("Warning: Failed to shutdown telemetry provider: {e:?}");
+            }
         }
     }
 }

cli/src/services/security.rs

@@ -171,7 +171,14 @@ pub fn ensure_directory_is_writable(path: &std::path::Path, context: &str) -> Re
             )
         })?;
 
-    let _ = std::fs::remove_file(&probe_path);
+    // Clean up probe file; best-effort with warning on failure
+    if let Err(e) = std::fs::remove_file(&probe_path) {
+        eprintln!(
+            "Warning: Failed to clean up write-probe file '{}': {}",
+            probe_path.display(),
+            e
+        );
+    }
     Ok(())
 }