huskies: merge 1032

This commit is contained in:
dave
2026-05-14 14:41:45 +00:00
parent bc99821274
commit 960b4f4d1d
9 changed files with 606 additions and 44 deletions
+3 -8
View File
@@ -66,14 +66,9 @@ pub(crate) async fn tool_rebuild_and_restart(ctx: &AppContext) -> Result<String,
/// WebSocket session, which presents a dialog to the user. Blocks until the
/// user approves or denies (with a 5-minute timeout).
pub(crate) fn tool_move_story(args: &Value, _ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let target_stage = args
.get("target_stage")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: target_stage")?;
let req = crate::validation::MoveStoryRequest::from_json(args)?;
let story_id = req.story_id.as_str();
let target_stage = req.target_stage.as_str();
let (from_stage, to_stage) = move_story_to_stage(story_id, target_stage)?;
@@ -435,7 +435,7 @@ mod tests {
&ctx,
);
assert!(result.is_err());
assert!(result.unwrap_err().contains("Invalid target_stage"));
assert!(result.unwrap_err().contains("InvalidValue"));
}
#[test]
+3 -8
View File
@@ -142,14 +142,9 @@ pub(super) async fn tool_move_story_to_merge(
args: &Value,
ctx: &AppContext,
) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let agent_name = args
.get("agent_name")
.and_then(|v| v.as_str())
.unwrap_or("mergemaster");
let req = crate::validation::MoveStoryToMergeRequest::from_json(args)?;
let story_id = req.story_id.as_str();
let agent_name = req.resolved_agent_name();
let project_root = ctx.services.agents.get_project_root(&ctx.state)?;
@@ -9,10 +9,8 @@ use serde_json::Value;
/// Accepts a `story_id` (full filename stem, e.g. `"42_story_foo"`) and
/// delegates to [`service::work_item::freeze::freeze`].
pub(crate) fn tool_freeze_story(args: &Value, _ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let req = crate::validation::FreezeStoryRequest::from_json(args)?;
let story_id = req.story_id.as_str();
match crate::service::work_item::freeze::freeze(story_id)? {
FreezeStatus::AlreadyFrozen => Ok(format!("Story '{story_id}' is already frozen.")),
@@ -27,10 +25,8 @@ pub(crate) fn tool_freeze_story(args: &Value, _ctx: &AppContext) -> Result<Strin
/// Accepts a `story_id` (full filename stem, e.g. `"42_story_foo"`) and
/// delegates to [`service::work_item::freeze::unfreeze`].
pub(crate) fn tool_unfreeze_story(args: &Value, _ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let req = crate::validation::FreezeStoryRequest::from_json(args)?;
let story_id = req.story_id.as_str();
match crate::service::work_item::freeze::unfreeze(story_id)? {
UnfreezeStatus::NotFrozen => Ok(format!(
@@ -218,23 +218,11 @@ pub(crate) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result<String
}
pub(crate) fn tool_unblock_story(args: &Value, ctx: &AppContext) -> Result<String, String> {
let story_id = args
.get("story_id")
.and_then(|v| v.as_str())
.ok_or("Missing required argument: story_id")?;
let req = crate::validation::UnblockStoryRequest::from_json(args)?;
let root = ctx.state.get_project_root()?;
// Extract the numeric prefix (e.g. "42" from "42" or from legacy "42_story_foo").
let story_number = story_id
.split('_')
.next()
.filter(|s| !s.is_empty() && s.chars().all(|c| c.is_ascii_digit()))
.ok_or_else(|| {
format!("Invalid story_id format: '{story_id}'. Expected a numeric ID (e.g. '42').")
})?;
let result = crate::chat::commands::unblock::unblock_by_number(&root, story_number);
let result =
crate::chat::commands::unblock::unblock_by_number(&root, req.story_id.numeric_prefix());
if result.contains("not blocked")
|| result.contains("not found")
|| result.contains("Error")
+20
View File
@@ -52,6 +52,15 @@ pub enum ValidationError {
InvalidUtf8 { field: String },
/// A `depends_on` entry references the same item being created or updated.
SelfReference { field: String },
/// A field's value is not one of the accepted choices.
InvalidValue {
/// Field name.
field: String,
/// The value that was supplied.
actual: String,
/// The complete set of accepted values.
allowed: Vec<String>,
},
}
impl fmt::Display for ValidationError {
@@ -114,6 +123,17 @@ impl fmt::Display for ValidationError {
"field '{field}' contains a self-reference (depends on itself)"
)
}
Self::InvalidValue {
field,
actual,
allowed,
} => {
write!(
f,
"field '{field}' value {actual:?} is not one of the allowed values: {}",
allowed.join(", ")
)
}
}
}
}
+5 -2
View File
@@ -18,8 +18,11 @@ mod requests;
mod sanitize;
pub use error::{ValidationError, format_errors_as_json};
pub use newtypes::{AcceptanceCriterion, DependsOnId, Description, StoryName};
pub use newtypes::{
AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage,
};
pub use requests::{
AddCriterionRequest, CreateBugRequest, CreateEpicRequest, CreateRefactorRequest,
CreateSpikeRequest, CreateStoryRequest, EditCriterionRequest, UpdateStoryRequest,
CreateSpikeRequest, CreateStoryRequest, EditCriterionRequest, FreezeStoryRequest,
MoveStoryRequest, MoveStoryToMergeRequest, UnblockStoryRequest, UpdateStoryRequest,
};
+203
View File
@@ -261,6 +261,122 @@ impl DependsOnId {
}
}
// ---------------------------------------------------------------------------
// StoryId newtype
// ---------------------------------------------------------------------------
/// A validated story / work-item identifier.
///
/// Must be non-empty, start with one or more ASCII digits, and contain only
/// ASCII alphanumeric characters or underscores. Optionally followed by a
/// slug suffix separated by `_` (e.g. `"42_story_foo"`).
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct StoryId(String);
impl StoryId {
/// Parse and validate a raw story ID string.
pub fn parse(raw: &str) -> Result<Self, Vec<ValidationError>> {
let trimmed = raw.trim();
let grammar_errors = check_grammar_tokens("story_id", trimmed);
if !grammar_errors.is_empty() {
return Err(grammar_errors);
}
if trimmed.is_empty() {
return Err(vec![ValidationError::EmptyAfterTrim {
field: "story_id".into(),
}]);
}
// Must start with at least one ASCII digit.
let first = trimmed.chars().next().unwrap();
if !first.is_ascii_digit() {
return Err(vec![ValidationError::InvalidCharacter {
field: "story_id".into(),
ch: first,
position: 0,
}]);
}
// All characters must be ASCII alphanumeric or underscore.
for (i, ch) in trimmed.chars().enumerate() {
if !ch.is_ascii_alphanumeric() && ch != '_' {
return Err(vec![ValidationError::InvalidCharacter {
field: "story_id".into(),
ch,
position: i,
}]);
}
}
Ok(StoryId(trimmed.to_string()))
}
/// Return the full story ID string (e.g. `"42_story_foo"` or `"42"`).
pub fn as_str(&self) -> &str {
&self.0
}
/// Return the numeric prefix only (e.g. `"42"` from `"42_story_foo"`).
pub fn numeric_prefix(&self) -> &str {
self.0.split('_').next().unwrap_or(&self.0)
}
}
// ---------------------------------------------------------------------------
// TargetStage enum
// ---------------------------------------------------------------------------
/// Valid target pipeline stage values for the `move_story` tool.
///
/// Each variant maps to a wire string accepted by `move_story_to_stage`.
#[derive(Debug, Clone, PartialEq)]
pub enum TargetStage {
/// Move to the backlog stage.
Backlog,
/// Move to the current/coding stage.
Current,
/// Move to the QA stage.
Qa,
/// Move to the merge stage.
Merge,
/// Move to the done stage.
Done,
}
impl TargetStage {
/// All accepted raw string values.
pub const ALLOWED: &'static [&'static str] = &["backlog", "current", "qa", "merge", "done"];
/// Parse a raw target stage string.
pub fn parse(raw: &str) -> Result<Self, Vec<ValidationError>> {
match raw.trim() {
"backlog" => Ok(TargetStage::Backlog),
"current" => Ok(TargetStage::Current),
"qa" => Ok(TargetStage::Qa),
"merge" => Ok(TargetStage::Merge),
"done" => Ok(TargetStage::Done),
other => Err(vec![ValidationError::InvalidValue {
field: "target_stage".into(),
actual: other.to_string(),
allowed: Self::ALLOWED.iter().map(|s| s.to_string()).collect(),
}]),
}
}
/// Return the wire string accepted by `move_story_to_stage`.
pub fn as_str(&self) -> &str {
match self {
TargetStage::Backlog => "backlog",
TargetStage::Current => "current",
TargetStage::Qa => "qa",
TargetStage::Merge => "merge",
TargetStage::Done => "done",
}
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -379,6 +495,93 @@ mod tests {
assert_eq!(id.get(), 42);
}
// --- StoryId ---
#[test]
fn story_id_accepts_numeric_only() {
let id = StoryId::parse("42").unwrap();
assert_eq!(id.as_str(), "42");
assert_eq!(id.numeric_prefix(), "42");
}
#[test]
fn story_id_accepts_numeric_with_slug() {
let id = StoryId::parse("42_story_foo").unwrap();
assert_eq!(id.as_str(), "42_story_foo");
assert_eq!(id.numeric_prefix(), "42");
}
#[test]
fn story_id_rejects_empty() {
let err = StoryId::parse("").unwrap_err();
assert!(matches!(err[0], ValidationError::EmptyAfterTrim { .. }));
}
#[test]
fn story_id_rejects_whitespace_only() {
let err = StoryId::parse(" ").unwrap_err();
assert!(matches!(err[0], ValidationError::EmptyAfterTrim { .. }));
}
#[test]
fn story_id_rejects_non_digit_start() {
let err = StoryId::parse("story_42").unwrap_err();
assert!(matches!(
err[0],
ValidationError::InvalidCharacter { position: 0, .. }
));
}
#[test]
fn story_id_rejects_invalid_chars() {
let err = StoryId::parse("42/bad").unwrap_err();
assert!(matches!(err[0], ValidationError::InvalidCharacter { .. }));
}
#[test]
fn story_id_rejects_grammar_token() {
let err = StoryId::parse("<thinking>").unwrap_err();
assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. }));
}
#[test]
fn story_id_trims_whitespace() {
let id = StoryId::parse(" 99 ").unwrap();
assert_eq!(id.as_str(), "99");
}
// --- TargetStage ---
#[test]
fn target_stage_accepts_all_valid_values() {
for &s in TargetStage::ALLOWED {
assert!(TargetStage::parse(s).is_ok(), "should accept '{s}'");
}
}
#[test]
fn target_stage_as_str_round_trips() {
for &s in TargetStage::ALLOWED {
let stage = TargetStage::parse(s).unwrap();
assert_eq!(stage.as_str(), s);
}
}
#[test]
fn target_stage_rejects_invalid() {
let err = TargetStage::parse("archived").unwrap_err();
assert!(matches!(
&err[0],
ValidationError::InvalidValue { field, .. } if field == "target_stage"
));
}
#[test]
fn target_stage_rejects_empty() {
let err = TargetStage::parse("").unwrap_err();
assert!(matches!(err[0], ValidationError::InvalidValue { .. }));
}
// --- Round-trip serde ---
#[test]
+364 -2
View File
@@ -8,7 +8,9 @@ use garde::Validate;
use serde_json::Value;
use super::error::{ValidationError, format_errors_as_json};
use super::newtypes::{AcceptanceCriterion, DependsOnId, Description, StoryName};
use super::newtypes::{
AcceptanceCriterion, DependsOnId, Description, StoryId, StoryName, TargetStage,
};
// ---------------------------------------------------------------------------
// Cross-field validators (used by garde derive)
@@ -329,7 +331,6 @@ impl CreateEpicRequest {
.unwrap_or_default()
}
}
// ---------------------------------------------------------------------------
// CreateBugRequest
// ---------------------------------------------------------------------------
@@ -1002,7 +1003,215 @@ impl EditCriterionRequest {
})
}
}
// ---------------------------------------------------------------------------
// MoveStoryRequest
// ---------------------------------------------------------------------------
/// Fully validated inputs for the `move_story` MCP tool.
#[derive(Debug)]
pub struct MoveStoryRequest {
/// Validated story identifier.
pub story_id: StoryId,
/// Validated target pipeline stage.
pub target_stage: TargetStage,
}
impl MoveStoryRequest {
/// Parse and validate a `move_story` JSON argument map.
pub fn from_json(args: &serde_json::Value) -> Result<Self, String> {
let mut errors: Vec<ValidationError> = Vec::new();
let story_id = match args.get("story_id").and_then(|v| v.as_str()) {
None => {
errors.push(ValidationError::FieldMissing {
field: "story_id".into(),
});
None
}
Some(raw) => match StoryId::parse(raw) {
Ok(id) => Some(id),
Err(mut errs) => {
errors.append(&mut errs);
None
}
},
};
let target_stage = match args.get("target_stage").and_then(|v| v.as_str()) {
None => {
errors.push(ValidationError::FieldMissing {
field: "target_stage".into(),
});
None
}
Some(raw) => match TargetStage::parse(raw) {
Ok(s) => Some(s),
Err(mut errs) => {
errors.append(&mut errs);
None
}
},
};
if !errors.is_empty() {
return Err(format_errors_as_json(&errors));
}
Ok(MoveStoryRequest {
story_id: story_id.unwrap(),
target_stage: target_stage.unwrap(),
})
}
}
// ---------------------------------------------------------------------------
// MoveStoryToMergeRequest
// ---------------------------------------------------------------------------
/// Fully validated inputs for the `move_story_to_merge` MCP tool.
#[derive(Debug)]
pub struct MoveStoryToMergeRequest {
/// Validated story identifier.
pub story_id: StoryId,
/// Optional agent name override; defaults to `"mergemaster"` if absent.
pub agent_name: Option<String>,
}
impl MoveStoryToMergeRequest {
/// Parse and validate a `move_story_to_merge` JSON argument map.
pub fn from_json(args: &serde_json::Value) -> Result<Self, String> {
let mut errors: Vec<ValidationError> = Vec::new();
let story_id = match args.get("story_id").and_then(|v| v.as_str()) {
None => {
errors.push(ValidationError::FieldMissing {
field: "story_id".into(),
});
None
}
Some(raw) => match StoryId::parse(raw) {
Ok(id) => Some(id),
Err(mut errs) => {
errors.append(&mut errs);
None
}
},
};
let agent_name = match args.get("agent_name").and_then(|v| v.as_str()) {
None => None,
Some(raw) => {
let trimmed = raw.trim();
if trimmed.is_empty() {
errors.push(ValidationError::EmptyAfterTrim {
field: "agent_name".into(),
});
None
} else {
Some(trimmed.to_string())
}
}
};
if !errors.is_empty() {
return Err(format_errors_as_json(&errors));
}
Ok(MoveStoryToMergeRequest {
story_id: story_id.unwrap(),
agent_name,
})
}
/// Return the resolved agent name, defaulting to `"mergemaster"`.
pub fn resolved_agent_name(&self) -> &str {
self.agent_name.as_deref().unwrap_or("mergemaster")
}
}
// ---------------------------------------------------------------------------
// UnblockStoryRequest
// ---------------------------------------------------------------------------
/// Fully validated inputs for the `unblock_story` MCP tool.
#[derive(Debug)]
pub struct UnblockStoryRequest {
/// Validated story identifier.
pub story_id: StoryId,
}
impl UnblockStoryRequest {
/// Parse and validate an `unblock_story` JSON argument map.
pub fn from_json(args: &serde_json::Value) -> Result<Self, String> {
let mut errors: Vec<ValidationError> = Vec::new();
let story_id = match args.get("story_id").and_then(|v| v.as_str()) {
None => {
errors.push(ValidationError::FieldMissing {
field: "story_id".into(),
});
None
}
Some(raw) => match StoryId::parse(raw) {
Ok(id) => Some(id),
Err(mut errs) => {
errors.append(&mut errs);
None
}
},
};
if !errors.is_empty() {
return Err(format_errors_as_json(&errors));
}
Ok(UnblockStoryRequest {
story_id: story_id.unwrap(),
})
}
}
// ---------------------------------------------------------------------------
// FreezeStoryRequest
// ---------------------------------------------------------------------------
/// Fully validated inputs for the `freeze_story` MCP tool.
#[derive(Debug)]
pub struct FreezeStoryRequest {
/// Validated story identifier.
pub story_id: StoryId,
}
impl FreezeStoryRequest {
/// Parse and validate a `freeze_story` JSON argument map.
pub fn from_json(args: &serde_json::Value) -> Result<Self, String> {
let mut errors: Vec<ValidationError> = Vec::new();
let story_id = match args.get("story_id").and_then(|v| v.as_str()) {
None => {
errors.push(ValidationError::FieldMissing {
field: "story_id".into(),
});
None
}
Some(raw) => match StoryId::parse(raw) {
Ok(id) => Some(id),
Err(mut errs) => {
errors.append(&mut errs);
None
}
},
};
if !errors.is_empty() {
return Err(format_errors_as_json(&errors));
}
Ok(FreezeStoryRequest {
story_id: story_id.unwrap(),
})
}
}
#[cfg(test)]
mod tests {
use super::*;
@@ -1545,4 +1754,157 @@ mod tests {
let err = EditCriterionRequest::from_json(&args).unwrap_err();
assert!(err.contains("FieldTooLong"));
}
// --- MoveStoryRequest ---
#[test]
fn move_story_request_valid() {
let args = json!({"story_id": "42", "target_stage": "qa"});
let req = MoveStoryRequest::from_json(&args).unwrap();
assert_eq!(req.story_id.as_str(), "42");
assert_eq!(req.target_stage.as_str(), "qa");
}
#[test]
fn move_story_request_valid_with_slug() {
let args = json!({"story_id": "42_story_foo", "target_stage": "backlog"});
let req = MoveStoryRequest::from_json(&args).unwrap();
assert_eq!(req.story_id.as_str(), "42_story_foo");
}
#[test]
fn move_story_request_missing_story_id() {
let args = json!({"target_stage": "qa"});
let err = MoveStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("FieldMissing"));
assert!(err.contains("story_id"));
}
#[test]
fn move_story_request_missing_target_stage() {
let args = json!({"story_id": "42"});
let err = MoveStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("FieldMissing"));
assert!(err.contains("target_stage"));
}
#[test]
fn move_story_request_invalid_target_stage() {
let args = json!({"story_id": "42", "target_stage": "archived"});
let err = MoveStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("InvalidValue"));
assert!(err.contains("target_stage"));
}
#[test]
fn move_story_request_invalid_story_id() {
let args = json!({"story_id": "not-a-number", "target_stage": "qa"});
let err = MoveStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("InvalidCharacter"));
}
#[test]
fn move_story_request_errors_are_json() {
let args = json!({});
let err = MoveStoryRequest::from_json(&args).unwrap_err();
let parsed: serde_json::Value = serde_json::from_str(&err).unwrap();
assert!(parsed.is_array());
assert_eq!(parsed.as_array().unwrap().len(), 2);
}
// --- MoveStoryToMergeRequest ---
#[test]
fn move_story_to_merge_request_valid_minimal() {
let args = json!({"story_id": "99"});
let req = MoveStoryToMergeRequest::from_json(&args).unwrap();
assert_eq!(req.story_id.as_str(), "99");
assert_eq!(req.resolved_agent_name(), "mergemaster");
}
#[test]
fn move_story_to_merge_request_custom_agent() {
let args = json!({"story_id": "99", "agent_name": "custom-agent"});
let req = MoveStoryToMergeRequest::from_json(&args).unwrap();
assert_eq!(req.resolved_agent_name(), "custom-agent");
}
#[test]
fn move_story_to_merge_request_empty_agent_name() {
let args = json!({"story_id": "99", "agent_name": " "});
let err = MoveStoryToMergeRequest::from_json(&args).unwrap_err();
assert!(err.contains("EmptyAfterTrim"));
assert!(err.contains("agent_name"));
}
#[test]
fn move_story_to_merge_request_missing_story_id() {
let args = json!({});
let err = MoveStoryToMergeRequest::from_json(&args).unwrap_err();
assert!(err.contains("FieldMissing"));
assert!(err.contains("story_id"));
}
// --- UnblockStoryRequest ---
#[test]
fn unblock_story_request_valid() {
let args = json!({"story_id": "7"});
let req = UnblockStoryRequest::from_json(&args).unwrap();
assert_eq!(req.story_id.as_str(), "7");
assert_eq!(req.story_id.numeric_prefix(), "7");
}
#[test]
fn unblock_story_request_valid_with_slug() {
let args = json!({"story_id": "100_some_story"});
let req = UnblockStoryRequest::from_json(&args).unwrap();
assert_eq!(req.story_id.numeric_prefix(), "100");
}
#[test]
fn unblock_story_request_missing_story_id() {
let args = json!({});
let err = UnblockStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("FieldMissing"));
assert!(err.contains("story_id"));
}
#[test]
fn unblock_story_request_invalid_story_id() {
let args = json!({"story_id": ""});
let err = UnblockStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("EmptyAfterTrim"));
}
// --- FreezeStoryRequest ---
#[test]
fn freeze_story_request_valid() {
let args = json!({"story_id": "55_story_example"});
let req = FreezeStoryRequest::from_json(&args).unwrap();
assert_eq!(req.story_id.as_str(), "55_story_example");
}
#[test]
fn freeze_story_request_missing_story_id() {
let args = json!({});
let err = FreezeStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("FieldMissing"));
assert!(err.contains("story_id"));
}
#[test]
fn freeze_story_request_grammar_token_in_story_id() {
let args = json!({"story_id": "<thinking>42</thinking>"});
let err = FreezeStoryRequest::from_json(&args).unwrap_err();
assert!(err.contains("AntiGrammarToken"));
}
#[test]
fn freeze_story_request_errors_are_json() {
let args = json!({"story_id": ""});
let err = FreezeStoryRequest::from_json(&args).unwrap_err();
let parsed: serde_json::Value = serde_json::from_str(&err).unwrap();
assert!(parsed.is_array());
}
}