fix: stop image blocks breaking turns on text-only models (runtime modality recovery)

crates/jcode-app-core/src/agent.rs

@@ -19,7 +19,7 @@ mod streaming;
 #[cfg(test)]
 mod swarm_reminder_tests;
 mod tool_markup_recovery;
-mod tools;
+pub(crate) mod tools;
 mod turn_execution;
 mod turn_loops;
 mod turn_streaming_mpsc;

crates/jcode-app-core/src/agent/prompting.rs

@@ -156,14 +156,20 @@ impl Agent {
             .as_ref()
             .map(std::path::PathBuf::from);
 
-        let (split, _context_info) = crate::prompt::build_system_prompt_split_with_agents_md(
-            skill_prompt.as_deref(),
-            available_skills,
-            self.session.is_canary,
-            memory_prompt,
-            working_dir.as_deref(),
-            self.agents_md_snapshot.clone(),
-        );
+        let (split, _context_info) =
+            crate::prompt::build_system_prompt_split_with_capabilities_and_agents_md(
+                skill_prompt.as_deref(),
+                available_skills,
+                self.session.is_canary,
+                memory_prompt,
+                working_dir.as_deref(),
+                {
+                    let mut capabilities = crate::prompt::PromptCapabilities::current();
+                    capabilities.text_only_model = !self.provider.supports_image_input();
+                    capabilities
+                },
+                self.agents_md_snapshot.clone(),
+            );
 
         split
     }

crates/jcode-app-core/src/agent_tests.rs

@@ -1,5 +1,6 @@
 use super::*;
 use crate::agent::environment::EnvSnapshotDetail;
+use crate::agent::tools::tool_output_to_content_blocks;
 use crate::message::{Message, StreamEvent, ToolDefinition};
 use crate::provider::{EventStream, Provider};
 use crate::tool::Registry;
@@ -667,6 +668,115 @@ fn tool_output_to_content_blocks_preserves_labeled_images() {
     }
 }
 
+/// Text-only stub (Provider default `supports_image_input` = false) used to
+/// repro the save/reload image-loss regression: tool results with images are
+/// recorded through the same session-persistence path production turns use.
+struct TextOnlyToolResultProvider;
+
+#[async_trait]
+impl Provider for TextOnlyToolResultProvider {
+    async fn complete(
+        &self,
+        _messages: &[Message],
+        _tools: &[ToolDefinition],
+        _system: &str,
+        _resume_session_id: Option<&str>,
+    ) -> Result<EventStream> {
+        unreachable!("TextOnlyToolResultProvider does not complete requests")
+    }
+
+    fn name(&self) -> &str {
+        "text-only-tool-result-stub"
+    }
+
+    fn fork(&self) -> Arc<dyn Provider> {
+        Arc::new(Self)
+    }
+}
+
+/// A text-only provider must not strip images from persisted history: the
+/// tool-result blocks written to the session (and reloaded later for the user
+/// and for history rendering) keep the real `ContentBlock::Image`. Only the
+/// outgoing provider request is filtered, at the `MultiProvider` request
+/// chokepoint (`image_clamp::filter_unsupported_outbound_images`), which has
+/// its own tests in jcode-base.
+#[tokio::test]
+async fn text_only_provider_persists_tool_images_for_history() {
+    struct RestoreHome(Option<std::ffi::OsString>);
+    impl Drop for RestoreHome {
+        fn drop(&mut self) {
+            if let Some(home) = &self.0 {
+                crate::env::set_var("JCODE_HOME", home);
+            } else {
+                crate::env::remove_var("JCODE_HOME");
+            }
+            crate::config::Config::invalidate_cache();
+        }
+    }
+
+    let _lock = crate::storage::lock_test_env();
+    let home = tempfile::tempdir().unwrap();
+    let _restore = RestoreHome(std::env::var_os("JCODE_HOME"));
+    crate::env::set_var("JCODE_HOME", home.path());
+    crate::config::Config::invalidate_cache();
+
+    // Provider trait default: supports_image_input() == false (text-only).
+    let provider: Arc<dyn Provider> = Arc::new(TextOnlyToolResultProvider);
+    let registry = Registry::new(provider.clone()).await;
+    let mut agent = Agent::new(provider, registry);
+    assert!(
+        !agent.provider.supports_image_input(),
+        "test provider must be text-only for the regression repro"
+    );
+
+    // Streaming save/reload check: a turn that executes an image-producing
+    // tool must leave the image in the saved session, so reloads and
+    // reconnects can rebuild it from history.
+    agent
+        .add_manual_tool_use(
+            "call_1".to_string(),
+            "read".to_string(),
+            serde_json::json!({}),
+        )
+        .expect("record tool use");
+    let output = ToolOutput::new("Image ready").with_labeled_image(
+        "image/png",
+        "ZmFrZQ==",
+        "screenshots/example.png",
+    );
+    agent
+        .add_manual_tool_result("call_1".to_string(), output, 10)
+        .expect("record tool result");
+
+    let persisted = crate::session::Session::load(agent.session_id()).expect("load saved session");
+    let tool_message = persisted
+        .messages
+        .iter()
+        .find(|message| {
+            message
+                .content
+                .iter()
+                .any(|block| matches!(block, ContentBlock::ToolResult { .. }))
+        })
+        .expect("tool result message must be saved");
+    let tool_blocks = &tool_message.content;
+    assert!(
+        tool_blocks.iter().any(
+            |block| matches!(block, ContentBlock::Image { media_type, data }
+                if media_type == "image/png" && data == "ZmFrZQ==")
+        ),
+        "a text-only provider must still persist tool images into history so \
+         reloads can render them (greptile P1: they vanish on reload otherwise)"
+    );
+    assert!(
+        !tool_blocks.iter().any(|block| {
+            matches!(block, ContentBlock::Text { text, .. }
+                if text.contains("Image output omitted"))
+        }),
+        "the omission note must not replace the image in saved history"
+    );
+}
+
 #[tokio::test]
 async fn queued_soft_interrupt_images_are_injected_as_image_blocks() {
     let provider: Arc<dyn Provider> = Arc::new(NativeAutoCompactionProvider);

crates/jcode-app-core/src/replay/tests.rs

@@ -5,7 +5,7 @@ use crate::session::{StoredReplayEvent, StoredReplayEventKind};
 use chrono::{Duration, Utc};
 use std::ffi::OsString;
 
-fn lock_env() -> std::sync::MutexGuard<'static, ()> {
+fn lock_env() -> crate::storage::TestEnvGuard {
     crate::storage::lock_test_env()
 }
 

crates/jcode-app-core/src/restart_snapshot_tests.rs

@@ -9,7 +9,7 @@ use std::ffi::OsString;
 struct TestEnvGuard {
     prev_home: Option<OsString>,
     _temp_home: tempfile::TempDir,
-    _lock: std::sync::MutexGuard<'static, ()>,
+    _lock: crate::storage::TestEnvGuard,
 }
 
 impl TestEnvGuard {

crates/jcode-app-core/src/server/client_lightweight_control.rs

@@ -1033,7 +1033,7 @@ mod lightweight_swarm_member_tests {
     struct HomeGuard {
         prev: Option<std::ffi::OsString>,
         _temp: tempfile::TempDir,
-        _lock: std::sync::MutexGuard<'static, ()>,
+        _lock: crate::storage::TestEnvGuard,
     }
 
     impl Drop for HomeGuard {

crates/jcode-app-core/src/server/comm_control_tests.rs

@@ -20,7 +20,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH};
 use tokio::sync::{Mutex, RwLock, broadcast, mpsc};
 
 struct RuntimeEnvGuard {
-    _guard: std::sync::MutexGuard<'static, ()>,
+    _guard: crate::storage::TestEnvGuard,
     prev_runtime: Option<std::ffi::OsString>,
 }
 

crates/jcode-app-core/src/server/comm_plan_tests.rs

@@ -16,7 +16,7 @@ use std::time::Instant;
 use tokio::sync::{RwLock, broadcast, mpsc};
 
 struct RuntimeEnvGuard {
-    _guard: std::sync::MutexGuard<'static, ()>,
+    _guard: crate::storage::TestEnvGuard,
     prev_runtime: Option<std::ffi::OsString>,
 }
 

crates/jcode-app-core/src/server/debug_command_exec.rs

@@ -730,7 +730,7 @@ mod tests {
     /// guarding one global serialize nothing, which showed up as a rotating set
     /// of failures under `cargo test` that all passed with `--test-threads=1`
     /// (issue #593). Everything touching the environment must share one lock.
-    fn lock_env() -> std::sync::MutexGuard<'static, ()> {
+    fn lock_env() -> crate::storage::TestEnvGuard {
         crate::storage::lock_test_env()
     }
 

crates/jcode-app-core/src/server/debug_swarm_write.rs

@@ -533,7 +533,7 @@ mod tests {
     use std::time::Duration;
 
     struct EnvGuard {
-        _lock: std::sync::MutexGuard<'static, ()>,
+        _lock: crate::storage::TestEnvGuard,
         runtime: Option<std::ffi::OsString>,
     }
 

crates/jcode-app-core/src/server/debug_testers_tests.rs

@@ -1,12 +1,12 @@
 use super::{load_testers, save_testers};
 use std::ffi::OsString;
 
-fn lock_env() -> std::sync::MutexGuard<'static, ()> {
+fn lock_env() -> crate::storage::TestEnvGuard {
     crate::storage::lock_test_env()
 }
 
 struct TestHomeGuard {
-    _lock: std::sync::MutexGuard<'static, ()>,
+    _lock: crate::storage::TestEnvGuard,
     prev_home: Option<OsString>,
     _temp_home: tempfile::TempDir,
 }

crates/jcode-app-core/src/server/debug_tests.rs

@@ -428,12 +428,12 @@ mod debug_execution_tests {
     use std::sync::Arc;
     use tokio::sync::{Mutex as AsyncMutex, RwLock};
 
-    fn lock_env() -> std::sync::MutexGuard<'static, ()> {
+    fn lock_env() -> crate::storage::TestEnvGuard {
         crate::storage::lock_test_env()
     }
 
     struct EnvVarGuard {
-        _lock: std::sync::MutexGuard<'static, ()>,
+        _lock: crate::storage::TestEnvGuard,
         key: &'static str,
         previous: Option<OsString>,
     }

crates/jcode-app-core/src/server/lifecycle.rs

@@ -287,7 +287,7 @@ mod tests {
     use super::*;
 
     struct EnvGuard {
-        _lock: std::sync::MutexGuard<'static, ()>,
+        _lock: crate::storage::TestEnvGuard,
         entries: Vec<(&'static str, Option<std::ffi::OsString>)>,
     }
 

crates/jcode-app-core/src/server/provider_control_tests.rs

@@ -6,9 +6,9 @@ use crate::tool::Registry;
 use async_trait::async_trait;
 use std::collections::HashMap;
 use std::pin::Pin;
+use std::sync::Mutex as StdMutex;
 use std::sync::RwLock as StdRwLock;
 use std::sync::atomic::{AtomicUsize, Ordering};
-use std::sync::{Mutex as StdMutex, MutexGuard as StdMutexGuard};
 
 #[tokio::test]
 async fn invalidate_openai_usage_acknowledges_after_clearing_pinned_daemon_cooldown() {
@@ -381,14 +381,14 @@ impl Provider for AuthChangeMockProvider {
 
 /// Shared process-wide lock: env vars are global, so a private mutex here would
 /// race every other env-mutating test (issue #593).
-fn lock_env() -> StdMutexGuard<'static, ()> {
+fn lock_env() -> crate::storage::TestEnvGuard {
     crate::storage::lock_test_env()
 }
 
 struct EnvGuard {
     saved: Vec<(&'static str, Option<String>)>,
     _temp_home: tempfile::TempDir,
-    _lock: StdMutexGuard<'static, ()>,
+    _lock: crate::storage::TestEnvGuard,
 }
 
 impl EnvGuard {

crates/jcode-app-core/src/server/swarm_mutation_state_tests.rs

@@ -5,7 +5,7 @@ use super::{
 use crate::protocol::ServerEvent;
 
 struct RuntimeEnvGuard {
-    _guard: std::sync::MutexGuard<'static, ()>,
+    _guard: crate::storage::TestEnvGuard,
     prev_runtime: Option<std::ffi::OsString>,
 }
 

crates/jcode-app-core/src/server/swarm_persistence_tests.rs

@@ -2,7 +2,7 @@ use super::*;
 use std::time::{Duration, Instant};
 
 struct EnvGuard {
-    _lock: std::sync::MutexGuard<'static, ()>,
+    _lock: crate::storage::TestEnvGuard,
     runtime: Option<std::ffi::OsString>,
 }
 

crates/jcode-app-core/src/tool/browser_session_tests.rs

@@ -4,7 +4,7 @@ use std::os::unix::fs::PermissionsExt;
 use std::time::{Duration, Instant};
 
 struct BrowserFixture {
-    _env_guard: std::sync::MutexGuard<'static, ()>,
+    _env_guard: crate::storage::TestEnvGuard,
     temp: tempfile::TempDir,
     previous_home: Option<std::ffi::OsString>,
     previous_session: Option<std::ffi::OsString>,

crates/jcode-app-core/src/tool/browser_user_browser_tests.rs

@@ -60,7 +60,7 @@ else:
 "#;
 
 struct Fixture {
-    _env_guard: std::sync::MutexGuard<'static, ()>,
+    _env_guard: crate::storage::TestEnvGuard,
     temp: tempfile::TempDir,
     previous_home: Option<std::ffi::OsString>,
     previous_session: Option<std::ffi::OsString>,

crates/jcode-app-core/src/tool/conversation_search.rs

@@ -356,7 +356,7 @@ mod tests {
         ConversationSearchTool::new(manager)
     }
 
-    fn env_lock() -> std::sync::MutexGuard<'static, ()> {
+    fn env_lock() -> crate::storage::TestEnvGuard {
         crate::storage::lock_test_env()
     }
 

crates/jcode-base/src/auth/lifecycle.rs

@@ -1120,10 +1120,9 @@ pub fn model_switch_request_for_provider_id(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::sync::MutexGuard;
 
     struct EnvGuard {
-        _lock: MutexGuard<'static, ()>,
+        _lock: crate::storage::TestEnvGuard,
         saved: Vec<(&'static str, Option<String>)>,
     }
 

crates/jcode-base/src/auth/test_sandbox.rs

@@ -1,10 +1,9 @@
 use std::path::{Path, PathBuf};
-use std::sync::MutexGuard;
 
 use crate::provider_catalog::{OpenAiCompatibleProfile, openai_compatible_profiles};
 
 pub struct AuthTestSandbox {
-    _lock: MutexGuard<'static, ()>,
+    _lock: crate::storage::TestEnvGuard,
     temp: tempfile::TempDir,
     saved_env: Vec<(String, Option<String>)>,
 }

crates/jcode-base/src/copilot_usage.rs

@@ -106,7 +106,7 @@ mod tests {
     };
     use std::ffi::OsString;
 
-    fn lock_env() -> std::sync::MutexGuard<'static, ()> {
+    fn lock_env() -> crate::storage::TestEnvGuard {
         crate::storage::lock_test_env()
     }
 

crates/jcode-base/src/gateway/control.rs

@@ -442,7 +442,7 @@ mod tests {
     /// left them racing the config and provider suites: a test here would swap
     /// `JCODE_HOME` out from under a provider test mid-assertion, failing it
     /// intermittently whenever the two modules happened to interleave.
-    fn lock_env() -> std::sync::MutexGuard<'static, ()> {
+    fn lock_env() -> crate::storage::TestEnvGuard {
         crate::storage::lock_test_env()
     }
 

crates/jcode-base/src/mcp/oauth_tests.rs

@@ -6,7 +6,7 @@ use tokio::io::{AsyncReadExt, AsyncWriteExt};
 // These tests serve real HTTP on ephemeral 127.0.0.1 sockets. No mocked client,
 // copied OAuth implementation, external provider, browser executable or secrets.
 struct Sandbox {
-    _lock: std::sync::MutexGuard<'static, ()>,
+    _lock: crate::storage::TestEnvGuard,
     temp: tempfile::TempDir,
     saved: Option<std::ffi::OsString>,
 }