fix(operation): use cdata for warning messages to prevent html escaping

crates/forge_app/src/fmt/fmt_output.rs

@@ -142,7 +142,7 @@ mod tests {
             output: FsCreateOutput {
                 path: "/home/user/project/new_file.txt".to_string(),
                 before: None,
-                warning: None,
+                errors: vec![],
                 content_hash: crate::compute_hash(content),
             },
         };
@@ -166,7 +166,7 @@ mod tests {
             output: FsCreateOutput {
                 path: "/home/user/project/existing_file.txt".to_string(),
                 before: Some("old content".to_string()),
-                warning: None,
+                errors: vec![],
                 content_hash: crate::compute_hash(content),
             },
         };
@@ -194,7 +194,11 @@ mod tests {
             output: FsCreateOutput {
                 path: "/home/user/project/file.txt".to_string(),
                 before: None,
-                warning: Some("File created outside project directory".to_string()),
+                errors: vec![forge_domain::SyntaxError {
+                    line: 5,
+                    column: 10,
+                    message: "Syntax error".to_string(),
+                }],
                 content_hash: crate::compute_hash(content),
             },
         };
@@ -313,7 +317,7 @@ mod tests {
                 operation: PatchOperation::Replace,
             },
             output: PatchOutput {
-                warning: None,
+                errors: vec![],
                 before: "Hello world\nThis is a test".to_string(),
                 after: after_content.to_string(),
                 content_hash: crate::compute_hash(after_content),
@@ -336,7 +340,11 @@ mod tests {
                 operation: PatchOperation::Replace,
             },
             output: PatchOutput {
-                warning: Some("Large file modification".to_string()),
+                errors: vec![forge_domain::SyntaxError {
+                    line: 10,
+                    column: 5,
+                    message: "Syntax error".to_string(),
+                }],
                 before: "line1\nline2".to_string(),
                 after: after_content.to_string(),
                 content_hash: crate::compute_hash(after_content),

crates/forge_app/src/operation.rs

@@ -189,6 +189,31 @@ fn create_stream_element<T: StreamElement>(
 
     Some(elem)
 }
+
+/// Creates a validation warning element for syntax errors
+///
+/// # Arguments
+/// * `path` - The file path
+/// * `errors` - Vector of syntax errors
+///
+/// Returns an Element containing the formatted warning with all error details
+fn create_validation_warning(path: &str, errors: &[forge_domain::SyntaxError]) -> Element {
+    Element::new("warning")
+        .append(Element::new("message").text("Syntax validation failed"))
+        .append(Element::new("file").attr("path", path))
+        .append(Element::new("details").text(format!(
+            "The file was written successfully but contains {} syntax error(s)",
+            errors.len()
+        )))
+        .append(errors.iter().map(|error| {
+            Element::new("error")
+                .attr("line", error.line.to_string())
+                .attr("column", error.column.to_string())
+                .cdata(&error.message)
+        }))
+        .append(Element::new("suggestion").text("Review and fix the syntax issues"))
+}
+
 impl ToolOperation {
     pub fn into_tool_output(
         self,
@@ -247,11 +272,11 @@ impl ToolOperation {
                 };
 
                 elm = elm
-                    .attr("path", input.path)
+                    .attr("path", &input.path)
                     .attr("total_lines", input.content.lines().count());
 
-                if let Some(warning) = output.warning {
-                    elm = elm.append(Element::new("warning").text(warning));
+                if !output.errors.is_empty() {
+                    elm = elm.append(create_validation_warning(&input.path, &output.errors));
                 }
 
                 forge_domain::ToolOutput::text(elm)
@@ -397,8 +422,8 @@ impl ToolOperation {
                     .attr("total_lines", output.after.lines().count())
                     .cdata(diff);
 
-                if let Some(warning) = &output.warning {
-                    elm = elm.append(Element::new("warning").text(warning));
+                if !output.errors.is_empty() {
+                    elm = elm.append(create_validation_warning(&input.path, &output.errors));
                 }
 
                 *metrics = metrics.clone().insert(
@@ -615,6 +640,20 @@ mod tests {
         result
     }
 
+    /// Creates test syntax errors for testing purposes
+    fn test_syntax_errors(errors: Vec<(u32, u32, &str)>) -> Vec<forge_domain::SyntaxError> {
+        use forge_domain::SyntaxError;
+
+        errors
+            .into_iter()
+            .map(|(line, column, message)| SyntaxError {
+                line,
+                column,
+                message: message.to_string(),
+            })
+            .collect()
+    }
+
     // Helper functions for semantic search tests
     mod sem_search_helpers {
         use fake::{Fake, Faker};
@@ -807,7 +846,7 @@ mod tests {
             output: FsCreateOutput {
                 path: "/home/user/new_file.txt".to_string(),
                 before: None,
-                warning: None,
+                errors: vec![],
                 content_hash: compute_hash(content),
             },
         };
@@ -836,7 +875,7 @@ mod tests {
             output: FsCreateOutput {
                 path: "/home/user/existing_file.txt".to_string(),
                 before: Some("Old content".to_string()),
-                warning: None,
+                errors: vec![],
                 content_hash: compute_hash(content),
             },
         };
@@ -1311,7 +1350,39 @@ mod tests {
             output: FsCreateOutput {
                 path: "/home/user/file_with_warning.txt".to_string(),
                 before: None,
-                warning: Some("File created in non-standard location".to_string()),
+                errors: test_syntax_errors(vec![(10, 5, "Syntax error on line 10")]),
+                content_hash: compute_hash(content),
+            },
+        };
+
+        let env = fixture_environment();
+
+        let actual = fixture.into_tool_output(
+            ToolKind::Write,
+            TempContentFiles::default(),
+            &env,
+            &mut Metrics::default(),
+        );
+
+        insta::assert_snapshot!(to_value(actual));
+    }
+
+    #[test]
+    fn test_fs_create_with_warning_xml_tags() {
+        let content = "Content with warning";
+        let fixture = ToolOperation::FsCreate {
+            input: forge_domain::FSWrite {
+                path: "/home/user/file_with_warning.txt".to_string(),
+                content: content.to_string(),
+                overwrite: false,
+            },
+            output: FsCreateOutput {
+                path: "/home/user/file_with_warning.txt".to_string(),
+                before: None,
+                errors: test_syntax_errors(vec![
+                    (10, 5, "Syntax error on line 10"),
+                    (20, 15, "Missing semicolon"),
+                ]),
                 content_hash: compute_hash(content),
             },
         };
@@ -1425,7 +1496,7 @@ mod tests {
                 content: "universe".to_string(),
             },
             output: PatchOutput {
-                warning: None,
+                errors: vec![],
                 before: "Hello world\nThis is a test".to_string(),
                 after: after_content.to_string(),
                 content_hash: compute_hash(after_content),
@@ -1455,7 +1526,45 @@ mod tests {
                 content: "\nnew line".to_string(),
             },
             output: PatchOutput {
-                warning: Some("Large file modification".to_string()),
+                errors: test_syntax_errors(vec![(5, 10, "Invalid syntax")]),
+                before: "line1\nline2".to_string(),
+                after: after_content.to_string(),
+                content_hash: compute_hash(after_content),
+            },
+        };
+
+        let env = fixture_environment();
+
+        let actual = fixture.into_tool_output(
+            ToolKind::Patch,
+            TempContentFiles::default(),
+            &env,
+            &mut Metrics::default(),
+        );
+
+        insta::assert_snapshot!(to_value(actual));
+    }
+
+    #[test]
+    fn test_fs_patch_with_warning_special_chars() {
+        let after_content = "line1\nnew line\nline2";
+        let fixture = ToolOperation::FsPatch {
+            input: forge_domain::FSPatch {
+                path: "/home/user/test.zsh".to_string(),
+                search: Some("line1".to_string()),
+                operation: forge_domain::PatchOperation::Append,
+                content: "\nnew line".to_string(),
+            },
+            output: PatchOutput {
+                errors: test_syntax_errors(vec![
+                    (
+                        22,
+                        1,
+                        r#"Syntax error at 'function dim() { echo "${_DIM}${1}${RESET}"'"#,
+                    ),
+                    (25, 5, "Unexpected token"),
+                    (30, 10, "Missing closing brace"),
+                ]),
                 before: "line1\nline2".to_string(),
                 after: after_content.to_string(),
                 content_hash: compute_hash(after_content),

crates/forge_app/src/services.rs

@@ -7,8 +7,8 @@ use forge_domain::{
     AgentId, AnyProvider, Attachment, AuthContextRequest, AuthContextResponse, AuthMethod,
     ChatCompletionMessage, CommandOutput, Context, Conversation, ConversationId, Environment, File,
     Image, InitAuth, LoginInfo, McpConfig, McpServers, Model, ModelId, Node, PatchOperation,
-    Provider, ProviderId, ResultStream, Scope, SearchParams, SyncProgress, Template, ToolCallFull,
-    ToolOutput, Workflow, WorkspaceAuth, WorkspaceId, WorkspaceInfo,
+    Provider, ProviderId, ResultStream, Scope, SearchParams, SyncProgress, SyntaxError, Template,
+    ToolCallFull, ToolOutput, Workflow, WorkspaceAuth, WorkspaceId, WorkspaceInfo,
 };
 use merge::Merge;
 use reqwest::Response;
@@ -27,7 +27,7 @@ pub struct ShellOutput {
 
 #[derive(Debug)]
 pub struct PatchOutput {
-    pub warning: Option<String>,
+    pub errors: Vec<SyntaxError>,
     pub before: String,
     pub after: String,
     pub content_hash: String,
@@ -96,7 +96,7 @@ pub struct FsCreateOutput {
     pub path: String,
     // Set when the file already exists
     pub before: Option<String>,
-    pub warning: Option<String>,
+    pub errors: Vec<SyntaxError>,
     pub content_hash: String,
 }
 

crates/forge_app/src/snapshots/forge_app__operation__tests__fs_create_with_warning.snap

@@ -6,5 +6,18 @@ expression: to_value(actual)
   path="/home/user/file_with_warning.txt"
   total_lines="1"
 >
-<warning>File created in non-standard location</warning>
+<warning>
+<message>Syntax validation failed</message>
+<file
+  path="/home/user/file_with_warning.txt"
+>
+</file>
+<details>The file was written successfully but contains 1 syntax error(s)</details>
+<error
+  line="10"
+  column="5"
+><![CDATA[Syntax error on line 10]]>
+</error>
+<suggestion>Review and fix the syntax issues</suggestion>
+</warning>
 </file_created>

crates/forge_app/src/snapshots/forge_app__operation__tests__fs_create_with_warning_xml_tags.snap

@@ -0,0 +1,28 @@
+---
+source: crates/forge_app/src/operation.rs
+expression: to_value(actual)
+---
+<file_created
+  path="/home/user/file_with_warning.txt"
+  total_lines="1"
+>
+<warning>
+<message>Syntax validation failed</message>
+<file
+  path="/home/user/file_with_warning.txt"
+>
+</file>
+<details>The file was written successfully but contains 2 syntax error(s)</details>
+<error
+  line="10"
+  column="5"
+><![CDATA[Syntax error on line 10]]>
+</error>
+<error
+  line="20"
+  column="15"
+><![CDATA[Missing semicolon]]>
+</error>
+<suggestion>Review and fix the syntax issues</suggestion>
+</warning>
+</file_created>

crates/forge_app/src/snapshots/forge_app__operation__tests__fs_patch_with_warning.snap

@@ -9,5 +9,18 @@ expression: to_value(actual)
   2 |+new line
 2 3 | line2
 ]]>
-<warning>Large file modification</warning>
+<warning>
+<message>Syntax validation failed</message>
+<file
+  path="/home/user/large_file.txt"
+>
+</file>
+<details>The file was written successfully but contains 1 syntax error(s)</details>
+<error
+  line="5"
+  column="10"
+><![CDATA[Invalid syntax]]>
+</error>
+<suggestion>Review and fix the syntax issues</suggestion>
+</warning>
 </file_diff>

crates/forge_app/src/snapshots/forge_app__operation__tests__fs_patch_with_warning_special_chars.snap

@@ -0,0 +1,36 @@
+---
+source: crates/forge_app/src/operation.rs
+expression: to_value(actual)
+---
+<file_diff
+  path="/home/user/test.zsh"
+  total_lines="3"
+><![CDATA[1 1 | line1
+  2 |+new line
+2 3 | line2
+]]>
+<warning>
+<message>Syntax validation failed</message>
+<file
+  path="/home/user/test.zsh"
+>
+</file>
+<details>The file was written successfully but contains 3 syntax error(s)</details>
+<error
+  line="22"
+  column="1"
+><![CDATA[Syntax error at 'function dim() { echo "${_DIM}${1}${RESET}"']]>
+</error>
+<error
+  line="25"
+  column="5"
+><![CDATA[Unexpected token]]>
+</error>
+<error
+  line="30"
+  column="10"
+><![CDATA[Missing closing brace]]>
+</error>
+<suggestion>Review and fix the syntax issues</suggestion>
+</warning>
+</file_diff>

crates/forge_domain/src/lib.rs

@@ -47,6 +47,7 @@ mod top_k;
 mod top_p;
 mod transformer;
 mod update;
+mod validation;
 mod workflow;
 mod workspace;
 mod xml;
@@ -97,6 +98,7 @@ pub use top_k::*;
 pub use top_p::*;
 pub use transformer::*;
 pub use update::*;
+pub use validation::*;
 pub use workflow::*;
 pub use workspace::*;
 pub use xml::*;