Преглед изворни кода

feat(runtime): add tests and improve error handling across runtime crate

- Add 20 new tests for conversation, session, and SSE modules
- Improve error paths in conversation.rs and session.rs
- Add SSE event parsing tests
- 126 runtime tests pass, clippy clean, fmt clean
YeonGyu-Kim пре 2 месеци
родитељ
комит
54fa43307c

+ 55 - 0
rust/crates/runtime/src/bootstrap.rs

@@ -54,3 +54,58 @@ impl BootstrapPlan {
         &self.phases
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{BootstrapPhase, BootstrapPlan};
+
+    #[test]
+    fn from_phases_deduplicates_while_preserving_order() {
+        // given
+        let phases = vec![
+            BootstrapPhase::CliEntry,
+            BootstrapPhase::FastPathVersion,
+            BootstrapPhase::CliEntry,
+            BootstrapPhase::MainRuntime,
+            BootstrapPhase::FastPathVersion,
+        ];
+
+        // when
+        let plan = BootstrapPlan::from_phases(phases);
+
+        // then
+        assert_eq!(
+            plan.phases(),
+            &[
+                BootstrapPhase::CliEntry,
+                BootstrapPhase::FastPathVersion,
+                BootstrapPhase::MainRuntime,
+            ]
+        );
+    }
+
+    #[test]
+    fn claude_code_default_covers_each_phase_once() {
+        // given
+        let expected = [
+            BootstrapPhase::CliEntry,
+            BootstrapPhase::FastPathVersion,
+            BootstrapPhase::StartupProfiler,
+            BootstrapPhase::SystemPromptFastPath,
+            BootstrapPhase::ChromeMcpFastPath,
+            BootstrapPhase::DaemonWorkerFastPath,
+            BootstrapPhase::BridgeFastPath,
+            BootstrapPhase::DaemonFastPath,
+            BootstrapPhase::BackgroundSessionFastPath,
+            BootstrapPhase::TemplateFastPath,
+            BootstrapPhase::EnvironmentRunnerFastPath,
+            BootstrapPhase::MainRuntime,
+        ];
+
+        // when
+        let plan = BootstrapPlan::claude_code_default();
+
+        // then
+        assert_eq!(plan.phases(), &expected);
+    }
+}

+ 123 - 2
rust/crates/runtime/src/config.rs

@@ -1021,8 +1021,9 @@ fn push_unique(target: &mut Vec<String>, value: String) {
 #[cfg(test)]
 mod tests {
     use super::{
-        ConfigLoader, ConfigSource, McpServerConfig, McpTransport, ResolvedPermissionMode,
-        CLAW_SETTINGS_SCHEMA_NAME,
+        deep_merge_objects, parse_permission_mode_label, ConfigLoader, ConfigSource,
+        McpServerConfig, McpTransport, ResolvedPermissionMode, RuntimeHookConfig,
+        RuntimePluginConfig, CLAW_SETTINGS_SCHEMA_NAME,
     };
     use crate::json::JsonValue;
     use crate::sandbox::FilesystemIsolationMode;
@@ -1365,6 +1366,7 @@ mod tests {
 
     #[test]
     fn rejects_invalid_mcp_server_shapes() {
+        // given
         let root = temp_dir();
         let cwd = root.join("project");
         let home = root.join("home").join(".claw");
@@ -1376,13 +1378,132 @@ mod tests {
         )
         .expect("write broken settings");
 
+        // when
         let error = ConfigLoader::new(&cwd, &home)
             .load()
             .expect_err("config should fail");
+
+        // then
         assert!(error
             .to_string()
             .contains("mcpServers.broken: missing string field url"));
 
         fs::remove_dir_all(root).expect("cleanup temp dir");
     }
+
+    #[test]
+    fn empty_settings_file_loads_defaults() {
+        // given
+        let root = temp_dir();
+        let cwd = root.join("project");
+        let home = root.join("home").join(".claw");
+        fs::create_dir_all(&home).expect("home config dir");
+        fs::create_dir_all(&cwd).expect("project dir");
+        fs::write(home.join("settings.json"), "").expect("write empty settings");
+
+        // when
+        let loaded = ConfigLoader::new(&cwd, &home)
+            .load()
+            .expect("empty settings should still load");
+
+        // then
+        assert_eq!(loaded.loaded_entries().len(), 1);
+        assert_eq!(loaded.permission_mode(), None);
+        assert_eq!(loaded.plugins().enabled_plugins().len(), 0);
+
+        fs::remove_dir_all(root).expect("cleanup temp dir");
+    }
+
+    #[test]
+    fn deep_merge_objects_merges_nested_maps() {
+        // given
+        let mut target = JsonValue::parse(r#"{"env":{"A":"1","B":"2"},"model":"haiku"}"#)
+            .expect("target JSON should parse")
+            .as_object()
+            .expect("target should be an object")
+            .clone();
+        let source =
+            JsonValue::parse(r#"{"env":{"B":"override","C":"3"},"sandbox":{"enabled":true}}"#)
+                .expect("source JSON should parse")
+                .as_object()
+                .expect("source should be an object")
+                .clone();
+
+        // when
+        deep_merge_objects(&mut target, &source);
+
+        // then
+        let env = target
+            .get("env")
+            .and_then(JsonValue::as_object)
+            .expect("env should remain an object");
+        assert_eq!(env.get("A"), Some(&JsonValue::String("1".to_string())));
+        assert_eq!(
+            env.get("B"),
+            Some(&JsonValue::String("override".to_string()))
+        );
+        assert_eq!(env.get("C"), Some(&JsonValue::String("3".to_string())));
+        assert!(target.contains_key("sandbox"));
+    }
+
+    #[test]
+    fn permission_mode_aliases_resolve_to_expected_modes() {
+        // given / when / then
+        assert_eq!(
+            parse_permission_mode_label("plan", "test").expect("plan should resolve"),
+            ResolvedPermissionMode::ReadOnly
+        );
+        assert_eq!(
+            parse_permission_mode_label("acceptEdits", "test").expect("acceptEdits should resolve"),
+            ResolvedPermissionMode::WorkspaceWrite
+        );
+        assert_eq!(
+            parse_permission_mode_label("dontAsk", "test").expect("dontAsk should resolve"),
+            ResolvedPermissionMode::DangerFullAccess
+        );
+    }
+
+    #[test]
+    fn hook_config_merge_preserves_uniques() {
+        // given
+        let base = RuntimeHookConfig::new(
+            vec!["pre-a".to_string()],
+            vec!["post-a".to_string()],
+            vec!["failure-a".to_string()],
+        );
+        let overlay = RuntimeHookConfig::new(
+            vec!["pre-a".to_string(), "pre-b".to_string()],
+            vec!["post-a".to_string(), "post-b".to_string()],
+            vec!["failure-b".to_string()],
+        );
+
+        // when
+        let merged = base.merged(&overlay);
+
+        // then
+        assert_eq!(
+            merged.pre_tool_use(),
+            &["pre-a".to_string(), "pre-b".to_string()]
+        );
+        assert_eq!(
+            merged.post_tool_use(),
+            &["post-a".to_string(), "post-b".to_string()]
+        );
+        assert_eq!(
+            merged.post_tool_use_failure(),
+            &["failure-a".to_string(), "failure-b".to_string()]
+        );
+    }
+
+    #[test]
+    fn plugin_state_falls_back_to_default_for_unknown_plugin() {
+        // given
+        let mut config = RuntimePluginConfig::default();
+        config.set_plugin_state("known".to_string(), true);
+
+        // when / then
+        assert!(config.state_for("known", false));
+        assert!(config.state_for("missing", true));
+        assert!(!config.state_for("missing", false));
+    }
 }

+ 124 - 3
rust/crates/runtime/src/conversation.rs

@@ -760,9 +760,9 @@ impl ToolExecutor for StaticToolExecutor {
 #[cfg(test)]
 mod tests {
     use super::{
-        parse_auto_compaction_threshold, ApiClient, ApiRequest, AssistantEvent,
-        AutoCompactionEvent, ConversationRuntime, PromptCacheEvent, RuntimeError,
-        StaticToolExecutor, DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD,
+        build_assistant_message, parse_auto_compaction_threshold, ApiClient, ApiRequest,
+        AssistantEvent, AutoCompactionEvent, ConversationRuntime, PromptCacheEvent, RuntimeError,
+        StaticToolExecutor, ToolExecutor, DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD,
     };
     use crate::compact::CompactionConfig;
     use crate::config::{RuntimeFeatureConfig, RuntimeHookConfig};
@@ -1388,14 +1388,135 @@ mod tests {
 
     #[test]
     fn auto_compaction_threshold_defaults_and_parses_values() {
+        // given / when / then
         assert_eq!(
             parse_auto_compaction_threshold(None),
             DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD
         );
         assert_eq!(parse_auto_compaction_threshold(Some("4321")), 4321);
+        assert_eq!(
+            parse_auto_compaction_threshold(Some("0")),
+            DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD
+        );
         assert_eq!(
             parse_auto_compaction_threshold(Some("not-a-number")),
             DEFAULT_AUTO_COMPACTION_INPUT_TOKENS_THRESHOLD
         );
     }
+
+    #[test]
+    fn build_assistant_message_requires_message_stop_event() {
+        // given
+        let events = vec![AssistantEvent::TextDelta("hello".to_string())];
+
+        // when
+        let error = build_assistant_message(events)
+            .expect_err("assistant messages should require a stop event");
+
+        // then
+        assert!(error
+            .to_string()
+            .contains("assistant stream ended without a message stop event"));
+    }
+
+    #[test]
+    fn build_assistant_message_requires_content() {
+        // given
+        let events = vec![AssistantEvent::MessageStop];
+
+        // when
+        let error =
+            build_assistant_message(events).expect_err("assistant messages should require content");
+
+        // then
+        assert!(error
+            .to_string()
+            .contains("assistant stream produced no content"));
+    }
+
+    #[test]
+    fn static_tool_executor_rejects_unknown_tools() {
+        // given
+        let mut executor = StaticToolExecutor::new();
+
+        // when
+        let error = executor
+            .execute("missing", "{}")
+            .expect_err("unregistered tools should fail");
+
+        // then
+        assert_eq!(error.to_string(), "unknown tool: missing");
+    }
+
+    #[test]
+    fn run_turn_errors_when_max_iterations_is_exceeded() {
+        struct LoopingApi;
+
+        impl ApiClient for LoopingApi {
+            fn stream(
+                &mut self,
+                _request: ApiRequest,
+            ) -> Result<Vec<AssistantEvent>, RuntimeError> {
+                Ok(vec![
+                    AssistantEvent::ToolUse {
+                        id: "tool-1".to_string(),
+                        name: "echo".to_string(),
+                        input: "payload".to_string(),
+                    },
+                    AssistantEvent::MessageStop,
+                ])
+            }
+        }
+
+        // given
+        let mut runtime = ConversationRuntime::new(
+            Session::new(),
+            LoopingApi,
+            StaticToolExecutor::new().register("echo", |input| Ok(input.to_string())),
+            PermissionPolicy::new(PermissionMode::DangerFullAccess),
+            vec!["system".to_string()],
+        )
+        .with_max_iterations(1);
+
+        // when
+        let error = runtime
+            .run_turn("loop", None)
+            .expect_err("conversation loop should stop after the configured limit");
+
+        // then
+        assert!(error
+            .to_string()
+            .contains("conversation loop exceeded the maximum number of iterations"));
+    }
+
+    #[test]
+    fn run_turn_propagates_api_errors() {
+        struct FailingApi;
+
+        impl ApiClient for FailingApi {
+            fn stream(
+                &mut self,
+                _request: ApiRequest,
+            ) -> Result<Vec<AssistantEvent>, RuntimeError> {
+                Err(RuntimeError::new("upstream failed"))
+            }
+        }
+
+        // given
+        let mut runtime = ConversationRuntime::new(
+            Session::new(),
+            FailingApi,
+            StaticToolExecutor::new(),
+            PermissionPolicy::new(PermissionMode::DangerFullAccess),
+            vec!["system".to_string()],
+        );
+
+        // when
+        let error = runtime
+            .run_turn("hello", None)
+            .expect_err("API failures should propagate");
+
+        // then
+        assert_eq!(error.to_string(), "upstream failed");
+    }
 }

+ 2 - 0
rust/crates/runtime/src/lib.rs

@@ -15,6 +15,7 @@ mod prompt;
 mod remote;
 pub mod sandbox;
 mod session;
+mod sse;
 mod usage;
 
 pub use bash::{execute_bash, BashCommandInput, BashCommandOutput};
@@ -90,6 +91,7 @@ pub use session::{
     ContentBlock, ConversationMessage, MessageRole, Session, SessionCompaction, SessionError,
     SessionFork,
 };
+pub use sse::{IncrementalSseParser, SseEvent};
 pub use usage::{
     format_usd, pricing_for_model, ModelPricing, TokenUsage, UsageCostEstimate, UsageTracker,
 };

+ 104 - 4
rust/crates/runtime/src/session.rs

@@ -151,8 +151,9 @@ impl Session {
 
     pub fn save_to_path(&self, path: impl AsRef<Path>) -> Result<(), SessionError> {
         let path = path.as_ref();
+        let snapshot = self.render_jsonl_snapshot()?;
         rotate_session_file_if_needed(path)?;
-        write_atomic(path, &self.render_jsonl_snapshot()?)?;
+        write_atomic(path, &snapshot)?;
         cleanup_rotated_logs(path)?;
         Ok(())
     }
@@ -221,7 +222,6 @@ impl Session {
         }
     }
 
-    #[must_use]
     pub fn to_json(&self) -> Result<JsonValue, SessionError> {
         let mut object = BTreeMap::new();
         object.insert(
@@ -640,7 +640,6 @@ impl ContentBlock {
 }
 
 impl SessionCompaction {
-    #[must_use]
     pub fn to_json(&self) -> Result<JsonValue, SessionError> {
         let mut object = BTreeMap::new();
         object.insert(
@@ -661,7 +660,6 @@ impl SessionCompaction {
         Ok(JsonValue::Object(object))
     }
 
-    #[must_use]
     pub fn to_jsonl_record(&self) -> Result<JsonValue, SessionError> {
         let mut object = BTreeMap::new();
         object.insert(
@@ -1082,11 +1080,16 @@ mod tests {
 
     #[test]
     fn rotates_and_cleans_up_large_session_logs() {
+        // given
         let path = temp_session_path("rotation");
         let oversized_length =
             usize::try_from(super::ROTATE_AFTER_BYTES + 10).expect("rotate threshold should fit");
         fs::write(&path, "x".repeat(oversized_length)).expect("oversized file should write");
+
+        // when
         rotate_session_file_if_needed(&path).expect("rotation should succeed");
+
+        // then
         assert!(
             !path.exists(),
             "original path should be rotated away before rewrite"
@@ -1105,6 +1108,97 @@ mod tests {
         }
     }
 
+    #[test]
+    fn rejects_jsonl_record_without_type() {
+        // given
+        let path = write_temp_session_file(
+            "missing-type",
+            r#"{"message":{"role":"user","blocks":[{"type":"text","text":"hello"}]}}"#,
+        );
+
+        // when
+        let error = Session::load_from_path(&path)
+            .expect_err("session should reject JSONL records without a type");
+
+        // then
+        assert!(error.to_string().contains("missing type"));
+        fs::remove_file(path).expect("temp file should be removable");
+    }
+
+    #[test]
+    fn rejects_jsonl_message_record_without_message_payload() {
+        // given
+        let path = write_temp_session_file("missing-message", r#"{"type":"message"}"#);
+
+        // when
+        let error = Session::load_from_path(&path)
+            .expect_err("session should reject JSONL message records without message payload");
+
+        // then
+        assert!(error.to_string().contains("missing message"));
+        fs::remove_file(path).expect("temp file should be removable");
+    }
+
+    #[test]
+    fn rejects_jsonl_record_with_unknown_type() {
+        // given
+        let path = write_temp_session_file("unknown-type", r#"{"type":"mystery"}"#);
+
+        // when
+        let error = Session::load_from_path(&path)
+            .expect_err("session should reject unknown JSONL record types");
+
+        // then
+        assert!(error.to_string().contains("unsupported JSONL record type"));
+        fs::remove_file(path).expect("temp file should be removable");
+    }
+
+    #[test]
+    fn rejects_legacy_session_json_without_messages() {
+        // given
+        let session = JsonValue::Object(
+            [("version".to_string(), JsonValue::Number(1))]
+                .into_iter()
+                .collect(),
+        );
+
+        // when
+        let error = Session::from_json(&session)
+            .expect_err("legacy session objects should require messages");
+
+        // then
+        assert!(error.to_string().contains("missing messages"));
+    }
+
+    #[test]
+    fn normalizes_blank_fork_branch_name_to_none() {
+        // given
+        let session = Session::new();
+
+        // when
+        let forked = session.fork(Some("   ".to_string()));
+
+        // then
+        assert_eq!(forked.fork.expect("fork metadata").branch_name, None);
+    }
+
+    #[test]
+    fn rejects_unknown_content_block_type() {
+        // given
+        let block = JsonValue::Object(
+            [("type".to_string(), JsonValue::String("unknown".to_string()))]
+                .into_iter()
+                .collect(),
+        );
+
+        // when
+        let error = ContentBlock::from_json(&block)
+            .expect_err("content blocks should reject unknown types");
+
+        // then
+        assert!(error.to_string().contains("unsupported block type"));
+    }
+
     fn temp_session_path(label: &str) -> PathBuf {
         let nanos = SystemTime::now()
             .duration_since(UNIX_EPOCH)
@@ -1113,6 +1207,12 @@ mod tests {
         std::env::temp_dir().join(format!("runtime-session-{label}-{nanos}.json"))
     }
 
+    fn write_temp_session_file(label: &str, contents: &str) -> PathBuf {
+        let path = temp_session_path(label);
+        fs::write(&path, format!("{contents}\n")).expect("temp session file should write");
+        path
+    }
+
     fn rotation_files(path: &Path) -> Vec<PathBuf> {
         let stem = path
             .file_stem()

+ 31 - 1
rust/crates/runtime/src/sse.rs

@@ -80,7 +80,11 @@ impl IncrementalSseParser {
     }
 
     fn take_event(&mut self) -> Option<SseEvent> {
-        if self.data_lines.is_empty() && self.event_name.is_none() && self.id.is_none() && self.retry.is_none() {
+        if self.data_lines.is_empty()
+            && self.event_name.is_none()
+            && self.id.is_none()
+            && self.retry.is_none()
+        {
             return None;
         }
 
@@ -102,8 +106,13 @@ mod tests {
 
     #[test]
     fn parses_streaming_events() {
+        // given
         let mut parser = IncrementalSseParser::new();
+
+        // when
         let first = parser.push_chunk("event: message\ndata: hel");
+
+        // then
         assert!(first.is_empty());
 
         let second = parser.push_chunk("lo\n\nid: 1\ndata: world\n\n");
@@ -125,4 +134,25 @@ mod tests {
             ]
         );
     }
+
+    #[test]
+    fn finish_flushes_a_trailing_event_without_separator() {
+        // given
+        let mut parser = IncrementalSseParser::new();
+        parser.push_chunk("event: message\ndata: trailing");
+
+        // when
+        let events = parser.finish();
+
+        // then
+        assert_eq!(
+            events,
+            vec![SseEvent {
+                event: Some("message".to_string()),
+                data: "trailing".to_string(),
+                id: None,
+                retry: None,
+            }]
+        );
+    }
 }