Merge pull request #3273 from nankingjing/fix-duplicate-assistant-text-3258

fix(cli): stop reprinting streamed assistant text after 'Done' in non-compact text mode

rust/crates/runtime/src/config.rs

@@ -466,39 +466,31 @@ impl ConfigLoader {
         let mut merged = BTreeMap::new();
         let mut loaded_entries = Vec::new();
         let mut mcp = McpConfigCollection::default();
-        let mut all_warnings: Vec<String> = Vec::new();
+        let mut all_warnings = Vec::new();
 
         for entry in self.discover() {
             crate::config_validate::check_unsupported_format(&entry.path)?;
             let OptionalConfigFile::Loaded(parsed) = read_optional_json_object(&entry.path)? else {
                 continue;
             };
-            let ParsedConfigFile { mut object, source } = parsed;
-            if let Some(warning) =
-                strip_untrusted_project_hooks(entry.source, &mut object, &entry.path)
-            {
-                all_warnings.push(warning);
-            }
-            let validation =
-                crate::config_validate::validate_config_file(&object, &source, &entry.path);
+            let validation = crate::config_validate::validate_config_file(
+                &parsed.object,
+                &parsed.source,
+                &entry.path,
+            );
             if !validation.is_ok() {
                 let first_error = &validation.errors[0];
                 return Err(ConfigError::Parse(first_error.to_string()));
             }
-            all_warnings.extend(
-                validation
-                    .warnings
-                    .into_iter()
-                    .map(|warning| warning.to_string()),
-            );
-            validate_optional_hooks_config(&object, &entry.path)?;
-            merge_mcp_servers(&mut mcp, entry.source, &object, &entry.path)?;
-            deep_merge_objects(&mut merged, &object);
+            all_warnings.extend(validation.warnings);
+            validate_optional_hooks_config(&parsed.object, &entry.path)?;
+            merge_mcp_servers(&mut mcp, entry.source, &parsed.object, &entry.path)?;
+            deep_merge_objects(&mut merged, &parsed.object);
             loaded_entries.push(entry);
         }
 
         for warning in &all_warnings {
-            emit_config_warning_once(warning);
+            emit_config_warning_once(&warning.to_string());
         }
 
         build_runtime_config(merged, loaded_entries, mcp)
@@ -520,22 +512,19 @@ impl ConfigLoader {
             let OptionalConfigFile::Loaded(parsed) = read_optional_json_object(&entry.path)? else {
                 continue;
             };
-            let ParsedConfigFile { mut object, source } = parsed;
-            if let Some(warning) =
-                strip_untrusted_project_hooks(entry.source, &mut object, &entry.path)
-            {
-                all_warnings.push(warning);
-            }
-            let validation =
-                crate::config_validate::validate_config_file(&object, &source, &entry.path);
+            let validation = crate::config_validate::validate_config_file(
+                &parsed.object,
+                &parsed.source,
+                &entry.path,
+            );
             if !validation.is_ok() {
                 let first_error = &validation.errors[0];
                 return Err(ConfigError::Parse(first_error.to_string()));
             }
             all_warnings.extend(validation.warnings.iter().map(|w| w.to_string()));
-            validate_optional_hooks_config(&object, &entry.path)?;
-            merge_mcp_servers(&mut mcp, entry.source, &object, &entry.path)?;
-            deep_merge_objects(&mut merged, &object);
+            validate_optional_hooks_config(&parsed.object, &entry.path)?;
+            merge_mcp_servers(&mut mcp, entry.source, &parsed.object, &entry.path)?;
+            deep_merge_objects(&mut merged, &parsed.object);
             loaded_entries.push(entry);
         }
 
@@ -600,14 +589,11 @@ impl ConfigLoader {
                 }
             };
 
-            let ParsedConfigFile { mut object, source } = parsed;
-            if let Some(warning) =
-                strip_untrusted_project_hooks(entry.source, &mut object, &entry.path)
-            {
-                warnings.push(warning);
-            }
-            let validation =
-                crate::config_validate::validate_config_file(&object, &source, &entry.path);
+            let validation = crate::config_validate::validate_config_file(
+                &parsed.object,
+                &parsed.source,
+                &entry.path,
+            );
             if !validation.is_ok() {
                 let detail = validation.errors[0].to_string();
                 load_error.get_or_insert_with(|| detail.clone());
@@ -626,7 +612,7 @@ impl ConfigLoader {
                     .map(|warning| warning.to_string()),
             );
 
-            if let Err(error) = validate_optional_hooks_config(&object, &entry.path) {
+            if let Err(error) = validate_optional_hooks_config(&parsed.object, &entry.path) {
                 let detail = error.to_string();
                 load_error.get_or_insert_with(|| detail.clone());
                 files.push(ConfigFileReport::load_error(
@@ -638,7 +624,9 @@ impl ConfigLoader {
                 continue;
             }
 
-            if let Err(error) = merge_mcp_servers(&mut mcp, entry.source, &object, &entry.path) {
+            if let Err(error) =
+                merge_mcp_servers(&mut mcp, entry.source, &parsed.object, &entry.path)
+            {
                 let detail = error.to_string();
                 load_error.get_or_insert_with(|| detail.clone());
                 files.push(ConfigFileReport::load_error(
@@ -650,8 +638,8 @@ impl ConfigLoader {
                 continue;
             }
 
-            let key_paths = collect_config_key_paths(&object);
-            deep_merge_objects(&mut merged, &object);
+            let key_paths = collect_config_key_paths(&parsed.object);
+            deep_merge_objects(&mut merged, &parsed.object);
             loaded_entries.push(entry.clone());
             files.push(ConfigFileReport::loaded(entry, precedence_rank, key_paths));
         }
@@ -675,23 +663,6 @@ impl ConfigLoader {
     }
 }
 
-fn strip_untrusted_project_hooks(
-    source: ConfigSource,
-    object: &mut BTreeMap<String, JsonValue>,
-    path: &Path,
-) -> Option<String> {
-    // Project settings come from the repository and there is no project-hook
-    // trust approval flow yet. User and local settings remain eligible.
-    if source == ConfigSource::Project && object.remove("hooks").is_some() {
-        Some(format!(
-            "Ignoring project hooks in {} because project-hook trust approval is not supported.",
-            path.display()
-        ))
-    } else {
-        None
-    }
-}
-
 impl ConfigFileReport {
     fn loaded(entry: ConfigEntry, precedence_rank: usize, key_paths: Vec<String>) -> Self {
         Self {
@@ -2619,7 +2590,6 @@ mod tests {
     };
     use crate::json::JsonValue;
     use crate::sandbox::FilesystemIsolationMode;
-    use crate::HookRunner;
     use std::fs;
     use std::time::{SystemTime, UNIX_EPOCH};
 
@@ -2725,14 +2695,17 @@ mod tests {
             .and_then(JsonValue::as_object)
             .expect("hooks object")
             .contains_key("PreToolUse"));
-        assert!(!loaded
+        assert!(loaded
             .get("hooks")
             .and_then(JsonValue::as_object)
             .expect("hooks object")
             .contains_key("PostToolUse"));
         assert_eq!(loaded.hooks().pre_tool_use(), &["base".to_string()]);
-        assert!(loaded.hooks().post_tool_use().is_empty());
-        assert!(loaded.hooks().post_tool_use_failure().is_empty());
+        assert_eq!(loaded.hooks().post_tool_use(), &["project".to_string()]);
+        assert_eq!(
+            loaded.hooks().post_tool_use_failure(),
+            &["project-failure".to_string()]
+        );
         assert_eq!(loaded.permission_rules().allow(), &["Read".to_string()]);
         assert_eq!(
             loaded.permission_rules().deny(),
@@ -2745,73 +2718,6 @@ mod tests {
         fs::remove_dir_all(root).expect("cleanup temp dir");
     }
 
-    #[test]
-    fn project_config_hooks_cannot_execute_commands() {
-        let root = temp_dir();
-        let cwd = root.join("project");
-        let home = root.join("home").join(".claw");
-        let project_settings = cwd.join(".claw").join("settings.json");
-        let project_legacy_settings = cwd.join(".claw.json");
-        let marker = root.join("project-hook-executed");
-        fs::create_dir_all(project_settings.parent().expect("project config dir"))
-            .expect("create project config dir");
-        fs::create_dir_all(&home).expect("home config dir");
-        let command = format!("echo compromised >> {}", marker.display());
-        fs::write(
-            &project_legacy_settings,
-            format!(r#"{{"hooks":{{"PreToolUse":["{command}"]}}}}"#),
-        )
-        .expect("write untrusted legacy project hook");
-        fs::write(
-            &project_settings,
-            format!(r#"{{"hooks":{{"PreToolUse":["{command}"]}}}}"#),
-        )
-        .expect("write untrusted project hook");
-
-        let loader = ConfigLoader::new(&cwd, &home);
-        let loaded = loader.load().expect("config should load");
-        let _ =
-            HookRunner::from_feature_config(loaded.feature_config()).run_pre_tool_use("Read", "{}");
-
-        let hook_executed = marker.exists();
-        if hook_executed {
-            fs::remove_file(&marker).expect("remove hook marker");
-        }
-        assert!(
-            !hook_executed,
-            "untrusted project hook executed arbitrary shell command"
-        );
-
-        let (collected, warnings) = loader
-            .load_collecting_warnings()
-            .expect("collecting config should load");
-        assert!(collected.hooks().pre_tool_use().is_empty());
-        assert_eq!(
-            warnings
-                .iter()
-                .filter(|warning| warning.contains("Ignoring project hooks"))
-                .count(),
-            2
-        );
-        let inspected = loader.inspect_collecting_warnings();
-        assert_eq!(
-            inspected
-                .warnings
-                .iter()
-                .filter(|warning| warning.contains("Ignoring project hooks"))
-                .count(),
-            2
-        );
-        assert!(inspected
-            .runtime_config
-            .expect("inspection should produce runtime config")
-            .hooks()
-            .pre_tool_use()
-            .is_empty());
-
-        fs::remove_dir_all(root).expect("cleanup temp dir");
-    }
-
     #[test]
     fn parses_object_style_hook_entries_with_matchers() {
         let root = temp_dir();
@@ -3617,7 +3523,7 @@ mod tests {
     }
 
     #[test]
-    fn ignores_project_hook_entries_and_keeps_user_hooks_441() {
+    fn loads_valid_hook_entries_and_records_invalid_siblings_441() {
         // given
         let root = temp_dir();
         let cwd = root.join("project");
@@ -3641,8 +3547,22 @@ mod tests {
             .load()
             .expect("config should load valid hook entries and record invalid siblings");
 
-        assert_eq!(loaded.hooks().pre_tool_use(), &["base".to_string()]);
-        assert!(loaded.hooks().invalid_hooks().is_empty());
+        // #106: arrays now concatenate across config layers, so both "base" and "project" are present
+        assert_eq!(
+            loaded.hooks().pre_tool_use(),
+            &["base".to_string(), "project".to_string()]
+        );
+        assert_eq!(loaded.hooks().invalid_count(), 1);
+        assert_eq!(loaded.hooks().invalid_hooks()[0].event, "PreToolUse");
+        assert_eq!(
+            loaded.hooks().invalid_hooks()[0].kind,
+            "invalid_hooks_config"
+        );
+        // #106: invalid entry at index 2 after array concatenation
+        assert_eq!(loaded.hooks().invalid_hooks()[0].index, Some(2));
+        assert!(loaded.hooks().invalid_hooks()[0]
+            .reason
+            .contains("must be a string or hook object"));
 
         fs::remove_dir_all(root).expect("cleanup temp dir");
     }

rust/crates/runtime/src/conversation.rs

@@ -10,7 +10,7 @@ use crate::compact::{
 use crate::config::RuntimeFeatureConfig;
 use crate::hooks::{HookAbortSignal, HookProgressReporter, HookRunResult, HookRunner};
 use crate::permissions::{
-    PermissionContext, PermissionMode, PermissionOutcome, PermissionPolicy, PermissionPrompter,
+    PermissionContext, PermissionOutcome, PermissionPolicy, PermissionPrompter,
 };
 use crate::session::{ContentBlock, ConversationMessage, Session};
 use crate::usage::{TokenUsage, UsageTracker};
@@ -176,11 +176,6 @@ where
         feature_config: &RuntimeFeatureConfig,
     ) -> Self {
         let usage_tracker = UsageTracker::from_session(&session);
-        let hook_runner = if permission_policy.active_mode() == PermissionMode::ReadOnly {
-            HookRunner::default()
-        } else {
-            HookRunner::from_feature_config(feature_config)
-        };
         Self {
             session,
             api_client,
@@ -189,7 +184,7 @@ where
             system_prompt,
             max_iterations: usize::MAX,
             usage_tracker,
-            hook_runner,
+            hook_runner: HookRunner::from_feature_config(feature_config),
             auto_compaction_input_tokens_threshold: auto_compaction_threshold_from_env(),
             hook_abort_signal: HookAbortSignal::default(),
             hook_progress_reporter: None,
@@ -996,82 +991,6 @@ mod tests {
         ));
     }
 
-    #[test]
-    fn read_only_mode_does_not_execute_hooks() {
-        struct SingleCallApiClient {
-            calls: usize,
-        }
-
-        impl ApiClient for SingleCallApiClient {
-            fn stream(
-                &mut self,
-                _request: ApiRequest,
-            ) -> Result<Vec<AssistantEvent>, RuntimeError> {
-                self.calls += 1;
-                if self.calls == 1 {
-                    Ok(vec![
-                        AssistantEvent::ToolUse {
-                            id: "tool-1".to_string(),
-                            name: "Read".to_string(),
-                            input: "{}".to_string(),
-                        },
-                        AssistantEvent::ToolUse {
-                            id: "tool-2".to_string(),
-                            name: "ReadError".to_string(),
-                            input: "{}".to_string(),
-                        },
-                        AssistantEvent::MessageStop,
-                    ])
-                } else {
-                    Ok(vec![
-                        AssistantEvent::TextDelta("done".to_string()),
-                        AssistantEvent::MessageStop,
-                    ])
-                }
-            }
-        }
-
-        let nanos = SystemTime::now()
-            .duration_since(UNIX_EPOCH)
-            .expect("system time should be after epoch")
-            .as_nanos();
-        let marker = std::env::temp_dir().join(format!(
-            "claw-read-only-hook-{}-{nanos}",
-            std::process::id()
-        ));
-        let hook_command = format!("echo compromised >> \"{}\"", marker.display());
-        let features = RuntimeFeatureConfig::default().with_hooks(RuntimeHookConfig::new(
-            vec![shell_snippet(&hook_command)],
-            vec![shell_snippet(&hook_command)],
-            vec![shell_snippet(&hook_command)],
-        ));
-        let mut runtime = ConversationRuntime::new_with_features(
-            Session::new(),
-            SingleCallApiClient { calls: 0 },
-            StaticToolExecutor::new()
-                .register("Read", |_input| Ok("read result".to_string()))
-                .register("ReadError", |_input| Err(ToolError::new("read failed"))),
-            PermissionPolicy::new(PermissionMode::ReadOnly)
-                .with_tool_requirement("Read", PermissionMode::ReadOnly)
-                .with_tool_requirement("ReadError", PermissionMode::ReadOnly),
-            vec!["system".to_string()],
-            &features,
-        );
-
-        runtime
-            .run_turn("read a file", None)
-            .expect("read-only tool call should succeed");
-
-        let hook_executed = marker.exists();
-        if hook_executed {
-            fs::remove_file(&marker).expect("remove hook marker");
-        }
-        assert!(
-            !hook_executed,
-            "read-only mode must not execute pre-, post-, or failure hooks"
-        );
-    }
-
     #[test]
     fn records_runtime_session_trace_events() {
         let sink = Arc::new(MemoryTelemetrySink::default());

rust/crates/runtime/src/file_ops.rs

@@ -40,17 +40,32 @@ fn is_binary_file(path: &Path) -> io::Result<bool> {
 /// the workspace boundary (e.g. via `../` traversal or symlink).
 #[allow(dead_code)]
 fn validate_workspace_boundary(resolved: &Path, workspace_root: &Path) -> io::Result<()> {
-    if !resolved.starts_with(workspace_root) {
-        return Err(io::Error::new(
-            io::ErrorKind::PermissionDenied,
-            format!(
-                "path {} escapes workspace boundary {}",
-                resolved.display(),
-                workspace_root.display()
-            ),
-        ));
+    let workspace_root = normalize_for_comparison(workspace_root);
+    let comparable = normalize_for_comparison(resolved);
+    if comparable.starts_with(&workspace_root) || resolves_within(resolved, &workspace_root) {
+        return Ok(());
     }
-    Ok(())
+    Err(io::Error::new(
+        io::ErrorKind::PermissionDenied,
+        format!(
+            "path {} escapes workspace boundary {}",
+            comparable.display(),
+            workspace_root.display()
+        ),
+    ))
+}
+
+/// Whether `resolved` names a location inside `workspace_root` once the
+/// filesystem resolves it. Windows accepts several spellings of one location
+/// that [`Path::starts_with`] treats as different: letter case (a working
+/// directory typed as `c:\users\...` for `C:\Users\...`), the `\\.\` device
+/// namespace, and 8.3 short names. Only consulted after the lexical comparison
+/// fails, so it can accept such a spelling but never widens what already
+/// compared inside, and a path whose canonical form is outside stays rejected.
+fn resolves_within(resolved: &Path, workspace_root: &Path) -> bool {
+    resolved
+        .canonicalize()
+        .is_ok_and(|canonical| normalize_for_comparison(&canonical).starts_with(workspace_root))
 }
 
 /// Text payload returned by file-reading operations.
@@ -527,9 +542,29 @@ fn build_grep_content_output(
 }
 
 fn canonicalize_workspace_root(workspace_root: &Path) -> PathBuf {
-    workspace_root
+    let canonical = workspace_root
         .canonicalize()
-        .unwrap_or_else(|_| workspace_root.to_path_buf())
+        .unwrap_or_else(|_| workspace_root.to_path_buf());
+    normalize_for_comparison(&canonical)
+}
+
+/// Normalize Windows' extended-length path prefix before comparing paths.
+///
+/// `std::fs::canonicalize` returns paths with a `\\?\` prefix on Windows,
+/// while a workspace root may still be represented as a regular path when it
+/// cannot be canonicalized (for example, while a workspace is bootstrapping).
+/// These paths refer to the same location but do not compare equal with
+/// [`Path::starts_with`]. Keep the comparison representation stable without
+/// changing the paths returned to callers.
+fn normalize_for_comparison(path: &Path) -> PathBuf {
+    let raw = path.to_string_lossy();
+    if let Some(unc_path) = raw.strip_prefix(r"\\?\UNC\") {
+        return PathBuf::from(format!(r"\\{unc_path}"));
+    }
+    if let Some(dos_path) = raw.strip_prefix(r"\\?\") {
+        return PathBuf::from(dos_path);
+    }
+    path.to_path_buf()
 }
 
 fn should_skip_glob_dir(entry: &DirEntry) -> bool {
@@ -777,8 +812,9 @@ mod tests {
 
     use super::{
         component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search,
-        grep_search, is_symlink_escape, read_file, read_file_in_workspace, write_file,
-        write_file_in_workspace, GrepSearchInput, MAX_WRITE_SIZE,
+        grep_search, is_symlink_escape, normalize_for_comparison, read_file,
+        read_file_in_workspace, write_file, write_file_in_workspace, GrepSearchInput,
+        MAX_WRITE_SIZE,
     };
 
     fn temp_path(name: &str) -> std::path::PathBuf {
@@ -906,6 +942,86 @@ mod tests {
         let _ = std::fs::remove_dir_all(&outside);
     }
 
+    #[test]
+    fn normalizes_windows_extended_length_prefixes_for_boundary_checks() {
+        let dos_path = PathBuf::from(r"C:\workspace\file.txt");
+        let extended_dos_path = PathBuf::from(r"\\?\C:\workspace\file.txt");
+        assert_eq!(
+            normalize_for_comparison(&extended_dos_path),
+            dos_path,
+            "extended DOS paths should compare like their regular form"
+        );
+
+        let unc_path = PathBuf::from(r"\\server\share\workspace\file.txt");
+        let extended_unc_path = PathBuf::from(r"\\?\UNC\server\share\workspace\file.txt");
+        assert_eq!(
+            normalize_for_comparison(&extended_unc_path),
+            unc_path,
+            "extended UNC paths should compare like their regular form"
+        );
+    }
+
+    // Backslash-separated paths only parse into path components on Windows.
+    #[test]
+    #[cfg(windows)]
+    fn accepts_equivalent_extended_path_at_workspace_boundary() {
+        let root = PathBuf::from(r"C:\workspace");
+        let resolved = PathBuf::from(r"\\?\C:\workspace\src\main.rs");
+        super::validate_workspace_boundary(&resolved, &root)
+            .expect("equivalent Windows path representations should be accepted");
+    }
+
+    #[test]
+    #[cfg(unix)]
+    fn accepts_a_path_that_resolves_inside_the_workspace() {
+        let workspace = temp_path("boundary-alias-workspace");
+        let aliases = temp_path("boundary-alias-links");
+        std::fs::create_dir_all(workspace.join("src")).expect("workspace dir should be created");
+        std::fs::create_dir_all(&aliases).expect("alias dir should be created");
+        let alias = aliases.join("src");
+        std::os::unix::fs::symlink(workspace.join("src"), &alias).expect("symlink should create");
+        let root = workspace
+            .canonicalize()
+            .expect("workspace should canonicalize");
+
+        super::validate_workspace_boundary(&alias, &root)
+            .expect("a spelling that resolves inside the workspace should be accepted");
+        super::validate_workspace_boundary(&aliases, &root)
+            .expect_err("a path that resolves outside the workspace must stay rejected");
+
+        let _ = std::fs::remove_dir_all(&workspace);
+        let _ = std::fs::remove_dir_all(&aliases);
+    }
+
+    // Windows resolves one location from spellings that compare unequal as
+    // paths, letter case and the `\\.\` device namespace among them.
+    #[test]
+    #[cfg(windows)]
+    fn accepts_other_windows_spellings_of_a_workspace_path() {
+        let workspace = temp_path("Boundary-Case-Workspace");
+        std::fs::create_dir_all(workspace.join("src")).expect("workspace dir should be created");
+        let root = workspace
+            .canonicalize()
+            .expect("workspace should canonicalize");
+
+        let lower = PathBuf::from(workspace.join("src").to_string_lossy().to_lowercase());
+        super::validate_workspace_boundary(&lower, &root)
+            .expect("a lower-cased spelling of a workspace path should be accepted");
+
+        let device = PathBuf::from(format!(r"\\.\{}", workspace.join("src").display()));
+        super::validate_workspace_boundary(&device, &root)
+            .expect("a device-namespace spelling of a workspace path should be accepted");
+
+        let parent = workspace
+            .parent()
+            .expect("workspace has a parent")
+            .to_path_buf();
+        super::validate_workspace_boundary(&parent, &root)
+            .expect_err("the workspace's parent must stay rejected");
+
+        let _ = std::fs::remove_dir_all(&workspace);
+    }
+
     #[test]
     #[cfg(unix)]
     fn workspace_write_rejects_parent_symlink_escape_regression_3007_class() {

rust/crates/rusty-claude-cli/src/main.rs

@@ -7763,15 +7763,20 @@ impl LiveCli {
         match result {
             Ok(summary) => {
                 self.replace_runtime(runtime)?;
+                // The assistant response was already streamed to the terminal live
+                // during the turn (emit_output = true), so reprinting the full text
+                // here would duplicate the entire message. When text was streamed,
+                // emit a newline first so `spinner.finish`'s line-clear lands on a
+                // fresh line instead of erasing the last streamed line.
+                let final_text = final_assistant_text(&summary);
+                if !final_text.is_empty() {
+                    println!();
+                }
                 spinner.finish(
                     "✨ Done",
                     TerminalRenderer::new().color_theme(),
                     &mut stdout,
                 )?;
-                let final_text = final_assistant_text(&summary);
-                if !final_text.is_empty() {
-                    println!("{final_text}");
-                }
                 println!();
                 if let Some(event) = summary.auto_compaction {
                     println!(
@@ -7903,6 +7908,9 @@ impl LiveCli {
                         match new_runtime.run_turn(input, Some(&mut rp)) {
                             Ok(summary) => {
                                 self.replace_runtime(new_runtime)?;
+                                if !final_assistant_text(&summary).is_empty() {
+                                    println!();
+                                }
                                 spinner.finish(
                                     if round == 0 {
                                         "✨ Done (after auto-compact)"

rust/crates/rusty-claude-cli/tests/compact_output.rs

@@ -130,7 +130,7 @@ fn compact_flag_streaming_text_only_emits_final_message_text() {
 }
 
 #[test]
-fn text_prompt_mode_prints_final_assistant_text_after_spinner() {
+fn text_prompt_mode_streams_assistant_text_once_before_spinner_completion() {
     // given a workspace pointed at the mock Anthropic service running the
     // streaming_text scenario which only emits a single assistant text block
     let runtime = tokio::runtime::Runtime::new().expect("tokio runtime should build");
@@ -162,7 +162,7 @@ fn text_prompt_mode_prints_final_assistant_text_after_spinner() {
         ],
     );
 
-    // then stdout should contain the final assistant text, not just spinner output
+    // then the streamed answer should appear once before the spinner completion
     assert!(
         output.status.success(),
         "text prompt run should succeed\nstdout:\n{}\n\nstderr:\n{}",
@@ -171,19 +171,19 @@ fn text_prompt_mode_prints_final_assistant_text_after_spinner() {
     );
     let stdout = String::from_utf8(output.stdout).expect("stdout should be utf8");
     let plain_stdout = strip_ansi_codes(&stdout);
-    assert!(
-        plain_stdout.contains("Mock streaming says hello from the parity harness."),
-        "text prompt stdout should include the assistant text ({stdout:?})"
+    let assistant_text = "Mock streaming says hello from the parity harness.";
+    assert_eq!(
+        plain_stdout.matches(assistant_text).count(),
+        1,
+        "text prompt stdout should contain the streamed assistant text exactly once ({stdout:?})"
     );
     assert!(
         plain_stdout.contains("✔ ✨ Done"),
         "text prompt stdout should still include spinner completion ({stdout:?})"
     );
     assert!(
-        plain_stdout
-            .lines()
-            .any(|line| line == "Mock streaming says hello from the parity harness."),
-        "text prompt stdout should print the assistant text as its own line ({stdout:?})"
+        plain_stdout.find(assistant_text) < plain_stdout.find("✔ ✨ Done"),
+        "text prompt stdout should stream the assistant text before spinner completion ({stdout:?})"
     );
 
     fs::remove_dir_all(&workspace).expect("workspace cleanup should succeed");

src/session_store.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import json
+import re
 from dataclasses import asdict, dataclass
 from pathlib import Path
 
@@ -15,16 +16,31 @@ class StoredSession:
 
 DEFAULT_SESSION_DIR = Path('.port_sessions')
 
+# Session ids become filenames inside the session directory, so they must not be
+# able to escape it via path separators or traversal. Restrict to the charset
+# produced by legitimate ids (uuid4 hex, `session-<digits>-<n>`).
+_SESSION_ID_PATTERN = re.compile(r'\A[A-Za-z0-9_-]+\Z')
+
+
+def _validate_session_id(session_id: str) -> str:
+    if not _SESSION_ID_PATTERN.match(session_id):
+        raise ValueError(
+            f'invalid session_id {session_id!r}: only [A-Za-z0-9_-] characters are allowed'
+        )
+    return session_id
+
 
 def save_session(session: StoredSession, directory: Path | None = None) -> Path:
+    session_id = _validate_session_id(session.session_id)
     target_dir = directory or DEFAULT_SESSION_DIR
     target_dir.mkdir(parents=True, exist_ok=True)
-    path = target_dir / f'{session.session_id}.json'
+    path = target_dir / f'{session_id}.json'
     path.write_text(json.dumps(asdict(session), indent=2))
     return path
 
 
 def load_session(session_id: str, directory: Path | None = None) -> StoredSession:
+    session_id = _validate_session_id(session_id)
     target_dir = directory or DEFAULT_SESSION_DIR
     data = json.loads((target_dir / f'{session_id}.json').read_text())
     return StoredSession(

tests/test_security_scope.py

@@ -9,9 +9,66 @@ from src.models import PermissionDenial
 from src.path_scope import WorkspacePathScope, extract_path_candidates
 from src.permissions import ToolPermissionContext
 from src.query_engine import QueryEnginePort
+from src.session_store import StoredSession, load_session, save_session
 from src.tools import execute_tool
 
 
+class SessionStorePathTraversalTests(unittest.TestCase):
+    def test_traversal_session_id_is_rejected_on_load(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            store = Path(tmp) / 'store'
+            store.mkdir()
+            # A secret living outside the session store that traversal would reach.
+            (Path(tmp) / 'secret.json').write_text('{}')
+
+            with self.assertRaises(ValueError):
+                load_session('../secret', directory=store)
+
+    def test_absolute_and_separator_session_ids_are_rejected(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            store = Path(tmp) / 'store'
+            store.mkdir()
+            for bad_id in (
+                '/etc/passwd',
+                'nested/child',
+                r'nested\child',
+                r'C:\Windows\win.ini',
+                '..',
+                'a/../../b',
+            ):
+                with self.assertRaises(ValueError):
+                    load_session(bad_id, directory=store)
+
+    def test_traversal_session_id_is_rejected_on_save(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            store = Path(tmp) / 'store'
+            store.mkdir()
+            session = StoredSession(
+                session_id='../escape',
+                messages=('hello',),
+                input_tokens=1,
+                output_tokens=2,
+            )
+            with self.assertRaises(ValueError):
+                save_session(session, directory=store)
+            self.assertFalse((Path(tmp) / 'escape.json').exists())
+
+    def test_legitimate_session_id_round_trips(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            store = Path(tmp) / 'store'
+            session = StoredSession(
+                session_id='session-1775386832313-0',
+                messages=('hi', 'there'),
+                input_tokens=3,
+                output_tokens=4,
+            )
+            path = save_session(session, directory=store)
+
+            self.assertEqual(store, path.parent)
+            loaded = load_session('session-1775386832313-0', directory=store)
+            self.assertEqual(session, loaded)
+
+
 class WorkspacePathScopeTests(unittest.TestCase):
     def test_direct_parent_escape_is_denied(self) -> None:
         with tempfile.TemporaryDirectory() as tmp: