refactor(diagnostic): move diagnostic UX out of core nono crate (#1155)

Relocate DiagnosticFormatter and all user-facing diagnostic rendering from
the core 'nono' crate to 'nono-cli', restoring the library/CLI boundary
documented in AGENTS.md: the library stays a pure sandbox primitive and the
CLI owns policy, footer text, flag suggestions, and stderr heuristics.

Core 'nono::diagnostic' now exposes only structured, policy-free facts:
DenialReason, DenialRecord, IpcDenialRecord, SandboxViolation, and the
seatbelt_operation_to_access mapping. Everything else (DiagnosticFormatter,
PolicyExplanation, ErrorObservation, analyze_error_output, and the footer
rendering with 'nono why'/'nono learn' guidance and CLI flag suggestions)
moves to crates/nono-cli/src/diagnostic/.

The bulk of this change is a near-verbatim move; the only behavioral edits:

- Remove dead code surfaced by the move (previously masked by pub visibility
  in the library): PolicyExplanation.details/policy_source fields (written but
  never read), and the unused DiagnosticFormatter methods
  with_blocked_protected_file, detect_protected_file_in_error, and
  format_summary.
- Deduplicate suggested_flag_parts: the formatter now uses the single
  query_ext implementation, which guards against suggesting access to the
  root filesystem. This fixes an edge case where a nonexistent path directly
  under '/' could yield a '--allow /' suggestion.

No change to diagnostic footer output in normal cases; the moved unit tests
are the rendering contract and pass unchanged.

Signed-off-by: Aleksy Siek <aleksy@alwaysfurther.ai>

Author: SequeI

crates/nono-cli/src/diagnostic/formatter.rs

@@ -0,0 +1,13 @@
+//! CLI-owned diagnostic rendering and stderr heuristics.
+//!
+//! The core `nono::diagnostic` module owns the structured denial records; this
+//! module owns all user-facing diagnostic UX: the `nono diagnostic` footer,
+//! CLI flag suggestions, policy explanations, and best-effort parsing of a
+//! command's own error output.
+
+mod formatter;
+
+pub use formatter::{
+    CommandContext, DiagnosticFormatter, DiagnosticMode, ErrorObservation, PolicyExplanation,
+    analyze_error_output,
+};

crates/nono-cli/src/diagnostic/mod.rs

@@ -14,6 +14,7 @@ mod env_sanitization;
 #[cfg(target_os = "linux")]
 mod supervisor_linux;
 
+use crate::diagnostic::{DiagnosticFormatter, DiagnosticMode};
 use crate::startup_prompt::{notify_startup_termination_for_child, print_terminal_safe_stderr};
 use crate::{DETACHED_CWD_PROMPT_RESPONSE_ENV, DETACHED_LAUNCH_ENV, DETACHED_SESSION_ID_ENV};
 use nix::libc;
@@ -22,9 +23,8 @@ use nix::sys::wait::{WaitPidFlag, WaitStatus, waitpid};
 use nix::unistd::{ForkResult, Pid, fork};
 use nono::supervisor::{ApprovalDecision, AuditEntry, SupervisorMessage, SupervisorResponse};
 use nono::{
-    ApprovalBackend, CapabilitySet, DenialReason, DenialRecord, DiagnosticFormatter,
-    DiagnosticMode, NonoError, Result, Sandbox, SupervisorListener, SupervisorSocket,
-    UnixSocketCapability, UnixSocketMode,
+    ApprovalBackend, CapabilitySet, DenialReason, DenialRecord, NonoError, Result, Sandbox,
+    SupervisorListener, SupervisorSocket, UnixSocketCapability, UnixSocketMode,
 };
 use std::collections::HashSet;
 use std::ffi::{CString, OsStr};
@@ -74,8 +74,8 @@ const MAX_TRACKED_REQUEST_IDS: usize = 4096;
 use crate::timeouts;
 
 struct ProfileSaveOffer<'a> {
-    policy_explanations: &'a [nono::diagnostic::PolicyExplanation],
-    error_observation: &'a nono::diagnostic::ErrorObservation,
+    policy_explanations: &'a [crate::diagnostic::PolicyExplanation],
+    error_observation: &'a crate::diagnostic::ErrorObservation,
     caps: &'a CapabilitySet,
     command: &'a [String],
     compared_profile: Option<&'a str>,
@@ -1281,7 +1281,7 @@ pub fn execute_supervised(
             let error_observation = pty_proxy
                 .as_ref()
                 .map(|p| {
-                    nono::diagnostic::analyze_error_output(
+                    crate::diagnostic::analyze_error_output(
                         &p.screen_plaintext(),
                         config.protected_paths,
                         Some(config.current_dir),
@@ -1371,7 +1371,7 @@ pub fn execute_supervised(
                     )
                     .with_canonical_denial_paths(canonical_denial_paths);
                 if let Some(program) = config.command.first() {
-                    formatter = formatter.with_command(nono::diagnostic::CommandContext {
+                    formatter = formatter.with_command(crate::diagnostic::CommandContext {
                         program: program.clone(),
                         resolved_path: config.resolved_program.to_path_buf(),
                         args: nono::scrub_argv_with_policy(config.command, redaction_policy),
@@ -1421,9 +1421,9 @@ fn build_policy_explanations(
     denials: &[nono::diagnostic::DenialRecord],
     sandbox_violations: &[nono::SandboxViolation],
     caps: &nono::CapabilitySet,
-) -> Vec<nono::diagnostic::PolicyExplanation> {
+) -> Vec<crate::diagnostic::PolicyExplanation> {
+    use crate::diagnostic::PolicyExplanation;
     use nono::AccessMode;
-    use nono::diagnostic::PolicyExplanation;
     use std::collections::BTreeMap;
 
     // Merge access modes per path so a path denied for both Read and Write
@@ -1475,17 +1475,13 @@ fn build_policy_explanations(
         match crate::query_ext::query_path(&path, access, caps, &[]) {
             Ok(crate::query_ext::QueryResult::Denied {
                 reason,
-                details,
-                policy_source,
                 suggested_flag,
                 ..
             }) => {
                 explanations.push(PolicyExplanation {
                     path,
                     access,
                     reason,
-                    details,
-                    policy_source,
                     suggested_flag,
                 });
             }
@@ -1535,7 +1531,7 @@ fn should_print_diagnostic_footer(
     denials: &[nono::diagnostic::DenialRecord],
     ipc_denials: &[nono::diagnostic::IpcDenialRecord],
     sandbox_violations: &[nono::SandboxViolation],
-    error_observation: &nono::diagnostic::ErrorObservation,
+    error_observation: &crate::diagnostic::ErrorObservation,
 ) -> bool {
     !no_diagnostics
         && (exit_code != 0
@@ -1566,8 +1562,8 @@ fn filter_suppressed_system_service_violations(
 fn should_offer_profile_save(
     no_diagnostics: bool,
     exit_code: i32,
-    policy_explanations: &[nono::diagnostic::PolicyExplanation],
-    error_observation: &nono::diagnostic::ErrorObservation,
+    policy_explanations: &[crate::diagnostic::PolicyExplanation],
+    error_observation: &crate::diagnostic::ErrorObservation,
     sandbox_violations: &[nono::SandboxViolation],
 ) -> bool {
     !no_diagnostics
@@ -3723,7 +3719,7 @@ mod tests {
             target: Some("/tmp/secret.txt".to_string()),
         }];
         let denials = Vec::new();
-        let observation = nono::diagnostic::ErrorObservation::default();
+        let observation = crate::diagnostic::ErrorObservation::default();
 
         assert!(should_print_diagnostic_footer(
             false,
@@ -3745,15 +3741,13 @@ mod tests {
 
     #[test]
     fn test_profile_save_prompt_triggers_on_policy_explanation_with_zero_exit() {
-        let explanations = vec![nono::diagnostic::PolicyExplanation {
+        let explanations = vec![crate::diagnostic::PolicyExplanation {
             path: PathBuf::from("/tmp/secret.txt"),
             access: nono::AccessMode::Read,
             reason: "path_not_granted".to_string(),
-            details: None,
-            policy_source: None,
             suggested_flag: Some("--read-file /tmp/secret.txt".to_string()),
         }];
-        let observation = nono::diagnostic::ErrorObservation::default();
+        let observation = crate::diagnostic::ErrorObservation::default();
 
         assert!(should_offer_profile_save(
             false,
@@ -3767,7 +3761,7 @@ mod tests {
     #[test]
     fn test_profile_save_prompt_triggers_on_user_preferences_violation_with_zero_exit() {
         let explanations = Vec::new();
-        let observation = nono::diagnostic::ErrorObservation::default();
+        let observation = crate::diagnostic::ErrorObservation::default();
         let violations = vec![nono::SandboxViolation {
             operation: "user-preference-read".to_string(),
             target: Some("kcfpreferencesanyapplication".to_string()),
@@ -3785,7 +3779,7 @@ mod tests {
     #[test]
     fn test_suppressed_system_service_violations_do_not_offer_profile_save() {
         let explanations = Vec::new();
-        let observation = nono::diagnostic::ErrorObservation::default();
+        let observation = crate::diagnostic::ErrorObservation::default();
         let violations = vec![nono::SandboxViolation {
             operation: "forbidden-exec-sugid".to_string(),
             target: None,
@@ -3833,7 +3827,7 @@ mod tests {
     #[test]
     fn test_profile_save_prompt_preserves_nonzero_exit_behavior() {
         let explanations = Vec::new();
-        let observation = nono::diagnostic::ErrorObservation::default();
+        let observation = crate::diagnostic::ErrorObservation::default();
 
         assert!(should_offer_profile_save(
             false,

crates/nono-cli/src/exec_strategy.rs

@@ -20,6 +20,7 @@ mod credential_runtime;
 mod deprecated_policy;
 mod deprecated_schema;
 mod deprecation_warnings;
+mod diagnostic;
 mod exec_strategy;
 mod execution_runtime;
 #[cfg(unix)]

crates/nono-cli/src/main.rs

@@ -1,9 +1,9 @@
 use crate::command_display::format_command_line;
+use crate::diagnostic::{ErrorObservation, PolicyExplanation};
 use crate::theme;
 use crate::{profile, query_ext};
 use colored::Colorize;
 use nono::SandboxViolation;
-use nono::diagnostic::{ErrorObservation, PolicyExplanation};
 use nono::{AccessMode, CapabilitySet, NonoError, Result};
 use std::collections::{BTreeMap, BTreeSet};
 use std::io::{BufRead, IsTerminal, Write};
@@ -1675,8 +1675,6 @@ mod tests {
             path: target,
             access: AccessMode::Read,
             reason: "sensitive_path".to_string(),
-            details: None,
-            policy_source: None,
             suggested_flag: None,
         };
 
@@ -1710,16 +1708,12 @@ mod tests {
             path: target.clone(),
             access: AccessMode::Read,
             reason: "path_not_granted".to_string(),
-            details: None,
-            policy_source: None,
             suggested_flag: Some(format!("--read-file {}", target.display())),
         };
         let write = PolicyExplanation {
             path: target,
             access: AccessMode::Write,
             reason: "insufficient_access".to_string(),
-            details: None,
-            policy_source: None,
             suggested_flag: None,
         };
 
@@ -1754,16 +1748,12 @@ mod tests {
             path: ignored.clone(),
             access: AccessMode::Read,
             reason: "path_not_granted".to_string(),
-            details: None,
-            policy_source: None,
             suggested_flag: None,
         };
         let saved_explanation = PolicyExplanation {
             path: saved,
             access: AccessMode::Read,
             reason: "path_not_granted".to_string(),
-            details: None,
-            policy_source: None,
             suggested_flag: None,
         };
 
@@ -1794,8 +1784,6 @@ mod tests {
             path: target.clone(),
             access: AccessMode::Read,
             reason: "path_not_granted".to_string(),
-            details: None,
-            policy_source: None,
             suggested_flag: None,
         };
 

crates/nono-cli/src/profile_save_runtime.rs

@@ -560,7 +560,7 @@ pub fn print_result(result: &QueryResult) {
     }
 }
 
-fn suggested_flag_for_path(path: &Path, requested: AccessMode) -> String {
+pub(crate) fn suggested_flag_for_path(path: &Path, requested: AccessMode) -> String {
     let (flag, target) = suggested_flag_parts(path, requested);
     format!("{flag} {}", target.display())
 }