memory: let a user see and edit what dirge remembers - #3

docs/features.md

@@ -33,7 +33,7 @@ headline differentiators, see the top-level [README](../README.md).
 - **Model-aware steering**: the harness detects the active model family and tailors guidance to it. DeepSeek **chat** models (v3/v4) get an extra preamble fragment — a Plan-Execute-Verify working method, structural-constraint framing (name files/functions/order, not "be modular"), an explicit success/never contract, and an anti-repetition rule (accept an errored or truncated tool result and adapt rather than re-issuing the same call) — appended last so it sits closest to the action boundary, where rules resist drift in long tool-calling loops. Other models, and the DeepSeek reasoner (which ignores the system prompt), are unaffected. Baked-in and automatic; no config key. See [prompts.md](prompts.md#model-aware-steering).
 - **Subagent support**: `task` tool spawns a subagent for research or general analysis subtasks. Optionally run the subagent under a named [agent profile](agents.md) (`task(agent="<name>")`) to give it its own model + system prompt, and opt a profile into **tooled subagents** (`subagent.tools: readonly` for read/grep/glob, or `readwrite` to also let the subagent edit/write/bash the repo directly), plus grant selected [MCP tools](mcp.md) via `subagent_mcp`.
 - **Background shells** (Claude-Code-style): the `bash` tool accepts `background: true` to run a command **detached and unbounded** (for dev servers, watch builds, long-running jobs) — it returns immediately with a shell id. The model reads accumulated output incrementally with the **`bash_output`** tool and stops the shell with **`kill_shell`** (both take the id); an optional `timeout` auto-kills after N seconds. Shells are tracked in a dedicated registry, capped at 8 concurrent, listed by `/tasks`, and killed when the session ends. The status bar shows live counts when any are running: `agents:N` (background subagents) and `shells:N` (background shells).
-- **Memory & self-improvement**: persistent per-project memory in the session DB (`.dirge/sessions/state.db`, `memories` table — facts under the `memory` target, anti-patterns under `pitfalls`), injected into the system prompt as a frozen snapshot. Memory is two-tiered: hot entries inline verbatim; when the inline budget fills, the least-salient entries demote to a breadcrumb index (one line of id + preview each) that the agent dereferences with `memory(action='expand')` or queries with `memory(action='search')` (FTS5). Removal archives (tombstones) rather than deletes — `restore` brings entries back. Legacy `MEMORY.md`/`PITFALLS.md` files are imported automatically on first load and parked as `*.imported`. A **global, cross-project memory tier** (a single store in the user data dir, default on) holds durable user preferences that follow you across every repo; it's injected into the prompt under its own header, and the `memory` tool writes/searches it with `scope: "global"` (default scope stays project). After an idle session, a unified post-session orchestrator runs (in order, fire-and-forget): a background review that extracts learnings into memory + skills, then a skills curator and a memory curator (stale-detection + lifecycle + LLM consolidation, with audit reports under each store's `.curator_reports/`). Procedural memories can carry a **falsifiable expectation** — a trigger plus the outcome expected when it fires (a command that should have run, or the run ending verified-green) — settled deterministically in the post-session pass against the session digest and the run's verification status, with no LLM in that path. Success, failure, and "the situation never arose" stay three distinct verdicts: collapsing the last two would make the expectation impossible to refute, and only a refutable one is worth recording. Memories without an expectation are untouched, and settling only moves the existing bounded effectiveness counters — it never deletes.
+- **Memory & self-improvement**: persistent per-project memory in the session DB (`.dirge/sessions/state.db`, `memories` table — facts under the `memory` target, anti-patterns under `pitfalls`), injected into the system prompt as a frozen snapshot. Memory is two-tiered: hot entries inline verbatim; when the inline budget fills, the least-salient entries demote to a breadcrumb index (one line of id + preview each) that the agent dereferences with `memory(action='expand')` or queries with `memory(action='search')` (FTS5). Removal archives (tombstones) rather than deletes — `restore` brings entries back. Legacy `MEMORY.md`/`PITFALLS.md` files are imported automatically on first load and parked as `*.imported`. A **global, cross-project memory tier** (a single store in the user data dir, default on) holds durable user preferences that follow you across every repo; it's injected into the prompt under its own header, and the `memory` tool writes/searches it with `scope: "global"` (default scope stays project). After an idle session, a unified post-session orchestrator runs (in order, fire-and-forget): a background review that extracts learnings into memory + skills, then a skills curator and a memory curator (stale-detection + lifecycle + LLM consolidation, with audit reports under each store's `.curator_reports/`). Procedural memories can carry a **falsifiable expectation** — a trigger plus the outcome expected when it fires (a command that should have run, or the run ending verified-green) — settled deterministically in the post-session pass against the session digest and the run's verification status, with no LLM in that path. Success, failure, and "the situation never arose" stay three distinct verdicts: collapsing the last two would make the expectation impossible to refute, and only a refutable one is worth recording. Memories without an expectation are untouched, and settling only moves the existing bounded effectiveness counters — it never deletes. Inspect what is stored with `/memory`, and change it with `/memory edit`, which opens the whole store in `$EDITOR` — reword a line to reword the memory, delete a block to forget it (archived, not destroyed), add a block to record something new. Each entry is anchored on its id so an edit is an in-place update: lineage, use counts, confidence and the procedural outcome counters survive being reworded.
 - **MCP support**: connect MCP servers for extended tooling (optional compile-time feature).
 - **dirge as an MCP server** (`dirge mcp`): run dirge itself as an MCP server so another agent (e.g. Claude Code) can **delegate implementation tasks to dirge and review them** — the caller plans/architects, dirge implements. Keeps a persistent per-project session (`delegate` extends it, `new_session` rotates); each delegation returns a summary + the files it changed for review. Built into the binary (`mcp-server` feature, default on). See [mcp-server.md](mcp-server.md).
 - **File-state rewind**: `Esc-Esc` opens the rewind picker, which rolls back the working tree, not just the conversation. Every write/edit/edit_lines/apply_patch (incl. delete/rename) snapshots the touched file's pre-mutation content keyed by the triggering user prompt; rewinding to a prompt restores all files to their pre-prompt state in lockstep with the conversation truncation, so a long autonomous run is safe to unwind. A file created in the rewound region is deleted on restore; a deleted file is recreated. Content is deduplicated through a content-addressed pool. In-memory and process-scoped (works within a live session, not across a restart).

src/extras/memory_db.rs

@@ -308,6 +308,24 @@ fn truncate_for_error(s: &str) -> String {
 
 // ── Store ────────────────────────────────────────────────────────────
 
+/// One entry as `/memory` shows it.
+///
+/// Carries the uid because that is what `/memory edit` anchors on, and the
+/// cheap usefulness signals (`tier`, `use_count`) so a reader can tell an
+/// inlined fact the agent leans on from a breadcrumb it has never touched.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct BrowseEntry {
+    pub uid: String,
+    /// `"memory"` or `"pitfalls"`.
+    pub target: String,
+    pub kind: String,
+    pub content: String,
+    /// `"hot"` (inlined in the prompt) or `"breadcrumb"` (index only).
+    pub tier: String,
+    pub use_count: i64,
+    pub updated_at: String,
+}
+
 /// One active row, as the matching/eviction logic sees it.
 #[derive(Clone)]
 struct ActiveRow {
@@ -1222,6 +1240,29 @@ impl SqliteMemoryStore {
             .map_err(|e| format!("Failed to begin transaction: {e}"))?;
         let rows = Self::active_rows(&tx, target)?;
         let idx = find_unique_match(&rows, old_text)?;
+        Self::apply_replacement(&tx, target, &rows, idx, &new_entry, kind)?;
+        tx.commit().map_err(|e| format!("Failed to commit: {e}"))?;
+        Ok(())
+    }
+
+    /// The in-place half of a replace, once the target row is known.
+    ///
+    /// Split out so `replace_entry` (matched by substring) and
+    /// `replace_entry_by_uid` (matched by stable id, used by `/memory edit`)
+    /// share one definition of what replacing means. The distinction that
+    /// matters is preserved here: this UPDATEs the row, so uid, lineage,
+    /// `created_at`, `use_count` and `confidence` all survive. Only a
+    /// *re-classification* resets the outcome counters, and only because a
+    /// procedural entry re-kinded to semantic must not keep a track record
+    /// that no longer applies to it.
+    fn apply_replacement(
+        tx: &Connection,
+        target: &str,
+        rows: &[ActiveRow],
+        idx: usize,
+        new_entry: &str,
+        kind: Option<MemoryKind>,
+    ) -> Result<(), String> {
         let id = rows[idx].id;
 
         // dirge-catw: reject a replacement that duplicates a DIFFERENT active
@@ -1267,7 +1308,7 @@ impl SqliteMemoryStore {
             .map_err(|e| format!("Failed to reindex entry: {e}"))?;
         tx.execute(
             "INSERT INTO memories_fts(rowid, content) VALUES (?1, ?2)",
-            params![id, redact_for_fts(&new_entry)],
+            params![id, redact_for_fts(new_entry)],
         )
         .map_err(|e| format!("Failed to reindex entry: {e}"))?;
 
@@ -1280,19 +1321,116 @@ impl SqliteMemoryStore {
             let effective_kind =
                 kind.unwrap_or_else(|| parse_kind(&rows[idx].kind).unwrap_or_default());
             let demoted = Self::make_room_in_hot(
-                &tx,
+                tx,
                 target,
                 0,
                 matches!(effective_kind, MemoryKind::Working),
             )?;
             if demoted > 0 {
-                Self::compact_breadcrumbs(&tx, target)?;
+                Self::compact_breadcrumbs(tx, target)?;
             }
         }
-        tx.commit().map_err(|e| format!("Failed to commit: {e}"))?;
         Ok(())
     }
 
+    /// Every active entry across both targets, for `/memory` and
+    /// `/memory edit`. Ordered target-then-id so the rendering is stable
+    /// between runs — an editable document that reshuffles itself is
+    /// unreviewable.
+    pub fn list_all(&self) -> Result<Vec<BrowseEntry>, String> {
+        let conn = self.conn.lock_ignore_poison();
+        let mut stmt = conn
+            .prepare(
+                "SELECT uid, target, kind, content, tier, use_count, updated_at
+                 FROM memories WHERE status = 'active' ORDER BY target, id",
+            )
+            .map_err(|e| format!("Failed to read memories: {e}"))?;
+        let rows = stmt
+            .query_map([], |row| {
+                Ok(BrowseEntry {
+                    uid: row.get(0)?,
+                    target: row.get(1)?,
+                    kind: row.get(2)?,
+                    content: row.get(3)?,
+                    tier: row.get(4)?,
+                    use_count: row.get(5)?,
+                    updated_at: row.get(6)?,
+                })
+            })
+            .map_err(|e| format!("Failed to read memories: {e}"))?
+            .collect::<Result<Vec<_>, _>>()
+            .map_err(|e| format!("Failed to read memories: {e}"))?;
+        Ok(rows)
+    }
+
+    /// Reword an entry identified by its stable uid.
+    ///
+    /// The uid is what makes `/memory edit` safe: matching on content would
+    /// mean an edit could not find the row it just changed, and re-creating
+    /// the row would silently reset `use_count`, `confidence`, the procedural
+    /// outcome counters and the supersession chain. This is an UPDATE, so all
+    /// of that survives.
+    // Unix-only: reached solely from `/memory edit`, which needs $EDITOR.
+    #[cfg(unix)]
+    pub fn replace_entry_by_uid(
+        &self,
+        uid: &str,
+        new_entry: &str,
+        kind: Option<MemoryKind>,
+    ) -> Result<(), String> {
+        scan_for_threats(new_entry)?;
+        let trimmed = new_entry.trim();
+        if trimmed.is_empty() {
+            return Err("Cannot replace with empty entry".to_string());
+        }
+        let new_entry = redact_for_fts(trimmed);
+
+        let mut conn = self.conn.lock_ignore_poison();
+        let tx = conn
+            .transaction()
+            .map_err(|e| format!("Failed to begin transaction: {e}"))?;
+
+        for target in ["memory", "pitfalls"] {
+            let rows = Self::active_rows(&tx, target)?;
+            if let Some(idx) = rows.iter().position(|r| r.uid == uid) {
+                let char_limit = char_limit_for(target);
+                if new_entry.len() > char_limit {
+                    return Err(format!(
+                        "Entry is {} chars but the entire memory budget is {char_limit}; \
+                         split it into smaller entries.",
+                        new_entry.len(),
+                    ));
+                }
+                Self::apply_replacement(&tx, target, &rows, idx, &new_entry, kind)?;
+                tx.commit().map_err(|e| format!("Failed to commit: {e}"))?;
+                return Ok(());
+            }
+        }
+        Err(format!("No active memory with id {uid}"))
+    }
+
+    /// Tombstone an entry by uid. Archives rather than deletes, exactly like
+    /// `remove_entry`, so `restore` can still bring it back — deleting a
+    /// line in an editor should not be more destructive than the tool's own
+    /// removal.
+    // Unix-only: reached solely from `/memory edit`, which needs $EDITOR.
+    #[cfg(unix)]
+    pub fn remove_entry_by_uid(&self, uid: &str) -> Result<(), String> {
+        let mut conn = self.conn.lock_ignore_poison();
+        let tx = conn
+            .transaction()
+            .map_err(|e| format!("Failed to begin transaction: {e}"))?;
+        for target in ["memory", "pitfalls"] {
+            let rows = Self::active_rows(&tx, target)?;
+            if let Some(row) = rows.iter().find(|r| r.uid == uid) {
+                Self::tombstone_row(&tx, row.id)?;
+                tx.commit().map_err(|e| format!("Failed to commit: {e}"))?;
+                return Ok(());
+            }
+        }
+        Err(format!("No active memory with id {uid}"))
+    }
+
     /// Supersede an active entry with a newer fact (dirge-fa10). Where
     /// `replace` is an in-place UPDATE for a reworded SAME fact (keeps
     /// uid, lineage, outcome record), supersession is for a

src/ui/external_editor.rs

@@ -0,0 +1,104 @@
+//! Hand a block of text to `$EDITOR` and read back what the user saved.
+//!
+//! Extracted from `Input::open_in_external_editor` so `/edit` and the memory
+//! review share one implementation. The tricky parts are all here:
+//!
+//! * The temp file is created with `create_new` (`O_EXCL`), so a symlink
+//!   pre-planted at the predictable path fails the open instead of being
+//!   written through.
+//! * dirge points fds 1/2 at its log file for the TUI session. They are
+//!   redirected to `/dev/tty` for the child's lifetime and restored after,
+//!   or the editor draws into the log.
+//! * The path is passed as a positional arg via `"$@"` rather than
+//!   interpolated, so spaces and metacharacters in it cannot break out.
+//!   `$EDITOR` itself still word-splits, which is what makes
+//!   `EDITOR="code --wait"` work.
+//!
+//! The caller MUST suspend the TUI first — see
+//! [`suspend_tui_for_subprocess`](crate::ui::terminal::suspend_tui_for_subprocess).
+
+#[cfg(unix)]
+use std::io::Write;
+
+/// Open `$EDITOR` on `seed` and return the saved contents.
+///
+/// `tag` distinguishes concurrent temp files and gives the editor a useful
+/// filename to syntax-highlight from (e.g. `"input"`, `"memory-review"`).
+///
+/// Returns `None` if the editor could not be spawned or exited non-zero —
+/// which is the deliberate way to abort (`:cq` in vim). Callers must treat
+/// `None` as "change nothing"; a failed edit must never be read as an empty
+/// document, or aborting would silently discard the caller's data. Errors
+/// are reported to the user before returning.
+#[cfg(unix)]
+pub(crate) fn edit_text(seed: &str, tag: &str) -> Option<String> {
+    let editor = std::env::var("EDITOR").unwrap_or_else(|_| "vi".to_string());
+
+    let path = std::env::temp_dir().join(format!("dirge-{tag}-{}.md", std::process::id()));
+    let _ = std::fs::remove_file(&path);
+    let write_result = std::fs::OpenOptions::new()
+        .write(true)
+        .create_new(true)
+        .open(&path)
+        .and_then(|mut f| f.write_all(seed.as_bytes()));
+    if let Err(e) = write_result {
+        crate::ui::notifications::notify_send(crate::ui::notifications::Notification::Error(
+            format!("External editor: failed to write temp file: {e}"),
+        ));
+        return None;
+    }
+
+    let saved: Option<(i32, i32, i32)> = unsafe {
+        let tty = libc::open(c"/dev/tty".as_ptr(), libc::O_RDWR);
+        if tty < 0 {
+            None
+        } else {
+            let so = libc::dup(1);
+            let se = libc::dup(2);
+            libc::dup2(tty, 1);
+            libc::dup2(tty, 2);
+            Some((tty, so, se))
+        }
+    };
+
+    let status = std::process::Command::new("sh")
+        .arg("-c")
+        .arg(format!("{editor} \"$@\""))
+        .arg(&editor) // $0
+        .arg(&path) // $1 → "$@"
+        .status();
+
+    if let Some((tty, so, se)) = saved {
+        unsafe {
+            libc::dup2(so, 1);
+            libc::dup2(se, 2);
+            libc::close(so);
+            libc::close(se);
+            libc::close(tty);
+        }
+    }
+
+    let result = match status {
+        Ok(s) if s.success() => std::fs::read_to_string(&path).ok(),
+        Ok(s) => {
+            crate::ui::notifications::notify_send(crate::ui::notifications::Notification::Error(
+                format!(
+                    "External editor exited with: {}",
+                    s.code()
+                        .map(|c| format!("code {c}"))
+                        .unwrap_or_else(|| "signal".into())
+                ),
+            ));
+            None
+        }
+        Err(e) => {
+            crate::ui::notifications::notify_send(crate::ui::notifications::Notification::Error(
+                format!("External editor: failed to spawn {editor}: {e}"),
+            ));
+            None
+        }
+    };
+
+    let _ = std::fs::remove_file(&path);
+    result
+}