From c324452b3865ff8bbb7deffdf4f4ad6d0e3c66e1 Mon Sep 17 00:00:00 2001 From: dave Date: Thu, 9 Apr 2026 22:35:52 +0000 Subject: [PATCH] fix: commit uncommitted native JSON type changes on master MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These changes (HashMap → HashMap for front matter, json_value_to_yaml_scalar, and oneOf schema for front_matter) were left uncommitted on master after a previous merge, blocking the cherry-pick step of story 509's merge. Co-Authored-By: Claude Opus 4.6 --- server/src/http/mcp/mod.rs | 10 +- server/src/http/mcp/story_tools.rs | 12 +-- server/src/http/workflow/story_ops.rs | 136 ++++++++++++++++++++++---- 3 files changed, 132 insertions(+), 26 deletions(-) diff --git a/server/src/http/mcp/mod.rs b/server/src/http/mcp/mod.rs index 5980ef06..8a31ff0c 100644 --- a/server/src/http/mcp/mod.rs +++ b/server/src/http/mcp/mod.rs @@ -664,9 +664,15 @@ fn handle_tools_list(id: Option) -> JsonRpcResponse { }, "front_matter": { "type": "object", - "description": "Arbitrary YAML front matter key-value pairs to set or update", + "description": "Arbitrary YAML front matter key-value pairs to set or update. Values may be strings, booleans, integers, numbers, or arrays (e.g. [490, 491]).", "additionalProperties": { - "type": "string" + "oneOf": [ + {"type": "string"}, + {"type": "boolean"}, + {"type": "integer"}, + {"type": "number"}, + {"type": "array"} + ] } } }, diff --git a/server/src/http/mcp/story_tools.rs b/server/src/http/mcp/story_tools.rs index 3fc6c284..a4a78df6 100644 --- a/server/src/http/mcp/story_tools.rs +++ b/server/src/http/mcp/story_tools.rs @@ -353,17 +353,15 @@ pub(super) fn tool_update_story(args: &Value, ctx: &AppContext) -> Result = HashMap::new(); + // Values are passed as serde_json::Value so native booleans, numbers, and arrays are + // preserved and encoded correctly as unquoted YAML by update_story_in_file. + let mut front_matter: HashMap = HashMap::new(); if let Some(agent) = args.get("agent").and_then(|v| v.as_str()) { - front_matter.insert("agent".to_string(), agent.to_string()); + front_matter.insert("agent".to_string(), Value::String(agent.to_string())); } if let Some(obj) = args.get("front_matter").and_then(|v| v.as_object()) { for (k, v) in obj { - let val = match v { - Value::String(s) => s.clone(), - other => other.to_string(), - }; - front_matter.insert(k.clone(), val); + front_matter.insert(k.clone(), v.clone()); } } let front_matter_opt = if front_matter.is_empty() { diff --git a/server/src/http/workflow/story_ops.rs b/server/src/http/workflow/story_ops.rs index 794a282e..2e093c39 100644 --- a/server/src/http/workflow/story_ops.rs +++ b/server/src/http/workflow/story_ops.rs @@ -1,4 +1,5 @@ use crate::io::story_metadata::set_front_matter_field; +use serde_json::Value; use std::collections::HashMap; use std::path::Path; @@ -173,15 +174,37 @@ pub fn add_criterion_to_file( Ok(()) } -/// Encode a string value as a YAML scalar. +/// Encode a JSON value as a YAML scalar string. +/// +/// Native JSON types map to native YAML types: +/// - Bool → unquoted `true`/`false` +/// - Integer → unquoted integer +/// - Float → unquoted float +/// - Array → unquoted inline sequence (e.g. `[490, 491]`) +/// - String → quoted unless it looks like a bool, integer, or inline sequence +fn json_value_to_yaml_scalar(value: &Value) -> String { + match value { + Value::Bool(b) => b.to_string(), + Value::Number(n) => n.to_string(), + Value::Array(arr) => { + let items: Vec = arr.iter().map(|v| v.to_string()).collect(); + format!("[{}]", items.join(", ")) + } + Value::String(s) => yaml_encode_str(s), + // Null and Object are not meaningful as YAML scalars; store as quoted strings. + other => format!("\"{}\"", other.to_string().replace('"', "\\\"").replace('\n', " ").replace('\r', "")), + } +} + +/// Encode a plain string as a YAML scalar. /// /// Booleans (`true`/`false`), integers, and inline sequences (`[...]`) are -/// written as native YAML types (unquoted). Everything else is written as a -/// quoted string to avoid ambiguity. -fn yaml_encode_scalar(value: &str) -> String { - match value { - "true" | "false" => value.to_string(), +/// written unquoted. Everything else is quoted to avoid ambiguity. +fn yaml_encode_str(s: &str) -> String { + match s { + "true" | "false" => s.to_string(), s if s.parse::().is_ok() => s.to_string(), + s if s.parse::().is_ok() => s.to_string(), // YAML inline sequences like [490] or [490, 491] — write unquoted so // serde_yaml can deserialise them as Vec. s if s.starts_with('[') && s.ends_with(']') => s.to_string(), @@ -198,7 +221,7 @@ pub fn update_story_in_file( story_id: &str, user_story: Option<&str>, description: Option<&str>, - front_matter: Option<&HashMap>, + front_matter: Option<&HashMap>, ) -> Result<(), String> { let has_front_matter_updates = front_matter.map(|m| !m.is_empty()).unwrap_or(false); if user_story.is_none() && description.is_none() && !has_front_matter_updates { @@ -212,7 +235,7 @@ pub fn update_story_in_file( if let Some(fields) = front_matter { for (key, value) in fields { - let yaml_value = yaml_encode_scalar(value); + let yaml_value = json_value_to_yaml_scalar(value); contents = set_front_matter_field(&contents, key, &yaml_value); } } @@ -489,7 +512,7 @@ mod tests { setup_story_in_fs(tmp.path(), "24_test", "---\nname: T\n---\n\n## User Story\n\nSome story\n"); let mut fields = HashMap::new(); - fields.insert("agent".to_string(), "dev".to_string()); + fields.insert("agent".to_string(), Value::String("dev".to_string())); update_story_in_file(tmp.path(), "24_test", None, None, Some(&fields)).unwrap(); let result = read_story_content(tmp.path(), "24_test").unwrap(); @@ -503,8 +526,8 @@ mod tests { setup_story_in_fs(tmp.path(), "25_test", "---\nname: T\n---\n\n## User Story\n\nSome story\n"); let mut fields = HashMap::new(); - fields.insert("qa".to_string(), "human".to_string()); - fields.insert("priority".to_string(), "high".to_string()); + fields.insert("qa".to_string(), Value::String("human".to_string())); + fields.insert("priority".to_string(), Value::String("high".to_string())); update_story_in_file(tmp.path(), "25_test", None, None, Some(&fields)).unwrap(); let result = read_story_content(tmp.path(), "25_test").unwrap(); @@ -519,7 +542,7 @@ mod tests { setup_story_in_fs(tmp.path(), "26_test", "---\nname: T\n---\n\nNo sections here.\n"); let mut fields = HashMap::new(); - fields.insert("agent".to_string(), "dev".to_string()); + fields.insert("agent".to_string(), Value::String("dev".to_string())); let result = update_story_in_file(tmp.path(), "26_test", None, None, Some(&fields)); assert!(result.is_ok(), "front-matter-only update should not require body sections"); @@ -532,8 +555,9 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); setup_story_in_fs(tmp.path(), "27_test", "---\nname: T\n---\n\nNo sections.\n"); + // String "false" still works (backwards compatibility). let mut fields = HashMap::new(); - fields.insert("blocked".to_string(), "false".to_string()); + fields.insert("blocked".to_string(), Value::String("false".to_string())); update_story_in_file(tmp.path(), "27_test", None, None, Some(&fields)).unwrap(); let result = read_story_content(tmp.path(), "27_test").unwrap(); @@ -546,8 +570,9 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); setup_story_in_fs(tmp.path(), "28_test", "---\nname: T\n---\n\nNo sections.\n"); + // String "0" still works (backwards compatibility). let mut fields = HashMap::new(); - fields.insert("retry_count".to_string(), "0".to_string()); + fields.insert("retry_count".to_string(), Value::String("0".to_string())); update_story_in_file(tmp.path(), "28_test", None, None, Some(&fields)).unwrap(); let result = read_story_content(tmp.path(), "28_test").unwrap(); @@ -561,7 +586,7 @@ mod tests { setup_story_in_fs(tmp.path(), "29_test", "---\nname: My Story\n---\n\nNo sections.\n"); let mut fields = HashMap::new(); - fields.insert("blocked".to_string(), "false".to_string()); + fields.insert("blocked".to_string(), Value::String("false".to_string())); update_story_in_file(tmp.path(), "29_test", None, None, Some(&fields)).unwrap(); let contents = read_story_content(tmp.path(), "29_test").unwrap(); @@ -576,8 +601,9 @@ mod tests { let tmp = tempfile::tempdir().unwrap(); setup_story_in_fs(tmp.path(), "30_test", "---\nname: T\n---\n\nNo sections.\n"); + // String "[490]" still works (backwards compatibility). let mut fields = HashMap::new(); - fields.insert("depends_on".to_string(), "[490]".to_string()); + fields.insert("depends_on".to_string(), Value::String("[490]".to_string())); update_story_in_file(tmp.path(), "30_test", None, None, Some(&fields)).unwrap(); let result = read_story_content(tmp.path(), "30_test").unwrap(); @@ -615,13 +641,89 @@ mod tests { assert_eq!(meta.depends_on, Some(vec![489])); } + // ── Story 504: native JSON types in front_matter ─────────────────────────── + + #[test] + fn update_story_native_bool_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "31_test", "---\nname: T\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::Bool(false)); + update_story_in_file(tmp.path(), "31_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "31_test").unwrap(); + assert!(result.contains("blocked: false"), "native bool false should be unquoted: {result}"); + assert!(!result.contains("blocked: \"false\""), "must not be quoted: {result}"); + } + + #[test] + fn update_story_native_bool_true_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "32_test", "---\nname: T\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::Bool(true)); + update_story_in_file(tmp.path(), "32_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "32_test").unwrap(); + assert!(result.contains("blocked: true"), "native bool true should be unquoted: {result}"); + assert!(!result.contains("blocked: \"true\""), "must not be quoted: {result}"); + } + + #[test] + fn update_story_native_integer_written_unquoted() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "33_test", "---\nname: T\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("retry_count".to_string(), serde_json::json!(3)); + update_story_in_file(tmp.path(), "33_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "33_test").unwrap(); + assert!(result.contains("retry_count: 3"), "native integer should be unquoted: {result}"); + assert!(!result.contains("retry_count: \"3\""), "must not be quoted: {result}"); + } + + #[test] + fn update_story_native_array_written_as_yaml_sequence() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "34_test", "---\nname: T\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("depends_on".to_string(), serde_json::json!([490, 491])); + update_story_in_file(tmp.path(), "34_test", None, None, Some(&fields)).unwrap(); + + let result = read_story_content(tmp.path(), "34_test").unwrap(); + assert!(result.contains("depends_on: [490, 491]"), "native array should be YAML sequence: {result}"); + assert!(!result.contains("depends_on: \"["), "must not be quoted: {result}"); + + let meta = parse_front_matter(&result).expect("front matter should parse"); + assert_eq!(meta.depends_on, Some(vec![490, 491])); + } + + #[test] + fn update_story_native_bool_parseable_after_write() { + let tmp = tempfile::tempdir().unwrap(); + setup_story_in_fs(tmp.path(), "35_test", "---\nname: My Story\n---\n\nNo sections.\n"); + + let mut fields = HashMap::new(); + fields.insert("blocked".to_string(), Value::Bool(false)); + update_story_in_file(tmp.path(), "35_test", None, None, Some(&fields)).unwrap(); + + let contents = read_story_content(tmp.path(), "35_test").unwrap(); + let meta = parse_front_matter(&contents).expect("front matter should parse"); + assert_eq!(meta.name.as_deref(), Some("My Story"), "name preserved after writing native bool"); + } + #[test] fn update_story_depends_on_multi_element_array() { let tmp = tempfile::tempdir().unwrap(); setup_story_in_fs(tmp.path(), "31_test", "---\nname: T\n---\n\nNo sections.\n"); + // String "[490, 491]" still works (backwards compatibility). let mut fields = HashMap::new(); - fields.insert("depends_on".to_string(), "[490, 491]".to_string()); + fields.insert("depends_on".to_string(), Value::String("[490, 491]".to_string())); update_story_in_file(tmp.path(), "31_test", None, None, Some(&fields)).unwrap(); let result = read_story_content(tmp.path(), "31_test").unwrap();