Enforce HookCommand's non-empty invariant

[errfld]

Summary

• make HookCommand construction fallible for empty and whitespace-only shell source
• deserialize config through a validated wire-to-domain conversion while preserving hook/entry error context and exact valid command bytes
• remove the scheduler's redundant empty-command fallback and migrate programmatic call sites to TryFrom

Test-driven development

• RED: focused constructor and deserialization regressions both failed against the previous infallible type
• GREEN: constructor, TOML, contextual CLI error, and normal execution tests now pass

Validation

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets --all-features -- -D warnings
• cargo test --workspace --all-targets --all-features

Fixes #191

Summary by CodeRabbit

• Bug Fixes

  • Configuration parsing now reports invalid or empty hook commands with the affected hook name and entry number.
  • Empty and whitespace-only commands are rejected earlier with clearer error messages.
  • Unknown configuration fields are rejected to help identify configuration mistakes.
  • Hook execution now correctly reports processes terminated by signals.

• Tests

  • Updated installation, configuration, and execution coverage for improved validation and error reporting.

[coderabbitai]

Warning

Review limit reached

@errfld, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 62f6dac1-f893-4689-b7ef-658ab1a3ec40

📥 Commits

Reviewing files that changed from the base of the PR and between e9a5a36 and aa45e1d.

📒 Files selected for processing (1)

• crates/git-smee-core/src/config.rs

📝 Walkthrough

Walkthrough

HookCommand now rejects empty and whitespace-only values during construction and deserialization. Configuration parsing reports hook and entry context, while executor and installer call sites use fallible command conversions and remove redundant empty-command handling.

Changes

Hook command invariant

Layer / File(s)	Summary
Configuration and command validation crates/git-smee-core/src/config.rs, crates/git-smee-core/tests/config_integration.rs, crates/git-smee-cli/tests/cli_integration.rs	Custom deserialization validates commands, preserves command text, reports hook and entry context, and updates parse-error coverage.
Execution error and conversion updates crates/git-smee-core/src/executor.rs, crates/git-smee-core/src/executor/scheduler.rs	NoCommandDefined and scheduler-side empty checks are removed; executor tests use fallible conversions and cover signal termination.
Installer command construction crates/git-smee-core/src/installer.rs, crates/git-smee-core/tests/installer_integration.rs	Installer examples and tests construct hook commands with try_into().unwrap().

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• errfld/git-smee#22: Covers configuration validation and contextual empty-command errors in the same parsing path.
• errfld/git-smee#24: Modifies executor error handling and tests around invalid commands.
• errfld/git-smee#184: Touches executor outcome handling for empty hook commands.

Poem

A bunny hops through TOML bright,
Empty commands vanish from sight.
Hooks keep their context, neat and clear,
Fallible conversions guide them here.
The scheduler nibbles clover with cheer!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title concisely matches the main change: enforcing HookCommand's non-empty invariant.
Linked Issues check	✅ Passed	The changes satisfy #191 by making HookCommand construction/deserialization fallible, preserving valid command bytes, and removing the scheduler fallback.
Out of Scope Changes check	✅ Passed	The modified config, executor, scheduler, installer, and tests all relate to the HookCommand invariant change.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch gh-191/hook-command-invariant

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/git-smee-core/src/config.rs`:
- Around line 196-202: Update HookCommand’s TryFrom<&str> implementation to
check whether value is empty before converting it to a String, returning
HookCommandError directly for empty input and delegating to the existing String
conversion only for non-empty input.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7217c82c-79ff-49ff-8b61-eb85b8d7abd9

📥 Commits

Reviewing files that changed from the base of the PR and between 50f1121 and e9a5a36.

📒 Files selected for processing (7)

• crates/git-smee-cli/tests/cli_integration.rs
• crates/git-smee-core/src/config.rs
• crates/git-smee-core/src/executor.rs
• crates/git-smee-core/src/executor/scheduler.rs
• crates/git-smee-core/src/installer.rs
• crates/git-smee-core/tests/config_integration.rs
• crates/git-smee-core/tests/installer_integration.rs

💤 Files with no reviewable changes (1)

• crates/git-smee-core/src/executor/scheduler.rs