feat: add rules validate subcommand (#177)
## Summary - New `cull-gmail rules [RULES] validate` subcommand validates a rules file without executing any actions or requiring Gmail credentials - `Rules::validate()` checks each rule for: non-empty label set, valid retention period (parseable as `MessageAge`), valid action (`Trash`/`Delete`) - Cross-rule check for duplicate labels (same label in multiple rules) - Returns all issues collected, not just the first - Exits 0 if valid, non-zero (with issue list on stderr) if invalid - Missing rules file fails with an error rather than creating defaults New public API: `cull_gmail::ValidationIssue` enum with `Display` impl. ## Test plan - [x] Unit tests: `Rules::validate()` — valid rules, empty labels, invalid retention, empty retention, invalid action, duplicate labels, multiple issues collected - [x] CLI integration tests: valid file → exit 0; invalid file → exit non-zero + "Rule #1" in output; duplicate labels → exit non-zero + label name in output; missing file → exit non-zero - [x] `cargo fmt --check` clean - [x] `cargo clippy -- -D warnings` clean - [x] Full test suite: 130+ tests pass - [x] `cargo audit` clean ## Usage in gmail-culler CI The validation workflow can now run `cull-gmail rules validate` on the committed rules file to verify it is syntactically and semantically correct — without needing Gmail credentials or executing any rules. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
@@ -298,6 +298,13 @@ async fn run(args: Cli) -> Result<()> {
|
||||
return init_cli.run().await;
|
||||
}
|
||||
|
||||
// Handle `rules validate` before loading config: it needs no Gmail credentials.
|
||||
if let Some(SubCmds::Rules(ref rules_cli)) = args.sub_command
|
||||
&& let Some(result) = rules_cli.run_if_validate()
|
||||
{
|
||||
return result;
|
||||
}
|
||||
|
||||
// For all other commands, load config normally
|
||||
let (config, client_config) = get_config()?;
|
||||
|
||||
|
||||
@@ -106,11 +106,13 @@ use std::path::{Path, PathBuf};
|
||||
|
||||
mod config_cli;
|
||||
mod run_cli;
|
||||
mod validate_cli;
|
||||
|
||||
use cull_gmail::{GmailClient, Result, Rules};
|
||||
|
||||
use config_cli::ConfigCli;
|
||||
use run_cli::RunCli;
|
||||
use validate_cli::ValidateCli;
|
||||
|
||||
/// Available subcommands for rules management and execution.
|
||||
///
|
||||
@@ -157,6 +159,16 @@ enum SubCmds {
|
||||
/// lifecycle management based on configured retention policies.
|
||||
#[clap(name = "run")]
|
||||
Run(RunCli),
|
||||
|
||||
/// Validate a rules file without executing any actions.
|
||||
///
|
||||
/// Checks each rule for a non-empty label set, a valid retention period,
|
||||
/// and a valid action. Also reports duplicate labels across rules.
|
||||
///
|
||||
/// Exits 0 if all rules are valid, non-zero otherwise. Does not require
|
||||
/// Gmail API credentials.
|
||||
#[clap(name = "validate")]
|
||||
Validate(ValidateCli),
|
||||
}
|
||||
|
||||
/// Command-line interface for Gmail message retention rule management.
|
||||
@@ -267,6 +279,23 @@ impl RulesCli {
|
||||
self.run_with_rules_path(client, None).await
|
||||
}
|
||||
|
||||
/// If the selected subcommand is `validate`, runs it and returns `Some(result)`.
|
||||
///
|
||||
/// Returns `None` for all other subcommands so the caller can proceed with
|
||||
/// normal config/client initialisation. Validate is the only subcommand that
|
||||
/// does not require a Gmail client or application config.
|
||||
pub fn run_if_validate(&self) -> Option<Result<()>> {
|
||||
if let SubCmds::Validate(validate_cli) = &self.sub_command {
|
||||
let mut rules_path: Option<&Path> = None;
|
||||
if let Some(p) = &self.rules {
|
||||
rules_path = Some(p.as_path());
|
||||
}
|
||||
Some(validate_cli.run(rules_path))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
/// Executes the rules command with an optional custom rules path.
|
||||
///
|
||||
/// # Arguments
|
||||
@@ -284,11 +313,18 @@ impl RulesCli {
|
||||
}
|
||||
log::info!("Rules path: {rules_path:?}");
|
||||
|
||||
// Validate does not need a GmailClient and must not fall back to
|
||||
// creating default rules when the file is missing.
|
||||
if let SubCmds::Validate(validate_cli) = &self.sub_command {
|
||||
return validate_cli.run(rules_path);
|
||||
}
|
||||
|
||||
let rules = get_rules_from(rules_path)?;
|
||||
|
||||
match &self.sub_command {
|
||||
SubCmds::Config(config_cli) => config_cli.run(rules),
|
||||
SubCmds::Run(run_cli) => run_cli.run(client, rules).await,
|
||||
SubCmds::Validate(_) => unreachable!("handled above"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
45
crates/cull-gmail/src/cli/rules_cli/validate_cli.rs
Normal file
45
crates/cull-gmail/src/cli/rules_cli/validate_cli.rs
Normal file
@@ -0,0 +1,45 @@
|
||||
//! Validation subcommand for Gmail retention rules.
|
||||
//!
|
||||
//! Loads a rules file and checks each rule for correctness without
|
||||
//! executing any actions or making API calls. Exits with a non-zero
|
||||
//! status if any issues are found.
|
||||
|
||||
use clap::Parser;
|
||||
use std::path::Path;
|
||||
|
||||
use cull_gmail::Rules;
|
||||
|
||||
use crate::Result;
|
||||
|
||||
/// Validate a rules file without executing any actions.
|
||||
///
|
||||
/// Checks each rule for:
|
||||
/// - Non-empty label set
|
||||
/// - Valid retention period (e.g. `d:30`, `m:6`, `y:2`)
|
||||
/// - Valid action (`Trash` or `Delete`)
|
||||
///
|
||||
/// Also checks across rules for duplicate labels.
|
||||
///
|
||||
/// Exits 0 if all rules are valid, non-zero otherwise.
|
||||
#[derive(Debug, Parser)]
|
||||
pub struct ValidateCli {}
|
||||
|
||||
impl ValidateCli {
|
||||
pub fn run(&self, rules_path: Option<&Path>) -> Result<()> {
|
||||
let rules = Rules::load_from(rules_path)?;
|
||||
let issues = rules.validate();
|
||||
|
||||
if issues.is_empty() {
|
||||
println!("Rules are valid.");
|
||||
Ok(())
|
||||
} else {
|
||||
for issue in &issues {
|
||||
eprintln!("{issue}");
|
||||
}
|
||||
Err(cull_gmail::Error::FileIo(format!(
|
||||
"{} validation issue(s) found",
|
||||
issues.len()
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -23,7 +23,7 @@ pub use client_config::ClientConfig;
|
||||
pub use gmail_client::GmailClient;
|
||||
pub(crate) use gmail_client::MessageSummary;
|
||||
pub use retention::Retention;
|
||||
pub use rules::Rules;
|
||||
pub use rules::{Rules, ValidationIssue};
|
||||
|
||||
pub use eol_action::EolAction;
|
||||
pub use error::Error;
|
||||
|
||||
@@ -43,7 +43,7 @@
|
||||
|
||||
use std::{
|
||||
collections::BTreeMap,
|
||||
env,
|
||||
env, fmt,
|
||||
fs::{self, read_to_string},
|
||||
path::Path,
|
||||
};
|
||||
@@ -664,6 +664,116 @@ impl Rules {
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Validates all rules in the set and returns a list of issues found.
|
||||
///
|
||||
/// Checks each rule for:
|
||||
/// - Non-empty label set
|
||||
/// - Valid retention period string (parseable as a `MessageAge`)
|
||||
/// - Valid action string (parseable as an `EolAction`)
|
||||
///
|
||||
/// Also checks across rules for duplicate labels (the same label appearing
|
||||
/// in more than one rule).
|
||||
///
|
||||
/// Returns an empty `Vec` if all rules are valid.
|
||||
///
|
||||
/// # Examples
|
||||
///
|
||||
/// ```
|
||||
/// use cull_gmail::Rules;
|
||||
///
|
||||
/// let rules = Rules::new();
|
||||
/// let issues = rules.validate();
|
||||
/// assert!(issues.is_empty(), "Default rules should all be valid");
|
||||
/// ```
|
||||
pub fn validate(&self) -> Vec<ValidationIssue> {
|
||||
let mut issues = Vec::new();
|
||||
let mut seen_labels: BTreeMap<String, usize> = BTreeMap::new();
|
||||
|
||||
for rule in self.rules.values() {
|
||||
let id = rule.id();
|
||||
|
||||
if rule.labels().is_empty() {
|
||||
issues.push(ValidationIssue::EmptyLabels { rule_id: id });
|
||||
}
|
||||
|
||||
if MessageAge::parse(rule.retention()).is_none() {
|
||||
issues.push(ValidationIssue::InvalidRetention {
|
||||
rule_id: id,
|
||||
retention: rule.retention().to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
if rule.action().is_none() {
|
||||
issues.push(ValidationIssue::InvalidAction {
|
||||
rule_id: id,
|
||||
action: rule.action_str().to_string(),
|
||||
});
|
||||
}
|
||||
|
||||
for label in rule.labels() {
|
||||
if let Some(&other_id) = seen_labels.get(&label) {
|
||||
if other_id != id {
|
||||
issues.push(ValidationIssue::DuplicateLabel {
|
||||
label: label.clone(),
|
||||
});
|
||||
}
|
||||
} else {
|
||||
seen_labels.insert(label, id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
issues
|
||||
}
|
||||
}
|
||||
|
||||
/// An issue found during rules validation.
|
||||
#[derive(Debug, PartialEq)]
|
||||
pub enum ValidationIssue {
|
||||
/// A rule has no labels configured.
|
||||
EmptyLabels {
|
||||
/// The ID of the offending rule.
|
||||
rule_id: usize,
|
||||
},
|
||||
/// A rule has a retention string that cannot be parsed as a `MessageAge`.
|
||||
InvalidRetention {
|
||||
/// The ID of the offending rule.
|
||||
rule_id: usize,
|
||||
/// The unparseable retention string.
|
||||
retention: String,
|
||||
},
|
||||
/// A rule has an action string that cannot be parsed as an `EolAction`.
|
||||
InvalidAction {
|
||||
/// The ID of the offending rule.
|
||||
rule_id: usize,
|
||||
/// The unparseable action string.
|
||||
action: String,
|
||||
},
|
||||
/// The same label appears in more than one rule.
|
||||
DuplicateLabel {
|
||||
/// The duplicated label.
|
||||
label: String,
|
||||
},
|
||||
}
|
||||
|
||||
impl fmt::Display for ValidationIssue {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
ValidationIssue::EmptyLabels { rule_id } => {
|
||||
write!(f, "Rule #{rule_id}: no labels configured")
|
||||
}
|
||||
ValidationIssue::InvalidRetention { rule_id, retention } => {
|
||||
write!(f, "Rule #{rule_id}: invalid retention '{retention}'")
|
||||
}
|
||||
ValidationIssue::InvalidAction { rule_id, action } => {
|
||||
write!(f, "Rule #{rule_id}: invalid action '{action}'")
|
||||
}
|
||||
ValidationIssue::DuplicateLabel { label } => {
|
||||
write!(f, "Label '{label}' is used in multiple rules")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -993,6 +1103,160 @@ mod tests {
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
// --- validate() tests ---
|
||||
|
||||
#[test]
|
||||
fn test_validate_default_rules_are_valid() {
|
||||
setup_test_environment();
|
||||
let rules = Rules::new();
|
||||
let issues = rules.validate();
|
||||
assert!(
|
||||
issues.is_empty(),
|
||||
"Default rules should be valid, got: {issues:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_empty_labels_reported() {
|
||||
setup_test_environment();
|
||||
let toml_str = r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = "d:30"
|
||||
labels = []
|
||||
action = "Trash"
|
||||
"#;
|
||||
let rules: Rules = toml::from_str(toml_str).unwrap();
|
||||
let issues = rules.validate();
|
||||
assert!(
|
||||
issues
|
||||
.iter()
|
||||
.any(|i| matches!(i, ValidationIssue::EmptyLabels { rule_id: 1 })),
|
||||
"Expected EmptyLabels for rule #1, got: {issues:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_invalid_retention_reported() {
|
||||
setup_test_environment();
|
||||
let toml_str = r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = "invalid"
|
||||
labels = ["some-label"]
|
||||
action = "Trash"
|
||||
"#;
|
||||
let rules: Rules = toml::from_str(toml_str).unwrap();
|
||||
let issues = rules.validate();
|
||||
assert!(
|
||||
issues
|
||||
.iter()
|
||||
.any(|i| matches!(i, ValidationIssue::InvalidRetention { rule_id: 1, .. })),
|
||||
"Expected InvalidRetention for rule #1, got: {issues:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_empty_retention_reported() {
|
||||
setup_test_environment();
|
||||
let toml_str = r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = ""
|
||||
labels = ["some-label"]
|
||||
action = "Trash"
|
||||
"#;
|
||||
let rules: Rules = toml::from_str(toml_str).unwrap();
|
||||
let issues = rules.validate();
|
||||
assert!(
|
||||
issues
|
||||
.iter()
|
||||
.any(|i| matches!(i, ValidationIssue::InvalidRetention { rule_id: 1, .. })),
|
||||
"Expected InvalidRetention for empty retention in rule #1, got: {issues:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_invalid_action_reported() {
|
||||
setup_test_environment();
|
||||
let toml_str = r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = "d:30"
|
||||
labels = ["some-label"]
|
||||
action = "invalid-action"
|
||||
"#;
|
||||
let rules: Rules = toml::from_str(toml_str).unwrap();
|
||||
let issues = rules.validate();
|
||||
assert!(
|
||||
issues
|
||||
.iter()
|
||||
.any(|i| matches!(i, ValidationIssue::InvalidAction { rule_id: 1, .. })),
|
||||
"Expected InvalidAction for rule #1, got: {issues:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_duplicate_label_reported() {
|
||||
setup_test_environment();
|
||||
let toml_str = r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = "d:30"
|
||||
labels = ["shared-label"]
|
||||
action = "Trash"
|
||||
|
||||
[rules."2"]
|
||||
id = 2
|
||||
retention = "d:60"
|
||||
labels = ["shared-label"]
|
||||
action = "Trash"
|
||||
"#;
|
||||
let rules: Rules = toml::from_str(toml_str).unwrap();
|
||||
let issues = rules.validate();
|
||||
assert!(
|
||||
issues.iter().any(|i| matches!(
|
||||
i,
|
||||
ValidationIssue::DuplicateLabel { label }
|
||||
if label == "shared-label"
|
||||
)),
|
||||
"Expected DuplicateLabel for 'shared-label', got: {issues:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_multiple_issues_collected() {
|
||||
setup_test_environment();
|
||||
let toml_str = r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = ""
|
||||
labels = []
|
||||
action = "bad"
|
||||
"#;
|
||||
let rules: Rules = toml::from_str(toml_str).unwrap();
|
||||
let issues = rules.validate();
|
||||
// All three issues should be present for the one rule
|
||||
assert!(
|
||||
issues
|
||||
.iter()
|
||||
.any(|i| matches!(i, ValidationIssue::EmptyLabels { .. })),
|
||||
"Expected EmptyLabels"
|
||||
);
|
||||
assert!(
|
||||
issues
|
||||
.iter()
|
||||
.any(|i| matches!(i, ValidationIssue::InvalidRetention { .. })),
|
||||
"Expected InvalidRetention"
|
||||
);
|
||||
assert!(
|
||||
issues
|
||||
.iter()
|
||||
.any(|i| matches!(i, ValidationIssue::InvalidAction { .. })),
|
||||
"Expected InvalidAction"
|
||||
);
|
||||
}
|
||||
|
||||
// Integration tests for save/load would require file system setup
|
||||
// These are marked as ignore to avoid interference with actual config files
|
||||
#[test]
|
||||
|
||||
@@ -290,6 +290,11 @@ impl EolRule {
|
||||
EolAction::parse(&self.action)
|
||||
}
|
||||
|
||||
/// Returns the raw action string as stored in the rule.
|
||||
pub(crate) fn action_str(&self) -> &str {
|
||||
&self.action
|
||||
}
|
||||
|
||||
/// Returns a human-readable description of what this rule does.
|
||||
///
|
||||
/// The description includes the rule ID, the action that will be performed,
|
||||
|
||||
@@ -874,3 +874,127 @@ mod async_integration_tests {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// --- rules validate tests ---
|
||||
|
||||
#[cfg(test)]
|
||||
mod rules_validate_tests {
|
||||
use super::test_utils::CliTestFixture;
|
||||
use std::fs;
|
||||
|
||||
fn valid_rules_toml() -> &'static str {
|
||||
r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = "d:30"
|
||||
labels = ["test-label"]
|
||||
action = "Trash"
|
||||
"#
|
||||
}
|
||||
|
||||
fn invalid_rules_toml() -> &'static str {
|
||||
r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = ""
|
||||
labels = []
|
||||
action = "bad-action"
|
||||
"#
|
||||
}
|
||||
|
||||
fn duplicate_label_rules_toml() -> &'static str {
|
||||
r#"
|
||||
[rules."1"]
|
||||
id = 1
|
||||
retention = "d:30"
|
||||
labels = ["shared"]
|
||||
action = "Trash"
|
||||
|
||||
[rules."2"]
|
||||
id = 2
|
||||
retention = "d:60"
|
||||
labels = ["shared"]
|
||||
action = "Trash"
|
||||
"#
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rules_validate_valid_file_exits_zero() {
|
||||
let fixture = CliTestFixture::new().expect("Failed to create test fixture");
|
||||
let rules_file = fixture.temp_dir.path().join("rules.toml");
|
||||
fs::write(&rules_file, valid_rules_toml()).unwrap();
|
||||
|
||||
let output = fixture
|
||||
.execute_cli(&["rules", rules_file.to_str().unwrap(), "validate"], None)
|
||||
.expect("Failed to execute CLI");
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"Expected exit 0 for valid rules, got: {}\nstdout: {}\nstderr: {}",
|
||||
output.status,
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rules_validate_invalid_file_exits_nonzero() {
|
||||
let fixture = CliTestFixture::new().expect("Failed to create test fixture");
|
||||
let rules_file = fixture.temp_dir.path().join("rules.toml");
|
||||
fs::write(&rules_file, invalid_rules_toml()).unwrap();
|
||||
|
||||
let output = fixture
|
||||
.execute_cli(&["rules", rules_file.to_str().unwrap(), "validate"], None)
|
||||
.expect("Failed to execute CLI");
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"Expected non-zero exit for invalid rules"
|
||||
);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let combined = format!("{stdout}{stderr}");
|
||||
assert!(
|
||||
combined.contains("Rule #1"),
|
||||
"Expected issue output mentioning rule #1, got: {combined}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rules_validate_duplicate_label_exits_nonzero() {
|
||||
let fixture = CliTestFixture::new().expect("Failed to create test fixture");
|
||||
let rules_file = fixture.temp_dir.path().join("rules.toml");
|
||||
fs::write(&rules_file, duplicate_label_rules_toml()).unwrap();
|
||||
|
||||
let output = fixture
|
||||
.execute_cli(&["rules", rules_file.to_str().unwrap(), "validate"], None)
|
||||
.expect("Failed to execute CLI");
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"Expected non-zero exit for duplicate label rules"
|
||||
);
|
||||
let stdout = String::from_utf8_lossy(&output.stdout);
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let combined = format!("{stdout}{stderr}");
|
||||
assert!(
|
||||
combined.contains("shared"),
|
||||
"Expected output mentioning 'shared' label, got: {combined}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_rules_validate_missing_file_exits_nonzero() {
|
||||
let fixture = CliTestFixture::new().expect("Failed to create test fixture");
|
||||
let rules_file = fixture.temp_dir.path().join("nonexistent.toml");
|
||||
|
||||
let output = fixture
|
||||
.execute_cli(&["rules", rules_file.to_str().unwrap(), "validate"], None)
|
||||
.expect("Failed to execute CLI");
|
||||
|
||||
assert!(
|
||||
!output.status.success(),
|
||||
"Expected non-zero exit for missing rules file"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user