fix: store validated PathBuf, remove dead code, delete duplicate SubscribeConfig

Addresses review comments:
- Store validated PathBuf from validate_safe_output_dir instead of
  discarding it (output_dir is now Option<PathBuf>)
- Remove duplicate SubscribeConfig from events/mod.rs
- Delete unused validate_msg_format (clap value_parser handles this)
- Remove all #[allow(dead_code)] annotations

Author: jpoehnelt-bot

AGENTS.md

@@ -85,7 +85,7 @@ When adding new helpers or CLI flags that accept file paths, **always validate**
 |---|---|---|
 | File path for writing (`--output-dir`) | `validate::validate_safe_output_dir()` | Absolute paths, `../` traversal, symlinks outside CWD, control chars |
 | File path for reading (`--dir`) | `validate::validate_safe_dir_path()` | Absolute paths, `../` traversal, control chars |
-| Enum/allowlist values (`--msg-format`) | `validate::validate_msg_format()` or clap `value_parser` | Any value not in the allowlist |
+| Enum/allowlist values (`--msg-format`) | clap `value_parser` (see `gmail/mod.rs`) | Any value not in the allowlist |
 
 ```rust
 // In your argument parser:

docs/CONTRIBUTING.md

@@ -83,7 +83,7 @@ This CLI is designed to be invoked by AI/LLM agents, so all user-supplied inputs
 | Embedding a value in a URL path segment | `helpers::encode_path_segment()` |
 | Passing query parameters | reqwest `.query()` builder (never string interpolation) |
 | Using a resource name in a URL (`--project`, `--space`) | `helpers::validate_resource_name()` |
-| Accepting an enum flag (`--msg-format`) | clap `value_parser` or `validate::validate_msg_format()` |
+| Accepting an enum flag (`--msg-format`) | clap `value_parser` (see `gmail/mod.rs`) |
 
 ### Testing Expectations
 

src/helpers/events/mod.rs

@@ -49,32 +49,6 @@ impl std::fmt::Display for SubscriptionName {
     }
 }
 
-#[allow(dead_code)]
-#[derive(Debug, Clone, Builder)]
-#[builder(setter(into))]
-pub struct SubscribeConfig {
-    #[builder(default)]
-    pub target: Option<String>,
-    #[builder(default)]
-    pub event_types: Vec<String>,
-    #[builder(default)]
-    pub project: Option<ProjectId>,
-    #[builder(default)]
-    pub subscription: Option<SubscriptionName>,
-    #[builder(default = "10")]
-    pub max_messages: u32,
-    #[builder(default = "5")]
-    pub poll_interval: u64,
-    #[builder(default = "false")]
-    pub once: bool,
-    #[builder(default = "false")]
-    pub cleanup: bool,
-    #[builder(default = "false")]
-    pub no_ack: bool,
-    #[builder(default)]
-    pub output_dir: Option<String>,
-}
-
 impl Helper for EventsHelper {
     fn inject_commands(
         &self,

src/helpers/events/subscribe.rs

@@ -1,4 +1,5 @@
 use super::*;
+use std::path::PathBuf;
 
 #[derive(Debug, Clone, Default, Builder)]
 #[builder(setter(into))]
@@ -22,7 +23,7 @@ pub struct SubscribeConfig {
     #[builder(default)]
     no_ack: bool,
     #[builder(default)]
-    output_dir: Option<String>,
+    output_dir: Option<PathBuf>,
 }
 
 fn parse_subscribe_args(matches: &ArgMatches) -> Result<SubscribeConfig, GwsError> {
@@ -66,8 +67,7 @@ fn parse_subscribe_args(matches: &ArgMatches) -> Result<SubscribeConfig, GwsErro
     builder.cleanup(matches.get_flag("cleanup"));
     builder.no_ack(matches.get_flag("no-ack"));
     if let Some(output_dir) = matches.get_one::<String>("output-dir") {
-        crate::validate::validate_safe_output_dir(output_dir)?;
-        builder.output_dir(Some(output_dir.clone()));
+        builder.output_dir(Some(crate::validate::validate_safe_output_dir(output_dir)?));
     }
 
     let config = builder
@@ -355,11 +355,11 @@ async fn pull_loop(
                     .duration_since(std::time::UNIX_EPOCH)
                     .map(|d| d.as_millis())
                     .unwrap_or(0);
-                let path = format!("{dir}/{ts}_{file_counter}.json");
+                let path = dir.join(format!("{ts}_{file_counter}.json"));
                 if let Err(e) = std::fs::write(&path, &json_str) {
-                    eprintln!("Warning: failed to write {path}: {e}");
+                    eprintln!("Warning: failed to write {}: {e}", path.display());
                 } else {
-                    eprintln!("Wrote {path}");
+                    eprintln!("Wrote {}", path.display());
                 }
             } else {
                 println!(

src/helpers/gmail/watch.rs

@@ -295,7 +295,7 @@ async fn watch_pull_loop(
                 gmail_token,
                 *last_history_id,
                 &config.format,
-                config.output_dir.as_deref(),
+                config.output_dir.as_ref(),
                 sanitize_config,
             )
             .await?;
@@ -377,7 +377,7 @@ async fn fetch_and_output_messages(
     gmail_token: &str,
     start_history_id: u64,
     msg_format: &str,
-    output_dir: Option<&str>,
+    output_dir: Option<&std::path::PathBuf>,
     sanitize_config: &crate::helpers::modelarmor::SanitizeConfig,
 ) -> Result<(), GwsError> {
     let url = format!(
@@ -432,11 +432,11 @@ async fn fetch_and_output_messages(
                 let json_str =
                     serde_json::to_string_pretty(&full_msg).unwrap_or_else(|_| "{}".to_string());
                 if let Some(dir) = output_dir {
-                    let path = format!("{dir}/{msg_id}.json");
+                    let path = dir.join(format!("{msg_id}.json"));
                     if let Err(e) = std::fs::write(&path, &json_str) {
-                        eprintln!("Warning: failed to write {path}: {e}");
+                        eprintln!("Warning: failed to write {}: {e}", path.display());
                     } else {
-                        eprintln!("Wrote {path}");
+                        eprintln!("Wrote {}", path.display());
                     }
                 } else {
                     println!(
@@ -512,7 +512,7 @@ struct WatchConfig {
     format: String,
     once: bool,
     cleanup: bool,
-    output_dir: Option<String>,
+    output_dir: Option<std::path::PathBuf>,
 }
 
 fn parse_watch_args(matches: &ArgMatches) -> Result<WatchConfig, GwsError> {
@@ -522,10 +522,10 @@ fn parse_watch_args(matches: &ArgMatches) -> Result<WatchConfig, GwsError> {
         .unwrap_or("full");
     // Note: msg-format is already constrained by clap's value_parser
 
-    let output_dir = matches.get_one::<String>("output-dir").cloned();
-    if let Some(ref dir) = output_dir {
-        crate::validate::validate_safe_output_dir(dir)?;
-    }
+    let output_dir = matches
+        .get_one::<String>("output-dir")
+        .map(|dir| crate::validate::validate_safe_output_dir(dir))
+        .transpose()?;
 
     Ok(WatchConfig {
         project: matches.get_one::<String>("project").cloned(),

src/validate.rs

@@ -21,27 +21,6 @@
 use crate::error::GwsError;
 use std::path::{Path, PathBuf};
 
-/// Allowed values for Gmail message format (`--msg-format`).
-#[allow(dead_code)]
-pub const VALID_MSG_FORMATS: &[&str] = &["full", "metadata", "minimal", "raw"];
-
-/// Validates that `value` is one of the allowed Gmail message formats.
-///
-/// Returns the validated value on success, or a descriptive
-/// [`GwsError::Validation`] listing the allowed options.
-#[allow(dead_code)]
-pub fn validate_msg_format(value: &str) -> Result<&str, GwsError> {
-    if VALID_MSG_FORMATS.contains(&value) {
-        Ok(value)
-    } else {
-        Err(GwsError::Validation(format!(
-            "Invalid message format '{}'. Allowed values: {}",
-            value,
-            VALID_MSG_FORMATS.join(", ")
-        )))
-    }
-}
-
 /// Validates that `dir` is a safe output directory.
 ///
 /// The path is resolved relative to CWD. The function rejects paths that
@@ -194,36 +173,6 @@ mod tests {
     use std::fs;
     use tempfile::tempdir;
 
-    // --- validate_msg_format ---
-
-    #[test]
-    fn test_valid_msg_formats() {
-        for fmt in VALID_MSG_FORMATS {
-            assert!(
-                validate_msg_format(fmt).is_ok(),
-                "expected '{fmt}' to be valid"
-            );
-        }
-    }
-
-    #[test]
-    fn test_invalid_msg_format() {
-        let err = validate_msg_format("FULL").unwrap_err();
-        let msg = err.to_string();
-        assert!(msg.contains("Invalid message format"), "got: {msg}");
-        assert!(msg.contains("full, metadata, minimal, raw"));
-    }
-
-    #[test]
-    fn test_empty_msg_format() {
-        assert!(validate_msg_format("").is_err());
-    }
-
-    #[test]
-    fn test_msg_format_injection_attempt() {
-        assert!(validate_msg_format("full&extra=1").is_err());
-    }
-
     // --- validate_safe_output_dir ---
 
     #[test]