feat(memory): temporal facts, knowledge history, cmd timeout + cancel fix

.github/workflows/opencode-review.yml

@@ -0,0 +1,26 @@
+name: opencode-review
+
+on:
+  pull_request:
+    types: [opened, synchronize, reopened, ready_for_review]
+
+jobs:
+  review:
+    runs-on: ubuntu-latest
+    permissions:
+      id-token: write
+      contents: read
+      pull-requests: write
+      issues: write
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+
+      - name: Run opencode review
+        uses: anomalyco/opencode/github@latest
+        env:
+          OPENCODE_API_KEY: ${{ secrets.OPENCODE_API_KEY }}
+        with:
+          model: opencode-go/qwen3.8-flash

.github/workflows/opencode.yml

@@ -0,0 +1,33 @@
+name: opencode
+
+on:
+  issue_comment:
+    types: [created]
+  pull_request_review_comment:
+    types: [created]
+
+jobs:
+  opencode:
+    if: |
+      contains(github.event.comment.body, ' /oc') ||
+      startsWith(github.event.comment.body, '/oc') ||
+      contains(github.event.comment.body, ' /opencode') ||
+      startsWith(github.event.comment.body, '/opencode')
+    runs-on: ubuntu-latest
+    permissions:
+      id-token: write
+      contents: read
+      pull-requests: write
+      issues: write
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+
+      - name: Run opencode
+        uses: anomalyco/opencode/github@latest
+        env:
+          OPENCODE_API_KEY: ${{ secrets.OPENCODE_API_KEY }}
+        with:
+          model: opencode-go/qwen3.8-flash

CLAUDE.md

@@ -141,11 +141,7 @@ All jobs use `dtolnay/rust-toolchain@stable` and `Swatinem/rust-cache@v2` for ca
 
 ## Testing
 
-No automated tests exist yet. When adding tests:
-
-- Place unit tests in `#[cfg(test)] mod tests` blocks within each source file
-- Integration tests go in a top-level `tests/` directory
-- The sandbox path validation logic in `tools.rs` and message splitting in `bot.rs` are good candidates for unit tests
+Unit tests live in `#[cfg(test)] mod tests` within source files; integration tests in top-level `tests/`. Run `cargo test` (and `cargo clippy -- -D warnings`) before opening a PR. Memory knowledge/history/facts and supervisor backends already have unit coverage — extend those patterns when adding behavior.
 
 ## Common Tasks
 

CONTEXT.md

@@ -30,4 +30,24 @@ Entry in Tool Notifier showing: friendly tool name, optional args preview, statu
 Human-readable label for built-in tools (e.g., "💻 Running a command" for `execute_command`).
 
 ### Args Preview
-Truncated (60 chars), redacted JSON args shown in Tool Notifier. Only in Verbose mode.
+Truncated (60 chars), redacted JSON args shown in Verbose mode.
+
+---
+
+## Memory
+
+### Knowledge
+Current key-value snapshot the agent stores under `(category, key) → value`. One live value per pair.
+_Avoid_: fact (when meaning KV), memory entry, note
+
+### Knowledge History
+Append-only archive of prior Knowledge values, written automatically on overwrite or delete.
+_Avoid_: audit log, version log (generic)
+
+### Fact
+Time-bounded triple `(entity, relation, value)` with `valid_from` / `valid_to`. At most one active value per `(entity, relation)` (`valid_to` null = still active).
+_Avoid_: subject/object_value, triple, statement, knowledge (KV sense)
+
+### valid_from / valid_to
+Inclusive start and exclusive-or-end bound of a Fact's validity window. `valid_to` null means still active. Stored as SQLite `datetime` TEXT.
+_Avoid_: valid_until, effective_from, learned_at (provenance, not validity)

config.example.toml

@@ -68,6 +68,8 @@ Be concise and helpful."""
 # The LLM's persistent workspace (file/command tools are confined here).
 # Leave unset to use <home>/workspace. Set an absolute path to override.
 # allowed_directory = "/absolute/path/to/workspace"
+# Wall-clock limit for execute_command (seconds). 0 = no timeout. Default: 90.
+# execute_timeout_secs = 90
 
 [memory]
 # Path to the SQLite database file for persistent memory

docs/adr/0004-temporal-facts-and-knowledge-history.md

@@ -0,0 +1,3 @@
+# Temporal Facts and Knowledge History
+
+Memory gains two layers: Knowledge History (auto-archive on `remember`/`forget` via SQLite triggers) and Facts (valid-time triples with `valid_from`/`valid_to`; `created_at` is audit only). Knowledge stays the live KV snapshot. Schema uses `entity`/`relation`/`value`/`valid_from`/`valid_to`. One active Fact per `(entity, relation)` (UNIQUE partial index); superseding values close the prior row; backdated inserts close against the active start without inverting windows. Timestamps normalize to UTC `YYYY-MM-DD HH:MM:SS` for compare with SQLite `datetime('now')`. `knowledge_as_of` reverse-applies history from the live row — a delete→recreate gap is not modeled as absence (triggers only fire on overwrite/delete). No agent-loop changes in this phase — data layer + MCP tools only. Agentic RAG and harness evolution deferred to separate issues.

docs/adr/0005-callback-query-not-serialized-per-chat.md

@@ -0,0 +1,3 @@
+# Callback queries bypass per-chat serialization
+
+Teloxide `distribution_function` keyed non-command updates by chat id so messages stay ordered. Callback queries (Cancel button) shared that key, so cancel handlers queued behind the in-flight message handler stuck in `execute_command` — Cancel appeared dead. Decision: return `None` for `CallbackQuery` (and keep `/` commands concurrent) so cancel runs while the tool is still executing. Message ordering unchanged.

src/command_tool.rs

@@ -3,8 +3,7 @@ use async_trait::async_trait;
 use serde_json::{json, Value};
 use std::path::PathBuf;
 use std::sync::Arc;
-use std::time::Instant;
-use tokio::io::AsyncReadExt;
+use std::time::{Duration, Instant};
 use tokio::process::Command as TokioCommand;
 use tracing::warn;
 
@@ -13,32 +12,31 @@ use crate::llm::{FunctionDefinition, ToolDefinition};
 use crate::platform::sender::PlatformSender;
 use crate::tool_registry::{ToolContext, ToolHandler, ToolResult, ToolUiMode};
 
-/// Controls how command execution messages are sent to Telegram.
 enum SendMode {
-    /// Full live output with cancel button.
     Verbose,
-    /// Cancel button only, no live edits. Message deleted on completion.
     Minimal,
-    /// No message sent. Tool notifier handles nothing (silent mode).
     Silent,
 }
 
 pub struct CommandTool {
     sandbox_dir: PathBuf,
     cancel_registry: Arc<CancelRegistry>,
     sender: Arc<dyn PlatformSender>,
+    execute_timeout_secs: u64,
 }
 
 impl CommandTool {
     pub fn new(
         sandbox_dir: PathBuf,
         cancel_registry: Arc<CancelRegistry>,
         sender: Arc<dyn PlatformSender>,
+        execute_timeout_secs: u64,
     ) -> Self {
         Self {
             sandbox_dir,
             cancel_registry,
             sender,
+            execute_timeout_secs,
         }
     }
 }
@@ -89,65 +87,47 @@ impl CommandTool {
 
         let escaped_cmd = crate::utils::telegram_markdown::escape_text(command);
 
+        let (cancel_tx, cancel_rx) = tokio::sync::oneshot::channel::<()>();
+        self.cancel_registry
+            .register(cmd_id.clone(), cancel_tx)
+            .await;
         // Verbose: cancel button + live output + final result
         // Minimal: cancel button (simple text) + no live output, delete on finish
         // Silent: no message at all (tool_notifier handles nothing)
         let (msg_id, send_mode) = match ctx.tool_ui_mode {
             ToolUiMode::Verbose => {
-                let status_text =
-                    format!("💻 Running: `{}`\n\n```\n⏳ Starting...\n```", escaped_cmd);
+                let st = format!("💻 Running: `{}`\n\n```\n⏳ Starting...\n```", escaped_cmd);
                 let id = self
                     .sender
-                    .show_cancel_button(&ctx.chat_id, &status_text, &cmd_id)
+                    .show_cancel_button(&ctx.chat_id, &st, &cmd_id)
                     .await?;
                 (Some(id), SendMode::Verbose)
             }
             ToolUiMode::Minimal => {
-                let status_text = format!("⏳ Running: `{}`", escaped_cmd);
+                let st = format!("⏳ Running: `{}`", escaped_cmd);
                 let id = self
                     .sender
-                    .show_cancel_button(&ctx.chat_id, &status_text, &cmd_id)
+                    .show_cancel_button(&ctx.chat_id, &st, &cmd_id)
                     .await?;
                 (Some(id), SendMode::Minimal)
             }
             ToolUiMode::Silent => (None, SendMode::Silent),
         };
 
-        let (cancel_tx, cancel_rx) = tokio::sync::oneshot::channel::<()>();
-        self.cancel_registry
-            .register(cmd_id.clone(), cancel_tx)
-            .await;
-
         let (output_tx, mut output_rx) = tokio::sync::mpsc::channel::<String>(256);
-        let output_tx2 = output_tx.clone();
-        let mut child_stdout = child.stdout.take();
-        let mut child_stderr = child.stderr.take();
+        let stdout_tx = output_tx.clone();
+        let stderr_tx = output_tx.clone();
+        let child_stdout = child.stdout.take();
+        let child_stderr = child.stderr.take();
 
         let stdout_handle = tokio::spawn(async move {
-            let mut buf = vec![0u8; 4096];
-            while let Some(stream) = child_stdout.as_mut() {
-                match stream.read(&mut buf).await {
-                    Ok(0) | Err(_) => break,
-                    Ok(n) => {
-                        let _ = output_tx
-                            .send(String::from_utf8_lossy(&buf[..n]).to_string())
-                            .await;
-                    }
-                }
+            if let Some(reader) = child_stdout {
+                crate::utils::process::drain_pipe(reader, stdout_tx).await;
             }
         });
-
         let stderr_handle = tokio::spawn(async move {
-            let mut buf = vec![0u8; 4096];
-            while let Some(stream) = child_stderr.as_mut() {
-                match stream.read(&mut buf).await {
-                    Ok(0) | Err(_) => break,
-                    Ok(n) => {
-                        let _ = output_tx2
-                            .send(String::from_utf8_lossy(&buf[..n]).to_string())
-                            .await;
-                    }
-                }
+            if let Some(reader) = child_stderr {
+                crate::utils::process::drain_pipe(reader, stderr_tx).await;
             }
         });
 
@@ -156,16 +136,21 @@ impl CommandTool {
         let mut last_edit = Instant::now();
         let mut exit_code: Option<i32> = None;
         let mut cancelled = false;
+        let mut timed_out = false;
         tokio::pin!(cancel_rx);
 
+        let timeout_secs = self.execute_timeout_secs;
+        let timeout_fut = crate::utils::process::optional_timeout(timeout_secs);
+        tokio::pin!(timeout_fut);
+
         loop {
             tokio::select! {
                 Some(chunk) = output_rx.recv() => {
                     output_buffer.push_str(&chunk);
                     if output_buffer.chars().count() > MAX_BUFFER_CHARS {
                         output_buffer = crate::utils::strings::truncate_tail(&output_buffer, MAX_BUFFER_CHARS);
                     }
-                    if matches!(send_mode, SendMode::Verbose) && last_edit.elapsed() >= std::time::Duration::from_millis(500) {
+                    if matches!(send_mode, SendMode::Verbose) && last_edit.elapsed() >= Duration::from_millis(500) {
                         let capped = crate::utils::strings::truncate_tail(&output_buffer, 3500);
                         let text = format!("💻 Running: `{}`\n\n```\n{}\n```", escaped_cmd, capped);
                         if let Some(mid) = &msg_id {
@@ -182,21 +167,25 @@ impl CommandTool {
                 }
                 _ = &mut cancel_rx => {
                     cancelled = true;
-                    #[cfg(unix)]
-                    if let Some(pid) = child.id() {
-                        let _ = nix::sys::signal::killpg(
-                            nix::unistd::Pid::from_raw(pid as i32),
-                            nix::sys::signal::Signal::SIGKILL,
-                        );
-                    }
-                    let _ = child.kill().await;
-                    let _ = child.wait().await;
+                    crate::utils::process::kill_child(&mut child).await;
+                    break;
+                }
+                _ = &mut timeout_fut => {
+                    timed_out = true;
+                    crate::utils::process::kill_child(&mut child).await;
                     break;
                 }
             }
         }
 
-        let _ = tokio::join!(stdout_handle, stderr_handle);
+        // Post-exit drain with timeout guard
+        let drain = tokio::time::timeout(Duration::from_secs(5), async move {
+            tokio::join!(stdout_handle, stderr_handle)
+        });
+        if drain.await.is_err() {
+            warn!("command_tool: drain timed out after child exit");
+        }
+
         while let Ok(chunk) = output_rx.try_recv() {
             output_buffer.push_str(&chunk);
         }
@@ -217,26 +206,38 @@ impl CommandTool {
             }
         }
 
-        let result = if cancelled {
+        let result = if cancelled || timed_out {
+            let label = if timed_out {
+                format!("Timed out after {}s", timeout_secs)
+            } else {
+                "Cancelled".to_string()
+            };
             if let Some(mid) = &msg_id {
                 match send_mode {
                     SendMode::Verbose => {
                         let body = format_body(&output_buffer, "");
                         let text = match body {
-                            None => format!("❌ Cancelled: `{}`", escaped_cmd),
-                            Some(b) => format!("❌ Cancelled: `{}`\n\n{}", escaped_cmd, b),
+                            None => format!("❌ {}: `{}`", label, escaped_cmd),
+                            Some(b) => format!("❌ {}: `{}`\n\n{}", label, escaped_cmd, b),
                         };
                         let _ = self.sender.edit_message(&ctx.chat_id, mid, &text).await;
                     }
                     SendMode::Minimal => {
-                        // Delete the minimal message
                         let _ = self.sender.delete_message(&ctx.chat_id, mid).await;
                     }
-                    // Silent mode sends no message; nothing to clean up.
                     SendMode::Silent => {}
                 }
             }
-            "⚠️ User cancelled the command".to_string()
+            let mut msg = if timed_out {
+                format!("⚠️ Command timed out after {}s", timeout_secs)
+            } else {
+                "⚠️ User cancelled the command".to_string()
+            };
+            if !output_buffer.is_empty() {
+                let capped = crate::utils::strings::truncate_tail(&output_buffer, 3500);
+                msg.push_str(&format!("\n\nPartial output:\n```\n{}\n```", capped));
+            }
+            msg
         } else if let Some(code) = exit_code {
             if let Some(mid) = &msg_id {
                 match send_mode {
@@ -257,10 +258,8 @@ impl CommandTool {
                         let _ = self.sender.edit_message(&ctx.chat_id, mid, &text).await;
                     }
                     SendMode::Minimal => {
-                        // Delete the minimal message
                         let _ = self.sender.delete_message(&ctx.chat_id, mid).await;
                     }
-                    // Silent mode sends no message; nothing to clean up.
                     SendMode::Silent => {}
                 }
             }

src/config.rs

@@ -184,10 +184,26 @@ pub struct OcrConfig {
     pub model_dir: std::path::PathBuf,
 }
 
-#[derive(Debug, Deserialize, Clone, Default)]
+#[derive(Debug, Deserialize, Clone)]
 pub struct SandboxConfig {
     #[serde(default)]
     pub allowed_directory: PathBuf,
+    /// Wall-clock limit for `execute_command`. 0 = no timeout.
+    #[serde(default = "default_execute_timeout_secs")]
+    pub execute_timeout_secs: u64,
+}
+
+impl Default for SandboxConfig {
+    fn default() -> Self {
+        Self {
+            allowed_directory: PathBuf::new(),
+            execute_timeout_secs: default_execute_timeout_secs(),
+        }
+    }
+}
+
+fn default_execute_timeout_secs() -> u64 {
+    90
 }
 
 #[derive(Debug, Deserialize, Clone)]