memory: confirm writes before storing them - #2
Open
wayniacal wants to merge 4 commits into
Open
Conversation
wayniacal
force-pushed
the
feat/memory-review
branch
from
August 24, 2026 09:40
d1efd4c to
35b315a
Compare
…rge-code#813) Plugins could register tools and intercept them, but not call one. A plugin that wanted a tool's output had to reimplement it — and therefore reimplement its permission checks — and could never reach MCP or semantic tools at all. Adds three Janet functions, modelled on the LSP bridge: (harness/tools?) bridge live? (harness/list-tools) JSON of {name, description, parameters} (harness/call-tool name args) -> @{:ok bool :output string} src/plugin/tool_bridge.rs holds the policy and the tokio responder; the FFI sits in worker.rs with the other C functions. The registry is republished on every agent build rather than captured once, since the agent is rebuilt at run boundaries and MCP tools attach late. Permission checks are unaffected: check_perm* runs inside each tool, so dispatching straight to LoopTool::execute keeps the gate. Verified — under --restrictive a plugin's bash and write return "Permission denied by user" and nothing runs. Two calls are refused rather than attempted, both of which would hang: plugin-registered tools (their handlers need the Janet worker, which is blocked awaiting the reply) and `task` (subagents run isolated from plugin hooks). Hooks do not fire for bridged calls, for the same re-entrancy reason maki's Emit::Silent exists. Two deadlocks found by running it, not by reading it: - The bridge must not touch the PluginManager lock. The hook dispatcher holds it across the Janet call and it is not reentrant, so asking for it from a harness C function — or from the responder the worker is blocked on — hangs the agent. Plugin tool names are now cached from the build path instead. - Headless --print dispatched on-prompt inline on the runtime thread, and the runtime is flavor = "current_thread". Any harness bridge that waits on a reply from the runtime could never be answered. Now dispatched via spawn_blocking, matching the tool hooks and the TUI path. This also fixes harness/lsp and harness/confirm from on-prompt under -p. Adds docs/plugins.md coverage, plugins/call_tool_example.janet, refusal and flatten unit tests, and Janet-level tests that the symbols exist and degrade to nil when the bridge is unwired. Co-authored-by: Wayne <wayne@grange.la>
…#814) * memory: let a user see and edit what dirge remembers `/memory` had one subcommand, `reload`. There was no way to see what dirge had remembered about you — the store is SQLite with an FTS index, so the options were to ask the agent to call the `memory` tool and hope, or open sqlite3 and risk desyncing the index against the content. Memory is injected verbatim into the system prompt of every session in the project, and under global scope every project, so "you cannot read it" is a real gap. `/memory` now lists the store. `/memory edit` opens it in $EDITOR: reword a line to reword the memory, delete a block to forget it, add a block to record something new. Each entry is anchored on its id, rendered short (`[n7x4bhbp]`) and resolved by prefix. That is the load-bearing part. A memory row carries far more than its text — uid lineage, created_at, use_count, confidence, the supersession audit chain, and the procedural success/failure counters that the post-session expectation pass exists to move. Matching edited text back by content, or applying an edit as delete-then-recreate, silently resets all of it. With the id we UPDATE in place through the same path `replace_entry` already used, so it survives. `replace_entry`'s body is split into `apply_replacement` so the substring and by-id paths share one definition of what replacing means, rather than growing a second one that drifts. Deleting a block tombstones rather than destroys, so `restore` still works — removing a line in an editor should not be more destructive than the tool's own removal. An unparseable document aborts the whole edit intact; aborting the editor (`:cq`) changes nothing. $EDITOR handling is extracted from `Input::open_in_external_editor` into `ui::external_editor` and shared: the O_EXCL temp file, the /dev/tty fd juggling and the git-style argv are each easy to get subtly wrong twice. `/edit` should be retested by hand. Verified end to end against the built binary. Two bugs found by running it, both now covered by tests: a new entry written as `[identity] ...` — which the document's own header invites — was read as an unknown id; and because the document always echoes the kind back, every rewording looked like a re-classification and reset the outcome counters. A rewording now preserves uid, use_count, confidence and success_count while changing the text. * gate the /memory edit path to unix for the windows build render/parse/apply and the *_by_uid store methods are only reached from /memory edit, which needs $EDITOR (edit_text is unix-only), so windows flagged them dead. summarize stays cross-platform — the /memory listing works everywhere. also drops a needless borrow clippy flagged. * gate the SqliteMemoryStore import to unix too apply() was its only consumer left on windows after the previous commit. --------- Co-authored-by: Wayne <wayne@grange.la> Co-authored-by: Yogthos <yogthos@gmail.com>
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.
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.
yogthos
force-pushed
the
feat/memory-review
branch
from
August 24, 2026 14:40
35b315a to
5ff6e9d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review copy inside the fork — not a PR against
dirge-code/dirge.dirge decides what to remember 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.
memory.confirm_writes(default off) queuesmemory addinstead./memory reviewopens the queue in$EDITOR.The file is the desired final state of the batch, not a diff — so reject
(delete the block), reword (edit it) and add (type a new one) are one
operation, and recording what the model missed is first-class rather than an
afterthought. Accepted entries go in through the normal
add_entrypath, so amemory you typed is indistinguishable from one the model proposed.
A queued entry rides on
status = 'pending'. Every read path already filtersstatus = 'active'— the prompt snapshot,view, and the FTS join insearch— so a proposal is inert everywhere with no new filtering and nomigration. It also skips hot-tier compaction: a merely proposed memory must not
demote an accepted one before anyone agreed to keep it.
Only
addis gated.replace/supersede/removeact on entries a humanalready accepted, and
supersedeusually fires because the user just correctedthe agent.
$EDITORhandling is extracted fromInput::open_in_external_editorintoui::external_editorand shared rather than copied — theO_EXCLtemp file,the
/dev/ttyfd juggling and the git-style argv are each easy to get subtlywrong twice.
/editshould be retested by hand.Verified end to end against the built binary, including both rendered
lines:
An aborted edit (
:cq) leaves queue and store untouched; an unparseable fileaborts the apply intact; with the gate off, behaviour is unchanged.
Rebased onto
33c95c5d, builds clean both with and without the pluginfeature.
cargo testhas never been run (the dev box OOMs linking the testbinary);
cargo check --profile testpasses.