From e67eff17add4033646b4dba705d00d651712ba57 Mon Sep 17 00:00:00 2001 From: Huskies Agent Date: Wed, 15 Jul 2026 23:58:18 +0000 Subject: [PATCH] huskies: merge 1158 story Reject boilerplate user stories at save time --- crates/source-map-gen/src/rust_adapter.rs | 10 +-- crates/source-map-gen/src/ts_adapter.rs | 10 +-- .../chat/transport/matrix/project_rebuild.rs | 5 +- server/src/http/mcp/wizard_tools.rs | 14 ++++ server/src/service/wizard/mod.rs | 25 +++++++ server/src/validation/error.rs | 11 +++ server/src/validation/mod.rs | 1 + server/src/validation/newtypes.rs | 75 +++++++++++++++++++ server/src/validation/requests.rs | 23 +++++- 9 files changed, 158 insertions(+), 16 deletions(-) diff --git a/crates/source-map-gen/src/rust_adapter.rs b/crates/source-map-gen/src/rust_adapter.rs index 4e8b05a5..e3968aff 100644 --- a/crates/source-map-gen/src/rust_adapter.rs +++ b/crates/source-map-gen/src/rust_adapter.rs @@ -111,10 +111,9 @@ fn parse_pub_item(line: &str) -> Option<(String, String)> { let rest = if let Some(r) = trimmed.strip_prefix("pub(") { let end = r.find(')')?; r[end + 1..].trim_start() - } else if let Some(r) = trimmed.strip_prefix("pub ") { - r.trim_start() } else { - return None; + let r = trimmed.strip_prefix("pub ")?; + r.trim_start() }; // Handle "async fn" @@ -139,10 +138,9 @@ fn parse_pub_item(line: &str) -> Option<(String, String)> { ("const", r.trim_start()) } else if let Some(r) = rest.strip_prefix("static ") { ("static", r.trim_start()) - } else if let Some(r) = rest.strip_prefix("mod ") { - ("mod", r.trim_start()) } else { - return None; + let r = rest.strip_prefix("mod ")?; + ("mod", r.trim_start()) }; let name: String = name_part diff --git a/crates/source-map-gen/src/ts_adapter.rs b/crates/source-map-gen/src/ts_adapter.rs index c256c1ad..9cbef384 100644 --- a/crates/source-map-gen/src/ts_adapter.rs +++ b/crates/source-map-gen/src/ts_adapter.rs @@ -123,10 +123,9 @@ fn parse_exported_item(line: &str) -> Option<(String, String)> { // Strip "export default" or "export" let rest = if let Some(r) = trimmed.strip_prefix("export default ") { r.trim_start() - } else if let Some(r) = trimmed.strip_prefix("export ") { - r.trim_start() } else { - return None; + let r = trimmed.strip_prefix("export ")?; + r.trim_start() }; // Strip optional "async" @@ -148,10 +147,9 @@ fn parse_exported_item(line: &str) -> Option<(String, String)> { ("const", r.trim_start()) } else if let Some(r) = rest.strip_prefix("let ") { ("let", r.trim_start()) - } else if let Some(r) = rest.strip_prefix("enum ") { - ("enum", r.trim_start()) } else { - return None; + let r = rest.strip_prefix("enum ")?; + ("enum", r.trim_start()) }; let name: String = name_part diff --git a/server/src/chat/transport/matrix/project_rebuild.rs b/server/src/chat/transport/matrix/project_rebuild.rs index 0e583f05..a703e78f 100644 --- a/server/src/chat/transport/matrix/project_rebuild.rs +++ b/server/src/chat/transport/matrix/project_rebuild.rs @@ -51,10 +51,9 @@ pub fn extract_project_rebuild_command( .trim() .trim_start_matches(|c: char| !c.is_alphanumeric()); - let rest = if let Some(r) = trimmed.strip_prefix("project-rebuild") { + let rest = { + let r = trimmed.strip_prefix("project-rebuild")?; r.trim() - } else { - return None; }; let mut parts = rest.split_whitespace(); diff --git a/server/src/http/mcp/wizard_tools.rs b/server/src/http/mcp/wizard_tools.rs index 69fbc36a..1218afca 100644 --- a/server/src/http/mcp/wizard_tools.rs +++ b/server/src/http/mcp/wizard_tools.rs @@ -156,6 +156,20 @@ mod tests { assert_eq!(state.steps[1].status, StepStatus::Generating); } + #[test] + fn wizard_generate_rejects_boilerplate_user_story_content() { + let dir = TempDir::new().unwrap(); + let ctx = setup(&dir); + let result = tool_wizard_generate( + &serde_json::json!({"content": "As a user, I want X, so that Y."}), + &ctx, + ); + let err = result.unwrap_err(); + assert!(err.contains("Invalid content")); + let state = WizardState::load(dir.path()).unwrap(); + assert_eq!(state.steps[1].status, StepStatus::Pending); + } + #[test] fn wizard_generate_with_content_stages_content() { let dir = TempDir::new().unwrap(); diff --git a/server/src/service/wizard/mod.rs b/server/src/service/wizard/mod.rs index 2dbbbea4..d7a73a3f 100644 --- a/server/src/service/wizard/mod.rs +++ b/server/src/service/wizard/mod.rs @@ -11,6 +11,7 @@ pub(crate) mod io; pub mod state_machine; use crate::io::wizard::{StepStatus, WizardState, WizardStep, format_wizard_state}; +use crate::validation::check_boilerplate_user_story; use std::path::Path; // ── Error type ──────────────────────────────────────────────────────────────── @@ -21,6 +22,7 @@ use std::path::Path; /// - [`Error::NotActive`] → 404 Not Found /// - [`Error::InvalidStateTransition`] → 400 Bad Request /// - [`Error::MissingInput`] → 400 Bad Request +/// - [`Error::InvalidContent`] → 400 Bad Request /// - [`Error::GenerationFailure`] → 500 Internal Server Error /// - [`Error::PersistenceFailure`] → 500 Internal Server Error #[derive(Debug)] @@ -32,6 +34,8 @@ pub enum Error { InvalidStateTransition(String), /// Required input was absent (e.g. content not staged before confirming). MissingInput(String), + /// Staged content failed validation (e.g. boilerplate user-story template). + InvalidContent(String), /// The LLM or agent failed to generate content for the step. GenerationFailure(String), /// A filesystem read or write operation failed. @@ -46,6 +50,7 @@ impl std::fmt::Display for Error { } Self::InvalidStateTransition(msg) => write!(f, "Invalid state transition: {msg}"), Self::MissingInput(msg) => write!(f, "Missing input: {msg}"), + Self::InvalidContent(msg) => write!(f, "Invalid content: {msg}"), Self::GenerationFailure(msg) => write!(f, "Generation failed: {msg}"), Self::PersistenceFailure(msg) => write!(f, "Persistence error: {msg}"), } @@ -104,6 +109,8 @@ pub fn status(root: &Path) -> Result { /// /// # Errors /// - [`Error::NotActive`] if no wizard is active. +/// - [`Error::InvalidContent`] if `content` matches the boilerplate +/// "As a …, I want …, so that …" user-story template. /// - [`Error::PersistenceFailure`] if saving state fails. pub fn generate(root: &Path, content: Option<&str>) -> Result { let mut state = io::load(root).ok_or(Error::NotActive)?; @@ -116,6 +123,9 @@ pub fn generate(root: &Path, content: Option<&str>) -> Result { let step = state.steps[current_idx].step; if let Some(c) = content { + if let Some(err) = check_boilerplate_user_story("content", c) { + return Err(Error::InvalidContent(err.to_string())); + } state.set_step_status(step, StepStatus::AwaitingConfirmation, Some(c.to_string())); io::save(&state, root)?; return Ok(format!( @@ -381,6 +391,21 @@ mod tests { assert_eq!(state.steps[1].status, StepStatus::AwaitingConfirmation); } + #[test] + fn generate_rejects_boilerplate_user_story_content() { + let dir = TempDir::new().unwrap(); + init_wizard(&dir); + let err = generate( + dir.path(), + Some("As a user, I want this, so that I get value."), + ) + .unwrap_err(); + assert!(matches!(err, Error::InvalidContent(_))); + // Content must not be staged when rejected. + let state = get_state(dir.path()).unwrap(); + assert_eq!(state.steps[1].status, StepStatus::Pending); + } + #[test] fn generate_no_content_marks_generating() { let dir = TempDir::new().unwrap(); diff --git a/server/src/validation/error.rs b/server/src/validation/error.rs index 7500ba78..463aeadc 100644 --- a/server/src/validation/error.rs +++ b/server/src/validation/error.rs @@ -29,6 +29,8 @@ pub enum ValidationError { }, /// A field value contains a tool-call grammar fragment that must be rejected. AntiGrammarToken { field: String, token: String }, + /// A field value matches the unfilled "As a …, I want …, so that …" boilerplate template. + BoilerplateUserStory { field: String }, /// A numeric field value is outside its allowed range. OutOfRange { field: String, @@ -91,6 +93,15 @@ impl fmt::Display for ValidationError { "field '{field}' contains a tool-call grammar fragment: {token:?}" ) } + Self::BoilerplateUserStory { field } => { + write!( + f, + "field '{field}' looks like the unfilled 'As a …, I want …, so that …' \ + template. Rephrase it as a concrete user story in plain language, \ + describing the actual actor, action, and benefit. If you cannot \ + determine the right phrasing, ask the commissioning user." + ) + } Self::OutOfRange { field, min, diff --git a/server/src/validation/mod.rs b/server/src/validation/mod.rs index 10c56e56..4c3376f4 100644 --- a/server/src/validation/mod.rs +++ b/server/src/validation/mod.rs @@ -20,6 +20,7 @@ mod sanitize; pub use error::{ValidationError, format_errors_as_json}; pub use newtypes::{ AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage, + check_boilerplate_user_story, }; pub use requests::{ AddCriterionRequest, ConvertItemTypeRequest, CreateBugRequest, CreateEpicRequest, diff --git a/server/src/validation/newtypes.rs b/server/src/validation/newtypes.rs index 5aab4bd3..a8bd0cad 100644 --- a/server/src/validation/newtypes.rs +++ b/server/src/validation/newtypes.rs @@ -6,7 +6,9 @@ //! in preference to nutype's lower-level `new()`. use nutype::nutype; +use regex::Regex; use serde::{Deserialize, Serialize}; +use std::sync::LazyLock; use super::error::ValidationError; use super::sanitize; @@ -51,6 +53,25 @@ fn check_grammar_tokens(field: &str, value: &str) -> Vec { .collect() } +/// Matches the classic "As a …, I want …, so that …" user-story template, +/// case-insensitively and tolerant of punctuation/whitespace variation between +/// the three clauses (commas, dashes, or nothing at all). +static BOILERPLATE_USER_STORY_RE: LazyLock = + LazyLock::new(|| Regex::new(r"(?is)\bas\s+an?\b.*?\bi\s+want\b.*?\bso\s+that\b").unwrap()); + +/// Return a [`ValidationError::BoilerplateUserStory`] if `value` matches the +/// unfilled "As a …, I want …, so that …" template rather than a concrete, +/// plain-language user story. +pub fn check_boilerplate_user_story(field: &str, value: &str) -> Option { + if BOILERPLATE_USER_STORY_RE.is_match(value) { + Some(ValidationError::BoilerplateUserStory { + field: field.to_string(), + }) + } else { + None + } +} + // --------------------------------------------------------------------------- // StoryName newtype // --------------------------------------------------------------------------- @@ -481,6 +502,60 @@ mod tests { assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. })); } + // --- check_boilerplate_user_story --- + + #[test] + fn boilerplate_user_story_rejects_classic_template() { + let err = check_boilerplate_user_story( + "user_story", + "As a user, I want to log in, so that I can access my account.", + ); + assert!(matches!( + err, + Some(ValidationError::BoilerplateUserStory { .. }) + )); + } + + #[test] + fn boilerplate_user_story_rejects_without_commas() { + let err = check_boilerplate_user_story( + "user_story", + "As a developer I want CI so that builds are faster", + ); + assert!(matches!( + err, + Some(ValidationError::BoilerplateUserStory { .. }) + )); + } + + #[test] + fn boilerplate_user_story_rejects_case_insensitively() { + let err = check_boilerplate_user_story( + "user_story", + "AS AN admin I WANT reports SO THAT I can audit usage", + ); + assert!(matches!( + err, + Some(ValidationError::BoilerplateUserStory { .. }) + )); + } + + #[test] + fn boilerplate_user_story_accepts_concrete_plain_language() { + let err = check_boilerplate_user_story( + "user_story", + "The pipeline dashboard should show test coverage next to each story so \ + QA doesn't have to check CI separately.", + ); + assert!(err.is_none()); + } + + #[test] + fn boilerplate_user_story_accepts_partial_phrase_without_so_that() { + let err = check_boilerplate_user_story("user_story", "As a user I want this"); + assert!(err.is_none()); + } + // --- DependsOnId --- #[test] diff --git a/server/src/validation/requests.rs b/server/src/validation/requests.rs index d2e11a5b..607ea216 100644 --- a/server/src/validation/requests.rs +++ b/server/src/validation/requests.rs @@ -10,6 +10,7 @@ use serde_json::Value; use super::error::{ValidationError, format_errors_as_json}; use super::newtypes::{ AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage, + check_boilerplate_user_story, }; // --------------------------------------------------------------------------- @@ -91,7 +92,16 @@ impl CreateStoryRequest { let user_story = match args.get("user_story").and_then(|v| v.as_str()) { None => None, Some(raw) => match Description::parse("user_story", raw) { - Ok(d) => Some(d), + Ok(d) => { + if let Some(boilerplate_err) = + check_boilerplate_user_story("user_story", d.as_str()) + { + errors.push(boilerplate_err); + None + } else { + Some(d) + } + } Err(mut errs) => { errors.append(&mut errs); None @@ -1365,6 +1375,17 @@ mod tests { assert!(err.contains("AntiGrammarToken")); } + #[test] + fn create_story_request_rejects_boilerplate_user_story() { + let args = json!({ + "name": "Valid Name", + "user_story": "As a user, I want to do a thing, so that I get value.", + "acceptance_criteria": ["AC1"] + }); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + assert!(err.contains("BoilerplateUserStory")); + } + #[test] fn create_story_request_with_all_optional_fields() { let args = json!({