Update: 将子项目从 submodule 转为完整内容

- 移除 GovAI, nomifun-tauri, 算力盒子的 submodule 引用 - 添加所有子项目的完整源代码 - 保留原始 .git 为 .git.bak 备份
2026-07-04 19:20:46 +08:00
parent 54d6465fa7
commit f7a720204a
3360 changed files with 802660 additions and 3 deletions
@@ -0,0 +1,422 @@
+//! Integration tests for AcpAgentManager.
+//!
+//! **Status: TEMPORARILY IGNORED** — These tests use mock shell scripts that
+//! produce line-delimited JSON on stdout. After the ACP SDK integration
+//! (replacing raw JSON-over-stdio with `agent-client-protocol` JSON-RPC),
+//! `AcpAgentManager::new()` now performs an SDK `initialize` handshake that
+//! mock shell scripts cannot respond to.
+//!
+//! To re-enable these tests, the mock scripts need to be replaced with a
+//! minimal JSON-RPC responder that handles `initialize`, `session/new`,
+//! `session/prompt`, and `session/update` notifications.
+//!
+//! Tests are serialized via `SERIAL_LOCK` to avoid OS-level resource
+//! contention from parallel subprocess spawning (pipes, I/O scheduling).
+
+// Pre-existing: serial() MutexGuard held across await points is intentional —
+// it serializes test execution. Useless .into() is a pre-existing nit.
+#![allow(clippy::await_holding_lock, clippy::useless_conversion)]
+
+use std::sync::{Arc, Mutex, MutexGuard};
+use std::time::Duration;
+
+use nomifun_ai_agent::factory::acp_assembler::{WorkspaceInfo, assemble_acp_params};
+use nomifun_ai_agent::manager::acp::AcpAgentManager;
+use nomifun_ai_agent::registry::AgentRegistry;
+use nomifun_ai_agent::{AgentInstance, AgentStreamEvent, IAgentTask};
+use nomifun_common::ConversationStatus;
+use nomifun_db::{SqliteAgentMetadataRepository, init_database_memory};
+use tokio::sync::broadcast;
+
+/// Timeout for receiving events from the relay.
+const EVENT_TIMEOUT: Duration = Duration::from_secs(5);
+
+/// Serialize integration tests to avoid OS-level resource contention
+/// from parallel subprocess spawning (pipes, I/O scheduling).
+static SERIAL_LOCK: Mutex<()> = Mutex::new(());
+
+/// Acquire the serial lock (panics on poison).
+fn serial() -> MutexGuard<'static, ()> {
+    SERIAL_LOCK.lock().unwrap_or_else(|e| e.into_inner())
+}
+
+/// Create an AcpAgentManager wrapping a mock shell script.
+///
+/// Returns the Arc-wrapped manager and a pre-subscribed event receiver
+/// (subscribed BEFORE the relay starts, so no events are missed).
+async fn make_mock_agent(script: &str, backend: &str) -> (Arc<AcpAgentManager>, broadcast::Receiver<AgentStreamEvent>) {
+    let temp_dir = std::env::temp_dir();
+    let script_path = temp_dir.join(format!(
+        "mock_acp_{}_{}.sh",
+        std::process::id(),
+        nomifun_common::now_ms()
+    ));
+    std::fs::write(&script_path, format!("#!/bin/sh\n{script}")).unwrap();
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        std::fs::set_permissions(&script_path, std::fs::Permissions::from_mode(0o755)).unwrap();
+    }
+
+    let config = nomifun_ai_agent::AcpBuildExtra {
+        desktop_gateway: false,
+        gateway_mcp_config: None,
+        open_mcp_config: None,
+        computer_mcp_config: None,
+        browser_mcp_config: None,
+        agent_id: None,
+        backend: Some(backend.to_owned()),
+        cli_path: Some(script_path.to_string_lossy().into_owned()),
+        agent_name: None,
+        custom_agent_id: None,
+        preset_context: None,
+        skills: vec![],
+        preset_assistant_id: None,
+        session_mode: None,
+        current_model_id: None,
+        cron_job_id: None,
+        team_mcp_stdio_config: None,
+        guide_mcp_config: None,
+        requirement_mcp_config: None,
+        knowledge_mcp_config: None,
+        mcp_server_ids: None,
+        session_mcp_servers: vec![],
+        user_id: None,
+        companion_id: None,
+        channel_platform: None,
+        knowledge_mounts: vec![],
+        knowledge_writeback: false,
+        knowledge_writeback_mode: None,
+        knowledge_writeback_eagerness: None,
+    };
+
+    let tmp_skills = tempfile::TempDir::new().unwrap();
+    let skill_paths = std::sync::Arc::new(nomifun_extension::resolve_skill_paths(
+        tmp_skills.path(),
+        tmp_skills.path(),
+    ));
+    let skill_manager = nomifun_ai_agent::AcpSkillManager::new(skill_paths);
+
+    let db = init_database_memory().await.unwrap();
+    let repo = Arc::new(SqliteAgentMetadataRepository::new(db.pool().clone()));
+    let registry = AgentRegistry::new(repo);
+    registry.hydrate().await.unwrap();
+
+    let metadata = registry
+        .find_builtin_by_backend(backend)
+        .await
+        .expect("seeded backend row must exist");
+    let catalog_tx = registry.catalog_sender();
+
+    let params = Arc::new(
+        assemble_acp_params(
+            "test-conv-1".into(),
+            WorkspaceInfo {
+                path: "/tmp".into(),
+                is_custom: true,
+            },
+            metadata,
+            nomifun_common::CommandSpec {
+                command: script_path.into(),
+                args: vec![],
+                env: vec![],
+                cwd: None,
+            },
+            config,
+            Vec::new(),
+            None,
+            std::env::temp_dir(),
+        )
+        .await,
+    );
+
+    let (manager, _, _) = AcpAgentManager::build(params, skill_manager, &catalog_tx)
+        .await
+        .expect("Failed to spawn mock ACP agent");
+
+    let arc = Arc::new(manager);
+
+    // Subscribe to typed events BEFORE starting handler to capture all events
+    let rx = arc.subscribe();
+    arc.start_permission_handler();
+
+    (arc, rx)
+}
+
+/// Wait until a specific event type is received, returning all collected events.
+async fn wait_for_event(
+    rx: &mut broadcast::Receiver<AgentStreamEvent>,
+    predicate: impl Fn(&AgentStreamEvent) -> bool,
+) -> Vec<AgentStreamEvent> {
+    let mut events = Vec::new();
+    loop {
+        match tokio::time::timeout(EVENT_TIMEOUT, rx.recv()).await {
+            Ok(Ok(event)) => {
+                let matched = predicate(&event);
+                events.push(event);
+                if matched {
+                    return events;
+                }
+            }
+            Ok(Err(broadcast::error::RecvError::Closed)) => {
+                panic!(
+                    "Event channel closed before target event. Received: {:?}",
+                    events.iter().map(event_type_name).collect::<Vec<_>>()
+                )
+            }
+            Ok(Err(broadcast::error::RecvError::Lagged(n))) => {
+                eprintln!("Warning: receiver lagged by {n} events");
+                continue;
+            }
+            Err(_) => panic!(
+                "Timed out waiting for target event. Received: {:?}",
+                events.iter().map(event_type_name).collect::<Vec<_>>()
+            ),
+        }
+    }
+}
+
+/// Get a short name for the event type (for debug output).
+fn event_type_name(event: &AgentStreamEvent) -> &'static str {
+    match event {
+        AgentStreamEvent::Start(_) => "Start",
+        AgentStreamEvent::Text(_) => "Text",
+        AgentStreamEvent::Tips(_) => "Tips",
+        AgentStreamEvent::ToolCall(_) => "ToolCall",
+        AgentStreamEvent::ToolGroup(_) => "ToolGroup",
+        AgentStreamEvent::AgentStatus(_) => "AgentStatus",
+        AgentStreamEvent::Thinking(_) => "Thinking",
+        AgentStreamEvent::Plan(_) => "Plan",
+        AgentStreamEvent::Permission(_) => "Permission",
+        AgentStreamEvent::AcpPermission(_) => "AcpPermission",
+        AgentStreamEvent::AcpToolCall(_) => "AcpToolCall",
+        AgentStreamEvent::AvailableCommands(_) => "AvailableCommands",
+        AgentStreamEvent::SkillSuggest(_) => "SkillSuggest",
+        AgentStreamEvent::CronTrigger(_) => "CronTrigger",
+        AgentStreamEvent::AcpModelInfo(_) => "AcpModelInfo",
+        AgentStreamEvent::AcpModeInfo(_) => "AcpModeInfo",
+        AgentStreamEvent::AcpConfigOption(_) => "AcpConfigOption",
+        AgentStreamEvent::AcpSessionInfo(_) => "AcpSessionInfo",
+        AgentStreamEvent::AcpContextUsage(_) => "AcpContextUsage",
+        AgentStreamEvent::AcpPromptHookWarning(_) => "AcpPromptHookWarning",
+        AgentStreamEvent::TurnCompleted(_) => "TurnCompleted",
+        AgentStreamEvent::Finish(_) => "Finish",
+        AgentStreamEvent::Error(_) => "Error",
+        AgentStreamEvent::System(_) => "System",
+        AgentStreamEvent::RequestTrace(_) => "RequestTrace",
+        AgentStreamEvent::SlashCommandsUpdated(_) => "SlashCommandsUpdated",
+        AgentStreamEvent::SessionAssigned(_) => "SessionAssigned",
+    }
+}
+
+#[test]
+fn acp_build_extra_populates_skills_from_extra_json() {
+    let json = serde_json::json!({
+        "backend": "claude",
+        "skills": ["cron", "pdf"],
+    });
+    let extra: nomifun_ai_agent::AcpBuildExtra = serde_json::from_value(json).unwrap();
+    assert_eq!(extra.skills, vec!["cron".to_owned(), "pdf".to_owned()]);
+}
+
+// -- Tests --
+// All tests below are #[ignore] because make_mock_agent() spawns shell scripts
+// that cannot respond to the SDK's JSON-RPC `initialize` handshake.
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_type_is_acp() {
+    let _guard = serial();
+    let (agent, _rx) = make_mock_agent(r#"echo '{"type":"finish","data":{}}'"#, "claude").await;
+
+    assert_eq!(agent.agent_type(), nomifun_common::AgentType::Acp);
+    assert_eq!(agent.conversation_id(), "test-conv-1");
+    assert_eq!(agent.workspace(), "/tmp");
+    assert_eq!(agent.backend(), Some("claude"));
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_receives_stream_events() {
+    let _guard = serial();
+    let (_agent, mut rx) = make_mock_agent(
+        r#"echo '{"type":"start","data":{"session_id":"sess-1"}}' && echo '{"type":"text","data":{"content":"Hello"}}' && echo '{"type":"finish","data":{"session_id":"sess-1"}}'"#,
+        "claude",
+    )
+    .await;
+
+    // Wait for finish event, collecting all events along the way
+    let events = wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::Finish(_))).await;
+
+    assert!(events.len() >= 2, "Expected at least 2 events, got {}", events.len());
+
+    let has_start = events.iter().any(|e| matches!(e, AgentStreamEvent::Start(_)));
+    let has_text = events.iter().any(|e| matches!(e, AgentStreamEvent::Text(_)));
+
+    assert!(has_start, "Should have received Start event");
+    assert!(has_text, "Should have received Text event");
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_session_id_captured_from_start() {
+    let _guard = serial();
+    let (agent, mut rx) = make_mock_agent(
+        r#"echo '{"type":"start","data":{"session_id":"sess-abc"}}' && sleep 1"#,
+        "claude",
+    )
+    .await;
+
+    wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::Start(_))).await;
+
+    let session_id = agent.session_id().await;
+    assert_eq!(session_id, Some("sess-abc".into()));
+
+    agent.kill(None).unwrap();
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_status_transitions() {
+    let _guard = serial();
+    let (agent, mut rx) = make_mock_agent(
+        r#"sleep 0.1 && echo '{"type":"start","data":{}}' && sleep 0.3 && echo '{"type":"finish","data":{}}'"#,
+        "claude",
+    )
+    .await;
+
+    // Initial status: None
+    assert_eq!(agent.status(), None);
+
+    // Wait for Start event
+    wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::Start(_))).await;
+    assert_eq!(agent.status(), Some(ConversationStatus::Running));
+
+    // Wait for Finish event
+    wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::Finish(_))).await;
+    assert_eq!(agent.status(), Some(ConversationStatus::Finished));
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_error_event_sets_finished() {
+    let _guard = serial();
+    let (agent, mut rx) = make_mock_agent(
+        r#"echo '{"type":"start","data":{}}' && sleep 0.1 && echo '{"type":"error","data":{"message":"timeout"}}'"#,
+        "claude",
+    )
+    .await;
+
+    wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::Error(_))).await;
+    assert_eq!(agent.status(), Some(ConversationStatus::Finished));
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_model_info_captured() {
+    let _guard = serial();
+    let (agent, mut rx) = make_mock_agent(
+        r#"echo '{"type":"acp_model_info","data":{"current_model_id":"claude-sonnet-4","current_model_label":"Claude Sonnet 4","available_models":[{"id":"claude-sonnet-4","label":"Claude Sonnet 4"},{"id":"claude-opus-4","label":"Claude Opus 4"}],"can_switch":true,"source":"models","source_detail":"acp-models"}}' && sleep 0.5"#,
+        "claude",
+    )
+    .await;
+
+    wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::AcpModelInfo(_))).await;
+
+    // Route through the public `AgentInstance` API rather than reaching
+    // into the private `AcpAgentManager::model()`: the ai-agent crate only
+    // exposes `AgentInstance` to downstream callers, so tests should
+    // exercise the same surface.
+    let instance = AgentInstance::Acp(agent.clone());
+    let resp = instance.get_model().await.expect("get_model should succeed");
+    let info = resp.model_info.expect("Model info should be captured");
+    assert_eq!(info.current_model_id.as_deref(), Some("claude-sonnet-4"));
+    assert_eq!(info.available_models.len(), 2);
+    assert_eq!(info.available_models[0].label, "Claude Sonnet 4");
+
+    agent.kill(None).unwrap();
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_kill_terminates_process() {
+    let _guard = serial();
+    let (agent, _rx) = make_mock_agent(r#"trap '' TERM; while true; do sleep 1; done"#, "claude").await;
+
+    assert!(agent.last_activity_at() > 0);
+
+    agent.kill(Some(nomifun_common::AgentKillReason::IdleTimeout)).unwrap();
+
+    tokio::time::sleep(Duration::from_millis(1000)).await;
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_last_activity_updates() {
+    let _guard = serial();
+    let (agent, _rx) = make_mock_agent(r#"sleep 10"#, "claude").await;
+
+    let initial = agent.last_activity_at();
+    assert!(initial > 0);
+
+    let now = nomifun_common::now_ms();
+    assert!(now - initial < 5000, "Last activity should be recent");
+
+    agent.kill(None).unwrap();
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_text_content_received() {
+    let _guard = serial();
+    let (_agent, mut rx) = make_mock_agent(
+        r#"echo '{"type":"text","data":{"content":"Hello from ACP"}}'"#,
+        "claude",
+    )
+    .await;
+
+    match tokio::time::timeout(EVENT_TIMEOUT, rx.recv()).await {
+        Ok(Ok(AgentStreamEvent::Text(data))) => {
+            assert_eq!(data.content, "Hello from ACP");
+        }
+        other => panic!("Expected Text event, got {:?}", other),
+    }
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_agent_status_event_captures_session() {
+    let _guard = serial();
+    let (agent, mut rx) = make_mock_agent(
+        r#"echo '{"type":"agent_status","data":{"backend":"claude","status":"running","session_id":"sess-xyz"}}' && sleep 1"#,
+        "claude",
+    )
+    .await;
+
+    wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::AgentStatus(_))).await;
+
+    let session = agent.session_id().await;
+    assert_eq!(session, Some("sess-xyz".into()));
+
+    agent.kill(None).unwrap();
+}
+
+#[tokio::test]
+#[ignore = "requires JSON-RPC mock agent"]
+async fn acp_agent_multiple_event_types() {
+    let _guard = serial();
+    let (_agent, mut rx) = make_mock_agent(
+        r#"echo '{"type":"start","data":{"session_id":"sess-multi"}}' && echo '{"type":"thinking","data":{"content":"Analyzing...","subject":"code","duration":100,"status":"in_progress"}}' && echo '{"type":"text","data":{"content":"Result"}}' && echo '{"type":"finish","data":{"session_id":"sess-multi"}}'"#,
+        "claude",
+    )
+    .await;
+
+    let events = wait_for_event(&mut rx, |e| matches!(e, AgentStreamEvent::Finish(_))).await;
+
+    assert!(events.len() >= 4, "Expected 4+ events, got {}", events.len());
+
+    assert!(matches!(&events[0], AgentStreamEvent::Start(d) if d.session_id == Some("sess-multi".into())));
+    assert!(matches!(&events[1], AgentStreamEvent::Thinking(d) if d.content == "Analyzing..."));
+    assert!(matches!(&events[2], AgentStreamEvent::Text(d) if d.content == "Result"));
+    assert!(matches!(&events[3], AgentStreamEvent::Finish(d) if d.session_id == Some("sess-multi".into())));
+}
@@ -0,0 +1,28 @@
+//! Compile-only smoke test for `manager::acp` public surface.
+//!
+//! During the Stage 1 refactor (splitting `manager/acp/agent.rs` into smaller
+//! submodules), this file pins the set of type names that must remain
+//! reachable through `nomifun_ai_agent::manager::acp`. It proves nothing about
+//! behaviour — only that the rename/move did not accidentally drop a public
+//! export. Behavioural correctness is guarded by the byte-level diff of the
+//! moved function bodies and by the stage's new targeted tests.
+#![allow(dead_code, unused_imports)]
+
+use nomifun_ai_agent::manager::acp::{
+    AcpSession, AcpSessionEvent, CatalogForwarder, PermissionRouter, ReconcileAction,
+};
+use nomifun_ai_agent::shared_kernel::PersistedSessionState;
+
+fn _surface_probe() {
+    let _ = std::any::type_name::<AcpSession>();
+    let _ = std::any::type_name::<AcpSessionEvent>();
+    let _ = std::any::type_name::<CatalogForwarder>();
+    let _ = std::any::type_name::<PermissionRouter>();
+    let _ = std::any::type_name::<PersistedSessionState>();
+    let _ = std::any::type_name::<ReconcileAction>();
+}
+
+#[test]
+fn public_surface_compiles() {
+    // The real assertion is that this file compiled at all.
+}
@@ -0,0 +1,303 @@
+//! Integration tests for agent type implementations and auxiliary features.
+//!
+//! These tests validate:
+//! - Each agent manager implements IAgentTask correctly
+//! - Agent factory can build all agent types
+//! - Idle scanner finds eligible tasks
+//! - Workspace browsing works with real filesystem
+//! - Nomi stub returns appropriate errors
+
+use std::sync::Arc;
+
+use nomifun_ai_agent::manager::nomi::NomiAgentManager;
+use nomifun_ai_agent::task_manager::AgentFactory;
+use nomifun_ai_agent::types::{BuildTaskOptions, NomiResolvedConfig, SendMessageData};
+use nomifun_ai_agent::*;
+use nomifun_ai_agent::{SkillIndex, build_system_instructions_with_skills_index};
+use nomifun_common::{AgentKillReason, AgentType, ConversationStatus, ProviderWithModel, TimestampMs, now_ms};
+use serde_json::json;
+use std::sync::atomic::{AtomicI64, Ordering};
+use tokio::sync::broadcast;
+
+// ---------------------------------------------------------------------------
+// Mock agent for WorkerTaskManager tests with different agent types
+// ---------------------------------------------------------------------------
+
+struct TypedMockAgent {
+    agent_type: AgentType,
+    conversation_id: String,
+    workspace: String,
+    status: Option<ConversationStatus>,
+    last_activity: AtomicI64,
+    event_tx: broadcast::Sender<AgentStreamEvent>,
+}
+
+impl TypedMockAgent {
+    fn new(agent_type: AgentType, conversation_id: &str, status: Option<ConversationStatus>) -> Self {
+        let (event_tx, _) = broadcast::channel(16);
+        Self {
+            agent_type,
+            conversation_id: conversation_id.to_owned(),
+            workspace: "/tmp/test".to_owned(),
+            status,
+            last_activity: AtomicI64::new(now_ms()),
+            event_tx,
+        }
+    }
+
+    fn with_last_activity(mut self, ts: TimestampMs) -> Self {
+        self.last_activity = AtomicI64::new(ts);
+        self
+    }
+}
+
+#[async_trait::async_trait]
+impl IAgentTask for TypedMockAgent {
+    fn agent_type(&self) -> AgentType {
+        self.agent_type
+    }
+    fn conversation_id(&self) -> &str {
+        &self.conversation_id
+    }
+    fn workspace(&self) -> &str {
+        &self.workspace
+    }
+    fn status(&self) -> Option<ConversationStatus> {
+        self.status
+    }
+    fn last_activity_at(&self) -> TimestampMs {
+        self.last_activity.load(Ordering::Relaxed)
+    }
+    fn subscribe(&self) -> broadcast::Receiver<AgentStreamEvent> {
+        self.event_tx.subscribe()
+    }
+    async fn send_message(&self, _data: SendMessageData) -> Result<(), nomifun_ai_agent::AgentSendError> {
+        Ok(())
+    }
+    async fn cancel(&self) -> Result<(), nomifun_common::AppError> {
+        Ok(())
+    }
+    fn kill(&self, _reason: Option<AgentKillReason>) -> Result<(), nomifun_common::AppError> {
+        Ok(())
+    }
+}
+
+impl IMockAgent for TypedMockAgent {}
+
+// ---------------------------------------------------------------------------
+// Nomi agent tests (real implementation with AgentEngine)
+// ---------------------------------------------------------------------------
+
+fn make_nomi_config() -> NomiResolvedConfig {
+    NomiResolvedConfig {
+        provider: "anthropic".into(),
+        api_key: "sk-test-key".into(),
+        model: "claude-sonnet-4-20250514".into(),
+        base_url: None,
+        system_prompt: None,
+        max_tokens: 4096,
+        max_turns: None,
+        context_limit: None,
+        compat_overrides: Default::default(),
+        session_directory: std::env::temp_dir().join("nomi-test-sessions"),
+        session_mode: None,
+        extra_mcp_servers: Default::default(),
+        bedrock_config: None,
+        computer_use: false,
+        browser_use: false,
+        browser_full_power: false,
+        browser_persistent_login: false,
+        browser_site_memory: false,
+        browser_takeover: false,
+        browser_visual_fallback: false,
+        goal: None,
+        browser_secret_vault: None,
+    }
+}
+
+#[tokio::test]
+async fn nomi_agent_kill_succeeds() {
+    let agent = NomiAgentManager::new("conv-1".into(), "/proj".into(), make_nomi_config(), None, None, None, None, Vec::new(), None, None, Vec::new(), false, None)
+        .await
+        .unwrap();
+    assert!(agent.kill(None).is_ok());
+    assert!(agent.kill(Some(AgentKillReason::IdleTimeout)).is_ok());
+}
+
+#[tokio::test]
+async fn nomi_agent_confirm_succeeds() {
+    let agent = NomiAgentManager::new("conv-1".into(), "/proj".into(), make_nomi_config(), None, None, None, None, Vec::new(), None, None, Vec::new(), false, None)
+        .await
+        .unwrap();
+    // `confirm` is an inherent method on `NomiAgentManager` (reached via
+    // `AgentInstance::Nomi(..)` in production); the test calls it
+    // directly on the concrete manager.
+    let result = agent.confirm("msg", "call", json!({}), false);
+    assert!(result.is_ok());
+}
+
+#[tokio::test]
+async fn nomi_agent_metadata() {
+    let agent = NomiAgentManager::new("conv-abc".into(), "/work".into(), make_nomi_config(), None, None, None, None, Vec::new(), None, None, Vec::new(), false, None)
+        .await
+        .unwrap();
+    assert_eq!(agent.agent_type(), AgentType::Nomi);
+    assert_eq!(agent.workspace(), "/work");
+    assert_eq!(agent.conversation_id(), "conv-abc");
+    assert_eq!(agent.status(), Some(ConversationStatus::Pending));
+    assert!(agent.get_confirmations().is_empty());
+    assert!(!agent.check_approval("any", None));
+}
+
+// ---------------------------------------------------------------------------
+// Idle scanner: collect_idle only finds ACP tasks
+// ---------------------------------------------------------------------------
+
+#[tokio::test]
+async fn collect_idle_ignores_non_acp_agent_types() {
+    use futures_util::FutureExt;
+    let old_ts = now_ms() - 600_000; // 10 min ago
+
+    // Build a factory that creates typed mocks (all finished + old)
+    let factory: AgentFactory = Arc::new(move |opts: BuildTaskOptions| {
+        async move {
+            let mock = TypedMockAgent::new(
+                opts.agent_type,
+                &opts.conversation_id,
+                Some(ConversationStatus::Finished),
+            )
+            .with_last_activity(old_ts);
+            Ok(AgentInstance::Mock(Arc::new(mock)))
+        }
+        .boxed()
+    });
+    let mgr = WorkerTaskManagerImpl::new(factory);
+
+    let make_opts = |agent_type: AgentType, id: &str| BuildTaskOptions {
+        agent_type,
+        workspace: "/tmp".into(),
+        model: ProviderWithModel {
+            provider_id: "p".into(),
+            model: "m".into(),
+            use_model: None,
+        },
+        conversation_id: id.into(),
+        extra: json!(null),
+    };
+
+    mgr.get_or_build_task("nanobot-1", make_opts(AgentType::Nanobot, "nanobot-1"))
+        .await
+        .unwrap();
+    mgr.get_or_build_task("openclaw-1", make_opts(AgentType::OpenclawGateway, "openclaw-1"))
+        .await
+        .unwrap();
+    mgr.get_or_build_task("acp-1", make_opts(AgentType::Acp, "acp-1"))
+        .await
+        .unwrap();
+    mgr.get_or_build_task("remote-1", make_opts(AgentType::Remote, "remote-1"))
+        .await
+        .unwrap();
+
+    assert_eq!(mgr.active_count(), 4);
+
+    // Only ACP should be collected
+    let idle = mgr.collect_idle(300_000); // 5-min threshold
+    assert_eq!(idle.len(), 1);
+    assert_eq!(idle[0], "acp-1");
+}
+
+// ---------------------------------------------------------------------------
+// Workspace browsing (uses real filesystem via tempdir)
+// ---------------------------------------------------------------------------
+
+#[tokio::test]
+async fn workspace_browse_reads_directory() {
+    let tmp = tempfile::TempDir::new().unwrap();
+    let base = tmp.path();
+
+    // Create test files and dirs
+    std::fs::create_dir(base.join("src")).unwrap();
+    std::fs::create_dir(base.join("tests")).unwrap();
+    std::fs::write(base.join("Cargo.toml"), "# test").unwrap();
+    std::fs::write(base.join("README.md"), "# readme").unwrap();
+
+    let mut entries = Vec::new();
+    let mut dir_reader = tokio::fs::read_dir(base).await.unwrap();
+    while let Ok(Some(entry)) = dir_reader.next_entry().await {
+        let name = entry.file_name().to_string_lossy().into_owned();
+        let ft = entry.file_type().await.unwrap();
+        let entry_type = if ft.is_dir() { "directory" } else { "file" };
+        entries.push((name, entry_type.to_string()));
+    }
+
+    assert_eq!(entries.len(), 4);
+
+    // Check that directories exist
+    let dir_names: Vec<&str> = entries
+        .iter()
+        .filter(|(_, t)| t == "directory")
+        .map(|(n, _)| n.as_str())
+        .collect();
+    assert!(dir_names.contains(&"src"));
+    assert!(dir_names.contains(&"tests"));
+
+    // Check that files exist
+    let file_names: Vec<&str> = entries
+        .iter()
+        .filter(|(_, t)| t == "file")
+        .map(|(n, _)| n.as_str())
+        .collect();
+    assert!(file_names.contains(&"Cargo.toml"));
+    assert!(file_names.contains(&"README.md"));
+}
+
+// ---------------------------------------------------------------------------
+// build_system_instructions_with_skills_index (M-16 fix)
+// ---------------------------------------------------------------------------
+
+#[test]
+fn build_system_instructions_with_skills_index_empty() {
+    let result = build_system_instructions_with_skills_index("Base prompt", &[]);
+    assert_eq!(result, "Base prompt");
+}
+
+#[test]
+fn build_system_instructions_with_skills_index_appends_index() {
+    let skills = vec![
+        SkillIndex {
+            name: "review".into(),
+            description: "Code review".into(),
+        },
+        SkillIndex {
+            name: "debug".into(),
+            description: "Debugging".into(),
+        },
+    ];
+    let result = build_system_instructions_with_skills_index("You are an AI assistant.", &skills);
+    assert!(result.starts_with("You are an AI assistant."));
+    assert!(result.contains("## Available Skills"));
+    assert!(result.contains("- **review**: Code review"));
+    assert!(result.contains("- **debug**: Debugging"));
+    assert!(result.contains("[LOAD_SKILL: skill-name]"));
+}
+
+// ---------------------------------------------------------------------------
+// Agent type metadata validation
+// ---------------------------------------------------------------------------
+
+#[test]
+fn agent_type_serde_all_variants() {
+    // Verify that all AgentType variants serialize/deserialize correctly
+    for (variant, expected_json) in [
+        (AgentType::Acp, "\"acp\""),
+        (AgentType::OpenclawGateway, "\"openclaw-gateway\""),
+        (AgentType::Nanobot, "\"nanobot\""),
+        (AgentType::Remote, "\"remote\""),
+        (AgentType::Nomi, "\"nomi\""),
+    ] {
+        let json = serde_json::to_string(&variant).unwrap();
+        assert_eq!(json, expected_json, "Failed for {:?}", variant);
+        let parsed: AgentType = serde_json::from_str(&json).unwrap();
+        assert_eq!(parsed, variant);
+    }
+}
@@ -0,0 +1,157 @@
+use nomifun_ai_agent::cc_switch::{
+    CcSwitchPaths, build_model_info_from_env, read_claude_model_info_with_paths, read_claude_provider_env_with_paths,
+};
+use rusqlite::Connection;
+use std::collections::HashMap;
+use std::fs;
+use tempfile::TempDir;
+
+fn create_test_db(dir: &std::path::Path, provider_id: &str, settings_config: &str) {
+    let db_path = dir.join("cc-switch.db");
+    let conn = Connection::open(&db_path).unwrap();
+    conn.execute_batch(
+        "CREATE TABLE IF NOT EXISTS providers (
+            id TEXT NOT NULL,
+            app_type TEXT NOT NULL,
+            name TEXT NOT NULL,
+            settings_config TEXT NOT NULL,
+            PRIMARY KEY (id, app_type)
+        );
+        CREATE TABLE IF NOT EXISTS model_pricing (
+            model_id TEXT PRIMARY KEY,
+            display_name TEXT NOT NULL
+        );",
+    )
+    .unwrap();
+    conn.execute(
+        "INSERT INTO providers (id, app_type, name, settings_config) VALUES (?1, 'claude', 'Test Provider', ?2)",
+        [provider_id, settings_config],
+    )
+    .unwrap();
+    conn.execute(
+        "INSERT INTO model_pricing (model_id, display_name) VALUES (?1, ?2)",
+        ["deepseek-v4-pro", "DeepSeek V4 Pro"],
+    )
+    .unwrap();
+    conn.execute(
+        "INSERT INTO model_pricing (model_id, display_name) VALUES (?1, ?2)",
+        ["deepseek-v4-max", "DeepSeek V4 Max"],
+    )
+    .unwrap();
+}
+
+#[test]
+fn reads_provider_env_from_fixture_db() {
+    let tmp = TempDir::new().unwrap();
+    let cc_switch_dir = tmp.path().join(".cc-switch");
+    fs::create_dir_all(&cc_switch_dir).unwrap();
+
+    let settings = r#"{"currentProviderClaude": "deepseek-relay"}"#;
+    fs::write(cc_switch_dir.join("settings.json"), settings).unwrap();
+
+    let config = serde_json::json!({
+        "env": {
+            "ANTHROPIC_BASE_URL": "https://relay.example.com/v1",
+            "ANTHROPIC_API_KEY": "sk-relay-test-key",
+            "ANTHROPIC_DEFAULT_SONNET_MODEL": "deepseek-v4-pro",
+            "ANTHROPIC_DEFAULT_OPUS_MODEL": "deepseek-v4-max"
+        },
+        "model": "default"
+    });
+    create_test_db(&cc_switch_dir, "deepseek-relay", &config.to_string());
+
+    let paths = CcSwitchPaths::from_home(tmp.path());
+    let env = read_claude_provider_env_with_paths(&paths);
+
+    assert_eq!(env.get("ANTHROPIC_BASE_URL").unwrap(), "https://relay.example.com/v1");
+    assert_eq!(env.get("ANTHROPIC_API_KEY").unwrap(), "sk-relay-test-key");
+    assert_eq!(env.get("ANTHROPIC_DEFAULT_SONNET_MODEL").unwrap(), "deepseek-v4-pro");
+    assert_eq!(env.get("ANTHROPIC_DEFAULT_OPUS_MODEL").unwrap(), "deepseek-v4-max");
+}
+
+#[test]
+fn reads_model_info_from_fixture_db() {
+    let tmp = TempDir::new().unwrap();
+    let cc_switch_dir = tmp.path().join(".cc-switch");
+    fs::create_dir_all(&cc_switch_dir).unwrap();
+
+    let settings = r#"{"currentProviderClaude": "deepseek-relay"}"#;
+    fs::write(cc_switch_dir.join("settings.json"), settings).unwrap();
+
+    let config = serde_json::json!({
+        "env": {
+            "ANTHROPIC_DEFAULT_SONNET_MODEL": "deepseek-v4-pro",
+            "ANTHROPIC_DEFAULT_OPUS_MODEL": "deepseek-v4-max"
+        },
+        "model": "default"
+    });
+    create_test_db(&cc_switch_dir, "deepseek-relay", &config.to_string());
+
+    let paths = CcSwitchPaths::from_home(tmp.path());
+    let info = read_claude_model_info_with_paths(&paths);
+
+    assert!(info.is_some());
+    let payload = info.unwrap();
+    assert_eq!(payload.available_models.len(), 2);
+    assert_eq!(payload.current_model_id.as_deref(), Some("default"));
+    assert_eq!(payload.current_model_label.as_deref(), Some("DeepSeek V4 Pro"));
+    assert_eq!(payload.available_models[0].label, "DeepSeek V4 Pro");
+    assert_eq!(payload.available_models[1].label, "DeepSeek V4 Max");
+}
+
+#[test]
+fn gracefully_handles_missing_cc_switch() {
+    let tmp = TempDir::new().unwrap();
+    let paths = CcSwitchPaths::from_home(tmp.path());
+
+    let env = read_claude_provider_env_with_paths(&paths);
+    assert!(env.is_empty());
+
+    let info = read_claude_model_info_with_paths(&paths);
+    assert!(info.is_none());
+}
+
+#[test]
+fn gracefully_handles_empty_provider_id() {
+    let tmp = TempDir::new().unwrap();
+    let cc_switch_dir = tmp.path().join(".cc-switch");
+    fs::create_dir_all(&cc_switch_dir).unwrap();
+
+    fs::write(cc_switch_dir.join("settings.json"), r#"{"currentProviderClaude": ""}"#).unwrap();
+
+    let paths = CcSwitchPaths::from_home(tmp.path());
+    let env = read_claude_provider_env_with_paths(&paths);
+    assert!(env.is_empty());
+}
+
+#[test]
+fn default_provider_returns_empty_env_when_no_env_configured() {
+    let tmp = TempDir::new().unwrap();
+    let cc_switch_dir = tmp.path().join(".cc-switch");
+    fs::create_dir_all(&cc_switch_dir).unwrap();
+
+    let settings = r#"{"currentProviderClaude": "default"}"#;
+    fs::write(cc_switch_dir.join("settings.json"), settings).unwrap();
+
+    let config = serde_json::json!({
+        "env": {}
+    });
+    create_test_db(&cc_switch_dir, "default", &config.to_string());
+
+    let paths = CcSwitchPaths::from_home(tmp.path());
+    let env = read_claude_provider_env_with_paths(&paths);
+    assert!(env.is_empty());
+}
+
+#[test]
+fn build_model_info_from_env_works_standalone() {
+    let mut env = HashMap::new();
+    env.insert("ANTHROPIC_DEFAULT_SONNET_MODEL".into(), "test-model".into());
+
+    let labels = HashMap::from([("test-model".to_owned(), "Test Model Display".to_owned())]);
+
+    let info = build_model_info_from_env(&env, &labels, None);
+    assert!(info.is_some());
+    let payload = info.unwrap();
+    assert_eq!(payload.available_models[0].label, "Test Model Display");
+}
@@ -0,0 +1,170 @@
+use std::path::PathBuf;
+use std::sync::Arc;
+
+use nomifun_ai_agent::AcpSessionSyncService;
+use nomifun_ai_agent::AcpSkillManager;
+use nomifun_ai_agent::factory::{AgentFactoryDeps, build_agent_factory};
+use nomifun_ai_agent::registry::AgentRegistry;
+use nomifun_ai_agent::types::BuildTaskOptions;
+use nomifun_common::{AgentType, ProviderWithModel, encrypt_string};
+use nomifun_db::{
+    CreateProviderParams, IAcpSessionRepository, IProviderRepository, SqliteAcpSessionRepository,
+    SqliteAgentMetadataRepository, SqliteProviderRepository, SqliteRemoteAgentRepository, init_database_memory,
+};
+
+fn test_encryption_key() -> [u8; 32] {
+    [0xABu8; 32]
+}
+
+async fn setup() -> (
+    Arc<dyn IProviderRepository>,
+    Arc<SqliteRemoteAgentRepository>,
+    Arc<AgentRegistry>,
+    Arc<AcpSessionSyncService>,
+) {
+    let db = init_database_memory().await.unwrap();
+    let pool = db.pool().clone();
+    let provider_repo: Arc<dyn IProviderRepository> = Arc::new(SqliteProviderRepository::new(pool.clone()));
+    let remote_agent_repo = Arc::new(SqliteRemoteAgentRepository::new(pool.clone()));
+    let metadata_repo = Arc::new(SqliteAgentMetadataRepository::new(pool.clone()));
+    let registry = AgentRegistry::new(metadata_repo);
+    registry.hydrate().await.unwrap();
+    let session_repo: Arc<dyn IAcpSessionRepository> = Arc::new(SqliteAcpSessionRepository::new(pool));
+    let acp_agent_service = AcpSessionSyncService::new(session_repo);
+    (provider_repo, remote_agent_repo, registry, acp_agent_service)
+}
+
+async fn insert_test_provider(repo: &dyn IProviderRepository, id: &str, platform: &str) {
+    let key = test_encryption_key();
+    let encrypted_api_key = encrypt_string("sk-test-key-12345", &key).unwrap();
+    repo.create(CreateProviderParams {
+        id: Some(id),
+        platform,
+        name: "Test Provider",
+        base_url: "https://api.example.com/v1",
+        api_key_encrypted: &encrypted_api_key,
+        models: r#"["gpt-4o","gpt-5.4"]"#,
+        enabled: true,
+        capabilities: "[]",
+        context_limit: None,
+        model_protocols: None,
+        model_enabled: None,
+        model_health: None,
+        bedrock_config: None,
+        is_full_url: false,
+    })
+    .await
+    .unwrap();
+}
+
+fn make_factory(
+    provider_repo: Arc<dyn IProviderRepository>,
+    remote_agent_repo: Arc<SqliteRemoteAgentRepository>,
+    agent_registry: Arc<AgentRegistry>,
+    acp_agent_service: Arc<AcpSessionSyncService>,
+) -> nomifun_ai_agent::task_manager::AgentFactory {
+    let tmp = tempfile::TempDir::new().unwrap();
+    let skill_paths = Arc::new(nomifun_extension::resolve_skill_paths(tmp.path(), tmp.path()));
+    build_agent_factory(AgentFactoryDeps {
+        cron_sink_factory: None,
+        gateway_mcp_config: None,
+        open_mcp_config: None,
+        computer_mcp_config: None,
+        browser_mcp_config: None,
+        client_prefs: None,
+        settings_repo: None,
+        companion_prompt: None,
+        companion_skill_sink: None,
+        skill_manager: AcpSkillManager::new(skill_paths),
+        remote_agent_repo,
+        provider_repo,
+        encryption_key: test_encryption_key(),
+        agent_registry,
+        acp_agent_service,
+        data_dir: PathBuf::from("/tmp/nomi-test"),
+        work_dir: PathBuf::from("/tmp/nomi-test"),
+        backend_binary_path: Arc::new(PathBuf::from("/tmp/nomi-test/nomicore")),
+        guide_mcp_config: None,
+        requirement_mcp_config: None,
+        knowledge_mcp_config: None,
+        mcp_server_repo: None,
+        requirement_sink: None,
+        companion_sink: None,
+        knowledge_retrieval: None,
+        knowledge_writeback: None,
+    })
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn nomi_factory_returns_error_for_missing_provider() {
+    let (provider_repo, remote_agent_repo, agent_registry, acp_agent_service) = setup().await;
+    let factory = make_factory(provider_repo, remote_agent_repo, agent_registry, acp_agent_service);
+
+    let options = BuildTaskOptions {
+        agent_type: AgentType::Nomi,
+        workspace: String::new(),
+        model: ProviderWithModel {
+            provider_id: "nonexistent-provider".into(),
+            model: "gpt-4o".into(),
+            use_model: None,
+        },
+        conversation_id: "conv-test-1".into(),
+        extra: serde_json::json!({}),
+    };
+
+    let result = factory(options).await;
+    match result {
+        Ok(_) => panic!("Expected error for missing provider, got Ok"),
+        Err(e) => {
+            let err_msg = e.to_string();
+            assert!(
+                err_msg.contains("not found"),
+                "Expected 'not found' error, got: {err_msg}"
+            );
+        }
+    }
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn nomi_factory_resolves_provider_from_db() {
+    let (provider_repo, remote_agent_repo, agent_registry, acp_agent_service) = setup().await;
+    insert_test_provider(&*provider_repo, "prov-001", "openai").await;
+    let factory = make_factory(provider_repo, remote_agent_repo, agent_registry, acp_agent_service);
+
+    let options = BuildTaskOptions {
+        agent_type: AgentType::Nomi,
+        workspace: "/tmp/test-workspace".into(),
+        model: ProviderWithModel {
+            provider_id: "prov-001".into(),
+            model: "gpt-4o".into(),
+            use_model: None,
+        },
+        conversation_id: "conv-test-2".into(),
+        extra: serde_json::json!({ "max_tokens": 2048 }),
+    };
+
+    let result = factory(options).await;
+    assert!(result.is_ok(), "Expected Ok, got: {:?}", result.err());
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn nomi_factory_respects_use_model_override() {
+    let (provider_repo, remote_agent_repo, agent_registry, acp_agent_service) = setup().await;
+    insert_test_provider(&*provider_repo, "prov-002", "openai").await;
+    let factory = make_factory(provider_repo, remote_agent_repo, agent_registry, acp_agent_service);
+
+    let options = BuildTaskOptions {
+        agent_type: AgentType::Nomi,
+        workspace: "/tmp/test-workspace".into(),
+        model: ProviderWithModel {
+            provider_id: "prov-002".into(),
+            model: "gpt-4o".into(),
+            use_model: Some("gpt-5.4".into()),
+        },
+        conversation_id: "conv-test-3".into(),
+        extra: serde_json::json!({}),
+    };
+
+    let result = factory(options).await;
+    assert!(result.is_ok(), "Expected Ok, got: {:?}", result.err());
+}
@@ -0,0 +1,63 @@
+//! E2E regression: the `knowledge_search` tool finds content in a base whose
+//! directory carries a self-`.gitignore: *` — the exact scenario where ripgrep
+//! (hidden-dir skip + gitignore) returned zero. Proves the hard bug is dead.
+
+use std::sync::Arc;
+
+use nomi_agent::knowledge_tools::KnowledgeSearchTool;
+use nomi_tools::Tool;
+use serde_json::json;
+
+/// `nomifun_realtime` does not ship a public no-op broadcaster (its only one is
+/// `pub(crate)` in nomifun-knowledge's testutil), so define a local one. The
+/// `EventBroadcaster` trait is a single `broadcast` method; the emitter never
+/// fires during a read-only search, but the service still requires one.
+struct NoopBroadcaster;
+
+impl nomifun_realtime::EventBroadcaster for NoopBroadcaster {
+    fn broadcast(&self, _event: nomifun_api_types::WebSocketMessage<serde_json::Value>) {}
+}
+
+async fn build_service() -> (Arc<nomifun_knowledge::KnowledgeService>, tempfile::TempDir) {
+    let db = nomifun_db::init_database_memory().await.expect("in-memory db");
+    let repo = Arc::new(nomifun_db::SqliteKnowledgeRepository::new(db.pool().clone()));
+    let tmp = tempfile::tempdir().unwrap();
+    let emitter = nomifun_knowledge::KnowledgeEventEmitter::new(Arc::new(NoopBroadcaster));
+    let svc = Arc::new(nomifun_knowledge::KnowledgeService::new(repo, tmp.path(), emitter));
+    (svc, tmp)
+}
+
+#[tokio::test]
+async fn knowledge_search_tool_finds_topic_through_full_stack() {
+    let (svc, _tmp) = build_service().await;
+    let info = svc.create_base("运维手册", "", None, None).await.unwrap();
+    let root = svc.data_dir().join("knowledge").join(&info.id);
+    // The self-ignore the mount writes — must NOT blind the search.
+    std::fs::write(root.join(".gitignore"), "*\n").unwrap();
+    std::fs::write(root.join("rollback.md"), "# 回滚流程\n回滚分三步\n").unwrap();
+
+    let sink: Arc<dyn nomi_agent::knowledge_tools::KnowledgeRetrievalSink> =
+        Arc::new(nomifun_ai_agent::LiveKnowledgeRetrievalSink { service: svc });
+    let tool = KnowledgeSearchTool::new(sink, vec![info.id]);
+
+    let res = tool.execute(json!({"query": "回滚"})).await;
+    assert!(!res.is_error, "tool errored: {}", res.content);
+    assert!(res.content.contains("rollback.md"), "must surface the doc:\n{}", res.content);
+    assert!(res.content.contains("回滚流程"), "must include heading:\n{}", res.content);
+}
+
+#[tokio::test]
+async fn knowledge_search_tool_reports_no_match_cleanly() {
+    let (svc, _tmp) = build_service().await;
+    let info = svc.create_base("库", "", None, None).await.unwrap();
+    let root = svc.data_dir().join("knowledge").join(&info.id);
+    std::fs::write(root.join("a.md"), "# A\nunrelated content\n").unwrap();
+
+    let sink: Arc<dyn nomi_agent::knowledge_tools::KnowledgeRetrievalSink> =
+        Arc::new(nomifun_ai_agent::LiveKnowledgeRetrievalSink { service: svc });
+    let tool = KnowledgeSearchTool::new(sink, vec![info.id]);
+
+    let res = tool.execute(json!({"query": "完全不存在的主题词"})).await;
+    assert!(!res.is_error);
+    assert!(res.content.contains("No matches"), "{}", res.content);
+}
@@ -0,0 +1,121 @@
+//! E2E regression for the unified write stack (P1). Drives the REAL nomi tool
+//! → `LiveKnowledge*Sink` → `KnowledgeService::write_document` chain to prove:
+//!   1. the reported bug is dead — a staged write-back where the model passes
+//!      the workspace-mount path lands in the review inbox mirroring the
+//!      original (NOT a new nested file), with the original untouched;
+//!   2. the search → read → write loop updates the original in place by handle,
+//!      with zero path arithmetic and no duplicate file.
+
+use std::sync::Arc;
+
+use nomi_agent::knowledge_tools::{
+    KnowledgeReadTool, KnowledgeRetrievalSink, KnowledgeSearchTool, KnowledgeWritebackSink, KnowledgeWriteTool, WriteMode,
+};
+use nomi_tools::Tool;
+use serde_json::json;
+
+/// `nomifun_realtime` ships no public no-op broadcaster, so define a local one
+/// (same pattern as `knowledge_search_e2e`).
+struct NoopBroadcaster;
+
+impl nomifun_realtime::EventBroadcaster for NoopBroadcaster {
+    fn broadcast(&self, _event: nomifun_api_types::WebSocketMessage<serde_json::Value>) {}
+}
+
+async fn build_service() -> (Arc<nomifun_knowledge::KnowledgeService>, tempfile::TempDir) {
+    let db = nomifun_db::init_database_memory().await.expect("in-memory db");
+    let repo = Arc::new(nomifun_db::SqliteKnowledgeRepository::new(db.pool().clone()));
+    let tmp = tempfile::tempdir().unwrap();
+    let emitter = nomifun_knowledge::KnowledgeEventEmitter::new(Arc::new(NoopBroadcaster));
+    let svc = Arc::new(nomifun_knowledge::KnowledgeService::new(repo, tmp.path(), emitter));
+    (svc, tmp)
+}
+
+#[tokio::test]
+async fn staged_write_tool_with_mount_prefixed_path_lands_in_inbox_not_nested() {
+    let (svc, _tmp) = build_service().await;
+    let info = svc.create_base("领域库", "", None, None).await.unwrap();
+    svc.write_file(&info.id, "terms.md", "ORIGINAL").await.unwrap();
+
+    let sink: Arc<dyn KnowledgeWritebackSink> =
+        Arc::new(nomifun_ai_agent::LiveKnowledgeWritebackSink { service: svc.clone() });
+    let tool = KnowledgeWriteTool::new(
+        sink,
+        vec![(info.id.clone(), info.name.clone())],
+        WriteMode::Staged { scope: "conv-9".into() },
+        vec![info.id.clone()],
+    );
+
+    // The exact reported mistake: the model passes the workspace-mount path.
+    let res = tool
+        .execute(json!({
+            "base": "领域库",
+            "rel_path": ".nomi/knowledge/领域库/terms.md",
+            "content": "PROPOSED EDIT"
+        }))
+        .await;
+    assert!(!res.is_error, "tool errored: {}", res.content);
+
+    // Original untouched; proposal staged under the mirrored path.
+    assert_eq!(svc.read_file(&info.id, "terms.md").await.unwrap().content, "ORIGINAL");
+    assert_eq!(
+        svc.read_file(&info.id, "_inbox/conv-9/terms.md").await.unwrap().content,
+        "PROPOSED EDIT"
+    );
+    // No stray nested file under the mount path.
+    let files = svc.list_files(&info.id).await.unwrap();
+    assert!(
+        !files.iter().any(|f| f.rel_path.contains(".nomi/knowledge")),
+        "must not create a nested mount-path file: {files:?}"
+    );
+}
+
+#[tokio::test]
+async fn search_read_write_handle_loop_updates_original_in_direct_mode() {
+    let (svc, _tmp) = build_service().await;
+    let info = svc.create_base("金融库", "", None, None).await.unwrap();
+    svc.write_file(&info.id, "terms.md", "# 术语表\n市盈率 = PER\n").await.unwrap();
+
+    let retrieval: Arc<dyn KnowledgeRetrievalSink> =
+        Arc::new(nomifun_ai_agent::LiveKnowledgeRetrievalSink { service: svc.clone() });
+    let writeback: Arc<dyn KnowledgeWritebackSink> =
+        Arc::new(nomifun_ai_agent::LiveKnowledgeWritebackSink { service: svc.clone() });
+
+    let search = KnowledgeSearchTool::new(retrieval.clone(), vec![info.id.clone()]);
+    let read = KnowledgeReadTool::new(retrieval, vec![info.id.clone()]);
+    let write = KnowledgeWriteTool::new(
+        writeback,
+        vec![(info.id.clone(), info.name.clone())],
+        WriteMode::Direct,
+        vec![info.id.clone()],
+    );
+
+    // 1. Search → extract the opaque handle from the rendered result.
+    let s = search.execute(json!({"query": "市盈率"})).await;
+    assert!(!s.is_error, "{}", s.content);
+    let handle = s
+        .content
+        .lines()
+        .find_map(|l| l.trim().strip_prefix("handle: "))
+        .expect("search result must carry a handle")
+        .to_owned();
+
+    // 2. Read the full document by handle (no path arithmetic).
+    let r = read.execute(json!({ "handle": handle })).await;
+    assert!(!r.is_error && r.content.contains("市盈率"), "read by handle: {}", r.content);
+
+    // 3. Update by handle in DIRECT mode → overwrites the original in place.
+    let w = write
+        .execute(json!({ "handle": handle, "content": "# 术语表\n市盈率 = PER\nROE = 净资产收益率\n" }))
+        .await;
+    assert!(!w.is_error, "write by handle: {}", w.content);
+
+    let updated = svc.read_file(&info.id, "terms.md").await.unwrap().content;
+    assert!(updated.contains("ROE"), "original must be updated in place: {updated}");
+    let files = svc.list_files(&info.id).await.unwrap();
+    assert_eq!(
+        files.iter().filter(|f| f.rel_path.ends_with("terms.md")).count(),
+        1,
+        "must not create a duplicate document: {files:?}"
+    );
+}
@@ -0,0 +1,400 @@
+//! Integration tests for the ACP prompt pipeline.
+//!
+//! Unlike acp_agent_integration.rs, these tests do not exercise
+//! AcpAgentManager or the JSON-RPC protocol. They construct a
+//! PromptPipeline with the two built-in hooks and invoke
+//! pre_send against a real PromptCtx, asserting the observable
+//! prompt transformation.
+
+use std::collections::HashMap;
+use std::sync::Arc;
+
+use nomifun_ai_agent::capability::prompt_pipeline::{PromptCtx, PromptPipeline};
+use nomifun_ai_agent::factory::acp_assembler::{AcpSessionParams, WorkspaceInfo, assemble_acp_params};
+use nomifun_ai_agent::manager::acp::{
+    AcpSession, KnowledgeContextHook, ModelIdentityReminderHook, SessionNewPreludeHook,
+};
+use nomifun_ai_agent::registry::AgentRegistry;
+use nomifun_ai_agent::shared_kernel::ModelId;
+use nomifun_ai_agent::{AcpBuildExtra, AcpSkillManager, AgentRuntime};
+use nomifun_db::{SqliteAgentMetadataRepository, init_database_memory};
+
+// ── Fixtures ──────────────────────────────────────────────────────────────────
+
+async fn fixture_params(
+    backend: &str,
+    preset_context: Option<&str>,
+    is_custom_workspace: bool,
+) -> Arc<AcpSessionParams> {
+    let db = init_database_memory().await.unwrap();
+    let repo = Arc::new(SqliteAgentMetadataRepository::new(db.pool().clone()));
+    let registry = AgentRegistry::new(repo);
+    registry.hydrate().await.unwrap();
+
+    let metadata = registry
+        .find_builtin_by_backend(backend)
+        .await
+        .expect("seeded backend row must exist");
+
+    let config = AcpBuildExtra {
+        desktop_gateway: false,
+        gateway_mcp_config: None,
+        open_mcp_config: None,
+        computer_mcp_config: None,
+        browser_mcp_config: None,
+        agent_id: None,
+        backend: Some(backend.to_owned()),
+        cli_path: None,
+        agent_name: None,
+        custom_agent_id: None,
+        preset_context: preset_context.map(str::to_owned),
+        skills: vec![],
+        preset_assistant_id: None,
+        session_mode: None,
+        current_model_id: None,
+        cron_job_id: None,
+        team_mcp_stdio_config: None,
+        guide_mcp_config: None,
+        requirement_mcp_config: None,
+        knowledge_mcp_config: None,
+        mcp_server_ids: None,
+        session_mcp_servers: vec![],
+        user_id: None,
+        companion_id: None,
+        channel_platform: None,
+        knowledge_mounts: vec![],
+        knowledge_writeback: false,
+        knowledge_writeback_mode: None,
+        knowledge_writeback_eagerness: None,
+    };
+
+    Arc::new(
+        assemble_acp_params(
+            "conv-pp-test".into(),
+            WorkspaceInfo {
+                path: "/tmp".into(),
+                is_custom: is_custom_workspace,
+            },
+            metadata,
+            nomifun_common::CommandSpec {
+                command: "/usr/bin/true".into(),
+                args: vec![],
+                env: vec![],
+                cwd: None,
+            },
+            config,
+            Vec::new(),
+            None,
+            std::env::temp_dir(),
+        )
+        .await,
+    )
+}
+
+/// Like [`fixture_params`] but with a single mounted knowledge base, so the
+/// assembled params carry a non-empty `knowledge_context`. preset_context is
+/// `None` and workspace custom — the knowledge section is independent of both.
+async fn fixture_params_with_knowledge(backend: &str) -> Arc<AcpSessionParams> {
+    let db = init_database_memory().await.unwrap();
+    let repo = Arc::new(SqliteAgentMetadataRepository::new(db.pool().clone()));
+    let registry = AgentRegistry::new(repo);
+    registry.hydrate().await.unwrap();
+    let metadata = registry
+        .find_builtin_by_backend(backend)
+        .await
+        .expect("seeded backend row must exist");
+
+    let config = AcpBuildExtra {
+        backend: Some(backend.to_owned()),
+        knowledge_mounts: vec![nomifun_api_types::KnowledgeMountInfo {
+            id: "kb_1".into(),
+            name: "领域知识".into(),
+            description: "团队约定".into(),
+            rel_path: ".nomi/knowledge/领域知识".into(),
+            toc: vec!["concepts/术语.md — 术语表".into()],
+            summary: Some("Covers domain terms.".into()),
+            live_sources: vec![],
+        }],
+        ..Default::default()
+    };
+
+    Arc::new(
+        assemble_acp_params(
+            "conv-pp-test".into(),
+            WorkspaceInfo {
+                path: "/tmp".into(),
+                is_custom: true,
+            },
+            metadata,
+            nomifun_common::CommandSpec {
+                command: "/usr/bin/true".into(),
+                args: vec![],
+                env: vec![],
+                cwd: None,
+            },
+            config,
+            Vec::new(),
+            None,
+            std::env::temp_dir(),
+        )
+        .await,
+    )
+}
+
+fn fixture_skill_manager() -> Arc<AcpSkillManager> {
+    let tmp = tempfile::TempDir::new().unwrap();
+    let paths = Arc::new(nomifun_extension::resolve_skill_paths(tmp.path(), tmp.path()));
+    // tmp dir needs to live until the test finishes.
+    // mem::forget is acceptable in test code — we just don't need the Drop cleanup.
+    std::mem::forget(tmp);
+    AcpSkillManager::new(paths)
+}
+
+fn fixture_runtime() -> AgentRuntime {
+    AgentRuntime::new("conv-pp-test", "/tmp", 64)
+}
+
+fn make_pipeline() -> PromptPipeline {
+    // Mirror the real registration order in AcpAgentManager::new.
+    PromptPipeline::new(vec![
+        Arc::new(KnowledgeContextHook),
+        Arc::new(SessionNewPreludeHook),
+        Arc::new(ModelIdentityReminderHook),
+    ])
+}
+
+// ── Tests ─────────────────────────────────────────────────────────────────────
+
+/// First prompt after session/new: prelude block injected, flag consumed.
+#[tokio::test(flavor = "current_thread")]
+async fn brand_new_first_prompt_injects_preset_context() {
+    let params = fixture_params("claude", Some("Rule A"), true).await;
+    let skill_manager = fixture_skill_manager();
+    let runtime = fixture_runtime();
+    let mut session = AcpSession::new(None, None, HashMap::new());
+
+    // Simulate: open_session_new just succeeded.
+    session.mark_pending_session_new_prelude();
+
+    let pipeline = make_pipeline();
+
+    let mut ctx = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+
+    let out = pipeline.pre_send(&mut ctx, "hello".into()).await;
+    assert!(out.contains("[Assistant Rules]"), "prelude block missing: {out}");
+    assert!(out.contains("Rule A"), "preset_context missing: {out}");
+    assert!(out.ends_with("hello"), "user content should be at the end: {out}");
+
+    // Flag must have been consumed.
+    assert!(
+        !session.take_pending_session_new_prelude(),
+        "pending_session_new_prelude must be false after pre_send consumed it"
+    );
+}
+
+/// Second prompt: no prelude, no reminder — pure passthrough.
+#[tokio::test(flavor = "current_thread")]
+async fn second_prompt_is_passthrough() {
+    let params = fixture_params("claude", Some("Rule A"), true).await;
+    let skill_manager = fixture_skill_manager();
+    let runtime = fixture_runtime();
+    let mut session = AcpSession::new(None, None, HashMap::new());
+    session.mark_pending_session_new_prelude();
+
+    let pipeline = make_pipeline();
+
+    // First prompt consumes the flag.
+    {
+        let mut ctx = PromptCtx {
+            session: &mut session,
+            params: &params,
+            skill_manager: &skill_manager,
+            runtime: &runtime,
+        };
+        let _ = pipeline.pre_send(&mut ctx, "first".into()).await;
+    }
+
+    // Second prompt: flag already consumed.
+    let mut ctx = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+    let out = pipeline.pre_send(&mut ctx, "second".into()).await;
+    assert_eq!(out, "second", "no prelude / no reminder expected on second turn");
+}
+
+/// Resume path: no mark_pending_session_new_prelude — prompt must be unchanged.
+#[tokio::test(flavor = "current_thread")]
+async fn resume_path_does_not_inject() {
+    let params = fixture_params("claude", Some("Rule A"), true).await;
+    let skill_manager = fixture_skill_manager();
+    let runtime = fixture_runtime();
+
+    // Resume: session opened by open_session_resume which does NOT call
+    // mark_pending_session_new_prelude. The flag stays false.
+    let mut session = AcpSession::new(None, None, HashMap::new());
+
+    let pipeline = make_pipeline();
+
+    let mut ctx = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+
+    let out = pipeline.pre_send(&mut ctx, "continue the story".into()).await;
+    assert_eq!(out, "continue the story");
+}
+
+/// L2 fix: the knowledge retrieval-protocol section is delivered on a RESUMED
+/// session (open_session_resume marks pending_knowledge_prelude) even though the
+/// new-session [Assistant Rules] prelude is NOT. This is what makes a
+/// resumed/restarted ACP session — or one rebuilt after a 挂载知识库 change —
+/// actually trigger retrieval.
+#[tokio::test(flavor = "current_thread")]
+async fn knowledge_section_delivered_on_resume() {
+    let params = fixture_params_with_knowledge("claude").await;
+    let skill_manager = fixture_skill_manager();
+    let runtime = fixture_runtime();
+    let mut session = AcpSession::new(None, None, HashMap::new());
+
+    // Resume: open_session_resume marks the knowledge prelude but NOT the
+    // new-session prelude.
+    session.mark_pending_knowledge_prelude();
+
+    let pipeline = make_pipeline();
+    let mut ctx = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+
+    let out = pipeline.pre_send(&mut ctx, "用领域知识回答".into()).await;
+    assert!(out.contains("[Knowledge Bases]"), "knowledge block missing: {out}");
+    assert!(
+        out.contains("## Knowledge bases (extended knowledge source)"),
+        "retrieval protocol section missing: {out}"
+    );
+    assert!(out.contains("Retrieval protocol"), "retrieval protocol missing: {out}");
+    assert!(out.contains("领域知识"), "mounted base name missing: {out}");
+    // The new-session rules prelude must NOT appear on a resumed session.
+    assert!(!out.contains("[Assistant Rules]"), "rules prelude must not inject on resume: {out}");
+    assert!(out.ends_with("用领域知识回答"), "user content must survive at the end: {out}");
+
+    // Flag consumed — a later turn in the same session is a passthrough.
+    let mut ctx2 = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+    let out2 = pipeline.pre_send(&mut ctx2, "再问一句".into()).await;
+    assert_eq!(out2, "再问一句", "knowledge section must be one-shot per session open");
+}
+
+/// Without the pending_knowledge_prelude flag the section is not injected, even
+/// when bases are mounted (e.g. an ordinary mid-session turn).
+#[tokio::test(flavor = "current_thread")]
+async fn knowledge_section_skipped_without_flag() {
+    let params = fixture_params_with_knowledge("claude").await;
+    let skill_manager = fixture_skill_manager();
+    let runtime = fixture_runtime();
+    let mut session = AcpSession::new(None, None, HashMap::new());
+
+    let pipeline = make_pipeline();
+    let mut ctx = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+
+    let out = pipeline.pre_send(&mut ctx, "hello".into()).await;
+    assert_eq!(out, "hello", "no knowledge flag → passthrough");
+}
+
+/// Pending model notice: reminder prepended, then drained so second call is clean.
+#[tokio::test(flavor = "current_thread")]
+async fn pending_model_notice_triggers_reminder_prepend() {
+    let params = fixture_params("claude", None, true).await;
+    let skill_manager = fixture_skill_manager();
+    let runtime = fixture_runtime();
+    let mut session = AcpSession::new(None, None, HashMap::new());
+
+    // Simulate set_model reconciled successfully and stuck the notice.
+    session.set_pending_model_notice(ModelId::new("claude-opus-4"));
+
+    let pipeline = make_pipeline();
+
+    let mut ctx = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+
+    let out = pipeline.pre_send(&mut ctx, "go".into()).await;
+    assert!(out.contains("<system-reminder>"), "reminder missing: {out}");
+    assert!(out.ends_with("go"), "user content must survive at the end: {out}");
+
+    // Second call: notice already drained — no reminder.
+    let mut ctx2 = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+    let out2 = pipeline.pre_send(&mut ctx2, "next".into()).await;
+    assert_eq!(out2, "next");
+}
+
+/// Both flags set: reminder (outermost) wraps the prelude block.
+#[tokio::test(flavor = "current_thread")]
+async fn both_flags_prepend_reminder_outermost() {
+    let params = fixture_params("claude", Some("Rule A"), true).await;
+    let skill_manager = fixture_skill_manager();
+    let runtime = fixture_runtime();
+    let mut session = AcpSession::new(None, None, HashMap::new());
+    session.mark_pending_session_new_prelude();
+    session.set_pending_model_notice(ModelId::new("claude-opus-4"));
+
+    let pipeline = make_pipeline();
+
+    let mut ctx = PromptCtx {
+        session: &mut session,
+        params: &params,
+        skill_manager: &skill_manager,
+        runtime: &runtime,
+    };
+
+    let out = pipeline.pre_send(&mut ctx, "hi".into()).await;
+    let reminder_idx = out.find("<system-reminder>").expect("reminder must be present");
+    let rules_idx = out.find("[Assistant Rules]").expect("rules block must be present");
+    assert!(
+        reminder_idx < rules_idx,
+        "reminder must sit outside (before) the assistant rules block:\n{out}"
+    );
+    assert!(out.ends_with("hi"));
+}
+
+/// Skeleton: unlock once inject_first_message_prefix surfaces errors.
+#[tokio::test(flavor = "current_thread")]
+#[ignore = "SessionNewPreludeHook relies on inject_first_message_prefix which currently swallows I/O errors internally; unlocking this test requires surfacing a fallible boundary"]
+async fn prelude_io_failure_emits_prompt_hook_warning() {
+    // When inject_first_message_prefix exposes an error path, the hook
+    // should call emit_hook_warning("session_new_prelude", ...) and
+    // return the user content unchanged. Subscribers on runtime.subscribe()
+    // must then receive an AgentStreamEvent::AcpPromptHookWarning whose
+    // payload deserializes to AcpPromptHookWarningPayload with
+    // hook == "session_new_prelude".
+    let _ = fixture_params("claude", Some("ctx"), true).await;
+}
@@ -0,0 +1,300 @@
+//! Integration tests for the skill system.
+//!
+//! These tests verify the full skill lifecycle:
+//! - Skill discovery across multiple directories
+//! - Skill index generation
+//! - Lazy loading of skill bodies
+//! - LOAD_SKILL detection in agent output
+//! - System instruction building
+//! - First message preparation
+
+// Pre-existing: ENV_MUTEX MutexGuard held across await points is intentional —
+// it serializes env-var mutation across tests.
+#![allow(clippy::await_holding_lock)]
+
+use std::fs;
+use std::sync::{Arc, Mutex};
+
+use nomifun_ai_agent::{
+    AcpSkillManager, build_skills_index_text, build_system_instructions, detect_skill_load_request,
+    prepare_first_message, prepare_first_message_with_skills_index,
+};
+use nomifun_extension::{BUILTIN_SKILLS_ENV_VAR, resolve_skill_paths};
+use tempfile::TempDir;
+/// Serialize env var mutations across tests — `BUILTIN_SKILLS_ENV_VAR` is
+/// process-global so concurrent tests that set it must not interleave.
+static ENV_MUTEX: Mutex<()> = Mutex::new(());
+
+// ---------------------------------------------------------------------------
+// 4.0 New API: discover via extension service
+// ---------------------------------------------------------------------------
+
+#[tokio::test]
+async fn discover_skills_uses_extension_service_layout() {
+    let _guard = ENV_MUTEX.lock().unwrap();
+    let tmp = TempDir::new().unwrap();
+    let builtin_src = tmp.path().join("builtin-skills-src");
+    let data_dir = tmp.path().join("data");
+    fs::create_dir_all(&data_dir).unwrap();
+
+    // auto-inject skill: builtin-src/auto-inject/cron/SKILL.md
+    let auto_dir = builtin_src.join("auto-inject").join("cron");
+    fs::create_dir_all(&auto_dir).unwrap();
+    fs::write(
+        auto_dir.join("SKILL.md"),
+        "---\nname: cron\ndescription: Cron helper\n---\nBody",
+    )
+    .unwrap();
+
+    // opt-in builtin: builtin-src/mermaid/SKILL.md
+    let opt_dir = builtin_src.join("mermaid");
+    fs::create_dir_all(&opt_dir).unwrap();
+    fs::write(
+        opt_dir.join("SKILL.md"),
+        "---\nname: mermaid\ndescription: Mermaid diagrams\n---\nBody",
+    )
+    .unwrap();
+
+    // user custom: data/skills/my-skill/SKILL.md
+    let user_dir = data_dir.join("skills").join("my-skill");
+    fs::create_dir_all(&user_dir).unwrap();
+    fs::write(
+        user_dir.join("SKILL.md"),
+        "---\nname: my-skill\ndescription: User skill\n---\nBody",
+    )
+    .unwrap();
+
+    unsafe {
+        std::env::set_var(BUILTIN_SKILLS_ENV_VAR, &builtin_src);
+    }
+
+    let paths = Arc::new(resolve_skill_paths(tmp.path(), &data_dir));
+    let mgr = AcpSkillManager::new(paths);
+
+    // No enabled_skills: opt-in builtin (mermaid) and custom (my-skill) should
+    // be skipped. Only the auto-inject builtin (cron) appears.
+    let idx = mgr.discover_skills(None, None).await;
+    let names: std::collections::HashSet<&str> = idx.iter().map(|s| s.name.as_str()).collect();
+    assert!(names.contains("cron"), "auto-inject skill missing: got {names:?}");
+    assert!(
+        !names.contains("mermaid"),
+        "opt-in builtin leaked without enabled_skills"
+    );
+    assert!(!names.contains("my-skill"), "custom leaked without enabled_skills");
+
+    unsafe {
+        std::env::remove_var(BUILTIN_SKILLS_ENV_VAR);
+    }
+}
+
+#[tokio::test]
+async fn get_skill_loads_builtin_body_via_read_builtin_skill() {
+    let _guard = ENV_MUTEX.lock().unwrap();
+    let tmp = TempDir::new().unwrap();
+    let builtin = tmp.path().join("builtin");
+    let auto = builtin.join("auto-inject").join("bodyskill");
+    fs::create_dir_all(&auto).unwrap();
+    fs::write(
+        auto.join("SKILL.md"),
+        "---\nname: bodyskill\ndescription: B\n---\nBuiltin body content",
+    )
+    .unwrap();
+
+    let data_dir = tmp.path().join("data");
+    fs::create_dir_all(&data_dir).unwrap();
+
+    unsafe {
+        std::env::set_var(BUILTIN_SKILLS_ENV_VAR, &builtin);
+    }
+
+    let paths = Arc::new(resolve_skill_paths(tmp.path(), &data_dir));
+    let mgr = AcpSkillManager::new(paths);
+    mgr.discover_skills(None, None).await;
+
+    let skill = mgr.get_skill("bodyskill").await.unwrap();
+    assert_eq!(
+        skill.body.as_deref(),
+        Some("Builtin body content"),
+        "builtin body should be loaded via read_builtin_skill + extract_body"
+    );
+
+    unsafe {
+        std::env::remove_var(BUILTIN_SKILLS_ENV_VAR);
+    }
+}
+
+#[tokio::test]
+async fn get_skill_loads_custom_body_via_fs_read() {
+    let _guard = ENV_MUTEX.lock().unwrap();
+    let tmp = TempDir::new().unwrap();
+    let data_dir = tmp.path().join("data");
+    let user_skill = data_dir.join("skills").join("mine");
+    fs::create_dir_all(&user_skill).unwrap();
+    fs::write(
+        user_skill.join("SKILL.md"),
+        "---\nname: mine\ndescription: Mine\n---\nCustom body here",
+    )
+    .unwrap();
+
+    // Ensure no stale BUILTIN_SKILLS_ENV_VAR interferes
+    unsafe {
+        std::env::remove_var(BUILTIN_SKILLS_ENV_VAR);
+    }
+
+    let paths = Arc::new(resolve_skill_paths(tmp.path(), &data_dir));
+    let mgr = AcpSkillManager::new(paths);
+    let enabled = vec!["mine".to_string()];
+    let idx = mgr.discover_skills(Some(&enabled), None).await;
+
+    let names: Vec<&str> = idx.iter().map(|s| s.name.as_str()).collect();
+    assert!(
+        names.contains(&"mine"),
+        "custom 'mine' should be in index; got {names:?}"
+    );
+
+    let skill = mgr.get_skill("mine").await.unwrap();
+    assert_eq!(skill.body.as_deref(), Some("Custom body here"));
+}
+
+#[tokio::test]
+async fn discover_skills_respects_exclude_builtin() {
+    let _guard = ENV_MUTEX.lock().unwrap();
+    let tmp = TempDir::new().unwrap();
+    let builtin_src = tmp.path().join("b");
+    let auto_dir = builtin_src.join("auto-inject").join("cron");
+    fs::create_dir_all(&auto_dir).unwrap();
+    fs::write(
+        auto_dir.join("SKILL.md"),
+        "---\nname: cron\ndescription: Cron\n---\nBody",
+    )
+    .unwrap();
+
+    unsafe {
+        std::env::set_var(BUILTIN_SKILLS_ENV_VAR, &builtin_src);
+    }
+
+    let data_dir = tmp.path().join("data");
+    fs::create_dir_all(&data_dir).unwrap();
+    let paths = Arc::new(resolve_skill_paths(tmp.path(), &data_dir));
+    let mgr = AcpSkillManager::new(paths);
+    let exclude = vec!["cron".to_string()];
+    let idx = mgr.discover_skills(None, Some(&exclude)).await;
+    assert!(idx.is_empty(), "excluded auto-inject skill should be dropped");
+
+    unsafe {
+        std::env::remove_var(BUILTIN_SKILLS_ENV_VAR);
+    }
+}
+
+// ---------------------------------------------------------------------------
+// 5.2 Skill Index (pure function)
+// ---------------------------------------------------------------------------
+
+#[test]
+fn build_index_text_contains_load_protocol() {
+    let skills = vec![
+        nomifun_ai_agent::SkillIndex {
+            name: "security".into(),
+            description: "Security review".into(),
+        },
+        nomifun_ai_agent::SkillIndex {
+            name: "tdd".into(),
+            description: "Test-driven development".into(),
+        },
+    ];
+    let text = build_skills_index_text(&skills);
+
+    assert!(text.contains("[LOAD_SKILL: skill-name]"));
+    assert!(text.contains("- **security**: Security review"));
+    assert!(text.contains("- **tdd**: Test-driven development"));
+}
+
+// ---------------------------------------------------------------------------
+// 5.4 LOAD_SKILL Detection (pure function)
+// ---------------------------------------------------------------------------
+
+#[test]
+fn detect_single_load_skill_request() {
+    let content = "I need to use [LOAD_SKILL: security-review] to check this code.";
+    let skills = detect_skill_load_request(content);
+    assert_eq!(skills, vec!["security-review"]);
+}
+
+#[test]
+fn detect_multiple_load_skill_requests() {
+    let content = "[LOAD_SKILL: a] then [LOAD_SKILL: b] and [LOAD_SKILL: c]";
+    let skills = detect_skill_load_request(content);
+    assert_eq!(skills, vec!["a", "b", "c"]);
+}
+
+#[test]
+fn detect_no_load_skill_in_normal_text() {
+    let content = "This is just normal text without any skill requests.";
+    let skills = detect_skill_load_request(content);
+    assert!(skills.is_empty());
+}
+
+#[test]
+fn detect_load_skill_handles_whitespace() {
+    let content = "[LOAD_SKILL:   spaced-name   ]";
+    let skills = detect_skill_load_request(content);
+    assert_eq!(skills, vec!["spaced-name"]);
+}
+
+// ---------------------------------------------------------------------------
+// System instruction and first message builders
+// ---------------------------------------------------------------------------
+
+#[test]
+fn system_instructions_with_loaded_skills() {
+    let skills = vec![nomifun_ai_agent::SkillDefinition {
+        name: "helper".into(),
+        description: "A helper".into(),
+        location: std::path::PathBuf::new(),
+        source: nomifun_extension::SkillSource::Custom,
+        relative_location: None,
+        body: Some("Complete helper instructions.".into()),
+    }];
+    let result = build_system_instructions("Base system prompt", &skills);
+
+    assert!(result.starts_with("Base system prompt"));
+    assert!(result.contains("## Skill: helper"));
+    assert!(result.contains("Complete helper instructions."));
+}
+
+#[test]
+fn first_message_with_skills_index_for_acp() {
+    let skills = vec![nomifun_ai_agent::SkillIndex {
+        name: "review".into(),
+        description: "Code review".into(),
+    }];
+    let result = prepare_first_message_with_skills_index("Please review my code.", &skills, None);
+
+    assert!(result.contains("[Assistant Rules]"));
+    assert!(result.contains("- **review**: Code review"));
+    assert!(result.contains("[/Assistant Rules]"));
+    assert!(result.ends_with("Please review my code."));
+}
+
+#[test]
+fn first_message_with_full_skills_for_gemini() {
+    let skills = vec![nomifun_ai_agent::SkillDefinition {
+        name: "debug".into(),
+        description: "Debug".into(),
+        location: std::path::PathBuf::new(),
+        source: nomifun_extension::SkillSource::Custom,
+        relative_location: None,
+        body: Some("Full debug skill content.".into()),
+    }];
+    let result = prepare_first_message("Hello", &skills, Some("Be helpful."));
+
+    assert!(result.contains("[Assistant Rules]"));
+    assert!(result.contains("Be helpful."));
+    assert!(result.contains("Full debug skill content."));
+    assert!(result.contains("[/Assistant Rules]"));
+    assert!(result.ends_with("Hello"));
+}
+
+// User-override tests moved to the BUILTIN_SKILLS_ENV_VAR-based discovery
+// tests at the top of this file — see Task 5 for get_skill body-loading
+// coverage against the new skill_service-backed API.