From f4c791e1eac7d22945cdcede3cfd1f1329b10659 Mon Sep 17 00:00:00 2001 From: archae0pteryx Date: Mon, 4 May 2026 17:55:15 -0700 Subject: [PATCH 1/3] [focus-invariants-domain]: move Focus/Task invariants into domain Closes issues/034-focus-invariants-in-domain.md Completion promise: Focus title and task text invariants are enforced once in crates/domain/ via typed constructors; no validation logic remains in the commands layer. - Add DomainError enum (EmptyTitle, EmptyTaskText) in domain crate - Add NewFocus::new validated constructor - Add TaskText newtype with validated new + as_str - Replace trim().is_empty() guards in Commands::create_focus and Commands::append_task; map DomainError -> CommandError::BadRequest - Drop stale ralph/ Taskfile include left over from dabed05 so task check runs again --- crates/commands/src/error.rs | 8 ++++++- crates/commands/src/focus.rs | 45 +++++++++++++++++++++++++---------- crates/domain/src/error.rs | 16 +++++++++++++ crates/domain/src/focus.rs | 37 ++++++++++++++++++++++++++++ crates/domain/src/lib.rs | 4 +++- crates/domain/src/proposal.rs | 42 ++++++++++++++++++++++++++++++++ 6 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 crates/domain/src/error.rs diff --git a/crates/commands/src/error.rs b/crates/commands/src/error.rs index 664f930..1bab327 100644 --- a/crates/commands/src/error.rs +++ b/crates/commands/src/error.rs @@ -1,4 +1,4 @@ -use adhd_ranch_domain::ProposalValidationError; +use adhd_ranch_domain::{DomainError, ProposalValidationError}; use adhd_ranch_storage::{FocusStoreError, JsonlError}; #[derive(Debug, serde::Serialize)] @@ -49,3 +49,9 @@ impl From for CommandError { CommandError::Validation(e.to_string()) } } + +impl From for CommandError { + fn from(e: DomainError) -> Self { + CommandError::BadRequest(e.to_string()) + } +} diff --git a/crates/commands/src/focus.rs b/crates/commands/src/focus.rs index fbb670d..a8f8501 100644 --- a/crates/commands/src/focus.rs +++ b/crates/commands/src/focus.rs @@ -1,6 +1,6 @@ use std::sync::Arc; -use adhd_ranch_domain::{Caps, Focus, FocusTimer, NewFocus, TimerPreset, TimerStatus}; +use adhd_ranch_domain::{Caps, Focus, FocusTimer, NewFocus, TaskText, TimerPreset, TimerStatus}; use adhd_ranch_storage::FocusStore; use serde::{Deserialize, Serialize}; @@ -39,19 +39,13 @@ impl Commands { } pub fn create_focus(&self, input: CreateFocusInput) -> Result { - if input.title.trim().is_empty() { - return Err(CommandError::BadRequest("title must not be empty".into())); - } let timer = input.timer_preset.as_ref().map(|preset| FocusTimer { duration_secs: preset.duration_secs(), started_at: (self.clock_secs)(), status: TimerStatus::Running, }); - let new_focus = NewFocus { - title: input.title, - description: input.description, - timer_preset: input.timer_preset, - }; + let mut new_focus = NewFocus::new(input.title, input.description)?; + new_focus.timer_preset = input.timer_preset; let slug = create_focus_in_store(&self.store, &self.clock, &self.id_gen, &new_focus, timer)?; Ok(CreatedFocus { id: slug }) @@ -63,10 +57,8 @@ impl Commands { } pub fn append_task(&self, focus_id: &str, text: &str) -> Result<(), CommandError> { - if text.trim().is_empty() { - return Err(CommandError::BadRequest("text must not be empty".into())); - } - self.store.append_task(focus_id, text)?; + let text = TaskText::new(text)?; + self.store.append_task(focus_id, text.as_str())?; Ok(()) } @@ -125,6 +117,33 @@ mod tests { assert!(focuses[0].timer.is_none()); } + #[test] + fn create_focus_blank_title_returns_bad_request() { + let (commands, _dir) = build_commands(0); + let err = commands + .create_focus(CreateFocusInput { + title: " ".into(), + description: String::new(), + timer_preset: None, + }) + .unwrap_err(); + assert!(matches!(err, CommandError::BadRequest(_))); + } + + #[test] + fn append_task_blank_text_returns_bad_request() { + let (commands, _dir) = build_commands(0); + commands + .create_focus(CreateFocusInput { + title: "Real focus".into(), + description: String::new(), + timer_preset: None, + }) + .unwrap(); + let err = commands.append_task("real-focus", " ").unwrap_err(); + assert!(matches!(err, CommandError::BadRequest(_))); + } + #[test] fn create_focus_with_preset_stores_timer_with_correct_duration() { let started_at = 1_700_000_000_i64; diff --git a/crates/domain/src/error.rs b/crates/domain/src/error.rs new file mode 100644 index 0000000..af93704 --- /dev/null +++ b/crates/domain/src/error.rs @@ -0,0 +1,16 @@ +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DomainError { + EmptyTitle, + EmptyTaskText, +} + +impl std::fmt::Display for DomainError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::EmptyTitle => f.write_str("title must not be empty"), + Self::EmptyTaskText => f.write_str("task text must not be empty"), + } + } +} + +impl std::error::Error for DomainError {} diff --git a/crates/domain/src/focus.rs b/crates/domain/src/focus.rs index 692f173..cd030cb 100644 --- a/crates/domain/src/focus.rs +++ b/crates/domain/src/focus.rs @@ -1,7 +1,25 @@ use serde::{Deserialize, Serialize}; +use crate::error::DomainError; use crate::timer::FocusTimer; +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TaskText(String); + +impl TaskText { + pub fn new(text: impl Into) -> Result { + let text = text.into(); + if text.trim().is_empty() { + return Err(DomainError::EmptyTaskText); + } + Ok(Self(text)) + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} + #[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] pub struct FocusId(pub String); @@ -26,6 +44,25 @@ pub struct Focus { mod tests { use super::*; + #[test] + fn task_text_new_preserves_input() { + let t = TaskText::new("ship it").unwrap(); + assert_eq!(t.as_str(), "ship it"); + } + + #[test] + fn task_text_new_rejects_empty() { + assert_eq!(TaskText::new("").unwrap_err(), DomainError::EmptyTaskText); + } + + #[test] + fn task_text_new_rejects_whitespace() { + assert_eq!( + TaskText::new(" ").unwrap_err(), + DomainError::EmptyTaskText + ); + } + #[test] fn focus_round_trips_via_serde() { let f = Focus { diff --git a/crates/domain/src/lib.rs b/crates/domain/src/lib.rs index 3913fee..4037714 100644 --- a/crates/domain/src/lib.rs +++ b/crates/domain/src/lib.rs @@ -1,6 +1,7 @@ pub mod cap_monitor; pub mod caps; pub mod decision; +pub mod error; pub mod focus; pub mod parse; pub mod pig_rect; @@ -12,7 +13,8 @@ pub mod timer; pub use cap_monitor::{CapTransition, OverCapMonitor}; pub use caps::{cap_state, CapState}; pub use decision::{Decision, DecisionKind}; -pub use focus::{Focus, FocusId, Task}; +pub use error::DomainError; +pub use focus::{Focus, FocusId, Task, TaskText}; pub use parse::{parse_focus_md, ParseError}; pub use pig_rect::{PigRect, RectUpdater}; pub use proposal::{NewFocus, Proposal, ProposalId, ProposalKind, ProposalValidationError}; diff --git a/crates/domain/src/proposal.rs b/crates/domain/src/proposal.rs index f6e96cf..d3d804e 100644 --- a/crates/domain/src/proposal.rs +++ b/crates/domain/src/proposal.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; +use crate::error::DomainError; use crate::timer::TimerPreset; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -13,6 +14,23 @@ pub struct NewFocus { pub timer_preset: Option, } +impl NewFocus { + pub fn new( + title: impl Into, + description: impl Into, + ) -> Result { + let title = title.into(); + if title.trim().is_empty() { + return Err(DomainError::EmptyTitle); + } + Ok(Self { + title, + description: description.into(), + timer_preset: None, + }) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "snake_case")] pub enum ProposalKind { @@ -168,6 +186,30 @@ mod tests { ); } + #[test] + fn new_focus_new_accepts_valid_title() { + let nf = NewFocus::new("Ship it", "the bug").unwrap(); + assert_eq!(nf.title, "Ship it"); + assert_eq!(nf.description, "the bug"); + assert_eq!(nf.timer_preset, None); + } + + #[test] + fn new_focus_new_rejects_empty_title() { + assert_eq!( + NewFocus::new("", "desc").unwrap_err(), + DomainError::EmptyTitle + ); + } + + #[test] + fn new_focus_new_rejects_whitespace_title() { + assert_eq!( + NewFocus::new(" ", "desc").unwrap_err(), + DomainError::EmptyTitle + ); + } + #[test] fn add_task_kind_serializes_with_tag() { let p = ok_proposal(ProposalKind::AddTask { From 3e1589d42a4916cfc9e8b23b06c168c09e23e99a Mon Sep 17 00:00:00 2001 From: archae0pteryx Date: Mon, 4 May 2026 21:47:43 -0700 Subject: [PATCH 2/3] refactor(034): make NewFocus title invariant non-bypassable Previously NewFocus had pub fields, so callers (including Deserialize) could construct invalid domain state without going through ::new(). The invariant only held by convention. Make fields private; expose accessors and a with_timer_preset builder. Implement Deserialize manually so wire-format input also routes through ::new() and rejects empty titles. CreateProposalInput now returns BadRequest when kind=new_focus is sent without a new_focus payload. --- crates/commands/src/focus.rs | 4 +-- crates/commands/src/lifecycle.rs | 8 ++---- crates/commands/src/proposal.rs | 9 +++--- crates/domain/src/proposal.rs | 46 +++++++++++++++++++++++++++---- crates/storage/src/focus_store.rs | 18 ++++-------- crates/storage/src/proposals.rs | 6 +--- 6 files changed, 54 insertions(+), 37 deletions(-) diff --git a/crates/commands/src/focus.rs b/crates/commands/src/focus.rs index a8f8501..ec17fd5 100644 --- a/crates/commands/src/focus.rs +++ b/crates/commands/src/focus.rs @@ -44,8 +44,8 @@ impl Commands { started_at: (self.clock_secs)(), status: TimerStatus::Running, }); - let mut new_focus = NewFocus::new(input.title, input.description)?; - new_focus.timer_preset = input.timer_preset; + let new_focus = + NewFocus::new(input.title, input.description)?.with_timer_preset(input.timer_preset); let slug = create_focus_in_store(&self.store, &self.clock, &self.id_gen, &new_focus, timer)?; Ok(CreatedFocus { id: slug }) diff --git a/crates/commands/src/lifecycle.rs b/crates/commands/src/lifecycle.rs index 975d791..e0fa298 100644 --- a/crates/commands/src/lifecycle.rs +++ b/crates/commands/src/lifecycle.rs @@ -71,7 +71,7 @@ impl ProposalLifecycle { Ok(Some(target_focus_id.clone())) } ProposalKind::NewFocus { new_focus } => { - let timer = new_focus.timer_preset.as_ref().map(|preset| FocusTimer { + let timer = new_focus.timer_preset().map(|preset| FocusTimer { duration_secs: preset.duration_secs(), started_at: (self.clock_secs)(), status: TimerStatus::Running, @@ -243,11 +243,7 @@ mod tests { .append(&proposal( "p1", ProposalKind::NewFocus { - new_focus: NewFocus { - title: "Customer X bug".into(), - description: "ship".into(), - timer_preset: None, - }, + new_focus: NewFocus::new("Customer X bug", "ship").unwrap(), }, )) .unwrap(); diff --git a/crates/commands/src/proposal.rs b/crates/commands/src/proposal.rs index fec83de..7087a7f 100644 --- a/crates/commands/src/proposal.rs +++ b/crates/commands/src/proposal.rs @@ -53,11 +53,10 @@ impl Commands { task_text: input.task_text.clone().unwrap_or_default(), }, "new_focus" => ProposalKind::NewFocus { - new_focus: input.new_focus.clone().unwrap_or(NewFocus { - title: String::new(), - description: String::new(), - timer_preset: None, - }), + new_focus: input + .new_focus + .clone() + .ok_or_else(|| CommandError::BadRequest("new_focus required".into()))?, }, "discard" => ProposalKind::Discard, other => { diff --git a/crates/domain/src/proposal.rs b/crates/domain/src/proposal.rs index d3d804e..d95e5af 100644 --- a/crates/domain/src/proposal.rs +++ b/crates/domain/src/proposal.rs @@ -1,4 +1,4 @@ -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use crate::error::DomainError; use crate::timer::TimerPreset; @@ -6,12 +6,12 @@ use crate::timer::TimerPreset; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ProposalId(pub String); -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct NewFocus { - pub title: String, - pub description: String, + title: String, + description: String, #[serde(skip_serializing_if = "Option::is_none", default)] - pub timer_preset: Option, + timer_preset: Option, } impl NewFocus { @@ -29,6 +29,40 @@ impl NewFocus { timer_preset: None, }) } + + pub fn title(&self) -> &str { + &self.title + } + + pub fn description(&self) -> &str { + &self.description + } + + pub fn timer_preset(&self) -> Option<&TimerPreset> { + self.timer_preset.as_ref() + } + + #[must_use] + pub fn with_timer_preset(mut self, timer_preset: Option) -> Self { + self.timer_preset = timer_preset; + self + } +} + +impl<'de> Deserialize<'de> for NewFocus { + fn deserialize>(d: D) -> Result { + #[derive(Deserialize)] + struct Raw { + title: String, + #[serde(default)] + description: String, + #[serde(default)] + timer_preset: Option, + } + let raw = Raw::deserialize(d)?; + let nf = NewFocus::new(raw.title, raw.description).map_err(serde::de::Error::custom)?; + Ok(nf.with_timer_preset(raw.timer_preset)) + } } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -98,7 +132,7 @@ impl Proposal { } } ProposalKind::NewFocus { new_focus } => { - if new_focus.title.trim().is_empty() { + if new_focus.title().trim().is_empty() { return Err(ProposalValidationError::NewFocusEmptyTitle); } } diff --git a/crates/storage/src/focus_store.rs b/crates/storage/src/focus_store.rs index 70332d0..aad75c3 100644 --- a/crates/storage/src/focus_store.rs +++ b/crates/storage/src/focus_store.rs @@ -134,7 +134,7 @@ impl FocusStore for MarkdownFocusStore { created_at: &str, timer: Option, ) -> Result { - let slug = slugify(&new_focus.title); + let slug = slugify(new_focus.title()); let dir = self.root.join(&slug); if dir.exists() { return Err(FocusStoreError::AlreadyExists(slug)); @@ -143,8 +143,8 @@ impl FocusStore for MarkdownFocusStore { let body = format!( "---\nid: {id}\ntitle: {title}\ndescription: {description}\ncreated_at: {created_at}\n---\n", - title = new_focus.title, - description = new_focus.description, + title = new_focus.title(), + description = new_focus.description(), ); atomic_write(&dir.join("focus.md"), body.as_bytes())?; @@ -360,11 +360,7 @@ mod tests { let store = MarkdownFocusStore::new(dir.path()); let slug = store .create_focus( - &NewFocus { - title: "Customer X bug".into(), - description: "ship it".into(), - timer_preset: None, - }, + &NewFocus::new("Customer X bug", "ship it").unwrap(), "id-1", "2026-04-30T12:00:00Z", None, @@ -384,11 +380,7 @@ mod tests { write_focus(dir.path(), "customer-x-bug", "existing"); let err = store .create_focus( - &NewFocus { - title: "Customer X bug".into(), - description: "".into(), - timer_preset: None, - }, + &NewFocus::new("Customer X bug", "").unwrap(), "id-2", "2026-04-30T12:00:00Z", None, diff --git a/crates/storage/src/proposals.rs b/crates/storage/src/proposals.rs index 73865cb..aa9043a 100644 --- a/crates/storage/src/proposals.rs +++ b/crates/storage/src/proposals.rs @@ -100,11 +100,7 @@ mod tests { let p = proposal( &format!("p{i}"), ProposalKind::NewFocus { - new_focus: NewFocus { - title: format!("T{i}"), - description: String::new(), - timer_preset: None, - }, + new_focus: NewFocus::new(format!("T{i}"), "").unwrap(), }, ); queue.append(&p).unwrap(); From 80e280428c1ff9a37b9bb31dcd9f8b5ac1196cd4 Mon Sep 17 00:00:00 2001 From: archae0pteryx Date: Mon, 4 May 2026 21:52:16 -0700 Subject: [PATCH 3/3] test(034): use NewFocus::new in round-trip test, capture id in append_task test --- crates/commands/src/focus.rs | 4 ++-- crates/domain/src/proposal.rs | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/commands/src/focus.rs b/crates/commands/src/focus.rs index ec17fd5..9b91ec5 100644 --- a/crates/commands/src/focus.rs +++ b/crates/commands/src/focus.rs @@ -133,14 +133,14 @@ mod tests { #[test] fn append_task_blank_text_returns_bad_request() { let (commands, _dir) = build_commands(0); - commands + let created = commands .create_focus(CreateFocusInput { title: "Real focus".into(), description: String::new(), timer_preset: None, }) .unwrap(); - let err = commands.append_task("real-focus", " ").unwrap_err(); + let err = commands.append_task(&created.id, " ").unwrap_err(); assert!(matches!(err, CommandError::BadRequest(_))); } diff --git a/crates/domain/src/proposal.rs b/crates/domain/src/proposal.rs index d95e5af..7ebd75c 100644 --- a/crates/domain/src/proposal.rs +++ b/crates/domain/src/proposal.rs @@ -191,6 +191,9 @@ mod tests { #[test] fn new_focus_requires_title() { + // Struct literal is deliberate: NewFocus::new rejects empty titles, so + // this exercises Proposal::validate as a defense-in-depth fallback for + // any path that bypasses the constructor. let p = ok_proposal(ProposalKind::NewFocus { new_focus: NewFocus { title: "".into(), @@ -259,11 +262,7 @@ mod tests { #[test] fn round_trip_preserves_kind() { let original = ok_proposal(ProposalKind::NewFocus { - new_focus: NewFocus { - title: "T".into(), - description: "D".into(), - timer_preset: None, - }, + new_focus: NewFocus::new("T", "D").unwrap(), }); let json = serde_json::to_string(&original).unwrap(); let back: Proposal = serde_json::from_str(&json).unwrap();