diff --git a/.beads/issues.jsonl b/.beads/issues.jsonl index 89956fbf6..95568ecea 100644 --- a/.beads/issues.jsonl +++ b/.beads/issues.jsonl @@ -1,3 +1,3 @@ -{"id":"terraphim-ai-2sz","title":"Add embedded device settings fallback to terraphim-cli","description":"Evaluate and implement an embedded DeviceSettings fallback (similar to terraphim-agent) so terraphim-cli doesn't fail on missing settings.","status":"open","priority":2,"issue_type":"task","owner":"alex@metacortex.engineer","created_at":"2026-02-10T08:23:48.689656434Z","created_by":"AlexMikhalev","updated_at":"2026-02-10T08:23:48.689656434Z"} -{"id":"terraphim-ai-cbm","title":"Clarify terraphim-agent TUI offline/server requirement","description":"Determine whether terraphim-agent TUI is expected to work fully offline or requires a running server; document requirement and adjust behavior if needed.","status":"open","priority":2,"issue_type":"task","owner":"alex@metacortex.engineer","created_at":"2026-02-10T08:23:40.310825316Z","created_by":"AlexMikhalev","updated_at":"2026-02-10T08:23:45.675303405Z"} -{"id":"terraphim-ai-tcw","title":"Define required feature parity between terraphim-agent and terraphim-cli","description":"Decide which commands/features must exist in both CLIs and document any intentional gaps for automation vs interactive use.","status":"open","priority":3,"issue_type":"task","owner":"alex@metacortex.engineer","created_at":"2026-02-10T08:23:51.548645229Z","created_by":"AlexMikhalev","updated_at":"2026-02-10T08:23:51.548645229Z"} +{"id":"terraphim-ai-2sz","title":"Add embedded device settings fallback to terraphim-cli","description":"Evaluate and implement an embedded DeviceSettings fallback (similar to terraphim-agent) so terraphim-cli doesn't fail on missing settings.","status":"open","priority":2,"issue_type":"task","owner":"alex@metacortex.engineer","created_at":"2026-02-10T08:23:48.689656434Z","created_by":"AlexMikhalev","updated_at":"2026-02-10T08:23:48.689656434Z","source_repo":".","compaction_level":0,"original_size":0} +{"id":"terraphim-ai-cbm","title":"Clarify terraphim-agent TUI offline/server requirement","description":"Determine whether terraphim-agent TUI is expected to work fully offline or requires a running server; document requirement and adjust behavior if needed.","design":"Phase 1/2 docs: docs/plans/terraphim-agent-tui-offline-server-research-2026-02-13.md and docs/plans/terraphim-agent-tui-offline-server-design-2026-02-13.md","acceptance_criteria":"Contract for fullscreen TUI vs REPL/offline is explicit in help/docs; actionable messaging when fullscreen TUI server is unreachable; tests cover mode behavior to prevent regressions.","notes":"Implemented on 2026-02-13: mode-contract wording in CLI/docs, fullscreen TUI server preflight with actionable repl fallback, and regression tests for help/non-TTY/server-failure paths. Validation: cargo fmt --package terraphim_agent; cargo clippy -p terraphim_agent --all-targets -- -D warnings; cargo test -p terraphim_agent --test offline_mode_tests; cargo test -p terraphim_agent --test server_mode_tests test_server_mode_config_show; targeted unit tests in main.rs for URL resolution and error messaging.","status":"closed","priority":2,"issue_type":"task","owner":"alex@metacortex.engineer","created_at":"2026-02-10T08:23:40.310825316Z","created_by":"AlexMikhalev","updated_at":"2026-02-13T14:41:42.093162011Z","closed_at":"2026-02-13T14:41:42.093136090Z","close_reason":"done","source_repo":".","compaction_level":0,"original_size":0} +{"id":"terraphim-ai-tcw","title":"Define required feature parity between terraphim-agent and terraphim-cli","description":"Decide which commands/features must exist in both CLIs and document any intentional gaps for automation vs interactive use.","status":"open","priority":3,"issue_type":"task","owner":"alex@metacortex.engineer","created_at":"2026-02-10T08:23:51.548645229Z","created_by":"AlexMikhalev","updated_at":"2026-02-10T08:23:51.548645229Z","source_repo":".","compaction_level":0,"original_size":0} diff --git a/Cargo.lock b/Cargo.lock index ea5fc7cc8..f3de82ba4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10476,9 +10476,6 @@ dependencies = [ "serenity", "teloxide", "tempfile", - "terraphim_automata", - "terraphim_config", - "terraphim_multi_agent", "thiserror 1.0.69", "tokio", "tokio-test", diff --git a/README.md b/README.md index 5039bb650..83a0ab61a 100644 --- a/README.md +++ b/README.md @@ -187,9 +187,12 @@ pip install terraphim-automata ### 🦀 Rust CLI/TUI ```bash -# Interactive mode with full features +# Fullscreen TUI (requires a running server at TERRAPHIM_SERVER or localhost:8000) terraphim-agent +# Offline-capable REPL +terraphim-agent repl + # Search commands terraphim-agent search "Rust async programming" terraphim-agent search --role engineer "microservices" @@ -382,9 +385,12 @@ $ terraphim-agent update # Build with all features cargo build -p terraphim_agent --features repl-full --release -# Launch interactive REPL +# Launch fullscreen TUI (server-backed) ./target/release/terraphim-agent +# Launch REPL (offline-capable by default) +./target/release/terraphim-agent repl + # Available REPL commands: /help # Show all commands /search "query" # Semantic search @@ -418,8 +424,8 @@ Search across AI coding assistant history from Claude Code, Cursor, and Aider se # Install with session search support cargo install terraphim_agent --features repl-full -# Launch REPL -terraphim-agent +# Launch REPL (offline-capable) +terraphim-agent repl # In REPL: /sessions sources # Detect available session sources (e.g., 419 Claude Code sessions) diff --git a/crates/terraphim_agent/docs/src/kg/test_ranking_kg.md b/crates/terraphim_agent/docs/src/kg/test_ranking_kg.md deleted file mode 100644 index e98c4ab7a..000000000 --- a/crates/terraphim_agent/docs/src/kg/test_ranking_kg.md +++ /dev/null @@ -1,13 +0,0 @@ -# Test Ranking Knowledge Graph - -### machine-learning -Machine learning enables systems to learn from experience. - -### rust -Rust is a systems programming language focused on safety. - -### python -Python is a high-level programming language. - -### search-algorithm -Search algorithms find data in structures. diff --git a/crates/terraphim_agent/src/main.rs b/crates/terraphim_agent/src/main.rs index 04516a092..4ba2aa8d1 100644 --- a/crates/terraphim_agent/src/main.rs +++ b/crates/terraphim_agent/src/main.rs @@ -46,12 +46,13 @@ enum LogicalOperatorCli { fn show_usage_info() { println!("Terraphim AI Agent v{}", env!("CARGO_PKG_VERSION")); println!(); - println!("Interactive Mode (requires TTY):"); - println!(" terraphim-agent # Start REPL or TUI"); - println!(" terraphim-agent repl # Explicit REPL mode"); + println!("Interactive Modes (requires TTY):"); + println!(" terraphim-agent # Start fullscreen TUI (requires running server)"); + println!(" terraphim-agent repl # Start REPL (offline-capable by default)"); + println!(" terraphim-agent repl --server # Start REPL in server mode"); println!(); println!("Common Commands:"); - println!(" search # Search documents"); + println!(" search # Search documents (offline-capable by default)"); println!(" roles list # List available roles"); println!(" config show # Show configuration"); println!(" replace # Replace terms using thesaurus"); @@ -62,6 +63,38 @@ fn show_usage_info() { println!(" terraphim-agent help # Show command-specific help"); } +fn resolve_tui_server_url(explicit: Option<&str>) -> String { + let env_server = std::env::var("TERRAPHIM_SERVER").ok(); + resolve_tui_server_url_with_env(explicit, env_server.as_deref()) +} + +fn resolve_tui_server_url_with_env(explicit: Option<&str>, env_server: Option<&str>) -> String { + explicit + .map(ToOwned::to_owned) + .or_else(|| env_server.map(ToOwned::to_owned)) + .unwrap_or_else(|| "http://localhost:8000".to_string()) +} + +fn tui_server_requirement_error(url: &str, cause: &anyhow::Error) -> anyhow::Error { + anyhow::anyhow!( + "Fullscreen TUI requires a running Terraphim server at {}. \ + Start terraphim_server or use offline mode with `terraphim-agent repl`. \ + Connection error: {}", + url, + cause + ) +} + +fn ensure_tui_server_reachable( + runtime: &tokio::runtime::Runtime, + api: &ApiClient, + url: &str, +) -> Result<()> { + runtime + .block_on(api.health()) + .map_err(|err| tui_server_requirement_error(url, &err)) +} + impl From for LogicalOperator { fn from(op: LogicalOperatorCli) -> Self { match op { @@ -362,6 +395,28 @@ mod tests { let text2 = "use npm, please"; assert!(is_at_word_boundary(text2, 4, 7)); // "npm" followed by comma } + + #[test] + fn resolve_tui_server_url_uses_explicit_then_env_then_default() { + let explicit = resolve_tui_server_url_with_env(Some("http://explicit:9000"), None); + assert_eq!(explicit, "http://explicit:9000"); + + let from_env = resolve_tui_server_url_with_env(None, Some("http://env:7000")); + assert_eq!(from_env, "http://env:7000"); + + let defaulted = resolve_tui_server_url_with_env(None, None); + assert_eq!(defaulted, "http://localhost:8000"); + } + + #[test] + fn tui_server_requirement_error_mentions_repl_fallback() { + let cause = anyhow::anyhow!("connect error"); + let err = tui_server_requirement_error("http://localhost:8000", &cause); + let msg = err.to_string(); + assert!(msg.contains("Fullscreen TUI requires a running Terraphim server")); + assert!(msg.contains("terraphim-agent repl")); + assert!(msg.contains("http://localhost:8000")); + } } #[derive(clap::ValueEnum, Debug, Clone, Default)] @@ -376,7 +431,11 @@ pub enum OutputFormat { } #[derive(Parser, Debug)] -#[command(name = "terraphim-agent", version, about = "Terraphim TUI interface")] +#[command( + name = "terraphim-agent", + version, + about = "Terraphim Agent: server-backed fullscreen TUI with offline-capable REPL and CLI commands" +)] struct Cli { /// Use server API mode instead of self-contained offline mode #[arg(long, default_value_t = false)] @@ -627,9 +686,8 @@ fn main() -> Result<()> { } } fn run_tui_offline_mode(transparent: bool) -> Result<()> { - // Note: TUI mode currently requires a running server to connect to. - // It will try TERRAPHIM_SERVER env var, or default to http://localhost:8000. - // For fully offline operation, use 'terraphim-agent repl' instead. + // Fullscreen TUI mode requires a running server. + // For offline operation, use `terraphim-agent repl`. run_tui(None, transparent) } @@ -1895,15 +1953,13 @@ fn ui_loop( let mut current_role = String::from("Terraphim Engineer"); // Default to Terraphim Engineer let mut selected_result_index = 0; let mut view_mode = ViewMode::Search; - // Use provided server_url, then TERRAPHIM_SERVER env var, then default to localhost - let effective_url = server_url.unwrap_or_else(|| { - std::env::var("TERRAPHIM_SERVER").unwrap_or_else(|_| "http://localhost:8000".to_string()) - }); - let api = ApiClient::new(effective_url); + let effective_url = resolve_tui_server_url(server_url.as_deref()); + let api = ApiClient::new(effective_url.clone()); // Create a tokio runtime for this TUI session // We need a local runtime because we're in a synchronous function (terminal event loop) let rt = tokio::runtime::Runtime::new()?; + ensure_tui_server_reachable(&rt, &api, &effective_url)?; // Initialize terms from rolegraph (selected role) if let Ok(cfg) = rt.block_on(async { api.get_config().await }) { diff --git a/crates/terraphim_agent/tests/offline_mode_tests.rs b/crates/terraphim_agent/tests/offline_mode_tests.rs index 13dff967c..91a8f344a 100644 --- a/crates/terraphim_agent/tests/offline_mode_tests.rs +++ b/crates/terraphim_agent/tests/offline_mode_tests.rs @@ -43,7 +43,7 @@ async fn test_offline_help_command() -> Result<()> { assert_eq!(code, 0, "Help command should succeed"); assert!( - stdout.contains("Terraphim TUI interface"), + stdout.contains("server-backed fullscreen TUI"), "Should show main help" ); assert!(stdout.contains("--server"), "Should show server flag"); @@ -65,6 +65,20 @@ async fn test_offline_help_command() -> Result<()> { Ok(()) } +#[tokio::test] +#[serial] +async fn test_non_tty_usage_clarifies_mode_contract() -> Result<()> { + let (stdout, _stderr, code) = run_offline_command(&[])?; + + assert_eq!(code, 0, "Non-TTY usage should exit cleanly"); + assert!(stdout.contains("Interactive Modes (requires TTY)")); + assert!(stdout.contains("fullscreen TUI (requires running server)")); + assert!(stdout.contains("repl")); + assert!(stdout.contains("offline-capable by default")); + + Ok(()) +} + #[tokio::test] #[serial] async fn test_offline_config_show() -> Result<()> { diff --git a/crates/terraphim_tinyclaw/Cargo.toml b/crates/terraphim_tinyclaw/Cargo.toml index 7cb281909..5fcbe54f6 100644 --- a/crates/terraphim_tinyclaw/Cargo.toml +++ b/crates/terraphim_tinyclaw/Cargo.toml @@ -11,11 +11,6 @@ name = "terraphim-tinyclaw" path = "src/main.rs" [dependencies] -# Terraphim crates -terraphim_multi_agent = { path = "../terraphim_multi_agent" } -terraphim_config = { path = "../terraphim_config" } -terraphim_automata = { path = "../terraphim_automata" } - # Async runtime tokio = { workspace = true } tokio-util = { version = "0.7", features = ["rt"] } diff --git a/crates/terraphim_tinyclaw/src/agent/agent_loop.rs b/crates/terraphim_tinyclaw/src/agent/agent_loop.rs index 9599b1443..9b3062fe9 100644 --- a/crates/terraphim_tinyclaw/src/agent/agent_loop.rs +++ b/crates/terraphim_tinyclaw/src/agent/agent_loop.rs @@ -16,6 +16,7 @@ use tokio_util::sync::CancellationToken; /// Configuration for the tool-calling loop. #[derive(Debug, Clone)] +#[allow(dead_code)] pub struct ToolCallingConfig { /// Maximum tool-calling iterations per message. pub max_iterations: usize, @@ -85,24 +86,22 @@ impl HybridLlmRouter { } } - /// Get a text-only response via direct GenAiLlmClient. - /// Used as fallback when proxy is unavailable. + /// Get a text-only response via proxy or direct LLM. + /// Used as fallback when proxy is unavailable for tool-calling. pub async fn text_only( &self, messages: Vec, system: Option, ) -> anyhow::Result { - // For now, return a placeholder - // In full implementation, this would use terraphim_multi_agent::GenAiLlmClient log::info!( - "Using direct LLM (provider: {}, model: {})", + "Using text-only mode (provider: {}, model: {})", self.direct_config.provider, self.direct_config.model ); // Try proxy first for text-only if available if self.proxy.is_available() { - match self.proxy.chat(messages, system).await { + match self.proxy.chat(messages.clone(), system.clone()).await { Ok(response) => { return Ok(response.content.unwrap_or_else(|| { "Tools are currently unavailable, answering from knowledge only." @@ -115,21 +114,134 @@ impl HybridLlmRouter { } } - // Fallback message - Ok("Tools are currently unavailable. I can answer questions from my training data, but cannot execute commands or access files.".to_string()) + // Try direct LLM (Ollama) + let base_url = self + .direct_config + .base_url + .clone() + .unwrap_or_else(|| "http://127.0.0.1:11434".to_string()); + + let last_user_msg = messages + .iter() + .rev() + .find(|m| m.role == "user") + .map(|m| m.content.clone()) + .unwrap_or_default(); + + let prompt = if let Some(sys) = &system { + format!("{}\n\nUser: {}", sys, last_user_msg) + } else { + last_user_msg + }; + + let client = reqwest::Client::new(); + let response = client + .post(format!("{}/api/generate", base_url)) + .json(&serde_json::json!({ + "model": &self.direct_config.model, + "prompt": prompt, + "stream": false + })) + .send() + .await; + + match response { + Ok(resp) if resp.status().is_success() => { + let body: serde_json::Value = resp.json().await?; + Ok(body["response"] + .as_str() + .unwrap_or("I received your message but could not generate a response.") + .to_string()) + } + _ => Ok( + "Tools and direct LLM are currently unavailable. Please check your configuration." + .to_string(), + ), + } } /// Compress context via direct LLM (cheap/local). - /// Never goes through proxy. + /// Never goes through proxy - uses Ollama or configured direct LLM. pub async fn compress( &self, - _messages: Vec, + messages: Vec, _system: String, ) -> anyhow::Result { - // Placeholder for LLM-based compression - // In full implementation, this would call GenAiLlmClient with Ollama - log::info!("Context compression via direct LLM (placeholder)"); - Ok("[Previous conversation summarized]".to_string()) + let base_url = self + .direct_config + .base_url + .clone() + .unwrap_or_else(|| "http://127.0.0.1:11434".to_string()); + let model = &self.direct_config.model; + + // Format conversation for summarization + let conversation = messages + .iter() + .map(|m| format!("{:?}: {}", m.role, m.content)) + .collect::>() + .join("\n"); + + let prompt = format!( + "Summarize the following conversation concisely, \ + preserving key facts, decisions, and context:\n\n{}", + conversation + ); + + log::info!( + "Context compression via {} ({}) - {} messages", + self.direct_config.provider, + model, + messages.len() + ); + + let client = reqwest::Client::new(); + let response = client + .post(format!("{}/api/generate", base_url)) + .json(&serde_json::json!({ + "model": model, + "prompt": prompt, + "stream": false + })) + .send() + .await; + + match response { + Ok(resp) if resp.status().is_success() => { + let body: serde_json::Value = resp.json().await?; + Ok(body["response"] + .as_str() + .unwrap_or("[Previous conversation summarized]") + .to_string()) + } + Ok(resp) => { + log::warn!( + "Direct LLM returned error ({}), using fallback summary", + resp.status() + ); + Ok(Self::fallback_summary(&messages)) + } + Err(e) => { + log::warn!("Direct LLM unreachable ({}), using fallback summary", e); + Ok(Self::fallback_summary(&messages)) + } + } + } + + /// Simple extractive fallback when no LLM is available. + fn fallback_summary(messages: &[ChatMessage]) -> String { + let total = messages.len(); + let recent: Vec = messages + .iter() + .rev() + .take(4) + .rev() + .map(|m| format!("{:?}: {}", m.role, &m.content[..m.content.len().min(100)])) + .collect(); + format!( + "[Summary of {} messages, recent context:]\n{}", + total, + recent.join("\n") + ) } } @@ -226,16 +338,22 @@ impl ToolCallingLoop { // Store messages for LLM conversion let session_messages: Vec<_> = session.messages.clone(); let session_clone = session.clone(); - let needs_compression = session.message_count() > 200; // Save session before releasing lock sessions_guard.save(&session_clone)?; drop(sessions_guard); - // Check if we need compression (handled separately to avoid borrow issues) - if needs_compression { - // Clone the messages we need for compression - let messages_to_compress = session_messages.clone(); + // Check if we need compression using configured ratio + let needs_compress = session_messages.len() > self.config.keep_last_messages * 2; + if needs_compress { + // Keep the last N messages, compress the rest + let keep_count = self.config.keep_last_messages; + let messages_to_compress: Vec<_> = if session_messages.len() > keep_count { + session_messages[..session_messages.len() - keep_count].to_vec() + } else { + session_messages.clone() + }; + let summary = self .router .compress(messages_to_compress, self.system_prompt.clone()) @@ -245,7 +363,18 @@ impl ToolCallingLoop { let mut sessions_guard = self.sessions.lock().await; let session = sessions_guard.get_or_create(&session_key); session.set_summary(summary); - session.clear_messages(); + // Keep only the recent messages + let recent: Vec<_> = session + .messages + .iter() + .rev() + .take(keep_count) + .cloned() + .collect::>() + .into_iter() + .rev() + .collect(); + session.messages = recent; let session_clone = session.clone(); sessions_guard.save(&session_clone)?; drop(sessions_guard); @@ -427,6 +556,7 @@ impl ToolCallingLoop { } /// Trigger graceful shutdown. + #[allow(dead_code)] pub fn shutdown(&self) { self.shutdown.cancel(); } @@ -457,7 +587,12 @@ mod tests { let messages = vec![Message::user("Hello")]; let response = router.text_only(messages, None).await.unwrap(); - assert!(response.contains("Tools are currently unavailable")); + // When both proxy and direct LLM are unavailable, returns unavailable message + assert!( + response.contains("unavailable") || response.contains("Hello"), + "Expected unavailable message or echoed input, got: {}", + response + ); } #[test] diff --git a/crates/terraphim_tinyclaw/src/agent/execution_guard.rs b/crates/terraphim_tinyclaw/src/agent/execution_guard.rs index e0f860214..d7b203e77 100644 --- a/crates/terraphim_tinyclaw/src/agent/execution_guard.rs +++ b/crates/terraphim_tinyclaw/src/agent/execution_guard.rs @@ -5,6 +5,7 @@ use serde_json::Value; /// Decision from execution guard evaluation. #[derive(Debug)] +#[allow(dead_code)] pub enum GuardDecision { /// Safe to execute. Allow, @@ -190,6 +191,7 @@ impl ExecutionGuard { } /// Convert a GuardDecision to a ToolError if it's a Block. + #[allow(dead_code)] pub fn to_error(&self, tool_name: &str, decision: GuardDecision) -> Result<(), ToolError> { match decision { GuardDecision::Allow => Ok(()), diff --git a/crates/terraphim_tinyclaw/src/agent/proxy_client.rs b/crates/terraphim_tinyclaw/src/agent/proxy_client.rs index 118f10b65..8f395a37b 100644 --- a/crates/terraphim_tinyclaw/src/agent/proxy_client.rs +++ b/crates/terraphim_tinyclaw/src/agent/proxy_client.rs @@ -269,6 +269,7 @@ pub struct ToolDefinition { /// Response from the proxy. #[derive(Debug)] +#[allow(dead_code)] pub struct ProxyResponse { pub content: Option, pub tool_calls: Vec, @@ -279,6 +280,7 @@ pub struct ProxyResponse { /// Token usage information. #[derive(Debug, Default)] +#[allow(dead_code)] pub struct TokenUsage { pub input_tokens: u32, pub output_tokens: u32, diff --git a/crates/terraphim_tinyclaw/src/channels/discord.rs b/crates/terraphim_tinyclaw/src/channels/discord.rs index 54d62027c..29528b9f9 100644 --- a/crates/terraphim_tinyclaw/src/channels/discord.rs +++ b/crates/terraphim_tinyclaw/src/channels/discord.rs @@ -29,19 +29,99 @@ impl Channel for DiscordChannel { "discord" } - async fn start(&self, _bus: Arc) -> anyhow::Result<()> { + async fn start(&self, bus: Arc) -> anyhow::Result<()> { log::info!("Discord channel starting"); self.running.store(true, Ordering::SeqCst); #[cfg(feature = "discord")] { - // Simplified implementation - just log for now - log::info!("Discord bot would start here"); + use crate::bus::InboundMessage; + use serenity::async_trait as serenity_async_trait; + use serenity::model::channel::Message as DiscordMessage; + use serenity::model::gateway::Ready; + use serenity::prelude::*; + + struct Handler { + inbound_tx: tokio::sync::mpsc::Sender, + allow_from: Vec, + } + + #[serenity_async_trait] + impl EventHandler for Handler { + async fn message(&self, _ctx: Context, msg: DiscordMessage) { + // Ignore bot messages + if msg.author.bot { + return; + } + + let sender_id = msg.author.id.to_string(); + let username = msg.author.name.clone(); + + // Check allowlist + if !self.allow_from.contains(&sender_id) && !self.allow_from.contains(&username) + { + log::warn!( + "Discord: rejected message from unauthorized user: {} ({})", + sender_id, + username + ); + return; + } + + let chat_id = msg.channel_id.to_string(); + let inbound = + InboundMessage::new("discord", &sender_id, &chat_id, &msg.content); + if let Err(e) = self.inbound_tx.send(inbound).await { + log::error!("Failed to forward Discord message to bus: {}", e); + } + } + + async fn ready(&self, _ctx: Context, ready: Ready) { + log::info!("Discord bot connected as {}", ready.user.name); + } + } + + let token = self.config.token.clone(); + let allow_from = self.config.allow_from.clone(); + let inbound_tx = bus.inbound_sender(); + let running = self.running.clone(); + + log::info!("Discord bot starting"); + + tokio::spawn(async move { + let intents = GatewayIntents::GUILD_MESSAGES + | GatewayIntents::DIRECT_MESSAGES + | GatewayIntents::MESSAGE_CONTENT; + + let handler = Handler { + inbound_tx, + allow_from, + }; + + let mut client = match Client::builder(&token, intents) + .event_handler(handler) + .await + { + Ok(c) => c, + Err(e) => { + log::error!("Failed to create Discord client: {}", e); + running.store(false, Ordering::SeqCst); + return; + } + }; + + if let Err(e) = client.start().await { + log::error!("Discord client error: {}", e); + } + running.store(false, Ordering::SeqCst); + }); + Ok(()) } #[cfg(not(feature = "discord"))] { + let _ = bus; anyhow::bail!("Discord feature not enabled") } } diff --git a/crates/terraphim_tinyclaw/src/channels/telegram.rs b/crates/terraphim_tinyclaw/src/channels/telegram.rs index b84bb18c2..6a44666fa 100644 --- a/crates/terraphim_tinyclaw/src/channels/telegram.rs +++ b/crates/terraphim_tinyclaw/src/channels/telegram.rs @@ -29,22 +29,79 @@ impl Channel for TelegramChannel { "telegram" } - async fn start(&self, _bus: Arc) -> anyhow::Result<()> { + async fn start(&self, bus: Arc) -> anyhow::Result<()> { log::info!("Telegram channel starting"); self.running.store(true, Ordering::SeqCst); #[cfg(feature = "telegram")] { - // Simplified implementation - just log for now + use crate::bus::InboundMessage; + use teloxide::prelude::*; + + let bot = teloxide::Bot::new(&self.config.token); + let allow_from = self.config.allow_from.clone(); + let inbound_tx = bus.inbound_sender(); + let running = self.running.clone(); + log::info!( - "Telegram bot would start here with token: {}...", + "Telegram bot starting (token: {}...)", &self.config.token[..self.config.token.len().min(10)] ); + + tokio::spawn(async move { + let handler = Update::filter_message().endpoint( + move |msg: teloxide::types::Message, _bot: teloxide::Bot| { + let tx = inbound_tx.clone(); + let allowed = allow_from.clone(); + async move { + let sender_id = msg + .from + .as_ref() + .map(|u| u.id.to_string()) + .unwrap_or_default(); + let username = msg + .from + .as_ref() + .and_then(|u| u.username.clone()) + .unwrap_or_default(); + + // Check allowlist by user ID or username + if !allowed.contains(&sender_id) && !allowed.contains(&username) { + log::warn!( + "Telegram: rejected message from unauthorized user: {} ({})", + sender_id, + username + ); + return Ok(()); + } + + if let Some(text) = msg.text() { + let chat_id = msg.chat.id.to_string(); + let inbound = + InboundMessage::new("telegram", &sender_id, &chat_id, text); + if let Err(e) = tx.send(inbound).await { + log::error!("Failed to forward Telegram message to bus: {}", e); + } + } + Ok::<(), anyhow::Error>(()) + } + }, + ); + + let mut dispatcher = Dispatcher::builder(bot, handler) + .default_handler(|_| async {}) + .build(); + + dispatcher.dispatch().await; + running.store(false, Ordering::SeqCst); + }); + Ok(()) } #[cfg(not(feature = "telegram"))] { + let _ = bus; anyhow::bail!("Telegram feature not enabled") } } diff --git a/crates/terraphim_tinyclaw/src/lib.rs b/crates/terraphim_tinyclaw/src/lib.rs index fd8b83a8e..697683c7d 100644 --- a/crates/terraphim_tinyclaw/src/lib.rs +++ b/crates/terraphim_tinyclaw/src/lib.rs @@ -8,7 +8,6 @@ //! - **Tools**: Extensible tool system for filesystem, web, shell operations //! - **Skills**: JSON-based reusable workflows //! - **Session Management**: Persistent conversation history -#![allow(dead_code)] pub mod agent; pub mod bus; diff --git a/crates/terraphim_tinyclaw/src/main.rs b/crates/terraphim_tinyclaw/src/main.rs index 534bce5e8..bd6cd3b01 100644 --- a/crates/terraphim_tinyclaw/src/main.rs +++ b/crates/terraphim_tinyclaw/src/main.rs @@ -1,12 +1,18 @@ -#![allow(dead_code)] mod agent; +#[allow(dead_code)] mod bus; +#[allow(dead_code)] mod channel; mod channels; +#[allow(dead_code)] mod config; +#[allow(dead_code)] mod format; +#[allow(dead_code)] mod session; +#[allow(dead_code)] mod skills; +#[allow(dead_code)] mod tools; use crate::agent::agent_loop::{HybridLlmRouter, ToolCallingLoop}; @@ -250,6 +256,18 @@ async fn run_gateway_mode(config: Config) -> anyhow::Result<()> { } }); + // Dispatch outbound messages to channels + let bus_clone = bus.clone(); + tokio::spawn(async move { + let mut outbound_rx = bus_clone.outbound_rx.lock().await; + while let Some(msg) = outbound_rx.recv().await { + log::debug!("Dispatching outbound to channel: {}", msg.channel); + if let Err(e) = channel_manager.send(msg).await { + log::error!("Failed to dispatch outbound message: {}", e); + } + } + }); + // Wait for shutdown signal match tokio::signal::ctrl_c().await { Ok(()) => { @@ -260,9 +278,6 @@ async fn run_gateway_mode(config: Config) -> anyhow::Result<()> { } } - // Graceful shutdown - channel_manager.stop_all().await?; - Ok(()) } diff --git a/crates/terraphim_tinyclaw/src/skills/executor.rs b/crates/terraphim_tinyclaw/src/skills/executor.rs index ff24e3fc3..6afd50c7f 100644 --- a/crates/terraphim_tinyclaw/src/skills/executor.rs +++ b/crates/terraphim_tinyclaw/src/skills/executor.rs @@ -3,6 +3,7 @@ #[cfg(test)] use crate::skills::types::SkillInput; use crate::skills::types::{Skill, SkillResult, SkillStatus, SkillStep, StepResult}; +use crate::tools::{ToolCall, ToolRegistry}; use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -35,6 +36,8 @@ pub struct SkillExecutor { storage_dir: PathBuf, /// Cancellation flag shared across execution cancelled: Arc, + /// Optional tool registry for executing tool steps + tool_registry: Option>, } impl SkillExecutor { @@ -46,9 +49,16 @@ impl SkillExecutor { Ok(Self { storage_dir, cancelled: Arc::new(AtomicBool::new(false)), + tool_registry: None, }) } + /// Set the tool registry for executing tool steps. + pub fn with_tool_registry(mut self, registry: Arc) -> Self { + self.tool_registry = Some(registry); + self + } + /// Get the default skills directory (~/.config/terraphim/skills). pub fn default_storage_dir() -> PathBuf { dirs::config_dir() @@ -310,11 +320,22 @@ impl SkillExecutor { let substituted = self.substitute_template(&args_str, inputs)?; let substituted_args: serde_json::Value = serde_json::from_str(&substituted)?; - // Placeholder - actual tool execution would integrate with the tool registry - Ok(format!( - "Executed tool '{}' with args: {}", - tool, substituted_args - )) + if let Some(ref registry) = self.tool_registry { + let call = ToolCall { + id: format!("skill_step_{}", uuid::Uuid::new_v4()), + name: tool.to_string(), + arguments: substituted_args, + }; + registry.execute(&call).await.map_err(|e| { + SkillError::Template(format!("Tool '{}' execution failed: {}", tool, e)) + }) + } else { + // No registry available - return descriptive message + Ok(format!( + "Executed tool '{}' with args: {} (no tool registry configured)", + tool, substituted_args + )) + } } async fn execute_llm_step( @@ -325,8 +346,48 @@ impl SkillExecutor { ) -> Result { let substituted = self.substitute_template(prompt, inputs)?; - // Placeholder - actual LLM execution would integrate with the agent - Ok(format!("LLM prompt: {}", substituted)) + // Try Ollama if OLLAMA_BASE_URL is set + let base_url = std::env::var("OLLAMA_BASE_URL") + .unwrap_or_else(|_| "http://127.0.0.1:11434".to_string()); + let model = std::env::var("OLLAMA_MODEL").unwrap_or_else(|_| "llama3.2".to_string()); + + let client = reqwest::Client::new(); + let response = client + .post(format!("{}/api/generate", base_url)) + .json(&serde_json::json!({ + "model": model, + "prompt": substituted, + "stream": false + })) + .send() + .await; + + match response { + Ok(resp) if resp.status().is_success() => { + let body: serde_json::Value = resp.json().await.map_err(|e| { + SkillError::Template(format!("Failed to parse Ollama response: {}", e)) + })?; + Ok(body["response"] + .as_str() + .unwrap_or("(empty response)") + .to_string()) + } + Ok(resp) => { + log::warn!("Ollama returned error: {}", resp.status()); + Ok(format!( + "LLM unavailable (status {}), prompt was: {}", + resp.status(), + substituted + )) + } + Err(e) => { + log::warn!("Ollama unreachable: {}", e); + Ok(format!( + "LLM unavailable ({}), prompt was: {}", + e, substituted + )) + } + } } async fn execute_shell_step( @@ -337,11 +398,36 @@ impl SkillExecutor { ) -> Result { let substituted = self.substitute_template(command, inputs)?; - // Placeholder - actual shell execution - Ok(format!( - "Shell command: {} (in {:?})", - substituted, working_dir - )) + let mut cmd = tokio::process::Command::new("sh"); + cmd.arg("-c").arg(&substituted); + + if let Some(dir) = working_dir { + let dir = self.substitute_template(dir, inputs)?; + cmd.current_dir(dir); + } + + let output = tokio::time::timeout(Duration::from_secs(120), cmd.output()) + .await + .map_err(|_| SkillError::Timeout)? + .map_err(SkillError::Io)?; + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + + if output.status.success() { + if stdout.is_empty() { + Ok("(exit 0, no output)".to_string()) + } else { + Ok(stdout.to_string()) + } + } else { + Ok(format!( + "Exit code: {}\nStdout: {}\nStderr: {}", + output.status.code().unwrap_or(-1), + stdout, + stderr + )) + } } } @@ -366,13 +452,13 @@ mod tests { description: "A test skill".to_string(), author: Some("Test".to_string()), steps: vec![ - SkillStep::Tool { - tool: "shell".to_string(), - args: serde_json::json!({"command": "echo {message}"}), + SkillStep::Shell { + command: "echo {message}".to_string(), + working_dir: None, }, - SkillStep::Llm { - prompt: "Analyze: {message}".to_string(), - use_context: true, + SkillStep::Shell { + command: "echo 'processed: {message}'".to_string(), + working_dir: None, }, ], inputs: vec![SkillInput { @@ -488,7 +574,24 @@ mod tests { async fn test_execute_skill_success() { let temp_dir = TempDir::new().unwrap(); let executor = SkillExecutor::new(temp_dir.path()).unwrap(); - let skill = create_test_skill(); + + // Use a skill with a real shell step and an LLM step + let skill = Skill { + name: "test-skill".to_string(), + version: "1.0.0".to_string(), + description: "A test skill".to_string(), + author: Some("Test".to_string()), + steps: vec![SkillStep::Shell { + command: "echo {message}".to_string(), + working_dir: None, + }], + inputs: vec![SkillInput { + name: "message".to_string(), + description: "Message to process".to_string(), + required: true, + default: None, + }], + }; let mut inputs = HashMap::new(); inputs.insert("message".to_string(), "world".to_string()); @@ -496,7 +599,7 @@ mod tests { let result = executor.execute_skill(&skill, inputs, None).await.unwrap(); assert_eq!(result.status, SkillStatus::Success); - assert_eq!(result.execution_log.len(), 2); + assert_eq!(result.execution_log.len(), 1); assert!(result.output.contains("world")); } @@ -517,14 +620,15 @@ mod tests { let temp_dir = TempDir::new().unwrap(); let executor = SkillExecutor::new(temp_dir.path()).unwrap(); + // Use a shell step with defaults for reliable testing let skill = Skill { name: "test-with-default".to_string(), version: "1.0.0".to_string(), description: "Test".to_string(), author: None, - steps: vec![SkillStep::Llm { - prompt: "Hello {name}".to_string(), - use_context: false, + steps: vec![SkillStep::Shell { + command: "echo Hello {name}".to_string(), + working_dir: None, }], inputs: vec![SkillInput { name: "name".to_string(), @@ -548,24 +652,24 @@ mod tests { let executor = SkillExecutor::new(temp_dir.path()).unwrap(); let executor_clone = SkillExecutor::new(temp_dir.path()).unwrap(); - // Create a skill with multiple steps + // Create a skill with multiple shell steps let skill = Skill { name: "multi-step".to_string(), version: "1.0.0".to_string(), description: "Multi-step skill".to_string(), author: None, steps: vec![ - SkillStep::Llm { - prompt: "Step 1".to_string(), - use_context: false, + SkillStep::Shell { + command: "echo step1".to_string(), + working_dir: None, }, - SkillStep::Llm { - prompt: "Step 2".to_string(), - use_context: false, + SkillStep::Shell { + command: "echo step2".to_string(), + working_dir: None, }, - SkillStep::Llm { - prompt: "Step 3".to_string(), - use_context: false, + SkillStep::Shell { + command: "echo step3".to_string(), + working_dir: None, }, ], inputs: vec![], @@ -606,13 +710,13 @@ mod tests { description: "Slow skill".to_string(), author: None, steps: vec![ - SkillStep::Llm { - prompt: "Step 1".to_string(), - use_context: false, + SkillStep::Shell { + command: "echo step1".to_string(), + working_dir: None, }, - SkillStep::Llm { - prompt: "Step 2".to_string(), - use_context: false, + SkillStep::Shell { + command: "echo step2".to_string(), + working_dir: None, }, ], inputs: vec![], diff --git a/docs/plans/pr-issues-design-2026-02-13.md b/docs/plans/pr-issues-design-2026-02-13.md new file mode 100644 index 000000000..c60429162 --- /dev/null +++ b/docs/plans/pr-issues-design-2026-02-13.md @@ -0,0 +1,234 @@ +# Implementation Plan: PR/Issue Execution with PR #527 Merged First + +**Status**: Draft +**Research Doc**: `docs/plans/pr-issues-research-2026-02-13.md` +**Author**: Codex +**Date**: 2026-02-13 +**Estimated Effort**: 5-7 working days (planning + execution) + +## Overview + +### Summary +This plan defines a merge-and-delivery sequence that starts by merging PR #527 first, then resumes architecture phases (#521-#526) with explicit gates. The objective is to reduce branch conflict risk, restore CI signal quality, and preserve architecture roadmap momentum. + +### Approach +Use three controlled lanes with hard gates: +1. `Feature Lane` (immediate): unblock and merge PR #527 first. +2. `Architecture Lane` (next): execute P0/P1/P2/P3/P4 in order (`#521` -> `#526`). +3. `Dependency Lane` (background): batch dependabot updates in scheduled windows, not continuously. + +### Scope +**In Scope:** +- Merge-first execution plan for PR #527 +- Gate definitions (entry/exit criteria) for post-#527 architecture phases +- File-level change plan for process/docs/CI artifacts +- Test and evidence strategy per phase +- Issue/PR update protocol using `gh` + +**Out of Scope:** +- Implementing code refactors in server/service crates +- Rewriting TinyClaw design +- Closing historical stale issues en masse in this same execution window + +**Avoid At All Cost** (5/25 discipline): +- Parallel deep refactors in `terraphim_server` and `terraphim_service` before #527 lands +- Merging dependency PRs opportunistically during #527 stabilization +- Introducing new architecture epics before #520 phase chain advances +- Coupling #527 merge criteria to unrelated stale backlog cleanup +- Running broad “big-bang” CI matrix changes during merge-critical window + +## Architecture + +### Component Diagram +``` +[PR #527 Readiness] + | + v +[Merge Gate Controller] + - mergeability + - required checks + - branch sync + - scope hygiene + | + +--> if pass --> [Merge #527] + | + +--> if fail --> [Targeted remediation loop] + +After merge #527: +[Phase Gate Controller] + P0 (#521/#522) -> P1 (#523) -> P2 (#524) -> P3 (#525) -> P4 (#526) + | + v +[Evidence Store in docs/ + PR notes + issue updates] +``` + +### Data Flow +``` +Issue/PR state -> Gate evaluation -> Required actions -> CI evidence -> Merge/advance decision +``` + +### Key Design Decisions +| Decision | Rationale | Alternatives Rejected | +|----------|-----------|----------------------| +| Merge #527 first | User-directed priority; reduces long-lived branch drift | Deferring #527 behind architecture P0/P1 | +| Gate-based readiness checklist for #527 | Prevents subjective merge calls and rework | Ad-hoc merge judgment | +| Freeze architecture-sensitive merges while stabilizing #527 | Minimizes conflicts and CI noise | Continue normal parallel merges | +| Batch dependency PR handling | Protects review bandwidth and CI clarity | Continuous unbatched dependabot merges | +| Sequential architecture phases after #527 | Aligns with existing #520 contract and acceptance criteria | Multi-phase concurrent execution | + +### Eliminated Options (Essentialism) +| Option Rejected | Why Rejected | Risk of Including | +|-----------------|--------------|-------------------| +| Start Phase 0 architecture before #527 merge | Conflicts with explicit user priority | Higher merge conflict probability | +| Re-scope #527 to include architecture cleanup | Scope creep | Delayed merge and unclear ownership | +| Add new CI pipelines immediately | Not required for first merge objective | Pipeline instability during critical merge | +| Mass close stale issues now | Not on critical path to #527 merge | Loss of focus and review churn | +| Full dependency upgrade wave now | Competes with merge-critical validation | Breakage surface expansion | + +### Simplicity Check +**What if this could be easy?** +Easy path: treat #527 as a bounded merge candidate, fix only blockers that affect mergeability/checks/scope, merge it, then execute architecture phases using already-defined issue contracts. + +**Senior Engineer Test:** passes; the plan is intentionally procedural and minimizes concurrent moving parts. + +**Nothing Speculative Checklist**: +- [x] No features beyond requested planning scope +- [x] No speculative abstractions +- [x] No "just in case" flexibility layers +- [x] No impossible-case error branches +- [x] No premature optimization tasks + +## File Changes + +### New Files +| File | Purpose | +|------|---------| +| `docs/plans/pr-issues-design-2026-02-13.md` | Phase 2 design plan for merge-first execution | + +### Modified Files +| File | Changes | +|------|---------| +| `docs/plans/pr-issues-research-2026-02-13.md` | No changes required in design phase | + +### Deleted Files +| File | Reason | +|------|--------| +| None | N/A | + +## API/Interface Design (Process-Level) + +### Gate Contract: PR #527 Merge Readiness +```text +Input: +- PR metadata (merge state, changed files, reviews) +- CI check statuses +- branch divergence state + +Output: +- READY_TO_MERGE | BLOCKED +- Blocker list with owner + remediation action +``` + +### Gate Contract: Architecture Phase Advancement +```text +Input: +- Prior phase completion evidence +- CI evidence bundle +- dependency diff/compile-time notes (where applicable) + +Output: +- ADVANCE_TO_NEXT_PHASE | HOLD +- Missing evidence checklist +``` + +### Error States +```text +BLOCKED_MERGEABILITY: PR merge state DIRTY or CONFLICTING +BLOCKED_CI: required checks failing +BLOCKED_SCOPE: PR contains non-target artifacts requiring split/revert strategy +BLOCKED_PHASE_EVIDENCE: missing ADR/baseline/check artifacts for phase gate +``` + +## Test Strategy + +### Unit-Level (Process Validation via Commands) +| Test | Command | Purpose | +|------|---------|---------| +| PR merge state check | `gh pr view 527 --json mergeStateStatus` | Ensure clean mergeability | +| CI status check | `gh pr checks 527` | Ensure required checks pass | +| PR scope check | `gh pr view 527 --json files` | Validate file set is intentional | + +### Integration-Level (Workflow Validation) +| Test | Evidence | Purpose | +|------|----------|---------| +| Merge-first workflow dry run | Checklist completion log | Verify end-to-end #527 readiness path | +| Phase gate transition P0->P1 | Issue update + CI artifacts | Verify architecture gating discipline | +| Dependency lane isolation | PR queue snapshot | Verify bot PRs do not interrupt critical lane | + +### Verification Commands by Stage +```bash +# Stage A: #527 merge readiness +gh pr view 527 --json mergeStateStatus,statusCheckRollup,files +gh pr checks 527 +cargo fmt --check +cargo clippy --workspace --all-targets --all-features +cargo test --workspace +ubs $(git diff --name-only) + +# Stage B: post-merge architecture phase gates +gh issue view 521 +gh issue view 522 +cargo tree -d --workspace -e no-dev +``` + +## Implementation Steps + +### Step 1: #527 Triage and Scope Hygiene +**Targets:** PR #527 metadata + file list +**Description:** classify blockers into mergeability, CI, and scope categories; decide exact remediation actions. +**Exit Criteria:** blocker list with clear owner/action; no ambiguous blocker remains. + +### Step 2: #527 Branch Synchronization +**Targets:** branch `claude/tinyclaw-terraphim-plan-lIt3V` +**Description:** rebase/merge latest `main`, resolve DIRTY state, re-run required checks. +**Exit Criteria:** `mergeStateStatus=CLEAN` (or equivalent mergeable), checks pending/pass. + +### Step 3: #527 CI Stabilization +**Targets:** failing workflows (`Quick Rust Validation`, `Warden review`) +**Description:** fix only failures directly preventing merge; avoid opportunistic refactors. +**Exit Criteria:** required checks green; no new failing required checks. + +### Step 4: #527 Review and Merge +**Targets:** PR review state + merge action +**Description:** request review, address comments, merge with conventional commit strategy. +**Exit Criteria:** PR #527 merged into `main`; issue #519 updated with merge note and follow-up list. + +### Step 5: Resume Architecture Phases in Order +**Targets:** #521 -> #522 -> #523 -> #524 -> #525 -> #526 +**Description:** execute each phase with explicit gate evidence before opening next phase PR. +**Exit Criteria:** each issue has linked PR/evidence; no phase skipping. + +### Step 6: Dependency PR Windowing +**Targets:** dependabot PR queue +**Description:** batch low-risk dependency updates into planned windows after architecture gate checkpoints. +**Exit Criteria:** reduced review noise and preserved critical-lane CI signal. + +## Acceptance Criteria + +1. PR #527 merges first, before architecture phase implementation PRs. +2. #527 merge occurs with passing required checks and resolved mergeability. +3. Post-#527 architecture execution follows #520 phase order without skips. +4. Each phase has attached objective evidence (CI + dependency/compile notes where relevant). +5. Dependency updates are processed in controlled batches, not continuously during critical merges. + +## Execution Checklist (Operator View) + +- [ ] Confirm #527 blocker list (mergeability/CI/scope) +- [ ] Resolve DIRTY merge state on #527 +- [ ] Pass required #527 checks +- [ ] Merge #527 +- [ ] Update #519 with merge summary + residual tasks +- [ ] Start #521 ADR/baseline deliverables +- [ ] Complete #522 CI dependency guard +- [ ] Advance #523, #524, #525, #526 in sequence + diff --git a/docs/plans/pr-issues-overall-design-2026-02-13.md b/docs/plans/pr-issues-overall-design-2026-02-13.md new file mode 100644 index 000000000..0c930f832 --- /dev/null +++ b/docs/plans/pr-issues-overall-design-2026-02-13.md @@ -0,0 +1,321 @@ +# Implementation Plan: Post-#527 Portfolio Execution (Overall) + +**Status**: Draft +**Research Doc**: `docs/plans/pr-issues-research-2026-02-13.md` +**Author**: Codex +**Date**: 2026-02-13 +**Estimated Effort**: 4-6 working days + +## Overview + +### Summary +This plan defines the overall execution sequence after `PR #527` was merged on 2026-02-13. It prioritizes finishing currently active human PRs (`#516`, `#492`, `#489`), then advancing architecture phases (`#521` -> `#526`), while controlling dependency PR noise. + +### Approach +Operate in three lanes with explicit gates: +1. `Stabilization Lane`: land active human PRs in deterministic order. +2. `Architecture Lane`: execute issue chain `#521` -> `#526` with evidence gates. +3. `Dependency Lane`: batch Dependabot PRs in scheduled windows only. + +### Scope +**In Scope:** +- Merge-order and gate design for open human PRs after `#527` +- Architecture issue execution order and entry/exit criteria +- CI evidence matrix and command-level verification strategy +- PR/issue operational workflow (`gh`-driven updates and decisions) + +**Out of Scope:** +- Implementing architecture refactors +- Broad stale-backlog closure campaign +- New product feature design unrelated to current open PRs/issues + +**Avoid At All Cost** (5/25 discipline): +- Parallel refactors across `terraphim_service`, `terraphim_server`, and `terraphim_agent` +- Merging `#489` without narrowing/splitting mixed scope +- Opportunistic dependency merges during human PR stabilization +- Re-introducing CI workflow churn during merge-critical window +- Adding new architecture epics before `#521`/`#522` are complete +- Rewriting merged `#527` outcomes in same cycle +- Expanding acceptance criteria mid-flight +- Mixing branch-protection policy changes with feature merges +- Running full workspace redesign while unresolved conflicts exist +- Deferring gate evidence to "post-merge cleanup" +- Treating queued CI runs as equivalent to passing CI +- Approving PRs with unresolved merge conflicts +- Bundling docs-only and runtime-heavy code in one PR without justification +- Ignoring issue-to-PR traceability on architecture phases +- Assuming non-blocking checks are safe without policy confirmation +- Adding dependency upgrades to unblock unrelated CI failures +- Allowing bot PRs to consume reviewer capacity during phase gates +- Using heroic manual coordination instead of explicit checklist gates +- Delaying rollback strategy definition +- Introducing unrequested optimization work + +## Architecture + +### Component Diagram +```text +[Open PR/Issue Inventory] + | + v +[Gate Controller] + - mergeability + - required checks + - scope hygiene + - phase prerequisites + | + +--> [Stabilization Lane: #516 -> #492 -> #489(split/fix)] + | + +--> [Architecture Lane: #521 -> #522 -> #523 -> #524 -> #525 -> #526] + | + +--> [Dependency Lane: batched dependabot windows] + | + v + [Evidence + Issue Updates] +``` + +### Data Flow +```text +GitHub state -> gate evaluation -> remediation action -> CI evidence -> merge/advance decision +``` + +### Key Design Decisions +| Decision | Rationale | Alternatives Rejected | +|----------|-----------|----------------------| +| Start from post-merge `#527` baseline | Reflects current repository truth | Re-planning as if `#527` still open | +| Merge order `#516` then `#492` then `#489` | `#516`/`#492` are mergeable; `#489` is conflicting and scope-mixed | Attempting `#489` first | +| Keep architecture phases strictly sequential (`#521`..`#526`) | Existing epic structure already defines dependency chain | Parallel multi-phase execution | +| Treat `#489` as split-first candidate | Reduces conflict and review ambiguity | Forcing a large mixed-scope merge | +| Defer dependency PR wave until stabilization complete | Protects review and CI bandwidth | Continuous background merges | + +### Eliminated Options (Essentialism) +| Option Rejected | Why Rejected | Risk of Including | +|-----------------|--------------|-------------------| +| Immediate all-PR merge blitz | Violates deterministic gating | High regression/conflict risk | +| Disable checks by deleting workflows | Mixes governance with delivery | Hidden quality regressions | +| Architecture + dependency + feature work simultaneously | Too many moving parts | Low signal, high churn | +| Massive stale-issue cleanup now | Not critical path | Execution drift | +| Broad codebase redesign while PRs pending | Not requested and speculative | Timeline slip | + +### Simplicity Check +> Minimum code/process needed to move portfolio safely: merge ready PRs, split/conflicting PRs, then run architecture phases in order. + +**What if this could be easy?** +Use one checklist gate per PR/phase, no concurrent major streams, and hard stop on unresolved conflicts/failing required checks. + +**Senior Engineer Test**: Pass. This is intentionally procedural, explicit, and non-speculative. + +**Nothing Speculative Checklist**: +- [x] No features not requested +- [x] No "future-proof" abstractions +- [x] No optional flexibility layers +- [x] No impossible-case branches +- [x] No premature optimization + +## File Changes + +### New Files +| File | Purpose | +|------|---------| +| `docs/plans/pr-issues-overall-design-2026-02-13.md` | Phase 2 overall plan from post-`#527` baseline | + +### Modified Files +| File | Changes | +|------|---------| +| None | Design-only artifact creation | + +### Deleted Files +| File | Reason | +|------|--------| +| None | N/A | + +## API Design (Process Contracts) + +### Contract: PR Gate +```text +Input: +- pr_number +- mergeability + merge state +- required check states +- changed-file scope summary + +Output: +- READY | BLOCKED +- blocker list (owner + action + ETA) +``` + +### Contract: Phase Gate +```text +Input: +- issue number (#521..#526) +- prior phase evidence +- CI/lint/test evidence bundle + +Output: +- ADVANCE | HOLD +- missing-evidence checklist +``` + +### Error States +```text +BLOCKED_CONFLICT: mergeable=CONFLICTING +BLOCKED_REQUIRED_CHECKS: any required check not successful +BLOCKED_SCOPE_MIX: PR combines unrelated docs/runtime changes requiring split +BLOCKED_PHASE_PREREQ: previous architecture phase incomplete +``` + +## Test Strategy + +### Unit-Level (Gate Checks via Commands) +| Test | Command | Purpose | +|------|---------|---------| +| PR mergeability | `gh pr view --json mergeable,mergeStateStatus` | Validate merge gate | +| Required checks | `gh pr checks ` | Validate check gate | +| Scope verification | `gh pr view --json files` | Detect mixed/unbounded scope | + +### Integration-Level (Workflow Evidence) +| Test | Evidence | Purpose | +|------|----------|---------| +| Stabilization sequence run | PR timeline + merge commits | Ensure order `#516 -> #492 -> #489` | +| Architecture gate transitions | Issue comments + linked PRs | Verify `#521..#526` sequencing | +| Dependency isolation | Open PR snapshot before/after windows | Confirm controlled batching | + +### Verification Commands by Stage +```bash +# PR stabilization checks +gh pr view 516 --json mergeable,mergeStateStatus,statusCheckRollup +gh pr view 492 --json mergeable,mergeStateStatus,statusCheckRollup +gh pr view 489 --json mergeable,mergeStateStatus,files + +# Architecture progression checks +gh issue view 521 +gh issue view 522 +gh issue view 523 +gh issue view 524 +gh issue view 525 +gh issue view 526 + +# Local quality gates when code changes are made in implementation phase +cargo fmt --check +cargo clippy --workspace --all-targets --all-features +cargo test --workspace +ubs $(git diff --name-only) +``` + +## Implementation Steps + +### Step 1: Close Stabilization PR `#516` +**Description:** Monitor required checks; merge immediately when green. +**Exit Criteria:** `#516` merged into `main`. + +### Step 2: Close Stabilization PR `#492` +**Description:** Keep branch synchronized with `main`, address only required-check failures, merge when green. +**Exit Criteria:** `#492` merged into `main`. + +### Step 3: Normalize PR `#489` (Retitle + Scope Disclosure) +**Description:** Keep single PR, retitle to match actual scope, and require an `Actual Scope` section in PR description listing changed top-level directories before resolving conflicts. +**Exit Criteria:** accurately titled PR with explicit scope disclosure, mergeable and reviewable. + +### Step 4: Execute Architecture P0 Gates (`#521`, `#522`) +**Description:** produce/verify ADR baseline and CI dependency-policy guard before deeper refactors. +**Exit Criteria:** `#521` and `#522` accepted with objective CI/doc evidence. + +### Step 5: Execute Architecture P1-P4 (`#523` -> `#526`) +**Description:** progress phase-by-phase with no skipping and no concurrent phase execution. +**Exit Criteria:** each phase issue has linked implementation evidence and closure criteria met. + +### Step 6: Run Dependency Batch Window +**Description:** process low-risk Dependabot PRs in a controlled window after human/architecture critical path. +**Exit Criteria:** dependency queue reduced without disrupting architecture lane. + +## Rollback Plan + +1. If a stabilization PR fails post-merge checks, revert merge commit and reopen targeted fix PR. +2. If `#489` cannot be normalized quickly, park it and continue with architecture P0/P1 to avoid blocking portfolio flow. +3. If architecture phase evidence is insufficient, hold phase advancement and reopen prior-phase remediation. + +## Dependencies + +### New Dependencies +None in design phase. + +### Dependency Updates +No version decisions in this design phase; handled in dependency batch windows. + +## Performance Considerations + +### Operational Targets +| Metric | Target | Measurement | +|--------|--------|-------------| +| Human PR cycle time (`#516/#492`) | < 1 business day each after green checks | PR timestamps | +| Conflicting PR count in critical path | 0 | `gh pr list` snapshot | +| Architecture phase overlap | 0 simultaneous active phases | issue timeline | + +## Open Items + +| Item | Status | Owner | +|------|--------|-------| +| Implement branch protection update removing required `Warden` check while keeping workflow disabled/removed | Pending execution | Repo admin / maintainer | +| Define exact non-breaking dependency batch cadence (weekly vs biweekly) | Pending | Maintainer | +| Operationalize UBS-driven dependency security triage rule in CI/docs | Pending | Maintainer + implementer | + +## Approval + +- [ ] Technical review complete +- [ ] Test strategy approved +- [ ] Performance targets agreed +- [ ] Human approval received + +--- + +## Specification Interview Findings + +**Interview Date**: 2026-02-13 +**Dimensions Covered**: Failure Modes & Recovery; Security & Governance; Integration Effects; Operational Concerns; Migration & Compatibility; Scope Management +**Convergence Status**: Complete (two rounds, decisions stabilized) + +### Key Decisions from Interview + +#### Concurrency & Race Conditions +- No additional concurrency-specific policy added for this phase; default existing PR/CI serial merge gates remain. + +#### Failure Modes +- Required-check flake handling permits maintainer/repo-admin override when checks are unrelated to changed files. +- Override must be evidence-based, not discretionary. + +#### Edge Cases +- PR `#489` remains single-PR but must be retitled to match actual code scope. +- A mandatory `Actual Scope` description section is required to prevent docs/code mismatch ambiguity. + +#### User Experience +- Reviewers should see explicit scope transparency in PR body before approval. + +#### Scale & Performance +- No new performance constraints introduced during specification interview; existing operational targets remain. + +#### Security & Privacy +- Dependency security prioritization will be driven using UBS scanning as policy signal. +- Non-security dependency changes remain planned and non-breaking. + +#### Integration Effects +- `Warden` is no longer required as a merge check. +- Policy intent is to remove required status and remove/disable the check from merge gating path. + +#### Migration & Compatibility +- Dependency lane allows security patches; non-breaking changes are planned in controlled windows. + +#### Accessibility +- Not applicable for this phase (portfolio/PR governance scope only). + +#### Operational Readiness +- Override authority includes both repo admins and maintainers. +- Evidence expectation: failing CI run reference plus local quality-gate evidence (`fmt/clippy/test`) before override. + +### Deferred Items +- Dependency batch cadence (weekly vs biweekly): deferred pending maintainer scheduling preference. +- Precise UBS-to-policy mapping (what counts as security patch candidate): deferred to CI/doc policy update task. + +### Interview Summary +The interview converted previously open governance choices into actionable rules for implementation. The most significant changes are: keep `#489` as one PR with truthful scope labeling, remove `Warden` as a required merge check, and permit evidence-backed overrides for flaky unrelated required checks. + +Security/dependency policy was narrowed to allow security patches with UBS as the decision tool, while non-breaking dependency updates remain planned in batches. Remaining ambiguity is operational, not architectural: exact dependency cadence and UBS policy encoding need one execution task before Phase 3 starts. diff --git a/docs/plans/pr-issues-research-2026-02-13.md b/docs/plans/pr-issues-research-2026-02-13.md new file mode 100644 index 000000000..cad0a0830 --- /dev/null +++ b/docs/plans/pr-issues-research-2026-02-13.md @@ -0,0 +1,109 @@ +# Disciplined Research: Current PR and Issue Portfolio + +- Date: 2026-02-13 +- Repository: terraphim/terraphim-ai +- Inputs reviewed: open PRs, open issues, architecture epics/issues (#520-#526), active feature issue (#519), and referenced architecture docs +- Method: portfolio triage focused on dependency, sequencing, and delivery risk + +## 1. Problem Statement + +The repository currently has a large mixed backlog of open pull requests and issues spanning architecture refactor phases, feature work (TinyClaw), and dependency update churn. Without an explicit integration plan, there is high risk of merge conflicts, context switching, and stalled architecture outcomes. + +## 2. Current-State Inventory (as of 2026-02-13) + +### Pull Requests +- Open PRs: 26 +- Human-authored PRs: 8 +- Dependabot PRs: 18 +- PRs updated within 7 days: 9 +- PRs stale over 30 days: 0 + +### Issues +- Open issues: 85 +- Architecture-labeled open issues: 12 +- Issues updated within 7 days: 8 +- Issues stale over 60 days: 59 +- Bug-labeled open issues: 2 + +### High-Signal Active Workstreams +1. Architecture modernization epic: #520 with phased breakdown #521-#526 +2. TinyClaw implementation: #519, paired with active PR #527 +3. Integration testing for agent mode consistency: PR #516 +4. Continuous dependency update stream (18 Dependabot PRs) + +## 3. Key Findings + +### F1. Architecture sequencing is defined but not yet execution-hardened +Issue #520 provides clear phase order (P0..P4) and measurable acceptance criteria. This is positive, but no evidence yet that all guardrails are active in CI (notably policy enforcement from #522 and baseline measurement from #521). + +### F2. Active feature branch has merge friction +PR #527 (`claude/tinyclaw-terraphim-plan-lIt3V`) is currently in `DIRTY` merge state, signaling rebase/merge-conflict pressure if architecture phases land first or in parallel without coordination. + +### F3. Integration testing PR exists but appears disconnected from architecture lane +PR #516 adds meaningful cross-mode tests, but there is no explicit visible linkage to architecture phase acceptance gates (#523/#524) despite overlapping concerns (route parity, mode consistency, service decomposition). + +### F4. Portfolio mix creates throughput drag +A high ratio of bot PRs (18/26) competes with human feature/architecture work for review bandwidth. Unbatched dependency updates increase noise and can mask high-value structural changes. + +### F5. Backlog aging suggests weak closure loop +59 issues are stale for over 60 days. This creates planning ambiguity and raises the chance of duplicated effort or reviving outdated direction. + +## 4. Dependency and Constraint Map + +### Confirmed Dependencies +1. #521 (ADRs/baseline) and #522 (CI dependency policy) are prerequisites for reliable measurement of #523-#526 outcomes. +2. #523 (bootstrap + route unification) should precede #524 (service decomposition) to reduce refactor churn and test drift. +3. #525 (dependency isolation) impacts build profiles and should gate broad rollout of new runtime-heavy features from #519. +4. #526 (CLI product-line consolidation) should incorporate outcomes from #516 (cross-mode consistency tests) as regression contracts. + +### Operational Constraints +- Any plan must preserve release flow while reducing dependency drift. +- Existing docs (`docs/architecture-review-report.md`, `docs/architecture-improvement-plan.md`) already define expected architecture direction; new work should align, not fork. +- Merge conflict risk is elevated for PR #527 unless branch synchronization cadence is enforced. + +## 5. Risk Register + +1. Risk: Architecture phase slippage due to parallel feature pressure from #519/#527. +- Likelihood: High +- Impact: High +- Mitigation: Freeze large feature merges into architecture-sensitive crates until P0/P1 merge. + +2. Risk: CI policy gap allows dependency drift despite issue #522. +- Likelihood: Medium +- Impact: High +- Mitigation: Implement and enforce dependency-policy check before Phase 1+ architecture merges. + +3. Risk: Test suite does not track structural architecture acceptance criteria. +- Likelihood: Medium +- Impact: Medium +- Mitigation: Map PR #516 tests to specific phase acceptance criteria and add missing tests where needed. + +4. Risk: Review bandwidth dilution from bot PR volume. +- Likelihood: High +- Impact: Medium +- Mitigation: Batch/queue dependency PRs and prioritize architecture/feature-critical PRs first. + +5. Risk: Stale issue backlog obscures current priorities. +- Likelihood: High +- Impact: Medium +- Mitigation: Run stale triage pass and close/archive non-actionable legacy items. + +## 6. Unknowns Requiring Design Decisions + +1. PR integration strategy for #527 relative to #523/#524 (rebase-after-each-phase vs temporary freeze). +2. Dependency PR policy during architecture phases (auto-merge low risk vs deferred batch windows). +3. Acceptance gate definition tying issue milestones to objective CI evidence. +4. Ownership model for architecture phases vs TinyClaw delivery to avoid context fragmentation. + +## 7. Research Conclusion + +The repository has a workable strategic architecture plan already defined (issues #520-#526 + architecture docs), but execution risk is primarily coordination and sequencing, not missing technical direction. The most leverage comes from enforcing phase prerequisites (P0), reducing merge entropy, and binding tests to phase acceptance criteria. + +## 8. Recommended Next Step (Phase 2 Input) + +Proceed to disciplined design with a portfolio execution design that defines: +1. Lane-based sequencing (Architecture lane, Feature lane, Dependency lane) +2. PR/issue gating rules by phase +3. Evidence matrix (what CI/test evidence is required for each phase) +4. Week-by-week integration cadence with merge conflict controls + diff --git a/docs/plans/terraphim-agent-tui-offline-server-design-2026-02-13.md b/docs/plans/terraphim-agent-tui-offline-server-design-2026-02-13.md new file mode 100644 index 000000000..6d8ad36bd --- /dev/null +++ b/docs/plans/terraphim-agent-tui-offline-server-design-2026-02-13.md @@ -0,0 +1,230 @@ +# Implementation Plan: Clarify `terraphim-agent` TUI Offline/Server Requirement (`terraphim-ai-cbm`) + +**Status**: Draft +**Research Doc**: `docs/plans/terraphim-agent-tui-offline-server-research-2026-02-13.md` +**Author**: Codex +**Date**: 2026-02-13 +**Estimated Effort**: 0.5-1.0 day + +## Overview + +### Summary + +This plan makes runtime behavior explicit and predictable without changing core architecture: fullscreen interactive TUI remains server-backed, while REPL/offline subcommands remain offline-capable. The implementation focuses on clarity in user messaging, docs alignment, and tests that prevent behavior drift. + +### Approach + +1. Define a single mode contract and reflect it consistently in CLI help and documentation. +2. Add explicit, actionable failure messaging when fullscreen TUI cannot reach server. +3. Backstop with tests for mode-specific behavior and messaging. + +### Scope + +**In Scope:** +- Clarify mode contract in help text and docs. +- Improve fullscreen TUI startup behavior/messaging for missing server. +- Add tests that validate the contract and avoid regressions. + +**Out of Scope:** +- Full offline rewrite of fullscreen ratatui TUI. +- `terraphim-cli` device settings fallback (issue `terraphim-ai-2sz`). +- Feature-parity redesign between `terraphim-agent` and `terraphim-cli` (issue `terraphim-ai-tcw`). + +**Avoid At All Cost**: +- Ambiguous terms where "TUI" and "REPL" are interchangeable. +- Broad architecture refactors hidden inside this clarification task. +- Breaking default automation/subcommand behavior. + +## Architecture + +### Component Diagram +``` +CLI Entry (main.rs) + | + +-- no args / interactive --> Fullscreen TUI (ratatui) --> ApiClient --> Server endpoints + | + +-- repl ------------------> ReplHandler::new_offline/new_server + | | | + | | +--> ApiClient + | +--> TuiService (embedded/local) + | + +-- subcommands -----------> run_offline_command (TuiService) OR run_server_command (ApiClient) +``` + +### Data Flow +``` +User input -> mode routing -> [ApiClient path OR TuiService path] -> output/messaging +``` + +### Key Design Decisions + +| Decision | Rationale | Alternatives Rejected | +|----------|-----------|----------------------| +| Keep fullscreen interactive TUI server-backed | Matches current implementation and minimizes risk | Full offline fullscreen rewrite | +| Keep REPL/subcommands offline-capable | Existing behavior and docs promise local-first operation | Force everything through server | +| Add explicit startup guidance for server-unreachable fullscreen TUI | Faster operator diagnosis, less confusion | Silent/implicit failure behavior | +| Normalize wording across help/docs | Root-cause fix for ambiguity | Patch one doc only | + +### Eliminated Options (Essentialism) + +| Option Rejected | Why Rejected | Risk of Including | +|-----------------|--------------|-------------------| +| Implement offline data provider in fullscreen TUI now | Too large for issue scope | Schedule slip, regression risk | +| Change no-arg behavior to REPL by default in same patch | Potentially breaking UX contract without sign-off | User-facing behavior break | +| Introduce new abstraction layer for all modes | Speculative complexity | Harder maintenance | + +### Simplicity Check + +The simplest change that solves the problem is contract clarity, not runtime redesign. + +**Senior Engineer Test**: Passes. +**Nothing Speculative Checklist**: +- [x] No features user did not request +- [x] No "future-proof" abstractions +- [x] No speculative flexibility layers +- [x] No premature optimization + +## File Changes + +### New Files + +| File | Purpose | +|------|---------| +| `docs/plans/terraphim-agent-tui-offline-server-design-2026-02-13.md` | Phase 2 design plan | + +### Modified Files + +| File | Changes | +|------|---------| +| `crates/terraphim_agent/src/main.rs` | Clarify no-arg interactive mode wording; add actionable server-requirement messaging for fullscreen TUI path | +| `docs/tui-usage.md` | Add explicit mode matrix (fullscreen TUI vs REPL vs subcommands) | +| `docs/src/tui.md` | Correct "Interactive REPL Mode" wording and align usage examples with real routing | +| `README.md` | Ensure usage snippets do not imply fullscreen TUI is offline-capable | +| `crates/terraphim_agent/tests/*` | Add/update tests for mode-specific behavior and messaging | + +### Deleted Files + +| File | Reason | +|------|--------| +| None | N/A | + +## API Design + +### Internal Functions (proposed) + +```rust +/// Resolve server URL for fullscreen TUI mode. +fn resolve_tui_server_url(explicit: Option<&str>) -> String; + +/// Preflight server availability for fullscreen TUI and return actionable error. +async fn ensure_tui_server_reachable(api: &ApiClient) -> anyhow::Result<()>; + +/// Build user-facing guidance when fullscreen TUI cannot reach server. +fn tui_server_requirement_error(url: &str, cause: &anyhow::Error) -> anyhow::Error; +``` + +### Error States + +```text +TUI_SERVER_UNREACHABLE: + Fullscreen TUI cannot reach server URL. + Message includes: + - URL attempted + - suggestion: start server OR use `terraphim-agent repl` for offline mode +``` + +## Test Strategy + +### Unit Tests + +| Test | Location | Purpose | +|------|----------|---------| +| `resolve_tui_server_url_uses_explicit_then_env_then_default` | `crates/terraphim_agent/src/main.rs` (or extracted module) | Validate deterministic URL selection | +| `tui_server_requirement_error_mentions_repl_fallback` | `crates/terraphim_agent/src/main.rs` (or extracted module) | Ensure actionable operator guidance | + +### Integration Tests + +| Test | Location | Purpose | +|------|----------|---------| +| `offline_subcommand_config_show_still_works_without_server` | `crates/terraphim_agent/tests/offline_mode_tests.rs` | Preserve offline command contract | +| `server_mode_config_show_fails_cleanly_when_server_down` | `crates/terraphim_agent/tests/server_mode_tests.rs` (or new targeted test) | Validate failure path messaging | +| `help_text_describes_mode_contract` | `crates/terraphim_agent/tests/integration_tests.rs` (or new) | Prevent future ambiguity | + +### Manual Acceptance Checks + +1. `terraphim-agent config show` works with no server running. +2. `terraphim-agent --server config show` fails with clear server connection error. +3. `terraphim-agent` no-arg usage/help text clearly distinguishes fullscreen TUI vs `repl`. +4. Docs contain a single consistent mode contract. + +## Implementation Steps + +### Step 1: Contract Wording Baseline +**Files:** `crates/terraphim_agent/src/main.rs`, `README.md`, `docs/tui-usage.md`, `docs/src/tui.md` +**Description:** Define and apply one authoritative mode matrix and terminology (`Fullscreen TUI`, `REPL`, `CLI subcommands`). +**Tests:** Help output checks. +**Estimated:** 1.5 hours. + +### Step 2: Fullscreen TUI Startup Messaging +**Files:** `crates/terraphim_agent/src/main.rs` +**Description:** Add preflight/failure guidance so missing server is explicit and points to `repl` fallback. +**Tests:** Unit tests for URL resolution and error message composition. +**Dependencies:** Step 1. +**Estimated:** 2 hours. + +### Step 3: Regression Tests +**Files:** `crates/terraphim_agent/tests/offline_mode_tests.rs`, `crates/terraphim_agent/tests/server_mode_tests.rs`, optional new test file +**Description:** Lock expected behavior for offline subcommands and server-backed paths. +**Tests:** `cargo test -p terraphim_agent offline_mode_tests server_mode_tests`. +**Dependencies:** Step 2. +**Estimated:** 2 hours. + +### Step 4: Final Documentation Sync +**Files:** `README.md`, `docs/tui-usage.md`, `docs/src/tui.md` +**Description:** Ensure examples and requirements reflect final runtime behavior and fallback guidance. +**Tests:** Markdown lint/docs build if available. +**Dependencies:** Step 3. +**Estimated:** 1 hour. + +## Rollback Plan + +If issues are found: +1. Revert messaging/preflight additions in `main.rs`. +2. Keep documentation-only clarifications while restoring prior runtime behavior. +3. Reopen follow-up issue for architectural decisions beyond clarification scope. + +## Dependencies + +### New Dependencies + +None planned. + +### Dependency Updates + +None planned. + +## Performance Considerations + +### Expected Performance + +| Metric | Target | Measurement | +|--------|--------|-------------| +| Offline subcommand startup | No regression | Existing test execution time comparison | +| Fullscreen TUI startup | Negligible added overhead | Optional preflight (single config call) | + +### Benchmarks to Add + +None required for this change class. + +## Open Items + +| Item | Status | Owner | +|------|--------|-------| +| Decide whether no-arg should remain fullscreen TUI or switch to REPL default in future | Pending approval | Maintainers | +| Confirm whether fullscreen TUI server dependency is long-term product direction | Pending | Maintainers | + +## Approval + +- [ ] Technical review complete +- [ ] Test strategy approved +- [ ] Human approval received diff --git a/docs/plans/terraphim-agent-tui-offline-server-research-2026-02-13.md b/docs/plans/terraphim-agent-tui-offline-server-research-2026-02-13.md new file mode 100644 index 000000000..e4a483099 --- /dev/null +++ b/docs/plans/terraphim-agent-tui-offline-server-research-2026-02-13.md @@ -0,0 +1,218 @@ +# Research Document: Clarify `terraphim-agent` TUI Offline/Server Requirement (`terraphim-ai-cbm`) + +**Status**: Draft +**Author**: Codex +**Date**: 2026-02-13 +**Reviewers**: AlexMikhalev + +## Executive Summary + +`terraphim-agent` currently has mixed runtime contracts: CLI subcommands and REPL offline mode run through embedded local services, while fullscreen interactive TUI uses HTTP API calls to a server endpoint. Documentation and help text mix these models, which creates user confusion about whether "TUI" is offline-capable. +Recommendation: keep current architecture (server-backed fullscreen TUI, offline-capable REPL/subcommands), but make the contract explicit in code-facing UX and docs, then add tests that lock the behavior. + +## Essential Questions Check + +| Question | Answer | Evidence | +|----------|--------|----------| +| Energizing? | Yes | This removes recurring operator confusion and support churn in a core UX path. | +| Leverages strengths? | Yes | The codebase already has both code paths (`ApiClient` and `TuiService`), so clarification is high leverage and low-risk. | +| Meets real need? | Yes | Ready issue `terraphim-ai-cbm` explicitly requests requirement clarification and behavior adjustment if needed. | + +**Proceed**: Yes (3/3 Yes) + +## Problem Statement + +### Description + +Clarify whether `terraphim-agent` interactive usage requires a running server, supports fully offline operation, or both depending on mode. + +### Impact + +- Users running `terraphim-agent` with no args can assume offline support from top-level messaging, but interactive path is server-backed. +- Documentation currently contains contradictory framing ("comprehensive REPL" vs "requires server"), increasing setup friction. +- Inconsistent expectations reduce trust in automation docs and operator workflows. + +### Success Criteria + +1. Runtime contract is explicit and consistent across code help text and docs. +2. Users can quickly pick correct mode: + - Fullscreen interactive TUI (server-backed) + - REPL and CLI subcommands (offline-capable default) +3. Tests enforce the contract to prevent drift. + +## Current State Analysis + +### Existing Implementation + +- `main` routes no-arg/`interactive` invocation to `run_tui_*` path (`crates/terraphim_agent/src/main.rs:587`). +- `run_tui_offline_mode` comment states TUI requires a running server and points users to REPL for offline (`crates/terraphim_agent/src/main.rs:629`). +- `ui_loop` always builds `ApiClient` from URL/environment default (`crates/terraphim_agent/src/main.rs:1898`). +- Offline CLI subcommands instantiate `TuiService::new()` and use embedded config/settings path (`crates/terraphim_agent/src/main.rs:640`, `crates/terraphim_agent/src/service.rs:17`). +- REPL has explicit offline/server constructors and mode banner (`crates/terraphim_agent/src/repl/handler.rs:198`, `crates/terraphim_agent/src/repl/handler.rs:2367`). + +### Code Locations + +| Component | Location | Purpose | +|-----------|----------|---------| +| CLI dispatch and interactive entry | `crates/terraphim_agent/src/main.rs` | Selects interactive TUI vs offline/server command paths | +| Local embedded service | `crates/terraphim_agent/src/service.rs` | Offline-capable config/search/role behavior | +| REPL runtime mode split | `crates/terraphim_agent/src/repl/handler.rs` | Distinct offline and server REPL modes | +| User-facing TUI guide | `docs/tui-usage.md` | States server requirements for TUI | +| Doc-site TUI page | `docs/src/tui.md` | Currently describes `terraphim-agent` as REPL-centric and server endpoint usage | + +### Data Flow + +1. **No args / `interactive`**: `main` -> `run_tui` -> `ui_loop` -> `ApiClient` HTTP calls (`/config`, `/search`, `/rolegraph`, etc.). +2. **Offline subcommands**: `main` -> `run_offline_command` -> `TuiService::new` -> local config/service path. +3. **REPL offline**: `repl::run_repl_offline_mode` -> `TuiService`. +4. **REPL server**: `repl::run_repl_server_mode` -> `ApiClient`. + +### Integration Points + +- Server endpoints used by fullscreen TUI and server-mode commands: `/config`, `/documents/search`, `/rolegraph`, `/chat`, `/documents/summarize`. +- Local/offline path uses embedded config + local persistence via `TuiService`. + +## Constraints + +### Technical Constraints + +- Fullscreen TUI event loop is currently tightly coupled to `ApiClient` operations. +- Existing offline functionality is already implemented for subcommands and REPL; duplicating full TUI logic onto `TuiService` would be non-trivial. +- Feature flags (`repl`, `repl-interactive`, `repl-full`) gate portions of behavior (`crates/terraphim_agent/Cargo.toml`). + +### Business Constraints + +- Need fast clarification and low regression risk (issue is a P2 task). +- Must preserve existing user workflows and scripts. + +### Non-Functional Requirements + +| Requirement | Target | Current | +|-------------|--------|---------| +| Contract clarity | Single consistent mode statement in help/docs | Inconsistent wording across docs/help | +| Backward compatibility | No breaking change to core subcommand behavior | Mostly stable today | +| Operator guidance | Fast diagnosis when server is missing | Weak for no-arg interactive expectations | + +## Vital Few (Essentialism) + +### Essential Constraints (Max 3) + +| Constraint | Why It's Vital | Evidence | +|------------|----------------|----------| +| Preserve offline subcommand/REPL behavior | Automation and local workflows depend on it | `run_offline_command` and REPL offline path are active today | +| Avoid fullscreen TUI rewrite in this issue | Large scope, high risk, low immediacy | Current loop depends on API client calls throughout | +| Remove ambiguity from docs/help/runtime messages | Root cause of reported confusion | Mixed statements in `show_usage_info`, `docs/tui-usage.md`, `docs/src/tui.md` | + +### Eliminated from Scope + +| Eliminated Item | Why Eliminated | +|-----------------|----------------| +| Re-architect fullscreen TUI to run fully offline | Not required to clarify contract; high complexity | +| Feature-parity redesign between `terraphim-agent` and `terraphim-cli` | Separate ready issue (`terraphim-ai-tcw`) | +| Device settings fallback work for `terraphim-cli` | Separate ready issue (`terraphim-ai-2sz`) | + +## Dependencies + +### Internal Dependencies + +| Dependency | Impact | Risk | +|------------|--------|------| +| `terraphim_agent::main` dispatch logic | Defines user mode behavior | High | +| `TuiService` embedded defaults path | Enables offline operation | Medium | +| Docs (`docs/tui-usage.md`, `docs/src/tui.md`, README snippets) | Controls user expectation | High | + +### External Dependencies + +| Dependency | Version | Risk | Alternative | +|------------|---------|------|-------------| +| Terraphim server endpoint availability | N/A (runtime service) | Medium | Offline REPL/subcommands | + +## Risks and Unknowns + +### Known Risks + +| Risk | Likelihood | Impact | Mitigation | +|------|------------|--------|------------| +| Users interpret "TUI" differently (fullscreen UI vs REPL umbrella) | High | High | Publish explicit mode matrix and examples | +| Contract changes introduce accidental behavior regressions | Medium | Medium | Add focused tests for mode routing and messaging | +| Documentation drift reappears | Medium | Medium | Link docs to tests/checklist in implementation plan | + +### Open Questions + +1. Should no-arg `terraphim-agent` remain fullscreen TUI, or default to offline REPL? +2. Is fullscreen TUI intentionally server-only long-term, or temporary architecture? +3. Should missing-server detection happen pre-loop with explicit actionable error? + +### Assumptions Explicitly Stated + +| Assumption | Basis | Risk if Wrong | Verified? | +|------------|-------|---------------|-----------| +| Fullscreen interactive TUI is intended to be server-backed | `ui_loop` uses `ApiClient` only; comment in `run_tui_offline_mode` | Plan may reinforce a temporary behavior | Partially | +| Offline support requirement applies to REPL/subcommands, not fullscreen TUI | Existing code already supports this split | Could conflict with product direction | No | +| Fast clarification is preferred over architectural rewrite for this issue | P2 issue scope and wording ("document requirement and adjust behavior if needed") | Might under-deliver if stakeholders expect full offline TUI | Partially | + +### Multiple Interpretations Considered + +| Interpretation | Implications | Why Chosen/Rejected | +|----------------|--------------|---------------------| +| A: "TUI" means fullscreen ratatui interface only | Server-backed requirement can remain; focus on clarity | Chosen as default interpretation based on current code path | +| B: "TUI" includes REPL and all interactive CLI | Product can claim offline capability if modes are distinguished clearly | Also valid; should be reflected in docs as mode matrix | +| C: Fullscreen TUI must be fully offline by default | Requires significant refactor and broader test surface | Rejected for this issue scope; candidate follow-up issue | + +## Research Findings + +### Key Insights + +1. Behavior is not uniformly server-dependent or offline; it is mode-dependent. +2. Current naming (`run_tui_offline_mode`) and help/docs text are the main confusion source. +3. Lowest-risk resolution is contract clarity + targeted UX messaging + regression tests. + +### Relevant Prior Art + +- Existing offline tests (`crates/terraphim_agent/tests/offline_mode_tests.rs`) already validate local command operation without server. +- Existing server-mode tests (`crates/terraphim_agent/tests/server_mode_tests.rs`) validate API-backed paths. + +### Technical Spikes Needed + +| Spike | Purpose | Estimated Effort | +|-------|---------|------------------| +| None mandatory | Existing code paths are sufficient for this issue | 0 | + +## Recommendations + +### Proceed/No-Proceed + +Proceed. + +### Scope Recommendations + +1. Define and document a clear mode contract: + - Fullscreen interactive TUI: server-backed. + - REPL (`terraphim-agent repl`) and offline subcommands: offline-capable. +2. Replace ambiguous/help text that suggests no-arg interactive may be either REPL or TUI. +3. Add actionable error/messaging when server is unavailable for fullscreen TUI path. + +### Risk Mitigation Recommendations + +1. Add tests that verify mode-specific messaging and behavior. +2. Keep behavior changes minimal; do not redesign runtime architecture in this issue. +3. Capture deferred architectural direction (full offline fullscreen TUI) as a follow-up if desired. + +## Next Steps + +If approved: +1. Produce Phase 2 implementation plan with concrete file-level changes. +2. Resolve open questions around default no-arg behavior before Phase 3. +3. Run disciplined quality evaluation on research/design docs before implementation. + +## Appendix + +### Reference Materials + +- `crates/terraphim_agent/src/main.rs:587` +- `crates/terraphim_agent/src/main.rs:629` +- `crates/terraphim_agent/src/main.rs:1898` +- `crates/terraphim_agent/src/service.rs:17` +- `crates/terraphim_agent/src/repl/handler.rs:198` +- `docs/tui-usage.md:409` +- `docs/src/tui.md:37` diff --git a/docs/src/tui.md b/docs/src/tui.md index b220632b9..0234e43c6 100644 --- a/docs/src/tui.md +++ b/docs/src/tui.md @@ -1,6 +1,9 @@ # Terraphim TUI -Terraphim includes a comprehensive terminal user interface (TUI) that provides both interactive REPL functionality and CLI commands for advanced operations including VM management, web operations, and intelligent file operations with semantic awareness. +Terraphim includes a terminal interface with three execution modes: +- Fullscreen TUI (`terraphim-agent`) - server-backed interactive UI +- REPL (`terraphim-agent repl`) - offline-capable by default +- CLI subcommands (`terraphim-agent `) - offline-capable by default unless `--server` is used ## Installation @@ -34,13 +37,19 @@ export TERRAPHIM_SERVER=http://localhost:8000 - `repl-sessions` - AI coding session history search (Claude Code, Cursor, Aider) - `repl-full` - All features enabled (recommended) -## Interactive REPL Mode +## Interactive Modes ```bash +# Fullscreen TUI (server-backed) terraphim-agent + +# REPL (offline-capable) +terraphim-agent repl ``` -The TUI provides a comprehensive REPL (Read-Eval-Print Loop) with access to all features: +Use `terraphim-agent repl --server --server-url http://localhost:8000` for server-backed REPL. + +REPL mode provides access to all slash commands below: **Navigation and Help:** - `/help` - Show all available commands @@ -125,6 +134,9 @@ Traditional CLI commands are also supported: ## Behavior +- `terraphim-agent` (no args) runs fullscreen TUI and requires a reachable server. +- `terraphim-agent repl` runs REPL offline by default (or server mode with `--server`). +- CLI subcommands (for example `search`, `config`, `roles`) run offline-capable mode by default. - Uses `/config`, `/config/selected_role`, `/documents/search`, and `/rolegraph` endpoints. - Chat posts to `/chat` (requires server compiled with openrouter feature and configured role or `OPENROUTER_KEY`). - VM operations require Firecracker integration and proper system permissions. diff --git a/docs/tui-usage.md b/docs/tui-usage.md index 9514b66f1..840bda8a8 100644 --- a/docs/tui-usage.md +++ b/docs/tui-usage.md @@ -1,12 +1,17 @@ # Terraphim Terminal User Interface (TUI) -Terraphim includes a comprehensive terminal user interface (TUI) that provides both interactive REPL functionality and CLI commands for advanced operations including VM management, web operations, and intelligent file operations with semantic awareness. The TUI is ideal for CI/CD environments, headless servers, development workflows, and integration with terminal-based automation. +Terraphim includes a terminal interface with three execution modes: +- Fullscreen TUI (`terraphim-agent`) - server-backed interactive UI +- REPL (`terraphim-agent repl`) - offline-capable by default +- CLI subcommands (`terraphim-agent `) - offline-capable by default unless `--server` is used + +These modes support VM management, web operations, and intelligent file operations with semantic awareness. ## Installation ### Prerequisites - Rust toolchain (cargo) -- Running Terraphim server (local or remote) +- Running Terraphim server (required for fullscreen TUI and `--server` mode) ### Building from Source @@ -53,19 +58,27 @@ Set the Terraphim server URL (defaults to `http://localhost:8000`): export TERRAPHIM_SERVER=http://localhost:8000 ``` -This environment variable is **required** for the TUI to connect to the server. If not set, the default will be used. +This environment variable is used by fullscreen TUI and `--server` mode. REPL and CLI subcommands in default mode can run offline. ## Command Reference -### Interactive REPL Mode - -The TUI features a comprehensive REPL (Read-Eval-Print Loop) that provides access to all advanced functionality: +### Fullscreen TUI Mode (Server-Backed) ```bash terraphim-agent ``` -In interactive mode, you have access to: +Use this mode when you want the fullscreen terminal UI and a running Terraphim server. + +### REPL Mode (Offline-Capable) + +```bash +terraphim-agent repl +``` + +Use `terraphim-agent repl --server --server-url http://localhost:8000` to run REPL in server mode. + +In REPL mode, you have access to: - **Smart Search**: Type queries and get intelligent suggestions from the rolegraph - **VM Management**: Control Firecracker VMs for isolated operations - **Web Operations**: Perform web requests through secure VM sandboxing @@ -359,7 +372,7 @@ Intelligent file operations with semantic understanding and analysis: | Variable | Required | Default | Description | |----------|----------|---------|-------------| -| `TERRAPHIM_SERVER` | Yes | `http://localhost:8000` | URL of the Terraphim server | +| `TERRAPHIM_SERVER` | Mode-dependent | `http://localhost:8000` | Server URL used by fullscreen TUI and `--server` mode | | `OPENROUTER_KEY` | No | None | OpenRouter API key for chat functionality | | `OLLAMA_BASE_URL` | No | `http://127.0.0.1:11434` | Ollama server URL for local models | | `VM_STORAGE_PATH` | No | `./vm-storage` | Directory for VM disk images | @@ -406,7 +419,7 @@ export MCP_AUTH_TOKEN=your-mcp-token ### Server Requirements -The TUI requires a running Terraphim server with the following endpoints: +Fullscreen TUI mode and `--server` mode require a running Terraphim server with the following endpoints: - `/config` - Configuration management - `/config/selected_role` - Role selection - `/documents/search` - Document search