You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Complete the HookCommand type boundary so invalid empty commands cannot be constructed as accepted domain values, and remove the duplicate scheduler fallback check.
Concrete file/function evidence
crates/git-smee-core/src/config.rs:131-142 defines HookCommand(String) with a private field, but public infallible From<String> and From<&str> implementations at :148-157 still allow HookCommand::from("") and whitespace-only values.
HookCommand::is_empty at crates/git-smee-core/src/config.rs:138 exposes validation as a query rather than enforcing the invariant at construction/deserialization.
SmeeConfig::validate at crates/git-smee-core/src/config.rs:69-87 checks each command after TOML deserialization.
crates/git-smee-core/src/executor/scheduler.rs:145 checks command.is_empty() again, so execution still treats an accepted HookCommand as potentially invalid.
This is a focused follow-up to the typed boundary introduced in Introduce a typed hook-command execution boundary #185; the type now flows through scheduler/runner APIs, but the non-empty invariant still has multiple owners.
Architectural concern
A domain type named HookCommand suggests callers can rely on its invariant, but public construction currently accepts values that configuration validation and execution reject. This leaves invalid states representable and couples the scheduler to config-validation policy. Future constructors or programmatic users can bypass SmeeConfig::from_toml and discover the error only during execution.
Proposed direction
Introduce a fallible constructor (HookCommand::new / TryFrom<String>) that trims only for validation while preserving the exact shell source. Use custom Serde deserialization or a validated wire-to-domain conversion so TOML parsing produces only valid HookCommand values. Remove or restrict the infallible conversions, then remove the scheduler's empty-command branch. Preserve the TOML shape and exact command bytes for valid inputs.
Acceptance criteria
Public construction and deserialization cannot produce an empty or whitespace-only HookCommand.
The exact non-empty shell source is preserved; this issue does not normalize command contents.
Empty-command validation has one owner and reports the existing actionable hook/entry context (or an equivalently specific typed error).
executor/scheduler.rs no longer re-checks an invariant guaranteed by HookCommand.
Existing TOML format and valid command execution remain compatible.
Validation expectations
Run cargo fmt --all -- --check.
Run cargo clippy --workspace --all-targets --all-features -- -D warnings.
Run cargo test --workspace --all-targets --all-features.
Add focused tests for empty and whitespace-only TOML commands, programmatic construction, exact shell-source preservation, and a normal execution path.
Summary
Complete the
HookCommandtype boundary so invalid empty commands cannot be constructed as accepted domain values, and remove the duplicate scheduler fallback check.Concrete file/function evidence
crates/git-smee-core/src/config.rs:131-142definesHookCommand(String)with a private field, but public infallibleFrom<String>andFrom<&str>implementations at:148-157still allowHookCommand::from("")and whitespace-only values.HookCommand::is_emptyatcrates/git-smee-core/src/config.rs:138exposes validation as a query rather than enforcing the invariant at construction/deserialization.SmeeConfig::validateatcrates/git-smee-core/src/config.rs:69-87checks each command after TOML deserialization.crates/git-smee-core/src/executor/scheduler.rs:145checkscommand.is_empty()again, so execution still treats an acceptedHookCommandas potentially invalid.Architectural concern
A domain type named
HookCommandsuggests callers can rely on its invariant, but public construction currently accepts values that configuration validation and execution reject. This leaves invalid states representable and couples the scheduler to config-validation policy. Future constructors or programmatic users can bypassSmeeConfig::from_tomland discover the error only during execution.Proposed direction
Introduce a fallible constructor (
HookCommand::new/TryFrom<String>) that trims only for validation while preserving the exact shell source. Use custom Serde deserialization or a validated wire-to-domain conversion so TOML parsing produces only validHookCommandvalues. Remove or restrict the infallible conversions, then remove the scheduler's empty-command branch. Preserve the TOML shape and exact command bytes for valid inputs.Acceptance criteria
HookCommand.executor/scheduler.rsno longer re-checks an invariant guaranteed byHookCommand.Validation expectations
cargo fmt --all -- --check.cargo clippy --workspace --all-targets --all-features -- -D warnings.cargo test --workspace --all-targets --all-features.