Skip to content

feat(core): add explicit config validation on load - #22

Merged
errfld merged 1 commit into
mainfrom
git-smee-4q5/config-validation-errors
Feb 4, 2026
Merged

feat(core): add explicit config validation on load#22
errfld merged 1 commit into
mainfrom
git-smee-4q5/config-validation-errors

Conversation

@errfld

@errfld errfld commented Feb 4, 2026

Copy link
Copy Markdown
Owner

What changed

  • added a dedicated SmeeConfig::validate() pass and invoke it during config load
  • reject empty hook lists and empty/whitespace commands with explicit ValidationError variants that include hook name and entry index
  • reject unknown fields in HookDefinition via #[serde(deny_unknown_fields)]
  • expanded config integration coverage for whitespace commands and unknown hook entry fields
  • added a CLI integration test verifying install reports config validation failures
  • removed a stray assertion from installer_integration.rs that prevented workspace tests from compiling

Validation

  • cargo test --workspace

Closes git-smee-4q5.

Summary by CodeRabbit

  • New Features

    • Configuration validation now enforces non-empty hook entries and commands, reporting detailed errors that identify the specific hook and entry with issues.
    • Unknown configuration fields are now rejected during parsing.
  • Tests

    • Added integration tests covering validation error reporting for invalid configurations.

@coderabbitai

coderabbitai Bot commented Feb 4, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Configuration validation is introduced with a new ValidationError enum supporting EmptyHookEntries and EmptyCommand variants. The SmeeConfig::validate() method enforces non-empty hook entries and commands after parsing. The from_toml() method now invokes validation, and HookDefinition gains deny_unknown_fields to reject unexpected fields in TOML.

Changes

Cohort / File(s) Summary
Config Validation System
crates/git-smee-core/src/config.rs
Introduces ValidationError enum with EmptyHookEntries and EmptyCommand variants. Adds SmeeConfig::validate() method to enforce non-empty entries and commands. Updates from_toml() to call validation after parsing. Adds #[serde(deny_unknown_fields)] to HookDefinition. Integrates ValidationError into main Error enum.
CLI Integration Tests
crates/git-smee-cli/tests/cli_integration.rs
Adds test given_invalid_config_when_install_then_validation_error_is_reported() to verify install command reports validation errors with correct hook_name and entry_index in stderr.
Config Unit Tests
crates/git-smee-core/tests/config_integration.rs
Adds tests verifying whitespace-only commands trigger ValidationError::EmptyCommand and unknown fields in hook definitions produce parsing errors with descriptive messages.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Config validation hops along
With empty commands ringing strong,
ValidationErrors catch the flaw,
Unknown fields denied by serde law!
Safety checks from the core so bright, 🌟

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(core): add explicit config validation on load' accurately and concisely describes the main change: adding explicit validation to the config loading process.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch git-smee-4q5/config-validation-errors

Comment @coderabbitai help to get the list of available commands and usage tips.

@errfld
errfld merged commit 536eb4f into main Feb 4, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant