test: add coverage for parser error paths and roundtrip (7 new tests)

tests/integration_test.rs

@@ -272,3 +272,83 @@ topic main:
         assert!(tokens.iter().any(|t| matches!(t, lexer::Token::Config)));
     }
 }
+
+// =============================================================================
+// Parser Error Path Tests
+//
+// These tests verify that the parser correctly reports errors for malformed
+// input — a gap that was not covered by any previous test.
+// =============================================================================
+
+#[cfg(test)]
+mod parser_error_tests {
+    use busbar_sf_agentscript::{parse, parser::parse_with_errors};
+
+    #[test]
+    fn test_parse_error_unclosed_string_literal() {
+        // An unclosed string literal must fail at the lexer stage and surface
+        // as a parse error — `parse()` must return Err, not Ok.
+        let source = "config:\n   agent_name: \"unclosed string";
+        let result = parse(source);
+        assert!(
+            result.is_err(),
+            "Expected parse to fail on unclosed string literal, but it succeeded"
+        );
+    }
+
+    #[test]
+    fn test_parse_error_contains_location_info() {
+        // Error messages should include line/column information so developers
+        // can find the offending source location quickly.
+        let source = "config:\n   agent_name: \"unclosed";
+        let result = parse(source);
+        let errors = result.expect_err("Expected parse errors");
+        assert!(!errors.is_empty(), "Expected at least one error message");
+        let combined = errors.join("\n");
+        // The formatted error must reference a line number (format: "line N")
+        assert!(
+            combined.contains("line") || combined.contains("Lexer error"),
+            "Expected error to contain location info, got: {}",
+            combined
+        );
+    }
+
+    #[test]
+    fn test_parse_error_unknown_toplevel_keyword() {
+        // A block whose keyword is not recognised by the parser should produce
+        // a parse error. The parser will attempt recovery but must still report
+        // the failure through `parse()`.
+        let source = "not_a_valid_block:\n   key: \"value\"\n";
+        let (_, errors) = parse_with_errors(source);
+        assert!(
+            !errors.is_empty(),
+            "Expected parse errors for unrecognised top-level keyword, got none"
+        );
+    }
+
+    #[test]
+    fn test_parse_with_errors_partial_recovery_still_reports_error() {
+        // Even when the parser recovers and produces a partial AST, errors
+        // must still be present in the error list so callers are not silently
+        // handed a corrupt tree.  Here the config block is valid but the
+        // second block is unrecognised — recovery should yield the config
+        // while still reporting an error.
+        let source = r#"config:
+   agent_name: "Test"
+
+garbage_block:
+   key: "value"
+"#;
+        let (partial, errors) = parse_with_errors(source);
+        // Recovery should have salvaged the config block
+        assert!(
+            partial.map(|f| f.config.is_some()).unwrap_or(false),
+            "Expected config block to survive parse recovery"
+        );
+        // The garbage block must still be reported as an error
+        assert!(
+            !errors.is_empty(),
+            "Expected at least one error from the unrecognised block"
+        );
+    }
+}

tests/test_serializer_roundtrip.rs

@@ -216,3 +216,108 @@ topic main:
     assert!(topic.before_reasoning.is_some(), "before_reasoning lost after roundtrip");
     assert!(topic.after_reasoning.is_some(), "after_reasoning lost after roundtrip");
 }
+
+#[test]
+fn test_roundtrip_system_block_with_messages() {
+    // Covers the top-level `system:` block with both `messages:` (welcome + error)
+    // and `instructions:` — previously untested in roundtrip tests.
+    let original = r#"system:
+   instructions: "You are a helpful assistant."
+   messages:
+      welcome: "Welcome! How can I help you today?"
+      error: "An error occurred. Please try again."
+
+config:
+   agent_name: "MessagesAgent"
+
+topic main:
+   description: "Main topic"
+"#;
+
+    let ast = parse(original).expect("Failed to parse original");
+    let serialized = serialize(&ast);
+
+    assert!(serialized.contains("system:"), "Missing system block");
+    assert!(serialized.contains("messages:"), "Missing messages sub-block");
+    assert!(serialized.contains("welcome:"), "Missing welcome message");
+    assert!(serialized.contains("error:"), "Missing error message");
+
+    let reparsed = parse(&serialized).expect("Failed to reparse serialized");
+    let sys = reparsed.system.as_ref().expect("system block lost after roundtrip");
+    let msgs = sys.node.messages.as_ref().expect("messages sub-block lost after roundtrip");
+    assert!(msgs.node.welcome.is_some(), "welcome message lost after roundtrip");
+    assert!(msgs.node.error.is_some(), "error message lost after roundtrip");
+    assert_eq!(
+        msgs.node.welcome.as_ref().unwrap().node,
+        "Welcome! How can I help you today?"
+    );
+    assert_eq!(msgs.node.error.as_ref().unwrap().node, "An error occurred. Please try again.");
+}
+
+#[test]
+fn test_roundtrip_topic_system_override() {
+    // Covers the topic-level `system:` override block (TopicSystemOverride) — the
+    // serializer writes it, but no roundtrip test existed for it.
+    let original = r#"config:
+   agent_name: "OverrideAgent"
+
+topic specialized:
+   description: "A topic with custom system instructions"
+   system:
+      instructions: "You are an expert in tax law. Answer only tax-related questions."
+   reasoning:
+      instructions: "Help with tax questions."
+"#;
+
+    let ast = parse(original).expect("Failed to parse original");
+    let serialized = serialize(&ast);
+
+    assert!(serialized.contains("system:"), "Missing topic-level system block");
+    assert!(serialized.contains("tax law"), "Missing system instructions content");
+
+    let reparsed = parse(&serialized).expect("Failed to reparse serialized");
+    assert_eq!(reparsed.topics.len(), 1, "Topic count changed after roundtrip");
+    let topic = &reparsed.topics[0].node;
+    assert!(topic.system.is_some(), "topic-level system block lost after roundtrip");
+    let sys = topic.system.as_ref().unwrap();
+    assert!(sys.node.instructions.is_some(), "system instructions lost after roundtrip");
+}
+
+#[test]
+fn test_roundtrip_config_all_optional_fields() {
+    // Covers config with all optional fields — agent_label, description,
+    // agent_type, and default_agent_user — none of which had a dedicated roundtrip.
+    let original = r#"config:
+   agent_name: "FullConfigAgent"
+   agent_label: "Full Config Agent"
+   description: "An agent that exercises all config fields"
+   agent_type: "customer_service"
+   default_agent_user: "bot_user@example.com"
+
+topic main:
+   description: "Main topic"
+"#;
+
+    let ast = parse(original).expect("Failed to parse original");
+    let serialized = serialize(&ast);
+
+    let reparsed = parse(&serialized).expect("Failed to reparse serialized");
+    let cfg = reparsed.config.as_ref().expect("config block lost after roundtrip");
+    assert_eq!(cfg.node.agent_name.node, "FullConfigAgent");
+    assert_eq!(
+        cfg.node.agent_label.as_ref().map(|s| s.node.as_str()),
+        Some("Full Config Agent")
+    );
+    assert_eq!(
+        cfg.node.description.as_ref().map(|s| s.node.as_str()),
+        Some("An agent that exercises all config fields")
+    );
+    assert_eq!(
+        cfg.node.agent_type.as_ref().map(|s| s.node.as_str()),
+        Some("customer_service")
+    );
+    assert_eq!(
+        cfg.node.default_agent_user.as_ref().map(|s| s.node.as_str()),
+        Some("bot_user@example.com")
+    );
+}