Refactor: Make AgentLoop stateless, clean up architecture

AGENTS.md

@@ -51,14 +51,17 @@ Channel → MessageBus → SessionManager → AgentLoop → (tools) → SessionM
 | `session` | Conversation session lifecycle, dialog operations | `SessionManager`, `Session` |
 | `agent` | LLM call loop, tool execution, context compression | `AgentLoop` |
 | `providers` | LLM API clients (OpenAI-compatible, Anthropic) | `LLMProvider` trait, factory `create_provider()` |
-| `tools` | Agent tools (bash, file operations, http, web) | `ToolRegistry`, `Tool` trait |
+| `tools` | Agent tools (bash, file operations, http, web, get_skill) | `ToolRegistry`, `Tool` trait |
+| `skills` | Skills loading, management, and prompt building | `SkillsLoader`, `Skill` |
 
 ### Functional Boundaries
 
 - **Channels** only send/receive messages via `MessageBus`; they know nothing about sessions or LLM
 - **MessageBus** is a pure async queue; it routes nothing, just passes messages
 - **SessionManager** owns session state and dialog operations; it does NOT call LLM directly
+  - SessionManager is responsible for injecting skills prompt into conversation history
 - **AgentLoop** receives dialog events from `SessionManager`, calls LLM via `providers`, executes tools, returns text responses
+  - AgentLoop is stateless; all state is managed by Session/SessionManager
 - **Providers** are pure HTTP clients; no bus/session/channel awareness
 - **Tools** are executed by `AgentLoop`; they receive raw arguments and return string results
 
@@ -70,4 +73,4 @@ Channel → MessageBus → SessionManager → AgentLoop → (tools) → SessionM
 
 ## Known Issues
 
-- `src/session/session.rs:657` — `LLMProviderConfig` struct requires `workspace_dir` but test helper at line 656-669 doesn't provide it; test code needs `workspace_dir: PathBuf::new()` added
+- (No known issues at this time)

src/agent/agent_loop.rs

@@ -6,7 +6,6 @@ use crate::observability::{
     truncate_args, Observer, ObserverEvent, ToolExecutionOutcome,
 };
 use crate::providers::{create_provider, LLMProvider, ChatCompletionRequest, Message, ToolCall};
-use crate::skills::SkillsLoader;
 use crate::tools::ToolRegistry;
 use std::collections::VecDeque;
 use std::hash::{Hash, Hasher};
@@ -227,7 +226,6 @@ pub struct AgentLoop {
     max_iterations: usize,
     workspace_dir: PathBuf,
     model_name: String,
-    skills_loader: Arc<SkillsLoader>,
 }
 
 #[derive(Debug, Clone)]
@@ -238,7 +236,7 @@ pub struct AgentProcessResult {
 
 impl AgentLoop {
     /// Create a new AgentLoop with a provider created from config.
-    pub fn new(provider_config: LLMProviderConfig, skills_loader: Arc<SkillsLoader>) -> Result<Self, AgentError> {
+    pub fn new(provider_config: LLMProviderConfig) -> Result<Self, AgentError> {
         let max_iterations = provider_config.max_tool_iterations;
         let model_name = provider_config.model_id.clone();
         let workspace_dir = provider_config.workspace_dir.clone();
@@ -252,12 +250,11 @@ impl AgentLoop {
             max_iterations,
             workspace_dir,
             model_name,
-            skills_loader,
         })
     }
 
     /// Create a new AgentLoop with provider created from config and given tools.
-    pub fn with_tools(provider_config: LLMProviderConfig, tools: Arc<ToolRegistry>, skills_loader: Arc<SkillsLoader>) -> Result<Self, AgentError> {
+    pub fn with_tools(provider_config: LLMProviderConfig, tools: Arc<ToolRegistry>) -> Result<Self, AgentError> {
         let max_iterations = provider_config.max_tool_iterations;
         let model_name = provider_config.model_id.clone();
         let workspace_dir = provider_config.workspace_dir.clone();
@@ -271,12 +268,11 @@ impl AgentLoop {
             max_iterations,
             workspace_dir,
             model_name,
-            skills_loader,
         })
     }
 
     /// Create a new AgentLoop with an existing shared provider.
-    pub fn with_provider(provider: Arc<dyn LLMProvider>, max_iterations: usize, model_name: String, workspace_dir: PathBuf, skills_loader: Arc<SkillsLoader>) -> Self {
+    pub fn with_provider(provider: Arc<dyn LLMProvider>, max_iterations: usize, model_name: String, workspace_dir: PathBuf) -> Self {
         Self {
             provider,
             tools: Arc::new(ToolRegistry::new()),
@@ -284,7 +280,6 @@ impl AgentLoop {
             max_iterations,
             workspace_dir,
             model_name,
-            skills_loader,
         }
     }
 
@@ -295,7 +290,6 @@ impl AgentLoop {
         max_iterations: usize,
         model_name: String,
         workspace_dir: PathBuf,
-        skills_loader: Arc<SkillsLoader>,
     ) -> Self {
         Self {
             provider,
@@ -304,7 +298,6 @@ impl AgentLoop {
             max_iterations,
             workspace_dir,
             model_name,
-            skills_loader,
         }
     }
 
@@ -338,17 +331,7 @@ impl AgentLoop {
         // Build and inject system prompt if not present
         let has_system = messages.first().map_or(false, |m| m.role == "system");
         if !has_system {
-            let mut system_prompt = build_system_prompt(&self.workspace_dir, &self.model_name, &self.tools);
+            let system_prompt = build_system_prompt(&self.workspace_dir, &self.model_name, &self.tools);
-            
-            // Add skills prompt if there are skills
-            let skills_prompt = self.skills_loader.build_skills_prompt();
-            if !skills_prompt.is_empty() {
-                if !system_prompt.is_empty() {
-                    system_prompt.push_str("\n\n");
-                }
-                system_prompt.push_str(&skills_prompt);
-            }
-            
             #[cfg(debug_assertions)]
             tracing::debug!("System prompt injected:\n{}", system_prompt);
             messages.insert(0, ChatMessage::system(system_prompt));

src/session/session.rs

@@ -36,7 +36,6 @@ pub struct Session {
     tools: Arc<ToolRegistry>,
     compressor: ContextCompressor,
     store: Arc<SessionStore>,
-    skills_loader: Arc<SkillsLoader>,
 }
 
 impl Session {
@@ -46,7 +45,6 @@ impl Session {
         user_tx: mpsc::Sender<WsOutbound>,
         tools: Arc<ToolRegistry>,
         store: Arc<SessionStore>,
-        skills_loader: Arc<SkillsLoader>,
     ) -> Result<Self, AgentError> {
         let provider_box = create_provider(provider_config.clone())
             .map_err(|e| AgentError::Other(format!("provider creation error: {}", e)))?;
@@ -66,7 +64,6 @@ impl Session {
             tools,
             compressor: ContextCompressor::with_config(provider.clone(), provider_config.token_limit, compressor_config),
             store,
-            skills_loader,
         })
     }
 
@@ -181,7 +178,6 @@ impl Session {
             self.provider_config.max_tool_iterations,
             self.provider_config.model_id.clone(),
             self.provider_config.workspace_dir.clone(),
-            self.skills_loader.clone(),
         ))
     }
 }
@@ -393,7 +389,6 @@ impl SessionManager {
             user_tx,
             self.tools.clone(),
             self.store.clone(),
-            self.skills_loader.clone(),
         ).await?;
 
         let arc = Arc::new(Mutex::new(session));
@@ -421,7 +416,6 @@ impl SessionManager {
                 user_tx,
                 self.tools.clone(),
                 self.store.clone(),
-                self.skills_loader.clone(),
             ).await?;
 
             let arc = Arc::new(Mutex::new(session));
@@ -437,7 +431,6 @@ impl SessionManager {
             user_tx,
             self.tools.clone(),
             self.store.clone(),
-            self.skills_loader.clone(),
         ).await?;
 
         let arc = Arc::new(Mutex::new(session));