memory: confirm writes before storing them - #2 (#815)

* memory: confirm writes before storing them

dirge decides what is worth remembering on its own. The agent writes
mid-session, and after an idle session the background review and memory
curator write more. All of it lands in the system prompt of every later
session in the project — and under global scope, every project. Nobody
approves any of it, a wrong or trivial memory persists silently, and
something the human knows matters is never recorded unless the model
happened to notice it.

`memory.confirm_writes` (default off) puts a human in that loop. An `add`
is queued instead of stored; `/memory review` opens the queue in $EDITOR.

The file is the desired final state of the batch, not a diff — so reject
(delete the block), reword (edit the text) and add (type a new one) are one
operation, and recording what the model missed is a first-class action
rather than an afterthought. Accepted entries go in through the normal
add_entry path, so a memory you typed is indistinguishable from one the
model proposed.

A queued entry rides on `status = 'pending'`. Every read path already
filters `status = 'active'` — the snapshot, `view`, and the FTS join in
`search` — so a proposal is inert everywhere with no new filtering and no
migration. It also skips hot-tier compaction: a merely proposed memory must
not demote an accepted one before anyone agreed to keep it.

Only `add` is gated. `replace`/`supersede`/`remove` act on entries a human
already accepted, and `supersede` usually fires because the user just
corrected the agent.

An aborted edit (`:cq`) changes nothing, which is why `edit_text` returns
None rather than an empty document — confusing the two would reject the
whole queue. An unparseable file aborts the apply intact rather than
half-applying.

$EDITOR handling is extracted from `Input::open_in_external_editor` into
`ui::external_editor` and shared, rather than copied: the O_EXCL temp file,
the /dev/tty fd juggling and the git-style argv are all easy to get subtly
wrong twice.

Verified end to end against the built binary: writes queue as pending; the
agent's own `view` and `search` report zero entries; review rewords one
entry, rejects another and adds a third, leaving exactly that in the store
with an empty queue; an editor exiting non-zero leaves queue and store
untouched; and with the gate off writes go straight through as before.

* gate the /memory review editor path to unix; collapse a nested if

collect/render/parse/apply and PendingEntry/list_pending/clear_pending
are only reached from /memory review, which needs $EDITOR, so windows
flagged them dead. notify_if_queued and add_pending stay cross-platform:
the queue itself is written and counted everywhere. clippy wanted the
indent-continuation if collapsed.

---------

Co-authored-by: Wayne <wayne@grange.la>
Co-authored-by: Yogthos <yogthos@gmail.com>

Author: 3 people

docs/config.md

@@ -292,6 +292,46 @@ Reciprocal Rank Fusion. It needs an OpenAI-compatible embeddings endpoint.
 | `embed_model`       | string  | Embedding model id. Default `text-embedding-3-small` — set it when pointing at a non-OpenAI endpoint. |
 | `embed_api_key_env` | string  | Name of the env var holding the API key (the key itself is never stored in config). Omit for a keyless local endpoint. |
 | `verbatim_pre_recall` | boolean | Each turn, auto-search memory on the verbatim user message and inject the hits as a supplemental context note (separate from the frozen system-prompt snapshot — it never changes the cached prefix). Surfaces relevant memory the agent wouldn't think to look up. Works with BM25 or hybrid. Default `false`. |
+| `confirm_writes`    | boolean | Require human confirmation before any memory `add` is stored. See [Confirming memory writes](#confirming-memory-writes). Default `false`. |
+
+### Confirming memory writes
+
+dirge decides what is worth remembering on its own. The agent writes mid-session
+whenever it judges something memorable, and after an idle session the background
+review and memory curator — forked LLM runners — write more. All of it lands in
+the system prompt of every later session in the project, and under global scope,
+of every project.
+
+`confirm_writes` puts a human in that loop:
+
+```json
+{ "memory": { "confirm_writes": true } }
+```
+
+An `add` is then *queued* rather than stored. A queued entry is inert — it is not
+in the prompt snapshot, not in `memory view`, and not in `memory search` — and the
+model is told plainly that it is not yet stored, so it does not treat the fact as
+durable. Review them with:
+
+```
+/memory review
+```
+
+which opens the queue in `$EDITOR`. **The file is the desired final state**: delete
+a block to reject it, edit the text to reword it, or type a new block to record
+something the model never noticed. Saving stores exactly what is in the file;
+quitting without saving (`:cq`) leaves the queue untouched for next time. Accepted
+entries go in through the normal write path, so a memory you typed is
+indistinguishable from one the agent proposed.
+
+Only `add` is gated. `replace`, `supersede` and `remove` act on entries you already
+accepted, and `supersede` usually fires because you just corrected the agent —
+asking you to confirm your own correction would be noise.
+
+A notice appears when the queue is non-empty, both after the post-session passes
+and once at startup. Headless `-p` has no one to ask, so entries simply queue there
+and wait for your next interactive session; nothing is auto-accepted and nothing is
+lost.
 
 Safe by default and on failure: with `hybrid_retrieval` off, or the endpoint
 unset/unreachable/timed out, search silently falls back to BM25 — it never

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. 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.
+- **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. Set `memory.confirm_writes` to put a human in that loop: an `add` is then queued instead of stored — inert, absent from the prompt and from search — and `/memory review` opens the queue in `$EDITOR`, where a block can be rejected by deleting it, reworded in place, or added outright for something the model never noticed. See [config.md](config.md#confirming-memory-writes).
 - **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/agent/builder/loop_tools.rs

@@ -126,11 +126,14 @@ pub(crate) async fn register_memory_tool(
     global_store: Option<std::sync::Arc<dyn crate::extras::memory_provider::MemoryProvider>>,
     permission: Option<PermCheck>,
     ask_tx: Option<AskSender>,
+    confirm_writes: bool,
 ) {
     use crate::agent::agent_loop::{RigToolAdapter, types::ToolExecutionMode};
     match memory_store {
         Some(store) => {
-            let tool = tools::MemoryTool::new(store, permission, ask_tx).with_global(global_store);
+            let tool = tools::MemoryTool::new(store, permission, ask_tx)
+                .with_global(global_store)
+                .with_confirm_writes(confirm_writes);
             let adapter = RigToolAdapter::new(Box::new(tool))
                 .await
                 .with_execution_mode(ToolExecutionMode::Sequential);
@@ -156,12 +159,14 @@ pub(crate) async fn build_review_memory_tool(
     global_store: Option<std::sync::Arc<dyn crate::extras::memory_provider::MemoryProvider>>,
     permission: Option<PermCheck>,
     ask_tx: Option<AskSender>,
+    confirm_writes: bool,
 ) -> Option<std::sync::Arc<dyn crate::agent::agent_loop::LoopTool>> {
     use crate::agent::agent_loop::{RigToolAdapter, types::ToolExecutionMode};
     let store = memory_store?;
     let tool = tools::MemoryTool::new(store, permission, ask_tx)
         .with_global(global_store)
-        .with_review_actions(true);
+        .with_review_actions(true)
+        .with_confirm_writes(confirm_writes);
     let adapter = RigToolAdapter::new(Box::new(tool))
         .await
         .with_execution_mode(ToolExecutionMode::Sequential);
@@ -844,11 +849,19 @@ pub async fn build_loop_tools(
     // dirge-ygm3: build the review-enabled memory tool BEFORE `global_store` is
     // moved into the main registration. It is returned separately, never added
     // to `tools`.
+    // The gate applies to BOTH instances. The review fork is the one that
+    // writes unattended, so exempting it would miss the case this exists for.
+    let confirm_writes = cfg
+        .memory
+        .as_ref()
+        .and_then(|m| m.confirm_writes)
+        .unwrap_or(false);
     let review_memory_tool = build_review_memory_tool(
         memory_store.clone(),
         global_store.clone(),
         permission.clone(),
         ask_tx.clone(),
+        confirm_writes,
     )
     .await;
     register_memory_tool(
@@ -857,6 +870,7 @@ pub async fn build_loop_tools(
         global_store,
         permission.clone(),
         ask_tx.clone(),
+        confirm_writes,
     )
     .await;
 

src/agent/builder/reminder_tests.rs

@@ -230,7 +230,7 @@ async fn memory_tool_registration_degrades_when_store_unavailable() {
 
     // Load failed → no tool, no panic.
     let mut tools: Vec<std::sync::Arc<dyn crate::agent::agent_loop::LoopTool>> = Vec::new();
-    register_memory_tool(&mut tools, None, None, None, None).await;
+    register_memory_tool(&mut tools, None, None, None, None, false).await;
     assert!(
         tools.is_empty(),
         "unavailable store must not register a memory tool"
@@ -249,7 +249,7 @@ async fn memory_tool_registration_degrades_when_store_unavailable() {
     let paths = crate::extras::dirge_paths::ProjectPaths::new(&dir);
     let store: std::sync::Arc<dyn crate::extras::memory_provider::MemoryProvider> =
         std::sync::Arc::new(crate::extras::memory_db::SqliteMemoryStore::load(&paths).unwrap());
-    register_memory_tool(&mut tools, Some(store), None, None, None).await;
+    register_memory_tool(&mut tools, Some(store), None, None, None, false).await;
     let names: Vec<&str> = tools.iter().map(|t| t.name()).collect();
     assert_eq!(names, vec!["memory"], "available store registers the tool");
     let _ = std::fs::remove_dir_all(&dir);

src/agent/post_session.rs

@@ -178,6 +178,11 @@ pub fn spawn_post_session(
         // baked-in system-prompt block wouldn't otherwise reflect these
         // writes until a restart.
         crate::agent::agent_loop::context_manager::mark_memories_dirty();
+        // With `memory.confirm_writes` on, the review and curator passes above
+        // wrote proposals rather than memories. Say so — this is the moment
+        // the queue grows, and it grows while the user is watching the
+        // session wind down rather than driving it.
+        crate::ui::memory_review::notify_if_queued(&paths);
     });
 }
 

src/agent/tools/memory.rs

@@ -26,6 +26,11 @@ pub struct MemoryTool {
     /// schema AND rejected at the call layer; the review runner gets a separate
     /// instance built with `true`.
     review_actions: bool,
+    /// `memory.confirm_writes`: route `add` to the review queue instead of
+    /// storing it. Set from config on every instance that can write —
+    /// including the background-review and curator forks, which is the
+    /// whole point: those are the writes nobody sees happen.
+    confirm_writes: bool,
 }
 
 impl MemoryTool {
@@ -40,9 +45,16 @@ impl MemoryTool {
             store,
             global_store: None,
             review_actions: false,
+            confirm_writes: false,
         }
     }
 
+    /// Gate `add` behind human review (`memory.confirm_writes`).
+    pub fn with_confirm_writes(mut self, enabled: bool) -> Self {
+        self.confirm_writes = enabled;
+        self
+    }
+
     /// Attach the global (cross-project) memory tier. `None` is a no-op.
     pub fn with_global(mut self, global_store: Option<Arc<dyn MemoryProvider>>) -> Self {
         self.global_store = global_store;
@@ -269,10 +281,25 @@ impl PortableTool for MemoryTool {
                     "content",
                     "add",
                 )?;
-                let resp = store
-                    .add(target, content, args.kind.as_deref())
-                    .map_err(ToolError::Msg)?;
-                crate::agent::review::fire_memory_write(store.as_ref(), "add", target, content);
+                // dirge memory review: when the gate is on, an add becomes a
+                // proposal. Only `add` is gated — `replace`/`supersede` edit
+                // facts a human already accepted, and `supersede` usually
+                // fires because the user just contradicted something, so
+                // asking them to confirm their own correction is noise.
+                let resp = if self.confirm_writes {
+                    store
+                        .queue_for_review(target, content, args.kind.as_deref())
+                        .map_err(ToolError::Msg)?
+                } else {
+                    store
+                        .add(target, content, args.kind.as_deref())
+                        .map_err(ToolError::Msg)?
+                };
+                // The write hooks observe real writes only; a queued entry
+                // has not happened yet and must not look like it did.
+                if !self.confirm_writes {
+                    crate::agent::review::fire_memory_write(store.as_ref(), "add", target, content);
+                }
                 Ok(serde_json::to_string_pretty(&resp)
                     .unwrap_or_else(|_| r#"{"error":"serialization failed"}"#.to_string()))
             }

src/config/mod.rs

@@ -515,6 +515,12 @@ pub struct MemoryConfig {
     /// snapshot). Surfaces relevant memory the agent wouldn't think to look
     /// up. Default off.
     pub verbatim_pre_recall: Option<bool>,
+    /// Require human confirmation before any `memory add` is stored. Entries
+    /// are queued instead, stay out of the prompt, and are accepted, edited,
+    /// rejected or added to via `/memory review`. Covers the background
+    /// review and memory curator forks as well as the agent's own writes.
+    /// Default off — it changes long-standing behavior.
+    pub confirm_writes: Option<bool>,
 }
 
 #[derive(Debug, Default, Clone, Deserialize)]